Changeset 1483 for trunk

Timestamp:

01/04/12 01:48:27 (5 months ago)

Author:

hailfinger

Message:

Replace --mainboard with -p internal:mainboard

--mainboard is a relic from a time before external programmers and makes
the CLI inconsistent.
Use a programmer parameter instead and free up the short option -m.

NOTE:
The --list-supported-wiki output changed to use -p internal:mainboard=
instead of -m
The --list-supported output changed the heading of the mainboard list
from

Vendor Board Status Required option
to
Vendor Board Status Required value for

-p internal:mainboard=

Fix lb_vendor_dev_from_string() not to write to the supplied string.

Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006@…>
Acked-by: Stefan Tauner <stefan.tauner@…>

Location:

trunk

Files:

• board_enable.c (modified) (3 diffs)
• cbtable.c (modified) (1 diff)
• cli_classic.c (modified) (5 diffs)
• flashrom.8 (modified) (3 diffs)
• internal.c (modified) (1 diff)
• layout.c (modified) (2 diffs)
• print.c (modified) (2 diffs)
• print_wiki.c (modified) (1 diff)
• programmer.h (modified) (1 diff)

trunk/board_enable.c

@@ -2071 +2071 @@
  * the ids uniquely matches the coreboot board identification string. When a
  * legacy bios is installed and when autodetection is not possible, these ids
- * can be used to identify the board through the -m command line argument.
+ * can be used to identify the board through the -p internal:mainboard=
+ * programmer parameter.
  *
  * When a board is identified through its coreboot ids (in both cases), the
@@ -2246 +2247 @@
                         msg_pinfo("At least vendors '%s' and '%s' match.\n",
                                   partmatch->lb_vendor, board->lb_vendor);
-                        msg_perr("Please use the full -m vendor:part syntax.\n");
+                        msg_perr("Please use the full -p internal:mainboard="
+                                 "vendor:part syntax.\n");
                         return NULL;
                 }
@@ -2260 +2262 @@
                  * expected to fix flashrom, too.
                  */
-                msg_perr("\nUnknown vendor:board from -m option: %s:%s\n\n",
+                msg_perr("\nUnknown vendor:board from -p internal:mainboard="
+                         " programmer parameter:\n%s:%s\n\n",
                          vendor, part);
         }

trunk/cbtable.c

@@ -34 +34 @@
 int partvendor_from_cbtable = 0;
 
-void lb_vendor_dev_from_string(char *boardstring)
-{
+/* Parse the [<vendor>:]<board> string specified by the user as part of
+ * -p internal:mainboard=[<vendor>:]<board> and set lb_vendor and lb_part
+ * to the extracted values.
+ * Note: strtok modifies the original string, so we work on a copy and allocate
+ * memory for lb_vendor and lb_part with strdup.
+ */
+void lb_vendor_dev_from_string(const char *boardstring)
+{
+        /* strtok may modify the original string. */
+        char *tempstr = strdup(boardstring);
         char *tempstr2 = NULL;
-        strtok(boardstring, ":");
+        strtok(tempstr, ":");
         tempstr2 = strtok(NULL, ":");
         if (tempstr2) {
-                lb_vendor = boardstring;
-                lb_part = tempstr2;
+                lb_vendor = strdup(tempstr);
+                lb_part = strdup(tempstr2);
         } else {
                 lb_vendor = NULL;
-                lb_part = boardstring;
-        }
+                lb_part = strdup(tempstr);
+        }
+        free(tempstr);
 }
 

trunk/cli_classic.c

@@ -107 +107 @@
 #endif
                  "-E|-r <file>|-w <file>|-v <file>]\n"
-               "       [-c <chipname>] [-m [<vendor>:]<part>] [-l <file>]\n"
+               "       [-c <chipname>] [-l <file>]\n"
                "       [-i <image>] [-p <programmername>[:<parameters>]]\n\n");
 
@@ -129 +129 @@
                "   -c | --chip <chipname>            probe only for specified "
                  "flash chip\n"
-#if CONFIG_INTERNAL == 1
-               /* FIXME: --mainboard should be a programmer parameter */
-               "   -m | --mainboard <[vendor:]part>  override mainboard "
-                 "detection\n"
-#endif
                "   -f | --force                      force specific operations "
                  "(see man page)\n"
@@ -191 +186 @@
                 {"noverify",            0, NULL, 'n'},
                 {"chip",                1, NULL, 'c'},
-                {"mainboard",           1, NULL, 'm'},
                 {"verbose",             0, NULL, 'V'},
                 {"force",               0, NULL, 'f'},
@@ -275 +269 @@
                         }
                         erase_it = 1;
-                        break;
-                case 'm':
-#if CONFIG_INTERNAL == 1
-                        tempstr = strdup(optarg);
-                        lb_vendor_dev_from_string(tempstr);
-#else
-                        fprintf(stderr, "Error: Internal programmer support "
-                                "was not compiled in and --mainboard only\n"
-                                "applies to the internal programmer. Aborting.\n");
-                        cli_classic_abort_usage();
-#endif
                         break;
                 case 'f':
@@ -426 +409 @@
         if (prog == PROGRAMMER_INVALID)
                 prog = default_programmer;
