Re: [PATCH net-next] net: Consistently define pci_device_ids using named initializers

Uwe Kleine-König (The Capable Hub) <[email protected]> Thu, 30 Apr 2026 15:13:13 +0200

Newsgroups	org.kernel.vger.linux-parisc,dev.linux.lists.brcm80211,org.kernel.vger.linux-can,org.kernel.vger.linux-kernel,org.kernel.vger.linux-rdma,org.kernel.vger.linux-wireless,org.kernel.vger.netdev,org.osuosl.intel-wired-lan
Message-ID	<afNUNeIsN4MrjWN7@monoceros>

Hello,

On Thu, Apr 30, 2026 at 02:53:29PM +0800, Jijie Shao wrote:
> on 2026/4/29 1:18, Uwe Kleine-König (The Capable Hub) wrote:
> > ... and PCI device helpers.
> > 
> > The various struct pci_device_id arrays were initialized mostly by one
> > the PCI_DEVICE macros and then list expressions. The latter 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* helper macros to assign .vendor, .device,
> > .subvendor and .subdevice where appropriate and skip explicit
> > assignments of 0 (which the compiler takes care of).
> > 
> > The secret plan is to make struct pci_device_id::driver_data an
> > anonymous union (similar to
> > https://lore.kernel.org/all/[email protected]/)
> > 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) <[email protected]>
> 
> ...
> 
> > diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_main.c b/drivers/net/ethernet/hisilicon/hibmcge/hbg_main.c
> > index 068da2fd1fea..b3e01b2f8319 100644
> > --- a/drivers/net/ethernet/hisilicon/hibmcge/hbg_main.c
> > +++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_main.c
> > @@ -489,7 +489,7 @@ static void hbg_shutdown(struct pci_dev *pdev)
> >   }
> >   static const struct pci_device_id hbg_pci_tbl[] = {
> > -	{PCI_VDEVICE(HUAWEI, 0x3730), 0},
> > +	{ PCI_VDEVICE(HUAWEI, 0x3730) },
> >   	{ }
> >   };
> 
> Reviewed-by: Jijie Shao <[email protected]>

Thanks.

> > +	{
> > +		PCI_VDEVICE(HUAWEI, HNAE3_DEV_ID_GE),
> > +		.driver_data = 0,
> > +	}, {
> > +		PCI_VDEVICE(HUAWEI, HNAE3_DEV_ID_25GE),
> > +		.driver_data = 0,
> 
> Thanks for your work.
> 
> If .driver_data = 0, is it possible to delete it to be consistent with other parts, for example:
> 
> { PCI_VDEVICE(HUAWEI, HNAE3_DEV_ID_GE) }
> 
> > +	}, {
> > +		PCI_VDEVICE(HUAWEI, HNAE3_DEV_ID_25GE_RDMA),
> > +		.driver_data = HNAE3_DEV_SUPPORT_ROCE_DCB_BITS,

However keeping the explicit .driver_data = 0 to have a contrast to
other `pci_device_id`s having a non-zero .driver_data in the same driver
is also a good reason to keep the (technically redundant) assignment.
For other drivers I dropped these assignments if this is possible for
all array members.

Having said that I don't intend to rework the patch for this suggestion.

Best regards
Uwe

signature.asc (application/pgp-signature, 488 B)