Skip to content

Conversation

AidenHu
Copy link
Contributor

@AidenHu AidenHu commented Aug 4, 2025

  1. Add USB host video class driver usbh_uvc.c/h . This class driver is implemented based on UVC 1.5
  2. The USB host video class driver is tightly coupled with the video subsystem. Users still use video API to operate it.
  3. Some key changes for usbh_core.c to support multiple classes that one USB device supports.
  4. Some necessary changes to make video capture example connect with USB device camera based on the new USB host video class driver.

Copy link

github-actions bot commented Aug 4, 2025

Hello @AidenHu, and thank you very much for your first pull request to the Zephyr project!
Our Continuous Integration pipeline will execute a series of checks on your Pull Request commit messages and code, and you are expected to address any failures by updating the PR. Please take a look at our commit message guidelines to find out how to format your commit messages, and at our contribution workflow to understand how to update your Pull Request. If you haven't already, please make sure to review the project's Contributor Expectations and update (by amending and force-pushing the commits) your pull request if necessary.
If you are stuck or need help please join us on Discord and ask your question there. Additionally, you can escalate the review when applicable. 😊

@AidenHu AidenHu force-pushed the enable_usb_host_video_class branch 2 times, most recently from 669c434 to 4ca6d07 Compare August 5, 2025 08:50
@dleach02 dleach02 added the area: USB Universal Serial Bus label Aug 5, 2025
uvc_host: uvc_host {
compatible = "zephyr,uvc-host";
};
}; No newline at end of file
Copy link
Member

@dleach02 dleach02 Aug 5, 2025

Choose a reason for hiding this comment

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

need blank line (repeat this on a couple of the files)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

need blank line (repeat this on a couple of the files)

Thanks, done it.

#include <zephyr/kernel.h>
#include <zephyr/device.h>

#include <zephyr/drivers/display.h>
Copy link
Member

Choose a reason for hiding this comment

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

I know this is draft but I think we want to allow this sample to support Video host and video device

@dleach02
Copy link
Member

dleach02 commented Aug 5, 2025

@AidenHu you are going to need to break this up into multiple commits. My suggestion is:

  1. subsys/usb/host, dts binding, and usbh.h changes
  2. RW612 board/soc changes
  3. sample changes

@AidenHu
Copy link
Contributor Author

AidenHu commented Aug 6, 2025

@AidenHu you are going to need to break this up into multiple commits. My suggestion is:

1. subsys/usb/host, dts binding, and usbh.h changes

2. RW612 board/soc changes

3. sample changes

Thanks @dleach02, I will split now.

@AidenHu AidenHu force-pushed the enable_usb_host_video_class branch 2 times, most recently from cb3ab4e to 76bed79 Compare August 6, 2025 02:59
@carlescufi carlescufi requested a review from josuah August 6, 2025 10:33
@AidenHu AidenHu force-pushed the enable_usb_host_video_class branch from 76bed79 to 9192550 Compare August 7, 2025 02:41
Josuah Demangeon added 5 commits August 13, 2025 15:13
Make the struct name match the device naming for ease of use, although
slightly longer name. Propagate the change to the subsystem, includes,
tests and samples.

Signed-off-by: Josuah Demangeon <[email protected]>
Allocate extra room at the end of the USB descriptor buffer to ensure
that the device ends with `desc->bLength == 0`. For now, validating
`desc->bLength` from the device to ensure no access past the buffer
is still not done.

Signed-off-by: Josuah Demangeon <[email protected]>
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. Add helpers to check the status.

Signed-off-by: Josuah Demangeon <[email protected]>
Hide the mutex lock details inside a wrapper call, like done for the
device stack API. This is not used for the per-device lock of the host
stack which use a different mutex lock.

Signed-off-by: Josuah Demangeon <[email protected]>
Update the USB shell sample to the new syntax. This is based on tests
done with native_sim, along with the virtual.conf, virtual.overlay and
device_and_host_prj.conf extra config files.

