Device tree binding: can not make work my touch device

[aveselinov]

I have a Goodix touch device that uses i2c interface. This is the doc on how to bind it.

And this my node:

    i2cmux {
        compatible = "i2c-mux-gpio";
        #address-cells = <1>;
        #size-cells = <0>;
        mux-gpios = <&gpio1 2 0>;
        i2c-parent = <&i2c1>;        i2c@0 {
            reg = <0>;
            #address-cells = <1>;
            #size-cells = <0>;

            /*my node*/

            gt9271@XX {
                    compatible = "goodix,gt9271";
#if GOODIX_5D
                   reg = <0x5d>;
#else
                   reg = <0x14>;
#endif
                   interrupt-parent = <&gpio1>;
                   interrupts = <4 0>;
                   irq-gpios = <&gpio1 4 0>;
                   resets = <&lcd_reset>;
            };

The driver loads correctly, the reset sequence is OK too, but there are no interrupts generation when I touch the screen. So I suspect if my device tree binding could be wrong. How should I set the 0 value of interrupts and irq-gpios properties? The INT pin is connected to GPIO4, which is configured as input and is on HIGH level by default.  I've tried different combinations (0 0, 0 1, 1 0, 1 1) but all remains the same.

[igorpadykov]

Hi Anatoli

one can check various touch configurations on

http://boundarydevices.com/configuring-i-mx6-machines-different-screens-nitrogen6x-sabre-lite/

Also description "GPIO4" sounds ambiguous, one can check what pad really connected interrupt,

as code uses "gpio1 4".

Best regards
igor

[vinodmaverickr0]

It seems that you are missing a lot of things in your device tree. Are you parsing "irq-gpios"  value in your device driver. Till the point it not parsed and handled in your driver you can not see the interrupt. Apart from that there are a lot of parameters (like ADC conversion value, sample avrg controll and  touch detect intr delay) are required for the interrupt generation. So better have a look in your driver.

Linux/Documentation/devicetree/bindings/input/touchscreen/goodix.txt - Linux Cross Reference - Free ... 

Feel free for any help.

[aveselinov]

I didn't post the entire device tree, only the relevant part I though.

BTw, I've "lied" about the initialization sequence, it isn't OK, doesn't follow the sequence described on the datasheet. So I will try to fix that first.

[vinodmaverickr0]

Hi Anatoli,

I understood. I seems that you may have to modify the driver source code as well for these parameters ( which you have added in the dts file). Pls ask if you need any help for the same.

Best Regards,

Vinod

[aveselinov]

What modifications do you mean? About adding reset during initialization phase? My vendor is helping me too and we added that.

I'm attaching you the driver + dtsi files with the corresponding patches applied. Also the initialization phase described on datasheet. The RESET pin does the falling edge for 10ms, but INT pin remains always HIGH.

Appreciate your help, thanks.

PS: The linux branch that I use: linux-2.6-imx.git - Freescale i.MX Linux Tree 

[vinodmaverickr0]

Hi Anatoli,

I am not seeing patch file of driver and dtsi file.

May be you forget to attach the applied patch. Anyways I went through linux branch on which you are working:

It is your driver.

drivers/input/touchscreen/goodix.c

But I can not see any parsing of the device tree variable in this code. So you may need to apply some patch on this driver. You can share the patch (if it is not confidential) so that we can have a look and proceed further.

Thanks,

Vinod

[aveselinov]

Uh strange? Here is the link of the attached file https://community.nxp.com/servlet/JiveServlet/download/871282-1-399211/driver_and_dtsi.tar.gz 

Would you mind to explain what u mean with parsing the device tree? How do u know that is not parsing it? I only know this about device trees Device Tree Usage - eLinux.org  and nothing about modules kernel >.<. I just can guess that the "main" function is the one called _probe.

btw, that linux repository is the one that my vendors linux is based on. So there are missing patches and .dtsi files. Here is their (experimental) yocto project: GitHub - compulab-yokneam/meta-compulab 

And another thing. I also tested with the master version of the driver. Same result.

[vinodmaverickr0]

Hi Anatoli,

Sorry to say; but still you are missing a lot of thing. In your patch file your calling device_reset(&client->dev), but I am not seeing any function definitions.

I can see from the snapshot that firstly you will get the interrupt before device device go out of reset. So your code sequence be like that only.

Parsing the device tree means if you are making any entry in the device tree (like in your case "resets = <&lcd_reset>;"); so have to parse the same thing in your driver as well. Otherwise your device tree changes will not be reflect in driver.

I can see in the link  linux/goodix.c at master · torvalds/linux · GitHub 

That they are parsing the device tree changes (As I have mentioned in the last mail ).

You can trace the function calling starting from probe function

#define GOODIX_GPIO_INT_NAME "irq"    //Matching to string in device tree

#define GOODIX_GPIO_RST_NAME "reset"   //Matching to string in device tree

error = goodix_get_gpio_config(ts);

static int goodix_get_gpio_config(struct goodix_ts_data *ts){

/* Get the interrupt GPIO pin number */

gpiod = devm_gpiod_get_optional(dev, GOODIX_GPIO_RST_NAME, GPIOD_IN);

/* Get the reset line GPIO pin number */

gpiod = devm_gpiod_get_optional(dev, GOODIX_GPIO_RST_NAME, GPIOD_IN);  

}

Please add the missing code from this link. 

Regards,

Vinod

[aveselinov]

I think I understand it now. As it's said on gpio.txt, the strings are specified by defining the property like this <name>-gpios on the device tree. It seems that the "older" version driver that I'm using it isn't specifying a name for INT and RESET pins. 

So, if I want to use the master version of the driver, as the driver is looking for "irq" and "reset" then on my device tree I should have: irq-gpios and reset-gpios. I realiaze that goodix.txt was telling me that already.

So... This is what I have now:

            gt9271@XX {
                 compatible = "goodix,gt9271";
#if GOODIX_5D
                reg = <0x5d>;
#else
                reg = <0x14>;
#endif
                interrupt-parent = <&gpio1>;
                interrupts = <4 0>;
                irq-gpios = <&gpio1 4 GPIO_ACTIVE_HIGH>;
                reset-gpios = <&pca9555 11 GPIO_ACTIVE_LOW>;
               };

But this is what the driver is telling me:

root@cm-fx6:~# dmesg | grep Goodix
[    4.781898] Goodix-TS 3-0014: I2C Address: 0x14
[    4.781968] Goodix-TS 3-0014: Failed to get reset GPIO: -16
[    4.781986] Goodix-TS: probe of 3-0014 failed with error -16

Error 16 means the resource is bussy.

I think there is a conflict with <&pca9555 11> gpio. Because that same gpio is used on another node too:

mipi_dsi_reset: mipi-dsi-reset {
        compatible = "gpio-reset";
        reset-gpios = <&pca9555 11 GPIO_ACTIVE_LOW>;
        reset-delay-us = <100>;
        gpio-can-sleep;
        #reset-cells = <0>;
    };

If I try to comment that node then the driver fails at I2C test. Do I need mipi-dsi-reset?

[    4.640186] Goodix-TS 3-0014: I2C Address: 0x14
[    4.743896] Goodix-TS 3-0014: i2c test failed attempt 1: -5
[    4.773092] Goodix-TS 3-0014: i2c test failed attempt 2: -5
[    4.802926] Goodix-TS 3-0014: I2C communication failure: -5
[    4.807224] Goodix-TS: probe of 3-0014 failed with error -5

[vinodmaverickr0]

Hi Anatoli,

I think string b/w your device tree and driver is not matching. Can you pass the string "reset" instead of "reset-gpios" from your device tree.

Thanks,

Vinod

[aveselinov]

Then doesn't fail.

root@cm-fx6:~dmesg | grep Goodix0
[    4.695933] Goodix-TS 3-0014: I2C Address: 0x14
[    4.696788] Goodix-TS 3-0014: ID 9271, version: 1020
[    4.712163] Goodix-TS 3-0014: Invalid config, using defaults
[    4.723478] input: Goodix Capacitive TouchScreen as /devices/soc0/soc/2100000.aips-bus/21a0000.i2c/i2c-0/i2c-3/3-0014/input/input0

But INT and RESET pin remains HIGH always.

[vinodmaverickr0]

Good...

I think you can track it from the code. It is very simple. But your driver is parsing the device changes successfully. You can print the reset pin and IRQ pin number in your driver.

If you want any further help then you need to share the driver as well as device tree file; otherwise it will be difficult for me to advise for the same.

Thanks,

Vinod

[aveselinov]

About my last question, yes I need mipi-dsi-reset node, if I comment it out it causes the I2C failure.

The driver is this linux/goodix.c at master · torvalds/linux · GitHub (I'm referring it as master version)

And the .dtsi files that I'm modifying are on the tarbal that I attached before https://community.nxp.com/servlet/JiveServlet/download/2004-443218-871282-399211/driver_and_dtsi.tar... 

On goodix_get_gpio_config function I try to print the gpio numbers like this:

    dev_dbg(&ts->client->dev,"int: %d",ts->gpiod_int);
    dev_dbg(&ts->client->dev,"reset: %d",ts->gpiod_rst);

This is what I get:

[    4.612677] Goodix-TS 3-0014: int: -1474916816
[    4.619639] Goodix-TS 3-0014: reset: 0

Does it make any sense these numbers?

Regards

[vinodmaverickr0]

Hi Antoli,

Can you please clear some of this things. In earlier your reply you said:

I think there is a confusion. The driver that I use is an older an simpler version, is the one that I attached on the tar.gz file. I'm NOT using the linked one on github, which is the master version, because that doesn't seems to perform better (reset pin stays always high).

Now you are saying that you are using the driver from  linux/goodix.c at master · torvalds/linux · GitHub link. Because if you are using the driver from this link then device_reset() function which you have shared in the patch doesn't have any meaning. 

Anyways you need to print the gpio number ts->gpiod_rst = gpiod;. Other it will print some garbage value.

Thanks,

Vinod

[aveselinov]

Sorry about that. That was before, now I'm using the driver on github without patches.

 

Anyways you need to print the gpio number ts->gpiod_rst = gpiod;. Other it will print some garbage value.

That's exactly what I did. Notice:

      dev_dbg(&ts->client->dev,"int: %d", ts->gpiod_int);
      dev_dbg(&ts->client->dev,"reset: %d", ts->gpiod_rst);

And I'm getting this:

[    4.612677] Goodix-TS 3-0014: int: -1474916816
[    4.619639] Goodix-TS 3-0014: reset: 0

Regards

[vinodmaverickr0]

gpiod_rst is pointer of structure type. So you need to print it proper way. I also went through your device tree patch

+ gt9271@XX {
+ compatible = "goodix,gt9271";
+#if GOODIX_5D
+ reg = <0x5d>;
+#else
+ reg = <0x14>;
+#endif
+ interrupt-parent = <&gpio1>;
+ interrupts = <4 0>;
+ irq-gpios = <&gpio1 4 0>;
+ resets = <&lcd_reset>;   //Pls change it
+ };

So can you please pass the reset like

reset = <&gpio1 1 0>;   ///Please change the gpio1 to your GPIO pad as well as the offset value.

It is expected that if you device will not come out of the reset; INTR pin will remain high (pls check the seq1.png which you share earlier).

Thanks,

Vinod.

[aveselinov]

I did

reset = <&pca9555 11 GPIO_ACTIVE_LOW>;

(pca9555 is an auxiliary gpio controller)

No errors from driver but neither INT nor RESET pin behaves as described on datasheet (the .png that I attached you) during the initialization phase. What I'm doing is checking the pin signal with oscilloscope  while I reset my board.

If you have noticed, there is a goodix_reset function on the driver that programs the INT and RESET pin to perform the initialization phase as described on datasheet. And it's not doing it. That's the issue.

Regards

[vinodmaverickr0]

Hi,

I went through your driver code. So it is using new method of accessing the GPIO devm_gpiod_get_optional. So you need to pass the reset-gpios  from your device tree. 

If  goodix_reset function is not calling it means there is some issue on goodix_get_gpio_config(ts) function which will get the reset and irq pin from the device tree.

Thanks,

Vinod

[aveselinov]

Hi vinod. I managed to print the gpio number,using desc_to_gpio from consumer.h. INT pin is correct, it returns me 4. So the problem is the reset gpio which I can't use because has been already taken. If I comment out the other node from device tree (mipi-dsi-reset) that is using my gpio then my driver is able to get the gpio but it provokes i2c failure too:

root@cm-fx6:~# dmesg | grep Goodix
[    4.853658] Goodix-TS 3-0014: int pin: 4
[    4.861367] Goodix-TS 3-0014: rst pin: 507
[    4.973077] Goodix-TS 3-0014: i2c test failed attempt 1: -5
[    5.013111] Goodix-TS 3-0014: i2c test failed attempt 2: -5
[    5.042953] Goodix-TS 3-0014: I2C communication failure: -5
[    5.047248] Goodix-TS: probe of 3-0014 failed with error -5

The only solution would be to use another pin? Which would mean to modify the hardware circuit :smileysad:

[aveselinov]

vinod, already works! haha I did something that I though it wouldn't work but it does and now the touch responds to my touches. Will update the solution a bit later. Thanks for your help, I appreciate it.

update: this is the workaround:

I tried to do the reset with the same approach that I was trying with the older version of the driver, using device_reset from reset.h header.

I commented out the part where it's looking for reset pin on goodix_get_gpio_config and every ts->gpio_rst match too. So I left goodix_reset like this:

static int goodix_reset(struct goodix_ts_data *ts)
{
int error;

/* begin select I2C slave addr */
// error = gpiod_direction_output(ts->gpiod_rst, 0);
error = device_reset(&ts->client->dev);
if (error)
return error;

// msleep(20); /* T2: > 10ms */  //I already specified 10ms delay on the device tree

/* HIGH: 0x28/0x29, LOW: 0xBA/0xBB */
error = gpiod_direction_output(ts->gpiod_int, ts->client->addr == 0x14);
if (error)
return error;

usleep_range(100, 2000); /* T3: > 100us */

// error = gpiod_direction_output(ts->gpiod_rst, 1);
// if (error)
// return error;

usleep_range(6000, 10000); /* T4: > 5ms */

/* end select I2C slave addr */
// error = gpiod_direction_input(ts->gpiod_rst);
// if (error)
// return error;

error = goodix_int_sync(ts);
if (error)
return error;

return 0;
}