* [PATCH] serial: jsm: Use named initializers for struct pci_device_id
@ 2026-05-05 7:30 Uwe Kleine-König (The Capable Hub)
2026-05-05 7:46 ` Jiri Slaby
0 siblings, 1 reply; 2+ messages in thread
From: Uwe Kleine-König (The Capable Hub) @ 2026-05-05 7:30 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby, Lukas Wunner, Dave Jiang,
Giovanni Cabiddu, Kees Cook
Cc: linux-serial, linux-kernel, Markus Schneider-Pargmann
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
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] serial: jsm: Use named initializers for struct pci_device_id
2026-05-05 7:30 [PATCH] serial: jsm: Use named initializers for struct pci_device_id Uwe Kleine-König (The Capable Hub)
@ 2026-05-05 7:46 ` Jiri Slaby
0 siblings, 0 replies; 2+ messages in thread
From: Jiri Slaby @ 2026-05-05 7:46 UTC (permalink / raw)
To: Uwe Kleine-König (The Capable Hub), Greg Kroah-Hartman,
Lukas Wunner, Dave Jiang, Giovanni Cabiddu, Kees Cook
Cc: linux-serial, linux-kernel, Markus Schneider-Pargmann
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
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2026-05-05 7:46 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-05 7:30 [PATCH] serial: jsm: Use named initializers for struct pci_device_id Uwe Kleine-König (The Capable Hub)
2026-05-05 7:46 ` Jiri Slaby
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox