[PATCH] clk: Do not disable clocks when clk_get_bulk() fails

[PATCH] clk: Do not disable clocks when clk_get_bulk() fails

[Yuho Choi]

clk_get_bulk() looks up the clocks of a device one by one. If the
lookup of one clock fails, it calls clk_release_all() on the clocks
already looked up before returning the error. clk_release_all() calls
clk_disable() on each of them, although clk_get_bulk() never enabled
them.

With CLK_CCF, clocks registered by id are reference counted. The extra
clk_disable() is ignored while nobody has the clock enabled, but if
another device has enabled it, it drops that device's reference and,
if that was the last one, turns the clock off. Without CLK_CCF, and
for CCF clocks that are not registered by id, clk_disable() calls the
provider's disable op directly. A device whose clocks property names a
clock that cannot be looked up, e.g. because its provider is not
enabled, can therefore turn off clocks listed before it that firmware
or another device relies on.

There is no release operation for a clock that has only been looked
up: clk_free() had become a no-op and was removed in
commit c9309f40a683 ("treewide: Remove clk_free"). Therefore, 
clk_release_all() does not undo the lookup, it only disables the clocks.
Return the error directly, and reset bulk->count so that the bulk is
left empty and a later clk_disable_bulk() or clk_release_bulk() on it
does nothing.

Fixes: a855be87da49 ("clk: Add get/enable/disable/release for a bulk of clocks")
Signed-off-by: Yuho Choi <oss.patchbox@gmail.com>
---
 drivers/clk/clk-uclass.c | 16 +++++-----------
 1 file changed, 5 insertions(+), 11 deletions(-)

diff --git a/drivers/clk/clk-uclass.c b/drivers/clk/clk-uclass.c
index 3fc772c8b4f..3d0b1c4dfff 100644
--- a/drivers/clk/clk-uclass.c
+++ b/drivers/clk/clk-uclass.c
@@ -159,7 +159,7 @@ int clk_get_by_index_nodev(ofnode node, int index, struct clk *clk)
 
 int clk_get_bulk(struct udevice *dev, struct clk_bulk *bulk)
 {
-	int i, ret, err, count;
+	int i, ret, count;
 
 	bulk->count = 0;
 
@@ -173,21 +173,15 @@ int clk_get_bulk(struct udevice *dev, struct clk_bulk *bulk)
 
 	for (i = 0; i < count; i++) {
 		ret = clk_get_by_index(dev, i, &bulk->clks[i]);
-		if (ret < 0)
-			goto bulk_get_err;
+		if (ret < 0) {
+			bulk->count = 0;
+			return ret;
+		}
 
 		++bulk->count;
 	}
 
 	return 0;
-
-bulk_get_err:
-	err = clk_release_all(bulk->clks, bulk->count);
-	if (err)
-		debug("%s: could not release all clocks for %s\n",
-		      __func__, dev_read_name(dev));
-
-	return ret;
 }
 
 static struct clk *clk_set_default_get_by_id(struct clk *clk)
-- 
2.43.0

Re: [PATCH] clk: Do not disable clocks when clk_get_bulk() fails

[Simon Glass]

On 2026-10-27T14:38:42, Yuho Choi <oss.patchbox@gmail.com> wrote:
> clk: Do not disable clocks when clk_get_bulk() fails
>
> clk_get_bulk() looks up the clocks of a device one by one. If the
> lookup of one clock fails, it calls clk_release_all() on the clocks
> already looked up before returning the error. clk_release_all() calls
> clk_disable() on each of them, although clk_get_bulk() never enabled
> them.
>
> With CLK_CCF, clocks registered by id are reference counted. The extra
> clk_disable() is ignored while nobody has the clock enabled, but if
> another device has enabled it, it drops that device's reference and,
> if that was the last one, turns the clock off. Without CLK_CCF, and
> for CCF clocks that are not registered by id, clk_disable() calls the
> provider's disable op directly. A device whose clocks property names a
> clock that cannot be looked up, e.g. because its provider is not
> enabled, can therefore turn off clocks listed before it that firmware
> or another device relies on.
>
> There is no release operation for a clock that has only been looked
> up: clk_free() had become a no-op and was removed in
> commit c9309f40a683 ("treewide: Remove clk_free"). Therefore,
> clk_release_all() does not undo the lookup, it only disables the clocks.
> Return the error directly, and reset bulk->count so that the bulk is
> left empty and a later clk_disable_bulk() or clk_release_bulk() on it
> does nothing.
>
> Fixes: a855be87da49 ("clk: Add get/enable/disable/release for a bulk of clocks")
> Signed-off-by: Yuho Choi <oss.patchbox@gmail.com>
>
>  drivers/clk/clk-uclass.c | 16 +++++-----------
>  1 file changed, 5 insertions(+), 11 deletions(-)

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