[nrf toup] nordic: Reorganize how gen_uicr.py is invoked #3202

SebastianBoe · 2025-08-15T08:49:10Z

Reorganize how gen_uicr.py is invoked

Ref: NCSDK-NONE

jonathannilsen · 2025-08-20T07:58:11Z

soc/nordic/common/uicr/gen_uicr.py

@@ -169,13 +149,25 @@ def main() -> None:
        "--out-uicr-hex",
        required=True,
        type=argparse.FileType("w", encoding="utf-8"),
-        help="Path to write the generated UICR HEX file to",
+        help="Path to write the generated merged UICR+PERIPHCONF HEX file to (typically zephyr.hex)",


These artifacts are useful to have separate depending on your application, so the choice to keep them separate was deliberate. Can we keep them as two different outputs and use something like mergehex afterwards instead?

How are they useful? :)

It is possible to have them separate. But it would complicate the build system, and also to a certain degree the user experience.

So I would prefer to not keep them separate unless necessary.

The PERIPHCONF area in MRAM can be modified even when UICR is locked, as long as you haven't put it in the PROTECTEDMEM region. So there is a use case for pulling out just the periphconf.hex part and including it in a DFU package without the UICR part.

customers might bundle the periphconf in their ota image but not the rest of the uicr

I see, I did not think of that, will fix.

I suppose even in non-DFU use-cases, you may have locked your UICR, and which to west flash a new periphconf, but not the UICR.

I could make this configurable via KConfig.

hex files restored

jonathannilsen · 2025-08-20T07:59:30Z

soc/nordic/common/uicr/gen_uicr.py

-                f"Expected a DT node with label '{UICR_NODELABEL}'."
-            ) from e
+            uicr.PERIPHCONF.ADDRESS = args.periphconf_address
+            uicr.PERIPHCONF.MAXCOUNT = math.floor(args.periphconf_size / 8)


nit: noticed this earlier, but this could just be args.periphconf_size // 8

How about we error-out when it's not divisible by 8?

karstenkoenig

Ah super nice, but please split into independent hex files again

karstenkoenig · 2025-08-20T10:10:15Z

soc/nordic/common/uicr/gen_uicr/Kconfig

+
+menu "UICR generator options"
+
+config UICR_SAMPLE_GENERATE_PERIPHCONF


What does the sample in the name stand for?

It is trying to communicate to the reader that this Kconfig option is specific to this gen_uicr "sample".

As opposed to most other Kconfigs in ~/ncs/zephyr/soc, which are read by all zephyr images.

Maybe

GEN_UICR_SAMPLE_GENERATE_PERIPHCONF

Would be clearer?

Or do you have another idea?

sample in the sense of example? That's what I assume and find confusing, this shouldn't be example code right?
Or in sample the sense of this one unit? image or target?

Wouldn't GEN_UICR_GENERATE_PERIPHCONF be enough? Or would that be confusing because it only applies to the uicr target and not actually be set in the app or radio build?

nordicjm

Looks good!

nordicjm · 2025-08-20T13:27:34Z

soc/nordic/common/uicr/Kconfig.sysbuild

+	default y
+	help
+	  Generate UICR HEX file.
+


trailing newline

nordicjm · 2025-08-20T13:31:25Z

soc/nordic/common/uicr/gen_uicr/CMakeLists.txt

+  dt_nodelabel(partition_path NODELABEL ${partition_nodelabel})
+  if(NOT DEFINED partition_path)
+    message(FATAL_ERROR "${partition_nodelabel} not found in devicetree")
+  endif()


Suggested change

dt_nodelabel(partition_path NODELABEL ${partition_nodelabel})

if(NOT DEFINED partition_path)

message(FATAL_ERROR "${partition_nodelabel} not found in devicetree")

endif()

dt_nodelabel(partition_path NODELABEL ${partition_nodelabel} REQUIRED)

Nice!

Was not aware.

nordicjm · 2025-08-20T13:32:33Z

soc/nordic/common/uicr/gen_uicr/CMakeLists.txt

+# Use CMAKE_VERBOSE_MAKEFILE to silence an unused-variable warning.
+if(CMAKE_VERBOSE_MAKEFILE)
+endif()


I got 5 lines of warning in the logs when this variable went unreferenced.

I assume SYSBUILD passes it to each image, and Zephyr usually references it, but since I have this minimal Zephyr it goes unreferenced.

nordicjm · 2025-08-20T13:33:09Z

soc/nordic/common/uicr/gen_uicr/CMakeLists.txt

+
+set(periphconf_args)
+set(periphconf_elfs)
+set(merged_hex_file ${APPLICATION_BINARY_DIR}/zephyr/zephyr.hex)


use the Kconfig for this for the bin name

fixed

I couldn't find neither a variable nor a Kconfig that contained the hex suffix unfortunately.

nordicjm · 2025-08-20T13:34:51Z

soc/nordic/common/uicr/gen_uicr/Kconfig

@@ -0,0 +1,13 @@
+# SPDX-License-Identifier: Apache-2.0
+
+source "Kconfig.zephyr"


nit. non-blocking - might want to put this at bottom so that the application Kconfigs appear at the top

nordicjm · 2025-08-20T13:35:31Z

soc/nordic/common/uicr/gen_uicr/prj.conf

don't think prj.conf is needed and it will fallback to empty.conf? Could be wrong on that though

I got an error without it earlier, but I'll retest.

I get this error without it:

-- Board: nrf54h20dk, Revision: 0.9.0, qualifiers: nrf54h20/cpurad CMake Error at /home/sebo/ncs/zephyr/cmake/modules/extensions.cmake:2919 (message): No prj.conf file(s) was found in the /home/sebo/ncs/zephyr/soc/nordic/common/uicr/gen_uicr folder(s), please read the Zephyr documentation on application development. Call Stack (most recent call first): /home/sebo/ncs/zephyr/cmake/modules/configuration_files.cmake:40 (zephyr_file) /home/sebo/ncs/zephyr/cmake/modules/zephyr_default.cmake:131 (include) /home/sebo/ncs/zephyr/share/zephyr-package/cmake/ZephyrConfig.cmake:75 (include) /home/sebo/ncs/zephyr/share/zephyr-package/cmake/ZephyrConfig.cmake:92 (include_boilerplate) CMakeLists.txt:15 (find_package) -- Configuring incomplete, errors occurred! CMake Error at cmake/modules/sysbuild_extensions.cmake:530 (message): CMake configure failed for Zephyr project: uicr Location: /home/sebo/ncs/zephyr/soc/nordic/common/uicr/gen_uicr Call Stack (most recent call first): cmake/modules/sysbuild_images.cmake:43 (ExternalZephyrProject_Cmake) cmake/modules/sysbuild_default.cmake:21 (include) /home/sebo/ncs/zephyr/share/zephyr-package/cmake/ZephyrConfig.cmake:75 (include) /home/sebo/ncs/zephyr/share/zephyr-package/cmake/ZephyrConfig.cmake:92 (include_boilerplate) /home/sebo/ncs/zephyr/share/sysbuild-package/cmake/SysbuildConfig.cmake:8 (include) template/CMakeLists.txt:10 (find_package) -- Configuring incomplete, errors occurred!

keep the empty file, maybe something to look at in future @tejlmand

nordicjm · 2025-08-20T13:35:50Z

soc/nordic/common/uicr/sysbuild.cmake

+
+# Ensure UICR is configured and built after the default image so EDT/ELFs exist.
+sysbuild_add_dependencies(CONFIGURE uicr ${DEFAULT_IMAGE})
+add_dependencies(                   uicr ${DEFAULT_IMAGE})


Suggested change

add_dependencies( uicr ${DEFAULT_IMAGE})

add_dependencies(uicr ${DEFAULT_IMAGE})

jonathannilsen · 2025-08-20T14:24:06Z

soc/nordic/common/uicr/gen_uicr.py

+            # Update UICR hex with the modified UICR data
+            uicr_data = bytes(uicr)
+            uicr_hex = IntelHex()  # Reset and rebuild with updated UICR
+            uicr_hex.frombytes(uicr_data, offset=args.uicr_address)


I think this might flow a bit cleaner if we populated each hex object just once, instead of populating them with initial data first and then overwriting it. For example:

periphconf_hex = IntelHex() if args.in_periphconf_elfs: # Check if periphconf data is provided periphconf_combined = extract_and_combine_periphconfs(args.in_periphconf_elfs) padding_len = args.periphconf_size - len(periphconf_combined) periphconf_final = periphconf_combined + bytes([0xFF for _ in range(padding_len)]) # Add periphconf data to separate and merged hex files periphconf_hex.frombytes(periphconf_final, offset=args.periphconf_address) uicr.PERIPHCONF.ENABLE = ENABLED_VALUE uicr.PERIPHCONF.ADDRESS = args.periphconf_address uicr.PERIPHCONF.MAXCOUNT = args.periphconf_size // 8 uicr_hex = IntelHex() uicr_hex.frombytes(bytes(uicr), offset=args.uicr_address) merged_hex = IntelHex() merged_hex.fromdict(uicr_hex.todict()) if periphconf_hex: merged_hex.fromdict(periphconf_hex.todict()) # Write the three hex files merged_hex.write_hex_file(args.out_merged_hex) uicr_hex.write_hex_file(args.out_uicr_hex) if args.out_periphconf_hex: periphconf_hex.write_hex_file(args.out_periphconf_hex)

