-----Original Message-----
From: Bastien Nocera [mailto:hadess@hadess.net]
Sent: 25 September, 2015 17:44
To: Tirdea, Irina; linux-input@vger.kernel.org
Cc: linux-kernel@vger.kernel.org; Rob Herring; Pawel Moll; Ian Campbell; Kumar Gala; Purdila, Octavian; Dmitry Torokhov; Mark
Rutland; devicetree@vger.kernel.org
Subject: Re: [PATCH v3 1/5] Input: goodix - reset device at init

On Thu, 2015-09-10 at 14:04 +0000, Tirdea, Irina wrote:

quoted

quoted

-----Original Message-----
From: Bastien Nocera [mailto:hadess@hadess.net]
Sent: 09 September, 2015 20:03
To: Tirdea, Irina; linux-input@vger.kernel.org
Cc: linux-kernel@vger.kernel.org; Rob Herring; Pawel Moll; Ian
Campbell; Kumar Gala; Purdila, Octavian; Dmitry Torokhov; Mark
Rutland; devicetree@vger.kernel.org
Subject: Re: [PATCH v3 1/5] Input: goodix - reset device at init

On Thu, 2015-07-30 at 11:27 +0000, Tirdea, Irina wrote:

quoted

I can send some additional patches that will simplify testing the
configuration update to the Goodix device. I think this feature
is
the easiest
to test so we can determine if writing to the interrupt pin
actually
works.
However, even if it is a BIOS problem and the code will work, the
warning
will still be printed in dmesg.

Somehow missed this mail before replying to the current patchset.
I'd
be fine with that, though it's still not clear to me whether the
BIOS/hardware is at fault, or the code that's being added to the
driver
;)

The reset procedure is described in the Goodix GT911 datasheet [1]
and is
used for power-on reset and power management. The power-on sequence
is described in chapter 6.1. I2C Timing, in the Power-on Timing
diagram.
The sequence for putting the device to sleep is described in chapter
7.1. Operating Modes, c) Sleep mode. These sequences use the
interrupt
pin as output.

The warning you mentioned comes from the following code in the goodix
driver, which sets the interrupt to be used as output:

+       error = gpiod_direction_output(ts->gpiod_int, ts->client-

quoted

addr == 0x14);

The gpiod_direction_output() call ends up in
drivers/pinctrl/intel/pinctrl-baytrail.c:
/*
 * Before making any direction modifications, do a check if gpio
 * is set for direct IRQ.  On baytrail, setting GPIO to output does
 * not make sense, so let's at least warn the caller before they
shoot
 * themselves in the foot.
 */
WARN(readl(conf_reg) & BYT_DIRECT_IRQ_EN,
	"Potential Error: Setting GPIO with direct_irq_en to output");

So the problem comes from using the gpio interrupt pin as output,
which
should not work on Baytrail if BYT_DIRECT_IRQ_EN is set by BIOS.
The above warning is introduced and discussed in [2] and [3]. As I
mentioned,
this could be a real HW issue or the BIOS sets BYT_DIRECT_IRQ_EN when
it should not. I have also tested these patches on a Baytrail
plarform
(that uses the same pinctrl driver), but I did not see any issues
since
BYT_DIRECT_IRQ_EN is not set in my case for the interrupt gpio pin.

Do we have a way to work-around this in the GPIO driver?

quoted

To determine if using the interrupt pin as output works, you can test
updating
the goodix configuration [4].

Right, the problem being that it's a later patch in the branch, and
that the driver will fail to probe if there's an error from the GPIO
call.

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.

Thanks,
Irina

diff --git a/drivers/input/touchscreen/goodix.c b/drivers/input/touchscreen/goodix.c
index 9c58583..f7ec0ba 100644
--- a/drivers/input/touchscreen/goodix.c
+++ b/drivers/input/touchscreen/goodix.c

@@ -528,9 +528,7 @@ static int goodix_ts_probe(struct i2c_client *client,
        ts->client = client;
        i2c_set_clientdata(client, ts);
 
-       error = goodix_get_gpio_config(ts, id);
-       if (error)
-               return error;
+       goodix_get_gpio_config(ts, id);
 
        if (ts->gpiod_int && ts->gpiod_rst) {
                /* reset the controller */

[1] http://lxr.free-electrons.com/source/drivers/pinctrl/intel/pinctrl-baytrail.c#L337