[U-Boot] [PATCH] Fix LS102xa timer setup to use 64-bit.

[U-Boot] [PATCH] Fix LS102xa timer setup to use 64-bit.

[Christopher Kilgour]

Fix LS102xa timer configuration to ensure timer compare value is set to
all-ones as a 64-bit number rather than a 32-bit number.

When the 32-bit all-ones was used, this could result in a timer compare value
of 2^32-1, which at 12.5 MHz will fire in ~344 seconds.  If the operating
system's timer support does not expect or handle the timer compare, it can
lock up when the timer compare fires and never clears (in Linux this shows up
as a stuck GIC 29).

It's also possible to interactively work with u-boot for longer than 344
seconds, and have the timer compare silently fire in the background without
impact on u-boot operation.  However, as soon as the operating system enables
interrupts, it can lock up.  On embedded Linux without early console support,
this can be a silent lockup without warning or diagnostic output.

It's likely Freescale wanted to set the timer compare to the largest possible
value of all-ones at 64-bits.  Rather than 344 seconds, this would fire after
~47k years.  Even though Linux systems are known for long uptimes, one assumes
this is Freescale's intended, safe value.

Signed-off-by: Christopher Kilgour <techie@whiterocker.com>
Cc: York Sun <yorksun@freescale.com>
---
 arch/arm/cpu/armv7/ls102xa/timer.c                | 7 ++++---
 arch/arm/include/asm/arch-ls102xa/immap_ls102xa.h | 2 +-
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/arch/arm/cpu/armv7/ls102xa/timer.c b/arch/arm/cpu/armv7/ls102xa/timer.c
index 11b17b2..746bfc0 100644
--- a/arch/arm/cpu/armv7/ls102xa/timer.c
+++ b/arch/arm/cpu/armv7/ls102xa/timer.c
@@ -56,7 +56,8 @@ static inline unsigned long long us_to_tick(unsigned long long usec)
 int timer_init(void)
 {
 	struct sctr_regs *sctr = (struct sctr_regs *)SCTR_BASE_ADDR;
-	unsigned long ctrl, val, freq;
+	unsigned long ctrl, freq;
+	unsigned long long val64;
 
 	/* Enable System Counter */
 	writel(SYS_COUNTER_CTRL_ENABLE, &sctr->cntcr);
@@ -69,8 +70,8 @@ int timer_init(void)
 	asm("mcr p15, 0, %0, c14, c2, 1" : : "r" (ctrl));
 
 	/* Set PL1 Physical Comp Value */
-	val = TIMER_COMP_VAL;
-	asm("mcrr p15, 2, %Q0, %R0, c14" : : "r" (val));
+	val64 = TIMER_COMP_VAL;
+	asm("mcrr p15, 2, %Q0, %R0, c14" : : "r" (val64));
 
 	gd->arch.tbl = 0;
 	gd->arch.tbu = 0;
diff --git a/arch/arm/include/asm/arch-ls102xa/immap_ls102xa.h b/arch/arm/include/asm/arch-ls102xa/immap_ls102xa.h
index ee547fb..1f55655 100644
--- a/arch/arm/include/asm/arch-ls102xa/immap_ls102xa.h
+++ b/arch/arm/include/asm/arch-ls102xa/immap_ls102xa.h
@@ -31,7 +31,7 @@
 #define RCWSR4_SRDS1_PRTCL_SHIFT	24
 #define RCWSR4_SRDS1_PRTCL_MASK		0xff000000
 
-#define TIMER_COMP_VAL			0xffffffff
+#define TIMER_COMP_VAL			((unsigned long long)(-1))
 #define ARCH_TIMER_CTRL_ENABLE		(1 << 0)
 #define SYS_COUNTER_CTRL_ENABLE		(1 << 24)
 
-- 
2.1.0

[U-Boot] [PATCH] Fix LS102xa timer setup to use 64-bit.

[Mark Rutland]

On Sun, Jul 12, 2015 at 05:05:34AM +0100, Christopher Kilgour wrote:
> Fix LS102xa timer configuration to ensure timer compare value is set to
> all-ones as a 64-bit number rather than a 32-bit number.
> 
> When the 32-bit all-ones was used, this could result in a timer compare value
> of 2^32-1, which at 12.5 MHz will fire in ~344 seconds.  If the operating
> system's timer support does not expect or handle the timer compare, it can
> lock up when the timer compare fires and never clears (in Linux this shows up
> as a stuck GIC 29).
> 
> It's also possible to interactively work with u-boot for longer than 344
> seconds, and have the timer compare silently fire in the background without
> impact on u-boot operation.  However, as soon as the operating system enables
> interrupts, it can lock up.  On embedded Linux without early console support,
> this can be a silent lockup without warning or diagnostic output.
> 
> It's likely Freescale wanted to set the timer compare to the largest possible
> value of all-ones at 64-bits.  Rather than 344 seconds, this would fire after
> ~47k years.  Even though Linux systems are known for long uptimes, one assumes
> this is Freescale's intended, safe value.

It would probably be better to set CNTP_CTL.IMASK, or clear
CNTP_CTL.ENABLE. That way no interrupt will be generated regardless of
the timer frequency, even in the case of a long uptime ;)

Thanks,
Mark.

> 
> Signed-off-by: Christopher Kilgour <techie@whiterocker.com>
> Cc: York Sun <yorksun@freescale.com>
> ---
>  arch/arm/cpu/armv7/ls102xa/timer.c                | 7 ++++---
>  arch/arm/include/asm/arch-ls102xa/immap_ls102xa.h | 2 +-
>  2 files changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm/cpu/armv7/ls102xa/timer.c b/arch/arm/cpu/armv7/ls102xa/timer.c
> index 11b17b2..746bfc0 100644
> --- a/arch/arm/cpu/armv7/ls102xa/timer.c
> +++ b/arch/arm/cpu/armv7/ls102xa/timer.c
> @@ -56,7 +56,8 @@ static inline unsigned long long us_to_tick(unsigned long long usec)
>  int timer_init(void)
>  {
>  	struct sctr_regs *sctr = (struct sctr_regs *)SCTR_BASE_ADDR;
> -	unsigned long ctrl, val, freq;
> +	unsigned long ctrl, freq;
> +	unsigned long long val64;
>  
>  	/* Enable System Counter */
>  	writel(SYS_COUNTER_CTRL_ENABLE, &sctr->cntcr);
> @@ -69,8 +70,8 @@ int timer_init(void)
>  	asm("mcr p15, 0, %0, c14, c2, 1" : : "r" (ctrl));
>  
>  	/* Set PL1 Physical Comp Value */
> -	val = TIMER_COMP_VAL;
> -	asm("mcrr p15, 2, %Q0, %R0, c14" : : "r" (val));
> +	val64 = TIMER_COMP_VAL;
> +	asm("mcrr p15, 2, %Q0, %R0, c14" : : "r" (val64));
>  
>  	gd->arch.tbl = 0;
>  	gd->arch.tbu = 0;
> diff --git a/arch/arm/include/asm/arch-ls102xa/immap_ls102xa.h b/arch/arm/include/asm/arch-ls102xa/immap_ls102xa.h
> index ee547fb..1f55655 100644
> --- a/arch/arm/include/asm/arch-ls102xa/immap_ls102xa.h
> +++ b/arch/arm/include/asm/arch-ls102xa/immap_ls102xa.h
> @@ -31,7 +31,7 @@
>  #define RCWSR4_SRDS1_PRTCL_SHIFT	24
>  #define RCWSR4_SRDS1_PRTCL_MASK		0xff000000
>  
> -#define TIMER_COMP_VAL			0xffffffff
> +#define TIMER_COMP_VAL			((unsigned long long)(-1))
>  #define ARCH_TIMER_CTRL_ENABLE		(1 << 0)
>  #define SYS_COUNTER_CTRL_ENABLE		(1 << 24)
>  
> -- 
> 2.1.0
> 
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
>