USBUS: Add URB support #17091

bergzand · 2021-10-31T20:42:13Z

Contribution description

This commit adds support for URBs (USB Request/Response Blocks). These
allow for submitting multi-transfer sized buffers with USBUS handling
the individual usbdev xmits. Multiple URBs can be queued at once for a
single endpoint and USBUS will handle them in the order of submission.

OUT endpoint URBs must always consist of a whole number of full-sized
transfers (N x MaxEndpointSize). They will automatically finish after
the endpoint received a transfer less than the endpoint size.

IN endpoints can be arbitrary-sized and do not have to consist of a
whole number of full-sized transmissions. They support a flag to
indicate that the last transfer in the sequence must be less than a full
sized transfer (USBUS_URB_FLAG_AUTO_ZLP) and this adds a zero length
transfer at the end of the transmissions if the last transfer was equal
to the maximum transfer size.

URBs can be cancelled, but if the URB is already being processed it will
be cancelled after the current transmission within the URB is finished.
If it is still in the queue it will immediately be removed from the
queue.

Testing procedure

tests/usbus_cdc_ecm should still work. Testing one of the usbdev-supported platform should be sufficient here.

Issues/PRs references

Needs #17064

bergzand · 2021-11-17T08:24:16Z

rebased, no longer waiting for other PRs

bergzand · 2022-10-05T12:28:22Z

Do be honest, I have no idea what the actual benefit of this feature is - since I don't know anything about USBUS. However, I realized that the PR increases ROM by approx. 0.5kB and RAM by at least 50B, so I'm just wondering if it would be worth the effort to make this feature optional?

It is useful for any kind of data transfer over USB where the payload to be transferred is larger than the maximum endpoint size (64 bytes) and must be split into multiple transfers. The URB code here moves the state machine that would have to be implemented by the class code into the main USBUS code by allowing to submit a single URB with a length larger than what the underlying endpoint can handle in a single transfer and be notified only when the whole set of transfers have been completed.

How do I test this? What are the expected results before / after this patch?

The CDC ECM code makes use of this to receive the full ethernet frame from the host with a single call to usbus. I expect that one to still work. I haven't found a good test case yet for IN type transfers testing the ZLP functionality.

I haven't found other use cases so far, but I've heard that @dylad might want to use this for his msc implementation.

riot-ci · 2022-10-05T12:45:31Z

Murdock results

✔️ PASSED

ba88608 usbus/cdc_ecm: Make use of URBs for inbound frames

Success	Failures	Total	Runtime
6865	0	6865	08m:25s

Artifacts

Documentation preview

dylad · 2022-10-05T12:49:41Z

I haven't found other use cases so far, but I've heard that @dylad might want to use this for his msc implementation.

One day, I swear, I will update this PR...

sys/include/usb/usbus.h

sys/usb/usbus/usbus.c

benpicco

Please squash

bergzand · 2023-02-27T09:05:17Z

Squashed, thanks for the fixups

benpicco · 2023-02-27T09:08:04Z

still needs a rebase

This commit adds support for URBs (USB Request/Response Blocks). These allow for submitting multi-transfer sized buffers with USBUS handling the individual usbdev xmits. Multiple URBs can be queued at once for a single endpoint and USBUS will handle them in the order of submission. OUT endpoint URBs must always consist of a whole number of full-sized transfers (N x MaxEndpointSize). They will automatically finish after the endpoint received a transfer less than the endpoint size. IN endpoints can be arbitrary-sized and do not have to consist of a whole number of full-sized transmissions. They support a flag to indicate that the last transfer in the sequence must be less than a full sized transfer (USBUS_URB_FLAG_AUTO_ZLP) and this adds a zero length transfer at the end of the transmissions if the last transfer was equal to the maximum transfer size. URBs can be cancelled, but if the URB is already being processed it will be cancelled after the current transmission within the URB is finished. If it is still in the queue it will immediately be removed from the queue.

bergzand · 2023-02-27T09:10:08Z

And rebased

benpicco · 2023-02-27T11:56:48Z

Bors merge

bors · 2023-02-27T13:41:59Z

Build succeeded:

