Re: [PATCH] From 2.6.39-rc1 onward, the Logitech Quickcam Fusion webcam (046d:08c1) stops

2012-07-15 Thread Laurent Pinchart 
Hi Oleksij,

On Monday 09 July 2012 07:35:54 Oleksij Rempel wrote:
 Am 08.07.2012 23:35, schrieb Laurent Pinchart:
  On Sunday 08 July 2012 10:45:45 Oleksij Rempel wrote:
  On 08.07.2012 09:22, Eric Ding wrote:
  On 07/08/2012 02:52 PM, Oleksij Rempel (fishor) wrote:
  Hi, can you please do one more test.
  Try to reproduce it with MJPEG stream. Use this command:
  luvcview -f jpg -C
  
  it will force jpeg (i think your cam support it) and store each frame
  separately. If you can reproduce it, please send me one of this frames.
  
  Sure, I reproduced it fairly easily (with the kernel corresponding to
  commit e1620d5).  I've attached the first frame dumped by luvcview (I
  already converted it from raw MJPEG to JPEG w/ Huffman tables to save
  you the trouble).
  
  Eric
  
  P.S. I actually attached a video showing another example of this problem
  to my original bug report on Launchpad:
  
  https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1017072/+attachment
  /3202254/+files/SecondTime.avi
  
  Ok,
  
  i have no more doubt about quirk.
  The image should have fallowing path inside of the cam:
  - sensor
  - demosaicking processor
  - jpeg compressor
  - uvc packer
  
  since correct jpeg and uvc headers are produced, then the problem is on
  first two steps.
  
  We could speculate on where the problem is inside the webcam, but that
  won't make much of a difference :-) Logitech webcams have been known in
  the past to have pretty serious race conditions in their firmware. Some
  of them have been fixed, but I wouldn't be surprised if several other
  similar bugs remained.
 Hi Laurent,
 
 i do this description for my self. First of all i wont to understand how
 it work, to make better diagnosis.

It's a good idea, but don't try too hard, Logitech webcams can be *really* 
buggy ;-)

 Then if i'm wrong, some body can correct me. It also should help find cam
 with same symptoms. Indeed there are non logitech cams with same symptoms.
 The problem, i thought there are irreparably defect :(

I personally consider many webcams to be broken enough to qualify for a 
refund. Of course vendors probably disagree...

  I can try contacting Logitech about this, but I'm not sure whether they
  will even be interested in investigating the problem. Their Linux support
  track record isn't as good as it was a couple of years ago :-/
  
  I also tested my cam with USB_QUIRK_RESET_RESUME without any
  regressions, in case you wont all_logitech_quirk. My cam is:
  +   /* Logitech Quickcam Notebook Pro */
  +   { USB_DEVICE(0x046d, 0x0991), .driver_info =
  USB_QUIRK_RESET_RESUME },

-- 
Regards,

Laurent Pinchart

--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] From 2.6.39-rc1 onward, the Logitech Quickcam Fusion webcam (046d:08c1) stops

2012-07-15 Thread Laurent Pinchart 
Hi Eric,

On Thursday 12 July 2012 16:05:47 Eric Ding wrote:
 On 07/09/2012 10:17 PM, Alan Stern wrote:
  On Sun, 8 Jul 2012, Jonathan Nieder wrote:
  Eric Ding wrote:
  So it looks like you'd have to both look for USB_CLASS_VIDEO and check
  uvc_ids[] too... which becomes somewhat hairy, since I assume you don't
  realy want usb_detect_quirks() to reference UVC-specific structs...
  which brings us back to the original laundry list approach of naming
  several affected webcams explicitly, no?
  
  Why wouldn't I want usb_detect_quirks() to reference UVC-specific
  structs?
  
  Well, it's a layering violation.  Not to mention a duplication of code.
  
  But if the alternative is to list every buggy webcam made by Logitech,
  it might be worthwhile.
 
 So... now what, then?  Who decides which is the better of two evils:
 obvious code duplication vs. layering violation?  FWIW, it does seem
 like the number of Logitech webcams which aren't USB_CLASS_VIDEO is
 finite, including only older webcams, so perhaps listing every buggy
 webcam made by Logitech in two places (one in UVC code, one in USB core
 code) is not an invitation for long-term code maintenance nightmares.

I'm fine with both solutions. Handling the quirks in the USB core has my 
preference, as it would ensure that no race condition will cause any issue at 
probe time.

-- 
Regards,

Laurent Pinchart

--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] From 2.6.39-rc1 onward, the Logitech Quickcam Fusion webcam (046d:08c1) stops

2012-07-15 Thread Eric Ding 
On 07/15/2012 08:21 PM, Laurent Pinchart wrote:
 On Thursday 12 July 2012 16:05:47 Eric Ding wrote:
 So... now what, then?  Who decides which is the better of two evils:
 obvious code duplication vs. layering violation?  FWIW, it does seem
 like the number of Logitech webcams which aren't USB_CLASS_VIDEO is
 finite, including only older webcams, so perhaps listing every buggy
 webcam made by Logitech in two places (one in UVC code, one in USB core
 code) is not an invitation for long-term code maintenance nightmares.
 
 I'm fine with both solutions. Handling the quirks in the USB core has my 
 preference, as it would ensure that no race condition will cause any issue at 
 probe time.

So who actually writes an appropriate patch?  Like I said before, I'm no
kernel hacker, so I think it's best if someone more familiar with this
code than I am actually moves forward with the code mods... :-)

Eric
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] From 2.6.39-rc1 onward, the Logitech Quickcam Fusion webcam (046d:08c1) stops

2012-07-12 Thread Eric Ding 
On 07/09/2012 10:17 PM, Alan Stern wrote:
 On Sun, 8 Jul 2012, Jonathan Nieder wrote:
 
 Eric Ding wrote:

 So it looks like you'd have to both look for USB_CLASS_VIDEO and check
 uvc_ids[] too... which becomes somewhat hairy, since I assume you don't
 realy want usb_detect_quirks() to reference UVC-specific structs...
 which brings us back to the original laundry list approach of naming
 several affected webcams explicitly, no?

 Why wouldn't I want usb_detect_quirks() to reference UVC-specific
 structs?
 
 Well, it's a layering violation.  Not to mention a duplication of code.
 
 But if the alternative is to list every buggy webcam made by Logitech, 
 it might be worthwhile.

So... now what, then?  Who decides which is the better of two evils:
obvious code duplication vs. layering violation?  FWIW, it does seem
like the number of Logitech webcams which aren't USB_CLASS_VIDEO is
finite, including only older webcams, so perhaps listing every buggy
webcam made by Logitech in two places (one in UVC code, one in USB core
code) is not an invitation for long-term code maintenance nightmares.

Eric
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] From 2.6.39-rc1 onward, the Logitech Quickcam Fusion webcam (046d:08c1) stops

2012-07-12 Thread Oleksij Rempel 
Am 12.07.2012 10:05, schrieb Eric Ding:
 On 07/09/2012 10:17 PM, Alan Stern wrote:
 On Sun, 8 Jul 2012, Jonathan Nieder wrote:

 Eric Ding wrote:

 So it looks like you'd have to both look for USB_CLASS_VIDEO and check
 uvc_ids[] too... which becomes somewhat hairy, since I assume you don't
 realy want usb_detect_quirks() to reference UVC-specific structs...
 which brings us back to the original laundry list approach of naming
 several affected webcams explicitly, no?

 Why wouldn't I want usb_detect_quirks() to reference UVC-specific
 structs?

 Well, it's a layering violation.  Not to mention a duplication of code.

 But if the alternative is to list every buggy webcam made by Logitech, 
 it might be worthwhile.
 
 So... now what, then?  Who decides which is the better of two evils:
 obvious code duplication vs. layering violation?  FWIW, it does seem
 like the number of Logitech webcams which aren't USB_CLASS_VIDEO is
 finite, including only older webcams, so perhaps listing every buggy
 webcam made by Logitech in two places (one in UVC code, one in USB core
 code) is not an invitation for long-term code maintenance nightmares.
 
 Eric
 

according to device list here:
http://www.ideasonboard.org/uvc/
we can safely use range of usb ids
plus combine information from here:
https://usb-ids.gowdy.us/read/UD/046d

it looks like we can use range from 0800 - 9ff, but it will include some
usb microphones. I do not know if they are affected too.

-- 
Regards,
Oleksij


--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] From 2.6.39-rc1 onward, the Logitech Quickcam Fusion webcam (046d:08c1) stops

2012-07-08 Thread Oleksij Rempel (fishor) 

On 08.07.2012 07:37, Eric Ding wrote:

On 07/08/2012 09:52 AM, Alan Stern wrote:

On Sat, 7 Jul 2012, Rafael J. Wysocki wrote:


Well, the quirk does make sense.  What doesn't make sense is why moving
the runtime PM operation pointers from usb_bus_type to usb_device_type
should cause any change in the autosuspend behavior.  That's what we
would like to know.


I think the reason was the way rpm_suspend() worked at the time of that
commit (it works a bit differently now, but not as much as to avoid the
problem).

Namely, before commit e1620d591a75a10b15cf61dbf8243a0b7e6731a2 the device
had a device type without runtime PM callbacks.  So, rpm_suspend() saw
that dev-type was set and dev-type-pm was set, so it assigned NULL to
callback.  As a result, nothing happened when rpm_callback(callback, dev)
was run.


I don't follow.  If that were the reason then no USB device would have
been runtime-suspended before the e1620d commit.

Are you saying this actually was true for some period of time (such as
between the commit that added the callback variable and the e1620d
commit)?


I think this is, in fact, what happened.  See rpm_suspend() before and
after commit 9659cc0; I suspect that between commit 9659cc0 and commit
e1620d5, no USB device was being runtime-suspended.  Since this was
after v2.6.38 and before v2.6.39-rc1, though, it wouldn't have been
widely seen or tested, right?

However, that doesn't fully explain why the webcam wouldn't have been
autosuspended in v2.6.38 -- perhaps you all can guess at this more
quickly than I can?  Was enough changed in the runtime PM architecture
from v2.6.38 to e1620d to explain this difference?

Eric



Hi, can you please do one more test.
Try to reproduce it with MJPEG stream. Use this command:
luvcview -f jpg -C

it will force jpeg (i think your cam support it) and store each frame 
separately. If you can reproduce it, please send me one of this frames.

--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] From 2.6.39-rc1 onward, the Logitech Quickcam Fusion webcam (046d:08c1) stops

2012-07-08 Thread Eric Ding 
On 07/08/2012 02:52 PM, Oleksij Rempel (fishor) wrote:

 Hi, can you please do one more test.
 Try to reproduce it with MJPEG stream. Use this command:
 luvcview -f jpg -C
 
 it will force jpeg (i think your cam support it) and store each frame
 separately. If you can reproduce it, please send me one of this frames.

Sure, I reproduced it fairly easily (with the kernel corresponding to
commit e1620d5).  I've attached the first frame dumped by luvcview (I
already converted it from raw MJPEG to JPEG w/ Huffman tables to save
you the trouble).

Eric

P.S. I actually attached a video showing another example of this problem
to my original bug report on Launchpad:

https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1017072/+attachment/3202254/+files/SecondTime.avi
attachment: frame000.jpg

Re: [PATCH] From 2.6.39-rc1 onward, the Logitech Quickcam Fusion webcam (046d:08c1) stops

2012-07-08 Thread Rafael J. Wysocki 
On Sunday, July 08, 2012, Alan Stern wrote:
 On Sat, 7 Jul 2012, Rafael J. Wysocki wrote:
 
   Well, the quirk does make sense.  What doesn't make sense is why moving 
   the runtime PM operation pointers from usb_bus_type to usb_device_type
   should cause any change in the autosuspend behavior.  That's what we 
   would like to know.
  
  I think the reason was the way rpm_suspend() worked at the time of that
  commit (it works a bit differently now, but not as much as to avoid the
  problem).
  
  Namely, before commit e1620d591a75a10b15cf61dbf8243a0b7e6731a2 the device
  had a device type without runtime PM callbacks.  So, rpm_suspend() saw
  that dev-type was set and dev-type-pm was set, so it assigned NULL to
  callback.  As a result, nothing happened when rpm_callback(callback, dev)
  was run.
 
 I don't follow.  If that were the reason then no USB device would have 
 been runtime-suspended before the e1620d commit.
 
 Are you saying this actually was true for some period of time (such as 
 between the commit that added the callback variable and the e1620d 
 commit)?

Yes, it was, unfortunately, which is entirely my fault.  Namely,

commit 9659cc0678b954f187290c6e8b247a673c5d37e1
Author: Rafael J. Wysocki r...@sisk.pl
Date:   Fri Feb 18 23:20:21 2011 +0100

PM: Make system-wide PM and runtime PM treat subsystems consistently

changed the ordering in which subsystem callbacks were invoked and that
apparently caused USB autosuspend to stop working.  Commit e1620d591a75a10b15
simply made it work again.  So, the bisection result is a red herring here,
due to my mistake.

Commit 9659cc0678b was introduced between 2.6.38 and 2.6.39-rc1 (i.e. during
the 2.6.39 merge window) and commit e1620d591a75 first appeared in 2.6.39-rc1
too (accortind to git tag --contains), so fortunately there are no released
kernels in which USB autosuspend doesn't work.  But bisection results regarding
USB autosuspend and pointing to between commits 9659cc0678b and e1620d591a75
cannot be trusted.

Sorry for the confusion,
Rafael
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] From 2.6.39-rc1 onward, the Logitech Quickcam Fusion webcam (046d:08c1) stops

2012-07-08 Thread Alan Stern 
On Sun, 8 Jul 2012, Eric Ding wrote:

  I think the reason was the way rpm_suspend() worked at the time of that
  commit (it works a bit differently now, but not as much as to avoid the
  problem).
 
  Namely, before commit e1620d591a75a10b15cf61dbf8243a0b7e6731a2 the device
  had a device type without runtime PM callbacks.  So, rpm_suspend() saw
  that dev-type was set and dev-type-pm was set, so it assigned NULL to
  callback.  As a result, nothing happened when rpm_callback(callback, dev)
  was run.
  
  I don't follow.  If that were the reason then no USB device would have 
  been runtime-suspended before the e1620d commit.
  
  Are you saying this actually was true for some period of time (such as 
  between the commit that added the callback variable and the e1620d 
  commit)?
 
 I think this is, in fact, what happened.  See rpm_suspend() before and
 after commit 9659cc0; I suspect that between commit 9659cc0 and commit
 e1620d5, no USB device was being runtime-suspended.  Since this was
 after v2.6.38 and before v2.6.39-rc1, though, it wouldn't have been
 widely seen or tested, right?

I guess so.  That would explain it.

 However, that doesn't fully explain why the webcam wouldn't have been
 autosuspended in v2.6.38 -- perhaps you all can guess at this more
 quickly than I can?  Was enough changed in the runtime PM architecture
 from v2.6.38 to e1620d to explain this difference?

I don't know...  And at this point I don't care very much about what
was going on in 2.6.38 or before.

In the meantime, can you try out this patch in place of your own?  (I 
haven't even tried to compile it, so there may be one or two small 
errors.)

Alan Stern



Index: usb-3.5/drivers/media/video/uvc/uvc_driver.c
===
--- usb-3.5.orig/drivers/media/video/uvc/uvc_driver.c
+++ usb-3.5/drivers/media/video/uvc/uvc_driver.c
@@ -29,6 +29,7 @@
 #include linux/module.h
 #include linux/slab.h
 #include linux/usb.h
+#include linux/usb/quirks.h
 #include linux/videodev2.h
 #include linux/vmalloc.h
 #include linux/wait.h
@@ -49,6 +50,8 @@ static unsigned int uvc_quirks_param = -
 unsigned int uvc_trace_param;
 unsigned int uvc_timeout_param = UVC_CTRL_STREAMING_TIMEOUT;
 
+#define USB_VENDOR_LOGITECH0x046d
+
 /* 
  * Video formats
  */
