* Re: [PATCH v3] platform/x86: ideapad-laptop: do not poke keyboard controller
2026-10-22 4:50 [PATCH v3] platform/x86: ideapad-laptop: do not poke keyboard controller Dmitry Torokhov
@ 2026-10-23 9:21 ` Uwe Kleine-König
2026-10-23 16:04 ` Alice Mikityanska
` (2 subsequent siblings)
3 siblings, 0 replies; 12+ messages in thread
From: Uwe Kleine-König @ 2026-10-23 9:21 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Mark Pearson, Derek J. Clark, Ike Panhc, Hans de Goede,
Ilpo Järvinen, Maxim Mikityanskiy, platform-driver-x86,
linux-kernel
[-- Attachment #1: Type: text/plain, Size: 1008 bytes --]
On Wed, Oct 21, 2026 at 09:50:56PM -0700, 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)
noooooooo, mod_devicetable.h is wrong, I want to get rid of that header
and input_device_id is already provided by <inux/input.h>.
If you want full iwyu, please use <linux/device-id/input.h> instead.
Best regards
Uwe
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH v3] platform/x86: ideapad-laptop: do not poke keyboard controller
2026-10-22 4:50 [PATCH v3] platform/x86: ideapad-laptop: do not poke keyboard controller Dmitry Torokhov
2026-10-23 9:21 ` Uwe Kleine-König
@ 2026-10-23 16:04 ` Alice Mikityanska
2026-10-23 16:37 ` Alice Mikityanska
2026-10-24 2:15 ` Rong Zhang
2026-10-25 2:25 ` Rong Zhang
3 siblings, 1 reply; 12+ messages in thread
From: Alice Mikityanska @ 2026-10-23 16:04 UTC (permalink / raw)
To: Dmitry Torokhov, Mark Pearson, Ike Panhc
Cc: platform-driver-x86, linux-kernel, Derek J. Clark, Hans de Goede,
Ilpo Järvinen, Uwe Kleine-König, Maxim Mikityanskiy
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.
> + 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(¶m, 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
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH v3] platform/x86: ideapad-laptop: do not poke keyboard controller
2026-10-23 16:04 ` Alice Mikityanska
@ 2026-10-23 16:37 ` Alice Mikityanska
2026-10-23 23:05 ` Dmitry Torokhov
0 siblings, 1 reply; 12+ messages in thread
From: Alice Mikityanska @ 2026-10-23 16:37 UTC (permalink / raw)
To: Dmitry Torokhov, Mark Pearson, Ike Panhc
Cc: platform-driver-x86, linux-kernel, Derek J. Clark, Hans de Goede,
Ilpo Järvinen, Uwe Kleine-König, Maxim Mikityanskiy
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(¶m, 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
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH v3] platform/x86: ideapad-laptop: do not poke keyboard controller
2026-10-23 16:37 ` Alice Mikityanska
@ 2026-10-23 23:05 ` Dmitry Torokhov
0 siblings, 0 replies; 12+ messages in thread
From: Dmitry Torokhov @ 2026-10-23 23:05 UTC (permalink / raw)
To: Alice Mikityanska
Cc: Mark Pearson, Ike Panhc, platform-driver-x86, linux-kernel,
Derek J. Clark, Hans de Goede, Ilpo Järvinen,
Uwe Kleine-König, Maxim Mikityanskiy
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3] platform/x86: ideapad-laptop: do not poke keyboard controller
2026-10-22 4:50 [PATCH v3] platform/x86: ideapad-laptop: do not poke keyboard controller Dmitry Torokhov
2026-10-23 9:21 ` Uwe Kleine-König
2026-10-23 16:04 ` Alice Mikityanska
@ 2026-10-24 2:15 ` Rong Zhang
2026-10-24 3:20 ` Dmitry Torokhov
2026-10-25 2:25 ` Rong Zhang
3 siblings, 1 reply; 12+ messages in thread
From: Rong Zhang @ 2026-10-24 2:15 UTC (permalink / raw)
To: Dmitry Torokhov, Mark Pearson, Derek J. Clark, Ike Panhc,
Hans de Goede, Ilpo Järvinen, Maxim Mikityanskiy
Cc: platform-driver-x86, linux-kernel
Hi Dmitry,
On Wed, 2026-10-21 at 21:50 -0700, 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");
By re-purposing ctrl_ps2_aux_port, you are breaking its existing users
*silently*.
Please don't do so, but rename module parameter to reflect the new
behavior. In this manner, the existing users should at least see some
clues in kmsg.
Thanks,
Rong
>
> 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);
> + 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(¶m, 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);
> }
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH v3] platform/x86: ideapad-laptop: do not poke keyboard controller
2026-10-24 2:15 ` Rong Zhang
@ 2026-10-24 3:20 ` Dmitry Torokhov
2026-10-24 15:30 ` Rong Zhang
0 siblings, 1 reply; 12+ messages in thread
From: Dmitry Torokhov @ 2026-10-24 3:20 UTC (permalink / raw)
To: Rong Zhang
Cc: Mark Pearson, Derek J. Clark, Ike Panhc, Hans de Goede,
Ilpo Järvinen, Maxim Mikityanskiy,
platform-driver-x86, linux-kernel
Hi Rong,
On Sat, Oct 24, 2026 at 10:15:56AM +0800, Rong Zhang wrote:
> > 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");
>
> By re-purposing ctrl_ps2_aux_port, you are breaking its existing users
> *silently*.
I'm not?
>
> Please don't do so, but rename module parameter to reflect the new
> behavior. In this manner, the existing users should at least see some
> clues in kmsg.
The purpose of ctrl_ps2_aux_port (introduced in commit c69e7d843d2c
("platform/x86: ideapad-laptop: Only toggle ps2 aux port on/off on
select models")) is to let the driver disable and re-enable the PS/2
touchpad on models where pressing the touchpad toggle hotkey only
toggles the LED in the EC without muting the touchpad itself.
This patch still targets the exact same i8042 PS/2 touchpad devices and
achieves the exact same user-visible behavior (muting and unmuting the
touchpad when the hotkey is pressed), just by filtering events in the
input layer instead of poking the 8042 controller behind the i8042 and
psmouse drivers' backs.
Anyone currently passing ideapad_laptop.ctrl_ps2_aux_port=1 on their
kernel command line or in modprobe configuration will continue to have a
working touchpad toggle. Renaming the module parameter, on the other
hand, is what would actually break those existing users by causing the
module to reject the parameter and stop muting their touchpad.
Thanks.
--
Dmitry
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH v3] platform/x86: ideapad-laptop: do not poke keyboard controller
2026-10-24 3:20 ` Dmitry Torokhov
@ 2026-10-24 15:30 ` Rong Zhang
2026-10-24 23:13 ` Dmitry Torokhov
0 siblings, 1 reply; 12+ messages in thread
From: Rong Zhang @ 2026-10-24 15:30 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Mark Pearson, Derek J. Clark, Ike Panhc, Hans de Goede,
Ilpo Järvinen, Maxim Mikityanskiy, platform-driver-x86,
linux-kernel
Hi Dmitry,
On Fri, 2026-10-23 at 20:20 -0700, Dmitry Torokhov wrote:
> Hi Rong,
>
> On Sat, Oct 24, 2026 at 10:15:56AM +0800, Rong Zhang wrote:
> > > 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");
> >
> > By re-purposing ctrl_ps2_aux_port, you are breaking its existing users
> > *silently*.
>
> I'm not?
Yes, and no.
When only focusing on the input events delivered to the userspace,
nothing is broken if everything works fine.
However, the methodology has been changed from hardware-based to
software-based. Because of the fundamental change, it has the potential
to break something if some users depends on the side-effect of toggling
the aux port.
While my device never needs the quirk, I can think of several useful
side-effects:
- pause the interrupt (and hence pause event handling) to save some
energy
- prevent the touchpad from waking up the platform
- reset faulty touchpads
By switching to the software-based methodology, all these capabilities
have gone. Of course, relying on these side-effects is a misuse, but
relying on the parameter itself without reporting it to the mailing list
is also a misuse! More below.
>
> >
> > Please don't do so, but rename module parameter to reflect the new
> > behavior. In this manner, the existing users should at least see some
> > clues in kmsg.
>
> The purpose of ctrl_ps2_aux_port (introduced in commit c69e7d843d2c
> ("platform/x86: ideapad-laptop: Only toggle ps2 aux port on/off on
> select models")) is to let the driver disable and re-enable the PS/2
> touchpad on models where pressing the touchpad toggle hotkey only
> toggles the LED in the EC without muting the touchpad itself.
>
> This patch still targets the exact same i8042 PS/2 touchpad devices and
> achieves the exact same user-visible behavior (muting and unmuting the
> touchpad when the hotkey is pressed), just by filtering events in the
> input layer instead of poking the 8042 controller behind the i8042 and
> psmouse drivers' backs.
Input events are not the only user-visible behavior.
>
> Anyone currently passing ideapad_laptop.ctrl_ps2_aux_port=1 on their
> kernel command line or in modprobe configuration will continue to have a
> working touchpad toggle. Renaming the module parameter, on the other
> hand, is what would actually break those existing users by causing the
> module to reject the parameter and stop muting their touchpad.
A completely new behavior reusing an old knob is always prone to break
*something* in a obscure way unless users are notified about the change.
Breaking something with a visible warning [1] in kmsg is much better than
potentially breaking something in radio silence.
We don't want to support poking i8042 anymore, so the use of
'ctrl_ps2_aux_port' is anyway becoming unsupported. The parameter was
introduced only for debugging and is expected to be removed at any time.
If users don't report their use of the parameter to the mailing list,
they are already relying on a broken setup.
By renaming it and breaking the unsupported setup loudly, we encourage
users to report quirky devices, as well as getting rid of an outdated
parameter name.
[1]: ideapad_laptop: unknown parameter 'ctrl_ps2_aux_port' ignored
Thanks,
Rong
>
> Thanks.
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH v3] platform/x86: ideapad-laptop: do not poke keyboard controller
2026-10-24 15:30 ` Rong Zhang
@ 2026-10-24 23:13 ` Dmitry Torokhov
2026-10-25 1:52 ` Rong Zhang
0 siblings, 1 reply; 12+ messages in thread
From: Dmitry Torokhov @ 2026-10-24 23:13 UTC (permalink / raw)
To: Rong Zhang
Cc: Mark Pearson, Derek J. Clark, Ike Panhc, Hans de Goede,
Ilpo Järvinen, Maxim Mikityanskiy,
platform-driver-x86, linux-kernel
Hi Rong,
On Sat, Oct 24, 2026 at 11:30:06PM +0800, Rong Zhang wrote:
> However, the methodology has been changed from hardware-based to
> software-based. Because of the fundamental change, it has the potential
> to break something if some users depends on the side-effect of toggling
> the aux port.
>
> While my device never needs the quirk, I can think of several useful
> side-effects:
>
> - pause the interrupt (and hence pause event handling) to save some
> energy
> - prevent the touchpad from waking up the platform
> - reset faulty touchpads
>
> By switching to the software-based methodology, all these capabilities
> have gone. Of course, relying on these side-effects is a misuse, but
> relying on the parameter itself without reporting it to the mailing list
> is also a misuse! More below.
This reminds me of https://xkcd.com/1172/ ("Workflow").
Also note that I8042_CMD_AUX_DISABLE / I8042_CMD_AUX_ENABLE only toggles
the AUX clock inhibit bit in the 8042 controller; it does not power down
or reset the touchpad (psmouse is unaware that ideapad-laptop poked the
controller behind its back, which is the whole problem), and i8042
already manages the AUX port and wakeup state across suspend and resume.
> A completely new behavior reusing an old knob is always prone to break
> *something* in a obscure way unless users are notified about the change.
> Breaking something with a visible warning [1] in kmsg is much better than
> potentially breaking something in radio silence.
>
> We don't want to support poking i8042 anymore, so the use of
> 'ctrl_ps2_aux_port' is anyway becoming unsupported. The parameter was
> introduced only for debugging and is expected to be removed at any time.
> If users don't report their use of the parameter to the mailing list,
> they are already relying on a broken setup.
>
> By renaming it and breaking the unsupported setup loudly, we encourage
> users to report quirky devices, as well as getting rid of an outdated
> parameter name.
>
> [1]: ideapad_laptop: unknown parameter 'ctrl_ps2_aux_port' ignored
We already have DMI matching for this quirk (ctrl_ps2_aux_port_list[],
which has only ever contained the 2011 IdeaPad Z570 since commit
c69e7d843d2c added it in 2022), and the module parameter simply sets the
exact same priv->features.ctrl_ps2_aux_port flag as the DMI table.
Intentionally breaking the module parameter for anyone who might be
using it for its intended purpose (making the touchpad toggle hotkey
work on an older IdeaPad) just because of hypothetical spacebar-heating
workflows does not make sense to me.
Thanks.
--
Dmitry
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH v3] platform/x86: ideapad-laptop: do not poke keyboard controller
2026-10-24 23:13 ` Dmitry Torokhov
@ 2026-10-25 1:52 ` Rong Zhang
2026-10-25 2:38 ` Dmitry Torokhov
0 siblings, 1 reply; 12+ messages in thread
From: Rong Zhang @ 2026-10-25 1:52 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Mark Pearson, Derek J. Clark, Ike Panhc, Hans de Goede,
Ilpo Järvinen, Maxim Mikityanskiy, platform-driver-x86,
linux-kernel
Hi Dmitry,
On Sat, 2026-10-24 at 16:13 -0700, Dmitry Torokhov wrote:
> Hi Rong,
>
> On Sat, Oct 24, 2026 at 11:30:06PM +0800, Rong Zhang wrote:
> > However, the methodology has been changed from hardware-based to
> > software-based. Because of the fundamental change, it has the potential
> > to break something if some users depends on the side-effect of toggling
> > the aux port.
> >
> > While my device never needs the quirk, I can think of several useful
> > side-effects:
> >
> > - pause the interrupt (and hence pause event handling) to save some
> > energy
> > - prevent the touchpad from waking up the platform
> > - reset faulty touchpads
> >
> > By switching to the software-based methodology, all these capabilities
> > have gone. Of course, relying on these side-effects is a misuse, but
> > relying on the parameter itself without reporting it to the mailing list
> > is also a misuse! More below.
>
> This reminds me of https://xkcd.com/1172/ ("Workflow").
>
> Also note that I8042_CMD_AUX_DISABLE / I8042_CMD_AUX_ENABLE only toggles
> the AUX clock inhibit bit in the 8042 controller; it does not power down
> or reset the touchpad (psmouse is unaware that ideapad-laptop poked the
> controller behind its back, which is the whole problem), and i8042
> already manages the AUX port and wakeup state across suspend and resume.
Thanks for the detailed explanation. I agree that there should be no
outstanding spacebar-heating workflow depending on I8042_CMD_AUX_DISABLE.
>
> > A completely new behavior reusing an old knob is always prone to break
> > *something* in a obscure way unless users are notified about the change.
> > Breaking something with a visible warning [1] in kmsg is much better than
> > potentially breaking something in radio silence.
> >
> > We don't want to support poking i8042 anymore, so the use of
> > 'ctrl_ps2_aux_port' is anyway becoming unsupported. The parameter was
> > introduced only for debugging and is expected to be removed at any time.
> > If users don't report their use of the parameter to the mailing list,
> > they are already relying on a broken setup.
> >
> > By renaming it and breaking the unsupported setup loudly, we encourage
> > users to report quirky devices, as well as getting rid of an outdated
> > parameter name.
> >
> > [1]: ideapad_laptop: unknown parameter 'ctrl_ps2_aux_port' ignored
>
> We already have DMI matching for this quirk (ctrl_ps2_aux_port_list[],
> which has only ever contained the 2011 IdeaPad Z570 since commit
> c69e7d843d2c added it in 2022), and the module parameter simply sets the
> exact same priv->features.ctrl_ps2_aux_port flag as the DMI table.
>
> Intentionally breaking the module parameter for anyone who might be
> using it for its intended purpose (making the touchpad toggle hotkey
> work on an older IdeaPad)
>
It's the flag's intended purpose, and never the module parameter's.
The module parameter's intended purpose is only for debugging and bug
reporting. And its name and description describe the intended behavior.
Renaming it does not change either of them, while reusing the knob makes
its name mismatch the intended behavior.
> just because of hypothetical spacebar-heating
> workflows does not make sense to me.
Using the parameter without reporting is also a spacebar-heating
workflow. That's why the description asks users to report to the pdx86
mailing list, so that we can add a new entry to the DMI quirk table. It's
similar to sysfs bind/unbind to a certain extent, which recently gained
TAINT_FORCED_BIND.
We shouldn't create a divergence between the intended behavior and the
parameter's name simply because such a workflow either.
Note that we've broken another module parameter before. See commit
cde7886b3517 ("platform/x86: ideapad-laptop: move ymc_trigger_ec from
lenovo-ymc"), which moved and renamed a parameter.
Thanks,
Rong
>
> Thanks.
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH v3] platform/x86: ideapad-laptop: do not poke keyboard controller
2026-10-25 1:52 ` Rong Zhang
@ 2026-10-25 2:38 ` Dmitry Torokhov
0 siblings, 0 replies; 12+ messages in thread
From: Dmitry Torokhov @ 2026-10-25 2:38 UTC (permalink / raw)
To: Rong Zhang
Cc: Mark Pearson, Derek J. Clark, Ike Panhc, Hans de Goede,
Ilpo Järvinen, Maxim Mikityanskiy, platform-driver-x86,
linux-kernel
On Sun, Oct 25, 2026 at 09:52:27AM +0800, Rong Zhang wrote:
> Hi Dmitry,
>
> On Sat, 2026-10-24 at 16:13 -0700, Dmitry Torokhov wrote:
> > Hi Rong,
> >
> > On Sat, Oct 24, 2026 at 11:30:06PM +0800, Rong Zhang wrote:
> > > However, the methodology has been changed from hardware-based to
> > > software-based. Because of the fundamental change, it has the potential
> > > to break something if some users depends on the side-effect of toggling
> > > the aux port.
> > >
> > > While my device never needs the quirk, I can think of several useful
> > > side-effects:
> > >
> > > - pause the interrupt (and hence pause event handling) to save some
> > > energy
> > > - prevent the touchpad from waking up the platform
> > > - reset faulty touchpads
> > >
> > > By switching to the software-based methodology, all these capabilities
> > > have gone. Of course, relying on these side-effects is a misuse, but
> > > relying on the parameter itself without reporting it to the mailing list
> > > is also a misuse! More below.
> >
> > This reminds me of https://xkcd.com/1172/ ("Workflow").
> >
> > Also note that I8042_CMD_AUX_DISABLE / I8042_CMD_AUX_ENABLE only toggles
> > the AUX clock inhibit bit in the 8042 controller; it does not power down
> > or reset the touchpad (psmouse is unaware that ideapad-laptop poked the
> > controller behind its back, which is the whole problem), and i8042
> > already manages the AUX port and wakeup state across suspend and resume.
>
> Thanks for the detailed explanation. I agree that there should be no
> outstanding spacebar-heating workflow depending on I8042_CMD_AUX_DISABLE.
>
> >
> > > A completely new behavior reusing an old knob is always prone to break
> > > *something* in a obscure way unless users are notified about the change.
> > > Breaking something with a visible warning [1] in kmsg is much better than
> > > potentially breaking something in radio silence.
> > >
> > > We don't want to support poking i8042 anymore, so the use of
> > > 'ctrl_ps2_aux_port' is anyway becoming unsupported. The parameter was
> > > introduced only for debugging and is expected to be removed at any time.
> > > If users don't report their use of the parameter to the mailing list,
> > > they are already relying on a broken setup.
> > >
> > > By renaming it and breaking the unsupported setup loudly, we encourage
> > > users to report quirky devices, as well as getting rid of an outdated
> > > parameter name.
> > >
> > > [1]: ideapad_laptop: unknown parameter 'ctrl_ps2_aux_port' ignored
> >
> > We already have DMI matching for this quirk (ctrl_ps2_aux_port_list[],
> > which has only ever contained the 2011 IdeaPad Z570 since commit
> > c69e7d843d2c added it in 2022), and the module parameter simply sets the
> > exact same priv->features.ctrl_ps2_aux_port flag as the DMI table.
> >
> > Intentionally breaking the module parameter for anyone who might be
> > using it for its intended purpose (making the touchpad toggle hotkey
> > work on an older IdeaPad)
> >
>
> It's the flag's intended purpose, and never the module parameter's.
>
> The module parameter's intended purpose is only for debugging and bug
> reporting. And its name and description describe the intended behavior.
>
> Renaming it does not change either of them, while reusing the knob makes
> its name mismatch the intended behavior.
>
> > just because of hypothetical spacebar-heating
> > workflows does not make sense to me.
>
> Using the parameter without reporting is also a spacebar-heating
> workflow. That's why the description asks users to report to the pdx86
> mailing list, so that we can add a new entry to the DMI quirk table. It's
> similar to sysfs bind/unbind to a certain extent, which recently gained
> TAINT_FORCED_BIND.
>
> We shouldn't create a divergence between the intended behavior and the
> parameter's name simply because such a workflow either.
OK, I'll just defer to Ilpo here. If he wants to rename module option I
can do that. I still believe that this gratuitously breaks users that
happen to use this module parameters. While we would like them to report
their systems they are under no obligation to do so.
Thanks.
--
Dmitry
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3] platform/x86: ideapad-laptop: do not poke keyboard controller
2026-10-22 4:50 [PATCH v3] platform/x86: ideapad-laptop: do not poke keyboard controller Dmitry Torokhov
` (2 preceding siblings ...)
2026-10-24 2:15 ` Rong Zhang
@ 2026-10-25 2:25 ` Rong Zhang
3 siblings, 0 replies; 12+ messages in thread
From: Rong Zhang @ 2026-10-25 2:25 UTC (permalink / raw)
To: Dmitry Torokhov, Mark Pearson, Derek J. Clark, Ike Panhc,
Hans de Goede, Ilpo Järvinen, Maxim Mikityanskiy
Cc: platform-driver-x86, linux-kernel
Hi Dmitry,
On Wed, 2026-10-21 at 21:50 -0700, 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,
Missing INPUT_DEVICE_ID_MATCH_BUS and INPUT_DEVICE_ID_MATCH_VENDOR?
Other than that and the module parameter's name, the patch looks good to
me. With them fixed:
Reviewed-by: Rong Zhang <i@rong.moe>
Tested-by: Rong Zhang <i@rong.moe> # not matching other touchpads
Thanks,
Rong
> + .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 +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(¶m, 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);
> }
^ permalink raw reply [flat|nested] 12+ messages in thread