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

[Frank Li]

On Fri, May 22, 2026 at 11:38:35PM +0900, Koichiro Den wrote:
> 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.

Yes, or direct set chip->cfg_non_ll at parse_caps().

Frank

>
> >
> > > +	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
> > >