On Sat, 2009-06-27 at 12:20 -0500, James Bottomley wrote: > OK, so there's no description whatsoever how am I supposed to divine the > reasons for doing this? You might look at the 0/19 patch description. http://lkml.org/lkml/2009/6/25/21 > Because if I look at an example: > > > --- a/drivers/scsi/3w-9xxx.c > > +++ b/drivers/scsi/3w-9xxx.c > > @@ -2278,14 +2278,10 @@ out_disable_device: > > > > /* PCI Devices supported by this driver */ > > static struct pci_device_id twa_pci_tbl[] __devinitdata = { > > - { PCI_VENDOR_ID_3WARE, PCI_DEVICE_ID_3WARE_9000, > > - PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0}, > > - { PCI_VENDOR_ID_3WARE, PCI_DEVICE_ID_3WARE_9550SX, > > - PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0}, > > - { PCI_VENDOR_ID_3WARE, PCI_DEVICE_ID_3WARE_9650SE, > > - PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0}, > > - { PCI_VENDOR_ID_3WARE, PCI_DEVICE_ID_3WARE_9690SA, > > - PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0}, > > + { PCI_VDEVICE(3WARE, PCI_DEVICE_ID_3WARE_9000), 0}, > > + { PCI_VDEVICE(3WARE, PCI_DEVICE_ID_3WARE_9550SX), 0}, > > + { PCI_VDEVICE(3WARE, PCI_DEVICE_ID_3WARE_9650SE), 0}, > > + { PCI_VDEVICE(3WARE, PCI_DEVICE_ID_3WARE_9690SA), 0}, > > It appears to take PCI matching on vendor device with any subvendor, > device and reproduce the results with a macro, which, for good measure > pastes PCI_VENDOR_ID on to the first argument but *doesn't* paste > PCI_DEVICE_ID on to the second? A single macro can't use both a named constant and a magic number as the second argument. Today, only about half of the pci device declarations use named constants, mostly defined in pci_ids.h Style 1: PCI_VDEVICE(VENDOR, PCI_DEVICE_ID_<foo>) $ grep -rP --include=*.[ch] \ "\bPCI_VDEVICE\s*\(\s*\w+\s*,\s*PCI_DEVICE_ID" * | \ wc -l 351 Style 2: PCI_VDEVICE(VENDOR, 0x<hex#>) $ grep -rP --include=*.[ch] \ "\bPCI_VDEVICE\s*\(\s*\w+\s*,\s*0[xX][0-9a-fA-F]{1,4}" * | \ wc -l 287 PCI_DEVICE(VENDOR, PCI_DEVICE_ID_<foo>) $ grep -rP --include=*.[ch] \ "\bPCI_DEVICE\s*\(\s*\w+\s*,\s*PCI_DEVICE_ID" * | \ wc -l 373 PCI_DEVICE(VENDOR, 0x<foo>) $ grep -rP --include=*.[ch] \ "\bPCI_DEVICE\s*\(\s*\w+\s*,\s*0[xX][0-9a-fA-F]{1,4}" * | \ wc -l 320 After this patchset: $ grep -rP --include=*.[ch] \ "\bPCI_VDEVICE\s*\(\s*\w+\s*,\s*PCI_DEVICE_ID" * | \ wc -l 831 $ grep -rP --include=*.[ch] \ "\bPCI_VDEVICE\s*\(\s*\w+\s*,\s*0[xX][0-9a-fA-F]{1,4}" * | \ wc -l 571 > So it transforms a relatively opaque initialiser which most people would > have to look up in pci.h into another relatively opaque initialiser > indirected via a macro, so now I have to look up both the structure and > the macro to figure out what's going on. > > If that's all it does, I'm having a hard time seeing any actual > improvement here. Greater standardization of declarations across multiple files. Do you have a suggestion or preference for another macro name for the named constants uses? drivers/edac/edac_core.h has: #define PCI_VEND_DEV(vend, dev) PCI_VENDOR_ID_ ## vend, \ PCI_DEVICE_ID_ ## vend ## _ ## dev cheers, Joe -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
