cpu/sam0_common: flashpage: clean up implementation #14502
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
benpicco
added
Type: cleanup
benpicco
added
the
CI: ready for build
benpicco
force-pushed
the
cpu/sam0_common/flashpage_cleanup
branch
from
a1fb41f to
1086c14
Compare
benpicco
force-pushed
the
cpu/sam0_common/flashpage_cleanup
branch
2 times, most recently
from
39d1b46 to
2aadddf
Compare
|
Tested on samr34-xpro (saml21) and it doesn't work (it gets a hardfault). It works on master. this PR:master:Here is the output of GDB when using the debug target after the crash: PROGRAMMER=openocd BUILD_IN_DOCKER=1 make BOARD=samr34-xpro -C tests/periph_flashpage/ debug
make: Entering directory '/work/riot/RIOT/tests/periph_flashpage'
/work/riot/RIOT/dist/tools/openocd/openocd.sh debug /work/riot/RIOT/tests/periph_flashpage/bin/samr34-xpro/tests_periph_flashpage.elf
### Starting Debugging ###
Open On-Chip Debugger 0.10.0+dev-01293-g7c88e76a-dirty (2020-07-01-21:36)
Licensed under GNU GPL v2
For bug reports, read
http://openocd.org/doc/doxygen/bugs.html
Reading symbols from /work/riot/RIOT/tests/periph_flashpage/bin/samr34-xpro/tests_periph_flashpage.elf...
Remote debugging using :3333
hard_fault_handler (sp=0x61 <vector_cpu+32>, corrupted=1090535424, exc_return=536873384,
r4_to_r11_stack=0x1a6f <flashpage_write_and_verify+10>) at /data/riotbuild/riotbase/cpu/cortexm_common/vectors_cortexm.c:411
411 /data/riotbuild/riotbase/cpu/cortexm_common/vectors_cortexm.c: No such file or directory.
(gdb) set $pc=0x2286
(gdb) frame 0
#0 0x00002286 in memcpy ()
(gdb) bt
#0 0x00002286 in memcpy ()
#1 0x00001606 in _write_page (cmd_write=<optimized out>, len=64, data=0x20000aac <page_mem>, dst=0x3ff00)
at /data/riotbuild/riotbase/cpu/sam0_common/periph/flashpage.c:120
#2 _write_row (len=<optimized out>, chunk_size=<optimized out>, cmd_write=<optimized out>, _data=<optimized out>,
dst=0x3ff00 '\377' <repeats 200 times>...) at /data/riotbuild/riotbase/cpu/sam0_common/periph/flashpage.c:160
#3 flashpage_write (page=page@entry=1023, data=data@entry=0x20000aac <page_mem>)
at /data/riotbuild/riotbase/cpu/sam0_common/periph/flashpage.c:177
#4 0x00001a6e in flashpage_write_and_verify (page=page@entry=1023, data=0x20000aac <page_mem>)
at /data/riotbuild/riotbase/drivers/periph_common/flashpage.c:60
#5 0x00001046 in cmd_test_last (argc=<optimized out>, argv=<optimized out>) at /data/riotbuild/riotbase/tests/periph_flashpage/main.c:338
#6 0x000019c8 in handle_input_line (line=<optimized out>, command_list=<optimized out>) at /data/riotbuild/riotbase/sys/shell/shell.c:311
#7 shell_run_once (shell_commands=0x73616c5f, shell_commands@entry=0x3cd0 <shell_commands>, line_buf=0x300074 "",
line_buf@entry=0x200009d8 <main_stack+1336> "test_last", len=1953719668, len@entry=128)
at /data/riotbuild/riotbase/sys/shell/shell.c:479
#8 0x0000145e in shell_run_forever (len=128, line_buf=0x200009d8 <main_stack+1336> "test_last", commands=0x3cd0 <shell_commands>)
at /data/riotbuild/riotbase/sys/include/shell.h:153
#9 shell_run (len=128, line_buf=0x200009d8 <main_stack+1336> "test_last", commands=0x3cd0 <shell_commands>)
at /data/riotbuild/riotbase/sys/include/shell.h:153
#10 main () at /data/riotbuild/riotbase/tests/periph_flashpage/main.c:575
(gdb) |
|
Arg, just the board I don't have right now. |
There are some samr30-xpro on iot-lab, they are very similar so maybe you can test on them ? |
Yes it changes something: it works 👍 |
Same for SAML10. But it works with #14512. |
|
Looks like |
|
The test now passes for me on BUILD_IN_DOCKER=1 make BOARD=samr34-xpro -C tests/periph_flashpage/ flash test |
fjmolinas
reviewed
Aug 4, 2020
|
Also works on |
fjmolinas
added
the
CI: run tests
|
Please squash @benpicco! |
Move common code into helper functions and extract the commands that differ between normal and RWWEE page reading / writing. This cuts down on `#ifdef` use.
benpicco
force-pushed
the
cpu/sam0_common/flashpage_cleanup
branch
from
9bc2824 to
e560042
Compare
|
|
|
Hmm.. way are there no |
I see these tests passed: |
those seem to either always run (relic) or to have been affected by a flashpage change. murdock caches positive results for unchanged tests or binaries... |
I'm sorry I didn't spot them because there where so many more |
fjmolinas
added
CI: ready for build
benpicco
added
CI: skip compile test
|
I'd say we can also skip the compile tests, everything has been compiled already by Murdock without error. |
benpicco
removed
the
CI: skip compile test
|
Ping? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Contribution description
periph/flashpage.cis a thicket of#ifdefsthat makes it hard to extend functionality.To clean this up, move all commands that differ between RWWEE and normal flash page writing into helper functions and rely on common code for both without the ifdefs, by using function pointers.
No functional changes so far, but will be the basis for writing to the config section of sam0 MCUs.
Testing procedure
Run
tests/periph_flashpageandtests/mtd_flashpageonIssues/PRs references