From 42d38a9dd1ef58870635c0e003b1a37e89a51ba7 Mon Sep 17 00:00:00 2001 From: Carl-Daniel Hailfinger Date: Tue, 19 Oct 2010 22:06:20 +0000 Subject: Always read the flash chip before writing This will allow flashrom to skip erase of already-erased blocks and to skip write of blocks which already have the wanted contents. Avoid emergency messages by checking if the chip contents after a failed write operation (erase/write) are unchanged. Keep the emergency messages after a failed pure erase. That part is debatable because if someone wants erase, he pretty sure doesn't care about the flash contents anymore. Please note that this introduces additional overhead of a full chip read before write. This is frowned upon by people with slow programmers. A followup patch will make this configurable. Corresponding to flashrom svn r1215. Signed-off-by: Carl-Daniel Hailfinger Acked-by: Stefan Reinauer --- flash.h | 2 +- flashrom.c | 77 +++++++++++++++++++++++++++++++++++++++++++++++++++++--------- layout.c | 9 ++++---- 3 files changed, 71 insertions(+), 17 deletions(-) diff --git a/flash.h b/flash.h index 1b7a47a..81fdbd7 100644 --- a/flash.h +++ b/flash.h @@ -239,7 +239,7 @@ int cli_classic(int argc, char *argv[]); /* layout.c */ int read_romlayout(char *name); int find_romentry(char *name); -int handle_romentries(uint8_t *buffer, struct flashchip *flash); +int handle_romentries(struct flashchip *flash, uint8_t *oldcontents, uint8_t *newcontents); /* spi.c */ struct spi_command { diff --git a/flashrom.c b/flashrom.c index 5451c60..f1edfca 100644 --- a/flashrom.c +++ b/flashrom.c @@ -1343,6 +1343,19 @@ int erase_flash(struct flashchip *flash) return ret; } +void nonfatal_help_message(void) +{ + msg_gerr("Writing to the flash chip apparently didn't do anything.\n" + "This means we have to add special support for your board, " + "programmer or flash chip.\n" + "Please report this on IRC at irc.freenode.net (channel " + "#flashrom) or\n" + "mail flashrom@flashrom.org!\n" + "-------------------------------------------------------------" + "------------------\n" + "You may now reboot or simply leave the machine running.\n"); +} + void emergency_help_message(void) { msg_gerr("Your flash chip is in an unknown state.\n" @@ -1602,9 +1615,10 @@ int chip_safety_check(struct flashchip *flash, int force, char *filename, int re */ int doit(struct flashchip *flash, int force, char *filename, int read_it, int write_it, int erase_it, int verify_it) { - uint8_t *buf; + uint8_t *oldcontents; + uint8_t *newcontents; int ret = 0; - unsigned long size; + unsigned long size = flash->total_size * 1024; if (chip_safety_check(flash, force, filename, read_it, write_it, erase_it, verify_it)) { msg_cerr("Aborting.\n"); @@ -1612,9 +1626,6 @@ int doit(struct flashchip *flash, int force, char *filename, int read_it, int wr return 1; } - size = flash->total_size * 1024; - buf = (uint8_t *) calloc(size, sizeof(char)); - /* Given the existence of read locks, we want to unlock for read, * erase and write. */ @@ -1627,6 +1638,12 @@ int doit(struct flashchip *flash, int force, char *filename, int read_it, int wr return ret; } if (erase_it) { + /* FIXME: Do we really want the scary warning if erase failed? + * After all, after erase the chip is either blank or partially + * blank or it has the old contents. A blank chip won't boot, + * so if the user wanted erase and reboots afterwards, the user + * knows very well that booting won't work. + */ if (erase_flash(flash)) { emergency_help_message(); programmer_shutdown(); @@ -1634,33 +1651,71 @@ int doit(struct flashchip *flash, int force, char *filename, int read_it, int wr } } + newcontents = (uint8_t *) calloc(size, sizeof(char)); + if (write_it || verify_it) { - if (read_buf_from_file(buf, flash->total_size * 1024, filename)) { + if (read_buf_from_file(newcontents, size, filename)) { programmer_shutdown(); return 1; } #if CONFIG_INTERNAL == 1 - show_id(buf, size, force); + if (programmer == PROGRAMMER_INTERNAL) + show_id(newcontents, size, force); #endif } + oldcontents = (uint8_t *) calloc(size, sizeof(char)); + + /* Read the whole chip to be able to check whether regions need to be + * erased and to give better diagnostics in case write fails. + * The alternative would be to read only the regions which are to be + * preserved, but in that case we might perform unneeded erase which + * takes time as well. + */ + msg_cdbg("Reading old flash chip contents...\n"); + if (flash->read(flash, oldcontents, 0, size)) { + programmer_shutdown(); + return 1; + } + // This should be moved into each flash part's code to do it // cleanly. This does the job. - handle_romentries(buf, flash); + handle_romentries(flash, oldcontents, newcontents); // //////////////////////////////////////////////////////////// if (write_it) { if (erase_flash(flash)) { + msg_cerr("Uh oh. Erase failed. Checking if anything " + "changed.\n"); + if (!flash->read(flash, newcontents, 0, size)) { + if (!memcmp(oldcontents, newcontents, size)) { + msg_cinfo("Good. It seems nothing was " + "changed.\n"); + nonfatal_help_message(); + programmer_shutdown(); + return 1; + } + } emergency_help_message(); programmer_shutdown(); return 1; } msg_cinfo("Writing flash chip... "); - ret = flash->write(flash, buf, 0, flash->total_size * 1024); + ret = flash->write(flash, newcontents, 0, size); if (ret) { - msg_cerr("FAILED!\n"); + msg_cerr("Uh oh. Write failed. Checking if anything " + "changed.\n"); + if (!flash->read(flash, newcontents, 0, size)) { + if (!memcmp(oldcontents, newcontents, size)) { + msg_cinfo("Good. It seems nothing was " + "changed.\n"); + nonfatal_help_message(); + programmer_shutdown(); + return 1; + } + } emergency_help_message(); programmer_shutdown(); return 1; @@ -1673,7 +1728,7 @@ int doit(struct flashchip *flash, int force, char *filename, int read_it, int wr /* Work around chips which need some time to calm down. */ if (write_it) programmer_delay(1000*1000); - ret = verify_flash(flash, buf); + ret = verify_flash(flash, newcontents); /* If we tried to write, and verification now fails, we * might have an emergency situation. */ diff --git a/layout.c b/layout.c index c01e09c..b24e4dd 100644 --- a/layout.c +++ b/layout.c @@ -205,7 +205,7 @@ int find_romentry(char *name) return -1; } -int handle_romentries(uint8_t *buffer, struct flashchip *flash) +int handle_romentries(struct flashchip *flash, uint8_t *oldcontents, uint8_t *newcontents) { int i; @@ -225,13 +225,12 @@ int handle_romentries(uint8_t *buffer, struct flashchip *flash) // normal will be updated and the rest will be kept. for (i = 0; i < romimages; i++) { - if (rom_entries[i].included) continue; - flash->read(flash, buffer + rom_entries[i].start, - rom_entries[i].start, - rom_entries[i].end - rom_entries[i].start + 1); + memcpy(newcontents + rom_entries[i].start, + oldcontents + rom_entries[i].start, + rom_entries[i].end - rom_entries[i].start + 1); } return 0; -- cgit v1.1