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

["Alice Mikityanska" <alice.kernel@fastmail.im>]

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.

>> +			input_sync(tp_dev);
>> +		}
>> +	}
>> +}
>> +
>>  /*
>>   * backlight
>>   */
>> @@ -1799,7 +1964,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 +1974,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 +2181,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 +2565,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 +2645,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 +2677,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);
>>  }
>>
>> -- 
>> Dmitry