Patchwork [U-Boot] OMAP[34]: fix broken timer

login
register
mail settings
Submitter John Rigby
Date Dec. 28, 2010, 12:33 a.m.
Message ID <1293496390-13372-1-git-send-email-john.rigby@linaro.org>
Download mbox | patch
Permalink /patch/76803/
State Accepted
Commit aadcfc179a5f9ef61c8201da9dff34f8394c1054
Delegated to: Albert ARIBAUD
Headers show

Comments

John Rigby - Dec. 28, 2010, 12:33 a.m.
As implemented now the timer used to implement __udelay counts
to 0xffffffff and then gets stuck there because the the programmed
reload value is 0xffffffff.  This value is not only wrong but
illegal according to the reference manual.

One can reproduce the bug by leaving a board at the u-boot prompt
for sometime then issuing a sleep command.  The sleep will hang
forever.

The timer is a count up timer that reloads as it rolls over
from 0xffffffff so the correct load value is 0.

Change TIMER_LOAD_VAL from 0xffffffff to 0 and introduce
a new constant called TIMER_OVERFLOW_VAL set to 0xffffffff.

Signed-off-by: John Rigby <john.rigby@linaro.org>
---
 arch/arm/cpu/armv7/omap-common/timer.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)
Igor Grinberg - June 28, 2011, 5:43 a.m.
Sandeep, Albert,


Please, apply this patch. It fixes a real issue with timer on omap.


Thanks


On 12/28/10 02:33, John Rigby wrote:

> As implemented now the timer used to implement __udelay counts
> to 0xffffffff and then gets stuck there because the the programmed
> reload value is 0xffffffff.  This value is not only wrong but
> illegal according to the reference manual.
>
> One can reproduce the bug by leaving a board at the u-boot prompt
> for sometime then issuing a sleep command.  The sleep will hang
> forever.
>
> The timer is a count up timer that reloads as it rolls over
> from 0xffffffff so the correct load value is 0.
>
> Change TIMER_LOAD_VAL from 0xffffffff to 0 and introduce
> a new constant called TIMER_OVERFLOW_VAL set to 0xffffffff.
>
> Signed-off-by: John Rigby <john.rigby@linaro.org>

Tested-by: Igor Grinberg <grinberg@compulab.co.il>

On DM3730, AM3703, OMAP3530.

> ---
>  arch/arm/cpu/armv7/omap-common/timer.c |    7 ++++---
>  1 files changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm/cpu/armv7/omap-common/timer.c b/arch/arm/cpu/armv7/omap-common/timer.c
> index 9beebb1..59bbca8 100644
> --- a/arch/arm/cpu/armv7/omap-common/timer.c
> +++ b/arch/arm/cpu/armv7/omap-common/timer.c
> @@ -43,8 +43,9 @@ static struct gptimer *timer_base = (struct gptimer *)CONFIG_SYS_TIMERBASE;
>   * Nothing really to do with interrupts, just starts up a counter.
>   */
>  
> -#define TIMER_CLOCK	(V_SCLK / (2 << CONFIG_SYS_PTV))
> -#define TIMER_LOAD_VAL	0xffffffff
> +#define TIMER_CLOCK		(V_SCLK / (2 << CONFIG_SYS_PTV))
> +#define TIMER_OVERFLOW_VAL	0xffffffff
> +#define TIMER_LOAD_VAL		0
>  
>  int timer_init(void)
>  {
> @@ -86,7 +87,7 @@ void __udelay(unsigned long usec)
>  	while (tmo > 0) {
>  		now = readl(&timer_base->tcrr);
>  		if (last > now) /* count up timer overflow */
> -			tmo -= TIMER_LOAD_VAL - last + now;
> +			tmo -= TIMER_OVERFLOW_VAL - last + now + 1;
>  		else
>  			tmo -= now - last;
>  		last = now;
Albert ARIBAUD - June 29, 2011, 6:35 a.m.
Hi John,

Le 28/12/2010 01:33, John Rigby a écrit :
> As implemented now the timer used to implement __udelay counts
> to 0xffffffff and then gets stuck there because the the programmed
> reload value is 0xffffffff.  This value is not only wrong but
> illegal according to the reference manual.
>
> One can reproduce the bug by leaving a board at the u-boot prompt
> for sometime then issuing a sleep command.  The sleep will hang
> forever.
>
> The timer is a count up timer that reloads as it rolls over
> from 0xffffffff so the correct load value is 0.
>
> Change TIMER_LOAD_VAL from 0xffffffff to 0 and introduce
> a new constant called TIMER_OVERFLOW_VAL set to 0xffffffff.
>
> Signed-off-by: John Rigby<john.rigby@linaro.org>

Applied to u-boot-arm/master, thanks.

Amicalement,

Patch

diff --git a/arch/arm/cpu/armv7/omap-common/timer.c b/arch/arm/cpu/armv7/omap-common/timer.c
index 9beebb1..59bbca8 100644
--- a/arch/arm/cpu/armv7/omap-common/timer.c
+++ b/arch/arm/cpu/armv7/omap-common/timer.c
@@ -43,8 +43,9 @@  static struct gptimer *timer_base = (struct gptimer *)CONFIG_SYS_TIMERBASE;
  * Nothing really to do with interrupts, just starts up a counter.
  */
 
-#define TIMER_CLOCK	(V_SCLK / (2 << CONFIG_SYS_PTV))
-#define TIMER_LOAD_VAL	0xffffffff
+#define TIMER_CLOCK		(V_SCLK / (2 << CONFIG_SYS_PTV))
+#define TIMER_OVERFLOW_VAL	0xffffffff
+#define TIMER_LOAD_VAL		0
 
 int timer_init(void)
 {
@@ -86,7 +87,7 @@  void __udelay(unsigned long usec)
 	while (tmo > 0) {
 		now = readl(&timer_base->tcrr);
 		if (last > now) /* count up timer overflow */
-			tmo -= TIMER_LOAD_VAL - last + now;
+			tmo -= TIMER_OVERFLOW_VAL - last + now + 1;
 		else
 			tmo -= now - last;
 		last = now;