[flashrom] [PATCH] Change programmer selection in cli and generic code
Stefan Tauner
stefan.tauner at student.tuwien.ac.at
Sun Sep 4 13:03:21 CEST 2011
On Sun, 04 Sep 2011 02:33:38 +0200
Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006 at gmx.net> 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 <c-d.hailfinger.devel.2006 at gmx.net>
>
> 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
More information about the flashrom
mailing list