Re: [ibm-acpi-devel] [PATCH v4 1/3] input: trackpoint - Enable doubletap by default on capable devices

[Dmitry T. <dmi...@gm...>]

Hi Vishnu,

On Sat, Nov 29, 2025 at 09:25:31AM +0900, 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 <vis...@gm...>
> Suggested-by: Mark Pearson <mpe...@sq...>
> ---
> 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 | 51 ++++++++++++++++++++++++++++++++
>  drivers/input/mouse/trackpoint.h |  5 ++++
>  2 files changed, 56 insertions(+)
> 
> diff --git a/drivers/input/mouse/trackpoint.c b/drivers/input/mouse/trackpoint.c
> index 5f6643b69a2c..67144c27bccd 100644
> --- a/drivers/input/mouse/trackpoint.c
> +++ b/drivers/input/mouse/trackpoint.c
> @@ -393,6 +393,48 @@ 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

Please finish the sentence with a period.

> + * The PNP ID format is "PNP: LEN030d PNP0f13".
> + */
> +static bool is_trackpoint_dt_capable(const char *pnp_id)

Let's call it trackpoint_is_dt_capable() to keep with common
"trackpoint_" prefix in the file.

> +{
> +	const char *id_start;
> +	char id[8];
> +	size_t i;
> +
> +	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 in the deny list */
> +	for (i = 0; i < ARRAY_SIZE(dt_incompatible_devices); i++) {
> +		if (strcmp(id, dt_incompatible_devices[i]) == 0)

Why can't we use strncmp(pnp_id + 5, dt_incompatible_devices[i], 7) here
(after ensuring that pnp_id is of sufficient length to begin with) and
avoid sscanf()?

> +			return false;
> +	}
> +	return true;
> +}
> +
> +static int trackpoint_set_doubletap(struct ps2dev *ps2dev, bool enable)
> +{
> +	return trackpoint_write(ps2dev, TP_DOUBLETAP, enable ? TP_DOUBLETAP_ENABLE : TP_DOUBLETAP_DISABLE);
> +}

This wrapper seems an overkill given that it is called only once and
always to enable the doubletap.

Thanks.

-- 
Dmitry