Skip to content

Conversation

valexandru
Copy link
Contributor

@valexandru valexandru commented Jun 8, 2020

Pull Request Overview

This pull request adds support for the NXP i.MX RT 1052 Crossover MCU and i.MX RT 1052 EVKB Board.

Supported features so far are:

UART
GPIO
Leds
I2C
Timer

This also adds support for the Cortex M7 MCU.

This also adds the fxos8700 component.

Testing Strategy

This pull request was tested using the i.MX RT 1052 EVKB Board.

TODO or Help Wanted

This pull request still uses a binary FCB Header as header creation is still a problem. Looking into using https://github.com/imxrt-rs/imxrt-boot-gen . Would this be a problem with the licensing?

Added clock frequency Freq2475MHz for the General Purpose Timer.

Documentation Updated

  • Updated the relevant files in /docs.

Formatting

  • Ran make prepush.

New Platform Checklist

  • Hardware is widely available.
  • I can support the platform, at least initially.
  • Basic features are implemented:
    • Console, including debug!() and userspace printf().
    • Timers.
    • GPIO with interrupts.

@valexandru valexandru marked this pull request as draft June 8, 2020 10:49
@hudson-ayers
Copy link
Contributor

I don't think we want to make the scheduler timeslice configuration a kernel config option if we can avoid it. In general, I don't understand why the high speed of the MCU matters for those values anyway, as the configuration is specified in microseconds, and your systick implementation should convert that time value to ticks internally, taking into account the MCU frequency.

What issues were you encountering that made you decide to increase those values?

@lschuermann
Copy link
Member

Thanks for applying the changes of #1913. For the future, please try to avoid opening multiple PRs in order to have a consistent review and discussion history. You should be able to easily change source and destination branch in the GitHub interface, and rebasing or force pushing your branch is always an option.

@valexandru
Copy link
Contributor Author

valexandru commented Jun 28, 2020

I don't think we want to make the scheduler timeslice configuration a kernel config option if we can avoid it. In general, I don't understand why the high speed of the MCU matters for those values anyway, as the configuration is specified in microseconds, and your systick implementation should convert that time value to ticks internally, taking into account the MCU frequency.

What issues were you encountering that made you decide to increase those values?

I figured out the issue. Even though syst_calib.SKEW = 0 and syst_calib.NOREF = 0, the calibration value
was not corelated to the actual speed of the CPU. If I choose an external clock source, which selects
a 24 MHz oscilator or manually set the frequency according the CPU frequency the systick works properly.
I created a PR here: #1987 in case other boards will have similar issues or will need external clock source.

I also removed the edits in config.rs and sched.rs.

Sorry for the late answer.

@valexandru
Copy link
Contributor Author

valexandru commented Jun 28, 2020

Thanks for applying the changes of #1913. For the future, please try to avoid opening multiple PRs in order to have a consistent review and discussion history. You should be able to easily change source and destination branch in the GitHub interface, and rebasing or force pushing your branch is always an option.

Ok, I will remember next time. Thank you.

@valexandru valexandru marked this pull request as ready for review June 28, 2020 17:00
Copy link
Contributor

@hudson-ayers hudson-ayers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a couple comments from an incomplete pass

@valexandru
Copy link
Contributor Author

For some reason, the last two tasks: ci-qemu and deploy-preview are failing. For deploy preview, I can not even see the details. It just tells me: "You might not have permissions to see this page."

@hudson-ayers
Copy link
Contributor

For some reason, the last two tasks: ci-qemu and deploy-preview are failing. For deploy preview, I can not even see the details. It just tells me: "You might not have permissions to see this page."

We have been seeing spurious errors for both checks lately, don't worry about it for now. qemu.org being down is one of the issues

