[flashrom] [PATCH] Refactor spi_read_status_register usage
Carl-Daniel Hailfinger
c-d.hailfinger.devel.2006 at gmx.net
Sat Mar 8 17:02:04 CET 2014
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 at 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);
--
http://www.hailfinger.org/
More information about the flashrom
mailing list