[coreboot] flashrom patch: Force read unknown flash chips

[Peter Stuge]

Hi Paul,

On Sun, Jun 15, 2008 at 09:25:08AM +0200, Paul Menzel wrote:
> as always I can just comment on the messages.

You can test too. :) But more eyes on messages is very good! I
appreciate your review.


> I hope someone else will ack this patch, since Carl-Daniel is idle
> for a while.

Everyone who can review and/or test is invited to do so.


This patch can be tested also on systems where flashrom is already
able to successfully detect the flash chip, by specifying -f -r -c
and specifying a different flash chip than what is really there in
the system as parameter to -c.

For example, on my laptop flashrom finds an SST chip:

# ./flashrom 
Calibrating delay loop... OK.
No coreboot table found.
Found chipset "Intel ICH3-M", enabling flash write... OK.
Found chip "SST SST49LF008A" (1024 KB) at physical address
0xfff00000.
===
This flash part has status UNTESTED for operations: ERASE WRITE
Please email a report to flashrom at coreboot.org if any of the above operations
work correctly for you with this flash part. Please include the full output
from the program, including chipset found. Thank you for your help!
===
No operations were specified.


But I can still test the patch by forcing (e.g.) a Winbond chip:

# ./flashrom -f -r -c W39V080A out.bin
Calibrating delay loop... OK.
No coreboot table found.
Found chipset "Intel ICH3-M", enabling flash write... OK.
No EEPROM/flash device found.
Force read (-f -r -c) requested, forcing chip probe success:
Found chip "Winbond W39V080A" (1024 KB) at physical address 0xfff00000.
Force reading flash...done

# xxd out.bin|head
0000000: 55aa 74e8 5528 cb28 2803 0000 0000 0000  U.t.U(.((.......
0000010: 0000 0000 0000 2000 4000 6000 a803 3001  ...... . at .`...0.
0000020: 554e 4449 16c4 0000 0102 6a0e 0008 b094  UNDI......j.....
0000030: e021 5043 4952 0000 0000 0000 0000 0000  .!PCIR..........
0000040: 5043 4952 8680 2912 0000 1800 0002 0000  PCIR..).........
0000050: 7400 0102 0080 0000 0000 0000 0000 ff00  t...............
0000060: 2450 6e50 0102 0000 00c9 0000 0000 9b00  $PnP............
0000070: bc00 0200 00e4 0000 0000 b80d 0000 0000  ................
0000080: 0d0a 436f 7079 7269 6768 7420 2843 2920  ..Copyright (C) 
0000090: 3139 3937 2d32 3030 322c 2049 6e74 656c  1997-2002, Intel


> > +                       printf("If you know which flash chip you have and if this version of flashrom\n");
> > +                       printf("supports a similar flash chip, you can try a force read. Run:\n");
> 
> Here you say ???force read???.

> > +               if (force && read_it && chip_to_probe) {
> > +                       printf("Detected forced read (-f -r -c options) - forcing chip probe success.\n");
> 
> Here you say ???forced read???.

Thanks for pointing out this inconsistency. I've updated the messages
a bit but will hold off sending a new patch a little to see if there
are some more comments.


> > +                       flashes[0] = probe_flash(flashchips, 1);
> > +                       if (!flashes[0]) {
> > +                               printf("flashrom does not support the flash chip '%s'.\n", chip_to_probe);
> > +                               printf("Run flashrom -L to print a list of the supported hardware in this version.\n");
> 
> Native speakers, is the article ???the??? correct here?

This message could indeed use some improvement. I changed the wording
to improve grammar and clarify, and besides, flashrom doesn't print
on paper, it just displays the list. :)


//Peter