1. Patchew
  2. linux
  3. View series

[PATCH] serial: jsm: Use named initializers for struct pci_device_id

Uwe Kleine-König (The Capable Hub) posted 1 patch 1 month, 1 week ago 
drivers/tty/serial/jsm/jsm_driver.c | 75 +++++++++++++++++++++--------
1 file changed, 56 insertions(+), 19 deletions(-)
[PATCH] serial: jsm: Use named initializers for struct pci_device_id
Posted by Uwe Kleine-König (The Capable Hub) 1 month, 1 week ago 
Initializing structures using list initializers is harder to read than
using named initializers. Seeing the member name is more ideomatic and
easier to understand.

Use named initializers for the driver's pci_device_id array. While at it
also drop an explicit zero in the terminating array entry.

There are no changes to the compiled result of the array; verified with
builds for x86 and arm64.

Signed-off-by: Uwe Kleine-König (The Capable Hub) <u.kleine-koenig@baylibre.com>
---
Hello,

this is a preparing change for making struct pci_device_id::driver_data an
anonymous union (similar to
https://lore.kernel.org/all/cover.1776579304.git.u.kleine-koenig@baylibre.com/).
This requires named initializers for .driver_data. But even without that
this is a nice cleanup making the array better readable.

Having said that I didn't find where .driver_data is used, so unless I
missed something the assignments can just go away???

For earlier patches of this type against other drivers I got the
feedback to use PCI_DEVICE_DATA() but I don't like that as it mixes
hardware properties (.vendor and .device) with driver specific software
properties (.driver_data) and thus I prefer the explicit listing
(involving more repetition) over the shorter variant (being more opaque
and harder to read).

Best regards
Uwe

 drivers/tty/serial/jsm/jsm_driver.c | 75 +++++++++++++++++++++--------
 1 file changed, 56 insertions(+), 19 deletions(-)

diff --git a/drivers/tty/serial/jsm/jsm_driver.c b/drivers/tty/serial/jsm/jsm_driver.c
index 4b73e51f83fb..422cac8d8d71 100644
--- a/drivers/tty/serial/jsm/jsm_driver.c
+++ b/drivers/tty/serial/jsm/jsm_driver.c
@@ -296,25 +296,62 @@ static void jsm_remove_one(struct pci_dev *pdev)
 }
 
 static const struct pci_device_id jsm_pci_tbl[] = {
-	{ PCI_DEVICE(PCI_VENDOR_ID_DIGI, PCI_DEVICE_ID_NEO_2DB9), 0, 0, 0 },
-	{ PCI_DEVICE(PCI_VENDOR_ID_DIGI, PCI_DEVICE_ID_NEO_2DB9PRI), 0, 0, 1 },
-	{ PCI_DEVICE(PCI_VENDOR_ID_DIGI, PCI_DEVICE_ID_NEO_2RJ45), 0, 0, 2 },
-	{ PCI_DEVICE(PCI_VENDOR_ID_DIGI, PCI_DEVICE_ID_NEO_2RJ45PRI), 0, 0, 3 },
-	{ PCI_DEVICE(PCI_VENDOR_ID_DIGI, PCIE_DEVICE_ID_NEO_4_IBM), 0, 0, 4 },
-	{ PCI_DEVICE(PCI_VENDOR_ID_DIGI, PCI_DEVICE_ID_DIGI_NEO_8), 0, 0, 5 },
-	{ PCI_DEVICE(PCI_VENDOR_ID_DIGI, PCI_DEVICE_ID_NEO_4), 0, 0, 6 },
-	{ PCI_DEVICE(PCI_VENDOR_ID_DIGI, PCI_DEVICE_ID_NEO_1_422), 0, 0, 7 },
-	{ PCI_DEVICE(PCI_VENDOR_ID_DIGI, PCI_DEVICE_ID_NEO_1_422_485), 0, 0, 8 },
-	{ PCI_DEVICE(PCI_VENDOR_ID_DIGI, PCI_DEVICE_ID_NEO_2_422_485), 0, 0, 9 },
-	{ PCI_DEVICE(PCI_VENDOR_ID_DIGI, PCIE_DEVICE_ID_NEO_8), 0, 0, 10 },
-	{ PCI_DEVICE(PCI_VENDOR_ID_DIGI, PCIE_DEVICE_ID_NEO_4), 0, 0, 11 },
-	{ PCI_DEVICE(PCI_VENDOR_ID_DIGI, PCIE_DEVICE_ID_NEO_4RJ45), 0, 0, 12 },
-	{ PCI_DEVICE(PCI_VENDOR_ID_DIGI, PCIE_DEVICE_ID_NEO_8RJ45), 0, 0, 13 },
-	{ PCI_DEVICE(PCI_VENDOR_ID_DIGI, PCI_DEVICE_ID_CLASSIC_4), 0, 0, 14 },
-	{ PCI_DEVICE(PCI_VENDOR_ID_DIGI, PCI_DEVICE_ID_CLASSIC_4_422), 0, 0, 15 },
-	{ PCI_DEVICE(PCI_VENDOR_ID_DIGI, PCI_DEVICE_ID_CLASSIC_8), 0, 0, 16 },
-	{ PCI_DEVICE(PCI_VENDOR_ID_DIGI, PCI_DEVICE_ID_CLASSIC_8_422), 0, 0, 17 },
-	{ 0, }
+	{
+		PCI_DEVICE(PCI_VENDOR_ID_DIGI, PCI_DEVICE_ID_NEO_2DB9),
+		.driver_data = 0,
+	}, {
+		PCI_DEVICE(PCI_VENDOR_ID_DIGI, PCI_DEVICE_ID_NEO_2DB9PRI),
+		.driver_data = 1,
+	}, {
+		PCI_DEVICE(PCI_VENDOR_ID_DIGI, PCI_DEVICE_ID_NEO_2RJ45),
+		.driver_data = 2,
+	}, {
+		PCI_DEVICE(PCI_VENDOR_ID_DIGI, PCI_DEVICE_ID_NEO_2RJ45PRI),
+		.driver_data = 3,
+	}, {
+		PCI_DEVICE(PCI_VENDOR_ID_DIGI, PCIE_DEVICE_ID_NEO_4_IBM),
+		.driver_data = 4,
+	}, {
+		PCI_DEVICE(PCI_VENDOR_ID_DIGI, PCI_DEVICE_ID_DIGI_NEO_8),
+		.driver_data = 5,
+	}, {
+		PCI_DEVICE(PCI_VENDOR_ID_DIGI, PCI_DEVICE_ID_NEO_4),
+		.driver_data = 6,
+	}, {
+		PCI_DEVICE(PCI_VENDOR_ID_DIGI, PCI_DEVICE_ID_NEO_1_422),
+		.driver_data = 7,
+	}, {
+		PCI_DEVICE(PCI_VENDOR_ID_DIGI, PCI_DEVICE_ID_NEO_1_422_485),
+		.driver_data = 8,
+	}, {
+		PCI_DEVICE(PCI_VENDOR_ID_DIGI, PCI_DEVICE_ID_NEO_2_422_485),
+		.driver_data = 9,
+	}, {
+		PCI_DEVICE(PCI_VENDOR_ID_DIGI, PCIE_DEVICE_ID_NEO_8),
+		.driver_data = 10,
+	}, {
+		PCI_DEVICE(PCI_VENDOR_ID_DIGI, PCIE_DEVICE_ID_NEO_4),
+		.driver_data = 11,
+	}, {
+		PCI_DEVICE(PCI_VENDOR_ID_DIGI, PCIE_DEVICE_ID_NEO_4RJ45),
+		.driver_data = 12,
+	}, {
+		PCI_DEVICE(PCI_VENDOR_ID_DIGI, PCIE_DEVICE_ID_NEO_8RJ45),
+		.driver_data = 13,
+	}, {
+		PCI_DEVICE(PCI_VENDOR_ID_DIGI, PCI_DEVICE_ID_CLASSIC_4),
+		.driver_data = 14,
+	}, {
+		PCI_DEVICE(PCI_VENDOR_ID_DIGI, PCI_DEVICE_ID_CLASSIC_4_422),
+		.driver_data = 15,
+	}, {
+		PCI_DEVICE(PCI_VENDOR_ID_DIGI, PCI_DEVICE_ID_CLASSIC_8),
+		.driver_data = 16,
+	}, {
+		PCI_DEVICE(PCI_VENDOR_ID_DIGI, PCI_DEVICE_ID_CLASSIC_8_422),
+		.driver_data = 17,
+	},
+	{ }
 };
 MODULE_DEVICE_TABLE(pci, jsm_pci_tbl);
 

