Change in flashrom[master]: Add "gingerly" flashing mode for the unreliable ISP environments

[Mike Banon (Code Review)]

Mike Banon has uploaded this change for review. ( https://review.coreboot.org/23840

Change subject: Add "gingerly" flashing mode for the unreliable ISP environments
......................................................................

Add "gingerly" flashing mode for the unreliable ISP environments

Some in-system-programming environments are very unreliable, e.g.
when you connect a SOIC8 test clip to the flash chip of WR841ND v9
wireless router: because the low power SOC of this router becomes
powered and is constantly trying to boot, the router's flash chip
is correctly detected only 30% of times, and when it is detected
just 30% of the operations are successful - the rest result in
zeroes or garbage data being read from / written to a flash chip

While in this new mode called "-g" / "--gingerly", flashrom will be
constantly probing for a flash chip until it is correctly detected -
and then, depending on the operation selected by a user, at the
moments of chip availability it will be reading or writing by the
small chunks of 256 bytes maximum size, verifying each small chunk
and retrying the operation until the verification becomes successful.
Constant probing between the small chunk operations is also being
done - to avoid a possible situation where we have got two identical
blocks of zeroes while reading a chunk and erroneously accepted it

Currently this mode is active only for those SPI25 chips which are
".write = spi_chip_write_256" and ".read = spi_chip_read", but later
could be expanded to more functions to become useful for more chips

Change-Id: Ie3f18276d9fb7233d082720cb29d154f31c77100
Signed-off-by: Mike Banon 
---
M cli_classic.c
M flash.h
M libflashrom.c
M libflashrom.h
M spi25.c
5 files changed, 120 insertions(+), 16 deletions(-)

git pull ssh://review.coreboot.org:29418/flashrom refs/changes/40/23840/1

diff --git a/cli_classic.c b/cli_classic.c
index 31f7394..d9f0c65 100644
--- a/cli_classic.c
+++ b/cli_classic.c
@@ -42,7 +42,7 @@
           "-z|"
 #endif
           "-p <programmername>[:<parameters>] [-c <chipname>]\n"
-	       "[-E|(-r|-w|-v) <file>] [(-l <layoutfile>|--ifd) [-i <imagename>]...] [-n] [-N] [-f]]\n"
+	       "[-E|(-r|-w|-v) <file>] [(-l <layoutfile>|--ifd) [-i <imagename>]...] [-n] [-N] [-g] [-f]]\n"
           "[-V[V[V]]] [-o <logfile>]\n\n", name);

printf(" -h | --help                        print this help text\n"
@@ -54,6 +54,7 @@
           " -V | --verbose                     more verbose output\n"
           " -c | --chip <chipname>             probe only for specified flash chip\n"
           " -f | --force                       force specific operations (see man page)\n"
+	       " -g | --gingerly                    verify each small chunk and probe constantly\n"
           " -n | --noverify                    don't auto-verify\n"
           " -N | --noverify-all                verify included regions only (cf. -i)\n"
           " -l | --layout <layoutfile>         read ROM layout from <layoutfile>\n"
@@ -105,7 +106,7 @@
 #if CONFIG_PRINT_WIKI == 1
    int list_supported_wiki = 0;
 #endif
-	int read_it = 0, write_it = 0, erase_it = 0, verify_it = 0;
+	int read_it = 0, write_it = 0, erase_it = 0, verify_it = 0, gingerly = 0;
    int dont_verify_it = 0, dont_verify_all = 0, list_supported = 0, operation_specified = 0;
    struct flashrom_layout *layout = NULL;
    enum programmer prog = PROGRAMMER_INVALID;
@@ -115,7 +116,7 @@
    };
    int ret = 0;

-	static const char optstring[] = "r:Rw:v:nNVEfc:l:i:p:Lzho:";
+	static const char optstring[] = "r:Rw:v:nNVEfgc:l:i:p:Lzho:";
    static const struct option long_options[] = {
    	{"read",		1, NULL, 'r'},
    	{"write",		1, NULL, 'w'},
@@ -126,6 +127,7 @@
    	{"chip",		1, NULL, 'c'},
    	{"verbose",		0, NULL, 'V'},
    	{"force",		0, NULL, 'f'},
+		{"gingerly",		0, NULL, 'g'},
    	{"layout",		1, NULL, 'l'},
    	{"ifd",			0, NULL, OPTION_IFD},
    	{"image",		1, NULL, 'i'},
@@ -224,6 +226,9 @@
    	case 'f':
    		force = 1;
    		break;
+		case 'g':
+			gingerly = 1;
+			break;
    	case 'l':
    		if (layoutfile) {
    			fprintf(stderr, "Error: --layout specified "
@@ -454,16 +459,21 @@
    msg_pdbg("The following protocols are supported: %s.\n", tempstr);
    free(tempstr);

-	for (j = 0; j < registered_master_count; j++) {
-		startchip = 0;
-		while (chipcount < ARRAY_SIZE(flashes)) {
-			startchip = probe_flash(&registered_masters[j], startchip, &flashes[chipcount], 0);
-			if (startchip == -1)
-				break;
-			chipcount++;
-			startchip++;
+	do {
+		chipcount = 0;
+		for (j = 0; j < registered_master_count; j++) {
+			startchip = 0;
+			while (chipcount < ARRAY_SIZE(flashes)) {
+				startchip = probe_flash(&registered_masters[j], startchip, &flashes[chipcount], 0);
+				if (startchip == -1)
+					break;
+				chipcount++;
+				startchip++;
+			}
    	}
-	}
+	} while (gingerly && ( !chipcount || // wait for the correct chip detection
+			((flashes[0].chip->manufacture_id == GENERIC_MANUF_ID) ||
+			 (flashes[0].chip->model_id == GENERIC_DEVICE_ID)) ) );

if (chipcount > 1) {
    	msg_cinfo("Multiple flash chip definitions match the detected chip(s): "%s"",
@@ -566,6 +576,7 @@

flashrom_layout_set(fill_flash, layout);
    flashrom_flag_set(fill_flash, FLASHROM_FLAG_FORCE, !!force);
+	flashrom_flag_set(fill_flash, FLASHROM_FLAG_GINGERLY, !!gingerly);
 #if CONFIG_INTERNAL == 1
    flashrom_flag_set(fill_flash, FLASHROM_FLAG_FORCE_BOARDMISMATCH, !!force_boardmismatch);
 #endif
diff --git a/flash.h b/flash.h
index a80a9c2..9e8a8ee 100644
--- a/flash.h
+++ b/flash.h
@@ -254,6 +254,7 @@
    struct {
    	bool force;
    	bool force_boardmismatch;
+		bool gingerly;
    	bool verify_after_write;
    	bool verify_whole_chip;
    } flags;
diff --git a/libflashrom.c b/libflashrom.c
index 6e0f42c..c0e4fcf 100644
--- a/libflashrom.c
+++ b/libflashrom.c
@@ -253,6 +253,7 @@
    switch (flag) {
    	case FLASHROM_FLAG_FORCE:		flashctx->flags.force = value; break;
    	case FLASHROM_FLAG_FORCE_BOARDMISMATCH:	flashctx->flags.force_boardmismatch = value; break;
+		case FLASHROM_FLAG_GINGERLY:		flashctx->flags.gingerly = value; break;
    	case FLASHROM_FLAG_VERIFY_AFTER_WRITE:	flashctx->flags.verify_after_write = value; break;
    	case FLASHROM_FLAG_VERIFY_WHOLE_CHIP:	flashctx->flags.verify_whole_chip = value; break;
    }
@@ -270,6 +271,7 @@
    switch (flag) {
    	case FLASHROM_FLAG_FORCE:		return flashctx->flags.force;
    	case FLASHROM_FLAG_FORCE_BOARDMISMATCH:	return flashctx->flags.force_boardmismatch;
+		case FLASHROM_FLAG_GINGERLY:		return flashctx->flags.gingerly;
    	case FLASHROM_FLAG_VERIFY_AFTER_WRITE:	return flashctx->flags.verify_after_write;
    	case FLASHROM_FLAG_VERIFY_WHOLE_CHIP:	return flashctx->flags.verify_whole_chip;
    	default:				return false;
diff --git a/libflashrom.h b/libflashrom.h
index 786147b..712df4f 100644
--- a/libflashrom.h
+++ b/libflashrom.h
@@ -53,6 +53,7 @@
 enum flashrom_flag {
    FLASHROM_FLAG_FORCE,
    FLASHROM_FLAG_FORCE_BOARDMISMATCH,
+	FLASHROM_FLAG_GINGERLY,
    FLASHROM_FLAG_VERIFY_AFTER_WRITE,
    FLASHROM_FLAG_VERIFY_WHOLE_CHIP,
 };
@@ -69,4 +70,4 @@
 void flashrom_layout_release(struct flashrom_layout *);
 void flashrom_layout_set(struct flashrom_flashctx *, const struct flashrom_layout *);

-#endif				/* !__LIBFLASHROM_H__ */
\ No newline at end of file
+#endif				/* !__LIBFLASHROM_H__ */
diff --git a/spi25.c b/spi25.c
index 00e0992..adcaff5 100644
--- a/spi25.c
+++ b/spi25.c
@@ -25,6 +25,7 @@
 #include <stddef.h>
 #include <string.h>
 #include <stdbool.h>
+#include <stdlib.h>
 #include "flash.h"
 #include "flashchips.h"
 #include "chipdrivers.h"
@@ -652,6 +653,46 @@
    return spi_send_command(flash, 1 + addr_len, len, cmd, bytes);
 }

+static int spi_rw_gingerly(struct flashctx *flash, uint8_t *buf1, uint8_t *buf2,
+			   unsigned int startrw_here, unsigned int torw_len, bool rw_flag) {
+
+	int rc = 0;
+
+	while (1) {
+
+		// FIRST PROBE
+		if (flash->chip->probe(flash) != 1)
+			continue;
+
+		// FIRST OPERATION
+		if (rw_flag == 0)
+			rc = spi_nbyte_read(flash, startrw_here, buf1, torw_len);
+		else
+			rc = spi_nbyte_program(flash, startrw_here, buf1, torw_len);
+
+		if (rc != 0)
+			break;
+
+		// SECOND PROBE
+		if (flash->chip->probe(flash) != 1)
+			continue;
+
+		// SECOND OPERATION
+		rc = spi_nbyte_read(flash, startrw_here, buf2, torw_len);
+
+		if (rc != 0)
+			break;
+
+		// COMPARE TWO BUFFERS
+		if (memcmp(buf1, buf2, torw_len))
+			continue;
+
+		break;
+	}
+
+	return rc;
+}
+
 /*
  * Read a part of the flash chip.
  * FIXME: Use the chunk code from Michael Karcher instead.
@@ -665,6 +706,20 @@
    /* Limit for multi-die 4-byte-addressing chips. */
    unsigned int area_size = min(flash->chip->total_size * 1024, 16 * 1024 * 1024);

+	unsigned int gingerly = flashrom_flag_get(flash, FLASHROM_FLAG_GINGERLY);
+	uint8_t *const buf1 = gingerly ? malloc(chunksize) : NULL;
+	uint8_t *const buf2 = gingerly ? malloc(chunksize) : NULL;
+
+	if (gingerly && (!buf1 || !buf2)) {
+		msg_gerr("Out of memory!\n");
+		goto _free_ret;
+	}
+
+	if (gingerly) {
+		msg_cdbg("spi_read_chunked: chunksize %i ---> %i\n", chunksize, min(chunksize, 256));
+		chunksize = min(chunksize, 256);
+	}
+
    /* Warning: This loop has a very unusual condition and body.
     * The loop needs to go through each area with at least one affected
     * byte. The lowest area number is (start / area_size) since that
@@ -674,6 +729,7 @@
     * (start + len - 1) / area_size. Since we want to include that last
     * area as well, the loop condition uses <=.
     */
+
    for (i = start / area_size; i <= (start + len - 1) / area_size; i++) {
    	/* Byte position of the first byte in the range in this area. */
    	/* starthere is an offset to the base address of the chip. */
@@ -682,7 +738,14 @@
    	lenhere = min(start + len, (i + 1) * area_size) - starthere;
    	for (j = 0; j < lenhere; j += chunksize) {
    		toread = min(chunksize, lenhere - j);
-			rc = spi_nbyte_read(flash, starthere + j, buf + starthere - start + j, toread);
+
+			if (gingerly) {
+				rc = spi_rw_gingerly(flash, buf1, buf2, starthere + j, toread, 0 /* read */);
+				memcpy(buf + starthere - start + j, buf1, toread);
+			}
+			else
+				rc = spi_nbyte_read(flash, starthere + j, buf + starthere - start + j, toread);
+
    		if (rc)
    			break;
    	}
@@ -690,6 +753,9 @@
    		break;
    }

+_free_ret:
+	free(buf1);
+	free(buf2);
    return rc;
 }

@@ -709,6 +775,20 @@
     */
    unsigned int page_size = flash->chip->page_size;

+	unsigned int gingerly = flashrom_flag_get(flash, FLASHROM_FLAG_GINGERLY);
+	uint8_t *const buf1 = gingerly ? malloc(chunksize) : NULL;
+	uint8_t *const buf2 = gingerly ? malloc(chunksize) : NULL;
+
+	if (gingerly && (!buf1 || !buf2)) {
+		msg_gerr("Out of memory!\n");
+		goto _free_ret;
+	}
+
+	if (gingerly) {
+		msg_cdbg("spi_read_chunked: chunksize %i ---> %i\n", chunksize, min(chunksize, 256));
+		chunksize = min(chunksize, 256);
+	}
+
    /* Warning: This loop has a very unusual condition and body.
     * The loop needs to go through each page with at least one affected
     * byte. The lowest page number is (start / page_size) since that
@@ -718,6 +798,7 @@
     * (start + len - 1) / page_size. Since we want to include that last
     * page as well, the loop condition uses <=.
     */
+
    for (i = start / page_size; i <= (start + len - 1) / page_size; i++) {
    	/* Byte position of the first byte in the range in this page. */
    	/* starthere is an offset to the base address of the chip. */
@@ -726,14 +807,22 @@
    	lenhere = min(start + len, (i + 1) * page_size) - starthere;
    	for (j = 0; j < lenhere; j += chunksize) {
    		int rc;
-
    		towrite = min(chunksize, lenhere - j);
-			rc = spi_nbyte_program(flash, starthere + j, buf + starthere - start + j, towrite);
+
+			if (gingerly) {
+				memcpy(buf1, buf + starthere - start + j, towrite);
+				rc = spi_rw_gingerly(flash, buf1, buf2, starthere + j, towrite, 1 /* write */);
+			}
+			else
+				rc = spi_nbyte_program(flash, starthere + j, buf + starthere - start + j, towrite);
    		if (rc)
    			return rc;
    	}
    }

+_free_ret:
+	free(buf1);
+	free(buf2);
    return 0;
 }