@@ -1813,6 +1816,16 @@ static int uvc_probe(struct usb_interfac
uvc_trace(UVC_TRACE_PROBE, Probing generic UVC device %s\n,
udev-devpath);
 
+   /* Many Logitech webcams require the RESET_RESUME quirk */
+   if (le16_to_cpu(udev-descriptor.idVendor) == USB_VENDOR_LOGITECH) {
+   udev-quirks |= USB_QUIRK_RESET_RESUME;
+   dev_dbg(udev-dev, adding reset-resume quirk\n);
+
+   /* Do a device reset, in case the device was just resumed */
+   if (usb_reset_device(udev)  0)
+   return -EIO;
+   }
+
/* Allocate memory for the device and initialize it. */
if ((dev = kzalloc(sizeof *dev, GFP_KERNEL)) == NULL)
return -ENOMEM;
Index: usb-3.5/drivers/usb/core/quirks.c
===
--- usb-3.5.orig/drivers/usb/core/quirks.c
+++ usb-3.5/drivers/usb/core/quirks.c
@@ -38,54 +38,6 @@ static const struct usb_device_id usb_qu
/* Creative SB Audigy 2 NX */
{ USB_DEVICE(0x041e, 0x3020), .driver_info = USB_QUIRK_RESET_RESUME },
 
-   /* Logitech Webcam C200 */
-   { USB_DEVICE(0x046d, 0x0802), .driver_info = USB_QUIRK_RESET_RESUME },
-
-   /* Logitech Webcam C250 */
-   { USB_DEVICE(0x046d, 0x0804), .driver_info = USB_QUIRK_RESET_RESUME },
-
-   /* Logitech Webcam C300 */
-   { USB_DEVICE(0x046d, 0x0805), .driver_info = USB_QUIRK_RESET_RESUME },
-
-   /* Logitech Webcam B/C500 */
-   { USB_DEVICE(0x046d, 0x0807), .driver_info = USB_QUIRK_RESET_RESUME },
-
-   /* Logitech Webcam C600 */
-   { USB_DEVICE(0x046d, 0x0808), .driver_info = USB_QUIRK_RESET_RESUME },
-
-   /* Logitech Webcam Pro 9000 */
-   { USB_DEVICE(0x046d, 0x0809), .driver_info = USB_QUIRK_RESET_RESUME },
-
-   /* Logitech Webcam C905 */
-   { USB_DEVICE(0x046d, 0x080a), .driver_info = USB_QUIRK_RESET_RESUME },
-
-   /* Logitech Webcam C210 */
-   { USB_DEVICE(0x046d, 0x0819), .driver_info = USB_QUIRK_RESET_RESUME },
-
-   /* Logitech Webcam C260 */
-   { USB_DEVICE(0x046d, 0x081a), .driver_info = USB_QUIRK_RESET_RESUME },
-
-   /* Logitech Webcam C310 */
-   { USB_DEVICE(0x046d, 0x081b), .driver_info = USB_QUIRK_RESET_RESUME },
-
-   /* Logitech Webcam C910 */
-   { USB_DEVICE(0x046d, 0x0821), .driver_info = USB_QUIRK_RESET_RESUME },
-
-   /* Logitech Webcam C160 */
-   { USB_DEVICE(0x046d, 0x0824), .driver_info = USB_QUIRK_RESET_RESUME },
-
-   /* Logitech Webcam C270 */
-   {

Re: [PATCH] From 2.6.39-rc1 onward, the Logitech Quickcam Fusion webcam (046d:08c1) stops

2012-07-08 Thread Oleksij Rempel 
On 08.07.2012 17:18, Alan Stern wrote:
 On Sun, 8 Jul 2012, Eric Ding wrote:
 
 I think the reason was the way rpm_suspend() worked at the time of that
 commit (it works a bit differently now, but not as much as to avoid the
 problem).

 Namely, before commit e1620d591a75a10b15cf61dbf8243a0b7e6731a2 the device
 had a device type without runtime PM callbacks.  So, rpm_suspend() saw
 that dev-type was set and dev-type-pm was set, so it assigned NULL to
 callback.  As a result, nothing happened when rpm_callback(callback, dev)
 was run.

 I don't follow.  If that were the reason then no USB device would have 
 been runtime-suspended before the e1620d commit.

 Are you saying this actually was true for some period of time (such as 
 between the commit that added the callback variable and the e1620d 
 commit)?

 I think this is, in fact, what happened.  See rpm_suspend() before and
 after commit 9659cc0; I suspect that between commit 9659cc0 and commit
 e1620d5, no USB device was being runtime-suspended.  Since this was
 after v2.6.38 and before v2.6.39-rc1, though, it wouldn't have been
 widely seen or tested, right?
 
 I guess so.  That would explain it.
 
 However, that doesn't fully explain why the webcam wouldn't have been
 autosuspended in v2.6.38 -- perhaps you all can guess at this more
 quickly than I can?  Was enough changed in the runtime PM architecture
 from v2.6.38 to e1620d to explain this difference?
 
 I don't know...  And at this point I don't care very much about what
 was going on in 2.6.38 or before.
 
 In the meantime, can you try out this patch in place of your own?  (I 
 haven't even tried to compile it, so there may be one or two small 
 errors.)
 
 Alan Stern
 
 
 
 Index: usb-3.5/drivers/media/video/uvc/uvc_driver.c
 ===
 --- usb-3.5.orig/drivers/media/video/uvc/uvc_driver.c
 +++ usb-3.5/drivers/media/video/uvc/uvc_driver.c
 @@ -29,6 +29,7 @@
  #include linux/module.h
  #include linux/slab.h
  #include linux/usb.h
 +#include linux/usb/quirks.h
  #include linux/videodev2.h
  #include linux/vmalloc.h
  #include linux/wait.h
 @@ -49,6 +50,8 @@ static unsigned int uvc_quirks_param = -
  unsigned int uvc_trace_param;
  unsigned int uvc_timeout_param = UVC_CTRL_STREAMING_TIMEOUT;
  
 +#define USB_VENDOR_LOGITECH  0x046d
 +
  /* 
   * Video formats
   */
 @@ -1813,6 +1816,16 @@ static int uvc_probe(struct usb_interfac
   uvc_trace(UVC_TRACE_PROBE, Probing generic UVC device %s\n,
   udev-devpath);
  
 + /* Many Logitech webcams require the RESET_RESUME quirk */
 + if (le16_to_cpu(udev-descriptor.idVendor) == USB_VENDOR_LOGITECH) {
 + udev-quirks |= USB_QUIRK_RESET_RESUME;
 + dev_dbg(udev-dev, adding reset-resume quirk\n);
 +
 + /* Do a device reset, in case the device was just resumed */
 + if (usb_reset_device(udev)  0)
 + return -EIO;
 + }
 +
   /* Allocate memory for the device and initialize it. */
   if ((dev = kzalloc(sizeof *dev, GFP_KERNEL)) == NULL)
   return -ENOMEM;
 Index: usb-3.5/drivers/usb/core/quirks.c
 ===
 --- usb-3.5.orig/drivers/usb/core/quirks.c
 +++ usb-3.5/drivers/usb/core/quirks.c
 @@ -38,54 +38,6 @@ static const struct usb_device_id usb_qu
   /* Creative SB Audigy 2 NX */
   { USB_DEVICE(0x041e, 0x3020), .driver_info = USB_QUIRK_RESET_RESUME },
  
 - /* Logitech Webcam C200 */
 - { USB_DEVICE(0x046d, 0x0802), .driver_info = USB_QUIRK_RESET_RESUME },
 -
 - /* Logitech Webcam C250 */
 - { USB_DEVICE(0x046d, 0x0804), .driver_info = USB_QUIRK_RESET_RESUME },
 -
 - /* Logitech Webcam C300 */
 - { USB_DEVICE(0x046d, 0x0805), .driver_info = USB_QUIRK_RESET_RESUME },
 -
 - /* Logitech Webcam B/C500 */
 - { USB_DEVICE(0x046d, 0x0807), .driver_info = USB_QUIRK_RESET_RESUME },
 -
 - /* Logitech Webcam C600 */
 - { USB_DEVICE(0x046d, 0x0808), .driver_info = USB_QUIRK_RESET_RESUME },
 -
 - /* Logitech Webcam Pro 9000 */
 - { USB_DEVICE(0x046d, 0x0809), .driver_info = USB_QUIRK_RESET_RESUME },
 -
 - /* Logitech Webcam C905 */
 - { USB_DEVICE(0x046d, 0x080a), .driver_info = USB_QUIRK_RESET_RESUME },
 -
 - /* Logitech Webcam C210 */
 - { USB_DEVICE(0x046d, 0x0819), .driver_info = USB_QUIRK_RESET_RESUME },
 -
 - /* Logitech Webcam C260 */
 - { USB_DEVICE(0x046d, 0x081a), .driver_info = USB_QUIRK_RESET_RESUME },
 -
 - /* Logitech Webcam C310 */
 - { USB_DEVICE(0x046d, 0x081b), .driver_info = USB_QUIRK_RESET_RESUME },
 -
 - /* Logitech Webcam C910 */
 - { USB_DEVICE(0x046d, 0x0821), .driver_info = USB_QUIRK_RESET_RESUME },
 -
 - /* Logitech Webcam C160 */
 - { USB_DEVICE(0x046d, 0x0824), .driver_info = USB_QUIRK_RESET_RESUME },
 -
 - /* Logitech

Re: [PATCH] From 2.6.39-rc1 onward, the Logitech Quickcam Fusion webcam (046d:08c1) stops

2012-07-08 Thread Laurent Pinchart 
On Sunday 08 July 2012 17:34:08 Oleksij Rempel wrote:
 On 08.07.2012 17:18, Alan Stern wrote:
  On Sun, 8 Jul 2012, Eric Ding wrote:
  I think the reason was the way rpm_suspend() worked at the time of that
  commit (it works a bit differently now, but not as much as to avoid the
  problem).
  
  Namely, before commit e1620d591a75a10b15cf61dbf8243a0b7e6731a2 the
  device
  had a device type without runtime PM callbacks.  So, rpm_suspend() saw
  that dev-type was set and dev-type-pm was set, so it assigned NULL
  to
  callback.  As a result, nothing happened when rpm_callback(callback,
  dev)
  was run.
  
  I don't follow.  If that were the reason then no USB device would have
  been runtime-suspended before the e1620d commit.
  
  Are you saying this actually was true for some period of time (such as
  between the commit that added the callback variable and the e1620d
  commit)?
  
  I think this is, in fact, what happened.  See rpm_suspend() before and
  after commit 9659cc0; I suspect that between commit 9659cc0 and commit
  e1620d5, no USB device was being runtime-suspended.  Since this was
  after v2.6.38 and before v2.6.39-rc1, though, it wouldn't have been
  widely seen or tested, right?
  
  I guess so.  That would explain it.
  
  However, that doesn't fully explain why the webcam wouldn't have been
  autosuspended in v2.6.38 -- perhaps you all can guess at this more
  quickly than I can?  Was enough changed in the runtime PM architecture
  from v2.6.38 to e1620d to explain this difference?
  
  I don't know...  And at this point I don't care very much about what
  was going on in 2.6.38 or before.
  
  In the meantime, can you try out this patch in place of your own?  (I
  haven't even tried to compile it, so there may be one or two small
  errors.)
  
  Alan Stern
  
  
  
  Index: usb-3.5/drivers/media/video/uvc/uvc_driver.c
  ===
  --- usb-3.5.orig/drivers/media/video/uvc/uvc_driver.c
  +++ usb-3.5/drivers/media/video/uvc/uvc_driver.c
  @@ -29,6 +29,7 @@
  
   #include linux/module.h
   #include linux/slab.h
   #include linux/usb.h
  
  +#include linux/usb/quirks.h
  
   #include linux/videodev2.h
   #include linux/vmalloc.h
   #include linux/wait.h
  
  @@ -49,6 +50,8 @@ static unsigned int uvc_quirks_param = -
  
   unsigned int uvc_trace_param;
   unsigned int uvc_timeout_param = UVC_CTRL_STREAMING_TIMEOUT;
  
  +#define USB_VENDOR_LOGITECH0x046d
  +
  
   /*
   
   
* Video formats
*/
  
  @@ -1813,6 +1816,16 @@ static int uvc_probe(struct usb_interfac
  
  uvc_trace(UVC_TRACE_PROBE, Probing generic UVC device %s\n,
  
  udev-devpath);
  
  +   /* Many Logitech webcams require the RESET_RESUME quirk */
  +   if (le16_to_cpu(udev-descriptor.idVendor) == USB_VENDOR_LOGITECH) {
  +   udev-quirks |= USB_QUIRK_RESET_RESUME;
  +   dev_dbg(udev-dev, adding reset-resume quirk\n);
  +
  +   /* Do a device reset, in case the device was just resumed */
  +   if (usb_reset_device(udev)  0)
  +   return -EIO;
  +   }
  +
  
  /* Allocate memory for the device and initialize it. */
  if ((dev = kzalloc(sizeof *dev, GFP_KERNEL)) == NULL)
  
  return -ENOMEM;
  
  Index: usb-3.5/drivers/usb/core/quirks.c
  ===
  --- usb-3.5.orig/drivers/usb/core/quirks.c
  +++ usb-3.5/drivers/usb/core/quirks.c
  @@ -38,54 +38,6 @@ static const struct usb_device_id usb_qu
  
  /* Creative SB Audigy 2 NX */
  { USB_DEVICE(0x041e, 0x3020), .driver_info = USB_QUIRK_RESET_RESUME 
},
  
  -   /* Logitech Webcam C200 */
  -   { USB_DEVICE(0x046d, 0x0802), .driver_info = USB_QUIRK_RESET_RESUME 
},
  -
  -   /* Logitech Webcam C250 */
  -   { USB_DEVICE(0x046d, 0x0804), .driver_info = USB_QUIRK_RESET_RESUME 
},
  -
  -   /* Logitech Webcam C300 */
  -   { USB_DEVICE(0x046d, 0x0805), .driver_info = USB_QUIRK_RESET_RESUME 
},
  -
  -   /* Logitech Webcam B/C500 */
  -   { USB_DEVICE(0x046d, 0x0807), .driver_info = USB_QUIRK_RESET_RESUME 
},
  -
  -   /* Logitech Webcam C600 */
  -   { USB_DEVICE(0x046d, 0x0808), .driver_info = USB_QUIRK_RESET_RESUME 
},
  -
  -   /* Logitech Webcam Pro 9000 */
  -   { USB_DEVICE(0x046d, 0x0809), .driver_info = USB_QUIRK_RESET_RESUME 
},
  -
  -   /* Logitech Webcam C905 */
  -   { USB_DEVICE(0x046d, 0x080a), .driver_info = USB_QUIRK_RESET_RESUME 
},
  -
  -   /* Logitech Webcam C210 */
  -   { USB_DEVICE(0x046d, 0x0819), .driver_info = USB_QUIRK_RESET_RESUME 
},
  -
  -   /* Logitech Webcam C260 */
  -   { USB_DEVICE(0x046d, 0x081a), .driver_info = USB_QUIRK_RESET_RESUME 
},
  -
  -   /* Logitech Webcam C310 */
  -   { USB_DEVICE(0x046d, 0x081b), .driver_info = USB_QUIRK_RESET_RESUME 
},
  -
  -   /* Logitech Webcam C910 */
  -   { USB_DEVICE(0x046d,

Re: [PATCH] From 2.6.39-rc1 onward, the Logitech Quickcam Fusion webcam (046d:08c1) stops

2012-07-08 Thread Laurent Pinchart 
Hi Alan,

On Friday 06 July 2012 11:47:01 Alan Stern wrote:
 On Fri, 6 Jul 2012, Alan Cox wrote:
   Yes, but we still need to know the reason why.  Neither Rafael nor I
   has been able to figure out why that commit messed things up.
  
  Was the driver doing any dynamic autosuspend at all until that point ?
 
 I don't know...  but whatever it was doing before the commit, it should
 be doing the same thing afterward.
 
  From the bug report it goes castors up in windows too if a suspend/resume
  occurs.
 
 Yes, it's definitely a bug in the webcam.
 
 On the other hand, there is a similar bug report for a Logitech C525
 webcam which works properly when suspended/resumed by an xHCI
 controller but crashes when suspended/resumed by an EHCI controller.  I
 have no idea why.
 
 Laurent, you might want to take a look at that thread, in particular,
 at this message:
 
   http://marc.info/?l=linux-usbm=134115837927619w=2
 
 Can you explain the difference between the USB-2 and USB-3 usbmon
 traces?

(quoting your e-mail from the above link)

 There are two significant differences between the USB-2 and USB-3 
 traces, although I don't know what to make of them.
 
 Initially it looks like the driver writes a bunch of values to the 
 webcam and asks it to repeat them back.  That's what happens in the 
 USB-3 trace, anyway.  The USB-2 trace shows the values being written, 
 but when asked to repeat them back the webcam sends nothing.
 
 For example, with USB-3:
 
 8804085042c0 470397794 S Co:3:003:0 s 22 01 0100 0086 0003 3 = 803e00
 8804085042c0 470399896 C Co:3:003:0 0 3 
 8804085042c0 47044 S Ci:3:003:0 s a2 81 0100 0086 0003 3 
 8804085042c0 470401270 C Ci:3:003:0 0 3 = 803e00
 
 The 803e00 values get sent and then received.  The analogous part of
 the USB-2 trace shows:
 
 88040cc1f5c0 208122798 S Co:1:006:0 s 22 01 0100 0086 0003 3 = 803e00
 88040cc1f5c0 208124781 C Co:1:006:0 0 3 
 88040cc1f5c0 208124818 S Ci:1:006:0 s a2 81 0100 0086 0003 3 
 88040cc1f5c0 208126156 C Ci:1:006:0 0 0
 
 Exactly the same values are sent and the request is the same, but no 
 data gets returned.  I have no idea what that means.

Neither do I, because those requests are sent to an audio endpoint. They're 
not related to UVC. It might be helpful to retest this without using audio.

 Secondly, both traces show that the webcam stopped being used for a
 short time and was suspended.  A few seconds later, when it was resumed
 again, it worked okay in the USB-3 trace.  But in the USB-2 trace, it
 didn't.  After the resume it failed to reply to the first packet sent
 (a Get-Device-Status request), and thus it had to be re-enumerated.

USB2 and USB3 timings are probably slightly different, and Logitech devices 
have been known to suffer from race conditions in the past. Some would crash 
pretty much immediately in Linux depending on the USB controller used and the 
overall system speed, while they worked fine on Windows. It turned out the 
firmware contained something like

events = read(IRQ_EVENTS register);
write(IRQ_EVENTS registers, 0x);

I'm unfortunately not surprised by device-side bugs anymore.

-- 
Regards,

Laurent Pinchart

--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] From 2.6.39-rc1 onward, the Logitech Quickcam Fusion webcam (046d:08c1) stops

2012-07-08 Thread Eric Ding 
On 07/09/2012 08:38 AM, Alan Stern wrote:
 On Sun, 8 Jul 2012, Laurent Pinchart wrote:
 
 this quirks also affect snd_usb_audio module. If for some reasons
 uvcvideo is not loaded, snd_usb_audio will fail - i mean haw mysterious
 sound problems.

 Good point. If we load the uvcvideo driver while the audio function is in 
 use, 
 I doubt resetting the device will lead to a good user experience. One could 
 argue whether this should happen in the first place though, as modules 
 should 
 be auto-loaded. Could there be a race between audio and video probe, leading 
 to audio probe failure because we reset the device in the middle of the 
 probe 
 sequence ?
 
 Even if there's no race and uvcvideo never gets loaded, there still
 could be a problem.  The device could get suspended while waiting for
 the snd-usb-audio module to load.  Then when the audio driver is
 probed, it could be looking at a webcam that has already crashed
 because the RESET_RESUME quirk was never set.
 
 An alternative would be to modify the usb_detect_quirks() routine 
 directly -- have it set RESET_RESUME whenever it sees a Logitech 
 webcam.  The difficulty there is how to recognize which devices are 
 webcams.  Look for a video interface?
 
 Any suggestions?

I may be misreading the code, but it looks like uvc_driver.c just looks
for the USB_CLASS_VIDEO interface class.  In saying this, though, it
appears that several Logitech (and other) webcams don't actually have
their interface class set to VENDOR_SPEC instead because they don't
announce themselves as UVC devices.

So it looks like you'd have to both look for USB_CLASS_VIDEO and check
uvc_ids[] too... which becomes somewhat hairy, since I assume you don't
realy want usb_detect_quirks() to reference UVC-specific structs...
which brings us back to the original laundry list approach of naming
several affected webcams explicitly, no?

Eric

--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] From 2.6.39-rc1 onward, the Logitech Quickcam Fusion webcam (046d:08c1) stops

2012-07-07 Thread Oleksij Rempel 
On 07.07.2012 11:20, Eric Ding wrote:
 On 07/06/2012 11:47 PM, Alan Stern wrote:
 On Fri, 6 Jul 2012, Alan Cox wrote:

 Yes, but we still need to know the reason why.  Neither Rafael nor I 
 has been able to figure out why that commit messed things up.

 Was the driver doing any dynamic autosuspend at all until that point ?

 I don't know...  but whatever it was doing before the commit, it should 
 be doing the same thing afterward.
 
 I'm really out of my league here; I only submitted a bug report because
 my webcam stopped working, never imagining it'd lead to 4-5 days of
 kernel bisection (which I've never done before) and/or submitting a
 kernel patch.  Still, though, if I can be of any further help in getting
 to the bottom of this, I'd like to try.
 
 I thought that getting some comparative usbmon logs might help, but I'm
 not equipped to parse them.  To generate these logs, I ran guvcview once
 after plugging in the webcam, then did the following upon quiting
 guvcview: start listening via usbmon, sleep for four seconds, start
 guvcview again and stop listening via usbmon after four seconds.
 
 I did this both at commit e1620d5 (the commit which triggered this
 issue) and once at commit 9975961 (one commit prior).  Going through
 these motions also confirmed that the problem is readily reproducible
 the second time I start guvcview at e1620d5, but not at 9975961.  I've
 attached both logs in case they're helpful.
 
 Please let me know if there's anything more I can do to help.
 
 Thanks,
 Eric
 

In both logs you send the cam start to stream video. Major difference is
that, after suspend/resume it need more time (about 10 seconds) to start
stream. Is it correct?

-- 
Regards,
Oleksij


--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] From 2.6.39-rc1 onward, the Logitech Quickcam Fusion webcam (046d:08c1) stops

2012-07-07 Thread Eric Ding 
On 07/07/2012 06:09 PM, Oleksij Rempel wrote:
 On 07.07.2012 11:20, Eric Ding wrote:
 On 07/06/2012 11:47 PM, Alan Stern wrote:
 On Fri, 6 Jul 2012, Alan Cox wrote:

 Yes, but we still need to know the reason why.  Neither Rafael nor I 
 has been able to figure out why that commit messed things up.

 Was the driver doing any dynamic autosuspend at all until that point ?

 I don't know...  but whatever it was doing before the commit, it should 
 be doing the same thing afterward.

 I thought that getting some comparative usbmon logs might help, but I'm
 not equipped to parse them.  To generate these logs, I ran guvcview once
 after plugging in the webcam, then did the following upon quiting
 guvcview: start listening via usbmon, sleep for four seconds, start
 guvcview again and stop listening via usbmon after four seconds.

 I did this both at commit e1620d5 (the commit which triggered this
 issue) and once at commit 9975961 (one commit prior).  Going through
 these motions also confirmed that the problem is readily reproducible
 the second time I start guvcview at e1620d5, but not at 9975961.  I've
 attached both logs in case they're helpful.

 In both logs you send the cam start to stream video. Major difference is
 that, after suspend/resume it need more time (about 10 seconds) to start
 stream. Is it correct?

My tests didn't continue a full 10 seconds after I restarted the webcam,
so I'm not sure I follow your question.  In the second case (with
e1620d5), the camera never returns a normal video image -- instead, it
gets a garbled video image of horizontal stripes, which change with what
the camera lens is seeing, but are obviously not the correct image.

The stream does take longer to come up on the second time than the
first, though, during which I see the following error message in
guvcview 1-3 times:

 Could not grab image (select timeout): Resource temporarily unavailable

Sorry if I'm not answering your question helpfully!

Eric
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] From 2.6.39-rc1 onward, the Logitech Quickcam Fusion webcam (046d:08c1) stops

2012-07-07 Thread Oleksij Rempel 
On 07.07.2012 13:38, Eric Ding wrote:
 On 07/07/2012 06:09 PM, Oleksij Rempel wrote:
 On 07.07.2012 11:20, Eric Ding wrote:
 On 07/06/2012 11:47 PM, Alan Stern wrote:
 On Fri, 6 Jul 2012, Alan Cox wrote:

 Yes, but we still need to know the reason why.  Neither Rafael nor I 
 has been able to figure out why that commit messed things up.

 Was the driver doing any dynamic autosuspend at all until that point ?

 I don't know...  but whatever it was doing before the commit, it should 
 be doing the same thing afterward.

 I thought that getting some comparative usbmon logs might help, but I'm
 not equipped to parse them.  To generate these logs, I ran guvcview once
 after plugging in the webcam, then did the following upon quiting
 guvcview: start listening via usbmon, sleep for four seconds, start
 guvcview again and stop listening via usbmon after four seconds.

 I did this both at commit e1620d5 (the commit which triggered this
 issue) and once at commit 9975961 (one commit prior).  Going through
 these motions also confirmed that the problem is readily reproducible
 the second time I start guvcview at e1620d5, but not at 9975961.  I've
 attached both logs in case they're helpful.

 In both logs you send the cam start to stream video. Major difference is
 that, after suspend/resume it need more time (about 10 seconds) to start
 stream. Is it correct?
 
 My tests didn't continue a full 10 seconds after I restarted the webcam,
 so I'm not sure I follow your question.  In the second case (with
 e1620d5), the camera never returns a normal video image -- instead, it
 gets a garbled video image of horizontal stripes, which change with what
 the camera lens is seeing, but are obviously not the correct image.
 
 The stream does take longer to come up on the second time than the
 first, though, during which I see the following error message in
 guvcview 1-3 times:
 
  Could not grab image (select timeout): Resource temporarily unavailable
 
 Sorry if I'm not answering your question helpfully!
 
 Eric
 

Ok,  i guess i know your problem but i doubt it will be completely fixed
by changing powermanagement behavior. Two logitech cams i tested is
really easy to confuse/brake/freeze. Just turn off the stream before it
will send first frame. It looks like it take longer to boot buildin
controller or image processor. On second (hot) start it take less time.
If the cam was suspended it need this boot time again. The problem, even
if we do hot start and turn of stream before first frame, like some apps
did, the cam will brake again. By disabling auto suspend, we will reduce
probability to brake it, but not fix it.

Some times i have the splitimage issue too, but i can't 100% reproduce it.
-- 
Regards,
Oleksij


--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] From 2.6.39-rc1 onward, the Logitech Quickcam Fusion webcam (046d:08c1) stops

