-
Notifications
You must be signed in to change notification settings - Fork 1.3k
samples: mcuboot: Add firmware loader entrance sample #23318
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
Conversation
CI InformationTo view the history of this post, clich the 'edited' button above Inputs:Sources:sdk-nrf: PR head: 832a7e42065b24e20ae3e3631e8e3e26773fdb93 more detailssdk-nrf:
zephyr:
Github labels
List of changed files detected by CI (16)
Outputs:ToolchainVersion: 684b32e022 Test Spec & Results: ✅ Success; ❌ Failure; 🟠 Queued; 🟡 Progress; ◻️ Skipped;
|
You can find the documentation preview for this PR here. |
The following west manifest projects have changed revision in this Pull Request:
✅ All manifest checks OK Note: This message is automatically posted and updated by the Manifest GitHub Action. |
1560707
to
c367787
Compare
Memory footprint analysis revealed the following potential issuesapplications.hpf.gpio.icbmsg[nrf54l15dk/nrf54l15/cpuflpr]: High RAM usage: 12430[B] - link (cc: @nrfconnect/ncs-ll-ursus) Note: This message is automatically posted and updated by the CI (latest/sdk-nrf/PR-23318/8) |
Adds support for using SHA512 hashes when signing firmware loader images Signed-off-by: Jamie McCrae <[email protected]>
Includes a fix for missing aliases for nrf54l15dk Signed-off-by: Jamie McCrae <[email protected]>
Fixes an issue with this shield Signed-off-by: Jamie McCrae <[email protected]>
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.
I did not review over the code itself.
Instead, I used what we can call "the lazy user test", where I just run the example soely based on my expectations, to see if it works out of the box.
This worked well, so I approve this PR.
Still, I added two comments for things I think can be improved.
|
||
k_work_init(&adv_work, adv_work_handler); | ||
advertising_start(); | ||
|
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.
I suggest that main prints out something along the lines of "Reset the device while holding Button 0/1 to enter DFU mode", so users understand how to enter firmware loader mode.
size: 0x60000 | ||
span: *id002 | ||
mcuboot_secondary: | ||
address: 0x6a800 |
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.
I might misunderstand the point of the Firmware Loader, but:
One of the main advantages of having two different images is that we can save space.
If this is the case, the example should showcase a smaller secondary slot than primary slot.
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.
actually it's a very good point. Actually there is only primary and firmware loader partition used as bootable images.
samples/mcuboot/firmware_loader_entrance/pm_static_nrf54l15dk_nrf54l15_cpuapp.yml
Show resolved
Hide resolved
Adds support for picking this image as a firmware loader image Signed-off-by: Jamie McCrae <[email protected]>
Adds a sample showing how to reboot into MCUboot's firmware loader application for being able to load firmware updates Signed-off-by: Jamie McCrae <[email protected]>
Assigns this to pluto Signed-off-by: Jamie McCrae <[email protected]>
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.
Looks good, but README is certainly missing :)
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.
I think Sigurd have good point on pm_static* which should show that primary partition is "big one". Also I'm not sure, but maybe secondary partition can be removed.
Already clarified: README is planed to be added in the subsequent patch.
NCSDK-28480