Re: [PATCH] serial: 8250_pci: Consistently define pci_device_ids using named initializers

["Uwe Kleine-König (The Capable Hub)" <u.kleine-koenig@baylibre.com>]

[-- Attachment #1: Type: text/plain, Size: 2413 bytes --]

Hello Andy,

On Mon, Apr 20, 2026 at 09:32:57PM +0300, Andy Shevchenko wrote:
> On Mon, Apr 20, 2026 at 07:56:48PM +0200, Uwe Kleine-König (The Capable Hub) wrote:
> > ... and PCI device helpers.
> > 
> > The various struct pci_device_id were defined using a mixture of
> > initialization by position and by name. Some use the PCI device helpers
> > (like PCI_DEVICE and PCI_DEVICE_SUB) and others don't.
> > 
> > Consistently use named initializers, drop assignments of 0 by position
> > for .class and .class_mask and use the PCI device helpers. Also use
> > consistent line-breaks and positioning for opening and closing curly
> > braces.
> > 
> > The secret plan is to make struct pci_device_id::driver_data an
> > anonymous union (similar to
> > https://lore.kernel.org/all/cover.1776579304.git.u.kleine-koenig@baylibre.com/)
> > and that requires named initializers. But it's also a nice cleanup on
> > its own.
> 
> Can we rather switch to PCI_DEVICE_DATA() and perhaps you would like to have
> PCI_DEVICE_SUB_DATA() implemented in the similar way.
> 
> This will shorten a lot the lines, the
> 
> 		PCI_DEVICE_SUB(PCI_VENDOR_ID_V3, PCI_DEVICE_ID_V3_V960,
> 			       PCI_SUBVENDOR_ID_CONNECT_TECH, PCI_SUBDEVICE_ID_CONNECT_TECH_BH8_232),
> 		.driver_data = pbn_b1_8_1382400,
> 
> to, for example,
> 
> 		PCI_DEVICE_SUB_DATA(V3, V960, CONNECT_TECH, BH8_232, pbn_b1_8_1382400),

Not sure I want to open that can. My gut feeling is that it's fine to
group .vendor, .device, .subvendor and .subdevice in a macro as they are
similar in spirit (hardware property, purpose of identifying the device)
but .driver_data doesn't really match that. Also I really like the
explicit mentioning of the member name for driver_data because while I
think the suggested macro name is as good as it can be, in my opinion
the first snippet is easier to grasp for someone who is not deep into
pci and so I consider the additional line space it requires to be well
spend.  Also prepending PCI_VENDOR_ID_ and PCI_VENDOR_ID_ ## vend ## _
is nice to get a more compact representation and enforce consistency,
which is both useful sometimes, it has the disadvantage that I cannot
easily make my editor jump to the definition of PCI_VENDOR_ID_V3_V960.

So in sum, while I see the advantages of your suggestion, I don't want
to make this part of my current quest.

Best regards
Uwe

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]