Introduce PinGuard and reconnecting an output signal to a different pin now clears previous connections (#3012)

* wip

* Fix and clean up

* Introduce PinGuard and reconnecting an output signal to a different pin clears previous connections

* Add demft for PinGuard

* Keep pins around in SpiDma

* Simplify i2c

* Changelog

* Clean up

* Don't use PeripheralRef directly

* Handle RTS pin

---------

Co-authored-by: Dániel Buga <[email protected]>

Author: JurajSadel

esp-hal/CHANGELOG.md

@@ -23,6 +23,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - `timer::wait` is now blocking (#2882)
 - By default, set `tx_idle_num` to 0 so that bytes written to TX FIFO are always immediately transmitted. (#2859)
 - `Rng` and `Trng` now implement `Peripheral<P = Self>` (#2992)
+- SPI, UART, I2C: `with_<pin>` functions of peripheral drivers now disconnect the previously assigned pins from the peripheral. (#3012)
+- SPI, UART, I2C: Dropping a driver now disconnects pins from their peripherals. (#3012)
+
 - `Async` drivers are no longer `Send` (#2980)
 - GPIO drivers now take configuration structs, and their constructors are fallible (#2990)
 

esp-hal/src/gpio/interconnect.rs

@@ -13,6 +13,7 @@ use crate::{
         OutputPin,
         OutputSignalType,
         Pin,
+        PinGuard,
         Pull,
         FUNC_IN_SEL_OFFSET,
         GPIO_FUNCTION,
@@ -106,7 +107,6 @@ impl gpio::OutputSignal {
     pub fn connect_to(self, pin: impl Peripheral<P = impl PeripheralOutput>) {
         crate::into_mapped_ref!(pin);
 
-        // FIXME: disconnect previous connection(s)
         pin.connect_peripheral_to_output(self);
     }
 
@@ -718,4 +718,20 @@ impl OutputConnection {
             fn disconnect_from_peripheral_output(&mut self, signal: gpio::OutputSignal);
         }
     }
+
+    pub(crate) fn connect_with_guard(
+        this: impl Peripheral<P = impl PeripheralOutput>,
+        signal: crate::gpio::OutputSignal,
+    ) -> PinGuard {
+        crate::into_mapped_ref!(this);
+        match &this.0 {
+            OutputConnectionInner::Output(pin) => {
+                PinGuard::new(unsafe { pin.pin.clone_unchecked() }, signal)
+            }
+            OutputConnectionInner::DirectOutput(pin) => {
+                PinGuard::new(unsafe { pin.pin.clone_unchecked() }, signal)
+            }
+            OutputConnectionInner::Constant(_) => PinGuard::new_unconnected(signal),
+        }
+    }
 }

esp-hal/src/gpio/mod.rs

@@ -112,6 +112,56 @@ impl CFnPtr {
     }
 }
 
+/// Represents a pin-peripheral connection that, when dropped, disconnects the
+/// peripheral from the pin.
+///
+/// This only needs to be applied to output signals, as it's not possible to
+/// connect multiple inputs to the same peripheral signal.
+#[derive(Debug)]
+#[cfg_attr(feature = "defmt", derive(defmt::Format))]
+pub(crate) struct PinGuard {
+    pin: u8,
+    signal: OutputSignal,
+}
+
+impl crate::private::Sealed for PinGuard {}
+impl Peripheral for PinGuard {
+    type P = Self;
+
+    unsafe fn clone_unchecked(&self) -> Self::P {
+        Self {
+            pin: self.pin,
+            signal: self.signal,
+        }
+    }
+}
+
+impl PinGuard {
+    pub(crate) fn new(mut pin: AnyPin, signal: OutputSignal) -> Self {
+        signal.connect_to(&mut pin);
+        Self {
+            pin: pin.number(),
+            signal,
+        }
+    }
+
+    pub(crate) fn new_unconnected(signal: OutputSignal) -> Self {
+        Self {
+            pin: u8::MAX,
+            signal,
+        }
+    }
+}
+
+impl Drop for PinGuard {
+    fn drop(&mut self) {
+        if self.pin != u8::MAX {
+            let mut pin = unsafe { AnyPin::steal(self.pin) };
+            self.signal.disconnect_from(&mut pin);
+        }
+    }
+}
+
 /// Event used to trigger interrupts.
 #[derive(Debug, Eq, PartialEq, Copy, Clone, Hash)]
 #[cfg_attr(feature = "defmt", derive(defmt::Format))]

esp-hal/src/i2c/master/mod.rs

@@ -36,7 +36,13 @@ use fugit::HertzU32;
 use crate::{
     asynch::AtomicWaker,
     clock::Clocks,
-    gpio::{interconnect::PeripheralOutput, InputSignal, OutputSignal, Pull},
+    gpio::{
+        interconnect::{OutputConnection, PeripheralOutput},
+        InputSignal,
+        OutputSignal,
+        PinGuard,
+        Pull,
+    },
     interrupt::{InterruptConfigurable, InterruptHandler},
     pac::i2c0::{RegisterBlock, COMD},
     peripheral::{Peripheral, PeripheralRef},
@@ -439,6 +445,8 @@ pub struct I2c<'d, Dm: DriverMode> {
     phantom: PhantomData<Dm>,
     config: Config,
     guard: PeripheralGuard,
+    sda_pin: PinGuard,
+    scl_pin: PinGuard,
 }
 
 #[cfg(any(doc, feature = "unstable"))]
@@ -542,27 +550,35 @@ impl<'d, Dm: DriverMode> I2c<'d, Dm> {
     }
 
     /// Connect a pin to the I2C SDA signal.
-    pub fn with_sda(self, sda: impl Peripheral<P = impl PeripheralOutput> + 'd) -> Self {
+    ///
+    /// This will replace previous pin assignments for this signal.
+    pub fn with_sda(mut self, sda: impl Peripheral<P = impl PeripheralOutput> + 'd) -> Self {
         let info = self.driver().info;
         let input = info.sda_input;
         let output = info.sda_output;
-        self.with_pin(sda, input, output)
+        Self::connect_pin(sda, input, output, &mut self.sda_pin);
+
+        self
     }
 
     /// Connect a pin to the I2C SCL signal.
-    pub fn with_scl(self, scl: impl Peripheral<P = impl PeripheralOutput> + 'd) -> Self {
+    ///
+    /// This will replace previous pin assignments for this signal.
+    pub fn with_scl(mut self, scl: impl Peripheral<P = impl PeripheralOutput> + 'd) -> Self {
         let info = self.driver().info;
         let input = info.scl_input;
         let output = info.scl_output;
-        self.with_pin(scl, input, output)
+        Self::connect_pin(scl, input, output, &mut self.scl_pin);
+
+        self
     }
 
-    fn with_pin(
-        self,
+    fn connect_pin(
         pin: impl Peripheral<P = impl PeripheralOutput> + 'd,
         input: InputSignal,
         output: OutputSignal,
-    ) -> Self {
+        guard: &mut PinGuard,
+    ) {
         crate::into_mapped_ref!(pin);
         // avoid the pin going low during configuration
         pin.set_output_high(true);
@@ -572,9 +588,8 @@ impl<'d, Dm: DriverMode> I2c<'d, Dm> {
         pin.pull_direction(Pull::Up);
 
         input.connect_to(&mut pin);
-        output.connect_to(&mut pin);
 
-        self
+        *guard = OutputConnection::connect_with_guard(pin, output);
     }
 }
 
@@ -588,11 +603,16 @@ impl<'d> I2c<'d, Blocking> {
 
         let guard = PeripheralGuard::new(i2c.info().peripheral);
 
+        let sda_pin = PinGuard::new_unconnected(i2c.info().sda_output);
+        let scl_pin = PinGuard::new_unconnected(i2c.info().scl_output);
+
         let i2c = I2c {
             i2c,
             phantom: PhantomData,
             config,
             guard,
+            sda_pin,
+            scl_pin,
         };
 
         i2c.driver().setup(&i2c.config)?;
@@ -652,6 +672,8 @@ impl<'d> I2c<'d, Blocking> {
             phantom: PhantomData,
             config: self.config,
             guard: self.guard,
+            sda_pin: self.sda_pin,
+            scl_pin: self.scl_pin,
         }
     }
 
@@ -904,6 +926,8 @@ impl<'d> I2c<'d, Async> {
             phantom: PhantomData,
             config: self.config,
             guard: self.guard,
+            sda_pin: self.sda_pin,
+            scl_pin: self.scl_pin,
         }
     }