diff mbox

uvcvideo: add SetInterface(0) in .reset_resume handler

Message ID 20110716115100.10f6f764@tom-ThinkPad-T410 (mailing list archive)
State Superseded
Headers show

Commit Message

Ming Lei July 16, 2011, 3:51 a.m. UTC
As commented in uvc_video_init,

	/* Alternate setting 0 should be the default, yet the XBox Live Vision
	 * Cam (and possibly other devices) crash or otherwise misbehave if
	 * they don't receive a SET_INTERFACE request before any other video
	 * control request.
	 */

so it does make sense to add the SetInterface(0) in .reset_resume
handler so that this kind of devices can work well if they are reseted
during resume from system or runtime suspend.

We have found, without the patch, Microdia camera(0c45:6437) can't send
stream data any longer after it is reseted during resume from
system suspend.

Cc: Jeremy Kerr <jeremy.kerr@canonical.com>
Cc: Alan Stern <stern@rowland.harvard.edu>
Signed-off-by: Ming Lei <ming.lei@canonical.com>
---
 drivers/media/video/uvc/uvc_driver.c |   14 +++++++++++++-
 1 files changed, 13 insertions(+), 1 deletions(-)

Comments

Laurent Pinchart July 31, 2011, 3:38 p.m. UTC | #1
Hi Ming,

Thanks for the patch. I've queued it for v3.2 with a small modification (the 
usb_set_interface() call has been moved to uvc_video.c).

On Saturday 16 July 2011 05:51:00 Ming Lei wrote:
> As commented in uvc_video_init,
> 
> 	/* Alternate setting 0 should be the default, yet the XBox Live Vision
> 	 * Cam (and possibly other devices) crash or otherwise misbehave if
> 	 * they don't receive a SET_INTERFACE request before any other video
> 	 * control request.
> 	 */
> 
> so it does make sense to add the SetInterface(0) in .reset_resume
> handler so that this kind of devices can work well if they are reseted
> during resume from system or runtime suspend.
> 
> We have found, without the patch, Microdia camera(0c45:6437) can't send
> stream data any longer after it is reseted during resume from
> system suspend.
> 
> Cc: Jeremy Kerr <jeremy.kerr@canonical.com>
> Cc: Alan Stern <stern@rowland.harvard.edu>
> Signed-off-by: Ming Lei <ming.lei@canonical.com>
> ---
>  drivers/media/video/uvc/uvc_driver.c |   14 +++++++++++++-
>  1 files changed, 13 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/media/video/uvc/uvc_driver.c
> b/drivers/media/video/uvc/uvc_driver.c index b6eae48..41c6d1a 100644
> --- a/drivers/media/video/uvc/uvc_driver.c
> +++ b/drivers/media/video/uvc/uvc_driver.c
> @@ -1959,8 +1959,20 @@ static int __uvc_resume(struct usb_interface *intf,
> int reset) }
> 
>  	list_for_each_entry(stream, &dev->streams, list) {
> -		if (stream->intf == intf)
> +		if (stream->intf == intf) {
> +			/*
> +			 * After usb bus reset, some devices may
> +			 * misbehave if SetInterface(0) is not done, for
> +			 * example, Microdia camera(0c45:6437) will stop
> +			 * sending streaming data. I think XBox Live
> +			 * Vision Cam needs it too, as commented in
> +			 * uvc_video_init.
> +			 */
> +			if (reset)
> +				usb_set_interface(stream->dev->udev,
> +					stream->intfnum, 0);
>  			return uvc_video_resume(stream);
> +		}
>  	}
> 
>  	uvc_trace(UVC_TRACE_SUSPEND, "Resume: video streaming USB interface "
Ming Lei Aug. 1, 2011, 12:56 a.m. UTC | #2
Hi,

On Sun, Jul 31, 2011 at 11:38 PM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> Hi Ming,
>
> Thanks for the patch. I've queued it for v3.2 with a small modification (the
> usb_set_interface() call has been moved to uvc_video.c).

Thanks for queuing it.

Considered it is a fix patch, could you queue it for 3.1 -rcX as fix patch?
But anyway, it is up to you, :-)

thanks,
Laurent Pinchart Aug. 1, 2011, 11:26 a.m. UTC | #3
Hi Ming,

On Monday 01 August 2011 02:56:59 Ming Lei wrote:
> On Sun, Jul 31, 2011 at 11:38 PM, Laurent Pinchart wrote:
> > Hi Ming,
> > 
> > Thanks for the patch. I've queued it for v3.2 with a small modification
> > (the usb_set_interface() call has been moved to uvc_video.c).
> 
> Thanks for queuing it.
> 
> Considered it is a fix patch, could you queue it for 3.1 -rcX as fix patch?
> But anyway, it is up to you, :-)

It's not completely up to me :-) This patch falls in the "features that never 
worked" category. I've heard that Linus didn't want such non-regression fixes 
during the 3.0-rc phase. Mauro, is it still true for v3.1-rc ? Can I push this 
patch for 3.1, or does it need to wait until 3.2 ?
Mauro Carvalho Chehab Aug. 1, 2011, 5:39 p.m. UTC | #4
Em 01-08-2011 08:26, Laurent Pinchart escreveu:
> Hi Ming,
> 
> On Monday 01 August 2011 02:56:59 Ming Lei wrote:
>> On Sun, Jul 31, 2011 at 11:38 PM, Laurent Pinchart wrote:
>>> Hi Ming,
>>>
>>> Thanks for the patch. I've queued it for v3.2 with a small modification
>>> (the usb_set_interface() call has been moved to uvc_video.c).
>>
>> Thanks for queuing it.
>>
>> Considered it is a fix patch, could you queue it for 3.1 -rcX as fix patch?
>> But anyway, it is up to you, :-)
> 
> It's not completely up to me :-) This patch falls in the "features that never 
> worked" category. I've heard that Linus didn't want such non-regression fixes 
> during the 3.0-rc phase. Mauro, is it still true for v3.1-rc ? Can I push this 
> patch for 3.1, or does it need to wait until 3.2 ?

Theoretically, we're still with the open window. Well, send it to me. It is
a fix. I'll likely queue it to 3.1 and send on a next pull request, together with
a few other fixes probably on the next weekend.

Thanks,
Mauro
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Laurent Pinchart Aug. 1, 2011, 10:19 p.m. UTC | #5
Hi Mauro,

The following changes since commit 46540f7ac646ada7f22912ea7ea9b761ff5c4718:

  [media] ir-mce_kbd-decoder: include module.h for its facilities (2011-07-29 
12:52:23 -0300)

are available in the git repository at:
  git://linuxtv.org/pinchartl/uvcvideo.git uvcvideo-stable

Ming Lei (1):
      uvcvideo: Set alternate setting 0 on resume if the bus has been reset

 drivers/media/video/uvc/uvc_driver.c |    2 +-
 drivers/media/video/uvc/uvc_video.c  |   10 +++++++++-
 drivers/media/video/uvc/uvcvideo.h   |    2 +-
 3 files changed, 11 insertions(+), 3 deletions(-)
diff mbox

Patch

diff --git a/drivers/media/video/uvc/uvc_driver.c b/drivers/media/video/uvc/uvc_driver.c
index b6eae48..41c6d1a 100644
--- a/drivers/media/video/uvc/uvc_driver.c
+++ b/drivers/media/video/uvc/uvc_driver.c
@@ -1959,8 +1959,20 @@  static int __uvc_resume(struct usb_interface *intf, int reset)
 	}
 
 	list_for_each_entry(stream, &dev->streams, list) {
-		if (stream->intf == intf)
+		if (stream->intf == intf) {
+			/*
+			 * After usb bus reset, some devices may
+			 * misbehave if SetInterface(0) is not done, for
+			 * example, Microdia camera(0c45:6437) will stop
+			 * sending streaming data. I think XBox Live
+			 * Vision Cam needs it too, as commented in
+			 * uvc_video_init.
+			 */
+			if (reset)
+				usb_set_interface(stream->dev->udev,
+					stream->intfnum, 0);
 			return uvc_video_resume(stream);
+		}
 	}
 
 	uvc_trace(UVC_TRACE_SUSPEND, "Resume: video streaming USB interface "