usb: Update USB Host Library

This commit updates the USB Host Library as follows:

- usb_helpers.h
    - Removed dependency on USB Host Library API
    - Added function to print string descriptors
- usbh
    - Fixed bug where an interface/endpoint could be claimed/allocated multiple times
    - Removed redundant device ref_count change
- Added unit test for USB Host Library API usage

components/usb/include/usb/usb_helpers.h

@@ -1,5 +1,5 @@
 /*
- * SPDX-FileCopyrightText: 2015-2021 Espressif Systems (Shanghai) CO LTD
+ * SPDX-FileCopyrightText: 2015-2022 Espressif Systems (Shanghai) CO LTD
  *
  * SPDX-License-Identifier: Apache-2.0
  */
@@ -111,6 +111,44 @@ const usb_ep_desc_t *usb_parse_endpoint_descriptor_by_index(const usb_intf_desc_
  */
 const usb_ep_desc_t *usb_parse_endpoint_descriptor_by_address(const usb_config_desc_t *config_desc, uint8_t bInterfaceNumber, uint8_t bAlternateSetting, uint8_t bEndpointAddress, int *offset);
 
+// ----------------------------------------------- Descriptor Printing -------------------------------------------------
+
+/**
+ * @brief Print class specific descriptor callback
+ *
+ * Optional callback to be provided to usb_print_config_descriptor() function.
+ * The callback is called when when a non-standard descriptor is encountered.
+ * The callback should decode the descriptor as print it.
+ */
+typedef void (*print_class_descriptor_cb)(const usb_standard_desc_t *);
+
+/**
+ * @brief Print device descriptor
+ *
+ * @param devc_desc Device descriptor
+ */
+void usb_print_device_descriptor(const usb_device_desc_t *devc_desc);
+
+/**
+ * @brief Print configuration descriptor
+ *
+ * - This function prints the full contents of a configuration descriptor (including interface and endpoint descriptors)
+ * - When a non-standard descriptor is encountered, this function will call the class_specific_cb if it is provided
+ *
+ * @param cfg_desc Configuration descriptor
+ * @param class_specific_cb Class specific descriptor callback. Can be NULL
+ */
+void usb_print_config_descriptor(const usb_config_desc_t *cfg_desc, print_class_descriptor_cb class_specific_cb);
+
+/**
+ * @brief Print a string descriptor
+ *
+ * This funciton will only print ASCII characters of the UTF-16 encoded string
+ *
+ * @param str_desc String descriptor
+ */
+void usb_print_string_descriptor(const usb_str_desc_t *str_desc);
+
 // ------------------------------------------------------ Misc ---------------------------------------------------------
 
 /**
@@ -118,6 +156,8 @@ const usb_ep_desc_t *usb_parse_endpoint_descriptor_by_address(const usb_config_d
  *
  * This is a convenience function to round up a size/length to an endpoint's MPS (Maximum packet size). This is useful
  * when calculating transfer or buffer lengths of IN endpoints.
+ * - If MPS <= 0, this function will return 0
+ * - If num_bytes <= 0, this function will return 0
  *
  * @param[in] num_bytes Number of bytes
  * @param[in] mps MPS
@@ -125,31 +165,12 @@ const usb_ep_desc_t *usb_parse_endpoint_descriptor_by_address(const usb_config_d
  */
 static inline int usb_round_up_to_mps(int num_bytes, int mps)
 {
-    if (num_bytes < 0 || mps < 0) {
+    if (num_bytes <= 0 || mps <= 0) {    //MPS can never be zero
         return 0;
     }
     return ((num_bytes + mps - 1) / mps) * mps;
 }
 
-/**
- * @brief Print class specific descriptor callback
- *
- * Optional callback to be provided to usb_print_descriptors() function.
- * The callback is called when when a non-standard descriptor is encountered.
- * The callback should decode the descriptor as print it.
- */
-
-typedef void (*print_class_descriptor_cb)(const usb_standard_desc_t *);
-
-/**
- * @brief Prints usb descriptors
- *
- * @param[in] device Handle to device
- * @param[in] class_specific_cb Optional callback to print class specific descriptors
- * @return esp_err_t
- */
-esp_err_t usb_print_descriptors(usb_device_handle_t device, print_class_descriptor_cb class_specific_cb);
-
 #ifdef __cplusplus
 }
 #endif

components/usb/include/usb/usb_host.h

@@ -148,7 +148,7 @@ esp_err_t usb_host_install(const usb_host_config_t *config);
  * - All devices must have been freed by calling usb_host_device_free_all() and receiving the
  *   USB_HOST_LIB_EVENT_FLAGS_ALL_FREE event flag
  *
- * @note If skip_phy_setup was set when the Host Library was installed, the user is responsible for diasbling the
+ * @note If skip_phy_setup was set when the Host Library was installed, the user is responsible for disabling the
  *       underlying Host Controller and USB PHY (internal or external).
  * @return esp_err_t
  */
@@ -159,10 +159,11 @@ esp_err_t usb_host_uninstall(void);
  *
  * - This function handles all of the USB Host Library's processing and should be called repeatedly in a loop
  * - Check event_flags_ret to see if an flags are set indicating particular USB Host Library events
+ * - This function should never be called by multiple threads simultaneously
  *
  * @note This function can block
  * @param[in] timeout_ticks Timeout in ticks to wait for an event to occur
- * @param[out] event_flags_ret Event flags that indicate what USB Host Library event occurred
+ * @param[out] event_flags_ret Event flags that indicate what USB Host Library event occurred.
  * @return esp_err_t
  */
 esp_err_t usb_host_lib_handle_events(TickType_t timeout_ticks, uint32_t *event_flags_ret);
@@ -213,6 +214,7 @@ esp_err_t usb_host_client_deregister(usb_host_client_handle_t client_hdl);
  * @brief USB Host Library client processing function
  *
  * - This function handles all of a client's processing and should be called repeatedly in a loop
+ * - For a particular client, this function should never be called by multiple threads simultaneously
  *
  * @note This function can block
  * @param[in] client_hdl Client handle
@@ -283,8 +285,7 @@ esp_err_t usb_host_device_free_all(void);
  *
  * - This function fills an empty list with the address of connected devices
  * - The Device addresses can then used in usb_host_device_open()
- * - If there are more devices than the list_len, this function will only fill
- *   up to list_len number of devices.
+ * - If there are more devices than the list_len, this function will only fill up to list_len number of devices.
  *
  * @param[in] list_len Length of the empty list
  * @param[inout] dev_addr_list Empty list to be filled

components/usb/test/usb_host/test_usb_host_async.c

@@ -45,7 +45,7 @@ Procedure:
     - Uninstall USB Host Library
 */
 
-TEST_CASE("Test USB Host async (single client)", "[usb_host][ignore]")
+TEST_CASE("Test USB Host async client (single client)", "[usb_host][ignore]")
 {
     test_usb_init_phy();    //Initialize the internal USB PHY and USB Controller for testing
     //Install USB Host
@@ -109,7 +109,7 @@ Procedure:
     - Free all devices
     - Uninstall USB Host Library
 */
-TEST_CASE("Test USB Host async (multi client)", "[usb_host][ignore]")
+TEST_CASE("Test USB Host async client (multi client)", "[usb_host][ignore]")
 {
     test_usb_init_phy();    //Initialize the internal USB PHY and USB Controller for testing
     //Install USB Host
@@ -163,3 +163,145 @@ TEST_CASE("Test USB Host async (multi client)", "[usb_host][ignore]")
     ESP_ERROR_CHECK(usb_host_uninstall());
     test_usb_deinit_phy();  //Deinitialize the internal USB PHY after testing
 }
+
+/*
+Test USB Host Asynchronous API Usage
+
+Requires: This test requires an MSC SCSI device to be attached (see the MSC mock class)
+
+Purpose:
+    - Test that incorrect usage of USB Host Asynchronous API will returns errors
+
+Procedure:
+    - Install USB Host Library
+    - Register two clients and all event handler functions from the same loop
+    - Wait for each client to detect device connection
+    - Check that both clients can open the same device
+    - Check that a client cannot open a non-existent device
+    - Check that only one client can claim a particular interface
+    - Check that a client cannot release an already released interface
+    - Wait for device disconnection
+    - Cleanup
+*/
+
+static uint8_t dev_addr = 0;
+
+typedef enum {
+    CLIENT_TEST_STAGE_NONE,
+    CLIENT_TEST_STAGE_CONN,
+    CLIENT_TEST_STAGE_DCONN,
+} client_test_stage_t;
+
+static void test_async_client_cb(const usb_host_client_event_msg_t *event_msg, void *arg)
+{
+    client_test_stage_t *stage = (client_test_stage_t *)arg;
+
+    switch (event_msg->event) {
+        case USB_HOST_CLIENT_EVENT_NEW_DEV:
+            if (dev_addr == 0) {
+                dev_addr = event_msg->new_dev.address;
+            } else {
+                TEST_ASSERT_EQUAL(dev_addr, event_msg->new_dev.address);
+            }
+            *stage = CLIENT_TEST_STAGE_CONN;
+            break;
+        case USB_HOST_CLIENT_EVENT_DEV_GONE:
+            *stage = CLIENT_TEST_STAGE_DCONN;
+            break;
+        default:
+            abort();
+            break;
+    }
+}
+
+TEST_CASE("Test USB Host async API", "[usb_host][ignore]")
+{
+    test_usb_init_phy();    //Initialize the internal USB PHY and USB Controller for testing
+
+    //Install USB Host
+    usb_host_config_t host_config = {
+        .skip_phy_setup = true,     //test_usb_init_phy() will already have setup the internal USB PHY for us
+        .intr_flags = ESP_INTR_FLAG_LEVEL1,
+    };
+    ESP_ERROR_CHECK(usb_host_install(&host_config));
+    printf("Installed\n");
+
+    //Register two clients
+    client_test_stage_t client0_stage = CLIENT_TEST_STAGE_NONE;
+    client_test_stage_t client1_stage = CLIENT_TEST_STAGE_NONE;
+
+    usb_host_client_config_t client_config = {
+        .is_synchronous = false,
+        .max_num_event_msg = 5,
+        .async = {
+            .client_event_callback = test_async_client_cb,
+            .callback_arg = (void *)&client0_stage,
+        },
+    };
+    usb_host_client_handle_t client0_hdl;
+    usb_host_client_handle_t client1_hdl;
+    TEST_ASSERT_EQUAL(ESP_OK, usb_host_client_register(&client_config, &client0_hdl));
+    client_config.async.callback_arg = (void *)&client1_stage;
+    TEST_ASSERT_EQUAL(ESP_OK, usb_host_client_register(&client_config, &client1_hdl));
+
+    //Wait until the device connects and the clients receive the event
+    while (!(client0_stage == CLIENT_TEST_STAGE_CONN && client1_stage == CLIENT_TEST_STAGE_CONN)) {
+        usb_host_lib_handle_events(0, NULL);
+        usb_host_client_handle_events(client0_hdl, 0);
+        usb_host_client_handle_events(client1_hdl, 0);
+        vTaskDelay(10);
+    }
+
+    //Check that both clients can open the device
+    TEST_ASSERT_NOT_EQUAL(0, dev_addr);
+    usb_device_handle_t client0_dev_hdl;
+    usb_device_handle_t client1_dev_hdl;
+    TEST_ASSERT_EQUAL(ESP_OK, usb_host_device_open(client0_hdl, dev_addr, &client0_dev_hdl));
+    TEST_ASSERT_EQUAL(ESP_OK, usb_host_device_open(client1_hdl, dev_addr, &client1_dev_hdl));
+    TEST_ASSERT_EQUAL(client0_dev_hdl, client1_dev_hdl);    //Check that its the same device
+    //Check that a client cannot open a non-existent device
+    TEST_ASSERT_NOT_EQUAL(ESP_OK, usb_host_device_open(client0_hdl, 0, &client0_dev_hdl));
+
+    //Check that the device cannot be opened again by the same client
+    usb_device_handle_t dummy_dev_hdl;
+    TEST_ASSERT_NOT_EQUAL(ESP_OK, usb_host_device_open(client0_hdl, dev_addr, &dummy_dev_hdl));
+    TEST_ASSERT_NOT_EQUAL(ESP_OK, usb_host_device_open(client1_hdl, dev_addr, &dummy_dev_hdl));
+    //Check that both clients cannot claim the same interface
+    TEST_ASSERT_EQUAL(ESP_OK, usb_host_interface_claim(client0_hdl, client0_dev_hdl, MOCK_MSC_SCSI_INTF_NUMBER, MOCK_MSC_SCSI_INTF_ALT_SETTING));
+    TEST_ASSERT_NOT_EQUAL(ESP_OK, usb_host_interface_claim(client1_hdl, client1_dev_hdl, MOCK_MSC_SCSI_INTF_NUMBER, MOCK_MSC_SCSI_INTF_ALT_SETTING));
+    //Check that client0 cannot claim the same interface multiple times
+    TEST_ASSERT_NOT_EQUAL(ESP_OK, usb_host_interface_claim(client0_hdl, client0_dev_hdl, MOCK_MSC_SCSI_INTF_NUMBER, MOCK_MSC_SCSI_INTF_ALT_SETTING));
+
+    //Check that client0 can release the interface
+    TEST_ASSERT_EQUAL(ESP_OK, usb_host_interface_release(client0_hdl, client0_dev_hdl, MOCK_MSC_SCSI_INTF_NUMBER));
+    //Check that client0 cannot release interface it has not claimed
+    TEST_ASSERT_NOT_EQUAL(ESP_OK, usb_host_interface_release(client0_hdl, client0_dev_hdl, MOCK_MSC_SCSI_INTF_NUMBER));
+
+    //Wait until the device disconnects and the clients receive the event
+    test_usb_set_phy_state(false, 0);
+    while (!(client0_stage == CLIENT_TEST_STAGE_DCONN && client1_stage == CLIENT_TEST_STAGE_DCONN)) {
+        usb_host_lib_handle_events(0, NULL);
+        usb_host_client_handle_events(client0_hdl, 0);
+        usb_host_client_handle_events(client1_hdl, 0);
+        vTaskDelay(10);
+    }
+    TEST_ASSERT_EQUAL(ESP_OK, usb_host_device_close(client0_hdl, client0_dev_hdl));
+    TEST_ASSERT_EQUAL(ESP_OK, usb_host_device_close(client1_hdl, client1_dev_hdl));
+
+    //Deregister the clients
+    TEST_ASSERT_EQUAL(ESP_OK, usb_host_client_deregister(client0_hdl));
+    TEST_ASSERT_EQUAL(ESP_OK, usb_host_client_deregister(client1_hdl));
+
+    while (1) {
+        uint32_t event_flags;
+        usb_host_lib_handle_events(0, &event_flags);
+        if (event_flags & USB_HOST_LIB_EVENT_FLAGS_NO_CLIENTS) {
+            break;
+        }
+        vTaskDelay(10);
+    }
+
+    //Cleanup
+    TEST_ASSERT_EQUAL(ESP_OK, usb_host_uninstall());
+    test_usb_deinit_phy();
+}

components/usb/usb_helpers.c

@@ -1,5 +1,5 @@
 /*
- * SPDX-FileCopyrightText: 2015-2021 Espressif Systems (Shanghai) CO LTD
+ * SPDX-FileCopyrightText: 2015-2022 Espressif Systems (Shanghai) CO LTD
  *
  * SPDX-License-Identifier: Apache-2.0
  */
@@ -15,8 +15,6 @@
 #include "esp_check.h"
 #include "usb/usb_host.h"
 
-static const char *TAG = "usb_helper";
-
 // ---------------------------------------- Configuration Descriptor Parsing -------------------------------------------
 
 const usb_standard_desc_t *usb_parse_next_descriptor(const usb_standard_desc_t *cur_desc, uint16_t wTotalLength, int *offset)
@@ -171,7 +169,7 @@ const usb_ep_desc_t *usb_parse_endpoint_descriptor_by_address(const usb_config_d
     return ep_desc;
 }
 
-// ------------------------------------------ Descriptor printing  ---------------------------------------------
+// ----------------------------------------------- Descriptor Printing -------------------------------------------------
 
 static void print_ep_desc(const usb_ep_desc_t *ep_desc)
 {
@@ -232,8 +230,12 @@ static void usbh_print_cfg_desc(const usb_config_desc_t *cfg_desc)
     printf("bMaxPower %dmA\n", cfg_desc->bMaxPower * 2);
 }
 
-static void print_device_descriptor(const usb_device_desc_t *devc_desc)
+void usb_print_device_descriptor(const usb_device_desc_t *devc_desc)
 {
+    if (devc_desc == NULL) {
+        return;
+    }
+
     printf("*** Device descriptor ***\n");
     printf("bLength %d\n", devc_desc->bLength);
     printf("bDescriptorType %d\n", devc_desc->bDescriptorType);
@@ -251,8 +253,12 @@ static void print_device_descriptor(const usb_device_desc_t *devc_desc)
     printf("bNumConfigurations %d\n", devc_desc->bNumConfigurations);
 }
 
-static void print_config_descriptor(const usb_config_desc_t *cfg_desc, print_class_descriptor_cb class_specific_cb)
+void usb_print_config_descriptor(const usb_config_desc_t *cfg_desc, print_class_descriptor_cb class_specific_cb)
 {
+    if (cfg_desc == NULL) {
+        return;
+    }
+
     int offset = 0;
     uint16_t wTotalLength = cfg_desc->wTotalLength;
     const usb_standard_desc_t *next_desc = (const usb_standard_desc_t *)cfg_desc;
@@ -280,22 +286,21 @@ static void print_config_descriptor(const usb_config_desc_t *cfg_desc, print_cla
     } while (next_desc != NULL);
 }
 
-esp_err_t usb_print_descriptors(usb_device_handle_t device, print_class_descriptor_cb class_specific_cb)
+void usb_print_string_descriptor(const usb_str_desc_t *str_desc)
 {
-    if (device == NULL) {
-        return ESP_ERR_INVALID_ARG;
+    if (str_desc == NULL) {
+        return;
     }
 
-    const usb_config_desc_t *config_desc;
-    const usb_device_desc_t *device_desc;
-
-    ESP_RETURN_ON_ERROR( usb_host_get_device_descriptor(device, &device_desc), TAG, "Failed to get devices descriptor" );
-    ESP_RETURN_ON_ERROR( usb_host_get_active_config_descriptor(device, &config_desc), TAG, "Failed to get config descriptor" );
-
-    print_device_descriptor(device_desc);
-    print_config_descriptor(config_desc, class_specific_cb);
-
-    return ESP_OK;
+    for (int i = 0; i < str_desc->bLength/2; i++) {
+        /*
+        USB String descriptors of UTF-16.
+        Right now We just skip any character larger than 0xFF to stay in BMP Basic Latin and Latin-1 Supplement range.
+        */
+        if (str_desc->wData[i] > 0xFF) {
+            continue;
+        }
+        printf("%c", (char)str_desc->wData[i]);
+    }
+    printf("\n");
 }
-
-// ------------------------------------------------------ Misc ---------------------------------------------------------

components/usb/usb_host.c

@@ -1012,14 +1012,14 @@ static esp_err_t interface_claim(client_t *client_obj, usb_device_handle_t dev_h
         if (ret != ESP_OK) {
             goto ep_alloc_err;
         }
-        //Store endpoint object into interface object
+        //Fill the interface object with the allocated endpoints
         intf_obj->constant.endpoints[i] = ep_obj;
     }
     //Add interface object to client (safe because we have already taken the mutex)
     TAILQ_INSERT_TAIL(&client_obj->mux_protected.interface_tailq, intf_obj, mux_protected.tailq_entry);
     //Add each endpoint to the client's endpoint list
     HOST_ENTER_CRITICAL();
-    for (int i = 0; i < intf_obj->constant.intf_desc->bNumEndpoints; i++) {
+    for (int i = 0; i < intf_desc->bNumEndpoints; i++) {
         TAILQ_INSERT_TAIL(&client_obj->dynamic.idle_ep_tailq, intf_obj->constant.endpoints[i], dynamic.tailq_entry);
     }
     HOST_EXIT_CRITICAL();
@@ -1029,7 +1029,7 @@ static esp_err_t interface_claim(client_t *client_obj, usb_device_handle_t dev_h
     return ret;
 
 ep_alloc_err:
-    for (int i = 0; i < intf_obj->constant.intf_desc->bNumEndpoints; i++) {
+    for (int i = 0; i < intf_desc->bNumEndpoints; i++) {
         endpoint_free(dev_hdl, intf_obj->constant.endpoints[i]);
         intf_obj->constant.endpoints[i] = NULL;
     }

components/usb/usbh.c

@@ -748,13 +748,8 @@ esp_err_t usbh_ep_alloc(usb_device_handle_t dev_hdl, usbh_ep_config_t *ep_config
 {
     USBH_CHECK(dev_hdl != NULL && ep_config != NULL && pipe_hdl_ret != NULL, ESP_ERR_INVALID_ARG);
     device_t *dev_obj = (device_t *)dev_hdl;
-
-    USBH_ENTER_CRITICAL();
-    USBH_CHECK_FROM_CRIT(dev_obj->dynamic.state == USB_DEVICE_STATE_CONFIGURED, ESP_ERR_INVALID_STATE);
-    dev_obj->dynamic.ref_count++;   //Increase the ref_count to keep the device alive while allocating the endpoint
-    USBH_EXIT_CRITICAL();
-
     esp_err_t ret;
+
     //Allocate HCD pipe
     hcd_pipe_config_t pipe_config = {
         .callback = ep_config->pipe_cb,
@@ -776,17 +771,22 @@ esp_err_t usbh_ep_alloc(usb_device_handle_t dev_hdl, usbh_ep_config_t *ep_config
 
     //We need to take the mux_lock to access mux_protected members
     xSemaphoreTake(p_usbh_obj->constant.mux_lock, portMAX_DELAY);
+    USBH_ENTER_CRITICAL();
+    //Check the device's state before we assign the pipes to the endpoint
+    if (dev_obj->dynamic.state != USB_DEVICE_STATE_CONFIGURED) {
+        USBH_EXIT_CRITICAL();
+        ret = ESP_ERR_INVALID_STATE;
+        goto assign_err;
+    }
+    USBH_EXIT_CRITICAL();
+    //Assign the allocated pipe to the correct endpoint
     if (is_in && dev_obj->mux_protected.ep_in[addr - 1] == NULL) {  //Is an IN EP
         dev_obj->mux_protected.ep_in[addr - 1] = pipe_hdl;
         assigned = true;
-    } else {
+    } else if (dev_obj->mux_protected.ep_out[addr - 1] == NULL) {   //Is an OUT EP
         dev_obj->mux_protected.ep_out[addr - 1] = pipe_hdl;
         assigned = true;
     }
-    //Restore ref_count
-    USBH_ENTER_CRITICAL();
-    dev_obj->dynamic.ref_count--;
-    USBH_EXIT_CRITICAL();
     xSemaphoreGive(p_usbh_obj->constant.mux_lock);
 
     if (!assigned) {

examples/peripherals/usb/host/msc/components/msc/src/msc_host.c

@@ -469,7 +469,14 @@ esp_err_t msc_host_get_device_info(msc_host_device_handle_t device, msc_host_dev
 
 esp_err_t msc_host_print_descriptors(msc_host_device_handle_t device)
 {
-    return usb_print_descriptors(((msc_device_t *)device)->handle, NULL);
+    msc_device_t *dev = (msc_device_t *)device;
+    const usb_device_desc_t *device_desc;
+    const usb_config_desc_t *config_desc;
+    MSC_RETURN_ON_ERROR( usb_host_get_device_descriptor(dev->handle, &device_desc) );
+    MSC_RETURN_ON_ERROR( usb_host_get_active_config_descriptor(dev->handle, &config_desc) );
+    usb_print_device_descriptor(device_desc);
+    usb_print_config_descriptor(config_desc, NULL);
+    return ESP_OK;
 }
 
 static void transfer_callback(usb_transfer_t *transfer)