Skip to content

Conversation

@tomchy
Copy link
Contributor

@tomchy tomchy commented Dec 8, 2025

The default overlay for FW loader sets code-partition to slot1_partition, which stays with direct conflict with the merged slot approach:

  • slot1_partition must point to the merged partition to allow SMP logic to parse the FW loader magic & version
  • if FW loader indeed uses slot1_partition with ROM_START_OFFSET, than it is impossible to correctly boot the radio core FW, since it cannot use the same value of the ROM_START_OFFSET (2kB NVM waste) and there is no way to tell the FW loader code in soc.c to boot the radio with a different value of ROM_START_OFFSET.
  • the size of the code-partition must be different from the size of the partition parsed by the SMP, so there is a protection against potential code partition overflow and the flash area API can reach past the code partition area.

One solution would be to change the contents of the FIRMWARE_LOADER_image_default.overlay, but since it would force every user of the FW loader within Zephyr to add a new partition alias, it is far less intrusive to remove this overlay from sysbuild.cmake script in those samples, that use merged slot approach.

Ref: NCSDK-36779

@tomchy tomchy requested a review from a team as a code owner December 8, 2025 16:31
Copilot AI review requested due to automatic review settings December 8, 2025 16:31
@tomchy tomchy requested a review from a team as a code owner December 8, 2025 16:31
@NordicBuilder NordicBuilder added the changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. label Dec 8, 2025
The default overlay for FW loader sets code-partition to
slot1_partition, which stays with direct conflict with the merged slot
approach:
 - slot1_partition must point to the merged partition to allow SMP logic
   to parse the FW loader magic & version
 - if FW loader indeed uses slot1_partition with ROM_START_OFFSET, than
   it is impossible to correctly boot the radio core FW, since it cannot
   use the same value of the ROM_START_OFFSET (2kB NVM waste) and there
   is no way to tell the FW loader code in soc.c to boot the radio with
   a different value of ROM_START_OFFSET.
 - the size of the code-partition must be different from the size of the
   partition parsed by the SMP, so there is a protection against
   potential code partition overflow and the flash area API can reach
   past the code partition area.

One solution would be to change the contents of the
FIRMWARE_LOADER_image_default.overlay, but since it would force every
user of the FW loader within Zephyr to add a new partition alias, it is
far less intrusive to remove this overlay from sysbuild.cmake script in
those samples, that use merged slot approach.

Ref: NCSDK-36779

Signed-off-by: Tomasz Chyrowicz <[email protected]>
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR addresses conflicts between the default firmware loader overlay and the merged slot approach used in the single_slot DFU sample. The main issue is that the default overlay sets code-partition to slot1_partition, which conflicts with the merged slot approach where slot1_partition must point to the merged partition for SMP logic to work correctly.

Key Changes:

  • Removes the default overlay for firmware loader images when using merged binary signing
  • Updates overlay files to directly set zephyr,code-partition instead of manipulating slot1_partition

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
samples/dfu/single_slot/sysbuild.cmake Adds logic to remove default overlay when SB_CONFIG_MCUBOOT_SIGN_MERGED_BINARY is enabled
samples/dfu/single_slot/sysbuild/fw_loader_ipc_radio.overlay Replaces node deletion/redefinition with direct zephyr,code-partition assignment
samples/dfu/single_slot/sysbuild/ble_mcumgr/boards/nrf54h20dk_nrf54h20_cpuapp.overlay Replaces node deletion/redefinition with direct zephyr,code-partition assignment

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

FirmwareUpdaterImage_Get(fw_loader_images)
foreach(image IN LISTS fw_loader_images)
# Remove the default overlay file.
set(${image}_EXTRA_DTC_OVERLAY_FILE "" CACHE INTERNAL "Application extra DTC overlay file"
Copy link

Copilot AI Dec 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment 'Application extra DTC overlay file' should be 'Image extra DTC overlay file' since this is setting the overlay for firmware loader images, not the application.

Suggested change
set(${image}_EXTRA_DTC_OVERLAY_FILE "" CACHE INTERNAL "Application extra DTC overlay file"
set(${image}_EXTRA_DTC_OVERLAY_FILE "" CACHE INTERNAL "Image extra DTC overlay file"

Copilot uses AI. Check for mistakes.
@tomchy tomchy force-pushed the bugfix/single_slot/NCSDK-NONE_Fix_fw_loader_partitions branch from d17065a to 5ddb364 Compare December 8, 2025 16:32
@NordicBuilder
Copy link
Contributor

NordicBuilder commented Dec 8, 2025

CI Information

To view the history of this post, click the 'edited' button above
Build number: 2

Inputs:

Sources:

sdk-nrf: PR head: 5ddb3640f5addef216034ef67e4fb876429a5edd

more details

sdk-nrf:

PR head: 5ddb3640f5addef216034ef67e4fb876429a5edd
merge base: 059095979f2ce8c57fa9213f2e63126475ca3857
target head (main): 059095979f2ce8c57fa9213f2e63126475ca3857
Diff

Github labels

Enabled Name Description
ci-disabled Disable the ci execution
ci-all-test Run all of ci, no test spec filtering will be done
ci-force-downstream Force execution of downstream even if twister fails
ci-run-twister Force run twister
ci-run-zephyr-twister Force run zephyr twister
List of changed files detected by CI (3)
samples
│  ├── dfu
│  │  ├── single_slot
│  │  │  ├── sysbuild.cmake
│  │  │  ├── sysbuild
│  │  │  │  ├── ble_mcumgr
│  │  │  │  │  ├── boards
│  │  │  │  │  │  │ nrf54h20dk_nrf54h20_cpuapp.overlay
│  │  │  │  │ fw_loader_ipc_radio.overlay

Outputs:

Toolchain

Version: 43683a87ea
Build docker image: docker-dtr.nordicsemi.no/sw-production/ncs-build:43683a87ea_5ea73affbf

Test Spec & Results: ✅ Success; ❌ Failure; 🟠 Queued; 🟡 Progress; ◻️ Skipped; ⚠️ Quarantine

  • ◻️ Toolchain - Skipped: existing toolchain is used
  • ✅ Build twister
    • sdk-nrf test count: 2
  • ✅ Integration tests
    • ✅ test-sdk-dfu
Disabled integration tests
    • test-fw-nrfconnect-nrf_lrcs_mosh
    • test-fw-nrfconnect-nrf_lrcs_positioning
    • desktop52_verification
    • test_ble_nrf_config
    • test-fw-nrfconnect-apps
    • test-fw-nrfconnect-ble_mesh
    • test-fw-nrfconnect-ble_samples
    • test-fw-nrfconnect-chip
    • test-fw-nrfconnect-fem
    • test-fw-nrfconnect-nfc
    • test-fw-nrfconnect-nrf-iot_cloud
    • test-fw-nrfconnect-nrf-iot_libmodem-nrf
    • test-fw-nrfconnect-nrf-iot_lwm2m
    • test-fw-nrfconnect-nrf-iot_samples
    • test-fw-nrfconnect-nrf-iot_thingy91
    • test-fw-nrfconnect-nrf-iot_zephyr_lwm2m
    • test-fw-nrfconnect-nrf_crypto
    • test-fw-nrfconnect-ps-main
    • test-fw-nrfconnect-rpc
    • test-fw-nrfconnect-rs
    • test-fw-nrfconnect-tfm
    • test-fw-nrfconnect-thread-main
    • test-low-level
    • test-sdk-audio
    • test-sdk-find-my
    • test-sdk-mcuboot
    • test-sdk-wifi
    • test-secdom-samples-public

Note: This message is automatically posted and updated by the CI

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants