Re: [PATCH v2 4/8] input: goodix: reset device at init

Octavian Purdila
2015-06-23T14:50:55Z

On Tue, Jun 23, 2015 at 5:12 PM, Bastien Nocera <hadess@hadess.net> wrote:
>
> On Tue, 2015-06-23 at 13:23 +0000, Tirdea, Irina wrote:
> >
> > > -----Original Message-----
> > > From: linux-input-owner@vger.kernel.org [mailto:
> > > linux-input-owner@vger.kernel.org] On Behalf Of Bastien Nocera
> > > Sent: 09 June, 2015 18:53
> > > To: Tirdea, Irina
> > > Cc: Dmitry Torokhov; Mark Rutland; linux-input@vger.kernel.org;
> > > devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; Rob
> > > Herring; Pawel Moll; Ian Campbell; Kumar Gala; Purdila, Octavian
> > > Subject: Re: [PATCH v2 4/8] input: goodix: reset device at init
> > >
> > > On Tue, 2015-06-09 at 17:34 +0200, Bastien Nocera wrote:
> > > > On Mon, 2015-06-08 at 17:37 +0300, Irina Tirdea wrote:
> > > > > After power on, it is recommended that the driver resets the
> > > > > device.
> > > > > The reset procedure timing is described in the datasheet and is
> > > > > used
> > > > > at device init (before writing device configuration) and
> > > > > for power management. It is a sequence of setting the interrupt
> > > > > and reset pins high/low at specific timing intervals. This
> > > > > procedure
> > > > > also includes setting the slave address to the one specified in
> > > > > the
> > > > > ACPI/device tree.
> > > >
> > > > This breaks the touchscreen on my Onda v975w:
> > > > [ 239.732858] Goodix-TS i2c-GDIX1001:00: ID 9271, version: 1020
> > > > [ 239.732977] Goodix-TS i2c-GDIX1001:00: Failed to get reset
> > > > GPIO:
> > > > -16
> > > > [ 239.736071] Goodix-TS: probe of i2c-GDIX1001:00 failed with
> > > > error
> > > > -16
> > > >
> > > > This is the DSDT for that device:
> > > > https://bugzilla.kernel.org/attachment.cgi?id=149331
> > >
> >
> > Oops. That's right - I am using named interrupts which are available
> > only for ACPI 5.1, so
> > devices already out there will not work.
> >
> > The problem here is that handling -ENOENT will not help. The gpio
> > pins are declared in the
> > ACPI DSDT, but are not named. The devm_gpiod_get API will look for
> > the named interrupt
> > first and fallback to searching by index if not found. Index will be
> > 0 in both cases, this is why
> > the first call will succeed and the second will fail with -EBUSY.
> >
> > One way to handle this would be to use indexed gpio pins instead of
> > named gpio pins:
> > e.g. consider the first gpio pin to be the reset pin and the second
> > to be the interrupt pin.
> > This is used in other drivers as well, e.g. zforce_ts. The problem
> > with this approach is that
> > any devices that declare the gpio pins in reversed order in the DSDT
> > will not work and give
> > no warning (the pins will be requested successfully, but some of the
> > functionality will not
> > work). The DSDT mentioned in
> > https://bugzilla.kernel.org/attachment.cgi?id=149331 lists
> > the reset pin first, while I am working on some devices that declare
> > the interrupt gpio pin
> > first.
> >
> > Another way to handle this is to treat -EBUSY like -ENOENT and not
> > use any functionality
> > that depends on the gpio pins. Unfortunately, this means we will not
> > have suspend, esd and
> > custom configs even if the pins are connected on the board and
> > available in ACPI(just not
> > named).
> >
> > I would go with the first approach and document the order requested
> > for the pins, but I would
> > like to hear what you think first. Is there a better way to do this?
> >
> > > (Note that this means that I haven't been able to test any
> > > following
> > > patches in that series than 4/8).
> >
> > Sure, that makes sense. The following patches depend on the gpio pins
> > so they would not have
> > worked either.
>
> We can apply quirks based on DMI information, so that devices with ACPI
> in different orders will work after applying a quirk (as long as
> there's a way to detect that it's in the wrong order, obviously).
>

I think even using the ACPI id (_HID) should be enough (at least in
the cases we know so far) to make the difference in how the pins are
used.