This is the mail archive of the
ecos-patches@sources.redhat.com
mailing list for the eCos project.
V2 flash - sort out loops
- From: Bart Veer <bartv at ecoscentric dot com>
- To: ecos-patches at ecos dot sourceware dot org
- Date: Sat, 20 Nov 2004 19:26:38 +0000 (GMT)
- Subject: V2 flash - sort out loops
Some of the boards I am using have flash right at the top of memory,
e.g. 0xFFE00000 to 0xFFFFFFFF. This revealed various problems with the
loops in the main flash code. Address comparisons of the form
"while (block <= end_addr)" will fail if end_addr is 0xffffffff and
block wraps around to 0, resulting in attempts to e.g. erase the
entire address space.
I have restructured all the loops so that they are based on counts
rather than address comparisons, which should sort things out.
Bart
Index: ChangeLog
===================================================================
RCS file: /cvs/ecos/ecos/packages/io/flash/current/ChangeLog,v
retrieving revision 1.38.2.10
diff -u -r1.38.2.10 ChangeLog
--- ChangeLog 6 Oct 2004 10:59:03 -0000 1.38.2.10
+++ ChangeLog 20 Nov 2004 19:17:27 -0000
@@ -1,3 +1,8 @@
+2004-11-20 Bart Veer <bartv@ecoscentric.com>
+
+ * src/flash.c: rearrange loops to avoid address comparisons, which
+ tend to go wrong if the flash is at the end of the address space
+
2004-10-06 Andrew Lunn <andrew.lunn@ascom.ch>
* src/legacy_dev.c (legacy_flash_init): The end is the size plus
Index: src/flash.c
===================================================================
RCS file: /cvs/ecos/ecos/packages/io/flash/current/src/flash.c,v
retrieving revision 1.26.2.6
diff -u -r1.26.2.6 flash.c
--- src/flash.c 30 Sep 2004 18:09:21 -0000 1.26.2.6
+++ src/flash.c 20 Nov 2004 19:17:42 -0000
@@ -74,9 +75,6 @@
#if !defined(CYG_HAL_STARTUP_RAM) && defined(RAM_FLASH_DEV_DEBUG)
# warning "Can only enable the flash debugging when configured for RAM startup"
#endif
-#ifndef MIN
-#define MIN(x,y) ((x)<(y) ? (x) : (y))
-#endif
// Has the FLASH IO library been initialise?
static bool init = false;
@@ -202,7 +200,7 @@
if (!init) return CYG_FLASH_ERR_NOT_INIT;
for (dev = flash_head; dev; dev=dev->next) {
- if ((dev->start <= address) && (dev->end > address))
+ if ((dev->start <= address) && (address <= dev->end))
return CYG_FLASH_ERR_OK;
}
return CYG_FLASH_ERR_INVALID;
@@ -249,7 +247,7 @@
if (!init) return CYG_FLASH_ERR_NOT_INIT;
for (dev = flash_head; dev ; dev=dev->next) {
- if ((dev->start <= flash_base) && ( dev->end > flash_base)) {
+ if ((dev->start <= flash_base) && ( flash_base <= dev->end)) {
info->start = dev->start;
info->end = dev->end;
info->num_block_infos = dev->num_block_infos;
@@ -271,7 +269,7 @@
if (!init) return CYG_FLASH_ERR_NOT_INIT;
for (dev = flash_head; dev ; dev=dev->next) {
- if ((dev->start <= from) && ( dev->end > from)) {
+ if ((dev->start <= from) && ( from <= dev->end)) {
cyg_mutex_lock(&dev->mutex);
if (dev->end > from+len) {
return CYG_FLASH_ERR_OK;
@@ -299,7 +297,7 @@
if (!init) return CYG_FLASH_ERR_NOT_INIT;
for (dev = flash_head; dev ; dev=dev->next) {
- if ((dev->start <= from) && ( dev->end > from)) {
+ if ((dev->start <= from) && ( from <= dev->end)) {
cyg_mutex_unlock(&dev->mutex);
if (dev->end > from+len) {
return CYG_FLASH_ERR_OK;
@@ -344,7 +342,7 @@
if (!init) return CYG_FLASH_ERR_NOT_INIT;
for (dev = flash_head;
- dev && !((dev->start <= flash_base) && ( dev->end > flash_base));
+ dev && !((dev->start <= flash_base) && ( flash_base <= dev->end));
dev=dev->next)
;
if (!dev) return CYG_FLASH_ERR_INVALID;
@@ -374,21 +372,21 @@
const size_t len,
cyg_flashaddr_t *err_address)
{
- cyg_flashaddr_t block, end_addr, addr;
+ cyg_flashaddr_t block, end_addr;
struct cyg_flash_dev * dev;
- size_t block_size;
+ size_t erase_count;
int stat = CYG_FLASH_ERR_OK;
int d_cache, i_cache;
if (!init) return CYG_FLASH_ERR_NOT_INIT;
#ifdef CYGSEM_IO_FLASH_SOFT_WRITE_PROTECT
- if (plf_flash_query_soft_wp(addr,len))
+ if (plf_flash_query_soft_wp(flash_base,len))
return CYG_FLASH_ERR_PROTECT;
#endif
for (dev = flash_head;
- dev && !((dev->start <= flash_base) && ( dev->end > flash_base));
+ dev && !((dev->start <= flash_base) && ( flash_base <= dev->end));
dev=dev->next)
;
if (!dev) return CYG_FLASH_ERR_INVALID;
@@ -396,30 +394,40 @@
#ifdef CYGPKG_KERNEL
cyg_mutex_lock(&dev->mutex);
#endif
- addr = flash_base;
- end_addr = flash_base + len - 1;
- if (end_addr > dev->end) {
- end_addr = dev->end;
- }
-
- block = flash_block_begin(addr, dev);
-
+
+ // Check whether or not we are going past the end of this device, on
+ // to the next one. If so the next device will be handled by a
+ // recursive call later on.
+ if (len > (dev->end + 1 - flash_base)) {
+ end_addr = dev->end;
+ } else {
+ end_addr = flash_base + len - 1;
+ }
+ // erase can only happen on a block boundary, so adjust for this
+ block = flash_block_begin(flash_base, dev);
+ erase_count = (end_addr + 1) - block;
+
#ifdef CYGSEM_IO_FLASH_CHATTER
dev->pf("... Erase from %p-%p: ", (void*)block, (void*)end_addr);
#endif
HAL_FLASH_CACHES_OFF(d_cache, i_cache);
- FLASH_Enable(block, end_addr);
- while (block <= end_addr) {
+ FLASH_Enable(flash_base, end_addr);
+ while (erase_count > 0) {
int i;
unsigned char *dp;
- bool erased = true;
+ bool erased = false;
+ size_t block_size = flash_block_size(dev, block);
- block_size = flash_block_size(dev, addr);
+ // Pad to the block boundary, if necessary
+ if (erase_count < block_size) {
+ erase_count = block_size;
+ }
// If there is a read function it probably means the flash
// cannot be read directly.
if (!dev->funs->flash_read) {
+ erased = true;
dp = (unsigned char *)block;
for (i = 0; i < block_size; i++) {
if (*dp++ != (unsigned char)0xFF) {
@@ -427,8 +435,6 @@
break;
}
}
- } else {
- erased=false;
}
if (!erased) {
stat = dev->funs->flash_erase_block(dev,block);
@@ -438,12 +444,13 @@
*err_address = block;
break;
}
- block += block_size;
+ block += block_size;
+ erase_count -= block_size;
#ifdef CYGSEM_IO_FLASH_CHATTER
dev->pf(".");
#endif
}
- FLASH_Disable(block, end_addr);
+ FLASH_Disable(flash_base, end_addr);
HAL_FLASH_CACHES_ON(d_cache, i_cache);
#ifdef CYGSEM_IO_FLASH_CHATTER
dev->pf("\n");
@@ -454,11 +461,14 @@
if (stat != CYG_FLASH_ERR_OK) {
return stat;
}
-
- if (flash_base + len - 1 > dev->end) {
- // The region to erase if bigger than this driver handles. Recurse
+
+ // If there are multiple flash devices in series the erase operation
+ // may touch successive devices. This can be handled by recursion.
+ // The stack overheads should be minimal because the number of
+ // devices will be small.
+ if (len > (dev->end + 1 - flash_base)) {
return cyg_flash_erase(dev->end+1,
- len - (dev->end - flash_base) - 1,
+ len - (dev->end + 1 - flash_base),
err_address);
}
return CYG_FLASH_ERR_OK;
@@ -471,9 +481,9 @@
cyg_flashaddr_t *err_address)
{
struct cyg_flash_dev * dev;
- cyg_flashaddr_t addr, end_addr;
+ cyg_flashaddr_t addr, end_addr, block;
unsigned char * ram = ram_base;
- size_t block_size, size, length, offset;
+ size_t write_count, offset;
int stat = CYG_FLASH_ERR_OK;
int d_cache, i_cache;
@@ -485,7 +495,7 @@
#endif
for (dev = flash_head;
- dev && !((dev->start <= flash_base) && ( dev->end > flash_base));
+ dev && !((dev->start <= flash_base) && ( flash_base <= dev->end));
dev=dev->next)
;
if (!dev) return CYG_FLASH_ERR_INVALID;
@@ -494,34 +504,45 @@
cyg_mutex_lock(&dev->mutex);
#endif
addr = flash_base;
- end_addr = flash_base + len - 1;
- if (end_addr > dev->end) {
+ if (len > (dev->end + 1 - flash_base)) {
end_addr = dev->end;
+ } else {
+ end_addr = flash_base + len - 1;
+ }
+ write_count = (end_addr + 1) - flash_base;
+
+ // The first write may be in the middle of a block. Do the necessary
+ // adjustment here rather than inside the loop.
+ block = flash_block_begin(flash_base, dev);
+ if (addr == block) {
+ offset = 0;
+ } else {
+ offset = addr - block;
}
- length = end_addr - addr + 1;
#ifdef CYGSEM_IO_FLASH_CHATTER
dev->pf("... Program from %p-%p to %p: ", ram_base,
- ((CYG_ADDRESS)ram_base)+length, addr);
+ ((CYG_ADDRESS)ram_base)+write_count, addr);
#endif
HAL_FLASH_CACHES_OFF(d_cache, i_cache);
- FLASH_Enable((unsigned short*)addr, (unsigned short *)(addr+len));
- while (addr <= end_addr) {
- block_size = flash_block_size(dev, addr);
- size = length;
- // Only one block at once
- if (size > block_size) size = block_size;
+ FLASH_Enable(flash_base, end_addr);
+ while (write_count > 0) {
+ size_t block_size = flash_block_size(dev, addr);
+ size_t this_write;
+ if (write_count > (block_size - offset)) {
+ this_write = block_size - offset;
+ } else {
+ this_write = write_count;
+ }
+ // Only the first block may need the offset.
+ offset = 0;
- // Writing from the middle of a block?
- offset = (size_t)(addr - dev->start) % block_size;
- if (offset)
- size = MIN(block_size - offset, size);
- stat = dev->funs->flash_program(dev, addr, ram, size);
+ stat = dev->funs->flash_program(dev, addr, ram, this_write);
stat = dev->funs->flash_hwr_map_error(dev,stat);
#ifdef CYGSEM_IO_FLASH_VERIFY_PROGRAM
if (CYG_FLASH_ERR_OK == stat) // Claims to be OK
- if (!dev->funs->flash_read && memcmp((void *)addr, ram, size) != 0) {
+ if (!dev->funs->flash_read && memcmp((void *)addr, ram, this_write) != 0) {
stat = CYG_FLASH_ERR_DRV_VERIFY;
#ifdef CYGSEM_IO_FLASH_CHATTER
dev->pf("V");
@@ -535,11 +556,11 @@
#ifdef CYGSEM_IO_FLASH_CHATTER
dev->pf(".");
#endif
- length -= size;
- addr += size;
- ram += size;
+ write_count -= this_write;
+ addr += this_write;
+ ram += this_write;
}
- FLASH_Disable((unsigned short*)addr, (unsigned short *)(addr+len));
+ FLASH_Disable(flash_base, end_addr);
HAL_FLASH_CACHES_ON(d_cache, i_cache);
#ifdef CYGSEM_IO_FLASH_CHATTER
dev->pf("\n");
@@ -550,9 +571,9 @@
if (stat != CYG_FLASH_ERR_OK) {
return (stat);
}
- if ( flash_base + len - 1 > dev->end) {
+ if (len > (dev->end + 1 - flash_base)) {
return cyg_flash_program(dev->end+1, ram,
- len - (dev->end - flash_base) - 1,
+ len - (dev->end + 1 - flash_base),
err_address);
}
return CYG_FLASH_ERR_OK;
@@ -565,16 +586,16 @@
cyg_flashaddr_t *err_address)
{
struct cyg_flash_dev * dev;
- cyg_flashaddr_t addr, end_addr;
+ cyg_flashaddr_t addr, end_addr, block;
unsigned char * ram = (unsigned char *)ram_base;
- size_t block_size, size, length, offset;
+ size_t read_count, offset;
int stat = CYG_FLASH_ERR_OK;
int d_cache, i_cache;
if (!init) return CYG_FLASH_ERR_NOT_INIT;
for (dev = flash_head;
- dev && !((dev->start <= flash_base) && ( dev->end > flash_base));
+ dev && !((dev->start <= flash_base) && ( flash_base <= dev->end));
dev=dev->next)
;
if (!dev) return CYG_FLASH_ERR_INVALID;
@@ -583,33 +604,44 @@
cyg_mutex_lock(&dev->mutex);
#endif
addr = flash_base;
- end_addr = flash_base + len - 1;
- if (end_addr > dev->end) {
- end_addr = dev->end;
+ if (len > (dev->end + 1 - flash_base)) {
+ end_addr = dev->end;
+ } else {
+ end_addr = flash_base + len - 1;
+ }
+ read_count = (end_addr + 1) - flash_base;
+
+ // The first read may be in the middle of a block. Do the necessary
+ // adjustment here rather than inside the loop.
+ block = flash_block_begin(flash_base, dev);
+ if (addr == block) {
+ offset = 0;
+ } else {
+ offset = addr - block;
}
- length = end_addr - addr + 1;
#ifdef CYGSEM_IO_FLASH_CHATTER
- dev->pf("... Read from %p-%p to %p: ", addr, addr+len, ram_base);
+ dev->pf("... Read from %p-%p to %p: ", addr, end_addr, ram_base);
#endif
HAL_FLASH_CACHES_OFF(d_cache, i_cache);
- FLASH_Enable((unsigned short*)addr, (unsigned short *)(addr+len));
- while (addr <= end_addr) {
- block_size = flash_block_size(dev, addr);
- size = length;
- // Only one block at once
- if (size > block_size) size = block_size;
+ FLASH_Enable(flash_base, end_addr);
+ while (read_count > 0) {
+ size_t block_size = flash_block_size(dev, addr);
+ size_t this_read;
+ if (read_count > (block_size - offset)) {
+ this_read = block_size - offset;
+ } else {
+ this_read = read_count;
+ }
+ // Only the first block may need the offset
+ offset = 0;
- // Reading from the middle of a block?
- offset = (size_t)(addr - dev->start) % block_size;
- if (offset)
- size = MIN(block_size - offset, size);
if (dev->funs->flash_read) {
- stat = dev->funs->flash_read(dev, addr, ram, size);
+ stat = dev->funs->flash_read(dev, addr, ram, this_read);
stat = dev->funs->flash_hwr_map_error(dev,stat);
} else {
- memcpy(ram, (void *)addr, size);
+ memcpy(ram, (void *)addr, this_read);
stat = CYG_FLASH_ERR_OK;
}
if (CYG_FLASH_ERR_OK != stat && err_address) {
@@ -619,11 +651,11 @@
#ifdef CYGSEM_IO_FLASH_CHATTER
dev->pf(".");
#endif
- length -= size;
- addr += size;
- ram += size;
+ read_count -= this_read;
+ addr += this_read;
+ ram += this_read;
}
- FLASH_Disable((unsigned short*)addr, (unsigned short *)(addr+len));
+ FLASH_Disable(flash_base, end_addr);
HAL_FLASH_CACHES_ON(d_cache, i_cache);
#ifdef CYGSEM_IO_FLASH_CHATTER
dev->pf("\n");
@@ -634,10 +666,10 @@
if (stat != CYG_FLASH_ERR_OK) {
return (stat);
}
- if ( flash_base + len - 1 > dev->end) {
- return cyg_flash_read(dev->end+1, ram,
- len - (dev->end - flash_base) - 1,
- err_address);
+ if (len > (dev->end + 1 - flash_base)) {
+ return cyg_flash_read(dev->end+1, ram,
+ len - (dev->end + 1 - flash_base),
+ err_address);
}
return CYG_FLASH_ERR_OK;
}
@@ -648,9 +680,9 @@
const size_t len,
cyg_flashaddr_t *err_address)
{
- cyg_flashaddr_t block, end_addr, addr;
+ cyg_flashaddr_t block, end_addr;
struct cyg_flash_dev * dev;
- size_t block_size;
+ size_t lock_count;
int stat = CYG_FLASH_ERR_OK;
int d_cache, i_cache;
@@ -662,7 +694,7 @@
#endif
for (dev = flash_head;
- dev && !((dev->start <= flash_base) && ( dev->end > flash_base));
+ dev && !((dev->start <= flash_base) && ( flash_base <= dev->end));
dev=dev->next)
;
if (!dev) return CYG_FLASH_ERR_INVALID;
@@ -671,21 +703,25 @@
#ifdef CYGPKG_KERNEL
cyg_mutex_lock(&dev->mutex);
#endif
- addr = flash_base;
- end_addr = flash_base + len - 1;
- if (end_addr > dev->end) {
- end_addr = dev->end;
+ if (len > (dev->end + 1 - flash_base)) {
+ end_addr = dev->end;
+ } else {
+ end_addr = flash_base + len - 1;
}
-
- block = flash_block_begin(addr, dev);
+ block = flash_block_begin(flash_base, dev);
+ lock_count = (end_addr + 1) - block;
#ifdef CYGSEM_IO_FLASH_CHATTER
dev->pf("... Locking from %p-%p: ", (void*)block, (void*)end_addr);
#endif
HAL_FLASH_CACHES_OFF(d_cache, i_cache);
- FLASH_Enable(block, end_addr);
- while (block <= end_addr) {
+ FLASH_Enable(flash_base, end_addr);
+ while (lock_count > 0) {
+ size_t block_size = flash_block_size(dev, block);
+ if (lock_count < block_size) {
+ lock_count = block_size;
+ }
stat = dev->funs->flash_block_lock(dev,block);
stat = dev->funs->flash_hwr_map_error(dev,stat);
@@ -693,12 +729,13 @@
*err_address = block;
break;
}
- block += flash_block_size(dev, addr);
+ block += block_size;
+ lock_count -= block_size;
#ifdef CYGSEM_IO_FLASH_CHATTER
dev->pf(".");
#endif
}
- FLASH_Disable(block, end_addr);
+ FLASH_Disable(flash_base, end_addr);
HAL_FLASH_CACHES_ON(d_cache, i_cache);
#ifdef CYGSEM_IO_FLASH_CHATTER
dev->pf("\n");
@@ -709,13 +746,14 @@
if (stat != CYG_FLASH_ERR_OK) {
return stat;
}
-
- if (flash_base + len - 1 > dev->end) {
- // The region to erase if bigger than this driver handles. Recurse
+
+ // Recurse if necessary for the next device
+ if (len > (dev->end + 1 - flash_base)) {
return cyg_flash_lock(dev->end+1,
- len - (dev->end - flash_base) - 1,
+ len - (dev->end + 1 - flash_base),
err_address);
}
+
return CYG_FLASH_ERR_OK;
}
@@ -724,9 +762,9 @@
const size_t len,
cyg_flashaddr_t *err_address)
{
- cyg_flashaddr_t block, end_addr, addr;
+ cyg_flashaddr_t block, end_addr;
struct cyg_flash_dev * dev;
- size_t block_size;
+ size_t unlock_count;
int stat = CYG_FLASH_ERR_OK;
int d_cache, i_cache;
@@ -738,7 +776,7 @@
#endif
for (dev = flash_head;
- dev && !((dev->start <= flash_base) && ( dev->end > flash_base));
+ dev && !((dev->start <= flash_base) && ( flash_base <= dev->end));
dev=dev->next)
;
if (!dev) return CYG_FLASH_ERR_INVALID;
@@ -747,21 +785,25 @@
#ifdef CYGPKG_KERNEL
cyg_mutex_lock(&dev->mutex);
#endif
- addr = flash_base;
- end_addr = flash_base + len - 1;
- if (end_addr > dev->end) {
- end_addr = dev->end;
+ if (len > (dev->end + 1 - flash_base)) {
+ end_addr = dev->end;
+ } else {
+ end_addr = flash_base + len - 1;
}
-
- block = block = flash_block_begin(addr, dev);
+ block = flash_block_begin(flash_base, dev);
+ unlock_count = (end_addr + 1) - block;
#ifdef CYGSEM_IO_FLASH_CHATTER
dev->pf("... Unlocking from %p-%p: ", (void*)block, (void*)end_addr);
#endif
HAL_FLASH_CACHES_OFF(d_cache, i_cache);
- FLASH_Enable(block, end_addr);
- while (block <= end_addr) {
+ FLASH_Enable(flash_base, end_addr);
+ while (unlock_count > 0) {
+ size_t block_size = flash_block_size(dev, block);
+ if (unlock_count < block_size) {
+ unlock_count = block_size;
+ }
stat = dev->funs->flash_block_unlock(dev,block);
stat = dev->funs->flash_hwr_map_error(dev,stat);
@@ -769,12 +811,14 @@
*err_address = block;
break;
}
- block += flash_block_size(dev, addr);
+ block += block_size;
+ unlock_count -= block_size;
+
#ifdef CYGSEM_IO_FLASH_CHATTER
dev->pf(".");
#endif
}
- FLASH_Disable(block, end_addr);
+ FLASH_Disable(flash_base, end_addr);
HAL_FLASH_CACHES_ON(d_cache, i_cache);
#ifdef CYGSEM_IO_FLASH_CHATTER
dev->pf("\n");
@@ -786,10 +830,10 @@
return stat;
}
- if (flash_base + len - 1 > dev->end) {
- // The region to erase if bigger than this driver handles. Recurse
+ // Recurse if necessary for the next device
+ if (len > (dev->end + 1 - flash_base)) {
return cyg_flash_lock(dev->end+1,
- len - (dev->end - flash_base) - 1,
+ len - (dev->end + 1 - flash_base),
err_address);
}
return CYG_FLASH_ERR_OK;