search

esp-rs / esp-hal

no_std Hardware Abstraction Layers for ESP32 microcontrollers
https://docs.esp-rs.org/esp-hal/
Apache License 2.0
699 stars 191 forks source link

Implement `Drop` for drivers to restore peripherals to their default states #1382

Open jessebraham opened 5 months ago

jessebraham commented 5 months ago

We currently are not implementing the Drop trait for (any of?) our drivers. For at least most of them, we should do so, and restore the peripherals back to their boot-default state.

I think perhaps GPIO pins would be an exception to this, and perhaps it does not make sense to implement this trait for other drivers as well. Some discussion will need to be had regarding this.

bjoernQ commented 5 months ago

Maybe we should disable the peripheral on drop (a System::disable would be needed) I am also thinking if we should actually reset the peripheral in enable ... not just clear RST?

MabezDev commented 5 months ago

Maybe we should disable the peripheral on drop (a System::disable would be needed) I am also thinking if we should actually reset the peripheral in enable ... not just clear RST?

For the most part, yes this is what we should do. It gets a bit more fun when a driver split's, for instance UartRx & UartTx. We need to only disable each "side" in this case, but also the last one dropped should fully disable the peripheral.

Dominaezzz commented 2 months ago

A thought I had about this is, what if the hal moves away from the Peripheral::take() pattern for the peripherals that need enabling/disabling, users can acquire peripherals from the system.

So instead of this.

let peripherals = Peripherals::take();
let system = SystemControl::new(peripherals.SYSTEM);

let spi2 = peripherals.SPI2.

We can have something like this.

let peripherals = Peripherals::take();
let system = SystemControl::new(peripherals.SYSTEM);

// Returns Option<SPI2>. If the peripheral is already enabled, None is returned.
let spi2 = system.take::<SPI2>::().unwrap();

// Use spi2 in spi driver.

// Disables the peripheral.
system.return(spi2);

The benefit of this is the critical section needed for resetting and enabling peripherals can be eliminated, users can now acquire individual peripherals at runtime which solves the partial move problem with the Peripheral struct and pre-main code can acquire (and use) peripherals before main runs.

Of course, each driver will still need a way to recreate the peripheral object from a split, which should be trivial if we drop the peripheral lending feature and just move things around.