You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
The new clock configuration infrastructure has introduced a misfeature that, I believe, wasn't available previously: It is now possible to use the clock for one peripheral, then modify it by calling the same methods it was set up with (like syscon.frg0.set_mul, for example).
This is pretty easy to do by accident, for example when setting up a second USART instance, and copy-pasting the set-up code from the first.
I have some ideas on how to solve this.
Solution 1: Keep a shared reference to the clock in the peripheral clock structs, and also make sure not to drop those while the peripheral is enabled (add it to the enabled state). That would prevent any modification of a clock while a peripheral using it was active. This is the most straight-forward solution, but the added lifetimes will complicate some use cases, specifically if the user wants a peripheral to live in a static.
Solution 2: Add type for the clocks (like Unconfigured/Configured or Ready/Initialized; not sure). Only allow configuration in the "ready" state, but have the peripheral clock structs require it in the "initialized" state. Just don't add any means to ever get back to the "ready" state, thereby preventing any clock that is used from ever being modified again. Also quite straight-forward, doesn't have the problems of solution 1, but is less flexible. Not sure how relevant that is to real use cases.
Solution 3: Add type state to the clocks that counts how many peripherals are using it, and only allow modifications while that number is zero. This could work something like this:
Peripheral clock structs don't have their own constructors. You get them by calling a method on the main clock struct (e.g. let (frg0, usart_clk) = syscon.frg0.get_usart_clock()).
This constructor method consumes the clock (e.g. FRG0 in this example) and returns another instance with the type state counter increased by one.
Peripherals keep the peripheral clocks structs around while enabled, return it again when disable.
The peripheral clock struct can then be returned (e.g. syscon.frg0 = frg0.return_usart_clock(usart_clk), which decreases the type state counter.
Eventually, we'll be able to use const generics for the type state counter. For now, using nested tuples (the same hack used in the SWM module) should do.
I'd be fine with any of these solutions being implemented. Solution 2 is probably the best for a quick fix, while solution 3 would be the best overall (although most complex) in my opinion.
The text was updated successfully, but these errors were encountered:
The new clock configuration infrastructure has introduced a misfeature that, I believe, wasn't available previously: It is now possible to use the clock for one peripheral, then modify it by calling the same methods it was set up with (like
syscon.frg0.set_mul
, for example).This is pretty easy to do by accident, for example when setting up a second USART instance, and copy-pasting the set-up code from the first.
I have some ideas on how to solve this.
Solution 1: Keep a shared reference to the clock in the peripheral clock structs, and also make sure not to drop those while the peripheral is enabled (add it to the enabled state). That would prevent any modification of a clock while a peripheral using it was active. This is the most straight-forward solution, but the added lifetimes will complicate some use cases, specifically if the user wants a peripheral to live in a
static
.Solution 2: Add type for the clocks (like
Unconfigured
/Configured
orReady
/Initialized
; not sure). Only allow configuration in the "ready" state, but have the peripheral clock structs require it in the "initialized" state. Just don't add any means to ever get back to the "ready" state, thereby preventing any clock that is used from ever being modified again. Also quite straight-forward, doesn't have the problems of solution 1, but is less flexible. Not sure how relevant that is to real use cases.Solution 3: Add type state to the clocks that counts how many peripherals are using it, and only allow modifications while that number is zero. This could work something like this:
let (frg0, usart_clk) = syscon.frg0.get_usart_clock()
).syscon.frg0 = frg0.return_usart_clock(usart_clk)
, which decreases the type state counter.I'd be fine with any of these solutions being implemented. Solution 2 is probably the best for a quick fix, while solution 3 would be the best overall (although most complex) in my opinion.
The text was updated successfully, but these errors were encountered: