From ac8c343fef785c9aa19ba628f746e72774b9e8c8 Mon Sep 17 00:00:00 2001 From: hathach Date: Fri, 13 Sep 2019 22:16:24 +0700 Subject: fix #154 forward all endpoint recipeint request to class driver fix typo remove magic number 0xff of driver id --- src/common/tusb_common.h | 2 +- src/device/usbd.c | 107 +++++++++++++++++++++++++++++++---------------- 2 files changed, 71 insertions(+), 38 deletions(-) (limited to 'src') diff --git a/src/common/tusb_common.h b/src/common/tusb_common.h index 4f0105d13..57c6e2fcf 100644 --- a/src/common/tusb_common.h +++ b/src/common/tusb_common.h @@ -38,7 +38,7 @@ //--------------------------------------------------------------------+ // Macros Helper //--------------------------------------------------------------------+ -#define TU_ARRAY_SZIE(_arr) ( sizeof(_arr) / sizeof(_arr[0]) ) +#define TU_ARRAY_SIZE(_arr) ( sizeof(_arr) / sizeof(_arr[0]) ) #define TU_MIN(_x, _y) ( (_x) < (_y) ) ? (_x) : (_y) ) #define TU_MAX(_x, _y) ( (_x) > (_y) ) ? (_x) : (_y) ) diff --git a/src/device/usbd.c b/src/device/usbd.c index 184702f1a..c92884334 100644 --- a/src/device/usbd.c +++ b/src/device/usbd.c @@ -61,6 +61,9 @@ typedef struct { static usbd_device_t _usbd_dev = { 0 }; +// Invalid driver ID in itf2drv[] ep2drv[][] mapping +enum { DRVID_INVALID = 0xff }; + //--------------------------------------------------------------------+ // Class Driver //--------------------------------------------------------------------+ @@ -144,7 +147,7 @@ static usbd_class_driver_t const usbd_class_drivers[] = #endif }; -enum { USBD_CLASS_DRIVER_COUNT = TU_ARRAY_SZIE(usbd_class_drivers) }; +enum { USBD_CLASS_DRIVER_COUNT = TU_ARRAY_SIZE(usbd_class_drivers) }; //--------------------------------------------------------------------+ // DCD Event @@ -213,8 +216,8 @@ static void usbd_reset(uint8_t rhport) { tu_varclr(&_usbd_dev); - memset(_usbd_dev.itf2drv, 0xff, sizeof(_usbd_dev.itf2drv)); // invalid mapping - memset(_usbd_dev.ep2drv , 0xff, sizeof(_usbd_dev.ep2drv )); // invalid mapping + memset(_usbd_dev.itf2drv, DRVID_INVALID, sizeof(_usbd_dev.itf2drv)); // invalid mapping + memset(_usbd_dev.ep2drv , DRVID_INVALID, sizeof(_usbd_dev.ep2drv )); // invalid mapping usbd_control_reset(rhport); @@ -287,19 +290,19 @@ void tud_task (void) { // Invoke the class callback associated with the endpoint address uint8_t const ep_addr = event.xfer_complete.ep_addr; - uint8_t const epnum = tu_edpt_number(ep_addr); - uint8_t const dir = tu_edpt_dir(ep_addr); + uint8_t const epnum = tu_edpt_number(ep_addr); + uint8_t const ep_dir = tu_edpt_dir(ep_addr); - _usbd_dev.ep_busy_map[dir] = (uint8_t) tu_bit_clear(_usbd_dev.ep_busy_map[dir], epnum); + _usbd_dev.ep_busy_map[ep_dir] = (uint8_t) tu_bit_clear(_usbd_dev.ep_busy_map[ep_dir], epnum); - if ( 0 == tu_edpt_number(ep_addr) ) + if ( 0 == epnum ) { // control transfer DATA stage callback usbd_control_xfer_cb(event.rhport, ep_addr, event.xfer_complete.result, event.xfer_complete.len); } else { - uint8_t const drv_id = _usbd_dev.ep2drv[tu_edpt_number(ep_addr)][tu_edpt_dir(ep_addr)]; + uint8_t const drv_id = _usbd_dev.ep2drv[epnum][ep_dir]; TU_ASSERT(drv_id < USBD_CLASS_DRIVER_COUNT,); usbd_class_drivers[drv_id].xfer_cb(event.rhport, ep_addr, event.xfer_complete.result, event.xfer_complete.len); @@ -435,7 +438,7 @@ static bool process_control_request(uint8_t rhport, tusb_control_request_t const case TUSB_REQ_RCPT_INTERFACE: { uint8_t const itf = tu_u16_low(p_request->wIndex); - TU_VERIFY(itf < TU_ARRAY_SZIE(_usbd_dev.itf2drv)); + TU_VERIFY(itf < TU_ARRAY_SIZE(_usbd_dev.itf2drv)); uint8_t const drvid = _usbd_dev.itf2drv[itf]; TU_VERIFY(drvid < USBD_CLASS_DRIVER_COUNT); @@ -465,18 +468,16 @@ 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 - usbd_control_set_complete_callback(usbd_class_drivers[drvid].control_complete); - // stall control endpoint if driver return false + usbd_control_set_complete_callback(usbd_class_drivers[drvid].control_complete); TU_ASSERT(usbd_class_drivers[drvid].control_request(rhport, p_request)); break; } }else { // forward to class driver: "non-STD request to Interface" - usbd_control_set_complete_callback(usbd_class_drivers[drvid].control_complete); - // stall control endpoint if driver return false + usbd_control_set_complete_callback(usbd_class_drivers[drvid].control_complete); TU_ASSERT(usbd_class_drivers[drvid].control_request(rhport, p_request)); } } @@ -484,37 +485,69 @@ static bool process_control_request(uint8_t rhport, tusb_control_request_t const //------------- Endpoint Request -------------// case TUSB_REQ_RCPT_ENDPOINT: - // Non standard request is not supported - TU_VERIFY( TUSB_REQ_TYPE_STANDARD == p_request->bmRequestType_bit.type ); + { + uint8_t const ep_addr = tu_u16_low(p_request->wIndex); + uint8_t const ep_num = tu_edpt_number(ep_addr); + uint8_t const ep_dir = tu_edpt_dir(ep_addr); - switch ( p_request->bRequest ) + 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); + + // 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 non-standard request: driver can ACK or Stall the request by return true/false + // - For standard request: usbd decide the ACK stage regardless of driver return value + bool ret; + + if ( TUSB_REQ_TYPE_STANDARD != p_request->bmRequestType_bit.type ) { - case TUSB_REQ_GET_STATUS: - { - uint16_t status = usbd_edpt_stalled(rhport, tu_u16_low(p_request->wIndex)) ? 0x0001 : 0x0000; - tud_control_xfer(rhport, p_request, &status, 2); - } - break; + // complete callback is also capable of stalling/acking the request + usbd_control_set_complete_callback(usbd_class_drivers[drv_id].control_complete); + } - case TUSB_REQ_CLEAR_FEATURE: - if ( TUSB_REQ_FEATURE_EDPT_HALT == p_request->wValue ) - { - usbd_edpt_clear_stall(rhport, tu_u16_low(p_request->wIndex)); - } - tud_control_status(rhport, p_request); - break; + // Invoke class driver first + ret = usbd_class_drivers[drv_id].control_request(rhport, p_request); - case TUSB_REQ_SET_FEATURE: - if ( TUSB_REQ_FEATURE_EDPT_HALT == p_request->wValue ) + // Then handle if it is standard request + if ( TUSB_REQ_TYPE_STANDARD == p_request->bmRequestType_bit.type ) + { + // force return true for standard request + ret = true; + + switch ( p_request->bRequest ) + { + case TUSB_REQ_GET_STATUS: { - usbd_edpt_stall(rhport, tu_u16_low(p_request->wIndex)); + uint16_t status = usbd_edpt_stalled(rhport, ep_addr) ? 0x0001 : 0x0000; + tud_control_xfer(rhport, p_request, &status, 2); } - tud_control_status(rhport, p_request); - break; + break; - // Unknown/Unsupported request - default: TU_BREAKPOINT(); return false; + case TUSB_REQ_CLEAR_FEATURE: + if ( TUSB_REQ_FEATURE_EDPT_HALT == p_request->wValue ) + { + usbd_edpt_clear_stall(rhport, ep_addr); + } + tud_control_status(rhport, p_request); + break; + + case TUSB_REQ_SET_FEATURE: + if ( TUSB_REQ_FEATURE_EDPT_HALT == p_request->wValue ) + { + usbd_edpt_stall(rhport, ep_addr); + } + tud_control_status(rhport, p_request); + break; + + // Unknown/Unsupported request + default: TU_BREAKPOINT(); return false; + } } + + return ret; + } break; // Unknown recipient @@ -560,7 +593,7 @@ static bool process_set_config(uint8_t rhport, uint8_t cfg_num) TU_ASSERT( drv_id < USBD_CLASS_DRIVER_COUNT ); // Interface number must not be used already TODO alternate interface - TU_ASSERT( 0xff == _usbd_dev.itf2drv[desc_itf->bInterfaceNumber] ); + TU_ASSERT( DRVID_INVALID == _usbd_dev.itf2drv[desc_itf->bInterfaceNumber] ); _usbd_dev.itf2drv[desc_itf->bInterfaceNumber] = drv_id; uint16_t itf_len=0; -- cgit v1.3.1 From c831311a12a42d27690c830cc9cefd4d4e6efe2d Mon Sep 17 00:00:00 2001 From: Nathan Conrad Date: Fri, 13 Sep 2019 23:37:47 -0400 Subject: Off-by-one error in Synopsys assertions. (and move some braces to their own lines. --- src/portable/st/synopsys/dcd_synopsys.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) (limited to 'src') diff --git a/src/portable/st/synopsys/dcd_synopsys.c b/src/portable/st/synopsys/dcd_synopsys.c index b0e7e04bd..2daa16a26 100644 --- a/src/portable/st/synopsys/dcd_synopsys.c +++ b/src/portable/st/synopsys/dcd_synopsys.c @@ -268,21 +268,22 @@ bool dcd_edpt_open (uint8_t rhport, tusb_desc_endpoint_t const * desc_edpt) uint8_t const epnum = tu_edpt_number(desc_edpt->bEndpointAddress); uint8_t const dir = tu_edpt_dir(desc_edpt->bEndpointAddress); - - // Unsupported endpoint numbers/size. - if((desc_edpt->wMaxPacketSize.size > 64) || (epnum > EP_MAX)) { - return false; - } - + + TU_ASSERT((desc_edpt->wMaxPacketSize.size <= 64); + TU_ASSERT(epnum < EP_MAX); + xfer_ctl_t * xfer = XFER_CTL_BASE(epnum, dir); xfer->max_size = desc_edpt->wMaxPacketSize.size; - if(dir == TUSB_DIR_OUT) { + if(dir == TUSB_DIR_OUT) + { out_ep[epnum].DOEPCTL |= (1 << USB_OTG_DOEPCTL_USBAEP_Pos) | \ desc_edpt->bmAttributes.xfer << USB_OTG_DOEPCTL_EPTYP_Pos | \ desc_edpt->wMaxPacketSize.size << USB_OTG_DOEPCTL_MPSIZ_Pos; dev->DAINTMSK |= (1 << (USB_OTG_DAINTMSK_OEPM_Pos + epnum)); - } else { + } + else + { // "USB Data FIFOs" section in reference manual // Peripheral FIFO architecture // -- cgit v1.3.1 From 4ea212a432c32dedfefa79d995d2190462a27a5f Mon Sep 17 00:00:00 2001 From: Nathan Conrad Date: Fri, 13 Sep 2019 23:52:43 -0400 Subject: This demonstrates that I don't pre-build things before committing them... oops. --- src/portable/st/synopsys/dcd_synopsys.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src') diff --git a/src/portable/st/synopsys/dcd_synopsys.c b/src/portable/st/synopsys/dcd_synopsys.c index 2daa16a26..f15b92edc 100644 --- a/src/portable/st/synopsys/dcd_synopsys.c +++ b/src/portable/st/synopsys/dcd_synopsys.c @@ -269,7 +269,7 @@ bool dcd_edpt_open (uint8_t rhport, tusb_desc_endpoint_t const * desc_edpt) uint8_t const epnum = tu_edpt_number(desc_edpt->bEndpointAddress); uint8_t const dir = tu_edpt_dir(desc_edpt->bEndpointAddress); - TU_ASSERT((desc_edpt->wMaxPacketSize.size <= 64); + TU_ASSERT(desc_edpt->wMaxPacketSize.size <= 64); TU_ASSERT(epnum < EP_MAX); xfer_ctl_t * xfer = XFER_CTL_BASE(epnum, dir); -- cgit v1.3.1 From 21ba37aec64aa8e436062aaa43be898ece993907 Mon Sep 17 00:00:00 2001 From: Nathan Conrad Date: Sat, 14 Sep 2019 09:02:21 -0400 Subject: Interpretation of assert vs verify. --- src/common/tusb_verify.h | 30 +++++++++++++++++++++++++++--- 1 file changed, 27 insertions(+), 3 deletions(-) (limited to 'src') diff --git a/src/common/tusb_verify.h b/src/common/tusb_verify.h index 2727ce043..fae0c88ae 100644 --- a/src/common/tusb_verify.h +++ b/src/common/tusb_verify.h @@ -36,10 +36,34 @@ * as C++ for the sake of code simplicity. Beware of a headache macro * manipulation that you are told to stay away. * - * e.g * - * - TU_VERIFY( cond ) will return false if cond is false - * - TU_VERIFY( cond, err) will return err instead if cond is false + * This contains macros for both VERIFY and ASSERT: + * + * VERIFY: Used when there is an error condition which is not the + * fault of the MCU. For example, bounds checking on data + * sent to the micro over USB should use this function. + * Another example is checking for buffer overflows, where + * returning from the active function causes a NAK. + * + * ASSERT: Used for error conditions that are caused by MCU firmware + * bugs. This is used to discover bugs in the code more + * quickly. One example would be adding assertions in library + * function calls to confirm a function's (untainted) + * parameters are valid. + * + * + * The difference in behaviour is that ASSERT triggers a breakpoint while + * verify does not. + * + * #define TU_VERIFY(cond) if(cond) return false; + * #define TU_VERIFY(cond,ret) if(cond) return ret; + * + * #define TU_VERIFY_HDLR(cond,handler) if(cond) {handler; return false;} + * #define TU_VERIFY_HDLR(cond,ret,handler) if(cond) {handler; return ret;} + * + * #define TU_ASSERT(cond) if(cond) {_MESS_FAILED(); TU_BREAKPOINT(), return false;} + * #define TU_ASSERT(cond,ret) if(cond) {_MESS_FAILED(); TU_BREAKPOINT(), return ret;} + * *------------------------------------------------------------------*/ #ifdef __cplusplus -- cgit v1.3.1 From 9c5ae0369b2113ca3c7c8ac1240315ac1e7f1190 Mon Sep 17 00:00:00 2001 From: hathach Date: Mon, 16 Sep 2019 11:13:17 +0700 Subject: follow up to suggestion to pr 155 --- src/common/tusb_types.h | 3 ++- src/device/usbd.c | 19 +++++++++++++------ 2 files changed, 15 insertions(+), 7 deletions(-) (limited to 'src') diff --git a/src/common/tusb_types.h b/src/common/tusb_types.h index a50e89934..ad42baad7 100644 --- a/src/common/tusb_types.h +++ b/src/common/tusb_types.h @@ -125,7 +125,8 @@ typedef enum { TUSB_REQ_TYPE_STANDARD = 0, TUSB_REQ_TYPE_CLASS, - TUSB_REQ_TYPE_VENDOR + TUSB_REQ_TYPE_VENDOR, + TUSB_REQ_TYPE_INVALID } tusb_request_type_t; typedef enum diff --git a/src/device/usbd.c b/src/device/usbd.c index c92884334..0366d1ece 100644 --- a/src/device/usbd.c +++ b/src/device/usbd.c @@ -62,7 +62,7 @@ typedef struct { static usbd_device_t _usbd_dev = { 0 }; // Invalid driver ID in itf2drv[] ep2drv[][] mapping -enum { DRVID_INVALID = 0xff }; +enum { DRVID_INVALID = 0xFFu }; //--------------------------------------------------------------------+ // Class Driver @@ -349,6 +349,8 @@ static bool process_control_request(uint8_t rhport, tusb_control_request_t const { usbd_control_set_complete_callback(NULL); + TU_ASSERT(p_request->bmRequestType_bit.type < TUSB_REQ_TYPE_INVALID); + // Vendor request if ( p_request->bmRequestType_bit.type == TUSB_REQ_TYPE_VENDOR ) { @@ -470,7 +472,8 @@ static bool process_control_request(uint8_t rhport, tusb_control_request_t const // 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_ASSERT(usbd_class_drivers[drvid].control_request(rhport, p_request)); + TU_ASSERT(usbd_class_drivers[drvid].control_request != NULL && + usbd_class_drivers[drvid].control_request(rhport, p_request)); break; } }else @@ -478,7 +481,8 @@ static bool process_control_request(uint8_t rhport, tusb_control_request_t const // 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_ASSERT(usbd_class_drivers[drvid].control_request(rhport, p_request)); + TU_ASSERT(usbd_class_drivers[drvid].control_request != NULL && + usbd_class_drivers[drvid].control_request(rhport, p_request)); } } break; @@ -499,7 +503,7 @@ static bool process_control_request(uint8_t rhport, tusb_control_request_t const // We will forward all request targeted endpoint to its class driver // - For non-standard request: driver can ACK or Stall the request by return true/false // - For standard request: usbd decide the ACK stage regardless of driver return value - bool ret; + bool ret = false; if ( TUSB_REQ_TYPE_STANDARD != p_request->bmRequestType_bit.type ) { @@ -507,8 +511,11 @@ static bool process_control_request(uint8_t rhport, tusb_control_request_t const usbd_control_set_complete_callback(usbd_class_drivers[drv_id].control_complete); } - // Invoke class driver first - ret = usbd_class_drivers[drv_id].control_request(rhport, p_request); + // Invoke class driver first if available + if ( usbd_class_drivers[drv_id].control_request ) + { + ret = usbd_class_drivers[drv_id].control_request(rhport, p_request); + } // Then handle if it is standard request if ( TUSB_REQ_TYPE_STANDARD == p_request->bmRequestType_bit.type ) -- cgit v1.3.1 From a37d2d64beaea07d17b24d8b61bfa09b89ee5bba Mon Sep 17 00:00:00 2001 From: hathach Date: Mon, 16 Sep 2019 13:32:47 +0700 Subject: fix #143 --- src/portable/nxp/lpc17_40/dcd_lpc17_40.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src') diff --git a/src/portable/nxp/lpc17_40/dcd_lpc17_40.c b/src/portable/nxp/lpc17_40/dcd_lpc17_40.c index 2b49f52e9..d5f90d2a7 100644 --- a/src/portable/nxp/lpc17_40/dcd_lpc17_40.c +++ b/src/portable/nxp/lpc17_40/dcd_lpc17_40.c @@ -293,7 +293,7 @@ bool dcd_edpt_open(uint8_t rhport, tusb_desc_endpoint_t const * p_endpoint_desc) break; case TUSB_XFER_ISOCHRONOUS: - TU_ASSERT((epnum % 3) == 3 && (epnum != 15)); + TU_ASSERT((epnum % 3) == 0 && (epnum != 0) && (epnum != 15)); break; default: -- cgit v1.3.1