[U-Boot] [PATCH] arm: ls1021a: Ensure LS1021 ARM Generic Timer CompareValue Set 64-bit

[Huan Wang]

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? How about removing the following code?

/* Set PL1 Physical Comp Value */
val = TIMER_COMP_VAL;
asm("mcrr p15, 2, %Q0, %R0, c14" : : "r" (val));

> > 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 at whiterocker.com>
> > > Signed-off-by: Alison Wang <alison.wang at 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 --git
> > > a/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 at lists.denx.de
> > > http://lists.denx.de/mailman/listinfo/u-boot
> > >