Hi Laurent,

On Wed, Jan 30, 2019 at 11:17:37AM +0200, Laurent Pinchart wrote:
> Hi Sakari,
> 
> Thank you for the patch.
> 
> On Tue, Jan 29, 2019 at 11:49:44PM +0200, Sakari Ailus wrote:
> > The UVC video driver converts the timestamp from hardware specific unit to
> > one known by the kernel at the time when the buffer is dequeued. This is
> > fine in general, but the streamoff operation consists of the following
> > steps (among other things):
> > 
> > 1. uvc_video_clock_cleanup --- the hardware clock sample array is
> >    released and the pointer to the array is set to NULL,
> > 
> > 2. buffers in active state are returned to the user and
> > 
> > 3. buf_finish callback is called on buffers that are prepared. buf_finish
> >    includes calling uvc_video_clock_update that accesses the hardware
> >    clock sample array.
> > 
> > The above is serialised by a queue specific mutex. Address the problem by
> > skipping the clock conversion if the hardware clock sample array is
> > already released.
> > 
> > Reported-by: Chiranjeevi Rapolu <chiranjeevi.rap...@intel.com>
> > Tested-by: Chiranjeevi Rapolu <chiranjeevi.rap...@intel.com>
> > Signed-off-by: Sakari Ailus <sakari.ai...@linux.intel.com>
> 
> The analysis looks good to me.
> 
> Reviewed-by: Laurent Pinchart <laurent.pinch...@ideasonboard.com>
> 
> > ---
> > Hi Laurent,
> > 
> > This seems like something that's been out there for a while... I'll figure
> > out soon which stable kernels should receive it, if any.
> 
> Should I wait for the proper Fixes: and Cc:stable tags before queuing
> this patch then ?

Please do. I'll figure these out in a moment...

> 
> >  drivers/media/usb/uvc/uvc_video.c | 7 +++++++
> >  1 file changed, 7 insertions(+)
> > 
> > diff --git a/drivers/media/usb/uvc/uvc_video.c 
> > b/drivers/media/usb/uvc/uvc_video.c
> > index 84525ff047450..a30c5e1893e72 100644
> > --- a/drivers/media/usb/uvc/uvc_video.c
> > +++ b/drivers/media/usb/uvc/uvc_video.c
> > @@ -676,6 +676,13 @@ void uvc_video_clock_update(struct uvc_streaming 
> > *stream,
> >     if (!uvc_hw_timestamps_param)
> >             return;
> >  
> > +   /*
> > +    * We may get called if there are buffers done but not dequeued by the
> > +    * user. Just bail out in that case.
> > +    */
> > +   if (!clock->samples)
> > +           return;
> > +
> >     spin_lock_irqsave(&clock->lock, flags);
> >  
> >     if (clock->count < clock->size)

-- 
Regards,

Sakari Ailus
sakari.ai...@linux.intel.com

Reply via email to