refactor(tinyusb): Made the tinyusb task deletion notification driven

- Fixed the _task_pending status change on error during tinyusb_task_start()
- Kept the legace way of deleting task when blocking time is UINT32_MAX (task deleted in the user context)
- Added blocking_timeout_ms parameter to the driver config, tinyusb task parameters

Author: roma-jam

device/esp_tinyusb/include/tinyusb.h

@@ -60,6 +60,19 @@ typedef struct {
     size_t size;                                /*!< USB Device Task size. */
     uint8_t priority;                           /*!< USB Device Task priority. */
     int  xCoreID;                               /*!< USB Device Task core affinity.  */
+    uint32_t blocking_timeout_ms;               /*!< USB Device Task blocking timeout in milliseconds.
+                                                     This feature is used to control the behavior of the tud_task_ext() function called
+                                                     inside the dedicated TinyUSB task. The timeout defines how long the tud_task_ext() function
+                                                     will block waiting for USB events and how frequently the driver will poll for the uninstall notififcation.
+                                                     If tinyusb_driver_uninstall() is not used, the blocking timeout can block indefinitely to reduce CPU usage.
+
+                                                     Values:
+                                                        0 - non-blocking mode.
+                                                        UINT_MAX - blocks indefinetely (legacy mode).
+
+                                                     Default value: 1000 ms.
+                                                     */
+
 } tinyusb_task_config_t;
 
 /**

device/esp_tinyusb/include/tinyusb_default_config.h

@@ -63,6 +63,12 @@ extern "C" {
 #define TINYUSB_DEFAULT_TASK_SIZE      4096
 // Default priority for task used in TinyUSB task creation
 #define TINYUSB_DEFAULT_TASK_PRIO      5
+// Default blocking timeout_ms for tud_task_ext(timeout_ms, false), depending on target to acieve similar CPU load
+#if CONFIG_IDF_TARGET_ESP32P4
+#define TINYUSB_DEFAULT_BLOCK_TIME_MS  1000
+#else
+#define TINYUSB_DEFAULT_BLOCK_TIME_MS  2000
+#endif // CONFIG_IDF_TARGET_ESP32P4
 
 #define TINYUSB_CONFIG_FULL_SPEED(event_hdl, arg)       \
     (tinyusb_config_t) {                                \
@@ -111,13 +117,16 @@ extern "C" {
         .size = TINYUSB_DEFAULT_TASK_SIZE,              \
         .priority = TINYUSB_DEFAULT_TASK_PRIO,          \
         .xCoreID = TINYUSB_DEFAULT_TASK_AFFINITY,       \
+        .blocking_timeout_ms = TINYUSB_DEFAULT_BLOCK_TIME_MS,\
     }
 
 /**
  * @brief TinyUSB Task configuration structure initializer
  *
  * This macro is used to create a custom TinyUSB Task configuration structure.
  *
+ * Note: blocking_timeout_ms is set to default value.
+ *
  * @param s Stack size of the task
  * @param p Task priority
  * @param a Task affinity (CPU core)
@@ -127,6 +136,25 @@ extern "C" {
         .size = (s),                                    \
         .priority = (p),                                \
         .xCoreID = (a),                                 \
+        .blocking_timeout_ms = TINYUSB_DEFAULT_BLOCK_TIME_MS, \
+    }
+
+/**
+ * @brief TinyUSB Task configuration structure initializer
+ *
+ * This macro is used to create a custom TinyUSB Task configuration structure.
+ *
+ * @param s Stack size of the task
+ * @param p Task priority
+ * @param a Task affinity (CPU core)
+ * @param t Blocking timeout in milliseconds for tud_task_ext() function
+ */
+#define TINYUSB_TASK_CUSTOM_NON_BLOCKING(s, p, a, t)    \
+    (tinyusb_task_config_t) {                           \
+        .size = (s),                                    \
+        .priority = (p),                                \
+        .xCoreID = (a),                                 \
+        .blocking_timeout_ms = (t),                     \
     }
 
 #ifdef __cplusplus

device/esp_tinyusb/tinyusb_task.c

@@ -24,6 +24,9 @@ static portMUX_TYPE tusb_task_lock = portMUX_INITIALIZER_UNLOCKED;
 #define TINYUSB_TASK_ENTER_CRITICAL()    portENTER_CRITICAL(&tusb_task_lock)
 #define TINYUSB_TASK_EXIT_CRITICAL()     portEXIT_CRITICAL(&tusb_task_lock)
 
