The remote wake-up function is a regular function on usb hid device
and I think keeping it enabled by default will make usb application
more convenient. This patch is to enable remote wakeup function for
usb hid device.
Signed-off-by: Yinbo Zhu <[email protected]>
---
drivers/hid/usbhid/hid-core.c | 13 ++++++++++---
1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
index 2dcaf31..3619b95 100644
--- a/drivers/hid/usbhid/hid-core.c
+++ b/drivers/hid/usbhid/hid-core.c
@@ -1063,7 +1063,9 @@ static int usbhid_start(struct hid_device *hid)
struct usb_host_interface *interface = intf->cur_altsetting;
struct usb_device *dev = interface_to_usbdev(intf);
struct usbhid_device *usbhid = hid->driver_data;
+ struct usb_host_config *config;
unsigned int n, insize = 0;
+ int ncfg;
int ret;
mutex_lock(&usbhid->mutex);
@@ -1179,14 +1181,19 @@ static int usbhid_start(struct hid_device *hid)
/* 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);
+ }
+
+ for (ncfg = 0; ncfg < dev->descriptor.bNumConfigurations; ncfg++) {
+ config = &dev->config[ncfg];
+ if ((config->desc.bmAttributes & (1 << 5)) == 0)
+ break;
+ if (ncfg + 1 == dev->descriptor.bNumConfigurations)
+ device_set_wakeup_enable(&dev->dev, 1);
}
mutex_unlock(&usbhid->mutex);
--
1.8.3.1
On 08.12.21 10:39, Yinbo Zhu wrote:
> The remote wake-up function is a regular function on usb hid device
> and I think keeping it enabled by default will make usb application
> more convenient. This patch is to enable remote wakeup function for
> usb hid device.
>
Hi,
I am afraid we cannot do this. It will cause regression.
Consider for example the case of laptops with touchscreens
that will trigger a wake up when the laptop is closed.
Regards
Oliver
On Wed, Dec 08, 2021 at 05:39:50PM +0800, Yinbo Zhu wrote:
> The remote wake-up function is a regular function on usb hid device
> and I think keeping it enabled by default will make usb application
> more convenient. This patch is to enable remote wakeup function for
> usb hid device.
How many devices did you test this on?
As Oliver said, this will cause problems, there's a reason no operating
system does this :(
sorry,
greg k-h
在 2021/12/8 下午6:03, Oliver Neukum 写道:
>
> On 08.12.21 10:39, Yinbo Zhu wrote:
>> The remote wake-up function is a regular function on usb hid device
>> and I think keeping it enabled by default will make usb application
>> more convenient. This patch is to enable remote wakeup function for
>> usb hid device.
>>
> Hi,
>
> I am afraid we cannot do this. It will cause regression.
> Consider for example the case of laptops with touchscreens
> that will trigger a wake up when the laptop is closed.
>
> Regards
>
> Oliver
Hi oliver,
system ask that must it must be accped a acpi lid open event then system
will always into resume state for laptop, otherwise, eventhough that
system be wakeuped by other event then system will continue into suspend.
and for laptop usb wakeup that as general ask bios to enable usb wakeup
then if need do more things to enable usb wakeup I think this usb wakeup
function isn't friendly and inconveient, so enable it by default.
after add this patch, if want to use usb wakeup function it only need
enable bios configure it think it is appropriate.
BRs,
Yinbo.
>
?? 2021/12/8 ????7:55, Greg Kroah-Hartman д??:
> On Wed, Dec 08, 2021 at 05:39:50PM +0800, Yinbo Zhu wrote:
>> The remote wake-up function is a regular function on usb hid device
>> and I think keeping it enabled by default will make usb application
>> more convenient. This patch is to enable remote wakeup function for
>> usb hid device.
>
> How many devices did you test this on?
>
> As Oliver said, this will cause problems, there's a reason no operating
> system does this :(
>
> sorry,
>
> greg k-h
Hi greg,
About that oliver said that I had expained, and I add this change was
according that usb device whether support remote wakeup and if it
support wakeup then to enabled it so I think it should be okay for all
hid device.
BRs,
Yinbo.
>
On Fri, Dec 10, 2021 at 05:54:33PM +0800, zhuyinbo wrote:
>
>
> 在 2021/12/8 下午7:55, Greg Kroah-Hartman 写道:
> > On Wed, Dec 08, 2021 at 05:39:50PM +0800, Yinbo Zhu wrote:
> > > The remote wake-up function is a regular function on usb hid device
> > > and I think keeping it enabled by default will make usb application
> > > more convenient. This patch is to enable remote wakeup function for
> > > usb hid device.
> >
> > How many devices did you test this on?
> >
> > As Oliver said, this will cause problems, there's a reason no operating
> > system does this :(
> >
> > sorry,
> >
> > greg k-h
> Hi greg,
>
> About that oliver said that I had expained, and I add this change was
> according that usb device whether support remote wakeup and if it support
> wakeup then to enabled it so I think it should be okay for all hid device.
Again, what devices did you test this on?
And look at other operating systems, as I said, there is a reason that
no one does this.
thanks,
greg k-h
On 10.12.21 10:50, zhuyinbo wrote:
Hi,
> system ask that must it must be accped a acpi lid open event then
> system will always into resume state for laptop, otherwise, eventhough
> that system be wakeuped by other event then system will continue into
> suspend.
Lid events are necesarily for the whole system.
>
> and for laptop usb wakeup that as general ask bios to enable usb
> wakeup then if need do more things to enable usb wakeup I think this
> usb wakeup function isn't friendly and inconveient, so enable it by
> default.
> after add this patch, if want to use usb wakeup function it only need
> enable bios configure it think it is appropriate.
>
No. If you wish your laptop to be resumed by USB events, that is one thing.
You can alter the system settings. That must work. But it is a different
issue
from the default.
In general any HID device must have wakeup capability to be usable for
selective suspend. You cannot draw conclusions from that.
Regards
Oliver
在 2021/12/14 下午10:21, Oliver Neukum 写道:
>
> On 10.12.21 10:50, zhuyinbo wrote:
> Hi,
>> system ask that must it must be accped a acpi lid open event then
>> system will always into resume state for laptop, otherwise, eventhough
>> that system be wakeuped by other event then system will continue into
>> suspend.
> Lid events are necesarily for the whole system.
>>
>> and for laptop usb wakeup that as general ask bios to enable usb
>> wakeup then if need do more things to enable usb wakeup I think this
>> usb wakeup function isn't friendly and inconveient, so enable it by
>> default.
>> after add this patch, if want to use usb wakeup function it only need
>> enable bios configure it think it is appropriate.
>>
> No. If you wish your laptop to be resumed by USB events, that is one thing.
> You can alter the system settings. That must work. But it is a different
> issue
Hi Oliver,
if you only talk about wakeup source you can think that usb-wakeup
source and acpi-lid wakeup source was different things. but if you talk
about laptop and distinguish lid and other event and you shoud know the
cannotation why system still continue sleep when lid closed then system
by other event wakeup. if you need test usb-wakeup for laptop and that
lid shouldn't be closed.
>
> from the default.
>
> In general any HID device must have wakeup capability to be usable for
> selective suspend. You cannot draw conclusions from that.
you still can has wakeup capability, but it should be keep enabled by
default. because the hid device should be convenient for human, if you
don't think so and I think HID definition is ridiculous.
In addition, I had said that laptop usb wakeup was disabled in system
bios by default and if user want enable usb wakeup that was only by
configure bios and doesn't need enable wakeup node if my patch was applied
>
> Regards
> Oliver
>
在 2021/12/10 下午6:45, Greg Kroah-Hartman 写道:
> On Fri, Dec 10, 2021 at 05:54:33PM +0800, zhuyinbo wrote:
>>
>>
>> 在 2021/12/8 下午7:55, Greg Kroah-Hartman 写道:
>>> On Wed, Dec 08, 2021 at 05:39:50PM +0800, Yinbo Zhu wrote:
>>>> The remote wake-up function is a regular function on usb hid device
>>>> and I think keeping it enabled by default will make usb application
>>>> more convenient. This patch is to enable remote wakeup function for
>>>> usb hid device.
>>>
>>> How many devices did you test this on?
>>>
>>> As Oliver said, this will cause problems, there's a reason no operating
>>> system does this :(
>>>
>>> sorry,
>>>
>>> greg k-h
>> Hi greg,
>>
>> About that oliver said that I had expained, and I add this change was
>> according that usb device whether support remote wakeup and if it support
>> wakeup then to enabled it so I think it should be okay for all hid device.
>
> Again, what devices did you test this on?
Hi greg k-h
mouse device and keyboard device. in hid code that keyboad was enabled
on wakeup node by default.
>
> And look at other operating systems, as I said, there is a reason that
> no one does this.
System should be do that. otherwise that lid open event was no make
sense. and you need know the order that after lid open then system will
be wakeup then system will accept lid open event by input subsystem .
in addition, whatever that usb wakeup was disabled in bios by default
even though after apply my patch.
>
> thanks,
>
> greg k-h
>
On 16.12.21 11:59, zhuyinbo wrote:
>
>
Hi,
> if you only talk about wakeup source you can think that usb-wakeup
> source and acpi-lid wakeup source was different things. but if you
> talk about laptop and distinguish lid and other event and you shoud
> know the cannotation why system still continue sleep when lid closed
> then system by other event wakeup. if you need test usb-wakeup for
> laptop and that lid shouldn't be closed.
I am sorry, I am not sure what you wish to say here. Could you rephrase it?
>> from the default.
>>
>> In general any HID device must have wakeup capability to be usable for
>> selective suspend. You cannot draw conclusions from that.
> you still can has wakeup capability, but it should be keep enabled by
> default. because the hid device should be convenient for human,
Well, no. We are talking about a kernel default. That needs to be so that it
always works on all systems. Convinience is secondary.
> if you don't think so and I think HID definition is ridiculous.
It does have its weaknesses, in particular with respect to differentiating
between events for wakeups. But we cannot change it.
>
>
> In addition, I had said that laptop usb wakeup was disabled in system
> bios by default and if user want enable usb wakeup that was only by
> configure bios and doesn't need enable wakeup node if my patch was
> applied
If you deviate from the default, you deviate. That is reducing the number of
changes is worth little. The default must be above everything else safe.
Regards
Oliver
在 2021/12/16 下午8:42, Oliver Neukum 写道:
>
> On 16.12.21 11:59, zhuyinbo wrote:
>>
>>
> Hi,
>
>
>> if you only talk about wakeup source you can think that usb-wakeup
>> source and acpi-lid wakeup source was different things. but if you
>> talk about laptop and distinguish lid and other event and you shoud
>> know the cannotation why system still continue sleep when lid closed
>> then system by other event wakeup. if you need test usb-wakeup for
>> laptop and that lid shouldn't be closed.
> I am sorry, I am not sure what you wish to say here. Could you rephrase it?
>>> from the default.
That connotation lid was closed represent human will not use laptop and
system must keep it was sleep and even though the laptop was
accidentally awakened.
>>>
>>> In general any HID device must have wakeup capability to be usable for
>>> selective suspend. You cannot draw conclusions from that.
>> you still can has wakeup capability, but it should be keep enabled by
>> default. because the hid device should be convenient for human,
>
> Well, no. We are talking about a kernel default. That needs to be so that it
> always works on all systems. Convinience is secondary.
if system can doesn't consider lid open event and ignore the connotation
about lid open event I think that system behavior is inappropriate. you
don't think my patch was inapproriate that on some system doesn't
consider lid open event.
In additon, if it doesn't include my patch and non-keyboard hid device
doesn't make system wakeup by ohci. because ohci driver doesn't export
wakeup property for usb slave device.
>
>
>> if you don't think so and I think HID definition is ridiculous.
> It does have its weaknesses, in particular with respect to differentiating
> between events for wakeups. But we cannot change it.
>>
>>
>> In addition, I had said that laptop usb wakeup was disabled in system
>> bios by default and if user want enable usb wakeup that was only by
>> configure bios and doesn't need enable wakeup node if my patch was
>> applied
> If you deviate from the default, you deviate. That is reducing the number of
> changes is worth little. The default must be above everything else safe.
>
> Regards
> Oliver
bios and kernel was two sets of things and they should has their own
indepdent configuration. if bios enable usb wakeup but wakeup is still
not work well. Do you think it is appropriate?
in additon, The keyboard device is enabled by default, and other hid
devices should also be enabled. Otherwise, it will be treated differently.
>