Hi Michael, On 07.03.2010 17:24, Michael Karcher wrote:

This also checks the testedness of boards in all cases, not just for PCI/DMI detection.

Signed-off-by: Michael Karcher <flashrom@mkarcher.dialup.fu-berlin.de>

Acked-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006@gmx.net>

Hello Carl-Daniel,

This should address all the issues you raised in your review.

Thanks! One minor comment, otherwise it looks OK. No need to repost, just commit the updated version.

Regards, Michael Karcher

diff --git a/board_enable.c b/board_enable.c index d28490c..7381940 100644 --- a/board_enable.c +++ b/board_enable.c @@ -1411,30 +1411,6 @@ static struct board_pciid_enable *board_match_pci_card_ids(void) } }

- if (board->status == NT) { - if (!force_boardenable) - { - printf("WARNING: Your mainboard is %s %s, but the mainboard-specific\n" - "code has not been marked as working. To help flashrom development, please\n" - "test flashrom on your board. As the code to support your board is untested,\n" - "we strongly recommend that as an additional safety measure you make\n" - "store backup of your current ROM contents (obtained by flashrom -r) on\n" - "a medium that can be accessed from a different computer (like an USB\n" - "drive or a network share of another system) before you try to erase or\n" - "write.\n" - "The untested code does not run unless you specify the\n" - " \"-p internal:boardenable=force\" command line option. Depending on your\n" - "hardware environment, erasing, writing or even probing can fail without\n" - "running the board specific code. Running the board-specific code might\n" - "cause your computer to behave erratically if it is wrong.\n" - "Please report the results of running the board enable code to\n" - "flashrom@flashrom.org.\n", - board->vendor_name, board->board_name); - continue; - } - printf("NOTE: Running an untested board enable procedure.\n" - "Please report success/failure to flashrom@flashrom.org\n"); - } return board; }

@@ -1452,6 +1428,23 @@ int board_flash_enable(const char *vendor, const char *part) if (!board) board = board_match_pci_card_ids();

+ if (board->status == NT) { + if (!force_boardenable) + { + printf("WARNING: Your mainboard is %s %s, but the mainboard-specific\n" + "code has not been tested, and thus will not not be executed by default.\n" + "Depending on your hardware environment, erasing, writing or even probing\n" + "can fail without running the board specific code.\n\n" + "Please see the man page (section PROGRAMMER SPECIFIC INFO, subsection\n" + "\"internal programmer\" for details\n", + board->vendor_name, board->board_name); + board = NULL; + } + else + printf("NOTE: Running an untested board enable procedure.\n" + "Please report success/failure to flashrom@flashrom.org\n"); + } + if (board) { if (board->max_rom_decode_parallel) max_rom_decode.parallel = diff --git a/flashrom.8 b/flashrom.8 index 696aba2..46dca21 100644 --- a/flashrom.8 +++ b/flashrom.8 @@ -175,6 +175,46 @@ colon. While some programmers take arguments at fixed positions, other programmers use a key/value interface in which the key and value is separated by an equal sign and different pairs are separated by a comma or a colon. .TP +.BR "internal " programmer +Some mainboards require to run mainboard specific code to enable erase and

... to enable chip erase and ...

+write support (on old systems with parallel flash even probe support). The

... (and probe support on old systems with parallel flash).

+type of the mainboard (if it requires specific code) is usually autodetected

The mainboard brand and model (....

+using one of the following mechanisms: If your system is running coreboot, +the mainboard type is determined from the coreboot table, otherwise, the +mainboard is detected by examining the onboard PCI devices and possibly DMI +info. If PCI and DMI do not contain information to uniquely identify the +mainboard (which is the exception), it might be needed to specify the

... might be necessary to specify ...

+mainboard using the \-m switch (see above). +.sp +Some of these board-specific flash enabling functions (called board enables) +in flashrom have not yet been tested. If your mainboard is detected needing an untested

untested untested? [Kill one of them]

+untested board-enable function, a warning message is printed and the board

... untested board enable ... [remove -]

+enable is not executed, because a wrong board enable function might cause the +system to behave erratically, as board enable functions touch the low-level +internals of a mainboard. This might cause detection or erasing

Not executing a board enable function (if one is needed) might cause ...

+failure. If your board protects only part of the flash (commonly the top +end, called boot block), flashrom might encounter the error only after

...encounter an error ...

+erasing the other part, so running without the board-enable function might

...erasing the unprotected part...

+be dangerous for erase and write (which includes erase). +.sp +The suggested procedure for a mainboard with untested board specific code is +to first try to probe the ROM (just invoke flashrom and check that it detects +your flash chip type) without running the board enable code (i.e. without any +parameters). If it finds your chip, fine, otherwise, retry probing your chip +with the board-enable code running, using +.sp +.B "flashrom -p internal:boardenable=force" +.sp +If your chip is still not detected, the board enable code seems to be broken +or the chip unsupported. Otherwise, make a backup of your current ROM

... or the flash chip is unsupported.

+contents (using \-r) and store it to a medium outside of your computer, like +an USB drive or a network share. If you needed to run the board enable code +already for probing, use it for reading too. Now you can try to write the +new image. You should enable the board enable code in any case now, as it +has been written because it is known that writing/erasing without the board +enable is going to fail. In any case (success or failure), please report +to the flashrom mailing list, see below. +.sp .BR "dummy " programmer An optional parameter specifies the bus types it should support. For that you have to use the

Looks good otherwise. I'm not a native speaker, so I might have overlooked something. Still, your text is way better than what we have now, so go ahead. Regards, Carl-Daniel -- "I do consider assignment statements and pointer variables to be among computer science's most valuable treasures." -- Donald E. Knuth