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.

Sign up for GitHub

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

Jump to bottom

pkg/tinyusb: support for multiple configuration descriptors #18983

Merged
merged 3 commits into from
Dec 6, 2022
Merged

pkg/tinyusb: support for multiple configuration descriptors #18983

merged 3 commits into from
Dec 6, 2022

Conversation

gschorcht
Copy link
Contributor

Contribution description

This PR extends the common USB descriptor implementation of the tinyUSB package with support for multiple device configuration descriptors.

Some device classes require the definition of multiple device configuration descriptors to support different protocols over the same interface. This is the case, for example, with USB network devices that support CDC ECM and RNDIS as protocols to use on Linux and Windows (Windows supports only RNDIS).

With this PR the following changes for the common USB descriptor implementation of the tinyUSB package are provided:

  • Replace the direct use of tinyUSB desciptor macros by paramterized macros to simplify the definition of multiple configuration descriptors (5d96a44).
  • Move remaining CONFIG_* symbols to tinyusb_descriptors.h to be visible in all source files (b9449b0).
  • Add debug output to some descriptor callbacks (b9449b0).
  • Change the conditional definitions of CFG_* symbols to unconditional definitions to ensure that Kconfig CONFIG_* symbols are always used.
  • Add support for a second device configuration descriptor (fe2f7c8). This could theoretically be used for any number of device configuration descriptors, but is currently limited to two.

This PR is now prerequisite for tinyUSB netdev in PR #18903 and the upcoming PR for tinyUSB DFU bootloader.

Testing procedure

  • Compilation in CI should succeed.
  • tests/pkg_tinyusb_cdc_acm_stdio should still succeed for one board.
  • tests/pkg_tinyusb_cdc_msc should still succeed for one board.

Issues/PRs references

Prerequisite for PR #18903

@github-actions github-actions bot added Area: Kconfig Area: Kconfig integration Area: pkg Area: External package ports labels Nov 27, 2022
@gschorcht gschorcht added Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Nov 27, 2022
@gschorcht gschorcht requested a review from benpicco November 27, 2022 10:32
@riot-ci
Copy link

riot-ci commented Nov 27, 2022 •
edited
Loading

Murdock results

✔️ PASSED

d66716f pkg/tinyusb: support second device configuration descriptor

Success Failures Total Runtime
117881 0 117882 01h:55m:20s

Artifacts

@gschorcht
pkg/tinyusb: parameterized descriptor macros
bd5ce7b
@gschorcht
pkg/tinyusb: some cleanup of configuration and descriptors
91c171a 
The commit includes the following changes:
- Remaining CONFIG_* symbols are moved to tinyusb_descriptors.h to be visible in all source files.
- Debug output is added to some descriptor callbacks.
- The conditional definitions of CFG_* symbols are changed to unconditional definitions to ensure that Kconfig CONFIG_* symbols are always used.
@gschorcht
pkg/tinyusb: support second device configuration descriptor
d66716f 
An alternative device configuration descriptor is required if multiple protocols, e.g. CDC ECM and RNDIS, should be used with same device interface. This commit is a prerequisite for the support of tinyusb netdev driver.
@gschorcht gschorcht force-pushed the pkg/tinyusb_multiple_config_descriptors branch from fe2f7c8 to d66716f Compare December 4, 2022 16:11
This was referenced Dec 5, 2022
Merged
Merged
@benpicco benpicco merged commit 64c49df into RIOT-OS:master Dec 6, 2022
@gschorcht
Copy link
Contributor Author

@benpicco Thanks for reviewing and merging.

@gschorcht gschorcht deleted the pkg/tinyusb_multiple_config_descriptors branch December 6, 2022 17:41
bors bot added a commit that referenced this pull request Jan 15, 2023
@bors @gschorcht @chrysn
Merge #19010 #19149
9ff9704 
19010: bootloaders/riotboot: add tinyUSB DFU support r=benpicco a=gschorcht

### Contribution description

This PR provides
- the tinyUSB DFU and DFU Runtime support and
- the `riotboot_tinyusb_dfu` bootloader that uses the tinyUSB DFU mode to flash new application images.

~This PR includes PR #18983 for now to be compilable.~

### Testing procedure

