| Message ID | s5hleyqwowl.wl-tiwai@suse.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Wrongly bound Elantech touchpad on Lenovo Yoga Slim 7 | expand |
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 > > > > > > >
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 > > > > > > > > > > > > > > >
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
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 >> >> >> >> >> >> >> >
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
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,