Re: [PATCH] ntb: amd: Use named initializer for pci_device_id::driver_data

[Dave Jiang]

On 5/7/26 12:41 AM, Uwe Kleine-König (The Capable Hub) wrote:
> The current list initialisation depends on the well hidden two zeros in
> the PCI_VDEVICE macro. Instead use a named initialisation that is more
> robust and easier to understand.
> 
> Signed-off-by: Uwe Kleine-König (The Capable Hub) <u.kleine-koenig@xxxxxxxxxxxx>

Reviewed-by: Dave Jiang <dave.jiang@xxxxxxxxx>

> ---
> Hello,
> 
> while being a cleanup that can stand on its own this is also a
> preparation for making .driver_data an anonymous union which allows to
> do:
> 
> 	diff --git a/drivers/ntb/hw/amd/ntb_hw_amd.c b/drivers/ntb/hw/amd/ntb_hw_amd.c
> 	index 55408b431365..ee77d1235ec4 100644
> 	--- a/drivers/ntb/hw/amd/ntb_hw_amd.c
> 	+++ b/drivers/ntb/hw/amd/ntb_hw_amd.c
> 	@@ -1233,7 +1233,7 @@ static int amd_ntb_pci_probe(struct pci_dev *pdev,
> 			goto err_ndev;
> 		}
> 
> 	-	ndev->dev_data = (struct ntb_dev_data *)id->driver_data;
> 	+	ndev->dev_data = id->driver_data_ptr;
> 
> 		ndev_init_struct(ndev, pdev);
> 
> 	@@ -1328,14 +1328,14 @@ static const struct ntb_dev_data dev_data[] = {
> 	 };
> 
> 	 static const struct pci_device_id amd_ntb_pci_tbl[] = {
> 	-	{ PCI_VDEVICE(AMD, 0x145b), .driver_data = (kernel_ulong_t)&dev_data[0] },
> 	-	{ PCI_VDEVICE(AMD, 0x148b), .driver_data = (kernel_ulong_t)&dev_data[1] },
> 	-	{ PCI_VDEVICE(AMD, 0x14c0), .driver_data = (kernel_ulong_t)&dev_data[1] },
> 	-	{ PCI_VDEVICE(AMD, 0x14c3), .driver_data = (kernel_ulong_t)&dev_data[1] },
> 	-	{ PCI_VDEVICE(AMD, 0x155a), .driver_data = (kernel_ulong_t)&dev_data[1] },
> 	-	{ PCI_VDEVICE(AMD, 0x17d4), .driver_data = (kernel_ulong_t)&dev_data[1] },
> 	-	{ PCI_VDEVICE(AMD, 0x17d7), .driver_data = (kernel_ulong_t)&dev_data[2] },
> 	-	{ PCI_VDEVICE(HYGON, 0x145b), .driver_data = (kernel_ulong_t)&dev_data[0] },
> 	+	{ PCI_VDEVICE(AMD, 0x145b), .driver_data_ptr = &dev_data[0] },
> 	+	{ PCI_VDEVICE(AMD, 0x148b), .driver_data_ptr = &dev_data[1] },
> 	+	{ PCI_VDEVICE(AMD, 0x14c0), .driver_data_ptr = &dev_data[1] },
> 	+	{ PCI_VDEVICE(AMD, 0x14c3), .driver_data_ptr = &dev_data[1] },
> 	+	{ PCI_VDEVICE(AMD, 0x155a), .driver_data_ptr = &dev_data[1] },
> 	+	{ PCI_VDEVICE(AMD, 0x17d4), .driver_data_ptr = &dev_data[1] },
> 	+	{ PCI_VDEVICE(AMD, 0x17d7), .driver_data_ptr = &dev_data[2] },
> 	+	{ PCI_VDEVICE(HYGON, 0x145b), .driver_data_ptr = &dev_data[0] },
> 		{ }
> 	 };
> 	 MODULE_DEVICE_TABLE(pci, amd_ntb_pci_tbl);
> 
> which is also a nice cleanup *and* further hints at also doing
> 
> 	diff --git a/drivers/ntb/hw/amd/ntb_hw_amd.h b/drivers/ntb/hw/amd/ntb_hw_amd.h
> 	index e8c3165fa38b..4ef7bafd21f7 100644
> 	--- a/drivers/ntb/hw/amd/ntb_hw_amd.h
> 	+++ b/drivers/ntb/hw/amd/ntb_hw_amd.h
> 	@@ -186,7 +186,7 @@ struct amd_ntb_dev {
> 		u32 cntl_sta;
> 		u32 peer_sta;
> 
> 	-	struct ntb_dev_data *dev_data;
> 	+	const struct ntb_dev_data *dev_data;
> 		unsigned char mw_count;
> 		unsigned char spad_count;
> 		unsigned char db_count;
> 
> due to a compiler warning. So all in all this yields easier to read
> initialisation (not shorter and more compact but easier to grasp),
> better type safety and harder to make mistakes due to the added const.
> 
> Best regards
> Uwe
> ---
>  drivers/ntb/hw/amd/ntb_hw_amd.c | 18 +++++++++---------
>  1 file changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/ntb/hw/amd/ntb_hw_amd.c b/drivers/ntb/hw/amd/ntb_hw_amd.c
> index 1a163596ddf5..55408b431365 100644
> --- a/drivers/ntb/hw/amd/ntb_hw_amd.c
> +++ b/drivers/ntb/hw/amd/ntb_hw_amd.c
> @@ -1328,15 +1328,15 @@ static const struct ntb_dev_data dev_data[] = {
>  };
>  
>  static const struct pci_device_id amd_ntb_pci_tbl[] = {
> -	{ PCI_VDEVICE(AMD, 0x145b), (kernel_ulong_t)&dev_data[0] },
> -	{ PCI_VDEVICE(AMD, 0x148b), (kernel_ulong_t)&dev_data[1] },
> -	{ PCI_VDEVICE(AMD, 0x14c0), (kernel_ulong_t)&dev_data[1] },
> -	{ PCI_VDEVICE(AMD, 0x14c3), (kernel_ulong_t)&dev_data[1] },
> -	{ PCI_VDEVICE(AMD, 0x155a), (kernel_ulong_t)&dev_data[1] },
> -	{ PCI_VDEVICE(AMD, 0x17d4), (kernel_ulong_t)&dev_data[1] },
> -	{ PCI_VDEVICE(AMD, 0x17d7), (kernel_ulong_t)&dev_data[2] },
> -	{ PCI_VDEVICE(HYGON, 0x145b), (kernel_ulong_t)&dev_data[0] },
> -	{ 0, }
> +	{ PCI_VDEVICE(AMD, 0x145b), .driver_data = (kernel_ulong_t)&dev_data[0] },
> +	{ PCI_VDEVICE(AMD, 0x148b), .driver_data = (kernel_ulong_t)&dev_data[1] },
> +	{ PCI_VDEVICE(AMD, 0x14c0), .driver_data = (kernel_ulong_t)&dev_data[1] },
> +	{ PCI_VDEVICE(AMD, 0x14c3), .driver_data = (kernel_ulong_t)&dev_data[1] },
> +	{ PCI_VDEVICE(AMD, 0x155a), .driver_data = (kernel_ulong_t)&dev_data[1] },
> +	{ PCI_VDEVICE(AMD, 0x17d4), .driver_data = (kernel_ulong_t)&dev_data[1] },
> +	{ PCI_VDEVICE(AMD, 0x17d7), .driver_data = (kernel_ulong_t)&dev_data[2] },
> +	{ PCI_VDEVICE(HYGON, 0x145b), .driver_data = (kernel_ulong_t)&dev_data[0] },
> +	{ }
>  };
>  MODULE_DEVICE_TABLE(pci, amd_ntb_pci_tbl);
>  
> 
> base-commit: 254f49634ee16a731174d2ae34bc50bd5f45e731