[PATCH] dmaengine: dw-axi-dmac: fix clock imbalance in resume error path

[PATCH] dmaengine: dw-axi-dmac: fix clock imbalance in resume error path

[Chaithanya Lagisetty]

axi_dma_resume() enables cfgr_clk and then core_clk. If enabling
core_clk fails, the function returns the error without disabling
cfgr_clk, which was already enabled, leaving the clock refcount
unbalanced.

Disable cfgr_clk on the core_clk enable failure path.

Fixes: 1fe20f1b8454 ("dmaengine: Introduce DW AXI DMAC driver")
Signed-off-by: Chaithanya Lagisetty <nagachaithanya9911@gmail.com>
---
 drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c
index bcefaff03b5c..9db8993e0bc6 100644
--- a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c
+++ b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c
@@ -1335,8 +1335,10 @@ static int axi_dma_resume(struct axi_dma_chip *chip)
 		return ret;
 
 	ret = clk_prepare_enable(chip->core_clk);
-	if (ret < 0)
+	if (ret < 0) {
+		clk_disable_unprepare(chip->cfgr_clk);
 		return ret;
+	}
 
 	axi_dma_enable(chip);
 	axi_dma_irq_enable(chip);
-- 
2.43.0

Re: [PATCH] dmaengine: dw-axi-dmac: fix clock imbalance in resume error path

[sashiko-bot]

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

Pre-existing issues:
- [High] The driver requests a shared interrupt before device clocks are enabled and channel registers are initialized, risking a kernel panic if a shared interrupt fires early.
- [High] Unbalanced PM runtime usage counter and hardware clock leaks on `dw_probe` error paths.
- [High] `dw_remove` leaks clock reference counts if the device was runtime active, leaving clocks permanently enabled.
- [High] Ignored return values from `clk_prepare_enable()` in `dw_remove()` can lead to fatal bus faults.
- [High] The driver shuts down hardware and kills tasklets in `dw_remove()` before unregistering the DMA engine, exposing it to fatal race conditions.
- [High] Bypassing dmaengine channel unregistration in `dw_remove()` causes resource leaks.
--

--- Patch [1]: [PATCH] dmaengine: dw-axi-dmac: fix clock imbalance in resume error path ---
commit c0653e2ca540837590c802ab80fc07b1689246e9
Author: Chaithanya Lagisetty <nagachaithanya9911@gmail.com>

dmaengine: dw-axi-dmac: fix clock imbalance in resume error path

This commit addresses a clock imbalance in the resume error path of the
dw-axi-dmac driver. It ensures the configuration clock is properly
disabled if the core clock fails to enable.

> diff --git a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c
> index bcefaff03b5c6..9db8993e0bc6a 100644
> --- a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c
> +++ b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c
> @@ -1335,8 +1335,10 @@ static int axi_dma_resume(struct axi_dma_chip *chip)
>  		return ret;
>  
>  	ret = clk_prepare_enable(chip->core_clk);
> -	if (ret < 0)
> +	if (ret < 0) {
> +		clk_disable_unprepare(chip->cfgr_clk);
>  		return ret;
> +	}
>  
>  	axi_dma_enable(chip);
>  	axi_dma_irq_enable(chip);

[Severity: High]
This is a pre-existing issue, but does this sequence in dw_probe() risk a
NULL pointer dereference if a shared interrupt fires early?

dw_probe() {
    ...
    ret = axi_req_irqs(pdev, chip);
    ...
}

Interrupts are requested here before chan_regs initialization and clock
enablement. If an interrupt fires early, dw_axi_dma_interrupt() calls
axi_dma_irq_disable() which accesses unclocked registers and subsequently
dereferences chan->chan_regs.

[Severity: High]
This is a pre-existing issue, but does dw_probe() leave the PM runtime
usage count unbalanced on error paths?

dw_probe() {
    ...
    goto err_pm_disable;
    ...
err_pm_disable:
    ...
}

If axi_dma_resume() fails, the execution jumps to err_pm_disable but fails
to call pm_runtime_put_noidle(). If dmaenginem_async_device_register()
fails, it jumps to err_pm_disable calling pm_runtime_disable() before the
scheduled pm_runtime_put() executes, leaving hardware clocks permanently
enabled.

[Severity: High]
This is a pre-existing issue, but does dw_remove() leak clock reference
counts if the device was runtime active?

dw_remove() {
    ...
    clk_prepare_enable(chip->cfgr_clk);
    clk_prepare_enable(chip->core_clk);
    ...
    axi_dma_suspend(chip);
    ...
}

If the device is active, PM runtime holds a clock reference. Calling
clk_prepare_enable() unconditionally increments it. axi_dma_suspend()
decrements it, but pm_runtime_disable() prevents PM runtime from dropping
the remaining reference, permanently leaving the clocks enabled.

[Severity: High]
This is a pre-existing issue, but can ignoring these return values lead to
fatal bus faults?

dw_remove() {
    ...
    clk_prepare_enable(chip->cfgr_clk);
    clk_prepare_enable(chip->core_clk);
    ...
}

If clock enablement fails here, the driver proceeds to access hardware
registers via axi_dma_irq_disable() and axi_chan_disable(), which will
trigger a fatal bus error.

[Severity: High]
This is a pre-existing issue, but does this tear down the hardware while
the DMA engine is still fully registered and accessible to clients?

dw_remove() {
    ...
    list_del(&chan->vc.chan.device_node);
    tasklet_kill(&chan->vc.task);
    ...
}

Since dw_probe() uses dmaenginem_async_device_register(), the actual DMA
engine unregistration is deferred until after dw_remove() returns. The
hardware is aggressively shut down here while clients can still submit
transactions, potentially causing use-after-free or bus faults.

[Severity: High]
This is a pre-existing issue, but does bypassing dmaengine channel
unregistration cause resource leaks?

dw_remove() {
    ...
    list_del(&chan->vc.chan.device_node);
    ...
}

Manually emptying the channel list breaks the subsequent deferred devres
unregistration loop. When dma_async_device_unregister() finally runs, the
list is empty, preventing __dma_async_device_channel_unregister() from
being called and leaking sysfs device nodes and IDAs for every channel.

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

Re: [PATCH] dmaengine: dw-axi-dmac: fix clock imbalance in resume error path

[Frank Li]

On Thu, Aug 13, 2026 at 10:54:32AM +0000, Chaithanya Lagisetty wrote:
> [You don't often get email from nagachaithanya9911@gmail.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
>
> axi_dma_resume() enables cfgr_clk and then core_clk. If enabling
> core_clk fails, the function returns the error without disabling
> cfgr_clk, which was already enabled, leaving the clock refcount
> unbalanced.
>
> Disable cfgr_clk on the core_clk enable failure path.
>
> Fixes: 1fe20f1b8454 ("dmaengine: Introduce DW AXI DMAC driver")
> Signed-off-by: Chaithanya Lagisetty <nagachaithanya9911@gmail.com>
> ---
>  drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c
> index bcefaff03b5c..9db8993e0bc6 100644
> --- a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c
> +++ b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c
> @@ -1335,8 +1335,10 @@ static int axi_dma_resume(struct axi_dma_chip *chip)
>                 return ret;
>
>         ret = clk_prepare_enable(chip->core_clk);
> -       if (ret < 0)
> +       if (ret < 0) {
> +               clk_disable_unprepare(chip->cfgr_clk);
>                 return ret;
> +       }

convert driver by use clk bulk API.

Frank

>
>         axi_dma_enable(chip);
>         axi_dma_irq_enable(chip);
> --
> 2.43.0
>

[PATCH v2] dmaengine: dw-axi-dmac: convert clock handling to clk_bulk API

[Chaithanya Lagisetty]

The driver managed its two mandatory clocks (core-clk and cfgr-clk)
individually. This was error prone: axi_dma_resume() enabled cfgr_clk
and then core_clk, and if enabling core_clk failed it returned the
error without disabling cfgr_clk, leaving the clock refcount
unbalanced.

Convert the driver to the clk_bulk API. The two clocks are always
acquired, enabled and disabled together, so a clk_bulk_data array
expresses this naturally and shrinks the get/enable/disable paths.
clk_bulk_prepare_enable() also unwinds any clock it already enabled
when a later one fails, which fixes the resume imbalance.

Fixes: 1fe20f1b8454 ("dmaengine: Introduce DW AXI DMAC driver")
Signed-off-by: Chaithanya Lagisetty <nagachaithanya9911@gmail.com>
---
Changes since v1:
- Convert the driver to the clk_bulk API instead of manually disabling
  cfgr_clk on the error path, as suggested by Frank Li.
- v1: https://lore.kernel.org/all/20260813105432.2577322-1-nagachaithanya9911@gmail.com/

 .../dma/dw-axi-dmac/dw-axi-dmac-platform.c    | 28 ++++++++-----------
 drivers/dma/dw-axi-dmac/dw-axi-dmac.h         |  3 +-
 2 files changed, 13 insertions(+), 18 deletions(-)

diff --git a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c
index bcefaff03b5c..254167a558ff 100644
--- a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c
+++ b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c
@@ -1320,8 +1320,7 @@ static int axi_dma_suspend(struct axi_dma_chip *chip)
 	axi_dma_irq_disable(chip);
 	axi_dma_disable(chip);
 
-	clk_disable_unprepare(chip->core_clk);
-	clk_disable_unprepare(chip->cfgr_clk);
+	clk_bulk_disable_unprepare(ARRAY_SIZE(chip->clks), chip->clks);
 
 	return 0;
 }
@@ -1330,11 +1329,7 @@ static int axi_dma_resume(struct axi_dma_chip *chip)
 {
 	int ret;
 
-	ret = clk_prepare_enable(chip->cfgr_clk);
-	if (ret < 0)
-		return ret;
-
-	ret = clk_prepare_enable(chip->core_clk);
+	ret = clk_bulk_prepare_enable(ARRAY_SIZE(chip->clks), chip->clks);
 	if (ret < 0)
 		return ret;
 
@@ -1524,13 +1519,11 @@ static int dw_probe(struct platform_device *pdev)
 
 	chip->dw->hdata->use_cfg2 = !!(flags & AXI_DMA_FLAG_USE_CFG2);
 
-	chip->core_clk = devm_clk_get(chip->dev, "core-clk");
-	if (IS_ERR(chip->core_clk))
-		return PTR_ERR(chip->core_clk);
-
-	chip->cfgr_clk = devm_clk_get(chip->dev, "cfgr-clk");
-	if (IS_ERR(chip->cfgr_clk))
-		return PTR_ERR(chip->cfgr_clk);
+	chip->clks[0].id = "core-clk";
+	chip->clks[1].id = "cfgr-clk";
+	ret = devm_clk_bulk_get(chip->dev, ARRAY_SIZE(chip->clks), chip->clks);
+	if (ret)
+		return dev_err_probe(chip->dev, ret, "failed to get clocks\n");
 
 	ret = parse_device_properties(chip);
 	if (ret)
@@ -1640,10 +1633,13 @@ static void dw_remove(struct platform_device *pdev)
 	struct dw_axi_dma *dw = chip->dw;
 	struct axi_dma_chan *chan, *_chan;
 	u32 i;
+	int ret;
 
 	/* Enable clk before accessing to registers */
-	clk_prepare_enable(chip->cfgr_clk);
-	clk_prepare_enable(chip->core_clk);
+	ret = clk_bulk_prepare_enable(ARRAY_SIZE(chip->clks), chip->clks);
+	if (ret)
+		dev_warn(chip->dev, "failed to enable clocks before remove: %d\n",
+			 ret);
 	axi_dma_irq_disable(chip);
 	for (i = 0; i < dw->hdata->nr_channels; i++) {
 		axi_chan_disable(&chip->dw->chan[i]);
diff --git a/drivers/dma/dw-axi-dmac/dw-axi-dmac.h b/drivers/dma/dw-axi-dmac/dw-axi-dmac.h
index 67cc199e24d1..039316c42f05 100644
--- a/drivers/dma/dw-axi-dmac/dw-axi-dmac.h
+++ b/drivers/dma/dw-axi-dmac/dw-axi-dmac.h
@@ -69,8 +69,7 @@ struct axi_dma_chip {
 	int			irq[DMAC_MAX_CHANNELS];
 	void __iomem		*regs;
 	void __iomem		*apb_regs;
-	struct clk		*core_clk;
-	struct clk		*cfgr_clk;
+	struct clk_bulk_data	clks[2];
 	struct dw_axi_dma	*dw;
 };
 
-- 
2.43.0

Re: [PATCH v2] dmaengine: dw-axi-dmac: convert clock handling to clk_bulk API

[sashiko-bot]

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

New issues:
- [High] The patch silently reverses the required clock enable and disable order, which can cause hardware initialization failures.

Pre-existing issues:
- [High] The interrupt handler unconditionally accesses unclocked device registers if an interrupt fires on a shared line while the device is in PM runtime suspend.
--

--- Patch [1]: [PATCH v2] dmaengine: dw-axi-dmac: convert clock handling to clk_bulk API ---
commit 9a52e2da75664a18bf3bb34d3bc595717db1bddc
Author: Chaithanya Lagisetty <nagachaithanya9911@gmail.com>
Subject: dmaengine: dw-axi-dmac: convert clock handling to clk_bulk API

This commit converts the driver to use the clk_bulk API for managing its two
mandatory clocks. It consolidates the clock getting, enabling, and disabling
logic to simplify error handling and fix a clock refcount imbalance.

Link: https://lore.kernel.org/all/20260813105432.2577322-1-nagachaithanya9911@gmail.com/

> diff --git a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c
> index bcefaff03b5c6..254167a558ff7 100644
> --- a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c
> +++ b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c

[ ... ]

> @@ -1524,13 +1519,11 @@ static int dw_probe(struct platform_device *pdev)
>  
>  	chip->dw->hdata->use_cfg2 = !!(flags & AXI_DMA_FLAG_USE_CFG2);
>  
> -	chip->core_clk = devm_clk_get(chip->dev, "core-clk");
> -	if (IS_ERR(chip->core_clk))
> -		return PTR_ERR(chip->core_clk);
> -
> -	chip->cfgr_clk = devm_clk_get(chip->dev, "cfgr-clk");
> -	if (IS_ERR(chip->cfgr_clk))
> -		return PTR_ERR(chip->cfgr_clk);
> +	chip->clks[0].id = "core-clk";
> +	chip->clks[1].id = "cfgr-clk";

[Severity: High]
Does this array initialization unintentionally reverse the clock enable and
disable sequence?

The old driver correctly enabled cfgr_clk before core_clk in axi_dma_resume(),
and disabled core_clk before cfgr_clk in axi_dma_suspend().

Because clk_bulk_prepare_enable() iterates forwards and
clk_bulk_disable_unprepare() iterates backwards, mapping core-clk to index 0
and cfgr-clk to index 1 applies the reverse hardware sequence for both.  Can
this cause hardware lockups or bus faults?

> +	ret = devm_clk_bulk_get(chip->dev, ARRAY_SIZE(chip->clks), chip->clks);
> +	if (ret)
> +		return dev_err_probe(chip->dev, ret, "failed to get clocks\n");

[Severity: High]
This is a pre-existing issue, but does the interrupt handler unconditionally
access unclocked device registers if an interrupt fires on a shared line
while the device is in PM runtime suspend?

In dw_probe(), the IRQs are requested using IRQF_SHARED:

dw-axi-dmac-platform.c:axi_req_irqs() {
	...
	ret = devm_request_irq(chip->dev, chip->irq[i], dw_axi_dma_interrupt,
			       IRQF_SHARED, KBUILD_MODNAME, chip);
	...
}

If the DMA controller is idle, runtime PM suspends the device, which disables
the clocks. If another device sharing the same IRQ line triggers an
interrupt, the handler is called:

dw-axi-dmac-platform.c:dw_axi_dma_interrupt() {
	...
	/* Disable DMAC interrupts. We'll enable them after processing channels */
	axi_dma_irq_disable(chip);
	...
}

Since axi_dma_irq_disable() accesses DMAC_CFG without checking if the device
is active, can this unclocked register read trigger a synchronous external
abort (kernel panic)?

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