| Message ID | 20110726113956.17e7acf3@tom-ThinkPad-T410 |
|---|---|
| State | New |
| Headers | show |
On Tue, Jul 26, 2011 at 11:39:56AM +0800, Ming Lei wrote: > From 2f485a24a5732b980f2d1c7097469f07cb848c70 Mon Sep 17 00:00:00 2001 > From: Ming Lei <ming.lei@canonical.com> > Date: Sat, 16 Jul 2011 11:29:45 +0800 > Subject: [PATCH][Natty SRU] UBUNTU: SAUCE: uvcvideo: add SetInterface(0) in .reset_resume handler > > 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. > > SRU Justification: > > Impact: > - without the patch, the Microdia camera(0c45:6437) can't > work after system resume > > Fix: > - After applying the patch, the camera can work well after > system resume > > BugLink: http://bugs.launchpad.net/bugs/733509 > > upstream discusstion: http://marc.info/?t=131037773100002&r=1&w=2 > > Signed-off-by: Ming Lei <ming.lei@canonical.com> > Cc: Jeremy Kerr <jeremy.kerr@canonical.com> > --- > BTW: The patch will enter 3.1-rc later, not sure how Oneiric will merge > the patch. > --- > 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. Looks like 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 " That is going to need some careful testing as it is applied to all devices. It appears to be sane however and do what it claims and upstream seems happy with the new behaviour. As I say this will need careful monitoring in testing. I am unsure if this might make it to Oneiric via stable, but for our purposes it might make sense to apply this there and see what fallout we get before committing it for Natty. Overally I guess: Acked-by: Andy Whitcroft <apw@canonical.com> -apw
On 07/25/2011 09:39 PM, Ming Lei wrote: > From 2f485a24a5732b980f2d1c7097469f07cb848c70 Mon Sep 17 00:00:00 2001 > From: Ming Lei<ming.lei@canonical.com> > Date: Sat, 16 Jul 2011 11:29:45 +0800 > Subject: [PATCH][Natty SRU] UBUNTU: SAUCE: uvcvideo: add SetInterface(0) in .reset_resume handler > > 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. > > SRU Justification: > > Impact: > - without the patch, the Microdia camera(0c45:6437) can't > work after system resume > > Fix: > - After applying the patch, the camera can work well after > system resume > > BugLink: http://bugs.launchpad.net/bugs/733509 > > upstream discusstion: http://marc.info/?t=131037773100002&r=1&w=2 > > Signed-off-by: Ming Lei<ming.lei@canonical.com> > Cc: Jeremy Kerr<jeremy.kerr@canonical.com> > --- > BTW: The patch will enter 3.1-rc later, not sure how Oneiric will merge > the patch. > --- > 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. Looks like 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 " Its best if you remind us when its been merged in Linus' tree so we can just cherry-pick it in its final form. rtg
On Tue, 26 Jul 2011 07:04:00 -0600 Tim Gardner <tim.gardner@canonical.com> wrote: > > > > uvc_trace(UVC_TRACE_SUSPEND, "Resume: video streaming USB interface " > > Its best if you remind us when its been merged in Linus' tree so we can > just cherry-pick it in its final form. No problem, I will put an eye on it, and let you know if the patch is merged into linus tree. thanks,
Hi, On Tue, 26 Jul 2011 22:02:45 +0800 Ming Lei <ming.lei@canonical.com> wrote: > On Tue, 26 Jul 2011 07:04:00 -0600 > Tim Gardner <tim.gardner@canonical.com> wrote: > > > > > > uvc_trace(UVC_TRACE_SUSPEND, "Resume: video streaming USB interface " > > > > Its best if you remind us when its been merged in Linus' tree so we can > > just cherry-pick it in its final form. > > No problem, I will put an eye on it, and let you know if the patch is > merged into linus tree. The patch has been in -next tree and media tree for some days, so could you take the patch now so that it won't block hw certification any longer. This one is OK for Natty, but for Oneric, you may need to take the one from -next or media tree. thanks,
On 07/25/2011 08:39 PM, Ming Lei wrote: > From 2f485a24a5732b980f2d1c7097469f07cb848c70 Mon Sep 17 00:00:00 2001 > From: Ming Lei<ming.lei@canonical.com> > Date: Sat, 16 Jul 2011 11:29:45 +0800 > Subject: [PATCH][Natty SRU] UBUNTU: SAUCE: uvcvideo: add SetInterface(0) in .reset_resume handler > > 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. > > SRU Justification: > > Impact: > - without the patch, the Microdia camera(0c45:6437) can't > work after system resume > > Fix: > - After applying the patch, the camera can work well after > system resume > > BugLink: http://bugs.launchpad.net/bugs/733509 > > upstream discusstion: http://marc.info/?t=131037773100002&r=1&w=2 > > Signed-off-by: Ming Lei<ming.lei@canonical.com> > Cc: Jeremy Kerr<jeremy.kerr@canonical.com> > --- > BTW: The patch will enter 3.1-rc later, not sure how Oneiric will merge > the patch. > --- > 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. Looks like 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 " Please note that the buglink referred to in the commit is a Private bug. Brad
On Thu, Sep 29, 2011 at 1:34 PM, Brad Figg <brad.figg@canonical.com> wrote: > On 07/25/2011 08:39 PM, Ming Lei wrote: >> >> From 2f485a24a5732b980f2d1c7097469f07cb848c70 Mon Sep 17 00:00:00 2001 >> From: Ming Lei<ming.lei@canonical.com> >> Date: Sat, 16 Jul 2011 11:29:45 +0800 >> Subject: [PATCH][Natty SRU] UBUNTU: SAUCE: uvcvideo: add SetInterface(0) >> in .reset_resume handler >> >> 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. >> >> SRU Justification: >> >> Impact: >> - without the patch, the Microdia camera(0c45:6437) can't >> work after system resume >> >> Fix: >> - After applying the patch, the camera can work well after >> system resume >> >> BugLink: http://bugs.launchpad.net/bugs/733509 >> >> upstream discusstion: http://marc.info/?t=131037773100002&r=1&w=2 >> >> Signed-off-by: Ming Lei<ming.lei@canonical.com> >> Cc: Jeremy Kerr<jeremy.kerr@canonical.com> >> --- >> BTW: The patch will enter 3.1-rc later, not sure how Oneiric will merge >> the patch. >> --- >> 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. Looks like 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 >> " > > Please note that the buglink referred to in the commit is a Private bug. We has a public butlink for it already: https://bugs.launchpad.net/bugs/816484 thanks, -- Ming Lei
Hi, On Thu, Sep 29, 2011 at 3:04 PM, Ming Lei <ming.lei@canonical.com> wrote: > On Thu, Sep 29, 2011 at 1:34 PM, Brad Figg <brad.figg@canonical.com> >> Please note that the buglink referred to in the commit is a Private bug. > > We has a public butlink for it already: > > https://bugs.launchpad.net/bugs/816484 Today we have another report on the same bug: https://bugs.launchpad.net/bugs/861685 thanks, -- Ming Lei
On Thu, Sep 29, 2011 at 11:53:35AM +0800, Ming Lei wrote: > > Hi, > > On Tue, 26 Jul 2011 22:02:45 +0800 > Ming Lei <ming.lei@canonical.com> wrote: > > > On Tue, 26 Jul 2011 07:04:00 -0600 > > Tim Gardner <tim.gardner@canonical.com> wrote: > > > > > > > > uvc_trace(UVC_TRACE_SUSPEND, "Resume: video streaming USB interface " > > > > > > Its best if you remind us when its been merged in Linus' tree so we can > > > just cherry-pick it in its final form. > > > > No problem, I will put an eye on it, and let you know if the patch is > > merged into linus tree. > > The patch has been in -next tree and media tree for some days, so could you > take the patch now so that it won't block hw certification any longer. > > This one is OK for Natty, but for Oneric, you may need to take the one from > -next or media tree. Could you point us at the patch in those trees? With the kernel.org debackle its hard to know even where to look for the tree let alone the patch. -apw
Hi, On Thu, Sep 29, 2011 at 5:03 PM, Andy Whitcroft <apw@canonical.com> > Could you point us at the patch in those trees? With the kernel.org > debackle its hard to know even where to look for the tree let alone the > patch. You may find the patch in the tree[1], and the pull request from maintainer in link[2]. [1],http://git.linuxtv.org/media_tree.git/commit/dd182e5416e872e30d90cf071758eec1cf6340d5 [2], http://marc.info/?l=linux-usb&m=131223713719091&w=2 thanks, -- Ming Lei
On Thu, 2011-09-29 at 17:27 +0800, Ming Lei wrote: > Hi, > > On Thu, Sep 29, 2011 at 5:03 PM, Andy Whitcroft <apw@canonical.com> > > Could you point us at the patch in those trees? With the kernel.org > > debackle its hard to know even where to look for the tree let alone the > > patch. > > You may find the patch in the tree[1], and the pull request from maintainer > in link[2]. > > > [1],http://git.linuxtv.org/media_tree.git/commit/dd182e5416e872e30d90cf071758eec1cf6340d5 This patch cherry-picks cleanly into Oneiric from the above repo. This does appear to have a higher risk of regression as it affects all devices. However, given the current upstream status and testing: Acked-by: Leann Ogasawara <leann.ogasawara@canonical.com> > [2], http://marc.info/?l=linux-usb&m=131223713719091&w=2 > > thanks, > -- > Ming Lei >
On 09/28/2011 09:53 PM, Ming Lei wrote: > > Hi, > > On Tue, 26 Jul 2011 22:02:45 +0800 > Ming Lei<ming.lei@canonical.com> wrote: > >> On Tue, 26 Jul 2011 07:04:00 -0600 >> Tim Gardner<tim.gardner@canonical.com> wrote: >>>> >>>> uvc_trace(UVC_TRACE_SUSPEND, "Resume: video streaming USB interface " >>> >>> Its best if you remind us when its been merged in Linus' tree so we can >>> just cherry-pick it in its final form. >> >> No problem, I will put an eye on it, and let you know if the patch is >> merged into linus tree. > > The patch has been in -next tree and media tree for some days, so could you > take the patch now so that it won't block hw certification any longer. > > This one is OK for Natty, but for Oneric, you may need to take the one from > -next or media tree. > > > thanks, Applied to Natty, Oneiric, and P
On Fri, Sep 30, 2011 at 12:42 AM, Leann Ogasawara <leann.ogasawara@canonical.com> wrote: > On Thu, 2011-09-29 at 17:27 +0800, Ming Lei wrote: >> Hi, >> >> On Thu, Sep 29, 2011 at 5:03 PM, Andy Whitcroft <apw@canonical.com> >> > Could you point us at the patch in those trees? With the kernel.org >> > debackle its hard to know even where to look for the tree let alone the >> > patch. >> >> You may find the patch in the tree[1], and the pull request from maintainer >> in link[2]. >> >> >> [1],http://git.linuxtv.org/media_tree.git/commit/dd182e5416e872e30d90cf071758eec1cf6340d5 > > This patch cherry-picks cleanly into Oneiric from the above repo. This > does appear to have a higher risk of regression as it affects all > devices. However, given the current upstream status and testing: > > Acked-by: Leann Ogasawara <leann.ogasawara@canonical.com> > >> [2], http://marc.info/?l=linux-usb&m=131223713719091&w=2 The patch has been merged into upstream v3.1-rc9, so could it be merged into Oneiric now? Our certification team is waiting for it [1], :-) [1], https://bugs.launchpad.net/bugs/816484. thanks, -- Ming Lei
On Wed, 2011-10-12 at 09:11 +0800, Ming Lei wrote: > On Fri, Sep 30, 2011 at 12:42 AM, Leann Ogasawara > <leann.ogasawara@canonical.com> wrote: > > On Thu, 2011-09-29 at 17:27 +0800, Ming Lei wrote: > >> Hi, > >> > >> On Thu, Sep 29, 2011 at 5:03 PM, Andy Whitcroft <apw@canonical.com> > >> > Could you point us at the patch in those trees? With the kernel.org > >> > debackle its hard to know even where to look for the tree let alone the > >> > patch. > >> > >> You may find the patch in the tree[1], and the pull request from maintainer > >> in link[2]. > >> > >> > >> [1],http://git.linuxtv.org/media_tree.git/commit/dd182e5416e872e30d90cf071758eec1cf6340d5 > > > > This patch cherry-picks cleanly into Oneiric from the above repo. This > > does appear to have a higher risk of regression as it affects all > > devices. However, given the current upstream status and testing: > > > > Acked-by: Leann Ogasawara <leann.ogasawara@canonical.com> > > > >> [2], http://marc.info/?l=linux-usb&m=131223713719091&w=2 > > The patch has been merged into upstream v3.1-rc9, so could it > be merged into Oneiric now? It was applied to Oneiric back on Sept 30: https://lists.ubuntu.com/archives/kernel-team/2011-September/017199.html ~/ubuntu-oneiric$ git show 13bede6cf7c329a3037d2dfbd5a7889bd1e6a016 commit 13bede6cf7c329a3037d2dfbd5a7889bd1e6a016 Author: Ming Lei <tom.leiming@gmail.com> Date: Sat Jul 16 00:51:00 2011 -0300 UBUNTU: SAUCE: [media] uvcvideo: Set alternate setting 0 on resume if the bus has been reset I would note that it's in the Oneiric master-next branch in our git repo. It will be released with the first Oneiric kernel SRU. Thanks, Leann > Our certification team is waiting for it [1], :-) > > [1], https://bugs.launchpad.net/bugs/816484. > > thanks, > -- > Ming Lei
Hi, On Wed, Oct 12, 2011 at 11:46 AM, Leann Ogasawara > I would note that it's in the Oneiric master-next branch in our git > repo. It will be released with the first Oneiric kernel SRU. Thanks. Sorry for not being aware of master-next branch, :-) thanks, -- Ming Lei
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. Looks like 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 "