Re: [flashrom] [PATCH] Change programmer selection in cli and generic code

[Stefan Tauner]

On Mon, 05 Sep 2011 01:15:35 +0200
Carl-Daniel Hailfinger  wrote:
> New patch.
>
> 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.
> Unexport the programmer variable.
> programmer_init requires the programmer as first parameter.
> The default programmer selection is now part of cli_classic.
>
> Signed-off-by: Carl-Daniel Hailfinger
> Index: flashrom-programmer_selection_fix/it87spi.c
>
> --- flashrom-programmer_selection_fix/it87spi.c	(Revision 1427)
> +++ flashrom-programmer_selection_fix/it87spi.c	(Arbeitskopie)
> @@ -129,10 +129,8 @@
>    enter_conf_mode_ite(port);
>    /* NOLDN, reg 0x24, mask out lowest bit (suspend) */
>    tmp = sio_read(port, 0x24) & 0xFE;
> /* If IT87SPI was not explicitly selected, we want to check
> * quickly if LPC->SPI translation is active.
> */
> if ((programmer == PROGRAMMER_INTERNAL) && !(tmp & (0x0E))) {
> /* Check if LPC->SPI translation is active. */
> if (!(tmp & 0x0e)) {

just curious: why was this needed/wanted before?
> […]
> Index: flashrom-programmer_selection_fix/flashrom.c
> ===================================================================
> --- flashrom-programmer_selection_fix/flashrom.c	(Revision 1427)
> +++ flashrom-programmer_selection_fix/flashrom.c	(Arbeitskopie)
> […]
> @@ -515,9 +449,15 @@
>    return 0;
>  }
>
> -int programmer_init(char *param)
> +int programmer_init(enum programmer prog, char *param)
>  {
>    int ret;
> if (prog >= PROGRAMMER_INVALID) {

should we also check against < 0? enums are based on int. the default
starting point is 0 (if the first entry does not have a specific value
assigned with =), but i guess one could cast (enum programmer)-1 or so?
untested and maybe stupid... :)
> msg_perr("Invalid programmer specified!\n");
> return -1;

why so negative? ;)
> […]

apart from that and our default programmer dispute it looks good to me.
so please think of it as
Acked-by: Stefan Tauner