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

[Carl-Daniel Hailfinger]

On 24.10.2009 00:42, Luc Verhaegen wrote:
> On Fri, Oct 23, 2009 at 06:49:18PM +0200, Carl-Daniel Hailfinger wrote:
>   
>> On 23.10.2009 16:27, Luc Verhaegen wrote:
>>     
>>> On Fri, Oct 23, 2009 at 01:41:41AM +0200, Carl-Daniel Hailfinger wrote:
>>>
>>>       
>> Yes, but the way you suggested keeps the description around in a comment
>> and an error message. That's duplication.
>>     
>
> Yeah, we can do away with this duplication by turning that into one 
> function which takes the name as the argument.
>   

If we're trying to rebuild the device name printing of lspci, we might
as well have a function which looks up the name.


>> Let's use your original pci_dev_find variant and refactor it later.
>> Right now this discussion is leading nowhere and I want the important
>> parts of the patch merged.
>>     
>
> Yeah, this way it is at least in the same shape as the other functions, 
> and this refactoring (of all) can be done later without much danger, the 
> touching of extra gpio bits now is much more likely to break things.
>   

Right.


>> If we get reports from our testers that everything is fine indeed, I'm
>> OK with either byte or word or dword access.
>>     
>
> Wait and see. I will try to get this one board enable resent, on top of 
> this patch here. And will get idwer his board enable out (finally). This 
> is two more testcases.
>   

Good.


>> The pain is in boards with multiple flash chips where flashrom may
>> toggle the chip select lines in the future (DualBIOS). Anyway, we can
>> handle this later.
>>     
>
> Right, solve it when it occurs and when we know what it is exactly that 
> we need to do.
>   

Just wanted to mention it. We'll probably deal with this later.


>>> * part of the dell comment restored.
>>>       
> Done.
>   

Thanks.


> Latest patch attached, difference is just those two comments and the 
> intel_ prependage (and the fact that i now went to git-svn so i can 
> juggle the patches more easily). INL/OUTL was kept while waiting for 
> testing.
>   

I have one more comment, but feel free to address this in a followup
patch: You're not checking the GPIO number against the upper limit of
available GPIOs per chipset. This can lead to unpleasant accidents if
someone specifies a GPIO bitmask instead of a GPIO number (like it
happened ~2 weeks ago).

If you're going to send a followup with the GPIO number checking
mentioned above, this is
Acked-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006 at gmx.net>

Regards,
Carl-Daniel

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