Merge #335

335: Fix `Pwm::set_output_pin` soundness, add `swap_output_pin` r=jonas-schievink a=jonas-schievink

Fixes #320 by taking the pin by-value (cc @Dirbaio)

To allow the same flexibility as before, this adds `swap_output_pin`, which returns the old pin (though I haven't tested this API).

Co-authored-by: Jonas Schievink <[email protected]>

Author: bors[bot]

examples/pwm-blinky-demo/src/main.rs

@@ -3,10 +3,10 @@
 
 use hal::{gpio, prelude::*, pwm, pwm::Pwm, timer, timer::Timer};
 use nb::block;
-#[cfg(feature = "52840")]
-use nrf52840_hal as hal;
 #[cfg(feature = "52832")]
 use nrf52832_hal as hal;
+#[cfg(feature = "52840")]
+use nrf52840_hal as hal;
 #[cfg(feature = "9160")]
 use nrf9160_hal as hal;
 use rtt_target::{rprintln, rtt_init_print};
@@ -29,7 +29,7 @@ fn main() -> ! {
     pwm.set_period(500u32.hz());
 
     rprintln!("PWM Blinky demo starting");
-    
+
     let wait_time = 1_000_000u32 / pwm.get_max_duty() as u32;
     loop {
         for duty in 0..pwm.get_max_duty() {
@@ -46,7 +46,7 @@ fn init_device(p: hal::pac::Peripherals) -> (Pwm<hal::pac::PWM0_NS>, Timer<hal::
     let pwm = Pwm::new(p.PWM0_NS);
     pwm.set_output_pin(
         pwm::Channel::C0,
-        &p0.p0_02.into_push_pull_output(gpio::Level::High).degrade(),
+        p0.p0_02.into_push_pull_output(gpio::Level::High).degrade(),
     );
 
     let timer = Timer::new(p.TIMER0_NS);
@@ -61,7 +61,7 @@ fn init_device(p: hal::pac::Peripherals) -> (Pwm<hal::pac::PWM0>, Timer<hal::pac
     let pwm = Pwm::new(p.PWM0);
     pwm.set_output_pin(
         pwm::Channel::C0,
-        &p0.p0_13.into_push_pull_output(gpio::Level::High).degrade(),
+        p0.p0_13.into_push_pull_output(gpio::Level::High).degrade(),
     );
 
     let timer = Timer::new(p.TIMER0);
@@ -76,15 +76,14 @@ fn init_device(p: hal::pac::Peripherals) -> (Pwm<hal::pac::PWM0>, Timer<hal::pac
     let pwm = Pwm::new(p.PWM0);
     pwm.set_output_pin(
         pwm::Channel::C0,
-        &p0.p0_30.into_push_pull_output(gpio::Level::High).degrade(),
+        p0.p0_30.into_push_pull_output(gpio::Level::High).degrade(),
     );
 
     let timer = Timer::new(p.TIMER0);
 
     (pwm, timer)
 }
 
-
 fn delay<T>(timer: &mut Timer<T>, cycles: u32)
 where
     T: timer::Instance,

examples/pwm-demo/src/main.rs

@@ -67,14 +67,20 @@ const APP: () = {
         let led3 = p0.p0_15.into_push_pull_output(Level::High).degrade();
         let led4 = p0.p0_16.into_push_pull_output(Level::High).degrade();
 
-        let pwm = Pwm::new(ctx.device.PWM0);
+        let mut pwm = Pwm::new(ctx.device.PWM0);
         pwm.set_period(500u32.hz())
-            .set_output_pin(Channel::C0, &led1)
-            .set_output_pin(Channel::C1, &led2)
-            .set_output_pin(Channel::C2, &led3)
-            .set_output_pin(Channel::C3, &led4)
-            .enable_interrupt(PwmEvent::Stopped)
-            .enable();
+            .set_output_pin(Channel::C0, led1)
+            .set_output_pin(Channel::C1, led2)
+            .set_output_pin(Channel::C2, led3)
+            .set_output_pin(Channel::C3, led4)
+            .enable_interrupt(PwmEvent::Stopped);
+
+        // In addition to `set_output_pin`, `swap_output_pin` and `clear_output_pin` can be used to
+        // get the old pin back.
+        let led1 = pwm.clear_output_pin(Channel::C0).unwrap();
+        assert!(pwm.swap_output_pin(Channel::C0, led1).is_none());
+
+        pwm.enable();
 
         let gpiote = Gpiote::new(ctx.device.GPIOTE);
         gpiote.port().input_pin(&btn1).low();

nrf-hal-common/src/gpio.rs

@@ -50,7 +50,9 @@ pub enum Port {
 // ===============================================================
 /// Generic $PX pin
 pub struct Pin<MODE> {
-    // Bit 7: Port, Bits 0-6: Pin
+    /// 00AB BBBB
+    /// A: Port
+    /// B: Pin
     pin_port: u8,
     _mode: PhantomData<MODE>,
 }

nrf-hal-common/src/pwm.rs

@@ -129,16 +129,56 @@ where
         }
     }
 
-    /// Sets the associated output pin for the PWM channel and enables it.
+    /// Sets the associated output pin for the PWM channel.
+    ///
+    /// Modifying the pin configuration while the PWM instance is enabled is not recommended.
     #[inline(always)]
-    pub fn set_output_pin(&self, channel: Channel, pin: &Pin<Output<PushPull>>) -> &Self {
+    pub fn set_output_pin(&self, channel: Channel, pin: Pin<Output<PushPull>>) -> &Self {
         self.pwm.psel.out[usize::from(channel)].write(|w| {
             unsafe { w.bits(pin.psel_bits()) };
             w.connect().connected()
         });
         self
     }
 
+    /// Sets the output pin of `channel`, and returns the old pin (if any).
+    ///
+    /// Modifying the pin configuration while the PWM instance is enabled is not recommended.
+    pub fn swap_output_pin(
+        &mut self,
+        channel: Channel,
+        pin: Pin<Output<PushPull>>,
+    ) -> Option<Pin<Output<PushPull>>> {
+        // (needs `&mut self` because it reads, then writes, to the register)
+        let psel = &self.pwm.psel.out[usize::from(channel)];
+        let old = psel.read();
+        let old = if old.connect().is_connected() {
+            unsafe { Some(Pin::from_psel_bits(old.bits())) }
+        } else {
+            None
+        };
+        self.set_output_pin(channel, pin);
+        old
+    }
+
+    /// Disables the output pin of `channel`.
+    ///
+    /// The output pin is returned, if one was previously configured.
+    ///
+    /// Modifying the pin configuration while the PWM instance is enabled is not recommended.
+    pub fn clear_output_pin(&mut self, channel: Channel) -> Option<Pin<Output<PushPull>>> {
+        // (needs `&mut self` because it reads, then writes, to the register)
+        let psel = &self.pwm.psel.out[usize::from(channel)];
+        let old = psel.read();
+        let old = if old.connect().is_connected() {
+            unsafe { Some(Pin::from_psel_bits(old.bits())) }
+        } else {
+            None
+        };
+        psel.reset();
+        old
+    }
+
     /// Enables the PWM generator.
     #[inline(always)]
     pub fn enable(&self) {
@@ -228,7 +268,7 @@ where
         self
     }
 
-    /// Returns selected operating mode of the wave counter.    
+    /// Returns selected operating mode of the wave counter.
     #[inline(always)]
     pub fn counter_mode(&self) -> CounterMode {
         match self.pwm.mode.read().updown().bit() {
@@ -351,7 +391,7 @@ where
         self.start_seq(Seq::Seq0);
     }
 
-    /// Returns duty cycle value for a PWM group.    
+    /// Returns duty cycle value for a PWM group.
     #[inline(always)]
     pub fn duty_on_group(&self, group: Group) -> u16 {
         self.duty_on_value(usize::from(group))
@@ -395,7 +435,7 @@ where
         self.start_seq(Seq::Seq0);
     }
 
-    /// Returns the duty cycle value for a PWM channel.        
+    /// Returns the duty cycle value for a PWM channel.
     #[inline(always)]
     pub fn duty_on(&self, channel: Channel) -> u16 {
         self.duty_on_value(usize::from(channel))