Synopsys-OTG: type-erase endpoint counts#6097
Merged
Merged
Conversation
fadd085 to
e70ef64
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
This PR removes const-generic endpoint/channel counts from the Synopsys OTG device/host drivers by introducing type-erased OtgState/OtgHostState views, moving endpoint allocation bookkeeping into State, and updating downstream HAL crates to pass the new state views. It also aims to expand host channel support up to 16 channels.
Changes:
- Introduce
OtgState/OtgHostStatetype-erased borrows and makeOtgInstance/OtgHostInstancenon-generic. - Move endpoint allocation tracking into
Stateand update FIFO sizing/IRQ mask computation to use shared state. - Update STM32 + nRF wrappers to use the new type-erased state and non-generic driver/bus types.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| embassy-usb-synopsys-otg/src/otg_v1.rs | Adjust host-channel register accessor bounds (but currently inconsistent). |
| embassy-usb-synopsys-otg/src/lib.rs | Introduce OtgState, move endpoint allocation to State, remove const generics from driver/bus/instance. |
| embassy-usb-synopsys-otg/src/host.rs | Introduce OtgHostState, remove const generics from host types, add runtime clamping in allocator. |
| embassy-usb-synopsys-otg/CHANGELOG.md | Document breaking API change + 16 host channel support. |
| embassy-stm32/src/usb/otg.rs | Adapt STM32 OTG wrapper to new type-erased state + non-generic Synopsys driver/bus. |
| embassy-nrf/src/usb/usbhs.rs | Adapt nRF USBHS wrapper to new type-erased state + non-generic Synopsys driver/bus. |
Comments suppressed due to low confidence (3)
embassy-usb-synopsys-otg/src/lib.rs:316
OtgState::ep_alloc_getindexesep_{in,out}_alloc[index]directly. With the new type-erased API, callers can pass anendpoint_countlarger than these slices, causing a panic. Consider using.get(index)and returningNonewhen out of range (and/or asserting the invariant once when constructing/usingOtgInstance).
pub(crate) fn ep_alloc_get(&self, dir: Direction, index: usize) -> Option<&EndpointData> {
unsafe {
match dir {
Direction::In => (*self.ep_in_alloc[index].get()).as_ref(),
Direction::Out => (*self.ep_out_alloc[index].get()).as_ref(),
embassy-usb-synopsys-otg/src/lib.rs:325
ep_fifo_size_initerates0..endpoint_countand callsep_alloc_get, which will panic ifendpoint_countexceeds the backing slice lengths. Sinceendpoint_countis now runtime-provided, clamp it tomin(endpoint_count, ep_in_alloc.len())(and similarly in the other helpers) or assert the invariant once and reuse the checked value.
pub(crate) fn ep_fifo_size_in(&self, endpoint_count: usize) -> u16 {
(0..endpoint_count)
.filter_map(|i| self.ep_alloc_get(Direction::In, i))
.map(|ep| ep.fifo_size_words)
.sum()
embassy-usb-synopsys-otg/src/host.rs:136
on_host_interruptguardsch_num >= ch_count, but after type-erasingHostState,ch_countis no longer type-coupled tostate.channels.len(). If a caller passes a largerch_countthan the slice length,state.channels[ch_num]can panic in the ISR. Clampch_counttostate.channels.len()(or assert the invariant) at the start of the handler.
pub unsafe fn on_host_interrupt(r: Otg, state: &OtgHostState<'_>, ch_count: usize) {
let gintsts = r.gintsts().read();
// Clear SOF interrupt immediately to avoid flooding.
if gintsts.sof() {
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
3ac81cb to
a28fd88
Compare
6676033 to
24cd844
Compare
Dirbaio
reviewed
May 17, 2026
Dirbaio
reviewed
May 17, 2026
Dirbaio
reviewed
May 17, 2026
996fe3d to
04556dd
Compare
This comment was marked as resolved.
This comment was marked as resolved.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR allows type-erasing the USB instance in HAL drivers without forcing them to use the same capacity endpoint/channel state buffer (i.e. without wasting memory for the "smaller" peripheral). This is done by introducing a type-erased state struct that holds slices and references, and also by moving the endpoint allocation buffers to the global state. This is the foundation for esp-rs/esp-hal#5560