-
Notifications
You must be signed in to change notification settings - Fork 8.1k
driver: flash: enable stm32 read-back protection at runtime #91945
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
base: main
Are you sure you want to change the base?
driver: flash: enable stm32 read-back protection at runtime #91945
Conversation
do not force option byte reloading in the write function because RDP byte cannot be reloaded that way: it needs a POR through Standby mode. when FLASH_CR_OBL_LAUNCH is defined, use it to force-reload option bytes directly in the locking function. Signed-off-by: Cyril Fougeray <[email protected]>
51b4abc
to
79d7d57
Compare
for the read-back protection option byte to be successfully reloaded at runtime, a power-on reset can be achieved by putting the microcontroller into Standby mode and waking up from that mode. Signed-off-by: Cyril Fougeray <[email protected]>
79d7d57
to
57a3135
Compare
|
This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time. |
/* enter SLEEP mode : WFE or WFI */ | ||
k_cpu_idle(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/* 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)
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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)
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
drivers/flash/flash_stm32l4x.c
Outdated
#include <stm32_ll_system.h> | ||
#include <stm32l4xx_ll_cortex.h> | ||
#include <stm32l4xx_ll_pwr.h> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why adding these header files?
} | ||
|
||
/* Force the option byte loading */ | ||
regs->CR |= FLASH_CR_OBL_LAUNCH; |
There was a problem hiding this comment.
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).
/* Force the option byte loading before locking */ | ||
if (enable) { | ||
regs->CR |= FLASH_CR_OBL_LAUNCH; | ||
} |
There was a problem hiding this comment.
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()
.
/* enter SLEEP mode : WFE or WFI */ | ||
k_cpu_idle(); |
There was a problem hiding this comment.
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.
@fouge Do you plan to push this PR further ? |
when writing the read-back protection option byte, the stm32 target must go through a power-on reset to reload the option bytes correctly, which is achieved by going into Standby or Shutdown mode.
using the OBL bit locks the microcontroller in a bad state waiting for a full power cycle, this buggy behavior has been added in that PR.
Instead of force-reloading when writing any option byte, it's now performed when re-locking the option bytes, and if OBL bit is defined. Locking doesn't apply when changing the RDP level, since it goes through a POR, so this solution allows to set and reload any option byte correctly.
see extract from stm32g4xx datasheet:
a few concerns: