mirror of
https://github.com/hathach/tinyusb.git
synced 2025-03-19 19:21:05 +00:00
Merge pull request #508 from hathach/fix-edpt-race
Fix edpt xfer race condition
This commit is contained in:
commit
745c15d5b5
@ -73,18 +73,29 @@ 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];
|
||||
|
||||
// skip if previous transfer not complete
|
||||
if ( usbd_edpt_busy(TUD_OPT_RHPORT, p_cdc->ep_out) ) return;
|
||||
uint8_t const rhport = TUD_OPT_RHPORT;
|
||||
uint16_t available = tu_fifo_remaining(&p_cdc->rx_ff);
|
||||
|
||||
// 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) )
|
||||
// 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);
|
||||
}
|
||||
}
|
||||
|
||||
@ -123,8 +134,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;
|
||||
}
|
||||
|
||||
@ -135,8 +147,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);
|
||||
}
|
||||
|
||||
//--------------------------------------------------------------------+
|
||||
@ -144,11 +157,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);
|
||||
}
|
||||
@ -161,17 +175,28 @@ 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
|
||||
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));
|
||||
|
||||
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 +219,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;
|
||||
@ -233,8 +258,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 )
|
||||
{
|
||||
@ -283,7 +307,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;
|
||||
}
|
||||
@ -408,7 +432,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.
|
||||
@ -421,12 +445,15 @@ 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);
|
||||
if ( usbd_edpt_claim(rhport, p_cdc->ep_in) )
|
||||
{
|
||||
usbd_edpt_xfer(rhport, p_cdc->ep_in, NULL, 0);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
@ -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);
|
||||
|
||||
|
@ -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);
|
||||
|
@ -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);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
@ -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,59 @@ 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
|
||||
// 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 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;
|
||||
}
|
||||
|
||||
#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 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;
|
||||
}
|
||||
|
||||
#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 +1152,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 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
|
||||
_usbd_dev.ep_status[epnum][dir].busy = true;
|
||||
@ -1093,7 +1165,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;
|
||||
|
@ -70,6 +70,13 @@ 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.
|
||||
// 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
|
||||
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);
|
||||
|
||||
|
Loading…
x
Reference in New Issue
Block a user