From 96d0eb94767cea6fbecc0c87d43a30d5b6d7e8e5 Mon Sep 17 00:00:00 2001 From: Dario Nieuwenhuis Date: Sun, 8 May 2022 21:37:37 +0200 Subject: [PATCH 01/15] stm32: Fix stm32f107 build. --- ci.sh | 1 + embassy-stm32/src/exti.rs | 2 +- embassy-stm32/src/rcc/mod.rs | 4 ++-- embassy-stm32/src/usb_otg.rs | 4 ++-- 4 files changed, 6 insertions(+), 5 deletions(-) diff --git a/ci.sh b/ci.sh index c6fe9c4a..00dc8cb9 100755 --- a/ci.sh +++ b/ci.sh @@ -58,6 +58,7 @@ cargo batch \ --- build --release --manifest-path embassy-stm32/Cargo.toml --target thumbv8m.main-none-eabihf --features nightly,stm32l552ze,defmt,exti,time-driver-any,unstable-traits \ --- build --release --manifest-path embassy-stm32/Cargo.toml --target thumbv6m-none-eabi --features nightly,stm32wl54jc-cm0p,defmt,exti,time-driver-any,unstable-traits \ --- build --release --manifest-path embassy-stm32/Cargo.toml --target thumbv7em-none-eabi --features nightly,stm32wle5ub,defmt,exti,time-driver-any,unstable-traits \ + --- build --release --manifest-path embassy-stm32/Cargo.toml --target thumbv7m-none-eabi --features nightly,stm32f107vc,defmt,exti,time-driver-any,unstable-traits \ --- build --release --manifest-path embassy-boot/nrf/Cargo.toml --target thumbv7em-none-eabi --features embassy-nrf/nrf52840 \ --- build --release --manifest-path embassy-boot/stm32/Cargo.toml --target thumbv7em-none-eabi --features embassy-stm32/stm32wl55jc-cm4 \ --- build --release --manifest-path docs/modules/ROOT/examples/basic/Cargo.toml --target thumbv7em-none-eabi \ diff --git a/embassy-stm32/src/exti.rs b/embassy-stm32/src/exti.rs index 957a1ca5..d065a555 100644 --- a/embassy-stm32/src/exti.rs +++ b/embassy-stm32/src/exti.rs @@ -371,7 +371,7 @@ pub(crate) unsafe fn init() { foreach_exti_irq!(enable_irq); - #[cfg(not(any(rcc_wb, rcc_wl5, rcc_wle, rcc_f1)))] + #[cfg(not(any(rcc_wb, rcc_wl5, rcc_wle, stm32f1)))] ::enable(); #[cfg(stm32f1)] ::enable(); diff --git a/embassy-stm32/src/rcc/mod.rs b/embassy-stm32/src/rcc/mod.rs index d3710b8c..959e5926 100644 --- a/embassy-stm32/src/rcc/mod.rs +++ b/embassy-stm32/src/rcc/mod.rs @@ -4,7 +4,7 @@ use crate::time::Hertz; use core::mem::MaybeUninit; #[cfg_attr(rcc_f0, path = "f0.rs")] -#[cfg_attr(rcc_f1, path = "f1.rs")] +#[cfg_attr(any(rcc_f1, rcc_f1cl), path = "f1.rs")] #[cfg_attr(rcc_f2, path = "f2.rs")] #[cfg_attr(rcc_f3, path = "f3.rs")] #[cfg_attr(any(rcc_f4, rcc_f410), path = "f4.rs")] @@ -56,7 +56,7 @@ pub struct Clocks { #[cfg(any(rcc_f2, rcc_f4, rcc_f410, rcc_f7))] pub pll48: Option, - #[cfg(rcc_f1)] + #[cfg(stm32f1)] pub adc: Hertz, #[cfg(any(rcc_h7, rcc_h7ab))] diff --git a/embassy-stm32/src/usb_otg.rs b/embassy-stm32/src/usb_otg.rs index 21b890d7..c3cd776c 100644 --- a/embassy-stm32/src/usb_otg.rs +++ b/embassy-stm32/src/usb_otg.rs @@ -3,7 +3,6 @@ use embassy::util::Unborrow; use embassy_hal_common::unborrow; use crate::gpio::sealed::AFType; -use crate::gpio::Speed; use crate::{peripherals, rcc::RccPeripheral}; macro_rules! config_ulpi_pins { @@ -13,7 +12,8 @@ macro_rules! config_ulpi_pins { critical_section::with(|_| unsafe { $( $pin.set_as_af($pin.af_num(), AFType::OutputPushPull); - $pin.set_speed(Speed::VeryHigh); + #[cfg(gpio_v2)] + $pin.set_speed(crate::gpio::Speed::VeryHigh); )* }) }; From b230ac9c1a2e33da7e2996284ae5dfdc1b832a30 Mon Sep 17 00:00:00 2001 From: Grant Miller Date: Sun, 8 May 2022 14:41:59 -0500 Subject: [PATCH 02/15] stm32/gpio: Add support for `set_speed` for gpio v1 --- embassy-stm32/src/gpio.rs | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/embassy-stm32/src/gpio.rs b/embassy-stm32/src/gpio.rs index 30f90031..d5941c2f 100644 --- a/embassy-stm32/src/gpio.rs +++ b/embassy-stm32/src/gpio.rs @@ -511,10 +511,20 @@ pub(crate) mod sealed { self.set_as_analog(); } - #[cfg(gpio_v2)] + #[cfg(any(gpio_v1, gpio_v2))] #[inline] unsafe fn set_speed(&self, speed: Speed) { let pin = self._pin() as usize; + + #[cfg(gpio_v1)] + { + let crlh = if pin < 8 { 0 } else { 1 }; + self.block().cr(crlh).modify(|w| { + w.set_mode(pin % 8, speed.into()); + }); + } + + #[cfg(gpio_v2)] self.block() .ospeedr() .modify(|w| w.set_ospeedr(pin, speed.into())); From acc176163730ef197ec7b030018bc63b6dd14720 Mon Sep 17 00:00:00 2001 From: Grant Miller Date: Sun, 8 May 2022 14:50:15 -0500 Subject: [PATCH 03/15] Remove unnecessary cfg --- embassy-stm32/src/gpio.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/embassy-stm32/src/gpio.rs b/embassy-stm32/src/gpio.rs index d5941c2f..b6982e91 100644 --- a/embassy-stm32/src/gpio.rs +++ b/embassy-stm32/src/gpio.rs @@ -511,7 +511,6 @@ pub(crate) mod sealed { self.set_as_analog(); } - #[cfg(any(gpio_v1, gpio_v2))] #[inline] unsafe fn set_speed(&self, speed: Speed) { let pin = self._pin() as usize; From 01fb447e9d8eec19c8bf766b5ae7f17c95233fd1 Mon Sep 17 00:00:00 2001 From: Matous Hybl Date: Sun, 8 May 2022 23:07:28 +0200 Subject: [PATCH 04/15] Allow maximal clock for F7 HCLK --- embassy-stm32/src/rcc/f7.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/embassy-stm32/src/rcc/f7.rs b/embassy-stm32/src/rcc/f7.rs index 9f2c63c1..f45a725c 100644 --- a/embassy-stm32/src/rcc/f7.rs +++ b/embassy-stm32/src/rcc/f7.rs @@ -170,7 +170,7 @@ pub(crate) unsafe fn init(config: Config) { // Calculate real AHB clock let hclk = sysclk / hpre_div; - assert!(hclk < max::HCLK_MAX); + assert!(hclk <= max::HCLK_MAX); let pclk1 = config .pclk1 From 2e104170de36295243608fbbebebdc6f52e8f8d0 Mon Sep 17 00:00:00 2001 From: Dario Nieuwenhuis Date: Mon, 9 May 2022 02:07:48 +0200 Subject: [PATCH 05/15] usb: remove address arg from endpoint allocation. --- embassy-nrf/src/usb.rs | 59 +++++++------------- embassy-usb/src/builder.rs | 14 ++--- embassy-usb/src/driver.rs | 2 - examples/nrf/src/bin/usb_hid_keyboard.rs | 2 +- examples/nrf/src/bin/usb_hid_mouse.rs | 2 +- examples/nrf/src/bin/usb_serial.rs | 2 +- examples/nrf/src/bin/usb_serial_multitask.rs | 4 +- 7 files changed, 32 insertions(+), 53 deletions(-) diff --git a/embassy-nrf/src/usb.rs b/embassy-nrf/src/usb.rs index c032b2cc..70393532 100644 --- a/embassy-nrf/src/usb.rs +++ b/embassy-nrf/src/usb.rs @@ -143,38 +143,32 @@ impl<'d, T: Instance> driver::Driver<'d> for Driver<'d, T> { fn alloc_endpoint_in( &mut self, - ep_addr: Option, ep_type: EndpointType, - max_packet_size: u16, + packet_size: u16, interval: u8, ) -> Result { - let index = self - .alloc_in - .allocate(ep_addr, ep_type, max_packet_size, interval)?; + let index = self.alloc_in.allocate(ep_type, packet_size, interval)?; let ep_addr = EndpointAddress::from_parts(index, UsbDirection::In); Ok(Endpoint::new(EndpointInfo { addr: ep_addr, ep_type, - max_packet_size, + max_packet_size: packet_size, interval, })) } fn alloc_endpoint_out( &mut self, - ep_addr: Option, ep_type: EndpointType, - max_packet_size: u16, + packet_size: u16, interval: u8, ) -> Result { - let index = self - .alloc_out - .allocate(ep_addr, ep_type, max_packet_size, interval)?; + let index = self.alloc_out.allocate(ep_type, packet_size, interval)?; let ep_addr = EndpointAddress::from_parts(index, UsbDirection::Out); Ok(Endpoint::new(EndpointInfo { addr: ep_addr, ep_type, - max_packet_size, + max_packet_size: packet_size, interval, })) } @@ -183,8 +177,10 @@ impl<'d, T: Instance> driver::Driver<'d> for Driver<'d, T> { &mut self, max_packet_size: u16, ) -> Result { - self.alloc_endpoint_out(Some(0x00.into()), EndpointType::Control, max_packet_size, 0)?; - self.alloc_endpoint_in(Some(0x80.into()), EndpointType::Control, max_packet_size, 0)?; + self.alloc_in.used |= 0x01; + self.alloc_in.lens[0] = max_packet_size as u8; + self.alloc_out.used |= 0x01; + self.alloc_out.lens[0] = max_packet_size as u8; Ok(ControlPipe { _phantom: PhantomData, max_packet_size, @@ -681,8 +677,7 @@ impl<'d, T: Instance> driver::ControlPipe for ControlPipe<'d, T> { .await; // Reset shorts - regs.shorts - .modify(|_, w| w.ep0datadone_ep0status().clear_bit()); + regs.shorts.write(|w| w); regs.events_ep0setup.reset(); let mut buf = [0; 8]; @@ -746,7 +741,7 @@ impl<'d, T: Instance> driver::ControlPipe for ControlPipe<'d, T> { } regs.shorts - .modify(|_, w| w.ep0datadone_ep0status().bit(last_packet)); + .write(|w| w.ep0datadone_ep0status().bit(last_packet)); regs.intenset.write(|w| { w.usbreset().set(); @@ -810,7 +805,6 @@ impl Allocator { fn allocate( &mut self, - ep_addr: Option, ep_type: EndpointType, max_packet_size: u16, _interval: u8, @@ -828,27 +822,16 @@ impl Allocator { // Endpoint directions are allocated individually. - let alloc_index = if let Some(ep_addr) = ep_addr { - match (ep_addr.index(), ep_type) { - (0, EndpointType::Control) => {} - (8, EndpointType::Isochronous) => {} - (n, EndpointType::Bulk) | (n, EndpointType::Interrupt) if n >= 1 && n <= 7 => {} - _ => return Err(driver::EndpointAllocError), - } - - ep_addr.index() - } else { - match ep_type { - EndpointType::Isochronous => 8, - EndpointType::Control => 0, - EndpointType::Interrupt | EndpointType::Bulk => { - // Find rightmost zero bit in 1..=7 - let ones = (self.used >> 1).trailing_ones() as usize; - if ones >= 7 { - return Err(driver::EndpointAllocError); - } - ones + 1 + let alloc_index = match ep_type { + EndpointType::Isochronous => 8, + EndpointType::Control => return Err(driver::EndpointAllocError), + EndpointType::Interrupt | EndpointType::Bulk => { + // Find rightmost zero bit in 1..=7 + let ones = (self.used >> 1).trailing_ones() as usize; + if ones >= 7 { + return Err(driver::EndpointAllocError); } + ones + 1 } }; diff --git a/embassy-usb/src/builder.rs b/embassy-usb/src/builder.rs index 8cf9c82a..698a5f76 100644 --- a/embassy-usb/src/builder.rs +++ b/embassy-usb/src/builder.rs @@ -372,7 +372,6 @@ impl<'a, 'd, D: Driver<'d>> InterfaceAltBuilder<'a, 'd, D> { fn endpoint_in( &mut self, - ep_addr: Option, ep_type: EndpointType, max_packet_size: u16, interval: u8, @@ -380,7 +379,7 @@ impl<'a, 'd, D: Driver<'d>> InterfaceAltBuilder<'a, 'd, D> { let ep = self .builder .driver - .alloc_endpoint_in(ep_addr, ep_type, max_packet_size, interval) + .alloc_endpoint_in(ep_type, max_packet_size, interval) .expect("alloc_endpoint_in failed"); self.builder.config_descriptor.endpoint(ep.info()); @@ -390,7 +389,6 @@ impl<'a, 'd, D: Driver<'d>> InterfaceAltBuilder<'a, 'd, D> { fn endpoint_out( &mut self, - ep_addr: Option, ep_type: EndpointType, max_packet_size: u16, interval: u8, @@ -398,7 +396,7 @@ impl<'a, 'd, D: Driver<'d>> InterfaceAltBuilder<'a, 'd, D> { let ep = self .builder .driver - .alloc_endpoint_out(ep_addr, ep_type, max_packet_size, interval) + .alloc_endpoint_out(ep_type, max_packet_size, interval) .expect("alloc_endpoint_out failed"); self.builder.config_descriptor.endpoint(ep.info()); @@ -411,7 +409,7 @@ impl<'a, 'd, D: Driver<'d>> InterfaceAltBuilder<'a, 'd, D> { /// Descriptors are written in the order builder functions are called. Note that some /// classes care about the order. pub fn endpoint_bulk_in(&mut self, max_packet_size: u16) -> D::EndpointIn { - self.endpoint_in(None, EndpointType::Bulk, max_packet_size, 0) + self.endpoint_in(EndpointType::Bulk, max_packet_size, 0) } /// Allocate a BULK OUT endpoint and write its descriptor. @@ -419,7 +417,7 @@ impl<'a, 'd, D: Driver<'d>> InterfaceAltBuilder<'a, 'd, D> { /// Descriptors are written in the order builder functions are called. Note that some /// classes care about the order. pub fn endpoint_bulk_out(&mut self, max_packet_size: u16) -> D::EndpointOut { - self.endpoint_out(None, EndpointType::Bulk, max_packet_size, 0) + self.endpoint_out(EndpointType::Bulk, max_packet_size, 0) } /// Allocate a INTERRUPT IN endpoint and write its descriptor. @@ -427,11 +425,11 @@ impl<'a, 'd, D: Driver<'d>> InterfaceAltBuilder<'a, 'd, D> { /// Descriptors are written in the order builder functions are called. Note that some /// classes care about the order. pub fn endpoint_interrupt_in(&mut self, max_packet_size: u16, interval: u8) -> D::EndpointIn { - self.endpoint_in(None, EndpointType::Interrupt, max_packet_size, interval) + self.endpoint_in(EndpointType::Interrupt, max_packet_size, interval) } /// Allocate a INTERRUPT OUT endpoint and write its descriptor. pub fn endpoint_interrupt_out(&mut self, max_packet_size: u16, interval: u8) -> D::EndpointOut { - self.endpoint_out(None, EndpointType::Interrupt, max_packet_size, interval) + self.endpoint_out(EndpointType::Interrupt, max_packet_size, interval) } } diff --git a/embassy-usb/src/driver.rs b/embassy-usb/src/driver.rs index e552dc7b..a782b377 100644 --- a/embassy-usb/src/driver.rs +++ b/embassy-usb/src/driver.rs @@ -25,7 +25,6 @@ pub trait Driver<'a> { /// * `interval` - Polling interval parameter for interrupt endpoints. fn alloc_endpoint_out( &mut self, - ep_addr: Option, ep_type: EndpointType, max_packet_size: u16, interval: u8, @@ -33,7 +32,6 @@ pub trait Driver<'a> { fn alloc_endpoint_in( &mut self, - ep_addr: Option, ep_type: EndpointType, max_packet_size: u16, interval: u8, diff --git a/examples/nrf/src/bin/usb_hid_keyboard.rs b/examples/nrf/src/bin/usb_hid_keyboard.rs index 3852dd8d..d855a3a5 100644 --- a/examples/nrf/src/bin/usb_hid_keyboard.rs +++ b/examples/nrf/src/bin/usb_hid_keyboard.rs @@ -71,7 +71,7 @@ async fn main(_spawner: Spawner, p: Peripherals) { let mut device_descriptor = [0; 256]; let mut config_descriptor = [0; 256]; let mut bos_descriptor = [0; 256]; - let mut control_buf = [0; 16]; + let mut control_buf = [0; 64]; let request_handler = MyRequestHandler {}; let device_state_handler = MyDeviceStateHandler::new(); diff --git a/examples/nrf/src/bin/usb_hid_mouse.rs b/examples/nrf/src/bin/usb_hid_mouse.rs index e70dc51a..c526c1c6 100644 --- a/examples/nrf/src/bin/usb_hid_mouse.rs +++ b/examples/nrf/src/bin/usb_hid_mouse.rs @@ -50,7 +50,7 @@ async fn main(_spawner: Spawner, p: Peripherals) { let mut device_descriptor = [0; 256]; let mut config_descriptor = [0; 256]; let mut bos_descriptor = [0; 256]; - let mut control_buf = [0; 16]; + let mut control_buf = [0; 64]; let request_handler = MyRequestHandler {}; let mut state = State::new(); diff --git a/examples/nrf/src/bin/usb_serial.rs b/examples/nrf/src/bin/usb_serial.rs index bc41c2ac..11651f82 100644 --- a/examples/nrf/src/bin/usb_serial.rs +++ b/examples/nrf/src/bin/usb_serial.rs @@ -48,7 +48,7 @@ async fn main(_spawner: Spawner, p: Peripherals) { let mut device_descriptor = [0; 256]; let mut config_descriptor = [0; 256]; let mut bos_descriptor = [0; 256]; - let mut control_buf = [0; 7]; + let mut control_buf = [0; 64]; let mut state = State::new(); diff --git a/examples/nrf/src/bin/usb_serial_multitask.rs b/examples/nrf/src/bin/usb_serial_multitask.rs index 31e0af48..d8fac7a2 100644 --- a/examples/nrf/src/bin/usb_serial_multitask.rs +++ b/examples/nrf/src/bin/usb_serial_multitask.rs @@ -64,7 +64,7 @@ async fn main(spawner: Spawner, p: Peripherals) { device_descriptor: [u8; 256], config_descriptor: [u8; 256], bos_descriptor: [u8; 256], - control_buf: [u8; 7], + control_buf: [u8; 64], serial_state: State<'static>, } static RESOURCES: Forever = Forever::new(); @@ -72,7 +72,7 @@ async fn main(spawner: Spawner, p: Peripherals) { device_descriptor: [0; 256], config_descriptor: [0; 256], bos_descriptor: [0; 256], - control_buf: [0; 7], + control_buf: [0; 64], serial_state: State::new(), }); From 7ed462a6575cba95e8f07d2d9516d5e7b33d7196 Mon Sep 17 00:00:00 2001 From: Dario Nieuwenhuis Date: Mon, 9 May 2022 02:11:02 +0200 Subject: [PATCH 06/15] usb: simplify control in/out handlng, calling response from a single place. --- embassy-usb-ncm/src/lib.rs | 14 +- embassy-usb/src/control.rs | 29 +--- embassy-usb/src/descriptor.rs | 1 + embassy-usb/src/lib.rs | 270 +++++++++++++++++++--------------- 4 files changed, 158 insertions(+), 156 deletions(-) diff --git a/embassy-usb-ncm/src/lib.rs b/embassy-usb-ncm/src/lib.rs index 3a5abee8..71d0691d 100644 --- a/embassy-usb-ncm/src/lib.rs +++ b/embassy-usb-ncm/src/lib.rs @@ -133,6 +133,7 @@ impl Default for ControlShared { struct CommControl<'a> { mac_addr_string: StringIndex, shared: &'a ControlShared, + mac_addr_str: [u8; 12], } impl<'d> ControlHandler for CommControl<'d> { @@ -178,24 +179,20 @@ impl<'d> ControlHandler for CommControl<'d> { } } - fn get_string<'a>( - &'a mut self, - index: StringIndex, - _lang_id: u16, - buf: &'a mut [u8], - ) -> Option<&'a str> { + fn get_string(&mut self, index: StringIndex, _lang_id: u16) -> Option<&str> { if index == self.mac_addr_string { let mac_addr = self.shared.mac_addr.get(); + let s = &mut self.mac_addr_str; for i in 0..12 { let n = (mac_addr[i / 2] >> ((1 - i % 2) * 4)) & 0xF; - buf[i] = match n { + s[i] = match n { 0x0..=0x9 => b'0' + n, 0xA..=0xF => b'A' + n - 0xA, _ => unreachable!(), } } - Some(unsafe { core::str::from_utf8_unchecked(&buf[..12]) }) + Some(unsafe { core::str::from_utf8_unchecked(s) }) } else { warn!("unknown string index requested"); None @@ -244,6 +241,7 @@ impl<'d, D: Driver<'d>> CdcNcmClass<'d, D> { iface.handler(state.comm_control.write(CommControl { mac_addr_string, shared: &control_shared, + mac_addr_str: [0; 12], })); let comm_if = iface.interface_number(); let mut alt = iface.alt_setting(USB_CLASS_CDC, CDC_SUBCLASS_NCM, CDC_PROTOCOL_NONE); diff --git a/embassy-usb/src/control.rs b/embassy-usb/src/control.rs index ff42f9d7..4fc65b6a 100644 --- a/embassy-usb/src/control.rs +++ b/embassy-usb/src/control.rs @@ -1,9 +1,7 @@ use core::mem; -use crate::descriptor::DescriptorWriter; -use crate::driver::{self, EndpointError}; - use super::types::*; +use crate::driver::{self, EndpointError}; /// Control request type. #[repr(u8)] @@ -191,16 +189,8 @@ pub trait ControlHandler { } /// Called when a GET_DESCRIPTOR STRING control request is received. - /// - /// Write the response string somewhere (usually to `buf`, but you may use another buffer - /// owned by yourself, or a static buffer), then return it. - fn get_string<'a>( - &'a mut self, - index: StringIndex, - lang_id: u16, - buf: &'a mut [u8], - ) -> Option<&'a str> { - let _ = (index, lang_id, buf); + fn get_string(&mut self, index: StringIndex, lang_id: u16) -> Option<&str> { + let _ = (index, lang_id); None } } @@ -316,19 +306,6 @@ impl ControlPipe { } } - pub(crate) async fn accept_in_writer( - &mut self, - req: Request, - stage: DataInStage, - f: impl FnOnce(&mut DescriptorWriter), - ) { - let mut buf = [0; 256]; - let mut w = DescriptorWriter::new(&mut buf); - f(&mut w); - let pos = w.position().min(usize::from(req.length)); - self.accept_in(&buf[..pos], stage).await - } - pub(crate) fn accept(&mut self, _: StatusStage) { trace!(" control accept"); self.control.accept(); diff --git a/embassy-usb/src/descriptor.rs b/embassy-usb/src/descriptor.rs index dce32678..7f23fd92 100644 --- a/embassy-usb/src/descriptor.rs +++ b/embassy-usb/src/descriptor.rs @@ -244,6 +244,7 @@ impl<'a> DescriptorWriter<'a> { } /// Writes a string descriptor. + #[allow(unused)] pub(crate) fn string(&mut self, string: &str) { let mut pos = self.position; diff --git a/embassy-usb/src/lib.rs b/embassy-usb/src/lib.rs index 3bfedc04..69030588 100644 --- a/embassy-usb/src/lib.rs +++ b/embassy-usb/src/lib.rs @@ -100,15 +100,19 @@ struct Interface<'d> { } pub struct UsbDevice<'d, D: Driver<'d>> { + control_buf: &'d mut [u8], + control: ControlPipe, + inner: Inner<'d, D>, +} + +struct Inner<'d, D: Driver<'d>> { bus: D::Bus, handler: Option<&'d dyn DeviceStateHandler>, - control: ControlPipe, config: Config<'d>, device_descriptor: &'d [u8], config_descriptor: &'d [u8], bos_descriptor: &'d [u8], - control_buf: &'d mut [u8], device_state: UsbDeviceState, suspended: bool, @@ -139,20 +143,23 @@ impl<'d, D: Driver<'d>> UsbDevice<'d, D> { let bus = driver.into_bus(); Self { - bus, - config, - handler, - control: ControlPipe::new(control), - device_descriptor, - config_descriptor, - bos_descriptor, control_buf, - device_state: UsbDeviceState::Disabled, - suspended: false, - remote_wakeup_enabled: false, - self_powered: false, - pending_address: 0, - interfaces, + control: ControlPipe::new(control), + inner: Inner { + bus, + config, + handler, + device_descriptor, + config_descriptor, + bos_descriptor, + + device_state: UsbDeviceState::Disabled, + suspended: false, + remote_wakeup_enabled: false, + self_powered: false, + pending_address: 0, + interfaces, + }, } } @@ -176,28 +183,60 @@ impl<'d, D: Driver<'d>> UsbDevice<'d, D> { /// 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; + if self.inner.device_state == UsbDeviceState::Disabled { + self.inner.bus.enable().await; + self.inner.device_state = UsbDeviceState::Default; - if let Some(h) = &self.handler { + if let Some(h) = &self.inner.handler { h.enabled(true); } } loop { let control_fut = self.control.setup(); - let bus_fut = self.bus.poll(); + let bus_fut = self.inner.bus.poll(); match select(bus_fut, control_fut).await { Either::First(evt) => { - self.handle_bus_event(evt); - if self.suspended { + self.inner.handle_bus_event(evt); + if self.inner.suspended { return; } } 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, + Setup::DataIn(req, mut stage) => { + // If we don't have an address yet, respond with max 1 packet. + // The host doesn't know our EP0 max packet size yet, and might assume + // a full-length packet is a short packet, thinking we're done sending data. + // See https://github.com/hathach/tinyusb/issues/184 + const DEVICE_DESCRIPTOR_LEN: u8 = 18; + if self.inner.pending_address == 0 + && self.inner.config.max_packet_size_0 < DEVICE_DESCRIPTOR_LEN + && (self.inner.config.max_packet_size_0 as usize) < stage.length + { + trace!("received control req while not addressed: capping response to 1 packet."); + stage.length = self.inner.config.max_packet_size_0 as _; + } + + match self.inner.handle_control_in(req, &mut self.control_buf) { + InResponse::Accepted(data) => self.control.accept_in(data, stage).await, + InResponse::Rejected => self.control.reject(), + } + } + Setup::DataOut(req, stage) => { + let (data, stage) = + match self.control.data_out(self.control_buf, stage).await { + Ok(data) => data, + Err(_) => { + warn!("usb: failed to read CONTROL OUT data stage."); + return; + } + }; + + match self.inner.handle_control_out(req, data) { + OutResponse::Accepted => self.control.accept(stage), + OutResponse::Rejected => self.control.reject(), + } + } }, } } @@ -205,13 +244,13 @@ impl<'d, D: Driver<'d>> UsbDevice<'d, D> { /// 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; + if self.inner.device_state != UsbDeviceState::Disabled { + self.inner.bus.disable().await; + self.inner.device_state = UsbDeviceState::Disabled; + self.inner.suspended = false; + self.inner.remote_wakeup_enabled = false; - if let Some(h) = &self.handler { + if let Some(h) = &self.inner.handler { h.enabled(false); } } @@ -221,9 +260,9 @@ impl<'d, D: Driver<'d>> UsbDevice<'d, D> { /// /// 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); + while self.inner.suspended { + let evt = self.inner.bus.poll().await; + self.inner.handle_bus_event(evt); } } @@ -236,11 +275,11 @@ impl<'d, D: Driver<'d>> UsbDevice<'d, D> { /// 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.suspended && self.remote_wakeup_enabled { - self.bus.remote_wakeup().await?; - self.suspended = false; + if self.inner.suspended && self.inner.remote_wakeup_enabled { + self.inner.bus.remote_wakeup().await?; + self.inner.suspended = false; - if let Some(h) = &self.handler { + if let Some(h) = &self.inner.handler { h.suspended(false); } @@ -249,7 +288,9 @@ impl<'d, D: Driver<'d>> UsbDevice<'d, D> { Err(RemoteWakeupError::InvalidState) } } +} +impl<'d, D: Driver<'d>> Inner<'d, D> { fn handle_bus_event(&mut self, evt: Event) { match evt { Event::Reset => { @@ -288,18 +329,10 @@ impl<'d, D: Driver<'d>> UsbDevice<'d, D> { } } - async fn handle_control_out(&mut self, req: Request, stage: DataOutStage) { + fn handle_control_out(&mut self, req: Request, data: &[u8]) -> OutResponse { const CONFIGURATION_NONE_U16: u16 = CONFIGURATION_NONE as u16; const CONFIGURATION_VALUE_U16: u16 = CONFIGURATION_VALUE as u16; - let (data, stage) = match self.control.data_out(self.control_buf, stage).await { - Ok(data) => data, - Err(_) => { - warn!("usb: failed to read CONTROL OUT data stage."); - return; - } - }; - match (req.request_type, req.recipient) { (RequestType::Standard, Recipient::Device) => match (req.request, req.value) { (Request::CLEAR_FEATURE, Request::FEATURE_DEVICE_REMOTE_WAKEUP) => { @@ -307,14 +340,14 @@ impl<'d, D: Driver<'d>> UsbDevice<'d, D> { if let Some(h) = &self.handler { h.remote_wakeup_enabled(false); } - self.control.accept(stage) + OutResponse::Accepted } (Request::SET_FEATURE, Request::FEATURE_DEVICE_REMOTE_WAKEUP) => { self.remote_wakeup_enabled = true; if let Some(h) = &self.handler { h.remote_wakeup_enabled(true); } - self.control.accept(stage) + OutResponse::Accepted } (Request::SET_ADDRESS, addr @ 1..=127) => { self.pending_address = addr as u8; @@ -323,7 +356,7 @@ impl<'d, D: Driver<'d>> UsbDevice<'d, D> { if let Some(h) = &self.handler { h.addressed(self.pending_address); } - self.control.accept(stage) + OutResponse::Accepted } (Request::SET_CONFIGURATION, CONFIGURATION_VALUE_U16) => { debug!("SET_CONFIGURATION: configured"); @@ -344,10 +377,10 @@ impl<'d, D: Driver<'d>> UsbDevice<'d, D> { h.configured(true); } - self.control.accept(stage) + OutResponse::Accepted } (Request::SET_CONFIGURATION, CONFIGURATION_NONE_U16) => match self.device_state { - UsbDeviceState::Default => self.control.accept(stage), + UsbDeviceState::Default => OutResponse::Accepted, _ => { debug!("SET_CONFIGURATION: unconfigured"); self.device_state = UsbDeviceState::Addressed; @@ -363,15 +396,15 @@ impl<'d, D: Driver<'d>> UsbDevice<'d, D> { h.configured(false); } - self.control.accept(stage) + OutResponse::Accepted } }, - _ => self.control.reject(), + _ => OutResponse::Rejected, }, (RequestType::Standard, Recipient::Interface) => { let iface = match self.interfaces.get_mut(req.index as usize) { Some(iface) => iface, - None => return self.control.reject(), + None => return OutResponse::Rejected, }; match req.request { @@ -380,7 +413,7 @@ impl<'d, D: Driver<'d>> UsbDevice<'d, D> { if new_altsetting >= iface.num_alt_settings { warn!("SET_INTERFACE: trying to select alt setting out of range."); - return self.control.reject(); + return OutResponse::Rejected; } iface.current_alt_setting = new_altsetting; @@ -402,55 +435,39 @@ impl<'d, D: Driver<'d>> UsbDevice<'d, D> { if let Some(handler) = &mut iface.handler { handler.set_alternate_setting(new_altsetting); } - self.control.accept(stage) + OutResponse::Accepted } - _ => self.control.reject(), + _ => OutResponse::Rejected, } } (RequestType::Standard, Recipient::Endpoint) => match (req.request, req.value) { (Request::SET_FEATURE, Request::FEATURE_ENDPOINT_HALT) => { let ep_addr = ((req.index as u8) & 0x8f).into(); self.bus.endpoint_set_stalled(ep_addr, true); - self.control.accept(stage) + OutResponse::Accepted } (Request::CLEAR_FEATURE, Request::FEATURE_ENDPOINT_HALT) => { let ep_addr = ((req.index as u8) & 0x8f).into(); self.bus.endpoint_set_stalled(ep_addr, false); - self.control.accept(stage) + OutResponse::Accepted } - _ => self.control.reject(), + _ => OutResponse::Rejected, }, (RequestType::Class, Recipient::Interface) => { let iface = match self.interfaces.get_mut(req.index as usize) { Some(iface) => iface, - None => return self.control.reject(), + None => return OutResponse::Rejected, }; match &mut iface.handler { - Some(handler) => match handler.control_out(req, data) { - OutResponse::Accepted => self.control.accept(stage), - OutResponse::Rejected => self.control.reject(), - }, - None => self.control.reject(), + Some(handler) => handler.control_out(req, data), + None => OutResponse::Rejected, } } - _ => self.control.reject(), + _ => OutResponse::Rejected, } } - async fn handle_control_in(&mut self, req: Request, mut stage: DataInStage) { - // If we don't have an address yet, respond with max 1 packet. - // The host doesn't know our EP0 max packet size yet, and might assume - // a full-length packet is a short packet, thinking we're done sending data. - // See https://github.com/hathach/tinyusb/issues/184 - const DEVICE_DESCRIPTOR_LEN: u8 = 18; - if self.pending_address == 0 - && self.config.max_packet_size_0 < DEVICE_DESCRIPTOR_LEN - && (self.config.max_packet_size_0 as usize) < stage.length - { - trace!("received control req while not addressed: capping response to 1 packet."); - stage.length = self.config.max_packet_size_0 as _; - } - + fn handle_control_in<'a>(&'a mut self, req: Request, buf: &'a mut [u8]) -> InResponse<'a> { match (req.request_type, req.recipient) { (RequestType::Standard, Recipient::Device) => match req.request { Request::GET_STATUS => { @@ -461,42 +478,41 @@ impl<'d, D: Driver<'d>> UsbDevice<'d, D> { if self.remote_wakeup_enabled { status |= 0x0002; } - self.control.accept_in(&status.to_le_bytes(), stage).await + buf[..2].copy_from_slice(&status.to_le_bytes()); + InResponse::Accepted(&buf[..2]) } - Request::GET_DESCRIPTOR => self.handle_get_descriptor(req, stage).await, + Request::GET_DESCRIPTOR => self.handle_get_descriptor(req, buf), Request::GET_CONFIGURATION => { let status = match self.device_state { UsbDeviceState::Configured => CONFIGURATION_VALUE, _ => CONFIGURATION_NONE, }; - self.control.accept_in(&status.to_le_bytes(), stage).await + buf[0] = status; + InResponse::Accepted(&buf[..1]) } - _ => self.control.reject(), + _ => InResponse::Rejected, }, (RequestType::Standard, Recipient::Interface) => { let iface = match self.interfaces.get_mut(req.index as usize) { Some(iface) => iface, - None => return self.control.reject(), + None => return InResponse::Rejected, }; match req.request { Request::GET_STATUS => { let status: u16 = 0; - self.control.accept_in(&status.to_le_bytes(), stage).await + buf[..2].copy_from_slice(&status.to_le_bytes()); + InResponse::Accepted(&buf[..2]) } Request::GET_INTERFACE => { - self.control - .accept_in(&[iface.current_alt_setting], stage) - .await; + buf[0] = iface.current_alt_setting; + InResponse::Accepted(&buf[..1]) } Request::GET_DESCRIPTOR => match &mut iface.handler { - Some(handler) => match handler.get_descriptor(req, self.control_buf) { - InResponse::Accepted(data) => self.control.accept_in(data, stage).await, - InResponse::Rejected => self.control.reject(), - }, - None => self.control.reject(), + Some(handler) => handler.get_descriptor(req, buf), + None => InResponse::Rejected, }, - _ => self.control.reject(), + _ => InResponse::Rejected, } } (RequestType::Standard, Recipient::Endpoint) => match req.request { @@ -506,44 +522,40 @@ impl<'d, D: Driver<'d>> UsbDevice<'d, D> { if self.bus.endpoint_is_stalled(ep_addr) { status |= 0x0001; } - self.control.accept_in(&status.to_le_bytes(), stage).await + buf[..2].copy_from_slice(&status.to_le_bytes()); + InResponse::Accepted(&buf[..2]) } - _ => self.control.reject(), + _ => InResponse::Rejected, }, (RequestType::Class, Recipient::Interface) => { let iface = match self.interfaces.get_mut(req.index as usize) { Some(iface) => iface, - None => return self.control.reject(), + None => return InResponse::Rejected, }; match &mut iface.handler { - Some(handler) => match handler.control_in(req, self.control_buf) { - InResponse::Accepted(data) => self.control.accept_in(data, stage).await, - InResponse::Rejected => self.control.reject(), - }, - None => self.control.reject(), + Some(handler) => handler.control_in(req, buf), + None => InResponse::Rejected, } } - _ => self.control.reject(), + _ => InResponse::Rejected, } } - async fn handle_get_descriptor(&mut self, req: Request, stage: DataInStage) { + fn handle_get_descriptor<'a>(&'a mut self, req: Request, buf: &'a mut [u8]) -> InResponse<'a> { let (dtype, index) = req.descriptor_type_index(); match dtype { - descriptor_type::BOS => self.control.accept_in(self.bos_descriptor, stage).await, - descriptor_type::DEVICE => self.control.accept_in(self.device_descriptor, stage).await, - descriptor_type::CONFIGURATION => { - self.control.accept_in(self.config_descriptor, stage).await - } + descriptor_type::BOS => InResponse::Accepted(self.bos_descriptor), + descriptor_type::DEVICE => InResponse::Accepted(self.device_descriptor), + descriptor_type::CONFIGURATION => InResponse::Accepted(self.config_descriptor), descriptor_type::STRING => { if index == 0 { - self.control - .accept_in_writer(req, stage, |w| { - w.write(descriptor_type::STRING, &lang_id::ENGLISH_US.to_le_bytes()); - }) - .await + buf[0] = 4; // len + buf[1] = descriptor_type::STRING; + buf[2] = lang_id::ENGLISH_US as u8; + buf[3] = (lang_id::ENGLISH_US >> 8) as u8; + InResponse::Accepted(&buf[..4]) } else { let s = match index { STRING_INDEX_MANUFACTURER => self.config.manufacturer, @@ -565,7 +577,7 @@ impl<'d, D: Driver<'d>> UsbDevice<'d, D> { if let Some(handler) = &mut iface.handler { let index = StringIndex::new(index); let lang_id = req.index; - handler.get_string(index, lang_id, self.control_buf) + handler.get_string(index, lang_id) } else { warn!("String requested to an interface with no handler."); None @@ -578,15 +590,29 @@ impl<'d, D: Driver<'d>> UsbDevice<'d, D> { }; if let Some(s) = s { - self.control - .accept_in_writer(req, stage, |w| w.string(s)) - .await + if buf.len() < 2 { + panic!("control buffer too small"); + } + + buf[1] = descriptor_type::STRING; + let mut pos = 2; + for c in s.encode_utf16() { + if pos >= buf.len() { + panic!("control buffer too small"); + } + + buf[pos..pos + 2].copy_from_slice(&c.to_le_bytes()); + pos += 2; + } + + buf[0] = pos as u8; + InResponse::Accepted(&buf[..pos]) } else { - self.control.reject() + InResponse::Rejected } } } - _ => self.control.reject(), + _ => InResponse::Rejected, } } } From 02ae1138e1b67a18a0fea4f1871751ee1ad858c0 Mon Sep 17 00:00:00 2001 From: Dario Nieuwenhuis Date: Mon, 9 May 2022 03:25:21 +0200 Subject: [PATCH 07/15] usb: merge Control logic into main code. Now that control stuff is called from just one place, there's no need to keep it as a separate struct. --- embassy-usb/src/control.rs | 123 ---------------------------------- embassy-usb/src/lib.rs | 131 ++++++++++++++++++++++++------------- 2 files changed, 86 insertions(+), 168 deletions(-) diff --git a/embassy-usb/src/control.rs b/embassy-usb/src/control.rs index 4fc65b6a..12e5303c 100644 --- a/embassy-usb/src/control.rs +++ b/embassy-usb/src/control.rs @@ -1,7 +1,6 @@ use core::mem; use super::types::*; -use crate::driver::{self, EndpointError}; /// Control request type. #[repr(u8)] @@ -194,125 +193,3 @@ pub trait ControlHandler { None } } - -/// Typestate representing a ControlPipe in the DATA IN stage -#[derive(Debug)] -#[cfg_attr(feature = "defmt", derive(defmt::Format))] -pub(crate) struct DataInStage { - pub(crate) length: usize, -} - -/// Typestate representing a ControlPipe in the DATA OUT stage -#[derive(Debug)] -#[cfg_attr(feature = "defmt", derive(defmt::Format))] -pub(crate) struct DataOutStage { - length: usize, -} - -/// Typestate representing a ControlPipe in the STATUS stage -#[derive(Debug)] -#[cfg_attr(feature = "defmt", derive(defmt::Format))] -pub(crate) struct StatusStage {} - -#[derive(Debug)] -#[cfg_attr(feature = "defmt", derive(defmt::Format))] -pub(crate) enum Setup { - DataIn(Request, DataInStage), - DataOut(Request, DataOutStage), -} - -pub(crate) struct ControlPipe { - control: C, -} - -impl ControlPipe { - pub(crate) fn new(control: C) -> Self { - ControlPipe { control } - } - - pub(crate) async fn setup(&mut self) -> Setup { - let req = self.control.setup().await; - trace!("control request: {:02x}", req); - - match (req.direction, req.length) { - (UsbDirection::Out, n) => Setup::DataOut( - req, - DataOutStage { - length: usize::from(n), - }, - ), - (UsbDirection::In, n) => Setup::DataIn( - req, - DataInStage { - length: usize::from(n), - }, - ), - } - } - - pub(crate) async fn data_out<'a>( - &mut self, - buf: &'a mut [u8], - stage: DataOutStage, - ) -> Result<(&'a [u8], StatusStage), EndpointError> { - if stage.length == 0 { - Ok((&[], StatusStage {})) - } else { - let req_length = stage.length; - let max_packet_size = self.control.max_packet_size(); - let mut total = 0; - - for chunk in buf.chunks_mut(max_packet_size) { - let size = self.control.data_out(chunk).await?; - total += size; - if size < max_packet_size || total == req_length { - break; - } - } - - let res = &buf[0..total]; - #[cfg(feature = "defmt")] - trace!(" control out data: {:02x}", res); - #[cfg(not(feature = "defmt"))] - trace!(" control out data: {:02x?}", res); - - Ok((res, StatusStage {})) - } - } - - pub(crate) async fn accept_in(&mut self, buf: &[u8], stage: DataInStage) { - #[cfg(feature = "defmt")] - trace!(" control in accept {:02x}", buf); - #[cfg(not(feature = "defmt"))] - trace!(" control in accept {:02x?}", buf); - - let req_len = stage.length; - let len = buf.len().min(req_len); - let max_packet_size = self.control.max_packet_size(); - let need_zlp = len != req_len && (len % usize::from(max_packet_size)) == 0; - - let mut chunks = buf[0..len] - .chunks(max_packet_size) - .chain(need_zlp.then(|| -> &[u8] { &[] })); - - while let Some(chunk) = chunks.next() { - match self.control.data_in(chunk, chunks.size_hint().0 == 0).await { - Ok(()) => {} - Err(e) => { - warn!("control accept_in failed: {:?}", e); - return; - } - } - } - } - - pub(crate) fn accept(&mut self, _: StatusStage) { - trace!(" control accept"); - self.control.accept(); - } - - pub(crate) fn reject(&mut self) { - trace!(" control reject"); - self.control.reject(); - } -} diff --git a/embassy-usb/src/lib.rs b/embassy-usb/src/lib.rs index 69030588..99c0f823 100644 --- a/embassy-usb/src/lib.rs +++ b/embassy-usb/src/lib.rs @@ -16,6 +16,7 @@ use embassy::util::{select, Either}; use heapless::Vec; use crate::descriptor_reader::foreach_endpoint; +use crate::driver::ControlPipe; use self::control::*; use self::descriptor::*; @@ -101,7 +102,7 @@ struct Interface<'d> { pub struct UsbDevice<'d, D: Driver<'d>> { control_buf: &'d mut [u8], - control: ControlPipe, + control: D::ControlPipe, inner: Inner<'d, D>, } @@ -144,7 +145,7 @@ impl<'d, D: Driver<'d>> UsbDevice<'d, D> { Self { control_buf, - control: ControlPipe::new(control), + control, inner: Inner { bus, config, @@ -192,52 +193,12 @@ impl<'d, D: Driver<'d>> UsbDevice<'d, D> { } } - loop { + while !self.inner.suspended { let control_fut = self.control.setup(); let bus_fut = self.inner.bus.poll(); match select(bus_fut, control_fut).await { - Either::First(evt) => { - self.inner.handle_bus_event(evt); - if self.inner.suspended { - return; - } - } - Either::Second(req) => match req { - Setup::DataIn(req, mut stage) => { - // If we don't have an address yet, respond with max 1 packet. - // The host doesn't know our EP0 max packet size yet, and might assume - // a full-length packet is a short packet, thinking we're done sending data. - // See https://github.com/hathach/tinyusb/issues/184 - const DEVICE_DESCRIPTOR_LEN: u8 = 18; - if self.inner.pending_address == 0 - && self.inner.config.max_packet_size_0 < DEVICE_DESCRIPTOR_LEN - && (self.inner.config.max_packet_size_0 as usize) < stage.length - { - trace!("received control req while not addressed: capping response to 1 packet."); - stage.length = self.inner.config.max_packet_size_0 as _; - } - - match self.inner.handle_control_in(req, &mut self.control_buf) { - InResponse::Accepted(data) => self.control.accept_in(data, stage).await, - InResponse::Rejected => self.control.reject(), - } - } - Setup::DataOut(req, stage) => { - let (data, stage) = - match self.control.data_out(self.control_buf, stage).await { - Ok(data) => data, - Err(_) => { - warn!("usb: failed to read CONTROL OUT data stage."); - return; - } - }; - - match self.inner.handle_control_out(req, data) { - OutResponse::Accepted => self.control.accept(stage), - OutResponse::Rejected => self.control.reject(), - } - } - }, + Either::First(evt) => self.inner.handle_bus_event(evt), + Either::Second(req) => self.handle_control(req).await, } } } @@ -288,6 +249,86 @@ impl<'d, D: Driver<'d>> UsbDevice<'d, D> { Err(RemoteWakeupError::InvalidState) } } + + async fn handle_control(&mut self, req: Request) { + trace!("control request: {:02x}", req); + + match req.direction { + UsbDirection::In => self.handle_control_in(req).await, + UsbDirection::Out => self.handle_control_out(req).await, + } + } + + async fn handle_control_in(&mut self, req: Request) { + let mut resp_length = req.length as usize; + let max_packet_size = self.control.max_packet_size(); + + // If we don't have an address yet, respond with max 1 packet. + // The host doesn't know our EP0 max packet size yet, and might assume + // a full-length packet is a short packet, thinking we're done sending data. + // See https://github.com/hathach/tinyusb/issues/184 + const DEVICE_DESCRIPTOR_LEN: usize = 18; + if self.inner.pending_address == 0 + && max_packet_size < DEVICE_DESCRIPTOR_LEN + && (max_packet_size as usize) < resp_length + { + trace!("received control req while not addressed: capping response to 1 packet."); + resp_length = max_packet_size; + } + + match self.inner.handle_control_in(req, &mut self.control_buf) { + InResponse::Accepted(data) => { + let len = data.len().min(resp_length); + let need_zlp = len != resp_length && (len % usize::from(max_packet_size)) == 0; + + let mut chunks = data[0..len] + .chunks(max_packet_size) + .chain(need_zlp.then(|| -> &[u8] { &[] })); + + while let Some(chunk) = chunks.next() { + match self.control.data_in(chunk, chunks.size_hint().0 == 0).await { + Ok(()) => {} + Err(e) => { + warn!("control accept_in failed: {:?}", e); + return; + } + } + } + } + InResponse::Rejected => self.control.reject(), + } + } + + async fn handle_control_out(&mut self, req: Request) { + let req_length = req.length as usize; + let max_packet_size = self.control.max_packet_size(); + let mut total = 0; + + for chunk in self.control_buf[..req_length].chunks_mut(max_packet_size) { + let size = match self.control.data_out(chunk).await { + Ok(x) => x, + Err(e) => { + warn!("usb: failed to read CONTROL OUT data stage: {:?}", e); + return; + } + }; + total += size; + if size < max_packet_size || total == req_length { + break; + } + } + + let data = &self.control_buf[0..total]; + #[cfg(feature = "defmt")] + trace!(" control out data: {:02x}", data); + #[cfg(not(feature = "defmt"))] + trace!(" control out data: {:02x?}", data); + + match self.inner.handle_control_out(req, data) { + OutResponse::Accepted => self.control.accept(), + OutResponse::Rejected => self.control.reject(), + } + } } impl<'d, D: Driver<'d>> Inner<'d, D> { From 1b2a10007b55a3512e3550a8a88059c441a71d9c Mon Sep 17 00:00:00 2001 From: Daniel Bevenius Date: Mon, 9 May 2022 16:56:28 +0200 Subject: [PATCH 08/15] Update re-export comment in embassy-rp/Cargo.toml --- embassy-rp/Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/embassy-rp/Cargo.toml b/embassy-rp/Cargo.toml index 17fce54e..9f295c75 100644 --- a/embassy-rp/Cargo.toml +++ b/embassy-rp/Cargo.toml @@ -15,7 +15,7 @@ flavors = [ [features] # Reexport the PAC for the currently enabled chip at `embassy_rp::pac`. -# This is unstable because semver-minor (non-breaking) releases of embassy-nrf may major-bump (breaking) the PAC version. +# This is unstable because semver-minor (non-breaking) releases of embassy-rp may major-bump (breaking) the PAC version. # If this is an issue for you, you're encouraged to directly depend on a fixed version of the PAC. # There are no plans to make this stable. unstable-pac = [] From 6af5f8eb2da6ba9e1eabb67871c3483ba1579dd2 Mon Sep 17 00:00:00 2001 From: Dario Nieuwenhuis Date: Tue, 10 May 2022 16:53:42 +0200 Subject: [PATCH 09/15] usb: merge `alloc_control_pipe` and `into_bus` into `start`. This prevents calling `alloc_control_pipe` twice at compile time, which was always an error. --- embassy-nrf/src/usb.rs | 47 ++++++++++++--------------------------- embassy-usb/src/driver.rs | 19 ++++++++-------- embassy-usb/src/lib.rs | 10 +++------ 3 files changed, 27 insertions(+), 49 deletions(-) diff --git a/embassy-nrf/src/usb.rs b/embassy-nrf/src/usb.rs index 70393532..1162946a 100644 --- a/embassy-nrf/src/usb.rs +++ b/embassy-nrf/src/usb.rs @@ -147,7 +147,7 @@ impl<'d, T: Instance> driver::Driver<'d> for Driver<'d, T> { packet_size: u16, interval: u8, ) -> Result { - let index = self.alloc_in.allocate(ep_type, packet_size, interval)?; + let index = self.alloc_in.allocate(ep_type)?; let ep_addr = EndpointAddress::from_parts(index, UsbDirection::In); Ok(Endpoint::new(EndpointInfo { addr: ep_addr, @@ -163,7 +163,7 @@ impl<'d, T: Instance> driver::Driver<'d> for Driver<'d, T> { packet_size: u16, interval: u8, ) -> Result { - let index = self.alloc_out.allocate(ep_type, packet_size, interval)?; + let index = self.alloc_out.allocate(ep_type)?; let ep_addr = EndpointAddress::from_parts(index, UsbDirection::Out); Ok(Endpoint::new(EndpointInfo { addr: ep_addr, @@ -173,24 +173,16 @@ impl<'d, T: Instance> driver::Driver<'d> for Driver<'d, T> { })) } - fn alloc_control_pipe( - &mut self, - max_packet_size: u16, - ) -> Result { - self.alloc_in.used |= 0x01; - self.alloc_in.lens[0] = max_packet_size as u8; - self.alloc_out.used |= 0x01; - self.alloc_out.lens[0] = max_packet_size as u8; - Ok(ControlPipe { - _phantom: PhantomData, - max_packet_size, - }) - } - - fn into_bus(self) -> Self::Bus { - Bus { - phantom: PhantomData, - } + fn start(self, control_max_packet_size: u16) -> (Self::Bus, Self::ControlPipe) { + ( + Bus { + phantom: PhantomData, + }, + ControlPipe { + _phantom: PhantomData, + max_packet_size: control_max_packet_size, + }, + ) } } @@ -791,24 +783,14 @@ fn dma_end() { struct Allocator { used: u16, - // Buffers can be up to 64 Bytes since this is a Full-Speed implementation. - lens: [u8; 9], } impl Allocator { fn new() -> Self { - Self { - used: 0, - lens: [0; 9], - } + Self { used: 0 } } - fn allocate( - &mut self, - ep_type: EndpointType, - max_packet_size: u16, - _interval: u8, - ) -> Result { + fn allocate(&mut self, ep_type: EndpointType) -> Result { // Endpoint addresses are fixed in hardware: // - 0x80 / 0x00 - Control EP0 // - 0x81 / 0x01 - Bulk/Interrupt EP1 @@ -840,7 +822,6 @@ impl Allocator { } self.used |= 1 << alloc_index; - self.lens[alloc_index] = max_packet_size as u8; Ok(alloc_index) } diff --git a/embassy-usb/src/driver.rs b/embassy-usb/src/driver.rs index a782b377..57f2b065 100644 --- a/embassy-usb/src/driver.rs +++ b/embassy-usb/src/driver.rs @@ -14,7 +14,7 @@ pub trait Driver<'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 - /// [`enable`](UsbBus::enable) is called. + /// [`start`](UsbBus::start) is called. /// /// # Arguments /// @@ -37,14 +37,15 @@ pub trait Driver<'a> { interval: u8, ) -> Result; - fn alloc_control_pipe( - &mut self, - max_packet_size: u16, - ) -> Result; - - /// 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 into_bus(self) -> Self::Bus; + /// Start operation of the USB device. + /// + /// This returns the `Bus` and `ControlPipe` instances that are used to operate + /// the USB device. Additionally, this makes all the previously allocated endpoints + /// start operating. + /// + /// This consumes the `Driver` instance, so it's no longer possible to allocate more + /// endpoints. + fn start(self, control_max_packet_size: u16) -> (Self::Bus, Self::ControlPipe); /// Indicates that `set_device_address` must be called before accepting the corresponding /// control transfer, not after. diff --git a/embassy-usb/src/lib.rs b/embassy-usb/src/lib.rs index 99c0f823..b135f5eb 100644 --- a/embassy-usb/src/lib.rs +++ b/embassy-usb/src/lib.rs @@ -126,7 +126,7 @@ struct Inner<'d, D: Driver<'d>> { impl<'d, D: Driver<'d>> UsbDevice<'d, D> { pub(crate) fn build( - mut driver: D, + driver: D, config: Config<'d>, handler: Option<&'d dyn DeviceStateHandler>, device_descriptor: &'d [u8], @@ -135,13 +135,9 @@ impl<'d, D: Driver<'d>> UsbDevice<'d, D> { interfaces: Vec, MAX_INTERFACE_COUNT>, control_buf: &'d mut [u8], ) -> UsbDevice<'d, D> { - let control = driver - .alloc_control_pipe(config.max_packet_size_0 as u16) - .expect("failed to alloc control endpoint"); - - // Enable the USB bus. + // Start the USB bus. // This prevent further allocation by consuming the driver. - let bus = driver.into_bus(); + let (bus, control) = driver.start(config.max_packet_size_0 as u16); Self { control_buf, From 6d4a49bca86415b88f282d084cfa4045c171e63a Mon Sep 17 00:00:00 2001 From: Daniel Bevenius Date: Wed, 11 May 2022 16:23:31 +0200 Subject: [PATCH 10/15] Implement Output::is_set_low for embassy-rp This commit implements a suggestion for the method is_set_low which is currently a 'todo', by reading last value written to GPIO_OUT. --- embassy-rp/src/gpio.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/embassy-rp/src/gpio.rs b/embassy-rp/src/gpio.rs index 59875903..9cdba8bf 100644 --- a/embassy-rp/src/gpio.rs +++ b/embassy-rp/src/gpio.rs @@ -127,8 +127,8 @@ impl<'d, T: Pin> Output<'d, T> { /// Is the output pin set as low? pub fn is_set_low(&self) -> bool { - // todo - true + // Reading from SIO: GPIO_OUT gives the last value written. + unsafe { self.pin.sio_out().value().read() == 0 } } } From 0bb428dcc01a01f15459acb9ebed4e045448bf5d Mon Sep 17 00:00:00 2001 From: Daniel Bevenius Date: Wed, 11 May 2022 18:26:25 +0200 Subject: [PATCH 11/15] squash! Implement Output::is_set_low for embassy-rp Add check for the bit of the current pin. --- embassy-rp/src/gpio.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/embassy-rp/src/gpio.rs b/embassy-rp/src/gpio.rs index 9cdba8bf..28dfce47 100644 --- a/embassy-rp/src/gpio.rs +++ b/embassy-rp/src/gpio.rs @@ -128,7 +128,8 @@ impl<'d, T: Pin> Output<'d, T> { /// Is the output pin set as low? pub fn is_set_low(&self) -> bool { // Reading from SIO: GPIO_OUT gives the last value written. - unsafe { self.pin.sio_out().value().read() == 0 } + let val = 1 << self.pin.pin(); + unsafe { (self.pin.sio_out().value().read() & val) == 0 } } } From 2a7afe4262fd3ff8288c6381598a8794a7beee37 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timo=20Kr=C3=B6ger?= Date: Thu, 12 May 2022 07:59:33 +0200 Subject: [PATCH 12/15] Make usb_serial examples work on windows Windows shows `error 10` when using CDC ACM on non composite devices. Workaround is to use IADS: https://developer.nordicsemi.com/nRF_Connect_SDK/doc/1.9.1/kconfig/CONFIG_CDC_ACM_IAD.html#help --- examples/nrf/src/bin/usb_serial.rs | 7 +++++++ examples/nrf/src/bin/usb_serial_multitask.rs | 7 +++++++ 2 files changed, 14 insertions(+) diff --git a/examples/nrf/src/bin/usb_serial.rs b/examples/nrf/src/bin/usb_serial.rs index bc41c2ac..f607781a 100644 --- a/examples/nrf/src/bin/usb_serial.rs +++ b/examples/nrf/src/bin/usb_serial.rs @@ -43,6 +43,13 @@ async fn main(_spawner: Spawner, p: Peripherals) { config.max_power = 100; config.max_packet_size_0 = 64; + // Required for windows compatiblity. + // https://developer.nordicsemi.com/nRF_Connect_SDK/doc/1.9.1/kconfig/CONFIG_CDC_ACM_IAD.html#help + config.device_class = 0xEF; + config.device_sub_class = 0x02; + config.device_protocol = 0x01; + config.composite_with_iads = true; + // Create embassy-usb DeviceBuilder using the driver and config. // It needs some buffers for building the descriptors. let mut device_descriptor = [0; 256]; diff --git a/examples/nrf/src/bin/usb_serial_multitask.rs b/examples/nrf/src/bin/usb_serial_multitask.rs index 31e0af48..e165cd43 100644 --- a/examples/nrf/src/bin/usb_serial_multitask.rs +++ b/examples/nrf/src/bin/usb_serial_multitask.rs @@ -60,6 +60,13 @@ async fn main(spawner: Spawner, p: Peripherals) { config.max_power = 100; config.max_packet_size_0 = 64; + // Required for windows compatiblity. + // https://developer.nordicsemi.com/nRF_Connect_SDK/doc/1.9.1/kconfig/CONFIG_CDC_ACM_IAD.html#help + config.device_class = 0xEF; + config.device_sub_class = 0x02; + config.device_protocol = 0x01; + config.composite_with_iads = true; + struct Resources { device_descriptor: [u8; 256], config_descriptor: [u8; 256], From f4677469f9e9fc7bdc24577d9111636b4de2df46 Mon Sep 17 00:00:00 2001 From: Ralf Date: Wed, 11 May 2022 19:57:56 +0200 Subject: [PATCH 13/15] stm32/usart: Data length is including parity. To get e.g. 8E1 you need to choose 9 data bits --- embassy-stm32/src/usart/mod.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/embassy-stm32/src/usart/mod.rs b/embassy-stm32/src/usart/mod.rs index 6feecd18..47268011 100644 --- a/embassy-stm32/src/usart/mod.rs +++ b/embassy-stm32/src/usart/mod.rs @@ -219,7 +219,11 @@ impl<'d, T: Instance, TxDma, RxDma> Uart<'d, T, TxDma, RxDma> { w.set_ue(true); w.set_te(true); w.set_re(true); - w.set_m0(vals::M0::BIT8); + w.set_m0(if config.parity != Parity::ParityNone { + vals::M0::BIT9 + } else { + vals::M0::BIT8 + }); w.set_pce(config.parity != Parity::ParityNone); w.set_ps(match config.parity { Parity::ParityOdd => vals::Ps::ODD, From 1a216958ac121befa7da6912db307516d1ddcb07 Mon Sep 17 00:00:00 2001 From: Ralf Date: Wed, 11 May 2022 20:54:09 +0200 Subject: [PATCH 14/15] stm32/rcc: Set flash prefetch buffer and half cycle access according to AHB clock prescaler --- embassy-stm32/src/rcc/f3.rs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/embassy-stm32/src/rcc/f3.rs b/embassy-stm32/src/rcc/f3.rs index ababc4f9..5a735144 100644 --- a/embassy-stm32/src/rcc/f3.rs +++ b/embassy-stm32/src/rcc/f3.rs @@ -93,7 +93,10 @@ pub(crate) unsafe fn init(config: Config) { assert!(pclk2 <= 72_000_000); // Set latency based on HCLK frquency - FLASH.acr().write(|w| { + // RM0316: "The prefetch buffer must be kept on when using a prescaler + // different from 1 on the AHB clock.", "Half-cycle access cannot be + // used when there is a prescaler different from 1 on the AHB clock" + FLASH.acr().modify(|w| { w.set_latency(if hclk <= 24_000_000 { Latency::WS0 } else if hclk <= 48_000_000 { @@ -101,6 +104,10 @@ pub(crate) unsafe fn init(config: Config) { } else { Latency::WS2 }); + if hpre_div != 1 { + w.set_hlfcya(false); + w.set_prftbe(true); + } }); // Enable HSE From c90968bb70e626c5d2c375befbbf19423f48ba5e Mon Sep 17 00:00:00 2001 From: Ralf Date: Wed, 11 May 2022 20:56:57 +0200 Subject: [PATCH 15/15] stm32/rcc: Modify only relevant CFGR bits and keep the settings previously done. PLL settings remained intact because these bits are not writable when PLL is enabled, but prescaler settings were overwritten by selecting PLL as sysclk (CFGR.SW[1:0]). --- embassy-stm32/src/rcc/f3.rs | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/embassy-stm32/src/rcc/f3.rs b/embassy-stm32/src/rcc/f3.rs index 5a735144..c2aec04c 100644 --- a/embassy-stm32/src/rcc/f3.rs +++ b/embassy-stm32/src/rcc/f3.rs @@ -111,8 +111,9 @@ pub(crate) unsafe fn init(config: Config) { }); // Enable HSE + // RM0316: "Bits 31:26 Reserved, must be kept at reset value." if config.hse.is_some() { - RCC.cr().write(|w| { + RCC.cr().modify(|w| { w.set_hsebyp(config.bypass_hse); // We turn on clock security to switch to HSI when HSE fails w.set_csson(true); @@ -122,27 +123,30 @@ pub(crate) unsafe fn init(config: Config) { } // Enable PLL + // RM0316: "Reserved, must be kept at reset value." if let Some(ref pll_config) = pll_config { - RCC.cfgr().write(|w| { + RCC.cfgr().modify(|w| { w.set_pllmul(pll_config.pll_mul); w.set_pllsrc(pll_config.pll_src); }); if let Some(pll_div) = pll_config.pll_div { - RCC.cfgr2().write(|w| w.set_prediv(pll_div)); + RCC.cfgr2().modify(|w| w.set_prediv(pll_div)); } RCC.cr().modify(|w| w.set_pllon(true)); while !RCC.cr().read().pllrdy() {} } + // CFGR has been written before (PLL) don't overwrite these settings if config.pll48 { let usb_pre = get_usb_pre(&config, sysclk, pclk1, &pll_config); - RCC.cfgr().write(|w| { + RCC.cfgr().modify(|w| { w.set_usbpre(usb_pre); }); } // Set prescalers - RCC.cfgr().write(|w| { + // CFGR has been written before (PLL, PLL48) don't overwrite these settings + RCC.cfgr().modify(|w| { w.set_ppre2(ppre2_bits); w.set_ppre1(ppre1_bits); w.set_hpre(hpre_bits); @@ -153,7 +157,8 @@ pub(crate) unsafe fn init(config: Config) { // 1 to 16 AHB cycles after write" cortex_m::asm::delay(16); - RCC.cfgr().write(|w| { + // CFGR has been written before (PLL, PLL48, clock divider) don't overwrite these settings + RCC.cfgr().modify(|w| { w.set_sw(match (pll_config, config.hse) { (Some(_), _) => Sw::PLL, (None, Some(_)) => Sw::HSE,