cpu/stm32: fix riotboot settings for L4 and WB by gschorcht · Pull Request #19618 · RIOT-OS/RIOT

gschorcht · 2023-05-19T00:51:26Z

Contribution description

This PR fixes the riotboot configuration for L4 and WB.

The family is not called stm32l4 or stm32wb but l4 and wb. That is, the riotboot configuration didn't work at all. Furthermore, a minimum RIOTBOOT_LEN of 0x2000 is required for L4.

Found when investigating the compilation errors for bootloaders/riotboot_serial in PR #19576.

Testing procedure

Green CI.

Use the following commands:

BOARD=nucleo-l496zg make -C tests/riotboot info-debug-variable-RIOTBOOT_HDR_LEN
BOARD=p-nucleo-wb55 make -C tests/riotboot info-debug-variable-RIOTBOOT_HDR_LEN

In master these commands give

0x400

With this PR these commands give

0x200

as expected.

Use the following commands:

BOARD=nucleo-l496zg make -C tests/riotboot info-debug-variable-RIOTBOOT_LEN
BOARD=p-nucleo-wb55 make -C tests/riotboot info-debug-variable-RIOTBOOT_LEN

In master these commands give

0x1000

With this PR these commands give

0x2000

as expected.

Issues/PRs references

riot-ci · 2023-05-19T04:20:06Z

Murdock results

✔️ PASSED

e6c1aec cpu/stm32: fix riotboot settings for L4 and WB

Success	Failures	Total	Runtime
6945	0	6946	10m:35s

Artifacts

Documentation preview

aabadie · 2023-05-22T10:30:55Z

I tested tests/riotboot with this PR on nucleo-l476rg and p-l496g-cell02: it works for both. But it also works on master so maybe I'm not testing the right thing ?

cpu/stm32/stm32_riotboot.mk

gschorcht · 2023-05-22T15:06:46Z

I tested tests/riotboot with this PR on nucleo-l476rg and p-l496g-cell02: it works for both. But it also works on master so maybe I'm not testing the right thing ?

It works since it used the default value of 0x400 for RIOTBOOT_HDR_LEN set in cpu/cortex_common/Makefile.include. You can use

BOARD=nucleo-l496zg make -C tests/riotboot info-debug-variable-RIOTBOOT_HDR_LEN

to verify.

gschorcht · 2023-05-22T15:23:59Z

I have changed as suggested and squashed it directly.

To check you should use the following commands:

BOARD=nucleo-l496zg make -C tests/riotboot info-debug-variable-RIOTBOOT_HDR_LEN
BOARD=p-nucleo-wb55 make -C tests/riotboot info-debug-variable-RIOTBOOT_HDR_LEN
BOARD=nucleo-l496zg make -C tests/riotboot info-debug-variable-RIOTBOOT_LEN
BOARD=p-nucleo-wb55 make -C tests/riotboot info-debug-variable-RIOTBOOT_LEN

You should observe the wrong values are given in master.

aabadie

On L4 the number of IRQs varies from 83 (L412) to 95 (L4R5 and the like). So 0x200 is enough in all cases.

I tested the values are correct following #19618 (comment)

ACK

aabadie · 2023-05-23T07:56:36Z

bors merge

aabadie · 2023-05-23T07:58:23Z

bors cancel

bors · 2023-05-23T07:58:26Z

Canceled.

aabadie · 2023-05-23T08:07:16Z

bors merge

19602: dist/tools/compile_commands: add another workaround r=chrysn a=maribu ### Contribution description Filter out GCC only `--param=min-pagesize=0` in `clangd` mode. This fixes compilation of rust applications, that now fails with: thread 'main' panicked at 'Unable to generate bindings: ClangDiagnostic("error: argument unused during compilation: '--param=min-pagesize=0' [-Wunused-command-line-argument]\n")', /home/maribu/.cargo/git/checkouts/rust-riot-sys-d12733b89271907c/b4bd4bd/build.rs:224:10 19618: cpu/stm32: fix riotboot settings for L4 and WB r=aabadie a=gschorcht ### Contribution description This PR fixes the `riotboot` configuration for L4 and WB. The family is not called `stm32l4` or `stm32wb` but `l4` and `wb`. That is, the `riotboot` configuration didn't work at all. Furthermore, a minimum `RIOTBOOT_LEN` of `0x2000` is required for L4. Found when investigating the compilation errors for `bootloaders/riotboot_serial` in PR #19576. ### Testing procedure 1. Green CI. 2. Use the following commands: ``` BOARD=nucleo-l496zg make -C tests/riotboot info-debug-variable-RIOTBOOT_HDR_LEN BOARD=p-nucleo-wb55 make -C tests/riotboot info-debug-variable-RIOTBOOT_HDR_LEN ``` In master these commands give ``` 0x400 ``` With this PR these commands give ``` 0x200 ``` as expected. 3. Use the following commands: ``` BOARD=nucleo-l496zg make -C tests/riotboot info-debug-variable-RIOTBOOT_LEN BOARD=p-nucleo-wb55 make -C tests/riotboot info-debug-variable-RIOTBOOT_LEN ``` In master these commands give ``` 0x1000 ``` With this PR these commands give ``` 0x2000 ``` as expected. ### Issues/PRs references 19643: examples/suit_update: some test fixes r=aabadie a=kaspar030 Co-authored-by: Marian Buschsieweke <[email protected]> Co-authored-by: Gunar Schorcht <[email protected]> Co-authored-by: Kaspar Schleiser <[email protected]>

