Re: [PATCH] ELAN touchpad i2c_hid bugs fix

[Hans de Goede]

Hi,

On 21-03-19 05:08, Kai-Heng Feng 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,
>  ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ }
>  ÂÂÂÂÂÂÂÂÂÂÂ })

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

Regards,

Hans