Merge pull request #230 from hathach/develop

fix #229

src/class/cdc/cdc_device.c

@@ -311,8 +311,8 @@ bool cdcd_control_complete(uint8_t rhport, tusb_control_request_t const * reques
 // return false to stall control endpoint (e.g unsupported request)
 bool cdcd_control_request(uint8_t rhport, tusb_control_request_t const * request)
 {
-  //------------- Class Specific Request -------------//
-  TU_ASSERT(request->bmRequestType_bit.type == TUSB_REQ_TYPE_CLASS);
+  // Handle class request only
+  TU_VERIFY(request->bmRequestType_bit.type == TUSB_REQ_TYPE_CLASS);
 
   // TODO Support multiple interfaces
   uint8_t const itf = 0;

src/class/dfu/dfu_rt_device.c

@@ -79,21 +79,17 @@ bool dfu_rtd_open(uint8_t rhport, tusb_desc_interface_t const * itf_desc, uint16
 bool dfu_rtd_control_complete(uint8_t rhport, tusb_control_request_t const * request)
 {
   (void) rhport;
+  (void) request;
 
-  //------------- Class Specific Request -------------//
-  TU_VERIFY(request->bmRequestType_bit.type == TUSB_REQ_TYPE_CLASS);
-  TU_VERIFY(request->bmRequestType_bit.recipient == TUSB_REQ_RCPT_INTERFACE);
-
+  // nothing to do
   return true;
 }
 
 bool dfu_rtd_control_request(uint8_t rhport, tusb_control_request_t const * request)
 {
-  (void) rhport;
-
-  //------------- Class Specific Request -------------//
-  TU_ASSERT(request->bmRequestType_bit.type == TUSB_REQ_TYPE_CLASS);
-  TU_ASSERT(request->bmRequestType_bit.recipient == TUSB_REQ_RCPT_INTERFACE);
+  // Handle class request only
+  TU_VERIFY(request->bmRequestType_bit.type == TUSB_REQ_TYPE_CLASS);
+  TU_VERIFY(request->bmRequestType_bit.recipient == TUSB_REQ_RCPT_INTERFACE);
 
   switch ( request->bRequest )
   {

src/class/hid/hid_device.c

@@ -191,89 +191,87 @@ bool hidd_open(uint8_t rhport, tusb_desc_interface_t const * desc_itf, uint16_t
 
 // Handle class control request
 // return false to stall control endpoint (e.g unsupported request)
-bool hidd_control_request(uint8_t rhport, tusb_control_request_t const * p_request)
+bool hidd_control_request(uint8_t rhport, tusb_control_request_t const * request)
 {
-  if (p_request->bmRequestType_bit.recipient != TUSB_REQ_RCPT_INTERFACE)
-  {
-    return false;
-  }
-  hidd_interface_t* p_hid = get_interface_by_itfnum( (uint8_t) p_request->wIndex );
+  TU_VERIFY(request->bmRequestType_bit.recipient == TUSB_REQ_RCPT_INTERFACE);
+
+  hidd_interface_t* p_hid = get_interface_by_itfnum( (uint8_t) request->wIndex );
   TU_ASSERT(p_hid);
 
-  if (p_request->bmRequestType_bit.type == TUSB_REQ_TYPE_STANDARD)
+  if (request->bmRequestType_bit.type == TUSB_REQ_TYPE_STANDARD)
   {
     //------------- STD Request -------------//
-    uint8_t const desc_type  = tu_u16_high(p_request->wValue);
-    uint8_t const desc_index = tu_u16_low (p_request->wValue);
+    uint8_t const desc_type  = tu_u16_high(request->wValue);
+    uint8_t const desc_index = tu_u16_low (request->wValue);
     (void) desc_index;
 
-    if (p_request->bRequest == TUSB_REQ_GET_DESCRIPTOR && desc_type == HID_DESC_TYPE_HID)
+    if (request->bRequest == TUSB_REQ_GET_DESCRIPTOR && desc_type == HID_DESC_TYPE_HID)
     {
       TU_VERIFY(p_hid->hid_descriptor != NULL);
-      TU_VERIFY(tud_control_xfer(rhport, p_request, (void*) p_hid->hid_descriptor, p_hid->hid_descriptor->bLength));
+      TU_VERIFY(tud_control_xfer(rhport, request, (void*) p_hid->hid_descriptor, p_hid->hid_descriptor->bLength));
     }
-    else if (p_request->bRequest == TUSB_REQ_GET_DESCRIPTOR && desc_type == HID_DESC_TYPE_REPORT)
+    else if (request->bRequest == TUSB_REQ_GET_DESCRIPTOR && desc_type == HID_DESC_TYPE_REPORT)
     {
       uint8_t const * desc_report = tud_hid_descriptor_report_cb();
-      tud_control_xfer(rhport, p_request, (void*) desc_report, p_hid->report_desc_len);
+      tud_control_xfer(rhport, request, (void*) desc_report, p_hid->report_desc_len);
     }
     else
     {
       return false; // stall unsupported request
     }
   }
-  else if (p_request->bmRequestType_bit.type == TUSB_REQ_TYPE_CLASS)
+  else if (request->bmRequestType_bit.type == TUSB_REQ_TYPE_CLASS)
   {
     //------------- Class Specific Request -------------//
-    switch( p_request->bRequest )
+    switch( request->bRequest )
     {
       case HID_REQ_CONTROL_GET_REPORT:
       {
         // wValue = Report Type | Report ID
-        uint8_t const report_type = tu_u16_high(p_request->wValue);
-        uint8_t const report_id   = tu_u16_low(p_request->wValue);
+        uint8_t const report_type = tu_u16_high(request->wValue);
+        uint8_t const report_id   = tu_u16_low(request->wValue);
 
-        uint16_t xferlen  = tud_hid_get_report_cb(report_id, (hid_report_type_t) report_type, p_hid->epin_buf, p_request->wLength);
+        uint16_t xferlen  = tud_hid_get_report_cb(report_id, (hid_report_type_t) report_type, p_hid->epin_buf, request->wLength);
         TU_ASSERT( xferlen > 0 );
 
-        tud_control_xfer(rhport, p_request, p_hid->epin_buf, xferlen);
+        tud_control_xfer(rhport, request, p_hid->epin_buf, xferlen);
       }
       break;
 
       case  HID_REQ_CONTROL_SET_REPORT:
-        TU_VERIFY(p_request->wLength <= sizeof(p_hid->epout_buf));
-        tud_control_xfer(rhport, p_request, p_hid->epout_buf, p_request->wLength);
+        TU_VERIFY(request->wLength <= sizeof(p_hid->epout_buf));
+        tud_control_xfer(rhport, request, p_hid->epout_buf, request->wLength);
       break;
 
       case HID_REQ_CONTROL_SET_IDLE:
-        p_hid->idle_rate = tu_u16_high(p_request->wValue);
+        p_hid->idle_rate = tu_u16_high(request->wValue);
         if ( tud_hid_set_idle_cb )
         {
           // stall request if callback return false
           if ( !tud_hid_set_idle_cb(p_hid->idle_rate) ) return false;
         }
 
-        tud_control_status(rhport, p_request);
+        tud_control_status(rhport, request);
       break;
 
       case HID_REQ_CONTROL_GET_IDLE:
         // TODO idle rate of report
-        tud_control_xfer(rhport, p_request, &p_hid->idle_rate, 1);
+        tud_control_xfer(rhport, request, &p_hid->idle_rate, 1);
       break;
 
       case HID_REQ_CONTROL_GET_PROTOCOL:
       {
         uint8_t protocol = (uint8_t)(1-p_hid->boot_mode);   // 0 is Boot, 1 is Report protocol
-        tud_control_xfer(rhport, p_request, &protocol, 1);
+        tud_control_xfer(rhport, request, &protocol, 1);
       }
       break;
 
       case HID_REQ_CONTROL_SET_PROTOCOL:
-        p_hid->boot_mode = 1 - p_request->wValue; // 0 is Boot, 1 is Report protocol
+        p_hid->boot_mode = 1 - request->wValue; // 0 is Boot, 1 is Report protocol
 
         if (tud_hid_boot_mode_cb) tud_hid_boot_mode_cb(p_hid->boot_mode);
 
-        tud_control_status(rhport, p_request);
+        tud_control_status(rhport, request);
       break;
 
       default: return false; // stall unsupported request

src/class/midi/midi_device.c

@@ -322,10 +322,9 @@ bool midid_control_complete(uint8_t rhport, tusb_control_request_t const * p_req
 bool midid_control_request(uint8_t rhport, tusb_control_request_t const * p_request)
 {
   (void) rhport;
+  (void) p_request;
 
-  //------------- Class Specific Request -------------//
-  if (p_request->bmRequestType_bit.type != TUSB_REQ_TYPE_CLASS) return false;
-
+  // driver doesn't support any request yet
   return false;
 }
 

src/class/msc/msc_device.c

@@ -149,7 +149,8 @@ bool mscd_open(uint8_t rhport, tusb_desc_interface_t const * itf_desc, uint16_t
 // return false to stall control endpoint (e.g unsupported request)
 bool mscd_control_request(uint8_t rhport, tusb_control_request_t const * p_request)
 {
-  TU_ASSERT(p_request->bmRequestType_bit.type == TUSB_REQ_TYPE_CLASS);
+  // Handle class request only
+  TU_VERIFY(p_request->bmRequestType_bit.type == TUSB_REQ_TYPE_CLASS);
 
   switch ( p_request->bRequest )
   {

src/device/usbd.c

@@ -84,7 +84,7 @@ typedef struct {
   void (* sof              ) (uint8_t rhport);
 } usbd_class_driver_t;
 
-static usbd_class_driver_t const usbd_class_drivers[] =
+static usbd_class_driver_t const _usbd_driver[] =
 {
   #if CFG_TUD_CDC
   {
@@ -183,7 +183,7 @@ static usbd_class_driver_t const usbd_class_drivers[] =
   #endif
 };
 
-enum { USBD_CLASS_DRIVER_COUNT = TU_ARRAY_SIZE(usbd_class_drivers) };
+enum { USBD_CLASS_DRIVER_COUNT = TU_ARRAY_SIZE(_usbd_driver) };
 
 //--------------------------------------------------------------------+
 // DCD Event
@@ -304,7 +304,7 @@ bool tud_init (void)
   for (uint8_t i = 0; i < USBD_CLASS_DRIVER_COUNT; i++)
   {
     TU_LOG2("%s init\r\n", _usbd_driver_str[i]);
-    usbd_class_drivers[i].init();
+    _usbd_driver[i].init();
   }
 
   // Init device controller driver
@@ -325,7 +325,7 @@ static void usbd_reset(uint8_t rhport)
 
   for (uint8_t i = 0; i < USBD_CLASS_DRIVER_COUNT; i++)
   {
-    if ( usbd_class_drivers[i].reset ) usbd_class_drivers[i].reset( rhport );
+    if ( _usbd_driver[i].reset ) _usbd_driver[i].reset( rhport );
   }
 }
 
@@ -414,7 +414,7 @@ void tud_task (void)
           TU_ASSERT(drv_id < USBD_CLASS_DRIVER_COUNT,);
 
           TU_LOG2("  %s xfer callback\r\n", _usbd_driver_str[drv_id]);
-          usbd_class_drivers[drv_id].xfer_cb(event.rhport, ep_addr, event.xfer_complete.result, event.xfer_complete.len);
+          _usbd_driver[drv_id].xfer_cb(event.rhport, ep_addr, event.xfer_complete.result, event.xfer_complete.len);
         }
       }
       break;
@@ -430,9 +430,9 @@ void tud_task (void)
       case DCD_EVENT_SOF:
         for ( uint8_t i = 0; i < USBD_CLASS_DRIVER_COUNT; i++ )
         {
-          if ( usbd_class_drivers[i].sof )
+          if ( _usbd_driver[i].sof )
           {
-            usbd_class_drivers[i].sof(event.rhport);
+            _usbd_driver[i].sof(event.rhport);
           }
         }
       break;
@@ -452,6 +452,16 @@ void tud_task (void)
 // Control Request Parser & Handling
 //--------------------------------------------------------------------+
 
+// Helper to invoke class driver control request handler
+static bool invoke_class_control(uint8_t rhport, uint8_t drvid, tusb_control_request_t const * request)
+{
+  TU_ASSERT(_usbd_driver[drvid].control_request);
+
+  usbd_control_set_complete_callback(_usbd_driver[drvid].control_complete);
+  TU_LOG2("  %s control request\r\n", _usbd_driver_str[drvid]);
+  return _usbd_driver[drvid].control_request(rhport, request);
+}
+
 // This handles the actual request and its response.
 // return false will cause its caller to stall control endpoint
 static bool process_control_request(uint8_t rhport, tusb_control_request_t const * p_request)
@@ -586,21 +596,13 @@ static bool process_control_request(uint8_t rhport, tusb_control_request_t const
           default:
             // forward to class driver: "STD request to Interface"
             // GET HID REPORT DESCRIPTOR falls into this case
-            // stall control endpoint if driver return false
-            usbd_control_set_complete_callback(usbd_class_drivers[drvid].control_complete);
-            TU_LOG2("  %s control request\r\n", _usbd_driver_str[drvid]);
-            TU_VERIFY(usbd_class_drivers[drvid].control_request != NULL &&
-                      usbd_class_drivers[drvid].control_request(rhport, p_request));
+            TU_VERIFY(invoke_class_control(rhport, drvid, p_request));
           break;
         }
       }else
       {
         // forward to class driver: "non-STD request to Interface"
-        // stall control endpoint if driver return false
-        usbd_control_set_complete_callback(usbd_class_drivers[drvid].control_complete);
-        TU_LOG2("  %s control request\r\n", _usbd_driver_str[drvid]);
-        TU_VERIFY(usbd_class_drivers[drvid].control_request != NULL &&
-                  usbd_class_drivers[drvid].control_request(rhport, p_request));
+        TU_VERIFY(invoke_class_control(rhport, drvid, p_request));
       }
     }
     break;
@@ -614,18 +616,12 @@ static bool process_control_request(uint8_t rhport, tusb_control_request_t const
 
       TU_ASSERT(ep_num < TU_ARRAY_SIZE(_usbd_dev.ep2drv) );
 
-      uint8_t const drv_id = _usbd_dev.ep2drv[ep_num][ep_dir];
-      TU_ASSERT(drv_id < USBD_CLASS_DRIVER_COUNT);
+      uint8_t const drvid = _usbd_dev.ep2drv[ep_num][ep_dir];
+      TU_ASSERT(drvid < USBD_CLASS_DRIVER_COUNT);
 
       bool ret = false;
 
-      if ( TUSB_REQ_TYPE_STANDARD != p_request->bmRequestType_bit.type )
-      {
-        // complete callback is also capable of stalling/acking the request
-        usbd_control_set_complete_callback(usbd_class_drivers[drv_id].control_complete);
-      }
-
-      // Then handle if it is standard request
+      // Handle STD request to endpoint
       if ( TUSB_REQ_TYPE_STANDARD == p_request->bmRequestType_bit.type )
       {
         // force return true for standard request
@@ -660,18 +656,21 @@ static bool process_control_request(uint8_t rhport, tusb_control_request_t const
           default: TU_BREAKPOINT(); return false;
         }
       }
-      // Some classes such as TMC needs to clear/re-init its buffer when receiving CLEAR_FEATURE request
-      // We will forward all request targeted endpoint to its class driver
-      // For class-type requests:  must (call tud_control_status(); return true) or (return false)
-      // For std-type requests:    non-std request codes are already discarded.
-      //                           must not call tud_control_status(), and return value will have no effect
-      // class driver is invoked last, so that EP already has EP stall cleared (in event of clear feature EP halt)
-      TU_LOG2("  %s control request\r\n", _usbd_driver_str[drv_id]);
-      if ( usbd_class_drivers[drv_id].control_request &&
-           usbd_class_drivers[drv_id].control_request(rhport, p_request))
+
+      // Some classes such as USBTMC needs to clear/re-init its buffer when receiving CLEAR_FEATURE request
+      // We will forward all request targeted endpoint to class drivers after
+      // - For class-type requests: driver is fully responsible to reply to host
+      // - For std-type requests  : driver init/re-init internal variable/buffer only, and
+      //                            must not call tud_control_status(), driver's return value will have no effect.
+      //                            EP state has already affected (stalled/cleared)
+      if ( invoke_class_control(rhport, drvid, p_request) ) ret = true;
+
+      if ( TUSB_REQ_TYPE_STANDARD == p_request->bmRequestType_bit.type )
       {
-        ret = true;
+        // Set complete callback = NULL since it can also stall the request.
+        usbd_control_set_complete_callback(NULL);
       }
+
       return ret;
     }
     break;
@@ -714,7 +713,7 @@ static bool process_set_config(uint8_t rhport, uint8_t cfg_num)
       uint8_t drv_id;
       for (drv_id = 0; drv_id < USBD_CLASS_DRIVER_COUNT; drv_id++)
       {
-        if ( usbd_class_drivers[drv_id].class_code == desc_itf->bInterfaceClass ) break;
+        if ( _usbd_driver[drv_id].class_code == desc_itf->bInterfaceClass ) break;
       }
       TU_ASSERT( drv_id < USBD_CLASS_DRIVER_COUNT );
 
@@ -724,7 +723,7 @@ static bool process_set_config(uint8_t rhport, uint8_t cfg_num)
 
       uint16_t itf_len=0;
       TU_LOG2("  %s open\r\n", _usbd_driver_str[drv_id]);
-      TU_ASSERT( usbd_class_drivers[drv_id].open(rhport, desc_itf, &itf_len) );
+      TU_ASSERT( _usbd_driver[drv_id].open(rhport, desc_itf, &itf_len) );
       TU_ASSERT( itf_len >= sizeof(tusb_desc_interface_t) );
 
       mark_interface_endpoint(_usbd_dev.ep2drv, p_desc, itf_len, drv_id);

src/device/usbd_control.c

@@ -130,7 +130,9 @@ void usbd_control_set_complete_callback( bool (*fp) (uint8_t, tusb_control_reque
   _ctrl_xfer.complete_cb = fp;
 }
 
-// callback when a transaction complete on DATA stage of control endpoint
+// callback when a transaction complete on
+// - DATA stage of control endpoint or
+// - Status stage
 bool usbd_control_xfer_cb (uint8_t rhport, uint8_t ep_addr, xfer_result_t result, uint32_t xferred_bytes)
 {
   (void) result;