boards: add support for nrf52840-dongle

[chrysn]

Contribution description

This adds board support for he nrf52840-dongle. From the docs that are part of the PR:

The nRF52840-Dongle is a bare USB-stick shaped device that houses barely
anything than the nRF52840 itself, which offers BLE and 802.15.4 and USB
connectivity.

Unlike similar sticks (like the boards_nrf52840-mdk), it features no
dedicated programmer hardware but relies on direct USB communication with a
built-in bootloader.

The board features two LEDs (LD1: green, LD2: RGB), a user (SW1) and a
reset button as well as 15 configurable external pins.

Testing procedure

• Get a dongle (they're cheap, about 10$ from several distributors

• Pick any example; particularly funny are those with network support like gcoap.

• Add this to the Makefile (see later on why this is necessary)

  CFLAGS += -DCONFIG_USB_VID=0x1209
  CFLAGS += -DCONFIG_USB_PID=0x7D00
  

• Install the nrfutil USB programmer, typically using pip install nrfutil (see doc.txt for when it's not)

• Plug the device into a USB port with its reset button (the horizontal one) pressed, it'll breathe a red LED

• Build and flash the example with make BOARD=nrf52840-dongle flash

• Run make BOARD=nrf52840-term for a USB console, or chat up the network stack that just showed up as a new USB network device :-)

Issues/PRs references

Fixes #12189.
Requires #13148

Open issues

• Default USB IDs are not available for devices that just out of the box (or by virtue of something like gnrc_netdev_default) ship a USB stack. I'd like to fix this properly with Have usable VID/PIDs for standard situations and prototype ones for examples #12273.
• The USB UART default might interact badly with examples that expect ethos (running which really makes no sense on such a board, that's what USB Ethernet aka CDC-ECM is for).
• Pulling things like a USB UART or a USB Ethernet device in by default is relatively unexplored territory in RIOT. I think that it might make sense to have, in parallel to gnrc_netdev_default, a gnrc_netdev_default_host -- which pulls in a by-board picked hostward netdev, whether that's ethos, CDC-ECM or any future semihosted tuntap solution. Having a similar entry point for stdio would make sense as well -- if all stdio examples requested stdio_default, the board could decide whether that's stdio over UART or over USB. (Currently, I'm pulling in stdio_usb_acm when shell is active, which is a good-enough heuristic.)
• ModemManager might gobble up the serial console for some time due to its heuristics. That's resolved in new versions of ModemManager and acknowledged in the doc.txt.
• I've written a custom linker script that works with the shipped bootloader. Should this be somehow optional to be usable by people who program it externally? (I think the bootloader is a good default, as it just works out of the box -- and should be reconsidered when flashing own RIOT USB bootloaders becomes a thing).
• I'm setting FLASHER and FFLAGS and add a preprocessing step (FLASHFILE = $(HEXILFE).zip) which the flasher needs. Should this be factored out into a generic makefiles/tools/nrfutil.inc.mk? (Note that my original hopes of using the same infrastructure to program the Thingy:52 have been swept away in support Thingy:52 bootloader #12829).
  • In particular, this makes building on a murdock impossible right now.
• Did I (again) fail somewhere in the code style area? I hope that the automated tests that get triggered with the PR will tell me.

Final open question:

• Is any of that too bad that a few words in the docs (and a plan on how to do it better) would stop this from being mergeable?

[edit: Added open questions on flashing / bootloader]
[edit: Added checkmarks for easier tracking, ticking off ModemManager as all is confirmed now, and adding the murdock trouble miri64 pointed out]

[chrysn]

Pinging @trickeydan, @benemorius and @maz3max as I can't request you as a reviewers, but you showed interest in the corresponding issue #12189.

[miri64]

nrfutil seems to be required for building (which surprises me as it supposed only to be the programmer) and it is not installed Murdock, so no use in trying to build this there for now.

[chrysn]

There is an additional stage before flashing: This programmer expects its input as a zip file, so ${FLASHFILE} = ${HEXFILE}.zip and built using nrfutil. (The idea being that the zip file includes a bit of metadata on what chip and bootloader this is expected to be flashed to, so the bootloader can refuse if need be).

If I use a different name than ${FLASHFILE} for the zip, I'd sidestep that, but possibly confuse people who expect make all to produce the files later passed to the programmer.

Alternatively, install nrfutil on murdock (should be just a matter of pip3 install nrfutil.

Do you prefer either of these options, or see another one?

[chrysn]

I'm confused by the buildsystem_sanity_check that raised during the build and I thought to have fixed with 27fad13.

I understand that the board directory could reside somewhere else, but this linker script is custom to this board as this board has a particular bootloader, and only that would need this script. Where am I supposed to place that, and how should I refer to it in the Makefile.include?

[miri64]

Maybe @fjmolinas can help with that? He introduced that check in #12999.

[fjmolinas]

@chrysn just replace LINKER_SCRIPT ?= $(BOARDSDIR)/$(BOARD)/ldscripts/application.ld by LINKER_SCRIPT ?= $(BOARDSDIR)/nrf52840-dongle/ldscripts/application.ld. Please see #12999 (comment)_ for a more detailed explanation of the reasoning.

[chrysn]

just replace

Done, still builds & passes the check. Thanks!

[fjmolinas]

I understand that the board directory could reside somewhere else, but this linker script is custom to this board as this board has a particular bootloader, and only that would need this script. Where am I supposed to place that, and how should I refer to it in the Makefile.include?

Even if custom for this BOARD I could solder bins and maybe add another sensor/radio or whatever to it, and might want to extended to a nrf52840-dongle-mine that would be the same board but with this extra sensors or whatever. I would probably use BOARDSDIR to just inherit everything from nrf52840-mdk and add those extra mappings. If that doesn't make sense I can extend my explanation from #12999 (comment)_, I was planning in adding a doc entry but have not found time yet. Also if the error message wasn't clear enough I'm happy to amend it!

[miri64]

Some thoughts.

[miri64]

I'm not sure I understand, even with the comment, why this is dependent on the shell.

[chrysn]

I did seem to have some mixup between stdin and stdio, revisiting that in more detail. (When I wrote this I was unaware that stdin is extra functionality atop of sdio)

The idea was: If an application depends on stdio (whether directly or via shell, but I though I couldn't catch the "directly" case), it will pull in a stdio implementation. A board should be able to provide a default stdio, but may want (kind of like netdev_default does), but an application may still want to use stdio_rtt on a USB-supporting board explicitly.

But as said, I'll go through this again with better awareness of stdio vs stdin.

[miri64]

What if there is no shell? Is there then no STDIO? I'm not sure we support that concept ^^" (see #10741).

[chrysn]

Changes with the last fixes:

• 860a140 removes the automatic USB Ethernet device; running examples over USB rather than wireless now usually entails replacing gnrc_netdev_default with usbus_cdc_ecm (with auto_init_usbus already pulled in by the serial part).
• ad0d897 does away with the custom linker script in favor of ROM_OFFSET and FW_ROM_LEN

My finding is addressed properly, so nothing blocking from my side.

[miri64]

Please rebase to incorporate #13148.

[chrysn]

Rebased on master (with fixups squashed).

I've looked into the stdio_* situation (which previously was a mess because I mixed up stdin and stdio), at the feather-nrf52840 module, and changed the stio_cdc_acm picking into a DEFAULT_MODULE instead -- this seems to be the way to go currently. As that module doesn't pull in auto_init_usbus automatically, that is made a default module as well, and the behavior described in the board documentation.

I'd prefer a solution where stdio is not expected to be a default module but rather an explicit dependency of modules like shell (that can then get populated in Modules.dep according to a board-specific default implementation, possibly pulling in things like auto_init_usbus), but one step at a time.

Building with make BOARD=nrf52840-dongle DISABLE_MODULE="stdio_cdc_acm" (ie. disabling the automatic USB stdio but leaving auto_init_usbus in there) does raise a missing module error now, but that looks like an unrelated undeclared dependency to investigate.

[miri64]

Rebased on master (with fixups squashed).

You could have also squash (so effectively removing) the reverted commit with the revert commit.

Regarding stdio, have you seen #12304?

[chrysn]

Squashed accordingly. I have accumulated some other commits as well that not strictly revert things but change how they're done -- but if I fuse them all, this whole branch might wind up being only one or two commits at all. What's the preferred style here?

I was unaware of #12304, but it looks like lots of thought went in there (and also in #12724), it's aligned with what happens here (in the end, just the notes in this board's documentation can be simplified when #12304 is merged), and confirms the path with both stdio_cdc_acm and auto_init_usbus as default modules.

[chrysn]

As I understand, all previous issues are addressed:

• History doesn't include reverts any more
• nrfutil was not pulled out into a dedicated nrfutil.inc.mk (as originally announced after having read boards/adafruit-clue: add initial support #13276) because there might not be a single unified nrfutil program yet that can be used across boards.
• The board directory / path discussion became moot with ROM_OFFSET etc being used instead of a linker script.
• The serial situation is still not fully clear, but using the same DEFAULT_MODULE += stdio_... approach that feather-nrf52840 already uses, and other PRs like boards/adafruit-clue: add initial support #13276 and boards/arduino-mkr: feather-m0: sodaq-*: provide stdio over USB and setup automatic flash with bossa #12304 use as well -- I don't think that's grave enough to block the incoming boards, and can still be worked on when it's not three separate PRs that all try to find The Solution in parallel, but when things can be explored with those PRs merged.

Do you see anything left that stands in the way of this being mergable?

[aabadie]

You can have FEATURES_REQUIRED += bootloader_nrfutil without USEMODULE += usb_board_reset

+1

[chrysn]

You can have FEATURES_REQUIRED += bootloader_nrfutil without USEMODULE += usb_board_reset

That seems like a good way to go (preparing that now) -- right now, this board is only and necessarily used with this bootloader (and in future, this may change). It'll remain to ensure riotboot builds don't fail with that, which is probably the eventual outcome of the https://github.com/RIOT-OS/RIOT/pull/13146/files/981c1411d4dff28848f4ce85d2306ec89b5396d5#diff-62d41b596b7713257c78acbe1b8616fd thread.

[aabadie]

Please move this line as suggested below. The bootloader_nrfutil is a convenient build system feature that is only useful when stdio_cdc_acm is used !!

[chrysn]

Good point, fixed.

[chrysn]

Argh, sorry, I mixed up your suggestion with the one of benpicco "below" in the issue view. Does 844a0db (modulo points of opinion in the comment) reflect what you meant?

[benpicco]

-- running on worker mobi7.inet.haw-hamburg.de thread 11, build number 151701.
make: Entering directory '/tmp/dwq.0.2955577806981107/24b81fb95968c0a882b26cdc9cd38035/tests/gnrc_rpl_srh'
Some feature requirements are blacklisted: bootloader_nrfutil
/tmp/dwq.0.2955577806981107/24b81fb95968c0a882b26cdc9cd38035/tests/gnrc_rpl_srh/../../Makefile.include:876: *** You can let the build continue on expected errors by setting CONTINUE_ON_EXPECTED_ERRORS=1 to the command line.  Stop.
make: Leaving directory '/tmp/dwq.0.2955577806981107/24b81fb95968c0a882b26cdc9cd38035/tests/gnrc_rpl_srh'
-- build directory size: 0

😩

[chrysn]

That probably explains why adafruit-clue requires the bootloader feature only when no explicit stdio is set -- the tests set stdio_null and then all is fine for them.

Given we're just doing workarounds here, I'd replicate more or less exactly what the adafruit-clue board is doing (for getting things here out of the way), and open a separate issue on odd workarounds with all the nice things that came up in the comments here but had to be reverted in order to get things to work.

[benpicco]

-- running on worker riotbuild thread 1, build number 57630.
make: Entering directory '/tmp/dwq.0.4177377369128673/b42a7eb2a4d96105c5bec84e1ce536c4/bootloaders/riotboot'
Building application "riotboot" for "nrf52840dongle" with MCU "nrf52".

/opt/gcc-arm-none-eabi-9-2019-q4-major/bin/../lib/gcc/arm-none-eabi/9.2.1/../../../../arm-none-eabi/bin/ld: Specified firmware size does not fit in ROM
collect2: error: ld returned 1 exit status
/tmp/dwq.0.4177377369128673/b42a7eb2a4d96105c5bec84e1ce536c4/bootloaders/riotboot/../..//Makefile.include:565: recipe for target '/tmp/dwq.0.4177377369128673/b42a7eb2a4d96105c5bec84e1ce536c4/build/riotboot.elf' failed
make: *** [/tmp/dwq.0.4177377369128673/b42a7eb2a4d96105c5bec84e1ce536c4/build/riotboot.elf] Error 1
make: Leaving directory '/tmp/dwq.0.4177377369128673/b42a7eb2a4d96105c5bec84e1ce536c4/bootloaders/riotboot'
-- build directory size: 720K

😩

[chrysn]

OK, but that's something different than directly conflicting -- it just means that the riotboot loader won't fit in the flash with the constraints the nrfutil bootloader is imposing. That, to me, would more lead to a resolution of "We can't satisfy this application's firmware requirements with that build setting", and thus blacklisting that application for that board for memory constraints (with a note that it will probably fit conceptually but needs adaptions to the memory positions).

(Looking at adafruit-clue for guidance won't help here probably, as has a different ROM_OFFSET and bootloader shape).

[benpicco]

Well go ahead and add it to BOARD_BLACKLIST then, although that feels like admitting defeat to the build system.

I suspect it has to do with

bootloaders/riotboot/Makefile

# limit riotboot bootloader size
# TODO: Manage to set this variable for boards which already embed a
# bootloader, currently it will be overwritten
ROM_LEN := $(RIOTBOOT_LEN)

[aabadie]

Remove this line to fix the remaining Murdock issues.

[benpicco]

And replace it with

ROM_LEN = 0xdf000

?

[chrysn]

Aehm, as I understand that would not only lead to things not working any more (as the code now thinks it's starting at 0x0 but is flashed to 0x1000, and we're not all PIE yet, are we?), but also (by not limiting FW_ROM_LEN) risk building overly large images that either will fail to flash or worse will overwrite the high portion of the bootloader.

[aabadie]

I don't think so because it's handled automatically by the build system (can we stop guessing things and move to the point where this can be merged ?)

[aabadie]

See here.

[chrysn]

I just tried, flashing succeeded but the board won't come up. Going for the the blacklisting solution, this should make it passable. (The game of whether this declares its memory wrong or whether the suit/riotboot can't work on exotic-memory-layout bootloaders can be had outside this issue.)

[benpicco]

I think @aabadie was referring to the FW_ROM_LEN line - you should leave the ROM_OFFSET in place.

[chrysn]

OK, only the FW_ROM_LEN, that does make sense if the information can be passed around somewhere else (eg. in ROM_LEN).

Thanks, that appears to fix it; riotboot and suit example now build, pushing it to murdock.

[benpicco]

Murdock is happy at last 🎉

[chrysn]

Finally -- thank you both! So squash all?

[aabadie]

So squash all?

Yes please :)

[aabadie]

ACK

[aabadie]

Sorry that this PR took so long to be merged and thank you for your patience @chrysn !

[SemjonWilke]

Thanks for this port!
So, am I understanding that right, you have to use nrfutil for python2 and you have to press the reset button right before flashing and there is no easy way to avoid this? Anyway with this restrictions I ran examples/default without further ado.

[chrysn]

nrfutil needs to be installed (Python 2 or 3 does not matter), as does every other tool in the toolchain. Pressing reset before flashing is necessary at very least for the initial programming (when some arbitrary other program is on there). make flash to reset it into the bootloader could be made to work, but will always to some extent depend on the cooperation of the current program, as unlike on other development kits there is no physical programmer that's connected to the PC.

[SemjonWilke]

nrfutil needs to be installed (Python 2 or 3 does not matter),

I figured out how to use nrfutil for python3 by reading your documentation, my bad.

make flash to reset it into the bootloader could be made to work

I think this feature is very important and should be tackled soon. Can I help with that somehow?
@haukepetersen suggested these boards for a cheap, local testbed with multiple nodes and thus it should be programmable with a script and without physical access.

but will always to some extent depend on the cooperation of the current program

I don't understand which program you mean.
Is there enough to discuss here to open an issue?

[benpicco]

make flash to reset it into the bootloader could be made to work, but will always to some extent depend on the cooperation of the current program

The watchdog on nrf5x supports executing a callback before the reset. So if the user desires so, they could set it up to always write the magic value when it fires.
So when RIOT crashes you’d end up in the bootloader again.