From 8b9893cadab88fa4abd17d5b9856e0d08ef03796 Mon Sep 17 00:00:00 2001
From: hathach <thach@tinyusb.org>
Date: Wed, 9 Sep 2020 15:48:11 +0700
Subject: [PATCH 1/9] 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 <thach@tinyusb.org>
Date: Wed, 9 Sep 2020 16:25:31 +0700
Subject: [PATCH 2/9] 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 <thach@tinyusb.org>
Date: Wed, 9 Sep 2020 16:29:45 +0700
Subject: [PATCH 3/9] 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 <thach@tinyusb.org>
Date: Wed, 9 Sep 2020 16:45:54 +0700
Subject: [PATCH 4/9] 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 <thach@tinyusb.org>
Date: Thu, 10 Sep 2020 23:32:08 +0700
Subject: [PATCH 5/9] 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_id<CFG_TUD_CDC; cdc_id++)
+ for(uint8_t cdc_id=0; cdc_id<CFG_TUD_CDC; cdc_id++)
{
if ( _cdcd_itf[cdc_id].ep_in == 0 )
{
@@ -292,7 +294,7 @@ uint16_t cdcd_open(uint8_t rhport, tusb_desc_interface_t const * itf_desc, uint1
}
// Prepare for incoming data
- _prep_out_transaction(cdc_id);
+ _prep_out_transaction(p_cdc);
return drv_len;
}
@@ -417,7 +419,7 @@ bool cdcd_xfer_cb(uint8_t rhport, uint8_t ep_addr, xfer_result_t result, uint32_
if (tud_cdc_rx_cb && tu_fifo_count(&p_cdc->rx_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 <thach@tinyusb.org>
Date: Fri, 11 Sep 2020 00:14:07 +0700
Subject: [PATCH 6/9] 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 <thach@tinyusb.org>
Date: Sat, 12 Sep 2020 08:48:49 +0700
Subject: [PATCH 7/9] 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 23e6ee2ea2e69f09b43c9f7736b95560a6e9ef7b Mon Sep 17 00:00:00 2001
From: hathach <thach@tinyusb.org>
Date: Mon, 14 Sep 2020 22:14:31 +0700
Subject: [PATCH 8/9] 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 <thach@tinyusb.org>
Date: Mon, 14 Sep 2020 22:23:59 +0700
Subject: [PATCH 9/9] 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;