[coreboot] patch: v3 dts support for multiple link devices such as k8 north

Carl-Daniel Hailfinger c-d.hailfinger.devel.2006 at gmx.net
Wed Sep 17 14:16:05 CEST 2008


On 17.09.2008 05:53, ron minnich wrote:
> On Tue, Sep 16, 2008 at 6:12 PM, Carl-Daniel Hailfinger
> <c-d.hailfinger.devel.2006 at gmx.net> wrote:
>
>   
>> I feel bad for pointing out more problems.
>> IIRC dev->bus.dev used to point to the parent device. With the current
>> patch, lots of devices are their own parents.
>>     
>
> OK, my bad, I just fixed it and it removed 3 lines:
>
> struct device dev_root = {
>         .bus = &dev_root.link[0],
> struct device dev_cpus = {
>         .bus = &dev_root.link[0],
> struct device dev_apic_0 = {
>         .bus = &dev_root.link[0],
> struct device dev_domain_0 = {
>         .bus = &dev_root.link[0],
> struct device dev_domain_0_pci_1_0 = {
>         .bus = &dev_domain_0.link[0],
> struct device dev_domain_0_pci0_18_0 = {
>         .bus = &dev_domain_0.link[0],
> struct device dev_domain_0_pci0_18_0_pci_0_0 = {
>         .bus = &dev_domain_0_pci0_18_0.link[0],
> struct device dev_domain_0_pci0_18_0_pci_4_0 = {
>         .bus = &dev_domain_0_pci0_18_0.link[0],
> struct device dev_domain_0_pci0_18_0_pci_5_0 = {
>         .bus = &dev_domain_0_pci0_18_0.link[0],
> struct device dev_domain_0_pci1_18_0 = {
>         .bus = &dev_domain_0_pci0_18_0.link[1],
> struct device dev_domain_0_pci2_18_0 = {
>         .bus = &dev_domain_0_pci0_18_0.link[2],
> struct device dev_domain_0_pci2_18_0_pci_2_0 = {
>         .bus = &dev_domain_0_pci0_18_0.link[2],
> struct device dev_domain_0_ioport_2e = {
>         .bus = &dev_domain_0.link[0],
> struct device_operations *all_device_operations[] = {
>         .bus = &dev_root.link[0],
>
> better?
>   

Yes. AFAICS it now matches the original code. Good.


>> To be honest, I think the design of the patch has potential for
>> improvement (my personal opinion only). Three points stick out:
>> - The dummy struct device with full config (including primary link
>> config) for each non-primary link seems like a fifth wheel.
>>     
>
> no, it is needed for the dynamic device tree code. What's going to
> happen is that the bus
> struct for that device, although in the primary link, points to that
> fifth wheel. It looks odd but
> IIRC it does work. Let's leave it in.
>   

Hm. I won't stop you now, but I'll work on a slightly different design RFC.


>> - The dual-purpose struct device for a device and its primary link will
>> make more complex HT scenarios a real nightmare, potentially triggering
>> a complete rewrite.
>>     
>
> Maybe. It has indeed worked for some years, and some weird configurations.
>   

I can't argue with that. Right now the priority is getting K8 working
reasonably well. Design cleanliness can come after stuff works.


>> - No explicit designation of link numbers for HT in the dts and usage of
>> clever magic instead.
>>     
>
> agreed. This is going to work for now. Once k8 is up and working we
> may need another spin
> on the device code.
>   

Go ahead with your patch. We need it now.


> Testing now on both platforms ... I assume if I post an "it works" and
> you see this fixed parent link, we are good to go?
>   

Yes. Then it is
Acked-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006 at gmx.net>


Regards,
Carl-Daniel

-- 
http://www.hailfinger.org/





More information about the coreboot mailing list