default value of power/wakeup

default value of power/wakeup

[Tom Yan]

I have the following two USB wireless mouse/keyboard receivers:

logitech-djreceiver 0003:046D:C52B.0003: hiddev0,hidraw0: USB HID
v1.11 Device [Logitech USB Receiver] on usb-0000:00:14.0-13/input2
hid-generic 0003:046D:C52E.0005: input,hidraw2: USB HID v1.11 Keyboard
[Logitech USB Receiver] on usb-0000:00:14.0-14/input0

The default values in their power/wakeup:
[tom@localhost ~]$ cat /sys/bus/usb/devices/3-13/power/wakeup
disabled
[tom@localhost ~]$ cat /sys/bus/usb/devices/3-14/power/wakeup
enabled

According to https://www.kernel.org/doc/Documentation/usb/power-management.txt:

power/wakeup

        This file is empty if the device does not support
        remote wakeup.  Otherwise the file contains either the
        word "enabled" or the word "disabled", ...

My question is, which part of the kernel or system determines the
default value? When I had a quick look on the source of the two driver
modules, I couldn't find any code related. So what caused the
difference?

P.S. I have no udev rule related to power/wakeup, which is the case I
am asking about.

[tom@localhost ~]$ cd /etc/udev/rules.d/
[tom@localhost rules.d]$ grep wakeup *
grep: *: No such file or directory
[tom@localhost rules.d]$ cd /lib/udev/rules.d/
[tom@localhost rules.d]$ grep wakeup *
42-usb-hid-pm.rules:# advertise remote wakeup support but don't
actually implement
42-usb-hid-pm.rules:# remote wakeup is working.

Re: default value of power/wakeup

[Alan Stern]

On Tue, 21 Apr 2015, Tom Yan wrote:

> I have the following two USB wireless mouse/keyboard receivers:
> 
> logitech-djreceiver 0003:046D:C52B.0003: hiddev0,hidraw0: USB HID
> v1.11 Device [Logitech USB Receiver] on usb-0000:00:14.0-13/input2
> hid-generic 0003:046D:C52E.0005: input,hidraw2: USB HID v1.11 Keyboard
> [Logitech USB Receiver] on usb-0000:00:14.0-14/input0
> 
> The default values in their power/wakeup:
> [tom@localhost ~]$ cat /sys/bus/usb/devices/3-13/power/wakeup
> disabled
> [tom@localhost ~]$ cat /sys/bus/usb/devices/3-14/power/wakeup
> enabled
> 
> According to https://www.kernel.org/doc/Documentation/usb/power-management.txt:
> 
> power/wakeup
> 
>         This file is empty if the device does not support
>         remote wakeup.  Otherwise the file contains either the
>         word "enabled" or the word "disabled", ...
> 
> My question is, which part of the kernel or system determines the
> default value? When I had a quick look on the source of the two driver
> modules, I couldn't find any code related. So what caused the
> difference?
> 
> P.S. I have no udev rule related to power/wakeup, which is the case I
> am asking about.
> 
> [tom@localhost ~]$ cd /etc/udev/rules.d/
> [tom@localhost rules.d]$ grep wakeup *
> grep: *: No such file or directory
> [tom@localhost rules.d]$ cd /lib/udev/rules.d/
> [tom@localhost rules.d]$ grep wakeup *
> 42-usb-hid-pm.rules:# advertise remote wakeup support but don't
> actually implement
> 42-usb-hid-pm.rules:# remote wakeup is working.

