Messages in this thread | | | Date | Thu, 22 Jan 2009 10:03:53 +0100 | From | Niels de Vos <> | Subject | Re: [PATCH] drivers/serial/8250_pci.c (add support for '8-port RS-232 MIC-3620 from advantech' |
| |
Tosoni wrote: >> On Wed, Jan 21, 2009 at 12:32:15PM +0000, Alan Cox wrote: >>>>>> static struct pci_device_id serial_pci_tbl[] = { >>>>>> + { PCI_VENDOR_ID_ADVANTECH, >> PCI_DEVICE_ID_ADVANTECH_PCI3620, >>>>>> + 0x3620, PCI_ANY_ID, 0, 0, >>>> Why not use PCI_VENDOR_ID_ADVANTECH as PCI_SUBVENDOR_ID too? >>> The Advantech vendor id is not 0x3620. This confused me as >> well which is >>> why I asked for an lspci. Advantech has stuck the device id in the >>> subvendor bits and '1' in the subdevice (so it should be 1 not >>> PCI_ANY_ID). >> is this better? >> >> + { PCI_VENDOR_ID_ADVANTECH, >> PCI_DEVICE_ID_ADVANTECH_PCI3620, >> + PCI_DEVICE_ID_ADVANTECH_PCI3620, 1, 0, 0, > > Since the name describes a device id where it should be a (sub)vendor id, > I would suggest that you add a line of comment to explain the case. > So that no one will be tempted to change it back to PCI_VENDOR_ID_ADVANTECH > in the future.
Definitely!
Niels
[unhandled content-type:application/pgp-signature] | |