Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow setting external 32khz quartz for slow rtc clock #2032

Closed
wants to merge 8 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions esp-hal/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -721,14 +721,19 @@ macro_rules! before_snippet {
use crate::{
clock::{ClockControl, Clocks, CpuClock},
peripherals::Peripherals,
rtc_cntl::rtc::configure_clock,
};

pub use crate::rtc_cntl::rtc::RtcSlowClock;

/// System configuration.
#[non_exhaustive]
#[derive(Default)]
pub struct Config {
/// The CPU clock configuration.
pub cpu_clock: CpuClock,
/// The slow RTC clock configuration. Some options choosed here may require a special bootloader!
pub rtc_slow_clock: RtcSlowClock,
}

/// Initialize the system.
Expand All @@ -737,6 +742,7 @@ pub struct Config {
pub fn init(config: Config) -> (Peripherals, Clocks<'static>) {
let peripherals = Peripherals::take();
let clocks = ClockControl::new(config.cpu_clock).freeze();
configure_clock(config.rtc_slow_clock);

(peripherals, clocks)
}
1 change: 0 additions & 1 deletion esp-hal/src/rtc_cntl/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,6 @@ impl<'d> Rtc<'d> {
/// Optionally an interrupt handler can be bound.
pub fn new(rtc_cntl: impl Peripheral<P = crate::peripherals::LPWR> + 'd) -> Self {
rtc::init();
rtc::configure_clock();

let this = Self {
_inner: rtc_cntl.into_ref(),
Expand Down
47 changes: 40 additions & 7 deletions esp-hal/src/rtc_cntl/rtc/esp32c6.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1198,21 +1198,34 @@ pub(crate) fn init() {
modem_clock_select_lp_clock_source(RadioPeripherals::Wifi, modem_lpclk_src, 0);
}

pub(crate) fn configure_clock() {
pub(crate) fn configure_clock(mut slow_clock: RtcSlowClock) {
assert!(matches!(
RtcClock::get_xtal_freq(),
XtalClock::RtcXtalFreq40M
));

RtcClock::set_fast_freq(RtcFastClock::RtcFastClockRcFast);

let mut c: u8 = 0;
let mut switch = false;
let cal_val = loop {
RtcClock::set_slow_freq(RtcSlowClock::RtcSlowClockRcSlow);
c = c + 1;
RtcClock::set_slow_freq(slow_clock);

let res = RtcClock::calibrate(RtcCalSel::RtcCalRtcMux, 1024);
let res = RtcClock::calibrate(slow_clock.into_cal(), slow_clock.into_cal_cycles());
if res != 0 {
break res;
}
if c > 5 {
if !switch {
switch = true;
slow_clock = RtcSlowClock::default();
c = 0;
warn!("Failed to init 32Khz clock, you need a special bootloader!");
} else {
panic!("Failed to switch back to default rtc clock");
}
}
};

unsafe {
Expand Down Expand Up @@ -1370,10 +1383,11 @@ impl Clock for RtcFastClock {
}

#[allow(clippy::enum_variant_names)]
#[derive(Debug, Clone, Copy, PartialEq)]
#[derive(Debug, Default, Clone, Copy, PartialEq)]
/// RTC SLOW_CLK frequency values
pub(crate) enum RtcSlowClock {
pub enum RtcSlowClock {
/// Select RC_SLOW_CLK as RTC_SLOW_CLK source
#[default]
RtcSlowClockRcSlow = 0,
/// Select XTAL32K_CLK as RTC_SLOW_CLK source
RtcSlowClock32kXtal = 1,
Expand All @@ -1383,6 +1397,25 @@ pub(crate) enum RtcSlowClock {
RtcSlowOscSlow = 3,
}

impl RtcSlowClock {
fn into_cal(&self) -> RtcCalSel {
match self {
RtcSlowClock::RtcSlowClockRcSlow => RtcCalSel::RtcCalRtcMux,
RtcSlowClock::RtcSlowClock32kXtal => RtcCalSel::RtcCal32kXtal,
RtcSlowClock::RtcSlowClock32kRc => RtcCalSel::RtcCal32kRc,
RtcSlowClock::RtcSlowOscSlow => RtcCalSel::RtcCal32kOscSlow,
}
}
fn into_cal_cycles(&self) -> u32 {
match self {
RtcSlowClock::RtcSlowClockRcSlow => 1024,
RtcSlowClock::RtcSlowClock32kXtal => 3000,
RtcSlowClock::RtcSlowClock32kRc => 3000,
RtcSlowClock::RtcSlowOscSlow => 3000, // Not sure https://docs.espressif.com/projects/esp-idf/en/stable/esp32s3/api-reference/kconfig.html#config-rtc-clk-cal-cycles
}
}
}

impl Clock for RtcSlowClock {
fn frequency(&self) -> HertzU32 {
match self {
Expand Down Expand Up @@ -1498,15 +1531,15 @@ impl RtcClock {

crate::rom::ets_delay_us(3);
}

/// Calibration of RTC_SLOW_CLK is performed using a special feature of
/// TIMG0. This feature counts the number of XTAL clock cycles within a
/// given number of RTC_SLOW_CLK cycles.
pub(crate) fn calibrate_internal(mut cal_clk: RtcCalSel, slowclk_cycles: u32) -> u32 {
const SOC_CLK_RC_FAST_FREQ_APPROX: u32 = 17_500_000;
const SOC_CLK_RC_SLOW_FREQ_APPROX: u32 = 136_000;
const SOC_CLK_XTAL32K_FREQ_APPROX: u32 = 32768;

if cal_clk == RtcCalSel::RtcCalRtcMux {
cal_clk = match cal_clk {
RtcCalSel::RtcCalRtcMux => match RtcClock::get_slow_freq() {
Expand Down
50 changes: 50 additions & 0 deletions examples/src/bin/32khz_xtal.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
//! This turns on the 32 khz xtal and then just prints hello world to UART. Requires a special bootloader
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see the value in this test. For one, what's "a special bootloader" that is required for it? Next, what does this example show, that changing the clock source doesn't break the whole system?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The bootloader needs to be build with CONFIG_RTC_CLK_SRC_EXT_CRYS=y

Well yes, also allows to measure the sine wave with an osciloscope

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we select the clock source here, instead of relying on the bootloader?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if (!rtc_clk_32k_enabled()) {

Unless that takes long to decide, this is safe to do. If the bootloader enabled the clock source, and the user selects in in esp-hap, we can skip initialization. Otherwise, in the future we might allow other bootloaders which may or may not set up the clock source, so I'd say let's enable the clock ourselves, if it's not done already.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please note it's inside the macro statement #if CONFIG_ESP_SYSTEM_RTC_EXT_XTAL

Well maybe it is safe but we lose some optimization, I don't think we should lose any performance because at default we don't control the bootloader fully. In the future we probably will by using a rust based bootloader so it will work the same just like now, but without the hassle of recompiling it

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we should lose any performance because at default we don't control the bootloader fully

Let's prefer the option that works by default. If the user has problems with the startup time, they can use a custom bootloader.

Copy link
Contributor Author

@Szybet Szybet Sep 6, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Making it work only in the application would require changing the code for it to wait to start up, not only is it not efficient but also I don't know how to do it, I looked at a lot of esp-idf code and haven't really found anything that waits for it, I ques booting the bootloader is enough time for it to start after all. Using a fixed time is also bad, would require cpu dependent testing to optimize it.

This is simply not the way to do it...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fine. At least please print a warning or panic if the bootloader forgot to enable the clock source.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I will try to find a way to detect it (probably just failing the calibration step)

//!
//! You can see the output with `espflash` if you provide the `--monitor`
//! option.

//% CHIPS: esp32 esp32c2 esp32c3 esp32c6 esp32h2 esp32s2 esp32s3

#![no_std]
#![no_main]

use core::fmt::Write;

use esp_backtrace as _;
use esp_hal::{delay::Delay, gpio::Io, prelude::*, uart::Uart, RtcSlowClock};

#[entry]
fn main() -> ! {
let mut conf = esp_hal::Config::default();
conf.rtc_slow_clock = RtcSlowClock::RtcSlowClock32kXtal;
let (peripherals, clocks) = esp_hal::init(conf);

let delay = Delay::new(&clocks);

let io = Io::new(peripherals.GPIO, peripherals.IO_MUX);

// Default pins for Uart/Serial communication
cfg_if::cfg_if! {
if #[cfg(feature = "esp32")] {
let (mut tx_pin, mut rx_pin) = (io.pins.gpio1, io.pins.gpio3);
} else if #[cfg(feature = "esp32c2")] {
let (mut tx_pin, mut rx_pin) = (io.pins.gpio20, io.pins.gpio19);
} else if #[cfg(feature = "esp32c3")] {
let (mut tx_pin, mut rx_pin) = (io.pins.gpio21, io.pins.gpio20);
} else if #[cfg(feature = "esp32c6")] {
let (mut tx_pin, mut rx_pin) = (io.pins.gpio16, io.pins.gpio17);
} else if #[cfg(feature = "esp32h2")] {
let (mut tx_pin, mut rx_pin) = (io.pins.gpio24, io.pins.gpio23);
} else if #[cfg(any(feature = "esp32s2", feature = "esp32s3"))] {
let (mut tx_pin, mut rx_pin) = (io.pins.gpio43, io.pins.gpio44);
}
}

let mut uart0 =
Uart::new_with_default_pins(peripherals.UART0, &clocks, &mut tx_pin, &mut rx_pin).unwrap();

loop {
writeln!(uart0, "Hello world!").unwrap();
delay.delay(1.secs());
}
}