Change maximum depth to 2^15 which allows for a fast modulo substitute.

Thus, however, overflows are detectable only for one time FIFO depth.

2 files changed, 37 insertions, 40 deletions

diff --git a/src/common/tusb_fifo.c b/src/common/tusb_fifo.c
index b83dfee4e..84fc9bb8b 100644
--- a/src/common/tusb_fifo.c
+++ b/src/common/tusb_fifo.c
@@ -58,6 +58,8 @@ static void tu_fifo_unlock(tu_fifo_t *f)
 
 bool tu_fifo_config(tu_fifo_t *f, void* buffer, uint16_t depth, uint16_t item_size, bool overwritable)
 {
+  if (depth > 0x8000) return false;               // Maximum depth is 2^15 items
+
   tu_fifo_lock(f);
 
   f->buffer = (uint8_t*) buffer;
@@ -65,8 +67,8 @@ bool tu_fifo_config(tu_fifo_t *f, void* buffer, uint16_t depth, uint16_t item_si
   f->item_size = item_size;
   f->overwritable = overwritable;
 
-  f->non_used_index_space = 0x10000 % depth;
-  f->max_pointer_idx = 0xFFFF - f->non_used_index_space;
+  f->max_pointer_idx = 2*depth - 1;               // Limit index space to 2*depth - this allows for a fast "modulo" calculation but limits the maximum depth to 2^16/2 = 2^15 and buffer overflows are detectable only if overflow happens once (important for unsupervised DMA applications)
+  f->non_used_index_space = 0xFFFF - f->max_pointer_idx;
 
   f->rd_idx = f->wr_idx = 0;
 
@@ -79,7 +81,9 @@ bool tu_fifo_config(tu_fifo_t *f, void* buffer, uint16_t depth, uint16_t item_si
 
 static inline uint16_t _ff_mod(uint16_t idx, uint16_t depth)
 {
-  return idx % depth;
+//  return idx % depth;
+  idx -= depth & -(idx > depth);
+  return idx -= depth & -(idx > depth);
 }
 
 // send one item to FIFO WITHOUT updating write pointer
@@ -194,15 +198,11 @@ static inline bool _tu_fifo_full(tu_fifo_t* f, uint16_t wAbs, uint16_t rAbs)
 }
 
 // Works on local copies of w and r
-//BE AWARE - THIS FUNCTION MIGHT NOT GIVE A CORRECT ANSWERE IN CASE WRITE POINTER "OVERFLOWS"
-//EXAMPLE with buffer depth: 100
-//Maximum index space: (2^16) - (2^16) % depth = 65500
-//If you produce 65500 / 100 + 1 = 656 buffer overflows, the write pointer will overflow as well and
-//the check _tu_fifo_overflow() will not give you a valid result! Avoid such nasty things!
-//All reading functions (read, peek) check for overflows and correct read pointer on their own such
-//that latest items are read.
-//If required (e.g. for DMA use) you can also correct the read pointer by
-//tu_fifo_correct_read_pointer().
+// BE AWARE - THIS FUNCTION MIGHT NOT GIVE A CORRECT ANSWERE IN CASE WRITE POINTER "OVERFLOWS"
+// Only one overflow is allowed for this function to work e.g. if depth = 100, you must not
+// write more than 2*depth-1 items in one rush without updating write pointer. Otherwise
+// write pointer wraps and you pointer states are messed up. This can only happen if you
+// use DMAs, write functions do not allow such an error.
 static inline bool _tu_fifo_overflow(tu_fifo_t* f, uint16_t wAbs, uint16_t rAbs)
 {
   return (_tu_fifo_count(f, wAbs, rAbs) > f->depth);
@@ -249,6 +249,7 @@ static uint16_t _tu_fifo_peek_at_n(tu_fifo_t* f, uint16_t pos, void * p_buffer,
   if (cnt > f->depth)
   {
     _tu_fifo_correct_read_pointer(f, wAbs);
+    rAbs = f->rd_idx;
     cnt = f->depth;
   }
 
@@ -401,13 +402,11 @@ bool tu_fifo_read(tu_fifo_t* f, void * buffer)
 {
   tu_fifo_lock(f);                                          // TODO: Here we may distinguish for read and write pointer mutexes!
 
-  uint16_t r = f->rd_idx;
-
   // Peek the data
-  bool ret = _tu_fifo_peek_at(f, 0, buffer, f->wr_idx, r);
+  bool ret = _tu_fifo_peek_at(f, 0, buffer, f->wr_idx, f->rd_idx);    // f->rd_idx might get modified in case of an overflow so we can not use a local variable
 
   // Advance pointer
-  f->rd_idx = advance_pointer(f, r, ret);
+  f->rd_idx = advance_pointer(f, f->rd_idx, ret);
 
   tu_fifo_unlock(f);
   return ret;
@@ -433,13 +432,11 @@ uint16_t tu_fifo_read_n(tu_fifo_t* f, void * buffer, uint16_t count)
 {
   tu_fifo_lock(f);                                          // TODO: Here we may distinguish for read and write pointer mutexes!
 
-  uint16_t r = f->rd_idx;
-
   // Peek the data
-  count = _tu_fifo_peek_at_n(f, 0, buffer, count, f->wr_idx, r);
+  count = _tu_fifo_peek_at_n(f, 0, buffer, count, f->wr_idx, f->rd_idx);        // f->rd_idx might get modified in case of an overflow so we can not use a local variable
 
   // Advance read pointer
-  f->rd_idx = advance_pointer(f, r, count);
+  f->rd_idx = advance_pointer(f, f->rd_idx, count);
 
   tu_fifo_unlock(f);
   return count;
diff --git a/src/common/tusb_fifo.h b/src/common/tusb_fifo.h
index 66888ef19..23f5d8449 100644
--- a/src/common/tusb_fifo.h
+++ b/src/common/tusb_fifo.h
@@ -48,7 +48,7 @@
 #include <stdbool.h>
 
 #ifdef __cplusplus
- extern "C" {
+extern "C" {
 #endif
 
 #if CFG_FIFO_MUTEX
@@ -61,16 +61,16 @@
  */
 typedef struct
 {
-           uint8_t* buffer                        ; ///< buffer pointer
-           uint16_t depth                         ; ///< max items
-           uint16_t item_size                     ; ///< size of each item
-           bool overwritable                      ;
+  uint8_t* buffer                        ; ///< buffer pointer
+  uint16_t depth                         ; ///< max items
+  uint16_t item_size                     ; ///< size of each item
+  bool overwritable                      ;
 
-           uint16_t non_used_index_space          ; ///< required for non-power-of-two buffer length
-           uint16_t max_pointer_idx               ; ///< maximum absolute pointer index
+  uint16_t non_used_index_space          ; ///< required for non-power-of-two buffer length
+  uint16_t max_pointer_idx               ; ///< maximum absolute pointer index
 
-  volatile uint16_t wr_idx                        ; ///< write pointer
-  volatile uint16_t rd_idx                        ; ///< read pointer
+  volatile uint16_t wr_idx               ; ///< write pointer
+  volatile uint16_t rd_idx               ; ///< read pointer
 
 #if CFG_FIFO_MUTEX
   tu_fifo_mutex_t mutex;
@@ -78,16 +78,16 @@ typedef struct
 
 } tu_fifo_t;
 
-#define TU_FIFO_DEF(_name, _depth, _type, _overwritable)              \
-  uint8_t _name##_buf[_depth*sizeof(_type)];                          \
-  tu_fifo_t _name = {                                                 \
-      .buffer                 = _name##_buf,                          \
-      .depth                  = _depth,                               \
-      .item_size              = sizeof(_type),                        \
-      .overwritable           = _overwritable,                        \
-      .non_used_index_space   = 0x10000 % _depth,                     \
-      .max_pointer_idx        = 0xFFFF - (0x10000 % _depth),          \
-  }
+#define TU_FIFO_DEF(_name, _depth, _type, _overwritable)                \
+    uint8_t _name##_buf[_depth*sizeof(_type)];                          \
+    tu_fifo_t _name = {                                                 \
+        .buffer                 = _name##_buf,                          \
+        .depth                  = _depth,                               \
+        .item_size              = sizeof(_type),                        \
+        .overwritable           = _overwritable,                        \
+        .max_pointer_idx        = 2*_depth-1,                           \
+        .non_used_index_space   = 0xFFFF - 2*_depth-1,                  \
+    }
 
 bool tu_fifo_clear(tu_fifo_t *f);
 bool tu_fifo_config(tu_fifo_t *f, void* buffer, uint16_t depth, uint16_t item_size, bool overwritable);
@@ -131,7 +131,7 @@ static inline uint16_t tu_fifo_depth(tu_fifo_t* f)
 }
 
 #ifdef __cplusplus
- }
+}
 #endif
 
 #endif /* _TUSB_FIFO_H_ */