[PATCH] clk: Fix phase init check

[PATCH] clk: Fix phase init check

[Maxime Ripard]

Commit 2760878662a2 ("clk: Bail out when calculating phase fails during clk
registration") introduced a check on error values at the time the clock is
registered to bail out when such an error occurs.

However, it doesn't check whether the returned value is positive which will
happen if the driver returns a non-zero phase, and ends up returning that
to the caller on success, breaking the API that implies that the driver
should return 0 on success.

Fixes: 2760878662a2 ("clk: Bail out when calculating phase fails during clk registration")
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
---
 drivers/clk/clk.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index ebfc1e2103cb..f122e9911b57 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -3344,6 +3344,7 @@ static int __clk_core_init(struct clk_core *core)
 	int ret;
 	struct clk_core *parent;
 	unsigned long rate;
+	int phase;
 
 	if (!core)
 		return -EINVAL;
@@ -3457,8 +3458,9 @@ static int __clk_core_init(struct clk_core *core)
 	 * Since a phase is by definition relative to its parent, just
 	 * query the current clock phase, or just assume it's in phase.
 	 */
-	ret = clk_core_get_phase(core);
-	if (ret < 0) {
+	phase = clk_core_get_phase(core);
+	if (phase < 0) {
+		ret = phase;
 		pr_warn("%s: Failed to get phase for clk '%s'\n", __func__,
 			core->name);
 		goto out;
-- 
2.24.1

Re: [PATCH] clk: Fix phase init check

[Stephen Boyd]

Quoting Maxime Ripard (2020-02-25 05:42:48)
> Commit 2760878662a2 ("clk: Bail out when calculating phase fails during clk
> registration") introduced a check on error values at the time the clock is
> registered to bail out when such an error occurs.
> 
> However, it doesn't check whether the returned value is positive which will
> happen if the driver returns a non-zero phase, and ends up returning that
> to the caller on success, breaking the API that implies that the driver
> should return 0 on success.

I had to read this a few times to understand. I'll reword it to indicate
that __clk_core_init() should return 0 on success and not the phase
which is typically a positive value.

> 
> Fixes: 2760878662a2 ("clk: Bail out when calculating phase fails during clk registration")
> Signed-off-by: Maxime Ripard <maxime@cerno.tech>

Thanks. I think we need a 

Reported-by: "kernelci.org bot" <bot@kernelci.org>

tag added as well.

> ---
>  drivers/clk/clk.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index ebfc1e2103cb..f122e9911b57 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -3344,6 +3344,7 @@ static int __clk_core_init(struct clk_core *core)
>         int ret;
>         struct clk_core *parent;
>         unsigned long rate;
> +       int phase;
>  
>         if (!core)
>                 return -EINVAL;
> @@ -3457,8 +3458,9 @@ static int __clk_core_init(struct clk_core *core)
>          * Since a phase is by definition relative to its parent, just
>          * query the current clock phase, or just assume it's in phase.
>          */
> -       ret = clk_core_get_phase(core);
> -       if (ret < 0) {
> +       phase = clk_core_get_phase(core);
> +       if (phase < 0) {
> +               ret = phase;
>                 pr_warn("%s: Failed to get phase for clk '%s'\n", __func__,
>                         core->name);
>                 goto out;

Re: [PATCH] clk: Fix phase init check

[Stephen Boyd]

Quoting Maxime Ripard (2020-02-25 05:42:48)
> Commit 2760878662a2 ("clk: Bail out when calculating phase fails during clk
> registration") introduced a check on error values at the time the clock is
> registered to bail out when such an error occurs.
> 
> However, it doesn't check whether the returned value is positive which will
> happen if the driver returns a non-zero phase, and ends up returning that
> to the caller on success, breaking the API that implies that the driver
> should return 0 on success.
> 
> Fixes: 2760878662a2 ("clk: Bail out when calculating phase fails during clk registration")
> Signed-off-by: Maxime Ripard <maxime@cerno.tech>
> ---

Applied to clk-next