Skip to content

Commit

Permalink
Rework USB OUT endpoint interrupts (atsamd-rs#459)
Browse files Browse the repository at this point in the history 
* Rework USB OUT endpoint interrupts

Previously, when the USB interrupt was unmasked and a host issued an OUT
transfer to an endpoint that was not `read()` before the next OUT
transfer attempt to that endpoint, the USB ISR would continually fire
until `read()` was called on that endpoint.  If a higher priority ISR
didn't somehow clear the condition that prevented the `read()`, the CPU
would lock up.

This change moves the clearing of the endpoint's transfer complete and
transfer failed interrupt flags in to `poll(), so that a USB Class can
safely skip a `read()` without having to chose between discarding data,
or locking up in the USB ISR.

* Optimisation in USB poll()
  • Loading branch information
@ianrrees @rtsuk
ianrrees authored and rtsuk committed Mar 29, 2022
1 parent 088610a commit e893102
Show file tree
Hide file tree
Showing 2 changed files with 83 additions and 61 deletions.
72 changes: 42 additions & 30 deletions hal/src/thumbv6m/usb/bus.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -394,13 +394,6 @@ impl<'a> Bank<'a, OutBank> {
.write(|w| w.trcpt0().set_bit().trfail0().set_bit());
}

/// Checks if data has been received. Returns true for failed transfers
/// as well as successful transfers.
#[inline]
fn is_transfer_complete(&self) -> bool {
self.epintflag(self.index()).read().trcpt0().bit()
}

/// Returns true if a Received Setup interrupt has occurred.
/// This indicates that the read buffer holds a SETUP packet.
#[inline]
Expand Down Expand Up @@ -750,6 +743,7 @@ impl Inner {
}

fn suspend(&self) {}

fn resume(&self) {}

fn alloc_ep(
Expand Down Expand Up @@ -822,49 +816,69 @@ impl Inner {
// unconnected & unsuspended, we do not handle them to avoid spurious
// transitions.

let intbits = self.usb().epintsmry.read().bits();
if intbits == 0 {
return PollResult::None;
}

let mut ep_out = 0;
let mut ep_in_complete = 0;
let mut ep_setup = 0;

let intbits = self.usb().epintsmry.read().bits();

for ep in 0..8u16 {
let mask = 1 << ep;
if (intbits & mask) == 0 {
continue;
}

let idx = ep as usize;

if let Ok(bank1) = self.bank1(EndpointAddress::from_parts(idx, UsbDirection::In)) {
if bank1.is_transfer_complete() {
bank1.clear_transfer_complete();
ep_in_complete |= mask;
if (intbits & mask) != 0 {
if let Ok(bank1) = self.bank1(EndpointAddress::from_parts(idx, UsbDirection::In)) {
if bank1.is_transfer_complete() {
bank1.clear_transfer_complete();
ep_in_complete |= mask;
}
}
}

// Can't test intbits, because bk0rdy doesn't interrupt
if let Ok(bank0) = self.bank0(EndpointAddress::from_parts(idx, UsbDirection::Out)) {
if bank0.received_setup_interrupt() {
ep_setup |= mask;
// usb-device crate:
// "This event should continue to be reported until the
// packet is read." So we don't clear the flag here,
// instead it is cleared in the read handler.

// The RXSTP interrupt is not cleared here, because doing so
// would allow the USB hardware to overwrite the received
// data, potentially before it is `read()` - see SAMD21
// datasheet "32.6.2.6 Management of SETUP Transactions".
// Setup events are only relevant for control endpoints, and
// in typical USB devices, endpoint 0 is the only control
// endpoint. The usb-device `poll()` method, which calls
// this `poll()`, will immediately `read()` endpoint 0 when
// its setup bit is set.
}

if bank0.is_transfer_complete() {
// Clear the transfer complete and transfer failed interrupt flags
// so that execution leaves the USB interrupt until the host makes
// another transaction. The transfer failed flag may have been set
// if an OUT transaction wasn't read() from the endpoint by the
// Class; the hardware will have NAKed (unless the endpoint is
// isochronous) and the host may retry.
bank0.clear_transfer_complete();

// Use the bk0rdy flag via is_ready() to indicate that data has been
// received successfully, rather than the interrupting trcpt0 via
// is_transfer_ready(), because data may have been received on an
// earlier poll() which cleared trcpt0. bk0rdy is cleared in the
// endpoint read().
if bank0.is_ready() {
ep_out |= mask;
}
}
}

PollResult::Data {
ep_out,
ep_in_complete,
ep_setup,
if ep_out == 0 && ep_in_complete == 0 && ep_setup == 0 {
PollResult::None
} else {
PollResult::Data {
ep_out,
ep_in_complete,
ep_setup,
}
}
}

Expand Down Expand Up @@ -898,8 +912,6 @@ impl Inner {
bank.clear_transfer_complete();
bank.set_ready(false);

drop(bank);

size
} else {
Err(UsbError::WouldBlock)
Expand Down
72 changes: 41 additions & 31 deletions hal/src/thumbv7em/usb/bus.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -393,13 +393,6 @@ impl<'a> Bank<'a, OutBank> {
.write(|w| w.trcpt0().set_bit().trfail0().set_bit());
}

/// Checks if data has been received. Returns true for failed transfers
/// as well as successful transfers.
#[inline]
fn is_transfer_complete(&self) -> bool {
self.epintflag(self.index()).read().trcpt0().bit()
}

/// Returns true if a Received Setup interrupt has occurred.
/// This indicates that the read buffer holds a SETUP packet.
#[inline]
Expand Down Expand Up @@ -555,7 +548,6 @@ impl Inner {

fn set_stall<EP: Into<EndpointAddress>>(&self, ep: EP, stall: bool) {
let ep = ep.into();

if ep.is_out() {
if let Ok(mut bank) = self.bank0(ep) {
bank.set_stall(stall);
Expand Down Expand Up @@ -767,49 +759,69 @@ impl Inner {
// unconnected & unsuspended, we do not handle them to avoid spurious
// transitions.

let intbits = self.usb().epintsmry.read().bits();
if intbits == 0 {
return PollResult::None;
}

let mut ep_out = 0;
let mut ep_in_complete = 0;
let mut ep_setup = 0;

let intbits = self.usb().epintsmry.read().bits();

for ep in 0..8u16 {
let mask = 1 << ep;
if (intbits & mask) == 0 {
continue;
}

let idx = ep as usize;

if let Ok(bank1) = self.bank1(EndpointAddress::from_parts(idx, UsbDirection::In)) {
if bank1.is_transfer_complete() {
bank1.clear_transfer_complete();
ep_in_complete |= mask;
if (intbits & mask) != 0 {
if let Ok(bank1) = self.bank1(EndpointAddress::from_parts(idx, UsbDirection::In)) {
if bank1.is_transfer_complete() {
bank1.clear_transfer_complete();
ep_in_complete |= mask;
}
}
}

// Can't test intbits, because bk0rdy doesn't interrupt
if let Ok(bank0) = self.bank0(EndpointAddress::from_parts(idx, UsbDirection::Out)) {
if bank0.received_setup_interrupt() {
ep_setup |= mask;
// usb-device crate:
// "This event should continue to be reported until the
// packet is read." So we don't clear the flag here,
// instead it is cleared in the read handler.

// The RXSTP interrupt is not cleared here, because doing so
// would allow the USB hardware to overwrite the received
// data, potentially before it is `read()` - see SAMD5x
// datasheet "38.6.2.6 Management of SETUP Transactions".
// Setup events are only relevant for control endpoints, and
// in typical USB devices, endpoint 0 is the only control
// endpoint. The usb-device `poll()` method, which calls
// this `poll()`, will immediately `read()` endpoint 0 when
// its setup bit is set.
}

if bank0.is_transfer_complete() {
// Clear the transfer complete and transfer failed interrupt flags
// so that execution leaves the USB interrupt until the host makes
// another transaction. The transfer failed flag may have been set
// if an OUT transaction wasn't read() from the endpoint by the
// Class; the hardware will have NAKed (unless the endpoint is
// isochronous) and the host may retry.
bank0.clear_transfer_complete();

// Use the bk0rdy flag via is_ready() to indicate that data has been
// received successfully, rather than the interrupting trcpt0 via
// is_transfer_ready(), because data may have been received on an
// earlier poll() which cleared trcpt0. bk0rdy is cleared in the
// endpoint read().
if bank0.is_ready() {
ep_out |= mask;
}
}
}

PollResult::Data {
ep_out,
ep_in_complete,
ep_setup,
if ep_out == 0 && ep_in_complete == 0 && ep_setup == 0 {
PollResult::None
} else {
PollResult::Data {
ep_out,
ep_in_complete,
ep_setup,
}
}
}

Expand Down Expand Up @@ -843,8 +855,6 @@ impl Inner {
bank.clear_transfer_complete();
bank.set_ready(false);

drop(bank);

size
} else {
Err(UsbError::WouldBlock)
Expand Down

0 comments on commit e893102

Please sign in to comment.