Refactor Dac1/Dac2 drivers into a single Dac driver (#1661)

* Refactor `Dac1`/`Dac2` drivers into a single `Dac` driver

* Fix unrelated build errors resulting from updated PACs

* Add new `Instance` trait to `prelude` module, update `dac` example

* Update `CHANGELOG.md`

* Fix example in module documentation

* Turns out these are 0-indexed after all

esp-hal/CHANGELOG.md

@@ -13,6 +13,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Changed
 
+- Refactor `Dac1`/`Dac2` drivers into a single `Dac` driver (#1661)
+
 ### Removed
 
 ## [0.18.0] - 2024-06-04

esp-hal/Cargo.toml

@@ -53,13 +53,13 @@ xtensa-lx                = { version = "0.9.0", optional = true }
 # IMPORTANT:
 # Each supported device MUST have its PAC included below along with a
 # corresponding feature.
-esp32   = { version = "0.31.0", features = ["critical-section", "rt"], optional = true }
+esp32   = { git = "https://github.com/esp-rs/esp-pacs", rev = "a7c72f7", features = ["critical-section", "rt"], optional = true }
-esp32c2 = { version = "0.20.0", features = ["critical-section", "rt"], optional = true }
+esp32c2 = { git = "https://github.com/esp-rs/esp-pacs", rev = "a7c72f7", features = ["critical-section", "rt"], optional = true }
-esp32c3 = { version = "0.23.0", features = ["critical-section", "rt"], optional = true }
+esp32c3 = { git = "https://github.com/esp-rs/esp-pacs", rev = "a7c72f7", features = ["critical-section", "rt"], optional = true }
-esp32c6 = { version = "0.14.0", features = ["critical-section", "rt"], optional = true }
+esp32c6 = { git = "https://github.com/esp-rs/esp-pacs", rev = "a7c72f7", features = ["critical-section", "rt"], optional = true }
-esp32h2 = { version = "0.10.0", features = ["critical-section", "rt"], optional = true }
+esp32h2 = { git = "https://github.com/esp-rs/esp-pacs", rev = "a7c72f7", features = ["critical-section", "rt"], optional = true }
-esp32s2 = { version = "0.22.0", features = ["critical-section", "rt"], optional = true }
+esp32s2 = { git = "https://github.com/esp-rs/esp-pacs", rev = "a7c72f7", features = ["critical-section", "rt"], optional = true }
-esp32s3 = { version = "0.26.0", features = ["critical-section", "rt"], optional = true }
+esp32s3 = { git = "https://github.com/esp-rs/esp-pacs", rev = "a7c72f7", features = ["critical-section", "rt"], optional = true }
 
 [target.'cfg(target_arch = "riscv32")'.dependencies]
 esp-riscv-rt = { version = "0.8.0", path = "../esp-riscv-rt" }

esp-hal/src/analog/dac.rs

@@ -16,8 +16,8 @@
 //! let gpio25 = io.pins.gpio25;
 //! let gpio26 = io.pins.gpio26;
 //!
-//! let mut dac1 = Dac1::new(peripherals.DAC1, gpio25);
+//! let mut dac1 = Dac::new(peripherals.DAC1, gpio25);
-//! let mut dac2 = Dac2::new(peripherals.DAC2, gpio26);
+//! let mut dac2 = Dac::new(peripherals.DAC2, gpio26);
 //!
 //! let mut delay = Delay::new(&clocks);
 //!
@@ -36,12 +36,16 @@
 //! }
 //! ```
 
+#![deny(missing_docs)]
+
 use crate::{
     gpio::{self, AnalogPin},
     peripheral::{Peripheral, PeripheralRef},
-    peripherals,
 };
 
+// Only specific pins can be used with each DAC peripheral, and of course
+// these pins are different depending on which chip you are using; for this
+// reason, we will type alias the pins for ease of use later in this module:
 cfg_if::cfg_if! {
     if #[cfg(esp32)] {
         type Dac1Gpio = gpio::Gpio25;
@@ -52,26 +56,33 @@ cfg_if::cfg_if! {
     }
 }
 
-/// Digital-to-Analog Converter (DAC) Channel 1
+/// Digital-to-Analog Converter (DAC) Channel
-pub struct Dac1<'d> {
+pub struct Dac<'d, T>
-    _inner: PeripheralRef<'d, peripherals::DAC1>,
+where
+    T: Instance,
+    T::Pin: AnalogPin,
+{
+    _inner: PeripheralRef<'d, T>,
 }
 
-impl<'d> Dac1<'d> {
+impl<'d, T> Dac<'d, T>
-    /// Constructs a new DAC instance.
+where
-    pub fn new(dac: impl Peripheral<P = peripherals::DAC1> + 'd, pin: Dac1Gpio) -> Self {
+    T: Instance,
+    T::Pin: AnalogPin,
+{
+    /// Construct a new instance of [`Dac`].
+    pub fn new(dac: impl Peripheral<P = T> + 'd, pin: T::Pin) -> Self {
         crate::into_ref!(dac);
-        // TODO revert this on drop
+
+        // TODO: Revert on drop.
         pin.set_analog(crate::private::Internal);
 
         #[cfg(esp32s2)]
-        unsafe { &*peripherals::SENS::PTR }
+        unsafe { &*crate::peripherals::SENS::PTR }
             .sar_dac_ctrl1()
             .modify(|_, w| w.dac_clkgate_en().set_bit());
 
-        unsafe { &*peripherals::RTC_IO::PTR }
+        T::enable_xpd();
-            .pad_dac1()
-            .modify(|_, w| w.pdac1_dac_xpd_force().set_bit().pdac1_xpd_dac().set_bit());
 
         Self { _inner: dac }
     }
@@ -81,51 +92,44 @@ impl<'d> Dac1<'d> {
     /// For each DAC channel, the output analog voltage can be calculated as
     /// follows: DACn_OUT = VDD3P3_RTC * PDACn_DAC/256
     pub fn write(&mut self, value: u8) {
+        T::set_pad_source();
+        T::write_byte(value);
+    }
+}
+
+#[doc(hidden)]
+pub trait Instance: crate::private::Sealed {
+    const INDEX: usize;
+
+    type Pin;
+
+    fn enable_xpd() {
+        unsafe { &*crate::peripherals::RTC_IO::PTR }
+            .pad_dac(Self::INDEX)
+            .modify(|_, w| w.dac_xpd_force().set_bit().xpd_dac().set_bit());
+    }
+
+    fn set_pad_source() {
         unsafe { &*crate::peripherals::SENS::PTR }
             .sar_dac_ctrl2()
-            .modify(|_, w| w.dac_cw_en1().clear_bit());
+            .modify(|_, w| w.dac_cw_en(Self::INDEX as u8).clear_bit());
+    }
 
+    fn write_byte(value: u8) {
         unsafe { &*crate::peripherals::RTC_IO::PTR }
-            .pad_dac1()
+            .pad_dac(Self::INDEX)
-            .modify(|_, w| unsafe { w.pdac1_dac().bits(value) });
+            .modify(|_, w| unsafe { w.dac().bits(value) });
     }
 }
 
-/// Digital-to-Analog Converter (DAC) Channel 2
+impl Instance for crate::peripherals::DAC1 {
-pub struct Dac2<'d> {
+    const INDEX: usize = 0;
-    _inner: PeripheralRef<'d, peripherals::DAC2>,
+
+    type Pin = Dac1Gpio;
 }
 
-impl<'d> Dac2<'d> {
+impl Instance for crate::peripherals::DAC2 {
-    /// Constructs a new DAC instance.
+    const INDEX: usize = 1;
-    pub fn new(dac: impl Peripheral<P = peripherals::DAC2> + 'd, pin: Dac2Gpio) -> Self {
-        crate::into_ref!(dac);
-        // TODO revert this on drop
-        pin.set_analog(crate::private::Internal);
 
-        #[cfg(esp32s2)]
+    type Pin = Dac2Gpio;
-        unsafe { &*peripherals::SENS::PTR }
-            .sar_dac_ctrl1()
-            .modify(|_, w| w.dac_clkgate_en().set_bit());
-
-        unsafe { &*peripherals::RTC_IO::PTR }
-            .pad_dac2()
-            .modify(|_, w| w.pdac2_dac_xpd_force().set_bit().pdac2_xpd_dac().set_bit());
-
-        Self { _inner: dac }
-    }
-
-    /// Writes the given value.
-    ///
-    /// For each DAC channel, the output analog voltage can be calculated as
-    /// follows: DACn_OUT = VDD3P3_RTC * PDACn_DAC/256
-    pub fn write(&mut self, value: u8) {
-        unsafe { &*crate::peripherals::SENS::PTR }
-            .sar_dac_ctrl2()
-            .modify(|_, w| w.dac_cw_en2().clear_bit());
-
-        unsafe { &*crate::peripherals::RTC_IO::PTR }
-            .pad_dac2()
-            .modify(|_, w| unsafe { w.pdac2_dac().bits(value) });
-    }
 }

esp-hal/src/prelude.rs

@@ -13,6 +13,8 @@ pub use embedded_dma::{
 pub use fugit::{ExtU64 as _fugit_ExtU64, RateExtU32 as _fugit_RateExtU32};
 pub use nb;
 
+#[cfg(dac)]
+pub use crate::analog::dac::Instance as _esp_hal_analog_dac_Instance;
 #[cfg(any(dport, pcr, system))]
 pub use crate::clock::Clock as _esp_hal_clock_Clock;
 #[cfg(gpio)]

esp-hal/src/rtc_cntl/mod.rs

@@ -788,9 +788,6 @@ impl Rwdt {
         let rtc_cntl = unsafe { &*LP_WDT::PTR };
 
         self.set_write_protection(false);
-        #[cfg(any(esp32c6, esp32h2))]
-        rtc_cntl.wdtfeed().write(|w| w.rtc_wdt_feed().set_bit());
-        #[cfg(not(any(esp32c6, esp32h2)))]
         rtc_cntl.wdtfeed().write(|w| w.wdt_feed().set_bit());
         self.set_write_protection(true);
     }

esp-hal/src/soc/esp32/gpio.rs

@@ -811,10 +811,10 @@ pub(crate) fn errata36(pin_num: u8, pull_up: Option<bool>, pull_down: Option<boo
         25 => {
             rtcio.pad_dac1().modify(|_, w| {
                 if let Some(pull_up) = pull_up {
-                    w.pdac1_rue().bit(pull_up);
+                    w.rue().bit(pull_up);
                 }
                 if let Some(pull_down) = pull_down {
-                    w.pdac1_rde().bit(pull_down);
+                    w.rde().bit(pull_down);
                 }
                 w
             });
@@ -822,10 +822,10 @@ pub(crate) fn errata36(pin_num: u8, pull_up: Option<bool>, pull_down: Option<boo
         26 => {
             rtcio.pad_dac2().modify(|_, w| {
                 if let Some(pull_up) = pull_up {
-                    w.pdac2_rue().bit(pull_up);
+                    w.rue().bit(pull_up);
                 }
                 if let Some(pull_down) = pull_down {
-                    w.pdac2_rde().bit(pull_down);
+                    w.rde().bit(pull_down);
                 }
                 w
             });
@@ -913,8 +913,8 @@ crate::gpio::analog! {
      (39, 3,  sensor_pads(),   sense4_mux_sel, sense4_fun_sel, sense4_fun_ie)
      (34, 4,  adc_pad(),         adc1_mux_sel,   adc1_fun_sel,   adc1_fun_ie)
      (35, 5,  adc_pad(),         adc2_mux_sel,   adc2_fun_sel,   adc1_fun_ie)
-     (25, 6,  pad_dac1(),       pdac1_mux_sel,  pdac1_fun_sel,  pdac1_fun_ie, pdac1_rue, pdac1_rde)
+     (25, 6,  pad_dac1(),             mux_sel,        fun_sel,        fun_ie, rue,       rde)
-     (26, 7,  pad_dac2(),       pdac2_mux_sel,  pdac2_fun_sel,  pdac2_fun_ie, pdac2_rue, pdac2_rde)
+     (26, 7,  pad_dac2(),             mux_sel,        fun_sel,        fun_ie, rue,       rde)
      (33, 8,  xtal_32k_pad(),    x32n_mux_sel,   x32n_fun_sel,   x32n_fun_ie, x32n_rue,  x32n_rde )
      (32, 9,  xtal_32k_pad(),    x32p_mux_sel,   x32p_fun_sel,   x32p_fun_ie, x32p_rue,  x32p_rde )
      (4,  10, touch_pad0(),           mux_sel,        fun_sel,        fun_ie, rue,       rde      )
@@ -934,8 +934,8 @@ crate::gpio::rtc_pins! {
         (39, 3,  sensor_pads(),    sense4_, sense4_hold_force )
         (34, 4,  adc_pad(),        adc1_,   adc1_hold_force )
         (35, 5,  adc_pad(),        adc2_,   adc2_hold_force )
-        (25, 6,  pad_dac1(),       pdac1_,  pdac1_hold_force,      pdac1_rue, pdac1_rde )
+        (25, 6,  pad_dac1(),       "",      pdac1_hold_force,      rue,       rde )
-        (26, 7,  pad_dac2(),       pdac2_,  pdac2_hold_force,      pdac2_rue, pdac2_rde )
+        (26, 7,  pad_dac2(),       "",      pdac2_hold_force,      rue,       rde )
         (33, 8,  xtal_32k_pad(),   x32n_,   x32n_hold_force,       x32n_rue,  x32n_rde  )
         (32, 9,  xtal_32k_pad(),   x32p_,   x32p_hold_force,       x32p_rue,  x32p_rde  )
         (4,  10, touch_pad0(),     "",      touch_pad0_hold_force, rue,       rde       )

esp-hal/src/soc/esp32s2/gpio.rs

@@ -502,8 +502,8 @@ crate::gpio::analog! {
     (14, 14,  touch_pad(14),   mux_sel,       fun_sel,       fun_ie,       rue,       rde)
     (15, 15,  xtal_32p_pad(),  x32p_mux_sel,  x32p_fun_sel,  x32p_fun_ie,  x32p_rue,  x32p_rde)
     (16, 16,  xtal_32n_pad(),  x32n_mux_sel,  x32n_fun_sel,  x32n_fun_ie,  x32n_rue,  x32n_rde)
-    (17, 17,  pad_dac1(),      pdac1_mux_sel, pdac1_fun_sel, pdac1_fun_ie, pdac1_rue, pdac1_rde)
+    (17, 17,  pad_dac1(),      mux_sel,       fun_sel,       fun_ie,       rue,       rde)
-    (18, 18,  pad_dac2(),      pdac2_mux_sel, pdac2_fun_sel, pdac2_fun_ie, pdac2_rue, pdac2_rde)
+    (18, 18,  pad_dac2(),      mux_sel,       fun_sel,       fun_ie,       rue,       rde)
     (19, 19,  rtc_pad19(),     mux_sel,       fun_sel,       fun_ie,       rue,       rde)
     (20, 20,  rtc_pad20(),     mux_sel,       fun_sel,       fun_ie,       rue,       rde)
     (21, 21,  rtc_pad21(),     mux_sel,       fun_sel,       fun_ie,       rue,       rde)
@@ -527,8 +527,8 @@ crate::gpio::rtc_pins! {
     (14, 14,  touch_pad(14),  "",     touch_pad14_hold, rue,       rde)
     (15, 15,  xtal_32p_pad(), x32p_,  x32p_hold,        x32p_rue,  x32p_rde)
     (16, 16,  xtal_32n_pad(), x32n_,  x32n_hold,        x32n_rue,  x32n_rde)
-    (17, 17,  pad_dac1(),     pdac1_, pdac1_hold,       pdac1_rue, pdac1_rde)
+    (17, 17,  pad_dac1(),     "",     pdac1_hold,       rue,       rde)
-    (18, 18,  pad_dac2(),     pdac2_, pdac2_hold,       pdac2_rue, pdac2_rde)
+    (18, 18,  pad_dac2(),     "",     pdac2_hold,       rue,       rde)
     (19, 19,  rtc_pad19(),    "",     pad19_hold,       rue,       rde)
     (20, 20,  rtc_pad20(),    "",     pad20_hold,       rue,       rde)
     (21, 21,  rtc_pad21(),    "",     pad21_hold,       rue,       rde)

examples/src/bin/dac.rs

@@ -13,7 +13,7 @@
 
 use esp_backtrace as _;
 use esp_hal::{
-    analog::dac::{Dac1, Dac2},
+    analog::dac::Dac,
     clock::ClockControl,
     delay::Delay,
     gpio::Io,
@@ -41,8 +41,8 @@ fn main() -> ! {
     }
 
     // Create DAC instances
-    let mut dac1 = Dac1::new(peripherals.DAC1, dac1_pin);
+    let mut dac1 = Dac::new(peripherals.DAC1, dac1_pin);
-    let mut dac2 = Dac2::new(peripherals.DAC2, dac2_pin);
+    let mut dac2 = Dac::new(peripherals.DAC2, dac2_pin);
 
     let delay = Delay::new(&clocks);