Patchwork Refactor spi_read_status_register usage

login
register
about
Submitter Carl-Daniel Hailfinger
Date 2014-03-08 16:02:04
Message ID <531B3EFC.4020403@gmx.net>
Download mbox | patch
Permalink /patch/4115/
State New
Headers show

Comments

Carl-Daniel Hailfinger - 2014-03-08 16:02:04
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>

Patch

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