Message ID | 1438653337-47307-1-git-send-email-b18965@freescale.com |
---|---|
State | Accepted |
Delegated to: | York Sun |
Headers | show |
On 08/03/2015 06:55 PM, 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. > > Apparently the bug is masked by some toolchains. Perhaps this is > explained by default compiler options, word sizes, or binutils versions. > > To fix the above issue, the generic physical timer is disabled > before jumping to the OS. > > [1] > https://lists.yoctoproject.org/pipermail/meta-freescale/2015-June/014400.html > > Signed-off-by: Chris Kilgour <techie@whiterocker.com> > Signed-off-by: Alison Wang <alison.wang@freescale.com> > --- > arch/arm/cpu/armv7/ls102xa/cpu.c | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/arch/arm/cpu/armv7/ls102xa/cpu.c b/arch/arm/cpu/armv7/ls102xa/cpu.c > index 75f0d8c..298422f 100644 > --- a/arch/arm/cpu/armv7/ls102xa/cpu.c > +++ b/arch/arm/cpu/armv7/ls102xa/cpu.c > @@ -346,3 +346,13 @@ void smp_kick_all_cpus(void) > out_be32(&gur->brrl, 0x2); > } > #endif > + > +void arch_preboot_os(void) > +{ > + unsigned long ctrl; > + > + /* Disable PL1 Physical Timer */ > + asm("mrc p15, 0, %0, c14, c2, 1" : "=r" (ctrl)); > + ctrl &= ~ARCH_TIMER_CTRL_ENABLE; > + asm("mcr p15, 0, %0, c14, c2, 1" : : "r" (ctrl)); > +} > Mark, Can you comment on this patch? I believe this is the follow-up after discussion here http://patchwork.ozlabs.org/patch/495476/. York
Hello Alison, On Tuesday 04 August 2015 09:55:37, 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. > > Apparently the bug is masked by some toolchains. Perhaps this is > explained by default compiler options, word sizes, or binutils versions. > > To fix the above issue, the generic physical timer is disabled > before jumping to the OS. > > [1] > https://lists.yoctoproject.org/pipermail/meta-freescale/2015-June/014400.html > > Signed-off-by: Chris Kilgour <techie@whiterocker.com> > Signed-off-by: Alison Wang <alison.wang@freescale.com> > --- > arch/arm/cpu/armv7/ls102xa/cpu.c | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/arch/arm/cpu/armv7/ls102xa/cpu.c b/arch/arm/cpu/armv7/ls102xa/cpu.c > index 75f0d8c..298422f 100644 > --- a/arch/arm/cpu/armv7/ls102xa/cpu.c > +++ b/arch/arm/cpu/armv7/ls102xa/cpu.c > @@ -346,3 +346,13 @@ void smp_kick_all_cpus(void) > out_be32(&gur->brrl, 0x2); > } > #endif > + > +void arch_preboot_os(void) > +{ > + unsigned long ctrl; > + > + /* Disable PL1 Physical Timer */ > + asm("mrc p15, 0, %0, c14, c2, 1" : "=r" (ctrl)); > + ctrl &= ~ARCH_TIMER_CTRL_ENABLE; > + asm("mcr p15, 0, %0, c14, c2, 1" : : "r" (ctrl)); > +} This only disables the timer when booting linux. Does the possibly misconfigured timer compare value (only lower 32-bits are set to 0xffffffff) have any side-effects within u-boot? I don't currently know the timer is used there. I would prefer that the inline assembly code in timer_init is fixed to pass an 64-bit variable to mcrr. Maybe someone will take that code snippet as an example and such problems occur again. Opinions? Best regards, Alexander
Hi, Alexander, > On Tuesday 04 August 2015 09:55:37, 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. > > > > Apparently the bug is masked by some toolchains. Perhaps this is > > explained by default compiler options, word sizes, or binutils > versions. > > > > To fix the above issue, the generic physical timer is disabled before > > jumping to the OS. > > > > [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/cpu.c | 10 ++++++++++ > > 1 file changed, 10 insertions(+) > > > > diff --git a/arch/arm/cpu/armv7/ls102xa/cpu.c > > b/arch/arm/cpu/armv7/ls102xa/cpu.c > > index 75f0d8c..298422f 100644 > > --- a/arch/arm/cpu/armv7/ls102xa/cpu.c > > +++ b/arch/arm/cpu/armv7/ls102xa/cpu.c > > @@ -346,3 +346,13 @@ void smp_kick_all_cpus(void) > > out_be32(&gur->brrl, 0x2); > > } > > #endif > > + > > +void arch_preboot_os(void) > > +{ > > + unsigned long ctrl; > > + > > + /* Disable PL1 Physical Timer */ > > + asm("mrc p15, 0, %0, c14, c2, 1" : "=r" (ctrl)); > > + ctrl &= ~ARCH_TIMER_CTRL_ENABLE; > > + asm("mcr p15, 0, %0, c14, c2, 1" : : "r" (ctrl)); } > > This only disables the timer when booting linux. Does the possibly > misconfigured timer compare value (only lower 32-bits are set to > 0xffffffff) have any side-effects within u-boot? I don't currently know > the timer is used there. > I would prefer that the inline assembly code in timer_init is fixed to > pass an 64-bit variable to mcrr. Maybe someone will take that code > snippet as an example and such problems occur again. Opinions? > [Alison Wang] Thanks for your reply. I agree with you that the misconfigured Timer compare value should be fixed too. So except this patch, the patch on http://patchwork.ozlabs.org/patch/495476/ need to be applied too. Best Regards, Alison Wang
Hi, Mark and Alexander, Do you have any comment about this patch? Thanks. Best Regards, Alison Wang > > On Tuesday 04 August 2015 09:55:37, 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. > > > > > > Apparently the bug is masked by some toolchains. Perhaps this is > > > explained by default compiler options, word sizes, or binutils > > versions. > > > > > > To fix the above issue, the generic physical timer is disabled > > > before jumping to the OS. > > > > > > [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/cpu.c | 10 ++++++++++ > > > 1 file changed, 10 insertions(+) > > > > > > diff --git a/arch/arm/cpu/armv7/ls102xa/cpu.c > > > b/arch/arm/cpu/armv7/ls102xa/cpu.c > > > index 75f0d8c..298422f 100644 > > > --- a/arch/arm/cpu/armv7/ls102xa/cpu.c > > > +++ b/arch/arm/cpu/armv7/ls102xa/cpu.c > > > @@ -346,3 +346,13 @@ void smp_kick_all_cpus(void) > > > out_be32(&gur->brrl, 0x2); > > > } > > > #endif > > > + > > > +void arch_preboot_os(void) > > > +{ > > > + unsigned long ctrl; > > > + > > > + /* Disable PL1 Physical Timer */ > > > + asm("mrc p15, 0, %0, c14, c2, 1" : "=r" (ctrl)); > > > + ctrl &= ~ARCH_TIMER_CTRL_ENABLE; > > > + asm("mcr p15, 0, %0, c14, c2, 1" : : "r" (ctrl)); } > > > > This only disables the timer when booting linux. Does the possibly > > misconfigured timer compare value (only lower 32-bits are set to > > 0xffffffff) have any side-effects within u-boot? I don't currently > > know the timer is used there. > > I would prefer that the inline assembly code in timer_init is fixed to > > pass an 64-bit variable to mcrr. Maybe someone will take that code > > snippet as an example and such problems occur again. Opinions? > > > > [Alison Wang] Thanks for your reply. I agree with you that the > misconfigured Timer compare value should be fixed too. So except this > patch, the patch on http://patchwork.ozlabs.org/patch/495476/ need to be > applied too. > > > Best Regards, > Alison Wang
Hi, On Tuesday 27 October 2015 02:52:43, Huan Wang wrote: > Do you have any comment about this patch? > Thanks. > > > On Tuesday 04 August 2015 09:55:37, 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. > > > > > > > > Apparently the bug is masked by some toolchains. Perhaps this is > > > > explained by default compiler options, word sizes, or binutils > > > versions. > > > > > > > > To fix the above issue, the generic physical timer is disabled > > > > before jumping to the OS. > > > > > > > > [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/cpu.c | 10 ++++++++++ > > > > 1 file changed, 10 insertions(+) > > > > > > > > diff --git a/arch/arm/cpu/armv7/ls102xa/cpu.c > > > > b/arch/arm/cpu/armv7/ls102xa/cpu.c > > > > index 75f0d8c..298422f 100644 > > > > --- a/arch/arm/cpu/armv7/ls102xa/cpu.c > > > > +++ b/arch/arm/cpu/armv7/ls102xa/cpu.c > > > > @@ -346,3 +346,13 @@ void smp_kick_all_cpus(void) > > > > out_be32(&gur->brrl, 0x2); > > > > } > > > > #endif > > > > + > > > > +void arch_preboot_os(void) > > > > +{ > > > > + unsigned long ctrl; > > > > + > > > > + /* Disable PL1 Physical Timer */ > > > > + asm("mrc p15, 0, %0, c14, c2, 1" : "=r" (ctrl)); > > > > + ctrl &= ~ARCH_TIMER_CTRL_ENABLE; > > > > + asm("mcr p15, 0, %0, c14, c2, 1" : : "r" (ctrl)); } > > > > > > This only disables the timer when booting linux. Does the possibly > > > misconfigured timer compare value (only lower 32-bits are set to > > > 0xffffffff) have any side-effects within u-boot? I don't currently > > > know the timer is used there. > > > I would prefer that the inline assembly code in timer_init is fixed to > > > pass an 64-bit variable to mcrr. Maybe someone will take that code > > > snippet as an example and such problems occur again. Opinions? > > > > > > > [Alison Wang] Thanks for your reply. I agree with you that the > > misconfigured Timer compare value should be fixed too. So except this > > patch, the patch on http://patchwork.ozlabs.org/patch/495476/ need to be > > applied too. Same opinion here. Both patches should be applied. Best regards, Alexander
On Tue, Oct 27, 2015 at 02:52:43AM +0000, Huan Wang wrote: > Hi, Mark and Alexander, > > Do you have any comment about this patch? > Thanks. The patch looks good to me. FWIW: Acked-by: Mark Rutland <mark.rutland@arm.com> Mark.
On 08/03/2015 06:55 PM, 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. > > Apparently the bug is masked by some toolchains. Perhaps this is > explained by default compiler options, word sizes, or binutils versions. > > To fix the above issue, the generic physical timer is disabled > before jumping to the OS. > > [1] > https://lists.yoctoproject.org/pipermail/meta-freescale/2015-June/014400.html > > Signed-off-by: Chris Kilgour <techie@whiterocker.com> > Signed-off-by: Alison Wang <alison.wang@freescale.com> > --- Applied to fsl-qoriq master. Thanks. York
diff --git a/arch/arm/cpu/armv7/ls102xa/cpu.c b/arch/arm/cpu/armv7/ls102xa/cpu.c index 75f0d8c..298422f 100644 --- a/arch/arm/cpu/armv7/ls102xa/cpu.c +++ b/arch/arm/cpu/armv7/ls102xa/cpu.c @@ -346,3 +346,13 @@ void smp_kick_all_cpus(void) out_be32(&gur->brrl, 0x2); } #endif + +void arch_preboot_os(void) +{ + unsigned long ctrl; + + /* Disable PL1 Physical Timer */ + asm("mrc p15, 0, %0, c14, c2, 1" : "=r" (ctrl)); + ctrl &= ~ARCH_TIMER_CTRL_ENABLE; + asm("mcr p15, 0, %0, c14, c2, 1" : : "r" (ctrl)); +}