Message-ID: <CABxCQKtEcFozTtuV3sutU3OyobTbpA82Uy=MyU0FQePPT7S2Wg@mail.gmail.com>
Date: Thu, 18 Sep 2025 11:37:52 +0900
From: Vishnu Sankar <vishnuocv@...il.com>
To: dmitry.torokhov@...il.com, hmh@....eng.br, hansg@...nel.org, 
	ilpo.jarvinen@...ux.intel.com, derekjohn.clark@...il.com
Cc: mpearson-lenovo@...ebb.ca, linux-input@...r.kernel.org, 
	linux-kernel@...r.kernel.org, ibm-acpi-devel@...ts.sourceforge.net, 
	platform-driver-x86@...r.kernel.org, vsankar@...ovo.com
Subject: Re: [PATCH v3 1/3] input: mouse: trackpoint: Add doubletap
 enable/disable support

Hello all,

Do we have any questions or concerns?
Thanks in advance!

On Mon, Sep 1, 2025 at 10:53 PM Vishnu Sankar <vishnuocv@...il.com> wrote:
>
> Add support for enabling and disabling doubletap on TrackPoint devices
> that support this functionality. The feature is detected using firmware
> ID and exposed via sysfs as `doubletap_enabled`.
>
> The feature is only available on newer ThinkPads (2023 and later).The driver
> exposes this capability via a new sysfs attribute:
> "/sys/bus/serio/devices/seriox/doubletap_enabled".
>
> The attribute is only created if the device is detected to be capable of
> doubletap via firmware and variant ID checks. This functionality will be
> used by platform drivers such as thinkpad_acpi to expose and control doubletap
> via user interfaces.
>
> Signed-off-by: Vishnu Sankar <vishnuocv@...il.com>
> Suggested-by: Mark Pearson <mpearson-lenovo@...ebb.ca>
> Suggested-by: Dmitry Torokhov <dmitry.torokhov@...il.com>
> ---
> Changes in v2:
> - Improve commit messages
> - Sysfs attributes moved to trackpoint.c
> - Removed unnecessary comments
> - Removed unnecessary debug messages
> - Using strstarts() instead of strcmp()
> - is_trackpoint_dt_capable() modified
> - Removed _BIT suffix and used BIT() define.
> - Reverse the trackpoint_doubletap_status() logic to return error first.
> - Removed export functions as a result of the design change
> - Changed trackpoint_dev->psmouse to parent_psmouse
> - The path of trackpoint.h is not changed.
> Changes in v3:
> - No changes.
> ---
>  drivers/input/mouse/trackpoint.c | 149 +++++++++++++++++++++++++++++++
>  drivers/input/mouse/trackpoint.h |  15 ++++
>  2 files changed, 164 insertions(+)
>
> diff --git a/drivers/input/mouse/trackpoint.c b/drivers/input/mouse/trackpoint.c
> index 5f6643b69a2c..c6f17b0dec3a 100644
> --- a/drivers/input/mouse/trackpoint.c
> +++ b/drivers/input/mouse/trackpoint.c
> @@ -16,6 +16,8 @@
>  #include "psmouse.h"
>  #include "trackpoint.h"
>
> +static struct trackpoint_data *trackpoint_dev;
> +
>  static const char * const trackpoint_variants[] = {
>         [TP_VARIANT_IBM]                = "IBM",
>         [TP_VARIANT_ALPS]               = "ALPS",
> @@ -63,6 +65,21 @@ static int trackpoint_write(struct ps2dev *ps2dev, u8 loc, u8 val)
>         return ps2_command(ps2dev, param, MAKE_PS2_CMD(3, 0, TP_COMMAND));
>  }
>
> +/* Read function for TrackPoint extended registers */
> +static int trackpoint_extended_read(struct ps2dev *ps2dev, u8 loc, u8 *val)
> +{
> +       u8 ext_param[2] = {TP_READ_MEM, loc};
> +       int error;
> +
> +       error = ps2_command(ps2dev,
> +                           ext_param, MAKE_PS2_CMD(2, 1, TP_COMMAND));
> +
> +       if (!error)
> +               *val = ext_param[0];
> +
> +       return error;
> +}
> +
>  static int trackpoint_toggle_bit(struct ps2dev *ps2dev, u8 loc, u8 mask)
>  {
>         u8 param[3] = { TP_TOGGLE, loc, mask };
> @@ -393,6 +410,131 @@ static int trackpoint_reconnect(struct psmouse *psmouse)
>         return 0;
>  }
>
> +/* List of known incapable device PNP IDs */
> +static const char * const dt_incompatible_devices[] = {
> +       "LEN0304",
> +       "LEN0306",
> +       "LEN0317",
> +       "LEN031A",
> +       "LEN031B",
> +       "LEN031C",
> +       "LEN031D",
> +};
> +
> +/*
> + * checks if it’s a doubletap capable device
> + * The PNP ID format eg: is "PNP: LEN030d PNP0f13".
> + */
> +static bool is_trackpoint_dt_capable(const char *pnp_id)
> +{
> +       const char *id_start;
> +       char id[8];
> +
> +       if (!strstarts(pnp_id, "PNP: LEN03"))
> +               return false;
> +
> +       /* Points to "LEN03xxxx" */
> +       id_start = pnp_id + 5;
> +       if (sscanf(id_start, "%7s", id) != 1)
> +               return false;
> +
> +       /* Check if it's blacklisted */
> +       for (size_t i = 0; i < ARRAY_SIZE(dt_incompatible_devices); ++i) {
> +               if (strcmp(id, dt_incompatible_devices[i]) == 0)
> +                       return false;
> +       }
> +       return true;
> +}
> +
> +/* Trackpoint doubletap status function */
> +static int trackpoint_doubletap_status(bool *status)
> +{
> +       struct trackpoint_data *tp = trackpoint_dev;
> +       struct ps2dev *ps2dev = &tp->parent_psmouse->ps2dev;
> +       u8 reg_val;
> +       int rc;
> +
> +       /* Reading the Doubletap register using extended read */
> +       rc = trackpoint_extended_read(ps2dev, TP_DOUBLETAP, &reg_val);
> +       if (rc)
> +               return rc;
> +
> +       *status = reg_val & TP_DOUBLETAP_STATUS ? true : false;
> +
> +       return 0;
> +}
> +
> +/* Trackpoint doubletap enable/disable function */
> +static int trackpoint_set_doubletap(bool enable)
> +{
> +       struct trackpoint_data *tp = trackpoint_dev;
> +       struct ps2dev *ps2dev = &tp->parent_psmouse->ps2dev;
> +       static u8 doubletap_state;
> +       u8 new_val;
> +
> +       if (!tp)
> +               return -ENODEV;
> +
> +       new_val = enable ? TP_DOUBLETAP_ENABLE : TP_DOUBLETAP_DISABLE;
> +
> +       if (doubletap_state == new_val)
> +               return 0;
> +
> +       doubletap_state = new_val;
> +
> +       return trackpoint_write(ps2dev, TP_DOUBLETAP, new_val);
> +}
> +
> +/*
> + * Trackpoint Doubletap Interface
> + * Control/Monitoring of Trackpoint Doubletap from:
> + * /sys/bus/serio/devices/seriox/doubletap_enabled
> + */
> +static ssize_t doubletap_enabled_show(struct device *dev,
> +                               struct device_attribute *attr, char *buf)
> +{
> +       struct serio *serio = to_serio_port(dev);
> +       struct psmouse *psmouse = psmouse_from_serio(serio);
> +       struct trackpoint_data *tp = psmouse->private;
> +       bool status;
> +       int rc;
> +
> +       if (!tp || !tp->doubletap_capable)
> +               return -ENODEV;
> +
> +       rc = trackpoint_doubletap_status(&status);
> +       if (rc)
> +               return rc;
> +
> +       return sysfs_emit(buf, "%d\n", status ? 1 : 0);
> +}
> +
> +static ssize_t doubletap_enabled_store(struct device *dev,
> +                                       struct device_attribute *attr,
> +                                       const char *buf, size_t count)
> +{
> +       struct serio *serio = to_serio_port(dev);
> +       struct psmouse *psmouse = psmouse_from_serio(serio);
> +       struct trackpoint_data *tp = psmouse->private;
> +       bool enable;
> +       int err;
> +
> +       if (!tp || !tp->doubletap_capable)
> +               return -ENODEV;
> +
> +       err = kstrtobool(buf, &enable);
> +       if (err)
> +               return err;
> +
> +       err = trackpoint_set_doubletap(enable);
> +       if (err)
> +               return err;
> +
> +       return count;
> +}
> +
> +static DEVICE_ATTR_RW(doubletap_enabled);
> +
>  int trackpoint_detect(struct psmouse *psmouse, bool set_properties)
>  {
>         struct ps2dev *ps2dev = &psmouse->ps2dev;
> @@ -425,6 +567,9 @@ int trackpoint_detect(struct psmouse *psmouse, bool set_properties)
>         psmouse->reconnect = trackpoint_reconnect;
>         psmouse->disconnect = trackpoint_disconnect;
>
> +       trackpoint_dev = psmouse->private;
> +       trackpoint_dev->parent_psmouse = psmouse;
> +
>         if (variant_id != TP_VARIANT_IBM) {
>                 /* Newer variants do not support extended button query. */
>                 button_info = 0x33;
> @@ -470,6 +615,10 @@ int trackpoint_detect(struct psmouse *psmouse, bool set_properties)
>                      psmouse->vendor, firmware_id,
>                      (button_info & 0xf0) >> 4, button_info & 0x0f);
>
> +       tp->doubletap_capable = is_trackpoint_dt_capable(ps2dev->serio->firmware_id);
> +       if (tp->doubletap_capable)
> +               device_create_file(&psmouse->ps2dev.serio->dev, &dev_attr_doubletap_enabled);
> +
>         return 0;
>  }
>
> diff --git a/drivers/input/mouse/trackpoint.h b/drivers/input/mouse/trackpoint.h
> index eb5412904fe0..256e8cb35581 100644
> --- a/drivers/input/mouse/trackpoint.h
> +++ b/drivers/input/mouse/trackpoint.h
> @@ -8,6 +8,8 @@
>  #ifndef _TRACKPOINT_H
>  #define _TRACKPOINT_H
>
> +#include <linux/bitops.h>
> +
>  /*
>   * These constants are from the TrackPoint System
>   * Engineering documentation Version 4 from IBM Watson
> @@ -69,6 +71,8 @@
>                                         /* (how hard it is to drag */
>                                         /* with Z-axis pressed) */
>
> +#define TP_DOUBLETAP           0x58    /* TrackPoint doubletap register */
> +
>  #define TP_MINDRAG             0x59    /* Minimum amount of force needed */
>                                         /* to trigger dragging */
>
> @@ -139,6 +143,14 @@
>  #define TP_DEF_TWOHAND         0x00
>  #define TP_DEF_SOURCE_TAG      0x00
>
> +/* Doubletap register values */
> +#define TP_DOUBLETAP_ENABLE    0xFF    /* Enable value */
> +#define TP_DOUBLETAP_DISABLE   0xFE    /* Disable value */
> +
> +#define TP_DOUBLETAP_STATUS_BIT 0      /* 0th bit defines enable/disable */
> +
> +#define TP_DOUBLETAP_STATUS   BIT(TP_DOUBLETAP_STATUS_BIT)
> +
>  #define MAKE_PS2_CMD(params, results, cmd) ((params<<12) | (results<<8) | (cmd))
>
>  struct trackpoint_data {
> @@ -150,11 +162,14 @@ struct trackpoint_data {
>         u8 thresh, upthresh;
>         u8 ztime, jenks;
>         u8 drift_time;
> +       bool doubletap_capable;
>
>         /* toggles */
>         bool press_to_select;
>         bool skipback;
>         bool ext_dev;
> +
> +       struct psmouse *parent_psmouse;
>  };
>
>  int trackpoint_detect(struct psmouse *psmouse, bool set_properties);
> --
> 2.48.1
>


-- 

Regards,

      Vishnu Sankar
     +817015150407 (Japan)

Powered by blists - more mailing lists