linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Felipe Balbi <balbi@kernel.org>
To: Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
	Roger Quadros <rogerq@ti.com>
Cc: 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, 07 Mar 2017 12:57:40 +0200	[thread overview]
Message-ID: <87bmtd1hi3.fsf@linux.intel.com> (raw)
In-Reply-To: <3028836.5DEGZBmlnn@avalon>

[-- 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 --]

  parent reply	other threads:[~2017-03-07 11:01 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
2017-03-07 10:57   ` Felipe Balbi [this message]
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=87bmtd1hi3.fsf@linux.intel.com \
    --to=balbi@kernel.org \
    --cc=b-liu@ti.com \
    --cc=laurent.pinchart@ideasonboard.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=nsekhar@ti.com \
    --cc=rogerq@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).