* [PATCH] dmaengine: Consistently define pci_device_ids using named initializers
@ 2026-05-04 10:20 Uwe Kleine-König (The Capable Hub)
2026-05-04 10:29 ` Andy Shevchenko
0 siblings, 1 reply; 7+ messages in thread
From: Uwe Kleine-König (The Capable Hub) @ 2026-05-04 10:20 UTC (permalink / raw)
To: Vinod Koul
Cc: Markus Schneider-Pargmann, Basavaraj Natikar, Frank Li,
Manivannan Sadhasivam, Viresh Kumar, Andy Shevchenko, dmaengine,
linux-kernel
The .driver_data member of the various struct pci_device_id arrays were
initialized 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 skip explicit assignments of 0 (which the compiler then
takes care of).
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>
---
Hello,
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.
The anonymous union will allow changes like the following:
- { PCI_VDEVICE(INTEL, 0x0827), .driver_data = (kernel_ulong_t)&dw_dma_chip_pdata },
+ { PCI_VDEVICE(INTEL, 0x0827), .driver_data_ptr = &dw_dma_chip_pdata },
(together with the respective change in the code when the value is
used). This gets rid of a bunch of casts and thus slightly improving
type safety.
Best regards
Uwe
---
drivers/dma/amd/ptdma/ptdma-pci.c | 4 ++--
drivers/dma/dw-edma/dw-edma-pcie.c | 2 +-
drivers/dma/dw/pci.c | 22 +++++++++++-----------
drivers/dma/hsu/pci.c | 4 ++--
drivers/dma/pch_dma.c | 26 +++++++++++++-------------
5 files changed, 29 insertions(+), 29 deletions(-)
diff --git a/drivers/dma/amd/ptdma/ptdma-pci.c b/drivers/dma/amd/ptdma/ptdma-pci.c
index 22739ff0c3c5..0b226bec950c 100644
--- a/drivers/dma/amd/ptdma/ptdma-pci.c
+++ b/drivers/dma/amd/ptdma/ptdma-pci.c
@@ -223,9 +223,9 @@ static const struct pt_dev_vdata dev_vdata[] = {
};
static const struct pci_device_id pt_pci_table[] = {
- { PCI_VDEVICE(AMD, 0x1498), (kernel_ulong_t)&dev_vdata[0] },
+ { PCI_VDEVICE(AMD, 0x1498), .driver_data = (kernel_ulong_t)&dev_vdata[0] },
/* Last entry must be zero */
- { 0, }
+ { }
};
MODULE_DEVICE_TABLE(pci, pt_pci_table);
diff --git a/drivers/dma/dw-edma/dw-edma-pcie.c b/drivers/dma/dw-edma/dw-edma-pcie.c
index 0b30ce138503..6c589d8b46e1 100644
--- a/drivers/dma/dw-edma/dw-edma-pcie.c
+++ b/drivers/dma/dw-edma/dw-edma-pcie.c
@@ -546,7 +546,7 @@ static void dw_edma_pcie_remove(struct pci_dev *pdev)
static const struct pci_device_id dw_edma_pcie_id_table[] = {
{ PCI_DEVICE_DATA(SYNOPSYS, EDDA, &snps_edda_data) },
{ PCI_VDEVICE(XILINX, PCI_DEVICE_ID_XILINX_B054),
- (kernel_ulong_t)&xilinx_mdb_data },
+ .driver_data = (kernel_ulong_t)&xilinx_mdb_data },
{ }
};
MODULE_DEVICE_TABLE(pci, dw_edma_pcie_id_table);
diff --git a/drivers/dma/dw/pci.c b/drivers/dma/dw/pci.c
index a3aae3d1c093..99565fab3565 100644
--- a/drivers/dma/dw/pci.c
+++ b/drivers/dma/dw/pci.c
@@ -98,29 +98,29 @@ static const struct dev_pm_ops dw_pci_dev_pm_ops = {
static const struct pci_device_id dw_pci_id_table[] = {
/* Medfield (GPDMA) */
- { PCI_VDEVICE(INTEL, 0x0827), (kernel_ulong_t)&dw_dma_chip_pdata },
+ { PCI_VDEVICE(INTEL, 0x0827), .driver_data = (kernel_ulong_t)&dw_dma_chip_pdata },
/* BayTrail */
- { PCI_VDEVICE(INTEL, 0x0f06), (kernel_ulong_t)&dw_dma_chip_pdata },
- { PCI_VDEVICE(INTEL, 0x0f40), (kernel_ulong_t)&dw_dma_chip_pdata },
+ { PCI_VDEVICE(INTEL, 0x0f06), .driver_data = (kernel_ulong_t)&dw_dma_chip_pdata },
+ { PCI_VDEVICE(INTEL, 0x0f40), .driver_data = (kernel_ulong_t)&dw_dma_chip_pdata },
/* Merrifield */
- { PCI_VDEVICE(INTEL, 0x11a2), (kernel_ulong_t)&idma32_chip_pdata },
+ { PCI_VDEVICE(INTEL, 0x11a2), .driver_data = (kernel_ulong_t)&idma32_chip_pdata },
/* Braswell */
- { PCI_VDEVICE(INTEL, 0x2286), (kernel_ulong_t)&dw_dma_chip_pdata },
- { PCI_VDEVICE(INTEL, 0x22c0), (kernel_ulong_t)&dw_dma_chip_pdata },
+ { PCI_VDEVICE(INTEL, 0x2286), .driver_data = (kernel_ulong_t)&dw_dma_chip_pdata },
+ { PCI_VDEVICE(INTEL, 0x22c0), .driver_data = (kernel_ulong_t)&dw_dma_chip_pdata },
/* Elkhart Lake iDMA 32-bit (PSE DMA) */
- { PCI_VDEVICE(INTEL, 0x4bb4), (kernel_ulong_t)&xbar_chip_pdata },
- { PCI_VDEVICE(INTEL, 0x4bb5), (kernel_ulong_t)&xbar_chip_pdata },
- { PCI_VDEVICE(INTEL, 0x4bb6), (kernel_ulong_t)&xbar_chip_pdata },
+ { PCI_VDEVICE(INTEL, 0x4bb4), .driver_data = (kernel_ulong_t)&xbar_chip_pdata },
+ { PCI_VDEVICE(INTEL, 0x4bb5), .driver_data = (kernel_ulong_t)&xbar_chip_pdata },
+ { PCI_VDEVICE(INTEL, 0x4bb6), .driver_data = (kernel_ulong_t)&xbar_chip_pdata },
/* Haswell */
- { PCI_VDEVICE(INTEL, 0x9c60), (kernel_ulong_t)&dw_dma_chip_pdata },
+ { PCI_VDEVICE(INTEL, 0x9c60), .driver_data = (kernel_ulong_t)&dw_dma_chip_pdata },
/* Broadwell */
- { PCI_VDEVICE(INTEL, 0x9ce0), (kernel_ulong_t)&dw_dma_chip_pdata },
+ { PCI_VDEVICE(INTEL, 0x9ce0), .driver_data = (kernel_ulong_t)&dw_dma_chip_pdata },
{ }
};
diff --git a/drivers/dma/hsu/pci.c b/drivers/dma/hsu/pci.c
index 0fcc0c0c22fc..b42c9c0887a8 100644
--- a/drivers/dma/hsu/pci.c
+++ b/drivers/dma/hsu/pci.c
@@ -116,8 +116,8 @@ static int hsu_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
}
static const struct pci_device_id hsu_pci_id_table[] = {
- { PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_MFLD_HSU_DMA), 0 },
- { PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_MRFLD_HSU_DMA), 0 },
+ { PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_MFLD_HSU_DMA) },
+ { PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_MRFLD_HSU_DMA) },
{ }
};
MODULE_DEVICE_TABLE(pci, hsu_pci_id_table);
diff --git a/drivers/dma/pch_dma.c b/drivers/dma/pch_dma.c
index e9fbfd5a3d51..0ecc10b9288d 100644
--- a/drivers/dma/pch_dma.c
+++ b/drivers/dma/pch_dma.c
@@ -956,19 +956,19 @@ static void pch_dma_remove(struct pci_dev *pdev)
#define PCI_DEVICE_ID_ML7831_DMA2_4CH 0x8815
static const struct pci_device_id pch_dma_id_table[] = {
- { PCI_VDEVICE(INTEL, PCI_DEVICE_ID_EG20T_PCH_DMA_8CH), 8 },
- { PCI_VDEVICE(INTEL, PCI_DEVICE_ID_EG20T_PCH_DMA_4CH), 4 },
- { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7213_DMA1_8CH), 8}, /* UART Video */
- { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7213_DMA2_8CH), 8}, /* PCMIF SPI */
- { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7213_DMA3_4CH), 4}, /* FPGA */
- { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7213_DMA4_12CH), 12}, /* I2S */
- { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7223_DMA1_4CH), 4}, /* UART */
- { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7223_DMA2_4CH), 4}, /* Video SPI */
- { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7223_DMA3_4CH), 4}, /* Security */
- { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7223_DMA4_4CH), 4}, /* FPGA */
- { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7831_DMA1_8CH), 8}, /* UART */
- { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7831_DMA2_4CH), 4}, /* SPI */
- { 0, },
+ { PCI_VDEVICE(INTEL, PCI_DEVICE_ID_EG20T_PCH_DMA_8CH), .driver_data = 8 },
+ { PCI_VDEVICE(INTEL, PCI_DEVICE_ID_EG20T_PCH_DMA_4CH), .driver_data = 4 },
+ { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7213_DMA1_8CH), .driver_data = 8 }, /* UART Video */
+ { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7213_DMA2_8CH), .driver_data = 8 }, /* PCMIF SPI */
+ { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7213_DMA3_4CH), .driver_data = 4 }, /* FPGA */
+ { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7213_DMA4_12CH), .driver_data = 12 }, /* I2S */
+ { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7223_DMA1_4CH), .driver_data = 4 }, /* UART */
+ { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7223_DMA2_4CH), .driver_data = 4 }, /* Video SPI */
+ { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7223_DMA3_4CH), .driver_data = 4 }, /* Security */
+ { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7223_DMA4_4CH), .driver_data = 4 }, /* FPGA */
+ { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7831_DMA1_8CH), .driver_data = 8 }, /* UART */
+ { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7831_DMA2_4CH), .driver_data = 4 }, /* SPI */
+ { },
};
static SIMPLE_DEV_PM_OPS(pch_dma_pm_ops, pch_dma_suspend, pch_dma_resume);
base-commit: 254f49634ee16a731174d2ae34bc50bd5f45e731
--
2.47.3
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] dmaengine: Consistently define pci_device_ids using named initializers
2026-05-04 10:20 [PATCH] dmaengine: Consistently define pci_device_ids using named initializers Uwe Kleine-König (The Capable Hub)
@ 2026-05-04 10:29 ` Andy Shevchenko
2026-05-04 13:55 ` Uwe Kleine-König (The Capable Hub)
0 siblings, 1 reply; 7+ messages in thread
From: Andy Shevchenko @ 2026-05-04 10:29 UTC (permalink / raw)
To: Uwe Kleine-König (The Capable Hub)
Cc: Vinod Koul, Markus Schneider-Pargmann, Basavaraj Natikar,
Frank Li, Manivannan Sadhasivam, Viresh Kumar, dmaengine,
linux-kernel
On Mon, May 04, 2026 at 12:20:06PM +0200, Uwe Kleine-König (The Capable Hub) wrote:
> The .driver_data member of the various struct pci_device_id arrays were
> initialized 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 skip explicit assignments of 0 (which the compiler then
> takes care of).
>
> This change doesn't introduce changes to the compiled pci_device_id
> arrays. Tested on x86 and arm64.
HSU driver has different change ("Also" is a strong sign to the split required).
...
> static const struct pci_device_id pch_dma_id_table[] = {
> - { PCI_VDEVICE(INTEL, PCI_DEVICE_ID_EG20T_PCH_DMA_8CH), 8 },
> - { PCI_VDEVICE(INTEL, PCI_DEVICE_ID_EG20T_PCH_DMA_4CH), 4 },
> - { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7213_DMA1_8CH), 8}, /* UART Video */
> - { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7213_DMA2_8CH), 8}, /* PCMIF SPI */
> - { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7213_DMA3_4CH), 4}, /* FPGA */
> - { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7213_DMA4_12CH), 12}, /* I2S */
> - { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7223_DMA1_4CH), 4}, /* UART */
> - { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7223_DMA2_4CH), 4}, /* Video SPI */
> - { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7223_DMA3_4CH), 4}, /* Security */
> - { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7223_DMA4_4CH), 4}, /* FPGA */
> - { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7831_DMA1_8CH), 8}, /* UART */
> - { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7831_DMA2_4CH), 4}, /* SPI */
> - { 0, },
> + { PCI_VDEVICE(INTEL, PCI_DEVICE_ID_EG20T_PCH_DMA_8CH), .driver_data = 8 },
> + { PCI_VDEVICE(INTEL, PCI_DEVICE_ID_EG20T_PCH_DMA_4CH), .driver_data = 4 },
> + { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7213_DMA1_8CH), .driver_data = 8 }, /* UART Video */
> + { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7213_DMA2_8CH), .driver_data = 8 }, /* PCMIF SPI */
> + { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7213_DMA3_4CH), .driver_data = 4 }, /* FPGA */
> + { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7213_DMA4_12CH), .driver_data = 12 }, /* I2S */
> + { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7223_DMA1_4CH), .driver_data = 4 }, /* UART */
> + { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7223_DMA2_4CH), .driver_data = 4 }, /* Video SPI */
> + { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7223_DMA3_4CH), .driver_data = 4 }, /* Security */
> + { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7223_DMA4_4CH), .driver_data = 4 }, /* FPGA */
> + { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7831_DMA1_8CH), .driver_data = 8 }, /* UART */
> + { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7831_DMA2_4CH), .driver_data = 4 }, /* SPI */
> + { },
> };
Use PCI_DEVICE_DATA() instead. Same may apply to DesignWare, but one needs to
define the device IDs. I think I may help with that.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] dmaengine: Consistently define pci_device_ids using named initializers
2026-05-04 10:29 ` Andy Shevchenko
@ 2026-05-04 13:55 ` Uwe Kleine-König (The Capable Hub)
2026-05-04 14:09 ` Andy Shevchenko
0 siblings, 1 reply; 7+ messages in thread
From: Uwe Kleine-König (The Capable Hub) @ 2026-05-04 13:55 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Vinod Koul, Markus Schneider-Pargmann, Basavaraj Natikar,
Frank Li, Manivannan Sadhasivam, Viresh Kumar, dmaengine,
linux-kernel
[-- Attachment #1: Type: text/plain, Size: 3564 bytes --]
On Mon, May 04, 2026 at 01:29:12PM +0300, Andy Shevchenko wrote:
> On Mon, May 04, 2026 at 12:20:06PM +0200, Uwe Kleine-König (The Capable Hub) wrote:
> > The .driver_data member of the various struct pci_device_id arrays were
> > initialized 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 skip explicit assignments of 0 (which the compiler then
> > takes care of).
> >
> > This change doesn't introduce changes to the compiled pci_device_id
> > arrays. Tested on x86 and arm64.
>
> HSU driver has different change ("Also" is a strong sign to the split required).
HSU is in the category "skip explicit assignments of 0", so I think
that's fine. I could be talked into splitting if that's what is wanted.
> ...
>
> > static const struct pci_device_id pch_dma_id_table[] = {
> > - { PCI_VDEVICE(INTEL, PCI_DEVICE_ID_EG20T_PCH_DMA_8CH), 8 },
> > - { PCI_VDEVICE(INTEL, PCI_DEVICE_ID_EG20T_PCH_DMA_4CH), 4 },
> > - { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7213_DMA1_8CH), 8}, /* UART Video */
> > - { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7213_DMA2_8CH), 8}, /* PCMIF SPI */
> > - { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7213_DMA3_4CH), 4}, /* FPGA */
> > - { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7213_DMA4_12CH), 12}, /* I2S */
> > - { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7223_DMA1_4CH), 4}, /* UART */
> > - { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7223_DMA2_4CH), 4}, /* Video SPI */
> > - { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7223_DMA3_4CH), 4}, /* Security */
> > - { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7223_DMA4_4CH), 4}, /* FPGA */
> > - { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7831_DMA1_8CH), 8}, /* UART */
> > - { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7831_DMA2_4CH), 4}, /* SPI */
> > - { 0, },
> > + { PCI_VDEVICE(INTEL, PCI_DEVICE_ID_EG20T_PCH_DMA_8CH), .driver_data = 8 },
> > + { PCI_VDEVICE(INTEL, PCI_DEVICE_ID_EG20T_PCH_DMA_4CH), .driver_data = 4 },
> > + { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7213_DMA1_8CH), .driver_data = 8 }, /* UART Video */
> > + { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7213_DMA2_8CH), .driver_data = 8 }, /* PCMIF SPI */
> > + { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7213_DMA3_4CH), .driver_data = 4 }, /* FPGA */
> > + { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7213_DMA4_12CH), .driver_data = 12 }, /* I2S */
> > + { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7223_DMA1_4CH), .driver_data = 4 }, /* UART */
> > + { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7223_DMA2_4CH), .driver_data = 4 }, /* Video SPI */
> > + { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7223_DMA3_4CH), .driver_data = 4 }, /* Security */
> > + { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7223_DMA4_4CH), .driver_data = 4 }, /* FPGA */
> > + { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7831_DMA1_8CH), .driver_data = 8 }, /* UART */
> > + { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7831_DMA2_4CH), .driver_data = 4 }, /* SPI */
> > + { },
> > };
>
> Use PCI_DEVICE_DATA() instead. Same may apply to DesignWare, but one needs to
> define the device IDs. I think I may help with that.
I'm not a fan of PCI_DEVICE_DATA. While it could indeed be used to
shorten the assignments here, it's less readable in my opinion.
Compare
{ PCI_VDEVICE(INTEL, PCI_DEVICE_ID_EG20T_PCH_DMA_4CH), .driver_data = 4 },
with
{ PCI_DEVICE_DATA(INTEL, PCI_DEVICE_ID_EG20T_PCH_DMA_4CH, 4) },
. For someone who doesn't know what PCI_DEVICE_DATA does, the latter is
less understandable.
Also PCI_DEVICE_DATA has a cast which is something I want to get rid of.
Best regards
Uwe
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] dmaengine: Consistently define pci_device_ids using named initializers
2026-05-04 13:55 ` Uwe Kleine-König (The Capable Hub)
@ 2026-05-04 14:09 ` Andy Shevchenko
2026-05-04 16:38 ` Uwe Kleine-König (The Capable Hub)
0 siblings, 1 reply; 7+ messages in thread
From: Andy Shevchenko @ 2026-05-04 14:09 UTC (permalink / raw)
To: Uwe Kleine-König (The Capable Hub)
Cc: Vinod Koul, Markus Schneider-Pargmann, Basavaraj Natikar,
Frank Li, Manivannan Sadhasivam, Viresh Kumar, dmaengine,
linux-kernel
On Mon, May 04, 2026 at 03:55:00PM +0200, Uwe Kleine-König (The Capable Hub) wrote:
> On Mon, May 04, 2026 at 01:29:12PM +0300, Andy Shevchenko wrote:
> > On Mon, May 04, 2026 at 12:20:06PM +0200, Uwe Kleine-König (The Capable Hub) wrote:
> > > The .driver_data member of the various struct pci_device_id arrays were
> > > initialized 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 skip explicit assignments of 0 (which the compiler then
> > > takes care of).
> > >
> > > This change doesn't introduce changes to the compiled pci_device_id
> > > arrays. Tested on x86 and arm64.
> >
> > HSU driver has different change ("Also" is a strong sign to the split required).
>
> HSU is in the category "skip explicit assignments of 0", so I think
> that's fine. I could be talked into splitting if that's what is wanted.
Yes, please. I will Rb/Ack it immediately when standalone change.
...
> > > static const struct pci_device_id pch_dma_id_table[] = {
> > > - { PCI_VDEVICE(INTEL, PCI_DEVICE_ID_EG20T_PCH_DMA_8CH), 8 },
> > > - { PCI_VDEVICE(INTEL, PCI_DEVICE_ID_EG20T_PCH_DMA_4CH), 4 },
> > > - { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7213_DMA1_8CH), 8}, /* UART Video */
> > > - { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7213_DMA2_8CH), 8}, /* PCMIF SPI */
> > > - { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7213_DMA3_4CH), 4}, /* FPGA */
> > > - { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7213_DMA4_12CH), 12}, /* I2S */
> > > - { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7223_DMA1_4CH), 4}, /* UART */
> > > - { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7223_DMA2_4CH), 4}, /* Video SPI */
> > > - { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7223_DMA3_4CH), 4}, /* Security */
> > > - { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7223_DMA4_4CH), 4}, /* FPGA */
> > > - { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7831_DMA1_8CH), 8}, /* UART */
> > > - { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7831_DMA2_4CH), 4}, /* SPI */
> > > - { 0, },
> > > + { PCI_VDEVICE(INTEL, PCI_DEVICE_ID_EG20T_PCH_DMA_8CH), .driver_data = 8 },
> > > + { PCI_VDEVICE(INTEL, PCI_DEVICE_ID_EG20T_PCH_DMA_4CH), .driver_data = 4 },
> > > + { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7213_DMA1_8CH), .driver_data = 8 }, /* UART Video */
> > > + { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7213_DMA2_8CH), .driver_data = 8 }, /* PCMIF SPI */
> > > + { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7213_DMA3_4CH), .driver_data = 4 }, /* FPGA */
> > > + { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7213_DMA4_12CH), .driver_data = 12 }, /* I2S */
> > > + { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7223_DMA1_4CH), .driver_data = 4 }, /* UART */
> > > + { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7223_DMA2_4CH), .driver_data = 4 }, /* Video SPI */
> > > + { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7223_DMA3_4CH), .driver_data = 4 }, /* Security */
> > > + { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7223_DMA4_4CH), .driver_data = 4 }, /* FPGA */
> > > + { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7831_DMA1_8CH), .driver_data = 8 }, /* UART */
> > > + { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7831_DMA2_4CH), .driver_data = 4 }, /* SPI */
> > > + { },
> > > };
> >
> > Use PCI_DEVICE_DATA() instead. Same may apply to DesignWare, but one needs to
> > define the device IDs. I think I may help with that.
>
> I'm not a fan of PCI_DEVICE_DATA. While it could indeed be used to
> shorten the assignments here, it's less readable in my opinion.
I'm not fun of these long unreadable lines with tons of repetitions :-)
> Compare
>
> { PCI_VDEVICE(INTEL, PCI_DEVICE_ID_EG20T_PCH_DMA_4CH), .driver_data = 4 },
>
> with
>
> { PCI_DEVICE_DATA(INTEL, PCI_DEVICE_ID_EG20T_PCH_DMA_4CH, 4) },
First of all, with
{ PCI_DEVICE_DATA(INTEL, EG20T_PCH_DMA_4CH, 4) },
> . For someone who doesn't know what PCI_DEVICE_DATA does, the latter is
> less understandable.
Same applicable to many other macros. I don't consider this argument viable.
> Also PCI_DEVICE_DATA has a cast which is something I want to get rid of.
Yes, and you will get rid of in one place instead of tons of them.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] dmaengine: Consistently define pci_device_ids using named initializers
2026-05-04 14:09 ` Andy Shevchenko
@ 2026-05-04 16:38 ` Uwe Kleine-König (The Capable Hub)
2026-05-05 7:01 ` Andy Shevchenko
0 siblings, 1 reply; 7+ messages in thread
From: Uwe Kleine-König (The Capable Hub) @ 2026-05-04 16:38 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Vinod Koul, Markus Schneider-Pargmann, Basavaraj Natikar,
Frank Li, Manivannan Sadhasivam, Viresh Kumar, dmaengine,
linux-kernel
[-- Attachment #1: Type: text/plain, Size: 4806 bytes --]
Hello Andy,
On Mon, May 04, 2026 at 05:09:55PM +0300, Andy Shevchenko wrote:
> On Mon, May 04, 2026 at 03:55:00PM +0200, Uwe Kleine-König (The Capable Hub) wrote:
> > On Mon, May 04, 2026 at 01:29:12PM +0300, Andy Shevchenko wrote:
> > > On Mon, May 04, 2026 at 12:20:06PM +0200, Uwe Kleine-König (The Capable Hub) wrote:
> > > > The .driver_data member of the various struct pci_device_id arrays were
> > > > initialized 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 skip explicit assignments of 0 (which the compiler then
> > > > takes care of).
> > > >
> > > > This change doesn't introduce changes to the compiled pci_device_id
> > > > arrays. Tested on x86 and arm64.
> > >
> > > HSU driver has different change ("Also" is a strong sign to the split required).
> >
> > HSU is in the category "skip explicit assignments of 0", so I think
> > that's fine. I could be talked into splitting if that's what is wanted.
>
> Yes, please. I will Rb/Ack it immediately when standalone change.
>
> ...
>
> > > > static const struct pci_device_id pch_dma_id_table[] = {
> > > > - { PCI_VDEVICE(INTEL, PCI_DEVICE_ID_EG20T_PCH_DMA_8CH), 8 },
> > > > - { PCI_VDEVICE(INTEL, PCI_DEVICE_ID_EG20T_PCH_DMA_4CH), 4 },
> > > > - { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7213_DMA1_8CH), 8}, /* UART Video */
> > > > - { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7213_DMA2_8CH), 8}, /* PCMIF SPI */
> > > > - { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7213_DMA3_4CH), 4}, /* FPGA */
> > > > - { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7213_DMA4_12CH), 12}, /* I2S */
> > > > - { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7223_DMA1_4CH), 4}, /* UART */
> > > > - { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7223_DMA2_4CH), 4}, /* Video SPI */
> > > > - { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7223_DMA3_4CH), 4}, /* Security */
> > > > - { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7223_DMA4_4CH), 4}, /* FPGA */
> > > > - { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7831_DMA1_8CH), 8}, /* UART */
> > > > - { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7831_DMA2_4CH), 4}, /* SPI */
> > > > - { 0, },
> > > > + { PCI_VDEVICE(INTEL, PCI_DEVICE_ID_EG20T_PCH_DMA_8CH), .driver_data = 8 },
> > > > + { PCI_VDEVICE(INTEL, PCI_DEVICE_ID_EG20T_PCH_DMA_4CH), .driver_data = 4 },
> > > > + { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7213_DMA1_8CH), .driver_data = 8 }, /* UART Video */
> > > > + { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7213_DMA2_8CH), .driver_data = 8 }, /* PCMIF SPI */
> > > > + { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7213_DMA3_4CH), .driver_data = 4 }, /* FPGA */
> > > > + { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7213_DMA4_12CH), .driver_data = 12 }, /* I2S */
> > > > + { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7223_DMA1_4CH), .driver_data = 4 }, /* UART */
> > > > + { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7223_DMA2_4CH), .driver_data = 4 }, /* Video SPI */
> > > > + { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7223_DMA3_4CH), .driver_data = 4 }, /* Security */
> > > > + { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7223_DMA4_4CH), .driver_data = 4 }, /* FPGA */
> > > > + { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7831_DMA1_8CH), .driver_data = 8 }, /* UART */
> > > > + { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7831_DMA2_4CH), .driver_data = 4 }, /* SPI */
> > > > + { },
> > > > };
> > >
> > > Use PCI_DEVICE_DATA() instead. Same may apply to DesignWare, but one needs to
> > > define the device IDs. I think I may help with that.
> >
> > I'm not a fan of PCI_DEVICE_DATA. While it could indeed be used to
> > shorten the assignments here, it's less readable in my opinion.
>
> I'm not fun of these long unreadable lines with tons of repetitions :-)
Seems to be subjective.
> > Compare
> >
> > { PCI_VDEVICE(INTEL, PCI_DEVICE_ID_EG20T_PCH_DMA_4CH), .driver_data = 4 },
> >
> > with
> >
> > { PCI_DEVICE_DATA(INTEL, PCI_DEVICE_ID_EG20T_PCH_DMA_4CH, 4) },
>
> First of all, with
>
> { PCI_DEVICE_DATA(INTEL, EG20T_PCH_DMA_4CH, 4) },
Agreed. That doesn't considerably weaken my reasoning however.
> > . For someone who doesn't know what PCI_DEVICE_DATA does, the latter is
> > less understandable.
>
> Same applicable to many other macros. I don't consider this argument viable.
Also agreed. But other bad macros don't justify using that (admittedly
subjectively) bad PCI_DEVICE_DATA macro that mixes device identity
(.vendor, .device, .subvendor and .subdevice) with a driver specific
struct member.
> > Also PCI_DEVICE_DATA has a cast which is something I want to get rid of.
>
> Yes, and you will get rid of in one place instead of tons of them.
This would require another (subjectively bad) macro PCI_DEVICE_DATAPTR.
I think I let someone else tackle that quest.
Best regards
Uwe
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] dmaengine: Consistently define pci_device_ids using named initializers
2026-05-04 16:38 ` Uwe Kleine-König (The Capable Hub)
@ 2026-05-05 7:01 ` Andy Shevchenko
2026-05-05 7:04 ` Andy Shevchenko
0 siblings, 1 reply; 7+ messages in thread
From: Andy Shevchenko @ 2026-05-05 7:01 UTC (permalink / raw)
To: Uwe Kleine-König (The Capable Hub)
Cc: Vinod Koul, Markus Schneider-Pargmann, Basavaraj Natikar,
Frank Li, Manivannan Sadhasivam, Viresh Kumar, dmaengine,
linux-kernel
On Mon, May 04, 2026 at 06:38:51PM +0200, Uwe Kleine-König (The Capable Hub) wrote:
> On Mon, May 04, 2026 at 05:09:55PM +0300, Andy Shevchenko wrote:
> > On Mon, May 04, 2026 at 03:55:00PM +0200, Uwe Kleine-König (The Capable Hub) wrote:
> > > On Mon, May 04, 2026 at 01:29:12PM +0300, Andy Shevchenko wrote:
> > > > On Mon, May 04, 2026 at 12:20:06PM +0200, Uwe Kleine-König (The Capable Hub) wrote:
...
> > > > > static const struct pci_device_id pch_dma_id_table[] = {
> > > > > - { PCI_VDEVICE(INTEL, PCI_DEVICE_ID_EG20T_PCH_DMA_8CH), 8 },
> > > > > - { PCI_VDEVICE(INTEL, PCI_DEVICE_ID_EG20T_PCH_DMA_4CH), 4 },
> > > > > - { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7213_DMA1_8CH), 8}, /* UART Video */
> > > > > - { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7213_DMA2_8CH), 8}, /* PCMIF SPI */
> > > > > - { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7213_DMA3_4CH), 4}, /* FPGA */
> > > > > - { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7213_DMA4_12CH), 12}, /* I2S */
> > > > > - { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7223_DMA1_4CH), 4}, /* UART */
> > > > > - { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7223_DMA2_4CH), 4}, /* Video SPI */
> > > > > - { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7223_DMA3_4CH), 4}, /* Security */
> > > > > - { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7223_DMA4_4CH), 4}, /* FPGA */
> > > > > - { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7831_DMA1_8CH), 8}, /* UART */
> > > > > - { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7831_DMA2_4CH), 4}, /* SPI */
> > > > > - { 0, },
> > > > > + { PCI_VDEVICE(INTEL, PCI_DEVICE_ID_EG20T_PCH_DMA_8CH), .driver_data = 8 },
> > > > > + { PCI_VDEVICE(INTEL, PCI_DEVICE_ID_EG20T_PCH_DMA_4CH), .driver_data = 4 },
> > > > > + { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7213_DMA1_8CH), .driver_data = 8 }, /* UART Video */
> > > > > + { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7213_DMA2_8CH), .driver_data = 8 }, /* PCMIF SPI */
> > > > > + { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7213_DMA3_4CH), .driver_data = 4 }, /* FPGA */
> > > > > + { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7213_DMA4_12CH), .driver_data = 12 }, /* I2S */
> > > > > + { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7223_DMA1_4CH), .driver_data = 4 }, /* UART */
> > > > > + { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7223_DMA2_4CH), .driver_data = 4 }, /* Video SPI */
> > > > > + { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7223_DMA3_4CH), .driver_data = 4 }, /* Security */
> > > > > + { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7223_DMA4_4CH), .driver_data = 4 }, /* FPGA */
> > > > > + { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7831_DMA1_8CH), .driver_data = 8 }, /* UART */
> > > > > + { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7831_DMA2_4CH), .driver_data = 4 }, /* SPI */
> > > > > + { },
> > > > > };
> > > >
> > > > Use PCI_DEVICE_DATA() instead. Same may apply to DesignWare, but one needs to
> > > > define the device IDs. I think I may help with that.
> > >
> > > I'm not a fan of PCI_DEVICE_DATA. While it could indeed be used to
> > > shorten the assignments here, it's less readable in my opinion.
> >
> > I'm not fun of these long unreadable lines with tons of repetitions :-)
>
> Seems to be subjective.
>
> > > Compare
> > >
> > > { PCI_VDEVICE(INTEL, PCI_DEVICE_ID_EG20T_PCH_DMA_4CH), .driver_data = 4 },
> > >
> > > with
> > >
> > > { PCI_DEVICE_DATA(INTEL, PCI_DEVICE_ID_EG20T_PCH_DMA_4CH, 4) },
> >
> > First of all, with
> >
> > { PCI_DEVICE_DATA(INTEL, EG20T_PCH_DMA_4CH, 4) },
>
> Agreed. That doesn't considerably weaken my reasoning however.
>
> > > . For someone who doesn't know what PCI_DEVICE_DATA does, the latter is
> > > less understandable.
> >
> > Same applicable to many other macros. I don't consider this argument viable.
>
> Also agreed. But other bad macros don't justify using that (admittedly
> subjectively) bad PCI_DEVICE_DATA macro that mixes device identity
> (.vendor, .device, .subvendor and .subdevice) with a driver specific
> struct member.
>
> > > Also PCI_DEVICE_DATA has a cast which is something I want to get rid of.
> >
> > Yes, and you will get rid of in one place instead of tons of them.
>
> This would require another (subjectively bad) macro PCI_DEVICE_DATAPTR.
> I think I let someone else tackle that quest.
No, it wouldn't. Since we support C11, we have _Generic(). It may be used.
And please use PCI_DEVICE_DATA() in this driver.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] dmaengine: Consistently define pci_device_ids using named initializers
2026-05-05 7:01 ` Andy Shevchenko
@ 2026-05-05 7:04 ` Andy Shevchenko
0 siblings, 0 replies; 7+ messages in thread
From: Andy Shevchenko @ 2026-05-05 7:04 UTC (permalink / raw)
To: Uwe Kleine-König (The Capable Hub)
Cc: Vinod Koul, Markus Schneider-Pargmann, Basavaraj Natikar,
Frank Li, Manivannan Sadhasivam, Viresh Kumar, dmaengine,
linux-kernel
On Tue, May 05, 2026 at 10:01:35AM +0300, Andy Shevchenko wrote:
> On Mon, May 04, 2026 at 06:38:51PM +0200, Uwe Kleine-König (The Capable Hub) wrote:
> > On Mon, May 04, 2026 at 05:09:55PM +0300, Andy Shevchenko wrote:
> > > On Mon, May 04, 2026 at 03:55:00PM +0200, Uwe Kleine-König (The Capable Hub) wrote:
> > > > On Mon, May 04, 2026 at 01:29:12PM +0300, Andy Shevchenko wrote:
> > > > > On Mon, May 04, 2026 at 12:20:06PM +0200, Uwe Kleine-König (The Capable Hub) wrote:
...
> > > > > > static const struct pci_device_id pch_dma_id_table[] = {
> > > > > > - { PCI_VDEVICE(INTEL, PCI_DEVICE_ID_EG20T_PCH_DMA_8CH), 8 },
> > > > > > - { PCI_VDEVICE(INTEL, PCI_DEVICE_ID_EG20T_PCH_DMA_4CH), 4 },
> > > > > > - { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7213_DMA1_8CH), 8}, /* UART Video */
> > > > > > - { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7213_DMA2_8CH), 8}, /* PCMIF SPI */
> > > > > > - { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7213_DMA3_4CH), 4}, /* FPGA */
> > > > > > - { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7213_DMA4_12CH), 12}, /* I2S */
> > > > > > - { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7223_DMA1_4CH), 4}, /* UART */
> > > > > > - { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7223_DMA2_4CH), 4}, /* Video SPI */
> > > > > > - { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7223_DMA3_4CH), 4}, /* Security */
> > > > > > - { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7223_DMA4_4CH), 4}, /* FPGA */
> > > > > > - { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7831_DMA1_8CH), 8}, /* UART */
> > > > > > - { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7831_DMA2_4CH), 4}, /* SPI */
> > > > > > - { 0, },
> > > > > > + { PCI_VDEVICE(INTEL, PCI_DEVICE_ID_EG20T_PCH_DMA_8CH), .driver_data = 8 },
> > > > > > + { PCI_VDEVICE(INTEL, PCI_DEVICE_ID_EG20T_PCH_DMA_4CH), .driver_data = 4 },
> > > > > > + { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7213_DMA1_8CH), .driver_data = 8 }, /* UART Video */
> > > > > > + { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7213_DMA2_8CH), .driver_data = 8 }, /* PCMIF SPI */
> > > > > > + { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7213_DMA3_4CH), .driver_data = 4 }, /* FPGA */
> > > > > > + { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7213_DMA4_12CH), .driver_data = 12 }, /* I2S */
> > > > > > + { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7223_DMA1_4CH), .driver_data = 4 }, /* UART */
> > > > > > + { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7223_DMA2_4CH), .driver_data = 4 }, /* Video SPI */
> > > > > > + { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7223_DMA3_4CH), .driver_data = 4 }, /* Security */
> > > > > > + { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7223_DMA4_4CH), .driver_data = 4 }, /* FPGA */
> > > > > > + { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7831_DMA1_8CH), .driver_data = 8 }, /* UART */
> > > > > > + { PCI_VDEVICE(ROHM, PCI_DEVICE_ID_ML7831_DMA2_4CH), .driver_data = 4 }, /* SPI */
> > > > > > + { },
> > > > > > };
> > > > >
> > > > > Use PCI_DEVICE_DATA() instead. Same may apply to DesignWare, but one needs to
> > > > > define the device IDs. I think I may help with that.
> > > >
> > > > I'm not a fan of PCI_DEVICE_DATA. While it could indeed be used to
> > > > shorten the assignments here, it's less readable in my opinion.
> > >
> > > I'm not fun of these long unreadable lines with tons of repetitions :-)
> >
> > Seems to be subjective.
> >
> > > > Compare
> > > >
> > > > { PCI_VDEVICE(INTEL, PCI_DEVICE_ID_EG20T_PCH_DMA_4CH), .driver_data = 4 },
> > > >
> > > > with
> > > >
> > > > { PCI_DEVICE_DATA(INTEL, PCI_DEVICE_ID_EG20T_PCH_DMA_4CH, 4) },
> > >
> > > First of all, with
> > >
> > > { PCI_DEVICE_DATA(INTEL, EG20T_PCH_DMA_4CH, 4) },
> >
> > Agreed. That doesn't considerably weaken my reasoning however.
> >
> > > > . For someone who doesn't know what PCI_DEVICE_DATA does, the latter is
> > > > less understandable.
> > >
> > > Same applicable to many other macros. I don't consider this argument viable.
> >
> > Also agreed. But other bad macros don't justify using that (admittedly
> > subjectively) bad PCI_DEVICE_DATA macro that mixes device identity
> > (.vendor, .device, .subvendor and .subdevice) with a driver specific
> > struct member.
> >
> > > > Also PCI_DEVICE_DATA has a cast which is something I want to get rid of.
> > >
> > > Yes, and you will get rid of in one place instead of tons of them.
> >
> > This would require another (subjectively bad) macro PCI_DEVICE_DATAPTR.
> > I think I let someone else tackle that quest.
>
> No, it wouldn't. Since we support C11, we have _Generic(). It may be used.
The example you probably want to look at is
d7cdbbc93c56 ("software node: allow referencing firmware nodes")
(yes, it's not a union there, but I think we can manage unions as well).
> And please use PCI_DEVICE_DATA() in this driver.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2026-05-05 7:04 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-04 10:20 [PATCH] dmaengine: Consistently define pci_device_ids using named initializers Uwe Kleine-König (The Capable Hub)
2026-05-04 10:29 ` Andy Shevchenko
2026-05-04 13:55 ` Uwe Kleine-König (The Capable Hub)
2026-05-04 14:09 ` Andy Shevchenko
2026-05-04 16:38 ` Uwe Kleine-König (The Capable Hub)
2026-05-05 7:01 ` Andy Shevchenko
2026-05-05 7:04 ` Andy Shevchenko
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox