Hi Mark,
Thank you for the new version of this series, overall this looks good,
one small remark below.
On 4/17/24 7:31 PM, Mark Pearson wrote:
> Lenovo trackpoints are adding the ability to generate a doubletap event.
> This handles the doubletap event and sends the KEY_PROG1 event to
> userspace.
>
> Signed-off-by: Mark Pearson <mpearson-lenovo@xxxxxxxxx>
> Signed-off-by: Vishnu Sankar <vishnuocv@xxxxxxxxx>
> ---
> Changes in v2:
> - Use KEY_PROG1 instead of KEY_DOUBLETAP as input maintainer doesn't
> want new un-specific key codes added.
> - Add doubletap to hotkey scan code table and use existing hotkey
> functionality.
> - Tested using evtest, and then gnome settings to configure a custom shortcut
> to launch an application.
>
> drivers/platform/x86/thinkpad_acpi.c | 18 ++++++++++++++++++
> 1 file changed, 18 insertions(+)
>
> diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
> index 3b48d893280f..6d04d45e8d45 100644
> --- a/drivers/platform/x86/thinkpad_acpi.c
> +++ b/drivers/platform/x86/thinkpad_acpi.c
> @@ -232,6 +232,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 */
> };
>
> /****************************************************************************
> @@ -1786,6 +1789,7 @@ enum { /* hot key scan codes (derived from ACPI DSDT) */
> TP_ACPI_HOTKEYSCAN_NOTIFICATION_CENTER,
> TP_ACPI_HOTKEYSCAN_PICKUP_PHONE,
> TP_ACPI_HOTKEYSCAN_HANGUP_PHONE,
I understand why you've done this but I think this needs a comment,
something like:
/*
* For TP_HKEY_EV_TRACK_DOUBLETAP, unlike the codes above which map to:
* (hkey_event - 0x1300) + TP_ACPI_HOTKEYSCAN_EXTENDED_START, this is
* hardcoded for TP_HKEY_EV_TRACK_DOUBLETAP handling. Therefor this must
* always be the last entry (after any 0x1300-0x13ff entries).
*/
+ TP_ACPI_HOTKEYSCAN_DOUBLETAP,
I see you got adding the new 0x13xx related hkeyscancodes right in the next
patch in this series but I think such a comment as above will be helpful
for future patches.
If you agree with adding this comment I can add this while merging, no need
to send a new version just for this.
Regards,
Hans
> /* Hotkey keymap size */
> TPACPI_HOTKEY_MAP_LEN
> @@ -3336,6 +3340,7 @@ static int __init hotkey_init(struct ibm_init_struct *iibm)
> KEY_NOTIFICATION_CENTER, /* Notification Center */
> KEY_PICKUP_PHONE, /* Answer incoming call */
> KEY_HANGUP_PHONE, /* Decline incoming call */
> + KEY_PROG1, /* Trackpoint doubletap */
> },
> };
>
> @@ -3996,6 +4001,15 @@ static bool hotkey_notify_6xxx(const u32 hkey,
> return true;
> }
>
> +static bool hotkey_notify_8xxx(const u32 hkey)
> +{
> + if (hkey == TP_HKEY_EV_TRACK_DOUBLETAP) {
> + tpacpi_input_send_key(TP_ACPI_HOTKEYSCAN_DOUBLETAP);
> + return true;
> + }
> + return false;
> +}
> +
> static void hotkey_notify(struct ibm_struct *ibm, u32 event)
> {
> u32 hkey;
> @@ -4079,6 +4093,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);
> + break;
> }
> if (!known_ev) {
> pr_notice("unhandled HKEY event 0x%04x\n", hkey);
