diff --git a/firmware/RAMNV1/Core/Inc/ramn_config.h b/firmware/RAMNV1/Core/Inc/ramn_config.h index 2abeb5e6..0d0cc6e2 100644 --- a/firmware/RAMNV1/Core/Inc/ramn_config.h +++ b/firmware/RAMNV1/Core/Inc/ramn_config.h @@ -64,7 +64,7 @@ #define USBD_LANGID_STRING 1033 #define USBD_MANUFACTURER_STRING "Toyota Motor Corporation" -#define USBD_PRODUCT_STRING "RAMN USB Composite Device" +#define USBD_PRODUCT_STRING "RAMN gs_usb Device" #define USBD_CONFIGURATION_STRING "MDC Config" #define USBD_INTERFACE_STRING "MDC Interface" @@ -127,7 +127,6 @@ #define GS_HOST_FRAME_SIZE 80 #define CAN_QUEUE_SIZE 512 #endif -#define GSUSB_DFU_INTERFACE_STRING_FS (uint8_t*) "RAMN gs_usb interface" #endif diff --git a/firmware/RAMNV1/Drivers/STM32L5xx_HAL_Driver/Src/stm32l5xx_hal_pcd.c b/firmware/RAMNV1/Drivers/STM32L5xx_HAL_Driver/Src/stm32l5xx_hal_pcd.c index 4d116938..c821d150 100644 --- a/firmware/RAMNV1/Drivers/STM32L5xx_HAL_Driver/Src/stm32l5xx_hal_pcd.c +++ b/firmware/RAMNV1/Drivers/STM32L5xx_HAL_Driver/Src/stm32l5xx_hal_pcd.c @@ -1470,8 +1470,21 @@ HAL_StatusTypeDef HAL_PCD_EP_Receive(PCD_HandleTypeDef *hpcd, uint8_t ep_addr, u ep->is_in = 0U; ep->num = ep_addr & EP_ADDR_MSK; + /* USER CODE BEGIN HAL_PCD_EP_Receive_Concurrency_Fix */ + /* WARNING: Disable USB IRQ to prevent HAL_PCD_EP_Receive and + * HAL_PCD_EP_Transmit from preempting each other, avoiding + * USBD_BUSY deadlocks on AMD xHCI controllers. + * Ref: https://community.st.com/t5/stm32-mcus-embedded-software/usb-cdc-device-receive-fails-on-transmit/td-p/472929 + * WARNING: STM32CubeMX will overwrite this file; reapply this fix. */ + HAL_NVIC_DisableIRQ(USB_FS_IRQn); + __HAL_LOCK(hpcd); + (void)USB_EPStartXfer(hpcd->Instance, ep); + __HAL_UNLOCK(hpcd); + HAL_NVIC_EnableIRQ(USB_FS_IRQn); + /* USER CODE END HAL_PCD_EP_Receive_Concurrency_Fix */ + return HAL_OK; } @@ -1508,8 +1521,21 @@ HAL_StatusTypeDef HAL_PCD_EP_Transmit(PCD_HandleTypeDef *hpcd, uint8_t ep_addr, ep->is_in = 1U; ep->num = ep_addr & EP_ADDR_MSK; + /* USER CODE BEGIN HAL_PCD_EP_Transmit_Concurrency_Fix */ + /* WARNING: Disable USB IRQ to prevent HAL_PCD_EP_Transmit and + * HAL_PCD_EP_Receive from preempting each other, avoiding + * USBD_BUSY deadlocks on AMD xHCI controllers. + * Ref: https://community.st.com/t5/stm32-mcus-embedded-software/usb-cdc-device-receive-fails-on-transmit/td-p/472929 + * WARNING: STM32CubeMX will overwrite this file; reapply this fix. */ + HAL_NVIC_DisableIRQ(USB_FS_IRQn); + __HAL_LOCK(hpcd); + (void)USB_EPStartXfer(hpcd->Instance, ep); + __HAL_UNLOCK(hpcd); + HAL_NVIC_EnableIRQ(USB_FS_IRQn); + /* USER CODE END HAL_PCD_EP_Transmit_Concurrency_Fix */ + return HAL_OK; } diff --git a/firmware/RAMNV1/Middlewares/ST/STM32_USB_Device_Library/USBClass/Composite/gs_usb/usbd_gs_usb.c b/firmware/RAMNV1/Middlewares/ST/STM32_USB_Device_Library/USBClass/Composite/gs_usb/usbd_gs_usb.c index f5a89d3a..5889f805 100644 --- a/firmware/RAMNV1/Middlewares/ST/STM32_USB_Device_Library/USBClass/Composite/gs_usb/usbd_gs_usb.c +++ b/firmware/RAMNV1/Middlewares/ST/STM32_USB_Device_Library/USBClass/Composite/gs_usb/usbd_gs_usb.c @@ -44,7 +44,7 @@ #include "gs_usb_breq.h" #ifdef ENABLE_GSUSB -static uint8_t gsusb_vendor_request(USBD_HandleTypeDef *pdev, USBD_SetupReqTypedef *req); +static uint8_t gsusb_config_request(USBD_HandleTypeDef *pdev, USBD_SetupReqTypedef *req); // device info static const struct gs_device_config gscan_dconf = { @@ -77,12 +77,13 @@ const struct gs_device_bt_const gscan_btconst = { }; -/* Microsoft Compatible ID Feature Descriptor */ +/* WARNING: Only 1 section mapping interface 0 (gs_usb) to WINUSB. + * Do NOT add sections for other interfaces (e.g. CDC). */ static const uint8_t USBD_MS_COMP_ID_FEATURE_DESC[] = { - 0x40, 0x00, 0x00, 0x00, /* length */ + 0x28, 0x00, 0x00, 0x00, /* length */ 0x00, 0x01, /* version 1.0 */ 0x04, 0x00, /* descr index (0x0004) */ - 0x02, /* number of sections */ + 0x01, /* number of sections */ 0x00, 0x00, 0x00, 0x00, /* reserved */ 0x00, 0x00, 0x00, 0x00, /* interface number */ @@ -92,14 +93,6 @@ static const uint8_t USBD_MS_COMP_ID_FEATURE_DESC[] = { 0x00, 0x00, 0x00, 0x00, /* sub-compatible ID */ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* reserved */ - 0x00, 0x00, - 0x01, /* interface number */ - 0x01, /* reserved */ - 0x57, 0x49, 0x4E, 0x55, /* compatible ID ("WINUSB\0\0") */ - 0x53, 0x42, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, /* sub-compatible ID */ - 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, /* reserved */ 0x00, 0x00 }; @@ -169,6 +162,11 @@ USBD_StatusTypeDef USBD_GSUSB_CustomDeviceRequest(USBD_HandleTypeDef *pdev, USBD } break; } + + /* WARNING: Must STALL unrecognized vendor requests. + * Windows enumeration fails without this. */ + USBD_CtlError(pdev, req); + return USBD_OK; } return USBD_FAIL; @@ -176,32 +174,6 @@ USBD_StatusTypeDef USBD_GSUSB_CustomDeviceRequest(USBD_HandleTypeDef *pdev, USBD #ifdef ENABLE_GSUSB -static uint8_t gsusb_dfu_request(USBD_HandleTypeDef *pdev, USBD_SetupReqTypedef *req) -{ - USBD_GS_CAN_HandleTypeDef *hcan = ((USBD_Composite_HandleTypeDef *)pdev->pClassData)->hcan; - switch (req->bRequest) { - - case 0: // DETACH request - hcan->dfu_detach_requested = true; - break; - - case 3: // GET_STATIS request - hcan->ep0_buf[0] = 0x00; // bStatus: 0x00 == OK - hcan->ep0_buf[1] = 0x00; // bwPollTimeout - hcan->ep0_buf[2] = 0x00; - hcan->ep0_buf[3] = 0x00; - hcan->ep0_buf[4] = 0x00; // bState: appIDLE - hcan->ep0_buf[5] = 0xFF; // status string descriptor index - USBD_CtlSendData(pdev, hcan->ep0_buf, 6); - break; - - default: - USBD_CtlError(pdev, req); - - } - return USBD_OK; -} - static uint8_t gsusb_config_request(USBD_HandleTypeDef *pdev, USBD_SetupReqTypedef *req) { USBD_GS_CAN_HandleTypeDef *hcan; @@ -252,22 +224,6 @@ static uint8_t gsusb_config_request(USBD_HandleTypeDef *pdev, USBD_SetupReqTyped return ret; } -static uint8_t gsusb_vendor_request(USBD_HandleTypeDef *pdev, USBD_SetupReqTypedef *req) -{ - uint8_t req_rcpt = req->bRequest & 0x1F; - uint8_t req_type = (req->bRequest >> 5) & 0x03; - - if ( - (req_type == 0x01) // class request - && (req_rcpt == 0x01) // recipient: interface - && (req->wIndex == DFU_INTERFACE_NUM) - ) { - return gsusb_dfu_request(pdev, req); - } else { - return gsusb_config_request(pdev, req); - } -} - USBD_StatusTypeDef USBD_GSUSB_Init(USBD_HandleTypeDef *pdev) { USBD_StatusTypeDef ret; @@ -308,7 +264,7 @@ USBD_StatusTypeDef USBD_GSUSB_Setup(USBD_HandleTypeDef *pdev, USBD_SetupReqTyped { case USB_REQ_TYPE_CLASS: case USB_REQ_TYPE_VENDOR: - return gsusb_vendor_request(pdev, req); + return gsusb_config_request(pdev, req); case USB_REQ_TYPE_STANDARD: switch (req->bRequest) @@ -318,13 +274,20 @@ USBD_StatusTypeDef USBD_GSUSB_Setup(USBD_HandleTypeDef *pdev, USBD_SetupReqTyped break; case USB_REQ_SET_INTERFACE: + break; + default: + /* WARNING: Must STALL unknown standard requests + * for Windows enumeration on AMD xHCI. */ + USBD_CtlError(pdev, req); break; } break; default: - ret = USBD_FAIL; + /* WARNING: Must STALL unhandled request types + * for Windows enumeration on AMD xHCI. */ + USBD_CtlError(pdev, req); break; } diff --git a/firmware/RAMNV1/Middlewares/ST/STM32_USB_Device_Library/USBClass/Composite/gs_usb/usbd_gs_usb.h b/firmware/RAMNV1/Middlewares/ST/STM32_USB_Device_Library/USBClass/Composite/gs_usb/usbd_gs_usb.h index 6d1b1652..afff2521 100644 --- a/firmware/RAMNV1/Middlewares/ST/STM32_USB_Device_Library/USBClass/Composite/gs_usb/usbd_gs_usb.h +++ b/firmware/RAMNV1/Middlewares/ST/STM32_USB_Device_Library/USBClass/Composite/gs_usb/usbd_gs_usb.h @@ -145,8 +145,6 @@ THE SOFTWARE. #define USB_CAN_CONFIG_DESC_SIZ 50 #define NUM_CAN_CHANNEL 1 #define USBD_GS_CAN_VENDOR_CODE 0x20 -#define DFU_INTERFACE_NUM 1 -#define DFU_INTERFACE_STR_INDEX 0xE0 // queue send/recv timeout[msec] #define CAN_QUEUE_TIMEOUT 100 @@ -264,7 +262,6 @@ typedef struct { uint32_t out_requests; uint32_t out_requests_fail; uint32_t out_requests_no_buf; - uint8_t dfu_detach_requested; uint8_t timestamps_enabled; uint32_t sof_timestamp_us; uint8_t pad_pkts_to_max_pkt_size; diff --git a/firmware/RAMNV1/Middlewares/ST/STM32_USB_Device_Library/USBClass/Composite/usbd_composite.c b/firmware/RAMNV1/Middlewares/ST/STM32_USB_Device_Library/USBClass/Composite/usbd_composite.c index 70c37ee7..d4a0f4ba 100644 --- a/firmware/RAMNV1/Middlewares/ST/STM32_USB_Device_Library/USBClass/Composite/usbd_composite.c +++ b/firmware/RAMNV1/Middlewares/ST/STM32_USB_Device_Library/USBClass/Composite/usbd_composite.c @@ -51,7 +51,10 @@ void (*USBD_serialCloseCallback_ptr)(USBD_HandleTypeDef* hUsbDeviceFS, uint8_t i /*=============================================================================* * Static global variable * *==============================================================================*/ -/* USB Standard Device Descriptor */ +/* WARNING: GetDeviceQualifierDescriptor callback MUST NOT be NULL. + * STM32 USB library zero-initializes dev_speed to USBD_SPEED_HIGH (0). + * Before HAL_PCD_ResetCallback sets speed to FULL, a Device Qualifier + * request will call this callback; if NULL, the device hard-faults. */ __ALIGN_BEGIN static uint8_t USBD_Composite_DeviceQualifierDesc[USB_LEN_DEV_QUALIFIER_DESC] __ALIGN_END = { USB_LEN_DEV_QUALIFIER_DESC, @@ -76,13 +79,27 @@ __ALIGN_BEGIN static uint8_t USBD_Composite_CfgFSDesc[] __ALIGN_END = USB_DESC_TYPE_CONFIGURATION, // bDescriptorType: Configuration USB_COMPOSITE_CONFIG_DESC_SIZ, // wTotalLength:no of returned bytes 0x00, - USBD_MAX_NUM_INTERFACES, // bNumInterfaces: 2 or 4 + USBD_MAX_NUM_INTERFACES, // bNumInterfaces: 1 or 3 0x01, // bConfigurationValue: Configuration value 0x00, // iConfiguration: Index of string descriptor describing the configuration 0x80, // bmAttributes: Bus powered 0xFA, // MaxPower 500 mA #ifdef ENABLE_GSUSB + //--------------------------------------------------------------------------- + // IAD for GS_USB + // WARNING: IAD is required for composite device enumeration on AMD + // xHCI controllers. bInterfaceCount MUST be 1 (gs_usb only). + //--------------------------------------------------------------------------- + 0x08, // bLength: Interface Association Descriptor size + 0x0B, // bDescriptorType: IAD + GSUSB_WINDEX, // bFirstInterface: gs_usb interface number + 0x01, // bInterfaceCount + 0xFF, // bFunctionClass: Vendor Specific + 0xFF, // bFunctionSubClass: Vendor Specific + 0xFF, // bFunctionProtocol: Vendor Specific + 0x00, // iFunction + //--------------------------------------------------------------------------- // GS_USB Interface Descriptor //--------------------------------------------------------------------------- @@ -111,39 +128,19 @@ __ALIGN_BEGIN static uint8_t USBD_Composite_CfgFSDesc[] __ALIGN_END = //--------------------------------------------------------------------------- // EP2 descriptor + // WARNING: wMaxPacketSize MUST be <= 64 for Full Speed bulk endpoints + // per USB 2.0 ยง5.8.3. Larger values cause enumeration failure on + // AMD xHCI controllers. //--------------------------------------------------------------------------- 0x07, // bLength USB_DESC_TYPE_ENDPOINT, // bDescriptorType GSUSB_OUT_EP, // bEndpointAddress 0x02, // bmAttributes: bulk - LOBYTE(GSUSB_RX_DATA_SIZE), // wMaxPacketSize - HIBYTE(GSUSB_RX_DATA_SIZE), + LOBYTE(CAN_DATA_MAX_PACKET_SIZE), // wMaxPacketSize + HIBYTE(CAN_DATA_MAX_PACKET_SIZE), 0x00, // bInterval: //--------------------------------------------------------------------------- - //--------------------------------------------------------------------------- - // DFU Interface Descriptor - //--------------------------------------------------------------------------- - 0x09, // bLength - USB_DESC_TYPE_INTERFACE, // bDescriptorType - GSUSB_WINDEX + 1, // bInterfaceNumber - 0x00, // bAlternateSetting - 0x00, // bNumEndpoints - 0xFE, // bInterfaceClass: Vendor Specific - 0x01, // bInterfaceSubClass - 0x01, // bInterfaceProtocol : Runtime mode - DFU_INTERFACE_STR_INDEX, // iInterface - - //--------------------------------------------------------------------------- - // Run-Time DFU Functional Descriptor - //--------------------------------------------------------------------------- - 0x09, // bLength - 0x21, // bDescriptorType: DFU FUNCTIONAL - 0x0B, // bmAttributes: detach, upload, download - 0xFF, 0x00, // wDetachTimeOut - 0x00, 0x64, // wTransferSize - 0x1a, 0x01, // bcdDFUVersion: 1.1a - #endif #ifdef ENABLE_CDC @@ -411,7 +408,7 @@ static uint8_t USBD_Composite_Setup(USBD_HandleTypeDef *pdev, USBD_SetupReqTyped // ret = USBD_OK; // #endif default: - //Error_Handler(); + ret = USBD_FAIL; break; } @@ -632,9 +629,6 @@ static uint8_t *USBD_Composite_GetStrdesc(USBD_HandleTypeDef *pdev, uint8_t inde UNUSED(pdev); switch (index) { - case DFU_INTERFACE_STR_INDEX: - USBD_GetString(GSUSB_DFU_INTERFACE_STRING_FS, USBD_StrDesc, length); - return USBD_StrDesc; case 0xEE: *length = sizeof(usbd_gscan_winusb_str); return usbd_gscan_winusb_str; diff --git a/firmware/RAMNV1/USB_CompositeDevice/App/usbd_desc.c b/firmware/RAMNV1/USB_CompositeDevice/App/usbd_desc.c index 17661347..b7b2818c 100644 --- a/firmware/RAMNV1/USB_CompositeDevice/App/usbd_desc.c +++ b/firmware/RAMNV1/USB_CompositeDevice/App/usbd_desc.c @@ -150,9 +150,12 @@ __ALIGN_BEGIN uint8_t USBD_MDC_DeviceDesc[USB_LEN_DEV_DESC] __ALIGN_END = USB_DESC_TYPE_DEVICE, /*bDescriptorType*/ 0x00, /*bcdUSB */ 0x02, - 0xEF, /*bDeviceClass*/ - 0x02, /*bDeviceSubClass*/ - 0x01, /*bDeviceProtocol*/ + /* WARNING: Class 0xEF/0x02/0x01 (Misc/Common/IAD) is required for + * composite device enumeration. Without this, AMD xHCI controllers + * fail to split interfaces into separate child devices. */ + 0xEF, /*bDeviceClass: Miscellaneous */ + 0x02, /*bDeviceSubClass: Common Class */ + 0x01, /*bDeviceProtocol: IAD */ USB_MAX_EP0_SIZE, /*bMaxPacketSize*/ LOBYTE(USBD_VID), /*idVendor*/ HIBYTE(USBD_VID), /*idVendor*/ diff --git a/firmware/RAMNV1/USB_CompositeDevice/Target/usbd_conf.h b/firmware/RAMNV1/USB_CompositeDevice/Target/usbd_conf.h index deb5adc7..21d4df76 100644 --- a/firmware/RAMNV1/USB_CompositeDevice/Target/usbd_conf.h +++ b/firmware/RAMNV1/USB_CompositeDevice/Target/usbd_conf.h @@ -70,21 +70,21 @@ #if defined(ENABLE_CDC) && defined(ENABLE_GSUSB) #define GSUSB_WINDEX 0x00 /* gs_usb interface number */ -#define CDC_WINDEX 0x02 /* CDC interface number */ -#define USBD_MAX_NUM_INTERFACES 4U -#define USB_COMPOSITE_CONFIG_DESC_SIZ 116 +#define CDC_WINDEX 0x01 /* CDC interface number */ +#define USBD_MAX_NUM_INTERFACES 3U +#define USB_COMPOSITE_CONFIG_DESC_SIZ 106 /* 9 config + 8 GS_USB IAD + 32 GS_USB + 8 CDC IAD + 49 CDC */ #endif #if defined(ENABLE_CDC) && !defined(ENABLE_GSUSB) #define CDC_WINDEX 0x00 /* CDC interface number */ #define USBD_MAX_NUM_INTERFACES 2U -#define USB_COMPOSITE_CONFIG_DESC_SIZ 75 +#define USB_COMPOSITE_CONFIG_DESC_SIZ 75 /* 67 base + 8 for CDC IAD */ #endif #if !defined(ENABLE_CDC) && defined(ENABLE_GSUSB) #define GSUSB_WINDEX 0x00 /* gs_usb interface number */ -#define USBD_MAX_NUM_INTERFACES 2U -#define USB_COMPOSITE_CONFIG_DESC_SIZ 50 +#define USBD_MAX_NUM_INTERFACES 1U +#define USB_COMPOSITE_CONFIG_DESC_SIZ 40 /* 9 config + 8 GS_USB IAD + 9 iface + 7 EP1 + 7 EP2 */ #endif //TODO: investigate issue below