[PATCH v6 0/5] add runtime suspend/resume support

[PATCH v6 0/5] add runtime suspend/resume support

[joy.zou]

Clean up driver FLAGs and introduce runtime suspend and resume support for
FSL eDMA.

Signed-off-by: Joy Zou <joy.zou@oss.nxp.com>
---
Changes in V6:
- add synchronize_irq() before disabling the channel clock in fsl_edma_chan_runtime_suspend()
  per AI review comments.
- add pm_runtime_get_if_active() in interrupt handlers to ensure registers can be accessed
  correctly per AI review comments.
- remove manual fsl_edma3_detach_pd() call when device_link_add() fails. The devres
  framework will handle cleanup automatically on probe failure per AI review comments.
- clear fsl_chan->pd_dev_link after freeing the device link to prevent potential 
  use-after-free issues per AI review comments..
- move fsl_edma->drvdata->setup_irq() atfer edma engine pm_runtime_resume_and_get().
- replace devm_clk_bulk_get_optional_enable() with devm_clk_get_optional()
  and clk_bulk_prepare_enable() in order to use runtime PM for power
  management later.
- replace devm_clk_get_optional_enable() with devm_clk_get_optional()
  and devm_clk_prepare_enable() in order to use runtime PM for power
  management later for patch #1 and #2.
- add new patch #5 fix use-after-free issue per AI review comments.
- modify the commit message for patch #1,#2 and #3.
- add Reviewed-by tag for patch #1,#2 and #3.
- Link to v5: https://lore.kernel.org/r/20260513-b4-b4-edma-runtime-opt-v5-0-1e595bfb8423@nxp.com

Changes in V5:
- add three new patches, two of which replace devm_clk_get_enabled() with devm_clk_get_optional_enabled(),
  and the other convert to clk bulk API.
- remove unnecessary flags FSL_EDMA_DRV_HAS_CHCLK and FSL_EDMA_DRV_HAS_DMACLK.
- remove redundant clk_disable_unprepare() due to the pm_runtime_put_sync_suspend() added.
- use devm_pm_runtime_enable() to replace pm_runtime_enable() and add return value check.
- add return value check for pm_runtime_get_sync().
- replace pm_runtime_get_sync() with pm_runtime_resume_and_get().
- replace DMAMUX clock handling with bulk clock API for edma engine runtime suspend/resume.
- remove dev_pm_domain_detach() when device_link_add() fail because the fsl_edma3_detach_pd()
  also call dev_pm_domain_detach().
- remove device_link_add() DL_FLAG_RPM_ACTIVE flag and pm_runtime_put_sync_suspend().
- add clk_bulk_disable_unprepare() for clk_prepare_enable() fail in fsl_edma_runtime_resume().
- remove the extra space before RUNTIME_PM_OPS.
- add skip channel comments for system suspend.
- add clk_disable_unprepare() for dmaclk at the end of probe function.
- add clk_bulk_disable_unprepare() for muxclk at the end of probe function.
- Link to v4: https://lore.kernel.org/imx/20251017-b4-edma-runtime-v4-1-87c64dd30229@nxp.com/

Changes for V4:
- fix a typo dmaegnine/dmaengine in the subject.
- Link to v3: https://lore.kernel.org/imx/20250912-b4-edma-runtime-v3-1-be22f7161745@nxp.com/

Changes for V3:
- rebased onto commit 8f21d9da4670 ("Add linux-next specific files for 20250911")
  to align with latest changes.
- Remove pm_runtime_dont_use_autosuspend() from fsl_edma3_detach_pd().
  because the autosuspend is not used.
- Move some edma channel registers initialization after the chan_dev
  pm_runtime_enable().
- Add clk_prepare_enable() return check in fsl_edma_runtime_resume.
- Add flag FSL_EDMA_DRV_HAS_DMACLK check in fsl_edma_runtime_resume/suspend().
- Link to v2: https://lore.kernel.org/imx/20241226052643.1951886-1-joy.zou@nxp.com/

Changes for V2:
- drop ret from fsl_edma_chan_runtime_suspend().
- drop ret from fsl_edma_chan_runtime_resume() and return clk_prepare_enable().
- add review tag
- Link to v1: https://lore.kernel.org/imx/20241220021109.2102294-1-joy.zou@nxp.com/

To: Frank Li <Frank.Li@nxp.com>
To: Vinod Koul <vkoul@kernel.org>
To: Joe Hattori <joe@pf.is.s.u-tokyo.ac.jp>
Cc: Frank Li <Frank.Li@kernel.org>
Cc: imx@lists.linux.dev
Cc: dmaengine@vger.kernel.org
Cc: linux-kernel@vger.kernel.org

---
Joy Zou (5):
      dmaengine: fsl-edma: use devm_clk_get_optional() for channel clock
      dmaengine: fsl-edma: use devm_clk_get_optional() for DMA engine clock
      dmaengine: fsl-edma: convert DMAMUX clock handling to bulk clock API
      dmaengine: fsl-edma: add runtime suspend/resume support
      dmaengine: fsl-edma: fix use-after-free after dev_pm_domain_detach()

 drivers/dma/fsl-edma-common.c |  14 +-
 drivers/dma/fsl-edma-common.h |   4 +-
 drivers/dma/fsl-edma-main.c   | 305 +++++++++++++++++++++++++++++++-----------
 3 files changed, 234 insertions(+), 89 deletions(-)
---
base-commit: f7af91adc230aa99e23330ecf85bc9badd9780ad
change-id: 20260617-b4-edma-runtime-opt-2b14d269bcee

Best regards,
--  
Joy Zou <joy.zou@oss.nxp.com>

[PATCH v6 1/5] dmaengine: fsl-edma: use devm_clk_get_optional() for channel clock

[joy.zou]

From: Joy Zou <joy.zou@nxp.com>

The channel clock is optional and not present on all platforms.
Replace devm_clk_get_enabled() with devm_clk_get_optional() and
devm_clk_prepare_enable(), and remove FSL_EDMA_DRV_HAS_CHCLK flag
to simplify clock handling.

Prepare to add channel runtime pm support.

Reviewed-by: Frank Li <Frank.Li@nxp.com>
Signed-off-by: Joy Zou <joy.zou@nxp.com>
---
Changes for v6:
- Replace devm_clk_get_optional_enable() with devm_clk_get_optional()
   and devm_clk_prepare_enable() in order to use runtime PM for power
   management later.
- Modify the commit message.
- Add Reviewed-by tag.
- Link to v5: https://lore.kernel.org/imx/20260513-b4-b4-edma-runtime-opt-v5-0-1e595bfb8423@nxp.com/
---
 drivers/dma/fsl-edma-common.c |  4 +---
 drivers/dma/fsl-edma-common.h |  1 -
 drivers/dma/fsl-edma-main.c   | 22 +++++++++-------------
 3 files changed, 10 insertions(+), 17 deletions(-)

diff --git a/drivers/dma/fsl-edma-common.c b/drivers/dma/fsl-edma-common.c
index bb7531c456df..e1ca25ff228d 100644
--- a/drivers/dma/fsl-edma-common.c
+++ b/drivers/dma/fsl-edma-common.c
@@ -844,9 +844,7 @@ int fsl_edma_alloc_chan_resources(struct dma_chan *chan)
 	struct fsl_edma_chan *fsl_chan = to_fsl_edma_chan(chan);
 	int ret = 0;
 
-	if (fsl_edma_drvflags(fsl_chan) & FSL_EDMA_DRV_HAS_CHCLK)
-		clk_prepare_enable(fsl_chan->clk);
-
+	clk_prepare_enable(fsl_chan->clk);
 	fsl_chan->tcd_pool = dma_pool_create("tcd_pool", chan->device->dev,
 				fsl_edma_drvflags(fsl_chan) & FSL_EDMA_DRV_TCD64 ?
 				sizeof(struct fsl_edma_hw_tcd64) : sizeof(struct fsl_edma_hw_tcd),
diff --git a/drivers/dma/fsl-edma-common.h b/drivers/dma/fsl-edma-common.h
index 205a96489094..f4354b586746 100644
--- a/drivers/dma/fsl-edma-common.h
+++ b/drivers/dma/fsl-edma-common.h
@@ -210,7 +210,6 @@ struct fsl_edma_desc {
 #define FSL_EDMA_DRV_WRAP_IO		BIT(3)
 #define FSL_EDMA_DRV_EDMA64		BIT(4)
 #define FSL_EDMA_DRV_HAS_PD		BIT(5)
-#define FSL_EDMA_DRV_HAS_CHCLK		BIT(6)
 #define FSL_EDMA_DRV_HAS_CHMUX		BIT(7)
 #define FSL_EDMA_DRV_MEM_REMOTE		BIT(8)
 /* control and status register is in tcd address space, edma3 reg layout */
diff --git a/drivers/dma/fsl-edma-main.c b/drivers/dma/fsl-edma-main.c
index 36155ab1602a..1e864cd4c784 100644
--- a/drivers/dma/fsl-edma-main.c
+++ b/drivers/dma/fsl-edma-main.c
@@ -567,8 +567,7 @@ static struct fsl_edma_drvdata imx8qm_data = {
 };
 
 static struct fsl_edma_drvdata imx8ulp_data = {
-	.flags = FSL_EDMA_DRV_HAS_CHMUX | FSL_EDMA_DRV_HAS_CHCLK | FSL_EDMA_DRV_HAS_DMACLK |
-		 FSL_EDMA_DRV_EDMA3,
+	.flags = FSL_EDMA_DRV_HAS_CHMUX | FSL_EDMA_DRV_HAS_DMACLK | FSL_EDMA_DRV_EDMA3,
 	.chreg_space_sz = 0x10000,
 	.chreg_off = 0x10000,
 	.mux_off = 0x10000 + offsetof(struct fsl_edma3_ch_reg, ch_mux),
@@ -808,22 +807,19 @@ static int fsl_edma_probe(struct platform_device *pdev)
 		fsl_chan->tcd = fsl_edma->membase
 				+ i * drvdata->chreg_space_sz + drvdata->chreg_off + len;
 		fsl_chan->mux_addr = fsl_edma->membase + drvdata->mux_off + i * drvdata->mux_skip;
-
-		if (drvdata->flags & FSL_EDMA_DRV_HAS_CHCLK) {
-			snprintf(clk_name, sizeof(clk_name), "ch%02d", i);
-			fsl_chan->clk = devm_clk_get_enabled(&pdev->dev,
-							     (const char *)clk_name);
-
-			if (IS_ERR(fsl_chan->clk))
-				return PTR_ERR(fsl_chan->clk);
-		}
+		snprintf(clk_name, sizeof(clk_name), "ch%02d", i);
+		fsl_chan->clk = devm_clk_get_optional(&pdev->dev, (const char *)clk_name);
+		if (IS_ERR(fsl_chan->clk))
+			return PTR_ERR(fsl_chan->clk);
+		ret = devm_clk_prepare_enable(&pdev->dev, fsl_chan->clk);
+		if (ret)
+			return dev_err_probe(&pdev->dev, ret, "Failed to enable clock\n");
 		fsl_chan->pdev = pdev;
 		vchan_init(&fsl_chan->vchan, &fsl_edma->dma_dev);
 
 		edma_write_tcdreg(fsl_chan, cpu_to_le32(0), csr);
 		fsl_edma_chan_mux(fsl_chan, 0, false);
-		if (fsl_chan->edma->drvdata->flags & FSL_EDMA_DRV_HAS_CHCLK)
-			clk_disable_unprepare(fsl_chan->clk);
+		clk_disable_unprepare(fsl_chan->clk);
 	}
 
 	ret = fsl_edma->drvdata->setup_irq(pdev, fsl_edma);

-- 
2.34.1

Re: [PATCH v6 1/5] dmaengine: fsl-edma: use devm_clk_get_optional() for channel clock

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The patch introduces a compilation failure by calling an undefined function `devm_clk_prepare_enable()`.
--

commit 36cfeebd16ae2d241bf40213a5b9272983d736d3
Author: Joy Zou <joy.zou@nxp.com>

dmaengine: fsl-edma: use devm_clk_get_optional() for channel clock
    
This commit replaces devm_clk_get_enabled() with devm_clk_get_optional()
and removes the FSL_EDMA_DRV_HAS_CHCLK flag for channel clocks. It prepares
the driver for adding channel runtime PM support.

> diff --git a/drivers/dma/fsl-edma-main.c b/drivers/dma/fsl-edma-main.c
> index 36155ab1602a9..1e864cd4c784e 100644
> --- a/drivers/dma/fsl-edma-main.c
> +++ b/drivers/dma/fsl-edma-main.c

[ ... ]

> @@ -808,22 +807,19 @@ static int fsl_edma_probe(struct platform_device *pdev)
>  		fsl_chan->tcd = fsl_edma->membase
>  				+ i * drvdata->chreg_space_sz + drvdata->chreg_off + len;
>  		fsl_chan->mux_addr = fsl_edma->membase + drvdata->mux_off + i * drvdata->mux_skip;
> -
> -		if (drvdata->flags & FSL_EDMA_DRV_HAS_CHCLK) {
> -			snprintf(clk_name, sizeof(clk_name), "ch%02d", i);
> -			fsl_chan->clk = devm_clk_get_enabled(&pdev->dev,
> -							     (const char *)clk_name);
> -
> -			if (IS_ERR(fsl_chan->clk))
> -				return PTR_ERR(fsl_chan->clk);
> -		}
> +		snprintf(clk_name, sizeof(clk_name), "ch%02d", i);
> +		fsl_chan->clk = devm_clk_get_optional(&pdev->dev, (const char *)clk_name);
> +		if (IS_ERR(fsl_chan->clk))
> +			return PTR_ERR(fsl_chan->clk);
> +		ret = devm_clk_prepare_enable(&pdev->dev, fsl_chan->clk);
> +		if (ret)
> +			return dev_err_probe(&pdev->dev, ret, "Failed to enable clock\n");

[Severity: Low]
Will this introduce a build failure? The function devm_clk_prepare_enable()
does not appear to be defined in the Linux kernel Common Clock Framework.

I see this compilation regression was resolved later in the patch series by
commit a5800ff547600d938eb443a21684d6291b4658d1 ("dmaengine: fsl-edma: add
runtime suspend/resume support"), but could it break git bisect if this
commit is tested independently?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260701-b4-edma-runtime-opt-v6-0-354ff4229c00@oss.nxp.com?part=1

[PATCH v6 2/5] dmaengine: fsl-edma: use devm_clk_get_optional() for DMA engine clock

[joy.zou]

From: Joy Zou <joy.zou@nxp.com>

The eDMA engine clock is optional and not present on all platforms.
Replace devm_clk_get_enabled() with devm_clk_get_optional() and
devm_clk_prepare_enable(), and remove FSL_EDMA_DRV_HAS_DMACLK flag
to simplify clock handling.

Prepare to add channel runtime pm support.

Reviewed-by: Frank Li <Frank.Li@nxp.com>
Signed-off-by: Joy Zou <joy.zou@nxp.com>
---
Changes for v6:
- Replace devm_clk_get_optional_enable() with devm_clk_get_optional()
   and devm_clk_prepare_enable() in order to use runtime PM for power
   management later.
- Modify the commit message.
- Add Reviewed-by tag.
- Link to v5: https://lore.kernel.org/imx/20260513-b4-b4-edma-runtime-opt-v5-0-1e595bfb8423@nxp.com/
---
 drivers/dma/fsl-edma-common.h |  1 -
 drivers/dma/fsl-edma-main.c   | 26 ++++++++++++++------------
 2 files changed, 14 insertions(+), 13 deletions(-)

diff --git a/drivers/dma/fsl-edma-common.h b/drivers/dma/fsl-edma-common.h
index f4354b586746..54128b3f45cb 100644
--- a/drivers/dma/fsl-edma-common.h
+++ b/drivers/dma/fsl-edma-common.h
@@ -204,7 +204,6 @@ struct fsl_edma_desc {
 	struct fsl_edma_sw_tcd		tcd[];
 };
 
-#define FSL_EDMA_DRV_HAS_DMACLK		BIT(0)
 #define FSL_EDMA_DRV_MUX_SWAP		BIT(1)
 #define FSL_EDMA_DRV_CONFIG32		BIT(2)
 #define FSL_EDMA_DRV_WRAP_IO		BIT(3)
diff --git a/drivers/dma/fsl-edma-main.c b/drivers/dma/fsl-edma-main.c
index 1e864cd4c784..88fc1b06e518 100644
--- a/drivers/dma/fsl-edma-main.c
+++ b/drivers/dma/fsl-edma-main.c
@@ -554,7 +554,7 @@ static struct fsl_edma_drvdata imx7ulp_data = {
 	.dmamuxs = 1,
 	.chreg_off = EDMA_TCD,
 	.chreg_space_sz = sizeof(struct fsl_edma_hw_tcd),
-	.flags = FSL_EDMA_DRV_HAS_DMACLK | FSL_EDMA_DRV_CONFIG32,
+	.flags = FSL_EDMA_DRV_CONFIG32,
 	.setup_irq = fsl_edma2_irq_init,
 };
 
@@ -567,7 +567,7 @@ static struct fsl_edma_drvdata imx8qm_data = {
 };
 
 static struct fsl_edma_drvdata imx8ulp_data = {
-	.flags = FSL_EDMA_DRV_HAS_CHMUX | FSL_EDMA_DRV_HAS_DMACLK | FSL_EDMA_DRV_EDMA3,
+	.flags = FSL_EDMA_DRV_HAS_CHMUX | FSL_EDMA_DRV_EDMA3,
 	.chreg_space_sz = 0x10000,
 	.chreg_off = 0x10000,
 	.mux_off = 0x10000 + offsetof(struct fsl_edma3_ch_reg, ch_mux),
@@ -576,14 +576,14 @@ static struct fsl_edma_drvdata imx8ulp_data = {
 };
 
 static struct fsl_edma_drvdata imx93_data3 = {
-	.flags = FSL_EDMA_DRV_HAS_DMACLK | FSL_EDMA_DRV_EDMA3 | FSL_EDMA_DRV_ERRIRQ_SHARE,
+	.flags = FSL_EDMA_DRV_EDMA3 | FSL_EDMA_DRV_ERRIRQ_SHARE,
 	.chreg_space_sz = 0x10000,
 	.chreg_off = 0x10000,
 	.setup_irq = fsl_edma3_irq_init,
 };
 
 static struct fsl_edma_drvdata imx93_data4 = {
-	.flags = FSL_EDMA_DRV_HAS_CHMUX | FSL_EDMA_DRV_HAS_DMACLK | FSL_EDMA_DRV_EDMA4
+	.flags = FSL_EDMA_DRV_HAS_CHMUX | FSL_EDMA_DRV_EDMA4
 		 | FSL_EDMA_DRV_ERRIRQ_SHARE,
 	.chreg_space_sz = 0x8000,
 	.chreg_off = 0x10000,
@@ -593,7 +593,7 @@ static struct fsl_edma_drvdata imx93_data4 = {
 };
 
 static struct fsl_edma_drvdata imx95_data5 = {
-	.flags = FSL_EDMA_DRV_HAS_CHMUX | FSL_EDMA_DRV_HAS_DMACLK | FSL_EDMA_DRV_EDMA4 |
+	.flags = FSL_EDMA_DRV_HAS_CHMUX | FSL_EDMA_DRV_EDMA4 |
 		 FSL_EDMA_DRV_TCD64 | FSL_EDMA_DRV_ERRIRQ_SHARE,
 	.chreg_space_sz = 0x8000,
 	.chreg_off = 0x10000,
@@ -733,13 +733,15 @@ static int fsl_edma_probe(struct platform_device *pdev)
 		regs = &fsl_edma->regs;
 	}
 
-	if (drvdata->flags & FSL_EDMA_DRV_HAS_DMACLK) {
-		fsl_edma->dmaclk = devm_clk_get_enabled(&pdev->dev, "dma");
-		if (IS_ERR(fsl_edma->dmaclk))
-			return dev_err_probe(&pdev->dev,
-					     PTR_ERR(fsl_edma->dmaclk),
-					     "Missing DMA block clock.\n");
-	}
+	fsl_edma->dmaclk = devm_clk_get_optional(&pdev->dev, "dma");
+	if (IS_ERR(fsl_edma->dmaclk))
+		return dev_err_probe(&pdev->dev,
+				     PTR_ERR(fsl_edma->dmaclk),
+				     "Failed to get/enable DMA clock.\n");
+
+	ret = devm_clk_prepare_enable(&pdev->dev, fsl_edma->dmaclk);
+	if (ret)
+		return dev_err_probe(&pdev->dev, ret, "Failed to enable clock\n");
 
 	ret = of_property_read_variable_u32_array(np, "dma-channel-mask", chan_mask, 1, 2);
 

-- 
2.34.1

Re: [PATCH v6 2/5] dmaengine: fsl-edma: use devm_clk_get_optional() for DMA engine clock

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The patch introduces calls to a non-existent function `devm_clk_prepare_enable()`, resulting in a build failure.
--

commit 52e4c7b7c140f88bd3ed6ad6e10c3513c2941e5c
Author: Joy Zou <joy.zou@nxp.com>

dmaengine: fsl-edma: use devm_clk_get_optional() for DMA engine clock

This commit updates the eDMA engine clock handling to treat it as optional,
removing the FSL_EDMA_DRV_HAS_DMACLK flag and replacing
devm_clk_get_enabled() with separate get and enable calls.

> diff --git a/drivers/dma/fsl-edma-main.c b/drivers/dma/fsl-edma-main.c
> --- a/drivers/dma/fsl-edma-main.c
> +++ b/drivers/dma/fsl-edma-main.c

[ ... ]

> @@ -733,13 +733,15 @@ static int fsl_edma_probe(struct platform_device *pdev)
>  		regs = &fsl_edma->regs;
>  	}
>  
> -	if (drvdata->flags & FSL_EDMA_DRV_HAS_DMACLK) {
> -		fsl_edma->dmaclk = devm_clk_get_enabled(&pdev->dev, "dma");
> -		if (IS_ERR(fsl_edma->dmaclk))
> -			return dev_err_probe(&pdev->dev,
> -					     PTR_ERR(fsl_edma->dmaclk),
> -					     "Missing DMA block clock.\n");
> -	}
> +	fsl_edma->dmaclk = devm_clk_get_optional(&pdev->dev, "dma");
> +	if (IS_ERR(fsl_edma->dmaclk))
> +		return dev_err_probe(&pdev->dev,
> +				     PTR_ERR(fsl_edma->dmaclk),
> +				     "Failed to get/enable DMA clock.\n");
> +
> +	ret = devm_clk_prepare_enable(&pdev->dev, fsl_edma->dmaclk);

[Severity: Low]
Does this function call cause a build regression? It appears that
devm_clk_prepare_enable() is not defined in the standard clock API, which
might result in an implicit declaration error and break git bisect.

I noticed that this is resolved later in the series in commit
a5800ff547600d938eb443a21684d6291b4658d1 (dmaengine: fsl-edma: add runtime
suspend/resume support), where these undefined function calls are removed
for both the engine clock and the channel clocks.

Is it possible to reorganize the patch series so that the build does not
fail at this intermediate step?

> +	if (ret)
> +		return dev_err_probe(&pdev->dev, ret, "Failed to enable clock\n");
>  
>  	ret = of_property_read_variable_u32_array(np, "dma-channel-mask", chan_mask, 1, 2);
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260701-b4-edma-runtime-opt-v6-0-354ff4229c00@oss.nxp.com?part=2

[PATCH v6 3/5] dmaengine: fsl-edma: convert DMAMUX clock handling to bulk clock API

[joy.zou]

From: Joy Zou <joy.zou@nxp.com>

Convert the DMAMUX clock management from individual clock operations
to the bulk clock API to simplify the code.

Prepare to add edma engine runtime pm support.

Reviewed-by: Frank Li <Frank.Li@nxp.com>
Signed-off-by: Joy Zou <joy.zou@nxp.com>
---
Changes for v6:
- Replace devm_clk_bulk_get_optional_enable() with devm_clk_get_optional()
   and clk_bulk_prepare_enable() in order to use runtime PM for power
   management later.
- add Reviewed-by tag.
- Link to v5: https://lore.kernel.org/imx/20260513-b4-b4-edma-runtime-opt-v5-0-1e595bfb8423@nxp.com/
---
 drivers/dma/fsl-edma-common.h |  2 +-
 drivers/dma/fsl-edma-main.c   | 51 +++++++++++++++++++++++++++----------------
 2 files changed, 33 insertions(+), 20 deletions(-)

diff --git a/drivers/dma/fsl-edma-common.h b/drivers/dma/fsl-edma-common.h
index 54128b3f45cb..824b7dd2b526 100644
--- a/drivers/dma/fsl-edma-common.h
+++ b/drivers/dma/fsl-edma-common.h
@@ -253,7 +253,7 @@ struct fsl_edma_engine {
 	struct dma_device	dma_dev;
 	void __iomem		*membase;
 	void __iomem		*muxbase[DMAMUX_NR];
-	struct clk		*muxclk[DMAMUX_NR];
+	struct clk_bulk_data    *muxclk;
 	struct clk		*dmaclk;
 	struct mutex		fsl_edma_mutex;
 	const struct fsl_edma_drvdata *drvdata;
diff --git a/drivers/dma/fsl-edma-main.c b/drivers/dma/fsl-edma-main.c
index 88fc1b06e518..fe02b68d75fd 100644
--- a/drivers/dma/fsl-edma-main.c
+++ b/drivers/dma/fsl-edma-main.c
@@ -526,12 +526,11 @@ static void fsl_edma_irq_exit(
 	}
 }
 
-static void fsl_disable_clocks(struct fsl_edma_engine *fsl_edma, int nr_clocks)
+static void fsl_edma_disable_muxclk(void *data)
 {
-	int i;
+	struct fsl_edma_engine *fsl_edma = data;
 
-	for (i = 0; i < nr_clocks; i++)
-		clk_disable_unprepare(fsl_edma->muxclk[i]);
+	clk_bulk_disable_unprepare(fsl_edma->drvdata->dmamuxs, fsl_edma->muxclk);
 }
 
 static struct fsl_edma_drvdata vf610_data = {
@@ -751,23 +750,37 @@ static int fsl_edma_probe(struct platform_device *pdev)
 		fsl_edma->chan_masked |= chan_mask[0];
 	}
 
-	for (i = 0; i < fsl_edma->drvdata->dmamuxs; i++) {
-		char clkname[32];
-
-		fsl_edma->muxbase[i] = devm_platform_ioremap_resource(pdev,
-								      1 + i);
-		if (IS_ERR(fsl_edma->muxbase[i])) {
-			/* on error: disable all previously enabled clks */
-			fsl_disable_clocks(fsl_edma, i);
-			return PTR_ERR(fsl_edma->muxbase[i]);
+	if (fsl_edma->drvdata->dmamuxs) {
+		fsl_edma->muxclk = devm_kcalloc(&pdev->dev, fsl_edma->drvdata->dmamuxs,
+						sizeof(*fsl_edma->muxclk), GFP_KERNEL);
+		if (!fsl_edma->muxclk)
+			return -ENOMEM;
+
+		for (i = 0; i < fsl_edma->drvdata->dmamuxs; i++) {
+			fsl_edma->muxbase[i] = devm_platform_ioremap_resource(pdev, 1 + i);
+			if (IS_ERR(fsl_edma->muxbase[i]))
+				return PTR_ERR(fsl_edma->muxbase[i]);
+
+			fsl_edma->muxclk[i].id = devm_kasprintf(&pdev->dev, GFP_KERNEL,
+								"dmamux%d", i);
+			if (!fsl_edma->muxclk[i].id)
+				return -ENOMEM;
 		}
 
-		sprintf(clkname, "dmamux%d", i);
-		fsl_edma->muxclk[i] = devm_clk_get_enabled(&pdev->dev, clkname);
-		if (IS_ERR(fsl_edma->muxclk[i]))
-			return dev_err_probe(&pdev->dev,
-					     PTR_ERR(fsl_edma->muxclk[i]),
-					     "Missing DMAMUX block clock.\n");
+		ret = devm_clk_bulk_get_optional(&pdev->dev, fsl_edma->drvdata->dmamuxs,
+						 fsl_edma->muxclk);
+		if (ret)
+			return dev_err_probe(&pdev->dev, ret,
+					     "Failed to get DMAMUX block clock.\n");
+
+		ret = clk_bulk_prepare_enable(fsl_edma->drvdata->dmamuxs, fsl_edma->muxclk);
+		if (ret)
+			return dev_err_probe(&pdev->dev, ret,
+					     "Failed to enable DMAMUX block clock.\n");
+
+		ret = devm_add_action_or_reset(&pdev->dev, fsl_edma_disable_muxclk, fsl_edma);
+		if (ret)
+			return dev_err_probe(&pdev->dev, ret, "Failed to add cleanup action.\n");
 	}
 
 	fsl_edma->big_endian = of_property_read_bool(np, "big-endian");

-- 
2.34.1

Re: [PATCH v6 3/5] dmaengine: fsl-edma: convert DMAMUX clock handling to bulk clock API

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Changing from `devm_clk_get_enabled` to `devm_clk_bulk_get_optional` silently ignores missing DMAMUX clocks, which were previously mandatory and can lead to synchronous bus errors.
--

commit 7885b2482d67d70ba7ccdebb1cd210d1f8dc8ba2
Author: Joy Zou <joy.zou@nxp.com>

dmaengine: fsl-edma: convert DMAMUX clock handling to bulk clock API

Convert the DMAMUX clock management from individual clock operations to the
bulk clock API to simplify the code. Prepare to add edma engine runtime pm
support.

> diff --git a/drivers/dma/fsl-edma-main.c b/drivers/dma/fsl-edma-main.c
> --- a/drivers/dma/fsl-edma-main.c
> +++ b/drivers/dma/fsl-edma-main.c
[ ... ]
> @@ -751,23 +750,37 @@ static int fsl_edma_probe(struct platform_device *pdev)
[ ... ]
> -		fsl_edma->muxclk[i] = devm_clk_get_enabled(&pdev->dev, clkname);
> -		if (IS_ERR(fsl_edma->muxclk[i]))
> -			return dev_err_probe(&pdev->dev,
> -					     PTR_ERR(fsl_edma->muxclk[i]),
> -					     "Missing DMAMUX block clock.\n");
> +		ret = devm_clk_bulk_get_optional(&pdev->dev, fsl_edma->drvdata->dmamuxs,
> +						 fsl_edma->muxclk);

[Severity: High]
Does this change to an optional clock fetch risk a synchronous bus error?

Previously, devm_clk_get_enabled() was used, which would fail probe if the
clock was missing. By switching to devm_clk_bulk_get_optional(), a missing
DMAMUX clock in the device tree will be silently ignored.

If the clocks are missing, devm_clk_bulk_get_optional() returns 0 and sets
the clock pointers to NULL. The subsequent clk_bulk_prepare_enable() call
will succeed safely. However, the driver will later unconditionally access the
unclocked DMAMUX registers:

drivers/dma/fsl-edma-common.c:fsl_edma_chan_mux() {
    ...
    if (fsl_chan->edma->drvdata->flags & FSL_EDMA_DRV_CONFIG32)
        mux_configure32(fsl_chan, muxaddr, ch_off, slot, enable);
    else
        mux_configure8(fsl_chan, muxaddr, ch_off, slot, enable);
    ...
}

Could this lead to a kernel panic when accessing the unclocked registers?
Should this use devm_clk_bulk_get() instead so probe correctly fails for
missing mandatory clocks?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260701-b4-edma-runtime-opt-v6-0-354ff4229c00@oss.nxp.com?part=3

[PATCH v6 4/5] dmaengine: fsl-edma: add runtime suspend/resume support

[joy.zou]

From: Joy Zou <joy.zou@nxp.com>

Introduce runtime suspend and resume support for FSL eDMA. Enable
per-channel power domain management to facilitate runtime suspend and
resume operations.

Implement runtime suspend and resume functions for the eDMA engine and
individual channels.

Link per-channel power domain device to eDMA per-channel device instead of
eDMA engine device. So Power Manage framework manage power state of linked
domain device when per-channel device request runtime resume/suspend.

Trigger the eDMA engine's runtime suspend when all channels are suspended,
disabling all common clocks through the runtime PM framework.

Signed-off-by: Joy Zou <joy.zou@nxp.com>
Signed-off-by: Frank Li <Frank.Li@nxp.com>
Reviewed-by: Frank Li <Frank.Li@nxp.com>
---
Changes in V6:
- add synchronize_irq() before disabling the channel clock in fsl_edma_chan_runtime_suspend()
  per AI review comments.
- add pm_runtime_get_if_active() in interrupt handlers to ensure registers can be accessed
  correctly per AI review comments.
- remove manual fsl_edma3_detach_pd() call when device_link_add() fails. The devres
  framework will handle cleanup automatically on probe failure.
- clear fsl_chan->pd_dev_link after freeing the device link to prevent potential
  use-after-free issues.
- move fsl_edma->drvdata->setup_irq() atfer edma engine pm_runtime_resume_and_get().
- Link to v5: https://lore.kernel.org/imx/20260513-b4-b4-edma-runtime-opt-v5-0-1e595bfb8423@nxp.com/

Changes in V5:
- remove unnecessary flags FSL_EDMA_DRV_HAS_CHCLK and FSL_EDMA_DRV_HAS_DMACLK.
- remove redundant clk_disable_unprepare() due to the pm_runtime_put_sync_suspend() added.
- use devm_pm_runtime_enable() to replace pm_runtime_enable() and add return value check.
- add return value check for pm_runtime_get_sync();
- replace pm_runtime_get_sync() with pm_runtime_resume_and_get().
- replace DMAMUX clock handling with bulk clock API for edma engine runtime suspend/resume.
- remove dev_pm_domain_detach() when device_link_add() fail because the fsl_edma3_detach_pd()
  also call dev_pm_domain_detach().
- remove device_link_add() DL_FLAG_RPM_ACTIVE flag and pm_runtime_put_sync_suspend().
- add clk_bulk_disable_unprepare() for clk_prepare_enable() fail in fsl_edma_runtime_resume().
- remove the extra space before RUNTIME_PM_OPS.
- add skip channel comments for system suspend.
- add clk_disable_unprepare() for dmaclk at the end of probe function.
- add clk_bulk_disable_unprepare() for muxclk at the end of probe function.
- Link to v4: https://lore.kernel.org/imx/20251017-b4-edma-runtime-v4-1-87c64dd30229@nxp.com/

Changes for V4:
- fix a typo dmaegnine/dmaengine in the subject.
- Link to v3: https://lore.kernel.org/imx/20250912-b4-edma-runtime-v3-1-be22f7161745@nxp.com/

Changes for V3:
- rebased onto commit 8f21d9da4670 ("Add linux-next specific files for 20250911")
  to align with latest changes.
- Remove pm_runtime_dont_use_autosuspend() from fsl_edma3_detach_pd().
  because the autosuspend is not used.
- Move some edma channel registers initialization after the chan_dev
  pm_runtime_enable().
- Add clk_prepare_enable() return check in fsl_edma_runtime_resume.
- Add flag FSL_EDMA_DRV_HAS_DMACLK check in fsl_edma_runtime_resume/suspend().
- Link to v2: https://lore.kernel.org/imx/20241226052643.1951886-1-joy.zou@nxp.com/

Changes for V2:
- drop ret from fsl_edma_chan_runtime_suspend().
- drop ret from fsl_edma_chan_runtime_resume() and return clk_prepare_enable().
- add review tag
- Link to v1: https://lore.kernel.org/imx/20241220021109.2102294-1-joy.zou@nxp.com/
---
 drivers/dma/fsl-edma-common.c |  14 +--
 drivers/dma/fsl-edma-main.c   | 246 ++++++++++++++++++++++++++++++++----------
 2 files changed, 199 insertions(+), 61 deletions(-)

diff --git a/drivers/dma/fsl-edma-common.c b/drivers/dma/fsl-edma-common.c
index e1ca25ff228d..132b900ee607 100644
--- a/drivers/dma/fsl-edma-common.c
+++ b/drivers/dma/fsl-edma-common.c
@@ -247,9 +247,6 @@ int fsl_edma_terminate_all(struct dma_chan *chan)
 	spin_unlock_irqrestore(&fsl_chan->vchan.lock, flags);
 	vchan_dma_desc_free_list(&fsl_chan->vchan, &head);
 
-	if (fsl_edma_drvflags(fsl_chan) & FSL_EDMA_DRV_HAS_PD)
-		pm_runtime_allow(fsl_chan->pd_dev);
-
 	return 0;
 }
 
@@ -844,7 +841,12 @@ int fsl_edma_alloc_chan_resources(struct dma_chan *chan)
 	struct fsl_edma_chan *fsl_chan = to_fsl_edma_chan(chan);
 	int ret = 0;
 
-	clk_prepare_enable(fsl_chan->clk);
+	ret = pm_runtime_resume_and_get(&fsl_chan->vchan.chan.dev->device);
+	if (ret < 0) {
+		dev_err(&fsl_chan->vchan.chan.dev->device, "Failed to resume device: %d\n", ret);
+		return ret;
+	}
+
 	fsl_chan->tcd_pool = dma_pool_create("tcd_pool", chan->device->dev,
 				fsl_edma_drvflags(fsl_chan) & FSL_EDMA_DRV_TCD64 ?
 				sizeof(struct fsl_edma_hw_tcd64) : sizeof(struct fsl_edma_hw_tcd),
@@ -871,7 +873,7 @@ int fsl_edma_alloc_chan_resources(struct dma_chan *chan)
 		free_irq(fsl_chan->txirq, fsl_chan);
 err_txirq:
 	dma_pool_destroy(fsl_chan->tcd_pool);
-	clk_disable_unprepare(fsl_chan->clk);
+	pm_runtime_put_sync_suspend(&fsl_chan->vchan.chan.dev->device);
 
 	return ret;
 }
@@ -903,7 +905,7 @@ void fsl_edma_free_chan_resources(struct dma_chan *chan)
 	fsl_chan->is_sw = false;
 	fsl_chan->srcid = 0;
 	fsl_chan->is_remote = false;
-	clk_disable_unprepare(fsl_chan->clk);
+	pm_runtime_put_sync_suspend(&fsl_chan->vchan.chan.dev->device);
 }
 
 void fsl_edma_cleanup_vchan(struct dma_device *dmadev)
diff --git a/drivers/dma/fsl-edma-main.c b/drivers/dma/fsl-edma-main.c
index fe02b68d75fd..3518dfb4292d 100644
--- a/drivers/dma/fsl-edma-main.c
+++ b/drivers/dma/fsl-edma-main.c
@@ -37,15 +37,27 @@ static irqreturn_t fsl_edma_tx_handler(int irq, void *dev_id)
 	unsigned int intr, ch;
 	struct edma_regs *regs = &fsl_edma->regs;
 
+	if (pm_runtime_get_if_active(fsl_edma->dma_dev.dev) <= 0)
+		return IRQ_NONE;
+
 	intr = edma_readl(fsl_edma, regs->intl);
-	if (!intr)
+	if (!intr) {
+		pm_runtime_put(fsl_edma->dma_dev.dev);
 		return IRQ_NONE;
+	}
 
+	pm_runtime_put(fsl_edma->dma_dev.dev);
 	for (ch = 0; ch < fsl_edma->n_chans; ch++) {
+		struct fsl_edma_chan *fsl_chan = &fsl_edma->chans[ch];
+
+		if (pm_runtime_get_if_active(&fsl_chan->vchan.chan.dev->device) <= 0)
+			continue;
+
 		if (intr & (0x1 << ch)) {
 			edma_writeb(fsl_edma, EDMA_CINT_CINT(ch), regs->cint);
 			fsl_edma_tx_chan_handler(&fsl_edma->chans[ch]);
 		}
+		pm_runtime_put(&fsl_chan->vchan.chan.dev->device);
 	}
 	return IRQ_HANDLED;
 }
@@ -132,13 +144,18 @@ static irqreturn_t fsl_edma3_tx_handler(int irq, void *dev_id)
 	struct fsl_edma_chan *fsl_chan = dev_id;
 	unsigned int intr;
 
-	intr = edma_readl_chreg(fsl_chan, ch_int);
-	if (!intr)
+	if (pm_runtime_get_if_active(&fsl_chan->vchan.chan.dev->device) <= 0)
 		return IRQ_NONE;
 
+	intr = edma_readl_chreg(fsl_chan, ch_int);
+	if (!intr) {
+		pm_runtime_put(&fsl_chan->vchan.chan.dev->device);
+		return IRQ_NONE;
+	}
 	edma_writel_chreg(fsl_chan, 1, ch_int);
 
 	fsl_edma_tx_chan_handler(fsl_chan);
+	pm_runtime_put(&fsl_chan->vchan.chan.dev->device);
 
 	return IRQ_HANDLED;
 }
@@ -185,20 +202,32 @@ static irqreturn_t fsl_edma3_or_err_handler(int irq, void *dev_id)
 	unsigned int err, ch, ch_es;
 	struct fsl_edma_chan *chan;
 
+	if (pm_runtime_get_if_active(fsl_edma->dma_dev.dev) <= 0)
+		return IRQ_NONE;
+
 	err = edma_readl(fsl_edma, regs->es);
-	if (!(err & EDMA_V3_MP_ES_VLD))
+	if (!(err & EDMA_V3_MP_ES_VLD)) {
+		pm_runtime_put(fsl_edma->dma_dev.dev);
 		return IRQ_NONE;
+	}
 
+	pm_runtime_put(fsl_edma->dma_dev.dev);
 	for (ch = 0; ch < fsl_edma->n_chans; ch++) {
 		chan = &fsl_edma->chans[ch];
 
+		if (pm_runtime_get_if_active(&chan->vchan.chan.dev->device) <= 0)
+			continue;
+
 		ch_es = edma_readl_chreg(chan, ch_es);
-		if (!(ch_es & EDMA_V3_CH_ES_ERR))
+		if (!(ch_es & EDMA_V3_CH_ES_ERR)) {
+			pm_runtime_put(&chan->vchan.chan.dev->device);
 			continue;
+		}
 
 		edma_writel_chreg(chan, EDMA_V3_CH_ES_ERR, ch_es);
 		fsl_edma_disable_request(chan);
 		fsl_edma->chans[ch].status = DMA_ERROR;
+		pm_runtime_put(&chan->vchan.chan.dev->device);
 	}
 
 	return IRQ_HANDLED;
@@ -210,16 +239,28 @@ static irqreturn_t fsl_edma_err_handler(int irq, void *dev_id)
 	unsigned int err, ch;
 	struct edma_regs *regs = &fsl_edma->regs;
 
+	if (pm_runtime_get_if_active(fsl_edma->dma_dev.dev) <= 0)
+		return IRQ_NONE;
+
 	err = edma_readl(fsl_edma, regs->errl);
-	if (!err)
+	if (!err) {
+		pm_runtime_put(fsl_edma->dma_dev.dev);
 		return IRQ_NONE;
+	}
 
+	pm_runtime_put(fsl_edma->dma_dev.dev);
 	for (ch = 0; ch < fsl_edma->n_chans; ch++) {
+		struct fsl_edma_chan *fsl_chan = &fsl_edma->chans[ch];
+
+		if (pm_runtime_get_if_active(&fsl_chan->vchan.chan.dev->device) <= 0)
+			continue;
+
 		if (err & (0x1 << ch)) {
 			fsl_edma_disable_request(&fsl_edma->chans[ch]);
 			edma_writeb(fsl_edma, EDMA_CERR_CERR(ch), regs->cerr);
 			fsl_edma_err_chan_handler(&fsl_edma->chans[ch]);
 		}
+		pm_runtime_put(&fsl_chan->vchan.chan.dev->device);
 	}
 	return IRQ_HANDLED;
 }
@@ -526,13 +567,6 @@ static void fsl_edma_irq_exit(
 	}
 }
 
-static void fsl_edma_disable_muxclk(void *data)
-{
-	struct fsl_edma_engine *fsl_edma = data;
-
-	clk_bulk_disable_unprepare(fsl_edma->drvdata->dmamuxs, fsl_edma->muxclk);
-}
-
 static struct fsl_edma_drvdata vf610_data = {
 	.dmamuxs = DMAMUX_NR,
 	.flags = FSL_EDMA_DRV_WRAP_IO,
@@ -632,11 +666,12 @@ static void fsl_edma3_detach_pd(struct fsl_edma_engine *fsl_edma)
 		if (fsl_edma->chan_masked & BIT(i))
 			continue;
 		fsl_chan = &fsl_edma->chans[i];
-		if (fsl_chan->pd_dev_link)
+		if (fsl_chan->pd_dev_link) {
 			device_link_del(fsl_chan->pd_dev_link);
+			fsl_chan->pd_dev_link = NULL;
+		}
 		if (fsl_chan->pd_dev) {
 			dev_pm_domain_detach(fsl_chan->pd_dev, false);
-			pm_runtime_dont_use_autosuspend(fsl_chan->pd_dev);
 			pm_runtime_set_suspended(fsl_chan->pd_dev);
 		}
 	}
@@ -667,23 +702,8 @@ static int fsl_edma3_attach_pd(struct platform_device *pdev, struct fsl_edma_eng
 			dev_err(dev, "Failed attach pd %d\n", i);
 			goto detach;
 		}
-
-		fsl_chan->pd_dev_link = device_link_add(dev, pd_chan, DL_FLAG_STATELESS |
-					     DL_FLAG_PM_RUNTIME |
-					     DL_FLAG_RPM_ACTIVE);
-		if (!fsl_chan->pd_dev_link) {
-			dev_err(dev, "Failed to add device_link to %d\n", i);
-			dev_pm_domain_detach(pd_chan, false);
-			goto detach;
-		}
-
 		fsl_chan->pd_dev = pd_chan;
-
-		pm_runtime_use_autosuspend(fsl_chan->pd_dev);
-		pm_runtime_set_autosuspend_delay(fsl_chan->pd_dev, 200);
-		pm_runtime_set_active(fsl_chan->pd_dev);
 	}
-
 	return 0;
 
 detach:
@@ -691,6 +711,36 @@ static int fsl_edma3_attach_pd(struct platform_device *pdev, struct fsl_edma_eng
 	return -EINVAL;
 }
 
+/* Per channel dma power domain */
+static int fsl_edma_chan_runtime_suspend(struct device *dev)
+{
+	struct fsl_edma_chan *fsl_chan = dev_get_drvdata(dev);
+	struct fsl_edma_engine *fsl_edma = fsl_chan->edma;
+
+	if (fsl_edma->txirq)
+		synchronize_irq(fsl_edma->txirq);
+
+	if (fsl_edma->errirq)
+		synchronize_irq(fsl_edma->errirq);
+
+	clk_disable_unprepare(fsl_chan->clk);
+
+	return 0;
+}
+
+static int fsl_edma_chan_runtime_resume(struct device *dev)
+{
+	struct fsl_edma_chan *fsl_chan = dev_get_drvdata(dev);
+
+	return clk_prepare_enable(fsl_chan->clk);
+}
+
+static struct dev_pm_domain fsl_edma_chan_pm_domain = {
+	.ops = {
+	       RUNTIME_PM_OPS(fsl_edma_chan_runtime_suspend, fsl_edma_chan_runtime_resume, NULL)
+	}
+};
+
 static int fsl_edma_probe(struct platform_device *pdev)
 {
 	struct device_node *np = pdev->dev.of_node;
@@ -738,10 +788,6 @@ static int fsl_edma_probe(struct platform_device *pdev)
 				     PTR_ERR(fsl_edma->dmaclk),
 				     "Failed to get/enable DMA clock.\n");
 
-	ret = devm_clk_prepare_enable(&pdev->dev, fsl_edma->dmaclk);
-	if (ret)
-		return dev_err_probe(&pdev->dev, ret, "Failed to enable clock\n");
-
 	ret = of_property_read_variable_u32_array(np, "dma-channel-mask", chan_mask, 1, 2);
 
 	if (ret > 0) {
@@ -772,15 +818,6 @@ static int fsl_edma_probe(struct platform_device *pdev)
 		if (ret)
 			return dev_err_probe(&pdev->dev, ret,
 					     "Failed to get DMAMUX block clock.\n");
-
-		ret = clk_bulk_prepare_enable(fsl_edma->drvdata->dmamuxs, fsl_edma->muxclk);
-		if (ret)
-			return dev_err_probe(&pdev->dev, ret,
-					     "Failed to enable DMAMUX block clock.\n");
-
-		ret = devm_add_action_or_reset(&pdev->dev, fsl_edma_disable_muxclk, fsl_edma);
-		if (ret)
-			return dev_err_probe(&pdev->dev, ret, "Failed to add cleanup action.\n");
 	}
 
 	fsl_edma->big_endian = of_property_read_bool(np, "big-endian");
@@ -826,21 +863,10 @@ static int fsl_edma_probe(struct platform_device *pdev)
 		fsl_chan->clk = devm_clk_get_optional(&pdev->dev, (const char *)clk_name);
 		if (IS_ERR(fsl_chan->clk))
 			return PTR_ERR(fsl_chan->clk);
-		ret = devm_clk_prepare_enable(&pdev->dev, fsl_chan->clk);
-		if (ret)
-			return dev_err_probe(&pdev->dev, ret, "Failed to enable clock\n");
 		fsl_chan->pdev = pdev;
 		vchan_init(&fsl_chan->vchan, &fsl_edma->dma_dev);
-
-		edma_write_tcdreg(fsl_chan, cpu_to_le32(0), csr);
-		fsl_edma_chan_mux(fsl_chan, 0, false);
-		clk_disable_unprepare(fsl_chan->clk);
 	}
 
-	ret = fsl_edma->drvdata->setup_irq(pdev, fsl_edma);
-	if (ret)
-		return ret;
-
 	dma_cap_set(DMA_PRIVATE, fsl_edma->dma_dev.cap_mask);
 	dma_cap_set(DMA_SLAVE, fsl_edma->dma_dev.cap_mask);
 	dma_cap_set(DMA_CYCLIC, fsl_edma->dma_dev.cap_mask);
@@ -891,6 +917,64 @@ static int fsl_edma_probe(struct platform_device *pdev)
 		return dev_err_probe(&pdev->dev, ret,
 				     "Can't register Freescale eDMA engine.\n");
 
+	ret = devm_pm_runtime_enable(&pdev->dev);
+	if (ret)
+		return dev_err_probe(&pdev->dev, ret, "Can't enable eDMA engine PM runtime!\n");
+
+	ret = pm_runtime_resume_and_get(&pdev->dev);
+	if (ret < 0)
+		return dev_err_probe(&pdev->dev, ret, "Failed to resume eDMA engine!\n");
+
+	ret = fsl_edma->drvdata->setup_irq(pdev, fsl_edma);
+	if (ret)
+		return ret;
+
+	for (i = 0; i < fsl_edma->n_chans; i++) {
+		struct fsl_edma_chan *fsl_chan = &fsl_edma->chans[i];
+		struct device *chan_dev;
+
+		if (fsl_edma->chan_masked & BIT(i))
+			continue;
+
+		chan_dev = &fsl_chan->vchan.chan.dev->device;
+		dev_set_drvdata(chan_dev, fsl_chan);
+		dev_pm_domain_set(chan_dev, &fsl_edma_chan_pm_domain);
+
+		if (fsl_chan->pd_dev) {
+			fsl_chan->pd_dev_link = device_link_add(chan_dev, fsl_chan->pd_dev,
+								DL_FLAG_STATELESS |
+								DL_FLAG_PM_RUNTIME);
+			if (!fsl_chan->pd_dev_link) {
+				return dev_err_probe(&pdev->dev, -EINVAL,
+						     "Failed to add device_link to %d!\n", i);
+			}
+		}
+		ret = devm_pm_runtime_enable(chan_dev);
+		if (ret)
+			return dev_err_probe(&pdev->dev, ret,
+					     "Can't enable eDMA channel PM runtime!\n");
+
+		if (fsl_chan->pd_dev) {
+			ret = pm_runtime_resume_and_get(fsl_chan->pd_dev);
+			if (ret)
+				return dev_err_probe(&pdev->dev, ret,
+						     "Failed to power on eDMA channel %d!\n",
+						     fsl_chan->vchan.chan.chan_id);
+		}
+
+		ret = pm_runtime_resume_and_get(chan_dev);
+		if (ret < 0) {
+			return dev_err_probe(&pdev->dev, ret,
+					     "Failed to resume eDMA channel %d!\n",
+					     fsl_chan->vchan.chan.chan_id);
+		}
+		edma_write_tcdreg(fsl_chan, cpu_to_le32(0), csr);
+		fsl_edma_chan_mux(fsl_chan, 0, false);
+		pm_runtime_put_sync_suspend(chan_dev);
+		if (fsl_chan->pd_dev)
+			pm_runtime_put_sync_suspend(fsl_chan->pd_dev);
+	}
+
 	ret = devm_of_dma_controller_register(&pdev->dev, np,
 			drvdata->dmamuxs ? fsl_edma_xlate : fsl_edma3_xlate,
 			fsl_edma);
@@ -902,6 +986,7 @@ static int fsl_edma_probe(struct platform_device *pdev)
 	if (!(drvdata->flags & FSL_EDMA_DRV_SPLIT_REG))
 		edma_writel(fsl_edma, EDMA_CR_ERGA | EDMA_CR_ERCA, regs->cr);
 
+	pm_runtime_put_sync_suspend(&pdev->dev);
 	return 0;
 }
 
@@ -924,6 +1009,19 @@ static int fsl_edma_suspend_late(struct device *dev)
 		fsl_chan = &fsl_edma->chans[i];
 		if (fsl_edma->chan_masked & BIT(i))
 			continue;
+
+		/*
+		 * Skip channel if:
+		 * 1. Runtime PM already suspended.
+		 * 2. Channel without power domain, and the channel source ID is zero,
+		 * so the channel isn't assigned.
+		 */
+		if (pm_runtime_status_suspended(&fsl_chan->vchan.chan.dev->device) ||
+		    (!(fsl_edma->drvdata->flags & FSL_EDMA_DRV_HAS_PD) &&
+		     (fsl_edma->drvdata->flags & FSL_EDMA_DRV_SPLIT_REG) &&
+		     !fsl_chan->srcid))
+			continue;
+
 		spin_lock_irqsave(&fsl_chan->vchan.lock, flags);
 		/* Make sure chan is idle or will force disable. */
 		if (unlikely(fsl_chan->status == DMA_IN_PROGRESS)) {
@@ -943,19 +1041,56 @@ static int fsl_edma_resume_early(struct device *dev)
 {
 	struct fsl_edma_engine *fsl_edma = dev_get_drvdata(dev);
 	struct fsl_edma_chan *fsl_chan;
-	struct edma_regs *regs = &fsl_edma->regs;
 	int i;
 
 	for (i = 0; i < fsl_edma->n_chans; i++) {
 		fsl_chan = &fsl_edma->chans[i];
 		if (fsl_edma->chan_masked & BIT(i))
 			continue;
+
+		if (pm_runtime_status_suspended(&fsl_chan->vchan.chan.dev->device) ||
+		    (!(fsl_edma->drvdata->flags & FSL_EDMA_DRV_HAS_PD) &&
+		     (fsl_edma->drvdata->flags & FSL_EDMA_DRV_SPLIT_REG) &&
+		     !fsl_chan->srcid))
+			continue;
+
 		fsl_chan->pm_state = RUNNING;
 		edma_write_tcdreg(fsl_chan, 0, csr);
 		if (fsl_chan->srcid != 0)
 			fsl_edma_chan_mux(fsl_chan, fsl_chan->srcid, true);
 	}
 
+	return 0;
+}
+
+/* edma engine runtime system/resume */
+static int fsl_edma_runtime_suspend(struct device *dev)
+{
+	struct fsl_edma_engine *fsl_edma = dev_get_drvdata(dev);
+
+	clk_bulk_disable_unprepare(fsl_edma->drvdata->dmamuxs, fsl_edma->muxclk);
+
+	clk_disable_unprepare(fsl_edma->dmaclk);
+
+	return 0;
+}
+
+static int fsl_edma_runtime_resume(struct device *dev)
+{
+	struct fsl_edma_engine *fsl_edma = dev_get_drvdata(dev);
+	struct edma_regs *regs = &fsl_edma->regs;
+	int ret;
+
+	ret = clk_bulk_prepare_enable(fsl_edma->drvdata->dmamuxs, fsl_edma->muxclk);
+	if (ret)
+		return ret;
+
+	ret = clk_prepare_enable(fsl_edma->dmaclk);
+	if (ret) {
+		clk_bulk_disable_unprepare(fsl_edma->drvdata->dmamuxs, fsl_edma->muxclk);
+		return ret;
+	}
+
 	if (!(fsl_edma->drvdata->flags & FSL_EDMA_DRV_SPLIT_REG))
 		edma_writel(fsl_edma, EDMA_CR_ERGA | EDMA_CR_ERCA, regs->cr);
 
@@ -970,6 +1105,7 @@ static int fsl_edma_resume_early(struct device *dev)
 static const struct dev_pm_ops fsl_edma_pm_ops = {
 	.suspend_late   = fsl_edma_suspend_late,
 	.resume_early   = fsl_edma_resume_early,
+	RUNTIME_PM_OPS(fsl_edma_runtime_suspend, fsl_edma_runtime_resume, NULL)
 };
 
 static struct platform_driver fsl_edma_driver = {

-- 
2.34.1

Re: [PATCH v6 4/5] dmaengine: fsl-edma: add runtime suspend/resume support

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [High] Skipping hardware interrupt acknowledgment for inactive PM channels leads to an unrecoverable infinite IRQ storm.
- [High] The channel runtime suspend callback calls `synchronize_irq()` with a negative error code and fails to synchronize actual per-channel shared IRQs.
- [High] Accessing a freed `device_link` in `fsl_edma3_detach_pd()` due to devres teardown order inversion.
- [Medium] The DMA engine and its channels are exposed to clients before their IRQs and runtime PM domains are fully initialized.
- [Medium] Runtime PM usage counters for the engine device and channel power domains are permanently leaked on multiple error paths during probe.
--

commit a5800ff547600d938eb443a21684d6291b4658d1
Author: Joy Zou <joy.zou@nxp.com>

dmaengine: fsl-edma: add runtime suspend/resume support

Introduce runtime suspend and resume support for FSL eDMA. Enable
per-channel power domain management to facilitate runtime suspend and
resume operations.

> diff --git a/drivers/dma/fsl-edma-main.c b/drivers/dma/fsl-edma-main.c
> index fe02b68d75fd3..3518dfb4292d2 100644
> --- a/drivers/dma/fsl-edma-main.c
> +++ b/drivers/dma/fsl-edma-main.c
> @@ -37,15 +37,27 @@ static irqreturn_t fsl_edma_tx_handler(int irq, void *dev_id)
>  	unsigned int intr, ch;
>  	struct edma_regs *regs = &fsl_edma->regs;
>  
> +	if (pm_runtime_get_if_active(fsl_edma->dma_dev.dev) <= 0)
> +		return IRQ_NONE;
> +
>  	intr = edma_readl(fsl_edma, regs->intl);
> -	if (!intr)
> +	if (!intr) {
> +		pm_runtime_put(fsl_edma->dma_dev.dev);
>  		return IRQ_NONE;
> +	}
>  
> +	pm_runtime_put(fsl_edma->dma_dev.dev);
>  	for (ch = 0; ch < fsl_edma->n_chans; ch++) {
> +		struct fsl_edma_chan *fsl_chan = &fsl_edma->chans[ch];
> +
> +		if (pm_runtime_get_if_active(&fsl_chan->vchan.chan.dev->device) <= 0)
> +			continue;
> +
>  		if (intr & (0x1 << ch)) {
>  			edma_writeb(fsl_edma, EDMA_CINT_CINT(ch), regs->cint);
>  			fsl_edma_tx_chan_handler(&fsl_edma->chans[ch]);
>  		}
> +		pm_runtime_put(&fsl_chan->vchan.chan.dev->device);
>  	}
>  	return IRQ_HANDLED;
>  }

[Severity: High]
If a channel is runtime suspended (e.g., due to a hardware race or a spurious
interrupt), the interrupt bit is never cleared because the loop evaluates
pm_runtime_get_if_active() <= 0 and continues to the next iteration.

However, the function ultimately returns IRQ_HANDLED unconditionally. 
Could returning IRQ_HANDLED without clearing the hardware interrupt flag 
trick the interrupt controller into thinking the event was serviced, 
potentially leading to an infinite IRQ storm?

This same pattern appears in fsl_edma_err_handler() and 
fsl_edma3_or_err_handler().

[ ... ]

> @@ -691,6 +711,36 @@ static int fsl_edma3_attach_pd(struct platform_device *pdev, struct fsl_edma_eng
>  	return -EINVAL;
>  }
>  
> +/* Per channel dma power domain */
> +static int fsl_edma_chan_runtime_suspend(struct device *dev)
> +{
> +	struct fsl_edma_chan *fsl_chan = dev_get_drvdata(dev);
> +	struct fsl_edma_engine *fsl_edma = fsl_chan->edma;
> +
> +	if (fsl_edma->txirq)
> +		synchronize_irq(fsl_edma->txirq);
> +
> +	if (fsl_edma->errirq)
> +		synchronize_irq(fsl_edma->errirq);
> +
> +	clk_disable_unprepare(fsl_chan->clk);
> +
> +	return 0;
> +}

[Severity: High]
Are we synchronizing the correct IRQs before disabling the channel clock?

During probe, global IRQs like fsl_edma->txirq are initialized to -EINVAL 
for SoCs using per-channel IRQs. This check evaluates to true because -EINVAL
is non-zero, causing a negative error code to be passed to synchronize_irq().

Additionally, this code doesn't synchronize the channel-specific shared 
IRQ (fsl_chan->txirq). Could this allow a shared IRQ handler to execute 
and access hardware after the clock is disabled?

[ ... ]

> @@ -891,6 +917,64 @@ static int fsl_edma_probe(struct platform_device *pdev)
>  		return dev_err_probe(&pdev->dev, ret,
>  				     "Can't register Freescale eDMA engine.\n");
>  
> +	ret = devm_pm_runtime_enable(&pdev->dev);
> +	if (ret)
> +		return dev_err_probe(&pdev->dev, ret, "Can't enable eDMA engine PM runtime!\n");
> +
> +	ret = pm_runtime_resume_and_get(&pdev->dev);
> +	if (ret < 0)
> +		return dev_err_probe(&pdev->dev, ret, "Failed to resume eDMA engine!\n");
> +
> +	ret = fsl_edma->drvdata->setup_irq(pdev, fsl_edma);
> +	if (ret)
> +		return ret;

[Severity: Medium]
By calling dmaenginem_async_device_register() just prior to enabling 
runtime PM and setting up IRQs, do we expose the channels to clients 
prematurely?

If a client allocates a channel concurrently before runtime PM is enabled, 
fsl_edma_alloc_chan_resources() will invoke pm_runtime_resume_and_get(), 
which could fail with -EINVAL.

[Severity: Medium]
If fsl_edma->drvdata->setup_irq() fails here, we return the error code 
directly. Does this leave the pdev->dev usage counter permanently 
incremented from the pm_runtime_resume_and_get() call just above?

> +	for (i = 0; i < fsl_edma->n_chans; i++) {
> +		struct fsl_edma_chan *fsl_chan = &fsl_edma->chans[i];
> +		struct device *chan_dev;
> +
> +		if (fsl_edma->chan_masked & BIT(i))
> +			continue;
> +
> +		chan_dev = &fsl_chan->vchan.chan.dev->device;
> +		dev_set_drvdata(chan_dev, fsl_chan);
> +		dev_pm_domain_set(chan_dev, &fsl_edma_chan_pm_domain);
> +
> +		if (fsl_chan->pd_dev) {
> +			fsl_chan->pd_dev_link = device_link_add(chan_dev, fsl_chan->pd_dev,
> +								DL_FLAG_STATELESS |
> +								DL_FLAG_PM_RUNTIME);
> +			if (!fsl_chan->pd_dev_link) {
> +				return dev_err_probe(&pdev->dev, -EINVAL,
> +						     "Failed to add device_link to %d!\n", i);
> +			}
> +		}

[Severity: High]
Could the teardown order of these stateless device links result in a 
use-after-free on probe failure or driver unbind?

Earlier in probe, devm_fsl_edma3_detach_pd is registered as a devres 
cleanup action before the DMA engine is registered. 

However, the stateless device link is added here with the channel device 
(chan_dev) as the consumer. On unbind or probe failure, devres actions 
execute in reverse order:

1. The DMA engine unregisters, destroying chan_dev.
2. The driver core automatically purges and frees all device links attached 
   to the deleted device.
3. devm_fsl_edma3_detach_pd executes and calls device_link_del() on the 
   now-freed fsl_chan->pd_dev_link.

> +		ret = devm_pm_runtime_enable(chan_dev);
> +		if (ret)
> +			return dev_err_probe(&pdev->dev, ret,
> +					     "Can't enable eDMA channel PM runtime!\n");
> +
> +		if (fsl_chan->pd_dev) {
> +			ret = pm_runtime_resume_and_get(fsl_chan->pd_dev);
> +			if (ret)
> +				return dev_err_probe(&pdev->dev, ret,
> +						     "Failed to power on eDMA channel %d!\n",
> +						     fsl_chan->vchan.chan.chan_id);
> +		}
> +
> +		ret = pm_runtime_resume_and_get(chan_dev);
> +		if (ret < 0) {
> +			return dev_err_probe(&pdev->dev, ret,
> +					     "Failed to resume eDMA channel %d!\n",
> +					     fsl_chan->vchan.chan.chan_id);
> +		}

[Severity: Medium]
If pm_runtime_resume_and_get(chan_dev) fails here, we return directly.
Does this leak both the engine's usage counter (pdev->dev) and the channel 
power domain's usage counter (fsl_chan->pd_dev) acquired earlier in this 
function?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260701-b4-edma-runtime-opt-v6-0-354ff4229c00@oss.nxp.com?part=4

Re: [PATCH v6 4/5] dmaengine: fsl-edma: add runtime suspend/resume support

[Frank Li]

On Wed, Jul 01, 2026 at 09:43:12AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
> - [High] Skipping hardware interrupt acknowledgment for inactive PM channels leads to an unrecoverable infinite IRQ storm.
> - [High] The channel runtime suspend callback calls `synchronize_irq()` with a negative error code and fails to synchronize actual per-channel shared IRQs.
> - [High] Accessing a freed `device_link` in `fsl_edma3_detach_pd()` due to devres teardown order inversion.
> - [Medium] The DMA engine and its channels are exposed to clients before their IRQs and runtime PM domains are fully initialized.
> - [Medium] Runtime PM usage counters for the engine device and channel power domains are permanently leaked on multiple error paths during probe.
> --
>
> commit a5800ff547600d938eb443a21684d6291b4658d1
> Author: Joy Zou <joy.zou@nxp.com>
>
> dmaengine: fsl-edma: add runtime suspend/resume support
>
> Introduce runtime suspend and resume support for FSL eDMA. Enable
> per-channel power domain management to facilitate runtime suspend and
> resume operations.
>
> > diff --git a/drivers/dma/fsl-edma-main.c b/drivers/dma/fsl-edma-main.c
> > index fe02b68d75fd3..3518dfb4292d2 100644
> > --- a/drivers/dma/fsl-edma-main.c
> > +++ b/drivers/dma/fsl-edma-main.c
> > @@ -37,15 +37,27 @@ static irqreturn_t fsl_edma_tx_handler(int irq, void *dev_id)
> >  	unsigned int intr, ch;
> >  	struct edma_regs *regs = &fsl_edma->regs;
> >
> > +	if (pm_runtime_get_if_active(fsl_edma->dma_dev.dev) <= 0)
> > +		return IRQ_NONE;
> > +
> >  	intr = edma_readl(fsl_edma, regs->intl);
> > -	if (!intr)
> > +	if (!intr) {
> > +		pm_runtime_put(fsl_edma->dma_dev.dev);
> >  		return IRQ_NONE;
> > +	}
> >
> > +	pm_runtime_put(fsl_edma->dma_dev.dev);
> >  	for (ch = 0; ch < fsl_edma->n_chans; ch++) {
> > +		struct fsl_edma_chan *fsl_chan = &fsl_edma->chans[ch];
> > +
> > +		if (pm_runtime_get_if_active(&fsl_chan->vchan.chan.dev->device) <= 0)
> > +			continue;
> > +
> >  		if (intr & (0x1 << ch)) {
> >  			edma_writeb(fsl_edma, EDMA_CINT_CINT(ch), regs->cint);
> >  			fsl_edma_tx_chan_handler(&fsl_edma->chans[ch]);
> >  		}
> > +		pm_runtime_put(&fsl_chan->vchan.chan.dev->device);
> >  	}
> >  	return IRQ_HANDLED;
> >  }
>
> [Severity: High]
> If a channel is runtime suspended (e.g., due to a hardware race or a spurious
> interrupt), the interrupt bit is never cleared because the loop evaluates
> pm_runtime_get_if_active() <= 0 and continues to the next iteration.

Make sense, please check other AI result

Frank

>
> However, the function ultimately returns IRQ_HANDLED unconditionally.
> Could returning IRQ_HANDLED without clearing the hardware interrupt flag
> trick the interrupt controller into thinking the event was serviced,
> potentially leading to an infinite IRQ storm?
>
> This same pattern appears in fsl_edma_err_handler() and
> fsl_edma3_or_err_handler().
>
> [ ... ]
>
> > @@ -691,6 +711,36 @@ static int fsl_edma3_attach_pd(struct platform_device *pdev, struct fsl_edma_eng
> >  	return -EINVAL;
> >  }
> >
> > +/* Per channel dma power domain */
> > +static int fsl_edma_chan_runtime_suspend(struct device *dev)
> > +{
> > +	struct fsl_edma_chan *fsl_chan = dev_get_drvdata(dev);
> > +	struct fsl_edma_engine *fsl_edma = fsl_chan->edma;
> > +
> > +	if (fsl_edma->txirq)
> > +		synchronize_irq(fsl_edma->txirq);
> > +
> > +	if (fsl_edma->errirq)
> > +		synchronize_irq(fsl_edma->errirq);
> > +
> > +	clk_disable_unprepare(fsl_chan->clk);
> > +
> > +	return 0;
> > +}
>
> [Severity: High]
> Are we synchronizing the correct IRQs before disabling the channel clock?
>
> During probe, global IRQs like fsl_edma->txirq are initialized to -EINVAL
> for SoCs using per-channel IRQs. This check evaluates to true because -EINVAL
> is non-zero, causing a negative error code to be passed to synchronize_irq().
>
> Additionally, this code doesn't synchronize the channel-specific shared
> IRQ (fsl_chan->txirq). Could this allow a shared IRQ handler to execute
> and access hardware after the clock is disabled?
>
> [ ... ]
>
> > @@ -891,6 +917,64 @@ static int fsl_edma_probe(struct platform_device *pdev)
> >  		return dev_err_probe(&pdev->dev, ret,
> >  				     "Can't register Freescale eDMA engine.\n");
> >
> > +	ret = devm_pm_runtime_enable(&pdev->dev);
> > +	if (ret)
> > +		return dev_err_probe(&pdev->dev, ret, "Can't enable eDMA engine PM runtime!\n");
> > +
> > +	ret = pm_runtime_resume_and_get(&pdev->dev);
> > +	if (ret < 0)
> > +		return dev_err_probe(&pdev->dev, ret, "Failed to resume eDMA engine!\n");
> > +
> > +	ret = fsl_edma->drvdata->setup_irq(pdev, fsl_edma);
> > +	if (ret)
> > +		return ret;
>
> [Severity: Medium]
> By calling dmaenginem_async_device_register() just prior to enabling
> runtime PM and setting up IRQs, do we expose the channels to clients
> prematurely?
>
> If a client allocates a channel concurrently before runtime PM is enabled,
> fsl_edma_alloc_chan_resources() will invoke pm_runtime_resume_and_get(),
> which could fail with -EINVAL.
>
> [Severity: Medium]
> If fsl_edma->drvdata->setup_irq() fails here, we return the error code
> directly. Does this leave the pdev->dev usage counter permanently
> incremented from the pm_runtime_resume_and_get() call just above?
>
> > +	for (i = 0; i < fsl_edma->n_chans; i++) {
> > +		struct fsl_edma_chan *fsl_chan = &fsl_edma->chans[i];
> > +		struct device *chan_dev;
> > +
> > +		if (fsl_edma->chan_masked & BIT(i))
> > +			continue;
> > +
> > +		chan_dev = &fsl_chan->vchan.chan.dev->device;
> > +		dev_set_drvdata(chan_dev, fsl_chan);
> > +		dev_pm_domain_set(chan_dev, &fsl_edma_chan_pm_domain);
> > +
> > +		if (fsl_chan->pd_dev) {
> > +			fsl_chan->pd_dev_link = device_link_add(chan_dev, fsl_chan->pd_dev,
> > +								DL_FLAG_STATELESS |
> > +								DL_FLAG_PM_RUNTIME);
> > +			if (!fsl_chan->pd_dev_link) {
> > +				return dev_err_probe(&pdev->dev, -EINVAL,
> > +						     "Failed to add device_link to %d!\n", i);
> > +			}
> > +		}
>
> [Severity: High]
> Could the teardown order of these stateless device links result in a
> use-after-free on probe failure or driver unbind?
>
> Earlier in probe, devm_fsl_edma3_detach_pd is registered as a devres
> cleanup action before the DMA engine is registered.
>
> However, the stateless device link is added here with the channel device
> (chan_dev) as the consumer. On unbind or probe failure, devres actions
> execute in reverse order:
>
> 1. The DMA engine unregisters, destroying chan_dev.
> 2. The driver core automatically purges and frees all device links attached
>    to the deleted device.
> 3. devm_fsl_edma3_detach_pd executes and calls device_link_del() on the
>    now-freed fsl_chan->pd_dev_link.
>
> > +		ret = devm_pm_runtime_enable(chan_dev);
> > +		if (ret)
> > +			return dev_err_probe(&pdev->dev, ret,
> > +					     "Can't enable eDMA channel PM runtime!\n");
> > +
> > +		if (fsl_chan->pd_dev) {
> > +			ret = pm_runtime_resume_and_get(fsl_chan->pd_dev);
> > +			if (ret)
> > +				return dev_err_probe(&pdev->dev, ret,
> > +						     "Failed to power on eDMA channel %d!\n",
> > +						     fsl_chan->vchan.chan.chan_id);
> > +		}
> > +
> > +		ret = pm_runtime_resume_and_get(chan_dev);
> > +		if (ret < 0) {
> > +			return dev_err_probe(&pdev->dev, ret,
> > +					     "Failed to resume eDMA channel %d!\n",
> > +					     fsl_chan->vchan.chan.chan_id);
> > +		}
>
> [Severity: Medium]
> If pm_runtime_resume_and_get(chan_dev) fails here, we return directly.
> Does this leak both the engine's usage counter (pdev->dev) and the channel
> power domain's usage counter (fsl_chan->pd_dev) acquired earlier in this
> function?
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260701-b4-edma-runtime-opt-v6-0-354ff4229c00@oss.nxp.com?part=4

[PATCH v6 5/5] dmaengine: fsl-edma: fix use-after-free after dev_pm_domain_detach()

[joy.zou]

From: Joy Zou <joy.zou@nxp.com>

Remove pm_runtime_set_suspended() call after dev_pm_domain_detach()
to prevent use-after-free. When a power domain is attached via
dev_pm_domain_attach_by_id(), calling dev_pm_domain_detach()
unregisters and frees the underlying virtual device, making
fsl_chan->pd_dev a dangling pointer.

Accessing the freed pointer in pm_runtime_set_suspended() triggers
undefined behavior and potential crashes.

Fixes: ccfa3131d4a0 ("dmaengine: fsl-edma: implement the cleanup path of fsl_edma3_attach_pd()")

Signed-off-by: Joy Zou <joy.zou@nxp.com>
---
 drivers/dma/fsl-edma-main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/dma/fsl-edma-main.c b/drivers/dma/fsl-edma-main.c
index 3518dfb4292d..266cc082a9f0 100644
--- a/drivers/dma/fsl-edma-main.c
+++ b/drivers/dma/fsl-edma-main.c
@@ -671,8 +671,8 @@ static void fsl_edma3_detach_pd(struct fsl_edma_engine *fsl_edma)
 			fsl_chan->pd_dev_link = NULL;
 		}
 		if (fsl_chan->pd_dev) {
-			dev_pm_domain_detach(fsl_chan->pd_dev, false);
 			pm_runtime_set_suspended(fsl_chan->pd_dev);
+			dev_pm_domain_detach(fsl_chan->pd_dev, false);
 		}
 	}
 }

-- 
2.34.1

Re: [PATCH v6 5/5] dmaengine: fsl-edma: fix use-after-free after dev_pm_domain_detach()

[sashiko-bot]

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

New issues:
- [Medium] The fix moves a meaningless `pm_runtime_set_suspended()` call instead of deleting it, creating dead code and an API misuse.

Pre-existing issues:
- [High] Use-after-free of device link pointer in the probe error path due to incorrect devres unroll ordering.
- [High] Missing PM runtime checks in `fsl_edma3_err_handler_per_chan` and `fsl_edma3_err_handler_shared` interrupt handlers.
- [High] The `fsl_edma_chan_runtime_suspend()` callback synchronizes the global engine IRQs instead of the shared per-channel IRQs.
- [High] Runtime PM usage counter leak on the probe error path.
- [High] Unacknowledged hardware interrupts cause an IRQ storm and system lockup if a channel is suspended.
--

commit be8ea96b6aa69c23219ef38987b11d3332453c91
Author: Joy Zou <joy.zou@nxp.com>

dmaengine: fsl-edma: fix use-after-free after dev_pm_domain_detach()

Remove pm_runtime_set_suspended() call after dev_pm_domain_detach()
to prevent use-after-free. When a power domain is attached via
dev_pm_domain_attach_by_id(), calling dev_pm_domain_detach()
unregisters and frees the underlying virtual device, making
fsl_chan->pd_dev a dangling pointer.

