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

[Heiner Kallweit <hkallweit1@gmail.com>]

On 29.08.2022 11:52, Jerome Brunet wrote:
> 
> 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.
> 
OK, I'll send a v2.

>>
>> 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