fs/spiffs: The MTD interfaces needs its own, dedicated page buffer for read-modify-write operations. It cannot share the normal 'working' page buffer because it is sometimes in-use when the MTD interface needs it. Also and more DEBUG output and controls for the MTD interface.

fs/spiffs/Kconfig

@@ -38,6 +38,8 @@ config SPIFFS_CACHEDBG
 	bool "Enable cache debug output"
 	default n
 	depends on DEBUG_FS_INFO
+	---help---
+		Dumps cache debug output.  Depends on CONFIG_DEBUG_FS_INFO
 
 comment "Garbage Collection (GC) Options"
 
@@ -95,6 +97,8 @@ config SPIFFS_GCDBG
 	bool "Enable garbage collection debug output"
 	default n
 	depends on DEBUG_FS_INFO
+	---help---
+		Dumps garbage collection debug output.  Depends on CONFIG_DEBUG_FS_INFO
 
 comment "Consistency Check Options"
 
@@ -110,6 +114,24 @@ config SPIFFS_CHECK_OUTPUT
 	bool "Enable consistency check SYSLOG output"
 	default n
 
+comment "MTD Interface Options"
+
+config SPIFFS_MTDDBG
+	bool "Enable MTD Interface DEBUG"
+	default n
+	depends on DEBUG_FS_INFO
+	---help---
+		Dumps MTD debug output.  Depends on CONFIG_DEBUG_FS_INFO
+
+config SPIFFS_MTDDUMP
+	bool "Enable MTD Data Dump"
+	default n
+	depends on SPIFFS_MTDDBG
+	---help---
+		Dumps every byte of data going to and coming from FLASH.  You
+		probably do not want this unless you are debugging a particular
+		MTD interfacing issue.
+
 comment "SPIFFS Core Options"
 
 config SPIFFS_NO_BLIND_WRITES

fs/spiffs/src/spiffs.h

@@ -145,6 +145,7 @@ struct spiffs_s
   FAR struct mtd_dev_s *mtd;        /* The contained MTD interface */
   FAR uint8_t *lu_work;             /* Primary work buffer, size of a logical page */
   FAR uint8_t *work;                /* Secondary work buffer, size of a logical page */
+  FAR uint8_t *mtd_work;            /* MTD I/O buffer for read-modify-write */
   FAR void *cache;                  /* Cache memory */
 #ifdef CONFIG_HAVE_LONG_LONG
   off64_t media_size;               /* Physical size of the SPI flash */

fs/spiffs/src/spiffs_cache.c

@@ -400,6 +400,9 @@ ssize_t spiffs_cache_read(FAR struct spiffs_s *fs, uint8_t op, int16_t objid,
   FAR struct spiffs_cache_page_s *cp;
   int ret = OK;
 
+  spiffs_cacheinfo("op=%02x, objid=%04x addr=%ld len=%lu\n",
+                   op, objid, (long)addr, (unsigned long)len);
+
   cache = spiffs_get_cache(fs);
   cp    = spiffs_cache_page_get(fs, SPIFFS_PADDR_TO_PAGE(fs, addr));
 
@@ -512,6 +515,9 @@ size_t spiffs_cache_write(FAR struct spiffs_s *fs, uint8_t op, int16_t objid,
   int16_t pgndx;
   int ret = OK;
 
+  spiffs_cacheinfo("op=%02x, objid=%04x addr=%ld len=%lu\n",
+                   op, objid, (long)addr, (unsigned long)len);
+
   pgndx = SPIFFS_PADDR_TO_PAGE(fs, addr);
   cache = spiffs_get_cache(fs);
   cp    = spiffs_cache_page_get(fs, pgndx);

fs/spiffs/src/spiffs_core.h

@@ -361,7 +361,7 @@ begin_packed_struct struct spiffs_page_header_s
   uint8_t flags;                        /* flags */
 } end_packed_struct;
 
-/* object index header page header */
+/* Object index header page header */
 
 begin_packed_struct struct spiffs_pgobj_ndxheader_s
 {
@@ -371,7 +371,7 @@ begin_packed_struct struct spiffs_pgobj_ndxheader_s
   uint8_t name[CONFIG_SPIFFS_NAME_MAX]; /* name of object */
 } end_packed_struct;
 
-/* object index page header */
+/* Object index page header */
 
 begin_packed_struct struct spiffs_page_objndx_s
 {

fs/spiffs/src/spiffs_mtd.c

@@ -52,9 +52,21 @@
 #include "spiffs_mtd.h"
 
 /****************************************************************************
- * Private Data
+ * Private Functions
  ****************************************************************************/
 
+#ifdef CONFIG_SPIFFS_MTDDUMP
+static inline void spiffs_mtd_dump(FAR const char *msg,
+                                   FAR const uint8_t *buffer,
+                                   unsigned int buflen)
+{
+  lib_dumpbuffer(msg, buffer, buflen);
+}
+
+#else
+#  define spiffs_mtd_dump(m,b,l)
+#endif
+
 /****************************************************************************
  * Public Functions
  ****************************************************************************/
@@ -80,16 +92,17 @@
 ssize_t spiffs_mtd_write(FAR struct spiffs_s *fs, off_t offset, size_t len,
                          FAR const uint8_t *src)
 {
-  int16_t blksize;
-  size_t blkoffset;
   size_t remaining;
-  ssize_t nblocks;
   ssize_t ret;
   off_t blkmask;
   off_t blkstart;
   off_t blkend;
+  int16_t blksize;
+  int16_t nblocks;
+  int16_t blkoffset;
 
-  finfo("offset=%ld len=%lu\n", (long)offset, (unsigned long)len);
+  spiffs_mtdinfo("offset=%ld len=%lu\n", (long)offset, (unsigned long)len);
+  spiffs_mtd_dump("Writing", src, len);
 
   DEBUGASSERT(fs != NULL && fs->mtd != NULL && src != NULL && len > 0);
 
@@ -118,8 +131,8 @@ ssize_t spiffs_mtd_write(FAR struct spiffs_s *fs, off_t offset, size_t len,
       blkstart  = offset / blksize;
       blkend    = (offset + len - 1) / blksize;
 
-      finfo("blkoffset=%lu blkstart=%ld blkend=%ld\n",
+      spiffs_mtdinfo("blkoffset=%d blkstart=%ld blkend=%ld\n",
-            blkoffset, blkstart, blkend);
+                     blkoffset, (long)blkstart, (long)blkend);
 
       /* Check if we have to do a read-modify-write on the first block.  We
        * need to do this if the blkoffset is not zero.  In that case we need
@@ -128,11 +141,10 @@ ssize_t spiffs_mtd_write(FAR struct spiffs_s *fs, off_t offset, size_t len,
 
       if (blkoffset != 0)
         {
-          FAR uint8_t *wptr = fs->work;
+          FAR uint8_t *wptr = fs->mtd_work;
-          size_t maxcopy;
+          int16_t maxcopy;
-          ssize_t nbytes;
+          int16_t nbytes;
 
-#warning "REVISIT: is fs->work available here?"
           ret = MTD_BREAD(fs->mtd, blkstart, 1, wptr);
           if (ret < 0)
             {
@@ -140,15 +152,23 @@ ssize_t spiffs_mtd_write(FAR struct spiffs_s *fs, off_t offset, size_t len,
               return (ssize_t)ret;
             }
 
+          spiffs_mtd_dump("Read leading partial block", wptr, blksize);
+
           /* Copy the data in place */
 
           maxcopy = blksize - blkoffset;
           nbytes  = MIN(remaining, maxcopy);
 
+          spiffs_mtdinfo("Leading partial block: "
+                         "blkstart=%ld blkoffset=%d nbytes=%d\n",
+                         (long)blkstart, blkoffset, nbytes);
+
           memcpy(&wptr[blkoffset], src, nbytes);
 
           /* Write back the modified block */
 
+          spiffs_mtd_dump("Write leading partial block", wptr, blksize);
+
           ret = MTD_BWRITE(fs->mtd, blkstart, 1, wptr);
           if (ret < 0)
             {
@@ -174,12 +194,14 @@ ssize_t spiffs_mtd_write(FAR struct spiffs_s *fs, off_t offset, size_t len,
           nblocks++;
         }
 
-      finfo("Whole blocks=%d blkstart=%lu remaining=%lu\n",
+      spiffs_mtdinfo("Whole blocks=%d blkstart=%lu remaining=%lu\n",
                      nblocks, (unsigned long)blkstart,
                      (unsigned long)remaining);
 
       if (nblocks > 0)
         {
+          spiffs_mtd_dump("Write whole blocks", src, blksize * nblocks);
+
           ret = MTD_BWRITE(fs->mtd, blkstart, nblocks, src);
           if (ret < 0)
             {
@@ -202,21 +224,30 @@ ssize_t spiffs_mtd_write(FAR struct spiffs_s *fs, off_t offset, size_t len,
 
       if (remaining > 0)
         {
-#warning "REVISIT: is fs->work available here?"
+          ret = MTD_BREAD(fs->mtd, blkend, 1, fs->mtd_work);
-          ret = MTD_BREAD(fs->mtd, blkend, 1, fs->work);
           if (ret < 0)
             {
               ferr("ERROR: MTD_BREAD() failed: %d\n", ret);
               return (ssize_t)ret;
             }
 
+          spiffs_mtd_dump("Read trailing partial block",
+                          fs->mtd_work, blksize);
+
           /* Copy the data in place */
 
-          memcpy(fs->work, src, remaining);
+          spiffs_mtdinfo("Trailing partial block: "
+                         "blkend=%ld remaining=%lu\n",
+                         (long)blkend, (unsigned long)remaining);
+
+          memcpy(fs->mtd_work, src, remaining);
 
           /* Write back the modified block */
 
-          ret = MTD_BWRITE(fs->mtd, blkend, 1, fs->work);
+          spiffs_mtd_dump("Write trailing partial block",
+                          fs->mtd_work, blksize);
+
+          ret = MTD_BWRITE(fs->mtd, blkend, 1, fs->mtd_work);
           if (ret < 0)
             {
               ferr("ERROR: MTD_BWRITE() failed: %d\n", ret);
@@ -249,16 +280,19 @@ ssize_t spiffs_mtd_write(FAR struct spiffs_s *fs, off_t offset, size_t len,
 ssize_t spiffs_mtd_read(FAR struct spiffs_s *fs, off_t offset, size_t len,
                         FAR uint8_t *dest)
 {
-  int16_t blksize;
+#ifdef CONFIG_SPIFFS_MTDDUMP
-  int16_t nblocks;
+  FAR uint8_t *saved_dest = dest;
-  size_t blkoffset;
+#endif
   size_t remaining;
   ssize_t ret;
   off_t blkmask;
   off_t blkstart;
   off_t blkend;
+  int16_t blksize;
+  int16_t nblocks;
+  int16_t blkoffset;
 
-  finfo("offset=%ld len=%lu\n", (long)offset, (unsigned long)len);
+  spiffs_mtdinfo("offset=%ld len=%lu\n", (long)offset, (unsigned long)len);
 
   DEBUGASSERT(fs != NULL && fs->mtd != NULL && dest != NULL && len > 0);
 
@@ -285,8 +319,8 @@ ssize_t spiffs_mtd_read(FAR struct spiffs_s *fs, off_t offset, size_t len,
       blkstart  = offset / blksize;
       blkend    = (offset + len - 1) / blksize;
 
-      finfo("blkoffset=%lu blkstart=%ld blkend=%ld\n",
+      spiffs_mtdinfo("blkoffset=%d blkstart=%ld blkend=%ld\n",
-            blkoffset, blkstart, blkend);
+                     blkoffset, (long)blkstart, (long)blkend);
 
       /* Check if we have to do a partial read on the first block.  We
        * need to do this if the blkoffset is not zero.  In that case we need
@@ -295,11 +329,10 @@ ssize_t spiffs_mtd_read(FAR struct spiffs_s *fs, off_t offset, size_t len,
 
       if (blkoffset != 0)
         {
-          FAR uint8_t *wptr = fs->work;
+          FAR uint8_t *wptr = fs->mtd_work;
           size_t maxcopy;
           ssize_t nbytes;
 
-#warning "REVISIT: is fs->work available here?"
           ret = MTD_BREAD(fs->mtd, blkstart, 1, wptr);
           if (ret < 0)
             {
@@ -307,11 +340,17 @@ ssize_t spiffs_mtd_read(FAR struct spiffs_s *fs, off_t offset, size_t len,
               return (ssize_t)ret;
             }
 
+          spiffs_mtd_dump("Read leading partial block", wptr, blksize);
+
           /* Copy the data from the block */
 
           maxcopy = blksize - blkoffset;
           nbytes  = MIN(remaining, maxcopy);
 
+          spiffs_mtdinfo("Leading partial block: "
+                         "blkstart=%ld blkoffset=%d nbytes=%d\n",
+                         (long)blkstart, blkoffset, nbytes);
+
           memcpy(dest, &wptr[blkoffset], nbytes);
 
           /* Update block numbers and counts */
@@ -332,7 +371,7 @@ ssize_t spiffs_mtd_read(FAR struct spiffs_s *fs, off_t offset, size_t len,
           nblocks++;
         }
 
-      finfo("Whole blocks=%d blkstart=%lu remaining=%lu\n",
+      spiffs_mtdinfo("Whole blocks=%d blkstart=%lu remaining=%lu\n",
                      nblocks, (unsigned long)blkstart,
                      (unsigned long)remaining);
 
@@ -345,6 +384,8 @@ ssize_t spiffs_mtd_read(FAR struct spiffs_s *fs, off_t offset, size_t len,
               return (ssize_t)ret;
             }
 
+          spiffs_mtd_dump("Read whole blocks", dest, blksize * nblocks);
+
           dest      += (remaining & ~blkmask);
           remaining  = (remaining & blkmask);
 
@@ -360,8 +401,7 @@ ssize_t spiffs_mtd_read(FAR struct spiffs_s *fs, off_t offset, size_t len,
 
       if (remaining > 0)
         {
-#warning "REVISIT: is fs->work available here?"
+          ret = MTD_BREAD(fs->mtd, blkend, 1, fs->mtd_work);
-          ret = MTD_BREAD(fs->mtd, blkend, 1, fs->work);
           if (ret < 0)
             {
               ferr("ERROR: MTD_BREAD() failed: %d\n", ret);
@@ -370,10 +410,17 @@ ssize_t spiffs_mtd_read(FAR struct spiffs_s *fs, off_t offset, size_t len,
 
           /* Copy the data from the block */
 
-          memcpy(dest, fs->work, remaining);
+          spiffs_mtdinfo("Trailing partial block: "
+                         "blkend=%ld remaining=%lu\n",
+                         (long)blkend, (unsigned long)remaining);
+          spiffs_mtd_dump("Read trailing partial block",
+                          fs->mtd_work, blksize);
+
+          memcpy(dest, fs->mtd_work, remaining);
         }
     }
 
+  spiffs_mtd_dump("Read", saved_dest, len);
   return (ssize_t)len;
 }
 
@@ -401,7 +448,7 @@ ssize_t spiffs_mtd_erase(FAR struct spiffs_s *fs, off_t offset, size_t len)
   off_t eblkend;
   ssize_t nerased;
 
-  finfo("offset=%ld len=%lu\n", (long)offset, (unsigned long)len);
+  spiffs_mtdinfo("offset=%ld len=%lu\n", (long)offset, (unsigned long)len);
 
   DEBUGASSERT(fs != NULL && fs->mtd != NULL);
 

fs/spiffs/src/spiffs_mtd.h

@@ -66,6 +66,24 @@ extern "C"
 #define SPIFFS_GEO_PAGE_SIZE(fs)        ((fs)->geo.blocksize)
 #define SPIFFS_GEO_PAGES_PER_BLOCK(fs)  ((fs)->pages_per_block)
 
+/* Debug */
+
+#ifdef CONFIG_SPIFFS_MTDDBG
+#  ifdef CONFIG_CPP_HAVE_VARARGS
+#    define spiffs_mtdinfo(format, ...) _info(format, ##__VA_ARGS__)
+#  else
+#    define spiffs_mtdinfo              _info
+#  endif
+#else
+#  ifdef CONFIG_CPP_HAVE_VARARGS
+#    define spiffs_mtdinfo(format, ...)
+#  else
+#    define spiffs_mtdinfo              (void)
+#  endif
+#endif
+
+/* Commonly used Macros */
+
 #ifndef MIN
 #  define MIN(a,b) ((a) < (b) ? (a) : (b))
 #endif

fs/spiffs/src/spiffs_vfs.c

@@ -1355,13 +1355,16 @@ static int spiffs_bind(FAR struct inode *mtdinode, FAR const void *data,
 
   spiffs_cache_initialize(fs);
 
-  /* Allocate the memory work buffer comprising 2*config->page_size bytes
+  /* Allocate the memory work buffer comprising 3*config->page_size bytes
    * used throughout all file system operations.
    *
    * NOTE: Currently page size is equivalent to block size.
+   *
+   * REVISIT:  The MTD work buffer was added.  With some careful analysis,
+   * it should, however, be possible to get by with fewer page buffers.
    */
 
-  work_size = SPIFFS_GEO_PAGE_SIZE(fs) << 1;
+  work_size = 3 * SPIFFS_GEO_PAGE_SIZE(fs);
   work      = (FAR uint8_t *)kmm_malloc(work_size);
 
   if (work == NULL)
@@ -1372,7 +1375,8 @@ static int spiffs_bind(FAR struct inode *mtdinode, FAR const void *data,
     }
 
   fs->work      = &work[0];
-  fs->lu_work      = &work[work_size >> 1];
+  fs->lu_work   = &work[SPIFFS_GEO_PAGE_SIZE(fs)];
+  fs->mtd_work  = &work[2 * SPIFFS_GEO_PAGE_SIZE(fs)];
 
   (void)nxsem_init(&fs->exclsem.sem, 0, 1);