Fix smart erase.

Smart erase is used by the haven private-CR51 firmware, I don't
know if other projects use it.

Smart erase attempts to speed up erase by checking if the block
to be erased is all ff's, and only erasing it if there is
content (not ff's).

The bug is that after erasing a block, the code does not wait
for completion of the erase before reading ahead to see if the
next block is already erased (all ff's). This is contrary to the
spec where the only valid operation is a check of the status
after issuing the erase.

On some eeproms, with some timings, this causes the smart erase to
give a flase positive erased block detection. Ie, the eeprom reads
back al ff's while it's busy doing the erase. The upshot is that
only the first non erased block is erased, and the rest of the
eeprom is left untouched.

The code before smart erase looked like:

do
  wait for not busy
  erase block
until all erased
wait for not busy

Smart erase was added by inserting the check for erased at the top of
the loop. If instead, it's moved down below the wait for not busy,
everything works fine. (Or, the wait for not busy is moved back to
top of the loop.) This is the fix used here.

TEST= Run without and with patch on a Starcard. Without patch
not all of the targeted flash is erased. With patch, all of the
targeted flash is erased.

BUG=b:144868388

BRANCH=barryt/smart

Signed-off-by: barryt@google.com

Change-Id: I679ad4d21c3c353252646394f5631abc42782ded
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/1931466
Reviewed-by: Jeff Andersen <jeffandersen@google.com>
Reviewed-by: Vadim Bendebury <vbendeb@chromium.org>
Reviewed-by: Barry Twycross <barryt@google.com>
Commit-Queue: Barry Twycross <barryt@google.com>
Tested-by: Barry Twycross <barryt@google.com>
Auto-Submit: Barry Twycross <barryt@google.com>

1 files changed, 12 insertions, 5 deletions

diff --git a/common/spi_nor.c b/common/spi_nor.c
index 09f5b28bdd..98a9a01af8 100644
--- a/common/spi_nor.c
+++ b/common/spi_nor.c
@@ -691,6 +691,11 @@ int spi_nor_erase(const struct spi_nor_device_t *spi_nor_device,
 		erase_opcode = SPI_NOR_DRIVER_SPECIFIED_OPCODE_4KIB_ERASE;
 		erase_size = 4096;
 
+		/* Wait for the previous operation to finish. */
+		rv = spi_nor_wait(spi_nor_device);
+		if (rv)
+			goto err_free;
+
 #ifdef CONFIG_SPI_NOR_BLOCK_ERASE
 		if (!(offset % 65536) && size >= 65536) {
 			erase_opcode =
@@ -710,6 +715,13 @@ int spi_nor_erase(const struct spi_nor_device_t *spi_nor_device,
 			assert(read_size >= 4 && (read_size % 4) == 0);
 			rv = spi_nor_read_internal(spi_nor_device, read_offset,
 						   read_size, buffer);
+
+			/* Note: the return value here is lost below
+			 * at the write enable, this is not a problem,
+			 * as this code is only an optimisation, if it
+			 * fails, the full erase functionality still
+			 * gets done, and the error from that returned
+			 */
 			if (rv != EC_SUCCESS)
 				break;
 			/* Aligned word verify reduced the overall (read +
@@ -741,11 +753,6 @@ int spi_nor_erase(const struct spi_nor_device_t *spi_nor_device,
 			continue;
 		}
 #endif
-		/* Wait for the previous operation to finish. */
-		rv = spi_nor_wait(spi_nor_device);
-		if (rv)
-			goto err_free;
-
 		/* Enable writing to serial NOR flash. */
 		rv = spi_nor_write_enable(spi_nor_device);
 		if (rv)