This is from usbhid_start() in drivers/hid/usbhid/hid-core.c:

	/* 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);
	}

Alan Stern

Re: default value of power/wakeup

[Tom Yan]

Thank you very much for your reply.

So I tried to see if there's any difference between the two devices. I
can see that the one didn't get enabled by default has an extra
interface with 0 for the two attributes. But does that matter? Because
if it does, shouldn't the mouse interface prevents the other one to
get enabled by default as well?

Seems to me the keyboard interface is the first interface for both devices:
[tom@localhost ~]$ cat
/sys/devices/pci0000\:00/0000\:00\:14.0/usb3/3-13/3-13\:1.0/bInterfaceSubClass
01
[tom@localhost ~]$ cat
/sys/devices/pci0000\:00/0000\:00\:14.0/usb3/3-13/3-13\:1.0/bInterfaceProtocol
01
[tom@localhost ~]$ cat
/sys/devices/pci0000\:00/0000\:00\:14.0/usb3/3-14/3-14\:1.0/bInterfaceSubClass
01
[tom@localhost ~]$ cat
/sys/devices/pci0000\:00/0000\:00\:14.0/usb3/3-14/3-14\:1.0/bInterfaceProtocol
01

[tom@localhost ~]$ diff <(sudo lsusb -v -s 003:002) <(sudo lsusb -v -s 003:003)
can't get device qualifier: Resource temporarily unavailable
can't get debug descriptor: Resource temporarily unavailable
can't get device qualifier: Resource temporarily unavailable
can't get debug descriptor: Resource temporarily unavailable
2c2
< Bus 003 Device 002: ID 046d:c52b Logitech, Inc. Unifying Receiver
---
> Bus 003 Device 003: ID 046d:c52e Logitech, Inc. MK260 Wireless Combo Receiver
12,13c12,13
<   idProduct          0xc52b Unifying Receiver
<   bcdDevice           12.01
---
>   idProduct          0xc52e MK260 Wireless Combo Receiver
>   bcdDevice           15.00
21,22c21,22
<     wTotalLength           84
<     bNumInterfaces          3
---
>     wTotalLength           59
>     bNumInterfaces          2
24c24
<     iConfiguration          4 RQR12.01_B0019
---
>     iConfiguration          4 RQR15.00_B0048
76c76
<           wDescriptorLength     148
---
>           wDescriptorLength     177
87,117c87
<         wMaxPacketSize     0x0008  1x 8 bytes
<         bInterval               2
<     Interface Descriptor:
<       bLength                 9
<       bDescriptorType         4
<       bInterfaceNumber        2
<       bAlternateSetting       0
<       bNumEndpoints           1
<       bInterfaceClass         3 Human Interface Device
<       bInterfaceSubClass      0
<       bInterfaceProtocol      0
<       iInterface              0
<         HID Device Descriptor:
<           bLength                 9
<           bDescriptorType        33
<           bcdHID               1.11
<           bCountryCode            0 Not supported
<           bNumDescriptors         1
<           bDescriptorType        34 Report
<           wDescriptorLength      98
<          Report Descriptors:
<            ** UNAVAILABLE **
<       Endpoint Descriptor:
<         bLength                 7
<         bDescriptorType         5
<         bEndpointAddress     0x83  EP 3 IN
<         bmAttributes            3
<           Transfer Type            Interrupt
<           Synch Type               None
<           Usage Type               Data
<         wMaxPacketSize     0x0020  1x 32 bytes
---
>         wMaxPacketSize     0x0014  1x 20 bytes

On 21 April 2015 at 01:41, Alan Stern <stern-nwvwT67g6+6dFdvTe/nMLpVzexx5G7lz@public.gmane.org> wrote:
> On Tue, 21 Apr 2015, Tom Yan wrote:
>
>> I have the following two USB wireless mouse/keyboard receivers:
>>
>> logitech-djreceiver 0003:046D:C52B.0003: hiddev0,hidraw0: USB HID
>> v1.11 Device [Logitech USB Receiver] on usb-0000:00:14.0-13/input2
>> hid-generic 0003:046D:C52E.0005: input,hidraw2: USB HID v1.11 Keyboard
>> [Logitech USB Receiver] on usb-0000:00:14.0-14/input0
>>
>> The default values in their power/wakeup:
>> [tom@localhost ~]$ cat /sys/bus/usb/devices/3-13/power/wakeup
>> disabled
>> [tom@localhost ~]$ cat /sys/bus/usb/devices/3-14/power/wakeup
>> enabled
>>
>> According to https://www.kernel.org/doc/Documentation/usb/power-management.txt:
>>
>> power/wakeup
>>
>>         This file is empty if the device does not support
>>         remote wakeup.  Otherwise the file contains either the
>>         word "enabled" or the word "disabled", ...
>>
>> My question is, which part of the kernel or system determines the
>> default value? When I had a quick look on the source of the two driver
>> modules, I couldn't find any code related. So what caused the
>> difference?
>>
>> P.S. I have no udev rule related to power/wakeup, which is the case I
>> am asking about.
>>
>> [tom@localhost ~]$ cd /etc/udev/rules.d/
>> [tom@localhost rules.d]$ grep wakeup *
>> grep: *: No such file or directory
>> [tom@localhost rules.d]$ cd /lib/udev/rules.d/
>> [tom@localhost rules.d]$ grep wakeup *
>> 42-usb-hid-pm.rules:# advertise remote wakeup support but don't
>> actually implement
>> 42-usb-hid-pm.rules:# remote wakeup is working.
>
> This is from usbhid_start() in drivers/hid/usbhid/hid-core.c:
>
>         /* 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);
>         }
>
> Alan Stern
>
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: default value of power/wakeup

[Alan Stern]

On Tue, 21 Apr 2015, Tom Yan wrote:

> Thank you very much for your reply.
> 
> So I tried to see if there's any difference between the two devices. I
> can see that the one didn't get enabled by default has an extra
> interface with 0 for the two attributes. But does that matter? Because
> if it does, shouldn't the mouse interface prevents the other one to
> get enabled by default as well?

No, it doesn't matter.

> Seems to me the keyboard interface is the first interface for both devices:
> [tom@localhost ~]$ cat
> /sys/devices/pci0000\:00/0000\:00\:14.0/usb3/3-13/3-13\:1.0/bInterfaceSubClass
> 01
> [tom@localhost ~]$ cat
> /sys/devices/pci0000\:00/0000\:00\:14.0/usb3/3-13/3-13\:1.0/bInterfaceProtocol
> 01
> [tom@localhost ~]$ cat
> /sys/devices/pci0000\:00/0000\:00\:14.0/usb3/3-14/3-14\:1.0/bInterfaceSubClass
> 01
> [tom@localhost ~]$ cat
> /sys/devices/pci0000\:00/0000\:00\:14.0/usb3/3-14/3-14\:1.0/bInterfaceProtocol
> 01
> 
> [tom@localhost ~]$ diff <(sudo lsusb -v -s 003:002) <(sudo lsusb -v -s 003:003)
> can't get device qualifier: Resource temporarily unavailable
> can't get debug descriptor: Resource temporarily unavailable
> can't get device qualifier: Resource temporarily unavailable
> can't get debug descriptor: Resource temporarily unavailable

It's a little difficult to tell what's going on just from the diff.  
Can you post the complete lsusb -v output for both devices?

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: default value of power/wakeup

[Tom Yan]

Yes of course:

Bus 003 Device 002: ID 046d:c52b Logitech, Inc. Unifying Receiver
Device Descriptor:
  bLength                18
  bDescriptorType         1
  bcdUSB               2.00
  bDeviceClass            0
  bDeviceSubClass         0
  bDeviceProtocol         0
  bMaxPacketSize0         8
  idVendor           0x046d Logitech, Inc.
  idProduct          0xc52b Unifying Receiver
  bcdDevice           12.01
  iManufacturer           1 Logitech
  iProduct                2 USB Receiver
  iSerial                 0
  bNumConfigurations      1
  Configuration Descriptor:
    bLength                 9
    bDescriptorType         2
    wTotalLength           84
    bNumInterfaces          3
    bConfigurationValue     1
    iConfiguration          4 RQR12.01_B0019
    bmAttributes         0xa0
      (Bus Powered)
      Remote Wakeup
    MaxPower               98mA
    Interface Descriptor:
      bLength                 9
      bDescriptorType         4
      bInterfaceNumber        0
      bAlternateSetting       0
      bNumEndpoints           1
      bInterfaceClass         3 Human Interface Device
      bInterfaceSubClass      1 Boot Interface Subclass
      bInterfaceProtocol      1 Keyboard
      iInterface              0
        HID Device Descriptor:
          bLength                 9
          bDescriptorType        33
          bcdHID               1.11
          bCountryCode            0 Not supported
          bNumDescriptors         1
          bDescriptorType        34 Report
          wDescriptorLength      59
         Report Descriptors:
           ** UNAVAILABLE **
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x81  EP 1 IN
        bmAttributes            3
          Transfer Type            Interrupt
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0008  1x 8 bytes
        bInterval               8
    Interface Descriptor:
      bLength                 9
      bDescriptorType         4
      bInterfaceNumber        1
      bAlternateSetting       0
      bNumEndpoints           1
      bInterfaceClass         3 Human Interface Device
      bInterfaceSubClass      1 Boot Interface Subclass
      bInterfaceProtocol      2 Mouse
      iInterface              0
        HID Device Descriptor:
          bLength                 9
          bDescriptorType        33
          bcdHID               1.11
          bCountryCode            0 Not supported
          bNumDescriptors         1
          bDescriptorType        34 Report
          wDescriptorLength     148
         Report Descriptors:
           ** UNAVAILABLE **
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x82  EP 2 IN
        bmAttributes            3
          Transfer Type            Interrupt
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0008  1x 8 bytes
        bInterval               2
    Interface Descriptor:
      bLength                 9
      bDescriptorType         4
      bInterfaceNumber        2
      bAlternateSetting       0
      bNumEndpoints           1
      bInterfaceClass         3 Human Interface Device
      bInterfaceSubClass      0
      bInterfaceProtocol      0
      iInterface              0
        HID Device Descriptor:
          bLength                 9
          bDescriptorType        33
          bcdHID               1.11
          bCountryCode            0 Not supported
          bNumDescriptors         1
          bDescriptorType        34 Report
          wDescriptorLength      98
         Report Descriptors:
           ** UNAVAILABLE **
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x83  EP 3 IN
        bmAttributes            3
          Transfer Type            Interrupt
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0020  1x 32 bytes
        bInterval               2
can't get device qualifier: Resource temporarily unavailable
can't get debug descriptor: Resource temporarily unavailable
Device Status:     0x0000
  (Bus Powered)

Bus 003 Device 003: ID 046d:c52e Logitech, Inc. MK260 Wireless Combo Receiver
Device Descriptor:
  bLength                18
  bDescriptorType         1
  bcdUSB               2.00
  bDeviceClass            0
  bDeviceSubClass         0
  bDeviceProtocol         0
  bMaxPacketSize0         8
  idVendor           0x046d Logitech, Inc.
  idProduct          0xc52e MK260 Wireless Combo Receiver
  bcdDevice           15.00
  iManufacturer           1 Logitech
  iProduct                2 USB Receiver
  iSerial                 0
  bNumConfigurations      1
  Configuration Descriptor:
    bLength                 9
    bDescriptorType         2
    wTotalLength           59
    bNumInterfaces          2
    bConfigurationValue     1
    iConfiguration          4 RQR15.00_B0048
    bmAttributes         0xa0
      (Bus Powered)
      Remote Wakeup
    MaxPower               98mA
    Interface Descriptor:
      bLength                 9
      bDescriptorType         4
      bInterfaceNumber        0
      bAlternateSetting       0
      bNumEndpoints           1
      bInterfaceClass         3 Human Interface Device
      bInterfaceSubClass      1 Boot Interface Subclass
      bInterfaceProtocol      1 Keyboard
      iInterface              0
        HID Device Descriptor:
          bLength                 9
          bDescriptorType        33
          bcdHID               1.11
          bCountryCode            0 Not supported
          bNumDescriptors         1
          bDescriptorType        34 Report
          wDescriptorLength      59
         Report Descriptors:
           ** UNAVAILABLE **
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x81  EP 1 IN
        bmAttributes            3
          Transfer Type            Interrupt
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0008  1x 8 bytes
        bInterval               8
    Interface Descriptor:
      bLength                 9
      bDescriptorType         4
      bInterfaceNumber        1
      bAlternateSetting       0
      bNumEndpoints           1
      bInterfaceClass         3 Human Interface Device
      bInterfaceSubClass      1 Boot Interface Subclass
      bInterfaceProtocol      2 Mouse
      iInterface              0
        HID Device Descriptor:
          bLength                 9
          bDescriptorType        33
          bcdHID               1.11
          bCountryCode            0 Not supported
          bNumDescriptors         1
          bDescriptorType        34 Report
          wDescriptorLength     177
         Report Descriptors:
           ** UNAVAILABLE **
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x82  EP 2 IN
        bmAttributes            3
          Transfer Type            Interrupt
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0014  1x 20 bytes
        bInterval               2
can't get device qualifier: Resource temporarily unavailable
can't get debug descriptor: Resource temporarily unavailable
Device Status:     0x0000
  (Bus Powered)

On 21 April 2015 at 03:08, Alan Stern <stern@rowland.harvard.edu> wrote:
> On Tue, 21 Apr 2015, Tom Yan wrote:
>
>> Thank you very much for your reply.
>>
>> So I tried to see if there's any difference between the two devices. I
>> can see that the one didn't get enabled by default has an extra
>> interface with 0 for the two attributes. But does that matter? Because
>> if it does, shouldn't the mouse interface prevents the other one to
>> get enabled by default as well?
>
> No, it doesn't matter.
>
>> Seems to me the keyboard interface is the first interface for both devices:
>> [tom@localhost ~]$ cat
>> /sys/devices/pci0000\:00/0000\:00\:14.0/usb3/3-13/3-13\:1.0/bInterfaceSubClass
>> 01
>> [tom@localhost ~]$ cat
>> /sys/devices/pci0000\:00/0000\:00\:14.0/usb3/3-13/3-13\:1.0/bInterfaceProtocol
>> 01
>> [tom@localhost ~]$ cat
>> /sys/devices/pci0000\:00/0000\:00\:14.0/usb3/3-14/3-14\:1.0/bInterfaceSubClass
>> 01
>> [tom@localhost ~]$ cat
>> /sys/devices/pci0000\:00/0000\:00\:14.0/usb3/3-14/3-14\:1.0/bInterfaceProtocol
>> 01
>>
>> [tom@localhost ~]$ diff <(sudo lsusb -v -s 003:002) <(sudo lsusb -v -s 003:003)
>> can't get device qualifier: Resource temporarily unavailable
>> can't get debug descriptor: Resource temporarily unavailable
>> can't get device qualifier: Resource temporarily unavailable
>> can't get debug descriptor: Resource temporarily unavailable
>
> It's a little difficult to tell what's going on just from the diff.
> Can you post the complete lsusb -v output for both devices?
>
> Alan Stern
>

Re: default value of power/wakeup

[Alan Stern]

On Tue, 21 Apr 2015, Tom Yan wrote:

> Yes of course:

Okay, the two devices appear to be the same as far as remote wakeup is
concerned.  The difference lies in the drivers.  From your original
report:

> logitech-djreceiver 0003:046D:C52B.0003: hiddev0,hidraw0: USB HID
> v1.11 Device [Logitech USB Receiver] on usb-0000:00:14.0-13/input2
> hid-generic 0003:046D:C52E.0005: input,hidraw2: USB HID v1.11 Keyboard
> [Logitech USB Receiver] on usb-0000:00:14.0-14/input0

As you can see, the first device uses the logitech-djreceiver driver
and the second uses the hid-generic driver.  The generic driver
includes code for enabling wakeup by default, but the
logitech-djreceiver driver doesn't.  In fact, it looks like none of the
special-purpose HID drivers will enable wakeup by default.

Alan Stern

Re: default value of power/wakeup

[Tom Yan]

On 21 April 2015 at 03:44, Alan Stern <stern-nwvwT67g6+6dFdvTe/nMLpVzexx5G7lz@public.gmane.org> wrote:

> The generic driver includes code for enabling wakeup by default,

Is there a part of code which can shows this? It seems to me that the
usbhid module is loaded for all USB HID devices, so I doubt a bit:

[tom@localhost ~]$ journalctl -b -1 | grep hid
Apr 21 05:00:36 localhost kernel: hidraw: raw HID events driver (C) Jiri Kosina
Apr 21 05:00:36 localhost kernel: usbcore: registered new interface
driver usbhid
Apr 21 05:00:36 localhost kernel: usbhid: USB HID core driver
Apr 21 05:00:36 localhost kernel: hid-generic 0003:046D:C52E.0001:
input,hidraw0: USB HID v1.11 Keyboard [Logitech USB Receiver] on
usb-0000:00:14.0-14/input0
Apr 21 05:00:36 localhost kernel: hid-generic 0003:046D:C52E.0002:
input,hiddev0,hidraw1: USB HID v1.11 Mouse [Logitech USB Receiver] on
usb-0000:00:14.0-14/input1

[tom@localhost ~]$ journalctl -b -2 | grep hid
Apr 21 04:59:58 localhost kernel: hidraw: raw HID events driver (C) Jiri Kosina
Apr 21 04:59:58 localhost kernel: usbcore: registered new interface
driver usbhid
Apr 21 04:59:58 localhost kernel: usbhid: USB HID core driver
Apr 21 04:59:58 localhost kernel: logitech-djreceiver
0003:046D:C52B.0003: hiddev0,hidraw0: USB HID v1.11 Device [Logitech
USB Receiver] on usb-0000:00:14.0-13/input2
Apr 21 04:59:58 localhost kernel: logitech-hidpp-device
0003:046D:4024.0004: input,hidraw1: USB HID v1.11 Keyboard [Logitech
K400] on usb-0000:00:14.0-13:1

[tom@localhost ~]$ lsusb -t
/:  Bus 04.Port 1: Dev 1, Class=root_hub, Driver=xhci_hcd/6p, 5000M
    |__ Port 1: Dev 2, If 0, Class=Mass Storage, Driver=usb-storage, 5000M
/:  Bus 03.Port 1: Dev 1, Class=root_hub, Driver=xhci_hcd/14p, 480M
    |__ Port 13: Dev 2, If 0, Class=Human Interface Device, Driver=usbhid, 12M
    |__ Port 13: Dev 2, If 1, Class=Human Interface Device, Driver=usbhid, 12M
    |__ Port 13: Dev 2, If 2, Class=Human Interface Device, Driver=usbhid, 12M
    |__ Port 14: Dev 3, If 0, Class=Human Interface Device, Driver=usbhid, 12M
    |__ Port 14: Dev 3, If 1, Class=Human Interface Device, Driver=usbhid, 12M
/:  Bus 02.Port 1: Dev 1, Class=root_hub, Driver=ehci-pci/2p, 480M
    |__ Port 1: Dev 2, If 0, Class=Hub, Driver=hub/8p, 480M
/:  Bus 01.Port 1: Dev 1, Class=root_hub, Driver=ehci-pci/2p, 480M
    |__ Port 1: Dev 2, If 0, Class=Hub, Driver=hub/6p, 480M
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: default value of power/wakeup

[Tom Yan]

By blacklisting the hid-generic, I think I can confirm hid-generic is
the module that matters. Though I'm curious how it use the code from
usbhid/hid-core.c because I couldn't find a clue in the short
hid-generic.c.

Anyway I don't know much about coding so it's not really my concern.
But I think the fact hid-generic sets a default (no matter if it's
"enabled" or "disabled") brings a problem. The reason is, the wakeup
attribute is not "initialized" or "created" by hid-generic, so udev
can apply a rule related to the attribute before hid-generic get
loaded (which seems to be what it is doing, see
https://bugs.freedesktop.org/show_bug.cgi?id=90041). If the module
gets loaded or reloaded afterwards, the value would be set to the
hard-coded default anyway.

Perhaps it is possible to change the behaviour by rewriting some bits
of udev, but I don't see the point of doing so. IMHO these hard-coded
default should be avoid from all drivers until it is really necessary,
and when it is needed, it's best to have the default set right at the
point when the attribute is initialized if possible.

On 21 April 2015 at 05:20, Tom Yan <tom.ty89-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On 21 April 2015 at 03:44, Alan Stern <stern-nwvwT67g6+6dFdvTe/nMLpVzexx5G7lz@public.gmane.org> wrote:
>
>> The generic driver includes code for enabling wakeup by default,
>
> Is there a part of code which can shows this? It seems to me that the
> usbhid module is loaded for all USB HID devices, so I doubt a bit:
>
> [tom@localhost ~]$ journalctl -b -1 | grep hid
> Apr 21 05:00:36 localhost kernel: hidraw: raw HID events driver (C) Jiri Kosina
> Apr 21 05:00:36 localhost kernel: usbcore: registered new interface
> driver usbhid
> Apr 21 05:00:36 localhost kernel: usbhid: USB HID core driver
> Apr 21 05:00:36 localhost kernel: hid-generic 0003:046D:C52E.0001:
> input,hidraw0: USB HID v1.11 Keyboard [Logitech USB Receiver] on
> usb-0000:00:14.0-14/input0
> Apr 21 05:00:36 localhost kernel: hid-generic 0003:046D:C52E.0002:
> input,hiddev0,hidraw1: USB HID v1.11 Mouse [Logitech USB Receiver] on
> usb-0000:00:14.0-14/input1
>
> [tom@localhost ~]$ journalctl -b -2 | grep hid
> Apr 21 04:59:58 localhost kernel: hidraw: raw HID events driver (C) Jiri Kosina
> Apr 21 04:59:58 localhost kernel: usbcore: registered new interface
> driver usbhid
> Apr 21 04:59:58 localhost kernel: usbhid: USB HID core driver
> Apr 21 04:59:58 localhost kernel: logitech-djreceiver
> 0003:046D:C52B.0003: hiddev0,hidraw0: USB HID v1.11 Device [Logitech
> USB Receiver] on usb-0000:00:14.0-13/input2
> Apr 21 04:59:58 localhost kernel: logitech-hidpp-device
> 0003:046D:4024.0004: input,hidraw1: USB HID v1.11 Keyboard [Logitech
> K400] on usb-0000:00:14.0-13:1
>
> [tom@localhost ~]$ lsusb -t
> /:  Bus 04.Port 1: Dev 1, Class=root_hub, Driver=xhci_hcd/6p, 5000M
>     |__ Port 1: Dev 2, If 0, Class=Mass Storage, Driver=usb-storage, 5000M
> /:  Bus 03.Port 1: Dev 1, Class=root_hub, Driver=xhci_hcd/14p, 480M
>     |__ Port 13: Dev 2, If 0, Class=Human Interface Device, Driver=usbhid, 12M
>     |__ Port 13: Dev 2, If 1, Class=Human Interface Device, Driver=usbhid, 12M
>     |__ Port 13: Dev 2, If 2, Class=Human Interface Device, Driver=usbhid, 12M
>     |__ Port 14: Dev 3, If 0, Class=Human Interface Device, Driver=usbhid, 12M
>     |__ Port 14: Dev 3, If 1, Class=Human Interface Device, Driver=usbhid, 12M
> /:  Bus 02.Port 1: Dev 1, Class=root_hub, Driver=ehci-pci/2p, 480M
>     |__ Port 1: Dev 2, If 0, Class=Hub, Driver=hub/8p, 480M
> /:  Bus 01.Port 1: Dev 1, Class=root_hub, Driver=ehci-pci/2p, 480M
>     |__ Port 1: Dev 2, If 0, Class=Hub, Driver=hub/6p, 480M
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: default value of power/wakeup

[Alan Stern]

On Tue, 21 Apr 2015, Tom Yan wrote:

> On 21 April 2015 at 03:44, Alan Stern <stern@rowland.harvard.edu> wrote:
> 
> > The generic driver includes code for enabling wakeup by default,
> 
> Is there a part of code which can shows this? It seems to me that the
> usbhid module is loaded for all USB HID devices, so I doubt a bit:
> 
> [tom@localhost ~]$ journalctl -b -1 | grep hid
> Apr 21 05:00:36 localhost kernel: hidraw: raw HID events driver (C) Jiri Kosina
> Apr 21 05:00:36 localhost kernel: usbcore: registered new interface
> driver usbhid
> Apr 21 05:00:36 localhost kernel: usbhid: USB HID core driver
> Apr 21 05:00:36 localhost kernel: hid-generic 0003:046D:C52E.0001:
> input,hidraw0: USB HID v1.11 Keyboard [Logitech USB Receiver] on
> usb-0000:00:14.0-14/input0
> Apr 21 05:00:36 localhost kernel: hid-generic 0003:046D:C52E.0002:
> input,hiddev0,hidraw1: USB HID v1.11 Mouse [Logitech USB Receiver] on
> usb-0000:00:14.0-14/input1
> 
> [tom@localhost ~]$ journalctl -b -2 | grep hid
> Apr 21 04:59:58 localhost kernel: hidraw: raw HID events driver (C) Jiri Kosina
> Apr 21 04:59:58 localhost kernel: usbcore: registered new interface
> driver usbhid
> Apr 21 04:59:58 localhost kernel: usbhid: USB HID core driver
> Apr 21 04:59:58 localhost kernel: logitech-djreceiver
> 0003:046D:C52B.0003: hiddev0,hidraw0: USB HID v1.11 Device [Logitech
> USB Receiver] on usb-0000:00:14.0-13/input2
> Apr 21 04:59:58 localhost kernel: logitech-hidpp-device
> 0003:046D:4024.0004: input,hidraw1: USB HID v1.11 Keyboard [Logitech
> K400] on usb-0000:00:14.0-13:1
> 
> [tom@localhost ~]$ lsusb -t
> /:  Bus 04.Port 1: Dev 1, Class=root_hub, Driver=xhci_hcd/6p, 5000M
>     |__ Port 1: Dev 2, If 0, Class=Mass Storage, Driver=usb-storage, 5000M
> /:  Bus 03.Port 1: Dev 1, Class=root_hub, Driver=xhci_hcd/14p, 480M
>     |__ Port 13: Dev 2, If 0, Class=Human Interface Device, Driver=usbhid, 12M
>     |__ Port 13: Dev 2, If 1, Class=Human Interface Device, Driver=usbhid, 12M
>     |__ Port 13: Dev 2, If 2, Class=Human Interface Device, Driver=usbhid, 12M
>     |__ Port 14: Dev 3, If 0, Class=Human Interface Device, Driver=usbhid, 12M
>     |__ Port 14: Dev 3, If 1, Class=Human Interface Device, Driver=usbhid, 12M
> /:  Bus 02.Port 1: Dev 1, Class=root_hub, Driver=ehci-pci/2p, 480M
>     |__ Port 1: Dev 2, If 0, Class=Hub, Driver=hub/8p, 480M
> /:  Bus 01.Port 1: Dev 1, Class=root_hub, Driver=ehci-pci/2p, 480M
>     |__ Port 1: Dev 2, If 0, Class=Hub, Driver=hub/6p, 480M

You see this because logitech_djreceiver overrides the setting of the 
ll_driver field in struct hid_device.

Briefly, usbhid/hid-core.c initializes the wakeup setting in 
usbhid_start(), which is the .start member of usb_hid_driver, which is 
assigned (in usbhid_probe()) to hid->ll_driver.

But the logi_dj_recv_add_djhid_device() routine in hid-logitech-dj.c 
changes dj_hiddev->ll_driver to &logi_dj_ll_driver, and its .start 
member is logi_dj_ll_start(), which does not initialize the wakeup 
setting.  Since usbhid_start() never gets called, the device doesn't 
get enabled for wakeup.

See the logi_dj_probe() routine for more details about how this works.

Alan Stern

Re: default value of power/wakeup

[Alan Stern]

On Tue, 21 Apr 2015, Tom Yan wrote:

> By blacklisting the hid-generic, I think I can confirm hid-generic is
> the module that matters. Though I'm curious how it use the code from
> usbhid/hid-core.c because I couldn't find a clue in the short
> hid-generic.c.

In fact, hid-generic.c contains no executable code at all!  What 
matters is that it is a driver and it can bind to a device.  Take a 
look at the hid_device_probe() routine in hid-core.c, and bear in mind 
that hid_hw_start() is an inline routine which calls 
hdev->ll_driver->start().

> Anyway I don't know much about coding so it's not really my concern.
> But I think the fact hid-generic sets a default (no matter if it's
> "enabled" or "disabled") brings a problem. The reason is, the wakeup
> attribute is not "initialized" or "created" by hid-generic, so udev
> can apply a rule related to the attribute before hid-generic get
> loaded (which seems to be what it is doing, see
> https://bugs.freedesktop.org/show_bug.cgi?id=90041). If the module
> gets loaded or reloaded afterwards, the value would be set to the
> hard-coded default anyway.

This is a similar problem affecting all devices.  The wakeup attribute
in sysfs (along with all the other power-related sysfs attributes) gets
created before the device is probed.  Consequently, any changes made by
drivers in their probe routines don't show up in the initial sysfs 
files.

> Perhaps it is possible to change the behaviour by rewriting some bits
> of udev, but I don't see the point of doing so. IMHO these hard-coded
> default should be avoid from all drivers until it is really necessary,
> and when it is needed, it's best to have the default set right at the
> point when the attribute is initialized if possible.

Setting the default when the attribute gets initialized is not possible
in this case.  The wakeup attribute belongs to the USB device, but the
fact that the device can act as a keyboard is hidden away in one of the
interfaces (and the interfaces are registered _after_ the device).  
Things would be a little better if the wakeup attribute were a property
of the interface, but it isn't.

Anyway, you're suggesting that drivers should never override sysfs
attribute values.  But there doesn't seem to be any other way to
implement the kernel's policy that wakeup should be enabled by default
for all keyboard devices.  After all, only the driver knows whether or
not the device it manages is a keyboard.

Alan Stern

Re: default value of power/wakeup

[Tom Yan]

On 21 April 2015 at 23:51, Alan Stern <stern-nwvwT67g6+6dFdvTe/nMLpVzexx5G7lz@public.gmane.org> wrote:
> Anyway, you're suggesting that drivers should never override sysfs
> attribute values.  But there doesn't seem to be any other way to
> implement the kernel's policy that wakeup should be enabled by default
> for all keyboard devices.

I just doubt if there should be any of these kinds of kernel's policy,
especially for non-critical attributes like wakeup. Obviously now udev
is put to a very embarassed position (supposedly it should be the one
managing these policy, but now the fact is the kernel took its job
from it). Also, from the case of my two receivers, we can see that it
also causes unnecessary inconsistency to user experience.

To me it's more of "driver's policy" than the kernel's. If it's not
trying to make people with same hardware capibilities share the same
experience on the same attributes, what's the meaning of these
policies then? Yes of course there might be a (great) chance that it
might make (many) people with certain hardware feel happier, but
objectively does it mean anything? Not to mention that not everyone
likes the policy. (Can anyone even confirm that the majority likes
wakeup to be enabled for keyboards by default?)

IMHO it would be best that any general policies considered important
to be off-loaded to udev (as a udev rule?). Only when there's no
better way (like "communicate directly" with udev?) for the driver to
set necessary specific policies for itself, it goes back to this
not-so-good method.

>  After all, only the driver knows whether or not the device it manages is a keyboard.

I am not sure that I understand what does this mean practically to this issue.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: default value of power/wakeup

[Alan Stern]

On Wed, 22 Apr 2015, Tom Yan wrote:

> On 21 April 2015 at 23:51, Alan Stern <stern-nwvwT67g6+6dFdvTe/nMLpVzexx5G7lz@public.gmane.org> wrote:
> > Anyway, you're suggesting that drivers should never override sysfs
> > attribute values.  But there doesn't seem to be any other way to
> > implement the kernel's policy that wakeup should be enabled by default
> > for all keyboard devices.
> 
> I just doubt if there should be any of these kinds of kernel's policy,

The kernel _has_ to have some policy about default values.  It's 
unavoidable -- even if you say that the default value for everything 
will be 0, that's still a policy!

> especially for non-critical attributes like wakeup. Obviously now udev
> is put to a very embarassed position (supposedly it should be the one
> managing these policy, but now the fact is the kernel took its job
> from it). Also, from the case of my two receivers, we can see that it
> also causes unnecessary inconsistency to user experience.

The inconsistency is a bug.  Not all keyboard drivers have implemented
the wakeup policy.  That can be fixed.  Try applying the patch below 
and let me know what happens.

Overriding udev is unfortunate and we should try to avoid it.  But at
the moment, I can't see any way to avoid it without making a lot of
users unhappy (because their keyboards will no longer automatically be
enabled for wakeup).

> To me it's more of "driver's policy" than the kernel's.

What's the difference?  Drivers are part of the kernel, after all.

>  If it's not
> trying to make people with same hardware capibilities share the same
> experience on the same attributes, what's the meaning of these
> policies then? Yes of course there might be a (great) chance that it
> might make (many) people with certain hardware feel happier, but
> objectively does it mean anything? Not to mention that not everyone
> likes the policy. (Can anyone even confirm that the majority likes
> wakeup to be enabled for keyboards by default?)

If the kernel developers never did anything until they had first 
checked to see that a majority of users were in favor, they would never 
do anything at all.

Do you think Microsoft checked with all their users before introducing
Windows Vista or Windows 8?  Obviously Linux is not like Windows, for 
many reasons, but in some respects their developments are similar.

> IMHO it would be best that any general policies considered important
> to be off-loaded to udev (as a udev rule?). Only when there's no
> better way (like "communicate directly" with udev?) for the driver to
> set necessary specific policies for itself, it goes back to this
> not-so-good method.

If we were to change the policy now, it would be a regression because
it would make lots of keyboards stop being wakeup-enabled by default.  
Kernel developers are not allowed to cause regressions except in a few
rare cases (such as those involving security problems).

> >  After all, only the driver knows whether or not the device it
> > manages is a keyboard.
> 
> I am not sure that I understand what does this mean practically to this issue.

It means that the wakeup setting cannot be initialized correctly when
the sysfs attribute is created, because the attribute is created before
the kernel knows that the device is a keyboard (since only the driver
knows this, and the driver doesn't get bound to the device until after
the attribute is created).

Alan Stern



Index: usb-4.0/drivers/hid/hid-logitech-dj.c
===================================================================
--- usb-4.0.orig/drivers/hid/hid-logitech-dj.c
+++ usb-4.0/drivers/hid/hid-logitech-dj.c
@@ -990,6 +990,7 @@ static int logi_dj_probe(struct hid_devi
 			 const struct hid_device_id *id)
 {
 	struct usb_interface *intf = to_usb_interface(hdev->dev.parent);
+	struct usb_device *udev = interface_to_usbdev(intf);
 	struct dj_receiver_dev *djrcv_dev;
 	int retval;
 
@@ -1078,6 +1079,9 @@ static int logi_dj_probe(struct hid_devi
 		goto logi_dj_recv_query_paired_devices_failed;
 	}
 
+	/* Keyboards are enabled for wakeup by default */
+	device_set_wakeup_enable(&udev->dev, 1);
+
 	return retval;
 
 logi_dj_recv_query_paired_devices_failed:

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: default value of power/wakeup

