Re: [PATCH] ELAN touchpad i2c_hid bugs fix

Kai-Heng Feng <kai.heng.feng@xxxxxxxxxxxxx> · Wed, 3 Apr 2019 17:24:37 +0800

at 22:08, <Mario.Limonciello@xxxxxxxx> <Mario.Limonciello@xxxxxxxx> wrote:

-----Original Message-----
From: Kai Heng Feng <kai.heng.feng@xxxxxxxxxxxxx>
Sent: Monday, April 1, 2019 11:18 PM
To: Limonciello, Mario
Cc: Andy Shevchenko; hdegoede@xxxxxxxxxx; benjamin.tissoires@xxxxxxxxxx;
hotwater438@xxxxxxxxxxxx; jikos@xxxxxxxxxx; swboyd@xxxxxxxxxxxx;
bigeasy@xxxxxxxxxxxxx; dtor@xxxxxxxxxxxx; linux-input@xxxxxxxxxxxxxxx;  
linux-
kernel@xxxxxxxxxxxxxxx
Subject: Re: [PATCH] ELAN touchpad i2c_hid bugs fix

[EXTERNAL EMAIL]

On Apr 2, 2019, at 5:37 AM, <Mario.Limonciello@xxxxxxxx>
<Mario.Limonciello@xxxxxxxx> wrote:

-----Original Message-----
From: Andy Shevchenko <andy.shevchenko@xxxxxxxxx>
Sent: Thursday, March 21, 2019 4:48 AM
To: Kai-Heng Feng; Limonciello, Mario
Cc: Hans de Goede; Benjamin Tissoires; hotwater438@xxxxxxxxxxxx; Jiri
Kosina;
Stephen Boyd; Sebastian Andrzej Siewior; Dmitry Torokhov; open list:HID
CORE
LAYER; lkml
Subject: Re: [PATCH] ELAN touchpad i2c_hid bugs fix

[EXTERNAL EMAIL]

+Cc: Mario

Mario, do you have any insights about the issue with touchpad on Dell
system described below?

My apologies, this got lost while I was on vacation.

On Thu, Mar 21, 2019 at 6:08 AM Kai-Heng Feng
<kai.heng.feng@xxxxxxxxxxxxx> wrote:
at 01:18, Andy Shevchenko <andy.shevchenko@xxxxxxxxx> wrote:

On Wed, Mar 20, 2019 at 6:55 PM Kai-Heng Feng
<kai.heng.feng@xxxxxxxxxxxxx> wrote:
at 23:39, Hans de Goede <hdegoede@xxxxxxxxxx> wrote:
On 3/20/19 3:37 PM, Benjamin Tissoires wrote:

Benjamin, what I find interesting here is that the BOGUS_IRQ quirk
is also used on Elan devices, I suspect that these Elan devices
likely also need the I2C_HID_QUIRK_FORCE_TRIGGER_FALLING quirk
and then they probably will no longer need the bogus IRQ flag,
if you know about bugreports with an acpidump for any of the devices
needing the bogus IRQ quirk, then I (or you) can check how the IRQ  
is
declared there, I suspect it will be declared as level-low, just  
like
with the laptop this patch was written for. And it probably need to
be edge-falling instead of level-low just like this case.

First, I’ve already tried using IRQF_TRIGGER_FALLING, unfortunately  
it
doesn’t solve the issue for me.

I talked to Elan once, and they confirm the correct IRQ trigger is
level
low. So forcing falling trigger may break other platforms.

As far as I understood Vladislav the quirk he got from Elan as well.

Ok, then this is really weird.

Recently we found that Elan touchpad doesn’t like GpioInt() from its
_CRS.
Once the Interrupt() is used instead, the issue goes away.

IIRC i2c core tries to get interrupt from Interrupt() resource and
then falls back to GpioInt().
See i2c_acpi_get_info() and i2c_device_probe().

Here’s its ASL:

    Scope (\_SB.PCI0.I2C4)
    {
        Device (TPD0)
        {
            Name (_ADR, One)  // _ADR: Address
            Name (_HID, "DELL08AE")  // _HID: Hardware ID
            Name (_CID, "PNP0C50" /* HID Protocol Device (I2C bus) */)  // _CID:
Compatible ID
           Name (_UID, One)  // _UID: Unique ID
            Name (_S0W, 0x04)  // _S0W: S0 Device Wake State
            Name (SBFB, ResourceTemplate ()
            {
                I2cSerialBusV2 (0x002C, ControllerInitiated, 0x00061A80,
                    AddressingMode7Bit, "\\_SB.PCI0.I2C4",
                    0x00, ResourceConsumer, , Exclusive,
                    )
            })
            Name (SBFG, ResourceTemplate ()
            {
                GpioInt (Level, ActiveLow, ExclusiveAndWake, PullUp, 0x0000,
                    "\\_SB.GPO1", 0x00, ResourceConsumer, ,
                    )
                    {   // Pin list
                        0x0012
                    }
            })
            Name (SBFI, ResourceTemplate ()
            {
                Interrupt (ResourceConsumer, Level, ActiveLow, ExclusiveAndWake, ,, )
                {
                    0x0000003C,
                }
            })
            Method (_INI, 0, NotSerialized)  // _INI: Initialize
            {
            }
            Method (_STA, 0, NotSerialized)  // _STA: Status
            {
                If ((TCPD == One))
                {
                    Return (0x0F)
                }

                Return (Zero)
            }
            Method (_CRS, 0, NotSerialized)  // _CRS: Current Resource Settings
            {
                If ((OSYS < 0x07DC))
                {
                    Return (SBFI) /* \_SB_.PCI0.I2C4.TPD0.SBFI */
                }

                Return (ConcatenateResTemplate (SBFB, SBFG))
            }
            Method (_DSM, 4, NotSerialized)  // _DSM: Device-Specific Method
            {
                If ((Arg0 == ToUUID ("3cdff6f7-4267-4555-ad05-b30a3d8938de") /*
HID
I2C Device */))
               {
                    If ((Arg2 == Zero))
                    {
                        If ((Arg1 == One))
                        {
                            Return (Buffer (One)
                            {
                                 0x03                                             // .
                            })
                        }
                        Else
                        {
                            Return (Buffer (One)
                            {
                                 0x00                                             // .
                            })
                        }
                    }
                    ElseIf ((Arg2 == One))
                    {
                        Return (0x20)
                    }
                    Else
                    {
                        Return (Buffer (One)
                        {
                             0x00                                             // .
                        })
                    }
                }
                ElseIf ((Arg0 == ToUUID ("ef87eb82-f951-46da-84ec-14871ac6f84b")))
                {
                    If ((Arg2 == Zero))
                    {
                        If ((Arg1 == One))
                        {
                            Return (Buffer (One)
                            {
                                 0x03                                             // .
                            })
                        }
                    }

                    If ((Arg2 == One))
                    {
                        Return (ConcatenateResTemplate (SBFB, SBFG))
                    }

                    Return (Buffer (One)
                    {
                         0x00                                             // .
                    })
                }
                }
                Else
                {
                    Return (Buffer (One)
                    {
                         0x00                                             // .
                    })
                }
            }
        }
    }

Change SBFG to SBFI in its _CRS can workaround the issue.
Is ASL in this form possible to do the flow you described?

Kai-Heng

But I am not sure how to patch its DSDT/SSDT in i2c-hid.

Is this pre-production HW?  If so, maybe this is a case that we should  
talk
about custom OSI string to run the ASL differently.

Some are already shipped.

The other option would be to create a new ASL method in FW and from Linux
side a quirk that calls this new ASL method.

Since this patch is for ASUS Laptop, I think a more generic solution from
driver layer is preferred.

I thought this ASL was from Dell laptop.  The HID make it looks like it  
at least.

Yes this ASL is from a Dell system.

           Name (_HID, "DELL08AE")  // _HID: Hardware ID

In general more generic solution from driver layer is preferred I agree.   
You might
need to check with ACPI guys to see if they have some ideas on situations  
that
GpioInt fail.

I’ve filed a bug here but we don’t find any proper solution:
https://bugzilla.kernel.org/show_bug.cgi?id=201311

Kai-Heng