patrick/zephyr
1
0
Fork 0
Code Issues Pull requests Actions 8 Packages Projects Releases Wiki Activity

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
* If device is connected to the host, it still needs an application
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
: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;
/* CDC ACM paused flag */
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;
};
@ -899,17 +903,18 @@ static int cdc_acm_line_ctrl_get(const struct device *dev,
static int cdc_acm_configure(const struct device *dev,
const struct uart_config *cfg)
{
ARG_UNUSED(dev);
ARG_UNUSED(cfg);
/*
* We cannot implement configure API because there is
* no notification of configuration changes provided
* for the Abstract Control Model and the UART controller
* is only emulated.
* However, it allows us to use CDC ACM UART together with
* subsystems like Modbus which require configure API for
* real controllers.
*/
struct cdc_acm_dev_data_t * const dev_data = dev->data;
switch (cfg->flow_ctrl) {
case UART_CFG_FLOW_CTRL_NONE:
dev_data->flow_ctrl = false;
break;
case UART_CFG_FLOW_CTRL_RTS_CTS:
dev_data->flow_ctrl = true;
break;
default:
return -ENOTSUP;
}
return 0;
}
@ -969,8 +974,8 @@ static int cdc_acm_config_get(const struct device *dev,
break;
};
/* USB CDC has no notion of flow control */
cfg->flow_ctrl = UART_CFG_FLOW_CTRL_NONE;
cfg->flow_ctrl = dev_data->flow_ctrl ? UART_CFG_FLOW_CTRL_RTS_CTS :
UART_CFG_FLOW_CTRL_NONE;
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.
*
* The poll function looks similar to cdc_acm_fifo_fill() and
* tries to do the best to mimic behavior of a hardware UART controller
* without flow control.
* This function does not block, if the USB subsystem
* 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
* from the tx_ringbuf is removed to make room for the new character.
* According to the UART API, the implementation of this routine should block
* if the transmitter is full. But 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.
*
* The behavior of CDC ACM poll out is:
* - 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)
{
@ -1010,13 +1019,13 @@ static void cdc_acm_poll_out(const struct device *dev, unsigned char c)
dev_data->tx_ready = false;
if (!ring_buf_put(dev_data->tx_ringbuf, &c, 1)) {
LOG_INF("Ring buffer full, drain buffer");
if (!ring_buf_get(dev_data->tx_ringbuf, NULL, 1) ||
!ring_buf_put(dev_data->tx_ringbuf, &c, 1)) {
LOG_ERR("Failed to drain buffer");
return;
while (!ring_buf_put(dev_data->tx_ringbuf, &c, 1)) {
if (k_is_in_isr() || !dev_data->flow_ctrl) {
LOG_INF("Ring buffer full, discard %c", c);
break;
}
k_msleep(1);
}
/* 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, \
.rx_ringbuf = &cdc_acm_rx_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