* [PATCH v7 0/4] add runtime suspend/resume support
@ 2026-10-23 10:59 joy.zou
2026-10-23 10:59 ` [PATCH v7 1/4] dmaengine: fsl-edma: use devm_clk_get_optional_enabled() for channel clock joy.zou
` (3 more replies)
0 siblings, 4 replies; 6+ messages in thread
From: joy.zou @ 2026-10-23 10:59 UTC (permalink / raw)
To: Frank Li, Vinod Koul, Joe Hattori
Cc: Joy Zou, Frank Li, imx, dmaengine, linux-kernel, 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 V7:
- replace devm_clk_get_optional() with devm_clk_get_optional_enable() for
patch #1 and #2.
- replace devm_clk_get_optional() with devm_clk_bulk_get_optional_enable()
for patch #3.
- replace devm_clk_get_optional_enable() with devm_clk_get_optional() for patch #4.
- replace devm_clk_bulk_get_optional_enable() with devm_clk_bulk_get_optional()
for patch #4.
- use PM_RUNTIME_ACQUIRE_IF_ENABLED()/PM_RUNTIME_ACQUIRE_ERR() guards to
replace pm_runtime_resume_and_get() and the manual error handling for both
the eDMA engine and per-channel devices.
- move dmaenginem_async_device_register() after runtime PM is enabled.
- move fsl_edma->drvdata->setup_irq() to the end of probe() and use
dev_err_probe() on failure.
- check "irq > 0" instead of "irq" before synchronize_irq() in
fsl_edma_chan_runtime_suspend().
- add fsl_chan txirq and errirq synchronize_irq() check in fsl_edma_chan_runtime_suspend().
- add comments explaining the pm_runtime_get_if_active() checks in the interrupt handlers.
- dropped patch #5 since this is a pre-existing issue that will be fixed in a separate patch.
- Link to v6: https://lore.kernel.org/r/20260701-b4-edma-runtime-opt-v6-0-354ff4229c00@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 (4):
dmaengine: fsl-edma: use devm_clk_get_optional_enabled() 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
drivers/dma/fsl-edma-common.c | 14 +-
drivers/dma/fsl-edma-common.h | 4 +-
drivers/dma/fsl-edma-main.c | 330 ++++++++++++++++++++++++++++++++----------
3 files changed, 261 insertions(+), 87 deletions(-)
---
base-commit: 3d229ce89862b8e82d71a9e11d6f840ca272a6d6
change-id: 20260811-b4-edma-runtime-opt-mo-0d19aeeb5dbe
Best regards,
--
Joy Zou <joy.zou@oss.nxp.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v7 1/4] dmaengine: fsl-edma: use devm_clk_get_optional_enabled() for channel clock
2026-10-23 10:59 [PATCH v7 0/4] add runtime suspend/resume support joy.zou
@ 2026-10-23 10:59 ` joy.zou
2026-10-23 10:59 ` [PATCH v7 2/4] dmaengine: fsl-edma: use devm_clk_get_optional() for DMA engine clock joy.zou
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: joy.zou @ 2026-10-23 10:59 UTC (permalink / raw)
To: Frank Li, Vinod Koul, Joe Hattori
Cc: Joy Zou, Frank Li, imx, dmaengine, linux-kernel, 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_enabled(),
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 v7:
- Replace devm_clk_get_optional() with devm_clk_get_optional_enable().
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 | 19 ++++++-------------
3 files changed, 7 insertions(+), 17 deletions(-)
diff --git a/drivers/dma/fsl-edma-common.c b/drivers/dma/fsl-edma-common.c
index 513b0f4e5be2..4f7a942673eb 100644
--- a/drivers/dma/fsl-edma-common.c
+++ b/drivers/dma/fsl-edma-common.c
@@ -1087,9 +1087,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 cbb077e98f55..4332b2b2e1df 100644
--- a/drivers/dma/fsl-edma-common.h
+++ b/drivers/dma/fsl-edma-common.h
@@ -213,7 +213,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 630285c85482..a85bebc25217 100644
--- a/drivers/dma/fsl-edma-main.c
+++ b/drivers/dma/fsl-edma-main.c
@@ -572,8 +572,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),
@@ -813,22 +812,16 @@ 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_enabled(&pdev->dev, (const char *)clk_name);
+ if (IS_ERR(fsl_chan->clk))
+ return PTR_ERR(fsl_chan->clk);
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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v7 2/4] dmaengine: fsl-edma: use devm_clk_get_optional() for DMA engine clock
2026-10-23 10:59 [PATCH v7 0/4] add runtime suspend/resume support joy.zou
2026-10-23 10:59 ` [PATCH v7 1/4] dmaengine: fsl-edma: use devm_clk_get_optional_enabled() for channel clock joy.zou
@ 2026-10-23 10:59 ` joy.zou
2026-10-23 10:59 ` [PATCH v7 3/4] dmaengine: fsl-edma: convert DMAMUX clock handling to bulk clock API joy.zou
2026-10-23 10:59 ` [PATCH v7 4/4] dmaengine: fsl-edma: add runtime suspend/resume support Joy Zou
3 siblings, 0 replies; 6+ messages in thread
From: joy.zou @ 2026-10-23 10:59 UTC (permalink / raw)
To: Frank Li, Vinod Koul, Joe Hattori
Cc: Joy Zou, Frank Li, imx, dmaengine, linux-kernel, 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_enabled(),
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 v7:
- Replace devm_clk_get_optional() with devm_clk_get_optional_enable().
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 | 22 ++++++++++------------
2 files changed, 10 insertions(+), 13 deletions(-)
diff --git a/drivers/dma/fsl-edma-common.h b/drivers/dma/fsl-edma-common.h
index 4332b2b2e1df..97b3a9872e90 100644
--- a/drivers/dma/fsl-edma-common.h
+++ b/drivers/dma/fsl-edma-common.h
@@ -207,7 +207,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 a85bebc25217..bc94554bc36c 100644
--- a/drivers/dma/fsl-edma-main.c
+++ b/drivers/dma/fsl-edma-main.c
@@ -559,7 +559,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,
};
@@ -572,7 +572,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),
@@ -581,14 +581,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,
@@ -598,7 +598,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,
@@ -738,13 +738,11 @@ 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_enabled(&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 = of_property_read_variable_u32_array(np, "dma-channel-mask", chan_mask, 1, 2);
--
2.34.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v7 3/4] dmaengine: fsl-edma: convert DMAMUX clock handling to bulk clock API
2026-10-23 10:59 [PATCH v7 0/4] add runtime suspend/resume support joy.zou
2026-10-23 10:59 ` [PATCH v7 1/4] dmaengine: fsl-edma: use devm_clk_get_optional_enabled() for channel clock joy.zou
2026-10-23 10:59 ` [PATCH v7 2/4] dmaengine: fsl-edma: use devm_clk_get_optional() for DMA engine clock joy.zou
@ 2026-10-23 10:59 ` joy.zou
2026-10-23 10:59 ` [PATCH v7 4/4] dmaengine: fsl-edma: add runtime suspend/resume support Joy Zou
3 siblings, 0 replies; 6+ messages in thread
From: joy.zou @ 2026-10-23 10:59 UTC (permalink / raw)
To: Frank Li, Vinod Koul, Joe Hattori
Cc: Joy Zou, Frank Li, imx, dmaengine, linux-kernel, 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 v7:
- Replace devm_clk_bulk_get_optional() with devm_clk_bulk_get_optional_enable().
Changes for v6:
- Replace devm_clk_bulk_get_optional_enable() with devm_clk_bulk_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 | 41 +++++++++++++++++++----------------------
2 files changed, 20 insertions(+), 23 deletions(-)
diff --git a/drivers/dma/fsl-edma-common.h b/drivers/dma/fsl-edma-common.h
index 97b3a9872e90..12db403204c5 100644
--- a/drivers/dma/fsl-edma-common.h
+++ b/drivers/dma/fsl-edma-common.h
@@ -260,7 +260,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 bc94554bc36c..be9bd9cc6630 100644
--- a/drivers/dma/fsl-edma-main.c
+++ b/drivers/dma/fsl-edma-main.c
@@ -531,14 +531,6 @@ static void fsl_edma_irq_exit(
}
}
-static void fsl_disable_clocks(struct fsl_edma_engine *fsl_edma, int nr_clocks)
-{
- int i;
-
- for (i = 0; i < nr_clocks; i++)
- clk_disable_unprepare(fsl_edma->muxclk[i]);
-}
-
static struct fsl_edma_drvdata vf610_data = {
.dmamuxs = DMAMUX_NR,
.flags = FSL_EDMA_DRV_WRAP_IO,
@@ -752,23 +744,28 @@ 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];
+ 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->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]);
+ 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_enable(&pdev->dev, fsl_edma->drvdata->dmamuxs,
+ fsl_edma->muxclk);
+ if (ret)
+ return dev_err_probe(&pdev->dev, ret,
+ "Failed to get/enable DMAMUX block clock.\n");
}
fsl_edma->big_endian = of_property_read_bool(np, "big-endian");
--
2.34.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v7 4/4] dmaengine: fsl-edma: add runtime suspend/resume support
2026-10-23 10:59 [PATCH v7 0/4] add runtime suspend/resume support joy.zou
` (2 preceding siblings ...)
2026-10-23 10:59 ` [PATCH v7 3/4] dmaengine: fsl-edma: convert DMAMUX clock handling to bulk clock API joy.zou
@ 2026-10-23 10:59 ` Joy Zou
2026-10-23 11:37 ` sashiko-bot
3 siblings, 1 reply; 6+ messages in thread
From: Joy Zou @ 2026-10-23 10:59 UTC (permalink / raw)
To: Frank Li, Vinod Koul, Joe Hattori
Cc: Joy Zou, Frank Li, imx, dmaengine, linux-kernel, Joy Zou
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 V7:
- replace devm_clk_get_optional_enable() with devm_clk_get_optional().
- replace devm_clk_bulk_get_optional_enable() with devm_clk_bulk_get_optional().
- use PM_RUNTIME_ACQUIRE_IF_ENABLED()/PM_RUNTIME_ACQUIRE_ERR() guards to
replace pm_runtime_resume_and_get() and the manual error handling for both
the eDMA engine and per-channel devices.
- move dmaenginem_async_device_register() after runtime PM is enabled.
- move fsl_edma->drvdata->setup_irq() to the end of probe() and use
dev_err_probe() on failure.
- check "irq > 0" instead of "irq" before synchronize_irq() in
fsl_edma_chan_runtime_suspend().
- add fsl_chan txirq and errirq synchronize_irq() check in fsl_edma_chan_runtime_suspend().
- add comments explaining the pm_runtime_get_if_active() checks in the interrupt handlers.
- Link to v6: https://patch.msgid.link/20260701-b4-edma-runtime-opt-v6-0-354ff4229c00@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.
- 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/
Signed-off-by: Joy Zou <joy.zou@oss.nxp.com>
---
drivers/dma/fsl-edma-common.c | 14 ++-
drivers/dma/fsl-edma-main.c | 264 ++++++++++++++++++++++++++++++++++++------
2 files changed, 234 insertions(+), 44 deletions(-)
diff --git a/drivers/dma/fsl-edma-common.c b/drivers/dma/fsl-edma-common.c
index 4f7a942673eb..4ab56294ac77 100644
--- a/drivers/dma/fsl-edma-common.c
+++ b/drivers/dma/fsl-edma-common.c
@@ -274,9 +274,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;
}
@@ -1087,7 +1084,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),
@@ -1114,7 +1116,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;
}
@@ -1146,7 +1148,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 be9bd9cc6630..c16280b83656 100644
--- a/drivers/dma/fsl-edma-main.c
+++ b/drivers/dma/fsl-edma-main.c
@@ -37,15 +37,32 @@ static irqreturn_t fsl_edma_tx_handler(int irq, void *dev_id)
unsigned int intr, ch;
struct edma_regs *regs = &fsl_edma->regs;
+ /*
+ * A racy or spurious interrupt for an already-suspended
+ * controller or channel is an extremely rare corner case and
+ * highly unlikely in practice.
+ */
+ 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 +149,23 @@ 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)
+ /*
+ * A racy or spurious interrupt for an already-suspended
+ * channel is an extremely rare corner case and highly unlikely in
+ * practice.
+ */
+ 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 +212,37 @@ static irqreturn_t fsl_edma3_or_err_handler(int irq, void *dev_id)
unsigned int err, ch, ch_es;
struct fsl_edma_chan *chan;
+ /*
+ * A racy or spurious interrupt for an already-suspended
+ * controller or channel is an extremely rare corner case and
+ * highly unlikely in practice.
+ */
+ 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 +254,33 @@ static irqreturn_t fsl_edma_err_handler(int irq, void *dev_id)
unsigned int err, ch;
struct edma_regs *regs = &fsl_edma->regs;
+ /*
+ * A racy or spurious interrupt for an already-suspended
+ * controller or channel is an extremely rare corner case and
+ * highly unlikely in practice.
+ */
+ 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;
}
@@ -630,11 +691,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);
}
}
@@ -665,23 +727,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:
@@ -689,6 +736,42 @@ 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 > 0)
+ synchronize_irq(fsl_edma->txirq);
+
+ if (fsl_edma->errirq > 0)
+ synchronize_irq(fsl_edma->errirq);
+
+ if (fsl_chan->txirq > 0)
+ synchronize_irq(fsl_chan->txirq);
+
+ if (fsl_chan->errirq > 0)
+ synchronize_irq(fsl_chan->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;
@@ -730,11 +813,11 @@ static int fsl_edma_probe(struct platform_device *pdev)
regs = &fsl_edma->regs;
}
- fsl_edma->dmaclk = devm_clk_get_optional_enabled(&pdev->dev, "dma");
+ 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");
+ "Failed to get DMA clock.\n");
ret = of_property_read_variable_u32_array(np, "dma-channel-mask", chan_mask, 1, 2);
@@ -761,11 +844,11 @@ static int fsl_edma_probe(struct platform_device *pdev)
return -ENOMEM;
}
- ret = devm_clk_bulk_get_optional_enable(&pdev->dev, fsl_edma->drvdata->dmamuxs,
- fsl_edma->muxclk);
+ 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/enable DMAMUX block clock.\n");
+ "Failed to get DMAMUX block clock.\n");
}
fsl_edma->big_endian = of_property_read_bool(np, "big-endian");
@@ -808,21 +891,13 @@ static int fsl_edma_probe(struct platform_device *pdev)
+ i * drvdata->chreg_space_sz + drvdata->chreg_off + len;
fsl_chan->mux_addr = fsl_edma->membase + drvdata->mux_off + i * drvdata->mux_skip;
snprintf(clk_name, sizeof(clk_name), "ch%02d", i);
- fsl_chan->clk = devm_clk_get_optional_enabled(&pdev->dev, (const char *)clk_name);
+ 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);
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);
@@ -870,11 +945,69 @@ static int fsl_edma_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, fsl_edma);
+ ret = devm_pm_runtime_enable(&pdev->dev);
+ if (ret)
+ return dev_err_probe(&pdev->dev, ret, "Can't enable eDMA engine PM runtime!\n");
+
+ PM_RUNTIME_ACQUIRE_IF_ENABLED(&pdev->dev, pm);
+ ret = PM_RUNTIME_ACQUIRE_ERR(&pm);
+ if (ret)
+ return dev_err_probe(&pdev->dev, ret, "Failed to resume eDMA engine!\n");
+
ret = dmaenginem_async_device_register(&fsl_edma->dma_dev);
if (ret)
return dev_err_probe(&pdev->dev, ret,
"Can't register Freescale eDMA engine.\n");
+ 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);
+ }
+
+ PM_RUNTIME_ACQUIRE_IF_ENABLED(chan_dev, pm);
+ ret = PM_RUNTIME_ACQUIRE_ERR(&pm);
+ if (ret) {
+ if (fsl_chan->pd_dev)
+ pm_runtime_put_sync_suspend(fsl_chan->pd_dev);
+
+ 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);
+ 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);
@@ -886,6 +1019,10 @@ 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);
+ ret = fsl_edma->drvdata->setup_irq(pdev, fsl_edma);
+ if (ret)
+ return dev_err_probe(&pdev->dev, ret, "Failed to setup IRQ!\n");
+
return 0;
}
@@ -908,6 +1045,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)) {
@@ -927,19 +1077,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);
@@ -954,6 +1141,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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v7 4/4] dmaengine: fsl-edma: add runtime suspend/resume support
2026-10-23 10:59 ` [PATCH v7 4/4] dmaengine: fsl-edma: add runtime suspend/resume support Joy Zou
@ 2026-10-23 11:37 ` sashiko-bot
0 siblings, 0 replies; 6+ messages in thread
From: sashiko-bot @ 2026-10-23 11:37 UTC (permalink / raw)
To: Joy Zou; +Cc: imx, dmaengine, vkoul, Frank.Li
Thank you for your contribution! Sashiko AI review found 12 potential issue(s) to consider:
- [Critical] dmaengine: fsl-edma: unclocked register read in fsl_edma3_err_check()
- [Critical] dmaengine: fsl-edma: NULL dereference on masked channels in IRQ handlers
- [High] dmaengine: fsl-edma: missing CR restore in fsl_edma_resume_early()
- [High] dmaengine: fsl-edma: clocks disabled and IRQs broken when CONFIG_PM=n
- [High] dmaengine: fsl-edma: UAF on pd_dev_link in fsl_edma3_detach_pd()
- [High] dmaengine: fsl-edma: DMA registration before setup_irq() and channel PM
- [High] dmaengine: fsl-edma: broken channel allocation on mcf-edma with CONFIG_PM=y
- [High] dmaengine: fsl-edma: use-after-free on pd_dev in fsl_edma3_detach_pd()
- [Medium] dmaengine: fsl-edma: async runtime PM put in fsl_edma_probe() leaks clocks
- [Medium] dmaengine: fsl-edma: missing txirq_16_31 sync in channel runtime suspend
- [Medium] dmaengine: fsl-edma: active channels with srcid 0 skipped in suspend/resume
- [Medium] dmaengine: fsl-edma: storm in tx/err IRQ handlers when channel is suspending
--
commit 9e2928aa0a1257007f05ec7ae65bb12d57332723
Author: Joy Zou <joy.zou@oss.nxp.com>
dmaengine: fsl-edma: add runtime suspend/resume support
This commit adds runtime PM support for the FSL eDMA engine and its
per-channel devices, managing engine and channel clocks as well as
per-channel power domains through runtime suspend and resume callbacks.
> diff --git a/drivers/dma/fsl-edma-common.c b/drivers/dma/fsl-edma-common.c
> index 4f7a942673ebc..4ab56294ac776 100644
> --- a/drivers/dma/fsl-edma-common.c
> +++ b/drivers/dma/fsl-edma-common.c
[ ... ]
> @@ -1087,7 +1084,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;
> + }
[Severity: High]
Does this break channel allocation for the ColdFire mcf-edma driver when
CONFIG_PM is enabled?
drivers/dma/fsl-edma-common.c is shared with drivers/dma/mcf-edma-main.c,
where mcf_edma_probe() wires up fsl_edma_alloc_chan_resources() and
fsl_edma_free_chan_resources():
drivers/dma/mcf-edma-main.c:mcf_edma_probe() {
...
mcf_edma->dma_dev.device_alloc_chan_resources =
fsl_edma_alloc_chan_resources;
mcf_edma->dma_dev.device_free_chan_resources =
fsl_edma_free_chan_resources;
...
}
Unlike fsl_edma_probe(), mcf_edma_probe() never calls
devm_pm_runtime_enable() on the per-channel devices, so each channel device
remains with disable_depth == 1 and runtime_status == RPM_SUSPENDED. When a
client requests a channel on mcf-edma with CONFIG_PM=y,
pm_runtime_resume_and_get(&fsl_chan->vchan.chan.dev->device) in
fsl_edma_alloc_chan_resources() will fail with -EACCES.
> diff --git a/drivers/dma/fsl-edma-main.c b/drivers/dma/fsl-edma-main.c
> index be9bd9cc66306..c16280b836560 100644
> --- a/drivers/dma/fsl-edma-main.c
> +++ b/drivers/dma/fsl-edma-main.c
> @@ -37,15 +37,32 @@ static irqreturn_t fsl_edma_tx_handler(int irq, void *dev_id)
> unsigned int intr, ch;
> struct edma_regs *regs = &fsl_edma->regs;
>
> + /*
> + * A racy or spurious interrupt for an already-suspended
> + * controller or channel is an extremely rare corner case and
> + * highly unlikely in practice.
> + */
> + if (pm_runtime_get_if_active(fsl_edma->dma_dev.dev) <= 0)
> + return IRQ_NONE;
[Severity: High]
What happens here and in fsl_edma_probe() when the kernel is built with
CONFIG_PM=n?
When CONFIG_PM=n, the stub for pm_runtime_get_if_active() in
include/linux/pm_runtime.h unconditionally returns -EINVAL:
static inline int pm_runtime_get_if_active(struct device *dev)
{
return -EINVAL;
}
That causes fsl_edma_tx_handler(), fsl_edma3_tx_handler(),
fsl_edma3_or_err_handler(), and fsl_edma_err_handler() to always return
IRQ_NONE.
In addition, fsl_edma_probe() replaces devm_clk_get_optional_enabled() and
devm_clk_bulk_get_optional_enable() with devm_clk_get_optional() and
devm_clk_bulk_get_optional(), and fsl_edma_alloc_chan_resources() replaces
clk_prepare_enable(fsl_chan->clk) with pm_runtime_resume_and_get(). Because
PM_RUNTIME_ACQUIRE_IF_ENABLED() and pm_runtime_resume_and_get() are no-ops
when CONFIG_PM=n, fsl_edma_runtime_resume() and
fsl_edma_chan_runtime_resume() never run, leaving dmaclk, muxclk, and
fsl_chan->clk disabled when accessing registers.
> +
> 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;
[Severity: Critical]
Can this dereference a NULL pointer when dma-channel-mask is used?
In fsl_edma_probe(), channels with bits set in fsl_edma->chan_masked are
skipped before vchan_init(&fsl_chan->vchan, &fsl_edma->dma_dev):
drivers/dma/fsl-edma-main.c:fsl_edma_probe() {
...
for (i = 0; i < fsl_edma->n_chans; i++) {
struct fsl_edma_chan *fsl_chan = &fsl_edma->chans[i];
int len;
if (fsl_edma->chan_masked & BIT(i))
continue;
...
vchan_init(&fsl_chan->vchan, &fsl_edma->dma_dev);
}
}
Because masked channels are not added to fsl_edma->dma_dev.channels,
dmaenginem_async_device_register() never allocates fsl_chan->vchan.chan.dev,
leaving it NULL.
Here in fsl_edma_tx_handler(), as well as in fsl_edma3_or_err_handler(),
fsl_edma_err_handler(), and fsl_edma3_or_tx_handler() ->
fsl_edma3_tx_handler(), the loop does not check fsl_edma->chan_masked &
BIT(ch) (or check intr & (0x1 << ch) / err & (0x1 << ch) first) before
passing &fsl_chan->vchan.chan.dev->device to pm_runtime_get_if_active().
> +
> 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: Medium]
What happens in fsl_edma_tx_handler() and fsl_edma_err_handler() if an
interrupt is pending when a channel is being runtime-suspended?
When fsl_edma_free_chan_resources() calls fsl_edma_disable_request() and
then pm_runtime_put_sync_suspend(&fsl_chan->vchan.chan.dev->device),
fsl_edma_disable_request() disables future hardware requests in regs->cerq
and regs->ceei, but does not clear any pending interrupt bit in regs->intl
or regs->errl.
Inside rpm_suspend(), the runtime PM core sets
chan_dev->power.runtime_status = RPM_SUSPENDING before invoking
fsl_edma_chan_runtime_suspend(), which calls synchronize_irq(fsl_edma->txirq)
and synchronize_irq(fsl_edma->errirq). When fsl_edma_tx_handler() or
fsl_edma_err_handler() runs for that pending interrupt,
pm_runtime_get_if_active(&fsl_chan->vchan.chan.dev->device) returns 0
because the status is already RPM_SUSPENDING. The loop then skips writing
EDMA_CINT_CINT(ch) to regs->cint (or EDMA_CERR_CERR(ch) to regs->cerr) while
the handler still returns IRQ_HANDLED, leaving the level-triggered interrupt
asserted in an infinite storm.
Also, both handlers call pm_runtime_put(fsl_edma->dma_dev.dev) before the
loop even though regs->cint, regs->cerr, regs->cerq, and regs->ceei accessed
inside the loop are global controller registers clocked by dmaclk.
> @@ -132,13 +149,23 @@ 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)
> + /*
> + * A racy or spurious interrupt for an already-suspended
> + * channel is an extremely rare corner case and highly unlikely in
> + * practice.
> + */
> + if (pm_runtime_get_if_active(&fsl_chan->vchan.chan.dev->device) <= 0)
> return IRQ_NONE;
[Severity: Critical]
Were fsl_edma3_err_check(), fsl_edma3_err_handler_per_chan(), and
fsl_edma3_err_handler_shared() missed when adding runtime PM checks to the
interrupt handlers?
Just above fsl_edma3_tx_handler(), fsl_edma3_err_handler_shared() loops over
all unmasked channels and calls fsl_edma3_err_check(), and
fsl_edma3_err_handler_per_chan() (registered with IRQF_SHARED) also calls
fsl_edma3_err_check():
drivers/dma/fsl-edma-main.c:fsl_edma3_err_check() {
...
scoped_guard(spinlock, &fsl_chan->vchan.lock) {
ch_err = edma_readl_chreg(fsl_chan, ch_es);
if (!(ch_err & EDMA_V3_CH_ERR))
return;
...
}
drivers/dma/fsl-edma-main.c:fsl_edma3_err_handler_shared() {
...
for (ch = 0; ch < fsl_edma->n_chans; ch++) {
if (fsl_edma->chan_masked & BIT(ch))
continue;
fsl_edma3_err_check(&fsl_edma->chans[ch]);
}
...
}
When idle channels are runtime-suspended with their channel clocks disabled
(or their per-channel power domains powered off on platforms with
FSL_EDMA_DRV_HAS_PD), an error interrupt on any active channel causes
fsl_edma3_err_handler_shared() -> fsl_edma3_err_check() to read ch_es on the
suspended channels without calling pm_runtime_get_if_active(), triggering a
bus hang or synchronous external abort.
[ ... ]
> @@ -630,11 +691,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;
> + }
[Severity: High]
Can device_link_del(fsl_chan->pd_dev_link) cause a use-after-free during
devres teardown?
In fsl_edma_probe(), devm_add_action_or_reset(&pdev->dev,
devm_fsl_edma3_detach_pd, fsl_edma) is registered before
dmaenginem_async_device_register(&fsl_edma->dma_dev), and
fsl_chan->pd_dev_link is now created with chan_dev
(&fsl_chan->vchan.chan.dev->device) as the consumer.
When probe fails after creating pd_dev_link or when the driver is unbound,
devres unwinds in reverse order:
1. dmaenginem_async_device_unregister() runs first and calls
device_unregister(chan_dev) -> device_del(chan_dev) ->
device_links_purge(chan_dev), which deletes and frees
fsl_chan->pd_dev_link.
2. devm_fsl_edma3_detach_pd() -> fsl_edma3_detach_pd() runs next and calls
device_link_del(fsl_chan->pd_dev_link) on the already-freed device_link.
> 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);
> }
> }
[Severity: High]
Does pm_runtime_set_suspended(fsl_chan->pd_dev) dereference fsl_chan->pd_dev
after it has already been freed by dev_pm_domain_detach()?
Because device_link_add() was moved out of fsl_edma3_attach_pd() into a
later loop in fsl_edma_probe(), if fsl_edma3_attach_pd() fails midway or if
fsl_edma_probe() fails before device_link_add() runs for a channel,
fsl_chan->pd_dev holds only the single initial reference from
dev_pm_domain_attach_by_id(). Calling dev_pm_domain_detach(fsl_chan->pd_dev,
false) invokes genpd_dev_pm_detach() -> device_unregister(fsl_chan->pd_dev),
which drops the last reference and frees fsl_chan->pd_dev before
pm_runtime_set_suspended(fsl_chan->pd_dev) accesses it.
[ ... ]
> @@ -689,6 +736,42 @@ 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 > 0)
> + synchronize_irq(fsl_edma->txirq);
> +
> + if (fsl_edma->errirq > 0)
> + synchronize_irq(fsl_edma->errirq);
> +
> + if (fsl_chan->txirq > 0)
> + synchronize_irq(fsl_chan->txirq);
> +
> + if (fsl_chan->errirq > 0)
> + synchronize_irq(fsl_chan->errirq);
> +
> + clk_disable_unprepare(fsl_chan->clk);
> +
> + return 0;
> +}
[Severity: Medium]
Should fsl_edma_chan_runtime_suspend() also synchronize
fsl_edma->txirq_16_31 (and should fsl_edma_probe() initialize
fsl_edma->txirq_16_31 = -EINVAL)?
On platforms using fsl_edma3_or_irq_init() (such as nxp,s32g2-edma),
channels 16..31 are serviced by fsl_edma->txirq_16_31 via
fsl_edma3_tx_16_31_handler(), which is not synchronized here before
disabling fsl_chan->clk.
[ ... ]
> @@ -870,11 +945,69 @@ static int fsl_edma_probe(struct platform_device *pdev)
>
> platform_set_drvdata(pdev, fsl_edma);
>
> + ret = devm_pm_runtime_enable(&pdev->dev);
> + if (ret)
> + return dev_err_probe(&pdev->dev, ret, "Can't enable eDMA engine PM runtime!\n");
> +
> + PM_RUNTIME_ACQUIRE_IF_ENABLED(&pdev->dev, pm);
> + ret = PM_RUNTIME_ACQUIRE_ERR(&pm);
> + if (ret)
> + return dev_err_probe(&pdev->dev, ret, "Failed to resume eDMA engine!\n");
> +
> ret = dmaenginem_async_device_register(&fsl_edma->dma_dev);
> if (ret)
> return dev_err_probe(&pdev->dev, ret,
> "Can't register Freescale eDMA engine.\n");
>
> + 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);
> + }
> +
> + PM_RUNTIME_ACQUIRE_IF_ENABLED(chan_dev, pm);
> + ret = PM_RUNTIME_ACQUIRE_ERR(&pm);
> + if (ret) {
> + if (fsl_chan->pd_dev)
> + pm_runtime_put_sync_suspend(fsl_chan->pd_dev);
> +
> + 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);
> + if (fsl_chan->pd_dev)
> + pm_runtime_put_sync_suspend(fsl_chan->pd_dev);
> + }
[Severity: Medium]
Does using PM_RUNTIME_ACQUIRE_IF_ENABLED() here leak prepared/enabled clocks
when probe fails, and prevent fsl_chan->pd_dev from suspending?
PM_RUNTIME_ACQUIRE_IF_ENABLED() releases its reference via asynchronous
pm_runtime_put() when exiting the scope. If fsl_edma_probe() returns an
error after resuming &pdev->dev or chan_dev, the guard queues an async idle
request on pm_wq, and devres teardown immediately runs pm_runtime_disable()
(from devm_pm_runtime_enable()), which calls __pm_runtime_barrier() and
cancels the pending work before fsl_edma_chan_runtime_suspend() or
fsl_edma_runtime_suspend() can disable fsl_chan->clk, fsl_edma->dmaclk, and
fsl_edma->muxclk.
In addition, inside the channel loop, PM_RUNTIME_ACQUIRE_IF_ENABLED(chan_dev,
pm) (which also shadows the outer pm variable) is still in scope and holds a
runtime PM reference on the supplier fsl_chan->pd_dev via
fsl_chan->pd_dev_link when pm_runtime_put_sync_suspend(fsl_chan->pd_dev) is
called at the end of the loop body, so pm_runtime_put_sync_suspend() on
pd_dev will fail with -EAGAIN.
> +
> ret = devm_of_dma_controller_register(&pdev->dev, np,
> drvdata->dmamuxs ? fsl_edma_xlate : fsl_edma3_xlate,
> fsl_edma);
> @@ -886,6 +1019,10 @@ 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);
>
> + ret = fsl_edma->drvdata->setup_irq(pdev, fsl_edma);
> + if (ret)
> + return dev_err_probe(&pdev->dev, ret, "Failed to setup IRQ!\n");
> +
> return 0;
> }
[Severity: High]
Is it safe to call dmaenginem_async_device_register() before enabling
per-channel runtime PM, and to call setup_irq() after both
dmaenginem_async_device_register() and devm_of_dma_controller_register()?
Once dmaenginem_async_device_register() and
devm_of_dma_controller_register() return, the DMA channels are published and
can be requested by consumers on another CPU before the channel loop has
configured chan_dev runtime PM (causing pm_runtime_resume_and_get() in
fsl_edma_alloc_chan_resources() to fail with -EACCES) or before setup_irq()
has populated fsl_chan->txirq, fsl_chan->errirq, fsl_chan->irq_handler, and
fsl_chan->errirq_handler (causing fsl_edma_alloc_chan_resources() to skip
request_irq() while fsl_edma_free_chan_resources() later calls free_irq()).
> @@ -908,6 +1045,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;
[Severity: Medium]
Can checking !fsl_chan->srcid here and in fsl_edma_resume_early() skip
active, allocated channels?
There are two cases where an allocated, runtime-active channel on a
FSL_EDMA_DRV_SPLIT_REG controller without FSL_EDMA_DRV_HAS_PD has
fsl_chan->srcid == 0:
1. A channel used for DMA_MEMCPY via fsl_edma_prep_memcpy(), which sets
fsl_chan->is_sw = true and leaves fsl_chan->srcid as 0.
2. Channel 0 on a controller without FSL_EDMA_DRV_HAS_CHMUX (such as
imx93_data3), where fsl_edma3_xlate() sets
fsl_chan->srcid = dma_spec->args[0] (the channel index 0).
Since unassigned channels are already runtime-suspended and filtered out by
pm_runtime_status_suspended(&fsl_chan->vchan.chan.dev->device), is the
extra !fsl_chan->srcid check needed?
> @@ -927,19 +1077,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);
>
[Severity: High]
If a DMA channel is allocated across system suspend and resume on a
non-FSL_EDMA_DRV_SPLIT_REG platform (such as vf610, ls1028a, or imx7ulp),
will regs->cr still be restored on resume?
Moving the EDMA_CR_ERGA | EDMA_CR_ERCA write out of fsl_edma_resume_early()
into fsl_edma_runtime_resume() means it only runs on a runtime PM resume
transition. When any channel is allocated during system suspend, the
channel holds a runtime PM reference that keeps the parent controller
RPM_ACTIVE across suspend/resume, so fsl_edma_resume_early() runs on system
resume while fsl_edma_runtime_resume() does not.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261023-b4-edma-runtime-opt-mo-v7-0-418e53c5f9b7@oss.nxp.com?part=4
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-10-23 11:37 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-23 10:59 [PATCH v7 0/4] add runtime suspend/resume support joy.zou
2026-10-23 10:59 ` [PATCH v7 1/4] dmaengine: fsl-edma: use devm_clk_get_optional_enabled() for channel clock joy.zou
2026-10-23 10:59 ` [PATCH v7 2/4] dmaengine: fsl-edma: use devm_clk_get_optional() for DMA engine clock joy.zou
2026-10-23 10:59 ` [PATCH v7 3/4] dmaengine: fsl-edma: convert DMAMUX clock handling to bulk clock API joy.zou
2026-10-23 10:59 ` [PATCH v7 4/4] dmaengine: fsl-edma: add runtime suspend/resume support Joy Zou
2026-10-23 11:37 ` sashiko-bot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox