[U-Boot] oamp3: bug in clock.c with gcc 4.5.1?

[Alexander Holler]

Hello,

Am 17.12.2010 15:20, schrieb Wolfgang Denk:

>> I think I've nailed down, why an u-boot compiled with gcc 4.5.1 fails
>> here. Compiling arch/arm/cpu/armv7/omap3/clock.c with
>> ...
>> That means get_sys_clk_speed() will allways return S12M, at least that
>> is what I'm reading here. And I don't see why gcc should be allowed to
>> optimize that cdiff to a fixed value and therefor returning only S12M.
>
> Well, if you look at the code after preprocessing it looks like this:
>
> struct gptimer *gpt1_base = (struct gptimer *)0x48318000;
> ...
> cstart = (*(volatile unsigned int *)(&gpt1_base->tcrr));
> ...
> cend = (*(volatile unsigned int *)(&gpt1_base->tcrr));
> cdiff = cend - cstart;
>
>
> Eventually that simple definition of readl() is no longer good enough
> for recent compilers.  There is probably a very good reason that Linux
> uses a "__iormb();" memory barrier in the definition of readl() and
> similar macros.
>
> We should probably update "arch/arm/include/asm/io.h" ...

I was unsure if that volatile is enough, therefor I've asked here. 
Looking at the kernel would have been my next step, but I thought it 
would be good to inform other ARM-users here early. In things belonging 
to lowlevel ARM I'm no expert too.

Anyway a memory barrier seems to be a very good idea in readl, even when 
ignoring the volatile should be considered as a bug of gcc.

Linux has a paper on that topic in 
Documentation/volatile-considered-harmful.txt.

I will modify readl here, check what happens, and will post a patch if 
that fixes the problem.

Regards,

Alexander