feat(usb_host): HCD tests ok

Author: peter-marcisovsky

.github/workflows/build_idf_examples.yml

@@ -16,7 +16,7 @@ name: Build ESP-IDF USB examples
 
 on:
   pull_request:
-    types: [opened, reopened, synchronize]
+    types: [opened, reopened]
 
 jobs:
   build:

host/usb/src/hcd_dwc.c

@@ -833,7 +833,6 @@ static hcd_port_event_t _intr_hdlr_hprt(port_t *port, usb_dwc_hal_port_event_t h
     }
     case USB_DWC_HAL_PORT_EVENT_REMOTE_WAKEUP: {
         esp_rom_printf("WAKE\n");
-        //port->state = HCD_PORT_STATE_ENABLED;
         port_event = HCD_PORT_EVENT_REMOTE_WAKEUP;
         break;
     }
@@ -1259,9 +1258,9 @@ static esp_err_t _port_cmd_power_on(port_t *port)
     // Port can only be powered on if it's currently unpowered
     if (port->state == HCD_PORT_STATE_NOT_POWERED) {
         port->state = HCD_PORT_STATE_DISCONNECTED;
+        //assert(_internal_clk_gate(port, false));           // Un-gate the phy clock                  TODO: Test this
         usb_dwc_hal_port_init(port->hal);
         usb_dwc_hal_port_toggle_power(port->hal, true);
-        //_internal_clk_gate(port, false);  // Un-gate the phy clock                  TODO: Test this
         ret = ESP_OK;
     } else {
         ret = ESP_ERR_INVALID_STATE;
@@ -1274,8 +1273,13 @@ static esp_err_t _port_cmd_power_off(port_t *port)
     esp_err_t ret;
     // Port can only be unpowered if already powered
     if (port->state != HCD_PORT_STATE_NOT_POWERED) {
+
+        if (port->state == HCD_PORT_STATE_SUSPENDED) {
+            // un-gate internal clock, to be able to toggle power on the port
+            assert(_internal_clk_gate(port, false));            // Un-gate the phy clock
+        }
         port->state = HCD_PORT_STATE_NOT_POWERED;
-        //_internal_clk_gate(port, false);  // Un-gate the phy clock                  TODO: Test this
+        //assert(_internal_clk_gate(port, false));            // Gate the phy clock                  TODO: Test this
         usb_dwc_hal_port_deinit(port->hal);
         usb_dwc_hal_port_toggle_power(port->hal, false);
         // If a device is currently connected, this should trigger a disconnect event
@@ -1568,7 +1572,6 @@ esp_err_t hcd_port_deinit(hcd_port_handle_t port_hdl)
     return ESP_OK;
 }
 
-
 esp_err_t hcd_port_command(hcd_port_handle_t port_hdl, hcd_port_cmd_t command)
 {
     esp_err_t ret = ESP_ERR_INVALID_STATE;
@@ -1677,6 +1680,7 @@ esp_err_t hcd_port_recover(hcd_port_handle_t port_hdl)
                         ESP_ERR_INVALID_STATE);
 
     // We are about to do a soft reset on the peripheral. Disable the peripheral throughout
+    //assert(_internal_clk_gate(port, false));           // Gate the phy clock
     esp_intr_disable(port->isr_hdl);
     usb_dwc_hal_core_soft_reset(port->hal);
     port->state = HCD_PORT_STATE_NOT_POWERED;
@@ -1686,6 +1690,7 @@ esp_err_t hcd_port_recover(hcd_port_handle_t port_hdl)
     // Clear the frame list. We set the frame list register and enable periodic scheduling after a successful reset
     memset(port->frame_list, 0, FRAME_LIST_LEN * sizeof(uint32_t));
     esp_intr_enable(port->isr_hdl);
+    //assert(_internal_clk_gate(port, true));           // Gate the phy clock
     HCD_EXIT_CRITICAL();
     return ESP_OK;
 }

host/usb/src/hub.c

@@ -456,14 +456,7 @@ static void root_port_handle_events(hcd_port_handle_t root_port_hdl)
         break;
     }
     case HCD_PORT_EVENT_REMOTE_WAKEUP:
-        // Root port, including all the connected devices were resumed (global resume)
-        // Clear all EPs and propagate the resumed event to clients
-        //usbh_devs_set_pm_actions_all(USBH_DEV_RESUME | USBH_DEV_RESUME_EVT);
-
-        // Change Port state
-        //HUB_DRIVER_ENTER_CRITICAL();
-        //p_hub_driver_obj->dynamic.root_port_state = ROOT_PORT_STATE_ENABLED;
-        //HUB_DRIVER_EXIT_CRITICAL();
+        // Mark root port as ready to be resumed
         hub_root_mark_resume();
         break;
     default:

