stm32-rs / stm32l0xx-hal

A hardware abstraction layer (HAL) for the STM32L0 series microcontrollers written in Rust
BSD Zero Clause License
96 stars 60 forks source link

Setup hsi16diven when running Rcc::freeze() #197

Closed airelil closed 2 years ago

airelil commented 2 years ago

As per CubeMX, some system clock frequencies like 3Mhz, 6MHz, 8MHz or 24MHz, can be achieved only if HSI16 divider is enabled.

RCC_CR, Bit 3 HSI16DIVEN HSI16 divider enable bit This bit is set and reset by software to enable/disable the 16 MHz HSI divider by 4. It can be written anytime. 0: no 16 MHz HSI division requested 1: 16 MHz HSI division by 4 requested

Example of 8MHz configuration:

let clock_config = hal::rcc::Config::pll(
    hal::rcc::PLLSource::HSI16(true),
    hal::rcc::PLLMul::Mul4,
    hal::rcc::PLLDiv::Div2,
    );
jamwaffles commented 2 years ago

IMO it would be much less confusing to add a new variant, e.g. PLLSource::HSI16Div4 instead of an ambiguous (and undocumented) bool - I stumbled upon its usage as a variable named div4 only by chance.

Ideally, is it possible to calculate whether div4 needs to be enabled or not automatically? I haven't looked at the code too closely so this might not be possible.

hannobraun commented 2 years ago

The CI failure looks unrelated to this pull request. I've opened #199.

airelil commented 2 years ago

That's a fair point about documenting. I'll probably change it to something like this:

/// HSI16 divider
#[derive(Clone, Copy)]
pub enum HSI16Div {
    Div1 = 1,
    Div4 = 4,
}

/// System clock mux source
#[derive(Clone, Copy)]
pub enum ClockSrc {
    MSI(MSIRange),
    PLL(PLLSource, PLLMul, PLLDiv),
    HSE(Hertz),
    HSI16(HSI16Div),
}

That would be consistent with the rest of clock sources definitions.

hannobraun commented 2 years ago

That looks good, @airelil!

airelil commented 2 years ago

Added HSIDiv enum. Also reverted the changes in examples so the fix is less intrusive now. I rebased the changes to include @jamwaffles' fix for Clippy.