Skip to content

Conversation

@agx
Copy link
Contributor

@agx agx commented Apr 19, 2024

Device tree devices currently have some issues using hwdb since they can't use dmi/id. The corresponding thing in DT is the compatible of the root node. Add a udev rule to make use of this and as example add the Librem 5's wakup keys to ignore (so this depends on #32261 .

I've split this out as the to get feedback on adding match by device tree compatible. This could be useful for lots of things that so far needed DT changes to export properties relevant for userspace only.

Device tree devices can't match via dmi modalias. The corresponding
thing there is the DT compatbile. Allow to match by the most specific
part of the device tree compatible. This allows to e.g. match the volume
keys on a Librem 5 like:

  evdev:name:gpio-keys:dt:purism,librem5*

matching on a specific model would look like

  evdev:name:gpio-keys:dt:purism,librem5r4
@github-actions github-actions bot added hwdb please-review PR is ready for (re-)review by a maintainer labels Apr 19, 2024
@bluca bluca added ci-fails/needs-rework 🔥 Please rework this, the CI noticed an issue with the PR and removed please-review PR is ready for (re-)review by a maintainer labels Apr 19, 2024
@poettering
Copy link
Member

lgtm, but the test suite needs some updating:

Failed to parse: 'WAKEUP_KEY_114=0'
Failed to parse: 'WAKEUP_KEY_115=0'

@agx
Copy link
Contributor Author

agx commented Apr 22, 2024

Thanks for having a look. Tests will pass once #32261 landed. I'll rebase then.

@github-actions github-actions bot added please-review PR is ready for (re-)review by a maintainer and removed ci-fails/needs-rework 🔥 Please rework this, the CI noticed an issue with the PR labels Apr 23, 2024
@agx agx changed the title Draft: Allow device tree devices to make use of 60-evdev.rules and add Libem 5 Allow device tree devices to make use of 60-evdev.rules Apr 23, 2024
@agx
Copy link
Contributor Author

agx commented Apr 23, 2024

I've removed the bits that depend on #32261 and undrafted.

@poettering
Copy link
Member

hmm, i'd prefer if this is merged with at least one matching entry in in hwdb.

I think it might make sense to just add this as an additional commit to #32261 and merge it together.

@poettering poettering added postponed and removed please-review PR is ready for (re-)review by a maintainer labels Apr 23, 2024
@agx
Copy link
Contributor Author

agx commented Apr 25, 2024

I couldn't make an entry up that is really needed atm but without this 60-evdev.rules is about impossible to use on DT based devices. Having that in would allow us to explore #32261 further out of tree and then come up with more use cases.

@whot
Copy link
Contributor

whot commented May 2, 2024

I couldn't make an entry up that is really needed atm but without this 60-evdev.rules is about impossible to use on DT based devices. Having that in would allow us to explore #32261 further out of tree and then come up with more use cases.

In #32261 (comment) I mentioned that rules and hwdb have a "contract" and for your case that is basically down to the hwdb lookup method. So you can have this:

$ cat /etc/udev/rules.d/99-wakeup.rules
ACTION=="remove", GOTO="wakeup_end"
KERNEL!="event*", GOTO="wakeup_end"

# Device matching the input device name and device tree compatible
KERNELS=="input*", \
  IMPORT{builtin}="hwdb 'wakeup:name:$attr{name}:dt:$attr{[devicetree/base]compatible}'", \
  ENV{.HAVE_HWDB_PROPERTIES}="1"

# Device matching the modalias string (bustype, vendor, product, version, other properties)
IMPORT{builtin}="hwdb --subsystem=input --lookup-prefix=wakeup:", \
    ENV{.HAVE_HWDB_PROPERTIES}="1"

LABEL="wakeup_end"

(Apologies for any typos 😄)

So basically: have a udev rule (which can be a near-duplicate of 60-evdev.rules) that does things the way you want to. In this case I just renamed evdev to wakeup for demonstration, your matching hwdb entries the need to have that same prefix:

wakeup:atkbd:dmi:bvn*:bvr*:svnPurism:pnLibrem11:*
  BLAH=1

Really, neither udev nor the hwdb care about the content of the match string or how it's composed, it's a simple "build this string" and "lookup this string as key" with the only special behaviour being the * and other wildcards.

If you want a udev rule that calls date and then uses the day of the week as hwdb lookup prefix then that is possible (don't ask me for the exact syntax though... but oh, the use-cases!).

So while the addition to 60-evdev.rules looks correct I agree with @poettering that without a hwdb entry this is just dead weight. And for your current use-case you don't need it added to 60-evdev.rules (and indeed if the hwdb is outside 60-evdev.hwdb arguably it should have its own rules file anyway).

@agx
Copy link
Contributor Author

agx commented May 2, 2024

So basically: have a udev rule (which can be a near-duplicate of 60-evdev.rules) that does things the way you want to. In this case I just renamed evdev to wakeup for demonstration, your matching hwdb entries the need to have that same prefix:

Switching the lookup prefix is a good idea. I'll do that downstream too. Thanks!

@agx agx closed this May 2, 2024
FakeShell pushed a commit to FuriLabs/gmobile that referenced this pull request May 16, 2024
On phones and tablets the only key that should usually unidle a device
is the power button while other exposed hardware keys like the volume
buttons shouldn't e.g. take the device out of idle / unblank the screen.

Allow to specify whether an input device has wakeup keys (default is
1) and to specify whether individual keys on that device are wakeup
keys or not. E.g.

gmobile:…
 GM_WAKEUP_KEY_DEFAULT=0
 GM_WAKEUP_KEY_116=1

would mark all keys on the matching input device as non-wakeup keys
with the exception of the power button. This would be a typical setting
for phones. Whereas on a convertible that has a single keyboard device
containing both the exposed volume keys as well as the regular input
keys

gmobile:…
 GM_WAKEUP_KEY_114=0
 GM_WAKEUP_KEY_115=0

would allow all keys to unidle the device except for the exposed volume
rocker.

See systemd/systemd#32261 for details

Device tree devices can't match via dmi modalias. The corresponding
thing there is the DT compatible. Allow to match by the most specific
part of the device tree compatible. This allows to e.g. match the volume
keys on a Librem 5 like:

  gmobile:name:gpio-keys:dt:purism,librem5*

matching on a specific model would look like

  gmobile:name:gpio-keys:dt:purism,librem5r4

See systemd/systemd#32362 for details

Part-of: <https://gitlab.gnome.org/World/Phosh/gmobile/-/merge_requests/41>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

4 participants