[PATCH 00/10] dmaengine: fsldma: devm conversion, fixups, and cleanups

[PATCH 00/10] dmaengine: fsldma: devm conversion, fixups, and cleanups

[Rosen Penev]

Convert the Freescale Elo DMA driver to use managed device resources
(devm), simplifying probe error handling and the remove path by
dropping explicit iounmap, kfree, and free_irq calls.

While doing so, fix a few issues uncovered along the way:

  - Kill the channel tasklet before removal to prevent a race with
    the IRQ handler.
  - Check the return value of dma_async_device_register() instead
    of silently returning success.
  - Replace the powerpc-specific I/O accessors with portable
    generic ones so the driver can be built on non-powerpc
    architectures.

Build-tested with LLVM=1 ARCH=powerpc allmodconfig.

Rosen Penev (10):
  dmaengine: fsldma: kill tasklet before removing channel
  dmaengine: fsldma: check dma_async_device_register() return value
  dmaengine: fsldma: convert to platform_get_irq_optional()
  dmaengine: fsldma: convert to devm_kzalloc and fix error path
  dmaengine: fsldma: convert ioremap to devm_platform_ioremap_resource
  dmaengine: fsldma: convert channel allocation to devm_kzalloc
  dmaengine: fsldma: convert channel ioremap to devm_of_iomap
  dmaengine: fsldma: replace irq_of_parse_and_map with of_irq_get
  dmaengine: fsldma: convert to devm_request_irq
  dmaengine: fsldma: replace ppc-specific accessors with portable
    generic ones

 drivers/dma/Kconfig  |   2 +-
 drivers/dma/fsldma.c | 139 +++++++++++++------------------------------
 drivers/dma/fsldma.h |  35 ++++++++++-
 3 files changed, 76 insertions(+), 100 deletions(-)

-- 
2.54.0

[PATCH 01/10] dmaengine: fsldma: kill tasklet before removing channel

[Rosen Penev]

Add tasklet_kill() in fsl_dma_chan_remove() to prevent a race
where the tasklet, scheduled by the IRQ handler, runs after
the channel has been torn down. With the recent devm conversions
the channel struct is no longer freed in the remove path, so
this is not a use-after-free crash fix, but rather correct
shutdown sequencing to avoid the tasklet operating on a
logically-removed channel.

Assisted-by: opencode:big-pickle
Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/dma/fsldma.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c
index 22d62d958abd..0e2f84862261 100644
--- a/drivers/dma/fsldma.c
+++ b/drivers/dma/fsldma.c
@@ -1205,6 +1205,7 @@ static int fsl_dma_chan_probe(struct fsldma_device *fdev,
 
 static void fsl_dma_chan_remove(struct fsldma_chan *chan)
 {
+	tasklet_kill(&chan->tasklet);
 	irq_dispose_mapping(chan->irq);
 	list_del(&chan->common.device_node);
 	iounmap(chan->regs);
-- 
2.54.0

Re: [PATCH 01/10] dmaengine: fsldma: kill tasklet before removing channel

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

New issues:
- [Medium] The commit message inaccurately claims the channel struct is 'no longer freed in the remove path' due to devm conversions, concluding this is not a use-after-free fix.

Pre-existing issues:
- [High] The driver holds a spinlock while executing client callbacks, causing a deadlock if the client submits new transactions.
- [High] The hardware DMA engine is not explicitly halted when the driver is unbound or when channel resources are freed, leading to DMA fetching from freed memory.
--

commit 2874e539ce862c4c0134a865315888712d6c8923
Author: Rosen Penev <rosenp@gmail.com>

dmaengine: fsldma: kill tasklet before removing channel

This commit adds tasklet_kill() to fsl_dma_chan_remove() to prevent a race
where the tasklet could run after the channel has been torn down.

> With the recent devm conversions
> the channel struct is no longer freed in the remove path, so
> this is not a use-after-free crash fix, but rather correct
> shutdown sequencing to avoid the tasklet operating on a
> logically-removed channel.

[Severity: Medium]
Is this description accurate for this exact point in the git history?

Looking at fsl_dma_chan_remove(), it still explicitly calls kfree() on the
channel struct:

drivers/dma/fsldma.c:fsl_dma_chan_remove() {
        tasklet_kill(&chan->tasklet);
        [ ... ]
        kfree(chan);
}

It appears the memory is indeed manually freed here, making this patch an
active prevention of a use-after-free crash. Could the commit message be
updated to reflect the current state of the code before the devm conversions
are applied?

> diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c
> index 22d62d958abd9..0e2f84862261c 100644
> --- a/drivers/dma/fsldma.c
> +++ b/drivers/dma/fsldma.c
> @@ -1205,6 +1205,7 @@ static int fsl_dma_chan_probe(struct fsldma_device *fdev,
>  
>  static void fsl_dma_chan_remove(struct fsldma_chan *chan)
>  {
> +	tasklet_kill(&chan->tasklet);
>  	irq_dispose_mapping(chan->irq);
>  	list_del(&chan->common.device_node);
>  	iounmap(chan->regs);

[Severity: High]
This is a pre-existing issue, but does the hardware DMA engine need to be
explicitly halted when the driver is unbound or resources are freed?

During driver unbind in fsldma_of_remove(), the driver unmaps MMIO registers
and frees structures:

drivers/dma/fsldma.c:fsldma_of_remove() {
        [ ... ]
        for (i = 0; i < FSL_DMA_MAX_CHANS_PER_DEVICE; i++) {
                if (fdev->chan[i])
                        fsl_dma_chan_remove(fdev->chan[i]);
        }
        irq_dispose_mapping(fdev->irq);
        iounmap(fdev->regs);
        kfree(fdev);
        [ ... ]
}

Similarly, fsl_dma_free_chan_resources() destroys the descriptor pool:

drivers/dma/fsldma.c:fsl_dma_free_chan_resources() {
        [ ... ]
        dma_pool_destroy(chan->desc_pool);
        [ ... ]
}

Since dma_halt() is never called on the channel, could an actively running
transfer continue executing in the background and fetch descriptors or write
data to physical memory pages that have already been freed?

[Severity: High]
This is another pre-existing issue, but can this code cause a recursive
spinlock deadlock if a client submits new transactions from a callback?

When the driver executes client callbacks, it holds the descriptor lock:

drivers/dma/fsldma.c:fsldma_cleanup_descriptors() {
        [ ... ]
        cookie = fsldma_run_tx_complete_actions(chan, desc, cookie);
        [ ... ]
}

If the client completion callback then invokes fsl_dma_tx_submit() to submit
a new DMA transaction, that function attempts to acquire the same lock:

drivers/dma/fsldma.c:fsl_dma_tx_submit() {
        [ ... ]
        spin_lock_bh(&chan->desc_lock);
        [ ... ]
}

Will this result in a deadlock on the same CPU?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260605220134.43295-1-rosenp@gmail.com?part=1

Re: [PATCH 01/10] dmaengine: fsldma: kill tasklet before removing channel

[Frank Li]

On Fri, Jun 05, 2026 at 03:01:25PM -0700, Rosen Penev wrote:
>
> Add tasklet_kill() in fsl_dma_chan_remove() to prevent a race
> where the tasklet, scheduled by the IRQ handler, runs after
> the channel has been torn down. With the recent devm conversions
> the channel struct is no longer freed in the remove path, so
> this is not a use-after-free crash fix, but rather correct
> shutdown sequencing to avoid the tasklet operating on a
> logically-removed channel.

Use below commit should be enough, needn't talk about use-after-free

Call tasklet_kill() in fsl_dma_chan_remove() to prevent a race where a
tasklet scheduled from the IRQ handler can run after the channel has been
torn down.

Frank

>
> Assisted-by: opencode:big-pickle
> Signed-off-by: Rosen Penev <rosenp@gmail.com>
> ---
>  drivers/dma/fsldma.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c
> index 22d62d958abd..0e2f84862261 100644
> --- a/drivers/dma/fsldma.c
> +++ b/drivers/dma/fsldma.c
> @@ -1205,6 +1205,7 @@ static int fsl_dma_chan_probe(struct fsldma_device *fdev,
>
>  static void fsl_dma_chan_remove(struct fsldma_chan *chan)
>  {
> +       tasklet_kill(&chan->tasklet);
>         irq_dispose_mapping(chan->irq);
>         list_del(&chan->common.device_node);
>         iounmap(chan->regs);
> --
> 2.54.0
>

[PATCH 02/10] dmaengine: fsldma: check dma_async_device_register() return value

[Rosen Penev]

Check the return value of dma_async_device_register() in the probe
path and propagate errors instead of silently returning success.
Previously, a registration failure would cause a NULL pointer
dereference in list_del_rcu() during remove when
dma_async_device_unregister() tried to remove the device's
global_node from a list it was never added to.

Assisted-by: opencode:big-pickle
Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/dma/fsldma.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c
index 0e2f84862261..89b88447be1b 100644
--- a/drivers/dma/fsldma.c
+++ b/drivers/dma/fsldma.c
@@ -1293,7 +1293,11 @@ static int fsldma_of_probe(struct platform_device *op)
 		goto out_free_fdev;
 	}
 
-	dma_async_device_register(&fdev->common);
+	err = dma_async_device_register(&fdev->common);
+	if (err) {
+		dev_err(fdev->dev, "unable to register DMA device\n");
+		goto out_free_fdev;
+	}
 	return 0;
 
 out_free_fdev:
-- 
2.54.0

[PATCH 03/10] dmaengine: fsldma: convert to platform_get_irq_optional()

[Rosen Penev]

Replace the per-controller irq_of_parse_and_map() call with
platform_get_irq_optional(). The controller IRQ is optional — when
absent (-ENXIO) the driver falls back to per-channel IRQs. Any other
error is treated as fatal. The corresponding irq_dispose_mapping()
calls in the probe error path and remove function are removed.

The per-channel IRQ mapping in fsl_dma_chan_probe() uses a child
device_node rather than the platform device's of_node, so it is not
converted here.

Assisted-by: opencode:big-pickle
Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/dma/fsldma.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c
index 89b88447be1b..0d28f8299bf8 100644
--- a/drivers/dma/fsldma.c
+++ b/drivers/dma/fsldma.c
@@ -1206,7 +1206,6 @@ static int fsl_dma_chan_probe(struct fsldma_device *fdev,
 static void fsl_dma_chan_remove(struct fsldma_chan *chan)
 {
 	tasklet_kill(&chan->tasklet);
-	irq_dispose_mapping(chan->irq);
 	list_del(&chan->common.device_node);
 	iounmap(chan->regs);
 	kfree(chan);
@@ -1239,7 +1238,14 @@ static int fsldma_of_probe(struct platform_device *op)
 	}
 
 	/* map the channel IRQ if it exists, but don't hookup the handler yet */
-	fdev->irq = irq_of_parse_and_map(op->dev.of_node, 0);
+	fdev->irq = platform_get_irq_optional(op, 0);
+	if (fdev->irq < 0) {
+		if (fdev->irq != -ENXIO) {
+			err = fdev->irq;
+			goto out_iounmap;
+		}
+		fdev->irq = 0;
+	}
 
 	dma_cap_set(DMA_MEMCPY, fdev->common.cap_mask);
 	dma_cap_set(DMA_SLAVE, fdev->common.cap_mask);
@@ -1305,7 +1311,7 @@ static int fsldma_of_probe(struct platform_device *op)
 		if (fdev->chan[i])
 			fsl_dma_chan_remove(fdev->chan[i]);
 	}
-	irq_dispose_mapping(fdev->irq);
+out_iounmap:
 	iounmap(fdev->regs);
 out_free:
 	kfree(fdev);
@@ -1327,7 +1333,6 @@ static void fsldma_of_remove(struct platform_device *op)
 		if (fdev->chan[i])
 			fsl_dma_chan_remove(fdev->chan[i]);
 	}
-	irq_dispose_mapping(fdev->irq);
 
 	iounmap(fdev->regs);
 	kfree(fdev);
-- 
2.54.0

[PATCH 04/10] dmaengine: fsldma: convert to devm_kzalloc and fix error path

[Rosen Penev]

Convert fdev allocation from kzalloc_obj to devm_kzalloc to simplify
the probe error and remove paths by dropping the explicit kfree.

While at it, fix a goto target mismatch introduced in the recent
platform_get_irq_optional() conversion: goto err_iounmap should
be goto out_iounmap.

Assisted-by: opencode:big-pickle
Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/dma/fsldma.c | 18 ++++++------------
 1 file changed, 6 insertions(+), 12 deletions(-)

diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c
index 0d28f8299bf8..2efa16d12679 100644
--- a/drivers/dma/fsldma.c
+++ b/drivers/dma/fsldma.c
@@ -1213,18 +1213,17 @@ static void fsl_dma_chan_remove(struct fsldma_chan *chan)
 
 static int fsldma_of_probe(struct platform_device *op)
 {
+	struct device *dev = &op->dev;
 	struct fsldma_device *fdev;
 	struct device_node *child;
 	unsigned int i;
 	int err;
 
-	fdev = kzalloc_obj(*fdev);
-	if (!fdev) {
-		err = -ENOMEM;
-		goto out_return;
-	}
+	fdev = devm_kzalloc(dev, sizeof(*fdev), GFP_KERNEL);
+	if (!fdev)
+		return -ENOMEM;
 
-	fdev->dev = &op->dev;
+	fdev->dev = dev;
 	INIT_LIST_HEAD(&fdev->common.channels);
 	/* The DMA address bits supported for this device. */
 	fdev->addr_bits = (long)device_get_match_data(fdev->dev);
@@ -1233,8 +1232,7 @@ static int fsldma_of_probe(struct platform_device *op)
 	fdev->regs = of_iomap(op->dev.of_node, 0);
 	if (!fdev->regs) {
 		dev_err(&op->dev, "unable to ioremap registers\n");
-		err = -ENOMEM;
-		goto out_free;
+		return -ENOMEM;
 	}
 
 	/* map the channel IRQ if it exists, but don't hookup the handler yet */
@@ -1313,9 +1311,6 @@ static int fsldma_of_probe(struct platform_device *op)
 	}
 out_iounmap:
 	iounmap(fdev->regs);
-out_free:
-	kfree(fdev);
-out_return:
 	return err;
 }
 
@@ -1335,7 +1330,6 @@ static void fsldma_of_remove(struct platform_device *op)
 	}
 
 	iounmap(fdev->regs);
-	kfree(fdev);
 }
 
 #ifdef CONFIG_PM
-- 
2.54.0

Re: [PATCH 04/10] dmaengine: fsldma: convert to devm_kzalloc and fix error path

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The DMA device object is freed before its reference count reaches zero during unbind, causing a Use-After-Free.
--

commit bb63aa80e614c90735ef23a9a63a8a58c655050a
Author: Rosen Penev <rosenp@gmail.com>

dmaengine: fsldma: convert to devm_kzalloc and fix error path

Convert fdev allocation from kzalloc_obj to devm_kzalloc to simplify
the probe error and remove paths by dropping the explicit kfree.

> diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c
> index 0d28f8299bf88..2efa16d12679a 100644
> --- a/drivers/dma/fsldma.c
> +++ b/drivers/dma/fsldma.c
> @@ -1213,18 +1213,17 @@ static void fsl_dma_chan_remove(struct fsldma_chan *chan)
>  
>  static int fsldma_of_probe(struct platform_device *op)
>  {
> +	struct device *dev = &op->dev;
>  	struct fsldma_device *fdev;

[ ... ]

> @@ -1335,7 +1330,6 @@ static void fsldma_of_remove(struct platform_device *op)
>  	}
>  
>  	iounmap(fdev->regs);
> -	kfree(fdev);
>  }

[Severity: High]
This isn't a bug introduced by this patch, but does relying on devres to
free the fdev structure leave the driver vulnerable to a use-after-free
during device unbind?

The DMA framework uses a reference count (fdev->common.ref) to track the
lifetime of the dma_device structure and requires drivers to provide a
device_release callback to free it only when the count drops to zero.

Since this driver does not provide a device_release callback, devres will
free the fdev memory synchronously immediately after fsldma_of_remove()
completes. If a DMA client still holds a reference to a channel when the
device is unbound via sysfs, the core framework will eventually drop its
reference and access the freed fdev memory, triggering a use-after-free.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260605220134.43295-1-rosenp@gmail.com?part=4

Re: [PATCH 04/10] dmaengine: fsldma: convert to devm_kzalloc and fix error path

[Frank Li]

On Fri, Jun 05, 2026 at 03:01:28PM -0700, Rosen Penev wrote:
>
> Convert fdev allocation from kzalloc_obj to devm_kzalloc to simplify
> the probe error and remove paths by dropping the explicit kfree.
>
> While at it, fix a goto target mismatch introduced in the recent
> platform_get_irq_optional() conversion: goto err_iounmap should
> be goto out_iounmap.

you should fix it at platform_get_irq_optional() conversion patch

Frank
>
> Assisted-by: opencode:big-pickle
> Signed-off-by: Rosen Penev <rosenp@gmail.com>
> ---
>  drivers/dma/fsldma.c | 18 ++++++------------
>  1 file changed, 6 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c
> index 0d28f8299bf8..2efa16d12679 100644
> --- a/drivers/dma/fsldma.c
> +++ b/drivers/dma/fsldma.c
> @@ -1213,18 +1213,17 @@ static void fsl_dma_chan_remove(struct fsldma_chan *chan)
>
>  static int fsldma_of_probe(struct platform_device *op)
>  {
> +       struct device *dev = &op->dev;
>         struct fsldma_device *fdev;
>         struct device_node *child;
>         unsigned int i;
>         int err;
>
> -       fdev = kzalloc_obj(*fdev);
> -       if (!fdev) {
> -               err = -ENOMEM;
> -               goto out_return;
> -       }
> +       fdev = devm_kzalloc(dev, sizeof(*fdev), GFP_KERNEL);
> +       if (!fdev)
> +               return -ENOMEM;
>
> -       fdev->dev = &op->dev;
> +       fdev->dev = dev;
>         INIT_LIST_HEAD(&fdev->common.channels);
>         /* The DMA address bits supported for this device. */
>         fdev->addr_bits = (long)device_get_match_data(fdev->dev);
> @@ -1233,8 +1232,7 @@ static int fsldma_of_probe(struct platform_device *op)
>         fdev->regs = of_iomap(op->dev.of_node, 0);
>         if (!fdev->regs) {
>                 dev_err(&op->dev, "unable to ioremap registers\n");
> -               err = -ENOMEM;
> -               goto out_free;
> +               return -ENOMEM;
>         }
>
>         /* map the channel IRQ if it exists, but don't hookup the handler yet */
> @@ -1313,9 +1311,6 @@ static int fsldma_of_probe(struct platform_device *op)
>         }
>  out_iounmap:
>         iounmap(fdev->regs);
> -out_free:
> -       kfree(fdev);
> -out_return:
>         return err;
>  }
>
> @@ -1335,7 +1330,6 @@ static void fsldma_of_remove(struct platform_device *op)
>         }
>
>         iounmap(fdev->regs);
> -       kfree(fdev);
>  }
>
>  #ifdef CONFIG_PM
> --
> 2.54.0
>

[PATCH 05/10] dmaengine: fsldma: convert ioremap to devm_platform_ioremap_resource

[Rosen Penev]

Convert of_iomap to devm_platform_ioremap_resource to let the devm
framework handle unmapping. This allows removing the out_iounmap
label, out_return label, and the explicit iounmap in both the probe
error path and the remove function.

The DGSR (fdev->regs) and per-channel registers (chan->regs) map
physically distinct regions in all supported variants
(EloPlus/Elo/Elo3), so there is no overlap risk.

Assisted-by: opencode:big-pickle
Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/dma/fsldma.c | 16 +++++-----------
 1 file changed, 5 insertions(+), 11 deletions(-)

diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c
index 2efa16d12679..2a6a247761a4 100644
--- a/drivers/dma/fsldma.c
+++ b/drivers/dma/fsldma.c
@@ -1229,19 +1229,17 @@ static int fsldma_of_probe(struct platform_device *op)
 	fdev->addr_bits = (long)device_get_match_data(fdev->dev);
 
 	/* ioremap the registers for use */
-	fdev->regs = of_iomap(op->dev.of_node, 0);
-	if (!fdev->regs) {
+	fdev->regs = devm_platform_ioremap_resource(op, 0);
+	if (IS_ERR(fdev->regs)) {
 		dev_err(&op->dev, "unable to ioremap registers\n");
-		return -ENOMEM;
+		return PTR_ERR(fdev->regs);
 	}
 
 	/* map the channel IRQ if it exists, but don't hookup the handler yet */
 	fdev->irq = platform_get_irq_optional(op, 0);
 	if (fdev->irq < 0) {
-		if (fdev->irq != -ENXIO) {
-			err = fdev->irq;
-			goto out_iounmap;
-		}
+		if (fdev->irq != -ENXIO)
+			return fdev->irq;
 		fdev->irq = 0;
 	}
 
@@ -1309,8 +1307,6 @@ static int fsldma_of_probe(struct platform_device *op)
 		if (fdev->chan[i])
 			fsl_dma_chan_remove(fdev->chan[i]);
 	}
-out_iounmap:
-	iounmap(fdev->regs);
 	return err;
 }
 
@@ -1328,8 +1324,6 @@ static void fsldma_of_remove(struct platform_device *op)
 		if (fdev->chan[i])
 			fsl_dma_chan_remove(fdev->chan[i]);
 	}
-
-	iounmap(fdev->regs);
 }
 
 #ifdef CONFIG_PM
-- 
2.54.0

Re: [PATCH 05/10] dmaengine: fsldma: convert ioremap to devm_platform_ioremap_resource

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Medium] Commit message inaccurately claims to remove the 'out_return' label.

Pre-existing issues:
- [High] Missing IRQ cleanup in probe error path leads to resource leak and Use-After-Free.
--

commit 377d9333b3cd60b40444ff2c8de7a2822f91747a
Author: Rosen Penev <rosenp@gmail.com>

dmaengine: fsldma: convert ioremap to devm_platform_ioremap_resource

This patch converts of_iomap() to devm_platform_ioremap_resource() in
the probe function of the fsldma driver. It delegates unmapping to the
devm framework and removes the manual unmapping from the error path and
remove function.

> Convert of_iomap to devm_platform_ioremap_resource to let the devm
> framework handle unmapping. This allows removing the out_iounmap
> label, out_return label, and the explicit iounmap in both the probe
> error path and the remove function.

[Severity: Medium]
Does the commit message inaccurately claim to remove the out_return label?

