Re: [PATCH v2 06/12] dmaengine: dw-edma-pcie: Rename vsec_data to dma_data

[Koichiro Den]

On Thu, Jun 04, 2026 at 04:35:47PM -0400, Frank Li wrote:
> On Mon, May 25, 2026 at 03:24:14PM +0900, Koichiro Den wrote:
> > dw_edma_pcie_probe() now obtains DMA layout data through device-specific
> > capability callbacks, not only from PCIe Vendor-Specific Extended
> > Capabilities. Rename the local data copy from vsec_data to dma_data
> > before adding endpoint DMA BAR metadata discovery, which does not rely
> > on VSEC.
> >
> > No functional change intended.
> >
> > Signed-off-by: Koichiro Den <den@xxxxxxxxxxxxx>
> > ---
> > Changes in v2:
> >   - Fix the commit title as Frank pointed out.
> >
> >  drivers/dma/dw-edma/dw-edma-pcie.c | 76 +++++++++++++++---------------
> >  1 file changed, 37 insertions(+), 39 deletions(-)
> >
> > diff --git a/drivers/dma/dw-edma/dw-edma-pcie.c b/drivers/dma/dw-edma/dw-edma-pcie.c
> > index 5a6f5af358d0..c7362f1bf80c 100644
> > --- a/drivers/dma/dw-edma/dw-edma-pcie.c
> > +++ b/drivers/dma/dw-edma/dw-edma-pcie.c
> > @@ -369,11 +369,6 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
> >  	int err, nr_irqs;
> >  	int i, mask;
> >
> > -	struct dw_edma_pcie_data *vsec_data __free(kfree) =
> > -		kmalloc_obj(*vsec_data);
> > -	if (!vsec_data)
> > -		return -ENOMEM;
> > -
> >  	/* Enable PCI device */
> >  	err = pcim_enable_device(pdev);
> >  	if (err) {
> > @@ -381,25 +376,28 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
> >  		return err;
> >  	}
> >
> > -	memcpy(vsec_data, pdata, sizeof(struct dw_edma_pcie_data));
> > +	struct dw_edma_pcie_data *dma_data __free(kfree) =
> > +		kmemdup(pdata, sizeof(*dma_data), GFP_KERNEL);
> > +	if (!dma_data)
> > +		return -ENOMEM;
> >
> 
> This is straigh forward patch, you move this block after pcim_enable_device();
> I suggest keep original place for easily review.
> 
> Reviewed-by: Frank Li <Frank.Li@xxxxxxx>

Agreed. (I placed it there to keep the copy close to the old memcpy point.)
I will move it back.

Thanks for the review,
Koichiro

> 
> 
> >  	/* Let device-specific discovery override the static template data. */
> >  	if (!match->parse_caps)
> >  		return -EINVAL;
> >
> > -	err = match->parse_caps(pdev, vsec_data);
> > +	err = match->parse_caps(pdev, dma_data);
> >  	if (err)
> >  		return err;
> >
> >  	/* Mapping PCI BAR regions */
> > -	mask = BIT(vsec_data->rg.bar);
> > -	for (i = 0; i < vsec_data->wr_ch_cnt; i++) {
> > -		mask |= BIT(vsec_data->ll_wr[i].bar);
> > -		mask |= BIT(vsec_data->dt_wr[i].bar);
> > +	mask = BIT(dma_data->rg.bar);
> > +	for (i = 0; i < dma_data->wr_ch_cnt; i++) {
> > +		mask |= BIT(dma_data->ll_wr[i].bar);
> > +		mask |= BIT(dma_data->dt_wr[i].bar);
> >  	}
> > -	for (i = 0; i < vsec_data->rd_ch_cnt; i++) {
> > -		mask |= BIT(vsec_data->ll_rd[i].bar);
> > -		mask |= BIT(vsec_data->dt_rd[i].bar);
> > +	for (i = 0; i < dma_data->rd_ch_cnt; i++) {
> > +		mask |= BIT(dma_data->ll_rd[i].bar);
> > +		mask |= BIT(dma_data->dt_rd[i].bar);
> >  	}
> >  	err = pcim_iomap_regions(pdev, mask, pci_name(pdev));
> >  	if (err) {
> > @@ -422,7 +420,7 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
> >  		return -ENOMEM;
> >
> >  	/* IRQs allocation */
> > -	nr_irqs = pci_alloc_irq_vectors(pdev, 1, vsec_data->irqs,
> > +	nr_irqs = pci_alloc_irq_vectors(pdev, 1, dma_data->irqs,
> >  					PCI_IRQ_MSI | PCI_IRQ_MSIX);
> >  	if (nr_irqs < 1) {
> >  		pci_err(pdev, "fail to alloc IRQ vector (number of IRQs=%u)\n",
> > @@ -433,23 +431,23 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
> >  	/* Data structure initialization */
> >  	chip->dev = dev;
> >
> > -	chip->mf = vsec_data->mf;
> > +	chip->mf = dma_data->mf;
> >  	chip->nr_irqs = nr_irqs;
> >  	chip->ops = &dw_edma_pcie_plat_ops;
> > -	chip->cfg_non_ll = vsec_data->cfg_non_ll;
> > +	chip->cfg_non_ll = dma_data->cfg_non_ll;
> >
> > -	chip->ll_wr_cnt = vsec_data->wr_ch_cnt;
> > -	chip->ll_rd_cnt = vsec_data->rd_ch_cnt;
> > +	chip->ll_wr_cnt = dma_data->wr_ch_cnt;
> > +	chip->ll_rd_cnt = dma_data->rd_ch_cnt;
> >
> > -	chip->reg_base = pcim_iomap_table(pdev)[vsec_data->rg.bar];
> > +	chip->reg_base = pcim_iomap_table(pdev)[dma_data->rg.bar];
> >  	if (!chip->reg_base)
> >  		return -ENOMEM;
> >
> > -	for (i = 0; i < chip->ll_wr_cnt && !vsec_data->cfg_non_ll; i++) {
> > +	for (i = 0; i < chip->ll_wr_cnt && !dma_data->cfg_non_ll; i++) {
> >  		struct dw_edma_region *ll_region = &chip->ll_region_wr[i];
> >  		struct dw_edma_region *dt_region = &chip->dt_region_wr[i];
> > -		struct dw_edma_block *ll_block = &vsec_data->ll_wr[i];
> > -		struct dw_edma_block *dt_block = &vsec_data->dt_wr[i];
> > +		struct dw_edma_block *ll_block = &dma_data->ll_wr[i];
> > +		struct dw_edma_block *dt_block = &dma_data->dt_wr[i];
> >
> >  		ll_region->vaddr.io = pcim_iomap_table(pdev)[ll_block->bar];
> >  		if (!ll_region->vaddr.io)
> > @@ -457,7 +455,7 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
> >
> >  		ll_region->vaddr.io += ll_block->off;
> >  		ll_region->paddr = dw_edma_get_phys_addr(pdev, match,
> > -							 vsec_data, ll_block->bar);
> > +							 dma_data, ll_block->bar);
> >  		ll_region->paddr += ll_block->off;
> >  		ll_region->sz = ll_block->sz;
> >
> > @@ -467,16 +465,16 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
> >
> >  		dt_region->vaddr.io += dt_block->off;
> >  		dt_region->paddr = dw_edma_get_phys_addr(pdev, match,
> > -							 vsec_data, dt_block->bar);
> > +							 dma_data, dt_block->bar);
> >  		dt_region->paddr += dt_block->off;
> >  		dt_region->sz = dt_block->sz;
> >  	}
> >
> > -	for (i = 0; i < chip->ll_rd_cnt && !vsec_data->cfg_non_ll; i++) {
> > +	for (i = 0; i < chip->ll_rd_cnt && !dma_data->cfg_non_ll; i++) {
> >  		struct dw_edma_region *ll_region = &chip->ll_region_rd[i];
> >  		struct dw_edma_region *dt_region = &chip->dt_region_rd[i];
> > -		struct dw_edma_block *ll_block = &vsec_data->ll_rd[i];
> > -		struct dw_edma_block *dt_block = &vsec_data->dt_rd[i];
> > +		struct dw_edma_block *ll_block = &dma_data->ll_rd[i];
> > +		struct dw_edma_block *dt_block = &dma_data->dt_rd[i];
> >
> >  		ll_region->vaddr.io = pcim_iomap_table(pdev)[ll_block->bar];
> >  		if (!ll_region->vaddr.io)
> > @@ -484,7 +482,7 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
> >
> >  		ll_region->vaddr.io += ll_block->off;
> >  		ll_region->paddr = dw_edma_get_phys_addr(pdev, match,
> > -							 vsec_data, ll_block->bar);
> > +							 dma_data, ll_block->bar);
> >  		ll_region->paddr += ll_block->off;
> >  		ll_region->sz = ll_block->sz;
> >
> > @@ -494,7 +492,7 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
> >
> >  		dt_region->vaddr.io += dt_block->off;
> >  		dt_region->paddr = dw_edma_get_phys_addr(pdev, match,
> > -							 vsec_data, dt_block->bar);
> > +							 dma_data, dt_block->bar);
> >  		dt_region->paddr += dt_block->off;
> >  		dt_region->sz = dt_block->sz;
> >  	}
> > @@ -512,31 +510,31 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
> >  		pci_dbg(pdev, "Version:\tUnknown (0x%x)\n", chip->mf);
> >
> >  	pci_dbg(pdev, "Registers:\tBAR=%u, off=0x%.8lx, sz=0x%zx bytes, addr(v=%p)\n",
> > -		vsec_data->rg.bar, vsec_data->rg.off, vsec_data->rg.sz,
> > +		dma_data->rg.bar, dma_data->rg.off, dma_data->rg.sz,
> >  		chip->reg_base);
> >
> >
> >  	for (i = 0; i < chip->ll_wr_cnt; i++) {
> >  		pci_dbg(pdev, "L. List:\tWRITE CH%.2u, BAR=%u, off=0x%.8lx, sz=0x%zx bytes, addr(v=%p, p=%pa)\n",
> > -			i, vsec_data->ll_wr[i].bar,
> > -			vsec_data->ll_wr[i].off, chip->ll_region_wr[i].sz,
> > +			i, dma_data->ll_wr[i].bar,
> > +			dma_data->ll_wr[i].off, chip->ll_region_wr[i].sz,
> >  			chip->ll_region_wr[i].vaddr.io, &chip->ll_region_wr[i].paddr);
> >
> >  		pci_dbg(pdev, "Data:\tWRITE CH%.2u, BAR=%u, off=0x%.8lx, sz=0x%zx bytes, addr(v=%p, p=%pa)\n",
> > -			i, vsec_data->dt_wr[i].bar,
> > -			vsec_data->dt_wr[i].off, chip->dt_region_wr[i].sz,
> > +			i, dma_data->dt_wr[i].bar,
> > +			dma_data->dt_wr[i].off, chip->dt_region_wr[i].sz,
> >  			chip->dt_region_wr[i].vaddr.io, &chip->dt_region_wr[i].paddr);
> >  	}
> >
> >  	for (i = 0; i < chip->ll_rd_cnt; i++) {
> >  		pci_dbg(pdev, "L. List:\tREAD CH%.2u, BAR=%u, off=0x%.8lx, sz=0x%zx bytes, addr(v=%p, p=%pa)\n",
> > -			i, vsec_data->ll_rd[i].bar,
> > -			vsec_data->ll_rd[i].off, chip->ll_region_rd[i].sz,
> > +			i, dma_data->ll_rd[i].bar,
> > +			dma_data->ll_rd[i].off, chip->ll_region_rd[i].sz,
> >  			chip->ll_region_rd[i].vaddr.io, &chip->ll_region_rd[i].paddr);
> >
> >  		pci_dbg(pdev, "Data:\tREAD CH%.2u, BAR=%u, off=0x%.8lx, sz=0x%zx bytes, addr(v=%p, p=%pa)\n",
> > -			i, vsec_data->dt_rd[i].bar,
> > -			vsec_data->dt_rd[i].off, chip->dt_region_rd[i].sz,
> > +			i, dma_data->dt_rd[i].bar,
> > +			dma_data->dt_rd[i].off, chip->dt_region_rd[i].sz,
> >  			chip->dt_region_rd[i].vaddr.io, &chip->dt_region_rd[i].paddr);
> >  	}
> >
> > --
> > 2.51.0
> >