diff --git a/doc/connectivity/usb/device_next/usb_device.rst b/doc/connectivity/usb/device_next/usb_device.rst index 1213c85fb51..7e7762997a0 100644 --- a/doc/connectivity/usb/device_next/usb_device.rst +++ b/doc/connectivity/usb/device_next/usb_device.rst @@ -23,31 +23,6 @@ classes and provides an API to implement custom USB functions. The new USB device support is considered experimental and will replace :ref:`usb_device_stack`. -Built-in functions -================== - -The USB device stack has built-in USB functions. Some can be used directly in -the user application through a special API, such as HID or Audio class devices, -while others use a general Zephyr RTOS driver API, such as MSC and CDC class -implementations. The *Identification string* identifies a class or function -instance (``n``) and is used as an argument to the :c:func:`usbd_register_class`. - -+-----------------------------------+-------------------------+-------------------------+ -| Class or function | User API (if any) | Identification string | -+===================================+=========================+=========================+ -| USB Audio 2 class | :ref:`uac2_device` | :samp:`uac2_{n}` | -+-----------------------------------+-------------------------+-------------------------+ -| USB CDC ACM class | :ref:`uart_api` | :samp:`cdc_acm_{n}` | -+-----------------------------------+-------------------------+-------------------------+ -| USB CDC ECM class | Ethernet device | :samp:`cdc_ecm_{n}` | -+-----------------------------------+-------------------------+-------------------------+ -| USB Mass Storage Class (MSC) | :ref:`usbd_msc_device` | :samp:`msc_{n}` | -+-----------------------------------+-------------------------+-------------------------+ -| USB Human Interface Devices (HID) | :ref:`usbd_hid_device` | :samp:`hid_{n}` | -+-----------------------------------+-------------------------+-------------------------+ -| Bluetooth HCI USB transport layer | :ref:`bt_hci_raw` | :samp:`bt_hci_{n}` | -+-----------------------------------+-------------------------+-------------------------+ - Samples ======= @@ -223,3 +198,94 @@ for this capability. :dedent: :start-after: doc device msg-cb start :end-before: doc device msg-cb end + +Built-in functions +****************** + +The USB device stack has built-in USB functions. Some can be used directly in +the user application through a special API, such as HID or Audio class devices, +while others use a general Zephyr RTOS driver API, such as MSC and CDC class +implementations. The *Identification string* identifies a class or function +instance (``n``) and is used as an argument to the :c:func:`usbd_register_class`. + ++-----------------------------------+-------------------------+-------------------------+ +| Class or function | User API (if any) | Identification string | ++===================================+=========================+=========================+ +| USB Audio 2 class | :ref:`uac2_device` | :samp:`uac2_{n}` | ++-----------------------------------+-------------------------+-------------------------+ +| USB CDC ACM class | :ref:`uart_api` | :samp:`cdc_acm_{n}` | ++-----------------------------------+-------------------------+-------------------------+ +| USB CDC ECM class | Ethernet device | :samp:`cdc_ecm_{n}` | ++-----------------------------------+-------------------------+-------------------------+ +| USB Mass Storage Class (MSC) | :ref:`usbd_msc_device` | :samp:`msc_{n}` | ++-----------------------------------+-------------------------+-------------------------+ +| USB Human Interface Devices (HID) | :ref:`usbd_hid_device` | :samp:`hid_{n}` | ++-----------------------------------+-------------------------+-------------------------+ +| Bluetooth HCI USB transport layer | :ref:`bt_hci_raw` | :samp:`bt_hci_{n}` | ++-----------------------------------+-------------------------+-------------------------+ + +CDC ACM UART +============ + +CDC ACM implements a virtual UART controller and provides Interrupt-driven UART +API and Polling UART API. + +Interrupt-driven UART API +------------------------- + +Internally the implementation uses two ringbuffers, these take over the +function of the TX/RX FIFOs (TX/RX buffers) from the :ref:`uart_interrupt_api`. + +As described in the :ref:`uart_interrupt_api`, the functions +:c:func:`uart_irq_update()`, :c:func:`uart_irq_is_pending`, +:c:func:`uart_irq_rx_ready()`, :c:func:`uart_irq_tx_ready()` +:c:func:`uart_fifo_read()`, and :c:func:`uart_fifo_fill()` +should be called from the interrupt handler, see +:c:func:`uart_irq_callback_user_data_set()`. To prevent undefined behaviour, +the implementation of these functions checks in what context they are called +and fails if it is not an interrupt handler. + +Also, as described in the UART API, :c:func:`uart_irq_is_pending` +:c:func:`uart_irq_rx_ready()`, and :c:func:`uart_irq_tx_ready()` +can only be called after :c:func:`uart_irq_update()`. + +Simplified, the interrupt handler should look something like: + +.. code-block:: c + + static void interrupt_handler(const struct device *dev, void *user_data) + { + while (uart_irq_update(dev) && uart_irq_is_pending(dev)) { + if (uart_irq_rx_ready(dev)) { + int len; + int n; + + /* ... */ + n = uart_fifo_read(dev, buffer, len); + /* ... */ + } + + if (uart_irq_tx_ready(dev)) { + int len; + int n; + + /* ... */ + n = uart_fifo_fill(dev, buffer, len); + /* ... */ + } + } + +All these functions are not directly dependent on the status of the USB device. +Filling the TX FIFO does not mean that data is being sent to the host. And +successfully reading the RX FIFO does not mean that the device is still +connected to the host. If there is space in the TX FIFO, and the TX interrupt +is enabled, :c:func:`uart_irq_tx_ready()` will succeed. If there is data in the +RX FIFO, and the RX interrupt is enabled, :c:func:`uart_irq_rx_ready()` will +succeed. Function :c:func:`uart_irq_tx_complete()` is not implemented yet. + +Polling UART API +---------------- + +The CDC ACM poll out implementation follows :ref:`uart_polling_api` and +blocks when the TX FIFO is full only if the hw-flow-control property is enabled +and called from a non-ISR context. diff --git a/doc/releases/migration-guide-4.0.rst b/doc/releases/migration-guide-4.0.rst index f8a47907859..3699a7fedcb 100644 --- a/doc/releases/migration-guide-4.0.rst +++ b/doc/releases/migration-guide-4.0.rst @@ -153,6 +153,10 @@ Sensors Serial ====== + * Users of :c:func:`uart_irq_tx_ready` now need to check for ``ret > 0`` to ensure that the FIFO + can accept data bytes, instead of ``ret == 1``. The function now returns a lower bound on the + number of bytes that can be provided to :c:func:`uart_fifo_fill` without truncation. + Regulator ========= diff --git a/drivers/serial/serial_test.c b/drivers/serial/serial_test.c index b1bb46228be..f04f5eee702 100644 --- a/drivers/serial/serial_test.c +++ b/drivers/serial/serial_test.c @@ -130,10 +130,10 @@ static void irq_tx_disable(const struct device *dev) static int irq_tx_ready(const struct device *dev) { struct serial_vnd_data *data = dev->data; - bool ready = (ring_buf_space_get(data->written) != 0); + int available = ring_buf_space_get(data->written); - LOG_DBG("tx ready: %d", ready); - return ready; + LOG_DBG("tx ready: %d", available); + return available; } static void irq_callback_set(const struct device *dev, uart_irq_callback_user_data_t cb, diff --git a/drivers/serial/uart_async_to_irq.c b/drivers/serial/uart_async_to_irq.c index 897ff50fcb5..b67dd6505aa 100644 --- a/drivers/serial/uart_async_to_irq.c +++ b/drivers/serial/uart_async_to_irq.c @@ -263,8 +263,10 @@ void z_uart_async_to_irq_irq_tx_disable(const struct device *dev) int z_uart_async_to_irq_irq_tx_ready(const struct device *dev) { struct uart_async_to_irq_data *data = get_data(dev); + bool ready = (data->flags & A2I_TX_IRQ_ENABLED) && !(data->flags & A2I_TX_BUSY); - return (data->flags & A2I_TX_IRQ_ENABLED) && !(data->flags & A2I_TX_BUSY); + /* async API handles arbitrary sizes */ + return ready ? data->tx.len : 0; } /** Interrupt driven receiver enabling function */ diff --git a/drivers/serial/uart_emul.c b/drivers/serial/uart_emul.c index f0207377737..a00f10486b1 100644 --- a/drivers/serial/uart_emul.c +++ b/drivers/serial/uart_emul.c @@ -231,7 +231,7 @@ static int uart_emul_fifo_read(const struct device *dev, uint8_t *rx_data, int s static int uart_emul_irq_tx_ready(const struct device *dev) { - bool ready = false; + int available = 0; struct uart_emul_data *data = dev->data; K_SPINLOCK(&data->tx_lock) { @@ -239,10 +239,10 @@ static int uart_emul_irq_tx_ready(const struct device *dev) K_SPINLOCK_BREAK; } - ready = ring_buf_space_get(data->tx_rb) > 0; + available = ring_buf_space_get(data->tx_rb); } - return ready; + return available; } static int uart_emul_irq_rx_ready(const struct device *dev) diff --git a/drivers/serial/uart_nrfx_uart.c b/drivers/serial/uart_nrfx_uart.c index 25dab17ea74..9fa634fe01f 100644 --- a/drivers/serial/uart_nrfx_uart.c +++ b/drivers/serial/uart_nrfx_uart.c @@ -878,10 +878,11 @@ static int uart_nrfx_irq_tx_ready_complete(const struct device *dev) * called after the TX interrupt is requested to be disabled but before * the disabling is actually performed (in the IRQ handler). */ - return nrf_uart_int_enable_check(uart0_addr, - NRF_UART_INT_MASK_TXDRDY) && - !disable_tx_irq && - event_txdrdy_check(); + bool ready = nrf_uart_int_enable_check(uart0_addr, + NRF_UART_INT_MASK_TXDRDY) && + !disable_tx_irq && + event_txdrdy_check(); + return ready ? 1 : 0; } /** Interrupt driven receiver ready function */ diff --git a/drivers/serial/uart_nrfx_uarte.c b/drivers/serial/uart_nrfx_uarte.c index 97edddbf278..00225464f37 100644 --- a/drivers/serial/uart_nrfx_uarte.c +++ b/drivers/serial/uart_nrfx_uarte.c @@ -1646,7 +1646,7 @@ static int uarte_nrfx_irq_tx_ready_complete(const struct device *dev) data->int_driven->fifo_fill_lock = 0; } - return ready; + return ready ? data->int_driven->tx_buff_size : 0; } static int uarte_nrfx_irq_rx_ready(const struct device *dev) diff --git a/drivers/usb/udc/udc_dwc2.c b/drivers/usb/udc/udc_dwc2.c index 0283b4a1b71..917910ad26d 100644 --- a/drivers/usb/udc/udc_dwc2.c +++ b/drivers/usb/udc/udc_dwc2.c @@ -904,7 +904,7 @@ static void dwc2_backup_registers(const struct device *dev) } static void dwc2_restore_essential_registers(const struct device *dev, - bool rwup) + bool rwup, bool bus_reset) { const struct udc_dwc2_config *const config = dev->config; struct usb_dwc2_reg *const base = config->base; @@ -925,6 +925,10 @@ static void dwc2_restore_essential_registers(const struct device *dev, sys_write32(backup->gusbcfg, (mem_addr_t)&base->gusbcfg); sys_write32(backup->dcfg, (mem_addr_t)&base->dcfg); + if (bus_reset) { + sys_write32(backup->dcfg, (mem_addr_t)&base->dcfg); + } + if (!rwup) { pcgcctl |= USB_DWC2_PCGCCTL_RESTOREMODE | USB_DWC2_PCGCCTL_RSTPDWNMODULE; } @@ -1026,7 +1030,8 @@ static void dwc2_enter_hibernation(const struct device *dev) LOG_DBG("Hibernated"); } -static void dwc2_exit_hibernation(const struct device *dev, bool rwup) +static void dwc2_exit_hibernation(const struct device *dev, + bool rwup, bool bus_reset) { const struct udc_dwc2_config *const config = dev->config; struct usb_dwc2_reg *const base = config->base; @@ -1066,13 +1071,15 @@ static void dwc2_exit_hibernation(const struct device *dev, bool rwup) /* Disable PMU interrupt */ sys_clear_bits(gpwrdn_reg, USB_DWC2_GPWRDN_PMUINTSEL); - dwc2_restore_essential_registers(dev, rwup); + dwc2_restore_essential_registers(dev, rwup, bus_reset); /* Note: in Remote Wakeup case 15 ms max signaling time starts now */ /* Wait for Restore Done Interrupt */ dwc2_wait_for_bit(dev, (mem_addr_t)&base->gintsts, USB_DWC2_GINTSTS_RSTRDONEINT); - sys_write32(0xFFFFFFFFUL, (mem_addr_t)&base->gintsts); + if (!bus_reset) { + sys_write32(0xFFFFFFFFUL, (mem_addr_t)&base->gintsts); + } /* Disable restore from PMU */ sys_clear_bits(gpwrdn_reg, USB_DWC2_GPWRDN_RESTORE); @@ -2027,7 +2034,7 @@ static int udc_dwc2_disable(const struct device *dev) config->irq_disable_func(dev); if (priv->hibernated) { - dwc2_exit_hibernation(dev, false); + dwc2_exit_hibernation(dev, false, true); priv->hibernated = 0; } @@ -2727,11 +2734,6 @@ static void udc_dwc2_isr_handler(const struct device *dev) /* Clear USB Suspend interrupt. */ sys_write32(USB_DWC2_GINTSTS_USBSUSP, gintsts_reg); - if (!priv->enumdone) { - /* Ignore stale suspend interrupt */ - continue; - } - /* Notify the stack */ udc_set_suspended(dev, true); udc_submit_event(dev, UDC_EVT_SUSPEND, 0); @@ -2749,7 +2751,7 @@ static void dwc2_handle_hibernation_exit(const struct device *dev, struct usb_dwc2_reg *const base = dwc2_get_base(dev); struct udc_dwc2_data *const priv = udc_get_private(dev); - dwc2_exit_hibernation(dev, rwup); + dwc2_exit_hibernation(dev, rwup, bus_reset); dwc2_restore_device_registers(dev, rwup); priv->hibernated = 0; diff --git a/include/zephyr/drivers/uart.h b/include/zephyr/drivers/uart.h index 845949ea964..7d56a78b056 100644 --- a/include/zephyr/drivers/uart.h +++ b/include/zephyr/drivers/uart.h @@ -902,9 +902,9 @@ static inline void z_impl_uart_irq_tx_disable(const struct device *dev) } /** - * @brief Check if UART TX buffer can accept a new char + * @brief Check if UART TX buffer can accept bytes * - * @details Check if UART TX buffer can accept at least one character + * @details Check if UART TX buffer can accept more bytes * for transmission (i.e. uart_fifo_fill() will succeed and return * non-zero). This function must be called in a UART interrupt * handler, or its result is undefined. Before calling this function @@ -913,9 +913,11 @@ static inline void z_impl_uart_irq_tx_disable(const struct device *dev) * * @param dev UART device instance. * - * @retval 1 If TX interrupt is enabled and at least one char can be - * written to UART. * @retval 0 If device is not ready to write a new byte. + * @retval >0 Minimum number of bytes that can be written in a single call to + * @ref uart_fifo_fill. It may be possible to write more bytes, but + * the actual number written must be checked in the return code from + * @ref uart_fifo_fill. * @retval -ENOSYS If this function is not implemented. * @retval -ENOTSUP If API is not enabled. */ diff --git a/subsys/usb/device/class/cdc_acm.c b/subsys/usb/device/class/cdc_acm.c index 130e571b0f6..5cc73f7bae9 100644 --- a/subsys/usb/device/class/cdc_acm.c +++ b/subsys/usb/device/class/cdc_acm.c @@ -622,7 +622,7 @@ static int cdc_acm_irq_tx_ready(const struct device *dev) struct cdc_acm_dev_data_t * const dev_data = dev->data; if (dev_data->tx_irq_ena && dev_data->tx_ready) { - return 1; + return ring_buf_space_get(dev_data->tx_ringbuf); } return 0; diff --git a/subsys/usb/device_next/class/usbd_cdc_acm.c b/subsys/usb/device_next/class/usbd_cdc_acm.c index eb0abb803ce..80f4a858386 100644 --- a/subsys/usb/device_next/class/usbd_cdc_acm.c +++ b/subsys/usb/device_next/class/usbd_cdc_acm.c @@ -47,7 +47,6 @@ UDC_BUF_POOL_DEFINE(cdc_acm_ep_pool, #define CDC_ACM_IRQ_RX_ENABLED 2 #define CDC_ACM_IRQ_TX_ENABLED 3 #define CDC_ACM_RX_FIFO_BUSY 4 -#define CDC_ACM_LOCK 5 static struct k_work_q cdc_acm_work_q; static K_KERNEL_STACK_DEFINE(cdc_acm_stack, @@ -105,8 +104,12 @@ struct cdc_acm_uart_data { struct k_work irq_cb_work; struct cdc_acm_uart_fifo rx_fifo; struct cdc_acm_uart_fifo tx_fifo; + /* When flow_ctrl is set, poll out is blocked when the buffer is full, + * roughly emulating flow control. + */ + bool flow_ctrl; /* USBD CDC ACM TX fifo work */ - struct k_work tx_fifo_work; + struct k_work_delayable tx_fifo_work; /* USBD CDC ACM RX fifo work */ struct k_work rx_fifo_work; atomic_t state; @@ -137,6 +140,12 @@ static ALWAYS_INLINE int cdc_acm_work_submit(struct k_work *work) return k_work_submit_to_queue(&cdc_acm_work_q, work); } +static ALWAYS_INLINE int cdc_acm_work_schedule(struct k_work_delayable *work, + k_timeout_t delay) +{ + return k_work_schedule_for_queue(&cdc_acm_work_q, work, delay); +} + static ALWAYS_INLINE bool check_wq_ctx(const struct device *dev) { return k_current_get() == k_work_queue_thread_get(&cdc_acm_work_q); @@ -269,12 +278,12 @@ static void usbd_cdc_acm_enable(struct usbd_class_data *const c_data) } if (atomic_test_bit(&data->state, CDC_ACM_IRQ_TX_ENABLED)) { - if (ring_buf_is_empty(data->tx_fifo.rb)) { + if (ring_buf_space_get(data->tx_fifo.rb)) { /* Raise TX ready interrupt */ cdc_acm_work_submit(&data->irq_cb_work); } else { /* Queue pending TX data on IN endpoint */ - cdc_acm_work_submit(&data->tx_fifo_work); + cdc_acm_work_schedule(&data->tx_fifo_work, K_NO_WAIT); } } } @@ -373,7 +382,8 @@ static void cdc_acm_update_uart_cfg(struct cdc_acm_uart_data *const data) break; }; - cfg->flow_ctrl = UART_CFG_FLOW_CTRL_NONE; + cfg->flow_ctrl = data->flow_ctrl ? UART_CFG_FLOW_CTRL_RTS_CTS : + UART_CFG_FLOW_CTRL_NONE; } static void cdc_acm_update_linestate(struct cdc_acm_uart_data *const data) @@ -516,13 +526,14 @@ static int cdc_acm_send_notification(const struct device *dev, */ static void cdc_acm_tx_fifo_handler(struct k_work *work) { + struct k_work_delayable *dwork = k_work_delayable_from_work(work); struct cdc_acm_uart_data *data; struct usbd_class_data *c_data; struct net_buf *buf; size_t len; int ret; - data = CONTAINER_OF(work, struct cdc_acm_uart_data, tx_fifo_work); + data = CONTAINER_OF(dwork, struct cdc_acm_uart_data, tx_fifo_work); c_data = data->c_data; if (!atomic_test_bit(&data->state, CDC_ACM_CLASS_ENABLED)) { @@ -535,15 +546,10 @@ static void cdc_acm_tx_fifo_handler(struct k_work *work) return; } - if (atomic_test_and_set_bit(&data->state, CDC_ACM_LOCK)) { - cdc_acm_work_submit(&data->tx_fifo_work); - return; - } - buf = cdc_acm_buf_alloc(cdc_acm_get_bulk_in(c_data)); if (buf == NULL) { - cdc_acm_work_submit(&data->tx_fifo_work); - goto tx_fifo_handler_exit; + cdc_acm_work_schedule(&data->tx_fifo_work, K_MSEC(1)); + return; } len = ring_buf_get(data->tx_fifo.rb, buf->data, buf->size); @@ -554,9 +560,6 @@ static void cdc_acm_tx_fifo_handler(struct k_work *work) LOG_ERR("Failed to enqueue"); net_buf_unref(buf); } - -tx_fifo_handler_exit: - atomic_clear_bit(&data->state, CDC_ACM_LOCK); } /* @@ -600,6 +603,9 @@ static void cdc_acm_rx_fifo_handler(struct k_work *work) return; } + /* Shrink the buffer size if operating on a full speed bus */ + buf->size = MIN(cdc_acm_get_bulk_mps(c_data), buf->size); + ret = usbd_ep_enqueue(c_data, buf); if (ret) { LOG_ERR("Failed to enqueue net_buf for 0x%02x", ep); @@ -613,7 +619,7 @@ static void cdc_acm_irq_tx_enable(const struct device *dev) atomic_set_bit(&data->state, CDC_ACM_IRQ_TX_ENABLED); - if (ring_buf_is_empty(data->tx_fifo.rb)) { + if (ring_buf_space_get(data->tx_fifo.rb)) { LOG_INF("tx_en: trigger irq_cb_work"); cdc_acm_work_submit(&data->irq_cb_work); } @@ -707,8 +713,8 @@ static int cdc_acm_irq_tx_ready(const struct device *dev) struct cdc_acm_uart_data *const data = dev->data; if (check_wq_ctx(dev)) { - if (ring_buf_space_get(data->tx_fifo.rb)) { - return 1; + if (data->tx_fifo.irq) { + return ring_buf_space_get(data->tx_fifo.rb); } } else { LOG_WRN("Invoked by inappropriate context"); @@ -723,7 +729,7 @@ static int cdc_acm_irq_rx_ready(const struct device *dev) struct cdc_acm_uart_data *const data = dev->data; if (check_wq_ctx(dev)) { - if (!ring_buf_is_empty(data->rx_fifo.rb)) { + if (data->rx_fifo.irq) { return 1; } } else { @@ -769,7 +775,7 @@ static int cdc_acm_irq_update(const struct device *dev) } if (atomic_test_bit(&data->state, CDC_ACM_IRQ_TX_ENABLED) && - ring_buf_is_empty(data->tx_fifo.rb)) { + ring_buf_space_get(data->tx_fifo.rb)) { data->tx_fifo.irq = true; } else { data->tx_fifo.irq = false; @@ -803,12 +809,6 @@ static void cdc_acm_irq_cb_handler(struct k_work *work) return; } - if (atomic_test_and_set_bit(&data->state, CDC_ACM_LOCK)) { - LOG_ERR("Polling is in progress"); - cdc_acm_work_submit(&data->irq_cb_work); - return; - } - data->tx_fifo.altered = false; data->rx_fifo.altered = false; data->rx_fifo.irq = false; @@ -826,7 +826,7 @@ static void cdc_acm_irq_cb_handler(struct k_work *work) if (data->tx_fifo.altered) { LOG_DBG("tx fifo altered, submit work"); - cdc_acm_work_submit(&data->tx_fifo_work); + cdc_acm_work_schedule(&data->tx_fifo_work, K_NO_WAIT); } if (atomic_test_bit(&data->state, CDC_ACM_IRQ_RX_ENABLED) && @@ -836,12 +836,10 @@ static void cdc_acm_irq_cb_handler(struct k_work *work) } if (atomic_test_bit(&data->state, CDC_ACM_IRQ_TX_ENABLED) && - ring_buf_is_empty(data->tx_fifo.rb)) { + ring_buf_space_get(data->tx_fifo.rb)) { LOG_DBG("tx irq pending, submit irq_cb_work"); cdc_acm_work_submit(&data->irq_cb_work); } - - atomic_clear_bit(&data->state, CDC_ACM_LOCK); } static void cdc_acm_irq_callback_set(const struct device *dev, @@ -860,13 +858,8 @@ static int cdc_acm_poll_in(const struct device *dev, unsigned char *const c) uint32_t len; int ret = -1; - if (atomic_test_and_set_bit(&data->state, CDC_ACM_LOCK)) { - LOG_ERR("IRQ callback is used"); - return -1; - } - if (ring_buf_is_empty(data->rx_fifo.rb)) { - goto poll_in_exit; + return ret; } len = ring_buf_get(data->rx_fifo.rb, c, 1); @@ -875,35 +868,37 @@ static int cdc_acm_poll_in(const struct device *dev, unsigned char *const c) ret = 0; } -poll_in_exit: - atomic_clear_bit(&data->state, CDC_ACM_LOCK); - return ret; } static void cdc_acm_poll_out(const struct device *dev, const unsigned char c) { struct cdc_acm_uart_data *const data = dev->data; + unsigned int lock; + uint32_t wrote; - if (atomic_test_and_set_bit(&data->state, CDC_ACM_LOCK)) { - LOG_ERR("IRQ callback is used"); - return; - } + while (true) { + lock = irq_lock(); + wrote = ring_buf_put(data->tx_fifo.rb, &c, 1); + irq_unlock(lock); - if (ring_buf_put(data->tx_fifo.rb, &c, 1)) { - goto poll_out_exit; - } + if (wrote == 1) { + break; + } - LOG_DBG("Ring buffer full, drain buffer"); - if (!ring_buf_get(data->tx_fifo.rb, NULL, 1) || - !ring_buf_put(data->tx_fifo.rb, &c, 1)) { - LOG_ERR("Failed to drain buffer"); - __ASSERT_NO_MSG(false); + if (k_is_in_isr() || !data->flow_ctrl) { + LOG_WRN_ONCE("Ring buffer full, discard data"); + break; + } + + k_msleep(1); } -poll_out_exit: - atomic_clear_bit(&data->state, CDC_ACM_LOCK); - cdc_acm_work_submit(&data->tx_fifo_work); + /* Schedule with minimal timeout to make it possible to send more than + * one byte per USB transfer. The latency increase is negligible while + * the increased throughput and reduced CPU usage is easily observable. + */ + cdc_acm_work_schedule(&data->tx_fifo_work, K_MSEC(1)); } #ifdef CONFIG_UART_LINE_CTRL @@ -976,17 +971,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 *const 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_uart_data *const data = dev->data; + + switch (cfg->flow_ctrl) { + case UART_CFG_FLOW_CTRL_NONE: + data->flow_ctrl = false; + break; + case UART_CFG_FLOW_CTRL_RTS_CTS: + data->flow_ctrl = true; + break; + default: + return -ENOTSUP; + } return 0; } @@ -1021,7 +1017,7 @@ static int usbd_cdc_acm_preinit(const struct device *dev) k_thread_name_set(&cdc_acm_work_q.thread, "cdc_acm_work_q"); - k_work_init(&data->tx_fifo_work, cdc_acm_tx_fifo_handler); + k_work_init_delayable(&data->tx_fifo_work, cdc_acm_tx_fifo_handler); k_work_init(&data->rx_fifo_work, cdc_acm_rx_fifo_handler); k_work_init(&data->irq_cb_work, cdc_acm_irq_cb_handler); @@ -1240,6 +1236,7 @@ const static struct usb_desc_header *cdc_acm_hs_desc_##n[] = { \ .c_data = &cdc_acm_##n, \ .rx_fifo.rb = &cdc_acm_rb_rx_##n, \ .tx_fifo.rb = &cdc_acm_rb_tx_##n, \ + .flow_ctrl = DT_INST_PROP(n, hw_flow_control), \ .notif_sem = Z_SEM_INITIALIZER(uart_data_##n.notif_sem, 0, 1), \ .desc = &cdc_acm_desc_##n, \ .fs_desc = cdc_acm_fs_desc_##n, \ diff --git a/subsys/usb/device_next/usbd_core.c b/subsys/usb/device_next/usbd_core.c index e6f1cf1faee..2502357bf8c 100644 --- a/subsys/usb/device_next/usbd_core.c +++ b/subsys/usb/device_next/usbd_core.c @@ -127,6 +127,8 @@ static int event_handler_bus_reset(struct usbd_context *const uds_ctx) uds_ctx->ch9_data.state = USBD_STATE_DEFAULT; + uds_ctx->status.rwup = false; + return 0; }