diff --git a/CHANGELOG.md b/CHANGELOG.md index 086bb0ef06..5c858c5634 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,7 +5,7 @@ This project adheres to [Semantic Versioning](http://semver.org/). ## [Unreleased] -## v0.5.0 - 2019-09-?? (currently in beta pre-release) +## v0.5.0 - 2019-??-?? (currently in beta pre-release) ### Added @@ -45,7 +45,9 @@ This project adheres to [Semantic Versioning](http://semver.org/). Cargo features are enabled. - [breaking-change][] the monotonic timer used to implement the `schedule` API - is now user configurable via the `#[app(monotonic = ..)]` argument. + is now user configurable via the `#[app(monotonic = ..)]` argument. IMPORTANT: + it is now the responsibility of the application author to configure and + initialize the chosen `monotonic` timer during the `#[init]` phase. - [breaking-change][] the `peripherals` field is not include in `init::Context` by default. One must opt-in using the `#[app(peripherals = ..)]` argument. diff --git a/book/en/src/by-example/timer-queue.md b/book/en/src/by-example/timer-queue.md index 7c8be382e8..94d228190b 100644 --- a/book/en/src/by-example/timer-queue.md +++ b/book/en/src/by-example/timer-queue.md @@ -34,6 +34,10 @@ first appear in the `schedule` argument of the context attribute. When scheduling a task the (user-defined) `Instant` at which the task should be executed must be passed as the first argument of the `schedule` invocation. +Additionally, the chosen `monotonic` timer must be configured and initialized +during the `#[init]` phase. Note that this is *also* the case if you choose to +use the `CYCCNT` provided by the `cortex-m-rtfm` crate. + The example below schedules two tasks from `init`: `foo` and `bar`. `foo` is scheduled to run 8 million clock cycles in the future. Next, `bar` is scheduled to run 4 million clock cycles in the future. Thus `bar` runs before `foo` since diff --git a/examples/baseline.rs b/examples/baseline.rs index b7144dd188..df0ff9a408 100644 --- a/examples/baseline.rs +++ b/examples/baseline.rs @@ -14,6 +14,8 @@ use panic_semihosting as _; const APP: () = { #[init(spawn = [foo])] fn init(cx: init::Context) { + // omitted: initialization of `CYCCNT` + hprintln!("init(baseline = {:?})", cx.start).unwrap(); // `foo` inherits the baseline of `init`: `Instant(0)` diff --git a/examples/periodic.rs b/examples/periodic.rs index ec110e1181..dca0ad565b 100644 --- a/examples/periodic.rs +++ b/examples/periodic.rs @@ -16,6 +16,8 @@ const PERIOD: u32 = 8_000_000; const APP: () = { #[init(schedule = [foo])] fn init(cx: init::Context) { + // omitted: initialization of `CYCCNT` + cx.schedule.foo(Instant::now() + PERIOD.cycles()).unwrap(); } diff --git a/examples/schedule.rs b/examples/schedule.rs index 27d3bd1f59..97818e36a1 100644 --- a/examples/schedule.rs +++ b/examples/schedule.rs @@ -1,6 +1,5 @@ //! examples/schedule.rs -#![deny(unsafe_code)] #![deny(warnings)] #![no_main] #![no_std] @@ -13,8 +12,15 @@ use rtfm::cyccnt::{Instant, U32Ext as _}; #[rtfm::app(device = lm3s6965, monotonic = rtfm::cyccnt::CYCCNT)] const APP: () = { #[init(schedule = [foo, bar])] - fn init(cx: init::Context) { - let now = Instant::now(); + fn init(mut cx: init::Context) { + // Initialize (enable) the monotonic timer (CYCCNT) + cx.core.DCB.enable_trace(); + // required on devices that software lock the DWT (e.g. STM32F7) + unsafe { cx.core.DWT.lar.write(0xC5ACCE55) } + cx.core.DWT.enable_cycle_counter(); + + // semantically, the monotonic timer is frozen at time "zero" during `init` + let now = cx.start; // the start time of the system hprintln!("init @ {:?}", now).unwrap(); diff --git a/src/cyccnt.rs b/src/cyccnt.rs index c8a1b7ee61..86969cb1a3 100644 --- a/src/cyccnt.rs +++ b/src/cyccnt.rs @@ -1,11 +1,9 @@ -//! Data Watchpoint Trace (DWT) unit's CYCle CouNTer +//! Data Watchpoint Trace (DWT) unit's CYCle CouNTer (CYCCNT) use core::{ cmp::Ordering, convert::{Infallible, TryInto}, - fmt, - marker::PhantomData, - ops, + fmt, ops, }; use cortex_m::peripheral::DWT; @@ -16,25 +14,25 @@ use crate::Fraction; /// /// This data type is only available on ARMv7-M /// -/// Note that this value is tied to the CYCCNT of one core and that sending it a different core -/// makes it lose its meaning -- each Cortex-M core has its own CYCCNT counter and these are usually -/// unsynchronized and they may even be running at different frequencies. +/// # Correctness +/// +/// Adding or subtracting a `Duration` of more than `(1 << 31)` cycles to an `Instant` effectively +/// makes it "wrap around" and creates an incorrect value. This is also true if the operation is +/// done in steps, e.g. `(instant + dur) + dur` where `dur` is `(1 << 30)` ticks. +/// +/// In multi-core contexts: this value is tied to the CYCCNT of *one* core so sending it a different +/// core makes it lose its meaning -- each Cortex-M core has its own CYCCNT counter and these are +/// usually unsynchronized and may even be running at different frequencies. #[derive(Clone, Copy, Eq, PartialEq)] pub struct Instant { inner: i32, - _not_send_or_sync: PhantomData<*mut ()>, } -unsafe impl Sync for Instant {} - -unsafe impl Send for Instant {} - impl Instant { /// Returns an instant corresponding to "now" pub fn now() -> Self { Instant { inner: DWT::get_cycle_count() as i32, - _not_send_or_sync: PhantomData, } } @@ -61,6 +59,8 @@ impl fmt::Debug for Instant { impl ops::AddAssign for Instant { fn add_assign(&mut self, dur: Duration) { + // NOTE this is a debug assertion because there's no foolproof way to detect a wrap around; + // the user may write `(instant + dur) + dur` where `dur` is `(1<<31)-1` ticks. debug_assert!(dur.inner < (1 << 31)); self.inner = self.inner.wrapping_add(dur.inner as i32); } @@ -77,7 +77,7 @@ impl ops::Add for Instant { impl ops::SubAssign for Instant { fn sub_assign(&mut self, dur: Duration) { - // XXX should this be a non-debug assertion? + // NOTE see the NOTE in `>::add_assign` debug_assert!(dur.inner < (1 << 31)); self.inner = self.inner.wrapping_sub(dur.inner as i32); } @@ -115,6 +115,12 @@ impl PartialOrd for Instant { /// A `Duration` type to represent a span of time. /// /// This data type is only available on ARMv7-M +/// +/// # Correctness +/// +/// This type is *not* appropriate for representing time spans in the order of, or larger than, +/// seconds because it can hold a maximum of `(1 << 31)` "ticks" where each tick is the inverse of +/// the CPU frequency, which usually is dozens of MHz. #[derive(Clone, Copy, Default, Eq, Ord, PartialEq, PartialOrd)] pub struct Duration { inner: u32, @@ -208,9 +214,6 @@ impl crate::Monotonic for CYCCNT { } fn zero() -> Instant { - Instant { - inner: 0, - _not_send_or_sync: PhantomData, - } + Instant { inner: 0 } } } diff --git a/src/lib.rs b/src/lib.rs index 959c9b7bc6..d661c54f67 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -138,9 +138,21 @@ pub trait Monotonic { fn ratio() -> Fraction; /// Returns the current time + /// + /// # Correctness + /// + /// This function is *allowed* to return nonsensical values if called before `reset` is invoked + /// by the runtime. Therefore application authors should *not* call this function during the + /// `#[init]` phase. fn now() -> Self::Instant; /// Resets the counter to *zero* + /// + /// # Safety + /// + /// This function will be called *exactly once* by the RTFM runtime after `#[init]` returns and + /// before tasks can start; this is also the case in multi-core applications. User code must + /// *never* call this function. unsafe fn reset(); /// A `Self::Instant` that represents a count of *zero*