Merge pull request #3509 from hathach/pr3506-followup

follow up to PR3506

AGENTS.md

@@ -101,9 +101,15 @@ make BOARD=raspberry_pi_pico all
 
 ## Hardware-in-the-Loop (HIL) Testing
 
+- `-B examples` means `examples` is the parent folder that contains multi-board build outputs such as `examples/cmake-build-BOARD_NAME/...`
+- Select config file before running HIL tests:
+    - if GitHub Actions self-hosted runner service is running, use `tinyusb.json`
+    - otherwise use `local.json`
+    - example:
+      `HIL_CONFIG=$( (systemctl list-units --type=service --state=running 2>/dev/null; systemctl --user list-units --type=service --state=running 2>/dev/null) | grep -q 'actions\.runner' && echo tinyusb.json || echo local.json )`
 - Run tests on actual hardware, one of following ways:
-    - test a specific board `python test/hil/hil_test.py -b BOARD_NAME -B examples local.json`
-    - test all boards in config `python test/hil/hil_test.py -B examples local.json`
+    - test a specific board `python test/hil/hil_test.py -b BOARD_NAME -B examples $HIL_CONFIG`
+    - test all boards in config `python test/hil/hil_test.py -B examples $HIL_CONFIG`
 - In case of error, enabled verbose mode with `-v` flag for detailed logs. Also try to observe script output, and try to
   modify hil_test.py (temporarily) to add more debug prints to pinpoint the issue.
 - Requires pre-built (all) examples for target boards (see Build Examples section 2)

src/portable/synopsys/dwc2/dcd_dwc2.c

