Re: [PATCH] ata: 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: 2091 bytes --]

On Mon, May 04, 2026 at 10:09:53AM +0200, Damien Le Moal wrote:
> On 4/30/26 19:06, Uwe Kleine-König (The Capable Hub) wrote:
> > ... and PCI device helpers.
> > 
> > The .driver_data member in the various struct pci_device_id arrays were
> > initialized mostly by list expressions. This isn't easily readable if
> > you're not into PCI. Using named initializers is more explicit and thus
> > easier to parse.
> > 
> > Also use PCI_DEVICE to conveniently assign .vendor, .device, .subvendor
> > and .subdevice where appropriate.
> > 
> > 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.
> > 
> > This change doesn't introduce changes to the compiled pci_device_id
> > arrays. Tested on x86 and arm64.
> > 
> > Signed-off-by: Uwe Kleine-König (The Capable Hub) <u.kleine-koenig@baylibre.com>
> 
> This looks like a nice cleanup to me. A couple of nits below.
> With that, feel free to add:
> 
> Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
> 
> [...]
> 
> > diff --git a/drivers/ata/ata_piix.c b/drivers/ata/ata_piix.c
> > index 495fa096dd65..fac1266f7fa6 100644
> > --- a/drivers/ata/ata_piix.c
> > +++ b/drivers/ata/ata_piix.c
> > @@ -154,184 +154,186 @@ static unsigned int in_module_init = 1;
> >  
> >  static const struct pci_device_id piix_pci_tbl[] = {
> >  	/* Intel PIIX3 for the 430HX etc */
> > -	{ 0x8086, 0x7010, PCI_ANY_ID, PCI_ANY_ID, 0, 0, piix_pata_mwdma },
> > +	{ PCI_DEVICE(0x8086, 0x7010), .driver_data = piix_pata_mwdma },
> 
> Please split the line before the .driver_data like for all the other changes.
> That will make things consistent...

I would go for

	{
		PCI_DEVICE(0x8086, 0x7010),
		.driver_data = piix_pata_mwdma,
	}, {
		...

then. Or would you prefer something more compact like:

	{ PCI_DEVICE(0x8086, 0x7010),
	  .driver_data = piix_pata_mwdma },

?

Best regards
Uwe

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