From bb7f581b6d046c1c358fab70f3640d0312527804 Mon Sep 17 00:00:00 2001 From: Nathan Conrad Date: Fri, 13 Sep 2019 20:28:26 -0400 Subject: TI compiler quirks, and le byte swapping functions. --- src/common/tusb_compiler.h | 83 ++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 73 insertions(+), 10 deletions(-) (limited to 'src') diff --git a/src/common/tusb_compiler.h b/src/common/tusb_compiler.h index 09768ef88..ca294c20f 100644 --- a/src/common/tusb_compiler.h +++ b/src/common/tusb_compiler.h @@ -53,6 +53,9 @@ // for declaration of reserved field, make use of _TU_COUNTER_ #define TU_RESERVED TU_XSTRCAT(reserved, _TU_COUNTER_) +#define TU_LITTLE_ENDIAN (0x12u) +#define TU_BIG_ENDIAN (0x21u) + //--------------------------------------------------------------------+ // Compiler porting with Attribute and Endian //--------------------------------------------------------------------+ @@ -67,20 +70,80 @@ // Endian conversion use well-known host to network (big endian) naming #if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__ - #define tu_htonl(u32) __builtin_bswap32(u32) - #define tu_ntohl(u32) __builtin_bswap32(u32) - - #define tu_htons(u16) __builtin_bswap16(u16) - #define tu_ntohs(u16) __builtin_bswap16(u16) + #define TU_BYTE_ORDER TU_LITTLE_ENDIAN #else - #define tu_htonl(u32) (u32) - #define tu_ntohl(u32) (u32) + #define TU_BYTE_ORDER TU_BIG_ENDIAN + #endif + + static inline uint16_t tu_bswap16(uint16_t u16) + { + return __builtin_bswap16(u16); + } + + static inline uint16_t tu_bswap32(uint16_t u32) + { + return __builtin_bswap32(u32); + } + + #define TU_BSWAP16 + +#elif defined(__TI_COMPILER_VERSION__) + #define TU_ATTR_ALIGNED(Bytes) __attribute__ ((aligned(Bytes))) + #define TU_ATTR_SECTION(sec_name) __attribute__ ((section(#sec_name))) + #define TU_ATTR_PACKED __attribute__ ((packed)) + #define TU_ATTR_PREPACKED + #define TU_ATTR_WEAK __attribute__ ((weak)) + #define TU_ATTR_DEPRECATED(mess) __attribute__ ((deprecated(mess))) // warn if function with this attribute is used + #define TU_ATTR_UNUSED __attribute__ ((unused)) // Function/Variable is meant to be possibly unused - #define tu_htons(u16) (u16) - #define tu_ntohs(u16) (u16) + // __BYTE_ORDER is defined in the TI ARM compiler, but not MSP430 (which is little endian) + #if ((__BYTE_ORDER__) == (__ORDER_LITTLE_ENDIAN__)) || defined(__MSP430__) + #define TU_BYTE_ORDER TU_LITTLE_ENDIAN + #else + #define TU_BYTE_ORDER TU_BIG_ENDIAN #endif + + static inline uint16_t tu_bswap16(uint16_t u16) + { + return __builtin_bswap16(u16); + } + + static inline uint16_t tu_bswap32(uint16_t u32) + { + return __builtin_bswap32(u32); + } +#else + #error "Compiler attribute porting is required" +#endif + +#if (TU_BYTE_ORDER == TU_LITTLE_ENDIAN) + #define tu_htonl(u32) tu_bswap32(u32) + #define tu_ntohl(u32) tu_bswap32(u32) + + #define tu_htons(u16) tu_bswap16(u16) + #define tu_ntohs(u16) tu_bswap16(u16) + + #define tu_htole16(x) (x) + #define tu_le16toh(x) (x) + + #define tu_htole32(x) (x) + #define tu_le32toh(x) (x) + +#elif (TU_BYTE_ORDER == TU_BIG_ENDIAN) + #define tu_htonl(u32) (x) + #define tu_ntohl(u32) (x) + + #define tu_htons(u16) (x) + #define tu_ntohs(u16) (x) + + #define tu_htole16(x) tu_bswap16(u32) + #define tu_le16toh(x) tu_bswap16(u32) + + #define tu_htole32(x) tu_bswap32(u32) + #define tu_le32toh(x) tu_bswap32(u32) + #else - #error "Compiler attribute porting are required" + #error Byte order is undefined #endif #endif /* _TUSB_COMPILER_H_ */ -- 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 9593463367cca64acc7732013efdc634e84c2a81 Mon Sep 17 00:00:00 2001 From: Nathan Conrad Date: Sun, 15 Sep 2019 15:20:01 -0400 Subject: Massive copy&paste typo of mine in the 32-bit byte swapping function.... --- src/common/tusb_compiler.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'src') diff --git a/src/common/tusb_compiler.h b/src/common/tusb_compiler.h index ca294c20f..df0326afd 100644 --- a/src/common/tusb_compiler.h +++ b/src/common/tusb_compiler.h @@ -80,7 +80,7 @@ return __builtin_bswap16(u16); } - static inline uint16_t tu_bswap32(uint16_t u32) + static inline uint32_t tu_bswap32(uint32_t u32) { return __builtin_bswap32(u32); } @@ -108,7 +108,7 @@ return __builtin_bswap16(u16); } - static inline uint16_t tu_bswap32(uint16_t u32) + static inline uint32_t tu_bswap32(uint32_t u32) { return __builtin_bswap32(u32); } -- 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 From dfe92542e63af0032c5f9b0fcd14a5f5971f78fe Mon Sep 17 00:00:00 2001 From: Nathan Conrad Date: Mon, 16 Sep 2019 11:27:05 -0400 Subject: Change inline functions to macros, and make all parameter names uniform. --- src/common/tusb_compiler.h | 58 ++++++++++++++++++---------------------------- 1 file changed, 23 insertions(+), 35 deletions(-) (limited to 'src') diff --git a/src/common/tusb_compiler.h b/src/common/tusb_compiler.h index df0326afd..58732b871 100644 --- a/src/common/tusb_compiler.h +++ b/src/common/tusb_compiler.h @@ -75,17 +75,8 @@ #define TU_BYTE_ORDER TU_BIG_ENDIAN #endif - static inline uint16_t tu_bswap16(uint16_t u16) - { - return __builtin_bswap16(u16); - } - - static inline uint32_t tu_bswap32(uint32_t u32) - { - return __builtin_bswap32(u32); - } - - #define TU_BSWAP16 +#define TU_BSWAP16(u16) (__builtin_bswap16(u16)) +#define TU_BSWAP32(u32) (__builtin_bswap32(u32)) #elif defined(__TI_COMPILER_VERSION__) #define TU_ATTR_ALIGNED(Bytes) __attribute__ ((aligned(Bytes))) @@ -103,44 +94,41 @@ #define TU_BYTE_ORDER TU_BIG_ENDIAN #endif - static inline uint16_t tu_bswap16(uint16_t u16) - { - return __builtin_bswap16(u16); - } + #define TU_BSWAP16(u16) (__builtin_bswap16(u16)) + #define TU_BSWAP32(u32) (__builtin_bswap32(u32)) - static inline uint32_t tu_bswap32(uint32_t u32) - { - return __builtin_bswap32(u32); - } #else #error "Compiler attribute porting is required" #endif #if (TU_BYTE_ORDER == TU_LITTLE_ENDIAN) - #define tu_htonl(u32) tu_bswap32(u32) - #define tu_ntohl(u32) tu_bswap32(u32) - #define tu_htons(u16) tu_bswap16(u16) - #define tu_ntohs(u16) tu_bswap16(u16) + #define tu_htons(u16) (TU_BSWAP16(u16)) + #define tu_ntohs(u16) (TU_BSWAP16(u16)) + + #define tu_htonl(u32) (TU_BSWAP32(u32)) + #define tu_ntohl(u32) (TU_BSWAP32(u32)) - #define tu_htole16(x) (x) - #define tu_le16toh(x) (x) + #define tu_htole16(u16) (u16) + #define tu_le16toh(u16) (u16) - #define tu_htole32(x) (x) - #define tu_le32toh(x) (x) + #define tu_htole32(u32) (u32) + #define tu_le32toh(u32) (u32) #elif (TU_BYTE_ORDER == TU_BIG_ENDIAN) - #define tu_htonl(u32) (x) - #define tu_ntohl(u32) (x) - #define tu_htons(u16) (x) - #define tu_ntohs(u16) (x) + #define tu_htons(u16) (u16) + #define tu_ntohs(u16) (u16) + + #define tu_htonl(u32) (u32) + #define tu_ntohl(u32) (u32) + - #define tu_htole16(x) tu_bswap16(u32) - #define tu_le16toh(x) tu_bswap16(u32) + #define tu_htole16(u16) (tu_bswap16(u16)) + #define tu_le16toh(u16) (tu_bswap16(u16)) - #define tu_htole32(x) tu_bswap32(u32) - #define tu_le32toh(x) tu_bswap32(u32) + #define tu_htole32(u32) (tu_bswap32(u32)) + #define tu_le32toh(u32) (tu_bswap32(u32)) #else #error Byte order is undefined -- cgit v1.3.1