Re: [ibm-acpi-devel] [PATCH v2 22/24] platform/x86: thinkpad_acpi: Support for trackpoint doubletap

[Mark P. <mpe...@sq...>]

Hi Hans,

On Wed, Apr 24, 2024, at 8:28 AM, Hans de Goede wrote:
> From: Mark Pearson <mpe...@sq...>
>
> 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 <mpe...@sq...>
> Signed-off-by: Vishnu Sankar <vis...@gm...>
> Link: https://lore.kernel.org/r/202...@sq...
> [hde...@re...: Adjust for switch to sparse-keymap keymaps]
> Tested-by: Mark Pearson <mpe...@sq...>
> Signed-off-by: Hans de Goede <hde...@re...>
> ---
>  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.

Note - the future patch for the FN+G key (patch 24/24) will also need modifying as we'll need to move the check if the doubletap has been disabled for the doubletap hkey, but it's two lines so still seems cleaner overall.

I prototyped it on my system, and it worked.

Mark