[Greg KH]

On Wed, Apr 22, 2015 at 11:28:29AM -0400, Alan Stern wrote:
> On Wed, 22 Apr 2015, Tom Yan wrote:
> 
> > On 21 April 2015 at 23:51, Alan Stern <stern@rowland.harvard.edu> wrote:
> > > Anyway, you're suggesting that drivers should never override sysfs
> > > attribute values.  But there doesn't seem to be any other way to
> > > implement the kernel's policy that wakeup should be enabled by default
> > > for all keyboard devices.
> > 
> > I just doubt if there should be any of these kinds of kernel's policy,
> 
> The kernel _has_ to have some policy about default values.  It's 
> unavoidable -- even if you say that the default value for everything 
> will be 0, that's still a policy!
> 
> > especially for non-critical attributes like wakeup. Obviously now udev
> > is put to a very embarassed position (supposedly it should be the one
> > managing these policy, but now the fact is the kernel took its job
> > from it). Also, from the case of my two receivers, we can see that it
> > also causes unnecessary inconsistency to user experience.
> 
> The inconsistency is a bug.  Not all keyboard drivers have implemented
> the wakeup policy.  That can be fixed.  Try applying the patch below 
> and let me know what happens.
> 
> Overriding udev is unfortunate and we should try to avoid it.  But at
> the moment, I can't see any way to avoid it without making a lot of
> users unhappy (because their keyboards will no longer automatically be
> enabled for wakeup).
> 
> > To me it's more of "driver's policy" than the kernel's.
> 
> What's the difference?  Drivers are part of the kernel, after all.
> 
> >  If it's not
> > trying to make people with same hardware capibilities share the same
> > experience on the same attributes, what's the meaning of these
> > policies then? Yes of course there might be a (great) chance that it
> > might make (many) people with certain hardware feel happier, but
> > objectively does it mean anything? Not to mention that not everyone
> > likes the policy. (Can anyone even confirm that the majority likes
> > wakeup to be enabled for keyboards by default?)
> 
> If the kernel developers never did anything until they had first 
> checked to see that a majority of users were in favor, they would never 
> do anything at all.
> 
> Do you think Microsoft checked with all their users before introducing
> Windows Vista or Windows 8?  Obviously Linux is not like Windows, for 
> many reasons, but in some respects their developments are similar.
> 
> > IMHO it would be best that any general policies considered important
> > to be off-loaded to udev (as a udev rule?). Only when there's no
> > better way (like "communicate directly" with udev?) for the driver to
> > set necessary specific policies for itself, it goes back to this
> > not-so-good method.
> 
> If we were to change the policy now, it would be a regression because
> it would make lots of keyboards stop being wakeup-enabled by default.  
> Kernel developers are not allowed to cause regressions except in a few
> rare cases (such as those involving security problems).
> 
> > >  After all, only the driver knows whether or not the device it
> > > manages is a keyboard.
> > 
> > I am not sure that I understand what does this mean practically to this issue.
> 
> It means that the wakeup setting cannot be initialized correctly when
> the sysfs attribute is created, because the attribute is created before
> the kernel knows that the device is a keyboard (since only the driver
> knows this, and the driver doesn't get bound to the device until after
> the attribute is created).
> 
> Alan Stern
> 
> 
> 
> Index: usb-4.0/drivers/hid/hid-logitech-dj.c
> ===================================================================
> --- usb-4.0.orig/drivers/hid/hid-logitech-dj.c
> +++ usb-4.0/drivers/hid/hid-logitech-dj.c
> @@ -990,6 +990,7 @@ static int logi_dj_probe(struct hid_devi
>  			 const struct hid_device_id *id)
>  {
>  	struct usb_interface *intf = to_usb_interface(hdev->dev.parent);
> +	struct usb_device *udev = interface_to_usbdev(intf);
>  	struct dj_receiver_dev *djrcv_dev;
>  	int retval;
>  
> @@ -1078,6 +1079,9 @@ static int logi_dj_probe(struct hid_devi
>  		goto logi_dj_recv_query_paired_devices_failed;
>  	}
>  
> +	/* Keyboards are enabled for wakeup by default */
> +	device_set_wakeup_enable(&udev->dev, 1);

But this device isn't always a keyboard.  For example, mine works with a
mouse.  It's a "universal receiver", you can't know what type of HID
device is plugged into it until it connects to it.  I don't mind making
it auto wakeup, if that works, but the comment isn't correct.

thanks,

greg k-h

Re: default value of power/wakeup

[Alan Stern]

On Wed, 22 Apr 2015, Greg KH wrote:

> > Index: usb-4.0/drivers/hid/hid-logitech-dj.c
> > ===================================================================
> > --- usb-4.0.orig/drivers/hid/hid-logitech-dj.c
> > +++ usb-4.0/drivers/hid/hid-logitech-dj.c
> > @@ -990,6 +990,7 @@ static int logi_dj_probe(struct hid_devi
> >  			 const struct hid_device_id *id)
> >  {
> >  	struct usb_interface *intf = to_usb_interface(hdev->dev.parent);
> > +	struct usb_device *udev = interface_to_usbdev(intf);
> >  	struct dj_receiver_dev *djrcv_dev;
> >  	int retval;
> >  
> > @@ -1078,6 +1079,9 @@ static int logi_dj_probe(struct hid_devi
> >  		goto logi_dj_recv_query_paired_devices_failed;
> >  	}
> >  
> > +	/* Keyboards are enabled for wakeup by default */
> > +	device_set_wakeup_enable(&udev->dev, 1);
> 
> But this device isn't always a keyboard.  For example, mine works with a
> mouse.  It's a "universal receiver", you can't know what type of HID
> device is plugged into it until it connects to it.  I don't mind making
> it auto wakeup, if that works, but the comment isn't correct.

Oh, okay, I didn't realize that.

Is there a reasonable way to enable wakeup only when the driver learns
that a keyboard is connected?  Where would the driver do this?

Alan Stern

Re: default value of power/wakeup

[Greg KH]

On Wed, Apr 22, 2015 at 12:15:36PM -0400, Alan Stern wrote:
> On Wed, 22 Apr 2015, Greg KH wrote:
> 
> > > Index: usb-4.0/drivers/hid/hid-logitech-dj.c
> > > ===================================================================
> > > --- usb-4.0.orig/drivers/hid/hid-logitech-dj.c
> > > +++ usb-4.0/drivers/hid/hid-logitech-dj.c
> > > @@ -990,6 +990,7 @@ static int logi_dj_probe(struct hid_devi
> > >  			 const struct hid_device_id *id)
> > >  {
> > >  	struct usb_interface *intf = to_usb_interface(hdev->dev.parent);
> > > +	struct usb_device *udev = interface_to_usbdev(intf);
> > >  	struct dj_receiver_dev *djrcv_dev;
> > >  	int retval;
> > >  
> > > @@ -1078,6 +1079,9 @@ static int logi_dj_probe(struct hid_devi
> > >  		goto logi_dj_recv_query_paired_devices_failed;
> > >  	}
> > >  
> > > +	/* Keyboards are enabled for wakeup by default */
> > > +	device_set_wakeup_enable(&udev->dev, 1);
> > 
> > But this device isn't always a keyboard.  For example, mine works with a
> > mouse.  It's a "universal receiver", you can't know what type of HID
> > device is plugged into it until it connects to it.  I don't mind making
> > it auto wakeup, if that works, but the comment isn't correct.
> 
> Oh, okay, I didn't realize that.
> 
> Is there a reasonable way to enable wakeup only when the driver learns
> that a keyboard is connected?  Where would the driver do this?

I don't know if the driver ever "knows" this, as you can pair lots of
different devices to this same receiver.  There's a userspace
application that lets you manage the device, called "solaar", that this
option could be changed in.

But really, putting the device to sleep should work for it no matter if
this is a keyboard or a mouse or a joystick, as the wakeup logic is in
the receiver, not in the device on the other end of the wireless link.

Turning autosuspend works for me for my mouse connected.  It doesn't
work for one of my "real" USB keyboards when it's connected to the
machine, which is why I can't enable autosuspend for it, as it drives me
crazy.

I don't have a keyboard to test the receiver with at the moment, to see
if autosuspend works for both things connected at the same time, or for
just the keyboard.

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: default value of power/wakeup

[Alan Stern]

On Wed, 22 Apr 2015, Greg KH wrote:

> > > But this device isn't always a keyboard.  For example, mine works with a
> > > mouse.  It's a "universal receiver", you can't know what type of HID
> > > device is plugged into it until it connects to it.  I don't mind making
> > > it auto wakeup, if that works, but the comment isn't correct.
> > 
> > Oh, okay, I didn't realize that.
> > 
> > Is there a reasonable way to enable wakeup only when the driver learns
> > that a keyboard is connected?  Where would the driver do this?
> 
> I don't know if the driver ever "knows" this, as you can pair lots of
> different devices to this same receiver.  There's a userspace
> application that lets you manage the device, called "solaar", that this
> option could be changed in.
> 
> But really, putting the device to sleep should work for it no matter if
> this is a keyboard or a mouse or a joystick, as the wakeup logic is in
> the receiver, not in the device on the other end of the wireless link.
> 
> Turning autosuspend works for me for my mouse connected.  It doesn't
> work for one of my "real" USB keyboards when it's connected to the
> machine, which is why I can't enable autosuspend for it, as it drives me
> crazy.
> 
> I don't have a keyboard to test the receiver with at the moment, to see
> if autosuspend works for both things connected at the same time, or for
> just the keyboard.

Tom and I have been talking about enabling wakeup, not autosuspend.  
The question is whether or not the default wakeup setting for the 
receiver should be "enabled".

The kernel's policy is that keyboards should be enabled for wakeup, by 
default.  I think that matches most people's expectations.  But when 
you've got a "universal" receiver, what then?

Should it always be enabled by default because a keyboard _might_ be
connected?  Should it be enabled only when a keyboard is detected?  
What if multiple devices are connected at the same time?

Shucks -- does the receiver even _work_ as a wakeup device?  It claims
to, but that would require it to remain in wireless contact with the
remote keyboard even while it's supposed to be in a low-power state,
which rather seems to defeat the purpose.

Alan Stern

PS: I've got wakeup enabled for the PS/2 keyboard attached to the
computer I'm using now, but it doesn't work.  I have to press the power 
button to wake the machine up from suspend.  Probably an issue in the 
BIOS or ACPI -- I haven't bothered to try and track it down.

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: default value of power/wakeup

[Greg KH]

On Wed, Apr 22, 2015 at 02:22:44PM -0400, Alan Stern wrote:
> On Wed, 22 Apr 2015, Greg KH wrote:
> 
> > > > But this device isn't always a keyboard.  For example, mine works with a
> > > > mouse.  It's a "universal receiver", you can't know what type of HID
> > > > device is plugged into it until it connects to it.  I don't mind making
> > > > it auto wakeup, if that works, but the comment isn't correct.
> > > 
> > > Oh, okay, I didn't realize that.
> > > 
> > > Is there a reasonable way to enable wakeup only when the driver learns
> > > that a keyboard is connected?  Where would the driver do this?
> > 
> > I don't know if the driver ever "knows" this, as you can pair lots of
> > different devices to this same receiver.  There's a userspace
> > application that lets you manage the device, called "solaar", that this
> > option could be changed in.
> > 
> > But really, putting the device to sleep should work for it no matter if
> > this is a keyboard or a mouse or a joystick, as the wakeup logic is in
> > the receiver, not in the device on the other end of the wireless link.
> > 
> > Turning autosuspend works for me for my mouse connected.  It doesn't
> > work for one of my "real" USB keyboards when it's connected to the
> > machine, which is why I can't enable autosuspend for it, as it drives me
> > crazy.
> > 
> > I don't have a keyboard to test the receiver with at the moment, to see
> > if autosuspend works for both things connected at the same time, or for
> > just the keyboard.
> 
> Tom and I have been talking about enabling wakeup, not autosuspend.  
> The question is whether or not the default wakeup setting for the 
> receiver should be "enabled".

Oh nevermind, sorry about that.

> The kernel's policy is that keyboards should be enabled for wakeup, by 
> default.  I think that matches most people's expectations.  But when 
> you've got a "universal" receiver, what then?
> 
> Should it always be enabled by default because a keyboard _might_ be
> connected?  Should it be enabled only when a keyboard is detected?  
> What if multiple devices are connected at the same time?
> 
> Shucks -- does the receiver even _work_ as a wakeup device?  It claims
> to, but that would require it to remain in wireless contact with the
> remote keyboard even while it's supposed to be in a low-power state,
> which rather seems to defeat the purpose.

I just tried it with my mouse connected, it didn't wake it up.  Couldn't
tell if it stayed connected to the mouse or not.

> PS: I've got wakeup enabled for the PS/2 keyboard attached to the
> computer I'm using now, but it doesn't work.  I have to press the power 
> button to wake the machine up from suspend.  Probably an issue in the 
> BIOS or ACPI -- I haven't bothered to try and track it down.

Same here for my USB keyboard, it doesn't wake the machine up either...

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: default value of power/wakeup

[Peter Chen]

 
> > >
> > > Oh, okay, I didn't realize that.
> > >
> > > Is there a reasonable way to enable wakeup only when the driver
> > > learns that a keyboard is connected?  Where would the driver do this?
> >
> > I don't know if the driver ever "knows" this, as you can pair lots of
> > different devices to this same receiver.  There's a userspace
> > application that lets you manage the device, called "solaar", that
> > this option could be changed in.
> >
> > But really, putting the device to sleep should work for it no matter
> > if this is a keyboard or a mouse or a joystick, as the wakeup logic is
> > in the receiver, not in the device on the other end of the wireless link.
> >
> > Turning autosuspend works for me for my mouse connected.  It doesn't
> > work for one of my "real" USB keyboards when it's connected to the
> > machine, which is why I can't enable autosuspend for it, as it drives
> > me crazy.
> >
> > I don't have a keyboard to test the receiver with at the moment, to
> > see if autosuspend works for both things connected at the same time,
> > or for just the keyboard.
> 
> Tom and I have been talking about enabling wakeup, not autosuspend.
> The question is whether or not the default wakeup setting for the receiver
> should be "enabled".
> 
> The kernel's policy is that keyboards should be enabled for wakeup, by default.
> I think that matches most people's expectations.  But when you've got a
> "universal" receiver, what then?
> 
> Should it always be enabled by default because a keyboard _might_ be
> connected?  Should it be enabled only when a keyboard is detected?
> What if multiple devices are connected at the same time?
> 

>From my point, the user option should not depend on kernel default value.
If the system you build needs some USB devices as system wakeup source,
the developers need to make sure the wakeups are enabled before system
enters suspend.

Peter 

> Shucks -- does the receiver even _work_ as a wakeup device?  It claims to, but
> that would require it to remain in wireless contact with the remote keyboard
> even while it's supposed to be in a low-power state, which rather seems to
> defeat the purpose.
> 
> Alan Stern
> 
> PS: I've got wakeup enabled for the PS/2 keyboard attached to the computer
> I'm using now, but it doesn't work.  I have to press the power button to wake
> the machine up from suspend.  Probably an issue in the BIOS or ACPI -- I
> haven't bothered to try and track it down.
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body
> of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at
> http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: default value of power/wakeup

[Tom Yan]

I'm not saying that the kernel shouldn't initialize the attributes or
have a default. But it should only set the default when the attribute
is initialized (It doesn't even matter to me whether it's enabled or
disabled).

It's just there should not be further manipulation from the kernel
(e.g. device_set_wakeup_enable) afterwards because
1. it's brings inconsistency because the function is adopted per driver
2. it's a user preference and responsibilty
3. third it prevent udev to apply a rule properly (regression / bug)

P.S. Alan for my case, I don't need a patch for logitech-dj, I just
need to remove device_set_enable_wakeup from hid_core.c, then I can
enable or disable the attribute with a udev rule happily for both
devices.

▶ Show quoted text
On 23 April 2015 at 09:21, Peter Chen <Peter.Chen@freescale.com> wrote:
>
>> > >
>> > > Oh, okay, I didn't realize that.
>> > >
>> > > Is there a reasonable way to enable wakeup only when the driver
>> > > learns that a keyboard is connected?  Where would the driver do this?
>> >
>> > I don't know if the driver ever "knows" this, as you can pair lots of
>> > different devices to this same receiver.  There's a userspace
>> > application that lets you manage the device, called "solaar", that
>> > this option could be changed in.
>> >
>> > But really, putting the device to sleep should work for it no matter
>> > if this is a keyboard or a mouse or a joystick, as the wakeup logic is
>> > in the receiver, not in the device on the other end of the wireless link.
>> >
>> > Turning autosuspend works for me for my mouse connected.  It doesn't
>> > work for one of my "real" USB keyboards when it's connected to the
>> > machine, which is why I can't enable autosuspend for it, as it drives
>> > me crazy.
>> >
>> > I don't have a keyboard to test the receiver with at the moment, to
>> > see if autosuspend works for both things connected at the same time,
>> > or for just the keyboard.
>>
>> Tom and I have been talking about enabling wakeup, not autosuspend.
>> The question is whether or not the default wakeup setting for the receiver
>> should be "enabled".
>>
>> The kernel's policy is that keyboards should be enabled for wakeup, by default.
>> I think that matches most people's expectations.  But when you've got a
>> "universal" receiver, what then?
>>
>> Should it always be enabled by default because a keyboard _might_ be
>> connected?  Should it be enabled only when a keyboard is detected?
>> What if multiple devices are connected at the same time?
>>
>
> From my point, the user option should not depend on kernel default value.
> If the system you build needs some USB devices as system wakeup source,
> the developers need to make sure the wakeups are enabled before system
> enters suspend.
>
> Peter
>
>> Shucks -- does the receiver even _work_ as a wakeup device?  It claims to, but
>> that would require it to remain in wireless contact with the remote keyboard
>> even while it's supposed to be in a low-power state, which rather seems to
>> defeat the purpose.
>>
>> Alan Stern
>>
>> PS: I've got wakeup enabled for the PS/2 keyboard attached to the computer
>> I'm using now, but it doesn't work.  I have to press the power button to wake
>> the machine up from suspend.  Probably an issue in the BIOS or ACPI -- I
>> haven't bothered to try and track it down.
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body
>> of a message to majordomo@vger.kernel.org More majordomo info at
>> http://vger.kernel.org/majordomo-info.html

RE: default value of power/wakeup

[Peter Chen]

 
> 
> I'm not saying that the kernel shouldn't initialize the attributes or have a default.
> But it should only set the default when the attribute is initialized (It doesn't
> even matter to me whether it's enabled or disabled).
> 
> It's just there should not be further manipulation from the kernel (e.g.
> device_set_wakeup_enable) afterwards because 1. it's brings inconsistency
> because the function is adopted per driver 2. it's a user preference and
> responsibilty 3. third it prevent udev to apply a rule properly (regression / bug)
> 
> P.S. Alan for my case, I don't need a patch for logitech-dj, I just need to remove
> device_set_enable_wakeup from hid_core.c, then I can enable or disable the
> attribute with a udev rule happily for both devices.
> 

You may apply your choice no matter what the default value is, would you tell us
why you can't do that?

Peter

> ▶ Show quoted text
> On 23 April 2015 at 09:21, Peter Chen <Peter.Chen@freescale.com> wrote:
> >
> >> > >
> >> > > Oh, okay, I didn't realize that.
> >> > >
> >> > > Is there a reasonable way to enable wakeup only when the driver
> >> > > learns that a keyboard is connected?  Where would the driver do this?
> >> >
> >> > I don't know if the driver ever "knows" this, as you can pair lots
> >> > of different devices to this same receiver.  There's a userspace
> >> > application that lets you manage the device, called "solaar", that
> >> > this option could be changed in.
> >> >
> >> > But really, putting the device to sleep should work for it no
> >> > matter if this is a keyboard or a mouse or a joystick, as the
> >> > wakeup logic is in the receiver, not in the device on the other end of the
> wireless link.
> >> >
> >> > Turning autosuspend works for me for my mouse connected.  It
> >> > doesn't work for one of my "real" USB keyboards when it's connected
> >> > to the machine, which is why I can't enable autosuspend for it, as
> >> > it drives me crazy.
> >> >
> >> > I don't have a keyboard to test the receiver with at the moment, to
> >> > see if autosuspend works for both things connected at the same
> >> > time, or for just the keyboard.
> >>
> >> Tom and I have been talking about enabling wakeup, not autosuspend.
> >> The question is whether or not the default wakeup setting for the
> >> receiver should be "enabled".
> >>
> >> The kernel's policy is that keyboards should be enabled for wakeup, by
> default.
> >> I think that matches most people's expectations.  But when you've got
> >> a "universal" receiver, what then?
> >>
> >> Should it always be enabled by default because a keyboard _might_ be
> >> connected?  Should it be enabled only when a keyboard is detected?
> >> What if multiple devices are connected at the same time?
> >>
> >
> > From my point, the user option should not depend on kernel default value.
> > If the system you build needs some USB devices as system wakeup
> > source, the developers need to make sure the wakeups are enabled
> > before system enters suspend.
> >
> > Peter
> >
> >> Shucks -- does the receiver even _work_ as a wakeup device?  It
> >> claims to, but that would require it to remain in wireless contact
> >> with the remote keyboard even while it's supposed to be in a
> >> low-power state, which rather seems to defeat the purpose.
> >>
> >> Alan Stern
> >>
> >> PS: I've got wakeup enabled for the PS/2 keyboard attached to the
> >> computer I'm using now, but it doesn't work.  I have to press the
> >> power button to wake the machine up from suspend.  Probably an issue
> >> in the BIOS or ACPI -- I haven't bothered to try and track it down.
> >>
> >> --
> >> To unsubscribe from this list: send the line "unsubscribe linux-usb"
> >> in the body of a message to majordomo@vger.kernel.org More majordomo
> >> info at http://vger.kernel.org/majordomo-info.html

Re: default value of power/wakeup

[Tom Yan]

https://bugs.freedesktop.org/show_bug.cgi?id=90041
Because udev's work would get overrided if there's further
manipulation. So if you want your udev rule to work, you have to make
sure you run a trigger command after the module is loaded or reloaded.
This would happen for all types of devices if their driver make sure
of function like device_set_wakeup_enable.

P.S. Though there could be exceptions if the module is loaded in early
user space. Then the rule will only get overrided if you somehow need
to reload the module.

On 23 April 2015 at 13:22, Peter Chen <Peter.Chen@freescale.com> wrote:
>
>>
>> I'm not saying that the kernel shouldn't initialize the attributes or have a default.
>> But it should only set the default when the attribute is initialized (It doesn't
>> even matter to me whether it's enabled or disabled).
>>
>> It's just there should not be further manipulation from the kernel (e.g.
>> device_set_wakeup_enable) afterwards because 1. it's brings inconsistency
>> because the function is adopted per driver 2. it's a user preference and
>> responsibilty 3. third it prevent udev to apply a rule properly (regression / bug)
>>
>> P.S. Alan for my case, I don't need a patch for logitech-dj, I just need to remove
>> device_set_enable_wakeup from hid_core.c, then I can enable or disable the
>> attribute with a udev rule happily for both devices.
>>
>
> You may apply your choice no matter what the default value is, would you tell us
> why you can't do that?
>
> Peter
>
>> ▶ Show quoted text
>> On 23 April 2015 at 09:21, Peter Chen <Peter.Chen@freescale.com> wrote:
>> >
>> >> > >
>> >> > > Oh, okay, I didn't realize that.
>> >> > >
>> >> > > Is there a reasonable way to enable wakeup only when the driver
>> >> > > learns that a keyboard is connected?  Where would the driver do this?
>> >> >
>> >> > I don't know if the driver ever "knows" this, as you can pair lots
>> >> > of different devices to this same receiver.  There's a userspace
>> >> > application that lets you manage the device, called "solaar", that
>> >> > this option could be changed in.
>> >> >
>> >> > But really, putting the device to sleep should work for it no
>> >> > matter if this is a keyboard or a mouse or a joystick, as the
>> >> > wakeup logic is in the receiver, not in the device on the other end of the
>> wireless link.
>> >> >
>> >> > Turning autosuspend works for me for my mouse connected.  It
>> >> > doesn't work for one of my "real" USB keyboards when it's connected
>> >> > to the machine, which is why I can't enable autosuspend for it, as
>> >> > it drives me crazy.
>> >> >
>> >> > I don't have a keyboard to test the receiver with at the moment, to
>> >> > see if autosuspend works for both things connected at the same
>> >> > time, or for just the keyboard.
>> >>
>> >> Tom and I have been talking about enabling wakeup, not autosuspend.
>> >> The question is whether or not the default wakeup setting for the
>> >> receiver should be "enabled".
>> >>
>> >> The kernel's policy is that keyboards should be enabled for wakeup, by
>> default.
>> >> I think that matches most people's expectations.  But when you've got
>> >> a "universal" receiver, what then?
>> >>
>> >> Should it always be enabled by default because a keyboard _might_ be
>> >> connected?  Should it be enabled only when a keyboard is detected?
>> >> What if multiple devices are connected at the same time?
>> >>
>> >
>> > From my point, the user option should not depend on kernel default value.
>> > If the system you build needs some USB devices as system wakeup
>> > source, the developers need to make sure the wakeups are enabled
>> > before system enters suspend.
>> >
>> > Peter
>> >
>> >> Shucks -- does the receiver even _work_ as a wakeup device?  It
>> >> claims to, but that would require it to remain in wireless contact
>> >> with the remote keyboard even while it's supposed to be in a
>> >> low-power state, which rather seems to defeat the purpose.
>> >>
>> >> Alan Stern
>> >>
>> >> PS: I've got wakeup enabled for the PS/2 keyboard attached to the
>> >> computer I'm using now, but it doesn't work.  I have to press the
>> >> power button to wake the machine up from suspend.  Probably an issue
>> >> in the BIOS or ACPI -- I haven't bothered to try and track it down.
>> >>
>> >> --
>> >> To unsubscribe from this list: send the line "unsubscribe linux-usb"
>> >> in the body of a message to majordomo@vger.kernel.org More majordomo
>> >> info at http://vger.kernel.org/majordomo-info.html

Re: default value of power/wakeup

[Alan Stern]

On Thu, 23 Apr 2015, Tom Yan wrote:

> I'm not saying that the kernel shouldn't initialize the attributes or
> have a default. But it should only set the default when the attribute
> is initialized (It doesn't even matter to me whether it's enabled or
> disabled).
> 
> It's just there should not be further manipulation from the kernel
> (e.g. device_set_wakeup_enable) afterwards because
> 1. it's brings inconsistency because the function is adopted per driver
> 2. it's a user preference and responsibilty
> 3. third it prevent udev to apply a rule properly (regression / bug)
> 
> P.S. Alan for my case, I don't need a patch for logitech-dj, I just
> need to remove device_set_enable_wakeup from hid_core.c, then I can
> enable or disable the attribute with a udev rule happily for both
> devices.

I understand that.  But removing device_set_enable_wakeup from
hid_core.c would annoy a lot of people.  Linus would probably decide
that it was a regression and would put the function call back in.

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: default value of power/wakeup

[Tom Yan]

I know. Honestly I never expect it to be fixed. I just want to draw
some attention so that people can be aware of this problem (some may
be suffering from this without understand what's going on). Though I
did  filed a bug report on bugzilla some time ago to request making
this function a no-op (sounds outrageous doesn't it). Hope that
something will be done for this one day anyway.

On 23 April 2015 at 22:40, Alan Stern <stern@rowland.harvard.edu> wrote:
> On Thu, 23 Apr 2015, Tom Yan wrote:
>
>> I'm not saying that the kernel shouldn't initialize the attributes or
>> have a default. But it should only set the default when the attribute
>> is initialized (It doesn't even matter to me whether it's enabled or
>> disabled).
>>
>> It's just there should not be further manipulation from the kernel
>> (e.g. device_set_wakeup_enable) afterwards because
>> 1. it's brings inconsistency because the function is adopted per driver
>> 2. it's a user preference and responsibilty
>> 3. third it prevent udev to apply a rule properly (regression / bug)
>>
>> P.S. Alan for my case, I don't need a patch for logitech-dj, I just
>> need to remove device_set_enable_wakeup from hid_core.c, then I can
>> enable or disable the attribute with a udev rule happily for both
>> devices.
>
> I understand that.  But removing device_set_enable_wakeup from
> hid_core.c would annoy a lot of people.  Linus would probably decide
> that it was a regression and would put the function call back in.
>
> Alan Stern
>