Skip to content

ports/mimxrt: Restructure nxp_sdk to match official mcux-sdk. #16235

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

ports/mimxrt: Restructure nxp_sdk to match official mcux-sdk. #16235

wants to merge 2 commits into from

Conversation

andrewleech
Copy link
Contributor

Summary

The official mcux-sdk follows a slightly different structure to the current nxp_sdk submodule with many drivers moved to a common location.

To ease updating to newer versions of the SDK and/or adding new families the nxp_sdk submodule has been updated to follow the structure of mcux-sdk, just trimmed down to families used here to considerably reduce the size.

This PR is an update on #11516
I've retargetted it to an updated nxp_driver submodule pointing directly at my fork; tinyusb are no longer using this repo so I believe it makes sense to move it here. I'd be quite happy for it to be moved from my github to micropython, but that is a separate discussion really.

The initial push of this in 4df2f8d is still based on mcux-sdk 2.11 which is equivalent to the existing version of nxp_driver, however it's been updated to match the official release from https://github.com/nxp-mcuxpresso/mcux-sdk/archive/refs/tags/MCUX_2.11.0.tar.gz using the automated update script in hathach/nxp_driver#10

Testing

I've built all existing boards to ensure they at least compile using:

for board in $(cd ports/mimxrt/boards; find * -type d); do [ -e ports/mimxrt/boards/$board/mpconfigboard.mk ] && make -C ports/mimxrt BOARD="$board" -j18; done

I'll be continuing to work with this branch on an mimxrt-1176 so will update with test results here.

Trade-offs and Alternatives

Originally #11516 was switching directly to the official sdk repo. Speaking to @dpgeorge there was a preference to keep using a trimmed down repo considering the dramatic difference in size (official ~1.6GB, trimmed < 100MB).

The changes made by @imi415 to work with the official sdk layout of the repo are solid though and a better path forward than my previous efforts to use symlinks in the trimmed nxp_sdk repo to emulate the original layout, hence using their changes here

The authorship of the commit has been kept as you @imi415 as the vast majority of the changes are still as per your PR. I hope you don't mind me making the minor tweaks for compatibility and squashing it together.

@andrewleech andrewleech mentioned this pull request Nov 14, 2024
Draft
@github-actions GitHub Actions
Copy link

github-actions bot commented Nov 14, 2024 •
edited
Loading

Code size report:

   bare-arm:    +0 +0.000% 
minimal x86:    +0 +0.000% 
   unix x64:    +0 +0.000% standard
      stm32:    +0 +0.000% PYBV10
     mimxrt:  -224 -0.061% TEENSY40
        rp2:    +0 +0.000% RPI_PICO_W
       samd:    +0 +0.000% ADAFRUIT_ITSYBITSY_M4_EXPRESS
  qemu rv32:    +0 +0.000% VIRT_RV32

@codecov Codecov
Copy link

codecov bot commented Nov 14, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.54%. Comparing base (016ae19) to head (d2c620f).

Additional details and impacted files 
@@           Coverage Diff           @@
##           master   #16235   +/-   ##
=======================================
  Coverage   98.54%   98.54%           
=======================================
  Files         169      169           
  Lines       21864    21864           
=======================================
  Hits        21545    21545           
  Misses        319      319

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@andrewleech andrewleech force-pushed the mcux_sdk branch 2 times, most recently from 679b0a7 to 952383f Compare November 14, 2024 02:03
@andrewleech andrewleech force-pushed the mcux_sdk branch 3 times, most recently from 870da96 to eafe053 Compare December 17, 2024 04:37
@andrewleech andrewleech force-pushed the mcux_sdk branch 2 times, most recently from 06b15f0 to 8d2ea2d Compare January 31, 2025 04:40
@robert-hh
Copy link
Contributor

Thanks for your ongoing work on this. It will take a few days before I can test it. The wrapper for fsl_lpuart.c looks cool. Did you test that to fire at the right moment?
I'm working at the moment on changing the way flash devices are defined. At the moment it's a single flash_config.c file for all boards. But that does not properly account for the difference in the various flash devices. So I change than to have a flash_config.c per board. Alternatively it could be a flash_config.c per flash type, since it seem to be just three flash families being used: Winbond, ISSI and Adesto.

@andrewleech
Copy link
Contributor Author

The wrapper for fsl_lpuart.c looks cool. Did you test that to fire at the right moment?

