<div class="gmail_quote"><meta http-equiv="content-type" content="text/html; charset=utf-8">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.</div>

<div class="gmail_quote"><br></div><div class="gmail_quote">On Fri, Oct 15, 2010 at 6:22 AM, Carl-Daniel Hailfinger <span dir="ltr"><<a href="mailto:c-d.hailfinger.devel.2006@gmx.net">c-d.hailfinger.devel.2006@gmx.net</a>></span> wrote:</div>

<div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">Always read the flash chip before writing. This will allow flashrom to<br>
skip erase of already-erased blocks and to skip write of blocks which<br>
already have the wanted contents.<br></blockquote><div><br></div><div>Good idea in general IMHO, however as you mention a config option would be nice.</div><div><br></div><div>Also, perhaps oldcontents should be written out if verification fails. That could wait for a follow-up patch.</div>

<div><br></div><div>On Fri, Oct 15, 2010 at 6:22 AM, Carl-Daniel Hailfinger <span dir="ltr"><<a href="mailto:c-d.hailfinger.devel.2006@gmx.net">c-d.hailfinger.devel.2006@gmx.net</a>></span> wrote: </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">

-int handle_romentries(uint8_t *buffer, struct flashchip *flash)<br>
+int handle_romentries(struct flashchip *flash, uint8_t *oldcontents, uint8_t *newcontents)<br>
 {<br>
        int i;<br>
<br>
@@ -225,13 +225,12 @@<br>
        // normal will be updated and the rest will be kept.<br>
<br>
        for (i = 0; i < romimages; i++) {<br>
-<br>
                if (rom_entries[i].included)<br>
                        continue;<br>
<br>
-               flash->read(flash, buffer + rom_entries[i].start,<br>
-                           rom_entries[i].start,<br>
-                           rom_entries[i].end - rom_entries[i].start + 1);<br>
+               memcpy(newcontents + rom_entries[i].start,<br>
+                      oldcontents + rom_entries[i].start,<br>
+                      rom_entries[i].end - rom_entries[i].start + 1);<br>
        }<br></blockquote><div><br></div><div>As discussed on IRC, this function can probably use a better name and comments... in another patch :-)</div><div><br></div><div><meta http-equiv="content-type" content="text/html; charset=utf-8">On Fri, Oct 15, 2010 at 6:22 AM, Carl-Daniel Hailfinger <span dir="ltr"><<a href="mailto:c-d.hailfinger.devel.2006@gmx.net">c-d.hailfinger.devel.2006@gmx.net</a>></span> wrote:</div>

<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">@@ -1634,33 +1651,71 @@<br>
                }<br>
        }<br>
<br>
+       newcontents = (uint8_t *) calloc(size, sizeof(char));<br></blockquote><div><br></div><div>+1 on Stefan's comment here... malloc / memset is probably considered better form by some. But that's just a matter of preference.</div>

</div><br>-- <br>David Hendricks (dhendrix)<br>Systems Software Engineer, Google Inc.<br>