+// Timeout for taking the start/stop semaphore in milliseconds
+#define TINYUSB_TASK_SEM_TAKE_TIMEOUT_MS    (5000)
+
 #define TINYUSB_TASK_CHECK(cond, ret_val) ({                \
     if (!(cond)) {                                          \
         return (ret_val);                                   \
@@ -44,11 +47,14 @@ typedef struct {
     tusb_rhport_init_t rhport_init;         /*!< USB Device RH port initialization configuration pointer */
     const tinyusb_desc_config_t *desc_cfg;  /*!< USB Device descriptors configuration pointer */
     // Task related
-    TaskHandle_t handle;                    /*!< Task handle */
-    volatile TaskHandle_t awaiting_handle;           /*!< Task handle, waiting to be notified after successful start of TinyUSB stack */
+    TaskHandle_t handle;                    /*!< TinyUSB Device task handle */
+    SemaphoreHandle_t start_stop_sem;       /*!< Semaphore to start or stop the task */
+    uint32_t blocking_timeout_ms;           /*!< USB Device Task blocking timeout in milliseconds. */
 } tinyusb_task_ctx_t;
 
-static bool _task_is_running = false;               // Locking flag for the task, access only from the critical section
+#define TASK_NOTIF_STOP_BIT                 (1u << 0)
+
+static bool _task_is_pending = false;               // Locking flag for the task, access only from the critical section
 static tinyusb_task_ctx_t *p_tusb_task_ctx = NULL;  // TinyUSB task context
 
 /**
@@ -57,12 +63,19 @@ static tinyusb_task_ctx_t *p_tusb_task_ctx = NULL;  // TinyUSB task context
 static void tinyusb_device_task(void *arg)
 {
     tinyusb_task_ctx_t *task_ctx = (tinyusb_task_ctx_t *)arg;
+    bool stop_in_pending = false;
 
     // Sanity check
     assert(task_ctx != NULL);
-    assert(task_ctx->awaiting_handle != NULL);
+    assert(task_ctx->start_stop_sem != NULL);
 
     ESP_LOGD(TAG, "TinyUSB task started");
+    ESP_LOGD(TAG, "\tPort %d", task_ctx->rhport);
+    if (task_ctx->blocking_timeout_ms == UINT32_MAX) {
+        ESP_LOGD(TAG, "\tBlocking timeout: MAX");
+    } else {
+        ESP_LOGD(TAG, "\tBlocking timeout: %" PRIu32 " ms", task_ctx->blocking_timeout_ms);
+    }
 
     if (tud_inited()) {
         ESP_LOGE(TAG, "TinyUSB stack is already initialized");
@@ -78,22 +91,55 @@ static void tinyusb_device_task(void *arg)
     }
 
     TINYUSB_TASK_ENTER_CRITICAL();
-    task_ctx->handle = xTaskGetCurrentTaskHandle(); // Save task handle
-    p_tusb_task_ctx = task_ctx;                     // Save global task context pointer
+    _task_is_pending = false;                           // Clear pending flag, task is running now
+    task_ctx->handle = xTaskGetCurrentTaskHandle();     // Save task handle
+    p_tusb_task_ctx = task_ctx;                         // Save global task context pointer
     TINYUSB_TASK_EXIT_CRITICAL();
 
-    xTaskNotifyGive(task_ctx->awaiting_handle);     // Notify parent task that TinyUSB stack was started successfully
+    // Notify parent task that TinyUSB stack was started successfully
+    if (xSemaphoreGive(task_ctx->start_stop_sem) != pdTRUE) {
+        // Fail only if semaphore queue is full, should never happen
+        ESP_LOGE(TAG, "Unable to notify that TinyUSB stack was started");
+        goto desc_free;
+    }
 
-    while (1) { // RTOS forever loop
-        tud_task();
+    uint32_t notify_blocking_ms = (task_ctx->blocking_timeout_ms == 0) ? 1 : 0; // Insignificant blocking time to check notifications, to not to waste CPU when tud_task_ext(0, false)
+    while (1) {
+        uint32_t notif = 0;
+        // Poll for notification bits
+        if (xTaskNotifyWait(
+                    0,                      // don't clear any bits on entry
+                    TASK_NOTIF_STOP_BIT,    // clear STOP bit on exit if it was set
+                    &notif,
+                    pdMS_TO_TICKS(notify_blocking_ms)
+                ) == pdTRUE && (notif & TASK_NOTIF_STOP_BIT)) {
+            // Stop requested
+            stop_in_pending = true;
+            break;
+        }
+        // TinyUSB Device task
+        tud_task_ext(task_ctx->blocking_timeout_ms, false);
     }
 
+    // Stop TinyUSB stack
+    if (!tusb_deinit(task_ctx->rhport)) {
+        ESP_LOGE(TAG, "Unable to deinit TinyUSB stack");
+    }
+
+    ESP_LOGD(TAG, "TinyUSB task has stopped");
+
 desc_free:
     tinyusb_descriptors_free();
 del:
     TINYUSB_TASK_ENTER_CRITICAL();
-    _task_is_running = false;       // Task is not running anymore
+    _task_is_pending = false;       // Clear pending flag
+    p_tusb_task_ctx = NULL;         // Clear global task context pointer
     TINYUSB_TASK_EXIT_CRITICAL();
+
+    if (stop_in_pending) {
+        // Notify that TinyUSB stack was stopped
+        xSemaphoreGive(task_ctx->start_stop_sem);
+    }
     vTaskDelete(NULL);
     // No return needed here: vTaskDelete(NULL) does not return
 }
@@ -117,22 +163,25 @@ esp_err_t tinyusb_task_start(tinyusb_port_t port, const tinyusb_task_config_t *c
 
     TINYUSB_TASK_ENTER_CRITICAL();
     TINYUSB_TASK_CHECK_FROM_CRIT(p_tusb_task_ctx == NULL, ESP_ERR_INVALID_STATE);     // Task shouldn't started
-    TINYUSB_TASK_CHECK_FROM_CRIT(!_task_is_running, ESP_ERR_INVALID_STATE);           // Task shouldn't be running
-    _task_is_running = true;                                                          // Task is running flag, will be cleared in task in case of the error
+    TINYUSB_TASK_CHECK_FROM_CRIT(!_task_is_pending, ESP_ERR_INVALID_STATE);           // Task shouldn't be pending
+    _task_is_pending = true;                                                          // Task is pending flag, will be cleared in task
     TINYUSB_TASK_EXIT_CRITICAL();
 
     esp_err_t ret;
     tinyusb_task_ctx_t *task_ctx = heap_caps_calloc(1, sizeof(tinyusb_task_ctx_t), MALLOC_CAP_DEFAULT);
-    if (task_ctx == NULL) {
-        return ESP_ERR_NO_MEM;
+    SemaphoreHandle_t sem = xSemaphoreCreateBinary();
+
+    if (task_ctx == NULL || sem == NULL) {
+        ret = ESP_ERR_NO_MEM;
+        goto no_task;
     }
 
-    task_ctx->awaiting_handle = xTaskGetCurrentTaskHandle();    // Save parent task handle
-    task_ctx->handle = NULL;                                    // TinyUSB task is not started
+    task_ctx->start_stop_sem = sem;                             // TunyUSB Device task start stop semaphore
     task_ctx->rhport = port;                                    // Peripheral port number
     task_ctx->rhport_init.role = TUSB_ROLE_DEVICE;              // Role selection: esp_tinyusb is always a device
     task_ctx->rhport_init.speed = (port == TINYUSB_PORT_FULL_SPEED_0) ? TUSB_SPEED_FULL : TUSB_SPEED_HIGH; // Speed selection
     task_ctx->desc_cfg = desc_cfg;
+    task_ctx->blocking_timeout_ms = config->blocking_timeout_ms;
 
     TaskHandle_t task_hdl = NULL;
     ESP_LOGD(TAG, "Creating TinyUSB main task on CPU%d", config->xCoreID);
@@ -147,19 +196,32 @@ esp_err_t tinyusb_task_start(tinyusb_port_t port, const tinyusb_task_config_t *c
     if (task_hdl == NULL) {
         ESP_LOGE(TAG, "Create TinyUSB main task failed");
         ret = ESP_ERR_NOT_FINISHED;
-        goto err;
+        goto no_task;
     }
 
-    // Wait until the Task notify that port is active, 5 sec is more than enough
-    if (ulTaskNotifyTake(pdTRUE, pdMS_TO_TICKS(5000)) == 0) {
+    // Wait for "start" semophore with timeout
+    if (xSemaphoreTake(task_ctx->start_stop_sem, pdMS_TO_TICKS(TINYUSB_TASK_SEM_TAKE_TIMEOUT_MS)) != pdTRUE) {
         ESP_LOGE(TAG, "Task wasn't able to start TinyUSB stack");
         ret = ESP_ERR_TIMEOUT;
         goto err;
     }
 
+    // Check the global task context pointer and pending state
+    TINYUSB_TASK_ENTER_CRITICAL();
+    TINYUSB_TASK_CHECK_FROM_CRIT(!_task_is_pending, ESP_ERR_INVALID_STATE);
+    TINYUSB_TASK_CHECK_FROM_CRIT(p_tusb_task_ctx == task_ctx, ESP_ERR_NOT_FOUND);
+    TINYUSB_TASK_EXIT_CRITICAL();
+
     return ESP_OK;
 
+no_task:
+    TINYUSB_TASK_ENTER_CRITICAL();
+    _task_is_pending = false;   // Clear pending flag
+    TINYUSB_TASK_EXIT_CRITICAL();
 err:
+    if (sem) {
+        vSemaphoreDelete(sem);
+    }
     heap_caps_free(task_ctx);
     return ret;
 }
@@ -168,20 +230,43 @@ esp_err_t tinyusb_task_stop(void)
 {
     TINYUSB_TASK_ENTER_CRITICAL();
     TINYUSB_TASK_CHECK_FROM_CRIT(p_tusb_task_ctx != NULL, ESP_ERR_INVALID_STATE);
+    TINYUSB_TASK_CHECK_FROM_CRIT(!_task_is_pending, ESP_ERR_INVALID_STATE);   // Task shouldn't be pending
     tinyusb_task_ctx_t *task_ctx = p_tusb_task_ctx;
-    p_tusb_task_ctx = NULL;
-    _task_is_running = false;
+    _task_is_pending = true;    // Task is pending to stop
     TINYUSB_TASK_EXIT_CRITICAL();
 
-    if (task_ctx->handle != NULL) {
+    if (task_ctx->blocking_timeout_ms != UINT32_MAX) {
+        // Request TinyUSB task to stop
+        if (xTaskNotify(task_ctx->handle, TASK_NOTIF_STOP_BIT, eSetBits) != pdPASS) {
+            ESP_LOGE(TAG, "Unable to notify TinyUSB task to stop");
+            return ESP_ERR_INVALID_STATE;
+        }
+        // Wait for "stop" semaphore with timeout
+        if (xSemaphoreTake(task_ctx->start_stop_sem, pdMS_TO_TICKS(TINYUSB_TASK_SEM_TAKE_TIMEOUT_MS)) != pdTRUE) {
+            ESP_LOGE(TAG, "Timeout while TinyUSB task stop");
+            return ESP_ERR_TIMEOUT;
+        }
+        // Check the global task context pointer and pending state
+        TINYUSB_TASK_ENTER_CRITICAL();
+        TINYUSB_TASK_CHECK_FROM_CRIT(!_task_is_pending, ESP_ERR_INVALID_STATE);
+        TINYUSB_TASK_CHECK_FROM_CRIT(p_tusb_task_ctx == NULL, ESP_ERR_INVALID_STATE);
+        TINYUSB_TASK_EXIT_CRITICAL();
+    } else {
+        // TinyUSB Task is in forever loop, delete the task the old way
         vTaskDelete(task_ctx->handle);
         task_ctx->handle = NULL;
+        // Free descriptors
+        tinyusb_descriptors_free();
+        // Deinit TinyUSB Stask
+        ESP_RETURN_ON_FALSE(tusb_deinit(task_ctx->rhport), ESP_ERR_INVALID_STATE, TAG, "Unable to deinit TinyUSB stack");
+        // Clean global context and pending state
+        TINYUSB_TASK_ENTER_CRITICAL();
+        _task_is_pending = false;
+        p_tusb_task_ctx = NULL;
+        TINYUSB_TASK_EXIT_CRITICAL();
     }
-    // Free descriptors
-    tinyusb_descriptors_free();
-    // Stop TinyUSB stack
-    ESP_RETURN_ON_FALSE(tusb_deinit(task_ctx->rhport), ESP_ERR_NOT_FINISHED, TAG, "Unable to teardown TinyUSB stack");
-    // Cleanup
+
+    vSemaphoreDelete(task_ctx->start_stop_sem);
     heap_caps_free(task_ctx);
     return ESP_OK;
 }