Re: [PATCH v3 1/5] Input: goodix - reset device at init

From: Bastien Nocera
Date: Wed Sep 30 2015 - 07:15:58 EST


On Tue, 2015-09-29 at 17:47 +0000, Tirdea, Irina wrote:
>
> > -----Original Message-----
> > From: Bastien Nocera [mailto:hadess@xxxxxxxxxx]
> > Sent: 29 September, 2015 5:04
> > To: Tirdea, Irina; linux-input@xxxxxxxxxxxxxxx
> > Cc: linux-kernel@xxxxxxxxxxxxxxx; Rob Herring; Pawel Moll; Ian
> > Campbell; Kumar Gala; Purdila, Octavian; Dmitry Torokhov; Mark
> > Rutland; devicetree@xxxxxxxxxxxxxxx
> > Subject: Re: [PATCH v3 1/5] Input: goodix - reset device at init
> >
> > On Fri, 2015-09-25 at 21:04 +0000, Tirdea, Irina wrote:
> > >
> <snip>
> > >
> > > The warning from your dmesg output will not cause probe to fail.
> > > If you look at the code for byt_gpio_direction_output, it will
> > > just
> > > print
> > > a warning and continue [1]:
> > > WARN(readl(conf_reg) & BYT_DIRECT_IRQ_EN,
> > > "Potential Error: Setting GPIO with direct_irq_en to
> > > output");
> > > I thought probe finishes successfully, but due to the warning in
> > > dmesg you
> > > are not sure whether the IRQ GPIO pin can be used as output.
> > > If probe fails, it must be for another reason than the
> > > direct_irq_en
> > > warning.
> > >
> > > > Would you have a patch for me to test that would bypass this
> > > > error,
> > > > or
> > > > at least fallback gracefully to not resetting, not probing
> > > > GPIOs if
> > > > they're badly setup?
> > >
> > > If the driver fails to initialize the GPIOs, it will at least
> > > print
> > > some
> > > "Failed to get GPIO" warnings in dmesg. Do you have such messages
> > > in
> > > dmesg or any additional information on why probe fails?
> > >
> > > The current code will ignore GPIOs if they are not defined in
> > > ACPI
> > > (see the check for -ENOENT), but does not ignore other error
> > > codes.
> > > If you want to bypass all GPIO errors, you can use the code
> > > below.
> >
> > The failure isn't there, it's when runningÂgoodix_i2c_test():
> > Sep 25 16:39:20 winbook kernel: Goodix-TS i2c-GDIX1001:00: i2c test
> > failed attempt 1: -121
> > Sep 25 16:39:20 winbook kernel: Goodix-TS i2c-GDIX1001:00: i2c test
> > failed attempt 2: -121
> > Sep 25 16:39:20 winbook kernel: Goodix-TS i2c-GDIX1001:00: I2C
> > communication failure: -121
> > Sep 25 16:39:20 winbook kernel: Goodix-TS: probe of i2c-GDIX1001:00
> > failed with error -121
> >
>
> Are you using v6 of the patches? There was an issue with reset that
> Aleksei reported
> and was fixed in v6 (although he had a different i2c error and a
> different scenario).

Pretty certain. Your current patchset is at:
https://github.com/hadess/gt9xx/tree/irina-tirdea

And the patches are yours, with the prefix and Documentation removed.

> > The GPIO setup seems to work (bar the warnings), and the reset as
> > well,
> > but then the device fails to communicate. Likely a fallout from the
> > reset actually failing.
> >
> > Swapping around the RST and INT pins leads to the same problem.
> > Either
> > this device's GPIO PINs aren't actually functional, and the
> > firmware
> > contains garbage, or something else is wrong.
> >
>
> I agree. Either the interrupt pin cannot be used as output in your
> configuration
> or there are some specifics in the ACPI tables that prevent using
> these pins.
>
> > I'm not sure how we can detect, and blacklist, those devices. At
> > least
> > my original device, the Onda v975w, and the WinBook TW100 would
> > have
> > those problems.
> >
>
> I can use DMI quirks to exclude these devices from using the features
> that
> depend on the gpio pins. I already have the DMI information for
> WinBook TW100
> and WinBook TW700. Could you tell me the DMI_SYS_VENDOR and
> DMI_PRODUCT_NAME for Onda v975w so I can add it as well?

I don't have access to the Onda v975w anymore, but let me CC: a few
people that could also help with testing.

Carlos, Cosimo, Christian, there's a patchset for you to test on the
Onda v975w at:
https://github.com/hadess/gt9xx/commits/irina-tirdea

Doing an "rmmod goodix ; insmod ./goodix_backport.ko" should be enough
to test whether the patch set works. If it doesn't work correctly,
you'll need to reboot the machine, swap the irq_idx and rst_idx values
at:
https://github.com/hadess/gt9xx/commit/c27de79f494c2b2e7a198ff4d27976ae93669dbd#diff-dddc2849e36327439530f3e2faacec4fR321

and try again.

If all that fails, could you please send the output of "dmidecode" to
Irina?

Cheers
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/