Re: [PATCH 05/12] dmaengine: dw-edma-pcie: Add capability match data

[Koichiro Den]

On Thu, May 21, 2026 at 12:06:48PM -0400, Frank Li wrote:
> On Thu, May 21, 2026 at 03:31:08PM +0900, Koichiro Den wrote:
> > Move device-specific capability parsing behind per-device match data.
> >
> > The existing probe path mixes two decisions: which static template a PCI
> > ID uses, and which device-specific capability parser adjusts that
> > template. Split those decisions so device-specific discovery can be
> > added through match data instead of adding more vendor checks to
> > dw_edma_pcie_probe().
> >
> > No functional change is intended for the existing Synopsys EDDA and
> > AMD/Xilinx MDB matches. They still copy the same static template data and
> > run the same capability parsing logic before BAR mapping. The MDB entry
> > also keeps using endpoint memory physical addresses for descriptor
> > windows through a new match-data flag.
> >
> > Signed-off-by: Koichiro Den <den@xxxxxxxxxxxxx>
> > ---
> >  drivers/dma/dw-edma/dw-edma-pcie.c | 127 +++++++++++++++++++----------
> >  1 file changed, 85 insertions(+), 42 deletions(-)
> >
> > diff --git a/drivers/dma/dw-edma/dw-edma-pcie.c b/drivers/dma/dw-edma/dw-edma-pcie.c
> > index 0b30ce138503..043a7f73bf79 100644
> > --- a/drivers/dma/dw-edma/dw-edma-pcie.c
> > +++ b/drivers/dma/dw-edma/dw-edma-pcie.c
> > @@ -74,6 +74,19 @@ struct dw_edma_pcie_data {
> >  	u64				devmem_phys_off;
> >  };
> >
> > +struct dw_edma_pcie_match_data {
> > +	const struct dw_edma_pcie_data *data;
> > +	/*
> > +	 * Mandatory callback. It may leave @pdata unchanged when the static
> > +	 * template already describes the device.
> > +	 */
> > +	int (*parse_caps)(struct pci_dev *pdev,
> > +			  struct dw_edma_pcie_data *pdata, bool *non_ll);
> 
> Needn't non_ll here. This information should be already save into
> dw_edma_chip::cfg_no_ll

The non_ll argument is used only to fill dw_edma_chip::cfg_non_ll later.

Do you mean that parse_caps() should not have a separate non_ll output
parameter, and that this should instead be kept in e.g. dw_edma_pcie_data?
That would make probe do:

  chip->cfg_non_ll = dma_data->cfg_non_ll;

and drop the local non_ll variable in dw_edma_pcie_probe().
If so, yes, I agree that would make the code a bit cleaner.

> 
> > +	unsigned long flags;
> > +};
> ...
> >
> > +static const struct dw_edma_pcie_match_data snps_edda_match_data = {
> > +	.data = &snps_edda_data,
> > +	.parse_caps = dw_edma_pcie_parse_synopsys_caps,
> > +};
> > +
> > +static const struct dw_edma_pcie_match_data xilinx_mdb_match_data = {
> > +	.data = &xilinx_mdb_data,
> > +	.parse_caps = dw_edma_pcie_parse_xilinx_caps,
> > +	.flags = DW_EDMA_PCIE_F_DEVMEM_PHYS_OFF,
> > +};
> > +
> >  static const struct pci_device_id dw_edma_pcie_id_table[] = {
> > -	{ PCI_DEVICE_DATA(SYNOPSYS, EDDA, &snps_edda_data) },
> > +	{ PCI_DEVICE_DATA(SYNOPSYS, EDDA, &snps_edda_match_data) },
> >  	{ PCI_VDEVICE(XILINX, PCI_DEVICE_ID_XILINX_B054),
> > -	  (kernel_ulong_t)&xilinx_mdb_data },
> > +	  (kernel_ulong_t)&xilinx_mdb_match_data },
> 
> On going thread
> https://lore.kernel.org/linux-i3c/afmEo54iWgk54M3Y@monoceros/
> 
> .driver_data = (kernel_ulong_t)&xilinx_mdb_data;

Thanks for the pointer, I wasn't aware of that work. I'll use a named
initializer here.

Best regards,
Koichiro

> 
> >  	{ }
> >  };
> >  MODULE_DEVICE_TABLE(pci, dw_edma_pcie_id_table);
> > --
> > 2.51.0
> >