Author: hailfinger Date: Wed Oct 20 00:06:20 2010 New Revision: 1215 URL: http://flashrom.org/trac/flashrom/changeset/1215
Log: 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.
Signed-off-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net Acked-by: Stefan Reinauer stepan@coreboot.org
Modified: trunk/flash.h trunk/flashrom.c trunk/layout.c
Modified: trunk/flash.h ============================================================================== --- trunk/flash.h Tue Oct 19 00:32:03 2010 (r1214) +++ trunk/flash.h Wed Oct 20 00:06:20 2010 (r1215) @@ -239,7 +239,7 @@ /* 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 {
Modified: trunk/flashrom.c ============================================================================== --- trunk/flashrom.c Tue Oct 19 00:32:03 2010 (r1214) +++ trunk/flashrom.c Wed Oct 20 00:06:20 2010 (r1215) @@ -1343,6 +1343,19 @@ 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 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 @@ 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 @@ 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 @@ } }
+ 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 @@ /* 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. */
Modified: trunk/layout.c ============================================================================== --- trunk/layout.c Tue Oct 19 00:32:03 2010 (r1214) +++ trunk/layout.c Wed Oct 20 00:06:20 2010 (r1215) @@ -205,7 +205,7 @@ 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 @@ // 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;