Am 05.09.2011 23:57 schrieb Stefan Tauner:
> On Mon, 05 Sep 2011 01:15:35 +0200
> Carl-Daniel Hailfinger <[email protected]> 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 <[email protected]>
>>
>> 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 <[email protected]>
>   

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/


_______________________________________________
flashrom mailing list
[email protected]
http://www.flashrom.org/mailman/listinfo/flashrom

Reply via email to