first attempt at fixing the 2nd channel problem

embassy-stm32/src/dac/mod.rs

@@ -1,5 +1,7 @@
 #![macro_use]
 
+use core::marker::PhantomData;
+
 use embassy_hal_common::{into_ref, PeripheralRef};
 
 use crate::dma::{Transfer, TransferOptions};
@@ -108,166 +110,108 @@ pub enum ValueArray<'a> {
     Bit12Right(&'a [u16]),
 }
 
-pub struct Dac<'d, T: Instance, Tx> {
+pub trait DacChannel<T: Instance, Tx> {
-    ch1: bool,
+    const CHANNEL: Channel;
-    ch2: bool,
-    txdma: PeripheralRef<'d, Tx>,
-    _peri: PeripheralRef<'d, T>,
-}
-
-impl<'d, T: Instance, Tx> Dac<'d, T, Tx> {
-    pub fn new_ch1(
-        peri: impl Peripheral<P = T> + 'd,
-        txdma: impl Peripheral<P = Tx> + 'd,
-        _ch1: impl Peripheral<P = impl DacPin<T, 1>> + 'd,
-    ) -> Self {
-        into_ref!(peri);
-        Self::new_inner(peri, true, false, txdma)
-    }
-
-    pub fn new_ch2(
-        peri: impl Peripheral<P = T> + 'd,
-        txdma: impl Peripheral<P = Tx> + 'd,
-        _ch2: impl Peripheral<P = impl DacPin<T, 2>> + 'd,
-    ) -> Self {
-        into_ref!(peri);
-        Self::new_inner(peri, false, true, txdma)
-    }
-
-    pub fn new_ch1_and_ch2(
-        peri: impl Peripheral<P = T> + 'd,
-        txdma: impl Peripheral<P = Tx> + 'd,
-        _ch1: impl Peripheral<P = impl DacPin<T, 1>> + 'd,
-        _ch2: impl Peripheral<P = impl DacPin<T, 2>> + 'd,
-    ) -> Self {
-        into_ref!(peri);
-        Self::new_inner(peri, true, true, txdma)
-    }
-
-    /// Perform initialisation steps for the DAC
-    fn new_inner(peri: PeripheralRef<'d, T>, ch1: bool, ch2: bool, txdma: impl Peripheral<P = Tx> + 'd) -> Self {
-        into_ref!(txdma);
-        T::enable();
-        T::reset();
-
-        let mut dac = Self {
-            ch1,
-            ch2,
-            txdma,
-            _peri: peri,
-        };
-
-        // Configure each activated channel. All results can be `unwrap`ed since they
-        // will only error if the channel is not configured (i.e. ch1, ch2 are false)
-        if ch1 {
-            dac.set_channel_mode(Channel::Ch1, 0).unwrap();
-            dac.enable_channel(Channel::Ch1).unwrap();
-            dac.set_trigger_enable(Channel::Ch1, true).unwrap();
-        }
-        if ch2 {
-            dac.set_channel_mode(Channel::Ch2, 0).unwrap();
-            dac.enable_channel(Channel::Ch2).unwrap();
-            dac.set_trigger_enable(Channel::Ch2, true).unwrap();
-        }
-
-        dac
-    }
-
-    /// Check the channel is configured
-    fn check_channel_configured(&self, ch: Channel) -> Result<(), Error> {
-        if (ch == Channel::Ch1 && !self.ch1) || (ch == Channel::Ch2 && !self.ch2) {
-            Err(Error::UnconfiguredChannel)
-        } else {
-            Ok(())
-        }
-    }
 
     /// Enable trigger of the given channel
-    fn set_trigger_enable(&mut self, ch: Channel, on: bool) -> Result<(), Error> {
+    fn set_trigger_enable(&mut self, on: bool) -> Result<(), Error> {
-        self.check_channel_configured(ch)?;
         T::regs().cr().modify(|reg| {
-            reg.set_ten(ch.index(), on);
+            reg.set_ten(Self::CHANNEL.index(), on);
         });
         Ok(())
     }
 
     /// Set mode register of the given channel
-    fn set_channel_mode(&mut self, ch: Channel, val: u8) -> Result<(), Error> {
+    fn set_channel_mode(&mut self, val: u8) -> Result<(), Error> {
-        self.check_channel_configured(ch)?;
         T::regs().mcr().modify(|reg| {
-            reg.set_mode(ch.index(), val);
+            reg.set_mode(Self::CHANNEL.index(), val);
         });
         Ok(())
     }
 
     /// Set enable register of the given channel
-    fn set_channel_enable(&mut self, ch: Channel, on: bool) -> Result<(), Error> {
+    fn set_channel_enable(&mut self, on: bool) -> Result<(), Error> {
-        self.check_channel_configured(ch)?;
         T::regs().cr().modify(|reg| {
-            reg.set_en(ch.index(), on);
+            reg.set_en(Self::CHANNEL.index(), on);
         });
         Ok(())
     }
 
     /// Enable the DAC channel `ch`
-    pub fn enable_channel(&mut self, ch: Channel) -> Result<(), Error> {
+    fn enable_channel(&mut self) -> Result<(), Error> {
-        self.set_channel_enable(ch, true)
+        self.set_channel_enable(true)
     }
 
     /// Disable the DAC channel `ch`
-    pub fn disable_channel(&mut self, ch: Channel) -> Result<(), Error> {
+    fn disable_channel(&mut self) -> Result<(), Error> {
-        self.set_channel_enable(ch, false)
+        self.set_channel_enable(false)
-    }
-
-    /// Select a new trigger for CH1 (disables the channel)
-    pub fn select_trigger_ch1(&mut self, trigger: Ch1Trigger) -> Result<(), Error> {
-        self.check_channel_configured(Channel::Ch1)?;
-        unwrap!(self.disable_channel(Channel::Ch1));
-        T::regs().cr().modify(|reg| {
-            reg.set_tsel1(trigger.tsel());
-        });
-        Ok(())
-    }
-
-    /// Select a new trigger for CH2 (disables the channel)  
-    pub fn select_trigger_ch2(&mut self, trigger: Ch2Trigger) -> Result<(), Error> {
-        self.check_channel_configured(Channel::Ch2)?;
-        unwrap!(self.disable_channel(Channel::Ch2));
-        T::regs().cr().modify(|reg| {
-            reg.set_tsel2(trigger.tsel());
-        });
-        Ok(())
     }
 
     /// Perform a software trigger on `ch`
-    pub fn trigger(&mut self, ch: Channel) -> Result<(), Error> {
+    fn trigger(&mut self) -> Result<(), Error> {
-        self.check_channel_configured(ch)?;
         T::regs().swtrigr().write(|reg| {
-            reg.set_swtrig(ch.index(), true);
+            reg.set_swtrig(Self::CHANNEL.index(), true);
         });
         Ok(())
     }
 
-    /// Perform a software trigger on all channels
-    pub fn trigger_all(&mut self) {
-        T::regs().swtrigr().write(|reg| {
-            reg.set_swtrig(Channel::Ch1.index(), true);
-            reg.set_swtrig(Channel::Ch2.index(), true);
-        });
-    }
-
     /// Set a value to be output by the DAC on trigger.
     ///
     /// The `value` is written to the corresponding "data holding register"
-    pub fn set(&mut self, ch: Channel, value: Value) -> Result<(), Error> {
+    fn set(&mut self, value: Value) -> Result<(), Error> {
-        self.check_channel_configured(ch)?;
         match value {
-            Value::Bit8(v) => T::regs().dhr8r(ch.index()).write(|reg| reg.set_dhr(v)),
+            Value::Bit8(v) => T::regs().dhr8r(Self::CHANNEL.index()).write(|reg| reg.set_dhr(v)),
-            Value::Bit12Left(v) => T::regs().dhr12l(ch.index()).write(|reg| reg.set_dhr(v)),
+            Value::Bit12Left(v) => T::regs().dhr12l(Self::CHANNEL.index()).write(|reg| reg.set_dhr(v)),
-            Value::Bit12Right(v) => T::regs().dhr12r(ch.index()).write(|reg| reg.set_dhr(v)),
+            Value::Bit12Right(v) => T::regs().dhr12r(Self::CHANNEL.index()).write(|reg| reg.set_dhr(v)),
         }
         Ok(())
     }
+}
+
+pub struct Dac<'d, T: Instance, Tx> {
+    ch1: DacCh1<'d, T, Tx>,
+    ch2: DacCh2<'d, T, Tx>,
+}
+
+pub struct DacCh1<'d, T: Instance, Tx> {
+    _peri: PeripheralRef<'d, T>,
+    dma: PeripheralRef<'d, Tx>,
+}
+
+pub struct DacCh2<'d, T: Instance, Tx> {
+    phantom: PhantomData<&'d mut T>,
+    dma: PeripheralRef<'d, Tx>,
+}
+
+impl<'d, T: Instance, Tx> DacCh1<'d, T, Tx> {
+    /// Perform initialisation steps for the DAC
+    pub fn new(
+        peri: impl Peripheral<P = T> + 'd,
+        dma: impl Peripheral<P = Tx> + 'd,
+        _pin: impl Peripheral<P = impl DacPin<T, 1>> + 'd,
+    ) -> Self {
+        into_ref!(peri, dma);
+        T::enable();
+        T::reset();
+
+        let mut dac = Self { _peri: peri, dma };
+
+        // Configure each activated channel. All results can be `unwrap`ed since they
+        // will only error if the channel is not configured (i.e. ch1, ch2 are false)
+        dac.set_channel_mode(0).unwrap();
+        dac.enable_channel().unwrap();
+        dac.set_trigger_enable(true).unwrap();
+
+        dac
+    }
+    /// Select a new trigger for CH1 (disables the channel)
+    pub fn select_trigger(&mut self, trigger: Ch1Trigger) -> Result<(), Error> {
+        unwrap!(self.disable_channel());
+        T::regs().cr().modify(|reg| {
+            reg.set_tsel1(trigger.tsel());
+        });
+        Ok(())
+    }
 
     /// Write `data` to the DAC CH1 via DMA.
     ///
@@ -276,36 +220,12 @@ impl<'d, T: Instance, Tx> Dac<'d, T, Tx> {
     /// Note that for performance reasons in circular mode the transfer complete interrupt is disabled.
     ///
     /// **Important:** Channel 1 has to be configured for the DAC instance!
-    pub async fn write_ch1(&mut self, data: ValueArray<'_>, circular: bool) -> Result<(), Error>
+    async fn write(&mut self, data: ValueArray<'_>, circular: bool) -> Result<(), Error>
     where
         Tx: Dma<T>,
     {
-        self.check_channel_configured(Channel::Ch1)?;
+        let channel = Channel::Ch1.index();
-        self.write_inner(data, circular, Channel::Ch1).await
+        debug!("Writing to channel {}", channel);
-    }
-
-    /// Write `data` to the DAC CH2 via DMA.
-    ///
-    /// To prevent delays/glitches when outputting a periodic waveform, the `circular` flag can be set.
-    /// This will configure a circular DMA transfer that periodically outputs the `data`.
-    /// Note that for performance reasons in circular mode the transfer complete interrupt is disabled.
-    ///
-    /// **Important:** Channel 2 has to be configured for the DAC instance!
-    pub async fn write_ch2(&mut self, data: ValueArray<'_>, circular: bool) -> Result<(), Error>
-    where
-        Tx: Dma<T>,
-    {
-        self.check_channel_configured(Channel::Ch2)?;
-        self.write_inner(data, circular, Channel::Ch2).await
-    }
-
-    /// Performs the dma write for the given channel.
-    /// TODO: Should self be &mut?
-    async fn write_inner(&self, data_ch1: ValueArray<'_>, circular: bool, channel: Channel) -> Result<(), Error>
-    where
-        Tx: Dma<T>,
-    {
-        let channel = channel.index();
 
         // Enable DAC and DMA
         T::regs().cr().modify(|w| {
@@ -313,11 +233,11 @@ impl<'d, T: Instance, Tx> Dac<'d, T, Tx> {
             w.set_dmaen(channel, true);
         });
 
-        let tx_request = self.txdma.request();
+        let tx_request = self.dma.request();
-        let dma_channel = &self.txdma;
+        let dma_channel = &self.dma;
 
         // Initiate the correct type of DMA transfer depending on what data is passed
-        let tx_f = match data_ch1 {
+        let tx_f = match data {
             ValueArray::Bit8(buf) => unsafe {
                 Transfer::new_write(
                     dma_channel,
@@ -374,6 +294,168 @@ impl<'d, T: Instance, Tx> Dac<'d, T, Tx> {
     }
 }
 
+impl<'d, T: Instance, Tx> DacCh2<'d, T, Tx> {
+    /// Perform initialisation steps for the DAC
+    pub fn new_ch2(
+        peri: impl Peripheral<P = T> + 'd,
+        dma: impl Peripheral<P = Tx> + 'd,
+        _pin: impl Peripheral<P = impl DacPin<T, 2>> + 'd,
+    ) -> Self {
+        into_ref!(peri, dma);
+        T::enable();
+        T::reset();
+
+        let mut dac = Self {
+            phantom: PhantomData,
+            dma,
+        };
+
+        // Configure each activated channel. All results can be `unwrap`ed since they
+        // will only error if the channel is not configured (i.e. ch1, ch2 are false)
+        dac.set_channel_mode(0).unwrap();
+        dac.enable_channel().unwrap();
+        dac.set_trigger_enable(true).unwrap();
+
+        dac
+    }
+
+    /// Select a new trigger for CH1 (disables the channel)
+    pub fn select_trigger(&mut self, trigger: Ch2Trigger) -> Result<(), Error> {
+        unwrap!(self.disable_channel());
+        T::regs().cr().modify(|reg| {
+            reg.set_tsel2(trigger.tsel());
+        });
+        Ok(())
+    }
+
+    /// Write `data` to the DAC CH1 via DMA.
+    ///
+    /// To prevent delays/glitches when outputting a periodic waveform, the `circular` flag can be set.
+    /// This will configure a circular DMA transfer that periodically outputs the `data`.
+    /// Note that for performance reasons in circular mode the transfer complete interrupt is disabled.
+    ///
+    /// **Important:** Channel 1 has to be configured for the DAC instance!
+    async fn write(&mut self, data: ValueArray<'_>, circular: bool) -> Result<(), Error>
+    where
+        Tx: Dma<T>,
+    {
+        let channel = Channel::Ch2.index();
+        debug!("Writing to channel {}", channel);
+
+        // Enable DAC and DMA
+        T::regs().cr().modify(|w| {
+            w.set_en(channel, true);
+            w.set_dmaen(channel, true);
+        });
+
+        let tx_request = self.dma.request();
+        let dma_channel = &self.dma;
+
+        // Initiate the correct type of DMA transfer depending on what data is passed
+        let tx_f = match data {
+            ValueArray::Bit8(buf) => unsafe {
+                Transfer::new_write(
+                    dma_channel,
+                    tx_request,
+                    buf,
+                    T::regs().dhr8r(channel).as_ptr() as *mut u8,
+                    TransferOptions {
+                        circular,
+                        half_transfer_ir: false,
+                        complete_transfer_ir: !circular,
+                    },
+                )
+            },
+            ValueArray::Bit12Left(buf) => unsafe {
+                Transfer::new_write(
+                    dma_channel,
+                    tx_request,
+                    buf,
+                    T::regs().dhr12l(channel).as_ptr() as *mut u16,
+                    TransferOptions {
+                        circular,
+                        half_transfer_ir: false,
+                        complete_transfer_ir: !circular,
+                    },
+                )
+            },
+            ValueArray::Bit12Right(buf) => unsafe {
+                Transfer::new_write(
+                    dma_channel,
+                    tx_request,
+                    buf,
+                    T::regs().dhr12r(channel).as_ptr() as *mut u16,
+                    TransferOptions {
+                        circular,
+                        half_transfer_ir: false,
+                        complete_transfer_ir: !circular,
+                    },
+                )
+            },
+        };
+
+        tx_f.await;
+
+        // finish dma
+        // TODO: Do we need to check any status registers here?
+        T::regs().cr().modify(|w| {
+            // Disable the DAC peripheral
+            w.set_en(channel, false);
+            // Disable the DMA. TODO: Is this necessary?
+            w.set_dmaen(channel, false);
+        });
+
+        Ok(())
+    }
+}
+
+impl<'d, T: Instance, Tx> Dac<'d, T, Tx> {
+    pub fn new(
+        peri: impl Peripheral<P = T> + 'd,
+        dma_ch1: impl Peripheral<P = Tx> + 'd,
+        dma_ch2: impl Peripheral<P = Tx> + 'd,
+        _pin_ch1: impl Peripheral<P = impl DacPin<T, 1>> + 'd,
+        _pin_ch2: impl Peripheral<P = impl DacPin<T, 2>> + 'd,
+    ) -> Self {
+        into_ref!(peri, dma_ch1, dma_ch2);
+        T::enable();
+        T::reset();
+
+        let mut dac_ch1 = DacCh1 {
+            _peri: peri,
+            dma: dma_ch1,
+        };
+
+        let mut dac_ch2 = DacCh2 {
+            phantom: PhantomData,
+            dma: dma_ch2,
+        };
+
+        // Configure each activated channel. All results can be `unwrap`ed since they
+        // will only error if the channel is not configured (i.e. ch1, ch2 are false)
+        dac_ch1.set_channel_mode(0).unwrap();
+        dac_ch1.enable_channel().unwrap();
+        dac_ch1.set_trigger_enable(true).unwrap();
+
+        dac_ch1.set_channel_mode(0).unwrap();
+        dac_ch1.enable_channel().unwrap();
+        dac_ch1.set_trigger_enable(true).unwrap();
+
+        Self {
+            ch1: dac_ch1,
+            ch2: dac_ch2,
+        }
+    }
+}
+
+impl<'d, T: Instance, Tx> DacChannel<T, Tx> for DacCh1<'d, T, Tx> {
+    const CHANNEL: Channel = Channel::Ch1;
+}
+
+impl<'d, T: Instance, Tx> DacChannel<T, Tx> for DacCh2<'d, T, Tx> {
+    const CHANNEL: Channel = Channel::Ch2;
+}
+
 pub(crate) mod sealed {
     pub trait Instance {
         fn regs() -> &'static crate::pac::dac::Dac;