Re: [PATCH v7 1/3] input: trackpoint - Enable doubletap by default on capable devices

[Vishnu Sankar]

Hi Ilpo,

Thank you so much for the review.

On Mon, Mar 9, 2026 at 5:01 PM Ilpo Järvinen
<ilpo.jarvinen@xxxxxxxxxxxxxxx> wrote:
>
> On Mon, 9 Feb 2026, Vishnu Sankar wrote:
>
> > Enable doubletap functionality by default on TrackPoint devices that
> > support it. The feature is detected using firmware ID pattern matching
> > (PNP: LEN03xxx) with a deny list of incompatible devices.
> >
> > This provides immediate doubletap functionality without requiring
> > userspace configuration. The hardware is enabled during device
> > detection, while event filtering continues to be handled by the
> > thinkpad_acpi driver as before.
> >
> > Signed-off-by: Vishnu Sankar <vishnuocv@xxxxxxxxx>
> > Suggested-by: Mark Pearson <mpearson-lenovo@xxxxxxxxx>
> > Acked-by: Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx>
> > ---
> > Changes in v7:
> > - Removed unwanted comments
> > - Removed psmouse_info ()
> >
> > Changes in v6:
> > - No Changes
> >
> > Changes in v5:
> > - Renamed function to trackpoint_is_dt_capable()
> > - Simplified string comparison without sscanf()
> > - Removed wrapper function as suggested
> > - Fixed missing period in comment
> >
> > Changes in v4:
> > - Simplified approach: removed all sysfs attributes and user interface
> > - Enable doubletap by default during device detection
> > - Removed global variables and complex attribute infrastructure
> > - Uses minimal firmware ID detection with deny list
> > - Follows KISS principle as suggested by reviewers
> >
> > Changes in v3:
> > - No changes
> >
> > 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
> > ---
> >  drivers/input/mouse/trackpoint.c | 45 ++++++++++++++++++++++++++++++++
> >  drivers/input/mouse/trackpoint.h |  5 ++++
> >  2 files changed, 50 insertions(+)
> >
> > diff --git a/drivers/input/mouse/trackpoint.c b/drivers/input/mouse/trackpoint.c
> > index 5f6643b69a2c..e12d76350252 100644
> > --- a/drivers/input/mouse/trackpoint.c
> > +++ b/drivers/input/mouse/trackpoint.c
> > @@ -393,6 +393,45 @@ 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 is "PNP: LEN030d PNP0f13".
> > + */
> > +static bool trackpoint_is_dt_capable(const char *pnp_id)
> > +{
> > +     size_t i;
> > +
> > +     if (!pnp_id)
> > +             return false;
> > +
> > +     /* Must start with "PNP: LEN03" */
> > +     if (!strstarts(pnp_id, "PNP: LEN03"))
>
> Missing include.
Sorry, I am a bit confused here:
strstarts() is already available through the existing
#include <linux/string.h> in thinkpad_acpi.c.

Do you think I should do anything else here?
>
> > +             return false;
> > +
> > +     /* Ensure enough length before comparing */
> > +     if (strlen(pnp_id) < 12)
> > +             return false;
> > +
> > +     /* Check deny-list */
> > +     for (i = 0; i < ARRAY_SIZE(dt_incompatible_devices); i++) {
>
> Missing include for ARRAY_SIZE().
Acked.
Will add linux/array_size.h to the include list.
>
> > +             if (!strncmp(pnp_id + 5,
> > +                          dt_incompatible_devices[i], 7))
>
> Fits to one line.
Understood.
will come up with v8 soon.
>
> > +                     return false;
> > +     }
> > +     return true;
> > +}
> > +
> >  int trackpoint_detect(struct psmouse *psmouse, bool set_properties)
> >  {
> >       struct ps2dev *ps2dev = &psmouse->ps2dev;
> > @@ -470,6 +509,12 @@ int trackpoint_detect(struct psmouse *psmouse, bool set_properties)
> >                    psmouse->vendor, firmware_id,
> >                    (button_info & 0xf0) >> 4, button_info & 0x0f);
> >
> > +     if (trackpoint_is_dt_capable(ps2dev->serio->firmware_id)) {
> > +             error = trackpoint_write(ps2dev, TP_DOUBLETAP, TP_DOUBLETAP_ENABLE);
> > +             if (error)
> > +                     psmouse_warn(psmouse, "Failed to enable doubletap: %d\n", error);
> > +     }
> > +
> >       return 0;
> >  }
> >
> > diff --git a/drivers/input/mouse/trackpoint.h b/drivers/input/mouse/trackpoint.h
> > index eb5412904fe0..3e03cdb39449 100644
> > --- a/drivers/input/mouse/trackpoint.h
> > +++ b/drivers/input/mouse/trackpoint.h
> > @@ -69,6 +69,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 */
> >
> > @@ -110,6 +112,9 @@
> >                                          external device will be forced to 1 */
> >  #define TP_MASK_EXT_TAG                      0x04
> >
> > +/* Doubletap register values */
> > +#define TP_DOUBLETAP_ENABLE  0xFF    /* Enable value */
> > +#define TP_DOUBLETAP_DISABLE 0xFE    /* Disable value */
> >
> >  /* Power on Self Test Results */
> >  #define TP_POR_SUCCESS               0x3B
> >
>
> --
>  i.
>


-- 

Regards,

      Vishnu Sankar