On 02/03/2015 02:55 PM, Laurent Pinchart wrote: > Hi Hans, > > Thank you for the patch. > > On Tuesday 03 February 2015 13:47:24 Hans Verkuil wrote: >> From: Hans Verkuil <[email protected]> >> >> Instead of .ioctl use unlocked_ioctl. While all the queue ops >> already use a lock, there was no lock to protect uvc_video, so >> add that one. > > There's more. streamon and streamoff need to be protected by a lock for > instance. Wouldn't it be easier to just set vdev->lock for this driver > instead > of adding manual locking ?
I could set vdev->lock to &video->mutex and remove the queue->mutex altogether since video->mutex will now be used for all locking. I only need to take the video->mutex in uvc_v4l2_release() as well. If you agree with that, then I'll make that change. > >> Signed-off-by: Hans Verkuil <[email protected]> >> --- >> drivers/usb/gadget/function/f_uvc.c | 1 + >> drivers/usb/gadget/function/uvc.h | 1 + >> drivers/usb/gadget/function/uvc_v4l2.c | 6 +++++- >> 3 files changed, 7 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/usb/gadget/function/f_uvc.c >> b/drivers/usb/gadget/function/f_uvc.c index 945b3bd..748a80c 100644 >> --- a/drivers/usb/gadget/function/f_uvc.c >> +++ b/drivers/usb/gadget/function/f_uvc.c >> @@ -817,6 +817,7 @@ static struct usb_function *uvc_alloc(struct >> usb_function_instance *fi) if (uvc == NULL) >> return ERR_PTR(-ENOMEM); >> >> + mutex_init(&uvc->video.mutex); > > We need a corresponding mutex_destroy() somewhere. Why? Few drivers do so. If you want it, then I'll do that, but it's not required to my knowledge. Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
