Without the volatile attribute, compilers are entitled to optimize out
the same asm().  In the case of __udelay() in syscounter.c, it calls
`get_ticks()` twice, one for the starting time and the second in the
loop to check the current time.  When compilers inline `get_ticks()`
they see the same `mrrc` instructions and optimize out the second one.
This leads to infinite loop since we don't get updated value from the
system counter.

Here is a portion of the disassembly of __udelay:

  88:   428b            cmp     r3, r1
  8a:   f8ce 20a4       str.w   r2, [lr, #164]  ; 0xa4
  8e:   bf08            it      eq
  90:   4282            cmpeq   r2, r0
  92:   f8ce 30a0       str.w   r3, [lr, #160]  ; 0xa0
  96:   d3f7            bcc.n   88 <__udelay+0x88>
  98:   e8bd 8cf0       ldmia.w sp!, {r4, r5, r6, r7, sl, fp, pc}

Note that final jump / loop at 96 to 88, we don't have any `mrrc`.

With a volatile attribute, the above changes to this:

  8a:   ec53 2f0e       mrrc    15, 0, r2, r3, cr14
  8e:   42ab            cmp     r3, r5
  90:   f8c1 20a4       str.w   r2, [r1, #164]  ; 0xa4
  94:   bf08            it      eq
  96:   42a2            cmpeq   r2, r4
  98:   f8c1 30a0       str.w   r3, [r1, #160]  ; 0xa0
  9c:   d3f5            bcc.n   8a <__udelay+0x8a>
  9e:   e8bd 8cf0       ldmia.w sp!, {r4, r5, r6, r7, sl, fp, pc}
  a2:   bf00            nop

I'm advised[1] to put volatile on all asm(), so this commit also adds it
to the asm() in timer_init().

[1]: https://lists.denx.de/pipermail/u-boot/2018-March/322062.html

Signed-off-by: Yasushi SHOJI <yasushi.sh...@gmail.com>
Reviewed-by: Fabio Estevam <fabio.este...@nxp.com>
---
 arch/arm/mach-imx/syscounter.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-imx/syscounter.c b/arch/arm/mach-imx/syscounter.c
index 9290918dca..1d4ebfe343 100644
--- a/arch/arm/mach-imx/syscounter.c
+++ b/arch/arm/mach-imx/syscounter.c
@@ -62,7 +62,7 @@ int timer_init(void)
        unsigned long val, freq;
 
        freq = CONFIG_SC_TIMER_CLK;
-       asm("mcr p15, 0, %0, c14, c0, 0" : : "r" (freq));
+       asm volatile("mcr p15, 0, %0, c14, c0, 0" : : "r" (freq));
 
        writel(freq, &sctr->cntfid0);
 
@@ -82,7 +82,7 @@ unsigned long long get_ticks(void)
 {
        unsigned long long now;
 
-       asm("mrrc p15, 0, %Q0, %R0, c14" : "=r" (now));
+       asm volatile("mrrc p15, 0, %Q0, %R0, c14" : "=r" (now));
 
        gd->arch.tbl = (unsigned long)(now & 0xffffffff);
        gd->arch.tbu = (unsigned long)(now >> 32);
-- 
2.16.2

_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Reply via email to