Hi Bhupesh,
On Thursday 09 August 2012 11:28:56 Bhupesh SHARMA wrote:
> On Thursday, August 09, 2012 3:12 AM Laurent Pinchart wrote:
> > On Thursday 09 August 2012 01:19:49 Bhupesh SHARMA wrote:
> > > On Wednesday, August 08, 2012 5:32 PM Laurent Pinchart wrote:
> > > > On Tuesday 07 August 2012 23:07:56 Bhupesh SHARMA wrote:
> > > > > On Tuesday, August 07, 2012 8:23 PM Laurent Pinchart wrote:
> > > > > > On Tuesday 31 July 2012 06:24:51 Bhupesh Sharma wrote:
> > [snip]
> >
> > > > > > > @@ -340,8 +344,13 @@ uvc_video_enable(struct uvc_video *video,
> > > > > > > int enable)
> > > > > > >
> > > > > > > }
> > > > > > >
> > > > > > > if (!enable) {
> > > > > > > - for (i = 0; i < UVC_NUM_REQUESTS; ++i)
> > > > > > > - usb_ep_dequeue(video->ep, video->req[i]);
> > > > > > > + /*
> > > > > > > + * dequeue requests on underlying UDC only if
> > > > > > > + * -ESHUTDOWN was not asserted by UDC earlier
> > > > > > > + */
> > > > > > > + if (!(queue->flags & UVC_QUEUE_DISCONNECTED))
> > > > > > > + for (i = 0; i < UVC_NUM_REQUESTS; ++i)
> > > > > > > + usb_ep_dequeue(video->ep, video->req[i]);
> > > > > >
> > > > > > What happens if you omit this check ?
> > > > >
> > > > > Actually the underlying UDCs clean-up their respective request list
> > > > > when they see a disconnect event from the Host.
> > > > >
> > > > > This makes them giveback all USB requests queued by an gadget with a
> > > > > status of ESHUTDOWN, so if we try to dequeue the 'already' dequeued
> > > > > requests from the UDC, it throws warning/errors that it doesn't have
> > > > > these requests queued any longer with it.
> > > >
> > > > OK. In that case, I'd like to fix the problem globally. When we run
> > > > out of V4L2 buffers the requests will end up being stored in the
> > > > req_free list by uvc_video_complete(). Those requests are not queued,
> > > > and should thus not be dequeued.
> > > >
> > > > Could you please split this change to another patch, and only dequeue
> > > > requests that are not queued ? This would require proper locking.
> > >
> > > Ok, but I have noticed that the underlying UDC will usually return all
> > > USB requests queued by the gadget when DISCONNECT happens.
> > >
> > > This means that no USB requests will be queued at UDC end when
> > > uvc_video_enable(0) is called. So if UDC saw a DISCONNECT from the Host,
> > > all the USB requests defined by the macro UVC_NUM_REQUESTS will become
> > > members of req_free list.
> > >
> > > So, in reality we don't have to dequeue any USB request on the UDC end,
> > > when uvc_video_enable(0) is called during DISCONNECT case (this is
> > > different from the case where user-space application itself working at
> > > the UVC gadget side is closed, in which case these requests will still
> > > be either queued to UDC or will be members of req_free list).
> > >
> > > In DISCONNECT from host, USB requests out of the UVC_NUM_REQUESTS count
> > > that were not already queued to the UDC are anyways not required to be
> > > dequeued and the rest have already been dequeued by the UDC itself. So,
> > > no USB requests need to be dequeued in this case.
> >
> > I agree with you about the disconnection case. My point was that the UVC
> > gadget driver tries to dequeue all requests unconditionally (or, with your
> > patch, in the non-disconnection case only). In the normal, non-
> > disconnection case, some requests can be in a non-queued state when
> > uvc_video_enable(0) is called, so we would run into the same problem as in
> > the disconnection case (trying to dequeue a non-queued request). I'd like
> > both issues to be fixed by a single patch, split from this one.
>
> Ok, now I got your point. You are talking about the non-disconnect case
> (disconnect initiated by the user-space application using the UVC gadget).
>
> I will send a separate patch to handle these cases, as suggested then.
Thank you. Please also drop the above change from this patch as it will then
be unneeded.
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
