[flashrom] [PATCH 3/5] layout: Add -i <image>[:<file>] support

[Stefan Tauner]

On Sun, 25 Dec 2011 01:59:35 +0100
Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006 at gmx.net> wrote:

> Author attribution seems to have been partially lost and/or another
> patch ("make read before write configurable") was mixed in.

right. i mangled and merged three of the patches together when rebasing
them for "Dispensored"'s X8DTU three days ago.
i will repost the series now as layout 2.1 in the hope that i have
separated them correctly (git helped a lot :) but please be aware of
possible artifacts.

some of the comments do not regard the --image patch but later ones. i
will work on those issues regardless of that - no need to re-comment
them.
 
> 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.
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 :)

without thinking too much about the possible implementation i would say
that we want to *require* the bare minimum of files and images for a given
operation to complete it successfully.

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) ':'?

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.

> - If one image is specified with a file name, will the file name for
> --read/--write be ignored? If not, this means we still can't deal with
> read-locked regions. If yes, what's the point in requiring a file name
> for --read/--write?
if one disables the complete reads (via the variable of the
infrastructure patch) it will not be ignored completely, but will not
get in the way either (i.e. provide "background" data for write
operations which is then not used for -i options where an image is also
included or for read operations as "complete" image).

> - IMHO specifying region file names should happen in a layout file, but
> that's arguably debatable.
no harm in supporting both :)
i did not do so due to the existing implementation in chromiumos and
the needed layout file changes.
the command line is easier to use in case you want to create
differently named files of the same region - for example when working
with multiple chips.
precedence of data to be written (to the chip) should imho be:
-i's file (name) > layout's file (name) > "complete" file

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).
-- 
Kind regards/Mit freundlichen Grüßen, Stefan Tauner