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/stm32f4: add riotboot requirements #11682

Merged
merged 3 commits into from
Aug 8, 2019

Conversation

fjmolinas
Copy link
Contributor

@fjmolinas fjmolinas commented Jun 12, 2019

Contribution description

This PR adds riotboot for stm32f4. Since F4 works with sectors for riotboot to work each slot must start at the beginning of a slotsector. For stm32f4 the smallest slotsector 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 it is highly related to:

Testing procedure

It has been tested on the supported boards: nucleo-f446re and stm32f429i-disc1.

1.- Test riotboot:

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

BOARD=nucleo-f446rFEATURES_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 reba se on top off flashpage: #11681 and #11683

and run tests/riotboot_flashwrite

The following patch can be applied to the test Makefile

diff --git a/tests/riotboot_flashwrite/Makefile b/tests/riotboot_flashwrite/Makefile
index 32f127d70..d3c9f20b4 100644
--- a/tests/riotboot_flashwrite/Makefile
+++ b/tests/riotboot_flashwrite/Makefile
@@ -39,11 +39,17 @@ LOW_MEMORY_BOARDS := nucleo-f334r8
 GNRC_NETIF_NUMOF := 2
 
 # uncomment these to use ethos
-#USEMODULE += ethos gnrc_uhcpc
+USEMODULE += ethos gnrc_uhcpc  stdio_uart_rx
 #
 ## ethos baudrate can be configured from make command
-#ETHOS_BAUDRATE ?= 115200
-#CFLAGS += -DETHOS_BAUDRATE=$(ETHOS_BAUDRATE) -DUSE_ETHOS_FOR_STDIO
+ETHOS_BAUDRATE ?= 115200
+CFLAGS += -DETHOS_BAUDRATE=$(ETHOS_BAUDRATE) -DUSE_ETHOS_FOR_STDIO
+
+TAP ?= tap0
+IPV6_PREFIX ?= 2001:db8::/64
+
+TERMPROG ?= sudo sh $(RIOTTOOLS)/ethos/start_network.sh
+TERMFLAGS ?= $(PORT) $(TAP) $(IPV6_PREFIX)
 
 ifneq (,$(filter $(BOARD),$(LOW_MEMORY_BOARDS)))
   $(info Using low-memory configuration for microcoap_server.)

Then to test:

  • provided the node make -C tests/riotboot_flashwrite/ BOARD=stm32f429i-disc1 riotboot/flash term
  • compile new firmware: make -C tests/riotboot_flashwrite/ BOARD=stm32f429i-disc1 riotboot
  • launch an updated: coap-client -m post coap://[fe80::2%tap0]/flashwrite -f tests/riotboot_flashwrite/bin/stm32f429i-disc1/tests_riotboot_flashwrite-slot1.riot.bin -b 64

When the update finished reboot the node manually it should have started from a different slot.

Issues/PRs references

Related to #11681 and #11683

@fjmolinas fjmolinas added Type: new feature The issue requests / The PR implemements a new feature for RIOT Area: OTA Area: Over-the-air updates labels Jun 12, 2019
@fjmolinas fjmolinas changed the title Pr stm32f4 riotboot cpu/stm32f4: add riotboot requirements Jun 12, 2019
@fjmolinas fjmolinas requested a review from aabadie June 18, 2019 08:01
@aabadie
Copy link
Contributor

aabadie commented Jun 19, 2019

Since F4 works with sectors for riotboot to work each slot must start at the beginning of a slot. For stm32f4 the smallest slot is of length 16kB

It seems there is a confusion between slots and sectors in this sentence. From my understanding, it should be: Since F4 works with sectors, for riotboot to work, each slot must start at the beginning of a sector. For stm32f4 the smallest sector is of length 16kB...

This PR also needs a rebase.

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.

I have comments on comments.

@fjmolinas fjmolinas force-pushed the pr_stm32f4_riotboot branch from cc16e33 to 275d566 Compare June 19, 2019 07:19
@fjmolinas
Copy link
Contributor Author

@aabadie comments addressed.

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.

Code changes are good and well explained. I tested on nucleo-f446re and on nucleo-f410rb (even if the feature is not added on this one) and it worked on both.

ACK

@aabadie
Copy link
Contributor

aabadie commented Aug 7, 2019

Please squash and trigger Murdock after.

@fjmolinas fjmolinas force-pushed the pr_stm32f4_riotboot branch 2 times, most recently from b702243 to cf6fb12 Compare August 7, 2019 16:28
- Stm32f4 use sectors instead of pages. They go from 16 KB to 128KB.
  The bootloader will use the first sector(16Kb). Slots must start
  at the begining of a sector to not overlap.
- Minimum required RIOBOOT_HDR_LEN or stm32f4 is 0x200
  to respect vector table alignment
@fjmolinas fjmolinas force-pushed the pr_stm32f4_riotboot branch from cf6fb12 to bfcb963 Compare August 7, 2019 16:29
@fjmolinas
Copy link
Contributor Author

@aabadie I squashed and modified the commit message, there where some typos. I also removed:

SLOT0_OFFSET ?= $(RIOTBOOT_LEN)	
SLOT1_OFFSET ?= $(shell echo $$(($(SLOT0_OFFSET)+ $(RIOTBOOT_LEN) + $(SLOT0_LEN))))

Which was already defined for riotboot.

@fjmolinas fjmolinas added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Aug 7, 2019
@aabadie
Copy link
Contributor

aabadie commented Aug 7, 2019

Ok, I'll have a second look tomorrow, if I find time, just in case.

@aabadie
Copy link
Contributor

aabadie commented Aug 8, 2019

I tested a second time this PR and it works. Let's go!

@aabadie aabadie merged commit f802bbb into RIOT-OS:master Aug 8, 2019
@fjmolinas
Copy link
Contributor Author

@aabadie Thanks for the review!

@fjmolinas fjmolinas deleted the pr_stm32f4_riotboot branch August 9, 2019 06:57
@kb2ma kb2ma added this to the Release 2019.10 milestone Sep 16, 2019
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 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.

3 participants