bradjc
bradjc previously approved these changes Nov 6, 2020
/// Vector which contains the FCB header
/// This section is extracted with objcopy from the hello world
/// example in the i.MX RT 1050 EVKB SDK examples.
pub const BOOT_HDR: [u8; 8192] = [
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't it possible to define the section of this directly in rust, without needing an assembly file?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the moment that is the only solution that I managed to implement. I am working with someone from NXP in order to see how to write to code for it, however it is really complicated and will take a lot of time to write.

hudson-ayers
hudson-ayers previously approved these changes Nov 6, 2020
Copy link
Contributor

@hudson-ayers hudson-ayers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me. The new chip/board will eventually need to be updated to use the new peripheral instantiation approach (#2069) but I think it can be merged as-is and switched over in a separate PR.

Co-authored-by: Brad Campbell <bradjc5@gmail.com>
@valexandru valexandru dismissed stale reviews from hudson-ayers and bradjc via 9466027 November 7, 2020 09:03
@valexandru
Copy link
Contributor Author

This looks good to me. The new chip/board will eventually need to be updated to use the new peripheral instantiation approach (#2069) but I think it can be merged as-is and switched over in a separate PR.

Ok, perfect.

@bradjc bradjc added the last-call Final review period for a pull request. label Nov 10, 2020
@bradjc
Copy link
Contributor

bradjc commented Nov 11, 2020

bors r+

@bors
Copy link
Contributor

bors bot commented Nov 11, 2020

Build failed:

@bradjc bradjc merged commit 370135f into tock:master Nov 11, 2020
This was referenced Nov 15, 2020
bors bot added a commit that referenced this pull request Dec 2, 2020
2200: Add Teensy 4 board r=bradjc a=mciantyre

### 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][teensy40] and [Teensy 4.1][teensy41] development boards. The PR builds on the i.MX RT chip foundation added in #1918.

[teensy40]: https://www.pjrc.com/store/teensy40.html
[teensy41]: https://www.pjrc.com/store/teensy41.html

The board makes use of

- the LED on pin 13
- UART2 on pins 14 and 15
- GPT1 as an alarm

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/imxrt1050` to `chips/imxrt10xx`*. I updated the `imxrt1050-evkb` board to use the renamed crate.

Additional changes to the i.MX RT chip crate include

- adding a LPUART2 peripheral
- renaming the `gpt1` module to `gpt`, and adding `GPT2`
- supporting periodic clock selection, allowing a user to select the static crystal oscillator as the GPT clock source

Changes to the chip crate should be backwards compatible for `imxrt1050-evkb` users. Let me know if we see an issue.

### Testing Strategy

I tested the PR by running `blink` and `console` libtock-c examples on both a Teensy 4.0 and 4.1 board. I repackaged the examples [here](https://github.com/mciantyre/tock-teensy4-apps).

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 `imxrt10xx` chip 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

- [x] ~~Updated the relevant files in `/docs`~~, or **no updates are required**

I've added documentation in `boards/teensy4`. The documentation

- lists the tools that you need to program a Teensy 4
- how to build the kernel and apps
- how to convert the program to a HEX file, which is necessary to program a board

### Formatting

- [x] Ran `make prepush`.

### New Platform Checklist

- [x] Hardware is widely available.
- [x] I can support the platform, which includes release testing for the platform, at least initially.
- Basic features are implemented:
  - [x] `Console`, including `debug!()` and userspace `printf()`.
  - [x] Timers.
  - [x] GPIO with interrupts.

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.


2216: add 15.4 and ble to nano33ble r=bradjc a=hudson-ayers

### Pull Request Overview

This pull request adds the 15.4 and BLE drivers to the nano33ble, rather than leaving support as commented out, as the comments had already fallen out-of-date.


### Testing Strategy

BLE was tested using the `ble_advertising` and `ble_passive_scanning` apps in `libtock-c`.

15.4 was tested using the `radio_tx` and `radio_rx` apps in `libtock-c` and sending messages back and forth with an nrf52840-dk.

### TODO or Help Wanted

N/A

### Documentation Updated

- [x] No updates are required.

### Formatting

- [x] Ran `make prepush`.


Co-authored-by: Ian McIntyre <ianpmcintyre@gmail.com>
Co-authored-by: Hudson Ayers <hayers@stanford.edu>
Co-authored-by: Hudson Ayers <32688905+hudson-ayers@users.noreply.github.com>
bors bot added a commit that referenced this pull request Jan 5, 2021
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
last-call Final review period for a pull request.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants