summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorSubrata Banik <subratabanik@google.com>2022-03-16 20:40:42 +0530
committerFelix Singer <felixsinger@posteo.net>2022-10-30 09:43:35 +0000
commit7cb43957c5fe405cd82584f0a54428f2d2d286ff (patch)
tree9981905ba97c0509e8686782855d97cffe77d80e
parent4cbc1cb32dec2a269a6c2fb8d391f36db174bb32 (diff)
downloadflashrom-1.2.x.tar.gz
flashrom-1.2.x.tar.bz2
flashrom-1.2.x.zip
ichspi: Unify timeouts across all SPI operations to 30sv1.2.1-rc1v1.2.11.2.x
Note: This patch was backported from the master branch and it was modified so that it can be applied on the 1.2.x branch. `ich_hwseq_wait_for_cycle_complete()` drops taking `timeout` as argument in favor of a fixed timeout of `30 seconds` for any given SPI operation as recommended by the SPI programming guide. Document: Alder Lake-P Client Platform SPI Programming Guide Rev 1.30 (supporting document for multi-master accessing the SPI Flash device.) Refer to below section to understand the problem in more detail and SPI operation timeout recommendation from Intel in multi-master scenarios. On Intel Chipsets that support multi-mastering access of the SPI flash may run into a timeout failure when the operation initiated from a single master just follows the SPI operational timeout recommendation as per the vendor datasheet (example: winbond spiflash W25Q256JV-DTR specification, table 9.7). In the multi-master SPI accessing scenario using hardware sequencing operation, it's impossible to know the actual status of the SPI bus prior to individual master starting the operation (SPI Cycle In Progress a.k.a SCIP bit represents the status of SPI operation on individual master). Thus, any SPI operation triggered in multi-master environment might need to account a worst case scenario where the most time consuming operation might have occupied the SPI bus from a master and an operation initiated by another master just timed out. Here is the timeout calculation for any hardware sequencing operation: Worst Case Operational Delay = (Maximum Time consumed by a SPI operation + Any marginal adjustment) Timeout Recommendation for Hardware Sequencing Operation = ((Worst Case Operational Delay) * (#No. Of SPI Master - 1) + Current Operational latency) Assume, on Intel platform with 6 SPI master like, Host CPU, CSE, EC, GbE and other reserved etc, hence, the Timeout Calculation for SPI erase Operation would look like as below: Maximum Time consumed by a SPI Operation = 5 seconds Worst Case Operational Delay = 5 seconds Timeout Recommendation for Hardware Seq Operation = 5 seconds * (6 - 1) + 5 seconds = 30 seconds BUG=b:223630977 TEST=Able to perform read/write/erase operation on PCH 600 series chipset (board name: Brya). Original-Signed-off-by: Subrata Banik <subratabanik@google.com> Original-Reviewed-on: https://review.coreboot.org/c/flashrom/+/62867 Original-Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Original-Reviewed-by: Anastasia Klimchuk <aklm@chromium.org> Original-Reviewed-by: Arthur Heymans <arthur@aheymans.xyz> Original-Reviewed-by: Edward O'Callaghan <quasisec@chromium.org> Change-Id: Ifa910dea794175d8ee2ad277549e5a0d69cba45b Signed-off-by: Felix Singer <felixsinger@posteo.net> Reviewed-on: https://review.coreboot.org/c/flashrom/+/68691 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Michael Niewöhner <foss@mniewoehner.de>
-rw-r--r--ichspi.c23
1 files changed, 12 insertions, 11 deletions
diff --git a/ichspi.c b/ichspi.c
index 12ee126fb..725bfb367 100644
--- a/ichspi.c
+++ b/ichspi.c
@@ -1282,20 +1282,24 @@ static uint32_t ich_hwseq_get_erase_block_size(unsigned int addr)
Resets all error flags in HSFS.
Returns 0 if the cycle completes successfully without errors within
timeout us, 1 on errors. */
-static int ich_hwseq_wait_for_cycle_complete(unsigned int timeout,
- unsigned int len)
+static int ich_hwseq_wait_for_cycle_complete(unsigned int len)
{
+ /*
+ * The SPI bus may be busy due to performing operations from other masters, hence
+ * introduce the long timeout of 30s to cover the worst case scenarios as well.
+ */
+ unsigned int timeout_us = 30 * 1000 * 1000;
uint16_t hsfs;
uint32_t addr;
- timeout /= 8; /* scale timeout duration to counter */
+ timeout_us /= 8; /* scale timeout duration to counter */
while ((((hsfs = REGREAD16(ICH9_REG_HSFS)) &
(HSFS_FDONE | HSFS_FCERR)) == 0) &&
- --timeout) {
+ --timeout_us) {
programmer_delay(8);
}
REGWRITE16(ICH9_REG_HSFS, REGREAD16(ICH9_REG_HSFS));
- if (!timeout) {
+ if (!timeout_us) {
addr = REGREAD32(ICH9_REG_FADDR) & hwseq_data.addr_mask;
msg_perr("Timeout error between offset 0x%08x and "
"0x%08x (= 0x%08x + %d)!\n",
@@ -1378,7 +1382,6 @@ static int ich_hwseq_block_erase(struct flashctx *flash, unsigned int addr,
{
uint32_t erase_block;
uint16_t hsfc;
- uint32_t timeout = 5000 * 1000; /* 5 s for max 64 kB */
erase_block = ich_hwseq_get_erase_block_size(addr);
if (len != erase_block) {
@@ -1418,7 +1421,7 @@ static int ich_hwseq_block_erase(struct flashctx *flash, unsigned int addr,
prettyprint_ich9_reg_hsfc(hsfc);
REGWRITE16(ICH9_REG_HSFC, hsfc);
- if (ich_hwseq_wait_for_cycle_complete(timeout, len))
+ if (ich_hwseq_wait_for_cycle_complete(len))
return -1;
return 0;
}
@@ -1427,7 +1430,6 @@ static int ich_hwseq_read(struct flashctx *flash, uint8_t *buf,
unsigned int addr, unsigned int len)
{
uint16_t hsfc;
- uint16_t timeout = 100 * 60;
uint8_t block_len;
if (addr + len > flash->chip->total_size * 1024) {
@@ -1455,7 +1457,7 @@ static int ich_hwseq_read(struct flashctx *flash, uint8_t *buf,
hsfc |= HSFC_FGO; /* start */
REGWRITE16(ICH9_REG_HSFC, hsfc);
- if (ich_hwseq_wait_for_cycle_complete(timeout, block_len))
+ if (ich_hwseq_wait_for_cycle_complete(block_len))
return 1;
ich_read_data(buf, block_len, ICH9_REG_FDATA0);
addr += block_len;
@@ -1468,7 +1470,6 @@ static int ich_hwseq_read(struct flashctx *flash, uint8_t *buf,
static int ich_hwseq_write(struct flashctx *flash, const uint8_t *buf, unsigned int addr, unsigned int len)
{
uint16_t hsfc;
- uint16_t timeout = 100 * 60;
uint8_t block_len;
if (addr + len > flash->chip->total_size * 1024) {
@@ -1497,7 +1498,7 @@ static int ich_hwseq_write(struct flashctx *flash, const uint8_t *buf, unsigned
hsfc |= HSFC_FGO; /* start */
REGWRITE16(ICH9_REG_HSFC, hsfc);
- if (ich_hwseq_wait_for_cycle_complete(timeout, block_len))
+ if (ich_hwseq_wait_for_cycle_complete(block_len))
return -1;
addr += block_len;
buf += block_len;