Possibly buggy split_without_reset

[usbalbin]

Found this nasty thing with the G4 stm32-rs/stm32g4xx-hal#180 not sure if the same thing applies for the H5 aswell. If so, then things like GPIOx.split_without_reset might be affected.

The issue for the G4 is that a delay of 1(or maybe 2?) clock cycles after RCC enable on a peripheral before trying to access its registers. From what I understand a reset just so happens to have enough of a delay as a side effect which makes enable -> reset -> access register work fine.

Throwing it out there just in case. Feel free to close :)

[astapleton]

Ah, that's interesting. It looks like it would apply to the H5 family. RM0491 includes similar language:

It looks like this affects all peripheral enables and the language also states that the delay needs to be 2 cycles of the peripheral bus clock, so I'm not sure that the proposed solution above is sufficient. If one of the peripheral bus clocks is significantly slower than HCLK (not the case for GPIOs at least), then I don't think simply operating on an RCC register (the reset operation) would be sufficient.

For this to be robust, we probably need to calculate the number of CPU cycles to wait based on HCLK and the peripheral clock.
And, given that this can affect all peripherals, I think we should update the ResetEnable implementations generated by macro here.

Another option is to perform 2 reads of a register for the peripheral in question, but that would require each peripheral driver to implement this separately on reset. That might be easier, tbh. I implemented a macro to handle what looks to be the same case, but for IRQ synchronization in #27 here (I'm actually no longer using it because I removed interrupt enable/disable code in that PR).

Thoughts?

[usbalbin]

[...] If one of the peripheral bus clocks is significantly slower[...]

Ouch! That is probably not great...

For this to be robust, we probably need to calculate the number of CPU cycles to wait based on HCLK and the peripheral clock. [...]

Do you think that would be possible to do that, without the calculation time taking a lot more time than the actual waiting? For example something like cycles_to_wait = core_speed/peripheral_speed needing a division. With some luck perhaps the optimizer can strip away the calculation.

Another option is to perform 2 reads of a register for the peripheral in question, but that would require each peripheral driver to implement this separately on reset.

I guess that would probably side step the division problem? Would it be possible to make a macro that reads the ISR register of a peripheral or some other register that almost all peripherals have, to make this easier? Perhaps not a great solution, but might save a bit of copy pasting.

[astapleton]

Do you think that would be possible to do that, without the calculation time taking a lot more time than the actual waiting? For example something like cycles_to_wait = core_speed/peripheral_speed needing a division. With some luck perhaps the optimizer can strip away the calculation.

I don't think we can get the compiler to optimize the calculation away because each peripheral has various clock source options so the peripheral clock value be determined at runtime.

Another option is to perform 2 reads of a register for the peripheral in question, but that would require each peripheral driver to implement this separately on reset.

I guess that would probably side step the division problem? Would it be possible to make a macro that reads the ISR register of a peripheral or some other register that almost all peripherals have, to make this easier? Perhaps not a great solution, but might save a bit of copy pasting.

The naming for the registers are different enough that I think that wouldn't be possible without a giant match statement. We could modify that macro to include a register to read when enabling. It adds more noise to that macro unfortunately, but perhaps better than doing it manually in each driver?