Hi Matthew,
A few months ago, you posted a patch that added a function,
usb_device_autosuspend_enable(), to the usb core code, and then modified
the uvc driver to call this at probe time.
I've been using this patch for a while on a platform that a few of us
are trying to optimize the power usage on. It was thought that this was
needed in order to put the uvc video driver to sleep while it was not
being used, which is the majority of the time. This was the equalivant
of echoing "auto" to the power/level file in sysfs.
Well, when someone actually got around to measuring the power usage, it
turned out that the kernel patch was not equalvant to echoing the value
in sysfs. I tracked this down to the fact that your function only did
the following:
void usb_device_autosuspend_enable(struct usb_device *udev)
{
udev->autosuspend_disabled = 0;
udev->autoresume_disabled = 0;
}
The usb or power core was never told that the device needed to go into
"sleep". So I tried to fix this by doing adding one line to the
function:
void usb_device_autosuspend_enable(struct usb_device *udev)
{
udev->autosuspend_disabled = 0;
udev->autoresume_disabled = 0;
usb_external_suspend_device(udev, PMSG_USER_SUSPEND);
}
But, again, in testing, this didn't seem to actually make any
difference. Well, it did for about 50% of the time the machine was
tested, but not the other 50%. The other 50% always worked when echoing
"auto" to the power/level file.
So, in summary:
- echoing "auto" to the sysfs file always seemed to work
- the above change to your function seemed to work 50% of the
time.
Any ideas?
Your patch (modified), and the uvc, and one for cdc_acm is attached
below, and is what me and Joey (on the CC:) are trying to get working
here.
Yes, I think the real solution is to provide "real" autosuspend support
for the cdc_acm and uvc drivers, right? But I originally approached
this as a, "do this quick hack for now" type fix, which seems to have
taken probably much longer than just adding autosuspend support to the
drivers :(
thanks,
greg k-h
From: Matthew Garrett <mjg@xxxxxxxxxx>
Date: Wed Jul 8 19:04:23 2009 +0100
Subject: usb: Allow drivers to enable USB autosuspend on a per-device basis
Patch-mainline: 2.6.33 possibly
usb: Allow drivers to enable USB autosuspend on a per-device basis
USB autosuspend is currently only enabled by default for hubs. On other
hardware the decision is made by userspace. This is unnecessary in cases
where we know that the hardware supports autosuspend, so this patch adds
a function to allow drivers to enable it at probe time.
Signed-off-by: Matthew Garrett <mjg@xxxxxxxxxx>
Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxx>
---
drivers/usb/core/driver.c | 16 ++++++++++++++++
include/linux/usb.h | 4 ++++
2 files changed, 20 insertions(+)
--- a/drivers/usb/core/driver.c
+++ b/drivers/usb/core/driver.c
@@ -1572,6 +1572,22 @@ void usb_autopm_put_interface_async(stru
EXPORT_SYMBOL_GPL(usb_autopm_put_interface_async);
/**
+ * usb_device_autosuspend_enable - enable autosuspend on a device
+ * @udev: the usb_device to be autosuspended
+ *
+ * This routine should be called by an interface driver when it knows that
+ * the device in question supports USB autosuspend.
+ *
+ */
+void usb_device_autosuspend_enable(struct usb_device *udev)
+{
+ udev->autosuspend_disabled = 0;
+ udev->autoresume_disabled = 0;
+ usb_external_suspend_device(udev, PMSG_USER_SUSPEND);
+}
+EXPORT_SYMBOL_GPL(usb_device_autosuspend_enable);
+
+/**
* usb_autopm_get_interface - increment a USB interface's PM-usage counter
* @intf: the usb_interface whose counter should be incremented
*
--- a/include/linux/usb.h
+++ b/include/linux/usb.h
@@ -524,6 +524,7 @@ extern struct usb_device *usb_find_devic
/* USB autosuspend and autoresume */
#ifdef CONFIG_USB_SUSPEND
+extern void usb_device_autosuspend_enable(struct usb_device *udev);
extern int usb_autopm_set_interface(struct usb_interface *intf);
extern int usb_autopm_get_interface(struct usb_interface *intf);
extern void usb_autopm_put_interface(struct usb_interface *intf);
@@ -549,6 +550,9 @@ static inline void usb_mark_last_busy(st
#else
+static inline void usb_device_autosuspend_enable(struct usb_device *udev)
+{ }
+
static inline int usb_autopm_set_interface(struct usb_interface *intf)
{ return 0; }
From: Matthew Garrett <mjg@xxxxxxxxxx>
Date: Sun Jul 19 02:24:49 2009 +0100
Subject: USB: enable autosuspend for uvc by default
Enable autosuspend on UVC by default
Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxx>
--- a/drivers/media/video/uvc/uvc_driver.c
+++ b/drivers/media/video/uvc/uvc_driver.c
@@ -1647,6 +1647,8 @@ static int uvc_probe(struct usb_interface *intf,
"supported.\n", ret);
}
+ usb_device_autosuspend_enable(udev);
+
uvc_trace(UVC_TRACE_PROBE, "UVC device initialized.\n");
return 0;
From: Greg Kroah-Hartman <gregkh@xxxxxxx>
Subject: USB: enable autosuspend for cdc-acm by default
Enable autosuspend on cdc-acm by default
Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxx>
---
drivers/usb/class/cdc-acm.c | 2 ++
1 file changed, 2 insertions(+)
--- a/drivers/usb/class/cdc-acm.c
+++ b/drivers/usb/class/cdc-acm.c
@@ -1183,6 +1183,8 @@ skip_countries:
usb_get_intf(control_interface);
tty_register_device(acm_tty_driver, minor, &control_interface->dev);
+ usb_device_autosuspend_enable(usb_dev);
+
acm_table[minor] = acm;
return 0;