@@ -442,14 +442,14 @@ bool dcd_init(uint8_t rhport, const tusb_rhport_init_t* rh_init) {
   tu_memclr(&_dcd_data, sizeof(_dcd_data));
 
   // Core Initialization
-  const bool highspeed_phy = dwc2_core_is_highspeed_phy(dwc2, TUD_OPT_HIGH_SPEED);
+  const bool is_hs_phy = dwc2_core_is_highspeed_phy(dwc2, TUD_OPT_HIGH_SPEED);
   const bool is_dma = dma_device_enabled(dwc2);
-  TU_ASSERT(dwc2_core_init(rhport, highspeed_phy, is_dma));
+  TU_ASSERT(dwc2_core_init(rhport, is_hs_phy, is_dma));
 
   //------------- 7.1 Device Initialization -------------//
   // Set device max speed
   uint32_t dcfg = dwc2->dcfg & ~DCFG_DSPD_Msk;
-  if (highspeed_phy) {
+  if (is_hs_phy) {
     // dcfg Highspeed's mask is 0
 
     // XCVRDLY: transceiver delay between xcvr_sel and txvalid during device chirp is required
@@ -479,7 +479,7 @@ bool dcd_init(uint8_t rhport, const tusb_rhport_init_t* rh_init) {
 
   #ifdef TUP_USBIP_DWC2_STM32
   dwc2_stm32_gccfg_cfg(dwc2, _tud_cfg.vbus_sensing, false);
-#endif
+  #endif
 
   // Enable required interrupts
   dwc2->gintmsk |= GINTMSK_OTGINT | GINTMSK_USBRST | GINTMSK_ENUMDNEM | GINTMSK_WUIM;

src/portable/synopsys/dwc2/dwc2_common.c

@@ -60,6 +60,7 @@ static void reset_core(dwc2_regs_t* dwc2) {
   while (!(dwc2->grstctl & GRSTCTL_AHBIDL)) {} // wait for AHB master IDLE
 }
 
+// Dedicated FS PHY is internal with a clock 48Mhz.
 static void phy_fs_init(dwc2_regs_t* dwc2) {
   TU_LOG(DWC2_COMMON_DEBUG, "Fullspeed PHY init\r\n");
 
@@ -86,6 +87,13 @@ static void phy_fs_init(dwc2_regs_t* dwc2) {
   dwc2_phy_update(dwc2, GHWCFG2_HSPHY_NOT_SUPPORTED);
 }
 
+/* dwc2 has 2 highspeed PHYs options
+ * - UTMI+ is internal highspeed PHY, can be clocked at 30/60 Mhz for fullspeed or 60 Mhz for highspeed. Can be either
+ * 8 or 16-bit interface.
+ * - ULPI is external highspeed PHY, clocked at 60Mhz with 8-bit interface.
+ *
+ * In addition, UTMI+/ULPI can be shared to run at fullspeed mode with 48Mhz
+ */
 static void phy_hs_init(dwc2_regs_t* dwc2) {
   uint32_t gusbcfg = dwc2->gusbcfg;
   const dwc2_ghwcfg2_t ghwcfg2 = {.value = dwc2->ghwcfg2};
@@ -180,30 +188,19 @@ static bool check_dwc2(dwc2_regs_t* dwc2) {
 //
 //--------------------------------------------------------------------
 bool dwc2_core_is_highspeed_phy(dwc2_regs_t* dwc2, bool prefer_hs_phy) {
-#ifdef TUP_USBIP_DWC2_STM32
-  if (dwc2->guid >= 0x5000) {
-    // femtoPHY UTMI+ PHY
-    return true;
-  }
-#endif
+  const dwc2_ghwcfg2_t ghwcfg2    = {.value = dwc2->ghwcfg2};
+  const bool           has_hs_phy = (ghwcfg2.hs_phy_type != GHWCFG2_HSPHY_NOT_SUPPORTED);
 
-  if (!prefer_hs_phy) {
-    return false;
+  if (prefer_hs_phy) {
+    return has_hs_phy;
+  } else {
+    const bool has_fs_phy = (ghwcfg2.fs_phy_type != GHWCFG2_FSPHY_NOT_SUPPORTED);
+    // false if has fs phy, otherwise true since hs phy is the only available phy
+    return !has_fs_phy && has_hs_phy;
   }
-
-  const dwc2_ghwcfg2_t ghwcfg2 = {.value = dwc2->ghwcfg2};
-  return ghwcfg2.hs_phy_type != GHWCFG2_HSPHY_NOT_SUPPORTED;
 }
 
-/* dwc2 has several PHYs option
- * - UTMI+ is internal highspeed PHY, clock can be 30 Mhz (8-bit) or 60 Mhz (16-bit)
- * - ULPI is external highspeed PHY, clock is 60Mhz with only 8-bit interface
- * - Dedicated FS PHY is internal with clock 48Mhz.
- *
- * In addition, UTMI+/ULPI can be shared to run at fullspeed mode with 48Mhz
- *
-*/
-bool dwc2_core_init(uint8_t rhport, bool highspeed_phy, bool is_dma) {
+bool dwc2_core_init(uint8_t rhport, bool is_hs_phy, bool is_dma) {
   dwc2_regs_t* dwc2 = DWC2_REG(rhport);
 
   // Check Synopsys ID register, failed if controller clock/power is not enabled
@@ -212,7 +209,7 @@ bool dwc2_core_init(uint8_t rhport, bool highspeed_phy, bool is_dma) {
   // disable global interrupt
   dwc2->gahbcfg &= ~GAHBCFG_GINT;
 
-  if (highspeed_phy) {
+  if (is_hs_phy) {
     phy_hs_init(dwc2);
   } else {
     phy_fs_init(dwc2);

src/portable/synopsys/dwc2/dwc2_common.h

@@ -84,8 +84,9 @@ TU_ATTR_ALWAYS_INLINE static inline dwc2_regs_t* DWC2_REG(uint8_t rhport) {
   return (dwc2_regs_t*)_dwc2_controller[rhport].reg_base;
 }
 
+// check if highspeed phy should be used
 bool dwc2_core_is_highspeed_phy(dwc2_regs_t* dwc2, bool prefer_hs_phy);
-bool dwc2_core_init(uint8_t rhport, bool highspeed_phy, bool is_dma);
+bool dwc2_core_init(uint8_t rhport, bool is_hs_phy, bool is_dma);
 void dwc2_core_handle_common_irq(uint8_t rhport, bool in_isr);
 
 //--------------------------------------------------------------------+

src/portable/synopsys/dwc2/hcd_dwc2.c

@@ -112,7 +112,6 @@ typedef struct {
 } hcd_data_t;
 
 static hcd_data_t _hcd_data;
-
 static tuh_configure_dwc2_t _tuh_cfg = {.use_hs_phy = TUH_OPT_HIGH_SPEED};
 
 //--------------------------------------------------------------------
@@ -352,7 +351,7 @@ TU_ATTR_ALWAYS_INLINE static inline uint8_t cal_next_pid(uint8_t pid, uint8_t pa
  * TX periodic (PTX)
  * - At least largest-EPsize*MulCount/4 (MulCount up to 3 for high-bandwidth ISO/interrupt)
 */
-static void dfifo_host_init(uint8_t rhport) {
+static void dfifo_host_init(uint8_t rhport, bool is_hs_phy) {
   const dwc2_controller_t* dwc2_controller = &_dwc2_controller[rhport];
   dwc2_regs_t* dwc2 = DWC2_REG(rhport);
   const dwc2_ghwcfg2_t ghwcfg2 = {.value = dwc2->ghwcfg2};
@@ -365,10 +364,9 @@ static void dfifo_host_init(uint8_t rhport) {
   }
 
   // fixed allocation for now, improve later:
-    // - ptx_largest is limited to 256 for FS since most FS core only has 1024 bytes total
-  bool highspeed_phy = dwc2_core_is_highspeed_phy(dwc2, _tuh_cfg.use_hs_phy);
-  uint32_t nptx_largest = highspeed_phy ? TUSB_EPSIZE_BULK_HS/4 : TUSB_EPSIZE_BULK_FS/4;
-  uint32_t ptx_largest = highspeed_phy ? TUSB_EPSIZE_ISO_HS_MAX/4 : 256/4;
+  // - ptx_largest is limited to 256 for FS since most FS core only has 1024 bytes total
+  uint32_t nptx_largest = is_hs_phy ? TUSB_EPSIZE_BULK_HS / 4 : TUSB_EPSIZE_BULK_FS / 4;
+  uint32_t ptx_largest  = is_hs_phy ? TUSB_EPSIZE_ISO_HS_MAX / 4 : 256 / 4;
 
   uint16_t nptxfsiz = 2 * nptx_largest;
   uint16_t rxfsiz = 2 * (ptx_largest + 2) + ghwcfg2.num_host_ch;
@@ -403,16 +401,14 @@ bool hcd_configure(uint8_t rhport, uint32_t cfg_id, const void* cfg_param) {
 // Initialize controller to host mode
 bool hcd_init(uint8_t rhport, const tusb_rhport_init_t* rh_init) {
   dwc2_regs_t* dwc2 = DWC2_REG(rhport);
-
   tu_memclr(&_hcd_data, sizeof(_hcd_data));
 
   // Core Initialization
-  const bool highspeed_phy = dwc2_core_is_highspeed_phy(dwc2, _tuh_cfg.use_hs_phy);
+  const bool is_hs_phy = dwc2_core_is_highspeed_phy(dwc2, _tuh_cfg.use_hs_phy);
   const bool is_dma = dma_host_enabled(dwc2);
-  TU_ASSERT(dwc2_core_init(rhport, highspeed_phy, is_dma));
+  TU_ASSERT(dwc2_core_init(rhport, is_hs_phy, is_dma));
 
   //------------- 3.1 Host Initialization -------------//
-
   // Enable HFIR reload
   if (dwc2->gsnpsid >= DWC2_CORE_REV_2_92a) {
     dwc2->hfir |= HFIR_RELOAD_CTRL;
@@ -420,29 +416,25 @@ bool hcd_init(uint8_t rhport, const tusb_rhport_init_t* rh_init) {
 
   // force host mode and wait for mode switch
   dwc2->gusbcfg = (dwc2->gusbcfg & ~GUSBCFG_FDMOD) | GUSBCFG_FHMOD;
-#if CFG_TUSB_MCU == OPT_MCU_STM32N6
+ #if CFG_TUSB_MCU == OPT_MCU_STM32N6
   // No hardware detection of Vbus B-session is available on the STM32N6
   dwc2->stm32_gccfg &= ~STM32_GCCFG_VBVALOVAL;
-#endif
+  #endif
+
   while ((dwc2->gintsts & GINTSTS_CMOD) != GINTSTS_CMODE_HOST) {}
 
-#ifdef TUP_USBIP_DWC2_STM32
+  #ifdef TUP_USBIP_DWC2_STM32
   dwc2_stm32_gccfg_cfg(dwc2, false, true);
-#endif
+  #endif
 
-  if (rh_init->speed < TUSB_SPEED_HIGH || !TUH_OPT_HIGH_SPEED) {
-    // disable high speed mode
-    dwc2->hcfg |= HCFG_FSLS_ONLY;
+  if (is_hs_phy && (rh_init->speed == TUSB_SPEED_HIGH || rh_init->speed == TUSB_SPEED_AUTO)) {
+    dwc2->hcfg &= ~HCFG_FSLS_ONLY; // max speed
+  } else {
+    dwc2->hcfg |= HCFG_FSLS_ONLY;  // disable high speed mode
   }
-#if TUH_OPT_HIGH_SPEED
-  else {
-    // work at max supported speed
-    dwc2->hcfg &= ~HCFG_FSLS_ONLY;
-  }
-#endif
 
-  // configure fixed-allocated fifo scheme
-  dfifo_host_init(rhport);
+  // configure a fixed-allocated fifo scheme
+  dfifo_host_init(rhport, is_hs_phy);
 
   dwc2->hprt = HPRT_W1_MASK; // clear all write-1-clear bits
   dwc2->hprt = HPRT_POWER; // turn on VBUS