Re: Configuring bias on an APCI GpioInt

[Andy Shevchenko <andy.shevchenko@xxxxxxxxx>]

On Wed, Oct 14, 2020 at 3:58 PM Andy Shevchenko
<andy.shevchenko@xxxxxxxxx> wrote:
>
> On Sat, Oct 10, 2020 at 11:46 PM Andy Shevchenko
> <andy.shevchenko@xxxxxxxxx> wrote:
> > On Sat, Oct 10, 2020 at 11:58 AM Jamie McClymont <jamie@xxxxxxxxxx> wrote:
>
> > > I have run into a second GPIO issue while writing a driver for the fingerprint sensor in my laptop*, configured in the ACPI table like so:
> > >
> > >    Method (_CRS, 0, Serialized)  // _CRS: Current Resource Settings
> > >    {
> > >        Name (RBUF, ResourceTemplate ()
> > >        {
> > >            // SPI
> > >            SpiSerialBusV2 (0x0000, PolarityLow, FourWireMode, 0x08,
> > >                ControllerInitiated, 0x00989680, ClockPolarityLow,
> > >                ClockPhaseFirst, "\\_SB.PCI0.SPI1",
> > >                0x00, ResourceConsumer, , Exclusive,
> > >                )
> > >
> > >            // Interrupt
> > >            GpioInt (Level, ActiveLow, ExclusiveAndWake, PullUp, 0x0000,
> > >                "\\_SB.PCI0.GPI0", 0x00, ResourceConsumer, ,
> > >                )
> > >                {   // Pin list
> > >                    0x0000
> > >                }
> > >
> > >            // Reset
> > >            GpioIo (Exclusive, PullUp, 0x0000, 0x0000, IoRestrictionOutputOnly,
> > >                "\\_SB.PCI0.GPI0", 0x00, ResourceConsumer, ,
> > >                )
> > >                {   // Pin list
> > >                    0x0008
> > >                }
> > >        })
> > >        CreateWordField (RBUF, 0x3B, GPIN)
> > >        CreateWordField (RBUF, 0x63, SPIN)
> > >        GPIN = GNUM (0x02000017)
> > >        SPIN = GNUM (0x0202000A)
> > >        Return (RBUF) /* \_SB_.SPBA._CRS.RBUF */
> > >    }
> > >
> > > I call devm_request_threaded_irq with the number provided in the irq field of the `struct spi_device` during the spi_probe.
> > >
> > > This configures the right IRQ number, and it triggers when it should, but it stays asserted for 140ms-600ms after it should have been cleared.
> >
> > I was wondering how it works since it should have the same issue as
> > with a regular GPIO. Now I understand that we simply ignore bias for
> > GpioInt() resources. If you enable it, we come to the same issue. So,
> > first we have to fix set bias followed by enabling it for GpioInt()
> > resources.
> >
> > I will look closer at this next week.
> >
> >
> > > Given it's an active-low level-triggered interrupt with a pull-up requested by the acpi table, my theory is:
> > >
> > > * The interrupt line is driven open-drain by the peripheral
> > > * The pullup is not being correctly configured
> > > * It is slooooowly pulled up by leakage current of the GPIO input, hence the interrupt being cleared after 140-600ms
> > >
> > > Looking at traces and confirming by source code**, it seems that no code ever attempts to configure the pin's bias during the irq setup.
> > >
> > > Is this a bug, or should I be manually setting up the GPIO some other way before requesting the IRQ?
> > >
> > > Thanks
> > > - Jamie McClymont
> > >
> > >
> > > *Hardware details
> > > ==============
> > >
> > > Laptop is a Huawei Matebook X Pro
> > > CPU is an Intel i5-8250U (the specific pinctrl is pinctrl_sunrisepoint)
> > > Fingerprint Sensor is a Goodix GXFP5187 (not well-documented anywhere, I'm working through reverse-engineering)
> > >
> > > ** Tested on 5.9-rc8, but I've been reading the source of linux-next and haven't picked up on any relevant differences
>
> I'm about to send the possible fix which has to be applied on top of
> the [1]. As per previous thread it would be nice to have [2] applied
> as well. Please, test it all together.
>
> [1]: https://lore.kernel.org/linux-gpio/20201014104638.84043-1-andriy.shevchenko@xxxxxxxxxxxxxxx/T/#u
> [2]: https://lore.kernel.org/linux-gpio/20201009184359.16427-1-andriy.shevchenko@xxxxxxxxxxxxxxx/

Done. See [3] (first patch in the series).

[3]: https://lore.kernel.org/linux-gpio/20201014133154.30610-2-andriy.shevchenko@xxxxxxxxxxxxxxx/T/#u


-- 
With Best Regards,
Andy Shevchenko