[PATCH 0/2] Input: Add ektf2232 support

[PATCH 0/2] Input: Add ektf2232 support

[Andreas Kemnade]

Add support for the EKTF2232 to the ektf2127 driver which
contains support for similar chips.

Add the needed compatible to bindings and convert them.

Andreas Kemnade (2):
  dt-bindings: touchscreen: convert elan,ektf2127 to json-schema
  Input: ektf2127 - add ektf2232 support

 .../bindings/input/touchscreen/ektf2127.txt   | 25 --------
 .../input/touchscreen/elan,ektf2127.yaml      | 59 +++++++++++++++++++
 drivers/input/touchscreen/ektf2127.c          | 15 ++++-
 3 files changed, 72 insertions(+), 27 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/input/touchscreen/ektf2127.txt
 create mode 100644 Documentation/devicetree/bindings/input/touchscreen/elan,ektf2127.yaml

-- 
2.39.2

[PATCH 1/2] dt-bindings: touchscreen: convert elan,ektf2127 to json-schema

[Andreas Kemnade]

Convert EKTF2127 infrared touchscreen controller binding to DT schema
and add ektf2232 compatible.

Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
---
 .../bindings/input/touchscreen/ektf2127.txt   | 25 --------
 .../input/touchscreen/elan,ektf2127.yaml      | 59 +++++++++++++++++++
 2 files changed, 59 insertions(+), 25 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/input/touchscreen/ektf2127.txt
 create mode 100644 Documentation/devicetree/bindings/input/touchscreen/elan,ektf2127.yaml

diff --git a/Documentation/devicetree/bindings/input/touchscreen/ektf2127.txt b/Documentation/devicetree/bindings/input/touchscreen/ektf2127.txt
deleted file mode 100644
index c9f2c9f578e34..0000000000000
--- a/Documentation/devicetree/bindings/input/touchscreen/ektf2127.txt
+++ /dev/null
@@ -1,25 +0,0 @@
-* Elan eKTF2127 I2C touchscreen controller
-
-Required properties:
- - compatible		  : "elan,ektf2127" or "elan,ektf2132"
- - reg			  : I2C slave address of the chip (0x40)
- - interrupts		  : interrupt specification for the ektf2127 interrupt
- - power-gpios		  : GPIO specification for the pin connected to the
-			    ektf2127's wake input. This needs to be driven high
-			    to take ektf2127 out of its low power state
-
-For additional optional properties see: touchscreen.txt
-
-Example:
-
-i2c@00000000 {
-	ektf2127: touchscreen@15 {
-		compatible = "elan,ektf2127";
-		reg = <0x15>;
-		interrupt-parent = <&pio>;
-		interrupts = <6 11 IRQ_TYPE_EDGE_FALLING>
-		power-gpios = <&pio 1 3 GPIO_ACTIVE_HIGH>;
-		touchscreen-inverted-x;
-		touchscreen-swapped-x-y;
-	};
-};
diff --git a/Documentation/devicetree/bindings/input/touchscreen/elan,ektf2127.yaml b/Documentation/devicetree/bindings/input/touchscreen/elan,ektf2127.yaml
new file mode 100644
index 0000000000000..66cda99a2ddd0
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/touchscreen/elan,ektf2127.yaml
@@ -0,0 +1,59 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/input/touchscreen/elan,ektf2127.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Elan eKTF2127 I2C touchscreen controller
+
+maintainers:
+  - Siebren Vroegindeweij <siebren.vroegindeweij@hotmail.com>
+
+allOf:
+  - $ref: touchscreen.yaml#
+
+properties:
+  compatible:
+    enum:
+      - elan,ektf2127
+      - elan,ektf2132
+      - elan,ektf2232
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  power-gpios:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - power-gpios
+
+unevaluatedProperties: false
+
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+    #include <dt-bindings/interrupt-controller/irq.h>
+
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        touchscreen@15 {
+            compatible = "elan,ektf2127";
+            reg = <0x15>;
+            interrupt-parent = <&pio>;
+            interrupts = <6 11 IRQ_TYPE_EDGE_FALLING>;
+            power-gpios = <&pio 1 3 GPIO_ACTIVE_HIGH>;
+            touchscreen-inverted-x;
+            touchscreen-swapped-x-y;
+        };
+    };
+...
-- 
2.39.2

Re: [PATCH 1/2] dt-bindings: touchscreen: convert elan,ektf2127 to json-schema

[Krzysztof Kozlowski]

On 02/05/2024 20:58, Andreas Kemnade wrote:
> Convert EKTF2127 infrared touchscreen controller binding to DT schema
> and add ektf2232 compatible.

This should be two commits.

> 
> Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
> ---
>  .../bindings/input/touchscreen/ektf2127.txt   | 25 --------
>  .../input/touchscreen/elan,ektf2127.yaml      | 59 +++++++++++++++++++
>  2 files changed, 59 insertions(+), 25 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/input/touchscreen/ektf2127.txt
>  create mode 100644 Documentation/devicetree/bindings/input/touchscreen/elan,ektf2127.yaml
> 

...

> +allOf:
> +  - $ref: touchscreen.yaml#
> +
> +properties:
> +  compatible:
> +    enum:
> +      - elan,ektf2127
> +      - elan,ektf2132
> +      - elan,ektf2232
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  power-gpios:
> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - power-gpios
> +
> +unevaluatedProperties: false
> +
> +

Just one blank line.

> +examples:
> +  - |
> +    #include <dt-bindings/gpio/gpio.h>
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +


Best regards,
Krzysztof

[PATCH 2/2] Input: ektf2127 - add ektf2232 support

[Andreas Kemnade]

The chip is similar, but has status bits at different positions,
so use the correct bits.

Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
---
 drivers/input/touchscreen/ektf2127.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/input/touchscreen/ektf2127.c b/drivers/input/touchscreen/ektf2127.c
