* [PATCH] dmaengine: sprd: fix clock imbalance when enabling ashb_clk fails
@ 2026-08-13 10:53 Chaithanya Lagisetty
2026-08-13 16:39 ` Frank Li
0 siblings, 1 reply; 7+ messages in thread
From: Chaithanya Lagisetty @ 2026-08-13 10:53 UTC (permalink / raw)
To: Vinod Koul, Orson Zhai, Baolin Wang, Chunyan Zhang
Cc: Frank Li, Chaithanya Lagisetty, dmaengine, linux-kernel
sprd_dma_enable() enables sdev->clk and then enables the optional
ashb_clk. If clk_prepare_enable() for ashb_clk fails, the function
returns the error without disabling sdev->clk, which was already
enabled. Both callers (probe and resume) treat a non-zero return as
"nothing was enabled" and bail out, so sdev->clk is leaked.
Disable sdev->clk on the ashb_clk enable failure path.
Fixes: 9b3b8171f7f4 ("dmaengine: sprd: Add Spreadtrum DMA driver")
Signed-off-by: Chaithanya Lagisetty <nagachaithanya9911@gmail.com>
---
drivers/dma/sprd-dma.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/dma/sprd-dma.c b/drivers/dma/sprd-dma.c
index 087fea3af2e4..3ea5c3ba1a8c 100644
--- a/drivers/dma/sprd-dma.c
+++ b/drivers/dma/sprd-dma.c
@@ -270,9 +270,16 @@ static int sprd_dma_enable(struct sprd_dma_dev *sdev)
* The ashb_clk is optional and only for AGCP DMA controller, so we
* need add one condition to check if the ashb_clk need enable.
*/
- if (!IS_ERR(sdev->ashb_clk))
+ if (!IS_ERR(sdev->ashb_clk)) {
ret = clk_prepare_enable(sdev->ashb_clk);
+ if (ret)
+ goto disable_clk;
+ }
+ return 0;
+
+disable_clk:
+ clk_disable_unprepare(sdev->clk);
return ret;
}
--
2.43.0
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] dmaengine: sprd: fix clock imbalance when enabling ashb_clk fails 2026-08-13 10:53 [PATCH] dmaengine: sprd: fix clock imbalance when enabling ashb_clk fails Chaithanya Lagisetty @ 2026-08-13 16:39 ` Frank Li 2026-08-16 5:50 ` [PATCH v2] dmaengine: sprd: use clk_bulk API to fix clock imbalance Chaithanya Lagisetty 0 siblings, 1 reply; 7+ messages in thread From: Frank Li @ 2026-08-13 16:39 UTC (permalink / raw) To: Chaithanya Lagisetty Cc: Vinod Koul, Orson Zhai, Baolin Wang, Chunyan Zhang, Frank Li, dmaengine, linux-kernel On Thu, Aug 13, 2026 at 10:53:54AM +0000, Chaithanya Lagisetty wrote: > [You don't often get email from nagachaithanya9911@gmail.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ] > > sprd_dma_enable() enables sdev->clk and then enables the optional > ashb_clk. If clk_prepare_enable() for ashb_clk fails, the function > returns the error without disabling sdev->clk, which was already > enabled. Both callers (probe and resume) treat a non-zero return as > "nothing was enabled" and bail out, so sdev->clk is leaked. > > Disable sdev->clk on the ashb_clk enable failure path. > > Fixes: 9b3b8171f7f4 ("dmaengine: sprd: Add Spreadtrum DMA driver") > Signed-off-by: Chaithanya Lagisetty <nagachaithanya9911@gmail.com> > --- > drivers/dma/sprd-dma.c | 9 ++++++++- > 1 file changed, 8 insertions(+), 1 deletion(-) > > diff --git a/drivers/dma/sprd-dma.c b/drivers/dma/sprd-dma.c > index 087fea3af2e4..3ea5c3ba1a8c 100644 > --- a/drivers/dma/sprd-dma.c > +++ b/drivers/dma/sprd-dma.c > @@ -270,9 +270,16 @@ static int sprd_dma_enable(struct sprd_dma_dev *sdev) > * The ashb_clk is optional and only for AGCP DMA controller, so we > * need add one condition to check if the ashb_clk need enable. > */ > - if (!IS_ERR(sdev->ashb_clk)) > + if (!IS_ERR(sdev->ashb_clk)) { > ret = clk_prepare_enable(sdev->ashb_clk); > + if (ret) > + goto disable_clk; > + } > > + return 0; > + > +disable_clk: > + clk_disable_unprepare(sdev->clk); Convert to clk bulk API. Frank > return ret; > } > > -- > 2.43.0 > ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2] dmaengine: sprd: use clk_bulk API to fix clock imbalance 2026-08-13 16:39 ` Frank Li @ 2026-08-16 5:50 ` Chaithanya Lagisetty 2026-08-17 14:48 ` Frank Li 0 siblings, 1 reply; 7+ messages in thread From: Chaithanya Lagisetty @ 2026-08-16 5:50 UTC (permalink / raw) To: vkoul Cc: Frank.Li, orsonzhai, baolin.wang, zhang.lyra, dmaengine, linux-kernel, nagachaithanya9911 sprd_dma_enable() enabled sdev->clk and then the optional ashb_clk. If enabling ashb_clk failed, sdev->clk was left enabled: both callers (probe and resume) treat a non-zero return as "nothing was enabled" and bail out, leaking sdev->clk. Convert the driver to the clk_bulk API. clk_bulk_prepare_enable() enables all clocks and unwinds them on failure, and clk_bulk_disable_unprepare() disables them, which fixes the imbalance and simplifies the enable/disable paths. The optional ashb_eb clock is fetched with devm_clk_get_optional(), so a missing clock becomes a NULL entry that the bulk helpers treat as a no-op. Fixes: 9b3b8171f7f4 ("dmaengine: sprd: Add Spreadtrum DMA driver") Signed-off-by: Chaithanya Lagisetty <nagachaithanya9911@gmail.com> --- Changes since v1: - Convert the enable/disable paths to the clk_bulk API instead of manually disabling sdev->clk on the ashb_clk failure path, and fetch the optional ashb_eb clock with devm_clk_get_optional() (Frank Li). v1: https://lore.kernel.org/dmaengine/20260813105354.2577040-1-nagachaithanya9911@gmail.com/ drivers/dma/sprd-dma.c | 44 +++++++++++++----------------------------- 1 file changed, 13 insertions(+), 31 deletions(-) diff --git a/drivers/dma/sprd-dma.c b/drivers/dma/sprd-dma.c index 087fea3af2e4..ac6fe6118ad1 100644 --- a/drivers/dma/sprd-dma.c +++ b/drivers/dma/sprd-dma.c @@ -208,8 +208,7 @@ struct sprd_dma_chn { struct sprd_dma_dev { struct dma_device dma_dev; void __iomem *glb_base; - struct clk *clk; - struct clk *ashb_clk; + struct clk_bulk_data clks[2]; int irq; u32 total_chns; struct sprd_dma_chn channels[] __counted_by(total_chns); @@ -260,31 +259,12 @@ static void sprd_dma_chn_update(struct sprd_dma_chn *schan, u32 reg, static int sprd_dma_enable(struct sprd_dma_dev *sdev) { - int ret; - - ret = clk_prepare_enable(sdev->clk); - if (ret) - return ret; - - /* - * The ashb_clk is optional and only for AGCP DMA controller, so we - * need add one condition to check if the ashb_clk need enable. - */ - if (!IS_ERR(sdev->ashb_clk)) - ret = clk_prepare_enable(sdev->ashb_clk); - - return ret; + return clk_bulk_prepare_enable(ARRAY_SIZE(sdev->clks), sdev->clks); } static void sprd_dma_disable(struct sprd_dma_dev *sdev) { - clk_disable_unprepare(sdev->clk); - - /* - * Need to check if we need disable the optional ashb_clk for AGCP DMA. - */ - if (!IS_ERR(sdev->ashb_clk)) - clk_disable_unprepare(sdev->ashb_clk); + clk_bulk_disable_unprepare(ARRAY_SIZE(sdev->clks), sdev->clks); } static void sprd_dma_set_uid(struct sprd_dma_chn *schan) @@ -1140,16 +1120,18 @@ static int sprd_dma_probe(struct platform_device *pdev) if (!sdev) return -ENOMEM; - sdev->clk = devm_clk_get(&pdev->dev, "enable"); - if (IS_ERR(sdev->clk)) { - dev_err(&pdev->dev, "get enable clock failed\n"); - return PTR_ERR(sdev->clk); - } + sdev->clks[0].id = "enable"; + sdev->clks[0].clk = devm_clk_get(&pdev->dev, "enable"); + if (IS_ERR(sdev->clks[0].clk)) + return dev_err_probe(&pdev->dev, PTR_ERR(sdev->clks[0].clk), + "get enable clock failed\n"); /* ashb clock is optional for AGCP DMA */ - sdev->ashb_clk = devm_clk_get(&pdev->dev, "ashb_eb"); - if (IS_ERR(sdev->ashb_clk)) - dev_warn(&pdev->dev, "no optional ashb eb clock\n"); + sdev->clks[1].id = "ashb_eb"; + sdev->clks[1].clk = devm_clk_get_optional(&pdev->dev, "ashb_eb"); + if (IS_ERR(sdev->clks[1].clk)) + return dev_err_probe(&pdev->dev, PTR_ERR(sdev->clks[1].clk), + "get ashb_eb clock failed\n"); /* * We have three DMA controllers: AP DMA, AON DMA and AGCP DMA. For AGCP -- 2.43.0 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] dmaengine: sprd: use clk_bulk API to fix clock imbalance 2026-08-16 5:50 ` [PATCH v2] dmaengine: sprd: use clk_bulk API to fix clock imbalance Chaithanya Lagisetty @ 2026-08-17 14:48 ` Frank Li 2026-08-19 5:35 ` [PATCH v3] " Chaithanya Lagisetty 0 siblings, 1 reply; 7+ messages in thread From: Frank Li @ 2026-08-17 14:48 UTC (permalink / raw) To: Chaithanya Lagisetty Cc: vkoul, Frank.Li, orsonzhai, baolin.wang, zhang.lyra, dmaengine, linux-kernel On Sun, Aug 16, 2026 at 05:50:05AM +0000, Chaithanya Lagisetty wrote: > sprd_dma_enable() enabled sdev->clk and then the optional ashb_clk. If > enabling ashb_clk failed, sdev->clk was left enabled: both callers > (probe and resume) treat a non-zero return as "nothing was enabled" and > bail out, leaking sdev->clk. > > Convert the driver to the clk_bulk API. clk_bulk_prepare_enable() > enables all clocks and unwinds them on failure, and > clk_bulk_disable_unprepare() disables them, which fixes the imbalance > and simplifies the enable/disable paths. The optional ashb_eb clock is > fetched with devm_clk_get_optional(), so a missing clock becomes a NULL > entry that the bulk helpers treat as a no-op. > > Fixes: 9b3b8171f7f4 ("dmaengine: sprd: Add Spreadtrum DMA driver") > Signed-off-by: Chaithanya Lagisetty <nagachaithanya9911@gmail.com> > --- > Changes since v1: > - Convert the enable/disable paths to the clk_bulk API instead of > manually disabling sdev->clk on the ashb_clk failure path, and fetch > the optional ashb_eb clock with devm_clk_get_optional() (Frank Li). > > v1: https://lore.kernel.org/dmaengine/20260813105354.2577040-1-nagachaithanya9911@gmail.com/ > > drivers/dma/sprd-dma.c | 44 +++++++++++++----------------------------- > 1 file changed, 13 insertions(+), 31 deletions(-) > > diff --git a/drivers/dma/sprd-dma.c b/drivers/dma/sprd-dma.c > index 087fea3af2e4..ac6fe6118ad1 100644 > --- a/drivers/dma/sprd-dma.c > +++ b/drivers/dma/sprd-dma.c > @@ -208,8 +208,7 @@ struct sprd_dma_chn { > struct sprd_dma_dev { > struct dma_device dma_dev; > void __iomem *glb_base; > - struct clk *clk; > - struct clk *ashb_clk; > + struct clk_bulk_data clks[2]; > int irq; > u32 total_chns; > struct sprd_dma_chn channels[] __counted_by(total_chns); > @@ -260,31 +259,12 @@ static void sprd_dma_chn_update(struct sprd_dma_chn *schan, u32 reg, > > static int sprd_dma_enable(struct sprd_dma_dev *sdev) > { > - int ret; > - > - ret = clk_prepare_enable(sdev->clk); > - if (ret) > - return ret; > - > - /* > - * The ashb_clk is optional and only for AGCP DMA controller, so we > - * need add one condition to check if the ashb_clk need enable. > - */ > - if (!IS_ERR(sdev->ashb_clk)) > - ret = clk_prepare_enable(sdev->ashb_clk); > - > - return ret; > + return clk_bulk_prepare_enable(ARRAY_SIZE(sdev->clks), sdev->clks); > } > > static void sprd_dma_disable(struct sprd_dma_dev *sdev) > { > - clk_disable_unprepare(sdev->clk); > - > - /* > - * Need to check if we need disable the optional ashb_clk for AGCP DMA. > - */ > - if (!IS_ERR(sdev->ashb_clk)) > - clk_disable_unprepare(sdev->ashb_clk); > + clk_bulk_disable_unprepare(ARRAY_SIZE(sdev->clks), sdev->clks); > } > > static void sprd_dma_set_uid(struct sprd_dma_chn *schan) > @@ -1140,16 +1120,18 @@ static int sprd_dma_probe(struct platform_device *pdev) > if (!sdev) > return -ENOMEM; > > - sdev->clk = devm_clk_get(&pdev->dev, "enable"); > - if (IS_ERR(sdev->clk)) { > - dev_err(&pdev->dev, "get enable clock failed\n"); > - return PTR_ERR(sdev->clk); > - } > + sdev->clks[0].id = "enable"; > + sdev->clks[0].clk = devm_clk_get(&pdev->dev, "enable"); > + if (IS_ERR(sdev->clks[0].clk)) > + return dev_err_probe(&pdev->dev, PTR_ERR(sdev->clks[0].clk), > + "get enable clock failed\n"); > > /* ashb clock is optional for AGCP DMA */ > - sdev->ashb_clk = devm_clk_get(&pdev->dev, "ashb_eb"); > - if (IS_ERR(sdev->ashb_clk)) > - dev_warn(&pdev->dev, "no optional ashb eb clock\n"); > + sdev->clks[1].id = "ashb_eb"; > + sdev->clks[1].clk = devm_clk_get_optional(&pdev->dev, "ashb_eb"); > + if (IS_ERR(sdev->clks[1].clk)) > + return dev_err_probe(&pdev->dev, PTR_ERR(sdev->clks[1].clk), > + "get ashb_eb clock failed\n"); sdev->clks[0].id = "enable"; sdev->clks[1].id = "ashb_eb"; ret = devm_clk_bulk_get_optional(dev, ARRAY_SIZE(sdev->clks), sdev->clks); if (ret && !sdev->clks[0].clk) return sdev->clks[0].clk(); Frank > > /* > * We have three DMA controllers: AP DMA, AON DMA and AGCP DMA. For AGCP > -- > 2.43.0 > ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v3] dmaengine: sprd: use clk_bulk API to fix clock imbalance 2026-08-17 14:48 ` Frank Li @ 2026-08-19 5:35 ` Chaithanya Lagisetty 2026-08-19 15:42 ` Frank Li 2026-09-10 14:10 ` Vinod Koul 0 siblings, 2 replies; 7+ messages in thread From: Chaithanya Lagisetty @ 2026-08-19 5:35 UTC (permalink / raw) To: frank.li Cc: Frank.Li, vkoul, orsonzhai, baolin.wang, zhang.lyra, dmaengine, linux-kernel, Chaithanya Lagisetty sprd_dma_enable() enabled sdev->clk and then the optional ashb_clk. If enabling ashb_clk failed, sdev->clk was left enabled: both callers (probe and resume) treat a non-zero return as "nothing was enabled" and bail out, leaking sdev->clk. Convert the driver to the clk_bulk API. clk_bulk_prepare_enable() enables all clocks and unwinds them on failure, and clk_bulk_disable_unprepare() disables them, which fixes the imbalance and simplifies the enable/disable paths. Both clocks are fetched with devm_clk_bulk_get_optional(); the optional ashb_eb clock simply becomes a NULL entry that the bulk helpers treat as a no-op, while the mandatory enable clock is still checked explicitly. Fixes: 9b3b8171f7f4 ("dmaengine: sprd: Add Spreadtrum DMA driver") Signed-off-by: Chaithanya Lagisetty <nagachaithanya9911@gmail.com> --- Changes since v2: - Fetch both clocks with a single devm_clk_bulk_get_optional() call instead of separate devm_clk_get()/devm_clk_get_optional() calls, and keep an explicit check for the mandatory "enable" clock (Frank Li). Changes since v1: - Convert the enable/disable paths to the clk_bulk API instead of manually disabling sdev->clk on the ashb_clk failure path (Frank Li). v2: https://lore.kernel.org/dmaengine/20260816055005.2667930-1-nagachaithanya9911@gmail.com/ v1: https://lore.kernel.org/dmaengine/20260813105354.2577040-1-nagachaithanya9911@gmail.com/ drivers/dma/sprd-dma.c | 45 ++++++++++++------------------------------ 1 file changed, 13 insertions(+), 32 deletions(-) diff --git a/drivers/dma/sprd-dma.c b/drivers/dma/sprd-dma.c index 087fea3af2e4..749cabd984eb 100644 --- a/drivers/dma/sprd-dma.c +++ b/drivers/dma/sprd-dma.c @@ -208,8 +208,7 @@ struct sprd_dma_chn { struct sprd_dma_dev { struct dma_device dma_dev; void __iomem *glb_base; - struct clk *clk; - struct clk *ashb_clk; + struct clk_bulk_data clks[2]; int irq; u32 total_chns; struct sprd_dma_chn channels[] __counted_by(total_chns); @@ -260,31 +259,12 @@ static void sprd_dma_chn_update(struct sprd_dma_chn *schan, u32 reg, static int sprd_dma_enable(struct sprd_dma_dev *sdev) { - int ret; - - ret = clk_prepare_enable(sdev->clk); - if (ret) - return ret; - - /* - * The ashb_clk is optional and only for AGCP DMA controller, so we - * need add one condition to check if the ashb_clk need enable. - */ - if (!IS_ERR(sdev->ashb_clk)) - ret = clk_prepare_enable(sdev->ashb_clk); - - return ret; + return clk_bulk_prepare_enable(ARRAY_SIZE(sdev->clks), sdev->clks); } static void sprd_dma_disable(struct sprd_dma_dev *sdev) { - clk_disable_unprepare(sdev->clk); - - /* - * Need to check if we need disable the optional ashb_clk for AGCP DMA. - */ - if (!IS_ERR(sdev->ashb_clk)) - clk_disable_unprepare(sdev->ashb_clk); + clk_bulk_disable_unprepare(ARRAY_SIZE(sdev->clks), sdev->clks); } static void sprd_dma_set_uid(struct sprd_dma_chn *schan) @@ -1140,16 +1120,17 @@ static int sprd_dma_probe(struct platform_device *pdev) if (!sdev) return -ENOMEM; - sdev->clk = devm_clk_get(&pdev->dev, "enable"); - if (IS_ERR(sdev->clk)) { - dev_err(&pdev->dev, "get enable clock failed\n"); - return PTR_ERR(sdev->clk); - } + /* The ashb_eb clock is optional and only present for AGCP DMA */ + sdev->clks[0].id = "enable"; + sdev->clks[1].id = "ashb_eb"; + ret = devm_clk_bulk_get_optional(&pdev->dev, ARRAY_SIZE(sdev->clks), + sdev->clks); + if (ret) + return dev_err_probe(&pdev->dev, ret, "get clocks failed\n"); - /* ashb clock is optional for AGCP DMA */ - sdev->ashb_clk = devm_clk_get(&pdev->dev, "ashb_eb"); - if (IS_ERR(sdev->ashb_clk)) - dev_warn(&pdev->dev, "no optional ashb eb clock\n"); + if (!sdev->clks[0].clk) + return dev_err_probe(&pdev->dev, -ENOENT, + "get enable clock failed\n"); /* * We have three DMA controllers: AP DMA, AON DMA and AGCP DMA. For AGCP -- 2.43.0 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v3] dmaengine: sprd: use clk_bulk API to fix clock imbalance 2026-08-19 5:35 ` [PATCH v3] " Chaithanya Lagisetty @ 2026-08-19 15:42 ` Frank Li 2026-09-10 14:10 ` Vinod Koul 1 sibling, 0 replies; 7+ messages in thread From: Frank Li @ 2026-08-19 15:42 UTC (permalink / raw) To: Chaithanya Lagisetty Cc: Frank.Li, vkoul, orsonzhai, baolin.wang, zhang.lyra, dmaengine, linux-kernel On Wed, Aug 19, 2026 at 05:35:18AM +0000, Chaithanya Lagisetty wrote: > sprd_dma_enable() enabled sdev->clk and then the optional ashb_clk. If > enabling ashb_clk failed, sdev->clk was left enabled: both callers > (probe and resume) treat a non-zero return as "nothing was enabled" and > bail out, leaking sdev->clk. > > Convert the driver to the clk_bulk API. clk_bulk_prepare_enable() > enables all clocks and unwinds them on failure, and > clk_bulk_disable_unprepare() disables them, which fixes the imbalance > and simplifies the enable/disable paths. Both clocks are fetched with > devm_clk_bulk_get_optional(); the optional ashb_eb clock simply becomes > a NULL entry that the bulk helpers treat as a no-op, while the mandatory > enable clock is still checked explicitly. > > Fixes: 9b3b8171f7f4 ("dmaengine: sprd: Add Spreadtrum DMA driver") > Signed-off-by: Chaithanya Lagisetty <nagachaithanya9911@gmail.com> > --- Reviewed-by: Frank Li <Frank.Li@nxp.com> > Changes since v2: > - Fetch both clocks with a single devm_clk_bulk_get_optional() call > instead of separate devm_clk_get()/devm_clk_get_optional() calls, and > keep an explicit check for the mandatory "enable" clock (Frank Li). > > Changes since v1: > - Convert the enable/disable paths to the clk_bulk API instead of > manually disabling sdev->clk on the ashb_clk failure path (Frank Li). > > v2: https://lore.kernel.org/dmaengine/20260816055005.2667930-1-nagachaithanya9911@gmail.com/ > v1: https://lore.kernel.org/dmaengine/20260813105354.2577040-1-nagachaithanya9911@gmail.com/ > > drivers/dma/sprd-dma.c | 45 ++++++++++++------------------------------ > 1 file changed, 13 insertions(+), 32 deletions(-) > > diff --git a/drivers/dma/sprd-dma.c b/drivers/dma/sprd-dma.c > index 087fea3af2e4..749cabd984eb 100644 > --- a/drivers/dma/sprd-dma.c > +++ b/drivers/dma/sprd-dma.c > @@ -208,8 +208,7 @@ struct sprd_dma_chn { > struct sprd_dma_dev { > struct dma_device dma_dev; > void __iomem *glb_base; > - struct clk *clk; > - struct clk *ashb_clk; > + struct clk_bulk_data clks[2]; > int irq; > u32 total_chns; > struct sprd_dma_chn channels[] __counted_by(total_chns); > @@ -260,31 +259,12 @@ static void sprd_dma_chn_update(struct sprd_dma_chn *schan, u32 reg, > > static int sprd_dma_enable(struct sprd_dma_dev *sdev) > { > - int ret; > - > - ret = clk_prepare_enable(sdev->clk); > - if (ret) > - return ret; > - > - /* > - * The ashb_clk is optional and only for AGCP DMA controller, so we > - * need add one condition to check if the ashb_clk need enable. > - */ > - if (!IS_ERR(sdev->ashb_clk)) > - ret = clk_prepare_enable(sdev->ashb_clk); > - > - return ret; > + return clk_bulk_prepare_enable(ARRAY_SIZE(sdev->clks), sdev->clks); > } > > static void sprd_dma_disable(struct sprd_dma_dev *sdev) > { > - clk_disable_unprepare(sdev->clk); > - > - /* > - * Need to check if we need disable the optional ashb_clk for AGCP DMA. > - */ > - if (!IS_ERR(sdev->ashb_clk)) > - clk_disable_unprepare(sdev->ashb_clk); > + clk_bulk_disable_unprepare(ARRAY_SIZE(sdev->clks), sdev->clks); > } > > static void sprd_dma_set_uid(struct sprd_dma_chn *schan) > @@ -1140,16 +1120,17 @@ static int sprd_dma_probe(struct platform_device *pdev) > if (!sdev) > return -ENOMEM; > > - sdev->clk = devm_clk_get(&pdev->dev, "enable"); > - if (IS_ERR(sdev->clk)) { > - dev_err(&pdev->dev, "get enable clock failed\n"); > - return PTR_ERR(sdev->clk); > - } > + /* The ashb_eb clock is optional and only present for AGCP DMA */ > + sdev->clks[0].id = "enable"; > + sdev->clks[1].id = "ashb_eb"; > + ret = devm_clk_bulk_get_optional(&pdev->dev, ARRAY_SIZE(sdev->clks), > + sdev->clks); > + if (ret) > + return dev_err_probe(&pdev->dev, ret, "get clocks failed\n"); > > - /* ashb clock is optional for AGCP DMA */ > - sdev->ashb_clk = devm_clk_get(&pdev->dev, "ashb_eb"); > - if (IS_ERR(sdev->ashb_clk)) > - dev_warn(&pdev->dev, "no optional ashb eb clock\n"); > + if (!sdev->clks[0].clk) > + return dev_err_probe(&pdev->dev, -ENOENT, > + "get enable clock failed\n"); > > /* > * We have three DMA controllers: AP DMA, AON DMA and AGCP DMA. For AGCP > -- > 2.43.0 > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v3] dmaengine: sprd: use clk_bulk API to fix clock imbalance 2026-08-19 5:35 ` [PATCH v3] " Chaithanya Lagisetty 2026-08-19 15:42 ` Frank Li @ 2026-09-10 14:10 ` Vinod Koul 1 sibling, 0 replies; 7+ messages in thread From: Vinod Koul @ 2026-09-10 14:10 UTC (permalink / raw) To: frank.li, Chaithanya Lagisetty Cc: Frank.Li, orsonzhai, baolin.wang, zhang.lyra, dmaengine, linux-kernel On Wed, 19 Aug 2026 05:35:18 +0000, Chaithanya Lagisetty wrote: > sprd_dma_enable() enabled sdev->clk and then the optional ashb_clk. If > enabling ashb_clk failed, sdev->clk was left enabled: both callers > (probe and resume) treat a non-zero return as "nothing was enabled" and > bail out, leaking sdev->clk. > > Convert the driver to the clk_bulk API. clk_bulk_prepare_enable() > enables all clocks and unwinds them on failure, and > clk_bulk_disable_unprepare() disables them, which fixes the imbalance > and simplifies the enable/disable paths. Both clocks are fetched with > devm_clk_bulk_get_optional(); the optional ashb_eb clock simply becomes > a NULL entry that the bulk helpers treat as a no-op, while the mandatory > enable clock is still checked explicitly. > > [...] Applied, thanks! [1/1] dmaengine: sprd: use clk_bulk API to fix clock imbalance commit: d09cd5e8bbc98969b5333015b45c525078ac0f6f Best regards, -- ~Vinod ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2026-09-10 14:10 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2026-08-13 10:53 [PATCH] dmaengine: sprd: fix clock imbalance when enabling ashb_clk fails Chaithanya Lagisetty 2026-08-13 16:39 ` Frank Li 2026-08-16 5:50 ` [PATCH v2] dmaengine: sprd: use clk_bulk API to fix clock imbalance Chaithanya Lagisetty 2026-08-17 14:48 ` Frank Li 2026-08-19 5:35 ` [PATCH v3] " Chaithanya Lagisetty 2026-08-19 15:42 ` Frank Li 2026-09-10 14:10 ` Vinod Koul
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®