usbus/dfu: add Device Firmware Upgrade support for USBUS (2nd attempt) #15460

dylad · 2020-11-17T19:33:41Z

Contribution description

This is the second attempt to add Device Firmware Upgrade (DFU) support to USBUS and thus replace #13856
This PR provides the same features from the previous one but with the following additions/changes:

DFU feature is no more an experimental feature
DFU integration to riotboot was added
Dedicated bootloader app was created
Kconfig support was also added.
no_idle_thread feature is now needed by riotboot bootloader when DFU is used
xtimer is required for the bootloader
dfu-util integration to RIOT buildsystem using make riotboot/flash-slotX
Alt interface support to manage the slot selection

Currently, the DFU bootloader mode is triggered by dfu_utils only and riotboot checks the last 4-bytes of RAM for a magic word to enter in DFU mode (otherwise we skip it)
Of course, other mechanisms could be implement in follow-up PRs.

This PR was tested with SAME54-XPRO and a Linux host machine. It would be great to test on nRF52 and STM32 but I don't own such hardware.
Note: periph_flashpage must be supported by your MCU to use DFU.

Testing procedure

Consider erasing the whole flash before starting.
#FLASH RIOTBOOT BOOTLOADER
USEMODULE=riotboot_usb_dfu BOARD=same54-xpro make -C bootloaders/riotboot_dfu flash

You can now flash any test app you want on riotboot slot0 or slot1 with the following command:
USB_VID=1209 USB_PID=7d01 DFU_ALT=0 USEMODULE=usbus_dfu PROGRAMMER=dfu-util make BOARD=same54-xpro -C tests/leds riotboot/flash-slot0
to flash slot1, please modify the DFU_ALT to 1 and use riotboot/flash-slot1

Slot selection is now made through the alt settings selection
you can use dfu-util --list
so you'll get in runtime mode:

dfu-util --list
dfu-util 0.9

Copyright 2005-2009 Weston Schmidt, Harald Welte and OpenMoko Inc.
Copyright 2010-2016 Tormod Volden and Stefan Schmidt
This program is Free Software and has ABSOLUTELY NO WARRANTY
Please report bugs to http://sourceforge.net/p/dfu-util/tickets/

Found Runtime: [1209:7d01] ver=0000, devnum=108, cfg=1, intf=0, path="2-1.2", alt=0, name="RIOT-OS bootloader", serial="UNKNOWN"

and DFU mode:

$ dfu-util --list
dfu-util 0.9

Copyright 2005-2009 Weston Schmidt, Harald Welte and OpenMoko Inc.
Copyright 2010-2016 Tormod Volden and Stefan Schmidt
This program is Free Software and has ABSOLUTELY NO WARRANTY
Please report bugs to http://sourceforge.net/p/dfu-util/tickets/

Found DFU: [1209:7d01] ver=0000, devnum=109, cfg=1, intf=0, path="2-1.2", alt=1, name="RIOT-OS Slot 1", serial="UNKNOWN"
Found DFU: [1209:7d01] ver=0000, devnum=109, cfg=1, intf=0, path="2-1.2", alt=0, name="RIOT-OS Slot 0", serial="UNKNOWN"

Issues/PRs references

Replaces #13856
Required #15555 #15565 #15566

bergzand

Some initial comments because of the flashpage rename.

sys/usb/usbus/dfu/dfu.c

dylad · 2020-11-18T19:50:35Z

rebase and push a fixup at the same time to reflect lastest changes in the flashpage API.
This PR still relies on periph_flashpage
I'll try to adapt to use riotboot_flashwrite instead at some point.

dylad · 2020-11-22T21:15:09Z

Update this PR to use riotboot/flashwrite instead of periph_flashpage.

benpicco

looks pretty straightforward.
Would be nice to have the flash process integrated into the build system, so make PROGRAMMER=dfu-util flash flash works instead of having to manually invoke dfu-util.

I tried to flash a third firmware:

APP_VER=5 BOARD=same54-xpro USEMODULE=usbus_dfu make -C tests/usbus_cdc_acm_stdio riotboot/slot
sudo dfu-util -a 0 -R -D tests/usbus_cdc_acm_stdio/bin/same54-xpro/tests_usbus_cdc_acm_stdio-slot0.bin

However, this didn't seem to work - the firmware in slot 1 is always booted.
Also, do we always have to flash alternating slots?

It's seems like I can't flash into slot 1 when the last firmware is running from slot1, but that shouldn't matter for riotboot.

It's also a bit strange that the user has to take care about the firmware version - shouldn't the firmware that was flashed by the user always be booted, not matter what version the previously installed firmware had?

sys/usb/usbus/usbus_control.c

benpicco · 2020-11-28T18:11:36Z

bootloaders/riotboot_dfu/main.c

+    for (unsigned i = 0; i < riotboot_slot_numof; i++) {
+        const riotboot_hdr_t *riot_hdr = riotboot_slot_get_hdr(i);
+        if (riotboot_slot_validate(i)) {
+            /* skip slot if metadata broken */
+            continue;
+        }
+        if (riot_hdr->start_addr != riotboot_slot_get_image_startaddr(i)) {
+            continue;
+        }
+        if (slot == -1 || riot_hdr->version > version) {
+            version = riot_hdr->version;
+            slot = i;
+        }
+    }


Should we also be able to use 'raw' images (without a riotboot header) when not using the riotboot_slot module?

In theory, if there is no slot, we could flash the firmware right after the bootloader. The main issue I have with this now: If something is wrong with the firmware resulting in a unbootable app. We will have no way to retrigger the bootloader DFU as it is called by dfu-utils if it find a device declaring a dfu application interface. If the application doesn't boot, this interface will not exist.
Hopefully, we can add other way to trigger the bootloader DFU: douple-tap reset, GPIO state at boottime, etc. But I like to keep this for another PR.

benpicco · 2020-11-28T18:13:33Z

sys/Makefile.include

+ifneq (,$(filter usbus_dfu,$(USEMODULE)))
+  CFLAGS += -DCPU_RAM_BASE=$(RAM_START_ADDR)
+  CFLAGS += -DCPU_RAM_SIZE=$(RAM_LEN)
+endif


I wonder if we should set this unconditionally

sys/usb/usbus/dfu/dfu.c

dylad · 2020-11-29T10:19:53Z

looks pretty straightforward.
Would be nice to have the flash process integrated into the build system, so make PROGRAMMER=dfu-util flash flash works instead of having to manually invoke dfu-util.

I didn't implement it yet because I'm not that familiar with RIOT build system. I can add it here now or in another PR later (I prefer the latter :) )

I tried to flash a third firmware:
However, this didn't seem to work - the firmware in slot 1 is always booted.

I'll look into this. This is not a normal behavior.

It's seems like I can't flash into slot 1 when the last firmware is running from slot1, but that shouldn't matter for riotboot.

Indeed, but since there is no way to tell which slot or address we want to flash with dfu-utils (and DFU 1.1 spec) I had to take a decision about the slot selection. Basically, we let riotboot check which slot SHOULD start and we flash the other slot available with the newer firmware.

It's also a bit strange that the user has to take care about the firmware version - shouldn't the firmware that was flashed by the user always be booted, not matter what version the previously installed firmware had?

I'll rework it when I'll integrate dfu-utils to the build system.

dylad · 2020-12-03T21:11:09Z

Indeed, but since there is no way to tell which slot or address we want to flash with dfu-utils (and DFU 1.1 spec) I had to take a decision about the slot selection. Basically, we let riotboot check which slot SHOULD start and we flash the other slot available with the newer firmware.

I read the DFU spec 1.1 once again. I think I have found the missing piece of the puzzle. I'll come up with a better solution in a couple of days.

dylad · 2020-12-06T20:36:41Z

Force push the branch.
I rework this PR quite a bit and thus I'll update the PR description to provide the new testing procedure.
This force push changes the following:

dfu-util build system integration for sam0, use PROGRAMMER=dfu-util.
delay the reboot using RTC to fix an error with dfu-util
Slot to flash can now be selected through dfu-util using --alt setting (0 or 1) thanks to the alt interface addition
Correctly handle DFU class request (no more dirty workaround)
bootloader will stay in DFU mode if no bootable slot were found

dylad · 2020-12-06T20:52:03Z

This PR is now based on #15555 #15565 #15566 and can be used to test them.

makefiles/boards/sam0.inc.mk

bergzand · 2020-12-08T15:52:52Z

I managed to get this working on the nrf52840dk by modifying the advertised transaction size to 64 B. The length field in the setup requests during download indicates the full length of the transaction, not the per-request chunk size, so my nrf52840dk thought there was 4KB data ready to write to the flash after a single setup request.

bergzand

Tested this on the nrf52840dk, works fine as long as the binaries transferred are compiled for the nrf52840dk and not for the samr21-xpro 😇

cpu/cortexm_common/Makefile.include

makefiles/boards/sam0.inc.mk

sys/include/usb/dfu.h

sys/usb/usbus/dfu/dfu.c

dylad · 2020-12-08T20:17:17Z

Give it a try on Windows 10 using dfu-util win64 binary. Works as expected after installing winusb driver for the board with Zadig tool.

benpicco · 2020-12-16T15:02:23Z

CI still has some complaints 😉

dylad · 2020-12-16T15:34:26Z

I'll fix them with the next round of comments.

dylad · 2021-01-04T20:57:12Z

Forced push and squashed to fix CI complaints !

bootloaders/riotboot_dfu/Makefile

sys/usb/usbus/dfu/Kconfig

sys/include/riotboot/usb_dfu.h

sys/include/usb/dfu.h

bergzand

One minor nit remaining, feel free to squash immediately

sys/include/riotboot/usb_dfu.h

dylad · 2021-01-12T10:39:34Z

Squashed and rebased to fix conflict introduced by #14629

bergzand

All green! Go!

dylad added Type: new feature The issue requests / The PR implemements a new feature for RIOT Area: USB Area: Universal Serial Bus labels Nov 17, 2020

dylad added this to the Release 2021.01 milestone Nov 17, 2020

dylad requested a review from bergzand November 17, 2020 19:33

dylad requested review from aabadie, cgundogan, DipSwitch, fjmolinas, haukepetersen, jia200x, kaspar030, leandrolanzieri and thomaseichinger as code owners November 17, 2020 19:33

dylad mentioned this pull request Nov 17, 2020

usbus: add Device Firmware Upgrade (DFU) support #13856

Closed

bergzand reviewed Nov 18, 2020

View reviewed changes

sys/usb/usbus/dfu/dfu.c Outdated Show resolved Hide resolved

sys/usb/usbus/dfu/dfu.c Outdated Show resolved Hide resolved

dylad force-pushed the pr/usbus/dfu_riotboot branch from 95cc041 to 4fceb34 Compare November 18, 2020 19:48

benpicco reviewed Nov 28, 2020

View reviewed changes

dylad mentioned this pull request Dec 5, 2020

usbus: add optional string descriptor for alt interface #15566

Merged

dylad force-pushed the pr/usbus/dfu_riotboot branch from 16932eb to fcec827 Compare December 6, 2020 20:26

dylad added the State: waiting for other PR State: The PR requires another PR to be merged first label Dec 6, 2020

benpicco reviewed Dec 7, 2020

View reviewed changes

makefiles/boards/sam0.inc.mk Outdated Show resolved Hide resolved

bergzand reviewed Dec 8, 2020

View reviewed changes

dylad force-pushed the pr/usbus/dfu_riotboot branch from c21ac97 to 4c2922c Compare December 15, 2020 16:18

benpicco added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Dec 16, 2020

dylad force-pushed the pr/usbus/dfu_riotboot branch from 4c2922c to a95267b Compare January 4, 2021 20:41

benpicco reviewed Jan 4, 2021

View reviewed changes

bootloaders/riotboot_dfu/Makefile Show resolved Hide resolved

dylad force-pushed the pr/usbus/dfu_riotboot branch from a95267b to 56ca22c Compare January 4, 2021 21:24

leandrolanzieri reviewed Jan 5, 2021

View reviewed changes

sys/usb/usbus/dfu/Kconfig Outdated Show resolved Hide resolved

dylad force-pushed the pr/usbus/dfu_riotboot branch from 56ca22c to 7c30a8d Compare January 5, 2021 19:26

bergzand requested changes Jan 12, 2021

View reviewed changes

sys/include/riotboot/usb_dfu.h Outdated Show resolved Hide resolved

sys/include/usb/dfu.h Outdated Show resolved Hide resolved

sys/include/usb/dfu.h Outdated Show resolved Hide resolved

bergzand self-assigned this Jan 12, 2021

bergzand reviewed Jan 12, 2021

View reviewed changes

sys/include/riotboot/usb_dfu.h Show resolved Hide resolved

dylad added 8 commits January 12, 2021 11:34

usbus/dfu: introduce initial Device Firmware Upgrade support for USBUS

669a8ec

cpu/cortexm_common: increase RIOTBOOT_LEN size when usbus_dfu is used

9d02efb

sys/auto_init: add support for USB DFU

f3908be

sys/riotboot: add DFU support to riotboot

beb0fb7

bootloader: create new bootloader application with DFU support

e87bb6a

Kconfig: add Kconfig support for DFU

25d0dff

cpu/nrf52: update RIOTBOOT_LEN w/ MODULE_USBUS_DFU

bca340d

makefiles/dfu-util: automatically generate DFU_USB_ID

7fc5bb4

dylad force-pushed the pr/usbus/dfu_riotboot branch from 8d3bf95 to 7fc5bb4 Compare January 12, 2021 10:35

bergzand approved these changes Jan 12, 2021

View reviewed changes

bergzand merged commit 2e2f950 into RIOT-OS:master Jan 12, 2021

dylad deleted the pr/usbus/dfu_riotboot branch January 12, 2021 11:06

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

usbus/dfu: add Device Firmware Upgrade support for USBUS (2nd attempt) #15460

usbus/dfu: add Device Firmware Upgrade support for USBUS (2nd attempt) #15460

dylad commented Nov 17, 2020 •

edited

Loading

bergzand left a comment

dylad commented Nov 18, 2020

dylad commented Nov 22, 2020

benpicco left a comment •

edited

Loading

benpicco Nov 28, 2020

dylad Nov 29, 2020

benpicco Nov 28, 2020

dylad commented Nov 29, 2020

dylad commented Dec 3, 2020

dylad commented Dec 6, 2020

dylad commented Dec 6, 2020 •

edited

Loading

bergzand commented Dec 8, 2020 •

edited

Loading

bergzand left a comment

dylad commented Dec 8, 2020

benpicco commented Dec 16, 2020

dylad commented Dec 16, 2020

dylad commented Jan 4, 2021

bergzand left a comment

dylad commented Jan 12, 2021

bergzand left a comment

usbus/dfu: add Device Firmware Upgrade support for USBUS (2nd attempt) #15460

usbus/dfu: add Device Firmware Upgrade support for USBUS (2nd attempt) #15460

Conversation

dylad commented Nov 17, 2020 • edited Loading

Contribution description

Testing procedure

Issues/PRs references

bergzand left a comment

Choose a reason for hiding this comment

dylad commented Nov 18, 2020

dylad commented Nov 22, 2020

benpicco left a comment • edited Loading

Choose a reason for hiding this comment

benpicco Nov 28, 2020

Choose a reason for hiding this comment

dylad Nov 29, 2020

Choose a reason for hiding this comment

benpicco Nov 28, 2020

Choose a reason for hiding this comment

dylad commented Nov 29, 2020

dylad commented Dec 3, 2020

dylad commented Dec 6, 2020

dylad commented Dec 6, 2020 • edited Loading

bergzand commented Dec 8, 2020 • edited Loading

bergzand left a comment

Choose a reason for hiding this comment

dylad commented Dec 8, 2020

benpicco commented Dec 16, 2020

dylad commented Dec 16, 2020

dylad commented Jan 4, 2021

bergzand left a comment

Choose a reason for hiding this comment

dylad commented Jan 12, 2021

bergzand left a comment

Choose a reason for hiding this comment

dylad commented Nov 17, 2020 •

edited

Loading

benpicco left a comment •

edited

Loading

dylad commented Dec 6, 2020 •

edited

Loading

bergzand commented Dec 8, 2020 •

edited

Loading