[PATCH v1] Add support for the ektf2127 touchscreencontroller

[PATCH v1] Add support for the ektf2127 touchscreencontroller

[Siebren Vroegindeweij]

Hey All,

The driver as submitted today has been tested on an ARM tablet with an ektf2127 touchscreencontroller.
For instantiation it uses the information from the devicetree.
If there is no information about the resolution in the devictree it will use the
resolution as provided by the chip itself.

Please review and merge,

Michel Verlaan and Siebren Vroegindeweij

[PATCH v1] Add support for the ektf2127 touchscreencontroller

[Siebren Vroegindeweij]

From: Siebren Vroegindeweij <siebren.vroegindeweij@hotmail.com>

The ELAN eKTF2127 driver is written for the ELAN series of
touchscreencontrollers. This version is especially written for the
eKTF2127 controller. The driver communicates to the controller over i2c.
The additional screen specifications can be read from the devicetree file.
The driver has also the ability to read the screen height and width from
the controller. When the screen is pressed, a interrupt is generated.
The interrupt wil be handled by a function that request a data string from
the controller. This string is then modified so that the right number of touches
and their x and y coordinates are available and given to userspace through
the input subsystem.

Signed-off-by: Michel Verlaan <michel.verl@gmail.com>
Signed-off-by: Siebren Vroegindeweij <siebren.vroegindeweij@hotmail.com>
---
 .../bindings/input/touchscreen/ektf2127.txt        |  40 +++
 drivers/input/touchscreen/Kconfig                  |  11 +
 drivers/input/touchscreen/Makefile                 |   1 +
 drivers/input/touchscreen/ektf2127.c               | 351 +++++++++++++++++++++
 4 files changed, 403 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/input/touchscreen/ektf2127.txt
 create mode 100644 drivers/input/touchscreen/ektf2127.c

diff --git a/Documentation/devicetree/bindings/input/touchscreen/ektf2127.txt b/Documentation/devicetree/bindings/input/touchscreen/ektf2127.txt
new file mode 100644
index 0000000..a774336
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/touchscreen/ektf2127.txt
@@ -0,0 +1,40 @@
+* Elan eKTF2127 I2C touchscreen controller
+
+Required properties:
+ - compatible		  : "elan,ektf2127"
+ - reg			  : I2C slave address of the chip (0x40)
+ - interrupt-parent	  : a phandle pointing to the interrupt controller
+			    serving the interrupt for this chip
+ - interrupts		  : interrupt specification for the eKTF2127 interrupt
+ - wake-gpios		  : GPIO specification for the WAKE input
+
+Optional properties:
+ - touchscreen-size-x	  : horizontal resolution of touchscreen (in pixels)
+ - touchscreen-size-y	  : vertical resolution of touchscreen (in pixels)
+ - touchscreen-fuzz-x	  : horizontal noise value of the absolute input
+			    device (in pixels)
+ - touchscreen-fuzz-y	  : vertical noise value of the absolute input
+			    device (in pixels)
+ - touchscreen-inverted-x : X axis is inverted (boolean)
+ - touchscreen-inverted-y : Y axis is inverted (boolean)
+ - touchscreen-swapped-x-y	  : X and Y axis are swapped (boolean)
+			    Swapping is done after inverting the axis
+
+Example:
+
+i2c@00000000 {
+
+	ektf2127: touchscreen@40 {
+		compatible = "elan,ektf2127";
+		reg = <0x40>;
+		interrupt-parent = <&pio>;
+		interrupts = <6 11 IRQ_TYPE_EDGE_FALLING>
+		/* pinctrl-names = "default";
+		pinctrl-0 = <&ts_wake_pin_p66>; */
+		power-gpios = <&pio 1 3 GPIO_ACTIVE_HIGH>;
+		touchscreen-inverted-x;
+		touchscreen-swapped-x-y;
+	};
+
+};
+
diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
index 66c6264..28fd425 100644
--- a/drivers/input/touchscreen/Kconfig
+++ b/drivers/input/touchscreen/Kconfig
@@ -1133,4 +1133,15 @@ config TOUCHSCREEN_ROHM_BU21023
 	  To compile this driver as a module, choose M here: the
 	  module will be called bu21023_ts.
 
+config TOUCHSCREEN_EKTF2127
+	tristate "ektf2127 I2C touchscreen"
+	depends on I2C
+	help
+	  Say Y here if you have a touchscreen using ektf2127.
+
+	  If unsure, say N.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called ektf2127.
+
 endif
diff --git a/drivers/input/touchscreen/Makefile b/drivers/input/touchscreen/Makefile
index 968ff12..bc0db43 100644
--- a/drivers/input/touchscreen/Makefile
+++ b/drivers/input/touchscreen/Makefile
@@ -93,3 +93,4 @@ obj-$(CONFIG_TOUCHSCREEN_TPS6507X)	+= tps6507x-ts.o
 obj-$(CONFIG_TOUCHSCREEN_ZFORCE)	+= zforce_ts.o
 obj-$(CONFIG_TOUCHSCREEN_COLIBRI_VF50)	+= colibri-vf50-ts.o
 obj-$(CONFIG_TOUCHSCREEN_ROHM_BU21023)	+= rohm_bu21023.o
+obj-$(CONFIG_TOUCHSCREEN_EKTF2127)	+= ektf2127.o
diff --git a/drivers/input/touchscreen/ektf2127.c b/drivers/input/touchscreen/ektf2127.c
new file mode 100644
index 0000000..e57fbbd
--- /dev/null
+++ b/drivers/input/touchscreen/ektf2127.c
@@ -0,0 +1,351 @@
+/*
+ * Driver for ELAN eKTF2127 i2c touchscreen controller
+ *
+ * For this driver the layout of the Chipone icn8318 i2c
+ * touchscreencontroller is used.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * Author:
+ * Michel Verlaan <michel.verl@gmail.com>
+ * Siebren Vroegindeweij <siebren.vroegindeweij@hotmail.com>
+ *
+ * Original chipone_icn8318 driver:
+ * Hans de Goede <hdegoede@redhat.com>
+ */
+
+#include <linux/gpio/consumer.h>
+#include <linux/interrupt.h>
+#include <linux/i2c.h>
+#include <linux/input.h>
+#include <linux/input/mt.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/delay.h>
+
+#define EKTF2127_REG_POWER		4
+#define EKTF2127_REG_TOUCHDATA		16
+
+#define EKTF2127_POWER_ACTIVE		0
+#define EKTF2127_POWER_MONITOR		1
+#define EKTF2127_POWER_HIBERNATE	2
+
+#define EKTF2127_MAX_TOUCHES		5
+
+/* The difference between 2 and 3 is unclear */
+#define EKTF2127_EVENT_NO_DATA	1 /* No finger seen yet since wakeup */
+#define EKTF2127_EVENT_UPDATE1	2 /* New or updated coordinates */
+#define EKTF2127_EVENT_UPDATE2	3 /* New or updated coordinates */
+#define EKTF2127_EVENT_END	4 /* Finger lifted */
+
+struct ektf2127_data {
+	struct i2c_client *client;
+	struct input_dev *input;
+	struct gpio_desc *power_gpios;
+	u32 max_x;
+	u32 max_y;
+	bool invert_x;
+	bool invert_y;
+	bool swap_x_y;
+};
+
+static void retrieve_coordinates(struct input_mt_pos *touches,
+				int touch_count, char *buf)
+{
+	int index = 0;
+	int i = 0;
+
+	for (i = 0; i < touch_count; i++) {
+		index = 2 + i * 3;
+
+		touches[i].x = (buf[index] & 0x0f);
+		touches[i].x <<= 8;
+		touches[i].x |= buf[index + 2];
+
+		touches[i].y = (buf[index] & 0xf0);
+		touches[i].y <<= 4;
+		touches[i].y |= buf[index + 1];
+	}
+}
+
+static irqreturn_t ektf2127_irq(int irq, void *dev_id)
+{
+	struct ektf2127_data *data = dev_id;
+	struct device *dev = &data->client->dev;
+	struct input_mt_pos touches[EKTF2127_MAX_TOUCHES];
+	int touch_count;
+	int slots[EKTF2127_MAX_TOUCHES];
+	char buff[25];
+	int i, ret;
+
+	ret = i2c_master_recv(data->client, buff, 25);
+	if (ret != 25) {
+		dev_err(dev, "Error reading touch data");
+		return IRQ_HANDLED;
+	}
+
+	touch_count = buff[1] & 0x07;
+
+	if (touch_count > EKTF2127_MAX_TOUCHES) {
+		dev_err(dev, "Too many touches %d > %d\n",
+			touch_count, EKTF2127_MAX_TOUCHES);
+		touch_count = EKTF2127_MAX_TOUCHES;
+	}
+
+	retrieve_coordinates(touches, touch_count, buff);
+
+	for (i = 0; i < touch_count; i++) {
+
+		input_mt_assign_slots(data->input, slots, touches,
+			touch_count, 0);
+
+		input_mt_slot(data->input, slots[i]);
+		input_mt_report_slot_state(data->input, MT_TOOL_FINGER, true);
+
+		input_event(data->input, EV_ABS, ABS_MT_POSITION_X,
+			touches[i].x);
+		input_event(data->input, EV_ABS, ABS_MT_POSITION_Y,
+			touches[i].y);
+	}
+
+	input_mt_sync_frame(data->input);
+	input_sync(data->input);
+
+	return IRQ_HANDLED;
+}
+
+static int ektf2127_start(struct input_dev *dev)
+{
+	struct ektf2127_data *data = input_get_drvdata(dev);
+
+	enable_irq(data->client->irq);
+	gpiod_set_value_cansleep(data->power_gpios, 1);
+
+	return 0;
+}
+
+static void ektf2127_stop(struct input_dev *dev)
+{
+	struct ektf2127_data *data = input_get_drvdata(dev);
+
+	disable_irq(data->client->irq);
+	gpiod_set_value_cansleep(data->power_gpios, 0);
+}
+
+static int ektf2127_suspend(struct device *dev)
+{
+	struct ektf2127_data *data = i2c_get_clientdata(to_i2c_client(dev));
+
+	mutex_lock(&data->input->mutex);
+	if (data->input->users)
+		ektf2127_stop(data->input);
+	mutex_unlock(&data->input->mutex);
+
+	return 0;
+}
+
+static int ektf2127_resume(struct device *dev)
+{
+	struct ektf2127_data *data = i2c_get_clientdata(to_i2c_client(dev));
+
+	mutex_lock(&data->input->mutex);
+	if (data->input->users)
+		ektf2127_start(data->input);
+	mutex_unlock(&data->input->mutex);
+
+	return 0;
+}
+
+static SIMPLE_DEV_PM_OPS(ektf2127_pm_ops, ektf2127_suspend,
+			ektf2127_resume);
+
+static int ektf2127_probe(struct i2c_client *client,
+			 const struct i2c_device_id *id)
+{
+	struct device *dev = &client->dev;
+	struct device_node *np = dev->of_node;
+	struct ektf2127_data *data;
+	struct input_dev *input;
+
+	u32 fuzz_x = 0, fuzz_y = 0;
+	char buff[25];
+	int error, ret = 0;
+
+	if (!client->irq) {
+		dev_err(dev, "Error no irq specified\n");
+		return -EINVAL;
+	}
+
+	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	data->power_gpios = devm_gpiod_get(dev, "power", GPIOD_OUT_HIGH);
+	msleep(20);
+	if (IS_ERR(data->power_gpios)) {
+		error = PTR_ERR(data->power_gpios);
+		if (error != -EPROBE_DEFER)
+			dev_err(dev, "Error getting power gpio: %d\n", error);
+		return error;
+	}
+
+	input = devm_input_allocate_device(dev);
+	if (!input)
+		return -ENOMEM;
+
+	input->name = client->name;
+	input->id.bustype = BUS_I2C;
+	input->open = ektf2127_start;
+	input->close = ektf2127_stop;
+	input->dev.parent = dev;
+
+	data->client = client;
+
+	/* read hello */
+	i2c_master_recv(data->client, buff, 4);
+
+	/* Read resolution from chip */
+
+	/* Request height */
+	buff[0] = 0x53; /* REQUEST */
+	buff[1] = 0x63; /* HEIGHT */
+	buff[2] = 0x00;
+	buff[3] = 0x00;
+	ret = i2c_master_send(data->client, buff, 4);
+	if (ret != 4) {
+		dev_err(dev, "Error requesting height");
+		return ret < 0 ? ret : -EIO;
+	}
+
+	msleep(20);
+
+	/* Read response */
+	ret = i2c_master_recv(data->client, buff, 4);
+	if (ret != 4) {
+		dev_err(dev, "Error receiving height");
+		return ret < 0 ? ret : -EIO;
+	}
+
+
+	if ((buff[0] == 0x52) && (buff[1] == 0x63)) {
+		data->max_y = ((buff[3] & 0xf0) << 4) | buff[2];
+	} else {
+		dev_err(dev, "Error receiving height data from"
+			" wrong register");
+		return ret < 0 ? ret : -EIO;
+	}
+
+	/* Request width */
+	buff[0] = 0x53; /* REQUEST */
+	buff[1] = 0x60; /* WIDTH */
+	buff[2] = 0x00;
+	buff[3] = 0x00;
+	ret = i2c_master_send(data->client, buff, 4);
+	if (ret != 4) {
+		dev_err(dev, "Error requesting width");
+		return ret < 0 ? ret : -EIO;
+	}
+
+	msleep(20);
+
+	/* Read response */
+	ret = i2c_master_recv(data->client, buff, 4);
+	if (ret != 4) {
+		dev_err(dev, "Error receiving width");
+		return ret < 0 ? ret : -EIO;
+	}
+
+
+	if ((buff[0] == 0x52) && (buff[1] == 0x60)) {
+		data->max_x = (((buff[3] & 0xf0) << 4) | buff[2]);
+	} else {
+		dev_err(dev, "Error receiving width data from"
+			" wrong register");
+		return ret < 0 ? ret : -EIO;
+	}
+
+
+	/* Touchscreen resolution can be overruled by devicetree*/
+	of_property_read_u32(np, "touchscreen-size-x", &data->max_x);
+	of_property_read_u32(np, "touchscreen-size-y", &data->max_y);
+
+	/* Optional */
+	of_property_read_u32(np, "touchscreen-fuzz-x", &fuzz_x);
+	of_property_read_u32(np, "touchscreen-fuzz-y", &fuzz_y);
+	data->invert_x = of_property_read_bool(np, "touchscreen-inverted-x");
+	data->invert_y = of_property_read_bool(np, "touchscreen-inverted-y");
+	data->swap_x_y = of_property_read_bool(np, "touchscreen-swapped-x-y");
+
+	if (!data->swap_x_y) {
+		input_set_abs_params(input, ABS_MT_POSITION_X, 0,
+				     data->max_x, fuzz_x, 0);
+		input_set_abs_params(input, ABS_MT_POSITION_Y, 0,
+				     data->max_y, fuzz_y, 0);
+	} else {
+		input_set_abs_params(input, ABS_MT_POSITION_X, 0,
+				     data->max_y, fuzz_y, 0);
+		input_set_abs_params(input, ABS_MT_POSITION_Y, 0,
+				     data->max_x, fuzz_x, 0);
+	}
+
+	error = input_mt_init_slots(input, EKTF2127_MAX_TOUCHES,
+				    INPUT_MT_DIRECT | INPUT_MT_DROP_UNUSED
+				    | INPUT_MT_TRACK);
+	if (error)
+		return error;
+
+	data->input = input;
+	input_set_drvdata(input, data);
+
+	error = devm_request_threaded_irq(dev, client->irq, NULL, ektf2127_irq,
+					  IRQF_ONESHOT, client->name, data);
+	if (error) {
+		dev_err(dev, "Error requesting irq: %d\n", error);
+		return error;
+	}
+
+	/* Stop device till opened */
+	ektf2127_stop(data->input);
+
+	error = input_register_device(input);
+	if (error)
+		return error;
+
+	i2c_set_clientdata(client, data);
+
+	return 0;
+}
+
+static const struct of_device_id ektf2127_of_match[] = {
+	{ .compatible = "elan,ektf2127" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, ektf2127_of_match);
+
+/* This is useless for OF-enabled devices,
+ * but it is needed by I2C subsystem
+ */
+static const struct i2c_device_id ektf2127_i2c_id[] = {
+	{ },
+};
+MODULE_DEVICE_TABLE(i2c, ektf2127_i2c_id);
+
+static struct i2c_driver ektf2127_driver = {
+	.driver = {
+		.name	= "elan_ektf2127",
+		.pm	= &ektf2127_pm_ops,
+		.of_match_table = ektf2127_of_match,
+	},
+	.probe = ektf2127_probe,
+	.id_table = ektf2127_i2c_id,
+};
+
+module_i2c_driver(ektf2127_driver);
+
+MODULE_DESCRIPTION("ELAN eKTF2127 I2C Touchscreen Driver");
+MODULE_AUTHOR("Michel Verlaan");
+MODULE_AUTHOR("Siebren Vroegindeweij");
+MODULE_LICENSE("GPL");
-- 
2.5.5

Re: [PATCH v1] Add support for the ektf2127 touchscreencontroller

[Rob Herring]

On Mon, Jul 04, 2016 at 11:29:05AM +0200, Siebren Vroegindeweij wrote:
> From: Siebren Vroegindeweij <siebren.vroegindeweij@hotmail.com>
> 
> The ELAN eKTF2127 driver is written for the ELAN series of
> touchscreencontrollers. This version is especially written for the
space        ^

> eKTF2127 controller. The driver communicates to the controller over i2c.
> The additional screen specifications can be read from the devicetree file.
> The driver has also the ability to read the screen height and width from
> the controller. When the screen is pressed, a interrupt is generated.
> The interrupt wil be handled by a function that request a data string from
> the controller. This string is then modified so that the right number of touches
> and their x and y coordinates are available and given to userspace through
> the input subsystem.
> 
> Signed-off-by: Michel Verlaan <michel.verl@gmail.com>
> Signed-off-by: Siebren Vroegindeweij <siebren.vroegindeweij@hotmail.com>
> ---
>  .../bindings/input/touchscreen/ektf2127.txt        |  40 +++
>  drivers/input/touchscreen/Kconfig                  |  11 +
>  drivers/input/touchscreen/Makefile                 |   1 +
>  drivers/input/touchscreen/ektf2127.c               | 351 +++++++++++++++++++++
>  4 files changed, 403 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/input/touchscreen/ektf2127.txt
>  create mode 100644 drivers/input/touchscreen/ektf2127.c
> 
> diff --git a/Documentation/devicetree/bindings/input/touchscreen/ektf2127.txt b/Documentation/devicetree/bindings/input/touchscreen/ektf2127.txt
> new file mode 100644
> index 0000000..a774336
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/touchscreen/ektf2127.txt
> @@ -0,0 +1,40 @@
> +* Elan eKTF2127 I2C touchscreen controller
> +
> +Required properties:
> + - compatible		  : "elan,ektf2127"
> + - reg			  : I2C slave address of the chip (0x40)
> + - interrupt-parent	  : a phandle pointing to the interrupt controller
> +			    serving the interrupt for this chip
> + - interrupts		  : interrupt specification for the eKTF2127 interrupt
> + - wake-gpios		  : GPIO specification for the WAKE input

Please state the active state and make it clear this is input to the 
touch controller (or host?).

> +
> +Optional properties:
> + - touchscreen-size-x	  : horizontal resolution of touchscreen (in pixels)
> + - touchscreen-size-y	  : vertical resolution of touchscreen (in pixels)
> + - touchscreen-fuzz-x	  : horizontal noise value of the absolute input
> +			    device (in pixels)
> + - touchscreen-fuzz-y	  : vertical noise value of the absolute input
> +			    device (in pixels)
> + - touchscreen-inverted-x : X axis is inverted (boolean)
> + - touchscreen-inverted-y : Y axis is inverted (boolean)
> + - touchscreen-swapped-x-y	  : X and Y axis are swapped (boolean)
> +			    Swapping is done after inverting the axis

Just state "See touchscreen.txt" for each of these.

> +
> +Example:
> +
> +i2c@00000000 {
> +
> +	ektf2127: touchscreen@40 {
> +		compatible = "elan,ektf2127";
> +		reg = <0x40>;
> +		interrupt-parent = <&pio>;
> +		interrupts = <6 11 IRQ_TYPE_EDGE_FALLING>
> +		/* pinctrl-names = "default";
> +		pinctrl-0 = <&ts_wake_pin_p66>; */

Not documented.

> +		power-gpios = <&pio 1 3 GPIO_ACTIVE_HIGH>;

Not documented.

> +		touchscreen-inverted-x;
> +		touchscreen-swapped-x-y;
> +	};
> +
> +};
> +

Re: [PATCH v1] Add support for the ektf2127 touchscreencontroller

[Dmitry Torokhov]

On Mon, Jul 04, 2016 at 11:29:05AM +0200, Siebren Vroegindeweij wrote:
> From: Siebren Vroegindeweij <siebren.vroegindeweij@hotmail.com>
> 
> The ELAN eKTF2127 driver is written for the ELAN series of
> touchscreencontrollers. This version is especially written for the
> eKTF2127 controller. The driver communicates to the controller over i2c.
> The additional screen specifications can be read from the devicetree file.
> The driver has also the ability to read the screen height and width from
> the controller. When the screen is pressed, a interrupt is generated.
> The interrupt wil be handled by a function that request a data string from
> the controller. This string is then modified so that the right number of touches
> and their x and y coordinates are available and given to userspace through
> the input subsystem.

Adding KT. Can eKTF2127 be driven with [modified] elants_i2c driver or
we are better off with separate driver.

...

> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 3 of the License, or
> + * (at your option) any later version.

Gpl V2 or later for kernel please, we won't take v3.

Thanks.

-- 
Dmitry

RE: [PATCH v1] Add support for the ektf2127 touchscreencontroller

[廖崇榮]

-----Original Message-----
From: Dmitry Torokhov [mailto:dmitry.torokhov@gmail.com] 
Sent: Thursday, July 14, 2016 5:50 AM
To: Siebren Vroegindeweij; KT Liao
Cc: linux-input@vger.kernel.org; devicetree; Hans de Goede; Michel Verlaan;
Siebren Vroegindeweij; KT Liao
Subject: Re: [PATCH v1] Add support for the ektf2127 touchscreencontroller

On Mon, Jul 04, 2016 at 11:29:05AM +0200, Siebren Vroegindeweij wrote:
> From: Siebren Vroegindeweij <siebren.vroegindeweij@hotmail.com>
> 
> The ELAN eKTF2127 driver is written for the ELAN series of 
> touchscreencontrollers. This version is especially written for the
> eKTF2127 controller. The driver communicates to the controller over i2c.
> The additional screen specifications can be read from the devicetree file.
> The driver has also the ability to read the screen height and width 
> from the controller. When the screen is pressed, a interrupt is generated.
> The interrupt wil be handled by a function that request a data string 
> from the controller. This string is then modified so that the right 
> number of touches and their x and y coordinates are available and 
> given to userspace through the input subsystem.

Adding KT. Can eKTF2127 be driven with [modified] elants_i2c driver or we
are better off with separate driver.

Hi Johnny,
Please check Dmitry's touchscreen issue.
...

> + *
> + * This program is free software; you can redistribute it and/or 
> + modify
> + * it under the terms of the GNU General Public License as published 
> + by
> + * the Free Software Foundation; either version 3 of the License, or
> + * (at your option) any later version.

Gpl V2 or later for kernel please, we won't take v3.

Thanks.

--
Dmitry

RE: [PATCH v1] Add support for the ektf2127 touchscreencontroller

[Johnny Chuang(莊佳霖)]

Hi Dmitry,

We suggest a separate driver for eKTF2127.

Many thanks,
Johnny
-----Original Message-----
From: 廖崇榮 [mailto:kt.liao-9cfG7bMpBgR9nmWX13WWKA@public.gmane.org] 
Sent: Thursday, July 14, 2016 6:13 PM
To: 'Dmitry Torokhov'; 'Siebren Vroegindeweij'; 'Johnny Chuang(莊佳霖)';
'James Chen'
Cc: linux-input-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; 'devicetree'; 'Hans de Goede'; 'Michel
Verlaan'; 'Siebren Vroegindeweij'
Subject: RE: [PATCH v1] Add support for the ektf2127 touchscreencontroller

-----Original Message-----
From: Dmitry Torokhov [mailto:dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org]
Sent: Thursday, July 14, 2016 5:50 AM
To: Siebren Vroegindeweij; KT Liao
Cc: linux-input-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; devicetree; Hans de Goede; Michel Verlaan;
Siebren Vroegindeweij; KT Liao
Subject: Re: [PATCH v1] Add support for the ektf2127 touchscreencontroller

On Mon, Jul 04, 2016 at 11:29:05AM +0200, Siebren Vroegindeweij wrote:
> From: Siebren Vroegindeweij <siebren.vroegindeweij-PkbjNfxxIARBDgjK7y7TUQ@public.gmane.org>
> 
> The ELAN eKTF2127 driver is written for the ELAN series of 
> touchscreencontrollers. This version is especially written for the
> eKTF2127 controller. The driver communicates to the controller over i2c.
> The additional screen specifications can be read from the devicetree file.
> The driver has also the ability to read the screen height and width 
> from the controller. When the screen is pressed, a interrupt is generated.
> The interrupt wil be handled by a function that request a data string 
> from the controller. This string is then modified so that the right 
> number of touches and their x and y coordinates are available and 
> given to userspace through the input subsystem.

Adding KT. Can eKTF2127 be driven with [modified] elants_i2c driver or we
are better off with separate driver.

Hi Johnny,
Please check Dmitry's touchscreen issue.
...

> + *
> + * This program is free software; you can redistribute it and/or 
> + modify
> + * it under the terms of the GNU General Public License as published 
> + by
> + * the Free Software Foundation; either version 3 of the License, or
> + * (at your option) any later version.

Gpl V2 or later for kernel please, we won't take v3.

Thanks.

--
Dmitry


--
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 v1] Add support for the ektf2127 touchscreencontroller