index cc3103b9cbfba..5c2a6bfc63d84 100644
--- a/drivers/input/touchscreen/ektf2127.c
+++ b/drivers/input/touchscreen/ektf2127.c
@@ -46,6 +46,7 @@ struct ektf2127_ts {
 	struct input_dev *input;
 	struct gpio_desc *power_gpios;
 	struct touchscreen_properties prop;
+	bool shifted_status;
 };
 
 static void ektf2127_parse_coordinates(const u8 *buf, unsigned int touch_count,
@@ -112,8 +113,13 @@ static void ektf2127_report2_contact(struct ektf2127_ts *ts, int slot,
 
 static void ektf2127_report2_event(struct ektf2127_ts *ts, const u8 *buf)
 {
-	ektf2127_report2_contact(ts, 0, &buf[1], !!(buf[7] & 2));
-	ektf2127_report2_contact(ts, 1, &buf[4], !!(buf[7] & 4));
+	if (ts->shifted_status) {
+		ektf2127_report2_contact(ts, 0, &buf[1], !!(buf[7] & 1));
+		ektf2127_report2_contact(ts, 1, &buf[4], !!(buf[7] & 2));
+	} else {
+		ektf2127_report2_contact(ts, 0, &buf[1], !!(buf[7] & 2));
+		ektf2127_report2_contact(ts, 1, &buf[4], !!(buf[7] & 4));
+	}
 
 	input_mt_sync_frame(ts->input);
 	input_sync(ts->input);
@@ -303,6 +309,10 @@ static int ektf2127_probe(struct i2c_client *client)
 		return error;
 
 	ts->input = input;
+	if (dev->of_node &&
+	    of_device_is_compatible(dev->of_node, "elan,ektf2232"))
+		ts->shifted_status = true;
+
 	input_set_drvdata(input, ts);
 
 	error = devm_request_threaded_irq(dev, client->irq,
@@ -329,6 +339,7 @@ static int ektf2127_probe(struct i2c_client *client)
 static const struct of_device_id ektf2127_of_match[] = {
 	{ .compatible = "elan,ektf2127" },
 	{ .compatible = "elan,ektf2132" },
+	{ .compatible = "elan,ektf2232" },
 	{}
 };
 MODULE_DEVICE_TABLE(of, ektf2127_of_match);
-- 
2.39.2

Re: [PATCH 2/2] Input: ektf2127 - add ektf2232 support

[Andy Shevchenko]

On Thu, May 2, 2024 at 9:58 PM Andreas Kemnade <andreas@kemnade.info> wrote:
>
> The chip is similar, but has status bits at different positions,
> so use the correct bits.

...

> +       if (ts->shifted_status) {
> +               ektf2127_report2_contact(ts, 0, &buf[1], !!(buf[7] & 1));
> +               ektf2127_report2_contact(ts, 1, &buf[4], !!(buf[7] & 2));

BIT(0)
BIT(1)

> +       } else {
> +               ektf2127_report2_contact(ts, 0, &buf[1], !!(buf[7] & 2));
> +               ektf2127_report2_contact(ts, 1, &buf[4], !!(buf[7] & 4));

BIT(1)
BIT(2)

> +       }

...

> +       if (dev->of_node &&
> +           of_device_is_compatible(dev->of_node, "elan,ektf2232"))

if (device_is_compatible(...))

> +               ts->shifted_status = true;

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 2/2] Input: ektf2127 - add ektf2232 support

[Dmitry Torokhov]

On Thu, May 02, 2024 at 10:16:01PM +0300, Andy Shevchenko wrote:
> On Thu, May 2, 2024 at 9:58 PM Andreas Kemnade <andreas@kemnade.info> wrote:
> >
> > The chip is similar, but has status bits at different positions,
> > so use the correct bits.
> 
> ...
> 
> > +       if (ts->shifted_status) {

Instead of the flag I think it would be better if you had
ts->status_shift and did

		status = buf[7] >> ts->status_shift;
		ektf2127_report2_contact(ts, 0, &buf[1], status & BIT(0));
		ektf2127_report2_contact(ts, 1, &buf[4], status & BIT(1));

> > +               ektf2127_report2_contact(ts, 0, &buf[1], !!(buf[7] & 1));
> > +               ektf2127_report2_contact(ts, 1, &buf[4], !!(buf[7] & 2));
> 
> BIT(0)
> BIT(1)
> 
> > +       } else {
> > +               ektf2127_report2_contact(ts, 0, &buf[1], !!(buf[7] & 2));
> > +               ektf2127_report2_contact(ts, 1, &buf[4], !!(buf[7] & 4));
> 
> BIT(1)
> BIT(2)
> 
> > +       }
> 
> ...
> 
> > +       if (dev->of_node &&
> > +           of_device_is_compatible(dev->of_node, "elan,ektf2232"))
> 
> if (device_is_compatible(...))

Actually I think this better come from data obtained via
device_get_match_data().

> 
> > +               ts->shifted_status = true;
> 

Thanks.

-- 
Dmitry

Re: [PATCH 2/2] Input: ektf2127 - add ektf2232 support

[Andy Shevchenko]

On Fri, May 3, 2024 at 2:10 AM Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> On Thu, May 02, 2024 at 10:16:01PM +0300, Andy Shevchenko wrote:
> > On Thu, May 2, 2024 at 9:58 PM Andreas Kemnade <andreas@kemnade.info> wrote:

...

> > > +       if (ts->shifted_status) {
>
> Instead of the flag I think it would be better if you had
> ts->status_shift and did
>
>                 status = buf[7] >> ts->status_shift;
>                 ektf2127_report2_contact(ts, 0, &buf[1], status & BIT(0));
>                 ektf2127_report2_contact(ts, 1, &buf[4], status & BIT(1));
>
> > > +               ektf2127_report2_contact(ts, 0, &buf[1], !!(buf[7] & 1));
> > > +               ektf2127_report2_contact(ts, 1, &buf[4], !!(buf[7] & 2));
> >
> > BIT(0)
> > BIT(1)
> >
> > > +       } else {
> > > +               ektf2127_report2_contact(ts, 0, &buf[1], !!(buf[7] & 2));
> > > +               ektf2127_report2_contact(ts, 1, &buf[4], !!(buf[7] & 4));
> >
> > BIT(1)
> > BIT(2)
> >
> > > +       }

...

> > > +       if (dev->of_node &&
> > > +           of_device_is_compatible(dev->of_node, "elan,ektf2232"))
> >
> > if (device_is_compatible(...))
>
> Actually I think this better come from data obtained via
> device_get_match_data().
>
> > > +               ts->shifted_status = true;

I agree with your comments. Hopefully the author as well.

-- 
With Best Regards,
Andy Shevchenko