[PATCH V2 0/8] dmaengine: zynqmp_dma: Fix error paths and follow-up cleanups

[PATCH V2 0/8] dmaengine: zynqmp_dma: Fix error paths and follow-up cleanups

[Golla Nagendra]

This series fixes error-path cleanup issues in alloc/probe flows and
applies follow-up cleanups in the Xilinx ZynqMP DMA driver.

This series fixes error-path cleanup issues in alloc/probe flows and
applies follow-up cleanups in the Xilinx ZynqMP DMA driver.

Patch 1 fixes runtime PM rollback when sw_desc_pool allocation fails
after pm_runtime_resume_and_get().

Patch 2 frees sw_desc_pool when desc_pool_v allocation fails after
sw_desc_pool has already been allocated.

Patch 3 fixes channel probe/remove error handling by preserving
platform_get_irq() errors, initializing/staging chan->irq safely, and
registering the channel node only after IRQ setup succeeds.

Patch 4 updates stale kerneldoc comments to match current behavior.

Patch 5 applies minor whitespace formatting cleanup in
zynqmp_dma_chan_probe().

Patch 6 switches dma-coherent detection to of_dma_is_coherent() instead
of parsing the common device-tree property directly.

Patch 7 rejects zero-length memcpy preparations and documents the return
behavior.

Patch 8 removes an unused define and drops a duplicate interrupt enable
bit from the default IRQ mask.

Changes in v2:
- Split the previous alloc_chan_resources cleanup into two focused
  patches:
  - Patch 1 for runtime PM rollback on sw_desc_pool allocation failure.
  - Patch 2 for sw_desc_pool release on desc_pool_v allocation failure.
- Reworked channel probe failure teardown to use the existing
  free_chan_resources goto path instead of adding direct tasklet kill in
  the outer probe failure block.
- Kept the whitespace-only change as a standalone patch and moved
  dma-coherent handling to a dedicated patch using
  of_dma_is_coherent().
- Updated commit descriptions to match exact per-patch behavior.

Golla Nagendra (8):
  dmaengine: zynqmp_dma: Fix PM rollback on sw_desc_pool alloc failure
  dmaengine: zynqmp_dma: Free sw_desc_pool on desc_pool_v alloc failure
  dmaengine: zynqmp_dma: Fix chan probe/remove error handling
  dmaengine: zynqmp_dma: Fix stale kerneldoc comments
  dmaengine: zynqmp_dma: Fix minor whitespace
  dmaengine: zynqmp_dma: Use of_dma_is_coherent for dma-coherent
  dmaengine: zynqmp_dma: Reject zero-length memcpy transfers
  dmaengine: zynqmp_dma: Remove unused define and duplicate IRQ bit

 drivers/dma/xilinx/zynqmp_dma.c | 73 +++++++++++++++++++++------------
 1 file changed, 46 insertions(+), 27 deletions(-)

-- 
2.44.4

[PATCH V2 1/8] dmaengine: zynqmp_dma: Fix PM rollback on sw_desc_pool alloc failure

[Golla Nagendra]

If sw_desc_pool allocation fails after pm_runtime_resume_and_get()
succeeds in zynqmp_dma_alloc_chan_resources(), the error path returns
without dropping the runtime PM reference.

Route that failure path through err_pm so the runtime PM reference is
balanced before returning the error.

Fixes: 8982d48af36d ("dmaengine: zynqmp_dma: Fix PM reference leak in zynqmp_dma_alloc_chan_resourc()")
Signed-off-by: Golla Nagendra <nagendra.golla@amd.com>
---
changes in v2:
- Reworked the patch so sw_desc_pool allocation failure goes through
  err_pm, ensuring the runtime PM reference is dropped before returning
  the error
- Updated commit description to match the new behavior
---
 drivers/dma/xilinx/zynqmp_dma.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/dma/xilinx/zynqmp_dma.c b/drivers/dma/xilinx/zynqmp_dma.c
index f6a812e49ddc..9f1e69700dd0 100644
--- a/drivers/dma/xilinx/zynqmp_dma.c
+++ b/drivers/dma/xilinx/zynqmp_dma.c
@@ -483,8 +483,10 @@ static int zynqmp_dma_alloc_chan_resources(struct dma_chan *dchan)
 		return ret;
 
 	chan->sw_desc_pool = kzalloc_objs(*desc, ZYNQMP_DMA_NUM_DESCS);
-	if (!chan->sw_desc_pool)
-		return -ENOMEM;
+	if (!chan->sw_desc_pool) {
+		ret = -ENOMEM;
+		goto err_pm;
+	}
 
 	chan->idle = true;
 	chan->desc_free_cnt = ZYNQMP_DMA_NUM_DESCS;
@@ -516,6 +518,10 @@ static int zynqmp_dma_alloc_chan_resources(struct dma_chan *dchan)
 	}
 
 	return ZYNQMP_DMA_NUM_DESCS;
+
+err_pm:
+	pm_runtime_put_autosuspend(chan->dev);
+	return ret;
 }
 
 /**
-- 
2.44.4

Re: [PATCH V2 1/8] dmaengine: zynqmp_dma: Fix PM rollback on sw_desc_pool alloc failure

[sashiko-bot]

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

Pre-existing issues:
- [High] The patch fixes a runtime PM reference leak when `sw_desc_pool` allocation fails, but misses an identical PM reference leak and a memory leak when `desc_pool_v` allocation fails.
- [High] The probe function leaks the PM runtime usage counter on error paths if PM runtime is enabled.
- [High] Dropping the spinlock inside `list_for_each_entry_safe` in `zynqmp_dma_chan_desc_cleanup` exposes the list to concurrent corruption.
--

--- Patch [1]: [PATCH V2 1/8] dmaengine: zynqmp_dma: Fix PM rollback on sw_desc_pool alloc failure ---
commit b97e6284964e1fb9d4c743aeb8e1818fb367a1d3
Author: Golla Nagendra <nagendra.golla@amd.com>

dmaengine: zynqmp_dma: Fix PM rollback on sw_desc_pool alloc failure

If sw_desc_pool allocation fails after pm_runtime_resume_and_get()
succeeds in zynqmp_dma_alloc_chan_resources(), the error path returns
without dropping the runtime PM reference. Route that failure path
through err_pm so the runtime PM reference is balanced before
returning the error.

> diff --git a/drivers/dma/xilinx/zynqmp_dma.c b/drivers/dma/xilinx/zynqmp_dma.c
> index f6a812e49ddca..9f1e69700dd02 100644
> --- a/drivers/dma/xilinx/zynqmp_dma.c
> +++ b/drivers/dma/xilinx/zynqmp_dma.c
> @@ -483,8 +483,10 @@ static int zynqmp_dma_alloc_chan_resources(struct dma_chan *dchan)
>  		return ret;
>  
>  	chan->sw_desc_pool = kzalloc_objs(*desc, ZYNQMP_DMA_NUM_DESCS);
> -	if (!chan->sw_desc_pool)
> -		return -ENOMEM;
> +	if (!chan->sw_desc_pool) {
> +		ret = -ENOMEM;
> +		goto err_pm;
> +	}
>  
>  	chan->idle = true;
>  	chan->desc_free_cnt = ZYNQMP_DMA_NUM_DESCS;

[ ... ]

> @@ -516,6 +518,10 @@ static int zynqmp_dma_alloc_chan_resources(struct dma_chan *dchan)
>  	}
>  
>  	return ZYNQMP_DMA_NUM_DESCS;
> +
> +err_pm:
> +	pm_runtime_put_autosuspend(chan->dev);
> +	return ret;
>  }

[Severity: High]
This is a pre-existing issue, but I noticed another leak in this same
function. If dma_alloc_coherent() fails:

drivers/dma/xilinx/zynqmp_dma.c:zynqmp_dma_alloc_chan_resources() {
    ...
    chan->desc_pool_v = dma_alloc_coherent(...);
    if (!chan->desc_pool_v)
        return -ENOMEM;
    ...
}

Does this path also leak the PM reference and chan->sw_desc_pool since
it returns directly instead of jumping to an error label?

[Severity: High]
This is a pre-existing issue, but the probe function seems to have a similar
PM leak on error paths.

drivers/dma/xilinx/zynqmp_dma.c:zynqmp_dma_probe() {
    ...
    ret = pm_runtime_resume_and_get(zdev->dev);
    ...
err_disable_pm:
    if (!pm_runtime_enabled(zdev->dev))
        zynqmp_dma_runtime_suspend(zdev->dev);
    pm_runtime_disable(zdev->dev);
    return ret;
}

If pm_runtime_resume_and_get() succeeds but a subsequent step fails, we jump
to err_disable_pm. Is the PM runtime usage counter leaked here since there
is no pm_runtime_put_noidle() called before disabling PM?

[Severity: High]
This is a pre-existing issue, but there might be a race condition in
zynqmp_dma_chan_desc_cleanup().

drivers/dma/xilinx/zynqmp_dma.c:zynqmp_dma_chan_desc_cleanup() {
    ...
    list_for_each_entry_safe(desc, next, &chan->done_list, node) {
        ...
        if (dmaengine_desc_callback_valid(&cb)) {
            spin_unlock_irqrestore(&chan->lock, irqflags);
            dmaengine_desc_callback_invoke(&cb, NULL);
            spin_lock_irqsave(&chan->lock, irqflags);
        }
    ...
}

Since the spinlock is temporarily dropped inside the list_for_each_entry_safe
loop, can concurrent modifications to the list invalidate the next pointer
and cause memory corruption when the lock is reacquired?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260814045616.1661199-1-nagendra.golla@amd.com?part=1

[PATCH V2 2/8] dmaengine: zynqmp_dma: Free sw_desc_pool on desc_pool_v alloc failure

[Golla Nagendra]

In zynqmp_dma_alloc_chan_resources(), sw_desc_pool is allocated before
desc_pool_v. If dma_alloc_coherent() for desc_pool_v fails, the function
currently exits through the runtime PM rollback path but leaves the
already allocated sw_desc_pool behind.

Add a dedicated err_free_sw_desc_pool label for the desc_pool_v failure
path, free sw_desc_pool there, then fall through to err_pm for
pm_runtime_put_autosuspend().

This keeps the error unwind symmetric and avoids leaking the software
descriptor pool on coherent descriptor allocation failure.

Fixes: b0cc417c1637 ("dmaengine: Add Xilinx zynqmp dma engine driver support")
Signed-off-by: Golla Nagendra <nagendra.golla@amd.com>
---
changes in v2:
- Split this cleanup from the previous combined alloc_chan_resources patch
- Added err_free_sw_desc_pool label for desc_pool_v allocation failure,
  freeing sw_desc_pool before falling through to err_pm
- Updated commit description to match the exact cleanup path
---
 drivers/dma/xilinx/zynqmp_dma.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/dma/xilinx/zynqmp_dma.c b/drivers/dma/xilinx/zynqmp_dma.c
index 9f1e69700dd0..b7c561280694 100644
--- a/drivers/dma/xilinx/zynqmp_dma.c
+++ b/drivers/dma/xilinx/zynqmp_dma.c
@@ -504,8 +504,10 @@ static int zynqmp_dma_alloc_chan_resources(struct dma_chan *dchan)
 					       (2 * ZYNQMP_DMA_DESC_SIZE(chan) *
 					       ZYNQMP_DMA_NUM_DESCS),
 					       &chan->desc_pool_p, GFP_KERNEL);
-	if (!chan->desc_pool_v)
-		return -ENOMEM;
+	if (!chan->desc_pool_v) {
+		ret = -ENOMEM;
+		goto err_free_sw_desc_pool;
+	}
 
 	for (i = 0; i < ZYNQMP_DMA_NUM_DESCS; i++) {
 		desc = chan->sw_desc_pool + i;
@@ -519,6 +521,9 @@ static int zynqmp_dma_alloc_chan_resources(struct dma_chan *dchan)
 
 	return ZYNQMP_DMA_NUM_DESCS;
 
+err_free_sw_desc_pool:
+	kfree(chan->sw_desc_pool);
+	chan->sw_desc_pool = NULL;
 err_pm:
 	pm_runtime_put_autosuspend(chan->dev);
 	return ret;
-- 
2.44.4

Re: [PATCH V2 2/8] dmaengine: zynqmp_dma: Free sw_desc_pool on desc_pool_v alloc failure

[sashiko-bot]

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

Pre-existing issues:
- [High] The IRQ handler accesses hardware registers without checking if the device is active, violating Runtime PM guidelines.
--

--- Patch [2]: [PATCH V2 2/8] dmaengine: zynqmp_dma: Free sw_desc_pool on desc_pool_v alloc failure ---
commit fa41ac69883ca2ff4e5d238f73142e53f1ec3b68
Author: Golla Nagendra <nagendra.golla@amd.com>
Subject: dmaengine: zynqmp_dma: Free sw_desc_pool on desc_pool_v alloc failure

This commit fixes a memory leak in zynqmp_dma_alloc_chan_resources() by
freeing the allocated sw_desc_pool if the subsequent desc_pool_v coherent
allocation fails. It adds a dedicated error label to ensure the software
descriptor pool is cleaned up before falling through to the runtime PM
rollback path.

> diff --git a/drivers/dma/xilinx/zynqmp_dma.c b/drivers/dma/xilinx/zynqmp_dma.c
> index 9f1e69700dd02..b7c5612806948 100644
> --- a/drivers/dma/xilinx/zynqmp_dma.c
> +++ b/drivers/dma/xilinx/zynqmp_dma.c

[ ... ]

> static irqreturn_t zynqmp_dma_irq_handler(int irq, void *data)
> {
> 	struct zynqmp_dma_chan *chan = (struct zynqmp_dma_chan *)data;
> 	u32 isr, imr, status;
> 	irqreturn_t ret = IRQ_NONE;
> 
> 	isr = readl(chan->regs + ZYNQMP_DMA_ISR);
> 	imr = readl(chan->regs + ZYNQMP_DMA_IMR);

[Severity: High]
This isn't a bug introduced by this patch, but does this interrupt handler
need to check the device's power state before accessing hardware registers?

During runtime suspend, the APB clock is disabled. If a spurious or
unexpected interrupt fires while the device is in this suspended state,
the handler executes and immediately reads ZYNQMP_DMA_ISR without calling
pm_runtime_get_if_active() to verify or resume the power state.

Could reading these registers while the clock is disabled cause a
synchronous external abort and lead to a system crash?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260814045616.1661199-1-nagendra.golla@amd.com?part=2

[PATCH V2 3/8] dmaengine: zynqmp_dma: Fix chan probe/remove error handling

[Golla Nagendra]

Keep the real platform_get_irq() error by returning ret directly, stage
IRQ setup via a local ret variable, and assign chan->irq only after
devm_request_irq() succeeds.

Initialize chan->irq to -1 and initialize chan->common.device_node
before it may be touched by teardown. In channel remove, free IRQ only
for valid IRQ numbers and delete the channel node only when it is linked.

Register the channel node only after successful IRQ setup. On channel
probe failure in zynqmp_dma_probe(), route cleanup through the existing
free_chan_resources teardown path.

Fixes: b0cc417c1637 ("dmaengine: Add Xilinx zynqmp dma engine driver support")
Signed-off-by: Golla Nagendra <nagendra.golla@amd.com>
---
changes in v2:
- Reworked probe failure cleanup to use the existing free_chan_resources teardown path instead of adding direct tasklet kill in the outer probe failure block
- Kept IRQ/list initialization and channel registration ordering fixes in this patch
---
 drivers/dma/xilinx/zynqmp_dma.c | 28 +++++++++++++++++-----------
 1 file changed, 17 insertions(+), 11 deletions(-)

diff --git a/drivers/dma/xilinx/zynqmp_dma.c b/drivers/dma/xilinx/zynqmp_dma.c
index b7c561280694..22b517c57003 100644
--- a/drivers/dma/xilinx/zynqmp_dma.c
+++ b/drivers/dma/xilinx/zynqmp_dma.c
@@ -205,7 +205,7 @@ struct zynqmp_dma_desc_sw {
  * @desc_pool_p: Physical allocated descriptor base
  * @desc_free_cnt: Descriptor available count
  * @dev: The dma device
- * @irq: Channel IRQ
+ * @irq: Linux IRQ number, or -1 when not registered
  * @is_dmacoherent: Tells whether dma operations are coherent or not
  * @tasklet: Cleanup work after irq
  * @idle : Channel status;
@@ -896,10 +896,11 @@ static void zynqmp_dma_chan_remove(struct zynqmp_dma_chan *chan)
 	if (!chan)
 		return;
 
-	if (chan->irq)
+	if (chan->irq >= 0)
 		devm_free_irq(chan->zdev->dev, chan->irq, chan);
 	tasklet_kill(&chan->tasklet);
-	list_del(&chan->common.device_node);
+	if (!list_empty(&chan->common.device_node))
+		list_del(&chan->common.device_node);
 }
 
 /**
@@ -915,13 +916,14 @@ static int zynqmp_dma_chan_probe(struct zynqmp_dma_device *zdev,
 	struct zynqmp_dma_chan *chan;
 	struct device_node *node = pdev->dev.of_node;
 	const struct zynqmp_dma_config *match_data;
-	int err;
+	int err, ret;
 
 	chan = devm_kzalloc(zdev->dev, sizeof(*chan), GFP_KERNEL);
 	if (!chan)
 		return -ENOMEM;
 	chan->dev = zdev->dev;
 	chan->zdev = zdev;
+	chan->irq = -1;
 
 	chan->regs = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(chan->regs))
@@ -954,22 +956,26 @@ static int zynqmp_dma_chan_probe(struct zynqmp_dma_device *zdev,
 	INIT_LIST_HEAD(&chan->pending_list);
 	INIT_LIST_HEAD(&chan->done_list);
 	INIT_LIST_HEAD(&chan->free_list);
+	INIT_LIST_HEAD(&chan->common.device_node);
 
 	dma_cookie_init(&chan->common);
 	chan->common.device = &zdev->common;
-	list_add_tail(&chan->common.device_node, &zdev->common.channels);
-
 	zynqmp_dma_init(chan);
-	chan->irq = platform_get_irq(pdev, 0);
-	if (chan->irq < 0)
-		return -ENXIO;
-	err = devm_request_irq(&pdev->dev, chan->irq, zynqmp_dma_irq_handler, 0,
+	ret = platform_get_irq(pdev, 0);
+	if (ret < 0)
+		return ret;
+
+	err = devm_request_irq(&pdev->dev, ret, zynqmp_dma_irq_handler, 0,
 			       "zynqmp-dma", chan);
 	if (err)
 		return err;
 
+	chan->irq = ret;
+
 	chan->desc_size = sizeof(struct zynqmp_dma_desc_ll);
 	chan->idle = true;
+	list_add_tail(&chan->common.device_node, &zdev->common.channels);
+
 	return 0;
 }
 
@@ -1134,7 +1140,7 @@ static int zynqmp_dma_probe(struct platform_device *pdev)
 	ret = zynqmp_dma_chan_probe(zdev, pdev);
 	if (ret) {
 		dev_err_probe(&pdev->dev, ret, "Probing channel failed\n");
-		goto err_disable_pm;
+		goto free_chan_resources;
 	}
 
 	p->dst_addr_widths = BIT(zdev->chan->bus_width / 8);
-- 
2.44.4

[PATCH V2 4/8] dmaengine: zynqmp_dma: Fix stale kerneldoc comments

[Golla Nagendra]

Correct kerneldoc comments that no longer matched the code.

Signed-off-by: Golla Nagendra <nagendra.golla@amd.com>
Reviewed-by: Frank Li <Frank.Li@nxp.com>
---
Changes in V2:
- No Change
---
 drivers/dma/xilinx/zynqmp_dma.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/dma/xilinx/zynqmp_dma.c b/drivers/dma/xilinx/zynqmp_dma.c
index 22b517c57003..a2fbefbb43b6 100644
--- a/drivers/dma/xilinx/zynqmp_dma.c
+++ b/drivers/dma/xilinx/zynqmp_dma.c
@@ -208,7 +208,7 @@ struct zynqmp_dma_desc_sw {
  * @irq: Linux IRQ number, or -1 when not registered
  * @is_dmacoherent: Tells whether dma operations are coherent or not
  * @tasklet: Cleanup work after irq
- * @idle : Channel status;
+ * @idle: Channel status
  * @desc_size: Size of the low level descriptor
  * @err: Channel has errors
  * @bus_width: Bus width
@@ -435,7 +435,7 @@ zynqmp_dma_get_descriptor(struct zynqmp_dma_chan *chan)
 }
 
 /**
- * zynqmp_dma_free_descriptor - Issue pending transactions
+ * zynqmp_dma_free_descriptor - Return a descriptor to the free pool
  * @chan: ZynqMP DMA channel pointer
  * @sdesc: Transaction descriptor pointer
  */
