-
Notifications
You must be signed in to change notification settings - Fork 7.9k
modules: mcuboot: enable support for RAMLOAD mode with revert #85254
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
Changes from all commits
5ce884e
a62c1e7
c9f27d1
cc38cf7
28790dc
29d4747
157485b
8b90114
8c46a99
c4f2cb7
16348e2
bb8c665
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
CONFIG_USE_DT_CODE_PARTITION=y |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,53 @@ | ||
/* | ||
* Copyright (c) 2025 Tenstorrent AI ULC | ||
* | ||
* SPDX-License-Identifier: Apache-2.0 | ||
*/ | ||
|
||
/* | ||
* Layout must match the hyperflash_ram_load overlay file within mcuboot | ||
* application configuration directory | ||
*/ | ||
|
||
/delete-node/ &sdram0; | ||
|
||
/ { | ||
sram@80007F00 { | ||
compatible = "zephyr,memory-region", "mmio-sram"; | ||
reg = <0x80007F00 0x100>; | ||
zephyr,memory-region = "RetainedMem"; | ||
status = "okay"; | ||
|
||
retainedmem { | ||
compatible = "zephyr,retained-ram"; | ||
status = "okay"; | ||
#address-cells = <1>; | ||
#size-cells = <1>; | ||
|
||
boot_info0: boot_info@0 { | ||
compatible = "zephyr,retention"; | ||
status = "okay"; | ||
reg = <0x0 0x100>; | ||
}; | ||
}; | ||
}; | ||
|
||
chosen { | ||
zephyr,bootloader-info = &boot_info0; | ||
zephyr,sram = &sdram_split; | ||
}; | ||
|
||
/* | ||
* Adjust sdram0 to reserve first 30KB for MCUBoot, and | ||
* remaining 2KB for retained memory | ||
*/ | ||
sdram_split: sdram_split@80008000 { | ||
reg = <0x80008000 (0x2000000 - DT_SIZE_K(32))>; | ||
}; | ||
|
||
}; | ||
|
||
/* Reduce size of slot 0 to match slot 1 */ | ||
&slot0_partition { | ||
reg = <0x40000 0x300000>; | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,9 +1,5 @@ | ||
# SPDX-License-Identifier: Apache-2.0 | ||
|
||
if("${FILE_SUFFIX}" STREQUAL "ram_load") | ||
set(mcuboot_EXTRA_DTC_OVERLAY_FILE "${CMAKE_CURRENT_LIST_DIR}/nrf52840dk_nrf52840_mcuboot_ram_load.overlay" CACHE INTERNAL "" FORCE) | ||
endif() | ||
|
||
find_package(Sysbuild REQUIRED HINTS $ENV{ZEPHYR_BASE}) | ||
|
||
project(sysbuild LANGUAGES) |
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -39,6 +39,11 @@ if(SB_CONFIG_BOOTLOADER_MCUBOOT) | |
set_config_bool(${ZCMAKE_APPLICATION} CONFIG_MCUBOOT_BOOTLOADER_MODE_RAM_LOAD y) | ||
set_config_bool(${ZCMAKE_APPLICATION} CONFIG_XIP n) | ||
set_config_int(${ZCMAKE_APPLICATION} CONFIG_FLASH_SIZE 0) | ||
elseif(SB_CONFIG_MCUBOOT_MODE_RAM_LOAD_WITH_REVERT) | ||
# RAM load mode requires XIP be disabled and flash size be set to 0 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unrelated to the core of the PR, do you know why RAM_LOAD mode requires XIP=n, but SINGLE_APP_RAM_LOAD does not? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't, no- not familiar with There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. An oversight probably There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. XIP is orthogonal to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Can you perhaps clarify this in the Kconfig help text, I think it's a very important point that will help many users. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
that is not consistent with what the Kconfig does:
so XIP is execute from flash, non-XIP is execute from RAM |
||
set_config_bool(${ZCMAKE_APPLICATION} CONFIG_MCUBOOT_BOOTLOADER_MODE_RAM_LOAD_WITH_REVERT y) | ||
set_config_bool(${ZCMAKE_APPLICATION} CONFIG_XIP n) | ||
set_config_int(${ZCMAKE_APPLICATION} CONFIG_FLASH_SIZE 0) | ||
elseif(SB_CONFIG_MCUBOOT_MODE_SINGLE_APP_RAM_LOAD) | ||
set_config_bool(${ZCMAKE_APPLICATION} CONFIG_MCUBOOT_BOOTLOADER_MODE_SINGLE_APP_RAM_LOAD y) | ||
elseif(SB_CONFIG_MCUBOOT_MODE_FIRMWARE_UPDATER) | ||
|
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.
fallback needed if write-block-size is not set. the generic spi-nor flash driver does not have this prop and also not allow this prop to be added. If it is not set imgtool will complain, that align is without argument
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'd argue we need to allow the generic spi-nor-flash driver to add that property in this case- typically this is done by including the
soc-nv-flash
compatible, but you can also define the property directlyThere 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.
added it here: #95152
Uh oh!
There was an error while loading. Please reload this page.
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.
@danieldegrasse This change indeed broke the RAMLOAD build on our side.
I see #95152 is supposed to fix it, but it seems blocked for the time being. Would it be possible to revert this specific change (if its absence doesn't break the whole goal of this PR) waiting to have a consensus on a way to avoid breaking existing users ?
Uh oh!
There was an error while loading. Please reload this page.
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.
would suggest putting this align part in an if() for the revert part, the normal RAM load part should never need a value here that is not 1