[PATCH v2] ARM: dts: sun5i: Enable touchscreen on Utoo P66

[PATCH v2] ARM: dts: sun5i: Enable touchscreen on Utoo P66

[Hans de Goede]

Add a node for the chipone-icn8318 touchscreen found on the Utoo P66 tablet.

Signed-off-by: Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
Changes in v2:
-Fix touchscreen node name
---
 arch/arm/boot/dts/sun5i-a13-utoo-p66.dts | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/arch/arm/boot/dts/sun5i-a13-utoo-p66.dts b/arch/arm/boot/dts/sun5i-a13-utoo-p66.dts
index 9108ecd..00bdf28 100644
--- a/arch/arm/boot/dts/sun5i-a13-utoo-p66.dts
+++ b/arch/arm/boot/dts/sun5i-a13-utoo-p66.dts
@@ -50,6 +50,7 @@
 #include "sunxi-common-regulators.dtsi"
 #include <dt-bindings/gpio/gpio.h>
 #include <dt-bindings/input/input.h>
+#include <dt-bindings/interrupt-controller/irq.h>
 #include <dt-bindings/pinctrl/sun4i-a10.h>
 
 / {
@@ -93,6 +94,20 @@
 	pinctrl-0 = <&i2c1_pins_a>;
 	status = "okay";
 
+	icn8318: touchscreen@40 {
+		compatible = "chipone,icn8318";
+		reg = <0x40>;
+		interrupt-parent = <&pio>;
+		interrupts = <9 IRQ_TYPE_EDGE_FALLING>; /* EINT9 (PG9) */
+		pinctrl-names = "default";
+		pinctrl-0 = <&ts_wake_pin_p66>;
+		wake-gpios = <&pio 1 3 GPIO_ACTIVE_HIGH>; /* PB3 */
+		touchscreen-size-x = <800>;
+		touchscreen-size-y = <480>;
+		touchscreen-inverted-x;
+		touchscreen-swapped-x-y;
+	};
+
 	pcf8563: rtc@51 {
 		compatible = "nxp,pcf8563";
 		reg = <0x51>;
@@ -172,6 +187,13 @@
 		allwinner,pull = <SUN4I_PINCTRL_PULL_UP>;
 	};
 
+	ts_wake_pin_p66: ts_wake_pin@0 {
+		allwinner,pins = "PB3";
+		allwinner,function = "gpio_out";
+		allwinner,drive = <SUN4I_PINCTRL_10_MA>;
+		allwinner,pull = <SUN4I_PINCTRL_NO_PULL>;
+	};
+
 	usb0_vbus_pin_a: usb0_vbus_pin@0 {
 		allwinner,pins = "PB4";
 		allwinner,function = "gpio_out";
-- 
2.3.4

Re: [PATCH v2] ARM: dts: sun5i: Enable touchscreen on Utoo P66

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 1443 bytes --]

Hi Hans,

On Wed, Apr 01, 2015 at 05:29:37PM +0200, Hans de Goede wrote:
> Add a node for the chipone-icn8318 touchscreen found on the Utoo P66 tablet.
> 
> Signed-off-by: Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> ---
> Changes in v2:
> -Fix touchscreen node name
> ---
>  arch/arm/boot/dts/sun5i-a13-utoo-p66.dts | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/sun5i-a13-utoo-p66.dts b/arch/arm/boot/dts/sun5i-a13-utoo-p66.dts
> index 9108ecd..00bdf28 100644
> --- a/arch/arm/boot/dts/sun5i-a13-utoo-p66.dts
> +++ b/arch/arm/boot/dts/sun5i-a13-utoo-p66.dts
> @@ -50,6 +50,7 @@
>  #include "sunxi-common-regulators.dtsi"
>  #include <dt-bindings/gpio/gpio.h>
>  #include <dt-bindings/input/input.h>
> +#include <dt-bindings/interrupt-controller/irq.h>
>  #include <dt-bindings/pinctrl/sun4i-a10.h>
>  
>  / {
> @@ -93,6 +94,20 @@
>  	pinctrl-0 = <&i2c1_pins_a>;
>  	status = "okay";
>  
> +	icn8318: touchscreen@40 {
> +		compatible = "chipone,icn8318";
> +		reg = <0x40>;
> +		interrupt-parent = <&pio>;
> +		interrupts = <9 IRQ_TYPE_EDGE_FALLING>; /* EINT9 (PG9) */

I don't think you sent a reply to this comment, but this should really
be in a pinctrl node so that no one else can request the pin and
change its muxing and it shows up in debugfs.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Re: [PATCH v2] ARM: dts: sun5i: Enable touchscreen on Utoo P66

[Hans de Goede]

Hi,

On 04-04-15 14:44, Maxime Ripard wrote:
> Hi Hans,
>
> On Wed, Apr 01, 2015 at 05:29:37PM +0200, Hans de Goede wrote:
>> Add a node for the chipone-icn8318 touchscreen found on the Utoo P66 tablet.
>>
>> Signed-off-by: Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>> ---
>> Changes in v2:
>> -Fix touchscreen node name
>> ---
>>   arch/arm/boot/dts/sun5i-a13-utoo-p66.dts | 22 ++++++++++++++++++++++
>>   1 file changed, 22 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/sun5i-a13-utoo-p66.dts b/arch/arm/boot/dts/sun5i-a13-utoo-p66.dts
>> index 9108ecd..00bdf28 100644
>> --- a/arch/arm/boot/dts/sun5i-a13-utoo-p66.dts
>> +++ b/arch/arm/boot/dts/sun5i-a13-utoo-p66.dts
>> @@ -50,6 +50,7 @@
>>   #include "sunxi-common-regulators.dtsi"
>>   #include <dt-bindings/gpio/gpio.h>
>>   #include <dt-bindings/input/input.h>
>> +#include <dt-bindings/interrupt-controller/irq.h>
>>   #include <dt-bindings/pinctrl/sun4i-a10.h>
>>
>>   / {
>> @@ -93,6 +94,20 @@
>>   	pinctrl-0 = <&i2c1_pins_a>;
>>   	status = "okay";
>>
>> +	icn8318: touchscreen@40 {
>> +		compatible = "chipone,icn8318";
>> +		reg = <0x40>;
>> +		interrupt-parent = <&pio>;
>> +		interrupts = <9 IRQ_TYPE_EDGE_FALLING>; /* EINT9 (PG9) */
>
> I don't think you sent a reply to this comment, but this should really
> be in a pinctrl node so that no one else can request the pin and
> change its muxing and it shows up in debugfs.

I did reply to that comment on v1 of this patch:

"""
That is not necessary when specifying a gpio interrupt through the
irq chip (rather then through a gpios property), the mux is automatically
set to irq by the kernel when requesting the irq.

Also note that we must use the interrupts notation here as this is specified
by the i2c client bindings. None of the other boards using i2c + oob irq
pins specifies a pinmux for the irq pin either.
"""

I've just double checked and I cannot (quickly) find any existing devicetree
files using interrupts = <foo> combined with pinctrl for i2c clients.

If you look in e.g. arch/arm/boot/dts/exynos4210-trats.dts and
arch/arm/boot/dts/exynos4412-trats2.dts you will see the same pattern of
using interrupts = <foo> pointing to a gpio without using pinctrl

And we already to the same for sdio oob interrupts in sun7i-a20-cubietruck.dts
and arch/arm/boot/dts/sun7i-a20-i12-tvbox.dts, so I vote for being consistent
here and not using pinctrl for the irq pin here either.

Regards,

Hans

Re: [PATCH v2] ARM: dts: sun5i: Enable touchscreen on Utoo P66

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 2911 bytes --]

On Sat, Apr 04, 2015 at 03:21:41PM +0200, Hans de Goede wrote:
> Hi,
> 
> On 04-04-15 14:44, Maxime Ripard wrote:
> >Hi Hans,
> >
> >On Wed, Apr 01, 2015 at 05:29:37PM +0200, Hans de Goede wrote:
> >>Add a node for the chipone-icn8318 touchscreen found on the Utoo P66 tablet.
> >>
> >>Signed-off-by: Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> >>---
> >>Changes in v2:
> >>-Fix touchscreen node name
> >>---
> >>  arch/arm/boot/dts/sun5i-a13-utoo-p66.dts | 22 ++++++++++++++++++++++
> >>  1 file changed, 22 insertions(+)
> >>
> >>diff --git a/arch/arm/boot/dts/sun5i-a13-utoo-p66.dts b/arch/arm/boot/dts/sun5i-a13-utoo-p66.dts
> >>index 9108ecd..00bdf28 100644
> >>--- a/arch/arm/boot/dts/sun5i-a13-utoo-p66.dts
> >>+++ b/arch/arm/boot/dts/sun5i-a13-utoo-p66.dts
> >>@@ -50,6 +50,7 @@
> >>  #include "sunxi-common-regulators.dtsi"
> >>  #include <dt-bindings/gpio/gpio.h>
> >>  #include <dt-bindings/input/input.h>
> >>+#include <dt-bindings/interrupt-controller/irq.h>
> >>  #include <dt-bindings/pinctrl/sun4i-a10.h>
> >>
> >>  / {
> >>@@ -93,6 +94,20 @@
> >>  	pinctrl-0 = <&i2c1_pins_a>;
> >>  	status = "okay";
> >>
> >>+	icn8318: touchscreen@40 {
> >>+		compatible = "chipone,icn8318";
> >>+		reg = <0x40>;
> >>+		interrupt-parent = <&pio>;
> >>+		interrupts = <9 IRQ_TYPE_EDGE_FALLING>; /* EINT9 (PG9) */
> >
> >I don't think you sent a reply to this comment, but this should really
> >be in a pinctrl node so that no one else can request the pin and
> >change its muxing and it shows up in debugfs.
> 
> I did reply to that comment on v1 of this patch:
> 
> """
> That is not necessary when specifying a gpio interrupt through the
> irq chip (rather then through a gpios property), the mux is automatically
> set to irq by the kernel when requesting the irq.
> 
> Also note that we must use the interrupts notation here as this is specified
> by the i2c client bindings. None of the other boards using i2c + oob irq
> pins specifies a pinmux for the irq pin either.
> """

I never received that mail, I guess we can blame Google Groups for
that.

> I've just double checked and I cannot (quickly) find any existing
> devicetree files using interrupts = <foo> combined with pinctrl for
> i2c clients.
> 
> If you look in e.g. arch/arm/boot/dts/exynos4210-trats.dts and
> arch/arm/boot/dts/exynos4412-trats2.dts you will see the same
> pattern of using interrupts = <foo> pointing to a gpio without using
> pinctrl
> 
> And we already to the same for sdio oob interrupts in
> sun7i-a20-cubietruck.dts and
> arch/arm/boot/dts/sun7i-a20-i12-tvbox.dts, so I vote for being
> consistent here and not using pinctrl for the irq pin here either.

That might turn out to be an issue, but ok.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v2] ARM: dts: sun5i: Enable touchscreen on Utoo P66

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 359 bytes --]

On Wed, Apr 01, 2015 at 05:29:37PM +0200, Hans de Goede wrote:
> Add a node for the chipone-icn8318 touchscreen found on the Utoo P66 tablet.
> 
> Signed-off-by: Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

Merged, thanks!

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com