2012-07-07 Thread Oleksij Rempel 
On 07.07.2012 15:55, Eric Ding wrote:
 On 07/07/2012 09:11 PM, Oleksij Rempel wrote:
 On 07.07.2012 13:38, Eric Ding wrote:
 On 07/07/2012 06:09 PM, Oleksij Rempel wrote:
 On 07.07.2012 11:20, Eric Ding wrote:
 On 07/06/2012 11:47 PM, Alan Stern wrote:
 On Fri, 6 Jul 2012, Alan Cox wrote:

 Yes, but we still need to know the reason why.  Neither Rafael nor I 
 has been able to figure out why that commit messed things up.

 Was the driver doing any dynamic autosuspend at all until that point ?

 I don't know...  but whatever it was doing before the commit, it should 
 be doing the same thing afterward.

 I thought that getting some comparative usbmon logs might help, but I'm
 not equipped to parse them.  To generate these logs, I ran guvcview once
 after plugging in the webcam, then did the following upon quiting
 guvcview: start listening via usbmon, sleep for four seconds, start
 guvcview again and stop listening via usbmon after four seconds.

 I did this both at commit e1620d5 (the commit which triggered this
 issue) and once at commit 9975961 (one commit prior).  Going through
 these motions also confirmed that the problem is readily reproducible
 the second time I start guvcview at e1620d5, but not at 9975961.  I've
 attached both logs in case they're helpful.

 In both logs you send the cam start to stream video. Major difference is
 that, after suspend/resume it need more time (about 10 seconds) to start
 stream. Is it correct?

 My tests didn't continue a full 10 seconds after I restarted the webcam,
 so I'm not sure I follow your question.  In the second case (with
 e1620d5), the camera never returns a normal video image -- instead, it
 gets a garbled video image of horizontal stripes, which change with what
 the camera lens is seeing, but are obviously not the correct image.

 Ok,  i guess i know your problem but i doubt it will be completely fixed
 by changing powermanagement behavior. Two logitech cams i tested is
 really easy to confuse/brake/freeze. Just turn off the stream before it
 will send first frame. It looks like it take longer to boot buildin
 controller or image processor. On second (hot) start it take less time.
 If the cam was suspended it need this boot time again. The problem, even
 if we do hot start and turn of stream before first frame, like some apps
 did, the cam will brake again. By disabling auto suspend, we will reduce
 probability to brake it, but not fix it.

 Some times i have the splitimage issue too, but i can't 100% reproduce it.
 
 So does your description of the problem explain why commit e1620d5
 causes this problem to happen 100% of the time on the second start, even
 though I cannot reproduce this problem at all before that commit?

