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

usb: device_next: add CDC ECM class implementation #54448

Merged

Conversation

jfischer-no
Copy link
Collaborator

Add CDC Ethernet Control Model class implementation for the
new experimental USB device support based on the existing
implementation subsys/usb/device/class/netusb/function_ecm.c.

The implementation forms virtual ethernet connection between
USB host and USB device and requires two corresponding MAC
addresses, one for the virtual interface on the device side,
and other for the host which is described by a string descriptor.

CDC ECM implementation supports multiple instances which are
specified using DT. The number of instances is limited by the
number of endpoints on the controller, two to three should usually
be possible.

@jfischer-no jfischer-no added area: USB Universal Serial Bus area: Ethernet Experimental Experimental features not enabled by default labels Feb 3, 2023
@jfischer-no jfischer-no added this to the v3.4.0 milestone Feb 3, 2023
@jfischer-no jfischer-no self-assigned this Feb 3, 2023
@zephyrbot zephyrbot added area: Devicetree Binding PR modifies or adds a Device Tree binding area: Networking labels Feb 3, 2023
@jfischer-no jfischer-no force-pushed the pr-device-next-cdc-ecm branch from 4d00db9 to 35666b8 Compare February 24, 2023 13:00
*/
#define CDC_ECM_IMAC_BASE 4

#define CDC_ECM_IFACE_UP 0
Copy link
Contributor

Choose a reason for hiding this comment

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

I think enum fits the purpose here better than defines.

int ret;

if (!atomic_test_bit(&data->state, CDC_ECM_CLASS_ENABLED)) {
return -EPERM;
Copy link
Contributor

Choose a reason for hiding this comment

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

I think ENODEV better reflects the error here.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ENODEV is confusing because there is a device but it can not be used yet, I would like to use EACCES and align it with 562d799#r1126070376

}

if (atomic_test_bit(&data->state, CDC_ECM_CLASS_SUSPENDED)) {
LOG_INF("USB support is suspended (FIXME)");
Copy link
Contributor

Choose a reason for hiding this comment

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

"USB device" is suspended, not "USB support"

if (!atomic_test_bit(&data->state, CDC_ECM_CLASS_ENABLED) ||
!atomic_test_bit(&data->state, CDC_ECM_IFACE_UP)) {
LOG_INF("Configuration is not enabled or interface not ready");
return -EACCES;
Copy link
Contributor

Choose a reason for hiding this comment

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

I think ENODEV is more appropriate here.

#define USBD_CDC_ECM_DT_DEVICE_DEFINE(n) \
CDC_ECM_DEFINE_DESCRIPTOR(n); \
USBD_DESC_STRING_DEFINE(mac_desc_nd_##n, \
DT_INST_PROP_OR(n, mac_address, "00005E005301"),\
Copy link
Contributor

Choose a reason for hiding this comment

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

If MAC address is changed to require property this hardcoded fallback will no longer be necessary.

Add bindings for CDC ECM virtual ethernet controller.

Signed-off-by: Johann Fischer <[email protected]>
Add CDC Ethernet Control Model class implementation for the
new experimental USB device support based on the existing
implementation subsys/usb/device/class/netusb/function_ecm.c.

The implementation forms virtual ethernet connection between
USB host and USB device and requires two corresponding MAC
addresses, one for the virtual interface on the device side,
and other for the host which is described by a string descriptor.
With upcoming changes it should also possible to use a real
ethernet controller as media access on the device side.

CDC ECM implementation supports multiple instances which are
specified using DT. The number of instances is limited by the
number of endpoints on the controller, two to three should usually
be possible.

Signed-off-by: Johann Fischer <[email protected]>
Add DT and Kconfig overlay for new experimental CDC ECM
implementation. USB device support configuration and
initialization can be done through shell by following
commands:

uart:~$ usbd config add 1
uart:~$ usbd class add cdc_ecm_0 1
uart:~$ usbd defaults
uart:~$ usbd init
uart:~$ usbd enable

Signed-off-by: Johann Fischer <[email protected]>
@carlescufi carlescufi merged commit 0929d73 into zephyrproject-rtos:main May 11, 2023
@jfischer-no jfischer-no deleted the pr-device-next-cdc-ecm branch May 11, 2023 09:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Devicetree Binding PR modifies or adds a Device Tree binding area: Ethernet area: Networking area: USB Universal Serial Bus Experimental Experimental features not enabled by default
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants