search

imxrt-rs / imxrt-hal

Rust for NXP i.MX RT
Apache License 2.0
122 stars 29 forks source link

RFC: A 0.5 HAL to support more i.MX RT chips #121

Closed mciantyre closed 1 year ago

mciantyre commented 1 year ago

I've infrequently played around with new imxrt-hal ideas. This PR summarizes those ideas. It's a re-write of the 0.4 HAL, a package which only supports 1060 chips. This PR is big and incomplete, but I wanted to talk about ideas with working code.

Unlike today's HAL, this exploratory HAL

This new HAL supports almost all peripherals from the 0.4 release, but everything is a breaking change.

Asks

If you have the time, here's what I'm hoping to get out of this discussion:

I don't expect a line-by-line review right now. After we discuss, I'll clean up the history, and make it easier to review and merge.

Try it out

All examples build and run on the

A number of examples also work for the Cortex M7 of the i.MX RT 1170 EVK. This chip support is WIP.

After pulling this branch, you can quickly light your Teensy's LED:

cargo objcopy --features=board/teensy4 --example=hal_led --target=thumbv7em-none-eabihf -- -O ihex hal_led.hex
# Load 'hal_led.hex' using your favorite Teensy tool.

You can also light your i.MX RT 1010 EVK's LED with cargo flash. You need an unreleased version of the tool:

cargo install cargo-flash --git=https://github.com/probe-rs/cargo-flash.git
cargo flash --features=board/imxrt1010evk --release --chip=mimxrt1010 --example=hal_led --target=thumbv7em-none-eabihf

See board/README.md for more information on board support. For tips on adding new boards, see the CONTRIBUTING guide.

What's new?

Design

The HAL implements #92. Drivers adapt resources from imxrt-ral and imxrt-iomuxc to provide a higher-level API and embedded-hal support. Clock control for peripherals is implemented separately, unlike in today's peripherals.

