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

boards: st: Minimize usage of "--erase" stm32cubeprogrammer option #69582

Closed
erwango opened this issue Feb 28, 2024 · 12 comments
Closed

boards: st: Minimize usage of "--erase" stm32cubeprogrammer option #69582

erwango opened this issue Feb 28, 2024 · 12 comments
Assignees
Labels
Enhancement Changes/Updates/Additions to existing features platform: STM32 ST Micro STM32

Comments

@erwango
Copy link
Member

erwango commented Feb 28, 2024

Is your enhancement proposal related to a problem? Please describe.
On stm32 boards, following stm32cubeprogrammer configuration is commonly found:

board_runner_args(stm32cubeprogrammer "--erase" "--port=swd" "--reset-mode=hw")

This raises an issue when flashing back to back a bootloader and an application image.

Describe the solution you'd like
Use the following:

if(CONFIG_BOOTLOADER_MCUBOOT)
board_runner_args(stm32cubeprogrammer "--port=swd" "--reset-mode=hw")
else()
board_runner_args(stm32cubeprogrammer "--erase" "--port=swd" "--reset-mode=hw")
endif()

Describe alternatives you've considered
A clear and concise description of any alternative solutions or features you've considered.

Additional context
Add any other context or graphics (drag-and-drop an image) about the feature request here.

@erwango erwango added Enhancement Changes/Updates/Additions to existing features platform: STM32 ST Micro STM32 Good first issue Good for a first time contributor to take labels Feb 28, 2024
@erwango
Copy link
Member Author

erwango commented Feb 28, 2024

As a follow up action:

  • create a boards/st/common/board.cmake file
  • put all boilerplate code in that file (suchas the "if/else/endif") snippet above
  • cleanup boards board.cmake files and replace it by include(${ZEPHYR_BASE}/boards/st/common/board.cmake)

@nordicjm
Copy link
Collaborator

The real solution should be that flashing never erases the full flash of the device (just the sectors being programmed) unless the --erase flash is provided to west flash. This already is the assumption of flash runners and will be going forward with things like sysbuild, so if there are oddities like this where it does an erase on its own, it is likely to break

@erwango erwango removed the Good first issue Good for a first time contributor to take label Feb 28, 2024
@erwango
Copy link
Member Author

erwango commented Feb 28, 2024

The real solution should be that flashing never erases the full flash of the device (just the sectors being programmed) unless the --erase flash is provided to west flash. This already is the assumption of flash runners and will be going forward with things like sysbuild, so if there are oddities like this where it does an erase on its own, it is likely to break

Makes sense. The issue is that erase is required in CI, at least in specific tests. So we need to check how to do this first.

@nordicjm
Copy link
Collaborator

@PerMac was asking about this recently and my opinion it's wrong for any test to not do a full erase, it contaminates the test with undefined data. If you have 2 tests, one writes something and another checks for data in a certain area and there's no erase in-between, that test case is not valid because you cannot be sure the previous test isn't giving a false positive/negative

@erwango
Copy link
Member Author

erwango commented Feb 29, 2024

my opinion it's wrong for any test to not do a full erase

We're in line. This is why we have these commands in the first place. So before removing the default "--erase" in boards.cmake, we need to find how to force them with twister. This wasn't possible when we looked at it, but potentially this is now possible

@nordicjm
Copy link
Collaborator

@nashif @PerMac please see the above

@erwango erwango changed the title boards: st: Use "--erase" only if CONFIG_BOOTLOADER_MCUBOOT=n boards: st: Minimize usage of "--erase" stm32cubeprogrammer option Feb 29, 2024
@PerMac
Copy link
Member

PerMac commented Feb 29, 2024

We run twister in CI with extra: --west-flash="--erase" to achieve clean board. This tells twister to call west for flashing and pass there --erase arg. I believe the arg to pass depends on the flashing tool you use. However, this won't work with susbuild since it is currently broken #50421 and the arg is used for every consecutive flash within the same "system", hence breaking stuff like bootloader tests with multiple images flashed in a sequence

