[coreboot] [PATCH] flashrom: Check if erase worked, check return codes
Urja Rannikko
urjaman at gmail.com
Sun Jun 14 22:13:27 CEST 2009
On Fri, Jun 12, 2009 at 17:01, Carl-Daniel
Hailfinger<c-d.hailfinger.devel.2006 at gmx.net> 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 <c-d.hailfinger.devel.2006 at gmx.net>
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 <urjaman at gmail.com>
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);
}
--
urjaman
-------------- next part --------------
A non-text attachment was scrubbed...
Name: flashrom_check_erase_range03.diff
Type: application/octet-stream
Size: 13675 bytes
Desc: not available
URL: <http://www.coreboot.org/pipermail/coreboot/attachments/20090614/ae406ab5/attachment.obj>
More information about the coreboot
mailing list