cortex-m icon indicating copy to clipboard operation
cortex-m copied to clipboard

Make SYST::get_current take &self?

Open nmattia opened this issue 2 years ago • 1 comments

Hi,

I'm just getting started with embedded rust, so apologies if this is a silly question!

While trying to get a tick counter, I realized SYST::get_current() would (almost) always return 0:

    /// Gets current value
    #[inline]
    pub fn get_current() -> u32 {
        // NOTE(unsafe) atomic read with no side effects
        unsafe { (*Self::PTR).cvr.read() }
    }

I hadn't read the docs properly and was actually using the systick somewhere else:

    let core = pac::CorePeripherals::take().unwrap();
    let mut syst = core.SYST;
    syst.set_reload(0); // very small value as an example
    syst.clear_current();
    syst.enable_counter();

Without reading the cortex docs and the cortex-m crate sources, I would never have figured the reason why get_current was almost always zero (reason as far as I understand: it will increment until the value passed to set_reload() and then wrap).

I was wondering, wouldn't it be safer to also have get_current() take a &SYST? Then it would also be clearer that get_current() and other functions (set_reload, enable_counter, etc) are related.

Again, apologies if this is silly! I'm just getting started with rust embedded.

nmattia avatar Jan 14 '23 22:01 nmattia

At least in my experience systick is mostly used as a debug tool to measure code execution performance and clock ticks. Hence, it is often used in random parts of the code and carrying &SYST everywhere would be very inconvenient.

I think best option would be to improve get_current() docs and mention how to properly setup and enable systick.

chemicstry avatar Feb 11 '23 01:02 chemicstry