Skip to content

Fix many quirks in USB implementation #87

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 1 commit into
base: development
Choose a base branch
from
Draft

Fix many quirks in USB implementation #87

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@

## [Unreleased]

### Fixed
- Fix issues with USB that were affecting reliable operation.

## [v0.4.6] 2025-03-31

### Changed
Expand Down
2 changes: 1 addition & 1 deletion hal/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ embedded-hal = { version = "0.2.7" }
nb = "1"
fugit = "0.3.6"
paste = "1"
usb-device = "0.3.1"
usb-device = "0.3.2"
bit-iter = "1"
rtic-monotonic = "1"
void = { version = "1", default-features = false }
Expand Down
138 changes: 82 additions & 56 deletions hal/src/usb.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,6 @@ impl Endpoints {

struct Inner {
endpoints: Endpoints,
set_address: bool,
}

/// [`usb_device`] implementation.
Expand All @@ -157,7 +156,6 @@ impl Usb {

let inner = Inner {
endpoints: Endpoints::new(),
set_address: false,
};

Self {
Expand Down Expand Up @@ -277,6 +275,12 @@ impl Inner {
.deveptidr_ctrl_mode(ep)
.write(|w| w.stallrqc().set_bit());

if ep == 0 {
self.reg()
.devepticr_ctrl_mode(ep)
.write(|w| w.rxoutic().set_bit());
}

if ep != 0 {
// Configure endpoint direction.
self.reg()
Expand All @@ -292,9 +296,15 @@ impl Inner {
.cfgok()
.bit_is_set()
{
self.reg()
.deveptier_ctrl_mode(ep)
.write(|w| w.rxstpes().set_bit());
if ep == 0 {
self.reg()
.deveptier_ctrl_mode(ep)
.write(|w| w.rxstpes().set_bit());
} else if conf.ep_dir == UsbDirection::Out {
self.reg()
.deveptier_blk_mode(ep)
.write(|w| w.rxoutes().set_bit());
}
self.enable_endpoint_interrupt(ep);
} else {
todo!("endpoint configuration failed");
Expand Down Expand Up @@ -326,11 +336,6 @@ impl Inner {
// enable interrupts
self.reg().devier().write(|w| {
w.eorstes().set_bit();
w.suspes().set_bit();
w.wakeupes().set_bit();

// should we use this?
w.sofes().set_bit();

w
});
Expand Down Expand Up @@ -411,16 +416,14 @@ impl Inner {
}

fn set_device_address(&mut self, addr: u8) {
// Set the address in hardware, but do not enable it. This is
// done in poll() on an TXINI.
// Set the address in hardware and enable it.
self.reg().devctrl().modify(|_, w| {
unsafe {
w.uadd().bits(addr);
}
w.adden().clear_bit();
w.adden().set_bit();
w
});
self.set_address = true;
}

fn poll(&mut self) -> PollResult {
Expand All @@ -441,27 +444,37 @@ impl Inner {
const DEVISR_PEPS_MASK: u32 = 0x3ff000;
const DEVISR_PEPS_OFFSET: u8 = 12;
for ep in BitIter::from((dev_isr.bits() & DEVISR_PEPS_MASK) >> DEVISR_PEPS_OFFSET) {
let sr = self.reg().deveptisr_ctrl_mode(ep).read();

// SETUP packet?
if sr.rxstpi().bit_is_set() {
ep_setup |= 1 << ep;
};

// OUT packet?
if sr.rxouti().bit_is_set() {
ep_out |= 1 << ep;
};
let isr = self.reg().deveptisr_ctrl_mode(ep).read();

// IN packet?
if sr.txini().bit_is_set() {
if self.set_address {
// commit the new address
self.reg().devctrl().modify(|_, w| w.adden().set_bit());
self.set_address = false;
if ep == 0 {
// OUT packet?
let imr = self.reg().deveptimr_ctrl_mode(ep).read();
if isr.rxouti().bit_is_set() && imr.rxoute().bit_is_set() {
ep_out |= 1;
}
// SETUP packet?
if ep_out == 0 {
if isr.rxstpi().bit_is_set() {
ep_setup |= 1;
};
}
// IN packet?
if isr.txini().bit_is_set() && imr.txine().bit_is_set() {
ep_in_complete |= 1;
// disable TXINI interrupt
self.reg()
.deveptidr_ctrl_mode(ep)
.write(|w| w.txinec().set_bit());
}
} else {
// OUT packet?
if isr.rxouti().bit_is_set() {
ep_out |= 1 << ep;
};
// IN packet?
if isr.txini().bit_is_set() {
ep_in_complete |= 1 << ep;
}

ep_in_complete |= 1 << ep;
};
}

Expand Down Expand Up @@ -514,25 +527,27 @@ impl Inner {

fn read(&self, ep_addr: EndpointAddress, buf: &mut [u8]) -> UsbResult<usize> {
let ep = ep_addr.index();
let len = core::cmp::min(
self.reg().deveptisr_ctrl_mode(ep).read().byct().bits() as usize,
buf.len(),
);
let isr = self.reg().deveptisr_ctrl_mode(ep).read();
let len = core::cmp::min(isr.byct().bits() as _, buf.len());

self.read_fifo(ep, &mut buf[0..len]);

if ep == 0 {
// control endpoints

// Clear RXSTPI interrupt, and make FIFO available
self.reg()
.devepticr_ctrl_mode(0)
.write(|w| w.rxstpic().set_bit());

// Clear RXOUTI
self.reg()
.devepticr_ctrl_mode(0)
.write(|w| w.rxoutic().set_bit());
if isr.rxouti().bit_is_set() {
// Clear RXOUTI
self.reg()
.devepticr_ctrl_mode(0)
.write(|w| w.rxoutic().set_bit());
} else if isr.rxstpi().bit_is_set() {
// Clear RXSTPI interrupt, and make FIFO available
self.reg()
.devepticr_ctrl_mode(0)
.write(|w| w.rxstpic().set_bit());
} else {
// Should never come here.
return Err(UsbError::WouldBlock);
}
} else {
// Other Endpoints

Expand All @@ -550,21 +565,32 @@ impl Inner {
Ok(len)
}

fn is_stalled(&self, _ep: EndpointAddress) -> bool {
// stub: seemingly not required
false
fn is_stalled(&self, ep_addr: EndpointAddress) -> bool {
let ep = ep_addr.index();
if ep == 0 {
self.reg().deveptimr_ctrl_mode(ep).read().stallrq().bit()
} else {
false
}
}

fn set_stalled(&self, _ep: EndpointAddress, _stalled: bool) {
// stub: seemingly not required
fn set_stalled(&self, ep_addr: EndpointAddress, stalled: bool) {
let ep = ep_addr.index();
if ep == 0 {
if stalled {
self.reg()
.deveptier_ctrl_mode(ep)
.write(|w| w.stallrqs().set_bit());
} else {
self.reg()
.deveptidr_ctrl_mode(ep)
.write(|w| w.stallrqc().set_bit());
}
}
}
}

impl usb_device::bus::UsbBus for Usb {
/// Ensure the address is set before write of zero sized packet to
/// confirm a SET_ADDRESS transaction.
const QUIRK_SET_ADDRESS_BEFORE_STATUS: bool = true;

fn enable(&mut self) {
interrupt::free(|cs| unsafe { &mut *self.inner.borrow(cs).get() }.enable());
}
Expand Down