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);