1. Use any board that supports the `riotboot´ and `tinyusb_device` features and flash the bootloader first, for example
   ```
   BOARD=nucleo-f767zi make -C bootloaders/riotboot_tinyusb_dfu flash
   ```
   and check that the `riotboot_tinyusb_dfu` bootloader is in DFU mode:
   ```
   dfu-util --list
   ```
3. Flash a first application using the following command:
    ```
   FEATURES_REQUIRED=riotboot USEMODULE=tinyusb_dfu BOARD=nucleo-f767zi \
   make -C tests/saul PROGRAMMER=dfu-util riotboot/flash-slot0
   ```
   and check that the application starts and is seen as upgradable:
   ```
   dfu-util --list
   ```
4. Restart the node in bootloader DFU mode by:
   ```
   dfu-util -e
   ```
   Flash a second application, for example
   ```
   FEATURES_REQUIRED=riotboot USEMODULE=tinyusb_dfu BOARD=nucleo-f767zi \
   make -C tests/shell PROGRAMMER=dfu-util riotboot/flash-slot1
   ```
   and check that the second application starts and is seen as upgradable:
   ```
   dfu-util --list
   ```
   
### Issues/PRs references

~Depends on PR #18983~

19149: SECURITY: Describe that declassification is an option r=benpicco a=chrysn

### Contribution description

Our security policy does not contain provisions for the case when what is reported is not what we consider an actual security issue. As it is described now, everything reported through security@ would go through the full treatment, including a point release.

I'm not sure it belongs into the text itself (as it's more about how security reporters interact with the project than internals), but declassification should IMO be backed at least by 3 maintainers, and no strong NACK.

### Issues/PRs references

#19141 followed that procedure after some chat on it on the maintainers channel. (In the discussion, I proposed declassification, with 2.5 people supporting it and one "I was about to, but can we be sure nobody is using it?" voice).

Co-authored-by: Gunar Schorcht <gunar@schorcht.net>
Co-authored-by: chrysn <chrysn@fsfe.org>
@kaspar030 kaspar030 added this to the Release 2023.01 milestone Jan 19, 2023
bors bot added a commit that referenced this pull request Feb 3, 2023
@bors @gschorcht
Merge #18903
0fb6a09 
18903: pkg/tinyusb: add tinyUSB netdev driver r=dylad a=gschorcht

### Contribution description

This PR adds the tinyUSB netdev driver.

The tinyUSB netdev driver is part of the tinyUSB package and is enabled by module `tinyusb_netdev`. It is available for boards that provide the `tinyusb_device` feature.

**Please note** Since the tinyUSB package is distinct from (and incompatible with) the USB stack provided around USBUS in RIOT (see USB), the tinyUSB netdev driver cannot be used together with with any USBUS device class.

The tinyUSB netdev driver uses Ethernet over USB and supports the following protocols:
- CDC ECM (Ethernet Control Model)
- CDC NCM (Network Control Model)
- RNDIS (Microsoft Remote NDIS)

While Linux and macOS support all these protocols, Microsoft Windows only supports the RNDIS protocol and since Windows version 11 also the CDC NCM protocol. macOS supports the RNDIS protocol since version 10.15 (Catalina).

Which protocol is used is selected by the corresponding pseudomodules `tinyusb_class_net_cdc_ecm`, `tinyusb_class_net_cdc_ncm` and `tinyusb_class_net_rndis`.

The CDC ECM protocol (`tinyusb_class_net_cdc_ecm`) and the RNDIS protocol (`tinyusb_class_net_rndis`) can be used simultaneously to support all operating systems, for example :
```
USEMODULE='tinyusb_netdev tinyusb_class_net_rndis tinyusb_class_net_cdc_ecm' \
BOARD=... make -C ... flash
 ```
In this case, the CDC ECM protocol is the default protocol and the RNDIS protocol the alternative protocol defined as second device configuration. The CDC NCM protocol cannot be used together with the CDC ECM or the RNDIS protocol.

This PR includes PR #18983 for now to be compilable. 

Comparison with USBUS CDC ECM (`nucleo-f767zi` board):
```
   text	   data	    bss	    dec	    hex	filename
  65916	    596	  18728	  85240	  14cf8	tests_pkg_tinyusb_netdev.elf
```
```
   text	   data	    bss	    dec	    hex	filename
  63120	    544	  15444	  79108	  13504	tests_usbus_cdc_ecm.elf

```

### Testing procedure

Use a board that is supported by tinyUSB. Compile and flash the test application for each protocol:

1. RNDIS
    ```
    BOARD=... make -j8 -C tests/pkg_tinyusb_netdev flash
    ```
2. CDC ECM
    ```
    CLASS=tinyusb_class_net_cdc_ecm BOARD=... make -j8 -C tests/pkg_tinyusb_netdev flash
    ```
3. CDC NCM
    ```
    CLASS=tinyusb_class_net_cdc_ncm BOARD=... make -j8 -C tests/pkg_tinyusb_netdev flash
    ```
For each test, a network interface should be added on the host. Use command `ifconfig` on USB device and on the host and check that both have a link local address. In syslog there should be an output like the following:
<details>

```
Nov 13 18:14:46 gunny8 kernel: [4611465.480025] usb 1-2.2: new full-speed USB device number 28 using xhci_hcd
Nov 13 18:14:47 gunny8 kernel: [4611465.581641] usb 1-2.2: New USB device found, idVendor=1209, idProduct=7d01, bcdDevice= 1.00
Nov 13 18:14:47 gunny8 kernel: [4611465.581646] usb 1-2.2: New USB device strings: Mfr=1, Product=2, SerialNumber=3
Nov 13 18:14:47 gunny8 kernel: [4611465.581650] usb 1-2.2: Product: nucleo-f767zi
Nov 13 18:14:47 gunny8 kernel: [4611465.581653] usb 1-2.2: Manufacturer: RIOT-os.org
Nov 13 18:14:47 gunny8 kernel: [4611465.581654] usb 1-2.2: SerialNumber: 6591620BCB270283
Nov 13 18:14:47 gunny8 vmnetBridge: RTM_NEWLINK: name:usb0 index:508 flags:0x00001002
Nov 13 18:14:47 gunny8 vmnet-natd: RTM_NEWLINK: name:usb0 index:508 flags:0x00001002
Nov 13 18:14:47 gunny8 NetworkManager[24229]: <info>  [1668359687.1066] manager: (usb0): new Ethernet device (/org/freedesktop/NetworkManager/Devices/528)
Nov 13 18:14:47 gunny8 kernel: [4611465.594604] rndis_host 1-2.2:1.0 usb0: register 'rndis_host' at usb-0000:00:14.0-2.2, RNDIS device, fa:db:7c:1b:58:80
Nov 13 18:14:47 gunny8 mtp-probe: checking bus 1, device 28: "/sys/devices/pci0000:00/0000:00:14.0/usb1/1-2/1-2.2"
Nov 13 18:14:47 gunny8 mtp-probe: bus: 1, device: 28 was not an MTP device
Nov 13 18:14:47 gunny8 systemd-udevd[17796]: link_config: autonegotiation is unset or enabled, the speed and duplex are not writable.
Nov 13 18:14:47 gunny8 vmnet-natd: RTM_NEWLINK: name:usb0 index:508 flags:0x00001002
Nov 13 18:14:47 gunny8 kernel: [4611465.643852] rndis_host 1-2.2:1.0 enp0s20f0u2u2: renamed from usb0
Nov 13 18:14:47 gunny8 vmnetBridge: RTM_NEWLINK: name:usb0 index:508 flags:0x00001002
Nov 13 18:14:47 gunny8 vmnet-natd: RTM_NEWLINK: name:enp0s20f0u2u2 index:508 flags:0x00001002
Nov 13 18:14:47 gunny8 vmnetBridge: RTM_NEWLINK: name:enp0s20f0u2u2 index:508 flags:0x00001002
Nov 13 18:14:47 gunny8 NetworkManager[24229]: <info>  [1668359687.1833] device (usb0): interface index 508 renamed iface from 'usb0' to 'enp0s20f0u2u2'
Nov 13 18:14:47 gunny8 upowerd[2845]: unhandled action 'bind' on /sys/devices/pci0000:00/0000:00:14.0/usb1/1-2/1-2.2/1-2.2:1.1
Nov 13 18:14:47 gunny8 NetworkManager[24229]: <info>  [1668359687.2037] device (enp0s20f0u2u2): state change: unmanaged -> unavailable (reason 'managed', sys-iface-state: 'external')
Nov 13 18:14:47 gunny8 vmnet-natd: RTM_NEWLINK: name:enp0s20f0u2u2 index:508 flags:0x00011043
Nov 13 18:14:47 gunny8 vmnetBridge: RTM_NEWLINK: name:enp0s20f0u2u2 index:508 flags:0x00011043
Nov 13 18:14:47 gunny8 vmnetBridge: Adding interface enp0s20f0u2u2 index:508
Nov 13 18:14:47 gunny8 NetworkManager[24229]: <info>  [1668359687.2075] device (enp0s20f0u2u2): carrier: link connected
Nov 13 18:14:47 gunny8 upowerd[2845]: unhandled action 'bind' on /sys/devices/pci0000:00/0000:00:14.0/usb1/1-2/1-2.2/1-2.2:1.0
Nov 13 18:14:47 gunny8 NetworkManager[24229]: <info>  [1668359687.2129] settings: (enp0s20f0u2u2): created default wired connection 'Kabelgebundene Verbindung 2'
Nov 13 18:14:47 gunny8 NetworkManager[24229]: <warn>  [1668359687.2142] device (enp0s20f0u2u2): connectivity: "/proc/sys/net/ipv4/conf/enp0s20f0u2u2/rp_filter" is set to "1". This might break connectivity checking for IPv4 on this device
Nov 13 18:14:47 gunny8 NetworkManager[24229]: <info>  [1668359687.2151] device (enp0s20f0u2u2): state change: unavailable -> disconnected (reason 'none', sys-iface-state: 'managed')
Nov 13 18:14:47 gunny8 vmnetBridge: RTM_NEWLINK: name:enp0s20f0u2u2 index:508 flags:0x00011043
Nov 13 18:14:47 gunny8 vmnet-natd: RTM_NEWLINK: name:enp0s20f0u2u2 index:508 flags:0x00011043
Nov 13 18:14:47 gunny8 upowerd[2845]: unhandled action 'bind' on /sys/devices/pci0000:00/0000:00:14.0/usb1/1-2/1-2.2
Nov 13 18:14:47 gunny8 systemd-udevd[17796]: link_config: autonegotiation is unset or enabled, the speed and duplex are not writable.
Nov 13 18:14:47 gunny8 NetworkManager[24229]: <info>  [1668359687.2403] policy: auto-activating connection 'Kabelgebundene Verbindung 2' (0b1ae45e-c76e-3efb-a2cd-138ca2b2a59c)
Nov 13 18:14:47 gunny8 NetworkManager[24229]: <info>  [1668359687.2414] device (enp0s20f0u2u2): Activation: starting connection 'Kabelgebundene Verbindung 2' (0b1ae45e-c76e-3efb-a2cd-138ca2b2a59c)
Nov 13 18:14:47 gunny8 NetworkManager[24229]: <info>  [1668359687.2419] device (enp0s20f0u2u2): state change: disconnected -> prepare (reason 'none', sys-iface-state: 'managed')
Nov 13 18:14:47 gunny8 NetworkManager[24229]: <info>  [1668359687.2429] device (enp0s20f0u2u2): state change: prepare -> config (reason 'none', sys-iface-state: 'managed')
Nov 13 18:14:47 gunny8 NetworkManager[24229]: <info>  [1668359687.2440] device (enp0s20f0u2u2): state change: config -> ip-config (reason 'none', sys-iface-state: 'managed')
Nov 13 18:14:47 gunny8 NetworkManager[24229]: <info>  [1668359687.2445] dhcp4 (enp0s20f0u2u2): activation: beginning transaction (timeout in 45 seconds)
Nov 13 18:14:47 gunny8 vmnetBridge: RTM_NEWLINK: name:enp0s20f0u2u2 index:508 flags:0x00011043
Nov 13 18:14:47 gunny8 vmnet-natd: RTM_NEWLINK: name:enp0s20f0u2u2 index:508 flags:0x00011043
Nov 13 18:14:47 gunny8 avahi-daemon[1464]: Joining mDNS multicast group on interface enp0s20f0u2u2.IPv6 with address fe80::dba4:adb8:9ffe:d93e.
Nov 13 18:14:47 gunny8 avahi-daemon[1464]: New relevant interface enp0s20f0u2u2.IPv6 for mDNS.
Nov 13 18:14:47 gunny8 avahi-daemon[1464]: Registering new address record for fe80::dba4:adb8:9ffe:d93e on enp0s20f0u2u2.*.
Nov 13 18:14:47 gunny8 kernel: [4611465.895046] userif-1: sent link down event.
Nov 13 18:14:47 gunny8 kernel: [4611465.895052] userif-1: sent link up event.
```

</details>

Ping from and to the host.

### Issues/PRs references

Depends on PR #18983

Co-authored-by: Gunar Schorcht <gunar@schorcht.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@benpicco benpicco benpicco approved these changes

@leandrolanzieri leandrolanzieri Awaiting requested review from leandrolanzieri leandrolanzieri is a code owner

@jia200x jia200x Awaiting requested review from jia200x jia200x is a code owner

@MrKevinWeiss MrKevinWeiss Awaiting requested review from MrKevinWeiss MrKevinWeiss is a code owner

Assignees
No one assigned
Labels
Area: Kconfig Area: Kconfig integration Area: pkg Area: External package ports CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation
Projects
None yet
Milestone
Release 2023.01
Development

Successfully merging this pull request may close these issues.
4 participants
@gschorcht @riot-ci @benpicco @kaspar030