[U-Boot-Users] cfi_flash.c and lost volatile qualifier

[Adrian Filipi]

 	Yeah, I agree it's bogus for the pointer to be volatile.  There 
shouldn't be anything unusual about that.

 	The assembler does show several additional memory accesses, so I 
think your theory is right.  I'm at a loss for what to to on the sync().

 	Adrian
--
Linux Software Engineer | EuroTech, Inc. | www.eurotech-inc.com

On Thu, 8 May 2008, Haavard Skinnemoen wrote:

> Adrian Filipi <adrian.filipi at eurotech.com> wrote:
>> diff -u -r1.1.1.2.2.2 cfi_flash.c
>> --- cfi_flash.c	23 Apr 2008 17:02:47 -0000	1.1.1.2.2.2
>> +++ cfi_flash.c	29 Apr 2008 18:55:47 -0000
>> @@ -464,7 +464,7 @@
>>   			     uint offset, uchar cmd)
>>   {
>>
>> -	void *addr;
>> +	void *volatile addr;
>
> That looks bogus. It makes the pointer itself volatile, not whatever it
> points to. All accesses through addr is performed by appropriate I/O
> accessors, so no volatile should be needed, and certainly not one that
> affects the pointer itself.
>
> I suspect the change adds a few extra memory accesses (probably to the
> stack) so that any memory ordering problems just happen to go away.
>
> flash_write_cmd() calls sync() after writing the command to the flash,
> so any memory ordering issues should have been taken care of. However,
> sync() does nothing on ARM. Maybe it should do something?
>
> Haavard
>