diff mbox

[05/12] ARM: dts: imx6ul-isiot-common: Add touchscreen node

Message ID 1490204897-14525-6-git-send-email-jagan@openedev.com (mailing list archive)
State New, archived
Headers show

Commit Message

Jagan Teki March 22, 2017, 5:48 p.m. UTC
From: Jagan Teki <jagan@amarulasolutions.com>

Add touchscreen node as i2c1 slave device on Engicam Is.IoT MX6UL
modules, the touchscreen controlled 'st,stmpe-ts' connected via
i2c with st,stmpe811 mfb interface.

Cc: Shawn Guo <shawnguo@kernel.org>
Cc: Matteo Lisi <matteo.lisi@engicam.com>
Cc: Michael Trimarchi <michael@amarulasolutions.com>
Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
---
 arch/arm/boot/dts/imx6ul-isiot-common.dtsi | 34 ++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

Comments

Shawn Guo March 24, 2017, 5:16 a.m. UTC | #1
On Wed, Mar 22, 2017 at 11:18:10PM +0530, Jagan Teki wrote:
> From: Jagan Teki <jagan@amarulasolutions.com>
> 
> Add touchscreen node as i2c1 slave device on Engicam Is.IoT MX6UL
> modules, the touchscreen controlled 'st,stmpe-ts' connected via
> i2c with st,stmpe811 mfb interface.

s/mfb/mfd?

