boards/common/nrf52: add openocd support for 'nordic_softdevice_ble' #11470
boards/common/nrf52: add openocd support for 'nordic_softdevice_ble' #11470aabadie merged 1 commit intoRIOT-OS:masterfrom
Conversation
|
I just give a quick try on this one and I'm having issues: if the board is previously flashed using OpenOCD with a firmware that doesn't contain SoftDevice, flashing gnrc_networking with SoftDevice using OpenOCD results in the application being broken. Then if I flash gnrc_networking using JLink, it works. If I flash again using OpenOCD, it still works. There is something weird. |
|
@aabadie do you have a reproducible failing case ? |
Yes, use nrf52d, which loads softdevice by default with gnrc_networking and do the following, in same order:
It seems that flashing SoftDevice with JLink puts the application back in a working state. Maybe adding erase with openocd is necessary ? |
|
Thank you, from my observations it was "not necessary" as said in
But I did not have a failing case to understand why it would be needed. |
|
When comparing the output from Instead of relying on erasing during flash, I updated to use a |
|
Now the |
aabadie
left a comment
There was a problem hiding this comment.
Tested and confirmed gnrc_networking is working (shell available, ifconfig returns a configured interface) when flashing with both jlink and openocd.
The python test script also works as expected but can be improved.
|
|
||
| import sys | ||
| from testrunner import run | ||
|
|
There was a problem hiding this comment.
There's an extra blank line here, 2 are enough ;)
|
|
||
|
|
||
|
|
||
| def _check_shell_is_working(child, tries=5): |
There was a problem hiding this comment.
No need for loops, just do the following:
| def _check_shell_is_working(child, tries=5): | |
| def _check_shell_is_working(child): | |
| """Just check that the shell is working. | |
| It ensures the application did not crash. | |
| """ | |
| child.expect_exact('All up, running the shell now') | |
| child.sendline('help') | |
| child.expect_exact('Command Description', timeout=1) | |
| child.expect_exact('---------------------------------------', timeout=1) | |
| child.expect_exact('> ', timeout=1) |
There was a problem hiding this comment.
No it failed for some executions on my local board. It failed when running the test_nordic_softdevice_openocd.sh.
Welcome to pyterm!
Type '/exit' to exit.
2019-05-27 16:23:11,315 - INFO # main(): This is RIOT! (Version: buildtest)
2019-05-27 16:23:11,316 - INFO # RIOT network stack example application
2019-05-27 16:23:11,316 - INFO # All up, running the shell now
> help
2019-05-27 16:23:13,581 - INFO # main(): This is RIOT! (Version: buildtest)
2019-05-27 16:23:13,584 - INFO # RIOT network stack example application
2019-05-27 16:23:13,587 - INFO # All up, running the shell now
> Timeout in expect script at "child.expect_exact('Command Description', timeout=1)" (examples/gnrc_networking/tests/01-run.py:20)
I should also maybe increase the timeout.
I would split this in a dedicated pull request anyway.
There was a problem hiding this comment.
I just rechecked the proposed change using test_nordic_softdevice_openocd.sh and it works like a charm. Maybe you missed the synchronization child.expect_exact('All up, running the shell now'), at line 18 ?
There was a problem hiding this comment.
diff --git a/examples/gnrc_networking/tests/01-run.py b/examples/gnrc_networking/tests/01-run.py
index bd778020d..baf937c7e 100755
--- a/examples/gnrc_networking/tests/01-run.py
+++ b/examples/gnrc_networking/tests/01-run.py
@@ -15,18 +15,12 @@ def _check_shell_is_working(child, tries=5):
It ensures the application did not crash.
"""
- for i in range(0, tries):
- child.sendline('help')
- try:
- child.expect_exact('Command Description', timeout=1)
- child.expect_exact('---------------------------------------',
- timeout=1)
- child.expect_exact('> ', timeout=1)
- break
- except Exception:
- pass
- else:
- child.expect('>')
+ child.expect_exact('All up, running the shell now')
+ child.sendline('help')
+ child.expect_exact('Command Description', timeout=1)
+ child.expect_exact('---------------------------------------',
+ timeout=1)
+ child.expect_exact('> ', timeout=1)
def testfunc(child):|
I found in #5863 why the I do a PR to use the modified |
|
The pull request for changing the |
ab2dcda to
50426b6
Compare
|
Rebased on top of #11620 |
09193b9 to
fb4c927
Compare
|
I rebased this one to remove all the test commits as the not resetting behavior was moved to #11620 . |
fb4c927 to
a970bf5
Compare
|
@aabadie do you still have concerns? |
Enable the handling of flashing `softdevice.hex` when flashing the firmware
for openocd.
However, for flashing, only the `hexfile` and `binfile` can currently be used.
The `elffile` is generated with local pages aligned to `0x10000` which makes
the program starting at `0x1f000` be flashed from `0x10000` with padding bytes
even if the `.text` section is indeed at `0x1f000`:
readelf --sections bin/nrf52dk/gnrc_networking.elf
...
[ 1] .text PROGBITS 0001f000 00f000 00f698 00 AX 0 0 16
...
readelf --segments bin/nrf52dk/gnrc_networking.elf
...
LOAD 0x000000 0x00010000 0x00010000 0x1e6a0 0x1e6a0 R E 0x10000
...
The padding bytes would go through `verify_image` in `openocd` so be expected
to not be overwritten but are by `softdevice.hex`
Using --nmagic at link time removes the local page alignement but would
need dedicated testing.
a970bf5 to
fe0b829
Compare
|
Rebased now that #11620 is merged. I did not cleaned that now the |
aabadie
left a comment
There was a problem hiding this comment.
With all related PRs merged, I retested flashing with OpenOCD and it now works like a charm.
I have one question regarding Murdock integration though.
aabadie
left a comment
There was a problem hiding this comment.
Let's go with this PR. ACK
|
Thank you for the review and for having added the openocd support in the first place :) |
|
I noticed one issue thank to this one. But it is not specific to this example but found out about it with this. When building with I will add |
|
Fix provided in #12207 |
Contribution description
Enable the handling of flashing
softdevice.hexwhen flashing the firmwarefor openocd.
However, for flashing, only the
hexfileandbinfilecan currently be used.The
elffileis generated with local pages aligned to0x10000which makesthe program starting at
0x1f000be flashed from0x10000with padding byteseven if the
.textsection is indeed at0x1f000:The padding bytes would go through
verify_imageinopenocdso be expectedto not be overwritten but are by
softdevice.hexUsing --nmagic at link time removes the local page alignement but would
need dedicated testing.
Testing procedure
It can be tested with
gnrc_networkingandtests/nordic_softdevice_ble.Murdocktest execution output is not showing it is working as it does not useopenocd. It would only show this did not break anything (even if the real changes are only in theopenocdbranch).hwrng issue fixed
Previous memory comparison that shown issues and that is now handled in #11620
Another testing procedure will be showing memory comparison with what Jlink flashes.
I added a test file to generate all the memory files to compare.
Prequisite info
softdevice.hexis not full, bytes in[0x8bc, 0x3000[are empty and the file is0x1eb58bytes long.So any change in unused bytes in the output is not important.
Comparison of the output of the objcopy with
--gap-fillOutput comparison
In the openocd case, I did not erase the memory as done for Jlink as I think it is not necessary.
Comparison with the
erasedversionComparison with setting the memory to
0x00All the changes are between
0x1000and0x2000so can be ignored.Comparison with flashing the memory from a previous firmware without setting memory
Here too, the difference is only between
0x1000and0x2000so within unused range.I provide the full output anyway if anybody is interested for reviewing without executing it themself.
diff -u jlink_flashed.hex openocd_dirty.hex
Issues/PRs references
I worked on this in the context of #10870 as my
nrf52dkdoes notresetanymore withJlinkbut does withopenocd.