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@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