Re: [PATCH v3 2/2] ata: Use named initializers for pci_device_id arrays

[Niklas Cassel]

Hello David,

On Fri, Jun 12, 2026 at 04:29:04PM +0100, David Laight wrote:
> On Fri, 12 Jun 2026 10:21:48 +0200
> Uwe Kleine-König (The Capable Hub) <u.kleine-koenig@xxxxxxxxxxxx> wrote:
> 
> > While being less compact, using named initializers allows to more easily
> > see which members of the structs are assigned which value without having
> > to lookup the declaration of the struct. And it's also more robust
> > against changes to the struct definition.
> 
> I think I'd try to keep each entry on one line for readability.
> Either by just letting the line be long or using a #define for the initialiser.
> 
> ...
> > diff --git a/drivers/ata/acard-ahci.c b/drivers/ata/acard-ahci.c
> > index 3999305b5356..402d3304b94b 100644
> > --- a/drivers/ata/acard-ahci.c
> > +++ b/drivers/ata/acard-ahci.c
> > @@ -91,9 +91,11 @@ static const struct ata_port_info acard_ahci_port_info[] = {
> >  };
> >  
> >  static const struct pci_device_id acard_ahci_pci_tbl[] = {
> > -	/* ACard */
> > -	{ PCI_VDEVICE(ARTOP, 0x000d), board_acard_ahci }, /* ATP8620 */
> > -
> > +	{
> > +		/* ACard ATP8620 */
> > +		PCI_VDEVICE(ARTOP, 0x000d),
> > +		.driver_data = board_acard_ahci,
> > +	},
> >  	{ }    /* terminate list */
> >  };
> 
> 
> Why not extend PCI_VDEVICE so you can pass it the .driver data
> (and other named initializers), something like:
> 
> #define PCI_VDEVICE(vend, dev, ...) \
> 	.vendor = PCI_VENDOR_ID_##vend, .device = (dev), \
> 	.subvendor = PCI_ANY_ID, .subdevice = PCI_ANY_ID, 0, 0 \
> 	__VA_OPT__(, .driver_data = __VA_ARGS__)
> 
> The change to all the files is then just to move the )
> (and the old versions will still compile).
> You can also do:
> 	PCI_VDEVICE(XXX, 0xx, board_xxx, .override_only = 1)
> 
> At which point you can move the {} inside - but that is a breaking change.
> 
> Note that sparse won't grok __VA_OPT__() so will need an alternate definition.

Sounds like a good idea to me, but it also sounds like it would touch
include/linux/pci.h, so probably would need to go via the PCI tree.

But if you manage to get your suggested macro included, I would be happy
to take patches that makes use of it. (Or even carry to carry a patch that
adds the macro if you manage to get an Acked-by from the PCI maintainers.)


Kind regards,
Niklas