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

RFC: New clocking API #429

Closed
glaeqen opened this issue Apr 16, 2021 · 5 comments
Closed

RFC: New clocking API #429

glaeqen opened this issue Apr 16, 2021 · 5 comments

Comments

@glaeqen
Copy link
Contributor

glaeqen commented Apr 16, 2021

Summary

New clocking API allowing to build a typesafe chain of dependent clock sources, generic clock generators and peripheral channels.

Motivation

Current clocking API assumes the following workflow:

let mut clocks = GenericClockController::with_{external,internal}_32kosc(...PAC...);
let gclk9 = clocks.configure_gclk_divider_and_source(ClockGenId::GCLK9, 1, ClockSource::DFLL48M, false);
let adc0_clock: Adc0Clock = clocks.adc0(&gclk9);

This API has the following limitations.

  1. By default the GenericClockController constructor configures and provides an opinionated set of clock sources and generic clock generators that cannot be changed afterwards. Following clock configuration is set for thumbv7em.
    • GCLK0 (Generic Clock Generator 0) is driven by DFLL48M (48MHz) through GCLK5 (divider: 24 -> 2MHz) through PCh1 (Peripheral Channel 1) through DPLL0 (multiplier: 60 -> 120MHz)
    • GCLK1 is driven either by XOSC32K (use_external_32kosc) or OSCULP32K (use_internal_32kosc)
  2. It is not possible to set up additional clock sources within the framework of current clocking API (without hacking around PACs).
  3. Once you setup the Clock Source - GCLK pair, it is impossible to change it later.
  4. Once you setup the GCLK - PeripheralChannel pair, it is impossible to change it later.

Main clock locked to 120MHz by HAL, even though being acceptable in basic scenarios, is a serious design flaw that severely diminishes flexibility of a HAL and might either encourage users to hack unsafe solutions on top of existing abstractions or discourage them from using the HAL altogether.

Having these points in mind and also the fact that current clocking API implementation would benefit from refactoring anyway, striving for improvement seems to be fully motivated and justified.

Detailed design

Introduction

ATSAMD clocking system assumes 3 types of major building blocks:

  • Clock sources (referred to further on by ClkSrc)
  • Generic Clock Generators (referred to further on by GClk)
  • Peripheral Channels (referred to further on by PCh)

Properties of ATSAMD clocking system:

  • GClks depend on ClkSrcs in a N:1 relationship
  • PChs depend on GClks in a M:1 relationship
  • Some ClkSrcs can depend on some PChs
    Specifically:
    • For thumbv7em-based MCUs
      • PCh:PCh0 can serve as a reference clock provider for ClkSrc:DFLL48M
      • PCh:PCh1 can serve as a reference clock provider for ClkSrc:DPLL0
      • PCh:PCh2 can serve as a reference clock provider for ClkSrc:DPLL1
      • PCh:PCh3? Take a look at #unresolved-questions
    • For thumbv6m-based MCUs
      • PCh:PCh0 (on thumbv6m Peripheral Channels are called GCLK Multiplexers) can serve as a reference for ClkSrc:DFLL48M
  • Some ClkSrc can depend on some other ClkSrcs
    Specifically
    • For thumbv7em-based MCUs
      • 32Khz signal of ClkSrc:XOSC32K can serve as a clock source for ClkSrc:DPLL{0, 1}
      • ClkSrc:XOSC{0, 1} can serve as a clock source for ClkSrc:DPLL{0, 1}

Mapping HW model to Rust

Updated: 2021-06-08

In order to model one-to-many type of relationship between dependencies a few additional concepts/abstractions were introduced.

Enabled type and its helper types/traits

Enabled type wrapper represents a clocking component in its enabled state while also holding an information about current amount of dependencies (usually 0 upon construction). This amount of dependencies is embedded into the type via second generic parameter leveraging typenum::{UInt, UTerm} types.

pub trait Counter {} /* implemented for every `typenum::Unsigned` */

pub trait Increment: Counter {
    /* implemented for every `typenum::Unsigned` and `Enabled` */
    type Inc: Counter;
    fn inc(self) -> Self::Inc;
}

pub trait Decrement: Counter {
    /* implemented for every `typenum::Unsigned` and `Enabled` */
    type Dec: Counter;
    fn dec(self) -> Self::Dec;
}

pub struct Enabled<T, N: Counter>(pub(crate) T, PhantomData<N>);

Via specialized implementation blocks for this type (like for Enabled<Gclk<G, H, U0>) it is possible to introduce special behaviour; e.g. fn disable will only exist for clocking component having U0 current users.

SourceMarker trait and its subtraits

This marker trait unifies family of more specific traits. These ones are essential during a construction fn ::{new, enable} and deconstruction fn ::{free, disable} of clocking components as they provide information to the constructed/deconstructed type what its source is (shown in the example later) and which variant of source (associated constant) is applicable while performing a HW register write.

pub trait SourceMarker {}

pub trait GclkSourceMarker: SourceMarker {
    const GCLK_SRC: crate::pac::gclk::genctrl::SRC_A /* GclkSourceEnum */;
}

pub trait PclkSourceMarker: GenNum + SourceMarker {
    const PCLK_SRC: crate::pac::gclk::pchctrl::GEN_A /* PclkSourceEnum */;
}

pub trait DpllSourceMarker: SourceMarker {
    const DPLL_SRC: crate::pac::oscctrl::dpll::dpllctrlb::REFCLK_A /* DpllSrc */;
}

pub trait GclkOutSourceMarker: GenNum + SourceMarker {}

These are implemented by marker types corresponding to existing clocking abstractions e.g.:

pub enum Pll1 {}
impl GclkSourceMarker for Pll1 {
    const GCLK_SRC: GclkSourceEnum = GclkSourceEnum::DPLL1;
}
// or
pub enum Dfll {}
impl GclkSourceMarker for Dfll {
    const GCLK_SRC: GclkSourceEnum = GclkSourceEnum::DFLL;
}

Source trait and its subtraits

This trait represents a source of clocking signal while subtraits its more specialized flavours (source of signal for Dpll, Pclk, Gclk, etc.).

pub trait Source {
    fn freq(&self) -> Hertz;
}

pub trait GclkSource<G: GenNum>: Source {
    type Type: GclkSourceMarker;
}

pub trait PclkSource: Source {
    type Type: PclkSourceMarker;
}

pub trait DpllSource: Source {
    type Type: DpllSourceMarker;
}

pub trait PrivateGclkOutSource: Source {
    fn enable_gclk_out(&mut self, polarity: bool);
    fn disable_gclk_out(&mut self);
}

pub trait GclkOutSource: PrivateGclkOutSource {
    type Type: GclkOutSourceMarker;
}

These are implemented by corresponding specialized types of Enabled e.g.:

impl<G, D, M, N> GclkSource<G> for Enabled<Dpll<D, M>, N>
where
    G: GenNum,
    D: DpllNum + GclkSourceMarker,
    M: SrcMode,
    N: Counter,
{
    type Type = D;
}

Regardless of how complicated it might seem to look, it can be roughly understood as: Enabled Dpll peripheral can be a source of signal for any Gclk.

*Token types

Unfortunately, PAC::Peripherals granularity is too low for them to be useful when spliting clocking system into such small, semi-independent pieces. In order to solve this problem, we consume PAC at the very beginning and return a set of tokens that internally use appropriate RegisterBlock directly, retrieved from a raw pointer . It is safe because register regions managed by different tokens do not overlap. Tokens cannot be created by a user; they are provided during initialization and do not expose any public API. Memory accesses are read/write-synchronized (Chapter 13.3; p. 138).

Source/SourceMarker traits usage in an API

This is a slightly simplified example of how more less every clocking component that relies on one-to-many depenedency relationships is implemented

impl<G, T> Gclk<G, T>
where
    // `GenNum` is a generalization of a Gclk compile time parameters
    // (e.g. ordering number via associated constant)
    G: GenNum,
    // Practical application of `SourceMarker`; it makes a connection between
    // `Source` and a `Gclk` used by it
    T: GclkSourceMarker,
{
    pub fn new<S>(token: GclkToken<G>, source: S) -> (Self, S::Inc)
    where
        S: GclkSource<G, Type = T> + Increment,
    {
        // .. implementation details ..
        let gclk = Gclk {
            token,
            /* ... */
        };
        (gclk, source.inc())
    }

    pub fn free<S>(self, source: S) -> (GclkToken<G>, S::Dec)
    where
        S: GclkSource<G, Type = T> + Decrement,
    {
        (self.token, source.dec())
    }

    pub fn enable(mut self) -> Enabled<Self, U0> {
        // HW register writes
        Enabled::new(self)
    }
    // Other functions operating on a disabled `Gclk<G, T>`
}
impl<G, T> Enabled<Gclk<G, T>, U0>
where
    G: GenNum,
    T: GclkSourceMarker,
{
    fn disable(mut self) -> Gclk<G, T> {
        // HW register writes
        self.0
    }
}

Gclk::new consumes, upon construction, a GclkSource provided to it and returns it with a type of Enabled<_, N++> (as mentioned previously, specializations of Enabled implement Source based traits). Analogically, Gclk::free consumes a GclkSource passed in and returns it with a new type of Enabled<_, N-->. By design it is impossible to go below 0, because there is always less or equal amount of users than a counter value.

Note: amount of users might be less than a value of a counter in case of special types like Gclk<Gen0> which always has an implicit single user -- synchronous clocking domain. Minimal amount of users for it is 1, making it impossible to disable and therefore consistent with its documented HW characteristics.

Version of this chapter from 2021-04-16

These properties are going to be reflected in Rust by traits (representing abstract concepts of ClkSrc, GClk and PCh) and structs implementing these traits (representing concrete HW instances of these concepts like ClkSrc:XOsc0, GClk:GClk0 or PCh:PCh0).

trait ClkSrc {}
trait GClk {}
trait PCh {}
// Eg.
struct XOsc1 { /* ... */ }
impl ClkSrc for XOsc1 {}

Because of this one-to-many type of relationships between dependencies and dependees, typical for Rust approach of taking ownership over the dependency while dependee is being constructed is not sufficient.

struct GClk10<TClkSrc: ClkSrc> {
    clk_src: TClkSrc,
    regs: GClk10Regs
}
struct GClk11<TClkSrc: ClkSrc> {
    // Like GClk10
}
impl<TClkSrc: ClkSrc> GClk10<TClkSrc> {
    pub fn new(regs: GClk10Regs, clk_src: TClkSrc) -> Self {
        Self { 
            regs, // PAC registers wrapped in an additional structs
            clk_src
        }
    }
}
impl<TClkSrc: ClkSrc> GClk11<TClkSrc> {
    pub fn new(regs: GClk11Regs, clk_src: TClkSrc) -> Self {
        // Like GClk10
    }
}

// Problem demonstration:
// NOTE: `gclk10_regs: GClk10Regs`, `gclk11_regs: Clk11Regs`, `xosc1: XOsc1` in scope
{
    let gclk10 = GClk10::new(gclk10_regs, xosc1);
    // let gclk11 = GClk11::new(gclk11_regs, xosc1); // Compilation error; value moved twice
}

To solve this, token-based approach might be used. Dependee, upon construction, consumes a token acquired from a dependency incrementing dependency's internal counter. Token can be returned back to the dependency causing its internal counter value to decrement. Dependency cannot be deconstructed as long as its internal counter is above 0. release(self) methods return self within Result::Err variant, allowing to recover after a failed release operation.

trait ClkSrcToken {}
struct XOsc1Token;
impl ClkSrcToken for XOsc1Token {}

struct GClk10<TClkSrcToken: ClkSrcToken> {
    clk_src_token: TClkSrcToken,
    regs: GClk10Regs
}
struct GClk11<TClkSrcToken: ClkSrcToken> {
    // Like GClk10
}

impl XOsc1 {
    pub fn new(regs: XOsc1Regs) -> Self {
        Self { regs, used_by: 0 }
    }

    pub fn acquire_token(&mut self) -> XOsc1Token {
        self.used_by += 1;
        XOsc1Token
    }

    pub fn dispose_token(&mut self, token: XOsc1Token) {
        self.used_by -= 1;
    }

    pub fn release(self) -> Result<XOsc1Regs, Self> {
        if self.used_by == 0 {
            Ok(self.regs)
        } else {
            Err(self)
        }
    }
}

impl<TClkSrcToken: ClkSrcToken> GClk10<TClkSrcToken> {
    pub fn new(regs: GClk10Regs, clk_src_token: TClkSrcToken) -> Self {
        Self { 
            regs, // PAC registers wrapped in an additional structs
            clk_src_token
        }
    }
    pub fn release(self) -> (GClk10Regs, TClkSrcToken) {
        let Self { regs, clk_src_token } = self;
        (regs, clk_src_token)
    }
}
impl<TClkSrcToken: ClkSrcToken> GClk11<TClkSrcToken> {
    pub fn new(regs: GClk11Regs, clk_src_token: TClkSrcToken) -> Self {
        // Like GClk10
    }
    pub fn release(self) -> (GClk11Regs, TClkSrcToken) {
        // Like GClk10
    }
}

// Usage demonstration
// NOTE: Should not fail
{
    let mut xosc1 = XOsc1::new(XOsc1Regs);
    let mut xosc1 = match xosc1.release() {
        Ok(regs) => XOsc1::new(regs),
        _ => unreachable!()
    };
    let gclk10 = GClk10::new(GClk10Regs, xosc1.acquire_token());
    let mut xosc1 = match xosc1.release() {
        Err(xosc1) => xosc1,
        _ => unreachable!()
    };
    let gclk11 = GClk11::new(GClk11Regs, xosc1.acquire_token());
    let mut xosc1 = match xosc1.release() {
        Err(xosc1) => xosc1,
        _ => unreachable!()
    };
    xosc1.dispose_token(gclk10.release().1);
    let mut xosc1 = match xosc1.release() {
        Err(xosc1) => xosc1,
        _ => unreachable!()
    };
    xosc1.dispose_token(gclk11.release().1);
    let _ = match xosc1.release() {
        Ok(regs) => XOsc1::new(regs),
        _ => unreachable!()
    };
}

This token-based approach will be used by all structs implementing aforementioned traits (ClkSrc, GClk, PCh).

Usage from the HAL perspective

Updated: 2021-06-08

HAL peripheral X that is associated by HW design to peripheral channel Y PCh:PChY Pclk<Y, _> will consume a token acquired from PCh:PChY Pclk<Y, _>.
Thus, clocking tree that is relevant for HAL peripheral X is locked and has to be released step by step. It is worth noting, that only that part of clocking tree is locked so the granularity is quite fine.

V1 clocking API compatibility

New: 2021-06-08

In order to support v1 clocking compatible peripherals, every clock::v1::*Clock object implements a From<Pclk<_, _>> trait. Therefore, it is feasible to use existing peripheral implementations with new clocking API.

Default state

Updated: 2021-06-08

fn crate::clock::v2::retrieve_clocks returns a tuple of following type

(
    Enabled<Gclk0<marker::Dfll>, U1>,
    Enabled<Dfll<OpenLoop>, U1>,
    Enabled<OscUlp32k, U0>,
    Tokens,
)

which is supposed represent a default state of clocking system after reset (Chapter 13.7; p. 141).

Version of this chapter from 2021-04-16 Idea is to have a stateless function returning `GCLK0` + `DFLL48M` instances populated with adequate tokens that represent default clocking configuration of the HW ([Chapter 13.7; p. 141][v7datasheet]). `GCLK0` and `DFLL48M` should not have public constructors. Moreover, `GCLK0` should have an _exchange_ method for managing `ClkSrc` instead of the release. This is because `GCLK0` is essential for MCU to function, as it drives MCLK and therefore the CPU.

