There are various reasons why a SPI command can fail. Amon 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.
Signed-off-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net
Index: flashrom-ichspi_better_errorcodes/spi.h =================================================================== --- flashrom-ichspi_better_errorcodes/spi.h (revision 3773) +++ flashrom-ichspi_better_errorcodes/spi.h (working copy) @@ -90,4 +90,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 3773) +++ flashrom-ichspi_better_errorcodes/ichspi.c (working copy) @@ -632,7 +632,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]);
On Fri, Nov 28, 2008 at 1:45 AM, Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net wrote:
+/* Error codes */ +#define SPI_INVALID_OPCODE 2 +#define SPI_INVALID_ADDRESS 3
I did meet access block problem because of reading addresses below BBA, when trying to implement sst25lf040a support. I was given "Transaction error", and it took me a little while to figure out the reason.
/* unknown / not programmed command */ if (opcode_index == -1) { printf_debug("Invalid OPCODE 0x%02x\n", cmd);
return 1;
return SPI_INVALID_OPCODE;
Looking forward to more diagnostic output.
yu ning
On 27.11.2008 19:46, FENG Yu Ning wrote:
On Fri, Nov 28, 2008 at 1:45 AM, Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net wrote:
+/* Error codes */ +#define SPI_INVALID_OPCODE 2 +#define SPI_INVALID_ADDRESS 3
I did meet access block problem because of reading addresses below BBA, when trying to implement sst25lf040a support. I was given "Transaction error", and it took me a little while to figure out the reason.
/* unknown / not programmed command */ if (opcode_index == -1) { printf_debug("Invalid OPCODE 0x%02x\n", cmd);
return 1;
return SPI_INVALID_OPCODE;Looking forward to more diagnostic output.
New patch: Improve error handling even more. 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 c-d.hailfinger.devel.2006@gmx.net
Index: flashrom-ichspi_better_errorcodes/flash.h =================================================================== --- flashrom-ichspi_better_errorcodes/flash.h (Revision 3775) +++ flashrom-ichspi_better_errorcodes/flash.h (Arbeitskopie) @@ -467,6 +467,7 @@ int spi_disable_blockprotect(void); void spi_byte_program(int address, uint8_t byte); int spi_nbyte_read(int address, uint8_t *bytes, int len); +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 3775) +++ flashrom-ichspi_better_errorcodes/spi.c (Arbeitskopie) @@ -53,9 +53,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; @@ -63,20 +65,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; } @@ -227,9 +251,12 @@ { const unsigned char cmd[JEDEC_RDSR_OUTSIZE] = { JEDEC_RDSR }; unsigned char readarr[JEDEC_RDSR_INSIZE]; + int ret;
/* Read Status Register */ - 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]; }
@@ -562,3 +589,9 @@
return 1; } + +uint32_t spi_get_valid_read_addr(void) +{ + /* Need to return BBAR for ICH chipsets. */ + return 0; +} Index: flashrom-ichspi_better_errorcodes/spi.h =================================================================== --- flashrom-ichspi_better_errorcodes/spi.h (Revision 3775) +++ flashrom-ichspi_better_errorcodes/spi.h (Arbeitskopie) @@ -95,4 +95,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 3775) +++ flashrom-ichspi_better_errorcodes/ichspi.c (Arbeitskopie) @@ -632,7 +632,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]);
On Fri, Nov 28, 2008 at 10:00 AM, Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net wrote:
New patch: Improve error handling even more. 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 c-d.hailfinger.devel.2006@gmx.net
Index: flashrom-ichspi_better_errorcodes/spi.c
--- flashrom-ichspi_better_errorcodes/spi.c (Revision 3775) +++ flashrom-ichspi_better_errorcodes/spi.c (Arbeitskopie) @@ -53,9 +53,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;
Maybe "rc" sounds better? That is also the name we use elsewhere. A minor problem, though.
@@ -63,20 +65,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;}
Just after IRC communication, I suddenly realized that ICH7(or the flash chip ctrl'ed by it) translate addresses in this way:
actual_addr = 0x 1 00 00 00 - total_size + addr_in_cmd.
Two facts support the above guess: 1. the ich7 spec(307013) says the following in 5.25.3.1: "Serial flash device must ignore the upper address bits such that an address of FF FF FFh simply aliases to the top of the flash memory." 2. I issued the Read ID cmd with address 0x f8 00 00 to the 512K sst25lf040a and got the expected result when its data sheet said the address should be 0x 00 00 00.
Maybe we could change those zero addresses to better ones for ich spi commands.
yu ning
-
Carl-Daniel Hailfinger -
FENG Yu Ning