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

[Dmitry Torokhov <dmitry.torokhov@gmail.com>]

On Fri, Oct 23, 2026 at 07:37:34PM +0300, Alice Mikityanska wrote:
> On Fri, Oct 23, 2026, at 19:04, Alice Mikityanska wrote:
> > On Thu, Oct 22, 2026, at 07:50, 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.
> >>
> >> Reviewed-by: Mark Pearson <mpearson-lenovo@squebb.ca>
> >> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> >> ---
> >>
> >> v3:
> >> - add missing header includes (compiler.h, container_of.h, errno.h,
> >>   mod_devicetable.h, slab.h, spinlock.h)
> >> - drop stray blank line in ideapad_tpswitch_filter()
> >> - collect Reviewed-by from Mark Pearson
> >>
> >> 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 | 189 ++++++++++++++++++-
> >>  1 file changed, 180 insertions(+), 9 deletions(-)
> >>
> >> diff --git a/drivers/platform/x86/lenovo/ideapad-laptop.c 
> >> b/drivers/platform/x86/lenovo/ideapad-laptop.c
> >> index 8213524504ee..be890e82b9ef 100644
> >> --- a/drivers/platform/x86/lenovo/ideapad-laptop.c
> >> +++ b/drivers/platform/x86/lenovo/ideapad-laptop.c
> >> @@ -14,23 +14,28 @@
> >>  #include <linux/bitops.h>
> >>  #include <linux/bug.h>
> >>  #include <linux/cleanup.h>
> >> +#include <linux/compiler.h>
> >> +#include <linux/container_of.h>
> >>  #include <linux/debugfs.h>
> >>  #include <linux/delay.h>
> >>  #include <linux/device.h>
> >>  #include <linux/dmi.h>
> >> -#include <linux/i8042.h>
> >> +#include <linux/errno.h>
> >>  #include <linux/init.h>
> >>  #include <linux/input.h>
> >>  #include <linux/input/sparse-keymap.h>
> >>  #include <linux/jiffies.h>
> >>  #include <linux/kernel.h>
> >>  #include <linux/leds.h>
> >> +#include <linux/mod_devicetable.h>
> >>  #include <linux/module.h>
> >>  #include <linux/platform_device.h>
> >>  #include <linux/platform_profile.h>
> >>  #include <linux/power_supply.h>
> >>  #include <linux/rfkill.h>
> >>  #include <linux/seq_file.h>
> >> +#include <linux/slab.h>
> >> +#include <linux/spinlock.h>
> >>  #include <linux/string_choices.h>
> >>  #include <linux/sysfs.h>
> >>  #include <linux/types.h>
> >> @@ -210,6 +215,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 +246,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@vger.kernel.org");
> >> +		 "Enable driver based suppression of touchpad events on touchpad 
> >> on/off toggle. If you need this please report this to: 
> >> platform-driver-x86@vger.kernel.org");
> >> 
> >>  static bool touchpad_ctrl_via_ec;
> >>  module_param(touchpad_ctrl_via_ec, bool, 0444);
> >> @@ -1462,6 +1474,159 @@ 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);
> >> +}
> >> +
> >> +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);
> >
> > Hello!
> >
> > Tested-by: Alice Mikityanska <alice.kernel@fastmail.im>
> >
> > Seems to work well, without any "lost synchronization" messages in
> > dmesg. There is a new effect, caused by the above lines: if the finger
> > stays on the surface the entire time, and the user disables and enables
> > the touchpad, the pointer won't move until the user lifts the finger and
> > puts it back. Although it's unlikely to break anyone's workflow, I need
> > to double-check that the evdev doesn't emit some invalid sequence of
> > events, e.g., touch 0 followed by some movements, or touch 0 followed by
> > another touch 0.
> 
> Yes, I see that BTN_TOUCH 0 can be followed by another BTN_TOUCH 0,
> either in the event of lifting the finger after reenabling the touchpad,
> or if disabling-enabling-disabling touchpad all while holding the
> finger. Not sure if it's valid semantics, maybe it is.
> 
> In the scenario I'm describing (put the finger, disable, enable,
> move the finger), EV_ABS events start being generated after enabling
> the touchpad, without the preceding BTN_TOUCH 1, so the pointer
> doesn't move.

I think this is acceptable. But if someone cares enough they can add
tracking of true state and re-issue BTN_* events when touchpad is
enabled again.

Thanks.

-- 
Dmitry