Stop installing custom udev rules from BSP packages |
|||||||||||||
Issue descriptionWe install a bunch of miscellaneous udev rules from boards' BSP packages now, typically with the name 92-powerd-overrides.rules. BSP packages are a pain to update (ew bumping symlinks) and this is fragile for a bunch of other reasons: - The files need to be named correctly so they run between powerd's 91- and 93- rules. - The tags used within the files are dependent on the particulars of powerd's default udev rules. - There's a bunch of copy-and-pasting going on here. I'd like to switch to conditionally installing additional rules files from the power_manager package when a board asks for it via a USE flag. I've already done some of this via a new keyboard_includes_side_buttons USE flag in issue 708180 . I'm going to use this to track the rest of it. Here's some of the stuff that I've noticed: a) We unconditionally disable touchpad wakeup on some convertible devices. I don't think that there's any good reason for this now; powerd configures input devices for wakeup mode and is already disabling touchpad wakeup when touchpads aren't usable. (I hope there aren't any convertibles that don't report tablet mode correctly.) b) We explicitly disable touchscreen wakeup for some convertible devices. We should do this unconditionally and add a USE flag to override the behavior. This is tracked in issue 487380 . c) Probably other stuff.
,
Apr 12 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/overlays/board-overlays/+/f4c9a4381c990baf93f953df849cbb6ac0a44019 commit f4c9a4381c990baf93f953df849cbb6ac0a44019 Author: Daniel Erat <derat@chromium.org> Date: Wed Apr 12 09:19:05 2017 overlays: Remove udev rules to disable touchpad wakeup. Several convertible devices (clapper, glimmer, kevin, pyro, reef, snappy) were installing udev rules from their BSP packages instructing powerd to completely disable touchpad wakeup. powerd configures devices for tablet mode now, and the installed-by-default udev rules say that touchpads aren't usable while in tablet mode and should only trigger wakeups while usable, so the custom rules are unnecessary. BUG= chromium:710472 TEST=none Change-Id: I55908c3e958a07d146f7bb078d0039771c85f0fe Reviewed-on: https://chromium-review.googlesource.com/474013 Commit-Ready: Dan Erat <derat@chromium.org> Tested-by: Dan Erat <derat@chromium.org> Reviewed-by: Mike Frysinger <vapier@chromium.org> [modify] https://crrev.com/f4c9a4381c990baf93f953df849cbb6ac0a44019/overlay-kevin/chromeos-base/chromeos-bsp-kevin/files/92-powerd-overrides.rules [delete] https://crrev.com/db1fea4a66e2f1536913ce36bfd96165c00cf214/overlay-glimmer/chromeos-base/chromeos-bsp-glimmer/files/92-powerd-overrides.rules [modify] https://crrev.com/f4c9a4381c990baf93f953df849cbb6ac0a44019/overlay-snappy/chromeos-base/chromeos-bsp-snappy/files/92-powerd-overrides.rules [rename] https://crrev.com/f4c9a4381c990baf93f953df849cbb6ac0a44019/overlay-reef/chromeos-base/chromeos-bsp-reef/chromeos-bsp-reef-0.0.1-r11.ebuild [rename] https://crrev.com/f4c9a4381c990baf93f953df849cbb6ac0a44019/overlay-kevin/chromeos-base/chromeos-bsp-kevin/chromeos-bsp-kevin-0.0.3-r7.ebuild [modify] https://crrev.com/f4c9a4381c990baf93f953df849cbb6ac0a44019/overlay-clapper/chromeos-base/chromeos-bsp-clapper/chromeos-bsp-clapper-0.0.1.ebuild [rename] https://crrev.com/f4c9a4381c990baf93f953df849cbb6ac0a44019/overlay-clapper/chromeos-base/chromeos-bsp-clapper/chromeos-bsp-clapper-0.0.1-r25.ebuild [rename] https://crrev.com/f4c9a4381c990baf93f953df849cbb6ac0a44019/overlay-pyro/chromeos-base/chromeos-bsp-pyro/chromeos-bsp-pyro-0.0.1-r11.ebuild [delete] https://crrev.com/db1fea4a66e2f1536913ce36bfd96165c00cf214/overlay-clapper/chromeos-base/chromeos-bsp-clapper/files/92-powerd-overrides.rules [modify] https://crrev.com/f4c9a4381c990baf93f953df849cbb6ac0a44019/overlay-glimmer/chromeos-base/chromeos-bsp-glimmer/chromeos-bsp-glimmer-0.0.1.ebuild [modify] https://crrev.com/f4c9a4381c990baf93f953df849cbb6ac0a44019/overlay-reef/chromeos-base/chromeos-bsp-reef/files/92-powerd-overrides.rules [rename] https://crrev.com/f4c9a4381c990baf93f953df849cbb6ac0a44019/overlay-snappy/chromeos-base/chromeos-bsp-snappy/chromeos-bsp-snappy-0.0.1-r10.ebuild [rename] https://crrev.com/f4c9a4381c990baf93f953df849cbb6ac0a44019/overlay-glimmer/chromeos-base/chromeos-bsp-glimmer/chromeos-bsp-glimmer-0.0.1-r27.ebuild [modify] https://crrev.com/f4c9a4381c990baf93f953df849cbb6ac0a44019/overlay-pyro/chromeos-base/chromeos-bsp-pyro/files/92-powerd-overrides.rules
,
Apr 13 2017
The following revision refers to this bug: https://chrome-internal.googlesource.com/chromeos/overlays/overlay-variant-veyron-minnie-private/+/f2d18ec52089c8ba50915b2d7d624922e79b71a1 commit f2d18ec52089c8ba50915b2d7d624922e79b71a1 Author: Daniel Erat <derat@chromium.org> Date: Thu Apr 13 03:22:31 2017
,
Apr 13 2017
Benson, re your comment at http://crbug.com/711033 #9 and http://b/35586256, do we actually not want the change that I made here to allow touchpad wakeup when the touchpad is considered to be usable?
,
Apr 13 2017
Hey Dan : The intent right now is for all convertible systems to not wake on the touchpad at all. I think that was the content of some of the custom udev rules scattered among board overlays. This is because we have no good way of managing the wake capability of the touchpad when the system is already asleep, and the user decides to change from clamshell mode to one of the tablet modes or vice versa. You could end up in a state where the system is in tent or stand mode, but the touchpad is still a wake source. Or, the system transitioned from tent or stand into clamshell, but the touchpad does not act as a wake source. The exploration in b/35586256 is to configure the EC to wake the system from S3 when we transition past 180-degrees regardless so the AP has a chance to enact a policy change on the wakeup sources... ie, wake up the system so we can configure the touchpad and keyboard as wakesources or not. Because doing what I propose requires an EC change, we'd roll it out one board at a time.
,
Apr 13 2017
Thanks for the background. If we're in S3 in clamshell mode and then get converted to tablet mode, I think that powerd should still disable touchpad wakeup after we resume. I agree that it's a bad user experience for us to wake the first time the touchpad is touched after going to tablet mode, though. So it sounds like I should probably introduce a touchpad_wake USE flag, set by default, that convertibles can unset to get the old behavior back. Does that sound right?
,
Apr 13 2017
New USE flag to set touchpad_wake sounds good to me.
,
Apr 14 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform2/+/452cdd05a9f1065681ea4cd0090f417a44784310 commit 452cdd05a9f1065681ea4cd0090f417a44784310 Author: Daniel Erat <derat@chromium.org> Date: Fri Apr 14 18:11:21 2017 power: Add optional udev rules to disable touchpad wakeup. Add a 92-powerd-tags-no-touchpad-wakeup.rules file that can be installed to instruct powerd to unconditionally disable touchpad wakeup. BUG= chromium:710472 TEST=none Change-Id: I45314138ceb9532cf50794e768ff30b53293ee02 Reviewed-on: https://chromium-review.googlesource.com/477592 Commit-Ready: Dan Erat <derat@chromium.org> Tested-by: Dan Erat <derat@chromium.org> Reviewed-by: Mike Frysinger <vapier@chromium.org> [add] https://crrev.com/452cdd05a9f1065681ea4cd0090f417a44784310/power_manager/udev/optional/92-powerd-tags-no-touchpad-wakeup.rules
,
Apr 14 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/842c75301dd8bfae127b8cec70b61547da51aae9 commit 842c75301dd8bfae127b8cec70b61547da51aae9 Author: Daniel Erat <derat@chromium.org> Date: Fri Apr 14 18:11:21 2017 power_manager: Honor touchpad_wakeup USE flag. Make the power_manager ebuild honor an on-by-default touchpad_wakeup USE flag. If the flag is unset by an overlay, 92-powerd-tags-no-touchpad-wakeup.rules is installed to add udev tags instructing powerd to unconditionally disable touchpad wakeup. This is currently needed for convertible devices that lack the ability to disable touchpad wakeup when they are switched from clamshell mode to tablet mode while already suspended. BUG= chromium:710472 TEST=built and saw that the rules file wasn't installed; rebuilt with USE=-touchpad_wakeup and saw that it was CQ-DEPEND=I45314138ceb9532cf50794e768ff30b53293ee02 Change-Id: I56f4e965f556ca6a7d0852f83c35a10853c49ee3 Reviewed-on: https://chromium-review.googlesource.com/477374 Commit-Ready: Dan Erat <derat@chromium.org> Tested-by: Dan Erat <derat@chromium.org> Reviewed-by: Mike Frysinger <vapier@chromium.org> [modify] https://crrev.com/842c75301dd8bfae127b8cec70b61547da51aae9/chromeos-base/power_manager/power_manager-9999.ebuild
,
Apr 17 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/overlays/board-overlays/+/0b6967ecbec5aad49d8f6827b59f57df29b1d3c4 commit 0b6967ecbec5aad49d8f6827b59f57df29b1d3c4 Author: Daniel Erat <derat@chromium.org> Date: Mon Apr 17 23:25:04 2017 overlays: Disable touchpad wakeup for convertibles. Unset the touchpad_wakeup USE flag in convertible boards' overlays to instruct powerd to unconditionally disable touchpad wakeup. These devices lack the ability to dynamically disable wakeup when they're converted to tablet mode while suspended. This effectively undoes f4c9a4381c990, which removed custom udev rules files installed by these boards' BSP packages, but switches us to use an easier-to-maintain USE flag. It also adds a bunch of boards that didn't have the udev rules before but should've. BUG= chromium:710472 TEST=emerged the power_manager package for kevin and verified that /lib/udev/rules.d/92-powerd-tags-no-touchpad-wakeup.rules was installed CQ-DEPEND=I56f4e965f556ca6a7d0852f83c35a10853c49ee3 Change-Id: I2b26cd20377ab08ceda1fe52b6a3536dcbd5b559 Reviewed-on: https://chromium-review.googlesource.com/477464 Commit-Ready: Dan Erat <derat@chromium.org> Tested-by: Dan Erat <derat@chromium.org> Reviewed-by: Benson Leung <bleung@chromium.org> [modify] https://crrev.com/0b6967ecbec5aad49d8f6827b59f57df29b1d3c4/overlay-glimmer/make.conf [modify] https://crrev.com/0b6967ecbec5aad49d8f6827b59f57df29b1d3c4/overlay-pyro/make.conf [modify] https://crrev.com/0b6967ecbec5aad49d8f6827b59f57df29b1d3c4/overlay-eve/make.conf [modify] https://crrev.com/0b6967ecbec5aad49d8f6827b59f57df29b1d3c4/overlay-gru/profiles/base/make.defaults [modify] https://crrev.com/0b6967ecbec5aad49d8f6827b59f57df29b1d3c4/overlay-variant-veyron-minnie/make.conf [modify] https://crrev.com/0b6967ecbec5aad49d8f6827b59f57df29b1d3c4/overlay-bob/profiles/base/make.defaults [modify] https://crrev.com/0b6967ecbec5aad49d8f6827b59f57df29b1d3c4/overlay-hana/make.conf [modify] https://crrev.com/0b6967ecbec5aad49d8f6827b59f57df29b1d3c4/overlay-nasher/profiles/base/make.defaults [modify] https://crrev.com/0b6967ecbec5aad49d8f6827b59f57df29b1d3c4/overlay-elm/make.conf [modify] https://crrev.com/0b6967ecbec5aad49d8f6827b59f57df29b1d3c4/overlay-kevin/profiles/base/make.defaults [modify] https://crrev.com/0b6967ecbec5aad49d8f6827b59f57df29b1d3c4/overlay-snappy/make.conf [modify] https://crrev.com/0b6967ecbec5aad49d8f6827b59f57df29b1d3c4/overlay-reef/make.conf [modify] https://crrev.com/0b6967ecbec5aad49d8f6827b59f57df29b1d3c4/overlay-pbody/make.conf [modify] https://crrev.com/0b6967ecbec5aad49d8f6827b59f57df29b1d3c4/overlay-cyan/make.conf [modify] https://crrev.com/0b6967ecbec5aad49d8f6827b59f57df29b1d3c4/overlay-clapper/make.conf [modify] https://crrev.com/0b6967ecbec5aad49d8f6827b59f57df29b1d3c4/overlay-caroline/make.conf [modify] https://crrev.com/0b6967ecbec5aad49d8f6827b59f57df29b1d3c4/overlay-wizpig/make.conf [modify] https://crrev.com/0b6967ecbec5aad49d8f6827b59f57df29b1d3c4/overlay-cave/make.conf
,
Apr 17 2017
,
Apr 18 2017
Waiting for testing on tot to be done before approving merge to branch.
,
Apr 18 2017
Your change meets the bar and is auto-approved for M59. Please go ahead and merge the CL to branch 3071 manually. Please contact milestone owner if you have questions. Owners: amineer@(Android), cmasso@(iOS), gkihumba@(ChromeOS), Abdul Syed@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Apr 19 2017
,
Apr 19 2017
,
Apr 19 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform2/+/ea63e3f96b3aeb470c493e39b9163392432172d0 commit ea63e3f96b3aeb470c493e39b9163392432172d0 Author: Daniel Erat <derat@chromium.org> Date: Wed Apr 19 18:35:19 2017 power: Add optional udev rules to disable touchpad wakeup. Add a 92-powerd-tags-no-touchpad-wakeup.rules file that can be installed to instruct powerd to unconditionally disable touchpad wakeup. BUG= chromium:710472 TEST=none Change-Id: I45314138ceb9532cf50794e768ff30b53293ee02 Reviewed-on: https://chromium-review.googlesource.com/477592 Commit-Ready: Dan Erat <derat@chromium.org> Tested-by: Dan Erat <derat@chromium.org> Reviewed-by: Mike Frysinger <vapier@chromium.org> (cherry picked from commit 452cdd05a9f1065681ea4cd0090f417a44784310) Reviewed-on: https://chromium-review.googlesource.com/481903 Reviewed-by: Dan Erat <derat@chromium.org> [add] https://crrev.com/ea63e3f96b3aeb470c493e39b9163392432172d0/power_manager/udev/optional/92-powerd-tags-no-touchpad-wakeup.rules
,
Apr 19 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/70ada9da78efba93d8508f24d9c94689a6e2aa2e commit 70ada9da78efba93d8508f24d9c94689a6e2aa2e Author: Daniel Erat <derat@chromium.org> Date: Wed Apr 19 18:35:56 2017 power_manager: Honor touchpad_wakeup USE flag. Make the power_manager ebuild honor an on-by-default touchpad_wakeup USE flag. If the flag is unset by an overlay, 92-powerd-tags-no-touchpad-wakeup.rules is installed to add udev tags instructing powerd to unconditionally disable touchpad wakeup. This is currently needed for convertible devices that lack the ability to disable touchpad wakeup when they are switched from clamshell mode to tablet mode while already suspended. BUG= chromium:710472 TEST=built and saw that the rules file wasn't installed; rebuilt with USE=-touchpad_wakeup and saw that it was CQ-DEPEND=I45314138ceb9532cf50794e768ff30b53293ee02 Change-Id: I56f4e965f556ca6a7d0852f83c35a10853c49ee3 Reviewed-on: https://chromium-review.googlesource.com/477374 Commit-Ready: Dan Erat <derat@chromium.org> Tested-by: Dan Erat <derat@chromium.org> Reviewed-by: Mike Frysinger <vapier@chromium.org> (cherry picked from commit 842c75301dd8bfae127b8cec70b61547da51aae9) Reviewed-on: https://chromium-review.googlesource.com/481923 Reviewed-by: Dan Erat <derat@chromium.org> [modify] https://crrev.com/70ada9da78efba93d8508f24d9c94689a6e2aa2e/chromeos-base/power_manager/power_manager-9999.ebuild
,
Apr 19 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/overlays/board-overlays/+/a174d0ef810f22338a3a496b4a0f794ac3ea5281 commit a174d0ef810f22338a3a496b4a0f794ac3ea5281 Author: Daniel Erat <derat@chromium.org> Date: Wed Apr 19 18:36:13 2017 overlays: Disable touchpad wakeup for convertibles. Unset the touchpad_wakeup USE flag in convertible boards' overlays to instruct powerd to unconditionally disable touchpad wakeup. These devices lack the ability to dynamically disable wakeup when they're converted to tablet mode while suspended. This effectively undoes f4c9a4381c990, which removed custom udev rules files installed by these boards' BSP packages, but switches us to use an easier-to-maintain USE flag. It also adds a bunch of boards that didn't have the udev rules before but should've. BUG= chromium:710472 TEST=emerged the power_manager package for kevin and verified that /lib/udev/rules.d/92-powerd-tags-no-touchpad-wakeup.rules was installed CQ-DEPEND=I56f4e965f556ca6a7d0852f83c35a10853c49ee3 Change-Id: I2b26cd20377ab08ceda1fe52b6a3536dcbd5b559 Reviewed-on: https://chromium-review.googlesource.com/477464 Commit-Ready: Dan Erat <derat@chromium.org> Tested-by: Dan Erat <derat@chromium.org> Reviewed-by: Benson Leung <bleung@chromium.org> (cherry picked from commit 0b6967ecbec5aad49d8f6827b59f57df29b1d3c4) Reviewed-on: https://chromium-review.googlesource.com/481863 Reviewed-by: Dan Erat <derat@chromium.org> [modify] https://crrev.com/a174d0ef810f22338a3a496b4a0f794ac3ea5281/overlay-glimmer/make.conf [modify] https://crrev.com/a174d0ef810f22338a3a496b4a0f794ac3ea5281/overlay-pyro/make.conf [modify] https://crrev.com/a174d0ef810f22338a3a496b4a0f794ac3ea5281/overlay-eve/make.conf [modify] https://crrev.com/a174d0ef810f22338a3a496b4a0f794ac3ea5281/overlay-gru/profiles/base/make.defaults [modify] https://crrev.com/a174d0ef810f22338a3a496b4a0f794ac3ea5281/overlay-variant-veyron-minnie/make.conf [modify] https://crrev.com/a174d0ef810f22338a3a496b4a0f794ac3ea5281/overlay-bob/profiles/base/make.defaults [modify] https://crrev.com/a174d0ef810f22338a3a496b4a0f794ac3ea5281/overlay-hana/make.conf [modify] https://crrev.com/a174d0ef810f22338a3a496b4a0f794ac3ea5281/overlay-nasher/profiles/base/make.defaults [modify] https://crrev.com/a174d0ef810f22338a3a496b4a0f794ac3ea5281/overlay-elm/make.conf [modify] https://crrev.com/a174d0ef810f22338a3a496b4a0f794ac3ea5281/overlay-kevin/profiles/base/make.defaults [modify] https://crrev.com/a174d0ef810f22338a3a496b4a0f794ac3ea5281/overlay-snappy/make.conf [modify] https://crrev.com/a174d0ef810f22338a3a496b4a0f794ac3ea5281/overlay-reef/make.conf [modify] https://crrev.com/a174d0ef810f22338a3a496b4a0f794ac3ea5281/overlay-pbody/make.conf [modify] https://crrev.com/a174d0ef810f22338a3a496b4a0f794ac3ea5281/overlay-cyan/make.conf [modify] https://crrev.com/a174d0ef810f22338a3a496b4a0f794ac3ea5281/overlay-clapper/make.conf [modify] https://crrev.com/a174d0ef810f22338a3a496b4a0f794ac3ea5281/overlay-caroline/make.conf [modify] https://crrev.com/a174d0ef810f22338a3a496b4a0f794ac3ea5281/overlay-wizpig/make.conf [modify] https://crrev.com/a174d0ef810f22338a3a496b4a0f794ac3ea5281/overlay-cave/make.conf
,
Apr 19 2017
,
Apr 19 2017
I should mention that (unrelated to the touchpad-wakeup fix that got merged) there's still a single custom rules file at src/overlays/overlay-kevin/chromeos-base/chromeos-bsp-kevin/files/92-powerd-overrides.rules:
# Pen eject shouldn't wake system when lid is closed.
SUBSYSTEM=="input", DRIVERS=="gpio-keys", ENV{POWERD_TAGS_USABLE}="usable_when_laptop usable_when_tablet usable_when_display_off", ENV{POWERD_TAGS_WAKEUP}="wakeup wakeup_only_when_usable"
It looks like Brian added this for http://b/35585491. If we want to bring it in line with the way we manage other devices, we may want to add an e.g. internal_stylus_eject role tag and set usable/inhibit/wakeup tags for it in powerd's standard 91-powerd-tags.rules file. Do we have a way to identify stylus-eject devices across all boards, or will we need to continue using one-off rules like kevin's?
,
Apr 20 2017
Unfortunately, the solution cooked for Kevin isn't very generic at all. It's really only workable given that we know the only (relevant) wakesource using gpio-keys is the pen ejection GPIO. At the moment, I don't even think that driver has the granularity to assign different capabilities to its different keys. One problem here is that we tend to use gpio-keys as a bit of a hacky catch-all for any wakesource on a GPIO. So for instance, Kevin also has a (not yet supported, actually) Wake-on-Bluetooth GPIO using that same device. Fortunately, when we *actually* enable support for it, it will no longer be using gpio-keys (so the aforementioned rule is still OK). But it demonstrates the non-generic nature. I suppose if we really wanted to generic-ize this, we could enforce: (a) separate out all independent wake sources into different gpio-keys devices (b) assign some better form of labeling to these devices (e.g., device tree flags?), so we can write a better rule I don't know to what extent you would like to go to remove one-offs like this. I can put a little more thought in, if you think something like the above sounds worthwhile.
,
Apr 20 2017
Thanks for all the details! Unless we expect to need identical rules on other boards, keeping this as a one-off for kevin seems like the way to go.
,
Apr 24 2017
This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible! If all merges have been completed, please remove any remaining Merge-Approved labels from this issue. Thanks for your time! To disable nags, add the Disable-Nags label. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Apr 24 2017
,
May 18 2017
Re #21: gpio_keys does not really need to assign different capabilities for different keys within the same device, given that we can easily split gpios into any number of dveices. So stylus eject is one device, power button is another, and volume keys are the rd (with wakeup disabled) and so on. We just need to be mindful of use cases when we group gpios into logical input devices.
,
May 18 2017
@25: Right, that's what I said, isn't it? "(a) separate out all independent wake sources into different gpio-keys devices" (OK, I guess my comment was confusing because I started by saying that we have a problem because all gpio-keys are lumped in the same device.) We'd also still need a way to know the difference between them (my point (b) in that comment) if we really wanted to generic-ize this. Anyway, I think derat@ decided that it was fine to keep Kevin as a one-off.
,
May 19 2017
Yep, sounds fine to me.
,
May 30 2017
,
Aug 1 2017
,
Jan 22 2018
|
|||||||||||||
►
Sign in to add a comment |
|||||||||||||
Comment 1 by derat@chromium.org
, Apr 12 2017