[PATCH] dt-bindings: input: Add the Goodix GTx5 series Touchscreen devicetree bindings

[PATCH] dt-bindings: input: Add the Goodix GTx5 series Touchscreen devicetree bindings

[Wang Yafei]

Signed-off-by: Wang Yafei <wangyafei@goodix.com>
---
 .../bindings/input/touchscreen/goodix-gtx5.txt     | 75 ++++++++++++++++++++++
 1 file changed, 75 insertions(+)
 create mode 100755 Documentation/devicetree/bindings/input/touchscreen/goodix-gtx5.txt

diff --git a/Documentation/devicetree/bindings/input/touchscreen/goodix-gtx5.txt b/Documentation/devicetree/bindings/input/touchscreen/goodix-gtx5.txt
new file mode 100755
index 0000000..116c3ec
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/touchscreen/goodix-gtx5.txt
@@ -0,0 +1,75 @@
+Device tree bindings for Goodix GTx5 series touchscreen controller
+
+Required properties:
+
+- compatible	: should be "goodix,gtx5" or "goodix,gsx" 
+
+- reg	: I2C address of the chip. Should be 0x5d or 0x14
+- interrupt-parent	: Inerrupt controller to which the chip is connected
+- interrupts	: Interrrupt to which the chip is connected
+- touchscreen-size-x	: horizontal resolution of touchscreen          
+                                  (in pixels)                                   
+- touchscreen-size-y	: vertical resolution of touchscreen            
+                                  (in pixels)                                  
+- touchscreen-max-id	: panel supported max touch number.
+- touchscreen-max-w	: panel max width value.
+
+
+Optional properties:
+- reset-gpios	: reset gpio.
+- irq-gpios	: interrupt gpio. 
+- irq-flags	: irq trigger type config, value should be:
+                       1 - rising edge,
+                       2 - falling edge,
+                       4 - high level,
+                       5 - low level.
+- touchscreen-swapped-x-y: swap  x/y axis coordinates.
+- touchscreen-key-map: keycode value map  /*KEY_HOMEPAGE, KEY_BACK*/
+- power-on-delay-us: delay after power on.
+- power-off-delay-us: delay after power off.
+- normal-cfg: touch device normal config data.
+- vtouch-supply	: power supply for the touch device.
+Example:
+i2c@00000000 {
+	/* ... */
+
+	goodix-ts-i2c@14 {
+		compatible = "goodix,gtx5";
+		reg = <0x14>;
+		interrupt-parent = <&msm_gpio>;
+		interrupts = <13 0x2800>;
+		vtouch-supply = <&pm8916_l15>;
+		reset-gpios = <&msm_gpio 12 0x0>;
+		irq-gpios = <&msm_gpio 13 0x2800>;
+		irq-flags = <1>; /* 1:trigger rising, 2:trigger falling;*/
+		touchscreen-max-id = <10>;
+		touchscreen-size-x = <400>;
+		touchscreen-size-y = <400>;
+		touchscreen-max-w = <400>;
+		touchscreen-max-pressure = <255>;
+		touchscreen-swapped-x-y;
+		touchscreen-key-map = <172 158>; /*KEY_HOMEPAGE, KEY_BACK*/
+		sensor0 {
+			normal-cfg = [
+				02 00 00 09 09 01 07 02 00 00 00 00 01 00 3C 00 07 07
+				00 00 00 00 00 00 40 01 40 01 C8 00 96 00 F4 01 F4 01
+				F4 01 20 01 11 0A 0A 03 14 14 14 14 0A 0C 01 01 11 11
+				11 00 14 14 14 14 14 14 14 14 14 00 00 0F 00 00 00 00
+				00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
+				00 00 00 00 00 00 00 00 00 00 11 09 10 00 31 32 33 34
+				00 00 00 00 00 00 00 00 00 00 00 00 00 01 00 CA 64 00
+				00 00 00 00 00 00 09 00 13 00 00 00 00 00 00 00 00 00
+				50 B0 19 00 19 00 05 00 00 00 00 0A 05 00 00 00 00 00
+				01 00 FF 00 0B 06 0D 02 FF 04 05 03 07 01 08 0A 0E 11
+				0F 10 09 13 0C 16 17 14 18 12 19 15 1D 1E 1C 1F 1B 20
+				1A 2A 29 28 25 2B 27 21 FF 24 22 2C 26 23 FF 00 00 00
+				00 00 00 00 00 00 00 00 00 00 00 00 80 80 80 80 80 80
+				80 80 80 80 80 80 80 80 80 80 9F 22 01 AE];
+		};
+		sensor1 {
+			normal-cfg = [ ];
+		};
+	};
+}
+
+
-- 
2.7.4

Re: [PATCH] dt-bindings: input: Add the Goodix GTx5 series Touchscreen devicetree bindings

[Bastien Nocera]

On Mon, 2017-06-19 at 18:40 +0800, Wang Yafei wrote:
> Signed-off-by: Wang Yafei <wangyafei-PiwChjOeyQzQT0dZR+AlfA@public.gmane.org>
> ---
>  .../bindings/input/touchscreen/goodix-gtx5.txt     | 75
> ++++++++++++++++++++++

Where was the driver sent? I didn't see it on linux-input. Why wasn't
this support added to the goodix driver instead?
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] dt-bindings: input: Add the Goodix GTx5 series Touchscreen devicetree bindings

[Wang Yafei]

On Tue, Jun 20, 2017 at 12:25:09AM +0200, Bastien Nocera wrote:
> On Mon, 2017-06-19 at 18:40 +0800, Wang Yafei wrote:
> > Signed-off-by: Wang Yafei <wangyafei@goodix.com>
> > ---
> >  .../bindings/input/touchscreen/goodix-gtx5.txt     | 75
> > ++++++++++++++++++++++
> 
> Where was the driver sent? I didn't see it on linux-input. Why wasn't
> this support added to the goodix driver instead?

Hi, Bastien,
I'm sorry for confused you. I have posted two patches one is touchscreen
driver another is this dt-bindings. The driver is under reviewing and hasn't
been applied to linux-input. Finally thanks for you advice I'll try add a
patch in ./bindings/input/touchscreen/goodix.txt rather than add a new file.

Re: [PATCH] dt-bindings: input: Add the Goodix GTx5 series Touchscreen devicetree bindings

[Bastien Nocera]

On Tue, 2017-06-20 at 16:53 +0800, Wang Yafei wrote:
> On Tue, Jun 20, 2017 at 12:25:09AM +0200, Bastien Nocera wrote:
> > On Mon, 2017-06-19 at 18:40 +0800, Wang Yafei wrote:
> > > Signed-off-by: Wang Yafei <wangyafei@goodix.com>
> > > ---
> > >  .../bindings/input/touchscreen/goodix-gtx5.txt     | 75
> > > ++++++++++++++++++++++
> > 
> > Where was the driver sent? I didn't see it on linux-input. Why
> > wasn't
> > this support added to the goodix driver instead?
> 
> Hi, Bastien,
> I'm sorry for confused you. I have posted two patches one is
> touchscreen
> driver another is this dt-bindings. The driver is under reviewing and
> hasn't
> been applied to linux-input.

But where's the driver? It should have been posted to the linux-input
mailing-list, and I cannot see it there.

>  Finally thanks for you advice I'll try add a
> patch in ./bindings/input/touchscreen/goodix.txt rather than add a
> new file.

That's not what I meant. I wonder about the reason why the existing
driver wasn't extended rather than writing a new one.

Good to see that Goodix is finally upstreaming its drivers though, my
attempts at contacting some of your colleagues fell on deaf ears a
couple of years ago.

Cheers

Re: [PATCH] dt-bindings: input: Add the Goodix GTx5 series Touchscreen devicetree bindings

[Wang Yafei]

On Tue, Jun 20, 2017 at 11:17:50AM +0200, Bastien Nocera wrote:
> On Tue, 2017-06-20 at 16:53 +0800, Wang Yafei wrote:
> > On Tue, Jun 20, 2017 at 12:25:09AM +0200, Bastien Nocera wrote:
> > > On Mon, 2017-06-19 at 18:40 +0800, Wang Yafei wrote:
> > > > Signed-off-by: Wang Yafei <wangyafei-PiwChjOeyQzQT0dZR+AlfA@public.gmane.org>
> > > > ---
> > > >  .../bindings/input/touchscreen/goodix-gtx5.txt     | 75
> > > > ++++++++++++++++++++++
> > > 
> > > Where was the driver sent? I didn't see it on linux-input. Why
> > > wasn't
> > > this support added to the goodix driver instead?
> > 
> > Hi, Bastien,
> > I'm sorry for confused you. I have posted two patches one is
> > touchscreen
> > driver another is this dt-bindings. The driver is under reviewing and
> > hasn't
> > been applied to linux-input.
> 
> But where's the driver? It should have been posted to the linux-input
> mailing-list, and I cannot see it there.

