[nrf noup] boot/zephyr: nRF54h20 resume from S2RAM #489
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
nvlsianpu
changed the title
nvlsianpu
force-pushed
the
nrf54h20_pm_s2rm_redirect
branch
from
5c05797 to
4d6d20f
Compare
nvlsianpu
changed the title
|
@kl-cruz This is not building due to issues with hal and other headers. |
nvlsianpu
force-pushed
the
nrf54h20_pm_s2rm_redirect
branch
from
4d6d20f to
70b82fb
Compare
|
nvlsianpu
force-pushed
the
nrf54h20_pm_s2rm_redirect
branch
from
70b82fb to
342ddf4
Compare
Merged
2 tasks
nvlsianpu
force-pushed
the
nrf54h20_pm_s2rm_redirect
branch
2 times, most recently
from
ba6ada5 to
c488b52
Compare
nvlsianpu
changed the title
nvlsianpu
force-pushed
the
nrf54h20_pm_s2rm_redirect
branch
from
c488b52 to
37eb7e0
Compare
adsz-nordic
reviewed
Sep 10, 2025
nvlsianpu
force-pushed
the
nrf54h20_pm_s2rm_redirect
branch
from
37eb7e0 to
0a19e33
Compare
pizi-nordic
approved these changes
Sep 10, 2025
nvlsianpu
force-pushed
the
nrf54h20_pm_s2rm_redirect
branch
from
709482e to
36452d3
Compare
kapi-no
reviewed
Sep 11, 2025
de-nordic
reviewed
Sep 11, 2025
nvlsianpu
force-pushed
the
nrf54h20_pm_s2rm_redirect
branch
from
36452d3 to
e66f82a
Compare
de-nordic
approved these changes
Sep 11, 2025
nvlsianpu
force-pushed
the
nrf54h20_pm_s2rm_redirect
branch
2 times, most recently
from
0c18806 to
e11c558
Compare
nordicjm
requested changes
Sep 15, 2025
Comment on lines
+50
to
+53
| /* This could be read from slot's image_header.ih_hdr_size, but immediate value | ||
| * is much faster to reach | ||
| */ | ||
| #define APP_EXE_START_OFFSET 0x800 /* nRF54H20 */ |
There was a problem hiding this comment.
by how much? Why can't it just be read?
There was a problem hiding this comment.
by the time needed to access word which encodes heder size of 0x800 in MRAM. No much, but always a dozen CPU cycles for reaching data which are expected to be a constant value. Rationale in sentence above.
boot/zephyr/nrf54h20_custom_s2ram.c
Outdated
| uint32_t reset_reason = nrf_resetinfo_resetreas_local_get(NRF_RESETINFO); | ||
|
|
||
| if (reset_reason != NRF_RESETINFO_RESETREAS_LOCAL_UNRETAINED_MASK) { | ||
| /* normal boot */ |
There was a problem hiding this comment.
comments start with capital letter
boot/zephyr/nrf54h20_custom_s2ram.c
Outdated
|
|
||
| if (reset_reason != NRF_RESETINFO_RESETREAS_LOCAL_UNRETAINED_MASK) { | ||
| /* normal boot */ | ||
| return false; |
There was a problem hiding this comment.
tab indent, other parts are space indent
| /* s2ram boot */ | ||
| struct arm_vector_table *vt; | ||
| vt = (struct arm_vector_table *) | ||
| (FIXED_PARTITION_ADDR(slot0_partition) + APP_EXE_START_OFFSET); |
There was a problem hiding this comment.
(non blocking) this will not work with direct-xip
There was a problem hiding this comment.
Yes we know - direct-xip not supported.
| } | ||
|
|
||
| /* s2ram boot */ | ||
| struct arm_vector_table *vt; |
There was a problem hiding this comment.
doesn't this need volatile to avoid the LTO issue?
There was a problem hiding this comment.
no - content of pointed data is non-volatile during function execution.
boot/zephyr/nrf54h20_custom_s2ram.c
Outdated
| __attribute__((section(DT_PROP(DT_NODELABEL(mcuboot_s2ram), zephyr_memory_region)))) | ||
| volatile struct mcuboot_resume_s _mcuboot_resume; | ||
| #else | ||
| #error "mcuboot resume support section not defined in dts" |
Comment on lines
28
to
31
| #define FIXED_PARTITION_ADDR(node_label) \ | ||
| (DT_REG_ADDR(DT_NODELABEL(node_label)) + \ | ||
| COND_CODE_0(DT_FIXED_PARTITION_EXISTS(DT_NODELABEL(node_label)), (0), \ | ||
| (DT_REG_ADDR(DT_GPARENT(DT_NODELABEL(node_label)))))) |
There was a problem hiding this comment.
use DT_FIXED_PARTITION_ADDR
There was a problem hiding this comment.
doesn't work for mram
nordicjm
reviewed
Sep 15, 2025
boot/zephyr/nrf54h20_custom_s2ram.c
Outdated
| * `zephyr,memory-region` compatible DT node with nodelabel `mcuboot_s2ram`. | ||
| */ | ||
| __attribute__((section(DT_PROP(DT_NODELABEL(mcuboot_s2ram), zephyr_memory_region)))) | ||
| volatile struct mcuboot_resume_s _mcuboot_resume; |
There was a problem hiding this comment.
also do not start variable with underscore, that type of naming is compiler reserved
|
I'm going to apply review comments shortly |
nvlsianpu
force-pushed
the
nrf54h20_pm_s2rm_redirect
branch
2 times, most recently
from
fb3ee90 to
f295558
Compare
nvlsianpu
force-pushed
the
nrf54h20_pm_s2rm_redirect
branch
from
f295558 to
0e7cea3
Compare
nordicjm
reviewed
Sep 16, 2025
boot/zephyr/nrf54h20_custom_s2ram.c
Outdated
|
|
||
| int soc_s2ram_suspend(pm_s2ram_system_off_fn_t system_off) | ||
| { | ||
| (void)(system_off); |
There was a problem hiding this comment.
still has tabs intermixed with spaces
nordicjm
reviewed
Sep 16, 2025
boot/zephyr/nrf54h20_custom_s2ram.c
Outdated
|
|
||
| bool pm_s2ram_mark_check_and_clear(void) | ||
| { | ||
| uint32_t reset_reason = nrf_resetinfo_resetreas_local_get(NRF_RESETINFO); |
Application need special support in the bootloader in order to resume for suspend to RAM. MCUboot is immediate actor which redirects execution to the application (application reset vector) when wake-up from S2RAM is detected. Detection is based on HW (NRF_RESETINFO) and hardened using additional check over independent source of truth (variable with magic value). Thanks to above the application is resuming using its routines - instead of mocking that by routines compiled in by the MCUboot. Implementation is able to support only MCUboot modes with a swap. Direct-XIP is not handled as it require a way to run-time recognization of active application slot. Signed-off-by: Karol Lasończyk <[email protected]> Signed-off-by: Tomasz Chyrowicz <[email protected]> Signed-off-by: Andrzej Puzdrowski <[email protected]>
Added configuration which pre-configures MCUboot so It is able to support operation of resuming the App from S2RAM by the application itself. Signed-off-by: Andrzej Puzdrowski <[email protected]>
nvlsianpu
force-pushed
the
nrf54h20_pm_s2rm_redirect
branch
from
f4ef86e to
1cb1def
Compare
|
nordicjm
approved these changes
Sep 16, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Application need special support in the bootloader in order to resume the application for suspend to RAM.
MCUboot Is just immediate actor which redirects execution to the application (application reset vector) when wake-up from S2RAM is detected. Detection is based on HW (NRF_RESETINFO) and hardened using additional check over independent source of truth (variable with magic value).
Thanks to above the application is resuming using its routines - instead of mocking that by routines compiled in by the MCUboot.
ref: NCSDK-34750
manifest-pr-skip