Subrata Banik has uploaded this change for review.

View Change

ichspi: Unified timeout (5sec) across all SPI operations

This patch removes taking `timeout` argument for different operations
using `ich_hwseq_wait_for_cycle_complete()`. Make use of 5sec unified
timeout for all SPI operations.

Document Number: TBD (supporting document for multi-master accessing
the SPI Flash device is awaited from Intel.)

BUG=b:223630977
TEST=Able to perform read/write/erase/status operation on brya.

Signed-off-by: Subrata Banik <subratabanik@google.com>
Change-Id: Ifa910dea794175d8ee2ad277549e5a0d69cba45b
---
M ichspi.c
1 file changed, 37 insertions(+), 7 deletions(-)

git pull ssh://review.coreboot.org:29418/flashrom refs/changes/67/62867/1
diff --git a/ichspi.c b/ichspi.c
index 08ca038..dc03422 100644
--- a/ichspi.c
+++ b/ichspi.c
@@ -32,6 +32,38 @@

/* Sunrise Point */

+/*
+ * On Intel Chipsets that support multi-master accessing the SPI flash might run
+ * into a timeout failure when the operation initiated from a single master just
+ * following the SPI operational timeout recommendation by the vendor datasheet (example:
+ * winbond spiflash W25Q256JV-DTR specification, table 9.7).
+ *
+ * At the underlying level of hardware sequencing operation, it's impossible to know
+ * the actual status of the SPI bus from a master prior to starting the operation.
+ * 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 2 SPI master like, Host CPU and CSE, the Timeout
+ * Calculation for SPI Write Operation would look like:
+ *
+ * Maximum Time consumed by a SPI Operation = 2 seconds
+ * (for SPI Erase Operation as per Winbond Data Sheet)
+ * Worst Case Operational Delay = (2 seconds + 50 milliseconds) =
+ * 2 seconds 50 milliseconds
+ * Timeout Recommendation for Hardware Seq Operation =
+ * (2 seconds 50 milliseconds) * (2 - 1) + 15 milliseconds
+ * (for SPI Write Operation as per Winbond Data Sheet)
+ * = 2 seconds 65 milliseconds
+ */
+#define ICH_HWSEQ_XFER_TIMEOUT_US 5000000 /* 5 seconds */
/* Added HSFS Status bits */
#define HSFS_WRSDIS_OFF 11 /* 11: Flash Configuration Lock-Down */
#define HSFS_WRSDIS (0x1 << HSFS_WRSDIS_OFF)
@@ -1275,8 +1307,9 @@
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)
{
+ unsigned int timeout = ICH_HWSEQ_XFER_TIMEOUT_US;
uint16_t hsfs;
uint32_t addr;

@@ -1371,7 +1404,6 @@
{
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) {
@@ -1415,7 +1447,7 @@
prettyprint_ich9_reg_hsfc(hsfc, ich_generation);
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;
}
@@ -1424,7 +1456,6 @@
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) {
@@ -1458,7 +1489,7 @@
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;
@@ -1471,7 +1502,6 @@
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) {
@@ -1506,7 +1536,7 @@
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;

To view, visit change 62867. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Ifa910dea794175d8ee2ad277549e5a0d69cba45b
Gerrit-Change-Number: 62867
Gerrit-PatchSet: 1
Gerrit-Owner: Subrata Banik <subratabanik@google.com>
Gerrit-MessageType: newchange