Skip to content

Commit

Permalink
subsys: usb: device_next: cdc_acm: Check of EP for TX to prevent loop
Browse files Browse the repository at this point in the history
In case the host doesn't pull the new data from the endpoint, the work task
would schedule itself again (at the max. priority). When there is no terminal
program or active applicaiton, this could result in an infinite loop blocking all
other communications.

By using a atomic flag for tx busy, the deadlock is solved.

Signed-off-by: Vincent van der Locht <[email protected]>
  • Loading branch information
SynchronicIT committed Sep 9, 2024
1 parent 00ad9ca commit 2738d32
Showing 1 changed file with 28 additions and 4 deletions.
32 changes: 28 additions & 4 deletions subsys/usb/device_next/class/usbd_cdc_acm.c
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ UDC_BUF_POOL_DEFINE(cdc_acm_ep_pool,
#define CDC_ACM_IRQ_TX_ENABLED 3
#define CDC_ACM_RX_FIFO_BUSY 4
#define CDC_ACM_LOCK 5
#define CDC_ACM_TX_FIFO_BUSY 6

Check notice on line 52 in subsys/usb/device_next/class/usbd_cdc_acm.c

View workflow job for this annotation

GitHub Actions / Run compliance checks on patch series (PR)

You may want to run clang-format on this change

subsys/usb/device_next/class/usbd_cdc_acm.c:52 -#define CDC_ACM_TX_FIFO_BUSY 6 +#define CDC_ACM_TX_FIFO_BUSY 6

Check notice on line 52 in subsys/usb/device_next/class/usbd_cdc_acm.c

View workflow job for this annotation

GitHub Actions / Run compliance checks on patch series (PR)

You may want to run clang-format on this change

subsys/usb/device_next/class/usbd_cdc_acm.c:52 -#define CDC_ACM_TX_FIFO_BUSY 6 +#define CDC_ACM_TX_FIFO_BUSY 6
static struct k_work_q cdc_acm_work_q;
static K_KERNEL_STACK_DEFINE(cdc_acm_stack,
Expand Down Expand Up @@ -217,6 +218,11 @@ static int usbd_cdc_acm_request(struct usbd_class_data *const c_data,
atomic_clear_bit(&data->state, CDC_ACM_RX_FIFO_BUSY);
}

if (bi->ep == cdc_acm_get_bulk_in(c_data)) {
atomic_clear_bit(&data->state, CDC_ACM_TX_FIFO_BUSY);
LOG_ERR("TX ERROR !!");
}

goto ep_request_error;
}

Expand All @@ -236,9 +242,19 @@ static int usbd_cdc_acm_request(struct usbd_class_data *const c_data,

if (bi->ep == cdc_acm_get_bulk_in(c_data)) {
/* TX transfer completion */
if (data->cb) {
cdc_acm_work_submit(&data->irq_cb_work);
atomic_clear_bit(&data->state, CDC_ACM_TX_FIFO_BUSY);

Check failure on line 246 in subsys/usb/device_next/class/usbd_cdc_acm.c

View workflow job for this annotation

GitHub Actions / Run compliance checks on patch series (PR)

TRAILING_WHITESPACE

subsys/usb/device_next/class/usbd_cdc_acm.c:246 trailing whitespace

Check failure on line 246 in subsys/usb/device_next/class/usbd_cdc_acm.c

View workflow job for this annotation

GitHub Actions / Run compliance checks on patch series (PR)

TRAILING_WHITESPACE

subsys/usb/device_next/class/usbd_cdc_acm.c:246 trailing whitespace

Check notice on line 247 in subsys/usb/device_next/class/usbd_cdc_acm.c

View workflow job for this annotation

GitHub Actions / Run compliance checks on patch series (PR)

You may want to run clang-format on this change

subsys/usb/device_next/class/usbd_cdc_acm.c:247 -

Check notice on line 247 in subsys/usb/device_next/class/usbd_cdc_acm.c

View workflow job for this annotation

GitHub Actions / Run compliance checks on patch series (PR)

You may want to run clang-format on this change

subsys/usb/device_next/class/usbd_cdc_acm.c:247 -
if (ring_buf_is_empty(data->tx_fifo.rb)) {
/* Raise TX ready interrupt */
if (data->cb) {
cdc_acm_work_submit(&data->irq_cb_work);
}
} else {
/* Queue pending TX data on IN endpoint */
cdc_acm_work_submit(&data->tx_fifo_work);
}

}

Check notice on line 258 in subsys/usb/device_next/class/usbd_cdc_acm.c

View workflow job for this annotation

GitHub Actions / Run compliance checks on patch series (PR)

You may want to run clang-format on this change

subsys/usb/device_next/class/usbd_cdc_acm.c:258 -

Check notice on line 258 in subsys/usb/device_next/class/usbd_cdc_acm.c

View workflow job for this annotation

GitHub Actions / Run compliance checks on patch series (PR)

You may want to run clang-format on this change

subsys/usb/device_next/class/usbd_cdc_acm.c:258 -

if (bi->ep == cdc_acm_get_int_in(c_data)) {
Expand Down Expand Up @@ -535,14 +551,20 @@ static void cdc_acm_tx_fifo_handler(struct k_work *work)
return;
}

if (atomic_test_and_set_bit(&data->state, CDC_ACM_TX_FIFO_BUSY)) {
LOG_WRN("TX transfer already in progress");
return;
}

if (atomic_test_and_set_bit(&data->state, CDC_ACM_LOCK)) {
atomic_clear_bit(&data->state, CDC_ACM_RX_FIFO_BUSY);
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);
// cdc_acm_work_submit(&data->tx_fifo_work);

Check failure on line 567 in subsys/usb/device_next/class/usbd_cdc_acm.c

View workflow job for this annotation

GitHub Actions / Run compliance checks on patch series (PR)

C99_COMMENTS

subsys/usb/device_next/class/usbd_cdc_acm.c:567 do not use C99 // comments

Check failure on line 567 in subsys/usb/device_next/class/usbd_cdc_acm.c

View workflow job for this annotation

GitHub Actions / Run compliance checks on patch series (PR)

C99_COMMENTS

subsys/usb/device_next/class/usbd_cdc_acm.c:567 do not use C99 // comments
goto tx_fifo_handler_exit;
}

Expand All @@ -551,6 +573,7 @@ static void cdc_acm_tx_fifo_handler(struct k_work *work)

ret = usbd_ep_enqueue(c_data, buf);
if (ret) {
atomic_clear_bit(&data->state, CDC_ACM_RX_FIFO_BUSY);
LOG_ERR("Failed to enqueue");
net_buf_unref(buf);
}
Expand Down Expand Up @@ -824,7 +847,8 @@ static void cdc_acm_irq_cb_handler(struct k_work *work)
cdc_acm_work_submit(&data->rx_fifo_work);
}

if (data->tx_fifo.altered) {
if (!atomic_test_bit(&data->state, CDC_ACM_TX_FIFO_BUSY) &&
data->tx_fifo.altered) {
LOG_DBG("tx fifo altered, submit work");

Check notice on line 852 in subsys/usb/device_next/class/usbd_cdc_acm.c

View workflow job for this annotation

GitHub Actions / Run compliance checks on patch series (PR)

You may want to run clang-format on this change

subsys/usb/device_next/class/usbd_cdc_acm.c:852 - if (!atomic_test_bit(&data->state, CDC_ACM_TX_FIFO_BUSY) && - data->tx_fifo.altered) { + if (!atomic_test_bit(&data->state, CDC_ACM_TX_FIFO_BUSY) && data->tx_fifo.altered) {

Check notice on line 852 in subsys/usb/device_next/class/usbd_cdc_acm.c

View workflow job for this annotation

GitHub Actions / Run compliance checks on patch series (PR)

You may want to run clang-format on this change

subsys/usb/device_next/class/usbd_cdc_acm.c:852 - if (!atomic_test_bit(&data->state, CDC_ACM_TX_FIFO_BUSY) && - data->tx_fifo.altered) { + if (!atomic_test_bit(&data->state, CDC_ACM_TX_FIFO_BUSY) && data->tx_fifo.altered) {
cdc_acm_work_submit(&data->tx_fifo_work);
}
Expand Down

0 comments on commit 2738d32

Please sign in to comment.