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

Huan Wang
2015-07-15T09:34:53Z

Hi, Mark

> -----Original Message-----
> From: Mark Rutland [mailto:mark.rutland at arm.com]
> Sent: Wednesday, July 15, 2015 5:14 PM
> To: Wang Huan-B18965
> Cc: Sun York-R58495; u-boot at lists.denx.de; Wang Huan-B18965;
> marc.zyngier at arm.com
> Subject: Re: [U-Boot] [PATCH] arm: ls1021a: Ensure LS1021 ARM Generic
> Timer CompareValue Set 64-bit
>
> Hi,
>
> Isn't this the same patch as a couple of days ago [2], which I replied
> to [3]?
[Alison Wang] Sorry, I missed that patch. I thought Kilgour would not send the patch.

>
> 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.
>
> 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 --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
> >