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:
compiler@p35:/sources/tmptrees/ready/flashrom-programmer_no_default> ./flashrom flashrom v0.9.5.2-r1547 on Linux 2.6.34.10-0.6-default (i686) Please select a programmer with --programmer . Valid choices are: internal, dummy, nic3com, nicrealtek, gfxnvidia, drkaiser, satasii, ft2232_spi, serprog, buspirate_spi, rayer_spi, pony_spi, nicintel, nicintel_spi, ogp_spi, satamv, linux_spi
compiler@p35:/sources/tmptrees/ready/flashrom-programmer_no_default> ./flashrom -p foo flashrom v0.9.5.2-r1547 on Linux 2.6.34.10-0.6-default (i686) Error: Unknown programmer foo. Valid choices are: internal, dummy, nic3com, nicrealtek, gfxnvidia, drkaiser, satasii, ft2232_spi, serprog, buspirate_spi, rayer_spi, pony_spi, nicintel, nicintel_spi, ogp_spi, satamv, linux_spi Please run "flashrom --help" for usage info.
compiler@p35:/sources/tmptrees/ready/flashrom-programmer_no_default> ./flashrom -p internal -p internal flashrom v0.9.5.2-r1547 on Linux 2.6.34.10-0.6-default (i686) Error: --programmer specified more than once. You can separate multiple parameters for a programmer with ",". Please see the man page for details. Please run "flashrom --help" for usage info.
1 result from a flashrom configuration with only dummy compiled in:
compiler@p35:/sources/tmptrees/ready/flashrom-programmer_no_default> ./flashrom flashrom v0.9.5.2-r1547 on Linux 2.6.34.10-0.6-default (i686) Calibrating delay loop... OK. No EEPROM/flash device found. Note: flashrom can never write if the flash chip isn't found automatically.
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.
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.
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,20 +31,19 @@ #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 only one programmer is compiled in, it is the default. + * In all other cases there is no default and the user has to specify the programmer with -p . */ +static enum programmer default_programmer = #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. + PROGRAMMER_INVALID +#else +#if CONFIG_INTERNAL == 1 + PROGRAMMER_INTERNAL #endif -static enum programmer default_programmer = +#if CONFIG_DUMMY == 1 + PROGRAMMER_DUMMY +#endif #if CONFIG_NIC3COM == 1 PROGRAMMER_NIC3COM #endif @@ -96,8 +95,8 @@ #if CONFIG_LINUX_SPI == 1 PROGRAMMER_LINUX_SPI #endif +#endif ; -#endif
static void cli_classic_usage(const char *name) { @@ -107,11 +106,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 +359,8 @@ } } 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; @@ -469,7 +468,15 @@ }
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; + }
/* FIXME: Delay calibration should happen in programmer code. */ myusec_calibrate_delay();
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. 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.
Acked-by: Stefan Tauner stefan.tauner@student.tuwien.ac.at iff 2 out of idwer, uwe, twice11 agree with it.
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 {
On Mon, 16 Jul 2012 23:28:22 +0200 Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net wrote:
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");
dont save changed line counts in the wrong places. please move the -p part right after "flashrom" here and in the other bits (e.g. manpage). i would even argue that all mandatory parameters should be mentioned first (i.e. -E | ... should be moved up too).
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"
either name all (known) CLI changes or just say something like "has been changed before…" imho, but it is ok if you want to leave the hunk as it is.
"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>]
see above
.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"
maybe rename the image name as proposed by idwer...?
.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
why did you choose this location for the check? wouldnt it make more sense to have it in the makefile?
const struct programmer_entry programmer_table[] = { #if CONFIG_INTERNAL == 1 {
Acked-by: Stefan Tauner stefan.tauner@student.tuwien.ac.at
Am 17.07.2012 00:42 schrieb Stefan Tauner:
On Mon, 16 Jul 2012 23:28:22 +0200 Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net wrote:
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) @@ -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");dont save changed line counts in the wrong places. please move the -p part right after "flashrom" here and in the other bits (e.g. manpage). i would even argue that all mandatory parameters should be mentioned first (i.e. -E | ... should be moved up too).
Hm yes. But how do we tell people that they need -p only for erase/read/write operations? Removing the square brackets around -p is not correct because it implies that -p is even required for --help.
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"either name all (known) CLI changes or just say something like "has been changed before…" imho, but it is ok if you want to leave the hunk as it is.
Or maybe "[...] may change before flashrom 1.0"
"Do not use flashrom in scripts or other automated tools " "without checking\n" "that your flashrom version won't interpret options in a "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>]see above
Similar question here.
.SH DESCRIPTION .B flashrom is a utility for detecting, reading, writing, verifying and erasing flash @@ -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"
maybe rename the image name as proposed by idwer...?
Hm. I don't really get the point of using an abstract name in a concrete example. Could you please enlighten me?
.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
why did you choose this location for the check? wouldnt it make more sense to have it in the makefile?
Good point. I wanted to implement it in the Makefile, but that would have taken ~50 lines of code or even more due to the braindead language of GNUmake files.
const struct programmer_entry programmer_table[] = { #if CONFIG_INTERNAL == 1 {
Acked-by: Stefan Tauner stefan.tauner@student.tuwien.ac.at
Thanks! I'll wait for your comments to the questions above before I resubmit and commit.
Regards, Carl-Daniel
New version.
Always require the --programmer parameter on the command line if any flash chip access (probe/read/write/erase/...) is requested.
Fix a few man page oddities as well.
Signed-off-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net Acked-by: Stefan Tauner stefan.tauner@student.tuwien.ac.at
Index: flashrom-programmer_no_default/cli_classic.c =================================================================== --- flashrom-programmer_no_default/cli_classic.c (Revision 1551) +++ flashrom-programmer_no_default/cli_classic.c (Arbeitskopie) @@ -31,87 +31,20 @@ #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|" + printf("Usage: flashrom [-h|-R|-L|" #if CONFIG_PRINT_WIKI == 1 - "-z|" + "-z|" #endif - "-E|-r <file>|-w <file>|-v <file>]\n" - " [-c <chipname>] [-l <file>] [-o <file>]\n" - " [-i <image>] [-p <programmername>[:<parameters>]]\n\n"); + "-p <programmername>[:<parameters>]\n" + " [-E|-r <file>|-w <file>|-v <file>] [-c <chipname>]\n" + " [-l <file> [-i <image>]] [-n] [-f]]\n" + " [-V[V[V]]] [-o <logfile>]\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 +293,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 +402,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 1551) +++ flashrom-programmer_no_default/flashrom.8 (Arbeitskopie) @@ -2,13 +2,12 @@ .SH NAME flashrom - detect, read, write, verify and erase flash chips .SH SYNOPSIS -.B flashrom \fR[\fB-n\fR] [\fB-V\fR] [\fB-f\fR] [\fB-h\fR|\fB-R\fR|\ -\fB-L\fR|\fB-z\fR|\fB-E\fR|\fB-r\fR <file>|\fB-w\fR <file>|\ -\fB-v\fR <file>] - [\fB-c\fR <chipname>] \ -[\fB-l\fR <file>] - [\fB-i\fR <image>] [\fB-p\fR <programmername>[:<parameters>]] - [\fB-o\fR <logfile>] +.B flashrom \fR[\fB-h\fR|\fB-R\fR|\fB-L\fR|\fB-z\fR|\ +\fB-p\fR <programmername>[:<parameters>] + [\fB-E\fR|\fB-r\fR <file>|\fB-w\fR <file>|\fB-v\fR <file>] \ +[\fB-c\fR <chipname>] + [\fB-l\fR <file> [\fB-i\fR <image>]] [\fB-n\fR] [\fB-f\fR]] + [\fB-V\fR[\fBV\fR[\fBV\fR]]] [\fB-o\fR <logfile>] .SH DESCRIPTION .B flashrom is a utility for detecting, reading, writing, verifying and erasing flash @@ -64,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 . @@ -106,31 +105,39 @@ .BR <file> . .sp flashrom supports ROM layouts. This allows you to flash certain parts of -the flash chip only. A ROM layout file looks like follows: +the flash chip only. A ROM layout file contains multiple lines with the +following syntax: .sp +.B " startaddr:endaddr imagename" +.sp +.BR "startaddr " "and " "endaddr " +are hexadecimal addresses within the ROM file and do not refer to any +physical address. Please note that using a 0x prefix for those hexadecimal +numbers is not necessary, but you can't specify decimal/octal numbers. +.BR "imagename " "is an arbitrary name for the region/image from" +.BR " startaddr " "to " "endaddr " "(both addresses included)." +.sp +Example: +.sp 00000000:00008fff gfxrom 00009000:0003ffff normal 00040000:0007ffff fallback .sp - i.e.: - startaddr:endaddr name +If you only want to update the image named +.BR "normal " "in a ROM based on the layout above, run" .sp -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: +.B " flashrom -p prog --layout rom.layout --image normal -w some.rom" .sp -.B " flashrom --layout rom.layout --image normal -w agami_aruma.rom" +To update only the images named +.BR "normal " "and " "fallback" ", run:" .sp -To update normal and fallback but leave the VGA BIOS alone, say: +.B " flashrom -p prog -l rom.layout -i normal -i fallback -w some.rom" .sp -.B " flashrom -l rom.layout -i normal " -.br -.B " -i fallback -w agami_aruma.rom" -.sp -Currently overlapping sections are not supported. +Overlapping sections are not supported. .TP -.B "-i, --image <name>" -Only flash image -.B <name> +.B "-i, --image <imagename>" +Only flash region/image +.B <imagename> from flash layout. .TP .B "-L, --list-supported" @@ -156,7 +163,8 @@ Please note that MediaWiki output is not compiled in by default. .TP .B "-p, --programmer <name>[:parameter[,parameter[,parameter]]]" -Specify the programmer device. Currently supported are: +Specify the programmer device. This is mandatory for all operations +involving any chip access (probe/read/write/...). Currently supported are: .sp .BR "* internal" " (default, for in-system flashing in the mainboard)" .sp @@ -330,7 +338,8 @@ .sp .B " flashrom -p internal:ich_spi_mode=value" .sp -syntax where value can be +syntax where +.BR "value " "can be" .BR auto ", " swseq " or " hwseq . By default .RB "(or when setting " ich_spi_mode=auto ) @@ -360,7 +369,9 @@ .sp .B " flashrom -p internal:fwh_idsel=value" .sp -syntax where value is the 48-bit hexadecimal raw value to be written in the +syntax where +.B value +is the 48-bit hexadecimal raw value to be written in the IDSEL registers of the Intel southbridge. The upper 32 bits use one hex digit each per 512 kB range between 0xffc00000 and 0xffffffff, and the lower 16 bits use one hex digit each per 1024 kB range between 0xff400000 and 0xff7fffff. @@ -486,7 +497,9 @@ .sp .B " flashrom -p dummy:spi_blacklist=commandlist" .sp -syntax where commandlist is a list of two-digit hexadecimal representations of +syntax where +.B commandlist +is a list of two-digit hexadecimal representations of SPI commands. If commandlist is e.g. 0302, flashrom will behave as if the SPI controller refuses to run command 0x03 (READ) and command 0x02 (WRITE). commandlist may be up to 512 characters (256 commands) long. @@ -500,7 +513,9 @@ .sp .B " flashrom -p dummy:spi_ignorelist=commandlist" .sp -syntax where commandlist is a list of two-digit hexadecimal representations of +syntax where +.B commandlist +is a list of two-digit hexadecimal representations of SPI commands. If commandlist is e.g. 0302, the emulated flash chip will ignore command 0x03 (READ) and command 0x02 (WRITE). commandlist may be up to 512 characters (256 commands) long. @@ -513,7 +528,9 @@ .sp .B " flashrom -p dummy:spi_status=content" .sp -syntax where content is an 8-bit hexadecimal value. +syntax where +.B content +is an 8-bit hexadecimal value. .SS .BR "nic3com" , " nicrealtek" , " nicsmc1211" , " nicnatsemi" , " nicintel\ " , " nicintel_spi" , " gfxnvidia" , " ogp_spi" , " drkaiser" , " satasii\ Index: flashrom-programmer_no_default/flashrom.c =================================================================== --- flashrom-programmer_no_default/flashrom.c (Revision 1551) +++ 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 {
Am 21.07.2012 23:30 schrieb Carl-Daniel Hailfinger:
Always require the --programmer parameter on the command line if any flash chip access (probe/read/write/erase/...) is requested.
Fix a few man page oddities as well.
Signed-off-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net Acked-by: Stefan Tauner stefan.tauner@student.tuwien.ac.at
Thanks for the multiple reviews!
Committed in r1552.
Regards, Carl-Daniel
On Fri, 06 Jul 2012 03:28:35 +0200 Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net wrote:
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.
-
Carl-Daniel Hailfinger -
Stefan Tauner