Re: [PATCH] ELAN touchpad i2c_hid bugs fix

[Hans de Goede <hdegoede@redhat.com>]

Hi,

On 21-03-19 05:08, Kai-Heng Feng wrote:
> at 01:18, Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> 
>> On Wed, Mar 20, 2019 at 6:55 PM Kai-Heng Feng
>> <kai.heng.feng@canonical.com> wrote:
>>> at 23:39, Hans de Goede <hdegoede@redhat.com> 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,
>                  }
>              })

OK, so both interrupt definitions declare the interrupt as Level, ActiveLow, so forcing
falling-edge here *might* help too.

Regards,

Hans