ignore fmap if we're erasing the ROM with -E

ignore fmap if we're erasing the ROM with -E


R=yjlou@chromium.org,reinauer@chromium.org
BUG=
TEST=

[Stefan Reinauer, 2011-03-25 22:35:32]

http://codereview.chromium.org/6749019/diff/1/flashrom.c
File flashrom.c (right):

http://codereview.chromium.org/6749019/diff/1/flashrom.c#newcode1901
flashrom.c:1901: if (!erase_it) {
should this be if (read_it || write_it) 
Does it make sense to parse fmaps for verify_it?

[dhendrix, 2011-03-25 22:50:44]

http://codereview.chromium.org/6749019/diff/1/flashrom.c
File flashrom.c (right):

http://codereview.chromium.org/6749019/diff/1/flashrom.c#newcode1901
flashrom.c:1901: if (!erase_it) {
On 2011/03/25 22:35:32, Stefan Reinauer wrote:
> should this be if (read_it || write_it) 
> Does it make sense to parse fmaps for verify_it?

Yes, it makes sense to verify using fmap data. The reason for this is because
users might want to verify only regions marked as "static".

This is most useful if your ROM image has a region dedicated to event logging or
other data which is expected to change at run-time. In this case, you'd mark the
regions you wish to include in verification as "static" so that benign/expected
changes at run-time do not cause verification to fail.

Should I change this to:
if (read_it || write_it || verify_it)

?

[Stefan Reinauer, 2011-03-25 23:55:07]

I think we should go with your original patch for now.

LGTM

[Louis, 2011-03-29 06:35:53]

Sorry for late reply. Why do we give erase an exception here? Is there any
reason to do this?

I would prefer if we can have consistent behavior between all:

  flashrom -i XXX -r
  flashrom -i XXX -w
  flashrom -i XXX -E

This makes the semantic equal: all actions apply to *partition XXX* only. Isn't
it?



On 2011/03/25 23:55:07, Stefan Reinauer wrote:
> I think we should go with your original patch for now.
> 
> LGTM

[Louis, 2011-03-29 08:11:18]

Okay, get the motive from CL 6731011. I am thinking the better solution is
adding options like:
  --skip_fmap: totally ignore fmap structures on flash
  --skip_crossystem_fmap: Don't use crossystem fmap address.
  --skip_fmap_search: Don't try to seach fmap on flash

I will send out CL later.

On 2011/03/29 06:35:53, Louis wrote:
> Sorry for late reply. Why do we give erase an exception here? Is there any
> reason to do this?
> 
> I would prefer if we can have consistent behavior between all:
> 
>   flashrom -i XXX -r
>   flashrom -i XXX -w
>   flashrom -i XXX -E
> 
> This makes the semantic equal: all actions apply to *partition XXX* only.
Isn't
> it?
> 
> 
> 
> On 2011/03/25 23:55:07, Stefan Reinauer wrote:
> > I think we should go with your original patch for now.
> > 
> > LGTM

[dhendrix, 2011-03-29 18:08:52]

On 2011/03/29 08:11:18, Louis wrote:
> Okay, get the motive from CL 6731011. I am thinking the better solution is
> adding options like:
>   --skip_fmap: totally ignore fmap structures on flash
>   --skip_crossystem_fmap: Don't use crossystem fmap address.
>   --skip_fmap_search: Don't try to seach fmap on flash

Let's try to do only --skip-fmap. It's better to minimize special flags IMHO.

> 
> I will send out CL later.
> 
> On 2011/03/29 06:35:53, Louis wrote:
> > Sorry for late reply. Why do we give erase an exception here? Is there any
> > reason to do this?
> > 
> > I would prefer if we can have consistent behavior between all:
> > 
> >   flashrom -i XXX -r
> >   flashrom -i XXX -w
> >   flashrom -i XXX -E
> > 
> > This makes the semantic equal: all actions apply to *partition XXX* only.
> Isn't
> > it?
> > 
> > 
> > 
> > On 2011/03/25 23:55:07, Stefan Reinauer wrote:
> > > I think we should go with your original patch for now.
> > > 
> > > LGTM

[Stefan Reinauer, 2011-03-29 19:23:50]

Yes, I agree. Let's keep the number of options to a minimum.

[Louis, 2011-04-01 03:10:23]

--ignore-fmap is in CL http://codereview.chromium.org/6791005. Please kindly
take a look. Thanks.

On 2011/03/29 19:23:50, Stefan Reinauer wrote:
> Yes, I agree. Let's keep the number of options to a minimum.