lkml.org 
[lkml]   [2015]   [Jun]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH][RFC] usbhid: enable autosuspend for internal devices
On Fri, Jun 26, 2015 at 04:08:20PM -0400, Alan Stern wrote:
> On Fri, 26 Jun 2015, Tom Gundersen wrote:
>
> > This policy used to be unconditionally applied by udev, but there
> > is no reason to make userspace be involved in this and in the future
> > udev will not be doing it by default.
> >
> > See: <https://github.com/systemd/systemd/pull/353>.
> >
> > Signed-off-by: Tom Gundersen <teg@jklm.no>
> > Cc: Jiri Kosina <jkosina@suse.cz>
> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > ---
> >
> > Hi,
> >
> > I don't have the right hardware for this, so it has only been compile-tested.
> > I'm therefore sending it as an RFC only. Mainly I want to bring it to people's
> > attention that it would be great to get this feature into the kernel as we want
> > to drop it from udev.
> >
> > Cheers,
> >
> > Tom
> >
> > drivers/hid/usbhid/hid-core.c | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
> > index bfbe1be..af80700 100644
> > --- a/drivers/hid/usbhid/hid-core.c
> > +++ b/drivers/hid/usbhid/hid-core.c
> > @@ -1358,6 +1358,9 @@ static int usbhid_probe(struct usb_interface *intf, const struct usb_device_id *
> > setup_timer(&usbhid->io_retry, hid_retry_timeout, (unsigned long) hid);
> > spin_lock_init(&usbhid->lock);
> >
> > + if (dev->removable == USB_DEVICE_FIXED)
> > + usb_enable_autosuspend(dev);
>
> This doesn't do what the patch title says. USB_DEVICE_FIXED means that
> the device can't be unplugged from its upstream port. It doesn't mean
> the device is internal to the computer.
>
> As an example, consider a composite Apple keyboard, which has an
> internal 3-port USB hub where two of the hub's ports are exposed on the
> edge of the keyboard case and the keyboard controller is permanently
> attached to the third hub port. Then the controller device would be
> marked USB_DEVICE_FIXED, even though the whole thing is external to
> the computer and can be unplugged.
>

Is that really how those devices are marked? I can't find any of my
keyboard with hubs that mark things that way.

> A reasonable compromise might be
>
> /*
> * Enable autosuspend for devices permanently attached
> * to the root hub.
> */
> if (!dev->parent->parent && dev->removable == USB_DEVICE_FIXED)
> usb_enable_autosuspend(dev);
>
> But this doesn't work if there's a permanently attached hub and a
> device permanently attached to that hub. To do this thoroughly, you
> have to iterate up the dev->parent chain, making sure at each step that
> the ->removable value is USB_DEVICE_FIXED.
>
> Also, are you really certain this is safe? Aren't there a number of
> built-in keyboards that will work badly if you allow them to
> autosuspend?

udev has been doing this for a while now by default, with no reports of
problems, so I think we should be safe.

thanks,

greg k-h


\
 
 \ /
  Last update: 2015-06-27 00:41    [W:0.084 / U:1.464 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site