boards: shield: Support touch for the NXP LCD-PAR-S035 LCD on MCXN947 #75353

groncarolonxp · 2024-07-03T05:02:30Z

NXP LCD-PAR-S035 LCD supports touch: enable it
The reset pin of the LCD and the touch controller are shared so we set alt-addr to select GT911 I2C address by probing

github-actions · 2024-07-03T05:03:10Z

Hello @groncarolonxp, and thank you very much for your first pull request to the Zephyr project!
Our Continuous Integration pipeline will execute a series of checks on your Pull Request commit messages and code, and you are expected to address any failures by updating the PR. Please take a look at our commit message guidelines to find out how to format your commit messages, and at our contribution workflow to understand how to update your Pull Request. If you haven't already, please make sure to review the project's Contributor Expectations and update (by amending and force-pushing the commits) your pull request if necessary.
If you are stuck or need help please join us on Discord and ask your question there. Additionally, you can escalate the review when applicable. 😊

groncarolonxp · 2024-07-04T04:51:46Z

The PR has been tested on samples/subsys/input/input_dump
For reference
west build -b frdm_mcxn947/mcxn947/cpu0 zephyr/samples/subsys/input/input_dump -- -DSHIELD=lcd_par_s035_8080

PetervdPerk-NXP · 2024-07-29T11:04:51Z

@groncarolonxp you've verified the orientation on LVGL as well?
https://docs.zephyrproject.org/latest/build/dts/api/bindings/input/zephyr,lvgl-pointer-input.html
See swap-xy etc,

groncarolonxp · 2024-07-30T05:52:54Z

@PetervdPerk-NXP you are absolutely right
I fixed the values for lvgl and tested with both
samples/modules/lvgl/demos/
and
zephyr/samples/subsys/display/lvgl

To have the second demo show the button I had to add

+config INPUT
into boards/shields/lcd_par_s035/Kconfig.defconfi

I have a doubt that is in general correct: what do you think?

Myabe something like

+++ b/boards/shields/lcd_par_s035/Kconfig.defconfig
@@ -4,10 +4,11 @@
 if SHIELD_LCD_PAR_S035
 if LVGL
 
+if BOARD_FRDM_MCXN947
 # Configure LVGL to use touchscreen with KSCAN API
-
 config INPUT
        default y
+endif # BOARD_FRDM_MCXN947
 
 # Enable double buffering
 config LV_Z_DOUBLE_VDB

would be acceptable?

PetervdPerk-NXP · 2024-07-30T06:54:21Z

would be acceptable?

It seems to match what do in other LCD touchscreen overlays as well

zephyr/boards/shields/rk043fn66hs_ctg/Kconfig.defconfig

Lines 8 to 10 in 3bcaa6f

    
           config INPUT 
        
           	default y

Maybe @danieldegrasse could shed a light on this.

kartben · 2024-07-30T16:55:58Z

boards/shields/lcd_par_s035/boards/frdm_mcxn947_mcxn947_cpu0.overlay

+&flexcomm2_lpi2c2 {
+	status = "okay";
+	gt911_lcd_par_s035: gt911-lcd_par_s035@5d {
+		compatible = "goodix,gt911";
+		reg = <0x5d>;
+		irq-gpios = <&gpio4 6 GPIO_ACTIVE_HIGH>;
+		alt-addr = <0x14>;
+	};
+};


Ideally there should be no board specific overlay? If the shield as a touch controller, it should be in the shield overlay. The irq-gpios should also be made generic and rely on a GPIO nexus so that "gpio4 6" is not hard-coded but instead references a physical pin on whatever connector is used to interface the shield with the board.
Thanks!

Thanks a lot for you comment: that was our first take and we added the GPIO in the dtsi of the board and the refenced in the shield overlay.
The issue is that on MCXn974 the reset pin of the LCD and the touch controller are shared so we would end up ( i fear) having to write this specific overlay anyway.
To be more clear: a generic implementation would not have
alt-addr = <0x14>;
but another GPIO for reset that cannot be specified for MCX
That is why we ended up with this solution and given it is board specific we specified the GPIO directly here.
We can put the GPIO into the mcx dts(i) and reference it here, but we would still need this specif board overlay.
Do you see my point?

It's one thing to have a board specific overlay for what's really unique to the board, but it's another to have the entire definition associated with the touch controller be 100% board specific. This kind of defeats the purpose of having shields in the first place :)

As the physical interface between the shield and the host board is a 28-pin FlexIO header, this header should be defined as a GPIO nexus, all the boards having such a connector (ex. MCXN947) should indicate what actual GPIOs are routed to each of the 28-pins, and the (generic) shield overlay should then once and for all indicate that the reset pin is pin #7 of the GPIO Nexus, as this will always be true.

Similarly, and sorry that I didn't catch it yesterday, the generic shield overlay should not make any mention of flexcomm2_lpi2c2, which is board specific, but instead attach the touch controller to something like flexio_i2c, which each board would define similarly to how boards with arduino headers inidcate which I2C bug is exposed as arduino_i2c.

BTW: it would be great if someone from NXP could also send a PR adding Arduino and MikroE headers definition for the FRDM MCXN947, i.e. something similar to

zephyr/boards/nxp/lpcxpresso55s06/lpcxpresso55s06_common.dtsi

Lines 68 to 117 in 763dba5

mikrobus_header: mikrobus-connector {

compatible = "mikro-bus";

#gpio-cells = <2>;

gpio-map-mask = <0xffffffff 0xffffffc0>;

gpio-map-pass-thru = <0 0x3f>;

gpio-map = <0 0 &gpio0 16 0>, /* AN */

/* Not a GPIO */ /* RST */

<2 0 &gpio1 1 0>, /* CS */

<3 0 &gpio1 2 0>, /* SCK */

<4 0 &gpio1 3 0>, /* MISO */

<5 0 &gpio0 26 0>, /* MOSI */

/* +3.3V */

/* GND */

<6 0 &gpio1 5 0>, /* PWM */

<7 0 &gpio0 28 0>, /* INT */

<8 0 &gpio1 10 0>, /* RX */

<9 0 &gpio1 11 0>, /* TX */

<10 0 &gpio0 24 0>, /* SCL */

<11 0 &gpio0 25 0>; /* SDA */

/* +5V */

/* GND */

};

arduino_header: arduino-connector {

compatible = "arduino-header-r3";

#gpio-cells = <2>;

gpio-map-mask = <0xffffffff 0xffffffc0>;

gpio-map-pass-thru = <0 0x3f>;

gpio-map = <0 0 &gpio0 16 0>, /* A0 */

<1 0 &gpio0 23 0>, /* A1 */

<2 0 &gpio0 9 0>, /* A2 */

<3 0 &gpio0 0 0>, /* A3 */

<4 0 &gpio0 13 0>, /* A4 */

<5 0 &gpio0 14 0>, /* A5 */

<6 0 &gpio1 10 0>, /* D0 */

<7 0 &gpio1 11 0>, /* D1 */

<8 0 &gpio0 15 0>, /* D2 */

<9 0 &gpio0 23 0>, /* D3 */

<10 0 &gpio0 22 0>, /* D4 */

<11 0 &gpio0 19 0>, /* D5 */

<12 0 &gpio0 18 0>, /* D6 */

<13 0 &gpio0 2 0>, /* D7 */

<14 0 &gpio0 10 0>, /* D8 */

<15 0 &gpio0 25 0>, /* D9 */

<16 0 &gpio1 1 0>, /* D10 */

<17 0 &gpio0 26 0>, /* D11 */

<18 0 &gpio1 3 0>, /* D12 */

<19 0 &gpio1 2 0>; /* D13 */

};

};

:)

Thanks a lot fot the pointers!
I will write it in that way and push it when ready

@kartben I hope I got it more as it should ( I will split the commit into more reasonable chunks as soon as the content is right) :)

PetervdPerk-NXP · 2024-08-01T06:56:56Z

boards/shields/lcd_par_s035/lcd_par_s035_8080.overlay

+	gt911_lcd_par_s035: gt911-lcd_par_s035@5d {
+		compatible = "goodix,gt911";
+		reg = <0x5d>;
+		irq-gpios = <&nxp_flexio_connector 0 GPIO_ACTIVE_HIGH>; // INT


This looks wrong irq-gpios is an phandle-array now the 2nd assignment overrules the first one.

you are right!

NXP LCD-PAR-S035 LCD supports touch: enable it The reset pin of the LCD and the touch controller are shared so we set alt-addr to select GT911 I2C address by probing Co-authored-by: Pascal Mareau <pascal.mareau@nxp.com> Co-authored-by: Guido Roncarolo <guido.roncarolo@nxp.com Signed-off-by: Guido Roncarolo <guido.roncarolo@nxp.com>

danieldegrasse · 2024-08-16T20:34:08Z

boards/shields/lcd_par_s035/boards/frdm_mcxn947_mcxn947_cpu0.overlay

+&flexio_i2c {
+	status = "okay";
+	gt911_lcd_par_s035: gt911-lcd_par_s035@5d {
+		compatible = "goodix,gt911";


No need to duplicate the full definition here, you can just delete the reset-gpios property and add alt-addr, right?

danieldegrasse · 2024-08-16T20:43:53Z

boards/nxp/frdm_mcxn947/frdm_mcxn947.dtsi

@@ -52,14 +52,26 @@
 			status = "disabled";
 		};
 	};
+
+	nxp_flexio_connector: flexio-connector {
+		compatible = "gpio-nexus";


We should be defining a custom compatible for the flexio connector, and adding a pin map in the dt binding. See here for an example for the 6 pin FPC touch connector present on NXP RT10xx EVKs:

zephyr/dts/bindings/gpio/nxp,i2c-tsc-fpc.yaml

Line 10 in cce7e9a

following assignments:

Also, should we be defining additional pins beyond the INT and RESET? The FlexIO connector is 28 pins in total, and many can be used as GPIOs, right?

Generally the approach taken is to define all pins for the interface in the binding file, and only define the pins that are actually in use as GPIOs in the devicetree for the board. A good example of this would be the 40 pin parallel LCD connector on the RT10xx boards:

zephyr/dts/bindings/gpio/nxp,parallel-lcd-connector.yaml

Line 13 in cce7e9a

1 LED backlight cathode

thanks for the suggestion!
duly applied

Signed-off-by: Guido Roncarolo <guido.roncarolo@nxp.com>

henrikbrixandersen · 2024-10-07T10:21:02Z

dts/bindings/gpio/nxp,flexio-lcd.yaml

This does not belong in dts/bindings/gpio/.

zephyrbot added the area: Shields Shields (add-on boards) label Jul 3, 2024

zephyrbot requested review from avisconti, erwango, jfischer-no and kartben July 3, 2024 05:03

zephyrbot assigned kartben Jul 3, 2024

butok requested a review from PetervdPerk-NXP July 29, 2024 10:59

groncarolonxp force-pushed the mcx_touch_2 branch from 13a4f8b to e67eff9 Compare July 30, 2024 05:56

butok requested review from danieldegrasse and DerekSnell July 30, 2024 07:47

kartben requested changes Jul 30, 2024

View reviewed changes

groncarolonxp force-pushed the mcx_touch_2 branch from e67eff9 to f5bf20b Compare August 1, 2024 06:45

zephyrbot added the platform: NXP NXP label Aug 1, 2024

zephyrbot requested review from butok, decsny, dleach02, EmilioCBen, mmahadevan108 and yvanderv August 1, 2024 06:45

PetervdPerk-NXP reviewed Aug 1, 2024

View reviewed changes

groncarolonxp force-pushed the mcx_touch_2 branch from f5bf20b to 82fcb0d Compare August 1, 2024 08:21

danieldegrasse reviewed Aug 16, 2024

View reviewed changes

groncarolonxp added 2 commits August 19, 2024 09:11

remove duplicate full definition

a49ebaf

Signed-off-by: Guido Roncarolo <guido.roncarolo@nxp.com>

use yaml

f810996

Signed-off-by: Guido Roncarolo <guido.roncarolo@nxp.com>

zephyrbot added platform: NXP Drivers NXP Semiconductors, drivers area: GPIO labels Sep 6, 2024

zephyrbot requested a review from henrikbrixandersen September 6, 2024 07:21

decsny removed their request for review September 6, 2024 17:16

henrikbrixandersen requested changes Oct 7, 2024

View reviewed changes

dts/bindings/gpio/nxp,flexio-lcd.yaml

Copy link

Member

henrikbrixandersen Oct 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does not belong in dts/bindings/gpio/.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

boards: shield: Support touch for the NXP LCD-PAR-S035 LCD on MCXN947 #75353

boards: shield: Support touch for the NXP LCD-PAR-S035 LCD on MCXN947 #75353

groncarolonxp commented Jul 3, 2024

github-actions bot commented Jul 3, 2024

groncarolonxp commented Jul 4, 2024

PetervdPerk-NXP commented Jul 29, 2024

groncarolonxp commented Jul 30, 2024 •

edited

Loading

PetervdPerk-NXP commented Jul 30, 2024

kartben Jul 30, 2024

groncarolonxp Jul 31, 2024 •

edited

Loading

kartben Jul 31, 2024 •

edited

Loading

groncarolonxp Jul 31, 2024

groncarolonxp Aug 1, 2024

PetervdPerk-NXP Aug 1, 2024

groncarolonxp Aug 1, 2024

danieldegrasse Aug 16, 2024

danieldegrasse Aug 16, 2024

groncarolonxp Sep 6, 2024

henrikbrixandersen Oct 7, 2024

	mikrobus_header: mikrobus-connector {
	compatible = "mikro-bus";
	#gpio-cells = <2>;
	gpio-map-mask = <0xffffffff 0xffffffc0>;
	gpio-map-pass-thru = <0 0x3f>;
	gpio-map = <0 0 &gpio0 16 0>, /* AN */
	/* Not a GPIO / / RST */
	<2 0 &gpio1 1 0>, /* CS */
	<3 0 &gpio1 2 0>, /* SCK */
	<4 0 &gpio1 3 0>, /* MISO */
	<5 0 &gpio0 26 0>, /* MOSI */
	/* +3.3V */
	/* GND */
	<6 0 &gpio1 5 0>, /* PWM */
	<7 0 &gpio0 28 0>, /* INT */
	<8 0 &gpio1 10 0>, /* RX */
	<9 0 &gpio1 11 0>, /* TX */
	<10 0 &gpio0 24 0>, /* SCL */
	<11 0 &gpio0 25 0>; /* SDA */
	/* +5V */
	/* GND */
	};

	arduino_header: arduino-connector {
	compatible = "arduino-header-r3";
	#gpio-cells = <2>;
	gpio-map-mask = <0xffffffff 0xffffffc0>;
	gpio-map-pass-thru = <0 0x3f>;
	gpio-map = <0 0 &gpio0 16 0>, /* A0 */
	<1 0 &gpio0 23 0>, /* A1 */
	<2 0 &gpio0 9 0>, /* A2 */
	<3 0 &gpio0 0 0>, /* A3 */
	<4 0 &gpio0 13 0>, /* A4 */
	<5 0 &gpio0 14 0>, /* A5 */
	<6 0 &gpio1 10 0>, /* D0 */
	<7 0 &gpio1 11 0>, /* D1 */
	<8 0 &gpio0 15 0>, /* D2 */
	<9 0 &gpio0 23 0>, /* D3 */
	<10 0 &gpio0 22 0>, /* D4 */
	<11 0 &gpio0 19 0>, /* D5 */
	<12 0 &gpio0 18 0>, /* D6 */
	<13 0 &gpio0 2 0>, /* D7 */
	<14 0 &gpio0 10 0>, /* D8 */
	<15 0 &gpio0 25 0>, /* D9 */
	<16 0 &gpio1 1 0>, /* D10 */
	<17 0 &gpio0 26 0>, /* D11 */
	<18 0 &gpio1 3 0>, /* D12 */
	<19 0 &gpio1 2 0>; /* D13 */
	};
	};

boards: shield: Support touch for the NXP LCD-PAR-S035 LCD on MCXN947 #75353

Are you sure you want to change the base?

boards: shield: Support touch for the NXP LCD-PAR-S035 LCD on MCXN947 #75353

Conversation

groncarolonxp commented Jul 3, 2024

github-actions bot commented Jul 3, 2024

groncarolonxp commented Jul 4, 2024

PetervdPerk-NXP commented Jul 29, 2024

groncarolonxp commented Jul 30, 2024 • edited Loading

PetervdPerk-NXP commented Jul 30, 2024

Choose a reason for hiding this comment

groncarolonxp Jul 31, 2024 • edited Loading

Choose a reason for hiding this comment

kartben Jul 31, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

groncarolonxp commented Jul 30, 2024 •

edited

Loading

groncarolonxp Jul 31, 2024 •

edited

Loading

kartben Jul 31, 2024 •

edited

Loading