Am 06.07.2012 05:42 schrieb Stefan Tauner: > On Fri, 06 Jul 2012 03:28:35 +0200 > Carl-Daniel Hailfinger <[email protected]> wrote: > >> If only one programmer driver is compiled in, make that driver the >> default. If more than one driver is compiled in, require --programmer >> specification at the command line. >> >> 3 results from a default flashrom configuration: >> […] >> This patch represents rough consensus from IRC. I would like to require >> --programmer in all cases to make sure nobody gets bitten by two >> different single-programmer builds (e.g. dediprog and internal), but >> this patch is already a step in the right direction. > Unlikely situation, but i would ack such a patch too.
In that case, I'd like to propose the patch below. > OTOH >90% of the users would just require the internal programmer, > but out of those 90%, 99% probably use pre-compiled versions with > the default config... > >> Please check that the printed error messages make sense. I took the >> liberty of removing "flashrom is free software..." from the output to >> keep this mail readable. > there is an easier way to get rid of that line than deleting it manually... > just saying :) > >> Signed-off-by: Carl-Daniel Hailfinger <[email protected]> >> >> Index: flashrom-programmer_no_default/cli_classic.c >> =================================================================== >> --- flashrom-programmer_no_default/cli_classic.c (Revision 1547) >> +++ flashrom-programmer_no_default/cli_classic.c (Arbeitskopie) >> […] >> if (prog == PROGRAMMER_INVALID) >> - prog = default_programmer; >> + if (default_programmer == PROGRAMMER_INVALID) { >> + /* More than one programmer compiled in, there is no >> default choice. */ >> + msg_perr("Please select a programmer with --programmer >> . Valid choices are:\n"); > > ^ while i see your point (pun intended), the space is still wrong imho. > What about "Please select a programmer with the --programmer parameter. Valid > choices are:\n"? > if the 80 column limit would be a problem (it is not afaics) then it could > become... > "Please select a programmer with the --programmer parameter.\nValid choices > are: " > that may look nicer anyway. Fixed. >> if (prog == PROGRAMMER_INVALID) >> - prog = default_programmer; >> + if (default_programmer == PROGRAMMER_INVALID) { >> + /* More than one programmer compiled in, there is no >> default choice. */ >> + msg_perr("Please select a programmer with --programmer >> . Valid choices are:\n"); >> + list_programmers_linebreak(0, 80, 0); >> + ret = 1; >> + goto out; >> + } else { >> + prog = default_programmer; >> + } > please add {} around the inner if, else gcc complains. Fixed. > Acked-by: Stefan Tauner <[email protected]> > iff 2 out of idwer, uwe, twice11 agree with it. New version. Always require the --programmer parameter on the command line if any flash chip access (probe/read/write/erase/...) is requested. Signed-off-by: Carl-Daniel Hailfinger <[email protected]> Index: flashrom-programmer_no_default/cli_classic.c =================================================================== --- flashrom-programmer_no_default/cli_classic.c (Revision 1547) +++ flashrom-programmer_no_default/cli_classic.c (Arbeitskopie) @@ -31,74 +31,6 @@ #include "flashchips.h" #include "programmer.h" -#if CONFIG_INTERNAL == 1 -static enum programmer default_programmer = PROGRAMMER_INTERNAL; -#elif CONFIG_DUMMY == 1 -static enum programmer default_programmer = PROGRAMMER_DUMMY; -#else -/* If neither internal nor dummy are selected, we must pick a sensible default. - * Since there is no reason to prefer a particular external programmer, we fail - * if more than one of them is selected. If only one is selected, it is clear - * that the user wants that one to become the default. - */ -#if CONFIG_NIC3COM+CONFIG_NICREALTEK+CONFIG_NICNATSEMI+CONFIG_GFXNVIDIA+CONFIG_DRKAISER+CONFIG_SATASII+CONFIG_ATAHPT+CONFIG_FT2232_SPI+CONFIG_SERPROG+CONFIG_BUSPIRATE_SPI+CONFIG_DEDIPROG+CONFIG_RAYER_SPI+CONFIG_NICINTEL+CONFIG_NICINTEL_SPI+CONFIG_OGP_SPI+CONFIG_SATAMV > 1 -#error Please enable either CONFIG_DUMMY or CONFIG_INTERNAL or disable support for all programmers except one. -#endif -static enum programmer default_programmer = -#if CONFIG_NIC3COM == 1 - PROGRAMMER_NIC3COM -#endif -#if CONFIG_NICREALTEK == 1 - PROGRAMMER_NICREALTEK -#endif -#if CONFIG_NICNATSEMI == 1 - PROGRAMMER_NICNATSEMI -#endif -#if CONFIG_GFXNVIDIA == 1 - PROGRAMMER_GFXNVIDIA -#endif -#if CONFIG_DRKAISER == 1 - PROGRAMMER_DRKAISER -#endif -#if CONFIG_SATASII == 1 - PROGRAMMER_SATASII -#endif -#if CONFIG_ATAHPT == 1 - PROGRAMMER_ATAHPT -#endif -#if CONFIG_FT2232_SPI == 1 - PROGRAMMER_FT2232_SPI -#endif -#if CONFIG_SERPROG == 1 - PROGRAMMER_SERPROG -#endif -#if CONFIG_BUSPIRATE_SPI == 1 - PROGRAMMER_BUSPIRATE_SPI -#endif -#if CONFIG_DEDIPROG == 1 - PROGRAMMER_DEDIPROG -#endif -#if CONFIG_RAYER_SPI == 1 - PROGRAMMER_RAYER_SPI -#endif -#if CONFIG_NICINTEL == 1 - PROGRAMMER_NICINTEL -#endif -#if CONFIG_NICINTEL_SPI == 1 - PROGRAMMER_NICINTEL_SPI -#endif -#if CONFIG_OGP_SPI == 1 - PROGRAMMER_OGP_SPI -#endif -#if CONFIG_SATAMV == 1 - PROGRAMMER_SATAMV -#endif -#if CONFIG_LINUX_SPI == 1 - PROGRAMMER_LINUX_SPI -#endif -; -#endif - static void cli_classic_usage(const char *name) { printf("Usage: flashrom [-n] [-V] [-f] [-h|-R|-L|" @@ -107,11 +39,11 @@ #endif "-E|-r <file>|-w <file>|-v <file>]\n" " [-c <chipname>] [-l <file>] [-o <file>]\n" - " [-i <image>] [-p <programmername>[:<parameters>]]\n\n"); + " [-i <image>] -p <programmername>[:<parameters>]\n\n"); printf("Please note that the command line interface for flashrom has " "changed between\n" - "0.9.1 and 0.9.2 and will change again before flashrom 1.0.\n" + "0.9.5 and 0.9.6 and will change again before flashrom 1.0.\n" "Do not use flashrom in scripts or other automated tools " "without checking\n" "that your flashrom version won't interpret options in a " @@ -360,8 +292,9 @@ } } if (prog == PROGRAMMER_INVALID) { - fprintf(stderr, "Error: Unknown programmer " - "%s.\n", optarg); + fprintf(stderr, "Error: Unknown programmer \"%s\". Valid choices are:\n", + optarg); + list_programmers_linebreak(0, 80, 0); cli_classic_abort_usage(); } break; @@ -468,8 +401,13 @@ flash = NULL; } - if (prog == PROGRAMMER_INVALID) - prog = default_programmer; + if (prog == PROGRAMMER_INVALID) { + msg_perr("Please select a programmer with the --programmer parameter.\n" + "Valid choices are:\n"); + list_programmers_linebreak(0, 80, 0); + ret = 1; + goto out; + } /* FIXME: Delay calibration should happen in programmer code. */ myusec_calibrate_delay(); Index: flashrom-programmer_no_default/flashrom.8 =================================================================== --- flashrom-programmer_no_default/flashrom.8 (Revision 1547) +++ flashrom-programmer_no_default/flashrom.8 (Arbeitskopie) @@ -7,7 +7,7 @@ \fB\-v\fR <file>] [\fB\-c\fR <chipname>] \ [\fB\-l\fR <file>] - [\fB\-i\fR <image>] [\fB\-p\fR <programmername>[:<parameters>]] + [\fB\-i\fR <image>] \fB\-p\fR <programmername>[:<parameters>] .SH DESCRIPTION .B flashrom is a utility for detecting, reading, writing, verifying and erasing flash @@ -63,7 +63,7 @@ feel that the time for verification takes too long. .sp Typical usage is: -.B "flashrom \-n \-w <file>" +.B "flashrom \-p prog \-n \-w <file>" .sp This option is only useful in combination with .BR \-\-write . @@ -117,13 +117,12 @@ All addresses are offsets within the file, not absolute addresses! If you only want to update the normal image in a ROM you can say: .sp -.B " flashrom \-\-layout rom.layout \-\-image normal \-w agami_aruma.rom" +.B " flashrom \-p prog \-\-layout rom.layout \-\-image normal \-w agami_aruma.rom" .sp To update normal and fallback but leave the VGA BIOS alone, say: .sp -.B " flashrom \-l rom.layout \-i normal \" -.br -.B " \-i fallback \-w agami_aruma.rom" +.B " flashrom \-p prog \-l rom.layout \-i normal \ +\-i fallback \-w agami_aruma.rom" .sp Currently overlapping sections are not supported. .TP Index: flashrom-programmer_no_default/flashrom.c =================================================================== --- flashrom-programmer_no_default/flashrom.c (Revision 1547) +++ flashrom-programmer_no_default/flashrom.c (Arbeitskopie) @@ -59,6 +59,10 @@ /* Is writing allowed with this programmer? */ int programmer_may_write; +#if CONFIG_INTERNAL+CONFIG_DUMMY+CONFIG_NIC3COM+CONFIG_NICREALTEK+CONFIG_NICNATSEMI+CONFIG_GFXNVIDIA+CONFIG_DRKAISER+CONFIG_SATASII+CONFIG_ATAHPT+CONFIG_FT2232_SPI+CONFIG_SERPROG+CONFIG_BUSPIRATE_SPI+CONFIG_DEDIPROG+CONFIG_RAYER_SPI+CONFIG_PONY_SPI+CONFIG_NICINTEL+CONFIG_NICINTEL_SPI+CONFIG_OGP_SPI+CONFIG_SATAMV+CONFIG_LINUX_SPI < 1 +#error You have to enable at least one programmer! +#endif + const struct programmer_entry programmer_table[] = { #if CONFIG_INTERNAL == 1 { -- http://www.hailfinger.org/ _______________________________________________ flashrom mailing list [email protected] http://www.flashrom.org/mailman/listinfo/flashrom
