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

[Stefan Tauner]

On Mon, 05 Sep 2011 01:15:35 +0200
Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006 at gmx.net> 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 <c-d.hailfinger.devel.2006 at gmx.net>
> 
> 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 <stefan.tauner at student.tuwien.ac.at>

-- 
Kind regards/Mit freundlichen Grüßen, Stefan Tauner