Skip to content

Conversation

hakonfam
Copy link
Contributor

@hakonfam hakonfam commented Oct 3, 2025

The update will fail if the address is outside of this range. This failure might trigger a bad state where the device is non-trivial to recover.

config UPDATE_BLOB_ADDRESS
hex "Address of the update blob"
default 0xe100000
default 0x0e100000
Copy link
Contributor

Choose a reason for hiding this comment

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

Shall we use a kconfig range for documentation's sake and to detect misconfig earlier?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought about the same, but was a bit reluctant as this is just the sample. But I'm open to it if anyone thinks that we should do it that way.

Copy link
Contributor

@SebastianBoe SebastianBoe Oct 3, 2025

Choose a reason for hiding this comment

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

(NB: Not feasible to have a range if we take these values from the MDK though...)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved the defines to the header file and added a build assert to the sample.

help
Maximum value of address passed to the update service.
The biggest update (USLOT) occupies 160kB, so this address is set so that a 160kB
update would still be within the limit of MRAM11 (0x0e200000).
Copy link
Contributor

Choose a reason for hiding this comment

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

Assuming we don't already, could we have this 160kB number in a header file somewhere, so that we could calculate MRAM11_START + MRAM11_SIZE - USLOT_MAX_SIZE ?

If we are not confident this 160kB number will be platform portable we could also #error-out when a new platform leaves it undefined. Something that is not as easy to do in Kconfig.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added this suggestion to the header file.

int err;
struct ironside_call_buf *const buf = ironside_call_alloc();

if ((uintptr_t)update < CONFIG_NRF_IRONSIDE_UPDATE_SERVICE_MIN_ADDRESS ||
Copy link
Contributor

Choose a reason for hiding this comment

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

I assume the MDK defines the start address of MRAM11?

Then porting to new platforms would be easier I would think.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The limit is not tied to MRAM11, so we cannot use MDK for this as it might differ across platforms.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Its not tied to MRAM11.


config NRF_IRONSIDE_UPDATE_SERVICE_MAX_ADDRESS
hex
default 0x0e1d8000
Copy link
Contributor Author

Choose a reason for hiding this comment

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

does it give any value to make these depend on the update service kconfig?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved to the header file.

@hakonfam hakonfam force-pushed the add-min-max-ironside-update branch from cfaaa99 to 625de6b Compare October 6, 2025 10:40
@hakonfam hakonfam requested a review from SebastianBoe October 6, 2025 10:54
SebastianBoe
SebastianBoe previously approved these changes Oct 6, 2025
The update will fail if the address is outside  of this range.
This failure might trigger a bad state where the device is
non-trivial to recover.

Signed-off-by: Håkon Amundsen <[email protected]>
Copy link

sonarqubecloud bot commented Oct 6, 2025

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

Successfully merging this pull request may close these issues.

6 participants