[PATCH] clk: Fix clk_set_parent() regression

[PATCH] clk: Fix clk_set_parent() regression

[Jonas Karlman]

The commit ac30d90f3367 ("clk: Ensure the parent clocks are enabled
while reparenting") add a call to clk_enable() for the parent clock.

For clock drivers that do not implement the enable() ops, like most
Rockchip clock drivers, this now cause the set_parent() ops to never
be called when CLK_CCF=n (default for Rockchip).

clk_enable() typically return -NOSYS when the enable() ops is not
implemented by the clock driver, with CLK_CCF=y clk_enable() instead
return 0 when the enable() ops is unimplemented.

Change to ignore -NOSYS from the newly introduced clk_enable() call to
fix this regression and restore the old behavior of set_parent() ops
being called regardless of if enable() ops is implemented or not.

Fixes: ac30d90f3367 ("clk: Ensure the parent clocks are enabled while reparenting")
Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
---
 drivers/clk/clk-uclass.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/clk-uclass.c b/drivers/clk/clk-uclass.c
index 4b3d812f9c65..93dd32ebe74d 100644
--- a/drivers/clk/clk-uclass.c
+++ b/drivers/clk/clk-uclass.c
@@ -624,7 +624,7 @@ int clk_set_parent(struct clk *clk, struct clk *parent)
 		return -ENOSYS;
 
 	ret = clk_enable(parent);
-	if (ret) {
+	if (ret && ret != -ENOSYS) {
 		printf("Cannot enable parent %s\n", parent->dev->name);
 		return ret;
 	}
-- 
2.49.0

Re: [PATCH] clk: Fix clk_set_parent() regression

[Dang Huynh]

Hi Jonas,

Tested on PineTab 2 (RK3566) and this got rid of "Cannot enable parent"
errors.

Tested-by: Dang Huynh <danct12@riseup.net>

On Sat, Apr 19, 2025 at 09:07:16AM +0000, Jonas Karlman wrote:
> The commit ac30d90f3367 ("clk: Ensure the parent clocks are enabled
> while reparenting") add a call to clk_enable() for the parent clock.
> 
> For clock drivers that do not implement the enable() ops, like most
> Rockchip clock drivers, this now cause the set_parent() ops to never
> be called when CLK_CCF=n (default for Rockchip).
> 
> clk_enable() typically return -NOSYS when the enable() ops is not
> implemented by the clock driver, with CLK_CCF=y clk_enable() instead
> return 0 when the enable() ops is unimplemented.
> 
> Change to ignore -NOSYS from the newly introduced clk_enable() call to
> fix this regression and restore the old behavior of set_parent() ops
> being called regardless of if enable() ops is implemented or not.
> 
> Fixes: ac30d90f3367 ("clk: Ensure the parent clocks are enabled while reparenting")
> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
> ---
>  drivers/clk/clk-uclass.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/clk/clk-uclass.c b/drivers/clk/clk-uclass.c
> index 4b3d812f9c65..93dd32ebe74d 100644
> --- a/drivers/clk/clk-uclass.c
> +++ b/drivers/clk/clk-uclass.c
> @@ -624,7 +624,7 @@ int clk_set_parent(struct clk *clk, struct clk *parent)
>  		return -ENOSYS;
>  
>  	ret = clk_enable(parent);
> -	if (ret) {
> +	if (ret && ret != -ENOSYS) {
>  		printf("Cannot enable parent %s\n", parent->dev->name);
>  		return ret;
>  	}
> -- 
> 2.49.0
> 

Re: [PATCH] clk: Fix clk_set_parent() regression

[Miquel Raynal]

On 19/04/2025 at 09:07:16 GMT, Jonas Karlman <jonas@kwiboo.se> wrote:

> The commit ac30d90f3367 ("clk: Ensure the parent clocks are enabled
> while reparenting") add a call to clk_enable() for the parent clock.
>
> For clock drivers that do not implement the enable() ops, like most
> Rockchip clock drivers, this now cause the set_parent() ops to never
> be called when CLK_CCF=n (default for Rockchip).
>
> clk_enable() typically return -NOSYS when the enable() ops is not
> implemented by the clock driver, with CLK_CCF=y clk_enable() instead
> return 0 when the enable() ops is unimplemented.

I am really not a big fan of this logic to return low-level information
in high-level helpers. I am constantly disagreeing with this approach
because this is exactly the kind of breakage it leads to.

> Change to ignore -NOSYS from the newly introduced clk_enable() call to
> fix this regression and restore the old behavior of set_parent() ops
> being called regardless of if enable() ops is implemented or not.
>
> Fixes: ac30d90f3367 ("clk: Ensure the parent clocks are enabled while reparenting")
> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>

Thanks for the fix,

Acked-by: Miquel Raynal <miquel.raynal@bootlin.com>

Miquèl