Overall the patch looks good and didn't cause any failures with some testing when I ran thru a basic set of read / erase / write / partial write tests.

On Fri, Oct 15, 2010 at 6:22 AM, Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006@gmx.net> wrote:
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.

Good idea in general IMHO, however as you mention a config option would be nice.

Also, perhaps oldcontents should be written out if verification fails. That could wait for a follow-up patch.

On Fri, Oct 15, 2010 at 6:22 AM, Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006@gmx.net> wrote: 
-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)

-               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);

As discussed on IRC, this function can probably use a better name and comments... in another patch :-)

On Fri, Oct 15, 2010 at 6:22 AM, Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006@gmx.net> wrote:
@@ -1634,33 +1651,71 @@

+       newcontents = (uint8_t *) calloc(size, sizeof(char));

+1 on Stefan's comment here... malloc / memset is probably considered better form by some. But that's just a matter of preference.

David Hendricks (dhendrix)
Systems Software Engineer, Google Inc.