[PATCH] flashrom: Check if erase worked, check return codes

[Carl-Daniel Hailfinger]

flashrom only checks for very few chips if the erase worked.
And even when it checks if the erase worked, the result of that check is
often ignored.

Fix the majority of these problems. More to come, but I'd like to get
this reviewed first.

Signed-off-by: Carl-Daniel Hailfinger

Index: flashrom-check_erase_range/flash.h
===================================================================
--- flashrom-check_erase_range/flash.h	(Revision 584)
+++ flashrom-check_erase_range/flash.h	(Arbeitskopie)
@@ -721,6 +721,10 @@
 #define printf_debug(x...) { if (verbose) printf(x); }
 void map_flash_registers(struct flashchip *flash);
 int read_memmapped(struct flashchip *flash, uint8_t *buf);
+int min(int a, int b);
+int max(int a, int b);
+int check_erased_range(struct flashchip *flash, int start, int len);
+int verify_range(struct flashchip *flash, uint8_t *cmpbuf, int start, int len, char *message);
 extern char *pcidev_bdf;

/* layout.c */
Index: flashrom-check_erase_range/en29f002a.c
===================================================================
--- flashrom-check_erase_range/en29f002a.c	(Revision 584)
+++ flashrom-check_erase_range/en29f002a.c	(Arbeitskopie)
@@ -98,7 +98,10 @@

//chip_writeb(0xF0, bios);
    programmer_delay(10);
-	erase_chip_jedec(flash);
+	if (erase_chip_jedec(flash)) {
+		fprintf(stderr, "ERASE FAILED!\n");
+		return -1;
+	}

printf("Programming page: ");
    for (i = 0; i < total_size; i++) {
Index: flashrom-check_erase_range/sharplhf00l04.c
===================================================================
--- flashrom-check_erase_range/sharplhf00l04.c	(Revision 584)
+++ flashrom-check_erase_range/sharplhf00l04.c	(Arbeitskopie)
@@ -124,6 +124,10 @@
    print_lhf00l04_status(status);
    printf("DONE BLOCK 0x%x\n", offset);

+	if (check_erased_range(flash, offset, flash->page_size)) {
+		fprintf(stderr, "ERASE FAILED!\n");
+		return -1;
+	}
    return 0;
 }

@@ -135,7 +139,10 @@
    printf("total_size is %d; flash->page_size is %d\n",
           total_size, flash->page_size);
    for (i = 0; i < total_size; i += flash->page_size)
-		erase_lhf00l04_block(flash, i);
+		if (erase_lhf00l04_block(flash, i)) {
+			fprintf(stderr, "ERASE FAILED!\n");
+			return -1;
+		}
    printf("DONE ERASE\n");

return 0;
@@ -161,9 +168,8 @@
    int page_size = flash->page_size;
    chipaddr bios = flash->virtual_memory;

