Refactor the Trace driver to be generic around its peripheral (#1140)

* Use patched version of PAC for C6/H2

* Rename `TRACE` peripheral to `TRACE0`

* Refactor `Trace` driver to be generic around its peripheral

* Update `CHANGELOG.md`

CHANGELOG.md

@@ -33,6 +33,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - DMA descriptor count no longer needs to be multiplied by 3 (#1054)
 - RMT channels no longer take the channel number as a generic param (#959)
 - The `esp-hal-common` package is now called `esp-hal` (#1131)
+- Refactor the `Trace` driver to be generic around its peripheral (#1140)
 
 ### Removed
 

esp-hal/Cargo.toml

@@ -65,9 +65,9 @@ ufmt-write = { version = "0.1.0", optional = true }
 esp32   = { version = "0.28.0", features = ["critical-section"], optional = true }
 esp32c2 = { version = "0.17.0", features = ["critical-section"], optional = true }
 esp32c3 = { version = "0.20.0", features = ["critical-section"], optional = true }
-esp32c6 = { version = "0.11.0", features = ["critical-section"], optional = true }
+esp32c6 = { git = "https://github.com/esp-rs/esp-pacs", rev = "a066f0e", features = ["critical-section"], optional = true }
-esp32h2 = { version = "0.7.0",  features = ["critical-section"], optional = true }
+esp32h2 = { git = "https://github.com/esp-rs/esp-pacs", rev = "a066f0e", features = ["critical-section"], optional = true }
-esp32p4 = { git = "https://github.com/esp-rs/esp-pacs", rev = "c801d10", optional = true }
+esp32p4 = { git = "https://github.com/esp-rs/esp-pacs", rev = "a066f0e", features = ["critical-section"], optional = true }
 esp32s2 = { version = "0.19.0", features = ["critical-section"], optional = true }
 esp32s3 = { version = "0.23.0", features = ["critical-section"], optional = true }
 

esp-hal/devices/esp32c6/device.toml

@@ -59,7 +59,7 @@ peripherals = [
     "tee",
     "timg0",
     "timg1",
-    "trace",
+    "trace0",
     "twai0",
     "twai1",
     "uart0",

esp-hal/devices/esp32h2/device.toml

@@ -52,7 +52,7 @@ peripherals = [
     "tee",
     "timg0",
     "timg1",
-    "trace",
+    "trace0",
     # "twai0",
     "uart0",
     "uart1",

esp-hal/src/lib.rs

@@ -146,7 +146,7 @@ pub mod system;
 pub mod systimer;
 #[cfg(any(timg0, timg1))]
 pub mod timer;
-#[cfg(trace)]
+#[cfg(trace0)]
 pub mod trace;
 #[cfg(any(twai0, twai1))]
 pub mod twai;

esp-hal/src/soc/esp32c6/peripherals.rs

@@ -78,7 +78,7 @@ crate::peripherals! {
     TEE <= TEE,
     TIMG0 <= TIMG0,
     TIMG1 <= TIMG1,
-    TRACE <= TRACE,
+    TRACE0 <= TRACE,
     TWAI0 <= TWAI0,
     TWAI1 <= TWAI1,
     UART0 <= UART0,

esp-hal/src/soc/esp32h2/peripherals.rs

@@ -70,7 +70,7 @@ crate::peripherals! {
     TEE <= TEE,
     TIMG0 <= TIMG0,
     TIMG1 <= TIMG1,
-    TRACE <= TRACE,
+    TRACE0 <= TRACE,
     TWAI0 <= TWAI0,
     UART0 <= UART0,
     UART1 <= UART1,

esp-hal/src/system.rs

@@ -101,8 +101,8 @@ pub enum Peripheral {
     Ecc,
     #[cfg(soc_etm)]
     Etm,
-    #[cfg(trace)]
+    #[cfg(trace0)]
-    Trace,
+    Trace0,
     #[cfg(lcd_cam)]
     LcdCam,
 }
@@ -592,8 +592,8 @@ impl PeripheralClockControl {
                 system.etm_conf().modify(|_, w| w.etm_clk_en().set_bit());
                 system.etm_conf().modify(|_, w| w.etm_rst_en().clear_bit());
             }
-            #[cfg(trace)]
+            #[cfg(trace0)]
-            Peripheral::Trace => {
+            Peripheral::Trace0 => {
                 system
                     .trace_conf()
                     .modify(|_, w| w.trace_clk_en().set_bit());

esp-hal/src/trace.rs

@@ -21,7 +21,7 @@
 //!
 //! ## Example
 //! ```no_run
-//! let mut trace = Trace::new(peripherals.TRACE);
+//! let mut trace = Trace::new(peripherals.TRACE0);
 //! let buffer = unsafe { &mut BUFFER[..] };
 //! trace.start_trace(buffer);
 //! // traced code
@@ -33,6 +33,7 @@
 
 use crate::{
     peripheral::{Peripheral, PeripheralRef},
+    peripherals::trace::RegisterBlock,
     system::PeripheralClockControl,
 };
 
@@ -50,17 +51,20 @@ pub struct TraceResult {
 }
 
 /// TRACE Encoder Instance
-pub struct Trace<'d> {
+pub struct Trace<'d, T> {
-    peripheral: PeripheralRef<'d, crate::peripherals::TRACE>,
+    peripheral: PeripheralRef<'d, T>,
     buffer: Option<&'d mut [u8]>,
 }
 
-impl<'d> Trace<'d> {
+impl<'d, T> Trace<'d, T>
+where
+    T: Instance,
+{
     /// Construct a new instance
-    pub fn new(peripheral: impl Peripheral<P = crate::peripherals::TRACE> + 'd) -> Self {
+    pub fn new(peripheral: impl Peripheral<P = T> + 'd) -> Self {
         crate::into_ref!(peripheral);
 
-        PeripheralClockControl::enable(crate::system::Peripheral::Trace);
+        PeripheralClockControl::enable(crate::system::Peripheral::Trace0);
 
         Self {
             peripheral,
@@ -70,29 +74,31 @@ impl<'d> Trace<'d> {
 
     /// Start tracing, writing data into the `buffer`
     pub fn start_trace(&mut self, buffer: &'d mut [u8]) {
-        self.peripheral.mem_start_addr().modify(|_, w| {
+        let reg_block = self.peripheral.register_block();
-            w.mem_staet_addr()
+
+        reg_block.mem_start_addr().modify(|_, w| {
+            w.mem_start_addr()
                 .variant(buffer.as_ptr() as *const _ as u32)
         });
-        self.peripheral.mem_end_addr().modify(|_, w| {
+        reg_block.mem_end_addr().modify(|_, w| {
             w.mem_end_addr()
                 .variant((buffer.as_ptr() as *const _ as u32) + (buffer.len() as u32))
         });
-        self.peripheral
+        reg_block
             .mem_addr_update()
             .write(|w| w.mem_current_addr_update().set_bit());
 
         // won't set bit in int-raw without enabling
-        self.peripheral
+        reg_block
             .intr_ena()
             .modify(|_, w| w.mem_full_intr_ena().set_bit());
 
         // for now always use looping mode
-        self.peripheral
+        reg_block
             .trigger()
             .write(|w| w.mem_loop().set_bit().restart_ena().set_bit());
 
-        self.peripheral.intr_clr().write(|w| {
+        reg_block.intr_clr().write(|w| {
             w.fifo_overflow_intr_clr()
                 .set_bit()
                 .mem_full_intr_clr()
@@ -100,7 +106,7 @@ impl<'d> Trace<'d> {
         });
 
         self.buffer.replace(buffer);
-        self.peripheral.trigger().write(|w| w.on().set_bit());
+        reg_block.trigger().write(|w| w.on().set_bit());
     }
 
     /// Stop tracing
@@ -108,7 +114,9 @@ impl<'d> Trace<'d> {
     /// Be aware that valid data might not start at index 0 and you need to
     /// account for wrapping when reading the data.
     pub fn stop_trace(&mut self) -> Result<TraceResult, Error> {
-        self.peripheral
+        let reg_block = self.peripheral.register_block();
+
+        reg_block
             .trigger()
             .write(|w| w.off().set_bit().restart_ena().clear_bit());
 
@@ -118,21 +126,11 @@ impl<'d> Trace<'d> {
 
         let buffer = self.buffer.take().unwrap();
 
-        loop {
+        while !reg_block.fifo_status().read().fifo_empty().bit_is_set() {}
-            if self
-                .peripheral
-                .fifo_status()
-                .read()
-                .fifo_empty()
-                .bit_is_set()
-            {
-                break;
-            }
-        }
 
-        let overflow = self.peripheral.intr_raw().read().mem_full_intr_raw().bit();
+        let overflow = reg_block.intr_raw().read().mem_full_intr_raw().bit();
         let idx = if overflow {
-            self.peripheral
+            reg_block
                 .mem_current_addr()
                 .read()
                 .mem_current_addr()
@@ -145,7 +143,7 @@ impl<'d> Trace<'d> {
         let len = if overflow {
             buffer.len()
         } else {
-            self.peripheral
+            reg_block
                 .mem_current_addr()
                 .read()
                 .mem_current_addr()
@@ -199,3 +197,13 @@ impl<'d> Trace<'d> {
         })
     }
 }
+
+pub trait Instance {
+    fn register_block(&self) -> &RegisterBlock;
+}
+
+impl Instance for crate::peripherals::TRACE0 {
+    fn register_block(&self) -> &RegisterBlock {
+        self
+    }
+}