From 06b8f4f013a6ce0d91d2f713c88313569d8a722c Mon Sep 17 00:00:00 2001 From: hathach Date: Sat, 13 Jun 2026 00:17:07 +0700 Subject: [PATCH 01/17] dcd/musb: extract pipe0_read_setup() helper The 8-byte EP0 SETUP drain (count0 assert + two FIFO word reads via a union) was duplicated verbatim between the IDLE case and the deferral case; a future fix applied to one copy but not the other would only show up on the rare deferred-race path. Share one helper. count0 is now read inside the only remaining user (DATA OUT drain). Review follow-up for #3643 (dcd_musb.c l.507 finding). Co-Authored-By: Claude Fable 5 --- src/portable/mentor/musb/dcd_musb.c | 36 ++++++++++++++--------------- 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/src/portable/mentor/musb/dcd_musb.c b/src/portable/mentor/musb/dcd_musb.c index e005850686..08d7dd700f 100644 --- a/src/portable/mentor/musb/dcd_musb.c +++ b/src/portable/mentor/musb/dcd_musb.c @@ -104,6 +104,19 @@ typedef struct { static dcd_data_t _dcd; +// Drain a SETUP packet (8 bytes) from the EP0 FIFO. Does not ack RxPktRdy. +static bool pipe0_read_setup(musb_regs_t* musb_regs, musb_ep_csr_t* ep_csr, tusb_control_request_t* req) { + TU_ASSERT(sizeof(tusb_control_request_t) == ep_csr->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]; + *req = setup_packet.req; + return true; +} + 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; @@ -466,23 +479,18 @@ static void process_ep0(uint8_t rhport) { // Receive Data (Setup or OUT) if (csrl & MUSB_CSRL0_RXRDY) { - const uint16_t count0 = ep_csr->count0; switch (_dcd.pipe0.state) { 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]; - pipe0_start_setup(rhport, ep_csr, &setup_packet.req, true); + tusb_control_request_t req; + TU_VERIFY(pipe0_read_setup(musb_regs, ep_csr, &req), ); + pipe0_start_setup(rhport, ep_csr, &req, true); break; } 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. + const uint16_t count0 = ep_csr->count0; if (count0) { tu_hwfifo_read(&musb_regs->fifo[0], _dcd.pipe0.buf, count0, NULL); _dcd.pipe0.remain_wlength -= count0; @@ -503,15 +511,7 @@ static void process_ep0(uint8_t rhport) { 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; + TU_VERIFY(pipe0_read_setup(musb_regs, ep_csr, &_dcd.pipe0.deferred_setup), ); _dcd.pipe0.deferred_setup_valid = true; goto process_status; } From 87eeab605ff571a5ac1784c2c26e944aa0129d7e Mon Sep 17 00:00:00 2001 From: hathach Date: Sat, 13 Jun 2026 00:17:30 +0700 Subject: [PATCH 02/17] dcd/musb: restore EP0 OUT RXRDY flow-control comment The pre-existing comment explaining why the OUT branch does not ack RxPktRdy was dropped when the SETUP handling moved into pipe0_start_setup(). It is load-bearing: acking before edpt0_xfer() arms the drain buffer would let the host send data with nowhere to put it. Restore it with the databook-deviation rationale so the branches don't get "unified" later. Review follow-up for #3643 (dcd_musb.c l.116 finding). Co-Authored-By: Claude Fable 5 --- src/portable/mentor/musb/dcd_musb.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/portable/mentor/musb/dcd_musb.c b/src/portable/mentor/musb/dcd_musb.c index 08d7dd700f..fdd2eaac92 100644 --- a/src/portable/mentor/musb/dcd_musb.c +++ b/src/portable/mentor/musb/dcd_musb.c @@ -128,6 +128,9 @@ static void pipe0_start_setup(uint8_t rhport, musb_ep_csr_t* ep_csr, _dcd.pipe0.state = PIPE0_STATE_DATA_IN; ep_csr->csr0l = MUSB_CSRL0_RXRDYC; } else { + // If OUT (rx) direction, let edpt0_xfer() clear RXRDY when it's ready to receive data. + // Deliberate deviation from the databook's canonical flow (ack right after unload), + // used as NAK flow control until usbd arms the drain buffer. _dcd.pipe0.state = PIPE0_STATE_DATA_OUT; } } From b21d59f8177af967e2b2757c4ef996df00d1e617 Mon Sep 17 00:00:00 2001 From: hathach Date: Sat, 13 Jun 2026 00:18:32 +0700 Subject: [PATCH 03/17] dcd/musb: replace deferral goto with per-state handling The goto jumped into the csrl==0 completion switch with RXRDY still set, making its "When CSRL0 is zero" guard comment untrue on that path. Handle each deferral state in a self-contained switch instead; the csrl==0 switch is now only reached with csrl==0 and its comment is truthful again. Behavior unchanged. Review follow-up for #3643 (dcd_musb.c l.523 finding). Co-Authored-By: Claude Fable 5 --- src/portable/mentor/musb/dcd_musb.c | 43 ++++++++++++++++++++++++++--- 1 file changed, 39 insertions(+), 4 deletions(-) diff --git a/src/portable/mentor/musb/dcd_musb.c b/src/portable/mentor/musb/dcd_musb.c index fdd2eaac92..9bdd6b080c 100644 --- a/src/portable/mentor/musb/dcd_musb.c +++ b/src/portable/mentor/musb/dcd_musb.c @@ -510,20 +510,55 @@ static void process_ep0(uint8_t rhport) { // - 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_DATA_IN: case PIPE0_STATE_STATUS_OUT: case PIPE0_STATE_STATUS_OUT_PENDING: - case PIPE0_STATE_STATUS_IN: - case PIPE0_STATE_DATA_IN: { + case PIPE0_STATE_STATUS_IN: { TU_VERIFY(pipe0_read_setup(musb_regs, ep_csr, &_dcd.pipe0.deferred_setup), ); _dcd.pipe0.deferred_setup_valid = true; - goto process_status; + + switch (_dcd.pipe0.state) { + case PIPE0_STATE_DATA_IN: + // Last DATA IN packet sent (TXRDY-clear coalesced with the SETUP IRQ). The STATUS OUT + // confirm IRQ is missed too — promote so edpt0_xfer(STATUS OUT) fires complete immediately. + if (_dcd.pipe0.remain_wlength == 0) { + _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: + // Status confirm IRQ coalesced with the SETUP — edpt0_xfer(STATUS OUT) fires complete. + _dcd.pipe0.state = PIPE0_STATE_STATUS_OUT_PENDING; + break; + + case PIPE0_STATE_STATUS_OUT_PENDING: + // edpt0_xfer(STATUS OUT) already called — fire complete and replay 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; + + default: + // PIPE0_STATE_STATUS_IN: ZLP-sent IRQ coalesced with the SETUP. + if (_dcd.pipe0.pending_addr) { + musb_regs->faddr = _dcd.pipe0.pending_addr; + _dcd.pipe0.pending_addr = 0; + } + _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; + } + break; } + + default: break; } 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 */ From 1ea385a6c431cee759695515a4bed94c6dff65c6 Mon Sep 17 00:00:00 2001 From: hathach Date: Sat, 13 Jun 2026 00:18:55 +0700 Subject: [PATCH 04/17] dcd/musb: check SentStall/SetupEnd before DATAEND guard MUSBMHDRC 21.1.5 requires the EP0 service routine to check SentStall and SetupEnd first; the early DATAEND return ran before both, and SentStall is most likely to fire exactly while DataEnd may still read back set (auto-STALL after DataEnd, 21.1.7), which would skip the recovery. The guard also moves below the RXRDY block so a coalesced DATAEND|RXRDY read cannot swallow a SETUP on cores where the CPU-set-only DataEnd bit reads back 1; the comment documents the vendor-dependent read-back. Review follow-up for #3643 (dcd_musb.c l.445 finding). Co-Authored-By: Claude Fable 5 --- src/portable/mentor/musb/dcd_musb.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/portable/mentor/musb/dcd_musb.c b/src/portable/mentor/musb/dcd_musb.c index 9bdd6b080c..86ae3ae9a8 100644 --- a/src/portable/mentor/musb/dcd_musb.c +++ b/src/portable/mentor/musb/dcd_musb.c @@ -458,10 +458,7 @@ 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; - } - + // 21.1.5: SentStall and SetupEnd must be checked before anything else. if (csrl & MUSB_CSRL0_STALLED) { ep_csr->csr0l = 0; _dcd.pipe0.state = PIPE0_STATE_IDLE; @@ -559,6 +556,13 @@ static void process_ep0(uint8_t rhport) { return; } + if (csrl & MUSB_CSRL0_DATAEND) { + // Last DATA IN chunk / STATUS IN arm wrote TXRDY|DATAEND and the status stage has not completed + // yet — nothing to service. DataEnd is CPU-set-only per the CSR access table; whether it ever + // reads back 1 is vendor-dependent (on cores where it reads 0 this guard is dead code). + return; + } + /* 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 */ From 91608e3c4f8c944674547e5c254759c1dda08379 Mon Sep 17 00:00:00 2001 From: hathach Date: Sat, 13 Jun 2026 00:20:37 +0700 Subject: [PATCH 05/17] dcd/musb: fix deferred-SETUP replay racing usbd's status call STATUS_OUT_PENDING conflated "edpt0_xfer(STATUS OUT) called, awaiting confirm IRQ" with "confirm IRQ seen, awaiting edpt0_xfer". The deferral path completed the status and replayed the saved SETUP from the ISR in both flavors; in the IRQ-first one, usbd's still- outstanding edpt0_xfer(STATUS OUT) for the old transfer (queued via status_stage_xact) then landed in the replayed transfer's state and corrupted it: NULL pipe0.buf armed plus RXRDYC, so the host's next DATA OUT drained through a NULL pointer. usbd processes EP0 XFER_COMPLETE events unconditionally, so nothing downstream defuses it. Split the state into STATUS_OUT_PENDING_XFER / _IRQ. The deferral completes and replays only in PENDING_XFER (old transfer already retired); in PENDING_IRQ it only holds the SETUP and the usbd-driven edpt0_xfer fires the completion and replays. The DATA_IN deferral now synthesizes PENDING_IRQ (its remain==0 invariant asserted: a SETUP before DataEnd raises SetupEnd instead), which also makes the old deferred-promotion in the csrl==0 DATA_IN case unreachable - dropped. Assert the drain buffer before the DATA OUT FIFO read as a cheap backstop for this corruption class. Review follow-up for #3643 (dcd_musb.c l.503 finding). Co-Authored-By: Claude Fable 5 --- src/portable/mentor/musb/dcd_musb.c | 59 +++++++++++++++++------------ 1 file changed, 35 insertions(+), 24 deletions(-) diff --git a/src/portable/mentor/musb/dcd_musb.c b/src/portable/mentor/musb/dcd_musb.c index 86ae3ae9a8..2c31e3b2fd 100644 --- a/src/portable/mentor/musb/dcd_musb.c +++ b/src/portable/mentor/musb/dcd_musb.c @@ -86,7 +86,8 @@ enum { 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 + PIPE0_STATE_STATUS_OUT_PENDING_XFER, // edpt0_xfer(STATUS OUT) called first; the confirmation IRQ fires xfer_complete + PIPE0_STATE_STATUS_OUT_PENDING_IRQ, // confirmation IRQ seen (or synthesized) first; edpt0_xfer(STATUS OUT) fires xfer_complete }; typedef struct { @@ -436,11 +437,12 @@ static bool edpt0_xfer(uint8_t rhport, uint8_t ep_addr, uint8_t *buffer, uint16_ case PIPE0_STATE_STATUS_OUT: TU_ASSERT(!dir_in && total_bytes == 0); // only STATUS OUT allowed // First event of the STATUS OUT pair — wait for the IRQ to fire complete. - _dcd.pipe0.state = PIPE0_STATE_STATUS_OUT_PENDING; + _dcd.pipe0.state = PIPE0_STATE_STATUS_OUT_PENDING_XFER; break; - case PIPE0_STATE_STATUS_OUT_PENDING: - // Second event — IRQ already arrived, fire complete now. + case PIPE0_STATE_STATUS_OUT_PENDING_IRQ: + // Second event — IRQ already arrived, fire complete now. The old transfer is retired here, + // so a deferred SETUP can be replayed safely. _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); @@ -492,6 +494,7 @@ static void process_ep0(uint8_t rhport) { // so the whole packet drains in one shot. const uint16_t count0 = ep_csr->count0; if (count0) { + TU_ASSERT(_dcd.pipe0.buf, ); tu_hwfifo_read(&musb_regs->fifo[0], _dcd.pipe0.buf, count0, NULL); _dcd.pipe0.remain_wlength -= count0; } @@ -503,39 +506,48 @@ static void process_ep0(uint8_t rhport) { 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. + // New SETUP packet arrived while the old control transfer's tail events are still in flight + // (IRQs coalesced under high CPU load), e.g.: + // - Status IN/OUT finished, its IRQ and the new SETUP IRQ arrive at the same time. + // - Data IN finished and status OUT is received, both IRQs and the new SETUP IRQ arrive at the same time. + // Save the SETUP; it is replayed only once the old transfer is fully retired — i.e. when usbd has + // made (or already made) its final edpt0_xfer() call for it. case PIPE0_STATE_DATA_IN: case PIPE0_STATE_STATUS_OUT: - case PIPE0_STATE_STATUS_OUT_PENDING: + case PIPE0_STATE_STATUS_OUT_PENDING_XFER: + case PIPE0_STATE_STATUS_OUT_PENDING_IRQ: case PIPE0_STATE_STATUS_IN: { TU_VERIFY(pipe0_read_setup(musb_regs, ep_csr, &_dcd.pipe0.deferred_setup), ); _dcd.pipe0.deferred_setup_valid = true; switch (_dcd.pipe0.state) { case PIPE0_STATE_DATA_IN: - // Last DATA IN packet sent (TXRDY-clear coalesced with the SETUP IRQ). The STATUS OUT - // confirm IRQ is missed too — promote so edpt0_xfer(STATUS OUT) fires complete immediately. - if (_dcd.pipe0.remain_wlength == 0) { - _dcd.pipe0.state = PIPE0_STATE_STATUS_OUT_PENDING; - } + // Coalesced: last DATA IN sent + status OUT done + new SETUP in one csrl read. Fire the + // DATA IN completion and synthesize the missed status confirm; usbd's edpt0_xfer(STATUS OUT) + // fires the status completion and replays. + TU_ASSERT(_dcd.pipe0.remain_wlength == 0, ); + _dcd.pipe0.state = PIPE0_STATE_STATUS_OUT_PENDING_IRQ; dcd_event_xfer_complete(rhport, TU_EP0_IN, _dcd.pipe0.xact_len, XFER_RESULT_SUCCESS, true); break; case PIPE0_STATE_STATUS_OUT: // Status confirm IRQ coalesced with the SETUP — edpt0_xfer(STATUS OUT) fires complete. - _dcd.pipe0.state = PIPE0_STATE_STATUS_OUT_PENDING; + _dcd.pipe0.state = PIPE0_STATE_STATUS_OUT_PENDING_IRQ; break; - case PIPE0_STATE_STATUS_OUT_PENDING: - // edpt0_xfer(STATUS OUT) already called — fire complete and replay now. + case PIPE0_STATE_STATUS_OUT_PENDING_XFER: + // edpt0_xfer(STATUS OUT) already called — old transfer retired, complete and replay 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_OUT_PENDING_IRQ: + // usbd has not called edpt0_xfer(STATUS OUT) for the old transfer yet — only hold the + // SETUP. Replaying here would let that still-outstanding call land in the replayed + // transfer's state and corrupt it (e.g. NULL DATA OUT drain buffer). + break; + default: // PIPE0_STATE_STATUS_IN: ZLP-sent IRQ coalesced with the SETUP. if (_dcd.pipe0.pending_addr) { @@ -573,27 +585,26 @@ static void process_ep0(uint8_t rhport) { // 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: // First event of the STATUS OUT pair — wait for edpt0_xfer(STATUS OUT) to fire complete. - _dcd.pipe0.state = PIPE0_STATE_STATUS_OUT_PENDING; + _dcd.pipe0.state = PIPE0_STATE_STATUS_OUT_PENDING_IRQ; break; - case PIPE0_STATE_STATUS_OUT_PENDING: + case PIPE0_STATE_STATUS_OUT_PENDING_XFER: // 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_OUT_PENDING_IRQ: + // Stale duplicate of the status confirm — already accounted for; edpt0_xfer fires complete. + break; + case PIPE0_STATE_STATUS_IN: if (_dcd.pipe0.pending_addr) { musb_regs->faddr = _dcd.pipe0.pending_addr; From 3b73ee7e926d228bfa6ea4961d15a11a0310ed44 Mon Sep 17 00:00:00 2001 From: hathach Date: Sat, 13 Jun 2026 00:22:03 +0700 Subject: [PATCH 06/17] dcd/musb: gate stale EP0 RXRDY interrupts with rxrdy_consumed The deferral path drains the SETUP but leaves RxPktRdy set, and the SETUP's IRQ latches after the ISR's clear-on-read intr_tx read - so a second process_ep0 pass (same ISR, via the intr_tx re-read merge) is guaranteed and misreads the leftovers: count0==0 fires a spurious DATA OUT completion, the replay's RXRDYC write turns the second pass into a phantom csrl==0 DATA IN completion, and a zero-length replay re-enters the deferral case on a drained FIFO (count0 assert or garbage saved as a SETUP). The registers cannot expose the staleness: RxPktRdy and count0 read unchanged until ServicedRxPktRdy is written. Track it in software: rxrdy_consumed means "RxPktRdy is set in hw but its packet was already consumed". Set wherever a drained packet's RXRDY is intentionally left set (OUT/zero-length flow-control parks, every DATA OUT drain awaiting the next arm, the deferral path); cleared at every RXRDYC write site (edpt0_xfer arms, dcd_set_address, STALLED/SETEND recovery, bus reset). The RXRDY block returns early while parked. Replayed IN requests skip the RXRDYC in pipe0_start_setup and keep the packet parked until the edpt0_xfer(DATA IN) arm acks it (before loading the shared FIFO), so the stale pass sees RXRDY+parked instead of csrl==0. The normal IDLE path is unchanged - master never re-entered these windows because the single SETUP edge was always consumed by the pass that parked it; the deferral is what introduced a pending second pass. Review follow-up for #3643 (dcd_musb.c l.516 finding). Co-Authored-By: Claude Fable 5 --- src/portable/mentor/musb/dcd_musb.c | 36 +++++++++++++++++++++++++---- 1 file changed, 32 insertions(+), 4 deletions(-) diff --git a/src/portable/mentor/musb/dcd_musb.c b/src/portable/mentor/musb/dcd_musb.c index 2c31e3b2fd..0485c4374f 100644 --- a/src/portable/mentor/musb/dcd_musb.c +++ b/src/portable/mentor/musb/dcd_musb.c @@ -99,6 +99,8 @@ typedef struct { 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; + bool rxrdy_consumed; // RxPktRdy left set in hw for an already-consumed packet (NAK flow control); + // RXRDY events are stale while set. Cleared when RXRDYC is written. } pipe0; pipe_state_t pipe[MUSB_PIPE_COUNT]; } dcd_data_t; @@ -123,16 +125,23 @@ static void pipe0_start_setup(uint8_t rhport, musb_ep_csr_t* ep_csr, _dcd.pipe0.remain_wlength = req->wLength; if (req->wLength == 0) { + // Leave RXRDY set; edpt0_xfer(STATUS IN) acks it together with DATAEND. _dcd.pipe0.state = PIPE0_STATE_STATUS_IN; + _dcd.pipe0.rxrdy_consumed = true; } else { if (req->bmRequestType & TUSB_DIR_IN_MASK) { _dcd.pipe0.state = PIPE0_STATE_DATA_IN; - ep_csr->csr0l = MUSB_CSRL0_RXRDYC; + // On a deferred replay the packet's RXRDY stays parked until the edpt0_xfer(DATA IN) arm + // acks it — a stale latched EP0 IRQ in between is gated by rxrdy_consumed. + if (!_dcd.pipe0.rxrdy_consumed) { + ep_csr->csr0l = MUSB_CSRL0_RXRDYC; + } } else { // If OUT (rx) direction, let edpt0_xfer() clear RXRDY when it's ready to receive data. // Deliberate deviation from the databook's canonical flow (ack right after unload), // used as NAK flow control until usbd arms the drain buffer. _dcd.pipe0.state = PIPE0_STATE_DATA_OUT; + _dcd.pipe0.rxrdy_consumed = true; } } @@ -412,6 +421,11 @@ static bool edpt0_xfer(uint8_t rhport, uint8_t ep_addr, uint8_t *buffer, uint16_ case PIPE0_STATE_DATA_OUT: { _dcd.pipe0.xact_len = total_bytes; if (dir_in) { + // Replayed SETUP keeps its RXRDY parked until here; ack it before loading the shared FIFO. + if (_dcd.pipe0.rxrdy_consumed) { + ep_csr->csr0l = MUSB_CSRL0_RXRDYC; + _dcd.pipe0.rxrdy_consumed = false; + } // DATA IN: load FIFO, set TXRDY. Add DATAEND on the last chunk // (ep0_remain_datalen == 0 after this load) to end the data stage. tu_hwfifo_write(&musb_regs->fifo[0], buffer, total_bytes, NULL); @@ -425,6 +439,7 @@ static bool edpt0_xfer(uint8_t rhport, uint8_t ep_addr, uint8_t *buffer, uint16_ // DATA OUT: arm drain target, ack RXRDY so host can send DATA OUT. _dcd.pipe0.buf = buffer; ep_csr->csr0l = MUSB_CSRL0_RXRDYC; + _dcd.pipe0.rxrdy_consumed = false; } break; } @@ -432,6 +447,7 @@ static bool edpt0_xfer(uint8_t rhport, uint8_t ep_addr, uint8_t *buffer, uint16_ case PIPE0_STATE_STATUS_IN: TU_ASSERT(dir_in && total_bytes == 0); // only STATUS IN allowed ep_csr->csr0l = MUSB_CSRL0_RXRDYC | MUSB_CSRL0_DATAEND; + _dcd.pipe0.rxrdy_consumed = false; break; case PIPE0_STATE_STATUS_OUT: @@ -465,6 +481,7 @@ static void process_ep0(uint8_t rhport) { ep_csr->csr0l = 0; _dcd.pipe0.state = PIPE0_STATE_IDLE; _dcd.pipe0.deferred_setup_valid = false; + _dcd.pipe0.rxrdy_consumed = false; return; } @@ -474,6 +491,7 @@ static void process_ep0(uint8_t rhport) { ep_csr->csr0l = MUSB_CSRL0_SETENDC; _dcd.pipe0.state = PIPE0_STATE_IDLE; _dcd.pipe0.deferred_setup_valid = false; + _dcd.pipe0.rxrdy_consumed = false; if (!(csrl & MUSB_CSRL0_RXRDY)) { return; /* no SETUP waiting behind it */ } @@ -481,6 +499,9 @@ static void process_ep0(uint8_t rhport) { // Receive Data (Setup or OUT) if (csrl & MUSB_CSRL0_RXRDY) { + if (_dcd.pipe0.rxrdy_consumed) { + return; // stale latched IRQ: this RXRDY's packet was already drained + } switch (_dcd.pipe0.state) { case PIPE0_STATE_IDLE: { tusb_control_request_t req; @@ -498,8 +519,10 @@ static void process_ep0(uint8_t rhport) { tu_hwfifo_read(&musb_regs->fifo[0], _dcd.pipe0.buf, count0, NULL); _dcd.pipe0.remain_wlength -= count0; } + // RXRDY stays set until the next edpt0_xfer arm acks it (NAK flow control): + // edpt0_xfer(DATA OUT) for a mid-stream packet, edpt0_xfer(STATUS IN) for the last. + _dcd.pipe0.rxrdy_consumed = true; 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; } dcd_event_xfer_complete(rhport, TU_EP0_OUT, count0, XFER_RESULT_SUCCESS, true); @@ -511,7 +534,8 @@ static void process_ep0(uint8_t rhport) { // - Status IN/OUT finished, its IRQ and the new SETUP IRQ arrive at the same time. // - Data IN finished and status OUT is received, both IRQs and the new SETUP IRQ arrive at the same time. // Save the SETUP; it is replayed only once the old transfer is fully retired — i.e. when usbd has - // made (or already made) its final edpt0_xfer() call for it. + // made (or already made) its final edpt0_xfer() call for it. Until the replayed + // packet is acked, its RXRDY stays parked so a stale latched EP0 IRQ cannot re-process it. case PIPE0_STATE_DATA_IN: case PIPE0_STATE_STATUS_OUT: case PIPE0_STATE_STATUS_OUT_PENDING_XFER: @@ -519,6 +543,7 @@ static void process_ep0(uint8_t rhport) { case PIPE0_STATE_STATUS_IN: { TU_VERIFY(pipe0_read_setup(musb_regs, ep_csr, &_dcd.pipe0.deferred_setup), ); _dcd.pipe0.deferred_setup_valid = true; + _dcd.pipe0.rxrdy_consumed = true; switch (_dcd.pipe0.state) { case PIPE0_STATE_DATA_IN: @@ -549,7 +574,8 @@ static void process_ep0(uint8_t rhport) { break; default: - // PIPE0_STATE_STATUS_IN: ZLP-sent IRQ coalesced with the SETUP. + // PIPE0_STATE_STATUS_IN: rxrdy_consumed gate + SetupEnd guarantee DATAEND was armed, i.e. + // usbd already made its status call; the ZLP-sent IRQ coalesced with the SETUP. if (_dcd.pipe0.pending_addr) { musb_regs->faddr = _dcd.pipe0.pending_addr; _dcd.pipe0.pending_addr = 0; @@ -633,6 +659,7 @@ static void process_bus_reset(uint8_t rhport) { _dcd.pipe0.xact_len = 0; _dcd.pipe0.remain_wlength = 0; _dcd.pipe0.deferred_setup_valid = false; + _dcd.pipe0.rxrdy_consumed = false; musb->intr_txen = 1; /* Enable only EP0 */ musb->intr_rxen = 0; @@ -708,6 +735,7 @@ void dcd_set_address(uint8_t rhport, uint8_t dev_addr) _dcd.pipe0.state = PIPE0_STATE_STATUS_IN; /* Send STATUS IN ZLP with DATAEND; host ACK fires the confirmation IRQ. */ ep_csr->csr0l = MUSB_CSRL0_RXRDYC | MUSB_CSRL0_DATAEND; + _dcd.pipe0.rxrdy_consumed = false; } // Wake up host From e47eabd49d1838dc2a4eb42b9bd167a1d24d7cf5 Mon Sep 17 00:00:00 2001 From: hathach Date: Sat, 13 Jun 2026 00:22:28 +0700 Subject: [PATCH 07/17] dcd/musb: replay deferred SETUP instead of stalling EP0 dcd_edpt_stall(EP0 OUT) discarded the deferred SETUP and armed SendStall. A deferred SETUP can only exist once the old transfer's status stage was seen on the wire, so the request usbd is rejecting (class callback failing at CONTROL_STAGE_DATA) already succeeded host-side and the hardware already ACKed the next SETUP - the STALL would land on that innocent request, which then fails host-side without any tud callback ever seeing it. Skip the stall and replay the deferred SETUP; the rejected transfer needs no wire-level stall since it is already over. Review follow-up for #3643 (dcd_musb.c l.860 finding). Co-Authored-By: Claude Fable 5 --- src/portable/mentor/musb/dcd_musb.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/src/portable/mentor/musb/dcd_musb.c b/src/portable/mentor/musb/dcd_musb.c index 0485c4374f..15928ec241 100644 --- a/src/portable/mentor/musb/dcd_musb.c +++ b/src/portable/mentor/musb/dcd_musb.c @@ -534,7 +534,7 @@ static void process_ep0(uint8_t rhport) { // - Status IN/OUT finished, its IRQ and the new SETUP IRQ arrive at the same time. // - Data IN finished and status OUT is received, both IRQs and the new SETUP IRQ arrive at the same time. // Save the SETUP; it is replayed only once the old transfer is fully retired — i.e. when usbd has - // made (or already made) its final edpt0_xfer() call for it. Until the replayed + // made (or already made) its final edpt0_xfer()/dcd_edpt_stall() call for it. Until the replayed // packet is acked, its RXRDY stays parked so a stale latched EP0 IRQ cannot re-process it. case PIPE0_STATE_DATA_IN: case PIPE0_STATE_STATUS_OUT: @@ -935,11 +935,17 @@ void dcd_edpt_stall(uint8_t rhport, uint8_t ep_addr) { musb_ep_csr_t* ep_csr = get_ep_csr(musb_regs, epn); if (0 == epn) { - if (ep_addr == TU_EP0_OUT) { /* Ignore EP0 OUT */ + if (ep_addr == TU_EP0_OUT) { /* Ignore EP0 IN */ _dcd.pipe0.state = PIPE0_STATE_IDLE; _dcd.pipe0.buf = NULL; - _dcd.pipe0.deferred_setup_valid = false; - ep_csr->csr0l = MUSB_CSRL0_STALL; + if (_dcd.pipe0.deferred_setup_valid) { + // The transfer being stalled already completed on the wire (a deferred SETUP can only exist + // once its status stage was seen) and the host's next request was already ACKed — SendStall + // would land on that innocent request. Skip the stall and replay the deferred SETUP instead. + pipe0_process_deferred_setup(rhport, ep_csr, false); + } else { + ep_csr->csr0l = MUSB_CSRL0_STALL; + } } } else { const tusb_dir_t ep_dir = tu_edpt_dir(ep_addr); From c8c63c30617d23bb604affcad423acbb11f8d10d Mon Sep 17 00:00:00 2001 From: hathach Date: Sat, 13 Jun 2026 23:26:23 +0700 Subject: [PATCH 08/17] dcd/musb: clear rxrdy_consumed when stalling EP0 The actual-STALL path (no deferred SETUP) forced EP0 to IDLE but left rxrdy_consumed set if the aborted transfer had parked RXRDY via NAK flow control (e.g. a rejected OUT-data request in DATA_OUT). A subsequent SETUP IRQ would then hit the parked-gate early return and be ignored, relying on SentStall/SetupEnd to clear the flag first. Clear it here so recovery never depends on that ordering. Addresses Copilot review on #3699. Co-Authored-By: Claude Fable 5 --- src/portable/mentor/musb/dcd_musb.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/portable/mentor/musb/dcd_musb.c b/src/portable/mentor/musb/dcd_musb.c index 15928ec241..5f0ac4546e 100644 --- a/src/portable/mentor/musb/dcd_musb.c +++ b/src/portable/mentor/musb/dcd_musb.c @@ -944,6 +944,9 @@ void dcd_edpt_stall(uint8_t rhport, uint8_t ep_addr) { // would land on that innocent request. Skip the stall and replay the deferred SETUP instead. pipe0_process_deferred_setup(rhport, ep_csr, false); } else { + // Forcing EP0 to IDLE: any RXRDY parked by the aborted transfer's flow control is stale, + // clear it so the next SETUP IRQ is not gated off. + _dcd.pipe0.rxrdy_consumed = false; ep_csr->csr0l = MUSB_CSRL0_STALL; } } From ff57edb3e5e8d36b2783971924bf7373b4fc39bc Mon Sep 17 00:00:00 2001 From: hathach Date: Sat, 13 Jun 2026 23:35:35 +0700 Subject: [PATCH 09/17] hil: make serial write timeout fatal MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit pyserial's posix write() raises SerialTimeoutException after partial progress with the byte count lost, so the retry loop re-sent from the same offset and could duplicate bytes on the wire — surfacing as bogus data-mismatch failures that look like device firmware bugs. write_timeout is already a total per-call deadline, so the loop added duplication risk without extending the budget: write once and treat a timeout as fatal. Default bumped 2 -> 10 s to keep the old overall bound; HIL_SERIAL_WRITE_DEADLINE removed. The per-character CLI loops keep their existing pacing (the 2 ms sleep between single-byte writes already spaces them on the wire); no unbounded ser.flush()/tcdrain is added. Review follow-up for #3643 (hil_test.py l.257/264 findings). Co-Authored-By: Claude Fable 5 --- test/hil/hil_test.py | 29 +++++++++-------------------- 1 file changed, 9 insertions(+), 20 deletions(-) diff --git a/test/hil/hil_test.py b/test/hil/hil_test.py index c2ba34f021..aee59e05a8 100755 --- a/test/hil/hil_test.py +++ b/test/hil/hil_test.py @@ -145,8 +145,7 @@ 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')) +SERIAL_WRITE_TIMEOUT = float(os.getenv('HIL_SERIAL_WRITE_TIMEOUT', '10')) def cmd_stdout_text(out: Any) -> str: @@ -247,24 +246,14 @@ 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 serial_write_all(ser: serial.Serial, data: bytes): + # write_timeout is a total deadline for the whole call (pyserial keeps partial progress + # internally). A timeout means the device stopped draining — treat it as fatal: pyserial + # loses the partial-write count on raise, so retrying would duplicate bytes on the wire. + try: + ser.write(data) + except serial.SerialTimeoutException: + raise AssertionError(f'Serial write timeout after {SERIAL_WRITE_TIMEOUT:.1f}s') def read_disk_file(uid: str, lun: int, fname: str) -> bytes: From 3bdf52fc1ba06eb96ca28e9c3656b0d6a59cd201 Mon Sep 17 00:00:00 2001 From: hathach Date: Mon, 15 Jun 2026 14:53:33 +0700 Subject: [PATCH 10/17] dcd/musb: name pipe0_state_t, use local pointer, group struct fields MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pure cleanup, no behavior change: - Extract the EP0 control-transfer state into a named pipe0_state_t typedef instead of an anonymous nested struct, and access it through a local pipe0_state_t* in the functions that touch it repeatedly. - Group the pipe0 fields so the two bools sit together and the larger tusb_control_request_t deferred_setup is last. - Reword the deferral comments: "coalesced" -> "combined". Note: separating the edpt0_xfer DATA_IN/DATA_OUT case (dispatch on state instead of dir_in) was attempted and reverted — it breaks ADI MUSB enumeration. usbd can arm the opposite-direction status while pipe0 is still in a DATA state, and only dir-dispatch routes that correctly; a comment on the combined case records this. Verified: HIL pass on ek_tm4c123gxl and max32666fthr (13/13 each). Co-Authored-By: Claude Fable 5 --- src/portable/mentor/musb/dcd_musb.c | 197 +++++++++++++++------------- 1 file changed, 105 insertions(+), 92 deletions(-) diff --git a/src/portable/mentor/musb/dcd_musb.c b/src/portable/mentor/musb/dcd_musb.c index 5f0ac4546e..f52ac10e34 100644 --- a/src/portable/mentor/musb/dcd_musb.c +++ b/src/portable/mentor/musb/dcd_musb.c @@ -90,18 +90,21 @@ enum { PIPE0_STATE_STATUS_OUT_PENDING_IRQ, // confirmation IRQ seen (or synthesized) first; edpt0_xfer(STATUS OUT) fires xfer_complete }; +// EP0 control-transfer state (own scalars, not a pipe[] slot). typedef struct { - struct { - uint8_t *buf; // DATA OUT drain target (only valid while EP0 is in DATA OUT stage) - uint16_t xact_len; // chunk length most recently armed via edpt0_xfer; reported in xfer_complete - 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; - bool rxrdy_consumed; // RxPktRdy left set in hw for an already-consumed packet (NAK flow control); - // RXRDY events are stale while set. Cleared when RXRDYC is written. - } pipe0; + uint8_t *buf; // DATA OUT drain target (only valid while EP0 is in DATA OUT stage) + uint16_t xact_len; // chunk length most recently armed via edpt0_xfer; reported in xfer_complete + 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 + bool rxrdy_consumed; // RxPktRdy left set in hw for an already-consumed packet (NAK flow control); + // RXRDY events are stale while set. Cleared when RXRDYC is written. + bool deferred_setup_valid; + tusb_control_request_t deferred_setup; +} pipe0_state_t; + +typedef struct { + pipe0_state_t pipe0; pipe_state_t pipe[MUSB_PIPE_COUNT]; } dcd_data_t; @@ -122,26 +125,27 @@ static bool pipe0_read_setup(musb_regs_t* musb_regs, musb_ep_csr_t* ep_csr, tusb 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; + pipe0_state_t* pipe0 = &_dcd.pipe0; + pipe0->remain_wlength = req->wLength; if (req->wLength == 0) { // Leave RXRDY set; edpt0_xfer(STATUS IN) acks it together with DATAEND. - _dcd.pipe0.state = PIPE0_STATE_STATUS_IN; - _dcd.pipe0.rxrdy_consumed = true; + pipe0->state = PIPE0_STATE_STATUS_IN; + pipe0->rxrdy_consumed = true; } else { if (req->bmRequestType & TUSB_DIR_IN_MASK) { - _dcd.pipe0.state = PIPE0_STATE_DATA_IN; + pipe0->state = PIPE0_STATE_DATA_IN; // On a deferred replay the packet's RXRDY stays parked until the edpt0_xfer(DATA IN) arm // acks it — a stale latched EP0 IRQ in between is gated by rxrdy_consumed. - if (!_dcd.pipe0.rxrdy_consumed) { + if (!pipe0->rxrdy_consumed) { ep_csr->csr0l = MUSB_CSRL0_RXRDYC; } } else { // If OUT (rx) direction, let edpt0_xfer() clear RXRDY when it's ready to receive data. // Deliberate deviation from the databook's canonical flow (ack right after unload), // used as NAK flow control until usbd arms the drain buffer. - _dcd.pipe0.state = PIPE0_STATE_DATA_OUT; - _dcd.pipe0.rxrdy_consumed = true; + pipe0->state = PIPE0_STATE_DATA_OUT; + pipe0->rxrdy_consumed = true; } } @@ -149,12 +153,13 @@ static void pipe0_start_setup(uint8_t rhport, musb_ep_csr_t* ep_csr, } static void pipe0_process_deferred_setup(uint8_t rhport, musb_ep_csr_t* ep_csr, bool is_isr) { - if (!_dcd.pipe0.deferred_setup_valid) { + pipe0_state_t* pipe0 = &_dcd.pipe0; + if (!pipe0->deferred_setup_valid) { return; } - _dcd.pipe0.deferred_setup_valid = false; - pipe0_start_setup(rhport, ep_csr, &_dcd.pipe0.deferred_setup, is_isr); + pipe0->deferred_setup_valid = false; + pipe0_start_setup(rhport, ep_csr, &pipe0->deferred_setup, is_isr); } // EP0 must not call this — it has its own scalars in dcd_data_t. @@ -414,32 +419,36 @@ static bool edpt0_xfer(uint8_t rhport, uint8_t ep_addr, uint8_t *buffer, uint16_ TU_ASSERT(total_bytes <= CFG_TUD_ENDPOINT0_SIZE); /* EP0 only supports 1 packet per dcd_edpt_xfer()*/ musb_regs_t* musb_regs = MUSB_REGS(rhport); musb_ep_csr_t* ep_csr = get_ep_csr(musb_regs, 0); + pipe0_state_t* pipe0 = &_dcd.pipe0; const unsigned dir_in = tu_edpt_dir(ep_addr); - switch (_dcd.pipe0.state) { + switch (pipe0->state) { + // Combined: usbd can arm the opposite-direction status/ZLP while pipe0 is still in a DATA + // state, so dispatch on the call direction (dir_in), not the state. (Splitting into separate + // DATA_IN/DATA_OUT cases mis-routes those dir != state calls and breaks ADI MUSB.) case PIPE0_STATE_DATA_IN: case PIPE0_STATE_DATA_OUT: { - _dcd.pipe0.xact_len = total_bytes; + pipe0->xact_len = total_bytes; if (dir_in) { // Replayed SETUP keeps its RXRDY parked until here; ack it before loading the shared FIFO. - if (_dcd.pipe0.rxrdy_consumed) { + if (pipe0->rxrdy_consumed) { ep_csr->csr0l = MUSB_CSRL0_RXRDYC; - _dcd.pipe0.rxrdy_consumed = false; + pipe0->rxrdy_consumed = false; } // DATA IN: load FIFO, set TXRDY. Add DATAEND on the last chunk - // (ep0_remain_datalen == 0 after this load) to end the data stage. + // (remain_wlength == 0 after this load) to end the data stage. tu_hwfifo_write(&musb_regs->fifo[0], buffer, total_bytes, NULL); - _dcd.pipe0.remain_wlength -= total_bytes; - if (_dcd.pipe0.remain_wlength == 0) { + pipe0->remain_wlength -= total_bytes; + if (pipe0->remain_wlength == 0) { ep_csr->csr0l = MUSB_CSRL0_TXRDY | MUSB_CSRL0_DATAEND; } else { ep_csr->csr0l = MUSB_CSRL0_TXRDY; } } else { // DATA OUT: arm drain target, ack RXRDY so host can send DATA OUT. - _dcd.pipe0.buf = buffer; + pipe0->buf = buffer; ep_csr->csr0l = MUSB_CSRL0_RXRDYC; - _dcd.pipe0.rxrdy_consumed = false; + pipe0->rxrdy_consumed = false; } break; } @@ -447,19 +456,19 @@ static bool edpt0_xfer(uint8_t rhport, uint8_t ep_addr, uint8_t *buffer, uint16_ case PIPE0_STATE_STATUS_IN: TU_ASSERT(dir_in && total_bytes == 0); // only STATUS IN allowed ep_csr->csr0l = MUSB_CSRL0_RXRDYC | MUSB_CSRL0_DATAEND; - _dcd.pipe0.rxrdy_consumed = false; + pipe0->rxrdy_consumed = false; break; case PIPE0_STATE_STATUS_OUT: TU_ASSERT(!dir_in && total_bytes == 0); // only STATUS OUT allowed // First event of the STATUS OUT pair — wait for the IRQ to fire complete. - _dcd.pipe0.state = PIPE0_STATE_STATUS_OUT_PENDING_XFER; + pipe0->state = PIPE0_STATE_STATUS_OUT_PENDING_XFER; break; case PIPE0_STATE_STATUS_OUT_PENDING_IRQ: // Second event — IRQ already arrived, fire complete now. The old transfer is retired here, // so a deferred SETUP can be replayed safely. - _dcd.pipe0.state = PIPE0_STATE_IDLE; + 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; @@ -474,14 +483,15 @@ static bool edpt0_xfer(uint8_t rhport, uint8_t ep_addr, uint8_t *buffer, uint16_ static void process_ep0(uint8_t rhport) { musb_regs_t* musb_regs = MUSB_REGS(rhport); musb_ep_csr_t* ep_csr = get_ep_csr(musb_regs, 0); + pipe0_state_t* pipe0 = &_dcd.pipe0; uint_fast8_t csrl = ep_csr->csr0l; // 21.1.5: SentStall and SetupEnd must be checked before anything else. if (csrl & MUSB_CSRL0_STALLED) { ep_csr->csr0l = 0; - _dcd.pipe0.state = PIPE0_STATE_IDLE; - _dcd.pipe0.deferred_setup_valid = false; - _dcd.pipe0.rxrdy_consumed = false; + pipe0->state = PIPE0_STATE_IDLE; + pipe0->deferred_setup_valid = false; + pipe0->rxrdy_consumed = false; return; } @@ -489,9 +499,9 @@ static void process_ep0(uint8_t rhport) { // Host aborted the current control transfer (new SETUP or premature STATUS). // 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; - _dcd.pipe0.rxrdy_consumed = false; + pipe0->state = PIPE0_STATE_IDLE; + pipe0->deferred_setup_valid = false; + pipe0->rxrdy_consumed = false; if (!(csrl & MUSB_CSRL0_RXRDY)) { return; /* no SETUP waiting behind it */ } @@ -499,10 +509,10 @@ static void process_ep0(uint8_t rhport) { // Receive Data (Setup or OUT) if (csrl & MUSB_CSRL0_RXRDY) { - if (_dcd.pipe0.rxrdy_consumed) { + if (pipe0->rxrdy_consumed) { return; // stale latched IRQ: this RXRDY's packet was already drained } - switch (_dcd.pipe0.state) { + switch (pipe0->state) { case PIPE0_STATE_IDLE: { tusb_control_request_t req; TU_VERIFY(pipe0_read_setup(musb_regs, ep_csr, &req), ); @@ -515,22 +525,22 @@ static void process_ep0(uint8_t rhport) { // so the whole packet drains in one shot. const uint16_t count0 = ep_csr->count0; if (count0) { - TU_ASSERT(_dcd.pipe0.buf, ); - tu_hwfifo_read(&musb_regs->fifo[0], _dcd.pipe0.buf, count0, NULL); - _dcd.pipe0.remain_wlength -= count0; + TU_ASSERT(pipe0->buf, ); + tu_hwfifo_read(&musb_regs->fifo[0], pipe0->buf, count0, NULL); + pipe0->remain_wlength -= count0; } // RXRDY stays set until the next edpt0_xfer arm acks it (NAK flow control): // edpt0_xfer(DATA OUT) for a mid-stream packet, edpt0_xfer(STATUS IN) for the last. - _dcd.pipe0.rxrdy_consumed = true; - if (_dcd.pipe0.remain_wlength == 0) { - _dcd.pipe0.state = PIPE0_STATE_STATUS_IN; + pipe0->rxrdy_consumed = true; + if (pipe0->remain_wlength == 0) { + pipe0->state = PIPE0_STATE_STATUS_IN; } dcd_event_xfer_complete(rhport, TU_EP0_OUT, count0, XFER_RESULT_SUCCESS, true); break; } // New SETUP packet arrived while the old control transfer's tail events are still in flight - // (IRQs coalesced under high CPU load), e.g.: + // (IRQs combined under high CPU load), e.g.: // - Status IN/OUT finished, its IRQ and the new SETUP IRQ arrive at the same time. // - Data IN finished and status OUT is received, both IRQs and the new SETUP IRQ arrive at the same time. // Save the SETUP; it is replayed only once the old transfer is fully retired — i.e. when usbd has @@ -541,28 +551,28 @@ static void process_ep0(uint8_t rhport) { case PIPE0_STATE_STATUS_OUT_PENDING_XFER: case PIPE0_STATE_STATUS_OUT_PENDING_IRQ: case PIPE0_STATE_STATUS_IN: { - TU_VERIFY(pipe0_read_setup(musb_regs, ep_csr, &_dcd.pipe0.deferred_setup), ); - _dcd.pipe0.deferred_setup_valid = true; - _dcd.pipe0.rxrdy_consumed = true; + TU_VERIFY(pipe0_read_setup(musb_regs, ep_csr, &pipe0->deferred_setup), ); + pipe0->deferred_setup_valid = true; + pipe0->rxrdy_consumed = true; - switch (_dcd.pipe0.state) { + switch (pipe0->state) { case PIPE0_STATE_DATA_IN: - // Coalesced: last DATA IN sent + status OUT done + new SETUP in one csrl read. Fire the + // Combined: last DATA IN sent + status OUT done + new SETUP in one csrl read. Fire the // DATA IN completion and synthesize the missed status confirm; usbd's edpt0_xfer(STATUS OUT) // fires the status completion and replays. - TU_ASSERT(_dcd.pipe0.remain_wlength == 0, ); - _dcd.pipe0.state = PIPE0_STATE_STATUS_OUT_PENDING_IRQ; - dcd_event_xfer_complete(rhport, TU_EP0_IN, _dcd.pipe0.xact_len, XFER_RESULT_SUCCESS, true); + TU_ASSERT(pipe0->remain_wlength == 0, ); + pipe0->state = PIPE0_STATE_STATUS_OUT_PENDING_IRQ; + dcd_event_xfer_complete(rhport, TU_EP0_IN, pipe0->xact_len, XFER_RESULT_SUCCESS, true); break; case PIPE0_STATE_STATUS_OUT: - // Status confirm IRQ coalesced with the SETUP — edpt0_xfer(STATUS OUT) fires complete. - _dcd.pipe0.state = PIPE0_STATE_STATUS_OUT_PENDING_IRQ; + // Status confirm IRQ combined with the SETUP — edpt0_xfer(STATUS OUT) fires complete. + pipe0->state = PIPE0_STATE_STATUS_OUT_PENDING_IRQ; break; case PIPE0_STATE_STATUS_OUT_PENDING_XFER: // edpt0_xfer(STATUS OUT) already called — old transfer retired, complete and replay now. - _dcd.pipe0.state = PIPE0_STATE_IDLE; + 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; @@ -575,12 +585,12 @@ static void process_ep0(uint8_t rhport) { default: // PIPE0_STATE_STATUS_IN: rxrdy_consumed gate + SetupEnd guarantee DATAEND was armed, i.e. - // usbd already made its status call; the ZLP-sent IRQ coalesced with the SETUP. - if (_dcd.pipe0.pending_addr) { - musb_regs->faddr = _dcd.pipe0.pending_addr; - _dcd.pipe0.pending_addr = 0; + // usbd already made its status call; the ZLP-sent IRQ combined with the SETUP. + if (pipe0->pending_addr) { + musb_regs->faddr = pipe0->pending_addr; + pipe0->pending_addr = 0; } - _dcd.pipe0.state = PIPE0_STATE_IDLE; + 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; @@ -604,25 +614,25 @@ static void process_ep0(uint8_t rhport) { /* 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) { + switch (pipe0->state) { 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; + // csrl == 0 in DATA IN = TXRDY just cleared, i.e. a DATA IN packet was successfully sent. If the + // just-sent packet was the last (DATAEND set when remain_wlength hit 0), transition to STATUS_OUT + // to await the host's STATUS-OUT ZLP confirmation IRQ. + if (pipe0->remain_wlength == 0) { + pipe0->state = PIPE0_STATE_STATUS_OUT; } - dcd_event_xfer_complete(rhport, TU_EP0_IN, _dcd.pipe0.xact_len, XFER_RESULT_SUCCESS, true); + dcd_event_xfer_complete(rhport, TU_EP0_IN, pipe0->xact_len, XFER_RESULT_SUCCESS, true); break; case PIPE0_STATE_STATUS_OUT: // First event of the STATUS OUT pair — wait for edpt0_xfer(STATUS OUT) to fire complete. - _dcd.pipe0.state = PIPE0_STATE_STATUS_OUT_PENDING_IRQ; + pipe0->state = PIPE0_STATE_STATUS_OUT_PENDING_IRQ; break; case PIPE0_STATE_STATUS_OUT_PENDING_XFER: // Second event — edpt0_xfer(STATUS OUT) already called, fire complete now. - _dcd.pipe0.state = PIPE0_STATE_IDLE; + 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; @@ -632,11 +642,11 @@ static void process_ep0(uint8_t rhport) { break; case PIPE0_STATE_STATUS_IN: - if (_dcd.pipe0.pending_addr) { - musb_regs->faddr = _dcd.pipe0.pending_addr; - _dcd.pipe0.pending_addr = 0; + if (pipe0->pending_addr) { + musb_regs->faddr = pipe0->pending_addr; + pipe0->pending_addr = 0; } - _dcd.pipe0.state = PIPE0_STATE_IDLE; + 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; @@ -654,12 +664,13 @@ static void process_bus_reset(uint8_t rhport) { alloced_fifo_bytes = CFG_TUD_ENDPOINT0_SIZE; #endif - _dcd.pipe0.state = PIPE0_STATE_IDLE; - _dcd.pipe0.buf = NULL; - _dcd.pipe0.xact_len = 0; - _dcd.pipe0.remain_wlength = 0; - _dcd.pipe0.deferred_setup_valid = false; - _dcd.pipe0.rxrdy_consumed = false; + pipe0_state_t* pipe0 = &_dcd.pipe0; + pipe0->state = PIPE0_STATE_IDLE; + pipe0->buf = NULL; + pipe0->xact_len = 0; + pipe0->remain_wlength = 0; + pipe0->deferred_setup_valid = false; + pipe0->rxrdy_consumed = false; musb->intr_txen = 1; /* Enable only EP0 */ musb->intr_rxen = 0; @@ -729,13 +740,14 @@ void dcd_set_address(uint8_t rhport, uint8_t dev_addr) musb_regs_t* musb_regs = MUSB_REGS(rhport); musb_ep_csr_t* ep_csr = get_ep_csr(musb_regs, 0); - _dcd.pipe0.pending_addr = dev_addr; - _dcd.pipe0.buf = NULL; - _dcd.pipe0.xact_len = 0; - _dcd.pipe0.state = PIPE0_STATE_STATUS_IN; + pipe0_state_t* pipe0 = &_dcd.pipe0; + pipe0->pending_addr = dev_addr; + pipe0->buf = NULL; + pipe0->xact_len = 0; + pipe0->state = PIPE0_STATE_STATUS_IN; /* Send STATUS IN ZLP with DATAEND; host ACK fires the confirmation IRQ. */ ep_csr->csr0l = MUSB_CSRL0_RXRDYC | MUSB_CSRL0_DATAEND; - _dcd.pipe0.rxrdy_consumed = false; + pipe0->rxrdy_consumed = false; } // Wake up host @@ -936,9 +948,10 @@ void dcd_edpt_stall(uint8_t rhport, uint8_t ep_addr) { if (0 == epn) { if (ep_addr == TU_EP0_OUT) { /* Ignore EP0 IN */ - _dcd.pipe0.state = PIPE0_STATE_IDLE; - _dcd.pipe0.buf = NULL; - if (_dcd.pipe0.deferred_setup_valid) { + pipe0_state_t* pipe0 = &_dcd.pipe0; + pipe0->state = PIPE0_STATE_IDLE; + pipe0->buf = NULL; + if (pipe0->deferred_setup_valid) { // The transfer being stalled already completed on the wire (a deferred SETUP can only exist // once its status stage was seen) and the host's next request was already ACKed — SendStall // would land on that innocent request. Skip the stall and replay the deferred SETUP instead. @@ -946,7 +959,7 @@ void dcd_edpt_stall(uint8_t rhport, uint8_t ep_addr) { } else { // Forcing EP0 to IDLE: any RXRDY parked by the aborted transfer's flow control is stale, // clear it so the next SETUP IRQ is not gated off. - _dcd.pipe0.rxrdy_consumed = false; + pipe0->rxrdy_consumed = false; ep_csr->csr0l = MUSB_CSRL0_STALL; } } From abc3114d53aee421a0a457ece561ac000ace2c67 Mon Sep 17 00:00:00 2001 From: hathach Date: Mon, 15 Jun 2026 16:42:10 +0700 Subject: [PATCH 11/17] dcd/musb: end EP0 IN data stage on short packet, split DATA case MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A short IN control response (device sends fewer bytes than wLength — e.g. the 18-byte device descriptor answering a 64-byte GET_DESCRIPTOR) left remain_wlength != 0, so the DATA_IN -> STATUS_OUT transition never fired and pipe0 stayed in DATA_IN through the status stage. usbd then armed the status-OUT while state was still DATA_IN. Set DATAEND and transition on the last packet: remain_wlength == 0, or a short packet (incl. a terminating ZLP) which ends the data stage. With state now tracking the stage, split edpt0_xfer's DATA handling into separate DATA_IN / DATA_OUT cases dispatching on state (asserting state == call direction) instead of the combined dir_in branch. Verified: HIL pass on ek_tm4c123gxl and max32666fthr (13/13 each), including the #3643 high-CPU-load IRQ-toggle coalescing stress. Co-Authored-By: Claude Fable 5 --- src/portable/mentor/musb/dcd_musb.c | 49 +++++++++++++---------------- 1 file changed, 22 insertions(+), 27 deletions(-) diff --git a/src/portable/mentor/musb/dcd_musb.c b/src/portable/mentor/musb/dcd_musb.c index f52ac10e34..261ff64e81 100644 --- a/src/portable/mentor/musb/dcd_musb.c +++ b/src/portable/mentor/musb/dcd_musb.c @@ -423,35 +423,31 @@ 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 (pipe0->state) { - // Combined: usbd can arm the opposite-direction status/ZLP while pipe0 is still in a DATA - // state, so dispatch on the call direction (dir_in), not the state. (Splitting into separate - // DATA_IN/DATA_OUT cases mis-routes those dir != state calls and breaks ADI MUSB.) + // DATA stage exits on its last packet, so state matches the call direction here. case PIPE0_STATE_DATA_IN: - case PIPE0_STATE_DATA_OUT: { + TU_ASSERT(dir_in); pipe0->xact_len = total_bytes; - if (dir_in) { - // Replayed SETUP keeps its RXRDY parked until here; ack it before loading the shared FIFO. - if (pipe0->rxrdy_consumed) { - ep_csr->csr0l = MUSB_CSRL0_RXRDYC; - pipe0->rxrdy_consumed = false; - } - // DATA IN: load FIFO, set TXRDY. Add DATAEND on the last chunk - // (remain_wlength == 0 after this load) to end the data stage. - tu_hwfifo_write(&musb_regs->fifo[0], buffer, total_bytes, NULL); - pipe0->remain_wlength -= total_bytes; - if (pipe0->remain_wlength == 0) { - ep_csr->csr0l = MUSB_CSRL0_TXRDY | MUSB_CSRL0_DATAEND; - } else { - ep_csr->csr0l = MUSB_CSRL0_TXRDY; - } - } else { - // DATA OUT: arm drain target, ack RXRDY so host can send DATA OUT. - pipe0->buf = buffer; + if (pipe0->rxrdy_consumed) { // replayed SETUP: ack its parked RXRDY before loading the FIFO ep_csr->csr0l = MUSB_CSRL0_RXRDYC; pipe0->rxrdy_consumed = false; } + tu_hwfifo_write(&musb_regs->fifo[0], buffer, total_bytes, NULL); + pipe0->remain_wlength -= total_bytes; + // DATAEND on the last packet: wLength met, or a short packet (incl. ZLP) ends the data stage. + if (pipe0->remain_wlength == 0 || total_bytes < CFG_TUD_ENDPOINT0_SIZE) { + ep_csr->csr0l = MUSB_CSRL0_TXRDY | MUSB_CSRL0_DATAEND; + } else { + ep_csr->csr0l = MUSB_CSRL0_TXRDY; + } + break; + + case PIPE0_STATE_DATA_OUT: + TU_ASSERT(!dir_in); + pipe0->xact_len = total_bytes; + pipe0->buf = buffer; // arm drain target, ack RXRDY so host can send DATA OUT + ep_csr->csr0l = MUSB_CSRL0_RXRDYC; + pipe0->rxrdy_consumed = false; break; - } case PIPE0_STATE_STATUS_IN: TU_ASSERT(dir_in && total_bytes == 0); // only STATUS IN allowed @@ -616,10 +612,9 @@ static void process_ep0(uint8_t rhport) { * - or status stage is complete (ZLP) after DataEnd is set */ switch (pipe0->state) { case PIPE0_STATE_DATA_IN: - // csrl == 0 in DATA IN = TXRDY just cleared, i.e. a DATA IN packet was successfully sent. If the - // just-sent packet was the last (DATAEND set when remain_wlength hit 0), transition to STATUS_OUT - // to await the host's STATUS-OUT ZLP confirmation IRQ. - if (pipe0->remain_wlength == 0) { + // DATA IN packet sent (TXRDY cleared). On the last packet (DATAEND condition above) move to + // STATUS_OUT to await the host's STATUS-OUT ZLP IRQ. + if (pipe0->remain_wlength == 0 || pipe0->xact_len < CFG_TUD_ENDPOINT0_SIZE) { pipe0->state = PIPE0_STATE_STATUS_OUT; } dcd_event_xfer_complete(rhport, TU_EP0_IN, pipe0->xact_len, XFER_RESULT_SUCCESS, true); From 9562f54f95da485c17192b07a9b2cc21126c8ef1 Mon Sep 17 00:00:00 2001 From: hathach Date: Mon, 15 Jun 2026 16:43:10 +0700 Subject: [PATCH 12/17] dcd/musb: suffix ISR-context process_* handlers with _isr Rename process_ep0/process_epin/process_epout/process_bus_reset (all invoked only from dcd_int_handler) to *_isr, making their ISR context explicit at every call site. pipe0_process_deferred_setup is left as-is since it also runs from task context (dcd_edpt_stall). Co-Authored-By: Claude Fable 5 --- src/portable/mentor/musb/dcd_musb.c | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/src/portable/mentor/musb/dcd_musb.c b/src/portable/mentor/musb/dcd_musb.c index 261ff64e81..eb5f83e738 100644 --- a/src/portable/mentor/musb/dcd_musb.c +++ b/src/portable/mentor/musb/dcd_musb.c @@ -299,7 +299,7 @@ static void pipe_write(musb_regs_t* musb_regs, pipe_state_t* pipe, uint8_t epnum // Called from the TX interrupt. If the last queued packet finished the transfer, // signal completion; otherwise queue the next packet. -static void process_epin(uint8_t rhport, musb_regs_t *musb_regs, uint8_t epnum) { +static void process_epin_isr(uint8_t rhport, musb_regs_t *musb_regs, uint8_t epnum) { musb_ep_csr_t* ep_csr = get_ep_csr(musb_regs, epnum); const uint_fast8_t csrl = ep_csr->tx_csrl; if (csrl & MUSB_TXCSRL1_STALLED) { @@ -329,7 +329,7 @@ static void process_epin(uint8_t rhport, musb_regs_t *musb_regs, uint8_t epnum) // Drain one packet from the Rx FIFO into pipe->buf/fifo, update pipe state, and // release the FIFO slot by clearing RXRDY. return true if short packet static bool pipe_read(musb_regs_t* musb_regs, pipe_state_t* pipe, uint8_t epnum) { - musb_ep_csr_t* ep_csr = &musb_regs->indexed_csr; // index already set in process_epout() + musb_ep_csr_t* ep_csr = &musb_regs->indexed_csr; // index already set in process_epout_isr() const uint16_t mps = ep_csr->rx_maxp & MUSB_RXMAXP_PACKET_SIZE_M; const uint16_t rx_count = ep_csr->rx_count; const uint16_t xact_len = tu_min16(tu_min16(pipe->remaining, mps), rx_count); @@ -348,7 +348,7 @@ static bool pipe_read(musb_regs_t* musb_regs, pipe_state_t* pipe, uint8_t epnum) return (xact_len < mps); } -static void process_epout(uint8_t rhport, musb_regs_t *musb_regs, uint8_t epnum, bool is_isr) { +static void process_epout_isr(uint8_t rhport, musb_regs_t *musb_regs, uint8_t epnum, bool is_isr) { musb_ep_csr_t* ep_csr = get_ep_csr(musb_regs, epnum); if (ep_csr->rx_csrl & MUSB_RXCSRL1_STALLED) { ep_csr->rx_csrl &= ~(MUSB_RXCSRL1_STALLED | MUSB_RXCSRL1_OVER); @@ -403,13 +403,13 @@ static bool edpt_n_xfer(uint8_t rhport, uint8_t ep_addr, void *buffer, uint16_t if (dir_in) { pipe_write(musb_regs, pipe, epnum); } else { - // Re-enable Rx interrupt (may have been masked by the no-buffer path in process_epout) + // Re-enable Rx interrupt (may have been masked by the no-buffer path in process_epout_isr) musb_regs->intr_rxen |= (uint16_t)TU_BIT(epnum); // Drain any packet staged in the Rx FIFO from a prior no-buffer interrupt. - // process_epout() fires dcd_event_xfer_complete() itself if the drain completes. + // process_epout_isr() fires dcd_event_xfer_complete() itself if the drain completes. if (ep_csr->rx_csrl & MUSB_RXCSRL1_RXRDY) { - process_epout(rhport, musb_regs, epnum, is_isr); + process_epout_isr(rhport, musb_regs, epnum, is_isr); } } return true; @@ -476,7 +476,7 @@ static bool edpt0_xfer(uint8_t rhport, uint8_t ep_addr, uint8_t *buffer, uint16_ } // 21.1.5: endpoint 0 service routine as peripheral -static void process_ep0(uint8_t rhport) { +static void process_ep0_isr(uint8_t rhport) { musb_regs_t* musb_regs = MUSB_REGS(rhport); musb_ep_csr_t* ep_csr = get_ep_csr(musb_regs, 0); pipe0_state_t* pipe0 = &_dcd.pipe0; @@ -652,7 +652,7 @@ static void process_ep0(uint8_t rhport) { // Upon BUS RESET is detected, hardware havs already done: // faddr = 0, index = 0, flushes all ep fifos, clears all ep csr, enabled all ep interrupts -static void process_bus_reset(uint8_t rhport) { +static void process_bus_reset_isr(uint8_t rhport) { musb_regs_t* musb = MUSB_REGS(rhport); #if MUSB_CFG_DYNAMIC_FIFO @@ -728,7 +728,7 @@ void dcd_int_disable(uint8_t rhport) { } // Receive Set Address request. Stash the new address here; hardware faddr is -// latched from pending_addr in process_ep0 once the STATUS IN completes (per +// latched from pending_addr in process_ep0_isr once the STATUS IN completes (per // USB spec, address must only take effect after the status stage). void dcd_set_address(uint8_t rhport, uint8_t dev_addr) { @@ -1008,7 +1008,7 @@ void dcd_int_handler(uint8_t rhport) { dcd_event_bus_signal(rhport, DCD_EVENT_SOF, true); } if (intr_usb & MUSB_IS_RESET) { - process_bus_reset(rhport); + process_bus_reset_isr(rhport); } if (intr_usb & MUSB_IS_RESUME) { dcd_event_bus_signal(rhport, DCD_EVENT_RESUME, true); @@ -1022,9 +1022,9 @@ void dcd_int_handler(uint8_t rhport) { while (intr_tx) { const unsigned epnum = __builtin_ctz(intr_tx); if (epnum == 0) { - process_ep0(rhport); // EP0 has its own state machine (control transfers) + process_ep0_isr(rhport); // EP0 has its own state machine (control transfers) } else { - process_epin(rhport, musb_regs, epnum); + process_epin_isr(rhport, musb_regs, epnum); } intr_tx &= ~TU_BIT(epnum); @@ -1039,7 +1039,7 @@ void dcd_int_handler(uint8_t rhport) { intr_rx &= musb_regs->intr_rxen; /* Clear disabled interrupts */ while (intr_rx) { unsigned const epnum = __builtin_ctz(intr_rx); - process_epout(rhport, musb_regs, epnum, true); + process_epout_isr(rhport, musb_regs, epnum, true); intr_rx &= ~TU_BIT(epnum); // Double packet endpoint: RxPktRdy is set and interrupt is generated immediately if 2nd packet is received From d4eeaf10cb1a95b6b74e18cd933c6320d31c2031 Mon Sep 17 00:00:00 2001 From: hathach Date: Mon, 15 Jun 2026 16:48:40 +0700 Subject: [PATCH 13/17] dcd/musb: extract pipe0_process_status_isr() to de-dup EP0 tail paths The deferral (RXRDY-combined) and csrl==0 tail paths in process_ep0_isr ran the same per-state status-stage logic. Move all of it into one pipe0_process_status_isr() helper covering every state including DATA_IN, which picks STATUS_OUT vs STATUS_OUT_PENDING_IRQ from deferred_setup_valid (a deferred SETUP means the status confirm was coalesced with it). Both callers now just invoke the helper; the deferral path saves the SETUP and sets deferred_setup_valid first. Also drops the deferral path's TU_ASSERT(remain_wlength == 0), which was wrong for a short last DATA-IN packet, and renames pipe0_process_deferred_setup -> pipe0_try_deferred_setup (it no-ops when nothing is deferred). Verified: HIL pass on ek_tm4c123gxl and max32666fthr (13/13 each), including the #3643 high-CPU-load IRQ-toggle coalescing stress. Co-Authored-By: Claude Fable 5 --- src/portable/mentor/musb/dcd_musb.c | 149 +++++++++++----------------- 1 file changed, 58 insertions(+), 91 deletions(-) diff --git a/src/portable/mentor/musb/dcd_musb.c b/src/portable/mentor/musb/dcd_musb.c index eb5f83e738..e769b08a57 100644 --- a/src/portable/mentor/musb/dcd_musb.c +++ b/src/portable/mentor/musb/dcd_musb.c @@ -152,7 +152,8 @@ static void pipe0_start_setup(uint8_t rhport, musb_ep_csr_t* ep_csr, 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) { +// Replay a previously deferred SETUP, if any. +static void pipe0_try_deferred_setup(uint8_t rhport, musb_ep_csr_t* ep_csr, bool is_isr) { pipe0_state_t* pipe0 = &_dcd.pipe0; if (!pipe0->deferred_setup_valid) { return; @@ -466,7 +467,7 @@ static bool edpt0_xfer(uint8_t rhport, uint8_t ep_addr, uint8_t *buffer, uint16_ // so a deferred SETUP can be replayed safely. 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); + pipe0_try_deferred_setup(rhport, ep_csr, is_isr); break; default: break; @@ -475,6 +476,52 @@ static bool edpt0_xfer(uint8_t rhport, uint8_t ep_addr, uint8_t *buffer, uint16_ return true; } +// Advance EP0's status-stage state machine on a tail event: the csrl==0 confirmation IRQ, or such a +// confirmation combined with a new SETUP (caller sets deferred_setup_valid first). ISR context only. +static void pipe0_process_status_isr(uint8_t rhport, musb_regs_t* musb_regs, musb_ep_csr_t* ep_csr) { + pipe0_state_t* pipe0 = &_dcd.pipe0; + switch (pipe0->state) { + case PIPE0_STATE_DATA_IN: + if (pipe0->remain_wlength == 0 || pipe0->xact_len < CFG_TUD_ENDPOINT0_SIZE) { // last DATA IN packet + if (pipe0->deferred_setup_valid) { + pipe0->state = PIPE0_STATE_STATUS_OUT_PENDING_IRQ; // status confirm coalesced with deferred SETUP + } else { + pipe0->state = PIPE0_STATE_STATUS_OUT; // await host's STATUS-OUT ZLP IRQ + } + } + dcd_event_xfer_complete(rhport, TU_EP0_IN, pipe0->xact_len, XFER_RESULT_SUCCESS, true); + break; + + case PIPE0_STATE_STATUS_OUT: + // Confirmation seen — await edpt0_xfer(STATUS OUT) to fire complete. + pipe0->state = PIPE0_STATE_STATUS_OUT_PENDING_IRQ; + break; + + case PIPE0_STATE_STATUS_OUT_PENDING_XFER: + // edpt0_xfer(STATUS OUT) already called — fire complete and replay now. + pipe0->state = PIPE0_STATE_IDLE; + dcd_event_xfer_complete(rhport, TU_EP0_OUT, 0, XFER_RESULT_SUCCESS, true); + pipe0_try_deferred_setup(rhport, ep_csr, true); + break; + + case PIPE0_STATE_STATUS_OUT_PENDING_IRQ: + // Confirmation already accounted for — the pairing edpt0_xfer(STATUS OUT) fires complete. + break; + + case PIPE0_STATE_STATUS_IN: + if (pipe0->pending_addr) { + musb_regs->faddr = pipe0->pending_addr; + pipe0->pending_addr = 0; + } + pipe0->state = PIPE0_STATE_IDLE; + dcd_event_xfer_complete(rhport, TU_EP0_IN, 0, XFER_RESULT_SUCCESS, true); + pipe0_try_deferred_setup(rhport, ep_csr, true); + break; + + default: break; + } +} + // 21.1.5: endpoint 0 service routine as peripheral static void process_ep0_isr(uint8_t rhport) { musb_regs_t* musb_regs = MUSB_REGS(rhport); @@ -535,64 +582,21 @@ static void process_ep0_isr(uint8_t rhport) { break; } - // New SETUP packet arrived while the old control transfer's tail events are still in flight - // (IRQs combined under high CPU load), e.g.: - // - Status IN/OUT finished, its IRQ and the new SETUP IRQ arrive at the same time. - // - Data IN finished and status OUT is received, both IRQs and the new SETUP IRQ arrive at the same time. - // Save the SETUP; it is replayed only once the old transfer is fully retired — i.e. when usbd has - // made (or already made) its final edpt0_xfer()/dcd_edpt_stall() call for it. Until the replayed - // packet is acked, its RXRDY stays parked so a stale latched EP0 IRQ cannot re-process it. + // New SETUP arrived while the old control transfer's tail events are still in flight (IRQs + // combined under high CPU load): the old transfer's status confirm and this SETUP land together. case PIPE0_STATE_DATA_IN: case PIPE0_STATE_STATUS_OUT: case PIPE0_STATE_STATUS_OUT_PENDING_XFER: case PIPE0_STATE_STATUS_OUT_PENDING_IRQ: - case PIPE0_STATE_STATUS_IN: { + case PIPE0_STATE_STATUS_IN: + // Save it, then finish the old transfer's tail event; deferred_setup_valid makes + // pipe0_process_status_isr() synthesize the coalesced status confirm and replay the SETUP + // once the old transfer is retired. Its RXRDY stays parked so a stale IRQ can't re-process it. TU_VERIFY(pipe0_read_setup(musb_regs, ep_csr, &pipe0->deferred_setup), ); pipe0->deferred_setup_valid = true; pipe0->rxrdy_consumed = true; - - switch (pipe0->state) { - case PIPE0_STATE_DATA_IN: - // Combined: last DATA IN sent + status OUT done + new SETUP in one csrl read. Fire the - // DATA IN completion and synthesize the missed status confirm; usbd's edpt0_xfer(STATUS OUT) - // fires the status completion and replays. - TU_ASSERT(pipe0->remain_wlength == 0, ); - pipe0->state = PIPE0_STATE_STATUS_OUT_PENDING_IRQ; - dcd_event_xfer_complete(rhport, TU_EP0_IN, pipe0->xact_len, XFER_RESULT_SUCCESS, true); - break; - - case PIPE0_STATE_STATUS_OUT: - // Status confirm IRQ combined with the SETUP — edpt0_xfer(STATUS OUT) fires complete. - pipe0->state = PIPE0_STATE_STATUS_OUT_PENDING_IRQ; - break; - - case PIPE0_STATE_STATUS_OUT_PENDING_XFER: - // edpt0_xfer(STATUS OUT) already called — old transfer retired, complete and replay now. - 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_OUT_PENDING_IRQ: - // usbd has not called edpt0_xfer(STATUS OUT) for the old transfer yet — only hold the - // SETUP. Replaying here would let that still-outstanding call land in the replayed - // transfer's state and corrupt it (e.g. NULL DATA OUT drain buffer). - break; - - default: - // PIPE0_STATE_STATUS_IN: rxrdy_consumed gate + SetupEnd guarantee DATAEND was armed, i.e. - // usbd already made its status call; the ZLP-sent IRQ combined with the SETUP. - if (pipe0->pending_addr) { - musb_regs->faddr = pipe0->pending_addr; - pipe0->pending_addr = 0; - } - 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; - } + pipe0_process_status_isr(rhport, musb_regs, ep_csr); break; - } default: break; } @@ -610,44 +614,7 @@ static void process_ep0_isr(uint8_t rhport) { /* 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 (pipe0->state) { - case PIPE0_STATE_DATA_IN: - // DATA IN packet sent (TXRDY cleared). On the last packet (DATAEND condition above) move to - // STATUS_OUT to await the host's STATUS-OUT ZLP IRQ. - if (pipe0->remain_wlength == 0 || pipe0->xact_len < CFG_TUD_ENDPOINT0_SIZE) { - pipe0->state = PIPE0_STATE_STATUS_OUT; - } - dcd_event_xfer_complete(rhport, TU_EP0_IN, pipe0->xact_len, XFER_RESULT_SUCCESS, true); - break; - - case PIPE0_STATE_STATUS_OUT: - // First event of the STATUS OUT pair — wait for edpt0_xfer(STATUS OUT) to fire complete. - pipe0->state = PIPE0_STATE_STATUS_OUT_PENDING_IRQ; - break; - - case PIPE0_STATE_STATUS_OUT_PENDING_XFER: - // Second event — edpt0_xfer(STATUS OUT) already called, fire complete now. - 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_OUT_PENDING_IRQ: - // Stale duplicate of the status confirm — already accounted for; edpt0_xfer fires complete. - break; - - case PIPE0_STATE_STATUS_IN: - if (pipe0->pending_addr) { - musb_regs->faddr = pipe0->pending_addr; - pipe0->pending_addr = 0; - } - 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; - } + pipe0_process_status_isr(rhport, musb_regs, ep_csr); } // Upon BUS RESET is detected, hardware havs already done: @@ -950,7 +917,7 @@ void dcd_edpt_stall(uint8_t rhport, uint8_t ep_addr) { // The transfer being stalled already completed on the wire (a deferred SETUP can only exist // once its status stage was seen) and the host's next request was already ACKed — SendStall // would land on that innocent request. Skip the stall and replay the deferred SETUP instead. - pipe0_process_deferred_setup(rhport, ep_csr, false); + pipe0_try_deferred_setup(rhport, ep_csr, false); } else { // Forcing EP0 to IDLE: any RXRDY parked by the aborted transfer's flow control is stale, // clear it so the next SETUP IRQ is not gated off. From 3d9468152c44148c6881fb3f30ff3dae91a09b68 Mon Sep 17 00:00:00 2001 From: hathach Date: Mon, 15 Jun 2026 22:19:34 +0700 Subject: [PATCH 14/17] dcd/musb: read EP0 SETUP into uint32_t[2], drop the double copy pipe0_read_setup() copied the FIFO into a local union, then copied that into the caller's struct. Read the two FIFO words straight into the caller's uint32_t[2] (one copy) and cast to tusb_control_request_t* in pipe0_start_setup(). pipe0.deferred_setup becomes uint32_t[2] so the deferral path reads directly into it as well. Verified: HIL pass on ek_tm4c123gxl and max32666fthr (13/13 each). Co-Authored-By: Claude Fable 5 --- src/portable/mentor/musb/dcd_musb.c | 30 +++++++++++++---------------- 1 file changed, 13 insertions(+), 17 deletions(-) diff --git a/src/portable/mentor/musb/dcd_musb.c b/src/portable/mentor/musb/dcd_musb.c index e769b08a57..5c2b80cf67 100644 --- a/src/portable/mentor/musb/dcd_musb.c +++ b/src/portable/mentor/musb/dcd_musb.c @@ -100,7 +100,7 @@ typedef struct { bool rxrdy_consumed; // RxPktRdy left set in hw for an already-consumed packet (NAK flow control); // RXRDY events are stale while set. Cleared when RXRDYC is written. bool deferred_setup_valid; - tusb_control_request_t deferred_setup; + uint32_t deferred_setup[2]; // raw SETUP words, replayed via pipe0_start_setup } pipe0_state_t; typedef struct { @@ -110,21 +110,17 @@ typedef struct { static dcd_data_t _dcd; -// Drain a SETUP packet (8 bytes) from the EP0 FIFO. Does not ack RxPktRdy. -static bool pipe0_read_setup(musb_regs_t* musb_regs, musb_ep_csr_t* ep_csr, tusb_control_request_t* req) { +// Read the 8-byte SETUP packet (2 words) from the EP0 FIFO into setup[]. Does not ack RxPktRdy. +static bool pipe0_read_setup(musb_regs_t* musb_regs, musb_ep_csr_t* ep_csr, uint32_t setup[2]) { TU_ASSERT(sizeof(tusb_control_request_t) == ep_csr->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]; - *req = setup_packet.req; + setup[0] = musb_regs->fifo[0]; + setup[1] = musb_regs->fifo[0]; return true; } static void pipe0_start_setup(uint8_t rhport, musb_ep_csr_t* ep_csr, - tusb_control_request_t const* req, bool is_isr) { + const uint32_t setup[2], bool is_isr) { + tusb_control_request_t const* req = (tusb_control_request_t const*) setup; pipe0_state_t* pipe0 = &_dcd.pipe0; pipe0->remain_wlength = req->wLength; @@ -149,7 +145,7 @@ static void pipe0_start_setup(uint8_t rhport, musb_ep_csr_t* ep_csr, } } - dcd_event_setup_received(rhport, (const uint8_t *) req, is_isr); + dcd_event_setup_received(rhport, (const uint8_t *) setup, is_isr); } // Replay a previously deferred SETUP, if any. @@ -160,7 +156,7 @@ static void pipe0_try_deferred_setup(uint8_t rhport, musb_ep_csr_t* ep_csr, bool } pipe0->deferred_setup_valid = false; - pipe0_start_setup(rhport, ep_csr, &pipe0->deferred_setup, is_isr); + pipe0_start_setup(rhport, ep_csr, pipe0->deferred_setup, is_isr); } // EP0 must not call this — it has its own scalars in dcd_data_t. @@ -557,9 +553,9 @@ static void process_ep0_isr(uint8_t rhport) { } switch (pipe0->state) { case PIPE0_STATE_IDLE: { - tusb_control_request_t req; - TU_VERIFY(pipe0_read_setup(musb_regs, ep_csr, &req), ); - pipe0_start_setup(rhport, ep_csr, &req, true); + uint32_t setup[2]; + TU_VERIFY(pipe0_read_setup(musb_regs, ep_csr, setup), ); + pipe0_start_setup(rhport, ep_csr, setup, true); break; } @@ -592,7 +588,7 @@ static void process_ep0_isr(uint8_t rhport) { // Save it, then finish the old transfer's tail event; deferred_setup_valid makes // pipe0_process_status_isr() synthesize the coalesced status confirm and replay the SETUP // once the old transfer is retired. Its RXRDY stays parked so a stale IRQ can't re-process it. - TU_VERIFY(pipe0_read_setup(musb_regs, ep_csr, &pipe0->deferred_setup), ); + TU_VERIFY(pipe0_read_setup(musb_regs, ep_csr, pipe0->deferred_setup), ); pipe0->deferred_setup_valid = true; pipe0->rxrdy_consumed = true; pipe0_process_status_isr(rhport, musb_regs, ep_csr); From f1080e158aeec31faee0f3371d2d5c6f624dd4f4 Mon Sep 17 00:00:00 2001 From: hathach Date: Mon, 15 Jun 2026 22:50:01 +0700 Subject: [PATCH 15/17] dcd/musb: harden EP0 DATA_OUT against short packet and host overrun Mirror the IN-side short-packet fix on the OUT drain: end the data stage (-> STATUS_IN) when wLength is received OR a short OUT packet (count0 < CFG_TUD_ENDPOINT0_SIZE) signals the host's end-of-data, not only when remain_wlength hits exactly 0. Also clamp the remain_wlength subtraction so a host that overruns wLength can't underflow it and strand the transfer. Without this, a control-OUT whose host sends fewer bytes than wLength left pipe0 in DATA_OUT; usbd then armed STATUS IN and tripped the split's TU_ASSERT(!dir_in). Found by /code-review; conformant hosts send exactly wLength so HIL was already green. Verified: HIL pass on ek_tm4c123gxl and max32666fthr (13/13 each). Co-Authored-By: Claude Fable 5 --- src/portable/mentor/musb/dcd_musb.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/portable/mentor/musb/dcd_musb.c b/src/portable/mentor/musb/dcd_musb.c index 5c2b80cf67..0a2df3e711 100644 --- a/src/portable/mentor/musb/dcd_musb.c +++ b/src/portable/mentor/musb/dcd_musb.c @@ -566,12 +566,13 @@ static void process_ep0_isr(uint8_t rhport) { if (count0) { TU_ASSERT(pipe0->buf, ); tu_hwfifo_read(&musb_regs->fifo[0], pipe0->buf, count0, NULL); - pipe0->remain_wlength -= count0; + pipe0->remain_wlength -= tu_min16(count0, pipe0->remain_wlength); // clamp: host may overrun } // RXRDY stays set until the next edpt0_xfer arm acks it (NAK flow control): // edpt0_xfer(DATA OUT) for a mid-stream packet, edpt0_xfer(STATUS IN) for the last. pipe0->rxrdy_consumed = true; - if (pipe0->remain_wlength == 0) { + // Last packet: wLength received, or a short packet (host's end-of-data). + if (pipe0->remain_wlength == 0 || count0 < CFG_TUD_ENDPOINT0_SIZE) { pipe0->state = PIPE0_STATE_STATUS_IN; } dcd_event_xfer_complete(rhport, TU_EP0_OUT, count0, XFER_RESULT_SUCCESS, true); From 148fabb96b16fe0899004ba71d6447ec3995cf55 Mon Sep 17 00:00:00 2001 From: hathach Date: Mon, 15 Jun 2026 23:24:17 +0700 Subject: [PATCH 16/17] dcd/musb: rename pipe0_process_status_isr -> pipe0_process_xfer_state_isr MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The helper advances the whole EP0 control state machine on a completion/confirmation IRQ — it dispatches on pipe0->state and also fires the DATA_IN completion, not just the status stage — so "process_status" undersold it. Matches the process_*_isr family. Co-Authored-By: Claude Fable 5 --- src/portable/mentor/musb/dcd_musb.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/portable/mentor/musb/dcd_musb.c b/src/portable/mentor/musb/dcd_musb.c index 0a2df3e711..e8146f7a7b 100644 --- a/src/portable/mentor/musb/dcd_musb.c +++ b/src/portable/mentor/musb/dcd_musb.c @@ -474,7 +474,7 @@ static bool edpt0_xfer(uint8_t rhport, uint8_t ep_addr, uint8_t *buffer, uint16_ // Advance EP0's status-stage state machine on a tail event: the csrl==0 confirmation IRQ, or such a // confirmation combined with a new SETUP (caller sets deferred_setup_valid first). ISR context only. -static void pipe0_process_status_isr(uint8_t rhport, musb_regs_t* musb_regs, musb_ep_csr_t* ep_csr) { +static void pipe0_process_xfer_state_isr(uint8_t rhport, musb_regs_t* musb_regs, musb_ep_csr_t* ep_csr) { pipe0_state_t* pipe0 = &_dcd.pipe0; switch (pipe0->state) { case PIPE0_STATE_DATA_IN: @@ -587,12 +587,12 @@ static void process_ep0_isr(uint8_t rhport) { case PIPE0_STATE_STATUS_OUT_PENDING_IRQ: case PIPE0_STATE_STATUS_IN: // Save it, then finish the old transfer's tail event; deferred_setup_valid makes - // pipe0_process_status_isr() synthesize the coalesced status confirm and replay the SETUP + // pipe0_process_xfer_state_isr() synthesize the coalesced status confirm and replay the SETUP // once the old transfer is retired. Its RXRDY stays parked so a stale IRQ can't re-process it. TU_VERIFY(pipe0_read_setup(musb_regs, ep_csr, pipe0->deferred_setup), ); pipe0->deferred_setup_valid = true; pipe0->rxrdy_consumed = true; - pipe0_process_status_isr(rhport, musb_regs, ep_csr); + pipe0_process_xfer_state_isr(rhport, musb_regs, ep_csr); break; default: break; @@ -611,7 +611,7 @@ static void process_ep0_isr(uint8_t rhport) { /* 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 */ - pipe0_process_status_isr(rhport, musb_regs, ep_csr); + pipe0_process_xfer_state_isr(rhport, musb_regs, ep_csr); } // Upon BUS RESET is detected, hardware havs already done: From ba3b2453e7cbf3572663dd1b7eadafdcc6b0a912 Mon Sep 17 00:00:00 2001 From: hathach Date: Tue, 16 Jun 2026 11:09:40 +0700 Subject: [PATCH 17/17] dcd/musb: extract pipe0_data_stage_done() and fix two EP0 comments Cleanup from a code-review pass, no behavior change: - Replace the open-coded "last DATA packet" test (remain_wlength == 0 || len < CFG_TUD_ENDPOINT0_SIZE), duplicated in the edpt0_xfer DATA IN arm, pipe0_process_xfer_state_isr, and the DATA OUT drain, with one inline pipe0_data_stage_done() so IN and OUT can't drift. - Correct the xact_len comment (only the IN path reports it; OUT reports count0) and the dcd_edpt_stall comment (a deferred SETUP means the old transfer ended on the wire, not that its status stage was "seen"). Co-Authored-By: Claude Fable 5 --- src/portable/mentor/musb/dcd_musb.c | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/src/portable/mentor/musb/dcd_musb.c b/src/portable/mentor/musb/dcd_musb.c index e8146f7a7b..1d1280bf4d 100644 --- a/src/portable/mentor/musb/dcd_musb.c +++ b/src/portable/mentor/musb/dcd_musb.c @@ -93,7 +93,7 @@ enum { // EP0 control-transfer state (own scalars, not a pipe[] slot). typedef struct { uint8_t *buf; // DATA OUT drain target (only valid while EP0 is in DATA OUT stage) - uint16_t xact_len; // chunk length most recently armed via edpt0_xfer; reported in xfer_complete + uint16_t xact_len; // DATA IN chunk length armed via edpt0_xfer; reported in its xfer_complete (OUT reports count0) 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 @@ -159,6 +159,11 @@ static void pipe0_try_deferred_setup(uint8_t rhport, musb_ep_csr_t* ep_csr, bool pipe0_start_setup(rhport, ep_csr, pipe0->deferred_setup, is_isr); } +// Last DATA packet: wLength satisfied, or a short packet (incl. ZLP) ends the stage. +TU_ATTR_ALWAYS_INLINE static inline bool pipe0_data_stage_done(uint16_t xfer_len) { + return _dcd.pipe0.remain_wlength == 0 || xfer_len < CFG_TUD_ENDPOINT0_SIZE; +} + // 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; @@ -430,8 +435,8 @@ static bool edpt0_xfer(uint8_t rhport, uint8_t ep_addr, uint8_t *buffer, uint16_ } tu_hwfifo_write(&musb_regs->fifo[0], buffer, total_bytes, NULL); pipe0->remain_wlength -= total_bytes; - // DATAEND on the last packet: wLength met, or a short packet (incl. ZLP) ends the data stage. - if (pipe0->remain_wlength == 0 || total_bytes < CFG_TUD_ENDPOINT0_SIZE) { + // Add DATAEND on the last packet to end the data stage. + if (pipe0_data_stage_done(total_bytes)) { ep_csr->csr0l = MUSB_CSRL0_TXRDY | MUSB_CSRL0_DATAEND; } else { ep_csr->csr0l = MUSB_CSRL0_TXRDY; @@ -478,7 +483,7 @@ static void pipe0_process_xfer_state_isr(uint8_t rhport, musb_regs_t* musb_regs, pipe0_state_t* pipe0 = &_dcd.pipe0; switch (pipe0->state) { case PIPE0_STATE_DATA_IN: - if (pipe0->remain_wlength == 0 || pipe0->xact_len < CFG_TUD_ENDPOINT0_SIZE) { // last DATA IN packet + if (pipe0_data_stage_done(pipe0->xact_len)) { if (pipe0->deferred_setup_valid) { pipe0->state = PIPE0_STATE_STATUS_OUT_PENDING_IRQ; // status confirm coalesced with deferred SETUP } else { @@ -571,8 +576,7 @@ static void process_ep0_isr(uint8_t rhport) { // RXRDY stays set until the next edpt0_xfer arm acks it (NAK flow control): // edpt0_xfer(DATA OUT) for a mid-stream packet, edpt0_xfer(STATUS IN) for the last. pipe0->rxrdy_consumed = true; - // Last packet: wLength received, or a short packet (host's end-of-data). - if (pipe0->remain_wlength == 0 || count0 < CFG_TUD_ENDPOINT0_SIZE) { + if (pipe0_data_stage_done(count0)) { pipe0->state = PIPE0_STATE_STATUS_IN; } dcd_event_xfer_complete(rhport, TU_EP0_OUT, count0, XFER_RESULT_SUCCESS, true); @@ -911,9 +915,8 @@ void dcd_edpt_stall(uint8_t rhport, uint8_t ep_addr) { pipe0->state = PIPE0_STATE_IDLE; pipe0->buf = NULL; if (pipe0->deferred_setup_valid) { - // The transfer being stalled already completed on the wire (a deferred SETUP can only exist - // once its status stage was seen) and the host's next request was already ACKed — SendStall - // would land on that innocent request. Skip the stall and replay the deferred SETUP instead. + // A deferred SETUP means the stalled transfer already ended on the wire and the host's next + // request was ACKed — SendStall would hit that innocent request. Replay it instead of stalling. pipe0_try_deferred_setup(rhport, ep_csr, false); } else { // Forcing EP0 to IDLE: any RXRDY parked by the aborted transfer's flow control is stale,