host/usb/src/usb_host.c

@@ -1340,9 +1340,7 @@ esp_err_t usb_host_get_active_config_descriptor(usb_device_handle_t dev_hdl, con
 static usb_transfer_status_t wait_for_transmission_done(usb_transfer_t *transfer)
 {
     SemaphoreHandle_t transfer_done = (SemaphoreHandle_t)transfer->context;
-    esp_rom_printf("Transfer wait\n");
     xSemaphoreTake(transfer_done, portMAX_DELAY);
-    esp_rom_printf("Transfer done\n");
     usb_transfer_status_t status = transfer->status;
 
     // EP0 halt->flush->clear is managed by USBH and lower layers

host/usb/test/target_test/common/hcd_common.c

@@ -127,7 +127,7 @@ void test_hcd_expect_port_event(hcd_port_handle_t port_hdl, hcd_port_event_t exp
     TEST_ASSERT_EQUAL_MESSAGE(pdPASS, ret, "Port event not generated on time");
     // Check the contents of that event message
     TEST_ASSERT_EQUAL(port_hdl, msg.port_hdl);
-    TEST_ASSERT_EQUAL_MESSAGE(expected_event, msg.port_event, "Unexpected event");
+    TEST_ASSERT_EQUAL_MESSAGE(expected_event, msg.port_event, "Unexpected port event");
     printf("\t-> Port event\n");
 }
 
@@ -142,7 +142,7 @@ void test_hcd_expect_pipe_event(hcd_pipe_handle_t pipe_hdl, hcd_pipe_event_t exp
     TEST_ASSERT_EQUAL_MESSAGE(pdPASS, ret, "Pipe event not generated on time");
     // Check the contents of that event message
     TEST_ASSERT_EQUAL(pipe_hdl, msg.pipe_hdl);
-    TEST_ASSERT_EQUAL_MESSAGE(expected_event, msg.pipe_event, "Unexpected event");
+    TEST_ASSERT_EQUAL_MESSAGE(expected_event, msg.pipe_event, "Unexpected pipe event");
 }
 
 void test_hcd_expect_no_pipe_event(hcd_pipe_handle_t pipe_hdl)

host/usb/test/target_test/common/phy_common.h

@@ -5,7 +5,7 @@
  */
 
 #pragma once
-#include "hal/usb_phy_types.h"
+#include "esp_private/usb_phy.h"
 
 /**
  * @brief Install USB PHY separately from the usb_host_install()

host/usb/test/target_test/hcd/main/test_hcd_port.c

@@ -242,6 +242,7 @@ TEST_CASE("Test HCD port suspend and resume sudden disconnect", "[port][low_spee
 
     // Power-off the port to trigger a disconnection
     TEST_ASSERT_EQUAL(ESP_OK, hcd_port_command(port_hdl, HCD_PORT_CMD_POWER_OFF));
+    printf("COmmand\n");
     // Disconnect event should have occurred. Handle the port event
     test_hcd_expect_port_event(port_hdl, HCD_PORT_EVENT_DISCONNECTION);
     TEST_ASSERT_EQUAL(HCD_PORT_EVENT_DISCONNECTION, hcd_port_handle_event(port_hdl));
@@ -505,8 +506,6 @@ TEST_CASE("Test HCD port remote wakeup", "[port][low_speed][full_speed][high_spe
     TEST_ASSERT_EQUAL(HCD_PORT_STATE_ENABLED, hcd_port_get_state(port_hdl));
     printf("Resumed\n");
 
-    //vTaskDelay(pdMS_TO_TICKS(5000));
-
     // Clear the pipe after the port has been resumed
     TEST_ASSERT_EQUAL(HCD_PIPE_STATE_HALTED, hcd_pipe_get_state(default_pipe));
     TEST_ASSERT_EQUAL(ESP_OK, hcd_pipe_command(default_pipe, HCD_PIPE_CMD_CLEAR));

host/usb/test/target_test/hcd/pytest_usb_hcd.py

@@ -3,10 +3,11 @@
 
 import pytest
 from pytest_embedded_idf.dut import IdfDut
+from pytest_embedded_idf.utils import idf_parametrize
 
-@pytest.mark.esp32s2
-@pytest.mark.esp32s3
-@pytest.mark.esp32p4
+#@pytest.mark.esp32s2
+#@pytest.mark.esp32s3
+#@pytest.mark.esp32p4
 @pytest.mark.usb_host_flash_disk
 
 # No build for different configs yet, leaving this commented out
@@ -15,10 +16,10 @@
 #    [('default', 'esp32s2'), ('default', 'esp32s3'), ('default', 'esp32p4'), ('esp32p4_psram', 'esp32p4')],
 #    indirect=['config', 'target'],
 #)
-#@idf_parametrize('target', ['esp32s2', 'esp32s3', 'esp32p4'], indirect=['target'])
+@idf_parametrize('target', ['esp32s2', 'esp32s3', 'esp32p4'], indirect=['target'])
 
 def test_usb_hcd(dut: IdfDut) -> None:
     if dut.target == 'esp32p4':
-        dut.run_all_single_board_cases(group='high_speed', reset=True)
+        dut.run_all_single_board_cases(group='high_speed')
     else:
-        dut.run_all_single_board_cases(group='full_speed', reset=True)
+        dut.run_all_single_board_cases(group='full_speed')

host/usb/test/target_test/usb_host/main/Kconfig.projbuild

@@ -22,4 +22,10 @@ menu "USB Host Library Test"
             USB Host tests that check string descriptors will check the serial string descriptor
             of the connected device.
 
+    config USB_HOST_TEST_DEVICE_HAS_REMOTE_WAKEUP
+        bool "Connected device has remote wakeup feature"
+        default n
+        help
+            Run the remote wakeup test with a device which has, or which does not have remote wakeup functionality
+
 endmenu

host/usb/test/target_test/usb_host/main/remote_wake_client.c

@@ -178,25 +178,32 @@ void test_remote_wake_finish(void)
  * Following functions must not be called from the usb host client task, because a control transfer is sent by those functions
  */
 
-void test_remote_wake_enable(void)
+void test_remote_wake_enable(bool has_remote_wake)
 {
     ESP_LOGI(REMOTE_WAKE_CLIENT_TAG, "Remote wake enable");
-    TEST_ASSERT_EQUAL(ESP_OK, usb_host_device_remote_wakeup_enable(s_remote_wake_obj->client_hdl, s_remote_wake_obj->dev_hdl, true));
+
+    esp_err_t ret = usb_host_device_remote_wakeup_enable(s_remote_wake_obj->client_hdl, s_remote_wake_obj->dev_hdl, true);
+    TEST_ASSERT_EQUAL(ret, ((has_remote_wake) ? (ESP_OK) : (ESP_ERR_NOT_ALLOWED)));
     xSemaphoreGive(s_remote_wake_obj->test_param.dev_ready_smp);
 }
 
-void test_remote_wake_disable(void)
+void test_remote_wake_disable(bool has_remote_wake)
 {
     ESP_LOGI(REMOTE_WAKE_CLIENT_TAG, "Remote wake disable");
-    TEST_ASSERT_EQUAL(ESP_OK, usb_host_device_remote_wakeup_enable(s_remote_wake_obj->client_hdl, s_remote_wake_obj->dev_hdl, false));
+
+    esp_err_t ret = usb_host_device_remote_wakeup_enable(s_remote_wake_obj->client_hdl, s_remote_wake_obj->dev_hdl, false);
+    TEST_ASSERT_EQUAL(ret, ((has_remote_wake) ? (ESP_OK) : (ESP_ERR_NOT_ALLOWED)));
     xSemaphoreGive(s_remote_wake_obj->test_param.dev_ready_smp);
 }
 
-void test_remote_wake_check(bool expected_remote_wake)
+void test_remote_wake_check(bool expected_current_remote_wake, bool has_remote_wake)
 {
     ESP_LOGI(REMOTE_WAKE_CLIENT_TAG, "Remote wake check");
+
     bool remote_wake_enabled = false;
-    TEST_ASSERT_EQUAL(ESP_OK, usb_host_device_remote_wakeup_check(s_remote_wake_obj->client_hdl, s_remote_wake_obj->dev_hdl, &remote_wake_enabled));
-    TEST_ASSERT_EQUAL_MESSAGE(expected_remote_wake, remote_wake_enabled, "Expected remote wakeup status not equal to the current status");
+    esp_err_t ret = usb_host_device_remote_wakeup_check(s_remote_wake_obj->client_hdl, s_remote_wake_obj->dev_hdl, &remote_wake_enabled);
+    TEST_ASSERT_EQUAL_MESSAGE(ret, ((has_remote_wake) ? (ESP_OK) : (ESP_ERR_NOT_ALLOWED)),
+                              "Unexpected remote wakeup check result");
+    TEST_ASSERT_EQUAL_MESSAGE(expected_current_remote_wake, remote_wake_enabled, "Expected remote wakeup status not equal to the current status");
     xSemaphoreGive(s_remote_wake_obj->test_param.dev_ready_smp);
 }