-
Notifications
You must be signed in to change notification settings - Fork 37
feat(usb_host): UVC Host add suspend/resume events #328
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
base: master
Are you sure you want to change the base?
Conversation
50c067a to
872764b
Compare
bf2051b to
5e417ee
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR is being reviewed by Cursor Bugbot
Details
Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
5e417ee to
76274d5
Compare
- usb_host lib supports global suspend and resume
- backward compatibility with older IDF releases
- UVC Host target tests
76274d5 to
4aac6fa
Compare
|
Updated the MR with the recent #333 PR (Refactor(usb_host): Rename auto suspend timer public API), fixed cursor review into a separate commit. And rebased. |
roma-jam
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Two questions from my side.
| goto exit; | ||
| } | ||
|
|
||
| if (UVC_ATOMIC_LOAD(uvc_stream->dynamic.streaming)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just to understand it better for me.
If we are streaming, logically the SUSPEND event should never occur, right?
Because there is a constant ongoing urb transfers, which restart the auto suspend timer every time.
So, the SUSPEND event might occur, when we have several devices attached to the host and num_urb_pending == 0 ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The suspend even can occur any time.
Yes, the suspend timer gets reset whenever there is an activity either from a class driver, or from the usb_host_lib itself. But the suspend timer is only used for automatic suspend.
The suspend event can occur anytime, as it is usb_host_lib call. Thus even during streaming. The usb_host_lib automatically clears and halts all the pipes (does not matter if a client is actively using some pipe or not). Once the suspend procedure is over, the client only gets a notification. Aka, there is nothing that client can do, to prevent the suspend from happening.
| goto exit; | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't need to change the state for the uvc_stream to protect the stream or to handle the stream resuming somehow later, because if device was suspended, we can just enable the streaming and this will automatically resume the root port?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Essentially yes. The driver just pauses the stream to return frames (there is already not any activity on the bus, as the usb_host_lib halts all the EPs) after receiving suspend event.
Upon receiving resume event, the stream must be unpaused explicitly by the user.
Or, there is the automatic resume by transfer submit feature, so the user can continue streaming just by calling uvc_host_stream_unpause(), as this function submits a transfer, which automatically resumes the root port.
After going into suspended state, the user still owns the stream_hdl so he can simply unpause the stream and the device will resume automatically. I added a test case, which tests this here
Description
This MR adds Suspend/Resume events for UVC class driver as a follow-up for the Global/Suspend resume
Changes
Added Suspend and Resume UVC Host interface events
Related
Testing
Checklist
Before submitting a Pull Request, please ensure the following:
Note
Adds suspend/resume handling to UVC Host, exposing new events and updating examples and tests accordingly.
UVC_HOST_DEVICE_SUSPENDEDandUVC_HOST_DEVICE_RESUMEDevents ininclude/usb/uvc_host.h(gated byUVC_HOST_SUSPEND_RESUME_API_SUPPORTED).uvc_host.c:uvc_host_device_suspended()to pause stream and ensure frames are returned.USB_HOST_CLIENT_EVENT_DEV_SUSPENDED/DEV_RESUMEDto notify clients and manage stream state.examples/basic_uvc_streamandexamples/camera_displayto log and handle new suspend/resume events (conditional on macro); refresh SPDX year incamera_display.c.test_app/main/test_uvc_host_pm.cwith suspend/resume scenarios (basic, while streaming, resume-on-transfer, close while suspended, open while suspended, concurrent suspend/disconnect, auto-suspend timer).test_app/sdkconfig.defaultsto enable indexed task notifications.CHANGELOG.mdwith “Added global suspend/resume support”.Written by Cursor Bugbot for commit 76274d5. This will update automatically on new commits. Configure here.