Support for the STM32F107?

[Rua]

I will be getting this board and I'd like to have support for it. Given its current lack, I had a look at what would be needed and if I could write basic support myself. The clock circuitry (RCC) is the biggest difference, as there's two dividers and two PLLs instead of just the single PLL like on the lower-end MCUs. This makes the freeze function incredibly complex. The number of possible combinations is huge, and finding the optimal one for the given input clock and desired sysclk becomes much more calculation-intensive.

I wonder if others have looked into this, and whether there is a workable solution?

[therealprof]

I was planning on adding support a while ago due to this being the simplest STM32 MCU with Ethernet support but I never come around to it and put the project on ice. I haven't looked into it too much but I also feel this is going to be tricky.

[nbraud]

Commenting here because I have some Olimex boards with that SoC (and was planning to send @Rua one). I'll probably help her sort out how to compute clock parameters (fastest way might be to solve a diophantine system).

However, this situation suggests a couple of changes to the HAL crate:

• clarify whether freeze() is expected to exactly fulfil the desired clock rates: it's not always possible, and perhaps there should be two functions (one which returns Result<Clocks, ClockError> and is exact, and one which cannot fail but may be approximate?) ;
• separate the computation of clock/pll/... parameters into a const fn: in the most common case, the desired clock frequencies are known at compile time and so the parameters can be precomputed.

[TheZoq2]

clarify whether freeze() is expected to exactly fulfil the desired clock rates: it's not always possible, and perhaps there should be two functions (one which returns Result<Clocks, ClockError> and is exact, and one which cannot fail but may be approximate?) ;

The current implementation approximates it which I'm not a huge fan of. Though I'd almost argue that a panic is more appropriate than a Result.

separate the computation of clock/pll/... parameters into a const fn: in the most common case, the desired clock frequencies are known at compile time and so the parameters can be precomputed.

That's not a bad idea. Is constfn stable enough that we can do everything we need? Last I checked they are pretty limited

[Rua]

Making the calculation part a const fn seems like a good idea. There's no real need to do it on the hardware if the inputs are compile-time constants, which they generally will be for many use cases.

I'm not sure what makes more sense if there is no exact configuration. Result gives the user the freedom to decide what they want to do, which panic doesn't. Approximating seems ok, but perhaps that should be a choice the user makes rather than always doing it.

[TheZoq2]

I'm not sure what makes more sense if there is no exact configuration. Result gives the user the freedom to decide what they want to do, which panic doesn't. Approximating seems ok, but perhaps that should be a choice the user makes rather than always doing it.

In most cases, it won't make a huge difference I suppose, most peripherals will behave normally, just slightly slower or faster. However, there are cases, like USB which only works at a sysclk of 48 or 72 MHz. I suppose that indicates that a result based system is best. The user can decide if they care about the slightly incorrect frequencies

[Rua]

Looking closer, the current implementation returns a Clocks structure, which contains information about how the algorithm decided to set the clocks up. I suppose the user could check if these values are satisfactory before continuing. But this also means that the clocks have already been set up by that point, whereas if an Err result were returned, the user would probably expect the function to be a no-op.

[TheZoq2]

Yep, that is the case, though I think it's pretty unclear that the user is expected to check for errors, especially considering the function panics in some cases.

How about something like:

type ClockResult = Result<Clocks, Clocks>;

Either way you get a clock config, and if an exact config is required, you can panic! if you don't receive Ok.

One disadvantage here is you get no info about what went wrong, so perhaps something like

enum ClockErrorKind {
    ApproximatedSysClk(u32)
    Approximated...(u32)
    /// When the current algorithm would panic. Revert to default?
    ImpossibleConfig
}

type ClockError = Result<Clock, (Clock, ClockErrorKind)>

is better

[TheZoq2]

Implemented in #205