2014-12-18 12:00:36

by Du, Changbin

[permalink] [raw]
Subject: [DISCUSSION] USB device remote wakeup is not working for S3 case

When I am checking usb remote wakeup code, I found that usb remote wakeup will not work after system going to S3 sate and I confirmed with my PC.
During enumeration, usb device will be set as wakeup capable by usb_set_device_state if it supports. Whether usb driver send SET_FEATURE(REMOTE_WAKUP) usb request when suspending on S3 depend on do_remote_wakeup flag which is set by choose_wakeup(). It can be simply presented as below.
do_remote_wakeup = device_may_wakeup(&udev->dev);
The return value is always false since usb device is not marked as wakeup enabled(that is no one call device_set_wakeup_enable() for usb device). As a result, usb device will not signal wakeup event to host.

Maybe we should not allow all remote wakeup supported device can wakeup system by default. But for usb keyboard/mouse, I think it is not reasonable to disable it by default.

A simple way to fix this is that replace the device_may_wakeup by device_can_wakeup in choose_wakeup() function, just like on auto suspend case (usb remote wakeup works for rpm). Another way is to make usb device wakeup able by default. But both of them will make all usb devices can wakeup system. Have any better idea?


2014-12-18 15:35:51

by Alan Stern

[permalink] [raw]
Subject: Re: [DISCUSSION] USB device remote wakeup is not working for S3 case

Please tell your email client to wrap lines after 72 columns or so.

On Thu, 18 Dec 2014, Du, Changbin wrote:

> When I am checking usb remote wakeup code, I found that usb remote
> wakeup will not work after system going to S3 sate and I confirmed
> with my PC.

USB remote _does_ work.

> During enumeration, usb device will be set as wakeup capable by
> usb_set_device_state if it supports. Whether usb driver send
> SET_FEATURE(REMOTE_WAKUP) usb request when suspending on S3 depend on
> do_remote_wakeup flag which is set by choose_wakeup(). It can be
> simply presented as below.
> do_remote_wakeup = device_may_wakeup(&udev->dev);

That's right.

> The return value is always false since usb device is not marked as
> wakeup enabled(that is no one call device_set_wakeup_enable() for usb
> device). As a result, usb device will not signal wakeup event to
> host.

There's a simple solution: Call device_set_wakeup_enable() for the
device! You can do this from the command line by:

echo auto >/sys/bus/usb/devices/.../power/control

where the "..." is the pathname for your device.

> Maybe we should not allow all remote wakeup supported device can
> wakeup system by default. But for usb keyboard/mouse, I think it is
> not reasonable to disable it by default.

For USB keyboards, wakeup _is_ enabled by default. See this section of
code close to the end of usbhid_start():

/* Some keyboards don't work until their LEDs have been set.
* Since BIOSes do set the LEDs, it must be safe for any device
* that supports the keyboard boot protocol.
* In addition, enable remote wakeup by default for all keyboard
* devices supporting the boot protocol.
*/
if (interface->desc.bInterfaceSubClass == USB_INTERFACE_SUBCLASS_BOOT &&
interface->desc.bInterfaceProtocol ==
USB_INTERFACE_PROTOCOL_KEYBOARD) {
usbhid_set_leds(hid);
device_set_wakeup_enable(&dev->dev, 1);
}

> A simple way to fix this is that replace the device_may_wakeup by
> device_can_wakeup in choose_wakeup() function, just like on auto
> suspend case (usb remote wakeup works for rpm). Another way is to
> make usb device wakeup able by default. But both of them will make
> all usb devices can wakeup system. Have any better idea?

How about leaving everything the way it is now? If you want to enable
wakeup for something like a USB mouse, you can write a udev script to
do it as shown above.

Alan Stern

2014-12-19 06:59:41

by Du, Changbin

[permalink] [raw]
Subject: RE: [DISCUSSION] USB device remote wakeup is not working for S3 case

> There's a simple solution: Call device_set_wakeup_enable() for the device!
> You can do this from the command line by:
>
> echo auto >/sys/bus/usb/devices/.../power/control
>
> where the "..." is the pathname for your device.
>
Yes, this can enable auto-suspend for usb device like a mouse. And remote wakeup
does work for RPM because rpm suspend refers to the needs_remote_wakeup flag.
But this doesn't impact system level suspend. The correct thing is:

echo enabled >/sys/bus/usb/devices/.../power/wakeup

This will call device_set_wakeup_enable() for the device and should work. But
unfortunately it seems didn't work even the wakeup file shows "enabled" which
means the device is wakeup enabled(Tried on 3.16 & 3.18 kernel). This is a different
issue, I will check.

