[flashrom] [PATCH] ICH SPI paranoid error handling
Stefan Reinauer
stefan.reinauer at coreboot.org
Wed Mar 16 23:31:30 CET 2011
* Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006 at gmx.net> [101125 00:45]:
> New version.
>
> Proper error handling for ICH/VIA SPI:
> Use 16-bit values for bit masks in 16-bit registers.
> Check for SPI Cycle In Progress and wait up to 60 ms.
> Do not touch reserved bits.
> Reduce SPI cycle timeout from 60 s to 60 ms.
> Clear transaction errors caused by our own SPI accesses.
> Add better debugging in case the hardware misbehaves.
>
> Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006 at gmx.net>
Acked-by: Stefan Reinauer <stefan.reinauer at coreboot.org>
> Index: flashrom-ich_spi_fcerr_fcdone_paranoia/ichspi.c
> ===================================================================
> --- flashrom-ich_spi_fcerr_fcdone_paranoia/ichspi.c (Revision 1236)
> +++ flashrom-ich_spi_fcerr_fcdone_paranoia/ichspi.c (Arbeitskopie)
> @@ -51,6 +51,7 @@
> #define SSFS_CDS 0x00000004
> #define SSFS_FCERR 0x00000008
> #define SSFS_AEL 0x00000010
> +#define SSFS_RESERVED_MASK 0x000000e2
>
> #define ICH9_REG_SSFC 0x91 /* 24 Bits */
> #define SSFC_SCGO 0x00000200
> @@ -63,6 +64,7 @@
> #define SSFC_SCF 0x01000000
> #define SSFC_SCF_20MHZ 0x00000000
> #define SSFC_SCF_33MHZ 0x01000000
> +#define SSFC_RESERVED_MASK 0xf8008100
>
> #define ICH9_REG_PREOP 0x94 /* 16 Bits */
> #define ICH9_REG_OPTYPE 0x96 /* 16 Bits */
> @@ -76,9 +78,11 @@
>
> // ICH7 registers
> #define ICH7_REG_SPIS 0x00 /* 16 Bits */
> -#define SPIS_SCIP 0x00000001
> -#define SPIS_CDS 0x00000004
> -#define SPIS_FCERR 0x00000008
> +#define SPIS_SCIP 0x0001
> +#define SPIS_GRANT 0x0002
> +#define SPIS_CDS 0x0004
> +#define SPIS_FCERR 0x0008
> +#define SPIS_RESERVED_MASK 0x7ff0
>
> /* VIA SPI is compatible with ICH7, but maxdata
> to transfer is 16 bytes.
> @@ -146,6 +150,11 @@
> return mmio_readw(ich_spibar + X);
> }
>
> +static uint16_t REGREAD8(int X)
> +{
> + return mmio_readb(ich_spibar + X);
> +}
> +
> #define REGWRITE32(X,Y) mmio_writel(Y, ich_spibar+X)
> #define REGWRITE16(X,Y) mmio_writew(Y, ich_spibar+X)
> #define REGWRITE8(X,Y) mmio_writeb(Y, ich_spibar+X)
> @@ -497,6 +506,15 @@
> write_cmd = 1;
> }
>
> + timeout = 100 * 60; /* 60 ms are 9.6 million cycles at 16 MHz. */
> + while ((REGREAD16(ICH7_REG_SPIS) & SPIS_SCIP) && --timeout) {
> + programmer_delay(10);
> + }
> + if (!timeout) {
> + msg_perr("Error: SCIP never cleared!\n");
> + return 1;
> + }
> +
> /* Programm Offset in Flash into FADDR */
> REGWRITE32(ICH7_REG_SPIA, (offset & 0x00FFFFFF)); /* SPI addresses are 24 BIT only */
>
> @@ -523,7 +541,9 @@
> }
>
> /* Assemble SPIS */
> - temp16 = 0;
> + temp16 = REGREAD16(ICH7_REG_SPIS);
> + /* keep reserved bits */
> + temp16 &= SPIS_RESERVED_MASK;
> /* clear error status registers */
> temp16 |= (SPIS_CDS + SPIS_FCERR);
> REGWRITE16(ICH7_REG_SPIS, temp16);
> @@ -570,18 +590,26 @@
> /* 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");
> + temp16 = REGREAD16(ICH7_REG_SPIS);
> + if (temp16 & SPIS_FCERR) {
> + msg_perr("Transaction error for opcode 0x%02x!\n",
> + op.opcode);
> + /* keep reserved bits */
> + temp16 &= SPIS_RESERVED_MASK;
> + REGWRITE16(ICH7_REG_SPIS, temp16 | SPIS_FCERR);
> return 1;
> }
>
> @@ -616,6 +644,15 @@
> write_cmd = 1;
> }
>
> + timeout = 100 * 60; /* 60 ms are 9.6 million cycles at 16 MHz. */
> + while ((REGREAD8(ICH9_REG_SSFS) & SSFS_SCIP) && --timeout) {
> + programmer_delay(10);
> + }
> + if (!timeout) {
> + msg_perr("Error: SCIP never cleared!\n");
> + return 1;
> + }
> +
> /* Programm Offset in Flash into FADDR */
> REGWRITE32(ICH9_REG_FADDR, (offset & 0x00FFFFFF)); /* SPI addresses are 24 BIT only */
>
> @@ -641,12 +678,13 @@
> }
>
> /* Assemble SSFS + SSFC */
> - /* keep reserved bits (23-19,7,0) */
> temp32 = REGREAD32(ICH9_REG_SSFS);
> - temp32 &= 0xF8008100;
> -
> + /* keep reserved bits */
> + temp32 &= SSFS_RESERVED_MASK | SSFC_RESERVED_MASK;
> /* clear error status registers */
> temp32 |= (SSFS_CDS + SSFS_FCERR);
> + REGWRITE32(ICH9_REG_SSFS, temp32);
> +
> /* Use 20 MHz */
> temp32 |= SSFC_SCF_20MHZ;
>
> @@ -691,18 +729,27 @@
> /* 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");
> + temp32 = REGREAD32(ICH9_REG_SSFS);
> + if (temp32 & SSFS_FCERR) {
> + msg_perr("Transaction error for opcode 0x%02x!\n",
> + op.opcode);
> + /* keep reserved bits */
> + temp32 &= SSFS_RESERVED_MASK | SSFC_RESERVED_MASK;
> + /* Clear the transaction error. */
> + REGWRITE32(ICH9_REG_SSFS, temp32 | SSFS_FCERR);
> return 1;
> }
>
> @@ -1042,7 +1089,6 @@
> msg_pdbg("0x%02x: 0x%08x (PBR%d)\n", offs,
> mmio_readl(ich_spibar + offs), i);
> }
> - msg_pdbg("\n");
> if (mmio_readw(ich_spibar) & (1 << 15)) {
> msg_pinfo("WARNING: SPI Configuration Lockdown activated.\n");
> ichspi_lock = 1;
> @@ -1082,8 +1128,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/
>
>
> _______________________________________________
> flashrom mailing list
> flashrom at flashrom.org
> http://www.flashrom.org/mailman/listinfo/flashrom
>
More information about the flashrom
mailing list