Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add initial Drop for numerous drivers #2484

Closed
wants to merge 3 commits into from
Closed

Add initial Drop for numerous drivers #2484

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1e6580a
Add initial Drop for numerous drivers
JurajSadel Nov 7, 2024
87c4e8e
Don't set reset in enable() when enable is false
JurajSadel Nov 8, 2024
cde74aa
fmt
JurajSadel Nov 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion esp-hal/src/aes/esp32.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use crate::{

impl<'d> Aes<'d> {
pub(super) fn init(&mut self) {
PeripheralClockControl::enable(PeripheralEnable::Aes);
PeripheralClockControl::enable(PeripheralEnable::Aes, true);
self.write_endianness(
Endianness::BigEndian,
Endianness::BigEndian,
Expand Down
2 changes: 1 addition & 1 deletion esp-hal/src/aes/esp32cX.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use crate::{

impl Aes<'_> {
pub(super) fn init(&mut self) {
PeripheralClockControl::enable(PeripheralEnable::Aes);
PeripheralClockControl::enable(PeripheralEnable::Aes, true);
self.write_dma(false);
}

Expand Down
2 changes: 1 addition & 1 deletion esp-hal/src/aes/esp32s2.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use crate::{

impl<'d> Aes<'d> {
pub(super) fn init(&mut self) {
PeripheralClockControl::enable(PeripheralEnable::Aes);
PeripheralClockControl::enable(PeripheralEnable::Aes, true);
self.write_dma(false);
self.write_endianness(
Endianness::BigEndian,
Expand Down
2 changes: 1 addition & 1 deletion esp-hal/src/aes/esp32s3.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use crate::{

impl<'d> Aes<'d> {
pub(super) fn init(&mut self) {
PeripheralClockControl::enable(PeripheralEnable::Aes);
PeripheralClockControl::enable(PeripheralEnable::Aes, true);
self.write_dma(false);
}

Expand Down
12 changes: 10 additions & 2 deletions esp-hal/src/aes/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,8 @@ use crate::{
peripheral::{Peripheral, PeripheralRef},
peripherals::AES,
reg_access::{AlignmentHelper, NativeEndianess},
system,
system::PeripheralClockControl,
};

#[cfg_attr(esp32, path = "esp32.rs")]
Expand Down Expand Up @@ -143,8 +145,8 @@ impl<'d> Aes<'d> {
pub fn new(aes: impl Peripheral<P = AES> + 'd) -> Self {
crate::into_ref!(aes);

crate::system::PeripheralClockControl::reset(crate::system::Peripheral::Aes);
crate::system::PeripheralClockControl::enable(crate::system::Peripheral::Aes);
PeripheralClockControl::reset(system::Peripheral::Aes);
PeripheralClockControl::enable(system::Peripheral::Aes, true);

let mut ret = Self {
aes,
Expand Down Expand Up @@ -190,6 +192,12 @@ impl<'d> Aes<'d> {
}
}

impl<'d> Drop for Aes<'d> {
fn drop(&mut self) {
PeripheralClockControl::enable(system::Peripheral::Aes, false);
}
}

/// Specifications for AES flavours
pub trait AesFlavour: crate::private::Sealed {
/// Type of the AES key, a fixed-size array of bytes
Expand Down
8 changes: 7 additions & 1 deletion esp-hal/src/analog/adc/riscv.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -409,7 +409,7 @@ where
config: AdcConfig<ADCI>,
) -> Self {
PeripheralClockControl::reset(Peripheral::ApbSarAdc);
PeripheralClockControl::enable(Peripheral::ApbSarAdc);
PeripheralClockControl::enable(Peripheral::ApbSarAdc, true);

unsafe { &*APB_SARADC::PTR }.ctrl().modify(|_, w| unsafe {
w.start_force().set_bit();
Expand Down Expand Up @@ -500,6 +500,12 @@ where
}
}

impl<'d, ADCI> Drop for Adc<'d, ADCI> {
fn drop(&mut self) {
PeripheralClockControl::enable(Peripheral::ApbSarAdc, false);
}
}

#[cfg(any(esp32c2, esp32c3, esp32c6))]
impl super::AdcCalEfuse for crate::peripherals::ADC1 {
fn get_init_code(atten: Attenuation) -> Option<u16> {
Expand Down
8 changes: 7 additions & 1 deletion esp-hal/src/analog/adc/xtensa.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -402,7 +402,7 @@ where
config: AdcConfig<ADCI>,
) -> Self {
PeripheralClockControl::reset(Peripheral::ApbSarAdc);
PeripheralClockControl::enable(Peripheral::ApbSarAdc);
PeripheralClockControl::enable(Peripheral::ApbSarAdc, true);

let sensors = unsafe { &*SENS::ptr() };

Expand Down Expand Up @@ -560,6 +560,12 @@ where
}
}

impl<'d, ADCI> Drop for Adc<'d, ADCI> {
fn drop(&mut self) {
PeripheralClockControl::enable(Peripheral::ApbSarAdc, false);
}
}

#[cfg(esp32s3)]
impl super::AdcCalEfuse for crate::peripherals::ADC1 {
fn get_init_code(atten: Attenuation) -> Option<u16> {
Expand Down
2 changes: 1 addition & 1 deletion esp-hal/src/dma/gdma.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -662,7 +662,7 @@ impl<'d> Dma<'d> {
) -> Dma<'d> {
crate::into_ref!(dma);

PeripheralClockControl::enable(Peripheral::Gdma);
PeripheralClockControl::enable(Peripheral::Gdma, true);
dma.misc_conf().modify(|_, w| w.ahbm_rst_inter().set_bit());
dma.misc_conf()
.modify(|_, w| w.ahbm_rst_inter().clear_bit());
Expand Down
2 changes: 1 addition & 1 deletion esp-hal/src/dma/pdma.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -878,7 +878,7 @@ impl<'d> Dma<'d> {
pub fn new(
dma: impl crate::peripheral::Peripheral<P = crate::peripherals::DMA> + 'd,
) -> Dma<'d> {
PeripheralClockControl::enable(Peripheral::Dma);
PeripheralClockControl::enable(Peripheral::Dma, true);

#[cfg(esp32)]
{
Expand Down
8 changes: 7 additions & 1 deletion esp-hal/src/ecc.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ impl<'d> Ecc<'d, crate::Blocking> {
crate::into_ref!(ecc);

PeripheralClockControl::reset(PeripheralEnable::Ecc);
PeripheralClockControl::enable(PeripheralEnable::Ecc);
PeripheralClockControl::enable(PeripheralEnable::Ecc, true);

Self {
ecc,
Expand All @@ -113,6 +113,12 @@ impl<'d> Ecc<'d, crate::Blocking> {
}
}

impl<'d, DM: crate::Mode> Drop for Ecc<'d, DM> {
fn drop(&mut self) {
PeripheralClockControl::enable(PeripheralEnable::Ecc, false);
}
}

impl crate::private::Sealed for Ecc<'_, crate::Blocking> {}

impl InterruptConfigurable for Ecc<'_, crate::Blocking> {
Expand Down
2 changes: 1 addition & 1 deletion esp-hal/src/etm.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ macro_rules! create_etm {
crate::into_ref!(peripheral);

PeripheralClockControl::reset(crate::system::Peripheral::Etm);
PeripheralClockControl::enable(crate::system::Peripheral::Etm);
PeripheralClockControl::enable(crate::system::Peripheral::Etm, true);

Self {
_peripheral: peripheral,
Expand Down
8 changes: 7 additions & 1 deletion esp-hal/src/hmac.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ impl<'d> Hmac<'d> {
crate::into_ref!(hmac);

PeripheralClockControl::reset(PeripheralEnable::Hmac);
PeripheralClockControl::enable(PeripheralEnable::Hmac);
PeripheralClockControl::enable(PeripheralEnable::Hmac, true);

Self {
hmac,
Expand Down Expand Up @@ -345,3 +345,9 @@ impl<'d> Hmac<'d> {
while self.is_busy() {}
}
}

impl<'d> Drop for Hmac<'d> {
fn drop(&mut self) {
PeripheralClockControl::enable(PeripheralEnable::Hmac, false);
}
}
4 changes: 2 additions & 2 deletions esp-hal/src/i2c/master/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -314,7 +314,7 @@ where
};

PeripheralClockControl::reset(i2c.info().peripheral);
PeripheralClockControl::enable(i2c.info().peripheral);
PeripheralClockControl::enable(i2c.info().peripheral, true);

let i2c = i2c.with_sda(sda).with_scl(scl);

Expand All @@ -328,7 +328,7 @@ where

fn internal_recover(&self) {
PeripheralClockControl::reset(self.info().peripheral);
PeripheralClockControl::enable(self.info().peripheral);
PeripheralClockControl::enable(self.info().peripheral, true);

self.info().setup(self.frequency, self.timeout);
}
Expand Down
2 changes: 1 addition & 1 deletion esp-hal/src/i2s/master.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -289,7 +289,7 @@ where
// the targets the same and force same configuration for both, TX and RX

PeripheralClockControl::reset(i2s.peripheral());
PeripheralClockControl::enable(i2s.peripheral());
PeripheralClockControl::enable(i2s.peripheral(), true);
i2s.set_clock(calculate_clock(sample_rate, 2, data_format.channel_bits()));
i2s.configure(&standard, &data_format);
i2s.set_master();
Expand Down
2 changes: 1 addition & 1 deletion esp-hal/src/i2s/parallel.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,7 @@ where
let channel = channel.degrade();

PeripheralClockControl::reset(i2s.peripheral());
PeripheralClockControl::enable(i2s.peripheral());
PeripheralClockControl::enable(i2s.peripheral(), true);
// configure the I2S peripheral for parallel mode
i2s.setup(frequency, pins.bus_width());
// setup the clock pin
Expand Down
2 changes: 1 addition & 1 deletion esp-hal/src/lcd_cam/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ impl<'d> LcdCam<'d, Blocking> {
crate::into_ref!(lcd_cam);

PeripheralClockControl::reset(system::Peripheral::LcdCam);
PeripheralClockControl::enable(system::Peripheral::LcdCam);
PeripheralClockControl::enable(system::Peripheral::LcdCam, true);

Self {
lcd: Lcd {
Expand Down
8 changes: 7 additions & 1 deletion esp-hal/src/ledc/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ impl<'d> Ledc<'d> {
crate::into_ref!(_instance);

PeripheralClockControl::reset(PeripheralEnable::Ledc);
PeripheralClockControl::enable(PeripheralEnable::Ledc);
PeripheralClockControl::enable(PeripheralEnable::Ledc, true);

let ledc = unsafe { &*crate::peripherals::LEDC::ptr() };
Ledc { _instance, ledc }
Expand Down Expand Up @@ -173,3 +173,9 @@ impl<'d> Ledc<'d> {
Channel::new(number, output_pin)
}
}

impl<'d> Drop for Ledc<'d> {
fn drop(&mut self) {
PeripheralClockControl::enable(PeripheralEnable::Ledc, false);
}
}
12 changes: 6 additions & 6 deletions esp-hal/src/mcpwm/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ impl<'d, PWM: PwmPeripheral> McPwm<'d, PWM> {
crate::into_ref!(peripheral);

PWM::reset();
PWM::enable();
PWM::enable(true);

#[cfg(not(esp32c6))]
{
Expand Down Expand Up @@ -317,7 +317,7 @@ pub struct FrequencyError;
/// A MCPWM peripheral
pub trait PwmPeripheral: Deref<Target = RegisterBlock> + crate::private::Sealed {
/// Enable peripheral
fn enable();
fn enable(enable: bool);
/// Reset peripheral
fn reset();
/// Get a pointer to the peripheral RegisterBlock
Expand All @@ -328,8 +328,8 @@ pub trait PwmPeripheral: Deref<Target = RegisterBlock> + crate::private::Sealed

#[cfg(mcpwm0)]
impl PwmPeripheral for crate::peripherals::MCPWM0 {
fn enable() {
PeripheralClockControl::enable(PeripheralEnable::Mcpwm0)
fn enable(enable: bool) {
PeripheralClockControl::enable(PeripheralEnable::Mcpwm0, enable)
}

fn reset() {
Expand All @@ -355,8 +355,8 @@ impl PwmPeripheral for crate::peripherals::MCPWM0 {

#[cfg(mcpwm1)]
impl PwmPeripheral for crate::peripherals::MCPWM1 {
fn enable() {
PeripheralClockControl::enable(PeripheralEnable::Mcpwm1)
fn enable(enable: bool) {
PeripheralClockControl::enable(PeripheralEnable::Mcpwm1, enable)
}

fn reset() {
Expand Down
8 changes: 7 additions & 1 deletion esp-hal/src/otg_fs.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ impl<'d> Usb<'d> {
M: UsbDm + Send + Sync,
{
PeripheralClockControl::reset(PeripheralEnable::Usb);
PeripheralClockControl::enable(PeripheralEnable::Usb);
PeripheralClockControl::enable(PeripheralEnable::Usb, true);

Self {
_usb0: usb0.into_ref(),
Expand Down Expand Up @@ -110,6 +110,12 @@ impl<'d> Usb<'d> {
}
}

impl<'d> Drop for Usb<'d> {
fn drop(&mut self) {
PeripheralClockControl::enable(PeripheralEnable::Usb, false);
}
}

unsafe impl<'d> Sync for Usb<'d> {}

unsafe impl<'d> UsbPeripheral for Usb<'d> {
Expand Down
2 changes: 1 addition & 1 deletion esp-hal/src/parl_io.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1275,7 +1275,7 @@ fn internal_init(frequency: HertzU32) -> Result<(), Error> {
}

PeripheralClockControl::reset(crate::system::Peripheral::ParlIo);
PeripheralClockControl::enable(crate::system::Peripheral::ParlIo);
PeripheralClockControl::enable(crate::system::Peripheral::ParlIo, true);

let pcr = unsafe { &*crate::peripherals::PCR::PTR };

Expand Down
2 changes: 1 addition & 1 deletion esp-hal/src/pcnt/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ impl<'d> Pcnt<'d> {

// Enable the PCNT peripherals clock in the system peripheral
PeripheralClockControl::reset(crate::system::Peripheral::Pcnt);
PeripheralClockControl::enable(crate::system::Peripheral::Pcnt);
PeripheralClockControl::enable(crate::system::Peripheral::Pcnt, true);

let pcnt = unsafe { &*crate::peripherals::PCNT::ptr() };

Expand Down
17 changes: 16 additions & 1 deletion esp-hal/src/rmt.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ use core::{
use embassy_sync::waitqueue::AtomicWaker;
use enumset::{EnumSet, EnumSetType};
use fugit::HertzU32;
use portable_atomic::{AtomicU8, Ordering::Relaxed};

use crate::{
gpio::interconnect::{PeripheralInput, PeripheralOutput},
Expand All @@ -101,6 +102,7 @@ use crate::{
Blocking,
InterruptConfigurable,
};
static REF_COUNTER: AtomicU8 = AtomicU8::new(0);

/// Errors
#[derive(Debug, Clone, Copy, PartialEq)]
Expand Down Expand Up @@ -230,7 +232,7 @@ where
}

PeripheralClockControl::reset(crate::system::Peripheral::Rmt);
PeripheralClockControl::enable(crate::system::Peripheral::Rmt);
PeripheralClockControl::enable(crate::system::Peripheral::Rmt, true);

cfg_if::cfg_if! {
if #[cfg(any(esp32, esp32s2))] {
Expand Down Expand Up @@ -325,6 +327,8 @@ fn configure_rx_channel<'d, P: PeripheralInput, T: RxChannelInternal<M>, M: crat
T::set_filter_threshold(config.filter_threshold);
T::set_idle_threshold(config.idle_threshold);

REF_COUNTER.fetch_add(1, Relaxed);

Ok(T::new())
}

Expand All @@ -345,6 +349,8 @@ fn configure_tx_channel<'d, P: PeripheralOutput, T: TxChannelInternal<M>, M: cra
);
T::set_idle_output(config.idle_output, config.idle_output_level);

REF_COUNTER.fetch_add(1, Relaxed);

Ok(T::new())
}

Expand Down Expand Up @@ -978,6 +984,15 @@ where
phantom: PhantomData<M>,
}

impl<M: crate::Mode, const CHANNEL: u8> Drop for Channel<M, CHANNEL> {
fn drop(&mut self) {
REF_COUNTER.fetch_sub(1, Relaxed);
if REF_COUNTER.load(Relaxed) == 0 {
Comment on lines +989 to +990
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
REF_COUNTER.fetch_sub(1, Relaxed);
if REF_COUNTER.load(Relaxed) == 0 {
if REF_COUNTER.fetch_sub(1, Relaxed) == 1 {

There's no reason to read one more time.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This Drop also means new_internal should no longer enable clocks. Otherwise it's possible to create Rmt (enables clocks), then drop it (it won't disable clocks because no Channels have been created).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this is a failed try for peripherals that has Channels/Operators etc. We come up with this idea with @bjoernQ but yeah, it's not quite good yet. I just wanted to keep it here (for now) to show what kind of issues we have to solve (or think of).

PeripheralClockControl::enable(crate::system::Peripheral::Rmt, false);
}
}
}

/// Channel in TX mode
pub trait TxChannel: TxChannelInternal<Blocking> {
/// Start transmitting the given pulse code sequence.
Expand Down
2 changes: 1 addition & 1 deletion esp-hal/src/rsa/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ impl<'d, DM: crate::Mode> Rsa<'d, DM> {
crate::into_ref!(rsa);

PeripheralClockControl::reset(PeripheralEnable::Rsa);
PeripheralClockControl::enable(PeripheralEnable::Rsa);
PeripheralClockControl::enable(PeripheralEnable::Rsa, true);

Self {
rsa,
Expand Down
Loading
Loading