[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