Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

modules: mcuboot: Disable Zephyr's MCUboot handling for child images #8

Closed

Conversation

MarekPieta
Copy link

Change disables Zephyr's MCUboot handling in child images. The NCS handles needed operations on its own. Change is needed to ensure consistency for main application image and child images.

Jira: NCSDK-22347

@de-nordic
Copy link

@nordicjm Can you take a look?

@nordicjm
Copy link

I do not think this is right and if the idea is to propagate up to the target ncs image, this will not work with sysbuild-ncs @tejlmand

@MarekPieta
Copy link
Author

Rebased

@MarekPieta
Copy link
Author

MarekPieta commented Jun 22, 2023

I do not think this is right and if the idea is to propagate up to the target ncs image, this will not work with sysbuild-ncs @tejlmand

It seems that we cannot remove the PARENT_SCOPE here (the information would no longer be propagated to Zephyr build system so it would not work as expected). I also understand that the solution is temporary, has its drawbacks and may need to be improved later (I copied it from the bottom of the CMakeLists.txt file to make handling mcuboot_secondary_app variant image consistent with the main application image).

@rlubos
Copy link
Owner

rlubos commented Jun 26, 2023

@nordicjm @tejlmand Any feedback wrt above comment?

@rlubos rlubos force-pushed the upmerge-19-06-2023 branch 2 times, most recently from 3433776 to 0f70010 Compare June 26, 2023 13:01
rlubos and others added 9 commits June 27, 2023 10:56
Update manifest to point to the latest Zephyr and Mcuboot revisions
after an upmerge.
Include fixes in other repositories.

Signed-off-by: Robert Lubos <robert.lubos@nordicsemi.no>
Since Zephyr 3.4 old Ztest API is deprecated and all tests with old
API fail during building by Twister.

Signed-off-by: Piotr Golyzniak <piotr.golyzniak@nordicsemi.no>
Pull alignment of UARTE CMSIS driver to nrfx 3.0.0

Signed-off-by: Adam Wojasinski <adam.wojasinski@nordicsemi.no>
New nrfx release added namespace prefix to RTC symbols. This patch
aligns code to renamed symbols.

Signed-off-by: Adam Wojasinski <adam.wojasinski@nordicsemi.no>
nrfx 3.0.0 introduces multi instance support for I2S driver.
Aligned I2S driver API used in application and Kconfig symbols
to enable instance 0.

Signed-off-by: Adam Wojasinski <adam.wojasinski@nordicsemi.no>
Configuration macro for TIMER takes as a parameter frequency of the
timer that user wants to set. This patch align macro call to that.

Signed-off-by: Adam Wojasinski <adam.wojasinski@nordicsemi.no>
GPIOTE driver has been complitely reworked in nrfx 3.0.0.
Aligned code in a way that is suggested in migration notes from
nrfx 2.11.0 to nrfx 3.0.0 that is available at
https://github.com/NordicSemiconductor/nrfx/wiki/nrfx-2.11.0-to-3.0.0

Signed-off-by: Adam Wojasinski <adam.wojasinski@nordicsemi.no>
Change disables CONFIG_BOOT_SERIAL_IMG_GRP_HASH in MCUboot bootloader
configuration of boards that use nRF52820 SoC. This is needed to reduce
the memory consumption.

Jira: NCSDK-21638

Signed-off-by: Marek Pieta <Marek.Pieta@nordicsemi.no>
SNTP library no longer relies on POSIX network APIs/headers, and there's
no need for date_time lib to rely on them either. Therefore, make the
library indifferent on POSIX configuration by using zsock APIs directly.

Signed-off-by: Robert Lubos <robert.lubos@nordicsemi.no>
rlubos and others added 15 commits June 27, 2023 10:56
Align zigbee codebase with upstream 15.4 API changes.

Signed-off-by: Robert Lubos <robert.lubos@nordicsemi.no>
Align NCS codebase with the new struct mgmt_callback format.

Signed-off-by: Robert Lubos <robert.lubos@nordicsemi.no>
sensor_read() is now a part of the public sensor AI, therefore need to
use a different name in the sample to avoid conflict.

Signed-off-by: Robert Lubos <robert.lubos@nordicsemi.no>
Align with the upstream SPI CS API changes.

Signed-off-by: Robert Lubos <robert.lubos@nordicsemi.no>
Align bluetooth mesh tests with the upstream API changes.

Signed-off-by: Robert Lubos <robert.lubos@nordicsemi.no>
CONFIG_MINIMAL_LIBC_MALLOC_ARENA_SIZE was deprecated and should no
longer be used.

Signed-off-by: Robert Lubos <robert.lubos@nordicsemi.no>
To reduce RAM usage, `SENSOR_SHELL` is disabled.

Signed-off-by: Marcin Jelinski <marcin.jelinski@nordicsemi.no>
Required modules have been added to prj.conf.
These were previously added by the shield defconfig but
this has been deleted upstream.

Signed-off-by: Andy Sinclair <andy.sinclair@nordicsemi.no>
CONFIG_BT_HOST_CCM, which is a dependency for CONFIG_BT_ENOCEAN is no
longer enabled automatically by Zephyr, hence enable it manually in the
project file.

Signed-off-by: Robert Lubos <robert.lubos@nordicsemi.no>
Decrease number of USBD endpoints in DTS to reduce FLASH consumption.
The MCUboot uses USB only for serial recovery over CDC ACM class.

Jira: NCSDK-22309

Signed-off-by: Marek Pieta <Marek.Pieta@nordicsemi.no>
Workaround for enabling the external flash on nRF9160 DK when using
the nRF7002ek shield. Add external flash overlay to fix build failure.

Signed-off-by: Gregers Gram Rygg <gregers.gram.rygg@nordicsemi.no>
High drive mode is now enabled on SPI3 for the board definitions in
Zephyr. Clean up unnecessary enabling of high drive mode in overlays.

Signed-off-by: Gregers Gram Rygg <gregers.gram.rygg@nordicsemi.no>
Change aligns the upload reset handling in the DFU MCUmgr application
module to newly introduced public API of MCUmgr.

Jira: NCSDK-21434

Signed-off-by: Marek Pieta <Marek.Pieta@nordicsemi.no>
nrfx UARTE driver renamed member of the configuration structure
and added new parameter to driver API.

Signed-off-by: Adam Wojasinski <adam.wojasinski@nordicsemi.no>
Signed-off-by: Robert Lubos <robert.lubos@nordicsemi.no>
After the upmerge the sample no longer fit into FLASH on
nrf5340dk_nrf5340_cpunet. Remove an obsolete workaround from the
sample configuration to reduce sample FLASH footprint.

Signed-off-by: Robert Lubos <robert.lubos@nordicsemi.no>
sebastiandraus and others added 2 commits June 27, 2023 14:41
This commit enables NRF_STORE_REBOOT_TYPE_GPREGRET Kconfig option
for all Zigbee samples so the reset reason is stored between device
reboot.

Signed-off-by: Sebastian Draus <sebastian.draus@nordicsemi.no>
Change disables Zephyr's MCUboot handling in child images. The NCS
handles needed operations on its own. Change is needed to ensure
consistency for main application image and child images.

Jira: NCSDK-22347

Signed-off-by: Marek Pieta <Marek.Pieta@nordicsemi.no>
@rlubos rlubos force-pushed the upmerge-19-06-2023 branch 2 times, most recently from c58b5f8 to 65f911c Compare June 28, 2023 13:11
Copy link

@tejlmand tejlmand left a comment

Choose a reason for hiding this comment

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

We should not start to manipulate Kconfig settings inside CMake.

Can result in unintended / unexpected errors, especially at NCS downstream users who are not aware of such manipulation.

@@ -28,6 +28,10 @@ endif()

if(CONFIG_BOOTLOADER_MCUBOOT)
if (CONFIG_NCS_IS_VARIANT_IMAGE)
# NCS Handles everything regarding mcuboot, ensure Zephyr doesn't interfere.
# This is a temporary solution until Zephyr signing has been made more modular.
set(CONFIG_BOOTLOADER_MCUBOOT False PARENT_SCOPE)

Choose a reason for hiding this comment

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

this is not safe.

CONFIG_BOOTLOADER_MCUBOOT is a setting imported from the .config.
I acknowledge that it only impacts CMake configure time, and that CONFIG_BOOTLOADER_MCUBOOT is still set in autoconf.h and elsewhere in the system, such as python code loading the .config.

But in CMake, there can be customer code checking for this flag for other purposes than signing.

Has it been investigated why this happens for XIP images but not for other variant builds ?

Copy link
Author

Choose a reason for hiding this comment

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

Hi @tejlmand,

Yes, I am aware that the solution is not perfect and we should not manipulate Kconfig values inside CMake. I just make the behavior consistent with handling main application image - I actually used the same workaround (please take a look at the bottom of this CMakeLists.txt file, you will see the same hotfix applied for the main image):

# NCS Handles everything regarding mcuboot, ensure Zephyr doesn't interfere.
# This is a temporary solution until Zephyr signing has been made more modular.
set(CONFIG_BOOTLOADER_MCUBOOT False PARENT_SCOPE)

Copy link
Author

Choose a reason for hiding this comment

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

Could you provide a proper solution for the problem during the ongoing upmerge? Or maybe we could accept my temporary workaround and improve it later on (together with improving integration for the main application image)?

Copy link

Choose a reason for hiding this comment

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

I see that.
Not fond of that solution either, but at least you are consistent with what we are already doing.

Solution accepted, but please do a rebase.

Copy link
Author

Choose a reason for hiding this comment

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

I need to open a new PR to sdk-nrf main branch. Give me moment.

@rlubos rlubos force-pushed the upmerge-19-06-2023 branch 3 times, most recently from c71c583 to dd3123d Compare July 3, 2023 13:10
@MarekPieta
Copy link
Author

Upmerge is done already, opened PR to main that introduces the change: nrfconnect#11696.
Closing this one

@MarekPieta MarekPieta closed this Jul 4, 2023
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.

10 participants