!boards: Simplify NuttX initialization#18408
!boards: Simplify NuttX initialization#18408linguini1 wants to merge 18 commits intoapache:masterfrom
Conversation
|
Notes for reviewers on the initial draft:
|
|
@linguini1 this is a great initiative. |
Hi @fdcavalcanti, if you're talking about making the changes, I think all ESP boards should be included in this patch already (xtensa/risc-v). They were actually quite easy since the board_late_init and board_app_init logic were pretty much identical. If you're talking about how to test, the process would be to configure the build system for one of the modified ESP32 configurations (i.e. nsh) and just boot into NuttX, check that things look okay and run Hope I understood correctly, thanks for your help! |
f30db82 to
31a4ad2
Compare
|
CI is passing! Hooray! The steps now are:
@fdcavalcanti this should be ready for ESP32 testing now if you're still able to help test it! |
BREAKING: In an effort to simplify NuttX initialization, NSH_ARCHINIT is removed. board_app_initialize is also removed. BOARD_LATE_INITIALIZE now performs all board initialization logic, and is by default enabled. All references to these symbols are removed. BOARDIOC_INIT remains, but will result in -ENOTTY when called. It is to be removed in a later commit. Quick fix: Boards relying on NSH_ARCHINIT should now enable CONFIG_BOARD_LATE_INITIALIZE instead. If the application needs fine-grained control over board initialization from userspace, the logic performed by BOARDIOC_INIT may be copied to the board_finalinitialize function and used instead via BOARDIOC_FINALINIT. All board_app_initialize logic provided by NuttX is now moved to board_late_initialize, and the same should be done for out-of-tree boards. Signed-off-by: Matteo Golin <matteo.golin@gmail.com>
Logic provided by board_app_initialize is replaced by board_late_initialize. Signed-off-by: Matteo Golin <matteo.golin@gmail.com>
Logic provided by board_app_initialize is removed due to the removal of BOARDIOC_INIT boardctl command. Logic inside board_late_initialize is to be used and is identical. Signed-off-by: Matteo Golin <matteo.golin@gmail.com>
Replaced board_app_initialize logic with board_late_initialize. Signed-off-by: Matteo Golin <matteo.golin@gmail.com>
Replaced board_app_initialize logic with board_late_initialize. Signed-off-by: Matteo Golin <matteo.golin@gmail.com>
Replaced board_app_initialize logic with board_late_initialize. Signed-off-by: Matteo Golin <matteo.golin@gmail.com>
Replaced board_app_initialize logic with board_late_initialize. Signed-off-by: Matteo Golin <matteo.golin@gmail.com>
Replaced board_app_initialize logic with board_late_initialize. Signed-off-by: Matteo Golin <matteo.golin@gmail.com>
Replaced board_app_initialize logic with board_late_initialize. Signed-off-by: Matteo Golin <matteo.golin@gmail.com>
Replaced board_app_initialize logic with board_late_initialize. Signed-off-by: Matteo Golin <matteo.golin@gmail.com>
Replaced board_app_initialize logic with board_late_initialize. Signed-off-by: Matteo Golin <matteo.golin@gmail.com>
Replaced board_app_initialize logic with board_late_initialize. Signed-off-by: Matteo Golin <matteo.golin@gmail.com>
Replaced board_app_initialize logic with board_late_initialize. Signed-off-by: Matteo Golin <matteo.golin@gmail.com>
Replaced board_app_initialize logic with board_late_initialize. Signed-off-by: Matteo Golin <matteo.golin@gmail.com>
Replaced board_app_initialize logic with board_late_initialize. Signed-off-by: Matteo Golin <matteo.golin@gmail.com>
Replaced board_app_initialize logic with board_late_initialize. Signed-off-by: Matteo Golin <matteo.golin@gmail.com>
Replaced board_app_initialize logic with board_late_initialize. Signed-off-by: Matteo Golin <matteo.golin@gmail.com>
Replaced board_app_initialize logic with board_late_initialize. Signed-off-by: Matteo Golin <matteo.golin@gmail.com>
|
This is now ready for review |
|
@linguini1 We are ready for branch out |
acassis
left a comment
There was a problem hiding this comment.
Don't merge before the 12.13 be created. I'm opening this change request just to avoid mistakes here.
Alin (same as nuttx-apps), let's follow with your suggested plan: Not big breaking changed to 12.13. |
12.13 branch created |
|
Hi @jerpelea , let's save this one for the 13.x branch on June 1 instead. |
|
@linguini1 great job! I noticed issues so far only on ESP32, but will keep testing: On
The thing in common on those configs is that they all use SMP. Seems the fix is to increase +1 to CONFIG_MM_REGIONS on the configs that have SMP, but I fail to see the reason right now. |
|
Thank you @fdcavalcanti for testing! Do you know if any of those issues were present prior to this patch? I do not have an esp32-devkitc board but I do have the Xiao C3, so I will try to reproduce there as well and see if I can figure out what it may be. |
I did a quick test on master and it is working fine for |
Summary
BREAKING CHANGE
This change simplifies the NuttX initialization logic by:
board_app_initialize did so any defconfigs relying on NSH_ARCHINIT will not
break.
This is related to #11321.
Twin PR in NuttX apps should be merged at the same time: apache/nuttx-apps#3405
Impact
Almost every single board/configuration in the NuttX source tree, since many
relied on NSH for initialization.
This is a breaking change that removes the user's ability to use BOARDIOC_INIT
as well. Users are provided with quick-fixes in the commit messages for how to
fix any breakages. BOARDIOC_FINALINIT should be used instead for applications
that truly require full control over the initialization process. For every other
application, BOARD_LATE_INITIALIZE should be enabled to have the NuttX kernel
perform initialization before launching into the app.
Testing
I will be testing on the platforms available to me (simulators and whatever
hardware I own). Testing from community members with hardware across the
architectures affected would be greatly appreciated. If you do want to help with
testing, please provide logs in the PR comments for the affected defconfigs you
tested.
Testing of applications can be seen in the twin PR: apache/nuttx-apps#3405
OS Test logs
sim:nsh: sim-nsh.txtqemu-armv8a:nsh: qemu-armv8a-nsh.txtqemu-armv7a:nsh: qemu-armv7a-nsh.txtrv-virt:nsh: rv-virt-nsh.txtraspberrypi-4b:sd: rpi4b-sd.txt (verify SD filesystem is mounted + ostest)samv71-xult:nsh,samv71-xult:netnsh: See comment