base-commit: 254f49634ee16a731174d2ae34bc50bd5f45e731
-- 
2.47.3
Re: [PATCH] serial: jsm: Use named initializers for struct pci_device_id
Posted by Jiri Slaby 1 month, 1 week ago 
On 05. 05. 26, 9:30, Uwe Kleine-König (The Capable Hub) wrote:
> Initializing structures using list initializers is harder to read than
> using named initializers. Seeing the member name is more ideomatic and
> easier to understand.
> 
> Use named initializers for the driver's pci_device_id array. While at it
> also drop an explicit zero in the terminating array entry.
> 
> There are no changes to the compiled result of the array; verified with
> builds for x86 and arm64.
> 
> Signed-off-by: Uwe Kleine-König (The Capable Hub) <u.kleine-koenig@baylibre.com>
> ---
> Hello,
> 
> this is a preparing change for making struct pci_device_id::driver_data an
> anonymous union (similar to
> https://lore.kernel.org/all/cover.1776579304.git.u.kleine-koenig@baylibre.com/).
> This requires named initializers for .driver_data. But even without that
> this is a nice cleanup making the array better readable.
> 
> Having said that I didn't find where .driver_data is used, so unless I
> missed something the assignments can just go away???
> 
> For earlier patches of this type against other drivers I got the
> feedback to use PCI_DEVICE_DATA() but I don't like that as it mixes
> hardware properties (.vendor and .device) with driver specific software
> properties (.driver_data) and thus I prefer the explicit listing
> (involving more repetition) over the shorter variant (being more opaque
> and harder to read).
> 
> Best regards
> Uwe
> 
>   drivers/tty/serial/jsm/jsm_driver.c | 75 +++++++++++++++++++++--------
>   1 file changed, 56 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/tty/serial/jsm/jsm_driver.c b/drivers/tty/serial/jsm/jsm_driver.c
> index 4b73e51f83fb..422cac8d8d71 100644
> --- a/drivers/tty/serial/jsm/jsm_driver.c
> +++ b/drivers/tty/serial/jsm/jsm_driver.c
> @@ -296,25 +296,62 @@ static void jsm_remove_one(struct pci_dev *pdev)
>   }
>   
>   static const struct pci_device_id jsm_pci_tbl[] = {
> -	{ PCI_DEVICE(PCI_VENDOR_ID_DIGI, PCI_DEVICE_ID_NEO_2DB9), 0, 0, 0 },
> -	{ PCI_DEVICE(PCI_VENDOR_ID_DIGI, PCI_DEVICE_ID_NEO_2DB9PRI), 0, 0, 1 },
> -	{ PCI_DEVICE(PCI_VENDOR_ID_DIGI, PCI_DEVICE_ID_NEO_2RJ45), 0, 0, 2 },
> -	{ PCI_DEVICE(PCI_VENDOR_ID_DIGI, PCI_DEVICE_ID_NEO_2RJ45PRI), 0, 0, 3 },
> -	{ PCI_DEVICE(PCI_VENDOR_ID_DIGI, PCIE_DEVICE_ID_NEO_4_IBM), 0, 0, 4 },
> -	{ PCI_DEVICE(PCI_VENDOR_ID_DIGI, PCI_DEVICE_ID_DIGI_NEO_8), 0, 0, 5 },
> -	{ PCI_DEVICE(PCI_VENDOR_ID_DIGI, PCI_DEVICE_ID_NEO_4), 0, 0, 6 },
> -	{ PCI_DEVICE(PCI_VENDOR_ID_DIGI, PCI_DEVICE_ID_NEO_1_422), 0, 0, 7 },
> -	{ PCI_DEVICE(PCI_VENDOR_ID_DIGI, PCI_DEVICE_ID_NEO_1_422_485), 0, 0, 8 },
> -	{ PCI_DEVICE(PCI_VENDOR_ID_DIGI, PCI_DEVICE_ID_NEO_2_422_485), 0, 0, 9 },
> -	{ PCI_DEVICE(PCI_VENDOR_ID_DIGI, PCIE_DEVICE_ID_NEO_8), 0, 0, 10 },
> -	{ PCI_DEVICE(PCI_VENDOR_ID_DIGI, PCIE_DEVICE_ID_NEO_4), 0, 0, 11 },
> -	{ PCI_DEVICE(PCI_VENDOR_ID_DIGI, PCIE_DEVICE_ID_NEO_4RJ45), 0, 0, 12 },
> -	{ PCI_DEVICE(PCI_VENDOR_ID_DIGI, PCIE_DEVICE_ID_NEO_8RJ45), 0, 0, 13 },
> -	{ PCI_DEVICE(PCI_VENDOR_ID_DIGI, PCI_DEVICE_ID_CLASSIC_4), 0, 0, 14 },
> -	{ PCI_DEVICE(PCI_VENDOR_ID_DIGI, PCI_DEVICE_ID_CLASSIC_4_422), 0, 0, 15 },
> -	{ PCI_DEVICE(PCI_VENDOR_ID_DIGI, PCI_DEVICE_ID_CLASSIC_8), 0, 0, 16 },
> -	{ PCI_DEVICE(PCI_VENDOR_ID_DIGI, PCI_DEVICE_ID_CLASSIC_8_422), 0, 0, 17 },
> -	{ 0, }
> +	{
> +		PCI_DEVICE(PCI_VENDOR_ID_DIGI, PCI_DEVICE_ID_NEO_2DB9),
> +		.driver_data = 0,
> +	}, {

Sorry, but this is definitely NOT "making the array better readable". 
Esp. due to the split to multiple lines. PCI_DEVICE_DATA() would be far 
better IMO. Or PCI_VDEVICE() + .driver_data on a single line...

Anyway, you're right, pci_device_id is not currently used at all. 
Instead, there are big switches in the probe doing the casing. This 
should be rewritten to actually use driver_data and drop the whole 
switch-case stuff from probe.

So you can likely drop driver_data completely for now. It won't/can't be 
used in the current form anyway.

thanks,
-- 
js
suse labs

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.