Signed-off-by: Josuah Demangeon <[email protected]>
@josuah
Copy link
Contributor

josuah commented Aug 14, 2025

Thank you for updating this PR with a more fine-grained commit history.

In parallel, I opened #94504 to add the missing USB Host API bits to enable this class to be implemented, using the content of this current PR:

And here is this current PR nxp-upstream:enable_usb_host_video_class but rebased on top of pr_usb_host_class_api2:

@AidenHu AidenHu closed this Aug 15, 2025
@AidenHu AidenHu reopened this Aug 15, 2025
@AidenHu
Copy link
Contributor Author

AidenHu commented Aug 15, 2025

Thank you for updating this PR with a more fine-grained commit history.
In parallel, I opened #94504 to add the missing USB Host API bits to enable this class to be implemented, using the content of this current PR.
For now pr_usb_host_class_api (#94504), pr_usb_host_class_api2, pr_usb_host_class_api3 etc.
And here is this current PR nxp-upstream:enable_usb_host_video_class but rebased on top of pr_usb_host_class_api2:

* [main...josuah:zephyr:enable_usb_host_video_class](https://github.com/zephyrproject-rtos/zephyr/compare/main...josuah:zephyr:enable_usb_host_video_class)

Thank you for the reply, @josuah

@AidenHu AidenHu force-pushed the enable_usb_host_video_class branch 2 times, most recently from 90739a9 to aef5e85 Compare August 15, 2025 02:09
Josuah Demangeon and others added 3 commits August 15, 2025 13:56
Add a "struct usbh_class_api" for the host implementation, and move all
the function poitners to it, as it is done for the device support.
Add more fields to the host struct usbh_class_data mirrorring the device
side struct, including the API pointer.

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 register 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]>
@AidenHu AidenHu force-pushed the enable_usb_host_video_class branch from aef5e85 to 629e311 Compare August 16, 2025 14:02
@josuah
Copy link
Contributor

josuah commented Aug 20, 2025

@josuah, is it ok to merge this PR #94662 now?

This might need review from developers at NXP as I am not able to give an official "ok" on that area.

Though I did not find any issue with #94662 and can be merged any time of course.

@AidenHu AidenHu force-pushed the enable_usb_host_video_class branch from 41b1fc8 to 9d15e7e Compare August 21, 2025 02:30
@AidenHu
Copy link
Contributor Author

AidenHu commented Aug 21, 2025

@josuah, is it ok to merge this PR #94662 now?

This might need review from developers at NXP as I am not able to give an official "ok" on that area.

Though I did not find any issue with #94662 and can be merged any time of course.

@josuah,thank you for the checking, will wait it to be merged.

@AidenHu
Copy link
Contributor Author

AidenHu commented Aug 22, 2025

Hi @josuah
Now I am enhancing the video stream transfer because there are still something unresolved. However, I will be OOO next week and back in 30th this month. When I am back I will continue the current task. Here thank you again about the comments you have raised for me.

@josuah
Copy link
Contributor

josuah commented Aug 22, 2025

Understood, thanks for letting me know @AidenHu!
This will give more time for reviews to happen in the meantime.

Update this sample is dedicated to support usb host
video. This sample can enumerate UVC device, configure
UVC device and start video stream transfers then
display it on LCD.

Signed-off-by: Aiden Hu <[email protected]>
usb clock enablement for UHC NXP EHCI on rw.

Signed-off-by: Aiden Hu <[email protected]>
1. Add interval for usb xfer.
2. Set right mps for during pipe init.
3. Set right direction for non-zero endpoint.
4. Check transferBuffer during init transfer.

Signed-off-by: Aiden Hu <[email protected]>
Use right parameters for usbh_class_connected and
usbh_class_removed.

Signed-off-by: Aiden Hu <[email protected]>
Incorrect start address is used for usbh_class_connected
for each supported independent class. Implement
dev_removed_handler to properly notify class
disconnection event.

Signed-off-by: Aiden Hu <[email protected]>
Move the pointer to the next descriptor to prevent
incorrect address usage.

Signed-off-by: Aiden Hu <[email protected]>
Change the default value for QH/QTD/ITD/SITD
of usb host ehci.

Signed-off-by: Aiden Hu <[email protected]>
1. Delete uvc_guid_map and related functions, use the
database and helper functions from usb_common_uvc.c
2. Rename uvc_host_parse_frame_intervals as
uvc_host_parse_frame_default_intervals to select
default frame interval.
3. Correct uvc_host_select_streaming_alternate to do
right alternate selection.
4. Implement uvc_host_removed function.
5. Enhance the video_usb_uvc_host_set_stream function.

Signed-off-by: Aiden Hu <[email protected]>
1. Use common usb_uvc.h header for usbh_uvc.c
2. Delete common definitions from usbh_uvc.h
3. Clean up and optimize usbh_uvc.c
4. Add necessary symbols for usb host uvc class.

Signed-off-by: Aiden Hu <[email protected]>
Some necessary definitions are added. Do some improvements for the
existing content based on the UVC spec.

Signed-off-by: Aiden Hu <[email protected]>
@AidenHu AidenHu force-pushed the enable_usb_host_video_class branch from 9d15e7e to 1ce0e22 Compare September 1, 2025 10:45
Adjust video stream callback function to be more robust.
Use multiple primes to improve the throughput.

Signed-off-by: Aiden Hu <[email protected]>
This new symbol is used to set usb transfer prime count.

Signed-off-by: Aiden Hu <[email protected]>
Set proper value for some resources. Use configurable fps.

Signed-off-by: Aiden Hu <[email protected]>
@AidenHu
Copy link
Contributor Author

AidenHu commented Sep 5, 2025

@josuah, I have pushed another 3 commits to enhance the video stream transfer and optimize resource usage.
I noticed that you have successfully merged pr_usb_host_class_api2. To expedite the merging process for the current host video class, appreciate your any feedback. Thanks

@josuah
Copy link
Contributor

josuah commented Sep 5, 2025

There is a discussion about how to handle various aspects of the class API in:
#94590

So as of now, API1 is merged, and API2 is going to have slight modifications.
In addition, unit tests will be added, which will make it possible for this PR to include unit tests.

Something which can help getting favorable review is to get the CI passing.
For instance these NXP board status:

Thank you for the update!

@josuah josuah added the area: Video Video subsystem label Sep 5, 2025
@josuah
Copy link
Contributor

josuah commented Sep 10, 2025

The current roadmap as I understand it:

I think this would ready to merge after this.

Future improvement on the side of video can be done on follow-up PR to cover more features.
It might even be possible to remove the USB controls altogether for the sake of making this PR smaller and easier to review/merge.

Would that make sense?

@AidenHu
Copy link
Contributor Author

AidenHu commented Sep 16, 2025

@josuah,
I have updated the processing about the video control based on the usb_common_uvc.c . The commit is usb_video_ctrl
If you have time, please help to review. Based on the current code, I have to expose the video_find_ctrl as an API and I have created another commit video_subsys_video_ctrl. Hope your any comment, thanks.

.type = UVC_CONTROL_SIGNED,
},
{
.size = 1,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @josuah,
Here the size should be 2 for PU_CONTRAST_CONTROL

1. Use the uvc_control_map from usb_common_uvc.c.
2. Use GET_MIN/GET_MAX/GET_RES/GET_DEF request to query device.
3. Unify the handling for single video control

Signed-off-by: Aiden Hu <[email protected]>
video_find_ctrl becomes an API.

Signed-off-by: Aiden Hu <[email protected]>
@AidenHu AidenHu force-pushed the enable_usb_host_video_class branch from 481d326 to 7af56c4 Compare September 17, 2025 08:35
Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
E Reliability Rating on New Code (required ≥ C)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: USB Universal Serial Bus area: Video Video subsystem
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants