[flashrom] [PATCH] Refactor spi_read_status_register usage

Carl-Daniel Hailfinger c-d.hailfinger.devel.2006 at gmx.net
Wed Mar 6 23:57:21 CET 2013


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 at 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);

-- 
http://www.hailfinger.org/





More information about the flashrom mailing list