Skip to content

Add function to allow re-init rcc config for stm32 #4124

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 10 commits into from
May 13, 2025

Conversation

mickem
Copy link
Contributor

@mickem mickem commented Apr 24, 2025

I had a need to configure an external clock generator and then switch to it which means I needed to call init and then configure RCC.
After som input from the matrix chat this is what I came up with.

Not sure if this makes sense, and I did not find any test so I have not added tests.
Might make sense to add an example but as a meaningful example would require a specific external clock generator I am unsure how to proceed but it would be possible to add:

#[embassy_executor::main]
async fn main(_spawner: Spawner) {
    let p = embassy_stm32::init(Default::default());

    // Configure the external clock here

    let mut config = Config::default();
    {
        use embassy_stm32::rcc::*;
        config.rcc.hse = Some(Hse {
            freq: mhz(8),
            mode: HseMode::Bypass,
        });
        config.rcc.pll = Some(Pll {
            src: PllSource::HSE,
            prediv: PllPreDiv::DIV1,
            mul: PllMul::MUL9,
        });
        config.rcc.sys = Sysclk::PLL1_P;
        config.rcc.ahb_pre = AHBPrescaler::DIV1;
        config.rcc.apb1_pre = APBPrescaler::DIV2;
        config.rcc.apb2_pre = APBPrescaler::DIV1;
    }
    embassy_stm32::reinitialize_rcc(config);

    info!("end program");
}

@mickem
Copy link
Contributor Author

mickem commented Apr 24, 2025

Build fail due to the critical section variable is not used in (I assume here) som configurations. Not sure of a good way to handle this I could rename the variable _cs or I could split the code but since it is a closure it is hard to add feature toggles on it...

@Dirbaio
Copy link
Member

Dirbaio commented Apr 24, 2025

Thanks!

  • Could you make the function embassy_stm32::rcc:reinit() instead of embassy_stm32::reinitialize_rcc()?
  • It should take only the RCC config, not the whole embassy-stm32 config.
  • Could you deduplicate the code inside the CS? This way if something gets added there in the future we ensure both init and reinit stay in sync.

///
/// This should only be called after `init`.
#[cfg(not(feature = "_dual-core"))]
pub fn reinit(config: rcc::Config) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could you move this to the RCC module so it's called as embassy_stm32::rcc:reinit() as I said in my last comment and not embassy_stm32::reinit()?

Copy link
Contributor Author

@mickem mickem Apr 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I did not see the rcc, please beware that we now have two rcc::init* functions which is somewhat awkward.
Also not 100% sure if the crate::time_driver::init(_cs) should also be moved but given your feedback I assume this is what we want.

One option for the two rcc:init function would be to wrap the old rcc::init inside the new rcc::init_rcc but unsure if this is what we want. I.e. rename or private the existing init and rename the init_rcc function to init.

Feel free to send me a message on matrix (Michael Medin, @mickem:matrix.org) if you want to discuss options here.

@jamesmunns
Copy link
Contributor

bender run

Copy link
Member

@Dirbaio Dirbaio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thank you!

@Dirbaio Dirbaio added this pull request to the merge queue May 13, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks May 13, 2025
@Dirbaio Dirbaio added this pull request to the merge queue May 13, 2025
Merged via the queue into embassy-rs:main with commit 5caa4ac May 13, 2025
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants