Skip to content

dcd/musb: defer EP0 SETUP during DATA_IN/STATUS race #3643

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

Open
HiFiPhile wants to merge 2 commits into
base: master
Choose a base branch
from
+111 −39
Open

dcd/musb: defer EP0 SETUP during DATA_IN/STATUS race #3643

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c4cd6c8
dcd/musb: defer EP0 SETUP during DATA_IN/STATUS race
HiFiPhile May 14, 2026
ae61e6c
fix test hung
HiFiPhile May 14, 2026
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
102 changes: 78 additions & 24 deletions src/portable/mentor/musb/dcd_musb.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,8 @@ typedef struct {

enum {
PIPE0_STATE_IDLE = 0, // no active control transfer
PIPE0_STATE_DATA, // DATA stage (IN or OUT — direction implied by CSR/dir)
PIPE0_STATE_DATA_IN, // DATA IN stage
PIPE0_STATE_DATA_OUT, // DATA OUT stage
PIPE0_STATE_STATUS_IN, // STATUS IN — device sends IN-ZLP; awaits send-ACK IRQ
PIPE0_STATE_STATUS_OUT, // post-DATAEND, neither edpt0_xfer(STATUS OUT) nor confirmation IRQ has happened yet
PIPE0_STATE_STATUS_OUT_PENDING, // one of {edpt0_xfer(STATUS OUT), confirmation IRQ} has happened; the other fires xfer_complete
Expand All @@ -95,12 +96,41 @@ typedef struct {
uint16_t remain_wlength; // bytes remaining in the control transfer's DATA stage
uint8_t state;
uint8_t pending_addr; // new USB address latched by dcd_set_address; applied when STATUS IN completes
tusb_control_request_t deferred_setup;
bool deferred_setup_valid;
} pipe0;
pipe_state_t pipe[MUSB_PIPE_COUNT];
} dcd_data_t;

static dcd_data_t _dcd;

static void pipe0_start_setup(uint8_t rhport, musb_ep_csr_t* ep_csr,
tusb_control_request_t const* req, bool is_isr) {
_dcd.pipe0.remain_wlength = req->wLength;

if (req->wLength == 0) {
_dcd.pipe0.state = PIPE0_STATE_STATUS_IN;
} else {
if (req->bmRequestType & TUSB_DIR_IN_MASK) {
_dcd.pipe0.state = PIPE0_STATE_DATA_IN;
ep_csr->csr0l = MUSB_CSRL0_RXRDYC;
} else {
_dcd.pipe0.state = PIPE0_STATE_DATA_OUT;
}
}

dcd_event_setup_received(rhport, (const uint8_t *) req, is_isr);
}

static void pipe0_process_deferred_setup(uint8_t rhport, musb_ep_csr_t* ep_csr, bool is_isr) {
if (!_dcd.pipe0.deferred_setup_valid) {
return;
}

_dcd.pipe0.deferred_setup_valid = false;
pipe0_start_setup(rhport, ep_csr, &_dcd.pipe0.deferred_setup, is_isr);
}

// EP0 must not call this — it has its own scalars in dcd_data_t.
TU_ATTR_ALWAYS_INLINE static inline pipe_state_t* pipe_get(uint8_t epnum, tusb_dir_t epdir) {
size_t idx = epnum - 1u;
Expand Down Expand Up @@ -323,7 +353,7 @@ static void process_epout(uint8_t rhport, musb_regs_t *musb_regs, uint8_t epnum,

static bool edpt_n_xfer(uint8_t rhport, uint8_t ep_addr, void *buffer, uint16_t total_bytes, bool use_fifo, bool is_isr) {
const uint8_t epnum = tu_edpt_number(ep_addr);
const unsigned dir_in = tu_edpt_dir(ep_addr);
const tusb_dir_t dir_in = tu_edpt_dir(ep_addr);

pipe_state_t *pipe = pipe_get(epnum, dir_in);
if (use_fifo) {
Expand Down Expand Up @@ -361,7 +391,8 @@ static bool edpt0_xfer(uint8_t rhport, uint8_t ep_addr, uint8_t *buffer, uint16_
const unsigned dir_in = tu_edpt_dir(ep_addr);

switch (_dcd.pipe0.state) {
case PIPE0_STATE_DATA: {
case PIPE0_STATE_DATA_IN:
case PIPE0_STATE_DATA_OUT: {
_dcd.pipe0.xact_len = total_bytes;
if (dir_in) {
// DATA IN: load FIFO, set TXRDY. Add DATAEND on the last chunk
Expand Down Expand Up @@ -396,6 +427,7 @@ static bool edpt0_xfer(uint8_t rhport, uint8_t ep_addr, uint8_t *buffer, uint16_
// Second event — IRQ already arrived, fire complete now.
_dcd.pipe0.state = PIPE0_STATE_IDLE;
dcd_event_xfer_complete(rhport, ep_addr, 0, XFER_RESULT_SUCCESS, is_isr);
pipe0_process_deferred_setup(rhport, ep_csr, is_isr);
break;

default: break;
Expand All @@ -410,9 +442,14 @@ static void process_ep0(uint8_t rhport) {
musb_ep_csr_t* ep_csr = get_ep_csr(musb_regs, 0);
uint_fast8_t csrl = ep_csr->csr0l;

if (csrl & MUSB_CSRL0_DATAEND) {
return;
}

Comment thread
HiFiPhile marked this conversation as resolved.
if (csrl & MUSB_CSRL0_STALLED) {
ep_csr->csr0l = 0;
_dcd.pipe0.state = PIPE0_STATE_IDLE;
_dcd.pipe0.deferred_setup_valid = false;
return;
}

Expand All @@ -421,6 +458,7 @@ static void process_ep0(uint8_t rhport) {
// do nothing, it is probably another setup packet, usbd will reset its state.
ep_csr->csr0l = MUSB_CSRL0_SETENDC;
_dcd.pipe0.state = PIPE0_STATE_IDLE;
_dcd.pipe0.deferred_setup_valid = false;
if (!(csrl & MUSB_CSRL0_RXRDY)) {
return; /* no SETUP waiting behind it */
}
Expand All @@ -430,30 +468,19 @@ static void process_ep0(uint8_t rhport) {
if (csrl & MUSB_CSRL0_RXRDY) {
const uint16_t count0 = ep_csr->count0;
switch (_dcd.pipe0.state) {
case PIPE0_STATE_IDLE:
case PIPE0_STATE_IDLE: {
TU_ASSERT(sizeof(tusb_control_request_t) == count0, );
union {
tusb_control_request_t req;
uint32_t u32[2];
} setup_packet;
setup_packet.u32[0] = musb_regs->fifo[0];
setup_packet.u32[1] = musb_regs->fifo[0];

_dcd.pipe0.remain_wlength = setup_packet.req.wLength;

if (setup_packet.req.wLength == 0) {
_dcd.pipe0.state = PIPE0_STATE_STATUS_IN;
} else {
_dcd.pipe0.state = PIPE0_STATE_DATA;
// If OUT (rx) direction, let edpt0_xfer() clear RXRDY when it's ready to receive data.
if (setup_packet.req.bmRequestType & TUSB_DIR_IN_MASK) {
ep_csr->csr0l = MUSB_CSRL0_RXRDYC;
}
}
dcd_event_setup_received(rhport, (const uint8_t *)&setup_packet.req, true);
pipe0_start_setup(rhport, ep_csr, &setup_packet.req, true);
break;
}

case PIPE0_STATE_DATA: {
case PIPE0_STATE_DATA_OUT: {
// EP0 OUT is single-packet (TU_ASSERT total_bytes <= EP0_SIZE in edpt0_xfer)
// so the whole packet drains in one shot.
if (count0) {
Expand All @@ -463,31 +490,54 @@ static void process_ep0(uint8_t rhport) {
if (_dcd.pipe0.remain_wlength == 0) {
// last packet: change state and leave RXRDY for edpt0_xfer(STATUS IN) to ack
_dcd.pipe0.state = PIPE0_STATE_STATUS_IN;
} else {
ep_csr->csr0l = MUSB_CSRL0_RXRDYC;
}
dcd_event_xfer_complete(rhport, TU_EP0_OUT, count0, XFER_RESULT_SUCCESS, true);
break;
}

default: break;
// New SETUP packet arrived while old control transfer is not finished yet. This could happen in following scenarios:
// - Status IN/OUT finished, IRQ and new setup packet IRQ arrive at the same time.
// - Data IN finished and status OUT is received, both IRQs and new setup packet IRQ arrive at the same time.
// could happen when CPU load is high, save the new setup packet for later processing after current status stage complete.
case PIPE0_STATE_STATUS_OUT:
case PIPE0_STATE_STATUS_OUT_PENDING:
case PIPE0_STATE_STATUS_IN:
case PIPE0_STATE_DATA_IN: {
TU_ASSERT(sizeof(tusb_control_request_t) == count0, );
union {
tusb_control_request_t req;
uint32_t u32[2];
} setup_packet;
setup_packet.u32[0] = musb_regs->fifo[0];
setup_packet.u32[1] = musb_regs->fifo[0];

_dcd.pipe0.deferred_setup = setup_packet.req;
_dcd.pipe0.deferred_setup_valid = true;
goto process_status;
}
}

return;
}

process_status:
/* When CSRL0 is zero, it means that either
* - completion of sending any length packet TxPktRdy clear
* - or status stage is complete (ZLP) after DataEnd is set */
switch (_dcd.pipe0.state) {
case PIPE0_STATE_DATA:
case PIPE0_STATE_DATA_IN:
// csrl == 0 in DATA state = TXRDY just cleared, i.e. a DATA IN packet was successfully sent. If the just-sent
// packet was the last (DATAEND was set when ep0_remain_datalen hit zero), transition
// to STATUS_OUT to await the host's STATUS-OUT ZLP confirmation IRQ.
if (_dcd.pipe0.remain_wlength == 0) {
_dcd.pipe0.state = PIPE0_STATE_STATUS_OUT;
// If a new SETUP was deferred then STATUS OUT IRQ is missed, manually transition to STATUS_OUT_PENDING to allow ep0_xfer(STATUS OUT) to fire complete immediately.
if (_dcd.pipe0.deferred_setup_valid) {
_dcd.pipe0.state = PIPE0_STATE_STATUS_OUT_PENDING;
}
}
dcd_event_xfer_complete(rhport, TU_EP0_IN, _dcd.pipe0.xact_len, XFER_RESULT_SUCCESS, true);

break;

case PIPE0_STATE_STATUS_OUT:
Expand All @@ -499,6 +549,7 @@ static void process_ep0(uint8_t rhport) {
// Second event — edpt0_xfer(STATUS OUT) already called, fire complete now.
_dcd.pipe0.state = PIPE0_STATE_IDLE;
dcd_event_xfer_complete(rhport, TU_EP0_OUT, 0, XFER_RESULT_SUCCESS, true);
pipe0_process_deferred_setup(rhport, ep_csr, true);
break;

case PIPE0_STATE_STATUS_IN:
Expand All @@ -508,6 +559,7 @@ static void process_ep0(uint8_t rhport) {
}
_dcd.pipe0.state = PIPE0_STATE_IDLE;
dcd_event_xfer_complete(rhport, TU_EP0_IN, 0, XFER_RESULT_SUCCESS, true);
pipe0_process_deferred_setup(rhport, ep_csr, true);
break;

default: break;
Expand All @@ -527,6 +579,7 @@ static void process_bus_reset(uint8_t rhport) {
_dcd.pipe0.buf = NULL;
_dcd.pipe0.xact_len = 0;
_dcd.pipe0.remain_wlength = 0;
_dcd.pipe0.deferred_setup_valid = false;

musb->intr_txen = 1; /* Enable only EP0 */
musb->intr_rxen = 0;
Expand Down Expand Up @@ -646,7 +699,7 @@ void dcd_sof_enable(uint8_t rhport, bool en)
bool dcd_edpt_open(uint8_t rhport, tusb_desc_endpoint_t const * ep_desc) {
const unsigned ep_addr = ep_desc->bEndpointAddress;
const unsigned epn = tu_edpt_number(ep_addr);
const unsigned epdir = tu_edpt_dir(ep_addr);
const tusb_dir_t epdir = tu_edpt_dir(ep_addr);
const unsigned mps = tu_edpt_packet_size(ep_desc);

pipe_state_t *pipe = pipe_get(epn, epdir);
Expand Down Expand Up @@ -689,7 +742,7 @@ bool dcd_edpt_iso_alloc(uint8_t rhport, uint8_t ep_addr, uint16_t largest_packet
bool dcd_edpt_iso_activate(uint8_t rhport, tusb_desc_endpoint_t const *ep_desc ) {
const unsigned ep_addr = ep_desc->bEndpointAddress;
const unsigned epn = tu_edpt_number(ep_addr);
const unsigned dir_in = tu_edpt_dir(ep_addr);
const tusb_dir_t dir_in = tu_edpt_dir(ep_addr);
const unsigned mps = tu_edpt_packet_size(ep_desc);

unsigned const ie = musb_dcd_get_int_enable(rhport);
Expand Down Expand Up @@ -804,6 +857,7 @@ void dcd_edpt_stall(uint8_t rhport, uint8_t ep_addr) {
if (ep_addr == TU_EP0_OUT) { /* Ignore EP0 OUT */
_dcd.pipe0.state = PIPE0_STATE_IDLE;
_dcd.pipe0.buf = NULL;
_dcd.pipe0.deferred_setup_valid = false;
ep_csr->csr0l = MUSB_CSRL0_STALL;
}
} else {
Expand Down
2 changes: 1 addition & 1 deletion src/portable/mentor/musb/musb_max32.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ extern "C" {
#define MUSB_CFG_SHARED_FIFO 1 // shared FIFO for TX and RX endpoints
#define MUSB_CFG_DYNAMIC_FIFO 0 // dynamic EP FIFO sizing

const uintptr_t MUSB_BASES[] = { MXC_BASE_USBHS };
static const uintptr_t MUSB_BASES[] = { MXC_BASE_USBHS };

#if CFG_TUD_ENABLED
#define USBHS_M31_CLOCK_RECOVERY
Expand Down
2 changes: 1 addition & 1 deletion src/portable/mentor/musb/musb_ti.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@
#define MUSB_CFG_DYNAMIC_FIFO 1
#define MUSB_CFG_DYNAMIC_FIFO_SIZE 4096

const uintptr_t MUSB_BASES[] = { USB0_BASE };
static const uintptr_t MUSB_BASES[] = { USB0_BASE };

// Header supports both device and host modes. Only include what's necessary
#if CFG_TUD_ENABLED
Expand Down
44 changes: 31 additions & 13 deletions test/hil/hil_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,9 @@ class HilConfig(TypedDict):

CMD_TIMEOUT = int(os.getenv('HIL_CMD_TIMEOUT', '180'))
POOL_TIMEOUT = int(os.getenv('HIL_POOL_TIMEOUT', '3000'))
SERIAL_READ_TIMEOUT = float(os.getenv('HIL_SERIAL_READ_TIMEOUT', '5'))
SERIAL_WRITE_TIMEOUT = float(os.getenv('HIL_SERIAL_WRITE_TIMEOUT', '2'))
SERIAL_WRITE_DEADLINE = float(os.getenv('HIL_SERIAL_WRITE_DEADLINE', '10'))


def cmd_stdout_text(out: Any) -> str:
Expand Down Expand Up @@ -218,7 +221,8 @@ def open_serial_dev(port: str):
while timeout > 0:
if os.path.exists(port):
try:
ser = serial.Serial(port, baudrate=115200, timeout=5)
ser = serial.Serial(port, baudrate=115200, timeout=SERIAL_READ_TIMEOUT,
write_timeout=SERIAL_WRITE_TIMEOUT)
break
except serial.SerialException:
print(f'serial {port} not reaady {timeout} sec')
Expand All @@ -231,6 +235,26 @@ def open_serial_dev(port: str):
return ser


def serial_write_all(ser: serial.Serial, data: bytes, deadline: float = SERIAL_WRITE_DEADLINE):
total = 0
end = time.monotonic() + deadline

while total < len(data):
try:
written = ser.write(data[total:])
except serial.SerialTimeoutException:
written = 0

if written:
total += written
continue

if time.monotonic() >= end:
raise AssertionError(f'Serial write timeout after {deadline:.1f}s')

time.sleep(0.01)


def read_disk_file(uid: str, lun: int, fname: str) -> bytes:
# open_fs("fat://{dev}) require 'pip install pyfatfs'
dev = get_disk_dev(uid, 'TinyUSB', lun)
Expand Down Expand Up @@ -698,8 +722,7 @@ def rand_ascii(length):
offset = 0
while offset < echo_len:
chunk_size = min(random.randint(1, packet_size), echo_len - offset)
ser.write(echo_data[offset:offset + chunk_size])
ser.flush()
serial_write_all(ser, echo_data[offset:offset + chunk_size])
# wait until this chunk is echoed back
echo = b''
t_end = time.monotonic() + 1.0
Expand Down Expand Up @@ -748,8 +771,7 @@ def test_host_msc_file_explorer(board):
time.sleep(1)
ser.reset_input_buffer()
for ch in 'cat README.TXT\r':
ser.write(ch.encode())
ser.flush()
serial_write_all(ser, ch.encode())
time.sleep(0.002)

resp = b''
Expand All @@ -771,8 +793,7 @@ def test_host_msc_file_explorer(board):
time.sleep(0.5)
ser.reset_input_buffer()
for ch in 'dd 1024\r':
ser.write(ch.encode())
ser.flush()
serial_write_all(ser, ch.encode())
time.sleep(0.002)

# Read dd output until prompt
Expand Down Expand Up @@ -827,8 +848,7 @@ def write_and_check(writer, payload : bytes):
# Write in chunks of random 1-64 bytes (device has 64-byte buffer)
while offset < payload_len:
chunk_size = min(random.randint(1, 64), payload_len - offset)
ser[writer].write(payload[offset:offset + chunk_size])
ser[writer].flush()
serial_write_all(ser[writer], payload[offset:offset + chunk_size])
rd0 += ser[0].read(chunk_size)
rd1 += ser[1].read(chunk_size)
offset += chunk_size
Expand Down Expand Up @@ -862,8 +882,7 @@ def rand_ascii(length):
# Write in chunks of random 1-64 bytes (device has 64-byte buffer)
while offset < size:
chunk_size = min(random.randint(1, 64), size - offset)
ser.write(test_str[offset:offset + chunk_size])
ser.flush()
serial_write_all(ser, test_str[offset:offset + chunk_size])
rd_str += ser.read(chunk_size)
offset += chunk_size
assert rd_str == test_str, f'CDC wrong data ({size} bytes):\n expected: {test_str}\n received: {rd_str}'
Expand Down Expand Up @@ -1124,8 +1143,7 @@ def lp_reader():
offset = 0
while offset < size:
chunk_size = min(random.randint(1, 64), size - offset)
ser.write(test_data[offset:offset + chunk_size])
ser.flush()
serial_write_all(ser, test_data[offset:offset + chunk_size])
time.sleep(0.01)
offset += chunk_size

Expand Down
Loading