[U-Boot] [PATCH] arm: ls1021a: Ensure Generic Timer disabled before jumping into the OS

[U-Boot] [PATCH] arm: ls1021a: Ensure Generic Timer disabled before jumping into the OS

[Alison Wang]

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));
+}
-- 
2.1.0.27.g96db324

[U-Boot] [PATCH] arm: ls1021a: Ensure Generic Timer disabled before jumping into the OS

[York Sun]

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

[U-Boot] arm: ls1021a: Ensure Generic Timer disabled before jumping into the OS

[Alexander Stein]

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
-- 
Dipl.-Inf. Alexander Stein
SYS TEC electronic GmbH
alexander.stein at systec-electronic.com

Legal and Commercial Address:
Am Windrad 2
08468 Heinsdorfergrund
Germany

Office: +49 (0) 3765 38600-0
Fax:    +49 (0) 3765 38600-4100
 
Managing Directors:
	Director Technology/CEO: Dipl.-Phys. Siegmar Schmidt;
	Director Commercial Affairs/COO: Dipl. Ing. (FH) Armin von Collrepp
Commercial Registry:
	Amtsgericht Chemnitz, HRB 28082; USt.-Id Nr. DE150534010

[U-Boot] arm: ls1021a: Ensure Generic Timer disabled before jumping into the OS

[Huan Wang]

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

[U-Boot] arm: ls1021a: Ensure Generic Timer disabled before jumping into the OS

[Huan 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

[U-Boot] arm: ls1021a: Ensure Generic Timer disabled before jumping into the OS

[Alexander Stein]

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
-- 
Dipl.-Inf. Alexander Stein
SYS TEC electronic GmbH
alexander.stein at systec-electronic.com

Legal and Commercial Address:
Am Windrad 2
08468 Heinsdorfergrund
Germany

Office: +49 (0) 3765 38600-0
Fax:    +49 (0) 3765 38600-4100
 
Managing Directors:
	Director Technology/CEO: Dipl.-Phys. Siegmar Schmidt;
	Director Commercial Affairs/COO: Dipl. Ing. (FH) Armin von Collrepp
Commercial Registry:
	Amtsgericht Chemnitz, HRB 28082; USt.-Id Nr. DE150534010

[U-Boot] arm: ls1021a: Ensure Generic Timer disabled before jumping into the OS

[Mark Rutland]

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.

[U-Boot] [PATCH] arm: ls1021a: Ensure Generic Timer disabled before jumping into the OS

[York Sun]

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