From 42054d5aaeb97396e22c71ca372f498610cfe178 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A1niel=20Buga?= Date: Fri, 20 Dec 2024 14:18:05 +0100 Subject: [PATCH 1/3] Remove panicking match from Flex::set_level --- esp-hal/src/gpio/mod.rs | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/esp-hal/src/gpio/mod.rs b/esp-hal/src/gpio/mod.rs index dfed16969b..1d47ee8dd6 100644 --- a/esp-hal/src/gpio/mod.rs +++ b/esp-hal/src/gpio/mod.rs @@ -1029,6 +1029,7 @@ macro_rules! gpio { )* impl $crate::gpio::Pin for $crate::gpio::GpioPin<$gpionum> { + #[inline(always)] fn number(&self) -> u8 { $gpionum } @@ -2037,8 +2038,8 @@ where /// Toggle pin output #[inline] pub fn toggle(&mut self) { - let level = !self.output_level(); - self.set_level(level); + let level = self.output_level(); + self.set_level(!level); } /// Configure the [DriveStrength] of the pin @@ -2124,6 +2125,7 @@ pub(crate) mod internal { } impl Pin for AnyPin { + #[inline(always)] fn number(&self) -> u8 { handle_gpio_input!(&self.0, target, { Pin::number(target) }) } @@ -2191,11 +2193,8 @@ pub(crate) mod internal { }) } - fn set_output_high(&mut self, high: bool, _: private::Internal) { - handle_gpio_output!(&mut self.0, target, { - OutputPin::set_output_high(target, high, private::Internal) - }) - } + // We use the default `set_output_high` implementation to avoid matching on pin + // type. We check the pin type to enable output functionality anyway. fn set_drive_strength(&mut self, strength: DriveStrength, _: private::Internal) { handle_gpio_output!(&mut self.0, target, { @@ -2220,12 +2219,6 @@ pub(crate) mod internal { OutputPin::internal_pull_down_in_sleep_mode(target, on, private::Internal) }) } - - fn is_set_high(&self, _: private::Internal) -> bool { - handle_gpio_output!(&self.0, target, { - OutputPin::is_set_high(target, private::Internal) - }) - } } #[cfg(any(xtensa, esp32c2, esp32c3, esp32c6))] From dd309c625135e1e35b6fe77eefc6668f740dc5f2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A1niel=20Buga?= Date: Fri, 20 Dec 2024 15:40:46 +0100 Subject: [PATCH 2/3] Rework AnyPin to contain a u8 --- esp-hal/CHANGELOG.md | 2 + esp-hal/src/gpio/interconnect.rs | 18 ++-- esp-hal/src/gpio/mod.rs | 173 +++++++++++++++++-------------- esp-hal/src/gpio/placeholder.rs | 4 +- 4 files changed, 111 insertions(+), 86 deletions(-) diff --git a/esp-hal/CHANGELOG.md b/esp-hal/CHANGELOG.md index 0fce97d4ae..a0ef7b434c 100644 --- a/esp-hal/CHANGELOG.md +++ b/esp-hal/CHANGELOG.md @@ -53,6 +53,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - The `ExtU64` and `RateExtU32` traits have been added to `esp_hal::time` (#2845) +- Added `AnyPin::steal(pin_number)` (#2854) + ### Changed - Bump MSRV to 1.83 (#2615) diff --git a/esp-hal/src/gpio/interconnect.rs b/esp-hal/src/gpio/interconnect.rs index 887ed71ae8..07a61e3f8e 100644 --- a/esp-hal/src/gpio/interconnect.rs +++ b/esp-hal/src/gpio/interconnect.rs @@ -301,7 +301,7 @@ impl InputSignal { #[doc(hidden)] to self.pin { pub fn pull_direction(&self, pull: Pull, _internal: private::Internal); - pub fn input_signals(&self, _internal: private::Internal) -> &[(AlternateFunction, gpio::InputSignal)]; + pub fn input_signals(&self, _internal: private::Internal) -> &'static [(AlternateFunction, gpio::InputSignal)]; pub fn init_input(&self, pull: Pull, _internal: private::Internal); pub fn is_input_high(&self, _internal: private::Internal) -> bool; pub fn enable_input(&mut self, on: bool, _internal: private::Internal); @@ -339,7 +339,7 @@ impl DirectInputSignal { delegate::delegate! { to self.pin { fn pull_direction(&self, pull: Pull, _internal: private::Internal); - fn input_signals(&self, _internal: private::Internal) -> &[(AlternateFunction, gpio::InputSignal)]; + fn input_signals(&self, _internal: private::Internal) -> &'static [(AlternateFunction, gpio::InputSignal)]; fn init_input(&self, pull: Pull, _internal: private::Internal); fn is_input_high(&self, _internal: private::Internal) -> bool; fn enable_input(&mut self, on: bool, _internal: private::Internal); @@ -433,12 +433,12 @@ impl OutputSignal { #[doc(hidden)] to self.pin { pub fn pull_direction(&self, pull: Pull, _internal: private::Internal); - pub fn input_signals(&self, _internal: private::Internal) -> &[(AlternateFunction, gpio::InputSignal)]; + pub fn input_signals(&self, _internal: private::Internal) -> &'static [(AlternateFunction, gpio::InputSignal)]; pub fn init_input(&self, pull: Pull, _internal: private::Internal); pub fn is_input_high(&self, _internal: private::Internal) -> bool; pub fn enable_input(&mut self, on: bool, _internal: private::Internal); - pub fn output_signals(&self, _internal: private::Internal) -> &[(AlternateFunction, gpio::OutputSignal)]; + pub fn output_signals(&self, _internal: private::Internal) -> &'static [(AlternateFunction, gpio::OutputSignal)]; pub fn set_to_open_drain_output(&mut self, _internal: private::Internal); pub fn set_to_push_pull_output(&mut self, _internal: private::Internal); pub fn enable_output(&mut self, on: bool, _internal: private::Internal); @@ -481,12 +481,12 @@ impl DirectOutputSignal { delegate::delegate! { to self.pin { fn pull_direction(&self, pull: Pull, _internal: private::Internal); - fn input_signals(&self, _internal: private::Internal) -> &[(AlternateFunction, gpio::InputSignal)]; + fn input_signals(&self, _internal: private::Internal) -> &'static [(AlternateFunction, gpio::InputSignal)]; fn init_input(&self, pull: Pull, _internal: private::Internal); fn is_input_high(&self, _internal: private::Internal) -> bool; fn enable_input(&mut self, on: bool, _internal: private::Internal); - fn output_signals(&self, _internal: private::Internal) -> &[(AlternateFunction, gpio::OutputSignal)]; + fn output_signals(&self, _internal: private::Internal) -> &'static [(AlternateFunction, gpio::OutputSignal)]; fn set_to_open_drain_output(&mut self, _internal: private::Internal); fn set_to_push_pull_output(&mut self, _internal: private::Internal); fn enable_output(&mut self, on: bool, _internal: private::Internal); @@ -598,7 +598,7 @@ impl InputConnection { pub fn pull_direction(&self, pull: Pull, _internal: private::Internal); pub fn init_input(&self, pull: Pull, _internal: private::Internal); pub fn is_input_high(&self, _internal: private::Internal) -> bool; - pub fn input_signals(&self, _internal: private::Internal) -> &[(AlternateFunction, gpio::InputSignal)]; + pub fn input_signals(&self, _internal: private::Internal) -> &'static [(AlternateFunction, gpio::InputSignal)]; } #[doc(hidden)] @@ -692,10 +692,10 @@ impl OutputConnection { OutputConnectionInner::Constant(level) => level, } { pub fn is_input_high(&self, _internal: private::Internal) -> bool; - pub fn input_signals(&self, _internal: private::Internal) -> &[(AlternateFunction, gpio::InputSignal)]; + pub fn input_signals(&self, _internal: private::Internal) -> &'static [(AlternateFunction, gpio::InputSignal)]; pub fn is_set_high(&self, _internal: private::Internal) -> bool; - pub fn output_signals(&self, _internal: private::Internal) -> &[(AlternateFunction, gpio::OutputSignal)]; + pub fn output_signals(&self, _internal: private::Internal) -> &'static [(AlternateFunction, gpio::OutputSignal)]; } #[doc(hidden)] to match &mut self.0 { diff --git a/esp-hal/src/gpio/mod.rs b/esp-hal/src/gpio/mod.rs index 1d47ee8dd6..f9c16706bc 100644 --- a/esp-hal/src/gpio/mod.rs +++ b/esp-hal/src/gpio/mod.rs @@ -72,7 +72,7 @@ use crate::{ }, peripheral::{Peripheral, PeripheralRef}, peripherals::{ - gpio::{handle_gpio_input, handle_gpio_output, AnyPinInner}, + gpio::{handle_gpio_input, handle_gpio_output}, Interrupt, GPIO, IO_MUX, @@ -386,10 +386,10 @@ pub trait Pin: Sealed { } #[doc(hidden)] - fn output_signals(&self, _: private::Internal) -> &[(AlternateFunction, OutputSignal)]; + fn output_signals(&self, _: private::Internal) -> &'static [(AlternateFunction, OutputSignal)]; #[doc(hidden)] - fn input_signals(&self, _: private::Internal) -> &[(AlternateFunction, InputSignal)]; + fn input_signals(&self, _: private::Internal) -> &'static [(AlternateFunction, InputSignal)]; #[doc(hidden)] fn pull_direction(&self, pull: Pull, _: private::Internal) { @@ -761,7 +761,7 @@ pub struct GpioPin; /// Type-erased GPIO pin #[derive(Debug)] #[cfg_attr(feature = "defmt", derive(defmt::Format))] -pub struct AnyPin(pub(crate) AnyPinInner); +pub struct AnyPin(pub(crate) u8); impl GpioPin where @@ -1035,10 +1035,10 @@ macro_rules! gpio { } fn degrade_pin(&self, _: $crate::private::Internal) -> $crate::gpio::AnyPin { - $crate::gpio::AnyPin(AnyPinInner::[< Gpio $gpionum >](unsafe { Self::steal() })) + $crate::gpio::AnyPin($gpionum) } - fn output_signals(&self, _: $crate::private::Internal) -> &[($crate::gpio::AlternateFunction, $crate::gpio::OutputSignal)] { + fn output_signals(&self, _: $crate::private::Internal) -> &'static [($crate::gpio::AlternateFunction, $crate::gpio::OutputSignal)] { &[ $( $( @@ -1051,7 +1051,7 @@ macro_rules! gpio { ] } - fn input_signals(&self, _: $crate::private::Internal) -> &[($crate::gpio::AlternateFunction, $crate::gpio::InputSignal)] { + fn input_signals(&self, _: $crate::private::Internal) -> &'static [($crate::gpio::AlternateFunction, $crate::gpio::InputSignal)] { &[ $( $( @@ -1072,22 +1072,27 @@ macro_rules! gpio { } )+ - #[derive(Debug)] - #[cfg_attr(feature = "defmt", derive(defmt::Format))] - pub(crate) enum AnyPinInner { - $( - []($crate::gpio::GpioPin<$gpionum>), - )+ - } - impl $crate::peripheral::Peripheral for $crate::gpio::AnyPin { type P = $crate::gpio::AnyPin; unsafe fn clone_unchecked(&self) -> Self { - match self.0 { - $(AnyPinInner::[](_) => { - Self(AnyPinInner::[< Gpio $gpionum >](unsafe { $crate::gpio::GpioPin::steal() })) - })+ - } + Self(self.0) + } + } + + impl $crate::gpio::AnyPin { + /// Conjure a new, type-erased GPIO pin out of thin air. + /// + /// # Safety + /// + /// The caller must ensure that only one instance of a pin is in use at one time. + /// + /// # Panics + /// + /// Panics if the pin with the given number does not exist. + pub unsafe fn steal(pin: u8) -> Self { + const PINS: &[u8] = &[$($gpionum),*]; + assert!(PINS.contains(&pin), "Pin {} does not exist", pin); + Self(pin) } } @@ -1096,15 +1101,17 @@ macro_rules! gpio { #[doc(hidden)] macro_rules! handle_gpio_output { ($this:expr, $inner:ident, $code:tt) => { - match $this { + match $this.number() { $( - AnyPinInner::[]($inner) => $crate::if_output_pin!($($type),* { + $gpionum => $crate::if_output_pin!($($type),* {{ + #[allow(unused_mut)] + let mut $inner = unsafe { GpioPin::<$gpionum>::steal() }; $code - } else {{ - let _ = $inner; + }} else {{ panic!("Unsupported") }}), )+ + _ => unreachable!(), } } } @@ -1112,10 +1119,15 @@ macro_rules! gpio { #[doc(hidden)] macro_rules! handle_gpio_input { ($this:expr, $inner:ident, $code:tt) => { - match $this { + match $this.number() { $( - AnyPinInner::[]($inner) => $code + $gpionum => {{ + #[allow(unused_mut)] + let mut $inner = unsafe { GpioPin::<$gpionum>::steal() }; + $code + }}, )+ + _ => unreachable!(), } } } @@ -1128,15 +1140,17 @@ macro_rules! gpio { #[doc(hidden)] macro_rules! handle_rtcio { ($this:expr, $inner:ident, $code:tt) => { - match $this { + match $this.number() { $( - AnyPinInner::[]($inner) => $crate::if_rtcio_pin!($($type),* { + $gpionum => $crate::if_rtcio_pin!($($type),* {{ + #[allow(unused_mut)] + let mut $inner = unsafe { GpioPin::<$gpionum>::steal() }; $code - } else {{ - let _ = $inner; + }} else {{ panic!("Unsupported") }}), )+ + _ => unreachable!(), } } } @@ -1144,20 +1158,21 @@ macro_rules! gpio { #[doc(hidden)] macro_rules! handle_rtcio_with_resistors { ($this:expr, $inner:ident, $code:tt) => { - match $this { + match $this.number() { $( - AnyPinInner::[]($inner) => $crate::if_rtcio_pin!($($type),* { - $crate::if_output_pin!($($type),* { + $gpionum => $crate::if_rtcio_pin!($($type),* { + $crate::if_output_pin!($($type),* {{ + #[allow(unused_mut)] + let mut $inner = unsafe { GpioPin::<$gpionum>::steal() }; $code - } else {{ - let _ = $inner; + }} else {{ panic!("Unsupported") }}) } else {{ - let _ = $inner; panic!("Unsupported") }}), )+ + _ => unreachable!(), } } } @@ -1995,6 +2010,7 @@ where { /// Set the GPIO to output mode. #[instability::unstable] + #[inline] pub fn set_as_output(&mut self) { self.pin.set_to_push_pull_output(private::Internal); } @@ -2081,8 +2097,8 @@ impl Pin for Flex<'_, P> { to self.pin { fn number(&self) -> u8; fn degrade_pin(&self, _internal: private::Internal) -> AnyPin; - fn output_signals(&self, _internal: private::Internal) -> &[(AlternateFunction, OutputSignal)]; - fn input_signals(&self, _internal: private::Internal) -> &[(AlternateFunction, InputSignal)]; + fn output_signals(&self, _internal: private::Internal) -> &'static [(AlternateFunction, OutputSignal)]; + fn input_signals(&self, _internal: private::Internal) -> &'static [(AlternateFunction, InputSignal)]; } } } @@ -2119,15 +2135,16 @@ pub(crate) mod internal { /// Peripheral signals allow connecting peripherals together without /// using external hardware. #[inline] + #[allow(unused_braces, reason = "False positive")] pub fn split(self) -> (interconnect::InputSignal, interconnect::OutputSignal) { - handle_gpio_input!(self.0, target, { target.split() }) + handle_gpio_input!(self, target, { target.split() }) } } impl Pin for AnyPin { #[inline(always)] fn number(&self) -> u8 { - handle_gpio_input!(&self.0, target, { Pin::number(target) }) + self.0 } fn degrade_pin(&self, _: private::Internal) -> AnyPin { @@ -2135,26 +2152,32 @@ pub(crate) mod internal { } fn sleep_mode(&mut self, on: bool, _: private::Internal) { - handle_gpio_input!(&mut self.0, target, { - Pin::sleep_mode(target, on, private::Internal) + handle_gpio_input!(self, target, { + Pin::sleep_mode(&mut target, on, private::Internal) }) } fn set_alternate_function(&mut self, alternate: AlternateFunction, _: private::Internal) { - handle_gpio_input!(&mut self.0, target, { - Pin::set_alternate_function(target, alternate, private::Internal) + handle_gpio_input!(self, target, { + Pin::set_alternate_function(&mut target, alternate, private::Internal) }) } - fn output_signals(&self, _: private::Internal) -> &[(AlternateFunction, OutputSignal)] { - handle_gpio_output!(&self.0, target, { - Pin::output_signals(target, private::Internal) + fn output_signals( + &self, + _: private::Internal, + ) -> &'static [(AlternateFunction, OutputSignal)] { + handle_gpio_output!(self, target, { + Pin::output_signals(&target, private::Internal) }) } - fn input_signals(&self, _: private::Internal) -> &[(AlternateFunction, InputSignal)] { - handle_gpio_input!(&self.0, target, { - Pin::input_signals(target, private::Internal) + fn input_signals( + &self, + _: private::Internal, + ) -> &'static [(AlternateFunction, InputSignal)] { + handle_gpio_input!(self, target, { + Pin::input_signals(&target, private::Internal) }) } } @@ -2170,9 +2193,9 @@ pub(crate) mod internal { input_enable: Option, _: private::Internal, ) { - handle_gpio_output!(&mut self.0, target, { + handle_gpio_output!(self, target, { OutputPin::init_output( - target, + &mut target, alternate, open_drain, input_enable, @@ -2182,14 +2205,14 @@ pub(crate) mod internal { } fn set_to_open_drain_output(&mut self, _: private::Internal) { - handle_gpio_output!(&mut self.0, target, { - OutputPin::set_to_open_drain_output(target, private::Internal) + handle_gpio_output!(self, target, { + OutputPin::set_to_open_drain_output(&mut target, private::Internal) }) } fn set_to_push_pull_output(&mut self, _: private::Internal) { - handle_gpio_output!(&mut self.0, target, { - OutputPin::set_to_push_pull_output(target, private::Internal) + handle_gpio_output!(self, target, { + OutputPin::set_to_push_pull_output(&mut target, private::Internal) }) } @@ -2197,26 +2220,26 @@ pub(crate) mod internal { // type. We check the pin type to enable output functionality anyway. fn set_drive_strength(&mut self, strength: DriveStrength, _: private::Internal) { - handle_gpio_output!(&mut self.0, target, { - OutputPin::set_drive_strength(target, strength, private::Internal) + handle_gpio_output!(self, target, { + OutputPin::set_drive_strength(&mut target, strength, private::Internal) }) } fn enable_open_drain(&mut self, on: bool, _: private::Internal) { - handle_gpio_output!(&mut self.0, target, { - OutputPin::enable_open_drain(target, on, private::Internal) + handle_gpio_output!(self, target, { + OutputPin::enable_open_drain(&mut target, on, private::Internal) }) } fn internal_pull_up_in_sleep_mode(&mut self, on: bool, _: private::Internal) { - handle_gpio_output!(&mut self.0, target, { - OutputPin::internal_pull_up_in_sleep_mode(target, on, private::Internal) + handle_gpio_output!(self, target, { + OutputPin::internal_pull_up_in_sleep_mode(&mut target, on, private::Internal) }) } fn internal_pull_down_in_sleep_mode(&mut self, on: bool, _: private::Internal) { - handle_gpio_output!(&mut self.0, target, { - OutputPin::internal_pull_down_in_sleep_mode(target, on, private::Internal) + handle_gpio_output!(self, target, { + OutputPin::internal_pull_down_in_sleep_mode(&mut target, on, private::Internal) }) } } @@ -2226,26 +2249,26 @@ pub(crate) mod internal { #[cfg(xtensa)] #[allow(unused_braces)] // False positive :/ fn rtc_number(&self) -> u8 { - handle_rtcio!(&self.0, target, { RtcPin::rtc_number(target) }) + handle_rtcio!(self, target, { RtcPin::rtc_number(&target) }) } #[cfg(any(xtensa, esp32c6))] fn rtc_set_config(&mut self, input_enable: bool, mux: bool, func: RtcFunction) { - handle_rtcio!(&mut self.0, target, { - RtcPin::rtc_set_config(target, input_enable, mux, func) + handle_rtcio!(self, target, { + RtcPin::rtc_set_config(&mut target, input_enable, mux, func) }) } fn rtcio_pad_hold(&mut self, enable: bool) { - handle_rtcio!(&mut self.0, target, { - RtcPin::rtcio_pad_hold(target, enable) + handle_rtcio!(self, target, { + RtcPin::rtcio_pad_hold(&mut target, enable) }) } #[cfg(any(esp32c2, esp32c3, esp32c6))] unsafe fn apply_wakeup(&mut self, wakeup: bool, level: u8) { - handle_rtcio!(&mut self.0, target, { - RtcPin::apply_wakeup(target, wakeup, level) + handle_rtcio!(self, target, { + RtcPin::apply_wakeup(&mut target, wakeup, level) }) } } @@ -2253,14 +2276,14 @@ pub(crate) mod internal { #[cfg(any(esp32c2, esp32c3, esp32c6, xtensa))] impl RtcPinWithResistors for AnyPin { fn rtcio_pullup(&mut self, enable: bool) { - handle_rtcio_with_resistors!(&mut self.0, target, { - RtcPinWithResistors::rtcio_pullup(target, enable) + handle_rtcio_with_resistors!(self, target, { + RtcPinWithResistors::rtcio_pullup(&mut target, enable) }) } fn rtcio_pulldown(&mut self, enable: bool) { - handle_rtcio_with_resistors!(&mut self.0, target, { - RtcPinWithResistors::rtcio_pulldown(target, enable) + handle_rtcio_with_resistors!(self, target, { + RtcPinWithResistors::rtcio_pulldown(&mut target, enable) }) } } diff --git a/esp-hal/src/gpio/placeholder.rs b/esp-hal/src/gpio/placeholder.rs index 8f4b12f2a1..2be2933bda 100644 --- a/esp-hal/src/gpio/placeholder.rs +++ b/esp-hal/src/gpio/placeholder.rs @@ -22,7 +22,7 @@ impl Level { pub(crate) fn input_signals( &self, _: private::Internal, - ) -> &[(AlternateFunction, InputSignal)] { + ) -> &'static [(AlternateFunction, InputSignal)] { &[] } @@ -60,7 +60,7 @@ impl Level { pub(crate) fn output_signals( &self, _: private::Internal, - ) -> &[(AlternateFunction, OutputSignal)] { + ) -> &'static [(AlternateFunction, OutputSignal)] { &[] } From c9dbb96268bfa8147a0ffa2f331af3df0b76a3e1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A1niel=20Buga?= Date: Fri, 20 Dec 2024 16:07:25 +0100 Subject: [PATCH 3/3] SPI --- esp-hal/src/spi/master.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/esp-hal/src/spi/master.rs b/esp-hal/src/spi/master.rs index d2783a2744..4c7ec6d786 100644 --- a/esp-hal/src/spi/master.rs +++ b/esp-hal/src/spi/master.rs @@ -3231,7 +3231,7 @@ impl Driver { // wait } } else if #[cfg(esp32)] { - xtensa_lx::timer::delay(1); + xtensa_lx::timer::delay(2); // ☠️ } else { // Doesn't seem to be needed for ESP32-S2 }