Re: [PATCH v1] dmaengine: dw-edma: Add Xilinx CPM6-DMA DeviceID

[Vinod Koul <vkoul@kernel.org>]

On 04-06-26, 12:12, Verma, Devendra wrote:
> Public

Pls do _not_ Top Post!

> 
> The pre-existing issues need to be taken up in a separate series.
> The changes require separate testing which is not related to the
> current patch.

While I agree to that and it wont block the review but would be good if
you can fix that as well. Sure affects your users...

> 
> Regards,
> Devendra
> 
> > -----Original Message-----
> > From: sashiko-bot@kernel.org <sashiko-bot@kernel.org>
> > Sent: Wednesday, June 3, 2026 20:15
> > To: Verma, Devendra <Devendra.Verma@amd.com>
> > Cc: dmaengine@vger.kernel.org; vkoul@kernel.org; Frank.Li@kernel.org;
> > linux-pci@vger.kernel.org
> > Subject: Re: [PATCH v1] dmaengine: dw-edma: Add Xilinx CPM6-DMA
> > DeviceID
> >
> > Thank you for your contribution! Sashiko AI review found 1 potential issue(s)
> > to consider:
> >
> > Pre-existing issues:
> > - [High] Missing pci_free_irq_vectors() cleanup on error paths in
> > dw_edma_pcie_probe() leading to IRQ resource leaks.
> > --
> >
> > commit a523468dbe7999a5962895c3e0d73662a2938006
> > Author: Devendra K Verma <devverma@amd.com>
> >
> > dmaengine: dw-edma: Add Xilinx CPM6-DMA DeviceID
> >
> > Add Device ID for AMD (Xilinx) CPM6 DMA IP to enable 64 Read and 64 Write
> > Channels. Adds the relevant dw_edma_pcie_data to use 8 Read and 8 Write
> > Channels for the initial commit.
> >
> > > diff --git a/drivers/dma/dw-edma/dw-edma-pcie.c
> > > b/drivers/dma/dw-edma/dw-edma-pcie.c
> > > index 0b30ce1385036..4ba368d18cb1d 100644
> > > --- a/drivers/dma/dw-edma/dw-edma-pcie.c
> > > +++ b/drivers/dma/dw-edma/dw-edma-pcie.c
> >
> > [ ... ]
> >
> > > @@ -547,6 +561,8 @@ 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 },
> > > +   { PCI_VDEVICE(XILINX, PCI_DEVICE_ID_XILINX_B00F),
> > > +     (kernel_ulong_t)&xilinx_cpm6_dma_data },
> >
> > [Severity: High]
> > This is a pre-existing issue, but adding this new device ID exposes the new
> > device to an IRQ resource leak in dw_edma_pcie_probe().
> >
> > When the device is probed, dw_edma_pcie_probe() successfully allocates
> > IRQs via pci_alloc_irq_vectors(). However, several subsequent error paths
> > return directly without calling pci_free_irq_vectors().
> >
> > For example, if the pcim_iomap_table() lookup fails:
> >
> > drivers/dma/dw-edma/dw-edma-pcie.c:dw_edma_pcie_probe() {
> >     ...
> >       chip->reg_base = pcim_iomap_table(pdev)[vsec_data->rg.bar];
> >       if (!chip->reg_base)
> >               return -ENOMEM;
> >     ...
> > }
> >
> > Or if the main eDMA probe fails later on:
> >
> > drivers/dma/dw-edma/dw-edma-pcie.c:dw_edma_pcie_probe() {
> >     ...
> >       err = dw_edma_probe(chip);
> >       if (err) {
> >               pci_err(pdev, "eDMA probe failed\n");
> >               return err;
> >       }
> >     ...
> > }
> >
> > Does this code leak the allocated IRQ vectors in these error paths? Because
> > IRQs are a strictly limited resource, repeated probe failures (such as deferred
> > probe or unbind/rebind cycles) could silently exhaust the system's IRQ
> > capacity.
> >
> > Would it make sense to ensure pci_free_irq_vectors() is called on all error
> > paths after successful allocation before enabling new devices?
> >
> > >     { }
> > >  };
> > >  MODULE_DEVICE_TABLE(pci, dw_edma_pcie_id_table);
> >
> > --
> > Sashiko AI review · https://sashiko.dev/#/patchset/20260603143158.3243500-
> > 1-devendra.verma@amd.com?part=1

-- 
~Vinod