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

From: "Limonciello, Mario" <mario.limonciello@amd.com>
To: Greg KH <gregkh@linuxfoundation.org>,
	Michael Wu <michael@allwinnertech.com>,
	Richard Gong <richard.gong@amd.com>
Cc: jikos@kernel.org, benjamin.tissoires@redhat.com,
	linux-usb@vger.kernel.org, linux-input@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] HID: usbhid: enable remote wakeup for mice
Date: Wed, 22 Feb 2023 13:50:31 -0600	[thread overview]
Message-ID: <c99a7ae3-b8ab-30e2-67bc-87b5a7736728@amd.com> (raw)
In-Reply-To: <Y/WwXBF37hoZBbQa@kroah.com>

+Richard

On 2/22/2023 00:04, 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.
> 
>> 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?
> 
>> 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@allwinnertech.com>
>> ---
>>   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.
> 
> thanks,
> 
> greg k-h
> 

Richard and I both sent out relatively similar patches in the past, but 
they never went anywhere.
We did confirm that Windows does set a similar policy as well though, 
which is what prompted us to attempt this.

As there is more interest again maybe we can revive that discussion and 
merge together some ideas from the sets of patches.

Previous submissions:

v4:
https://lore.kernel.org/linux-usb/20220825045517.16791-1-mario.limonciello@amd.com/

v3:
https://lore.kernel.org/linux-usb/20220701023328.2783-10-mario.limonciello@amd.com/

v2:
https://lore.kernel.org/linux-usb/20220616183142.14472-1-mario.limonciello@amd.com/

v1:
https://lore.kernel.org/linux-usb/20220404214557.3329796-1-richard.gong@amd.com/