Skip to content

Commit

Permalink
fix(embassy-init): move out peripherals required for drivers earlier
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
ROMemories committed Jul 1, 2024
1 parent 22bcdc8 commit d2cbe4e
Show file tree
Hide file tree
Showing 6 changed files with 49 additions and 40 deletions.
6 changes: 3 additions & 3 deletions src/riot-rs-embassy/src/arch/dummy/usb.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@ use embassy_usb::driver::{
EndpointIn, EndpointInfo, EndpointOut, EndpointType, Event, Unsupported,
};

use crate::arch;

pub struct UsbDriver;

impl<'a> Driver<'a> for UsbDriver {
Expand Down Expand Up @@ -34,7 +32,9 @@ impl<'a> Driver<'a> for UsbDriver {
}
}

pub fn driver(_peripherals: &mut arch::OptionalPeripherals) -> UsbDriver {
crate::define_peripherals!(Peripherals {});

pub fn driver(_peripherals: Peripherals) -> UsbDriver {
unimplemented!();
}

Expand Down
9 changes: 4 additions & 5 deletions src/riot-rs-embassy/src/arch/nrf/usb.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,6 @@ use embassy_nrf::{
},
};

use crate::arch;

#[cfg(context = "nrf52")]
bind_interrupts!(struct Irqs {
USBD => usb::InterruptHandler<peripherals::USBD>;
Expand All @@ -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))
}
9 changes: 4 additions & 5 deletions src/riot-rs-embassy/src/arch/rp2040/usb.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,14 @@ use embassy_rp::{
usb::{Driver, InterruptHandler},
};

use crate::arch;

bind_interrupts!(struct Irqs {
USBCTRL_IRQ => InterruptHandler<peripherals::USB>;
});

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)
}
21 changes: 15 additions & 6 deletions src/riot-rs-embassy/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -143,13 +143,23 @@ fn init() {

#[embassy_executor::task]
async fn init_task(mut peripherals: arch::OptionalPeripherals) {
use crate::define_peripherals::TakePeripherals;

Check warning on line 146 in src/riot-rs-embassy/src/lib.rs

View workflow job for this annotation

GitHub Actions / lint

unused import: `crate::define_peripherals::TakePeripherals`

warning: unused import: `crate::define_peripherals::TakePeripherals` --> src/riot-rs-embassy/src/lib.rs:146:9 | 146 | use crate::define_peripherals::TakePeripherals; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = note: `#[warn(unused_imports)]` on by default

println!("riot-rs-embassy::init_task()");

#[cfg(feature = "hwrng")]
arch::hwrng::construct_rng(&mut peripherals);
// 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
Expand All @@ -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);
}
Expand All @@ -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(
Expand Down Expand Up @@ -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")]
{
Expand Down
28 changes: 13 additions & 15 deletions src/riot-rs-embassy/src/wifi/cyw43.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>;

Expand All @@ -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");

Expand All @@ -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());
Expand Down
16 changes: 10 additions & 6 deletions src/riot-rs-embassy/src/wifi/esp_wifi.rs
Original file line number Diff line number Diff line change
@@ -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>;

Expand All @@ -14,8 +16,10 @@ pub type NetworkDevice = WifiDevice<'static, WifiStaDevice>;
// sure.
pub static WIFI_INIT: OnceCell<EspWifiInitialization> = 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();

Expand Down

0 comments on commit d2cbe4e

Please sign in to comment.