[PATCHv3 00/15] dmaengine: fsldma: devm conversion, fixups, and cleanups

[PATCHv3 00/15] dmaengine: fsldma: devm conversion, fixups, and cleanups

[Rosen Penev]

  - 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

v3: even more sashiko fixes
v2: add extra fixes to satisfy sashiko

Rosen Penev (15):
  dmaengine: fsldma: kill tasklet before removing channel
  dmaengine: fsldma: drop desc_lock before invoking client callback
  dmaengine: fsldma: halt DMA engine before freeing resources
  dmaengine: fsldma: provide device_release callback
  dmaengine: fsldma: check dma_async_device_register() return value
  dmaengine: fsldma: fix probe error path not freeing IRQs
  dmaengine: fsldma: fix request_irqs unwind freeing unregistered IRQ
  dmaengine: fsldma: convert to platform_get_irq_optional()
  dmaengine: fsldma: use devm for kzalloc()
  dmaengine: fsldma: use devm_platform_ioremap_resource()
  dmaengine: fsldma: convert channel allocation to devm_kzalloc()
  dmaengine: fsldma: use devm for of_iomap()
  dmaengine: fsldma: replace irq_of_parse_and_map with of_irq_get
  dmaengine: fsldma: replace ppc-specific accessors with portable
    generic ones
  dmaengine: fsldma: fix kernel-doc param names to match function
    signatures

 drivers/dma/Kconfig  |   2 +-
 drivers/dma/fsldma.c | 253 +++++++++++++++++++++++--------------------
 drivers/dma/fsldma.h |  35 +++++-
 3 files changed, 167 insertions(+), 123 deletions(-)

--
2.54.0

[PATCHv3 01/15] 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 freed.

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

[PATCHv3 02/15] dmaengine: fsldma: drop desc_lock before invoking client callback

[Rosen Penev]

fsldma_run_tx_complete_actions() calls dmaengine_desc_get_callback_invoke()
while still holding chan->desc_lock.  If the client submits a new
transaction from their completion callback, fsl_dma_tx_submit()
tries to acquire the same non-recursive spinlock, causing a
self-deadlock.

Fix by extracting the callback info under the lock, removing the
descriptor from ld_running, dropping the lock, then invoking the
callback and running dependencies outside the lock.

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

diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c
index 0e2f84862261..455d21d738de 100644
--- a/drivers/dma/fsldma.c
+++ b/drivers/dma/fsldma.c
@@ -496,16 +496,19 @@ static void fsldma_clean_completed_descriptor(struct fsldma_chan *chan)
 }
 
 /**
- * fsldma_run_tx_complete_actions - cleanup a single link descriptor
+ * fsldma_run_tx_complete_actions - unmap and extract callback from a descriptor
  * @chan: Freescale DMA channel
- * @desc: descriptor to cleanup and free
+ * @desc: descriptor to process
  * @cookie: Freescale DMA transaction identifier
+ * @cb: returned callback information
  *
- * This function is used on a descriptor which has been executed by the DMA
- * controller. It will run any callbacks, submit any dependencies.
+ * Unmap the descriptor if it has been submitted and extract its callback
+ * into @cb.  The caller must invoke the callback and run dependencies
+ * after releasing chan->desc_lock.
  */
 static dma_cookie_t fsldma_run_tx_complete_actions(struct fsldma_chan *chan,
-		struct fsl_desc_sw *desc, dma_cookie_t cookie)
+		struct fsl_desc_sw *desc, dma_cookie_t cookie,
+		struct dmaengine_desc_callback *cb)
 {
 	struct dma_async_tx_descriptor *txd = &desc->async_tx;
 	dma_cookie_t ret = cookie;
@@ -514,49 +517,14 @@ static dma_cookie_t fsldma_run_tx_complete_actions(struct fsldma_chan *chan,
 
 	if (txd->cookie > 0) {
 		ret = txd->cookie;
-
 		dma_descriptor_unmap(txd);
-		/* Run the link descriptor callback function */
-		dmaengine_desc_get_callback_invoke(txd, NULL);
 	}
 
-	/* Run any dependencies */
-	dma_run_dependencies(txd);
+	dmaengine_desc_get_callback(txd, cb);
 
 	return ret;
 }
 
-/**
- * fsldma_clean_running_descriptor - move the completed descriptor from
- * ld_running to ld_completed
- * @chan: Freescale DMA channel
- * @desc: the descriptor which is completed
- *
- * Free the descriptor directly if acked by async_tx api, or move it to
- * queue ld_completed.
- */
-static void fsldma_clean_running_descriptor(struct fsldma_chan *chan,
-		struct fsl_desc_sw *desc)
-{
-	/* Remove from the list of transactions */
-	list_del(&desc->node);
-
-	/*
-	 * the client is allowed to attach dependent operations
-	 * until 'ack' is set
-	 */
-	if (!async_tx_test_ack(&desc->async_tx)) {
-		/*
-		 * Move this descriptor to the list of descriptors which is
-		 * completed, but still awaiting the 'ack' bit to be set.
-		 */
-		list_add_tail(&desc->node, &chan->ld_completed);
-		return;
-	}
-
-	dma_pool_free(chan->desc_pool, desc, desc->async_tx.phys);
-}
-
 /**
  * fsl_chan_xfer_ld_queue - transfer any pending transactions
  * @chan : Freescale DMA channel
@@ -635,22 +603,23 @@ static void fsl_chan_xfer_ld_queue(struct fsldma_chan *chan)
  */
 static void fsldma_cleanup_descriptors(struct fsldma_chan *chan)
 {
-	struct fsl_desc_sw *desc, *_desc;
+	struct fsl_desc_sw *desc;
 	dma_cookie_t cookie = 0;
 	dma_addr_t curr_phys = get_cdar(chan);
 	int seen_current = 0;
 
 	fsldma_clean_completed_descriptor(chan);
 
-	/* Run the callback for each descriptor, in order */
-	list_for_each_entry_safe(desc, _desc, &chan->ld_running, node) {
-		/*
-		 * do not advance past the current descriptor loaded into the
-		 * hardware channel, subsequent descriptors are either in
-		 * process or have not been submitted
-		 */
-		if (seen_current)
-			break;
+	/*
+	 * Take descriptors one at a time from the front of the running
+	 * queue.  We re-read the list each iteration so that we don't
+	 * chase a stale next pointer across the lock-drop below.
+	 */
+	while (!seen_current && !list_empty(&chan->ld_running)) {
+		struct dmaengine_desc_callback cb;
+
+		desc = list_first_entry(&chan->ld_running,
+					struct fsl_desc_sw, node);
 
 		/*
 		 * stop the search if we reach the current descriptor and the
@@ -662,9 +631,42 @@ static void fsldma_cleanup_descriptors(struct fsldma_chan *chan)
 				break;
 		}
 
-		cookie = fsldma_run_tx_complete_actions(chan, desc, cookie);
+		cookie = fsldma_run_tx_complete_actions(chan, desc, cookie, &cb);
 
-		fsldma_clean_running_descriptor(chan, desc);
+		/*
+		 * Remove from the running list before dropping the lock so
+		 * that terminate_all cannot free this descriptor while we
+		 * call into the client below.
+		 */
+		list_del(&desc->node);
+
+		/*
+		 * Prevent dma_run_dependencies() from calling
+		 * fsl_chan_xfer_ld_queue() while we are not holding the
+		 * lock.  That would splice pending descriptors into
+		 * ld_running before they have been completed by hardware.
+		 * fsl_chan_xfer_ld_queue at the end of this function will
+		 * re-evaluate the situation.
+		 */
+		chan->idle = false;
+
+		/*
+		 * Drop the lock before invoking the client callback, since
+		 * the DMAengine API explicitly allows clients to submit new
+		 * transactions from their completion callback.  Otherwise
+		 * we self-deadlock on chan->desc_lock.
+		 */
+		spin_unlock(&chan->desc_lock);
+		dmaengine_desc_callback_invoke(&cb, NULL);
+		dma_run_dependencies(&desc->async_tx);
+		spin_lock(&chan->desc_lock);
+
+		chan->idle = true;
+
+		if (!async_tx_test_ack(&desc->async_tx))
+			list_add_tail(&desc->node, &chan->ld_completed);
+		else
+			dma_pool_free(chan->desc_pool, desc, desc->async_tx.phys);
 	}
 
 	/*
-- 
2.54.0

[PATCHv3 03/15] dmaengine: fsldma: halt DMA engine before freeing resources

[Rosen Penev]

When a channel is released (fsl_dma_free_chan_resources) or the driver is
unbound (fsl_dma_chan_remove), the descriptor pool and channel resources
are freed without stopping the DMA hardware first.  An active transfer
could continue executing in the background, fetching descriptors or
writing data to physical memory pages that have already been freed.

Fix by calling dma_halt() in both paths before cleaning up, matching
the pattern already used in fsl_dma_device_terminate_all().

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

diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c
index 455d21d738de..1ba10d065278 100644
--- a/drivers/dma/fsldma.c
+++ b/drivers/dma/fsldma.c
@@ -748,6 +748,7 @@ static void fsl_dma_free_chan_resources(struct dma_chan *dchan)
 
 	chan_dbg(chan, "free all channel resources\n");
 	spin_lock_bh(&chan->desc_lock);
+	dma_halt(chan);
 	fsldma_cleanup_descriptors(chan);
 	fsldma_free_desc_list(chan, &chan->ld_pending);
 	fsldma_free_desc_list(chan, &chan->ld_running);
@@ -1207,6 +1208,10 @@ static int fsl_dma_chan_probe(struct fsldma_device *fdev,
 
 static void fsl_dma_chan_remove(struct fsldma_chan *chan)
 {
+	spin_lock_bh(&chan->desc_lock);
+	dma_halt(chan);
+	spin_unlock_bh(&chan->desc_lock);
+
 	tasklet_kill(&chan->tasklet);
 	irq_dispose_mapping(chan->irq);
 	list_del(&chan->common.device_node);
-- 
2.54.0

[PATCHv3 04/15] dmaengine: fsldma: provide device_release callback

[Rosen Penev]

The DMA core requires drivers to set dma_device.device_release so that
the container structure is only freed after all references to it have
been dropped (see the comment above dma_async_device_register()).

This driver violated that contract: fdev was devm_kzalloc()'d with no
device_release callback.  If a client still held a channel reference
when the driver was unbound, dma_device_release() would eventually
run on freed memory, causing a use-after-free.

Fix by allocating fdev with kzalloc_obj(), adding
fsldma_device_release() to free it, and setting device_release.
fsldma_of_remove() now saves channel pointers and frees IRQs before
calling dma_async_device_unregister(), since fdev may be freed by
the release callback inside that call.

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

diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c
index 1ba10d065278..43d817f6ded1 100644
--- a/drivers/dma/fsldma.c
+++ b/drivers/dma/fsldma.c
@@ -1219,6 +1219,8 @@ static void fsl_dma_chan_remove(struct fsldma_chan *chan)
 	kfree(chan);
 }
 
+static void fsldma_device_release(struct dma_device *dma_dev);
+
 static int fsldma_of_probe(struct platform_device *op)
 {
 	struct fsldma_device *fdev;
@@ -1257,6 +1259,7 @@ static int fsldma_of_probe(struct platform_device *op)
 	fdev->common.device_issue_pending = fsl_dma_memcpy_issue_pending;
 	fdev->common.device_config = fsl_dma_device_config;
 	fdev->common.device_terminate_all = fsl_dma_device_terminate_all;
+	fdev->common.device_release = fsldma_device_release;
 	fdev->common.dev = &op->dev;
 
 	fdev->common.src_addr_widths = FSL_DMA_BUSWIDTHS;
@@ -1316,19 +1319,33 @@ static int fsldma_of_probe(struct platform_device *op)
 	return err;
 }
 
+static void fsldma_device_release(struct dma_device *dma_dev)
+{
+	struct fsldma_device *fdev = container_of(dma_dev, struct fsldma_device,
+						  common);
+	kfree(fdev);
+}
+
 static void fsldma_of_remove(struct platform_device *op)
 {
-	struct fsldma_device *fdev;
+	struct fsldma_device *fdev = platform_get_drvdata(op);
+	struct fsldma_chan *chans[FSL_DMA_MAX_CHANS_PER_DEVICE];
 	unsigned int i;
 
-	fdev = platform_get_drvdata(op);
-	dma_async_device_unregister(&fdev->common);
+	for (i = 0; i < FSL_DMA_MAX_CHANS_PER_DEVICE; i++)
+		chans[i] = fdev->chan[i];
 
 	fsldma_free_irqs(fdev);
 
+	/*
+	 * fdev may be freed by fsldma_device_release inside this call;
+	 * use saved copies of the channel pointers afterwards.
+	 */
+	dma_async_device_unregister(&fdev->common);
+
 	for (i = 0; i < FSL_DMA_MAX_CHANS_PER_DEVICE; i++) {
-		if (fdev->chan[i])
-			fsl_dma_chan_remove(fdev->chan[i]);
+		if (chans[i])
+			fsl_dma_chan_remove(chans[i]);
 	}
 	irq_dispose_mapping(fdev->irq);
 
-- 
2.54.0

[PATCHv3 05/15] 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 43d817f6ded1..3009e1531292 100644
--- a/drivers/dma/fsldma.c
+++ b/drivers/dma/fsldma.c
@@ -1303,7 +1303,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

[PATCHv3 06/15] dmaengine: fsldma: fix probe error path not freeing IRQs

[Rosen Penev]

If dma_async_device_register() fails after fsldma_request_irqs()
succeeded, the error path jumped to out_free_fdev which only removed
channels but never freed the already-registered IRQs.  A subsequent
interrupt would access freed memory.

Fix by adding an out_free_irqs label that calls fsldma_free_irqs()
before falling through to the existing channel cleanup.

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

diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c
index 3009e1531292..4475d50a94f5 100644
--- a/drivers/dma/fsldma.c
+++ b/drivers/dma/fsldma.c
@@ -1306,10 +1306,12 @@ static int fsldma_of_probe(struct platform_device *op)
 	err = dma_async_device_register(&fdev->common);
 	if (err) {
 		dev_err(fdev->dev, "unable to register DMA device\n");
-		goto out_free_fdev;
+		goto out_free_irqs;
 	}
 	return 0;
 
+out_free_irqs:
+	fsldma_free_irqs(fdev);
 out_free_fdev:
 	for (i = 0; i < FSL_DMA_MAX_CHANS_PER_DEVICE; i++) {
 		if (fdev->chan[i])
-- 
2.54.0

[PATCHv3 07/15] dmaengine: fsldma: fix request_irqs unwind freeing unregistered IRQ

[Rosen Penev]

When fsldma_request_irqs() fails on a per-channel IRQ, the unwind
loop starts at the current index i, which calls free_irq() on the
IRQ that request_irq() just failed to register.  Decrement i before
the loop to skip the failed channel.

Bug introduced by commit 586f54672b33 ("dmaengine: fsldma: convert
to platform_get_irq_optional()").

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

diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c
index 4475d50a94f5..c04a7fbd2ed0 100644
--- a/drivers/dma/fsldma.c
+++ b/drivers/dma/fsldma.c
@@ -1088,7 +1088,7 @@ static int fsldma_request_irqs(struct fsldma_device *fdev)
 	return 0;
 
 out_unwind:
-	for (/* none */; i >= 0; i--) {
+	for (i--; i >= 0; i--) {
 		chan = fdev->chan[i];
 		if (!chan)
 			continue;
-- 
2.54.0

[PATCHv3 08/15] 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 c04a7fbd2ed0..eba194d64105 100644
--- a/drivers/dma/fsldma.c
+++ b/drivers/dma/fsldma.c
@@ -1213,7 +1213,6 @@ static void fsl_dma_chan_remove(struct fsldma_chan *chan)
 	spin_unlock_bh(&chan->desc_lock);
 
 	tasklet_kill(&chan->tasklet);
-	irq_dispose_mapping(chan->irq);
 	list_del(&chan->common.device_node);
 	iounmap(chan->regs);
 	kfree(chan);
@@ -1248,7 +1247,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);
@@ -1317,7 +1323,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);
@@ -1353,7 +1359,6 @@ static void fsldma_of_remove(struct platform_device *op)
 		if (chans[i])
 			fsl_dma_chan_remove(chans[i]);
 	}
-	irq_dispose_mapping(fdev->irq);
 
 	iounmap(fdev->regs);
 	kfree(fdev);
-- 
2.54.0

[PATCHv3 09/15] dmaengine: fsldma: use devm for kzalloc()

[Rosen Penev]

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

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 eba194d64105..dac12de06ef5 100644
--- a/drivers/dma/fsldma.c
+++ b/drivers/dma/fsldma.c
@@ -1222,18 +1222,17 @@ static void fsldma_device_release(struct dma_device *dma_dev);
 
 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);
@@ -1242,8 +1241,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 */
@@ -1325,9 +1323,6 @@ static int fsldma_of_probe(struct platform_device *op)
 	}
 out_iounmap:
 	iounmap(fdev->regs);
-out_free:
-	kfree(fdev);
-out_return:
 	return err;
 }
 
@@ -1361,7 +1356,6 @@ static void fsldma_of_remove(struct platform_device *op)
 	}
 
 	iounmap(fdev->regs);
-	kfree(fdev);
 }
 
 #ifdef CONFIG_PM
-- 
2.54.0

[PATCHv3 10/15] dmaengine: fsldma: use 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 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 | 18 +++++-------------
 1 file changed, 5 insertions(+), 13 deletions(-)

diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c
index dac12de06ef5..e4a3315a7d9d 100644
--- a/drivers/dma/fsldma.c
+++ b/drivers/dma/fsldma.c
@@ -1238,19 +1238,15 @@ 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) {
-		dev_err(&op->dev, "unable to ioremap registers\n");
-		return -ENOMEM;
-	}
+	fdev->regs = devm_platform_ioremap_resource(op, 0);
+	if (IS_ERR(fdev->regs))
+		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;
 	}
 
@@ -1321,8 +1317,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;
 }
 
@@ -1354,8 +1348,6 @@ static void fsldma_of_remove(struct platform_device *op)
 		if (chans[i])
 			fsl_dma_chan_remove(chans[i]);
 	}
-
-	iounmap(fdev->regs);
 }
 
 #ifdef CONFIG_PM
-- 
2.54.0

[PATCHv3 11/15] 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. Remove 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 e4a3315a7d9d..0df09789187d 100644
--- a/drivers/dma/fsldma.c
+++ b/drivers/dma/fsldma.c
@@ -1114,11 +1114,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);
@@ -1200,9 +1198,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;
 }
 
@@ -1215,7 +1210,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 void fsldma_device_release(struct dma_device *dma_dev);
-- 
2.54.0

[PATCHv3 12/15] dmaengine: fsldma: use devm for 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 | 22 ++++++----------------
 1 file changed, 6 insertions(+), 16 deletions(-)

diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c
index 0df09789187d..a3792864f02a 100644
--- a/drivers/dma/fsldma.c
+++ b/drivers/dma/fsldma.c
@@ -1111,7 +1111,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);
@@ -1119,17 +1118,14 @@ 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) {
-		dev_err(fdev->dev, "unable to ioremap registers\n");
-		err = -ENOMEM;
-		goto out_free_chan;
-	}
+	chan->regs = devm_of_iomap(fdev->dev, node, 0, NULL);
+	if (IS_ERR(chan->regs))
+		return dev_err_probe(fdev->dev, PTR_ERR(chan->regs), "unable to ioremap registers\n");
 
-	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;
@@ -1148,8 +1144,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;
@@ -1195,10 +1190,6 @@ 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)
@@ -1209,7 +1200,6 @@ static void fsl_dma_chan_remove(struct fsldma_chan *chan)
 
 	tasklet_kill(&chan->tasklet);
 	list_del(&chan->common.device_node);
-	iounmap(chan->regs);
 }
 
 static void fsldma_device_release(struct dma_device *dma_dev);
-- 
2.54.0

[PATCHv3 13/15] 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. Split the IRQ validation check
in fsldma_request_irqs to handle three cases:

  - chan->irq < 0: propagate the error (e.g. -EPROBE_DEFER)
  - chan->irq == 0: IRQ not found, return -ENODEV
  - chan->irq > 0: valid IRQ, proceed

The fsldma_free_irqs() function's !chan->irq check is unchanged
since both 0 and negative values mean no IRQ to free.

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

diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c
index a3792864f02a..7d0c80121aa4 100644
--- a/drivers/dma/fsldma.c
+++ b/drivers/dma/fsldma.c
@@ -1070,6 +1070,12 @@ static int fsldma_request_irqs(struct fsldma_device *fdev)
 		if (!chan)
 			continue;
 
+		if (chan->irq < 0) {
+			if (chan->irq != -EPROBE_DEFER)
+				chan_err(chan, "interrupts property missing in device tree\n");
+			ret = chan->irq;
+			goto out_unwind;
+		}
 		if (!chan->irq) {
 			chan_err(chan, "interrupts property missing in device tree\n");
 			ret = -ENODEV;
@@ -1093,7 +1099,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);
@@ -1181,7 +1187,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

[PATCHv3 14/15] 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 7d0c80121aa4..d4c9b81ade0d 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);
 }
@@ -1007,8 +1009,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

[PATCHv3 15/15] dmaengine: fsldma: fix kernel-doc param names to match function signatures

[Rosen Penev]

Fix kernel-doc warnings where the documented parameter names
(@chan) no longer match the actual function signatures (@dchan),
and add the missing @cookie and @txstate parameters to
fsl_tx_status.

These are pre-existing mismatches that predate the recent
devm conversion series.

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

diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c
index d4c9b81ade0d..c0dbcd09999e 100644
--- a/drivers/dma/fsldma.c
+++ b/drivers/dma/fsldma.c
@@ -685,7 +685,7 @@ static void fsldma_cleanup_descriptors(struct fsldma_chan *chan)
 
 /**
  * fsl_dma_alloc_chan_resources - Allocate resources for DMA channel.
- * @chan : Freescale DMA channel
+ * @dchan : Freescale DMA channel
  *
  * This function will create a dma pool for descriptor allocation.
  *
@@ -742,7 +742,7 @@ static void fsldma_free_desc_list_reverse(struct fsldma_chan *chan,
 
 /**
  * fsl_dma_free_chan_resources - Free all resources of the channel.
- * @chan : Freescale DMA channel
+ * @dchan : Freescale DMA channel
  */
 static void fsl_dma_free_chan_resources(struct dma_chan *dchan)
 {
@@ -878,7 +878,7 @@ static int fsl_dma_device_config(struct dma_chan *dchan,
 
 /**
  * fsl_dma_memcpy_issue_pending - Issue the DMA start command
- * @chan : Freescale DMA channel
+ * @dchan : Freescale DMA channel
  */
 static void fsl_dma_memcpy_issue_pending(struct dma_chan *dchan)
 {
@@ -891,7 +891,9 @@ static void fsl_dma_memcpy_issue_pending(struct dma_chan *dchan)
 
 /**
  * fsl_tx_status - Determine the DMA status
- * @chan : Freescale DMA channel
+ * @dchan : Freescale DMA channel
+ * @cookie : DMA transaction identifier
+ * @txstate : DMA transaction state
  */
 static enum dma_status fsl_tx_status(struct dma_chan *dchan,
 					dma_cookie_t cookie,
-- 
2.54.0

Re: [PATCHv3 07/15] dmaengine: fsldma: fix request_irqs unwind freeing unregistered IRQ

[sashiko-bot]

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

Pre-existing issues:
- [High] Failure to kill the tasklet before freeing the channel structure leads to a potential Use-After-Free.
--

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

dmaengine: fsldma: fix request_irqs unwind freeing unregistered IRQ

This commit fixes the unwind loop in fsldma_request_irqs() to skip the
failed channel by decrementing the index before the loop. This prevents
freeing an unregistered IRQ when request_irq() fails.

> diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c
> --- a/drivers/dma/fsldma.c
> +++ b/drivers/dma/fsldma.c
> @@ -1085,7 +1085,7 @@ static int fsldma_request_irqs(struct fsldma_device *fdev)
>  	return 0;
>  
>  out_unwind:
> -	for (/* none */; i >= 0; i--) {
> +	for (i--; i >= 0; i--) {

[Severity: High]
This is a pre-existing issue, but does the cleanup process fail to kill
the tasklet before freeing the channel?

If fsldma_request_irqs() fails and triggers this unwind, the error path
eventually leads to fsldma_of_probe() calling fsl_dma_chan_remove() for
the previously initialized channels.

fsl_dma_chan_remove() unmaps registers and frees the channel memory via
kfree(chan), but it omits tasklet_kill(&chan->tasklet):

drivers/dma/fsldma.c:fsl_dma_chan_remove() {
...
	irq_dispose_mapping(chan->irq);
	list_del(&chan->common.device_node);
	iounmap(chan->regs);
	kfree(chan);
}

Since interrupts are shared via IRQF_SHARED, the interrupt handler could
schedule the tasklet at any point after request_irq() is called. If the
tasklet is scheduled just before free_irq() executes during this unwind
path, could it remain pending and run after kfree(chan), resulting in a
use-after-free?

>  		chan = fdev->chan[i];
>  		if (!chan)
>  			continue;

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

Re: [PATCHv3 04/15] dmaengine: fsldma: provide device_release callback

[sashiko-bot]

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

New issues:
- [Critical] The patch introduces a double-free of `fdev` and multiple use-after-free (UAF) vulnerabilities during device teardown by leaving explicit cleanup operations in `fsldma_of_remove` after the device might have been freed.

Pre-existing issues:
- [High] Individual DMA channel structures (`struct fsldma_chan`) are unconditionally destroyed in `fsldma_of_remove()`, causing use-after-free and invalid hardware access if dmaengine clients still hold references.
- [High] The driver frees the DMA channel structure without ensuring its associated tasklet is killed, which can lead to a use-after-free if the tasklet executes after the channel is freed.
--

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

dmaengine: fsldma: provide device_release callback

The DMA core requires drivers to set dma_device.device_release so that
the container structure is only freed after all references to it have
been dropped. This patch allocates fdev with kzalloc_obj, provides the
device_release callback, and saves channel pointers to avoid UAF during
removal.

> diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c
> index 22d62d958abd9..06a6996a53229 100644
> --- a/drivers/dma/fsldma.c
> +++ b/drivers/dma/fsldma.c
> @@ -1211,6 +1211,8 @@ static void fsl_dma_chan_remove(struct fsldma_chan *chan)
>  	kfree(chan);

[Severity: High]
This isn't a bug introduced by this patch, but does this code free the DMA
channel structure without ensuring its associated tasklet is stopped?

While fsldma_free_irqs() prevents new hardware interrupts, an interrupt
that fired previously could have scheduled the tasklet. If the tasklet is
pending in the softirq queue, it will execute dma_do_tasklet() after chan
has been freed, resulting in a use-after-free.

Would it make sense to call tasklet_kill(&chan->tasklet) before freeing
the channel?

>  }
>  

[ ... ]

> @@ -1308,19 +1311,33 @@ static int fsldma_of_probe(struct platform_device *op)
>  	return err;
>  }
>  
> +static void fsldma_device_release(struct dma_device *dma_dev)
> +{
> +	struct fsldma_device *fdev = container_of(dma_dev, struct fsldma_device,
> +						  common);
> +	kfree(fdev);
> +}
> +
>  static void fsldma_of_remove(struct platform_device *op)
>  {
> -	struct fsldma_device *fdev;
> +	struct fsldma_device *fdev = platform_get_drvdata(op);
> +	struct fsldma_chan *chans[FSL_DMA_MAX_CHANS_PER_DEVICE];
>  	unsigned int i;
>  
> -	fdev = platform_get_drvdata(op);
> -	dma_async_device_unregister(&fdev->common);
> +	for (i = 0; i < FSL_DMA_MAX_CHANS_PER_DEVICE; i++)
> +		chans[i] = fdev->chan[i];
>  
>  	fsldma_free_irqs(fdev);
>  
> +	/*
> +	 * fdev may be freed by fsldma_device_release inside this call;
> +	 * use saved copies of the channel pointers afterwards.
> +	 */
> +	dma_async_device_unregister(&fdev->common);
> +
>  	for (i = 0; i < FSL_DMA_MAX_CHANS_PER_DEVICE; i++) {
> -		if (fdev->chan[i])
> -			fsl_dma_chan_remove(fdev->chan[i]);
> +		if (chans[i])
> +			fsl_dma_chan_remove(chans[i]);

[Severity: Critical]
If dma_async_device_unregister() drops the last reference and triggers
fsldma_device_release() to free fdev, does fsl_dma_chan_remove() cause
a use-after-free?

Inside fsl_dma_chan_remove(), list_del(&chan->common.device_node) modifies
the list head linked to fdev->common.channels, which belongs to the newly
freed fdev.

[Severity: High]
This is a pre-existing issue, but does destroying the channel structures
synchronously in fsldma_of_remove() lead to invalid hardware access and
use-after-free if dmaengine clients still hold references?

Since the channel lifetime needs to match the DMA device container, memory
is freed and I/O mapped registers are unmapped here while clients might
still have active references. Could the channel teardown be moved to the
fsldma_device_release() callback instead?

>  	}
>  	irq_dispose_mapping(fdev->irq);

[Severity: Critical]
Does accessing fdev->irq here cause a use-after-free if fdev was already
freed by dma_async_device_unregister() earlier in this function?

The unquoted code immediately following this in fsldma_of_remove() also
proceeds to call iounmap(fdev->regs) and kfree(fdev), which appears to
cause a double-free if the release callback already ran.

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

Re: [PATCHv3 05/15] dmaengine: fsldma: check dma_async_device_register() return value

[sashiko-bot]

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

New issues:
- [High] The newly added error path for `dma_async_device_register()` jumps to the wrong cleanup label, failing to free requested IRQs and causing a potential Use-After-Free.

Pre-existing issues:
- [High] Missing `tasklet_kill()` during channel teardown leads to Use-After-Free if a tasklet is pending.
- [High] Synchronous device removal without waiting for an RCU grace period violates RCU teardown ordering and causes a Use-After-Free.
--

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

dmaengine: fsldma: check dma_async_device_register() return value

This patch checks the return value of dma_async_device_register() in the
probe path and propagates errors instead of silently returning success. It
addresses a potential NULL pointer dereference during device removal if
registration had failed.

> diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c
> index 22d62d958abd9..0927b386ae7f7 100644
> --- a/drivers/dma/fsldma.c
> +++ b/drivers/dma/fsldma.c
[ ... ]
> @@ -1292,7 +1292,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;

[Severity: High]
Does this new error path introduce a regression by skipping the IRQ cleanup?

Looking at fsldma_of_probe(), we previously successfully called
fsldma_request_irqs(), which registers shared interrupt handlers. If
dma_async_device_register() fails, jumping directly to out_free_fdev skips
unregistering these IRQs.

Since the handlers are on a shared interrupt line, an incoming interrupt
from another device could trigger the handler. Will the handler then
dereference the freed fdev or chan pointers, resulting in a use-after-free
regression?

> +	}
>  	return 0;
>  
>  out_free_fdev:

[Severity: High]
This isn't a regression introduced by this patch, but I noticed a potential
issue in fsl_dma_chan_remove() during channel teardown.

drivers/dma/fsldma.c:fsl_dma_chan_remove() {
	irq_dispose_mapping(chan->irq);
	list_del(&chan->common.device_node);
	iounmap(chan->regs);
	kfree(chan);
}

The driver initializes a tasklet for each channel during probe. When
fsldma_of_remove() unregisters the hardware interrupt, it synchronizes with
hardirqs but not with softirqs.

If a tasklet was scheduled right before free_irq() finished, could it still
be pending? Without a call to tasklet_kill() before kfree(chan), could the
delayed tasklet execute dma_do_tasklet and dereference the freed chan
pointer?

[Severity: High]
This is a pre-existing issue, but does synchronous device removal in
fsldma_of_remove() violate RCU teardown ordering?

drivers/dma/fsldma.c:fsldma_of_remove() {
	dma_async_device_unregister(&fdev->common);
	fsldma_free_irqs(fdev);
	...
	kfree(fdev);
}

The call to dma_async_device_unregister() removes the device from the
RCU-protected dma_device_list. However, the memory for fdev is freed
synchronously afterwards.

Could concurrent RCU readers traversing dma_device_list access the device
after it has been freed, causing a use-after-free? Should this wait for an
RCU grace period before freeing?

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

Re: [PATCHv3 01/15] dmaengine: fsldma: kill tasklet before removing channel

[sashiko-bot]

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

Pre-existing issues:
- [High] Unhandled Error Return from dma_async_device_register() leads to silent probe failure and resource leaks.
- [High] Off-by-one error in the fsldma_request_irqs() error unwind path attempts to free the IRQ that just failed to be requested.
- [High] Double unlock in fsldma_suspend_late() error path causes unbalanced spinlocks.
- [High] Client callback is invoked with the channel descriptor spinlock held, which can lead to a self-deadlock.
--

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

dmaengine: fsldma: kill tasklet before removing channel
    
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 freed.

> 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 fsldma_of_probe() ignore the return
value of dma_async_device_register()?

If registration fails due to an IDA allocation error, the probe function
unconditionally returns 0, falsely reporting success to the driver core.
Later, when the device is removed, fsldma_of_remove() will call
dma_async_device_unregister() on a device that was never fully registered.

Can this corrupt IDA states or trigger assertions and kernel panics?

Will allocated resources like memory, IRQs, and iomappings leak because the
error cleanup path is bypassed?

drivers/dma/fsldma.c:fsldma_of_probe() {
    ...
	dma_async_device_register(&fdev->common);
	return 0;
}

[Severity: High]
This is a pre-existing issue, but could the rollback loop in
fsldma_request_irqs() be missing a decrement of 'i' before the first
iteration?

If request_irq() fails for a channel at index 'i', the code jumps to
out_unwind and begins the cleanup loop starting at the exact same index 'i'.
It unconditionally calls free_irq() for this channel. Because request_irq()
just failed, the IRQ was never successfully allocated.

Will calling free_irq() on an unrequested IRQ trigger a kernel warning?

drivers/dma/fsldma.c:fsldma_request_irqs() {
    ...
out_unwind:
	for (/* none */; i >= 0; i--) {
		chan = fdev->chan[i];
		if (!chan)
			continue;

		if (!chan->irq)
			continue;

		free_irq(chan->irq, chan);
	}
}

[Severity: High]
This is a pre-existing issue, but are we double unlocking chan->desc_lock
in the error path of fsldma_suspend_late()?

In fsldma_suspend_late(), the forward loop acquires the lock, updates state,
and releases it. If a subsequent channel is not idle, it jumps to out: and
iterates backward, unlocking all previously processed channels:

drivers/dma/fsldma.c:fsldma_suspend_late() {
    ...
out:
	for (; i >= 0; i--) {
		chan = fdev->chan[i];
		if (!chan)
			continue;
		chan->pm_state = RUNNING;
		spin_unlock_bh(&chan->desc_lock);
	}
}

Since the locks for these earlier channels were already released during the
forward iteration, might this corrupt the lock state and local_bh preemption
count, leading to warnings and potential crashes?

[Severity: High]
This is a pre-existing issue, but is the client callback invoked with the
channel descriptor spinlock held?

In dma_do_tasklet(), chan->desc_lock is acquired before cleaning up completed
descriptors:

drivers/dma/fsldma.c:dma_do_tasklet() {
    ...
	spin_lock(&chan->desc_lock);

	/* the hardware is now idle and ready for more */
	chan->idle = true;

	/* Run all cleanup for descriptors which have been completed */
	fsldma_cleanup_descriptors(chan);
    ...
}

The cleanup chain eventually calls dmaengine_desc_get_callback_invoke()
without dropping the spinlock:

drivers/dma/fsldma.c:fsldma_run_tx_complete_actions() {
    ...
		/* Run the link descriptor callback function */
		dmaengine_desc_get_callback_invoke(txd, NULL);
    ...
}

If the client callback attempts to submit new transfers, it reaches
fsl_dma_tx_submit() which attempts to acquire the same non-recursive lock:

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

Could this lead to a self-deadlock?

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

Re: [PATCHv3 02/15] dmaengine: fsldma: drop desc_lock before invoking client callback

[sashiko-bot]

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

New issues:
- [Critical] Hardware State Corruption due to incorrect `chan->idle` assignment.
- [Critical] Premature completion of active descriptors due to stale `curr_phys` caching.
- [High] Concurrency, callback ordering violation, and completion cookie regression.
- [High] Use-After-Free / NULL pointer dereference of descriptor pool during teardown.

Pre-existing issues:
- [High] Missing `tasklet_kill()` in `fsl_dma_chan_remove` leading to Use-After-Free on `chan`.
--

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

dmaengine: fsldma: drop desc_lock before invoking client callback

This commit modifies the descriptor cleanup routine in the fsldma driver to
extract callback information under a lock, drop the lock, and invoke the client
callback. This is intended to fix a self-deadlock when a client submits a new
transaction from the completion callback.

> diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c
> index 22d62d958abd9..5c8a8fe25617d 100644
> --- a/drivers/dma/fsldma.c
> +++ b/drivers/dma/fsldma.c
[ ... ]
> @@ -603,22 +603,23 @@ static void fsldma_cleanup_descriptors(struct fsldma_chan *chan)
>  {
>  	struct fsl_desc_sw *desc;
>  	dma_cookie_t cookie = 0;
>  	dma_addr_t curr_phys = get_cdar(chan);

[Severity: Critical]
Is it safe to cache curr_phys outside the while loop when dropping the lock
inside the loop?

If the lock is dropped inside the loop to invoke callbacks, a concurrent thread
could enter, process the remaining completed descriptors (including the one
matching this thread's cached curr_phys), and remove them from ld_running.

When the first thread reacquires the lock, its cached curr_phys is now stale and
no longer matches any descriptor in the list. As a result, the loop's safety
break condition:

        if (desc->async_tx.phys == curr_phys) {
            seen_current = 1;

will never be met. The first thread will erroneously process all remaining
descriptors in ld_running as completed, running their callbacks and freeing them
even if the hardware is actively executing them.

>  	int seen_current = 0;
>  
>  	fsldma_clean_completed_descriptor(chan);
>  
[ ... ]
> @@ -662,9 +631,42 @@ static void fsldma_cleanup_descriptors(struct fsldma_chan *chan)
>  				break;
>  		}
>  
> -		cookie = fsldma_run_tx_complete_actions(chan, desc, cookie);
> +		cookie = fsldma_run_tx_complete_actions(chan, desc, cookie, &cb);
>  
> -		fsldma_clean_running_descriptor(chan, desc);
> +		/*
> +		 * Remove from the running list before dropping the lock so
> +		 * that terminate_all cannot free this descriptor while we
> +		 * call into the client below.
> +		 */
> +		list_del(&desc->node);
> +
> +		/*
> +		 * Prevent dma_run_dependencies() from calling
> +		 * fsl_chan_xfer_ld_queue() while we are not holding the
> +		 * lock.  That would splice pending descriptors into
> +		 * ld_running before they have been completed by hardware.
> +		 * fsl_chan_xfer_ld_queue at the end of this function will
> +		 * re-evaluate the situation.
> +		 */
> +		chan->idle = false;
> +
> +		/*
> +		 * Drop the lock before invoking the client callback, since
> +		 * the DMAengine API explicitly allows clients to submit new
> +		 * transactions from their completion callback.  Otherwise
> +		 * we self-deadlock on chan->desc_lock.
> +		 */
> +		spin_unlock(&chan->desc_lock);
> +		dmaengine_desc_callback_invoke(&cb, NULL);
> +		dma_run_dependencies(&desc->async_tx);
> +		spin_lock(&chan->desc_lock);

[Severity: High]
Does dropping the lock here expose the list iteration to concurrent execution?

Since the lock is dropped within the loop over the shared ld_running list,
multiple threads (such as a tasklet and a thread calling fsl_tx_status()) can
concurrently enter fsldma_cleanup_descriptors() and process descriptors
simultaneously.

This could cause callbacks to be invoked concurrently or out of order.

Additionally, each thread updates chan->common.completed_cookie at the end of
the function using their local cookie variable:

    if (cookie > 0)
        chan->common.completed_cookie = cookie;

If a thread processing an older descriptor finishes after a thread processing a
newer descriptor, the completed_cookie will regress to a smaller value, causing
clients waiting for DMA completion to hang.

> +
> +		chan->idle = true;

[Severity: Critical]
Can this unconditional assignment to chan->idle mask the active busy state
of the hardware?

If a descriptor completes, the loop drops the lock, runs the callback,
reacquires the lock, and sets chan->idle = true.

If the next descriptor in the queue is currently being executed by the hardware,
the loop breaks. At this point, chan->idle incorrectly remains true from the
previous iteration, despite the hardware actively processing the current
descriptor.

When fsl_chan_xfer_ld_queue() is subsequently called at the end of the function,
it checks this variable:

drivers/dma/fsldma.c:fsl_chan_xfer_ld_queue() {
    ...
    if (!chan->idle) {
        chan_dbg(chan, "DMA controller still busy\n");
        return;
    }
    ...
}

Because chan->idle is improperly true, fsl_chan_xfer_ld_queue() will bypass its
safety check, inappropriately restart the controller, and overwrite the hardware
registers while the hardware is actively running.

> +
> +		if (!async_tx_test_ack(&desc->async_tx))
> +			list_add_tail(&desc->node, &chan->ld_completed);
> +		else
> +			dma_pool_free(chan->desc_pool, desc, desc->async_tx.phys);

[Severity: High]
Is it possible for chan->desc_pool to be destroyed by concurrent teardown before
we call dma_pool_free() here?

Because chan->desc_lock was dropped earlier to invoke callbacks, this breaks
mutual exclusion with the channel teardown.

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

If fsl_dma_free_chan_resources() runs on another CPU while the lock is dropped
here, it can finish teardown and destroy the pool. When the lock is reacquired
and this code is executed, it will dereference the destroyed (NULL) pool.

>  	}
>  
>  	/*

[Severity: High]
This isn't a bug introduced by this patch, but is there a missing tasklet_kill()
when removing the channel?

drivers/dma/fsldma.c:fsl_dma_chan_remove() {
    ...
    kfree(chan);
    ...
}

If a tasklet is scheduled (e.g., by an interrupt) just before or during channel
removal, the tasklet will execute dma_do_tasklet() after chan has been freed,
dereferencing the freed pointer and causing a use-after-free.

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

Re: [PATCHv3 13/15] dmaengine: fsldma: replace irq_of_parse_and_map with of_irq_get

[sashiko-bot]

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

New issues:
- [High] The patch adds deferred probe support for per-channel IRQs but neglects the per-controller IRQ.
- [Medium] Negative error codes from `of_irq_get()` are incorrectly logged and passed to the IRQ mapping subsystem.

Pre-existing issues:
- [High] Missing `tasklet_kill()` leads to potential use-after-free of the channel structure.
--

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

dmaengine: fsldma: replace irq_of_parse_and_map with of_irq_get
    
Use of_irq_get() which returns a negative error code on failure
instead of silently returning 0. Split the IRQ validation check
in fsldma_request_irqs to handle three cases.

> The fsldma_free_irqs() function's !chan->irq check is unchanged
> since both 0 and negative values mean no IRQ to free.

[Severity: Medium]
Is this description accurate? In C, negative values like -EPROBE_DEFER
evaluate to true. This means !chan->irq will be false for negative error
codes, skipping the intended check, and potentially passing a negative
value to free_irq() or irq_dispose_mapping() later.

> Assisted-by: opencode:big-pickle
> Signed-off-by: Rosen Penev <rosenp@gmail.com>

> diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c
> index 22d62d958abd9..b2259f8f69ab9 100644
> --- a/drivers/dma/fsldma.c
> +++ b/drivers/dma/fsldma.c

[ ... ]

> @@ -1185,7 +1191,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);

[Severity: High]
Does this change miss updating the per-controller IRQ in fsldma_of_probe()?

If irq_of_parse_and_map() remains in fsldma_of_probe() to set fdev->irq:

    fdev->irq = irq_of_parse_and_map(op->dev.of_node, 0);

It will silently return 0 on deferral. The driver then falls back to
checking per-channel IRQs. If they don't exist, probe fails completely
with -EINVAL instead of correctly deferring, preventing deferred probe
from working for the standard per-controller IRQ configuration.

[Severity: Medium]
Will this logging logic incorrectly evaluate negative error codes? In
fsl_dma_chan_probe(), just after this block:

    dev_info(fdev->dev, "#%d (%s), irq %d\n", chan->id, compatible,
             chan->irq ? chan->irq : fdev->irq);

Since chan->irq now stores negative error codes from of_irq_get(), the
ternary operator will evaluate them as true and log the error code
instead of falling back to the controller IRQ.

Additionally, is it safe to unconditionally pass a negative error code to
irq_dispose_mapping() in fsl_dma_chan_remove() if chan->irq failed?

    static void fsl_dma_chan_remove(struct fsldma_chan *chan)
    {
        irq_dispose_mapping(chan->irq);

[Severity: High]
This isn't a bug introduced by this patch, but is it possible for a
tasklet to trigger a use-after-free here?

During teardown, fsl_dma_chan_remove() does:

    static void fsl_dma_chan_remove(struct fsldma_chan *chan)
    {
        irq_dispose_mapping(chan->irq);
        list_del(&chan->common.device_node);
        iounmap(chan->regs);
        kfree(chan);
    }

It frees the channel memory without first calling
tasklet_kill(&chan->tasklet). If a tasklet was scheduled and remains
pending in the softirq queue, it will execute dma_do_tasklet() after the
chan structure has been freed, dereferencing freed memory.

>  
>  	/* Add the channel to DMA device channel list */
>  	list_add_tail(&chan->common.device_node, &fdev->common.channels);

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

Re: [PATCHv3 01/15] dmaengine: fsldma: kill tasklet before removing channel

[Frank Li]

On Tue, Jun 09, 2026 at 03:19:12PM -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 freed.

Nit: please wrap at 75 char

Add tasklet_kill() in fsl_dma_chan_remove() to prevent a race where the
tasklet is scheduled by the IRQ handler and runs after the channel has been
freed.

>
> Assisted-by: opencode:big-pickle
> Signed-off-by: Rosen Penev <rosenp@gmail.com>
> ---

Reviewed-by: Frank Li <Frank.Li@nxp.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: [PATCHv3 12/15] dmaengine: fsldma: use devm for of_iomap()

[Frank Li]

On Tue, Jun 09, 2026 at 03:19:23PM -0700, Rosen Penev wrote:

Nit: subject: dmaengine: fsldma: use devm_of_iomap() to simplify code

> 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 | 22 ++++++----------------
>  1 file changed, 6 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c
> index 0df09789187d..a3792864f02a 100644
> --- a/drivers/dma/fsldma.c
> +++ b/drivers/dma/fsldma.c
> @@ -1111,7 +1111,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);
> @@ -1119,17 +1118,14 @@ 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) {
> -		dev_err(fdev->dev, "unable to ioremap registers\n");
> -		err = -ENOMEM;
> -		goto out_free_chan;
> -	}
> +	chan->regs = devm_of_iomap(fdev->dev, node, 0, NULL);
> +	if (IS_ERR(chan->regs))
> +		return dev_err_probe(fdev->dev, PTR_ERR(chan->regs), "unable to ioremap registers\n");
>
> -	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;

you touch this line,

	if (err)
		return dev_err_proble(...)

Frank
>  	}
>
>  	chan->feature = feature;
> @@ -1148,8 +1144,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;
> @@ -1195,10 +1190,6 @@ 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)
> @@ -1209,7 +1200,6 @@ static void fsl_dma_chan_remove(struct fsldma_chan *chan)
>
>  	tasklet_kill(&chan->tasklet);
>  	list_del(&chan->common.device_node);
> -	iounmap(chan->regs);
>  }
>
>  static void fsldma_device_release(struct dma_device *dma_dev);
> --
> 2.54.0
>

Re: [PATCHv3 09/15] dmaengine: fsldma: use devm for kzalloc()

[Frank Li]

On Tue, Jun 09, 2026 at 03:19:20PM -0700, Rosen Penev wrote:

nit: subject

dmaengine: fsldma: use devm_kzalloc() to simplify code.

> Convert fdev allocation from kzalloc_obj() to devm_kzalloc() to simplify
> the probe error and remove paths by dropping the explicit kfree.
>
> 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 eba194d64105..dac12de06ef5 100644
> --- a/drivers/dma/fsldma.c
> +++ b/drivers/dma/fsldma.c
> @@ -1222,18 +1222,17 @@ static void fsldma_device_release(struct dma_device *dma_dev);
>
>  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;

not big beanfit add dev in this patch.

If you like, create new patch, replace all op->dev with 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);
> @@ -1242,8 +1241,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;

return dev_err_probe()

Frank
>  	}
>
>  	/* map the channel IRQ if it exists, but don't hookup the handler yet */
> @@ -1325,9 +1323,6 @@ static int fsldma_of_probe(struct platform_device *op)
>  	}
>  out_iounmap:
>  	iounmap(fdev->regs);
> -out_free:
> -	kfree(fdev);
> -out_return:
>  	return err;
>  }
>
> @@ -1361,7 +1356,6 @@ static void fsldma_of_remove(struct platform_device *op)
>  	}
>
>  	iounmap(fdev->regs);
> -	kfree(fdev);
>  }
>
>  #ifdef CONFIG_PM
> --
> 2.54.0
>