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 <[email protected]>

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/


_______________________________________________
flashrom mailing list
[email protected]
http://www.flashrom.org/mailman/listinfo/flashrom

Reply via email to