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

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

2015-11-30 Thread York Sun



On 07/15/2015 12:13 AM, 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.
> 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/014400.html
> 
> Signed-off-by: Chris Kilgour 
> Signed-off-by: Alison Wang 
> ---

Applied to fsl-qoriq master. Thanks.

York



___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

2015-07-17 Thread Huan Wang

Hi, Marc,

 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));

There is two aspects to it:

- if you're not using the timer at all, there is no point writing to the
comparator.

- but whether you're using it or not, it is good practice to turn it off
before jumping into the OS: this OS may run non-secure and will then be
unable to turn the secure timer off.

[Alison Wang] Yes, I understand. Thanks for your explanation.

Thanks,

M.
--
Jazz is not dead. It just smells funny...
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

2015-07-17 Thread Huan Wang

Hi, Mark,

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.
[Alison Wang] Thanks for reminding me. It's terrible if the interrupt 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.
[Alison Wang] Yes, I think so. 
Thanks.

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 tec...@whiterocker.com
Signed-off-by: Alison Wang alison.w...@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_MASK0xff00
   
-#define TIMER_COMP_VAL 0x
+#define TIMER_COMP_VAL 0xull
 #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@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
   

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

2015-07-17 Thread 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 tec...@whiterocker.com
   Signed-off-by: Alison Wang alison.w...@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  0xff00
  
   -#define TIMER_COMP_VAL   0x
   +#define TIMER_COMP_VAL   0xull
#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@lists.denx.de
   http://lists.denx.de/mailman/listinfo/u-boot
  
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

2015-07-17 Thread Marc Zyngier

Alison,

On 17/07/15 11:01, 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? How about removing the following code?
 
 /* Set PL1 Physical Comp Value */
 val = TIMER_COMP_VAL;
 asm(mcrr p15, 2, %Q0, %R0, c14 : : r (val));

There is two aspects to it:

- if you're not using the timer at all, there is no point writing to the
comparator.

- but whether you're using it or not, it is good practice to turn it off
before jumping into the OS: this OS may run non-secure and will then be
unable to turn the secure timer off.

Thanks,

M.
-- 
Jazz is not dead. It just smells funny...
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

2015-07-17 Thread Mark Rutland

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 tec...@whiterocker.com
Signed-off-by: Alison Wang alison.w...@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_MASK0xff00
   
-#define TIMER_COMP_VAL 0x
+#define TIMER_COMP_VAL 0xull
 #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@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
   
 
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

2015-07-15 Thread Huan Wang

Hi, Mark

 -Original Message-
 From: Mark Rutland [mailto:mark.rutl...@arm.com]
 Sent: Wednesday, July 15, 2015 5:14 PM
 To: Wang Huan-B18965
 Cc: Sun York-R58495; u-boot@lists.denx.de; Wang Huan-B18965;
 marc.zyng...@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 tec...@whiterocker.com
  Signed-off-by: Alison Wang alison.w...@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_MASK0xff00

  -#define TIMER_COMP_VAL 0x
  +#define TIMER_COMP_VAL 0xull
   #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@lists.denx.de
  http://lists.denx.de/mailman/listinfo/u-boot

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

2015-07-15 Thread Mark Rutland

Hi,

Isn't this the same patch as a couple of days ago [2], which I replied
to [3]?

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.

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/014400.html
 Signed-off-by: Chris Kilgour tec...@whiterocker.com
 Signed-off-by: Alison Wang alison.w...@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  0xff00
  
 -#define TIMER_COMP_VAL   0x
 +#define TIMER_COMP_VAL   0xull
  #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@lists.denx.de
 http://lists.denx.de/mailman/listinfo/u-boot
 
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

2015-07-15 Thread 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.
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/014400.html

Signed-off-by: Chris Kilgour tec...@whiterocker.com
Signed-off-by: Alison Wang alison.w...@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_MASK0xff00
 
-#define TIMER_COMP_VAL 0x
+#define TIMER_COMP_VAL 0xull
 #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@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

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

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

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

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

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

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

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

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

9 matches

Site Navigation

Mail list logo

Footer information