> 
> Cc: Shawn Guo <shawnguo@kernel.org>
> Cc: Matteo Lisi <matteo.lisi@engicam.com>
> Cc: Michael Trimarchi <michael@amarulasolutions.com>
> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> ---
>  arch/arm/boot/dts/imx6ul-isiot-common.dtsi | 34 ++++++++++++++++++++++++++++++
>  1 file changed, 34 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/imx6ul-isiot-common.dtsi b/arch/arm/boot/dts/imx6ul-isiot-common.dtsi
> index 9660e20..b149b5e 100644
> --- a/arch/arm/boot/dts/imx6ul-isiot-common.dtsi
> +++ b/arch/arm/boot/dts/imx6ul-isiot-common.dtsi
> @@ -40,6 +40,34 @@
>   *     OTHER DEALINGS IN THE SOFTWARE.
>   */
>  
> +&i2c1 {
> +	touch: stmpe811@44 {

Label a MFD device 'touch'?  Also, the node name should be generic.  Not
sure what should be used for MFD device though.

> +		compatible = "st,stmpe811";
> +		reg = <0x44>;
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&pinctrl_stmpe_ts>;

If this is for stmpe-ts, shouldn't be put into touchscreen child node?

> +		interrupt-parent = <&gpio1>;
> +		interrupts = <18 IRQ_TYPE_EDGE_FALLING>;
> +		interrupt-controller;
> +		#interrupt-cells = <2>;
> +
> +		stmpe_touchscreen {

'stmpe: touchscreen {' might be better.

Shawn

> +			compatible = "st,stmpe-ts";
> +			st,sample-time = <4>;
> +			st,mod-12b = <1>;
> +			st,ref-sel = <0>;
> +			st,adc-freq = <1>;
> +			st,ave-ctrl = <1>;
> +			st,touch-det-delay = <2>;
> +			st,settling = <2>;
> +			st,fraction-z = <7>;
> +			st,i-drive = <1>;
> +		};
> +	};
> +};
> +
>  &lcdif {
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&pinctrl_lcdif_dat
> @@ -104,4 +132,10 @@
>  			MX6UL_PAD_LCD_DATA17__LCDIF_DATA17  0x79
>  		>;
>  	};
> +
> +	pinctrl_stmpe_ts:  stmpe-tsgrp  {
> +		fsl,pins = <
> +			MX6UL_PAD_UART1_CTS_B__GPIO1_IO18 0x1b0b0
> +		>;
> +	};
>  };
> -- 
> 1.9.1
>
Jagan Teki March 24, 2017, 6:59 a.m. UTC | #2
On Fri, Mar 24, 2017 at 10:46 AM, Shawn Guo <shawnguo@kernel.org> wrote:
> On Wed, Mar 22, 2017 at 11:18:10PM +0530, Jagan Teki wrote:
>> From: Jagan Teki <jagan@amarulasolutions.com>
>>
>> Add touchscreen node as i2c1 slave device on Engicam Is.IoT MX6UL
>> modules, the touchscreen controlled 'st,stmpe-ts' connected via
>> i2c with st,stmpe811 mfb interface.
>
> s/mfb/mfd?
>
>>
>> Cc: Shawn Guo <shawnguo@kernel.org>
>> Cc: Matteo Lisi <matteo.lisi@engicam.com>
>> Cc: Michael Trimarchi <michael@amarulasolutions.com>
>> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
>> ---
>>  arch/arm/boot/dts/imx6ul-isiot-common.dtsi | 34 ++++++++++++++++++++++++++++++
>>  1 file changed, 34 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/imx6ul-isiot-common.dtsi b/arch/arm/boot/dts/imx6ul-isiot-common.dtsi
>> index 9660e20..b149b5e 100644
>> --- a/arch/arm/boot/dts/imx6ul-isiot-common.dtsi
>> +++ b/arch/arm/boot/dts/imx6ul-isiot-common.dtsi
>> @@ -40,6 +40,34 @@
>>   *     OTHER DEALINGS IN THE SOFTWARE.
>>   */
>>
>> +&i2c1 {
>> +     touch: stmpe811@44 {
>
> Label a MFD device 'touch'?  Also, the node name should be generic.  Not
> sure what should be used for MFD device though.

May be I can use stmpe811@41 the existing imx6qdl-apalis.dtsi using
the similar notation.

>
>> +             compatible = "st,stmpe811";
>> +             reg = <0x44>;
>> +             #address-cells = <1>;
>> +             #size-cells = <0>;
>> +             pinctrl-names = "default";
>> +             pinctrl-0 = <&pinctrl_stmpe_ts>;
>
> If this is for stmpe-ts, shouldn't be put into touchscreen child node?

Actually this is interrupt pad, for the touch controller via stmpe so
I can rename pinctrl_touch_int.

>
>> +             interrupt-parent = <&gpio1>;
>> +             interrupts = <18 IRQ_TYPE_EDGE_FALLING>;
>> +             interrupt-controller;
>> +             #interrupt-cells = <2>;
>> +
>> +             stmpe_touchscreen {
>
> 'stmpe: touchscreen {' might be better.

stmpe_touchscreen {

This is also compatible with existing dtsi files

thanks!
Shawn Guo March 24, 2017, 7:25 a.m. UTC | #3
On Fri, Mar 24, 2017 at 12:29:49PM +0530, Jagan Teki wrote:
> On Fri, Mar 24, 2017 at 10:46 AM, Shawn Guo <shawnguo@kernel.org> wrote:
> > On Wed, Mar 22, 2017 at 11:18:10PM +0530, Jagan Teki wrote:
> >> From: Jagan Teki <jagan@amarulasolutions.com>
> >>
> >> Add touchscreen node as i2c1 slave device on Engicam Is.IoT MX6UL
> >> modules, the touchscreen controlled 'st,stmpe-ts' connected via
> >> i2c with st,stmpe811 mfb interface.
> >
> > s/mfb/mfd?
> >
> >>
> >> Cc: Shawn Guo <shawnguo@kernel.org>
> >> Cc: Matteo Lisi <matteo.lisi@engicam.com>
> >> Cc: Michael Trimarchi <michael@amarulasolutions.com>
> >> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> >> ---
> >>  arch/arm/boot/dts/imx6ul-isiot-common.dtsi | 34 ++++++++++++++++++++++++++++++
> >>  1 file changed, 34 insertions(+)
> >>
> >> diff --git a/arch/arm/boot/dts/imx6ul-isiot-common.dtsi b/arch/arm/boot/dts/imx6ul-isiot-common.dtsi
> >> index 9660e20..b149b5e 100644
> >> --- a/arch/arm/boot/dts/imx6ul-isiot-common.dtsi
> >> +++ b/arch/arm/boot/dts/imx6ul-isiot-common.dtsi
> >> @@ -40,6 +40,34 @@
> >>   *     OTHER DEALINGS IN THE SOFTWARE.
> >>   */
> >>
> >> +&i2c1 {
> >> +     touch: stmpe811@44 {
> >
> > Label a MFD device 'touch'?  Also, the node name should be generic.  Not
> > sure what should be used for MFD device though.
> 
> May be I can use stmpe811@41 the existing imx6qdl-apalis.dtsi using
> the similar notation.

Per application notes of stmpe811 below, I would suggest you name it
gpio-expander.

www.st.com/resource/zh/datasheet/CD00186725.pdf

> 
> >
> >> +             compatible = "st,stmpe811";
> >> +             reg = <0x44>;
> >> +             #address-cells = <1>;
> >> +             #size-cells = <0>;
> >> +             pinctrl-names = "default";
> >> +             pinctrl-0 = <&pinctrl_stmpe_ts>;
> >
> > If this is for stmpe-ts, shouldn't be put into touchscreen child node?
> 
> Actually this is interrupt pad, for the touch controller via stmpe so
> I can rename pinctrl_touch_int.

Or just pinctrl_stmpe.

> 
> >
> >> +             interrupt-parent = <&gpio1>;
> >> +             interrupts = <18 IRQ_TYPE_EDGE_FALLING>;
> >> +             interrupt-controller;
> >> +             #interrupt-cells = <2>;
> >> +
> >> +             stmpe_touchscreen {
> >
> > 'stmpe: touchscreen {' might be better.
> 
> stmpe_touchscreen {
> 
> This is also compatible with existing dtsi files

We do not need to keep compatibility with something not good.

Shawn
Jagan Teki March 24, 2017, 7:32 a.m. UTC | #4
On Fri, Mar 24, 2017 at 12:55 PM, Shawn Guo <shawnguo@kernel.org> wrote:
> On Fri, Mar 24, 2017 at 12:29:49PM +0530, Jagan Teki wrote:
>> On Fri, Mar 24, 2017 at 10:46 AM, Shawn Guo <shawnguo@kernel.org> wrote:
>> > On Wed, Mar 22, 2017 at 11:18:10PM +0530, Jagan Teki wrote:
>> >> From: Jagan Teki <jagan@amarulasolutions.com>
>> >>
>> >> Add touchscreen node as i2c1 slave device on Engicam Is.IoT MX6UL
>> >> modules, the touchscreen controlled 'st,stmpe-ts' connected via
>> >> i2c with st,stmpe811 mfb interface.
>> >
>> > s/mfb/mfd?
>> >
>> >>
>> >> Cc: Shawn Guo <shawnguo@kernel.org>
>> >> Cc: Matteo Lisi <matteo.lisi@engicam.com>
>> >> Cc: Michael Trimarchi <michael@amarulasolutions.com>
>> >> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
>> >> ---
>> >>  arch/arm/boot/dts/imx6ul-isiot-common.dtsi | 34 ++++++++++++++++++++++++++++++
>> >>  1 file changed, 34 insertions(+)
>> >>
>> >> diff --git a/arch/arm/boot/dts/imx6ul-isiot-common.dtsi b/arch/arm/boot/dts/imx6ul-isiot-common.dtsi
>> >> index 9660e20..b149b5e 100644
>> >> --- a/arch/arm/boot/dts/imx6ul-isiot-common.dtsi
>> >> +++ b/arch/arm/boot/dts/imx6ul-isiot-common.dtsi
>> >> @@ -40,6 +40,34 @@
>> >>   *     OTHER DEALINGS IN THE SOFTWARE.
>> >>   */
>> >>
>> >> +&i2c1 {
>> >> +     touch: stmpe811@44 {
>> >
>> > Label a MFD device 'touch'?  Also, the node name should be generic.  Not
>> > sure what should be used for MFD device though.
>>
>> May be I can use stmpe811@41 the existing imx6qdl-apalis.dtsi using
>> the similar notation.
>
> Per application notes of stmpe811 below, I would suggest you name it
> gpio-expander.
>
> www.st.com/resource/zh/datasheet/CD00186725.pdf

OK, will this be ok?

stmpe811: gpio-expander@44 {

>
>>
>> >
>> >> +             compatible = "st,stmpe811";
>> >> +             reg = <0x44>;
>> >> +             #address-cells = <1>;
>> >> +             #size-cells = <0>;
>> >> +             pinctrl-names = "default";
>> >> +             pinctrl-0 = <&pinctrl_stmpe_ts>;
>> >
>> > If this is for stmpe-ts, shouldn't be put into touchscreen child node?
>>
>> Actually this is interrupt pad, for the touch controller via stmpe so
>> I can rename pinctrl_touch_int.
>
> Or just pinctrl_stmpe.
>
>>
>> >
>> >> +             interrupt-parent = <&gpio1>;
>> >> +             interrupts = <18 IRQ_TYPE_EDGE_FALLING>;
>> >> +             interrupt-controller;
>> >> +             #interrupt-cells = <2>;
>> >> +
>> >> +             stmpe_touchscreen {
>> >
>> > 'stmpe: touchscreen {' might be better.
>>
>> stmpe_touchscreen {
>>
>> This is also compatible with existing dtsi files
>
> We do not need to keep compatibility with something not good.

thanks!
Shawn Guo March 24, 2017, 7:35 a.m. UTC | #5
On Fri, Mar 24, 2017 at 01:02:35PM +0530, Jagan Teki wrote:
> On Fri, Mar 24, 2017 at 12:55 PM, Shawn Guo <shawnguo@kernel.org> wrote:
> > On Fri, Mar 24, 2017 at 12:29:49PM +0530, Jagan Teki wrote:
> >> On Fri, Mar 24, 2017 at 10:46 AM, Shawn Guo <shawnguo@kernel.org> wrote:
> >> > On Wed, Mar 22, 2017 at 11:18:10PM +0530, Jagan Teki wrote:
> >> >> From: Jagan Teki <jagan@amarulasolutions.com>
> >> >>
> >> >> Add touchscreen node as i2c1 slave device on Engicam Is.IoT MX6UL
> >> >> modules, the touchscreen controlled 'st,stmpe-ts' connected via
> >> >> i2c with st,stmpe811 mfb interface.
> >> >
> >> > s/mfb/mfd?
> >> >
> >> >>
> >> >> Cc: Shawn Guo <shawnguo@kernel.org>
> >> >> Cc: Matteo Lisi <matteo.lisi@engicam.com>
> >> >> Cc: Michael Trimarchi <michael@amarulasolutions.com>
> >> >> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> >> >> ---
> >> >>  arch/arm/boot/dts/imx6ul-isiot-common.dtsi | 34 ++++++++++++++++++++++++++++++
> >> >>  1 file changed, 34 insertions(+)
> >> >>
> >> >> diff --git a/arch/arm/boot/dts/imx6ul-isiot-common.dtsi b/arch/arm/boot/dts/imx6ul-isiot-common.dtsi
> >> >> index 9660e20..b149b5e 100644
> >> >> --- a/arch/arm/boot/dts/imx6ul-isiot-common.dtsi
> >> >> +++ b/arch/arm/boot/dts/imx6ul-isiot-common.dtsi
> >> >> @@ -40,6 +40,34 @@
> >> >>   *     OTHER DEALINGS IN THE SOFTWARE.
> >> >>   */
> >> >>
> >> >> +&i2c1 {
> >> >> +     touch: stmpe811@44 {
> >> >
> >> > Label a MFD device 'touch'?  Also, the node name should be generic.  Not
> >> > sure what should be used for MFD device though.
> >>
> >> May be I can use stmpe811@41 the existing imx6qdl-apalis.dtsi using
> >> the similar notation.
> >
> > Per application notes of stmpe811 below, I would suggest you name it
> > gpio-expander.
> >
> > www.st.com/resource/zh/datasheet/CD00186725.pdf
> 
> OK, will this be ok?
> 
> stmpe811: gpio-expander@44 {

Yes.

Shawn
diff mbox

Patch

diff --git a/arch/arm/boot/dts/imx6ul-isiot-common.dtsi b/arch/arm/boot/dts/imx6ul-isiot-common.dtsi
index 9660e20..b149b5e 100644
--- a/arch/arm/boot/dts/imx6ul-isiot-common.dtsi
+++ b/arch/arm/boot/dts/imx6ul-isiot-common.dtsi
@@ -40,6 +40,34 @@ 
  *     OTHER DEALINGS IN THE SOFTWARE.
  */
 
+&i2c1 {
+	touch: stmpe811@44 {
+		compatible = "st,stmpe811";
+		reg = <0x44>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&pinctrl_stmpe_ts>;
+		interrupt-parent = <&gpio1>;
+		interrupts = <18 IRQ_TYPE_EDGE_FALLING>;
+		interrupt-controller;
+		#interrupt-cells = <2>;
+
+		stmpe_touchscreen {
+			compatible = "st,stmpe-ts";
+			st,sample-time = <4>;
+			st,mod-12b = <1>;
+			st,ref-sel = <0>;
+			st,adc-freq = <1>;
+			st,ave-ctrl = <1>;
+			st,touch-det-delay = <2>;
+			st,settling = <2>;
+			st,fraction-z = <7>;
+			st,i-drive = <1>;
+		};
+	};
+};
+
 &lcdif {
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_lcdif_dat
@@ -104,4 +132,10 @@ 
 			MX6UL_PAD_LCD_DATA17__LCDIF_DATA17  0x79
 		>;
 	};
+
+	pinctrl_stmpe_ts:  stmpe-tsgrp  {
+		fsl,pins = <
+			MX6UL_PAD_UART1_CTS_B__GPIO1_IO18 0x1b0b0
+		>;
+	};
 };