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_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 },
>
> 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 ?
--
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