Accessing the freed pointer in pm_runtime_set_suspended() triggers
undefined behavior and potential crashes.

Fixes: ccfa3131d4a0 ("dmaengine: fsl-edma: implement the cleanup path of fsl_edma3_attach_pd()")

> diff --git a/drivers/dma/fsl-edma-main.c b/drivers/dma/fsl-edma-main.c
> index 3518dfb4292d2..266cc082a9f04 100644
> --- a/drivers/dma/fsl-edma-main.c
> +++ b/drivers/dma/fsl-edma-main.c
> @@ -671,8 +671,8 @@ static void fsl_edma3_detach_pd(struct fsl_edma_engine *fsl_edma)
>  			fsl_chan->pd_dev_link = NULL;
>  		}

[Severity: High]
This isn't a bug introduced by this patch, but does this attempt to delete
a device link that has already been purged and freed by the dmaengine devres
rollback?

If fsl_edma_probe() fails after successfully calling device_link_add(), the
devres unwinding triggers. First, dmaenginem_async_device_unregister()
executes, which unregisters chan_dev. 

The driver core automatically purges and frees all device links associated
with this device, freeing fsl_chan->pd_dev_link. Next in the devres stack,
devm_fsl_edma3_detach_pd() executes and calls
device_link_del(fsl_chan->pd_dev_link), which could result in a
use-after-free.

>  		if (fsl_chan->pd_dev) {
> -			dev_pm_domain_detach(fsl_chan->pd_dev, false);
>  			pm_runtime_set_suspended(fsl_chan->pd_dev);
> +			dev_pm_domain_detach(fsl_chan->pd_dev, false);

[Severity: Medium]
The commit message states "Remove pm_runtime_set_suspended() call after
dev_pm_domain_detach()", but the call was moved to occur before detach rather
than being completely deleted.

Since fsl_chan->pd_dev is a virtual device with disable_depth == 0, doesn't
pm_runtime_set_suspended() immediately return -EAGAIN and do nothing?

Should this call be completely removed to avoid dead code and API misuse?

>  		}
>  	}
>  }

While reviewing the file, I noticed a few additional pre-existing issues.

In drivers/dma/fsl-edma-main.c:fsl_edma3_err_handler_per_chan():

	fsl_edma3_err_check(fsl_chan);

[Severity: High]
This is a pre-existing issue, but does the per-channel error handler lack the
necessary pm_runtime_get_if_active() check?

If a shared interrupt triggers or a spurious interrupt fires while the eDMA
hardware is suspended, these handlers invoke fsl_edma3_err_check(), which
performs direct MMIO register reads. Accessing registers on a suspended device
with its clock disabled will crash the system.

A similar issue appears in fsl_edma3_err_handler_shared():

		fsl_edma3_err_check(&fsl_edma->chans[ch]);

[Severity: High]
This is a pre-existing issue, but the shared error handler accesses all
channel registers without verifying their PM runtime state.


In drivers/dma/fsl-edma-main.c:fsl_edma_chan_runtime_suspend():

	if (fsl_edma->txirq)
		synchronize_irq(fsl_edma->txirq);

	if (fsl_edma->errirq)
		synchronize_irq(fsl_edma->errirq);

[Severity: High]
This is a pre-existing issue, but does the runtime suspend callback omit the
synchronize_irq() calls for the actual IRQF_SHARED IRQs requested for the
specific channel?

In fsl_edma_alloc_chan_resources(), the driver requests per-channel IRQs
using the IRQF_SHARED flag. However, fsl_edma_chan_runtime_suspend()
synchronizes the global engine IRQs instead of the channel's IRQs. Could a
per-channel IRQ handler execute concurrently while the channel hardware
clock/power is being disabled?


In drivers/dma/fsl-edma-main.c:fsl_edma_probe():

	ret = fsl_edma->drvdata->setup_irq(pdev, fsl_edma);
	if (ret)
		return ret;

[Severity: High]
This is a pre-existing issue, but does this early return leak the PM usage
counter of pdev->dev and any already resumed channels?

During probe, the driver calls pm_runtime_resume_and_get(&pdev->dev) and
pm_runtime_resume_and_get(chan_dev), incrementing their usage counters. If an
error occurs subsequently, the function returns directly without dropping the
counters, preventing the hardware from suspending upon subsequent unbind or
retry.


In drivers/dma/fsl-edma-main.c:fsl_edma_tx_handler():

		if (pm_runtime_get_if_active(&fsl_chan->vchan.chan.dev->device) <= 0)
			continue;

		if (intr & (0x1 << ch)) {
			edma_writeb(fsl_edma, EDMA_CINT_CINT(ch), regs->cint);

[Severity: High]
This is a pre-existing issue, but if the channel's PM reference cannot be
acquired, does the loop skip clearing the completion interrupt and leave the
hardware IRQ asserted?

If a channel with a pending interrupt is suspended or suspending,
pm_runtime_get_if_active() returns <= 0 and the handler skips the channel
using continue. Because the hardware interrupt source is never cleared, but
the handler returns IRQ_HANDLED, the interrupt controller will immediately
re-trigger the ISR, potentially causing an infinite IRQ storm and CPU lockup.

This also appears in fsl_edma_err_handler():

		if (pm_runtime_get_if_active(&fsl_chan->vchan.chan.dev->device) <= 0)
			continue;

		if (err & (0x1 << ch)) {
			fsl_edma_disable_request(&fsl_edma->chans[ch]);
			edma_writeb(fsl_edma, EDMA_CERR_CERR(ch), regs->cerr);

[Severity: High]
This is a pre-existing issue, but it appears to skip clearing the error
interrupt here under the same conditions.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260701-b4-edma-runtime-opt-v6-0-354ff4229c00@oss.nxp.com?part=5

Re: [PATCH v6 5/5] dmaengine: fsl-edma: fix use-after-free after dev_pm_domain_detach()

[Frank Li]

On Wed, Jul 01, 2026 at 05:29:27PM +0800, joy.zou@oss.nxp.com wrote:
> From: Joy Zou <joy.zou@nxp.com>
>
> Remove pm_runtime_set_suspended() call after dev_pm_domain_detach()

s/Remove/move

> to prevent use-after-free. When a power domain is attached via
> dev_pm_domain_attach_by_id(), calling dev_pm_domain_detach()
> unregisters and frees the underlying virtual device, making

what's means of "underlying virtual devic"

> fsl_chan->pd_dev a dangling pointer.
>
> Accessing the freed pointer in pm_runtime_set_suspended() triggers
> undefined behavior and potential crashes.

Need this paragraph, every one know that. UAF is common issue.

Frank
>
> Fixes: ccfa3131d4a0 ("dmaengine: fsl-edma: implement the cleanup path of fsl_edma3_attach_pd()")
>
> Signed-off-by: Joy Zou <joy.zou@nxp.com>
> ---
>  drivers/dma/fsl-edma-main.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/dma/fsl-edma-main.c b/drivers/dma/fsl-edma-main.c
> index 3518dfb4292d..266cc082a9f0 100644
> --- a/drivers/dma/fsl-edma-main.c
> +++ b/drivers/dma/fsl-edma-main.c
> @@ -671,8 +671,8 @@ static void fsl_edma3_detach_pd(struct fsl_edma_engine *fsl_edma)
>  			fsl_chan->pd_dev_link = NULL;
>  		}
>  		if (fsl_chan->pd_dev) {
> -			dev_pm_domain_detach(fsl_chan->pd_dev, false);
>  			pm_runtime_set_suspended(fsl_chan->pd_dev);
> +			dev_pm_domain_detach(fsl_chan->pd_dev, false);
>  		}
>  	}
>  }
>
> --
> 2.34.1
>