Autodetect xtal-freq (#1165)

* feat: Autodetect xtal-freq * docs: Update changelog * style: Clippy lints * feat: Remove XtalClock::RtcXtalFreq24M variant * feat: Adjust visibility of estimate_xtal_frequency * feat: Remove xtal freq features
esp-rs · Feb 14, 2024 · 3282442 · 3282442
1 parent 2b8db5c
commit 3282442
Show file tree

Hide file tree

Showing 14 changed files with 42 additions and 138 deletions.
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -56,7 +56,7 @@ jobs:
         run: cd esp-hal-smartled/ && cargo +nightly build -Zbuild-std=core --target=riscv32imac-unknown-none-elf --features=esp32h2
       # Build all Xtensa targets:
       - name: build (esp32)
-        run: cd esp-hal-smartled/ && cargo +esp build -Zbuild-std=core --target=xtensa-esp32-none-elf --features=esp32,xtal-40mhz
+        run: cd esp-hal-smartled/ && cargo +esp build -Zbuild-std=core --target=xtensa-esp32-none-elf --features=esp32
       - name: build (esp32s2)
         run: cd esp-hal-smartled/ && cargo +esp build -Zbuild-std=core --target=xtensa-esp32s2-none-elf --features=esp32s2
       - name: build (esp32s3)
@@ -167,7 +167,7 @@ jobs:
         run: cd esp32-hal/ && cargo check --example=psram --features=psram-2m --release # This example requires release!
       # Make sure we can build without default features enabled, too!
       - name: check esp32-hal (no default features)
-        run: cd esp32-hal/ && cargo build --no-default-features --features=xtal-40mhz
+        run: cd esp32-hal/ && cargo build --no-default-features
       # Ensure documentation can be built
       - name: rustdoc
         run: cd esp32-hal/ && cargo doc --features=eh1
@@ -222,7 +222,7 @@ jobs:
           cargo +nightly check --examples --features=embassy,embassy-time-timg0,embassy-executor-thread,log
       # Make sure we can build without default features enabled, too!
       - name: check esp32c2-hal (no default features)
-        run: cd esp32c2-hal/ && cargo build --no-default-features --features=xtal-40mhz
+        run: cd esp32c2-hal/ && cargo build --no-default-features
       # Ensure documentation can be built
       - name: rustdoc
         run: cd esp32c2-hal/ && cargo doc --features=eh1
@@ -715,7 +715,7 @@ jobs:
 
       # Run 'cargo clippy' on all packages targeting RISC-V:
       - name: clippy (esp32c2-hal)
-        run: cargo clippy --manifest-path=esp-hal/Cargo.toml --target=riscv32imc-unknown-none-elf --features=esp32c2,xtal-40mhz -- -D warnings
+        run: cargo clippy --manifest-path=esp-hal/Cargo.toml --target=riscv32imc-unknown-none-elf --features=esp32c2 -- -D warnings
       - name: clippy (esp32c3-hal)
         run: cargo clippy --manifest-path=esp-hal/Cargo.toml --target=riscv32imc-unknown-none-elf --features=esp32c3 -- -D warnings
       - name: clippy (esp32c6-hal)
@@ -747,7 +747,7 @@ jobs:
 
       # Run 'cargo clippy' on all packages targeting Xtensa:
       - name: clippy (esp32-hal)
-        run: cargo clippy -Zbuild-std=core --manifest-path=esp-hal/Cargo.toml --target=xtensa-esp32-none-elf --features=esp32,xtal-40mhz -- -D warnings
+        run: cargo clippy -Zbuild-std=core --manifest-path=esp-hal/Cargo.toml --target=xtensa-esp32-none-elf --features=esp32 -- -D warnings
       - name: clippy (esp32s2-hal)
         run: cargo clippy -Zbuild-std=core --manifest-path=esp-hal/Cargo.toml --target=xtensa-esp32s2-none-elf --features=esp32s2 -- -D warnings
       - name: clippy (esp32s3-hal)

diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -12,7 +12,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Added
 
 - Add initial support for the ESP32-P4 (#1101)
-- Implement `embedded_hal::pwm::SetDutyCycle` trait for `ledc::channel::Channel` (#1097) 
+- Implement `embedded_hal::pwm::SetDutyCycle` trait for `ledc::channel::Channel` (#1097)
 - ESP32-P4: Add initial GPIO support (#1109)
 - ESP32-P4: Add initial support for interrupts (#1112)
 - ESP32-P4: Add efuse reading support (#1114)
@@ -37,8 +37,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - 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)
+- Auto detect crystal frequency based on `RtcClock::estimate_xtal_frequency()` (#1165)
 
 ### Removed
+- Remove `xtal-26mhz` and `xtal-40mhz` features (#1165)
 
 ### Breaking
 

diff --git a/esp-hal-smartled/Cargo.toml b/esp-hal-smartled/Cargo.toml
@@ -35,9 +35,3 @@ esp32h2 = ["esp-hal/esp32h2"]
 esp32s2 = ["esp-hal/esp32s2"]
 ## Target the ESP32-S3.
 esp32s3 = ["esp-hal/esp32s3"]
-
-#! ### Crystal Frequency Feature Flags
-## Target device has a 26MHz crystal.
-xtal-26mhz = ["esp-hal/xtal-26mhz"]
-## Target device has a 40MHz crystal.
-xtal-40mhz = ["esp-hal/xtal-40mhz"]
diff --git a/esp-hal/Cargo.toml b/esp-hal/Cargo.toml
@@ -84,10 +84,6 @@ esp32p4 = ["dep:esp32p4", "riscv",  "procmacros/esp32p4"]
 esp32s2 = ["dep:esp32s2", "xtensa", "procmacros/esp32s2", "xtensa-lx/esp32s2", "xtensa-lx-rt?/esp32s2", "usb-otg", "portable-atomic/critical-section"]
 esp32s3 = ["dep:esp32s3", "xtensa", "procmacros/esp32s3", "xtensa-lx/esp32s3", "xtensa-lx-rt?/esp32s3", "usb-otg"]
 
-# Crystal frequency selection (ESP32 and ESP32-C2 only!)
-xtal-26mhz = []
-xtal-40mhz = []
-
 # Runetime support
 rt-riscv  = ["esp-riscv-rt/zero-bss", "esp32c2?/rt", "esp32c3?/rt", "esp32c6?/rt", "esp32h2?/rt", "esp32p4?/rt"]
 rt-xtensa = ["dep:xtensa-lx-rt", "esp32?/rt", "esp32s2?/rt", "esp32s3?/rt"]

diff --git a/esp-hal/build.rs b/esp-hal/build.rs
@@ -107,13 +107,6 @@ fn main() -> Result<(), Box<dyn Error>> {
         "esp32", "esp32c2", "esp32c3", "esp32c6", "esp32h2", "esp32p4", "esp32s2", "esp32s3"
     );
 
-    // Handle the features for the ESP32's and ESP32-C2's different crystal
-    // frequencies:
-    #[cfg(any(feature = "esp32", feature = "esp32c2"))]
-    {
-        assert_unique_used_features!("xtal-26mhz", "xtal-40mhz");
-    }
-
     // If the `embassy` feature is enabled, ensure that a time driver implementation
     // is available:
     #[cfg(feature = "embassy")]

diff --git a/esp-hal/src/clock/clocks_ll/esp32.rs b/esp-hal/src/clock/clocks_ll/esp32.rs
@@ -80,15 +80,6 @@ pub(crate) fn esp32_rtc_bbpll_configure(xtal_freq: XtalClock, pll_freq: PllClock
                     bw = 1;
                 }
 
-                XtalClock::RtcXtalFreq24M => {
-                    div_ref = 11;
-                    div7_0 = 224;
-                    div10_8 = 4;
-                    lref = 1;
-                    dcur = 0;
-                    bw = 1;
-                }
-
                 XtalClock::RtcXtalFreqOther(_) => {
                     div_ref = 12;
                     div7_0 = 224;
@@ -137,15 +128,6 @@ pub(crate) fn esp32_rtc_bbpll_configure(xtal_freq: XtalClock, pll_freq: PllClock
                     bw = 1;
                 }
 
-                XtalClock::RtcXtalFreq24M => {
-                    div_ref = 11;
-                    div7_0 = 144;
-                    div10_8 = 4;
-                    lref = 1;
-                    dcur = 0;
-                    bw = 1;
-                }
-
                 XtalClock::RtcXtalFreqOther(_) => {
                     div_ref = 12;
                     div7_0 = 224;

diff --git a/esp-hal/src/clock/mod.rs b/esp-hal/src/clock/mod.rs
@@ -55,6 +55,8 @@
 //! ```
 use fugit::HertzU32;
 
+#[cfg(any(esp32, esp32c2))]
+use crate::rtc_cntl::RtcClock;
 use crate::{
     peripheral::{Peripheral, PeripheralRef},
     system::SystemClockControl,
@@ -134,8 +136,6 @@ impl Clock for CpuClock {
 #[derive(Debug, Clone, Copy)]
 #[non_exhaustive]
 pub enum XtalClock {
-    #[cfg(esp32)]
-    RtcXtalFreq24M,
     #[cfg(any(esp32, esp32c2))]
     RtcXtalFreq26M,
     #[cfg(any(esp32c3, esp32h2, esp32s3))]
@@ -148,8 +148,6 @@ pub enum XtalClock {
 impl Clock for XtalClock {
     fn frequency(&self) -> HertzU32 {
         match self {
-            #[cfg(esp32)]
-            XtalClock::RtcXtalFreq24M => HertzU32::MHz(24),
             #[cfg(any(esp32, esp32c2))]
             XtalClock::RtcXtalFreq26M => HertzU32::MHz(26),
             #[cfg(any(esp32c3, esp32h2, esp32s3))]
@@ -354,42 +352,35 @@ impl<'d> ClockControl<'d> {
     pub fn boot_defaults(
         clock_control: impl Peripheral<P = SystemClockControl> + 'd,
     ) -> ClockControl<'d> {
-        #[cfg(feature = "xtal-40mhz")]
-        return ClockControl {
-            _private: clock_control.into_ref(),
-            desired_rates: RawClocks {
-                cpu_clock: HertzU32::MHz(80),
-                apb_clock: HertzU32::MHz(80),
-                xtal_clock: HertzU32::MHz(40),
-                i2c_clock: HertzU32::MHz(80),
-                pwm_clock: HertzU32::MHz(160),
-            },
+        let xtal_freq = if RtcClock::estimate_xtal_frequency() > 33 {
+            40
+        } else {
+            26
         };
 
-        #[cfg(feature = "xtal-26mhz")]
-        return ClockControl {
+        ClockControl {
             _private: clock_control.into_ref(),
             desired_rates: RawClocks {
                 cpu_clock: HertzU32::MHz(80),
                 apb_clock: HertzU32::MHz(80),
-                xtal_clock: HertzU32::MHz(26),
+                xtal_clock: HertzU32::MHz(xtal_freq),
                 i2c_clock: HertzU32::MHz(80),
                 pwm_clock: HertzU32::MHz(160),
             },
-        };
+        }
     }
 
     /// Configure the CPU clock speed.
     pub fn configure(
         clock_control: impl Peripheral<P = SystemClockControl> + 'd,
         cpu_clock_speed: CpuClock,
     ) -> ClockControl<'d> {
-        // like NuttX use 40M hardcoded - if it turns out to be a problem
-        // we will take care then
-        #[cfg(feature = "xtal-40mhz")]
-        let xtal_freq = XtalClock::RtcXtalFreq40M;
-        #[cfg(feature = "xtal-26mhz")]
-        let xtal_freq = XtalClock::RtcXtalFreq26M;
+        let xtal_freq = if RtcClock::estimate_xtal_frequency() > 33 {
+            XtalClock::RtcXtalFreq40M
+        } else {
+            XtalClock::RtcXtalFreq26M
+        };
+
         let pll_freq = match cpu_clock_speed {
             CpuClock::Clock80MHz => PllClock::Pll320MHz,
             CpuClock::Clock160MHz => PllClock::Pll320MHz,
@@ -428,25 +419,20 @@ impl<'d> ClockControl<'d> {
     pub fn boot_defaults(
         clock_control: impl Peripheral<P = SystemClockControl> + 'd,
     ) -> ClockControl<'d> {
-        #[cfg(feature = "xtal-40mhz")]
-        return ClockControl {
-            _private: clock_control.into_ref(),
-            desired_rates: RawClocks {
-                cpu_clock: HertzU32::MHz(80),
-                apb_clock: HertzU32::MHz(40),
-                xtal_clock: HertzU32::MHz(40),
-            },
+        let xtal_freq = if RtcClock::estimate_xtal_frequency() > 33 {
+            40
+        } else {
+            26
         };
 
-        #[cfg(feature = "xtal-26mhz")]
-        return ClockControl {
+        ClockControl {
             _private: clock_control.into_ref(),
             desired_rates: RawClocks {
                 cpu_clock: HertzU32::MHz(80),
                 apb_clock: HertzU32::MHz(40),
-                xtal_clock: HertzU32::MHz(26),
+                xtal_clock: HertzU32::MHz(xtal_freq),
             },
-        };
+        }
     }
 
     /// Configure the CPU clock speed.
@@ -455,10 +441,13 @@ impl<'d> ClockControl<'d> {
         cpu_clock_speed: CpuClock,
     ) -> ClockControl<'d> {
         let apb_freq;
-        #[cfg(feature = "xtal-40mhz")]
-        let xtal_freq = XtalClock::RtcXtalFreq40M;
-        #[cfg(feature = "xtal-26mhz")]
-        let xtal_freq = XtalClock::RtcXtalFreq26M;
+
+        let xtal_freq = if RtcClock::estimate_xtal_frequency() > 33 {
+            XtalClock::RtcXtalFreq40M
+        } else {
+            XtalClock::RtcXtalFreq26M
+        };
+
         let pll_freq = PllClock::Pll480MHz;
 
         if cpu_clock_speed.mhz() <= xtal_freq.mhz() {

diff --git a/esp-hal/src/rtc_cntl/mod.rs b/esp-hal/src/rtc_cntl/mod.rs
@@ -383,8 +383,6 @@ impl RtcClock {
             32 => XtalClock::RtcXtalFreq32M,
             #[cfg(any(esp32, esp32c2))]
             26 => XtalClock::RtcXtalFreq26M,
-            #[cfg(esp32)]
-            24 => XtalClock::RtcXtalFreq24M,
             other => XtalClock::RtcXtalFreqOther(other),
         }
     }
@@ -631,9 +629,9 @@ impl RtcClock {
         (100_000_000 * 1000 / period) as u16
     }
 
-    // TODO: implement for ESP32-C6
+    // TODO: implement for ESP32-C6, and H2
     #[cfg(not(any(esp32c6, esp32h2)))]
-    fn estimate_xtal_frequency() -> u32 {
+    pub(crate) fn estimate_xtal_frequency() -> u32 {
         // Number of 8M/256 clock cycles to use for XTAL frequency estimation.
         const XTAL_FREQ_EST_CYCLES: u32 = 10;
 

diff --git a/esp-hal/src/rtc_cntl/rtc/esp32.rs b/esp-hal/src/rtc_cntl/rtc/esp32.rs
@@ -1,25 +1,13 @@
 use strum::FromRepr;
 
 use crate::{
-    clock::XtalClock,
     peripherals::RTC_CNTL,
     rtc_cntl::{RtcCalSel, RtcClock, RtcFastClock, RtcSlowClock},
 };
 
 pub(crate) fn init() {}
 
 pub(crate) fn configure_clock() {
-    #[cfg(feature = "xtal-40mhz")]
-    assert!(matches!(
-        RtcClock::get_xtal_freq(),
-        XtalClock::RtcXtalFreq40M
-    ));
-    #[cfg(feature = "xtal-26mhz")]
-    assert!(
-        matches!(RtcClock::get_xtal_freq(), XtalClock::RtcXtalFreq26M),
-        "Did you flash the right bootloader configured for 26Mhz xtal?"
-    );
-
     RtcClock::set_fast_freq(RtcFastClock::RtcFastClock8m);
 
     let cal_val = loop {

diff --git a/esp-hal/src/rtc_cntl/rtc/esp32c2.rs b/esp-hal/src/rtc_cntl/rtc/esp32c2.rs
@@ -1,7 +1,6 @@
 use strum::FromRepr;
 
 use crate::{
-    clock::XtalClock,
     peripherals::{APB_CTRL, EXTMEM, RTC_CNTL, SPI0, SPI1, SYSTEM},
     regi2c_write_mask,
     rtc_cntl::{RtcCalSel, RtcClock, RtcFastClock, RtcSlowClock},
@@ -56,17 +55,6 @@ pub(crate) fn init() {
 }
 
 pub(crate) fn configure_clock() {
-    #[cfg(feature = "xtal-40mhz")]
-    assert!(matches!(
-        RtcClock::get_xtal_freq(),
-        XtalClock::RtcXtalFreq40M
-    ));
-    #[cfg(feature = "xtal-26mhz")]
-    assert!(
-        matches!(RtcClock::get_xtal_freq(), XtalClock::RtcXtalFreq26M),
-        "Did you flash the right bootloader configured for 26MHz xtal?"
-    );
-
     RtcClock::set_fast_freq(RtcFastClock::RtcFastClock8m);
 
     let cal_val = loop {

diff --git a/esp32-hal/Cargo.toml b/esp32-hal/Cargo.toml
@@ -49,7 +49,7 @@ ssd1306            = "0.8.4"
 static_cell        = { version = "2.0.0", features = ["nightly"] }
 
 [features]
-default = ["embassy-integrated-timers", "rt", "vectored", "xtal-40mhz"]
+default = ["embassy-integrated-timers", "rt", "vectored"]
 
 ## Enable support for using the Bluetooth radio.
 bluetooth = []
@@ -61,10 +61,6 @@ log = ["esp-hal/log", "esp-println/log"]
 rt = ["esp-hal/rt-xtensa"]
 ## Enable interrupt vectoring.
 vectored = ["esp-hal/vectored"]
-## Target device has a 26MHz crystal.
-xtal-26mhz = ["esp-hal/xtal-26mhz"]
-## Target device has a 40MHz crystal.
-xtal-40mhz = ["esp-hal/xtal-40mhz"]
 
 #! ### Trait Implementation Feature Flags
 ## Enable support for asynchronous operation, with interfaces provided by

diff --git a/esp32c2-hal/Cargo.toml b/esp32c2-hal/Cargo.toml
@@ -52,7 +52,7 @@ ssd1306            = "0.8.4"
 static_cell        = { version = "2.0.0", features = ["nightly"] }
 
 [features]
-default = ["embassy-integrated-timers", "rt", "vectored", "xtal-40mhz"]
+default = ["embassy-integrated-timers", "rt", "vectored"]
 
 ## Enable debug features in the HAL (used for development).
 debug = ["esp-hal/debug"]
@@ -66,10 +66,6 @@ log = ["esp-hal/log", "esp-println/log"]
 rt = ["esp-hal/rt-riscv"]
 ## Enable interrupt vectoring.
 vectored = ["esp-hal/vectored"]
-## Target device has a 26MHz crystal.
-xtal-26mhz = ["esp-hal/xtal-26mhz"]
-## Target device has a 40MHz crystal.
-xtal-40mhz = ["esp-hal/xtal-40mhz"]
 
 #! ### Trait Implementation Feature Flags
 ## Enable support for asynchronous operation, with interfaces provided by

diff --git a/esp32c2-hal/build.rs b/esp32c2-hal/build.rs
@@ -1,8 +1,6 @@
 use std::{env, error::Error, path::PathBuf};
 
 fn main() -> Result<(), Box<dyn Error>> {
-    check_features();
-
     // Put the linker script somewhere the linker can find it
     let out = &PathBuf::from(env::var_os("OUT_DIR").unwrap());
     println!("cargo:rustc-link-search={}", out.display());
@@ -16,9 +14,3 @@ fn main() -> Result<(), Box<dyn Error>> {
 
     Ok(())
 }
-
-fn check_features() {
-    if cfg!(feature = "xtal-40mhz") && cfg!(feature = "xtal-26mhz") {
-        panic!("Only one xtal speed feature can be selected");
-    }
-}