Hi Alan,
On Sunday 08 July 2012 11:18:35 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
The patch looks good (although I'd replace dev_dbg with uvc_trace). It
compiles but I haven't tested it yet, I have no Logitech webcam with me right
now.
> 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 Webcam C270 */
> - { USB_DEVICE(0x046d, 0x0825), .driver_info = USB_QUIRK_RESET_RESUME },
> -
> - /* Logitech Quickcam Pro 9000 */
> - { USB_DEVICE(0x046d, 0x0990), .driver_info = USB_QUIRK_RESET_RESUME },
> -
> - /* Logitech Quickcam E3500 */
> - { USB_DEVICE(0x046d, 0x09a4), .driver_info = USB_QUIRK_RESET_RESUME },
> -
> - /* Logitech Quickcam Vision Pro */
> - { USB_DEVICE(0x046d, 0x09a6), .driver_info = USB_QUIRK_RESET_RESUME },
> -
> /* Logitech Harmony 700-series */
> { USB_DEVICE(0x046d, 0xc122), .driver_info = USB_QUIRK_DELAY_INIT },
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
