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 [EMAIL PROTECTED] 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 ...... [EMAIL PROTECTED]
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
--
coreboot mailing list
[email protected]
http://www.coreboot.org/mailman/listinfo/coreboot