I was working in that last year now and can't quite remember what I tested on it. I did test a few things to do with the interrupts and wrappers because I was making a second similar change elsewhere.
I think I broke the functionality that depends on this idle hook though and then this wrapper fixed it so I decided it must be ok.

I'm working at the moment on changing the way flash devices are defined.

Neat! I'd love to consolidate spiflash handing across ports more, at least in terms of definition differences between the chip vendors. That's a much bigger job though...
In this case my vote would be for a common file/files that define the chips that can then just be referenced in the board mk/h files to select the one used.

@robert-hh
Copy link
Contributor

In this case my vote would be for a common file/files that define the chips

Looking at it in detail it gets less clean. The Winbond devices have minor difference, so it cannot be just a Winbond type of definition. So I'm working on making a flash_config per board and reducing the #defines for chip properties set in this definitions. Then everything is at a single place. The only #define that would stay is the one for the flash clock frequency, which may have to be changed at runtime.

robert-hh referenced this pull request Feb 22, 2025
@robert-hh @dpgeorge
mimxrt/machine_uart: Implement a Python UART IRQ handler.
b7fa4e2 
Supported triggers are: IRQ_RXIDLE and IRQ_TXIDLE.

When IRQ_RXIDLE is set, the handler will be called 3 character times after
the data in burst stopped.

When IRQ_TXIDLE is set, the handler will be called immediately after the
data has been sent.

This commit requires a change to fsl_lpuart.c, because the existing code
does not support under-run appropriately.

The irq.flags() value is cleared only at an expected event.  Do not change
it otherwise.

Signed-off-by: robert-hh <robert@hammelrath.com>
@robert-hh
Copy link
Contributor

@andrewleech Did you make any progress in this task?

@andrewleech
Copy link
Contributor Author

Hi @robert-hh other than the new merge conflict to resolve I do think this PR is in pretty good shape.

@robert-hh
Copy link
Contributor

That is a trivial conflict about just one #include statement in mpconfigport.h being present or not.

#include "lib/cmsis/inc/core_cm7.h"

I tried to rebase onto the common master, with a rebase glitch in machine_rtc.c.
In May 2022 I had set up short list of to-do's for the switch from 2.10 to 2.11:

  • enable retry-timer for I2C, SPI, UART and return timeout error
  • format clock_config.c files with uncrustify
  • reduce backport and fallback code from hal/backport.*, but not sdcard.c

Not sure how much of that is still valid. The last bullet is a lot of work, but not for you.

@robert-hh
Copy link
Contributor

I made a few tests with that PR, building all boards and running functional tests with

  • MIMXRT1010_EVK / Adafruit Metro M7
  • MIMXRT1020_EVK
  • Teensy 4.1
  • MIMXRT1176_EVK

Tested festures:

  • UART with UART_IRQ_RXIDLE
  • SPI, using a SPI attached SD card
  • I2C in combination with the I2S test
  • I2S
  • ADC
  • PWM
  • Ethernet, where applicable (not i.MX RT 1011)
  • Built-in SD card supprt, where applicable (not i.MX RT 1011)

Result:

  • All boards build.
  • The above listed tests pass.

@andrewleech
Copy link
Contributor Author

andrewleech commented Mar 3, 2025 •
edited
Loading

Thanks for the testing @robert-hh that sounds great! I've just rebased to current master, hopefully it's essentially the same as what you tested (just resolving the conflict by removing the include, no other manual changes).

edit: Ok I see the rebase/merge fail in machine_rtc.c, that's fixed now.

imi415 added 2 commits March 3, 2025 11:45
@imi415 @pi-anl
ports/mimxrt: Restructure nxp_sdk to match official mcux-sdk.
747b494 
The official mcux-sdk follows a slightly different structure to the
current nxp_sdk submodule, with many drivers moved to a common location.

To ease updating the newer versions of the SDK and/or add new families
the nxp_sdk submodule has been updated to follow the structure of
mcux-sdk, just trimmed down to families used here to considerably
reduce the size.

Signed-off-by: Andrew Leech <andrew.leech@planetinnovation.com.au>
@imi415 @pi-anl
mimxrt/boards: Re-generate MIMXRT1052 clock config files.
d2c620f 
These were regenerated by the NXP Config tool for v2.11.

The board_init update was needed to ensure CLOCK_SetMode() is run
at the appropriate time during startup.

Signed-off-by: Andrew Leech <andrew.leech@planetinnovation.com.au>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
port-mimxrt
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@andrewleech @robert-hh @imi415