Merge pull request #1799 from hathach/update-osal-mutex

Update osal mutex

src/class/cdc/cdc_device.c

@@ -62,10 +62,8 @@ typedef struct
   uint8_t rx_ff_buf[CFG_TUD_CDC_RX_BUFSIZE];
   uint8_t tx_ff_buf[CFG_TUD_CDC_TX_BUFSIZE];
 
-#if CFG_FIFO_MUTEX
-  osal_mutex_def_t rx_ff_mutex;
-  osal_mutex_def_t tx_ff_mutex;
-#endif
+  OSAL_MUTEX_DEF(rx_ff_mutex);
+  OSAL_MUTEX_DEF(tx_ff_mutex);
 
   // Endpoint Transfer buffer
   CFG_TUSB_MEM_ALIGN uint8_t epout_buf[CFG_TUD_CDC_EP_BUFSIZE];
@@ -248,10 +246,8 @@ void cdcd_init(void)
     // In this way, the most current data is prioritized.
     tu_fifo_config(&p_cdc->tx_ff, p_cdc->tx_ff_buf, TU_ARRAY_SIZE(p_cdc->tx_ff_buf), 1, true);
 
-#if CFG_FIFO_MUTEX
     tu_fifo_config_mutex(&p_cdc->rx_ff, NULL, osal_mutex_create(&p_cdc->rx_ff_mutex));
     tu_fifo_config_mutex(&p_cdc->tx_ff, osal_mutex_create(&p_cdc->tx_ff_mutex), NULL);
-#endif
   }
 }
 

src/class/usbtmc/usbtmc_device.c

@@ -157,12 +157,12 @@ static bool handle_devMsgOut(uint8_t rhport, void *data, size_t len, size_t pack
 static uint8_t termChar;
 static uint8_t termCharRequested = false;
 
-osal_mutex_def_t usbtmcLockBuffer;
+OSAL_MUTEX_DEF(usbtmcLockBuffer);
 static osal_mutex_t usbtmcLock;
 
 // Our own private lock, mostly for the state variable.
-#define criticalEnter() do {osal_mutex_lock(usbtmcLock,OSAL_TIMEOUT_WAIT_FOREVER); } while (0)
-#define criticalLeave() do {osal_mutex_unlock(usbtmcLock); } while (0)
+#define criticalEnter() do { (void) osal_mutex_lock(usbtmcLock,OSAL_TIMEOUT_WAIT_FOREVER); } while (0)
+#define criticalLeave() do { (void) osal_mutex_unlock(usbtmcLock); } while (0)
 
 bool atomicChangeState(usbtmcd_state_enum expectedState, usbtmcd_state_enum newState)
 {

src/common/tusb_fifo.h

@@ -42,15 +42,13 @@ extern "C" {
 // within a certain number (see tu_fifo_overflow()).
 
 #include "common/tusb_common.h"
+#include "osal/osal.h"
+
+#define tu_fifo_mutex_t  osal_mutex_t
 
 // mutex is only needed for RTOS
 // for OS None, we don't get preempted
-#define CFG_FIFO_MUTEX      (CFG_TUSB_OS != OPT_OS_NONE)
-
-#if CFG_FIFO_MUTEX
-#include "osal/osal.h"
-#define tu_fifo_mutex_t  osal_mutex_t
-#endif
+#define CFG_FIFO_MUTEX      OSAL_MUTEX_REQUIRED
 
 typedef struct
 {
@@ -65,7 +63,7 @@ typedef struct
   volatile uint16_t wr_idx      ; ///< write pointer
   volatile uint16_t rd_idx      ; ///< read pointer
 
-#if CFG_FIFO_MUTEX
+#if OSAL_MUTEX_REQUIRED
   tu_fifo_mutex_t mutex_wr;
   tu_fifo_mutex_t mutex_rd;
 #endif
@@ -99,13 +97,18 @@ bool tu_fifo_set_overwritable(tu_fifo_t *f, bool overwritable);
 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);
 
-#if CFG_FIFO_MUTEX
+#if OSAL_MUTEX_REQUIRED
 TU_ATTR_ALWAYS_INLINE static inline
 void tu_fifo_config_mutex(tu_fifo_t *f, tu_fifo_mutex_t write_mutex_hdl, tu_fifo_mutex_t read_mutex_hdl)
 {
   f->mutex_wr = write_mutex_hdl;
   f->mutex_rd = read_mutex_hdl;
 }
+
+#else
+
+#define tu_fifo_config_mutex(_f, _wr_mutex, _rd_mutex)
+
 #endif
 
 bool     tu_fifo_write                  (tu_fifo_t* f, void const * p_data);

src/common/tusb_mcu.h

@@ -281,6 +281,10 @@
 // Default Values
 //--------------------------------------------------------------------+
 
+#ifndef TUP_MCU_MULTIPLE_CORE
+#define TUP_MCU_MULTIPLE_CORE 0
+#endif
+
 #ifndef TUP_DCD_ENDPOINT_MAX
   #warning "TUP_DCD_ENDPOINT_MAX is not defined for this MCU, default to 8"
   #define TUP_DCD_ENDPOINT_MAX    8

src/device/usbd.c

@@ -272,10 +272,12 @@ static uint8_t _usbd_rhport = RHPORT_INVALID;
 OSAL_QUEUE_DEF(usbd_int_set, _usbd_qdef, CFG_TUD_TASK_QUEUE_SZ, dcd_event_t);
 static osal_queue_t _usbd_q;
 
-// Mutex for claiming endpoint, only needed when using with preempted RTOS
-#if CFG_TUSB_OS != OPT_OS_NONE
-static osal_mutex_def_t _ubsd_mutexdef;
-static osal_mutex_t _usbd_mutex;
+// Mutex for claiming endpoint
+#if OSAL_MUTEX_REQUIRED
+  static osal_mutex_def_t _ubsd_mutexdef;
+  static osal_mutex_t _usbd_mutex;
+#else
+  #define _usbd_mutex   NULL
 #endif
 
 
@@ -389,7 +391,7 @@ bool tud_init (uint8_t rhport)
 
   tu_varclr(&_usbd_dev);
 
-#if CFG_TUSB_OS != OPT_OS_NONE
+#if OSAL_MUTEX_REQUIRED
   // Init device mutex
   _usbd_mutex = osal_mutex_create(&_ubsd_mutexdef);
   TU_ASSERT(_usbd_mutex);
@@ -1209,11 +1211,7 @@ bool usbd_edpt_claim(uint8_t rhport, uint8_t ep_addr)
   uint8_t const dir         = tu_edpt_dir(ep_addr);
   tu_edpt_state_t* ep_state = &_usbd_dev.ep_status[epnum][dir];
 
-#if TUSB_OPT_MUTEX
   return tu_edpt_claim(ep_state, _usbd_mutex);
-#else
-  return tu_edpt_claim(ep_state, NULL);
-#endif
 }
 
 bool usbd_edpt_release(uint8_t rhport, uint8_t ep_addr)
@@ -1224,11 +1222,7 @@ bool usbd_edpt_release(uint8_t rhport, uint8_t ep_addr)
   uint8_t const dir         = tu_edpt_dir(ep_addr);
   tu_edpt_state_t* ep_state = &_usbd_dev.ep_status[epnum][dir];
 
-#if TUSB_OPT_MUTEX
   return tu_edpt_release(ep_state, _usbd_mutex);
-#else
-  return tu_edpt_release(ep_state, NULL);
-#endif
 }
 
 bool usbd_edpt_xfer(uint8_t rhport, uint8_t ep_addr, uint8_t * buffer, uint16_t total_bytes)

src/host/usbh.c

@@ -212,28 +212,12 @@ static usbh_dev0_t _dev0;
 // TODO: hub can has its own simpler struct to save memory
 CFG_TUSB_MEM_SECTION usbh_device_t _usbh_devices[TOTAL_DEVICES];
 
-// Mutex for claiming endpoint, only needed when using with preempted RTOS
-#if TUSB_OPT_MUTEX
-static osal_mutex_def_t _usbh_mutexdef;
-static osal_mutex_t _usbh_mutex;
-
-TU_ATTR_ALWAYS_INLINE static inline void usbh_lock(void)
-{
-  osal_mutex_lock(_usbh_mutex, OSAL_TIMEOUT_WAIT_FOREVER);
-}
-
-TU_ATTR_ALWAYS_INLINE static inline void usbh_unlock(void)
-{
-  osal_mutex_unlock(_usbh_mutex);
-}
-
+// Mutex for claiming endpoint
+#if OSAL_MUTEX_REQUIRED
+  static osal_mutex_def_t _usbh_mutexdef;
+  static osal_mutex_t _usbh_mutex;
 #else
-
-#define _usbh_mutex   NULL
-
-#define usbh_lock()
-#define usbh_unlock()
-
+  #define _usbh_mutex   NULL
 #endif
 
 // Event queue
@@ -277,8 +261,6 @@ static bool usbh_control_xfer_cb (uint8_t daddr, uint8_t ep_addr, xfer_result_t
 // TODO rework time-related function later
 void osal_task_delay(uint32_t msec)
 {
-  (void) msec;
-
   const uint32_t start = hcd_frame_number(_usbh_controller);
   while ( ( hcd_frame_number(_usbh_controller) - start ) < msec ) {}
 }
@@ -352,8 +334,8 @@ bool tuh_init(uint8_t controller_id)
   _usbh_q = osal_queue_create( &_usbh_qdef );
   TU_ASSERT(_usbh_q != NULL);
 
-#if TUSB_OPT_MUTEX
-  // Mutex
+#if OSAL_MUTEX_REQUIRED
+  // Init mutex
   _usbh_mutex = osal_mutex_create(&_usbh_mutexdef);
   TU_ASSERT(_usbh_mutex);
 #endif
@@ -537,8 +519,7 @@ bool tuh_control_xfer (tuh_xfer_t* xfer)
 
   uint8_t const daddr = xfer->daddr;
 
-  // TODO probably better to use semaphore as resource management than mutex
-  usbh_lock();
+  (void) osal_mutex_lock(_usbh_mutex, OSAL_TIMEOUT_WAIT_FOREVER);
 
   bool const is_idle = (_ctrl_xfer.stage == CONTROL_STAGE_IDLE);
   if (is_idle)
@@ -553,7 +534,7 @@ bool tuh_control_xfer (tuh_xfer_t* xfer)
     _ctrl_xfer.user_data   = xfer->user_data;
   }
 
-  usbh_unlock();
+  (void) osal_mutex_unlock(_usbh_mutex);
 
   TU_VERIFY(is_idle);
   const uint8_t rhport = usbh_get_rhport(daddr);
@@ -597,9 +578,9 @@ bool tuh_control_xfer (tuh_xfer_t* xfer)
 
 TU_ATTR_ALWAYS_INLINE static inline void _set_control_xfer_stage(uint8_t stage)
 {
-  usbh_lock();
+  (void) osal_mutex_lock(_usbh_mutex, OSAL_TIMEOUT_WAIT_FOREVER);
   _ctrl_xfer.stage = stage;
-  usbh_unlock();
+  (void) osal_mutex_unlock(_usbh_mutex);
 }
 
 static void _xfer_complete(uint8_t daddr, xfer_result_t result)

src/osal/osal.h

@@ -33,17 +33,24 @@
 
 #include "common/tusb_common.h"
 
-// Return immediately
-#define OSAL_TIMEOUT_NOTIMEOUT     (0)
-// Default timeout
-#define OSAL_TIMEOUT_NORMAL        (10)
-// Wait forever
-#define OSAL_TIMEOUT_WAIT_FOREVER  (UINT32_MAX)
+typedef void (*osal_task_func_t)( void * );
 
+// Timeout
+#define OSAL_TIMEOUT_NOTIMEOUT     (0)          // Return immediately
+#define OSAL_TIMEOUT_NORMAL        (10)         // Default timeout
+#define OSAL_TIMEOUT_WAIT_FOREVER  (UINT32_MAX) // Wait forever
 #define OSAL_TIMEOUT_CONTROL_XFER  OSAL_TIMEOUT_WAIT_FOREVER
 
-typedef void (*osal_task_func_t)( void * );
+// Mutex is required when using a preempted RTOS or MCU has multiple cores
+#if (CFG_TUSB_OS == OPT_OS_NONE) && !TUP_MCU_MULTIPLE_CORE
+  #define OSAL_MUTEX_REQUIRED   0
+  #define OSAL_MUTEX_DEF(_name)
+#else
+  #define OSAL_MUTEX_REQUIRED   1
+  #define OSAL_MUTEX_DEF(_name) osal_mutex_def_t _name
+#endif
 
+// OS thin implementation
 #if CFG_TUSB_OS == OPT_OS_NONE
   #include "osal_none.h"
 #elif CFG_TUSB_OS == OPT_OS_FREERTOS

src/osal/osal_none.h

@@ -82,6 +82,10 @@ TU_ATTR_ALWAYS_INLINE static inline void osal_semaphore_reset(osal_semaphore_t s
 typedef osal_semaphore_def_t osal_mutex_def_t;
 typedef osal_semaphore_t osal_mutex_t;
 
+#if OSAL_MUTEX_REQUIRED
+// Note: multiple cores MCUs usually do provide IPC API for mutex
+// or we can use std atomic function
+
 TU_ATTR_ALWAYS_INLINE static inline osal_mutex_t osal_mutex_create(osal_mutex_def_t* mdef)
 {
   mdef->count = 1;
@@ -98,6 +102,14 @@ TU_ATTR_ALWAYS_INLINE static inline bool osal_mutex_unlock(osal_mutex_t mutex_hd
   return osal_semaphore_post(mutex_hdl, false);
 }
 
+#else
+
+#define osal_mutex_create(_mdef)          (NULL)
+#define osal_mutex_lock(_mutex_hdl, _ms)  (true)
+#define osal_mutex_unlock(_mutex_hdl)     (true)
+
+#endif
+
 //--------------------------------------------------------------------+
 // QUEUE API
 //--------------------------------------------------------------------+

src/tusb.c

@@ -74,11 +74,9 @@ bool tu_edpt_claim(tu_edpt_state_t* ep_state, osal_mutex_t mutex)
 {
   (void) mutex;
 
-#if TUSB_OPT_MUTEX
   // pre-check to help reducing mutex lock
   TU_VERIFY((ep_state->busy == 0) && (ep_state->claimed == 0));
-  osal_mutex_lock(mutex, OSAL_TIMEOUT_WAIT_FOREVER);
-#endif
+  (void) osal_mutex_lock(mutex, OSAL_TIMEOUT_WAIT_FOREVER);
 
   // can only claim the endpoint if it is not busy and not claimed yet.
   bool const available = (ep_state->busy == 0) && (ep_state->claimed == 0);
@@ -87,9 +85,7 @@ bool tu_edpt_claim(tu_edpt_state_t* ep_state, osal_mutex_t mutex)
     ep_state->claimed = 1;
   }
 
-#if TUSB_OPT_MUTEX
-  osal_mutex_unlock(mutex);
-#endif
+  (void) osal_mutex_unlock(mutex);
 
   return available;
 }
@@ -98,9 +94,7 @@ bool tu_edpt_release(tu_edpt_state_t* ep_state, osal_mutex_t mutex)
 {
   (void) mutex;
 
-#if TUSB_OPT_MUTEX
-  osal_mutex_lock(mutex, OSAL_TIMEOUT_WAIT_FOREVER);
-#endif
+  (void) osal_mutex_lock(mutex, OSAL_TIMEOUT_WAIT_FOREVER);
 
   // can only release the endpoint if it is claimed and not busy
   bool const ret = (ep_state->claimed == 1) && (ep_state->busy == 0);
@@ -109,9 +103,7 @@ bool tu_edpt_release(tu_edpt_state_t* ep_state, osal_mutex_t mutex)
     ep_state->claimed = 0;
   }
 
-#if TUSB_OPT_MUTEX
-  osal_mutex_unlock(mutex);
-#endif
+  (void) osal_mutex_unlock(mutex);
 
   return ret;
 }

src/tusb_option.h

@@ -299,9 +299,6 @@ typedef int make_iso_compilers_happy;
   #define CFG_TUSB_OS_INC_PATH
 #endif
 
-// mutex is only needed for RTOS TODO also required with multiple core MCUs
-#define TUSB_OPT_MUTEX      (CFG_TUSB_OS != OPT_OS_NONE)
-
 //--------------------------------------------------------------------
 // Device Options (Default)
 //--------------------------------------------------------------------

test/unit-test/test/device/msc/test_msc_device.c

@@ -28,6 +28,7 @@
 #include "unity.h"
 
 // Files to test
+#include "osal/osal.h"
 #include "tusb_fifo.h"
 #include "tusb.h"
 #include "usbd.h"

test/unit-test/test/device/usbd/test_usbd.c

@@ -25,6 +25,7 @@
 #include "unity.h"
 
 // Files to test
+#include "osal/osal.h"
 #include "tusb_fifo.h"
 #include "tusb.h"
 #include "usbd.h"

test/unit-test/test/test_fifo.c

@@ -26,6 +26,8 @@
 
 #include <string.h>
 #include "unity.h"
+
+#include "osal/osal.h"
 #include "tusb_fifo.h"
 
 #define FIFO_SIZE 10
@@ -315,4 +317,4 @@ void test_rd_idx_wrap()
   n = tu_fifo_read_n(&ff10, dst, 4);
   TEST_ASSERT_EQUAL(n, 2);
   TEST_ASSERT_EQUAL(ff10.rd_idx, 6);
-}
+}