<div dir="ltr">On Sun, Sep 15, 2013 at 6:36 PM, Carl-Daniel Hailfinger <span dir="ltr"><<a href="mailto:c-d.hailfinger.devel.2006@gmx.net" target="_blank">c-d.hailfinger.devel.2006@gmx.net</a>></span> wrote:<br><div class="gmail_extra">

<div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div class="im">>>>>> +This is useful for example if you want to write only a part of the flash chip and leave everything else alone<br>


>>>>> +without preparing an image of the complete chip manually:<br>
>>>> Uhh... sounds denglish. Suggested rewording:<br>
>>>> This allows replacing part of a ROM file with contents from another file<br>
>>>> before writing the resulting contents to the flash chip, eliminating the<br>
>>>> separate step of manually assembling a combined ROM image.<br>
>>> You seem to imply that the -i parameters do only overwrite parts of the<br>
>>> file given by -w. While this is true for the current implementation,<br>
>>> David and I discussed about making the file parameter after -w<br>
>>> optional.<br>
>> Ouch! I think that is a really really bad idea. I can see reasons to<br>
>> avoid full image files, but it feels wrong to have undefined regions in<br>
>> the image. Especially if writing fails and recovery is needed.<br>
> There is no such thing as an undefined region (yet at least). For<br>
> address ranges for which no content is supplied by the user in the form<br>
> of an <image file> or a <region file>, the old content of the flash<br>
> chip is what should be there after any flashrom invocation. Cases of<br>
> "-w -i bla" where neither an <image file> nor a <region file> are given<br>
> need to be handled as errors of course.<br>
<br>
</div>That would make an implicit read from the flash chip before any write a<br>
hard requirement. I hear people complaining from time to time about the<br>
initial chip read in flashrom, and this won't make it better.<br>
<br>
Besides that, writes where part of the image is read on-the-fly from the<br>
flash chip directly before writing are not reproducible if an error<br>
occurs. Think of the following scenario: Writing fails, and the flash<br>
chip is now partially corrupted due to an erase command which affected a<br>
larger region size than expected. As a result, parts of the implicitly<br>
read region are now erased. Repeating the same write command with a<br>
fixed flashrom version will _not_ yield the correct flash chip contents.<br>
User despairs, smashes the computer and takes up a career in pottery. If<br>
the image had been built completely from on-disk files (i.e. without<br>
implicit chip reads), repeating the same write command would have<br>
resulted in the correct flash chip contents.<br></blockquote><div><br></div><div>The old contents of the flash are already read regardless. I don't see how recovering from failure is an issue here, unless we change the default behavior (I don't recommend doing that).</div>

<div><br></div><div>For chromeos we added a "--fast-verify" option to skip reading and verifying non-included regions when doing particular writes. It's intended for use in controlled circumstances (e.g. development with an external programmer).</div>

<div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div class="im">> But anyway, your sentence still implies that the file given in -w is<br>
> modified and that's what I was concerned about actually. No idea why I<br>
> brought up the file-less -w at all (anymore) sorry.<br>
<br>
</div>I see. Not sure how to express that best. Next try:<br>
This allows replacing the contents of one ROM image region with contents<br>
<div class="im">from another file before writing the resulting contents to the flash<br>
chip, eliminating the separate step of manually assembling a combined<br>
ROM image.<br></div></blockquote><div><br></div><div>I'd recommend stating this in concrete terms that translate directly to command-line usage: The contents of any -i option takes priority over the contents of the -w option.</div>

<div><br></div><div>BTW, I tried to distill some of these rules for syntax parsing @ line 686 here: <a href="https://chromium.googlesource.com/chromiumos/third_party/flashrom/+/master/cli_mfg.c">https://chromium.googlesource.com/chromiumos/third_party/flashrom/+/master/cli_mfg.c</a> .</div>

<div><br></div><div>Feel free to borrow any wording if it helps.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">

<div class="im"><span style="color:rgb(34,34,34)">There's another problem with the patch, though: Allowing region files</span><br></div>
for read (instead of just for write) does violate the principle of least<br>
surprise: Some users might expect regions to be written to separate<br>
files on disk, i.e. having -r -i as reverse operation to -w -i. Right<br>
now that assumption is not true.<br></blockquote><div><br></div><div>Why would a user provide a filename to a -i option along with -r if they did not intend for a file to be written to disk?</div></div><div><br></div>-- <br>

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