* [PATCH v5 1/4] dmaengine: fsl-edma: use devm_clk_get_optional_enabled() for channel clock
2026-05-13 11:23 [PATCH v5 0/4] add runtime suspend/resume support Joy Zou
@ 2026-05-13 11:23 ` Joy Zou
2026-05-13 14:51 ` Frank Li
2026-05-13 11:23 ` [PATCH v5 2/4] dmaengine: fsl-edma: use devm_clk_get_optional_enabled() for DMA engine clock Joy Zou
` (2 subsequent siblings)
3 siblings, 1 reply; 11+ messages in thread
From: Joy Zou @ 2026-05-13 11:23 UTC (permalink / raw)
To: Frank Li, Vinod Koul; +Cc: Frank Li, imx, dmaengine, linux-kernel, Joy Zou
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.
Signed-off-by: Joy Zou <joy.zou@nxp.com>
---
drivers/dma/fsl-edma-common.c | 4 +---
drivers/dma/fsl-edma-common.h | 1 -
drivers/dma/fsl-edma-main.c | 18 ++++++------------
3 files changed, 7 insertions(+), 16 deletions(-)
diff --git a/drivers/dma/fsl-edma-common.c b/drivers/dma/fsl-edma-common.c
index bb7531c456dfa0a8812883a2cf3e9e2e23b0f55e..e1ca25ff228dbe392bb800f6ecac5a85ca326bf1 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 205a96489094805aa728b72a51ae101cd88fa003..f4354b586746d64faf375cc9ce04e15a7b6d86ab 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 36155ab1602a9b264df73dbde3ec2b3aa6cc27c0..87f575d6ccafff455d47f8c794a503abf97e2af1 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,17 @@ 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;
+ snprintf(clk_name, sizeof(clk_name), "ch%02d", i);
+ fsl_chan->clk = devm_clk_get_optional_enabled(&pdev->dev, (const char *)clk_name);
- 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);
- }
+ 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.37.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH v5 1/4] dmaengine: fsl-edma: use devm_clk_get_optional_enabled() for channel clock
2026-05-13 11:23 ` [PATCH v5 1/4] dmaengine: fsl-edma: use devm_clk_get_optional_enabled() for channel clock Joy Zou
@ 2026-05-13 14:51 ` Frank Li
0 siblings, 0 replies; 11+ messages in thread
From: Frank Li @ 2026-05-13 14:51 UTC (permalink / raw)
To: Joy Zou; +Cc: Vinod Koul, Frank Li, imx, dmaengine, linux-kernel
On Wed, May 13, 2026 at 07:23:47PM +0800, Joy Zou wrote:
> 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.
>
> Signed-off-by: Joy Zou <joy.zou@nxp.com>
> ---
Reviewed-by: Frank Li <Frank.Li@nxp.com>
> drivers/dma/fsl-edma-common.c | 4 +---
> drivers/dma/fsl-edma-common.h | 1 -
> drivers/dma/fsl-edma-main.c | 18 ++++++------------
> 3 files changed, 7 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/dma/fsl-edma-common.c b/drivers/dma/fsl-edma-common.c
> index bb7531c456dfa0a8812883a2cf3e9e2e23b0f55e..e1ca25ff228dbe392bb800f6ecac5a85ca326bf1 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 205a96489094805aa728b72a51ae101cd88fa003..f4354b586746d64faf375cc9ce04e15a7b6d86ab 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 36155ab1602a9b264df73dbde3ec2b3aa6cc27c0..87f575d6ccafff455d47f8c794a503abf97e2af1 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,17 @@ 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;
> + snprintf(clk_name, sizeof(clk_name), "ch%02d", i);
> + fsl_chan->clk = devm_clk_get_optional_enabled(&pdev->dev, (const char *)clk_name);
>
> - 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);
> - }
> + 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.37.1
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v5 2/4] dmaengine: fsl-edma: use devm_clk_get_optional_enabled() for DMA engine clock
2026-05-13 11:23 [PATCH v5 0/4] add runtime suspend/resume support Joy Zou
2026-05-13 11:23 ` [PATCH v5 1/4] dmaengine: fsl-edma: use devm_clk_get_optional_enabled() for channel clock Joy Zou
@ 2026-05-13 11:23 ` Joy Zou
2026-05-13 14:53 ` Frank Li
2026-05-14 4:55 ` sashiko-bot
2026-05-13 11:23 ` [PATCH v5 3/4] dmaengine: fsl-edma: convert DMAMUX clock handling to bulk clock API Joy Zou
2026-05-13 11:23 ` [PATCH v5 4/4] dmaengine: fsl-edma: add runtime suspend/resume support Joy Zou
3 siblings, 2 replies; 11+ messages in thread
From: Joy Zou @ 2026-05-13 11:23 UTC (permalink / raw)
To: Frank Li, Vinod Koul; +Cc: Frank Li, imx, dmaengine, linux-kernel, Joy Zou
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.
Signed-off-by: Joy Zou <joy.zou@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 f4354b586746d64faf375cc9ce04e15a7b6d86ab..54128b3f45cb399e1c11d9f86d64adce5c65c102 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 87f575d6ccafff455d47f8c794a503abf97e2af1..ecd14967bfbc07d373a74790e87f9aa36b60e6c9 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,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.37.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH v5 2/4] dmaengine: fsl-edma: use devm_clk_get_optional_enabled() for DMA engine clock
2026-05-13 11:23 ` [PATCH v5 2/4] dmaengine: fsl-edma: use devm_clk_get_optional_enabled() for DMA engine clock Joy Zou
@ 2026-05-13 14:53 ` Frank Li
2026-05-14 4:55 ` sashiko-bot
1 sibling, 0 replies; 11+ messages in thread
From: Frank Li @ 2026-05-13 14:53 UTC (permalink / raw)
To: Joy Zou; +Cc: Vinod Koul, Frank Li, imx, dmaengine, linux-kernel
On Wed, May 13, 2026 at 07:23:48PM +0800, Joy Zou wrote:
> 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.
>
> Signed-off-by: Joy Zou <joy.zou@nxp.com>
> ---
Reviewed-by: Frank Li <Frank.Li@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 f4354b586746d64faf375cc9ce04e15a7b6d86ab..54128b3f45cb399e1c11d9f86d64adce5c65c102 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 87f575d6ccafff455d47f8c794a503abf97e2af1..ecd14967bfbc07d373a74790e87f9aa36b60e6c9 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,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.37.1
>
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH v5 2/4] dmaengine: fsl-edma: use devm_clk_get_optional_enabled() for DMA engine clock
2026-05-13 11:23 ` [PATCH v5 2/4] dmaengine: fsl-edma: use devm_clk_get_optional_enabled() for DMA engine clock Joy Zou
2026-05-13 14:53 ` Frank Li
@ 2026-05-14 4:55 ` sashiko-bot
1 sibling, 0 replies; 11+ messages in thread
From: sashiko-bot @ 2026-05-14 4:55 UTC (permalink / raw)
To: Joy Zou; +Cc: imx, dmaengine, vkoul, Frank.Li
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Removing mandatory clock validation risks kernel crashes on misconfigured device trees.
- [Medium] API Mismatch: `devm_clk_get_optional_enabled()` conflicts with the stated runtime PM intent, leading to unbalanced clock disables.
--
commit 571d7197ad2ed67c4508436798804c62669d46f2
Author: Joy Zou <joy.zou@nxp.com>
dmaengine: fsl-edma: use devm_clk_get_optional_enabled() for DMA engine clock
This commit replaces devm_clk_get_enabled() with
devm_clk_get_optional_enabled() for the eDMA engine clock and removes the
FSL_EDMA_DRV_HAS_DMACLK flag. The stated goal is to simplify clock handling
and prepare for adding channel runtime PM support.
> Prepare to add channel runtime pm support.
If the intention is to manage this clock via runtime PM, does using
devm_clk_get_optional_enabled() conflict with that?
The devm_clk_get_optional_enabled() helper registers a devres action that
automatically calls clk_disable_unprepare() upon driver unbind. If the device
is runtime suspended and the clock is already disabled when the driver is
unbound, the devres cleanup will attempt to disable the clock again.
Can this trigger a WARN_ON in the clock core for an unbalanced enable count?
Should the driver use devm_clk_get_optional() and manually handle
clk_prepare_enable() instead?
> diff --git a/drivers/dma/fsl-edma-main.c b/drivers/dma/fsl-edma-main.c
> index 87f575d6ccaff..ecd14967bfbc0 100644
> --- a/drivers/dma/fsl-edma-main.c
> +++ b/drivers/dma/fsl-edma-main.c
[ ... ]
> @@ -733,13 +733,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");
Does removing the driver-level validation for this clock risk kernel crashes
on misconfigured device trees?
On platforms where the eDMA engine clock is physically required (e.g.,
i.MX7ULP, i.MX8ULP, i.MX93), a missing "dma" clock in the device tree
previously caused a graceful probe failure. Now,
devm_clk_get_optional_enabled() will silently return NULL and probing will
continue.
If the driver subsequently writes to unclocked eDMA registers, such as during
edma_write_tcdreg(), can this trigger a synchronous external abort (bus fault)
and panic the kernel?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260513-b4-b4-edma-runtime-opt-v5-0-1e595bfb8423@nxp.com?part=2
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v5 3/4] dmaengine: fsl-edma: convert DMAMUX clock handling to bulk clock API
2026-05-13 11:23 [PATCH v5 0/4] add runtime suspend/resume support Joy Zou
2026-05-13 11:23 ` [PATCH v5 1/4] dmaengine: fsl-edma: use devm_clk_get_optional_enabled() for channel clock Joy Zou
2026-05-13 11:23 ` [PATCH v5 2/4] dmaengine: fsl-edma: use devm_clk_get_optional_enabled() for DMA engine clock Joy Zou
@ 2026-05-13 11:23 ` Joy Zou
2026-05-13 14:54 ` Frank Li
2026-05-14 5:18 ` sashiko-bot
2026-05-13 11:23 ` [PATCH v5 4/4] dmaengine: fsl-edma: add runtime suspend/resume support Joy Zou
3 siblings, 2 replies; 11+ messages in thread
From: Joy Zou @ 2026-05-13 11:23 UTC (permalink / raw)
To: Frank Li, Vinod Koul; +Cc: Frank Li, imx, dmaengine, linux-kernel, Joy Zou
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.
Signed-off-by: Joy Zou <joy.zou@nxp.com>
---
drivers/dma/fsl-edma-common.h | 2 +-
drivers/dma/fsl-edma-main.c | 43 ++++++++++++++++++++-----------------------
2 files changed, 21 insertions(+), 24 deletions(-)
diff --git a/drivers/dma/fsl-edma-common.h b/drivers/dma/fsl-edma-common.h
index 54128b3f45cb399e1c11d9f86d64adce5c65c102..824b7dd2b52618b826154e55fb96a82c27e846ee 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 ecd14967bfbc07d373a74790e87f9aa36b60e6c9..c12126ea6552d51b773bdd61c018570dbd618602 100644
--- a/drivers/dma/fsl-edma-main.c
+++ b/drivers/dma/fsl-edma-main.c
@@ -526,14 +526,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,
@@ -747,23 +739,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];
-
- 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_enable(&pdev->dev, fsl_edma->drvdata->dmamuxs,
+ fsl_edma->muxclk);
+ if (ret)
+ return dev_err_probe(&pdev->dev, ret,
+ "Failed to enable DMAMUX block clock.\n");
}
fsl_edma->big_endian = of_property_read_bool(np, "big-endian");
--
2.37.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH v5 3/4] dmaengine: fsl-edma: convert DMAMUX clock handling to bulk clock API
2026-05-13 11:23 ` [PATCH v5 3/4] dmaengine: fsl-edma: convert DMAMUX clock handling to bulk clock API Joy Zou
@ 2026-05-13 14:54 ` Frank Li
2026-05-14 5:18 ` sashiko-bot
1 sibling, 0 replies; 11+ messages in thread
From: Frank Li @ 2026-05-13 14:54 UTC (permalink / raw)
To: Joy Zou; +Cc: Vinod Koul, Frank Li, imx, dmaengine, linux-kernel
On Wed, May 13, 2026 at 07:23:49PM +0800, Joy Zou wrote:
> 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.
>
> Signed-off-by: Joy Zou <joy.zou@nxp.com>
> ---
Reviewed-by: Frank Li <Frank.Li@nxp.com>
> drivers/dma/fsl-edma-common.h | 2 +-
> drivers/dma/fsl-edma-main.c | 43 ++++++++++++++++++++-----------------------
> 2 files changed, 21 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/dma/fsl-edma-common.h b/drivers/dma/fsl-edma-common.h
> index 54128b3f45cb399e1c11d9f86d64adce5c65c102..824b7dd2b52618b826154e55fb96a82c27e846ee 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 ecd14967bfbc07d373a74790e87f9aa36b60e6c9..c12126ea6552d51b773bdd61c018570dbd618602 100644
> --- a/drivers/dma/fsl-edma-main.c
> +++ b/drivers/dma/fsl-edma-main.c
> @@ -526,14 +526,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,
> @@ -747,23 +739,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];
> -
> - 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_enable(&pdev->dev, fsl_edma->drvdata->dmamuxs,
> + fsl_edma->muxclk);
> + if (ret)
> + return dev_err_probe(&pdev->dev, ret,
> + "Failed to enable DMAMUX block clock.\n");
> }
>
> fsl_edma->big_endian = of_property_read_bool(np, "big-endian");
>
> --
> 2.37.1
>
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH v5 3/4] dmaengine: fsl-edma: convert DMAMUX clock handling to bulk clock API
2026-05-13 11:23 ` [PATCH v5 3/4] dmaengine: fsl-edma: convert DMAMUX clock handling to bulk clock API Joy Zou
2026-05-13 14:54 ` Frank Li
@ 2026-05-14 5:18 ` sashiko-bot
1 sibling, 0 replies; 11+ messages in thread
From: sashiko-bot @ 2026-05-14 5:18 UTC (permalink / raw)
To: Joy Zou; +Cc: Frank.Li, vkoul, dmaengine, imx
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Switching to `devm_clk_bulk_get_optional_enable()` silently drops validation for missing DMAMUX clocks, leading to a system bus hang during probe on misconfigured device trees.
--
commit ac85fd72d931ccb00828f78ca0682e23cee58abe
Author: Joy Zou <joy.zou@nxp.com>
dmaengine: fsl-edma: convert DMAMUX clock handling to bulk clock API
This commit converts the DMAMUX clock management from individual clock
operations to the bulk clock API to simplify the code. This is in
preparation for adding runtime PM support.
> diff --git a/drivers/dma/fsl-edma-main.c b/drivers/dma/fsl-edma-main.c
> index ecd14967bfbc0..c12126ea6552d 100644
> --- a/drivers/dma/fsl-edma-main.c
> +++ b/drivers/dma/fsl-edma-main.c
[ ... ]
> @@ -747,23 +739,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];
> -
> - 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_enable(&pdev->dev, fsl_edma->drvdata->dmamuxs,
> + fsl_edma->muxclk);
Does using devm_clk_bulk_get_optional_enable() silently drop validation for
missing DMAMUX clocks?
The previous code used devm_clk_get_enabled(), which safely aborted the probe
if clocks were missing. Because the optional variant returns 0 when the clocks
are missing in the device tree, the probe will continue.
If the probe continues without the clocks enabled, the driver later calls
fsl_edma_chan_mux() which accesses the unclocked muxbase registers. On NXP
SoCs, accessing unclocked registers can trigger a system bus hang or
asynchronous external abort.
Should this use a mandatory bulk clock API to ensure the clocks are actually
present and enabled?
> + if (ret)
> + return dev_err_probe(&pdev->dev, ret,
> + "Failed to enable DMAMUX block clock.\n");
> }
>
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260513-b4-b4-edma-runtime-opt-v5-0-1e595bfb8423@nxp.com?part=3
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v5 4/4] dmaengine: fsl-edma: add runtime suspend/resume support
2026-05-13 11:23 [PATCH v5 0/4] add runtime suspend/resume support Joy Zou
` (2 preceding siblings ...)
2026-05-13 11:23 ` [PATCH v5 3/4] dmaengine: fsl-edma: convert DMAMUX clock handling to bulk clock API Joy Zou
@ 2026-05-13 11:23 ` Joy Zou
2026-05-14 5:55 ` sashiko-bot
3 siblings, 1 reply; 11+ messages in thread
From: Joy Zou @ 2026-05-13 11:23 UTC (permalink / raw)
To: Frank Li, Vinod Koul; +Cc: 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 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 | 141 ++++++++++++++++++++++++++++++++++++------
2 files changed, 129 insertions(+), 26 deletions(-)
diff --git a/drivers/dma/fsl-edma-common.c b/drivers/dma/fsl-edma-common.c
index e1ca25ff228dbe392bb800f6ecac5a85ca326bf1..132b900ee6071206b9e2c8f67fdf60ceb8dccb8f 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 c12126ea6552d51b773bdd61c018570dbd618602..9446a0c3bc576c23b0d0277604b41e36dfba0e14 100644
--- a/drivers/dma/fsl-edma-main.c
+++ b/drivers/dma/fsl-edma-main.c
@@ -629,7 +629,6 @@ static void fsl_edma3_detach_pd(struct fsl_edma_engine *fsl_edma)
device_link_del(fsl_chan->pd_dev_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);
}
}
@@ -660,23 +659,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:
@@ -684,6 +668,29 @@ 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);
+
+ 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;
@@ -809,10 +816,6 @@ static int fsl_edma_probe(struct platform_device *pdev)
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);
@@ -869,6 +872,51 @@ 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!");
+
+ 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) {
+ fsl_edma3_detach_pd(fsl_edma);
+ 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!");
+
+ 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);
+ }
+
+ edma_write_tcdreg(fsl_chan, cpu_to_le32(0), csr);
+ fsl_edma_chan_mux(fsl_chan, 0, false);
+ clk_disable_unprepare(fsl_chan->clk);
+ 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);
@@ -880,6 +928,9 @@ 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);
+ clk_disable_unprepare(fsl_edma->dmaclk);
+ clk_bulk_disable_unprepare(fsl_edma->drvdata->dmamuxs, fsl_edma->muxclk);
+
return 0;
}
@@ -902,6 +953,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)) {
@@ -928,6 +992,13 @@ static int fsl_edma_resume_early(struct device *dev)
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)
@@ -940,6 +1011,35 @@ static int fsl_edma_resume_early(struct device *dev)
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);
+ 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;
+ }
+ return 0;
+}
+
/*
* eDMA provides the service to others, so it should be suspend late
* and resume early. When eDMA suspend, all of the clients should stop
@@ -948,6 +1048,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.37.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH v5 4/4] dmaengine: fsl-edma: add runtime suspend/resume support
2026-05-13 11:23 ` [PATCH v5 4/4] dmaengine: fsl-edma: add runtime suspend/resume support Joy Zou
@ 2026-05-14 5:55 ` sashiko-bot
0 siblings, 0 replies; 11+ messages in thread
From: sashiko-bot @ 2026-05-14 5:55 UTC (permalink / raw)
To: Joy Zou; +Cc: vkoul, dmaengine, Frank.Li, imx
Thank you for your contribution! Sashiko AI review found 9 potential issue(s) to consider:
- [Critical] Double-free and Use-After-Free in the probe error path if device_link_add() fails.
- [Critical] Use-After-Free during driver unbind due to manual device link deletion after core purging.
- [Critical] Pre-existing Use-After-Free vulnerability in `fsl_edma3_detach_pd`.
- [Critical] Unclocked register access in shared IRQ handlers causing system crashes.
- [High] Initialization race condition exposing uninitialized DMA channels to consumers.
- [High] System hang risk due to enabling channel clocks before their corresponding power domains are resumed.
- [High] Unbalanced clock disable and CCF underflow on driver removal.
- [High] Hardware remains inaccessible, crashing the system if CONFIG_PM is disabled.
- [High] Unclocked register access during system resume (`resume_early`).
--
commit 29a9c658afa0aacc5ffd271f1b4ac1fc006b0536
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 c12126ea6552d..9446a0c3bc576 100644
> --- a/drivers/dma/fsl-edma-main.c
> +++ b/drivers/dma/fsl-edma-main.c
> @@ -629,7 +629,6 @@ static void fsl_edma3_detach_pd(struct fsl_edma_engine *fsl_edma)
> device_link_del(fsl_chan->pd_dev_link);
During driver unbind, the devres cleanups are executed in reverse order.
The DMA engine and its channel devices are unregistered first via
dmaenginem_async_device_unregister(), which triggers device_links_purge()
to delete all links where the channel is a consumer.
When the devres action later calls fsl_edma3_detach_pd() and reaches this
device_link_del(), will it cause a use-after-free by attempting to delete
a link that has already been purged by the driver core?
> 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);
This issue pre-dates the patch, but could this cause a use-after-free?
For power domains attached via dev_pm_domain_attach_by_id(), calling
dev_pm_domain_detach() unregisters and frees the underlying virtual device.
Passing fsl_chan->pd_dev to pm_runtime_set_suspended() immediately after
appears to use a freed pointer.
[ ... ]
> @@ -684,6 +668,29 @@ 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);
> +
> + clk_disable_unprepare(fsl_chan->clk);
> +
> + return 0;
> +}
Does this function need to call synchronize_irq() before disabling the clock?
Since the driver uses shared interrupts (IRQF_SHARED), if another device on
the same IRQ line triggers an interrupt while the channel is suspended,
the eDMA interrupt handler might attempt to read hardware registers while the
clock is disabled.
Should the interrupt handlers (like fsl_edma3_tx_handler) also use
pm_runtime_get_if_active() to ensure the device is powered before checking
status registers?
[ ... ]
> @@ -869,6 +872,51 @@ static int fsl_edma_probe(struct platform_device *pdev)
> return dev_err_probe(&pdev->dev, ret,
> "Can't register Freescale eDMA engine.\n");
Is there a race condition here?
By calling dmaenginem_async_device_register() before setting up the runtime
PM domains and hardware registers below, clients can immediately request a
channel.
If a client allocates a channel concurrently, it could call
fsl_edma_alloc_chan_resources() before devm_pm_runtime_enable() is called,
or race with the unconditional clk_disable_unprepare() and edma_write_tcdreg()
in the loop below, potentially corrupting hardware state.
> + ret = devm_pm_runtime_enable(&pdev->dev);
> + if (ret)
> + return dev_err_probe(&pdev->dev, ret, "Can't enable eDMA engine PM runtime!");
> +
> + 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) {
> + fsl_edma3_detach_pd(fsl_edma);
> + return dev_err_probe(&pdev->dev, -EINVAL,
> + "Failed to add device_link to %d\n", i);
If device_link_add() fails, fsl_edma3_detach_pd() is called manually to detach
domains and delete links, but it doesn't clear the pointers.
Since devm_fsl_edma3_detach_pd() was already registered as a devres action
earlier in probe, returning an error here will cause the devres framework to
call fsl_edma3_detach_pd() a second time. Could this lead to a double-free
of the device link and double-detach of the PM domain?
> + }
> + }
> + ret = devm_pm_runtime_enable(chan_dev);
> + if (ret)
> + return dev_err_probe(&pdev->dev, ret,
> + "Can't enable eDMA channel PM runtime!");
> +
> + if (fsl_chan->pd_dev) {
> + ret = pm_runtime_resume_and_get(fsl_chan->pd_dev);
Since the earlier loop in probe acquires the channel clocks with
devm_clk_get_optional_enabled(), the clocks are already enabled before we
reach this point.
Because the previous DL_FLAG_RPM_ACTIVE flag was removed from the device link,
does enabling the channel clock before its parent power domain is resumed
risk an asynchronous external abort on NXP SoCs?
> + if (ret)
> + return dev_err_probe(&pdev->dev, ret,
> + "Failed to power on 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);
> + clk_disable_unprepare(fsl_chan->clk);
If the kernel is compiled with CONFIG_PM=n, pm_runtime_resume_and_get()
evaluates to 1 (success) but does not call the runtime PM resume callbacks.
By unconditionally disabling the clocks here, do they remain permanently
disabled when CONFIG_PM=n, leading to bus faults on subsequent DMA requests?
> + 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);
> @@ -880,6 +928,9 @@ 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);
>
> + clk_disable_unprepare(fsl_edma->dmaclk);
> + clk_bulk_disable_unprepare(fsl_edma->drvdata->dmamuxs, fsl_edma->muxclk);
These clocks were acquired using devm_clk_get_optional_enabled() earlier in
probe.
If they are manually disabled here, and the device is suspended during
driver unbind, won't the devres cleanup attempt to disable them again,
causing a Common Clock Framework underflow warning?
Similarly, if CONFIG_PM=n, these engine clocks will never be re-enabled.
[ ... ]
> @@ -928,6 +992,13 @@ static int fsl_edma_resume_early(struct device *dev)
> 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)
> @@ -940,6 +1011,35 @@ static int fsl_edma_resume_early(struct device *dev)
> return 0;
> }
Before returning, fsl_edma_resume_early() writes to regs->cr:
if (!(fsl_edma->drvdata->flags & FSL_EDMA_DRV_SPLIT_REG))
edma_writel(fsl_edma, EDMA_CR_ERGA | EDMA_CR_ERCA, regs->cr);
If all channels were idle prior to system suspend, the eDMA engine is in a
runtime-suspended state and dmaclk is gated. Since the PM core doesn't
automatically wake runtime-suspended devices during resume_early, could
writing to regs->cr while dmaclk is gated trigger a bus fault?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260513-b4-b4-edma-runtime-opt-v5-0-1e595bfb8423@nxp.com?part=4
^ permalink raw reply [flat|nested] 11+ messages in thread