The diff for this patch only removes the out_iounmap label, so it appears
out_return may have been removed in an earlier patch.

> diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c
> index 2efa16d12679a..2a6a247761a47 100644
> --- a/drivers/dma/fsldma.c
> +++ b/drivers/dma/fsldma.c
[ ... ]
> @@ -1309,8 +1307,6 @@ static int fsldma_of_probe(struct platform_device *op)
>  		if (fdev->chan[i])
>  			fsl_dma_chan_remove(fdev->chan[i]);
>  	}

[Severity: High]
This is a pre-existing issue, but does this error path fail to free the
requested IRQs before returning?

If dma_async_device_register() fails during fsldma_of_probe(), the code
jumps to this error path. At this point in the series, the IRQs are managed
via manual request_irq() calls inside fsldma_request_irqs(). However, this
error path fails to call fsldma_free_irqs().

As a result, the IRQ handlers remain registered while the underlying
devres-managed memory (fdev and chan) is automatically freed upon returning.
A subsequently firing interrupt will access freed memory, causing a
use-after-free.

While a later patch in the series converts this to devm_request_irq()
(which fixes the leak), the use-after-free risk persists even at the end
of the series because devres unregisters the IRQ after fsldma_of_probe()
exits, allowing an interrupt to fire and reschedule a killed tasklet.

> -out_iounmap:
> -	iounmap(fdev->regs);
>  	return err;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260605220134.43295-1-rosenp@gmail.com?part=5

Re: [PATCH 05/10] dmaengine: fsldma: convert ioremap to devm_platform_ioremap_resource

[Frank Li]

On Fri, Jun 05, 2026 at 03:01:29PM -0700, Rosen Penev wrote:
>
> Convert of_iomap to devm_platform_ioremap_resource to let the devm
> framework handle unmapping. This allows removing the out_iounmap
> label, out_return label, and the explicit iounmap in both the probe
> error path and the remove function.
>
> The DGSR (fdev->regs) and per-channel registers (chan->regs) map
> physically distinct regions in all supported variants
> (EloPlus/Elo/Elo3), so there is no overlap risk.
>
> Assisted-by: opencode:big-pickle
> Signed-off-by: Rosen Penev <rosenp@gmail.com>
> ---
>  drivers/dma/fsldma.c | 16 +++++-----------
>  1 file changed, 5 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c
> index 2efa16d12679..2a6a247761a4 100644
> --- a/drivers/dma/fsldma.c
> +++ b/drivers/dma/fsldma.c
> @@ -1229,19 +1229,17 @@ static int fsldma_of_probe(struct platform_device *op)
>         fdev->addr_bits = (long)device_get_match_data(fdev->dev);
>
>         /* ioremap the registers for use */
> -       fdev->regs = of_iomap(op->dev.of_node, 0);
> -       if (!fdev->regs) {
> +       fdev->regs = devm_platform_ioremap_resource(op, 0);
> +       if (IS_ERR(fdev->regs)) {
>                 dev_err(&op->dev, "unable to ioremap registers\n");
> -               return -ENOMEM;
> +               return PTR_ERR(fdev->regs);
>         }

devm_platform_ioremap_resource() should print error message, so you can
remove above dev_err() also

Frank

>
>         /* map the channel IRQ if it exists, but don't hookup the handler yet */
>         fdev->irq = platform_get_irq_optional(op, 0);
>         if (fdev->irq < 0) {
> -               if (fdev->irq != -ENXIO) {
> -                       err = fdev->irq;
> -                       goto out_iounmap;
> -               }
> +               if (fdev->irq != -ENXIO)
> +                       return fdev->irq;
>                 fdev->irq = 0;
>         }
>
> @@ -1309,8 +1307,6 @@ static int fsldma_of_probe(struct platform_device *op)
>                 if (fdev->chan[i])
>                         fsl_dma_chan_remove(fdev->chan[i]);
>         }
> -out_iounmap:
> -       iounmap(fdev->regs);
>         return err;
>  }
>
> @@ -1328,8 +1324,6 @@ static void fsldma_of_remove(struct platform_device *op)
>                 if (fdev->chan[i])
>                         fsl_dma_chan_remove(fdev->chan[i]);
>         }
> -
> -       iounmap(fdev->regs);
>  }
>
>  #ifdef CONFIG_PM
> --
> 2.54.0
>

[PATCH 06/10] dmaengine: fsldma: convert channel allocation to devm_kzalloc

[Rosen Penev]

Convert fsl_dma_chan_probe from kzalloc_obj to devm_kzalloc, tying
the channel lifetime to the parent DMA device. This removes the
need for kfree(chan) in both the probe error path and the remove
function.

Assisted-by: opencode:big-pickle
Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/dma/fsldma.c | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c
index 2a6a247761a4..ee6e595c2972 100644
--- a/drivers/dma/fsldma.c
+++ b/drivers/dma/fsldma.c
@@ -1111,11 +1111,9 @@ static int fsl_dma_chan_probe(struct fsldma_device *fdev,
 	int err;
 
 	/* alloc channel */
-	chan = kzalloc_obj(*chan);
-	if (!chan) {
-		err = -ENOMEM;
-		goto out_return;
-	}
+	chan = devm_kzalloc(fdev->dev, sizeof(*chan), GFP_KERNEL);
+	if (!chan)
+		return -ENOMEM;
 
 	/* ioremap registers for use */
 	chan->regs = of_iomap(node, 0);
@@ -1197,9 +1195,6 @@ static int fsl_dma_chan_probe(struct fsldma_device *fdev,
 
 out_iounmap_regs:
 	iounmap(chan->regs);
-out_free_chan:
-	kfree(chan);
-out_return:
 	return err;
 }
 
@@ -1208,7 +1203,6 @@ static void fsl_dma_chan_remove(struct fsldma_chan *chan)
 	tasklet_kill(&chan->tasklet);
 	list_del(&chan->common.device_node);
 	iounmap(chan->regs);
-	kfree(chan);
 }
 
 static int fsldma_of_probe(struct platform_device *op)
-- 
2.54.0

Re: [PATCH 06/10] dmaengine: fsldma: convert channel allocation to devm_kzalloc

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Permanent IRQ mapping leak due to missing `irq_dispose_mapping()` for channels.
--

commit 0a179f7b88544d88df0377a17f9feb95d32cd67e
Author: Rosen Penev <rosenp@gmail.com>

dmaengine: fsldma: convert channel allocation to devm_kzalloc

This commit converts fsl_dma_chan_probe from using kzalloc_obj to
devm_kzalloc. It ties the channel lifetime to the parent DMA device and
removes the need for kfree in the probe error path and the remove function.

> diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c
> --- a/drivers/dma/fsldma.c
> +++ b/drivers/dma/fsldma.c
[ ... ]
> @@ -1208,7 +1203,6 @@ static void fsl_dma_chan_remove(struct fsldma_chan *chan)
>  	tasklet_kill(&chan->tasklet);
>  	list_del(&chan->common.device_node);
>  	iounmap(chan->regs);
> -	kfree(chan);
>  }

[Severity: Medium]
Earlier in the patch series, the call to irq_dispose_mapping() for chan->irq
was removed from fsl_dma_chan_remove().

Although IRQ parsing is updated to use of_irq_get(), that function still
internally allocates an IRQ mapping via irq_create_of_mapping().

Without a matching irq_dispose_mapping() during teardown, does unbinding
the driver permanently leak the IRQ mapping for each channel?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260605220134.43295-1-rosenp@gmail.com?part=6

Re: [PATCH 06/10] dmaengine: fsldma: convert channel allocation to devm_kzalloc

[Frank Li]

On Fri, Jun 05, 2026 at 03:01:30PM -0700, Rosen Penev wrote:
>
> Convert fsl_dma_chan_probe from kzalloc_obj to devm_kzalloc, tying
> the channel lifetime to the parent DMA device. This removes the

Nit: Remove kfree(chan) in ..

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

> need for kfree(chan) in both the probe error path and the remove
> function.
>
> Assisted-by: opencode:big-pickle
> Signed-off-by: Rosen Penev <rosenp@gmail.com>
> ---
>  drivers/dma/fsldma.c | 12 +++---------
>  1 file changed, 3 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c
> index 2a6a247761a4..ee6e595c2972 100644
> --- a/drivers/dma/fsldma.c
> +++ b/drivers/dma/fsldma.c
> @@ -1111,11 +1111,9 @@ static int fsl_dma_chan_probe(struct fsldma_device *fdev,
>         int err;
>
>         /* alloc channel */
> -       chan = kzalloc_obj(*chan);
> -       if (!chan) {
> -               err = -ENOMEM;
> -               goto out_return;
> -       }
> +       chan = devm_kzalloc(fdev->dev, sizeof(*chan), GFP_KERNEL);
> +       if (!chan)
> +               return -ENOMEM;
>
>         /* ioremap registers for use */
>         chan->regs = of_iomap(node, 0);
> @@ -1197,9 +1195,6 @@ static int fsl_dma_chan_probe(struct fsldma_device *fdev,
>
>  out_iounmap_regs:
>         iounmap(chan->regs);
> -out_free_chan:
> -       kfree(chan);
> -out_return:
>         return err;
>  }
>
> @@ -1208,7 +1203,6 @@ static void fsl_dma_chan_remove(struct fsldma_chan *chan)
>         tasklet_kill(&chan->tasklet);
>         list_del(&chan->common.device_node);
>         iounmap(chan->regs);
> -       kfree(chan);
>  }
>
>  static int fsldma_of_probe(struct platform_device *op)
> --
> 2.54.0
>

[PATCH 07/10] dmaengine: fsldma: convert channel ioremap to devm_of_iomap

[Rosen Penev]

Replace of_iomap with devm_of_iomap for per-channel register
mappings. This eliminates the iounmap calls in both the probe
error path and fsl_dma_chan_remove, and simplifies the error
handling by returning directly on failure.

Assisted-by: opencode:big-pickle
Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/dma/fsldma.c | 20 ++++++--------------
 1 file changed, 6 insertions(+), 14 deletions(-)

diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c
index ee6e595c2972..0d73ce3dbfe6 100644
--- a/drivers/dma/fsldma.c
+++ b/drivers/dma/fsldma.c
@@ -1108,7 +1108,6 @@ static int fsl_dma_chan_probe(struct fsldma_device *fdev,
 {
 	struct fsldma_chan *chan;
 	struct resource res;
-	int err;
 
 	/* alloc channel */
 	chan = devm_kzalloc(fdev->dev, sizeof(*chan), GFP_KERNEL);
@@ -1116,17 +1115,16 @@ static int fsl_dma_chan_probe(struct fsldma_device *fdev,
 		return -ENOMEM;
 
 	/* ioremap registers for use */
-	chan->regs = of_iomap(node, 0);
-	if (!chan->regs) {
+	chan->regs = devm_of_iomap(fdev->dev, node, 0, NULL);
+	if (IS_ERR(chan->regs)) {
 		dev_err(fdev->dev, "unable to ioremap registers\n");
-		err = -ENOMEM;
-		goto out_free_chan;
+		return PTR_ERR(chan->regs);
 	}
 
-	err = of_address_to_resource(node, 0, &res);
+	int err = of_address_to_resource(node, 0, &res);
 	if (err) {
 		dev_err(fdev->dev, "unable to find 'reg' property\n");
-		goto out_iounmap_regs;
+		return err;
 	}
 
 	chan->feature = feature;
@@ -1145,8 +1143,7 @@ static int fsl_dma_chan_probe(struct fsldma_device *fdev,
 		   ((res.start - 0x200) & 0xfff) >> 7;
 	if (chan->id >= FSL_DMA_MAX_CHANS_PER_DEVICE) {
 		dev_err(fdev->dev, "too many channels for device\n");
-		err = -EINVAL;
-		goto out_iounmap_regs;
+		return -EINVAL;
 	}
 
 	fdev->chan[chan->id] = chan;
@@ -1192,17 +1189,12 @@ static int fsl_dma_chan_probe(struct fsldma_device *fdev,
 		 chan->irq ? chan->irq : fdev->irq);
 
 	return 0;
-
-out_iounmap_regs:
-	iounmap(chan->regs);
-	return err;
 }
 
 static void fsl_dma_chan_remove(struct fsldma_chan *chan)
 {
 	tasklet_kill(&chan->tasklet);
 	list_del(&chan->common.device_node);
-	iounmap(chan->regs);
 }
 
 static int fsldma_of_probe(struct platform_device *op)
-- 
2.54.0

Re: [PATCH 07/10] dmaengine: fsldma: convert channel ioremap to devm_of_iomap

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Missing `irq_dispose_mapping()` for per-channel IRQs causes a mapping leak when a channel is removed.
- [High] Failure to propagate `-EPROBE_DEFER` and other negative error codes from `of_irq_get()`.
--

commit 6845fba6088aa0e893f716aacd6cf99383012ca1
Author: Rosen Penev <rosenp@gmail.com>

dmaengine: fsldma: convert channel ioremap to devm_of_iomap

Replace of_iomap with devm_of_iomap for per-channel register
mappings. This eliminates the iounmap calls in both the probe
error path and fsl_dma_chan_remove, and simplifies the error
handling by returning directly on failure.

> diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c
> index ee6e595c2972c..0d73ce3dbfe6a 100644
> --- a/drivers/dma/fsldma.c
> +++ b/drivers/dma/fsldma.c

[ ... ]

> @@ -1192,17 +1189,12 @@ static int fsl_dma_chan_probe(struct fsldma_device *fdev,
>  		 chan->irq ? chan->irq : fdev->irq);
>  
>  	return 0;

[Severity: High]
Is there a failure to propagate -EPROBE_DEFER and other negative error codes
from of_irq_get() here?

The commit 'dmaengine: fsldma: replace irq_of_parse_and_map with of_irq_get'
switched to of_irq_get() specifically to handle negative error codes such as
-EPROBE_DEFER.

However, the error handling logic in fsldma_request_irqs() maps all values <=
0 to -ENODEV:

drivers/dma/fsldma.c:fsldma_request_irqs() {
	...
		if (chan->irq <= 0) {
			chan_err(chan, "interrupts property missing in device tree\n");
			return -ENODEV;
		}
	...
}

This forces negative error codes like -EPROBE_DEFER to become -ENODEV,
defeating the purpose of the migration by permanently aborting the probe
instead of deferring it.

> -
> -out_iounmap_regs:
> -	iounmap(chan->regs);
> -	return err;
>  }
>  
>  static void fsl_dma_chan_remove(struct fsldma_chan *chan)
>  {
>  	tasklet_kill(&chan->tasklet);
>  	list_del(&chan->common.device_node);
> -	iounmap(chan->regs);

[Severity: High]
Are we leaking the IRQ mapping when removing the channel?

In an earlier patch in the series ('dmaengine: fsldma: convert to
platform_get_irq_optional()'), the call to irq_dispose_mapping(chan->irq) was
removed from fsl_dma_chan_remove().

However, chan->irq is acquired using of_irq_get() (or previously
irq_of_parse_and_map()), which internally calls irq_create_of_mapping() and
allocates a mapping in the IRQ domain.

This mapping must be explicitly disposed of. Because the cleanup call was
removed and never re-introduced, the IRQ mapping is permanently leaked
whenever a DMA channel is removed or the driver unbinds.

>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260605220134.43295-1-rosenp@gmail.com?part=7

Re: [PATCH 07/10] dmaengine: fsldma: convert channel ioremap to devm_of_iomap

[Frank Li]

On Fri, Jun 05, 2026 at 03:01:31PM -0700, Rosen Penev wrote:
>
> Replace of_iomap with devm_of_iomap for per-channel register

all funcation need (), devm_of_iomap(), please check subject and other
patches

> mappings. This eliminates the iounmap calls in both the probe

Needn't "This", just,  Eeliminate the eliminates()

> error path and fsl_dma_chan_remove, and simplifies the error
> handling by returning directly on failure.

commit message allow 75 chars each line, can you wrap it at 75 to reduce
line number.

>
> Assisted-by: opencode:big-pickle
> Signed-off-by: Rosen Penev <rosenp@gmail.com>
> ---
>  drivers/dma/fsldma.c | 20 ++++++--------------
>  1 file changed, 6 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c
> index ee6e595c2972..0d73ce3dbfe6 100644
> --- a/drivers/dma/fsldma.c
> +++ b/drivers/dma/fsldma.c
> @@ -1108,7 +1108,6 @@ static int fsl_dma_chan_probe(struct fsldma_device *fdev,
>  {
>         struct fsldma_chan *chan;
>         struct resource res;
> -       int err;
>
>         /* alloc channel */
>         chan = devm_kzalloc(fdev->dev, sizeof(*chan), GFP_KERNEL);
> @@ -1116,17 +1115,16 @@ static int fsl_dma_chan_probe(struct fsldma_device *fdev,
>                 return -ENOMEM;
>
>         /* ioremap registers for use */
> -       chan->regs = of_iomap(node, 0);
> -       if (!chan->regs) {
> +       chan->regs = devm_of_iomap(fdev->dev, node, 0, NULL);
> +       if (IS_ERR(chan->regs)) {
>                 dev_err(fdev->dev, "unable to ioremap registers\n");
> -               err = -ENOMEM;
> -               goto out_free_chan;
> +               return PTR_ERR(chan->regs);

dev_err follow return use

return dev_err_probe()

Frank

>         }
>
> -       err = of_address_to_resource(node, 0, &res);
> +       int err = of_address_to_resource(node, 0, &res);
>         if (err) {
>                 dev_err(fdev->dev, "unable to find 'reg' property\n");
> -               goto out_iounmap_regs;
> +               return err;
>         }
>
>         chan->feature = feature;
> @@ -1145,8 +1143,7 @@ static int fsl_dma_chan_probe(struct fsldma_device *fdev,
>                    ((res.start - 0x200) & 0xfff) >> 7;
>         if (chan->id >= FSL_DMA_MAX_CHANS_PER_DEVICE) {
>                 dev_err(fdev->dev, "too many channels for device\n");
> -               err = -EINVAL;
> -               goto out_iounmap_regs;
> +               return -EINVAL;
>         }
>
>         fdev->chan[chan->id] = chan;
> @@ -1192,17 +1189,12 @@ static int fsl_dma_chan_probe(struct fsldma_device *fdev,
>                  chan->irq ? chan->irq : fdev->irq);
>
>         return 0;
> -
> -out_iounmap_regs:
> -       iounmap(chan->regs);
> -       return err;
>  }
>
>  static void fsl_dma_chan_remove(struct fsldma_chan *chan)
>  {
>         tasklet_kill(&chan->tasklet);
>         list_del(&chan->common.device_node);
> -       iounmap(chan->regs);
>  }
>
>  static int fsldma_of_probe(struct platform_device *op)
> --
> 2.54.0
>

[PATCH 08/10] dmaengine: fsldma: replace irq_of_parse_and_map with of_irq_get

[Rosen Penev]

Use of_irq_get which returns a negative error code on failure
instead of silently returning 0. Update the IRQ validation checks
in fsldma_request_irqs from !chan->irq to chan->irq <= 0 to handle
both 0 and negative error returns correctly.

Assisted-by: opencode:big-pickle
Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/dma/fsldma.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c
index 0d73ce3dbfe6..79a268139b9f 100644
--- a/drivers/dma/fsldma.c
+++ b/drivers/dma/fsldma.c
@@ -1067,7 +1067,7 @@ static int fsldma_request_irqs(struct fsldma_device *fdev)
 		if (!chan)
 			continue;
 
-		if (!chan->irq) {
+		if (chan->irq <= 0) {
 			chan_err(chan, "interrupts property missing in device tree\n");
 			ret = -ENODEV;
 			goto out_unwind;
@@ -1090,7 +1090,7 @@ static int fsldma_request_irqs(struct fsldma_device *fdev)
 		if (!chan)
 			continue;
 
-		if (!chan->irq)
+		if (chan->irq <= 0)
 			continue;
 
 		free_irq(chan->irq, chan);
@@ -1180,7 +1180,7 @@ static int fsl_dma_chan_probe(struct fsldma_device *fdev,
 	dma_cookie_init(&chan->common);
 
 	/* find the IRQ line, if it exists in the device tree */
-	chan->irq = irq_of_parse_and_map(node, 0);
+	chan->irq = of_irq_get(node, 0);
 
 	/* Add the channel to DMA device channel list */
 	list_add_tail(&chan->common.device_node, &fdev->common.channels);
-- 
2.54.0

Re: [PATCH 08/10] dmaengine: fsldma: replace irq_of_parse_and_map with of_irq_get

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] The patch replaces `irq_of_parse_and_map()` with `of_irq_get()`, which can correctly return negative error codes such as `-EPROBE_DEFER` when the interrupt controller is not yet ready. However, in `fsldma_request_irqs()`, the updated error handling block catches these negative codes but incorrectly hardcodes the return value to `-ENODEV`. This squashes the actual error code and permanently fails the probe process, breaking probe deferral.

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but DMA client callbacks are invoked with the channel lock (`chan->desc_lock`) held, which causes an AB-BA self-deadlock if the client submits a new transaction from the callback. The DMAengine API explicitly allows clients to submit new transactions from their completion callbacks. When a client does this, `fsl_dma_tx_submit()` is called, which unconditionally attempts to acquire `spin_lock_bh(&chan->desc_lock)`. Since `fsldma_cleanup_descriptors()` already holds this non-recursive spinlock when invoking the callback via `dmaengine_desc_get_callback_invoke()`, the system will deadlock.
--

commit b81f03cbd2cccca3be94e35a5c287310fb30a9de
Author: Rosen Penev <rosenp@gmail.com>

dmaengine: fsldma: replace irq_of_parse_and_map with of_irq_get

This commit replaces irq_of_parse_and_map() with of_irq_get() to handle
negative error codes appropriately. It updates the IRQ validation checks in
fsldma_request_irqs() to handle both 0 and negative error returns correctly.

> diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c
> index 0d73ce3dbfe6a..79a268139b9f8 100644
> --- a/drivers/dma/fsldma.c
> +++ b/drivers/dma/fsldma.c
> @@ -1067,7 +1067,7 @@ static int fsldma_request_irqs(struct fsldma_device *fdev)
>  		if (!chan)
>  			continue;
>  
> -		if (!chan->irq) {
> +		if (chan->irq <= 0) {
>  			chan_err(chan, "interrupts property missing in device tree\n");
>  			ret = -ENODEV;

[Severity: High]
Does overriding the return value here to -ENODEV cause a regression with
probe deferral?

If of_irq_get() returns a valid negative error code like -EPROBE_DEFER
when the interrupt controller is not yet ready, catching it here but
hardcoding the return value to -ENODEV permanently squashes the actual
error code. This will cause the DMA controller to permanently fail to probe
if it initializes before its interrupt controller.

>  			goto out_unwind;
>  		}

[ ... ]

[Severity: High]
This isn't a bug introduced by this patch, but I noticed an existing locking
issue with how DMA client callbacks are invoked.

Looking at fsldma_run_tx_complete_actions(), dmaengine_desc_get_callback_invoke()
is called synchronously without dropping chan->desc_lock.

If a client submits a new transaction from their completion callback, it will
call into fsl_dma_tx_submit() which unconditionally attempts to acquire
spin_lock_bh(&chan->desc_lock). Since this non-recursive spinlock is already
held, this creates an AB-BA self-deadlock.

Could this be updated to drop the lock before invoking the callback, as the
DMAengine API explicitly allows clients to queue new transfers from their
completion callbacks?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260605220134.43295-1-rosenp@gmail.com?part=8

[PATCH 09/10] dmaengine: fsldma: convert to devm_request_irq

[Rosen Penev]

Replace request_irq/free_irq with devm_request_irq, tying IRQ
lifetimes to the parent DMA device. This removes fsldma_free_irqs()
entirely, eliminates the out_unwind error unwind label, and drops
the explicit free_irq call from fsldma_of_remove.

Assisted-by: opencode:big-pickle
Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/dma/fsldma.c | 50 ++++++--------------------------------------
 1 file changed, 6 insertions(+), 44 deletions(-)

diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c
index 79a268139b9f..01c9cd27e795 100644
--- a/drivers/dma/fsldma.c
+++ b/drivers/dma/fsldma.c
@@ -1027,26 +1027,6 @@ static irqreturn_t fsldma_ctrl_irq(int irq, void *data)
 	return IRQ_RETVAL(handled);
 }
 
-static void fsldma_free_irqs(struct fsldma_device *fdev)
-{
-	struct fsldma_chan *chan;
-	int i;
-
-	if (fdev->irq) {
-		dev_dbg(fdev->dev, "free per-controller IRQ\n");
-		free_irq(fdev->irq, fdev);
-		return;
-	}
-
-	for (i = 0; i < FSL_DMA_MAX_CHANS_PER_DEVICE; i++) {
-		chan = fdev->chan[i];
-		if (chan && chan->irq) {
-			chan_dbg(chan, "free per-channel IRQ\n");
-			free_irq(chan->irq, chan);
-		}
-	}
-}
-
 static int fsldma_request_irqs(struct fsldma_device *fdev)
 {
 	struct fsldma_chan *chan;
@@ -1056,9 +1036,8 @@ static int fsldma_request_irqs(struct fsldma_device *fdev)
 	/* if we have a per-controller IRQ, use that */
 	if (fdev->irq) {
 		dev_dbg(fdev->dev, "request per-controller IRQ\n");
-		ret = request_irq(fdev->irq, fsldma_ctrl_irq, IRQF_SHARED,
-				  "fsldma-controller", fdev);
-		return ret;
+		return devm_request_irq(fdev->dev, fdev->irq, fsldma_ctrl_irq,
+				       IRQF_SHARED, "fsldma-controller", fdev);
 	}
 
 	/* no per-controller IRQ, use the per-channel IRQs */
@@ -1069,34 +1048,19 @@ static int fsldma_request_irqs(struct fsldma_device *fdev)
 
 		if (chan->irq <= 0) {
 			chan_err(chan, "interrupts property missing in device tree\n");
-			ret = -ENODEV;
-			goto out_unwind;
+			return -ENODEV;
 		}
 
 		chan_dbg(chan, "request per-channel IRQ\n");
-		ret = request_irq(chan->irq, fsldma_chan_irq, IRQF_SHARED,
-				  "fsldma-chan", chan);
+		ret = devm_request_irq(fdev->dev, chan->irq, fsldma_chan_irq,
+				       IRQF_SHARED, "fsldma-chan", chan);
 		if (ret) {
 			chan_err(chan, "unable to request per-channel IRQ\n");
-			goto out_unwind;
+			return ret;
 		}
 	}
 
 	return 0;
-
-out_unwind:
-	for (/* none */; i >= 0; i--) {
-		chan = fdev->chan[i];
-		if (!chan)
-			continue;
-
-		if (chan->irq <= 0)
-			continue;
-
-		free_irq(chan->irq, chan);
-	}
-
-	return ret;
 }
 
 /*----------------------------------------------------------------------------*/
@@ -1304,8 +1268,6 @@ static void fsldma_of_remove(struct platform_device *op)
 	fdev = platform_get_drvdata(op);
 	dma_async_device_unregister(&fdev->common);
 
-	fsldma_free_irqs(fdev);
-
 	for (i = 0; i < FSL_DMA_MAX_CHANS_PER_DEVICE; i++) {
 		if (fdev->chan[i])
 			fsl_dma_chan_remove(fdev->chan[i]);
-- 
2.54.0

Re: [PATCH 09/10] dmaengine: fsldma: convert to devm_request_irq

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Replacing `request_irq()` with `devm_request_irq()` without adjusting the teardown sequence introduces a fatal use-after-free race condition involving channel tasklets.
--

commit 7b8afa7a0cf79eb3a5fc201866996718ea5d9be8
Author: Rosen Penev <rosenp@gmail.com>

dmaengine: fsldma: convert to devm_request_irq

This commit converts the driver to use devm_request_irq() for managing
interrupt lifetimes. It removes explicit calls to free_irq() and the
fsldma_free_irqs() function.

> diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c
> index 79a268139b9f8..01c9cd27e7951 100644
> --- a/drivers/dma/fsldma.c
> +++ b/drivers/dma/fsldma.c

[ ... ]

> @@ -1304,8 +1268,6 @@ static void fsldma_of_remove(struct platform_device *op)
>  	fdev = platform_get_drvdata(op);
>  	dma_async_device_unregister(&fdev->common);
>  
> -	fsldma_free_irqs(fdev);
> -
[Severity: High]
With the explicit fsldma_free_irqs() call removed, does this leave the IRQ
active while the channels are being removed?

If fsl_dma_chan_remove() is called while the IRQ is still active, it invokes
tasklet_kill(&chan->tasklet). However, if an interrupt fires during or after
this point, the IRQ handler fsldma_chan_irq() can unconditionally call
tasklet_schedule(&chan->tasklet).

Later, devres unwinds allocations by first freeing the IRQ and then
freeing the channel memory. If the locally scheduled tasklet executes
after the channel memory is freed, could this regression result in a
use-after-free and softirq list corruption?

This could also happen in the out_free_fdev error path in
fsldma_of_probe(), which calls fsl_dma_chan_remove() to kill tasklets
while successfully requested devm IRQs are still active.

>  	for (i = 0; i < FSL_DMA_MAX_CHANS_PER_DEVICE; i++) {
>  		if (fdev->chan[i])
>  			fsl_dma_chan_remove(fdev->chan[i]);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260605220134.43295-1-rosenp@gmail.com?part=9

[PATCH 10/10] dmaengine: fsldma: replace ppc-specific accessors with portable generic ones

[Rosen Penev]

- Convert remaining in_be32/in_le32 calls to FSL_DMA_IN macro
- Replace __ilog2 with generic ilog2 (pull in linux/log2.h)
- Add linux/io.h include
- Expand non-PPC accessor support from ARM-only to all architectures
- Guard 64-bit generic accessors with CONFIG_64BIT; provide
  emulation using 32-bit accessors on 32-bit platforms

Add COMPILE_TEST support as a result for extra compile coverage.

Assisted-by: opencode:big-pickle
Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/dma/Kconfig  |  2 +-
 drivers/dma/fsldma.c | 11 ++++++-----
 drivers/dma/fsldma.h | 35 ++++++++++++++++++++++++++++++++---
 3 files changed, 39 insertions(+), 9 deletions(-)

diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
index 302021540d76..9b13e7aa31c7 100644
--- a/drivers/dma/Kconfig
+++ b/drivers/dma/Kconfig
@@ -206,7 +206,7 @@ config EP93XX_DMA
 
 config FSL_DMA
 	tristate "Freescale Elo series DMA support"
-	depends on FSL_SOC
+	depends on FSL_SOC || COMPILE_TEST
 	select DMA_ENGINE
 	select ASYNC_TX_ENABLE_CHANNEL_SWITCH
 	help
diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c
index 01c9cd27e795..a7c1f1b4c9ac 100644
--- a/drivers/dma/fsldma.c
+++ b/drivers/dma/fsldma.c
@@ -32,6 +32,8 @@
 #include <linux/of_address.h>
 #include <linux/of_irq.h>
 #include <linux/platform_device.h>
+#include <linux/io.h>
+#include <linux/log2.h>
 #include <linux/fsldma.h>
 #include "dmaengine.h"
 #include "fsldma.h"
@@ -266,7 +268,7 @@ static void fsl_chan_set_src_loop_size(struct fsldma_chan *chan, int size)
 	case 4:
 	case 8:
 		mode &= ~FSL_DMA_MR_SAHTS_MASK;
-		mode |= FSL_DMA_MR_SAHE | (__ilog2(size) << 14);
+		mode |= FSL_DMA_MR_SAHE | (ilog2(size) << 14);
 		break;
 	}
 
@@ -299,7 +301,7 @@ static void fsl_chan_set_dst_loop_size(struct fsldma_chan *chan, int size)
 	case 4:
 	case 8:
 		mode &= ~FSL_DMA_MR_DAHTS_MASK;
-		mode |= FSL_DMA_MR_DAHE | (__ilog2(size) << 16);
+		mode |= FSL_DMA_MR_DAHE | (ilog2(size) << 16);
 		break;
 	}
 
@@ -326,7 +328,7 @@ static void fsl_chan_set_request_count(struct fsldma_chan *chan, int size)
 
 	mode = get_mr(chan);
 	mode &= ~FSL_DMA_MR_BWC_MASK;
-	mode |= (__ilog2(size) << 24) & FSL_DMA_MR_BWC_MASK;
+	mode |= (ilog2(size) << 24) & FSL_DMA_MR_BWC_MASK;
 
 	set_mr(chan, mode);
 }
@@ -1004,8 +1006,7 @@ static irqreturn_t fsldma_ctrl_irq(int irq, void *data)
 	u32 gsr, mask;
 	int i;
 
-	gsr = (fdev->feature & FSL_DMA_BIG_ENDIAN) ? in_be32(fdev->regs)
-						   : in_le32(fdev->regs);
+	gsr = FSL_DMA_IN(fdev, fdev->regs, 32);
 	mask = 0xff000000;
 	dev_dbg(fdev->dev, "IRQ: gsr 0x%.8x\n", gsr);
 
diff --git a/drivers/dma/fsldma.h b/drivers/dma/fsldma.h
index d7b7a3138b85..01f93123b233 100644
--- a/drivers/dma/fsldma.h
+++ b/drivers/dma/fsldma.h
@@ -232,17 +232,46 @@ static void fsl_iowrite64be(u64 val, u64 __iomem *addr)
 	out_be32((u32 __iomem *)addr + 1, (u32)val);
 }
 #endif
-#endif
-
-#if defined(CONFIG_ARM64) || defined(CONFIG_ARM)
+#else
 #define fsl_ioread32(p)		ioread32(p)
 #define fsl_ioread32be(p)	ioread32be(p)
 #define fsl_iowrite32(v, p)	iowrite32(v, p)
 #define fsl_iowrite32be(v, p)	iowrite32be(v, p)
+
+#ifdef CONFIG_64BIT
 #define fsl_ioread64(p)		ioread64(p)
 #define fsl_ioread64be(p)	ioread64be(p)
 #define fsl_iowrite64(v, p)	iowrite64(v, p)
 #define fsl_iowrite64be(v, p)	iowrite64be(v, p)
+#else
+static inline u64 fsl_ioread64(const u64 __iomem *addr)
+{
+	u32 val_lo = ioread32((u32 __iomem *)addr);
+	u32 val_hi = ioread32((u32 __iomem *)addr + 1);
+
+	return ((u64)val_hi << 32) + val_lo;
+}
+
+static inline void fsl_iowrite64(u64 val, u64 __iomem *addr)
+{
+	iowrite32(val >> 32, (u32 __iomem *)addr + 1);
+	iowrite32((u32)val, (u32 __iomem *)addr);
+}
+
+static inline u64 fsl_ioread64be(const u64 __iomem *addr)
+{
+	u32 val_hi = ioread32be((u32 __iomem *)addr);
+	u32 val_lo = ioread32be((u32 __iomem *)addr + 1);
+
+	return ((u64)val_hi << 32) + val_lo;
+}
+
+static inline void fsl_iowrite64be(u64 val, u64 __iomem *addr)
+{
+	iowrite32be(val >> 32, (u32 __iomem *)addr);
+	iowrite32be((u32)val, (u32 __iomem *)addr + 1);
+}
+#endif
 #endif
 
 #define FSL_DMA_IN(fsl_dma, addr, width)			\
-- 
2.54.0

Re: [PATCH 00/10] dmaengine: fsldma: devm conversion, fixups, and cleanups

[Frank Li]

On Fri, Jun 05, 2026 at 03:01:24PM -0700, Rosen Penev wrote:
>
> Convert the Freescale Elo DMA driver to use managed device resources
> (devm), simplifying probe error handling and the remove path by
> dropping explicit iounmap, kfree, and free_irq calls.
>
> While doing so, fix a few issues uncovered along the way:
>
>   - Kill the channel tasklet before removal to prevent a race with
>     the IRQ handler.
>   - Check the return value of dma_async_device_register() instead
>     of silently returning success.
>   - Replace the powerpc-specific I/O accessors with portable
>     generic ones so the driver can be built on non-powerpc
>     architectures.
>
> Build-tested with LLVM=1 ARCH=powerpc allmodconfig.

Suppose this v2, please add V2 at subject after [PATCH v2 ...]
Add change log here or each patch after --- to show what change in new version

Frank

>
> Rosen Penev (10):
>   dmaengine: fsldma: kill tasklet before removing channel
>   dmaengine: fsldma: check dma_async_device_register() return value
>   dmaengine: fsldma: convert to platform_get_irq_optional()
>   dmaengine: fsldma: convert to devm_kzalloc and fix error path
>   dmaengine: fsldma: convert ioremap to devm_platform_ioremap_resource
>   dmaengine: fsldma: convert channel allocation to devm_kzalloc
>   dmaengine: fsldma: convert channel ioremap to devm_of_iomap
>   dmaengine: fsldma: replace irq_of_parse_and_map with of_irq_get
>   dmaengine: fsldma: convert to devm_request_irq
>   dmaengine: fsldma: replace ppc-specific accessors with portable
>     generic ones
>
>  drivers/dma/Kconfig  |   2 +-
>  drivers/dma/fsldma.c | 139 +++++++++++++------------------------------
>  drivers/dma/fsldma.h |  35 ++++++++++-
>  3 files changed, 76 insertions(+), 100 deletions(-)
>
> --
> 2.54.0
>