[v2] ARM: dts: imx: e60k02: Add touchscreen

[PATCH v2] ARM: dts: imx: e60k02: Add touchscreen

Posted by Andreas Kemnade 3 years, 5 months ago

Add the touchscreen now, since the driver is available.

Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
---
Changes in v2: fix pinmux naming

 arch/arm/boot/dts/e60k02.dtsi              | 11 ++++++++++-
 arch/arm/boot/dts/imx6sl-tolino-shine3.dts |  7 +++++++
 arch/arm/boot/dts/imx6sll-kobo-clarahd.dts |  7 +++++++
 3 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/e60k02.dtsi b/arch/arm/boot/dts/e60k02.dtsi
index 935e2359f8df..94944cc21931 100644
--- a/arch/arm/boot/dts/e60k02.dtsi
+++ b/arch/arm/boot/dts/e60k02.dtsi
@@ -104,7 +104,16 @@ &i2c2 {
 	clock-frequency = <100000>;
 	status = "okay";
 
-	/* TODO: CYTTSP5 touch controller at 0x24 */
+	touchscreen@24 {
+		compatible = "cypress,tt21000";
+		reg = <0x24>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&pinctrl_cyttsp5_gpio>;
+		interrupt-parent = <&gpio5>;
+		interrupts = <6 IRQ_TYPE_EDGE_FALLING>;
+		reset-gpios = <&gpio5 13 GPIO_ACTIVE_LOW>;
+		vdd-supply = <&ldo5_reg>;
+	};
 
 	/* TODO: TPS65185 PMIC for E Ink at 0x68 */
 
diff --git a/arch/arm/boot/dts/imx6sl-tolino-shine3.dts b/arch/arm/boot/dts/imx6sl-tolino-shine3.dts
index e3f1e8d79528..db5d8509935f 100644
--- a/arch/arm/boot/dts/imx6sl-tolino-shine3.dts
+++ b/arch/arm/boot/dts/imx6sl-tolino-shine3.dts
@@ -52,6 +52,13 @@ &iomuxc {
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_hog>;
 
+	pinctrl_cyttsp5_gpio: cyttsp5-gpiogrp {
+		fsl,pins = <
+			MX6SL_PAD_SD1_DAT3__GPIO5_IO06                0x17059 /* TP_INT */
+			MX6SL_PAD_SD1_DAT2__GPIO5_IO13                0x10059 /* TP_RST */
+		>;
+	};
+
 	pinctrl_gpio_keys: gpio-keysgrp {
 		fsl,pins = <
 			MX6SL_PAD_SD1_DAT1__GPIO5_IO08	0x17059	/* PWR_SW */
diff --git a/arch/arm/boot/dts/imx6sll-kobo-clarahd.dts b/arch/arm/boot/dts/imx6sll-kobo-clarahd.dts
index 90b32f5eb529..c7cfe0b70f04 100644
--- a/arch/arm/boot/dts/imx6sll-kobo-clarahd.dts
+++ b/arch/arm/boot/dts/imx6sll-kobo-clarahd.dts
@@ -62,6 +62,13 @@ &iomuxc {
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_hog>;
 
+	pinctrl_cyttsp5_gpio: cyttsp5-gpiogrp {
+		fsl,pins = <
+			MX6SLL_PAD_SD1_DATA3__GPIO5_IO06                0x17059 /* TP_INT */
+			MX6SLL_PAD_SD1_DATA2__GPIO5_IO13                0x10059 /* TP_RST */
+		>;
+	};
+
 	pinctrl_gpio_keys: gpio-keysgrp {
 		fsl,pins = <
 			MX6SLL_PAD_SD1_DATA1__GPIO5_IO08	0x17059	/* PWR_SW */
-- 
2.30.2

Re: [PATCH v2] ARM: dts: imx: e60k02: Add touchscreen

Posted by Shawn Guo 3 years, 4 months ago

On Fri, Nov 04, 2022 at 09:42:51PM +0100, Andreas Kemnade wrote:
> Add the touchscreen now, since the driver is available.
> 
> Signed-off-by: Andreas Kemnade <andreas@kemnade.info>

Applied, thanks!

Re: [PATCH v2] ARM: dts: imx: e60k02: Add touchscreen

Posted by Marco Felsch 3 years, 5 months ago

Hi Andreas,

On 22-11-04, Andreas Kemnade wrote:
> Add the touchscreen now, since the driver is available.
> 
> Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
> ---
> Changes in v2: fix pinmux naming
> 
>  arch/arm/boot/dts/e60k02.dtsi              | 11 ++++++++++-
>  arch/arm/boot/dts/imx6sl-tolino-shine3.dts |  7 +++++++
>  arch/arm/boot/dts/imx6sll-kobo-clarahd.dts |  7 +++++++
>  3 files changed, 24 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/boot/dts/e60k02.dtsi b/arch/arm/boot/dts/e60k02.dtsi
> index 935e2359f8df..94944cc21931 100644
> --- a/arch/arm/boot/dts/e60k02.dtsi
> +++ b/arch/arm/boot/dts/e60k02.dtsi
> @@ -104,7 +104,16 @@ &i2c2 {
>  	clock-frequency = <100000>;
>  	status = "okay";
>  
> -	/* TODO: CYTTSP5 touch controller at 0x24 */
> +	touchscreen@24 {
> +		compatible = "cypress,tt21000";
> +		reg = <0x24>;
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&pinctrl_cyttsp5_gpio>;
> +		interrupt-parent = <&gpio5>;
> +		interrupts = <6 IRQ_TYPE_EDGE_FALLING>;
> +		reset-gpios = <&gpio5 13 GPIO_ACTIVE_LOW>;
> +		vdd-supply = <&ldo5_reg>;
> +	};
>  
>  	/* TODO: TPS65185 PMIC for E Ink at 0x68 */
>  
> diff --git a/arch/arm/boot/dts/imx6sl-tolino-shine3.dts b/arch/arm/boot/dts/imx6sl-tolino-shine3.dts
> index e3f1e8d79528..db5d8509935f 100644
> --- a/arch/arm/boot/dts/imx6sl-tolino-shine3.dts
> +++ b/arch/arm/boot/dts/imx6sl-tolino-shine3.dts
> @@ -52,6 +52,13 @@ &iomuxc {
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&pinctrl_hog>;
>  
> +	pinctrl_cyttsp5_gpio: cyttsp5-gpiogrp {
> +		fsl,pins = <
> +			MX6SL_PAD_SD1_DAT3__GPIO5_IO06                0x17059 /* TP_INT */
> +			MX6SL_PAD_SD1_DAT2__GPIO5_IO13                0x10059 /* TP_RST */
> +		>;
> +	};

Hm.. I don't like that you enable it within the base dtsi and add the
missing dependencies here. I would rather add the touchscreen within the
base dtsi but with with status set to disabled and without specifying
the interrupt and reset gpio. Within the dts and the below you can
reference the touchscreen via phandle and add the missing properties.

Regards,
  Marco

> +
>  	pinctrl_gpio_keys: gpio-keysgrp {
>  		fsl,pins = <
>  			MX6SL_PAD_SD1_DAT1__GPIO5_IO08	0x17059	/* PWR_SW */
> diff --git a/arch/arm/boot/dts/imx6sll-kobo-clarahd.dts b/arch/arm/boot/dts/imx6sll-kobo-clarahd.dts
> index 90b32f5eb529..c7cfe0b70f04 100644
> --- a/arch/arm/boot/dts/imx6sll-kobo-clarahd.dts
> +++ b/arch/arm/boot/dts/imx6sll-kobo-clarahd.dts
> @@ -62,6 +62,13 @@ &iomuxc {
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&pinctrl_hog>;
>  
> +	pinctrl_cyttsp5_gpio: cyttsp5-gpiogrp {
> +		fsl,pins = <
> +			MX6SLL_PAD_SD1_DATA3__GPIO5_IO06                0x17059 /* TP_INT */
> +			MX6SLL_PAD_SD1_DATA2__GPIO5_IO13                0x10059 /* TP_RST */
> +		>;
> +	};
> +
>  	pinctrl_gpio_keys: gpio-keysgrp {
>  		fsl,pins = <
>  			MX6SLL_PAD_SD1_DATA1__GPIO5_IO08	0x17059	/* PWR_SW */
> -- 
> 2.30.2
> 
> 
>

Re: [PATCH v2] ARM: dts: imx: e60k02: Add touchscreen

Posted by Andreas Kemnade 3 years, 5 months ago

Hi,

On Mon, 7 Nov 2022 09:12:34 +0100
Marco Felsch <m.felsch@pengutronix.de> wrote:

> Hi Andreas,
> 
[...]
> > diff --git a/arch/arm/boot/dts/imx6sl-tolino-shine3.dts b/arch/arm/boot/dts/imx6sl-tolino-shine3.dts
> > index e3f1e8d79528..db5d8509935f 100644
> > --- a/arch/arm/boot/dts/imx6sl-tolino-shine3.dts
> > +++ b/arch/arm/boot/dts/imx6sl-tolino-shine3.dts
> > @@ -52,6 +52,13 @@ &iomuxc {
> >  	pinctrl-names = "default";
> >  	pinctrl-0 = <&pinctrl_hog>;
> >  
> > +	pinctrl_cyttsp5_gpio: cyttsp5-gpiogrp {
> > +		fsl,pins = <
> > +			MX6SL_PAD_SD1_DAT3__GPIO5_IO06                0x17059 /* TP_INT */
> > +			MX6SL_PAD_SD1_DAT2__GPIO5_IO13                0x10059 /* TP_RST */
> > +		>;
> > +	};  
> 
> Hm.. I don't like that you enable it within the base dtsi and add the
> missing dependencies here. I would rather add the touchscreen within the
> base dtsi but with with status set to disabled and without specifying
> the interrupt and reset gpio. Within the dts and the below you can
> reference the touchscreen via phandle and add the missing properties.
> 
hmm, that makes things more complex and unreadable. Remember, we have two boards
differ in a pin-compatible soc here. Why I should set something present
in all cases to disabled?
And why I should add hardware just to set to disabled?

I notice that I have not followed the agreed pattern for this things like in
the lm3630 setting, there we have


       pinctrl_lm3630a_bl_gpio: lm3630a-bl-gpiogrp {
                fsl,pins = <
                        MX6SLL_PAD_EPDC_PWR_CTRL3__GPIO2_IO10   0x10059 /* HWEN */
                >;
        };
[...]
&lm3630a {
        pinctrl-names = "default";
        pinctrl-0 = <&pinctrl_lm3630a_bl_gpio>;
};

in the dts file, so there is no phandle pointing from dtsi to dts.
I will send an updated version following this widely adopted style.

Regards,
Andreas