Skip to content

Conversation

@SebastianBoe
Copy link
Contributor

@SebastianBoe SebastianBoe commented Apr 24, 2025

cpuconf

Comment on lines 6 to 7
OR CONFIG_BOARD_NRF54H20DK_NRF54H20_CPUAPP_IRON
OR CONFIG_BOARD_NRF54H20DK_NRF54H20_CPURAD_IRON)
Copy link
Contributor

Choose a reason for hiding this comment

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

This formatting makes my eyes bleed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You want one option on each line right?

Copy link
Contributor

Choose a reason for hiding this comment

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

OR on end-of-line, perhaps? CONFIG_ on same vertical?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah one per line would be nice instead of two on one then one on each of the next.

Sorry for the super-unclear comment :D (not sarcastic)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

Comment on lines 12 to 13
/* This is not yet an exhaustive memory map, and contain only a minimum required to boot
* the radio core.
Copy link
Contributor

Choose a reason for hiding this comment

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

Seems a bit off given that this is the radio core, a slight rephrase might be needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

Comment on lines 13 to 14
ram: 192 # Guessed value
flash: 256 # Guessed value
Copy link
Contributor

Choose a reason for hiding this comment

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

The RAM is as per spec, and flash corresponds to the memory map we have in the docs, so I would say that the comments here are superfluous

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

@@ -0,0 +1,28 @@
# Copyright (c) 2024 Nordic Semiconductor ASA
Copy link
Contributor

Choose a reason for hiding this comment

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

2025

select HAS_POWEROFF

config SOC_NRF54H20_CPURAD_ENABLE
bool "NRF54H20 Radio core is enabled at boot time"
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: "Enable nRF54H20 Radio core at boot time"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This broke CI.

Some kind of rule about prompts starting with Enable.

bool "NRF54H20 Radio core is enabled at boot time"
default y if NRF_802154_SER_HOST
help
This option enables releasing the Radio 'force off' signal, which
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this correct?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed


static int nrf54h20_cpurad_init(void)
{
uint32_t dummy = 0xbadcafe;
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we comment on why this is here? (missing boot report)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

Copy link
Contributor

Choose a reason for hiding this comment

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

I think at this point we should move the memory map into one file (I see that zephyrproject-rtos/zephyr#88937 does this)

Copy link
Contributor Author

@SebastianBoe SebastianBoe Apr 28, 2025

Choose a reason for hiding this comment

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

discussed offline. will be done at a later time

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

hakonfam and others added 6 commits April 28, 2025 16:11
…rd variant

The required jlink script was not added due to missing entry in the
board check. Also change the check for app vs rad to be on SOC level.

Signed-off-by: Håkon Amundsen <[email protected]>
(cherry picked from commit 4edc003)
This is needed for next generation Secure Domain firmware.

Upstream PR #: 88908

Signed-off-by: Sebastian Bøe <[email protected]>
Encode the radiocore's address into the application's DT as the
application needs to instruct IRONside SE about where the radiocore
should be booted from.

Upstream PR #: 89009

Signed-off-by: Sebastian Bøe <[email protected]>
Port SYS_INIT to use soc_early_init_hook as SYS_INITs are legacy.

Upstream PR #: 88908

Signed-off-by: Sebastian Bøe <[email protected]>
…nit_hook

Boot the radiocore from the app in soc_late_init_hook.

Upstream PR #: 88908

Signed-off-by: Sebastian Bøe <[email protected]>
…/cpuapp/iron

Add support for nrf54h20dk/nrf54h20/cpuapp/iron to the
samples/sysbuild/hello_world sample.

Upstream PR #: 88908

Signed-off-by: Sebastian Bøe <[email protected]>
@sonarqubecloud
Copy link

@SebastianBoe
Copy link
Contributor Author

creating a new PR due to the desire to have a bot manage the manifest PR

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants