Re: [PATCH v6 04/18] clk: qcom: clk-hfpll: use poll_timeout macro

[Stephen Boyd <sboyd@kernel.org>]

Quoting Ansuel Smith (2022-03-21 16:15:34)
> Use regmap_read_poll_timeout macro instead of do-while structure.

Please add a note that this isn't an equivalent translation. Now we
timeout if the PLL fails to lock whereas before we would loop
potentially forever.

> diff --git a/drivers/clk/qcom/clk-hfpll.c b/drivers/clk/qcom/clk-hfpll.c
> index e847d586a73a..a4e347eb4d4d 100644
> --- a/drivers/clk/qcom/clk-hfpll.c
> +++ b/drivers/clk/qcom/clk-hfpll.c
> @@ -12,6 +12,8 @@
>  #include "clk-regmap.h"
>  #include "clk-hfpll.h"
>  
> +#define HFPLL_BUSY_WAIT_TIMEOUT        100

We don't really need a define for this.

> +
>  #define PLL_OUTCTRL    BIT(0)
>  #define PLL_BYPASSNL   BIT(1)
>  #define PLL_RESET_N    BIT(2)
> @@ -72,13 +74,12 @@ static void __clk_hfpll_enable(struct clk_hw *hw)
>         regmap_update_bits(regmap, hd->mode_reg, PLL_RESET_N, PLL_RESET_N);
>  
>         /* Wait for PLL to lock. */
> -       if (hd->status_reg) {
> -               do {
> -                       regmap_read(regmap, hd->status_reg, &val);
> -               } while (!(val & BIT(hd->lock_bit)));
> -       } else {
> +       if (hd->status_reg)
> +               regmap_read_poll_timeout(regmap, hd->status_reg, val,
> +                                        !(val & BIT(hd->lock_bit)), USEC_PER_MSEC * 2,

Why are we waiting between reads?

> +                                        HFPLL_BUSY_WAIT_TIMEOUT * USEC_PER_MSEC);
> +       else
>                 udelay(60);
> -       }
>