Hi,
we've got a bug report on openSUSE Bugzilla about the broken touchpad
on Lenovo Yoga Slim 7:
https://bugzilla.opensuse.org/show_bug.cgi?id=1193064
The touchpad is an Elantech one, connected over i2c, and there are two
drivers supporting it. Unfortunately, the default one the system
binds, elan-i2c input driver, doesn't seem working properly, while
i2c-hid driver works.
I'm not sure what's the best fix for this, but below a quick
workaround using a deny list with DMI matching.
If this is OK, I can resubmit the patch for merging.
Any comments appreciated.
thanks,
Takashi
-- 8< --
From: Takashi Iwai <[email protected]>
Subject: [PATCH] Input: elan_i2c: Add deny list for Lenovo Yoga Slim 7
The touchpad on Lenovo Yoga Slim 7 doesn't work well with elan-i2c but
rather better with i2c-hid. Add a deny list for avoiding to bind with
elan-i2c.
BugLink: https://bugzilla.opensuse.org/show_bug.cgi?id=1193064
Signed-off-by: Takashi Iwai <[email protected]>
---
drivers/input/mouse/elan_i2c_core.c | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/drivers/input/mouse/elan_i2c_core.c b/drivers/input/mouse/elan_i2c_core.c
index 47af62c12267..fd08481f7aea 100644
--- a/drivers/input/mouse/elan_i2c_core.c
+++ b/drivers/input/mouse/elan_i2c_core.c
@@ -18,6 +18,7 @@
#include <linux/acpi.h>
#include <linux/delay.h>
#include <linux/device.h>
+#include <linux/dmi.h>
#include <linux/firmware.h>
#include <linux/i2c.h>
#include <linux/init.h>
@@ -1222,6 +1223,20 @@ static void elan_disable_regulator(void *_data)
regulator_disable(data->vcc);
}
+static const struct dmi_system_id elan_i2c_denylist[] __initconst = {
+#if IS_ENABLED(CONFIG_I2C_HID_ACPI)
+ {
+ /* Lenovo Yoga Slim 7 is better supported by i2c-hid */
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
+ DMI_MATCH(DMI_PRODUCT_NAME, "82A3"),
+ DMI_MATCH(DMI_PRODUCT_VERSION, "Yoga Slim 7 14ITL05"),
+ },
+ },
+#endif
+ { }
+};
+
static int elan_probe(struct i2c_client *client,
const struct i2c_device_id *dev_id)
{
@@ -1233,6 +1248,10 @@ static int elan_probe(struct i2c_client *client,
if (IS_ENABLED(CONFIG_MOUSE_ELAN_I2C_I2C) &&
i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
+ if (dmi_check_system(elan_i2c_denylist)) {
+ dev_info(dev, "Hits deny list, skipping\n");
+ return -ENODEV;
+ }
transport_ops = &elan_i2c_ops;
} else if (IS_ENABLED(CONFIG_MOUSE_ELAN_I2C_SMBUS) &&
i2c_check_functionality(client->adapter,
--
2.31.1
Hi Takashi,
I love your patch! Perhaps something to improve:
[auto build test WARNING on dtor-input/next]
[also build test WARNING on hid/for-next linux/master linus/master v5.17-rc2 next-20220204]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Takashi-Iwai/Wrongly-bound-Elantech-touchpad-on-Lenovo-Yoga-Slim-7/20220205-005753
base: https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git next
config: x86_64-randconfig-a003-20220131 (https://download.01.org/0day-ci/archive/20220205/[email protected]/config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce (this is a W=1 build):
# https://github.com/0day-ci/linux/commit/9f3fbdd527662d97eb0bece1005d96a0a1b0fac2
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Takashi-Iwai/Wrongly-bound-Elantech-touchpad-on-Lenovo-Yoga-Slim-7/20220205-005753
git checkout 9f3fbdd527662d97eb0bece1005d96a0a1b0fac2
# save the config file to linux build tree
mkdir build_dir
make W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>
All warnings (new ones prefixed by >>, old ones prefixed by <<):
>> WARNING: modpost: vmlinux.o(.text+0x87e136): Section mismatch in reference from the function elan_probe() to the variable .init.rodata:elan_i2c_denylist
The function elan_probe() references
the variable __initconst elan_i2c_denylist.
This is often because elan_probe lacks a __initconst
annotation or the annotation of elan_i2c_denylist is wrong.
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]
Hi,
On 2/4/22 18:39, Benjamin Tissoires wrote:
> Hi,
>
> [adding Dmitry, the maintainer of the input tree and Hans, a colleague of mine]
>
> On Fri, Feb 4, 2022 at 5:57 PM Takashi Iwai <[email protected]> wrote:
>>
>> Hi,
>>
>> we've got a bug report on openSUSE Bugzilla about the broken touchpad
>> on Lenovo Yoga Slim 7:
>> https://bugzilla.opensuse.org/show_bug.cgi?id=1193064
>>
>> The touchpad is an Elantech one, connected over i2c, and there are two
>> drivers supporting it. Unfortunately, the default one the system
>> binds, elan-i2c input driver, doesn't seem working properly, while
>> i2c-hid driver works.
>
> Hans, we do have a similar bug on RHEL at
> https://bugzilla.redhat.com/show_bug.cgi?id=2029078 (sorry, private
> bug).
>
> IIRC you worked on the discrimination between i2c-hid and elan_i2c (I
> might be completely wrong though).
Yes I did work on that, but then the other way around making sure
that the i2c-hid driver would not bind to some devices which need
the elan_i2c touch*pad* driver.
And indeed as Dmitry points out:
> I believe we need to do what Hans did for Elan Touch*screen* driver and
> avoid binding to the device if it has i2c-hid-specific _DMS in ACPI.
> I.e. we need to replicate elants_acpi_is_hid_device().
>
> Even better would be to factor it out, maybe not into a shared module
> but simply shared header with static inline function that we could share
> between elan drivers and maybe others as well.
I did fix a similar problem for the touchscreen driver last year or so.
I agree with Dmitry that we should try to avoid DMI matching here;
and I also agree that having some header with a static inline
acpi_is_hid_device() device helper would be good.
I'm a bit worried about the acpi_is_hid_device() approach though,
there is a lot of copy and pasting going on when vendors create
ACPI tables and sometimes a "PNP0C50" CID is present combined
with a valid i2c-hid _DSM method even though the device is not
an i2c-hid device, also see the i2c_hid_acpi_blacklist[] in
drivers/hid/i2c-hid/i2c-hid-acpi.c .
It seems to me that the problem is that the Lenovo Yoga Slim 7
is using what seems to be a very generic "ELAN0000" ACPI hardware
id instead of one of the many more specific ones.
So we could limit the acpi_is_hid_device() check to just the
"ELAN0000" ACPI hardware id I guess?
So I see the following 2 options:
1. Add an unconditional acpi_is_hid_device() check to elan_probe()
and watch out for any bug-reports that this is causing breakage
elsehwere
2. Add an acpi_is_hid_device() check to elan_probe() for ACPI enumerated
touchpads with a hardware-id of ELAN0000 only; and still
watch out for any bug-reports that this is causing breakage
elsehwere just to be sure
Regards,
Hans
>> I'm not sure what's the best fix for this, but below a quick
>> workaround using a deny list with DMI matching.
>> If this is OK, I can resubmit the patch for merging.
>>
>> Any comments appreciated.
>>
>>
>> thanks,
>>
>> Takashi
>>
>> -- 8< --
>> From: Takashi Iwai <[email protected]>
>> Subject: [PATCH] Input: elan_i2c: Add deny list for Lenovo Yoga Slim 7
>>
>> The touchpad on Lenovo Yoga Slim 7 doesn't work well with elan-i2c but
>> rather better with i2c-hid. Add a deny list for avoiding to bind with
>> elan-i2c.
>>
>> BugLink: https://bugzilla.opensuse.org/show_bug.cgi?id=1193064
>> Signed-off-by: Takashi Iwai <[email protected]>
>> ---
>> drivers/input/mouse/elan_i2c_core.c | 19 +++++++++++++++++++
>> 1 file changed, 19 insertions(+)
>>
>> diff --git a/drivers/input/mouse/elan_i2c_core.c b/drivers/input/mouse/elan_i2c_core.c
>> index 47af62c12267..fd08481f7aea 100644
>> --- a/drivers/input/mouse/elan_i2c_core.c
>> +++ b/drivers/input/mouse/elan_i2c_core.c
>> @@ -18,6 +18,7 @@
>> #include <linux/acpi.h>
>> #include <linux/delay.h>
>> #include <linux/device.h>
>> +#include <linux/dmi.h>
>> #include <linux/firmware.h>
>> #include <linux/i2c.h>
>> #include <linux/init.h>
>> @@ -1222,6 +1223,20 @@ static void elan_disable_regulator(void *_data)
>> regulator_disable(data->vcc);
>> }
>>
>> +static const struct dmi_system_id elan_i2c_denylist[] __initconst = {
>> +#if IS_ENABLED(CONFIG_I2C_HID_ACPI)
>> + {
>> + /* Lenovo Yoga Slim 7 is better supported by i2c-hid */
>> + .matches = {
>> + DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
>> + DMI_MATCH(DMI_PRODUCT_NAME, "82A3"),
>> + DMI_MATCH(DMI_PRODUCT_VERSION, "Yoga Slim 7 14ITL05"),
>> + },
>> + },
>> +#endif
>> + { }
>> +};
>> +
>> static int elan_probe(struct i2c_client *client,
>> const struct i2c_device_id *dev_id)
>> {
>> @@ -1233,6 +1248,10 @@ static int elan_probe(struct i2c_client *client,
>>
>> if (IS_ENABLED(CONFIG_MOUSE_ELAN_I2C_I2C) &&
>> i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
>> + if (dmi_check_system(elan_i2c_denylist)) {
>> + dev_info(dev, "Hits deny list, skipping\n");
>> + return -ENODEV;
>> + }
>> transport_ops = &elan_i2c_ops;
>> } else if (IS_ENABLED(CONFIG_MOUSE_ELAN_I2C_SMBUS) &&
>> i2c_check_functionality(client->adapter,
>> --
>> 2.31.1
>>
>>
>>
>>
>>
>>
>>
>
Hi Takashi,
I love your patch! Perhaps something to improve:
[auto build test WARNING on dtor-input/next]
[also build test WARNING on hid/for-next linux/master linus/master v5.17-rc3 next-20220204]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Takashi-Iwai/Wrongly-bound-Elantech-touchpad-on-Lenovo-Yoga-Slim-7/20220205-005753
base: https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git next
config: i386-randconfig-a004 (https://download.01.org/0day-ci/archive/20220207/[email protected]/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project 6daaf5a44925592c764c59219b0024ee06317028)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/9f3fbdd527662d97eb0bece1005d96a0a1b0fac2
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Takashi-Iwai/Wrongly-bound-Elantech-touchpad-on-Lenovo-Yoga-Slim-7/20220205-005753
git checkout 9f3fbdd527662d97eb0bece1005d96a0a1b0fac2
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 SHELL=/bin/bash
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>
All warnings (new ones prefixed by >>, old ones prefixed by <<):
>> WARNING: modpost: vmlinux.o(.text+0xe016ae): Section mismatch in reference from the function elan_probe() to the variable .init.rodata:elan_i2c_denylist
The function elan_probe() references
the variable __initconst elan_i2c_denylist.
This is often because elan_probe lacks a __initconst
annotation or the annotation of elan_i2c_denylist is wrong.
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]
Hi,
[adding Dmitry, the maintainer of the input tree and Hans, a colleague of mine]
On Fri, Feb 4, 2022 at 5:57 PM Takashi Iwai <[email protected]> wrote:
>
> Hi,
>
> we've got a bug report on openSUSE Bugzilla about the broken touchpad
> on Lenovo Yoga Slim 7:
> https://bugzilla.opensuse.org/show_bug.cgi?id=1193064
>
> The touchpad is an Elantech one, connected over i2c, and there are two
> drivers supporting it. Unfortunately, the default one the system
> binds, elan-i2c input driver, doesn't seem working properly, while
> i2c-hid driver works.
Hans, we do have a similar bug on RHEL at
https://bugzilla.redhat.com/show_bug.cgi?id=2029078 (sorry, private
bug).
IIRC you worked on the discrimination between i2c-hid and elan_i2c (I
might be completely wrong though).
Would this patch be OK with you?
Cheers,
Benjamin
>
> I'm not sure what's the best fix for this, but below a quick
> workaround using a deny list with DMI matching.
> If this is OK, I can resubmit the patch for merging.
>
> Any comments appreciated.
>
>
> thanks,
>
> Takashi
>
> -- 8< --
> From: Takashi Iwai <[email protected]>
> Subject: [PATCH] Input: elan_i2c: Add deny list for Lenovo Yoga Slim 7
>
> The touchpad on Lenovo Yoga Slim 7 doesn't work well with elan-i2c but
> rather better with i2c-hid. Add a deny list for avoiding to bind with
> elan-i2c.
>
> BugLink: https://bugzilla.opensuse.org/show_bug.cgi?id=1193064
> Signed-off-by: Takashi Iwai <[email protected]>
> ---
> drivers/input/mouse/elan_i2c_core.c | 19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)
>
> diff --git a/drivers/input/mouse/elan_i2c_core.c b/drivers/input/mouse/elan_i2c_core.c
> index 47af62c12267..fd08481f7aea 100644
> --- a/drivers/input/mouse/elan_i2c_core.c
> +++ b/drivers/input/mouse/elan_i2c_core.c
> @@ -18,6 +18,7 @@
> #include <linux/acpi.h>
> #include <linux/delay.h>
> #include <linux/device.h>
> +#include <linux/dmi.h>
> #include <linux/firmware.h>
> #include <linux/i2c.h>
> #include <linux/init.h>
> @@ -1222,6 +1223,20 @@ static void elan_disable_regulator(void *_data)
> regulator_disable(data->vcc);
> }
>
> +static const struct dmi_system_id elan_i2c_denylist[] __initconst = {
> +#if IS_ENABLED(CONFIG_I2C_HID_ACPI)
> + {
> + /* Lenovo Yoga Slim 7 is better supported by i2c-hid */
> + .matches = {
> + DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
> + DMI_MATCH(DMI_PRODUCT_NAME, "82A3"),
> + DMI_MATCH(DMI_PRODUCT_VERSION, "Yoga Slim 7 14ITL05"),
> + },
> + },
> +#endif
> + { }
> +};
> +
> static int elan_probe(struct i2c_client *client,
> const struct i2c_device_id *dev_id)
> {
> @@ -1233,6 +1248,10 @@ static int elan_probe(struct i2c_client *client,
>
> if (IS_ENABLED(CONFIG_MOUSE_ELAN_I2C_I2C) &&
> i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
> + if (dmi_check_system(elan_i2c_denylist)) {
> + dev_info(dev, "Hits deny list, skipping\n");
> + return -ENODEV;
> + }
> transport_ops = &elan_i2c_ops;
> } else if (IS_ENABLED(CONFIG_MOUSE_ELAN_I2C_SMBUS) &&
> i2c_check_functionality(client->adapter,
> --
> 2.31.1
>
>
>
>
>
>
>