Please note, i said i guess, i'm not sure if it is same issue. With
3.5.0-rc5-00098-g9e85a6f i can't reproduce this issue at least after
fresh start. After long work some times i get some thing similar to your
description.

 Isn't
 that the mystery that still remains unsolved?  Do the usbmon logs not
 answer that question?

no. Same configuration sequence, same responses. I see only one
difference, on e1620d5 the cam needed longer to start streaming.  This
is know on other cams, but they work.

 Going back also to the patch I submitted, it seems (at least in my
 testing) that USB_QUIRK_RESET_RESUME does work around this issue
 consistently, at least for my webcam.

ok. the problem is, e1620d5 moves existing CONFIG_USB_SUSPEND from one
place to another. uvcvideo used autosuspend before. This is why this
quirk make no sense.

-- 
Regards,
Oleksij


--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] From 2.6.39-rc1 onward, the Logitech Quickcam Fusion webcam (046d:08c1) stops

2012-07-07 Thread Rafael J. Wysocki 
On Saturday, July 07, 2012, Alan Stern wrote:
 On Sat, 7 Jul 2012, Oleksij Rempel wrote:
 
   Ok,  i guess i know your problem but i doubt it will be completely fixed
   by changing powermanagement behavior. Two logitech cams i tested is
   really easy to confuse/brake/freeze. Just turn off the stream before it
   will send first frame. It looks like it take longer to boot buildin
   controller or image processor. On second (hot) start it take less time.
   If the cam was suspended it need this boot time again. The problem, even
   if we do hot start and turn of stream before first frame, like some apps
   did, the cam will brake again. By disabling auto suspend, we will reduce
   probability to brake it, but not fix it.
 
 I don't think that was the problem.  As far as I can tell, the problem 
 with these Logitech webcams is that they often fail to resume correctly 
 following a suspend.  They require a reset before they will start 
 working again.
 
 Apparently the behavior before commit e1620d5 was that the webcam
 didn't get suspended, and after the commit it did.  Unfortunately
 the usbmon traces do not explain this difference; all they show is
 when/whether a suspend took place.
 
 For example, the prelog.9975961.out trace shows that the webcam wasn't 
 suspended before the trace began, and the postlog.e1620d5.out trace 
 shows that it was (although when the webcam was resumed, it did work 
 okay -- this was one of the times it didn't crash during the resume).
 
   Some times i have the splitimage issue too, but i can't 100% reproduce 
   it.
   
   So does your description of the problem explain why commit e1620d5
   causes this problem to happen 100% of the time on the second start, even
   though I cannot reproduce this problem at all before that commit?
  
  Please note, i said i guess, i'm not sure if it is same issue. With
  3.5.0-rc5-00098-g9e85a6f i can't reproduce this issue at least after
  fresh start. After long work some times i get some thing similar to your
  description.
  
   Isn't
   that the mystery that still remains unsolved?  Do the usbmon logs not
   answer that question?
  
  no. Same configuration sequence, same responses. I see only one
  difference, on e1620d5 the cam needed longer to start streaming.  This
  is know on other cams, but they work.
  
   Going back also to the patch I submitted, it seems (at least in my
   testing) that USB_QUIRK_RESET_RESUME does work around this issue
   consistently, at least for my webcam.
 
 The quirk tells Linux to reset the webcam whenever it is resumed.
 
  ok. the problem is, e1620d5 moves existing CONFIG_USB_SUSPEND from one
  place to another. uvcvideo used autosuspend before. This is why this
  quirk make no sense.
 
 Well, the quirk does make sense.  What doesn't make sense is why moving 
 the runtime PM operation pointers from usb_bus_type to usb_device_type
 should cause any change in the autosuspend behavior.  That's what we 
 would like to know.

I think the reason was the way rpm_suspend() worked at the time of that
commit (it works a bit differently now, but not as much as to avoid the
problem).

Namely, before commit e1620d591a75a10b15cf61dbf8243a0b7e6731a2 the device
had a device type without runtime PM callbacks.  So, rpm_suspend() saw
that dev-type was set and dev-type-pm was set, so it assigned NULL to
callback.  As a result, nothing happened when rpm_callback(callback, dev)
was run.

After the change, though, the runtime callbacks are present in
usb_device_pm_ops, so the same code assigns the address of
usb_runtime_suspend() to callback and the device is actually suspended
by rpm_callback(callback, dev).

Thanks,
Rafael
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] From 2.6.39-rc1 onward, the Logitech Quickcam Fusion webcam (046d:08c1) stops

2012-07-07 Thread Alan Stern 
On Sat, 7 Jul 2012, Rafael J. Wysocki wrote:

  Well, the quirk does make sense.  What doesn't make sense is why moving 
  the runtime PM operation pointers from usb_bus_type to usb_device_type
  should cause any change in the autosuspend behavior.  That's what we 
  would like to know.
 
 I think the reason was the way rpm_suspend() worked at the time of that
 commit (it works a bit differently now, but not as much as to avoid the
 problem).
 
 Namely, before commit e1620d591a75a10b15cf61dbf8243a0b7e6731a2 the device
 had a device type without runtime PM callbacks.  So, rpm_suspend() saw
 that dev-type was set and dev-type-pm was set, so it assigned NULL to
 callback.  As a result, nothing happened when rpm_callback(callback, dev)
 was run.

I don't follow.  If that were the reason then no USB device would have 
been runtime-suspended before the e1620d commit.

Are you saying this actually was true for some period of time (such as 
between the commit that added the callback variable and the e1620d 
commit)?

Alan Stern

--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] From 2.6.39-rc1 onward, the Logitech Quickcam Fusion webcam (046d:08c1) stops

2012-07-07 Thread Eric Ding 
On 07/08/2012 09:52 AM, Alan Stern wrote:
 On Sat, 7 Jul 2012, Rafael J. Wysocki wrote:
 
 Well, the quirk does make sense.  What doesn't make sense is why moving 
 the runtime PM operation pointers from usb_bus_type to usb_device_type
 should cause any change in the autosuspend behavior.  That's what we 
 would like to know.

 I think the reason was the way rpm_suspend() worked at the time of that
 commit (it works a bit differently now, but not as much as to avoid the
 problem).

 Namely, before commit e1620d591a75a10b15cf61dbf8243a0b7e6731a2 the device
 had a device type without runtime PM callbacks.  So, rpm_suspend() saw
 that dev-type was set and dev-type-pm was set, so it assigned NULL to
 callback.  As a result, nothing happened when rpm_callback(callback, dev)
 was run.
 
 I don't follow.  If that were the reason then no USB device would have 
 been runtime-suspended before the e1620d commit.
 
 Are you saying this actually was true for some period of time (such as 
 between the commit that added the callback variable and the e1620d 
 commit)?

I think this is, in fact, what happened.  See rpm_suspend() before and
after commit 9659cc0; I suspect that between commit 9659cc0 and commit
e1620d5, no USB device was being runtime-suspended.  Since this was
after v2.6.38 and before v2.6.39-rc1, though, it wouldn't have been
widely seen or tested, right?

However, that doesn't fully explain why the webcam wouldn't have been
autosuspended in v2.6.38 -- perhaps you all can guess at this more
quickly than I can?  Was enough changed in the runtime PM architecture
from v2.6.38 to e1620d to explain this difference?

Eric
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] From 2.6.39-rc1 onward, the Logitech Quickcam Fusion webcam (046d:08c1) stops

2012-07-07 Thread Eric Ding 
On 07/08/2012 03:18 AM, Alan Stern wrote:

 Apparently the behavior before commit e1620d5 was that the webcam
 didn't get suspended, and after the commit it did.  Unfortunately
 the usbmon traces do not explain this difference; all they show is
 when/whether a suspend took place.
 
 For example, the prelog.9975961.out trace shows that the webcam wasn't 
 suspended before the trace began, and the postlog.e1620d5.out trace 
 shows that it was (although when the webcam was resumed, it did work 
 okay -- this was one of the times it didn't crash during the resume).

Let me clarify: the webcam did *not* work okay during the session
represented by postlog.e1620d5.out; it turned back on, but only returned
a garbled video image.  So even here, it really needs the reset-resume
quirk.

 Eric, are your kernel-hacking skills up to debugging this?  I can tell 
 you where to look and what to look for.

I'm not a kernel hacker -- almost all my development/coding experience
is at the application/UI level.  But I can read and modify C, and I can
compile a kernel.  :-)  So if you don't mind a little hand-holding, I am
happy to help however I can.

Eric
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] From 2.6.39-rc1 onward, the Logitech Quickcam Fusion webcam (046d:08c1) stops

2012-07-06 Thread Alan Cox 
On Fri, 6 Jul 2012 10:37:30 -0400 (EDT)
Alan Stern st...@rowland.harvard.edu wrote:

 On Fri, 6 Jul 2012, Laurent Pinchart wrote:
 
  Hi Alan,
  
  On Friday 06 July 2012 09:50:30 Alan Stern wrote:
   On Fri, 6 Jul 2012, Alan Cox wrote:
From: Eric Ding ericd...@alum.mit.edu

working if I turn it on (e.g., guvcview), then shut it off, wait a few
seconds, and then turn it on again.  From the second time onward, all I
get is screen noise rather than the expected video images.

I did a kernel bisection, which revealed the commit that caused the
regression:

commit e1620d591a75a10b15cf61dbf8243a0b7e6731a2
Author: Rafael J. Wysocki r...@sisk.pl
Date: Fri Mar 18 19:55:36 2011 +0100

USB: Move runtime PM callbacks to usb_device_pm_ops
   
   Can you figure out exactly how that commit caused the problem?  As far
   as I can tell, it should not have made any difference to anything.
  
  That patch has already been pointed out as problematic:
  
  http://www.mail-archive.com/linux-media@vger.kernel.org/msg48624.html
  
  (CC'ing the relevant people)
 
 Yes, but we still need to know the reason why.  Neither Rafael nor I 
 has been able to figure out why that commit messed things up.

Was the driver doing any dynamic autosuspend at all until that point ?

From the bug report it goes castors up in windows too if a suspend/resume
occurs.

Alan
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] From 2.6.39-rc1 onward, the Logitech Quickcam Fusion webcam (046d:08c1) stops

2012-07-06 Thread Alan Stern 
On Fri, 6 Jul 2012, Alan Cox wrote:

  Yes, but we still need to know the reason why.  Neither Rafael nor I 
  has been able to figure out why that commit messed things up.
 
 Was the driver doing any dynamic autosuspend at all until that point ?

I don't know...  but whatever it was doing before the commit, it should 
be doing the same thing afterward.

 From the bug report it goes castors up in windows too if a suspend/resume
 occurs.

Yes, it's definitely a bug in the webcam.

On the other hand, there is a similar bug report for a Logitech C525 
webcam which works properly when suspended/resumed by an xHCI 
controller but crashes when suspended/resumed by an EHCI controller.  I 
have no idea why.

Laurent, you might want to take a look at that thread, in particular, 
at this message:

http://marc.info/?l=linux-usbm=134115837927619w=2

Can you explain the difference between the USB-2 and USB-3 usbmon 
traces?

Alan Stern

--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html