[flashrom] [PATCH] No default driver if more than one driver is available

Carl-Daniel Hailfinger c-d.hailfinger.devel.2006 at gmx.net
Mon Jul 16 23:28:22 CEST 2012


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 at 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 at 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 at 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 at 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
 	{


-- 
http://www.hailfinger.org/





More information about the flashrom mailing list