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

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

Closed
wants to merge 103 commits into from

Conversation

mciantyre
Copy link
Member

@mciantyre mciantyre commented Sep 26, 2022

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

  • provides common peripherals for all i.MX RT chips supported by imxrt-ral, including multi-core 1176 chips.
  • maintains hardware examples and tests in tree, and supports development on three different boards.
  • provides a path towards a split HAL (RFC: A Split HAL #56).

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:

  • Try out examples on hardware. Better yet, extend the board package, and see if you can turn on your board's LED. Ping me below or on the imxrt-rs Matrix chat for help / feedback / complaints.
  • Review the higher-level changes (below) and the HAL API. Testing shows that drivers are no worse than what we had in the 0.4 HAL. I also tried to fix issues like Spurious CS high when using SPI enable_chip_select_0 #111. I want to know if you think this is or isn't an improvement, and if we could use this as a foundation for future development. "Future development" includes adding new drivers, defining BSPs, supporting the broader embedded Rust ecosystem, and whatever else you want to do.
  • If you like what you see, let me know what you think some next steps are. I have my list below.

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

  • Teensy 4 (4.0 and 4.1).
  • i.MX RT 1010 EVK.

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

  • Port the TEMPMON peripheral.
  • Re-introduce ADC DMA support.
  • Finish LPSPI-DMA integration.
  • Build the HAL without warnings in CI.
  • Double-check all embedded-hal 0.2 implementations; make sure they're all there.
  • Implement the relevant embedded-hal 1.0 alpha interface.
    • De-scoped. Alpha releases have been a moving target, and I've no personal need for them. Happy to help others contribute here, if there's a need for alpha support.
  • Drop the inline FCB in board/src/imxrt1010evk.rs; use imxrt1010evk-fcb.
  • Support more examples on the Cortex M7 of the i.MX RT 1170 EVK.
  • Re-introduce RTC and SNVS driver.
  • Clean up commits.

Ecosystem TODOs

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

Delete all old sources, and prepare to checkout sources from the
prototyping branch.
The LED example works on both the teensy4 and imxrt1010evk boards. Use
the 'run' Cargo aliases to flash. You'll need additional tools to flash.

Need documentation on flashing.
The CCM API is broken into a few separate modules:

- clock_gate provides a named API for interfacing peripheral clock
  gates.
- clock_tree builds on root clock modules, providing presets for system
  and peripheral clocks.
- all other modules are root clock modules.

ccm doesn't try to encapsulate the CCM and CCM_ANALOG peripherals;
instead, it just works with references to these peripherals.

The DCDC follows a similar design, though it's very simple today.

The clock_tree API depends on the RunMode configuration.
Only enable RTIC examples when the example's RTIC feature is enabled.
Preferring to develop the DMA driver with the HAL for convenience.
Taken at commit 7693b78 with slight lifetime modifications on the
futures.
RTIC 1.0 behavior loops on nop. We can wait for interrupts instead.
TODO: Document the example. Describe how the menu works.
TODO: Finish testing all clocks. AHB, IPG, and PERCLK look great.
TODO: Fix runtime and memory layout. We can't feasibly put all things in
      the 1010's OCRAM. Needed to optimize for size in order to get the
      clock_out example linking.
Move this into a runtime support crate
It's cheap to copy clock gate locators, and we can remove accesses to
read-only memory by inlining constants.
Pairs with latest changes in cortex-m-rt fork. The reset handler now
conditionally copies instructions, vector tables, and read-only data to
their proper regions. Use the released 'set-vtor' feature to set VTOR
to the VMA region.
Use pyOCD to observe the panic message:

  pyocd rtt --target=mimxrt1010 --address=0x20000000 --size=0x8000
Board is now a helper crate linked into the HAL as a dev dependency. The
RAL shim library is now part of the board package.
Would need to pass &mut CCM reference through the callback. It's
annoying.
Mapped directly from the feature.
Slim down the config modules, and permit the CCM / DMA modules to
directly reference chip-specific APIs.
Add *_with_config functions to handle advanced frontend and / or backend
configurations.
Played around with adding more function pointers. Even though they're
not in this commit, this makes it easier to add them in the future.
@mciantyre
Copy link
Member Author

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
Copy link
Member

teburd commented Nov 20, 2022

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

Refactor configurations in build.rs.
Move CCM items into a CCM namespace.
Use recent releases of each package.
@mciantyre
Copy link
Member Author

Status update for those following along:

  • All dependencies with a check in the PR description have a release on crates.io.
  • I'm waiting for feedback on the ral-registers PR. We'll need that feature in a new release.
    • If review stalls, I may re-integrate the module into imxrt-ral, as it was before. I'm holding off on an imxrt-ral release to make this option easier.
  • I missed the RTC driver. Need to port and test that driver.
  • Still need to clean up this branch.

It's more flexible than the clock-tree-dependent approach, and it means
the LPI2C module is completely stand-alone. The routine to compute
timing settings assumes negligible rise times and no need for filters.
It's inspired by the 0.4 imxrt-hal routine, but it runs in a const
context.
Previously, the SRTC driver took the entire set of SNVS registers. This
new design separates the SNVS registers by their functional
capabilities. This should make it easier for us to add more SNVS
capabilities without introducing breakages.

The SNVS modules, and its submodules, are under common. The items
exposed by today's modules are portable across 10xx and 11xx MCUs.
However, there do seem to be some small differences in some register
names; I'm coming to this conclusion after just a quick imxrt-ral study.
When adding future SNVS features, we'll need to make sure that they can
actually fit within common. Otherwise, we can take a split approach, as
demonstrated by DMA.

The new srtc example works on all three supported boards.
Includes all of the same content as the manual re-export, and also
includes all of the modules for pin traits and type tags.
Two new examples demonstrate how to use a GPIO as an input. The board
module configures the pad with a PU / PD, and the examples toggle an LED
state based on the input's state or change.

The 1010EVK has the easiest experience, since it already has an on-board
user button. I'm choosing Pin 7 of the Teensy 4 for a external switch.

The two boards now have a BOARD_BUTTON interrupt handler. I'm not sure
how this will work if we start adding more GPIO inputs. The approach I'm
taking bets that this isn't going to happen. Otherwise, we would need to
figure how how to keep the examples consistent while varying how ISRs
are mapped to their vectors.

There's no support for the 1170EVK in this commit. The buttons on that
board are routed to IOMUXC_SNVS pads, which we're not exposing from
imxrt-iomuxc. At this time, it's non-trivial to introduce those pads, so
the next commit adds a HAL work-around to avoid the limitation.
The last commit message mentioned that there's no imxrt-iomuxc API for
IOMUXC_SNVS pads. The HAL can work around this limitation by letting
users manually define the GPIO inputs and outputs. This effectively
works around the pad resource management strategy, but I still think its
safe. Users can elect not to use this API if they don't want to.

With this in place, we can manually configure the 1170EVK's WAKEUP input
using RAL APIs, and then fabricate a GPIO input. The two examples work
as expected.
With the recent imxrt-iomuxc 1170 big bug, and given that we're slow to
add pin implementations, I wanted to make sure HAL drivers are still
useful regardless of the imxrt-iomuxc state. This commit adds
constructors and methods so that we can use I/O drivers without their
pin objects. Users who choose this API can configure their pins
themselves. They should use type aliases that use the unit type '()' for
the generic pin 'P' type argument.

A previous commit effectively did this for the GPIO inputs and outputs,
so this is an equivalent change for other drivers. Most of the code had
a small scope for Pin trait constraints, so I'm expecting that nearly
all methods still work without any more changes.
Bringing in the DMA package was just a development convenience. This
commit drops those files. I've re-exported the changes to the imxrt-dma
module.
Allows the user to re-acquire the register block.
Recommended by the rustdoc book, and applies to both the HAL and the
logging crate. This commit also adds documentation for items that were
missing documentation.
@mciantyre
Copy link
Member Author

mciantyre commented Dec 30, 2022

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:

  • Changing the primary branch lets us easily ignore imxrt-ral and imxrt-iomuxc commits that were made in this repository. Those commits are already in their respective repositories.
  • Most drivers are re-written, so tracing from master may not be meaningful.
  • A larger change like this seems like a good opportunity to clean up the commit history while maintaining contributor attributions.

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
Copy link
Member Author

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.

@mciantyre mciantyre closed this Jan 5, 2023
@teburd
Copy link
Member

teburd commented Jan 5, 2023

Fantastic!

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

Successfully merging this pull request may close these issues.

None yet

2 participants