@@ -617,7 +617,6 @@ static void zynqmp_dma_start_transfer(struct zynqmp_dma_chan *chan)
 	zynqmp_dma_start(chan);
 }
 
-
 /**
  * zynqmp_dma_chan_desc_cleanup - Cleanup the completed descriptors
  * @chan: ZynqMP DMA channel
@@ -908,7 +907,7 @@ static void zynqmp_dma_chan_remove(struct zynqmp_dma_chan *chan)
  * @zdev: Driver specific device structure
  * @pdev: Pointer to the platform_device structure
  *
- * Return: '0' on success and failure value on error
+ * Return: 0 on success and negative error code on failure
  */
 static int zynqmp_dma_chan_probe(struct zynqmp_dma_device *zdev,
 			   struct platform_device *pdev)
@@ -1042,11 +1041,11 @@ static int __maybe_unused zynqmp_dma_runtime_suspend(struct device *dev)
 }
 
 /**
- * zynqmp_dma_runtime_resume - Runtime suspend method for the driver
+ * zynqmp_dma_runtime_resume - Runtime resume method for the driver
  * @dev:	Address of the device structure
  *
- * Put the driver into low power mode.
- * Return: 0 always
+ * Enable device clocks.
+ * Return: 0 on success and failure value on error
  */
 static int __maybe_unused zynqmp_dma_runtime_resume(struct device *dev)
 {
-- 
2.44.4

Re: [PATCH V2 4/8] dmaengine: zynqmp_dma: Fix stale kerneldoc comments

[sashiko-bot]

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

Pre-existing issues:
- [High] Race condition causing list corruption and double-frees in `zynqmp_dma_chan_desc_cleanup()` when client callbacks terminate transfers.
- [High] PM runtime usage counter leak in `zynqmp_dma_probe()` error paths.
--

--- Patch [4]: [PATCH V2 4/8] dmaengine: zynqmp_dma: Fix stale kerneldoc comments ---
commit 50bd84a068af6c47ff8d29181173cde44baa590d
Author: Golla Nagendra <nagendra.golla@amd.com>

dmaengine: zynqmp_dma: Fix stale kerneldoc comments

Correct kerneldoc comments that no longer matched the code.

> diff --git a/drivers/dma/xilinx/zynqmp_dma.c b/drivers/dma/xilinx/zynqmp_dma.c
> index 22b517c570038..a2fbefbb43b65 100644
> --- a/drivers/dma/xilinx/zynqmp_dma.c
> +++ b/drivers/dma/xilinx/zynqmp_dma.c

[ ... ]

> @@ -617,7 +617,6 @@ static void zynqmp_dma_start_transfer(struct zynqmp_dma_chan *chan)
>  	zynqmp_dma_start(chan);
>  }
>  
> -
>  /**
>   * zynqmp_dma_chan_desc_cleanup - Cleanup the completed descriptors
>   * @chan: ZynqMP DMA channel

[Severity: High]
This is a pre-existing issue, but could there be a race condition in
zynqmp_dma_chan_desc_cleanup() if the client callback terminates the transfer?

When iterating over chan->done_list with list_for_each_entry_safe(), the lock
is dropped before invoking the callback:

	spin_unlock_irqrestore(&chan->lock, irqflags);
	dmaengine_desc_callback_invoke(&cb, NULL);
	spin_lock_irqsave(&chan->lock, irqflags);

If the client callback calls dmaengine_terminate_all(), it acquires the lock,
empties the done_list, and moves pending descriptors to the free_list.

When zynqmp_dma_chan_desc_cleanup() resumes, it will continue iterating using
the stale next pointer which now points into free_list. This could lead to a
double-free when zynqmp_dma_free_descriptor() is called.

[ ... ]

[Severity: High]
This is another pre-existing issue, but does zynqmp_dma_probe() leak a PM
runtime usage counter in its error paths?

If initialization fails after pm_runtime_resume_and_get(zdev->dev) succeeds,
the error path jumps to err_disable_pm:

drivers/dma/xilinx/zynqmp_dma.c:zynqmp_dma_probe() {
    ...
err_disable_pm:
	if (!pm_runtime_enabled(zdev->dev))
		zynqmp_dma_runtime_suspend(zdev->dev);
	pm_runtime_disable(zdev->dev);
	return ret;
}

It calls pm_runtime_disable(), but it doesn't seem to call
pm_runtime_put_noidle() to drop the usage counter acquired earlier.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260814045616.1661199-1-nagendra.golla@amd.com?part=4

[PATCH V2 5/8] dmaengine: zynqmp_dma: Fix minor whitespace

[Golla Nagendra]

Add a blank line in zynqmp_dma_chan_probe() to separate channel
initialization from hardware setup.

Signed-off-by: Golla Nagendra <nagendra.golla@amd.com>
---
changes in v2:
- Split whitespace-only cleanup from the kerneldoc patch into this dedicated patch
---
 drivers/dma/xilinx/zynqmp_dma.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/dma/xilinx/zynqmp_dma.c b/drivers/dma/xilinx/zynqmp_dma.c
index a2fbefbb43b6..5e81e0d950e8 100644
--- a/drivers/dma/xilinx/zynqmp_dma.c
+++ b/drivers/dma/xilinx/zynqmp_dma.c
@@ -959,6 +959,7 @@ static int zynqmp_dma_chan_probe(struct zynqmp_dma_device *zdev,
 
 	dma_cookie_init(&chan->common);
 	chan->common.device = &zdev->common;
+
 	zynqmp_dma_init(chan);
 	ret = platform_get_irq(pdev, 0);
 	if (ret < 0)
-- 
2.44.4

[PATCH V2 6/8] dmaengine: zynqmp_dma: Use of_dma_is_coherent for dma-coherent

[Golla Nagendra]

Use of_dma_is_coherent() instead of parsing the common dma-coherent
device-tree property directly.

Signed-off-by: Golla Nagendra <nagendra.golla@amd.com>
---
changes in v2:
- Added as a dedicated patch based on review feedback to use of_dma_is_coherent() 
  for the common dma-coherent property
---
 drivers/dma/xilinx/zynqmp_dma.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/dma/xilinx/zynqmp_dma.c b/drivers/dma/xilinx/zynqmp_dma.c
index 5e81e0d950e8..194abfd6cb25 100644
--- a/drivers/dma/xilinx/zynqmp_dma.c
+++ b/drivers/dma/xilinx/zynqmp_dma.c
@@ -12,6 +12,7 @@
 #include <linux/io.h>
 #include <linux/module.h>
 #include <linux/of.h>
+#include <linux/of_address.h>
 #include <linux/of_dma.h>
 #include <linux/platform_device.h>
 #include <linux/slab.h>
@@ -947,7 +948,7 @@ static int zynqmp_dma_chan_probe(struct zynqmp_dma_device *zdev,
 	if (match_data)
 		chan->irq_offset = match_data->offset;
 
-	chan->is_dmacoherent =  of_property_read_bool(node, "dma-coherent");
+	chan->is_dmacoherent = of_dma_is_coherent(node);
 	zdev->chan = chan;
 	tasklet_setup(&chan->tasklet, zynqmp_dma_do_tasklet);
 	spin_lock_init(&chan->lock);
-- 
2.44.4

Re: [PATCH V2 6/8] dmaengine: zynqmp_dma: Use of_dma_is_coherent for dma-coherent

[sashiko-bot]

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

Pre-existing issues:
- [High] Missing hardware context restore on system resume leads to DMA data corruption.
- [High] DMA descriptor structures lack explicit little-endian types, breaking on big-endian kernels.
--

--- Patch [6]: [PATCH V2 6/8] dmaengine: zynqmp_dma: Use of_dma_is_coherent for dma-coherent ---
commit 85ed10a27c3d9299f4dbcb91c70a55052ad9638d
Author: Golla Nagendra <nagendra.golla@amd.com>

dmaengine: zynqmp_dma: Use of_dma_is_coherent for dma-coherent

Use of_dma_is_coherent() instead of parsing the common dma-coherent
device-tree property directly.

> diff --git a/drivers/dma/xilinx/zynqmp_dma.c b/drivers/dma/xilinx/zynqmp_dma.c
> index 5e81e0d950e8f..194abfd6cb25d 100644
> --- a/drivers/dma/xilinx/zynqmp_dma.c
> +++ b/drivers/dma/xilinx/zynqmp_dma.c

[ ... ]

> @@ -947,7 +948,7 @@ static int zynqmp_dma_chan_probe(struct zynqmp_dma_device *zdev,
>  	if (match_data)
>  		chan->irq_offset = match_data->offset;
>  
> -	chan->is_dmacoherent =  of_property_read_bool(node, "dma-coherent");
> +	chan->is_dmacoherent = of_dma_is_coherent(node);

[Severity: High]
This is a pre-existing issue, but does the system resume path fail to
restore the DMA hardware register context?

A system suspend-to-RAM cycle causes the PM domain to power off, which
wipes the DMA register context. Upon resume, zynqmp_dma_resume() calls
pm_runtime_force_resume():

drivers/dma/xilinx/zynqmp_dma.c:zynqmp_dma_resume() {
    ...
    if (!device_may_wakeup(dev))
        return pm_runtime_force_resume(dev);
    ...
}

This re-enables clocks but doesn't call zynqmp_dma_init() to restore
critical registers like ZYNQMP_DMA_DSCR_ATTR and ZYNQMP_DMA_DATA_ATTR.

Will subsequent DMA transfers proceed with non-coherent hardware defaults
and silently bypass CPU cache snooping?


[Severity: High]
This is also a pre-existing issue, but are the hardware descriptor fields
missing explicit little-endian types?

The hardware descriptor layout uses native C types instead of __le32 and
__le64:

struct zynqmp_dma_desc_ll {
    u64 addr;
    u32 size;
    u32 ctrl;
    u64 nxtdscraddr;
    u64 rsvd;
};

When the driver is built on a big-endian architecture (like ARM64 with
CPU_BIG_ENDIAN), the native assignments in zynqmp_dma_config_sg_ll_desc()
will store values in big-endian format:

drivers/dma/xilinx/zynqmp_dma.c:zynqmp_dma_config_sg_ll_desc() {
    ...
    sdesc->size = ddesc->size = len;
    sdesc->addr = src;
    ddesc->addr = dst;
    ...
}

Could the little-endian ZynqMP DMA hardware misinterpret these swapped
sizes and addresses, leading to corrupted DMA transfers and memory corruption?

>  	zdev->chan = chan;
>  	tasklet_setup(&chan->tasklet, zynqmp_dma_do_tasklet);
>  	spin_lock_init(&chan->lock);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260814045616.1661199-1-nagendra.golla@amd.com?part=6

[PATCH V2 7/8] dmaengine: zynqmp_dma: Reject zero-length memcpy transfers

[Golla Nagendra]

Zero-length prep_memcpy() needlessly consumed a descriptor slot. Reject
zero-length memcpy at prep time and document the behaviour.

Signed-off-by: Golla Nagendra <nagendra.golla@amd.com>
---
Changes in V2:
- No change
---
 drivers/dma/xilinx/zynqmp_dma.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/dma/xilinx/zynqmp_dma.c b/drivers/dma/xilinx/zynqmp_dma.c
index 194abfd6cb25..921d3d9f6ed6 100644
--- a/drivers/dma/xilinx/zynqmp_dma.c
+++ b/drivers/dma/xilinx/zynqmp_dma.c
@@ -837,7 +837,8 @@ static void zynqmp_dma_synchronize(struct dma_chan *dchan)
  * @len: Transfer length
  * @flags: transfer ack flags
  *
- * Return: Async transaction descriptor on success and NULL on failure
+ * Return: Async transaction descriptor on success and NULL on failure or
+ *	   zero length transfer
  */
 static struct dma_async_tx_descriptor *zynqmp_dma_prep_memcpy(
 				struct dma_chan *dchan, dma_addr_t dma_dst,
@@ -852,6 +853,9 @@ static struct dma_async_tx_descriptor *zynqmp_dma_prep_memcpy(
 
 	chan = to_chan(dchan);
 
+	if (!len)
+		return NULL;
+
 	desc_cnt = DIV_ROUND_UP(len, ZYNQMP_DMA_MAX_TRANS_LEN);
 
 	spin_lock_irqsave(&chan->lock, irqflags);
-- 
2.44.4

[PATCH V2 8/8] dmaengine: zynqmp_dma: Remove unused define and duplicate IRQ bit

[Golla Nagendra]

Remove the unused ZYNQMP_DMA_SRC_ISSUE_RST_VAL define.
ZYNQMP_DMA_DST_DSCR_DONE was also set twice in the default interrupt enable
mask; remove the duplicate bit.

Signed-off-by: Golla Nagendra <nagendra.golla@amd.com>
Reviewed-by: Frank Li <Frank.Li@nxp.com>
---
Changesin V2:
- No Change
---
 drivers/dma/xilinx/zynqmp_dma.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/dma/xilinx/zynqmp_dma.c b/drivers/dma/xilinx/zynqmp_dma.c
index 921d3d9f6ed6..b2a9b60f3052 100644
--- a/drivers/dma/xilinx/zynqmp_dma.c
+++ b/drivers/dma/xilinx/zynqmp_dma.c
@@ -113,8 +113,7 @@
 #define ZYNQMP_DMA_INT_DONE	(ZYNQMP_DMA_DONE | ZYNQMP_DMA_DST_DSCR_DONE)
 #define ZYNQMP_DMA_INT_EN_DEFAULT_MASK	(ZYNQMP_DMA_INT_DONE | \
 					ZYNQMP_DMA_INT_ERR | \
-					ZYNQMP_DMA_INT_OVRFL | \
-					ZYNQMP_DMA_DST_DSCR_DONE)
+					ZYNQMP_DMA_INT_OVRFL)
 
 /* Max number of descriptors per channel */
 #define ZYNQMP_DMA_NUM_DESCS	32
@@ -129,8 +128,6 @@
 /* Reset values for data attributes */
 #define ZYNQMP_DMA_AXCACHE_VAL		0xF
 
-#define ZYNQMP_DMA_SRC_ISSUE_RST_VAL	0x1F
-
 #define ZYNQMP_DMA_IDS_DEFAULT_MASK	0xFFF
 
 /* Bus width in bits */
-- 
2.44.4