From 909891325ad6867fc6d9596bfe0e27d6033c3d48 Mon Sep 17 00:00:00 2001 From: Scott Shawcroft Date: Thu, 21 Mar 2019 14:52:56 -0700 Subject: [PATCH 1/3] Fix slow CDC OUT by NAKing This NAKs CDC OUT packets when the ring buffer doesn't have enough space for it. This makes CDC OUT reliable rather than allowing overwriting into the ring buffer. --- src/class/cdc/cdc_device.c | 39 ++++++++++++++++++++++++++++++++------ 1 file changed, 33 insertions(+), 6 deletions(-) diff --git a/src/class/cdc/cdc_device.c b/src/class/cdc/cdc_device.c index 0cfb2377b..43af2a502 100644 --- a/src/class/cdc/cdc_device.c +++ b/src/class/cdc/cdc_device.c @@ -87,6 +87,24 @@ typedef struct //--------------------------------------------------------------------+ CFG_TUSB_MEM_SECTION static cdcd_interface_t _cdcd_itf[CFG_TUD_CDC] = { { 0 } }; +bool pending_read_from_host; +static void _prep_next_transaction(void) { + uint8_t const itf = 0; + cdcd_interface_t* p_cdc = &_cdcd_itf[itf]; + + // skip if previous transfer not complete + if (pending_read_from_host) { + return; + } + + // 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 >= CFG_TUD_CDC_EPSIZE) { + dcd_edpt_xfer(TUD_OPT_RHPORT, p_cdc->ep_out, p_cdc->epout_buf, CFG_TUD_CDC_EPSIZE); + pending_read_from_host = true; + } +} + //--------------------------------------------------------------------+ // APPLICATION API //--------------------------------------------------------------------+ @@ -123,12 +141,19 @@ uint32_t tud_cdc_n_available(uint8_t itf) char tud_cdc_n_read_char(uint8_t itf) { char ch; - return tu_fifo_read(&_cdcd_itf[itf].rx_ff, &ch) ? ch : (-1); + if (!tu_fifo_read(&_cdcd_itf[itf].rx_ff, &ch)) { + ch = -1; + } + + _prep_next_transaction(); + return ch; } uint32_t tud_cdc_n_read(uint8_t itf, void* buffer, uint32_t bufsize) { - return tu_fifo_read_n(&_cdcd_itf[itf].rx_ff, buffer, bufsize); + uint32_t num_read = tu_fifo_read_n(&_cdcd_itf[itf].rx_ff, buffer, bufsize); + _prep_next_transaction(); + return num_read; } char tud_cdc_n_peek(uint8_t itf, int pos) @@ -140,6 +165,7 @@ char tud_cdc_n_peek(uint8_t itf, int pos) void tud_cdc_n_read_flush (uint8_t itf) { tu_fifo_clear(&_cdcd_itf[itf].rx_ff); + _prep_next_transaction(); } //--------------------------------------------------------------------+ @@ -207,7 +233,7 @@ void cdcd_init(void) p_cdc->line_coding.data_bits = 8; // config fifo - tu_fifo_config(&p_cdc->rx_ff, p_cdc->rx_ff_buf, CFG_TUD_CDC_RX_BUFSIZE, 1, true); + tu_fifo_config(&p_cdc->rx_ff, p_cdc->rx_ff_buf, CFG_TUD_CDC_RX_BUFSIZE, 1, false); tu_fifo_config(&p_cdc->tx_ff, p_cdc->tx_ff_buf, CFG_TUD_CDC_TX_BUFSIZE, 1, false); #if CFG_FIFO_MUTEX @@ -290,7 +316,8 @@ bool cdcd_open(uint8_t rhport, tusb_desc_interface_t const * itf_desc, uint16_t } // Prepare for incoming data - TU_ASSERT( dcd_edpt_xfer(rhport, p_cdc->ep_out, p_cdc->epout_buf, CFG_TUD_CDC_EPSIZE) ); + pending_read_from_host = false; + _prep_next_transaction(); return true; } @@ -383,8 +410,8 @@ bool cdcd_xfer_cb(uint8_t rhport, uint8_t ep_addr, xfer_result_t result, uint32_ // invoke receive callback (if there is still data) if (tud_cdc_rx_cb && tu_fifo_count(&p_cdc->rx_ff) ) tud_cdc_rx_cb(itf); - // prepare for incoming data - TU_ASSERT( dcd_edpt_xfer(rhport, p_cdc->ep_out, p_cdc->epout_buf, CFG_TUD_CDC_EPSIZE) ); + pending_read_from_host = false; + _prep_next_transaction(); } // nothing to do with in and notif endpoint From aaf5714268fd8f6e3e0f0e337634338dd79344ec Mon Sep 17 00:00:00 2001 From: hathach Date: Mon, 25 Mar 2019 11:38:16 +0700 Subject: [PATCH 2/3] follow up to pr #46 --- src/class/cdc/cdc_device.c | 52 ++++++++++++++++---------------------- 1 file changed, 22 insertions(+), 30 deletions(-) diff --git a/src/class/cdc/cdc_device.c b/src/class/cdc/cdc_device.c index 0f18144ab..9a4deb3cc 100644 --- a/src/class/cdc/cdc_device.c +++ b/src/class/cdc/cdc_device.c @@ -75,22 +75,19 @@ typedef struct //--------------------------------------------------------------------+ CFG_TUSB_MEM_SECTION static cdcd_interface_t _cdcd_itf[CFG_TUD_CDC] = { { 0 } }; -bool pending_read_from_host; -static void _prep_next_transaction(void) { - uint8_t const itf = 0; - cdcd_interface_t* p_cdc = &_cdcd_itf[itf]; +static void _prep_out_transaction (uint8_t itf) +{ + cdcd_interface_t* p_cdc = &_cdcd_itf[itf]; - // skip if previous transfer not complete - if (pending_read_from_host) { - return; - } + // skip if previous transfer not complete + if ( dcd_edpt_busy(TUD_OPT_RHPORT, p_cdc->ep_out) ) return; - // 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 >= CFG_TUD_CDC_EPSIZE) { - dcd_edpt_xfer(TUD_OPT_RHPORT, p_cdc->ep_out, p_cdc->epout_buf, CFG_TUD_CDC_EPSIZE); - pending_read_from_host = true; - } + // 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 >= CFG_TUD_CDC_EPSIZE ) + { + dcd_edpt_xfer(TUD_OPT_RHPORT, p_cdc->ep_out, p_cdc->epout_buf, CFG_TUD_CDC_EPSIZE); + } } //--------------------------------------------------------------------+ @@ -129,18 +126,13 @@ uint32_t tud_cdc_n_available(uint8_t itf) char tud_cdc_n_read_char(uint8_t itf) { char ch; - if (!tu_fifo_read(&_cdcd_itf[itf].rx_ff, &ch)) { - ch = -1; - } - - _prep_next_transaction(); - return ch; + return tud_cdc_n_read(itf, &ch, 1) ? ch : (-1); } 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_next_transaction(); + _prep_out_transaction(itf); return num_read; } @@ -153,7 +145,7 @@ char tud_cdc_n_peek(uint8_t itf, int pos) void tud_cdc_n_read_flush (uint8_t itf) { tu_fifo_clear(&_cdcd_itf[itf].rx_ff); - _prep_next_transaction(); + _prep_out_transaction(itf); } //--------------------------------------------------------------------+ @@ -254,11 +246,12 @@ bool cdcd_open(uint8_t rhport, tusb_desc_interface_t const * itf_desc, uint16_t // Find available interface cdcd_interface_t * p_cdc = NULL; - for(uint8_t i=0; irx_ff) ) tud_cdc_rx_cb(itf); - pending_read_from_host = false; - _prep_next_transaction(); + // prepare for OUT transaction + _prep_out_transaction(itf); } - // nothing to do with in and notif endpoint + // nothing to do with in and notif endpoint for now return true; } From 89b9ee2f52a970ffa162ccd1d9244fe5ead90ed8 Mon Sep 17 00:00:00 2001 From: hathach Date: Wed, 27 Mar 2019 00:39:14 +0700 Subject: [PATCH 3/3] revert to use pending_read_from_host (temp) since --- src/class/cdc/cdc_device.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/class/cdc/cdc_device.c b/src/class/cdc/cdc_device.c index 9a4deb3cc..48e94a6e1 100644 --- a/src/class/cdc/cdc_device.c +++ b/src/class/cdc/cdc_device.c @@ -75,18 +75,23 @@ typedef struct //--------------------------------------------------------------------+ CFG_TUSB_MEM_SECTION static cdcd_interface_t _cdcd_itf[CFG_TUD_CDC] = { { 0 } }; +// TODO will be replaced by dcd_edpt_busy() +bool pending_read_from_host; static void _prep_out_transaction (uint8_t itf) { cdcd_interface_t* p_cdc = &_cdcd_itf[itf]; // skip if previous transfer not complete - if ( dcd_edpt_busy(TUD_OPT_RHPORT, p_cdc->ep_out) ) return; + // dcd_edpt_busy() doesn't work, probably transfer is complete but not properly handled by the stack +// if ( dcd_edpt_busy(TUD_OPT_RHPORT, p_cdc->ep_out) ) return; + if (pending_read_from_host) return; // 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 >= CFG_TUD_CDC_EPSIZE ) { dcd_edpt_xfer(TUD_OPT_RHPORT, p_cdc->ep_out, p_cdc->epout_buf, CFG_TUD_CDC_EPSIZE); + pending_read_from_host = true; } } @@ -297,6 +302,7 @@ bool cdcd_open(uint8_t rhport, tusb_desc_interface_t const * itf_desc, uint16_t } // Prepare for incoming data + pending_read_from_host = false; _prep_out_transaction(cdc_id); return true; @@ -367,6 +373,7 @@ bool cdcd_control_request(uint8_t rhport, tusb_control_request_t const * request bool cdcd_xfer_cb(uint8_t rhport, uint8_t ep_addr, xfer_result_t result, uint32_t xferred_bytes) { + (void) rhport; (void) result; // TODO Support multiple interfaces @@ -391,6 +398,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 + pending_read_from_host = false; _prep_out_transaction(itf); }