Hi, Mark,
On Fri, Jul 17, 2015 at 11:01:01AM +0100, Huan Wang wrote:
Hi, Mark,
On Wed, Jul 15, 2015 at 08:13:05AM +0100, Alison Wang wrote:
This patch addresses a problem mentioned recently on this mailing
list:
[1].
In that posting a LS1021 based system was locking up at about 5 minutes after boot, but the problem was mysteriously related to the toolchain used for building u-boot. Debugging the problem reveals
a
stuck interrupt 29 on the GIC.
It appears Freescale's LS1021 support in u-boot erroneously sets
the
64-bit ARM generic PL1 physical time CompareValue register to all-
ones
with a 32-bit value. This causes the timer compare to fire 344 seconds after u-boot configures it. Depending on how fast u-boot
gets
the kernel booted, this amounts to about 5-minutes of Linux uptime before locking up.
If as in [2] this is an attempt to not generate interrupts that Linux doesn't expect, it would be far better to simply disable the timer interrupt before leaving U-Boot, ensuring that unexpected interrupts are never generated regardless of the width or rate of the counter.
There are bits in CNTP_CTL to do this.
[Alison Wang] Yes, your idea is far better.
[Alison Wang] If the CompareValue register is not written, is there any unexpected Interrupt?
If you don't write to CNTP_CVAL, you have the exact same problem. The only difference is that CNTP_CVAL contains an UNKNOWN value, and so the interrutp could trigger at any point in time. [Alison Wang] Thanks for reminding me. It's terrible if the interrupt could trigger at any point in time.
How about removing the following code?
/* Set PL1 Physical Comp Value */ val = TIMER_COMP_VAL; asm("mcrr p15, 2, %Q0, %R0, c14" : : "r" (val));
To stop the interrupt from firing at all you can clear CNTP_CTL.ENABLE, which will disable the comparator. You could instead set CNTP_CTL.IMASK, but I think clearing ENABLE is preferable because you might also save power. [Alison Wang] Yes, I think so. Thanks.
Thanks, Mark.
Thanks, Mark.
[2] http://lists.denx.de/pipermail/u-boot/2015-July/218937.html [3] http://lists.denx.de/pipermail/u-boot/2015-July/218979.html
Apparently the bug is masked by some toolchains. Perhaps this is explained by default compiler options, word sizes, or binutils
versions.
At any rate this patch makes the manipulation explicitly 64-bit which alleviates the issue.
[1] https://lists.yoctoproject.org/pipermail/meta-freescale/2015-
June/0144
00.html Signed-off-by: Chris Kilgour techie@whiterocker.com Signed-off-by: Alison Wang alison.wang@freescale.com
arch/arm/cpu/armv7/ls102xa/timer.c | 3 ++- arch/arm/include/asm/arch-ls102xa/immap_ls102xa.h | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/arch/arm/cpu/armv7/ls102xa/timer.c b/arch/arm/cpu/armv7/ls102xa/timer.c index 11b17b2..e6a32ca 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 val; /* Enable System Counter */ writel(SYS_COUNTER_CTRL_ENABLE, &sctr->cntcr); diff --gita/arch/arm/include/asm/arch-ls102xa/immap_ls102xa.h b/arch/arm/include/asm/arch-ls102xa/immap_ls102xa.h index ee547fb..34854da 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 0xffffffffffffffffull #define ARCH_TIMER_CTRL_ENABLE (1 << 0) #define SYS_COUNTER_CTRL_ENABLE (1 << 24)
-- 2.1.0.27.g96db324
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot