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@xxxxxxxxxx, 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@xxxxxxx> 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