Hi Dmitry, On 8/12/24 7:24 PM, Dmitry Torokhov wrote: > 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: >>> On Mon, 05 Aug 2024 at 17:45:19 +0200, Hans de Goede wrote: >>>> On 8/5/24 5:30 PM, Maxim Mikityanskiy wrote: >>>>> 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(¶m, 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. >> >>> 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. I agree. I think your suggestion of using the new(ish) [un]inhibit support in the input subsystem for this instead of poking at the i8042 is a good idea. As I mentioned when you first suggested this, I guess this requires 2 things: 1. Some helper to find the struct input_dev for the input_dev related to the ps/2 aux port 2. In kernel API / functions to do inhibit/uninhibit (maybe these already exist?) > 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. I think using the inhibit stuff would be better no? The biggest problems with trying to fix this are: 1. Finding time to work on this 2. Finding someone willing to test the patches Finding the time is going to be an issue for me since the i8042_command() calls are only still done on a single model laptop (using a DMI quirk) inside ideapad-laptop now, so this is pretty low priority IMHO. Which in practice means that I will simply never get around to this, sorry... Regards, Hans
