RE: [PATCH iwl-next 1/2] PCI: Add PCI_VDEVICE_SUB helper macro
From: Kwapulinski, Piotr
Date: Mon Oct 21 2024 - 05:34:44 EST
>-----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.