-	erase_lhf00l04(flash);
-	if (chip_readb(bios) != 0xff) {
-		printf("ERASE FAILED!\n");
+	if (erase_lhf00l04(flash)) {
+		fprintf(stderr, "ERASE FAILED!\n");
    	return -1;
    }
    printf("Programming page: ");
Index: flashrom-check_erase_range/w39v040c.c
===================================================================
--- flashrom-check_erase_range/w39v040c.c	(Revision 584)
+++ flashrom-check_erase_range/w39v040c.c	(Arbeitskopie)
@@ -60,16 +60,13 @@
 {
    int i;
    unsigned int total_size = flash->total_size * 1024;
-	chipaddr bios = flash->virtual_memory;

-	for (i = 0; i < total_size; i += flash->page_size)
-		erase_sector_jedec(flash->virtual_memory, i);
-
-	for (i = 0; i < total_size; i++)
-		if (0xff != chip_readb(bios + i)) {
-			printf("ERASE FAILED at 0x%08x!  Expected=0xff, Read=0x%02x\n", i, chip_readb(bios + i));
+	for (i = 0; i < total_size; i += flash->page_size) {
+		if (erase_sector_jedec(flash->virtual_memory, i)) {
+			fprintf(stderr, "ERASE FAILED!\n");
    		return -1;
    	}
+	}

return 0;
 }
@@ -81,8 +78,10 @@
    int page_size = flash->page_size;
    chipaddr bios = flash->virtual_memory;

-	if (flash->erase(flash))
+	if (flash->erase(flash)) {
+		fprintf(stderr, "ERASE FAILED!\n");
    	return -1;
+	}

printf("Programming page: ");
    for (i = 0; i < total_size / page_size; i++) {
Index: flashrom-check_erase_range/stm50flw0x0x.c
===================================================================
--- flashrom-check_erase_range/stm50flw0x0x.c	(Revision 584)
+++ flashrom-check_erase_range/stm50flw0x0x.c	(Arbeitskopie)
@@ -163,7 +163,6 @@
 int erase_block_stm50flw0x0x(struct flashchip *flash, int offset)
 {
    chipaddr bios = flash->virtual_memory + offset;
-	int j;

// clear status register
    chip_writeb(0x50, bios);
@@ -175,13 +174,10 @@

wait_stm50flw0x0x(flash->virtual_memory);

-	for (j = 0; j < flash->page_size; j++) {
-		if (chip_readb(bios + j) != 0xFF) {
-			printf("Erase failed at 0x%x\n", offset + j);
-			return -1;
-		}
+	if (check_erased_range(flash, offset, flash->page_size)) {
+		fprintf(stderr, "ERASE FAILED!\n");
+		return -1;
    }
-
    printf("DONE BLOCK 0x%x\n", offset);

return 0;
@@ -230,24 +226,29 @@
  */
 int erase_stm50flw0x0x(struct flashchip *flash)
 {
-	int i, rc = 0;
+	int i;
    int total_size = flash->total_size * 1024;
    int page_size = flash->page_size;
    chipaddr bios = flash->virtual_memory;

printf("Erasing page:\n");
-	for (i = 0; (i < total_size / page_size) && (rc == 0); i++) {
+	for (i = 0; i < total_size / page_size; i++) {
    	printf
    	    ("\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b");
    	printf("%04d at address: 0x%08x ", i, i * page_size);
-		rc = unlock_block_stm50flw0x0x(flash, i * page_size);
-		if (!rc)
-			rc = erase_block_stm50flw0x0x(flash, i * page_size);
+		if (unlock_block_stm50flw0x0x(flash, i * page_size)) {
+			fprintf(stderr, "UNLOCK FAILED!\n");
+			return -1;
+		}
+		if (erase_block_stm50flw0x0x(flash, i * page_size)) {
+			fprintf(stderr, "ERASE FAILED!\n");
+			return -1;
+		}
    }
    printf("\n");
    protect_stm50flw0x0x(bios);

-	return rc;
+	return 0;
 }

int write_stm50flw0x0x(struct flashchip *flash, uint8_t * buf)
Index: flashrom-check_erase_range/am29f040b.c
===================================================================
--- flashrom-check_erase_range/am29f040b.c	(Revision 584)
+++ flashrom-check_erase_range/am29f040b.c	(Arbeitskopie)
@@ -20,8 +20,11 @@

#include "flash.h"

-static int erase_sector_29f040b(chipaddr bios, unsigned long address)
+static int erase_sector_29f040b(struct flashchip *flash, unsigned long address)
 {
+	int page_size = flash->page_size;
+	chipaddr bios = flash->virtual_memory;
+
    chip_writeb(0xAA, bios + 0x555);
    chip_writeb(0x55, bios + 0x2AA);
    chip_writeb(0x80, bios + 0x555);
@@ -34,6 +37,10 @@
    /* wait for Toggle bit ready         */
    toggle_ready_jedec(bios + address);

+	if (check_erased_range(flash, address, page_size)) {
+		fprintf(stderr, "ERASE FAILED!\n");
+		return -1;
+	}
    return 0;
 }

@@ -86,6 +93,7 @@

int erase_29f040b(struct flashchip *flash)
 {
+	int total_size = flash->total_size * 1024;
    chipaddr bios = flash->virtual_memory;

chip_writeb(0xAA, bios + 0x555);
@@ -98,6 +106,10 @@
    programmer_delay(10);
    toggle_ready_jedec(bios);

+	if (check_erased_range(flash, 0, total_size)) {
+		fprintf(stderr, "ERASE FAILED!\n");
+		return -1;
+	}
    return 0;
 }

@@ -111,7 +123,10 @@
    printf("Programming page ");
    for (i = 0; i < total_size / page_size; i++) {
    	/* erase the page before programming */
-		erase_sector_29f040b(bios, i * page_size);
+		if (erase_sector_29f040b(flash, i * page_size)) {
+			fprintf(stderr, "ERASE FAILED!\n");
+			return -1;
+		}

/* write to the sector */
    	printf("%04d at address: ", i);
Index: flashrom-check_erase_range/sst_fwhub.c
===================================================================
--- flashrom-check_erase_range/sst_fwhub.c	(Revision 584)
+++ flashrom-check_erase_range/sst_fwhub.c	(Arbeitskopie)
@@ -104,7 +104,10 @@
    	return 1;
    }

-	erase_block_jedec(flash->virtual_memory, offset);
+	if (erase_block_jedec(flash->virtual_memory, offset)) {
+		fprintf(stderr, "ERASE FAILED!\n");
+		return -1;
+	}
    toggle_ready_jedec(flash->virtual_memory);

return 0;
@@ -114,15 +117,10 @@
 {
    int i;
    unsigned int total_size = flash->total_size * 1024;
-	chipaddr bios = flash->virtual_memory;

-	for (i = 0; i < total_size; i += flash->page_size)
-		erase_sst_fwhub_block(flash, i);
-
-	// dumb check if erase was successful.
-	for (i = 0; i < total_size; i++) {
-		if (chip_readb(bios + i) != 0xff) {
-			printf("ERASE FAILED!\n");
+	for (i = 0; i < total_size; i += flash->page_size) {
+		if (erase_sst_fwhub_block(flash, i)) {
+			fprintf(stderr, "ERASE FAILED!\n");
    		return -1;
    	}
    }
Index: flashrom-check_erase_range/w39v080fa.c
===================================================================
--- flashrom-check_erase_range/w39v080fa.c	(Revision 584)
+++ flashrom-check_erase_range/w39v080fa.c	(Arbeitskopie)
@@ -142,9 +142,10 @@
    return 0;
 }

-static int erase_sector_winbond_fwhub(chipaddr bios,
+static int erase_sector_winbond_fwhub(struct flashchip *flash,
    			      unsigned int sector)
 {
+	chipaddr bios = flash->virtual_memory;
    /* Remember: too much sleep can waste your day. */

printf("0x%08x\b\b\b\b\b\b\b\b\b\b", sector);
@@ -161,30 +162,30 @@
    /* wait for Toggle bit ready */
    toggle_ready_jedec(bios);

+	if (check_erased_range(flash, sector, flash->page_size)) {
+		fprintf(stderr, "ERASE FAILED!\n");
+		return -1;
+	}
    return 0;
 }

int erase_winbond_fwhub(struct flashchip *flash)
 {
    int i, total_size = flash->total_size * 1024;
-	chipaddr bios = flash->virtual_memory;

unlock_winbond_fwhub(flash);

printf("Erasing:     ");

-	for (i = 0; i < total_size; i += flash->page_size)
-		erase_sector_winbond_fwhub(bios, i);
-
-	printf("\n");
-
-	for (i = 0; i < total_size; i++) {
-		if (chip_readb(bios + i) != 0xff) {
-			fprintf(stderr, "Error: Flash chip erase failed at 0x%08x(0x%02x)\n", i, chip_readb(bios + i));
+	for (i = 0; i < total_size; i += flash->page_size) {
+		if (erase_sector_winbond_fwhub(flash, i)) {
+			fprintf(stderr, "ERASE FAILED!\n");
    		return -1;
    	}
    }

+	printf("\n");
+
    return 0;
 }

Index: flashrom-check_erase_range/flashrom.c
===================================================================
--- flashrom-check_erase_range/flashrom.c	(Revision 584)
+++ flashrom-check_erase_range/flashrom.c	(Arbeitskopie)
@@ -200,6 +200,16 @@
    return 0;
 }

+int min(int a, int b)
+{
+	return (a < b) ? a : b;
+}
+
+int max(int a, int b)
+{
+	return (a > b) ? a : b;
+}
+
 char *strcat_realloc(char *dest, const char *src)
 {
    dest = realloc(dest, strlen(dest) + strlen(src) + 1);
@@ -240,6 +250,72 @@
    return ret;
 }

+/* start is an offset to the base address of the flash chip */
+int check_erased_range(struct flashchip *flash, int start, int len)
+{
+	int page_size = flash->page_size;
+	uint8_t *cmpbuf = malloc(page_size);
+
+	if (!cmpbuf) {
+		fprintf(stderr, "Could not allocate memory!\n");
+		exit(1);
+	}
+	memset(cmpbuf, 0xff, len);
+	return verify_range(flash, cmpbuf, start, len, "ERASE");
+}
+
+/* start is an offset to the base address of the flash chip */
+int verify_range(struct flashchip *flash, uint8_t *cmpbuf, int start, int len, char *message)
+{
+	int i, j, starthere, lenhere;
+	chipaddr bios = flash->virtual_memory;
+	int page_size = flash->page_size;
+	uint8_t *readbuf = malloc(page_size);
+
+	if (!readbuf) {
+		fprintf(stderr, "Could not allocate memory!\n");
+		exit(1);
+	}
+	if (start + len > flash->total_size * 1024) {
+		fprintf(stderr, "Error: %s called with start 0x%x + len 0x%x >"
+			" total_size 0x%x\n", __func__, start, len,
+			flash->total_size * 1024);
+		return -1;
+	}
+	if (!len)
+		return 0;
+	if (!message)
+		message = "VERIFY";
+	
+	/* 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
+	 * division rounds down. The highest page number we want is the page
+	 * where the last byte of the range lives. That last byte has the
+	 * address (start + len - 1), thus the highest page number is
+	 * (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 = max(start, i * page_size);
+		/* Length of bytes in the range in this page. */
+		lenhere = min(start + len, (i + 1) * page_size) - starthere;
+		/* starthere is an offset to the base address of the chip. */
+		chip_readn(readbuf, bios + starthere, lenhere);
+		for (j = 0; j < lenhere; j++) {
+			if (cmpbuf[starthere + j] != readbuf[j]) {
+				fprintf(stderr, "%s FAILED at 0x%08x! "
+					"Expected=0x%02x, Read=0x%02x\n",
+					message, starthere + j,
+					cmpbuf[starthere + j], readbuf[j]);
+				return -1;
+			}
+		}
+	}
+	return 0;
+}
+
 struct flashchip *probe_flash(struct flashchip *first_flash, int force)
 {
    struct flashchip *flash;
Index: flashrom-check_erase_range/82802ab.c
===================================================================
--- flashrom-check_erase_range/82802ab.c	(Revision 584)
+++ flashrom-check_erase_range/82802ab.c	(Arbeitskopie)
@@ -110,7 +110,6 @@
 {
    chipaddr bios = flash->virtual_memory + offset;
    chipaddr wrprotect = flash->virtual_registers + offset + 2;
-	int j;
    uint8_t status;

// clear status register
@@ -129,11 +128,9 @@
    // now let's see what the register is
    status = wait_82802ab(flash->virtual_memory);
    //print_82802ab_status(status);
-	for (j = 0; j < flash->page_size; j++) {
-		if (chip_readb(bios + j) != 0xFF) {
-			printf("BLOCK ERASE failed at 0x%x\n", offset);
-			return -1;
-		}
+	if (check_erased_range(flash, offset, flash->page_size)) {
+		fprintf(stderr, "ERASE FAILED!\n");
+		return -1;
    }
    printf("DONE BLOCK 0x%x\n", offset);

@@ -148,7 +145,10 @@
    printf("total_size is %d; flash->page_size is %d\n",
           total_size, flash->page_size);
    for (i = 0; i < total_size; i += flash->page_size)
-		erase_82802ab_block(flash, i);
+		if (erase_82802ab_block(flash, i)) {
+			fprintf(stderr, "ERASE FAILED!\n");
+			return -1;
+		}
    printf("DONE ERASE\n");

return 0;
@@ -199,7 +199,10 @@
    	}

/* erase block by block and write block by block; this is the most secure way */
-		erase_82802ab_block(flash, i * page_size);
+		if (erase_82802ab_block(flash, i * page_size)) {
+			fprintf(stderr, "ERASE FAILED!\n");
+			return -1;
+		}
    	write_page_82802ab(bios, buf + i * page_size,
    			   bios + i * page_size, page_size);
    }

[PATCH] flashrom: Check if erase worked, check return codes

[Urja Rannikko]

On Fri, Jun 12, 2009 at 17:01, Carl-Daniel
Hailfinger wrote:
> flashrom only checks for very few chips if the erase worked.
> And even when it checks if the erase worked, the result of that check is
> often ignored.
>
> Fix the majority of these problems. More to come, but I'd like to get
> this reviewed first.
>
> Signed-off-by: Carl-Daniel Hailfinger

I'll do first a review-like listing of my changes and then inline my
new patch (and attach too).

Attached patch:
Signed-off-by: Urja Rannikko

Also carl-daniel's patch with my additions is Acked-by me, but...
> +/* start is an offset to the base address of the flash chip */
> +int check_erased_range(struct flashchip *flash, int start, int len)
> +{
> int page_size = flash->page_size;
> uint8_t *cmpbuf = malloc(page_size);

malloc (len), not page_size
> if (!cmpbuf) {
> fprintf(stderr, "Could not allocate memory!\n");
> exit(1);
> }
> memset(cmpbuf, 0xff, len);
> return verify_range(flash, cmpbuf, start, len, "ERASE");

add variable to hold return value, free cmpbuf after calling verify_range
> +}
>
> +/* start is an offset to the base address of the flash chip */
> +int verify_range(struct flashchip *flash, uint8_t *cmpbuf, int start, int len, char *message)
> +{
> int i, j, starthere, lenhere;
> chipaddr bios = flash->virtual_memory;
> int page_size = flash->page_size;
> uint8_t *readbuf = malloc(page_size);
> if (!readbuf) {
> fprintf(stderr, "Could not allocate memory!\n");
> exit(1);
> }
> if (start + len > flash->total_size * 1024) {
> fprintf(stderr, "Error: %s called with start 0x%x + len 0x%x >"
> " total_size 0x%x\n", __func__, start, len,
> flash->total_size * 1024);

free readbuf
> return -1;
> }
> if (!len)
> return 0;

free readbuf
> if (!message)
> message = "VERIFY";
> /* 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
> * division rounds down. The highest page number we want is the page
> * where the last byte of the range lives. That last byte has the
> * address (start + len - 1), thus the highest page number is
> * (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 = max(start, i * page_size);
> /* Length of bytes in the range in this page. */
> lenhere = min(start + len, (i + 1) * page_size) - starthere;
> /* starthere is an offset to the base address of the chip. */
> chip_readn(readbuf, bios + starthere, lenhere);
> for (j = 0; j < lenhere; j++) {
> if (cmpbuf[starthere + j] != readbuf[j]) {
> fprintf(stderr, "%s FAILED at 0x%08x! "
> "Expected=0x%02x, Read=0x%02x\n",
> message, starthere + j,
> cmpbuf[starthere + j], readbuf[j]);

as above
> return -1;
> }
> }
> }

as above
> return 0;
>
> +}

Also, changed the erase check in jedec.c to use check_erased_range.

My new patch inlined:
Index: flash.h
===================================================================
--- flash.h	(revision 589)
+++ flash.h	(working copy)
@@ -722,6 +722,9 @@
 void map_flash_registers(struct flashchip *flash);
 int read_memmapped(struct flashchip *flash, uint8_t *buf);
 int min(int a, int b);
+int max(int a, int b);
+int check_erased_range(struct flashchip *flash, int start, int len);
+int verify_range(struct flashchip *flash, uint8_t *cmpbuf, int start,
int len, char *message);
 extern char *pcidev_bdf;

/* layout.c */
Index: en29f002a.c
===================================================================
--- en29f002a.c	(revision 589)
+++ en29f002a.c	(working copy)
@@ -98,7 +98,10 @@

//chip_writeb(0xF0, bios);
    programmer_delay(10);
-	erase_chip_jedec(flash);
+	if (erase_chip_jedec(flash)) {
+		fprintf(stderr, "ERASE FAILED!\n");
+		return -1;
+	}

printf("Programming page: ");
    for (i = 0; i < total_size; i++) {
Index: jedec.c
===================================================================
--- jedec.c	(revision 589)
+++ jedec.c	(working copy)
@@ -344,13 +344,11 @@

erase_chip_jedec(flash);
    // dumb check if erase was successful.
-	for (i = 0; i < total_size; i++) {
-		if (chip_readb(bios + i) != 0xff) {
-			printf("ERASE FAILED @%d, val %02x!\n", i, chip_readb(bios + i));
-			return -1;
-		}
+	if (check_erased_range(flash, 0, total_size)) {
+		fprintf(stderr,"ERASE FAILED!\n");
+		return -1;
    }
-
+	
    printf("Programming page: ");
    for (i = 0; i < total_size / page_size; i++) {
    	printf("%04d at address: 0x%08x", i, i * page_size);
Index: sharplhf00l04.c
===================================================================
--- sharplhf00l04.c	(revision 589)
+++ sharplhf00l04.c	(working copy)
@@ -124,6 +124,10 @@
    print_lhf00l04_status(status);
    printf("DONE BLOCK 0x%x\n", offset);

+	if (check_erased_range(flash, offset, flash->page_size)) {
+		fprintf(stderr, "ERASE FAILED!\n");
+		return -1;
+	}
    return 0;
 }

@@ -135,7 +139,10 @@
    printf("total_size is %d; flash->page_size is %d\n",
           total_size, flash->page_size);
    for (i = 0; i < total_size; i += flash->page_size)
-		erase_lhf00l04_block(flash, i);
+		if (erase_lhf00l04_block(flash, i)) {
+			fprintf(stderr, "ERASE FAILED!\n");
+			return -1;
+		}
    printf("DONE ERASE\n");

return 0;
@@ -161,9 +168,8 @@
    int page_size = flash->page_size;
    chipaddr bios = flash->virtual_memory;

-	erase_lhf00l04(flash);
-	if (chip_readb(bios) != 0xff) {
-		printf("ERASE FAILED!\n");
+	if (erase_lhf00l04(flash)) {
+		fprintf(stderr, "ERASE FAILED!\n");
    	return -1;
    }
    printf("Programming page: ");
Index: w39v040c.c
===================================================================
--- w39v040c.c	(revision 589)
+++ w39v040c.c	(working copy)
@@ -60,16 +60,13 @@
 {
    int i;
    unsigned int total_size = flash->total_size * 1024;
-	chipaddr bios = flash->virtual_memory;

-	for (i = 0; i < total_size; i += flash->page_size)
-		erase_sector_jedec(flash->virtual_memory, i);
-
-	for (i = 0; i < total_size; i++)
-		if (0xff != chip_readb(bios + i)) {
-			printf("ERASE FAILED at 0x%08x!  Expected=0xff, Read=0x%02x\n", i,
chip_readb(bios + i));
+	for (i = 0; i < total_size; i += flash->page_size) {
+		if (erase_sector_jedec(flash->virtual_memory, i)) {
+			fprintf(stderr, "ERASE FAILED!\n");
    		return -1;
    	}
+	}

return 0;
 }
@@ -81,8 +78,10 @@
    int page_size = flash->page_size;
    chipaddr bios = flash->virtual_memory;

-	if (flash->erase(flash))
+	if (flash->erase(flash)) {
+		fprintf(stderr, "ERASE FAILED!\n");
    	return -1;
+	}

printf("Programming page: ");
    for (i = 0; i < total_size / page_size; i++) {
Index: stm50flw0x0x.c
===================================================================
--- stm50flw0x0x.c	(revision 589)
+++ stm50flw0x0x.c	(working copy)
@@ -163,7 +163,6 @@
 int erase_block_stm50flw0x0x(struct flashchip *flash, int offset)
 {
    chipaddr bios = flash->virtual_memory + offset;
-	int j;

// clear status register
    chip_writeb(0x50, bios);
@@ -175,13 +174,10 @@

wait_stm50flw0x0x(flash->virtual_memory);

-	for (j = 0; j < flash->page_size; j++) {
-		if (chip_readb(bios + j) != 0xFF) {
-			printf("Erase failed at 0x%x\n", offset + j);
-			return -1;
-		}
+	if (check_erased_range(flash, offset, flash->page_size)) {
+		fprintf(stderr, "ERASE FAILED!\n");
+		return -1;
    }
-
    printf("DONE BLOCK 0x%x\n", offset);

return 0;
@@ -230,24 +226,29 @@
  */
 int erase_stm50flw0x0x(struct flashchip *flash)
 {
-	int i, rc = 0;
+	int i;
    int total_size = flash->total_size * 1024;
    int page_size = flash->page_size;
    chipaddr bios = flash->virtual_memory;

printf("Erasing page:\n");
-	for (i = 0; (i < total_size / page_size) && (rc == 0); i++) {
+	for (i = 0; i < total_size / page_size; i++) {
    	printf
    	    ("\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b");
    	printf("%04d at address: 0x%08x ", i, i * page_size);
-		rc = unlock_block_stm50flw0x0x(flash, i * page_size);
-		if (!rc)
-			rc = erase_block_stm50flw0x0x(flash, i * page_size);
+		if (unlock_block_stm50flw0x0x(flash, i * page_size)) {
+			fprintf(stderr, "UNLOCK FAILED!\n");
+			return -1;
+		}
+		if (erase_block_stm50flw0x0x(flash, i * page_size)) {
+			fprintf(stderr, "ERASE FAILED!\n");
+			return -1;
+		}
    }
    printf("\n");
    protect_stm50flw0x0x(bios);

-	return rc;
+	return 0;
 }

int write_stm50flw0x0x(struct flashchip *flash, uint8_t * buf)
Index: sst_fwhub.c
===================================================================
--- sst_fwhub.c	(revision 589)
+++ sst_fwhub.c	(working copy)
@@ -104,7 +104,10 @@
    	return 1;
    }

-	erase_block_jedec(flash->virtual_memory, offset);
+	if (erase_block_jedec(flash->virtual_memory, offset)) {
+		fprintf(stderr, "ERASE FAILED!\n");
+		return -1;
+	}
    toggle_ready_jedec(flash->virtual_memory);

return 0;
@@ -114,15 +117,10 @@
 {
    int i;
    unsigned int total_size = flash->total_size * 1024;
-	chipaddr bios = flash->virtual_memory;

-	for (i = 0; i < total_size; i += flash->page_size)
-		erase_sst_fwhub_block(flash, i);
-
-	// dumb check if erase was successful.
-	for (i = 0; i < total_size; i++) {
-		if (chip_readb(bios + i) != 0xff) {
-			printf("ERASE FAILED!\n");
+	for (i = 0; i < total_size; i += flash->page_size) {
+		if (erase_sst_fwhub_block(flash, i)) {
+			fprintf(stderr, "ERASE FAILED!\n");
    		return -1;
    	}
    }
Index: am29f040b.c
===================================================================
--- am29f040b.c	(revision 589)
+++ am29f040b.c	(working copy)
@@ -20,8 +20,11 @@

#include "flash.h"

-static int erase_sector_29f040b(chipaddr bios, unsigned long address)
+static int erase_sector_29f040b(struct flashchip *flash, unsigned long address)
 {
+	int page_size = flash->page_size;
+	chipaddr bios = flash->virtual_memory;
+
    chip_writeb(0xAA, bios + 0x555);
    chip_writeb(0x55, bios + 0x2AA);
    chip_writeb(0x80, bios + 0x555);
@@ -34,6 +37,10 @@
    /* wait for Toggle bit ready         */
    toggle_ready_jedec(bios + address);

+	if (check_erased_range(flash, address, page_size)) {
+		fprintf(stderr, "ERASE FAILED!\n");
+		return -1;
+	}
    return 0;
 }

@@ -86,6 +93,7 @@

int erase_29f040b(struct flashchip *flash)
 {
+	int total_size = flash->total_size * 1024;
    chipaddr bios = flash->virtual_memory;

chip_writeb(0xAA, bios + 0x555);
@@ -98,6 +106,10 @@
    programmer_delay(10);
    toggle_ready_jedec(bios);

+	if (check_erased_range(flash, 0, total_size)) {
+		fprintf(stderr, "ERASE FAILED!\n");
+		return -1;
+	}
    return 0;
 }

@@ -111,7 +123,10 @@
    printf("Programming page ");
    for (i = 0; i < total_size / page_size; i++) {
    	/* erase the page before programming */
-		erase_sector_29f040b(bios, i * page_size);
+		if (erase_sector_29f040b(flash, i * page_size)) {
+			fprintf(stderr, "ERASE FAILED!\n");
+			return -1;
+		}

/* write to the sector */
    	printf("%04d at address: ", i);
Index: w39v080fa.c
===================================================================
--- w39v080fa.c	(revision 589)
+++ w39v080fa.c	(working copy)
@@ -142,9 +142,10 @@
    return 0;
 }

-static int erase_sector_winbond_fwhub(chipaddr bios,
+static int erase_sector_winbond_fwhub(struct flashchip *flash,
    			      unsigned int sector)
 {
+	chipaddr bios = flash->virtual_memory;
    /* Remember: too much sleep can waste your day. */

printf("0x%08x\b\b\b\b\b\b\b\b\b\b", sector);
@@ -161,30 +162,30 @@
    /* wait for Toggle bit ready */
    toggle_ready_jedec(bios);

+	if (check_erased_range(flash, sector, flash->page_size)) {
+		fprintf(stderr, "ERASE FAILED!\n");
+		return -1;
+	}
    return 0;
 }

int erase_winbond_fwhub(struct flashchip *flash)
 {
    int i, total_size = flash->total_size * 1024;
-	chipaddr bios = flash->virtual_memory;

unlock_winbond_fwhub(flash);

printf("Erasing:     ");

-	for (i = 0; i < total_size; i += flash->page_size)
-		erase_sector_winbond_fwhub(bios, i);
-
-	printf("\n");
-
-	for (i = 0; i < total_size; i++) {
-		if (chip_readb(bios + i) != 0xff) {
-			fprintf(stderr, "Error: Flash chip erase failed at
0x%08x(0x%02x)\n", i, chip_readb(bios + i));
+	for (i = 0; i < total_size; i += flash->page_size) {
+		if (erase_sector_winbond_fwhub(flash, i)) {
+			fprintf(stderr, "ERASE FAILED!\n");
    		return -1;
    	}
    }

+	printf("\n");
+
    return 0;
 }

Index: flashrom.c
===================================================================
--- flashrom.c	(revision 589)
+++ flashrom.c	(working copy)
@@ -205,6 +205,11 @@
    return (a < b) ? a : b;
 }

+int max(int a, int b)
+{
+	return (a > b) ? a : b;
+}
+
 char *strcat_realloc(char *dest, const char *src)
 {
    dest = realloc(dest, strlen(dest) + strlen(src) + 1);
@@ -245,6 +250,79 @@
    return ret;
 }

+/* start is an offset to the base address of the flash chip */
+int check_erased_range(struct flashchip *flash, int start, int len)
+{
+	int rv;
+	uint8_t *cmpbuf = malloc(len);
+
+	if (!cmpbuf) {
+		fprintf(stderr, "Could not allocate memory!\n");
+		exit(1);
+	}
+	memset(cmpbuf, 0xff, len);
+	rv = verify_range(flash, cmpbuf, start, len, "ERASE");
+	free(cmpbuf);
+	return rv;
+}
+
+/* start is an offset to the base address of the flash chip */
+int verify_range(struct flashchip *flash, uint8_t *cmpbuf, int start,
int len, char *message)
+{
+	int i, j, starthere, lenhere;
+	chipaddr bios = flash->virtual_memory;
+	int page_size = flash->page_size;
+	uint8_t *readbuf = malloc(page_size);
+
+	if (!readbuf) {
+		fprintf(stderr, "Could not allocate memory!\n");
+		exit(1);
+	}
+	if (start + len > flash->total_size * 1024) {
+		fprintf(stderr, "Error: %s called with start 0x%x + len 0x%x >"
+			" total_size 0x%x\n", __func__, start, len,
+			flash->total_size * 1024);
+		free(readbuf);
+		return -1;
+	}
+	if (!len) {
+		free(readbuf);
+		return 0;
+	}
+	if (!message)
+		message = "VERIFY";
+	
+	/* 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
+	 * division rounds down. The highest page number we want is the page
+	 * where the last byte of the range lives. That last byte has the
+	 * address (start + len - 1), thus the highest page number is
+	 * (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 = max(start, i * page_size);
+		/* Length of bytes in the range in this page. */
+		lenhere = min(start + len, (i + 1) * page_size) - starthere;
+		/* starthere is an offset to the base address of the chip. */
+		chip_readn(readbuf, bios + starthere, lenhere);
+		for (j = 0; j < lenhere; j++) {
+			if (cmpbuf[starthere + j] != readbuf[j]) {
+				fprintf(stderr, "%s FAILED at 0x%08x! "
+					"Expected=0x%02x, Read=0x%02x\n",
+					message, starthere + j,
+					cmpbuf[starthere + j], readbuf[j]);
+				free(readbuf);
+				return -1;
+			}
+		}
+	}
+	free(readbuf);
+	return 0;
+}
+
 struct flashchip *probe_flash(struct flashchip *first_flash, int force)
 {
    struct flashchip *flash;
Index: 82802ab.c
===================================================================
--- 82802ab.c	(revision 589)
+++ 82802ab.c	(working copy)
@@ -110,7 +110,6 @@
 {
    chipaddr bios = flash->virtual_memory + offset;
    chipaddr wrprotect = flash->virtual_registers + offset + 2;
-	int j;
    uint8_t status;

// clear status register
@@ -129,11 +128,9 @@
    // now let's see what the register is
    status = wait_82802ab(flash->virtual_memory);
    //print_82802ab_status(status);
-	for (j = 0; j < flash->page_size; j++) {
-		if (chip_readb(bios + j) != 0xFF) {
-			printf("BLOCK ERASE failed at 0x%x\n", offset);
-			return -1;
-		}
+	if (check_erased_range(flash, offset, flash->page_size)) {
+		fprintf(stderr, "ERASE FAILED!\n");
+		return -1;
    }
    printf("DONE BLOCK 0x%x\n", offset);

@@ -148,7 +145,10 @@
    printf("total_size is %d; flash->page_size is %d\n",
           total_size, flash->page_size);
    for (i = 0; i < total_size; i += flash->page_size)
-		erase_82802ab_block(flash, i);
+		if (erase_82802ab_block(flash, i)) {
+			fprintf(stderr, "ERASE FAILED!\n");
+			return -1;
+		}
    printf("DONE ERASE\n");

return 0;
@@ -199,7 +199,10 @@
    	}

/* erase block by block and write block by block; this is the most
secure way */
-		erase_82802ab_block(flash, i * page_size);
+		if (erase_82802ab_block(flash, i * page_size)) {
+			fprintf(stderr, "ERASE FAILED!\n");
+			return -1;
+		}
    	write_page_82802ab(bios, buf + i * page_size,
    			   bios + i * page_size, page_size);
    }

[PATCH] flashrom: Check if erase worked, check return codes

[Carl-Daniel Hailfinger]

On 14.06.2009 22:13, Urja Rannikko wrote:
> On Fri, Jun 12, 2009 at 17:01, Carl-Daniel
> Hailfinger wrote:
> > flashrom only checks for very few chips if the erase worked.
> > And even when it checks if the erase worked, the result of that check is
> > often ignored.
> >
> > Fix the majority of these problems. More to come, but I'd like to get
> > this reviewed first.
> >
> > Signed-off-by: Carl-Daniel Hailfinger
>
> I'll do first a review-like listing of my changes and then inline my
> new patch (and attach too).
>
> Attached patch:
> Signed-off-by: Urja Rannikko

Thanks!
> Also carl-daniel's patch with my additions is Acked-by me, but...
> > +/* start is an offset to the base address of the flash chip */
> > +int check_erased_range(struct flashchip *flash, int start, int len)
> > +{
> >   int page_size = flash->page_size;
> >   uint8_t *cmpbuf = malloc(page_size);
>
> malloc (len), not page_size

Ouch, yes. This is a leftover from an earlier version of the patch.
> >   if (!cmpbuf) {
> >           fprintf(stderr, "Could not allocate memory!\n");
> >           exit(1);
> >   }
> >   memset(cmpbuf, 0xff, len);
> >   return verify_range(flash, cmpbuf, start, len, "ERASE");
>
> add variable to hold return value, free cmpbuf after calling verify_range

I wasn't really awake while changing that code. Sorry. Same for the
other bugs you found.

Compared to your version, I changed the following:
- Check inside all erase_*_jedec routines if erase worked, not outside.
- Rename rv to ret. Most functions in flashrom call the return variable ret.
- erase_sector_jedec and erase_block_jedec have changed prototypes to
enable erase checking.
- verify_range uses goto out_free to make sure we don't forget to free().
- Convert all remaining erase functions and actually check return codes
almost everywhere.

Urja, would you remind reviewing the whole patch again? At 1087 lines of
manual conversions, it is too big for me to be 100% confident that I got
everything right.

Signed-off-by: Carl-Daniel Hailfinger

Index: flashrom-check_erase_range/flash.h
===================================================================
--- flashrom-check_erase_range/flash.h	(Revision 591)
+++ flashrom-check_erase_range/flash.h	(Arbeitskopie)
@@ -724,6 +724,9 @@
 void map_flash_registers(struct flashchip *flash);
 int read_memmapped(struct flashchip *flash, uint8_t *buf);
 int min(int a, int b);
+int max(int a, int b);
+int check_erased_range(struct flashchip *flash, int start, int len);
+int verify_range(struct flashchip *flash, uint8_t *cmpbuf, int start, int len, char *message);
 extern char *pcidev_bdf;

/* layout.c */
@@ -831,8 +834,8 @@
 int probe_jedec(struct flashchip *flash);
 int erase_chip_jedec(struct flashchip *flash);
 int write_jedec(struct flashchip *flash, uint8_t *buf);
-int erase_sector_jedec(chipaddr bios, unsigned int page);
-int erase_block_jedec(chipaddr bios, unsigned int page);
+int erase_sector_jedec(struct flashchip *flash, unsigned int page, int pagesize);
+int erase_block_jedec(struct flashchip *flash, unsigned int page, int blocksize);
 int write_sector_jedec(chipaddr bios, uint8_t *src,
    	       chipaddr dst, unsigned int page_size);

@@ -844,8 +847,7 @@
 /* m29f400bt.c */
 int probe_m29f400bt(struct flashchip *flash);
 int erase_m29f400bt(struct flashchip *flash);
-int block_erase_m29f400bt(chipaddr bios,
-				 chipaddr dst);
+int block_erase_m29f400bt(struct flashchip *flash, int start, int len);
 int write_m29f400bt(struct flashchip *flash, uint8_t *buf);
 int write_coreboot_m29f400bt(struct flashchip *flash, uint8_t *buf);
 void toggle_ready_m29f400bt(chipaddr dst);
Index: flashrom-check_erase_range/pm49fl00x.c
===================================================================
--- flashrom-check_erase_range/pm49fl00x.c	(Revision 591)
+++ flashrom-check_erase_range/pm49fl00x.c	(Arbeitskopie)
@@ -53,7 +53,6 @@
    int i;
    int total_size = flash->total_size * 1024;
    int page_size = flash->page_size;
-	chipaddr bios = flash->virtual_memory;

/* unprotected */
    write_lockbits_49fl00x(flash->virtual_registers,
@@ -69,7 +68,10 @@
    		continue;

/* erase the page */
-		erase_block_jedec(bios, i * page_size);
+		if (erase_block_jedec(flash, i * page_size, page_size)) {
+			fprintf(stderr, "ERASE FAILED!\n");
+			return -1;
+		}
    	printf("%04d at address: 0x%08x", i, i * page_size);
    	printf("\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b");
    	fflush(stdout);
@@ -100,7 +102,10 @@
    		continue;

/* erase the page before programming */
-		erase_block_jedec(bios, i * page_size);
+		if (erase_block_jedec(flash, i * page_size, page_size)) {
+			fprintf(stderr, "ERASE FAILED!\n");
+			return -1;
+		}

/* write to the sector */
    	printf("%04d at address: 0x%08x", i, i * page_size);
Index: flashrom-check_erase_range/en29f002a.c
===================================================================
--- flashrom-check_erase_range/en29f002a.c	(Revision 591)
+++ flashrom-check_erase_range/en29f002a.c	(Arbeitskopie)
@@ -98,7 +98,10 @@

//chip_writeb(0xF0, bios);
    programmer_delay(10);
-	erase_chip_jedec(flash);
+	if (erase_chip_jedec(flash)) {
+		fprintf(stderr, "ERASE FAILED!\n");
+		return -1;
+	}

printf("Programming page: ");
    for (i = 0; i < total_size; i++) {
Index: flashrom-check_erase_range/jedec.c
===================================================================
--- flashrom-check_erase_range/jedec.c	(Revision 591)
+++ flashrom-check_erase_range/jedec.c	(Arbeitskopie)
@@ -175,8 +175,10 @@
    return 0;
 }

-int erase_sector_jedec(chipaddr bios, unsigned int page)
+int erase_sector_jedec(struct flashchip *flash, unsigned int page, int pagesize)
 {
+	chipaddr bios = flash->virtual_memory;
+
    /*  Issue the Sector Erase command   */
    chip_writeb(0xAA, bios + 0x5555);
    programmer_delay(10);
@@ -195,11 +197,17 @@
    /* wait for Toggle bit ready         */
    toggle_ready_jedec(bios);

+	if (check_erased_range(flash, page, pagesize)) {
+		fprintf(stderr,"ERASE FAILED!\n");
+		return -1;
+	}
    return 0;
 }

-int erase_block_jedec(chipaddr bios, unsigned int block)
+int erase_block_jedec(struct flashchip *flash, unsigned int block, int blocksize)
 {
+	chipaddr bios = flash->virtual_memory;
+
    /*  Issue the Sector Erase command   */
    chip_writeb(0xAA, bios + 0x5555);
    programmer_delay(10);
@@ -218,11 +226,16 @@
    /* wait for Toggle bit ready         */
    toggle_ready_jedec(bios);

+	if (check_erased_range(flash, block, blocksize)) {
+		fprintf(stderr,"ERASE FAILED!\n");
+		return -1;
+	}
    return 0;
 }

int erase_chip_jedec(struct flashchip *flash)
 {
+	int total_size = flash->total_size * 1024;
    chipaddr bios = flash->virtual_memory;

/*  Issue the JEDEC Chip Erase command   */
@@ -242,6 +255,10 @@

toggle_ready_jedec(bios);

+	if (check_erased_range(flash, 0, total_size)) {
+		fprintf(stderr,"ERASE FAILED!\n");
+		return -1;
+	}
    return 0;
 }

@@ -342,15 +359,11 @@
    int page_size = flash->page_size;
    chipaddr bios = flash->virtual_memory;

-	erase_chip_jedec(flash);
-	// dumb check if erase was successful.
-	for (i = 0; i < total_size; i++) {
-		if (chip_readb(bios + i) != 0xff) {
-			printf("ERASE FAILED @%d, val %02x!\n", i, chip_readb(bios + i));
-			return -1;
-		}
+	if (erase_chip_jedec(flash)) {
+		fprintf(stderr,"ERASE FAILED!\n");
+		return -1;
    }
-
+	
    printf("Programming page: ");
    for (i = 0; i < total_size / page_size; i++) {
    	printf("%04d at address: 0x%08x", i, i * page_size);
Index: flashrom-check_erase_range/pm29f002.c
===================================================================
--- flashrom-check_erase_range/pm29f002.c	(Revision 591)
+++ flashrom-check_erase_range/pm29f002.c	(Arbeitskopie)
@@ -27,7 +27,10 @@
    chipaddr dst = bios;

/* Pm29F002T/B use the same erase method... */
-	erase_29f040b(flash);
+	if (erase_29f040b(flash)) {
+		fprintf(stderr, "ERASE FAILED!\n");
+		return -1;
+	}

printf("Programming page: ");
    for (i = 0; i < total_size; i++) {
Index: flashrom-check_erase_range/sst49lfxxxc.c
===================================================================
--- flashrom-check_erase_range/sst49lfxxxc.c	(Revision 591)
+++ flashrom-check_erase_range/sst49lfxxxc.c	(Arbeitskopie)
@@ -75,9 +75,10 @@
    return 0;
 }

-static int erase_sector_49lfxxxc(chipaddr bios, unsigned long address)
+static int erase_sector_49lfxxxc(struct flashchip *flash, unsigned long address, int sector_size)
 {
    unsigned char status;
+	chipaddr bios = flash->virtual_memory;

chip_writeb(SECTOR_ERASE, bios);
    chip_writeb(ERASE, bios + address);
@@ -91,6 +92,10 @@
    	}
    } while (!(status & STATUS_WSMS));

+	if (check_erased_range(flash, address, sector_size)) {
+		fprintf(stderr, "ERASE FAILED!\n");
+		return -1;
+	}
    return 0;
 }

@@ -156,7 +161,7 @@

write_lockbits_49lfxxxc(flash, 0);
    for (i = 0; i < total_size; i += flash->page_size)
-		if (erase_sector_49lfxxxc(bios, i) != 0)
+		if (erase_sector_49lfxxxc(flash, i, flash->page_size))
    		return (-1);

chip_writeb(RESET, bios);
@@ -175,7 +180,10 @@
    printf("Programming page: ");
    for (i = 0; i < total_size / page_size; i++) {
    	/* erase the page before programming */
-		erase_sector_49lfxxxc(bios, i * page_size);
+		if (erase_sector_49lfxxxc(flash, i * page_size, flash->page_size)) {
+			fprintf(stderr, "ERASE FAILED!\n");
+			return -1;
+		}

/* write to the sector */
    	printf("%04d at address: 0x%08x", i, i * page_size);
Index: flashrom-check_erase_range/sharplhf00l04.c
===================================================================
--- flashrom-check_erase_range/sharplhf00l04.c	(Revision 591)
+++ flashrom-check_erase_range/sharplhf00l04.c	(Arbeitskopie)
@@ -124,6 +124,10 @@
    print_lhf00l04_status(status);
    printf("DONE BLOCK 0x%x\n", offset);

+	if (check_erased_range(flash, offset, flash->page_size)) {
+		fprintf(stderr, "ERASE FAILED!\n");
+		return -1;
+	}
    return 0;
 }

@@ -135,7 +139,10 @@
    printf("total_size is %d; flash->page_size is %d\n",
           total_size, flash->page_size);
    for (i = 0; i < total_size; i += flash->page_size)
-		erase_lhf00l04_block(flash, i);
+		if (erase_lhf00l04_block(flash, i)) {
+			fprintf(stderr, "ERASE FAILED!\n");
+			return -1;
+		}
    printf("DONE ERASE\n");

return 0;
@@ -161,9 +168,8 @@
    int page_size = flash->page_size;
    chipaddr bios = flash->virtual_memory;

-	erase_lhf00l04(flash);
-	if (chip_readb(bios) != 0xff) {
-		printf("ERASE FAILED!\n");
+	if (erase_lhf00l04(flash)) {
+		fprintf(stderr, "ERASE FAILED!\n");
    	return -1;
    }
    printf("Programming page: ");
Index: flashrom-check_erase_range/wbsio_spi.c
===================================================================
--- flashrom-check_erase_range/wbsio_spi.c	(Revision 591)
+++ flashrom-check_erase_range/wbsio_spi.c	(Arbeitskopie)
@@ -196,7 +196,10 @@
    	return 1;
    }

-	flash->erase(flash);
+	if (flash->erase(flash)) {
+		fprintf(stderr, "ERASE FAILED!\n");
+		return -1;
+	}
    result = spi_write_enable();
    if (result)
    	return result;
Index: flashrom-check_erase_range/w39v040c.c
===================================================================
--- flashrom-check_erase_range/w39v040c.c	(Revision 591)
+++ flashrom-check_erase_range/w39v040c.c	(Arbeitskopie)
@@ -60,16 +60,13 @@
 {
    int i;
    unsigned int total_size = flash->total_size * 1024;
-	chipaddr bios = flash->virtual_memory;

-	for (i = 0; i < total_size; i += flash->page_size)
-		erase_sector_jedec(flash->virtual_memory, i);
-
-	for (i = 0; i < total_size; i++)
-		if (0xff != chip_readb(bios + i)) {
-			printf("ERASE FAILED at 0x%08x!  Expected=0xff, Read=0x%02x\n", i, chip_readb(bios + i));
+	for (i = 0; i < total_size; i += flash->page_size) {
+		if (erase_sector_jedec(flash, i, flash->page_size)) {
+			fprintf(stderr, "ERASE FAILED!\n");
    		return -1;
    	}
+	}

return 0;
 }
@@ -81,8 +78,10 @@
    int page_size = flash->page_size;
    chipaddr bios = flash->virtual_memory;

-	if (flash->erase(flash))
+	if (flash->erase(flash)) {
+		fprintf(stderr, "ERASE FAILED!\n");
    	return -1;
+	}

printf("Programming page: ");
    for (i = 0; i < total_size / page_size; i++) {
Index: flashrom-check_erase_range/m29f002.c
===================================================================
--- flashrom-check_erase_range/m29f002.c	(Revision 591)
+++ flashrom-check_erase_range/m29f002.c	(Arbeitskopie)
@@ -31,12 +31,19 @@
    chip_writeb(0x10, bios + 0x555);
    programmer_delay(10);
    toggle_ready_jedec(bios);
+	if (check_erased_range(flash, 0, flash->total_size * 1024)) {
+		fprintf(stderr, "ERASE FAILED!\n");
+		return -1;
+	}
    return 0;
 }

-static void rewrite_block(chipaddr bios, uint8_t *src,
-			  chipaddr dst, int size)
+static int rewrite_block(struct flashchip *flash, uint8_t *src,
+			  unsigned long start, int size)
 {
+	chipaddr bios = flash->virtual_memory;
+	chipaddr dst = bios + start;
+
    /* erase */
    chip_writeb(0xaa, bios + 0x555);
    chip_writeb(0x55, bios + 0xaaa);
@@ -46,6 +53,10 @@
    chip_writeb(0x30, dst);
    programmer_delay(10);
    toggle_ready_jedec(bios);
+	if (check_erased_range(flash, start, size)) {
+		fprintf(stderr, "ERASE FAILED!\n");
+		return -1;
+	}

/* program */
    while (size--) {
@@ -57,20 +68,24 @@
    	dst++;
    	src++;
    }
+	return 0;
 }

-static void do_block(chipaddr bios, uint8_t *src, int i,
+static int do_block(struct flashchip *flash, uint8_t *src, int i,
    	     unsigned long start, int size)
 {
+	int ret;
    printf("%d at address: 0x%08lx", i, start);
-	rewrite_block(bios, src + start, bios + start, size);
+	ret = rewrite_block(flash, src + start, start, size);
+	if (ret)
+		return ret;
    printf("\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b");
+	return 0;
 }

int write_m29f002t(struct flashchip *flash, uint8_t *buf)
 {
    int i, page_size = flash->page_size;
-	chipaddr bios = flash->virtual_memory;

/* M29F002(N)T has 7 blocks. From bottom to top their sizes are:
     * 64k 64k 64k 32k 8k 8k 16k
@@ -79,11 +94,11 @@

printf("Programming block: ");
    for (i = 0; i < 3; i++)
-		do_block(bios, buf, i, i * page_size, page_size);
-	do_block(bios, buf, i++, 0x30000, 32 * 1024);
-	do_block(bios, buf, i++, 0x38000, 8 * 1024);
-	do_block(bios, buf, i++, 0x3a000, 8 * 1024);
-	do_block(bios, buf, i, 0x3c000, 16 * 1024);
+		do_block(flash, buf, i, i * page_size, page_size);
+	do_block(flash, buf, i++, 0x30000, 32 * 1024);
+	do_block(flash, buf, i++, 0x38000, 8 * 1024);
+	do_block(flash, buf, i++, 0x3a000, 8 * 1024);
+	do_block(flash, buf, i, 0x3c000, 16 * 1024);

printf("\n");
    return 0;
@@ -92,7 +107,6 @@
 int write_m29f002b(struct flashchip *flash, uint8_t *buf)
 {
    int i = 0, page_size = flash->page_size;
-	chipaddr bios = flash->virtual_memory;

/* M29F002B has 7 blocks. From bottom to top their sizes are:
     * 16k 8k 8k 32k 64k 64k 64k
@@ -100,12 +114,12 @@
     */

printf("Programming block: ");
-	do_block(bios, buf, i++, 0x00000, 16 * 1024);
-	do_block(bios, buf, i++, 0x04000, 8 * 1024);
-	do_block(bios, buf, i++, 0x06000, 8 * 1024);
-	do_block(bios, buf, i++, 0x08000, 32 * 1024);
+	do_block(flash, buf, i++, 0x00000, 16 * 1024);
+	do_block(flash, buf, i++, 0x04000, 8 * 1024);
+	do_block(flash, buf, i++, 0x06000, 8 * 1024);
+	do_block(flash, buf, i++, 0x08000, 32 * 1024);
    for (; i < 7; i++)
-		do_block(bios, buf, i, (i - 3) * page_size, page_size);
+		do_block(flash, buf, i, (i - 3) * page_size, page_size);

printf("\n");
    return 0;
Index: flashrom-check_erase_range/w49f002u.c
===================================================================
--- flashrom-check_erase_range/w49f002u.c	(Revision 591)
+++ flashrom-check_erase_range/w49f002u.c	(Arbeitskopie)
@@ -27,7 +27,10 @@
    int page_size = flash->page_size;
    chipaddr bios = flash->virtual_memory;

-	erase_chip_jedec(flash);
+	if (erase_chip_jedec(flash)) {
+		fprintf(stderr, "ERASE FAILED!\n");
+		return -1;
+	}

printf("Programming page: ");
    for (i = 0; i < total_size / page_size; i++) {
Index: flashrom-check_erase_range/sst28sf040.c
===================================================================
--- flashrom-check_erase_range/sst28sf040.c	(Revision 591)
+++ flashrom-check_erase_range/sst28sf040.c	(Arbeitskopie)
@@ -54,14 +54,20 @@
    tmp = chip_readb(bios + 0x041A);
 }

-static int erase_sector_28sf040(chipaddr bios, unsigned long address)
+static int erase_sector_28sf040(struct flashchip *flash, unsigned long address, int sector_size)
 {
+	chipaddr bios = flash->virtual_memory;
+
    chip_writeb(AUTO_PG_ERASE1, bios);
    chip_writeb(AUTO_PG_ERASE2, bios + address);

/* wait for Toggle bit ready         */
    toggle_ready_jedec(bios);

+	if (check_erased_range(flash, address, sector_size)) {
+		fprintf(stderr, "ERASE FAILED!\n");
+		return -1;
+	}
    return 0;
 }

@@ -124,6 +130,10 @@
    programmer_delay(10);
    toggle_ready_jedec(bios);

+	if (check_erased_range(flash, 0, flash->total_size * 1024)) {
+		fprintf(stderr, "ERASE FAILED!\n");
+		return -1;
+	}
    return 0;
 }

@@ -139,7 +149,10 @@
    printf("Programming page: ");
    for (i = 0; i < total_size / page_size; i++) {
    	/* erase the page before programming */
-		erase_sector_28sf040(bios, i * page_size);
+		if (erase_sector_28sf040(flash, i * page_size, page_size)) {
+			fprintf(stderr, "ERASE FAILED!\n");
+			return -1;
+		}

/* write to the sector */
    	printf("%04d at address: 0x%08x", i, i * page_size);
Index: flashrom-check_erase_range/stm50flw0x0x.c
===================================================================
--- flashrom-check_erase_range/stm50flw0x0x.c	(Revision 591)
+++ flashrom-check_erase_range/stm50flw0x0x.c	(Arbeitskopie)
@@ -163,7 +163,6 @@
 int erase_block_stm50flw0x0x(struct flashchip *flash, int offset)
 {
    chipaddr bios = flash->virtual_memory + offset;
-	int j;

// clear status register
    chip_writeb(0x50, bios);
@@ -175,13 +174,10 @@

wait_stm50flw0x0x(flash->virtual_memory);

-	for (j = 0; j < flash->page_size; j++) {
-		if (chip_readb(bios + j) != 0xFF) {
-			printf("Erase failed at 0x%x\n", offset + j);
-			return -1;
-		}
+	if (check_erased_range(flash, offset, flash->page_size)) {
+		fprintf(stderr, "ERASE FAILED!\n");
+		return -1;
    }
-
    printf("DONE BLOCK 0x%x\n", offset);

return 0;
@@ -230,24 +226,29 @@
  */
 int erase_stm50flw0x0x(struct flashchip *flash)
 {
-	int i, rc = 0;
+	int i;
    int total_size = flash->total_size * 1024;
    int page_size = flash->page_size;
    chipaddr bios = flash->virtual_memory;

printf("Erasing page:\n");
-	for (i = 0; (i < total_size / page_size) && (rc == 0); i++) {
+	for (i = 0; i < total_size / page_size; i++) {
    	printf
    	    ("\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b");
    	printf("%04d at address: 0x%08x ", i, i * page_size);
-		rc = unlock_block_stm50flw0x0x(flash, i * page_size);
-		if (!rc)
-			rc = erase_block_stm50flw0x0x(flash, i * page_size);
+		if (unlock_block_stm50flw0x0x(flash, i * page_size)) {
+			fprintf(stderr, "UNLOCK FAILED!\n");
+			return -1;
+		}
+		if (erase_block_stm50flw0x0x(flash, i * page_size)) {
+			fprintf(stderr, "ERASE FAILED!\n");
+			return -1;
+		}
    }
    printf("\n");
    protect_stm50flw0x0x(bios);

-	return rc;
+	return 0;
 }

int write_stm50flw0x0x(struct flashchip *flash, uint8_t * buf)
Index: flashrom-check_erase_range/am29f040b.c
===================================================================
--- flashrom-check_erase_range/am29f040b.c	(Revision 591)
+++ flashrom-check_erase_range/am29f040b.c	(Arbeitskopie)
@@ -20,8 +20,11 @@

#include "flash.h"

-static int erase_sector_29f040b(chipaddr bios, unsigned long address)
+static int erase_sector_29f040b(struct flashchip *flash, unsigned long address)
 {
+	int page_size = flash->page_size;
+	chipaddr bios = flash->virtual_memory;
+
    chip_writeb(0xAA, bios + 0x555);
    chip_writeb(0x55, bios + 0x2AA);
    chip_writeb(0x80, bios + 0x555);
@@ -34,6 +37,10 @@
    /* wait for Toggle bit ready         */
    toggle_ready_jedec(bios + address);

+	if (check_erased_range(flash, address, page_size)) {
+		fprintf(stderr, "ERASE FAILED!\n");
+		return -1;
+	}
    return 0;
 }

@@ -86,6 +93,7 @@

int erase_29f040b(struct flashchip *flash)
 {
+	int total_size = flash->total_size * 1024;
    chipaddr bios = flash->virtual_memory;

chip_writeb(0xAA, bios + 0x555);
@@ -98,6 +106,10 @@
    programmer_delay(10);
    toggle_ready_jedec(bios);

+	if (check_erased_range(flash, 0, total_size)) {
+		fprintf(stderr, "ERASE FAILED!\n");
+		return -1;
+	}
    return 0;
 }

@@ -111,7 +123,10 @@
    printf("Programming page ");
    for (i = 0; i < total_size / page_size; i++) {
    	/* erase the page before programming */
-		erase_sector_29f040b(bios, i * page_size);
+		if (erase_sector_29f040b(flash, i * page_size)) {
+			fprintf(stderr, "ERASE FAILED!\n");
+			return -1;
+		}

/* write to the sector */
    	printf("%04d at address: ", i);
Index: flashrom-check_erase_range/sst_fwhub.c
===================================================================
--- flashrom-check_erase_range/sst_fwhub.c	(Revision 591)
+++ flashrom-check_erase_range/sst_fwhub.c	(Arbeitskopie)
@@ -94,7 +94,7 @@
    return 1;
 }

-int erase_sst_fwhub_block(struct flashchip *flash, int offset)
+int erase_sst_fwhub_block(struct flashchip *flash, int offset, int page_size)
 {
    uint8_t blockstatus = clear_sst_fwhub_block_lock(flash, offset);

@@ -104,7 +104,10 @@
    	return 1;
    }

-	erase_block_jedec(flash->virtual_memory, offset);
+	if (erase_block_jedec(flash, offset, page_size)) {
+		fprintf(stderr, "ERASE FAILED!\n");
+		return -1;
+	}
    toggle_ready_jedec(flash->virtual_memory);

return 0;
@@ -114,15 +117,10 @@
 {
    int i;
    unsigned int total_size = flash->total_size * 1024;
-	chipaddr bios = flash->virtual_memory;

-	for (i = 0; i < total_size; i += flash->page_size)
-		erase_sst_fwhub_block(flash, i);
-
-	// dumb check if erase was successful.
-	for (i = 0; i < total_size; i++) {
-		if (chip_readb(bios + i) != 0xff) {
-			printf("ERASE FAILED!\n");
+	for (i = 0; i < total_size; i += flash->page_size) {
+		if (erase_sst_fwhub_block(flash, i, flash->page_size)) {
+			fprintf(stderr, "ERASE FAILED!\n");
    		return -1;
    	}
    }
@@ -139,8 +137,10 @@
    uint8_t blockstatus;

// FIXME: We want block wide erase instead of ironing the whole chip
-	if (erase_sst_fwhub(flash))
+	if (erase_sst_fwhub(flash)) {
+		fprintf(stderr, "ERASE FAILED!\n");
    	return -1;
+	}

printf("Programming page: ");
    for (i = 0; i < total_size / page_size; i++) {
Index: flashrom-check_erase_range/sb600spi.c
===================================================================
--- flashrom-check_erase_range/sb600spi.c	(Revision 591)
+++ flashrom-check_erase_range/sb600spi.c	(Arbeitskopie)
@@ -63,7 +63,10 @@

/* Erase first */
    printf("Erasing flash before programming... ");
-	flash->erase(flash);
+	if (flash->erase(flash)) {
+		fprintf(stderr, "ERASE FAILED!\n");
+		return -1;
+	}
    printf("done.\n");

printf("Programming flash");
Index: flashrom-check_erase_range/w39v080fa.c
===================================================================
--- flashrom-check_erase_range/w39v080fa.c	(Revision 591)
+++ flashrom-check_erase_range/w39v080fa.c	(Arbeitskopie)
@@ -142,9 +142,10 @@
    return 0;
 }

-static int erase_sector_winbond_fwhub(chipaddr bios,
+static int erase_sector_winbond_fwhub(struct flashchip *flash,
    			      unsigned int sector)
 {
+	chipaddr bios = flash->virtual_memory;
    /* Remember: too much sleep can waste your day. */

printf("0x%08x\b\b\b\b\b\b\b\b\b\b", sector);
@@ -161,30 +162,30 @@
    /* wait for Toggle bit ready */
    toggle_ready_jedec(bios);

+	if (check_erased_range(flash, sector, flash->page_size)) {
+		fprintf(stderr, "ERASE FAILED!\n");
+		return -1;
+	}
    return 0;
 }

int erase_winbond_fwhub(struct flashchip *flash)
 {
    int i, total_size = flash->total_size * 1024;
-	chipaddr bios = flash->virtual_memory;

unlock_winbond_fwhub(flash);

printf("Erasing:     ");

-	for (i = 0; i < total_size; i += flash->page_size)
-		erase_sector_winbond_fwhub(bios, i);
-
-	printf("\n");
-
-	for (i = 0; i < total_size; i++) {
-		if (chip_readb(bios + i) != 0xff) {
-			fprintf(stderr, "Error: Flash chip erase failed at 0x%08x(0x%02x)\n", i, chip_readb(bios + i));
+	for (i = 0; i < total_size; i += flash->page_size) {
+		if (erase_sector_winbond_fwhub(flash, i)) {
+			fprintf(stderr, "ERASE FAILED!\n");
    		return -1;
    	}
    }

+	printf("\n");
+
    return 0;
 }

Index: flashrom-check_erase_range/flashrom.c
===================================================================
--- flashrom-check_erase_range/flashrom.c	(Revision 591)
+++ flashrom-check_erase_range/flashrom.c	(Arbeitskopie)
@@ -205,6 +205,11 @@
    return (a < b) ? a : b;
 }

+int max(int a, int b)
+{
+	return (a > b) ? a : b;
+}
+
 char *strcat_realloc(char *dest, const char *src)
 {
    dest = realloc(dest, strlen(dest) + strlen(src) + 1);
@@ -245,6 +250,81 @@
    return ret;
 }

+/* start is an offset to the base address of the flash chip */
+int check_erased_range(struct flashchip *flash, int start, int len)
+{
+	int ret;
+	uint8_t *cmpbuf = malloc(len);
+
+	if (!cmpbuf) {
+		fprintf(stderr, "Could not allocate memory!\n");
+		exit(1);
+	}
+	memset(cmpbuf, 0xff, len);
+	ret = verify_range(flash, cmpbuf, start, len, "ERASE");
+	free(cmpbuf);
+	return ret;
+}
+
+/* start is an offset to the base address of the flash chip */
+int verify_range(struct flashchip *flash, uint8_t *cmpbuf, int start, int len, char *message)
+{
+	int i, j, starthere, lenhere, ret = 0;
+	chipaddr bios = flash->virtual_memory;
+	int page_size = flash->page_size;
+	uint8_t *readbuf = malloc(page_size);
+
+	if (!len)
+		goto out_free;
+
+	if (!readbuf) {
+		fprintf(stderr, "Could not allocate memory!\n");
+		exit(1);
+	}
+
+	if (start + len > flash->total_size * 1024) {
+		fprintf(stderr, "Error: %s called with start 0x%x + len 0x%x >"
+			" total_size 0x%x\n", __func__, start, len,
+			flash->total_size * 1024);
+		ret = -1;
+		goto out_free;
+	}
+	if (!message)
+		message = "VERIFY";
+	
+	/* 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
+	 * division rounds down. The highest page number we want is the page
+	 * where the last byte of the range lives. That last byte has the
+	 * address (start + len - 1), thus the highest page number is
+	 * (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 = max(start, i * page_size);
+		/* Length of bytes in the range in this page. */
+		lenhere = min(start + len, (i + 1) * page_size) - starthere;
+		/* starthere is an offset to the base address of the chip. */
+		chip_readn(readbuf, bios + starthere, lenhere);
+		for (j = 0; j < lenhere; j++) {
+			if (cmpbuf[starthere + j] != readbuf[j]) {
+				fprintf(stderr, "%s FAILED at 0x%08x! "
+					"Expected=0x%02x, Read=0x%02x\n",
+					message, starthere + j,
+					cmpbuf[starthere + j], readbuf[j]);
+				ret = -1;
+				goto out_free;
+			}
+		}
+	}
+
+out_free:
+	free(readbuf);
+	return ret;
+}
+
 struct flashchip *probe_flash(struct flashchip *first_flash, int force)
 {
    struct flashchip *flash;
@@ -388,6 +468,9 @@
    }
    flash->erase(flash);

+	/* FIXME: The lines below are superfluous. We should check the result
+	 * of flash->erase(flash) instead.
+	 */
    if (!flash->read) {
    	printf("FAILED!\n");
    	fprintf(stderr, "ERROR: flashrom has no read function for this flash chip.\n");
Index: flashrom-check_erase_range/82802ab.c
===================================================================
--- flashrom-check_erase_range/82802ab.c	(Revision 591)
+++ flashrom-check_erase_range/82802ab.c	(Arbeitskopie)
@@ -110,7 +110,6 @@
 {
    chipaddr bios = flash->virtual_memory + offset;
    chipaddr wrprotect = flash->virtual_registers + offset + 2;
-	int j;
    uint8_t status;

// clear status register
@@ -129,11 +128,9 @@
    // now let's see what the register is
    status = wait_82802ab(flash->virtual_memory);
    //print_82802ab_status(status);
-	for (j = 0; j < flash->page_size; j++) {
-		if (chip_readb(bios + j) != 0xFF) {
-			printf("BLOCK ERASE failed at 0x%x\n", offset);
-			return -1;
-		}
+	if (check_erased_range(flash, offset, flash->page_size)) {
+		fprintf(stderr, "ERASE FAILED!\n");
+		return -1;
    }
    printf("DONE BLOCK 0x%x\n", offset);

@@ -148,7 +145,10 @@
    printf("total_size is %d; flash->page_size is %d\n",
           total_size, flash->page_size);
    for (i = 0; i < total_size; i += flash->page_size)
-		erase_82802ab_block(flash, i);
+		if (erase_82802ab_block(flash, i)) {
+			fprintf(stderr, "ERASE FAILED!\n");
+			return -1;
+		}
    printf("DONE ERASE\n");

return 0;
@@ -199,7 +199,10 @@
    	}

/* erase block by block and write block by block; this is the most secure way */
-		erase_82802ab_block(flash, i * page_size);
+		if (erase_82802ab_block(flash, i * page_size)) {
+			fprintf(stderr, "ERASE FAILED!\n");
+			return -1;
+		}
    	write_page_82802ab(bios, buf + i * page_size,
    			   bios + i * page_size, page_size);
    }
Index: flashrom-check_erase_range/mx29f002.c
===================================================================
--- flashrom-check_erase_range/mx29f002.c	(Revision 591)
+++ flashrom-check_erase_range/mx29f002.c	(Arbeitskopie)
@@ -71,6 +71,10 @@
    chip_writeb(0x30, bios + 0x3bfff);
 #endif

+	if (check_erased_range(flash, 0, flash->total_size * 1024)) {
+		fprintf(stderr, "ERASE FAILED!\n");
+		return -1;
+	}
    return 0;
 }

@@ -83,7 +87,10 @@

chip_writeb(0xF0, bios);
    programmer_delay(10);
-	erase_29f002(flash);
+	if (erase_29f002(flash)) {
+		fprintf(stderr, "ERASE FAILED!\n");
+		return -1;
+	}
    //*bios = 0xF0;
 #if 1
    printf("Programming page: ");
Index: flashrom-check_erase_range/m29f400bt.c
===================================================================
--- flashrom-check_erase_range/m29f400bt.c	(Revision 591)
+++ flashrom-check_erase_range/m29f400bt.c	(Arbeitskopie)
@@ -98,11 +98,17 @@
    programmer_delay(10);
    toggle_ready_jedec(bios);

+	if (check_erased_range(flash, 0, flash->total_size * 1024)) {
+		fprintf(stderr, "ERASE FAILED!\n");
+		return -1;
+	}
    return 0;
 }

-int block_erase_m29f400bt(chipaddr bios, chipaddr dst)
+int block_erase_m29f400bt(struct flashchip *flash, int start, int len)
 {
+	chipaddr bios = flash->virtual_memory;
+	chipaddr dst = bios + start;

chip_writeb(0xAA, bios + 0xAAA);
    chip_writeb(0x55, bios + 0x555);
@@ -116,6 +122,10 @@
    programmer_delay(10);
    toggle_ready_jedec(bios);

+	if (check_erased_range(flash, start, len)) {
+		fprintf(stderr, "ERASE FAILED!\n");
+		return -1;
+	}
    return 0;
 }

@@ -146,26 +156,41 @@
    printf("total_size/page_size = %d\n", total_size / page_size);
    for (i = 0; i < (total_size / page_size) - 1; i++) {
    	printf("%04d at address: 0x%08x\n", i, i * page_size);
-		block_erase_m29f400bt(bios, bios + i * page_size);
+		if (block_erase_m29f400bt(flash, i * page_size, page_size)) {
+			fprintf(stderr, "ERASE FAILED!\n");
+			return -1;
+		}
    	write_page_m29f400bt(bios, buf + i * page_size,
    			     bios + i * page_size, page_size);
    	printf("\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b");
    }

printf("%04d at address: 0x%08x\n", 7, 0x70000);
-	block_erase_m29f400bt(bios, bios + 0x70000);
+	if (block_erase_m29f400bt(flash, 0x70000, 32 * 1024)) {
+		fprintf(stderr, "ERASE FAILED!\n");
+		return -1;
+	}
    write_page_m29f400bt(bios, buf + 0x70000, bios + 0x70000, 32 * 1024);

printf("%04d at address: 0x%08x\n", 8, 0x78000);
-	block_erase_m29f400bt(bios, bios + 0x78000);
+	if (block_erase_m29f400bt(flash, 0x78000, 8 * 1024)) {
+		fprintf(stderr, "ERASE FAILED!\n");
+		return -1;
+	}
    write_page_m29f400bt(bios, buf + 0x78000, bios + 0x78000, 8 * 1024);

printf("%04d at address: 0x%08x\n", 9, 0x7a000);
-	block_erase_m29f400bt(bios, bios + 0x7a000);
+	if (block_erase_m29f400bt(flash, 0x7a000, 8 * 1024)) {
+		fprintf(stderr, "ERASE FAILED!\n");
+		return -1;
+	}
    write_page_m29f400bt(bios, buf + 0x7a000, bios + 0x7a000, 8 * 1024);

printf("%04d at address: 0x%08x\n", 10, 0x7c000);
-	block_erase_m29f400bt(bios, bios + 0x7c000);
+	if (block_erase_m29f400bt(flash, 0x7c000, 16 * 1024)) {
+		fprintf(stderr, "ERASE FAILED!\n");
+		return -1;
+	}
    write_page_m29f400bt(bios, buf + 0x7c000, bios + 0x7c000, 16 * 1024);

printf("\n");
@@ -195,19 +220,31 @@
    * 64	0x00000		0x0ffff		BOTTOM
    *********************************/
    printf("%04d at address: 0x%08x\n", 7, 0x00000);
-	block_erase_m29f400bt(bios, bios + 0x00000);
+	if (block_erase_m29f400bt(flash, 0x00000, 64 * 1024)) {
+		fprintf(stderr, "ERASE FAILED!\n");
+		return -1;
+	}
    write_page_m29f400bt(bios, buf + 0x00000, bios + 0x00000, 64 * 1024);

printf("%04d at address: 0x%08x\n", 7, 0x10000);
-	block_erase_m29f400bt(bios, bios + 0x10000);
+	if (block_erase_m29f400bt(flash, 0x10000, 64 * 1024)) {
+		fprintf(stderr, "ERASE FAILED!\n");
+		return -1;
+	}
    write_page_m29f400bt(bios, buf + 0x10000, bios + 0x10000, 64 * 1024);

printf("%04d at address: 0x%08x\n", 7, 0x20000);
-	block_erase_m29f400bt(bios, bios + 0x20000);
+	if (block_erase_m29f400bt(flash, 0x20000, 64 * 1024)) {
+		fprintf(stderr, "ERASE FAILED!\n");
+		return -1;
+	}
    write_page_m29f400bt(bios, buf + 0x20000, bios + 0x20000, 64 * 1024);

printf("%04d at address: 0x%08x\n", 7, 0x30000);
-	block_erase_m29f400bt(bios, bios + 0x30000);
+	if (block_erase_m29f400bt(flash, 0x30000, 64 * 1024)) {
+		fprintf(stderr, "ERASE FAILED!\n");
+		return -1;
+	}
    write_page_m29f400bt(bios, buf + 0x30000, bios + 0x30000, 64 * 1024);

printf("\n");
Index: flashrom-check_erase_range/sst49lf040.c
===================================================================
--- flashrom-check_erase_range/sst49lf040.c	(Revision 591)
+++ flashrom-check_erase_range/sst49lf040.c	(Arbeitskopie)
@@ -25,12 +25,14 @@
    int i;
    int total_size = flash->total_size * 1024;
    int page_size = flash->page_size;
-	chipaddr bios = flash->virtual_memory;

for (i = 0; i < total_size / page_size; i++) {
    	/* Chip erase only works in parallel programming mode
    	 * for the 49lf040. Use sector-erase instead */
-		erase_sector_jedec(bios, i * page_size);
+		if (erase_sector_jedec(flash, i * page_size, page_size)) {
+			fprintf(stderr, "ERASE FAILED!\n");
+			return -1;
+		}
    }

return 0;
@@ -48,7 +50,10 @@
    	/* erase the page before programming
    	 * Chip erase only works in parallel programming mode
    	 * for the 49lf040. Use sector-erase instead */
-		erase_sector_jedec(bios, i * page_size);
+		if (erase_sector_jedec(flash, i * page_size, page_size)) {
+			fprintf(stderr, "ERASE FAILED!\n");
+			return -1;
+		}

/* write to the sector */
    	if (i % 10 == 0)

[PATCH] flashrom: Check if erase worked, check return codes

[Uwe Hermann]

On Mon, Jun 15, 2009 at 12:26:46PM +0200, Carl-Daniel Hailfinger wrote:
> I wasn't really awake while changing that code. Sorry. Same for the
> other bugs you found.
>
> Compared to your version, I changed the following:
> Check inside all erase_*_jedec routines if erase worked, not outside.
> Rename rv to ret. Most functions in flashrom call the return variable ret.
> erase_sector_jedec and erase_block_jedec have changed prototypes to
>
> enable erase checking.
> verify_range uses goto out_free to make sure we don't forget to free().
> Convert all remaining erase functions and actually check return codes
>
> almost everywhere.
>
> Urja, would you remind reviewing the whole patch again? At 1087 lines of
> manual conversions, it is too big for me to be 100% confident that I got
> everything right.
>
> Signed-off-by: Carl-Daniel Hailfinger

With the two bugfixes discussed on IRC this is:

Acked-by: Uwe Hermann

I successfully tested LPC on an CK804 box and SPI on some sb600 box.
> +/* start is an offset to the base address of the flash chip */

Please start sentences with capital letter and end them with full stop
for consistency.
> +/* start is an offset to the base address of the flash chip */

Ditto.
> @@ -146,26 +156,41 @@
>    printf("total_size/page_size = %d\n", total_size / page_size);
>    for (i = 0; i < (total_size / page_size) - 1; i++) {
>    	printf("%04d at address: 0x%08x\n", i, i * page_size);
> block_erase_m29f400bt(bios, bios + i * page_size);
> if (block_erase_m29f400bt(flash, i * page_size, page_size)) {
> 	fprintf(stderr, "ERASE FAILED!\n");
> 	return -1;
> }
>
> write_page_m29f400bt(bios, buf + i * page_size,
> 		     bios + i * page_size, page_size);
> printf("\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b");
> }
>
> printf("%04d at address: 0x%08x\n", 7, 0x70000);
> block_erase_m29f400bt(bios, bios + 0x70000);
>
> if (block_erase_m29f400bt(flash, 0x70000, 32 * 1024)) {
> fprintf(stderr, "ERASE FAILED!\n");
> return -1;
>
> }
> write_page_m29f400bt(bios, buf + 0x70000, bios + 0x70000, 32 * 1024);
>
> printf("%04d at address: 0x%08x\n", 8, 0x78000);
> block_erase_m29f400bt(bios, bios + 0x78000);
>
> if (block_erase_m29f400bt(flash, 0x78000, 8 * 1024)) {
> fprintf(stderr, "ERASE FAILED!\n");
> return -1;
>
> }
> write_page_m29f400bt(bios, buf + 0x78000, bios + 0x78000, 8 * 1024);
>
> printf("%04d at address: 0x%08x\n", 9, 0x7a000);
> block_erase_m29f400bt(bios, bios + 0x7a000);
>
> if (block_erase_m29f400bt(flash, 0x7a000, 8 * 1024)) {
> fprintf(stderr, "ERASE FAILED!\n");
> return -1;
>
> }
> write_page_m29f400bt(bios, buf + 0x7a000, bios + 0x7a000, 8 * 1024);
>
> printf("%04d at address: 0x%08x\n", 10, 0x7c000);
> block_erase_m29f400bt(bios, bios + 0x7c000);
> if (block_erase_m29f400bt(flash, 0x7c000, 16 * 1024)) {
> fprintf(stderr, "ERASE FAILED!\n");
> return -1;
> }

This code need some refactoring IMHO, we can easily make an array
of block sizes and loop over that (that's for another patch though).

Uwe.

[PATCH] flashrom: Check if erase worked, check return codes

[Carl-Daniel Hailfinger]

On 15.06.2009 18:12, Uwe Hermann wrote:
> On Mon, Jun 15, 2009 at 12:26:46PM +0200, Carl-Daniel Hailfinger wrote:
> > I wasn't really awake while changing that code. Sorry. Same for the
> > other bugs you found.
> >
> > Compared to your version, I changed the following:
> > Check inside all erase_*_jedec routines if erase worked, not outside.
> > Rename rv to ret. Most functions in flashrom call the return variable ret.
> > erase_sector_jedec and erase_block_jedec have changed prototypes to
> >
> > enable erase checking.
> > verify_range uses goto out_free to make sure we don't forget to free().
> > Convert all remaining erase functions and actually check return codes
> >
> > almost everywhere.
> >
> > Urja, would you remind reviewing the whole patch again? At 1087 lines of
> > manual conversions, it is too big for me to be 100% confident that I got
> > everything right.
> >
> > Signed-off-by: Carl-Daniel Hailfinger
>
> With the two bugfixes discussed on IRC this is:
>
> Acked-by: Uwe Hermann
>
> I successfully tested LPC on an CK804 box and SPI on some sb600 box.

Thanks, committed with your suggestions in r595.
I added some doxygen comments to verify_range() as well.
> > printf("%04d at address: 0x%08x\n", 9, 0x7a000);
> > block_erase_m29f400bt(bios, bios + 0x7a000);
> >
> > if (block_erase_m29f400bt(flash, 0x7a000, 8 * 1024)) {
> > fprintf(stderr, "ERASE FAILED!\n");
> > return -1;
> >
> > }
> > write_page_m29f400bt(bios, buf + 0x7a000, bios + 0x7a000, 8 * 1024);
> >
> > printf("%04d at address: 0x%08x\n", 10, 0x7c000);
> > block_erase_m29f400bt(bios, bios + 0x7c000);
> > if (block_erase_m29f400bt(flash, 0x7c000, 16 * 1024)) {
> > fprintf(stderr, "ERASE FAILED!\n");
> > return -1;
> > }
>
> This code need some refactoring IMHO, we can easily make an array
> of block sizes and loop over that (that's for another patch though).

Actually, my eraseblock patch solves this nicely.

Regards,
Carl-Daniel