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

cpu/stm32f2: add riotboot requirements #11900

Merged
merged 2 commits into from Aug 9, 2019

Conversation

fjmolinas
Copy link
Contributor

Contribution description

This PR adds riotboot for stm32f2. Since F2 works with sectors for riotboot to work each slot must start at the beginning of a sector. For stm32f2 the smallest sector is of length 16kB therefore that must be the length of the bootloader and the same amount must be taken from the second half of the flash to optimize flash usage.

This PR is part of 3, since just adding riotboot doesn't make much sense if it can't perform ota. Although it doesn't depend on them but it is highly related to:

Also pulling #11808 can ease testing significantly.

Testing procedure

It has been tested on the supported boards: nucleo-f207zg

1.- Test riotboot:

BOARD=nucleo-f207zg FEATURES_REQUIRED+=riotboot make -C tests/xtimer_usleep riotboot/flash-extended-slot0 term

BOARD=nucleo-f207zg FEATURES_REQUIRED+=riotboot make -C tests/xtimer_usleep riotboot/flash-slot1 term

In both cases it should run without issues.

2.- Since the sector division is important when updating the slots by writing to flash directly it would be ideal to rebase on top off flashpage: #11681 and #11705, as well as #11808

and run tests/riotboot_flashwrite

The following patch can be applied to the test Makefile

make -C tests/riotboot_flashwrite/ BOARD=nucleo-f207zg flash test

Issues/PRs references

@fjmolinas fjmolinas added Platform: ARM Platform: This PR/issue effects ARM-based platforms Type: new feature The issue requests / The PR implemements a new feature for RIOT Area: OTA Area: Over-the-air updates labels Jul 24, 2019
@fjmolinas fjmolinas requested a review from aabadie July 24, 2019 08:52
@fjmolinas fjmolinas force-pushed the pr_stm32f2_riotboot_requirements branch from f2b5b93 to 5edfd0c Compare August 3, 2019 19:46
Copy link
Contributor

@aabadie aabadie left a comment

Choose a reason for hiding this comment

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

The code changes look good but my tests were not successful. The first part of the testing procedure worked, I didn't tried the second part and then I just tried tests/riotboot and noticed that after flashing slot1, the active slot was still slot0.

@fjmolinas
Copy link
Contributor Author

@aabadie I know whats the problem with this. The code is good, the problem is two-fold:

  • sometimes openocd fails probe the correct start of the flash and attempts to flash at 0x000000 which results in an error
  • Secondly tests/ritoboot was changed so APP_VER is 0 so when you call PORT=/dev/ttyACM0 make -C tests/riotboot BOARD=nucleo-f207zg riotboot/flash-slot1 APP_VER doesn't change so it fails to switch slots, if you do this it works:

APP_VER=1 PORT=/dev/ttyACM0 make -C tests/riotboot BOARD=nucleo-f207zg riotboot/flash-slot0
APP_VER=2 PORT=/dev/ttyACM0 make -C tests/riotboot BOARD=nucleo-f207zg riotboot/flash-slot1

I will look into the probing problem, but it seems unrelated to the PR.

- stm32f2 uses sectors instead of pages, they go from 16KB to
  128KB. Smaller sectors are at the begining of the flash. Slots
  must start at the begining of a sector to not overlap.
- Minimum required RIOBOOT_HDR_LEN or stm32f2 is 0x200
  to respect vector table alignment
@fjmolinas fjmolinas force-pushed the pr_stm32f2_riotboot_requirements branch from 5edfd0c to 7231088 Compare August 7, 2019 16:35
@aabadie aabadie added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Aug 9, 2019
Copy link
Contributor

@aabadie aabadie left a comment

Choose a reason for hiding this comment

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

Changes are good and well explained. I could retest this PR on nucleo-f207zg following the discussion and confirm this is working. Good job!

ACK

@aabadie aabadie merged commit 6f14de3 into RIOT-OS:master Aug 9, 2019
@kb2ma kb2ma added this to the Release 2019.10 milestone Sep 16, 2019
@fjmolinas fjmolinas deleted the pr_stm32f2_riotboot_requirements branch July 31, 2020 07:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: OTA Area: Over-the-air updates CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Platform: ARM Platform: This PR/issue effects ARM-based platforms Type: new feature The issue requests / The PR implemements a new feature for RIOT
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants