Hi Laurent, > -----Original Message----- > From: Laurent Pinchart [mailto:laurent.pinchart@xxxxxxxxxxxxxxxx] > Sent: Thursday, August 09, 2012 3:12 AM > To: Bhupesh SHARMA > Cc: linux-usb@xxxxxxxxxxxxxxx; balbi@xxxxxx > Subject: Re: [PATCH V2 1/2] usb: gadget/uvc: Port UVC webcam gadget to > use videobuf2 framework > > Hi Bhupesh, > > 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. Regards, Bhupesh -- 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
