Skip to content

Conversation

@carlescufi
Copy link
Contributor

Default to nrfutil on all boards and also add some changes made upstream in the meantime.

Default to nrfutil on all boards and also add some changes made upstream
in the meantime.

Signed-off-by: Carles Cufi <[email protected]>
Copilot AI review requested due to automatic review settings November 13, 2025 15:35
@carlescufi carlescufi requested a review from gmarull as a code owner November 13, 2025 15:35
@NordicBuilder NordicBuilder added the changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. label Nov 13, 2025
@carlescufi
Copy link
Contributor Author

@nordicjm could you review this one please?

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 pull request updates board generation templates to default to nrfutil and incorporates upstream changes. The main objective is to prioritize nrfutil as the default board runner across all nRF SoC families.

Key changes:

  • Reordered board runner includes to prioritize nrfutil over nrfjprog
  • Added TFM configuration blocks for nrf54l boards
  • Added nrfutil-specific external memory configuration arguments for nrf52 and nrf53 boards

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File Description
scripts/west_commands/create_board/templates/nrf91/board.cmake.jinja2 Adds nrfutil include before nrfjprog to prioritize nrfutil as default runner
scripts/west_commands/create_board/templates/nrf54l/board.cmake.jinja2 Adds TFM configuration blocks and includes nrfutil as the primary runner
scripts/west_commands/create_board/templates/nrf53/board.cmake.jinja2 Adds nrfutil external memory config and reorders includes to prioritize nrfutil
scripts/west_commands/create_board/templates/nrf52/board.cmake.jinja2 Adds nrfutil external memory config for QSPI and reorders includes to prioritize nrfutil

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

endif()

if(CONFIG_TFM_FLASH_MERGED_BINARY)
set_property(TARGET runners_yaml_props_target PROPERTY hex_file "${CMAKE_BINARY_DIR}/tfm_merged.hex")
Copy link

Copilot AI Nov 13, 2025

Choose a reason for hiding this comment

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

The hex_file path should use tfm_merged.hex instead of "${CMAKE_BINARY_DIR}/tfm_merged.hex" to be consistent with existing boards. All other boards in the repository (nrf54lv10dk, nrf7120pdk, tpm530mevk) use just tfm_merged.hex without the CMAKE_BINARY_DIR prefix.

Suggested change
set_property(TARGET runners_yaml_props_target PROPERTY hex_file "${CMAKE_BINARY_DIR}/tfm_merged.hex")
set_property(TARGET runners_yaml_props_target PROPERTY hex_file "tfm_merged.hex")

Copilot uses AI. Check for mistakes.
endif()

if(CONFIG_BOARD_{{ board | upper }}_NRF5340_CPUAPP OR CONFIG_BOARD_{{ board | upper }}_NRF5340_CPUAPP_NS)
board_runner_args(nrfutil "--ext-mem-config-file=${BOARD_DIR}/support/nrf5340dk_qspi_nrfutil_config.json")
Copy link

Copilot AI Nov 13, 2025

Choose a reason for hiding this comment

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

The config file path contains a hardcoded board name "nrf5340dk" which will be incorrect for boards generated from this template. This should either use a template variable like {{ board }} or be removed if there's no generic way to handle this configuration.

Suggested change
board_runner_args(nrfutil "--ext-mem-config-file=${BOARD_DIR}/support/nrf5340dk_qspi_nrfutil_config.json")
board_runner_args(nrfutil "--ext-mem-config-file=${BOARD_DIR}/support/{{ board }}_qspi_nrfutil_config.json")

Copilot uses AI. Check for mistakes.
@@ -1,10 +1,13 @@
board_runner_args(jlink "--device={{ soc | replace("nrf", "nRF") }}_xx{{ variant[2:] | upper }}" "--speed=4000")
if(CONFIG_SOC_NRF52840_QIAA)
board_runner_args(nrfutil "--ext-mem-config-file=${BOARD_DIR}/support/nrf52840dk_qspi_nrfutil_config.json")
Copy link

Copilot AI Nov 13, 2025

Choose a reason for hiding this comment

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

The config file path contains a hardcoded board name "nrf52840dk" which will be incorrect for boards generated from this template. This should either use a template variable like {{ board }} or be removed if there's no generic way to handle this configuration.

Suggested change
board_runner_args(nrfutil "--ext-mem-config-file=${BOARD_DIR}/support/nrf52840dk_qspi_nrfutil_config.json")
board_runner_args(nrfutil "--ext-mem-config-file=${BOARD_DIR}/support/{{ board }}_qspi_nrfutil_config.json")

Copilot uses AI. Check for mistakes.
@NordicBuilder
Copy link
Contributor

NordicBuilder commented Nov 13, 2025

CI Information

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

Inputs:

Sources:

sdk-nrf: PR head: 6d0ab2deac3ef97bce4086d9a2e1c09402f41e9b

more details

sdk-nrf:

PR head: 6d0ab2deac3ef97bce4086d9a2e1c09402f41e9b
merge base: 9022759eb4627eb67ccbc8465c2305e11f9fb78e
target head (main): 31417239e6281319c64218bec52ee98075475420
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 (5)
scripts
│  ├── requirements-fixed.txt
│  ├── west_commands
│  │  ├── create_board
│  │  │  ├── templates
│  │  │  │  ├── nrf52
│  │  │  │  │  │ board.cmake.jinja2
│  │  │  │  ├── nrf53
│  │  │  │  │  │ board.cmake.jinja2
│  │  │  │  ├── nrf54l
│  │  │  │  │  │ board.cmake.jinja2
│  │  │  │  ├── nrf91
│  │  │  │  │  │ board.cmake.jinja2

Outputs:

Toolchain

Version:
Build docker image:

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

  • ❌ Toolchain
  • ❌ Build twister
  • ❌ Integration tests
    • ❌ test-fw-nrfconnect-ble_mesh
Disabled integration tests
    • test-fw-nrfconnect-nrf_lrcs_positioning
    • desktop52_verification
    • test_ble_nrf_config
    • test-fw-nrfconnect-apps
    • test-fw-nrfconnect-ble_samples
    • test-fw-nrfconnect-chip
    • test-fw-nrfconnect-fem
    • test-fw-nrfconnect-nfc
    • 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-dfu
    • 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

@@ -1,10 +1,13 @@
board_runner_args(jlink "--device={{ soc | replace("nrf", "nRF") }}_xx{{ variant[2:] | upper }}" "--speed=4000")
if(CONFIG_SOC_NRF52840_QIAA)
Copy link
Contributor

Choose a reason for hiding this comment

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

this should only be present if the soc is nrf52840, not as in "if(CONFIG..." in the output file but just the one line if soc == nrf52840. Also this depends on if they have QSPI set up or even wired in...

endif()

if(CONFIG_BOARD_{{ board | upper }}_NRF5340_CPUAPP OR CONFIG_BOARD_{{ board | upper }}_NRF5340_CPUAPP_NS)
board_runner_args(nrfutil "--ext-mem-config-file=${BOARD_DIR}/support/nrf5340dk_qspi_nrfutil_config.json")
Copy link
Contributor

Choose a reason for hiding this comment

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

this too, a board might not have QSPI flash at all, or if it does it might have a very different configuration, on different pins, different mode, etc. plus I don't see this file even here?

The current version has a vulnerability:
GHSA-f83h-ghpp-7wcc

update to 20251107.

Signed-off-by: Carles Cufi <[email protected]>
@carlescufi carlescufi requested review from a team as code owners November 14, 2025 12:48
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.

3 participants