Merge pull request #2710 from hathach/dwc2-test-mode-followup

Author: hathach

src/common/tusb_mcu.h

@@ -195,7 +195,6 @@
 #elif TU_CHECK_MCU(OPT_MCU_STM32F4)
   #define TUP_USBIP_DWC2
   #define TUP_USBIP_DWC2_STM32
-  #define TUP_USBIP_DWC2_TEST_MODE
 
   // For most mcu, FS has 4, HS has 6. TODO 446/469/479 HS has 9
   #define TUP_DCD_ENDPOINT_MAX    6
@@ -210,7 +209,6 @@
   // MCU with on-chip HS Phy
   #if defined(STM32F723xx) || defined(STM32F730xx) || defined(STM32F733xx)
     #define TUP_RHPORT_HIGHSPEED  1 // Port0: FS, Port1: HS
-    #define TUP_USBIP_DWC2_TEST_MODE
   #endif
 
 #elif TU_CHECK_MCU(OPT_MCU_STM32H7)
@@ -285,7 +283,6 @@
         defined(STM32U5F7xx) || defined(STM32U5F9xx) || defined(STM32U5G7xx) || defined(STM32U5G9xx)
       #define TUP_DCD_ENDPOINT_MAX  9
       #define TUP_RHPORT_HIGHSPEED  1
-      #define TUP_USBIP_DWC2_TEST_MODE
     #else
       #define TUP_DCD_ENDPOINT_MAX  6
     #endif

src/common/tusb_types.h

@@ -214,6 +214,15 @@ enum {
 
 #define TUSB_DESC_CONFIG_POWER_MA(x)  ((x)/2)
 
+// USB 2.0 Spec Table 9-7: Test Mode Selectors
+typedef enum {
+  TUSB_FEATURE_TEST_J = 1,
+  TUSB_FEATURE_TEST_K,
+  TUSB_FEATURE_TEST_SE0_NAK,
+  TUSB_FEATURE_TEST_PACKET,
+  TUSB_FEATURE_TEST_FORCE_ENABLE,
+} tusb_feature_test_mode_t;
+
 //--------------------------------------------------------------------+
 //
 //--------------------------------------------------------------------+

src/device/dcd.h

@@ -89,14 +89,6 @@ typedef struct TU_ATTR_ALIGNED(4) {
   };
 } dcd_event_t;
 
-typedef enum {
-  TEST_J = 1,
-  TEST_K,
-  TEST_SE0_NAK,
-  TEST_PACKET,
-  TEST_FORCE_ENABLE,
-} test_mode_t;
-
 //TU_VERIFY_STATIC(sizeof(dcd_event_t) <= 12, "size is not correct");
 
 //--------------------------------------------------------------------+
@@ -150,11 +142,8 @@ void dcd_disconnect(uint8_t rhport);
 void dcd_sof_enable(uint8_t rhport, bool en);
 
 #if CFG_TUD_TEST_MODE
-// Check if the test mode is supported, returns true is test mode selector is supported
-bool dcd_check_test_mode_support(test_mode_t test_selector) TU_ATTR_WEAK;
-
 // Put device into a test mode (needs power cycle to quit)
-void dcd_enter_test_mode(uint8_t rhport, test_mode_t test_selector) TU_ATTR_WEAK;
+void dcd_enter_test_mode(uint8_t rhport, tusb_feature_test_mode_t test_selector);
 #endif
 //--------------------------------------------------------------------+
 // Endpoint API

src/device/usbd.c

@@ -321,9 +321,17 @@ TU_ATTR_ALWAYS_INLINE static inline bool queue_event(dcd_event_t const * event,
 static bool process_control_request(uint8_t rhport, tusb_control_request_t const * p_request);
 static bool process_set_config(uint8_t rhport, uint8_t cfg_num);
 static bool process_get_descriptor(uint8_t rhport, tusb_control_request_t const * p_request);
+
 #if CFG_TUD_TEST_MODE
-static bool process_test_mode_cb(uint8_t rhport, uint8_t stage, tusb_control_request_t const * request);
+static bool process_test_mode_cb(uint8_t rhport, uint8_t stage, tusb_control_request_t const * request) {
+  TU_VERIFY(CONTROL_STAGE_ACK == stage);
+  uint8_t const selector = tu_u16_high(request->wIndex);
+  TU_LOG_USBD("    Enter Test Mode (test selector index: %d)\r\n", selector);
+  dcd_enter_test_mode(rhport, (tusb_feature_test_mode_t) selector);
+  return true;
+}
 #endif
+
 // from usbd_control.c
 void usbd_control_reset(void);
 void usbd_control_set_request(tusb_control_request_t const *request);
@@ -695,7 +703,7 @@ static bool process_control_request(uint8_t rhport, tusb_control_request_t const
       }
 
       if ( TUSB_REQ_TYPE_STANDARD != p_request->bmRequestType_bit.type ) {
-        // Non standard request is not supported
+        // Non-standard request is not supported
         TU_BREAKPOINT();
         return false;
       }
@@ -759,43 +767,27 @@ static bool process_control_request(uint8_t rhport, tusb_control_request_t const
         break;
 
         case TUSB_REQ_SET_FEATURE:
-          // Handle the feature selector
-          switch(p_request->wValue)
-          {
-            // Support for remote wakeup
+          switch(p_request->wValue) {
             case TUSB_REQ_FEATURE_REMOTE_WAKEUP:
               TU_LOG_USBD("    Enable Remote Wakeup\r\n");
-
               // Host may enable remote wake up before suspending especially HID device
               _usbd_dev.remote_wakeup_en = true;
               tud_control_status(rhport, p_request);
             break;
 
-#if CFG_TUD_TEST_MODE
-            // Support for TEST_MODE
+            #if CFG_TUD_TEST_MODE
             case TUSB_REQ_FEATURE_TEST_MODE: {
               // Only handle the test mode if supported and valid
-              TU_VERIFY(dcd_enter_test_mode && dcd_check_test_mode_support && 0 == tu_u16_low(p_request->wIndex));
-
-              uint8_t selector = tu_u16_high(p_request->wIndex);
-
-              // Stall request if the selected test mode isn't supported
-              if (!dcd_check_test_mode_support((test_mode_t)selector))
-              {
-                TU_LOG_USBD("    Unsupported Test Mode (test selector index: %d)\r\n", selector);
+              TU_VERIFY(0 == tu_u16_low(p_request->wIndex));
 
-                return false;
-              }
-
-              // Acknowledge request
-              tud_control_status(rhport, p_request);
-
-              TU_LOG_USBD("    Enter Test Mode (test selector index: %d)\r\n", selector);
+              uint8_t const selector = tu_u16_high(p_request->wIndex);
+              TU_VERIFY(TUSB_FEATURE_TEST_J <= selector && selector <= TUSB_FEATURE_TEST_FORCE_ENABLE);
 
               usbd_control_set_complete_callback(process_test_mode_cb);
+              tud_control_status(rhport, p_request);
               break;
             }
-#endif /* CFG_TUD_TEST_MODE */
+            #endif /* CFG_TUD_TEST_MODE */
 
             // Stall unsupported feature selector
             default: return false;
@@ -1127,20 +1119,6 @@ static bool process_get_descriptor(uint8_t rhport, tusb_control_request_t const
   }
 }
 
-#if CFG_TUD_TEST_MODE
-static bool process_test_mode_cb(uint8_t rhport, uint8_t stage, tusb_control_request_t const * request)
-{
-  // At this point it should already be ensured that dcd_enter_test_mode() is defined
-
-  // Only enter the test mode after the request for it has completed
-  TU_VERIFY(CONTROL_STAGE_ACK == stage);
-
-  dcd_enter_test_mode(rhport, (test_mode_t)tu_u16_high(request->wIndex));
-
-  return true;
-}
-#endif /* CFG_TUD_TEST_MODE */
-
 //--------------------------------------------------------------------+
 // DCD Event Handler
 //--------------------------------------------------------------------+

src/portable/synopsys/dwc2/dcd_dwc2.c

@@ -1195,25 +1195,13 @@ void dcd_int_handler(uint8_t rhport) {
   //  }
 }
 
-#if defined(TUP_USBIP_DWC2_TEST_MODE) && CFG_TUD_TEST_MODE
-
-bool dcd_check_test_mode_support(test_mode_t test_selector) {
-  // Check if test mode selector is unsupported
-  if (TEST_FORCE_ENABLE < test_selector || TEST_J > test_selector) {
-    return false;
-  }
-
-  return true;
-}
-
-void dcd_enter_test_mode(uint8_t rhport, test_mode_t test_selector) {
-  // Get port address...
+#if CFG_TUD_TEST_MODE
+void dcd_enter_test_mode(uint8_t rhport, tusb_feature_test_mode_t test_selector) {
   dwc2_regs_t* dwc2 = DWC2_REG(rhport);
 
   // Enable the test mode
-  dwc2->dctl = (dwc2->dctl & ~DCTL_TCTL_Msk) | (test_selector << DCTL_TCTL_Pos);
+  dwc2->dctl = (dwc2->dctl & ~DCTL_TCTL_Msk) | (((uint8_t) test_selector) << DCTL_TCTL_Pos);
 }
-
-#endif /* TUP_USBIP_DWC2_TEST_MODE && CFG_TUD_TEST_MODE */
+#endif
 
 #endif

src/tusb_option.h

@@ -381,7 +381,7 @@
   #error "CFG_TUD_ENDPPOINT_MAX must be less than or equal to TUP_DCD_ENDPOINT_MAX"
 #endif
 
-// USB 2.0 compliance test mode support
+// USB 2.0 7.1.20: compliance test mode support
 #ifndef CFG_TUD_TEST_MODE
   #define CFG_TUD_TEST_MODE       0
 #endif