rcc: I2S and SAI clocks

[mgottschlag]

Preface

Some of this code is probably pretty ugly. In parts, the code looks pretty bad, and it imposes pretty arbitrary limitations (see below) in terms of functionality. I propose merging something like this on the basis that it is strictly more powerful than the previous implementation - in cases where no I2S or SAI clocks are requested, the old code is still used providing identical performance and identical results.

Situation

The current RCC code only provides the sysclk, but no I2S and SAI clocks. Future pull requests (#212? I am currently working on support for SAI) require these clocks. Different MCUs have wildly differing clock trees, though. Some microcontrollers (e.g. STM32F411/446) have separate I2SPLLM dividers, others do not. F413/423 do not have an SAI PLL even though SAI is supported. F410 does not even have an I2S PLL and the I2S clock is generated by the main PLL. The different amount of SAIs and I2S instances causes differences in the clock selection code. See #240 for a table of all those differences. Note: The table does not include that F413/423 have a different SAI divider range compared to the other models.

Approach

This wide range of different configurations and the flexible, yet very model-specific connections (e.g., SAI clocks can usually be generated by the I2S PLL and vice versa) make writing a completely flexible solution difficult. The computational complexity of such a solution to generate the ideal results might very likely be prohibitive for any runtime implementation.

I therefore decided on an approach with some limitations to keep both implementation and computational complexity down. In particular, I2S clocks, if required, are always generated by the I2S PLL, and likewise SAI clocks are always generated by the SAI PLL, with special cases for the models where these PLLs do not exist. This means that there can be only one I2S frequency (and likewise SAI frequency) that does not match the provided I2S_CKIN external frequency. This should be enough for all applications except for applications which implement resampling between e.g. 44100 and 48000 Hz audio and have to implement an I2S master for both.

The code decouples clock selection (the bottom half of the table in #240) from PLL optimization to reduce the numbers of special cases required in each part - clock selection is implemented in the form of I2sClocks and SaiClocks types in rcc.rs.

Alternatives

I believe the restrictions described above are valid for now. If, eventually, we want a more complete and more flexible interface and do not care about API backwards compatibility, we can implement clock tree configuration in the form of an external compile-time tool - either via procedural macros or via a library that can be used within build.rs. In this case, the RCC library should probably be changed to provide two functions "RCC.apply_static_config()" or "RCC.apply_runtime_config()", where the former just applies a set of precalculated config options and the latter executes the current cheap yet limited runtime configuration algorithms.

Status/Testing

At the moment, most of the code is still completely untested and I only have a STM32F429 discovery board for testing:

• Main PLL (most models): The old main PLL code to generate sysclk has been tested with a blinky example on a STM32F429 board. Most models should behave identically.
• Main PLL (STM32F410) when an I2S clock has been requested: TODO, does anybody have an STM32F410 and can run any blinky example modified to request ".i2s_clk(64.mhz())"?
• I2S PLL (STM32F405/407/415/419/427/429/437/439): WIP. If anyone working on I2S wants to spend some time on debugging, feel free ;-)
• SAI PLL (STM32F427-429, 469, 479): The code seems to generate the correct MCK signal.
• I2S PLL (STM32F411-413, 423, 447): I do not have the required MCU. Most of the code is identical to the other models, though. Let me test on the 429 first before you give it a go.
• SAI PLL (STM32F446): I do not have the required MCU. The code path should be identical to the STM32F429 without main PLL though, and that works.

Once the tests on the F429 are done, I need help with the other models - and I could use help with the F410 right away. Do not expect the code to work as-is, though.

[mgottschlag]

Note that #212 contains a remark that I2S frequencies are difficult because configuration is spread out between I2S and RCC. I do not think this is a problem. I think we should just list a number of good intermediate frequencies in the documentation for systems where the PLL VCO output can be a multiple of 1MHz, which will be the case for most systems (for example, 86 MHz can be used for precise generation of I2S frequencies for 48KHz audio).

[xoviat]

This impacts #246 because the rcc doesn't have the APB1 clock either. Rather than rewriting it like this, would it be possible to copy the approach done in stm32f1. Also, could you include the APB1 clock and make sure that this is unsable for canbus? Thanks.

[mgottschlag]

Could you be more specific about what you mean with the stm32f1 remark? I do not quite understand what you mean with that.

w.r.t. APB1 clock: doesn't pclk1(...) set the APB1 clock already?

Edit: If the latter really answers the question, then that shows how much the existing documentation is lacking. I had to look through the implementation to see that pclk1 means "APB1 clock".

[xoviat]

What I mean is that the rcc module in stm32f1 has the APB1 struct that you can use as such:

/// Advanced Peripheral Bus 1 (APB1) registers
///
/// Aquired through the `Rcc` registers:
///
/// ```rust
/// let dp = pac::Peripherals::take().unwrap();
/// let mut rcc = dp.RCC.constrain();
/// function_that_uses_apb1(&mut rcc.apb1)
/// ```

It also has other such similar structs. It is similar to copy these structs from stm32f1 to stm32f4 because the layout is currently about the same. If you modify the layout, it will not be so simple. I just need APB1, but it would be good to add the other structs as well.

[mgottschlag]

Ah, I see. I think that is completely orthogonal to this PR, I'll concentrate on the PLL stuff.

Although I personally do not mind the current practice to just make an unsafe access to the apb1enr/rst registers, given that the writes are atomic and that the user API is less cluttered.

[mgottschlag]

With the fixes, the code generates correct SAI frequencies both with active and inactive main PLL on the STM32F429. That should cover the SAI code path of the STM32F446 as well. The I2S code path is sufficiently similar that I'd consider this ready for review.

[therealprof]

Missing CHANGELOG entry, other than that looks good to me on visual inspection.

[mgottschlag]

@therealprof Thanks for the review!

I fixed the clippy lints and added a changelog entry.

[mgottschlag]

... and I rebased to fix the merge conflict.

[therealprof]

LGTM, thanks.

bors r+

[bors]

Build succeeded:

• ci (stm32f401, stable)
• ci (stm32f405, stable)
• ci (stm32f407, stable)
• ci (stm32f410, stable)
• ci (stm32f411, stable)
• ci (stm32f412, stable)
• ci (stm32f413, stable)
• ci (stm32f415, stable)
• ci (stm32f417, stable)
• ci (stm32f423, stable)
• ci (stm32f427, stable)
• ci (stm32f429, stable)
• ci (stm32f437, stable)
• ci (stm32f439, stable)
• ci (stm32f446, stable)
• ci (stm32f469, stable)
• ci (stm32f479, stable)
• Rustfmt

[thalesfragoso]

Was this supposed to be pub ?

[mgottschlag]

I do not see why it should be, but it was already pub before I started.

[mgottschlag]

Apparently I messed up and caused some warnings about unused fields on F410/413/423. I will create a follow-up PR for those.