spi_read_status_register() is used in open-coded loops everywhere just to check if SPI_SR_WIP bit cleared. The logic is missing a timeout detection, and we can save quite a lot of code by introducing a function which waits until SPI_SR_WIP is cleared or a timeout is reached.
Untested. May explode. Unfinished. More functions need to be converted to the new style. Will change behaviour if the chip is too slow or hangs. Should prevent flashrom from hanging without any visible output.
Signed-off-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net
Index: flashrom-spi_rdsr_refactor/spi25.c =================================================================== --- flashrom-spi_rdsr_refactor/spi25.c (Revision 1653) +++ flashrom-spi_rdsr_refactor/spi25.c (Arbeitskopie) @@ -328,14 +328,9 @@ __func__); return result; } - /* Wait until the Write-In-Progress bit is cleared. - * 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) - programmer_delay(1000 * 1000); /* FIXME: Check the status register for errors. */ - return 0; + result = spi_wait_status_register_ready(flash, 120 * 1000 * 1000, JEDEC_CE_60); + return result; }
int spi_chip_erase_62(struct flashctx *flash) @@ -365,14 +360,9 @@ __func__); return result; } - /* Wait until the Write-In-Progress bit is cleared. - * This usually takes 2-5 s, so wait in 100 ms steps. - */ - /* FIXME: We assume spi_read_status_register will never fail. */ - while (spi_read_status_register(flash) & SPI_SR_WIP) - programmer_delay(100 * 1000); /* FIXME: Check the status register for errors. */ - return 0; + result = spi_wait_status_register_ready(flash, 120 * 1000 * 1000, JEDEC_CE_62); + return result; }
int spi_chip_erase_c7(struct flashctx *flash) @@ -401,14 +391,9 @@ msg_cerr("%s failed during command execution\n", __func__); return result; } - /* Wait until the Write-In-Progress bit is cleared. - * 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) - programmer_delay(1000 * 1000); /* FIXME: Check the status register for errors. */ - return 0; + result = spi_wait_status_register_ready(flash, 120 * 1000 * 1000, JEDEC_CE_C7); + return result; }
int spi_block_erase_52(struct flashctx *flash, unsigned int addr, @@ -444,13 +429,9 @@ __func__, addr); return result; } - /* 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) - programmer_delay(100 * 1000); /* FIXME: Check the status register for errors. */ - return 0; + result = spi_wait_status_register_ready(flash, 10 * 1000 * 1000, JEDEC_BE_52); + return result; }
/* Block size is usually @@ -491,13 +472,9 @@ __func__, addr); return result; } - /* 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) - programmer_delay(100 * 1000); /* FIXME: Check the status register for errors. */ - return 0; + result = spi_wait_status_register_ready(flash, 10 * 1000 * 1000, JEDEC_BE_D8); + return result; }
/* Block size is usually @@ -536,13 +513,9 @@ __func__, addr); return result; } - /* 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) - programmer_delay(100 * 1000); /* FIXME: Check the status register for errors. */ - return 0; + result = spi_wait_status_register_ready(flash, 10 * 1000 * 1000, JEDEC_BE_D7); + return result; }
/* Sector size is usually 4k, though Macronix eliteflash has 64k */ @@ -579,13 +552,9 @@ __func__, addr); return result; } - /* 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) - programmer_delay(10 * 1000); /* FIXME: Check the status register for errors. */ - return 0; + result = spi_wait_status_register_ready(flash, 10 * 1000 * 1000, JEDEC_SE); + return result; }
int spi_block_erase_50(struct flashctx *flash, unsigned int addr, unsigned int blocklen) @@ -619,13 +588,9 @@ msg_cerr("%s failed during command execution at address 0x%x\n", __func__, addr); return result; } - /* Wait until the Write-In-Progress bit is cleared. - * This usually takes 10 ms, so wait in 1 ms steps. - */ - while (spi_read_status_register(flash) & SPI_SR_WIP) - programmer_delay(1 * 1000); /* FIXME: Check the status register for errors. */ - return 0; + result = spi_wait_status_register_ready(flash, 10 * 1000 * 1000, JEDEC_BE_50); + return result; }
int spi_block_erase_81(struct flashctx *flash, unsigned int addr, unsigned int blocklen) @@ -659,13 +624,9 @@ msg_cerr("%s failed during command execution at address 0x%x\n", __func__, addr); return result; } - /* Wait until the Write-In-Progress bit is cleared. - * This usually takes 8 ms, so wait in 1 ms steps. - */ - while (spi_read_status_register(flash) & SPI_SR_WIP) - programmer_delay(1 * 1000); /* FIXME: Check the status register for errors. */ - return 0; + result = spi_wait_status_register_ready(flash, 10 * 1000 * 1000, JEDEC_BE_81); + return result; }
int spi_block_erase_60(struct flashctx *flash, unsigned int addr, Index: flashrom-spi_rdsr_refactor/spi25_statusreg.c =================================================================== --- flashrom-spi_rdsr_refactor/spi25_statusreg.c (Revision 1653) +++ flashrom-spi_rdsr_refactor/spi25_statusreg.c (Arbeitskopie) @@ -43,7 +43,6 @@ static int spi_write_status_register_flag(struct flashctx *flash, int status, const unsigned char enable_opcode) { int result; - int i = 0; /* * WRSR requires either EWSR or WREN depending on chip type. * The code below relies on the fact hat EWSR and WREN have the same @@ -78,17 +77,10 @@ /* WRSR performs a self-timed erase before the changes take effect. * This may take 50-85 ms in most cases, and some chips apparently * allow running RDSR only once. Therefore pick an initial delay of - * 100 ms, then wait in 10 ms steps until a total of 5 s have elapsed. + * 100 ms, then wait until a total of 5 s have elapsed. */ programmer_delay(100 * 1000); - while (spi_read_status_register(flash) & SPI_SR_WIP) { - if (++i > 490) { - msg_cerr("Error: WIP bit after WRSR never cleared\n"); - return TIMEOUT_ERROR; - } - programmer_delay(10 * 1000); - } - return 0; + return spi_wait_status_register_ready(flash, 4900 * 1000, JEDEC_WRSR); }
int spi_write_status_register(struct flashctx *flash, int status) @@ -108,6 +100,28 @@ return ret; }
+/* timeout is specified in usecs. */ +int spi_wait_status_register_ready(struct flashctx *flash, int timeout, uint8_t opcode) +{ + /* At least 1 usec between iterations. */ + int single_delay = (timeout / 100) ? : 1; + int elapsed = 0; + int halftime = 0; + + while (spi_read_status_register(flash) & SPI_SR_WIP) { + elapsed += single_delay; + if ((elapsed > timeout / 2) && !halftime) { + msg_cdbg("Debug: WIP bit after %02x didn't clear within %i us.\n", opcode, timeout/2); + } + if (elapsed >= timeout) { + msg_cerr("Error: WIP bit after %02x didn't clear within %i us.\n", opcode, timeout); + return TIMEOUT_ERROR; + } + programmer_delay(single_delay); + } + return 0; +} + uint8_t spi_read_status_register(struct flashctx *flash) { static const unsigned char cmd[JEDEC_RDSR_OUTSIZE] = { JEDEC_RDSR }; Index: flashrom-spi_rdsr_refactor/chipdrivers.h =================================================================== --- flashrom-spi_rdsr_refactor/chipdrivers.h (Revision 1653) +++ flashrom-spi_rdsr_refactor/chipdrivers.h (Arbeitskopie) @@ -60,6 +60,7 @@
/* spi25_statusreg.c */ uint8_t spi_read_status_register(struct flashctx *flash); +int spi_wait_status_register_ready(struct flashctx *flash, int timeout, uint8_t opcode); int spi_write_status_register(struct flashctx *flash, int status); int spi_prettyprint_status_register_plain(struct flashctx *flash); int spi_prettyprint_status_register_default_bp1(struct flashctx *flash);
New version. Should work. Untested. Needs a full code review because it's not just trivial search/replace (timings and opcodes need to be checked).
spi_read_status_register() is used in open-coded loops everywhere just to check if SPI_SR_WIP bit cleared. The logic is missing a timeout detection, and we can save quite a lot of code by introducing a function which waits until SPI_SR_WIP is cleared or a timeout is reached.
Will change behaviour if the chip is too slow or hangs. Should prevent flashrom from hanging without any visible output.
Signed-off-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net
Index: flashrom-spi_rdsr_refactor/it87spi.c =================================================================== --- flashrom-spi_rdsr_refactor/it87spi.c (Revision 1765) +++ flashrom-spi_rdsr_refactor/it87spi.c (Arbeitskopie) @@ -365,10 +365,11 @@ mmio_writeb(buf[i], (void *)(bios + start + i)); OUTB(0, it8716f_flashport); /* Wait until the Write-In-Progress bit is cleared. - * This usually takes 1-10 ms, so wait in 1 ms steps. + * This usually takes 1-10 ms. */ - while (spi_read_status_register(flash) & SPI_SR_WIP) - programmer_delay(1000); + result = spi_wait_status_register_ready(flash, 100 * 1000, JEDEC_BYTE_PROGRAM); + if (result) + return result; return 0; }
Index: flashrom-spi_rdsr_refactor/spi25.c =================================================================== --- flashrom-spi_rdsr_refactor/spi25.c (Revision 1765) +++ flashrom-spi_rdsr_refactor/spi25.c (Arbeitskopie) @@ -348,14 +348,9 @@ __func__); return result; } - /* Wait until the Write-In-Progress bit is cleared. - * 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) - programmer_delay(1000 * 1000); /* FIXME: Check the status register for errors. */ - return 0; + result = spi_wait_status_register_ready(flash, 120 * 1000 * 1000, JEDEC_CE_60); + return result; }
int spi_chip_erase_62(struct flashctx *flash) @@ -385,14 +380,9 @@ __func__); return result; } - /* Wait until the Write-In-Progress bit is cleared. - * This usually takes 2-5 s, so wait in 100 ms steps. - */ - /* FIXME: We assume spi_read_status_register will never fail. */ - while (spi_read_status_register(flash) & SPI_SR_WIP) - programmer_delay(100 * 1000); /* FIXME: Check the status register for errors. */ - return 0; + result = spi_wait_status_register_ready(flash, 120 * 1000 * 1000, JEDEC_CE_62); + return result; }
int spi_chip_erase_c7(struct flashctx *flash) @@ -421,14 +411,9 @@ msg_cerr("%s failed during command execution\n", __func__); return result; } - /* Wait until the Write-In-Progress bit is cleared. - * 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) - programmer_delay(1000 * 1000); /* FIXME: Check the status register for errors. */ - return 0; + result = spi_wait_status_register_ready(flash, 120 * 1000 * 1000, JEDEC_CE_C7); + return result; }
int spi_block_erase_52(struct flashctx *flash, unsigned int addr, @@ -464,13 +449,9 @@ __func__, addr); return result; } - /* 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) - programmer_delay(100 * 1000); /* FIXME: Check the status register for errors. */ - return 0; + result = spi_wait_status_register_ready(flash, 10 * 1000 * 1000, JEDEC_BE_52); + return result; }
/* Block size is usually @@ -508,12 +489,11 @@ return result; } /* Wait until the Write-In-Progress bit is cleared. - * This usually takes 240-480 s, so wait in 500 ms steps. + * This usually takes 240-480 s. */ - while (spi_read_status_register(flash) & SPI_SR_WIP) - programmer_delay(500 * 1000 * 1000); /* FIXME: Check the status register for errors. */ - return 0; + result = spi_wait_status_register_ready(flash, 500 * 1000 * 1000, JEDEC_BE_C4); + return result; }
/* Block size is usually @@ -554,13 +534,9 @@ __func__, addr); return result; } - /* 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) - programmer_delay(100 * 1000); /* FIXME: Check the status register for errors. */ - return 0; + result = spi_wait_status_register_ready(flash, 10 * 1000 * 1000, JEDEC_BE_D8); + return result; }
/* Block size is usually @@ -599,13 +575,9 @@ __func__, addr); return result; } - /* 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) - programmer_delay(100 * 1000); /* FIXME: Check the status register for errors. */ - return 0; + result = spi_wait_status_register_ready(flash, 10 * 1000 * 1000, JEDEC_BE_D7); + return result; }
/* Page erase (usually 256B blocks) */ @@ -642,10 +614,9 @@ }
/* Wait until the Write-In-Progress bit is cleared. - * This takes up to 20 ms usually (on worn out devices up to the 0.5s range), so wait in 1 ms steps. */ - while (spi_read_status_register(flash) & SPI_SR_WIP) - programmer_delay(1 * 1000); + * This takes up to 20 ms usually (on worn out devices up to the 0.5s range). */ /* FIXME: Check the status register for errors. */ + result = spi_wait_status_register_ready(flash, 500 * 1000, JEDEC_PE); return 0; }
@@ -683,13 +654,9 @@ __func__, addr); return result; } - /* 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) - programmer_delay(10 * 1000); /* FIXME: Check the status register for errors. */ - return 0; + result = spi_wait_status_register_ready(flash, 10 * 1000 * 1000, JEDEC_SE); + return result; }
int spi_block_erase_50(struct flashctx *flash, unsigned int addr, unsigned int blocklen) @@ -723,13 +690,9 @@ msg_cerr("%s failed during command execution at address 0x%x\n", __func__, addr); return result; } - /* Wait until the Write-In-Progress bit is cleared. - * This usually takes 10 ms, so wait in 1 ms steps. - */ - while (spi_read_status_register(flash) & SPI_SR_WIP) - programmer_delay(1 * 1000); /* FIXME: Check the status register for errors. */ - return 0; + result = spi_wait_status_register_ready(flash, 10 * 1000 * 1000, JEDEC_BE_50); + return result; }
int spi_block_erase_81(struct flashctx *flash, unsigned int addr, unsigned int blocklen) @@ -763,13 +726,9 @@ msg_cerr("%s failed during command execution at address 0x%x\n", __func__, addr); return result; } - /* Wait until the Write-In-Progress bit is cleared. - * This usually takes 8 ms, so wait in 1 ms steps. - */ - while (spi_read_status_register(flash) & SPI_SR_WIP) - programmer_delay(1 * 1000); /* FIXME: Check the status register for errors. */ - return 0; + result = spi_wait_status_register_ready(flash, 10 * 1000 * 1000, JEDEC_BE_81); + return result; }
int spi_block_erase_60(struct flashctx *flash, unsigned int addr, @@ -1015,8 +974,9 @@ rc = spi_nbyte_program(flash, starthere + j, buf + starthere - start + j, towrite); if (rc) break; - while (spi_read_status_register(flash) & SPI_SR_WIP) - programmer_delay(10); + rc = spi_wait_status_register_ready(flash, 1000, JEDEC_BYTE_PROGRAM); + if (rc) + break; } if (rc) break; @@ -1041,9 +1001,10 @@ 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) - programmer_delay(10); + return result; + result = spi_wait_status_register_ready(flash, 1000, JEDEC_BYTE_PROGRAM); + if (result) + return result; }
return 0; @@ -1136,8 +1097,9 @@ */ return result; } - while (spi_read_status_register(flash) & SPI_SR_WIP) - programmer_delay(10); + result = spi_wait_status_register_ready(flash, 1000, JEDEC_AAI_WORD_PROGRAM); + if (result) + return result;
/* We already wrote 2 bytes in the multicommand step. */ pos += 2; @@ -1148,8 +1110,9 @@ 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) - programmer_delay(10); + result = spi_wait_status_register_ready(flash, 1000, JEDEC_AAI_WORD_PROGRAM); + if (result) + return result; }
/* Use WRDI to exit AAI mode. This needs to be done before issuing any Index: flashrom-spi_rdsr_refactor/spi25_statusreg.c =================================================================== --- flashrom-spi_rdsr_refactor/spi25_statusreg.c (Revision 1765) +++ flashrom-spi_rdsr_refactor/spi25_statusreg.c (Arbeitskopie) @@ -43,7 +43,6 @@ static int spi_write_status_register_flag(struct flashctx *flash, int status, const unsigned char enable_opcode) { int result; - int i = 0; /* * WRSR requires either EWSR or WREN depending on chip type. * The code below relies on the fact hat EWSR and WREN have the same @@ -78,17 +77,10 @@ /* WRSR performs a self-timed erase before the changes take effect. * This may take 50-85 ms in most cases, and some chips apparently * allow running RDSR only once. Therefore pick an initial delay of - * 100 ms, then wait in 10 ms steps until a total of 5 s have elapsed. + * 100 ms, then wait until a total of 5 s have elapsed. */ programmer_delay(100 * 1000); - while (spi_read_status_register(flash) & SPI_SR_WIP) { - if (++i > 490) { - msg_cerr("Error: WIP bit after WRSR never cleared\n"); - return TIMEOUT_ERROR; - } - programmer_delay(10 * 1000); - } - return 0; + return spi_wait_status_register_ready(flash, 4900 * 1000, JEDEC_WRSR); }
int spi_write_status_register(struct flashctx *flash, int status) @@ -108,6 +100,32 @@ return ret; }
+/* timeout is specified in usecs. */ +int spi_wait_status_register_ready(struct flashctx *flash, int timeout, uint8_t opcode) +{ + /* At least 1 usec between iterations. */ + int single_delay = (timeout / 100) ? : 1; + int elapsed = 0; + int halftime = 0; + uint8_t status; + + while (status = spi_read_status_register(flash), status & SPI_SR_WIP) { + if ((elapsed > timeout / 2) && !halftime) { + halftime = 1; + msg_cdbg("WIP bit after command %02x slow (still set after %i us), status=0x%02x.\n", + opcode, timeout/2, status); + } + if (elapsed >= timeout) { + msg_cerr("Error: WIP bit after command %02x still set after %i us, status=0x%02x.\n", + opcode, timeout, status); + return TIMEOUT_ERROR; + } + programmer_delay(single_delay); + elapsed += single_delay; + } + return 0; +} + uint8_t spi_read_status_register(struct flashctx *flash) { static const unsigned char cmd[JEDEC_RDSR_OUTSIZE] = { JEDEC_RDSR }; Index: flashrom-spi_rdsr_refactor/chipdrivers.h =================================================================== --- flashrom-spi_rdsr_refactor/chipdrivers.h (Revision 1765) +++ flashrom-spi_rdsr_refactor/chipdrivers.h (Arbeitskopie) @@ -63,6 +63,7 @@
/* spi25_statusreg.c */ uint8_t spi_read_status_register(struct flashctx *flash); +int spi_wait_status_register_ready(struct flashctx *flash, int timeout, uint8_t opcode); int spi_write_status_register(struct flashctx *flash, int status); void spi_prettyprint_status_register_bit(uint8_t status, int bit); int spi_prettyprint_status_register_plain(struct flashctx *flash);
-
Carl-Daniel Hailfinger