[PATCH 0/4] dmaengine: dw-edma: Fix probe paths and register races

[PATCH 0/4] dmaengine: dw-edma: Fix probe paths and register races

[Koichiro Den]

Hi,

This series fixes pre-existing dw-edma issues flagged by Sashiko in:
https://lore.kernel.org/dmaengine/20260521063115.2842238-1-den@valinux.co.jp/

Note: Patch 4 was based on a patch Frank posted in January:
      https://lore.kernel.org/dmaengine/20260109-edma_ll-v2-1-5c0b27b2c664@nxp.com/
      Since it has not been merged, I included it here. Frank, please let me
      know if you prefer a different handling.

Best regards,
Koichiro


Frank Li (1):
  dmaengine: dw-edma: Add spinlock to protect DONE_INT_MASK and
    ABORT_INT_MASK

Koichiro Den (3):
  dmaengine: dw-edma-pcie: Free IRQ vectors on probe failures
  dmaengine: dw-edma-pcie: Reject devices without driver data
  dmaengine: dw-edma: Initialize IRQ data before requesting IRQs

 drivers/dma/dw-edma/dw-edma-core.c    |  3 +-
 drivers/dma/dw-edma/dw-edma-core.h    |  2 +-
 drivers/dma/dw-edma/dw-edma-pcie.c    | 42 +++++++++++++++++++--------
 drivers/dma/dw-edma/dw-edma-v0-core.c |  6 ++++
 4 files changed, 39 insertions(+), 14 deletions(-)

-- 
2.51.0

[PATCH 1/4] dmaengine: dw-edma-pcie: Free IRQ vectors on probe failures

[Koichiro Den]

dw_edma_pcie_probe() leaks IRQ vectors by returning without calling
pci_free_irq_vectors() in error paths after pci_alloc_irq_vectors()
succeeds.

Route the post-allocation failures through a common cleanup path so the
vectors are released before probe returns.

Fixes: 41aaff2a2ac0 ("dmaengine: Add Synopsys eDMA IP PCIe glue-logic")
Cc: stable@vger.kernel.org
Signed-off-by: Koichiro Den <den@valinux.co.jp>
---
 drivers/dma/dw-edma/dw-edma-pcie.c | 39 +++++++++++++++++++++---------
 1 file changed, 27 insertions(+), 12 deletions(-)

diff --git a/drivers/dma/dw-edma/dw-edma-pcie.c b/drivers/dma/dw-edma/dw-edma-pcie.c
index 0b30ce138503..87c31d01fb10 100644
--- a/drivers/dma/dw-edma/dw-edma-pcie.c
+++ b/drivers/dma/dw-edma/dw-edma-pcie.c
@@ -410,8 +410,10 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
 	chip->ll_rd_cnt = vsec_data->rd_ch_cnt;
 
 	chip->reg_base = pcim_iomap_table(pdev)[vsec_data->rg.bar];
