spi_read_status_register now returns success/failure and stores the status register value via call-by-reference. That way, status register reading failure (only possible if the programmer couldn't run RDSR) can be detected and acted upon in the future.
Signed-off-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net
Index: flashrom-spi_rdsr_errorcheck/it87spi.c =================================================================== --- flashrom-spi_rdsr_errorcheck/it87spi.c (Revision 1539) +++ flashrom-spi_rdsr_errorcheck/it87spi.c (Arbeitskopie) @@ -321,6 +321,7 @@ { unsigned int i; int result; + uint8_t status; chipaddr bios = flash->virtual_memory;
result = spi_write_enable(flash); @@ -335,7 +336,7 @@ /* Wait until the Write-In-Progress bit is cleared. * This usually takes 1-10 ms, so wait in 1 ms steps. */ - while (spi_read_status_register(flash) & SPI_SR_WIP) + while (spi_read_status_register(flash, &status), status & SPI_SR_WIP) programmer_delay(1000); return 0; } Index: flashrom-spi_rdsr_errorcheck/a25.c =================================================================== --- flashrom-spi_rdsr_errorcheck/a25.c (Revision 1539) +++ flashrom-spi_rdsr_errorcheck/a25.c (Arbeitskopie) @@ -33,7 +33,7 @@ { uint8_t status;
- status = spi_read_status_register(flash); + spi_read_status_register(flash, &status); msg_cdbg("Chip status register is %02x\n", status);
spi_prettyprint_status_register_amic_a25_srwd(status); @@ -49,7 +49,7 @@ { uint8_t status;
- status = spi_read_status_register(flash); + spi_read_status_register(flash, &status); msg_cdbg("Chip status register is %02x\n", status);
spi_prettyprint_status_register_amic_a25_srwd(status); @@ -64,7 +64,7 @@ { uint8_t status;
- status = spi_read_status_register(flash); + spi_read_status_register(flash, &status); msg_cdbg("Chip status register is %02x\n", status);
spi_prettyprint_status_register_amic_a25_srwd(status); @@ -82,7 +82,7 @@ { uint8_t status;
- status = spi_read_status_register(flash); + spi_read_status_register(flash, &status); msg_cdbg("Chip status register is %02x\n", status);
spi_prettyprint_status_register_amic_a25_srwd(status); Index: flashrom-spi_rdsr_errorcheck/at25.c =================================================================== --- flashrom-spi_rdsr_errorcheck/at25.c (Revision 1539) +++ flashrom-spi_rdsr_errorcheck/at25.c (Arbeitskopie) @@ -61,7 +61,7 @@ { uint8_t status;
- status = spi_read_status_register(flash); + spi_read_status_register(flash, &status); msg_cdbg("Chip status register is %02x\n", status);
spi_prettyprint_status_register_atmel_at25_srpl(status); @@ -84,7 +84,7 @@ { uint8_t status;
- status = spi_read_status_register(flash); + spi_read_status_register(flash, &status); msg_cdbg("Chip status register is %02x\n", status);
spi_prettyprint_status_register_atmel_at25_srpl(status); @@ -103,7 +103,7 @@ { uint8_t status;
- status = spi_read_status_register(flash); + spi_read_status_register(flash, &status); msg_cdbg("Chip status register is %02x\n", status);
msg_cdbg("Chip status register: Status Register Write Protect (WPEN) " @@ -127,7 +127,7 @@ { uint8_t status;
- status = spi_read_status_register(flash); + spi_read_status_register(flash, &status); msg_cdbg("Chip status register is %02x\n", status);
msg_cdbg("Chip status register: Status Register Write Protect (WPEN) " @@ -151,7 +151,7 @@ { uint8_t status;
- status = spi_read_status_register(flash); + spi_read_status_register(flash, &status); msg_cdbg("Chip status register is %02x\n", status);
spi_prettyprint_status_register_atmel_at25_srpl(status); @@ -168,7 +168,7 @@ uint8_t status; int result;
- status = spi_read_status_register(flash); + spi_read_status_register(flash, &status); /* If block protection is disabled, stop here. */ if ((status & (3 << 2)) == 0) return 0; @@ -195,7 +195,7 @@ msg_cerr("spi_write_status_register failed\n"); return result; } - status = spi_read_status_register(flash); + spi_read_status_register(flash, &status); if ((status & (3 << 2)) != 0) { msg_cerr("Block protection could not be disabled!\n"); return 1; @@ -223,7 +223,7 @@ uint8_t status; int result;
- status = spi_read_status_register(flash); + spi_read_status_register(flash, &status); /* If block protection is disabled, stop here. */ if ((status & 0x6c) == 0) return 0; @@ -244,7 +244,7 @@ msg_cerr("spi_write_status_register failed\n"); return result; } - status = spi_read_status_register(flash); + spi_read_status_register(flash, &status); if ((status & 0x6c) != 0) { msg_cerr("Block protection could not be disabled!\n"); return 1; @@ -257,7 +257,7 @@ uint8_t status; int result;
- status = spi_read_status_register(flash); + spi_read_status_register(flash, &status); /* If block protection is disabled, stop here. */ if ((status & 0x7c) == 0) return 0; @@ -278,7 +278,7 @@ msg_cerr("spi_write_status_register failed\n"); return result; } - status = spi_read_status_register(flash); + spi_read_status_register(flash, &status); if ((status & 0x7c) != 0) { msg_cerr("Block protection could not be disabled!\n"); return 1; Index: flashrom-spi_rdsr_errorcheck/spi25.c =================================================================== --- flashrom-spi_rdsr_errorcheck/spi25.c (Revision 1539) +++ flashrom-spi_rdsr_errorcheck/spi25.c (Arbeitskopie) @@ -301,7 +301,7 @@ return 1; }
-uint8_t spi_read_status_register(struct flashctx *flash) +int spi_read_status_register(struct flashctx *flash, uint8_t *status) { static const unsigned char cmd[JEDEC_RDSR_OUTSIZE] = { JEDEC_RDSR }; /* FIXME: No workarounds for driver/hardware bugs in generic code. */ @@ -311,10 +311,14 @@ /* Read Status Register */ ret = spi_send_command(flash, sizeof(cmd), sizeof(readarr), cmd, readarr); - if (ret) + if (ret) { msg_cerr("RDSR failed!\n"); + *status = 0xff; + return ret; + }
- return readarr[0]; + *status = readarr[0]; + return 0; }
/* Prettyprint the status register. Common definitions. */ @@ -418,7 +422,7 @@ { uint8_t status;
- status = spi_read_status_register(flash); + spi_read_status_register(flash, &status); msg_cdbg("Chip status register is %02x\n", status); switch (flash->manufacture_id) { case ST_ID: @@ -451,6 +455,7 @@ int spi_chip_erase_60(struct flashctx *flash) { int result; + uint8_t status; struct spi_command cmds[] = { { .writecnt = JEDEC_WREN_OUTSIZE, @@ -479,7 +484,7 @@ * This usually takes 1-85 s, so wait in 1 s steps. */ /* FIXME: We assume spi_read_status_register will never fail. */ - while (spi_read_status_register(flash) & SPI_SR_WIP) + while (spi_read_status_register(flash, &status), status & SPI_SR_WIP) programmer_delay(1000 * 1000); /* FIXME: Check the status register for errors. */ return 0; @@ -488,6 +493,7 @@ int spi_chip_erase_c7(struct flashctx *flash) { int result; + uint8_t status; struct spi_command cmds[] = { { .writecnt = JEDEC_WREN_OUTSIZE, @@ -515,7 +521,7 @@ * This usually takes 1-85 s, so wait in 1 s steps. */ /* FIXME: We assume spi_read_status_register will never fail. */ - while (spi_read_status_register(flash) & SPI_SR_WIP) + while (spi_read_status_register(flash, &status), status & SPI_SR_WIP) programmer_delay(1000 * 1000); /* FIXME: Check the status register for errors. */ return 0; @@ -525,6 +531,7 @@ unsigned int blocklen) { int result; + uint8_t status; struct spi_command cmds[] = { { .writecnt = JEDEC_WREN_OUTSIZE, @@ -557,7 +564,7 @@ /* Wait until the Write-In-Progress bit is cleared. * This usually takes 100-4000 ms, so wait in 100 ms steps. */ - while (spi_read_status_register(flash) & SPI_SR_WIP) + while (spi_read_status_register(flash, &status), status & SPI_SR_WIP) programmer_delay(100 * 1000); /* FIXME: Check the status register for errors. */ return 0; @@ -572,6 +579,7 @@ unsigned int blocklen) { int result; + uint8_t status; struct spi_command cmds[] = { { .writecnt = JEDEC_WREN_OUTSIZE, @@ -604,7 +612,7 @@ /* Wait until the Write-In-Progress bit is cleared. * This usually takes 100-4000 ms, so wait in 100 ms steps. */ - while (spi_read_status_register(flash) & SPI_SR_WIP) + while (spi_read_status_register(flash, &status), status & SPI_SR_WIP) programmer_delay(100 * 1000); /* FIXME: Check the status register for errors. */ return 0; @@ -617,6 +625,7 @@ unsigned int blocklen) { int result; + uint8_t status; struct spi_command cmds[] = { { .writecnt = JEDEC_WREN_OUTSIZE, @@ -649,7 +658,7 @@ /* Wait until the Write-In-Progress bit is cleared. * This usually takes 100-4000 ms, so wait in 100 ms steps. */ - while (spi_read_status_register(flash) & SPI_SR_WIP) + while (spi_read_status_register(flash, &status), status & SPI_SR_WIP) programmer_delay(100 * 1000); /* FIXME: Check the status register for errors. */ return 0; @@ -660,6 +669,7 @@ unsigned int blocklen) { int result; + uint8_t status; struct spi_command cmds[] = { { .writecnt = JEDEC_WREN_OUTSIZE, @@ -692,7 +702,7 @@ /* Wait until the Write-In-Progress bit is cleared. * This usually takes 15-800 ms, so wait in 10 ms steps. */ - while (spi_read_status_register(flash) & SPI_SR_WIP) + while (spi_read_status_register(flash, &status), status & SPI_SR_WIP) programmer_delay(10 * 1000); /* FIXME: Check the status register for errors. */ return 0; @@ -764,7 +774,8 @@ * This is according the SST25VF016 datasheet, who knows it is more * generic that this... */ -static int spi_write_status_register_flag(struct flashctx *flash, int status, const unsigned char enable_opcode) +static int spi_write_status_register_flag(struct flashctx *flash, uint8_t status, + const unsigned char enable_opcode) { int result; int i = 0; @@ -781,7 +792,7 @@ .readarr = NULL, }, { .writecnt = JEDEC_WRSR_OUTSIZE, - .writearr = (const unsigned char[]){ JEDEC_WRSR, (unsigned char) status }, + .writearr = (const unsigned char[]){ JEDEC_WRSR, status }, .readcnt = 0, .readarr = NULL, }, { @@ -805,7 +816,7 @@ * 100 ms, then wait in 10 ms steps until a total of 5 s have elapsed. */ programmer_delay(100 * 1000); - while (spi_read_status_register(flash) & SPI_SR_WIP) { + while (spi_read_status_register(flash, &status), status & SPI_SR_WIP) { if (++i > 490) { msg_cerr("Error: WIP bit after WRSR never cleared\n"); return TIMEOUT_ERROR; @@ -925,7 +936,7 @@ uint8_t status; int result;
- status = spi_read_status_register(flash); + spi_read_status_register(flash, &status); /* If block protection is disabled, stop here. */ if ((status & 0x3c) == 0) return 0; @@ -936,7 +947,7 @@ msg_cerr("spi_write_status_register failed\n"); return result; } - status = spi_read_status_register(flash); + spi_read_status_register(flash, &status); if ((status & 0x3c) != 0) { msg_cerr("Block protection could not be disabled!\n"); return 1; @@ -1007,6 +1018,7 @@ unsigned int len, unsigned int chunksize) { int rc = 0; + uint8_t status; unsigned int i, j, starthere, lenhere, towrite; /* FIXME: page_size is the wrong variable. We need max_writechunk_size * in struct flashctx to do this properly. All chips using @@ -1035,7 +1047,7 @@ rc = spi_nbyte_program(flash, starthere + j, buf + starthere - start + j, towrite); if (rc) break; - while (spi_read_status_register(flash) & SPI_SR_WIP) + while (spi_read_status_register(flash, &status), status & SPI_SR_WIP) programmer_delay(10); } if (rc) @@ -1056,13 +1068,14 @@ unsigned int len) { unsigned int i; + uint8_t status; int result = 0;
for (i = start; i < start + len; i++) { result = spi_byte_program(flash, i, buf[i - start]); if (result) return 1; - while (spi_read_status_register(flash) & SPI_SR_WIP) + while (spi_read_status_register(flash, &status), status & SPI_SR_WIP) programmer_delay(10); }
@@ -1074,6 +1087,7 @@ { uint32_t pos = start; int result; + uint8_t status; unsigned char cmd[JEDEC_AAI_WORD_PROGRAM_CONT_OUTSIZE] = { JEDEC_AAI_WORD_PROGRAM, }; @@ -1157,7 +1171,7 @@ */ return result; } - while (spi_read_status_register(flash) & SPI_SR_WIP) + while (spi_read_status_register(flash, &status), status & SPI_SR_WIP) programmer_delay(10);
/* We already wrote 2 bytes in the multicommand step. */ @@ -1169,7 +1183,7 @@ cmd[2] = buf[pos++ - start]; spi_send_command(flash, JEDEC_AAI_WORD_PROGRAM_CONT_OUTSIZE, 0, cmd, NULL); - while (spi_read_status_register(flash) & SPI_SR_WIP) + while (spi_read_status_register(flash, &status), status & SPI_SR_WIP) programmer_delay(10); }
Index: flashrom-spi_rdsr_errorcheck/chipdrivers.h =================================================================== --- flashrom-spi_rdsr_errorcheck/chipdrivers.h (Revision 1539) +++ flashrom-spi_rdsr_errorcheck/chipdrivers.h (Arbeitskopie) @@ -45,7 +45,7 @@ int spi_chip_write_1(struct flashctx *flash, uint8_t *buf, unsigned int start, unsigned int len); int spi_chip_write_256(struct flashctx *flash, uint8_t *buf, unsigned int start, unsigned int len); int spi_chip_read(struct flashctx *flash, uint8_t *buf, unsigned int start, int unsigned len); -uint8_t spi_read_status_register(struct flashctx *flash); +int spi_read_status_register(struct flashctx *flash, uint8_t *status); int spi_write_status_register(struct flashctx *flash, int status); void spi_prettyprint_status_register_bit(uint8_t status, int bit); void spi_prettyprint_status_register_bp3210(uint8_t status, int bp);
On Sun, 03 Jun 2012 16:51:18 +0200 Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net wrote:
spi_read_status_register now returns success/failure and stores the status register value via call-by-reference. That way, status register reading failure (only possible if the programmer couldn't run RDSR) can be detected and acted upon in the future.
with this patch this failure is largely ignored and i am not sure that this does not change behavior due to the 0xff status value that is returned on errors.
Signed-off-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net
Index: flashrom-spi_rdsr_errorcheck/it87spi.c
--- flashrom-spi_rdsr_errorcheck/it87spi.c (Revision 1539) +++ flashrom-spi_rdsr_errorcheck/it87spi.c (Arbeitskopie) @@ -321,6 +321,7 @@ { unsigned int i; int result;
uint8_t status; chipaddr bios = flash->virtual_memory;
result = spi_write_enable(flash);
@@ -335,7 +336,7 @@ /* Wait until the Write-In-Progress bit is cleared. * This usually takes 1-10 ms, so wait in 1 ms steps. */
- while (spi_read_status_register(flash) & SPI_SR_WIP)
- while (spi_read_status_register(flash, &status), status & SPI_SR_WIP) programmer_delay(1000); return 0;
for example here it would be easy to store the return value of spi_read_status_register and return it instead of the fixed 0
} Index: flashrom-spi_rdsr_errorcheck/a25.c =================================================================== --- flashrom-spi_rdsr_errorcheck/a25.c (Revision 1539) +++ flashrom-spi_rdsr_errorcheck/a25.c (Arbeitskopie) @@ -33,7 +33,7 @@ { uint8_t status;
- status = spi_read_status_register(flash);
- spi_read_status_register(flash, &status); msg_cdbg("Chip status register is %02x\n", status);
here the output might change depending on what the programmers' send_spi_command routine "return" in case of errors etc.
in general i like the idea very much, but i would rather see a "complete" patch, that does acts appropriately on errors... try to convince me if you disagree :)
On Sun, 03 Jun 2012 16:51:18 +0200 Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net wrote:
only possible if the programmer couldn't run RDSR
or whatever read status register opcode it is... i miss this feature atm while working on at45db ;)
On Tue, 28 Aug 2012 04:31:45 +0200 Stefan Tauner stefan.tauner@student.tuwien.ac.at wrote:
On Sun, 03 Jun 2012 16:51:18 +0200 Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net wrote:
only possible if the programmer couldn't run RDSR
or whatever read status register opcode it is... i miss this feature atm while working on at45db ;)
hm no. what i actually miss is returning the status register value by the pretty print/"unlock" function. *sigh*
-
Carl-Daniel Hailfinger -
Stefan Tauner