[flashrom] [PATCH] Boards: Unify all intel ICH GPIO raising.

Carl-Daniel Hailfinger c-d.hailfinger.devel.2006 at gmx.net
Fri Oct 23 01:46:52 CEST 2009


On 22.10.2009 23:44, Uwe Hermann wrote:
> On Thu, Oct 22, 2009 at 03:16:16PM +0200, Luc Verhaegen wrote:
>
> I agree we should drop the totally useless 0x48 register index, 0x0c
> offset, and 0xffc0 bitmask from the function prototype.
>   

Absolutely.


> Also, IMHO "name" is pretty useless, it's on my TODO list to drop that anyway.
>   

Indeed.


> However, I very much think that we _do_ want the PCI IDs to stay in there.
>
> Example call would look like this:
>
>   ich_gpio_raise(name, 0x8086, 0x27B8, 34);
>
> I'm fine with also dropping 0x8086 (PCI vendor ID) as that should always
> be the same for the ich_* function. If we later also drop "name" this
> becomes:
>
>   ich_gpio_raise(0x27B8, 34);
>
> Which is perfect.
>   

Hm yes. It is a bit ugly in the case of multiple GPIOs per board, but
other than that... please see my mail from a few minutes ago for more
commentary.


>> Or even this, which still does not make me comfortable, and which 
>> requires the big switch statement in the ich_gpio_raise function anyway.
>>
>> {
>>         int ret;
>>         ret = ich_gpio_raise(name, 0x27B8, 34);
>>         if (!ret)
>>                 ret = ich_gpio_raise(name, 0x27B8, 35);
>>
>>         return ret;     
>> }
>>     
>
> This is perfectly fine if you ask me. Many board enables only raise
> one pin and thus are as simple as:
>
>   {
>   	return ich_gpio_raise(name, 0x27B8, 34);
>   }
>
> Which I like a lot. And the above example is very nice and readable,too.
>   

Well yes.


>> Once you have seen a few of those pci dev finding things, and they all 
>> get copy/pasted around anyway, you quickly see what is important to 
>> them, and they are all very formulaic anyway. You learn to ignore the 
>> rest.
>>     
>
> No need to copy any of it around, that's just useless code duplication.
> Just put them in the ich_gpio_raise() function, they're fine in there.
>   

We should strive to keep the board enables
1. short and
2. easy to understand.

The old code clearly fails #2, while some of the new proposals fail #1.

Apart from that, this discussion really helps us define our goals for
the future.

Regards,
Carl-Daniel

-- 
Developer quote of the week: 
"We are juggling too many chainsaws and flaming arrows and tigers."





More information about the flashrom mailing list