* [PATCH] uvc-gadget: Fix Set Interface (alternate setting) response behaviour @ 2017-03-03 11:17 Roger Quadros 2017-03-06 14:29 ` Felipe Balbi 2017-03-06 20:50 ` Laurent Pinchart 0 siblings, 2 replies; 8+ messages in thread From: Roger Quadros @ 2017-03-03 11:17 UTC (permalink / raw) To: laurent.pinchart Cc: balbi, b-liu, nsekhar, linux-usb, linux-kernel, Roger Quadros <<< No Message Collected >>> ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] uvc-gadget: Fix Set Interface (alternate setting) response behaviour 2017-03-03 11:17 [PATCH] uvc-gadget: Fix Set Interface (alternate setting) response behaviour Roger Quadros @ 2017-03-06 14:29 ` Felipe Balbi 2017-03-06 14:39 ` Bin Liu 2017-03-06 20:50 ` Laurent Pinchart 1 sibling, 1 reply; 8+ messages in thread From: Felipe Balbi @ 2017-03-06 14:29 UTC (permalink / raw) To: Roger Quadros, laurent.pinchart Cc: b-liu, nsekhar, linux-usb, linux-kernel, Roger Quadros [-- Attachment #1: Type: text/plain, Size: 195 bytes --] Hi, Roger Quadros <rogerq@ti.com> writes: > <<< No Message Collected >>> You need to resend this. See also [1] [1] https://marc.info/?l=linux-usb&m=148854335620717&w=2 -- balbi [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 832 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] uvc-gadget: Fix Set Interface (alternate setting) response behaviour 2017-03-06 14:29 ` Felipe Balbi @ 2017-03-06 14:39 ` Bin Liu 0 siblings, 0 replies; 8+ messages in thread From: Bin Liu @ 2017-03-06 14:39 UTC (permalink / raw) To: Felipe Balbi Cc: Roger Quadros, laurent.pinchart, nsekhar, linux-usb, linux-kernel On Mon, Mar 06, 2017 at 04:29:33PM +0200, Felipe Balbi wrote: > > Hi, > > Roger Quadros <rogerq@ti.com> writes: > > <<< No Message Collected >>> > > You need to resend this. See also [1] Not sure what is wrong. This happens to me too, see [2]. And I sent the patch v2 an hour ago, but the patch is still not on the mailing list yet, normally it doesn't take that long... > > [1] https://marc.info/?l=linux-usb&m=148854335620717&w=2 [2] http://www.spinics.net/lists/linux-usb/msg154158.html Regards, -Bin. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] uvc-gadget: Fix Set Interface (alternate setting) response behaviour 2017-03-03 11:17 [PATCH] uvc-gadget: Fix Set Interface (alternate setting) response behaviour Roger Quadros 2017-03-06 14:29 ` Felipe Balbi @ 2017-03-06 20:50 ` Laurent Pinchart 2017-03-07 8:41 ` Roger Quadros 2017-03-07 10:57 ` Felipe Balbi 1 sibling, 2 replies; 8+ messages in thread From: Laurent Pinchart @ 2017-03-06 20:50 UTC (permalink / raw) To: Roger Quadros; +Cc: balbi, b-liu, nsekhar, linux-usb, linux-kernel 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. > 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@ti.com> > --- > 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); -- Regards, Laurent Pinchart ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] uvc-gadget: Fix Set Interface (alternate setting) response behaviour 2017-03-06 20:50 ` Laurent Pinchart @ 2017-03-07 8:41 ` Roger Quadros 2017-03-07 10:57 ` Felipe Balbi 1 sibling, 0 replies; 8+ messages in thread From: Roger Quadros @ 2017-03-07 8:41 UTC (permalink / raw) To: Laurent Pinchart; +Cc: balbi, b-liu, nsekhar, linux-usb, linux-kernel 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@ti.com> >> --- >> 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] uvc-gadget: Fix Set Interface (alternate setting) response behaviour 2017-03-06 20:50 ` Laurent Pinchart 2017-03-07 8:41 ` Roger Quadros @ 2017-03-07 10:57 ` Felipe Balbi 2017-03-07 12:29 ` Laurent Pinchart 1 sibling, 1 reply; 8+ messages in thread From: Felipe Balbi @ 2017-03-07 10:57 UTC (permalink / raw) To: Laurent Pinchart, Roger Quadros; +Cc: b-liu, nsekhar, linux-usb, linux-kernel [-- Attachment #1: Type: text/plain, Size: 1684 bytes --] Hi, Laurent Pinchart <laurent.pinchart@ideasonboard.com> writes: > 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 have this in my testing/fixes and was planning to send it to Greg this week. I can drop it from my queue, no problem, but then let me know as you would need my acked-by. -- balbi [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 832 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] uvc-gadget: Fix Set Interface (alternate setting) response behaviour 2017-03-07 10:57 ` Felipe Balbi @ 2017-03-07 12:29 ` Laurent Pinchart 2017-03-07 12:56 ` Felipe Balbi 0 siblings, 1 reply; 8+ messages in thread From: Laurent Pinchart @ 2017-03-07 12:29 UTC (permalink / raw) To: Felipe Balbi; +Cc: Roger Quadros, b-liu, nsekhar, linux-usb, linux-kernel Hi Felipe, On Tuesday 07 Mar 2017 12:57:40 Felipe Balbi wrote: > Laurent Pinchart writes: > > 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 have this in my testing/fixes and was planning to send it to Greg this > week. I can drop it from my queue, no problem, but then let me know as > you would need my acked-by. This is a userspace application patch. Feel free to send it to Greg, but I don't think he will know what to do with it :-) Were you maybe confusing this patch with the kernel fix that Roger sent a few days ago ? That one should be queued, please keep it in your tree. -- Regards, Laurent Pinchart ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] uvc-gadget: Fix Set Interface (alternate setting) response behaviour 2017-03-07 12:29 ` Laurent Pinchart @ 2017-03-07 12:56 ` Felipe Balbi 0 siblings, 0 replies; 8+ messages in thread From: Felipe Balbi @ 2017-03-07 12:56 UTC (permalink / raw) To: Laurent Pinchart; +Cc: Roger Quadros, b-liu, nsekhar, linux-usb, linux-kernel [-- Attachment #1: Type: text/plain, Size: 2186 bytes --] Hi, Laurent Pinchart <laurent.pinchart@ideasonboard.com> writes: > Hi Felipe, > > On Tuesday 07 Mar 2017 12:57:40 Felipe Balbi wrote: >> Laurent Pinchart writes: >> > 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 have this in my testing/fixes and was planning to send it to Greg this >> week. I can drop it from my queue, no problem, but then let me know as >> you would need my acked-by. > > This is a userspace application patch. Feel free to send it to Greg, but I > don't think he will know what to do with it :-) Were you maybe confusing this > patch with the kernel fix that Roger sent a few days ago ? That one should be > queued, please keep it in your tree. heh, my bad I got confused. I thought I was revieweing the kernel patch :-p -- balbi [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 832 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2017-03-07 12:59 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-03-03 11:17 [PATCH] uvc-gadget: Fix Set Interface (alternate setting) response behaviour Roger Quadros 2017-03-06 14:29 ` Felipe Balbi 2017-03-06 14:39 ` Bin Liu 2017-03-06 20:50 ` Laurent Pinchart 2017-03-07 8:41 ` Roger Quadros 2017-03-07 10:57 ` Felipe Balbi 2017-03-07 12:29 ` Laurent Pinchart 2017-03-07 12:56 ` Felipe Balbi
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).