Merge pull request #2198 from adamgreig/stm32-opamp

STM32: Don't enable opamps in new(), wait until configured

embassy-stm32/src/opamp.rs

@@ -31,12 +31,9 @@ impl From<OpAmpSpeed> for crate::pac::opamp::vals::OpampCsrOpahsm {
 
 /// OpAmp external outputs, wired to a GPIO pad.
 ///
-/// The GPIO output pad is held by this struct to ensure it cannot be used elsewhere.
-///
 /// This struct can also be used as an ADC input.
-pub struct OpAmpOutput<'d, 'p, T: Instance, P: OutputPin<T>> {
+pub struct OpAmpOutput<'d, T: Instance> {
     _inner: &'d OpAmp<'d, T>,
-    _output: &'p mut P,
 }
 
 /// OpAmp internal outputs, wired directly to ADC inputs.
@@ -54,19 +51,12 @@ pub struct OpAmp<'d, T: Instance> {
 impl<'d, T: Instance> OpAmp<'d, T> {
     /// Create a new driver instance.
     ///
-    /// Enables the OpAmp and configures the speed, but
+    /// Does not enable the opamp, but does set the speed mode on some families.
-    /// does not set any other configuration.
     pub fn new(opamp: impl Peripheral<P = T> + 'd, #[cfg(opamp_g4)] speed: OpAmpSpeed) -> Self {
         into_ref!(opamp);
 
-        #[cfg(opamp_f3)]
-        T::regs().opampcsr().modify(|w| {
-            w.set_opampen(true);
-        });
-
         #[cfg(opamp_g4)]
         T::regs().opamp_csr().modify(|w| {
-            w.set_opaen(true);
             w.set_opahsm(speed.into());
         });
 
@@ -74,24 +64,23 @@ impl<'d, T: Instance> OpAmp<'d, T> {
     }
 
     /// Configure the OpAmp as a buffer for the provided input pin,
-    /// outputting to the provided output pin.
+    /// outputting to the provided output pin, and enable the opamp.
     ///
     /// The input pin is configured for analogue mode but not consumed,
     /// so it may subsequently be used for ADC or comparator inputs.
     ///
     /// The output pin is held within the returned [`OpAmpOutput`] struct,
     /// preventing it being used elsewhere. The `OpAmpOutput` can then be
-    /// directly used as an ADC input.
+    /// directly used as an ADC input. The opamp will be disabled when the
-    pub fn buffer_ext<'a, 'b, IP, OP>(
+    /// [`OpAmpOutput`] is dropped.
-        &'a mut self,
+    pub fn buffer_ext(
-        in_pin: &IP,
+        &'d mut self,
-        out_pin: &'b mut OP,
+        in_pin: impl Peripheral<P = impl NonInvertingPin<T> + crate::gpio::sealed::Pin>,
+        out_pin: impl Peripheral<P = impl OutputPin<T> + crate::gpio::sealed::Pin> + 'd,
         gain: OpAmpGain,
-    ) -> OpAmpOutput<'a, 'b, T, OP>
+    ) -> OpAmpOutput<'d, T> {
-    where
+        into_ref!(in_pin);
-        IP: NonInvertingPin<T> + crate::gpio::sealed::Pin,
+        into_ref!(out_pin);
-        OP: OutputPin<T> + crate::gpio::sealed::Pin,
-    {
         in_pin.set_as_analog();
         out_pin.set_as_analog();
 
@@ -122,24 +111,24 @@ impl<'d, T: Instance> OpAmp<'d, T> {
             w.set_opaen(true);
         });
 
-        OpAmpOutput {
+        OpAmpOutput { _inner: self }
-            _inner: self,
-            _output: out_pin,
-        }
     }
 
     /// Configure the OpAmp as a buffer for the provided input pin,
-    /// with the output only used internally.
+    /// with the output only used internally, and enable the opamp.
     ///
     /// The input pin is configured for analogue mode but not consumed,
     /// so it may be subsequently used for ADC or comparator inputs.
     ///
     /// The returned `OpAmpInternalOutput` struct may be used as an ADC input.
+    /// The opamp output will be disabled when it is dropped.
     #[cfg(opamp_g4)]
-    pub fn buffer_int<'a, P>(&'a mut self, pin: &P, gain: OpAmpGain) -> OpAmpInternalOutput<'a, T>
+    pub fn buffer_int(
-    where
+        &'d mut self,
-        P: NonInvertingPin<T> + crate::gpio::sealed::Pin,
+        pin: impl Peripheral<P = impl NonInvertingPin<T> + crate::gpio::sealed::Pin>,
-    {
+        gain: OpAmpGain,
+    ) -> OpAmpInternalOutput<'d, T> {
+        into_ref!(pin);
         pin.set_as_analog();
 
         let (vm_sel, pga_gain) = match gain {
@@ -163,7 +152,21 @@ impl<'d, T: Instance> OpAmp<'d, T> {
     }
 }
 
-impl<'d, T: Instance> Drop for OpAmp<'d, T> {
+impl<'d, T: Instance> Drop for OpAmpOutput<'d, T> {
+    fn drop(&mut self) {
+        #[cfg(opamp_f3)]
+        T::regs().opampcsr().modify(|w| {
+            w.set_opampen(false);
+        });
+
+        #[cfg(opamp_g4)]
+        T::regs().opamp_csr().modify(|w| {
+            w.set_opaen(false);
+        });
+    }
+}
+
+impl<'d, T: Instance> Drop for OpAmpInternalOutput<'d, T> {
     fn drop(&mut self) {
         #[cfg(opamp_f3)]
         T::regs().opampcsr().modify(|w| {
@@ -203,16 +206,16 @@ macro_rules! impl_opamp_external_output {
     ($inst:ident, $adc:ident, $ch:expr) => {
         foreach_adc!(
             ($adc, $common_inst:ident, $adc_clock:ident) => {
-                impl<'d, 'p, P: OutputPin<crate::peripherals::$inst>> crate::adc::sealed::AdcPin<crate::peripherals::$adc>
+                impl<'d> crate::adc::sealed::AdcPin<crate::peripherals::$adc>
-                    for OpAmpOutput<'d, 'p, crate::peripherals::$inst, P>
+                    for OpAmpOutput<'d, crate::peripherals::$inst>
                 {
                     fn channel(&self) -> u8 {
                         $ch
                     }
                 }
 
-                impl<'d, 'p, P: OutputPin<crate::peripherals::$inst>> crate::adc::AdcPin<crate::peripherals::$adc>
+                impl<'d> crate::adc::AdcPin<crate::peripherals::$adc>
-                    for OpAmpOutput<'d, 'p, crate::peripherals::$inst, P>
+                    for OpAmpOutput<'d, crate::peripherals::$inst>
                 {
                 }
             };

examples/stm32f334/src/bin/opamp.rs

@@ -39,7 +39,7 @@ async fn main(_spawner: Spawner) -> ! {
 
     let mut vrefint = adc.enable_vref(&mut Delay);
     let mut temperature = adc.enable_temperature();
-    let mut buffer = opamp.buffer_ext(&p.PA7, &mut p.PA6, OpAmpGain::Mul1);
+    let mut buffer = opamp.buffer_ext(&mut p.PA7, &mut p.PA6, OpAmpGain::Mul1);
 
     loop {
         let vref = adc.read(&mut vrefint).await;