HAL drivers use const-generic instances from imxrt-ral (imxrt-rs/imxrt-ral#24) and pair them with const-tagged resources from imxrt-iomuxc (imxrt-rs/imxrt-iomuxc#10). This lets us statically ensure that peripherals and pins work together, as we have today. It also lets us express some const-heavy APIs, like the ones use for CCM clock gating (impl, example).

Usage and structure

imxrt-hal supports multiple chips from one crate. There's common modules for all imxrt-ral targets. The common modules need one imxrt-ral feature, and zero imxrt-hal features, to build. See the HAL API docs for more info on this idea.

There's also conditionally-compiled modules which use coarse features to target select chips. Internally, the package uses module path attributes and configuration modules to maximize code reuse. If you want more info, I ramble about this in the CONTRIBUTING doc. I'll stress that this is not #56. It was a prototyping convenience, and I think it gives us a path towards a split HAL.

CCM and DMA drivers are back in the HAL. Just another prototyping convenience. I'm happy to split these back out if we think they're useful elsewhere (though I don't feel I demonstrated that utility, and there's less value when HALs aren't split).

Extras

There's a new logging package to supersede imxrt-uart-log. It supports both log and defmt, and it works with both USB serial and LPUART peripherals. This was my reason to explore a high-speed USB driver; that's in its own repo.

There's an experimental runtime, imxrt-rt, that can boot all three boards. This should make it easier to bring up new boards with various FlexRAM / OCRAM / flash configurations.

11xx support

imxrt-rs/imxrt-ral#26 notes early issues when trying to add common HAL support for 11xx chips:

  1. Clusters and register arrays can be complex, and imxrtral.py doesn't handle that complexity.
  2. Symbols in 11xx SVDs differ from symbols in 10xx SVDs, so the HAL code isn't portable without patching.
  3. Today's imxrt-ral resource management doesn't work for all multi-core cases.

I tackle the first two issues with a new imxrt-ral code generator. I forked chiptool and turned it into raltool. raltool emits an imxrtral.py-compatible register access layer, making it easy to adopt the tool output. The chiptool intermediate representation made it easy to handle issue 1. The IR also made it easier to implement a new "combine" phase that consolidates peripherals across devices. The combine phase and the chiptool transforms let us solve 2 without patching SVDs. Quick testing shows that raltool-generated crates build faster than svd2rust- and chiptool-generated crates, so changing the chiptool codegen was for more than just easier adoption.

I'm punting problem 3 by removing any notion of resource management. imxrt-ral no longer implements a resource management policy. Acquiring a RAL instance is unsafe, no matter what chip you're targeting. This aligns with the original chiptool codegen. In our case, it keeps the RAL simple, and lets us explore resource management separately / later.

Next steps

I think this is ready for feedback, but it's not ready to merge. It's not at parity with the 0.4 HAL, and dependencies are in various states of development. These are some of my TODOs I think this needs before it's merged.

HAL TODOs

Ecosystem TODOs

Finish work, clean up commits, then upstream changes to all dependencies:

teburd commented 1 year ago

I tried the sample listed for the 1010 and it Just Worked which is fantastic.

Having recently used chiptool myself for something else, I'm happy to see it getting reused here. I'd like to think the author might accept a different output generator using the same intermediate representation that it builds up. It would be very cool to see the ral like output upstreamed. @adamgreig @dirbaio any thoughts?

The drivers in common (e.g. lpi2c.rs, lpspi.rs, etc) look super clean and the docs are fantastic.

Do note that I2C terminology in the spec (from NXP) has been changed from Master/Slave to Controller/Target.

I'd also note that cargo doc didn't work out of the box for me and there's no instructions in the README to build them.

Overall this seems like a really nice direction. I think one of the best parts is the shared board crate that the examples can use. This will hopefully save a lot of time in adding new boards.

mciantyre commented 1 year ago

I tried the sample listed for the 1010 and it Just Worked which is fantastic.

Thanks for the test and feedback @teburd.

I'd like to think the [chiptool] author might accept a different output generator using the same intermediate representation that it builds up.

I don't have immediate plans to upstream the raltool backend, but we can talk more in imxrt-rs/imxrt-ral#29.

Do note that I2C terminology in the spec (from NXP) has been changed from Master/Slave to Controller/Target. [...] I'd also note that cargo doc didn't work out of the box for me and there's no instructions in the README to build them.

Updated in 81c0176, and documented in d4472cf.

I'm accumulating dependent imxrt PRs in the top-level description. I plan to start merging them within the next few days. If there's interest in reviewing, add yourself as a reviewer or leave a note on the PR. I'll keep them in review.

teburd commented 1 year ago

I tried the sample listed for the 1010 and it Just Worked which is fantastic.

Thanks for the test and feedback @teburd.

Its about the most I can do at the moment unfortunately

I'd like to think the [chiptool] author might accept a different output generator using the same intermediate representation that it builds up.

I don't have immediate plans to upstream the raltool backend, but we can talk more in imxrt-rs/imxrt-ral#29.

Yeah right on makes sense

mciantyre commented 1 year ago

Status update for those following along:

mciantyre commented 1 year ago

All imxrt-rs-maintained dependencies are now available on crates.io, so this work is almost ready for a release. We're waiting on ral-registers support; I'll check in on the review once we're in the new year.

I cleaned up these commits and orphaned them into the main branch of my fork. Note that they're not connected to master, our primary development / integration branch. I'm proposing that we use something resembling those commits as our new primary branch. Here's my thinking:

Let me know if you disagree with this approach, either now or later. I consider this low-risk and reversible; I'm happy to rebase the orphaned commits back onto master if we're concerned about losing development history. Otherwise, I'll do some branch renaming in this remote to change the primary branch and maintain the older master branch.

mciantyre commented 1 year ago

I'm proposing that we use something resembling those commits as our new primary branch.

This work is now the main branch of this remote. The former master branch is now called pre-v0.5.

I've released this work as imxrt-hal 0.5.0. Looking forward to getting more users and feedback as we play with it.

teburd commented 1 year ago

Fantastic!