From b4564641bec7e4d6b2070f6052fee8f35ece6a78 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 9 Jun 2020 17:00:56 +0200 Subject: [PATCH 1/7] hwdb: generate a hwdb file instead of rules for autosuspend --- hwdb.d/meson.build | 8 ++++++++ rules.d/meson.build | 8 -------- tools/make-autosuspend-rules.py | 21 ++++++++++++++++++++- 3 files changed, 28 insertions(+), 9 deletions(-) diff --git a/hwdb.d/meson.build b/hwdb.d/meson.build index 4df6dabf89..e5ac23a8ff 100644 --- a/hwdb.d/meson.build +++ b/hwdb.d/meson.build @@ -52,3 +52,11 @@ run_target( run_target( 'autosuspend-update', command : [autosuspend_update_sh, project_source_root + '/tools/chromiumos']) + +auto_suspend_rules = custom_target( + '60-autosuspend-chromiumos.hwdb', + output : '60-autosuspend-chromiumos.hwdb', + command : make_autosuspend_rules_py, + capture : true, + install : true, + install_dir: udevhwdbdir) diff --git a/rules.d/meson.build b/rules.d/meson.build index 13d1d330cf..b4684486c2 100644 --- a/rules.d/meson.build +++ b/rules.d/meson.build @@ -45,11 +45,3 @@ foreach file : rules_in install_dir : udevrulesdir) all_rules += gen endforeach - -auto_suspend_rules = custom_target( - '60-autosuspend-chromiumos.rules', - output : '60-autosuspend-chromiumos.rules', - command : make_autosuspend_rules_py, - capture : true, - install : true, - install_dir: [udevrulesdir]) diff --git a/tools/make-autosuspend-rules.py b/tools/make-autosuspend-rules.py index 25b261ea0d..3faf5f4d71 100755 --- a/tools/make-autosuspend-rules.py +++ b/tools/make-autosuspend-rules.py @@ -8,7 +8,26 @@ import sys import chromiumos.gen_autosuspend_rules +HWDB_FILE = """\ +%(usb_entries)s\ +%(pci_entries)s\ +""" + if __name__ == '__main__': if len(sys.argv) > 1: sys.stdout = open(sys.argv[1], 'w') - chromiumos.gen_autosuspend_rules.main() + + pci_entries = '' + for dev_ids in chromiumos.gen_autosuspend_rules.PCI_IDS: + vendor, device = dev_ids.split(':') + + pci_entries += ('usb:v%sp%s*\n' + ' ID_AUTOSUSPEND=1\n' % (vendor, device)) + usb_entries = '' + for dev_ids in chromiumos.gen_autosuspend_rules.USB_IDS: + vendor, device = dev_ids.split(':') + + usb_entries += ('pci:v%sp%s*\n' + ' ID_AUTOSUSPEND=1\n' % (vendor, device)) + + print(HWDB_FILE % {'pci_entries' : pci_entries, 'usb_entries': usb_entries}) From 876c75fe870846b09b54423a6b719d80bc879b27 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sun, 14 Jun 2020 18:34:27 +0200 Subject: [PATCH 2/7] udev: do not execute hwdb builtin import twice or thrice Running the import more than once shouldn't matter, but it's a bit confusing and causes a slowdown too. This patch reworks the rules to avoid duplicted runs, but tries to keep the same imports. (E.g. import for pci devices without MODALIAS is only done for tty devices.) Note that this is only about hwdb import with argument, which uses MODALIAS as the lookup key. There are other imports done with different lookup keys. --- rules.d/50-udev-default.rules.in | 3 ++- rules.d/60-serial.rules | 5 +++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/rules.d/50-udev-default.rules.in b/rules.d/50-udev-default.rules.in index 50747a1988..cef78f9d68 100644 --- a/rules.d/50-udev-default.rules.in +++ b/rules.d/50-udev-default.rules.in @@ -10,8 +10,9 @@ SUBSYSTEM=="virtio-ports", KERNEL=="vport*", ATTR{name}=="?*", SYMLINK+="virtio- SUBSYSTEM=="rtc", ATTR{hctosys}=="1", SYMLINK+="rtc" SUBSYSTEM=="rtc", KERNEL=="rtc0", SYMLINK+="rtc", OPTIONS+="link_priority=-100" -SUBSYSTEM=="usb", ENV{DEVTYPE}=="usb_device", IMPORT{builtin}="usb_id", IMPORT{builtin}="hwdb --subsystem=usb" +SUBSYSTEM=="usb", ENV{DEVTYPE}=="usb_device", IMPORT{builtin}="usb_id", IMPORT{builtin}="hwdb --subsystem=usb", GOTO="default_hwdb_imported" ENV{MODALIAS}!="", IMPORT{builtin}="hwdb --subsystem=$env{SUBSYSTEM}" +LABEL="default_hwdb_imported" ACTION!="add", GOTO="default_end" diff --git a/rules.d/60-serial.rules b/rules.d/60-serial.rules index f303e27fd5..b1626650b7 100644 --- a/rules.d/60-serial.rules +++ b/rules.d/60-serial.rules @@ -4,8 +4,9 @@ ACTION=="remove", GOTO="serial_end" SUBSYSTEM!="tty", GOTO="serial_end" SUBSYSTEMS=="pci", ENV{ID_BUS}="pci", ENV{ID_VENDOR_ID}="$attr{vendor}", ENV{ID_MODEL_ID}="$attr{device}" -SUBSYSTEMS=="pci", IMPORT{builtin}="hwdb --subsystem=pci" -SUBSYSTEMS=="usb", IMPORT{builtin}="usb_id", IMPORT{builtin}="hwdb --subsystem=usb" +# We already ran the hwdb builtin for devices with MODALIAS in 50-default.rules. +# Let's cover the remaining case here, where we walk up the tree to find a node with $MODALIAS. +ENV{MODALIAS}=="", SUBSYSTEMS=="pci", IMPORT{builtin}="hwdb --subsystem=pci" # /dev/serial/by-path/, /dev/serial/by-id/ for USB devices KERNEL!="ttyUSB[0-9]*|ttyACM[0-9]*", GOTO="serial_end" From 79dc5d35dd7eb372afeb638cc1f86d1da2045623 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sat, 13 Jun 2020 17:52:41 +0200 Subject: [PATCH 3/7] tools: rewrite make-autosuspend-rules.py and add udev rules Concatenating strings is not a very efficient approach. And in this case fully unnecessary. We also need some rules to make use of those hwdb entries. PCI needs to be 8 characters, not 4. And we need to use uppercase hexadecimal for both. With udev rules this made no difference, but hwdb match is case sensitive. Fixes #16119. --- rules.d/60-autosuspend.rules | 14 +++++++++++++ rules.d/meson.build | 1 + tools/make-autosuspend-rules.py | 37 +++++++++++++-------------------- 3 files changed, 29 insertions(+), 23 deletions(-) create mode 100644 rules.d/60-autosuspend.rules diff --git a/rules.d/60-autosuspend.rules b/rules.d/60-autosuspend.rules new file mode 100644 index 0000000000..1f9ebef631 --- /dev/null +++ b/rules.d/60-autosuspend.rules @@ -0,0 +1,14 @@ +# do not edit this file, it will be overwritten on update + +ACTION!="add", GOTO="autosuspend_end" + +# I2C rules +SUBSYSTEM=="i2c", ATTR{name}=="cyapa", \ + ATTR{power/control}="on", GOTO="autosuspend_end" + +# Enable autosuspend if hwdb says so. Here we are relying on +# the hwdb import done earlier based on MODALIAS. +ENV{ID_AUTOSUSPEND}=="1", TEST=="power/control", \ + ATTR{power/control}="auto" + +LABEL="autosuspend_end" diff --git a/rules.d/meson.build b/rules.d/meson.build index b4684486c2..ffa010d8e3 100644 --- a/rules.d/meson.build +++ b/rules.d/meson.build @@ -1,6 +1,7 @@ # SPDX-License-Identifier: LGPL-2.1+ rules = files(''' + 60-autosuspend.rules 60-block.rules 60-cdrom_id.rules 60-drm.rules diff --git a/tools/make-autosuspend-rules.py b/tools/make-autosuspend-rules.py index 3faf5f4d71..1a99e02aad 100755 --- a/tools/make-autosuspend-rules.py +++ b/tools/make-autosuspend-rules.py @@ -2,32 +2,23 @@ # SPDX-License-Identifier: LGPL-2.1+ # Generate autosuspend rules for devices that have been whitelisted (IE tested) -# by the Chromium OS team. Please keep this script in sync with: +# by the Chromium OS team. Based on # https://chromium.googlesource.com/chromiumos/platform2/+/master/power_manager/udev/gen_autosuspend_rules.py -import sys import chromiumos.gen_autosuspend_rules -HWDB_FILE = """\ -%(usb_entries)s\ -%(pci_entries)s\ -""" +print('# pci:v<00VENDOR>d<00DEVICE> (8 uppercase hexadecimal digits twice)') +for entry in chromiumos.gen_autosuspend_rules.PCI_IDS: + vendor, device = entry.split(':') + vendor = int(vendor, 16) + device = int(device, 16) + print(f'pci:v{vendor:08X}d{device:08X}*') -if __name__ == '__main__': - if len(sys.argv) > 1: - sys.stdout = open(sys.argv[1], 'w') +print('# usb:vp (4 uppercase hexadecimal digits twice') +for entry in chromiumos.gen_autosuspend_rules.USB_IDS: + vendor, product = entry.split(':') + vendor = int(vendor, 16) + product = int(product, 16) + print(f'usb:v{vendor:04X}p{product:04X}*') - pci_entries = '' - for dev_ids in chromiumos.gen_autosuspend_rules.PCI_IDS: - vendor, device = dev_ids.split(':') - - pci_entries += ('usb:v%sp%s*\n' - ' ID_AUTOSUSPEND=1\n' % (vendor, device)) - usb_entries = '' - for dev_ids in chromiumos.gen_autosuspend_rules.USB_IDS: - vendor, device = dev_ids.split(':') - - usb_entries += ('pci:v%sp%s*\n' - ' ID_AUTOSUSPEND=1\n' % (vendor, device)) - - print(HWDB_FILE % {'pci_entries' : pci_entries, 'usb_entries': usb_entries}) +print(' ID_AUTOSUSPEND=1') From aa549ff3972b067c4225db0a845f5c638842fba3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sun, 14 Jun 2020 12:43:42 +0200 Subject: [PATCH 4/7] test: also verify the generated autosuspend hwdb file Hint: meson test -C build parse-hwdb -v --- hwdb.d/meson.build | 43 +++++++++++++++++++++++++------------------ hwdb.d/parse_hwdb.py | 18 +++++++++++++++++- 2 files changed, 42 insertions(+), 19 deletions(-) diff --git a/hwdb.d/meson.build b/hwdb.d/meson.build index e5ac23a8ff..95e32729bf 100644 --- a/hwdb.d/meson.build +++ b/hwdb.d/meson.build @@ -1,6 +1,9 @@ # SPDX-License-Identifier: LGPL-2.1+ -hwdb_files = files(''' +# Those files right now are not supported by the grammar. Also, +# they are very long but quite repetitive and the parser is not very fast. +# So we don't "test" them. +hwdb_files_notest = files(''' 20-pci-vendor-model.hwdb 20-pci-classes.hwdb 20-usb-vendor-model.hwdb @@ -12,6 +15,9 @@ hwdb_files = files(''' 20-OUI.hwdb 20-net-ifname.hwdb 20-vmbus-class.hwdb +'''.split()) + +hwdb_files_test = files(''' 60-evdev.hwdb 60-input-id.hwdb 60-keyboard.hwdb @@ -23,7 +29,16 @@ hwdb_files = files(''' '''.split()) if conf.get('ENABLE_HWDB') == 1 - install_data(hwdb_files, + auto_suspend_rules = custom_target( + '60-autosuspend-chromiumos.hwdb', + output : '60-autosuspend-chromiumos.hwdb', + command : make_autosuspend_rules_py, + capture : true, + install : true, + install_dir: udevhwdbdir) + + install_data(hwdb_files_notest, + hwdb_files_test, install_dir : udevhwdbdir) meson.add_install_script('sh', '-c', @@ -32,15 +47,15 @@ if conf.get('ENABLE_HWDB') == 1 meson.add_install_script('sh', '-c', 'test -n "$DESTDIR" || @0@/systemd-hwdb update' .format(rootbindir)) -endif -############################################################ - -parse_hwdb_py = find_program('parse_hwdb.py') -if want_tests != 'false' - test('parse-hwdb', - parse_hwdb_py, - timeout : 90) + if want_tests != 'false' + parse_hwdb_py = find_program('parse_hwdb.py') + test('parse-hwdb', + parse_hwdb_py, + args : [hwdb_files_test, + auto_suspend_rules], + timeout : 90) + endif endif ############################################################ @@ -52,11 +67,3 @@ run_target( run_target( 'autosuspend-update', command : [autosuspend_update_sh, project_source_root + '/tools/chromiumos']) - -auto_suspend_rules = custom_target( - '60-autosuspend-chromiumos.hwdb', - output : '60-autosuspend-chromiumos.hwdb', - command : make_autosuspend_rules_py, - capture : true, - install : true, - install_dir: udevhwdbdir) diff --git a/hwdb.d/parse_hwdb.py b/hwdb.d/parse_hwdb.py index abef56728f..8e1b5fdafa 100755 --- a/hwdb.d/parse_hwdb.py +++ b/hwdb.d/parse_hwdb.py @@ -59,6 +59,7 @@ REAL = Combine((INTEGER + Optional('.' + Optional(INTEGER))) ^ ('.' + INTEGER)) SIGNED_REAL = Combine(Optional(Word('-+')) + REAL) UDEV_TAG = Word(string.ascii_uppercase, alphanums + '_') +# Those patterns are used in type-specific matches TYPES = {'mouse': ('usb', 'bluetooth', 'ps2', '*'), 'evdev': ('name', 'atkbd', 'input'), 'id-input': ('modalias'), @@ -68,13 +69,27 @@ TYPES = {'mouse': ('usb', 'bluetooth', 'ps2', '*'), 'sensor': ('modalias', ), } +# Patterns that are used to set general properties on a device +GENERAL_MATCHES = {'acpi', + 'bluetooth', + 'usb', + 'pci', + 'sdio', + 'vmbus', + 'OUI', + } + @lru_cache() def hwdb_grammar(): ParserElement.setDefaultWhitespaceChars('') prefix = Or(category + ':' + Or(conn) + ':' for category, conn in TYPES.items()) - matchline = Combine(prefix + Word(printables + ' ' + '®')) + EOL + + matchline_typed = Combine(prefix + Word(printables + ' ' + '®')) + matchline_general = Combine(Or(GENERAL_MATCHES) + ':' + Word(printables)) + matchline = (matchline_typed | matchline_general) + EOL + propertyline = (White(' ', exact=1).suppress() + Combine(UDEV_TAG - '=' - Word(alphanums + '_=:@*.!-;, "') - Optional(pythonStyleComment)) + EOL) @@ -102,6 +117,7 @@ def property_grammar(): ('MOUSE_WHEEL_CLICK_ANGLE_HORIZONTAL', INTEGER), ('MOUSE_WHEEL_CLICK_COUNT', INTEGER), ('MOUSE_WHEEL_CLICK_COUNT_HORIZONTAL', INTEGER), + ('ID_AUTOSUSPEND', Literal('1')), ('ID_INPUT', Literal('1')), ('ID_INPUT_ACCELEROMETER', Literal('1')), ('ID_INPUT_JOYSTICK', Literal('1')), From 39d5b72e32c15c1d5e7e9b70d38707b9e5a780bf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 18 Jun 2020 18:38:58 +0200 Subject: [PATCH 5/7] hwdb: enable autosuspend for Wacom Pen and EMV Smartcard Reader Replaces #16113. --- hwdb.d/60-autosuspend.hwdb | 44 ++++++++++++++++++++++++++++++++++++++ hwdb.d/meson.build | 1 + 2 files changed, 45 insertions(+) create mode 100644 hwdb.d/60-autosuspend.hwdb diff --git a/hwdb.d/60-autosuspend.hwdb b/hwdb.d/60-autosuspend.hwdb new file mode 100644 index 0000000000..0de49afa6c --- /dev/null +++ b/hwdb.d/60-autosuspend.hwdb @@ -0,0 +1,44 @@ +# This file is part of systemd. +# +# The lookup keys are $MODALIAS strings, see udev's hwdb builtin. +# +# Match string formats: +# : +# +# pci:vd +# usb:vp +# +# To add local entries, create a new file +# /etc/udev/hwdb.d/61-autosuspend-local.hwdb +# and add your rules there. To load the new rules execute (as root): +# systemd-hwdb update +# udevadm trigger /dev/… +# +# If your changes are generally applicable, preferably send them as a pull +# request to +# https://github.com/systemd/systemd +# or create a bug report on https://github.com/systemd/systemd/issues and +# include your new rules, a description of the device, and the output of +# udevadm info +# the device. +# +# Allowed properties are: +# ID_AUTOSUSPEND=1 + +# +# Sort by brand, model + +######################################### +# Alcor +######################################### + +# AU9540 Smartcard Reader +usb:v058Fp9540* + ID_AUTOSUSPEND=1 + +######################################### +# Wacom +######################################### + +usb:v056Ap51A0* + ID_AUTOSUSPEND=1 diff --git a/hwdb.d/meson.build b/hwdb.d/meson.build index 95e32729bf..5c77387a26 100644 --- a/hwdb.d/meson.build +++ b/hwdb.d/meson.build @@ -18,6 +18,7 @@ hwdb_files_notest = files(''' '''.split()) hwdb_files_test = files(''' + 60-autosuspend.hwdb 60-evdev.hwdb 60-input-id.hwdb 60-keyboard.hwdb From 88936ed6f95b8fdc9ff57b5837e2c918229f900b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Mon, 22 Jun 2020 14:38:01 +0200 Subject: [PATCH 6/7] rules: remove 61-autosuspend-manual.rules It doesn't hurt, but there's no point in keeping it now. Any changes can be added to 60-autosuspend.hwdb. --- rules.d/61-autosuspend-manual.rules | 19 ------------------- rules.d/meson.build | 1 - 2 files changed, 20 deletions(-) delete mode 100644 rules.d/61-autosuspend-manual.rules diff --git a/rules.d/61-autosuspend-manual.rules b/rules.d/61-autosuspend-manual.rules deleted file mode 100644 index 2b973af960..0000000000 --- a/rules.d/61-autosuspend-manual.rules +++ /dev/null @@ -1,19 +0,0 @@ -# This udev rule is for any devices that should enter automatic suspend -# but are not already included in generated rules from Chromium OS via -# tools/make-autosuspend-rules.py -# - -ACTION!="add", GOTO="autosuspend_manual_end" -SUBSYSTEM!="usb", GOTO="autosuspend_manual_end" - -SUBSYSTEM=="usb", GOTO="autosuspend_manual_usb" - -# USB rules -LABEL="autosuspend_manual_usb" -GOTO="autosuspend_manual_end" - -# Enable autosuspend -LABEL="autosuspend_manual_enable" -TEST=="power/control", ATTR{power/control}="auto", GOTO="autosuspend_manual_end" - -LABEL="autosuspend_manual_end" diff --git a/rules.d/meson.build b/rules.d/meson.build index ffa010d8e3..ca4445d774 100644 --- a/rules.d/meson.build +++ b/rules.d/meson.build @@ -15,7 +15,6 @@ rules = files(''' 60-persistent-v4l.rules 60-sensor.rules 60-serial.rules - 61-autosuspend-manual.rules 70-joystick.rules 70-mouse.rules 70-touchpad.rules From 7b33ff7388a4d1de9376ef2883a73272107d44a9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Mon, 22 Jun 2020 14:41:50 +0200 Subject: [PATCH 7/7] make-autosuspend-rules: remove one instance of "whitelist" Let's be clear what we mean exactly. Also see https://lwn.net/Articles/823224/ for general justification. --- tools/make-autosuspend-rules.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/make-autosuspend-rules.py b/tools/make-autosuspend-rules.py index 1a99e02aad..065752ced4 100755 --- a/tools/make-autosuspend-rules.py +++ b/tools/make-autosuspend-rules.py @@ -1,8 +1,8 @@ #!/usr/bin/env python3 # SPDX-License-Identifier: LGPL-2.1+ -# Generate autosuspend rules for devices that have been whitelisted (IE tested) -# by the Chromium OS team. Based on +# Generate autosuspend rules for devices that have been tested to work properly +# with autosuspend by the Chromium OS team. Based on # https://chromium.googlesource.com/chromiumos/platform2/+/master/power_manager/udev/gen_autosuspend_rules.py import chromiumos.gen_autosuspend_rules