19371: sys/usbus: check for the number of required and provided EPs in static configurations r=kaspar030 a=gschorcht ### Contribution description This PR provides a static check at compile time whether the number of EPs required in a static configuration does not exceed the number of EPs provided by the USB device. #### Background In issue #19359 the problem was reported that `usbus_cdc_ecm` didn't work together with `stdio_cdc_acm` on some STM32 boards. The reason for some of the boards was simply that the application tried to allocate more EPs than available and simply ignored this and just didn't work. #### Solution Since `auto_init_usb` uses a static configuration with exactly one USBUS stack instance and one USB device, at least in case `auto_init` is used a static check can be carried out to make sure that the number of EPs required by the application doesn't exceed the number of EPs provided by the USB device. For this purpose, each `usbus_*` module defines the number of IN and OUT EPs required by that module. Each USB device driver defines the number of EPs provided by USB device if it differs from the default of 8 EPs. During the auto initialization the total number of required IN and OUT EPs is then compared with the number of EPs provided by the USB device using a static assert. ### Testing procedure 1. Green CI 2. Compilation of ```python USEMODULE='stdio_cdc_acm' BOARD=nucleo-f439zi make -j8 -C tests/usbus_cdc_ecm ``` should lead to compilation error ```python sys/auto_init/usb/auto_init_usb.c:81:1: error: static assertion failed: "Number of required IN endpoints exceeded" _Static_assert(USBUS_EP_IN_REQUIRED_NUMOF <= USBDEV_NUM_ENDPOINTS, ^~~~~~~~~~~~~~ Makefile.base:146: recipe for target 'tests/usbus_cdc_ecm/bin/nucleo-f439zi/auto_init_usbus/auto_init_usb.o' failed ``` while compilation of ``` USEMODULE='stdio_cdc_acm' BOARD=nucleo-f767zi make -j8 -C tests/usbus_cdc_ecm ``` should work. ### Issues/PRs references Fixes issue #19359 partially. 19382: tests/pkg_nanors: use static allocation r=kaspar030 a=benpicco 19388: drivers/usbdev_synopsys_dwc2: disable DMA mode r=kaspar030 a=gschorcht ### Contribution description This PR disables the DMA mode for HS cores due to several problems found: - The STALL bit of the OUT control endpoint does not seem to be cleared automatically on the next SETUP received. At least the USB OTG HS core does not generate an interrupt on the next SETUP received. This happens, for example, when CDC ACM is used and the host sends the `SET_LINE_CODING` request which is answered with setting the STALL bit of the OUT endpoint. In this case the enumeration of further interfaces, for example CDC ECM, is stopped. This problem was described in #17085 (comment) - The Enumeration fails for CDC ECM interface which uses URB support (PR #17091) ### Testing procedure Use a STM32 board with USH OTG HS interface: ```python USEMODULE='stdio_cdc_acm periph_usbdev_hs_utmi' BOARD=stm32f723e-disco make -j8 -C tests/usbus_cdc_ecm flash USEMODULE='stdio_cdc_acm periph_usbdev_hs_ulpi' BOARD=stm32f746g-disco make -j8 -C tests/usbus_cdc_ecm flash ``` Without this PR, either the enumeration completely fails (mostly for `stm32f723e-disco`) ```python [377629.753895] usb 1-2.3: new high-speed USB device number 76 using xhci_hcd [377629.854349] usb 1-2.3: device descriptor read/all, error -71 [377629.937990] usb 1-2.3: new high-speed USB device number 77 using xhci_hcd [377630.038261] usb 1-2.3: device descriptor read/all, error -71 [377630.038711] usb 1-2-port3: attempt power cycle [377630.641970] usb 1-2.3: new high-speed USB device number 78 using xhci_hcd [377630.666066] usb 1-2.3: device descriptor read/8, error -71 [377630.794076] usb 1-2.3: device descriptor read/8, error -71 [377630.981806] usb 1-2.3: new high-speed USB device number 79 using xhci_hcd [377631.002092] usb 1-2.3: device descriptor read/8, error -71 [377631.130091] usb 1-2.3: device descriptor read/8, error -71 [377631.238344] usb 1-2-port3: unable to enumerate USB device ``` or the enumeration of the CDC ECM interface stops with error. ```python [377972.828168] usb 1-2.3: new high-speed USB device number 100 using xhci_hcd [377972.928762] usb 1-2.3: config 1 interface 0 altsetting 0 endpoint 0x81 has an invalid bInterval 255, changing to 11 [377972.928765] usb 1-2.3: config 1 interface 1 altsetting 0 bulk endpoint 0x1 has invalid maxpacket 64 [377972.928767] usb 1-2.3: config 1 interface 1 altsetting 0 bulk endpoint 0x82 has invalid maxpacket 64 [377972.929225] usb 1-2.3: New USB device found, idVendor=1209, idProduct=7d00, bcdDevice= 1.00 [377972.929228] usb 1-2.3: New USB device strings: Mfr=3, Product=2, SerialNumber=4 [377972.929230] usb 1-2.3: Product: stm32f723e-disco [377972.929232] usb 1-2.3: Manufacturer: RIOT-os.org [377972.929233] usb 1-2.3: SerialNumber: A6BAC4E1B1E0806B [377972.932399] cdc_acm 1-2.3:1.0: ttyACM1: USB ACM device [377972.933905] cdc_ether: probe of 1-2.3:1.2 failed with error -32 [377973.184377] usb 1-4.3.4: reset high-speed USB device number 32 using xhci_hcd ``` With this PR the enumeration should work as it should: ```python [378480.097974] usb 1-4.3.4: reset high-speed USB device number 32 using xhci_hcd [378484.289762] usb 1-2.3: new high-speed USB device number 16 using xhci_hcd [378484.394638] usb 1-2.3: config 1 interface 0 altsetting 0 endpoint 0x81 has an invalid bInterval 255, changing to 11 [378484.394642] usb 1-2.3: config 1 interface 1 altsetting 0 bulk endpoint 0x1 has invalid maxpacket 64 [378484.394644] usb 1-2.3: config 1 interface 1 altsetting 0 bulk endpoint 0x82 has invalid maxpacket 64 [378484.395296] usb 1-2.3: New USB device found, idVendor=1209, idProduct=7d00, bcdDevice= 1.00 [378484.395299] usb 1-2.3: New USB device strings: Mfr=3, Product=2, SerialNumber=4 [378484.395301] usb 1-2.3: Product: stm32f723e-disco [378484.395303] usb 1-2.3: Manufacturer: RIOT-os.org [378484.395304] usb 1-2.3: SerialNumber: A6BAC4E1B1E0806B [378484.398547] cdc_acm 1-2.3:1.0: ttyACM1: USB ACM device [378484.401007] cdc_ether 1-2.3:1.2 usb0: register 'cdc_ether' at usb-0000:00:14.0-2.3, CDC Ethernet Device, e6:75:97:3a:74:ba [378484.449870] cdc_ether 1-2.3:1.2 enp0s20f0u2u3i2: renamed from usb0 ``` ### Issues/PRs references Co-authored-by: Gunar Schorcht <[email protected]> Co-authored-by: Benjamin Valentin <[email protected]>

19388: drivers/usbdev_synopsys_dwc2: disable DMA mode r=gschorcht a=gschorcht ### Contribution description This PR disables the DMA mode for HS cores due to several problems found: - The STALL bit of the OUT control endpoint does not seem to be cleared automatically on the next SETUP received. At least the USB OTG HS core does not generate an interrupt on the next SETUP received. This happens, for example, when CDC ACM is used and the host sends the `SET_LINE_CODING` request which is answered with setting the STALL bit of the OUT endpoint. In this case the enumeration of further interfaces, for example CDC ECM, is stopped. This problem was described in #17085 (comment) - The Enumeration fails for CDC ECM interface which uses URB support (PR #17091) ### Testing procedure Use a STM32 board with USH OTG HS interface: ```python USEMODULE='stdio_cdc_acm periph_usbdev_hs_utmi' BOARD=stm32f723e-disco make -j8 -C tests/usbus_cdc_ecm flash USEMODULE='stdio_cdc_acm periph_usbdev_hs_ulpi' BOARD=stm32f746g-disco make -j8 -C tests/usbus_cdc_ecm flash ``` Without this PR, either the enumeration completely fails (mostly for `stm32f723e-disco`) ```python [377629.753895] usb 1-2.3: new high-speed USB device number 76 using xhci_hcd [377629.854349] usb 1-2.3: device descriptor read/all, error -71 [377629.937990] usb 1-2.3: new high-speed USB device number 77 using xhci_hcd [377630.038261] usb 1-2.3: device descriptor read/all, error -71 [377630.038711] usb 1-2-port3: attempt power cycle [377630.641970] usb 1-2.3: new high-speed USB device number 78 using xhci_hcd [377630.666066] usb 1-2.3: device descriptor read/8, error -71 [377630.794076] usb 1-2.3: device descriptor read/8, error -71 [377630.981806] usb 1-2.3: new high-speed USB device number 79 using xhci_hcd [377631.002092] usb 1-2.3: device descriptor read/8, error -71 [377631.130091] usb 1-2.3: device descriptor read/8, error -71 [377631.238344] usb 1-2-port3: unable to enumerate USB device ``` or the enumeration of the CDC ECM interface stops with error. ```python [377972.828168] usb 1-2.3: new high-speed USB device number 100 using xhci_hcd [377972.928762] usb 1-2.3: config 1 interface 0 altsetting 0 endpoint 0x81 has an invalid bInterval 255, changing to 11 [377972.928765] usb 1-2.3: config 1 interface 1 altsetting 0 bulk endpoint 0x1 has invalid maxpacket 64 [377972.928767] usb 1-2.3: config 1 interface 1 altsetting 0 bulk endpoint 0x82 has invalid maxpacket 64 [377972.929225] usb 1-2.3: New USB device found, idVendor=1209, idProduct=7d00, bcdDevice= 1.00 [377972.929228] usb 1-2.3: New USB device strings: Mfr=3, Product=2, SerialNumber=4 [377972.929230] usb 1-2.3: Product: stm32f723e-disco [377972.929232] usb 1-2.3: Manufacturer: RIOT-os.org [377972.929233] usb 1-2.3: SerialNumber: A6BAC4E1B1E0806B [377972.932399] cdc_acm 1-2.3:1.0: ttyACM1: USB ACM device [377972.933905] cdc_ether: probe of 1-2.3:1.2 failed with error -32 [377973.184377] usb 1-4.3.4: reset high-speed USB device number 32 using xhci_hcd ``` With this PR the enumeration should work as it should: ```python [378480.097974] usb 1-4.3.4: reset high-speed USB device number 32 using xhci_hcd [378484.289762] usb 1-2.3: new high-speed USB device number 16 using xhci_hcd [378484.394638] usb 1-2.3: config 1 interface 0 altsetting 0 endpoint 0x81 has an invalid bInterval 255, changing to 11 [378484.394642] usb 1-2.3: config 1 interface 1 altsetting 0 bulk endpoint 0x1 has invalid maxpacket 64 [378484.394644] usb 1-2.3: config 1 interface 1 altsetting 0 bulk endpoint 0x82 has invalid maxpacket 64 [378484.395296] usb 1-2.3: New USB device found, idVendor=1209, idProduct=7d00, bcdDevice= 1.00 [378484.395299] usb 1-2.3: New USB device strings: Mfr=3, Product=2, SerialNumber=4 [378484.395301] usb 1-2.3: Product: stm32f723e-disco [378484.395303] usb 1-2.3: Manufacturer: RIOT-os.org [378484.395304] usb 1-2.3: SerialNumber: A6BAC4E1B1E0806B [378484.398547] cdc_acm 1-2.3:1.0: ttyACM1: USB ACM device [378484.401007] cdc_ether 1-2.3:1.2 usb0: register 'cdc_ether' at usb-0000:00:14.0-2.3, CDC Ethernet Device, e6:75:97:3a:74:ba [378484.449870] cdc_ether 1-2.3:1.2 enp0s20f0u2u3i2: renamed from usb0 ``` ### Issues/PRs references Co-authored-by: Gunar Schorcht <[email protected]>

benpicco · 2023-05-24T16:21:36Z

sys/usb/usbus/usbus.c

+void usbus_urb_submit(usbus_t *usbus, usbus_endpoint_t *endpoint, usbus_urb_t *urb)
+{
+    (void)usbus;
+    assert(!(clist_find(&endpoint->urb_list, &urb->list)));


This assertion triggers when the host reboots #19663 😕

bergzand added the Type: new feature The issue requests / The PR implemements a new feature for RIOT label Oct 31, 2021

bergzand requested review from aabadie, benpicco, DipSwitch, dylad, fjmolinas, haukepetersen, keestux, leandrolanzieri, MichelRottleuthner, MrKevinWeiss, smlng and vincent-d as code owners October 31, 2021 20:42

github-actions bot added Area: cpu Area: CPU/MCU ports Area: drivers Area: Device drivers Area: sys Area: System Area: tests Area: tests and testing framework Area: USB Area: Universal Serial Bus Platform: ARM Platform: This PR/issue effects ARM-based platforms labels Oct 31, 2021

bergzand force-pushed the pr/usbus/queue branch from 16a98b1 to 4b9278a Compare October 31, 2021 21:55

bergzand added the State: waiting for other PR State: The PR requires another PR to be merged first label Nov 4, 2021

bergzand force-pushed the pr/usbus/queue branch from 4b9278a to 963f7dd Compare November 17, 2021 08:23

bergzand removed the State: waiting for other PR State: The PR requires another PR to be merged first label Nov 17, 2021

github-actions bot removed Platform: ARM Platform: This PR/issue effects ARM-based platforms Area: drivers Area: Device drivers Area: tests Area: tests and testing framework Area: cpu Area: CPU/MCU ports labels Nov 17, 2021

dylad self-assigned this Nov 17, 2021

dylad added this to the Release 2022.01 milestone Nov 17, 2021

aabadie added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Oct 5, 2022

benpicco removed the State: waiting for author State: Action by the author of the PR is required label Oct 5, 2022

benpicco reviewed Oct 19, 2022

View reviewed changes

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

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

github-actions bot added the Area: build system Area: Build system label Feb 25, 2023

benpicco approved these changes Feb 25, 2023

View reviewed changes

benpicco force-pushed the pr/usbus/queue branch from cdb4c3d to 703dc6e Compare February 25, 2023 23:35

dylad mentioned this pull request Feb 26, 2023

usbus/msc: add initial Mass Storage Class support #19242

Merged

bergzand force-pushed the pr/usbus/queue branch from 703dc6e to f9a11bf Compare February 27, 2023 09:04

bergzand added 2 commits February 27, 2023 10:09

usbus/cdc_ecm: Make use of URBs for inbound frames

ba88608

bergzand force-pushed the pr/usbus/queue branch from f9a11bf to ba88608 Compare February 27, 2023 09:10

bors bot merged commit f0b60d5 into RIOT-OS:master Feb 27, 2023

gschorcht mentioned this pull request Mar 14, 2023

drivers/usbdev_synopsys_dwc2: disable DMA mode #19388

Merged

MrKevinWeiss added this to the Release 2023.04 milestone Apr 25, 2023

benpicco reviewed May 24, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

USBUS: Add URB support #17091

USBUS: Add URB support #17091

bergzand commented Oct 31, 2021

bergzand commented Nov 17, 2021

bergzand commented Oct 5, 2022

riot-ci commented Oct 5, 2022 •

edited

Loading

dylad commented Oct 5, 2022

benpicco left a comment

bergzand commented Feb 27, 2023

benpicco commented Feb 27, 2023

bergzand commented Feb 27, 2023

benpicco commented Feb 27, 2023

bors bot commented Feb 27, 2023

benpicco May 24, 2023

USBUS: Add URB support #17091

USBUS: Add URB support #17091

Conversation

bergzand commented Oct 31, 2021

Contribution description

Testing procedure

Issues/PRs references

bergzand commented Nov 17, 2021

bergzand commented Oct 5, 2022

riot-ci commented Oct 5, 2022 • edited Loading

Murdock results

Artifacts

dylad commented Oct 5, 2022

benpicco left a comment

Choose a reason for hiding this comment

bergzand commented Feb 27, 2023

benpicco commented Feb 27, 2023

bergzand commented Feb 27, 2023

benpicco commented Feb 27, 2023

bors bot commented Feb 27, 2023

benpicco May 24, 2023

Choose a reason for hiding this comment

riot-ci commented Oct 5, 2022 •

edited

Loading