imx: syscounter: make sure asm is volatile
authorYasushi SHOJI <yasushi.shoji@gmail.com>
Thu, 8 Mar 2018 04:21:10 +0000 (13:21 +0900)
committerStefano Babic <sbabic@denx.de>
Fri, 9 Mar 2018 12:06:14 +0000 (13:06 +0100)
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.shoji@gmail.com>
Reviewed-by: Fabio Estevam <fabio.estevam@nxp.com>
arch/arm/mach-imx/syscounter.c

index 9290918dca225b7fa0ea2c706a51d06deb4d31a6..1d4ebfe3439ff1fb078ca820674fcdd957b058f8 100644 (file)
@@ -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);