Using device-tree to configure GPIO - basic questions

[procount]

I am trying to write a device driver and specify the GPIO pins that it uses with a device-tree overlay, but I'm getting confused, so I hope someone can confirm or correct my understanding.

AIUI, the device-tree is just a logical description of the hardware that is fitted and how it should be configured.
Just adding nodes etc. to a devce-tree overlay will not actually do anything, since it is the responsibility of the various device drivers (matched through the "compatible" field) that are responsible for carrying out those configurations.
So if I create my own device driver, I am at liberty to specify my own configuration nodes, read the appropriate values from them, and then allocate and configure any GPIO pins as I require using appropriate api calls such as gpiod_get(). Is that a correct view?

But there are some overlays that configure GPIO pins, but don't have their own device driver, such as the vga666-overlay.dts.
So am I right in thinking that it is using the overlay technique to insert its configuration nodes into the nodes of existing device drivers and using those to configure the gpio pins for it instead?
In this case, it defines a vga666_pins nodes that contains the configuration of the GPIOs it requires, and inserts it into the nodes of the gpio driver for it to allocate them?
It also seems to require a pinctrl node to configure the pins, but again it uses another device driver to do this for it. In this case it seems to be the platfrom led driver as there is a note to that effect. But why must the "borrowed" led driver not already have a pinctrl in it - can there only be 1 pinctrl per device node? If there are 2 overlays that want to configure pinctrl, how can a clash or overuse of the LED driver node pinctrl be avoided?

So I assume a device driver writer has 2 choices when creating the dts-overlay file:
1. Overlay other platfrom device nodes with the required GPIO and pinctrl configurations and let them configure the GPIO pins on my behalf, or
2. Keep the configuration of such GPIOs in my own device node, but I must read and apply all the configuration of those GPIO pins within the driver's probe function.

And I'm guessing option 1. is quite prevalent because it avoids having to write additional code in my own driver?

[PhilE]

You seem to already have a pretty good understanding of Device Tree and how Linux uses it.

So if I create my own device driver, I am at liberty to specify my own configuration nodes, read the appropriate values from them, and then allocate and configure any GPIO pins as I require using appropriate api calls such as gpiod_get(). Is that a correct view?

That's probably correct, depending on your exact understanding of what gpiod_get() does (which is to glue "-gpios" and "-gpio" onto the name you supply and look for a property of that name in your DT node, then parse the DT gpio description to end up with a descriptor).

So am I right in thinking that it is using the overlay technique to insert its configuration nodes into the nodes of existing device drivers and using those to configure the gpio pins for it instead?

That's largely correct, but a) be careful with your use of "gpio" - at this level, GPIO is just two (in and out) of the possible pin functions that can be selected - and b) it isn't the "victim" device driver that ends up doing the work, as you'll see in a moment.

[ As far as Linux is concerned, GPIO is software-controlled input and output pins. There are devices which purely provide GPIO functionality, and other devices which require pin control to select between multiple functions where none of them are GPIO. ]

In this case, it defines a vga666_pins nodes that contains the configuration of the GPIOs it requires, and inserts it into the nodes of the gpio driver for it to allocate them?

It's more accurate to say that the overlay gets the DT framework to request those pins from the pinctrl driver, on behalf of the leds driver (which doesn't know it is asking for them).

But why must the "borrowed" led driver not already have a pinctrl in it - can there only be 1 pinctrl per device node?

Essentially, yes. pinctrl states are mutually exclusive (and all the devices I've come across only use one state - "default"), and an overlay completely overwrites a property - it can't append to it (except for the magic "bootargs" string handling in the firmware). You could create an overlay that effectively appends its own pinctrl node to another devices by incorporating the original into its own, but it would have to assume what the other device had set it to, and it would break if the overlays were applied in the reverse order.

If there are 2 overlays that want to configure pinctrl, how can a clash or overuse of the LED driver node pinctrl be avoided?

The problem only arises if the overlay isn't creating a device node of its own, which is rare. There is almost certainly a driver which can be instantiated multiple times that claims no resources of its own - perhaps another instance of the gpio-leds driver but with no LEDs configured? No - that would return -ENODEV, but you get the idea.

So I assume a device driver writer has 2 choices when creating the dts-overlay file:
1. Overlay other platfrom device nodes with the required GPIO and pinctrl configurations and let them configure the GPIO pins on my behalf, or
2. Keep the configuration of such GPIOs in my own device node, but I must read and apply all the configuration of those GPIO pins within the driver's probe function.

As I think you've probably worked out by now, there is option 3 - get the standard pinctrl driver to request the pin functions you need on your behalf, without writing any code, just leaving any GPIO requesting and usage for your driver to do.

[procount]

Thanks, Phil

You seem to already have a pretty good understanding of Device Tree and how Linux uses it.

At least you've confirmed my basic understanding is nearly there. Of course I can just copy/paste/adapt some other overlay, but I'm trying to understand what I'm doing so I do it right and some of it is still gobbledygook to me, but the fog is slowly lifting.

So if I create my own device driver, I am at liberty to specify my own configuration nodes, read the appropriate values from them, and then allocate and configure any GPIO pins as I require using appropriate api calls such as gpiod_get(). Is that a correct view?

That's probably correct, depending on your exact understanding of what gpiod_get() does (which is to glue "-gpios" and "-gpio" onto the name you supply and look for a property of that name in your DT node, then parse the DT gpio description to end up with a descriptor).

That's a better description than I've read previously! But yes, and then I have to configure the use of that gpio according to the properties that were supplied.

In this case, it defines a vga666_pins nodes that contains the configuration of the GPIOs it requires, and inserts it into the nodes of the gpio driver for it to allocate them?

It's more accurate to say that the overlay gets the DT framework to request those pins from the pinctrl driver, on behalf of the leds driver (which doesn't know it is asking for them).

Oh, ok.

If there are 2 overlays that want to configure pinctrl, how can a clash or overuse of the LED driver node pinctrl be avoided?

The problem only arises if the overlay isn't creating a device node of its own, which is rare. There is almost certainly a driver which can be instantiated multiple times that claims no resources of its own - perhaps another instance of the gpio-leds driver but with no LEDs configured? No - that would return -ENODEV, but you get the idea.

So in the case where I am creating my own driver, I have my own device node in which I can put all the pinctrl stuff without having to hijack another device such as the LED driver.

So I assume a device driver writer has 2 choices when creating the dts-overlay file:
1. Overlay other platfrom device nodes with the required GPIO and pinctrl configurations and let them configure the GPIO pins on my behalf, or
2. Keep the configuration of such GPIOs in my own device node, but I must read and apply all the configuration of those GPIO pins within the driver's probe function.

As I think you've probably worked out by now, there is option 3 - get the standard pinctrl driver to request the pin functions you need on your behalf, without writing any code, just leaving any GPIO requesting and usage for your driver to do.

So I still have to use API functions in my driver to request and configure my own GPIOs? I thought targetting the <gpios> with an overlay of "mydevice-pins" with their configuration would get the gpio driver to do this for me?
It's this last bit of allocation of responsibility between the DTS and the device driver that's confusing me a little.

But you've helped a lot. I'll write my DTS and I'll post it here if you wouldn't mind giving it the once over?
Initially I'm going to write the driver to poll a GPIO pin for activity, but then I want to change it to be interrupt driven. Finally I was hoping to include a DT parameter to determine whether it should be interrupt driven or polled, if that is indeed possible.

[PhilE]

So in the case where I am creating my own driver, I have my own device node in which I can put all the pinctrl stuff without having to hijack another device such as the LED driver.

Exactly.

So I still have to use API functions in my driver to request and configure my own GPIOs?

You make it sound like it's a lot work. At one end, the pinctrl node will have configured those pins to be GPIOs, and at the other end your application needs a way of referring to and distinguishing the GPIOs which have been assigned to it in order to set their states (unless your GPIO usage is strictly static, start-of-day initialisation, but that has to be a relatively rare use case). By my reckoning you would need at least one API call to say "give me a handle to a GPIO with the specified role/index", which is exactly what devm_gpiod_get() does. By supplying the appropriate flags it will be configured as an input or output (driving high or low).

[procount]

You make it sound like it's a lot work

No, it's not, but I don't want to duplicate stuff I don't need to.
I think you've clarified how I need to progress, though.
Thanks.

[procount]

Here is my first attempt at a combined DPI interface with i2c touchscreen interface:
I have added comments to explain my understanding of what each fragment is supposed to do, interspersed with some questions about why certain parts exist or if they could be done better.

To briefly explain: It should configure DPI mode for the display, plus a software bitbanged i2c interface on GPIO10/11 for the touchscreen, disable the standard i2c and spi interfaces because they are used by dpi, and defines GPIO 27 as an active-high input from the touchscreen (to indicate new data available) which will initially be polled but hopefully converted to an interrupt later on.

Comments are appreciated!

Code: Select all

/*
 * Device tree overlay for Cypress Cy8c20466 Touchscreen
 *
 * Compile:
 * dtc [email protected] -I dts -O dtb -o cypress.dtbo cypress.dts
 */

/dts-v1/;
/plugin/;