Forgive my carelessness, This is my first time posted patches to kernel,
I'm not sure which mailbox to send my patches, so I Cc the
linux-input-u79uwXL29TY76Z2rM5mHXA@public.gmane.org and linux-kernel-u79uwXL29TaiAVqoAR/hOA@public.gmane.org And as a
result you can find the maillist only in "LKML".

> >  Finally thanks for you advice I'll try add a
> > patch in ./bindings/input/touchscreen/goodix.txt rather than add a
> > new file.
> 
> That's not what I meant. I wonder about the reason why the existing
> driver wasn't extended rather than writing a new one.

Goodix driver that the kernel already have, is for our gt9xx series
touchscreen controllers. This new driver is for out new generation
touchscreen controllers, consider this two generations are very
different with each other. We wrote a new driver instead of make
a patch on the old driver.

> Good to see that Goodix is finally upstreaming its drivers though, my
> attempts at contacting some of your colleagues fell on deaf ears a
> couple of years ago.

Thank you very mouch for your attention, And very sorry for the negligence.
I believe in the future we will pay more and more attention to keep the
the Goodix driver up to date.

Sincerely

> Cheers
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] dt-bindings: input: Add the Goodix GTx5 series Touchscreen devicetree bindings

[Rob Herring]

On Mon, Jun 19, 2017 at 06:40:32PM +0800, Wang Yafei wrote:
> Signed-off-by: Wang Yafei <wangyafei-PiwChjOeyQzQT0dZR+AlfA@public.gmane.org>
> ---
>  .../bindings/input/touchscreen/goodix-gtx5.txt     | 75 ++++++++++++++++++++++
>  1 file changed, 75 insertions(+)
>  create mode 100755 Documentation/devicetree/bindings/input/touchscreen/goodix-gtx5.txt
> 
> diff --git a/Documentation/devicetree/bindings/input/touchscreen/goodix-gtx5.txt b/Documentation/devicetree/bindings/input/touchscreen/goodix-gtx5.txt
> new file mode 100755
> index 0000000..116c3ec
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/touchscreen/goodix-gtx5.txt
> @@ -0,0 +1,75 @@
> +Device tree bindings for Goodix GTx5 series touchscreen controller
> +
> +Required properties:
> +
> +- compatible	: should be "goodix,gtx5" or "goodix,gsx" 
> +
> +- reg	: I2C address of the chip. Should be 0x5d or 0x14
> +- interrupt-parent	: Inerrupt controller to which the chip is connected
> +- interrupts	: Interrrupt to which the chip is connected
> +- touchscreen-size-x	: horizontal resolution of touchscreen          
> +                                  (in pixels)                                   
> +- touchscreen-size-y	: vertical resolution of touchscreen            
> +                                  (in pixels)                                  
> +- touchscreen-max-id	: panel supported max touch number.
> +- touchscreen-max-w	: panel max width value.

Just "See touchscreen.txt" for all of these common props.

> +
> +
> +Optional properties:
> +- reset-gpios	: reset gpio.
> +- irq-gpios	: interrupt gpio. 

Use interrupts binding.

> +- irq-flags	: irq trigger type config, value should be:
> +                       1 - rising edge,
> +                       2 - falling edge,
> +                       4 - high level,
> +                       5 - low level.

Part of the flags for interrupt cells.

> +- touchscreen-swapped-x-y: swap  x/y axis coordinates.

"See touchscreen.txt"

> +- touchscreen-key-map: keycode value map  /*KEY_HOMEPAGE, KEY_BACK*/

This hardly seems sufficient to map screen areas to keys. There's been 
some discussion about doing that in the past.

> +- power-on-delay-us: delay after power on.
> +- power-off-delay-us: delay after power off.

These should be implied by the compatible. If not, you need more 
specific compatible strings.

> +- normal-cfg: touch device normal config data.

There's no mention these are a sub-node sensorX.

Needs a vendor prefix too.

