Am 06.07.2012 05:42 schrieb Stefan Tauner:
On Fri, 06 Jul 2012 03:28:35 +0200 Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net 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 c-d.hailfinger.devel.2006@gmx.net
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 stefan.tauner@student.tuwien.ac.at 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 c-d.hailfinger.devel.2006@gmx.net
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 {