ideapad-laptop touchpad handling problems, request for help

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

 



Hi All,

I'm emailing you all because you have written patches or
reported bugs related to the ideapad-laptop touchpad
handling the past.

1. History
==========

I have done a bit of digging into the history of
the touchpad handling:

What I believe is the troublesome part of the touchpad handling
started in 2012 with:

07a4a4fc83dd ("ideapad: add Lenovo IdeaPad Z570 support (part 2)")
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=07a4a4fc83dd

Which added the ideapad_sync_touchpad_state() function which depending
on the result of reading VPCCMD_R_TOUCHPAD send an i8042 command to
enable/disable the aux port of the ps2 controller *and* which sends
KEY_TOUCHPAD_OFF/KEY_TOUCHPAD_ON events to userspace to let userspace
know the state of the touchpad.

The first commit to optionally disable ideapad_sync_touchpad_state()
was actually written by me in 2014, for a "Lenovo Yoga 2 11":

f79a901331a8 ("ideapad-laptop: Disable touchpad interface on Yoga models")
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=f79a901331a8

The problem on the "Lenovo Yoga 2 11" was a spurious KEY_TOUCHPAD_OFF
event on resume, other then that there were no bad side effects.

This patch got reverted soon afterwards in commit 3b264d279e72 because
it stopped the touchpad enable/disable button from working on
a "Lenovo Yoga 2 13".

Then in 2021 a patch was added to again disable ideapad_sync_touchpad_state()
on some models, this time based on the ACPI HID (ELAN0634) of the touchpad:

d69cd7eea93e ("platform/x86: ideapad-laptop: Disable touchpad_switch for ELAN0634")
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=d69cd7eea93e

And the last couple of weeks the following 2 patches were added to disable
ideapad_sync_touchpad_state() on more models based on DMI ids for the first
patch (already merged) + adding a new ACPI HID for the second patch:

a231224a601c ("platform/x86: ideapad-laptop: Disable touchpad_switch")
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=a231224a601c

https://patchwork.kernel.org/project/platform-driver-x86/patch/20221029120311.11152-8-erayorcunus@xxxxxxxxx/

As mentioned in the commit msg of d69cd7eea93e ("platform/x86:
ideapad-laptop: Disable touchpad_switch for ELAN0634") part of
the problem is VPCCMD_R_TOUCHPAD returning 0 leading to the aux
ps/2 port getting turned off.

This can be a problem even on devices where the touchpad shows up as
an i2c/smbus device because often on those devices the touchpad is
connected over both ps/2 + i2c and at least for synaptics devices
the touchpad needs to be contacted over i2c within a couple of
100s of ms of doing a ps/2 reset for it to switch to i2c mode.


2. Possible solutions
=====================

1. Do something with BIOS date to only enable touchpad_ctrl_via_ec on
older models. Problem is that BIOS updates happen and those can be
of much later date then the production year of the model

2. Move to an allow list for setting touchpad_ctrl_via_ec to true, given
how soon after my initial patch to disable touchpad_ctrl_via_ec I got
a bug report about this, which even was due to a deny-list DMI entry
not being narrow enough this seems like a bad idea. OTOH missing
the ability to turn the touchpad on/off is less of a big deal
then a non working touchpad. So if we fail to find a better
solution this might be the best thing to do.

3. Since the problems are caused when VPCCMD_R_TOUCHPAD reads as 0 at
boot, causing ideapad_sync_touchpad_state() to turn off the ps/2 aux port
and since the touchpad is normally on at boot, we can check for
VPCCMD_R_TOUCHPAD reading as 0 at boot and if that happens assume that
means touchpad-ctrl via the EC is not available. I have attached
a patch implementing this approach.


3. Please test
==============

If you have ideapads where touchpad_ctrl_via_ec should be 1 because
it is needed to toggle the touchpad on/off with the hotkey.

Or the exact opposite you have ideapads where it should be disabled
because ideapad_sync_touchpad_state() turning off the ps/2 aux port
is causing problems.

Then please give the attached patch a try. Note this applies on
top of Torvald's current master, or on top of 6.0 with :
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=a231224a601c
added.

Regards,

Hans

From c52f2286e40291cb7337e9e9d7966365828d8d3d Mon Sep 17 00:00:00 2001
From: Hans de Goede <hdegoede@xxxxxxxxxx>
Date: Wed, 9 Nov 2022 21:39:37 +0100
Subject: [PATCH] platform/x86: ideapad-laptop: Improve touchpad_ctrl_via_ec
 detection
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

On newer ideapads touchpad ctrl via the EC is not available and allowing
ideapad_sync_touchpad_state() to turn of the PS/2 aux port on a 0 read
from VPCCMD_R_TOUCHPAD stops the touchpad from working.

So far we have been using a deny-list based approach to disable
ideapad_sync_touchpad_state() on models where it is causing issues
based on a mix of touchpad ACPI-HID + DMI string matches. But this
does not work well (it results in a game of whack a mole).

Since ideapad_sync_touchpad_state() causes the problem only when
VPCCMD_R_TOUCHPAD reads 0 and since normally the touchpad is always
on at boot, so VPCCMD_R_TOUCHPAD should read as 1, we can avoid
models on which touchpad_ctrl_via_ec causes problems by only allowing
touchpad_ctrl_via_ec when VPCCMD_R_TOUCHPAD reads non 0 at boot.

Fixes: d69cd7eea93e ("platform/x86: ideapad-laptop: Disable touchpad_switch for ELAN0634")
Fixes: a231224a601c ("platform/x86: ideapad-laptop: Disable touchpad_switch")
Cc: Maxim Mikityanskiy <maxtram95@xxxxxxxxx>
Cc: GOESSEL Guillaume <g_goessel@xxxxxxxxxxx>
Cc: Jiaxun Yang <jiaxun.yang@xxxxxxxxxxx>
Cc: Manyi Li <limanyi@xxxxxxxxxxxxx>
Cc: Eray Orçunus <erayorcunus@xxxxxxxxx>
Cc: Ike Panhc <ike.pan@xxxxxxxxxxxxx>
Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx>
---
 drivers/platform/x86/ideapad-laptop.c | 37 +++++++++------------------
 1 file changed, 12 insertions(+), 25 deletions(-)

diff --git a/drivers/platform/x86/ideapad-laptop.c b/drivers/platform/x86/ideapad-laptop.c
index 33b3dfdd1b08..499e75c84476 100644
--- a/drivers/platform/x86/ideapad-laptop.c
+++ b/drivers/platform/x86/ideapad-laptop.c
@@ -1533,38 +1533,25 @@ static const struct dmi_system_id hw_rfkill_list[] = {
 	{}
 };
 
-static const struct dmi_system_id no_touchpad_switch_list[] = {
-	{
-	.ident = "Lenovo Yoga 3 Pro 1370",
-	.matches = {
-		DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
-		DMI_MATCH(DMI_PRODUCT_VERSION, "Lenovo YOGA 3"),
-		},
-	},
-	{
-	.ident = "ZhaoYang K4e-IML",
-	.matches = {
-		DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
-		DMI_MATCH(DMI_PRODUCT_VERSION, "ZhaoYang K4e-IML"),
-		},
-	},
-	{}
-};
-
 static void ideapad_check_features(struct ideapad_private *priv)
 {
 	acpi_handle handle = priv->adev->handle;
 	unsigned long val;
+	int ret;
 
 	priv->features.hw_rfkill_switch = dmi_check_system(hw_rfkill_list);
 
-	/* Most ideapads with ELAN0634 touchpad don't use EC touchpad switch */
-	if (acpi_dev_present("ELAN0634", NULL, -1))
-		priv->features.touchpad_ctrl_via_ec = 0;
-	else if (dmi_check_system(no_touchpad_switch_list))
-		priv->features.touchpad_ctrl_via_ec = 0;
-	else
-		priv->features.touchpad_ctrl_via_ec = 1;
+	/*
+	 * On newer ideapads touchpad ctrl via the EC is not available and
+	 * allowing ideapad_sync_touchpad_state() to turn of the PS/2 aux
+	 * port on a 0 read from VPCCMD_R_TOUCHPAD stops the touchpad from
+	 * working.
+	 *
+	 * Assume that the touchpad is always on at boot and that a 0 read
+	 * from VPCCMD_R_TOUCHPAD means EC touchpad ctrl is not available.
+	 */
+	ret = read_ec_data(priv->adev->handle, VPCCMD_R_TOUCHPAD, &val);
+	priv->features.touchpad_ctrl_via_ec = ret == 0 && val != 0;
 
 	if (!read_ec_data(handle, VPCCMD_R_FAN, &val))
 		priv->features.fan_mode = true;
-- 
2.37.3


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

  Powered by Linux