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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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(&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.

> 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.

Good luck with the BTRFS problem.

Regards,

Hans






[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux