Skip to content

API for request MCUboot mode by the application #44512

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.

Sign up for GitHub

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

Closed
wants to merge 3 commits into from
Closed

API for request MCUboot mode by the application #44512

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6a59acb
dfu/Kconfig: clean up MCUBOOT_IMG_MANAGER dependence
nvlsianpu Apr 4, 2022
8738722
dfu: API for request MCUboot mode
nvlsianpu Apr 4, 2022
8fb78c4
soc/arm/nordic_nrf: implementation of request and fetch the MCUboot mode
nvlsianpu Apr 4, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions soc/arm/nordic_nrf/nrf52/soc.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,22 @@ extern void z_arm_nmi_init(void);
#define LOG_LEVEL CONFIG_SOC_LOG_LEVEL
LOG_MODULE_REGISTER(soc);

int soc_mcuboot_mode_set(uint8_t mode)
Copy link
Contributor

Choose a reason for hiding this comment

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

Nothing wrong with this line of code, but the Signed-off-by: line in the commit message has the wrong format.

{
nrf_power_gpregret2_set(NRF_POWER, mode);
Copy link
Contributor

Choose a reason for hiding this comment

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

From spot-checking a product specification, the GPREGRET and GPREGRET2 registers seem to be available for application developer use too. Is there any risk in using them for this purpose?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

GPREGRET is used by NCS internally for instance, it is also place where nRF SoC stores reboot-type in zephyr.

Copy link
Collaborator Author

@nvlsianpu nvlsianpu May 23, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

I can use GPREGRET for transfer both the reboot-type and the recovery-dfu request. This implies that I need to introduce an API for retrieve reboot-type as well.
What your opinion @mbolivar

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, I don't understand your last comment

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

If you see usage of GPREGRET2 as the waste of resources I can make a better solution:

The idea would be to start using GPREGRET for request of recovery-dfu as well (along with reboot-type to keep of which it is used currently).
As intermediate steep I would implement API for retrieve reboot-type as there is sys_arch_reboot(int type) call but there is no a counterpart to it which would allows to check what a reboot-type was used.
Of course this mean that zephyr common API for get reboot type will be needed:
there is void sys_reboot(int type) but zephyr doesn't have API for retrieve the type

@mbolivar-nordic What is your opinion on that ^^ idea?

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, OK, I think I understand now. No, if we're already using GPREGRET, from my side I see no harm in using GPREGRET2 as well. You're saying you're not aware of any customers already using this that will be negatively impacted, right?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

alternative: #46084


return 0;
}

uint8_t soc_mcuboot_mode_get(void)
{
uint8_t mode = nrf_power_gpregret2_get(NRF_POWER);

nrf_power_gpregret2_set(NRF_POWER, 0);

return mode;
}

/* Overrides the weak ARM implementation:
Set general purpose retention register and reboot */
void sys_arch_reboot(int type)
Expand Down
20 changes: 20 additions & 0 deletions soc/arm/nordic_nrf/nrf53/soc.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,13 @@
#include <hal/nrf_gpio.h>
#include <hal/nrf_oscillators.h>
#include <hal/nrf_regulators.h>
#include <hal/nrf_power.h>
#elif defined(CONFIG_SOC_NRF5340_CPUNET)
#include <hal/nrf_nvmc.h>
#endif
#include <soc_secure.h>


#define PIN_XL1 0
#define PIN_XL2 1

Expand Down Expand Up @@ -66,6 +68,24 @@ extern void z_arm_nmi_init(void);
#define LOG_LEVEL CONFIG_SOC_LOG_LEVEL
LOG_MODULE_REGISTER(soc);

#if defined(CONFIG_SOC_NRF5340_CPUAPP)
int soc_mcuboot_mode_set(uint8_t mode)
{
nrf_power_gpregret2_set(NRF_POWER, mode);

return 0;
}

uint8_t soc_mcuboot_mode_get(void)
{
uint8_t mode = nrf_power_gpregret2_get(NRF_POWER);

nrf_power_gpregret2_set(NRF_POWER, 0);

return mode;
}
#endif

static int nordicsemi_nrf53_init(const struct device *arg)
{
uint32_t key;
Expand Down
17 changes: 17 additions & 0 deletions soc/arm/nordic_nrf/nrf91/soc.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#include <arch/arm/aarch32/cortex_m/cmsis.h>
#include <soc/nrfx_coredep.h>
#include <logging/log.h>
#include <hal/nrf_power.h>

#ifdef CONFIG_RUNTIME_NMI
extern void z_arm_nmi_init(void);
Expand All @@ -34,6 +35,22 @@ extern void z_arm_nmi_init(void);
#define LOG_LEVEL CONFIG_SOC_LOG_LEVEL
LOG_MODULE_REGISTER(soc);

int soc_mcuboot_mode_set(uint8_t mode)
{
nrf_power_gpregret_ext_set(NRF_POWER, 1, mode);

return 0;
}

uint8_t soc_mcuboot_mode_set(void)
{
uint8_t mode = nrf_power_gpregret_ext_get(NRF_POWER, 1);

nrf_power_gpregret_ext_set(NRF_POWER, 1, 0);

return mode;
}

static int nordicsemi_nrf91_init(const struct device *arg)
{
uint32_t key;
Expand Down