On Mon, May 19, 2025 at 05:00:04PM +0200, Herve Codina wrote:
> On Thu, 8 May 2025 22:21:41 +0300
> Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx> wrote:
> > On Wed, May 07, 2025 at 09:13:05AM +0200, Herve Codina wrote:
...
> > > static struct pci_device_id lan966x_pci_ids[] = {
> > > - { PCI_DEVICE(PCI_VENDOR_ID_EFAR, 0x9660) },
> > > + { PCI_VDEVICE(EFAR, 0x9660), (kernel_ulong_t)&evb_lan9662_nic_info },
> >
> > PCI_DEVICE_DATA() ?
>
> PCI_DEVICE_DATA() need the device ID defined using a #define in the form
> PCI_DEVICE_ID_##vend##_##dev
>
> PCI_VDEVICE() allows the device ID value passed as an integer in the same
> way as for PCI_DEVICE().
>
> Also, according to its kdoc, it allows the next field to follow as the
> device private data.
>
> IMHO, I think PCI_VDEVICE() use is correct here and I will keep it.
It's correct, no doubts, but using PCI_DEVICE_DATA() makes sense when you need
to supply driver_data. In particular it will take care of needed castings and
also as you noticed asks users to apply the regular pattern for PCI ID
definitions.
Moreover, the 0x9660 is used in two drivers and it's a good candidate to be in
pci_ids.h. (Note drivers/pci/quirks.c:6286)
--
With Best Regards,
Andy Shevchenko