> if (interface->desc.bInterfaceSubClass ==
> USB_INTERFACE_SUBCLASS_BOOT &&
> interface->desc.bInterfaceProtocol ==
> USB_INTERFACE_PROTOCOL_KEYBOARD) {
> usbhid_set_leds(hid);
> device_set_wakeup_enable(&dev->dev, 1);
> }
>
> How about leaving everything the way it is now? If you want to enable
> wakeup for something like a USB mouse, you can write a udev script to do it
> as shown above.
>
> Alan Stern

Could we also enable wakeup for usb mouse? Or is there any concern to enable it?
Per my opinion, most people may expect clicking mouse can awake system.

Regards,
Du, Changbin

2014-12-19 15:25:34

by Alan Stern

[permalink] [raw]
Subject: RE: [DISCUSSION] USB device remote wakeup is not working for S3 case

On Fri, 19 Dec 2014, Du, Changbin wrote:

> > There's a simple solution: Call device_set_wakeup_enable() for the device!
> > You can do this from the command line by:
> >
> > echo auto >/sys/bus/usb/devices/.../power/control
> >
> > where the "..." is the pathname for your device.
> >
> Yes, this can enable auto-suspend for usb device like a mouse. And remote wakeup
> does work for RPM because rpm suspend refers to the needs_remote_wakeup flag.
> But this doesn't impact system level suspend. The correct thing is:
>
> echo enabled >/sys/bus/usb/devices/.../power/wakeup

Yes, I'm sorry, you're right. It's easy to mix up autosuspend and
wakeup.

> This will call device_set_wakeup_enable() for the device and should work. But
> unfortunately it seems didn't work even the wakeup file shows "enabled" which
> means the device is wakeup enabled(Tried on 3.16 & 3.18 kernel). This is a different
> issue, I will check.

You have to make sure that wakeup is also enabled for the host
controller the device is attached to. For some host controllers, it
may also be necessary to enable wakeup for the root hub.

> Could we also enable wakeup for usb mouse? Or is there any concern to enable it?
> Per my opinion, most people may expect clicking mouse can awake system.

It doesn't matter to me, but you should ask the people on the
linux-input mailing list.

Also, what about wakeup for a non-USB mouse? Shouldn't that be enabled
as well?

Alan Stern

2014-12-23 05:40:43

by Du, Changbin

[permalink] [raw]
Subject: RE: [DISCUSSION] USB device remote wakeup is not working for S3 case

> You have to make sure that wakeup is also enabled for the host
> controller the device is attached to. For some host controllers, it
> may also be necessary to enable wakeup for the root hub.
>
Yes, the root-hub is not wakeup enabled by default, actually hub
devices are not enabled. It works after I enable it via sysfs interface.

> > Could we also enable wakeup for usb mouse? Or is there any concern to
> enable it?
> > Per my opinion, most people may expect clicking mouse can awake system.
>
> It doesn't matter to me, but you should ask the people on the
> linux-input mailing list.
>
> Also, what about wakeup for a non-USB mouse? Shouldn't that be enabled
> as well?
Unfortunately the mouse I found are all of usb so cannot confirm it. The
concern is if we want usb keyboard/mouse wakeup work by default, it does
make sense only if the parent hubs are also enabled by default. This is a policy
issue and may be better place it on userspace. I have no better idea :) .

>
> Alan Stern
Regards
Du, Changbin

2014-12-23 20:19:51

by Alan Stern

[permalink] [raw]
Subject: RE: [DISCUSSION] USB device remote wakeup is not working for S3 case

On Tue, 23 Dec 2014, Du, Changbin wrote:

> > You have to make sure that wakeup is also enabled for the host
> > controller the device is attached to. For some host controllers, it
> > may also be necessary to enable wakeup for the root hub.
> >
> Yes, the root-hub is not wakeup enabled by default, actually hub
> devices are not enabled. It works after I enable it via sysfs interface.

On some hardware, device wakeup requests can work even when root-hub
wakeup is disabled -- it varies.

In general we do not want hubs to be enabled for wakeup. If they were,
then your computer would wake up the moment you unplugged a USB device.
You would not be able to (for example) to put your laptop to sleep,
unplug a USB mouse, and put the laptop in its case.

> > > Could we also enable wakeup for usb mouse? Or is there any concern to
> > enable it?
> > > Per my opinion, most people may expect clicking mouse can awake system.
> >
> > It doesn't matter to me, but you should ask the people on the
> > linux-input mailing list.
> >
> > Also, what about wakeup for a non-USB mouse? Shouldn't that be enabled
> > as well?
> Unfortunately the mouse I found are all of usb so cannot confirm it. The
> concern is if we want usb keyboard/mouse wakeup work by default, it does
> make sense only if the parent hubs are also enabled by default. This is a policy
> issue and may be better place it on userspace. I have no better idea :) .

As mentioned above, it depends on the hardware.

Alan Stern