Re: [PATCH v2] platform/x86: ideapad-laptop: do not poke keyboard controller

[Mark Pearson]

On Wed, Aug 5, 2026, at 2:21 AM, Dmitry Torokhov wrote:
> On Ideapad Z570 the driver tries to disable and re-enable data coming
> from the touchpad by poking directly into 8042 keyboard controller.
> This may coincide with the controller resuming and leads to spews in
> dmesg and potentially other instabilities.
>
> Instead of using i8042_command() to control the touchpad state create
> an input handler that serves as a filter and drop events coming from
> the touchpad when it is supposed to be off.
>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx>
> ---
>
> v2:
> - fix inverted condition in ideapad_tpswitch_filter()
> - mark the handler as passive_observer so that touchpad is not activated
>   until there are real users
> - use READ_ONCE()/WRITE_ONCE() to access shared state flag
> - drop stray semicolon in to_tpswitch_handle()
> - update comments
>
>  drivers/platform/x86/lenovo/ideapad-laptop.c | 184 ++++++++++++++++++-
>  1 file changed, 175 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/platform/x86/lenovo/ideapad-laptop.c 
> b/drivers/platform/x86/lenovo/ideapad-laptop.c
> index 4fbc904f1fc3..8b4c41073b82 100644
> --- a/drivers/platform/x86/lenovo/ideapad-laptop.c
> +++ b/drivers/platform/x86/lenovo/ideapad-laptop.c
> @@ -18,7 +18,6 @@
>  #include <linux/delay.h>
>  #include <linux/device.h>
>  #include <linux/dmi.h>
> -#include <linux/i8042.h>
>  #include <linux/init.h>
>  #include <linux/input.h>
>  #include <linux/input/sparse-keymap.h>
> @@ -210,6 +209,13 @@ struct ideapad_private {
>  		struct led_classdev led;
>  		unsigned int last_brightness;
>  	} fn_lock;
> +	struct {
> +		bool initialized;
> +		bool active; /* true means touchpad is enabled */
> +		struct input_handler handler;
> +		struct input_dev *tp_dev;
> +		spinlock_t lock;
> +	} tp_switch;
>  };
> 
>  static bool no_bt_rfkill;
> @@ -234,7 +240,7 @@ MODULE_PARM_DESC(set_fn_lock_led,
>  static bool ctrl_ps2_aux_port;
>  module_param(ctrl_ps2_aux_port, bool, 0444);
>  MODULE_PARM_DESC(ctrl_ps2_aux_port,
> -		 "Enable driver based PS/2 aux port en-/dis-abling on touchpad 
> on/off toggle. If you need this please report this to: 
> platform-driver-x86@xxxxxxxxxxxxxxx");
> +		 "Enable driver based suppression of touchpad events on touchpad 
> on/off toggle. If you need this please report this to: 
> platform-driver-x86@xxxxxxxxxxxxxxx");
> 
>  static bool touchpad_ctrl_via_ec;
>  module_param(touchpad_ctrl_via_ec, bool, 0444);
> @@ -1462,6 +1468,160 @@ static void 
> ideapad_check_special_buttons(struct ideapad_private *priv)
>  	}
>  }
> 
> +struct ideapad_tpswitch_handle {
> +	struct input_handle handle;
> +	struct ideapad_private *priv;
> +};
> +
> +#define to_tpswitch_handle(h) \
> +	container_of(h, struct ideapad_tpswitch_handle, handle)
> +
> +static int ideapad_tpswitch_connect(struct input_handler *handler,
> +				    struct input_dev *dev,
> +				    const struct input_device_id *id)
> +{
> +	struct ideapad_private *priv =
> +		container_of(handler, struct ideapad_private, tp_switch.handler);
> +	struct ideapad_tpswitch_handle *h;
> +	int error;
> +
> +	h = kzalloc_obj(*h);
> +	if (!h)
> +		return -ENOMEM;
> +
> +	h->priv = priv;
> +	h->handle.dev = dev;
> +	h->handle.handler = handler;
> +	h->handle.name = "ideapad-tpswitch";
> +
> +	error = input_register_handle(&h->handle);
> +	if (error)
> +		goto err_free_handle;
> +
> +	/*
> +	 * We need to open the input handle so the input core invokes our
> +	 * filter, but since we mark our handler as passive observer, this
> +	 * will not cause the underlying touchpad hardware to be started
> +	 * if there are no other users.
> +	 */
> +	error = input_open_device(&h->handle);
> +	if (error)
> +		goto err_unregister_handle;
> +
> +	scoped_guard(spinlock_irq, &priv->tp_switch.lock)
> +		priv->tp_switch.tp_dev = dev;
> +
> +	return 0;
> +
> +err_unregister_handle:
> +	input_unregister_handle(&h->handle);
> +err_free_handle:
> +	kfree(h);
> +	return error;
> +}
> +
> +static void ideapad_tpswitch_disconnect(struct input_handle *handle)
> +{
> +	struct ideapad_tpswitch_handle *h = to_tpswitch_handle(handle);
> +	struct ideapad_private *priv = h->priv;
> +
> +	scoped_guard(spinlock_irq, &priv->tp_switch.lock)
> +		priv->tp_switch.tp_dev = NULL;
> +
> +	input_close_device(handle);
> +	input_unregister_handle(handle);
> +	kfree(h);
> +}
> +
> +static bool ideapad_tpswitch_filter(struct input_handle *handle,
> +				    unsigned int type, unsigned int code,
> +				    int value)
> +{
> +	struct ideapad_tpswitch_handle *h = to_tpswitch_handle(handle);
> +	struct ideapad_private *priv = h->priv;
> +
> +	if (READ_ONCE(priv->tp_switch.active))
> +		return false;
> +
> +	/* Allow passing button release events, drop everything else */
> +	return !(type == EV_KEY && value == 0) &&
> +	       !(type == EV_SYN && code == SYN_REPORT);
> +

Nit - extra empty line

> +}
> +
> +static const struct input_device_id ideapad_tpswitch_ids[] = {
> +	{
> +		.flags = INPUT_DEVICE_ID_MATCH_EVBIT |
> +				INPUT_DEVICE_ID_MATCH_KEYBIT |
> +				INPUT_DEVICE_ID_MATCH_ABSBIT,
> +		.bustype = BUS_I8042,
> +		.vendor = 0x0002,
> +		.evbit = { BIT_MASK(EV_KEY) | BIT_MASK(EV_ABS) },
> +		.keybit = { [BIT_WORD(BTN_TOOL_FINGER)] =
> +				BIT_MASK(BTN_TOOL_FINGER) },
> +		.absbit = { BIT_MASK(ABS_X) | BIT_MASK(ABS_Y) |
> +				BIT_MASK(ABS_PRESSURE) |
> +				BIT_MASK(ABS_TOOL_WIDTH) },
> +	},
> +	{ }
> +};
> +
> +static int ideapad_tpswitch_init(struct ideapad_private *priv)
> +{
> +	int error;
> +
> +	if (!priv->features.ctrl_ps2_aux_port)
> +		return 0;
> +
> +	spin_lock_init(&priv->tp_switch.lock);
> +
> +	priv->tp_switch.handler.name = "ideapad-tpswitch";
> +	priv->tp_switch.handler.id_table = ideapad_tpswitch_ids;
> +	priv->tp_switch.handler.filter = ideapad_tpswitch_filter;
> +	priv->tp_switch.handler.connect = ideapad_tpswitch_connect;
> +	priv->tp_switch.handler.disconnect = ideapad_tpswitch_disconnect;
> +	priv->tp_switch.handler.passive_observer = true;
> +
> +	error = input_register_handler(&priv->tp_switch.handler);
> +	if (error) {
> +		dev_err(&priv->platform_device->dev,
> +			"failed to register touchpad switch handler: %d",
> +			error);
> +		return error;
> +	}
> +
> +	priv->tp_switch.initialized = true;
> +	return 0;
> +}
> +
> +static void ideapad_tpswitch_exit(struct ideapad_private *priv)
> +{
> +	if (priv->tp_switch.initialized) {
> +		input_unregister_handler(&priv->tp_switch.handler);
> +		priv->tp_switch.initialized = false;
> +	}
> +}
> +
> +static void ideapad_tpswitch_toggle(struct ideapad_private *priv, bool 
> on)
> +{
> +	guard(spinlock_irq)(&priv->tp_switch.lock);
> +
> +	WRITE_ONCE(priv->tp_switch.active, on);
> +	if (!on) {
> +		struct input_dev *tp_dev = priv->tp_switch.tp_dev;
> +		if (tp_dev) {
> +			input_report_key(tp_dev, BTN_TOUCH, 0);
> +			input_report_key(tp_dev, BTN_TOOL_FINGER, 0);
> +			input_report_key(tp_dev, BTN_TOOL_DOUBLETAP, 0);
> +			input_report_key(tp_dev, BTN_TOOL_TRIPLETAP, 0);
> +			input_report_key(tp_dev, BTN_LEFT, 0);
> +			input_report_key(tp_dev, BTN_RIGHT, 0);
> +			input_report_key(tp_dev, BTN_MIDDLE, 0);
> +			input_sync(tp_dev);
> +		}
> +	}
> +}
> +
>  /*
>   * backlight
>   */
> @@ -1799,7 +1959,6 @@ static void ideapad_fn_lock_led_exit(struct 
> ideapad_private *priv)
>  static void ideapad_sync_touchpad_state(struct ideapad_private *priv, 
> bool send_events)
>  {
>  	unsigned long value;
> -	unsigned char param;
>  	int ret;
> 
>  	/* Without reading from EC touchpad LED doesn't switch state */
> @@ -1810,12 +1969,11 @@ static void ideapad_sync_touchpad_state(struct 
> ideapad_private *priv, bool send_
> 
>  	/*
>  	 * Some IdeaPads don't really turn off touchpad - they only
> -	 * switch the LED state. We (de)activate KBC AUX port to turn
> -	 * touchpad off and on. We send KEY_TOUCHPAD_OFF and
> -	 * KEY_TOUCHPAD_ON to not to get out of sync with LED
> +	 * switch the LED state. On such devices we use an input filter to
> +	 * suppress events from the touchpad.
>  	 */
>  	if (priv->features.ctrl_ps2_aux_port)
> -		i8042_command(&param, value ? I8042_CMD_AUX_ENABLE : I8042_CMD_AUX_DISABLE);
> +		ideapad_tpswitch_toggle(priv, value);
> 
>  	/*
>  	 * On older models the EC controls the touchpad and toggles it on/off
> @@ -2018,8 +2176,8 @@ static const struct dmi_system_id hw_rfkill_list[] = {
>  /*
>   * On some models the EC toggles the touchpad muted LED on touchpad toggle
>   * hotkey presses, but the EC does not actually disable the touchpad itself.
> - * On these models the driver needs to explicitly enable/disable the i8042
> - * (PS/2) aux port.
> + * On these models the driver needs to explicitly suppress data coming from
> + * the touchpad.
>   */
>  static const struct dmi_system_id ctrl_ps2_aux_port_list[] = {
>  	{
> @@ -2402,6 +2560,10 @@ static int ideapad_acpi_add(struct platform_device *pdev)
>  	if (err)
>  		goto input_failed;
> 
> +	err = ideapad_tpswitch_init(priv);
> +	if (err)
> +		goto tpswitch_failed;
> +
>  	err = ideapad_kbd_bl_init(priv);
>  	if (err) {
>  		if (err != -ENODEV)
> @@ -2478,6 +2640,9 @@ static int ideapad_acpi_add(struct platform_device *pdev)
> 
>  	ideapad_fn_lock_led_exit(priv);
>  	ideapad_kbd_bl_exit(priv);
> +	ideapad_tpswitch_exit(priv);
> +
> +tpswitch_failed:
>  	ideapad_input_exit(priv);
> 
>  input_failed:
> @@ -2507,6 +2672,7 @@ static void ideapad_acpi_remove(struct 
> platform_device *pdev)
> 
>  	ideapad_fn_lock_led_exit(priv);
>  	ideapad_kbd_bl_exit(priv);
> +	ideapad_tpswitch_exit(priv);
>  	ideapad_input_exit(priv);
>  	ideapad_debugfs_exit(priv);
>  }
> -- 

Looks good to me. Cool folk are using a 14 year old Ideapad (tends to be more common on the Thinkpads). Go Linux!

Reviewed-by: Mark Pearson <mpearson-lenovo@xxxxxxxxx>