Add Teensy 4 board #2200
Add Teensy 4 board #2200
Conversation
There was a problem hiding this comment.
I looked over most of this this evening, and this generally looks fantastic. I really appreciate the detailed comments through some of the more complex bits; this was really quite smooth to review. The fancy boot sector is interesting, haven't seen anything like that before.
The README for the board is great. It'd be nice to maybe have a few less manual steps and some supporting make targets that do that work for you, but that's not a blocker by any means.
Only real nit is the possibly spurious deletion of a frequency from the time HIL?
boards/teensy4/Makefile
Outdated
|
|
||
| # For testing with a blinky LED | ||
| BLINK=../../../libtock-c/examples/blink/build/cortex-m7/cortex-m7.tbf | ||
| .PHONY: program |
There was a problem hiding this comment.
| .PHONY: program |
| /// 24.75MHz `Frequency` | ||
| #[derive(Debug)] | ||
| pub struct Freq2475MHz; | ||
| impl Frequency for Freq2475MHz { | ||
| fn frequency() -> u32 { | ||
| 24750000 | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
#1918 introduced Freq2475MHz. But after this PR, it's unused. It's replaced by a frequency value computed at runtime, based on an input clock selection and divider.
I thought this removal, plus the change to compute clock frequency, would do for our current and future goals:
- It let me express the original author's intent -- run the general purpose timers (GPT) on the IPG clock -- while also supporting my goal: run the GPT on the oscillator clock, with root- and peripheral-level clock dividers*.
- From the original author's notes, this frequency would eventually change once we configured the ARM clock (which generates the IPG clock):
tock/chips/imxrt1050/src/gpt1.rs
Lines 247 to 253 in d695df5
Although the runtime frequency computation works, I'm looking for other suggestions to solve a perceived "how to I set a chip's clock frequency, based on the board?" problem. It seems I'm the first to propose a Frequency::frequency() implementation that doesn't return a constant value, and I'm not sure I want to set that example. Just want to make sure that it's OK to go with this approach.
* I prefer the 24MHz oscillator clock source since it's not affected by varing processor speeds, it's consistent across all i.MX RT processors, and it's fast enough for me.
boards/teensy4/layout.ld
Outdated
| } | ||
|
|
||
| INCLUDE ../kernel_layout.ld | ||
| ENTRY(_ivt); |
There was a problem hiding this comment.
Interesting -- is this actually used by the tooling?
There was a problem hiding this comment.
Nope! Just a leftover from some earlier prototyping.
I'll remove it. There's some other comments in this file that need correction.
boards/teensy4/README.md
Outdated
| - [Teensy 4.0 Development Board](https://www.pjrc.com/store/teensy40.html) | ||
| - [Teensy 4.1 Development Board](https://www.pjrc.com/store/teensy41.html) |
There was a problem hiding this comment.
It looks like (based on https://www.pjrc.com/store/teensy41.html) these two boards vary quite a bit. Can we choose just one that this has been tested on? Otherwise it's not clear how the linker file should be set, or if it is ok to add an sdcard driver.
There was a problem hiding this comment.
I updated the README to state that we guarantee support for the Teensy 4.0 board. The teensy4 crate will never include features that are only for the 4.1 board. I've left more thoughts in 6f08ab7's commit message.
Would you prefer to rename the crate to teensy40 as a stronger support signal? Or, could there be a future where the teensy4 board supports the common functions between 4.0 and 4.1, and we use cargo feature flags to enable 4.1 features?
There was a problem hiding this comment.
Typically, Tock avoids cargo feature flags for enabling features specific to a particular chip within a family of supported chips. Instead, Tock structures modules in a specific way so that more specific chips inherit from a library of shared functionality, re-exporting from the shared library all functionality that is unchanged by the more specific chip. For the best example of this, see chips/nrf5x', chips/nrf52, and chips/nrf52840`.
Given that you currently don't have any features only supported by one chip and not another, and have only tested on a teensy 4.0, I think it is best to name the chip crate teensy40. We can revisit that if explicit, tested support for the teensy 4.1 is added later.
That transition is still TODO and is not urgent, this can be merged first. And if you are able to support that cleanup in a separate PR I would really appreciate it :) |
The imxrt1050 implementation suffices for the imxrt1060 procesor family. I propose that we renamte the imxrt1050 crate to imxrt10xx so that we can share the drivers across the 1050 EVK board, and the Teensy 4 boards.
I'm keeping the imxrt1050 name intact, just for convenience. I'm happy to apply the rename throughout the EVK board crate, if that makes it clearer to the developers.
There are two general purpose timers. The commit removes the '1' suffix so that we could more easily support a second timer. The commit then adds that second GPT timer.
This follows the convention established in the other Cortex-M arch crates, and it lets us remove the implementation in the imxrt10xx chip crate.
- remove unhandled_interrupt implementation - remove direct dependency on cortexm arch crate; all the useful bits are re-exported by the m7 crate.
Integrate the teensy4 board with the imxrt10xx API.
i.MX RT GPT clocks can run off of the IPG clock root (default clock), or the crystal oscillator. Before this commit, the GPTs were using the IPG clock, which was assuming a default value. However, this value will change when we update the ARM clock speed, since the IPG clock is derived from the ARM clock root. We'd like to use the oscillator as the GPT clock source, since the oscillator frequency isn't susceptible to clock scaling. This commit updates the GPT to support either IPG or crystal oscillator selection. We extend the CCM interface to permit perclk selection and dividing. We then extend the GPT start method to select the appropriate clock. Since the clock frequency now varies at runtime, we remove the 2475MHz frequency type, which provides a static frequency value. We implement a dynamic frequency inside the GPT module. Each GPT dispatches to the dynamic frequency value in calls to hil::timer::Timer. The commit should be backwards compatible. We patched the imxrt1050 EVK board to continue using the IPG clock frequency. In the teensy4 board, we select the crystal oscillator and a divider of 8. This results in a 1MHz GPT frequency, perfect for 1us intervals.
- change imxrt10xx to imxrt1060, just in case we end up adding chip- specific imxrt crates in the future - cleanup, remove comments
- superfluous PHONY in makefile - ENTRY in linker script was from early prototying, and it doesn't do anything in the output hex image - remove comment about "prebuilt binary;" another leftover from linker script prototyping - correct FCB terminology: it's a FlexSPI configuration block. The Firmware Configuration Block (FCB) is specific for serial NAND flash (at least, that's what I gather from the reference manual...)
In our first review, we identified that we may want to only support one physical board from the Tock board crate. I've selected the 4.0 development board, since - it's a little older than the Teensy 4.1, which should mean that it's easier source, and easier to find documentation and guides - it's cheaper than the Teensy 4.1, so should be more accessible - it's the board that I use for development and testing The Teensy 4.1 board is a "bigger" 4.0 board with more features. There are many commonalities: they can share a linker script, they share the same chip, and they have the same pinout for all pins 0 - 33. If it works on the Teensy 4.0, it most likely also works on the Teensy 4.1 board. The README makes these ideas clear. We guarantee support for the 4.0 board, and we note that the crate may work on the 4.1 board. We will not accept 4.1-specific features in this board. We can consider how to support Tock on the 4.1 boards later. Couple of options could be - feature flags to conditionally add 4.1-specific features - a completely separate board crate
The commit further specializes the board for the Teensy 4.0 hardware.
|
bors r+ |
1 similar comment
|
bors r+ |
2311: Board based instantiation of chip drivers and interrupt mappings: imxrt10xx r=bradjc a=mciantyre ### Pull Request Overview The PR refactors the `imxrt10xx` chip and boards to support board-based initialization (first proposed in #2069). It builds on the work of #1918 and #2200, where we noted the effort as a TODO. After this PR, you may instantiate and configure i.MX RT peripherals in a board, or use the default peripherals provided by the chip. Most peripherals were easily transitioned to the new API. The exception was the GPIO driver, which referenced `static` GPIO ports and pins throughout the code. I may have found some issues in the driver, so I took the refactoring opportunity to update the implementation, trying to follow [this suggestion](https://github.com/tock/tock/blob/master/chips/stm32f4xx/src/gpio.rs#L595). I'll leave more details in review comments. Other changes include - fix a GPT clock gating bug introduced in #2200 - move UART root clock initialization out of the `configure()` method, and into CCM peripheral setup ### Testing Strategy Tested `boards/teensy40` on a Teensy 4.0 using the tests from #2200. `boards/imxrt1050-evkb` continues to compile, but I don't have the hardware to test examples. ### TODO or Help Wanted Looking for feedback on the refactor. Let me know if it deviates too much from the other chips. Holding as draft to - [x] ~~wait for #2310, since the new GPIO driver needs `min_const_generics`~~ accepting as-is, and we'll remove the feature later - [x] figure out a way to remove the remaining `static` peripheral in `iomuxc_snvs` - [x] remove `std` dependency in new unit tests, avoiding the conditional `no_std` in the crate - [x] leave thoughts on GPIO changes - [x] clean up commit messages ### Documentation Updated - [x] ~~Updated the relevant files in `/docs`, or~~ no updates are required. ### Formatting - [x] Ran `make prepush`. Co-authored-by: Ian McIntyre <ianpmcintyre@gmail.com>
Pull Request Overview
The PR proposes Tock support for the Teensy 4. As of this PR, the Tock kernel, and a variety of small examples, can run on Teensy 4.0 and Teensy 4.1 development boards. The PR builds on the i.MX RT chip foundation added in #1918.
The board makes use of
The Teensy 4 boards use i.MX RT 1062 chips. Given our current chip features, the 1060 chip family is identical to the 1050 chip family. Before integrating the Teensy 4 board, I renamed
chips/imxrt1050tochips/imxrt10xx. I updated theimxrt1050-evkbboard to use the renamed crate.Additional changes to the i.MX RT chip crate include
gpt1module togpt, and addingGPT2Changes to the chip crate should be backwards compatible for
imxrt1050-evkbusers. Let me know if we see an issue.Testing Strategy
I tested the PR by running
blinkandconsolelibtock-c examples on both a Teensy 4.0 and 4.1 board. I repackaged the examples here.The PR was not tested on an NXP i.MX RT 1050 evaluation board; I don't have that hardware.
TODO or Help Wanted
This pull request may be tested on a 1050 evaluation board. @valexandru, if you have an opportunity to test this work and review these changes, I'd appreciate it!
This PR does not address any of the TODOs noted in #1918. In particular, the
imxrt10xxchip still does not use the new peripheral instantiation approach (#2069). If this is still TODO and not urgent, I'm happy to support that cleanup in a separate PR.This Teensy 4 support was based on a different chip implementation. That chip implementation supported DMA, and a UART driver that used DMA. If we see value in a DMA driver for i.MX RT chips, I'm happy to contribute the driver.
Documentation Updated
Updated the relevant files in, or no updates are required/docsI've added documentation in
boards/teensy4. The documentationFormatting
make prepush.New Platform Checklist
Console, includingdebug!()and userspaceprintf().I assume the basic chip features were provided by #1918. Let me know if we need to make all three features available through the board. As of this writing, the Teensy 4 board does not expose an input GPIO that responds to an interrupt.