From 8b9893cadab88fa4abd17d5b9856e0d08ef03796 Mon Sep 17 00:00:00 2001 From: hathach Date: Wed, 9 Sep 2020 15:48:11 +0700 Subject: [PATCH 01/13] introduce optional usbd_edpt_claim, usbd_edpt_release which can be used to gain exclusive access to usbd_edpt_xfer --- src/class/cdc/cdc_device.c | 38 +++++++++++++------- src/device/usbd.c | 71 +++++++++++++++++++++++++++++++++++++- src/device/usbd_pvt.h | 6 ++++ 3 files changed, 101 insertions(+), 14 deletions(-) diff --git a/src/class/cdc/cdc_device.c b/src/class/cdc/cdc_device.c index b4ebfc92e..cd8904042 100644 --- a/src/class/cdc/cdc_device.c +++ b/src/class/cdc/cdc_device.c @@ -77,8 +77,8 @@ static void _prep_out_transaction (uint8_t itf) { cdcd_interface_t* p_cdc = &_cdcd_itf[itf]; - // skip if previous transfer not complete - if ( usbd_edpt_busy(TUD_OPT_RHPORT, p_cdc->ep_out) ) return; + // claim endpoint + TU_VERIFY( usbd_edpt_claim(TUD_OPT_RHPORT, p_cdc->ep_out), ); // Prepare for incoming data but only allow what we can store in the ring buffer. uint16_t max_read = tu_fifo_remaining(&p_cdc->rx_ff); @@ -161,17 +161,29 @@ uint32_t tud_cdc_n_write_flush (uint8_t itf) { cdcd_interface_t* p_cdc = &_cdcd_itf[itf]; - // skip if previous transfer not complete yet - TU_VERIFY( !usbd_edpt_busy(TUD_OPT_RHPORT, p_cdc->ep_in), 0 ); + // No data to send + if ( !tu_fifo_count(&p_cdc->tx_ff) ) return 0; - uint16_t count = tu_fifo_read_n(&p_cdc->tx_ff, p_cdc->epin_buf, sizeof(p_cdc->epin_buf)); - if ( count ) + uint8_t const rhport = TUD_OPT_RHPORT; + + // claim the endpoint first + TU_VERIFY( usbd_edpt_claim(rhport, p_cdc->ep_in), 0 ); + + // we can be blocked by another write()/write_flush() from other thread. + // causing us to attempt to transfer on an busy endpoint. + uint16_t const count = tu_fifo_read_n(&p_cdc->tx_ff, p_cdc->epin_buf, sizeof(p_cdc->epin_buf)); + + if ( count && tud_cdc_n_connected(itf) ) { - TU_VERIFY( tud_cdc_n_connected(itf), 0 ); // fifo is empty if not connected - TU_ASSERT( usbd_edpt_xfer(TUD_OPT_RHPORT, p_cdc->ep_in, p_cdc->epin_buf, count), 0 ); + TU_ASSERT( usbd_edpt_xfer(rhport, p_cdc->ep_in, p_cdc->epin_buf, count), 0 ); + return count; + }else + { + // Release endpoint since we don't make any transfer + // Note: data is dropped if terminal is not connected + usbd_edpt_release(rhport, p_cdc->ep_in); + return 0; } - - return count; } uint32_t tud_cdc_n_write_available (uint8_t itf) @@ -194,7 +206,7 @@ void cdcd_init(void) p_cdc->wanted_char = -1; // default line coding is : stop bit = 1, parity = none, data bits = 8 - p_cdc->line_coding.bit_rate = 115200; + p_cdc->line_coding.bit_rate = 115200; p_cdc->line_coding.stop_bits = 0; p_cdc->line_coding.parity = 0; p_cdc->line_coding.data_bits = 8; @@ -421,10 +433,10 @@ bool cdcd_xfer_cb(uint8_t rhport, uint8_t ep_addr, xfer_result_t result, uint32_ if ( 0 == tud_cdc_n_write_flush(itf) ) { - // There is no data left, a ZLP should be sent if + // If there is no data left, a ZLP should be sent if // xferred_bytes is multiple of EP size and not zero // FIXME CFG_TUD_CDC_EP_BUFSIZE is not Endpoint packet size - if ( xferred_bytes && (0 == (xferred_bytes % CFG_TUD_CDC_EP_BUFSIZE)) ) + if ( !tu_fifo_count(&p_cdc->tx_ff) && xferred_bytes && (0 == (xferred_bytes % CFG_TUD_CDC_EP_BUFSIZE)) ) { usbd_edpt_xfer(rhport, p_cdc->ep_in, NULL, 0); } diff --git a/src/device/usbd.c b/src/device/usbd.c index 6b91048b6..25d3748e1 100644 --- a/src/device/usbd.c +++ b/src/device/usbd.c @@ -63,9 +63,11 @@ typedef struct { volatile bool busy : 1; volatile bool stalled : 1; + volatile bool claimed : 1; // TODO merge ep2drv here, 4-bit should be sufficient }ep_status[8][2]; + }usbd_device_t; static usbd_device_t _usbd_dev; @@ -237,6 +239,13 @@ static inline usbd_class_driver_t const * get_driver(uint8_t drvid) OSAL_QUEUE_DEF(OPT_MODE_DEVICE, _usbd_qdef, CFG_TUD_TASK_QUEUE_SZ, dcd_event_t); static osal_queue_t _usbd_q; +// Mutex for claiming endpoint, only needed when using with preempted RTOS +#if CFG_TUSB_OS != OPT_OS_NONE +static osal_mutex_def_t _ubsd_mutexdef; +static osal_mutex_t _usbd_mutex; +#endif + + //--------------------------------------------------------------------+ // Prototypes //--------------------------------------------------------------------+ @@ -351,9 +360,15 @@ bool tud_init (void) tu_varclr(&_usbd_dev); +#if CFG_TUSB_OS != OPT_OS_NONE + // Init device mutex + _usbd_mutex = osal_mutex_create(&_ubsd_mutexdef); + TU_ASSERT(_usbd_mutex); +#endif + // Init device queue & task _usbd_q = osal_queue_create(&_usbd_qdef); - TU_ASSERT(_usbd_q != NULL); + TU_ASSERT(_usbd_q); // Get application driver if available if ( usbd_app_driver_get_cb ) @@ -478,6 +493,7 @@ void tud_task (void) TU_LOG2("on EP %02X with %u bytes\r\n", ep_addr, (unsigned int) event.xfer_complete.len); _usbd_dev.ep_status[epnum][ep_dir].busy = false; + _usbd_dev.ep_status[epnum][ep_dir].claimed = 0; if ( 0 == epnum ) { @@ -1076,6 +1092,56 @@ bool usbd_edpt_open(uint8_t rhport, tusb_desc_endpoint_t const * desc_ep) return dcd_edpt_open(rhport, desc_ep); } +bool usbd_edpt_claim(uint8_t rhport, uint8_t ep_addr) +{ + (void) rhport; + + uint8_t const epnum = tu_edpt_number(ep_addr); + uint8_t const dir = tu_edpt_dir(ep_addr); + +#if CFG_TUSB_OS != OPT_OS_NONE + osal_mutex_lock(_usbd_mutex, OSAL_TIMEOUT_WAIT_FOREVER); +#endif + + // can only claim the endpoint if it is not busy and not claimed yet. + bool ret = (_usbd_dev.ep_status[epnum][dir].busy == 0) && (_usbd_dev.ep_status[epnum][dir].claimed == 0); + if (ret) + { + _usbd_dev.ep_status[epnum][dir].claimed = 1; + } + +#if CFG_TUSB_OS != OPT_OS_NONE + osal_mutex_unlock(_usbd_mutex); +#endif + + return ret; +} + +bool usbd_edpt_release(uint8_t rhport, uint8_t ep_addr) +{ + (void) rhport; + + uint8_t const epnum = tu_edpt_number(ep_addr); + uint8_t const dir = tu_edpt_dir(ep_addr); + +#if CFG_TUSB_OS != OPT_OS_NONE + osal_mutex_lock(_usbd_mutex, OSAL_TIMEOUT_WAIT_FOREVER); +#endif + + // can only release the endpoint if it is claimed and not busy + bool ret = (_usbd_dev.ep_status[epnum][dir].busy == 0) && (_usbd_dev.ep_status[epnum][dir].claimed == 1); + if (ret) + { + _usbd_dev.ep_status[epnum][dir].claimed = 0; + } + +#if CFG_TUSB_OS != OPT_OS_NONE + osal_mutex_unlock(_usbd_mutex); +#endif + + return ret; +} + bool usbd_edpt_xfer(uint8_t rhport, uint8_t ep_addr, uint8_t * buffer, uint16_t total_bytes) { uint8_t const epnum = tu_edpt_number(ep_addr); @@ -1083,6 +1149,9 @@ bool usbd_edpt_xfer(uint8_t rhport, uint8_t ep_addr, uint8_t * buffer, uint16_t TU_LOG2(" Queue EP %02X with %u bytes ... ", ep_addr, total_bytes); + // Attempt to transfer on busy or not claimed (skip now) endpoint, sound like an race condition ! + TU_ASSERT(_usbd_dev.ep_status[epnum][dir].busy == 0 /*&& _usbd_dev.ep_status[epnum][dir].claimed == 1*/); + // Set busy first since the actual transfer can be complete before dcd_edpt_xfer() could return // and usbd task can preempt and clear the busy _usbd_dev.ep_status[epnum][dir].busy = true; diff --git a/src/device/usbd_pvt.h b/src/device/usbd_pvt.h index a5d223329..5fa9a72a5 100644 --- a/src/device/usbd_pvt.h +++ b/src/device/usbd_pvt.h @@ -70,6 +70,12 @@ void usbd_edpt_close(uint8_t rhport, uint8_t ep_addr); // Submit a usb transfer bool usbd_edpt_xfer(uint8_t rhport, uint8_t ep_addr, uint8_t * buffer, uint16_t total_bytes); +// Claim an endpoint before submitting a transfer +bool usbd_edpt_claim(uint8_t rhport, uint8_t ep_addr); + +// Release an endpoint without submitting a transfer +bool usbd_edpt_release(uint8_t rhport, uint8_t ep_addr); + // Check if endpoint transferring is complete bool usbd_edpt_busy(uint8_t rhport, uint8_t ep_addr); From 33f0a18523accfcdebf186f33271dd92f8681b56 Mon Sep 17 00:00:00 2001 From: hathach Date: Wed, 9 Sep 2020 16:25:31 +0700 Subject: [PATCH 02/13] update cdc edpt read --- src/class/cdc/cdc_device.c | 12 +++++------- src/device/usbd.c | 2 ++ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/class/cdc/cdc_device.c b/src/class/cdc/cdc_device.c index cd8904042..317355ed7 100644 --- a/src/class/cdc/cdc_device.c +++ b/src/class/cdc/cdc_device.c @@ -77,15 +77,13 @@ static void _prep_out_transaction (uint8_t itf) { cdcd_interface_t* p_cdc = &_cdcd_itf[itf]; + // Prepare for incoming data but only allow what we can store in the ring buffer. + uint16_t const available = tu_fifo_remaining(&p_cdc->rx_ff); + TU_VERIFY( available >= sizeof(p_cdc->epout_buf), ); + // claim endpoint TU_VERIFY( usbd_edpt_claim(TUD_OPT_RHPORT, p_cdc->ep_out), ); - - // Prepare for incoming data but only allow what we can store in the ring buffer. - uint16_t max_read = tu_fifo_remaining(&p_cdc->rx_ff); - if ( max_read >= sizeof(p_cdc->epout_buf) ) - { - usbd_edpt_xfer(TUD_OPT_RHPORT, p_cdc->ep_out, p_cdc->epout_buf, sizeof(p_cdc->epout_buf)); - } + usbd_edpt_xfer(TUD_OPT_RHPORT, p_cdc->ep_out, p_cdc->epout_buf, sizeof(p_cdc->epout_buf)); } //--------------------------------------------------------------------+ diff --git a/src/device/usbd.c b/src/device/usbd.c index 25d3748e1..5a7d72124 100644 --- a/src/device/usbd.c +++ b/src/device/usbd.c @@ -1162,7 +1162,9 @@ bool usbd_edpt_xfer(uint8_t rhport, uint8_t ep_addr, uint8_t * buffer, uint16_t return true; }else { + // DCD error, mark endpoint as ready to allow next transfer _usbd_dev.ep_status[epnum][dir].busy = false; + _usbd_dev.ep_status[epnum][dir].claimed = 0; TU_LOG2("failed\r\n"); TU_BREAKPOINT(); return false; From fe1b5dfa235550cc4eb63e237da50689fe407bf4 Mon Sep 17 00:00:00 2001 From: hathach Date: Wed, 9 Sep 2020 16:29:45 +0700 Subject: [PATCH 03/13] clean up --- src/class/cdc/cdc_device.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/class/cdc/cdc_device.c b/src/class/cdc/cdc_device.c index 317355ed7..8ea44b28d 100644 --- a/src/class/cdc/cdc_device.c +++ b/src/class/cdc/cdc_device.c @@ -164,11 +164,10 @@ uint32_t tud_cdc_n_write_flush (uint8_t itf) uint8_t const rhport = TUD_OPT_RHPORT; - // claim the endpoint first - TU_VERIFY( usbd_edpt_claim(rhport, p_cdc->ep_in), 0 ); + // Claim the endpoint first + TU_VnERIFY( usbd_edpt_claim(rhport, p_cdc->ep_in), 0 ); - // we can be blocked by another write()/write_flush() from other thread. - // causing us to attempt to transfer on an busy endpoint. + // Pull data from FIFO uint16_t const count = tu_fifo_read_n(&p_cdc->tx_ff, p_cdc->epin_buf, sizeof(p_cdc->epin_buf)); if ( count && tud_cdc_n_connected(itf) ) From ed6d48b81e8a5d1ed8b518921b30b87c7daecee5 Mon Sep 17 00:00:00 2001 From: hathach Date: Wed, 9 Sep 2020 16:45:54 +0700 Subject: [PATCH 04/13] typo --- src/class/cdc/cdc_device.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/class/cdc/cdc_device.c b/src/class/cdc/cdc_device.c index 8ea44b28d..0792e745d 100644 --- a/src/class/cdc/cdc_device.c +++ b/src/class/cdc/cdc_device.c @@ -165,7 +165,7 @@ uint32_t tud_cdc_n_write_flush (uint8_t itf) uint8_t const rhport = TUD_OPT_RHPORT; // Claim the endpoint first - TU_VnERIFY( usbd_edpt_claim(rhport, p_cdc->ep_in), 0 ); + TU_VERIFY( usbd_edpt_claim(rhport, p_cdc->ep_in), 0 ); // Pull data from FIFO uint16_t const count = tu_fifo_read_n(&p_cdc->tx_ff, p_cdc->epin_buf, sizeof(p_cdc->epin_buf)); From 801f8b5b38b90cb3e15d0a1bb2accae58cde6130 Mon Sep 17 00:00:00 2001 From: hathach Date: Thu, 10 Sep 2020 23:32:08 +0700 Subject: [PATCH 05/13] update claim edpt for hid and midi --- src/class/cdc/cdc_device.c | 41 ++++++++++++++++++++---------------- src/class/hid/hid_device.c | 13 +++++++----- src/class/midi/midi_device.c | 27 ++++++++++++++++++------ src/device/usbd_pvt.h | 3 ++- 4 files changed, 54 insertions(+), 30 deletions(-) diff --git a/src/class/cdc/cdc_device.c b/src/class/cdc/cdc_device.c index 0792e745d..6c1bf6bb2 100644 --- a/src/class/cdc/cdc_device.c +++ b/src/class/cdc/cdc_device.c @@ -73,17 +73,17 @@ typedef struct //--------------------------------------------------------------------+ CFG_TUSB_MEM_SECTION static cdcd_interface_t _cdcd_itf[CFG_TUD_CDC]; -static void _prep_out_transaction (uint8_t itf) +static void _prep_out_transaction (cdcd_interface_t* p_cdc) { - cdcd_interface_t* p_cdc = &_cdcd_itf[itf]; - // Prepare for incoming data but only allow what we can store in the ring buffer. uint16_t const available = tu_fifo_remaining(&p_cdc->rx_ff); TU_VERIFY( available >= sizeof(p_cdc->epout_buf), ); - // claim endpoint - TU_VERIFY( usbd_edpt_claim(TUD_OPT_RHPORT, p_cdc->ep_out), ); - usbd_edpt_xfer(TUD_OPT_RHPORT, p_cdc->ep_out, p_cdc->epout_buf, sizeof(p_cdc->epout_buf)); + // claim endpoint and submit transfer + if ( usbd_edpt_claim(TUD_OPT_RHPORT, p_cdc->ep_out) ) + { + usbd_edpt_xfer(TUD_OPT_RHPORT, p_cdc->ep_out, p_cdc->epout_buf, sizeof(p_cdc->epout_buf)); + } } //--------------------------------------------------------------------+ @@ -121,8 +121,9 @@ uint32_t tud_cdc_n_available(uint8_t itf) uint32_t tud_cdc_n_read(uint8_t itf, void* buffer, uint32_t bufsize) { - uint32_t num_read = tu_fifo_read_n(&_cdcd_itf[itf].rx_ff, buffer, bufsize); - _prep_out_transaction(itf); + cdcd_interface_t* p_cdc = &_cdcd_itf[itf]; + uint32_t num_read = tu_fifo_read_n(&p_cdc->rx_ff, buffer, bufsize); + _prep_out_transaction(p_cdc); return num_read; } @@ -133,8 +134,9 @@ bool tud_cdc_n_peek(uint8_t itf, int pos, uint8_t* chr) void tud_cdc_n_read_flush (uint8_t itf) { - tu_fifo_clear(&_cdcd_itf[itf].rx_ff); - _prep_out_transaction(itf); + cdcd_interface_t* p_cdc = &_cdcd_itf[itf]; + tu_fifo_clear(&p_cdc->rx_ff); + _prep_out_transaction(p_cdc); } //--------------------------------------------------------------------+ @@ -142,11 +144,12 @@ void tud_cdc_n_read_flush (uint8_t itf) //--------------------------------------------------------------------+ uint32_t tud_cdc_n_write(uint8_t itf, void const* buffer, uint32_t bufsize) { - uint16_t ret = tu_fifo_write_n(&_cdcd_itf[itf].tx_ff, buffer, bufsize); + cdcd_interface_t* p_cdc = &_cdcd_itf[itf]; + uint16_t ret = tu_fifo_write_n(&p_cdc->tx_ff, buffer, bufsize); #if 0 // TODO issue with circuitpython's REPL // flush if queue more than endpoint size - if ( tu_fifo_count(&_cdcd_itf[itf].tx_ff) >= CFG_TUD_CDC_EP_BUFSIZE ) + if ( tu_fifo_count(&p_cdc->tx_ff) >= CFG_TUD_CDC_EP_BUFSIZE ) { tud_cdc_n_write_flush(itf); } @@ -164,7 +167,7 @@ uint32_t tud_cdc_n_write_flush (uint8_t itf) uint8_t const rhport = TUD_OPT_RHPORT; - // Claim the endpoint first + // Claim the endpoint TU_VERIFY( usbd_edpt_claim(rhport, p_cdc->ep_in), 0 ); // Pull data from FIFO @@ -242,8 +245,7 @@ uint16_t cdcd_open(uint8_t rhport, tusb_desc_interface_t const * itf_desc, uint1 // Find available interface cdcd_interface_t * p_cdc = NULL; - uint8_t cdc_id; - for(cdc_id=0; cdc_idrx_ff) ) tud_cdc_rx_cb(itf); // prepare for OUT transaction - _prep_out_transaction(itf); + _prep_out_transaction(p_cdc); } // Data sent to host, we continue to fetch from tx fifo to send. @@ -435,7 +437,10 @@ bool cdcd_xfer_cb(uint8_t rhport, uint8_t ep_addr, xfer_result_t result, uint32_ // FIXME CFG_TUD_CDC_EP_BUFSIZE is not Endpoint packet size if ( !tu_fifo_count(&p_cdc->tx_ff) && xferred_bytes && (0 == (xferred_bytes % CFG_TUD_CDC_EP_BUFSIZE)) ) { - usbd_edpt_xfer(rhport, p_cdc->ep_in, NULL, 0); + if ( usbd_edpt_claim(rhport, p_cdc->ep_in) ) + { + usbd_edpt_xfer(rhport, p_cdc->ep_in, NULL, 0); + } } } } diff --git a/src/class/hid/hid_device.c b/src/class/hid/hid_device.c index c46fc591a..10267d734 100644 --- a/src/class/hid/hid_device.c +++ b/src/class/hid/hid_device.c @@ -72,18 +72,21 @@ static inline hidd_interface_t* get_interface_by_itfnum(uint8_t itf_num) //--------------------------------------------------------------------+ bool tud_hid_ready(void) { - uint8_t itf = 0; + uint8_t const itf = 0; uint8_t const ep_in = _hidd_itf[itf].ep_in; - return tud_ready() && (ep_in != 0) && usbd_edpt_ready(TUD_OPT_RHPORT, ep_in); + return tud_ready() && (ep_in != 0) && !usbd_edpt_busy(TUD_OPT_RHPORT, ep_in); } bool tud_hid_report(uint8_t report_id, void const* report, uint8_t len) { - TU_VERIFY( tud_hid_ready() ); - - uint8_t itf = 0; + uint8_t const rhport = 0; + uint8_t const itf = 0; hidd_interface_t * p_hid = &_hidd_itf[itf]; + // claim endpoint + TU_VERIFY( usbd_edpt_claim(rhport, p_hid->ep_in) ); + + // prepare data if (report_id) { len = tu_min8(len, CFG_TUD_HID_EP_BUFSIZE-1); diff --git a/src/class/midi/midi_device.c b/src/class/midi/midi_device.c index 376b3670a..00b1bde92 100644 --- a/src/class/midi/midi_device.c +++ b/src/class/midi/midi_device.c @@ -153,15 +153,25 @@ void midi_rx_done_cb(midid_interface_t* midi, uint8_t const* buffer, uint32_t bu static uint32_t write_flush(midid_interface_t* midi) { + // No data to send + if ( !tu_fifo_count(&midi->tx_ff) ) return 0; + + uint8_t const rhport = TUD_OPT_RHPORT; + // skip if previous transfer not complete - TU_VERIFY( !usbd_edpt_busy(TUD_OPT_RHPORT, midi->ep_in) ); + TU_VERIFY( usbd_edpt_claim(rhport, midi->ep_in), 0 ); uint16_t count = tu_fifo_read_n(&midi->tx_ff, midi->epin_buf, CFG_TUD_MIDI_EP_BUFSIZE); if (count > 0) { - TU_ASSERT( usbd_edpt_xfer(TUD_OPT_RHPORT, midi->ep_in, midi->epin_buf, count) ); + TU_ASSERT( usbd_edpt_xfer(rhport, midi->ep_in, midi->epin_buf, count), 0 ); + return count; + }else + { + // Release endpoint since we don't make any transfer + usbd_edpt_release(rhport, midi->ep_in); + return 0; } - return count; } uint32_t tud_midi_n_write(uint8_t itf, uint8_t jack_id, uint8_t const* buffer, uint32_t bufsize) @@ -405,17 +415,22 @@ bool midid_xfer_cb(uint8_t rhport, uint8_t ep_addr, xfer_result_t result, uint32 if (tud_midi_rx_cb) tud_midi_rx_cb(itf); // prepare for next + // TODO for now ep_out is not used by public API therefore there is no race condition, + // and does not need to claim like ep_in TU_ASSERT(usbd_edpt_xfer(rhport, p_midi->ep_out, p_midi->epout_buf, CFG_TUD_MIDI_EP_BUFSIZE), false); } else if ( ep_addr == p_midi->ep_in ) { if (0 == write_flush(p_midi)) { - // There is no data left, a ZLP should be sent if + // If there is no data left, a ZLP should be sent if // xferred_bytes is multiple of EP size and not zero - if ( xferred_bytes && (0 == (xferred_bytes % CFG_TUD_MIDI_EP_BUFSIZE)) ) + if ( !tu_fifo_count(&p_midi->tx_ff) && xferred_bytes && (0 == (xferred_bytes % CFG_TUD_MIDI_EP_BUFSIZE)) ) { - usbd_edpt_xfer(rhport, p_midi->ep_in, NULL, 0); + if ( usbd_edpt_claim(rhport, p_midi->ep_in) ) + { + usbd_edpt_xfer(rhport, p_midi->ep_in, NULL, 0); + } } } } diff --git a/src/device/usbd_pvt.h b/src/device/usbd_pvt.h index 5fa9a72a5..09b285581 100644 --- a/src/device/usbd_pvt.h +++ b/src/device/usbd_pvt.h @@ -70,7 +70,8 @@ void usbd_edpt_close(uint8_t rhport, uint8_t ep_addr); // Submit a usb transfer bool usbd_edpt_xfer(uint8_t rhport, uint8_t ep_addr, uint8_t * buffer, uint16_t total_bytes); -// Claim an endpoint before submitting a transfer +// Claim an endpoint before submitting a transfer. +// If caller does not make any transfer, it must release endpoint for others. bool usbd_edpt_claim(uint8_t rhport, uint8_t ep_addr); // Release an endpoint without submitting a transfer From ce4a9b9c3ad15a8a714959c89c7a39ba67970cfd Mon Sep 17 00:00:00 2001 From: hathach Date: Fri, 11 Sep 2020 00:14:07 +0700 Subject: [PATCH 06/13] clean up --- src/device/usbd.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/device/usbd.c b/src/device/usbd.c index 5a7d72124..8fc0092eb 100644 --- a/src/device/usbd.c +++ b/src/device/usbd.c @@ -1149,8 +1149,8 @@ bool usbd_edpt_xfer(uint8_t rhport, uint8_t ep_addr, uint8_t * buffer, uint16_t TU_LOG2(" Queue EP %02X with %u bytes ... ", ep_addr, total_bytes); - // Attempt to transfer on busy or not claimed (skip now) endpoint, sound like an race condition ! - TU_ASSERT(_usbd_dev.ep_status[epnum][dir].busy == 0 /*&& _usbd_dev.ep_status[epnum][dir].claimed == 1*/); + // Attempt to transfer on a busy endpoint, sound like an race condition ! + TU_ASSERT(_usbd_dev.ep_status[epnum][dir].busy == 0); // Set busy first since the actual transfer can be complete before dcd_edpt_xfer() could return // and usbd task can preempt and clear the busy From 1804dba615653591a754c9221028c94a32e50495 Mon Sep 17 00:00:00 2001 From: hathach Date: Sat, 12 Sep 2020 08:48:49 +0700 Subject: [PATCH 07/13] typo --- src/class/cdc/cdc_device.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/class/cdc/cdc_device.h b/src/class/cdc/cdc_device.h index 145381c42..3c679c48e 100644 --- a/src/class/cdc/cdc_device.h +++ b/src/class/cdc/cdc_device.h @@ -92,7 +92,7 @@ uint32_t tud_cdc_n_write (uint8_t itf, void const* buffer, uint32_t bu static inline uint32_t tud_cdc_n_write_char (uint8_t itf, char ch); -// Write a nul-terminated string +// Write a null-terminated string static inline uint32_t tud_cdc_n_write_str (uint8_t itf, char const* str); From 25bb8830c5db7985906d2540da2a35e49edddb75 Mon Sep 17 00:00:00 2001 From: hathach Date: Sat, 12 Sep 2020 09:26:41 +0700 Subject: [PATCH 08/13] doc: merge example/readme.md into docs/getting_started.md --- .github/ISSUE_TEMPLATE/bug_report.md | 2 +- docs/getting_started.md | 114 ++++++++++++++++++++++++--- examples/readme.md | 104 ------------------------ 3 files changed, 103 insertions(+), 117 deletions(-) delete mode 100644 examples/readme.md diff --git a/.github/ISSUE_TEMPLATE/bug_report.md b/.github/ISSUE_TEMPLATE/bug_report.md index 5160e9f84..9f956f9d8 100644 --- a/.github/ISSUE_TEMPLATE/bug_report.md +++ b/.github/ISSUE_TEMPLATE/bug_report.md @@ -26,4 +26,4 @@ Steps to reproduce the behavior: If applicable, add screenshots, bus capture to help explain your problem. **Log** -Please provide the stack's log (uart/rtt/swo) where the issue occurred, best with comments to explain the actual events. To enable logging, add `LOG=2` to to the make command if building with stock examples or set `CFG_TUSB_DEBUG=2` in your tusb_config.h. More information can be found at [example's readme](/examples/readme.md) +Please provide the stack's log (uart/rtt/swo) where the issue occurred, best with comments to explain the actual events. To enable logging, add `LOG=2` to to the make command if building with stock examples or set `CFG_TUSB_DEBUG=2` in your tusb_config.h. More information can be found at [example's readme](/docs/getting_started.md) diff --git a/docs/getting_started.md b/docs/getting_started.md index e282b4171..8d22a3127 100644 --- a/docs/getting_started.md +++ b/docs/getting_started.md @@ -1,14 +1,6 @@ # Getting Started # -## Get - -``` -git clone git@github.com:hathach/tinyusb.git tinyusb -``` - -*examples* is the folder where all the application & project files are located. There are demos for both device and hosts. For each, there are different projects for each of supported RTOS. Click to have more information on how to [build](../examples/readme.md) and run [device](../examples/device/readme.md) demos. - -## Add tinyusb to your project +## Add TinyUSB to your project It is relatively simple to incorporate tinyusb to your (existing) project @@ -37,6 +29,104 @@ int main(void) } ~~~ -[//]: # "\subpage md_boards_readme" -[//]: # "\subpage md_doxygen_started_demo" -[//]: # "\subpage md_tools_readme" +## Examples + +For your convenience, TinyUSB contains a handful of examples for both host and device with/without RTOS to quickly test the functionality as well as demonstrate how API() should be used. Most examples will work on most of [the supported Boards](boards.md). Firstly we need to `git clone` if not already + +``` +$ git clone https://github.com/hathach/tinyusb tinyusb +$ cd tinyusb +``` + +TinyUSB examples includes external repos aka submodules to provide low-level MCU peripheral's driver as well as external libraries such as FreeRTOS to compile with. Therefore we will firstly fetch those mcu driver repo by running this command in the top folder repo + +``` +$ git submodule update --init --recursive +``` + +It will takes a bit of time due to the number of supported MCUs, luckily we only need to do this once. Or if you only want to test with a specific mcu, you could only fetch its driver submodule. + +### Build + +To build example, first change directory to an example folder. + +``` +$ cd examples/device/cdc_msc +``` + +Then compile with `make BOARD=[board_name] all`, for example + +``` +$ make BOARD=feather_nrf52840_express all +``` + +#### Port Selection + +If a board has several ports, one port is chosen by default in the individual board.mk file. Use option `PORT=x` To choose another port. For example to select the HS port of a STM32F746Disco board, use: + +``` +$ make BOARD=stm32f746disco PORT=1 all +``` + +#### Port Speed + +A MCU can support multiple operational speed. By default, the example build system will use the fastest supported on the board. Use option `SPEED=full/high` e.g To force F723 operate at full instead of default high speed + +``` +$ make BOARD=stm32f746disco SPEED=full all +``` + +### Debug + +To compile for debugging add `DEBUG=1`, for example + +``` +$ make BOARD=feather_nrf52840_express DEBUG=1 all +``` + +#### Log + +Should you have an issue running example and/or submitting an bug report. You could enable TinyUSB built-in debug logging with optional `LOG=`. LOG=1 will only print out error message, LOG=2 print more information with on-going events. LOG=3 or higher is not used yet. + +``` +$ make BOARD=feather_nrf52840_express LOG=2 all +``` + +#### Logger + +By default log message is printed via on-board UART which is slow and take lots of CPU time comparing to USB speed. If your board support on-board/external debugger, it would be more efficient to use it for logging. There are 2 protocols: + +- `LOGGER=rtt`: use [Segger RTT protocol](https://www.segger.com/products/debug-probes/j-link/technology/about-real-time-transfer/) + - Cons: requires jlink as the debugger. + - Pros: work with most if not all MCUs + - Software viewer is JLink RTT Viewer/Client/Logger which is bundled with JLink driver package. +- `LOGGER=swo`: Use dedicated SWO pin of ARM Cortex SWD debug header. + - Cons: only work with ARM Cortex MCUs minus M0 + - Pros: should be compatible with more debugger that support SWO. + - Software viewer should be provided along with your debugger driver. + +``` +$ make BOARD=feather_nrf52840_express LOG=2 LOGGER=rtt all +$ make BOARD=feather_nrf52840_express LOG=2 LOGGER=swo all +``` + +### Flash + +`flash` target will use the default on-board debugger (jlink/cmsisdap/stlink/dfu) to flash the binary, please install those support software in advance. Some board use bootloader/DFU via serial which is required to pass to make command + +``` +$ make BOARD=feather_nrf52840_express flash +$ make SERIAL=/dev/ttyACM0 BOARD=feather_nrf52840_express flash +``` + +Since jlink can be used with most of the boards, there is also `flash-jlink` target for your convenience. + +``` +$ make BOARD=feather_nrf52840_express flash-jlink +``` + +Some board use uf2 bootloader for drag & drop in to mass storage device, uf2 can be generated with `uf2` target + +``` +$ make BOARD=feather_nrf52840_express all uf2 +``` diff --git a/examples/readme.md b/examples/readme.md deleted file mode 100644 index f6cd380b6..000000000 --- a/examples/readme.md +++ /dev/null @@ -1,104 +0,0 @@ -# Examples - -## Clone this repo - -``` -$ git clone https://github.com/hathach/tinyusb tinyusb -$ cd tinyusb -``` - -## Fetch submodule MCUs drivers - -TinyUSB examples includes external repos aka submodules to provide low-level MCU peripheral's driver to compile with. Therefore we will firstly fetch those mcu driver repo by running this command in the top folder repo - -``` -$ git submodule update --init --recursive -``` - -It will takes a bit of time due to the number of supported MCUs, luckily we only need to do this once. Or if you only want to test with a specific mcu, you could only update its driver submodule - -## Build - -[Here is the list of supported Boards](docs/boards.md) that should work out of the box with provided examples (hopefully). -To build example, first change directory to example folder. - -``` -$ cd examples/device/cdc_msc -``` - -Then compile with `make BOARD=[your_board] all`, for example - -``` -$ make BOARD=feather_nrf52840_express all -``` - -**Port Selection** - -If a board has several ports, one port is chosen by default in the individual board.mk file. Use option `PORT=x` To choose another port. For example to select the HS port of a STM32F746Disco board, use: - -``` -$ make BOARD=stm32f746disco PORT=1 all -``` - -**Port Speed** - -A MCU can support multiple operational speed. By default, the example build system will use the fastest supported on the board. Use option `SPEED=full/high` e.g To force F723 operate at full instead of default high speed - -``` -$ make BOARD=stm32f746disco SPEED=full all -``` - -### Debug - -To compile for debugging with debug symbols add `DEBUG=1`, for example - -``` -$ make BOARD=feather_nrf52840_express DEBUG=1 all -``` - -#### Log - -Should you have an issue running example and/or submitting an bug report. You could enable TinyUSB built-in debug logging with optional `LOG=`. LOG=1 will only print out error message, LOG=2 print more information with on-going events. LOG=3 or higher is not used yet. - -``` -$ make BOARD=feather_nrf52840_express LOG=2 all -``` - -#### Logger - -By default log message is printed via on-board UART which is slow and take lots of CPU time comparing to USB speed. If your board support on-board/external debugger, it would be more efficient to use it for logging. There are 2 protocols: - -- `LOGGER=rtt`: use [Segger RTT protocol](https://www.segger.com/products/debug-probes/j-link/technology/about-real-time-transfer/) - - Cons: requires jlink as the debugger. - - Pros: work with most if not all MCUs - - Software viewer is JLink RTT Viewer/Client/Logger which is bundled with JLink driver package. -- `LOGGER=swo`: Use dedicated SWO pin of ARM Cortex SWD debug header. - - Cons: only work with ARM Cortex MCUs minus M0 - - Pros: should be compatible with more debugger that support SWO. - - Software viewer should be provided along with your debugger driver. - -``` -$ make BOARD=feather_nrf52840_express LOG=2 LOGGER=rtt all -$ make BOARD=feather_nrf52840_express LOG=2 LOGGER=swo all -``` - -## Flash - -`flash` target will use the default on-board debugger (jlink/cmsisdap/stlink/dfu) to flash the binary, please install those support software in advance. Some board use bootloader/DFU via serial which is required to pass to make command - -``` -$ make BOARD=feather_nrf52840_express flash -$ make SERIAL=/dev/ttyACM0 BOARD=feather_nrf52840_express flash -``` - -Since jlink can be used with most of the boards, there is also `flash-jlink` target for your convenience. - -``` -$ make BOARD=feather_nrf52840_express flash-jlink -``` - -Some board use uf2 bootloader for drag & drop in to mass storage device, uf2 can be generated with `uf2` target - -``` -$ make BOARD=feather_nrf52840_express all uf2 -``` From 5931d19666667e9a5b2b99082fb24392e3983399 Mon Sep 17 00:00:00 2001 From: hathach Date: Sun, 13 Sep 2020 15:01:20 +0700 Subject: [PATCH 09/13] correct the TUD_HID_REPORT_DESC_GAMEPAD --- src/class/hid/hid_device.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/class/hid/hid_device.h b/src/class/hid/hid_device.h index f7ad38bab..1e584e4c9 100644 --- a/src/class/hid/hid_device.h +++ b/src/class/hid/hid_device.h @@ -264,7 +264,7 @@ TU_ATTR_WEAK bool tud_hid_set_idle_cb(uint8_t idle_rate); HID_LOGICAL_MAX ( 1 ) ,\ HID_REPORT_COUNT ( 16 ) ,\ HID_REPORT_SIZE ( 1 ) ,\ - HID_INPUT ( HID_DATA | HID_ARRAY | HID_ABSOLUTE ) ,\ + HID_INPUT ( HID_DATA | HID_VARIABLE | HID_ABSOLUTE ) ,\ /* X, Y, Z, Rz (min -127, max 127 ) */ \ HID_USAGE_PAGE ( HID_USAGE_PAGE_DESKTOP ) ,\ HID_LOGICAL_MIN ( 0x81 ) ,\ From 3b0216d3bfe568e949fe1ea7c2c9f3f3a8dc6a4b Mon Sep 17 00:00:00 2001 From: Mark Lentczner Date: Sun, 13 Sep 2020 15:05:18 -0700 Subject: [PATCH 10/13] Update midi_device.c Fix a bug in writing SysEx messages. At the start of a new USB packet (4 bytes), while in the middle of a SysEx, the code mistakenly set the buffer length to 4, not the target length. As a consequence, the 3rd and 4th bytes from the last packet were included, after every byte of the SysEx after the first packet of three. The fix is simple, as it was just a typo, as can bee seen from the other branches in the same section of if/else statements: At the start of a new packet, the code should set up the target length... the buffer length should be left at 2 (as set on line 180). --- src/class/midi/midi_device.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/class/midi/midi_device.c b/src/class/midi/midi_device.c index 376b3670a..1a0bbd170 100644 --- a/src/class/midi/midi_device.c +++ b/src/class/midi/midi_device.c @@ -183,7 +183,7 @@ uint32_t tud_midi_n_write(uint8_t itf, uint8_t jack_id, uint8_t const* buffer, u if (data == 0xf7) { midi->write_buffer[0] = 0x5; } else { - midi->write_buffer_length = 4; + midi->write_target_length = 4; } } else if ((msg >= 0x8 && msg <= 0xB) || msg == 0xE) { midi->write_buffer[0] = jack_id << 4 | msg; From 23e6ee2ea2e69f09b43c9f7736b95560a6e9ef7b Mon Sep 17 00:00:00 2001 From: hathach Date: Mon, 14 Sep 2020 22:14:31 +0700 Subject: [PATCH 11/13] cdc device: claim endpoint before checking fifo availability - add pre-check to reduce mutex lock in usbd_edpt_claim --- src/class/cdc/cdc_device.c | 25 +++++++++++++++++++------ src/device/usbd.c | 5 ++++- 2 files changed, 23 insertions(+), 7 deletions(-) diff --git a/src/class/cdc/cdc_device.c b/src/class/cdc/cdc_device.c index 6c1bf6bb2..e39adc5fa 100644 --- a/src/class/cdc/cdc_device.c +++ b/src/class/cdc/cdc_device.c @@ -75,14 +75,27 @@ CFG_TUSB_MEM_SECTION static cdcd_interface_t _cdcd_itf[CFG_TUD_CDC]; static void _prep_out_transaction (cdcd_interface_t* p_cdc) { - // Prepare for incoming data but only allow what we can store in the ring buffer. - uint16_t const available = tu_fifo_remaining(&p_cdc->rx_ff); - TU_VERIFY( available >= sizeof(p_cdc->epout_buf), ); + uint8_t const rhport = TUD_OPT_RHPORT; + uint16_t available = tu_fifo_remaining(&p_cdc->rx_ff); - // claim endpoint and submit transfer - if ( usbd_edpt_claim(TUD_OPT_RHPORT, p_cdc->ep_out) ) + // Prepare for incoming data but only allow what we can store in the ring buffer. + // TODO Actually we can still carry out the transfer, keeping count of received bytes + // and slowly move it to the FIFO when read(). + // This pre-check reduces endpoint claiming + TU_VERIFY(available >= sizeof(p_cdc->epout_buf), ); + + // claim endpoint + TU_VERIFY(usbd_edpt_claim(rhport, p_cdc->ep_out), ); + + // fifo can be changed before endpoint is claimed + available = tu_fifo_remaining(&p_cdc->rx_ff); + + if ( available >= sizeof(p_cdc->epout_buf) ) { + usbd_edpt_xfer(rhport, p_cdc->ep_out, p_cdc->epout_buf, sizeof(p_cdc->epout_buf)); + }else { - usbd_edpt_xfer(TUD_OPT_RHPORT, p_cdc->ep_out, p_cdc->epout_buf, sizeof(p_cdc->epout_buf)); + // Release endpoint since we don't make any transfer + usbd_edpt_release(rhport, p_cdc->ep_out); } } diff --git a/src/device/usbd.c b/src/device/usbd.c index 8fc0092eb..dfe30578e 100644 --- a/src/device/usbd.c +++ b/src/device/usbd.c @@ -1100,11 +1100,14 @@ bool usbd_edpt_claim(uint8_t rhport, uint8_t ep_addr) uint8_t const dir = tu_edpt_dir(ep_addr); #if CFG_TUSB_OS != OPT_OS_NONE + // pre-check to help reducing mutex lock + TU_VERIFY((_usbd_dev.ep_status[epnum][dir].busy == 0) && (_usbd_dev.ep_status[epnum][dir].claimed == 0)); + osal_mutex_lock(_usbd_mutex, OSAL_TIMEOUT_WAIT_FOREVER); #endif // can only claim the endpoint if it is not busy and not claimed yet. - bool ret = (_usbd_dev.ep_status[epnum][dir].busy == 0) && (_usbd_dev.ep_status[epnum][dir].claimed == 0); + bool const ret = (_usbd_dev.ep_status[epnum][dir].busy == 0) && (_usbd_dev.ep_status[epnum][dir].claimed == 0); if (ret) { _usbd_dev.ep_status[epnum][dir].claimed = 1; From 9c0d15fc43818282b6a92432e675dd214e51735e Mon Sep 17 00:00:00 2001 From: hathach Date: Mon, 14 Sep 2020 22:23:59 +0700 Subject: [PATCH 12/13] more const --- src/device/usbd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/device/usbd.c b/src/device/usbd.c index dfe30578e..d1e984a0e 100644 --- a/src/device/usbd.c +++ b/src/device/usbd.c @@ -1132,7 +1132,7 @@ bool usbd_edpt_release(uint8_t rhport, uint8_t ep_addr) #endif // can only release the endpoint if it is claimed and not busy - bool ret = (_usbd_dev.ep_status[epnum][dir].busy == 0) && (_usbd_dev.ep_status[epnum][dir].claimed == 1); + bool const ret = (_usbd_dev.ep_status[epnum][dir].busy == 0) && (_usbd_dev.ep_status[epnum][dir].claimed == 1); if (ret) { _usbd_dev.ep_status[epnum][dir].claimed = 0; From 62a76c0e049998c6fda2f349503bee2fafd12bad Mon Sep 17 00:00:00 2001 From: Jerzy Kasenberg Date: Tue, 15 Sep 2020 16:08:23 +0200 Subject: [PATCH 13/13] nrf52: Fix edpt_dma_start() wrong condition check Operator < used in while condition was obviously incorrect. Loop starts with checking if unsigned variable is less then 0. This condition is always false. This reverses condition to follow intention of of the code. --- src/portable/nordic/nrf5x/dcd_nrf5x.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/portable/nordic/nrf5x/dcd_nrf5x.c b/src/portable/nordic/nrf5x/dcd_nrf5x.c index cd35f2e5b..60da9fb25 100644 --- a/src/portable/nordic/nrf5x/dcd_nrf5x.c +++ b/src/portable/nordic/nrf5x/dcd_nrf5x.c @@ -122,7 +122,7 @@ static void edpt_dma_start(volatile uint32_t* reg_startep) // for the DMA complete by comparing current pending DMA with number of ENDED Events uint32_t ended = 0; - while ( _dcd.dma_pending < ((uint8_t) ended) ) + while ( _dcd.dma_pending > ((uint8_t) ended) ) { ended = NRF_USBD->EVENTS_ENDISOIN + NRF_USBD->EVENTS_ENDISOOUT;