> +- vtouch-supply	: power supply for the touch device.
> +Example:
> +i2c@00000000 {
> +	/* ... */
> +
> +	goodix-ts-i2c@14 {

touchscreen@14

> +		compatible = "goodix,gtx5";
> +		reg = <0x14>;
> +		interrupt-parent = <&msm_gpio>;
> +		interrupts = <13 0x2800>;
> +		vtouch-supply = <&pm8916_l15>;
> +		reset-gpios = <&msm_gpio 12 0x0>;
> +		irq-gpios = <&msm_gpio 13 0x2800>;
> +		irq-flags = <1>; /* 1:trigger rising, 2:trigger falling;*/
> +		touchscreen-max-id = <10>;
> +		touchscreen-size-x = <400>;
> +		touchscreen-size-y = <400>;
> +		touchscreen-max-w = <400>;
> +		touchscreen-max-pressure = <255>;
> +		touchscreen-swapped-x-y;
> +		touchscreen-key-map = <172 158>; /*KEY_HOMEPAGE, KEY_BACK*/
> +		sensor0 {

How many sensors can there be? That needs to be documented.

Why do you need a sub-node?

> +			normal-cfg = [
> +				02 00 00 09 09 01 07 02 00 00 00 00 01 00 3C 00 07 07
> +				00 00 00 00 00 00 40 01 40 01 C8 00 96 00 F4 01 F4 01
> +				F4 01 20 01 11 0A 0A 03 14 14 14 14 0A 0C 01 01 11 11
> +				11 00 14 14 14 14 14 14 14 14 14 00 00 0F 00 00 00 00
> +				00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> +				00 00 00 00 00 00 00 00 00 00 11 09 10 00 31 32 33 34
> +				00 00 00 00 00 00 00 00 00 00 00 00 00 01 00 CA 64 00
> +				00 00 00 00 00 00 09 00 13 00 00 00 00 00 00 00 00 00
> +				50 B0 19 00 19 00 05 00 00 00 00 0A 05 00 00 00 00 00
> +				01 00 FF 00 0B 06 0D 02 FF 04 05 03 07 01 08 0A 0E 11
> +				0F 10 09 13 0C 16 17 14 18 12 19 15 1D 1E 1C 1F 1B 20
> +				1A 2A 29 28 25 2B 27 21 FF 24 22 2C 26 23 FF 00 00 00
> +				00 00 00 00 00 00 00 00 00 00 00 00 80 80 80 80 80 80
> +				80 80 80 80 80 80 80 80 80 80 9F 22 01 AE];
> +		};
> +		sensor1 {
> +			normal-cfg = [ ];
> +		};
> +	};
> +}
> +
> +
> -- 
> 2.7.4
> 
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] dt-bindings: input: Add the Goodix GTx5 series Touchscreen devicetree bindings

[Wang Yafei]

Hi Rob,

Thank you for the review.

On Fri, Jun 23, 2017 at 03:37:06PM -0500, Rob Herring wrote:
> On Mon, Jun 19, 2017 at 06:40:32PM +0800, Wang Yafei wrote:
> > Signed-off-by: Wang Yafei <wangyafei-PiwChjOeyQzQT0dZR+AlfA@public.gmane.org>
> > ---
> >  .../bindings/input/touchscreen/goodix-gtx5.txt     | 75 ++++++++++++++++++++++
> >  1 file changed, 75 insertions(+)
> >  create mode 100755 Documentation/devicetree/bindings/input/touchscreen/goodix-gtx5.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/input/touchscreen/goodix-gtx5.txt b/Documentation/devicetree/bindings/input/touchscreen/goodix-gtx5.txt
> > new file mode 100755
> > index 0000000..116c3ec
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/input/touchscreen/goodix-gtx5.txt
> > @@ -0,0 +1,75 @@
> > +Device tree bindings for Goodix GTx5 series touchscreen controller
> > +
> > +Required properties:
> > +
> > +- compatible	: should be "goodix,gtx5" or "goodix,gsx" 
> > +
> > +- reg	: I2C address of the chip. Should be 0x5d or 0x14
> > +- interrupt-parent	: Inerrupt controller to which the chip is connected
> > +- interrupts	: Interrrupt to which the chip is connected
> > +- touchscreen-size-x	: horizontal resolution of touchscreen          
> > +                                  (in pixels)                                   
> > +- touchscreen-size-y	: vertical resolution of touchscreen            
> > +                                  (in pixels)                                  
> > +- touchscreen-max-id	: panel supported max touch number.
> > +- touchscreen-max-w	: panel max width value.
> 
> Just "See touchscreen.txt" for all of these common props.

Okay, I have noticed that.

> > +
> > +
> > +Optional properties:
> > +- reset-gpios	: reset gpio.
> > +- irq-gpios	: interrupt gpio. 
> 
> Use interrupts binding.
> 
> > +- irq-flags	: irq trigger type config, value should be:
> > +                       1 - rising edge,
> > +                       2 - falling edge,
> > +                       4 - high level,
> > +                       5 - low level.
> 
> Part of the flags for interrupt cells.

Okay, I will remove this property and use the flowing property:
interrupts =  <13 IRQ_TYPE_EDGE_FALLING>;

> > +- touchscreen-swapped-x-y: swap  x/y axis coordinates.
> 
> "See touchscreen.txt"
> 
> > +- touchscreen-key-map: keycode value map  /*KEY_HOMEPAGE, KEY_BACK*/
> 
> This hardly seems sufficient to map screen areas to keys. There's been 
> some discussion about doing that in the past.

Sorry haven't notice that before, I will change it to the flowing
property format:
linux,keycodes = <KEY_HOMEPAGE>, <KEY_MENU>, <KEY_BACK>;

> > +- power-on-delay-us: delay after power on.
> > +- power-off-delay-us: delay after power off.
> 
> These should be implied by the compatible. If not, you need more 
> specific compatible strings.

Okay, I will remove this props from DT.

> > +- normal-cfg: touch device normal config data.
> 
> There's no mention these are a sub-node sensorX.
> 
> Needs a vendor prefix too.
> 
> > +- vtouch-supply	: power supply for the touch device.
> > +Example:
> > +i2c@00000000 {
> > +	/* ... */
> > +
> > +	goodix-ts-i2c@14 {
> 
> touchscreen@14
> 
> > +		compatible = "goodix,gtx5";
> > +		reg = <0x14>;
> > +		interrupt-parent = <&msm_gpio>;
> > +		interrupts = <13 0x2800>;
> > +		vtouch-supply = <&pm8916_l15>;
> > +		reset-gpios = <&msm_gpio 12 0x0>;
> > +		irq-gpios = <&msm_gpio 13 0x2800>;
> > +		irq-flags = <1>; /* 1:trigger rising, 2:trigger falling;*/
> > +		touchscreen-max-id = <10>;
> > +		touchscreen-size-x = <400>;
> > +		touchscreen-size-y = <400>;
> > +		touchscreen-max-w = <400>;
> > +		touchscreen-max-pressure = <255>;
> > +		touchscreen-swapped-x-y;
> > +		touchscreen-key-map = <172 158>; /*KEY_HOMEPAGE, KEY_BACK*/
> > +		sensor0 {
> 
> How many sensors can there be? That needs to be documented.
> 
> Why do you need a sub-node?

sensor is an internal call, we use senesorX to represent the touchpanel
is produced by which factory. A panel factory will have it corresponding
config data to get the best touch results. It will delete the sub-node 
and change to the following format:
	panel0,normal-cfg = [...]  /* panelX,cfg : X is no more than 10*/
 
> > +			normal-cfg = [
> > +				02 00 00 09 09 01 07 02 00 00 00 00 01 00 3C 00 07 07
> > +				00 00 00 00 00 00 40 01 40 01 C8 00 96 00 F4 01 F4 01
> > +				F4 01 20 01 11 0A 0A 03 14 14 14 14 0A 0C 01 01 11 11
> > +				11 00 14 14 14 14 14 14 14 14 14 00 00 0F 00 00 00 00
> > +				00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> > +				00 00 00 00 00 00 00 00 00 00 11 09 10 00 31 32 33 34
> > +				00 00 00 00 00 00 00 00 00 00 00 00 00 01 00 CA 64 00
> > +				00 00 00 00 00 00 09 00 13 00 00 00 00 00 00 00 00 00
> > +				50 B0 19 00 19 00 05 00 00 00 00 0A 05 00 00 00 00 00
> > +				01 00 FF 00 0B 06 0D 02 FF 04 05 03 07 01 08 0A 0E 11
> > +				0F 10 09 13 0C 16 17 14 18 12 19 15 1D 1E 1C 1F 1B 20
> > +				1A 2A 29 28 25 2B 27 21 FF 24 22 2C 26 23 FF 00 00 00
> > +				00 00 00 00 00 00 00 00 00 00 00 00 80 80 80 80 80 80
> > +				80 80 80 80 80 80 80 80 80 80 9F 22 01 AE];
> > +		};
> > +		sensor1 {
> > +			normal-cfg = [ ];
> > +		};
> > +	};
> > +}
> > +
> > +
> > -- 
> > 2.7.4
> > 

Sincerely.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] dt-bindings: input: Add the Goodix GTx5 series Touchscreen devicetree bindings

[Rob Herring]

On Mon, Jun 26, 2017 at 8:22 AM, Wang Yafei <wangyafei-PiwChjOeyQzQT0dZR+AlfA@public.gmane.org> wrote:
> Hi Rob,
>
> Thank you for the review.
>
> On Fri, Jun 23, 2017 at 03:37:06PM -0500, Rob Herring wrote:
>> On Mon, Jun 19, 2017 at 06:40:32PM +0800, Wang Yafei wrote:
>> > Signed-off-by: Wang Yafei <wangyafei-PiwChjOeyQzQT0dZR+AlfA@public.gmane.org>
>> > ---
>> >  .../bindings/input/touchscreen/goodix-gtx5.txt     | 75 ++++++++++++++++++++++
>> >  1 file changed, 75 insertions(+)
>> >  create mode 100755 Documentation/devicetree/bindings/input/touchscreen/goodix-gtx5.txt

[...]

>> > +- touchscreen-key-map: keycode value map  /*KEY_HOMEPAGE, KEY_BACK*/
>>
>> This hardly seems sufficient to map screen areas to keys. There's been
>> some discussion about doing that in the past.
>
> Sorry haven't notice that before, I will change it to the flowing
> property format:
> linux,keycodes = <KEY_HOMEPAGE>, <KEY_MENU>, <KEY_BACK>;

That's better, but not really what I meant. There's still some
assumption about what region each key is located in. Is every design 3
keys? How do you know what region of the touchscreen is a given key?
iOW, what are the valid ranges of coordinates for each key?


>> > +           compatible = "goodix,gtx5";
>> > +           reg = <0x14>;
>> > +           interrupt-parent = <&msm_gpio>;
>> > +           interrupts = <13 0x2800>;
>> > +           vtouch-supply = <&pm8916_l15>;
>> > +           reset-gpios = <&msm_gpio 12 0x0>;
>> > +           irq-gpios = <&msm_gpio 13 0x2800>;
>> > +           irq-flags = <1>; /* 1:trigger rising, 2:trigger falling;*/
>> > +           touchscreen-max-id = <10>;
>> > +           touchscreen-size-x = <400>;
>> > +           touchscreen-size-y = <400>;
>> > +           touchscreen-max-w = <400>;
>> > +           touchscreen-max-pressure = <255>;
>> > +           touchscreen-swapped-x-y;
>> > +           touchscreen-key-map = <172 158>; /*KEY_HOMEPAGE, KEY_BACK*/
>> > +           sensor0 {
>>
>> How many sensors can there be? That needs to be documented.
>>
>> Why do you need a sub-node?
>
> sensor is an internal call, we use senesorX to represent the touchpanel
> is produced by which factory. A panel factory will have it corresponding
> config data to get the best touch results. It will delete the sub-node
> and change to the following format:
>         panel0,normal-cfg = [...]  /* panelX,cfg : X is no more than 10*/

panel0 is not a vendor and normal doesn't tell us anything. How about
just "goodix,panel-cfg-X"?

Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] dt-bindings: input: Add the Goodix GTx5 series Touchscreen devicetree bindings

[Wang Yafei]

On Mon, Jun 26, 2017 at 09:16:40AM -0500, Rob Herring wrote:
> On Mon, Jun 26, 2017 at 8:22 AM, Wang Yafei <wangyafei-PiwChjOeyQzQT0dZR+AlfA@public.gmane.org> wrote:
> > Hi Rob,
> >
> > Thank you for the review.
> >
> > On Fri, Jun 23, 2017 at 03:37:06PM -0500, Rob Herring wrote:
> >> On Mon, Jun 19, 2017 at 06:40:32PM +0800, Wang Yafei wrote:
> >> > Signed-off-by: Wang Yafei <wangyafei-PiwChjOeyQzQT0dZR+AlfA@public.gmane.org>
> >> > ---
> >> >  .../bindings/input/touchscreen/goodix-gtx5.txt     | 75 ++++++++++++++++++++++
> >> >  1 file changed, 75 insertions(+)
> >> >  create mode 100755 Documentation/devicetree/bindings/input/touchscreen/goodix-gtx5.txt
> 
> [...]
> 
> >> > +- touchscreen-key-map: keycode value map  /*KEY_HOMEPAGE, KEY_BACK*/
> >>
> >> This hardly seems sufficient to map screen areas to keys. There's been
> >> some discussion about doing that in the past.
> >
> > Sorry haven't notice that before, I will change it to the flowing
> > property format:
> > linux,keycodes = <KEY_HOMEPAGE>, <KEY_MENU>, <KEY_BACK>;
> 
> That's better, but not really what I meant. There's still some
> assumption about what region each key is located in. Is every design 3
> keys? How do you know what region of the touchscreen is a given key?
> iOW, what are the valid ranges of coordinates for each key?

