-
-
Notifications
You must be signed in to change notification settings - Fork 8.2k
ports: mimxrt: Switch to official GitHub SDK (WIP) #11516
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
base: master
Are you sure you want to change the base?
Conversation
Code size report:
|
Thanks for the attempt. I had considered switching to the mcux_sdk a while ago, but refused so, since the mcux_sdk is huge (>807MB), containing many boards that are out of scope. And tests with the boards were not promising. They compiled, but did not work well. So switching requires each and every IO device to be tested again. I do not know how much testing of the peripherals you did with the 1020 EVK board. |
Thanks @robert-hh , and I also agree that doing extensive tests with every boards is going to be slow and painful, and that's why this PR is still a WIP draft - and will still be a draft until all implemented drivers in machine modules are tested with every supported boards (there aren't much, are they?). Since there's still not any automated and reproduceable method of doing so, I'll post my manual or (semi-)auto test result matrix and method/code (maybe in another repository) if anyone is interested. |
At the moment we have: MIMXRT1010_EVK The MCU's are i.mx rt1011, 1015, 1021, 1052, 1062, 1064 and 1176, as far as I can tell. So yes, it's way less then the ~170 devices supported by the mcux_sdk. |
I once started with a kind of test shield for the EVK boards with some wiring and external components on it for some amount of automated testing, but abandoned that. It did not help much. I can support in testing, especially for the non-NXP boards. |
I have been using 2.13.1 for a while with 60/62/64 with no problems, it build with MicroPython with one change just need to fix the macro names in |
I think this is a great idea worth trying, an Arduino shield with testing peripherals
Thanks, and that would be great help. |
I had issues when trying to move from 2.10 to 2.11. All compiled well, but there were issues with the clock configuration. That was before support for I2S was added. |
@robert-hh Speaking of clock config, those a very board-specific files ( |
That's what it was initially, but when more boards for a specific MCU were added, Damien asked me to move these files to the boards directory. Nowadays, I would consider a mcu/xxyy directory structure. Besides that, a board specific clock_config.c file could also be introduced using Makefile/mpconfigboard.mk variations. |
I started a few build & run tests. Nothing special. MIXRT1176 and Olimex worked fine. Seeed Arch Mix requires the same change for mpconfigboard.h like you made for the MIMXRT1050_EVKB. I have attached the updated file. It's better if changes are just made at one place. For the MIMXRT1176 and the SEEED Arch Mix board I just tested, whether they compile & boot, show REPL, and whether the LAN interface works. It does. The max speed I got with the MIMXRT1170_EVKB was ~300 MBit/s at the 1G interface, streaming data via TCP from a Python script into netcat running on my 10 year old PC. |
Thanks @robert-hh , I have just completed a prototype based on your idea about making a shield which could make the tests automated, the tricky parts are standard peripherals, like GPIO, UART, I2C, SPI, PWM etc., while the USB CDC, SDCard and Ethernet can be easily verified on the DUT itself. The boards are currently out for manufacturing, and I'll update the files in another repo after the operations and firmware of that shield are tested. |
Wow. That's fast. For UART I had just connections between RX and TX for a local loopback. Fir I2C I had added an external I2C DAC, which also supplied a varying analog voltage for the ADC. But I2C can also be tested with the I2S and the WM8960, or the Ethernet PHY, and for ADC you can also just use a resistor ladder. SPI is more tricky. I had implemented peripheral mode once, but it was rejected because first the API had to be defined (if ever). But for that an external device could be used as well. Alternatively, a small micro can be added to the test shield, which itself does not require many external components and which can act as a counterpart. Obviously that makes the set-up more complicated than just passive components or simple external devices. |
That's exactly what I actually did here. The firmware might be a tricky part, but we'll get there after the hardware is built. |
That looks excellent. Yes, the software might get some work. But it can grow step-by-step. It could have a kind of command interface e.g. connected to the UART at D0/D1, making it possible to control the operation. |
Would it make more sense to just mirror the used devices in a new repo, and just update them every now and then, something like: https://github.com/micropython/stm32lib Would also allow for the possibility of patching the SDK (if that's ever needed). |
Of course. I did similar before, until Hatach kindly updated his copy of the repository. But that would require to put that subset at a suitable URL and have it administered. For STM32 it is the MicroPyhton repository. But I have doubts that Damien is interested to do the administration. Obviously someone else could have a look at the versions and make PRs whenever needed. |
@imi415 I made short tests for All tests worked fine. I have to test further. The performance tests seemed to run a little bit slower. |
Testing PWM: |
About MIMXRT1052 PWM. The line: |
@imi415 Did you make progress with your test shield? I could not open the Kicad files because the most recent KiCad I can run is v6.x. Seems like I need a new PC, or have to switch to Ubuntu. |
Hi Robert, I'm very grateful for the testing effort and sorry for the late response. Yes, the project does requires KiCAD 7 to open. I have received the boards a week ago, the hardware thankfully works despite a few minor issues (saved by the SWM). The firmware communicates with host using I2C and works like a I2C to GPIO expander but with more functionality. I have finished the I2C, GPIO, ADC/DAC part and still left SPI, UART, timer capture for PWM to go. The firmware code is public however it is currently on a self-hosted instance out of GitHub, I'll put the repo on GitHub after finishing a working prototype. |
|
||
ENET_Init(ENET, &g_handle, &enet_config, &buffConfig[0], hw_addr, source_clock); | ||
ENET_SetCallback(&g_handle, eth_irq_handler, (void *)self); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed somewhere between v2.13 and v2.15, callback is in config struct.
@@ -157,7 +157,9 @@ void machine_rtc_start(void) { | |||
// Do a basic init. | |||
SNVS_LP_Init(SNVS); | |||
// Disable all external Tamper | |||
#if defined(FSL_FEATURE_SNVS_HAS_MULTIPLE_TAMPER) && (FSL_FEATURE_SNVS_HAS_MULTIPLE_TAMPER > 0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some parts (RT1021) does not have SNVS tamper.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #11516 +/- ##
=======================================
Coverage 98.39% 98.39%
=======================================
Files 161 161
Lines 21200 21200
=======================================
Hits 20860 20860
Misses 340 340 ☔ View full report in Codecov by Sentry. |
Would shallow submodules help? At least they're possible (maybe we should use them for all submodules?): https://git-scm.com/docs/git-submodule#Documentation/git-submodule.txt---no-recommend-shallow |
It is in the actual master branch and in the Makefile for MIMXRT. hal/fsl_lpuart.c was added later in #14041
Sure. It can be made as part of the move to more recent versions of the library. Only it has to be done. And since that part of the code is not broken, there is no urge. |
I do not know. If we switch, it may be best to first just change the submodule source path and make sure all works fine with v2.10. And then progress the versions. At other submodules we often do not use the most recent commit. |
Hi @robert-hh, Thanks & BR, |
@imi415 Thank you for the note. If that results in a smaller repository for the MIMXRT devices it will help. |
I'm coming to this as a relatively new mimxrt user. At work we're evaluating mimxrt1176 for use with LVGL as a potential standardised platform for ongoing use. This started my efforts in hathach/nxp_driver#9 to automate the updating of the repo currently used here for NXP drivers but my preference really would be to use the official NXP repo. Using their repo means one less thing that needs maintenance to pull in upstream improvements by NXP. Keeping up to date with the official nxp repo would help us get support from NXP if needed; they're getting involved directly in other open source communities like zephyr, it'd be great to encourage their input here also. The large repo size may be an issue in slowing CI down here having to pull a large amount each time, though as suggested shallow clone might help a lot here. Large checkouts are painful for some users I know, though for most people you'll only need to do this once for their checkout and then never think of it again. Not everyone can afford large disks and download quotas certainly but toolchain downloads measured in GB are quite common these days, avoiding that comes at increased maintenance costs. Is there a different downside to large submodules I'm overlooking? In terms of upstream version support, I know NXP are still doing something quirky with regards to releases from their mcux package being different somehow to the GitHub master, the actual release packs on the GitHub page come from isolated commits outside of the main git tree. I think the differences between the two are being reduced though, to the point I think it might be easier to migrate to newer upstream version than old. For that it's worth, before I learned of this PR I had a test build for 1176 compiled against mcux 2.16 from GitHub with only a couple of hashdef changes to micropython, plus replacing hal/fsl_lpuart.c with the official one. I haven't had a chance to test the build on hardware though so I'm sure there's things I'm missing relating to the changes made here and mentioned above. |
As said, changing is possible. This PR is still in the draft mode, so it cannot be merged. But it will need anyhow a different approach to start, even with the huge submodule. It should first just switch the submodule, but not the version. and then progress, Changing library versions require a lot of testing.
As far as I could tell when isolating hal/fsl_lpuart.c the only change to the library version was the handling of the line_idle signal. When you use the lib version of fsl_lpuart.c, the event IRQ_RXIDLE will happen way more often that it should. It works more like IRQ_RX. Maybe the most recent version was fixed. |
Ok, as a first trial I've raised #16163 to demonstrate reducing the size of submodules (history) and will check how much impact that has on the nxp repo size. Ok, performing a filtered clone locally doesn't really make much difference:
|
Thank, @andrewleech . In any case, I will start testing again with the mcux-sdk once the struggle with PWM is settled. PWM is surprisingly complicated. |
@imi415 & @robert-hh I've got a slightly updated copy of this PR pushed to #16235 ( still with your as the commit author @imi415 ) I had a chance to talk to Damien briefly and his preference was slightly towards keeping a cut down sdk repo, likely eventually moved into the micropython github project space. This pattern is used for stm and works pretty well really, particularly as there's such a dramatic difference in size. I've improved my nxp_sdk repo update script a bit and used it to mirror the official sdk @ 2.11 for the included targets, so that MR can be used to test compatibility with the existing drivers & boards. It's also been rebased to current micropython master I believe I've preserved the fsl_uart changes you've made recently for irq handling @robert-hh I need a newer version of the sdk for use with lvgl so will update that as a separate commit in a new PR to test separately. |
Thanks. I will test it in the next days, hopefully today. |
I made a few tests. It compiles for all MCUs. I tested with boards for all MCUs except i.MX RT 1064, lacking a board. Boards used for the test: Adafruit Metro M7 (1011), MIMXRT1015_EVK, MIMXRT1020_EVK, MIMXRT1050_EVKB (1052, Hyoerflash), Seeed ArchMix(1052, SPI flash), Teensy 4.1 (1062), and MIMXRT1170_EVK. |
There is more trouble with the Seeed Arch Mix support. When the new firmware is loaded, I cannot connect to the device any more using JLink as it used to work. Only if I attempt to connect immediately after releasing the Reset button JLink connects. Less severe issues with MIMXRT1052_EVK. It connects but shows an error which did not exist before: |
How strange! That suggests the swd port / debug core is being disabled. Perhaps the charge in cmsis library being used? Or was the pinouts on those boards changed in the update? |
Was the clock config changed for that processor / board perhaps? |
Yes. That's the next I wanted to check. At first glance, looking through all changes, it looked ,mostly like different formatting. But there were also changes in the ENET section. I do not know why this was changed only for MIMXRT1052. I will revert that changes & try. |
Rolling back the clock changes made it work again. So I have to check if any of these changes were needed. Found one so far. |
Maybe those changes were simply copied from an offer SDK builder export that didn't have so the peripherals enabled? That being said we may not want to rush into that too much, a person from NXP told me that more board support libraries were going to be pushed to their GitHub soon to reduce reliance on the web based builder. I was asking about the LVGL integration libraries but hopefully that would extend to there other things like board/clock/pins etc. Then at least the board profile here could include a link back to the upstream repo (if not include this directly from a submodule) |
Maybe. So for the actual PR you should revert the changes to
|
@imi415 do you remember where your changes to the files in the board profiles came from? |
Board file changes came from "Config Tool" by importing source files and regenerating them. These files comes with YAML comments can be imported back to CT and they needs to be updated if SDK changed significantly. I suggest using Config Tool to import the source files, make modifications graphically, then copy the generated sources files back to keep YAML comments in sync with the code for future modifications. |
@imi415 That explains the way the file is generated, but not,
Did you create a configuration for a different board? |
Since the configuration is imported from the original MPY's source file, I can only assume there's a mismatch between the existing code and YAML settings. (file is modified manually after it was generated).
I was only able to test this setup on RT1052, and this is partially why this is still WIP.
No, there are no "board" settings for the tool when imported from existing source file. |
The clock config file for the MicroPython RT1052 (Clocks V5.0) is identical to the file for RT1052 library v2.10. But clock_config.c for v2.10 is different to v2.11. Difference are in the setting of Edit: Looking into the data sheet, the maximum for ARM_TRACE_CLK is 70MHz (chapter 4.4.4, Table 41). Given that, both configurations are wrong, only 117Mhz is more close to 70Mhz. Setting the divider to 5 results in ~70Mhz, which works and allows JTAG to connect. Edit2: On further reading, ARM_TRACE_CLK is TRACE_CLK_ROOT / 2, so 132MHz should be withing range as well...... |
Finally, it was only the missing call to |
On a tangential note, @robert-hh I don't suppose you've previously worked with the DAC on these mimxrt parts by any chance? |
I looked at a DAC functionality of the MIMXRT10xx devices, but I only found it as part of the ACMP unit, which has an internal 6 bit DAC. I did not see any way of getting the output signal of that DAC to a Pin, not even using XBAR. Do you know more? Edit: Scanning through the data sheets again, DAC seems available at the 117x devices, ACMP is available at MIMXRT102x, -1052, -106x. |
Ah, yes 1176 is the main cpu I'm working with right now for which i saw the dac driver in the SDK headers. I didn't realise it was the first series of the mimxrt family to include one. Thanks for the info about the ACMP, I haven't looked at that peripheral myself. It's quite strange they provisioned dac hardware just for that without making it available on an output. |
I integrated this in my updated PR as a separate commit, moving this line to |
This (WIP) PR will replace nxp_sdk submodule with official mcux_sdk released here
The current nxp_sdk was used for both mpy and TinyUSB project, given the fact that TinyUSB project has switched to mcux_sdk already (At least for Kinetis, LPC55, and i.MXRT devices), and nxp_sdk drivers are kept on an fairly old version with limited devices supported, the mcux_sdk might be a better choice for existing and future devices.
The PR also updated the (Config Tool Generated) clock_config presets and pinmux table for RT1052 series, which was updated due to device macro names has changed in recent revisions.
The CMSIS core headers is nolonger shipped in official mcux_sdk, so mpy's CMSIS lib is used instead.
Currently tested platform:
Other EVKs only performed compilation tests, will be updated here after testing.