USB auto-suspend: exclude HID devices

[pohly]

The requested change to enable auto-suspend for all USB devices had the expected negative effect with at least one mouse (reported by @jlaako): when not moving the mouse constantly, it stops working when suspend kicks in and one has to press a (mouse?) button to make it work again.

We should update the udev rule once more such that it detects HID devices and never enables suspend for them.

[pohly]

@jlaako tried some changes (PR #61, PR #60). I'll also look into it soonish.

[pohly]

Here's a solution that worked for me. I tested with a keyboard that has a builtin hub and a mouse attached to that hub. With this change, both keyboard and mouse have auto-suspend off. The hub has auto-suspend on. No idea whether that is the correct configuration in this case.

diff --git a/meta-ostro-xt/recipes-bsp/joule-udev-rules/files/usb-power.rules b/meta-ostro-xt/recipes-bsp/joule-udev-rules/files/usb-power.rules
index 8186011..6374ddd 100644
--- a/meta-ostro-xt/recipes-bsp/joule-udev-rules/files/usb-power.rules
+++ b/meta-ostro-xt/recipes-bsp/joule-udev-rules/files/usb-power.rules
@@ -6,7 +6,7 @@
 # users).

 # Check whether device is blacklisted.
-ACTION=="add", SUBSYSTEMS=="usb", PROGRAM="/bin/sh -c 'grep -q ^$attr{idVendor}:$attr{idProduct} /lib/udev/usb-autosuspend-blacklist || ( [ -e /etc/udev/usb-autosuspend-blacklist ] && grep -q $attr{idVendor}:$attr{idProduct} /etc/udev/usb-autosuspend-blacklist)'", GOTO="power_usb_rules_end"
+ACTION=="add", SUBSYSTEMS=="usb", PROGRAM="/bin/sh -c 'grep -q -w 03 $sys/$devpath/*/bInterfaceClass || grep -q ^$attr{idVendor}:$attr{idProduct} /lib/udev/usb-autosuspend-blacklist || ( [ -e /etc/udev/usb-autosuspend-blacklist ] && grep -q $attr{idVendor}:$attr{idProduct} /etc/udev/usb-autosuspend-blacklist)'", GOTO="power_usb_rules_end"

 # Enable autosuspend.
 ACTION=="add", SUBSYSTEMS=="usb", TEST=="power/control", ATTR{power/control}="auto"

@mythi please let me know whether you want a PR with this change.

I briefly tried @jlaako's approach from PR #61, but it did not become clear to me how ENV{INTERFACE}!="3/*" was meant to work (is INTERFACE a "device property value"? There's more than one for each device.) and therefore I wasn't able to do it that way (would have been nicer, IMHO).

[jlaako]

@pohly as visible from the rule, INTERFACE is udev environment variable, not a property value. If you list environments with udevadm (monitor) you can see it. It contains three slash separated values. The first value is device class. You can check other rules that use ENV{} if you are not familiar with it.

[pohly]

@jlaako did you mean device class or interface class? We need to test for the interface class(es).

According to "man udev", ENV refers to property values:

ENV{key}
           Match against a device property value.

Is that what you meant with "udev environment variable"?

I did not seeing anything about INTERFACE in "udevadm test" for the device. I do see it now in "udev monitor -p", but it is still unclear to me what the value is. If a fictional USB device has 10 endpoints implementing different interfaces, then what's in the INTERFACE variable? Is that documented somewhere? I'm not seeing it used anywhere else in /lib/udev/rules.d.

But that aside, how do you want to proceed? Do you want to take another stab at getting it working based on ENV{INTERFACE}, or use the shell-based approach?

[jlaako]

@pohly as usual, pretty much nothing is documented anywhere.

My (failed) attempt was based on what I found on mailing list discussions related to device-class specific rules.

I personally find it extremely ugly to have to call external utilities like "grep" from within the rules to achieve simple task of having a device class specific udev rule. IMO, it should be possible to say it straight in the single autosuspend rule in a simple way. But maybe udev is not up to the task it was designed to handle. As a next step I would personally have just written a simple little C daemon to monitor the USB devices and apply the autosuspend rule instead of using udev.

And no, I don't want to touch udev if I can avoid it, so I'm completely fine with anything that works.