I'm afraid it hard to show each key located in. The location of touch
key is determined by touch panel hardware design and the touchscreen
controller configuration data. In general, there are two different
implementations of touch key, one is specified some area on the top of
LCD, another is on the out side of LCD and will print some pattern to
indicate the key info. No matter what kind kind of implementation, the
touch controller will report a specialized key event to driver different
from coordinate events, and the driver no need to care about the rages of
coordinates. When receive a key event the only thing that driver need to
do is map it to a certain linux keycode and report to OS. The
corresponding linux keycodes of each key is determined by this device
property. The key number in not immutable, for example on some x86 tablets
there only have one home button with windows logo on the front. The key
number is determined by touch controller firmware usually no more than
5.

> 
> >> > +           compatible = "goodix,gtx5";
> >> > +           reg = <0x14>;
> >> > +           interrupt-parent = <&msm_gpio>;
> >> > +           interrupts = <13 0x2800>;
> >> > +           vtouch-supply = <&pm8916_l15>;
> >> > +           reset-gpios = <&msm_gpio 12 0x0>;
> >> > +           irq-gpios = <&msm_gpio 13 0x2800>;
> >> > +           irq-flags = <1>; /* 1:trigger rising, 2:trigger falling;*/
> >> > +           touchscreen-max-id = <10>;
> >> > +           touchscreen-size-x = <400>;
> >> > +           touchscreen-size-y = <400>;
> >> > +           touchscreen-max-w = <400>;
> >> > +           touchscreen-max-pressure = <255>;
> >> > +           touchscreen-swapped-x-y;
> >> > +           touchscreen-key-map = <172 158>; /*KEY_HOMEPAGE, KEY_BACK*/
> >> > +           sensor0 {
> >>
> >> How many sensors can there be? That needs to be documented.
> >>
> >> Why do you need a sub-node?
> >
> > sensor is an internal call, we use senesorX to represent the touchpanel
> > is produced by which factory. A panel factory will have it corresponding
> > config data to get the best touch results. It will delete the sub-node
> > and change to the following format:
> >         panel0,normal-cfg = [...]  /* panelX,cfg : X is no more than 10*/
> 
> panel0 is not a vendor and normal doesn't tell us anything. How about
> just "goodix,panel-cfg-X"?

Okay, goodix,panel-cfg-X, seems more reasonable, I will use this.

Sincerely.

> Rob
> --
> To unsubscribe from this list: send the line "unsubscribe linux-input" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] dt-bindings: input: Add the Goodix GTx5 series Touchscreen devicetree bindings

[Wang Yafei]

On Mon, Jun 26, 2017 at 09:16:40AM -0500, Rob Herring wrote:
> On Mon, Jun 26, 2017 at 8:22 AM, Wang Yafei <wangyafei-PiwChjOeyQzQT0dZR+AlfA@public.gmane.org> wrote:
> > Hi Rob,
> >
> > Thank you for the review.
> >
> > On Fri, Jun 23, 2017 at 03:37:06PM -0500, Rob Herring wrote:
> >> On Mon, Jun 19, 2017 at 06:40:32PM +0800, Wang Yafei wrote:
> >> > Signed-off-by: Wang Yafei <wangyafei-PiwChjOeyQzQT0dZR+AlfA@public.gmane.org>
> >> > ---
> >> >  .../bindings/input/touchscreen/goodix-gtx5.txt     | 75 ++++++++++++++++++++++
> >> >  1 file changed, 75 insertions(+)
> >> >  create mode 100755 Documentation/devicetree/bindings/input/touchscreen/goodix-gtx5.txt
> 
> [...]
> 
> >> > +- touchscreen-key-map: keycode value map  /*KEY_HOMEPAGE, KEY_BACK*/
> >>
> >> This hardly seems sufficient to map screen areas to keys. There's been
> >> some discussion about doing that in the past.
> >
> > Sorry haven't notice that before, I will change it to the flowing
> > property format:
> > linux,keycodes = <KEY_HOMEPAGE>, <KEY_MENU>, <KEY_BACK>;
> 
> That's better, but not really what I meant. There's still some
> assumption about what region each key is located in. Is every design 3
> keys? How do you know what region of the touchscreen is a given key?
> iOW, what are the valid ranges of coordinates for each key?
> 
> 
> >> > +           compatible = "goodix,gtx5";
> >> > +           reg = <0x14>;
> >> > +           interrupt-parent = <&msm_gpio>;
> >> > +           interrupts = <13 0x2800>;
> >> > +           vtouch-supply = <&pm8916_l15>;
> >> > +           reset-gpios = <&msm_gpio 12 0x0>;
> >> > +           irq-gpios = <&msm_gpio 13 0x2800>;
> >> > +           irq-flags = <1>; /* 1:trigger rising, 2:trigger falling;*/
> >> > +           touchscreen-max-id = <10>;
> >> > +           touchscreen-size-x = <400>;
> >> > +           touchscreen-size-y = <400>;
> >> > +           touchscreen-max-w = <400>;
> >> > +           touchscreen-max-pressure = <255>;
> >> > +           touchscreen-swapped-x-y;
> >> > +           touchscreen-key-map = <172 158>; /*KEY_HOMEPAGE, KEY_BACK*/
> >> > +           sensor0 {
> >>
> >> How many sensors can there be? That needs to be documented.
> >>
> >> Why do you need a sub-node?
> >
> > sensor is an internal call, we use senesorX to represent the touchpanel
> > is produced by which factory. A panel factory will have it corresponding
> > config data to get the best touch results. It will delete the sub-node
> > and change to the following format:
> >         panel0,normal-cfg = [...]  /* panelX,cfg : X is no more than 10*/
> 
> panel0 is not a vendor and normal doesn't tell us anything. How about
> just "goodix,panel-cfg-X"?

Can I use "goodix,normal-cfg-X" rather than "goodix,panel-cfg-X"? The
string "normal-cfg" is meaningful to driver.

> Rob
> --
> To unsubscribe from this list: send the line "unsubscribe linux-input" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] dt-bindings: input: Add the Goodix GTx5 series Touchscreen devicetree bindings

