On Tue, Oct 05, 2021 at 01:29:25AM +0300, Laurent Pinchart wrote:
Hi Michael, Thank you for the patch. On Thu, Sep 30, 2021 at 12:27:15PM +0200, Michael Grzeschik wrote:If the streaming endpoint is not enabled, the worker has nothing to do. In the case it still got enqueued, this patch ensueres that it will bails/it still got enqueued/buffers are still queued/ s/ensueres/ensures/
Thanks, will fix this and your other comments on the patches in v3.
out without handling any data.Does this happen when uvc_function_set_alt() calls usb_ep_disable() ? The current implementation will cause usb_ep_queue() (called from uvcg_video_ep_queue(), from uvcg_video_pump()) to return an error in that case, which will result in uvcg_queue_cancel() being called in uvcg_video_pump(). With this patch, I believe this will still work fine as userspace is notified of the stream off event and calls VIDIOC_STREAMOFF, which in turn calls uvcg_video_enable(0) from uvc_v4l2_streamoff() (or uvcg_video_enable(0) gets called from uvc_v4l2_release() in the worst case if the application closes the device). Could you confirm that your understanding matches this analysis ? If so,
Yes! I can confirm your analysis correct.
Reviewed-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>Signed-off-by: Michael Grzeschik <m.grzeschik@xxxxxxxxxxxxxx> --- drivers/usb/gadget/function/uvc_video.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/drivers/usb/gadget/function/uvc_video.c b/drivers/usb/gadget/function/uvc_video.c index ccee35177411d..152647495fa61 100644 --- a/drivers/usb/gadget/function/uvc_video.c +++ b/drivers/usb/gadget/function/uvc_video.c @@ -335,12 +335,12 @@ static void uvcg_video_pump(struct work_struct *work) { struct uvc_video *video = container_of(work, struct uvc_video, pump); struct uvc_video_queue *queue = &video->queue; - struct usb_request *req; + struct usb_request *req = NULL; struct uvc_buffer *buf; unsigned long flags; int ret; - while (1) { + while (video->ep->enabled) { /* Retrieve the first available USB request, protected by the * request lock. */ @@ -390,6 +390,9 @@ static void uvcg_video_pump(struct work_struct *work) video->req_int_count++; } + if (!req) + return; + spin_lock_irqsave(&video->req_lock, flags); list_add_tail(&req->list, &video->req_free); spin_unlock_irqrestore(&video->req_lock, flags);-- Regards, Laurent Pinchart
-- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
Attachment:
signature.asc
Description: PGP signature