Wrongly bound Elantech touchpad on Lenovo Yoga Slim 7

Wrongly bound Elantech touchpad on Lenovo Yoga Slim 7

[Takashi Iwai]

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 <tiwai@suse.de>
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 <tiwai@suse.de>
---
 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

Re: Wrongly bound Elantech touchpad on Lenovo Yoga Slim 7

[Benjamin Tissoires]

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 <tiwai@suse.de> 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 <tiwai@suse.de>
> 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 <tiwai@suse.de>
> ---
>  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
>
>
>
>
>
>
>

Re: Wrongly bound Elantech touchpad on Lenovo Yoga Slim 7

[Dmitry Torokhov]

Hi Benjamin,

On Fri, Feb 04, 2022 at 06:39:40PM +0100, 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 <tiwai@suse.de> 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?

I would prefer avoid DMI if possible.

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.

Thanks.

> 
> 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 <tiwai@suse.de>
> > 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 <tiwai@suse.de>
> > ---
> >  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
> >
> >
> >
> >
> >
> >
> >
> 

-- 
Dmitry

Re: Wrongly bound Elantech touchpad on Lenovo Yoga Slim 7

[kernel test robot]

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/202202050657.m9Z8VsGr-lkp@intel.com/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 <lkp@intel.com>

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/kbuild-all@lists.01.org

Re: Wrongly bound Elantech touchpad on Lenovo Yoga Slim 7

[Hans de Goede]

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 <tiwai@suse.de> 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 <tiwai@suse.de>
>> 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 <tiwai@suse.de>
>> ---
>>  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
>>
>>
>>
>>
>>
>>
>>
> 

Re: Wrongly bound Elantech touchpad on Lenovo Yoga Slim 7

[kernel test robot]

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/202202071511.Vkx9UNQz-lkp@intel.com/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 <lkp@intel.com>

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/kbuild-all@lists.01.org

Re: Wrongly bound Elantech touchpad on Lenovo Yoga Slim 7

[Jiri Slaby]

Hi,

I am resuming an ooold thread...

On 05. 02. 22, 12:12, Hans de Goede wrote:
> 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 <tiwai@suse.de> 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

I assume noone ever tried to implement this, right? Or was some 
alternative approach merged in the meantime?

openSUSE still drags the Takashi's downstream patch with a DMI check. It 
would be nice to sort this out upstream and drop that private one ;).

Thanks.

>>> 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 <tiwai@suse.de>
>>> 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 <tiwai@suse.de>
>>> ---
>>>   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
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>
> 
> 

-- 
js
suse labs

Re: Wrongly bound Elantech touchpad on Lenovo Yoga Slim 7

[Jiri Slaby]

Use hansg@kernel.org, not @redhat.com

On 08. 09. 26, 10:27, Jiri Slaby wrote:
> Hi,
> 
> I am resuming an ooold thread...
> 
> On 05. 02. 22, 12:12, Hans de Goede wrote:
>> 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 <tiwai@suse.de> 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
> 
> I assume noone ever tried to implement this, right? Or was some 
> alternative approach merged in the meantime?
> 
> openSUSE still drags the Takashi's downstream patch with a DMI check. It 
> would be nice to sort this out upstream and drop that private one ;).
> 
> Thanks.
> 
>>>> 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 <tiwai@suse.de>
>>>> 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 <tiwai@suse.de>
>>>> ---
>>>>   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
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>
>>
>>
> 

-- 
js
suse labs

Re: Wrongly bound Elantech touchpad on Lenovo Yoga Slim 7

[Hans de Goede]

Hi,

On 8-Sep-26 10:44, Jiri Slaby wrote:
> Use hansg@kernel.org, not @redhat.com

Thx.

> On 08. 09. 26, 10:27, Jiri Slaby wrote:
>> Hi,
>>
>> I am resuming an ooold thread...
>>
>> On 05. 02. 22, 12:12, Hans de Goede wrote:
>>> 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 <tiwai@suse.de> 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
>>
>> I assume noone ever tried to implement this, right? Or was some alternative approach merged in the meantime?

I indeed believe that no-one ever tried to implement this (but I did
not check).

>> openSUSE still drags the Takashi's downstream patch with a DMI check. It would be nice to sort this out upstream and drop that private one ;).

Ack, that would be great also to fix the issue on other distros.

Note my suggested approach of using an acpi_is_hid_device() check
in the elan touchpad driver does carry a risk of causing regressions.

But I think it is worth a try, maybe combined with limiting
it to a HID of "ELAN0000" or first try without and on
regressions on devices with another HID add that ?

And if this approach fails (causes regressions) then we should
upstream Takashi's downstream patch with a DMI check.

Regards,

Hans