[flashrom] [PATCH] ICH SPI paranoid error handling

[Carl-Daniel Hailfinger]

On 23.06.2010 21:58, Rudolf Marek wrote:
> It works on VT8237S, or at least it works while trying -v -w -r

Great. Updated version follows.

Our current ICH SPI code doesn't really follow the datasheets and uses
lots of hope instead of error checking in the wrong places.

This patch will fail early and fail often if something is odd. As such,
merge is not recommended, but running it should definitely generating
interesting results on all Intel chipsets with SPI.
It probably won't fix the hang, but it should be pretty good
at spewing an error before the machine dies.

Tested with VT8237S SPI.


Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006 at gmx.net>

Index: flashrom-ich_spi_fcerr_fcdone_paranoia/ichspi.c
===================================================================
--- flashrom-ich_spi_fcerr_fcdone_paranoia/ichspi.c	(Revision 1217)
+++ flashrom-ich_spi_fcerr_fcdone_paranoia/ichspi.c	(Arbeitskopie)
@@ -485,6 +485,12 @@
 		write_cmd = 1;
 	}
 
+	/* FIXME: This should be a time-limited while loop. */
+	if (REGREAD16(ICH7_REG_SPIS) & SPIS_SCIP) {
+		msg_perr("Error: Totally unhandled SCIP condition!\n");
+		return 1;
+	}
+
 	/* Programm Offset in Flash into FADDR */
 	REGWRITE32(ICH7_REG_SPIA, (offset & 0x00FFFFFF));	/* SPI addresses are 24 BIT only */
 
@@ -558,18 +564,23 @@
 	/* write it */
 	REGWRITE16(ICH7_REG_SPIC, temp16);
 
-	/* wait for cycle complete */
-	timeout = 100 * 1000 * 60;	// 60s is a looong timeout.
-	while (((REGREAD16(ICH7_REG_SPIS) & SPIS_CDS) == 0) && --timeout) {
+	/* Wait for Cycle Done Status or Flash Cycle Error. */
+	timeout = 100 * 60;	/* 60 ms are 9.6 million cycles at 16 MHz. */
+	while (((REGREAD16(ICH7_REG_SPIS) & (SPIS_CDS | SPIS_FCERR)) == 0) &&
+	       --timeout) {
 		programmer_delay(10);
 	}
 	if (!timeout) {
-		msg_perr("timeout\n");
+		msg_perr("timeout, ICH7_REG_SPIS=0x%04x\n",
+			 REGREAD16(ICH7_REG_SPIS));
+		return 1;
 	}
 
 	/* FIXME: make sure we do not needlessly cause transaction errors. */
-	if ((REGREAD16(ICH7_REG_SPIS) & SPIS_FCERR) != 0) {
-		msg_pdbg("Transaction error!\n");
+	if (REGREAD16(ICH7_REG_SPIS) & SPIS_FCERR) {
+		msg_perr("Transaction error for opcode 0x%02x!\n",
+			 op.opcode);
+		REGWRITE16(ICH7_REG_SPIS, SPIS_FCERR);
 		return 1;
 	}
 
@@ -604,6 +615,12 @@
 		write_cmd = 1;
 	}
 
+	/* FIXME: This should be a time-limited while loop. */
+	if (REGREAD16(ICH9_REG_SSFS) & SSFS_SCIP) {
+		msg_perr("Error: Totally unhandled SCIP condition!\n");
+		return 1;
+	}
+
 	/* Programm Offset in Flash into FADDR */
 	REGWRITE32(ICH9_REG_FADDR, (offset & 0x00FFFFFF));	/* SPI addresses are 24 BIT only */
 
@@ -679,18 +696,24 @@
 	/* write it */
 	REGWRITE32(ICH9_REG_SSFS, temp32);
 
-	/*wait for cycle complete */
-	timeout = 100 * 1000 * 60;	// 60s is a looong timeout.
-	while (((REGREAD32(ICH9_REG_SSFS) & SSFS_CDS) == 0) && --timeout) {
+	/* Wait for Cycle Done Status or Flash Cycle Error. */
+	timeout = 100 * 60;	/* 60 ms are 9.6 million cycles at 16 MHz. */
+	while (((REGREAD32(ICH9_REG_SSFS) & (SSFS_CDS | SSFS_FCERR)) == 0) &&
+	       --timeout) {
 		programmer_delay(10);
 	}
 	if (!timeout) {
-		msg_perr("timeout\n");
+		msg_perr("timeout, ICH9_REG_SSFS=0x%08x\n",
+			 REGREAD32(ICH9_REG_SSFS));
+		return 1;
 	}
 
 	/* FIXME make sure we do not needlessly cause transaction errors. */
-	if ((REGREAD32(ICH9_REG_SSFS) & SSFS_FCERR) != 0) {
-		msg_pdbg("Transaction error!\n");
+	if (REGREAD32(ICH9_REG_SSFS) & SSFS_FCERR) {
+		msg_perr("Transaction error for opcode 0x%02x!\n",
+			 op.opcode);
+		//FIXME: Really REGWRITE8?
+		REGWRITE8(ICH9_REG_SSFS, SSFS_FCERR);
 		return 1;
 	}
 
@@ -1069,8 +1092,20 @@
 			     mmio_readl(ich_spibar + 0x80));
 		msg_pdbg("0x84: 0x%08x (PR4)\n",
 			     mmio_readl(ich_spibar + 0x84));
-		msg_pdbg("0x90: 0x%08x (SSFS, SSFC)\n",
-			     mmio_readl(ich_spibar + 0x90));
+
+		tmp = mmio_readl(ich_spibar + 0x90);
+		msg_pdbg("0x90: 0x%02x (SSFS)\n", tmp & 0xff);
+		msg_pdbg("AEL %i, ", (tmp >> 4) & 1);
+		msg_pdbg("FCERR %i, ", (tmp >> 3) & 1);
+		msg_pdbg("FDONE %i, ", (tmp >> 2) & 1);
+		msg_pdbg("SCIP %i\n", (tmp >> 0) & 1);
+		if (tmp & (1 << 3)) {
+			msg_pdbg("Clearing SSFS.FCERR\n");
+			mmio_writeb(1 << 3, ich_spibar + 0x90);
+		}
+		tmp >>= 8;
+		msg_pdbg("0x91: 0x%06x (SSFC)\n", tmp);
+
 		msg_pdbg("0x94: 0x%04x     (PREOP)\n",
 			     mmio_readw(ich_spibar + 0x94));
 		msg_pdbg("0x96: 0x%04x     (OPTYPE)\n",


-- 
http://www.hailfinger.org/