-
Notifications
You must be signed in to change notification settings - Fork 34
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
[WIP] RFC: construct drivers from RAL instances #92
Comments
I'm definitely a bigger fan of what I think is a simpler pattern of usage in async-hal with regards to creating peripheral drivers, clocking them (individually even, which is nice and how the hardware works) and definitely feel like it's a good place to build from for a next version of this hal. I was looking to port the clock interface work you've done in async-hal especially as I quite like the way you've done it there. |
Thanks for the feedback. I've already separated that module into it's own crate, and I've integrated it back into the async HAL. I'll make the crate available for all of us soon. |
This has found its way into imxrt-hal through #121, and it's the main way to construct drivers in the 0.5 release. Reflecting on the original discussion:
0.5 has dropped the explicit RTIC support. Once users acquire their imxrt-ral instances -- either from RTIC or from another API -- they can start constructing imxrt-hal drivers. We have hardware examples in this repo that show one way to achieve this.
We've realized this in 0.5 of imxrt-ral. It plays well with the constified imxrt-iomuxc types.
0.5 drops the "enforce drive clocking" part of this statement. I don't think I ever found a nice, higher-level CCM design that would make this as usable and flexible as I would like. The 0.5 CCM API ends up being a thin, better-named layer over imxrt-ral. Users (BSPs, higher-level libraries, application developers) are responsible for making sure clocking states make sense for their drivers. |
This more of a brain dump, not a well-formed RFC. I don't think it's ready for feedback. That being said, feel free to leave early thoughts if you see a thread of ideas. I'll revise this, then assign reviewers, when ideas are more coherent. My TODOs are emphasized throughout.
The RFC proposes that we change HAL driver initialization. Today, we have single
Peripherals
object that userstake()
orsteal()
. Instead of this API, we should let users supply HAL drivers with RAL instances, while still enforcing the driver states that we require.Background
There's a few reasons we have the
Peripherals
object as the HAL driver entrypoint:svd2rust
-generated peripheral access crate (PAC). This was the PAC API at the time, which we lifted into our API.take()
orsteal()
methods, without any extra thinking.Downsides
Suppose a user wants to directly use the RAL API to control UART, but they want to use the rest of our HAL drivers. After calling
Peripherals::take()
, the user will need to unsafely steal the RALLPUART2
instance. This is becausePeripherals::take
indiscriminately takes all of the RAL instances, leaving nothing for the user. Flip the sequence: the user safely takes the RAL'sLPUART2
instance, then callsPeripherals::take()
. The latter returnsNone
, since the implementation sees thatLPUART2
was already taken. The user needs to unsafely steal all of the peripherals. In either sequence, we see unsafe.The approach requires that we mimic PAC APIs to support other embedded Rust frameworks. In #69, we implemented a
steal()
method onPeripherals
to accommodate RTIC's requirements. Without this change, there would be no safe way for users to combine RTIC with theimxrt-hal
. An alternative initialization API would let us use the HAL without duplicating API requirements in both our RAL and HAL.Proposed Approach
HAL drivers accept RAL instances as inputs. We continue to enforce the driver clocking and other validity states through the type system.
(Insert self-contained, minimal example here. In lieu of that self-contained example, see the WIP
imxrt-async-hal
, which has prototyped the driver initialization flow I'm envisioning.)Requirements
imxrt-iomuxc
crate. Specifically, we need to catch incorrect pairings of RAL peripheral instances with pads (i.e. you cannot combine a LPUART1 RAL instance with a LPUART2_TX pad). This should be caught at compile time.Discussion
This seems to be the favored approach in other Rust HALs. Recent study of STM32 HALs show a pattern of HAL drivers that accept PAC peripheral instances. We observe the same pattern in nrf-hal, which we're using as a model in #56.
The HAL would no longer need to explicitly support RTIC. The RAL is already compatible with RTIC. We teach users that, to use RTIC, you need to use the RAL, then construct HAL drivers from the RAL components.
Prototyping in
imxrt-async-hal
has shown ways to meet the three requirements, though they could be improved. Requirement 1 needs strongly-typed RAL instances, which we don't have. Strongly-typed RAL instances would signal their module number (the '2' in LPUART2) in the type system. To overcome this, the async HAL uses theinstance
interface. This incurs a runtime check, but it lets us meet the compile-time requirement, and it localizes an invalid state to a single call site. A new RAL API that signaled strongly-typed instances could help, but that comes with other trade-offs.Requirement 2, specifically the clocking states, is enforced by combining the async HAL's
ccm
interface with peripheral initialization APIs. (Shown throughout the async HAL interfaces; see the code until I add more ideas here.)The text was updated successfully, but these errors were encountered: