From 67e9c60a233dbba716aed9b1bb8abba8feba8807 Mon Sep 17 00:00:00 2001
From: Jesse Braham <jessebraham@users.noreply.github.com>
Date: Wed, 31 May 2023 06:45:53 -0700
Subject: [PATCH] Simplify the `Delay` driver, derive `Clone` and `Copy` (#568)

* Simplify the `Delay` driver, derive `Clone` and `Copy`

The RISC-V and Xtensa implementations of the delay functionality are both safe to access from multiple contexts, and the `Delay` struct does not take ownership of any peripheral struct, so `Delay` is safe to clone/copy

* Update the CHANGELOG
---
 CHANGELOG.md                |  1 +
 esp-hal-common/src/delay.rs | 59 ++++++++++++++++++-------------------
 2 files changed, 29 insertions(+), 31 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 0345b4b43..e3cf00294 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -32,6 +32,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Add initial support for ASSIST_DEBUG in ESP32-H2 (#566)
 - Add all `SPI` examples for the ESP32-H2 (#549)
 - Add initial support for ADC in ESP32-H2 (#564)
+- Simplify the `Delay` driver, derive `Clone` and `Copy` (#568)
 
 ### Changed
 
diff --git a/esp-hal-common/src/delay.rs b/esp-hal-common/src/delay.rs
index c5f0bd287..4ffb9a97d 100644
--- a/esp-hal-common/src/delay.rs
+++ b/esp-hal-common/src/delay.rs
@@ -1,10 +1,24 @@
-//! Delay driver
+//! Delay driver implement the blocking [DelayMs] and [DelayUs] traits from
+//! [embedded-hal].
 //!
-//! Implement the `DelayMs` and `DelayUs` traits from [embedded-hal].
+//! The delays are implemented in a "best-effort" way, meaning that the CPU will
+//! block for at least the amount of time specified, but accuracy can be
+//! affected by many factors, including interrupt usage.
 //!
+//! [DelayMs]: embedded_hal::blocking::delay::DelayMs
+//! [DelayUs]: embedded_hal::blocking::delay::DelayUs
 //! [embedded-hal]: https://docs.rs/embedded-hal/latest/embedded_hal/
 
-pub use self::delay::Delay;
+use fugit::HertzU64;
+
+/// Delay driver
+///
+/// Uses the `SYSTIMER` peripheral internally for RISC-V devices, and the
+/// built-in Xtensa timer for Xtensa devices.
+#[derive(Clone, Copy)]
+pub struct Delay {
+    freq: HertzU64,
+}
 
 impl<T> embedded_hal::blocking::delay::DelayMs<T> for Delay
 where
@@ -35,33 +49,24 @@ impl embedded_hal_1::delay::DelayUs for Delay {
 
 #[cfg(riscv)]
 mod delay {
-    use fugit::HertzU64;
-
+    use super::*;
     use crate::{clock::Clocks, systimer::SystemTimer};
 
-    /// Uses the `SYSTIMER` peripheral for counting clock cycles, as
-    /// unfortunately the ESP32-C3 does NOT implement the `mcycle` CSR, which is
-    /// how we would normally do this.
-    pub struct Delay {
-        freq: HertzU64,
-    }
-
     impl Delay {
-        /// Create a new Delay instance
+        /// Create a new `Delay` instance
         pub fn new(clocks: &Clocks) -> Self {
-            // The counters and comparators are driven using `XTAL_CLK`. The average clock
-            // frequency is fXTAL_CLK/2.5, which is 16 MHz. The timer counting is
-            // incremented by 1/16 μs on each `CNT_CLK` cycle.
-
+            // The counters and comparators are driven using `XTAL_CLK`.
+            // The average clock frequency is fXTAL_CLK/2.5, which is 16 MHz.
+            // The timer counting is incremented by 1/16 μs on each `CNT_CLK` cycle.
             Self {
-                freq: HertzU64::MHz((clocks.xtal_clock.to_MHz() * 10 / 25) as u64),
+                freq: HertzU64::MHz(clocks.xtal_clock.to_MHz() as u64 * 10 / 25),
             }
         }
 
         /// Delay for the specified number of microseconds
         pub fn delay(&self, us: u32) {
             let t0 = SystemTimer::now();
-            let clocks = (us as u64 * self.freq.raw()) / HertzU64::MHz(1).raw();
+            let clocks = us as u64 * (self.freq / HertzU64::MHz(1));
 
             while SystemTimer::now().wrapping_sub(t0) & SystemTimer::BIT_MASK <= clocks {}
         }
@@ -70,28 +75,20 @@ mod delay {
 
 #[cfg(xtensa)]
 mod delay {
-    use fugit::HertzU64;
-
+    use super::*;
     use crate::clock::Clocks;
 
-    /// Delay driver
-    ///
-    /// Uses the built-in Xtensa timer from the `xtensa_lx` crate.
-    pub struct Delay {
-        freq: HertzU64,
-    }
-
     impl Delay {
-        /// Instantiate the `Delay` driver
+        /// Create a new `Delay` instance
         pub fn new(clocks: &Clocks) -> Self {
             Self {
-                freq: HertzU64::MHz(clocks.cpu_clock.to_MHz() as u64),
+                freq: clocks.cpu_clock.into(),
             }
         }
 
         /// Delay for the specified number of microseconds
         pub fn delay(&self, us: u32) {
-            let clocks = (us as u64 * self.freq.raw()) / HertzU64::MHz(1).raw();
+            let clocks = us as u64 * (self.freq / HertzU64::MHz(1));
             xtensa_lx::timer::delay(clocks as u32);
         }
     }