Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1661722 > unrolled thread

[PATCH 0/3] USB: add API for interface driver to vote for autosuspend

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/3] USB: add API for interface driver to vote for autosuspend Yueyao Zhu <yueyao.zhu@gmail.com> - 2017-06-09 00:00 +0200
    [PATCH 3/3] sound: usb: allow interfaces that the driver claims to autosuspend Yueyao Zhu <yueyao.zhu@gmail.com> - 2017-06-09 00:00 +0200
    [PATCH 2/3] HID: usbhid: enable autosuspend for devices whose ... Yueyao Zhu <yueyao.zhu@gmail.com> - 2017-06-09 00:10 +0200
    Re: [PATCH 0/3] USB: add API for interface driver to vote for  autosuspend Alan Stern <stern@rowland.harvard.edu> - 2017-06-09 17:00 +0200

#1661722 — [PATCH 0/3] USB: add API for interface driver to vote for autosuspend

From: Yueyao Zhu <yueyao@google.com>

Currently, if a USB driver would like to enable autosuspend on the USB
device, usb_enable_autosuspend() seems to be the only option. However,
this acts on the device level, and other interfaces might not desire
to autosuspend the USB device.

For example, for the usb digital audio driver to enable autosuspend on
a device, calling usb_enable_autosuspend() from the interface driver
might not be a good idea as the USB device might have a keyboard HID
interface which generally doesn't handle autosupend very well.

This patch series introduces an API for interface driver to vote
for autosuspend on the interface, and when all interfaces agree to
it, autosuspend can then be enabled on the usb device.

Yueyao Zhu (3):
  USB: add API for interface driver to vote for autosuspend
  HID: usbhid: enable autosuspend for devices whose ...
  sound: usb: allow interfaces the driver claims to autosuspend

 drivers/hid/usbhid/hid-core.c | 37 +++++++++++++++++++++++++++++
 drivers/usb/core/driver.c     | 55 +++++++++++++++++++++++++++++++++++++++++++
 include/linux/usb.h           | 10 ++++++++
 sound/usb/card.c              |  3 +++
 4 files changed, 105 insertions(+)

-- 
2.13.0.506.g27d5fe0cd-goog

[toc] | [next] | [standalone]

#1661724 — [PATCH 3/3] sound: usb: allow interfaces that the driver claims to autosuspend

From: Yueyao Zhu <yueyao@google.com>

Instead of calling usb_enable_autosuspend() to change the configuration
of a USB device as an interface driver, enable autosuspend for every
interfaces that the driver claims.

Signed-off-by: Yueyao Zhu <yueyao@google.com>
---
 sound/usb/card.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/sound/usb/card.c b/sound/usb/card.c
index 6640277a725b..79bec2883740 100644
--- a/sound/usb/card.c
+++ b/sound/usb/card.c
@@ -184,6 +184,7 @@ static int snd_usb_create_stream(struct snd_usb_audio *chip, int ctrlif, int int
 			return -EINVAL;
 		}
 		usb_driver_claim_interface(&usb_audio_driver, iface, (void *)-1L);
+		usb_allow_interface_autosuspend(iface);
 
 		return 0;
 	}
