adafruit-mirror/zephyr
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions 10

usb: device: cdc_acm: block in uart_poll_out() routine

Browse source 
According to the UART API documentation, implementation must block when
the transceiver is full. For CDC ACM UART, TX ringbuffer can be
considered as transceiver buffer/FIFO. Blocking when the USB subsystem
is not ready is considered highly undesirable behavior. Blocking may
also be undesirable when CDC ACM UART is used as a logging backend.

Change the behavior of CDC ACM poll out to:
 - Block if the TX ring buffer is full, hw_flow_control property is
   enabled, and called from a non-ISR context.
 - Do not block if the USB subsystem is not ready, poll out
   implementation is called from an ISR context, or hw_flow_control
   property is disabled.

Signed-off-by: Johann Fischer <johann.fischer@nordicsemi.no>
This commit is contained in:
Johann Fischer 2023-09-30 00:12:48 +02:00 committed by Carles Cufí
parent 9112c7e0ff
commit c152e0980c
2 changed files with 39 additions and 26 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

3
doc/connectivity/usb/device/usb_device.rst
View file

@ -82,6 +82,9 @@ But there are two important differences in behavior to a real UART controller:
initialized and started, until then any data is discarded initialized and started, until then any data is discarded
* If device is connected to the host, it still needs an application * If device is connected to the host, it still needs an application
on the host side which requests the data on the host side which requests the data
* The CDC ACM poll out implementation follows the API and blocks when the TX
ring buffer is full only if the hw-flow-control property is enabled and
called from a non-ISR context.
The devicetree compatible property for CDC ACM UART is The devicetree compatible property for CDC ACM UART is
:dtcompatible:`zephyr,cdc-acm-uart`. :dtcompatible:`zephyr,cdc-acm-uart`.

62
subsys/usb/device/class/cdc_acm.c
View file

@ -126,6 +126,10 @@ struct cdc_acm_dev_data_t {
bool suspended; bool suspended;
/* CDC ACM paused flag */ /* CDC ACM paused flag */
bool rx_paused; bool rx_paused;
/* When flow_ctrl is set, poll out is blocked when the buffer is full,
* roughly emulating flow control.
*/
bool flow_ctrl;
struct usb_dev_data common; struct usb_dev_data common;
}; };
@ -899,17 +903,18 @@ static int cdc_acm_line_ctrl_get(const struct device *dev,
static int cdc_acm_configure(const struct device *dev, static int cdc_acm_configure(const struct device *dev,
const struct uart_config *cfg) const struct uart_config *cfg)
{ {
ARG_UNUSED(dev); struct cdc_acm_dev_data_t * const dev_data = dev->data;
ARG_UNUSED(cfg);
/* switch (cfg->flow_ctrl) {
* We cannot implement configure API because there is case UART_CFG_FLOW_CTRL_NONE:
* no notification of configuration changes provided dev_data->flow_ctrl = false;
* for the Abstract Control Model and the UART controller break;
* is only emulated. case UART_CFG_FLOW_CTRL_RTS_CTS:
* However, it allows us to use CDC ACM UART together with dev_data->flow_ctrl = true;
* subsystems like Modbus which require configure API for break;
* real controllers. default:
*/ return -ENOTSUP;
}
return 0; return 0;
} }
@ -969,8 +974,8 @@ static int cdc_acm_config_get(const struct device *dev,
break; break;
}; };
/* USB CDC has no notion of flow control */ cfg->flow_ctrl = dev_data->flow_ctrl ? UART_CFG_FLOW_CTRL_RTS_CTS :
cfg->flow_ctrl = UART_CFG_FLOW_CTRL_NONE; UART_CFG_FLOW_CTRL_NONE;
return 0; return 0;
} }
@ -991,13 +996,17 @@ static int cdc_acm_poll_in(const struct device *dev, unsigned char *c)
/* /*
* @brief Output a character in polled mode. * @brief Output a character in polled mode.
* *
* The poll function looks similar to cdc_acm_fifo_fill() and * According to the UART API, the implementation of this routine should block
* tries to do the best to mimic behavior of a hardware UART controller * if the transmitter is full. But blocking when the USB subsystem is not ready
* without flow control. * is considered highly undesirable behavior. Blocking may also be undesirable
* This function does not block, if the USB subsystem * when CDC ACM UART is used as a logging backend.
* is not ready, no data is transferred to the buffer, that is, c is dropped. *
* If the USB subsystem is ready and the buffer is full, the first character * The behavior of CDC ACM poll out is:
* from the tx_ringbuf is removed to make room for the new character. * - Block if the TX ring buffer is full, hw_flow_control property is enabled,
* and called from a non-ISR context.
* - Do not block if the USB subsystem is not ready, poll out implementation
* is called from an ISR context, or hw_flow_control property is disabled.
*
*/ */
static void cdc_acm_poll_out(const struct device *dev, unsigned char c) static void cdc_acm_poll_out(const struct device *dev, unsigned char c)
{ {
@ -1010,13 +1019,13 @@ static void cdc_acm_poll_out(const struct device *dev, unsigned char c)
dev_data->tx_ready = false; dev_data->tx_ready = false;
if (!ring_buf_put(dev_data->tx_ringbuf, &c, 1)) { while (!ring_buf_put(dev_data->tx_ringbuf, &c, 1)) {
LOG_INF("Ring buffer full, drain buffer"); if (k_is_in_isr() || !dev_data->flow_ctrl) {
if (!ring_buf_get(dev_data->tx_ringbuf, NULL, 1) || LOG_INF("Ring buffer full, discard %c", c);
!ring_buf_put(dev_data->tx_ringbuf, &c, 1)) { break;
LOG_ERR("Failed to drain buffer");
return;
} }
k_msleep(1);
} }
/* Schedule with minimal timeout to make it possible to send more than /* Schedule with minimal timeout to make it possible to send more than
@ -1183,6 +1192,7 @@ static const struct uart_driver_api cdc_acm_driver_api = {
.line_coding = CDC_ACM_DEFAULT_BAUDRATE, \ .line_coding = CDC_ACM_DEFAULT_BAUDRATE, \
.rx_ringbuf = &cdc_acm_rx_rb_##x, \ .rx_ringbuf = &cdc_acm_rx_rb_##x, \
.tx_ringbuf = &cdc_acm_tx_rb_##x, \ .tx_ringbuf = &cdc_acm_tx_rb_##x, \
.flow_ctrl = DT_INST_PROP_OR(x, hw_flow_control, false),\
}; };
#define DT_DRV_COMPAT zephyr_cdc_acm_uart #define DT_DRV_COMPAT zephyr_cdc_acm_uart