Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sys/usbus: answer get_status if request is standard type #20475

Merged
merged 1 commit into from
Mar 18, 2024

Conversation

dylad
Copy link
Member

@dylad dylad commented Mar 17, 2024

Contribution description

this PR intends to fix #20474. In current master, a device doesn't behave properly when dfu-util is sending a detach request.
Once this request is sent, device must reboot into bootloader. However, it doesn't work in master since #17090.

#17090 introduces (among other things) a way for USBUS to answer standard requests (received on the control endpoints aka EP0).
However, DFU is an USB class which uses the same endpoints to operate. When dfu-util sends the detach request, USBUS wrongly interprets it as a standard get_status request so the stack answers it instead of the DFU interface.
Indeed, both the DFU detach request and the standard get_status have the same request number. USB_SETUP_REQ_GET_STATUS and DFU_DETACH are both 0x00.
Thus, this PR introduces a new check to ensure if the request should be processed by USBUS or by an interface.
The request type of a DFU detach request is 0x21 which correspond to USB_SETUP_REQUEST_RECIPIENT_INTERFACE | USB_SETUP_REQUEST_TYPE_CLASS.
This way, when dfu-util send a detach request, USBUS will now pass it to the interface and will only process USB_SETUP_REQUEST_TYPE_STANDARD, as it is suppose to do.

Testing procedure

Flash riotboot_dfu bootloader into your board:
make BOARD=same54-xpro -C bootloaders/riotboot_dfu flash
Then flash a first app into your board through dfu;
FEATURES_REQUIRED=riotboot PROGRAMMER=dfu-util USEMODULE=usbus_dfu make -j4 BOARD=same54-xpro -C tests/sys/shell riotboot/flash-slot0
Finally, flash a second app while the first is running:
DFU_USB_ID=1209:7d00 FEATURES_REQUIRED=riotboot PROGRAMMER=dfu-util USEMODULE=usbus_dfu make -j4 BOARD=same54-xpro -C tests/leds riotboot/flash-slot1

On current master, the last step will fail, the device will never reboot into bootloader and stays in the first app.
With this PR, device will reboot, flash the second application then runs it.

Issues/PRs references

Fixes #20474

Signed-off-by: Dylan Laduranty <dylan.laduranty@mesotic.com>
@dylad dylad requested a review from aabadie as a code owner March 17, 2024 15:57
@github-actions github-actions bot added Area: USB Area: Universal Serial Bus Area: sys Area: System labels Mar 17, 2024
@dylad dylad added the Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) label Mar 17, 2024
@benpicco benpicco added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Mar 18, 2024
Copy link
Member

@bergzand bergzand left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Untested Ack!

@riot-ci
Copy link

riot-ci commented Mar 18, 2024

Murdock results

✔️ PASSED

f44ef81 sys/usbus: answer get_status if request is standard type

Success Failures Total Runtime
10009 0 10009 08m:34s

Artifacts

@dylad
Copy link
Member Author

dylad commented Mar 18, 2024

@bergzand for the record:

Here are my terminal output with this PR:

DFU_USB_ID=1209:7d00 FEATURES_REQUIRED=riotboot PROGRAMMER=dfu-util USEMODULE=usbus_dfu make -j4 BOARD=same54-xpro -C tests/leds riotboot/flash-slot1
make : on entre dans le répertoire « /home/dylad/software/RIOT/tests/leds »
/home/dylad/software/RIOT/tests/leds/../../Makefile.include:496: 'dfu-util' programmer is not supported by this board. Supported programmers: 'openocd edbg jlink'
compiling /home/dylad/software/RIOT/dist/tools/riotboot_gen_hdr/bin/genhdr...
make: Nothing to be done for 'all'.
"make" -C /home/dylad/software/RIOT/pkg/cmsis/ 
"make" -C /home/dylad/software/RIOT/boards/common/init
"make" -C /home/dylad/software/RIOT/boards/same54-xpro
"make" -C /home/dylad/software/RIOT/core
"make" -C /home/dylad/software/RIOT/core/lib
"make" -C /home/dylad/software/RIOT/cpu/samd5x
"make" -C /home/dylad/software/RIOT/cpu/cortexm_common
"make" -C /home/dylad/software/RIOT/cpu/cortexm_common/periph
"make" -C /home/dylad/software/RIOT/cpu/sam0_common
"make" -C /home/dylad/software/RIOT/cpu/sam0_common/periph
"make" -C /home/dylad/software/RIOT/cpu/samd5x/periph
"make" -C /home/dylad/software/RIOT/drivers
"make" -C /home/dylad/software/RIOT/drivers/periph_common
"make" -C /home/dylad/software/RIOT/sys
"make" -C /home/dylad/software/RIOT/sys/auto_init
"make" -C /home/dylad/software/RIOT/sys/auto_init/usb
"make" -C /home/dylad/software/RIOT/sys/checksum
"make" -C /home/dylad/software/RIOT/sys/div
"make" -C /home/dylad/software/RIOT/sys/event
"make" -C /home/dylad/software/RIOT/sys/fmt
"make" -C /home/dylad/software/RIOT/sys/libc
"make" -C /home/dylad/software/RIOT/sys/luid
"make" -C /home/dylad/software/RIOT/sys/malloc_thread_safe
"make" -C /home/dylad/software/RIOT/sys/newlib_syscalls_default
"make" -C /home/dylad/software/RIOT/sys/pm_layered
"make" -C /home/dylad/software/RIOT/sys/preprocessor
"make" -C /home/dylad/software/RIOT/sys/riotboot
"make" -C /home/dylad/software/RIOT/sys/stdio
"make" -C /home/dylad/software/RIOT/sys/stdio_uart
"make" -C /home/dylad/software/RIOT/sys/usb/usbus
"make" -C /home/dylad/software/RIOT/sys/usb/usbus/dfu
creating /home/dylad/software/RIOT/tests/leds/bin/same54-xpro/riotboot_files/slot1.1710787925.bin...
dfu-util --device 1209:7d00 --alt 1 --download /home/dylad/software/RIOT/tests/leds/bin/same54-xpro/riotboot_files/slot1.1710787925.bin 
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/

dfu-util: Invalid DFU suffix signature
dfu-util: A valid DFU suffix will be required in a future dfu-util release!!!
Opening DFU capable USB device...
ID 1209:7d00
Run-time device DFU version 0110
Claiming USB DFU Runtime Interface...
Determining device status: state = appIDLE, status = 0
Device really in Runtime Mode, send DFU detach request...
dfu-util: error detaching
Device will detach and reattach...
Opening DFU USB Device...
Claiming USB DFU Interface...
Setting Alternate Setting #1 ...
Determining device status: state = dfuIDLE, status = 0
dfuIDLE, continuing
DFU mode device DFU version 0110
Device returned transfer size 64
Copying data from PC to DFU device
Download	[=========================] 100%        17164 bytes
Download done.
state(5) = dfuDNLOAD-IDLE, status(0) = No error condition is present
Done!
make : on quitte le répertoire « /home/dylad/software/RIOT/tests/leds »

Against, the output of current master:

DFU_USB_ID=1209:7d00 FEATURES_REQUIRED=riotboot PROGRAMMER=dfu-util USEMODULE=usbus_dfu make -j4 BOARD=same54-xpro -C tests/leds riotboot/flash-slot1
make : on entre dans le répertoire « /home/dylad/software/RIOT/tests/leds »
/home/dylad/software/RIOT/tests/leds/../../Makefile.include:496: 'dfu-util' programmer is not supported by this board. Supported programmers: 'openocd edbg jlink'
compiling /home/dylad/software/RIOT/dist/tools/riotboot_gen_hdr/bin/genhdr...
make: Nothing to be done for 'all'.
"make" -C /home/dylad/software/RIOT/pkg/cmsis/ 
"make" -C /home/dylad/software/RIOT/boards/common/init
"make" -C /home/dylad/software/RIOT/boards/same54-xpro
"make" -C /home/dylad/software/RIOT/core
"make" -C /home/dylad/software/RIOT/core/lib
"make" -C /home/dylad/software/RIOT/cpu/samd5x
"make" -C /home/dylad/software/RIOT/cpu/cortexm_common
"make" -C /home/dylad/software/RIOT/drivers
"make" -C /home/dylad/software/RIOT/drivers/periph_common
"make" -C /home/dylad/software/RIOT/sys
"make" -C /home/dylad/software/RIOT/sys/auto_init
"make" -C /home/dylad/software/RIOT/sys/auto_init/usb
"make" -C /home/dylad/software/RIOT/sys/checksum
"make" -C /home/dylad/software/RIOT/cpu/cortexm_common/periph
"make" -C /home/dylad/software/RIOT/sys/div
"make" -C /home/dylad/software/RIOT/sys/event
"make" -C /home/dylad/software/RIOT/cpu/sam0_common
"make" -C /home/dylad/software/RIOT/cpu/sam0_common/periph
"make" -C /home/dylad/software/RIOT/sys/fmt
"make" -C /home/dylad/software/RIOT/sys/libc
"make" -C /home/dylad/software/RIOT/sys/luid
"make" -C /home/dylad/software/RIOT/sys/malloc_thread_safe
"make" -C /home/dylad/software/RIOT/sys/newlib_syscalls_default
"make" -C /home/dylad/software/RIOT/cpu/samd5x/periph
"make" -C /home/dylad/software/RIOT/sys/pm_layered
"make" -C /home/dylad/software/RIOT/sys/preprocessor
"make" -C /home/dylad/software/RIOT/sys/riotboot
"make" -C /home/dylad/software/RIOT/sys/stdio
"make" -C /home/dylad/software/RIOT/sys/stdio_uart
"make" -C /home/dylad/software/RIOT/sys/usb/usbus
"make" -C /home/dylad/software/RIOT/sys/usb/usbus/dfu
creating /home/dylad/software/RIOT/tests/leds/bin/same54-xpro/riotboot_files/slot1.1710788119.bin...
dfu-util --device 1209:7d00 --alt 1 --download /home/dylad/software/RIOT/tests/leds/bin/same54-xpro/riotboot_files/slot1.1710788119.bin 
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/

dfu-util: Invalid DFU suffix signature
dfu-util: A valid DFU suffix will be required in a future dfu-util release!!!
Opening DFU capable USB device...
ID 1209:7d00
Run-time device DFU version 0110
Claiming USB DFU Runtime Interface...
Determining device status: state = appIDLE, status = 0
Device really in Runtime Mode, send DFU detach request...
Device will detach and reattach...
dfu-util: Lost device after RESET?
make: *** [/home/dylad/software/RIOT/makefiles/boot/riotboot.mk:148 : riotboot/flash-slot1] Erreur 74
make : on quitte le répertoire « /home/dylad/software/RIOT/tests/leds »

@bergzand bergzand added this pull request to the merge queue Mar 18, 2024
Merged via the queue into RIOT-OS:master with commit a365a97 Mar 18, 2024
28 checks passed
@dylad
Copy link
Member Author

dylad commented Mar 19, 2024

Thanks @bergzand !

@dylad dylad deleted the pr/usbus/fix_get_status_response branch March 19, 2024 08:12
@MrKevinWeiss MrKevinWeiss added this to the Release 2024.04 milestone Apr 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: sys Area: System Area: USB Area: Universal Serial Bus CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

usbus/dfu: cannot detach device to reboot into bootloader
5 participants