[U-Boot] [PATCH] imx: syscounter: make sure asm is volatile

[Stefano Babic]

On 09/03/2018 11:46, Fabio Estevam wrote:
> [Adding Stefano]
> 
> On Thu, Mar 8, 2018 at 1:21 AM, Yasushi SHOJI <yasushi.shoji at gmail.com> wrote:
>> 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 at gmail.com>
>> Reviewed-by: Fabio Estevam <fabio.estevam at nxp.com>
> 
> Hi Stefano,
> 
> Do you think this one could be applied for the upcoming release?
> 


Applied as fix as already discussed in other thread, thanks !

Best regards,
Stefano

-- 
=====================================================================
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================