clock::v2: DPLL bug, potential UB?

[jbeaurivage]

I've been trying out the clock::v2 module, and I've encountered some pretty strange behavior when trying to setup the DPLL to speed up the main CPU clock.

My test setup

• Metro M4 (SAMD51)
• The following example code:

#![no_std]
#![no_main]

use defmt_rtt as _;
use panic_probe as _;

use atsamd_hal::{
    clock::v2::{
        clock_system_at_reset,
        dpll::Dpll,
        gclk::{Gclk, GclkDiv16},
        pclk::Pclk,
    },
    pac::Peripherals,
};

#[cortex_m_rt::entry]
fn main() -> ! {
    let mut peripherals = Peripherals::take().unwrap();

    let (_buses, clocks, tokens) = clock_system_at_reset(
        peripherals.oscctrl,
        peripherals.osc32kctrl,
        peripherals.gclk,
        peripherals.mclk,
        &mut peripherals.nvmctrl,
    );

    // Set GCLK1 to use the 48 MHz DFLL...
    let (gclk1, dfll) = Gclk::from_source(tokens.gclks.gclk1, clocks.dfll);
    // ...and divide it down to 2 MHz
    let gclk1 = gclk1.div(GclkDiv16::Div(24)).enable();
    // Feed GCLK1 to the DPLL
    let (pclk_dpll0, _gclk1) = Pclk::enable(tokens.pclks.dpll0, gclk1);
    // Now start the DPLL with a 50x multiplier, to get a 100 MHz clock,
    // which is the maximum speed the ADC may run at.
    let dpll0 = Dpll::from_pclk(tokens.dpll0, pclk_dpll0)
        .loop_div(50, 0)
        .enable();

    defmt::info!(
        "DPLL: Ready: {}, Locked: {}",
        dpll0.is_ready(),
        dpll0.is_locked()
    );

    // When the DPLL is ready, swap the GCLK0 source to run the CPU
    // on the 100 MHz DPLL
    while !(dpll0.is_ready() && dpll0.is_locked()) {}
    let (_gclk0, _dfll, dpll0) = clocks.gclk0.swap_sources(dfll, dpll0);

    defmt::info!(
        "DPLL: Ready: {}, Locked: {}",
        dpll0.is_ready(),
        dpll0.is_locked()
    );

    loop {
        // A long `nop` delay as a rough way of comparing CPU clock speeds
        defmt::debug!("Tick");
        cortex_m::asm::delay(100_000_000);
    }
}

Execution always stalls at this part:

while !(dpll0.is_ready() && dpll0.is_locked()) {
        core::hint::spin_loop();
}

which is making me believe there might be a missed step in setting up the clock tree. What's weird, is that if I move the wait to after the call to swap_sources, then the CLKRDY and LOCK bits are set correctly, and execution resumes.

What's even weirder however, is that seemingly random changes in the code affect whether or not the CPU clock is actually sped up to 100 MHz. For example, adding the following line before main makes the clock run at the expected speed:

rtc_monotonic!(Mono, rtc_clock::Clock32k);

But when it's removed, the CPU clock seems to stay stuck at 48 MHz, which is the speed at reset. However that macro is just a bunch of struct and function declarations, and should have no effect on runtime behavior... Leading me to believe that there must be some kind of undefined behavior or misoptimization lurking somewhere.

I haven't dove deeper into the issue yet, but I wanted to document my findings while it's still fresh.

[jbeaurivage]

Potentially another report here: #809 (comment)