[PATCH] flashrom: Differentiate SPI errors

[Carl-Daniel Hailfinger]

There are various reasons why a SPI command can fail. Among others, I
have seen the following problems:
- The SPI opcode is not supported by the controller. ICH-style
controllers exhibit this if SPI config is locked down.
- The address in in a prohibited area. This can happen on ICH for any
access (BBAR) and for writes in chipset write protected areas.
- There is no SPI controller.

Introduce separate error codes for unsupported opcode and prohibited
address.

Add the ability to adjust REMS and RES addresses to the minium supported
read address with the help of spi_get_valid_read_addr(). That function
needs to call SPI controller specific functions like reading BBAR on ICH.

Signed-off-by: Carl-Daniel Hailfinger

Index: flashrom-ichspi_better_errorcodes/flash.h
===================================================================
--- flashrom-ichspi_better_errorcodes/flash.h	(Revision 495)
+++ flashrom-ichspi_better_errorcodes/flash.h	(Arbeitskopie)
@@ -640,6 +640,7 @@
 void spi_byte_program(int address, uint8_t byte);
 int spi_nbyte_read(int address, uint8_t *bytes, int len);
 int spi_aai_write(struct flashchip *flash, uint8_t *buf);
+uint32_t spi_get_valid_read_addr(void);

/* 82802ab.c */
 int probe_82802ab(struct flashchip *flash);
Index: flashrom-ichspi_better_errorcodes/spi.c
===================================================================
--- flashrom-ichspi_better_errorcodes/spi.c	(Revision 495)
+++ flashrom-ichspi_better_errorcodes/spi.c	(Arbeitskopie)
@@ -57,9 +57,11 @@
 static int spi_rdid(unsigned char *readarr, int bytes)
 {
    const unsigned char cmd[JEDEC_RDID_OUTSIZE] = { JEDEC_RDID };
+	int ret;

-	if (spi_command(sizeof(cmd), bytes, cmd, readarr))
-		return 1;
+	ret = spi_command(sizeof(cmd), bytes, cmd, readarr);
+	if (ret)
+		return ret;
    printf_debug("RDID returned %02x %02x %02x.\n", readarr[0], readarr[1],
    	     readarr[2]);
    return 0;
@@ -67,20 +69,42 @@

static int spi_rems(unsigned char *readarr)
 {
-	const unsigned char cmd[JEDEC_REMS_OUTSIZE] = { JEDEC_REMS, 0, 0, 0 };
+	unsigned char cmd[JEDEC_REMS_OUTSIZE] = { JEDEC_REMS, 0, 0, 0 };
+	uint32_t readaddr;
+	int ret;

-	if (spi_command(sizeof(cmd), JEDEC_REMS_INSIZE, cmd, readarr))
-		return 1;
+	ret = spi_command(sizeof(cmd), JEDEC_REMS_INSIZE, cmd, readarr);
+	if (ret == SPI_INVALID_ADDRESS) {
+		/* Find the lowest even address allowed for reads. */
+		readaddr = (spi_get_valid_read_addr() + 1) & ~1;
+		cmd[1] = (readaddr >> 16) & 0xff,
+		cmd[2] = (readaddr >> 8) & 0xff,
+		cmd[3] = (readaddr >> 0) & 0xff,
+		ret = spi_command(sizeof(cmd), JEDEC_REMS_INSIZE, cmd, readarr);
+	}
+	if (ret)
+		return ret;
    printf_debug("REMS returned %02x %02x.\n", readarr[0], readarr[1]);
    return 0;
 }

static int spi_res(unsigned char *readarr)
 {
-	const unsigned char cmd[JEDEC_RES_OUTSIZE] = { JEDEC_RES, 0, 0, 0 };
+	unsigned char cmd[JEDEC_RES_OUTSIZE] = { JEDEC_RES, 0, 0, 0 };
+	uint32_t readaddr;
+	int ret;

-	if (spi_command(sizeof(cmd), JEDEC_RES_INSIZE, cmd, readarr))
-		return 1;
+	ret = spi_command(sizeof(cmd), JEDEC_RES_INSIZE, cmd, readarr);
+	if (ret == SPI_INVALID_ADDRESS) {
+		/* Find the lowest even address allowed for reads. */
+		readaddr = (spi_get_valid_read_addr() + 1) & ~1;
+		cmd[1] = (readaddr >> 16) & 0xff,
+		cmd[2] = (readaddr >> 8) & 0xff,
+		cmd[3] = (readaddr >> 0) & 0xff,
+		ret = spi_command(sizeof(cmd), JEDEC_RES_INSIZE, cmd, readarr);
+	}
+	if (ret)
+		return ret;
    printf_debug("RES returned %02x.\n", readarr[0]);
    return 0;
 }
@@ -248,13 +272,16 @@
 {
    const unsigned char cmd[JEDEC_RDSR_OUTSIZE] = { JEDEC_RDSR };
    unsigned char readarr[2]; /* JEDEC_RDSR_INSIZE=1 but wbsio needs 2 */
+	int ret;

/* Read Status Register */
    if (flashbus == BUS_TYPE_SB600_SPI) {
    	/* SB600 uses a different way to read status register. */
    	return sb600_read_status_register();
    } else {
-		spi_command(sizeof(cmd), sizeof(readarr), cmd, readarr);
+		ret = spi_command(sizeof(cmd), sizeof(readarr), cmd, readarr);
+		if (ret)
+			printf_debug("RDSR failed!\n");
    }

return readarr[0];
@@ -666,6 +693,12 @@
    return 1;
 }

+uint32_t spi_get_valid_read_addr(void)
+{
+	/* Need to return BBAR for ICH chipsets. */
+	return 0;
+}
+
 int spi_aai_write(struct flashchip *flash, uint8_t *buf)
 {
    uint32_t pos = 2, size = flash->total_size * 1024;
Index: flashrom-ichspi_better_errorcodes/spi.h
===================================================================
--- flashrom-ichspi_better_errorcodes/spi.h	(Revision 495)
+++ flashrom-ichspi_better_errorcodes/spi.h	(Arbeitskopie)
@@ -105,4 +105,8 @@
 #define JEDEC_BYTE_PROGRAM_OUTSIZE	0x05
 #define JEDEC_BYTE_PROGRAM_INSIZE	0x00

+/* Error codes */
+#define SPI_INVALID_OPCODE	-2
+#define SPI_INVALID_ADDRESS	-3
+
 #endif		/* !__SPI_H__ */
Index: flashrom-ichspi_better_errorcodes/ichspi.c
===================================================================
--- flashrom-ichspi_better_errorcodes/ichspi.c	(Revision 495)
+++ flashrom-ichspi_better_errorcodes/ichspi.c	(Arbeitskopie)
@@ -767,7 +767,7 @@
    /* unknown / not programmed command */
    if (opcode_index == -1) {
    	printf_debug("Invalid OPCODE 0x%02x\n", cmd);
-		return 1;
+		return SPI_INVALID_OPCODE;
    }

opcode = &(curopcodes->opcode[opcode_index]);

[Uwe Hermann]

On Mon, May 11, 2009 at 07:40:39PM +0200, Carl-Daniel Hailfinger wrote:
> There are various reasons why a SPI command can fail. Among others, I
> have seen the following problems:
> The SPI opcode is not supported by the controller. ICH-style
>
> controllers exhibit this if SPI config is locked down.
> The address in in a prohibited area. This can happen on ICH for any
>
> access (BBAR) and for writes in chipset write protected areas.
> There is no SPI controller.
>
> Introduce separate error codes for unsupported opcode and prohibited
> address.
>
> Add the ability to adjust REMS and RES addresses to the minium supported
> read address with the help of spi_get_valid_read_addr(). That function
> needs to call SPI controller specific functions like reading BBAR on ICH.
>
> Signed-off-by: Carl-Daniel Hailfinger

Acked-by: Uwe Hermann

Uwe.

[Carl-Daniel Hailfinger]

On 12.05.2009 17:14, Uwe Hermann wrote:
> On Mon, May 11, 2009 at 07:40:39PM +0200, Carl-Daniel Hailfinger wrote:
> > There are various reasons why a SPI command can fail. Among others, I
> > have seen the following problems:
> > The SPI opcode is not supported by the controller. ICH-style
> >
> > controllers exhibit this if SPI config is locked down.
> > The address in in a prohibited area. This can happen on ICH for any
> >
> > access (BBAR) and for writes in chipset write protected areas.
> > There is no SPI controller.
> >
> > Introduce separate error codes for unsupported opcode and prohibited
> > address.
> >
> > Add the ability to adjust REMS and RES addresses to the minium supported
> > read address with the help of spi_get_valid_read_addr(). That function
> > needs to call SPI controller specific functions like reading BBAR on ICH.
> >
> > Signed-off-by: Carl-Daniel Hailfinger
>
> Acked-by: Uwe Hermann

Thanks, committed in r500.

Regards,
Carl-Daniel