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-len...@squebb.ca>
> Signed-off-by: Vishnu Sankar <vishnu...@gmail.com>
> ---
> 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);



_______________________________________________
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel

Reply via email to