On Thu, Jun 16, 2022 at 01:31:42PM -0500, Mario Limonciello wrote: > The USB HID transport layer doesn't set mice for wakeup by default so users > can not wake system from s2idle using wired USB mouse. However, users can > wake the same system from Modern Standby on Windows with the same wired > USB mouse. > > Microsoft documentation indicates that all USB mice and touchpads should > be waking the system from Modern Standby. To align expectations from users > make this behavior the same when the system is configured by the OEM and > the user to use s2idle in Linux. > > Link: https://docs.microsoft.com/en-us/windows-hardware/design/device-experiences/modern-standby-wake-sources#input-devices-1 > Link: https://lore.kernel.org/linux-usb/20220404214557.3329796-1-richard.gong@xxxxxxx/ > Suggested-by: Richard Gong <richard.gong@xxxxxxx> > Signed-off-by: Mario Limonciello <mario.limonciello@xxxxxxx> > --- > More people keep coming to us confused that they couldn't wake a Linux system > up from sleep using a mouse, so this patch is being revived. How many different devices did you test this on? > > Microsoft documentation doesn't indicate any allowlist for this behavior, and > they actually prescribe it for all USB mice and touchpads. > v1->v2: > * Resubmit by Mario > * Update commit message > * Only activate on systems configured by user and OEM for using s2idle > > drivers/hid/usbhid/hid-core.c | 36 ++++++++++++++++++++++++----------- > 1 file changed, 25 insertions(+), 11 deletions(-) > > diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c > index 4490e2f7252a..3a1214ecec49 100644 > --- a/drivers/hid/usbhid/hid-core.c > +++ b/drivers/hid/usbhid/hid-core.c > @@ -12,6 +12,7 @@ > /* > */ > > +#include <linux/acpi.h> > #include <linux/module.h> > #include <linux/slab.h> > #include <linux/init.h> > @@ -26,6 +27,7 @@ > #include <linux/wait.h> > #include <linux/workqueue.h> > #include <linux/string.h> > +#include <linux/suspend.h> > > #include <linux/usb.h> > > @@ -1176,17 +1178,29 @@ static int usbhid_start(struct hid_device *hid) > usb_autopm_put_interface(usbhid->intf); > } > > - /* 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); > + if (interface->desc.bInterfaceSubClass == USB_INTERFACE_SUBCLASS_BOOT) { > + switch (interface->desc.bInterfaceProtocol) { > + /* 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. > + */ > + case USB_INTERFACE_PROTOCOL_KEYBOARD: > + usbhid_set_leds(hid); > + device_set_wakeup_enable(&dev->dev, 1); > + break; > +#ifdef CONFIG_ACPI Why a #ifdef? > + /* Setup remote wakeup by default for mice supporting boot > + * protocol if the system supports s2idle > + */ > + case USB_INTERFACE_PROTOCOL_MOUSE: > + if (acpi_gbl_FADT.flags & ACPI_FADT_LOW_POWER_S0 && That seems odd, there's no acpi/pm call for this instead? > + pm_suspend_default_s2idle()) > + device_set_wakeup_enable(&dev->dev, 1); > + break; > +#endif thanks, greg k-h