(I think IntelHex.todict() works there, but haven't tried running it)

much better!

jonathannilsen · 2025-08-20T14:28:02Z

soc/nordic/common/uicr/gen_uicr.py

+        if args.in_periphconf_elfs:  # Check if periphconf data is provided
+            periphconf_combined = extract_and_combine_periphconfs(args.in_periphconf_elfs)


I think we should retain the ability to generate an empty (0xFFFF_FFFFs) periphconf blob if no elf is provided here - an empty periphconf is still valid

soc/nordic/common/uicr/gen_uicr.py

SebastianBoe · 2025-08-21T09:08:17Z

@karstenkoenig , @jonathannilsen , @nordicjm :

All feedback addressed, will reupload to upstream zephyr soon.

jonathannilsen · 2025-08-21T09:58:40Z

soc/nordic/common/uicr/gen_uicr/zephyr/CMakeLists.txt

Should there be a license header in this file?

jonathannilsen · 2025-08-21T10:03:49Z

soc/nordic/common/uicr/gen_uicr/CMakeLists.txt

+    endif()
+
+    if(EXISTS ${_dir}/zephyr/zephyr.dts)
+      list(APPEND periphconf_elfs ${_dir}/zephyr/zephyr.elf)


I guess this should also take into account whether KERNEL_BIN_NAME has been changed in the image it's getting from? Not sure how that is best handled

I don't expect this to ever be anything but zephyr

Reorganize how gen_uicr.py is invoked. Instead of invoking it from one of the Zephyr images we invoke it from a new special Zephyr image called uicr. This uicr Zephyr image is flashed in the same way as normal Zephyr images so special handling in the runner is no longer necessary. Also, we simplify gen_uicr.py by moving parsing of Kconfig/DT from gen_uicr.py to CMake. Signed-off-by: Sebastian Bøe <[email protected]>

SebastianBoe · 2025-08-21T10:18:49Z

These patches are now up for review upstream in zephyrproject-rtos/zephyr#94796

NordicBuilder mentioned this pull request Aug 15, 2025

manifest: sdk-zephyr: [NOT FOR REVIEW] [FOR CI TESTING ONLY] [nrf toup] nordic: Reorganize how gen_uicr.py is invoked nrfconnect/sdk-nrf#24011

Open

SebastianBoe force-pushed the ncs_ps branch 2 times, most recently from dc098ca to 546e163 Compare August 18, 2025 10:20

NordicBuilder mentioned this pull request Aug 18, 2025

ironside: Generate periphconf from Zephyr images instead of sysbuild nrfconnect/sdk-nrf#24028

Open

SebastianBoe changed the title ~~[NOT FOR REVIEW] [FOR CI TESTING ONLY] [nrf toup] nordic: Reorganize how gen_uicr.py is invoked~~ [nrf toup] nordic: Reorganize how gen_uicr.py is invoked Aug 18, 2025

jonathannilsen requested changes Aug 20, 2025

View reviewed changes

karstenkoenig approved these changes Aug 20, 2025

View reviewed changes

SebastianBoe force-pushed the ncs_ps branch from 546e163 to 5562195 Compare August 20, 2025 12:07

nordicjm reviewed Aug 20, 2025

View reviewed changes

jonathannilsen self-requested a review August 20, 2025 13:39

jonathannilsen reviewed Aug 20, 2025

View reviewed changes

jonathannilsen reviewed Aug 21, 2025

View reviewed changes

soc/nordic/common/uicr/gen_uicr.py Show resolved Hide resolved

SebastianBoe force-pushed the ncs_ps branch from 5562195 to 9393ba2 Compare August 21, 2025 09:05

SebastianBoe force-pushed the ncs_ps branch from 9393ba2 to f55f142 Compare August 21, 2025 09:48

jonathannilsen approved these changes Aug 21, 2025

View reviewed changes

SebastianBoe force-pushed the ncs_ps branch from f55f142 to 9f197a3 Compare August 21, 2025 10:08


		menu "UICR generator options"

		config UICR_SAMPLE_GENERATE_PERIPHCONF

		@@ -0,0 +1,13 @@
		# SPDX-License-Identifier: Apache-2.0

		source "Kconfig.zephyr"

	add_dependencies( uicr ${DEFAULT_IMAGE})
	add_dependencies(uicr ${DEFAULT_IMAGE})

		if args.in_periphconf_elfs: # Check if periphconf data is provided
		periphconf_combined = extract_and_combine_periphconfs(args.in_periphconf_elfs)

[nrf toup] nordic: Reorganize how gen_uicr.py is invoked #3202

Are you sure you want to change the base?

[nrf toup] nordic: Reorganize how gen_uicr.py is invoked #3202

Conversation

SebastianBoe commented Aug 15, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

karstenkoenig left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nordicjm left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SebastianBoe Aug 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SebastianBoe Aug 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SebastianBoe Aug 20, 2025 •

edited

Loading

SebastianBoe Aug 21, 2025 •

edited

Loading

jonathannilsen Aug 20, 2025 •

edited

Loading