Change in flashrom[master]: ichspi: Unified timeout (5sec) across all SPI operations

[Subrata Banik (Code Review)]

Subrata Banik has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/62867 )

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

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 
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;