@PerMac
Copy link
Member

PerMac commented Feb 29, 2024

I was thinking that maybe marking only tests that require an erase in their yamls might be the way to go, so that stuff like mcuboot or sysbuild are not screwed in a common run, but I also understand the reasoning behind getting the clean state every time. Then sysbuild is to be fixed first and --west-flash="your_erase_options" might work

AlexFabre added a commit to AlexFabre/zephyr that referenced this issue Jun 20, 2024
A `west flash` with STLink must not implicitly perform a mass erase on its own.

If a flash erase is required, the option has to be passed manually.

zephyrproject-rtos#69582
AlexFabre added a commit to AlexFabre/zephyr that referenced this issue Jun 20, 2024
A `west flash` with STLink must not implicitly perform a mass erase on its own.

If a flash erase is required, the option has to be passed manually.

zephyrproject-rtos#69582
AlexFabre added a commit to AlexFabre/zephyr that referenced this issue Jun 20, 2024
A `west flash` with STLink must not implicitly perform a mass erase on its own.

If a flash erase is required, the option has to be passed manually.

zephyrproject-rtos#69582
AlexFabre added a commit to AlexFabre/zephyr that referenced this issue Jun 20, 2024
A `west flash` with STLink must not implicitly perform a mass erase on its own.

If a flash erase is required, the option has to be passed manually.

zephyrproject-rtos#69582
AlexFabre added a commit to AlexFabre/zephyr that referenced this issue Jun 20, 2024
Problem:

When flashing a multi-image project with STLink through sysbuild,
the flash utility is told to erase the whole flash between each
single image flash.

Resulting in a partial flash where only the last image is effectively
stored on flash...

Correction:

A `west flash` must not implicitly perform a mass erase on its own.

If a flash erase is required, the option has to be passed manually.

zephyrproject-rtos#69582
AlexFabre added a commit to AlexFabre/zephyr that referenced this issue Jun 20, 2024
Problem:

When flashing a multi-image project with STLink through sysbuild,
the flash utility is told to erase the whole flash between each
single image flash.

Resulting in a partial flash where only the last image is effectively
stored on flash...

Correction:

A `west flash` must not implicitly perform a mass erase on its own.

If a flash erase is required, the option has to be passed manually.

The problem is discussed in the following issue:
zephyrproject-rtos#69582

*note: This is my first contribution to the project*

Signed-off-by: Alex Fabre <[email protected]>
AlexFabre added a commit to AlexFabre/zephyr that referenced this issue Jun 20, 2024
Problem:

When flashing a multi-image project with STLink through sysbuild,
the flash utility is told to erase the whole flash between each
single image flash.

Resulting in a partial flash where only the last image is effectively
stored on flash...

Correction:

A `west flash` must not implicitly perform a mass erase on its own.

If a flash erase is required, the option has to be passed manually.

The problem is discussed in the following issue:
zephyrproject-rtos#69582

*note: This is my first contribution to the project*

Signed-off-by: Alex Fabre <[email protected]>
AlexFabre added a commit to AlexFabre/zephyr that referenced this issue Jun 24, 2024
Problem:

When flashing a multi-image project with STLink through sysbuild,
the flash utility is told to erase the whole flash between each
single image flash.

Resulting in a partial flash where only the last image is effectively
stored on flash...

Correction:

A `west flash` must not implicitly perform a mass erase on its own.

If a flash erase is required, the option has to be passed manually.

The problem is discussed in the following issue:
zephyrproject-rtos#69582

*note: This is my first contribution to the project*

Signed-off-by: Alex Fabre <[email protected]>
AlexFabre added a commit to AlexFabre/zephyr that referenced this issue Jun 24, 2024
Problem:

When flashing a multi-image project with STLink through sysbuild,
the flash utility is told to erase the whole flash between each
single image flash.

