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

[David Laight <david.laight.linux@gmail.com>]

On Fri, 12 Jun 2026 10:21:48 +0200
Uwe Kleine-König (The Capable Hub) <u.kleine-koenig@baylibre.com> 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.

-- David