From f5656e354485888cc7c2697052ae2c79b94a59ad Mon Sep 17 00:00:00 2001 From: alexmoon Date: Sun, 10 Apr 2022 15:41:51 -0400 Subject: [PATCH 1/7] Add DeviceStateHandler, DeviceCommand channel, and remote wakeup support --- embassy-nrf/src/usb.rs | 104 +++++++++----- embassy-usb-hid/src/lib.rs | 13 +- embassy-usb-serial/src/lib.rs | 5 +- embassy-usb/src/builder.rs | 77 +++++++++- embassy-usb/src/driver.rs | 44 +++--- embassy-usb/src/lib.rs | 144 ++++++++++++++++--- embassy-usb/src/util.rs | 72 +++++++--- examples/nrf/src/bin/usb_hid_keyboard.rs | 114 ++++++++++++++- examples/nrf/src/bin/usb_hid_mouse.rs | 3 +- examples/nrf/src/bin/usb_serial.rs | 3 +- examples/nrf/src/bin/usb_serial_multitask.rs | 3 +- 11 files changed, 472 insertions(+), 110 deletions(-) diff --git a/embassy-nrf/src/usb.rs b/embassy-nrf/src/usb.rs index 5e2f585f2..3d90f4b9a 100644 --- a/embassy-nrf/src/usb.rs +++ b/embassy-nrf/src/usb.rs @@ -10,7 +10,7 @@ use embassy::util::Unborrow; use embassy::waitqueue::AtomicWaker; use embassy_hal_common::unborrow; use embassy_usb::control::Request; -use embassy_usb::driver::{self, EndpointError, Event}; +use embassy_usb::driver::{self, EndpointError, Event, Unsupported}; use embassy_usb::types::{EndpointAddress, EndpointInfo, EndpointType, UsbDirection}; use futures::future::poll_fn; use futures::Future; @@ -140,7 +140,6 @@ impl<'d, T: Instance> driver::Driver<'d> for Driver<'d, T> { type EndpointIn = Endpoint<'d, T, In>; type ControlPipe = ControlPipe<'d, T>; type Bus = Bus<'d, T>; - type EnableFuture = impl Future + 'd; fn alloc_endpoint_in( &mut self, @@ -192,7 +191,27 @@ impl<'d, T: Instance> driver::Driver<'d> for Driver<'d, T> { }) } - fn enable(self) -> Self::EnableFuture { + fn into_bus(self) -> Self::Bus { + Bus { + phantom: PhantomData, + alloc_in: self.alloc_in, + alloc_out: self.alloc_out, + } + } +} + +pub struct Bus<'d, T: Instance> { + phantom: PhantomData<&'d mut T>, + alloc_in: Allocator, + alloc_out: Allocator, +} + +impl<'d, T: Instance> driver::Bus for Bus<'d, T> { + type EnableFuture<'a> = impl Future + 'a where Self: 'a; + type PollFuture<'a> = impl Future + 'a where Self: 'a; + type RemoteWakeupFuture<'a> = impl Future> + 'a where Self: 'a; + + fn enable(&mut self) -> Self::EnableFuture<'_> { async move { let regs = T::regs(); @@ -226,33 +245,23 @@ impl<'d, T: Instance> driver::Driver<'d> for Driver<'d, T> { // Enable the USB pullup, allowing enumeration. regs.usbpullup.write(|w| w.connect().enabled()); trace!("enabled"); - - Bus { - phantom: PhantomData, - alloc_in: self.alloc_in, - alloc_out: self.alloc_out, - } } } -} -pub struct Bus<'d, T: Instance> { - phantom: PhantomData<&'d mut T>, - alloc_in: Allocator, - alloc_out: Allocator, -} - -impl<'d, T: Instance> driver::Bus for Bus<'d, T> { - type PollFuture<'a> = impl Future + 'a where Self: 'a; + fn disable(&mut self) { + let regs = T::regs(); + regs.enable.write(|x| x.enable().disabled()); + } fn poll<'a>(&'a mut self) -> Self::PollFuture<'a> { - poll_fn(|cx| { + poll_fn(move |cx| { BUS_WAKER.register(cx.waker()); let regs = T::regs(); if regs.events_usbreset.read().bits() != 0 { regs.events_usbreset.reset(); regs.intenset.write(|w| w.usbreset().set()); + self.set_configured(false); return Poll::Ready(Event::Reset); } @@ -268,11 +277,12 @@ impl<'d, T: Instance> driver::Bus for Bus<'d, T> { } if r.suspend().bit() { regs.eventcause.write(|w| w.suspend().set_bit()); - trace!("USB event: suspend"); + regs.lowpower.write(|w| w.lowpower().low_power()); + return Poll::Ready(Event::Suspend); } if r.resume().bit() { regs.eventcause.write(|w| w.resume().set_bit()); - trace!("USB event: resume"); + return Poll::Ready(Event::Resume); } if r.ready().bit() { regs.eventcause.write(|w| w.ready().set_bit()); @@ -283,11 +293,6 @@ impl<'d, T: Instance> driver::Bus for Bus<'d, T> { }) } - #[inline] - fn reset(&mut self) { - self.set_configured(false); - } - #[inline] fn set_configured(&mut self, configured: bool) { let regs = T::regs(); @@ -343,18 +348,43 @@ impl<'d, T: Instance> driver::Bus for Bus<'d, T> { } #[inline] - fn suspend(&mut self) { - let regs = T::regs(); - regs.lowpower.write(|w| w.lowpower().low_power()); - } + fn remote_wakeup(&mut self) -> Self::RemoteWakeupFuture<'_> { + async move { + let regs = T::regs(); - #[inline] - fn resume(&mut self) { - let regs = T::regs(); + if regs.lowpower.read().lowpower().is_low_power() { + errata::pre_wakeup(); - errata::pre_wakeup(); + regs.lowpower.write(|w| w.lowpower().force_normal()); - regs.lowpower.write(|w| w.lowpower().force_normal()); + poll_fn(|cx| { + BUS_WAKER.register(cx.waker()); + let regs = T::regs(); + let r = regs.eventcause.read(); + + if regs.events_usbreset.read().bits() != 0 { + Poll::Ready(()) + } else if r.resume().bit() { + Poll::Ready(()) + } else if r.usbwuallowed().bit() { + regs.eventcause.write(|w| w.usbwuallowed().set_bit()); + + regs.dpdmvalue.write(|w| w.state().resume()); + regs.tasks_dpdmdrive + .write(|w| w.tasks_dpdmdrive().set_bit()); + + Poll::Ready(()) + } else { + Poll::Pending + } + }) + .await; + + errata::post_wakeup(); + } + + Ok(()) + } } } @@ -845,6 +875,7 @@ mod errata { pub fn pre_enable() { // Works around Erratum 187 on chip revisions 1 and 2. + #[cfg(any(feature = "nrf52840", feature = "nrf52833", feature = "nrf52820"))] unsafe { poke(0x4006EC00, 0x00009375); poke(0x4006ED14, 0x00000003); @@ -858,6 +889,7 @@ mod errata { post_wakeup(); // Works around Erratum 187 on chip revisions 1 and 2. + #[cfg(any(feature = "nrf52840", feature = "nrf52833", feature = "nrf52820"))] unsafe { poke(0x4006EC00, 0x00009375); poke(0x4006ED14, 0x00000000); @@ -868,6 +900,7 @@ mod errata { pub fn pre_wakeup() { // Works around Erratum 171 on chip revisions 1 and 2. + #[cfg(feature = "nrf52840")] unsafe { if peek(0x4006EC00) == 0x00000000 { poke(0x4006EC00, 0x00009375); @@ -881,6 +914,7 @@ mod errata { pub fn post_wakeup() { // Works around Erratum 171 on chip revisions 1 and 2. + #[cfg(feature = "nrf52840")] unsafe { if peek(0x4006EC00) == 0x00000000 { poke(0x4006EC00, 0x00009375); diff --git a/embassy-usb-hid/src/lib.rs b/embassy-usb-hid/src/lib.rs index e870becf5..4a1df0eab 100644 --- a/embassy-usb-hid/src/lib.rs +++ b/embassy-usb-hid/src/lib.rs @@ -11,6 +11,7 @@ use core::mem::MaybeUninit; use core::ops::Range; use core::sync::atomic::{AtomicUsize, Ordering}; +use embassy::blocking_mutex::raw::RawMutex; use embassy::time::Duration; use embassy_usb::driver::EndpointOut; use embassy_usb::{ @@ -88,8 +89,8 @@ impl<'d, D: Driver<'d>, const IN_N: usize> HidClass<'d, D, (), IN_N> { /// high performance uses, and a value of 255 is good for best-effort usecases. /// /// This allocates an IN endpoint only. - pub fn new( - builder: &mut UsbDeviceBuilder<'d, D>, + pub fn new( + builder: &mut UsbDeviceBuilder<'d, D, M>, state: &'d mut State<'d, IN_N, OUT_N>, report_descriptor: &'static [u8], request_handler: Option<&'d dyn RequestHandler>, @@ -132,8 +133,8 @@ impl<'d, D: Driver<'d>, const IN_N: usize, const OUT_N: usize> /// high performance uses, and a value of 255 is good for best-effort usecases. /// /// This allocates two endpoints (IN and OUT). - pub fn with_output_ep( - builder: &mut UsbDeviceBuilder<'d, D>, + pub fn with_output_ep( + builder: &mut UsbDeviceBuilder<'d, D, M>, state: &'d mut State<'d, IN_N, OUT_N>, report_descriptor: &'static [u8], request_handler: Option<&'d dyn RequestHandler>, @@ -392,9 +393,9 @@ impl<'a> Control<'a> { } } - fn build<'d, D: Driver<'d>>( + fn build<'d, D: Driver<'d>, M: RawMutex>( &'d mut self, - builder: &mut UsbDeviceBuilder<'d, D>, + builder: &mut UsbDeviceBuilder<'d, D, M>, ep_out: Option<&D::EndpointOut>, ep_in: &D::EndpointIn, ) { diff --git a/embassy-usb-serial/src/lib.rs b/embassy-usb-serial/src/lib.rs index 7b25398d0..7f006c0fd 100644 --- a/embassy-usb-serial/src/lib.rs +++ b/embassy-usb-serial/src/lib.rs @@ -8,6 +8,7 @@ pub(crate) mod fmt; use core::cell::Cell; use core::mem::{self, MaybeUninit}; use core::sync::atomic::{AtomicBool, Ordering}; +use embassy::blocking_mutex::raw::RawMutex; use embassy::blocking_mutex::CriticalSectionMutex; use embassy_usb::control::{self, ControlHandler, InResponse, OutResponse, Request}; use embassy_usb::driver::{Endpoint, EndpointError, EndpointIn, EndpointOut}; @@ -162,8 +163,8 @@ impl<'d> ControlHandler for Control<'d> { impl<'d, D: Driver<'d>> CdcAcmClass<'d, D> { /// Creates a new CdcAcmClass with the provided UsbBus and max_packet_size in bytes. For /// full-speed devices, max_packet_size has to be one of 8, 16, 32 or 64. - pub fn new( - builder: &mut UsbDeviceBuilder<'d, D>, + pub fn new( + builder: &mut UsbDeviceBuilder<'d, D, M>, state: &'d mut State<'d>, max_packet_size: u16, ) -> Self { diff --git a/embassy-usb/src/builder.rs b/embassy-usb/src/builder.rs index 4bbcd3e56..30d31ac74 100644 --- a/embassy-usb/src/builder.rs +++ b/embassy-usb/src/builder.rs @@ -1,9 +1,14 @@ +use embassy::blocking_mutex::raw::{NoopRawMutex, RawMutex}; +use embassy::channel::Channel; use heapless::Vec; +use crate::DeviceCommand; + use super::control::ControlHandler; use super::descriptor::{BosWriter, DescriptorWriter}; use super::driver::{Driver, EndpointAllocError}; use super::types::*; +use super::DeviceStateHandler; use super::UsbDevice; use super::MAX_INTERFACE_COUNT; @@ -93,6 +98,11 @@ pub struct Config<'a> { /// Default: 100mA /// Max: 500mA pub max_power: u16, + + /// Whether the USB bus should be enabled when built. + /// + /// Default: true + pub start_enabled: bool, } impl<'a> Config<'a> { @@ -112,15 +122,18 @@ impl<'a> Config<'a> { supports_remote_wakeup: false, composite_with_iads: false, max_power: 100, + start_enabled: true, } } } /// Used to build new [`UsbDevice`]s. -pub struct UsbDeviceBuilder<'d, D: Driver<'d>> { +pub struct UsbDeviceBuilder<'d, D: Driver<'d>, M: RawMutex> { config: Config<'d>, + handler: Option<&'d dyn DeviceStateHandler>, interfaces: Vec<(u8, &'d mut dyn ControlHandler), MAX_INTERFACE_COUNT>, control_buf: &'d mut [u8], + commands: Option<&'d Channel>, driver: D, next_interface_number: u8, @@ -132,7 +145,7 @@ pub struct UsbDeviceBuilder<'d, D: Driver<'d>> { pub bos_descriptor: BosWriter<'d>, } -impl<'d, D: Driver<'d>> UsbDeviceBuilder<'d, D> { +impl<'d, D: Driver<'d>> UsbDeviceBuilder<'d, D, NoopRawMutex> { /// Creates a builder for constructing a new [`UsbDevice`]. /// /// `control_buf` is a buffer used for USB control request data. It should be sized @@ -145,6 +158,58 @@ impl<'d, D: Driver<'d>> UsbDeviceBuilder<'d, D> { config_descriptor_buf: &'d mut [u8], bos_descriptor_buf: &'d mut [u8], control_buf: &'d mut [u8], + handler: Option<&'d dyn DeviceStateHandler>, + ) -> Self { + Self::new_inner( + driver, + config, + device_descriptor_buf, + config_descriptor_buf, + bos_descriptor_buf, + control_buf, + handler, + None, + ) + } +} + +impl<'d, D: Driver<'d>, M: RawMutex> UsbDeviceBuilder<'d, D, M> { + /// Creates a builder for constructing a new [`UsbDevice`]. + /// + /// `control_buf` is a buffer used for USB control request data. It should be sized + /// large enough for the length of the largest control request (in or out) + /// anticipated by any class added to the device. + pub fn new_with_channel( + driver: D, + config: Config<'d>, + device_descriptor_buf: &'d mut [u8], + config_descriptor_buf: &'d mut [u8], + bos_descriptor_buf: &'d mut [u8], + control_buf: &'d mut [u8], + handler: Option<&'d dyn DeviceStateHandler>, + channel: &'d Channel, + ) -> Self { + Self::new_inner( + driver, + config, + device_descriptor_buf, + config_descriptor_buf, + bos_descriptor_buf, + control_buf, + handler, + Some(channel), + ) + } + + fn new_inner( + driver: D, + config: Config<'d>, + device_descriptor_buf: &'d mut [u8], + config_descriptor_buf: &'d mut [u8], + bos_descriptor_buf: &'d mut [u8], + control_buf: &'d mut [u8], + handler: Option<&'d dyn DeviceStateHandler>, + channel: Option<&'d Channel>, ) -> Self { // Magic values specified in USB-IF ECN on IADs. if config.composite_with_iads @@ -174,9 +239,12 @@ impl<'d, D: Driver<'d>> UsbDeviceBuilder<'d, D> { UsbDeviceBuilder { driver, + handler, config, interfaces: Vec::new(), control_buf, + commands: channel, + next_interface_number: 0, next_string_index: 4, @@ -187,20 +255,21 @@ impl<'d, D: Driver<'d>> UsbDeviceBuilder<'d, D> { } /// Creates the [`UsbDevice`] instance with the configuration in this builder. - pub async fn build(mut self) -> UsbDevice<'d, D> { + pub fn build(mut self) -> UsbDevice<'d, D, M> { self.config_descriptor.end_configuration(); self.bos_descriptor.end_bos(); UsbDevice::build( self.driver, self.config, + self.handler, + self.commands, self.device_descriptor.into_buf(), self.config_descriptor.into_buf(), self.bos_descriptor.writer.into_buf(), self.interfaces, self.control_buf, ) - .await } /// Allocates a new interface number. diff --git a/embassy-usb/src/driver.rs b/embassy-usb/src/driver.rs index 875ceafcb..99610deef 100644 --- a/embassy-usb/src/driver.rs +++ b/embassy-usb/src/driver.rs @@ -11,7 +11,6 @@ pub trait Driver<'a> { type EndpointIn: EndpointIn + 'a; type ControlPipe: ControlPipe + 'a; type Bus: Bus + 'a; - type EnableFuture: Future + 'a; /// Allocates an endpoint and specified endpoint parameters. This method is called by the device /// and class implementations to allocate endpoints, and can only be called before @@ -47,7 +46,7 @@ pub trait Driver<'a> { /// Enables and initializes the USB peripheral. Soon after enabling the device will be reset, so /// there is no need to perform a USB reset in this method. - fn enable(self) -> Self::EnableFuture; + fn into_bus(self) -> Self::Bus; /// Indicates that `set_device_address` must be called before accepting the corresponding /// control transfer, not after. @@ -57,19 +56,25 @@ pub trait Driver<'a> { } pub trait Bus { + type EnableFuture<'a>: Future + 'a + where + Self: 'a; type PollFuture<'a>: Future + 'a where Self: 'a; + type RemoteWakeupFuture<'a>: Future> + 'a + where + Self: 'a; + + /// Enables the USB peripheral. Soon after enabling the device will be reset, so + /// there is no need to perform a USB reset in this method. + fn enable(&mut self) -> Self::EnableFuture<'_>; + + /// Disables and powers down the USB peripheral. + fn disable(&mut self); fn poll<'a>(&'a mut self) -> Self::PollFuture<'a>; - /// Called when the host resets the device. This will be soon called after - /// [`poll`](crate::device::UsbDevice::poll) returns [`PollResult::Reset`]. This method should - /// reset the state of all endpoints and peripheral flags back to a state suitable for - /// enumeration, as well as ensure that all endpoints previously allocated with alloc_ep are - /// initialized as specified. - fn reset(&mut self); - /// Sets the device USB address to `addr`. fn set_device_address(&mut self, addr: u8); @@ -83,17 +88,6 @@ pub trait Bus { /// Gets whether the STALL condition is set for an endpoint. Only used during control transfers. fn is_stalled(&mut self, ep_addr: EndpointAddress) -> bool; - /// Causes the USB peripheral to enter USB suspend mode, lowering power consumption and - /// preparing to detect a USB wakeup event. This will be called after - /// [`poll`](crate::device::UsbDevice::poll) returns [`PollResult::Suspend`]. The device will - /// continue be polled, and it shall return a value other than `Suspend` from `poll` when it no - /// longer detects the suspend condition. - fn suspend(&mut self); - - /// Resumes from suspend mode. This may only be called after the peripheral has been previously - /// suspended. - fn resume(&mut self); - /// Simulates a disconnect from the USB bus, causing the host to reset and re-enumerate the /// device. /// @@ -106,6 +100,16 @@ pub trait Bus { fn force_reset(&mut self) -> Result<(), Unsupported> { Err(Unsupported) } + + /// Initiates a remote wakeup of the host by the device. + /// + /// The default implementation just returns `Unsupported`. + /// + /// # Errors + /// + /// * [`Unsupported`](crate::UsbError::Unsupported) - This UsbBus implementation doesn't support + /// remote wakeup or it has not been enabled at creation time. + fn remote_wakeup(&mut self) -> Self::RemoteWakeupFuture<'_>; } pub trait Endpoint { diff --git a/embassy-usb/src/lib.rs b/embassy-usb/src/lib.rs index e98cbdee3..2c53d9214 100644 --- a/embassy-usb/src/lib.rs +++ b/embassy-usb/src/lib.rs @@ -12,6 +12,9 @@ pub mod driver; pub mod types; mod util; +use driver::Unsupported; +use embassy::blocking_mutex::raw::{NoopRawMutex, RawMutex}; +use embassy::channel::Channel; use heapless::Vec; use self::control::*; @@ -28,8 +31,12 @@ pub use self::builder::UsbDeviceBuilder; /// In general class traffic is only possible in the `Configured` state. #[repr(u8)] #[derive(PartialEq, Eq, Copy, Clone, Debug)] +#[cfg_attr(feature = "defmt", derive(defmt::Format))] pub enum UsbDeviceState { - /// The USB device has just been created or reset. + /// The USB device is disabled. + Disabled, + + /// The USB device has just been enabled or reset. Default, /// The USB device has received an address from the host. @@ -37,9 +44,6 @@ pub enum UsbDeviceState { /// The USB device has been configured and is fully functional. Configured, - - /// The USB device has been suspended by the host or it has been unplugged from the USB bus. - Suspend, } /// The bConfiguration value for the not configured state. @@ -53,8 +57,39 @@ pub const DEFAULT_ALTERNATE_SETTING: u8 = 0; pub const MAX_INTERFACE_COUNT: usize = 4; -pub struct UsbDevice<'d, D: Driver<'d>> { +#[derive(PartialEq, Eq, Copy, Clone, Debug)] +#[cfg_attr(feature = "defmt", derive(defmt::Format))] +pub enum DeviceCommand { + Enable, + Disable, + RemoteWakeup, +} + +/// A handler trait for changes in the device state of the [UsbDevice]. +pub trait DeviceStateHandler { + /// Called when the host resets the device. + fn reset(&self) {} + + /// Called when the host has set the address of the device to `addr`. + fn addressed(&self, _addr: u8) {} + + /// Called when the host has enabled or disabled the configuration of the device. + fn configured(&self, _configured: bool) {} + + /// Called when the bus has entered or exited the suspend state. + fn suspended(&self, _suspended: bool) {} + + /// Called when remote wakeup feature is enabled or disabled. + fn remote_wakeup_enabled(&self, _enabled: bool) {} + + /// Called when the USB device has been disabled. + fn disabled(&self) {} +} + +pub struct UsbDevice<'d, D: Driver<'d>, M: RawMutex = NoopRawMutex> { bus: D::Bus, + handler: Option<&'d dyn DeviceStateHandler>, + commands: Option<&'d Channel>, control: ControlPipe, config: Config<'d>, @@ -64,6 +99,7 @@ pub struct UsbDevice<'d, D: Driver<'d>> { control_buf: &'d mut [u8], device_state: UsbDeviceState, + suspended: bool, remote_wakeup_enabled: bool, self_powered: bool, pending_address: u8, @@ -71,33 +107,38 @@ pub struct UsbDevice<'d, D: Driver<'d>> { interfaces: Vec<(u8, &'d mut dyn ControlHandler), MAX_INTERFACE_COUNT>, } -impl<'d, D: Driver<'d>> UsbDevice<'d, D> { - pub(crate) async fn build( +impl<'d, D: Driver<'d>, M: RawMutex> UsbDevice<'d, D, M> { + pub(crate) fn build( mut driver: D, config: Config<'d>, + handler: Option<&'d dyn DeviceStateHandler>, + commands: Option<&'d Channel>, device_descriptor: &'d [u8], config_descriptor: &'d [u8], bos_descriptor: &'d [u8], interfaces: Vec<(u8, &'d mut dyn ControlHandler), MAX_INTERFACE_COUNT>, control_buf: &'d mut [u8], - ) -> UsbDevice<'d, D> { + ) -> UsbDevice<'d, D, M> { let control = driver .alloc_control_pipe(config.max_packet_size_0 as u16) .expect("failed to alloc control endpoint"); // Enable the USB bus. // This prevent further allocation by consuming the driver. - let bus = driver.enable().await; + let bus = driver.into_bus(); Self { bus, config, + handler, + commands, control: ControlPipe::new(control), device_descriptor, config_descriptor, bos_descriptor, control_buf, device_state: UsbDeviceState::Default, + suspended: false, remote_wakeup_enabled: false, self_powered: false, pending_address: 0, @@ -105,41 +146,94 @@ impl<'d, D: Driver<'d>> UsbDevice<'d, D> { } } - pub async fn run(&mut self) { + pub async fn run(&mut self) -> ! { + if self.config.start_enabled { + self.bus.enable().await; + } else { + self.wait_for_enable().await + } + loop { let control_fut = self.control.setup(); let bus_fut = self.bus.poll(); - match select(bus_fut, control_fut).await { - Either::Left(evt) => match evt { + let commands_fut = recv_or_wait(self.commands); + + match select3(bus_fut, control_fut, commands_fut).await { + Either3::First(evt) => match evt { Event::Reset => { trace!("usb: reset"); - self.bus.reset(); - self.device_state = UsbDeviceState::Default; + self.suspended = false; self.remote_wakeup_enabled = false; self.pending_address = 0; for (_, h) in self.interfaces.iter_mut() { h.reset(); } + + if let Some(h) = &mut self.handler { + h.reset(); + } } Event::Resume => { trace!("usb: resume"); + self.suspended = false; + if let Some(h) = &mut self.handler { + h.suspended(false); + } } Event::Suspend => { trace!("usb: suspend"); - self.bus.suspend(); - self.device_state = UsbDeviceState::Suspend; + self.suspended = true; + if let Some(h) = &mut self.handler { + h.suspended(true); + } } }, - Either::Right(req) => match req { + Either3::Second(req) => match req { Setup::DataIn(req, stage) => self.handle_control_in(req, stage).await, Setup::DataOut(req, stage) => self.handle_control_out(req, stage).await, }, + Either3::Third(cmd) => match cmd { + DeviceCommand::Enable => warn!("usb: Enable command received while enabled."), + DeviceCommand::Disable => { + trace!("usb: disable"); + self.bus.disable(); + self.device_state = UsbDeviceState::Disabled; + if let Some(h) = &mut self.handler { + h.disabled(); + } + self.wait_for_enable().await; + } + DeviceCommand::RemoteWakeup => { + if self.remote_wakeup_enabled { + match self.bus.remote_wakeup().await { + Ok(()) => (), + Err(Unsupported) => warn!("Remote wakeup is unsupported!"), + } + } else { + warn!("Remote wakeup not enabled."); + } + } + }, } } } + async fn wait_for_enable(&mut self) { + loop { + // When disabled just wait until we're told to re-enable + match recv_or_wait(self.commands).await { + DeviceCommand::Enable => break, + cmd => warn!("usb: {:?} received while disabled", cmd), + } + } + + trace!("usb: enable"); + self.bus.enable().await; + self.device_state = UsbDeviceState::Default; + } + async fn handle_control_out(&mut self, req: Request, stage: DataOutStage) { const CONFIGURATION_NONE_U16: u16 = CONFIGURATION_NONE as u16; const CONFIGURATION_VALUE_U16: u16 = CONFIGURATION_VALUE as u16; @@ -156,20 +250,33 @@ impl<'d, D: Driver<'d>> UsbDevice<'d, D> { (RequestType::Standard, Recipient::Device) => match (req.request, req.value) { (Request::CLEAR_FEATURE, Request::FEATURE_DEVICE_REMOTE_WAKEUP) => { self.remote_wakeup_enabled = false; + if let Some(h) = &mut self.handler { + h.remote_wakeup_enabled(false); + } self.control.accept(stage) } (Request::SET_FEATURE, Request::FEATURE_DEVICE_REMOTE_WAKEUP) => { self.remote_wakeup_enabled = true; + if let Some(h) = &mut self.handler { + h.remote_wakeup_enabled(true); + } self.control.accept(stage) } (Request::SET_ADDRESS, addr @ 1..=127) => { self.pending_address = addr as u8; self.bus.set_device_address(self.pending_address); + self.device_state = UsbDeviceState::Addressed; + if let Some(h) = &mut self.handler { + h.addressed(self.pending_address); + } self.control.accept(stage) } (Request::SET_CONFIGURATION, CONFIGURATION_VALUE_U16) => { self.device_state = UsbDeviceState::Configured; self.bus.set_configured(true); + if let Some(h) = &mut self.handler { + h.configured(true); + } self.control.accept(stage) } (Request::SET_CONFIGURATION, CONFIGURATION_NONE_U16) => match self.device_state { @@ -177,6 +284,9 @@ impl<'d, D: Driver<'d>> UsbDevice<'d, D> { _ => { self.device_state = UsbDeviceState::Addressed; self.bus.set_configured(false); + if let Some(h) = &mut self.handler { + h.configured(false); + } self.control.accept(stage) } }, diff --git a/embassy-usb/src/util.rs b/embassy-usb/src/util.rs index 18cc875c6..4fc55461f 100644 --- a/embassy-usb/src/util.rs +++ b/embassy-usb/src/util.rs @@ -1,45 +1,85 @@ use core::future::Future; +use core::marker::PhantomData; use core::pin::Pin; use core::task::{Context, Poll}; +use embassy::blocking_mutex::raw::RawMutex; +use embassy::channel::Channel; + #[derive(Debug, Clone)] -pub enum Either { - Left(A), - Right(B), +pub enum Either3 { + First(A), + Second(B), + Third(C), } -pub fn select(a: A, b: B) -> Select +/// Same as [`select`], but with more futures. +pub fn select3(a: A, b: B, c: C) -> Select3 where A: Future, B: Future, + C: Future, { - Select { a, b } + Select3 { a, b, c } } -pub struct Select { +/// Future for the [`select3`] function. +#[derive(Debug)] +#[must_use = "futures do nothing unless you `.await` or poll them"] +pub struct Select3 { a: A, b: B, + c: C, } -impl Unpin for Select {} - -impl Future for Select +impl Future for Select3 where A: Future, B: Future, + C: Future, { - type Output = Either; + type Output = Either3; fn poll(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll { let this = unsafe { self.get_unchecked_mut() }; let a = unsafe { Pin::new_unchecked(&mut this.a) }; let b = unsafe { Pin::new_unchecked(&mut this.b) }; - match a.poll(cx) { - Poll::Ready(x) => Poll::Ready(Either::Left(x)), - Poll::Pending => match b.poll(cx) { - Poll::Ready(x) => Poll::Ready(Either::Right(x)), - Poll::Pending => Poll::Pending, - }, + let c = unsafe { Pin::new_unchecked(&mut this.c) }; + if let Poll::Ready(x) = a.poll(cx) { + return Poll::Ready(Either3::First(x)); + } + if let Poll::Ready(x) = b.poll(cx) { + return Poll::Ready(Either3::Second(x)); + } + if let Poll::Ready(x) = c.poll(cx) { + return Poll::Ready(Either3::Third(x)); + } + Poll::Pending + } +} + +pub struct Pending { + _phantom: PhantomData, +} + +impl Pending { + fn new() -> Self { + Pending { + _phantom: PhantomData, } } } + +impl Future for Pending { + type Output = T; + fn poll(self: Pin<&mut Self>, _cx: &mut Context<'_>) -> Poll { + Poll::Pending + } +} + +pub async fn recv_or_wait(ch: Option<&Channel>) -> T { + match ch { + Some(ch) => ch.recv().await, + None => Pending::new().await, + } +} diff --git a/examples/nrf/src/bin/usb_hid_keyboard.rs b/examples/nrf/src/bin/usb_hid_keyboard.rs index 0812697e4..483d86b81 100644 --- a/examples/nrf/src/bin/usb_hid_keyboard.rs +++ b/examples/nrf/src/bin/usb_hid_keyboard.rs @@ -4,8 +4,12 @@ #![feature(type_alias_impl_trait)] use core::mem; +use core::sync::atomic::{AtomicBool, Ordering}; use defmt::*; +use embassy::blocking_mutex::raw::CriticalSectionRawMutex; +use embassy::channel::Channel; use embassy::executor::Spawner; +use embassy::interrupt::InterruptExt; use embassy::time::Duration; use embassy_nrf::gpio::{Input, Pin, Pull}; use embassy_nrf::interrupt; @@ -13,7 +17,7 @@ use embassy_nrf::pac; use embassy_nrf::usb::Driver; use embassy_nrf::Peripherals; use embassy_usb::control::OutResponse; -use embassy_usb::{Config, UsbDeviceBuilder}; +use embassy_usb::{Config, DeviceCommand, DeviceStateHandler, UsbDeviceBuilder}; use embassy_usb_hid::{HidClass, ReportId, RequestHandler, State}; use futures::future::join; use usbd_hid::descriptor::{KeyboardReport, SerializedDescriptor}; @@ -21,6 +25,29 @@ use usbd_hid::descriptor::{KeyboardReport, SerializedDescriptor}; use defmt_rtt as _; // global logger use panic_probe as _; +static USB_COMMANDS: Channel = Channel::new(); +static SUSPENDED: AtomicBool = AtomicBool::new(false); + +fn on_power_interrupt(_: *mut ()) { + let regs = unsafe { &*pac::POWER::ptr() }; + + if regs.events_usbdetected.read().bits() != 0 { + regs.events_usbdetected.reset(); + info!("Vbus detected, enabling USB..."); + if USB_COMMANDS.try_send(DeviceCommand::Enable).is_err() { + warn!("Failed to send enable command to USB channel"); + } + } + + if regs.events_usbremoved.read().bits() != 0 { + regs.events_usbremoved.reset(); + info!("Vbus removed, disabling USB..."); + if USB_COMMANDS.try_send(DeviceCommand::Disable).is_err() { + warn!("Failed to send disable command to USB channel"); + }; + } +} + #[embassy::main] async fn main(_spawner: Spawner, p: Peripherals) { let clock: pac::CLOCK = unsafe { mem::transmute(()) }; @@ -30,10 +57,6 @@ async fn main(_spawner: Spawner, p: Peripherals) { clock.tasks_hfclkstart.write(|w| unsafe { w.bits(1) }); while clock.events_hfclkstarted.read().bits() != 1 {} - info!("Waiting for vbus..."); - while !power.usbregstatus.read().vbusdetect().is_vbus_present() {} - info!("vbus OK"); - // Create the driver, from the HAL. let irq = interrupt::take!(USBD); let driver = Driver::new(p.USBD, irq); @@ -45,6 +68,8 @@ async fn main(_spawner: Spawner, p: Peripherals) { config.serial_number = Some("12345678"); config.max_power = 100; config.max_packet_size_0 = 64; + config.supports_remote_wakeup = true; + config.start_enabled = false; // Create embassy-usb DeviceBuilder using the driver and config. // It needs some buffers for building the descriptors. @@ -53,16 +78,19 @@ async fn main(_spawner: Spawner, p: Peripherals) { let mut bos_descriptor = [0; 256]; let mut control_buf = [0; 16]; let request_handler = MyRequestHandler {}; + let device_state_handler = MyDeviceStateHandler::new(); let mut state = State::<8, 1>::new(); - let mut builder = UsbDeviceBuilder::new( + let mut builder = UsbDeviceBuilder::new_with_channel( driver, config, &mut device_descriptor, &mut config_descriptor, &mut bos_descriptor, &mut control_buf, + Some(&device_state_handler), + &USB_COMMANDS, ); // Create classes on the builder. @@ -76,7 +104,7 @@ async fn main(_spawner: Spawner, p: Peripherals) { ); // Build the builder. - let mut usb = builder.build().await; + let mut usb = builder.build(); // Run the USB device. let usb_fut = usb.run(); @@ -90,6 +118,12 @@ async fn main(_spawner: Spawner, p: Peripherals) { loop { button.wait_for_low().await; info!("PRESSED"); + + if SUSPENDED.load(Ordering::Acquire) { + info!("Triggering remote wakeup"); + USB_COMMANDS.send(DeviceCommand::RemoteWakeup); + } + let report = KeyboardReport { keycodes: [4, 0, 0, 0, 0, 0], leds: 0, @@ -119,6 +153,16 @@ async fn main(_spawner: Spawner, p: Peripherals) { let out_fut = async { hid_out.run(false, &request_handler).await; }; + + let power_irq = interrupt::take!(POWER_CLOCK); + power_irq.set_handler(on_power_interrupt); + power_irq.unpend(); + power_irq.enable(); + + power + .intenset + .write(|w| w.usbdetected().set().usbremoved().set()); + // Run everything concurrently. // If we had made everything `'static` above instead, we could do this using separate tasks instead. join(usb_fut, join(in_fut, out_fut)).await; @@ -146,3 +190,59 @@ impl RequestHandler for MyRequestHandler { None } } + +struct MyDeviceStateHandler { + configured: AtomicBool, +} + +impl MyDeviceStateHandler { + fn new() -> Self { + MyDeviceStateHandler { + configured: AtomicBool::new(false), + } + } +} + +impl DeviceStateHandler for MyDeviceStateHandler { + fn reset(&self) { + self.configured.store(false, Ordering::Relaxed); + info!("Bus reset, the Vbus current limit is 100mA"); + } + + fn addressed(&self, addr: u8) { + self.configured.store(false, Ordering::Relaxed); + info!("USB address set to: {}", addr); + } + + fn configured(&self, configured: bool) { + self.configured.store(configured, Ordering::Relaxed); + if configured { + info!( + "Device configured, it may now draw up to the configured current limit from Vbus." + ) + } else { + info!("Device is no longer configured, the Vbus current limit is 100mA."); + } + } + + fn suspended(&self, suspended: bool) { + if suspended { + info!("Device suspended, the Vbus current limit is 500µA (or 2.5mA for high-power devices with remote wakeup enabled)."); + SUSPENDED.store(true, Ordering::Release); + } else { + SUSPENDED.store(false, Ordering::Release); + if self.configured.load(Ordering::Relaxed) { + info!( + "Device resumed, it may now draw up to the configured current limit from Vbus" + ); + } else { + info!("Device resumed, the Vbus current limit is 100mA"); + } + } + } + + fn disabled(&self) { + self.configured.store(false, Ordering::Relaxed); + info!("Device disabled"); + } +} diff --git a/examples/nrf/src/bin/usb_hid_mouse.rs b/examples/nrf/src/bin/usb_hid_mouse.rs index ca9383827..fe27e76fb 100644 --- a/examples/nrf/src/bin/usb_hid_mouse.rs +++ b/examples/nrf/src/bin/usb_hid_mouse.rs @@ -61,6 +61,7 @@ async fn main(_spawner: Spawner, p: Peripherals) { &mut config_descriptor, &mut bos_descriptor, &mut control_buf, + None, ); // Create classes on the builder. @@ -74,7 +75,7 @@ async fn main(_spawner: Spawner, p: Peripherals) { ); // Build the builder. - let mut usb = builder.build().await; + let mut usb = builder.build(); // Run the USB device. let usb_fut = usb.run(); diff --git a/examples/nrf/src/bin/usb_serial.rs b/examples/nrf/src/bin/usb_serial.rs index 684322837..987cc4139 100644 --- a/examples/nrf/src/bin/usb_serial.rs +++ b/examples/nrf/src/bin/usb_serial.rs @@ -54,13 +54,14 @@ async fn main(_spawner: Spawner, p: Peripherals) { &mut config_descriptor, &mut bos_descriptor, &mut control_buf, + None, ); // Create classes on the builder. let mut class = CdcAcmClass::new(&mut builder, &mut state, 64); // Build the builder. - let mut usb = builder.build().await; + let mut usb = builder.build(); // Run the USB device. let usb_fut = usb.run(); diff --git a/examples/nrf/src/bin/usb_serial_multitask.rs b/examples/nrf/src/bin/usb_serial_multitask.rs index bfb09014c..5fcb0e052 100644 --- a/examples/nrf/src/bin/usb_serial_multitask.rs +++ b/examples/nrf/src/bin/usb_serial_multitask.rs @@ -79,13 +79,14 @@ async fn main(spawner: Spawner, p: Peripherals) { &mut res.config_descriptor, &mut res.bos_descriptor, &mut res.control_buf, + None, ); // Create classes on the builder. let class = CdcAcmClass::new(&mut builder, &mut res.serial_state, 64); // Build the builder. - let usb = builder.build().await; + let usb = builder.build(); unwrap!(spawner.spawn(usb_task(usb))); unwrap!(spawner.spawn(echo_task(class))); From 3a30b08936e49c48b7e90e183a402d91ba829021 Mon Sep 17 00:00:00 2001 From: alexmoon Date: Sun, 10 Apr 2022 16:25:40 -0400 Subject: [PATCH 2/7] Remove dead code on nrf5340 --- embassy-nrf/src/usb.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/embassy-nrf/src/usb.rs b/embassy-nrf/src/usb.rs index 3d90f4b9a..7f99b9a1c 100644 --- a/embassy-nrf/src/usb.rs +++ b/embassy-nrf/src/usb.rs @@ -864,11 +864,13 @@ macro_rules! impl_usb { mod errata { /// Writes `val` to `addr`. Used to apply Errata workarounds. + #[cfg(any(feature = "nrf52840", feature = "nrf52833", feature = "nrf52820"))] unsafe fn poke(addr: u32, val: u32) { (addr as *mut u32).write_volatile(val); } /// Reads 32 bits from `addr`. + #[cfg(feature = "nrf52840")] unsafe fn peek(addr: u32) -> u32 { (addr as *mut u32).read_volatile() } From 7fde3abd5d7e04f6d79d325d4454d6eafabebc1f Mon Sep 17 00:00:00 2001 From: alexmoon Date: Mon, 11 Apr 2022 12:00:05 -0400 Subject: [PATCH 3/7] Remote wakeup bug fixes --- embassy-usb/src/lib.rs | 30 ++++++++++++++---------- examples/nrf/src/bin/usb_hid_keyboard.rs | 24 +++++++++---------- 2 files changed, 30 insertions(+), 24 deletions(-) diff --git a/embassy-usb/src/lib.rs b/embassy-usb/src/lib.rs index 2c53d9214..210908c2c 100644 --- a/embassy-usb/src/lib.rs +++ b/embassy-usb/src/lib.rs @@ -171,21 +171,21 @@ impl<'d, D: Driver<'d>, M: RawMutex> UsbDevice<'d, D, M> { h.reset(); } - if let Some(h) = &mut self.handler { + if let Some(h) = &self.handler { h.reset(); } } Event::Resume => { trace!("usb: resume"); self.suspended = false; - if let Some(h) = &mut self.handler { + if let Some(h) = &self.handler { h.suspended(false); } } Event::Suspend => { trace!("usb: suspend"); self.suspended = true; - if let Some(h) = &mut self.handler { + if let Some(h) = &self.handler { h.suspended(true); } } @@ -200,19 +200,25 @@ impl<'d, D: Driver<'d>, M: RawMutex> UsbDevice<'d, D, M> { trace!("usb: disable"); self.bus.disable(); self.device_state = UsbDeviceState::Disabled; - if let Some(h) = &mut self.handler { + if let Some(h) = &self.handler { h.disabled(); } self.wait_for_enable().await; } DeviceCommand::RemoteWakeup => { - if self.remote_wakeup_enabled { + trace!("usb: remote wakeup"); + if self.suspended && self.remote_wakeup_enabled { match self.bus.remote_wakeup().await { - Ok(()) => (), + Ok(()) => { + self.suspended = false; + if let Some(h) = &self.handler { + h.suspended(false); + } + } Err(Unsupported) => warn!("Remote wakeup is unsupported!"), } } else { - warn!("Remote wakeup not enabled."); + warn!("Remote wakeup requested when not enabled or not suspended."); } } }, @@ -250,14 +256,14 @@ impl<'d, D: Driver<'d>, M: RawMutex> UsbDevice<'d, D, M> { (RequestType::Standard, Recipient::Device) => match (req.request, req.value) { (Request::CLEAR_FEATURE, Request::FEATURE_DEVICE_REMOTE_WAKEUP) => { self.remote_wakeup_enabled = false; - if let Some(h) = &mut self.handler { + if let Some(h) = &self.handler { h.remote_wakeup_enabled(false); } self.control.accept(stage) } (Request::SET_FEATURE, Request::FEATURE_DEVICE_REMOTE_WAKEUP) => { self.remote_wakeup_enabled = true; - if let Some(h) = &mut self.handler { + if let Some(h) = &self.handler { h.remote_wakeup_enabled(true); } self.control.accept(stage) @@ -266,7 +272,7 @@ impl<'d, D: Driver<'d>, M: RawMutex> UsbDevice<'d, D, M> { self.pending_address = addr as u8; self.bus.set_device_address(self.pending_address); self.device_state = UsbDeviceState::Addressed; - if let Some(h) = &mut self.handler { + if let Some(h) = &self.handler { h.addressed(self.pending_address); } self.control.accept(stage) @@ -274,7 +280,7 @@ impl<'d, D: Driver<'d>, M: RawMutex> UsbDevice<'d, D, M> { (Request::SET_CONFIGURATION, CONFIGURATION_VALUE_U16) => { self.device_state = UsbDeviceState::Configured; self.bus.set_configured(true); - if let Some(h) = &mut self.handler { + if let Some(h) = &self.handler { h.configured(true); } self.control.accept(stage) @@ -284,7 +290,7 @@ impl<'d, D: Driver<'d>, M: RawMutex> UsbDevice<'d, D, M> { _ => { self.device_state = UsbDeviceState::Addressed; self.bus.set_configured(false); - if let Some(h) = &mut self.handler { + if let Some(h) = &self.handler { h.configured(false); } self.control.accept(stage) diff --git a/examples/nrf/src/bin/usb_hid_keyboard.rs b/examples/nrf/src/bin/usb_hid_keyboard.rs index 483d86b81..fb3a198a7 100644 --- a/examples/nrf/src/bin/usb_hid_keyboard.rs +++ b/examples/nrf/src/bin/usb_hid_keyboard.rs @@ -121,20 +121,20 @@ async fn main(_spawner: Spawner, p: Peripherals) { if SUSPENDED.load(Ordering::Acquire) { info!("Triggering remote wakeup"); - USB_COMMANDS.send(DeviceCommand::RemoteWakeup); + USB_COMMANDS.send(DeviceCommand::RemoteWakeup).await; + } else { + let report = KeyboardReport { + keycodes: [4, 0, 0, 0, 0, 0], + leds: 0, + modifier: 0, + reserved: 0, + }; + match hid_in.serialize(&report).await { + Ok(()) => {} + Err(e) => warn!("Failed to send report: {:?}", e), + }; } - let report = KeyboardReport { - keycodes: [4, 0, 0, 0, 0, 0], - leds: 0, - modifier: 0, - reserved: 0, - }; - match hid_in.serialize(&report).await { - Ok(()) => {} - Err(e) => warn!("Failed to send report: {:?}", e), - }; - button.wait_for_high().await; info!("RELEASED"); let report = KeyboardReport { From 2915e858baac442e71bac4b565746401deed22bd Mon Sep 17 00:00:00 2001 From: alexmoon Date: Tue, 12 Apr 2022 17:51:50 -0400 Subject: [PATCH 4/7] Make Driver::disable async and fix comment --- embassy-nrf/src/usb.rs | 9 ++++++--- embassy-usb/src/driver.rs | 7 ++++--- embassy-usb/src/lib.rs | 2 +- 3 files changed, 11 insertions(+), 7 deletions(-) diff --git a/embassy-nrf/src/usb.rs b/embassy-nrf/src/usb.rs index 7f99b9a1c..b67201e67 100644 --- a/embassy-nrf/src/usb.rs +++ b/embassy-nrf/src/usb.rs @@ -208,6 +208,7 @@ pub struct Bus<'d, T: Instance> { impl<'d, T: Instance> driver::Bus for Bus<'d, T> { type EnableFuture<'a> = impl Future + 'a where Self: 'a; + type DisableFuture<'a> = impl Future + 'a where Self: 'a; type PollFuture<'a> = impl Future + 'a where Self: 'a; type RemoteWakeupFuture<'a> = impl Future> + 'a where Self: 'a; @@ -248,9 +249,11 @@ impl<'d, T: Instance> driver::Bus for Bus<'d, T> { } } - fn disable(&mut self) { - let regs = T::regs(); - regs.enable.write(|x| x.enable().disabled()); + fn disable(&mut self) -> Self::DisableFuture<'_> { + async move { + let regs = T::regs(); + regs.enable.write(|x| x.enable().disabled()); + } } fn poll<'a>(&'a mut self) -> Self::PollFuture<'a> { diff --git a/embassy-usb/src/driver.rs b/embassy-usb/src/driver.rs index 99610deef..cedd349fb 100644 --- a/embassy-usb/src/driver.rs +++ b/embassy-usb/src/driver.rs @@ -57,6 +57,9 @@ pub trait Driver<'a> { pub trait Bus { type EnableFuture<'a>: Future + 'a + where + Self: 'a; + type DisableFuture<'a>: Future + 'a where Self: 'a; type PollFuture<'a>: Future + 'a @@ -71,7 +74,7 @@ pub trait Bus { fn enable(&mut self) -> Self::EnableFuture<'_>; /// Disables and powers down the USB peripheral. - fn disable(&mut self); + fn disable(&mut self) -> Self::DisableFuture<'_>; fn poll<'a>(&'a mut self) -> Self::PollFuture<'a>; @@ -103,8 +106,6 @@ pub trait Bus { /// Initiates a remote wakeup of the host by the device. /// - /// The default implementation just returns `Unsupported`. - /// /// # Errors /// /// * [`Unsupported`](crate::UsbError::Unsupported) - This UsbBus implementation doesn't support diff --git a/embassy-usb/src/lib.rs b/embassy-usb/src/lib.rs index 210908c2c..11561430b 100644 --- a/embassy-usb/src/lib.rs +++ b/embassy-usb/src/lib.rs @@ -198,7 +198,7 @@ impl<'d, D: Driver<'d>, M: RawMutex> UsbDevice<'d, D, M> { DeviceCommand::Enable => warn!("usb: Enable command received while enabled."), DeviceCommand::Disable => { trace!("usb: disable"); - self.bus.disable(); + self.bus.disable().await; self.device_state = UsbDeviceState::Disabled; if let Some(h) = &self.handler { h.disabled(); From 1d875fab2dc9d765ebf9f4b37112581301f2ed7e Mon Sep 17 00:00:00 2001 From: alexmoon Date: Tue, 12 Apr 2022 18:55:57 -0400 Subject: [PATCH 5/7] Use embassy::util::select3 --- embassy-usb/src/lib.rs | 1 + embassy-usb/src/util.rs | 52 ----------------------------------------- 2 files changed, 1 insertion(+), 52 deletions(-) diff --git a/embassy-usb/src/lib.rs b/embassy-usb/src/lib.rs index 11561430b..102ccbb3a 100644 --- a/embassy-usb/src/lib.rs +++ b/embassy-usb/src/lib.rs @@ -15,6 +15,7 @@ mod util; use driver::Unsupported; use embassy::blocking_mutex::raw::{NoopRawMutex, RawMutex}; use embassy::channel::Channel; +use embassy::util::{select3, Either3}; use heapless::Vec; use self::control::*; diff --git a/embassy-usb/src/util.rs b/embassy-usb/src/util.rs index 4fc55461f..3f20262c6 100644 --- a/embassy-usb/src/util.rs +++ b/embassy-usb/src/util.rs @@ -6,58 +6,6 @@ use core::task::{Context, Poll}; use embassy::blocking_mutex::raw::RawMutex; use embassy::channel::Channel; -#[derive(Debug, Clone)] -pub enum Either3 { - First(A), - Second(B), - Third(C), -} - -/// Same as [`select`], but with more futures. -pub fn select3(a: A, b: B, c: C) -> Select3 -where - A: Future, - B: Future, - C: Future, -{ - Select3 { a, b, c } -} - -/// Future for the [`select3`] function. -#[derive(Debug)] -#[must_use = "futures do nothing unless you `.await` or poll them"] -pub struct Select3 { - a: A, - b: B, - c: C, -} - -impl Future for Select3 -where - A: Future, - B: Future, - C: Future, -{ - type Output = Either3; - - fn poll(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll { - let this = unsafe { self.get_unchecked_mut() }; - let a = unsafe { Pin::new_unchecked(&mut this.a) }; - let b = unsafe { Pin::new_unchecked(&mut this.b) }; - let c = unsafe { Pin::new_unchecked(&mut this.c) }; - if let Poll::Ready(x) = a.poll(cx) { - return Poll::Ready(Either3::First(x)); - } - if let Poll::Ready(x) = b.poll(cx) { - return Poll::Ready(Either3::Second(x)); - } - if let Poll::Ready(x) = c.poll(cx) { - return Poll::Ready(Either3::Third(x)); - } - Poll::Pending - } -} - pub struct Pending { _phantom: PhantomData, } From ff7c6b350e2338a0b1e4327f4b2eb0435468f313 Mon Sep 17 00:00:00 2001 From: alexmoon Date: Wed, 13 Apr 2022 13:09:08 -0400 Subject: [PATCH 6/7] Remove channel and make run future cancelable --- embassy-usb-hid/src/lib.rs | 13 +-- embassy-usb-serial/src/lib.rs | 5 +- embassy-usb/src/builder.rs | 71 +----------- embassy-usb/src/lib.rs | 140 +++++++++++------------ embassy-usb/src/util.rs | 33 ------ examples/nrf/src/bin/usb_hid_keyboard.rs | 63 +++++++--- 6 files changed, 129 insertions(+), 196 deletions(-) delete mode 100644 embassy-usb/src/util.rs diff --git a/embassy-usb-hid/src/lib.rs b/embassy-usb-hid/src/lib.rs index 4a1df0eab..e870becf5 100644 --- a/embassy-usb-hid/src/lib.rs +++ b/embassy-usb-hid/src/lib.rs @@ -11,7 +11,6 @@ use core::mem::MaybeUninit; use core::ops::Range; use core::sync::atomic::{AtomicUsize, Ordering}; -use embassy::blocking_mutex::raw::RawMutex; use embassy::time::Duration; use embassy_usb::driver::EndpointOut; use embassy_usb::{ @@ -89,8 +88,8 @@ impl<'d, D: Driver<'d>, const IN_N: usize> HidClass<'d, D, (), IN_N> { /// high performance uses, and a value of 255 is good for best-effort usecases. /// /// This allocates an IN endpoint only. - pub fn new( - builder: &mut UsbDeviceBuilder<'d, D, M>, + pub fn new( + builder: &mut UsbDeviceBuilder<'d, D>, state: &'d mut State<'d, IN_N, OUT_N>, report_descriptor: &'static [u8], request_handler: Option<&'d dyn RequestHandler>, @@ -133,8 +132,8 @@ impl<'d, D: Driver<'d>, const IN_N: usize, const OUT_N: usize> /// high performance uses, and a value of 255 is good for best-effort usecases. /// /// This allocates two endpoints (IN and OUT). - pub fn with_output_ep( - builder: &mut UsbDeviceBuilder<'d, D, M>, + pub fn with_output_ep( + builder: &mut UsbDeviceBuilder<'d, D>, state: &'d mut State<'d, IN_N, OUT_N>, report_descriptor: &'static [u8], request_handler: Option<&'d dyn RequestHandler>, @@ -393,9 +392,9 @@ impl<'a> Control<'a> { } } - fn build<'d, D: Driver<'d>, M: RawMutex>( + fn build<'d, D: Driver<'d>>( &'d mut self, - builder: &mut UsbDeviceBuilder<'d, D, M>, + builder: &mut UsbDeviceBuilder<'d, D>, ep_out: Option<&D::EndpointOut>, ep_in: &D::EndpointIn, ) { diff --git a/embassy-usb-serial/src/lib.rs b/embassy-usb-serial/src/lib.rs index 7f006c0fd..7b25398d0 100644 --- a/embassy-usb-serial/src/lib.rs +++ b/embassy-usb-serial/src/lib.rs @@ -8,7 +8,6 @@ pub(crate) mod fmt; use core::cell::Cell; use core::mem::{self, MaybeUninit}; use core::sync::atomic::{AtomicBool, Ordering}; -use embassy::blocking_mutex::raw::RawMutex; use embassy::blocking_mutex::CriticalSectionMutex; use embassy_usb::control::{self, ControlHandler, InResponse, OutResponse, Request}; use embassy_usb::driver::{Endpoint, EndpointError, EndpointIn, EndpointOut}; @@ -163,8 +162,8 @@ impl<'d> ControlHandler for Control<'d> { impl<'d, D: Driver<'d>> CdcAcmClass<'d, D> { /// Creates a new CdcAcmClass with the provided UsbBus and max_packet_size in bytes. For /// full-speed devices, max_packet_size has to be one of 8, 16, 32 or 64. - pub fn new( - builder: &mut UsbDeviceBuilder<'d, D, M>, + pub fn new( + builder: &mut UsbDeviceBuilder<'d, D>, state: &'d mut State<'d>, max_packet_size: u16, ) -> Self { diff --git a/embassy-usb/src/builder.rs b/embassy-usb/src/builder.rs index 30d31ac74..486672055 100644 --- a/embassy-usb/src/builder.rs +++ b/embassy-usb/src/builder.rs @@ -1,9 +1,5 @@ -use embassy::blocking_mutex::raw::{NoopRawMutex, RawMutex}; -use embassy::channel::Channel; use heapless::Vec; -use crate::DeviceCommand; - use super::control::ControlHandler; use super::descriptor::{BosWriter, DescriptorWriter}; use super::driver::{Driver, EndpointAllocError}; @@ -98,11 +94,6 @@ pub struct Config<'a> { /// Default: 100mA /// Max: 500mA pub max_power: u16, - - /// Whether the USB bus should be enabled when built. - /// - /// Default: true - pub start_enabled: bool, } impl<'a> Config<'a> { @@ -122,18 +113,16 @@ impl<'a> Config<'a> { supports_remote_wakeup: false, composite_with_iads: false, max_power: 100, - start_enabled: true, } } } /// Used to build new [`UsbDevice`]s. -pub struct UsbDeviceBuilder<'d, D: Driver<'d>, M: RawMutex> { +pub struct UsbDeviceBuilder<'d, D: Driver<'d>> { config: Config<'d>, handler: Option<&'d dyn DeviceStateHandler>, interfaces: Vec<(u8, &'d mut dyn ControlHandler), MAX_INTERFACE_COUNT>, control_buf: &'d mut [u8], - commands: Option<&'d Channel>, driver: D, next_interface_number: u8, @@ -145,7 +134,7 @@ pub struct UsbDeviceBuilder<'d, D: Driver<'d>, M: RawMutex> { pub bos_descriptor: BosWriter<'d>, } -impl<'d, D: Driver<'d>> UsbDeviceBuilder<'d, D, NoopRawMutex> { +impl<'d, D: Driver<'d>> UsbDeviceBuilder<'d, D> { /// Creates a builder for constructing a new [`UsbDevice`]. /// /// `control_buf` is a buffer used for USB control request data. It should be sized @@ -159,57 +148,6 @@ impl<'d, D: Driver<'d>> UsbDeviceBuilder<'d, D, NoopRawMutex> { bos_descriptor_buf: &'d mut [u8], control_buf: &'d mut [u8], handler: Option<&'d dyn DeviceStateHandler>, - ) -> Self { - Self::new_inner( - driver, - config, - device_descriptor_buf, - config_descriptor_buf, - bos_descriptor_buf, - control_buf, - handler, - None, - ) - } -} - -impl<'d, D: Driver<'d>, M: RawMutex> UsbDeviceBuilder<'d, D, M> { - /// Creates a builder for constructing a new [`UsbDevice`]. - /// - /// `control_buf` is a buffer used for USB control request data. It should be sized - /// large enough for the length of the largest control request (in or out) - /// anticipated by any class added to the device. - pub fn new_with_channel( - driver: D, - config: Config<'d>, - device_descriptor_buf: &'d mut [u8], - config_descriptor_buf: &'d mut [u8], - bos_descriptor_buf: &'d mut [u8], - control_buf: &'d mut [u8], - handler: Option<&'d dyn DeviceStateHandler>, - channel: &'d Channel, - ) -> Self { - Self::new_inner( - driver, - config, - device_descriptor_buf, - config_descriptor_buf, - bos_descriptor_buf, - control_buf, - handler, - Some(channel), - ) - } - - fn new_inner( - driver: D, - config: Config<'d>, - device_descriptor_buf: &'d mut [u8], - config_descriptor_buf: &'d mut [u8], - bos_descriptor_buf: &'d mut [u8], - control_buf: &'d mut [u8], - handler: Option<&'d dyn DeviceStateHandler>, - channel: Option<&'d Channel>, ) -> Self { // Magic values specified in USB-IF ECN on IADs. if config.composite_with_iads @@ -243,8 +181,6 @@ impl<'d, D: Driver<'d>, M: RawMutex> UsbDeviceBuilder<'d, D, M> { config, interfaces: Vec::new(), control_buf, - commands: channel, - next_interface_number: 0, next_string_index: 4, @@ -255,7 +191,7 @@ impl<'d, D: Driver<'d>, M: RawMutex> UsbDeviceBuilder<'d, D, M> { } /// Creates the [`UsbDevice`] instance with the configuration in this builder. - pub fn build(mut self) -> UsbDevice<'d, D, M> { + pub fn build(mut self) -> UsbDevice<'d, D> { self.config_descriptor.end_configuration(); self.bos_descriptor.end_bos(); @@ -263,7 +199,6 @@ impl<'d, D: Driver<'d>, M: RawMutex> UsbDeviceBuilder<'d, D, M> { self.driver, self.config, self.handler, - self.commands, self.device_descriptor.into_buf(), self.config_descriptor.into_buf(), self.bos_descriptor.writer.into_buf(), diff --git a/embassy-usb/src/lib.rs b/embassy-usb/src/lib.rs index 102ccbb3a..ccea8bc7a 100644 --- a/embassy-usb/src/lib.rs +++ b/embassy-usb/src/lib.rs @@ -10,19 +10,14 @@ pub mod control; pub mod descriptor; pub mod driver; pub mod types; -mod util; -use driver::Unsupported; -use embassy::blocking_mutex::raw::{NoopRawMutex, RawMutex}; -use embassy::channel::Channel; -use embassy::util::{select3, Either3}; +use embassy::util::{select, Either}; use heapless::Vec; use self::control::*; use self::descriptor::*; use self::driver::{Bus, Driver, Event}; use self::types::*; -use self::util::*; pub use self::builder::Config; pub use self::builder::UsbDeviceBuilder; @@ -47,6 +42,19 @@ pub enum UsbDeviceState { Configured, } +#[derive(PartialEq, Eq, Copy, Clone, Debug)] +#[cfg_attr(feature = "defmt", derive(defmt::Format))] +pub enum RemoteWakeupError { + InvalidState, + Unsupported, +} + +impl From for RemoteWakeupError { + fn from(_: driver::Unsupported) -> Self { + RemoteWakeupError::Unsupported + } +} + /// The bConfiguration value for the not configured state. pub const CONFIGURATION_NONE: u8 = 0; @@ -58,16 +66,11 @@ pub const DEFAULT_ALTERNATE_SETTING: u8 = 0; pub const MAX_INTERFACE_COUNT: usize = 4; -#[derive(PartialEq, Eq, Copy, Clone, Debug)] -#[cfg_attr(feature = "defmt", derive(defmt::Format))] -pub enum DeviceCommand { - Enable, - Disable, - RemoteWakeup, -} - /// A handler trait for changes in the device state of the [UsbDevice]. pub trait DeviceStateHandler { + /// Called when the USB device has been enabled or disabled. + fn enabled(&self, _enabled: bool) {} + /// Called when the host resets the device. fn reset(&self) {} @@ -82,15 +85,11 @@ pub trait DeviceStateHandler { /// Called when remote wakeup feature is enabled or disabled. fn remote_wakeup_enabled(&self, _enabled: bool) {} - - /// Called when the USB device has been disabled. - fn disabled(&self) {} } -pub struct UsbDevice<'d, D: Driver<'d>, M: RawMutex = NoopRawMutex> { +pub struct UsbDevice<'d, D: Driver<'d>> { bus: D::Bus, handler: Option<&'d dyn DeviceStateHandler>, - commands: Option<&'d Channel>, control: ControlPipe, config: Config<'d>, @@ -101,6 +100,7 @@ pub struct UsbDevice<'d, D: Driver<'d>, M: RawMutex = NoopRawMutex> { device_state: UsbDeviceState, suspended: bool, + in_control_handler: bool, remote_wakeup_enabled: bool, self_powered: bool, pending_address: u8, @@ -108,18 +108,17 @@ pub struct UsbDevice<'d, D: Driver<'d>, M: RawMutex = NoopRawMutex> { interfaces: Vec<(u8, &'d mut dyn ControlHandler), MAX_INTERFACE_COUNT>, } -impl<'d, D: Driver<'d>, M: RawMutex> UsbDevice<'d, D, M> { +impl<'d, D: Driver<'d>> UsbDevice<'d, D> { pub(crate) fn build( mut driver: D, config: Config<'d>, handler: Option<&'d dyn DeviceStateHandler>, - commands: Option<&'d Channel>, device_descriptor: &'d [u8], config_descriptor: &'d [u8], bos_descriptor: &'d [u8], interfaces: Vec<(u8, &'d mut dyn ControlHandler), MAX_INTERFACE_COUNT>, control_buf: &'d mut [u8], - ) -> UsbDevice<'d, D, M> { + ) -> UsbDevice<'d, D> { let control = driver .alloc_control_pipe(config.max_packet_size_0 as u16) .expect("failed to alloc control endpoint"); @@ -132,14 +131,14 @@ impl<'d, D: Driver<'d>, M: RawMutex> UsbDevice<'d, D, M> { bus, config, handler, - commands, control: ControlPipe::new(control), device_descriptor, config_descriptor, bos_descriptor, control_buf, - device_state: UsbDeviceState::Default, + device_state: UsbDeviceState::Disabled, suspended: false, + in_control_handler: false, remote_wakeup_enabled: false, self_powered: false, pending_address: 0, @@ -148,19 +147,24 @@ impl<'d, D: Driver<'d>, M: RawMutex> UsbDevice<'d, D, M> { } pub async fn run(&mut self) -> ! { - if self.config.start_enabled { + if self.device_state == UsbDeviceState::Disabled { self.bus.enable().await; - } else { - self.wait_for_enable().await + self.device_state = UsbDeviceState::Default; + + if let Some(h) = &self.handler { + h.enabled(true); + } + } else if self.in_control_handler { + warn!("usb: control request interrupted"); + self.control.reject(); + self.in_control_handler = false; } loop { let control_fut = self.control.setup(); let bus_fut = self.bus.poll(); - let commands_fut = recv_or_wait(self.commands); - - match select3(bus_fut, control_fut, commands_fut).await { - Either3::First(evt) => match evt { + match select(bus_fut, control_fut).await { + Either::First(evt) => match evt { Event::Reset => { trace!("usb: reset"); self.device_state = UsbDeviceState::Default; @@ -191,54 +195,48 @@ impl<'d, D: Driver<'d>, M: RawMutex> UsbDevice<'d, D, M> { } } }, - Either3::Second(req) => match req { - Setup::DataIn(req, stage) => self.handle_control_in(req, stage).await, - Setup::DataOut(req, stage) => self.handle_control_out(req, stage).await, - }, - Either3::Third(cmd) => match cmd { - DeviceCommand::Enable => warn!("usb: Enable command received while enabled."), - DeviceCommand::Disable => { - trace!("usb: disable"); - self.bus.disable().await; - self.device_state = UsbDeviceState::Disabled; - if let Some(h) = &self.handler { - h.disabled(); - } - self.wait_for_enable().await; + Either::Second(req) => { + self.in_control_handler = true; + match req { + Setup::DataIn(req, stage) => self.handle_control_in(req, stage).await, + Setup::DataOut(req, stage) => self.handle_control_out(req, stage).await, } - DeviceCommand::RemoteWakeup => { - trace!("usb: remote wakeup"); - if self.suspended && self.remote_wakeup_enabled { - match self.bus.remote_wakeup().await { - Ok(()) => { - self.suspended = false; - if let Some(h) = &self.handler { - h.suspended(false); - } - } - Err(Unsupported) => warn!("Remote wakeup is unsupported!"), - } - } else { - warn!("Remote wakeup requested when not enabled or not suspended."); - } - } - }, + self.in_control_handler = false; + } } } } - async fn wait_for_enable(&mut self) { - loop { - // When disabled just wait until we're told to re-enable - match recv_or_wait(self.commands).await { - DeviceCommand::Enable => break, - cmd => warn!("usb: {:?} received while disabled", cmd), + pub async fn disable(&mut self) { + if self.device_state != UsbDeviceState::Disabled { + self.bus.disable().await; + self.device_state = UsbDeviceState::Disabled; + self.suspended = false; + self.remote_wakeup_enabled = false; + self.in_control_handler = false; + + if let Some(h) = &self.handler { + h.enabled(false); } } + } - trace!("usb: enable"); - self.bus.enable().await; - self.device_state = UsbDeviceState::Default; + pub async fn remote_wakeup(&mut self) -> Result<(), RemoteWakeupError> { + if self.device_state == UsbDeviceState::Configured + && self.suspended + && self.remote_wakeup_enabled + { + self.bus.remote_wakeup().await?; + self.suspended = false; + + if let Some(h) = &self.handler { + h.suspended(false); + } + + Ok(()) + } else { + Err(RemoteWakeupError::InvalidState) + } } async fn handle_control_out(&mut self, req: Request, stage: DataOutStage) { diff --git a/embassy-usb/src/util.rs b/embassy-usb/src/util.rs deleted file mode 100644 index 3f20262c6..000000000 --- a/embassy-usb/src/util.rs +++ /dev/null @@ -1,33 +0,0 @@ -use core::future::Future; -use core::marker::PhantomData; -use core::pin::Pin; -use core::task::{Context, Poll}; - -use embassy::blocking_mutex::raw::RawMutex; -use embassy::channel::Channel; - -pub struct Pending { - _phantom: PhantomData, -} - -impl Pending { - fn new() -> Self { - Pending { - _phantom: PhantomData, - } - } -} - -impl Future for Pending { - type Output = T; - fn poll(self: Pin<&mut Self>, _cx: &mut Context<'_>) -> Poll { - Poll::Pending - } -} - -pub async fn recv_or_wait(ch: Option<&Channel>) -> T { - match ch { - Some(ch) => ch.recv().await, - None => Pending::new().await, - } -} diff --git a/examples/nrf/src/bin/usb_hid_keyboard.rs b/examples/nrf/src/bin/usb_hid_keyboard.rs index fb3a198a7..32659dfbb 100644 --- a/examples/nrf/src/bin/usb_hid_keyboard.rs +++ b/examples/nrf/src/bin/usb_hid_keyboard.rs @@ -11,13 +11,14 @@ use embassy::channel::Channel; use embassy::executor::Spawner; use embassy::interrupt::InterruptExt; use embassy::time::Duration; +use embassy::util::select; use embassy_nrf::gpio::{Input, Pin, Pull}; use embassy_nrf::interrupt; use embassy_nrf::pac; use embassy_nrf::usb::Driver; use embassy_nrf::Peripherals; use embassy_usb::control::OutResponse; -use embassy_usb::{Config, DeviceCommand, DeviceStateHandler, UsbDeviceBuilder}; +use embassy_usb::{Config, DeviceStateHandler, UsbDeviceBuilder}; use embassy_usb_hid::{HidClass, ReportId, RequestHandler, State}; use futures::future::join; use usbd_hid::descriptor::{KeyboardReport, SerializedDescriptor}; @@ -25,7 +26,14 @@ use usbd_hid::descriptor::{KeyboardReport, SerializedDescriptor}; use defmt_rtt as _; // global logger use panic_probe as _; -static USB_COMMANDS: Channel = Channel::new(); +#[derive(defmt::Format)] +enum Command { + Enable, + Disable, + RemoteWakeup, +} + +static USB_COMMANDS: Channel = Channel::new(); static SUSPENDED: AtomicBool = AtomicBool::new(false); fn on_power_interrupt(_: *mut ()) { @@ -34,7 +42,7 @@ fn on_power_interrupt(_: *mut ()) { if regs.events_usbdetected.read().bits() != 0 { regs.events_usbdetected.reset(); info!("Vbus detected, enabling USB..."); - if USB_COMMANDS.try_send(DeviceCommand::Enable).is_err() { + if USB_COMMANDS.try_send(Command::Enable).is_err() { warn!("Failed to send enable command to USB channel"); } } @@ -42,7 +50,7 @@ fn on_power_interrupt(_: *mut ()) { if regs.events_usbremoved.read().bits() != 0 { regs.events_usbremoved.reset(); info!("Vbus removed, disabling USB..."); - if USB_COMMANDS.try_send(DeviceCommand::Disable).is_err() { + if USB_COMMANDS.try_send(Command::Disable).is_err() { warn!("Failed to send disable command to USB channel"); }; } @@ -69,7 +77,6 @@ async fn main(_spawner: Spawner, p: Peripherals) { config.max_power = 100; config.max_packet_size_0 = 64; config.supports_remote_wakeup = true; - config.start_enabled = false; // Create embassy-usb DeviceBuilder using the driver and config. // It needs some buffers for building the descriptors. @@ -82,7 +89,7 @@ async fn main(_spawner: Spawner, p: Peripherals) { let mut state = State::<8, 1>::new(); - let mut builder = UsbDeviceBuilder::new_with_channel( + let mut builder = UsbDeviceBuilder::new( driver, config, &mut device_descriptor, @@ -90,7 +97,6 @@ async fn main(_spawner: Spawner, p: Peripherals) { &mut bos_descriptor, &mut control_buf, Some(&device_state_handler), - &USB_COMMANDS, ); // Create classes on the builder. @@ -107,7 +113,22 @@ async fn main(_spawner: Spawner, p: Peripherals) { let mut usb = builder.build(); // Run the USB device. - let usb_fut = usb.run(); + let usb_fut = async { + enable_command().await; + loop { + match select(usb.run(), USB_COMMANDS.recv()).await { + embassy::util::Either::First(_) => defmt::unreachable!(), + embassy::util::Either::Second(cmd) => match cmd { + Command::Enable => warn!("Enable when already enabled!"), + Command::Disable => { + usb.disable().await; + enable_command().await; + } + Command::RemoteWakeup => unwrap!(usb.remote_wakeup().await), + }, + } + } + }; let mut button = Input::new(p.P0_11.degrade(), Pull::Up); @@ -121,7 +142,7 @@ async fn main(_spawner: Spawner, p: Peripherals) { if SUSPENDED.load(Ordering::Acquire) { info!("Triggering remote wakeup"); - USB_COMMANDS.send(DeviceCommand::RemoteWakeup).await; + USB_COMMANDS.send(Command::RemoteWakeup).await; } else { let report = KeyboardReport { keycodes: [4, 0, 0, 0, 0, 0], @@ -168,6 +189,15 @@ async fn main(_spawner: Spawner, p: Peripherals) { join(usb_fut, join(in_fut, out_fut)).await; } +async fn enable_command() { + loop { + match USB_COMMANDS.recv().await { + Command::Enable => break, + cmd => warn!("Received command {:?} when disabled!", cmd), + } + } +} + struct MyRequestHandler {} impl RequestHandler for MyRequestHandler { @@ -204,6 +234,16 @@ impl MyDeviceStateHandler { } impl DeviceStateHandler for MyDeviceStateHandler { + fn enabled(&self, enabled: bool) { + self.configured.store(false, Ordering::Relaxed); + SUSPENDED.store(false, Ordering::Release); + if enabled { + info!("Device enabled"); + } else { + info!("Device disabled"); + } + } + fn reset(&self) { self.configured.store(false, Ordering::Relaxed); info!("Bus reset, the Vbus current limit is 100mA"); @@ -240,9 +280,4 @@ impl DeviceStateHandler for MyDeviceStateHandler { } } } - - fn disabled(&self) { - self.configured.store(false, Ordering::Relaxed); - info!("Device disabled"); - } } From b0725c14d36cb9c508cb9fcec5b92ca9e0148583 Mon Sep 17 00:00:00 2001 From: alexmoon Date: Wed, 13 Apr 2022 16:04:31 -0400 Subject: [PATCH 7/7] Split UsbDevice::run into run and run_until_suspend --- embassy-usb/src/lib.rs | 129 ++++++++++++++--------- examples/nrf/src/bin/usb_hid_keyboard.rs | 60 ++++++----- 2 files changed, 113 insertions(+), 76 deletions(-) diff --git a/embassy-usb/src/lib.rs b/embassy-usb/src/lib.rs index ccea8bc7a..baeca099f 100644 --- a/embassy-usb/src/lib.rs +++ b/embassy-usb/src/lib.rs @@ -100,7 +100,6 @@ pub struct UsbDevice<'d, D: Driver<'d>> { device_state: UsbDeviceState, suspended: bool, - in_control_handler: bool, remote_wakeup_enabled: bool, self_powered: bool, pending_address: u8, @@ -138,7 +137,6 @@ impl<'d, D: Driver<'d>> UsbDevice<'d, D> { control_buf, device_state: UsbDeviceState::Disabled, suspended: false, - in_control_handler: false, remote_wakeup_enabled: false, self_powered: false, pending_address: 0, @@ -146,7 +144,26 @@ impl<'d, D: Driver<'d>> UsbDevice<'d, D> { } } + /// Runs the `UsbDevice` forever. + /// + /// This future may leave the bus in an invalid state if it is dropped. + /// After dropping the future, [`UsbDevice::disable()`] should be called + /// before calling any other `UsbDevice` methods to fully reset the + /// peripheral. pub async fn run(&mut self) -> ! { + loop { + self.run_until_suspend().await; + self.wait_resume().await; + } + } + + /// Runs the `UsbDevice` until the bus is suspended. + /// + /// This future may leave the bus in an invalid state if it is dropped. + /// After dropping the future, [`UsbDevice::disable()`] should be called + /// before calling any other `UsbDevice` methods to fully reset the + /// peripheral. + pub async fn run_until_suspend(&mut self) -> () { if self.device_state == UsbDeviceState::Disabled { self.bus.enable().await; self.device_state = UsbDeviceState::Default; @@ -154,66 +171,33 @@ impl<'d, D: Driver<'d>> UsbDevice<'d, D> { if let Some(h) = &self.handler { h.enabled(true); } - } else if self.in_control_handler { - warn!("usb: control request interrupted"); - self.control.reject(); - self.in_control_handler = false; } loop { let control_fut = self.control.setup(); let bus_fut = self.bus.poll(); match select(bus_fut, control_fut).await { - Either::First(evt) => match evt { - Event::Reset => { - trace!("usb: reset"); - self.device_state = UsbDeviceState::Default; - self.suspended = false; - self.remote_wakeup_enabled = false; - self.pending_address = 0; - - for (_, h) in self.interfaces.iter_mut() { - h.reset(); - } - - if let Some(h) = &self.handler { - h.reset(); - } + Either::First(evt) => { + self.handle_bus_event(evt); + if self.suspended { + return; } - Event::Resume => { - trace!("usb: resume"); - self.suspended = false; - if let Some(h) = &self.handler { - h.suspended(false); - } - } - Event::Suspend => { - trace!("usb: suspend"); - self.suspended = true; - if let Some(h) = &self.handler { - h.suspended(true); - } - } - }, - Either::Second(req) => { - self.in_control_handler = true; - match req { - Setup::DataIn(req, stage) => self.handle_control_in(req, stage).await, - Setup::DataOut(req, stage) => self.handle_control_out(req, stage).await, - } - self.in_control_handler = false; } + Either::Second(req) => match req { + Setup::DataIn(req, stage) => self.handle_control_in(req, stage).await, + Setup::DataOut(req, stage) => self.handle_control_out(req, stage).await, + }, } } } + /// Disables the USB peripheral. pub async fn disable(&mut self) { if self.device_state != UsbDeviceState::Disabled { self.bus.disable().await; self.device_state = UsbDeviceState::Disabled; self.suspended = false; self.remote_wakeup_enabled = false; - self.in_control_handler = false; if let Some(h) = &self.handler { h.enabled(false); @@ -221,11 +205,26 @@ impl<'d, D: Driver<'d>> UsbDevice<'d, D> { } } + /// Waits for a resume condition on the USB bus. + /// + /// This future is cancel-safe. + pub async fn wait_resume(&mut self) { + while self.suspended { + let evt = self.bus.poll().await; + self.handle_bus_event(evt); + } + } + + /// Initiates a device remote wakeup on the USB bus. + /// + /// If the bus is not suspended or remote wakeup is not enabled, an error + /// will be returned. + /// + /// This future may leave the bus in an inconsistent state if dropped. + /// After dropping the future, [`UsbDevice::disable()`] should be called + /// before calling any other `UsbDevice` methods to fully reset the peripheral. pub async fn remote_wakeup(&mut self) -> Result<(), RemoteWakeupError> { - if self.device_state == UsbDeviceState::Configured - && self.suspended - && self.remote_wakeup_enabled - { + if self.suspended && self.remote_wakeup_enabled { self.bus.remote_wakeup().await?; self.suspended = false; @@ -239,6 +238,40 @@ impl<'d, D: Driver<'d>> UsbDevice<'d, D> { } } + fn handle_bus_event(&mut self, evt: Event) { + match evt { + Event::Reset => { + trace!("usb: reset"); + self.device_state = UsbDeviceState::Default; + self.suspended = false; + self.remote_wakeup_enabled = false; + self.pending_address = 0; + + for (_, h) in self.interfaces.iter_mut() { + h.reset(); + } + + if let Some(h) = &self.handler { + h.reset(); + } + } + Event::Resume => { + trace!("usb: resume"); + self.suspended = false; + if let Some(h) = &self.handler { + h.suspended(false); + } + } + Event::Suspend => { + trace!("usb: suspend"); + self.suspended = true; + if let Some(h) = &self.handler { + h.suspended(true); + } + } + } + } + async fn handle_control_out(&mut self, req: Request, stage: DataOutStage) { const CONFIGURATION_NONE_U16: u16 = CONFIGURATION_NONE as u16; const CONFIGURATION_VALUE_U16: u16 = CONFIGURATION_VALUE as u16; diff --git a/examples/nrf/src/bin/usb_hid_keyboard.rs b/examples/nrf/src/bin/usb_hid_keyboard.rs index 32659dfbb..5f03f5126 100644 --- a/examples/nrf/src/bin/usb_hid_keyboard.rs +++ b/examples/nrf/src/bin/usb_hid_keyboard.rs @@ -6,12 +6,11 @@ use core::mem; use core::sync::atomic::{AtomicBool, Ordering}; use defmt::*; -use embassy::blocking_mutex::raw::CriticalSectionRawMutex; -use embassy::channel::Channel; +use embassy::channel::Signal; use embassy::executor::Spawner; use embassy::interrupt::InterruptExt; use embassy::time::Duration; -use embassy::util::select; +use embassy::util::{select, select3, Either, Either3}; use embassy_nrf::gpio::{Input, Pin, Pull}; use embassy_nrf::interrupt; use embassy_nrf::pac; @@ -26,14 +25,7 @@ use usbd_hid::descriptor::{KeyboardReport, SerializedDescriptor}; use defmt_rtt as _; // global logger use panic_probe as _; -#[derive(defmt::Format)] -enum Command { - Enable, - Disable, - RemoteWakeup, -} - -static USB_COMMANDS: Channel = Channel::new(); +static ENABLE_USB: Signal = Signal::new(); static SUSPENDED: AtomicBool = AtomicBool::new(false); fn on_power_interrupt(_: *mut ()) { @@ -42,17 +34,13 @@ fn on_power_interrupt(_: *mut ()) { if regs.events_usbdetected.read().bits() != 0 { regs.events_usbdetected.reset(); info!("Vbus detected, enabling USB..."); - if USB_COMMANDS.try_send(Command::Enable).is_err() { - warn!("Failed to send enable command to USB channel"); - } + ENABLE_USB.signal(true); } if regs.events_usbremoved.read().bits() != 0 { regs.events_usbremoved.reset(); info!("Vbus removed, disabling USB..."); - if USB_COMMANDS.try_send(Command::Disable).is_err() { - warn!("Failed to send disable command to USB channel"); - }; + ENABLE_USB.signal(false); } } @@ -112,20 +100,35 @@ async fn main(_spawner: Spawner, p: Peripherals) { // Build the builder. let mut usb = builder.build(); + let remote_wakeup = Signal::new(); + // Run the USB device. let usb_fut = async { enable_command().await; loop { - match select(usb.run(), USB_COMMANDS.recv()).await { - embassy::util::Either::First(_) => defmt::unreachable!(), - embassy::util::Either::Second(cmd) => match cmd { - Command::Enable => warn!("Enable when already enabled!"), - Command::Disable => { + match select(usb.run_until_suspend(), ENABLE_USB.wait()).await { + Either::First(_) => {} + Either::Second(enable) => { + if enable { + warn!("Enable when already enabled!"); + } else { usb.disable().await; enable_command().await; } - Command::RemoteWakeup => unwrap!(usb.remote_wakeup().await), - }, + } + } + + match select3(usb.wait_resume(), ENABLE_USB.wait(), remote_wakeup.wait()).await { + Either3::First(_) => (), + Either3::Second(enable) => { + if enable { + warn!("Enable when already enabled!"); + } else { + usb.disable().await; + enable_command().await; + } + } + Either3::Third(_) => unwrap!(usb.remote_wakeup().await), } } }; @@ -142,7 +145,7 @@ async fn main(_spawner: Spawner, p: Peripherals) { if SUSPENDED.load(Ordering::Acquire) { info!("Triggering remote wakeup"); - USB_COMMANDS.send(Command::RemoteWakeup).await; + remote_wakeup.signal(()); } else { let report = KeyboardReport { keycodes: [4, 0, 0, 0, 0, 0], @@ -191,9 +194,10 @@ async fn main(_spawner: Spawner, p: Peripherals) { async fn enable_command() { loop { - match USB_COMMANDS.recv().await { - Command::Enable => break, - cmd => warn!("Received command {:?} when disabled!", cmd), + if ENABLE_USB.wait().await { + break; + } else { + warn!("Received disable signal when already disabled!"); } } }