Skip to content

USB: cdc_acm: use ringbuf library and fixes #14677

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Mar 25, 2019
Merged

USB: cdc_acm: use ringbuf library and fixes #14677

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
cd04e84
usb: cdc_acm: Make parameters boolean
finikorg Mar 19, 2019
da6c374
usb: cdc_acm: Use ringbuf for RX path
finikorg Mar 19, 2019
f290dd1
samples: usb: cdc_acm_composite: Fix missing irq handling
finikorg Mar 19, 2019
9b3e03f
usb: cdc_acm: Make ring buffer size configurable
finikorg Mar 20, 2019
f1473f4
samples: usb: console: Select console device name
finikorg Mar 20, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 5 additions & 7 deletions samples/subsys/usb/cdc_acm_composite/src/main.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,16 +30,14 @@ static void interrupt_handler(void *user_data)
const struct serial_data *dev_data = user_data;
struct device *dev = dev_data->dev;

uart_irq_update(dev);

if (uart_irq_tx_ready(dev)) {
/* TODO */
}

if (uart_irq_rx_ready(dev)) {
while (uart_irq_update(dev) && uart_irq_is_pending(dev)) {
struct device *peer = dev_data->peer;
u8_t byte;

if (!uart_irq_rx_ready(dev)) {
break;
}

uart_fifo_read(dev, &byte, sizeof(byte));
uart_poll_out(peer, byte);

Expand Down
3 changes: 3 additions & 0 deletions samples/subsys/usb/console/prj.conf
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
CONFIG_GPIO=y

CONFIG_USB=y
CONFIG_USB_DEVICE_STACK=y
CONFIG_USB_DEVICE_PRODUCT="Zephyr USB console sample"
CONFIG_USB_UART_CONSOLE=y

CONFIG_UART_INTERRUPT_DRIVEN=y
CONFIG_UART_LINE_CTRL=y
CONFIG_UART_CONSOLE_ON_DEV_NAME="CDC_ACM_0"
7 changes: 7 additions & 0 deletions subsys/usb/class/Kconfig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,18 @@ config USB_CDC_ACM
bool "USB CDC ACM Device Class Driver"
select SERIAL_HAS_DRIVER
select SERIAL_SUPPORT_INTERRUPT
select RING_BUFFER
help
USB CDC ACM device class driver

if USB_CDC_ACM

config USB_CDC_ACM_RINGBUF_SIZE
int "USB CDC ACM ring buffer size"
default 1024
help
USB CDC ACM ring buffer size

config CDC_ACM_PORT_NAME_0
string "CDC ACM class device driver port name"
default "CDC_ACM_0"
Expand Down
97 changes: 34 additions & 63 deletions subsys/usb/class/cdc_acm.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
#include <init.h>
#include <uart.h>
#include <string.h>
#include <ring_buffer.h>
#include <misc/byteorder.h>
#include <usb/class/usb_cdc.h>
#include <usb/usb_device.h>
Expand Down Expand Up @@ -185,13 +186,12 @@ struct cdc_acm_dev_data_t {
void *cb_data;
struct k_work cb_work;
/* Tx ready status. Signals when */
u8_t tx_ready;
u8_t rx_ready; /* Rx ready status */
u8_t tx_irq_ena; /* Tx interrupt enable status */
u8_t rx_irq_ena; /* Rx interrupt enable status */
bool tx_ready;
bool rx_ready; /* Rx ready status */
bool tx_irq_ena; /* Tx interrupt enable status */
bool rx_irq_ena; /* Rx interrupt enable status */
u8_t rx_buf[CDC_ACM_BUFFER_SIZE];/* Internal Rx buffer */
u32_t rx_buf_head; /* Head of the internal Rx buffer */
u32_t rx_buf_tail; /* Tail of the internal Rx buffer */
struct ring_buf *rx_ringbuf;
/* Interface data buffer */
#ifndef CONFIG_USB_COMPOSITE_DEVICE
u8_t interface_data[CDC_CLASS_REQ_MAX_DATA_SIZE];
Expand Down Expand Up @@ -294,7 +294,8 @@ static void cdc_acm_bulk_in(u8_t ep, enum usb_dc_ep_cb_status_code ep_status)

dev_data = CONTAINER_OF(common, struct cdc_acm_dev_data_t, common);

dev_data->tx_ready = 1U;
dev_data->tx_ready = true;

k_sem_give(&poll_wait_sem);
/* Call callback only if tx irq ena */
if (dev_data->cb && dev_data->tx_irq_ena) {
Expand All @@ -313,9 +314,8 @@ static void cdc_acm_bulk_in(u8_t ep, enum usb_dc_ep_cb_status_code ep_status)
static void cdc_acm_bulk_out(u8_t ep, enum usb_dc_ep_cb_status_code ep_status)
{
struct cdc_acm_dev_data_t *dev_data;
u32_t bytes_to_read, i, j, buf_head;
struct usb_dev_data *common;
u8_t tmp_buf[4];
u32_t bytes_to_read, read;

ARG_UNUSED(ep_status);

Expand All @@ -328,36 +328,18 @@ static void cdc_acm_bulk_out(u8_t ep, enum usb_dc_ep_cb_status_code ep_status)
dev_data = CONTAINER_OF(common, struct cdc_acm_dev_data_t, common);

/* Check how many bytes were received */
usb_read(ep, NULL, 0, &bytes_to_read);
usb_read(ep, NULL, 0, &read);

buf_head = dev_data->rx_buf_head;
bytes_to_read = MIN(read, sizeof(dev_data->rx_buf));

/*
* Quark SE USB controller is always storing data
* in the FIFOs per 32-bit words.
*/
for (i = 0U; i < bytes_to_read; i += 4) {
usb_read(ep, tmp_buf, 4, NULL);

for (j = 0U; j < 4; j++) {
if (i + j == bytes_to_read) {
/* We read all the data */
break;
}

if (((buf_head + 1) % CDC_ACM_BUFFER_SIZE) ==
dev_data->rx_buf_tail) {
/* FIFO full, discard data */
LOG_ERR("CDC buffer full!");
} else {
dev_data->rx_buf[buf_head] = tmp_buf[j];
buf_head = (buf_head + 1) % CDC_ACM_BUFFER_SIZE;
}
}
usb_read(ep, dev_data->rx_buf, bytes_to_read, &read);

if (!ring_buf_put(dev_data->rx_ringbuf, dev_data->rx_buf, read)) {
LOG_ERR("Ring buffer full");
}

dev_data->rx_buf_head = buf_head;
dev_data->rx_ready = 1U;
dev_data->rx_ready = true;

/* Call callback only if rx irq ena */
if (dev_data->cb && dev_data->rx_irq_ena) {
k_work_submit(&dev_data->cb_work);
Expand Down Expand Up @@ -413,7 +395,7 @@ static void cdc_acm_do_cb(struct cdc_acm_dev_data_t *dev_data,
LOG_DBG("USB device connected");
break;
case USB_DC_CONFIGURED:
dev_data->tx_ready = 1;
dev_data->tx_ready = true;
LOG_DBG("USB device configured");
break;
case USB_DC_DISCONNECTED:
Expand Down Expand Up @@ -595,7 +577,7 @@ static int cdc_acm_fifo_fill(struct device *dev,
return 0;
}

dev_data->tx_ready = 0U;
dev_data->tx_ready = false;

/* FIXME: On Quark SE Family processor, restrict writing more than
* 4 bytes into TX USB Endpoint. When more than 4 bytes are written,
Expand Down Expand Up @@ -625,34 +607,18 @@ static int cdc_acm_fifo_fill(struct device *dev,
*
* @return Number of bytes read.
*/
static int cdc_acm_fifo_read(struct device *dev, u8_t *rx_data,
const int size)
static int cdc_acm_fifo_read(struct device *dev, u8_t *rx_data, const int size)
{
u32_t avail_data, bytes_read, i;
struct cdc_acm_dev_data_t * const dev_data = DEV_DATA(dev);
u32_t len;

avail_data = (CDC_ACM_BUFFER_SIZE + dev_data->rx_buf_head -
dev_data->rx_buf_tail) % CDC_ACM_BUFFER_SIZE;
if (avail_data > size) {
bytes_read = size;
} else {
bytes_read = avail_data;
}

for (i = 0U; i < bytes_read; i++) {
rx_data[i] = dev_data->rx_buf[(dev_data->rx_buf_tail + i) %
CDC_ACM_BUFFER_SIZE];
}

dev_data->rx_buf_tail = (dev_data->rx_buf_tail + bytes_read) %
CDC_ACM_BUFFER_SIZE;
len = ring_buf_get(dev_data->rx_ringbuf, rx_data, size);

if (dev_data->rx_buf_tail == dev_data->rx_buf_head) {
/* Buffer empty */
dev_data->rx_ready = 0U;
if (ring_buf_is_empty(dev_data->rx_ringbuf)) {
dev_data->rx_ready = false;
}

return bytes_read;
return len;
}

/**
Expand All @@ -666,7 +632,8 @@ static void cdc_acm_irq_tx_enable(struct device *dev)
{
struct cdc_acm_dev_data_t * const dev_data = DEV_DATA(dev);

dev_data->tx_irq_ena = 1U;
dev_data->tx_irq_ena = true;

if (dev_data->cb && dev_data->tx_ready) {
k_work_submit(&dev_data->cb_work);
}
Expand All @@ -683,7 +650,7 @@ static void cdc_acm_irq_tx_disable(struct device *dev)
{
struct cdc_acm_dev_data_t * const dev_data = DEV_DATA(dev);

dev_data->tx_irq_ena = 0U;
dev_data->tx_irq_ena = false;
}

/**
Expand Down Expand Up @@ -715,7 +682,8 @@ static void cdc_acm_irq_rx_enable(struct device *dev)
{
struct cdc_acm_dev_data_t * const dev_data = DEV_DATA(dev);

dev_data->rx_irq_ena = 1U;
dev_data->rx_irq_ena = true;

if (dev_data->cb && dev_data->rx_ready) {
k_work_submit(&dev_data->cb_work);
}
Expand All @@ -732,7 +700,7 @@ static void cdc_acm_irq_rx_disable(struct device *dev)
{
struct cdc_acm_dev_data_t * const dev_data = DEV_DATA(dev);

dev_data->rx_irq_ena = 0U;
dev_data->rx_irq_ena = false;
}

/**
Expand Down Expand Up @@ -1075,9 +1043,12 @@ static const struct uart_driver_api cdc_acm_driver_api = {
#endif /* CONFIG_USB_COMPOSITE_DEVICE */

#define DEFINE_CDC_ACM_DEV_DATA(x) \
RING_BUF_DECLARE(rx_ringbuf_##x, \
CONFIG_USB_CDC_ACM_RINGBUF_SIZE); \
static struct cdc_acm_dev_data_t cdc_acm_dev_data_##x = { \
.usb_status = USB_DC_UNKNOWN, \
.line_coding = CDC_ACM_DEFAUL_BAUDRATE, \
.rx_ringbuf = &rx_ringbuf_##x, \
}

#define DEFINE_CDC_ACM_DEVICE(x) \
Expand Down