imxrt-hal icon indicating copy to clipboard operation
imxrt-hal copied to clipboard

[WIP] RFC: construct drivers from RAL instances

Open mciantyre opened this issue 3 years ago • 2 comments

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 users take() or steal(). 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:

  • We previously used an svd2rust-generated peripheral access crate (PAC). This was the PAC API at the time, which we lifted into our API.
  • The approach lets us conveniently enforce driver states in the type system, without any extra thinking. For instance, we signal that all drivers are in an "unclocked" state, which requires that you "clock" them. We can insert this type state before users have any other ways to get the driver.
  • The approach let us include global system setup directly in the take() or steal() methods, without any extra thinking.
  • I thought I saw other HAL crates do this.
  • (Other reasons we did this that I don't remember...)

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 RAL LPUART2 instance. This is because Peripherals::take indiscriminately takes all of the RAL instances, leaving nothing for the user. Flip the sequence: the user safely takes the RAL's LPUART2 instance, then calls Peripherals::take(). The latter returns None, since the implementation sees that LPUART2 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 on Peripherals to accommodate RTIC's requirements. Without this change, there would be no safe way for users to combine RTIC with the imxrt-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

  1. Compatible with the 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.
  2. Enforce driver clocking, validity states through the type system.
  3. Amenable to a configure / release pattern (#20).

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 the instance 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.)

mciantyre avatar Oct 17 '20 15:10 mciantyre

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.

teburd avatar Oct 20 '20 19:10 teburd

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.

mciantyre avatar Oct 20 '20 22:10 mciantyre