Skip to content
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

Clock configuration can be modified while it is in use #205

Open
hannobraun opened this issue Jan 30, 2020 · 0 comments
Open

Clock configuration can be modified while it is in use #205

hannobraun opened this issue Jan 30, 2020 · 0 comments

Comments

@hannobraun
Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant