[PATCH] clk: Disable parent clock when clk_enable() fails

[PATCH] clk: Disable parent clock when clk_enable() fails

[Yuho Choi]

With CONFIG_CLK_CCF, clk_enable() on a clock that is registered by id
and has an enable_count of zero first enables the parent clock if the
parent device is a clock, and only then calls the clock's own enable
op. If that op fails, for example because clk_fracn_gppll_prepare()
returns -ETIMEDOUT when an i.MX93 PLL does not lock, clk_enable()
returns the error without undoing the parent enable.

The enable_count of the clock itself is not incremented in that case, so
a later clk_disable() of it returns early with "already disabled" and
never propagates to the parent. Every failed attempt therefore leaves
the parent's enable_count one higher, and the parent stays enabled even
after all of its real users have disabled it.

Disable the parent again when the enable op fails, the same way
clk_disable() drops the parent reference when the last user of a clock
goes away.

Fixes: 0520be0f67e3 ("clk: prograte clk enable/disable to parent")
Signed-off-by: Yuho Choi <oss.patchbox@gmail.com>
---
 drivers/clk/clk-uclass.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/clk-uclass.c b/drivers/clk/clk-uclass.c
index 0069ce9a887..3fc772c8b4f 100644
--- a/drivers/clk/clk-uclass.c
+++ b/drivers/clk/clk-uclass.c
@@ -669,7 +669,7 @@ int clk_set_parent(struct clk *clk, struct clk *parent)
 int clk_enable(struct clk *clk)
 {
 	const struct clk_ops *ops;
-	struct clk *clkp = NULL;
+	struct clk *clkp = NULL, *parent = NULL;
 	int ret;
 
 	debug("%s(clk=%p name=%s)\n", __func__, clk, clk ? clk->dev->name : "NULL");
@@ -687,7 +687,8 @@ int clk_enable(struct clk *clk)
 			}
 			if (clkp->dev->parent &&
 			    device_get_uclass_id(clkp->dev->parent) == UCLASS_CLK) {
-				ret = clk_enable(dev_get_clk_ptr(clkp->dev->parent));
+				parent = dev_get_clk_ptr(clkp->dev->parent);
+				ret = clk_enable(parent);
 				if (ret) {
 					printf("Enable %s failed\n",
 					       clkp->dev->parent->name);
@@ -700,6 +701,8 @@ int clk_enable(struct clk *clk)
 			ret = ops->enable(clkp ? clkp : clk);
 			if (ret) {
 				printf("Enable %s failed\n", clk->dev->name);
+				if (parent)
+					clk_disable(parent);
 				return ret;
 			}
 		}
-- 
2.43.0

Re: [PATCH] clk: Disable parent clock when clk_enable() fails

[Simon Glass]

On 2026-10-27T18:40:16, Yuho Choi <oss.patchbox@gmail.com> wrote:
> clk: Disable parent clock when clk_enable() fails
>
> With CONFIG_CLK_CCF, clk_enable() on a clock that is registered by id
> and has an enable_count of zero first enables the parent clock if the
> parent device is a clock, and only then calls the clock's own enable
> op. If that op fails, for example because clk_fracn_gppll_prepare()
> returns -ETIMEDOUT when an i.MX93 PLL does not lock, clk_enable()
> returns the error without undoing the parent enable.
>
> The enable_count of the clock itself is not incremented in that case, so
> a later clk_disable() of it returns early with "already disabled" and
> never propagates to the parent. Every failed attempt therefore leaves
> the parent's enable_count one higher, and the parent stays enabled even
> after all of its real users have disabled it.
>
> Disable the parent again when the enable op fails, the same way
> clk_disable() drops the parent reference when the last user of a clock
> goes away.
>
> Fixes: 0520be0f67e3 ("clk: prograte clk enable/disable to parent")
> Signed-off-by: Yuho Choi <oss.patchbox@gmail.com>
>
>  drivers/clk/clk-uclass.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>