Re: [PATCH v5 04/17] omap3isp: Add vb2_queue lock

[Ezequiel Garcia <ezequiel@collabora.com>]

On Thu, 2018-06-21 at 10:18 +0200, Hans Verkuil wrote:
> On 06/20/18 19:42, Ezequiel Garcia wrote:
> > vb2_queue locks is now mandatory. Add it, remove driver ad-hoc
> > locks,
> > and implement wait_{prepare, finish}.
> > 
> > Also, remove stream_lock mutex. Sicen the ioctls operations
> 
> Sicen -> Since
> 
> > are protected by the queue mutex, the stream_lock mutex is
> 
> are protected -> are now protected
> 

Will fix.

[snip]
> > @@ -1363,10 +1323,9 @@ static int isp_video_release(struct file
> > *file)
> > 
> >  	struct v4l2_fh *vfh = file->private_data;
> >  	struct isp_video_fh *handle = to_isp_video_fh(vfh);
> >  
> > +	mutex_lock(&video->queue_lock);

See below.

> >  	/* Disable streaming and free the buffers queue resources.
> > */
> >  	isp_video_streamoff(file, vfh, video->type);
> > -
> > -	mutex_lock(&video->queue_lock);
> >  	vb2_queue_release(&handle->queue);
> >  	mutex_unlock(&video->queue_lock);
> 
> Hmm, this mutex_unlock is not removed, did you miss this one?
> 

The mutex_lock call is moved before the call to isp_video_streamoff,
because .streamoff is called with the queue lock held,
and so it seemed more consistent.

I will send a v6 with the amended commit log.

Let me know if you catch anything else, it's a long series
and the devil is always in the detail!

Thanks,
Eze