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

Stephen Boyd
2022-03-25T01:09:14Z

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);
> - }
>