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
2025-09-28 21:00:59 +00:00 · 2024-02-14 12:09:35 +01:00 · 2024-02-14 12:09:35 +01:00 · 32824422a1
commit 32824422a1
parent 2b8db5c2c2
14 changed files with 42 additions and 138 deletions
--- 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)
--- 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

--- 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"]
--- 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"]
--- 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")]
--- 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;
--- 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,29 +352,22 @@ 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.
@ -384,12 +375,12 @@ impl<'d> ClockControl<'d> {
        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() {
--- 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;

--- a/esp-hal/src/rtc_cntl/rtc/esp32.rs
+++ b/esp-hal/src/rtc_cntl/rtc/esp32.rs
@ -1,7 +1,6 @@
 use strum::FromRepr;

 use crate::{
-    clock::XtalClock,
    peripherals::RTC_CNTL,
    rtc_cntl::{RtcCalSel, RtcClock, RtcFastClock, RtcSlowClock},
 };
@ -9,17 +8,6 @@ use crate::{
 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 {
--- 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 {
--- 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
--- 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
--- 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");
-    }
-}
--- a/xtask/src/lib.rs
+++ b/xtask/src/lib.rs
@ -124,22 +124,12 @@ pub fn build_documentation(

    log::info!("Building '{package_name}' documentation targeting '{chip}'");

-    let mut features = vec![chip.to_string()];
-
-    // The ESP32 and ESP32-C2 must have their Xtal frequencies explicitly stated
-    // when using `esp-hal` or `esp-hal-smartled`:
-    use Chip::*;
-    use Package::*;
-    if matches!(chip, Esp32 | Esp32c2) && matches!(package, EspHal | EspHalSmartled) {
-        features.push("xtal-40mhz".into());
-    }
-
    // Build up an array of command-line arguments to pass to `cargo doc`:
    let mut args = vec![
        "doc".into(),
        "-Zbuild-std=core".into(), // Required for Xtensa, for some reason
        format!("--target={target}"),
-        format!("--features={}", features.join(",")),
+        format!("--features={}", chip.to_string()),
    ];
    if open {
        args.push("--open".into());