Re: [PATCH 3/5] uvc gadget: switch to unlocked_ioctl.

[Hans Verkuil <hverkuil@xs4all.nl>]

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 <hans.verkuil@cisco.com>
>>
>> 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 <hans.verkuil@cisco.com>
>> ---
>>  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