About Goodix-TS on Bay Trail, and ACPI and interrupts

About Goodix-TS on Bay Trail, and ACPI and interrupts

[Antonio Ospite]

Hi,

I am trying to make the Goodix driver (drivers/input/touchscreen/goodix.c)
working with a Teclast X98 Air 3G, a tablet based on Intel Bay Trail,
but I am new to ACPI and I could use some help.

I am working with a 3.19-rc4 kernel compiled for x86_64.

This is the DSDT section in the UEFI firmware:

            Device (TCS0)
            {
                Name (_ADR, Zero)  // _ADR: Address
                Name (_HID, "GODX0911")  // _HID: Hardware ID
                Name (_CID, "PNP0C50" /* HID Protocol Device (I2C bus) */)  // _CID: Compatible ID
                Name (_S0W, Zero)  // _S0W: S0 Device Wake State
                Name (_DEP, Package (0x02)  // _DEP: Dependencies
                {
                    GPO1, 
                    I2C5
                })
                Method (_PS3, 0, Serialized)  // _PS3: Power State 3
                {
                    If ((^^^I2C5.PMIC.AVBG == One)) {}
                }

                Method (_PS0, 0, Serialized)  // _PS0: Power State 0
                {
                    If ((^^^GPO1.AVBL == One))
                    {
                        ^^^GPO1.TCD3 = Zero
                    }

                    Sleep (0x05)
                    If ((^^^I2C5.PMIC.AVBG == One)) {}
                    Sleep (0x1E)
                    If ((^^^GPO1.AVBL == One))
                    {
                        ^^^GPO1.TCD3 = One
                    }

                    Sleep (0x78)
                }

                Method (_CRS, 0, NotSerialized)  // _CRS: Current Resource Settings
                {
                    Name (RBUF, ResourceTemplate ()
                    {
                        I2cSerialBus (0x0014, ControllerInitiated, 0x0019F0A0,
                            AddressingMode7Bit, "\\_SB.I2C4",
                            0x00, ResourceConsumer, ,
                            )
                        GpioIo (Exclusive, PullDefault, 0x0000, 0x0000, IoRestrictionInputOnly,
                            "\\_SB.GPO2", 0x00, ResourceConsumer, ,
                            )
                            {   // Pin list
                                0x0044
                            }
                    })
                    Name (ABUF, ResourceTemplate ()
                    {
                        I2cSerialBus (0x0014, ControllerInitiated, 0x0019F0A0,
                            AddressingMode7Bit, "\\_SB.I2C4",
                            0x00, ResourceConsumer, ,
                            )
                        GpioIo (Exclusive, PullDefault, 0x0000, 0x0000, IoRestrictionInputOnly,
                            "\\_SB.GPO2", 0x00, ResourceConsumer, ,
                            )
                            {   // Pin list
                                0x0044
                            }
                        GpioIo (Exclusive, PullDefault, 0x0000, 0x0000, IoRestrictionOutputOnly,
                            "\\_SB.GPO1", 0x00, ResourceConsumer, ,
                            )
                            {   // Pin list
                                0x001A
                            }
                    })
                    If ((OSSL && 0x80))
                    {
                        Return (ABUF) /* \_SB_.I2C4.TCS0._CRS.ABUF */
                    }
                    Else
                    {
                        Return (RBUF) /* \_SB_.I2C4.TCS0._CRS.RBUF */
                    }
                }

                Method (_DSM, 4, Serialized)  // _DSM: Device-Specific Method
                {
                    Name (_T_1, Zero)  // _T_x: Emitted by ASL Compiler
                    Name (_T_0, Zero)  // _T_x: Emitted by ASL Compiler
                    Debug = "Method _DSM begin"
                    If ((Arg0 == ToUUID ("3cdff6f7-4267-4555-ad05-b30a3d8938de") /* HID I2C Device */))
                    {
                        While (One)
                        {
                            _T_0 = ToInteger (Arg2)
                            If ((_T_0 == Zero))
                            {
                                While (One)
                                {
                                    _T_1 = ToInteger (Arg1)
                                    If ((_T_1 == One))
                                    {
                                        Debug = "Method _DSM Function Query"
                                        Return (Buffer (One)
                                        {
                                             0x03                                             /* . */
                                        })
                                    }
                                    Else
                                    {
                                        Return (Buffer (One)
                                        {
                                             0x00                                             /* . */
                                        })
                                    }

                                    Break
                                }
                            }
                            Else
                            {
                                If ((_T_0 == One))
                                {
                                    Debug = "Method _DSM Function HID"
                                    Return (Zero)
                                }
                                Else
                                {
                                    Return (Zero)
                                }
                            }

                            Break
                        }
                    }
                    Else
                    {
                        Return (Buffer (One)
                        {
                             0x00                                             /* . */
                        })
                    }
                }

                Method (_STA, 0, NotSerialized)  // _STA: Status
                {
                    If ((OSSL == 0x83))
                    {
                        Return (Zero)
                    }

                    Return (0x0F)
                }
            }

BTW in the DSDT there are two TCS0 entries, but the one above should be
the one referring to the actual hardware on my unit, and it's the one
picked up by linux anyway.

Full DSDT here: http://ao2.it/tmp/Goodix-TS_Teclast-X98-Air-3G/dsdt.dsl

I added the new ACPI _HID to the Goodix-TS driver and the driver gets
loaded.

I2C communication seems to work fine as I can read the product id, but
the driver probing fails to complete because it cannot request the IRQ.

Here is the full dmesg:
http://ao2.it/tmp/Goodix-TS_Teclast-X98-Air-3G/dmesg_Teclast-X98-Air-3G_mainline.log

Grepping for GODX0911 also shows that the i2c-hid driver tries to do
something because of the _DSM method (also _CID == "PNP0C50").

Comparing with Bastien's DSDT I noticed that there is no ACPI Interrupt
resource listed above, and so I thought that linux couldn't get the irq
number from ACPI.

However the Android driver works with IRQs, not in polling mode:
<6>[    7.585262] Goodix_TS 4-0014: GTP I2C Address: 0x14
<6>[    7.585354] Goodix_TS 4-0014: INT gpio 133 to irq 389

Here is the full Android dmesg:
http://ao2.it/tmp/Goodix-TS_Teclast-X98-Air-3G/dmesg_Teclast-X98-Air-3G_Android.log

They could have worked around the missing ACPI resource in the code, but
Windows also works on this tablet (I do not have it installed tho).
All the vendor firmwares I found use the same DSDT.

The pinctrl-baytrail driver used for the gpio setup _seems_ to be fine,
i.e. input from other gpios works.

Any ideas?

I tried retrieving the gpio number with devm_gpiod_get_index() from the
GpioIo resource but I am not sure whether this is OK, is that resource
meant to indicate the same "touch" irq which I would have expected in an
Interrupt resource, or are those GpioIo for power/reset?

Patch is here, anyway:
http://ao2.it/tmp/Goodix-TS_Teclast-X98-Air-3G/0001-XXX-goodix-add-support-for-GODX0911.patch

Anyhow, even if this approach was OK, or we wanted to use the GpioIo to
control power explicitly, I'd notice that the pin number is 0x44 (hex)
on GPO2 (the third bank), but GPO2 has max 44 (decimal) pins, so it would
be still invalid. 

Obviously I am still missing something.

Thanks,
   Antonio

-- 
Antonio Ospite
http://ao2.it

A: Because it messes up the order in which people normally read text.
   See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?

Re: About Goodix-TS on Bay Trail, and ACPI and interrupts

[Benjamin Tissoires]

Hi Antonio,

[adding Mika in CC, he implemented most of the ACPI and GPIO for
i2c-hid]

On Jan 17 2015 or thereabouts, Antonio Ospite wrote:
> Hi,
> 
> I am trying to make the Goodix driver (drivers/input/touchscreen/goodix.c)
> working with a Teclast X98 Air 3G, a tablet based on Intel Bay Trail,
> but I am new to ACPI and I could use some help.
> 
> I am working with a 3.19-rc4 kernel compiled for x86_64.
> 
> This is the DSDT section in the UEFI firmware:
> 
>             Device (TCS0)
>             {
>                 Name (_ADR, Zero)  // _ADR: Address
>                 Name (_HID, "GODX0911")  // _HID: Hardware ID
>                 Name (_CID, "PNP0C50" /* HID Protocol Device (I2C bus) */)  // _CID: Compatible ID

urgh, this is bad. It declares itself as i2c-hid, but it is not :(
Anyway, according to your logs, i2c-hid probe() just fails, so it's not
a big problem.

>                 Name (_S0W, Zero)  // _S0W: S0 Device Wake State
>                 Name (_DEP, Package (0x02)  // _DEP: Dependencies
>                 {
>                     GPO1, 
>                     I2C5
>                 })
>                 Method (_PS3, 0, Serialized)  // _PS3: Power State 3
>                 {
>                     If ((^^^I2C5.PMIC.AVBG == One)) {}
>                 }
> 
>                 Method (_PS0, 0, Serialized)  // _PS0: Power State 0
>                 {
>                     If ((^^^GPO1.AVBL == One))
>                     {
>                         ^^^GPO1.TCD3 = Zero
>                     }
> 
>                     Sleep (0x05)
>                     If ((^^^I2C5.PMIC.AVBG == One)) {}
>                     Sleep (0x1E)
>                     If ((^^^GPO1.AVBL == One))
>                     {
>                         ^^^GPO1.TCD3 = One
>                     }
> 
>                     Sleep (0x78)
>                 }
> 
>                 Method (_CRS, 0, NotSerialized)  // _CRS: Current Resource Settings
>                 {
>                     Name (RBUF, ResourceTemplate ()
>                     {
>                         I2cSerialBus (0x0014, ControllerInitiated, 0x0019F0A0,
>                             AddressingMode7Bit, "\\_SB.I2C4",
>                             0x00, ResourceConsumer, ,
>                             )
>                         GpioIo (Exclusive, PullDefault, 0x0000, 0x0000, IoRestrictionInputOnly,
>                             "\\_SB.GPO2", 0x00, ResourceConsumer, ,
>                             )
>                             {   // Pin list
>                                 0x0044
>                             }
>                     })
>                     Name (ABUF, ResourceTemplate ()
>                     {
>                         I2cSerialBus (0x0014, ControllerInitiated, 0x0019F0A0,
>                             AddressingMode7Bit, "\\_SB.I2C4",
>                             0x00, ResourceConsumer, ,
>                             )
>                         GpioIo (Exclusive, PullDefault, 0x0000, 0x0000, IoRestrictionInputOnly,
>                             "\\_SB.GPO2", 0x00, ResourceConsumer, ,
>                             )
>                             {   // Pin list
>                                 0x0044
>                             }
>                         GpioIo (Exclusive, PullDefault, 0x0000, 0x0000, IoRestrictionOutputOnly,
>                             "\\_SB.GPO1", 0x00, ResourceConsumer, ,
>                             )
>                             {   // Pin list
>                                 0x001A
>                             }
>                     })

It looks like the GPIOs are correctly declared. The ACPI code should set
the client->irq auto-magically. It's not the case, so I guess Mika
should be able to tell us more on that.


>                     If ((OSSL && 0x80))
>                     {
>                         Return (ABUF) /* \_SB_.I2C4.TCS0._CRS.ABUF */
>                     }
>                     Else
>                     {
>                         Return (RBUF) /* \_SB_.I2C4.TCS0._CRS.RBUF */
>                     }
>                 }
> 
>                 Method (_DSM, 4, Serialized)  // _DSM: Device-Specific Method
>                 {
>                     Name (_T_1, Zero)  // _T_x: Emitted by ASL Compiler
>                     Name (_T_0, Zero)  // _T_x: Emitted by ASL Compiler
>                     Debug = "Method _DSM begin"
>                     If ((Arg0 == ToUUID ("3cdff6f7-4267-4555-ad05-b30a3d8938de") /* HID I2C Device */))
>                     {
>                         While (One)
>                         {
>                             _T_0 = ToInteger (Arg2)
>                             If ((_T_0 == Zero))
>                             {
>                                 While (One)
>                                 {
>                                     _T_1 = ToInteger (Arg1)
>                                     If ((_T_1 == One))
>                                     {
>                                         Debug = "Method _DSM Function Query"
>                                         Return (Buffer (One)
>                                         {
>                                              0x03                                             /* . */
>                                         })
>                                     }
>                                     Else
>                                     {
>                                         Return (Buffer (One)
>                                         {
>                                              0x00                                             /* . */
>                                         })
>                                     }
> 
>                                     Break
>                                 }
>                             }
>                             Else
>                             {
>                                 If ((_T_0 == One))
>                                 {
>                                     Debug = "Method _DSM Function HID"
>                                     Return (Zero)
>                                 }
>                                 Else
>                                 {
>                                     Return (Zero)
>                                 }
>                             }
> 
>                             Break
>                         }
>                     }
>                     Else
>                     {
>                         Return (Buffer (One)
>                         {
>                              0x00                                             /* . */
>                         })
>                     }
>                 }
> 
>                 Method (_STA, 0, NotSerialized)  // _STA: Status
>                 {
>                     If ((OSSL == 0x83))
>                     {
>                         Return (Zero)
>                     }
> 
>                     Return (0x0F)
>                 }
>             }
> 
> BTW in the DSDT there are two TCS0 entries, but the one above should be
> the one referring to the actual hardware on my unit, and it's the one
> picked up by linux anyway.
> 
> Full DSDT here: http://ao2.it/tmp/Goodix-TS_Teclast-X98-Air-3G/dsdt.dsl
> 
> I added the new ACPI _HID to the Goodix-TS driver and the driver gets
> loaded.
> 
> I2C communication seems to work fine as I can read the product id, but
> the driver probing fails to complete because it cannot request the IRQ.
> 
> Here is the full dmesg:
> http://ao2.it/tmp/Goodix-TS_Teclast-X98-Air-3G/dmesg_Teclast-X98-Air-3G_mainline.log
> 
> Grepping for GODX0911 also shows that the i2c-hid driver tries to do
> something because of the _DSM method (also _CID == "PNP0C50").

Yeah. Actually, it should bail out earlier because client->irq is -1.
I guess we should change the first test to be "if (client->irq <= 0) {
goto err;}".

> 
> Comparing with Bastien's DSDT I noticed that there is no ACPI Interrupt
> resource listed above, and so I thought that linux couldn't get the irq
> number from ACPI.
> 
> However the Android driver works with IRQs, not in polling mode:
> <6>[    7.585262] Goodix_TS 4-0014: GTP I2C Address: 0x14
> <6>[    7.585354] Goodix_TS 4-0014: INT gpio 133 to irq 389

IIRC, the ACPI generic code would bind the first GPIO as the interrupt
resource. It does not, so either something is wrong in the DSDT, or
something is wrong in the GPIO/IRQ association.

> 
> Here is the full Android dmesg:
> http://ao2.it/tmp/Goodix-TS_Teclast-X98-Air-3G/dmesg_Teclast-X98-Air-3G_Android.log
> 
> They could have worked around the missing ACPI resource in the code, but
> Windows also works on this tablet (I do not have it installed tho).
> All the vendor firmwares I found use the same DSDT.
> 
> The pinctrl-baytrail driver used for the gpio setup _seems_ to be fine,
> i.e. input from other gpios works.
> 
> Any ideas?
> 
> I tried retrieving the gpio number with devm_gpiod_get_index() from the
> GpioIo resource but I am not sure whether this is OK, is that resource
> meant to indicate the same "touch" irq which I would have expected in an
> Interrupt resource, or are those GpioIo for power/reset?
> 
> Patch is here, anyway:
> http://ao2.it/tmp/Goodix-TS_Teclast-X98-Air-3G/0001-XXX-goodix-add-support-for-GODX0911.patch
> 
> Anyhow, even if this approach was OK, or we wanted to use the GpioIo to
> control power explicitly, I'd notice that the pin number is 0x44 (hex)
> on GPO2 (the third bank), but GPO2 has max 44 (decimal) pins, so it would
> be still invalid. 
> 
> Obviously I am still missing something.
> 

I hope Mika will be more helpful than I can be. This whole ACPI parsing
and setting is too deep in acpica for me, unfortunately.

Cheers,
Benjamin

Re: About Goodix-TS on Bay Trail, and ACPI and interrupts

[Mika Westerberg]

[-- Attachment #1: Type: text/plain, Size: 4522 bytes --]

On Mon, Jan 19, 2015 at 10:37:58AM -0500, Benjamin Tissoires wrote:
> Hi Antonio,
> 
> [adding Mika in CC, he implemented most of the ACPI and GPIO for
> i2c-hid]
> 
> On Jan 17 2015 or thereabouts, Antonio Ospite wrote:
> > Hi,
> > 
> > I am trying to make the Goodix driver (drivers/input/touchscreen/goodix.c)
> > working with a Teclast X98 Air 3G, a tablet based on Intel Bay Trail,
> > but I am new to ACPI and I could use some help.
> > 
> > I am working with a 3.19-rc4 kernel compiled for x86_64.
> > 
> > This is the DSDT section in the UEFI firmware:
> > 
> >             Device (TCS0)
> >             {
> >                 Name (_ADR, Zero)  // _ADR: Address
> >                 Name (_HID, "GODX0911")  // _HID: Hardware ID
> >                 Name (_CID, "PNP0C50" /* HID Protocol Device (I2C bus) */)  // _CID: Compatible ID
> 
> urgh, this is bad. It declares itself as i2c-hid, but it is not :(
> Anyway, according to your logs, i2c-hid probe() just fails, so it's not
> a big problem.


Actually, I think this device should use i2c-hid. All the ACPI plumbing
is there including _DSM.

What makes you think it should use the goodix driver?

> 
> >                 Name (_S0W, Zero)  // _S0W: S0 Device Wake State
> >                 Name (_DEP, Package (0x02)  // _DEP: Dependencies
> >                 {
> >                     GPO1, 
> >                     I2C5
> >                 })
> >                 Method (_PS3, 0, Serialized)  // _PS3: Power State 3
> >                 {
> >                     If ((^^^I2C5.PMIC.AVBG == One)) {}
> >                 }
> > 
> >                 Method (_PS0, 0, Serialized)  // _PS0: Power State 0
> >                 {
> >                     If ((^^^GPO1.AVBL == One))
> >                     {
> >                         ^^^GPO1.TCD3 = Zero
> >                     }
> > 
> >                     Sleep (0x05)
> >                     If ((^^^I2C5.PMIC.AVBG == One)) {}
> >                     Sleep (0x1E)
> >                     If ((^^^GPO1.AVBL == One))
> >                     {
> >                         ^^^GPO1.TCD3 = One
> >                     }
> > 
> >                     Sleep (0x78)
> >                 }
> > 
> >                 Method (_CRS, 0, NotSerialized)  // _CRS: Current Resource Settings
> >                 {
> >                     Name (RBUF, ResourceTemplate ()
> >                     {
> >                         I2cSerialBus (0x0014, ControllerInitiated, 0x0019F0A0,
> >                             AddressingMode7Bit, "\\_SB.I2C4",
> >                             0x00, ResourceConsumer, ,
> >                             )
> >                         GpioIo (Exclusive, PullDefault, 0x0000, 0x0000, IoRestrictionInputOnly,
> >                             "\\_SB.GPO2", 0x00, ResourceConsumer, ,
> >                             )
> >                             {   // Pin list
> >                                 0x0044
> >                             }
> >                     })
> >                     Name (ABUF, ResourceTemplate ()
> >                     {
> >                         I2cSerialBus (0x0014, ControllerInitiated, 0x0019F0A0,
> >                             AddressingMode7Bit, "\\_SB.I2C4",
> >                             0x00, ResourceConsumer, ,
> >                             )
> >                         GpioIo (Exclusive, PullDefault, 0x0000, 0x0000, IoRestrictionInputOnly,
> >                             "\\_SB.GPO2", 0x00, ResourceConsumer, ,
> >                             )
> >                             {   // Pin list
> >                                 0x0044
> >                             }
> >                         GpioIo (Exclusive, PullDefault, 0x0000, 0x0000, IoRestrictionOutputOnly,
> >                             "\\_SB.GPO1", 0x00, ResourceConsumer, ,
> >                             )
> >                             {   // Pin list
> >                                 0x001A
> >                             }
> >                     })
> 
> It looks like the GPIOs are correctly declared. The ACPI code should set
> the client->irq auto-magically. It's not the case, so I guess Mika
> should be able to tell us more on that.

The current i2c-hid.c does not cope with GPIO interrupts. I've attached
an experimental patch that should convert the driver to use them.

Antonio, can you try that out and check if i2c-hid driver gets you
working touch screen? Also please add "i2c_hid.debug=1" to the kernel
command line so we can see if it returns proper HID descriptor.

[-- Attachment #2: 0001-HID-i2c-hid-Preliminary-support-for-GPIO-interrupts.patch --]
[-- Type: text/plain, Size: 5600 bytes --]

>From 9b7518976295978b28e1ad7a2404a139b0cd9345 Mon Sep 17 00:00:00 2001
From: Mika Westerberg <mika.westerberg@linux.intel.com>
Date: Fri, 3 Oct 2014 17:07:10 +0300
Subject: [PATCH] HID: i2c-hid: Preliminary support for GPIO interrupts

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
 drivers/hid/i2c-hid/i2c-hid.c | 70 ++++++++++++++++++++++++++++++++-----------
 1 file changed, 52 insertions(+), 18 deletions(-)

diff --git a/drivers/hid/i2c-hid/i2c-hid.c b/drivers/hid/i2c-hid/i2c-hid.c
index d43e967e7533..fd22e638830f 100644
--- a/drivers/hid/i2c-hid/i2c-hid.c
+++ b/drivers/hid/i2c-hid/i2c-hid.c
@@ -37,6 +37,7 @@
 #include <linux/mutex.h>
 #include <linux/acpi.h>
 #include <linux/of.h>
+#include <linux/gpio/consumer.h>
 
 #include <linux/i2c/i2c-hid.h>
 
@@ -144,6 +145,8 @@ struct i2c_hid {
 	unsigned long		flags;		/* device flags */
 
 	wait_queue_head_t	wait;		/* For waiting the interrupt */
+	struct gpio_desc	*desc;
+	int			irq;
 
 	struct i2c_hid_platform_data pdata;
 };
@@ -782,16 +785,16 @@ static int i2c_hid_init_irq(struct i2c_client *client)
 	struct i2c_hid *ihid = i2c_get_clientdata(client);
 	int ret;
 
-	dev_dbg(&client->dev, "Requesting IRQ: %d\n", client->irq);
+	dev_dbg(&client->dev, "Requesting IRQ: %d\n", ihid->irq);
 
-	ret = request_threaded_irq(client->irq, NULL, i2c_hid_irq,
+	ret = request_threaded_irq(ihid->irq, NULL, i2c_hid_irq,
 			IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
 			client->name, ihid);
 	if (ret < 0) {
 		dev_warn(&client->dev,
 			"Could not register for %s interrupt, irq = %d,"
 			" ret = %d\n",
-			client->name, client->irq, ret);
+			client->name, ihid->irq, ret);
 
 		return ret;
 	}
@@ -838,6 +841,14 @@ static int i2c_hid_fetch_hid_descriptor(struct i2c_hid *ihid)
 }
 
 #ifdef CONFIG_ACPI
+
+/* Default GPIO mapping */
+static const struct acpi_gpio_params i2c_hid_irq_gpio = { 0, 0, true };
+static const struct acpi_gpio_mapping i2c_hid_acpi_gpios[] = {
+	{ "irq-gpios", &i2c_hid_irq_gpio, 1 },
+	{ },
+};
+
 static int i2c_hid_acpi_pdata(struct i2c_client *client,
 		struct i2c_hid_platform_data *pdata)
 {
@@ -863,7 +874,7 @@ static int i2c_hid_acpi_pdata(struct i2c_client *client,
 	pdata->hid_descriptor_address = obj->integer.value;
 	ACPI_FREE(obj);
 
-	return 0;
+	return acpi_dev_add_driver_gpios(adev, i2c_hid_acpi_gpios);
 }
 
 static const struct acpi_device_id i2c_hid_acpi_match[] = {
@@ -927,12 +938,6 @@ static int i2c_hid_probe(struct i2c_client *client,
 
 	dbg_hid("HID probe called for i2c 0x%02x\n", client->addr);
 
-	if (!client->irq) {
-		dev_err(&client->dev,
-			"HID over i2c has not been provided an Int IRQ\n");
-		return -EINVAL;
-	}
-
 	ihid = kzalloc(sizeof(struct i2c_hid), GFP_KERNEL);
 	if (!ihid)
 		return -ENOMEM;
@@ -952,6 +957,25 @@ static int i2c_hid_probe(struct i2c_client *client,
 		ihid->pdata = *platform_data;
 	}
 
+	if (client->irq > 0) {
+		ihid->irq = client->irq;
+	} else {
+		ihid->desc = gpiod_get(&client->dev, "irq");
+		if (IS_ERR(ihid->desc)) {
+			dev_err(&client->dev, "Failed to get GPIO interrupt\n");
+			return PTR_ERR(ihid->desc);
+		}
+
+		gpiod_direction_input(ihid->desc);
+
+		ihid->irq = gpiod_to_irq(ihid->desc);
+		if (ihid->irq < 0) {
+			gpiod_put(ihid->desc);
+			dev_err(&client->dev, "Failed to convert GPIO to IRQ\n");
+			return ihid->irq;
+		}
+	}
+
 	i2c_set_clientdata(client, ihid);
 
 	ihid->client = client;
@@ -1014,13 +1038,16 @@ err_mem_free:
 	hid_destroy_device(hid);
 
 err_irq:
-	free_irq(client->irq, ihid);
+	free_irq(ihid->irq, ihid);
 
 err_pm:
 	pm_runtime_put_noidle(&client->dev);
 	pm_runtime_disable(&client->dev);
 
 err:
+	if (ihid->desc)
+		gpiod_put(ihid->desc);
+
 	i2c_hid_free_buffers(ihid);
 	kfree(ihid);
 	return ret;
@@ -1039,13 +1066,18 @@ static int i2c_hid_remove(struct i2c_client *client)
 	hid = ihid->hid;
 	hid_destroy_device(hid);
 
-	free_irq(client->irq, ihid);
+	free_irq(ihid->irq, ihid);
 
 	if (ihid->bufsize)
 		i2c_hid_free_buffers(ihid);
 
+	if (ihid->desc)
+		gpiod_put(ihid->desc);
+
 	kfree(ihid);
 
+	acpi_dev_remove_driver_gpios(ACPI_COMPANION(&client->dev));
+
 	return 0;
 }
 
@@ -1057,9 +1089,9 @@ static int i2c_hid_suspend(struct device *dev)
 	struct hid_device *hid = ihid->hid;
 	int ret = 0;
 
-	disable_irq(client->irq);
+	disable_irq(ihid->irq);
 	if (device_may_wakeup(&client->dev))
-		enable_irq_wake(client->irq);
+		enable_irq_wake(ihid->irq);
 
 	if (hid->driver && hid->driver->suspend)
 		ret = hid->driver->suspend(hid, PMSG_SUSPEND);
@@ -1077,13 +1109,13 @@ static int i2c_hid_resume(struct device *dev)
 	struct i2c_hid *ihid = i2c_get_clientdata(client);
 	struct hid_device *hid = ihid->hid;
 
-	enable_irq(client->irq);
+	enable_irq(ihid->irq);
 	ret = i2c_hid_hwreset(client);
 	if (ret)
 		return ret;
 
 	if (device_may_wakeup(&client->dev))
-		disable_irq_wake(client->irq);
+		disable_irq_wake(ihid->irq);
 
 	if (hid->driver && hid->driver->reset_resume) {
 		ret = hid->driver->reset_resume(hid);
@@ -1098,17 +1130,19 @@ static int i2c_hid_resume(struct device *dev)
 static int i2c_hid_runtime_suspend(struct device *dev)
 {
 	struct i2c_client *client = to_i2c_client(dev);
+	struct i2c_hid *ihid = i2c_get_clientdata(client);
 
 	i2c_hid_set_power(client, I2C_HID_PWR_SLEEP);
-	disable_irq(client->irq);
+	disable_irq(ihid->irq);
 	return 0;
 }
 
 static int i2c_hid_runtime_resume(struct device *dev)
 {
 	struct i2c_client *client = to_i2c_client(dev);
+	struct i2c_hid *ihid = i2c_get_clientdata(client);
 
-	enable_irq(client->irq);
+	enable_irq(ihid->irq);
 	i2c_hid_set_power(client, I2C_HID_PWR_ON);
 	return 0;
 }
-- 
2.1.4

Re: About Goodix-TS on Bay Trail, and ACPI and interrupts

[Benjamin Tissoires]

On Jan 20 2015 or thereabouts, Mika Westerberg wrote:
> On Mon, Jan 19, 2015 at 10:37:58AM -0500, Benjamin Tissoires wrote:
> > Hi Antonio,
> > 
> > [adding Mika in CC, he implemented most of the ACPI and GPIO for
> > i2c-hid]
> > 
> > On Jan 17 2015 or thereabouts, Antonio Ospite wrote:
> > > Hi,
> > > 
> > > I am trying to make the Goodix driver (drivers/input/touchscreen/goodix.c)
> > > working with a Teclast X98 Air 3G, a tablet based on Intel Bay Trail,
> > > but I am new to ACPI and I could use some help.
> > > 
> > > I am working with a 3.19-rc4 kernel compiled for x86_64.
> > > 
> > > This is the DSDT section in the UEFI firmware:
> > > 
> > >             Device (TCS0)
> > >             {
> > >                 Name (_ADR, Zero)  // _ADR: Address
> > >                 Name (_HID, "GODX0911")  // _HID: Hardware ID
> > >                 Name (_CID, "PNP0C50" /* HID Protocol Device (I2C bus) */)  // _CID: Compatible ID
> > 
> > urgh, this is bad. It declares itself as i2c-hid, but it is not :(
> > Anyway, according to your logs, i2c-hid probe() just fails, so it's not
> > a big problem.
> 
> 
> Actually, I think this device should use i2c-hid. All the ACPI plumbing
> is there including _DSM.
> 
> What makes you think it should use the goodix driver?

The fact that the 3.19-rc4 log says:
[    2.424370] i2c_hid i2c-GODX0911:01: unexpected HID descriptor bcdVersion (0x0000)

Which is rather troublesome and shows that we might have received a 0
answer when requesting the HID descriptor. Then Antonio tried to poke
the device with the goodix driver and got a better answer from the
version point of view.

Cheers,
Benjamin

Re: About Goodix-TS on Bay Trail, and ACPI and interrupts

[Mika Westerberg]

On Tue, Jan 20, 2015 at 11:31:52AM -0500, Benjamin Tissoires wrote:
> > What makes you think it should use the goodix driver?
> 
> The fact that the 3.19-rc4 log says:
> [    2.424370] i2c_hid i2c-GODX0911:01: unexpected HID descriptor bcdVersion (0x0000)
> 
> Which is rather troublesome and shows that we might have received a 0
> answer when requesting the HID descriptor. Then Antonio tried to poke
> the device with the goodix driver and got a better answer from the
> version point of view.

Right, I missed that from the logs.

Re: About Goodix-TS on Bay Trail, and ACPI and interrupts

[Antonio Ospite]

On Tue, 20 Jan 2015 12:05:48 +0200
Mika Westerberg <mika.westerberg@intel.com> wrote:

> On Mon, Jan 19, 2015 at 10:37:58AM -0500, Benjamin Tissoires wrote:
> > Hi Antonio,
> > 
> > [adding Mika in CC, he implemented most of the ACPI and GPIO for
> > i2c-hid]
> > 
> > On Jan 17 2015 or thereabouts, Antonio Ospite wrote:
> > > Hi,
> > > 
> > > I am trying to make the Goodix driver (drivers/input/touchscreen/goodix.c)
> > > working with a Teclast X98 Air 3G, a tablet based on Intel Bay Trail,
> > > but I am new to ACPI and I could use some help.
> > > 
> > > I am working with a 3.19-rc4 kernel compiled for x86_64.
> > > 
> > > This is the DSDT section in the UEFI firmware:
> > > 
> > >             Device (TCS0)
> > >             {
> > >                 Name (_ADR, Zero)  // _ADR: Address
> > >                 Name (_HID, "GODX0911")  // _HID: Hardware ID
> > >                 Name (_CID, "PNP0C50" /* HID Protocol Device (I2C bus) */)  // _CID: Compatible ID
> > 
[...]
> > >                     Name (ABUF, ResourceTemplate ()
> > >                     {
> > >                         I2cSerialBus (0x0014, ControllerInitiated, 0x0019F0A0,
> > >                             AddressingMode7Bit, "\\_SB.I2C4",
> > >                             0x00, ResourceConsumer, ,
> > >                             )
> > >                         GpioIo (Exclusive, PullDefault, 0x0000, 0x0000, IoRestrictionInputOnly,
> > >                             "\\_SB.GPO2", 0x00, ResourceConsumer, ,
> > >                             )
> > >                             {   // Pin list
> > >                                 0x0044
> > >                             }
> > >                         GpioIo (Exclusive, PullDefault, 0x0000, 0x0000, IoRestrictionOutputOnly,
> > >                             "\\_SB.GPO1", 0x00, ResourceConsumer, ,
> > >                             )
> > >                             {   // Pin list
> > >                                 0x001A
> > >                             }
> > >                     })
> > 
> > It looks like the GPIOs are correctly declared. The ACPI code should set
> > the client->irq auto-magically. It's not the case, so I guess Mika
> > should be able to tell us more on that.
> 
> The current i2c-hid.c does not cope with GPIO interrupts. I've attached
> an experimental patch that should convert the driver to use them.
> 
> Antonio, can you try that out and check if i2c-hid driver gets you
> working touch screen? Also please add "i2c_hid.debug=1" to the kernel
> command line so we can see if it returns proper HID descriptor.

Hi Mika, I tested the patch but I still can't get the IRQ.

The new code in i2c-hid does more or less what I was trying to do in
the goodix driver as proof of concept:
http://ao2.it/tmp/Goodix-TS_Teclast-X98-Air-3G/0001-XXX-goodix-add-support-for-GODX0911.patch

The gpio chip correspondent to the pin can be retrieved, but the gpio
descriptor can't be obtained and so the conversion from gpio to irq
can't happen, here is the full dmesg:
http://ao2.it/tmp/Goodix-TS_Teclast-X98-Air-3G/dmesg_Teclast_X98_Air_3G_mainline_i2c-hid_patch.log

I am pasting here the interesting parts, with some more printouts I
added to understand what was going on:

[    9.056071] i2c_hid i2c-GODX0911:01: GPIO lookup for consumer irq
[    9.056080] i2c_hid i2c-GODX0911:01: using ACPI for GPIO lookup
[    9.056086] acpi GODX0911:01: GPIO: looking up irq-gpios
[    9.056093] acpi GODX0911:01: GPIO: _DSD returned GODX0911:01 3 0 0 1
[    9.056160] acpi_find_gpio: ares->type: 19
[    9.056164] acpi_find_gpio: ares->type: 17
[    9.056167] acpi_find_gpio: pin_index: 0, agpio->pin_table_length: 1
[    9.056170] acpi_get_gpiod: path: \_SB.GPO2 pin: 68
[    9.056176] acpi_get_gpiod: handle found
[    9.056180] acpi_get_gpiod: gpiochip found
[    9.056183] acpi_get_gpiod: pin: 68
[    9.056186] acpi_get_gpiod: offset: 68
[    9.056189] gpiochip_get_desc: hwnum: 68 chip->ngpio: 44
[    9.056192] acpi_find_gpio: desc_error: -22
[    9.056195] acpi_find_gpio: ares->type: 17
[    9.056198] acpi_find_gpio: ares->type: 7
[    9.056204] acpi GODX0911:01: GPIO: looking up irq-gpio
[    9.056209] acpi GODX0911:01: GPIO: looking up 0 in _CRS
[    9.056272] acpi_find_gpio: ares->type: 19
[    9.056276] acpi_find_gpio: ares->type: 17
[    9.056280] acpi_find_gpio: pin_index: 0, agpio->pin_table_length: 1
[    9.056283] acpi_get_gpiod: path: \_SB.GPO2 pin: 68
[    9.056288] acpi_get_gpiod: handle found
[    9.056291] acpi_get_gpiod: gpiochip found
[    9.056294] acpi_get_gpiod: pin: 68
[    9.056297] acpi_get_gpiod: offset: 68
[    9.056300] gpiochip_get_desc: hwnum: 68 chip->ngpio: 44
[    9.056303] acpi_find_gpio: desc_error: -22
[    9.056306] acpi_find_gpio: ares->type: 17
[    9.056308] acpi_find_gpio: ares->type: 7
[    9.056314] i2c_hid i2c-GODX0911:01: lookup for GPIO irq failed
[    9.056319] i2c_hid i2c-GODX0911:01: Failed to get GPIO interrupt
[    9.073895] i2c_hid: probe of i2c-GODX0911:01 failed with error -22

As I was trying to say in the original mail, the DSDT declares a gpio
pin number (68) grater than the number of gpios for GPO2 (which on Bay
Trail is 44, isn't it?).

Is the DSDT just wrong? Or could there be anything missing in the
pinctrl-baytrail driver?

Thanks,
   Antonio

-- 
Antonio Ospite
http://ao2.it

A: Because it messes up the order in which people normally read text.
   See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?

Re: About Goodix-TS on Bay Trail, and ACPI and interrupts

[Mika Westerberg]

On Tue, Jan 20, 2015 at 05:56:45PM +0100, Antonio Ospite wrote:
> Hi Mika, I tested the patch but I still can't get the IRQ.
> 
> The new code in i2c-hid does more or less what I was trying to do in
> the goodix driver as proof of concept:
> http://ao2.it/tmp/Goodix-TS_Teclast-X98-Air-3G/0001-XXX-goodix-add-support-for-GODX0911.patch
> 
> The gpio chip correspondent to the pin can be retrieved, but the gpio
> descriptor can't be obtained and so the conversion from gpio to irq
> can't happen, here is the full dmesg:
> http://ao2.it/tmp/Goodix-TS_Teclast-X98-Air-3G/dmesg_Teclast_X98_Air_3G_mainline_i2c-hid_patch.log
> 
> I am pasting here the interesting parts, with some more printouts I
> added to understand what was going on:
> 
> [    9.056071] i2c_hid i2c-GODX0911:01: GPIO lookup for consumer irq
> [    9.056080] i2c_hid i2c-GODX0911:01: using ACPI for GPIO lookup
> [    9.056086] acpi GODX0911:01: GPIO: looking up irq-gpios
> [    9.056093] acpi GODX0911:01: GPIO: _DSD returned GODX0911:01 3 0 0 1
> [    9.056160] acpi_find_gpio: ares->type: 19
> [    9.056164] acpi_find_gpio: ares->type: 17
> [    9.056167] acpi_find_gpio: pin_index: 0, agpio->pin_table_length: 1
> [    9.056170] acpi_get_gpiod: path: \_SB.GPO2 pin: 68
> [    9.056176] acpi_get_gpiod: handle found
> [    9.056180] acpi_get_gpiod: gpiochip found
> [    9.056183] acpi_get_gpiod: pin: 68
> [    9.056186] acpi_get_gpiod: offset: 68
> [    9.056189] gpiochip_get_desc: hwnum: 68 chip->ngpio: 44
> [    9.056192] acpi_find_gpio: desc_error: -22
> [    9.056195] acpi_find_gpio: ares->type: 17
> [    9.056198] acpi_find_gpio: ares->type: 7
> [    9.056204] acpi GODX0911:01: GPIO: looking up irq-gpio
> [    9.056209] acpi GODX0911:01: GPIO: looking up 0 in _CRS
> [    9.056272] acpi_find_gpio: ares->type: 19
> [    9.056276] acpi_find_gpio: ares->type: 17
> [    9.056280] acpi_find_gpio: pin_index: 0, agpio->pin_table_length: 1
> [    9.056283] acpi_get_gpiod: path: \_SB.GPO2 pin: 68
> [    9.056288] acpi_get_gpiod: handle found
> [    9.056291] acpi_get_gpiod: gpiochip found
> [    9.056294] acpi_get_gpiod: pin: 68
> [    9.056297] acpi_get_gpiod: offset: 68
> [    9.056300] gpiochip_get_desc: hwnum: 68 chip->ngpio: 44
> [    9.056303] acpi_find_gpio: desc_error: -22
> [    9.056306] acpi_find_gpio: ares->type: 17
> [    9.056308] acpi_find_gpio: ares->type: 7
> [    9.056314] i2c_hid i2c-GODX0911:01: lookup for GPIO irq failed
> [    9.056319] i2c_hid i2c-GODX0911:01: Failed to get GPIO interrupt
> [    9.073895] i2c_hid: probe of i2c-GODX0911:01 failed with error -22
> 
> As I was trying to say in the original mail, the DSDT declares a gpio
> pin number (68) grater than the number of gpios for GPO2 (which on Bay
> Trail is 44, isn't it?).

That's right.

> Is the DSDT just wrong? Or could there be anything missing in the
> pinctrl-baytrail driver?

I think the DSDT is wrong here.

I found out from my mail archives that similar Goodix panel was used on
some internal reference design boards (it also used Goodix driver, btw).
Those also had the 0x44 GPIO there but that was wrong and it looked like
the correct GPIO number is 3 (in GPO2).

Not sure if that's the case here but maybe worth trying.

Re: About Goodix-TS on Bay Trail, and ACPI and interrupts

[Antonio Ospite]

On Wed, 21 Jan 2015 12:16:54 +0200
Mika Westerberg <mika.westerberg@intel.com> wrote:

> On Tue, Jan 20, 2015 at 05:56:45PM +0100, Antonio Ospite wrote:
> > Hi Mika, I tested the patch but I still can't get the IRQ.
> > 
> > The new code in i2c-hid does more or less what I was trying to do in
> > the goodix driver as proof of concept:
> > http://ao2.it/tmp/Goodix-TS_Teclast-X98-Air-3G/0001-XXX-goodix-add-support-for-GODX0911.patch
> > 
> > The gpio chip correspondent to the pin can be retrieved, but the gpio
> > descriptor can't be obtained and so the conversion from gpio to irq
> > can't happen, here is the full dmesg:
> > http://ao2.it/tmp/Goodix-TS_Teclast-X98-Air-3G/dmesg_Teclast_X98_Air_3G_mainline_i2c-hid_patch.log
> > 
> > I am pasting here the interesting parts, with some more printouts I
> > added to understand what was going on:
> > 
> > [    9.056071] i2c_hid i2c-GODX0911:01: GPIO lookup for consumer irq
> > [    9.056080] i2c_hid i2c-GODX0911:01: using ACPI for GPIO lookup
> > [    9.056086] acpi GODX0911:01: GPIO: looking up irq-gpios
> > [    9.056093] acpi GODX0911:01: GPIO: _DSD returned GODX0911:01 3 0 0 1
> > [    9.056160] acpi_find_gpio: ares->type: 19
> > [    9.056164] acpi_find_gpio: ares->type: 17
> > [    9.056167] acpi_find_gpio: pin_index: 0, agpio->pin_table_length: 1
> > [    9.056170] acpi_get_gpiod: path: \_SB.GPO2 pin: 68
> > [    9.056176] acpi_get_gpiod: handle found
> > [    9.056180] acpi_get_gpiod: gpiochip found
> > [    9.056183] acpi_get_gpiod: pin: 68
> > [    9.056186] acpi_get_gpiod: offset: 68
> > [    9.056189] gpiochip_get_desc: hwnum: 68 chip->ngpio: 44
> > [    9.056192] acpi_find_gpio: desc_error: -22
> > [    9.056195] acpi_find_gpio: ares->type: 17
> > [    9.056198] acpi_find_gpio: ares->type: 7
> > [    9.056204] acpi GODX0911:01: GPIO: looking up irq-gpio
> > [    9.056209] acpi GODX0911:01: GPIO: looking up 0 in _CRS
> > [    9.056272] acpi_find_gpio: ares->type: 19
> > [    9.056276] acpi_find_gpio: ares->type: 17
> > [    9.056280] acpi_find_gpio: pin_index: 0, agpio->pin_table_length: 1
> > [    9.056283] acpi_get_gpiod: path: \_SB.GPO2 pin: 68
> > [    9.056288] acpi_get_gpiod: handle found
> > [    9.056291] acpi_get_gpiod: gpiochip found
> > [    9.056294] acpi_get_gpiod: pin: 68
> > [    9.056297] acpi_get_gpiod: offset: 68
> > [    9.056300] gpiochip_get_desc: hwnum: 68 chip->ngpio: 44
> > [    9.056303] acpi_find_gpio: desc_error: -22
> > [    9.056306] acpi_find_gpio: ares->type: 17
> > [    9.056308] acpi_find_gpio: ares->type: 7
> > [    9.056314] i2c_hid i2c-GODX0911:01: lookup for GPIO irq failed
> > [    9.056319] i2c_hid i2c-GODX0911:01: Failed to get GPIO interrupt
> > [    9.073895] i2c_hid: probe of i2c-GODX0911:01 failed with error -22
> > 
> > As I was trying to say in the original mail, the DSDT declares a gpio
> > pin number (68) grater than the number of gpios for GPO2 (which on Bay
> > Trail is 44, isn't it?).
> 
> That's right.
> 
> > Is the DSDT just wrong? Or could there be anything missing in the
> > pinctrl-baytrail driver?
> 
> I think the DSDT is wrong here.
> 
> I found out from my mail archives that similar Goodix panel was used on
> some internal reference design boards (it also used Goodix driver, btw).
> Those also had the 0x44 GPIO there but that was wrong and it looked like
> the correct GPIO number is 3 (in GPO2).

First of all, thanks for looking.

The goodix driver under Android also says it uses gpio 133 which seems
consistent with this, as GPO2 base is 130 in the gpio-valleyview2
driver used in Android.

> Not sure if that's the case here but maybe worth trying.

I tried overriding the DSDT but I am stuck with this issue:
http://article.gmane.org/gmane.linux.acpi.devel/73176

I'll try hard-coding the gpio number in the code as quick test.

Ciao,
   Antonio

-- 
Antonio Ospite
http://ao2.it

A: Because it messes up the order in which people normally read text.
   See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?

Re: About Goodix-TS on Bay Trail, and ACPI and interrupts

[Antonio Ospite]

On Tue, 27 Jan 2015 15:45:59 +0100
Antonio Ospite <ao2@ao2.it> wrote:

> On Wed, 21 Jan 2015 12:16:54 +0200
> Mika Westerberg <mika.westerberg@intel.com> wrote:
> 
> > On Tue, Jan 20, 2015 at 05:56:45PM +0100, Antonio Ospite wrote:
[...]
> > > [    9.056300] gpiochip_get_desc: hwnum: 68 chip->ngpio: 44
> > > [    9.056303] acpi_find_gpio: desc_error: -22
> > > [    9.056306] acpi_find_gpio: ares->type: 17
> > > [    9.056308] acpi_find_gpio: ares->type: 7
> > > [    9.056314] i2c_hid i2c-GODX0911:01: lookup for GPIO irq failed
> > > [    9.056319] i2c_hid i2c-GODX0911:01: Failed to get GPIO interrupt
> > > [    9.073895] i2c_hid: probe of i2c-GODX0911:01 failed with error -22
> > > 
> > > As I was trying to say in the original mail, the DSDT declares a gpio
> > > pin number (68) grater than the number of gpios for GPO2 (which on Bay
> > > Trail is 44, isn't it?).
> > 
> > That's right.
> > 
> > > Is the DSDT just wrong? Or could there be anything missing in the
> > > pinctrl-baytrail driver?
> > 
> > I think the DSDT is wrong here.
> > 
> > I found out from my mail archives that similar Goodix panel was used on
> > some internal reference design boards (it also used Goodix driver, btw).
> > Those also had the 0x44 GPIO there but that was wrong and it looked like
> > the correct GPIO number is 3 (in GPO2).
> 

And indeed it was the DSDT which had wrong both the interrupt pin _and_
the reset pin (GPO1.TCD3)...

Now I am sure that the android kernel doesn't use the platform data from
ACPI but provides it to drivers with some machine specific code, which I
don't have access to.

I was able to make the touchscreen work by using these resources:
	Reset line (GPO1.TCD3) is: pin 9 on GPO1
	Interrupt line is:         pin 3 on GPO2

The reset sequence is performed when calling the _PS0 method, and with
the correct values the goodix driver can even retrieve the stored
configuration, no need to use the hardcoded default one.

Last doubt, now that I am fixing the DSDT I am going to add an
Interrupt resource, but what is the correct way to specify its value?
Pin 3 of GPO2 is mapped to IRQ 220 by the mainline kernel, so I added
this:

  Interrupt (ResourceConsumer, Edge, ActiveHigh, Exclusive, ,, )
  {
    0x000000DC,
  }

And it works, but doesn't this rely on the way linux maps interrupt
numbers? Is this going to be OS agnostic? My doubt arises from the
fact that on the Android kernel the same pin is mapped to IRQ
389.

JFTR since I had no access to the kernel source (the device
manufacturer does not reply...), I had to derive the correct gpios from
the running Android kernel.

The interrupt one was easy, to verify that it was pin 3 of GPO2 I looked
in /proc/interrupts, done some subtractions and figured it out.

The reset line was trickier for me, first I thought that I could rmmod
the android goodix driver and look at some status changes, but the
driver was not rmmod-safe, the kernel hung... then I thought that the
touchscreen may be turned off when the screen is turned off and that did
it; in the end I managed to spot it by comparing /sys/kernel/debug/gpio
with the screen on and off; inteltool from coreboot would have worked
just as well.

Thanks,
   Antonio

-- 
Antonio Ospite
http://ao2.it

A: Because it messes up the order in which people normally read text.
   See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?

Re: About Goodix-TS on Bay Trail, and ACPI and interrupts

[Mika Westerberg]

On Fri, Feb 06, 2015 at 05:00:13PM +0100, Antonio Ospite wrote:
> Last doubt, now that I am fixing the DSDT I am going to add an
> Interrupt resource, but what is the correct way to specify its value?
> Pin 3 of GPO2 is mapped to IRQ 220 by the mainline kernel, so I added
> this:
> 
>   Interrupt (ResourceConsumer, Edge, ActiveHigh, Exclusive, ,, )
>   {
>     0x000000DC,
>   }
> 
> And it works, but doesn't this rely on the way linux maps interrupt
> numbers? Is this going to be OS agnostic? My doubt arises from the
> fact that on the Android kernel the same pin is mapped to IRQ
> 389.

You should specify GpioInt() instead and turn it to interrupt using
gpiod_to_irq().