[Rob Herring]

On Mon, Jun 26, 2017 at 10:27 PM, Wang Yafei <wangyafei-PiwChjOeyQzQT0dZR+AlfA@public.gmane.org> wrote:
> On Mon, Jun 26, 2017 at 09:16:40AM -0500, Rob Herring wrote:
>> On Mon, Jun 26, 2017 at 8:22 AM, Wang Yafei <wangyafei-PiwChjOeyQzQT0dZR+AlfA@public.gmane.org> wrote:
>> > Hi Rob,
>> >
>> > Thank you for the review.
>> >
>> > On Fri, Jun 23, 2017 at 03:37:06PM -0500, Rob Herring wrote:
>> >> On Mon, Jun 19, 2017 at 06:40:32PM +0800, Wang Yafei wrote:
>> >> > Signed-off-by: Wang Yafei <wangyafei-PiwChjOeyQzQT0dZR+AlfA@public.gmane.org>
>> >> > ---
>> >> >  .../bindings/input/touchscreen/goodix-gtx5.txt     | 75 ++++++++++++++++++++++
>> >> >  1 file changed, 75 insertions(+)
>> >> >  create mode 100755 Documentation/devicetree/bindings/input/touchscreen/goodix-gtx5.txt
>>
>> [...]

>> >> > +           compatible = "goodix,gtx5";
>> >> > +           reg = <0x14>;
>> >> > +           interrupt-parent = <&msm_gpio>;
>> >> > +           interrupts = <13 0x2800>;
>> >> > +           vtouch-supply = <&pm8916_l15>;
>> >> > +           reset-gpios = <&msm_gpio 12 0x0>;
>> >> > +           irq-gpios = <&msm_gpio 13 0x2800>;
>> >> > +           irq-flags = <1>; /* 1:trigger rising, 2:trigger falling;*/
>> >> > +           touchscreen-max-id = <10>;
>> >> > +           touchscreen-size-x = <400>;
>> >> > +           touchscreen-size-y = <400>;
>> >> > +           touchscreen-max-w = <400>;
>> >> > +           touchscreen-max-pressure = <255>;
>> >> > +           touchscreen-swapped-x-y;
>> >> > +           touchscreen-key-map = <172 158>; /*KEY_HOMEPAGE, KEY_BACK*/
>> >> > +           sensor0 {
>> >>
>> >> How many sensors can there be? That needs to be documented.
>> >>
>> >> Why do you need a sub-node?
>> >
>> > sensor is an internal call, we use senesorX to represent the touchpanel
>> > is produced by which factory. A panel factory will have it corresponding
>> > config data to get the best touch results. It will delete the sub-node
>> > and change to the following format:
>> >         panel0,normal-cfg = [...]  /* panelX,cfg : X is no more than 10*/
>>
>> panel0 is not a vendor and normal doesn't tell us anything. How about
>> just "goodix,panel-cfg-X"?
>
> Can I use "goodix,normal-cfg-X" rather than "goodix,panel-cfg-X"? The
> string "normal-cfg" is meaningful to driver.

Okay.

Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html