Merge branch 'refactor/usb_merge_p4_phy_backport_v5.4' into 'release/v5.4'

refactor(usb/phy): Merge P4 and S2/S3 PHYs into one module (backport 5.4)

See merge request espressif/esp-idf!36449

Author: suda-morris

components/usb/CMakeLists.txt

@@ -19,12 +19,9 @@ if(CONFIG_SOC_USB_OTG_SUPPORTED)
                      "usb_helpers.c"
                      "usb_host.c"
                      "usb_private.c"
-                     "usbh.c")
-    if(NOT ${target} STREQUAL "esp32p4")
-        list(APPEND srcs "usb_phy.c")
-    else()
-        list(APPEND srcs "usb_phy_p4.c")
-    endif()
+                     "usbh.c"
+                     "usb_phy.c"
+                     )
     list(APPEND include "include")
     list(APPEND priv_includes "private_include")
 endif()

components/usb/include/esp_private/usb_phy.h

@@ -11,6 +11,8 @@
 #include "soc/soc_caps.h"
 #include "hal/usb_phy_types.h"
 
+#define USB_PHY_SUPPORTS_P4_OTG11 1 // This version of usb_phy supports P4 OTG1.1 PHY
+
 #ifdef __cplusplus
 extern "C" {
 #endif

components/usb/include/usb/usb_host.h

@@ -1,5 +1,5 @@
 /*
- * SPDX-FileCopyrightText: 2015-2024 Espressif Systems (Shanghai) CO LTD
+ * SPDX-FileCopyrightText: 2015-2025 Espressif Systems (Shanghai) CO LTD
  *
  * SPDX-License-Identifier: Apache-2.0
  */

components/usb/test_apps/.build-test-rules.yml

@@ -21,10 +21,6 @@ components/usb/test_apps:
 components/usb/test_apps/phy:
   enable:
     - if: SOC_USB_OTG_SUPPORTED == 1
-  disable_test:
-    - if: IDF_TARGET in ["esp32p4"]
-      temporary: true
-      reason: ESP32-P4 PHY driver not yet migrated
   depends_components:
     - usb
   depends_filepatterns:

components/usb/test_apps/hcd/CMakeLists.txt

@@ -6,6 +6,6 @@ include($ENV{IDF_PATH}/tools/cmake/project.cmake)
 set(EXTRA_COMPONENT_DIRS "../common")
 
 # "Trim" the build. Include the minimal set of components, main, and anything it depends on.
-set(COMPONENTS main)
+idf_build_set_property(MINIMAL_BUILD ON)
 
 project(test_app_usb_host)

components/usb/test_apps/hcd/main/test_hcd_common.c

@@ -1,5 +1,5 @@
 /*
- * SPDX-FileCopyrightText: 2015-2024 Espressif Systems (Shanghai) CO LTD
+ * SPDX-FileCopyrightText: 2015-2025 Espressif Systems (Shanghai) CO LTD
  *
  * SPDX-License-Identifier: Apache-2.0
  */
@@ -153,7 +153,11 @@ hcd_port_handle_t test_hcd_setup(void)
     // Initialize the internal USB PHY to connect to the USB OTG peripheral
     usb_phy_config_t phy_config = {
         .controller = USB_PHY_CTRL_OTG,
+#if CONFIG_IDF_TARGET_ESP32P4 // ESP32-P4 has 2 USB-DWC peripherals, each with its dedicated PHY. We support HS+UTMI only ATM.
+        .target = USB_PHY_TARGET_UTMI,
+#else
         .target = USB_PHY_TARGET_INT,
+#endif
         .otg_mode = USB_OTG_MODE_HOST,
         .otg_speed = USB_PHY_SPEED_UNDEFINED,   // In Host mode, the speed is determined by the connected device
         .ext_io_conf = NULL,

components/usb/test_apps/phy/CMakeLists.txt

@@ -4,6 +4,6 @@ cmake_minimum_required(VERSION 3.16)
 include($ENV{IDF_PATH}/tools/cmake/project.cmake)
 
 # "Trim" the build. Include the minimal set of components, main, and anything it depends on.
-set(COMPONENTS main)
+idf_build_set_property(MINIMAL_BUILD ON)
 
 project(test_app_usb_phy)

components/usb/test_apps/phy/main/test_app_main.c

@@ -10,6 +10,7 @@
 #include "esp_private/usb_phy.h"
 #include "hal/usb_wrap_ll.h" // For USB_WRAP_LL_EXT_PHY_SUPPORTED symbol
 #include "soc/soc_caps.h"    // For SOC_USB_UTMI_PHY_NUM symbol
+#include "sdkconfig.h"       // For CONFIG_IDF_TARGET_***
 
 #if USB_WRAP_LL_EXT_PHY_SUPPORTED
 #define EXT_PHY_SUPPORTED 1
@@ -55,9 +56,13 @@ void app_main(void)
 
 /**
  * Test init and deinit of internal FSLS PHY
+ *
+ * 1. Init + deinit in Host mode
+ * 2. Init + deinit in Device mode
  */
 TEST_CASE("Init internal FSLS PHY", "[phy]")
 {
+    // Host mode
     usb_phy_handle_t phy_handle = NULL;
     const usb_phy_config_t phy_config = {
         .controller = USB_PHY_CTRL_OTG,
@@ -70,6 +75,20 @@ TEST_CASE("Init internal FSLS PHY", "[phy]")
     TEST_ASSERT_EQUAL(ESP_OK, usb_new_phy(&phy_config, &phy_handle));
     TEST_ASSERT_NOT_NULL(phy_handle);
     TEST_ASSERT_EQUAL(ESP_OK, usb_del_phy(phy_handle));
+
+    // Device mode
+    usb_phy_handle_t phy_handle_2 = NULL;
+    const usb_phy_config_t phy_config_2 = {
+        .controller = USB_PHY_CTRL_OTG,
+        .target = USB_PHY_TARGET_INT,
+        .otg_mode = USB_OTG_MODE_DEVICE,
+        .otg_speed = USB_PHY_SPEED_FULL,
+        .ext_io_conf = NULL,
+        .otg_io_conf = NULL,
+    };
+    TEST_ASSERT_EQUAL(ESP_OK, usb_new_phy(&phy_config_2, &phy_handle_2));
+    TEST_ASSERT_NOT_NULL(phy_handle_2);
+    TEST_ASSERT_EQUAL(ESP_OK, usb_del_phy(phy_handle_2));
 }
 
 /**
@@ -137,44 +156,85 @@ TEST_CASE("Init internal UTMI PHY", "[phy]")
 }
 
 /**
- * Test init and deinit of all PHYs at the same time
+ * Test init and deinit of all PHYs at the same time multiple times
  */
-TEST_CASE("Init all PHYs", "[phy]")
+TEST_CASE("Init all PHYs in a loop", "[phy]")
 {
+    for (int i = 0; i < 2; i++) {
+        usb_phy_handle_t phy_handle = NULL;
+        usb_phy_handle_t phy_handle_2 = NULL;
+        usb_phy_config_t phy_config = {
+            .controller = USB_PHY_CTRL_OTG,
+            .target = USB_PHY_TARGET_INT,
+            .otg_mode = USB_OTG_MODE_HOST,
+            .otg_speed = USB_PHY_SPEED_UNDEFINED,
+            .ext_io_conf = NULL,
+            .otg_io_conf = NULL,
+        };
+        TEST_ASSERT_EQUAL(ESP_OK, usb_new_phy(&phy_config, &phy_handle));
+        TEST_ASSERT_NOT_NULL(phy_handle);
+
+        // Our current targets support either UTMI or external PHY
+        // so if/else suffice here
+#if UTMI_PHY_SUPPORTED
+        phy_config.target = USB_PHY_TARGET_UTMI;
+#else
+        phy_config.target = USB_PHY_TARGET_EXT;
+        const usb_phy_ext_io_conf_t ext_io_conf = { // Some random values
+            .vp_io_num = 1,
+            .vm_io_num = 1,
+            .rcv_io_num = 1,
+            .suspend_n_io_num = 1,
+            .oen_io_num = 1,
+            .vpo_io_num = 1,
+            .vmo_io_num = 1,
+            .fs_edge_sel_io_num = 1,
+        };
+        phy_config.ext_io_conf = &ext_io_conf;
+#endif
+        TEST_ASSERT_EQUAL(ESP_OK, usb_new_phy(&phy_config, &phy_handle_2));
+        TEST_ASSERT_NOT_NULL(phy_handle_2);
+
+        TEST_ASSERT_EQUAL(ESP_OK, usb_del_phy(phy_handle));
+        TEST_ASSERT_EQUAL(ESP_OK, usb_del_phy(phy_handle_2));
+    }
+}
+
+#if CONFIG_IDF_TARGET_ESP32P4
+/**
+ * Test backward compatibility of ESP32-P4 PHY
+ *
+ * Initial P4 device support was for USB-DWC HS and UTMI PHY. To maintain backward compatibility on ESP32-P4 in USB Device mode,
+ * we select UTMI PHY in case otg_speed is UNDEFINED or HIGH
+ */
+TEST_CASE("ESP32-P4 TinyUSB backward compatibility", "[phy]")
+{
+    // This configuration is used in esp_tinyusb
     usb_phy_handle_t phy_handle = NULL;
-    usb_phy_handle_t phy_handle_2 = NULL;
     usb_phy_config_t phy_config = {
         .controller = USB_PHY_CTRL_OTG,
         .target = USB_PHY_TARGET_INT,
-        .otg_mode = USB_OTG_MODE_HOST,
+        .otg_mode = USB_OTG_MODE_DEVICE,
         .otg_speed = USB_PHY_SPEED_UNDEFINED,
         .ext_io_conf = NULL,
         .otg_io_conf = NULL,
     };
     TEST_ASSERT_EQUAL(ESP_OK, usb_new_phy(&phy_config, &phy_handle));
     TEST_ASSERT_NOT_NULL(phy_handle);
+    TEST_ASSERT_EQUAL(ESP_OK, usb_del_phy(phy_handle));
 
-    // Our current targets support either UTMI or external PHY
-    // so if/else suffice here
-#if UTMI_PHY_SUPPORTED
-    phy_config.target = USB_PHY_TARGET_UTMI;
-#else
-    const usb_phy_ext_io_conf_t ext_io_conf = { // Some random values
-        .vp_io_num = 1,
-        .vm_io_num = 1,
-        .rcv_io_num = 1,
-        .suspend_n_io_num = 1,
-        .oen_io_num = 1,
-        .vpo_io_num = 1,
-        .vmo_io_num = 1,
-        .fs_edge_sel_io_num = 1,
+    // This configuration is used in upstream tinyusb examples
+    usb_phy_handle_t phy_handle_2 = NULL;
+    usb_phy_config_t phy_config_2 = {
+        .controller = USB_PHY_CTRL_OTG,
+        .target = USB_PHY_TARGET_INT,
+        .otg_mode = USB_OTG_MODE_DEVICE,
+        .otg_speed = USB_PHY_SPEED_HIGH,
+        .ext_io_conf = NULL,
+        .otg_io_conf = NULL,
     };
-    phy_config.target = USB_PHY_TARGET_EXT;
-    phy_config.ext_io_conf = &ext_io_conf;
-#endif
-    TEST_ASSERT_EQUAL(ESP_OK, usb_new_phy(&phy_config, &phy_handle_2));
+    TEST_ASSERT_EQUAL(ESP_OK, usb_new_phy(&phy_config_2, &phy_handle_2));
     TEST_ASSERT_NOT_NULL(phy_handle_2);
-
-    TEST_ASSERT_EQUAL(ESP_OK, usb_del_phy(phy_handle));
     TEST_ASSERT_EQUAL(ESP_OK, usb_del_phy(phy_handle_2));
 }
+#endif

components/usb/test_apps/phy/pytest_usb_phy.py

@@ -8,6 +8,5 @@
 @pytest.mark.esp32s3
 @pytest.mark.esp32p4
 @pytest.mark.generic
-@pytest.mark.temp_skip_ci(targets=['esp32p4'], reason='P4 PHY not yet migrated')
 def test_usb_phy(dut: Dut) -> None:
     dut.run_all_single_board_cases(group='phy')

components/usb/test_apps/usb_host/CMakeLists.txt

@@ -6,6 +6,6 @@ include($ENV{IDF_PATH}/tools/cmake/project.cmake)
 set(EXTRA_COMPONENT_DIRS "../common")
 
 # "Trim" the build. Include the minimal set of components, main, and anything it depends on.
-set(COMPONENTS main)
+idf_build_set_property(MINIMAL_BUILD ON)
 
 project(test_app_usb_host)