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 - cut down on the #ifdef's in the code


This patch eliminates most of the #ifdef's in the main code by
encapsulating the functionality in macros. The resulting code is
easier to read IMO, and may offer opportunities in future for other
clean-ups. For example there is an issue of mutex locking inside
RedBoot's fis/fconfig code when accessed via a virtual vector from an
application that also manipulates the flash. I am not sure yet how to
tackle that one.

Bart

Index: ChangeLog
===================================================================
RCS file: /cvs/ecos/ecos/packages/io/flash/current/ChangeLog,v
retrieving revision 1.38.2.12
diff -u -r1.38.2.12 ChangeLog
--- ChangeLog	20 Nov 2004 23:32:00 -0000	1.38.2.12
+++ ChangeLog	21 Nov 2004 11:18:52 -0000
@@ -1,3 +1,8 @@
+2004-11-21  Bart Veer  <bartv@ecoscentric.com>
+
+	* src/flash.c: encapsulate various optional bits of code in
+	macros, to cut down on the #ifdef's in the main code
+
 2004-11-20  Bart Veer  <bartv@ecoscentric.com>
 
 	* cdl/io_flash.cdl, src/flash.c(cyg_flash_read): add an interface
Index: src/flash.c
===================================================================
RCS file: /cvs/ecos/ecos/packages/io/flash/current/src/flash.c,v
retrieving revision 1.26.2.8
diff -u -r1.26.2.8 flash.c
--- src/flash.c	20 Nov 2004 23:31:59 -0000	1.26.2.8
+++ src/flash.c	21 Nov 2004 11:13:25 -0000
@@ -76,6 +76,36 @@
 # warning "Can only enable the flash debugging when configured for RAM startup"
 #endif
 
+// Optional verbosity. Using a macro here avoids lots of ifdefs in the
+// rest of the code.
+#ifdef CYGSEM_IO_FLASH_CHATTER
+# define CHATTER(_dev_, _fmt_, ...) (*(_dev_)->pf)((_fmt_), ## __VA_ARGS__)
+#else
+# define CHATTER(_dev_, _fmt_, ...) CYG_EMPTY_STATEMENT
+#endif
+
+// Per-thread locking. Again using macros avoids lots of ifdefs
+#ifdef CYGPKG_KERNEL
+# define LOCK_INIT(_dev_)   cyg_mutex_init(&((_dev_)->mutex))
+# define LOCK(_dev_)        cyg_mutex_lock(&((_dev_)->mutex))
+# define UNLOCK(_dev_)      cyg_mutex_unlock(&((_dev_)->mutex))
+#else
+# define LOCK_INIT(_dev_)   CYG_EMPTY_STATEMENT
+# define LOCK(_dev_)        CYG_EMPTY_STATEMENT
+# define UNLOCK(_dev_)      CYG_EMPTY_STATEMENT
+#endif
+
+// Software write-protect. Very rarely used.
+#ifdef CYGSEM_IO_FLASH_SOFT_WRITE_PROTECT
+# define CHECK_SOFT_WRITE_PROTECT(_addr_, _len_)    \
+  CYG_MACRO_START                                   \
+  if (plf_flash_query_soft_wp((_addr_), (_len_)))   \
+    return CYG_FLASH_ERR_PROTECT;                   \
+  CYG_MACRO_END
+#else
+#define CHECK_SOFT_WRITE_PROTECT(_addr_, _len_) CYG_EMPTY_STATEMENT
+#endif
+
 // Has the FLASH IO library been initialise?
 static bool init = false;
 // We have a linked list of flash drivers. This is the head of the
@@ -151,9 +181,7 @@
   
   for (dev = &cyg_flashdevtab[0]; dev != &cyg_flashdevtab_end; dev++) {
     dev->pf = pf;
-#ifdef CYGPKG_KERNEL
-    cyg_mutex_init(&dev->mutex);
-#endif
+    LOCK_INIT(dev);
     
     err = dev->funs->flash_init(dev);
     if (err != CYG_FLASH_ERR_OK) {
@@ -259,7 +287,7 @@
 }
 
 #ifdef CYGPKG_KERNEL
-//Lock the mutex's for a range of addresses
+// Lock the mutex's for a range of addresses
 __externC int
 cyg_flash_mutex_lock(const cyg_flashaddr_t from, const size_t len) 
 {
@@ -287,7 +315,7 @@
   return CYG_FLASH_ERR_INVALID;
 }
 
-//Unlock the mutex's for a range of addresses
+// Unlock the mutex's for a range of addresses
 __externC int
 cyg_flash_mutex_unlock(const cyg_flashaddr_t from, const size_t len) 
 {
@@ -380,20 +408,15 @@
   
   if (!init) return CYG_FLASH_ERR_NOT_INIT;
   
-#ifdef CYGSEM_IO_FLASH_SOFT_WRITE_PROTECT
-  if (plf_flash_query_soft_wp(flash_base,len))
-    return CYG_FLASH_ERR_PROTECT;
-#endif
-  
   for (dev = flash_head; 
        dev && !((dev->start <= flash_base) && ( flash_base <= dev->end));
        dev=dev->next)
     ;
   if (!dev) return CYG_FLASH_ERR_INVALID;
+
+  CHECK_SOFT_WRITE_PROTECT(flash_base, len);
   
-#ifdef CYGPKG_KERNEL
-  cyg_mutex_lock(&dev->mutex);
-#endif
+  LOCK(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
@@ -407,9 +430,7 @@
   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
+  CHATTER(dev, "... Erase from %p-%p: ", (void*)block, (void*)end_addr);
   
   HAL_FLASH_CACHES_OFF(d_cache, i_cache);
   FLASH_Enable(flash_base, end_addr);
@@ -446,18 +467,12 @@
     }
     block       += block_size;
     erase_count -= block_size;
-#ifdef CYGSEM_IO_FLASH_CHATTER
-    dev->pf(".");
-#endif
+    CHATTER(dev, ".");
   }
   FLASH_Disable(flash_base, end_addr);
   HAL_FLASH_CACHES_ON(d_cache, i_cache);
-#ifdef CYGSEM_IO_FLASH_CHATTER
-  dev->pf("\n");
-#endif
-#ifdef CYGPKG_KERNEL
-  cyg_mutex_unlock(&dev->mutex);
-#endif
+  CHATTER(dev, "\n");
+  UNLOCK(dev);
   if (stat != CYG_FLASH_ERR_OK) {
     return stat;
   }
@@ -488,21 +503,16 @@
   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))
-    return CYG_FLASH_ERR_PROTECT;
-#endif
-  
+
   for (dev = flash_head; 
        dev && !((dev->start <= flash_base) && ( flash_base <= dev->end));
        dev=dev->next)
     ;
   if (!dev) return CYG_FLASH_ERR_INVALID;
+
+  CHECK_SOFT_WRITE_PROTECT(flash_base, len);
   
-#ifdef CYGPKG_KERNEL
-  cyg_mutex_lock(&dev->mutex);
-#endif
+  LOCK(dev);
   addr = flash_base;
   if (len > (dev->end + 1 - flash_base)) {
     end_addr = dev->end;
@@ -520,10 +530,7 @@
       offset = addr - block;
   }
   
-#ifdef CYGSEM_IO_FLASH_CHATTER
-  dev->pf("... Program from %p-%p to %p: ", ram_base, 
-          ((CYG_ADDRESS)ram_base)+write_count, addr);
-#endif
+  CHATTER(dev, "... Program from %p-%p to %p: ", ram_base, ((CYG_ADDRESS)ram_base)+write_count, addr);
   
   HAL_FLASH_CACHES_OFF(d_cache, i_cache);
   FLASH_Enable(flash_base, end_addr);
@@ -544,30 +551,22 @@
     if (CYG_FLASH_ERR_OK == stat) // Claims to be OK
       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");
-#endif
+        CHATTER(dev, "V");
       }
 #endif
     if (CYG_FLASH_ERR_OK != stat && err_address) {
       *err_address = addr;
       break;
     }
-#ifdef CYGSEM_IO_FLASH_CHATTER
-    dev->pf(".");
-#endif
+    CHATTER(dev, ".");
     write_count -= this_write;
     addr        += this_write;
     ram         += this_write;
   }
   FLASH_Disable(flash_base, end_addr);
   HAL_FLASH_CACHES_ON(d_cache, i_cache);
-#ifdef CYGSEM_IO_FLASH_CHATTER
-    dev->pf("\n");
-#endif
-#ifdef CYGPKG_KERNEL
-  cyg_mutex_unlock(&dev->mutex);
-#endif
+  CHATTER(dev, "\n");
+  UNLOCK(dev);
   if (stat != CYG_FLASH_ERR_OK) {
     return (stat);
   }
@@ -599,9 +598,7 @@
     ;
   if (!dev) return CYG_FLASH_ERR_INVALID;
 
-#ifdef CYGPKG_KERNEL
-  cyg_mutex_lock(&dev->mutex);
-#endif
+  LOCK(dev);
   addr = flash_base;
   if (len > (dev->end + 1 - flash_base)) {
       end_addr = dev->end;
@@ -610,9 +607,7 @@
   }
   read_count = (end_addr + 1) - flash_base;
 
-#ifdef CYGSEM_IO_FLASH_CHATTER
-  dev->pf("... Read from %p-%p to %p: ", addr, end_addr, ram_base);
-#endif
+  CHATTER(dev, "... Read from %p-%p to %p: ", addr, end_addr, ram_base);
 
   // If the flash is directly accessible, just read it in one go. This
   // still happens with the mutex locked to protect against concurrent
@@ -654,9 +649,7 @@
               *err_address = addr;
               break;
           }
-#ifdef CYGSEM_IO_FLASH_CHATTER
-          dev->pf(".");
-#endif
+          CHATTER(dev, ".");
           read_count  -= this_read;
           addr        += this_read;
           ram         += this_read;
@@ -665,12 +658,8 @@
       HAL_FLASH_CACHES_ON(d_cache, i_cache);
 #endif      
   }
-#ifdef CYGSEM_IO_FLASH_CHATTER
-  dev->pf("\n");
-#endif
-#ifdef CYGPKG_KERNEL
-  cyg_mutex_unlock(&dev->mutex);
-#endif
+  CHATTER(dev, "\n");
+  UNLOCK(dev);
   if (stat != CYG_FLASH_ERR_OK) {
     return (stat);
   }
@@ -695,12 +684,7 @@
   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))
-    return CYG_FLASH_ERR_PROTECT;
-#endif
-  
+
   for (dev = flash_head; 
        dev && !((dev->start <= flash_base) && ( flash_base <= dev->end));
        dev=dev->next)
@@ -708,9 +692,9 @@
   if (!dev) return CYG_FLASH_ERR_INVALID;
   if (!dev->funs->flash_block_lock) return CYG_FLASH_ERR_INVALID;
 
-#ifdef CYGPKG_KERNEL
-  cyg_mutex_lock(&dev->mutex);
-#endif
+  CHECK_SOFT_WRITE_PROTECT(flash_base, len);
+  
+  LOCK(dev);
   if (len > (dev->end + 1 - flash_base)) {
       end_addr = dev->end;
   } else {
@@ -719,9 +703,7 @@
   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
+  CHATTER(dev, "... Locking from %p-%p: ", (void*)block, (void*)end_addr);
   
   HAL_FLASH_CACHES_OFF(d_cache, i_cache);
   FLASH_Enable(flash_base, end_addr);
@@ -739,18 +721,12 @@
     }
     block       += block_size;
     lock_count  -= block_size;
-#ifdef CYGSEM_IO_FLASH_CHATTER
-    dev->pf(".");
-#endif
+    CHATTER(dev, ".");
   }
   FLASH_Disable(flash_base, end_addr);
   HAL_FLASH_CACHES_ON(d_cache, i_cache);
-#ifdef CYGSEM_IO_FLASH_CHATTER
-  dev->pf("\n");
-#endif
-#ifdef CYGPKG_KERNEL
-  cyg_mutex_unlock(&dev->mutex);
-#endif
+  CHATTER(dev, "\n");
+  UNLOCK(dev);
   if (stat != CYG_FLASH_ERR_OK) {
     return stat;
   }
@@ -777,22 +753,17 @@
   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))
-    return CYG_FLASH_ERR_PROTECT;
-#endif
-  
+
   for (dev = flash_head; 
        dev && !((dev->start <= flash_base) && ( flash_base <= dev->end));
        dev=dev->next)
     ;
   if (!dev) return CYG_FLASH_ERR_INVALID;
   if (!dev->funs->flash_block_unlock) return CYG_FLASH_ERR_INVALID;
+
+  CHECK_SOFT_WRITE_PROTECT(flash_base, len);
   
-#ifdef CYGPKG_KERNEL
-  cyg_mutex_lock(&dev->mutex);
-#endif
+  LOCK(dev);
   if (len > (dev->end + 1 - flash_base)) {
       end_addr = dev->end;
   } else {
@@ -801,9 +772,7 @@
   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
+  CHATTER(dev, "... Unlocking from %p-%p: ", (void*)block, (void*)end_addr);
   
   HAL_FLASH_CACHES_OFF(d_cache, i_cache);
   FLASH_Enable(flash_base, end_addr);
@@ -822,18 +791,12 @@
     block           += block_size;
     unlock_count    -= block_size;
     
-#ifdef CYGSEM_IO_FLASH_CHATTER
-    dev->pf(".");
-#endif
+    CHATTER(dev, ".");
   }
   FLASH_Disable(flash_base, end_addr);
   HAL_FLASH_CACHES_ON(d_cache, i_cache);
-#ifdef CYGSEM_IO_FLASH_CHATTER
-  dev->pf("\n");
-#endif
-#ifdef CYGPKG_KERNEL
-  cyg_mutex_unlock(&dev->mutex);
-#endif
+  CHATTER(dev, "\n");
+  UNLOCK(dev);
   if (stat != CYG_FLASH_ERR_OK) {
     return stat;
   }


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