Re: [PATCH] HID: usbhid: enable remote wakeup for mice

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Dear Greg:

On 2/23/2023 7:23 PM, Greg KH wrote:
On Thu, Feb 23, 2023 at 07:18:12PM +0800, Michael Wu wrote:
Dear Greg,

On 2/22/2023 2:04 PM, Greg KH wrote:
On Wed, Feb 22, 2023 at 09:39:44AM +0800, Michael Wu wrote:
This patch fixes a problem that USB mouse can't wake up the device that
enters standby.

This not a problem, it is that way by design.

I got it, maybe it's a little problem to say that.

It is.

At present, the kernel only checks whether certain USB manufacturers
support wake-up, which will easily cause inconvenience to the
development work of other manufacturers and add unnecessary work to the
maintenance of kernel.

The USB protocol supports judging whether a usb supports the wake-up
function, so it should be more reasonable to add a wake-up source by
directly checking the settings from the USB protocol.

But you do not do that in this patch, why not?

I just want to explain the background of my patch, to prove we could use a
similar way to avoid such a "disturbing" situation.
To reduce the influence, my patch enables remote wakeup for USB mouse
devices refer to what keyboard do.

Keyboards are not mice :)

Sorry, What I wanted to say is that we registered the mouse wake-up source by referring to the practice of the keyboard.


There was a similar issue on the keyboard before, which was fixed by
this patch (3d61510f4eca), but now the problem happened on the mouse.
This patch uses a similar idea to fix this problem.

Signed-off-by: Michael Wu <michael@xxxxxxxxxxxxxxxxx>
---
   drivers/hid/usbhid/hid-core.c | 8 ++++++++
   drivers/hid/usbhid/usbmouse.c | 1 +
   2 files changed, 9 insertions(+)

diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
index be4c731aaa65..d3a6755cca09 100644
--- a/drivers/hid/usbhid/hid-core.c
+++ b/drivers/hid/usbhid/hid-core.c
@@ -1189,6 +1189,14 @@ static int usbhid_start(struct hid_device *hid)
   		device_set_wakeup_enable(&dev->dev, 1);
   	}
+	/**
+	 * NOTE: enable remote wakeup by default for all mouse devices
+	 * supporting the boot protocol.
+	 */
+	if (interface->desc.bInterfaceSubClass == USB_INTERFACE_SUBCLASS_BOOT &&
+	    interface->desc.bInterfaceProtocol == USB_INTERFACE_PROTOCOL_MOUSE)
+		device_set_wakeup_enable(&dev->dev, 1);

Sorry, but we can not take this unless it is proven that this will work
properly for all of these devices.  Other operating systems do not do
this last I checked, so there will be problems.

As Mario Limonciello says, they has confirmed that the Microsoft Windows
does set a similar policy as well. Can we talk about more in this topic: why
does Linux not support it?
Of course, if you have other great idea, I will appreciate that if we can
have some further discussion.

You need to provide some sort of "proof" that this has been heavily
tested on a huge range of devices before we can change this.

When this was first implemented, Windows did not work this way and many
devices on the market were broken if this were to be enabled.  I'm sure
the mailing list archives from 20+ years ago have many more details,
please dig around there for specifics.

If you feel strongly that this is the way forward, why not do it in
userspace today for your systems as part of testing this out?  It should
not require a kernel change, right?

Thanks for your advises. I'm clear now. I will try it in userspace.


thanks,

greg k-h

--
Regards,
Michael Wu



[Index of Archives]     [Linux Media Devel]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Linux Wireless Networking]     [Linux Omap]

  Powered by Linux