Message-ID: <cr7jgtztf65balwxu6cpu6hqzzzluitrwu2f66o75kcip5k2zd@sxixvhotead5>
Date: Wed, 17 Sep 2025 21:57:11 -0700
From: Dmitry Torokhov <dmitry.torokhov@...il.com>
To: Vishnu Sankar <vishnuocv@...il.com>
Cc: hmh@....eng.br, hansg@...nel.org, ilpo.jarvinen@...ux.intel.com, 
	derekjohn.clark@...il.com, 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

Hi Vishnu,

On Mon, Sep 01, 2025 at 10:53:05PM +0900, Vishnu Sankar 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;

Please do not use globals.

> +
>  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);

Please use existing facilities in psmouse driver to define and register
protocol-specific attributes. Use is_visible() to control whether the
attribute is accessible or not.

Thanks.

-- 
Dmitry

Powered by blists - more mailing lists