>-----Original Message----- >From: Bjorn Helgaas <helgaas@xxxxxxxxxx> >Sent: Thursday, October 17, 2024 4:22 PM >To: Kwapulinski, Piotr <piotr.kwapulinski@xxxxxxxxx> >Cc: intel-wired-lan@xxxxxxxxxxxxxxxx; netdev@xxxxxxxxxxxxxxx; bhelgaas@xxxxxxxxxx; linux-pci@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; Kitszel, Przemyslaw <przemyslaw.kitszel@xxxxxxxxx> >Subject: Re: [PATCH iwl-next 1/2] PCI: Add PCI_VDEVICE_SUB helper macro > >On Thu, Oct 17, 2024 at 03:16:47PM +0200, Piotr Kwapulinski wrote: >> PCI_VDEVICE_SUB generates the pci_device_id struct layout for the >> specific PCI device/subdevice. The subvendor field is set to >> PCI_ANY_ID. Private data may follow the output. >> >> Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@xxxxxxxxx> >> Signed-off-by: Piotr Kwapulinski <piotr.kwapulinski@xxxxxxxxx> >> --- >> include/linux/pci.h | 14 ++++++++++++++ >> 1 file changed, 14 insertions(+) >> >> This patch is a part of the series from netdev. >> >> diff --git a/include/linux/pci.h b/include/linux/pci.h index >> 573b4c4..2b6b2c8 100644 >> --- a/include/linux/pci.h >> +++ b/include/linux/pci.h >> @@ -1050,6 +1050,20 @@ struct pci_driver { >> .vendor = PCI_VENDOR_ID_##vend, .device = (dev), \ >> .subvendor = PCI_ANY_ID, .subdevice = PCI_ANY_ID, 0, 0 >> >> +/** >> + * PCI_VDEVICE_SUB - describe a specific PCI device/subdevice in a >> +short form >> + * @vend: the vendor name >> + * @dev: the 16 bit PCI Device ID >> + * @subdev: the 16 bit PCI Subdevice ID >> + * >> + * Generate the pci_device_id struct layout for the specific PCI >> + * device/subdevice. The subvendor field is set to PCI_ANY_ID. >> +Private data >> + * may follow the output. >> + */ >> +#define PCI_VDEVICE_SUB(vend, dev, subdev) \ >> + .vendor = PCI_VENDOR_ID_##vend, .device = (dev), \ >> + .subvendor = PCI_ANY_ID, .subdevice = subdev, 0, 0 > >I don't think it's right to specify the subdevice (actually "Subsystem ID" per spec) without specifying the subvendor ("Subsystem Vendor ID" >in the spec). > >Subsystem IDs are assigned by the vendor, so they have to be used in conjunction with the Subsystem Vendor ID. See PCIe r6.0, sec >7.5.1.2.3: I'll add the subvendor to the interface. Thank you for comments. Piotr > > Values for the Subsystem ID are vendor assigned. Subsystem ID > values, in conjunction with the Subsystem Vendor ID, form a unique > identifier for the PCI product. Subsystem ID and Device ID values > are distinct and unrelated to each other, and software should not > assume any relationship between them.