From mboxrd@z Thu Jan 1 00:00:00 1970 From: Mark Rutland Date: Fri, 17 Jul 2015 11:26:18 +0100 Subject: [U-Boot] [PATCH] arm: ls1021a: Ensure LS1021 ARM Generic Timer CompareValue Set 64-bit In-Reply-To: References: <1436944385-28323-1-git-send-email-b18965@freescale.com> <20150715091426.GA9627@leverpostej> Message-ID: <20150717102528.GA26091@leverpostej> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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. > 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. 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 > > > > Signed-off-by: Alison Wang > > > > --- > > > > 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 > > > > >