[PATCH] sh: clk: Fix clk_enable() to return 0 on NULL clk

[PATCH] sh: clk: Fix clk_enable() to return 0 on NULL clk

[Geert Uytterhoeven]

On SH, devm_clk_get_optional_enabled() fails with -EINVAL if the clock
is not found.  This happens because __devm_clk_get() assumes it can pass
a NULL clock pointer (as returned by clk_get_optional()) to the init()
function (clk_prepare_enable() in this case), while the SH
implementation of clk_enable() considers that an error.

Fix this by making the SH clk_enable() implementation return zero
instead, like the Common Clock Framework does.

Reported-by: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
Exposed by commit 599566c1c3692052 ("r8169: use
devm_clk_get_optional_enabled() to simplify the code"), cfr.
https://lore.kernel.org/all/585c4b48790d71ca43b66fc24ea8d84917c4a0e1.camel@physik.fu-berlin.de

Boot-tested on qemu/rts7751r2d, which did not show the problem though.
---
 drivers/sh/clk/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/sh/clk/core.c b/drivers/sh/clk/core.c
index d996782a710642cd..7a73f5e4a1fc70cc 100644
--- a/drivers/sh/clk/core.c
+++ b/drivers/sh/clk/core.c
@@ -295,7 +295,7 @@ int clk_enable(struct clk *clk)
 	int ret;
 
 	if (!clk)
-		return -EINVAL;
+		return 0;
 
 	spin_lock_irqsave(&clock_lock, flags);
 	ret = __clk_enable(clk);
-- 
2.34.1

Re: [PATCH] sh: clk: Fix clk_enable() to return 0 on NULL clk

[John Paul Adrian Glaubitz]

Hi Geert!

On Thu, 2023-02-02 at 17:20 +0100, Geert Uytterhoeven wrote:
> On SH, devm_clk_get_optional_enabled() fails with -EINVAL if the clock
> is not found.  This happens because __devm_clk_get() assumes it can pass
> a NULL clock pointer (as returned by clk_get_optional()) to the init()
> function (clk_prepare_enable() in this case), while the SH
> implementation of clk_enable() considers that an error.
> 
> Fix this by making the SH clk_enable() implementation return zero
> instead, like the Common Clock Framework does.
> 
> Reported-by: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> Exposed by commit 599566c1c3692052 ("r8169: use
> devm_clk_get_optional_enabled() to simplify the code"), cfr.
> https://lore.kernel.org/all/585c4b48790d71ca43b66fc24ea8d84917c4a0e1.camel@physik.fu-berlin.de
> 
> Boot-tested on qemu/rts7751r2d, which did not show the problem though.
> ---
>  drivers/sh/clk/core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/sh/clk/core.c b/drivers/sh/clk/core.c
> index d996782a710642cd..7a73f5e4a1fc70cc 100644
> --- a/drivers/sh/clk/core.c
> +++ b/drivers/sh/clk/core.c
> @@ -295,7 +295,7 @@ int clk_enable(struct clk *clk)
>  	int ret;
>  
>  	if (!clk)
> -		return -EINVAL;
> +		return 0;
>  
>  	spin_lock_irqsave(&clock_lock, flags);
>  	ret = __clk_enable(clk);

I can confirm that this patch makes the r8169 driver work again!

Tested-by: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>

Adrian

-- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer
`. `'   Physicist
  `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913

Re: [PATCH] sh: clk: Fix clk_enable() to return 0 on NULL clk

[Stephen Boyd]

Quoting Geert Uytterhoeven (2023-02-02 08:20:55)
> On SH, devm_clk_get_optional_enabled() fails with -EINVAL if the clock
> is not found.  This happens because __devm_clk_get() assumes it can pass
> a NULL clock pointer (as returned by clk_get_optional()) to the init()
> function (clk_prepare_enable() in this case), while the SH
> implementation of clk_enable() considers that an error.
> 
> Fix this by making the SH clk_enable() implementation return zero
> instead, like the Common Clock Framework does.
> 
> Reported-by: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---

Acked-by: Stephen Boyd <sboyd@kernel.org>