[U-Boot-Users] cfi_flash.c and lost volatile qualifier
Adrian Filipi
adrian.filipi at eurotech.com
Thu May 8 16:05:23 CEST 2008
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
>
More information about the U-Boot
mailing list