Fix MonoTimers frequency

[teskje]

The MonoTimer internally counts the cycles of the Cortex core. According to the reference (RM0008, Section 8.2) the core is driven by the HCLK, not the SYSCLK, so that should be the frequency reported by MonoTimer.

I originally encountered that issue on an STM32F3 board and submitted a fix to the stm32f3xx-hal here. Unfortunately, I'm not able to verify this bug and its fix for an F1 board as I don't own one. So it would be good if someone else could verify.

[TheZoq2]

Thanks for the PR and sorry for the delay, life has kept me busy recently.

The RM says the following:

The RCC feeds the Cortex® System Timer (SysTick) external clock with the AHB clock (HCLK) divided by 8. The SysTick can work either with this clock or with the Cortex® clock (HCLK), configurable in the SysTick Control and Status Register

So it sounds to me like we need to also take the division by 8 into account, at least in some cases. I'm not sure which one we're using at the moment

[teskje]

Good catch! I've looked a bit more into this:

The MonoTimer uses the Cortex DWT peripheral provided by cortex-m to read the "clock cycle count" (CYCCNT). Looking up the Cortex-M3 reference, chapter 8 on the DWT, there is no explicit mention of which clock's cycles are counted. In particular, there is no mention of SysTick, which leads me to believe "the clock" is the core clock (and therefore HCLK).

Then again, I'm not an expert on ARM CPUs and may be missing something.

[TheZoq2]

Hmm, have you tested this on hardware to check if it behaves correctly? I haven't had time to check the datasheets you linked, but hopefully I can do that soon

[teskje]

No, as I mentioned above, I don't own any F1 hardware, unfortunately. I found this bug on an STM32F3-Discovery board and there this fix solved it.

[therealprof]

Hi @ra-kete, can you resolve the conflict?

[TheZoq2]

This has been lingoring a bit too long, sorry about that.

The MonoTimer uses the Cortex DWT peripheral provided by cortex-m to read the "clock cycle count" (CYCCNT). Looking up the Cortex-M3 reference, chapter 8 on the DWT, there is no explicit mention of which clock's cycles are counted. In particular, there is no mention of SysTick, which leads me to believe "the clock" is the core clock (and therefore HCLK).

This makes sense to me, counting cycles using any other clock seems strange. Would you mind rebasing again? @ra-kete

[TheZoq2]

I finally got around to testing this in hardware and it looks like the current implementation is correct, though I'm not 100% sure why.

This is the code I used:

#![no_main]
#![no_std]

use panic_halt as _;

use stm32f1xx_hal as hal;

use crate::hal::{
    gpio::{gpioc, Output, PushPull},
    pac::{Peripherals, CorePeripherals, TIM2},
    prelude::*,
    timer::{CountDownTimer, Event, Timer},
    time::MonoTimer,
};

use core::cell::RefCell;
use cortex_m::{asm::wfi, interrupt::Mutex};
use cortex_m_rt::entry;
use embedded_hal::digital::v2::OutputPin;
use nb::block;
use cortex_m_semihosting::hprintln;

#[entry]
fn main() -> ! {
    let dp = Peripherals::take().unwrap();
    let cp = CorePeripherals::take().unwrap();

    let mut rcc = dp.RCC.constrain();
    let mut flash = dp.FLASH.constrain();
    let clocks = rcc
        .cfgr
        .sysclk(24.mhz())
        .hclk(16.mhz())
        .freeze(&mut flash.acr);

    let mono = MonoTimer::new(cp.DWT, clocks);

    // Set up a timer expiring after 1s
    let mut timer = Timer::tim2(
        dp.TIM2,
        &clocks,
        &mut rcc.apb1
    ).start_count_down(1.hz());

    let mut start = mono.now();

    loop {
        block!(timer.wait()).unwrap();
        hprintln!("{}", start.elapsed() as f32 / mono.frequency().0 as f32).unwrap();
        timer.start(1.hz());
        start = mono.now();
    }
}

And the output is consistently 1.000026 which is to be expected, I think. Or did I missunderstand something here?

[teskje]

I've run the same test on my F3 and got the same results (1.0000142 when using SYSCLK for the MonoTimer). But I also get the same results when MonoTimer uses HCLK instead. At least in my case that's because SYSCLK and HCLK are actually both set to 24 MHz. I suspect this is the case for you too, just check what's returned by clocks.hclk().

The RCC freeze code tries to be extra user friendly and, if it is unable to apply the requested configuration, selects the "closest" value instead of giving up. I'd argue that this behavior actually is less user friendly because now you cannot rely on the clock speeds unless you double check them. And I expect most users don't double check, because they don't expect behavior like this. If the freeze would simply panic on impossible values, the user could simply fix the configuration and we wouldn't have issues like this.

Excuse my ranting :)

Anyway, on the F3 the combination SYSCLK 24 MHz, HCLK 12 MHz works for me as a test case. With this config and MonoTimer using SYSCLK, it outputs ~0.5.

[TheZoq2]

Ohh right. I should have verified the frequency. And you're right, just returning a configuration that is "good enough" is kind of horrible

[TheZoq2]

Closed by #236