Re: C99 Initialisers

[junkio]

>>>>> "GKH" == Greg KH <greg@xxxxxxxxx> writes:

GKH> How about this patch?  If you like it I'll add the pci.h change to the
GKH> tree and let you take the tg3.c part.

Two comments:

GKH> ...
GKH> +	{ PCI_DEVICE(PCI_VENDOR_ID_BROADCOM, PCI_DEVICE_ID_TIGON3_5704S) },
GKH> +	{ PCI_DEVICE(PCI_VENDOR_ID_BROADCOM, PCI_DEVICE_ID_TIGON3_5702A3) },
GKH> +	{ PCI_DEVICE(PCI_VENDOR_ID_BROADCOM, PCI_DEVICE_ID_TIGON3_5703A3) },
GKH> ...

GKH> +#define PCI_DEVICE(vend,dev) \
GKH> +	.vendor = (vend), .device = (dev), \
GKH> +	.subvendor = PCI_ANY_ID, .subdevice = PCI_ANY_ID
 
 (1) If .subvendor and .subdevice are always PCI_ANY_ID, are
     there any reason to keep them in the structure in the first
     place?  I imagine there are some devices but not in the
     tg3_pci_tbl list that need to have different values there,
     but if that is the case we may want to generalize the macro
     PCI_DEVICE like this:

        #define PCI_DEVICE(vend, dev) \
            PCI_DEVICE_WITH_SUB(vend, dev, PCI_ANY_ID, PCI_ANY_ID)
        #define PCI_DEVICE_WITH_SUB(vend, dev, subv, subd) \
         .vendor = (vend), \
         .device = (dev), \
         .subvendor = (subv), \
         .subdevice = (subd)

 (2) PCI_VENDOR_ID_ and PCI_DEVICE_ID_ seem to be common prefix,
     so how about doing something like this?

     #define PCI_DEVICE(vend,dev) \
         .vendor = (PCI_VENDOR_ID_ ## vend), \
         .device = (PCI_DEVICE_ID_ ## dev), \
         .subvendor = PCI_ANY_ID, \
         .subdevice = PCI_ANY_ID

     Then the table becomes much shorter:

     static struct pci_device_id tg3_pci_tbl[] = {
     ...
       { PCI_DEVICE(BROADCOM, TIGON3_5700) },
       { PCI_DEVICE(BROADCOM, TIGON3_5701) },
     ...


    PCI_DEVICE_ID_xxx definitions for some devices that
    currently lack them need to be added, of course,
    e.g. SYSKONNECT 0x4400.

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/