Thread (17 messages) 17 messages, 3 authors, 2024-09-22

Re: [PATCH] platform/x86: ideapad-laptop: Stop calling i8042_command()

From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Date: 2024-08-12 17:24:30
Also in: platform-driver-x86, stable

On Mon, Aug 12, 2024 at 04:41:50PM +0200, Hans de Goede wrote:
Hi Maxim,

On 8/12/24 4:37 PM, Maxim Mikityanskiy wrote:
quoted
On Mon, 05 Aug 2024 at 17:45:19 +0200, Hans de Goede wrote:
quoted
On 8/5/24 5:30 PM, Maxim Mikityanskiy wrote:
quoted
That means, userspace is not filtering out events upon receiving
KEY_TOUCHPAD_OFF. If we wanted to rely on that, we would need to send
KEY_TOUCHPAD_TOGGLE from the driver, but we actually can't, because Z570
is weird. It maintains the touchpad state in firmware to light up the
status LED, but the firmware doesn't do the actual touchpad disablement.

That is, if we use TOGGLE, the LED will get out of sync. If we use
ON/OFF, the touchpad won't be disabled, unless we do it in the kernel.
Ack.

So how about this instead:
diff --git a/drivers/platform/x86/ideapad-laptop.c b/drivers/platform/x86/ideapad-laptop.c
index 1ace711f7442..b7fa06f793cb 100644
--- a/drivers/platform/x86/ideapad-laptop.c
+++ b/drivers/platform/x86/ideapad-laptop.c
@@ -1574,7 +1574,7 @@ static void ideapad_sync_touchpad_state(struct ideapad_private *priv, bool send_
 	 * touchpad off and on. We send KEY_TOUCHPAD_OFF and
 	 * KEY_TOUCHPAD_ON to not to get out of sync with LED
 	 */
-	if (priv->features.ctrl_ps2_aux_port)
+	if (send_events && priv->features.ctrl_ps2_aux_port)
 		i8042_command(&param, value ? I8042_CMD_AUX_ENABLE : I8042_CMD_AUX_DISABLE);
 
 	/*
Maxmime, if you still have your Z570 can you check if the touchpad state after a suspend/resume
correctly reflects the state before suspend/resume in both touchpad on / off states ?
*Maxim
Oops, sorry.
quoted
Just a heads-up, my Z570 now belongs to a family member, we'll test what
you asked, but right now there is a btrfs corruption on that laptop that
we need to fix first, it interferes with kernel compilation =/
Note as discussed in another part of the thread the original bug report
actually was not on a Z570, so the whole usage of i8042_command() on
suspend/resume was a bit of a red herring. And the suspend/resume issue
has been fixed in another way in the mean time.

So there really is no need to test this change anymore. At the moment
there are no planned changes to ideapad-laptop related to this.
I think we still need to stop ideapad-laptop poking into 8042,
especially ahead of time. If we do not want to wait for userspace to
handle this properly, I wonder if we could not create an
input_handler that would attach to the touchpad device and filter out
all events coming from the touchpad if touchpad is supposed to be off.

Thanks.

-- 
Dmitry
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help