@@ -206,6 +207,7 @@ static int snd_usb_create_stream(struct snd_usb_audio *chip, int ctrlif, int int
 	if (! snd_usb_parse_audio_interface(chip, interface)) {
 		usb_set_interface(dev, interface, 0); /* reset the current interface */
 		usb_driver_claim_interface(&usb_audio_driver, iface, (void *)-1L);
+		usb_allow_interface_autosuspend(iface);
 	}
 
 	return 0;
@@ -618,6 +620,7 @@ static int usb_audio_probe(struct usb_interface *intf,
 	usb_chip[chip->index] = chip;
 	chip->num_interfaces++;
 	usb_set_intfdata(intf, chip);
+	usb_allow_interface_autosuspend(intf);
 	atomic_dec(&chip->active);
 	mutex_unlock(&register_mutex);
 	return 0;
-- 
2.13.0.506.g27d5fe0cd-goog

[toc] | [prev] | [next] | [standalone]

#1661744 — [PATCH 2/3] HID: usbhid: enable autosuspend for devices whose ...

From: Yueyao Zhu <yueyao@google.com>

usage pages only points to CONSUMER. This allows autosuspend
to be enabled and thus power saved on CONSUMER hid devices.
Yet hid devices use other usage pages are not affected, e.g.
keyboard, mouse.

Signed-off-by: Yueyao Zhu <yueyao@google.com>
---
 drivers/hid/usbhid/hid-core.c | 37 +++++++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
index 83772fa7d92a..f2696b4098b5 100644
--- a/drivers/hid/usbhid/hid-core.c
+++ b/drivers/hid/usbhid/hid-core.c
@@ -1270,6 +1270,31 @@ static struct hid_ll_driver usb_hid_driver = {
 	.idle = usbhid_idle,
 };
 
+static bool usbhid_usage_is_consumer(struct hid_device *hid)
+{
+	int i;
+	struct hid_report_enum *input_report_enum =
+			hid->report_enum + HID_INPUT_REPORT;
+	struct list_head *report_list = &input_report_enum->report_list;
+	struct hid_report *report;
+	unsigned int usage_page;
+
+	if (list_empty(report_list))
+		return false;
+
+	list_for_each_entry(report, report_list, list) {
+		for (i = 0; i < report->maxfield; i++) {
+			/* only checks Application field */
+			usage_page =
+				report->field[i]->application & HID_USAGE_PAGE;
+			if (usage_page != HID_UP_CONSUMER)
+				return false;
+		}
+	}
+
+	return true;
+}
+
 static int usbhid_probe(struct usb_interface *intf, const struct usb_device_id *id)
 {
 	struct usb_host_interface *interface = intf->cur_altsetting;
@@ -1363,6 +1388,18 @@ static int usbhid_probe(struct usb_interface *intf, const struct usb_device_id *
 		goto err_free;
 	}
 
+	/*
+	 * Autosuspend interfaces which have only consumer usage page inputs.
+	 * This allows autosuspending devices which adhere to the Android USB
+	 * headset spec while not autosuspending general keyboards, mice, and
+	 * joysticks which may misbehave.
+	 */
+	if (usbhid_usage_is_consumer(hid)) {
+		intf->needs_remote_wakeup = true;
+		usb_allow_interface_autosuspend(intf);
+		hid_info(hid, "autosuspend enabled on the hid interface\n");
+	}
+
 	return 0;
 err_free:
 	kfree(usbhid);
-- 
2.13.0.506.g27d5fe0cd-goog

[toc] | [prev] | [next] | [standalone]

#1662540 — Re: [PATCH 0/3] USB: add API for interface driver to vote for autosuspend

On Thu, 8 Jun 2017, Yueyao Zhu wrote:

> From: Yueyao Zhu <yueyao@google.com>
> 
> Currently, if a USB driver would like to enable autosuspend on the USB
> device, usb_enable_autosuspend() seems to be the only option. However,
> this acts on the device level, and other interfaces might not desire
> to autosuspend the USB device.
> 
> For example, for the usb digital audio driver to enable autosuspend on
> a device, calling usb_enable_autosuspend() from the interface driver
> might not be a good idea as the USB device might have a keyboard HID
> interface which generally doesn't handle autosupend very well.
> 
> This patch series introduces an API for interface driver to vote
> for autosuspend on the interface, and when all interfaces agree to
> it, autosuspend can then be enabled on the usb device.

The whole idea of this seems questionable.  USB interface drivers are
generally not supposed to enable or disable autosuspend -- that is a
policy decision left up to userspace.  There are a few exceptions for 
things like hubs, but this is generally true.

Why should the USB digital audio driver want to enable autosuspend?

Alan Stern

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web