On Sun, Dec 25, 2011 at 8:42 AM, Stefan Tauner <stefan.tauner@student.tuwien.ac.at> wrote:
> Design issues:
> Specifying a file name from which flashrom should fetch a region and to
> which flashrom should write a region has three main issues:
> - It kills UI consistency. What happens if you have a region called
> "all" which covers the whole flash chip? Specifying a filename for
> region "all" means any supplied filename as --read or --write parameter
> will be completely ignored but still be required.

for --read it is not ignored iirc but only those regions marked to be
fetched via -i are included and the rest is filled with 0x00s. this
creates a chip-sized file which might be useful to some e.g. if they
want to manipulate it by editing some stuff. i will call this (i.e.
files which are parameters to --write/--read/--verify but are
used/filled only partially) "complete" files below.

Yes -- the idea here is to be able to quickly read only the specified region(s), manipulate them, and then write them back.

It's kind of a funky usage model, but it's helped us save a lot of precious time during device manufacturing.

in the case of --read'ing "all" this would produce two identical files
(i guess).
for --write you are right to the point.
i notices this problem but did not even try to mitigate it because i
wanted to discuss the whole approached first so thanks for bringing it
up :)

For read, if the sum of regions included covers the entire chip, then you will end up with two identical files. Included regions will also show up in the -r argument, while the rest is filled in with 0xff's.

For write, the -w argument gets ignored if a region and input file is specified with -i region:filename. The -w argument is used if the :filename portion is omitted, or if no -i argument is used. It's kind of a kludge, and the "write_it" variable needs to be set somehow which is why -w is required in any case.

In both cases, the assumption is that users to specify -i explicitly wish to supersede default behavior of -r and -w. To make this more explicit, perhaps -r and -w should no longer require an argument?

currently i always write a "complete" image for --read operations but
this might not be wanted and could be optional - if the -i option is
used at least once. atm it is not necessary to give an image name
because it defaults to the range's name. but this also forcibly creates
files, when the -i option is used - even if the user does not need it.
maybe we should let the desire for an image be defined by including the
(optional) ':'?

Using the :filename syntax is currently optional, and does not forcibly create files. (Maybe there is something in your patch that is not in chromium os branch that I am missing?)

for --write we need enough data to cover the to be written range(s).
this can come from the "complete" image or the -i ones. and in case we
care for the erase block layout we also need to be sure that the data
is read from the chip where it is needed because it would get deleted
but was not supplied/included via files. the latter is not an issue
(yet) because the complete chip is (still) read in the case of writes.

Yes... I think the old infrastructure was not quite sufficient to take into account the erase block sizes and intelligently read all the data necessary to do the partial write. So we ended up just reading the entire ROM anyway.

another point which needs to be discussed is the precedence of
overlapping regions. without any -i's files given this is not an issue.
we just merge the regions automatically - the data for a single address
is the same anyway.
when different files are given this is no longer true. i think the best
option would be to abort and/or require --force in that case in
addition to a defined precedence (e.g. last -i wins).

I'd prefer to simply abort if -i files are given and the regions overlap. Better to simply avoid doing something damaging in that case, IMHO.

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