On Mon, Apr 29, 2024, at 5:57 AM, Hans de Goede wrote: > Hi Mark, > > On 4/24/24 8:19 PM, Mark Pearson wrote: >> Hi Hans, >> >> On Wed, Apr 24, 2024, at 8:28 AM, Hans de Goede wrote: >>> From: Mark Pearson <mpearson-lenovo@xxxxxxxxx> >>> >>> Lenovo trackpoints are adding the ability to generate a doubletap event. >>> This handles the doubletap event and sends the KEY_PROG4 event to >>> userspace. Despite the driver itself not using KEY_PROG1 - KEY_PROG3 this >>> still uses KEY_PROG4 because of some keys being remapped to KEY_PROG1 - >>> KEY_PROG3 by default by the upstream udev hwdb containing: >>> >>> evdev:name:ThinkPad Extra Buttons:dmi:bvn*:bvr*:bd*:svnLENOVO*:pn*:* >>> ... >>> KEYBOARD_KEY_17=prog1 >>> KEYBOARD_KEY_1a=f20 # Microphone mute button >>> KEYBOARD_KEY_45=bookmarks >>> KEYBOARD_KEY_46=prog2 # Fn + PrtSc, on Windows: Snipping tool >>> KEYBOARD_KEY_4a=prog3 # Fn + Right shift, on Windows: No idea >>> >>> Signed-off-by: Mark Pearson <mpearson-lenovo@xxxxxxxxx> >>> Signed-off-by: Vishnu Sankar <vishnuocv@xxxxxxxxx> >>> Link: https://lore.kernel.org/r/20240417173124.9953-2-mpearson-lenovo@xxxxxxxxx >>> [hdegoede@xxxxxxxxxx: Adjust for switch to sparse-keymap keymaps] >>> Tested-by: Mark Pearson <mpearson-lenovo@xxxxxxxxx> >>> Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx> >>> --- >>> drivers/platform/x86/thinkpad_acpi.c | 19 +++++++++++++++++++ >>> 1 file changed, 19 insertions(+) >>> >>> diff --git a/drivers/platform/x86/thinkpad_acpi.c >>> b/drivers/platform/x86/thinkpad_acpi.c >>> index a53b00fecf1a..b6d6466215e1 100644 >>> --- a/drivers/platform/x86/thinkpad_acpi.c >>> +++ b/drivers/platform/x86/thinkpad_acpi.c >>> @@ -248,6 +248,9 @@ enum tpacpi_hkey_event_t { >>> >>> /* Misc */ >>> TP_HKEY_EV_RFKILL_CHANGED = 0x7000, /* rfkill switch changed */ >>> + >>> + /* Misc2 */ >>> + TP_HKEY_EV_TRACK_DOUBLETAP = 0x8036, /* trackpoint doubletap */ >>> }; >>> >>> >>> /**************************************************************************** >>> @@ -3268,6 +3271,7 @@ static const struct key_entry keymap_lenovo[] >>> __initconst = { >>> * after switching to sparse keymap support. The mappings above use >>> translated >>> * scancodes to preserve uAPI compatibility, see >>> tpacpi_input_send_key(). >>> */ >>> + { KE_KEY, TP_HKEY_EV_TRACK_DOUBLETAP /* 0x8036 */, { KEY_PROG4 } }, >>> { KE_END } >>> }; >>> >>> @@ -3817,6 +3821,17 @@ static bool hotkey_notify_6xxx(const u32 hkey, >>> bool *send_acpi_ev) >>> return true; >>> } >>> >>> +static bool hotkey_notify_8xxx(const u32 hkey, bool *send_acpi_ev) >>> +{ >>> + switch (hkey) { >>> + case TP_HKEY_EV_TRACK_DOUBLETAP: >>> + tpacpi_input_send_key(hkey, send_acpi_ev); >>> + return true; >>> + default: >>> + return false; >>> + } >>> +} >>> + >>> static void hotkey_notify(struct ibm_struct *ibm, u32 event) >>> { >>> u32 hkey; >>> @@ -3893,6 +3908,10 @@ static void hotkey_notify(struct ibm_struct >>> *ibm, u32 event) >>> known_ev = true; >>> } >>> break; >>> + case 8: >>> + /* 0x8000-0x8FFF: misc2 */ >>> + known_ev = hotkey_notify_8xxx(hkey, &send_acpi_ev); >>> + break; >>> } >>> if (!known_ev) { >>> pr_notice("unhandled HKEY event 0x%04x\n", hkey); >>> -- >>> 2.44.0 >> >> Instead of needing hotkey_notify_8xxx, now we are using the sparse_keymap can we just use hotkey_notify_hotkey for case 8? No need to check what the hkey is either. > > I prefer to keep things consistent and have each case #: path call a separate > helper for those #xxx codes. > > ATM some of the simpler cases handle things directly, but as more > handling for > different events get added that becomes a bit messy IMHO. I would > actually > like to see those other cases converted to use a small helper function > too > (with a switch-case in the helper for future proofing) to make things > consistent. > Got it - no problem. > Patches to do this small cleanup are welcome. > Sounds good. Will look at doing this. Mark
