Patchworkβ ICH SPI paranoid error handling

login
register
about
Submitter Carl-Daniel Hailfinger
Date 2010-06-20 21:52:12
Message ID <4C1E8D8C.2000409@gmx.net>
Download mbox | patch
Permalink /patch/1532/
State Superseded
Headers show

Comments

Carl-Daniel Hailfinger - 2010-06-20 21:52:12
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@gmx.net>
Rudolf Marek - 2010-06-21 07:20:01
> Untested. Highly likely to explode on VIA chipsets which use the same
> interface but have docs only under NDA.

No true. Check the VX800 datasheet, page 445 (in xpdf).

http://linux.via.com.tw/support/beginDownload.action?eleid=161&fid=241

I can test it this evening, Differences between ICH7 and VIA
are described in source code:

/*
  VIA SPI is compatible with ICH7, but maxdata
    to transfer is 16 bytes.

    DATA byte count on ICH7 is 8:13, on VIA 8:11

    bit 12 is port select CS0 CS1
    bit 13 is FAST READ enable
    bit 7  is used with fast read and one shot controls CS de-assert?
*/

Thanks,
Rudolf
Rudolf Marek - 2010-06-23 19:58:12
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1


It works on VT8237S, or at least it works while trying -v -w -r

Thanks,
Rudolf
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEUEARECAAYFAkwiZ1MACgkQ3J9wPJqZRNUMFACeKHSZjJYI8Jw+RmrUTxyh82LL
j/4AmNnZIEAo36L/o2rRj2UJvq4NuS0=
=vhO8
-----END PGP SIGNATURE-----

Patch

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