// TODO Show an example of GClk0<T: ClkSrcToken>::exchange<U: ClkSrcToken>(clk_src_token: U) -> GClk0<U>

// TODO: Fix further chapters.

Access to PAC

Instead of having single ClockController-like object managing the clock tree, new API assumes usage of smaller types that cannot be misused or misconstructed.
To achieve this, one needs to be able to split PAC components according to the granularity of clocking API components. Here, idea is to have a split function (similar to one available within GpioExt trait) that would output a tuple of structs wrapping PAC registers within.

Handling of MCLK

It is possible that some of these structs will need to share registers among each other. For example, PACPeripherals::MCLK owns ahbmask and apb{a, b, c, d}mask registers that are managing synchronous clock domain for a variety of peripherals. As long as we make sure that the implementation of these structs is correct (meaning eg. no overlapping bitmask operations), it should be fine.

As an entry point for a discussion we are proposing to split PACPeripherals::MCLK into structs that correspond to would-be PCh types and make them PChs' dependencies. Even though PChs are part of the asynchronous clocking domain, they are more less corresponding to the existing peripherals. It'd probably make sense to have it initialized there, as PChs are going to be HAL peripheral dependencies anyway.

thumbv7em vs thumbv6m

General idea revolves around generalizing PAC access as early as possible. Eg. for PACPeripherals::{MCLK, PM} (MCLK for thumbv7em; PM for thumbv6m) having a split() implementation for PM that would split it into power management related registers and a thumbv7em's MCLK like struct.

// TODO get into details

Embedded-time

ATSAMD seems to be heading towards embedded-time, as evident from Issue #333.

This proposal does not conflict with this path forward as it is agnostic to what stores the notion of time.

How We Teach This

Documentation should be updated along with a migration guide from the older abstraction. It would also be beneficial for users to have some reference examples for common clocking scenarios.

Drawbacks

Why should we not do this?

It is a major breaking change.

Alternatives

What other designs have been considered? What is the impact of not doing this?

Retaining the similar but subtly different clocking abstractions for thumbv7em and thumbv6m, with the drawbacks as discussed above.

Unresolved questions

What parts of the design are still TBD?

NOTE: These questions are mostly related to thumbv7em.

  • genctrl::SRC_A: PAC defined clock sources, what are GCLKIN and GCLKGEN1 (in a diagram on a page 142 GCLK1 seems to be able to become an input for other GCLKs even though it is not apparent on a diagram from page 136)?
  • How to incorporate CLK_RTC_OSC (RTC clock source), CLK_WDT_OSC (WDT clock source) and CLK_ULP32K (EIC clock source)?
  • What's the nature of relationship between USB peripheral and DFLL48M. Can it be incorporated to the typesafe hierachy in a coherent manner (page 136)?
  • What is PCh3 specifically used for (what is the GCLK_DPLLn_32K signal)?
    Hypothesis:
    • DPLL0 can rely either on a reference clock from PCh1 (GCLK_DPLL0) or PCh3 (GCLK_DPLL0_32K)
    • DPLL1 can rely either on a reference clock from PCh2 (GCLK_DPLL1) or PCh3 (GCLK_DPLL1_32K)

References

If there's a reference to an external document with a specified page, it is implicit reference to SAM D5x/E5x Family Data Sheet (DS60001507F) in a context of thumbv7em based MCUs.
For thumbv6m-based MCU, it is a reference to SAM D21/DA1 Family Data Sheet (DS40001882F).

@bradleyharden
Copy link
Contributor

@vccggorski and @vcchtjader, as discussed on Matrix, I already have a nearly-complete clock::v2 module. I haven't worked on it in a few weeks, so I forgot that it is only for thumbv7em right now. I haven't read through the D21 or D11 datasheets, though, so I don't know how easy it would be to generalize it.

Before digging in, I would recommend you understand what I've started to call the AnyKind trait pattern. I use it in gpio::v2 and sercom::v2, and @jbeaurivage has also used it in dmac. It ends up being really useful for this kind of type-level programming. I think my best explanation of it is here, the documentation for AnyPin. I'm working on adding some documentation to the typelevel module to explain the general concept. Then, each implementation can point to that centralized documentation.

I've read your proposal in a little bit more detail, and I have to say that you hit the nail on the head in every count.

  • I have a clock::Source wrapper struct for several different types of clock source, with an AnySource trait; a Gclk type and an AnyGclk trait; and a Pclk type and an AnyPclk trait.
  • I use some type-level magic and the typenum crate to implement a compile-time LockCount trait that handles the M:1 problem. Once a Source or a Gclk has been locked, it can't be modified. But it can be locked and unlocked multiple times, all tracked at compile time.
  • I also handle most of the cases where Pclks wrap back around and are used by clock Sources. However, I think I missed that Dfll can take Pclk0 as a source. I didn't pay much attention to Dfll, honestly. It's not a huge priority for me personally.

For the MCLK stuff, I have ApbClk and AhbClk types that represent enabled APB and AHB clocks, while ApbToken and AhbToken types represent the disabled clocks.

I tried to add some comments just now. I hope the are helpful. After a few minutes, I realized that I could spend hours writing comments for everything, so I decided to stop for now. Honestly, that's why I haven't worked on it in a while. There's just so much documentation to write. But I think that documentation is really important if we want this to be accessible to people.

Have a look and let me know what you think. Maybe we can collaborate and use this as a starting point. @jbeaurivage and I collaborated quite a bit on the dmac module.

@bradleyharden
Copy link
Contributor

bradleyharden commented Apr 17, 2021

I forgot to include the link.

Also, here is a good example of the entire module in action. It's extensively commented, so I think it should be easy to follow. This is an entirely real use case. It's almost exactly my actual clocking structure for my work project.

Also, FYI, the compile-time locking trait (LockCount) used to be called RefCount, and the language around it referred to "borrowing". @jbeaurivage helped me realize that it represents a lock more than a borrow/reference, so I renamed it. I mention this in case you run into the old language somewhere in the code.

@bradleyharden
Copy link
Contributor

@vccggorski, we missed celebrating the one year anniversary of this PR! 😢

We'll get there eventually. 🥳

@glaeqen
Copy link
Contributor Author

glaeqen commented Apr 24, 2022

You're right, damn :/ I have to focus and allocate some time for it, argh.

bradleyharden pushed a commit that referenced this issue Dec 26, 2022
Add a new API for the `clock` module on `thumbv7em` targets

Add a new `clock` module API that enforces correctness by construction. It is impossible to create an invalid clock tree without using `unsafe`. Specifically, use typed tokens and clocks to guarantee memory safety and act as proof that a clock is correctly configured, and use type-level numbers to count consumer clocks at compile-time and restrict a given clock's API while it is in use.

This commit comes after two years of work, starting with #272, then with #429 and culminating with PR #450.

Co-authored-by: Gabriel Górski <gabriel.gorski@volvocars.com>
Co-authored-by: Henrik Tjäder <henrik.tjader@volvocars.com>
@bradleyharden
Copy link
Contributor

Closed with #450

bradleyharden added a commit that referenced this issue Dec 26, 2022
Add a new API for the `clock` module on `thumbv7em` targets

Add a new `clock` module API that enforces correctness by construction.
It is impossible to create an invalid clock tree without using `unsafe`.
Specifically, use typed tokens and clocks to guarantee memory safety and
act as proof that a clock is correctly configured, and use type-level
numbers to count consumer clocks at compile-time and restrict a given
clock's API while it is in use.

This commit comes after two years of work, starting with #272, then
with #429 and culminating with PR #450.

Co-authored-by: Bradley Harden <bradleyharden@gmail.com>
Co-authored-by: Gabriel Górski <gabriel.gorski@volvocars.com>
Co-authored-by: Henrik Tjäder <henrik.tjader@volvocars.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants