Re: [coreboot] r593 -

[Carl-Daniel Hailfinger]

On 15.02.2008 01:25, ron minnich wrote:
> Try this, simplifies the dts, removes redundant ids.
>
> might even allow us to totally yank the pciid, domainid, etc. keywords

I like it.
> Index: northbridge/amd/geodelx/geodelx.c
>
> --- northbridge/amd/geodelx/geodelx.c	(revision 592)
> +++ northbridge/amd/geodelx/geodelx.c	(working copy)
> @@ -354,24 +354,23 @@
> The constructor for the device.
> Domain ops and APIC cluster ops and PCI device ops are different.
>
> */
> -struct constructor geodelx_north_constructors[] = {
> +struct constructor geodelx_north_domain = {
>    /* Northbridge running a PCI domain. */
> {.id = {.type = DEVICE_ID_PCI_DOMAIN,
> .id = {.type = DEVICE_ID_PCI_DOMAIN,
> .u = {.pci_domain = {.vendor = PCI_VENDOR_ID_AMD,
> 		     .device = PCI_DEVICE_ID_AMD_LXBRIDGE}}},
> .ops = &geodelx_pcidomain_ops},
> geodelx_north_apic = {

+ struct constructor geodelx_north_apic = {
> /* Northbridge running an APIC cluster. */
> {.id = {.type = DEVICE_ID_APIC_CLUSTER,
>
> .id = {.type = DEVICE_ID_APIC_CLUSTER,
> .u = {.apic_cluster = {.vendor = PCI_VENDOR_ID_AMD,
> 		       .device = PCI_DEVICE_ID_AMD_LXBRIDGE}}},
> .ops = &geodelx_apic_ops},
>
> /* Northbridge running a PCI device. */
> {.id = {.type = DEVICE_ID_PCI,
> geodelx_north_pci = {
> .id = {.type = DEVICE_ID_PCI,
> .u = {.pci = {.vendor = PCI_VENDOR_ID_AMD,
> 	      .device = PCI_DEVICE_ID_AMD_LXBRIDGE}}},
> .ops = &geodelx_pci_ops},
> {.ops = 0},
>
> -};
> .ops = &geodelx_pci_ops};

Two comments about the style while it is not yet carved in stone.
- Can you prefix every constructor with "struct constructor"?
- Can we possibly use anonymous unions to get rid of that obnoxious .u?
(separate patch)

Regards,
Carl-Daniel