Skip to content

Commit 737ada2

Browse files
committed
usb: fix rx buffer overflow with proper flow control
- Check buffer space before reading USB data - Track rx_stalled state to minimize kickoff overhead - Only retry reading when previously stalled and space available - Prevents data loss while maintaining USB NAK flow control - Document ring buffer interrupt safety guarantees
1 parent 566b3b1 commit 737ada2

File tree

3 files changed

+36
-5
lines changed

3 files changed

+36
-5
lines changed

src/driver/mcu/at32/usb.c

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ extern volatile bool usb_device_configured;
1414

1515
static otg_core_type otg_core_struct;
1616

17+
static volatile bool rx_stalled = false;
1718
static volatile bool tx_stalled = true;
1819

1920
static void usb_enable_clock() {
@@ -74,6 +75,14 @@ void usb_drv_init() {
7475
void usb_cdc_rx_handler() {
7576
usb_device_configured = true;
7677

78+
// Check if we have enough space before reading
79+
if (ring_buffer_free(&usb_rx_buffer) < USBD_CDC_IN_MAXPACKET_SIZE) {
80+
// Don't read data, USB will NAK automatically
81+
rx_stalled = true;
82+
return;
83+
}
84+
rx_stalled = false;
85+
7786
static uint8_t buf[USBD_CDC_IN_MAXPACKET_SIZE];
7887

7988
const uint16_t len = usb_vcp_get_rxdata(&otg_core_struct.dev, buf);
@@ -83,6 +92,16 @@ void usb_cdc_rx_handler() {
8392
ring_buffer_write_multi(&usb_rx_buffer, buf, len);
8493
}
8594

95+
static void usb_cdc_kickoff_rx() {
96+
if (!rx_stalled) {
97+
return;
98+
}
99+
100+
interrupt_disable(OTGFS1_IRQn);
101+
usb_cdc_rx_handler();
102+
interrupt_enable(OTGFS1_IRQn, USB_PRIORITY);
103+
}
104+
86105
void usb_cdc_tx_handler() {
87106
usb_device_configured = true;
88107

@@ -126,7 +145,9 @@ uint32_t usb_serial_read(uint8_t *data, uint32_t len) {
126145
if (data == NULL || len == 0) {
127146
return 0;
128147
}
129-
return ring_buffer_read_multi(&usb_rx_buffer, data, len);
148+
const uint32_t read = ring_buffer_read_multi(&usb_rx_buffer, data, len);
149+
usb_cdc_kickoff_rx();
150+
return read;
130151
}
131152

132153
void usb_serial_write(uint8_t *data, uint32_t len) {

src/driver/mcu/stm32/usb.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -240,8 +240,8 @@ static usbd_respond cdc_control(usbd_device *dev, usbd_ctlreq *req, usbd_rqc_cal
240240
}
241241

242242
static void cdc_rxonly(usbd_device *dev, uint8_t event, uint8_t ep) {
243-
if (ring_buffer_free(&usb_rx_buffer) <= CDC_DATA_SZ) {
244-
interrupt_disable(USB_IRQ);
243+
if (ring_buffer_free(&usb_rx_buffer) < CDC_DATA_SZ) {
244+
// Don't read data, USB will NAK automatically
245245
rx_stalled = true;
246246
return;
247247
}

src/util/ring_buffer.h

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,18 @@
22

33
#include <stdint.h>
44

5-
// This Ring Buffer _should_ be interrupt-safe given a single consumer/producer scenario.
6-
// Monitor usage closely to ensure above condition is true. All internal blocking has been removed.
5+
// This Ring Buffer implementation is interrupt-safe for single producer/single consumer scenarios.
6+
//
7+
// Safety guarantees:
8+
// - One context writes (producer), one context reads (consumer)
9+
// - Head is only modified by producer, tail only by consumer
10+
// - ring_buffer_free() and ring_buffer_available() take atomic snapshots of volatile pointers
11+
// - These functions may return slightly stale values if called during concurrent access,
12+
// but the values are always valid and safe (conservative for buffer overflow prevention)
13+
//
14+
// Typical usage:
15+
// - ISR writes to RX buffer, main reads from RX buffer
16+
// - Main writes to TX buffer, ISR reads from TX buffer
717
typedef struct {
818
uint8_t *const buffer;
919
volatile uint32_t head;

0 commit comments

Comments
 (0)