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 <[email protected]> >
Acked-by: Carl-Daniel Hailfinger <[email protected]> > 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" > - "[email protected].\n", > - board->vendor_name, board->board_name); > - continue; > - } > - printf("NOTE: Running an untested board enable > procedure.\n" > - "Please report success/failure to > [email protected]\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 > [email protected]\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 _______________________________________________ flashrom mailing list [email protected] http://www.flashrom.org/mailman/listinfo/flashrom
