Skip to content

Commit f829c8f

Browse files
Fix C2 delays (#1981)
* Re-enable delay tests on S2 and C2 * Systimer: use fn instead of constant to retrieve tick freq * Reformulate delay using current_time * Take actual XTAL into account * Re-enable tests * Fix changelog * Disable defmt * Remove unused esp32 code * Update esp-hal/src/delay.rs Co-authored-by: Jesse Braham <[email protected]> --------- Co-authored-by: Jesse Braham <[email protected]>
1 parent 69cf454 commit f829c8f

File tree

10 files changed

+117
-108
lines changed

10 files changed

+117
-108
lines changed

esp-hal/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -287,6 +287,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
287287
- Auto detect crystal frequency based on `RtcClock::estimate_xtal_frequency()` (#1165)
288288
- ESP32-S3: Configure 32k ICACHE (#1169)
289289
- Lift the minimal buffer size requirement for I2S (#1189)
290+
- Replaced `SystemTimer::TICKS_PER_SEC` with `SystemTimer::ticks_per_sec()` (#1981)
290291

291292
### Removed
292293

esp-hal/src/clock/mod.rs

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,8 @@
7171
//! ```
7272
7373
use fugit::HertzU32;
74+
#[cfg(esp32c2)]
75+
use portable_atomic::{AtomicU32, Ordering};
7476

7577
#[cfg(any(esp32, esp32c2))]
7678
use crate::rtc_cntl::RtcClock;
@@ -333,6 +335,26 @@ pub struct RawClocks {
333335
pub pll_96m_clock: HertzU32,
334336
}
335337

338+
cfg_if::cfg_if! {
339+
if #[cfg(esp32c2)] {
340+
static XTAL_FREQ_MHZ: AtomicU32 = AtomicU32::new(40);
341+
342+
pub(crate) fn xtal_freq_mhz() -> u32 {
343+
XTAL_FREQ_MHZ.load(Ordering::Relaxed)
344+
}
345+
} else if #[cfg(esp32h2)] {
346+
pub(crate) fn xtal_freq_mhz() -> u32 {
347+
32
348+
}
349+
} else if #[cfg(any(esp32, esp32s2))] {
350+
// Function would be unused
351+
} else {
352+
pub(crate) fn xtal_freq_mhz() -> u32 {
353+
40
354+
}
355+
}
356+
}
357+
336358
/// Used to configure the frequencies of the clocks present in the chip.
337359
///
338360
/// After setting all frequencies, call the freeze function to apply the
@@ -429,6 +451,7 @@ impl<'d> ClockControl<'d> {
429451
} else {
430452
26
431453
};
454+
XTAL_FREQ_MHZ.store(xtal_freq, Ordering::Relaxed);
432455

433456
ClockControl {
434457
_private: clock_control.into_ref(),
@@ -452,6 +475,7 @@ impl<'d> ClockControl<'d> {
452475
} else {
453476
XtalClock::RtcXtalFreq26M
454477
};
478+
XTAL_FREQ_MHZ.store(xtal_freq.mhz(), Ordering::Relaxed);
455479

456480
let pll_freq = PllClock::Pll480MHz;
457481

esp-hal/src/delay.rs

Lines changed: 38 additions & 84 deletions
Original file line numberDiff line numberDiff line change
@@ -30,36 +30,25 @@
3030
//! [DelayUs]: embedded_hal_02::blocking::delay::DelayUs
3131
//! [embedded-hal]: https://docs.rs/embedded-hal/1.0.0/embedded_hal/delay/index.html
3232
33-
use fugit::HertzU64;
3433
pub use fugit::MicrosDurationU64;
3534

35+
use crate::clock::Clocks;
36+
3637
/// Delay driver
3738
///
3839
/// Uses the `SYSTIMER` peripheral internally for RISC-V devices, and the
3940
/// built-in Xtensa timer for Xtensa devices.
4041
#[derive(Clone, Copy)]
41-
pub struct Delay {
42-
freq: HertzU64,
43-
}
44-
45-
impl Delay {
46-
/// Delay for the specified number of milliseconds
47-
pub fn delay_millis(&self, ms: u32) {
48-
for _ in 0..ms {
49-
self.delay_micros(1000);
50-
}
51-
}
52-
}
42+
#[non_exhaustive]
43+
pub struct Delay;
5344

5445
#[cfg(feature = "embedded-hal-02")]
5546
impl<T> embedded_hal_02::blocking::delay::DelayMs<T> for Delay
5647
where
5748
T: Into<u32>,
5849
{
5950
fn delay_ms(&mut self, ms: T) {
60-
for _ in 0..ms.into() {
61-
self.delay_micros(1000);
62-
}
51+
self.delay_millis(ms.into());
6352
}
6453
}
6554

@@ -80,83 +69,48 @@ impl embedded_hal::delay::DelayNs for Delay {
8069
}
8170
}
8271

83-
#[cfg(riscv)]
84-
mod implementation {
85-
use super::*;
86-
use crate::{clock::Clocks, timer::systimer::SystemTimer};
87-
88-
impl Delay {
89-
/// Create a new `Delay` instance
90-
pub fn new(clocks: &Clocks<'_>) -> Self {
91-
// The counters and comparators are driven using `XTAL_CLK`.
92-
// The average clock frequency is fXTAL_CLK/2.5, which is 16 MHz.
93-
// The timer counting is incremented by 1/16 μs on each `CNT_CLK` cycle.
94-
Self {
95-
#[cfg(not(esp32h2))]
96-
freq: HertzU64::MHz(clocks.xtal_clock.to_MHz() as u64 * 10 / 25),
97-
#[cfg(esp32h2)]
98-
// esp32h2 TRM, section 11.2 Clock Source Selection
99-
freq: HertzU64::MHz(clocks.xtal_clock.to_MHz() as u64 * 10 / 20),
100-
}
101-
}
102-
103-
/// Delay for the specified time
104-
pub fn delay(&self, time: MicrosDurationU64) {
105-
let t0 = SystemTimer::now();
106-
let rate: HertzU64 = MicrosDurationU64::from_ticks(1).into_rate();
107-
let clocks = time.ticks() * (self.freq / rate);
72+
impl Delay {
73+
/// Creates a new `Delay` instance.
74+
// Do not remove the argument, it makes sure that the clocks are initialized.
75+
pub fn new(_clocks: &Clocks<'_>) -> Self {
76+
Self {}
77+
}
10878

109-
while SystemTimer::now().wrapping_sub(t0) & SystemTimer::BIT_MASK <= clocks {}
110-
}
79+
/// Delay for the specified time
80+
pub fn delay(&self, delay: MicrosDurationU64) {
81+
let start = crate::time::current_time();
11182

112-
/// Delay for the specified number of microseconds
113-
pub fn delay_micros(&self, us: u32) {
114-
let t0 = SystemTimer::now();
115-
let clocks = us as u64 * (self.freq / HertzU64::MHz(1));
83+
while elapsed_since(start) < delay {}
84+
}
11685

117-
while SystemTimer::now().wrapping_sub(t0) & SystemTimer::BIT_MASK <= clocks {}
86+
/// Delay for the specified number of milliseconds
87+
pub fn delay_millis(&self, ms: u32) {
88+
for _ in 0..ms {
89+
self.delay_micros(1000);
11890
}
91+
}
11992

120-
/// Delay for the specified number of nanoseconds
121-
pub fn delay_nanos(&self, ns: u32) {
122-
let t0 = SystemTimer::now();
123-
let clocks = ns as u64 * (self.freq / HertzU64::MHz(1)) / 1000;
93+
/// Delay for the specified number of microseconds
94+
pub fn delay_micros(&self, us: u32) {
95+
let delay = MicrosDurationU64::micros(us as u64);
96+
self.delay(delay);
97+
}
12498

125-
while SystemTimer::now().wrapping_sub(t0) & SystemTimer::BIT_MASK <= clocks {}
126-
}
99+
/// Delay for the specified number of nanoseconds
100+
pub fn delay_nanos(&self, ns: u32) {
101+
let delay = MicrosDurationU64::nanos(ns as u64);
102+
self.delay(delay);
127103
}
128104
}
129105

130-
#[cfg(xtensa)]
131-
mod implementation {
132-
use super::*;
133-
use crate::clock::Clocks;
134-
135-
impl Delay {
136-
/// Create a new `Delay` instance
137-
pub fn new(clocks: &Clocks<'_>) -> Self {
138-
Self {
139-
freq: clocks.cpu_clock.into(),
140-
}
141-
}
142-
143-
/// Delay for the specified time
144-
pub fn delay(&self, time: MicrosDurationU64) {
145-
let rate: HertzU64 = MicrosDurationU64::from_ticks(1).into_rate();
146-
let clocks = time.ticks() * (self.freq / rate);
147-
xtensa_lx::timer::delay(clocks as u32);
148-
}
149-
150-
/// Delay for the specified number of microseconds
151-
pub fn delay_micros(&self, us: u32) {
152-
let clocks = us as u64 * (self.freq / HertzU64::MHz(1));
153-
xtensa_lx::timer::delay(clocks as u32);
154-
}
106+
fn elapsed_since(start: fugit::Instant<u64, 1, 1_000_000>) -> MicrosDurationU64 {
107+
let now = crate::time::current_time();
155108

156-
/// Delay for the specified number of nanoseconds
157-
pub fn delay_nanos(&self, ns: u32) {
158-
let clocks = ns as u64 * (self.freq / HertzU64::MHz(1)) / 1000;
159-
xtensa_lx::timer::delay(clocks as u32);
160-
}
109+
if start.ticks() <= now.ticks() {
110+
now - start
111+
} else {
112+
// current_time specifies at least 7 happy years, let's ignore this issue for
113+
// now.
114+
panic!("Time has wrapped around, which we currently don't handle");
161115
}
162116
}

esp-hal/src/time.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ pub fn current_time() -> fugit::Instant<u64, 1, 1_000_000> {
5050
let ticks = crate::timer::systimer::SystemTimer::now();
5151
(
5252
ticks,
53-
(crate::timer::systimer::SystemTimer::TICKS_PER_SECOND / 1_000_000),
53+
(crate::timer::systimer::SystemTimer::ticks_per_second() / 1_000_000),
5454
)
5555
};
5656

esp-hal/src/timer/systimer.rs

Lines changed: 24 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -112,20 +112,37 @@ impl<'d> SystemTimer<'d> {
112112
if #[cfg(esp32s2)] {
113113
/// Bitmask to be applied to the raw register value.
114114
pub const BIT_MASK: u64 = u64::MAX;
115-
/// The ticks per second the underlying peripheral uses.
116-
pub const TICKS_PER_SECOND: u64 = 80_000_000;
117115
// Bitmask to be applied to the raw period register value.
118116
const PERIOD_MASK: u64 = 0x1FFF_FFFF;
119117
} else {
120118
/// Bitmask to be applied to the raw register value.
121119
pub const BIT_MASK: u64 = 0xF_FFFF_FFFF_FFFF;
122-
/// The ticks per second the underlying peripheral uses.
123-
pub const TICKS_PER_SECOND: u64 = 16_000_000;
124120
// Bitmask to be applied to the raw period register value.
125121
const PERIOD_MASK: u64 = 0x3FF_FFFF;
126122
}
127123
}
128124

125+
/// Returns the tick frequency of the underlying timer unit.
126+
pub fn ticks_per_second() -> u64 {
127+
cfg_if::cfg_if! {
128+
if #[cfg(esp32s2)] {
129+
80_000_000
130+
} else if #[cfg(esp32h2)] {
131+
// The counters and comparators are driven using `XTAL_CLK`.
132+
// The average clock frequency is fXTAL_CLK/2, which is 16 MHz.
133+
// The timer counting is incremented by 1/16 μs on each `CNT_CLK` cycle.
134+
const MULTIPLIER: u64 = 10_000_000 / 20;
135+
crate::clock::xtal_freq_mhz() as u64 * MULTIPLIER
136+
} else {
137+
// The counters and comparators are driven using `XTAL_CLK`.
138+
// The average clock frequency is fXTAL_CLK/2.5, which is 16 MHz.
139+
// The timer counting is incremented by 1/16 μs on each `CNT_CLK` cycle.
140+
const MULTIPLIER: u64 = 10_000_000 / 25;
141+
crate::clock::xtal_freq_mhz() as u64 * MULTIPLIER
142+
}
143+
}
144+
}
145+
129146
/// Create a new instance.
130147
pub fn new(_systimer: impl Peripheral<P = SYSTIMER> + 'd) -> Self {
131148
#[cfg(soc_etm)]
@@ -810,7 +827,7 @@ where
810827
}
811828

812829
let us = period.ticks();
813-
let ticks = us * (SystemTimer::TICKS_PER_SECOND / 1_000_000) as u32;
830+
let ticks = us * (SystemTimer::ticks_per_second() / 1_000_000) as u32;
814831

815832
self.comparator.set_mode(ComparatorMode::Period);
816833
self.comparator.set_period(ticks);
@@ -879,7 +896,7 @@ where
879896
}
880897
};
881898

882-
let us = ticks / (SystemTimer::TICKS_PER_SECOND / 1_000_000);
899+
let us = ticks / (SystemTimer::ticks_per_second() / 1_000_000);
883900

884901
Instant::<u64, 1, 1_000_000>::from_ticks(us)
885902
}
@@ -888,7 +905,7 @@ where
888905
let mode = self.comparator.get_mode();
889906

890907
let us = value.ticks();
891-
let ticks = us * (SystemTimer::TICKS_PER_SECOND / 1_000_000);
908+
let ticks = us * (SystemTimer::ticks_per_second() / 1_000_000);
892909

893910
if matches!(mode, ComparatorMode::Period) {
894911
// Period mode

examples/src/bin/systimer.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -74,11 +74,11 @@ fn main() -> ! {
7474
alarm0.enable_interrupt(true);
7575

7676
alarm1.set_interrupt_handler(systimer_target1);
77-
alarm1.set_target(SystemTimer::now() + (SystemTimer::TICKS_PER_SECOND * 2));
77+
alarm1.set_target(SystemTimer::now() + (SystemTimer::ticks_per_second() * 2));
7878
alarm1.enable_interrupt(true);
7979

8080
alarm2.set_interrupt_handler(systimer_target2);
81-
alarm2.set_target(SystemTimer::now() + (SystemTimer::TICKS_PER_SECOND * 3));
81+
alarm2.set_target(SystemTimer::now() + (SystemTimer::ticks_per_second() * 3));
8282
alarm2.enable_interrupt(true);
8383

8484
ALARM0.borrow_ref_mut(cs).replace(alarm0);

hil-test/tests/delay.rs

Lines changed: 23 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
//! Delay Test
22
3-
// esp32c2 is disabled currently as it fails
4-
//% CHIPS: esp32 esp32c3 esp32c6 esp32s3
3+
//% CHIPS: esp32 esp32c2 esp32c3 esp32c6 esp32s2 esp32s3
54

65
#![no_std]
76
#![no_main]
@@ -37,25 +36,33 @@ mod tests {
3736
}
3837

3938
#[test]
40-
#[timeout(1)]
39+
#[timeout(2)]
4140
fn delay_ns(mut ctx: Context) {
4241
let t1 = esp_hal::time::current_time();
4342
ctx.delay.delay_ns(600_000_000);
4443
let t2 = esp_hal::time::current_time();
4544

4645
assert!(t2 > t1);
47-
assert!((t2 - t1).to_nanos() >= 600_000_000u64);
46+
assert!(
47+
(t2 - t1).to_nanos() >= 600_000_000u64,
48+
"diff: {:?}",
49+
(t2 - t1).to_nanos()
50+
);
4851
}
4952

5053
#[test]
51-
#[timeout(1)]
54+
#[timeout(2)]
5255
fn delay_700millis(ctx: Context) {
5356
let t1 = esp_hal::time::current_time();
5457
ctx.delay.delay_millis(700);
5558
let t2 = esp_hal::time::current_time();
5659

5760
assert!(t2 > t1);
58-
assert!((t2 - t1).to_millis() >= 700u64);
61+
assert!(
62+
(t2 - t1).to_millis() >= 700u64,
63+
"diff: {:?}",
64+
(t2 - t1).to_millis()
65+
);
5966
}
6067

6168
#[test]
@@ -66,7 +73,11 @@ mod tests {
6673
let t2 = esp_hal::time::current_time();
6774

6875
assert!(t2 > t1);
69-
assert!((t2 - t1).to_micros() >= 1_500_000u64);
76+
assert!(
77+
(t2 - t1).to_micros() >= 1_500_000u64,
78+
"diff: {:?}",
79+
(t2 - t1).to_micros()
80+
);
7081
}
7182

7283
#[test]
@@ -77,6 +88,10 @@ mod tests {
7788
let t2 = esp_hal::time::current_time();
7889

7990
assert!(t2 > t1);
80-
assert!((t2 - t1).to_millis() >= 3000u64);
91+
assert!(
92+
(t2 - t1).to_millis() >= 3000u64,
93+
"diff: {:?}",
94+
(t2 - t1).to_millis()
95+
);
8196
}
8297
}

hil-test/tests/embassy_timers_executors.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
//! Embassy timer and executor Test
22
3-
// esp32c2 is disabled currently as it fails
4-
//% CHIPS: esp32 esp32c3 esp32c6 esp32h2 esp32s3
3+
//% CHIPS: esp32 esp32c2 esp32c3 esp32c6 esp32h2 esp32s2 esp32s3
54

65
#![no_std]
76
#![no_main]

0 commit comments

Comments
 (0)