usb: device_next: support BOS descriptor with vendor request code #76326

jfischer-no · 2024-07-26T12:07:48Z

Support BOS descriptor with vendor request code and add WebUSB sample for the new USB device stack.

Add a function similar to sample_usbd_init_device(), but one that does not initialize the device. It allows the application to set additional features, such as additional descriptors. Signed-off-by: Johann Fischer <johann.fischer@nordicsemi.no>

tmon-nordic · 2024-07-29T07:21:30Z

subsys/usb/device_next/usbd_ch9.c

+{
+	struct usbd_desc_node *desc_nd;
+
+	SYS_DLIST_FOR_EACH_CONTAINER(&uds_ctx->descriptors, desc_nd, node) {


Why abuse the descriptors list? I think it would be better to just have a separate list for the vendor requests. The vendor requests should not be limited to BOS-only, but generally any vendor (here: application developer) should be able to provide their own vendor commands in application.

Why abuse the descriptors list?

It is a descriptor and it does not abusing anything.

I think it would be better to just have a separate list for the vendor requests. The vendor requests should not be limited to BOS-only, but generally any vendor (here: application developer) should be able to provide their own vendor commands in application.

There are no requirements in the specification to allow arbitrary vendor request with recipient device. Using the BOS platform capability is the most portable.

How is this a descriptor? This is not descriptor in any way, just a way for application to register vendor commands. USB specification doesn't require the devices to implement any optional functionality (vendor commands are optional functionality). In this context the specification states "A device vendor may also define requests supported by the device.".

What do you mean by "most portable"? What does portability have to do with a stack functionality to allow application developer to support vendor commands?

Please familiarize yourself with how MSOS2 and WebUSB use the BOS descriptor, I am pretty sure you will understand it.

tmon-nordic · 2024-07-29T07:24:16Z

samples/subsys/usb/webusb/src/msosv2.h

+struct bos_msosv2_descriptor bos_msosv2_desc = {
+	/*
+	 * Microsoft OS 2.0 Platform Capability Descriptor,
+	 * see Microsoft OS 2.0 Descriptors Specification
+	 */
+	.platform = {
+		.bLength = sizeof(struct usb_bos_platform_descriptor)
+			 + sizeof(struct usb_bos_capability_msos),
+		.bDescriptorType = USB_DESC_DEVICE_CAPABILITY,
+		.bDevCapabilityType = USB_BOS_CAPABILITY_PLATFORM,
+		.bReserved = 0,
+		/* Microsoft OS 2.0 descriptor platform capability UUID
+		 * D8DD60DF-4589-4CC7-9CD2-659D9E648A9F
+		 */
+		.PlatformCapabilityUUID = {
+			0xDF, 0x60, 0xDD, 0xD8,
+			0x89, 0x45,
+			0xC7, 0x4C,
+			0x9C, 0xD2,
+			0x65, 0x9D, 0x9E, 0x64, 0x8A, 0x9F,
+		},
+	},
+	.cap = {
+		.dwWindowsVersion = sys_cpu_to_le32(SAMPLE_MSOS2_OS_VERSION),
+		.wMSOSDescriptorSetTotalLength = sys_cpu_to_le16(sizeof(msosv2_desc)),
+		.bMS_VendorCode = SAMPLE_MSOS2_VENDOR_CODE,
+		.bAltEnumCode = 0x00
+	},
+};
+
+static int msosv2_to_host_cb(const struct usbd_context *const ctx,
+			     const struct usb_setup_packet *const setup,
+			     struct net_buf *const buf)
+{
+	LOG_INF("Vendor callback to host");
+
+	if (setup->bRequest == SAMPLE_MSOS2_VENDOR_CODE &&
+	    setup->wIndex == MS_OS_20_DESCRIPTOR_INDEX) {
+		LOG_INF("Get MS OS 2.0 Descriptor Set");
+
+		net_buf_add_mem(buf, &msosv2_desc,
+				MIN(net_buf_tailroom(buf), sizeof(msosv2_desc)));
+
+		return 0;
+	}
+
+	return -ENOTSUP;
+}
+
+USBD_DESC_BOS_VREQ_DEFINE(bos_vreq_msosv2, sizeof(bos_msosv2_desc), &bos_msosv2_desc,
+			  SAMPLE_MSOS2_VENDOR_CODE, msosv2_to_host_cb, NULL);


Why this is defined in a header?

Why not? Or what is the problem? This is limited to a specific part and can be used as a template.

Everything compilation unit that includes this header will have the variables in the corresponding object file.

There is only that include it. So where is the problem?

tmon-nordic · 2024-07-29T07:25:20Z

samples/subsys/usb/webusb/src/webusb.h

+static const struct usb_bos_webusb_desc bos_cap_webusb = {
+	/* WebUSB Platform Capability Descriptor:
+	 * https://wicg.github.io/webusb/#webusb-platform-capability-descriptor
+	 */
+	.platform = {
+		.bLength = sizeof(struct usb_bos_platform_descriptor)
+			 + sizeof(struct usb_bos_capability_webusb),
+		.bDescriptorType = USB_DESC_DEVICE_CAPABILITY,
+		.bDevCapabilityType = USB_BOS_CAPABILITY_PLATFORM,
+		.bReserved = 0,
+		/* WebUSB Platform Capability UUID
+		 * 3408b638-09a9-47a0-8bfd-a0768815b665
+		 */
+		.PlatformCapabilityUUID = {
+			0x38, 0xB6, 0x08, 0x34,
+			0xA9, 0x09,
+			0xA0, 0x47,
+			0x8B, 0xFD,
+			0xA0, 0x76, 0x88, 0x15, 0xB6, 0x65,
+		},
+	},
+	.cap = {
+		.bcdVersion = sys_cpu_to_le16(0x0100),
+		.bVendorCode = SAMPLE_WEBUSB_VENDOR_CODE,
+		.iLandingPage = SAMPLE_WEBUSB_LANDING_PAGE
+	}
+};
+
+/* WebUSB URL Descriptor, see https://wicg.github.io/webusb/#webusb-descriptors */
+static const uint8_t webusb_origin_url[] = {
+	/* bLength, bDescriptorType, bScheme, UTF-8 encoded URL */
+	0x11, WEBUSB_DESC_TYPE_URL, WEBUSB_URL_PREFIX_HTTP,
+	'l', 'o', 'c', 'a', 'l', 'h', 'o', 's', 't', ':', '8', '0', '0', '0'
+};
+
+static int webusb_to_host_cb(const struct usbd_context *const ctx,
+			     const struct usb_setup_packet *const setup,
+			     struct net_buf *const buf)
+{
+	LOG_INF("Vendor callback to host");
+
+	if (setup->wIndex == WEBUSB_REQ_GET_URL) {
+		uint8_t index = USB_GET_DESCRIPTOR_INDEX(setup->wValue);
+
+		if (index != SAMPLE_WEBUSB_LANDING_PAGE) {
+			return -ENOTSUP;
+		}
+
+		LOG_INF("Get URL request, index %u", index);
+		net_buf_add_mem(buf, &webusb_origin_url,
+				MIN(net_buf_tailroom(buf), sizeof(webusb_origin_url)));
+
+		return 0;
+	}
+
+	return -ENOTSUP;
+}
+
+USBD_DESC_BOS_VREQ_DEFINE(bos_vreq_webusb, sizeof(bos_cap_webusb), &bos_cap_webusb,
+			  SAMPLE_WEBUSB_VENDOR_CODE, webusb_to_host_cb, NULL);


Why this is defined in a header?

tmon-nordic · 2024-07-29T07:28:23Z

samples/subsys/usb/webusb/src/msosv2.h

+	'3', 0x00, '9', 0x00, '5', 0x00, '6', 0x00, '2', 0x00, 'B', 0x00, \
+	'3', 0x00, '}', 0x00, 0x00, 0x00, 0x00, 0x00
+
+struct msosv2_descriptor {


Would it be possible to somehow take the advantage of the new stack to enable automatic struct msosv2_function_subset_header insertion when there is more than 1 interface?

The old stack did have #66449 as a solution, but that's fragile approach.

I do not think we can handle it. IMO it is better to write a comprehensive documentation about it, with some examples. I think the average user has no idea where and how to use MSOSv2 descriptors.

Comprehensive documentation is not a real solution but a way to avoid having to come up with a real solution. While the average user might not have idea about individual MSOSv2 descriptors, it would be good to provide "just-working" experience (if possible).

henrikbrixandersen

I would prefer a generic way to register vendor requests targeting the device (not a class nor an interface) from the application code. This could easily be used for handling MSOSv2 vendor requests, but wouldn't be limited to that (not all vendor requests would require a BOS).

carlescufi · 2024-10-23T17:54:00Z

@tmon-nordic @henrikbrixandersen can you please take another look?

tmon-nordic

The new bRequest registration seems fine, just few things to fix.

include/zephyr/usb/usbd.h

tmon-nordic · 2024-10-24T12:55:20Z

include/zephyr/usb/usbd.h

+ * @param vto_dev   Vendor callback for to-device direction request
+ */
+#define USBD_VREQUEST_DEFINE(name, vcode, vto_host, vto_dev)			\
+	static struct usbd_vreq_node name = {					\


I would prefer some prefix on the node, e.g.

Suggested change

static struct usbd_vreq_node name = { \

static struct usbd_vreq_node usbd_vreq_node_##name = { \

Then the user does not know the real structure name. This makes sense for more abstract macros, but not here.

subsys/usb/device_next/usbd_ch9.c

subsys/usb/device_next/usbd_device.h

tmon-nordic · 2024-10-24T13:06:21Z

samples/subsys/usb/webusb-next/src/msosv2.h

+	'3', 0x00, '}', 0x00, 0x00, 0x00, 0x00, 0x00
+
+struct msosv2_descriptor {
+	struct msosv2_descriptor_set_header header;


If not implement it right now, at least add a TODO: Add subset header when there is more than one interface.

See #66449 for more information.

I have aligned it with the descriptor in the legacy sample and added a comment.

Allow the user to register a vendor request node identified by the vendor code (bRequest) and containing two callbacks to handle the vendor request. The device stack uses the vendor request node to call the vendor request callbacks when it receives a request of type Vendor, recipient Device, and bRequest value equal to the vendor code. Signed-off-by: Johann Fischer <johann.fischer@nordicsemi.no>

Platform capability descriptors such as MSOSv2 or WebUSB BOS have a vendor request code that is used by the host to perform vendor-specific requests. Add a convenient way to define and register a platform capability descriptor with a vendor request node. Signed-off-by: Johann Fischer <johann.fischer@nordicsemi.no>

Add a WebUSB sample that uses the new USB device support. Signed-off-by: Johann Fischer <johann.fischer@nordicsemi.no>

carlescufi · 2024-11-06T11:09:36Z

@henrikbrixandersen PTAL

jfischer-no added area: USB Universal Serial Bus Experimental Experimental features not enabled by default labels Jul 26, 2024

jfischer-no added this to the v4.0.0 milestone Jul 26, 2024

jfischer-no requested review from henrikbrixandersen, carlescufi and tmon-nordic July 26, 2024 12:07

jfischer-no self-assigned this Jul 26, 2024

zephyrbot added the area: Samples Samples label Jul 26, 2024

zephyrbot requested review from kartben and nashif July 26, 2024 12:08

tmon-nordic requested changes Jul 29, 2024

View reviewed changes

henrikbrixandersen reviewed Aug 13, 2024

View reviewed changes

carlescufi mentioned this pull request Sep 24, 2024

Planned USB device/host support rework/enhancements #42066

Open

56 tasks

github-actions bot added the Stale label Oct 13, 2024

henrikbrixandersen removed the Stale label Oct 13, 2024

zephyrproject-rtos deleted a comment from github-actions bot Oct 17, 2024

jfischer-no force-pushed the pr-device_next-sample-webusb branch 3 times, most recently from 5291081 to 6f692c4 Compare October 21, 2024 10:51

carlescufi requested review from tmon-nordic and henrikbrixandersen October 23, 2024 17:53

tmon-nordic requested changes Oct 24, 2024

View reviewed changes

jfischer-no added 2 commits October 25, 2024 16:00

jfischer-no force-pushed the pr-device_next-sample-webusb branch from 6f692c4 to eb96943 Compare October 25, 2024 16:24

jfischer-no modified the milestones: v4.0.0, 4.1 Oct 25, 2024

jfischer-no added this to the v4.1.0 milestone Oct 25, 2024

samples: usb: add new WebUSB sample

210cda7

Add a WebUSB sample that uses the new USB device support. Signed-off-by: Johann Fischer <johann.fischer@nordicsemi.no>

jfischer-no force-pushed the pr-device_next-sample-webusb branch from eb96943 to 210cda7 Compare October 25, 2024 16:39

tmon-nordic approved these changes Oct 28, 2024

View reviewed changes

henrikbrixandersen approved these changes Nov 19, 2024

View reviewed changes

henrikbrixandersen linked an issue Nov 19, 2024 that may be closed by this pull request

usb: device_next: unable to handle vendor requests in address state #76224

Closed

henrikbrixandersen mentioned this pull request Nov 19, 2024

Create a WebUSB subsys driver #12527

Open

4 tasks

nashif merged commit 1687e19 into zephyrproject-rtos:main Nov 19, 2024
23 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

usb: device_next: support BOS descriptor with vendor request code #76326

usb: device_next: support BOS descriptor with vendor request code #76326

jfischer-no commented Jul 26, 2024

tmon-nordic Jul 29, 2024

jfischer-no Jul 29, 2024

tmon-nordic Jul 29, 2024

jfischer-no Oct 17, 2024

tmon-nordic Jul 29, 2024

jfischer-no Jul 29, 2024

tmon-nordic Jul 29, 2024

jfischer-no Oct 17, 2024

tmon-nordic Jul 29, 2024

tmon-nordic Jul 29, 2024

jfischer-no Jul 29, 2024

tmon-nordic Jul 29, 2024

henrikbrixandersen left a comment

carlescufi commented Oct 23, 2024

tmon-nordic left a comment

tmon-nordic Oct 24, 2024

jfischer-no Oct 25, 2024

tmon-nordic Oct 24, 2024

jfischer-no Oct 25, 2024

carlescufi commented Nov 6, 2024

	static struct usbd_vreq_node name = { \
	static struct usbd_vreq_node usbd_vreq_node_##name = { \

usb: device_next: support BOS descriptor with vendor request code #76326

usb: device_next: support BOS descriptor with vendor request code #76326

Conversation

jfischer-no commented Jul 26, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

henrikbrixandersen left a comment

Choose a reason for hiding this comment

carlescufi commented Oct 23, 2024

tmon-nordic left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

carlescufi commented Nov 6, 2024