Messages in this thread Patch in this message | ![/](/images/icornerl.gif) | | Date | Tue, 12 Aug 2003 11:01:58 -0700 | From | Greg KH <> | Subject | Re: C99 Initialisers |
| |
On Tue, Aug 12, 2003 at 12:27:29PM +0100, Matthew Wilcox wrote: > On Mon, Aug 11, 2003 at 10:38:27PM -0700, Greg KH wrote: > > On Tue, Aug 12, 2003 at 03:39:36AM +0100, Matthew Wilcox wrote: > > > I don't think anyone would appreciate you converting that to: > > > > > > static struct pci_device_id tg3_pci_tbl[] __devinitdata = { > > > { > > > .vendor = PCI_VENDOR_ID_BROADCOM, > > > .device = PCI_DEVICE_ID_TIGON3_5700, > > > .subvendor = PCI_ANY_ID, > > > .subdevice = PCI_ANY_ID, > > > .class = 0, > > > .class_mask = 0, > > > .driver_data = 0, > > > }, > > > > I sure would. Oh, you can drop the .class, .class_mask, and > > .driver_data lines, and then it even looks cleaner. > > > > I would love to see that kind of change made for pci drivers. > > I really strongly disagree. For a struct that's as well-known as > pci_device_id, the argument of making it clearer doesn't make sense.
It's not _that_ well known. I often have to go look it up to mentally match up the fields when looking at pci drivers again after a few weeks away. With the above change, I can instantly know what is going on.
> It's also not subject to change, unlike say file_operations, so the > argument of adding new elements without breaking anything is also not > relevant.
Who knows, it might change in the future, never say never :)
> In tg3, the table definition is already 32 lines long with 2 lines per > device_id. In the scheme you favour so much, that becomes 92 lines, for > no real gain that I can see.
In the end, it's up to the maintainer of the driver what they want to do. So, Jeff and David, here's a patch against the latest 2.6.0-test3 tg3.c that converts the pci_device_id table to C99 initializers. If you want to, please apply it.
thanks,
greg k-h
# Convert tg3 pci_device_id table to C99 initializers
--- a/drivers/net/tg3.c Mon Aug 11 09:21:41 2003 +++ b/drivers/net/tg3.c Tue Aug 12 10:58:30 2003 @@ -128,36 +128,96 @@ static int tg3_debug = -1; /* -1 == use TG3_DEF_MSG_ENABLE as value */ static struct pci_device_id tg3_pci_tbl[] = { - { PCI_VENDOR_ID_BROADCOM, PCI_DEVICE_ID_TIGON3_5700, - PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0UL }, - { PCI_VENDOR_ID_BROADCOM, PCI_DEVICE_ID_TIGON3_5701, - PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0UL }, - { PCI_VENDOR_ID_BROADCOM, PCI_DEVICE_ID_TIGON3_5702, - PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0UL }, - { PCI_VENDOR_ID_BROADCOM, PCI_DEVICE_ID_TIGON3_5703, - PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0UL }, - { PCI_VENDOR_ID_BROADCOM, PCI_DEVICE_ID_TIGON3_5704, - PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0UL }, - { PCI_VENDOR_ID_BROADCOM, PCI_DEVICE_ID_TIGON3_5702FE, - PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0UL }, - { PCI_VENDOR_ID_BROADCOM, PCI_DEVICE_ID_TIGON3_5702X, - PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0UL }, - { PCI_VENDOR_ID_BROADCOM, PCI_DEVICE_ID_TIGON3_5703X, - PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0UL }, - { PCI_VENDOR_ID_BROADCOM, PCI_DEVICE_ID_TIGON3_5704S, - PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0UL }, - { PCI_VENDOR_ID_BROADCOM, PCI_DEVICE_ID_TIGON3_5702A3, - PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0UL }, - { PCI_VENDOR_ID_BROADCOM, PCI_DEVICE_ID_TIGON3_5703A3, - PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0UL }, - { PCI_VENDOR_ID_SYSKONNECT, 0x4400, - PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0UL }, - { PCI_VENDOR_ID_ALTIMA, PCI_DEVICE_ID_ALTIMA_AC1000, - PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0UL }, - { PCI_VENDOR_ID_ALTIMA, PCI_DEVICE_ID_ALTIMA_AC1001, - PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0UL }, - { PCI_VENDOR_ID_ALTIMA, PCI_DEVICE_ID_ALTIMA_AC9100, - PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0UL }, + { + .vendor = PCI_VENDOR_ID_BROADCOM, + .device = PCI_DEVICE_ID_TIGON3_5700, + .subvendor = PCI_ANY_ID, + .subdevice = PCI_ANY_ID + }, + { + .vendor = PCI_VENDOR_ID_BROADCOM, + .device = PCI_DEVICE_ID_TIGON3_5701, + .subvendor = PCI_ANY_ID, + .subdevice = PCI_ANY_ID + }, + { + .vendor = PCI_VENDOR_ID_BROADCOM, + .device = PCI_DEVICE_ID_TIGON3_5702, + .subvendor = PCI_ANY_ID, + .subdevice = PCI_ANY_ID + }, + { + .vendor = PCI_VENDOR_ID_BROADCOM, + .device = PCI_DEVICE_ID_TIGON3_5703, + .subvendor = PCI_ANY_ID, + .subdevice = PCI_ANY_ID + }, + { + .vendor = PCI_VENDOR_ID_BROADCOM, + .device = PCI_DEVICE_ID_TIGON3_5704, + .subvendor = PCI_ANY_ID, + .subdevice = PCI_ANY_ID + }, + { + .vendor = PCI_VENDOR_ID_BROADCOM, + .device = PCI_DEVICE_ID_TIGON3_5702FE, + .subvendor = PCI_ANY_ID, + .subdevice = PCI_ANY_ID + }, + { + .vendor = PCI_VENDOR_ID_BROADCOM, + .device = PCI_DEVICE_ID_TIGON3_5702X, + .subvendor = PCI_ANY_ID, + .subdevice = PCI_ANY_ID + }, + { + .vendor = PCI_VENDOR_ID_BROADCOM, + .device = PCI_DEVICE_ID_TIGON3_5703X, + .subvendor = PCI_ANY_ID, + .subdevice = PCI_ANY_ID + }, + { + .vendor = PCI_VENDOR_ID_BROADCOM, + .device = PCI_DEVICE_ID_TIGON3_5704S, + .subvendor = PCI_ANY_ID, + .subdevice = PCI_ANY_ID + }, + { + .vendor = PCI_VENDOR_ID_BROADCOM, + .device = PCI_DEVICE_ID_TIGON3_5702A3, + .subvendor = PCI_ANY_ID, + .subdevice = PCI_ANY_ID + }, + { + .vendor = PCI_VENDOR_ID_BROADCOM, + .device = PCI_DEVICE_ID_TIGON3_5703A3, + .subvendor = PCI_ANY_ID, + .subdevice = PCI_ANY_ID + }, + { + .vendor = PCI_VENDOR_ID_SYSKONNECT, + .device = 0x4400, + .subvendor = PCI_ANY_ID, + .subdevice = PCI_ANY_ID + }, + { + .vendor = PCI_VENDOR_ID_ALTIMA, + .device = PCI_DEVICE_ID_ALTIMA_AC1000, + .subvendor = PCI_ANY_ID, + .subdevice = PCI_ANY_ID + }, + { + .vendor = PCI_VENDOR_ID_ALTIMA, + .device = PCI_DEVICE_ID_ALTIMA_AC1001, + .subvendor = PCI_ANY_ID, + .subdevice = PCI_ANY_ID + }, + { + .vendor = PCI_VENDOR_ID_ALTIMA, + .device = PCI_DEVICE_ID_ALTIMA_AC9100, + .subvendor = PCI_ANY_ID, + .subdevice = PCI_ANY_ID + }, { 0, } }; - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| ![\](/images/icornerr.gif) |