Laurent, On 06/03/17 22:50, Laurent Pinchart wrote: > Hi Roger, > > Thank you for the patch. > > On Friday 03 Mar 2017 13:17:15 Roger Quadros wrote: >> On alternate setting change, webcam gadget sends us a UVC_EVENT_STREAMON >> or UVC_EVENT_STREAMOFF event. It expects delayed status response on >> STREAMON event only but doesn't expect us to send that response over USB. >> It sends the delayed response when we issue the VIDIOC_STREAMON ioctl. >> >> So we must not send UVCIOC_SEND_RESPONSE ioctl in these cases that too >> with invalid response length. > > The commit message only explains why we should not call UVCIOC_SEND_RESPONSE > in response to a STREAMON event, but not why we shouldn't either in response > to a STREAMOFF event. The patch is correct changing both, but I propose > wording the above two paragraphs as follows. > > "uvc-gadget: Do not send Set Interface (alternate setting) response twice > > On alternate setting change, the webcam gadget sends us a UVC_EVENT_STREAMON > or UVC_EVENT_STREAMOFF event. In the first case, the driver will issue a > delayed status response automatically when we call the VIDIOC_STREAMON ioctl. > In the second case, the driver sends the status response immediately. We must > thus not send the status response manually with UVCIOC_SEND_RESPONSE in any of > those cases." > > If you're fine with that I'll change the message when applying, there's no > need to resend the patch. I'm fine with your suggested commit message. Thanks. > >> Without this, the ISO streaming doesn't work if host application >> (e.g. luvcview) is closed and restarted. >> On dwc3 gadget controller it was resulting in Buffer Expiry error on >> the ISO endpoint. >> >> Signed-off-by: Roger Quadros <rogerq@xxxxxx> >> --- >> uvc-gadget.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/uvc-gadget.c b/uvc-gadget.c >> index 9ef315c..4d59ab8 100644 >> --- a/uvc-gadget.c >> +++ b/uvc-gadget.c >> @@ -597,12 +597,12 @@ uvc_events_process(struct uvc_device *dev) >> case UVC_EVENT_STREAMON: >> uvc_video_reqbufs(dev, 4); >> uvc_video_stream(dev, 1); >> - break; >> + return; >> >> case UVC_EVENT_STREAMOFF: >> uvc_video_stream(dev, 0); >> uvc_video_reqbufs(dev, 0); >> - break; >> + return; >> } >> >> ioctl(dev->fd, UVCIOC_SEND_RESPONSE, &resp); > -- cheers, -roger -- 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