-
-#if CONFIG_INTERNAL == 1
-        if ((prog != PROGRAMMER_INTERNAL) && (lb_part || lb_vendor)) {
-                fprintf(stderr, "Error: --mainboard requires the internal "
-                                "programmer. Aborting.\n");
-                cli_classic_abort_usage();
-        }
-#endif
 
         /* FIXME: Delay calibration should happen in programmer code. */

trunk/flashrom.8

@@ -6 +6 @@
 \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\-m\fR [<vendor>:]<board>] \
+         [\fB\-c\fR <chipname>] \
 [\fB\-l\fR <file>]
          [\fB\-i\fR <image>] [\fB\-p\fR <programmername>[:<parameters>]]
@@ -89 +89 @@
 case sensitive.
 .TP
-.B "\-m, \-\-mainboard" [<vendor>:]<board>
-Override mainboard settings.
-.sp
-flashrom reads the coreboot table to determine the current mainboard. If no
-coreboot table could be read or if you want to override these values, you can
-specify \-m, e.g.:
-.sp
-.B "  flashrom \-\-mainboard AGAMI:ARUMA \-w agami_aruma.rom"
-.sp
-See the 'Known boards' or 'Known laptops' section in the output
-of 'flashrom \-L' for a list of boards which require the specification of
-the board name, if no coreboot table is found.
-.TP
 .B "\-f, \-\-force"
 Force one or more of the following actions:
@@ -246 +233 @@
 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 necessary to
-specify the mainboard using the
-.B \-m
-switch (see above).
+identify the mainboard (which is the exception), or if you want to override
+the detected mainboard model, you can specify the mainboard using the
+.sp
+.B "  flashrom \-p internal:mainboard=[<vendor>:]<board>"
+syntax.
+.sp
+See the 'Known boards' or 'Known laptops' section in the output
+of 'flashrom \-L' for a list of boards which require the specification of
+the board name, if no coreboot table is found.
 .sp
 Some of these board-specific flash enabling functions (called

trunk/internal.c

@@ -214 +214 @@
         free(arg);
 
+        arg = extract_programmer_param("mainboard");
+        if (arg && strlen(arg)) {
+                lb_vendor_dev_from_string(arg);
+        } else if (arg && !strlen(arg)) {
+                msg_perr("Missing argument for mainboard.\n");
+                free(arg);
+                return 1;
+        }
+        free(arg);
+
         get_io_perms();
         if (register_shutdown(internal_shutdown, NULL))

trunk/layout.c

@@ -107 +107 @@
         /*
          * If lb_vendor is not set, the coreboot table was
-         * not found. Nor was -m VENDOR:PART specified.
+         * not found. Nor was -p internal:mainboard=VENDOR:PART specified.
          */
         if (!lb_vendor || !lb_part) {
-                msg_pinfo("Note: If the following flash access fails, "
-                       "try -m <vendor>:<mainboard>.\n");
+                msg_pinfo("Note: If the following flash access fails, try "
+                          "-p internal:mainboard=<vendor>:<mainboard>.\n");
                 return 0;
         }
@@ -127 +127 @@
                 } else {
                         msg_pinfo("ERROR: Your firmware image (%s:%s) does not "
-                               "appear to\n       be correct for the detected "
-                               "mainboard (%s:%s)\n\nOverride with -p internal:"
-                               "boardmismatch=force if you are absolutely sure "
-                               "that\nyou are using a correct "
-                               "image for this mainboard or override\nthe detected "
-                               "values with --mainboard <vendor>:<mainboard>.\n\n",
-                               mainboard_vendor, mainboard_part, lb_vendor,
-                               lb_part);
+                                  "appear to\n"
+                                  "       be correct for the detected "
+                                  "mainboard (%s:%s)\n\n"
+                                  "Override with -p internal:boardmismatch="
+                                  "force to ignore the board name in the\n"
+                                  "firmware image or override the detected "
+                                  "mainboard with\n"
+                                  "-p internal:mainboard=<vendor>:<mainboard>."
+                                  "\n\n",
+                                  mainboard_vendor, mainboard_part, lb_vendor,
+                                  lb_part);
                         exit(1);
                 }

trunk/print.c

@@ -390 +390 @@
                 msg_ginfo(" ");
 
-        msg_ginfo("Status  Required option\n\n");
+        msg_ginfo("Status  Required value for\n");
+        for (i = 0; i < maxvendorlen + maxboardlen + strlen("Status  "); i++)
+                msg_ginfo(" ");
+        msg_ginfo("-p internal:mainboard=\n");
 
         for (b = boards; b->vendor != NULL; b++) {
@@ -408 +411 @@
                                 msg_ginfo("(autodetected)");
                         else
-                                msg_ginfo("-m %s:%s", e->lb_vendor,
+                                msg_ginfo("%s:%s", e->lb_vendor,
                                                    e->lb_part);
                 }

trunk/print_wiki.c

@@ -168 +168 @@
                        boards[i].name,
                        boards[i].url ? "]" : "",
-                       b[k].lb_vendor ? "-m " : "—",
+                       b[k].lb_vendor ? "-p internal:mainboard=" : "—",
                        b[k].lb_vendor ? b[k].lb_vendor : "",
                        b[k].lb_vendor ? ":" : "",

trunk/programmer.h

@@ -265 +265 @@
 
 /* cbtable.c */
-void lb_vendor_dev_from_string(char *boardstring);
+void lb_vendor_dev_from_string(const char *boardstring);
 int coreboot_init(void);
 extern char *lb_part, *lb_vendor;