[U-Boot] [PATCH v3] ARM: Avoid compiler optimization for usages of readb, writeb and friends.

[Alexander Holler <holler@ahsoftware.de>]

Am 01.01.2011 20:21, schrieb Dirk Behme:
> On 01.01.2011 19:47, Alexander Holler wrote:
>> Am 01.01.2011 19:25, schrieb Dirk Behme:
>>> On 01.01.2011 18:52, Alexander Holler wrote:
>>>> Hello,
>>>>
>>>> Am 01.01.2011 13:04, schrieb Dirk Behme:
>>>>> On 22.12.2010 12:04, Alexander Holler wrote:
>>>>>> gcc 4.5.1 seems to ignore (at least some) volatile definitions,
>>>>>> avoid that as done in the kernel.
>>>>>>
>>>>>> Reading C99 6.7.3 8 and the comment 114) there, I think it is a
>>>>>> bug of
>>>>>> that
>>>>>> gcc version to ignore the volatile type qualifier used e.g. in
>>>>>> __arch_getl().
>>>>>> Anyway, using a definition as in the kernel headers avoids such
>>>>>> optimizations when
>>>>>> gcc 4.5.1 is used.
>>>>>>
>>>>>> Maybe the headers as used in the current linux-kernel should be
>>>>>> used,
>>>>>> but to avoid large changes, I've just added a small change to the
>>>>>> current headers.
>>>>
>>>>> Do you like to test the patch in the attachment? I named it 'v4'.
>>>>>
>>>>> After some thinking and testing, it seems to me that the volatile
>>>>> optimization issue this patch shall fix is only with the readx()
>>>>> macros.
>>>>> So the idea is to drop all writex() changes done in the v3 version of
>>>>> this patch. With dropping the writex() changes, we would drop all
>>>>> issues
>>>>> we discussed with e.g. the GCC statement-expression and the do while
>>>>> workaround, too.
>>>>
>>>> I've come across a bug which reads as the problem might be fixed in
>>>> gcc 4.5.2:
>>>>
>>>> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=45052
>>>>
>>>> I will test gcc 4.5.2 in the next days.
>>>
>>> Have you been able to test v4 of the patch I sent with gcc 4.5.1?
>>
>> No, sorry, I don't have a test case for consequent write* and I will
>> have to write one.
>
> ?
>
> If I remember correctly, the test case for this patch was compiling
> U-Boot with 4.5.1 and then check
>
> a) if it boots at Beagle (correct clock.c)
> b) if NAND works ok (correct omap_gpmc.c)
>
> ?

No. None of those must fail when the compiler optimizes consequent 
write* to one write* because the compiler ignores the volatile keyword.
I've only found the problem with consequent read* (in clock.c), but 
there might be problems with consequent write* somewhere else too. So if 
you remove the change for those write* some other problems might arise 
and just through booting a kernel those might not be found. So I think 
it would be dangerous to remove the change for write* when using gcc 4.5.x

And because the patch fixes only write* and read* some stuff in u-boot 
which uses volatile in another context might still fail, therefore I 
vote to use the current kernel headers where other things besides read* 
and write* are using those barriers too.

Regards,

Alexander