On Sun, 04 Sep 2011 02:33:38 +0200 Carl-Daniel Hailfinger <[email protected]> wrote:
> Change programmer selection in cli and generic code > > Bugfix: Do not accept multiple conflicting --programmer selections. > Restriction: Do not accept multiple --programmer selections even if > there is no conflict. > Do not rely on exported programmer variable anymore. > programmer_init requires the programmer as first parameter. > > Signed-off-by: Carl-Daniel Hailfinger <[email protected]> > > Index: flashrom-programmer_selection_fix/cli_classic.c > =================================================================== > --- flashrom-programmer_selection_fix/cli_classic.c (Revision 1427) > +++ flashrom-programmer_selection_fix/cli_classic.c (Arbeitskopie) > @@ -111,6 +111,7 @@ > #endif > int read_it = 0, write_it = 0, erase_it = 0, verify_it = 0; > int dont_verify_it = 0, list_supported = 0, operation_specified = 0; > + enum programmer prog = PROGRAMMER_INVALID; > int ret = 0; > > static const char optstring[] = "r:Rw:v:nVEfc:m:l:i:p:Lzh"; > @@ -258,8 +259,16 @@ > #endif > break; > case 'p': > - for (programmer = 0; programmer < PROGRAMMER_INVALID; > programmer++) { > - name = programmer_table[programmer].name; > + if (prog != PROGRAMMER_INVALID) { > + fprintf(stderr, "Error: --programmer specified " > + "more than once. You can specify " > + "multiple progammer parameters with " > + "\",\". Please see the man page for " - You can specify multiple progammer parameters with ",". + You can specify multiple parameters for a progammer separated with ",". or similar... the above one sounds like multiple programmers are supported somehow imo. > + "details.\n"); > + cli_classic_abort_usage(); > + } > + for (prog = 0; prog < PROGRAMMER_INVALID; prog++) { > + name = programmer_table[prog].name; > namelen = strlen(name); > if (strncmp(optarg, name, namelen) == 0) { > switch (optarg[namelen]) { > @@ -283,7 +292,7 @@ > break; > } > } > - if (programmer == PROGRAMMER_INVALID) { > + if (prog == PROGRAMMER_INVALID) { > fprintf(stderr, "Error: Unknown programmer " > "%s.\n", optarg); > cli_classic_abort_usage(); > @@ -332,14 +341,6 @@ > } > #endif > > -#if CONFIG_INTERNAL == 1 > - if ((programmer != PROGRAMMER_INTERNAL) && (lb_part || lb_vendor)) { > - fprintf(stderr, "Error: --mainboard requires the internal " > - "programmer. Aborting.\n"); > - cli_classic_abort_usage(); > - } > -#endif > - > if (chip_to_probe) { > for (flash = flashchips; flash && flash->name; flash++) > if (!strcmp(flash->name, chip_to_probe)) > @@ -355,10 +356,21 @@ > flash = NULL; > } > > + 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. */ > myusec_calibrate_delay(); > > - if (programmer_init(pparam)) { > + if (programmer_init(prog, pparam)) { > fprintf(stderr, "Error: Programmer initialization failed.\n"); > ret = 1; > goto out_shutdown; > Index: flashrom-programmer_selection_fix/flashrom.c > =================================================================== > --- flashrom-programmer_selection_fix/flashrom.c (Revision 1427) > +++ flashrom-programmer_selection_fix/flashrom.c (Arbeitskopie) > @@ -42,10 +42,12 @@ > char *chip_to_probe = NULL; > int verbose = 0; > > +enum programmer programmer = PROGRAMMER_INVALID; > + everything below (i.e. the declaration and definition of default_programmer) should be moved to the cli file(s). selecting the default programmer is in the scope of the library user. and it slashes away one global. this may make chromiumos cry a bit though. if it is moved, these ifdefs could go inline into main()... i do not suggest this, just mentioning. a static variable is probably better. > #if CONFIG_INTERNAL == 1 > -enum programmer programmer = PROGRAMMER_INTERNAL; > +enum programmer default_programmer = PROGRAMMER_INTERNAL; > #elif CONFIG_DUMMY == 1 > -enum programmer programmer = PROGRAMMER_DUMMY; > +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 > @@ -55,7 +57,7 @@ > #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 way too much convenience for a (non-existent?) theoretical package maintainer at the cost of really existing library maintainers. i would remove the monster if and check for a compiler flag DEFAULT_PROGRAMMER or so first. if that's not defined, look for internal and dummy as fall-back then and just bail out with an error similar to the one below (but with mentioning DEFAULT_PROGRAMMER). this would also have benefits for the packager: he could select a default programmer no matter what other programmers are enabled. > #error Please enable either CONFIG_DUMMY or CONFIG_INTERNAL or disable > support for all programmers except one. > #endif > -enum programmer programmer = > +enum programmer default_programmer = > #if CONFIG_NIC3COM == 1 > PROGRAMMER_NIC3COM > #endif > @@ -515,9 +517,15 @@ > return 0; > } > > -int programmer_init(char *param) > +int programmer_init(enum programmer prog, char *param) > { > int ret; > + > + if (prog >= PROGRAMMER_INVALID) { > + msg_perr("Invalid programmer specified!\n"); > + return -1; > + } > + programmer = prog; > /* Initialize all programmer specific data. */ > /* Default to unlimited decode sizes. */ > max_rom_decode = (const struct decode_sizes) { > Index: flashrom-programmer_selection_fix/programmer.h > =================================================================== > --- flashrom-programmer_selection_fix/programmer.h (Revision 1427) > +++ flashrom-programmer_selection_fix/programmer.h (Arbeitskopie) > @@ -86,6 +86,7 @@ > }; > > extern enum programmer programmer; > +extern enum programmer default_programmer; to be removed, see above > struct programmer_entry { > const char *vendor; > @@ -110,7 +111,7 @@ > > extern const struct programmer_entry programmer_table[]; > > -int programmer_init(char *param); > +int programmer_init(enum programmer prog, char *param); > int programmer_shutdown(void); > > enum bitbang_spi_master_type { > > -- Kind regards/Mit freundlichen Grüßen, Stefan Tauner _______________________________________________ flashrom mailing list [email protected] http://www.flashrom.org/mailman/listinfo/flashrom
