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

Carl-Daniel Hailfinger c-d.hailfinger.devel.2006 at gmx.net
Tue Sep 6 02:20:31 CEST 2011


Am 05.09.2011 23:57 schrieb 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?
>   

This was a hunk I forgot to remove when I killed the separate it87spi
programmer. That piece of code was the only location which needed a
global programmer variable.


>> […]
>> 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.

Not sure about that:
http://software.intel.com/en-us/articles/strict-ansi-switch-in-linux-and-mac/
Are they really guaranteed to be signed int? If not, some compilers will
warn about "comparison always yields false".


> 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... :)
>   

I thought about it, and decided to only check for >= PROGRAMMER_INVALID.


>> +		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>
>   

Thanks, I'll commit in the next ~24 hours. If you're unhappy about my
enum answer, please tell me and I'll dig into the standard again.


Regards,
Carl-Daniel

-- 
http://www.hailfinger.org/





More information about the flashrom mailing list