Skip to content

Conversation

@simon-iversen
Copy link
Contributor

@simon-iversen simon-iversen commented May 12, 2022

Adds check for network core addresses when NSIB(Nordic Secure Immutable Bootlaoder) is enabled, so that network core updates still will work.

Ref. NCSIDB-696

@simon-iversen simon-iversen requested a review from sigvartmh May 12, 2022 16:20
@simon-iversen simon-iversen changed the title Now it's possible to update the network core when NSIB is enabled Make it's possible to update the network core when NSIB is enabled May 12, 2022
@sigvartmh
Copy link
Contributor

sigvartmh commented May 12, 2022

The commit message needs a fix otherwise the code looks good.

Add a title to the commit in the form of

[nrf noup] bootutil: loader: Add check for network core address when NSIB is enabled (maybe this title is too long)

Content should be something like:

This enables network core updates when NSIB and MCUBoot are enabled.

Ref. NCSIDB-696

Also remember to sign your commits with git commit -s

Try to have similar formatting as shown here e7db825

@sigvartmh sigvartmh added the bug Something isn't working label May 13, 2022
@sigvartmh sigvartmh added this to the ncs-2.0.0 milestone May 13, 2022
Copy link
Contributor

@sigvartmh sigvartmh left a comment

Choose a reason for hiding this comment

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

Code is okei, commit message needs a fixup.

@sigvartmh sigvartmh requested review from hakonfam and nvlsianpu May 13, 2022 09:57
@simon-iversen simon-iversen changed the title Make it's possible to update the network core when NSIB is enabled [nrf noup] bootutil: loader: Add check for net core address when NSIB is enabled May 13, 2022
@sigvartmh
Copy link
Contributor

sigvartmh commented May 16, 2022

Still needs a bit clean up in history, you now have 3 commits should be 1. If you don't want to maintain this I can take it over just ping me.

You could do git rebase -i HEAD~3 in your local repository then squash 1 of the commits and drop the other.

a3b29db
060c83d

@simon-iversen simon-iversen force-pushed the network_core_update_nsib_enabled branch from 060c83d to 0334f6a Compare May 16, 2022 11:32
@simon-iversen
Copy link
Contributor Author

Does it look good now?

If you don't want to maintain this I can take it over just ping me.

One time has to be the first. It's nice to go through this and learn how it's done.

@sigvartmh sigvartmh self-assigned this May 16, 2022
@simon-iversen simon-iversen force-pushed the network_core_update_nsib_enabled branch from 0334f6a to 1dd9ced Compare May 16, 2022 14:06
@simon-iversen simon-iversen changed the title [nrf noup] bootutil: loader: Add check for net core address when NSIB is enabled [nrf noup] bootutil: loader: Add check for netcore addr if NSIB enabled May 16, 2022
reset_addr = vtable[1];
#ifdef PM_S1_ADDRESS
const struct flash_area *primary_fa;
int rc = flash_area_open(flash_area_id_from_multi_image_slot(
Copy link
Contributor

Choose a reason for hiding this comment

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

This would work for the network image pair, but its primary slot is emulated in RAM, therefore the flash_area doesn't have real offset.

@nvlsianpu
Copy link
Contributor

@simon-iversen Can you open manifest PR ASAP?

@simon-iversen
Copy link
Contributor Author

@simon-iversen Can you open manifest PR ASAP?

nrfconnect/sdk-nrf#7669

@simon-iversen simon-iversen force-pushed the network_core_update_nsib_enabled branch from 299e29e to 7922381 Compare May 19, 2022 14:05
Add check for netcore addr if NSIB is enabled so netcore updates works

Ref. NCSIDB-696

Signed-off-by: Simon Iversen <simon.iversen@nordicsemi.no>
@simon-iversen simon-iversen force-pushed the network_core_update_nsib_enabled branch from 7922381 to c23f293 Compare May 20, 2022 15:50
@anangl anangl merged commit 129b631 into nrfconnect:main May 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants