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 <[email protected]>
Acked-by: Stefan Reinauer <[email protected]>

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 [email protected]!\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;

_______________________________________________
flashrom mailing list
[email protected]
http://www.flashrom.org/mailman/listinfo/flashrom

Reply via email to