This is the mail archive of the ecos-patches@sources.redhat.com mailing list for the eCos project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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);


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]