[PATCH 1/1] ARM: imx: clk-pllv3: change wait method for PLL lock

[u.kleine-koenig@pengutronix.de (Uwe Kleine-König)]

Hello

[added jstultz to Cc:]

On Thu, Jun 06, 2013 at 02:55:26PM +0800, Peter Chen wrote:
> For tickless system, the jiffies may be updated long time (>20ms).
... may not be updated for a long time ... ?

> At high loading system, the current waiting method will cause waiting
> timeout, and cause a kernel dump at below case.
> After timeout = jiffies + msecs_to_jiffies(10),
> the timer interrupt occurs, it updates jiffies (eg,  + 2 jiffies),
> then return back from interrupt, the time between above operations
> are tiny, the PLL is still not locked, but the timeout condition is satisfied.
Hmm, I admit I didn't follow the tickless stuff, but still I wonder if
the analysis is right. I thought on tickless jiffies are updated as
before by the boot cpu that cannot run in tickless mode?

Anyhow, this only affects the commit log, not the problem.
 
> Signed-off-by: Peter Chen <peter.chen@freescale.com>
> ---
>  arch/arm/mach-imx/clk-pllv3.c |    9 ++++++---
>  1 files changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm/mach-imx/clk-pllv3.c b/arch/arm/mach-imx/clk-pllv3.c
> index 36aac94..eefc6c2 100644
> --- a/arch/arm/mach-imx/clk-pllv3.c
> +++ b/arch/arm/mach-imx/clk-pllv3.c
> @@ -16,6 +16,7 @@
>  #include <linux/slab.h>
>  #include <linux/jiffies.h>
>  #include <linux/err.h>
> +#include <linux/delay.h>
>  #include "clk.h"
>  
>  #define PLL_NUM_OFFSET		0x10
> @@ -50,7 +51,7 @@ struct clk_pllv3 {
>  static int clk_pllv3_prepare(struct clk_hw *hw)
>  {
>  	struct clk_pllv3 *pll = to_clk_pllv3(hw);
> -	unsigned long timeout = jiffies + msecs_to_jiffies(10);
> +	int count = 100;
>  	u32 val;
>  
>  	val = readl_relaxed(pll->base);
> @@ -62,9 +63,11 @@ static int clk_pllv3_prepare(struct clk_hw *hw)
>  	writel_relaxed(val, pll->base);
>  
>  	/* Wait for PLL to lock */
> -	while (!(readl_relaxed(pll->base) & BM_PLL_LOCK))
> -		if (time_after(jiffies, timeout))
> +	while (!(readl_relaxed(pll->base) & BM_PLL_LOCK)) {
> +		udelay(100);
> +		if (--count == 0)
>  			return -ETIMEDOUT;
> +	}
Maybe it's enough to do timeout = jiffies + msecs_to_jiffies(10); just
after the pll is reprogrammed? i.e. 

diff --git a/arch/arm/mach-imx/clk-pllv3.c b/arch/arm/mach-imx/clk-pllv3.c
index d09bc3d..37f734e 100644
--- a/arch/arm/mach-imx/clk-pllv3.c
+++ b/arch/arm/mach-imx/clk-pllv3.c
@@ -48,7 +48,7 @@ struct clk_pllv3 {
 static int clk_pllv3_prepare(struct clk_hw *hw)
 {
 	struct clk_pllv3 *pll = to_clk_pllv3(hw);
-	unsigned long timeout = jiffies + msecs_to_jiffies(10);
+	unsigned long timeout;
 	u32 val;
 
 	val = readl_relaxed(pll->base);
@@ -59,6 +59,8 @@ static int clk_pllv3_prepare(struct clk_hw *hw)
 		val &= ~BM_PLL_POWER;
 	writel_relaxed(val, pll->base);
 
+	timeout = jiffies + msecs_to_jiffies(10);
+
 	/* Wait for PLL to lock */
 	while (!(readl_relaxed(pll->base) & BM_PLL_LOCK))
 		if (time_after(jiffies, timeout))

Then at least the pll tries to look while the process is interrupted.

What is msecs_to_jiffies(10) for you? John, would you expect the problem
here that Peter describes?

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |