flashrom patch: Force read unknown flash chips

[Peter Stuge]

Patch attached.

//Peter

[Paul Menzel]

Dear Peter,

as always I can just comment on the messages. I hope someone else will
ack this patch, since Carl-Daniel is idle for a while.

Am Sonntag, den 15.06.2008, 04:53 +0200 schrieb Peter Stuge:
> @@ -427,6 +428,44 @@
>                 exit(1);
>         } else if (!flashes[0]) {
>                 printf("No EEPROM/flash device found.\n");
>           if (!force || !chip_to_probe) {
>                   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”.
>                   printf("flashrom -f -r -c similar_supported_flash_chip filename\n");
>                   printf("\n");
>                   printf("Note: flashrom can never write when the flash chip isn't found automatically.\n");
>           }
>           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”.
>                   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?

Thanks,

Paul

[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 [e-mail] 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  ...... .@.`...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

[Ward Vandewege]

On Sun, Jun 15, 2008 at 04:53:14AM +0200, Peter Stuge wrote:
> flashrom: Force read unknown flash chips
>
> When flash chip detection fails, it is still useful and possible to read the
> flash chip contents. If no flash chip is found in normal probes and the
> -f -r -c CHIPNAME options are given, a successful probe for the specified
> chip is forced, and flashrom reads the flash chip using either the read
> function for the specified chip, or if there is none, a simple memcpy().
>
> The patch also moves the global variable int force in flashrom.c into main()
> and passes it as a parameter to layout.c:show_id(), which is the only other
> function that uses the variable. This is needed to avoid confusion with the
> new parameter int force which is added to flashrom.c:probe_flash() to force
> a probe success.
>
> Signed-off-by: Peter Stuge

Tested on alix.2c3, works there.

Acked-by: Ward Vandewege

with the changes proposed by Paul in this thread.

Thanks,
Ward.

[Peter Stuge]

On Tue, Jun 17, 2008 at 09:44:37PM -0400, Ward Vandewege wrote:
> > Signed-off-by: Peter Stuge
>
> Tested on alix.2c3, works there.
>
> Acked-by: Ward Vandewege 
> with the changes proposed by Paul in this thread.

Thanks! r3367

//Peter