Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
7 changes: 7 additions & 0 deletions drivers/flash/flash_stm32_ex_op.c
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,13 @@ int flash_stm32_option_bytes_lock(const struct device *dev, bool enable)
{
FLASH_TypeDef *regs = FLASH_STM32_REGS(dev);

#if defined(FLASH_CR_OBL_LAUNCH)
/* Force the option byte loading before locking */
if (enable) {
regs->CR |= FLASH_CR_OBL_LAUNCH;
}
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 this change makes sense but would deserve a dedicated commit, aside proposing changes in flash_stm32_option_bytes_write().

#endif

#if defined(FLASH_OPTCR_OPTLOCK) /* F2, F4, F7 or H7 */
if (enable) {
regs->OPTCR |= FLASH_OPTCR_OPTLOCK;
Expand Down
19 changes: 11 additions & 8 deletions drivers/flash/flash_stm32g4x.c
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ LOG_MODULE_REGISTER(LOG_DOMAIN);
#include <zephyr/init.h>
#include <soc.h>
#include <stm32_ll_system.h>
#include <stm32_ll_cortex.h>
#include <stm32_ll_pwr.h>

#include "flash_stm32.h"

Expand Down Expand Up @@ -280,14 +282,6 @@ int flash_stm32_option_bytes_write(const struct device *dev, uint32_t mask,
/* Make sure previous write is completed. */
barrier_dsync_fence_full();

rc = flash_stm32_wait_flash_idle(dev);
if (rc < 0) {
return rc;
}

/* Force the option byte loading */
regs->CR |= FLASH_CR_OBL_LAUNCH;

return flash_stm32_wait_flash_idle(dev);
}

Expand Down Expand Up @@ -326,6 +320,15 @@ void flash_stm32_set_rdp_level(const struct device *dev, uint8_t level)
{
flash_stm32_option_bytes_write(dev, FLASH_OPTR_RDP_Msk,
(uint32_t)level << FLASH_OPTR_RDP_Pos);

/*
* POR needed to reload option bytes, done at runtime by entering standby mode
* /!\ make sure a wakeup source is configured!
*/
LL_PWR_SetPowerMode(LL_PWR_MODE_STANDBY);
LL_LPM_EnableDeepSleep();
/* enter SLEEP mode : WFE or WFI */
k_cpu_idle();
Comment on lines +330 to +331
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
/* enter SLEEP mode : WFE or WFI */
k_cpu_idle();
__disable_irq();
__SEV();
__WFE();
__WFE();

otherwise entry in low-power state is not guaranteed (and even this sequence may not be 100% safe)

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 we should have a STM32 SoC helper function to go into Standby mode.

Copy link
Contributor

Choose a reason for hiding this comment

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

This should go in common flash code if anywhere; PM needs something else (we don't want to force entry in low-power mode while doing PM, unlike here)

Copy link
Contributor

Choose a reason for hiding this comment

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

Note that is function should never return unless it fails (to reach SoC standby mode). I think the function declaration inline description should reflect this.

Copy link
Contributor

Choose a reason for hiding this comment

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

True too, I guess it should be FUNC_NORETURN + end with an infinite loop

}
#endif /* CONFIG_FLASH_STM32_READOUT_PROTECTION */

Expand Down
20 changes: 12 additions & 8 deletions drivers/flash/flash_stm32l4x.c
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@ LOG_MODULE_REGISTER(LOG_DOMAIN);
#include <zephyr/sys/barrier.h>
#include <zephyr/init.h>
#include <soc.h>
#include <stm32_ll_system.h>
#include <stm32_ll_cortex.h>
#include <stm32_ll_pwr.h>

#include "flash_stm32.h"

Expand Down Expand Up @@ -271,14 +274,6 @@ int flash_stm32_option_bytes_write(const struct device *dev, uint32_t mask,
/* Make sure previous write is completed. */
barrier_dsync_fence_full();

rc = flash_stm32_wait_flash_idle(dev);
if (rc < 0) {
return rc;
}

/* Force the option byte loading */
regs->CR |= FLASH_CR_OBL_LAUNCH;
Copy link
Contributor

Choose a reason for hiding this comment

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

Option bytes other than RDP in OTPR require OBL_LAUNCH or power reset to be loaded.
I think setting here OBL_LAUNCH is still applicable while flash_stm32_set_rdp_level() can still force a power reset (as proposed in next commit).


return flash_stm32_wait_flash_idle(dev);
}

Expand Down Expand Up @@ -317,6 +312,15 @@ void flash_stm32_set_rdp_level(const struct device *dev, uint8_t level)
{
flash_stm32_option_bytes_write(dev, FLASH_OPTR_RDP_Msk,
(uint32_t)level << FLASH_OPTR_RDP_Pos);

/*
* POR needed to reload option bytes, done at runtime by entering standby mode
* /!\ make sure a wakeup source is configured!
*/
LL_PWR_SetPowerMode(LL_PWR_MODE_STANDBY);
LL_LPM_EnableDeepSleep();
/* enter SLEEP mode : WFE or WFI */
k_cpu_idle();
}
#endif /* CONFIG_FLASH_STM32_READOUT_PROTECTION */

Expand Down