From 6dae50c88354a72795059518373f3a2d1db2ca18 Mon Sep 17 00:00:00 2001 From: Andrew Leech Date: Tue, 16 Jun 2026 06:44:43 +1000 Subject: [PATCH 1/2] hcd/rp2040: implement host endpoint abort and close Fill in the hcd_edpt_abort_xfer and hcd_edpt_close TODO stubs for the rp2040/rp2350 host controller, against the EPSTATE / round-robin EPX driver. hcd_edpt_abort_xfer stops any in-flight transfer and leaves the endpoint re-armable (no double-arm or panic on the next hcd_edpt_xfer): - active EPX control/bulk: sie_stop_xfer (host SIE STOP_TRANS; the usb_hw->abort handshake is device-only), retract the AVAIL buffer, reset to IDLE, clear the latched EPX buf_status/sie_status so the ISR cannot fire into the idle slot, then re-home the round-robin scheduler (promote next pending EP or disarm). - queued control/bulk: drop to IDLE, removing it from the pending set. - interrupt EP: drop to IDLE first so a just-latched buf_status hits the IDLE guard, de-arm the buffer, clear its buf_status bit; polling stays enabled (abort is not close). All under the existing critical section (hw_endpoint_lock_update is a no-op here). hcd_edpt_close deconfigures one endpoint and frees its ep_pool slot, mirroring the per-slot teardown in hcd_device_close, clearing max_packet_size last. Compiles warning-free and links for the rp2040 host example; on-hardware validation of the abort race timing and round-robin interleaving is pending. --- src/portable/raspberrypi/rp2040/hcd_rp2040.c | 127 +++++++++++++++++-- 1 file changed, 118 insertions(+), 9 deletions(-) diff --git a/src/portable/raspberrypi/rp2040/hcd_rp2040.c b/src/portable/raspberrypi/rp2040/hcd_rp2040.c index 064834efb1..c69be6e505 100644 --- a/src/portable/raspberrypi/rp2040/hcd_rp2040.c +++ b/src/portable/raspberrypi/rp2040/hcd_rp2040.c @@ -589,19 +589,128 @@ bool hcd_edpt_open(uint8_t rhport, uint8_t dev_addr, const tusb_desc_endpoint_t return true; } -bool hcd_edpt_close(uint8_t rhport, uint8_t daddr, uint8_t ep_addr) { - (void)rhport; - (void)daddr; - (void)ep_addr; - return false; // TODO not implemented yet +// Disarm the EPX round-robin scheduler. Call when the active EPX endpoint is torn +// down and no other endpoint is pending, so a stale SOF / NAK-stop wakeup cannot +// resurrect the aborted transfer. Mirrors the "no more pending" cleanup in the ISR. +TU_ATTR_ALWAYS_INLINE static inline void epx_scheduler_disarm(void) { + #ifdef HAS_STOP_EPX_ON_NAK + usb_hw_clear->nak_poll = USB_NAK_POLL_STOP_EPX_ON_NAK_BITS; + usb_hw_clear->nak_poll = USB_NAK_POLL_EPX_STOPPED_ON_NAK_BITS; // clear any latched NAK-stop + #else + usb_hw_clear->inte = USB_INTE_HOST_SOF_BITS; + usb_hw->nak_poll = USB_NAK_POLL_RESET; + epx_switch_request = false; + #endif } +// Abort an in-flight (or queued) transfer and leave the endpoint configured and +// re-armable. The caller (tuh_edpt_abort_xfer) has already cleared the usbh-layer +// BUSY/claim, so this only quiesces hardware + the EPSTATE machine. The endpoint is +// NOT deconfigured here (that is hcd_edpt_close's job). bool hcd_edpt_abort_xfer(uint8_t rhport, uint8_t dev_addr, uint8_t ep_addr) { (void)rhport; - (void)dev_addr; - (void)ep_addr; - // TODO not implemented yet - return false; + + hw_endpoint_t *ep = edpt_find(dev_addr, ep_addr); + TU_ASSERT(ep); + + // hw_endpoint_lock_update is a no-op on this port; the critical section is the + // only serialisation against hcd_rp2040_irq, which mutates epx/state/buf_status. + rp2usb_critical_enter(); + + if (ep->interrupt_num > 0) { + // Interrupt endpoint: dedicated hardware that polls autonomously, not on the + // shared EPX path. There is no STOP_TRANS for it. Drop state to IDLE first so a + // buf_status latched just before this is ignored by rp2usb_xfer_continue's IDLE + // guard, then de-arm the buffer (clears AVAIL) and clear its latched completion. + // Leave int_ep_ctrl polling enabled (abort != close): the next hcd_edpt_xfer + // re-arms via the buffer control. + ep->state = EPSTATE_IDLE; + *dpram_int_ep_buffer_ctrl(ep->interrupt_num) = 0; + // buf_status bit layout: IN at (interrupt_num << 1), OUT at ((interrupt_num << 1) | 1) + const uint8_t bit_idx = (uint8_t)((ep->interrupt_num << 1u) | (tu_edpt_dir(ep->ep_addr) == TUSB_DIR_IN ? 0u : 1u)); + usb_hw_clear->buf_status = TU_BIT(bit_idx); + rp2usb_reset_transfer(ep); + } else if (epx == ep && ep->state == EPSTATE_ACTIVE) { + // Active control/bulk endpoint owning the shared EPX engine. Stop the in-flight + // transaction on the wire BEFORE touching state (host stop primitive is SIE + // STOP_TRANS via sie_stop_xfer; the device usb_hw->abort handshake is device-only). + sie_stop_xfer(); + + // Retract any half still marked AVAIL so the controller cannot poll a stale + // buffer after we reset state. Unlike epx_save_context (the preemption path), + // we do NOT roll back PID/len and re-queue: abort means do-not-resume. + usbh_dpram->epx_buf_ctrl = 0; + + // Drop the endpoint to IDLE, then clear latched completions so the ISR cannot + // fire xfer_complete_isr into the now-idle EP. + rp2usb_reset_transfer(ep); + usb_hw_clear->buf_status = 1u; // BUF_STATUS_EPX (bit 0) + usb_hw_clear->sie_status = + USB_SIE_STATUS_TRANS_COMPLETE_BITS | USB_SIE_STATUS_RX_TIMEOUT_BITS | USB_SIE_STATUS_STALL_REC_BITS; + + // epx now points at an idle slot. Promote a queued endpoint if any (the engine + // is free); otherwise leave epx on the idle slot and disarm the scheduler. + hw_endpoint_t *next_ep = epx_next_pending(epx); + if (next_ep != NULL) { + epx_switch_ep(next_ep); + } else { + epx_scheduler_disarm(); + } + } else if (ep->state >= EPSTATE_PENDING) { + // Queued control/bulk endpoint (PENDING or PENDING_SETUP) that does NOT own the + // engine: another endpoint is on the wire. No SIE stop / buf_ctrl touch. Setting + // state to IDLE removes it from the implicit pending set (epx_next_pending only + // matches state >= EPSTATE_PENDING); the staged setup_packet bytes are harmless. + rp2usb_reset_transfer(ep); + + // If that emptied the pending set and EPX itself is idle, disarm the scheduler. + if (epx->state != EPSTATE_ACTIVE && epx_next_pending(epx) == NULL) { + epx_scheduler_disarm(); + } + } + // else: already IDLE, nothing to stop (idempotent). + + rp2usb_critical_exit(); + return true; +} + +// Deconfigure a single endpoint and free its ep_pool slot. The caller +// (tuh_edpt_close) always aborts first and rejects EP0, so by here the transfer is +// already stopped; this only deconfigures hardware and releases the slot. Mirrors +// the per-slot body of hcd_device_close. +bool hcd_edpt_close(uint8_t rhport, uint8_t daddr, uint8_t ep_addr) { + (void)rhport; + + // Reject EP0 / the shared EPX control slot: it is never closed and ep_pool[0] + // must not be freed. + TU_VERIFY(tu_edpt_number(ep_addr) != 0); + + hw_endpoint_t *ep = edpt_find(daddr, ep_addr); + TU_VERIFY(ep); + + rp2usb_critical_enter(); + + ep->state = EPSTATE_IDLE; // defensive: drop from any scheduling (abort already did) + + if (ep->interrupt_num > 0) { + // Interrupt endpoint: tear down the dedicated hardware so the int_idx is returned + // to the free pool that hcd_edpt_open scans. Disable SIE polling, clear the + // address control, then zero the DPRAM buffer + control words. + usb_hw_clear->int_ep_ctrl = TU_BIT(ep->interrupt_num); + usb_hw->int_ep_addr_ctrl[ep->interrupt_num - 1] = 0; + + io_rw_32 *ep_reg = dpram_int_ep_ctrl(ep->interrupt_num); + io_rw_32 *buf_reg = dpram_int_ep_buffer_ctrl(ep->interrupt_num); + *buf_reg = 0; + *ep_reg = 0; + } + // Control/bulk endpoints share the EPX registers (re-prepared on every xfer), so + // there is no per-endpoint hardware to deconfigure here. + + ep->max_packet_size = 0; // mark slot unused (must be last: the allocation sentinel) + + rp2usb_critical_exit(); + return true; } bool hcd_edpt_xfer(uint8_t rhport, uint8_t dev_addr, uint8_t ep_addr, uint8_t *buffer, uint16_t buflen) { From b414cc7d87b20c02a7e7d237feab102f576bfd8b Mon Sep 17 00:00:00 2001 From: Andrew Leech Date: Tue, 16 Jun 2026 07:07:25 +1000 Subject: [PATCH 2/2] hcd/rp2040: address review of host abort/close - abort (RP2350): clear a latched EPX_STOPPED_ON_NAK before promoting the next queued endpoint, so a stale NAK-stop cannot fire a spurious preemption against the transfer just started on the promoted endpoint. - close: defensively disarm the scheduler when the slot being freed is still the active epx. tuh_edpt_close aborts first so this is a no-op on the normal path, but the HCD API does not mandate abort-first. - comments: note the data toggle is intentionally not preserved across abort (same stance as the device-side hw_endpoint_abort_xfer); de-overclaim the interrupt-EP IDLE-guard note (the critical section already blocks the ISR); record the ACTIVE-control/bulk-is-epx invariant. Behaviour on the validated paths (active-EPX abort+resubmit, close+reopen) is unchanged; the new NAK-latch clear and close self-disarm cover branches the single-endpoint hardware test did not exercise. --- src/portable/raspberrypi/rp2040/hcd_rp2040.c | 32 +++++++++++++++----- 1 file changed, 25 insertions(+), 7 deletions(-) diff --git a/src/portable/raspberrypi/rp2040/hcd_rp2040.c b/src/portable/raspberrypi/rp2040/hcd_rp2040.c index c69be6e505..287db845b4 100644 --- a/src/portable/raspberrypi/rp2040/hcd_rp2040.c +++ b/src/portable/raspberrypi/rp2040/hcd_rp2040.c @@ -619,11 +619,11 @@ bool hcd_edpt_abort_xfer(uint8_t rhport, uint8_t dev_addr, uint8_t ep_addr) { if (ep->interrupt_num > 0) { // Interrupt endpoint: dedicated hardware that polls autonomously, not on the - // shared EPX path. There is no STOP_TRANS for it. Drop state to IDLE first so a - // buf_status latched just before this is ignored by rp2usb_xfer_continue's IDLE - // guard, then de-arm the buffer (clears AVAIL) and clear its latched completion. - // Leave int_ep_ctrl polling enabled (abort != close): the next hcd_edpt_xfer - // re-arms via the buffer control. + // shared EPX path. There is no STOP_TRANS for it. Drop state to IDLE, de-arm the + // buffer (clears AVAIL), and clear its latched completion bit. The critical + // section already blocks the ISR; the cleared bit plus the IDLE state also cover + // the post-exit window. Leave int_ep_ctrl polling enabled (abort != close): the + // next hcd_edpt_xfer re-arms via the buffer control. ep->state = EPSTATE_IDLE; *dpram_int_ep_buffer_ctrl(ep->interrupt_num) = 0; // buf_status bit layout: IN at (interrupt_num << 1), OUT at ((interrupt_num << 1) | 1) @@ -638,7 +638,11 @@ bool hcd_edpt_abort_xfer(uint8_t rhport, uint8_t dev_addr, uint8_t ep_addr) { // Retract any half still marked AVAIL so the controller cannot poll a stale // buffer after we reset state. Unlike epx_save_context (the preemption path), - // we do NOT roll back PID/len and re-queue: abort means do-not-resume. + // we do NOT roll back PID/len and re-queue: abort means do-not-resume. The data + // toggle (next_pid) is therefore left as-armed, the same stance as the + // device-side hw_endpoint_abort_xfer; a bulk EP re-armed after abort (without a + // close or a CLEAR_FEATURE(ENDPOINT_HALT) toggle reset) may resend with an + // unexpected toggle. usbh_dpram->epx_buf_ctrl = 0; // Drop the endpoint to IDLE, then clear latched completions so the ISR cannot @@ -652,6 +656,11 @@ bool hcd_edpt_abort_xfer(uint8_t rhport, uint8_t dev_addr, uint8_t ep_addr) { // is free); otherwise leave epx on the idle slot and disarm the scheduler. hw_endpoint_t *next_ep = epx_next_pending(epx); if (next_ep != NULL) { + #ifdef HAS_STOP_EPX_ON_NAK + // Drop any latched NAK-stop so it cannot fire a spurious preemption round-trip + // against the transfer we are about to start on the promoted endpoint. + usb_hw_clear->nak_poll = USB_NAK_POLL_EPX_STOPPED_ON_NAK_BITS; + #endif epx_switch_ep(next_ep); } else { epx_scheduler_disarm(); @@ -668,7 +677,8 @@ bool hcd_edpt_abort_xfer(uint8_t rhport, uint8_t dev_addr, uint8_t ep_addr) { epx_scheduler_disarm(); } } - // else: already IDLE, nothing to stop (idempotent). + // else: already IDLE, nothing to stop (idempotent). Invariant: a control/bulk EP + // is ACTIVE only while it is epx, so the branches above catch every active case. rp2usb_critical_exit(); return true; @@ -692,6 +702,14 @@ bool hcd_edpt_close(uint8_t rhport, uint8_t daddr, uint8_t ep_addr) { ep->state = EPSTATE_IDLE; // defensive: drop from any scheduling (abort already did) + // Defensive: tuh_edpt_close aborts first, but the HCD API does not require it. If + // this slot is somehow still the active EPX owner, disarm the scheduler so a stale + // SOF/NAK wakeup cannot start a transfer on the slot we are about to free. After + // the normal abort-first path this is a no-op. + if (epx == ep) { + epx_scheduler_disarm(); + } + if (ep->interrupt_num > 0) { // Interrupt endpoint: tear down the dedicated hardware so the int_idx is returned // to the free pool that hcd_edpt_open scans. Disable SIE polling, clear the