[flashrom] [PATCH] ICH SPI paranoid error handling

[Carl-Daniel Hailfinger]

Some people have reported that running flashrom on Ibex Peak (after
patching in the PCI ID 0x8086:0x3b02) will not only hang flashrom during
the first RES/REMS command issued, it may even cause the machine to lock
up or issue a hard reset. We suspect this is because RES/REMS are
commands with an address, whereas RDID does not take an address and
works fine.

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, and especially on
Ibex Peak. It probably won't fix the hang, but it should be pretty good
at spewing an error before the machine dies.

Untested. Highly likely to explode on VIA chipsets which use the same
interface but have docs only under NDA.

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

Index: flashrom-ich_spi_fcerr_fcdone_paranoia/chipset_enable.c
===================================================================
--- flashrom-ich_spi_fcerr_fcdone_paranoia/chipset_enable.c	(Revision 1052)
+++ flashrom-ich_spi_fcerr_fcdone_paranoia/chipset_enable.c	(Arbeitskopie)
@@ -579,8 +579,20 @@
 			     mmio_readl(spibar + 0x80));
 		msg_pdbg("0x84: 0x%08x (PR4)\n",
 			     mmio_readl(spibar + 0x84));
-		msg_pdbg("0x90: 0x%08x (SSFS, SSFC)\n",
-			     mmio_readl(spibar + 0x90));
+
+		tmp = mmio_readl(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, spibar + 0x90);
+		}
+		tmp >>= 8;
+		msg_pdbg("0x91: 0x%06x (SSFC)\n", tmp);
+
 		msg_pdbg("0x94: 0x%04x     (PREOP)\n",
 			     mmio_readw(spibar + 0x94));
 		msg_pdbg("0x96: 0x%04x     (OPTYPE)\n",
Index: flashrom-ich_spi_fcerr_fcdone_paranoia/ichspi.c
===================================================================
--- flashrom-ich_spi_fcerr_fcdone_paranoia/ichspi.c	(Revision 1052)
+++ flashrom-ich_spi_fcerr_fcdone_paranoia/ichspi.c	(Arbeitskopie)
@@ -413,6 +413,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 */
 
@@ -486,18 +492,22 @@
 	/* 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");
+		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;
 	}
 
@@ -532,6 +542,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 */
 
@@ -605,18 +621,23 @@
 	/* 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");
+		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;
 	}
 


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