hcd/rp2040: implement host endpoint abort and close

[andrewleech]

Summary

This extends the rp2040/rp2350 host driver to implement two HCD endpoint methods that were left as TODOs when host support landed, hcd_edpt_abort_xfer and hcd_edpt_close. I came at them building raw bulk endpoint forwarding over the rp2 host, but they're on the normal host paths too, tuh_edpt_close aborts then closes an endpoint and usbh aborts every endpoint on a SET_CONFIGURATION, so filling them in rounds out the existing host support.

The abort runs the EP_ABORT / EP_ABORT_DONE handshake so the controller stops accessing an interrupt endpoint before we clear its buffer, and leaves the endpoint configured so the next submit re-arms cleanly. Without that the buffer stays armed while the stack thinks the endpoint is idle, so the next transfer arms an already-available buffer and trips panic("ep %02X was already available"). Control (EPX) just clears its buffer and resets. The USB interrupt is masked across the teardown since hw_endpoint_lock_update is a no-op on this port.

The close deconfigures the endpoint the same way hcd_device_close already does per endpoint, which frees the ep_pool slot for reuse. Without it tuh_edpt_close aborts the transfer but leaves the endpoint allocated, so a later tuh_edpt_open of the same endpoint takes a fresh slot and works through the 15 interrupt slots.

Both land on rp2350 as well since it shares the driver.

Testing

Tested on RP2350 (Pico 2 W) acting as a USB host against a full-speed CDC device, driving raw bulk endpoint transfers with CFG_TUH_API_EDPT_XFER=1. Before this, aborting a pending bulk-IN read and resubmitting on the same endpoint panicked, confirmed over SWD with the endpoint still active and its buffer armed. After, repeated cancel/resubmit cycles run clean with no panic and no duplicate or stuck endpoint-pool entries.

EP_ABORT and EP_ABORT_DONE are tagged "Device only" in the datasheet, so I instrumented the spin to check the handshake actually works host-side: across 48 aborts from a clean boot, EP_ABORT_DONE asserted within 0 spin iterations every time, so it does work in host mode for these interrupt endpoints. Haven't tested on an actual rp2040 yet though, the abort register path is identical on both.

Trade-offs and Alternatives

The abort waits on abort_done before clearing the buffer, the same handshake the device side uses in rp2040_usb.c. It runs at task level, not in the ISR, and done asserts within a few cycles for these endpoints (measured, see Testing), so the spin's fine. The alternative that avoids the "Device only" register entirely is to disable the endpoint's int_ep_ctrl polling and then clear the buffer (what hcd_device_close does), but given the handshake measurably works and gives a clean "controller is idle" signal, I went with EP_ABORT.

Generative AI

I used generative AI tools when creating this PR, but a human has checked the code and is responsible for the code and the description above.

[Copilot]

Pull request overview

Implements the missing rp2040 host-controller endpoint lifecycle APIs so host-mode endpoint cancellation and teardown behave correctly and endpoint pool slots can be reused.

Changes:

• Implement hcd_edpt_abort_xfer() for rp2040/rp2350 host mode, including EP_ABORT/EP_ABORT_DONE handling for “interrupt” endpoints.
• Implement hcd_edpt_close() to deconfigure an endpoint and free its ep_pool slot for reuse.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[hathach]

@claude review

[claude]

Claude finished @hathach's task in 3m 5s —— View job

---

Code Review — hcd/rp2040: implement host endpoint abort and close

• Gather context
• Read changed file and diff
• Review hcd_edpt_abort_xfer implementation
• Review hcd_edpt_close implementation
• Check against existing copilot review comments
• Post findings

---

Overall: The design direction is correct — the EP_ABORT/ABORT_DONE handshake for interrupt endpoints and the pool-slot teardown in hcd_edpt_close are both the right approach. There are a few concrete bugs and robustness gaps worth addressing before merge.

---

Critical — EPX abort does not clear latched completion bits

File: src/portable/raspberrypi/rp2040/hcd_rp2040.c, EPX branch of hcd_edpt_abort_xfer (~line 637)

*ep->buffer_control = 0;
hw_endpoint_reset_transfer(ep);   // sets ep->active = false
// ... then hcd_int_enable()

If buf_status bit 0 (EPX) or sie_status STALL_REC / TRANS_COMPLETE are already latched when the IRQ is re-enabled, the ISR fires immediately and hits:

• _handle_buff_status_bit() → assert(ep->active) → panic (ep->active is now false)
• hw_trans_complete() for an in-flight setup → assert(ep->active) → panic

Fix: before hcd_int_enable(), drain the pending status:

*ep->buffer_control = 0;
usb_hw_clear->buf_status = 1u;                                           // bit 0 = EPX
usb_hw_clear->sie_status = USB_SIE_STATUS_TRANS_COMPLETE_BITS
                          | USB_SIE_STATUS_STALL_REC_BITS;
hw_endpoint_reset_transfer(ep);

Fix this →

---

Moderate — abort_done cleared via wrong register alias

File: hcd_edpt_abort_xfer, lines 656 and 664

usb_hw->abort_done = bit;   // line 656 — stale-done clear
// ...
usb_hw->abort_done = bit;   // line 664 — post-abort clear

