Hi Brandon,
On Thursday 13 December 2007, Brandon Philips wrote:
> This patch enables usb power management in the UVC driver. As cameras
> are built into more and more modern laptops it is important that we get
> power management for them working.
Thanks for the patch. Comments inlined.
> How-to Test
> -----------
> 0. Apply and recompile with CONFIG_USB_DEBUG
> 1. Plug in the camera
> 2. rmmod usb-snd-audio # no pm in usb-snd-audio yet and many cameras
> have a mic
> 3. Locate the device with lsusb
> 4. Go to its sysfs directory
> 5. echo "auto" > power/level
> 6. wait for messages about the device and its root hub being suspended
> in syslog. It'll take a few seconds
>
> Signed-off-by: Oliver Neukum <[EMAIL PROTECTED]>
> Reviewed-by: Brandon Philips <[EMAIL PROTECTED]>
>
> ---
> uvc_driver.c | 7 ++++++-
> uvc_v4l2.c | 6 ++++++
> 2 files changed, 12 insertions(+), 1 deletion(-)
>
> Index: uvc/uvc_v4l2.c
> ===================================================================
> --- uvc.orig/uvc_v4l2.c
> +++ uvc/uvc_v4l2.c
> @@ -413,10 +413,15 @@ static int uvc_v4l2_open(struct inode *i
> goto done;
> }
>
> + ret = usb_autopm_get_interface(video->dev->intf);
> + if (ret < 0)
> + goto done;
> +
> /* Create the device handle. */
> handle = kzalloc(sizeof *handle, GFP_KERNEL);
> if (handle == NULL) {
> ret = -ENOMEM;
> + usb_autopm_put_interface(video->dev->intf);
> goto done;
> }
>
> @@ -455,6 +460,7 @@ static int uvc_v4l2_release(struct inode
> kfree(handle);
> file->private_data = NULL;
>
> + usb_autopm_put_interface(video->dev->intf);
> kref_put(&video->dev->kref, uvc_delete);
> return 0;
> }
Is autosuspend interface-based or device-based ? If I understand things
correctly, a device won't be autosuspended as long as at least one of its
interfaces have a PM counter greater than 0. In that case incrementing the PM
counter for the control interface should be enough, there's no need to
increment the streaming interface PM counter. Am I right ?
> Index: uvc/uvc_driver.c
> ===================================================================
> --- uvc.orig/uvc_driver.c
> +++ uvc/uvc_driver.c
> @@ -678,8 +678,10 @@ static int uvc_parse_streaming(struct uv
> format->frame = frame;
> ret = uvc_parse_format(dev, streaming, format,
> &interval, buffer, buflen);
> - if (ret < 0)
> + if (ret < 0) {
> + kfree(format);
> return ret;
> + }
Are you sure about this ? format is freed in uvc_delete().
> frame += format->nframes;
> format++;
> @@ -1605,6 +1607,8 @@ static int uvc_suspend(struct usb_interf
> uvc_trace(UVC_TRACE_SUSPEND, "Suspending interface %u\n",
> intf->cur_altsetting->desc.bInterfaceNumber);
>
> + usb_kill_urb(dev->int_urb);
> +
The URB should be resubmitted on resume. I'll take care of this.
> /* Controls are cached on the fly so they don't need to be saved. */
> if (intf->cur_altsetting->desc.bInterfaceSubClass == SC_VIDEOCONTROL)
> return 0;
> @@ -1809,6 +1813,7 @@ struct uvc_driver uvc_driver = {
> .suspend = uvc_suspend,
> .resume = uvc_resume,
> .id_table = uvc_ids,
> + .supports_autosuspend = 1,
> },
> };
What about devices that don't implement suspend properly ? Do I need some kind
of quirk for them ?
Best regards,
Laurent Pinchart
_______________________________________________
Linux-uvc-devel mailing list
[email protected]
https://lists.berlios.de/mailman/listinfo/linux-uvc-devel
