[flashrom] [PATCH 2/4] layout: Add -i <image>[:<file>] support.

[Stefan Tauner]

On Mon, 16 Sep 2013 00:46:48 +0200
Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006 at gmx.net> wrote:

> Am 15.09.2013 20:14 schrieb Stefan Tauner:
> > On Sun, 15 Sep 2013 04:15:44 +0200
> > Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006 at gmx.net> wrote:
> >
> >> Am 12.09.2013 22:40 schrieb Stefan Tauner:
> >>> -Read ROM layout from
> >>> +Read layout entries from
> >> Do we want to call them "ROM layout", "flash layout" or simply "layout"?
> >> My preferences are 1. "ROM layout" 2. "flash layout".
> > I dont like the word ROM in there at all. It is actually untrue in
> > context of flashrom and the concept behind the whole layout thing (i.e.
> > address ranges with attributes) are not specific to them at all. IMHO
> > ROM layout implies some kind of chip-specific partitioning scheme.
> > Please give some rationale of your preferences.
> 
> Think of "ROM" as generic name for an image file of the contents of some
> flash/ROM chip. Just google for "ROMs" and you'll see what I mean.

Fair enough, but that is even worse: there is no strict definition of
what a ROM is, how it is formatted and to be interpreted - actually
there are hundreds of formats out there. So what impression does a
reader get when reading "ROM layout"? I have no idea but it is
certainly far from being non-ambiguous.

Why do you want to include it rather than just use "layout" alone and
declare what we mean exactly? We need to do the later anyway including a
description of the format of the file. There is no benefit in adding
"ROM" then.

i'd go with
 - 'region' for an address range (and associated attributes)
 - 'layout' (alternatively 'flash layout' or 'flashrom layout') for the
   data structure/concept that defines regions and their attributes
   (only its name for now, arguable other attributes assigned by the
   command line (e.g. if a region should be used))
 - 'layout file' for a file containing a clear text description of a
   layout
 - 'layout entry' for a continuous, delimited part in a layout file
   that describes a region and its attributes (which is a bit fuzzy and
   overlapping with 'region', but I like to distinguish the thing in
   files and the thing in RAM)

Then there is the "image problem":

> >>> -are hexadecimal addresses within the ROM file and do not refer to any
> >>> -physical address. Please note that using a 0x prefix for those hexadecimal
> >>> +are hexadecimal addresses within the ROM file representing the flash ROM contents.
> >> are hexadecimal addresses within the virtual file representing the flash
> >> chip contents
> > So... "virtual file representing the flash chip contents" seems to be
> > the new wording for... that. We should try to come up with a shorter
> > way to refer to that. (VFRTFCC hm... nope. :)
> 
> "ROM image"? "ROM file"? I do prefer "ROM image".
> Suggested explanation to be added somewhere in the man page:
> "For historical reasons, the contents read from or written to a flash
> chip are called ROM image. Addresses within a ROM image start at 0
> regardless of the address where the flash chip is mapped in hardware."

If we can avoid using "ROM" in relation to layouts we should avoid it
here too IMO. The main problem here is to distinguish the VFRTFCC (aka
old_- and new_contents or the concept behind that), from the image files
of whole chips, and from the files used to supply/retrieve region
contents.

 - no idea regarding VFRTFCC, maybe we can get away with paraphrasing it
 - image file for files containing the content of a whole flash chip
 - region file? like above but for address ranges/regions only

Till now I was thinking of image files as images reflecting the
contents of flash... but also if they only reflect a part of the flash,
i.e. if they are really <region files>. Using it for the "big" files
only may provoke confusion(?)

> >>>  .BR <file> .
> >>>  .sp
> >>> -flashrom supports ROM layouts. This allows you to flash certain parts of
> >>> -the flash chip only. A ROM layout file contains multiple lines with the
> >>> -following syntax:
> >>> +A layout entry describes an address region of the flash chip and gives it a name. This allows to access certain
> >>> +parts of the flash chip only. A layout file can contain multiple entries one per line with the following syntax:
> >> I am not a native speaker, but the sentences above look odd. Suggested
> >> rewording :
> >>
> >> flashrom supports reading/writing/erasing flash chips in whole (default)
> >> or in part (when a ROM layout is given).
> > That is actually not true. Giving the layout alone does not make
> > flashrom access only parts. Arguably "supporting" does not necessarily
> > imply that it *does* so, but I would rather not write it like that in
> > the manual.
> 
> What about
> "[...] (when a ROM layout file is given and certain other conditions are
> met)."

Counterproposal:
flashrom supports reading/writing/erasing flash chips in whole
(default) or in part. To access only parts of a chip one has to use
$layout files and respective arguments described below.

> >>> +This is useful for example if you want to write only a part of the flash chip and leave everything else alone
> >>> +without preparing an image of the complete chip manually:
> >> Uhh... sounds denglish. Suggested rewording:
> >> This allows replacing part of a ROM file with contents from another file
> >> before writing the resulting contents to the flash chip, eliminating the
> >> separate step of manually assembling a combined ROM image.
> > You seem to imply that the -i parameters do only overwrite parts of the
> > file given by -w. While this is true for the current implementation,
> > David and I discussed about making the file parameter after -w
> > optional.
> 
> Ouch! I think that is a really really bad idea. I can see reasons to
> avoid full image files, but it feels wrong to have undefined regions in
> the image. Especially if writing fails and recovery is needed.

There is no such thing as an undefined region (yet at least). For
address ranges for which no content is supplied by the user in the form
of an <image file> or a <region file>, the old content of the flash
chip is what should be there after any flashrom invocation. Cases of
"-w -i bla" where neither an <image file> nor a <region file> are given
need to be handled as errors of course.

But anyway, your sentence still implies that the file given in -w is
modified and that's what I was concerned about actually. No idea why I
brought up the file-less -w at all (anymore) sorry.
-- 
Kind regards/Mit freundlichen Grüßen, Stefan Tauner