From d040644b6cb055532de7d3da3477e20067f1f15f Mon Sep 17 00:00:00 2001 From: Frank Voorburg Date: Sat, 29 Jun 2024 14:25:02 +0200 Subject: [PATCH 1/3] Additional fix related to issue #1018. Corrects the usage of TU_ATTR_WEAK for the Keil compiler for the callback functions: * tud_descriptor_bos_cb() * tud_vendor_control_xfer_cb() * tud_mount_cb() * tud_umount_cb() * tud_suspend_cb() * tud_resume_cb() Without the fix for the first two functions, the USB device won't enumerate properly, if the device makes use of a BOS description. For example when using a Microsoft OS 2.0 platform capability descriptor to set a specific Device Interface GUID for WinUSB. The fix for the other four functions were added, because it's probably just a matter of time before someone runs into the same problem with those callback functions. --- src/device/usbd.c | 34 +++++++++++++++++++++++++++++----- src/device/usbd.h | 12 ++++++------ 2 files changed, 35 insertions(+), 11 deletions(-) diff --git a/src/device/usbd.c b/src/device/usbd.c index 4e723c8b9..719183521 100644 --- a/src/device/usbd.c +++ b/src/device/usbd.c @@ -55,6 +55,30 @@ TU_ATTR_WEAK void tud_sof_cb(uint32_t frame_count) { (void)frame_count; } +TU_ATTR_WEAK uint8_t const * tud_descriptor_bos_cb(void) { + return NULL; +} + +TU_ATTR_WEAK void tud_mount_cb(void) { +} + +TU_ATTR_WEAK void tud_umount_cb(void) { +} + +TU_ATTR_WEAK void tud_suspend_cb(bool remote_wakeup_en) { + (void)remote_wakeup_en; +} + +TU_ATTR_WEAK void tud_resume_cb(void) { +} + +TU_ATTR_WEAK bool tud_vendor_control_xfer_cb(uint8_t rhport, uint8_t stage, tusb_control_request_t const * request) { + (void)rhport; + (void)stage; + (void)request; + return false; +} + TU_ATTR_WEAK bool dcd_deinit(uint8_t rhport) { (void) rhport; return false; @@ -557,7 +581,7 @@ void tud_task_ext(uint32_t timeout_ms, bool in_isr) { case DCD_EVENT_UNPLUGGED: TU_LOG_USBD("\r\n"); usbd_reset(event.rhport); - if (tud_umount_cb) tud_umount_cb(); + tud_umount_cb(); break; case DCD_EVENT_SETUP_RECEIVED: @@ -617,7 +641,7 @@ void tud_task_ext(uint32_t timeout_ms, bool in_isr) { // e.g suspend -> resume -> unplug/plug. Skip suspend/resume if not connected if (_usbd_dev.connected) { TU_LOG_USBD(": Remote Wakeup = %u\r\n", _usbd_dev.remote_wakeup_en); - if (tud_suspend_cb) tud_suspend_cb(_usbd_dev.remote_wakeup_en); + tud_suspend_cb(_usbd_dev.remote_wakeup_en); } else { TU_LOG_USBD(" Skipped\r\n"); } @@ -626,7 +650,7 @@ void tud_task_ext(uint32_t timeout_ms, bool in_isr) { case DCD_EVENT_RESUME: if (_usbd_dev.connected) { TU_LOG_USBD("\r\n"); - if (tud_resume_cb) tud_resume_cb(); + tud_resume_cb(); } else { TU_LOG_USBD(" Skipped\r\n"); } @@ -758,9 +782,9 @@ static bool process_control_request(uint8_t rhport, tusb_control_request_t const _usbd_dev.cfg_num = 0; return false; } - if ( tud_mount_cb ) tud_mount_cb(); + tud_mount_cb(); } else { - if ( tud_umount_cb ) tud_umount_cb(); + tud_umount_cb(); } } diff --git a/src/device/usbd.h b/src/device/usbd.h index e47f674ea..042f2b089 100644 --- a/src/device/usbd.h +++ b/src/device/usbd.h @@ -126,7 +126,7 @@ uint16_t const* tud_descriptor_string_cb(uint8_t index, uint16_t langid); // Invoked when received GET BOS DESCRIPTOR request // Application return pointer to descriptor -TU_ATTR_WEAK uint8_t const * tud_descriptor_bos_cb(void); +uint8_t const * tud_descriptor_bos_cb(void); // Invoked when received GET DEVICE QUALIFIER DESCRIPTOR request // Application return pointer to descriptor, whose contents must exist long enough for transfer to complete. @@ -140,17 +140,17 @@ TU_ATTR_WEAK uint8_t const* tud_descriptor_device_qualifier_cb(void); TU_ATTR_WEAK uint8_t const* tud_descriptor_other_speed_configuration_cb(uint8_t index); // Invoked when device is mounted (configured) -TU_ATTR_WEAK void tud_mount_cb(void); +void tud_mount_cb(void); // Invoked when device is unmounted -TU_ATTR_WEAK void tud_umount_cb(void); +void tud_umount_cb(void); // Invoked when usb bus is suspended // Within 7ms, device must draw an average of current less than 2.5 mA from bus -TU_ATTR_WEAK void tud_suspend_cb(bool remote_wakeup_en); +void tud_suspend_cb(bool remote_wakeup_en); // Invoked when usb bus is resumed -TU_ATTR_WEAK void tud_resume_cb(void); +void tud_resume_cb(void); // Invoked when there is a new usb event, which need to be processed by tud_task()/tud_task_ext() void tud_event_hook_cb(uint8_t rhport, uint32_t eventid, bool in_isr); @@ -159,7 +159,7 @@ void tud_event_hook_cb(uint8_t rhport, uint32_t eventid, bool in_isr); void tud_sof_cb(uint32_t frame_count); // Invoked when received control request with VENDOR TYPE -TU_ATTR_WEAK bool tud_vendor_control_xfer_cb(uint8_t rhport, uint8_t stage, tusb_control_request_t const * request); +bool tud_vendor_control_xfer_cb(uint8_t rhport, uint8_t stage, tusb_control_request_t const * request); //--------------------------------------------------------------------+ // Binary Device Object Store (BOS) Descriptor Templates From e92acf0a91b071a723c408385add083dfe337e8b Mon Sep 17 00:00:00 2001 From: hathach Date: Fri, 19 Jul 2024 20:53:23 +0700 Subject: [PATCH 2/3] also migrate tud_descriptor_device_qualifier_cb() / tud_descriptor_other_speed_configuration_cb() --- src/device/usbd.c | 58 ++++++++++++++++++++++------------------------- src/device/usbd.h | 6 ++--- 2 files changed, 30 insertions(+), 34 deletions(-) diff --git a/src/device/usbd.c b/src/device/usbd.c index 719183521..f0d7c45b1 100644 --- a/src/device/usbd.c +++ b/src/device/usbd.c @@ -46,16 +46,25 @@ // Weak stubs: invoked if no strong implementation is available //--------------------------------------------------------------------+ TU_ATTR_WEAK void tud_event_hook_cb(uint8_t rhport, uint32_t eventid, bool in_isr) { - (void)rhport; - (void)eventid; - (void)in_isr; + (void) rhport; + (void) eventid; + (void) in_isr; } TU_ATTR_WEAK void tud_sof_cb(uint32_t frame_count) { - (void)frame_count; + (void) frame_count; } -TU_ATTR_WEAK uint8_t const * tud_descriptor_bos_cb(void) { +TU_ATTR_WEAK uint8_t const* tud_descriptor_bos_cb(void) { + return NULL; +} + +TU_ATTR_WEAK uint8_t const* tud_descriptor_device_qualifier_cb(void) { + return NULL; +} + +TU_ATTR_WEAK uint8_t const* tud_descriptor_other_speed_configuration_cb(uint8_t index) { + (void) index; return NULL; } @@ -66,16 +75,16 @@ TU_ATTR_WEAK void tud_umount_cb(void) { } TU_ATTR_WEAK void tud_suspend_cb(bool remote_wakeup_en) { - (void)remote_wakeup_en; + (void) remote_wakeup_en; } TU_ATTR_WEAK void tud_resume_cb(void) { } -TU_ATTR_WEAK bool tud_vendor_control_xfer_cb(uint8_t rhport, uint8_t stage, tusb_control_request_t const * request) { - (void)rhport; - (void)stage; - (void)request; +TU_ATTR_WEAK bool tud_vendor_control_xfer_cb(uint8_t rhport, uint8_t stage, tusb_control_request_t const* request) { + (void) rhport; + (void) stage; + (void) request; return false; } @@ -1075,15 +1084,12 @@ static bool process_get_descriptor(uint8_t rhport, tusb_control_request_t const } // break; // unreachable - case TUSB_DESC_BOS: - { + case TUSB_DESC_BOS: { TU_LOG_USBD(" BOS\r\n"); // requested by host if USB > 2.0 ( i.e 2.1 or 3.x ) - if (!tud_descriptor_bos_cb) return false; - uintptr_t desc_bos = (uintptr_t) tud_descriptor_bos_cb(); - TU_ASSERT(desc_bos); + TU_VERIFY(desc_bos); // Use offsetof to avoid pointer to the odd/misaligned address uint16_t const total_len = tu_le16toh( tu_unaligned_read16((const void*) (desc_bos + offsetof(tusb_desc_bos_t, wTotalLength))) ); @@ -1093,24 +1099,20 @@ static bool process_get_descriptor(uint8_t rhport, tusb_control_request_t const // break; // unreachable case TUSB_DESC_CONFIGURATION: - case TUSB_DESC_OTHER_SPEED_CONFIG: - { + case TUSB_DESC_OTHER_SPEED_CONFIG: { uintptr_t desc_config; - if ( desc_type == TUSB_DESC_CONFIGURATION ) - { + if ( desc_type == TUSB_DESC_CONFIGURATION ) { TU_LOG_USBD(" Configuration[%u]\r\n", desc_index); desc_config = (uintptr_t) tud_descriptor_configuration_cb(desc_index); - }else - { + TU_ASSERT(desc_config); + }else { // Host only request this after getting Device Qualifier descriptor TU_LOG_USBD(" Other Speed Configuration\r\n"); - TU_VERIFY( tud_descriptor_other_speed_configuration_cb ); desc_config = (uintptr_t) tud_descriptor_other_speed_configuration_cb(desc_index); + TU_VERIFY(desc_config); } - TU_ASSERT(desc_config); - // Use offsetof to avoid pointer to the odd/misaligned address uint16_t const total_len = tu_le16toh( tu_unaligned_read16((const void*) (desc_config + offsetof(tusb_desc_configuration_t, wTotalLength))) ); @@ -1131,16 +1133,10 @@ static bool process_get_descriptor(uint8_t rhport, tusb_control_request_t const } // break; // unreachable - case TUSB_DESC_DEVICE_QUALIFIER: - { + case TUSB_DESC_DEVICE_QUALIFIER: { TU_LOG_USBD(" Device Qualifier\r\n"); - - TU_VERIFY( tud_descriptor_device_qualifier_cb ); - uint8_t const* desc_qualifier = tud_descriptor_device_qualifier_cb(); TU_VERIFY(desc_qualifier); - - // first byte of descriptor is its size return tud_control_xfer(rhport, p_request, (void*) (uintptr_t) desc_qualifier, tu_desc_len(desc_qualifier)); } // break; // unreachable diff --git a/src/device/usbd.h b/src/device/usbd.h index 042f2b089..3d3e83f41 100644 --- a/src/device/usbd.h +++ b/src/device/usbd.h @@ -109,7 +109,7 @@ bool tud_control_xfer(uint8_t rhport, tusb_control_request_t const * request, vo bool tud_control_status(uint8_t rhport, tusb_control_request_t const * request); //--------------------------------------------------------------------+ -// Application Callbacks (WEAK is optional) +// Application Callbacks //--------------------------------------------------------------------+ // Invoked when received GET DEVICE DESCRIPTOR request @@ -132,12 +132,12 @@ uint8_t const * tud_descriptor_bos_cb(void); // Application return pointer to descriptor, whose contents must exist long enough for transfer to complete. // device_qualifier descriptor describes information about a high-speed capable device that would // change if the device were operating at the other speed. If not highspeed capable stall this request. -TU_ATTR_WEAK uint8_t const* tud_descriptor_device_qualifier_cb(void); +uint8_t const* tud_descriptor_device_qualifier_cb(void); // Invoked when received GET OTHER SEED CONFIGURATION DESCRIPTOR request // Application return pointer to descriptor, whose contents must exist long enough for transfer to complete // Configuration descriptor in the other speed e.g if high speed then this is for full speed and vice versa -TU_ATTR_WEAK uint8_t const* tud_descriptor_other_speed_configuration_cb(uint8_t index); +uint8_t const* tud_descriptor_other_speed_configuration_cb(uint8_t index); // Invoked when device is mounted (configured) void tud_mount_cb(void); From 8183433600a675a54964d0a36efb814f77f38a54 Mon Sep 17 00:00:00 2001 From: hathach Date: Fri, 19 Jul 2024 21:05:10 +0700 Subject: [PATCH 3/3] fix compile with tud_vendor_control_xfer_cb() and check tud_descriptor_device_cb() --- src/device/usbd.c | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/src/device/usbd.c b/src/device/usbd.c index f0d7c45b1..b93216006 100644 --- a/src/device/usbd.c +++ b/src/device/usbd.c @@ -708,8 +708,6 @@ static bool process_control_request(uint8_t rhport, tusb_control_request_t const // Vendor request if ( p_request->bmRequestType_bit.type == TUSB_REQ_TYPE_VENDOR ) { - TU_VERIFY(tud_vendor_control_xfer_cb); - usbd_control_set_complete_callback(tud_vendor_control_xfer_cb); return tud_vendor_control_xfer_cb(rhport, CONTROL_STAGE_SETUP, p_request); } @@ -1060,25 +1058,23 @@ static bool process_get_descriptor(uint8_t rhport, tusb_control_request_t const switch(desc_type) { - case TUSB_DESC_DEVICE: - { + case TUSB_DESC_DEVICE: { TU_LOG_USBD(" Device\r\n"); void* desc_device = (void*) (uintptr_t) tud_descriptor_device_cb(); + TU_ASSERT(desc_device); // Only response with exactly 1 Packet if: not addressed and host requested more data than device descriptor has. // This only happens with the very first get device descriptor and EP0 size = 8 or 16. if ((CFG_TUD_ENDPOINT0_SIZE < sizeof(tusb_desc_device_t)) && !_usbd_dev.addressed && - ((tusb_control_request_t const*) p_request)->wLength > sizeof(tusb_desc_device_t)) - { + ((tusb_control_request_t const*) p_request)->wLength > sizeof(tusb_desc_device_t)) { // Hack here: we modify the request length to prevent usbd_control response with zlp // since we are responding with 1 packet & less data than wLength. tusb_control_request_t mod_request = *p_request; mod_request.wLength = CFG_TUD_ENDPOINT0_SIZE; return tud_control_xfer(rhport, &mod_request, desc_device, CFG_TUD_ENDPOINT0_SIZE); - }else - { + }else { return tud_control_xfer(rhport, p_request, desc_device, sizeof(tusb_desc_device_t)); } }