On Sun, Apr 21, 2024, at 1:17 PM, Mark Pearson wrote: > Thanks Hans! > > On Sun, Apr 21, 2024, at 11:44 AM, Hans de Goede wrote: >> Hi All, >> >> My reply in the "[PATCH v2 1/4] platform/x86: thinkpad_acpi: >> simplify known_ev handling" handling where I indicated that I would work >> on converting the thinkpad_acpi hotkey handling to use sparse-keymaps >> underestimated the work this required quite a bit. >> >> The hotkey code is quite old and crufty and full of special cases to >> support many generations of ThinkPads, so I ended up doing some >> significant refactoring and cleanup before doing the actual conversion >> to sparse-keymaps. >> >> I have been vary careful to not introduce any changes wrt support for >> the original ThinkPad models / hotkeys which use the hotkey_*_mask >> related code. >> >> I've also done my best to avoid any *significant* functional change but >> there are still some functional changes, which should all not impact >> userspace compatibility: >> >> 1. Adaptive keyboard special keys will now also send EV_MSC events with >> the scancode, just like all the other hotkeys. >> >> 2. Rely on the input core for KEY_RESERVED suppression. This means that >> keys marked as KEY_RESERVED will still send EV_MSC evdev events when >> pressed (which helps users with mapping them to non reserved KEY_FOO >> values if desired). >> >> 3. Align the keycodes for volume up/down/mute and brightness up/down with >> those set by userspace through udev upstream's hwdb. Note these are all >> for keys which are suppressed by hotkey_reserved_mask by default. >> So this is only a functional change for users who override the default >> hotkey-mask *and* who do not have udev's default hwdb installed. >> >> 4. Suppress ACPI netlink event generation for unknown 0x1xxx hkey events to >> avoid userspace starting to rely on the netlink events for new hotkeys >> before these have been added to the keymap, only to have the netlink >> events get disabled by the adding of the new hotkeys to the keymap. >> >> This should not cause a behavior change for existing models since all >> currently known 0x1xxx events have a mapping. >> >> Here is a quick breakdown of the patches in this series: >> >> Patch 1 - 2: Fix a small locking issue on rmmod the only problem here >> really is a lockdep warning, so I plan to route these fixes through >> for-next together with the rest to keep things simple. >> >> Patch 3 - 14: Do a bunch of cleanups and refactoring >> >> Patch 15: Implements functional change no 4. I really kinda want to just >> completely disable ACPI netlink event generation when also sending evdev >> events instead of reporting these twice. But for the 0x11xx / 0x13xx >> hkey events the kernel has send ACPI netlink events for years now. So >> this disables ACPI netlink events for any new hotkeys going forward. >> >> Patch 16 - 18: Refactor / cleanup reserved key handling >> >> Patch 19: Actually move to sparse-keymaps >> >> Patch 20: Update the keymap for 2 adaptive kbd Fn row keys >> >> Patch 21 - 24: Mark's original patches adding support for the new Fn + N >> key combo and for trackpoint doubletap slightly reworked to use >> the new sparse-keymap. >> >> Mark if you can make some time to review patches 1-20 that would be great. >> > Definitely will do. > > I'll do some testing on platforms here as much as I can. If there's > anything in particular that you think is worth taking extra care over > let me know (with a caveat that my team doesn't have all the platforms, > and for anything older than 5 years it can be hard to get hold of them) > > Thanks for your work on this. > > Mark > I've tested the series on a couple of platforms (Z13 G2 and X1 Carbon G12) and so far all looking good. - tried all hotkey combinations that are supported that I can think of and they work - including brightness control, volume control, platform profile toggle, airplane, snipping tool and privacy screen. - trackpoint double tap is working well on the Z13 G2 (set up custom key setting in gnome-settings and launched browser on a doubletap) - FN+N is working well on the X1 Carbon G12 (tested with evtest to confirm vendor key generated) So for the series: Tested-by: Mark Pearson <mpearson-lenovo@xxxxxxxxx> I have read through the patches, but not in enough depth for it to count as a review yet. Need a bit more time for that. Mark
