boards/frdm-kw41z-k64f: add riotboot #11562
Conversation
fjmolinas
added
State: WIP
|
Currently I have issue with the test output: The board should be whitelisted here: And the flash fails too with And currently, |
Oh... sorry I forgot that riotboot changed and know you need to
The command is |
|
Somehow I think handling the https://cache.freescale.com/files/microcontrollers/doc/app_note/AN4507.pdf For me it seems like this And we could just say "We do not want to try flashing a binary if it does not start at 0 or is over Even if the |
It is supposed to work with # tests/riotboot/Makefile
# Target 'all' will generate the combined file directly.
# It also makes 'flash' and 'flash-only' work without specific command.
FLASHFILE = $(RIOTBOOT_COMBINED_BIN)To make it work in the current version, I also needed to export diff --git a/sys/riotboot/Makefile.include b/sys/riotboot/Makefile.include
index 4e2d9b6c6..607a1fe1d 100644
--- a/sys/riotboot/Makefile.include
+++ b/sys/riotboot/Makefile.include
@@ -1,7 +1,7 @@
# Indicate the reserved space for a header, 256B by default
# Notice that it must be 256B aligned. This is restricted by
# the Cortex-M0+/3/4/7 architecture
-RIOTBOOT_HDR_LEN ?= 0x100
+export RIOTBOOT_HDR_LEN ?= 0x100
# By default, slot 0 is found just after RIOTBOOT_LEN. Slot 1 after
# slot 0. The values might be overridden to add more or less offsetAnd then the test succeed !!!! 👍 otherwise I got |
Yes sorry, I had rebased my branch to remove a couple of uneeded commits and removed that. I had realized that but was trying to address all your comments before pushing updates.
Yes I think you are right, I'll try to implement! Thanks for the comments! |
No worry, I play stupid first, execute the command, post the error, and then try to understand :) It's working already, so it's good :) |
fjmolinas
force-pushed
the
pr_kinetis_riotboot_frdm_kw41z
branch
from
bcc4d23 to
ea9b013
Compare
This is true although it would still mean having two linker scripts: one when at |
No I would keep using the same linkerscript for simplicity. I was referring more to checking the |
|
Could you split moving to the common |
Will do, do you mind if I push force to take into account all your comments and clean up the commit history? |
|
@fjmolinas no problem, I followed the changes and will be able to review the rebased version. |
fjmolinas
force-pushed
the
pr_kinetis_riotboot_frdm_kw41z
branch
from
ea9b013 to
2341dcf
Compare
|
After the linkerscript change, I think you can add all the When refactoring,
To put the |
fjmolinas
force-pushed
the
pr_kinetis_riotboot_frdm_kw41z
branch
from
2341dcf to
96f67b3
Compare
It is needed for Anyway thats not the point. I can add support for
Ok will need to look into a it a bit! |
fjmolinas
changed the title
fjmolinas
removed
the
State: WIP
|
@cladmi I found the reason why it wasn't working for pba-d-01-kw2x:
For MKW22D5 the number of INT_VECTORS is (82 + 16), so it need to be In this case by default it is located at This need to be done for all M4, I had struggled a while ago debugging why Anyway I will open separate PR's for this. |
|
@fjmolinas Congratulation for finding it. This details deserve its place too in https://github.com/RIOT-OS/RIOT/tree/master/bootloaders/riotboot on the
|
|
@fjmolinas can you rebase this one on master ? we could get this ones in already. |
fjmolinas
force-pushed
the
pr_kinetis_riotboot_frdm_kw41z
branch
from
723b2c4 to
937276a
Compare
fjmolinas
added
CI: ready for build
|
@cladmi Rebased and all is green! |
|
Thanks, I will re-run the tests once more this afternoon. |
cpu/kinetis/Makefile.include
Outdated
| # If RIOTBOOT_LEN uses an uneven number of flashpages, the remainder of the | ||
| # flash cannot be divided by two slots while staying FLASHPAGE_SIZE aligned. | ||
| ifeq (K, $(KINETIS_SERIES)) | ||
| RIOTBOOT_LEN ?= 0x2000 |
There was a problem hiding this comment.
Would need two spaces here for indentation
| RIOTBOOT_LEN ?= 0x2000 | |
| RIOTBOOT_LEN ?= 0x2000 |
Could you also add that the flashpage size is 4k (0x1000) in the comment ?
You can squash them directly.
|
I could successfully test the |
|
However for the |
|
For the And if I then reset, I get a normal full output of the |
|
We could go for only the I did not know there would be more issues with the |
fjmolinas
force-pushed
the
pr_kinetis_riotboot_frdm_kw41z
branch
from
937276a to
6f978a5
Compare
fjmolinas
removed
the
CI: ready for build
fjmolinas
force-pushed
the
pr_kinetis_riotboot_frdm_kw41z
branch
from
6f978a5 to
42b1118
Compare
|
@cladmi You are right about frdm-k64f, it's has the same vector table alignment problem. But for me running
And then:
Everything is as expected. Maybe you need to run |
|
@fjmolinas after I will see if I can get a clean failure output. |
That is weird, I opened a terminal before flashing and from another terminal executed:
The application restes 4 times during flashing, I don't now why but when flashing k64f I always detect an eternal reset after which the flash hangs for a while. But everything succeeds eventually, without any manual |
cladmi
added
the
CI: ready for build
|
I re-tried many times on my side and I could not reproduce the issue. |
|
I managed to reproduce it again, but not consistently with flashing it two times. with: It may be an As it works after |
There was a problem hiding this comment.
ACK tested with tests/riotboot and tests/xtimer_usleep for both boards.
I did not double check the number of interrupt set in the Makefile.include, but it was not working without RIOTBOOT_HDR_LEN = 0x200 for the frdm-k64f
I had some nondeterministic issues when flashing the combined/extended slot0 with xtimer_usleep but it looked more like a flasher issue and it was working after reset.
The slot alignment is consistent with the 4k (0x1000) FLASHPAGE_SIZE.
|
Congratulation for taking care of adding the support from start to end. It has been a long and tough process. |
|
@cladmi Thanks a lot for the review and very helpfull advice! |
Contribution description
This PR adds riotboot support for kw41z. To do this the following changes had to be done:
I know the linker script related changes should be in another PR but I wanted to get @cladmi feedback on them before splitting them up.
Testing procedure
Run ldscript test:
BOARD=frdm-kw41z APP_VER=$(date +%s) make -C tests/cortexm_common_ldscript/Run riotboot tests:
BOARD=frdm-kw41z FEATURES_REQUIRED+=riotboot APP_VER=$(date +%s) make -C examples/hello-world/ riotboot/flash-extended-slot0BOARD=frdm-kw41z FEATURES_REQUIRED+=riotboot APP_VER=$(date +%s) make -C tests/xtimer_usleep riotboot/flash-extended-slot0BOARD=frdm-kw41z FEATURES_REQUIRED+=riotboot APP_VER=$(date +%s) make -C examples/hello-world/ riotboot/flash-slot1BOARD=frdm-kw41z FEATURES_REQUIRED+=riotboot APP_VER=$(date +%s) make -C tests/xtimer_usleep riotboot/flash-slot1Issues/PRs references
Depends on: