[Idea] Device agnostic access to the core clock frequency

[japaric]

NOTE I don't think the idea presented here should live in this crate, but it could live in a crate maintained by the HAL team. In any case, the only reason I'm posting this here is that it's more likely to be seen by authors of HAL crates, but feel free to move this thread elsewhere.

Goal

Expose the core clock frequency in a device agnostic manner for use in device agnostic APIs. This piece of information is required to translate human time (e.g. milliseconds) into "core clock cycles" which is what time-related peripherals, like timers, work with.

Motivation

The cortex-m crate provides an asm::delay(x) API to perform a delay of x clock cycles. Application authors usually don't (directly or indirectly) use that API because they want an API that works with human time like milliseconds.

Although asm::delay is device agnostic (but not architecture agnostic) it's not possible to create a device agnostic asm::delay_ms on top of it because the core clock (CPU) frequency can't be accessed in a device agnostic manner. The result? Code duplication: HAL crates end up providing their own delay_ms API on top of asm::delay or cortex_m::SYST that takes or interacts with some struct that represents the clock configuration of the device.

With this proposal we could create a cortex-m-hal crate that depends on cortex-m and provides a device agnostic function like delay_ms(x: u32).

Design

We use the global singleton pattern to implement a crate with zero dependencies that provides access to the core clock frequency.

// crate: core-frequency

use core::sync::atomic::{AtomicU32, Ordering};

#[macro_export]
macro_rules! initial {
    ($initial:expr) => {
        // TODO use a symbol name less likely to cause a collision
        #[no_mangle]
        static CORE_FREQUENCY: core::sync::atomic::AtomicU32 =
            core::sync::atomic::AtomicU32::new($initial);

        // IMPORTANT never expose this to application authors; they shouldn't be
        // directly manipulating the core clock frequency
        pub fn set_core_frequency(freq: u32) {
            CORE_FREQUENCY.store(freq, core::sync::atomic::Ordering::Release)
        }
    };
}

/// Returns the core clock frequency in Hz
pub fn get() -> u32 {
    extern "C" {
        static CORE_FREQUENCY: AtomicU32;
    }

    unsafe { CORE_FREQUENCY.load(Ordering::Acquire) }
}

Then we can create a crate like cortex-m-hal to provide methods like delay_ms.

pub fn delay_ms(ms: u32) {
    let cycles = (core_frequency::get() / 1_000) * ms;

    cortex_m::asm::delay(cycles);
}

pub struct AsmDelay;

impl embedded_hal::DelayMs for AsmDelay {
    pub fn delay_ms(ms: u32) {
        delay_ms(ms);
    }
}

pub struct SystDelay(SYST);

impl embedded_hal::DelayMs for SystDelay {
    pub fn delay_ms(ms: u32) {
        let cycles = (core_frequency::get() / 1_000) * ms;

         // ..
    }
}

HAL crates would integrate with core_frequency like this:

// crate: stm32fxyz-hal

// the core clock frequency at reset (usually an internal RC clock)
core_frequency::initial!(8_000_000);
//~^ if the device needs an external oscillator then this need to be called by
// the end user (application author)

// Singleton proxy for the clock peripheral
pub struct ClockPeripheral {
    _0: (),
}

// omitted: passing configuration like prescalers as arguments
//
// IMPORTANT: here you *need* a mutable (unique) reference to the clock
// peripheral to ensure that  `CORE_FREQUENCY` will be set "atomically" with all
// the clock registers.
//
// "Why?" Imagine what would happen if you get preempted at spot (A) by an
// interrupt and the handler calls `configure_clocks`. If `configure_clocks`
// takes `&mut ClockPeripheral` that can't happen unless someone breaks Rust
// aliasing rules
pub fn configure_clocks(clock: &mut ClockPeripheral) {
    // .. do stuff with registers ..

    // (A)

    // then
    set_core_frequency(frequency_computed_above);
}

Other thoughts

This only uses atomic stores / loads so it also works with architectures that don't have CAS instructions, like ARMv6-M and MSP430.

A few HAL APIs could be simplified with this API: for example the clocks argument could be removed from stm32f1xx::MonoTimer constructor.

The core_frequency crate plus a few more atomics (for the prescalers) could replace the stm32f1xx::Clocks struct. This change would reduce the number of arguments of many constructors by one.

// APB1 pre-scaler
static PPRE1: AtomicU8 = AtomicU8::new(1);

impl Serial1 {
    pub fn new(
        usart1: USART1,
        // clocks: Clocks, // no longer required
        // ..
    ) {
        // APB1 clock frequency
        let apb1 = core_clock::get() / u32::from(PPRE1.load(Ordering::Acquire));

        // ..
    }
}

RTFM could use this to provide a variation of the schedule API that works with human time (the current API works with core clock cycles).

Suggestion: do not make the cortex-m crate depend on core-frequency. That would make users of cortex-m run into weird linker errors ("undefined reference to CORE_FREQUENCY") if they are not using a HAL crate that integrates with core-frequency. Also it would make the versioning of the cortex-m crate harder: bumping the minor of version core-frequency would require bumping the minor version of the cortex-m crate.

Suggestion: do not make embedded-hal depend on core-frequency. embedded-hal is a pure crate that mainly contains interfaces; making it depend on core-frequency would add "impurity" to it making it depend on a symbol defined elsewhere. It also would make versioning harder.

[mathk]

@japaric this quit related to the intention I had behind my ticklock and my branch on cortex-m I would be happy to give an help on that issue.

[little-arhat]

On the somewhat related note, maybe it's time to move time.rs that is currently copied to almost every hal implementation somewhere? I found https://github.com/dfrankland/bitrate, but maybe @rust-embedded should provide smth strandard.

[dbrgn]

I would love to see something like this. Has there been any discussion / progress on this topic so far outside of this GitHub issue?

[PTaylor-us]

I would also like to see this abstraction. The Duration struct from time.rs is now (as of 0.2.10) usable in embedded contexts. I would like to have an architecture/device agnostic way to retrieve the clock frequency. I believe this would enable crates like RTFM to allow scheduling and other time-related operations be done in human time rather than clock cycles.

[tib888]

I also feel the need for a good abstaraction over durations, instant, delays, clock freq., etc.

For example these existing solutions have big overlap:
https://github.com/rtfm-rs/cortex-m-rtfm/blob/master/src/cyccnt.rs
https://github.com/stm32-rs/stm32f1xx-hal/blob/master/src/time.rs

I like the cyccnt.rs more, but it is still not good/generic enough (so I have my own version).

In my opinion we need an Instant and a Duration with meaningful arithmetic operations implemented on them (like in cyccnt.rs above). But this implementation should be generic over the counter data type (u32 / u64) depending on the performance needs) and the unit (with a macro implementation for cycles / us / ms / s / etc.).

If one wants to work with few us accuracy (like one-wire protocol) he will not have the luxury to work with 64 bit counters, and more expensive div/mul on a 32 bit processor, also he wants to count with every clock. This of course means that on a 72Mhz clock rate he will get a counter overflow in every minute, so in his arithmetic, the choice between the add / wrapping add / saturating add is pretty important.

On the other hand other things can be fine with 'ms' or sec accuracy and with 64 bit counters so no need to worry about overflows.

Of course there is a need to sometimes jump from one time/duration unit to an other and in this case the clock freq may be needed to implement From / Into. Accessing it "globally" is much better than to store it in every Duration struct instance, or requiring it as a parameter at every conversion.

[PTaylor-us]

What I've been currently pursuing is an Instant struct with a nanoseconds: i64 field and a const Ratio to where the the tick count (from my underlying counter/timer) * the Ratio = nanoseconds. I also copied time.rs's duration literals (numberical_traits) where 2.seconds() creates a Duration of 2 seconds (obvs). At this point, any Instant and Duration is stored as nanoseconds.

One requirement for me is to have signed Durations.

I have used C++'s std::chrono library extensively over the years and would like to have something at least as easy to use as that.

@tib888 This certainly wouldn't be a good solution for something fast like you mentioned, but seems to work well as a general purpose system time solution. I'm a new Rustacean, but I'm going to try to wrap my head around what it might look like to convert between different time bases.

[PTaylor-us]

I've put my current effort up here: embedded-time. Comments, suggestions, and PRs are welcome.

[PTaylor-us]

I've put my current effort up here: embedded-time. Comments, suggestions, and PRs are welcome.

I've looked at several approaches including const generics and others. At the moment my ratiometric branch seems the most promising. It can (or hopefully will be able to) store the value in either i32 or i64 and have a const num-rational::Ratio<i32> serving as both a tag for the unit of the duration, and also allowing conversion between units (C++'s std::chrono does nearly the identical structure except it actually does use const generics).