From 340dcb81bfc7074ca01bbee6177ea52562aa3b51 Mon Sep 17 00:00:00 2001 From: Nathan Conrad Date: Thu, 12 Sep 2019 15:39:51 -0400 Subject: [PATCH 1/4] For control transfers, compare the transmitted length against the requested length to know if a ZLP needs to happen. (fixes #139) --- src/device/usbd_control.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/src/device/usbd_control.c b/src/device/usbd_control.c index 4ec43218..e54d0445 100644 --- a/src/device/usbd_control.c +++ b/src/device/usbd_control.c @@ -43,8 +43,9 @@ typedef struct tusb_control_request_t request; void* buffer; - uint16_t total_len; + uint16_t len; uint16_t total_transferred; + uint16_t requested_len; bool (*complete_cb) (uint8_t, tusb_control_request_t const *); } usbd_control_xfer_t; @@ -68,7 +69,7 @@ bool tud_control_status(uint8_t rhport, tusb_control_request_t const * request) // Each transaction is up to endpoint0's max packet size static bool start_control_data_xact(uint8_t rhport) { - uint16_t const xact_len = tu_min16(_control_state.total_len - _control_state.total_transferred, CFG_TUD_ENDPOINT0_SIZE); + uint16_t const xact_len = tu_min16(_control_state.len - _control_state.total_transferred, CFG_TUD_ENDPOINT0_SIZE); uint8_t ep_addr = EDPT_CTRL_OUT; @@ -89,10 +90,16 @@ void usbd_control_set_complete_callback( bool (*fp) (uint8_t, tusb_control_reque bool tud_control_xfer(uint8_t rhport, tusb_control_request_t const * request, void* buffer, uint16_t len) { + // transmitted length must be <= requested length (USB 2.0 spec: 8.5.3.1 ) + // FIXME: Should logic be here or in place that calls this function? + if(len > request->wLength) + len = request->wLength; + _control_state.request = (*request); _control_state.buffer = buffer; - _control_state.total_len = tu_min16(len, request->wLength); _control_state.total_transferred = 0; + _control_state.requested_len = request->wLength; + _control_state.len = len; if ( len ) { @@ -124,7 +131,7 @@ bool usbd_control_xfer_cb (uint8_t rhport, uint8_t ep_addr, xfer_result_t result _control_state.total_transferred += xferred_bytes; _control_state.buffer += xferred_bytes; - if ( _control_state.total_len == _control_state.total_transferred || xferred_bytes < CFG_TUD_ENDOINT0_SIZE ) + if ( (_control_state.requested_len == _control_state.total_transferred) || xferred_bytes < CFG_TUD_ENDOINT0_SIZE ) { // DATA stage is complete bool is_ok = true; From 8cca287683a4376f7483b579dcf36c2d1daf8fab Mon Sep 17 00:00:00 2001 From: Nathan Conrad Date: Thu, 12 Sep 2019 16:03:45 -0400 Subject: [PATCH 2/4] Add verification that there is enough buffer space for HID OUT control transfer. --- src/class/hid/hid_device.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/class/hid/hid_device.c b/src/class/hid/hid_device.c index 572f2cad..7270904c 100644 --- a/src/class/hid/hid_device.c +++ b/src/class/hid/hid_device.c @@ -230,6 +230,7 @@ bool hidd_control_request(uint8_t rhport, tusb_control_request_t const * p_reque break; case HID_REQ_CONTROL_SET_REPORT: + TU_VERIFY(p_request->wLength <=sizeof(p_hid->epout_buf)); tud_control_xfer(rhport, p_request, p_hid->epout_buf, p_request->wLength); break; From 1e17e2e7647b5ed329b58886ad4a89fc39f02fdc Mon Sep 17 00:00:00 2001 From: Nathan Conrad Date: Thu, 12 Sep 2019 16:54:25 -0400 Subject: [PATCH 3/4] Add details on dcd_edpt_xfer in the porting document. --- docs/porting.md | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/docs/porting.md b/docs/porting.md index ae701a94..ded35e7b 100644 --- a/docs/porting.md +++ b/docs/porting.md @@ -127,15 +127,27 @@ Also make sure to enable endpoint specific interrupts. `dcd_edpt_xfer` is used for all endpoints including the control endpoint 0. Make sure to handle the zero-length packet STATUS packet on endpoint 0 correctly. It may be a special transaction to the peripheral. -Besides that, all other transactions are relatively straight-forward. The endpoint address provides the endpoint number and direction which usually determines where to write the buffer info. The buffer and its length are usually written to a specific location in memory and the peripheral is told the data is valid. (Maybe by writing a 1 to a register or setting a counter register to 0 for OUT or length for IN.) +Besides that, all other transactions are relatively straight-forward. The endpoint address provides the endpoint +number and direction which usually determines where to write the buffer info. The buffer and its length are usually +written to a specific location in memory and the peripheral is told the data is valid. (Maybe by writing a 1 to a +register or setting a counter register to 0 for OUT or length for IN.) -TODO: can we promise the buffer is word aligned? +The transmit buffer is NOT guarenteed to be word-aligned. -One potential pitfall is that the buffer may be longer than the maximum endpoint size of one USB packet. Some peripherals can handle transmitting multiple USB packets for a provided buffer (like the SAMD21). Others (like the nRF52) may need each USB packet queued individually. To make this work you'll need to track some state for yourself and queue up an intermediate USB packet from the interrupt handler. +One potential pitfall is that the buffer may be longer than the maximum endpoint size of one USB +packet. Some peripherals can handle transmitting multiple USB packets for a provided buffer (like the SAMD21). +Others (like the nRF52) may need each USB packet queued individually. To make this work you'll need to track +some state for yourself and queue up an intermediate USB packet from the interrupt handler. Once the transaction is going, the interrupt handler will notify TinyUSB of transfer completion. +During transmission, the IN data buffer is guarenteed to remain unchanged in memory until the `dcd_xfer_complete` function is called. -TODO: who handles zero-length data packets? +The dcd_edpt_xfer function must never add zero-length-packets (ZLP) on its own to a transfer. If a ZLP is required, +then it must be explicitly sent by the module calling dcd_edpt_xfer(), by calling dcd_edpt_xfer() a second time with len=0. +For control transfers, this is automatically done in `usbd_control.c`. + +At the moment, only a single buffer can be transmitted at once. There is no provision for double-buffering. new dcd_edpt_xfer() will not +be called again until the driver calls dcd_xfer_complete() (except in cases of USB resets). ##### dcd_xfer_complete From 4ea2a4ed60c41c8b55bfb40459e5f54615e2dc72 Mon Sep 17 00:00:00 2001 From: Nathan Conrad Date: Sun, 29 Sep 2019 10:45:13 -0400 Subject: [PATCH 4/4] Github's web interface changed line endings without asking me. Oops. --- src/device/usbd_control.c | 334 +++++++++++++++++++------------------- 1 file changed, 167 insertions(+), 167 deletions(-) diff --git a/src/device/usbd_control.c b/src/device/usbd_control.c index de107503..e8bb648c 100644 --- a/src/device/usbd_control.c +++ b/src/device/usbd_control.c @@ -1,167 +1,167 @@ -/* - * The MIT License (MIT) - * - * Copyright (c) 2019 Ha Thach (tinyusb.org) - * - * Permission is hereby granted, free of charge, to any person obtaining a copy - * of this software and associated documentation files (the "Software"), to deal - * in the Software without restriction, including without limitation the rights - * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell - * copies of the Software, and to permit persons to whom the Software is - * furnished to do so, subject to the following conditions: - * - * The above copyright notice and this permission notice shall be included in - * all copies or substantial portions of the Software. - * - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE - * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER - * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, - * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN - * THE SOFTWARE. - * - * This file is part of the TinyUSB stack. - */ - -#include "tusb_option.h" - -#if TUSB_OPT_DEVICE_ENABLED - -#include "tusb.h" -#include "device/usbd_pvt.h" -#include "dcd.h" - -enum -{ - EDPT_CTRL_OUT = 0x00, - EDPT_CTRL_IN = 0x80 -}; - -typedef struct -{ - tusb_control_request_t request; - - void* buffer; - uint16_t len; - uint16_t total_transferred; - uint16_t requested_len; - - bool (*complete_cb) (uint8_t, tusb_control_request_t const *); -} usbd_control_xfer_t; - -static usbd_control_xfer_t _control_state; - -CFG_TUSB_MEM_SECTION CFG_TUSB_MEM_ALIGN uint8_t _usbd_ctrl_buf[CFG_TUD_ENDPOINT0_SIZE]; - -void usbd_control_reset (uint8_t rhport) -{ - (void) rhport; - tu_varclr(&_control_state); -} - -bool tud_control_status(uint8_t rhport, tusb_control_request_t const * request) -{ - // status direction is reversed to one in the setup packet - return dcd_edpt_xfer(rhport, request->bmRequestType_bit.direction ? EDPT_CTRL_OUT : EDPT_CTRL_IN, NULL, 0); -} - -// Each transaction is up to endpoint0's max packet size -static bool start_control_data_xact(uint8_t rhport) -{ - uint16_t const xact_len = tu_min16(_control_state.len - _control_state.total_transferred, CFG_TUD_ENDPOINT0_SIZE); - - uint8_t ep_addr = EDPT_CTRL_OUT; - - if ( _control_state.request.bmRequestType_bit.direction == TUSB_DIR_IN ) - { - ep_addr = EDPT_CTRL_IN; - memcpy(_usbd_ctrl_buf, _control_state.buffer, xact_len); - } - - return dcd_edpt_xfer(rhport, ep_addr, _usbd_ctrl_buf, xact_len); -} - -// TODO may find a better way -void usbd_control_set_complete_callback( bool (*fp) (uint8_t, tusb_control_request_t const * ) ) -{ - _control_state.complete_cb = fp; -} - -bool tud_control_xfer(uint8_t rhport, tusb_control_request_t const * request, void* buffer, uint16_t len) -{ - // transmitted length must be <= requested length (USB 2.0 spec: 8.5.3.1 ) - // FIXME: Should logic be here or in place that calls this function? - if(len > request->wLength) - len = request->wLength; - - _control_state.request = (*request); - _control_state.buffer = buffer; - _control_state.total_transferred = 0; - _control_state.requested_len = request->wLength; - _control_state.len = len; - - if ( len ) - { - TU_ASSERT(buffer); - - // Data stage - TU_ASSERT( start_control_data_xact(rhport) ); - }else - { - // Status stage - TU_ASSERT( tud_control_status(rhport, request) ); - } - - return true; -} - -// callback when a transaction complete on DATA stage of control endpoint -bool usbd_control_xfer_cb (uint8_t rhport, uint8_t ep_addr, xfer_result_t result, uint32_t xferred_bytes) -{ - (void) result; - (void) ep_addr; - - if ( _control_state.request.bmRequestType_bit.direction == TUSB_DIR_OUT ) - { - TU_VERIFY(_control_state.buffer); - memcpy(_control_state.buffer, _usbd_ctrl_buf, xferred_bytes); - } - - _control_state.total_transferred += xferred_bytes; - _control_state.buffer = ((uint8_t*)_control_state.buffer) + xferred_bytes; - - if ( (_control_state.requested_len == _control_state.total_transferred) || xferred_bytes < CFG_TUD_ENDPOINT0_SIZE ) - - { - // DATA stage is complete - bool is_ok = true; - - // invoke complete callback if set - // callback can still stall control in status phase e.g out data does not make sense - if ( _control_state.complete_cb ) - { - is_ok = _control_state.complete_cb(rhport, &_control_state.request); - } - - if ( is_ok ) - { - // Send status - TU_ASSERT( tud_control_status(rhport, &_control_state.request) ); - }else - { - // Stall both IN and OUT control endpoint - dcd_edpt_stall(rhport, EDPT_CTRL_OUT); - dcd_edpt_stall(rhport, EDPT_CTRL_IN); - } - } - else - { - // More data to transfer - TU_ASSERT( start_control_data_xact(rhport) ); - } - - return true; -} - -#endif +/* + * The MIT License (MIT) + * + * Copyright (c) 2019 Ha Thach (tinyusb.org) + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + * + * This file is part of the TinyUSB stack. + */ + +#include "tusb_option.h" + +#if TUSB_OPT_DEVICE_ENABLED + +#include "tusb.h" +#include "device/usbd_pvt.h" +#include "dcd.h" + +enum +{ + EDPT_CTRL_OUT = 0x00, + EDPT_CTRL_IN = 0x80 +}; + +typedef struct +{ + tusb_control_request_t request; + + void* buffer; + uint16_t len; + uint16_t total_transferred; + uint16_t requested_len; + + bool (*complete_cb) (uint8_t, tusb_control_request_t const *); +} usbd_control_xfer_t; + +static usbd_control_xfer_t _control_state; + +CFG_TUSB_MEM_SECTION CFG_TUSB_MEM_ALIGN uint8_t _usbd_ctrl_buf[CFG_TUD_ENDPOINT0_SIZE]; + +void usbd_control_reset (uint8_t rhport) +{ + (void) rhport; + tu_varclr(&_control_state); +} + +bool tud_control_status(uint8_t rhport, tusb_control_request_t const * request) +{ + // status direction is reversed to one in the setup packet + return dcd_edpt_xfer(rhport, request->bmRequestType_bit.direction ? EDPT_CTRL_OUT : EDPT_CTRL_IN, NULL, 0); +} + +// Each transaction is up to endpoint0's max packet size +static bool start_control_data_xact(uint8_t rhport) +{ + uint16_t const xact_len = tu_min16(_control_state.len - _control_state.total_transferred, CFG_TUD_ENDPOINT0_SIZE); + + uint8_t ep_addr = EDPT_CTRL_OUT; + + if ( _control_state.request.bmRequestType_bit.direction == TUSB_DIR_IN ) + { + ep_addr = EDPT_CTRL_IN; + memcpy(_usbd_ctrl_buf, _control_state.buffer, xact_len); + } + + return dcd_edpt_xfer(rhport, ep_addr, _usbd_ctrl_buf, xact_len); +} + +// TODO may find a better way +void usbd_control_set_complete_callback( bool (*fp) (uint8_t, tusb_control_request_t const * ) ) +{ + _control_state.complete_cb = fp; +} + +bool tud_control_xfer(uint8_t rhport, tusb_control_request_t const * request, void* buffer, uint16_t len) +{ + // transmitted length must be <= requested length (USB 2.0 spec: 8.5.3.1 ) + // FIXME: Should logic be here or in place that calls this function? + if(len > request->wLength) + len = request->wLength; + + _control_state.request = (*request); + _control_state.buffer = buffer; + _control_state.total_transferred = 0; + _control_state.requested_len = request->wLength; + _control_state.len = len; + + if ( len ) + { + TU_ASSERT(buffer); + + // Data stage + TU_ASSERT( start_control_data_xact(rhport) ); + }else + { + // Status stage + TU_ASSERT( tud_control_status(rhport, request) ); + } + + return true; +} + +// callback when a transaction complete on DATA stage of control endpoint +bool usbd_control_xfer_cb (uint8_t rhport, uint8_t ep_addr, xfer_result_t result, uint32_t xferred_bytes) +{ + (void) result; + (void) ep_addr; + + if ( _control_state.request.bmRequestType_bit.direction == TUSB_DIR_OUT ) + { + TU_VERIFY(_control_state.buffer); + memcpy(_control_state.buffer, _usbd_ctrl_buf, xferred_bytes); + } + + _control_state.total_transferred += xferred_bytes; + _control_state.buffer = ((uint8_t*)_control_state.buffer) + xferred_bytes; + + if ( (_control_state.requested_len == _control_state.total_transferred) || xferred_bytes < CFG_TUD_ENDPOINT0_SIZE ) + + { + // DATA stage is complete + bool is_ok = true; + + // invoke complete callback if set + // callback can still stall control in status phase e.g out data does not make sense + if ( _control_state.complete_cb ) + { + is_ok = _control_state.complete_cb(rhport, &_control_state.request); + } + + if ( is_ok ) + { + // Send status + TU_ASSERT( tud_control_status(rhport, &_control_state.request) ); + }else + { + // Stall both IN and OUT control endpoint + dcd_edpt_stall(rhport, EDPT_CTRL_OUT); + dcd_edpt_stall(rhport, EDPT_CTRL_IN); + } + } + else + { + // More data to transfer + TU_ASSERT( start_control_data_xact(rhport) ); + } + + return true; +} + +#endif