-	if (!chip->reg_base)
-		return -ENOMEM;
+	if (!chip->reg_base) {
+		err = -ENOMEM;
+		goto err_free_irq_vectors;
+	}
 
 	for (i = 0; i < chip->ll_wr_cnt && !non_ll; i++) {
 		struct dw_edma_region *ll_region = &chip->ll_region_wr[i];
@@ -420,8 +422,10 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
 		struct dw_edma_block *dt_block = &vsec_data->dt_wr[i];
 
 		ll_region->vaddr.io = pcim_iomap_table(pdev)[ll_block->bar];
-		if (!ll_region->vaddr.io)
-			return -ENOMEM;
+		if (!ll_region->vaddr.io) {
+			err = -ENOMEM;
+			goto err_free_irq_vectors;
+		}
 
 		ll_region->vaddr.io += ll_block->off;
 		ll_region->paddr = dw_edma_get_phys_addr(pdev, vsec_data,
@@ -430,8 +434,10 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
 		ll_region->sz = ll_block->sz;
 
 		dt_region->vaddr.io = pcim_iomap_table(pdev)[dt_block->bar];
-		if (!dt_region->vaddr.io)
-			return -ENOMEM;
+		if (!dt_region->vaddr.io) {
+			err = -ENOMEM;
+			goto err_free_irq_vectors;
+		}
 
 		dt_region->vaddr.io += dt_block->off;
 		dt_region->paddr = dw_edma_get_phys_addr(pdev, vsec_data,
@@ -447,8 +453,10 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
 		struct dw_edma_block *dt_block = &vsec_data->dt_rd[i];
 
 		ll_region->vaddr.io = pcim_iomap_table(pdev)[ll_block->bar];
-		if (!ll_region->vaddr.io)
-			return -ENOMEM;
+		if (!ll_region->vaddr.io) {
+			err = -ENOMEM;
+			goto err_free_irq_vectors;
+		}
 
 		ll_region->vaddr.io += ll_block->off;
 		ll_region->paddr = dw_edma_get_phys_addr(pdev, vsec_data,
@@ -457,8 +465,10 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
 		ll_region->sz = ll_block->sz;
 
 		dt_region->vaddr.io = pcim_iomap_table(pdev)[dt_block->bar];
-		if (!dt_region->vaddr.io)
-			return -ENOMEM;
+		if (!dt_region->vaddr.io) {
+			err = -ENOMEM;
+			goto err_free_irq_vectors;
+		}
 
 		dt_region->vaddr.io += dt_block->off;
 		dt_region->paddr = dw_edma_get_phys_addr(pdev, vsec_data,
@@ -513,20 +523,25 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
 	/* Validating if PCI interrupts were enabled */
 	if (!pci_dev_msi_enabled(pdev)) {
 		pci_err(pdev, "enable interrupt failed\n");
-		return -EPERM;
+		err = -EPERM;
+		goto err_free_irq_vectors;
 	}
 
 	/* Starting eDMA driver */
 	err = dw_edma_probe(chip);
 	if (err) {
 		pci_err(pdev, "eDMA probe failed\n");
-		return err;
+		goto err_free_irq_vectors;
 	}
 
 	/* Saving data structure reference */
 	pci_set_drvdata(pdev, chip);
 
 	return 0;
+
+err_free_irq_vectors:
+	pci_free_irq_vectors(pdev);
+	return err;
 }
 
 static void dw_edma_pcie_remove(struct pci_dev *pdev)
-- 
2.51.0

Re: [PATCH 1/4] dmaengine: dw-edma-pcie: Free IRQ vectors on probe failures

[Frank Li]

On Thu, May 21, 2026 at 11:21:50PM +0900, Koichiro Den wrote:
> dw_edma_pcie_probe() leaks IRQ vectors by returning without calling
> pci_free_irq_vectors() in error paths after pci_alloc_irq_vectors()
> succeeds.

I remember pcim_enable_device() already auto manage irqs.

Frank

>
> Route the post-allocation failures through a common cleanup path so the
> vectors are released before probe returns.
>
> Fixes: 41aaff2a2ac0 ("dmaengine: Add Synopsys eDMA IP PCIe glue-logic")
> Cc: stable@vger.kernel.org
> Signed-off-by: Koichiro Den <den@valinux.co.jp>
> ---
>  drivers/dma/dw-edma/dw-edma-pcie.c | 39 +++++++++++++++++++++---------
>  1 file changed, 27 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/dma/dw-edma/dw-edma-pcie.c b/drivers/dma/dw-edma/dw-edma-pcie.c
> index 0b30ce138503..87c31d01fb10 100644
> --- a/drivers/dma/dw-edma/dw-edma-pcie.c
> +++ b/drivers/dma/dw-edma/dw-edma-pcie.c
> @@ -410,8 +410,10 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
>  	chip->ll_rd_cnt = vsec_data->rd_ch_cnt;
>
>  	chip->reg_base = pcim_iomap_table(pdev)[vsec_data->rg.bar];
> -	if (!chip->reg_base)
> -		return -ENOMEM;
> +	if (!chip->reg_base) {
> +		err = -ENOMEM;
> +		goto err_free_irq_vectors;
> +	}
>
>  	for (i = 0; i < chip->ll_wr_cnt && !non_ll; i++) {
>  		struct dw_edma_region *ll_region = &chip->ll_region_wr[i];
> @@ -420,8 +422,10 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
>  		struct dw_edma_block *dt_block = &vsec_data->dt_wr[i];
>
>  		ll_region->vaddr.io = pcim_iomap_table(pdev)[ll_block->bar];
> -		if (!ll_region->vaddr.io)
> -			return -ENOMEM;
> +		if (!ll_region->vaddr.io) {
> +			err = -ENOMEM;
> +			goto err_free_irq_vectors;
> +		}
>
>  		ll_region->vaddr.io += ll_block->off;
>  		ll_region->paddr = dw_edma_get_phys_addr(pdev, vsec_data,
> @@ -430,8 +434,10 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
>  		ll_region->sz = ll_block->sz;
>
>  		dt_region->vaddr.io = pcim_iomap_table(pdev)[dt_block->bar];
> -		if (!dt_region->vaddr.io)
> -			return -ENOMEM;
> +		if (!dt_region->vaddr.io) {
> +			err = -ENOMEM;
> +			goto err_free_irq_vectors;
> +		}
>
>  		dt_region->vaddr.io += dt_block->off;
>  		dt_region->paddr = dw_edma_get_phys_addr(pdev, vsec_data,
> @@ -447,8 +453,10 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
>  		struct dw_edma_block *dt_block = &vsec_data->dt_rd[i];
>
>  		ll_region->vaddr.io = pcim_iomap_table(pdev)[ll_block->bar];
> -		if (!ll_region->vaddr.io)
> -			return -ENOMEM;
> +		if (!ll_region->vaddr.io) {
> +			err = -ENOMEM;
> +			goto err_free_irq_vectors;
> +		}
>
>  		ll_region->vaddr.io += ll_block->off;
>  		ll_region->paddr = dw_edma_get_phys_addr(pdev, vsec_data,
> @@ -457,8 +465,10 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
>  		ll_region->sz = ll_block->sz;
>
>  		dt_region->vaddr.io = pcim_iomap_table(pdev)[dt_block->bar];
> -		if (!dt_region->vaddr.io)
> -			return -ENOMEM;
> +		if (!dt_region->vaddr.io) {
> +			err = -ENOMEM;
> +			goto err_free_irq_vectors;
> +		}
>
>  		dt_region->vaddr.io += dt_block->off;
>  		dt_region->paddr = dw_edma_get_phys_addr(pdev, vsec_data,
> @@ -513,20 +523,25 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
>  	/* Validating if PCI interrupts were enabled */
>  	if (!pci_dev_msi_enabled(pdev)) {
>  		pci_err(pdev, "enable interrupt failed\n");
> -		return -EPERM;
> +		err = -EPERM;
> +		goto err_free_irq_vectors;
>  	}
>
>  	/* Starting eDMA driver */
>  	err = dw_edma_probe(chip);
>  	if (err) {
>  		pci_err(pdev, "eDMA probe failed\n");
> -		return err;
> +		goto err_free_irq_vectors;
>  	}
>
>  	/* Saving data structure reference */
>  	pci_set_drvdata(pdev, chip);
>
>  	return 0;
> +
> +err_free_irq_vectors:
> +	pci_free_irq_vectors(pdev);
> +	return err;
>  }
>
>  static void dw_edma_pcie_remove(struct pci_dev *pdev)
> --
> 2.51.0
>

Re: [PATCH 1/4] dmaengine: dw-edma-pcie: Free IRQ vectors on probe failures

[Koichiro Den]

On Thu, May 21, 2026 at 10:39:09AM -0400, Frank Li wrote:
> On Thu, May 21, 2026 at 11:21:50PM +0900, Koichiro Den wrote:
> > dw_edma_pcie_probe() leaks IRQ vectors by returning without calling
> > pci_free_irq_vectors() in error paths after pci_alloc_irq_vectors()
> > succeeds.
> 
> I remember pcim_enable_device() already auto manage irqs.

You are right, pcim_enable_device() already manages the IRQ vectors.

Thanks for pointing it out. I'll drop patch 1 if I need to respin, or ask the
maintainers to disregard it when applying.

Best regards,
Koichiro

> 
> Frank
> 
> >
> > Route the post-allocation failures through a common cleanup path so the
> > vectors are released before probe returns.
> >
> > Fixes: 41aaff2a2ac0 ("dmaengine: Add Synopsys eDMA IP PCIe glue-logic")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Koichiro Den <den@valinux.co.jp>
> > ---
> >  drivers/dma/dw-edma/dw-edma-pcie.c | 39 +++++++++++++++++++++---------
> >  1 file changed, 27 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/dma/dw-edma/dw-edma-pcie.c b/drivers/dma/dw-edma/dw-edma-pcie.c
> > index 0b30ce138503..87c31d01fb10 100644
> > --- a/drivers/dma/dw-edma/dw-edma-pcie.c
> > +++ b/drivers/dma/dw-edma/dw-edma-pcie.c
> > @@ -410,8 +410,10 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
> >  	chip->ll_rd_cnt = vsec_data->rd_ch_cnt;
> >
> >  	chip->reg_base = pcim_iomap_table(pdev)[vsec_data->rg.bar];
> > -	if (!chip->reg_base)
> > -		return -ENOMEM;
> > +	if (!chip->reg_base) {
> > +		err = -ENOMEM;
> > +		goto err_free_irq_vectors;
> > +	}
> >
> >  	for (i = 0; i < chip->ll_wr_cnt && !non_ll; i++) {
> >  		struct dw_edma_region *ll_region = &chip->ll_region_wr[i];
> > @@ -420,8 +422,10 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
> >  		struct dw_edma_block *dt_block = &vsec_data->dt_wr[i];
> >
> >  		ll_region->vaddr.io = pcim_iomap_table(pdev)[ll_block->bar];
> > -		if (!ll_region->vaddr.io)
> > -			return -ENOMEM;
> > +		if (!ll_region->vaddr.io) {
> > +			err = -ENOMEM;
> > +			goto err_free_irq_vectors;
> > +		}
> >
> >  		ll_region->vaddr.io += ll_block->off;
> >  		ll_region->paddr = dw_edma_get_phys_addr(pdev, vsec_data,
> > @@ -430,8 +434,10 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
> >  		ll_region->sz = ll_block->sz;
> >
> >  		dt_region->vaddr.io = pcim_iomap_table(pdev)[dt_block->bar];
> > -		if (!dt_region->vaddr.io)
> > -			return -ENOMEM;
> > +		if (!dt_region->vaddr.io) {
> > +			err = -ENOMEM;
> > +			goto err_free_irq_vectors;
> > +		}
> >
> >  		dt_region->vaddr.io += dt_block->off;
> >  		dt_region->paddr = dw_edma_get_phys_addr(pdev, vsec_data,
> > @@ -447,8 +453,10 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
> >  		struct dw_edma_block *dt_block = &vsec_data->dt_rd[i];
> >
> >  		ll_region->vaddr.io = pcim_iomap_table(pdev)[ll_block->bar];
> > -		if (!ll_region->vaddr.io)
> > -			return -ENOMEM;
> > +		if (!ll_region->vaddr.io) {
> > +			err = -ENOMEM;
> > +			goto err_free_irq_vectors;
> > +		}
> >
> >  		ll_region->vaddr.io += ll_block->off;
> >  		ll_region->paddr = dw_edma_get_phys_addr(pdev, vsec_data,
> > @@ -457,8 +465,10 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
> >  		ll_region->sz = ll_block->sz;
> >
> >  		dt_region->vaddr.io = pcim_iomap_table(pdev)[dt_block->bar];
> > -		if (!dt_region->vaddr.io)
> > -			return -ENOMEM;
> > +		if (!dt_region->vaddr.io) {
> > +			err = -ENOMEM;
> > +			goto err_free_irq_vectors;
> > +		}
> >
> >  		dt_region->vaddr.io += dt_block->off;
> >  		dt_region->paddr = dw_edma_get_phys_addr(pdev, vsec_data,
> > @@ -513,20 +523,25 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
> >  	/* Validating if PCI interrupts were enabled */
> >  	if (!pci_dev_msi_enabled(pdev)) {
> >  		pci_err(pdev, "enable interrupt failed\n");
> > -		return -EPERM;
> > +		err = -EPERM;
> > +		goto err_free_irq_vectors;
> >  	}
> >
> >  	/* Starting eDMA driver */
> >  	err = dw_edma_probe(chip);
> >  	if (err) {
> >  		pci_err(pdev, "eDMA probe failed\n");
> > -		return err;
> > +		goto err_free_irq_vectors;
> >  	}
> >
> >  	/* Saving data structure reference */
> >  	pci_set_drvdata(pdev, chip);
> >
> >  	return 0;
> > +
> > +err_free_irq_vectors:
> > +	pci_free_irq_vectors(pdev);
> > +	return err;
> >  }
> >
> >  static void dw_edma_pcie_remove(struct pci_dev *pdev)
> > --
> > 2.51.0
> >

[PATCH 2/4] dmaengine: dw-edma-pcie: Reject devices without driver data

[Koichiro Den]

dw_edma_pcie_probe() treats the PCI device ID driver_data as the
template for the controller layout and copies it unconditionally. A
device bound dynamically via sysfs can match the driver without that
data, which leads to a NULL pointer dereference.

Reject such matches before enabling the device.

Fixes: 41aaff2a2ac0 ("dmaengine: Add Synopsys eDMA IP PCIe glue-logic")
Cc: stable@vger.kernel.org
Signed-off-by: Koichiro Den <den@valinux.co.jp>
---
 drivers/dma/dw-edma/dw-edma-pcie.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/dma/dw-edma/dw-edma-pcie.c b/drivers/dma/dw-edma/dw-edma-pcie.c
index 87c31d01fb10..c2024fa824e0 100644
--- a/drivers/dma/dw-edma/dw-edma-pcie.c
+++ b/drivers/dma/dw-edma/dw-edma-pcie.c
@@ -314,6 +314,9 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
 	int i, mask;
 	bool non_ll = false;
 
+	if (!pdata)
+		return -ENODEV;
+
 	struct dw_edma_pcie_data *vsec_data __free(kfree) =
 		kmalloc_obj(*vsec_data);
 	if (!vsec_data)
-- 
2.51.0

Re: [PATCH 2/4] dmaengine: dw-edma-pcie: Reject devices without driver data

[Frank Li]

On Thu, May 21, 2026 at 11:21:51PM +0900, Koichiro Den wrote:
> dw_edma_pcie_probe() treats the PCI device ID driver_data as the
> template for the controller layout and copies it unconditionally. A
> device bound dynamically via sysfs can match the driver without that
> data, which leads to a NULL pointer dereference.
>
> Reject such matches before enabling the device.
>
> Fixes: 41aaff2a2ac0 ("dmaengine: Add Synopsys eDMA IP PCIe glue-logic")
> Cc: stable@vger.kernel.org
> Signed-off-by: Koichiro Den <den@valinux.co.jp>
> ---

Reviewed-by: Frank Li <Frank.Li@nxp.com>

>  drivers/dma/dw-edma/dw-edma-pcie.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/drivers/dma/dw-edma/dw-edma-pcie.c b/drivers/dma/dw-edma/dw-edma-pcie.c
> index 87c31d01fb10..c2024fa824e0 100644
> --- a/drivers/dma/dw-edma/dw-edma-pcie.c
> +++ b/drivers/dma/dw-edma/dw-edma-pcie.c
> @@ -314,6 +314,9 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
>  	int i, mask;
>  	bool non_ll = false;
>
> +	if (!pdata)
> +		return -ENODEV;
> +
>  	struct dw_edma_pcie_data *vsec_data __free(kfree) =
>  		kmalloc_obj(*vsec_data);
>  	if (!vsec_data)
> --
> 2.51.0
>

[PATCH 3/4] dmaengine: dw-edma: Initialize IRQ data before requesting IRQs

[Koichiro Den]

dw_edma_irq_request() passes struct dw_edma_irq to request_irq()
before dw_edma_channel_setup() fills the back pointer. A shared
interrupt can therefore enter the handler with dw_irq->dw still NULL,
leading to a NULL pointer dereference.

Set the back pointer before installing each handler.

Fixes: e63d79d1ffcd ("dmaengine: Add Synopsys eDMA IP core driver")
Cc: stable@vger.kernel.org
Signed-off-by: Koichiro Den <den@valinux.co.jp>
---
 drivers/dma/dw-edma/dw-edma-core.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
index c2feb3adc79f..d221e3efcb36 100644
--- a/drivers/dma/dw-edma/dw-edma-core.c
+++ b/drivers/dma/dw-edma/dw-edma-core.c
@@ -929,7 +929,6 @@ static int dw_edma_channel_setup(struct dw_edma *dw, u32 wr_alloc, u32 rd_alloc)
 		else
 			irq->rd_mask |= BIT(chan->id);
 
-		irq->dw = dw;
 		memcpy(&chan->msi, &irq->msi, sizeof(chan->msi));
 
 		dev_vdbg(dev, "MSI:\t\tChannel %s[%u] addr=0x%.8x%.8x, data=0x%.8x\n",
@@ -1018,6 +1017,7 @@ static int dw_edma_irq_request(struct dw_edma *dw,
 	if (chip->nr_irqs == 1) {
 		/* Common IRQ shared among all channels */
 		irq = chip->ops->irq_vector(dev, 0);
+		dw->irq[0].dw = dw;
 		err = request_irq(irq, dw_edma_interrupt_common,
 				  IRQF_SHARED, dw->name, &dw->irq[0]);
 		if (err) {
@@ -1043,6 +1043,7 @@ static int dw_edma_irq_request(struct dw_edma *dw,
 
 		for (i = 0; i < (*wr_alloc + *rd_alloc); i++) {
 			irq = chip->ops->irq_vector(dev, i);
+			dw->irq[i].dw = dw;
 			err = request_irq(irq,
 					  i < *wr_alloc ?
 						dw_edma_interrupt_write :
-- 
2.51.0

Re: [PATCH 3/4] dmaengine: dw-edma: Initialize IRQ data before requesting IRQs

[Frank Li]

On Thu, May 21, 2026 at 11:21:52PM +0900, Koichiro Den wrote:
> dw_edma_irq_request() passes struct dw_edma_irq to request_irq()
> before dw_edma_channel_setup() fills the back pointer. A shared
> interrupt can therefore enter the handler with dw_irq->dw still NULL,
> leading to a NULL pointer dereference.
>
> Set the back pointer before installing each handler.
>
> Fixes: e63d79d1ffcd ("dmaengine: Add Synopsys eDMA IP core driver")
> Cc: stable@vger.kernel.org
> Signed-off-by: Koichiro Den <den@valinux.co.jp>
> ---

Reviewed-by: Frank Li <Frank.Li@nxp.com>

>  drivers/dma/dw-edma/dw-edma-core.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
> index c2feb3adc79f..d221e3efcb36 100644
> --- a/drivers/dma/dw-edma/dw-edma-core.c
> +++ b/drivers/dma/dw-edma/dw-edma-core.c
> @@ -929,7 +929,6 @@ static int dw_edma_channel_setup(struct dw_edma *dw, u32 wr_alloc, u32 rd_alloc)
>  		else
>  			irq->rd_mask |= BIT(chan->id);
>
> -		irq->dw = dw;
>  		memcpy(&chan->msi, &irq->msi, sizeof(chan->msi));
>
>  		dev_vdbg(dev, "MSI:\t\tChannel %s[%u] addr=0x%.8x%.8x, data=0x%.8x\n",
> @@ -1018,6 +1017,7 @@ static int dw_edma_irq_request(struct dw_edma *dw,
>  	if (chip->nr_irqs == 1) {
>  		/* Common IRQ shared among all channels */
>  		irq = chip->ops->irq_vector(dev, 0);
> +		dw->irq[0].dw = dw;
>  		err = request_irq(irq, dw_edma_interrupt_common,
>  				  IRQF_SHARED, dw->name, &dw->irq[0]);
>  		if (err) {
> @@ -1043,6 +1043,7 @@ static int dw_edma_irq_request(struct dw_edma *dw,
>
>  		for (i = 0; i < (*wr_alloc + *rd_alloc); i++) {
>  			irq = chip->ops->irq_vector(dev, i);
> +			dw->irq[i].dw = dw;
>  			err = request_irq(irq,
>  					  i < *wr_alloc ?
>  						dw_edma_interrupt_write :
> --
> 2.51.0
>

Re: [PATCH 3/4] dmaengine: dw-edma: Initialize IRQ data before requesting IRQs

[Koichiro Den]

On Thu, May 21, 2026 at 10:44:47AM -0400, Frank Li wrote:
> On Thu, May 21, 2026 at 11:21:52PM +0900, Koichiro Den wrote:
> > dw_edma_irq_request() passes struct dw_edma_irq to request_irq()
> > before dw_edma_channel_setup() fills the back pointer. A shared
> > interrupt can therefore enter the handler with dw_irq->dw still NULL,
> > leading to a NULL pointer dereference.
> >
> > Set the back pointer before installing each handler.
> >
> > Fixes: e63d79d1ffcd ("dmaengine: Add Synopsys eDMA IP core driver")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Koichiro Den <den@valinux.co.jp>
> > ---
> 
> Reviewed-by: Frank Li <Frank.Li@nxp.com>

Hi Frank,

Thanks for reviewing.

After Sashiko raised another point about patch 3, I looked into the init
ordering again. I now think patch 3 is half-baked fix, and probably not
needed in this small-fixes series.

The two Sashiko comments were about the IRQ handler being registered before
dw_edma_channel_setup():

  https://lore.kernel.org/dmaengine/20260521072453.E5AD21F00A3C@smtp.kernel.org/
  https://lore.kernel.org/dmaengine/20260521155834.D8DFF1F00A3C@smtp.kernel.org/

For current upstream, I don't think a shared IRQ alone can reach
dw_edma_done_interrupt() or dw_edma_abort_interrupt(). dw_edma_core_off() runs
before dw_edma_irq_request(), and the handler still needs DONE/ABORT status bits
before it dispatches to those callbacks.

So patch 3 only fixes part of a defensive ordering concern, and the later
Sashiko comment shows that moving irq->dw alone would be incomplete anyway.
If we want to harden this path, I think the cleaner change would be to split and
reorder the setup flow like this:

  Before:
    (1). dw_edma_irq_request()
      (1-a). allocate/populate dw->irq[] and cache MSI messages
      (1-b). request_irq()
    (2). dw_edma_channel_setup()
      (2-a). initialize channels, including vchan_init() and
             dw_edma_core_ch_config()
      (2-b). register the DMA device with dma_async_device_register()

  After:
    (1-a) -> (2-a) -> (1-b) -> (2-b)

But that is more of a cleanup/hardening change than a small pre-existing fix.
(If preferred, I can send a separate patch for that.)

So my conclusion is that only patches 2 and 4 are really needed in this series.
Patch 3 should be dropped. If a respin is needed for any resons, I will send v2
with dropping Patch 1 and 3.

Best regards,
Koichiro

> 
> >  drivers/dma/dw-edma/dw-edma-core.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
> > index c2feb3adc79f..d221e3efcb36 100644
> > --- a/drivers/dma/dw-edma/dw-edma-core.c
> > +++ b/drivers/dma/dw-edma/dw-edma-core.c
> > @@ -929,7 +929,6 @@ static int dw_edma_channel_setup(struct dw_edma *dw, u32 wr_alloc, u32 rd_alloc)
> >  		else
> >  			irq->rd_mask |= BIT(chan->id);
> >
> > -		irq->dw = dw;
> >  		memcpy(&chan->msi, &irq->msi, sizeof(chan->msi));
> >
> >  		dev_vdbg(dev, "MSI:\t\tChannel %s[%u] addr=0x%.8x%.8x, data=0x%.8x\n",
> > @@ -1018,6 +1017,7 @@ static int dw_edma_irq_request(struct dw_edma *dw,
> >  	if (chip->nr_irqs == 1) {
> >  		/* Common IRQ shared among all channels */
> >  		irq = chip->ops->irq_vector(dev, 0);
> > +		dw->irq[0].dw = dw;
> >  		err = request_irq(irq, dw_edma_interrupt_common,
> >  				  IRQF_SHARED, dw->name, &dw->irq[0]);
> >  		if (err) {
> > @@ -1043,6 +1043,7 @@ static int dw_edma_irq_request(struct dw_edma *dw,
> >
> >  		for (i = 0; i < (*wr_alloc + *rd_alloc); i++) {
> >  			irq = chip->ops->irq_vector(dev, i);
> > +			dw->irq[i].dw = dw;
> >  			err = request_irq(irq,
> >  					  i < *wr_alloc ?
> >  						dw_edma_interrupt_write :
> > --
> > 2.51.0
> >

[PATCH 4/4] dmaengine: dw-edma: Add spinlock to protect DONE_INT_MASK and ABORT_INT_MASK

[Koichiro Den]

From: Frank Li <Frank.Li@nxp.com>

The DONE_INT_MASK and ABORT_INT_MASK registers are shared by all DMA
channels, and modifying them requires a read-modify-write sequence.
Because this operation is not atomic, concurrent calls to
dw_edma_v0_core_start() can introduce race conditions if two channels
update these registers simultaneously.

Add a spinlock to serialize access to these registers and prevent race
conditions.

Fixes: 7e4b8a4fbe2c ("dmaengine: Add Synopsys eDMA IP version 0 support")
Cc: stable@vger.kernel.org
Signed-off-by: Frank Li <Frank.Li@nxp.com>
[den: update dw_edma.lock comment]
Link: https://lore.kernel.org/dmaengine/20260109-edma_ll-v2-1-5c0b27b2c664@nxp.com/
Signed-off-by: Koichiro Den <den@valinux.co.jp>
---
 drivers/dma/dw-edma/dw-edma-core.h    | 2 +-
 drivers/dma/dw-edma/dw-edma-v0-core.c | 6 ++++++
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/dma/dw-edma/dw-edma-core.h b/drivers/dma/dw-edma/dw-edma-core.h
index 902574b1ba86..6474cacf7195 100644
--- a/drivers/dma/dw-edma/dw-edma-core.h
+++ b/drivers/dma/dw-edma/dw-edma-core.h
@@ -109,7 +109,7 @@ struct dw_edma {
 
 	struct dw_edma_chan		*chan;
 
-	raw_spinlock_t			lock;		/* Only for legacy */
+	raw_spinlock_t			lock;		/* Protect v0 shared registers */
 
 	struct dw_edma_chip             *chip;
 
diff --git a/drivers/dma/dw-edma/dw-edma-v0-core.c b/drivers/dma/dw-edma/dw-edma-v0-core.c
index 69e8279adec8..cfdd6463252e 100644
--- a/drivers/dma/dw-edma/dw-edma-v0-core.c
+++ b/drivers/dma/dw-edma/dw-edma-v0-core.c
@@ -364,6 +364,7 @@ static void dw_edma_v0_core_start(struct dw_edma_chunk *chunk, bool first)
 {
 	struct dw_edma_chan *chan = chunk->chan;
 	struct dw_edma *dw = chan->dw;
+	unsigned long flags;
 	u32 tmp;
 
 	dw_edma_v0_core_write_chunk(chunk);
@@ -408,6 +409,8 @@ static void dw_edma_v0_core_start(struct dw_edma_chunk *chunk, bool first)
 			}
 		}
 		/* Interrupt unmask - done, abort */
+		raw_spin_lock_irqsave(&dw->lock, flags);
+
 		tmp = GET_RW_32(dw, chan->dir, int_mask);
 		tmp &= ~FIELD_PREP(EDMA_V0_DONE_INT_MASK, BIT(chan->id));
 		tmp &= ~FIELD_PREP(EDMA_V0_ABORT_INT_MASK, BIT(chan->id));
@@ -416,6 +419,9 @@ static void dw_edma_v0_core_start(struct dw_edma_chunk *chunk, bool first)
 		tmp = GET_RW_32(dw, chan->dir, linked_list_err_en);
 		tmp |= FIELD_PREP(EDMA_V0_LINKED_LIST_ERR_MASK, BIT(chan->id));
 		SET_RW_32(dw, chan->dir, linked_list_err_en, tmp);
+
+		raw_spin_unlock_irqrestore(&dw->lock, flags);
+
 		/* Channel control */
 		SET_CH_32(dw, chan->dir, chan->id, ch_control1,
 			  (DW_EDMA_V0_CCS | DW_EDMA_V0_LLE));
-- 
2.51.0

Re: [PATCH 0/4] dmaengine: dw-edma: Fix probe paths and register races

[Koichiro Den]

On Thu, May 21, 2026 at 11:21:49PM +0900, Koichiro Den wrote:
> Hi,
> 
> This series fixes pre-existing dw-edma issues flagged by Sashiko in:
> https://lore.kernel.org/dmaengine/20260521063115.2842238-1-den@valinux.co.jp/
> 
> Note: Patch 4 was based on a patch Frank posted in January:
>       https://lore.kernel.org/dmaengine/20260109-edma_ll-v2-1-5c0b27b2c664@nxp.com/
>       Since it has not been merged, I included it here. Frank, please let me
>       know if you prefer a different handling.
> 
> Best regards,
> Koichiro
> 
> 
> Frank Li (1):
>   dmaengine: dw-edma: Add spinlock to protect DONE_INT_MASK and
>     ABORT_INT_MASK
> 
> Koichiro Den (3):
>   dmaengine: dw-edma-pcie: Free IRQ vectors on probe failures
>   dmaengine: dw-edma-pcie: Reject devices without driver data
>   dmaengine: dw-edma: Initialize IRQ data before requesting IRQs

Frank, thank you for reviewing.

Mani, Vinod, if there are no objections, could you please consider applying only
patches 2 and 4 from this series?

- Patch 1 should be dropped. As Frank pointed out, pcim_enable_device() already
  manages IRQ vectors (i.e. sort of false-positive from Sashiko).

- Patch 2 still looks valid to me. After dropping patch 1, the new issue
  reported by Sashiko no longer applies, and I think the remaining concern is a
  false positive.

- Patch 3 should be dropped. I rechecked the initialization path and I no longer
  think this patch is needed. See:
  https://lore.kernel.org/dmaengine/kjslqii4bs3g4pi22mxh72hxnlm7nkesdd3va6zi5fhmjamerw@j7lbrlq5oszd/

- Patch 4 still looks valid to me as an independent fix. Sashiko's feedback
  against patch 4 also revealed a broader in-use unbind issue, but Frank's
  original path fixes a real race issue on its own. I am not sure whether we
  should add in-use unbind support right now. See:
  https://lore.kernel.org/dmaengine/ne76elxedfnngi7dilpyvpzwm7tghyj6kpg4ninwxecxsajkkx@zkarppyurl2s/

Best regards,
Koichiro

> 
>  drivers/dma/dw-edma/dw-edma-core.c    |  3 +-
>  drivers/dma/dw-edma/dw-edma-core.h    |  2 +-
>  drivers/dma/dw-edma/dw-edma-pcie.c    | 42 +++++++++++++++++++--------
>  drivers/dma/dw-edma/dw-edma-v0-core.c |  6 ++++
>  4 files changed, 39 insertions(+), 14 deletions(-)
> 
> -- 
> 2.51.0
> 
> 

Re: (subset) [PATCH 0/4] dmaengine: dw-edma: Fix probe paths and register races

[Vinod Koul]

On Thu, 21 May 2026 23:21:49 +0900, Koichiro Den wrote:
> This series fixes pre-existing dw-edma issues flagged by Sashiko in:
> https://lore.kernel.org/dmaengine/20260521063115.2842238-1-den@valinux.co.jp/
> 
> Note: Patch 4 was based on a patch Frank posted in January:
>       https://lore.kernel.org/dmaengine/20260109-edma_ll-v2-1-5c0b27b2c664@nxp.com/
>       Since it has not been merged, I included it here. Frank, please let me
>       know if you prefer a different handling.
> 
> [...]

Applied, thanks!

[2/4] dmaengine: dw-edma-pcie: Reject devices without driver data
      commit: 11d7cfe0c119691b2dafbb699bbca90258c678aa
[4/4] dmaengine: dw-edma: Add spinlock to protect DONE_INT_MASK and ABORT_INT_MASK
      commit: 8ffba0171c6bbce5f093c6dba5a02c0805b31203

Best regards,
-- 
~Vinod