From ce2fc0470cbdbd64faa7b4460d3745d78722d49f Mon Sep 17 00:00:00 2001 From: hathach Date: Thu, 22 Mar 2018 14:15:16 +0700 Subject: [PATCH] improve usbd control transfer --- tinyusb/class/cdc/cdc_device.c | 15 +++-- tinyusb/class/msc/msc_device.c | 9 +-- tinyusb/device/usbd.c | 117 +++++++++++++++++---------------- tinyusb/device/usbd.h | 19 ------ tinyusb/device/usbd_pvt.h | 73 ++++++++++++++++++++ tinyusb/osal/osal.h | 6 +- tinyusb/osal/osal_none.h | 22 +++---- tinyusb/tusb.c | 1 + 8 files changed, 164 insertions(+), 98 deletions(-) create mode 100644 tinyusb/device/usbd_pvt.h diff --git a/tinyusb/class/cdc/cdc_device.c b/tinyusb/class/cdc/cdc_device.c index ebb3e8ead..914e8d036 100644 --- a/tinyusb/class/cdc/cdc_device.c +++ b/tinyusb/class/cdc/cdc_device.c @@ -44,8 +44,9 @@ //--------------------------------------------------------------------+ // INCLUDE //--------------------------------------------------------------------+ -#include +#include "common/tusb_common.h" #include "cdc_device.h" +#include "device/usbd_pvt.h" //--------------------------------------------------------------------+ // MACRO CONSTANT TYPEDEF @@ -221,13 +222,13 @@ tusb_error_t cdcd_control_request_subtask(uint8_t port, tusb_control_request_t c if (CDC_REQUEST_GET_LINE_CODING == p_request->bRequest) { - SUBTASK_INVOKE( usbd_control_xfer_substak(port, (tusb_dir_t) p_request->bmRequestType_bit.direction, - (uint8_t*) &cdcd_line_coding[port], min16_of(sizeof(cdc_line_coding_t), p_request->wLength)), err ); + SUBTASK_INVOKE( usbd_control_xfer_stask(port, (tusb_dir_t) p_request->bmRequestType_bit.direction, + (uint8_t*) &cdcd_line_coding[port], min16_of(sizeof(cdc_line_coding_t), p_request->wLength)), err ); } else if (CDC_REQUEST_SET_LINE_CODING == p_request->bRequest) { - SUBTASK_INVOKE( usbd_control_xfer_substak(port, (tusb_dir_t) p_request->bmRequestType_bit.direction, - (uint8_t*) &cdcd_line_coding[port], min16_of(sizeof(cdc_line_coding_t), p_request->wLength)), err ); + SUBTASK_INVOKE( usbd_control_xfer_stask(port, (tusb_dir_t) p_request->bmRequestType_bit.direction, + (uint8_t*) &cdcd_line_coding[port], min16_of(sizeof(cdc_line_coding_t), p_request->wLength)), err ); // TODO notify application on xfer complete } else if (CDC_REQUEST_SET_CONTROL_LINE_STATE == p_request->bRequest ) @@ -253,10 +254,12 @@ tusb_error_t cdcd_control_request_subtask(uint8_t port, tusb_control_request_t c // De-active --> disconnected p_cdc->connected = false; } + + usbd_control_status(port, p_request->bmRequestType_bit.direction); } else { - SUBTASK_RETURN(TUSB_ERROR_DCD_CONTROL_REQUEST_NOT_SUPPORT); + usbd_control_stall(port); // stall unsupported request } OSAL_SUBTASK_END diff --git a/tinyusb/class/msc/msc_device.c b/tinyusb/class/msc/msc_device.c index 3244e3c86..4968faa1c 100644 --- a/tinyusb/class/msc/msc_device.c +++ b/tinyusb/class/msc/msc_device.c @@ -44,8 +44,9 @@ //--------------------------------------------------------------------+ // INCLUDE //--------------------------------------------------------------------+ -#include +#include "common/tusb_common.h" #include "msc_device.h" +#include "device/usbd_pvt.h" //--------------------------------------------------------------------+ // MACRO CONSTANT TYPEDEF @@ -133,16 +134,16 @@ tusb_error_t mscd_control_request_subtask(uint8_t port, tusb_control_request_t c if(MSC_REQUEST_RESET == p_request->bRequest) { - usbd_control_status(port, TUSB_DIR_IN); + usbd_control_status(port, p_request->bmRequestType_bit.direction); } else if (MSC_REQUEST_GET_MAX_LUN == p_request->bRequest) { // Note: lpc11/13u need xfer data's address to be aligned 64 -> make use of scsi_data instead of using max_lun directly p_msc->scsi_data[0] = p_msc->max_lun; - SUBTASK_INVOKE( usbd_control_xfer_substak(port, TUSB_DIR_IN, p_msc->scsi_data, 1), err); + SUBTASK_INVOKE( usbd_control_xfer_stask(port, p_request->bmRequestType_bit.direction, p_msc->scsi_data, 1), err); }else { - SUBTASK_RETURN(TUSB_ERROR_DCD_CONTROL_REQUEST_NOT_SUPPORT); + usbd_control_stall(port); // stall unsupported request } OSAL_SUBTASK_END diff --git a/tinyusb/device/usbd.c b/tinyusb/device/usbd.c index 6f61b7bdc..b88dbf3f9 100644 --- a/tinyusb/device/usbd.c +++ b/tinyusb/device/usbd.c @@ -47,6 +47,7 @@ //--------------------------------------------------------------------+ #include "tusb.h" #include "usbd.h" +#include "device/usbd_pvt.h" //--------------------------------------------------------------------+ // MACRO CONSTANT TYPEDEF @@ -112,8 +113,8 @@ enum { USBD_CLASS_DRIVER_COUNT = sizeof(usbd_class_drivers) / sizeof(usbd_class_ //--------------------------------------------------------------------+ // INTERNAL OBJECT & FUNCTION DECLARATION //--------------------------------------------------------------------+ -static tusb_error_t usbd_set_configure_received(uint8_t port, uint8_t config_number); -static tusb_error_t get_descriptor(uint8_t port, tusb_control_request_t const * const p_request, uint8_t const ** pp_buffer, uint16_t * p_length); +static tusb_error_t proc_set_config_req(uint8_t port, uint8_t config_number); +static uint16_t get_descriptor(uint8_t port, tusb_control_request_t const * const p_request, uint8_t const ** pp_buffer); //--------------------------------------------------------------------+ // APPLICATION INTERFACE @@ -171,8 +172,8 @@ static osal_queue_t usbd_queue_hdl; //--------------------------------------------------------------------+ // IMPLEMENTATION //--------------------------------------------------------------------+ -tusb_error_t usbd_control_request_subtask(uint8_t port, tusb_control_request_t const * const p_request); -static tusb_error_t usbd_body_subtask(void); +static tusb_error_t proc_control_request_stask(uint8_t port, tusb_control_request_t const * const p_request); +static tusb_error_t usbd_main_stask(void); tusb_error_t usbd_init (void) { @@ -216,11 +217,11 @@ void usbd_task( void* param) (void) param; OSAL_TASK_BEGIN - usbd_body_subtask(); + usbd_main_stask(); OSAL_TASK_END } -static tusb_error_t usbd_body_subtask(void) +static tusb_error_t usbd_main_stask(void) { static usbd_task_event_t event; @@ -254,7 +255,7 @@ static tusb_error_t usbd_body_subtask(void) if ( USBD_EVENTID_SETUP_RECEIVED == event.event_id ) { - SUBTASK_INVOKE( usbd_control_request_subtask(event.port, &event.setup_received), error ); + SUBTASK_INVOKE( proc_control_request_stask(event.port, &event.setup_received), error ); }else if (USBD_EVENTID_XFER_DONE == event.event_id) { // Call class handling function, Class that endpoint not belong to should check and return @@ -286,7 +287,7 @@ static tusb_error_t usbd_body_subtask(void) //--------------------------------------------------------------------+ // CONTROL REQUEST //--------------------------------------------------------------------+ -tusb_error_t usbd_control_xfer_substak(uint8_t port, tusb_dir_t dir, uint8_t * buffer, uint16_t length) +tusb_error_t usbd_control_xfer_stask(uint8_t port, tusb_dir_t dir, uint8_t * buffer, uint16_t length) { OSAL_SUBTASK_BEGIN @@ -302,34 +303,35 @@ tusb_error_t usbd_control_xfer_substak(uint8_t port, tusb_dir_t dir, uint8_t * b } // Status opposite direction with Zero Length - usbd_control_status(port, 1-dir); - - // no need to blocking wait for status to complete + // No need to wait for status to complete therefore + // status phase must not call tusb_dcd_control_complete/tusb_dcd_xfer_complete + usbd_control_status(port, dir); OSAL_SUBTASK_END } -tusb_error_t usbd_control_request_subtask(uint8_t port, tusb_control_request_t const * const p_request) +static tusb_error_t proc_control_request_stask(uint8_t port, tusb_control_request_t const * const p_request) { OSAL_SUBTASK_BEGIN tusb_error_t error; error = TUSB_ERROR_NONE; - //------------- Standard Control e.g in enumeration -------------// + //------------- Standard Request e.g in enumeration -------------// if( TUSB_REQ_RCPT_DEVICE == p_request->bmRequestType_bit.recipient && TUSB_REQ_TYPE_STANDARD == p_request->bmRequestType_bit.type ) { if ( TUSB_REQ_GET_DESCRIPTOR == p_request->bRequest ) { - uint8_t const * p_buffer = NULL; - uint16_t length = 0; + uint8_t const * buffer = NULL; + uint16_t const len = get_descriptor(port, p_request, &buffer); - error = get_descriptor(port, p_request, &p_buffer, &length); - - if ( TUSB_ERROR_NONE == error ) + if ( len ) { - SUBTASK_INVOKE ( usbd_control_xfer_substak(port, (tusb_dir_t) p_request->bmRequestType_bit.direction, (uint8_t*) p_buffer, length ), error ); + SUBTASK_INVOKE( usbd_control_xfer_stask(port, p_request->bmRequestType_bit.direction, (uint8_t*) buffer, len ), error ); + }else + { + usbd_control_stall(port); // stall unsupported descriptor } } else if ( TUSB_REQ_SET_ADDRESS == p_request->bRequest ) @@ -337,19 +339,21 @@ tusb_error_t usbd_control_request_subtask(uint8_t port, tusb_control_request_t c tusb_dcd_set_address(port, (uint8_t) p_request->wValue); usbd_devices[port].state = TUSB_DEVICE_STATE_ADDRESSED; - #ifdef NRF52840_XXAA - // nrf52 auto handle set address, no need to return status - SUBTASK_RETURN(TUSB_ERROR_NONE); + #ifndef NRF52840_XXAA // nrf52 auto handle set address, we must not return status + usbd_control_status(port, p_request->bmRequestType_bit.direction); #endif } else if ( TUSB_REQ_SET_CONFIGURATION == p_request->bRequest ) { - usbd_set_configure_received(port, (uint8_t) p_request->wValue); - }else + proc_set_config_req(port, (uint8_t) p_request->wValue); + usbd_control_status(port, p_request->bmRequestType_bit.direction); + } + else { - error = TUSB_ERROR_DCD_CONTROL_REQUEST_NOT_SUPPORT; + usbd_control_stall(port); // Stall unsupported request } } + //------------- Class/Interface Specific Request -------------// else if ( TUSB_REQ_RCPT_INTERFACE == p_request->bmRequestType_bit.recipient) { @@ -364,28 +368,28 @@ tusb_error_t usbd_control_request_subtask(uint8_t port, tusb_control_request_t c SUBTASK_INVOKE( usbd_class_drivers[class_code].control_request_subtask(port, p_request), error ); }else { - error = TUSB_ERROR_DCD_CONTROL_REQUEST_NOT_SUPPORT; + usbd_control_stall(port); // Stall unsupported request } } //------------- Endpoint Request -------------// else if ( TUSB_REQ_RCPT_ENDPOINT == p_request->bmRequestType_bit.recipient && - TUSB_REQ_TYPE_STANDARD == p_request->bmRequestType_bit.type && - TUSB_REQ_CLEAR_FEATURE == p_request->bRequest ) + TUSB_REQ_TYPE_STANDARD == p_request->bmRequestType_bit.type) { - tusb_dcd_edpt_clear_stall(port, u16_low_u8(p_request->wIndex) ); - } else - { - error = TUSB_ERROR_DCD_CONTROL_REQUEST_NOT_SUPPORT; + if (TUSB_REQ_CLEAR_FEATURE == p_request->bRequest ) + { + tusb_dcd_edpt_clear_stall(port, u16_low_u8(p_request->wIndex) ); + usbd_control_status(port, p_request->bmRequestType_bit.direction); + } else + { + usbd_control_stall(port); // Stall unsupported request + } } - if(TUSB_ERROR_NONE != error) + //------------- Unsupported Request -------------// + else { - // Response with Protocol Stall if request is not supported - tusb_dcd_edpt_stall(port, 0); - }else if (p_request->wLength == 0) - { - usbd_control_status(port, 1-p_request->bmRequestType_bit.direction); + usbd_control_stall(port); // Stall unsupported request } OSAL_SUBTASK_END @@ -393,7 +397,7 @@ tusb_error_t usbd_control_request_subtask(uint8_t port, tusb_control_request_t c // TODO Host (windows) can get HID report descriptor before set configured // may need to open interface before set configured -static tusb_error_t usbd_set_configure_received(uint8_t port, uint8_t config_number) +static tusb_error_t proc_set_config_req(uint8_t port, uint8_t config_number) { tusb_dcd_set_config(port, config_number); usbd_devices[port].state = TUSB_DEVICE_STATE_CONFIGURED; @@ -437,53 +441,56 @@ static tusb_error_t usbd_set_configure_received(uint8_t port, uint8_t config_num return TUSB_ERROR_NONE; } -static tusb_error_t get_descriptor(uint8_t port, tusb_control_request_t const * const p_request, uint8_t const ** pp_buffer, uint16_t * p_length) +static uint16_t get_descriptor(uint8_t port, tusb_control_request_t const * const p_request, uint8_t const ** pp_buffer) { tusb_desc_type_t const desc_type = (tusb_desc_type_t) u16_high_u8(p_request->wValue); uint8_t const desc_index = u16_low_u8( p_request->wValue ); - uint8_t const * p_data = NULL ; + uint8_t const * desc_data = NULL ; + uint16_t len = 0; switch(desc_type) { case TUSB_DESC_DEVICE: - p_data = tusbd_descriptor_pointers.p_device; - (*p_length) = sizeof(tusb_descriptor_device_t); + desc_data = tusbd_descriptor_pointers.p_device; + len = sizeof(tusb_descriptor_device_t); break; case TUSB_DESC_CONFIGURATION: - p_data = tusbd_descriptor_pointers.p_configuration; - (*p_length) = ((tusb_descriptor_configuration_t*)tusbd_descriptor_pointers.p_configuration)->wTotalLength; + desc_data = tusbd_descriptor_pointers.p_configuration; + len = ((tusb_descriptor_configuration_t*)tusbd_descriptor_pointers.p_configuration)->wTotalLength; break; case TUSB_DESC_STRING: - if ( !(desc_index < 100) ) return TUSB_ERROR_DCD_CONTROL_REQUEST_NOT_SUPPORT; // windows sometimes ask for string at index 238 !!! + // windows sometimes ask for string at index 238 !!! + if ( !(desc_index < 100) ) return 0; - p_data = tusbd_descriptor_pointers.p_string_arr[desc_index]; - ASSERT( p_data != NULL, TUSB_ERROR_FAILED); + desc_data = tusbd_descriptor_pointers.p_string_arr[desc_index]; + VERIFY( desc_data != NULL, 0 ); - (*p_length) = p_data[0]; // first byte of descriptor is its size + len = desc_data[0]; // first byte of descriptor is its size break; case TUSB_DESC_DEVICE_QUALIFIER: // TODO If not highspeed capable stall this request otherwise // return the descriptor that could work in highspeed - return TUSB_ERROR_DCD_CONTROL_REQUEST_NOT_SUPPORT; + return 0; break; // TODO Report Descriptor (HID Generic) // TODO HID Descriptor - default: return TUSB_ERROR_DCD_CONTROL_REQUEST_NOT_SUPPORT; + default: return 0; } - (*p_length) = min16_of(p_request->wLength, (*p_length) ); // cannot return more than hosts requires - ASSERT( (*p_length) <= TUSB_CFG_DEVICE_ENUM_BUFFER_SIZE, TUSB_ERROR_NOT_ENOUGH_MEMORY); + // up to Host's length + len = min16_of(p_request->wLength, len ); + ASSERT( len <= TUSB_CFG_DEVICE_ENUM_BUFFER_SIZE, TUSB_ERROR_NOT_ENOUGH_MEMORY); - memcpy(usbd_enum_buffer, p_data, (*p_length)); + memcpy(usbd_enum_buffer, desc_data, len); (*pp_buffer) = usbd_enum_buffer; - return TUSB_ERROR_NONE; + return len; } //--------------------------------------------------------------------+ // USBD-CLASS API diff --git a/tinyusb/device/usbd.h b/tinyusb/device/usbd.h index f86a3b0a8..6ced5d6be 100644 --- a/tinyusb/device/usbd.h +++ b/tinyusb/device/usbd.h @@ -123,25 +123,6 @@ void tud_umount_cb(uint8_t port); //void tud_device_suspended_cb(uint8_t port); -//--------------------------------------------------------------------+ -// CLASS-USBD & INTERNAL API -//--------------------------------------------------------------------+ -#ifdef _TINY_USB_SOURCE_FILE_ - -extern osal_semaphore_t usbd_control_xfer_sem_hdl; - -tusb_error_t usbd_control_xfer_substak(uint8_t port, tusb_dir_t dir, uint8_t * buffer, uint16_t length); - -static inline bool usbd_control_status(uint8_t port, tusb_dir_t dir) -{ - return tusb_dcd_control_xfer(port, dir, NULL, 0); -} - -tusb_error_t usbd_init(void); -void usbd_task( void* param); - -#endif - #ifdef __cplusplus } #endif diff --git a/tinyusb/device/usbd_pvt.h b/tinyusb/device/usbd_pvt.h new file mode 100644 index 000000000..067052e5f --- /dev/null +++ b/tinyusb/device/usbd_pvt.h @@ -0,0 +1,73 @@ +/**************************************************************************/ +/*! + @file usbd_pvt.h + @author hathach + + @section LICENSE + + Software License Agreement (BSD License) + + Copyright (c) 2018, hathach (tinyusb.org) + All rights reserved. + + Redistribution and use in source and binary forms, with or without + modification, are permitted provided that the following conditions are met: + 1. Redistributions of source code must retain the above copyright + notice, this list of conditions and the following disclaimer. + 2. Redistributions in binary form must reproduce the above copyright + notice, this list of conditions and the following disclaimer in the + documentation and/or other materials provided with the distribution. + 3. Neither the name of the copyright holders nor the + names of its contributors may be used to endorse or promote products + derived from this software without specific prior written permission. + + THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS ''AS IS'' AND ANY + EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED + WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE + DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER BE LIABLE FOR ANY + DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES + (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; + LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND + ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS + SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. +*/ +/**************************************************************************/ +#ifndef USBD_PVT_H_ +#define USBD_PVT_H_ + + +#ifdef __cplusplus + extern "C" { +#endif + +//--------------------------------------------------------------------+ +// INTERNAL API +//--------------------------------------------------------------------+ +extern osal_semaphore_t usbd_control_xfer_sem_hdl; + +tusb_error_t usbd_init(void); +void usbd_task( void* param); + +// Carry out Data and Status stage of control transfer +tusb_error_t usbd_control_xfer_stask(uint8_t port, tusb_dir_t dir, uint8_t * buffer, uint16_t length); + +// Return Status of control transfer +// Note dir is value of direction bit in setup packet (aka DATA stage direction) +static inline bool usbd_control_status(uint8_t port, tusb_dir_t dir) +{ + // status direction is reversed to one in the setup packet + return tusb_dcd_control_xfer(port, 1-dir, NULL, 0); +} + +static inline void usbd_control_stall(uint8_t port) +{ + tusb_dcd_edpt_stall(port, 0); +} + + +#ifdef __cplusplus + } +#endif + +#endif /* USBD_PVT_H_ */ diff --git a/tinyusb/osal/osal.h b/tinyusb/osal/osal.h index 4faca3d5c..e4827bf37 100644 --- a/tinyusb/osal/osal.h +++ b/tinyusb/osal/osal.h @@ -72,10 +72,10 @@ //------------- Sub Task -------------// #define OSAL_SUBTASK_BEGIN - #define OSAL_SUBTASK_END return TUSB_ERROR_NONE; + #define OSAL_SUBTASK_END return TUSB_ERROR_NONE; - #define SUBTASK_RETURN(error) return error; - #define SUBTASK_INVOKE(subtask, status) status = subtask + #define SUBTASK_RETURN(_error) return _error; + #define SUBTASK_INVOKE(_subtask, _status) (_status) = _subtask //------------- Sub Task Assert -------------// #define SUBTASK_ASSERT_STATUS(sts) VERIFY_STATUS(sts) diff --git a/tinyusb/osal/osal_none.h b/tinyusb/osal/osal_none.h index f2f3dcefc..9bf7367ec 100644 --- a/tinyusb/osal/osal_none.h +++ b/tinyusb/osal/osal_none.h @@ -107,19 +107,19 @@ static inline osal_task_t osal_task_create(osal_func_t code, const char* name, u // SUBTASK (a sub function that uses OS blocking services & called by a task //--------------------------------------------------------------------+ #define OSAL_SUBTASK_BEGIN OSAL_TASK_BEGIN -#define OSAL_SUBTASK_END \ - default: TASK_RESTART; break; \ - }}\ + +#define OSAL_SUBTASK_END \ + default: TASK_RESTART; break; \ + }} \ return TUSB_ERROR_NONE; -#define SUBTASK_INVOKE(_subtask, _status) \ - do {\ - _state = __LINE__; case __LINE__:\ - {\ - _status = _subtask; /* invoke sub task */\ - if (TUSB_ERROR_OSAL_WAITING == _status) /* sub task not finished -> continue waiting */\ - return TUSB_ERROR_OSAL_WAITING;\ - }\ +#define SUBTASK_INVOKE(_subtask, _status) \ + do { \ + _state = __LINE__; case __LINE__: \ + { \ + (_status) = _subtask; /* invoke sub task */ \ + if (TUSB_ERROR_OSAL_WAITING == (_status)) return TUSB_ERROR_OSAL_WAITING; \ + } \ }while(0) //------------- Sub Task Assert -------------// diff --git a/tinyusb/tusb.c b/tinyusb/tusb.c index 29f1ade95..cb944a761 100644 --- a/tinyusb/tusb.c +++ b/tinyusb/tusb.c @@ -39,6 +39,7 @@ #define _TINY_USB_SOURCE_FILE_ #include "tusb.h" +#include "device/usbd_pvt.h" tusb_error_t tusb_init(void) {