[coreboot] [RFC] kill unwanted_vpci, use "hidden" property instead

Carl-Daniel Hailfinger c-d.hailfinger.devel.2006 at gmx.net
Thu May 8 16:12:12 CEST 2008


On 08.05.2008 16:04, Stefan Reinauer wrote:
> Carl-Daniel Hailfinger wrote:
>   
>> Hi,
>>
>> since the Geode/CS553x platform emulates PCI headers for
>> chipset/processor devices, we have the ability to switch off that
>> emulation and hide those virtual PCI headers. Right now we do this in a
>> fairly uncoordinated fashion: I counted at least three different
>> open-coded code sequences with the same effect.
>>
>> My current plan is:
>> - encapsulate all those code sequences into calls of one or two functions
>> - switch all calls from hide_vpci(some_value_calculated_by_hand) to
>> hide_vpci_dev(struct device *)
>> - replace the central array of hand-calculated values with a
>> per-PCI-device "hidden" property.
>>
>> This should result in an overall improvement of code readability and a
>> reduction of code size. As a nice side benefit the dts is a lot easier
>> to manipulate.
>>   
>>     
> I like this idea a lot.
>
> Maybe we could use the already existing "enabled" property for that?
> Usually if a device is not "enabled", coreboot would not assign
> resources to it. Instead, if the device driver knows how to disable the
> device, it could completely disable it. I think the i82801xx driver in
> v2 attempts a similar thing.
>   

Interesting proposal. If I understand you correctly, enabled=0 would mean
- disabled on all chipsets
- hidden on those chipsets where hiding is supported.

There are two problems I have with that approach, though:
- I don't understand PCI well enough to know how a device is disabled.
Does disabling the device mean disabling the resources as well? Does it
mean the device won't respond to config cycles outside a special "enable
cycle" anymore?
- With automatic hiding, we lose the ability to leave a device
untouched. Do we need an enum(disabled, hidden, enabled, untouched) for
a full representation of possible states?


Regards,
Carl-Daniel




More information about the coreboot mailing list