From: Roger Quadros <rogerq@ti.com>
To: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: <balbi@kernel.org>, <b-liu@ti.com>, <nsekhar@ti.com>,
<linux-usb@vger.kernel.org>, <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] uvc-gadget: Fix Set Interface (alternate setting) response behaviour
Date: Tue, 7 Mar 2017 10:41:06 +0200 [thread overview]
Message-ID: <4c33f742-cdb7-92b2-9596-efe99443f89c@ti.com> (raw)
In-Reply-To: <3028836.5DEGZBmlnn@avalon>
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
next prev parent reply other threads:[~2017-03-07 8:41 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
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 [this message]
2017-03-07 10:57 ` Felipe Balbi
2017-03-07 12:29 ` Laurent Pinchart
2017-03-07 12:56 ` Felipe Balbi
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4c33f742-cdb7-92b2-9596-efe99443f89c@ti.com \
--to=rogerq@ti.com \
--cc=b-liu@ti.com \
--cc=balbi@kernel.org \
--cc=laurent.pinchart@ideasonboard.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-usb@vger.kernel.org \
--cc=nsekhar@ti.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).