[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