dcd_rp2040.c uses the CLEAR alias consistently:

usb_hw_clear->abort_done = abort_mask;

abort_done is write-1-to-clear (W1C), so writing bit to the base address is functionally equivalent (only the written bits are affected). But writing to the base address reads as "write the value to this register" whereas the CLEAR alias has a well-defined "write 1 to clear" semantic that matches the intent and is the established pattern everywhere else in this driver. Prefer usb_hw_clear->abort_done = bit for both occurrences for consistency and clarity.

Fix this →

---

Moderate — unbounded spin with IRQs masked

File: hcd_edpt_abort_xfer, line 658

while ( !(usb_hw->abort_done & bit) ) {}

With the USB IRQ masked and no timeout, a pathological endpoint state (very long or stalled packet in-flight, or unexpected silicon behavior on B0/B1) turns this into a permanent hang with no recovery path. The device-side code in dcd_rp2040.c already guards the entire abort on rp2040_chip_version() >= 2 to avoid the RP2040-E2 errata; the host driver makes no such check and doesn't document why it's safe to skip it.

Recommended: add a bounded iteration counter (e.g., 1000 iterations) and, on timeout, fall back to disabling the endpoint's int_ep_ctrl bit before clearing the buffer (the approach hcd_device_close already uses).

---

Minor — hcd_edpt_close lacks defensive active-check

File: hcd_edpt_close, line 537

The comment correctly notes that tuh_edpt_close aborts before calling this, but the function silently succeeds even if ep->active is still true (a race if the abort was skipped). Adding TU_ASSERT(!ep->active) before the teardown would make contract violations visible during development:

TU_ASSERT(!ep->active);   // caller must have aborted first
hcd_int_disable(rhport);
...

---

Nit — hcd_edpt_close IRQ masking inconsistent with hcd_device_close

hcd_device_close performs the same per-slot teardown without hcd_int_disable/enable. hcd_edpt_close adds the masking, which is correct and defensive — but it means the same sequence is unprotected in hcd_device_close. Consider aligning them (or at minimum add a comment explaining why hcd_device_close is safe without masking — e.g., called only after disconnect when no transfers are in flight).

---

Summary

Severity	Issue
Critical	EPX abort: latched buf_status/sie_status bits must be cleared before re-enabling IRQ
Moderate	abort_done should use usb_hw_clear->abort_done alias, not base register
Moderate	Unbounded spin with IRQs masked; no RP2040-E2 chip-rev guard
Minor	hcd_edpt_close should assert !ep->active
Nit	hcd_device_close teardown inconsistently unmasked vs hcd_edpt_close

The EP_ABORT handshake is the right primitive here and the bit-index calculation for interrupt endpoints is correct. Fixing the EPX latched-status bug and the two moderate issues would make this solid.

[andrewleech]

Rebased onto current master and force-pushed. The branch was sitting a long way behind master so the original abort/close didn't line up with the current rp2040 host driver (epx is a round-robin pointer, an EPSTATE state machine, sie_stop_xfer to stop a transfer). I've reimplemented hcd_edpt_abort_xfer and hcd_edpt_close against the current model, with the review points folded in.

abort stops the active EPX transfer with sie_stop_xfer (the host STOP_TRANS, not the device-only abort handshake), drops the ep to IDLE and clears the latched buf_status / sie_status before re-homing the round-robin scheduler, so the ep stays re-armable with no double-arm. close tears the ep down and frees its ep_pool slot the same way hcd_device_close does.

Tested on an RP2350 (Pico 2 W) hosting a CDC device: aborting a pending bulk-IN read and resubmitting on the same endpoint runs clean now, no ep .. was already available panic, and close/reopen works over a couple of cycles. Haven't exercised the multi-endpoint promote path or an actual rp2040 on hardware yet though.

[github-actions]

MemBrowse Memory Report

Top 8 targets by memory change (%) (out of 2384 targets) View Project Dashboard →

target	.text	.rodata	.data	.bss	total	% diff
raspberry_pi_pico/hid_controller	24,032 → 24,296 (+264)	—	—	—	25,232 → 25,496 (+264)	+1.0%
raspberry_pi_pico/device_info	24,752 → 25,016 (+264)	—	—	—	26,048 → 26,312 (+264)	+1.0%
raspberry_pi_pico/bare_api	25,408 → 25,672 (+264)	—	—	—	26,792 → 27,056 (+264)	+1.0%
raspberry_pi_pico/midi_rx	26,024 → 26,288 (+264)	—	—	—	26,980 → 27,244 (+264)	+1.0%
raspberry_pi_pico/midi2_host	26,416 → 26,680 (+264)	—	—	—	27,652 → 27,916 (+264)	+1.0%
raspberry_pi_pico/cdc_msc_hid	36,640 → 36,904 (+264)	—	—	—	38,964 → 39,228 (+264)	+0.7%
raspberry_pi_pico/msc_file_explorer	44,296 → 44,560 (+264)	—	—	—	48,200 → 48,464 (+264)	+0.5%
raspberry_pi_pico/msc_file_explorer_freertos	51,356 → 51,620 (+264)	—	—	—	55,484 → 55,748 (+264)	+0.5%