Simplify traits and erratum 36 workaround, add Pins::steal (#2335)

* Simplify erratum 36

* Add Pins::steal

* Fix typo

* Move pin operations into Flex

* Cleanup braces

* Avoid panicking when handling erratum

* Add disable_usb_pad to other devices

* Touch up changelog

* Fix changelog

esp-hal/CHANGELOG.md

@@ -11,8 +11,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 - A new config option `PLACE_SWITCH_TABLES_IN_RAM` to improve performance (especially for interrupts) at the cost of slightly more RAM usage (#2331)
 - A new config option `PLACE_ANON_IN_RAM` to improve performance (especially for interrupts) at the cost of RAM usage (#2331)
-- Add burst transfer support to DMA buffers (#2236)
+- Add burst transfer support to DMA buffers (#2336)
 - `AnyPin` now implements `From<GpioPin<N>>`. (#2326)
+- `Pins::steal()` to unsafely obtain GPIO. (#2335)
 
 ### Changed
 

esp-hal/src/gpio/mod.rs

@@ -448,75 +448,7 @@ pub trait PeripheralOutput: PeripheralSignal {
 }
 
 /// Trait implemented by pins which can be used as inputs.
-pub trait InputPin: Pin + PeripheralInput + Into<AnyPin> + 'static {
+pub trait InputPin: Pin + PeripheralInput + Into<AnyPin> + 'static {}
-    /// Listen for interrupts
-    #[doc(hidden)]
-    fn listen(&mut self, event: Event, _: private::Internal) {
-        self.listen_with_options(event, true, false, false, private::Internal)
-    }
-
-    /// Checks if listening for interrupts is enabled for this Pin
-    #[doc(hidden)]
-    fn is_listening(&self, _: private::Internal) -> bool {
-        is_listening(self.number())
-    }
-
-    /// Listen for interrupts
-    #[doc(hidden)]
-    fn listen_with_options(
-        &mut self,
-        event: Event,
-        int_enable: bool,
-        nmi_enable: bool,
-        wake_up_from_light_sleep: bool,
-        _: private::Internal,
-    ) {
-        if wake_up_from_light_sleep {
-            match event {
-                Event::AnyEdge | Event::RisingEdge | Event::FallingEdge => {
-                    panic!("Edge triggering is not supported for wake-up from light sleep");
-                }
-                _ => {}
-            }
-        }
-
-        set_int_enable(
-            self.number(),
-            gpio_intr_enable(int_enable, nmi_enable),
-            event as u8,
-            wake_up_from_light_sleep,
-        )
-    }
-
-    /// Stop listening for interrupts
-    #[doc(hidden)]
-    fn unlisten(&mut self, _: private::Internal) {
-        unsafe {
-            (*GPIO::PTR)
-                .pin(self.number() as usize)
-                .modify(|_, w| w.int_ena().bits(0).int_type().bits(0).int_ena().bits(0));
-        }
-    }
-
-    /// Checks if the interrupt status bit for this Pin is set
-    #[doc(hidden)]
-    fn is_interrupt_set(&self, _: private::Internal) -> bool {
-        self.gpio_bank(private::Internal).read_interrupt_status() & 1 << (self.number() % 32) != 0
-    }
-
-    /// Clear the interrupt status bit for this Pin
-    #[doc(hidden)]
-    fn clear_interrupt(&mut self, _: private::Internal) {
-        self.gpio_bank(private::Internal)
-            .write_interrupt_status_clear(1 << (self.number() % 32));
-    }
-
-    /// Enable this pin as a wake up source
-    #[doc(hidden)]
-    fn wakeup_enable(&mut self, enable: bool, event: WakeEvent, _: private::Internal) {
-        self.listen_with_options(event.into(), false, false, enable, private::Internal);
-    }
-}
 
 /// Trait implemented by pins which can be used as outputs.
 pub trait OutputPin: Pin + PeripheralOutput + Into<AnyPin> + 'static {}
@@ -722,17 +654,13 @@ impl<const GPIONUM: u8> GpioPin<GPIONUM>
 where
     Self: Pin,
 {
-    pub(crate) fn new() -> Self {
-        Self
-    }
-
     /// Create a pin out of thin air.
     ///
     /// # Safety
     ///
     /// Ensure that only one instance of a pin exists at one time.
     pub unsafe fn steal() -> Self {
-        Self::new()
+        Self
     }
 
     /// Returns a peripheral [input][interconnect::InputSignal] connected to
@@ -747,13 +675,19 @@ where
 
 /// Workaround to make D+ and D- work on the ESP32-C3 and ESP32-S3, which by
 /// default are assigned to the `USB_SERIAL_JTAG` peripheral.
-#[cfg(any(esp32c3, esp32s3))]
+#[cfg(usb_device)]
 fn disable_usb_pads(gpionum: u8) {
     cfg_if::cfg_if! {
         if #[cfg(esp32c3)] {
             let pins = [18, 19];
+        } else if #[cfg(esp32c6)] {
+            let pins = [12, 13];
+        } else if #[cfg(esp32h2)] {
+            let pins = [26, 27];
         } else if #[cfg(esp32s3)] {
             let pins = [19, 20];
+        } else {
+            compile_error!("Please define USB pins for this chip");
         }
     }
 
@@ -792,7 +726,7 @@ where
         let pull_down = pull == Pull::Down;
 
         #[cfg(esp32)]
-        crate::soc::gpio::errata36(GPIONUM, Some(pull_up), Some(pull_down));
+        crate::soc::gpio::errata36(self.degrade_pin(private::Internal), pull_up, pull_down);
 
         get_io_mux_reg(GPIONUM).modify(|_, w| {
             w.fun_wpd().bit(pull_down);
@@ -808,7 +742,7 @@ where
     fn init_input(&self, pull: Pull, _: private::Internal) {
         self.pull_direction(pull, private::Internal);
 
-        #[cfg(any(esp32c3, esp32s3))]
+        #[cfg(usb_device)]
         disable_usb_pads(GPIONUM);
 
         get_io_mux_reg(GPIONUM).modify(|_, w| unsafe {
@@ -982,10 +916,10 @@ impl Io {
     /// *Note:* You probably don't want to use this, it is intended to be used
     /// in very specific use cases. Async GPIO functionality will not work
     /// when instantiating `Io` using this constructor.
-    pub fn new_no_bind_interrupt(gpio: GPIO, _io_mux: IO_MUX) -> Self {
+    pub fn new_no_bind_interrupt(_gpio: GPIO, _io_mux: IO_MUX) -> Self {
         Io {
             _io_mux,
-            pins: gpio.pins(),
+            pins: unsafe { Pins::steal() },
         }
     }
 }
@@ -1080,11 +1014,16 @@ macro_rules! gpio {
                 )+
             }
 
-            impl GPIO {
+            impl Pins {
-                pub(crate) fn pins(self) -> Pins {
+                /// Unsafely create GPIO pins.
-                    Pins {
+                ///
+                /// # Safety
+                ///
+                /// The caller must ensure that only one instance of a pin is in use at one time.
+                pub unsafe fn steal() -> Self {
+                    Self {
                         $(
-                            [< gpio $gpionum >]: GpioPin::new(),
+                            [< gpio $gpionum >]: GpioPin::steal(),
                         )+
                     }
                 }
@@ -1255,7 +1194,7 @@ macro_rules! rtc_pins {
                     let rtcio = unsafe { &*$crate::peripherals::RTC_IO::PTR };
 
                     paste::paste! {
-                        rtcio.$pin_reg.modify(|_, w| w.[< $prefix rue >]().bit([< enable >]));
+                        rtcio.$pin_reg.modify(|_, w| w.[< $prefix rue >]().bit(enable));
                     }
                 }
 
@@ -1263,7 +1202,7 @@ macro_rules! rtc_pins {
                     let rtcio = unsafe { &*$crate::peripherals::RTC_IO::PTR };
 
                     paste::paste! {
-                        rtcio.$pin_reg.modify(|_, w| w.[< $prefix rde >]().bit([< enable >]));
+                        rtcio.$pin_reg.modify(|_, w| w.[< $prefix rde >]().bit(enable));
                     }
                 }
             }
@@ -1277,6 +1216,23 @@ macro_rules! rtc_pins {
             $crate::gpio::rtc_pins!($pin_num, $rtc_pin, $pin_reg, $prefix, $hold $(, $rue )?);
         )+
 
+        #[cfg(esp32)]
+        pub(crate) fn errata36(mut pin: AnyPin, pull_up: bool, pull_down: bool) {
+            use $crate::gpio::{Pin, RtcPinWithResistors};
+
+            let has_pullups = match pin.number() {
+                $(
+                    $( $pin_num => $rue, )?
+                )+
+                _ => false,
+            };
+
+            if has_pullups {
+                pin.rtcio_pullup(pull_up);
+                pin.rtcio_pulldown(pull_down);
+            }
+        }
+
         #[doc(hidden)]
         #[macro_export]
         macro_rules! handle_rtcio {
@@ -1474,7 +1430,7 @@ macro_rules! analog {
             impl $crate::gpio::AnalogPin for GpioPin<$pin_num> {
                 /// Configures the pin for analog mode.
                 fn set_analog(&self, _: $crate::private::Internal) {
-                    use $crate::peripherals::{GPIO};
+                    use $crate::peripherals::GPIO;
 
                     get_io_mux_reg($pin_num).modify(|_,w| unsafe {
                         w.mcu_sel().bits(1);
@@ -1986,27 +1942,57 @@ where
         self.pin.is_input_high(private::Internal).into()
     }
 
+    fn listen_with_options(
+        &self,
+        event: Event,
+        int_enable: bool,
+        nmi_enable: bool,
+        wake_up_from_light_sleep: bool,
+    ) {
+        if wake_up_from_light_sleep {
+            match event {
+                Event::AnyEdge | Event::RisingEdge | Event::FallingEdge => {
+                    panic!("Edge triggering is not supported for wake-up from light sleep");
+                }
+                _ => {}
+            }
+        }
+
+        set_int_enable(
+            self.pin.number(),
+            gpio_intr_enable(int_enable, nmi_enable),
+            event as u8,
+            wake_up_from_light_sleep,
+        )
+    }
+
     /// Listen for interrupts
     #[inline]
     pub fn listen(&mut self, event: Event) {
-        self.pin.listen(event, private::Internal);
+        self.listen_with_options(event, true, false, false)
     }
 
     /// Stop listening for interrupts
     pub fn unlisten(&mut self) {
-        self.pin.unlisten(private::Internal);
+        set_int_enable(self.pin.number(), 0, 0, false);
     }
 
     /// Clear the interrupt status bit for this Pin
     #[inline]
     pub fn clear_interrupt(&mut self) {
-        self.pin.clear_interrupt(private::Internal);
+        self.pin
+            .gpio_bank(private::Internal)
+            .write_interrupt_status_clear(1 << (self.pin.number() % 32));
     }
 
     /// Checks if the interrupt status bit for this Pin is set
     #[inline]
     pub fn is_interrupt_set(&self) -> bool {
-        self.pin.is_interrupt_set(private::Internal)
+        self.pin
+            .gpio_bank(private::Internal)
+            .read_interrupt_status()
+            & 1 << (self.pin.number() % 32)
+            != 0
     }
 
     /// Enable as a wake-up source.
@@ -2014,7 +2000,7 @@ where
     /// This will unlisten for interrupts
     #[inline]
     pub fn wakeup_enable(&mut self, enable: bool, event: WakeEvent) {
-        self.pin.wakeup_enable(enable, event, private::Internal);
+        self.listen_with_options(event.into(), false, false, enable);
     }
 
     /// Returns a peripheral [input][interconnect::InputSignal] connected to
@@ -2230,51 +2216,7 @@ pub(crate) mod internal {
         }
     }
 
-    impl InputPin for AnyPin {
+    impl InputPin for AnyPin {}
-        fn listen_with_options(
-            &mut self,
-            event: Event,
-            int_enable: bool,
-            nmi_enable: bool,
-            wake_up_from_light_sleep: bool,
-            _: private::Internal,
-        ) {
-            handle_gpio_input!(&mut self.0, target, {
-                InputPin::listen_with_options(
-                    target,
-                    event,
-                    int_enable,
-                    nmi_enable,
-                    wake_up_from_light_sleep,
-                    private::Internal,
-                )
-            })
-        }
-
-        fn unlisten(&mut self, _: private::Internal) {
-            handle_gpio_input!(&mut self.0, target, {
-                InputPin::unlisten(target, private::Internal)
-            })
-        }
-
-        fn is_interrupt_set(&self, _: private::Internal) -> bool {
-            handle_gpio_input!(&self.0, target, {
-                InputPin::is_interrupt_set(target, private::Internal)
-            })
-        }
-
-        fn clear_interrupt(&mut self, _: private::Internal) {
-            handle_gpio_input!(&mut self.0, target, {
-                InputPin::clear_interrupt(target, private::Internal)
-            })
-        }
-
-        fn listen(&mut self, event: Event, _: private::Internal) {
-            handle_gpio_input!(&mut self.0, target, {
-                InputPin::listen(target, event, private::Internal)
-            })
-        }
-    }
 
     impl PeripheralOutput for AnyPin {
         fn set_to_open_drain_output(&mut self, _: private::Internal) {
@@ -2415,7 +2357,7 @@ fn is_listening(pin_num: u8) -> bool {
 }
 
 fn set_int_enable(gpio_num: u8, int_ena: u8, int_type: u8, wake_up_from_light_sleep: bool) {
-    let gpio = unsafe { &*crate::peripherals::GPIO::PTR };
+    let gpio = unsafe { &*GPIO::PTR };
     gpio.pin(gpio_num as usize).modify(|_, w| unsafe {
         w.int_ena().bits(int_ena);
         w.int_type().bits(int_type);

esp-hal/src/rtc_cntl/sleep/esp32c6.rs

@@ -4,7 +4,6 @@ use crate::{
     clock::Clock,
     efuse::Efuse,
     gpio::{Pins, RtcFunction},
-    peripherals::Peripherals,
     rtc_cntl::{
         rtc::{
             rtc_clk_cpu_freq_set_xtal,
@@ -900,13 +899,12 @@ impl RtcSleepConfig {
     fn wake_io_reset() {
         // loosely based on esp_deep_sleep_wakeup_io_reset
 
-        let peripherals = unsafe {
+        let mut pins = unsafe {
-            // We're stealing peripherals to do some uninitialization after waking up from
+            // We're stealing pins to do some uninitialization after waking up from
             // deep sleep. We have to be careful to only touch settings that were enabled
             // by deep sleep setup.
-            Peripherals::steal()
+            Pins::steal()
         };
-        let mut pins = peripherals.GPIO.pins();
 
         Ext1WakeupSource::wake_io_reset(&mut pins);
     }

esp-hal/src/soc/esp32/gpio.rs

@@ -527,147 +527,6 @@ pub enum OutputSignal {
     MTDO,
 }
 
-pub(crate) fn errata36(pin_num: u8, pull_up: Option<bool>, pull_down: Option<bool>) {
-    use crate::peripherals::RTC_IO;
-    let rtcio = unsafe { &*RTC_IO::PTR };
-
-    match pin_num {
-        0 => {
-            rtcio.touch_pad1().modify(|_, w| {
-                if let Some(pull_up) = pull_up {
-                    w.rue().bit(pull_up);
-                }
-                if let Some(pull_down) = pull_down {
-                    w.rde().bit(pull_down);
-                }
-                w
-            });
-        }
-        2 => {
-            rtcio.touch_pad2().modify(|_, w| {
-                if let Some(pull_up) = pull_up {
-                    w.rue().bit(pull_up);
-                }
-                if let Some(pull_down) = pull_down {
-                    w.rde().bit(pull_down);
-                }
-                w
-            });
-        }
-        4 => {
-            rtcio.touch_pad0().modify(|_, w| {
-                if let Some(pull_up) = pull_up {
-                    w.rue().bit(pull_up);
-                }
-                if let Some(pull_down) = pull_down {
-                    w.rde().bit(pull_down);
-                }
-                w
-            });
-        }
-        12 => {
-            rtcio.touch_pad5().modify(|_, w| {
-                if let Some(pull_up) = pull_up {
-                    w.rue().bit(pull_up);
-                }
-                if let Some(pull_down) = pull_down {
-                    w.rde().bit(pull_down);
-                }
-                w
-            });
-        }
-        13 => {
-            rtcio.touch_pad4().modify(|_, w| {
-                if let Some(pull_up) = pull_up {
-                    w.rue().bit(pull_up);
-                }
-                if let Some(pull_down) = pull_down {
-                    w.rde().bit(pull_down);
-                }
-                w
-            });
-        }
-        14 => {
-            rtcio.touch_pad6().modify(|_, w| {
-                if let Some(pull_up) = pull_up {
-                    w.rue().bit(pull_up);
-                }
-                if let Some(pull_down) = pull_down {
-                    w.rde().bit(pull_down);
-                }
-                w
-            });
-        }
-        15 => {
-            rtcio.touch_pad3().modify(|_, w| {
-                if let Some(pull_up) = pull_up {
-                    w.rue().bit(pull_up);
-                }
-                if let Some(pull_down) = pull_down {
-                    w.rde().bit(pull_down);
-                }
-                w
-            });
-        }
-        25 => {
-            rtcio.pad_dac1().modify(|_, w| {
-                if let Some(pull_up) = pull_up {
-                    w.rue().bit(pull_up);
-                }
-                if let Some(pull_down) = pull_down {
-                    w.rde().bit(pull_down);
-                }
-                w
-            });
-        }
-        26 => {
-            rtcio.pad_dac2().modify(|_, w| {
-                if let Some(pull_up) = pull_up {
-                    w.rue().bit(pull_up);
-                }
-                if let Some(pull_down) = pull_down {
-                    w.rde().bit(pull_down);
-                }
-                w
-            });
-        }
-        27 => {
-            rtcio.touch_pad7().modify(|_, w| {
-                if let Some(pull_up) = pull_up {
-                    w.rue().bit(pull_up);
-                }
-                if let Some(pull_down) = pull_down {
-                    w.rde().bit(pull_down);
-                }
-                w
-            });
-        }
-        32 => {
-            rtcio.xtal_32k_pad().modify(|_, w| {
-                if let Some(pull_up) = pull_up {
-                    w.x32p_rue().bit(pull_up);
-                }
-                if let Some(pull_down) = pull_down {
-                    w.x32p_rde().bit(pull_down);
-                }
-                w
-            });
-        }
-        33 => {
-            rtcio.xtal_32k_pad().modify(|_, w| {
-                if let Some(pull_up) = pull_up {
-                    w.x32n_rue().bit(pull_up);
-                }
-                if let Some(pull_down) = pull_down {
-                    w.x32n_rde().bit(pull_down);
-                }
-                w
-            });
-        }
-        _ => (),
-    }
-}
-
 crate::gpio::gpio! {
     (0, 0, InputOutputAnalogTouch (5 => EMAC_TX_CLK) (1 => CLK_OUT1))
     (1, 0, InputOutput (5 => EMAC_RXD2) (0 => U0TXD 1 => CLK_OUT3))

hil-test/tests/gpio.rs

@@ -69,7 +69,7 @@ mod tests {
     }
 
     #[test]
-    async fn test_async_edge(ctx: Context) {
+    async fn async_edge(ctx: Context) {
         let counter = AtomicUsize::new(0);
         let Context {
             test_gpio1,
@@ -99,7 +99,7 @@ mod tests {
     }
 
     #[test]
-    async fn test_a_pin_can_wait(ctx: Context) {
+    async fn a_pin_can_wait(ctx: Context) {
         let mut first = Input::new(ctx.test_gpio1, Pull::Down);
 
         embassy_futures::select::select(
@@ -112,7 +112,7 @@ mod tests {
     }
 
     #[test]
-    fn test_gpio_input(ctx: Context) {
+    fn gpio_input(ctx: Context) {
         let test_gpio1 = Input::new(ctx.test_gpio1, Pull::Down);
         // `InputPin`:
         assert_eq!(test_gpio1.is_low(), true);
@@ -120,7 +120,7 @@ mod tests {
     }
 
     #[test]
-    fn test_gpio_output(ctx: Context) {
+    fn gpio_output(ctx: Context) {
         let mut test_gpio2 = Output::new(ctx.test_gpio2, Level::Low);
 
         // `StatefulOutputPin`:
@@ -140,7 +140,7 @@ mod tests {
     }
 
     #[test]
-    fn test_gpio_output_embedded_hal_0_2(ctx: Context) {
+    fn gpio_output_embedded_hal_0_2(ctx: Context) {
         let test_gpio1 = Input::new(ctx.test_gpio1, Pull::Down);
         let mut test_gpio2 = Output::new(ctx.test_gpio2, Level::Low);
 
@@ -187,7 +187,7 @@ mod tests {
     }
 
     #[test]
-    fn test_gpio_output_embedded_hal_1_0(ctx: Context) {
+    fn gpio_output_embedded_hal_1_0(ctx: Context) {
         let test_gpio1 = Input::new(ctx.test_gpio1, Pull::Down);
         let mut test_gpio2 = Output::new(ctx.test_gpio2, Level::Low);
 
@@ -234,7 +234,7 @@ mod tests {
     }
 
     #[test]
-    fn test_gpio_interrupt(ctx: Context) {
+    fn gpio_interrupt(ctx: Context) {
         let mut test_gpio1 = Input::new(ctx.test_gpio1, Pull::Down);
         let mut test_gpio2 = Output::new(ctx.test_gpio2, Level::Low);
 
@@ -271,7 +271,7 @@ mod tests {
     }
 
     #[test]
-    fn test_gpio_od(ctx: Context) {
+    fn gpio_od(ctx: Context) {
         let mut test_gpio1 = OutputOpenDrain::new(ctx.test_gpio1, Level::High, Pull::Up);
         let mut test_gpio2 = OutputOpenDrain::new(ctx.test_gpio2, Level::High, Pull::Up);
 
@@ -317,7 +317,7 @@ mod tests {
     }
 
     #[test]
-    fn test_gpio_flex(ctx: Context) {
+    fn gpio_flex(ctx: Context) {
         let mut test_gpio1 = Flex::new(ctx.test_gpio1);
         let mut test_gpio2 = Flex::new(ctx.test_gpio2);
 
@@ -359,7 +359,7 @@ mod tests {
     // Tests touch pin (GPIO2) as AnyPin and Output
     // https://github.com/esp-rs/esp-hal/issues/1943
     #[test]
-    fn test_gpio_touch_anypin_output(ctx: Context) {
+    fn gpio_touch_anypin_output(ctx: Context) {
         let any_pin2 = ctx.test_gpio1;
         let any_pin3 = ctx.test_gpio2;
 
@@ -373,7 +373,7 @@ mod tests {
     // Tests touch pin (GPIO2) as AnyPin and Input
     // https://github.com/esp-rs/esp-hal/issues/1943
     #[test]
-    fn test_gpio_touch_anypin_input(ctx: Context) {
+    fn gpio_touch_anypin_input(ctx: Context) {
         let any_pin2 = ctx.test_gpio1;
         let any_pin3 = ctx.test_gpio2;
 
@@ -383,4 +383,12 @@ mod tests {
         assert_eq!(out_pin.is_set_high(), false);
         assert_eq!(in_pin.is_high(), false);
     }
+
+    #[cfg(esp32)]
+    #[test]
+    fn can_configure_rtcio_pins_as_input() {
+        let pins = unsafe { esp_hal::gpio::Pins::steal() };
+
+        _ = Input::new(pins.gpio37, Pull::Down);
+    }
 }