[Dmitry Torokhov]

Hi Siebren,

On Mon, Jul 04, 2016 at 11:29:05AM +0200, Siebren Vroegindeweij wrote:
> From: Siebren Vroegindeweij <siebren.vroegindeweij-PkbjNfxxIARBDgjK7y7TUQ@public.gmane.org>
> 
> The ELAN eKTF2127 driver is written for the ELAN series of
> touchscreencontrollers. This version is especially written for the
> eKTF2127 controller. The driver communicates to the controller over i2c.
> The additional screen specifications can be read from the devicetree file.
> The driver has also the ability to read the screen height and width from
> the controller. When the screen is pressed, a interrupt is generated.
> The interrupt wil be handled by a function that request a data string from
> the controller. This string is then modified so that the right number of touches
> and their x and y coordinates are available and given to userspace through
> the input subsystem.
> 
> Signed-off-by: Michel Verlaan <michel.verl-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Signed-off-by: Siebren Vroegindeweij <siebren.vroegindeweij-PkbjNfxxIARBDgjK7y7TUQ@public.gmane.org>
> ---
>  .../bindings/input/touchscreen/ektf2127.txt        |  40 +++
>  drivers/input/touchscreen/Kconfig                  |  11 +
>  drivers/input/touchscreen/Makefile                 |   1 +
>  drivers/input/touchscreen/ektf2127.c               | 351 +++++++++++++++++++++
>  4 files changed, 403 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/input/touchscreen/ektf2127.txt
>  create mode 100644 drivers/input/touchscreen/ektf2127.c
> 
> diff --git a/Documentation/devicetree/bindings/input/touchscreen/ektf2127.txt b/Documentation/devicetree/bindings/input/touchscreen/ektf2127.txt
> new file mode 100644
> index 0000000..a774336
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/touchscreen/ektf2127.txt
> @@ -0,0 +1,40 @@
> +* Elan eKTF2127 I2C touchscreen controller
> +
> +Required properties:
> + - compatible		  : "elan,ektf2127"
> + - reg			  : I2C slave address of the chip (0x40)
> + - interrupt-parent	  : a phandle pointing to the interrupt controller
> +			    serving the interrupt for this chip
> + - interrupts		  : interrupt specification for the eKTF2127 interrupt
> + - wake-gpios		  : GPIO specification for the WAKE input
> +
> +Optional properties:
> + - touchscreen-size-x	  : horizontal resolution of touchscreen (in pixels)
> + - touchscreen-size-y	  : vertical resolution of touchscreen (in pixels)
> + - touchscreen-fuzz-x	  : horizontal noise value of the absolute input
> +			    device (in pixels)
> + - touchscreen-fuzz-y	  : vertical noise value of the absolute input
> +			    device (in pixels)
> + - touchscreen-inverted-x : X axis is inverted (boolean)
> + - touchscreen-inverted-y : Y axis is inverted (boolean)
> + - touchscreen-swapped-x-y	  : X and Y axis are swapped (boolean)
> +			    Swapping is done after inverting the axis
> +
> +Example:
> +
> +i2c@00000000 {
> +
> +	ektf2127: touchscreen@40 {
> +		compatible = "elan,ektf2127";
> +		reg = <0x40>;
> +		interrupt-parent = <&pio>;
> +		interrupts = <6 11 IRQ_TYPE_EDGE_FALLING>
> +		/* pinctrl-names = "default";
> +		pinctrl-0 = <&ts_wake_pin_p66>; */
> +		power-gpios = <&pio 1 3 GPIO_ACTIVE_HIGH>;
> +		touchscreen-inverted-x;
> +		touchscreen-swapped-x-y;
> +	};
> +
> +};
> +
> diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
> index 66c6264..28fd425 100644
> --- a/drivers/input/touchscreen/Kconfig
> +++ b/drivers/input/touchscreen/Kconfig
> @@ -1133,4 +1133,15 @@ config TOUCHSCREEN_ROHM_BU21023
>  	  To compile this driver as a module, choose M here: the
>  	  module will be called bu21023_ts.
>  
> +config TOUCHSCREEN_EKTF2127
> +	tristate "ektf2127 I2C touchscreen"
> +	depends on I2C
> +	help
> +	  Say Y here if you have a touchscreen using ektf2127.
> +
> +	  If unsure, say N.
> +
> +	  To compile this driver as a module, choose M here: the
> +	  module will be called ektf2127.
> +
>  endif
> diff --git a/drivers/input/touchscreen/Makefile b/drivers/input/touchscreen/Makefile
> index 968ff12..bc0db43 100644
> --- a/drivers/input/touchscreen/Makefile
> +++ b/drivers/input/touchscreen/Makefile
> @@ -93,3 +93,4 @@ obj-$(CONFIG_TOUCHSCREEN_TPS6507X)	+= tps6507x-ts.o
>  obj-$(CONFIG_TOUCHSCREEN_ZFORCE)	+= zforce_ts.o
>  obj-$(CONFIG_TOUCHSCREEN_COLIBRI_VF50)	+= colibri-vf50-ts.o
>  obj-$(CONFIG_TOUCHSCREEN_ROHM_BU21023)	+= rohm_bu21023.o
> +obj-$(CONFIG_TOUCHSCREEN_EKTF2127)	+= ektf2127.o
> diff --git a/drivers/input/touchscreen/ektf2127.c b/drivers/input/touchscreen/ektf2127.c
> new file mode 100644
> index 0000000..e57fbbd
> --- /dev/null
> +++ b/drivers/input/touchscreen/ektf2127.c
> @@ -0,0 +1,351 @@
> +/*
> + * Driver for ELAN eKTF2127 i2c touchscreen controller
> + *
> + * For this driver the layout of the Chipone icn8318 i2c
> + * touchscreencontroller is used.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 3 of the License, or
> + * (at your option) any later version.

As I mentioned the kernel is GPL v2, so please change it from 3 to 2 for
me to be able to accept the driver.

> + *
> + * Author:
> + * Michel Verlaan <michel.verl-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> + * Siebren Vroegindeweij <siebren.vroegindeweij-PkbjNfxxIARBDgjK7y7TUQ@public.gmane.org>
> + *
> + * Original chipone_icn8318 driver:
> + * Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> + */
> +
> +#include <linux/gpio/consumer.h>
> +#include <linux/interrupt.h>
> +#include <linux/i2c.h>
> +#include <linux/input.h>
> +#include <linux/input/mt.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/delay.h>
> +
> +#define EKTF2127_REG_POWER		4
> +#define EKTF2127_REG_TOUCHDATA		16
> +
> +#define EKTF2127_POWER_ACTIVE		0
> +#define EKTF2127_POWER_MONITOR		1
> +#define EKTF2127_POWER_HIBERNATE	2
> +
> +#define EKTF2127_MAX_TOUCHES		5
> +
> +/* The difference between 2 and 3 is unclear */
> +#define EKTF2127_EVENT_NO_DATA	1 /* No finger seen yet since wakeup */
> +#define EKTF2127_EVENT_UPDATE1	2 /* New or updated coordinates */
> +#define EKTF2127_EVENT_UPDATE2	3 /* New or updated coordinates */
> +#define EKTF2127_EVENT_END	4 /* Finger lifted */
> +
> +struct ektf2127_data {
> +	struct i2c_client *client;
> +	struct input_dev *input;
> +	struct gpio_desc *power_gpios;
> +	u32 max_x;
> +	u32 max_y;
> +	bool invert_x;
> +	bool invert_y;
> +	bool swap_x_y;
> +};
> +
> +static void retrieve_coordinates(struct input_mt_pos *touches,
> +				int touch_count, char *buf)
> +{
> +	int index = 0;
> +	int i = 0;
> +
> +	for (i = 0; i < touch_count; i++) {
> +		index = 2 + i * 3;
> +
> +		touches[i].x = (buf[index] & 0x0f);
> +		touches[i].x <<= 8;
> +		touches[i].x |= buf[index + 2];
> +
> +		touches[i].y = (buf[index] & 0xf0);
> +		touches[i].y <<= 4;
> +		touches[i].y |= buf[index + 1];
> +	}
> +}
> +
> +static irqreturn_t ektf2127_irq(int irq, void *dev_id)
> +{
> +	struct ektf2127_data *data = dev_id;
> +	struct device *dev = &data->client->dev;
> +	struct input_mt_pos touches[EKTF2127_MAX_TOUCHES];
> +	int touch_count;
> +	int slots[EKTF2127_MAX_TOUCHES];
> +	char buff[25];
> +	int i, ret;
> +
> +	ret = i2c_master_recv(data->client, buff, 25);
> +	if (ret != 25) {
> +		dev_err(dev, "Error reading touch data");
> +		return IRQ_HANDLED;
> +	}
> +
> +	touch_count = buff[1] & 0x07;
> +
> +	if (touch_count > EKTF2127_MAX_TOUCHES) {
> +		dev_err(dev, "Too many touches %d > %d\n",
> +			touch_count, EKTF2127_MAX_TOUCHES);
> +		touch_count = EKTF2127_MAX_TOUCHES;
> +	}
> +
> +	retrieve_coordinates(touches, touch_count, buff);
> +
> +	for (i = 0; i < touch_count; i++) {
> +
> +		input_mt_assign_slots(data->input, slots, touches,
> +			touch_count, 0);

You only want to assign the slots once, not repeat it every time you
report a slot.

> +
> +		input_mt_slot(data->input, slots[i]);
> +		input_mt_report_slot_state(data->input, MT_TOOL_FINGER, true);
> +
> +		input_event(data->input, EV_ABS, ABS_MT_POSITION_X,
> +			touches[i].x);
> +		input_event(data->input, EV_ABS, ABS_MT_POSITION_Y,
> +			touches[i].y);
> +	}
> +
> +	input_mt_sync_frame(data->input);
> +	input_sync(data->input);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static int ektf2127_start(struct input_dev *dev)
> +{
> +	struct ektf2127_data *data = input_get_drvdata(dev);
> +
> +	enable_irq(data->client->irq);
> +	gpiod_set_value_cansleep(data->power_gpios, 1);
> +
> +	return 0;
> +}
> +
> +static void ektf2127_stop(struct input_dev *dev)
> +{
> +	struct ektf2127_data *data = input_get_drvdata(dev);
> +
> +	disable_irq(data->client->irq);
> +	gpiod_set_value_cansleep(data->power_gpios, 0);
> +}
> +
> +static int ektf2127_suspend(struct device *dev)
> +{
> +	struct ektf2127_data *data = i2c_get_clientdata(to_i2c_client(dev));
> +
> +	mutex_lock(&data->input->mutex);
> +	if (data->input->users)
> +		ektf2127_stop(data->input);
> +	mutex_unlock(&data->input->mutex);
> +
> +	return 0;
> +}
> +
> +static int ektf2127_resume(struct device *dev)
> +{
> +	struct ektf2127_data *data = i2c_get_clientdata(to_i2c_client(dev));
> +
> +	mutex_lock(&data->input->mutex);
> +	if (data->input->users)
> +		ektf2127_start(data->input);
> +	mutex_unlock(&data->input->mutex);
> +
> +	return 0;
> +}
> +
> +static SIMPLE_DEV_PM_OPS(ektf2127_pm_ops, ektf2127_suspend,
> +			ektf2127_resume);
> +
> +static int ektf2127_probe(struct i2c_client *client,
> +			 const struct i2c_device_id *id)
> +{
> +	struct device *dev = &client->dev;
> +	struct device_node *np = dev->of_node;
> +	struct ektf2127_data *data;
> +	struct input_dev *input;
> +
> +	u32 fuzz_x = 0, fuzz_y = 0;
> +	char buff[25];
> +	int error, ret = 0;
> +
> +	if (!client->irq) {
> +		dev_err(dev, "Error no irq specified\n");
> +		return -EINVAL;
> +	}
> +
> +	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
> +	if (!data)
> +		return -ENOMEM;
> +
> +	data->power_gpios = devm_gpiod_get(dev, "power", GPIOD_OUT_HIGH);
> +	msleep(20);

Why do you need sleep here, before checking for error?

> +	if (IS_ERR(data->power_gpios)) {
> +		error = PTR_ERR(data->power_gpios);
> +		if (error != -EPROBE_DEFER)
> +			dev_err(dev, "Error getting power gpio: %d\n", error);
> +		return error;
> +	}
> +
> +	input = devm_input_allocate_device(dev);
> +	if (!input)
> +		return -ENOMEM;
> +
> +	input->name = client->name;
> +	input->id.bustype = BUS_I2C;
> +	input->open = ektf2127_start;
> +	input->close = ektf2127_stop;
> +	input->dev.parent = dev;

Not needed if you allocated with devm_input_allocate_device().

> +
> +	data->client = client;
> +
> +	/* read hello */
> +	i2c_master_recv(data->client, buff, 4);
> +
> +	/* Read resolution from chip */
> +
> +	/* Request height */
> +	buff[0] = 0x53; /* REQUEST */
> +	buff[1] = 0x63; /* HEIGHT */
> +	buff[2] = 0x00;
> +	buff[3] = 0x00;
> +	ret = i2c_master_send(data->client, buff, 4);
> +	if (ret != 4) {
> +		dev_err(dev, "Error requesting height");
> +		return ret < 0 ? ret : -EIO;
> +	}
> +
> +	msleep(20);
> +
> +	/* Read response */
> +	ret = i2c_master_recv(data->client, buff, 4);
> +	if (ret != 4) {
> +		dev_err(dev, "Error receiving height");
> +		return ret < 0 ? ret : -EIO;
> +	}
> +
> +
> +	if ((buff[0] == 0x52) && (buff[1] == 0x63)) {
> +		data->max_y = ((buff[3] & 0xf0) << 4) | buff[2];
> +	} else {
> +		dev_err(dev, "Error receiving height data from"
> +			" wrong register");
> +		return ret < 0 ? ret : -EIO;
> +	}
> +
> +	/* Request width */
> +	buff[0] = 0x53; /* REQUEST */
> +	buff[1] = 0x60; /* WIDTH */
> +	buff[2] = 0x00;
> +	buff[3] = 0x00;
> +	ret = i2c_master_send(data->client, buff, 4);
> +	if (ret != 4) {
> +		dev_err(dev, "Error requesting width");
> +		return ret < 0 ? ret : -EIO;
> +	}
> +
> +	msleep(20);
> +
> +	/* Read response */
> +	ret = i2c_master_recv(data->client, buff, 4);
> +	if (ret != 4) {
> +		dev_err(dev, "Error receiving width");
> +		return ret < 0 ? ret : -EIO;
> +	}
> +
> +
> +	if ((buff[0] == 0x52) && (buff[1] == 0x60)) {
> +		data->max_x = (((buff[3] & 0xf0) << 4) | buff[2]);
> +	} else {
> +		dev_err(dev, "Error receiving width data from"
> +			" wrong register");
> +		return ret < 0 ? ret : -EIO;
> +	}
> +
> +
> +	/* Touchscreen resolution can be overruled by devicetree*/
> +	of_property_read_u32(np, "touchscreen-size-x", &data->max_x);
> +	of_property_read_u32(np, "touchscreen-size-y", &data->max_y);
> +
> +	/* Optional */
> +	of_property_read_u32(np, "touchscreen-fuzz-x", &fuzz_x);
> +	of_property_read_u32(np, "touchscreen-fuzz-y", &fuzz_y);
> +	data->invert_x = of_property_read_bool(np, "touchscreen-inverted-x");
> +	data->invert_y = of_property_read_bool(np, "touchscreen-inverted-y");
> +	data->swap_x_y = of_property_read_bool(np, "touchscreen-swapped-x-y");

Please use touchscreen_parse_properties() with struct
touchscreen_properties *prop argument and then use
touchscreen_set_mt_pos() to aply the needed conversions (this is a  new
API) instead of doing it yourself by hand.



> +
> +	if (!data->swap_x_y) {
> +		input_set_abs_params(input, ABS_MT_POSITION_X, 0,
> +				     data->max_x, fuzz_x, 0);
> +		input_set_abs_params(input, ABS_MT_POSITION_Y, 0,
> +				     data->max_y, fuzz_y, 0);
> +	} else {
> +		input_set_abs_params(input, ABS_MT_POSITION_X, 0,
> +				     data->max_y, fuzz_y, 0);
> +		input_set_abs_params(input, ABS_MT_POSITION_Y, 0,
> +				     data->max_x, fuzz_x, 0);
> +	}
> +
> +	error = input_mt_init_slots(input, EKTF2127_MAX_TOUCHES,
> +				    INPUT_MT_DIRECT | INPUT_MT_DROP_UNUSED
> +				    | INPUT_MT_TRACK);
> +	if (error)
> +		return error;
> +
> +	data->input = input;
> +	input_set_drvdata(input, data);
> +
> +	error = devm_request_threaded_irq(dev, client->irq, NULL, ektf2127_irq,
> +					  IRQF_ONESHOT, client->name, data);
> +	if (error) {
> +		dev_err(dev, "Error requesting irq: %d\n", error);
> +		return error;
> +	}
> +
> +	/* Stop device till opened */
> +	ektf2127_stop(data->input);
> +
> +	error = input_register_device(input);
> +	if (error)
> +		return error;
> +
> +	i2c_set_clientdata(client, data);
> +
> +	return 0;
> +}
> +

#ifdef CONFIG_OF
> +static const struct of_device_id ektf2127_of_match[] = {
> +	{ .compatible = "elan,ektf2127" },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, ektf2127_of_match);
#endif

> +
> +/* This is useless for OF-enabled devices,
> + * but it is needed by I2C subsystem
> + */
> +static const struct i2c_device_id ektf2127_i2c_id[] = {
> +	{ },

I think you want to have "ektf2127" here.

> +};
> +MODULE_DEVICE_TABLE(i2c, ektf2127_i2c_id);
> +
> +static struct i2c_driver ektf2127_driver = {
> +	.driver = {
> +		.name	= "elan_ektf2127",
> +		.pm	= &ektf2127_pm_ops,
> +		.of_match_table = ektf2127_of_match,

of_match_ptr(ektf2127_of_match)

> +	},
> +	.probe = ektf2127_probe,
> +	.id_table = ektf2127_i2c_id,
> +};
> +
> +module_i2c_driver(ektf2127_driver);
> +
> +MODULE_DESCRIPTION("ELAN eKTF2127 I2C Touchscreen Driver");
> +MODULE_AUTHOR("Michel Verlaan");
> +MODULE_AUTHOR("Siebren Vroegindeweij");
> +MODULE_LICENSE("GPL");

Thanks.

-- 
Dmitry
--
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