/{
    /* Identify the RPi models this is compatible with (is this sufficient?) */
    compatible = "brcm,bcm2835", "brcm,bcm2836", "brcm,bcm2708", "brcm,bcm2709";

    /* Disable SPI - because DPI uses it? */
    [email protected] {
        target = <&spi0>;
        __overlay__ {
            status = "disabled";
        };
    };

    /* There is no DPI driver module, but we need a platform device      */
    /* node (that doesn't already use pinctrl) to hang the pinctrl       */
    /* reference on - leds will do                                       */
    /* (Can I add these to my own pictrl for Cy8c20466 or cypress_pins?) */
    [email protected] {
        target = <&leds>;
        __overlay__ {
            pinctrl-names = "default";
            pinctrl-0 = <&dpi18_pins>;
        };
    };

    /* Define a group of pins to be configured by the GPIO driver as function 6 = ALT2 = DPI mode */
    [email protected] {
        target = <&gpio>;
        __overlay__ {
            dpi18_pins: dpi18_pins {
                brcm,pins = <0 1 2 3 4 5 6 7 8 9 12 13 14 15 16 17 20 21 22 23 24 25>;
                brcm,function = <6>;
                brcm,pull = <0>;
            };
        };
    };

    /* Create i2c-gpio driver instance for software i2c on gpio 10 & 11 as i2c-3. 4us delay for 100kHz */
    /* Why is the node called [email protected] not [email protected]? */
    [email protected] {
        target-path = "/";
        __overlay__ {
            i2c_gpio: [email protected] {
                compatible = "i2c-gpio";
                gpios = <&gpio 10 0 /* sda */
                         &gpio 11 0 /* scl */
                        >;
                i2c-gpio,delay-us = <4>;
                #address-cells = <1>;
                #size-cells = <0>;
            };
        };
    };

    /* Add the touchscreen controller as a device under i2c_gpio */
    [email protected] {
        target = <&i2c_gpio>;
        __overlay__ {
            /* needed to avoid dtc warning */
            #address-cells = <1>;
            #size-cells = <0>;
            Cy8c20466: [email protected] {
                compatible = "cypress,Cy8c20466";
                reg = <0x5c>;
                pinctrl-names = "default";
                pinctrl-0 = <&cypress_pins>;
                interrupt-parent = <&gpio>;
                interrupts = <27 2>;
                irq-gpios = <&gpio 27 0>; // specify  GPIO27 as the irq line from the TS controller
                touchscreen-size-x = <800>;
                touchscreen-size-y = <480>;
                touchscreen-x-mm = <68>;
                touchscreen-y-mm = <46>;
            };
        };
    };

    /* Disable i2c_arm - because it is used by dpi? */
    [email protected] {
        target = <&i2c_arm>;
        __overlay__ {
            status = "disabled";
        };
    };

    /* Why? */
    [email protected] {
        target-path = "/aliases";
        __overlay__ {
            i2c_gpio = "/[email protected]";
        };
    };

    /* Same as @6 - why? */
    [email protected] {
        target-path = "/__symbols__";
        __overlay__ {
            i2c_gpio = "/[email protected]";
        };
    };

    /* Define cypress_pins as child of gpio group */
    /* referenced by pinctrl to set up gpio 27 as input with pull-down */
	[email protected] {
		target = <&gpio>;
		__overlay__ {
			cypress_pins: cypress_pins {
				brcm,pins = <27>; // interrupt
				brcm,function = <0>; // in
				brcm,pull = <2>; // pull-down?
			};
		};
	};
};

[PhilE]

Code: Select all

    /* Disable SPI - because DPI uses it? */

SPI needs to be disabled because DPI in 18-bit mode wants to use the same pins. However, it should be disabled by default. If something does ever try to enable SPI it will cause contention in pinctrl, and one of them will lose.

Code: Select all

    /* There is no DPI driver module, but we need a platform device      */
    /* node (that doesn't already use pinctrl) to hang the pinctrl       */
    /* reference on - leds will do                                       */
    /* (Can I add these to my own pictrl for Cy8c20466 or cypress_pins?) */

Yes - just use:

Code: Select all

                pinctrl-0 = <&cypress_pins &dpi18_pins> ;

Code: Select all

 /* Why is the node called [email protected] not [email protected]? */

The top level doesn't use addressing, so this "@0" just serves to separate it from any other top-level i2c nodes. Linux device numbers don't have to match the addresses here - we use DT aliases to configure the mapping.

Code: Select all

            Cy8c20466: [email protected] {
                compatible = "cypress,Cy8c20466";

Please stick to lowercase names - everything else does.

Code: Select all

/* Disable i2c_arm - because it is used by dpi? */

Again, it's because we want the pins, but again it should be disabled by default.

Code: Select all

    /* Same as @6 - why? */

Symbols are ways of sharing labels between DT files, and like labels they only have meaning within DT. The compiler converts them to phandles - small integers attached to nodes. Aliases are usable by DT clients, either by explicitly looking within "/aliases" or by using an alias as the first element of a path (without the initial "/").

Tidy that up, placing labelled nodes before the references to them, then show me version 2.0.

[procount]

Hopefully v2 is better now.

Code: Select all

/*
 * Device tree overlay for Cypress Cy8c20466 Touchscreen
 * V2
 *
 * Compile:
 * dtc [email protected] -I dts -O dtb -o cypress.dtbo cypress.dts
 */

/dts-v1/;
/plugin/;

/{
    /* Identify the RPi models this is compatible with (is this sufficient?) */
    compatible = "brcm,bcm2835", "brcm,bcm2836", "brcm,bcm2708", "brcm,bcm2709";

    /* Define a group of pins to be configured by the GPIO driver as function 6 = ALT2 = DPI mode */
    [email protected] {
        target = <&gpio>;
        __overlay__ {
            dpi18_pins: dpi18_pins {
                brcm,pins = <0 1 2 3 4 5 6 7 8 9 12 13 14 15 16 17 20 21 22 23 24 25>;
                brcm,function = <6>;
                brcm,pull = <0>;
            };
        };
    };

    /* Define cypress_pins as child of gpio group */
    /* referenced by pinctrl to set up gpio 27 as input with pull-down */
	[email protected] {
		target = <&gpio>;
		__overlay__ {
			cypress_pins: cypress_pins {
				brcm,pins = <27>; // interrupt
				brcm,function = <0>; // in
				brcm,pull = <2>; // pull-down?
			};
		};
	};

    /* Create i2c-gpio driver instance for software i2c on gpio 10 & 11 as i2c-3. 4us delay for 100kHz */
    [email protected] {
        target-path = "/";
        __overlay__ {
            i2c_gpio: [email protected] {
                compatible = "i2c-gpio";
                gpios = <&gpio 10 0 /* sda */
                         &gpio 11 0 /* scl */
                        >;
                i2c-gpio,delay-us = <4>;
                #address-cells = <1>;
                #size-cells = <0>;
            };
        };
    };

    /* Add the touchscreen controller as a device under i2c_gpio */
    [email protected] {
        target = <&i2c_gpio>;
        __overlay__ {
            /* needed to avoid dtc warning */
            #address-cells = <1>;
            #size-cells = <0>;
            cy8c20466: [email protected] {
                compatible = "cypress,cy8c20466";
                reg = <0x5c>;
                pinctrl-names = "default";
                pinctrl-0 = <&cypress_pins &dpi18_pins>;
                interrupt-parent = <&gpio>;
                interrupts = <27 2>;
                irq-gpios = <&gpio 27 0>; // specify  GPIO27 as the irq line from the TS controller
                touchscreen-size-x = <800>;
                touchscreen-size-y = <480>;
                touchscreen-x-mm = <68>;
                touchscreen-y-mm = <46>;
            };
        };
    };

    [email protected] {
        target-path = "/aliases";
        __overlay__ {
            i2c_gpio = "/[email protected]";
        };
    };

    [email protected] {
        target-path = "/__symbols__";
        __overlay__ {
            i2c_gpio = "/[email protected]";
        };
    };

    /* Disable SPI - because DPI uses it. Not necessary, just in case */
    [email protected] {
        target = <&spi0>;
        __overlay__ {
            status = "disabled";
        };
    };

    /* Disable i2c_arm - because DPI uses it. Not necessary, just in case */
    [email protected] {
        target = <&i2c_arm>;
        __overlay__ {
            status = "disabled";
        };
    };
};

EDIT: Woops - forgot to update fragment numbers, but done now.

[PhilE]

That looks good. You can avoid one dtc warning by giving your [email protected] node a "reg = <0>;" property. It would also allow you to renumber it using an overlay parameter, should you ever need to avoid a clash.

Just for information, it is possible to combine fragments 0&1 and 2&3, but it is nice and clear as it is.

[procount]

Brill!
Thanks for your expert guidance. I'm still not sure I understand all of it, but probably enough to see me through.

[procount]

By combining, you mean like this?:

Code: Select all

    [email protected] {
        target = <&gpio>;
        __overlay__ {
            dpi18_pins: dpi18_pins {
                brcm,pins = <0 1 2 3 4 5 6 7 8 9 12 13 14 15 16 17 20 21 22 23 24 25>;
                brcm,function = <6>;
                brcm,pull = <0>;
            };
            /* Define cypress_pins as child of gpio group */
            cypress_pins: cypress_pins {
                brcm,pins = <27>; // interrupt
                brcm,function = <0>; // in
                brcm,pull = <2>; // pull-down?
            };
        };
    };

    /* Create i2c-gpio driver instance for software i2c on gpio 10 & 11 as i2c-3. 4us delay for 100kHz */
    [email protected] {
        target-path = "/";
        __overlay__ {
            i2c_gpio: [email protected] {
                compatible = "i2c-gpio";
                reg = <0>;
                gpios = <&gpio 10 0 /* sda */
                         &gpio 11 0 /* scl */
                        >;
                i2c-gpio,delay-us = <4>;
                #address-cells = <1>;
                #size-cells = <0>;
                cy8c20466: [email protected] {
                    compatible = "cypress,cy8c20466";
                    reg = <0x5c>;
                    pinctrl-names = "default";
                    pinctrl-0 = <&cypress_pins &dpi18_pins>;
                    interrupt-parent = <&gpio>;
                    interrupts = <27 2>;
                    irq-gpios = <&gpio 27 0>; // specify  GPIO27 as the irq line from the TS controller
                    touchscreen-size-x = <800>;
                    touchscreen-size-y = <480>;
                    touchscreen-x-mm = <68>;
                    touchscreen-y-mm = <46>;
                };
            };
        };
    };

[PhilE]

Yes, that's what I meant.

[procount]

I'm starting to work on the touchscreen driver that will be loaded by this overlay, now.
I've pushed this to github at https://github.com/procount/cypress where you can find the final DTS file and this driver file

The driver loads ok, detects the hardware is present, and even reads the touch co-ordinates over the i2c interface, but I don't think I have configured the GPIO pin that indicates new data is present correctly, as it always reads high. This means the driver is reading the hardware even when it doesn't need to, which will be a performance problem.

I've cut down the driver to the minimum needed to highlight the problem.
It is a port of a userland python driver, which I've also placed in my github for reference. This is about all the information I have on the device, but given the python driver works, it should be sufficient.

It should be using BCM 27 (which is on pin 13) to detect when new data is available. I've used `gpio readall` to prove the GPIO pin is in input mode and I can see it change state when I manually connect it high or low. But when the device is attached, my driver always reads it high.

I'm struggling to debug it fully because I'm building the driver as a builtin driver in buildroot (for PINN), but it doesn't have all the debug tools like `gpio` so I'm testing the overlay configuration in Raspbian.

I cant see anything specifically in the python driver that resets this INT pin (unless I missed it), so I assume just reading the registers is sufficient.

I appreciate that this may be a problem in understanding how the hardware itself works, but I wondered if anyone can see anything I've done wrong in the driver before I pursue other routes.

[PhilE]

There's an error in your overlay that I didn't spot earlier - the GPIO pull is in the wrong direction (2 means up, 1 is down and 0 is no pull), which probably explains why the interrupt pin is always high.

[procount]

Good spot, but I think I mislead you there. I started with the value of 1 for pull-down, but then tried a pull-up and no pull at all to see if it made any difference, which it didn't. i guess I forgot to reset it back again.

Here is a typical log when I went from 0 touch -> 1 touch -> 2 touch -> none again.

Code: Select all

Feb  2 23:29:57 recovery kern.debug kernel: [  696.603412] Cypress-TS 0-005c: process_events 27: 1
Feb  2 23:29:57 recovery kern.debug kernel: [  696.604496] Cypress-TS 0-005c: .
Feb  2 23:29:57 recovery kern.debug kernel: [  696.723412] Cypress-TS 0-005c: process_events 27: 1
Feb  2 23:29:57 recovery kern.debug kernel: [  696.724496] Cypress-TS 0-005c: .
Feb  2 23:29:57 recovery kern.debug kernel: [  696.843412] Cypress-TS 0-005c: process_events 27: 1
Feb  2 23:29:57 recovery kern.debug kernel: [  696.846030] Cypress-TS 0-005c: .
Feb  2 23:29:57 recovery kern.debug kernel: [  696.963412] Cypress-TS 0-005c: process_events 27: 1
Feb  2 23:29:57 recovery kern.debug kernel: [  696.965382] Cypress-TS 0-005c: .
Feb  2 23:29:57 recovery kern.debug kernel: [  696.965386] Cypress-TS 0-005c: x1,y1= 216, 332
Feb  2 23:29:57 recovery kern.debug kernel: [  697.083412] Cypress-TS 0-005c: process_events 27: 1
Feb  2 23:29:57 recovery kern.debug kernel: [  697.087684] Cypress-TS 0-005c: .
Feb  2 23:29:57 recovery kern.debug kernel: [  697.087688] Cypress-TS 0-005c: x1,y1= 223, 337
Feb  2 23:29:57 recovery kern.debug kernel: [  697.203412] Cypress-TS 0-005c: process_events 27: 1
Feb  2 23:29:57 recovery kern.debug kernel: [  697.204536] Cypress-TS 0-005c: .
Feb  2 23:29:57 recovery kern.debug kernel: [  697.204540] Cypress-TS 0-005c: x1,y1= 223, 337
Feb  2 23:29:57 recovery kern.debug kernel: [  697.323418] Cypress-TS 0-005c: process_events 27: 1
Feb  2 23:29:57 recovery kern.debug kernel: [  697.329598] Cypress-TS 0-005c: .
Feb  2 23:29:57 recovery kern.debug kernel: [  697.329602] Cypress-TS 0-005c: x1,y1= 229, 206
Feb  2 23:29:57 recovery kern.debug kernel: [  697.329606] Cypress-TS 0-005c: x2,y2= 468, 338
Feb  2 23:29:58 recovery kern.debug kernel: [  697.443417] Cypress-TS 0-005c: process_events 27: 1
Feb  2 23:29:58 recovery kern.debug kernel: [  697.444474] Cypress-TS 0-005c: .
Feb  2 23:29:58 recovery kern.debug kernel: [  697.444478] Cypress-TS 0-005c: x1,y1= 229, 206
Feb  2 23:29:58 recovery kern.debug kernel: [  697.444482] Cypress-TS 0-005c: x2,y2= 466, 337
Feb  2 23:29:58 recovery kern.debug kernel: [  697.563413] Cypress-TS 0-005c: process_events 27: 1
Feb  2 23:29:58 recovery kern.debug kernel: [  697.564506] Cypress-TS 0-005c: .
Feb  2 23:29:58 recovery kern.debug kernel: [  697.564509] Cypress-TS 0-005c: x1,y1= 228, 206
Feb  2 23:29:58 recovery kern.debug kernel: [  697.564513] Cypress-TS 0-005c: x2,y2= 466, 337
Feb  2 23:29:58 recovery kern.debug kernel: [  697.683412] Cypress-TS 0-005c: process_events 27: 1
Feb  2 23:29:58 recovery kern.debug kernel: [  697.684840] Cypress-TS 0-005c: .
Feb  2 23:29:58 recovery kern.debug kernel: [  697.684844] Cypress-TS 0-005c: x1,y1= 229, 207
Feb  2 23:29:58 recovery kern.debug kernel: [  697.684848] Cypress-TS 0-005c: x2,y2= 468, 339
Feb  2 23:29:58 recovery kern.debug kernel: [  697.803412] Cypress-TS 0-005c: process_events 27: 1
Feb  2 23:29:58 recovery kern.debug kernel: [  697.804449] Cypress-TS 0-005c: .
Feb  2 23:29:58 recovery kern.debug kernel: [  697.923412] Cypress-TS 0-005c: process_events 27: 1
Feb  2 23:29:58 recovery kern.debug kernel: [  697.924472] Cypress-TS 0-005c: .
Feb  2 23:29:58 recovery kern.debug kernel: [  698.043412] Cypress-TS 0-005c: process_events 27: 1
Feb  2 23:29:58 recovery kern.debug kernel: [  698.044571] Cypress-TS 0-005c: .
Feb  2 23:29:58 recovery kern.debug kernel: [  698.163412] Cypress-TS 0-005c: process_events 27: 1

As you can see the INT pin is always high, even with a pull-down.

[PhilE]

The documentation I could find on the touchscreen controller didn't mention an IRQ pin - it sounded like a very programmable device where the pin usage was firmware defined.

1. The C does seem to be equivalent to the Python, but you can verify that it is (at least at the level of I2C atraffic):

   Code: Select all

   $ sudo sh -c "echo 1 > /sys/kernel/debug/trace/events/i2c/enable"
   # [ run python test ]
   $ sudo cat /sys/kernel/debug/trace/trace > python_trace.txt
   $ sudo sh -c "echo > /sys/kernel/debug/trace/trace"
   # [ run C test ]
   $ sudo cat /sys/kernel/debug/trace/trace > c_trace.txt
   $ sudo sh -c "echo > /sys/kernel/debug/trace/trace"
   $ sudo sh -c "echo 0 > /sys/kernel/debug/trace/events/i2c/enable"
   

   Now you can compare the two traces. Although there will be timing differences, something like "meld" should make it clear if there are substantive differences.
   
   If you replace the "i2c" directory above with "gpio" you get a similar trace, only this time you will have to post-process to filter out the ACT LED activity, etc. and focus on GPIO 27.
2. Are you certain that the Python script sees IRQ as being low at any point?

[procount]

Yes, I believe it's just a microcontroller, so the IRQ pin is just a programmable output pin. Gadgetoid has only been able to give me limited information on the interface it provides.
No, I'm not certain the python program ever sees it low yet - It's on my list to check, (but I suspect it's ok from what I discovered below)

But first, I tried to debug the DTS in plain Raspbian (without the device driver)
My first step was to remove the python device driver. That's when I discovered that `gpio readall` indicated that GPIO 27 (BCM) was an OUTPUT!
It was the python driver that was configuring it as an input, not my DTS overlay, a fact I wasn't aware of in my buildroot env.

So I cut down my dts overlay to touch.dts as in the following fragment (using LEDS to set the pinctrl):

Code: Select all

/dts-v1/;
/plugin/;

/{
    compatible = "brcm,bcm2835", "brcm,bcm2836", "brcm,bcm2708", "brcm,bcm2709", "brcm,bcm2710", "brcm,bcm2837";

    [email protected] {
        target = <&gpio>;
        __overlay__ {
            cypress_pins: cypress_pins {
                brcm,pins = <27>; // interrupt
                brcm,function = <0>; // in
                brcm,pull = <1>; // pull-down?
            };
        };
    };

    [email protected] {
        target = <&leds>;
        __overlay__ {
            pinctrl-names = "default";
            pinctrl-0 = <&cypress_pins>;
        };
    };

};

Setting dtdebug=1 in config.txt results in the following log from `vcdbg log msg`

Code: Select all

002252.264: dtdebug: Opened overlay file 'overlays/touch.dtbo'
002252.796: brfs: File read: /mfs/sd/overlays/touch.dtbo
002267.234: Loaded overlay 'touch'
002267.537: dtdebug: Found fragment 0 (offset 100)
002270.575: dtdebug: merge_fragment(/soc/[email protected],/[email protected]/__overlay__)
002278.205: dtdebug: merge_fragment(/soc/[email protected]/cypress_pins,/[email protected]/__overlay__/cypress_pins)
002278.233: dtdebug:   +prop(brcm,pins)
002281.068: dtdebug:   +prop(brcm,function)
002283.931: dtdebug:   +prop(brcm,pull)
002286.811: dtdebug:   +prop(linux,phandle)
002290.225: dtdebug:   +prop(phandle)
002293.073: dtdebug: merge_fragment() end
002293.106: dtdebug: merge_fragment() end
002293.188: dtdebug: Found fragment 1 (offset 260)
002315.790: dtdebug: merge_fragment(/leds,/[email protected]/__overlay__)
002315.817: dtdebug:   +prop(pinctrl-names)
002316.908: dtdebug:   +prop(pinctrl-0)
002318.021: dtdebug: merge_fragment() end
002318.191: brfs: File read: 777 bytes
002329.219: dtdebug: Opened overlay file 'overlays/hyperpixel-gpio-backlight.dtbo'

But `gpio readall` yields:

Code: Select all

 +-----+-----+---------+------+---+---Pi 3+--+---+------+---------+-----+-----+
 | BCM | wPi |   Name  | Mode | V | Physical | V | Mode | Name    | wPi | BCM |
 +-----+-----+---------+------+---+----++----+---+------+---------+-----+-----+
 |     |     |    3.3v |      |   |  1 || 2  |   |      | 5v      |     |     |
 |   2 |   8 |   SDA.1 |   IN | 1 |  3 || 4  |   |      | 5v      |     |     |
 |   3 |   9 |   SCL.1 |   IN | 1 |  5 || 6  |   |      | 0v      |     |     |
 |   4 |   7 | GPIO. 7 |   IN | 0 |  7 || 8  | 0 | IN   | TxD     | 15  | 14  |
 |     |     |      0v |      |   |  9 || 10 | 1 | IN   | RxD     | 16  | 15  |
 |  17 |   0 | GPIO. 0 |   IN | 0 | 11 || 12 | 1 | OUT  | GPIO. 1 | 1   | 18  |
 |  27 |   2 | GPIO. 2 |  OUT | 1 | 13 || 14 |   |      | 0v      |     |     |     <------------ BCM 27 = OUT, value 1
 |  22 |   3 | GPIO. 3 |   IN | 0 | 15 || 16 | 1 | IN   | GPIO. 4 | 4   | 23  |
 |     |     |    3.3v |      |   | 17 || 18 | 0 | IN   | GPIO. 5 | 5   | 24  |
 |  10 |  12 |    MOSI |   IN | 0 | 19 || 20 |   |      | 0v      |     |     |
 |   9 |  13 |    MISO |   IN | 0 | 21 || 22 | 0 | IN   | GPIO. 6 | 6   | 25  |
 |  11 |  14 |    SCLK |   IN | 0 | 23 || 24 | 0 | IN   | CE0     | 10  | 8   |
 |     |     |      0v |      |   | 25 || 26 | 0 | IN   | CE1     | 11  | 7   |
 |   0 |  30 |   SDA.0 |   IN | 1 | 27 || 28 | 1 | IN   | SCL.0   | 31  | 1   |
 |   5 |  21 | GPIO.21 |   IN | 0 | 29 || 30 |   |      | 0v      |     |     |
 |   6 |  22 | GPIO.22 |   IN | 0 | 31 || 32 | 0 | IN   | GPIO.26 | 26  | 12  |
 |  13 |  23 | GPIO.23 |   IN | 0 | 33 || 34 |   |      | 0v      |     |     |
 |  19 |  24 | GPIO.24 |  OUT | 0 | 35 || 36 | 0 | IN   | GPIO.27 | 27  | 16  |
 |  26 |  25 | GPIO.25 |  OUT | 0 | 37 || 38 | 0 | IN   | GPIO.28 | 28  | 20  |
 |     |     |      0v |      |   | 39 || 40 | 0 | IN   | GPIO.29 | 29  | 21  |
 +-----+-----+---------+------+---+----++----+---+------+---------+-----+-----+
 | BCM | wPi |   Name  | Mode | V | Physical | V | Mode | Name    | wPi | BCM |
 +-----+-----+---------+------+---+---Pi 3+--+---+------+---------+-----+-----+

So the pin is not being set as an input as I expected it to.
I still don't think I quite understand who has the responsibility to set the pin as an Input according to the overlay. In this case, should it be the compatible driver that is associated to the LEDs node?
So when I put the pinctrl under my own node, does that mean my driver now has to set it to input mode according to the DTS configuration?

[PhilE]

I think the pinctrl value are sufficient to make the pin an input (which is the default, anyway), so I think somethin else is interfering with the pin. You don't state whether that "gpio readall" is before running any code - can you confirm or retest, accordingly?

[procount]

AFAIK, it is a plain Raspbian install (which had the touchscreen drivers, installed but I've removed them again) so I've just added this touch overlay to config.txt. There's no other code running for the touchscreen (although the driver for the display is still being loaded.
Maybe I will spin up another fresh copy and JUST put touch.dtbo in it to see what happens...

EDIT:`gpio readall` is being executed manually by ssh as soon as the desktop has booted and I'm able to login, so after any device-tree stuff should have happened.

[PhilE]

One observation: your driver won't report an error if it fails to claim the GPIO because devm_gpiod_get_optional returns a valid pointer or NULL, not an error (that's what the _optional part means).

[procount]

OK will fix that.
Yes, something else is setting the pin. A fresh Raspbian does not suffer from this. Got some more debugging to do....

[procount]

Ah - the fog is lifting!
Pin 13 (BCM27) is ALSO used to clock data into the LCD driver chip for initialisation, and it is being left in the output high state.
The python Touchscreen driver runs much later and resets it to an input, so it doesn't rely on DT to set the state.
SO, I will have to manually set the pin to input state again in my touchscreen driver, or at the end of the LCD initialisation.
That explains why my DT settings are having no effect whatsoever.

[procount]

Setting the pin to input at the end of the LCD initialisation was not sufficient.
I had to add `gpio_direction_input ( desc_to_gpio(ts->gpiod_int) );` to the initialisation of the touchscreen driver (unless there's a better way), but now it is behaving as expected.

Thanks for all your help @PhilE.

[procount]

(Maybe I should create a new thread in interfacing or HATS? but I'm going to continue here as it's the same project of creating a touchscreen driver)

I completed my cypress.c driver, it reads the device tree, detects the hardware, reads the i2c touchscreen data and can process the touches correctly. But it does not generate the correct events.
I have based it off goodix.c but when I run them both under evtest I notice that the supported events of both drivers are dramatically different and I can't tell where this difference is coming from. I'm assuming it is this capability that is blocking the other ABS_MT events from coming through?

Here is the result of my cypress.c driver:

Code: Select all

# evtest event0
Input driver version is 1.0.1
Input device ID: bus 0x18 vendor 0x416 product 0x0 version 0x0
Input device name: "Cypress Capacitive TouchScreen"
Supported events:
  Event type 0 (EV_SYN)
  Event type 3 (EV_ABS)
    Event code 53 (ABS_MT_POSITION_X)
      Value      0
      Min        0
      Max        0
    Event code 54 (ABS_MT_POSITION_Y)
      Value      0
      Min        0
      Max        0
Properties:
Testing ... (interrupt to exit)
Event: time 847.601478, type 3 (EV_ABS), code 53 (ABS_MT_POSITION_X), value 543
Event: time 847.601478, type 3 (EV_ABS), code 54 (ABS_MT_POSITION_Y), value 469
Event: time 847.601478, -------------- EV_SYN ------------

My debug messages showing the logic is working is as follows:

Code: Select all

[  847.488248] Cypress-TS 0-005c: Press 1
[  847.604846] Cypress-TS 0-005c: Move 1: (543,469)
[  847.726344] Cypress-TS 0-005c: Release 1

But the goodix code has many more supported events:

Code: Select all

# evtest event0
Input driver version is 1.0.1
Input device ID: bus 0x18 vendor 0x416 product 0x38f version 0x1060
Input device name: "Goodix Capacitive TouchScreen"
Supported events:
  Event type 0 (EV_SYN)
  Event type 1 (EV_KEY)
    Event code 125 (KEY_LEFTMETA)
    Event code 330 (BTN_TOUCH)
  Event type 3 (EV_ABS)
    Event code 0 (ABS_X)
      Value      0
      Min        0
      Max      800
    Event code 1 (ABS_Y)
      Value      0
      Min        0
      Max      480
    Event code 47 (ABS_MT_SLOT)
      Value      0
      Min        0
      Max        4
    Event code 48 (ABS_MT_TOUCH_MAJOR)
      Value      0
      Min        0
      Max      255
    Event code 50 (ABS_MT_WIDTH_MAJOR)
      Value      0
      Min        0
      Max      255
    Event code 53 (ABS_MT_POSITION_X)
      Value      0
      Min        0
      Max      800
    Event code 54 (ABS_MT_POSITION_Y)
      Value      0
      Min        0
      Max      480
    Event code 57 (ABS_MT_TRACKING_ID)
      Value      0
      Min        0
      Max    65535
Properties:
  Property type 1 (INPUT_PROP_DIRECT)
Testing ... (interrupt to exit)
Event: time 70.931811, type 3 (EV_ABS), code 57 (ABS_MT_TRACKING_ID), value 0
Event: time 70.931811, type 3 (EV_ABS), code 53 (ABS_MT_POSITION_X), value 363
Event: time 70.931811, type 3 (EV_ABS), code 54 (ABS_MT_POSITION_Y), value 184
Event: time 70.931811, type 3 (EV_ABS), code 48 (ABS_MT_TOUCH_MAJOR), value 71
Event: time 70.931811, type 3 (EV_ABS), code 50 (ABS_MT_WIDTH_MAJOR), value 71
Event: time 70.931811, type 1 (EV_KEY), code 330 (BTN_TOUCH), value 1
Event: time 70.931811, type 3 (EV_ABS), code 0 (ABS_X), value 363
Event: time 70.931811, type 3 (EV_ABS), code 1 (ABS_Y), value 184
Event: time 70.931811, -------------- EV_SYN ------------
Event: time 71.103546, type 3 (EV_ABS), code 57 (ABS_MT_TRACKING_ID), value -1
Event: time 71.103546, type 1 (EV_KEY), code 330 (BTN_TOUCH), value 0
Event: time 71.103546, -------------- EV_SYN ------------
Event: time 83.168580, type 3 (EV_ABS), code 57 (ABS_MT_TRACKING_ID), value 1
Event: time 83.168580, type 3 (EV_ABS), code 53 (ABS_MT_POSITION_X), value 317
Event: time 83.168580, type 3 (EV_ABS), code 54 (ABS_MT_POSITION_Y), value 173
Event: time 83.168580, type 3 (EV_ABS), code 48 (ABS_MT_TOUCH_MAJOR), value 79
Event: time 83.168580, type 3 (EV_ABS), code 50 (ABS_MT_WIDTH_MAJOR), value 79
Event: time 83.168580, type 1 (EV_KEY), code 330 (BTN_TOUCH), value 1
Event: time 83.168580, type 3 (EV_ABS), code 0 (ABS_X), value 317
Event: time 83.168580, type 3 (EV_ABS), code 1 (ABS_Y), value 173
Event: time 83.168580, -------------- EV_SYN ------------
Event: time 83.252907, type 3 (EV_ABS), code 57 (ABS_MT_TRACKING_ID), value -1
Event: time 83.252907, type 1 (EV_KEY), code 330 (BTN_TOUCH), value 0
Event: time 83.252907, -------------- EV_SYN ------------

I also note that the goodix driver creates a /dev/input/mouse0 node, but my cypress. driver does not.

What am I missing? (source code is on github)

[PhilE]

The touchscreen dimensions are configured in cypress_get_gpio_config and registered in cypress_request_input_dev, but your probe function says:

Code: Select all

  cypress_request_input_dev(ts);

  cypress_get_gpio_config(ts);

i.e. the values are used before they are properly configured.

Also, goodix_request_input_dev is clearly registering more input types than cypress_request_input_dev, so I don't know why you would be surprised at the different output from evtest.