Resulting in a partial flash where only the last image is effectively
stored on flash...

Correction:

A `west flash` must not implicitly perform a mass erase on its own.

If a flash erase is required, the option has to be passed manually.

The problem is discussed in the following issue:
zephyrproject-rtos#69582

*note: This is my first contribution to the project*

Signed-off-by: Alex Fabre <[email protected]>
@erwango
Copy link
Member Author

erwango commented Jun 25, 2024

This tells twister to call west for flashing and pass there --erase arg. I believe the arg to pass depends on the flashing tool you use.

@PerMac About this, I've been trying to use --west-flash option and at first sight it doesn't play well with twister's hardware-map option which we're using extensively on our bench. Is this a known issue or maybe I'm doing something wrong ?

Also, it seems that it all depends on runner supporting --erase, which isn't the case for openocd, but which should work for stm32cubeprogrammer which is easy to switch to (if hardware-map is supported, otherwise it will require to update impacted boards upstream configurations)

@PerMac
Copy link
Member

PerMac commented Jun 25, 2024

@erwango we also use hardware-map and --west-flash=--erase in the call and it works. Not sure what you mean. You can play with max verbosity (-v -v) and see what west command is used for flashing. I add --west-flash="--erase" to the CLI call of twister and use a hardware map. If I have runner: jlink in hw map I see west calling
Flash command: ['west', 'flash', '--skip-rebuild', '-d', '/home/maciej/NCS-bis/nrf/twister-out/nrf9160dk_nrf9160/samples/hello_world/sample.basic.helloworld', '--runner', 'jlink', '--tool-opt=-SelectEmuBySN 000960033217', '--', '--erase']

using the same twister's command but having runner: nrfjprog in hw map I get:
Flash command: ['west', 'flash', '--skip-rebuild', '-d', '/home/maciej/NCS-bis/nrf/twister-out/nrf9160dk_nrf9160/samples/hello_world/sample.basic.helloworld', '--runner', 'nrfjprog', '--', '--erase', '--dev-id', '000960033217']
To me it looks as expected. You can use hw map where you specify a runner and twister will pass this info in addition to west, along with value you passed with --west-flash

@erwango
Copy link
Member Author

erwango commented Jun 25, 2024

@PerMac Ok, it seems the only issue was coming from openocd not supporting --erase command. Using runner stm32cubeprogrammer, this is all fine. Thanks!

AlexFabre added a commit to AlexFabre/zephyr that referenced this issue Jun 28, 2024
Problem:

When flashing a multi-image project with STLink through sysbuild,
the flash utility is told to erase the whole flash between each
single image flash.

Resulting in a partial flash where only the last image is effectively
stored on flash...

Correction:

A `west flash` must not implicitly perform a mass erase on its own.

If a flash erase is required, the option has to be passed manually.

The problem is discussed in the following issue:
zephyrproject-rtos#69582

*note: This is my first contribution to the project*

Signed-off-by: Alex Fabre <[email protected]>
AlexFabre added a commit to AlexFabre/zephyr that referenced this issue Jun 28, 2024
Problem:

When flashing a multi-image project with STLink through sysbuild,
the flash utility is told to erase the whole flash between each
single image flash.

Resulting in a partial flash where only the last image is effectively
stored on flash...

Correction:

A `west flash` must not implicitly perform a mass erase on its own.

If a flash erase is required, the option has to be passed manually.

The problem is discussed in the following issue:
zephyrproject-rtos#69582

Due to CI tests errors, the correction is not applied on
eval board `b_u585i_iot02a`.

See following issue:
zephyrproject-rtos#75164

Signed-off-by: Alex Fabre <[email protected]>
nashif pushed a commit that referenced this issue Jul 1, 2024
Problem:

When flashing a multi-image project with STLink through sysbuild,
the flash utility is told to erase the whole flash between each
single image flash.

Resulting in a partial flash where only the last image is effectively
stored on flash...

Correction:

A `west flash` must not implicitly perform a mass erase on its own.

If a flash erase is required, the option has to be passed manually.

The problem is discussed in the following issue:
#69582

Due to CI tests errors, the correction is not applied on
eval board `b_u585i_iot02a`.

See following issue:
#75164

Signed-off-by: Alex Fabre <[email protected]>
coreboot-org-bot pushed a commit to coreboot/zephyr-cros that referenced this issue Jul 5, 2024
Problem:

When flashing a multi-image project with STLink through sysbuild,
the flash utility is told to erase the whole flash between each
single image flash.

Resulting in a partial flash where only the last image is effectively
stored on flash...

Correction:

A `west flash` must not implicitly perform a mass erase on its own.

If a flash erase is required, the option has to be passed manually.

The problem is discussed in the following issue:
zephyrproject-rtos/zephyr#69582

Due to CI tests errors, the correction is not applied on
eval board `b_u585i_iot02a`.

See following issue:
zephyrproject-rtos/zephyr#75164

(cherry picked from commit 9b9d455)

Original-Signed-off-by: Alex Fabre <[email protected]>
GitOrigin-RevId: 9b9d455
Change-Id: I255ff9585942b264d0cd87092e5bf6890c96bd75
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/zephyr/+/5672378
Reviewed-by: Fabio Baltieri <[email protected]>
Tested-by: ChromeOS Prod (Robot) <[email protected]>
Reviewed-by: Eric Yilun Lin <[email protected]>
Commit-Queue: Fabio Baltieri <[email protected]>
CZKikin pushed a commit to nxp-upstream/zephyr that referenced this issue Jul 18, 2024
Problem:

When flashing a multi-image project with STLink through sysbuild,
the flash utility is told to erase the whole flash between each
single image flash.

Resulting in a partial flash where only the last image is effectively
stored on flash...

Correction:

A `west flash` must not implicitly perform a mass erase on its own.

If a flash erase is required, the option has to be passed manually.

The problem is discussed in the following issue:
zephyrproject-rtos#69582

Due to CI tests errors, the correction is not applied on
eval board `b_u585i_iot02a`.

See following issue:
zephyrproject-rtos#75164

Signed-off-by: Alex Fabre <[email protected]>
Devansh0210 pushed a commit to Devansh0210/zephyr that referenced this issue Jul 20, 2024
Problem:

When flashing a multi-image project with STLink through sysbuild,
the flash utility is told to erase the whole flash between each
single image flash.

Resulting in a partial flash where only the last image is effectively
stored on flash...

Correction:

A `west flash` must not implicitly perform a mass erase on its own.

If a flash erase is required, the option has to be passed manually.

The problem is discussed in the following issue:
zephyrproject-rtos#69582

Due to CI tests errors, the correction is not applied on
eval board `b_u585i_iot02a`.

See following issue:
zephyrproject-rtos#75164

Signed-off-by: Alex Fabre <[email protected]>
DashingR pushed a commit to tsisw/zephyr that referenced this issue Aug 15, 2024
Problem:

When flashing a multi-image project with STLink through sysbuild,
the flash utility is told to erase the whole flash between each
single image flash.

Resulting in a partial flash where only the last image is effectively
stored on flash...

Correction:

A `west flash` must not implicitly perform a mass erase on its own.

If a flash erase is required, the option has to be passed manually.

The problem is discussed in the following issue:
zephyrproject-rtos#69582

Due to CI tests errors, the correction is not applied on
eval board `b_u585i_iot02a`.

See following issue:
zephyrproject-rtos#75164

Signed-off-by: Alex Fabre <[email protected]>
@erwango
Copy link
Member Author

erwango commented Aug 22, 2024

Fixed: #74627

@erwango erwango closed this as completed Aug 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement Changes/Updates/Additions to existing features platform: STM32 ST Micro STM32
Projects
None yet
Development

No branches or pull requests

3 participants