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: USB CDC NCM class implementation #79508

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

jukkar
Copy link
Member

@jukkar jukkar commented Oct 7, 2024

This implements USB Network Control Model (NCM) driver for the USB next stack.

Following things are not implemented (yet):

  • ntb32 format support
  • Sending more than 1 Ethernet frames / USB packet to host. Note that it is possible to receive more than one Ethernet frames / USB packet to device.

How to try this:

west build -p -b nrf5340dk/nrf5340/cpuapp -d ../build/usb-next-zperf samples/net/zperf/ -- \
-DEXTRA_DTC_OVERLAY_FILE="usbd_next_ncm.overlay" \
-DOVERLAY_CONFIG="overlay-usbd_next.conf" \
-DCONFIG_NET_SHELL=y -DCONFIG_NET_LOG=y \
-DCONFIG_NET_L2_ETHERNET=y \
-DCONFIG_USB_DEVICE_STACK_NEXT=y \
-DCONFIG_USBD_SHELL=y \
-DCONFIG_USBD_CDC_NCM_LOG_LEVEL_DBG=n \
-DCONFIG_NET_IF_LOG_LEVEL_DBG=n

@jukkar jukkar force-pushed the devel/usb-ncm branch 2 times, most recently from f299a12 to d1efbbf Compare October 9, 2024 06:33
@jukkar
Copy link
Member Author

jukkar commented Oct 9, 2024

  • CI fixes

rlubos
rlubos previously approved these changes Oct 10, 2024
Copy link
Contributor

@rlubos rlubos left a comment

Choose a reason for hiding this comment

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

Net part looks ok.

subsys/usb/device_next/class/Kconfig.cdc_ncm Outdated Show resolved Hide resolved
subsys/usb/device_next/class/usbd_cdc_ncm.c Outdated Show resolved Hide resolved
subsys/usb/device_next/class/usbd_cdc_ncm.c Outdated Show resolved Hide resolved
subsys/usb/device_next/class/usbd_cdc_ncm.c Outdated Show resolved Hide resolved
subsys/usb/device_next/class/usbd_cdc_ncm.c Outdated Show resolved Hide resolved
subsys/usb/device_next/class/usbd_cdc_ncm.c Outdated Show resolved Hide resolved
@jfischer-no jfischer-no changed the title USB next NCM driver usb: device_next: USB CDC NCM class implementation Oct 15, 2024
@jfischer-no jfischer-no assigned jfischer-no and unassigned kartben Oct 15, 2024
@jukkar
Copy link
Member Author

jukkar commented Oct 15, 2024

  • Updated according to comments
  • Rebased against latest main

Comment on lines 794 to 795
LOG_WRN("New configuration, interface %u alternate %u",
iface, alternate);
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it should be LOG_INF, otherwise we will get bug reports for this.

	LOG_INF("New configuration, interface %u alternate %u",
		iface, alternate);

Comment on lines 917 to 923
case SET_NTB_FORMAT:
LOG_DBG("SET_NTB_FORMAT");
break;

case SET_ETHERNET_PACKET_FILTER:
LOG_DBG("SET_ETHERNET_PACKET_FILTER");
break;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Missed that. Looks like typo, GET_NTB_FORMAT? (Needs to be implemented or removed if not required).

But there is no GET_ETHERNET_PACKET_FILTER.

(Set request, SET_.*, are handled above in the control-to-device requests. You will never get this kind of requests in control-to-host.)

net_buf_add_mem(buf, &input_size, sizeof(input_size));
break;
}
case SET_NTB_INPUT_SIZE:
Copy link
Collaborator

Choose a reason for hiding this comment

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

See above.

Comment on lines 946 to 950
out:
if (ret < 0) {
errno = -ret;
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

It looks like you do not need ret at all.

USB NCM Ethernet driver implementation.

Signed-off-by: Jukka Rissanen <[email protected]>
Add NCM class to sample usbd initialization and the code
triple is set properly.

Signed-off-by: Jukka Rissanen <[email protected]>
Add support for USB cdc_ncm to zperf application.

Signed-off-by: Jukka Rissanen <[email protected]>
Add support for USB cdc_ncm to echo-server application.

Signed-off-by: Jukka Rissanen <[email protected]>
@jukkar
Copy link
Member Author

jukkar commented Oct 15, 2024

  • Updated according to comments

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants