* [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).