This is the mail archive of the
ecos-patches@sources.redhat.com
mailing list for the eCos project.
V2 flash - optimize for a single flash device
- From: Bart Veer <bartv at ecoscentric dot com>
- To: ecos-patches at ecos dot sourceware dot org
- Date: Sun, 21 Nov 2004 23:38:29 +0000 (GMT)
- Subject: V2 flash - optimize for a single flash device
Most boards only have a single flash device, in which case there is no
point in creating a sorted linked list of devices and searching this
list for just about every operation. This patch moves the search code
into a utility find_dev() which is implemented differently for targets
with a single device or multiple devices. For a redboot build on a
board with just one flash device this saves a couple of hundred bytes.
As a side effect, find_dev() checks the per-device init flag.
Previously this flag was never checked so flash operations were
permitted even if the device had failed to initialize. That seemed
like a bad idea.
Bart
Index: ChangeLog
===================================================================
RCS file: /cvs/ecos/ecos/packages/io/flash/current/ChangeLog,v
retrieving revision 1.38.2.16
diff -u -r1.38.2.16 ChangeLog
--- ChangeLog 21 Nov 2004 18:39:16 -0000 1.38.2.16
+++ ChangeLog 21 Nov 2004 23:36:41 -0000
@@ -1,5 +1,7 @@
2004-11-21 Bart Veer <bartv@ecoscentric.com>
+ * src/flash.c, include/flash_priv.h, cdl/io_flash.h: optimize
+ for the case of a single flash device.
* include/flash.h, src/flash.c, doc/flash.sgml: remove
cyg_flash_code_overlaps()
* include/flash_priv.h, include/flash.h:
Index: cdl/io_flash.cdl
===================================================================
RCS file: /cvs/ecos/ecos/packages/io/flash/current/cdl/io_flash.cdl,v
retrieving revision 1.17.2.3
diff -u -r1.17.2.3 io_flash.cdl
--- cdl/io_flash.cdl 21 Nov 2004 17:33:49 -0000 1.17.2.3
+++ cdl/io_flash.cdl 21 Nov 2004 23:36:58 -0000
@@ -66,9 +66,11 @@
cdl_interface CYGHWR_IO_FLASH_DEVICE {
display "Hardware FLASH device drivers"
+ requires { CYGHWR_IO_FLASH_DEVICE >= 1 }
description "
This calculated option gives the number of flash devices
- on the current platform."
+ on the current platform. The generic flash support requires
+ at least one device."
}
cdl_interface CYGHWR_IO_FLASH_BLOCK_LOCKING {
Index: include/flash_priv.h
===================================================================
RCS file: /cvs/ecos/ecos/packages/io/flash/current/include/Attic/flash_priv.h,v
retrieving revision 1.1.2.2
diff -u -r1.1.2.2 flash_priv.h
--- include/flash_priv.h 21 Nov 2004 17:33:48 -0000 1.1.2.2
+++ include/flash_priv.h 21 Nov 2004 23:37:11 -0000
@@ -54,6 +54,9 @@
#ifndef _IO_FLASH_PRIV_H_
#define _IO_FLASH_PRIV_H_
+#include <pkgconf/system.h>
+#include <pkgconf/io_flash.h>
+
// Forward reference of the device structure
struct cyg_flash_dev;
@@ -94,7 +97,9 @@
#ifdef CYGPKG_KERNEL
cyg_mutex_t mutex; // Mutex for thread safeness
#endif
+#if (CYGHWR_IO_FLASH_DEVICE > 1)
struct cyg_flash_dev *next; // Pointer to next device
+#endif
} CYG_HAL_TABLE_TYPE;
#define CYG_FLASH_FUNS(funs,init,query,erase,prog,read,map,lock,unlock)\
Index: src/flash.c
===================================================================
RCS file: /cvs/ecos/ecos/packages/io/flash/current/src/flash.c,v
retrieving revision 1.26.2.11
diff -u -r1.26.2.11 flash.c
--- src/flash.c 21 Nov 2004 18:39:15 -0000 1.26.2.11
+++ src/flash.c 21 Nov 2004 23:37:26 -0000
@@ -106,11 +106,8 @@
#define CHECK_SOFT_WRITE_PROTECT(_addr_, _len_) CYG_EMPTY_STATEMENT
#endif
-// Has the FLASH IO library been initialise?
+// Has the FLASH IO library been initialised?
static bool init = false;
-// We have a linked list of flash drivers. This is the head of the
-// list
-static struct cyg_flash_dev *flash_head = NULL;
// This array contains entries for all flash devices that are
// installed in the system.
@@ -121,18 +118,56 @@
__externC struct cyg_flash_dev cyg_flashdevtab_end;
CYG_HAL_TABLE_END(cyg_flashdevtab_end, cyg_flashdev);
-// Sort the linked list into ascending order of flash address. Use a
-// primitive ripple sort, but since we don't expect to have many
-// devices this should be OK.
-static void flash_sort(void)
+#if (1 == CYGHWR_IO_FLASH_DEVICE)
+
+// Optimize the code for a single flash device, which is the common case.
+// The flash subsystem must have been initialized, the single device must
+// contain the specified address, and the device itself must have
+// initialized successfully.
+static struct cyg_flash_dev*
+find_dev(cyg_flashaddr_t addr, int* stat)
+{
+ if (!init) {
+ *stat = CYG_FLASH_ERR_NOT_INIT;
+ return NULL;
+ }
+ if (! ((addr >= cyg_flashdevtab[0].start) && (addr <= cyg_flashdevtab[0].end))) {
+ *stat = CYG_FLASH_ERR_INVALID;
+ return NULL;
+ }
+ if (! cyg_flashdevtab[0].init) {
+ *stat = CYG_FLASH_ERR_NOT_INIT;
+ return NULL;
+ }
+ return &cyg_flashdevtab[0];
+}
+
+#else
+
+// There are multiple devices. For convenience these are kept in a
+// linked list, sorted by address. This is the head of the list
+static struct cyg_flash_dev *flash_head = NULL;
+
+static bool flash_sort_and_check(void)
{
bool moved;
struct cyg_flash_dev *dev, **previous_next;
+
+ // Place all devices on the list, unsorted for now.
+ for (dev = &cyg_flashdevtab[0]; dev != &cyg_flashdevtab_end; dev++) {
+ dev->next = flash_head;
+ flash_head = dev;
+ }
- // If there is zero or one device, short cut
- if (flash_head == NULL || flash_head->next == NULL)
- return;
-
+ // If there are no devices, abort. This should not happen because of
+ // the constraints on CYGHWR_IO_FLASH_DEVICE.
+ if (flash_head == NULL) {
+ return false;
+ }
+
+ // Sort the linked list into ascending order of flash address. Use a
+ // primitive ripple sort, but since we don't expect to have many
+ // devices this should be OK.
do {
moved=false;
for (dev=flash_head, previous_next=&flash_head;
@@ -147,25 +182,40 @@
}
}
} while (moved);
-}
-
-// Walk the linked list and see if there are any overlaps in the
-// addresses the devices claim to use using.
-static bool flash_check_overlap(void)
-{
- struct cyg_flash_dev *dev;
- // If there is zero or one device, short cut
- if (flash_head == NULL || flash_head->next == NULL)
- return false;
-
+ // Now walk the linked list and see if there are any overlaps in the
+ // addresses the devices claim to use using.
for (dev=flash_head; dev->next; dev=dev->next){
if (dev->end >= dev->next->start)
- return true;
+ return false;
}
- return false;
+ return true;
}
+// Find the device at the specified address, if any.
+static struct cyg_flash_dev*
+find_dev(cyg_flashaddr_t addr, int* stat)
+{
+ struct cyg_flash_dev* dev;
+ if (!init) {
+ *stat = CYG_FLASH_ERR_NOT_INIT;
+ return NULL;
+ }
+ for (dev = flash_head; dev; dev = dev->next) {
+ if ((dev->start <= addr) && (addr <= dev->end)) {
+ if (! dev->init) {
+ *stat = CYG_FLASH_ERR_NOT_INIT;
+ return NULL;
+ }
+ return dev;
+ }
+ }
+ *stat = CYG_FLASH_ERR_INVALID;
+ return NULL;
+}
+
+#endif
+
// Initialise all registered device. Any device that fails to
// initialise we leave dev->init as false. Then sort the devices into
// ascending order of address and put them into a linked list. Lastly
@@ -177,7 +227,8 @@
struct cyg_flash_dev * dev;
if (init) return CYG_FLASH_ERR_OK;
- init = true;
+
+ CYG_ASSERT(&(cyg_flashdevtab[CYGHWR_IO_FLASH_DEVICE]) == &cyg_flashdevtab_end, "incorrect number of flash devices");
for (dev = &cyg_flashdevtab[0]; dev != &cyg_flashdevtab_end; dev++) {
dev->pf = pf;
@@ -188,7 +239,7 @@
continue;
}
CYG_ASSERT(dev->funs, "No flash functions");
- CYG_ASSERT(dev->num_block_infos, "No number of block infoss");
+ CYG_ASSERT(dev->num_block_infos, "No number of block infos");
CYG_ASSERT(dev->block_info, "No block infos");
CYG_ASSERT(!(((cyg_flashaddr_t)dev->block_info >= dev->start) &&
((cyg_flashaddr_t)dev->block_info < dev->end)),
@@ -207,15 +258,25 @@
}
#endif
dev->init = true;
- dev->next = flash_head;
- flash_head = dev;
}
- flash_sort();
-
- if (flash_check_overlap()) {
+#if (1 == CYGHWR_IO_FLASH_DEVICE)
+ // Make sure there is one device, otherwise we could end up
+ // accessing a non-existent cyg_flash_dev structure.
+ if (&(cyg_flashdevtab[0]) == &cyg_flashdevtab_end) {
+ return CYG_FLASH_ERR_INVALID;
+ }
+#else
+ // Place the devices on a sorted linked list and check that there
+ // are no overlaps in the address space.
+ if (! flash_sort_and_check() ) {
return CYG_FLASH_ERR_INVALID;
}
+#endif
+
+ // Only mark the flash subsystem as initialized if the world is
+ // consistent.
+ init = true;
return CYG_FLASH_ERR_OK;
}
@@ -223,15 +284,9 @@
__externC int
cyg_flash_verify_addr(const cyg_flashaddr_t address)
{
- struct cyg_flash_dev * dev;
-
- if (!init) return CYG_FLASH_ERR_NOT_INIT;
-
- for (dev = flash_head; dev; dev=dev->next) {
- if ((dev->start <= address) && (address <= dev->end))
- return CYG_FLASH_ERR_OK;
- }
- return CYG_FLASH_ERR_INVALID;
+ int stat = CYG_FLASH_ERR_OK;
+ (void) find_dev(address, &stat);
+ return stat;
}
// Return information about the Nth driver
@@ -241,17 +296,25 @@
struct cyg_flash_dev * dev;
if (!init) return CYG_FLASH_ERR_NOT_INIT;
-
+
+#if (1 == CYGHWR_IO_FLASH_DEVICE)
+ if (0 == Nth) {
+ dev = &(cyg_flashdevtab[0]);
+ } else {
+ return CYG_FLASH_ERR_INVALID;
+ }
+#else
for (dev = flash_head; dev && Nth; dev=dev->next, Nth--)
;
- if (dev && !Nth) {
- info->start = dev->start;
- info->end = dev->end;
- info->num_block_infos = dev->num_block_infos;
- info->block_info = dev->block_info;
- return CYG_FLASH_ERR_OK;
+ if (!dev || !dev->init) {
+ return CYG_FLASH_ERR_INVALID;
}
- return CYG_FLASH_ERR_INVALID;
+#endif
+ info->start = dev->start;
+ info->end = dev->end;
+ info->num_block_infos = dev->num_block_infos;
+ info->block_info = dev->block_info;
+ return CYG_FLASH_ERR_OK;
}
// Return information about the flash at the given address
@@ -259,19 +322,16 @@
cyg_flash_get_info_addr(cyg_flashaddr_t flash_base, cyg_flash_info_t * info)
{
struct cyg_flash_dev *dev;
-
- if (!init) return CYG_FLASH_ERR_NOT_INIT;
-
- for (dev = flash_head; dev ; dev=dev->next) {
- if ((dev->start <= flash_base) && ( flash_base <= dev->end)) {
+ int stat = CYG_FLASH_ERR_OK;
+
+ dev = find_dev(flash_base, &stat);
+ if (dev) {
info->start = dev->start;
info->end = dev->end;
info->num_block_infos = dev->num_block_infos;
info->block_info = dev->block_info;
- return CYG_FLASH_ERR_OK;
- }
}
- return CYG_FLASH_ERR_INVALID;
+ return stat;
}
#ifdef CYGPKG_KERNEL
@@ -279,52 +339,45 @@
__externC int
cyg_flash_mutex_lock(const cyg_flashaddr_t from, const size_t len)
{
- struct cyg_flash_dev * dev;
- int err;
-
- if (!init) return CYG_FLASH_ERR_NOT_INIT;
-
- for (dev = flash_head; dev ; dev=dev->next) {
- if ((dev->start <= from) && ( from <= dev->end)) {
- cyg_mutex_lock(&dev->mutex);
- if (dev->end > from+len) {
- return CYG_FLASH_ERR_OK;
- } else { // Check off by one?
- //The region is bigger than this driver. Recurse
- err = cyg_flash_mutex_lock(dev->end+1, (len - (dev->end - from)));
- if (err != CYG_FLASH_ERR_OK) {
- // Something went wrong, unlock what we just locked
- cyg_mutex_unlock(&dev->mutex);
- }
- return err;
+ struct cyg_flash_dev * dev;
+ int stat = CYG_FLASH_ERR_OK;
+
+ dev = find_dev(from, &stat);
+ if (dev) {
+ LOCK(dev);
+ if (len > (dev->end + 1 - from)) {
+ stat = cyg_flash_mutex_lock(dev->end + 1, len - (dev->end + 1 - from));
+ if (CYG_FLASH_ERR_OK != stat) {
+ // Something went wrong, unlock what we just locked
+ UNLOCK(dev);
}
}
}
- return CYG_FLASH_ERR_INVALID;
+ return stat;
}
// Unlock the mutex's for a range of addresses
__externC int
cyg_flash_mutex_unlock(const cyg_flashaddr_t from, const size_t len)
{
- struct cyg_flash_dev * dev;
- int err;
-
- if (!init) return CYG_FLASH_ERR_NOT_INIT;
-
- for (dev = flash_head; dev ; dev=dev->next) {
- if ((dev->start <= from) && ( from <= dev->end)) {
- cyg_mutex_unlock(&dev->mutex);
- if (dev->end > from+len) {
- return CYG_FLASH_ERR_OK;
- } else { // Check off by one?
- //The region is bigger than this driver. Recurse
- err = cyg_flash_mutex_unlock(dev->end+1, (len - (dev->end - from)));
- return err;
+ struct cyg_flash_dev * dev;
+ int stat = CYG_FLASH_ERR_OK;
+
+ dev = find_dev(from, &stat);
+ if (dev) {
+ UNLOCK(dev);
+ if (len > (dev->end + 1 - from)) {
+ stat = cyg_flash_mutex_lock(dev->end + 1, len - (dev->end + 1 - from));
+ if (CYG_FLASH_ERR_OK != stat) {
+ // Something went wrong, relock what we just unlocked. This may not
+ // be worth it since things must be pretty messed up, and could
+ // conceivably end in deadlock if there is a concurrent call to
+ // cyg_flash_mutex_lock();
+ LOCK(dev);
}
}
}
- return CYG_FLASH_ERR_INVALID;
+ return stat;
}
#endif
@@ -335,7 +388,6 @@
int i;
size_t offset;
-
CYG_ASSERT((addr >= dev->start) && (addr <= dev->end), "Not inside device");
offset = addr - dev->start;
@@ -353,16 +405,11 @@
__externC size_t
cyg_flash_block_size(const cyg_flashaddr_t flash_base)
{
- struct cyg_flash_dev * dev;
-
- if (!init) return CYG_FLASH_ERR_NOT_INIT;
+ struct cyg_flash_dev * dev;
+ int stat;
- for (dev = flash_head;
- dev && !((dev->start <= flash_base) && ( flash_base <= dev->end));
- dev=dev->next)
- ;
- if (!dev) return CYG_FLASH_ERR_INVALID;
-
+ dev = find_dev(flash_base, &stat);
+ if (!dev) return stat;
return flash_block_size(dev, flash_base);
}
@@ -393,14 +440,9 @@
size_t erase_count;
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) && ( flash_base <= dev->end));
- dev=dev->next)
- ;
- if (!dev) return CYG_FLASH_ERR_INVALID;
+
+ dev = find_dev(flash_base, &stat);
+ if (!dev) return stat;
CHECK_SOFT_WRITE_PROTECT(flash_base, len);
@@ -489,14 +531,9 @@
size_t write_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) && ( flash_base <= dev->end));
- dev=dev->next)
- ;
- if (!dev) return CYG_FLASH_ERR_INVALID;
+ dev = find_dev(flash_base, &stat);
+ if (!dev) return stat;
CHECK_SOFT_WRITE_PROTECT(flash_base, len);
@@ -577,14 +614,9 @@
unsigned char * ram = (unsigned char *)ram_base;
size_t read_count;
int stat = CYG_FLASH_ERR_OK;
-
- if (!init) return CYG_FLASH_ERR_NOT_INIT;
-
- for (dev = flash_head;
- dev && !((dev->start <= flash_base) && ( flash_base <= dev->end));
- dev=dev->next)
- ;
- if (!dev) return CYG_FLASH_ERR_INVALID;
+
+ dev = find_dev(flash_base, &stat);
+ if (!dev) return stat;
LOCK(dev);
addr = flash_base;
@@ -673,14 +705,9 @@
size_t lock_count;
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) && ( flash_base <= dev->end));
- dev=dev->next)
- ;
- if (!dev) return CYG_FLASH_ERR_INVALID;
+ dev = find_dev(flash_base, &stat);
+ if (!dev) return stat;
if (!dev->funs->flash_block_lock) return CYG_FLASH_ERR_INVALID;
CHECK_SOFT_WRITE_PROTECT(flash_base, len);
@@ -742,14 +769,9 @@
size_t unlock_count;
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) && ( flash_base <= dev->end));
- dev=dev->next)
- ;
- if (!dev) return CYG_FLASH_ERR_INVALID;
+ dev = find_dev(flash_base, &stat);
+ if (!dev) return stat;
if (!dev->funs->flash_block_unlock) return CYG_FLASH_ERR_INVALID;
CHECK_SOFT_WRITE_PROTECT(flash_base, len);