USB Host: integrate class API [2: helpers] #94590

josuah · 2025-08-17T20:03:03Z

Dependency:

USB Host: integrate class API [1: types] #94504

Downstream:

NOTE: this PR also contains the commit of #94504

It contains all the commits of the previous PR, and in addition:

Wrapper for the class API
Split the struct usbh_class_data and usbh_class_api
Add utilities to register classes as a linked list (NXP)
Add a filter logic to match a device class to host class instances (NXP)

This also contains functions imported from #94085 by @AidenHu.

So far, this was only tested with this command to make sure it builds and run, but not in hardware yet:

west build -t run -b native_sim samples/subsys/usb/shell/ \
-DEXTRA_CONF_FILE=virtual.conf \
-DDTC_OVERLAY_FILE=virtual.overlay \
-DEXTRA_CONF_FILE=device_and_host_prj.conf

The PR #94085 got then completely rebased on top of this, available at josuah:enable_usb_host_video_class_api2, which is meant as a drop-in replacement for #94085.

It was also built (but not run) with this command:

west build -b rd_rw612_bga -S video-sw-generator samples/drivers/video/capture

All tests are done on top of main...josuah:zephyr:pr_usb_host_class_api2

subsys/usb/host/usbh_device.c

jfischer-no · 2025-09-05T14:23:45Z

include/zephyr/usb/usbh.h

+	/** Pointer to host support class API */
+	struct usbh_class_api *api;
+	/** Pointer to private data */
+	void *priv;


I dug out the branch where I started working on the CDC ACM driver, cleaned it up a bit, and pushed it to my Zephyr repository, https://github.com/jfischer-no/zephyr/commits/wip-usbh-class-driver/, please take a look and reuse what makes sense.

I think I still have these checks missing, left for the class to implement them:

if_desc = (void *)udev->ifaces[i].dhp; if (if_desc->bInterfaceClass == code->dclass && if_desc->bInterfaceSubClass == code->sub && if_desc->bInterfaceProtocol == code->proto) {

But that is something that I can add. Is it possible for a standard device to only expect interface or interface-association to be used for class selection?

I really need to check the standards again.

jfischer-no · 2025-09-05T14:26:53Z

include/zephyr/usb/usbh.h

 	/** Request completion event handler */
-	int (*request)(struct usbh_context *const uhs_ctx,
-			struct uhc_transfer *const xfer, int err);
+	int (*request)(struct usbh_class_data *cdata,


Perhaps we should name it completion_cb int (*completion_cb)(struct usbh_class_data *cdata,....

Naming change request() -> completion_cb() done.
Let me know if other APIs should also have a _cb suffix.

I am taking the opportunity to remove the int err argument as uhc_trasfer.err exists already. Maybe these were errors on a different layer though.

Naming change request() -> completion_cb() done. Let me know if other APIs should also have a _cb suffix.

I think only completion is literally callback, so I prefer not to postfix others with _cb.

jfischer-no · 2025-09-05T14:29:27Z

include/zephyr/usb/usbh.h

+	int (*connected)(struct usbh_class_data *cdata,
+			 void *desc_start_addr,
+			 void *desc_end_addr);


I went with the probe and interface number, jfischer-no@7d9c9d8#diff-2882b8550e23be245dbdaa95a7518e57e19dcf0af85dfd7bd3f8f7b6e070fba9R105-R107, but I think int (*probe)(struct usbh_class_data *const c_data, struct usb_device *const udev) could work as well.

I converted this to (*probe) to use udev and the interface number. The class gets the configuration descriptor from udev and then parses the descriptor itself to check if the class is supported or not.

Extra helpers for browsing and searching the descriptors were added.

P.S. I just noticed I used ifnum instead of iface, I can switch to iface for variable names everywhere

jfischer-no · 2025-09-05T14:32:37Z

include/zephyr/usb/usbh.h

 	/** Pointer to USB host stack context structure */
 	struct usbh_context *uhs_ctx;
+	/** System linked list node for registered classes */
+	sys_snode_t node;


Here we need to separate it in usbh_class_data and usbh_class_node, which is hidden from the users, if usbh_class_data needs to be managed in a slist. See jfischer-no@7d9c9d8

There is now a parent struct usbh_class_node that contains only struct usbh_class_data for now, but to which more internal fields for internal use only can be added.

jfischer-no · 2025-09-05T14:47:40Z

subsys/usb/host/usbh_class.c

+	STRUCT_SECTION_FOREACH(usbh_class_data, cdata_existing) {
+		struct usbh_class_data *cdata_registered;
+		bool already_registered = false;


I started out in exactly the same way, but then switched to using iterable sections. These are already in place. Unlike on the device side, where a set of functions may vary between configurations, a host either has a driver for a function (class) or it does not. My idea was therefore to simply call a probe() from STRUCT_SECTION_FOREACH() and allow the class driver to tacitly claim a function. For now, this should unblock the work with the class driver, and we can re-evaluate how to claim a function later.

Although I used code triple in the probe, I think it would be better to move the detection logic to the driver and perform the comparison there, jfischer-no@7d9c9d8#diff-8abee9f852f1e345184cb4919b1620c6780c5d6e2db3dc3ea9bda6a4b057e51bR294-R302

Then the whole subsys/usb/host/usbh_class.c file can be dropped and this simplifies everything.
I will switch everything from SYS_SLIST_FOR_EACH_CONTAINER to just use STRUCT_SECTION_FOREACH directly.

jfischer-no · 2025-09-05T15:00:05Z

include/zephyr/usb/class/usb_uvc.h

We may have subsys/usb/common/include/. If the application does not need to include this header, then you can move it there.

header moved to subsys/usb/common/include/

jfischer-no · 2025-09-05T15:02:47Z

subsys/usb/host/usbh_class.c

+bool usbh_class_is_matching(struct usbh_class_data *cdata,
+			    struct usbh_class_filter *device_info)


Please use const extensively for the parameters.

bool usbh_class_is_matching(struct usbh_class_data *const cdata, struct usbh_class_filter *const device_info)

I did not understand why the name of the second parameter is device_info, would not just filter be a better name?

[EDIT: new approach below]

This function needs to know:

The list of filters configured for that class (cdata->filters, passed through cdata and not explicitly).

Information from the device descriptor to match against this list of rules.

I am probably abusing this same struct for these two purpose. Maybe this would make more sense?

bool usbh_class_is_matching(struct usbh_class_filter *filters, size_t num_filters, struct usbh_class_filter *device_info);

It got imported into the current UVC PR from NXP which gives an usage example:
https://github.com/zephyrproject-rtos/zephyr/pull/94085/files#diff-46dc301268b060b9975dc2b009b8c30c129b395c64a9c4c12e4e329dc203cd95R108

The device information is first collected from the descriptors upon device connection:

if (desc->bDescriptorType == USB_DESC_INTERFACE) { struct usb_if_descriptor *if_desc = (void *)desc; device_info.code_triple.dclass = if_desc->bInterfaceClass; device_info.code_triple.sub = if_desc->bInterfaceSubClass; device_info.code_triple.proto = if_desc->bInterfaceProtocol; found_interface = true; }

And then once this is done, this aggregated info is tested against every class to find one that is matching:

SYS_SLIST_FOR_EACH_CONTAINER(&ctx->class_list, cdata, node) { if (usbh_class_is_matching(cdata, &device_info)) { LOG_INF("Class driver %s matched for class 0x%02x", cdata->name, device_info.code_triple.dclass); [...] } }

Is there a better way?

[EDIT: new new approach below based on feedback]
It might not be possible to handle all the complexity of device selection with just a table.
For instance, an eye-tracking composite device with a single USB cable but multiple UVC interfaces labeled "Left eye", "Right eye", "Point of view"... Or setup with multiple identical devices only differentiated by iSerial.

I am now trying with a api->match()

I would imagine usbh_class_is_matching() should be called from the function driver for the rough check, usbd_class_probe()->usbh_class_is_matching(). How a specific feature of the function is discovered is up to the driver. From your example, all the functions you list should be handled by the same function driver.
I suggest not being UVC specific, or trying to file an API for UVC. Let me take your PR and add a simpler class skeleton to that, as opposed to device_next loopback.c.

I am connecting this thread with "move the detection logic to the driver and perform the comparison there" [1], and understands better now thank you.

struct usbh_class_filter *const device_info could just be struct usb_device_descriptor *const desc to match against, as it already contains all of the "device info" we need in the matching...

It took me time to figure the proper answer to this question but hopefully I am getting there!

jfischer-no · 2025-09-05T15:06:34Z

include/zephyr/usb/usbh.h

+	/** Mask of match types for selecting which rules to apply */
+	uint32_t flags;
+	/** The device's class code, subclass code, protocol code. */
+	struct usbh_code_triple code_triple;


Maybe just

/** Device Class Code */ uint8_t dclass; /** Class Subclass Code */ uint8_t sub; /** Class Protocol Code */ uint8_t proto;

Then it looks a bit less painful in usbh_class_is_matching().

I unpacked these fields out of the struct usbh_code_triple.

jfischer-no · 2025-09-05T15:23:34Z

subsys/usb/host/usbh_desc.c

+struct usb_desc_header *usbh_desc_get_by_type(const uint8_t *const start_addr,
+					      const uint8_t *const end_addr,
+					      uint32_t type_mask)


We need some simple function driver code to test it, IMO jfischer-no@28f064b is too ugly for that, maybe you or me can sand down it to loopback.c skeleton (to be counterpart of device_next/class/loopback.c), just checking for the testusb bulk interface and testusb sample VID/PID.

This should help downstream PRs a lot.

Should that be using UVB connecting the device and host class together? I tried it on a WIP branch and can try to collect bits from various branches and integrate unit tests into API2 that way.

Is there another way I should test the host functionality than UVB?

main...josuah:zephyr:pr_sample_uvc_uvb#diff-6b7a858e2f07001df9a5d4de454923a5cf06ed38afb7fd4040bb84b45089cc34

Or this can also be some manually crafted descriptor, it seems a different concern than testing everything end-to-end, and easier to reproduce bugs. I will try this thank you.

jfischer-no · 2025-09-16T14:24:42Z

subsys/usb/host/usbh_class_api.h

+static inline int usbh_class_rwup(struct usbh_class_data *const c_data,
+				  struct usb_device *const udev)


I guess it is my copy/paste error. I actually have no idea how it could be used. What do you think?

I am now back on track with this and should be able to un-block this PR. Sorry to keep you waiting 2 weeks!

Maybe it is not something that a particular class needs to do?

I removed the function which does not prevent someone needing it to propose a PR for re-introducing it...

jfischer-no · 2025-09-16T15:38:50Z

subsys/usb/host/usbh_desc.c

+	while (curr_addr < end_addr) {
+		struct usb_desc_header *desc = (void *)curr_addr;
+
+		if (desc->bLength == 0) {
+			break;
+		}


Is something missing here?

jfischer-no · 2025-09-16T15:40:14Z

include/zephyr/usb/usbh.h

Is commit message outdated?

I removed the off-topic sentence at the end of the first commit. (add host status to the host context)

jfischer-no · 2025-09-16T16:01:15Z

tests/subsys/usb/host/src/main.c

Looks like it is class API test and should be tests/subsys/usb/host/class_api/src/main.c.

josuah · 2025-09-30T22:20:21Z

Force-push:

Rebase on main

josuah · 2025-09-30T22:21:38Z

Force-push:

Add some unit tests
Apply changes as requested

Still more changes to go...

Add a "struct usbh_status" that contains a bitmask of flags to keep track of the global state of the host context, like done for the device_next implementation. Signed-off-by: Josuah Demangeon <[email protected]>

Add a "struct usbh_class_api" for the host implementation, and move all the function poitners to it. Add more fields to "struct usbh_class_data". Signed-off-by: Josuah Demangeon <[email protected]>

Add API wrappers around the function pointers in struct usbh_class_api, while also documenting the USB host class internal API. Signed-off-by: Josuah Demangeon <[email protected]>

Add functions to probe/remove all classes as part of a new usbh_class.c and a matching usbh_class.h. These functions are called from the function usbh_init_device_intl() in usbh_core.c to initialize every class upon connection of a device. The class instances are registered and connected together as a linked list. Co-authored-by: Aiden Hu <[email protected]> Signed-off-by: Josuah Demangeon <[email protected]>

Add utilities to filter a class and search the next descriptor of a given type. This can be used to in combination to seek device information from the USB descriptors and try to match a host class instance given a set of filters. Co-authored-by: Aiden Hu <[email protected]> Signed-off-by: Josuah Demangeon <[email protected]>

Move the UVC header with all the definitions from the UVC standard to share it between USB host and device class implementation. Signed-off-by: Josuah Demangeon <[email protected]>

Add tests making sure the USB Host class APIs introduced build and run as expected. Signed-off-by: Josuah Demangeon <[email protected]>

josuah

Here is another cycle.

I believe I addressed most points but if there remain any, I will make sure to be more reactive and allow this to be adjusted quicly.

josuah · 2025-10-01T10:20:07Z

include/zephyr/usb/class/usb_uvc.h

header moved to subsys/usb/common/include/

josuah · 2025-10-01T11:31:54Z

subsys/usb/host/usbh_class_api.h

+static inline int usbh_class_rwup(struct usbh_class_data *const c_data,
+				  struct usb_device *const udev)


Maybe it is not something that a particular class needs to do?

I removed the function which does not prevent someone needing it to propose a PR for re-introducing it...

josuah · 2025-10-06T11:06:47Z

include/zephyr/usb/usbh.h

+	/** Mask of match types for selecting which rules to apply */
+	uint32_t flags;
+	/** The device's class code, subclass code, protocol code. */
+	struct usbh_code_triple code_triple;


I unpacked these fields out of the struct usbh_code_triple.

josuah · 2025-10-06T11:08:54Z

include/zephyr/usb/usbh.h

 	/** Pointer to USB host stack context structure */
 	struct usbh_context *uhs_ctx;
+	/** System linked list node for registered classes */
+	sys_snode_t node;


There is now a parent struct usbh_class_node that contains only struct usbh_class_data for now, but to which more internal fields for internal use only can be added.

josuah · 2025-10-06T11:13:28Z

include/zephyr/usb/usbh.h

+	int (*connected)(struct usbh_class_data *cdata,
+			 void *desc_start_addr,
+			 void *desc_end_addr);


I converted this to (*probe) to use udev and the interface number. The class gets the configuration descriptor from udev and then parses the descriptor itself to check if the class is supported or not.

Extra helpers for browsing and searching the descriptors were added.

P.S. I just noticed I used ifnum instead of iface, I can switch to iface for variable names everywhere

josuah · 2025-10-06T11:17:00Z

include/zephyr/usb/usbh.h

I removed the off-topic sentence at the end of the first commit. (add host status to the host context)

josuah · 2025-10-06T11:30:06Z

include/zephyr/usb/usbh.h

+	/** Pointer to host support class API */
+	struct usbh_class_api *api;
+	/** Pointer to private data */
+	void *priv;


I think I still have these checks missing, left for the class to implement them:

if_desc = (void *)udev->ifaces[i].dhp; if (if_desc->bInterfaceClass == code->dclass && if_desc->bInterfaceSubClass == code->sub && if_desc->bInterfaceProtocol == code->proto) {

But that is something that I can add. Is it possible for a standard device to only expect interface or interface-association to be used for class selection?

I really need to check the standards again.

sonarqubecloud · 2025-10-06T12:25:30Z

Quality Gate passed

Issues
18 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
7.7% Duplication on New Code

See analysis details on SonarQube Cloud

josuah · 2025-10-06T18:26:36Z

I guess the best way for me to discover if the API is good is to try to use it with a class so I will pursue with the rest:

USB Host: integrate class API [3: UVC] #94591

josuah changed the title ~~USB Host: integrate class API [2]~~ USB Host: integrate class API [2: helpers] Aug 17, 2025

This was referenced Aug 17, 2025

USB Host: integrate class API [1: types] #94504

Merged

USB Host: integrate class API [3: UVC] #94591

Draft

usb: host: class: support for usb host video class #94085

Draft

josuah marked this pull request as ready for review August 17, 2025 21:05

zephyrbot added area: USB Universal Serial Bus area: Samples Samples labels Aug 17, 2025

zephyrbot requested review from jfischer-no, kartben, nashif and tmon-nordic August 17, 2025 21:06

zephyrbot assigned jfischer-no Aug 17, 2025

josuah mentioned this pull request Sep 4, 2025

drivers: uhc_dwc2: Initial support [WIP] #94266

Draft

josuah force-pushed the pr_usb_host_class_api2 branch from e57477d to b3b4bc4 Compare September 4, 2025 13:46

zephyrbot requested a review from JarmouniA September 4, 2025 13:47

josuah force-pushed the pr_usb_host_class_api2 branch 3 times, most recently from e672ee8 to d04bbfb Compare September 5, 2025 12:01

tmon-nordic reviewed Sep 5, 2025

View reviewed changes

subsys/usb/host/usbh_device.c Outdated Show resolved Hide resolved

josuah force-pushed the pr_usb_host_class_api2 branch from d04bbfb to 7073da8 Compare September 5, 2025 13:27

jfischer-no reviewed Sep 5, 2025

View reviewed changes

jfischer-no added the Experimental Experimental features not enabled by default label Sep 5, 2025

jfischer-no mentioned this pull request Sep 8, 2025

Planned USB host support work #95661

Open

12 tasks

josuah mentioned this pull request Sep 10, 2025

Planned USB host video implementation #93517

Open

15 tasks

josuah marked this pull request as draft September 11, 2025 00:05

josuah force-pushed the pr_usb_host_class_api2 branch 4 times, most recently from f095c02 to 3e111d0 Compare September 12, 2025 20:58

jfischer-no reviewed Sep 16, 2025

View reviewed changes

josuah force-pushed the pr_usb_host_class_api2 branch from 3e111d0 to 7f7ae73 Compare September 30, 2025 22:19

josuah force-pushed the pr_usb_host_class_api2 branch from 7f7ae73 to 1ea9a74 Compare September 30, 2025 22:20

josuah force-pushed the pr_usb_host_class_api2 branch 2 times, most recently from 6b6a2d9 to 50d00f1 Compare October 1, 2025 21:43

Josuah Demangeon added 3 commits October 1, 2025 22:02

usb: host: add host status to the host context

398f3ce

Add a "struct usbh_status" that contains a bitmask of flags to keep track of the global state of the host context, like done for the device_next implementation. Signed-off-by: Josuah Demangeon <[email protected]>

usb: host: class: implement more of the class/data struct fields

90fad56

Add a "struct usbh_class_api" for the host implementation, and move all the function poitners to it. Add more fields to "struct usbh_class_data". Signed-off-by: Josuah Demangeon <[email protected]>

usb: host: introduce wrappers to access the class function pointers

cad9ff3

Add API wrappers around the function pointers in struct usbh_class_api, while also documenting the USB host class internal API. Signed-off-by: Josuah Demangeon <[email protected]>

josuah force-pushed the pr_usb_host_class_api2 branch 3 times, most recently from e57d01c to c2e30a0 Compare October 6, 2025 11:05

Josuah Demangeon and others added 4 commits October 6, 2025 11:58

usb: uvc: move the header definition to include/

f30b2ee

Move the UVC header with all the definitions from the UVC standard to share it between USB host and device class implementation. Signed-off-by: Josuah Demangeon <[email protected]>

tests: usb: host: test basic API functionality

1175047

Add tests making sure the USB Host class APIs introduced build and run as expected. Signed-off-by: Josuah Demangeon <[email protected]>

josuah force-pushed the pr_usb_host_class_api2 branch from c2e30a0 to 1175047 Compare October 6, 2025 11:59

josuah commented Oct 6, 2025

View reviewed changes

josuah marked this pull request as ready for review October 6, 2025 13:57

zephyrbot added the area: Tests Issues related to a particular existing or missing test label Oct 6, 2025

		bool usbh_class_is_matching(struct usbh_class_data *cdata,
		struct usbh_class_filter *device_info)

		static inline int usbh_class_rwup(struct usbh_class_data *const c_data,
		struct usb_device *const udev)

USB Host: integrate class API [2: helpers] #94590

Are you sure you want to change the base?

USB Host: integrate class API [2: helpers] #94590

Conversation

josuah commented Aug 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jfischer-no Sep 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

josuah Sep 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

josuah Sep 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jfischer-no Sep 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

josuah Sep 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

josuah Sep 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

josuah Sep 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

josuah commented Aug 17, 2025 •

edited

Loading

jfischer-no Sep 10, 2025 •

edited

Loading

josuah Sep 5, 2025 •

edited

Loading

josuah Sep 10, 2025 •

edited

Loading

jfischer-no Sep 10, 2025 •

edited

Loading

josuah Sep 11, 2025 •

edited

Loading

josuah Sep 5, 2025 •

edited

Loading

josuah Sep 30, 2025 •

edited

Loading