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 <bugadani@gmail.com>
This commit is contained in:
Juraj Sadel 2025-01-23 15:04:09 +01:00 committed by GitHub
parent fc2815b9fc
commit 2bb0a55cd3
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 224 additions and 51 deletions

View File

@ -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)

View File

@ -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),
}
}
}

View File

@ -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))]

View File

@ -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,
}
}

View File

@ -51,10 +51,11 @@ use crate::{
clock::Clocks,
dma::{DmaChannelFor, DmaEligible, DmaRxBuffer, DmaTxBuffer, Rx, Tx},
gpio::{
interconnect::{PeripheralInput, PeripheralOutput},
interconnect::{OutputConnection, PeripheralInput, PeripheralOutput},
InputSignal,
NoPin,
OutputSignal,
PinGuard,
},
interrupt::{InterruptConfigurable, InterruptHandler},
pac::spi2::RegisterBlock,
@ -454,6 +455,17 @@ impl Default for Config {
}
}
#[derive(Debug)]
#[cfg_attr(feature = "defmt", derive(defmt::Format))]
struct SpiPinGuard {
mosi_pin: PinGuard,
sclk_pin: PinGuard,
cs_pin: PinGuard,
sio1_pin: PinGuard,
sio2_pin: Option<PinGuard>,
sio3_pin: Option<PinGuard>,
}
/// Configuration errors.
#[non_exhaustive]
#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
@ -483,6 +495,7 @@ pub struct Spi<'d, Dm> {
spi: PeripheralRef<'d, AnySpi>,
_mode: PhantomData<Dm>,
guard: PeripheralGuard,
pins: SpiPinGuard,
}
impl<Dm: DriverMode> Sealed for Spi<'_, Dm> {}
@ -529,10 +542,25 @@ impl<'d> Spi<'d, Blocking> {
let guard = PeripheralGuard::new(spi.info().peripheral);
let mosi_pin = PinGuard::new_unconnected(spi.info().mosi);
let sclk_pin = PinGuard::new_unconnected(spi.info().sclk);
let cs_pin = PinGuard::new_unconnected(spi.info().cs);
let sio1_pin = PinGuard::new_unconnected(spi.info().sio1_output);
let sio2_pin = spi.info().sio2_output.map(PinGuard::new_unconnected);
let sio3_pin = spi.info().sio3_output.map(PinGuard::new_unconnected);
let mut this = Spi {
spi,
_mode: PhantomData,
guard,
pins: SpiPinGuard {
mosi_pin,
sclk_pin,
cs_pin,
sio1_pin,
sio2_pin,
sio3_pin,
},
};
this.driver().init();
@ -562,6 +590,7 @@ impl<'d> Spi<'d, Blocking> {
spi: self.spi,
_mode: PhantomData,
guard: self.guard,
pins: self.pins,
}
}
@ -608,7 +637,11 @@ impl<'d> Spi<'d, Blocking> {
where
CH: DmaChannelFor<AnySpi>,
{
SpiDma::new(self.spi, channel.map(|ch| ch.degrade()).into_ref())
SpiDma::new(
self.spi,
self.pins,
channel.map(|ch| ch.degrade()).into_ref(),
)
}
#[cfg_attr(
@ -653,6 +686,7 @@ impl<'d> Spi<'d, Async> {
spi: self.spi,
_mode: PhantomData,
guard: self.guard,
pins: self.pins,
}
}
@ -684,15 +718,20 @@ where
{
/// Assign the MOSI (Master Out Slave In) pin for the SPI instance.
///
/// Enables output functionality for the pin, and connects it to the MOSI.
/// Enables output functionality for the pin, and connects it as the MOSI
/// signal. You want to use this for full-duplex SPI or
/// if you intend to use [DataMode::SingleTwoDataLines].
///
/// You want to use this for full-duplex SPI or
/// [DataMode::SingleTwoDataLines]
pub fn with_mosi<MOSI: PeripheralOutput>(self, mosi: impl Peripheral<P = MOSI> + 'd) -> Self {
/// Disconnects the previous pin that was assigned with `with_mosi` or
/// `with_sio0`.
pub fn with_mosi<MOSI: PeripheralOutput>(
mut self,
mosi: impl Peripheral<P = MOSI> + 'd,
) -> Self {
crate::into_mapped_ref!(mosi);
mosi.enable_output(false);
self.driver().info.mosi.connect_to(&mut mosi);
self.pins.mosi_pin = OutputConnection::connect_with_guard(mosi, self.driver().info.mosi);
self
}
@ -702,19 +741,25 @@ where
/// Enables both input and output functionality for the pin, and connects it
/// to the MOSI signal and SIO0 input signal.
///
/// Disconnects the previous pin that was assigned with `with_sio0` or
/// `with_mosi`.
///
/// Use this if any of the devices on the bus use half-duplex SPI.
///
/// The pin is configured to open-drain mode.
///
/// Note: You do not need to call [Self::with_mosi] when this is used.
#[instability::unstable]
pub fn with_sio0<MOSI: PeripheralOutput>(self, mosi: impl Peripheral<P = MOSI> + 'd) -> Self {
pub fn with_sio0<MOSI: PeripheralOutput>(
mut self,
mosi: impl Peripheral<P = MOSI> + 'd,
) -> Self {
crate::into_mapped_ref!(mosi);
mosi.enable_output(true);
mosi.enable_input(true);
self.driver().info.mosi.connect_to(&mut mosi);
self.driver().info.sio0_input.connect_to(&mut mosi);
self.pins.mosi_pin = OutputConnection::connect_with_guard(mosi, self.driver().info.mosi);
self
}
@ -740,49 +785,59 @@ where
/// Enables both input and output functionality for the pin, and connects it
/// to the MISO signal and SIO1 input signal.
///
/// Disconnects the previous pin that was assigned with `with_sio1`.
///
/// Use this if any of the devices on the bus use half-duplex SPI.
///
/// The pin is configured to open-drain mode.
///
/// Note: You do not need to call [Self::with_miso] when this is used.
#[instability::unstable]
pub fn with_sio1<SIO1: PeripheralOutput>(self, miso: impl Peripheral<P = SIO1> + 'd) -> Self {
pub fn with_sio1<SIO1: PeripheralOutput>(
mut self,
miso: impl Peripheral<P = SIO1> + 'd,
) -> Self {
crate::into_mapped_ref!(miso);
miso.enable_input(true);
miso.enable_output(true);
self.driver().info.miso.connect_to(&mut miso);
self.driver().info.sio1_output.connect_to(&mut miso);
self.pins.sio1_pin =
OutputConnection::connect_with_guard(miso, self.driver().info.sio1_output);
self
}
/// Assign the SCK (Serial Clock) pin for the SPI instance.
///
/// Sets the specified pin to push-pull output and connects it to the SPI
/// clock signal.
pub fn with_sck<SCK: PeripheralOutput>(self, sclk: impl Peripheral<P = SCK> + 'd) -> Self {
/// Configures the specified pin to push-pull output and connects it to the
/// SPI clock signal.
///
/// Disconnects the previous pin that was assigned with `with_sck`.
pub fn with_sck<SCK: PeripheralOutput>(mut self, sclk: impl Peripheral<P = SCK> + 'd) -> Self {
crate::into_mapped_ref!(sclk);
sclk.set_to_push_pull_output();
self.driver().info.sclk.connect_to(sclk);
self.pins.sclk_pin = OutputConnection::connect_with_guard(sclk, self.driver().info.sclk);
self
}
/// Assign the CS (Chip Select) pin for the SPI instance.
///
/// Sets the specified pin to push-pull output and connects it to the SPI CS
/// signal.
/// Configures the specified pin to push-pull output and connects it to the
/// SPI CS signal.
///
/// Disconnects the previous pin that was assigned with `with_cs`.
///
/// # Current Stability Limitations
/// The hardware chip select functionality is limited; only one CS line can
/// be set, regardless of the total number available. There is no
/// mechanism to select which CS line to use.
#[instability::unstable]
pub fn with_cs<CS: PeripheralOutput>(self, cs: impl Peripheral<P = CS> + 'd) -> Self {
pub fn with_cs<CS: PeripheralOutput>(mut self, cs: impl Peripheral<P = CS> + 'd) -> Self {
crate::into_mapped_ref!(cs);
cs.set_to_push_pull_output();
self.driver().info.cs.connect_to(cs);
self.pins.cs_pin = OutputConnection::connect_with_guard(cs, self.driver().info.cs);
self
}
@ -820,14 +875,21 @@ where
/// QSPI operations are unstable, associated pins configuration is
/// inefficient.
#[instability::unstable]
pub fn with_sio2<SIO2: PeripheralOutput>(self, sio2: impl Peripheral<P = SIO2> + 'd) -> Self {
pub fn with_sio2<SIO2: PeripheralOutput>(
mut self,
sio2: impl Peripheral<P = SIO2> + 'd,
) -> Self {
// TODO: panic if not QSPI?
crate::into_mapped_ref!(sio2);
sio2.enable_input(true);
sio2.enable_output(true);
unwrap!(self.driver().info.sio2_input).connect_to(&mut sio2);
unwrap!(self.driver().info.sio2_output).connect_to(&mut sio2);
self.pins.sio2_pin = self
.driver()
.info
.sio2_output
.map(|signal| OutputConnection::connect_with_guard(sio2, signal));
self
}
@ -841,14 +903,21 @@ where
/// QSPI operations are unstable, associated pins configuration is
/// inefficient.
#[instability::unstable]
pub fn with_sio3<SIO3: PeripheralOutput>(self, sio3: impl Peripheral<P = SIO3> + 'd) -> Self {
pub fn with_sio3<SIO3: PeripheralOutput>(
mut self,
sio3: impl Peripheral<P = SIO3> + 'd,
) -> Self {
// TODO: panic if not QSPI?
crate::into_mapped_ref!(sio3);
sio3.enable_input(true);
sio3.enable_output(true);
unwrap!(self.driver().info.sio3_input).connect_to(&mut sio3);
unwrap!(self.driver().info.sio3_output).connect_to(&mut sio3);
self.pins.sio3_pin = self
.driver()
.info
.sio3_output
.map(|signal| OutputConnection::connect_with_guard(sio3, signal));
self
}
@ -1025,6 +1094,7 @@ mod dma {
#[cfg(all(esp32, spi_address_workaround))]
address_buffer: DmaTxBuf,
guard: PeripheralGuard,
pins: SpiPinGuard,
}
impl<Dm> crate::private::Sealed for SpiDma<'_, Dm> where Dm: DriverMode {}
@ -1041,6 +1111,7 @@ mod dma {
#[cfg(all(esp32, spi_address_workaround))]
address_buffer: self.address_buffer,
guard: self.guard,
pins: self.pins,
}
}
}
@ -1057,6 +1128,7 @@ mod dma {
#[cfg(all(esp32, spi_address_workaround))]
address_buffer: self.address_buffer,
guard: self.guard,
pins: self.pins,
}
}
}
@ -1117,6 +1189,7 @@ mod dma {
impl<'d> SpiDma<'d, Blocking> {
pub(super) fn new(
spi: PeripheralRef<'d, AnySpi>,
pins: SpiPinGuard,
channel: PeripheralRef<'d, PeripheralDmaChannel<AnySpi>>,
) -> Self {
let channel = Channel::new(channel);
@ -1151,6 +1224,7 @@ mod dma {
tx_transfer_in_progress: false,
rx_transfer_in_progress: false,
guard,
pins,
}
}
}

View File

@ -125,9 +125,10 @@ use crate::{
asynch::AtomicWaker,
clock::Clocks,
gpio::{
interconnect::{PeripheralInput, PeripheralOutput},
interconnect::{OutputConnection, PeripheralInput, PeripheralOutput},
InputSignal,
OutputSignal,
PinGuard,
Pull,
},
interrupt::{InterruptConfigurable, InterruptHandler},
@ -415,6 +416,9 @@ where
let rx_guard = PeripheralGuard::new(self.uart.parts().0.peripheral);
let tx_guard = PeripheralGuard::new(self.uart.parts().0.peripheral);
let rts_pin = PinGuard::new_unconnected(self.uart.info().rts_signal);
let tx_pin = PinGuard::new_unconnected(self.uart.info().tx_signal);
let mut serial = Uart {
rx: UartRx {
uart: unsafe { self.uart.clone_unchecked() },
@ -425,6 +429,8 @@ where
uart: self.uart,
phantom: PhantomData,
guard: tx_guard,
rts_pin,
tx_pin,
},
};
serial.init(config)?;
@ -444,6 +450,8 @@ pub struct UartTx<'d, Dm> {
uart: PeripheralRef<'d, AnyUart>,
phantom: PhantomData<Dm>,
guard: PeripheralGuard,
rts_pin: PinGuard,
tx_pin: PinGuard,
}
/// UART (Receive)
@ -524,10 +532,10 @@ where
Dm: DriverMode,
{
/// Configure RTS pin
pub fn with_rts(self, rts: impl Peripheral<P = impl PeripheralOutput> + 'd) -> Self {
pub fn with_rts(mut self, rts: impl Peripheral<P = impl PeripheralOutput> + 'd) -> Self {
crate::into_mapped_ref!(rts);
rts.set_to_push_pull_output();
self.uart.info().rts_signal.connect_to(rts);
self.rts_pin = OutputConnection::connect_with_guard(rts, self.uart.info().rts_signal);
self
}
@ -536,12 +544,14 @@ where
///
/// Sets the specified pin to push-pull output and connects it to the UART
/// TX signal.
pub fn with_tx(self, tx: impl Peripheral<P = impl PeripheralOutput> + 'd) -> Self {
///
/// Disconnects the previous pin that was assigned with `with_tx`.
pub fn with_tx(mut self, tx: impl Peripheral<P = impl PeripheralOutput> + 'd) -> Self {
crate::into_mapped_ref!(tx);
// Make sure we don't cause an unexpected low pulse on the pin.
tx.set_output_high(true);
tx.set_to_push_pull_output();
self.uart.info().tx_signal.connect_to(tx);
self.tx_pin = OutputConnection::connect_with_guard(tx, self.uart.info().tx_signal);
self
}
@ -659,6 +669,8 @@ impl<'d> UartTx<'d, Blocking> {
uart: self.uart,
phantom: PhantomData,
guard: self.guard,
rts_pin: self.rts_pin,
tx_pin: self.tx_pin,
}
}
}
@ -678,6 +690,8 @@ impl<'d> UartTx<'d, Async> {
uart: self.uart,
phantom: PhantomData,
guard: self.guard,
rts_pin: self.rts_pin,
tx_pin: self.tx_pin,
}
}
}
@ -995,11 +1009,8 @@ impl<'d> Uart<'d, Blocking> {
/// configure the driver side (i.e. the TX pin), or ensure that the line is
/// initially high, to avoid receiving a non-data byte caused by an
/// initial low signal level.
pub fn with_rx(self, rx: impl Peripheral<P = impl PeripheralInput> + 'd) -> Self {
crate::into_mapped_ref!(rx);
rx.init_input(Pull::Up);
self.rx.uart.info().rx_signal.connect_to(rx);
pub fn with_rx(mut self, rx: impl Peripheral<P = impl PeripheralInput> + 'd) -> Self {
self.rx = self.rx.with_rx(rx);
self
}
@ -1007,13 +1018,8 @@ impl<'d> Uart<'d, Blocking> {
///
/// Sets the specified pin to push-pull output and connects it to the UART
/// TX signal.
pub fn with_tx(self, tx: impl Peripheral<P = impl PeripheralOutput> + 'd) -> Self {
crate::into_mapped_ref!(tx);
// Make sure we don't cause an unexpected low pulse on the pin.
tx.set_output_high(true);
tx.set_to_push_pull_output();
self.tx.uart.info().tx_signal.connect_to(tx);
pub fn with_tx(mut self, tx: impl Peripheral<P = impl PeripheralOutput> + 'd) -> Self {
self.tx = self.tx.with_tx(tx);
self
}
}