Re: [PATCH] clk: meson: pll: adjust timeout in meson_clk_pll_wait_lock()

[Jerome Brunet <jbrunet@baylibre.com>]

On Sun 14 Aug 2022 at 23:29, Heiner Kallweit <hkallweit1@gmail.com> wrote:

> Currently we loop over meson_parm_read() up to 24mln times.
> This results in a unpredictable timeout period. In my case
> it's over 5s on a S905X4-based system. Make the timeout
> period predictable and set it to 100ms.
>
> Whilst we're at it: All callers of this function return -EIO
> in case of failure, therefore we can return this value directly
> in the timeout case.

I'm okay with this change but I'd prefer if one change addressed a
single topic. Please split this out.

>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> ---
>  drivers/clk/meson/clk-pll.c | 15 ++++++---------
>  1 file changed, 6 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/clk/meson/clk-pll.c b/drivers/clk/meson/clk-pll.c
> index daa025b6d..d70bee331 100644
> --- a/drivers/clk/meson/clk-pll.c
> +++ b/drivers/clk/meson/clk-pll.c
> @@ -277,17 +277,17 @@ static int meson_clk_pll_wait_lock(struct clk_hw *hw)
>  {
>  	struct clk_regmap *clk = to_clk_regmap(hw);
>  	struct meson_clk_pll_data *pll = meson_clk_pll_data(clk);
> -	int delay = 24000000;
> +	int delay = 5000;
>  
>  	do {
> -		/* Is the clock locked now ? */
> +		/* Is the clock locked now ? Time out after 100ms. */
>  		if (meson_parm_read(clk->map, &pll->l))
>  			return 0;
>  
> -		delay--;
> -	} while (delay > 0);
> +		udelay(20);
> +	} while (--delay);
>  
> -	return -ETIMEDOUT;
> +	return -EIO;
>  }
>  
>  static int meson_clk_pll_init(struct clk_hw *hw)
> @@ -350,10 +350,7 @@ static int meson_clk_pll_enable(struct clk_hw *hw)
>  	/* Take the pll out reset */
>  	meson_parm_write(clk->map, &pll->rst, 0);
>  
> -	if (meson_clk_pll_wait_lock(hw))
> -		return -EIO;
> -
> -	return 0;
> +	return meson_clk_pll_wait_lock(hw);
>  }
>  
>  static void meson_clk_pll_disable(struct clk_hw *hw)


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic