From 69c482f629776b4c2e27af3758b4329348fddc7b Mon Sep 17 00:00:00 2001 From: ROMemories Date: Mon, 1 Jul 2024 16:43:41 +0200 Subject: [PATCH] fix(embassy-init): move out peripherals required for drivers earlier Previously, it would have been possible for user tasks to take peripherals out of `OptionalPeripherals` *before* the required peripherals had been extracted for drivers. This now takes the peripherals before starting the user tasks, making this scenario impossible. --- src/riot-rs-embassy/src/arch/nrf/usb.rs | 9 ++++--- src/riot-rs-embassy/src/arch/rp2040/usb.rs | 9 ++++--- src/riot-rs-embassy/src/lib.rs | 21 +++++++++++----- src/riot-rs-embassy/src/wifi/cyw43.rs | 28 ++++++++++------------ src/riot-rs-embassy/src/wifi/esp_wifi.rs | 16 ++++++++----- 5 files changed, 46 insertions(+), 37 deletions(-) diff --git a/src/riot-rs-embassy/src/arch/nrf/usb.rs b/src/riot-rs-embassy/src/arch/nrf/usb.rs index 8772a2ead..9b18dbcd8 100644 --- a/src/riot-rs-embassy/src/arch/nrf/usb.rs +++ b/src/riot-rs-embassy/src/arch/nrf/usb.rs @@ -7,8 +7,6 @@ use embassy_nrf::{ }, }; -use crate::arch; - #[cfg(context = "nrf52")] bind_interrupts!(struct Irqs { USBD => usb::InterruptHandler; @@ -23,7 +21,8 @@ bind_interrupts!(struct Irqs { pub type UsbDriver = Driver<'static, peripherals::USBD, HardwareVbusDetect>; -pub fn driver(peripherals: &mut arch::OptionalPeripherals) -> UsbDriver { - let usbd = peripherals.USBD.take().unwrap(); - Driver::new(usbd, Irqs, HardwareVbusDetect::new(Irqs)) +crate::define_peripherals!(Peripherals { usbd: USBD }); + +pub fn driver(peripherals: Peripherals) -> UsbDriver { + Driver::new(peripherals.usbd, Irqs, HardwareVbusDetect::new(Irqs)) } diff --git a/src/riot-rs-embassy/src/arch/rp2040/usb.rs b/src/riot-rs-embassy/src/arch/rp2040/usb.rs index 24c598455..32d5fd9dc 100644 --- a/src/riot-rs-embassy/src/arch/rp2040/usb.rs +++ b/src/riot-rs-embassy/src/arch/rp2040/usb.rs @@ -3,15 +3,14 @@ use embassy_rp::{ usb::{Driver, InterruptHandler}, }; -use crate::arch; - bind_interrupts!(struct Irqs { USBCTRL_IRQ => InterruptHandler; }); pub type UsbDriver = Driver<'static, peripherals::USB>; -pub fn driver(peripherals: &mut arch::OptionalPeripherals) -> UsbDriver { - let usb = peripherals.USB.take().unwrap(); - Driver::new(usb, Irqs) +crate::define_peripherals!(Peripherals { usb: USB }); + +pub fn driver(peripherals: Peripherals) -> UsbDriver { + Driver::new(peripherals.usb, Irqs) } diff --git a/src/riot-rs-embassy/src/lib.rs b/src/riot-rs-embassy/src/lib.rs index 695cc70b6..eb4a5ed89 100644 --- a/src/riot-rs-embassy/src/lib.rs +++ b/src/riot-rs-embassy/src/lib.rs @@ -143,6 +143,8 @@ fn init() { #[embassy_executor::task] async fn init_task(mut peripherals: arch::OptionalPeripherals) { + use crate::define_peripherals::TakePeripherals; + println!("riot-rs-embassy::init_task()"); #[cfg(feature = "hwrng")] @@ -150,6 +152,14 @@ async fn init_task(mut peripherals: arch::OptionalPeripherals) { // Clock startup and entropy collection may lend themselves to parallelization, provided that // doesn't impact runtime RAM or flash use. + // Move out the peripherals required for drivers, so that tasks cannot mistakenly take them. + #[cfg(feature = "usb")] + let usb_peripherals: arch::usb::Peripherals = (&mut peripherals).take_peripherals(); + #[cfg(feature = "wifi-cyw43")] + let wifi_cyw43_peripherals: wifi::cyw43::Peripherals = (&mut peripherals).take_peripherals(); + #[cfg(feature = "wifi-esp")] + let esp_wifi_peripherals: wifi::esp_wifi::Peripherals = (&mut peripherals).take_peripherals(); + #[cfg(all(context = "nrf", feature = "usb"))] { // nrf52840 @@ -162,6 +172,8 @@ async fn init_task(mut peripherals: arch::OptionalPeripherals) { let spawner = Spawner::for_current_executor().await; + // Tasks have to be started before driver initializations so that the tasks are able to + // configure the drivers using hooks. for task in EMBASSY_TASKS { task(spawner, &mut peripherals); } @@ -170,7 +182,7 @@ async fn init_task(mut peripherals: arch::OptionalPeripherals) { let mut usb_builder = { let usb_config = usb::config(); - let usb_driver = arch::usb::driver(&mut peripherals); + let usb_driver = arch::usb::driver(usb_peripherals); // Create embassy-usb DeviceBuilder using the driver and config. let builder = usb::UsbBuilder::new( @@ -226,13 +238,10 @@ async fn init_task(mut peripherals: arch::OptionalPeripherals) { } #[cfg(feature = "wifi-cyw43")] - let (device, control) = { - let (net_device, control) = wifi::cyw43::device(&mut peripherals, &spawner).await; - (net_device, control) - }; + let (device, control) = wifi::cyw43::init(wifi_cyw43_peripherals, &spawner).await; #[cfg(feature = "wifi-esp")] - let device = wifi::esp_wifi::init(&mut peripherals, spawner); + let device = wifi::esp_wifi::init(esp_wifi_peripherals, spawner); #[cfg(feature = "net")] { diff --git a/src/riot-rs-embassy/src/wifi/cyw43.rs b/src/riot-rs-embassy/src/wifi/cyw43.rs index 32a712c73..3db67ddb4 100644 --- a/src/riot-rs-embassy/src/wifi/cyw43.rs +++ b/src/riot-rs-embassy/src/wifi/cyw43.rs @@ -9,8 +9,10 @@ use embassy_rp::{ use riot_rs_debug::println; -use self::rpi_pico_w::{Cyw43Periphs, CywSpi, Irqs}; -use crate::{arch::OptionalPeripherals, make_static}; +use self::rpi_pico_w::{CywSpi, Irqs}; +use crate::make_static; + +pub use rpi_pico_w::Cyw43Periphs as Peripherals; pub type NetworkDevice = cyw43::NetDriver<'static>; @@ -34,14 +36,10 @@ async fn wifi_cyw43_task(runner: Runner<'static, Output<'static>, CywSpi>) -> ! runner.run().await } -pub async fn device<'a, 'b: 'a>( - mut p: &'a mut OptionalPeripherals, +pub async fn init<'a>( + periperals: Peripherals, spawner: &crate::Spawner, -) -> (embassy_net_driver_channel::Device<'b, 1514>, Control<'b>) { - use crate::define_peripherals::TakePeripherals; - - let pins: Cyw43Periphs = p.take_peripherals(); - +) -> (embassy_net_driver_channel::Device<'a, 1514>, Control<'a>) { let fw = include_bytes!("cyw43/firmware/43439A0.bin"); let clm = include_bytes!("cyw43/firmware/43439A0_clm.bin"); @@ -52,17 +50,17 @@ pub async fn device<'a, 'b: 'a>( //let fw = unsafe { core::slice::from_raw_parts(0x10100000 as *const u8, 230321) }; //let clm = unsafe { core::slice::from_raw_parts(0x10140000 as *const u8, 4752) }; - let pwr = Output::new(pins.pwr, Level::Low); - let cs = Output::new(pins.cs, Level::High); - let mut pio = Pio::new(pins.pio, Irqs); + let pwr = Output::new(periperals.pwr, Level::Low); + let cs = Output::new(periperals.cs, Level::High); + let mut pio = Pio::new(periperals.pio, Irqs); let spi = CywSpi::new( &mut pio.common, pio.sm0, pio.irq0, cs, - pins.dio, - pins.clk, - pins.dma, + periperals.dio, + periperals.clk, + periperals.dma, ); let state = make_static!(cyw43::State::new()); diff --git a/src/riot-rs-embassy/src/wifi/esp_wifi.rs b/src/riot-rs-embassy/src/wifi/esp_wifi.rs index b85f16533..7edf8455b 100644 --- a/src/riot-rs-embassy/src/wifi/esp_wifi.rs +++ b/src/riot-rs-embassy/src/wifi/esp_wifi.rs @@ -1,9 +1,11 @@ -use esp_wifi::{wifi::WifiStaDevice, EspWifiInitialization}; +use esp_hal::peripherals; +use esp_wifi::{ + wifi::{WifiController, WifiDevice, WifiStaDevice}, + EspWifiInitialization, +}; use once_cell::sync::OnceCell; -use crate::{arch::OptionalPeripherals, Spawner}; - -use esp_wifi::wifi::{WifiController, WifiDevice}; +use crate::Spawner; pub type NetworkDevice = WifiDevice<'static, WifiStaDevice>; @@ -14,8 +16,10 @@ pub type NetworkDevice = WifiDevice<'static, WifiStaDevice>; // sure. pub static WIFI_INIT: OnceCell = OnceCell::new(); -pub fn init(peripherals: &mut OptionalPeripherals, spawner: Spawner) -> NetworkDevice { - let wifi = peripherals.WIFI.take().unwrap(); +crate::define_peripherals!(Peripherals { wifi: WIFI }); + +pub fn init(peripherals: Peripherals, spawner: Spawner) -> NetworkDevice { + let wifi = peripherals.wifi; let init = WIFI_INIT.get().unwrap(); let (device, controller) = esp_wifi::wifi::new_with_mode(init, wifi, WifiStaDevice).unwrap();