change(usb/host): Remove some handler function event flags

This commit removes internal event flags in the USB Host Library event handling
functions (i.e., usb_host_lib_handle_events() and usb_host_client_handle_events()).

Previously, these flags were added to reduce the number of times semaphores
were given. However, these flags were removed as the performance gain is
negligible and made the logic more complicated.

For usb_host_client_handle_events(), the following flags were removed:

- Remove 'events_pending' flag. The semaphore is now always given
- Remove 'blocked' flag. The 'handling_events' flag is already sufficient
- Critical sections are now shortened due to simplication of semaphore usage.

For usb_host_lib_handle_events(), the following flags were removed:

- Remove 'process_pending' flag. The semaphore is now always given
- Renamed 'blocked' flag to 'handling_events'

components/usb/usb_host.c

@@ -1,5 +1,5 @@
 /*
- * SPDX-FileCopyrightText: 2015-2022 Espressif Systems (Shanghai) CO LTD
+ * SPDX-FileCopyrightText: 2015-2023 Espressif Systems (Shanghai) CO LTD
  *
  * SPDX-License-Identifier: Apache-2.0
  */
@@ -94,11 +94,9 @@ struct client_s {
         TAILQ_HEAD(tailhead_done_ctrl_xfers, urb_s) done_ctrl_xfer_tailq;
         union {
             struct {
-                uint32_t events_pending: 1;
                 uint32_t handling_events: 1;
-                uint32_t blocked: 1;
                 uint32_t taking_mux: 1;
-                uint32_t reserved4: 4;
+                uint32_t reserved6: 6;
                 uint32_t num_intf_claimed: 8;
                 uint32_t reserved16: 16;
             };
@@ -128,10 +126,8 @@ typedef struct {
         uint32_t lib_event_flags;
         union {
             struct {
-                uint32_t process_pending: 1;
                 uint32_t handling_events: 1;
-                uint32_t blocked: 1;
+                uint32_t reserved7: 7;
-                uint32_t reserved5: 5;
                 uint32_t num_clients: 8;
                 uint32_t reserved16: 16;
             };
@@ -176,24 +172,16 @@ static inline bool _check_client_opened_device(client_t *client_obj, uint8_t dev
 
 static bool _unblock_client(client_t *client_obj, bool in_isr)
 {
-    bool send_sem;
+    bool yield;
-    if (!client_obj->dynamic.flags.events_pending && !client_obj->dynamic.flags.handling_events) {
-        client_obj->dynamic.flags.events_pending = 1;
-        send_sem = true;
-    } else {
-        send_sem = false;
-    }
 
     HOST_EXIT_CRITICAL_SAFE();
-    bool yield = false;
-    if (send_sem) {
     if (in_isr) {
         BaseType_t xTaskWoken = pdFALSE;
         xSemaphoreGiveFromISR(client_obj->constant.event_sem, &xTaskWoken);
         yield = (xTaskWoken == pdTRUE);
     } else {
         xSemaphoreGive(client_obj->constant.event_sem);
-        }
+        yield = false;
     }
     HOST_ENTER_CRITICAL_SAFE();
 
@@ -202,24 +190,16 @@ static bool _unblock_client(client_t *client_obj, bool in_isr)
 
 static bool _unblock_lib(bool in_isr)
 {
-    bool send_sem;
+    bool yield;
-    if (!p_host_lib_obj->dynamic.flags.process_pending && !p_host_lib_obj->dynamic.flags.handling_events) {
-        p_host_lib_obj->dynamic.flags.process_pending = 1;
-        send_sem = true;
-    } else {
-        send_sem = false;
-    }
 
     HOST_EXIT_CRITICAL_SAFE();
-    bool yield = false;
-    if (send_sem) {
     if (in_isr) {
         BaseType_t xTaskWoken = pdFALSE;
         xSemaphoreGiveFromISR(p_host_lib_obj->constant.event_sem, &xTaskWoken);
         yield = (xTaskWoken == pdTRUE);
     } else {
         xSemaphoreGive(p_host_lib_obj->constant.event_sem);
-        }
+        yield = false;
     }
     HOST_ENTER_CRITICAL_SAFE();
 
@@ -515,45 +495,49 @@ esp_err_t usb_host_uninstall(void)
 
 esp_err_t usb_host_lib_handle_events(TickType_t timeout_ticks, uint32_t *event_flags_ret)
 {
-    esp_err_t ret;
+    // Check arguments and state
-    uint32_t event_flags = 0;
+    HOST_CHECK(p_host_lib_obj != NULL, ESP_ERR_INVALID_STATE);
+
+    esp_err_t ret = (timeout_ticks == 0) ? ESP_OK : ESP_ERR_TIMEOUT;    // We don't want to return ESP_ERR_TIMEOUT if we aren't blocking
+    uint32_t event_flags;
 
     HOST_ENTER_CRITICAL();
-    if (!p_host_lib_obj->dynamic.flags.process_pending) {
+    // Set handling_events flag. This prevents the host library from being uninstalled
-        // There is currently processing that needs to be done. Wait for some processing
+    p_host_lib_obj->dynamic.flags.handling_events = 1;
     HOST_EXIT_CRITICAL();
-        BaseType_t sem_ret = xSemaphoreTake(p_host_lib_obj->constant.event_sem, timeout_ticks);
+
-        if (sem_ret == pdFALSE) {
+    while (1) {
-            ret = ESP_ERR_TIMEOUT;
+        // Loop until there are no more events
-            goto exit;
+        if (xSemaphoreTake(p_host_lib_obj->constant.event_sem, timeout_ticks) == pdFALSE) {
-        }
+            // Timed out waiting for semaphore or currently no events
-        HOST_ENTER_CRITICAL();
+            break;
         }
+
         // Read and clear process pending flags
+        HOST_ENTER_CRITICAL();
         uint32_t process_pending_flags = p_host_lib_obj->dynamic.process_pending_flags;
         p_host_lib_obj->dynamic.process_pending_flags = 0;
-    p_host_lib_obj->dynamic.flags.handling_events = 1;
-    while (process_pending_flags) {
         HOST_EXIT_CRITICAL();
+
         if (process_pending_flags & PROCESS_REQUEST_PENDING_FLAG_USBH) {
             ESP_ERROR_CHECK(usbh_process());
         }
         if (process_pending_flags & PROCESS_REQUEST_PENDING_FLAG_HUB) {
             ESP_ERROR_CHECK(hub_process());
         }
-        HOST_ENTER_CRITICAL();
+
-        // Read and clear process pending flags again, and loop back if there is more to process
+        ret = ESP_OK;
-        process_pending_flags = p_host_lib_obj->dynamic.process_pending_flags;
+        // Set timeout_ticks to 0 so that we can check for events again without blocking
-        p_host_lib_obj->dynamic.process_pending_flags = 0;
+        timeout_ticks = 0;
     }
-    p_host_lib_obj->dynamic.flags.process_pending = 0;
+
+    HOST_ENTER_CRITICAL();
     p_host_lib_obj->dynamic.flags.handling_events = 0;
+    // Read and clear any event flags
     event_flags = p_host_lib_obj->dynamic.lib_event_flags;
     p_host_lib_obj->dynamic.lib_event_flags = 0;
     HOST_EXIT_CRITICAL();
 
-    ret = ESP_OK;
-exit:
     if (event_flags_ret != NULL) {
         *event_flags_ret = event_flags;
     }
@@ -709,7 +693,6 @@ esp_err_t usb_host_client_deregister(usb_host_client_handle_t client_hdl)
             !TAILQ_EMPTY(&client_obj->dynamic.idle_ep_tailq) ||
             !TAILQ_EMPTY(&client_obj->dynamic.done_ctrl_xfer_tailq) ||
             client_obj->dynamic.flags.handling_events ||
-            client_obj->dynamic.flags.blocked ||
             client_obj->dynamic.flags.taking_mux ||
             client_obj->dynamic.flags.num_intf_claimed != 0 ||
             client_obj->dynamic.num_done_ctrl_xfer != 0 ||
@@ -746,28 +729,26 @@ exit:
 
 esp_err_t usb_host_client_handle_events(usb_host_client_handle_t client_hdl, TickType_t timeout_ticks)
 {
+    // Check arguments and state
     HOST_CHECK(client_hdl != NULL, ESP_ERR_INVALID_ARG);
-    esp_err_t ret;
+    HOST_CHECK(p_host_lib_obj != NULL, ESP_ERR_INVALID_STATE);
+
+    esp_err_t ret = (timeout_ticks == 0) ? ESP_OK : ESP_ERR_TIMEOUT;    // We don't want to return ESP_ERR_TIMEOUT if we aren't blocking
     client_t *client_obj = (client_t *)client_hdl;
 
     HOST_ENTER_CRITICAL();
-    if (!client_obj->dynamic.flags.events_pending) {
+    // Set handling_events flag. This prevents the client from being deregistered
-        // There are currently no events, wait for one to occur
-        client_obj->dynamic.flags.blocked = 1;
-        HOST_EXIT_CRITICAL();
-        BaseType_t sem_ret = xSemaphoreTake(client_obj->constant.event_sem, timeout_ticks);
-        HOST_ENTER_CRITICAL();
-        client_obj->dynamic.flags.blocked = 0;
-        if (sem_ret == pdFALSE) {
-            HOST_EXIT_CRITICAL();
-            // Timed out waiting for semaphore
-            ret = ESP_ERR_TIMEOUT;
-            goto exit;
-        }
-    }
-    // Mark that we're processing events
     client_obj->dynamic.flags.handling_events = 1;
-    while (client_obj->dynamic.flags.handling_events) {
+    HOST_EXIT_CRITICAL();
+
+    while (1) {
+        // Loop until there are no more events
+        if (xSemaphoreTake(client_obj->constant.event_sem, timeout_ticks) == pdFALSE) {
+            // Timed out waiting for semaphore or currently no events
+            break;
+        }
+
+        HOST_ENTER_CRITICAL();
         // Handle pending endpoints
         if (!TAILQ_EMPTY(&client_obj->dynamic.pending_ep_tailq)) {
             _handle_pending_ep(client_obj);
@@ -784,29 +765,26 @@ esp_err_t usb_host_client_handle_events(usb_host_client_handle_t client_hdl, Tic
             urb->transfer.callback(&urb->transfer);
             HOST_ENTER_CRITICAL();
         }
+        HOST_EXIT_CRITICAL();
+
         // Handle event messages
         while (uxQueueMessagesWaiting(client_obj->constant.event_msg_queue) > 0) {
-            HOST_EXIT_CRITICAL();
             // Dequeue the event message and call the client event callback
             usb_host_client_event_msg_t event_msg;
             BaseType_t queue_ret = xQueueReceive(client_obj->constant.event_msg_queue, &event_msg, 0);
             assert(queue_ret == pdTRUE);
             client_obj->constant.event_callback(&event_msg, client_obj->constant.callback_arg);
-            HOST_ENTER_CRITICAL();
         }
-        // Check each event again to see any new events occurred
-        if (TAILQ_EMPTY(&client_obj->dynamic.pending_ep_tailq) &&
-                client_obj->dynamic.num_done_ctrl_xfer == 0 &&
-                uxQueueMessagesWaiting(client_obj->constant.event_msg_queue) == 0) {
-            // All pending endpoints and event messages handled
-            client_obj->dynamic.flags.events_pending = 0;
-            client_obj->dynamic.flags.handling_events = 0;
-        }
-    }
-    HOST_EXIT_CRITICAL();
 
         ret = ESP_OK;
-exit:
+        // Set timeout_ticks to 0 so that we can check for events again without blocking
+        timeout_ticks = 0;
+    }
+
+    HOST_ENTER_CRITICAL();
+    client_obj->dynamic.flags.handling_events = 0;
+    HOST_EXIT_CRITICAL();
+
     return ret;
 }