diff mbox series

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

Message ID a801afc0-a8f2-a0a4-0f2b-a7201351d563@gmail.com (mailing list archive)
State Awaiting Upstream, archived
Headers show
Series [v2] clk: meson: pll: adjust timeout in meson_clk_pll_wait_lock() | expand

Commit Message

Heiner Kallweit Aug. 29, 2022, 6:52 p.m. UTC
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.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
v2:
- remove an unrelated change
---
 drivers/clk/meson/clk-pll.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

Comments

Jerome Brunet Sept. 6, 2022, 3:29 p.m. UTC | #1
On Mon 29 Aug 2022 at 20:52, 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.

Nitpick: next time please try to make commit not about your system, but
about the system and the codebase.

For example: In my case -> on an S905X4 platform (even better if you
precise the platform here). Thx.

>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>

Applied, Thx
Heiner Kallweit Oct. 22, 2022, 11:17 a.m. UTC | #2
On 29.08.2022 20:52, Heiner Kallweit 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.
> 
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> ---
> v2:
> - remove an unrelated change
> ---
>  drivers/clk/meson/clk-pll.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/clk/meson/clk-pll.c b/drivers/clk/meson/clk-pll.c
> index daa025b6d..53b8e17e4 100644
> --- a/drivers/clk/meson/clk-pll.c
> +++ b/drivers/clk/meson/clk-pll.c
> @@ -277,15 +277,15 @@ 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;
>  }

Hi Jerome,
on Sep 6th you responded that the patch has been applied.
https://patchwork.kernel.org/project/linux-amlogic/patch/a801afc0-a8f2-a0a4-0f2b-a7201351d563@gmail.com/
However I still don't see it in linux-next. Is it pending in some other tree?

Heiner
Martin Blumenstingl Oct. 22, 2022, 11:21 a.m. UTC | #3
Hi Heiner,

On Sat, Oct 22, 2022 at 1:17 PM Heiner Kallweit <hkallweit1@gmail.com> wrote:
[...]
> Hi Jerome,
> on Sep 6th you responded that the patch has been applied.
> https://patchwork.kernel.org/project/linux-amlogic/patch/a801afc0-a8f2-a0a4-0f2b-a7201351d563@gmail.com/
> However I still don't see it in linux-next. Is it pending in some other tree?
Jerome has merged both your patches and they are part of the linux-clk
pull request, see [0]
But it seems like that hasn't been merged by the clock maintainers
(Stephen) yet.

Do you have that other mail in your inbox? If so, can you please ping
Stephen in that thread?


Best regards,
Martin


[0] https://lore.kernel.org/linux-amlogic/1jh71b3asq.fsf@starbuckisacylon.baylibre.com/
Jerome Brunet Oct. 22, 2022, 12:10 p.m. UTC | #4
On Sat 22 Oct 2022 at 13:21, Martin Blumenstingl <martin.blumenstingl@googlemail.com> wrote:

> Hi Heiner,
>
> On Sat, Oct 22, 2022 at 1:17 PM Heiner Kallweit <hkallweit1@gmail.com> wrote:
> [...]
>> Hi Jerome,
>> on Sep 6th you responded that the patch has been applied.
>> https://patchwork.kernel.org/project/linux-amlogic/patch/a801afc0-a8f2-a0a4-0f2b-a7201351d563@gmail.com/
>> However I still don't see it in linux-next. Is it pending in some other tree?
> Jerome has merged both your patches and they are part of the linux-clk
> pull request, see [0]
> But it seems like that hasn't been merged by the clock maintainers
> (Stephen) yet.
>
> Do you have that other mail in your inbox? If so, can you please ping
> Stephen in that thread?
>

Indeed, It missed the merged window. I'll send it again soon

>
> Best regards,
> Martin
>
>
> [0] https://lore.kernel.org/linux-amlogic/1jh71b3asq.fsf@starbuckisacylon.baylibre.com/
diff mbox series

Patch

diff --git a/drivers/clk/meson/clk-pll.c b/drivers/clk/meson/clk-pll.c
index daa025b6d..53b8e17e4 100644
--- a/drivers/clk/meson/clk-pll.c
+++ b/drivers/clk/meson/clk-pll.c
@@ -277,15 +277,15 @@  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;
 }