Hi Mark,

On 3/24/24 10:07 PM, Mark Pearson wrote:
> Lenovo trackpoints are adding the ability to generate a doubletap event.
> This handles the doubletap event and sends the KEY_DOUBLECLICK event to
> userspace.
> 
> Signed-off-by: Mark Pearson <mpearson-len...@squebb.ca>
> Signed-off-by: Vishnu Sankar <vsan...@lenovo.com>
> ---
>  drivers/platform/x86/thinkpad_acpi.c | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/drivers/platform/x86/thinkpad_acpi.c 
> b/drivers/platform/x86/thinkpad_acpi.c
> index 82429e59999d..2bbb32c898e9 100644
> --- a/drivers/platform/x86/thinkpad_acpi.c
> +++ b/drivers/platform/x86/thinkpad_acpi.c
> @@ -232,6 +232,7 @@ enum tpacpi_hkey_event_t {
>  
>       /* Misc */
>       TP_HKEY_EV_RFKILL_CHANGED       = 0x7000, /* rfkill switch changed */
> +     TP_HKEY_EV_TRACKPOINT_DOUBLETAP = 0x8036, /* doubletap on Trackpoint*/
>  };
>  
>  /****************************************************************************
> @@ -4081,6 +4082,22 @@ static void hotkey_notify(struct ibm_struct *ibm, u32 
> event)
>                               break;
>                       }
>                       fallthrough;    /* to default */

This now no longer fallsthrough to default. IMHO the best thing to do
here is add a new preparation patch which initializes known_ev to false
inside the while before the switch-case (together with the send_acpi_ev
and ignore_acpi_ev init). and then change this fallthrough to a break
in the preparation patch. You can then also remove the default case
altogether in this prep patch.

> +             case 8:
> +                     /* 0x8036: Trackpoint doubletaps */
> +                     if (hkey == TP_HKEY_EV_TRACKPOINT_DOUBLETAP) {
> +                             send_acpi_ev = true;
> +                             ignore_acpi_ev = false;

These 2 values are set as the default above the switch-case, please
drop these 2 lines.

> +                             known_ev = true;
> +                             /* Send to user space */
> +                             mutex_lock(&tpacpi_inputdev_send_mutex);
> +                             input_report_key(tpacpi_inputdev, 
> KEY_DOUBLECLICK, 1);
> +                             input_sync(tpacpi_inputdev);
> +                             input_report_key(tpacpi_inputdev, 
> KEY_DOUBLECLICK, 0);
> +                             input_sync(tpacpi_inputdev);
> +                             mutex_unlock(&tpacpi_inputdev_send_mutex);

This code duplicates tpacpi_input_send_key(), what you want to do here
is define a hotkey_keycode_map scancode range for new 0x8xxx codes like how this
was done when extended scancodes where added to deal with the new 0x13xx hotkey
event codes for the 2017+ models.

See commit 696c6523ec8f ("platform/x86: thinkpad_acpi: add mapping for new 
hotkeys")

Despite re-using tpacpi_input_send_key() there are 2 reasons why we want
scancodes for these new "keys".

1. By adding the keys to the hotkey_keycode_map they automatically
also get input_set_capability(tpacpi_inputdev, EV_KEY, hotkey_keycode_map[i]);
called on them advertising to userspace that tpacpi_inputdev can actually
generate these keypresses. Something which is currently lacking from your
patch. Related to this did you test this with evtest? I think that the input
core will suppress the events when you do not set the capability ?

2. This allows remapping scancodes to different KEY_foo values with hwdb
entries.

Regards,

Hans








> +                             break;
> +                     }
> +                     fallthrough;    /* to default */
>               default:
>                       known_ev = false;
>               }



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

Reply via email to