bors · 2023-05-23T10:44:34Z

Build failed (retrying...):

Murdock

19618: cpu/stm32: fix riotboot settings for L4 and WB r=aabadie a=gschorcht ### Contribution description This PR fixes the `riotboot` configuration for L4 and WB. The family is not called `stm32l4` or `stm32wb` but `l4` and `wb`. That is, the `riotboot` configuration didn't work at all. Furthermore, a minimum `RIOTBOOT_LEN` of `0x2000` is required for L4. Found when investigating the compilation errors for `bootloaders/riotboot_serial` in PR #19576. ### Testing procedure 1. Green CI. 2. Use the following commands: ``` BOARD=nucleo-l496zg make -C tests/riotboot info-debug-variable-RIOTBOOT_HDR_LEN BOARD=p-nucleo-wb55 make -C tests/riotboot info-debug-variable-RIOTBOOT_HDR_LEN ``` In master these commands give ``` 0x400 ``` With this PR these commands give ``` 0x200 ``` as expected. 3. Use the following commands: ``` BOARD=nucleo-l496zg make -C tests/riotboot info-debug-variable-RIOTBOOT_LEN BOARD=p-nucleo-wb55 make -C tests/riotboot info-debug-variable-RIOTBOOT_LEN ``` In master these commands give ``` 0x1000 ``` With this PR these commands give ``` 0x2000 ``` as expected. ### Issues/PRs references 19643: examples/suit_update: some test fixes r=aabadie a=kaspar030 Co-authored-by: Gunar Schorcht <[email protected]> Co-authored-by: Kaspar Schleiser <[email protected]>

bors · 2023-05-23T11:50:37Z

Build failed (retrying...):

Murdock

aabadie · 2023-05-23T12:09:33Z

Hmm, Murdock spotted a problem with nucleo-l4r5zi

gschorcht · 2023-05-23T14:13:01Z

Hmm, Murdock spotted a problem with nucleo-l4r5zi

Due to the bug, RIOTBOOT_LEN wasn't set to 0x2000 here

RIOT/cpu/stm32/stm32_riotboot.mk

Line 44 in 8408d69

RIOTBOOT_LEN ?= 0x2000

but in cpu/cortexm_common:

RIOT/cpu/cortexm_common/Makefile.include

Lines 73 to 77 in 8408d69

    
           ifneq (,$(filter usbus_dfu tinyusb_dfu,$(USEMODULE))) 
        
             RIOTBOOT_LEN ?= 0x4000 
        
           else 
        
             RIOTBOOT_LEN ?= 0x1000 
        
           endif

Therefore, RIOTBOOT_LEN was set to 0x4000 in case usbus_dfu or tinyusb_dfu are used. It seems that we also need to deal with this case for L4 and WB.

19618: cpu/stm32: fix riotboot settings for L4 and WB r=aabadie a=gschorcht ### Contribution description This PR fixes the `riotboot` configuration for L4 and WB. The family is not called `stm32l4` or `stm32wb` but `l4` and `wb`. That is, the `riotboot` configuration didn't work at all. Furthermore, a minimum `RIOTBOOT_LEN` of `0x2000` is required for L4. Found when investigating the compilation errors for `bootloaders/riotboot_serial` in PR #19576. ### Testing procedure 1. Green CI. 2. Use the following commands: ``` BOARD=nucleo-l496zg make -C tests/riotboot info-debug-variable-RIOTBOOT_HDR_LEN BOARD=p-nucleo-wb55 make -C tests/riotboot info-debug-variable-RIOTBOOT_HDR_LEN ``` In master these commands give ``` 0x400 ``` With this PR these commands give ``` 0x200 ``` as expected. 3. Use the following commands: ``` BOARD=nucleo-l496zg make -C tests/riotboot info-debug-variable-RIOTBOOT_LEN BOARD=p-nucleo-wb55 make -C tests/riotboot info-debug-variable-RIOTBOOT_LEN ``` In master these commands give ``` 0x1000 ``` With this PR these commands give ``` 0x2000 ``` as expected. ### Issues/PRs references 19636: sys: model ecc, evtimer, pipe and shell_lock in kconfig r=aabadie a=aabadie 19639: tests/net/gnrc_mac_timeout: add automated test r=aabadie a=aabadie Co-authored-by: Gunar Schorcht <[email protected]> Co-authored-by: Alexandre Abadie <[email protected]>

aabadie · 2023-05-23T14:43:53Z

bors cancel

bors · 2023-05-23T14:43:55Z

Canceled.

gschorcht · 2023-05-23T15:20:11Z

Therefore, RIOTBOOT_LEN was set to 0x4000 in case usbus_dfu or tinyusb_dfu are used. It seems that we also need to deal with this case for L4 and WB.

However, I wonder why we should increase RIOTBOOT_LEN to 8 KByte for all WB and L4 boards, when it worked with the bug even with only 4 kByte, especially for MCUs with only 64 kByte or 128 kByte of flash. The problem appeared only with stm32l496g-disco, because it must use additionally the module periph_lpuart and 4 KByte as RIOTBOOT_LEN are just 200 byte too little.

On the other hand, I'm not sure whether it would make sense to define RIOTBOOT_LEN on board level.

gschorcht · 2023-05-23T15:26:32Z

If I'm right, for WB family a RIOTBOOT_LEN of 0x1000 is definitely wrong since WB MCUs have a flash page size of 4 kByte so that the minimum RIOTBOOT_LEN has to 16 kByte to get a multiple of 4 kBytes when dividing total_flash_size - RIOTBOOT_LEN by 2.

The family is not called `stm32l4` or `stm32wb` but `l4` and `wb`. That is, the `riotboot` configuration didn't work. A minimum `RIOTBOOT_LEN` of `0x2000` is required for WB.

gschorcht · 2023-05-23T16:18:39Z

I have changed it back for L4 so that RIOTBOOT_LEN is set to the default values as defined in cpu/cortexm_common/Makefile.include. I fixed RIOTBOOT_LEN for WB. The problem with stm32l496g-disco is solved on board level.

benpicco · 2023-05-23T17:12:44Z

bors merge

bors · 2023-05-23T23:08:21Z

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

gschorcht · 2023-05-25T15:22:44Z

Thanks for reviewing and merging 😄

gschorcht requested review from DipSwitch, aabadie, fjmolinas and vincent-d as code owners May 19, 2023 00:51

github-actions bot added Area: build system Area: Build system Area: cpu Area: CPU/MCU ports Platform: ARM Platform: This PR/issue effects ARM-based platforms labels May 19, 2023

gschorcht added Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels May 19, 2023

This was referenced May 21, 2023

boards: add stm32l496g-disco support #19576

Merged

sys/riotboot/flashwrite: unaligned write when skipping RIOTBOOT_MAGIC on stm32wb #15917

Closed

aabadie reviewed May 22, 2023

View reviewed changes

cpu/stm32/stm32_riotboot.mk Show resolved Hide resolved

aabadie approved these changes May 23, 2023

View reviewed changes

cpu/stm32: fix riotboot settings for L4 and WB

e6c1aec

The family is not called `stm32l4` or `stm32wb` but `l4` and `wb`. That is, the `riotboot` configuration didn't work. A minimum `RIOTBOOT_LEN` of `0x2000` is required for WB.

gschorcht force-pushed the cpu/stm32/fix_riotboot_settings branch from 2b96630 to e6c1aec Compare May 23, 2023 15:47

bors bot merged commit a272abb into RIOT-OS:master May 23, 2023

gschorcht deleted the cpu/stm32/fix_riotboot_settings branch May 25, 2023 15:23

benpicco added this to the Release 2023.07 milestone Aug 2, 2023

Conversation

gschorcht commented May 19, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Contribution description

Testing procedure

Issues/PRs references

Uh oh!

riot-ci commented May 19, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Murdock results

Artifacts

Uh oh!

aabadie commented May 22, 2023

Uh oh!

Uh oh!

gschorcht commented May 22, 2023

Uh oh!

gschorcht commented May 22, 2023

Uh oh!

aabadie left a comment

Choose a reason for hiding this comment

Uh oh!

aabadie commented May 23, 2023

Uh oh!

aabadie commented May 23, 2023

Uh oh!

bors bot commented May 23, 2023

Uh oh!

aabadie commented May 23, 2023

Uh oh!

bors bot commented May 23, 2023

Uh oh!

bors bot commented May 23, 2023

Uh oh!

aabadie commented May 23, 2023

Uh oh!

gschorcht commented May 23, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

aabadie commented May 23, 2023

Uh oh!

bors bot commented May 23, 2023

Uh oh!

gschorcht commented May 23, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gschorcht commented May 23, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gschorcht commented May 23, 2023

Uh oh!

benpicco commented May 23, 2023

Uh oh!

bors bot commented May 23, 2023

Uh oh!

gschorcht commented May 25, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

gschorcht commented May 19, 2023 •

edited

Loading

riot-ci commented May 19, 2023 •

edited

Loading

gschorcht commented May 23, 2023 •

edited

Loading

gschorcht commented May 23, 2023 •

edited

Loading

gschorcht commented May 23, 2023 •

edited

Loading