Touch screen driver for the EDT FT5x06 based "polytouch" touchscreens

Touch screen driver for the EDT FT5x06 based "polytouch" touchscreens

[simon.budig]

Hi all.

This is a driver for the EDT ft5x06 based capacitive touchscreens.

The driver supports up to 5 touch points, the "gain", "threshold" and "offset"
parameters can be configured via some files in the sysfs.

It also is possible to acquire some raw data from the sensor by putting the
sensor in a factory mode ("mode" file in sysfs) and then reading the "raw" 
file, which contains the raw touch data. In factory mode the reporting of
regular events will stop.

I hope this can get integrated upstream, I appreciate any hints and 
suggestions regarding the code.

Thanks,
        Simon Budig

[PATCH] Touchscreen driver for FT5x06 based EDT displays

[simon.budig]

From: Simon Budig <simon@budig.de>

---
 drivers/input/touchscreen/Kconfig      |    5 +
 drivers/input/touchscreen/Makefile     |    1 +
 drivers/input/touchscreen/edt-ft5x06.c |  714 ++++++++++++++++++++++++++++++++
 include/linux/input/edt-ft5x06.h       |   16 +
 4 files changed, 736 insertions(+), 0 deletions(-)
 create mode 100644 drivers/input/touchscreen/edt-ft5x06.c
 create mode 100644 include/linux/input/edt-ft5x06.h

diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
index 0b28adf..4c0672c 100644
--- a/drivers/input/touchscreen/Kconfig
+++ b/drivers/input/touchscreen/Kconfig
@@ -339,6 +339,11 @@ config TOUCHSCREEN_PENMOUNT
 	  To compile this driver as a module, choose M here: the
 	  module will be called penmount.
 
+config TOUCHSCREEN_EDT_FT5X06
+	tristate "EDT FocalTech FT5x06 I2C Touchscreen support"
+	help
+	  If unsure, say N.
+
 config TOUCHSCREEN_QT602240
 	tristate "QT602240 I2C Touchscreen"
 	depends on I2C
diff --git a/drivers/input/touchscreen/Makefile b/drivers/input/touchscreen/Makefile
index bd54dfe..1288ab6 100644
--- a/drivers/input/touchscreen/Makefile
+++ b/drivers/input/touchscreen/Makefile
@@ -18,6 +18,7 @@ obj-$(CONFIG_TOUCHSCREEN_BU21013)       += bu21013_ts.o
 obj-$(CONFIG_TOUCHSCREEN_CY8CTMG110)	+= cy8ctmg110_ts.o
 obj-$(CONFIG_TOUCHSCREEN_DA9034)	+= da9034-ts.o
 obj-$(CONFIG_TOUCHSCREEN_DYNAPRO)	+= dynapro.o
+obj-$(CONFIG_TOUCHSCREEN_EDT_FT5X06)	+= edt-ft5x06.o
 obj-$(CONFIG_TOUCHSCREEN_HAMPSHIRE)	+= hampshire.o
 obj-$(CONFIG_TOUCHSCREEN_GUNZE)		+= gunze.o
 obj-$(CONFIG_TOUCHSCREEN_EETI)		+= eeti_ts.o
diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c
new file mode 100644
index 0000000..7e2b04b
--- /dev/null
+++ b/drivers/input/touchscreen/edt-ft5x06.c
@@ -0,0 +1,714 @@
+/* drivers/input/touchscreen/edt-ft5x06.c
+ *
+ * Copyright (C) 2011 Simon Budig, <simon.budig@kernelconcepts.de>
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ */
+
+/*
+ * This is a driver for the EDT "Polytouch" family of touch controllers
+ * based on the FocalTech FT5x06 line of chips.
+ *
+ * Development of this driver has been sponsored by Glyn:
+ *    http://www.glyn.com/Products/Displays
+ */
+
+#include <linux/module.h>
+#include <linux/interrupt.h>
+#include <linux/input.h>
+#include <linux/i2c.h>
+#include <asm/uaccess.h>
+#include <linux/smp_lock.h>
+#include <linux/delay.h>
+#include <linux/slab.h>
+
+#include <linux/gpio.h>
+
+#include "linux/input/edt-ft5x06.h"
+
+#define DRIVER_VERSION "v0.5"
+
+#define WORK_REGISTER_THRESHOLD   0x00
+#define WORK_REGISTER_GAIN        0x30
+#define WORK_REGISTER_OFFSET      0x31
+#define WORK_REGISTER_NUM_X       0x33
+#define WORK_REGISTER_NUM_Y       0x34
+
+#define WORK_REGISTER_OPMODE      0x3c
+#define FACTORY_REGISTER_OPMODE   0x01
+
+static struct i2c_driver edt_ft5x06_i2c_ts_driver;
+
+struct edt_ft5x06_i2c_ts_data
+{
+	struct i2c_client *client;
+	struct input_dev *input;
+	int irq;
+	int irq_pin;
+	int reset_pin;
+	int num_x;
+	int num_y;
+
+	struct mutex mutex;
+	int factory_mode;
+	int threshold;
+	int gain;
+	int offset;
+};
+
+static int edt_ft5x06_ts_readwrite (struct i2c_client *client,
+                                    u16 wr_len, u8 *wr_buf,
+                                    u16 rd_len, u8 *rd_buf)
+{
+	struct i2c_msg wrmsg[2];
+	int i, ret;
+
+	i = 0;
+	if (wr_len) {
+		wrmsg[i].addr  = client->addr;
+		wrmsg[i].flags = 0;
+		wrmsg[i].len = wr_len;
+		wrmsg[i].buf = wr_buf;
+		i++;
+	}
+	if (rd_len) {
+		wrmsg[i].addr  = client->addr;
+		wrmsg[i].flags = I2C_M_RD;
+		wrmsg[i].len = rd_len;
+		wrmsg[i].buf = rd_buf;
+		i++;
+	}
+
+	ret = i2c_transfer (client->adapter, wrmsg, i);
+	if (ret < 0) {
+		dev_info (&client->dev, "i2c_transfer failed: %d\n", ret);
+		return ret;
+	}
+
+	return ret;
+}
+
+
+static irqreturn_t edt_ft5x06_ts_isr (int irq, void *dev_id)
+{
+	struct edt_ft5x06_i2c_ts_data *tsdata = dev_id;
+	unsigned char touching = 0;
+	unsigned char rdbuf[26], wrbuf[1];
+	int i, have_abs, type, ret;
+
+	memset (wrbuf, 0, sizeof (wrbuf));
+	memset (rdbuf, 0, sizeof (rdbuf));
+
+	wrbuf[0] = 0xf9;
+
+	mutex_lock (&tsdata->mutex);
+	ret = edt_ft5x06_ts_readwrite (tsdata->client,
+	                               1, wrbuf,
+	                               sizeof (rdbuf), rdbuf);
+	mutex_unlock (&tsdata->mutex);
+	if (ret < 0) {
+		dev_err (&tsdata->client->dev, "Unable to write to i2c touchscreen!\n");
+		goto out;
+	}
+
+	if (rdbuf[0] != 0xaa || rdbuf[1] != 0xaa || rdbuf[2] != 26) {
+		dev_err (&tsdata->client->dev, "Unexpected header: %02x%02x%02x!\n", rdbuf[0], rdbuf[1], rdbuf[2]);
+	}
+
+	have_abs = 0;
+	touching = rdbuf[3];
+	for (i = 0; i < touching; i++) {
+		type = rdbuf[i*4+5] >> 6;
+		if (type == 0x01 || type == 0x03)  /* ignore Touch Down and Reserved events */
+			continue;
+
+		if (!have_abs) {
+			input_report_key (tsdata->input, BTN_TOUCH,    1);
+			input_report_abs (tsdata->input, ABS_PRESSURE, 1);
+			input_report_abs (tsdata->input, ABS_X, ((rdbuf[i*4+5] << 8) | rdbuf[i*4+6]) & 0x0fff);
+			input_report_abs (tsdata->input, ABS_Y, ((rdbuf[i*4+7] << 8) | rdbuf[i*4+8]) & 0x0fff);
+			have_abs = 1;
+		}
+		input_report_abs (tsdata->input, ABS_MT_POSITION_X, ((rdbuf[i*4+5] << 8) | rdbuf[i*4+6]) & 0x0fff);
+		input_report_abs (tsdata->input, ABS_MT_POSITION_Y, ((rdbuf[i*4+7] << 8) | rdbuf[i*4+8]) & 0x0fff);
+		input_report_abs (tsdata->input, ABS_MT_TRACKING_ID, (rdbuf[i*4+7] >> 4) & 0x0f);
+		input_mt_sync (tsdata->input);
+	}
+	if (!have_abs) {
+		input_report_key (tsdata->input, BTN_TOUCH,    0);
+		input_report_abs (tsdata->input, ABS_PRESSURE, 0);
+	}
+	input_sync (tsdata->input);
+
+out:
+	return IRQ_HANDLED;
+}
+
+
+static int edt_ft5x06_i2c_register_write (struct edt_ft5x06_i2c_ts_data *tsdata,
+                                          u8 addr, u8 value)
+{
+	u8 wrbuf[4];
+	int ret;
+
+	wrbuf[0]  = tsdata->factory_mode ? 0xf3 : 0xfc;
+	wrbuf[1]  = tsdata->factory_mode ? addr & 0x7f : addr & 0x3f;
+	wrbuf[2]  = value;
+	wrbuf[3]  = wrbuf[0] ^ wrbuf[1] ^ wrbuf[2];
+
+	disable_irq (tsdata->irq);
+
+	ret = edt_ft5x06_ts_readwrite (tsdata->client,
+	                               4, wrbuf,
+	                               0, NULL);
+
+	enable_irq (tsdata->irq);
+
+	return ret;
+}
+
+static int edt_ft5x06_i2c_register_read (struct edt_ft5x06_i2c_ts_data *tsdata,
+                                         u8 addr)
+{
+	u8 wrbuf[2], rdbuf[2];
+	int ret;
+
+	wrbuf[0]  = tsdata->factory_mode ? 0xf3 : 0xfc;
+	wrbuf[1]  = tsdata->factory_mode ? addr & 0x7f : addr & 0x3f;
+	wrbuf[1] |= tsdata->factory_mode ? 0x80 : 0x40;
+
+	disable_irq (tsdata->irq);
+
+	ret = edt_ft5x06_ts_readwrite (tsdata->client,
+	                               2, wrbuf,
+	                               2, rdbuf);
+
+	enable_irq (tsdata->irq);
+
+#if 0
+	dev_info (&tsdata->client->dev, "wr: %02x %02x -> rd: %02x %02x\n",
+	          wrbuf[0], wrbuf[1], rdbuf[0], rdbuf[1]);
+#endif
+
+	if ((wrbuf[0] ^ wrbuf[1] ^ rdbuf[0]) != rdbuf[1])
+		dev_err (&tsdata->client->dev, "crc error: 0x%02x expected, got 0x%02x\n",
+		         (wrbuf[0] ^ wrbuf[1] ^ rdbuf[0]), rdbuf[1]);
+
+	return ret < 0 ? ret : rdbuf[0];
+}
+
+static ssize_t edt_ft5x06_i2c_setting_show (struct device *dev,
+                                            struct device_attribute *attr,
+                                            char *buf)
+{
+	struct edt_ft5x06_i2c_ts_data *tsdata = dev_get_drvdata (dev);
+	struct i2c_client *client = tsdata->client;
+	int ret = 0;
+	int *value;
+	u8 addr;
+
+	switch (attr->attr.name[0]) {
+		case 't':    /* threshold */
+			addr = WORK_REGISTER_THRESHOLD;
+			value = &tsdata->threshold;
+			break;
+		case 'g':    /* gain */
+			addr = WORK_REGISTER_GAIN;
+			value = &tsdata->gain;
+			break;
+		case 'o':    /* offset */
+			addr = WORK_REGISTER_OFFSET;
+			value = &tsdata->offset;
+			break;
+		default:
+			dev_err (&client->dev, "unknown attribute for edt_ft5x06_i2c_setting_show: %s\n", attr->attr.name);
+			return -EINVAL;
+	}
+
+	mutex_lock (&tsdata->mutex);
+
+	if (tsdata->factory_mode == 1) {
+		dev_err (dev, "setting register not available in factory mode\n");
+		return -EIO;
+	}
+
+	ret = edt_ft5x06_i2c_register_read (tsdata, addr);
+	if (ret < 0) {
+		dev_err (&tsdata->client->dev, "Unable to write to i2c touchscreen!\n");
+		mutex_unlock (&tsdata->mutex);
+		return ret;
+	}
+	mutex_unlock (&tsdata->mutex);
+
+	if (ret != *value) {
+		dev_info (&tsdata->client->dev, "i2c read (%d) and stored value (%d) differ. Huh?\n", ret, *value);
+		*value = ret;
+	}
+
+	return sprintf (buf, "%d\n", ret);
+}
+
+static ssize_t edt_ft5x06_i2c_setting_store (struct device *dev,
+                                             struct device_attribute *attr,
+                                             const char *buf, size_t count)
+{
+	struct edt_ft5x06_i2c_ts_data *tsdata = dev_get_drvdata (dev);
+	struct i2c_client *client = tsdata->client;
+	int ret = 0;
+	u8 addr;
+	unsigned int val;
+
+	mutex_lock (&tsdata->mutex);
+
+	if (tsdata->factory_mode == 1) {
+		dev_err (dev, "setting register not available in factory mode\n");
+		ret = -EIO;
+		goto out;
+	}
+
+	if (sscanf(buf, "%u", &val) != 1) {
+		dev_err (dev, "Invalid value for attribute %s\n", attr->attr.name);
+		ret = -EINVAL;
+		goto out;
+	}
+
+	switch (attr->attr.name[0]) {
+		case 't':    /* threshold */
+			addr = WORK_REGISTER_THRESHOLD;
+			val = val < 20 ? 20 : val > 80 ? 80 : val;
+			tsdata->threshold = val;
+			break;
+		case 'g':    /* gain */
+			addr = WORK_REGISTER_GAIN;
+			val = val < 0 ? 0 : val > 31 ? 31 : val;
+			tsdata->gain = val;
+			break;
+		case 'o':    /* offset */
+			addr = WORK_REGISTER_OFFSET;
+			val = val < 0 ? 0 : val > 31 ? 31 : val;
+			tsdata->offset = val;
+			break;
+		default:
+			dev_err (&client->dev, "unknown attribute for edt_ft5x06_i2c_setting_show: %s\n", attr->attr.name);
+			ret = -EINVAL;
+			goto out;
+	}
+
+	ret = edt_ft5x06_i2c_register_write (tsdata, addr, val);
+
+	if (ret < 0) {
+		dev_err (&tsdata->client->dev, "Unable to write to i2c touchscreen!\n");
+		goto out;
+	}
+
+	mutex_unlock (&tsdata->mutex);
+	return count;
+
+out:
+	mutex_unlock (&tsdata->mutex);
+	return ret;
+}
+
+
+static ssize_t edt_ft5x06_i2c_mode_show (struct device *dev,
+                                         struct device_attribute *attr,
+                                         char *buf)
+{
+	struct edt_ft5x06_i2c_ts_data *tsdata = dev_get_drvdata (dev);
+	return sprintf (buf, "%d\n", tsdata->factory_mode);
+}
+
+static ssize_t edt_ft5x06_i2c_mode_store (struct device *dev,
+                                          struct device_attribute *attr,
+                                          const char *buf, size_t count)
+{
+	struct edt_ft5x06_i2c_ts_data *tsdata = dev_get_drvdata (dev);
+	int i, ret = 0;
+	unsigned int mode;
+
+	if (sscanf(buf, "%u", &mode) != 1 || (mode | 1) != 1) {
+		dev_err (dev, "Invalid value for operation mode\n");
+		return -EINVAL;
+	}
+
+	/* no change, return without doing anything */
+	if (mode == tsdata->factory_mode)
+		return count;
+
+	mutex_lock (&tsdata->mutex);
+	if (tsdata->factory_mode == 0) { /* switch to factory mode */
+		disable_irq (tsdata->irq);
+		/* mode register is 0x3c when in the work mode */
+		ret = edt_ft5x06_i2c_register_write (tsdata, WORK_REGISTER_OPMODE, 0x03);
+		if (ret < 0) {
+			dev_err (dev, "failed to switch to factory mode (%d)\n", ret);
+		} else {
+			tsdata->factory_mode = 1;
+			for (i = 0; i < 10; i++) {
+				mdelay (5);
+				/* mode register is 0x01 when in the factory mode */
+				ret = edt_ft5x06_i2c_register_read (tsdata, FACTORY_REGISTER_OPMODE);
+				if (ret == 0x03)
+					break;
+			}
+			if (i == 10)
+				dev_err (dev, "not in factory mode after %dms.\n", i*5);
+		}
+	} else {  /* switch to work mode */
+		/* mode register is 0x01 when in the factory mode */
+		ret = edt_ft5x06_i2c_register_write (tsdata, FACTORY_REGISTER_OPMODE, 0x01);
+		if (ret < 0) {
+			dev_err (dev, "failed to switch to work mode (%d)\n", ret);
+		} else {
+			tsdata->factory_mode = 0;
+			for (i = 0; i < 10; i++) {
+				mdelay (5);
+				/* mode register is 0x01 when in the factory mode */
+				ret = edt_ft5x06_i2c_register_read (tsdata, WORK_REGISTER_OPMODE);
+				if (ret == 0x01)
+					break;
+			}
+			if (i == 10)
+				dev_err (dev, "not in work mode after %dms.\n", i*5);
+
+			/* restore parameters */
+			edt_ft5x06_i2c_register_write (tsdata, WORK_REGISTER_THRESHOLD, tsdata->threshold);
+			edt_ft5x06_i2c_register_write (tsdata, WORK_REGISTER_GAIN, tsdata->gain);
+			edt_ft5x06_i2c_register_write (tsdata, WORK_REGISTER_OFFSET, tsdata->offset);
+
+			enable_irq (tsdata->irq);
+		}
+	}
+
+	mutex_unlock (&tsdata->mutex);
+	return count;
+}
+
+
+static ssize_t edt_ft5x06_i2c_raw_data_show (struct device *dev,
+                                             struct device_attribute *attr,
+                                             char *buf)
+{
+	struct edt_ft5x06_i2c_ts_data *tsdata = dev_get_drvdata (dev);
+	int i, ret;
+	char *ptr, wrbuf[3];
+
+	if (tsdata->factory_mode == 0) {
+		dev_err (dev, "raw data not available in work mode\n");
+		return -EIO;
+	}
+
+	mutex_lock (&tsdata->mutex);
+	ret = edt_ft5x06_i2c_register_write (tsdata, 0x08, 0x01);
+	for (i = 0; i < 100; i++) {
+		ret = edt_ft5x06_i2c_register_read (tsdata, 0x08);
+		if (ret < 1)
+			break;
+		udelay (1000);
+	}
+
+	if (i == 100 || ret < 0) {
+		dev_err (dev, "waiting time exceeded or error: %d\n", ret);
+		mutex_unlock (&tsdata->mutex);
+		return ret || ETIMEDOUT;
+	}
+
+	ptr = buf;
+	wrbuf[0] = 0xf5;
+	wrbuf[1] = 0x0e;
+	for (i = 0; i <= tsdata->num_x; i++) {
+		wrbuf[2] = i;
+		ret = edt_ft5x06_ts_readwrite (tsdata->client,
+		                               3, wrbuf,
+		                               tsdata->num_y * 2, ptr);
+		if (ret < 0) {
+			mutex_unlock (&tsdata->mutex);
+			return ret;
+		}
+
+		ptr += tsdata->num_y * 2;
+	}
+
+	mutex_unlock (&tsdata->mutex);
+	return ptr - buf;
+}
+
+
+static DEVICE_ATTR(gain,      0664, edt_ft5x06_i2c_setting_show, edt_ft5x06_i2c_setting_store);
+static DEVICE_ATTR(offset,    0664, edt_ft5x06_i2c_setting_show, edt_ft5x06_i2c_setting_store);
+static DEVICE_ATTR(threshold, 0664, edt_ft5x06_i2c_setting_show, edt_ft5x06_i2c_setting_store);
+static DEVICE_ATTR(mode,      0664, edt_ft5x06_i2c_mode_show, edt_ft5x06_i2c_mode_store);
+static DEVICE_ATTR(raw_data,  0444, edt_ft5x06_i2c_raw_data_show, NULL);
+
+static struct attribute *edt_ft5x06_i2c_attrs[] = {
+	&dev_attr_gain.attr,
+	&dev_attr_offset.attr,
+	&dev_attr_threshold.attr,
+	&dev_attr_mode.attr,
+	&dev_attr_raw_data.attr,
+	NULL
+};
+
+static const struct attribute_group edt_ft5x06_i2c_attr_group = {
+	.attrs = edt_ft5x06_i2c_attrs,
+};
+
+static int edt_ft5x06_i2c_ts_probe (struct i2c_client *client,
+                                    const struct i2c_device_id *id)
+{
+
+	struct edt_ft5x06_i2c_ts_data *tsdata;
+	struct input_dev *input;
+	int error;
+	u8 rdbuf[23];
+	char *model_name = NULL;
+	char *fw_version = NULL;
+
+	dev_info (&client->dev, "probing for EDT FT5x06 I2C\n");
+
+	if (!client->irq) {
+		dev_dbg (&client->dev, "no IRQ?\n");
+		return -ENODEV;
+	}
+
+	if (!client->dev.platform_data) {
+		dev_dbg (&client->dev, "no reset pin in platform data?\n");
+		return -ENODEV;
+	}
+
+	tsdata = kzalloc (sizeof (*tsdata), GFP_KERNEL);
+	if (!tsdata) {
+		dev_err (&client->dev, "failed to allocate driver data!\n");
+		dev_set_drvdata (&client->dev, NULL);
+		return -ENOMEM;
+	}
+
+	dev_set_drvdata (&client->dev, tsdata);
+	tsdata->client = client;
+
+	tsdata->reset_pin = ((struct edt_ft5x06_platform_data *) client->dev.platform_data)->reset_pin;
+	mutex_init (&tsdata->mutex);
+
+	error = gpio_request (tsdata->reset_pin, NULL);
+	if (error < 0) {
+		dev_err (&client->dev,
+		         "Failed to request GPIO %d as reset pin, error %d\n",
+		         tsdata->reset_pin, error);
+		error = -ENOMEM;
+		goto err_free_tsdata;
+	}
+
+	/* this pulls reset down, enabling the low active reset */
+	if (gpio_direction_output (tsdata->reset_pin, 0) < 0) {
+		dev_info (&client->dev, "switching to output failed\n");
+		error = -ENOMEM;
+		goto err_free_reset_pin;
+	}
+
+	/* request IRQ pin */
+	tsdata->irq = client->irq;
+	tsdata->irq_pin = irq_to_gpio (tsdata->irq);
+
+	error = gpio_request (tsdata->irq_pin, NULL);
+	if (error < 0) {
+		dev_err (&client->dev,
+		         "Failed to request GPIO %d for IRQ %d, error %d\n",
+		         tsdata->irq_pin, tsdata->irq, error);
+		error = -ENOMEM;
+		goto err_free_reset_pin;
+	}
+	gpio_direction_input (tsdata->irq_pin);
+
+	/* release reset */
+	mdelay (50);
+	gpio_set_value (tsdata->reset_pin, 1);
+	mdelay (100);
+
+	mutex_lock (&tsdata->mutex);
+
+	tsdata->factory_mode = 0;
+
+	if (edt_ft5x06_ts_readwrite (client, 1, "\xbb", 22, rdbuf) < 0) {
+		dev_err (&client->dev, "probing failed\n");
+		error = -ENODEV;
+		goto err_free_irq_pin;
+	}
+
+	rdbuf[22] = '\0';
+	if (rdbuf[21] == '$')
+		rdbuf[21] = '\0';
+
+	model_name = rdbuf + 1;
+	fw_version = rdbuf;
+	/* look for Model/Version separator */
+	while (fw_version[0] != '\0' && fw_version[0] != '*')
+		fw_version++;
+
+	tsdata->threshold = edt_ft5x06_i2c_register_read (tsdata, WORK_REGISTER_THRESHOLD);
+	tsdata->gain      = edt_ft5x06_i2c_register_read (tsdata, WORK_REGISTER_GAIN);
+	tsdata->offset    = edt_ft5x06_i2c_register_read (tsdata, WORK_REGISTER_OFFSET);
+	tsdata->num_x     = edt_ft5x06_i2c_register_read (tsdata, WORK_REGISTER_NUM_X);
+	tsdata->num_y     = edt_ft5x06_i2c_register_read (tsdata, WORK_REGISTER_NUM_Y);
+
+	mutex_unlock (&tsdata->mutex);
+
+	if (fw_version[0] == '*') {
+		fw_version[0] = '\0';
+		fw_version++;
+		dev_info (&client->dev, "Model \"%s\", Rev. \"%s\", %dx%d sensors\n",
+		          model_name, fw_version, tsdata->num_x, tsdata->num_y);
+	} else {
+		dev_info (&client->dev, "Product ID \"%s\"\n", rdbuf + 1);
+	}
+
+	input = input_allocate_device ();
+	if (!input) {
+		dev_err (&client->dev, "failed to allocate input device!\n");
+		error = -ENOMEM;
+		goto err_free_irq_pin;
+	}
+
+	set_bit (EV_SYN, input->evbit);
+	set_bit (EV_KEY, input->evbit);
+	set_bit (EV_ABS, input->evbit);
+	set_bit (BTN_TOUCH, input->keybit);
+	input_set_abs_params (input, ABS_X, 0, tsdata->num_x * 64 - 1, 0, 0);
+	input_set_abs_params (input, ABS_Y, 0, tsdata->num_y * 64 - 1, 0, 0);
+	input_set_abs_params (input, ABS_PRESSURE, 0, 1, 0, 0);
+	input_set_abs_params (input, ABS_MT_POSITION_X, 0, tsdata->num_x * 64 - 1, 0, 0);
+	input_set_abs_params (input, ABS_MT_POSITION_Y, 0, tsdata->num_y * 64 - 1, 0, 0);
+	input_set_abs_params (input, ABS_MT_TRACKING_ID, 0, 15, 0, 0);
+
+	input->name = kstrdup (model_name, GFP_NOIO);
+	input->id.bustype = BUS_I2C;
+	input->dev.parent = &client->dev;
+
+	input_set_drvdata (input, tsdata);
+
+	tsdata->input = input;
+
+	if ((error = input_register_device (input)))
+		goto err_free_input_device;
+
+	if (request_threaded_irq (tsdata->irq, NULL, edt_ft5x06_ts_isr,
+	                          IRQF_TRIGGER_LOW | IRQF_ONESHOT,
+	                          client->name, tsdata)) {
+		dev_err (&client->dev, "Unable to request touchscreen IRQ.\n");
+		input = NULL;
+		error = -ENOMEM;
+		goto err_unregister_device;
+	}
+
+	error = sysfs_create_group (&client->dev.kobj, &edt_ft5x06_i2c_attr_group);
+	if (error)
+		goto err_free_irq;
+
+	device_init_wakeup (&client->dev, 1);
+
+	dev_info (&tsdata->client->dev,
+	          "EDT FT5x06 initialized: IRQ pin %d, Reset pin %d.\n",
+	          tsdata->irq_pin, tsdata->reset_pin);
+
+	return 0;
+
+err_free_irq:
+	free_irq (client->irq, tsdata);
+err_unregister_device:
+	input_unregister_device (input);
+err_free_input_device:
+	kfree (input->name);
+	input_free_device (input);
+err_free_irq_pin:
+	gpio_free (tsdata->irq_pin);
+err_free_reset_pin:
+	gpio_free (tsdata->reset_pin);
+err_free_tsdata:
+	kfree (tsdata);
+	return error;
+}
+
+static int edt_ft5x06_i2c_ts_remove (struct i2c_client *client)
+{
+	struct edt_ft5x06_i2c_ts_data *tsdata = dev_get_drvdata (&client->dev);
+
+	sysfs_remove_group (&client->dev.kobj, &edt_ft5x06_i2c_attr_group);
+
+	free_irq (client->irq, tsdata);
+	input_unregister_device (tsdata->input);
+	kfree (tsdata->input->name);
+	input_free_device (tsdata->input);
+	gpio_free (tsdata->irq_pin);
+	gpio_free (tsdata->reset_pin);
+	kfree (tsdata);
+
+	dev_set_drvdata (&client->dev, NULL);
+	return 0;
+}
+
+static int edt_ft5x06_i2c_ts_suspend (struct i2c_client *client, pm_message_t mesg)
+{
+	struct edt_ft5x06_i2c_ts_data *tsdata = dev_get_drvdata (&client->dev);
+
+	if (device_may_wakeup (&client->dev))
+		enable_irq_wake (tsdata->irq);
+
+	return 0;
+}
+
+static int edt_ft5x06_i2c_ts_resume (struct i2c_client *client)
+{
+	struct edt_ft5x06_i2c_ts_data *tsdata = dev_get_drvdata (&client->dev);
+
+	if (device_may_wakeup (&client->dev))
+		disable_irq_wake (tsdata->irq);
+
+	return 0;
+}
+
+static const struct i2c_device_id edt_ft5x06_i2c_ts_id[] =
+{
+	{ "edt-ft5x06", 0 },
+	{ }
+};
+MODULE_DEVICE_TABLE (i2c, edt_ft5x06_i2c_ts_id);
+
+static struct i2c_driver edt_ft5x06_i2c_ts_driver =
+{
+	.driver = {
+		.owner = THIS_MODULE,
+		.name = "edt_ft5x06_i2c",
+	},
+	.id_table = edt_ft5x06_i2c_ts_id,
+	.probe    = edt_ft5x06_i2c_ts_probe,
+	.remove   = edt_ft5x06_i2c_ts_remove,
+	.suspend  = edt_ft5x06_i2c_ts_suspend,
+	.resume   = edt_ft5x06_i2c_ts_resume,
+};
+
+static int __init edt_ft5x06_i2c_ts_init (void)
+{
+	return i2c_add_driver (&edt_ft5x06_i2c_ts_driver);
+}
+module_init (edt_ft5x06_i2c_ts_init);
+
+static void __exit edt_ft5x06_i2c_ts_exit (void)
+{
+	i2c_del_driver (&edt_ft5x06_i2c_ts_driver);
+}
+module_exit (edt_ft5x06_i2c_ts_exit);
+
+MODULE_AUTHOR ("Simon Budig <simon.budig@kernelconcepts.de>");
+MODULE_DESCRIPTION ("EDT FT5x06 I2C Touchscreen Driver");
+MODULE_LICENSE (GPL);
+
diff --git a/include/linux/input/edt-ft5x06.h b/include/linux/input/edt-ft5x06.h
new file mode 100644
index 0000000..d7fd990
--- /dev/null
+++ b/include/linux/input/edt-ft5x06.h
@@ -0,0 +1,16 @@
+#ifndef _EDT_FT5X06_H
+#define _EDT_FT5X06_H
+
+/*
+ * Copyright (c) 2011 Simon Budig, <simon.budig@kernelconcepts.de>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published by
+ * the Free Software Foundation.
+ */
+
+struct edt_ft5x06_platform_data {
+	unsigned int reset_pin;
+};
+
+#endif /* _EDT_FT5X06_H */
-- 
1.7.4.1

Re: [PATCH] Touchscreen driver for FT5x06 based EDT displays

[Simon Budig]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hi all.

On 09/26/2011 06:06 PM, simon.budig@kernelconcepts.de wrote:
> From: Simon Budig <simon@budig.de>

Gah, managed to mess up with git send-email again. Sorry for that.

This is the message that was supposed to go along with the patch:

Hi all.

This is a driver for the EDT ft5x06 based capacitive touchscreens.

The driver supports up to 5 touch points, the "gain", "threshold" and
"offset" parameters can be configured via some files in the sysfs.

It also is possible to acquire some raw data from the sensor by putting
the sensor in a factory mode ("mode" file in sysfs) and then reading the
"raw" file, which contains the raw touch data. In factory mode the
reporting of regular events will stop.

I hope this can get integrated upstream, I appreciate any hints and
suggestions regarding the code.

Thanks,
        Simon Budig

- -- 
       Simon Budig                        kernel concepts GbR
       simon.budig@kernelconcepts.de      Sieghuetter Hauptweg 48
       +49-271-771091-17                  D-57072 Siegen

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAk6ApOQACgkQO2O/RXesiHCXtgCeJ6DMV2mRJDKcJ7jbhttsvfCb
BSkAn0gNXBg7t2JYfxXoeAa8gkxRvwrr
=T2A2
-----END PGP SIGNATURE-----

Re: [PATCH] Touchscreen driver for FT5x06 based EDT displays

[Dmitry Torokhov]

Hi Simon,

On Mon, Sep 26, 2011 at 06:06:29PM +0200, simon.budig@kernelconcepts.de wrote:
> From: Simon Budig <simon@budig.de>
> 
> ---
>  drivers/input/touchscreen/Kconfig      |    5 +
>  drivers/input/touchscreen/Makefile     |    1 +
>  drivers/input/touchscreen/edt-ft5x06.c |  714 ++++++++++++++++++++++++++++++++
>  include/linux/input/edt-ft5x06.h       |   16 +
>  4 files changed, 736 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/input/touchscreen/edt-ft5x06.c
>  create mode 100644 include/linux/input/edt-ft5x06.h
> 
> diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
> index 0b28adf..4c0672c 100644
> --- a/drivers/input/touchscreen/Kconfig
> +++ b/drivers/input/touchscreen/Kconfig
> @@ -339,6 +339,11 @@ config TOUCHSCREEN_PENMOUNT
>  	  To compile this driver as a module, choose M here: the
>  	  module will be called penmount.
>  
> +config TOUCHSCREEN_EDT_FT5X06
> +	tristate "EDT FocalTech FT5x06 I2C Touchscreen support"
> +	help

A bit longer description of the touchscreen here would be nice.

> +	  If unsure, say N.
> +

"To compile this driver as a module..."

>  config TOUCHSCREEN_QT602240
>  	tristate "QT602240 I2C Touchscreen"
>  	depends on I2C
> diff --git a/drivers/input/touchscreen/Makefile b/drivers/input/touchscreen/Makefile
> index bd54dfe..1288ab6 100644
> --- a/drivers/input/touchscreen/Makefile
> +++ b/drivers/input/touchscreen/Makefile
> @@ -18,6 +18,7 @@ obj-$(CONFIG_TOUCHSCREEN_BU21013)       += bu21013_ts.o
>  obj-$(CONFIG_TOUCHSCREEN_CY8CTMG110)	+= cy8ctmg110_ts.o
>  obj-$(CONFIG_TOUCHSCREEN_DA9034)	+= da9034-ts.o
>  obj-$(CONFIG_TOUCHSCREEN_DYNAPRO)	+= dynapro.o
> +obj-$(CONFIG_TOUCHSCREEN_EDT_FT5X06)	+= edt-ft5x06.o
>  obj-$(CONFIG_TOUCHSCREEN_HAMPSHIRE)	+= hampshire.o
>  obj-$(CONFIG_TOUCHSCREEN_GUNZE)		+= gunze.o
>  obj-$(CONFIG_TOUCHSCREEN_EETI)		+= eeti_ts.o
> diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c
> new file mode 100644
> index 0000000..7e2b04b
> --- /dev/null
> +++ b/drivers/input/touchscreen/edt-ft5x06.c
> @@ -0,0 +1,714 @@
> +/* drivers/input/touchscreen/edt-ft5x06.c

No file names in comments please.

> + *
> + * Copyright (C) 2011 Simon Budig, <simon.budig@kernelconcepts.de>
> + *
> + * This software is licensed under the terms of the GNU General Public
> + * License version 2, as published by the Free Software Foundation, and
> + * may be copied, distributed, and modified under those terms.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public
> + * License along with this library; if not, write to the Free Software
> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
> + */
> +
> +/*
> + * This is a driver for the EDT "Polytouch" family of touch controllers
> + * based on the FocalTech FT5x06 line of chips.
> + *
> + * Development of this driver has been sponsored by Glyn:
> + *    http://www.glyn.com/Products/Displays
> + */
> +
> +#include <linux/module.h>
> +#include <linux/interrupt.h>
> +#include <linux/input.h>
> +#include <linux/i2c.h>
> +#include <asm/uaccess.h>
> +#include <linux/smp_lock.h>

This file is long gone from mainline.

> +#include <linux/delay.h>
> +#include <linux/slab.h>
> +
> +#include <linux/gpio.h>
> +
> +#include "linux/input/edt-ft5x06.h"

Should use <>.

> +
> +#define DRIVER_VERSION "v0.5"
> +
> +#define WORK_REGISTER_THRESHOLD   0x00
> +#define WORK_REGISTER_GAIN        0x30
> +#define WORK_REGISTER_OFFSET      0x31
> +#define WORK_REGISTER_NUM_X       0x33
> +#define WORK_REGISTER_NUM_Y       0x34
> +
> +#define WORK_REGISTER_OPMODE      0x3c
> +#define FACTORY_REGISTER_OPMODE   0x01
> +
> +static struct i2c_driver edt_ft5x06_i2c_ts_driver;

I don't think this forward declaration is needed.

> +
> +struct edt_ft5x06_i2c_ts_data
> +{
> +	struct i2c_client *client;
> +	struct input_dev *input;
> +	int irq;
> +	int irq_pin;
> +	int reset_pin;
> +	int num_x;
> +	int num_y;
> +
> +	struct mutex mutex;
> +	int factory_mode;

This looks like a bool.

> +	int threshold;
> +	int gain;
> +	int offset;
> +};
> +
> +static int edt_ft5x06_ts_readwrite (struct i2c_client *client,
> +                                    u16 wr_len, u8 *wr_buf,
> +                                    u16 rd_len, u8 *rd_buf)
> +{
> +	struct i2c_msg wrmsg[2];
> +	int i, ret;
> +
> +	i = 0;
> +	if (wr_len) {
> +		wrmsg[i].addr  = client->addr;
> +		wrmsg[i].flags = 0;
> +		wrmsg[i].len = wr_len;
> +		wrmsg[i].buf = wr_buf;
> +		i++;
> +	}
> +	if (rd_len) {
> +		wrmsg[i].addr  = client->addr;
> +		wrmsg[i].flags = I2C_M_RD;
> +		wrmsg[i].len = rd_len;
> +		wrmsg[i].buf = rd_buf;
> +		i++;
> +	}
> +
> +	ret = i2c_transfer (client->adapter, wrmsg, i);

Please no spaces between function names and opening parenthesis (the
keywords should have the space). This applies to the entire driver.

> +	if (ret < 0) {
> +		dev_info (&client->dev, "i2c_transfer failed: %d\n", ret);

Should probably be dev_err() or dev_warn() to denote proper severity.

> +		return ret;
> +	}
> +
> +	return ret;
> +}
> +
> +
> +static irqreturn_t edt_ft5x06_ts_isr (int irq, void *dev_id)
> +{
> +	struct edt_ft5x06_i2c_ts_data *tsdata = dev_id;
> +	unsigned char touching = 0;
> +	unsigned char rdbuf[26], wrbuf[1];
> +	int i, have_abs, type, ret;
> +
> +	memset (wrbuf, 0, sizeof (wrbuf));
> +	memset (rdbuf, 0, sizeof (rdbuf));
> +
> +	wrbuf[0] = 0xf9;
> +
> +	mutex_lock (&tsdata->mutex);
> +	ret = edt_ft5x06_ts_readwrite (tsdata->client,
> +	                               1, wrbuf,
> +	                               sizeof (rdbuf), rdbuf);

i2c_transfer() already provides necessary locking on adapter level so
several transfers would not race with each other. This locking seems
excessive.

> +	mutex_unlock (&tsdata->mutex);
> +	if (ret < 0) {
> +		dev_err (&tsdata->client->dev, "Unable to write to i2c touchscreen!\n");
> +		goto out;
> +	}
> +
> +	if (rdbuf[0] != 0xaa || rdbuf[1] != 0xaa || rdbuf[2] != 26) {
> +		dev_err (&tsdata->client->dev, "Unexpected header: %02x%02x%02x!\n", rdbuf[0], rdbuf[1], rdbuf[2]);
> +	}
> +
> +	have_abs = 0;
> +	touching = rdbuf[3];
> +	for (i = 0; i < touching; i++) {
> +		type = rdbuf[i*4+5] >> 6;
> +		if (type == 0x01 || type == 0x03)  /* ignore Touch Down and Reserved events */
> +			continue;
> +
> +		if (!have_abs) {
> +			input_report_key (tsdata->input, BTN_TOUCH,    1);
> +			input_report_abs (tsdata->input, ABS_PRESSURE, 1);

If the device does not report true pressure readings please do not fake
them. BTN_TOUCH alone should suffice.

> +			input_report_abs (tsdata->input, ABS_X, ((rdbuf[i*4+5] << 8) | rdbuf[i*4+6]) & 0x0fff);
> +			input_report_abs (tsdata->input, ABS_Y, ((rdbuf[i*4+7] << 8) | rdbuf[i*4+8]) & 0x0fff);
> +			have_abs = 1;
> +		}
> +		input_report_abs (tsdata->input, ABS_MT_POSITION_X, ((rdbuf[i*4+5] << 8) | rdbuf[i*4+6]) & 0x0fff);
> +		input_report_abs (tsdata->input, ABS_MT_POSITION_Y, ((rdbuf[i*4+7] << 8) | rdbuf[i*4+8]) & 0x0fff);
> +		input_report_abs (tsdata->input, ABS_MT_TRACKING_ID, (rdbuf[i*4+7] >> 4) & 0x0f);
> +		input_mt_sync (tsdata->input);

Any change to use stateful MT-B protocol here?

> +	}
> +	if (!have_abs) {
> +		input_report_key (tsdata->input, BTN_TOUCH,    0);
> +		input_report_abs (tsdata->input, ABS_PRESSURE, 0);
> +	}
> +	input_sync (tsdata->input);
> +
> +out:
> +	return IRQ_HANDLED;
> +}
> +
> +
> +static int edt_ft5x06_i2c_register_write (struct edt_ft5x06_i2c_ts_data *tsdata,
> +                                          u8 addr, u8 value)
> +{
> +	u8 wrbuf[4];
> +	int ret;
> +
> +	wrbuf[0]  = tsdata->factory_mode ? 0xf3 : 0xfc;
> +	wrbuf[1]  = tsdata->factory_mode ? addr & 0x7f : addr & 0x3f;
> +	wrbuf[2]  = value;
> +	wrbuf[3]  = wrbuf[0] ^ wrbuf[1] ^ wrbuf[2];
> +
> +	disable_irq (tsdata->irq);

Do you really need to disable IRQ here? We already established that
i2c_transefr()s won't race.

> +
> +	ret = edt_ft5x06_ts_readwrite (tsdata->client,
> +	                               4, wrbuf,
> +	                               0, NULL);
> +
> +	enable_irq (tsdata->irq);
> +
> +	return ret;
> +}
> +
> +static int edt_ft5x06_i2c_register_read (struct edt_ft5x06_i2c_ts_data *tsdata,
> +                                         u8 addr)
> +{
> +	u8 wrbuf[2], rdbuf[2];
> +	int ret;
> +
> +	wrbuf[0]  = tsdata->factory_mode ? 0xf3 : 0xfc;
> +	wrbuf[1]  = tsdata->factory_mode ? addr & 0x7f : addr & 0x3f;
> +	wrbuf[1] |= tsdata->factory_mode ? 0x80 : 0x40;
> +
> +	disable_irq (tsdata->irq);
> +
> +	ret = edt_ft5x06_ts_readwrite (tsdata->client,
> +	                               2, wrbuf,
> +	                               2, rdbuf);
> +
> +	enable_irq (tsdata->irq);
> +
> +#if 0
> +	dev_info (&tsdata->client->dev, "wr: %02x %02x -> rd: %02x %02x\n",
> +	          wrbuf[0], wrbuf[1], rdbuf[0], rdbuf[1]);
> +#endif

Either lose it or turn to dev_dbg().

> +
> +	if ((wrbuf[0] ^ wrbuf[1] ^ rdbuf[0]) != rdbuf[1])
> +		dev_err (&tsdata->client->dev, "crc error: 0x%02x expected, got 0x%02x\n",
> +		         (wrbuf[0] ^ wrbuf[1] ^ rdbuf[0]), rdbuf[1]);
> +
> +	return ret < 0 ? ret : rdbuf[0];
> +}
> +
> +static ssize_t edt_ft5x06_i2c_setting_show (struct device *dev,
> +                                            struct device_attribute *attr,
> +                                            char *buf)
> +{
> +	struct edt_ft5x06_i2c_ts_data *tsdata = dev_get_drvdata (dev);
> +	struct i2c_client *client = tsdata->client;
> +	int ret = 0;
> +	int *value;
> +	u8 addr;
> +
> +	switch (attr->attr.name[0]) {
> +		case 't':    /* threshold */

case label should be aligned with switch().

> +			addr = WORK_REGISTER_THRESHOLD;
> +			value = &tsdata->threshold;
> +			break;
> +		case 'g':    /* gain */
> +			addr = WORK_REGISTER_GAIN;
> +			value = &tsdata->gain;
> +			break;
> +		case 'o':    /* offset */
> +			addr = WORK_REGISTER_OFFSET;
> +			value = &tsdata->offset;
> +			break;
> +		default:
> +			dev_err (&client->dev, "unknown attribute for edt_ft5x06_i2c_setting_show: %s\n", attr->attr.name);
> +			return -EINVAL;
> +	}
> +
> +	mutex_lock (&tsdata->mutex);
> +
> +	if (tsdata->factory_mode == 1) {
> +		dev_err (dev, "setting register not available in factory mode\n");

You just left with mutex locked. Mutex is most likely not needed at all.

> +		return -EIO;
> +	}
> +
> +	ret = edt_ft5x06_i2c_register_read (tsdata, addr);
> +	if (ret < 0) {
> +		dev_err (&tsdata->client->dev, "Unable to write to i2c touchscreen!\n");
> +		mutex_unlock (&tsdata->mutex);
> +		return ret;
> +	}
> +	mutex_unlock (&tsdata->mutex);
> +
> +	if (ret != *value) {
> +		dev_info (&tsdata->client->dev, "i2c read (%d) and stored value (%d) differ. Huh?\n", ret, *value);
> +		*value = ret;
> +	}
> +
> +	return sprintf (buf, "%d\n", ret);
> +}
> +
> +static ssize_t edt_ft5x06_i2c_setting_store (struct device *dev,
> +                                             struct device_attribute *attr,
> +                                             const char *buf, size_t count)
> +{
> +	struct edt_ft5x06_i2c_ts_data *tsdata = dev_get_drvdata (dev);
> +	struct i2c_client *client = tsdata->client;
> +	int ret = 0;
> +	u8 addr;
> +	unsigned int val;
> +
> +	mutex_lock (&tsdata->mutex);
> +
> +	if (tsdata->factory_mode == 1) {
> +		dev_err (dev, "setting register not available in factory mode\n");
> +		ret = -EIO;
> +		goto out;
> +	}
> +
> +	if (sscanf(buf, "%u", &val) != 1) {
> +		dev_err (dev, "Invalid value for attribute %s\n", attr->attr.name);
> +		ret = -EINVAL;
> +		goto out;
> +	}
> +
> +	switch (attr->attr.name[0]) {
> +		case 't':    /* threshold */
> +			addr = WORK_REGISTER_THRESHOLD;
> +			val = val < 20 ? 20 : val > 80 ? 80 : val;
> +			tsdata->threshold = val;
> +			break;
> +		case 'g':    /* gain */
> +			addr = WORK_REGISTER_GAIN;
> +			val = val < 0 ? 0 : val > 31 ? 31 : val;
> +			tsdata->gain = val;
> +			break;
> +		case 'o':    /* offset */
> +			addr = WORK_REGISTER_OFFSET;
> +			val = val < 0 ? 0 : val > 31 ? 31 : val;
> +			tsdata->offset = val;
> +			break;
> +		default:
> +			dev_err (&client->dev, "unknown attribute for edt_ft5x06_i2c_setting_show: %s\n", attr->attr.name);
> +			ret = -EINVAL;
> +			goto out;
> +	}
> +
> +	ret = edt_ft5x06_i2c_register_write (tsdata, addr, val);
> +
> +	if (ret < 0) {
> +		dev_err (&tsdata->client->dev, "Unable to write to i2c touchscreen!\n");
> +		goto out;
> +	}
> +
> +	mutex_unlock (&tsdata->mutex);
> +	return count;
> +
> +out:
> +	mutex_unlock (&tsdata->mutex);
> +	return ret;

Just use "return  ret < 0 ? ret : count;" and have single mutex_unlock()
path (if locking is needed).


> +}
> +
> +
> +static ssize_t edt_ft5x06_i2c_mode_show (struct device *dev,
> +                                         struct device_attribute *attr,
> +                                         char *buf)
> +{
> +	struct edt_ft5x06_i2c_ts_data *tsdata = dev_get_drvdata (dev);
> +	return sprintf (buf, "%d\n", tsdata->factory_mode);
> +}
> +
> +static ssize_t edt_ft5x06_i2c_mode_store (struct device *dev,
> +                                          struct device_attribute *attr,
> +                                          const char *buf, size_t count)
> +{
> +	struct edt_ft5x06_i2c_ts_data *tsdata = dev_get_drvdata (dev);
> +	int i, ret = 0;
> +	unsigned int mode;
> +
> +	if (sscanf(buf, "%u", &mode) != 1 || (mode | 1) != 1) {
> +		dev_err (dev, "Invalid value for operation mode\n");
> +		return -EINVAL;
> +	}
> +
> +	/* no change, return without doing anything */
> +	if (mode == tsdata->factory_mode)
> +		return count;
> +
> +	mutex_lock (&tsdata->mutex);
> +	if (tsdata->factory_mode == 0) { /* switch to factory mode */
> +		disable_irq (tsdata->irq);
> +		/* mode register is 0x3c when in the work mode */
> +		ret = edt_ft5x06_i2c_register_write (tsdata, WORK_REGISTER_OPMODE, 0x03);
> +		if (ret < 0) {
> +			dev_err (dev, "failed to switch to factory mode (%d)\n", ret);
> +		} else {
> +			tsdata->factory_mode = 1;
> +			for (i = 0; i < 10; i++) {
> +				mdelay (5);
> +				/* mode register is 0x01 when in the factory mode */
> +				ret = edt_ft5x06_i2c_register_read (tsdata, FACTORY_REGISTER_OPMODE);
> +				if (ret == 0x03)
> +					break;
> +			}
> +			if (i == 10)
> +				dev_err (dev, "not in factory mode after %dms.\n", i*5);
> +		}
> +	} else {  /* switch to work mode */
> +		/* mode register is 0x01 when in the factory mode */
> +		ret = edt_ft5x06_i2c_register_write (tsdata, FACTORY_REGISTER_OPMODE, 0x01);
> +		if (ret < 0) {
> +			dev_err (dev, "failed to switch to work mode (%d)\n", ret);
> +		} else {
> +			tsdata->factory_mode = 0;
> +			for (i = 0; i < 10; i++) {
> +				mdelay (5);
> +				/* mode register is 0x01 when in the factory mode */
> +				ret = edt_ft5x06_i2c_register_read (tsdata, WORK_REGISTER_OPMODE);
> +				if (ret == 0x01)
> +					break;
> +			}
> +			if (i == 10)
> +				dev_err (dev, "not in work mode after %dms.\n", i*5);
> +
> +			/* restore parameters */
> +			edt_ft5x06_i2c_register_write (tsdata, WORK_REGISTER_THRESHOLD, tsdata->threshold);
> +			edt_ft5x06_i2c_register_write (tsdata, WORK_REGISTER_GAIN, tsdata->gain);
> +			edt_ft5x06_i2c_register_write (tsdata, WORK_REGISTER_OFFSET, tsdata->offset);
> +
> +			enable_irq (tsdata->irq);
> +		}
> +	}
> +
> +	mutex_unlock (&tsdata->mutex);
> +	return count;
> +}
> +
> +
> +static ssize_t edt_ft5x06_i2c_raw_data_show (struct device *dev,
> +                                             struct device_attribute *attr,
> +                                             char *buf)
> +{
> +	struct edt_ft5x06_i2c_ts_data *tsdata = dev_get_drvdata (dev);
> +	int i, ret;
> +	char *ptr, wrbuf[3];
> +
> +	if (tsdata->factory_mode == 0) {
> +		dev_err (dev, "raw data not available in work mode\n");
> +		return -EIO;
> +	}
> +
> +	mutex_lock (&tsdata->mutex);
> +	ret = edt_ft5x06_i2c_register_write (tsdata, 0x08, 0x01);
> +	for (i = 0; i < 100; i++) {
> +		ret = edt_ft5x06_i2c_register_read (tsdata, 0x08);
> +		if (ret < 1)
> +			break;
> +		udelay (1000);
> +	}
> +
> +	if (i == 100 || ret < 0) {
> +		dev_err (dev, "waiting time exceeded or error: %d\n", ret);
> +		mutex_unlock (&tsdata->mutex);
> +		return ret || ETIMEDOUT;

-ENOPARSE.

> +	}
> +
> +	ptr = buf;
> +	wrbuf[0] = 0xf5;
> +	wrbuf[1] = 0x0e;
> +	for (i = 0; i <= tsdata->num_x; i++) {
> +		wrbuf[2] = i;
> +		ret = edt_ft5x06_ts_readwrite (tsdata->client,
> +		                               3, wrbuf,
> +		                               tsdata->num_y * 2, ptr);
> +		if (ret < 0) {
> +			mutex_unlock (&tsdata->mutex);
> +			return ret;
> +		}
> +
> +		ptr += tsdata->num_y * 2;
> +	}
> +
> +	mutex_unlock (&tsdata->mutex);
> +	return ptr - buf;
> +}
> +
> +
> +static DEVICE_ATTR(gain,      0664, edt_ft5x06_i2c_setting_show, edt_ft5x06_i2c_setting_store);
> +static DEVICE_ATTR(offset,    0664, edt_ft5x06_i2c_setting_show, edt_ft5x06_i2c_setting_store);
> +static DEVICE_ATTR(threshold, 0664, edt_ft5x06_i2c_setting_show, edt_ft5x06_i2c_setting_store);
> +static DEVICE_ATTR(mode,      0664, edt_ft5x06_i2c_mode_show, edt_ft5x06_i2c_mode_store);
> +static DEVICE_ATTR(raw_data,  0444, edt_ft5x06_i2c_raw_data_show, NULL);
> +
> +static struct attribute *edt_ft5x06_i2c_attrs[] = {
> +	&dev_attr_gain.attr,
> +	&dev_attr_offset.attr,
> +	&dev_attr_threshold.attr,
> +	&dev_attr_mode.attr,
> +	&dev_attr_raw_data.attr,
> +	NULL
> +};
> +
> +static const struct attribute_group edt_ft5x06_i2c_attr_group = {
> +	.attrs = edt_ft5x06_i2c_attrs,
> +};
> +
> +static int edt_ft5x06_i2c_ts_probe (struct i2c_client *client,
> +                                    const struct i2c_device_id *id)
> +{
> +
> +	struct edt_ft5x06_i2c_ts_data *tsdata;
> +	struct input_dev *input;
> +	int error;
> +	u8 rdbuf[23];
> +	char *model_name = NULL;
> +	char *fw_version = NULL;

Why is this initialization needed?

> +
> +	dev_info (&client->dev, "probing for EDT FT5x06 I2C\n");

dev_dbg();

> +
> +	if (!client->irq) {
> +		dev_dbg (&client->dev, "no IRQ?\n");
> +		return -ENODEV;
> +	}
> +
> +	if (!client->dev.platform_data) {
> +		dev_dbg (&client->dev, "no reset pin in platform data?\n");

The message does not really match the code.

> +		return -ENODEV;
> +	}
> +
> +	tsdata = kzalloc (sizeof (*tsdata), GFP_KERNEL);
> +	if (!tsdata) {
> +		dev_err (&client->dev, "failed to allocate driver data!\n");
> +		dev_set_drvdata (&client->dev, NULL);
> +		return -ENOMEM;
> +	}
> +
> +	dev_set_drvdata (&client->dev, tsdata);
> +	tsdata->client = client;
> +
> +	tsdata->reset_pin = ((struct edt_ft5x06_platform_data *) client->dev.platform_data)->reset_pin;

Just have a temp for it.

> +	mutex_init (&tsdata->mutex);
> +
> +	error = gpio_request (tsdata->reset_pin, NULL);
> +	if (error < 0) {
> +		dev_err (&client->dev,
> +		         "Failed to request GPIO %d as reset pin, error %d\n",
> +		         tsdata->reset_pin, error);
> +		error = -ENOMEM;

Why -ENOMEM? Return the error that gpio_request() gave you.

> +		goto err_free_tsdata;
> +	}
> +
> +	/* this pulls reset down, enabling the low active reset */
> +	if (gpio_direction_output (tsdata->reset_pin, 0) < 0) {
> +		dev_info (&client->dev, "switching to output failed\n");
> +		error = -ENOMEM;
> +		goto err_free_reset_pin;
> +	}
> +
> +	/* request IRQ pin */
> +	tsdata->irq = client->irq;
> +	tsdata->irq_pin = irq_to_gpio (tsdata->irq);
> +
> +	error = gpio_request (tsdata->irq_pin, NULL);
> +	if (error < 0) {
> +		dev_err (&client->dev,
> +		         "Failed to request GPIO %d for IRQ %d, error %d\n",
> +		         tsdata->irq_pin, tsdata->irq, error);
> +		error = -ENOMEM;

Why -ENOMEM? Return the error that gpio_request() gave you.

> +		goto err_free_reset_pin;
> +	}
> +	gpio_direction_input (tsdata->irq_pin);
> +
> +	/* release reset */
> +	mdelay (50);
> +	gpio_set_value (tsdata->reset_pin, 1);
> +	mdelay (100);
> +
> +	mutex_lock (&tsdata->mutex);
> +
> +	tsdata->factory_mode = 0;
> +
> +	if (edt_ft5x06_ts_readwrite (client, 1, "\xbb", 22, rdbuf) < 0) {
> +		dev_err (&client->dev, "probing failed\n");
> +		error = -ENODEV;
> +		goto err_free_irq_pin;
> +	}
> +
> +	rdbuf[22] = '\0';
> +	if (rdbuf[21] == '$')
> +		rdbuf[21] = '\0';
> +
> +	model_name = rdbuf + 1;
> +	fw_version = rdbuf;
> +	/* look for Model/Version separator */
> +	while (fw_version[0] != '\0' && fw_version[0] != '*')
> +		fw_version++;

strchr()?

> +
> +	tsdata->threshold = edt_ft5x06_i2c_register_read (tsdata, WORK_REGISTER_THRESHOLD);
> +	tsdata->gain      = edt_ft5x06_i2c_register_read (tsdata, WORK_REGISTER_GAIN);
> +	tsdata->offset    = edt_ft5x06_i2c_register_read (tsdata, WORK_REGISTER_OFFSET);
> +	tsdata->num_x     = edt_ft5x06_i2c_register_read (tsdata, WORK_REGISTER_NUM_X);
> +	tsdata->num_y     = edt_ft5x06_i2c_register_read (tsdata, WORK_REGISTER_NUM_Y);
> +
> +	mutex_unlock (&tsdata->mutex);
> +
> +	if (fw_version[0] == '*') {
> +		fw_version[0] = '\0';
> +		fw_version++;
> +		dev_info (&client->dev, "Model \"%s\", Rev. \"%s\", %dx%d sensors\n",
> +		          model_name, fw_version, tsdata->num_x, tsdata->num_y);
> +	} else {
> +		dev_info (&client->dev, "Product ID \"%s\"\n", rdbuf + 1);
> +	}
> +
> +	input = input_allocate_device ();
> +	if (!input) {
> +		dev_err (&client->dev, "failed to allocate input device!\n");
> +		error = -ENOMEM;
> +		goto err_free_irq_pin;
> +	}
> +
> +	set_bit (EV_SYN, input->evbit);
> +	set_bit (EV_KEY, input->evbit);
> +	set_bit (EV_ABS, input->evbit);
> +	set_bit (BTN_TOUCH, input->keybit);

__set_bit() please, no need for locked instructions.

> +	input_set_abs_params (input, ABS_X, 0, tsdata->num_x * 64 - 1, 0, 0);
> +	input_set_abs_params (input, ABS_Y, 0, tsdata->num_y * 64 - 1, 0, 0);
> +	input_set_abs_params (input, ABS_PRESSURE, 0, 1, 0, 0);
> +	input_set_abs_params (input, ABS_MT_POSITION_X, 0, tsdata->num_x * 64 - 1, 0, 0);
> +	input_set_abs_params (input, ABS_MT_POSITION_Y, 0, tsdata->num_y * 64 - 1, 0, 0);
> +	input_set_abs_params (input, ABS_MT_TRACKING_ID, 0, 15, 0, 0);
> +
> +	input->name = kstrdup (model_name, GFP_NOIO);
> +	input->id.bustype = BUS_I2C;
> +	input->dev.parent = &client->dev;
> +
> +	input_set_drvdata (input, tsdata);
> +
> +	tsdata->input = input;
> +
> +	if ((error = input_register_device (input)))
> +		goto err_free_input_device;
> +
> +	if (request_threaded_irq (tsdata->irq, NULL, edt_ft5x06_ts_isr,
> +	                          IRQF_TRIGGER_LOW | IRQF_ONESHOT,
> +	                          client->name, tsdata)) {
> +		dev_err (&client->dev, "Unable to request touchscreen IRQ.\n");
> +		input = NULL;
> +		error = -ENOMEM;
> +		goto err_unregister_device;
> +	}
> +
> +	error = sysfs_create_group (&client->dev.kobj, &edt_ft5x06_i2c_attr_group);
> +	if (error)
> +		goto err_free_irq;
> +
> +	device_init_wakeup (&client->dev, 1);
> +
> +	dev_info (&tsdata->client->dev,
> +	          "EDT FT5x06 initialized: IRQ pin %d, Reset pin %d.\n",
> +	          tsdata->irq_pin, tsdata->reset_pin);

dev_dbg() at most.

> +
> +	return 0;
> +
> +err_free_irq:
> +	free_irq (client->irq, tsdata);
> +err_unregister_device:
> +	input_unregister_device (input);
> +err_free_input_device:
> +	kfree (input->name);
> +	input_free_device (input);

Calls to input_free_device() are prohibited after calling
input_unregister_device().

> +err_free_irq_pin:
> +	gpio_free (tsdata->irq_pin);
> +err_free_reset_pin:
> +	gpio_free (tsdata->reset_pin);
> +err_free_tsdata:
> +	kfree (tsdata);
> +	return error;
> +}
> +
> +static int edt_ft5x06_i2c_ts_remove (struct i2c_client *client)
> +{
> +	struct edt_ft5x06_i2c_ts_data *tsdata = dev_get_drvdata (&client->dev);
> +
> +	sysfs_remove_group (&client->dev.kobj, &edt_ft5x06_i2c_attr_group);
> +
> +	free_irq (client->irq, tsdata);
> +	input_unregister_device (tsdata->input);

tsdata->input is most likely already freed here.

> +	kfree (tsdata->input->name);

So this is use after free.

> +	input_free_device (tsdata->input);

As is this.

> +	gpio_free (tsdata->irq_pin);
> +	gpio_free (tsdata->reset_pin);
> +	kfree (tsdata);
> +
> +	dev_set_drvdata (&client->dev, NULL);

Not needed in mainline.

> +	return 0;
> +}
> +
> +static int edt_ft5x06_i2c_ts_suspend (struct i2c_client *client, pm_message_t mesg)
> +{
> +	struct edt_ft5x06_i2c_ts_data *tsdata = dev_get_drvdata (&client->dev);
> +
> +	if (device_may_wakeup (&client->dev))
> +		enable_irq_wake (tsdata->irq);
> +
> +	return 0;
> +}
> +
> +static int edt_ft5x06_i2c_ts_resume (struct i2c_client *client)
> +{
> +	struct edt_ft5x06_i2c_ts_data *tsdata = dev_get_drvdata (&client->dev);
> +
> +	if (device_may_wakeup (&client->dev))
> +		disable_irq_wake (tsdata->irq);
> +
> +	return 0;
> +}
> +
> +static const struct i2c_device_id edt_ft5x06_i2c_ts_id[] =
> +{
> +	{ "edt-ft5x06", 0 },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE (i2c, edt_ft5x06_i2c_ts_id);
> +
> +static struct i2c_driver edt_ft5x06_i2c_ts_driver =
> +{
> +	.driver = {
> +		.owner = THIS_MODULE,
> +		.name = "edt_ft5x06_i2c",
> +	},
> +	.id_table = edt_ft5x06_i2c_ts_id,
> +	.probe    = edt_ft5x06_i2c_ts_probe,
> +	.remove   = edt_ft5x06_i2c_ts_remove,
> +	.suspend  = edt_ft5x06_i2c_ts_suspend,
> +	.resume   = edt_ft5x06_i2c_ts_resume,
> +};
> +
> +static int __init edt_ft5x06_i2c_ts_init (void)
> +{
> +	return i2c_add_driver (&edt_ft5x06_i2c_ts_driver);
> +}
> +module_init (edt_ft5x06_i2c_ts_init);
> +
> +static void __exit edt_ft5x06_i2c_ts_exit (void)
> +{
> +	i2c_del_driver (&edt_ft5x06_i2c_ts_driver);
> +}
> +module_exit (edt_ft5x06_i2c_ts_exit);
> +
> +MODULE_AUTHOR ("Simon Budig <simon.budig@kernelconcepts.de>");
> +MODULE_DESCRIPTION ("EDT FT5x06 I2C Touchscreen Driver");
> +MODULE_LICENSE (GPL);
> +
> diff --git a/include/linux/input/edt-ft5x06.h b/include/linux/input/edt-ft5x06.h
> new file mode 100644
> index 0000000..d7fd990
> --- /dev/null
> +++ b/include/linux/input/edt-ft5x06.h
> @@ -0,0 +1,16 @@
> +#ifndef _EDT_FT5X06_H
> +#define _EDT_FT5X06_H
> +
> +/*
> + * Copyright (c) 2011 Simon Budig, <simon.budig@kernelconcepts.de>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License version 2 as published by
> + * the Free Software Foundation.
> + */
> +
> +struct edt_ft5x06_platform_data {
> +	unsigned int reset_pin;
> +};
> +
> +#endif /* _EDT_FT5X06_H */


Thanks.

-- 
Dmitry

Re: [PATCH] Touchscreen driver for FT5x06 based EDT displays

[Simon Budig]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hi Dmitry

Thanks for your review. I used a lot of your hints.

Sorry, the patch was developed for an embedded platform which
unfortunately still is stuck on 2.6.38. I now have adapted the patch for
current GIT head, but I could not yet test it properly (for the lack of
the proper kernel for my testhardware). So the following patch should
probably be taken with a grain of salt.

On 09/28/2011 07:47 AM, Dmitry Torokhov wrote:
>> +	mutex_lock (&tsdata->mutex);
>> +	ret = edt_ft5x06_ts_readwrite (tsdata->client,
>> +	                               1, wrbuf,
>> +	                               sizeof (rdbuf), rdbuf);
> 
> i2c_transfer() already provides necessary locking on adapter level so
> several transfers would not race with each other. This locking seems
> excessive.

The reason for this locking is two fold.

First, the touch sensor has two modes, a "operation mode" and a "factory
mode". The problem is, that the register numbering in the two modes is
mostly disjunct. I.e. reading the "gain" register in factory mode gives
a number, which has no real connection to the actual gain value. Since
the mode can be changed via a sysfs file I have a potential race
condition when e.g. concurrent access to the sysfs entries happen:

Lets say I want to read the gain value, while something else tries to
switch to factory mode.

1) edt_ft5x06_i2c_setting_show checks for factory_mode == 0
2) edt_ft5x06_i2c_mode_store changes factory_mode to 1
3) edt_ft5x06_i2c_setting_show reads register 0x30, reading and
returning a bogus value.

Also reading raw values is a series of i2c transactions (for each column
of the sensor) and I need to avoid at least a mode change inbetween.
That is the purpose of the mutex.

>> +			input_report_abs (tsdata->input, ABS_PRESSURE, 1);
> 
> If the device does not report true pressure readings please do not fake
> them. BTN_TOUCH alone should suffice.

Hum, ok. I need to check again, but at some point tslib had a problem
with a missing ABS_PRESSURE axis. Yeah, this would need fixing in tslib
obviously. Not sure what the current state of this problem is though.

>> +		input_report_abs (tsdata->input, ABS_MT_POSITION_X, ((rdbuf[i*4+5] << 8) | rdbuf[i*4+6]) & 0x0fff);
>> +		input_report_abs (tsdata->input, ABS_MT_POSITION_Y, ((rdbuf[i*4+7] << 8) | rdbuf[i*4+8]) & 0x0fff);
>> +		input_report_abs (tsdata->input, ABS_MT_TRACKING_ID, (rdbuf[i*4+7] >> 4) & 0x0f);
>> +		input_mt_sync (tsdata->input);
> 
> Any change to use stateful MT-B protocol here?

I will look into it. For now the application software expects MT-A,
although changing this shouldnt be too hard.

>> +	disable_irq (tsdata->irq);
> 
> Do you really need to disable IRQ here? We already established that
> i2c_transefr()s won't race.

I need to think about that. Not sure at the moment.

>> +	mutex_lock (&tsdata->mutex);
>> +
>> +	if (tsdata->factory_mode == 1) {
>> +		dev_err (dev, "setting register not available in factory mode\n");
> 
> You just left with mutex locked. Mutex is most likely not needed at all.

Oops, thanks.

>> +static ssize_t edt_ft5x06_i2c_raw_data_show (struct device *dev,
>> +                                             struct device_attribute *attr,
>> +                                             char *buf)
>> +{
>> +	struct edt_ft5x06_i2c_ts_data *tsdata = dev_get_drvdata (dev);
>> +	int i, ret;
>> +	char *ptr, wrbuf[3];
>> +
>> +	if (tsdata->factory_mode == 0) {
>> +		dev_err (dev, "raw data not available in work mode\n");
>> +		return -EIO;
>> +	}
>> +
>> +	mutex_lock (&tsdata->mutex);
>> +	ret = edt_ft5x06_i2c_register_write (tsdata, 0x08, 0x01);
>> +	for (i = 0; i < 100; i++) {
>> +		ret = edt_ft5x06_i2c_register_read (tsdata, 0x08);
>> +		if (ret < 1)
>> +			break;
>> +		udelay (1000);
>> +	}
>> +
>> +	if (i == 100 || ret < 0) {
>> +		dev_err (dev, "waiting time exceeded or error: %d\n", ret);
>> +		mutex_unlock (&tsdata->mutex);
>> +		return ret || ETIMEDOUT;
> 
> -ENOPARSE.

Ok, maybe this is written a bit convoluted.

I need to trigger the readout of the raw values (the register_write) and
then wait until the raw values have been prepared. That is the loop
which waits up to 100 msecs. The loop ends when the register_read either
returns an error (< 0) or the register value has changed to 0 (== 0).

If ret is < 0 (an error happened) or I ended up doing 100 iterations of
the loop (the timeout) I return either the error or ETIMEDOUT.

...at least that was the intention. I just realize there indeed is a
bug, the return statement is bogus. I have adressed this in the next
iteration.

>> +
>> +	return 0;
>> +
>> +err_free_irq:
>> +	free_irq (client->irq, tsdata);
>> +err_unregister_device:
>> +	input_unregister_device (input);
>> +err_free_input_device:
>> +	kfree (input->name);
>> +	input_free_device (input);
> 
> Calls to input_free_device() are prohibited after calling
> input_unregister_device().

Hmm, Ok. I missed that other modules set the input pointer to NULL in
their cleanup path.

I will change that, the updated patch follows soon.

Thanks,
        Simon
- -- 
       Simon Budig                        kernel concepts GbR
       simon.budig@kernelconcepts.de      Sieghuetter Hauptweg 48
       +49-271-771091-17                  D-57072 Siegen

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAk6EktEACgkQO2O/RXesiHDWrgCfUb7hZ5v+Fs4/js92QfRGbj8o
aFQAn3MENdFAM503X1QE0dMppnnxrDQh
=EeeG
-----END PGP SIGNATURE-----

[PATCH v3] Touchscreen driver for FT5x06 based EDT displays

[simon.budig]

This is a driver for the EDT ft5x06 based capacitive touchscreens.

The driver supports up to 5 touch points, the "gain", "threshold" and "offset"
parameters can be configured via some files in the sysfs.

It also is possible to acquire some raw data from the sensor by putting the
sensor in a factory mode ("mode" file in sysfs) and then reading the "raw" 
file, which contains the raw touch data. In factory mode the reporting of
regular events will stop.

This is the 3rd iteration of the patch, based on the head of Linus' kernel
tree.

Thanks,
        Simon Budig

[PATCH] Touchscreen driver for FT5x06 based EDT displays

[simon.budig]

From: Simon Budig <simon.budig@kernelconcepts.de>

This is a driver for the EDT "Polytouch" family of touch controllers
based on the FocalTech FT5x06 line of chips.

Signed-off-by: Simon Budig <simon.budig@kernelconcepts.de>
---
 drivers/input/touchscreen/Kconfig      |   12 +
 drivers/input/touchscreen/Makefile     |    1 +
 drivers/input/touchscreen/edt-ft5x06.c |  757 ++++++++++++++++++++++++++++++++
 include/linux/input/edt-ft5x06.h       |   17 +
 4 files changed, 787 insertions(+), 0 deletions(-)
 create mode 100644 drivers/input/touchscreen/edt-ft5x06.c
 create mode 100644 include/linux/input/edt-ft5x06.h

diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
index 4af2a18..56efc99 100644
--- a/drivers/input/touchscreen/Kconfig
+++ b/drivers/input/touchscreen/Kconfig
@@ -386,6 +386,18 @@ config TOUCHSCREEN_PENMOUNT
 	  To compile this driver as a module, choose M here: the
 	  module will be called penmount.
 
+config TOUCHSCREEN_EDT_FT5X06
+	tristate "EDT FocalTech FT5x06 I2C Touchscreen support"
+	help
+	  Say Y here if you have an EDT "Polytouch" touchscreen based
+	  on the FocalTech FT5x06 family of controllers connected to
+	  your system.
+
+	  If unsure, say N.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called edt-ft5x06.
+
 config TOUCHSCREEN_MIGOR
 	tristate "Renesas MIGO-R touchscreen"
 	depends on SH_MIGOR && I2C
diff --git a/drivers/input/touchscreen/Makefile b/drivers/input/touchscreen/Makefile
index 496091e..f030dd2 100644
--- a/drivers/input/touchscreen/Makefile
+++ b/drivers/input/touchscreen/Makefile
@@ -20,6 +20,7 @@ obj-$(CONFIG_TOUCHSCREEN_BU21013)       += bu21013_ts.o
 obj-$(CONFIG_TOUCHSCREEN_CY8CTMG110)	+= cy8ctmg110_ts.o
 obj-$(CONFIG_TOUCHSCREEN_DA9034)	+= da9034-ts.o
 obj-$(CONFIG_TOUCHSCREEN_DYNAPRO)	+= dynapro.o
+obj-$(CONFIG_TOUCHSCREEN_EDT_FT5X06)	+= edt-ft5x06.o
 obj-$(CONFIG_TOUCHSCREEN_HAMPSHIRE)	+= hampshire.o
 obj-$(CONFIG_TOUCHSCREEN_GUNZE)		+= gunze.o
 obj-$(CONFIG_TOUCHSCREEN_EETI)		+= eeti_ts.o
diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c
new file mode 100644
index 0000000..a941fa7
--- /dev/null
+++ b/drivers/input/touchscreen/edt-ft5x06.c
@@ -0,0 +1,757 @@
+/*
+ * Copyright (C) 2011 Simon Budig, <simon.budig@kernelconcepts.de>
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ */
+
+/*
+ * This is a driver for the EDT "Polytouch" family of touch controllers
+ * based on the FocalTech FT5x06 line of chips.
+ *
+ * Development of this driver has been sponsored by Glyn:
+ *    http://www.glyn.com/Products/Displays
+ */
+
+#include <linux/module.h>
+#include <linux/interrupt.h>
+#include <linux/input.h>
+#include <linux/i2c.h>
+#include <linux/uaccess.h>
+#include <linux/delay.h>
+#include <linux/slab.h>
+
+#include <linux/gpio.h>
+
+#include <linux/input/edt-ft5x06.h>
+
+#define DRIVER_VERSION "v0.5"
+
+#define WORK_REGISTER_THRESHOLD   0x00
+#define WORK_REGISTER_GAIN        0x30
+#define WORK_REGISTER_OFFSET      0x31
+#define WORK_REGISTER_NUM_X       0x33
+#define WORK_REGISTER_NUM_Y       0x34
+
+#define WORK_REGISTER_OPMODE      0x3c
+#define FACTORY_REGISTER_OPMODE   0x01
+
+struct edt_ft5x06_i2c_ts_data {
+	struct i2c_client *client;
+	struct input_dev *input;
+	int irq;
+	int irq_pin;
+	int reset_pin;
+	int num_x;
+	int num_y;
+
+	struct mutex mutex;
+	bool factory_mode;
+	int threshold;
+	int gain;
+	int offset;
+};
+
+static int edt_ft5x06_ts_readwrite(struct i2c_client *client,
+                                   u16 wr_len, u8 *wr_buf,
+                                   u16 rd_len, u8 *rd_buf)
+{
+	struct i2c_msg wrmsg[2];
+	int i, ret;
+
+	i = 0;
+	if (wr_len) {
+		wrmsg[i].addr  = client->addr;
+		wrmsg[i].flags = 0;
+		wrmsg[i].len = wr_len;
+		wrmsg[i].buf = wr_buf;
+		i++;
+	}
+	if (rd_len) {
+		wrmsg[i].addr  = client->addr;
+		wrmsg[i].flags = I2C_M_RD;
+		wrmsg[i].len = rd_len;
+		wrmsg[i].buf = rd_buf;
+		i++;
+	}
+
+	ret = i2c_transfer(client->adapter, wrmsg, i);
+	if (ret < 0) {
+		dev_err(&client->dev, "i2c_transfer failed: %d\n", ret);
+		return ret;
+	}
+
+	return ret;
+}
+
+
+static irqreturn_t edt_ft5x06_ts_isr(int irq, void *dev_id)
+{
+	struct edt_ft5x06_i2c_ts_data *tsdata = dev_id;
+	unsigned char touching = 0;
+	unsigned char rdbuf[26], wrbuf[1];
+	int i, have_abs, type, ret;
+
+	memset(wrbuf, 0, sizeof(wrbuf));
+	memset(rdbuf, 0, sizeof(rdbuf));
+
+	wrbuf[0] = 0xf9;
+
+	mutex_lock(&tsdata->mutex);
+	ret = edt_ft5x06_ts_readwrite(tsdata->client,
+	                              1, wrbuf,
+	                              sizeof(rdbuf), rdbuf);
+	mutex_unlock(&tsdata->mutex);
+	if (ret < 0) {
+		dev_err(&tsdata->client->dev,
+		        "Unable to write to i2c touchscreen!\n");
+		goto out;
+	}
+
+	if (rdbuf[0] != 0xaa || rdbuf[1] != 0xaa || rdbuf[2] != 26) {
+		dev_err(&tsdata->client->dev,
+		        "Unexpected header: %02x%02x%02x!\n",
+		        rdbuf[0], rdbuf[1], rdbuf[2]);
+	}
+
+	have_abs = 0;
+	touching = rdbuf[3];
+	for (i = 0; i < touching; i++) {
+		type = rdbuf[i*4+5] >> 6;
+		/* ignore Touch Down and Reserved events */
+		if (type == 0x01 || type == 0x03)
+			continue;
+
+		if (!have_abs) {
+			input_report_key(tsdata->input, BTN_TOUCH,    1);
+			input_report_abs(tsdata->input, ABS_PRESSURE, 1);
+			input_report_abs(tsdata->input, ABS_X,
+			                 ((rdbuf[i*4+5] << 8) |
+			                  rdbuf[i*4+6]) & 0x0fff);
+			input_report_abs(tsdata->input, ABS_Y,
+			                 ((rdbuf[i*4+7] << 8) |
+			                  rdbuf[i*4+8]) & 0x0fff);
+			have_abs = 1;
+		}
+		input_report_abs(tsdata->input, ABS_MT_POSITION_X,
+		                 ((rdbuf[i*4+5] << 8) | rdbuf[i*4+6]) & 0x0fff);
+		input_report_abs(tsdata->input, ABS_MT_POSITION_Y,
+		                 ((rdbuf[i*4+7] << 8) | rdbuf[i*4+8]) & 0x0fff);
+		input_report_abs(tsdata->input, ABS_MT_TRACKING_ID,
+		                 (rdbuf[i*4+7] >> 4) & 0x0f);
+		input_mt_sync(tsdata->input);
+	}
+	if (!have_abs) {
+		input_report_key(tsdata->input, BTN_TOUCH,    0);
+		input_report_abs(tsdata->input, ABS_PRESSURE, 0);
+	}
+	input_sync(tsdata->input);
+
+out:
+	return IRQ_HANDLED;
+}
+
+
+static int edt_ft5x06_i2c_register_write(struct edt_ft5x06_i2c_ts_data *tsdata,
+                                         u8 addr, u8 value)
+{
+	u8 wrbuf[4];
+	int ret;
+
+	wrbuf[0]  = tsdata->factory_mode ? 0xf3 : 0xfc;
+	wrbuf[1]  = tsdata->factory_mode ? addr & 0x7f : addr & 0x3f;
+	wrbuf[2]  = value;
+	wrbuf[3]  = wrbuf[0] ^ wrbuf[1] ^ wrbuf[2];
+
+	disable_irq(tsdata->irq);
+
+	ret = edt_ft5x06_ts_readwrite(tsdata->client,
+	                              4, wrbuf,
+	                              0, NULL);
+
+	enable_irq(tsdata->irq);
+
+	return ret;
+}
+
+static int edt_ft5x06_i2c_register_read(struct edt_ft5x06_i2c_ts_data *tsdata,
+                                        u8 addr)
+{
+	u8 wrbuf[2], rdbuf[2];
+	int ret;
+
+	wrbuf[0]  = tsdata->factory_mode ? 0xf3 : 0xfc;
+	wrbuf[1]  = tsdata->factory_mode ? addr & 0x7f : addr & 0x3f;
+	wrbuf[1] |= tsdata->factory_mode ? 0x80 : 0x40;
+
+	disable_irq(tsdata->irq);
+
+	ret = edt_ft5x06_ts_readwrite(tsdata->client,
+	                              2, wrbuf,
+	                              2, rdbuf);
+
+	enable_irq(tsdata->irq);
+
+	if ((wrbuf[0] ^ wrbuf[1] ^ rdbuf[0]) != rdbuf[1])
+		dev_err(&tsdata->client->dev,
+		        "crc error: 0x%02x expected, got 0x%02x\n",
+		        (wrbuf[0] ^ wrbuf[1] ^ rdbuf[0]), rdbuf[1]);
+
+	return ret < 0 ? ret : rdbuf[0];
+}
+
+static ssize_t edt_ft5x06_i2c_setting_show(struct device *dev,
+                                           struct device_attribute *attr,
+                                           char *buf)
+{
+	struct edt_ft5x06_i2c_ts_data *tsdata = dev_get_drvdata(dev);
+	struct i2c_client *client = tsdata->client;
+	int ret = 0;
+	int *value;
+	u8 addr;
+
+	switch (attr->attr.name[0]) {
+	case 't':    /* threshold */
+		addr = WORK_REGISTER_THRESHOLD;
+		value = &tsdata->threshold;
+		break;
+	case 'g':    /* gain */
+		addr = WORK_REGISTER_GAIN;
+		value = &tsdata->gain;
+		break;
+	case 'o':    /* offset */
+		addr = WORK_REGISTER_OFFSET;
+		value = &tsdata->offset;
+		break;
+	default:
+		dev_err(&client->dev,
+		        "unknown attribute for edt_ft5x06_i2c_setting_show: %s\n",
+		        attr->attr.name);
+		return -EINVAL;
+	}
+
+	mutex_lock(&tsdata->mutex);
+
+	if (tsdata->factory_mode) {
+		dev_err(dev,
+		        "setting register not available in factory mode\n");
+		mutex_unlock(&tsdata->mutex);
+		return -EIO;
+	}
+
+	ret = edt_ft5x06_i2c_register_read(tsdata, addr);
+	if (ret < 0) {
+		dev_err(&tsdata->client->dev,
+		        "Unable to write to i2c touchscreen!\n");
+		mutex_unlock(&tsdata->mutex);
+		return ret;
+	}
+	mutex_unlock(&tsdata->mutex);
+
+	if (ret != *value) {
+		dev_info(&tsdata->client->dev,
+		         "i2c read (%d) and stored value (%d) differ. Huh?\n",
+		         ret, *value);
+		*value = ret;
+	}
+
+	return sprintf(buf, "%d\n", ret);
+}
+
+static ssize_t edt_ft5x06_i2c_setting_store(struct device *dev,
+                                            struct device_attribute *attr,
+                                            const char *buf, size_t count)
+{
+	struct edt_ft5x06_i2c_ts_data *tsdata = dev_get_drvdata(dev);
+	struct i2c_client *client = tsdata->client;
+	int ret = 0;
+	u8 addr;
+	unsigned int val;
+
+	mutex_lock(&tsdata->mutex);
+
+	if (tsdata->factory_mode) {
+		dev_err(dev,
+		        "setting register not available in factory mode\n");
+		ret = -EIO;
+		goto out;
+	}
+
+	if (sscanf(buf, "%u", &val) != 1) {
+		dev_err(dev, "Invalid value for attribute %s\n",
+		        attr->attr.name);
+		ret = -EINVAL;
+		goto out;
+	}
+
+	switch (attr->attr.name[0]) {
+	case 't':    /* threshold */
+		addr = WORK_REGISTER_THRESHOLD;
+		val = val < 20 ? 20 : val > 80 ? 80 : val;
+		tsdata->threshold = val;
+		break;
+	case 'g':    /* gain */
+		addr = WORK_REGISTER_GAIN;
+		val = val < 0 ? 0 : val > 31 ? 31 : val;
+		tsdata->gain = val;
+		break;
+	case 'o':    /* offset */
+		addr = WORK_REGISTER_OFFSET;
+		val = val < 0 ? 0 : val > 31 ? 31 : val;
+		tsdata->offset = val;
+		break;
+	default:
+		dev_err(&client->dev,
+		        "unknown attribute for edt_ft5x06_i2c_setting_show: %s\n",
+		        attr->attr.name);
+		ret = -EINVAL;
+		goto out;
+	}
+
+	ret = edt_ft5x06_i2c_register_write(tsdata, addr, val);
+
+	if (ret < 0) {
+		dev_err(&tsdata->client->dev,
+		        "Unable to write to i2c touchscreen!\n");
+		goto out;
+	}
+
+out:
+	mutex_unlock(&tsdata->mutex);
+	return ret < 0 ? ret : count;
+}
+
+
+static ssize_t edt_ft5x06_i2c_mode_show(struct device *dev,
+                                        struct device_attribute *attr,
+                                        char *buf)
+{
+	struct edt_ft5x06_i2c_ts_data *tsdata = dev_get_drvdata(dev);
+	return sprintf(buf, "%d\n", tsdata->factory_mode ? 1 : 0);
+}
+
+static ssize_t edt_ft5x06_i2c_mode_store(struct device *dev,
+                                         struct device_attribute *attr,
+                                         const char *buf, size_t count)
+{
+	struct edt_ft5x06_i2c_ts_data *tsdata = dev_get_drvdata(dev);
+	int i, ret = 0;
+	unsigned int mode;
+
+	if (sscanf(buf, "%u", &mode) != 1 || (mode | 1) != 1) {
+		dev_err(dev, "Invalid value for operation mode\n");
+		return -EINVAL;
+	}
+
+	/* no change, return without doing anything */
+	if (mode == tsdata->factory_mode)
+		return count;
+
+	mutex_lock(&tsdata->mutex);
+	if (!tsdata->factory_mode) { /* switch to factory mode */
+		disable_irq(tsdata->irq);
+		/* mode register is 0x3c when in the work mode */
+		ret = edt_ft5x06_i2c_register_write(tsdata,
+		                                    WORK_REGISTER_OPMODE, 0x03);
+		if (ret < 0) {
+			dev_err(dev, "failed to switch to factory mode (%d)\n",
+			        ret);
+		} else {
+			tsdata->factory_mode = 1;
+			for (i = 0; i < 10; i++) {
+				mdelay(5);
+				/* mode register is 0x01 when in factory mode */
+				ret = edt_ft5x06_i2c_register_read(tsdata, FACTORY_REGISTER_OPMODE);
+				if (ret == 0x03)
+					break;
+			}
+			if (i == 10)
+				dev_err(dev,
+				        "not in factory mode after %dms.\n",
+				        i*5);
+		}
+	} else {  /* switch to work mode */
+		/* mode register is 0x01 when in the factory mode */
+		ret = edt_ft5x06_i2c_register_write(tsdata,
+		                                    FACTORY_REGISTER_OPMODE,
+		                                    0x01);
+		if (ret < 0) {
+			dev_err(dev, "failed to switch to work mode (%d)\n",
+			        ret);
+		} else {
+			tsdata->factory_mode = 0;
+			for (i = 0; i < 10; i++) {
+				mdelay(5);
+				/* mode register is 0x01 when in factory mode */
+				ret = edt_ft5x06_i2c_register_read(tsdata, WORK_REGISTER_OPMODE);
+				if (ret == 0x01)
+					break;
+			}
+			if (i == 10)
+				dev_err(dev, "not in work mode after %dms.\n",
+				        i*5);
+
+			/* restore parameters */
+			edt_ft5x06_i2c_register_write(tsdata,
+			                              WORK_REGISTER_THRESHOLD,
+			                              tsdata->threshold);
+			edt_ft5x06_i2c_register_write(tsdata,
+			                              WORK_REGISTER_GAIN,
+			                              tsdata->gain);
+			edt_ft5x06_i2c_register_write(tsdata,
+			                              WORK_REGISTER_OFFSET,
+			                              tsdata->offset);
+
+			enable_irq(tsdata->irq);
+		}
+	}
+
+	mutex_unlock(&tsdata->mutex);
+	return count;
+}
+
+
+static ssize_t edt_ft5x06_i2c_raw_data_show(struct device *dev,
+                                            struct device_attribute *attr,
+                                            char *buf)
+{
+	struct edt_ft5x06_i2c_ts_data *tsdata = dev_get_drvdata(dev);
+	int i, ret;
+	char *ptr, wrbuf[3];
+
+	if (!tsdata->factory_mode) {
+		dev_err(dev, "raw data not available in work mode\n");
+		return -EIO;
+	}
+
+	mutex_lock(&tsdata->mutex);
+	ret = edt_ft5x06_i2c_register_write(tsdata, 0x08, 0x01);
+	for (i = 0; i < 100; i++) {
+		ret = edt_ft5x06_i2c_register_read(tsdata, 0x08);
+		if (ret < 1)
+			break;
+		udelay(1000);
+	}
+
+	if (i == 100 || ret < 0) {
+		dev_err(dev, "waiting time exceeded or error: %d\n", ret);
+		mutex_unlock(&tsdata->mutex);
+		return ret < 0 ? ret : -ETIMEDOUT;
+	}
+
+	ptr = buf;
+	wrbuf[0] = 0xf5;
+	wrbuf[1] = 0x0e;
+	for (i = 0; i <= tsdata->num_x; i++) {
+		wrbuf[2] = i;
+		ret = edt_ft5x06_ts_readwrite(tsdata->client,
+		                               3, wrbuf,
+		                               tsdata->num_y * 2, ptr);
+		if (ret < 0) {
+			mutex_unlock(&tsdata->mutex);
+			return ret;
+		}
+
+		ptr += tsdata->num_y * 2;
+	}
+
+	mutex_unlock(&tsdata->mutex);
+	return ptr - buf;
+}
+
+
+static DEVICE_ATTR(gain,      0664,
+                   edt_ft5x06_i2c_setting_show, edt_ft5x06_i2c_setting_store);
+static DEVICE_ATTR(offset,    0664,
+                   edt_ft5x06_i2c_setting_show, edt_ft5x06_i2c_setting_store);
+static DEVICE_ATTR(threshold, 0664,
+                   edt_ft5x06_i2c_setting_show, edt_ft5x06_i2c_setting_store);
+static DEVICE_ATTR(mode,      0664,
+                   edt_ft5x06_i2c_mode_show, edt_ft5x06_i2c_mode_store);
+static DEVICE_ATTR(raw_data,  0444,
+                   edt_ft5x06_i2c_raw_data_show, NULL);
+
+static struct attribute *edt_ft5x06_i2c_attrs[] = {
+	&dev_attr_gain.attr,
+	&dev_attr_offset.attr,
+	&dev_attr_threshold.attr,
+	&dev_attr_mode.attr,
+	&dev_attr_raw_data.attr,
+	NULL
+};
+
+static const struct attribute_group edt_ft5x06_i2c_attr_group = {
+	.attrs = edt_ft5x06_i2c_attrs,
+};
+
+static int edt_ft5x06_i2c_ts_probe(struct i2c_client *client,
+                                   const struct i2c_device_id *id)
+{
+
+	struct edt_ft5x06_i2c_ts_data *tsdata;
+	struct edt_ft5x06_platform_data *pdata;
+	struct input_dev *input;
+	int error;
+	u8 rdbuf[23];
+	char *model_name, *fw_version;
+
+	dev_dbg(&client->dev, "probing for EDT FT5x06 I2C\n");
+
+	if (!client->irq) {
+		dev_err(&client->dev, "no IRQ?\n");
+		return -ENODEV;
+	}
+
+	if (!client->dev.platform_data) {
+		dev_err(&client->dev, "no platform data?\n");
+		return -ENODEV;
+	}
+
+	tsdata = kzalloc(sizeof(*tsdata), GFP_KERNEL);
+	if (!tsdata) {
+		dev_err(&client->dev, "failed to allocate driver data!\n");
+		dev_set_drvdata(&client->dev, NULL);
+		return -ENOMEM;
+	}
+
+	dev_set_drvdata(&client->dev, tsdata);
+	tsdata->client = client;
+	pdata = client->dev.platform_data;
+
+	tsdata->reset_pin = pdata->reset_pin;
+	mutex_init(&tsdata->mutex);
+
+	if (tsdata->reset_pin >= 0) {
+		error = gpio_request(tsdata->reset_pin, NULL);
+		if (error < 0) {
+			dev_err(&client->dev,
+			        "Failed to request GPIO %d as reset pin, error %d\n",
+			         tsdata->reset_pin, error);
+			error = -ENOMEM;
+			goto err_free_tsdata;
+		}
+
+		/* this pulls reset down, enabling the low active reset */
+		if (gpio_direction_output(tsdata->reset_pin, 0) < 0) {
+			dev_info(&client->dev, "switching to output failed\n");
+			goto err_free_reset_pin;
+		}
+	}
+
+	/* request IRQ pin */
+	tsdata->irq_pin = pdata->irq_pin;
+	tsdata->irq = gpio_to_irq(tsdata->irq_pin);
+
+	error = gpio_request(tsdata->irq_pin, NULL);
+	if (error < 0) {
+		dev_err(&client->dev,
+		        "Failed to request GPIO %d for IRQ %d, error %d\n",
+		        tsdata->irq_pin, tsdata->irq, error);
+		goto err_free_reset_pin;
+	}
+	gpio_direction_input(tsdata->irq_pin);
+
+	if (tsdata->reset_pin >= 0) {
+		/* release reset */
+		mdelay(50);
+		gpio_set_value(tsdata->reset_pin, 1);
+		mdelay(100);
+	}
+
+	mutex_lock(&tsdata->mutex);
+
+	tsdata->factory_mode = 0;
+
+	if (edt_ft5x06_ts_readwrite(client, 1, "\xbb", 22, rdbuf) < 0) {
+		dev_err(&client->dev, "probing failed\n");
+		error = -ENODEV;
+		goto err_free_irq_pin;
+	}
+
+	tsdata->threshold = edt_ft5x06_i2c_register_read(tsdata,
+	                                                 WORK_REGISTER_THRESHOLD);
+	tsdata->gain      = edt_ft5x06_i2c_register_read(tsdata,
+	                                                 WORK_REGISTER_GAIN);
+	tsdata->offset    = edt_ft5x06_i2c_register_read(tsdata,
+	                                                 WORK_REGISTER_OFFSET);
+	tsdata->num_x     = edt_ft5x06_i2c_register_read(tsdata,
+	                                                 WORK_REGISTER_NUM_X);
+	tsdata->num_y     = edt_ft5x06_i2c_register_read(tsdata,
+	                                                 WORK_REGISTER_NUM_Y);
+
+	mutex_unlock(&tsdata->mutex);
+
+	/* remove last '$' end marker */
+	rdbuf[22] = '\0';
+	if (rdbuf[21] == '$')
+		rdbuf[21] = '\0';
+
+	model_name = rdbuf + 1;
+	/* look for Model/Version separator */
+	fw_version = strchr(rdbuf, '*');
+
+	if (fw_version) {
+		fw_version[0] = '\0';
+		fw_version++;
+		dev_info(&client->dev,
+		         "Model \"%s\", Rev. \"%s\", %dx%d sensors\n",
+		         model_name, fw_version, tsdata->num_x, tsdata->num_y);
+	} else {
+		dev_info(&client->dev, "Product ID \"%s\"\n", model_name);
+	}
+
+	input = input_allocate_device();
+	if (!input) {
+		dev_err(&client->dev, "failed to allocate input device!\n");
+		error = -ENOMEM;
+		goto err_free_irq_pin;
+	}
+
+	__set_bit(EV_SYN, input->evbit);
+	__set_bit(EV_KEY, input->evbit);
+	__set_bit(EV_ABS, input->evbit);
+	__set_bit(BTN_TOUCH, input->keybit);
+	input_set_abs_params(input, ABS_X, 0, tsdata->num_x * 64 - 1, 0, 0);
+	input_set_abs_params(input, ABS_Y, 0, tsdata->num_y * 64 - 1, 0, 0);
+	input_set_abs_params(input, ABS_PRESSURE, 0, 1, 0, 0);
+	input_set_abs_params(input, ABS_MT_POSITION_X,
+	                     0, tsdata->num_x * 64 - 1, 0, 0);
+	input_set_abs_params(input, ABS_MT_POSITION_Y,
+	                     0, tsdata->num_y * 64 - 1, 0, 0);
+	input_set_abs_params(input, ABS_MT_TRACKING_ID, 0, 15, 0, 0);
+
+	input->name = kstrdup(model_name, GFP_NOIO);
+	input->id.bustype = BUS_I2C;
+	input->dev.parent = &client->dev;
+
+	input_set_drvdata(input, tsdata);
+
+	tsdata->input = input;
+
+	error = input_register_device(input);
+	if (error)
+		goto err_free_input_device;
+
+	if (request_threaded_irq(tsdata->irq, NULL, edt_ft5x06_ts_isr,
+	                         IRQF_TRIGGER_LOW | IRQF_ONESHOT,
+	                         client->name, tsdata)) {
+		dev_err(&client->dev, "Unable to request touchscreen IRQ.\n");
+		input = NULL;
+		error = -ENOMEM;
+		goto err_unregister_device;
+	}
+
+	error = sysfs_create_group(&client->dev.kobj,
+	                           &edt_ft5x06_i2c_attr_group);
+	if (error)
+		goto err_free_irq;
+
+	device_init_wakeup(&client->dev, 1);
+
+	dev_dbg(&tsdata->client->dev,
+	        "EDT FT5x06 initialized: IRQ pin %d, Reset pin %d.\n",
+	        tsdata->irq_pin, tsdata->reset_pin);
+
+	return 0;
+
+err_free_irq:
+	free_irq(client->irq, tsdata);
+err_unregister_device:
+	input_unregister_device(input);
+	input = NULL;
+err_free_input_device:
+	if (input) {
+		kfree(input->name);
+		input_free_device(input);
+	}
+err_free_irq_pin:
+	gpio_free(tsdata->irq_pin);
+err_free_reset_pin:
+	if (tsdata->reset_pin >= 0)
+		gpio_free(tsdata->reset_pin);
+err_free_tsdata:
+	kfree(tsdata);
+	return error;
+}
+
+static int edt_ft5x06_i2c_ts_remove(struct i2c_client *client)
+{
+	struct edt_ft5x06_i2c_ts_data *tsdata = dev_get_drvdata(&client->dev);
+
+	sysfs_remove_group(&client->dev.kobj, &edt_ft5x06_i2c_attr_group);
+
+	free_irq(client->irq, tsdata);
+	kfree(tsdata->input->name);
+	input_unregister_device(tsdata->input);
+	gpio_free(tsdata->irq_pin);
+	if (tsdata->reset_pin >= 0)
+		gpio_free(tsdata->reset_pin);
+	kfree(tsdata);
+
+	return 0;
+}
+
+static int edt_ft5x06_i2c_ts_suspend(struct i2c_client *client,
+                                     pm_message_t mesg)
+{
+	struct edt_ft5x06_i2c_ts_data *tsdata = dev_get_drvdata(&client->dev);
+
+	if (device_may_wakeup(&client->dev))
+		enable_irq_wake(tsdata->irq);
+
+	return 0;
+}
+
+static int edt_ft5x06_i2c_ts_resume(struct i2c_client *client)
+{
+	struct edt_ft5x06_i2c_ts_data *tsdata = dev_get_drvdata(&client->dev);
+
+	if (device_may_wakeup(&client->dev))
+		disable_irq_wake(tsdata->irq);
+
+	return 0;
+}
+
+static const struct i2c_device_id edt_ft5x06_i2c_ts_id[] = {
+	{ "edt-ft5x06", 0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, edt_ft5x06_i2c_ts_id);
+
+static struct i2c_driver edt_ft5x06_i2c_ts_driver = {
+	.driver = {
+		.owner = THIS_MODULE,
+		.name = "edt_ft5x06_i2c",
+	},
+	.id_table = edt_ft5x06_i2c_ts_id,
+	.probe    = edt_ft5x06_i2c_ts_probe,
+	.remove   = edt_ft5x06_i2c_ts_remove,
+	.suspend  = edt_ft5x06_i2c_ts_suspend,
+	.resume   = edt_ft5x06_i2c_ts_resume,
+};
+
+static int __init edt_ft5x06_i2c_ts_init(void)
+{
+	return i2c_add_driver(&edt_ft5x06_i2c_ts_driver);
+}
+module_init(edt_ft5x06_i2c_ts_init);
+
+static void __exit edt_ft5x06_i2c_ts_exit(void)
+{
+	i2c_del_driver(&edt_ft5x06_i2c_ts_driver);
+}
+module_exit(edt_ft5x06_i2c_ts_exit);
+
+MODULE_AUTHOR("Simon Budig <simon.budig@kernelconcepts.de>");
+MODULE_DESCRIPTION("EDT FT5x06 I2C Touchscreen Driver");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/input/edt-ft5x06.h b/include/linux/input/edt-ft5x06.h
new file mode 100644
index 0000000..db4130d
--- /dev/null
+++ b/include/linux/input/edt-ft5x06.h
@@ -0,0 +1,17 @@
+#ifndef _EDT_FT5X06_H
+#define _EDT_FT5X06_H
+
+/*
+ * Copyright (c) 2011 Simon Budig, <simon.budig@kernelconcepts.de>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published by
+ * the Free Software Foundation.
+ */
+
+struct edt_ft5x06_platform_data {
+	int irq_pin;
+	int reset_pin;
+};
+
+#endif /* _EDT_FT5X06_H */
-- 
1.7.2.5

[PATCH v4] Touchscreen driver for FT5x06 based EDT displays

[simon.budig]

Hi all.

Here is a new iteration of the EDT polytouch driver. Compared to v3 it fixes
some of the interrupt handling and adds support for the report rate setting.

Please give feedback on problems or on further plans for inclusion into
mainline.

Thanks,
        Simon

[PATCH v5] Touchscreen driver for FT5x06 based EDT displays

[simon.budig]

Hi all.

Here is a new iteration of the EDT polytouch driver. Compared to v4 it
incorporates the feedback from Anatolij Gustschin and some fixes from Lothar
Waßmann (bug on module unloading and obsolete power management api use).

Please give feedback on problems or on further plans for inclusion into
mainline.

Thanks,
        Simon

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

[PATCH] Touchscreen driver for FT5x06 based EDT displays

[simon.budig]

From: Simon Budig <simon.budig@kernelconcepts.de>

This is a driver for the EDT "Polytouch" family of touch controllers
based on the FocalTech FT5x06 line of chips.

Signed-off-by: Simon Budig <simon.budig@kernelconcepts.de>
---
 drivers/input/touchscreen/Kconfig      |   13 +
 drivers/input/touchscreen/Makefile     |    1 +
 drivers/input/touchscreen/edt-ft5x06.c |  771 ++++++++++++++++++++++++++++++++
 include/linux/input/edt-ft5x06.h       |   17 +
 4 files changed, 802 insertions(+), 0 deletions(-)
 create mode 100644 drivers/input/touchscreen/edt-ft5x06.c
 create mode 100644 include/linux/input/edt-ft5x06.h

diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
index 2a21419..0166a15 100644
--- a/drivers/input/touchscreen/Kconfig
+++ b/drivers/input/touchscreen/Kconfig
@@ -431,6 +431,19 @@ config TOUCHSCREEN_PENMOUNT
 	  To compile this driver as a module, choose M here: the
 	  module will be called penmount.
 
+config TOUCHSCREEN_EDT_FT5X06
+	tristate "EDT FocalTech FT5x06 I2C Touchscreen support"
+	depends on I2C
+	help
+	  Say Y here if you have an EDT "Polytouch" touchscreen based
+	  on the FocalTech FT5x06 family of controllers connected to
+	  your system.
+
+	  If unsure, say N.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called edt-ft5x06.
+
 config TOUCHSCREEN_MIGOR
 	tristate "Renesas MIGO-R touchscreen"
 	depends on SH_MIGOR && I2C
diff --git a/drivers/input/touchscreen/Makefile b/drivers/input/touchscreen/Makefile
index 3d5cf8c..8a68c84 100644
--- a/drivers/input/touchscreen/Makefile
+++ b/drivers/input/touchscreen/Makefile
@@ -23,6 +23,7 @@ obj-$(CONFIG_TOUCHSCREEN_CYTTSP_I2C)	+= cyttsp_i2c.o
 obj-$(CONFIG_TOUCHSCREEN_CYTTSP_SPI)	+= cyttsp_spi.o
 obj-$(CONFIG_TOUCHSCREEN_DA9034)	+= da9034-ts.o
 obj-$(CONFIG_TOUCHSCREEN_DYNAPRO)	+= dynapro.o
+obj-$(CONFIG_TOUCHSCREEN_EDT_FT5X06)	+= edt-ft5x06.o
 obj-$(CONFIG_TOUCHSCREEN_HAMPSHIRE)	+= hampshire.o
 obj-$(CONFIG_TOUCHSCREEN_GUNZE)		+= gunze.o
 obj-$(CONFIG_TOUCHSCREEN_EETI)		+= eeti_ts.o
diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c
new file mode 100644
index 0000000..7c4f9f5
--- /dev/null
+++ b/drivers/input/touchscreen/edt-ft5x06.c
@@ -0,0 +1,771 @@
+/*
+ * Copyright (C) 2011 Simon Budig, <simon.budig@kernelconcepts.de>
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ */
+
+/*
+ * This is a driver for the EDT "Polytouch" family of touch controllers
+ * based on the FocalTech FT5x06 line of chips.
+ *
+ * Development of this driver has been sponsored by Glyn:
+ *    http://www.glyn.com/Products/Displays
+ */
+
+#include <linux/module.h>
+#include <linux/interrupt.h>
+#include <linux/input.h>
+#include <linux/i2c.h>
+#include <linux/uaccess.h>
+#include <linux/delay.h>
+#include <linux/slab.h>
+
+#include <linux/gpio.h>
+
+#include <linux/input/edt-ft5x06.h>
+
+#define DRIVER_VERSION "v0.5"
+
+#define WORK_REGISTER_THRESHOLD   0x00
+#define WORK_REGISTER_REPORT_RATE 0x08
+#define WORK_REGISTER_GAIN        0x30
+#define WORK_REGISTER_OFFSET      0x31
+#define WORK_REGISTER_NUM_X       0x33
+#define WORK_REGISTER_NUM_Y       0x34
+
+#define WORK_REGISTER_OPMODE      0x3c
+#define FACTORY_REGISTER_OPMODE   0x01
+
+struct edt_ft5x06_i2c_ts_data {
+	struct i2c_client *client;
+	struct input_dev *input;
+	int irq;
+	int irq_pin;
+	int reset_pin;
+	int num_x;
+	int num_y;
+
+	struct mutex mutex;
+	bool factory_mode;
+	int threshold;
+	int gain;
+	int offset;
+	int report_rate;
+};
+
+static int edt_ft5x06_ts_readwrite(struct i2c_client *client,
+                                   u16 wr_len, u8 *wr_buf,
+                                   u16 rd_len, u8 *rd_buf)
+{
+	struct i2c_msg wrmsg[2];
+	int i, ret;
+
+	i = 0;
+	if (wr_len) {
+		wrmsg[i].addr  = client->addr;
+		wrmsg[i].flags = 0;
+		wrmsg[i].len = wr_len;
+		wrmsg[i].buf = wr_buf;
+		i++;
+	}
+	if (rd_len) {
+		wrmsg[i].addr  = client->addr;
+		wrmsg[i].flags = I2C_M_RD;
+		wrmsg[i].len = rd_len;
+		wrmsg[i].buf = rd_buf;
+		i++;
+	}
+
+	ret = i2c_transfer(client->adapter, wrmsg, i);
+	if (ret < 0) {
+		dev_err(&client->dev, "i2c_transfer failed: %d\n", ret);
+		return ret;
+	}
+
+	return ret;
+}
+
+
+static irqreturn_t edt_ft5x06_ts_isr(int irq, void *dev_id)
+{
+	struct edt_ft5x06_i2c_ts_data *tsdata = dev_id;
+	unsigned char touching = 0;
+	unsigned char rdbuf[26], wrbuf[1];
+	int i, have_abs, type, ret;
+
+	memset(wrbuf, 0, sizeof(wrbuf));
+	memset(rdbuf, 0, sizeof(rdbuf));
+
+	wrbuf[0] = 0xf9;
+
+	mutex_lock(&tsdata->mutex);
+	ret = edt_ft5x06_ts_readwrite(tsdata->client,
+	                              1, wrbuf,
+	                              sizeof(rdbuf), rdbuf);
+	mutex_unlock(&tsdata->mutex);
+	if (ret < 0) {
+		dev_err(&tsdata->client->dev,
+		        "Unable to write to i2c touchscreen!\n");
+		goto out;
+	}
+
+	if (rdbuf[0] != 0xaa || rdbuf[1] != 0xaa || rdbuf[2] != 26) {
+		dev_err(&tsdata->client->dev,
+		        "Unexpected header: %02x%02x%02x!\n",
+		        rdbuf[0], rdbuf[1], rdbuf[2]);
+	}
+
+	have_abs = 0;
+	touching = rdbuf[3];
+	for (i = 0; i < touching; i++) {
+		type = rdbuf[i*4+5] >> 6;
+		/* ignore Touch Down and Reserved events */
+		if (type == 0x01 || type == 0x03)
+			continue;
+
+		if (!have_abs) {
+			input_report_key(tsdata->input, BTN_TOUCH,    1);
+			input_report_abs(tsdata->input, ABS_PRESSURE, 1);
+			input_report_abs(tsdata->input, ABS_X,
+			                 ((rdbuf[i*4+5] << 8) |
+			                  rdbuf[i*4+6]) & 0x0fff);
+			input_report_abs(tsdata->input, ABS_Y,
+			                 ((rdbuf[i*4+7] << 8) |
+			                  rdbuf[i*4+8]) & 0x0fff);
+			have_abs = 1;
+		}
+		input_report_abs(tsdata->input, ABS_MT_POSITION_X,
+		                 ((rdbuf[i*4+5] << 8) | rdbuf[i*4+6]) & 0x0fff);
+		input_report_abs(tsdata->input, ABS_MT_POSITION_Y,
+		                 ((rdbuf[i*4+7] << 8) | rdbuf[i*4+8]) & 0x0fff);
+		input_report_abs(tsdata->input, ABS_MT_TRACKING_ID,
+		                 (rdbuf[i*4+7] >> 4) & 0x0f);
+		input_mt_sync(tsdata->input);
+	}
+	if (!have_abs) {
+		input_report_key(tsdata->input, BTN_TOUCH,    0);
+		input_report_abs(tsdata->input, ABS_PRESSURE, 0);
+	}
+	input_sync(tsdata->input);
+
+out:
+	return IRQ_HANDLED;
+}
+
+
+static int edt_ft5x06_i2c_register_write(struct edt_ft5x06_i2c_ts_data *tsdata,
+                                         u8 addr, u8 value)
+{
+	u8 wrbuf[4];
+	int ret;
+
+	wrbuf[0]  = tsdata->factory_mode ? 0xf3 : 0xfc;
+	wrbuf[1]  = tsdata->factory_mode ? addr & 0x7f : addr & 0x3f;
+	wrbuf[2]  = value;
+	wrbuf[3]  = wrbuf[0] ^ wrbuf[1] ^ wrbuf[2];
+
+	disable_irq(tsdata->irq);
+
+	ret = edt_ft5x06_ts_readwrite(tsdata->client,
+	                              4, wrbuf,
+	                              0, NULL);
+
+	enable_irq(tsdata->irq);
+
+	return ret;
+}
+
+static int edt_ft5x06_i2c_register_read(struct edt_ft5x06_i2c_ts_data *tsdata,
+                                        u8 addr)
+{
+	u8 wrbuf[2], rdbuf[2];
+	int ret;
+
+	wrbuf[0]  = tsdata->factory_mode ? 0xf3 : 0xfc;
+	wrbuf[1]  = tsdata->factory_mode ? addr & 0x7f : addr & 0x3f;
+	wrbuf[1] |= tsdata->factory_mode ? 0x80 : 0x40;
+
+	disable_irq(tsdata->irq);
+
+	ret = edt_ft5x06_ts_readwrite(tsdata->client,
+	                              2, wrbuf,
+	                              2, rdbuf);
+
+	enable_irq(tsdata->irq);
+
+	if ((wrbuf[0] ^ wrbuf[1] ^ rdbuf[0]) != rdbuf[1])
+		dev_err(&tsdata->client->dev,
+		        "crc error: 0x%02x expected, got 0x%02x\n",
+		        (wrbuf[0] ^ wrbuf[1] ^ rdbuf[0]), rdbuf[1]);
+
+	return ret < 0 ? ret : rdbuf[0];
+}
+
+static ssize_t edt_ft5x06_i2c_setting_show(struct device *dev,
+                                           struct device_attribute *attr,
+                                           char *buf)
+{
+	struct edt_ft5x06_i2c_ts_data *tsdata = dev_get_drvdata(dev);
+	struct i2c_client *client = tsdata->client;
+	int ret = 0;
+	int *value;
+	u8 addr;
+
+	switch (attr->attr.name[0]) {
+	case 't':    /* threshold */
+		addr = WORK_REGISTER_THRESHOLD;
+		value = &tsdata->threshold;
+		break;
+	case 'g':    /* gain */
+		addr = WORK_REGISTER_GAIN;
+		value = &tsdata->gain;
+		break;
+	case 'o':    /* offset */
+		addr = WORK_REGISTER_OFFSET;
+		value = &tsdata->offset;
+		break;
+	case 'r':    /* report rate */
+		addr = WORK_REGISTER_REPORT_RATE;
+		value = &tsdata->report_rate;
+		break;
+	default:
+		dev_err(&client->dev,
+		        "unknown attribute for edt_ft5x06_i2c_setting_show: %s\n",
+		        attr->attr.name);
+		return -EINVAL;
+	}
+
+	mutex_lock(&tsdata->mutex);
+
+	if (tsdata->factory_mode) {
+		dev_err(dev,
+		        "setting register not available in factory mode\n");
+		mutex_unlock(&tsdata->mutex);
+		return -EIO;
+	}
+
+	ret = edt_ft5x06_i2c_register_read(tsdata, addr);
+	if (ret < 0) {
+		dev_err(&tsdata->client->dev,
+		        "Unable to write to i2c touchscreen!\n");
+		mutex_unlock(&tsdata->mutex);
+		return ret;
+	}
+	mutex_unlock(&tsdata->mutex);
+
+	if (ret != *value) {
+		dev_info(&tsdata->client->dev,
+		         "i2c read (%d) and stored value (%d) differ. Huh?\n",
+		         ret, *value);
+		*value = ret;
+	}
+
+	return sprintf(buf, "%d\n", ret);
+}
+
+static ssize_t edt_ft5x06_i2c_setting_store(struct device *dev,
+                                            struct device_attribute *attr,
+                                            const char *buf, size_t count)
+{
+	struct edt_ft5x06_i2c_ts_data *tsdata = dev_get_drvdata(dev);
+	struct i2c_client *client = tsdata->client;
+	int ret = 0;
+	u8 addr;
+	unsigned int val;
+
+	mutex_lock(&tsdata->mutex);
+
+	if (tsdata->factory_mode) {
+		dev_err(dev,
+		        "setting register not available in factory mode\n");
+		ret = -EIO;
+		goto out;
+	}
+
+	if (sscanf(buf, "%u", &val) != 1) {
+		dev_err(dev, "Invalid value for attribute %s\n",
+		        attr->attr.name);
+		ret = -EINVAL;
+		goto out;
+	}
+
+	switch (attr->attr.name[0]) {
+	case 't':    /* threshold */
+		addr = WORK_REGISTER_THRESHOLD;
+		val = val < 20 ? 20 : val > 80 ? 80 : val;
+		tsdata->threshold = val;
+		break;
+	case 'g':    /* gain */
+		addr = WORK_REGISTER_GAIN;
+		val = val < 0 ? 0 : val > 31 ? 31 : val;
+		tsdata->gain = val;
+		break;
+	case 'o':    /* offset */
+		addr = WORK_REGISTER_OFFSET;
+		val = val < 0 ? 0 : val > 31 ? 31 : val;
+		tsdata->offset = val;
+		break;
+	case 'r':    /* report rate */
+		addr = WORK_REGISTER_REPORT_RATE;
+		val = val < 3 ? 3 : val > 14 ? 14 : val;
+		tsdata->report_rate = val;
+		break;
+	default:
+		dev_err(&client->dev,
+		        "unknown attribute for edt_ft5x06_i2c_setting_show: %s\n",
+		        attr->attr.name);
+		ret = -EINVAL;
+		goto out;
+	}
+
+	ret = edt_ft5x06_i2c_register_write(tsdata, addr, val);
+
+	if (ret < 0) {
+		dev_err(&tsdata->client->dev,
+		        "Unable to write to i2c touchscreen!\n");
+		goto out;
+	}
+
+out:
+	mutex_unlock(&tsdata->mutex);
+	return ret < 0 ? ret : count;
+}
+
+
+static ssize_t edt_ft5x06_i2c_mode_show(struct device *dev,
+                                        struct device_attribute *attr,
+                                        char *buf)
+{
+	struct edt_ft5x06_i2c_ts_data *tsdata = dev_get_drvdata(dev);
+	return sprintf(buf, "%d\n", tsdata->factory_mode ? 1 : 0);
+}
+
+static ssize_t edt_ft5x06_i2c_mode_store(struct device *dev,
+                                         struct device_attribute *attr,
+                                         const char *buf, size_t count)
+{
+	struct edt_ft5x06_i2c_ts_data *tsdata = dev_get_drvdata(dev);
+	int i, ret = 0;
+	unsigned int mode;
+
+	if (sscanf(buf, "%u", &mode) != 1 || (mode | 1) != 1) {
+		dev_err(dev, "Invalid value for operation mode\n");
+		return -EINVAL;
+	}
+
+	/* no change, return without doing anything */
+	if (mode == tsdata->factory_mode)
+		return count;
+
+	mutex_lock(&tsdata->mutex);
+	if (!tsdata->factory_mode) { /* switch to factory mode */
+		disable_irq(tsdata->irq);
+		/* mode register is 0x3c when in the work mode */
+		ret = edt_ft5x06_i2c_register_write(tsdata,
+		                                    WORK_REGISTER_OPMODE, 0x03);
+		if (ret < 0) {
+			dev_err(dev, "failed to switch to factory mode (%d)\n",
+			        ret);
+		} else {
+			tsdata->factory_mode = 1;
+			for (i = 0; i < 10; i++) {
+				mdelay(5);
+				/* mode register is 0x01 when in factory mode */
+				ret = edt_ft5x06_i2c_register_read(tsdata, FACTORY_REGISTER_OPMODE);
+				if (ret == 0x03)
+					break;
+			}
+			if (i == 10)
+				dev_err(dev,
+				        "not in factory mode after %dms.\n",
+				        i*5);
+		}
+	} else {  /* switch to work mode */
+		/* mode register is 0x01 when in the factory mode */
+		ret = edt_ft5x06_i2c_register_write(tsdata,
+		                                    FACTORY_REGISTER_OPMODE,
+		                                    0x01);
+		if (ret < 0) {
+			dev_err(dev, "failed to switch to work mode (%d)\n",
+			        ret);
+		} else {
+			tsdata->factory_mode = 0;
+			for (i = 0; i < 10; i++) {
+				mdelay(5);
+				/* mode register is 0x01 when in factory mode */
+				ret = edt_ft5x06_i2c_register_read(tsdata, WORK_REGISTER_OPMODE);
+				if (ret == 0x01)
+					break;
+			}
+			if (i == 10)
+				dev_err(dev, "not in work mode after %dms.\n",
+				        i*5);
+
+			/* restore parameters */
+			edt_ft5x06_i2c_register_write(tsdata,
+			                              WORK_REGISTER_THRESHOLD,
+			                              tsdata->threshold);
+			edt_ft5x06_i2c_register_write(tsdata,
+			                              WORK_REGISTER_GAIN,
+			                              tsdata->gain);
+			edt_ft5x06_i2c_register_write(tsdata,
+			                              WORK_REGISTER_OFFSET,
+			                              tsdata->offset);
+			edt_ft5x06_i2c_register_write(tsdata,
+			                              WORK_REGISTER_REPORT_RATE,
+			                              tsdata->report_rate);
+
+			enable_irq(tsdata->irq);
+		}
+	}
+
+	mutex_unlock(&tsdata->mutex);
+	return count;
+}
+
+
+static ssize_t edt_ft5x06_i2c_raw_data_show(struct device *dev,
+                                            struct device_attribute *attr,
+                                            char *buf)
+{
+	struct edt_ft5x06_i2c_ts_data *tsdata = dev_get_drvdata(dev);
+	int i, ret;
+	char *ptr, wrbuf[3];
+
+	if (!tsdata->factory_mode) {
+		dev_err(dev, "raw data not available in work mode\n");
+		return -EIO;
+	}
+
+	mutex_lock(&tsdata->mutex);
+	ret = edt_ft5x06_i2c_register_write(tsdata, 0x08, 0x01);
+	for (i = 0; i < 100; i++) {
+		ret = edt_ft5x06_i2c_register_read(tsdata, 0x08);
+		if (ret < 1)
+			break;
+		udelay(1000);
+	}
+
+	if (i == 100 || ret < 0) {
+		dev_err(dev, "waiting time exceeded or error: %d\n", ret);
+		mutex_unlock(&tsdata->mutex);
+		return ret < 0 ? ret : -ETIMEDOUT;
+	}
+
+	ptr = buf;
+	wrbuf[0] = 0xf5;
+	wrbuf[1] = 0x0e;
+	for (i = 0; i <= tsdata->num_x; i++) {
+		wrbuf[2] = i;
+		ret = edt_ft5x06_ts_readwrite(tsdata->client,
+		                               3, wrbuf,
+		                               tsdata->num_y * 2, ptr);
+		if (ret < 0) {
+			mutex_unlock(&tsdata->mutex);
+			return ret;
+		}
+
+		ptr += tsdata->num_y * 2;
+	}
+
+	mutex_unlock(&tsdata->mutex);
+	return ptr - buf;
+}
+
+
+static DEVICE_ATTR(gain,      0664,
+                   edt_ft5x06_i2c_setting_show, edt_ft5x06_i2c_setting_store);
+static DEVICE_ATTR(offset,    0664,
+                   edt_ft5x06_i2c_setting_show, edt_ft5x06_i2c_setting_store);
+static DEVICE_ATTR(threshold, 0664,
+                   edt_ft5x06_i2c_setting_show, edt_ft5x06_i2c_setting_store);
+static DEVICE_ATTR(report_rate, 0664,
+                   edt_ft5x06_i2c_setting_show, edt_ft5x06_i2c_setting_store);
+static DEVICE_ATTR(mode,      0664,
+                   edt_ft5x06_i2c_mode_show, edt_ft5x06_i2c_mode_store);
+static DEVICE_ATTR(raw_data,  0444,
+                   edt_ft5x06_i2c_raw_data_show, NULL);
+
+static struct attribute *edt_ft5x06_i2c_attrs[] = {
+	&dev_attr_gain.attr,
+	&dev_attr_offset.attr,
+	&dev_attr_threshold.attr,
+	&dev_attr_report_rate.attr,
+	&dev_attr_mode.attr,
+	&dev_attr_raw_data.attr,
+	NULL
+};
+
+static const struct attribute_group edt_ft5x06_i2c_attr_group = {
+	.attrs = edt_ft5x06_i2c_attrs,
+};
+
+static int edt_ft5x06_i2c_ts_probe(struct i2c_client *client,
+                                   const struct i2c_device_id *id)
+{
+
+	struct edt_ft5x06_i2c_ts_data *tsdata;
+	struct edt_ft5x06_platform_data *pdata;
+	struct input_dev *input;
+	int error;
+	u8 rdbuf[23];
+	char *model_name, *fw_version;
+
+	dev_dbg(&client->dev, "probing for EDT FT5x06 I2C\n");
+
+	if (!client->dev.platform_data) {
+		dev_err(&client->dev, "no platform data?\n");
+		return -ENODEV;
+	}
+
+	tsdata = kzalloc(sizeof(*tsdata), GFP_KERNEL);
+	if (!tsdata) {
+		dev_err(&client->dev, "failed to allocate driver data!\n");
+		dev_set_drvdata(&client->dev, NULL);
+		return -ENOMEM;
+	}
+
+	dev_set_drvdata(&client->dev, tsdata);
+	tsdata->client = client;
+	pdata = client->dev.platform_data;
+
+	tsdata->reset_pin = pdata->reset_pin;
+	mutex_init(&tsdata->mutex);
+
+	if (tsdata->reset_pin >= 0) {
+		error = gpio_request(tsdata->reset_pin, "edt-ft5x06 reset");
+		if (error < 0) {
+			dev_err(&client->dev,
+			        "Failed to request GPIO %d as reset pin, error %d\n",
+			         tsdata->reset_pin, error);
+			goto err_free_tsdata;
+		}
+
+		/* this pulls reset down, enabling the low active reset */
+		if (gpio_direction_output(tsdata->reset_pin, 0) < 0) {
+			dev_info(&client->dev, "switching to output failed\n");
+			goto err_free_reset_pin;
+		}
+	}
+
+	/* request IRQ pin */
+	tsdata->irq_pin = pdata->irq_pin;
+	tsdata->irq = gpio_to_irq(tsdata->irq_pin);
+
+	error = gpio_request(tsdata->irq_pin, "edt-ft5x06 irq");
+	if (error < 0) {
+		dev_err(&client->dev,
+		        "Failed to request GPIO %d for IRQ %d, error %d\n",
+		        tsdata->irq_pin, tsdata->irq, error);
+		goto err_free_reset_pin;
+	}
+	gpio_direction_input(tsdata->irq_pin);
+
+	if (tsdata->reset_pin >= 0) {
+		/* release reset */
+		mdelay(50);
+		gpio_set_value(tsdata->reset_pin, 1);
+		mdelay(100);
+	}
+
+	mutex_lock(&tsdata->mutex);
+
+	tsdata->factory_mode = 0;
+
+	if (edt_ft5x06_ts_readwrite(client, 1, "\xbb", 22, rdbuf) < 0) {
+		dev_err(&client->dev, "probing failed\n");
+		error = -ENODEV;
+		goto err_free_irq_pin;
+	}
+
+	tsdata->threshold = edt_ft5x06_i2c_register_read(tsdata,
+	                                                 WORK_REGISTER_THRESHOLD);
+	tsdata->gain      = edt_ft5x06_i2c_register_read(tsdata,
+	                                                 WORK_REGISTER_GAIN);
+	tsdata->offset    = edt_ft5x06_i2c_register_read(tsdata,
+	                                                 WORK_REGISTER_OFFSET);
+	tsdata->report_rate = edt_ft5x06_i2c_register_read(tsdata,
+	                                                 WORK_REGISTER_REPORT_RATE);
+	tsdata->num_x     = edt_ft5x06_i2c_register_read(tsdata,
+	                                                 WORK_REGISTER_NUM_X);
+	tsdata->num_y     = edt_ft5x06_i2c_register_read(tsdata,
+	                                                 WORK_REGISTER_NUM_Y);
+
+	mutex_unlock(&tsdata->mutex);
+
+	/* remove last '$' end marker */
+	rdbuf[22] = '\0';
+	if (rdbuf[21] == '$')
+		rdbuf[21] = '\0';
+
+	model_name = rdbuf + 1;
+	/* look for Model/Version separator */
+	fw_version = strchr(rdbuf, '*');
+
+	if (fw_version) {
+		fw_version[0] = '\0';
+		fw_version++;
+		dev_info(&client->dev,
+		         "Model \"%s\", Rev. \"%s\", %dx%d sensors\n",
+		         model_name, fw_version, tsdata->num_x, tsdata->num_y);
+	} else {
+		dev_info(&client->dev, "Product ID \"%s\"\n", model_name);
+	}
+
+	input = input_allocate_device();
+	if (!input) {
+		dev_err(&client->dev, "failed to allocate input device!\n");
+		error = -ENOMEM;
+		goto err_free_irq_pin;
+	}
+
+	__set_bit(EV_SYN, input->evbit);
+	__set_bit(EV_KEY, input->evbit);
+	__set_bit(EV_ABS, input->evbit);
+	__set_bit(BTN_TOUCH, input->keybit);
+	input_set_abs_params(input, ABS_X, 0, tsdata->num_x * 64 - 1, 0, 0);
+	input_set_abs_params(input, ABS_Y, 0, tsdata->num_y * 64 - 1, 0, 0);
+	input_set_abs_params(input, ABS_PRESSURE, 0, 1, 0, 0);
+	input_set_abs_params(input, ABS_MT_POSITION_X,
+	                     0, tsdata->num_x * 64 - 1, 0, 0);
+	input_set_abs_params(input, ABS_MT_POSITION_Y,
+	                     0, tsdata->num_y * 64 - 1, 0, 0);
+	input_set_abs_params(input, ABS_MT_TRACKING_ID, 0, 15, 0, 0);
+
+	input->name = kstrdup(model_name, GFP_NOIO);
+	input->id.bustype = BUS_I2C;
+	input->dev.parent = &client->dev;
+
+	input_set_drvdata(input, tsdata);
+
+	tsdata->input = input;
+
+	error = input_register_device(input);
+	if (error)
+		goto err_free_input_device;
+
+	if (request_threaded_irq(tsdata->irq, NULL, edt_ft5x06_ts_isr,
+	                         IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
+	                         client->name, tsdata)) {
+		dev_err(&client->dev, "Unable to request touchscreen IRQ.\n");
+		input = NULL;
+		error = -ENOMEM;
+		goto err_unregister_device;
+	}
+
+	error = sysfs_create_group(&client->dev.kobj,
+	                           &edt_ft5x06_i2c_attr_group);
+	if (error)
+		goto err_free_irq;
+
+	device_init_wakeup(&client->dev, 1);
+
+	dev_dbg(&tsdata->client->dev,
+	        "EDT FT5x06 initialized: IRQ pin %d, Reset pin %d.\n",
+	        tsdata->irq_pin, tsdata->reset_pin);
+
+	return 0;
+
+err_free_irq:
+	free_irq(tsdata->irq, tsdata);
+err_unregister_device:
+	input_unregister_device(input);
+	input = NULL;
+err_free_input_device:
+	if (input) {
+		kfree(input->name);
+		input_free_device(input);
+	}
+err_free_irq_pin:
+	gpio_free(tsdata->irq_pin);
+err_free_reset_pin:
+	if (tsdata->reset_pin >= 0)
+		gpio_free(tsdata->reset_pin);
+err_free_tsdata:
+	kfree(tsdata);
+	return error;
+}
+
+static int edt_ft5x06_i2c_ts_remove(struct i2c_client *client)
+{
+	struct edt_ft5x06_i2c_ts_data *tsdata = dev_get_drvdata(&client->dev);
+
+	sysfs_remove_group(&client->dev.kobj, &edt_ft5x06_i2c_attr_group);
+
+	free_irq(tsdata->irq, tsdata);
+	kfree(tsdata->input->name);
+	input_unregister_device(tsdata->input);
+	gpio_free(tsdata->irq_pin);
+	if (tsdata->reset_pin >= 0)
+		gpio_free(tsdata->reset_pin);
+	kfree(tsdata);
+
+	return 0;
+}
+
+static int edt_ft5x06_i2c_ts_suspend(struct device *dev)
+{
+	struct edt_ft5x06_i2c_ts_data *tsdata = dev_get_drvdata(dev);
+
+	if (device_may_wakeup(dev))
+		enable_irq_wake(tsdata->irq);
+
+	return 0;
+}
+
+static int edt_ft5x06_i2c_ts_resume(struct device *dev)
+{
+	struct edt_ft5x06_i2c_ts_data *tsdata = dev_get_drvdata(dev);
+
+	if (device_may_wakeup(dev))
+		disable_irq_wake(tsdata->irq);
+
+	return 0;
+}
+
+static SIMPLE_DEV_PM_OPS(edt_ft5x06_i2c_ts_pm_ops,
+                         edt_ft5x06_i2c_ts_suspend, edt_ft5x06_i2c_ts_resume);
+
+static const struct i2c_device_id edt_ft5x06_i2c_ts_id[] = {
+	{ "edt-ft5x06", 0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, edt_ft5x06_i2c_ts_id);
+
+static struct i2c_driver edt_ft5x06_i2c_ts_driver = {
+	.driver = {
+		.owner = THIS_MODULE,
+		.name = "edt_ft5x06_i2c",
+		.pm = &edt_ft5x06_i2c_ts_pm_ops,
+	},
+	.id_table = edt_ft5x06_i2c_ts_id,
+	.probe    = edt_ft5x06_i2c_ts_probe,
+	.remove   = edt_ft5x06_i2c_ts_remove,
+};
+
+static int __init edt_ft5x06_i2c_ts_init(void)
+{
+	return i2c_add_driver(&edt_ft5x06_i2c_ts_driver);
+}
+module_init(edt_ft5x06_i2c_ts_init);
+
+static void __exit edt_ft5x06_i2c_ts_exit(void)
+{
+	i2c_del_driver(&edt_ft5x06_i2c_ts_driver);
+}
+module_exit(edt_ft5x06_i2c_ts_exit);
+
+MODULE_AUTHOR("Simon Budig <simon.budig@kernelconcepts.de>");
+MODULE_DESCRIPTION("EDT FT5x06 I2C Touchscreen Driver");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/input/edt-ft5x06.h b/include/linux/input/edt-ft5x06.h
new file mode 100644
index 0000000..db4130d
--- /dev/null
+++ b/include/linux/input/edt-ft5x06.h
@@ -0,0 +1,17 @@
+#ifndef _EDT_FT5X06_H
+#define _EDT_FT5X06_H
+
+/*
+ * Copyright (c) 2011 Simon Budig, <simon.budig@kernelconcepts.de>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published by
+ * the Free Software Foundation.
+ */
+
+struct edt_ft5x06_platform_data {
+	int irq_pin;
+	int reset_pin;
+};
+
+#endif /* _EDT_FT5X06_H */
-- 
1.7.2.5

Re: [PATCH] Touchscreen driver for FT5x06 based EDT displays

[Dmitry Torokhov]

Hi Simon,

On Wed, Apr 04, 2012 at 08:27:59PM +0200, simon.budig@kernelconcepts.de wrote:
> From: Simon Budig <simon.budig@kernelconcepts.de>
> 
> This is a driver for the EDT "Polytouch" family of touch controllers
> based on the FocalTech FT5x06 line of chips.
> 
> Signed-off-by: Simon Budig <simon.budig@kernelconcepts.de>
> ---
>  drivers/input/touchscreen/Kconfig      |   13 +
>  drivers/input/touchscreen/Makefile     |    1 +
>  drivers/input/touchscreen/edt-ft5x06.c |  771 ++++++++++++++++++++++++++++++++
>  include/linux/input/edt-ft5x06.h       |   17 +
>  4 files changed, 802 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/input/touchscreen/edt-ft5x06.c
>  create mode 100644 include/linux/input/edt-ft5x06.h
> 
> diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
> index 2a21419..0166a15 100644
> --- a/drivers/input/touchscreen/Kconfig
> +++ b/drivers/input/touchscreen/Kconfig
> @@ -431,6 +431,19 @@ config TOUCHSCREEN_PENMOUNT
>  	  To compile this driver as a module, choose M here: the
>  	  module will be called penmount.
>  
> +config TOUCHSCREEN_EDT_FT5X06
> +	tristate "EDT FocalTech FT5x06 I2C Touchscreen support"
> +	depends on I2C
> +	help
> +	  Say Y here if you have an EDT "Polytouch" touchscreen based
> +	  on the FocalTech FT5x06 family of controllers connected to
> +	  your system.
> +
> +	  If unsure, say N.
> +
> +	  To compile this driver as a module, choose M here: the
> +	  module will be called edt-ft5x06.
> +
>  config TOUCHSCREEN_MIGOR
>  	tristate "Renesas MIGO-R touchscreen"
>  	depends on SH_MIGOR && I2C
> diff --git a/drivers/input/touchscreen/Makefile b/drivers/input/touchscreen/Makefile
> index 3d5cf8c..8a68c84 100644
> --- a/drivers/input/touchscreen/Makefile
> +++ b/drivers/input/touchscreen/Makefile
> @@ -23,6 +23,7 @@ obj-$(CONFIG_TOUCHSCREEN_CYTTSP_I2C)	+= cyttsp_i2c.o
>  obj-$(CONFIG_TOUCHSCREEN_CYTTSP_SPI)	+= cyttsp_spi.o
>  obj-$(CONFIG_TOUCHSCREEN_DA9034)	+= da9034-ts.o
>  obj-$(CONFIG_TOUCHSCREEN_DYNAPRO)	+= dynapro.o
> +obj-$(CONFIG_TOUCHSCREEN_EDT_FT5X06)	+= edt-ft5x06.o
>  obj-$(CONFIG_TOUCHSCREEN_HAMPSHIRE)	+= hampshire.o
>  obj-$(CONFIG_TOUCHSCREEN_GUNZE)		+= gunze.o
>  obj-$(CONFIG_TOUCHSCREEN_EETI)		+= eeti_ts.o
> diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c
> new file mode 100644
> index 0000000..7c4f9f5
> --- /dev/null
> +++ b/drivers/input/touchscreen/edt-ft5x06.c
> @@ -0,0 +1,771 @@
> +/*
> + * Copyright (C) 2011 Simon Budig, <simon.budig@kernelconcepts.de>
> + *
> + * This software is licensed under the terms of the GNU General Public
> + * License version 2, as published by the Free Software Foundation, and
> + * may be copied, distributed, and modified under those terms.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public
> + * License along with this library; if not, write to the Free Software
> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
> + */
> +
> +/*
> + * This is a driver for the EDT "Polytouch" family of touch controllers
> + * based on the FocalTech FT5x06 line of chips.
> + *
> + * Development of this driver has been sponsored by Glyn:
> + *    http://www.glyn.com/Products/Displays
> + */
> +
> +#include <linux/module.h>
> +#include <linux/interrupt.h>
> +#include <linux/input.h>
> +#include <linux/i2c.h>
> +#include <linux/uaccess.h>
> +#include <linux/delay.h>
> +#include <linux/slab.h>
> +
> +#include <linux/gpio.h>
> +
> +#include <linux/input/edt-ft5x06.h>
> +
> +#define DRIVER_VERSION "v0.5"
> +
> +#define WORK_REGISTER_THRESHOLD   0x00
> +#define WORK_REGISTER_REPORT_RATE 0x08
> +#define WORK_REGISTER_GAIN        0x30
> +#define WORK_REGISTER_OFFSET      0x31
> +#define WORK_REGISTER_NUM_X       0x33
> +#define WORK_REGISTER_NUM_Y       0x34
> +
> +#define WORK_REGISTER_OPMODE      0x3c
> +#define FACTORY_REGISTER_OPMODE   0x01
> +
> +struct edt_ft5x06_i2c_ts_data {
> +	struct i2c_client *client;
> +	struct input_dev *input;
> +	int irq;
> +	int irq_pin;
> +	int reset_pin;
> +	int num_x;
> +	int num_y;
> +
> +	struct mutex mutex;
> +	bool factory_mode;
> +	int threshold;
> +	int gain;
> +	int offset;
> +	int report_rate;
> +};
> +
> +static int edt_ft5x06_ts_readwrite(struct i2c_client *client,
> +                                   u16 wr_len, u8 *wr_buf,
> +                                   u16 rd_len, u8 *rd_buf)
> +{
> +	struct i2c_msg wrmsg[2];
> +	int i, ret;
> +
> +	i = 0;
> +	if (wr_len) {
> +		wrmsg[i].addr  = client->addr;
> +		wrmsg[i].flags = 0;
> +		wrmsg[i].len = wr_len;
> +		wrmsg[i].buf = wr_buf;
> +		i++;
> +	}
> +	if (rd_len) {
> +		wrmsg[i].addr  = client->addr;
> +		wrmsg[i].flags = I2C_M_RD;
> +		wrmsg[i].len = rd_len;
> +		wrmsg[i].buf = rd_buf;
> +		i++;
> +	}
> +
> +	ret = i2c_transfer(client->adapter, wrmsg, i);
> +	if (ret < 0) {
> +		dev_err(&client->dev, "i2c_transfer failed: %d\n", ret);
> +		return ret;
> +	}
> +
> +	return ret;
> +}
> +
> +
> +static irqreturn_t edt_ft5x06_ts_isr(int irq, void *dev_id)
> +{
> +	struct edt_ft5x06_i2c_ts_data *tsdata = dev_id;
> +	unsigned char touching = 0;
> +	unsigned char rdbuf[26], wrbuf[1];
> +	int i, have_abs, type, ret;
> +
> +	memset(wrbuf, 0, sizeof(wrbuf));
> +	memset(rdbuf, 0, sizeof(rdbuf));
> +
> +	wrbuf[0] = 0xf9;
> +
> +	mutex_lock(&tsdata->mutex);
> +	ret = edt_ft5x06_ts_readwrite(tsdata->client,
> +	                              1, wrbuf,
> +	                              sizeof(rdbuf), rdbuf);
> +	mutex_unlock(&tsdata->mutex);
> +	if (ret < 0) {
> +		dev_err(&tsdata->client->dev,
> +		        "Unable to write to i2c touchscreen!\n");
> +		goto out;
> +	}
> +
> +	if (rdbuf[0] != 0xaa || rdbuf[1] != 0xaa || rdbuf[2] != 26) {
> +		dev_err(&tsdata->client->dev,
> +		        "Unexpected header: %02x%02x%02x!\n",
> +		        rdbuf[0], rdbuf[1], rdbuf[2]);
> +	}
> +
> +	have_abs = 0;
> +	touching = rdbuf[3];
> +	for (i = 0; i < touching; i++) {
> +		type = rdbuf[i*4+5] >> 6;
> +		/* ignore Touch Down and Reserved events */
> +		if (type == 0x01 || type == 0x03)
> +			continue;
> +
> +		if (!have_abs) {
> +			input_report_key(tsdata->input, BTN_TOUCH,    1);
> +			input_report_abs(tsdata->input, ABS_PRESSURE, 1);
> +			input_report_abs(tsdata->input, ABS_X,
> +			                 ((rdbuf[i*4+5] << 8) |
> +			                  rdbuf[i*4+6]) & 0x0fff);
> +			input_report_abs(tsdata->input, ABS_Y,
> +			                 ((rdbuf[i*4+7] << 8) |
> +			                  rdbuf[i*4+8]) & 0x0fff);
> +			have_abs = 1;


The mt pointer emulation should do this for you.

> +		}
> +		input_report_abs(tsdata->input, ABS_MT_POSITION_X,
> +		                 ((rdbuf[i*4+5] << 8) | rdbuf[i*4+6]) & 0x0fff);
> +		input_report_abs(tsdata->input, ABS_MT_POSITION_Y,
> +		                 ((rdbuf[i*4+7] << 8) | rdbuf[i*4+8]) & 0x0fff);
> +		input_report_abs(tsdata->input, ABS_MT_TRACKING_ID,
> +		                 (rdbuf[i*4+7] >> 4) & 0x0f);
> +		input_mt_sync(tsdata->input);
> +	}
> +	if (!have_abs) {
> +		input_report_key(tsdata->input, BTN_TOUCH,    0);
> +		input_report_abs(tsdata->input, ABS_PRESSURE, 0);
> +	}
> +	input_sync(tsdata->input);
> +
> +out:
> +	return IRQ_HANDLED;
> +}
> +
> +
> +static int edt_ft5x06_i2c_register_write(struct edt_ft5x06_i2c_ts_data *tsdata,
> +                                         u8 addr, u8 value)
> +{
> +	u8 wrbuf[4];
> +	int ret;
> +
> +	wrbuf[0]  = tsdata->factory_mode ? 0xf3 : 0xfc;
> +	wrbuf[1]  = tsdata->factory_mode ? addr & 0x7f : addr & 0x3f;
> +	wrbuf[2]  = value;
> +	wrbuf[3]  = wrbuf[0] ^ wrbuf[1] ^ wrbuf[2];
> +
> +	disable_irq(tsdata->irq);
> +
> +	ret = edt_ft5x06_ts_readwrite(tsdata->client,
> +	                              4, wrbuf,
> +	                              0, NULL);
> +
> +	enable_irq(tsdata->irq);
> +
> +	return ret;
> +}
> +
> +static int edt_ft5x06_i2c_register_read(struct edt_ft5x06_i2c_ts_data *tsdata,
> +                                        u8 addr)
> +{
> +	u8 wrbuf[2], rdbuf[2];
> +	int ret;
> +
> +	wrbuf[0]  = tsdata->factory_mode ? 0xf3 : 0xfc;
> +	wrbuf[1]  = tsdata->factory_mode ? addr & 0x7f : addr & 0x3f;
> +	wrbuf[1] |= tsdata->factory_mode ? 0x80 : 0x40;
> +
> +	disable_irq(tsdata->irq);
> +
> +	ret = edt_ft5x06_ts_readwrite(tsdata->client,
> +	                              2, wrbuf,
> +	                              2, rdbuf);
> +
> +	enable_irq(tsdata->irq);
> +
> +	if ((wrbuf[0] ^ wrbuf[1] ^ rdbuf[0]) != rdbuf[1])
> +		dev_err(&tsdata->client->dev,
> +		        "crc error: 0x%02x expected, got 0x%02x\n",
> +		        (wrbuf[0] ^ wrbuf[1] ^ rdbuf[0]), rdbuf[1]);
> +
> +	return ret < 0 ? ret : rdbuf[0];
> +}
> +
> +static ssize_t edt_ft5x06_i2c_setting_show(struct device *dev,
> +                                           struct device_attribute *attr,
> +                                           char *buf)
> +{
> +	struct edt_ft5x06_i2c_ts_data *tsdata = dev_get_drvdata(dev);
> +	struct i2c_client *client = tsdata->client;
> +	int ret = 0;
> +	int *value;
> +	u8 addr;
> +
> +	switch (attr->attr.name[0]) {
> +	case 't':    /* threshold */
> +		addr = WORK_REGISTER_THRESHOLD;
> +		value = &tsdata->threshold;
> +		break;
> +	case 'g':    /* gain */
> +		addr = WORK_REGISTER_GAIN;
> +		value = &tsdata->gain;
> +		break;
> +	case 'o':    /* offset */
> +		addr = WORK_REGISTER_OFFSET;
> +		value = &tsdata->offset;
> +		break;
> +	case 'r':    /* report rate */
> +		addr = WORK_REGISTER_REPORT_RATE;
> +		value = &tsdata->report_rate;
> +		break;
> +	default:
> +		dev_err(&client->dev,
> +		        "unknown attribute for edt_ft5x06_i2c_setting_show: %s\n",
> +		        attr->attr.name);
> +		return -EINVAL;
> +	}
> +
> +	mutex_lock(&tsdata->mutex);
> +
> +	if (tsdata->factory_mode) {
> +		dev_err(dev,
> +		        "setting register not available in factory mode\n");
> +		mutex_unlock(&tsdata->mutex);
> +		return -EIO;
> +	}
> +
> +	ret = edt_ft5x06_i2c_register_read(tsdata, addr);
> +	if (ret < 0) {
> +		dev_err(&tsdata->client->dev,
> +		        "Unable to write to i2c touchscreen!\n");
> +		mutex_unlock(&tsdata->mutex);
> +		return ret;
> +	}
> +	mutex_unlock(&tsdata->mutex);
> +
> +	if (ret != *value) {
> +		dev_info(&tsdata->client->dev,
> +		         "i2c read (%d) and stored value (%d) differ. Huh?\n",
> +		         ret, *value);
> +		*value = ret;
> +	}
> +
> +	return sprintf(buf, "%d\n", ret);
> +}
> +
> +static ssize_t edt_ft5x06_i2c_setting_store(struct device *dev,
> +                                            struct device_attribute *attr,
> +                                            const char *buf, size_t count)
> +{
> +	struct edt_ft5x06_i2c_ts_data *tsdata = dev_get_drvdata(dev);
> +	struct i2c_client *client = tsdata->client;
> +	int ret = 0;
> +	u8 addr;
> +	unsigned int val;
> +
> +	mutex_lock(&tsdata->mutex);
> +
> +	if (tsdata->factory_mode) {
> +		dev_err(dev,
> +		        "setting register not available in factory mode\n");

This will spam logs, just return error silently.

> +		ret = -EIO;
> +		goto out;
> +	}
> +
> +	if (sscanf(buf, "%u", &val) != 1) {
> +		dev_err(dev, "Invalid value for attribute %s\n",
> +		        attr->attr.name);

As will this.

> +		ret = -EINVAL;
> +		goto out;
> +	}
> +
> +	switch (attr->attr.name[0]) {
> +	case 't':    /* threshold */
> +		addr = WORK_REGISTER_THRESHOLD;
> +		val = val < 20 ? 20 : val > 80 ? 80 : val;
> +		tsdata->threshold = val;
> +		break;
> +	case 'g':    /* gain */
> +		addr = WORK_REGISTER_GAIN;
> +		val = val < 0 ? 0 : val > 31 ? 31 : val;
> +		tsdata->gain = val;
> +		break;
> +	case 'o':    /* offset */
> +		addr = WORK_REGISTER_OFFSET;
> +		val = val < 0 ? 0 : val > 31 ? 31 : val;
> +		tsdata->offset = val;
> +		break;
> +	case 'r':    /* report rate */
> +		addr = WORK_REGISTER_REPORT_RATE;
> +		val = val < 3 ? 3 : val > 14 ? 14 : val;
> +		tsdata->report_rate = val;
> +		break;

See if you could wrap an attribute into your own structure that will
allow you to get to the address, field and limits without matching on
attribute name.

> +	default:
> +		dev_err(&client->dev,
> +		        "unknown attribute for edt_ft5x06_i2c_setting_show: %s\n",
> +		        attr->attr.name);
> +		ret = -EINVAL;
> +		goto out;
> +	}
> +
> +	ret = edt_ft5x06_i2c_register_write(tsdata, addr, val);
> +
> +	if (ret < 0) {
> +		dev_err(&tsdata->client->dev,
> +		        "Unable to write to i2c touchscreen!\n");
> +		goto out;
> +	}
> +
> +out:
> +	mutex_unlock(&tsdata->mutex);
> +	return ret < 0 ? ret : count;
> +}
> +
> +
> +static ssize_t edt_ft5x06_i2c_mode_show(struct device *dev,
> +                                        struct device_attribute *attr,
> +                                        char *buf)
> +{
> +	struct edt_ft5x06_i2c_ts_data *tsdata = dev_get_drvdata(dev);
> +	return sprintf(buf, "%d\n", tsdata->factory_mode ? 1 : 0);
> +}
> +
> +static ssize_t edt_ft5x06_i2c_mode_store(struct device *dev,
> +                                         struct device_attribute *attr,
> +                                         const char *buf, size_t count)
> +{
> +	struct edt_ft5x06_i2c_ts_data *tsdata = dev_get_drvdata(dev);
> +	int i, ret = 0;
> +	unsigned int mode;
> +
> +	if (sscanf(buf, "%u", &mode) != 1 || (mode | 1) != 1) {
> +		dev_err(dev, "Invalid value for operation mode\n");
> +		return -EINVAL;
> +	}
> +
> +	/* no change, return without doing anything */
> +	if (mode == tsdata->factory_mode)
> +		return count;
> +
> +	mutex_lock(&tsdata->mutex);
> +	if (!tsdata->factory_mode) { /* switch to factory mode */
> +		disable_irq(tsdata->irq);
> +		/* mode register is 0x3c when in the work mode */
> +		ret = edt_ft5x06_i2c_register_write(tsdata,
> +		                                    WORK_REGISTER_OPMODE, 0x03);
> +		if (ret < 0) {
> +			dev_err(dev, "failed to switch to factory mode (%d)\n",
> +			        ret);
> +		} else {
> +			tsdata->factory_mode = 1;

= true;

> +			for (i = 0; i < 10; i++) {
> +				mdelay(5);
> +				/* mode register is 0x01 when in factory mode */
> +				ret = edt_ft5x06_i2c_register_read(tsdata, FACTORY_REGISTER_OPMODE);
> +				if (ret == 0x03)
> +					break;
> +			}
> +			if (i == 10)
> +				dev_err(dev,
> +				        "not in factory mode after %dms.\n",
> +				        i*5);
> +		}
> +	} else {  /* switch to work mode */
> +		/* mode register is 0x01 when in the factory mode */
> +		ret = edt_ft5x06_i2c_register_write(tsdata,
> +		                                    FACTORY_REGISTER_OPMODE,
> +		                                    0x01);
> +		if (ret < 0) {
> +			dev_err(dev, "failed to switch to work mode (%d)\n",
> +			        ret);
> +		} else {
> +			tsdata->factory_mode = 0;

= false;

> +			for (i = 0; i < 10; i++) {
> +				mdelay(5);
> +				/* mode register is 0x01 when in factory mode */
> +				ret = edt_ft5x06_i2c_register_read(tsdata, WORK_REGISTER_OPMODE);
> +				if (ret == 0x01)
> +					break;
> +			}
> +			if (i == 10)
> +				dev_err(dev, "not in work mode after %dms.\n",
> +				        i*5);
> +
> +			/* restore parameters */
> +			edt_ft5x06_i2c_register_write(tsdata,
> +			                              WORK_REGISTER_THRESHOLD,
> +			                              tsdata->threshold);
> +			edt_ft5x06_i2c_register_write(tsdata,
> +			                              WORK_REGISTER_GAIN,
> +			                              tsdata->gain);
> +			edt_ft5x06_i2c_register_write(tsdata,
> +			                              WORK_REGISTER_OFFSET,
> +			                              tsdata->offset);
> +			edt_ft5x06_i2c_register_write(tsdata,
> +			                              WORK_REGISTER_REPORT_RATE,
> +			                              tsdata->report_rate);
> +
> +			enable_irq(tsdata->irq);
> +		}
> +	}
> +
> +	mutex_unlock(&tsdata->mutex);
> +	return count;
> +}
> +
> +
> +static ssize_t edt_ft5x06_i2c_raw_data_show(struct device *dev,
> +                                            struct device_attribute *attr,
> +                                            char *buf)
> +{
> +	struct edt_ft5x06_i2c_ts_data *tsdata = dev_get_drvdata(dev);
> +	int i, ret;
> +	char *ptr, wrbuf[3];
> +
> +	if (!tsdata->factory_mode) {
> +		dev_err(dev, "raw data not available in work mode\n");
> +		return -EIO;
> +	}
> +
> +	mutex_lock(&tsdata->mutex);

Instead of taking mutex why don't you disable IRQ?

> +	ret = edt_ft5x06_i2c_register_write(tsdata, 0x08, 0x01);
> +	for (i = 0; i < 100; i++) {
> +		ret = edt_ft5x06_i2c_register_read(tsdata, 0x08);
> +		if (ret < 1)
> +			break;
> +		udelay(1000);
> +	}
> +
> +	if (i == 100 || ret < 0) {
> +		dev_err(dev, "waiting time exceeded or error: %d\n", ret);
> +		mutex_unlock(&tsdata->mutex);
> +		return ret < 0 ? ret : -ETIMEDOUT;
> +	}
> +
> +	ptr = buf;
> +	wrbuf[0] = 0xf5;
> +	wrbuf[1] = 0x0e;
> +	for (i = 0; i <= tsdata->num_x; i++) {
> +		wrbuf[2] = i;
> +		ret = edt_ft5x06_ts_readwrite(tsdata->client,
> +		                               3, wrbuf,
> +		                               tsdata->num_y * 2, ptr);
> +		if (ret < 0) {
> +			mutex_unlock(&tsdata->mutex);
> +			return ret;
> +		}
> +
> +		ptr += tsdata->num_y * 2;
> +	}
> +
> +	mutex_unlock(&tsdata->mutex);
> +	return ptr - buf;
> +}
> +
> +
> +static DEVICE_ATTR(gain,      0664,
> +                   edt_ft5x06_i2c_setting_show, edt_ft5x06_i2c_setting_store);
> +static DEVICE_ATTR(offset,    0664,
> +                   edt_ft5x06_i2c_setting_show, edt_ft5x06_i2c_setting_store);
> +static DEVICE_ATTR(threshold, 0664,
> +                   edt_ft5x06_i2c_setting_show, edt_ft5x06_i2c_setting_store);
> +static DEVICE_ATTR(report_rate, 0664,
> +                   edt_ft5x06_i2c_setting_show, edt_ft5x06_i2c_setting_store);
> +static DEVICE_ATTR(mode,      0664,
> +                   edt_ft5x06_i2c_mode_show, edt_ft5x06_i2c_mode_store);
> +static DEVICE_ATTR(raw_data,  0444,
> +                   edt_ft5x06_i2c_raw_data_show, NULL);
> +
> +static struct attribute *edt_ft5x06_i2c_attrs[] = {
> +	&dev_attr_gain.attr,
> +	&dev_attr_offset.attr,
> +	&dev_attr_threshold.attr,
> +	&dev_attr_report_rate.attr,
> +	&dev_attr_mode.attr,
> +	&dev_attr_raw_data.attr,
> +	NULL
> +};
> +
> +static const struct attribute_group edt_ft5x06_i2c_attr_group = {
> +	.attrs = edt_ft5x06_i2c_attrs,
> +};
> +
> +static int edt_ft5x06_i2c_ts_probe(struct i2c_client *client,
> +                                   const struct i2c_device_id *id)
> +{
> +
> +	struct edt_ft5x06_i2c_ts_data *tsdata;
> +	struct edt_ft5x06_platform_data *pdata;

const.

> +	struct input_dev *input;
> +	int error;
> +	u8 rdbuf[23];
> +	char *model_name, *fw_version;
> +
> +	dev_dbg(&client->dev, "probing for EDT FT5x06 I2C\n");
> +
> +	if (!client->dev.platform_data) {
> +		dev_err(&client->dev, "no platform data?\n");
> +		return -ENODEV;
> +	}
> +
> +	tsdata = kzalloc(sizeof(*tsdata), GFP_KERNEL);
> +	if (!tsdata) {
> +		dev_err(&client->dev, "failed to allocate driver data!\n");
> +		dev_set_drvdata(&client->dev, NULL);
> +		return -ENOMEM;
> +	}
> +
> +	dev_set_drvdata(&client->dev, tsdata);
> +	tsdata->client = client;
> +	pdata = client->dev.platform_data;
> +
> +	tsdata->reset_pin = pdata->reset_pin;
> +	mutex_init(&tsdata->mutex);
> +
> +	if (tsdata->reset_pin >= 0) {
> +		error = gpio_request(tsdata->reset_pin, "edt-ft5x06 reset");
> +		if (error < 0) {
> +			dev_err(&client->dev,
> +			        "Failed to request GPIO %d as reset pin, error %d\n",
> +			         tsdata->reset_pin, error);
> +			goto err_free_tsdata;
> +		}
> +
> +		/* this pulls reset down, enabling the low active reset */
> +		if (gpio_direction_output(tsdata->reset_pin, 0) < 0) {
> +			dev_info(&client->dev, "switching to output failed\n");
> +			goto err_free_reset_pin;
> +		}
> +	}
> +
> +	/* request IRQ pin */
> +	tsdata->irq_pin = pdata->irq_pin;
> +	tsdata->irq = gpio_to_irq(tsdata->irq_pin);

Take from the client.

> +
> +	error = gpio_request(tsdata->irq_pin, "edt-ft5x06 irq");
> +	if (error < 0) {
> +		dev_err(&client->dev,
> +		        "Failed to request GPIO %d for IRQ %d, error %d\n",
> +		        tsdata->irq_pin, tsdata->irq, error);
> +		goto err_free_reset_pin;
> +	}
> +	gpio_direction_input(tsdata->irq_pin);

Should this GPIO configuration be done by the driver or by the board
code that configures i2c client? I think latter is better.

> +
> +	if (tsdata->reset_pin >= 0) {
> +		/* release reset */
> +		mdelay(50);
> +		gpio_set_value(tsdata->reset_pin, 1);
> +		mdelay(100);
> +	}
> +
> +	mutex_lock(&tsdata->mutex);

Why are you taking this lock here?

> +
> +	tsdata->factory_mode = 0;
> +
> +	if (edt_ft5x06_ts_readwrite(client, 1, "\xbb", 22, rdbuf) < 0) {
> +		dev_err(&client->dev, "probing failed\n");
> +		error = -ENODEV;
> +		goto err_free_irq_pin;
> +	}
> +
> +	tsdata->threshold = edt_ft5x06_i2c_register_read(tsdata,
> +	                                                 WORK_REGISTER_THRESHOLD);
> +	tsdata->gain      = edt_ft5x06_i2c_register_read(tsdata,
> +	                                                 WORK_REGISTER_GAIN);
> +	tsdata->offset    = edt_ft5x06_i2c_register_read(tsdata,
> +	                                                 WORK_REGISTER_OFFSET);
> +	tsdata->report_rate = edt_ft5x06_i2c_register_read(tsdata,
> +	                                                 WORK_REGISTER_REPORT_RATE);
> +	tsdata->num_x     = edt_ft5x06_i2c_register_read(tsdata,
> +	                                                 WORK_REGISTER_NUM_X);
> +	tsdata->num_y     = edt_ft5x06_i2c_register_read(tsdata,
> +	                                                 WORK_REGISTER_NUM_Y);
> +
> +	mutex_unlock(&tsdata->mutex);
> +
> +	/* remove last '$' end marker */
> +	rdbuf[22] = '\0';
> +	if (rdbuf[21] == '$')
> +		rdbuf[21] = '\0';
> +
> +	model_name = rdbuf + 1;
> +	/* look for Model/Version separator */
> +	fw_version = strchr(rdbuf, '*');
> +
> +	if (fw_version) {
> +		fw_version[0] = '\0';
> +		fw_version++;
> +		dev_info(&client->dev,
> +		         "Model \"%s\", Rev. \"%s\", %dx%d sensors\n",
> +		         model_name, fw_version, tsdata->num_x, tsdata->num_y);
> +	} else {
> +		dev_info(&client->dev, "Product ID \"%s\"\n", model_name);
> +	}
> +
> +	input = input_allocate_device();
> +	if (!input) {
> +		dev_err(&client->dev, "failed to allocate input device!\n");
> +		error = -ENOMEM;
> +		goto err_free_irq_pin;
> +	}
> +
> +	__set_bit(EV_SYN, input->evbit);
> +	__set_bit(EV_KEY, input->evbit);
> +	__set_bit(EV_ABS, input->evbit);
> +	__set_bit(BTN_TOUCH, input->keybit);
> +	input_set_abs_params(input, ABS_X, 0, tsdata->num_x * 64 - 1, 0, 0);
> +	input_set_abs_params(input, ABS_Y, 0, tsdata->num_y * 64 - 1, 0, 0);
> +	input_set_abs_params(input, ABS_PRESSURE, 0, 1, 0, 0);
> +	input_set_abs_params(input, ABS_MT_POSITION_X,
> +	                     0, tsdata->num_x * 64 - 1, 0, 0);
> +	input_set_abs_params(input, ABS_MT_POSITION_Y,
> +	                     0, tsdata->num_y * 64 - 1, 0, 0);
> +	input_set_abs_params(input, ABS_MT_TRACKING_ID, 0, 15, 0, 0);
> +
> +	input->name = kstrdup(model_name, GFP_NOIO);

Why don't you just allocate a few bytes in tsdata structure instead of
allocating and managing this separately. You'll also avoid race when
freeing it.

> +	input->id.bustype = BUS_I2C;
> +	input->dev.parent = &client->dev;
> +
> +	input_set_drvdata(input, tsdata);
> +
> +	tsdata->input = input;
> +
> +	error = input_register_device(input);
> +	if (error)
> +		goto err_free_input_device;
> +
> +	if (request_threaded_irq(tsdata->irq, NULL, edt_ft5x06_ts_isr,
> +	                         IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
> +	                         client->name, tsdata)) {
> +		dev_err(&client->dev, "Unable to request touchscreen IRQ.\n");
> +		input = NULL;

Why?

> +		error = -ENOMEM;
> +		goto err_unregister_device;
> +	}
> +
> +	error = sysfs_create_group(&client->dev.kobj,
> +	                           &edt_ft5x06_i2c_attr_group);
> +	if (error)
> +		goto err_free_irq;
> +
> +	device_init_wakeup(&client->dev, 1);
> +
> +	dev_dbg(&tsdata->client->dev,
> +	        "EDT FT5x06 initialized: IRQ pin %d, Reset pin %d.\n",
> +	        tsdata->irq_pin, tsdata->reset_pin);
> +
> +	return 0;
> +
> +err_free_irq:
> +	free_irq(tsdata->irq, tsdata);
> +err_unregister_device:
> +	input_unregister_device(input);
> +	input = NULL;

And here you leak input->name. Just store it in tsdata (as char[16] or
something), it's easier.

> +err_free_input_device:
> +	if (input) {
> +		kfree(input->name);
> +		input_free_device(input);
> +	}
> +err_free_irq_pin:
> +	gpio_free(tsdata->irq_pin);
> +err_free_reset_pin:
> +	if (tsdata->reset_pin >= 0)
> +		gpio_free(tsdata->reset_pin);
> +err_free_tsdata:
> +	kfree(tsdata);
> +	return error;
> +}
> +
> +static int edt_ft5x06_i2c_ts_remove(struct i2c_client *client)
> +{
> +	struct edt_ft5x06_i2c_ts_data *tsdata = dev_get_drvdata(&client->dev);
> +
> +	sysfs_remove_group(&client->dev.kobj, &edt_ft5x06_i2c_attr_group);
> +
> +	free_irq(tsdata->irq, tsdata);
> +	kfree(tsdata->input->name);

You are opening potential for dereferencing NULL pointer (someone could
access the name attribute before input device is unregistered).

> +	input_unregister_device(tsdata->input);
> +	gpio_free(tsdata->irq_pin);
> +	if (tsdata->reset_pin >= 0)
> +		gpio_free(tsdata->reset_pin);
> +	kfree(tsdata);
> +
> +	return 0;
> +}
> +

#ifdef CONFIG_PM_SLEEP
> +static int edt_ft5x06_i2c_ts_suspend(struct device *dev)
> +{
> +	struct edt_ft5x06_i2c_ts_data *tsdata = dev_get_drvdata(dev);
> +
> +	if (device_may_wakeup(dev))
> +		enable_irq_wake(tsdata->irq);
> +
> +	return 0;
> +}
> +
> +static int edt_ft5x06_i2c_ts_resume(struct device *dev)
> +{
> +	struct edt_ft5x06_i2c_ts_data *tsdata = dev_get_drvdata(dev);
> +
> +	if (device_may_wakeup(dev))
> +		disable_irq_wake(tsdata->irq);
> +
> +	return 0;
> +}
#endif

> +
> +static SIMPLE_DEV_PM_OPS(edt_ft5x06_i2c_ts_pm_ops,
> +                         edt_ft5x06_i2c_ts_suspend, edt_ft5x06_i2c_ts_resume);
> +
> +static const struct i2c_device_id edt_ft5x06_i2c_ts_id[] = {
> +	{ "edt-ft5x06", 0 },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(i2c, edt_ft5x06_i2c_ts_id);
> +
> +static struct i2c_driver edt_ft5x06_i2c_ts_driver = {
> +	.driver = {
> +		.owner = THIS_MODULE,
> +		.name = "edt_ft5x06_i2c",
> +		.pm = &edt_ft5x06_i2c_ts_pm_ops,
> +	},
> +	.id_table = edt_ft5x06_i2c_ts_id,
> +	.probe    = edt_ft5x06_i2c_ts_probe,
> +	.remove   = edt_ft5x06_i2c_ts_remove,
> +};
> +
> +static int __init edt_ft5x06_i2c_ts_init(void)
> +{
> +	return i2c_add_driver(&edt_ft5x06_i2c_ts_driver);
> +}
> +module_init(edt_ft5x06_i2c_ts_init);
> +
> +static void __exit edt_ft5x06_i2c_ts_exit(void)
> +{
> +	i2c_del_driver(&edt_ft5x06_i2c_ts_driver);
> +}
> +module_exit(edt_ft5x06_i2c_ts_exit);

Replace with:

module_i2c_driver(edt_ft5x06_i2c_ts_driver);

> +
> +MODULE_AUTHOR("Simon Budig <simon.budig@kernelconcepts.de>");
> +MODULE_DESCRIPTION("EDT FT5x06 I2C Touchscreen Driver");
> +MODULE_LICENSE("GPL");
> diff --git a/include/linux/input/edt-ft5x06.h b/include/linux/input/edt-ft5x06.h
> new file mode 100644
> index 0000000..db4130d
> --- /dev/null
> +++ b/include/linux/input/edt-ft5x06.h
> @@ -0,0 +1,17 @@
> +#ifndef _EDT_FT5X06_H
> +#define _EDT_FT5X06_H
> +
> +/*
> + * Copyright (c) 2011 Simon Budig, <simon.budig@kernelconcepts.de>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License version 2 as published by
> + * the Free Software Foundation.
> + */
> +
> +struct edt_ft5x06_platform_data {
> +	int irq_pin;
> +	int reset_pin;
> +};
> +
> +#endif /* _EDT_FT5X06_H */
> -- 
> 1.7.2.5
> 

-- 
Dmitry

Re: [PATCH] Touchscreen driver for FT5x06 based EDT displays

[Simon Budig]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hi Dmitry.

On 04/04/2012 09:10 PM, Dmitry Torokhov wrote:
> On Wed, Apr 04, 2012 at 08:27:59PM +0200, simon.budig@kernelconcepts.de wrote:
>> +		if (!have_abs) {
>> +			input_report_key(tsdata->input, BTN_TOUCH,    1);
>> +			input_report_abs(tsdata->input, ABS_PRESSURE, 1);
>> +			input_report_abs(tsdata->input, ABS_X,
>> +			                 ((rdbuf[i*4+5] << 8) |
>> +			                  rdbuf[i*4+6]) & 0x0fff);
>> +			input_report_abs(tsdata->input, ABS_Y,
>> +			                 ((rdbuf[i*4+7] << 8) |
>> +			                  rdbuf[i*4+8]) & 0x0fff);
>> +			have_abs = 1;
> 
> 
> The mt pointer emulation should do this for you.

Can you point me to some documentation on that? Do I need to enable this?

[...]

>> +	mutex_lock(&tsdata->mutex);
>> +
>> +	if (tsdata->factory_mode) {
>> +		dev_err(dev,
>> +		        "setting register not available in factory mode\n");
> 
> This will spam logs, just return error silently.

Hmm, the idea was to give the user a hint for an attempt to read the
attribute values in factory mode instead of just silently failing.

Where would be a proper place to document such device-specific behaviour?

[...]

> See if you could wrap an attribute into your own structure that will
> allow you to get to the address, field and limits without matching on
> attribute name.

will try.

>> +	mutex_lock(&tsdata->mutex);
> 
> Instead of taking mutex why don't you disable IRQ?

Does the Linux kernel guarantee that there is just one attribute access
at a time?

Otherwise:

The reason for this locking is two fold.

First, the touch sensor has two modes, a "operation mode" and a "factory
mode". The problem is, that the register numbering in the two modes is
mostly disjunct. I.e. reading the "gain" register in factory mode gives
a number, which has no real connection to the actual gain value. Since
the mode can be changed via a sysfs file I have a potential race
condition when e.g. concurrent access to the sysfs entries happen:

Lets say I want to read the gain value, while something else tries to
switch to factory mode.

1) edt_ft5x06_i2c_setting_show checks for factory_mode == 0
2) edt_ft5x06_i2c_mode_store changes factory_mode to 1
3) edt_ft5x06_i2c_setting_show reads register 0x30, reading and
returning a bogus value.

Also reading raw values is a series of i2c transactions (for each column
of the sensor) and I need to avoid at least a mode change inbetween.
That is the purpose of the mutex.

[...]
>> +static int edt_ft5x06_i2c_ts_probe(struct i2c_client *client,
>> +                                   const struct i2c_device_id *id)
>> +{
>> +
>> +	struct edt_ft5x06_i2c_ts_data *tsdata;
>> +	struct edt_ft5x06_platform_data *pdata;
> 
> const.

What do you mean?

[..]
>> +	/* request IRQ pin */
>> +	tsdata->irq_pin = pdata->irq_pin;
>> +	tsdata->irq = gpio_to_irq(tsdata->irq_pin);
> 
> Take from the client.
[...]
> Should this GPIO configuration be done by the driver or by the board
> code that configures i2c client? I think latter is better.

I got conflicting opinions when I asked for advice on this last december
(before changing it to a pin-based configuration). Why do you think that
your suggestion is better?

>> +	mutex_lock(&tsdata->mutex);
> 
> Why are you taking this lock here?

Paranoia. I wanted to ensure that the controller stays in
non-factory-mode because I am about to read some configuration
registers- However, the attributes are probably not yet available to
userspace, so I can probably skip the mutex in _probe.

>> +	input->name = kstrdup(model_name, GFP_NOIO);
> 
> Why don't you just allocate a few bytes in tsdata structure instead of
> allocating and managing this separately. You'll also avoid race when
> freeing it.

Yeah, I guess that is simpler.

Thanks for the review.
        Simon
- -- 
       Simon Budig                        kernel concepts GmbH
       simon.budig@kernelconcepts.de      Sieghuetter Hauptweg 48
       +49-271-771091-17                  D-57072 Siegen

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAk98tI4ACgkQO2O/RXesiHAopwCfTnRzBEiFbN/tGcRl/TLBl7yR
KpwAn13Bzx+Q6Avj0edwwC+6qkPjAhfq
=Zg0q
-----END PGP SIGNATURE-----

Re: [PATCH] Touchscreen driver for FT5x06 based EDT displays

[Dmitry Torokhov]

On Wed, Apr 04, 2012 at 10:52:30PM +0200, Simon Budig wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> Hi Dmitry.
> 
> On 04/04/2012 09:10 PM, Dmitry Torokhov wrote:
> > On Wed, Apr 04, 2012 at 08:27:59PM +0200, simon.budig@kernelconcepts.de wrote:
> >> +		if (!have_abs) {
> >> +			input_report_key(tsdata->input, BTN_TOUCH,    1);
> >> +			input_report_abs(tsdata->input, ABS_PRESSURE, 1);
> >> +			input_report_abs(tsdata->input, ABS_X,
> >> +			                 ((rdbuf[i*4+5] << 8) |
> >> +			                  rdbuf[i*4+6]) & 0x0fff);
> >> +			input_report_abs(tsdata->input, ABS_Y,
> >> +			                 ((rdbuf[i*4+7] << 8) |
> >> +			                  rdbuf[i*4+8]) & 0x0fff);
> >> +			have_abs = 1;
> > 
> > 
> > The mt pointer emulation should do this for you.
> 
> Can you point me to some documentation on that? Do I need to enable this?
> 

Just do

	input_mt_report_pointer_emulation(tsdata->input, true);

> [...]
> 
> >> +	mutex_lock(&tsdata->mutex);
> >> +
> >> +	if (tsdata->factory_mode) {
> >> +		dev_err(dev,
> >> +		        "setting register not available in factory mode\n");
> > 
> > This will spam logs, just return error silently.
> 
> Hmm, the idea was to give the user a hint for an attempt to read the
> attribute values in factory mode instead of just silently failing.
> 
> Where would be a proper place to document such device-specific behaviour?


Maybe Documentation/input/...? Thisis hardware-mandated behavior, right?

> 
> [...]
> 
> > See if you could wrap an attribute into your own structure that will
> > allow you to get to the address, field and limits without matching on
> > attribute name.
> 
> will try.
> 
> >> +	mutex_lock(&tsdata->mutex);
> > 
> > Instead of taking mutex why don't you disable IRQ?
> 
> Does the Linux kernel guarantee that there is just one attribute access
> at a time?
> 
> Otherwise:
> 
> The reason for this locking is two fold.
> 
> First, the touch sensor has two modes, a "operation mode" and a "factory
> mode". The problem is, that the register numbering in the two modes is
> mostly disjunct. I.e. reading the "gain" register in factory mode gives
> a number, which has no real connection to the actual gain value. Since
> the mode can be changed via a sysfs file I have a potential race
> condition when e.g. concurrent access to the sysfs entries happen:
> 
> Lets say I want to read the gain value, while something else tries to
> switch to factory mode.
> 
> 1) edt_ft5x06_i2c_setting_show checks for factory_mode == 0
> 2) edt_ft5x06_i2c_mode_store changes factory_mode to 1
> 3) edt_ft5x06_i2c_setting_show reads register 0x30, reading and
> returning a bogus value.
> 
> Also reading raw values is a series of i2c transactions (for each column
> of the sensor) and I need to avoid at least a mode change inbetween.
> That is the purpose of the mutex.

Ah, I see.

> 
> [...]
> >> +static int edt_ft5x06_i2c_ts_probe(struct i2c_client *client,
> >> +                                   const struct i2c_device_id *id)
> >> +{
> >> +
> >> +	struct edt_ft5x06_i2c_ts_data *tsdata;
> >> +	struct edt_ft5x06_platform_data *pdata;
> > 
> > const.
> 
> What do you mean?

	const struct edt_ft5x06_platform_data *pdata = client->dev.platform_data;

> 
> [..]
> >> +	/* request IRQ pin */
> >> +	tsdata->irq_pin = pdata->irq_pin;
> >> +	tsdata->irq = gpio_to_irq(tsdata->irq_pin);
> > 
> > Take from the client.
> [...]
> > Should this GPIO configuration be done by the driver or by the board
> > code that configures i2c client? I think latter is better.
> 
> I got conflicting opinions when I asked for advice on this last december
> (before changing it to a pin-based configuration). Why do you think that
> your suggestion is better?

Can the device be connected via a pin not plugged into gpio subsystem?
Other than deriving IRQ from it you do not seem to be using it.

> 
> >> +	mutex_lock(&tsdata->mutex);
> > 
> > Why are you taking this lock here?
> 
> Paranoia. I wanted to ensure that the controller stays in
> non-factory-mode because I am about to read some configuration
> registers- However, the attributes are probably not yet available to
> userspace, so I can probably skip the mutex in _probe.
> 
> >> +	input->name = kstrdup(model_name, GFP_NOIO);
> > 
> > Why don't you just allocate a few bytes in tsdata structure instead of
> > allocating and managing this separately. You'll also avoid race when
> > freeing it.
> 
> Yeah, I guess that is simpler.
> 
> Thanks for the review.
>         Simon
> - -- 
>        Simon Budig                        kernel concepts GmbH
>        simon.budig@kernelconcepts.de      Sieghuetter Hauptweg 48
>        +49-271-771091-17                  D-57072 Siegen
> 
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.11 (GNU/Linux)
> Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
> 
> iEYEARECAAYFAk98tI4ACgkQO2O/RXesiHAopwCfTnRzBEiFbN/tGcRl/TLBl7yR
> KpwAn13Bzx+Q6Avj0edwwC+6qkPjAhfq
> =Zg0q
> -----END PGP SIGNATURE-----

-- 
Dmitry

Re: [PATCH] Touchscreen driver for FT5x06 based EDT displays

[Simon Budig]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

[Sorry if the quoting level is messed up in this mail, apparently
thunderbird thinks that it is a good idea to exchange the quoting levels
of the last two replies. I tried fixing it manually...]

Hi Dmitry.

On 04/04/2012 11:09 PM, Dmitry Torokhov wrote:
> > > The mt pointer emulation should do this for you.
> >
> > Can you point me to some documentation on that? Do I need to enable this?
>
> Just do
> 
> 	input_mt_report_pointer_emulation(tsdata->input, true);

That only seems to work when using slots (which I don't do yet and while
I am willing to change it I can't do it right now since other software
depends on the type-a protocol...).

> > Where would be a proper place to document such device-specific behaviour?
> Maybe Documentation/input/...? Thisis hardware-mandated behavior, right?

Yeah, I guess I'll cook up a small document.

> > > Should this GPIO configuration be done by the driver or by the board
> > > code that configures i2c client? I think latter is better.
> > I got conflicting opinions when I asked for advice on this last december
> > (before changing it to a pin-based configuration). Why do you think that
> > your suggestion is better?
> 
> Can the device be connected via a pin not plugged into gpio subsystem?
> Other than deriving IRQ from it you do not seem to be using it.

Well, I do configure the pin as input as well.

Which would then probably have to move to the board file, where there is
no real good place to put it. Or you invent some sort of callback
functions in the platform device structure so that the driver calls back
into the boardfile.

I dunno. Seems like a lot of overhead for very rare/obscure usecases.

I'll prepare a new version of the patch incorporating the other feedback
from you. Hopefully I'll manage to do this in the next few days.

Bye,
        Simon

- -- 
       Simon Budig                        kernel concepts GmbH
       simon.budig@kernelconcepts.de      Sieghuetter Hauptweg 48
       +49-271-771091-17                  D-57072 Siegen

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAk99c6kACgkQO2O/RXesiHBszwCgsiQBnnOC6qeK379gSeLTUHz+
QWEAoKcVK1kKaE0Uq3KiPmaf7dGozzPA
=W+St
-----END PGP SIGNATURE-----

Re: [PATCH] Touchscreen driver for FT5x06 based EDT displays

[Simon Budig]

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

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 04/04/2012 09:10 PM, Dmitry Torokhov wrote:
[suggestions for my patch]

Based on your input I have prepared two patches which might be easier to
review than the complete diff.

The first one is IMHO pretty straightforward and implements most of the
small bits.

The second one sits on top of the first one and is an attempt at more
generic attribute handling. I must say that I am not too happy about it,
but maybe I am thinking too convoluted here. It saves about 3 lines of
code and exchanges a lot of easy and straightforward code with stuff
that is harder to follow - especially the lookup of the matching
attribute makes me unhappy. There are bits in it I like and will
probably use. But the lookup...

any suggestions are welcome.

Bye,
        Simon

- -- 
       Simon Budig                        kernel concepts GmbH
       simon.budig@kernelconcepts.de      Sieghuetter Hauptweg 48
       +49-271-771091-17                  D-57072 Siegen

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAk99lfoACgkQO2O/RXesiHAE6gCgw2EK5b6MKzfYY7ekEwdNy4FG
GlUAn2db538XupC6i7OZoP06Zb+RFaBI
=UTET
-----END PGP SIGNATURE-----

[-- Attachment #2: edt-fixes-1.patch --]
[-- Type: text/x-patch, Size: 6481 bytes --]

commit c4429d03fd6bffd51eb5b47dfb173ecd6c015e10
Author: Simon Budig <simon.budig@kernelconcepts.de>
Date:   Wed Apr 4 23:21:51 2012 +0200

    incorporate a set of fixes from Dmitry Torokhov

diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c
index 58e35c3..d96eb8f 100644
--- a/drivers/input/touchscreen/edt-ft5x06.c
+++ b/drivers/input/touchscreen/edt-ft5x06.c
@@ -62,6 +62,7 @@ struct edt_ft5x06_i2c_ts_data {
 	int gain;
 	int offset;
 	int report_rate;
+	char name[24];
 };
 
 static int edt_ft5x06_ts_readwrite(struct i2c_client *client,
@@ -102,7 +103,7 @@ static irqreturn_t edt_ft5x06_ts_isr(int irq, void *dev_id)
 	struct edt_ft5x06_i2c_ts_data *tsdata = dev_id;
 	unsigned char touching = 0;
 	unsigned char rdbuf[26], wrbuf[1];
-	int i, have_abs, type, ret;
+	int i, have_abs, type, x, y, id, ret;
 
 	memset(wrbuf, 0, sizeof(wrbuf));
 	memset(rdbuf, 0, sizeof(rdbuf));
@@ -126,7 +127,7 @@ static irqreturn_t edt_ft5x06_ts_isr(int irq, void *dev_id)
 		        rdbuf[0], rdbuf[1], rdbuf[2]);
 	}
 
-	have_abs = 0;
+	have_abs = false;
 	touching = rdbuf[3];
 	for (i = 0; i < touching; i++) {
 		type = rdbuf[i*4+5] >> 6;
@@ -134,23 +135,20 @@ static irqreturn_t edt_ft5x06_ts_isr(int irq, void *dev_id)
 		if (type == 0x01 || type == 0x03)
 			continue;
 
+		x = ((rdbuf[i*4+5] << 8) | rdbuf[i*4+6]) & 0x0fff;
+		y = ((rdbuf[i*4+7] << 8) | rdbuf[i*4+8]) & 0x0fff;
+		id = (rdbuf[i*4+7] >> 4) & 0x0f;
+
 		if (!have_abs) {
 			input_report_key(tsdata->input, BTN_TOUCH,    1);
 			input_report_abs(tsdata->input, ABS_PRESSURE, 1);
-			input_report_abs(tsdata->input, ABS_X,
-			                 ((rdbuf[i*4+5] << 8) |
-			                  rdbuf[i*4+6]) & 0x0fff);
-			input_report_abs(tsdata->input, ABS_Y,
-			                 ((rdbuf[i*4+7] << 8) |
-			                  rdbuf[i*4+8]) & 0x0fff);
-			have_abs = 1;
+			input_report_abs(tsdata->input, ABS_X,        x);
+			input_report_abs(tsdata->input, ABS_Y,        y);
+			have_abs = true;
 		}
-		input_report_abs(tsdata->input, ABS_MT_POSITION_X,
-		                 ((rdbuf[i*4+5] << 8) | rdbuf[i*4+6]) & 0x0fff);
-		input_report_abs(tsdata->input, ABS_MT_POSITION_Y,
-		                 ((rdbuf[i*4+7] << 8) | rdbuf[i*4+8]) & 0x0fff);
-		input_report_abs(tsdata->input, ABS_MT_TRACKING_ID,
-		                 (rdbuf[i*4+7] >> 4) & 0x0f);
+		input_report_abs(tsdata->input, ABS_MT_POSITION_X, x);
+		input_report_abs(tsdata->input, ABS_MT_POSITION_Y, y);
+		input_report_abs(tsdata->input, ABS_MT_TRACKING_ID, id);
 		input_mt_sync(tsdata->input);
 	}
 	if (!have_abs) {
@@ -378,7 +376,7 @@ static ssize_t edt_ft5x06_i2c_mode_store(struct device *dev,
 			dev_err(dev, "failed to switch to factory mode (%d)\n",
 			        ret);
 		} else {
-			tsdata->factory_mode = 1;
+			tsdata->factory_mode = true;
 			for (i = 0; i < 10; i++) {
 				mdelay(5);
 				/* mode register is 0x01 when in factory mode */
@@ -400,7 +398,7 @@ static ssize_t edt_ft5x06_i2c_mode_store(struct device *dev,
 			dev_err(dev, "failed to switch to work mode (%d)\n",
 			        ret);
 		} else {
-			tsdata->factory_mode = 0;
+			tsdata->factory_mode = false;
 			for (i = 0; i < 10; i++) {
 				mdelay(5);
 				/* mode register is 0x01 when in factory mode */
@@ -515,8 +513,8 @@ static int edt_ft5x06_i2c_ts_probe(struct i2c_client *client,
                                    const struct i2c_device_id *id)
 {
 
+	const struct edt_ft5x06_platform_data *pdata = client->dev.platform_data;
 	struct edt_ft5x06_i2c_ts_data *tsdata;
-	struct edt_ft5x06_platform_data *pdata;
 	struct input_dev *input;
 	int error;
 	u8 rdbuf[23];
@@ -524,7 +522,7 @@ static int edt_ft5x06_i2c_ts_probe(struct i2c_client *client,
 
 	dev_dbg(&client->dev, "probing for EDT FT5x06 I2C\n");
 
-	if (!client->dev.platform_data) {
+	if (!pdata) {
 		dev_err(&client->dev, "no platform data?\n");
 		return -ENODEV;
 	}
@@ -538,7 +536,6 @@ static int edt_ft5x06_i2c_ts_probe(struct i2c_client *client,
 
 	dev_set_drvdata(&client->dev, tsdata);
 	tsdata->client = client;
-	pdata = client->dev.platform_data;
 
 	tsdata->reset_pin = pdata->reset_pin;
 	mutex_init(&tsdata->mutex);
@@ -581,7 +578,7 @@ static int edt_ft5x06_i2c_ts_probe(struct i2c_client *client,
 
 	mutex_lock(&tsdata->mutex);
 
-	tsdata->factory_mode = 0;
+	tsdata->factory_mode = false;
 
 	if (edt_ft5x06_ts_readwrite(client, 1, "\xbb", 22, rdbuf) < 0) {
 		dev_err(&client->dev, "probing failed\n");
@@ -622,6 +619,7 @@ static int edt_ft5x06_i2c_ts_probe(struct i2c_client *client,
 	} else {
 		dev_info(&client->dev, "Product ID \"%s\"\n", model_name);
 	}
+	strncpy (tsdata->name, model_name, sizeof (tsdata->name) - 1);
 
 	input = input_allocate_device();
 	if (!input) {
@@ -643,7 +641,7 @@ static int edt_ft5x06_i2c_ts_probe(struct i2c_client *client,
 	                     0, tsdata->num_y * 64 - 1, 0, 0);
 	input_set_abs_params(input, ABS_MT_TRACKING_ID, 0, 15, 0, 0);
 
-	input->name = kstrdup(model_name, GFP_NOIO);
+	input->name = tsdata->name;
 	input->id.bustype = BUS_I2C;
 	input->dev.parent = &client->dev;
 
@@ -659,7 +657,6 @@ static int edt_ft5x06_i2c_ts_probe(struct i2c_client *client,
 	                         IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
 	                         client->name, tsdata)) {
 		dev_err(&client->dev, "Unable to request touchscreen IRQ.\n");
-		input = NULL;
 		error = -ENOMEM;
 		goto err_unregister_device;
 	}
@@ -683,10 +680,8 @@ err_unregister_device:
 	input_unregister_device(input);
 	input = NULL;
 err_free_input_device:
-	if (input) {
-		kfree(input->name);
+	if (input)
 		input_free_device(input);
-	}
 err_free_irq_pin:
 	gpio_free(tsdata->irq_pin);
 err_free_reset_pin:
@@ -704,7 +699,6 @@ static int edt_ft5x06_i2c_ts_remove(struct i2c_client *client)
 	sysfs_remove_group(&client->dev.kobj, &edt_ft5x06_i2c_attr_group);
 
 	free_irq(tsdata->irq, tsdata);
-	kfree(tsdata->input->name);
 	input_unregister_device(tsdata->input);
 	gpio_free(tsdata->irq_pin);
 	if (tsdata->reset_pin >= 0)
@@ -714,6 +708,7 @@ static int edt_ft5x06_i2c_ts_remove(struct i2c_client *client)
 	return 0;
 }
 
+#ifdef CONFIG_PM_SLEEP
 static int edt_ft5x06_i2c_ts_suspend(struct device *dev)
 {
 	struct edt_ft5x06_i2c_ts_data *tsdata = dev_get_drvdata(dev);
@@ -733,6 +728,7 @@ static int edt_ft5x06_i2c_ts_resume(struct device *dev)
 
 	return 0;
 }
+#endif
 
 static SIMPLE_DEV_PM_OPS(edt_ft5x06_i2c_ts_pm_ops,
                          edt_ft5x06_i2c_ts_suspend, edt_ft5x06_i2c_ts_resume);

[-- Attachment #3: edt-fixes-2.patch --]
[-- Type: text/x-patch, Size: 11238 bytes --]

commit c79729c84cbdde766517a863ade907d8e9a830d6
Author: Simon Budig <simon.budig@kernelconcepts.de>
Date:   Thu Apr 5 14:43:18 2012 +0200

    rework attribute handling. Not sure if this improves things

diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c
index d96eb8f..9e5900c 100644
--- a/drivers/input/touchscreen/edt-ft5x06.c
+++ b/drivers/input/touchscreen/edt-ft5x06.c
@@ -35,7 +35,7 @@
 
 #include <linux/input/edt-ft5x06.h>
 
-#define DRIVER_VERSION "v0.5"
+#define DRIVER_VERSION "v0.6"
 
 #define WORK_REGISTER_THRESHOLD   0x00
 #define WORK_REGISTER_REPORT_RATE 0x08
@@ -47,6 +47,14 @@
 #define WORK_REGISTER_OPMODE      0x3c
 #define FACTORY_REGISTER_OPMODE   0x01
 
+enum edt_ft5x06_setting {
+	SETTING_THRESHOLD,
+	SETTING_GAIN,
+	SETTING_OFFSET,
+	SETTING_REPORT_RATE,
+	N_SETTINGS,    /* must be last entry */
+};
+
 struct edt_ft5x06_i2c_ts_data {
 	struct i2c_client *client;
 	struct input_dev *input;
@@ -58,13 +66,69 @@ struct edt_ft5x06_i2c_ts_data {
 
 	struct mutex mutex;
 	bool factory_mode;
-	int threshold;
-	int gain;
-	int offset;
-	int report_rate;
+	int settings[N_SETTINGS];
 	char name[24];
 };
 
+struct edt_ft5x06_settings_data_entry {
+	u8 addr;
+	u8 min;
+	u8 max;
+};
+
+static const struct edt_ft5x06_settings_data_entry
+edt_ft5x06_settings_data[N_SETTINGS] = {
+	[SETTING_THRESHOLD]   = { WORK_REGISTER_THRESHOLD,   20, 80 },
+	[SETTING_GAIN]        = { WORK_REGISTER_GAIN,         0, 31 },
+	[SETTING_OFFSET]      = { WORK_REGISTER_OFFSET,       0, 31 },
+	[SETTING_REPORT_RATE] = { WORK_REGISTER_REPORT_RATE,  3, 14 },
+};
+
+
+static ssize_t edt_ft5x06_i2c_setting_show(struct device *dev,
+                                           struct device_attribute *attr,
+                                           char *buf);
+static ssize_t edt_ft5x06_i2c_setting_store(struct device *dev,
+                                            struct device_attribute *attr,
+                                            const char *buf, size_t count);
+static ssize_t edt_ft5x06_i2c_mode_show(struct device *dev,
+                                        struct device_attribute *attr,
+                                        char *buf);
+static ssize_t edt_ft5x06_i2c_mode_store(struct device *dev,
+                                         struct device_attribute *attr,
+                                         const char *buf, size_t count);
+static ssize_t edt_ft5x06_i2c_raw_data_show(struct device *dev,
+                                            struct device_attribute *attr,
+                                            char *buf);
+
+static DEVICE_ATTR(gain,      0664,
+                   edt_ft5x06_i2c_setting_show, edt_ft5x06_i2c_setting_store);
+static DEVICE_ATTR(offset,    0664,
+                   edt_ft5x06_i2c_setting_show, edt_ft5x06_i2c_setting_store);
+static DEVICE_ATTR(threshold, 0664,
+                   edt_ft5x06_i2c_setting_show, edt_ft5x06_i2c_setting_store);
+static DEVICE_ATTR(report_rate, 0664,
+                   edt_ft5x06_i2c_setting_show, edt_ft5x06_i2c_setting_store);
+static DEVICE_ATTR(mode,      0664,
+                   edt_ft5x06_i2c_mode_show, edt_ft5x06_i2c_mode_store);
+static DEVICE_ATTR(raw_data,  0444,
+                   edt_ft5x06_i2c_raw_data_show, NULL);
+
+static struct attribute *edt_ft5x06_i2c_attrs[] = {
+	[SETTING_GAIN]        = &dev_attr_gain.attr,
+	[SETTING_OFFSET]      = &dev_attr_offset.attr,
+	[SETTING_THRESHOLD]   = &dev_attr_threshold.attr,
+	[SETTING_REPORT_RATE] = &dev_attr_report_rate.attr,
+	&dev_attr_mode.attr,
+	&dev_attr_raw_data.attr,
+	NULL
+};
+
+static const struct attribute_group edt_ft5x06_i2c_attr_group = {
+	.attrs = edt_ft5x06_i2c_attrs,
+};
+
+
 static int edt_ft5x06_ts_readwrite(struct i2c_client *client,
                                    u16 wr_len, u8 *wr_buf,
                                    u16 rd_len, u8 *rd_buf)
@@ -217,27 +281,14 @@ static ssize_t edt_ft5x06_i2c_setting_show(struct device *dev,
 	struct edt_ft5x06_i2c_ts_data *tsdata = dev_get_drvdata(dev);
 	struct i2c_client *client = tsdata->client;
 	int ret = 0;
-	int *value;
-	u8 addr;
+	int i;
 
-	switch (attr->attr.name[0]) {
-	case 't':    /* threshold */
-		addr = WORK_REGISTER_THRESHOLD;
-		value = &tsdata->threshold;
-		break;
-	case 'g':    /* gain */
-		addr = WORK_REGISTER_GAIN;
-		value = &tsdata->gain;
-		break;
-	case 'o':    /* offset */
-		addr = WORK_REGISTER_OFFSET;
-		value = &tsdata->offset;
-		break;
-	case 'r':    /* report rate */
-		addr = WORK_REGISTER_REPORT_RATE;
-		value = &tsdata->report_rate;
-		break;
-	default:
+	for (i = 0; i < N_SETTINGS; i++) {
+		if (edt_ft5x06_i2c_attrs[i] == &attr->attr)
+			break;
+	}
+	
+	if (i == N_SETTINGS) {
 		dev_err(&client->dev,
 		        "unknown attribute for edt_ft5x06_i2c_setting_show: %s\n",
 		        attr->attr.name);
@@ -253,20 +304,21 @@ static ssize_t edt_ft5x06_i2c_setting_show(struct device *dev,
 		return -EIO;
 	}
 
-	ret = edt_ft5x06_i2c_register_read(tsdata, addr);
+	ret = edt_ft5x06_i2c_register_read(tsdata,
+	                                   edt_ft5x06_settings_data[i].addr);
 	if (ret < 0) {
 		dev_err(&tsdata->client->dev,
-	                "Unable to write to i2c touchscreen!\n");
+	                "Unable to read from i2c touchscreen!\n");
 		mutex_unlock(&tsdata->mutex);
 		return ret;
 	}
 	mutex_unlock(&tsdata->mutex);
 
-	if (ret != *value) {
+	if (ret != tsdata->settings[i]) {
 		dev_info(&tsdata->client->dev,
 		         "i2c read (%d) and stored value (%d) differ. Huh?\n",
-		         ret, *value);
-		*value = ret;
+		         ret, tsdata->settings[i]);
+		tsdata->settings[i] = ret;
 	}
 
 	return sprintf(buf, "%d\n", ret);
@@ -278,10 +330,21 @@ static ssize_t edt_ft5x06_i2c_setting_store(struct device *dev,
 {
 	struct edt_ft5x06_i2c_ts_data *tsdata = dev_get_drvdata(dev);
 	struct i2c_client *client = tsdata->client;
-	int ret = 0;
-	u8 addr;
+	int i, ret = 0;
 	unsigned int val;
 
+	for (i = 0; i < N_SETTINGS; i++) {
+		if (edt_ft5x06_i2c_attrs[i] == &attr->attr)
+			break;
+	}
+	
+	if (i == N_SETTINGS) {
+		dev_err(&client->dev,
+		        "unknown attribute for edt_ft5x06_i2c_setting_show: %s\n",
+		        attr->attr.name);
+		return -EINVAL;
+	}
+
 	mutex_lock(&tsdata->mutex);
 
 	if (tsdata->factory_mode) {
@@ -298,36 +361,14 @@ static ssize_t edt_ft5x06_i2c_setting_store(struct device *dev,
 		goto out;
 	}
 
-	switch (attr->attr.name[0]) {
-	case 't':    /* threshold */
-		addr = WORK_REGISTER_THRESHOLD;
-		val = val < 20 ? 20 : val > 80 ? 80 : val;
-		tsdata->threshold = val;
-		break;
-	case 'g':    /* gain */
-		addr = WORK_REGISTER_GAIN;
-		val = val < 0 ? 0 : val > 31 ? 31 : val;
-		tsdata->gain = val;
-		break;
-	case 'o':    /* offset */
-		addr = WORK_REGISTER_OFFSET;
-		val = val < 0 ? 0 : val > 31 ? 31 : val;
-		tsdata->offset = val;
-		break;
-	case 'r':    /* report rate */
-		addr = WORK_REGISTER_REPORT_RATE;
-		val = val < 3 ? 3 : val > 14 ? 14 : val;
-		tsdata->report_rate = val;
-		break;
-	default:
-		dev_err(&client->dev,
-		        "unknown attribute for edt_ft5x06_i2c_setting_show: %s\n",
-		        attr->attr.name);
-		ret = -EINVAL;
-		goto out;
-	}
-
-	ret = edt_ft5x06_i2c_register_write(tsdata, addr, val);
+	if (val < edt_ft5x06_settings_data[i].min)
+		val = edt_ft5x06_settings_data[i].min;
+	else if (val > edt_ft5x06_settings_data[i].max)
+		val = edt_ft5x06_settings_data[i].max;
+	tsdata->settings[i] = val;
+	ret = edt_ft5x06_i2c_register_write(tsdata,
+	                                    edt_ft5x06_settings_data[i].addr,
+	                                    val);
 
 	if (ret < 0) {
 		dev_err(&tsdata->client->dev,
@@ -411,18 +452,11 @@ static ssize_t edt_ft5x06_i2c_mode_store(struct device *dev,
 				        i*5);
 
 			/* restore parameters */
-			edt_ft5x06_i2c_register_write(tsdata,
-			                              WORK_REGISTER_THRESHOLD,
-			                              tsdata->threshold);
-			edt_ft5x06_i2c_register_write(tsdata,
-			                              WORK_REGISTER_GAIN,
-			                              tsdata->gain);
-			edt_ft5x06_i2c_register_write(tsdata,
-			                              WORK_REGISTER_OFFSET,
-			                              tsdata->offset);
-			edt_ft5x06_i2c_register_write(tsdata,
-			                              WORK_REGISTER_REPORT_RATE,
-			                              tsdata->report_rate);
+			for (i = 0; i < N_SETTINGS; i++) {
+				edt_ft5x06_i2c_register_write(tsdata,
+				                              edt_ft5x06_settings_data[i].addr,
+				                              tsdata->settings[i]);
+			}
 
 			enable_irq(tsdata->irq);
 		}
@@ -482,33 +516,6 @@ static ssize_t edt_ft5x06_i2c_raw_data_show(struct device *dev,
 }
 
 
-static DEVICE_ATTR(gain,      0664,
-                   edt_ft5x06_i2c_setting_show, edt_ft5x06_i2c_setting_store);
-static DEVICE_ATTR(offset,    0664,
-                   edt_ft5x06_i2c_setting_show, edt_ft5x06_i2c_setting_store);
-static DEVICE_ATTR(threshold, 0664,
-                   edt_ft5x06_i2c_setting_show, edt_ft5x06_i2c_setting_store);
-static DEVICE_ATTR(report_rate, 0664,
-                   edt_ft5x06_i2c_setting_show, edt_ft5x06_i2c_setting_store);
-static DEVICE_ATTR(mode,      0664,
-                   edt_ft5x06_i2c_mode_show, edt_ft5x06_i2c_mode_store);
-static DEVICE_ATTR(raw_data,  0444,
-                   edt_ft5x06_i2c_raw_data_show, NULL);
-
-static struct attribute *edt_ft5x06_i2c_attrs[] = {
-	&dev_attr_gain.attr,
-	&dev_attr_offset.attr,
-	&dev_attr_threshold.attr,
-	&dev_attr_report_rate.attr,
-	&dev_attr_mode.attr,
-	&dev_attr_raw_data.attr,
-	NULL
-};
-
-static const struct attribute_group edt_ft5x06_i2c_attr_group = {
-	.attrs = edt_ft5x06_i2c_attrs,
-};
-
 static int edt_ft5x06_i2c_ts_probe(struct i2c_client *client,
                                    const struct i2c_device_id *id)
 {
@@ -516,7 +523,7 @@ static int edt_ft5x06_i2c_ts_probe(struct i2c_client *client,
 	const struct edt_ft5x06_platform_data *pdata = client->dev.platform_data;
 	struct edt_ft5x06_i2c_ts_data *tsdata;
 	struct input_dev *input;
-	int error;
+	int i, error;
 	u8 rdbuf[23];
 	char *model_name, *fw_version;
 
@@ -586,14 +593,10 @@ static int edt_ft5x06_i2c_ts_probe(struct i2c_client *client,
 		goto err_free_irq_pin;
 	}
 
-	tsdata->threshold = edt_ft5x06_i2c_register_read(tsdata,
-	                                                 WORK_REGISTER_THRESHOLD);
-	tsdata->gain      = edt_ft5x06_i2c_register_read(tsdata,
-	                                                 WORK_REGISTER_GAIN);
-	tsdata->offset    = edt_ft5x06_i2c_register_read(tsdata,
-	                                                 WORK_REGISTER_OFFSET);
-	tsdata->report_rate = edt_ft5x06_i2c_register_read(tsdata,
-	                                                 WORK_REGISTER_REPORT_RATE);
+	for (i = 0; i < N_SETTINGS; i++) {
+		tsdata->settings[i] = edt_ft5x06_i2c_register_read(tsdata,
+		                                                   edt_ft5x06_settings_data[i].addr);
+	}
 	tsdata->num_x     = edt_ft5x06_i2c_register_read(tsdata,
 	                                                 WORK_REGISTER_NUM_X);
 	tsdata->num_y     = edt_ft5x06_i2c_register_read(tsdata,

Re: [PATCH] Touchscreen driver for FT5x06 based EDT displays

[Dmitry Torokhov]

Hi Simon,

On Thu, Apr 05, 2012 at 02:54:18PM +0200, Simon Budig wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> On 04/04/2012 09:10 PM, Dmitry Torokhov wrote:
> [suggestions for my patch]
> 
> Based on your input I have prepared two patches which might be easier to
> review than the complete diff.
> 
> The first one is IMHO pretty straightforward and implements most of the
> small bits.
> 
> The second one sits on top of the first one and is an attempt at more
> generic attribute handling. I must say that I am not too happy about it,
> but maybe I am thinking too convoluted here. It saves about 3 lines of
> code and exchanges a lot of easy and straightforward code with stuff
> that is harder to follow - especially the lookup of the matching
> attribute makes me unhappy. There are bits in it I like and will
> probably use. But the lookup...
> 
> any suggestions are welcome.

Sorry for the long delay. I agree that the new version of attribute
handling code is not any better than previous one, and this is not what
I had in mind.

Below is a patch on top of your original one plus the
first patch you send in the last email. It implements the attribute
handling the way I see it and also has some additional changes. Could
you please give it a spin and let me know if the device still works?

Thanks.

-- 
Dmitry


Input: miscellaneous edt fixes

- reworked attribute handling
- remove fake ABS_PRESSURE
- locking - take mutex on sysfs operations; disable interrupts
  when switching mode to factory. Do not take mutex in interrupt;
  rely on I2C code to provide necessary locking.
- do not carry around reset pin gpio.
- use gpio_request_one()
- use IRQ from client structure.

Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
---

 drivers/input/touchscreen/edt-ft5x06.c |  889 +++++++++++++++++---------------
 1 files changed, 461 insertions(+), 428 deletions(-)


diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c
index edd7f9f..acd00ba 100644
--- a/drivers/input/touchscreen/edt-ft5x06.c
+++ b/drivers/input/touchscreen/edt-ft5x06.c
@@ -30,29 +30,30 @@
 #include <linux/uaccess.h>
 #include <linux/delay.h>
 #include <linux/slab.h>
-
 #include <linux/gpio.h>
-
 #include <linux/input/edt-ft5x06.h>
 
 #define DRIVER_VERSION "v0.5"
 
-#define WORK_REGISTER_THRESHOLD   0x00
-#define WORK_REGISTER_REPORT_RATE 0x08
-#define WORK_REGISTER_GAIN        0x30
-#define WORK_REGISTER_OFFSET      0x31
-#define WORK_REGISTER_NUM_X       0x33
-#define WORK_REGISTER_NUM_Y       0x34
+#define WORK_REGISTER_THRESHOLD		0x00
+#define WORK_REGISTER_REPORT_RATE	0x08
+#define WORK_REGISTER_GAIN		0x30
+#define WORK_REGISTER_OFFSET		0x31
+#define WORK_REGISTER_NUM_X		0x33
+#define WORK_REGISTER_NUM_Y		0x34
 
-#define WORK_REGISTER_OPMODE      0x3c
-#define FACTORY_REGISTER_OPMODE   0x01
+#define WORK_REGISTER_OPMODE		0x3c
+#define FACTORY_REGISTER_OPMODE		0x01
+
+#define EDT_NAME_LEN			23
+#define EDT_SWITCH_MODE_RETRIES		10
+#define EDT_SWITCH_MODE_DELAY		5 /* msec */
+#define EDT_RAW_DATA_RETRIES		100
+#define EDT_RAW_DATA_DELAY		1 /* msec */
 
 struct edt_ft5x06_i2c_ts_data {
 	struct i2c_client *client;
 	struct input_dev *input;
-	int irq;
-	int irq_pin;
-	int reset_pin;
 	int num_x;
 	int num_y;
 
@@ -62,7 +63,8 @@ struct edt_ft5x06_i2c_ts_data {
 	int gain;
 	int offset;
 	int report_rate;
-	char name[24];
+
+	char name[EDT_NAME_LEN];
 };
 
 static int edt_ft5x06_ts_readwrite(struct i2c_client *client,
@@ -70,7 +72,8 @@ static int edt_ft5x06_ts_readwrite(struct i2c_client *client,
                                    u16 rd_len, u8 *rd_buf)
 {
 	struct i2c_msg wrmsg[2];
-	int i, ret;
+	int i = 0;
+	int ret;
 
 	i = 0;
 	if (wr_len) {
@@ -89,544 +92,594 @@ static int edt_ft5x06_ts_readwrite(struct i2c_client *client,
 	}
 
 	ret = i2c_transfer(client->adapter, wrmsg, i);
-	if (ret < 0) {
-		dev_err(&client->dev, "i2c_transfer failed: %d\n", ret);
+	if (ret < 0)
 		return ret;
-	}
+	if (ret != i)
+		return -EIO;
 
-	return ret;
+	return 0;
 }
 
-
 static irqreturn_t edt_ft5x06_ts_isr(int irq, void *dev_id)
 {
 	struct edt_ft5x06_i2c_ts_data *tsdata = dev_id;
-	unsigned char touching = 0;
-	unsigned char rdbuf[26], wrbuf[1];
-	int i, have_abs, type, x, y, id, ret;
+	u8 num_touching;
+	u8 cmd = 0xf9;
+	u8 rdbuf[26];
+	bool have_abs = false;
+	int i, type, x, y, id;
+	int error;
 
-	memset(wrbuf, 0, sizeof(wrbuf));
 	memset(rdbuf, 0, sizeof(rdbuf));
 
-	wrbuf[0] = 0xf9;
-
-	mutex_lock(&tsdata->mutex);
-	ret = edt_ft5x06_ts_readwrite(tsdata->client,
-	                              1, wrbuf,
-	                              sizeof(rdbuf), rdbuf);
-	mutex_unlock(&tsdata->mutex);
-	if (ret < 0) {
+	error = edt_ft5x06_ts_readwrite(tsdata->client,
+					sizeof(cmd), &cmd,
+					sizeof(rdbuf), rdbuf);
+	if (error) {
 		dev_err(&tsdata->client->dev,
-		        "Unable to write to i2c touchscreen!\n");
+			"Unable to write to fetch data, error: %d\n", error);
 		goto out;
 	}
 
 	if (rdbuf[0] != 0xaa || rdbuf[1] != 0xaa || rdbuf[2] != 26) {
 		dev_err(&tsdata->client->dev,
-		        "Unexpected header: %02x%02x%02x!\n",
-		        rdbuf[0], rdbuf[1], rdbuf[2]);
+			"Unexpected header: %02x%02x%02x!\n",
+			rdbuf[0], rdbuf[1], rdbuf[2]);
+		goto out;
 	}
 
-	have_abs = false;
-	touching = rdbuf[3];
-	for (i = 0; i < touching; i++) {
-		type = rdbuf[i*4+5] >> 6;
+	num_touching = rdbuf[3];
+	for (i = 0; i < num_touching; i++) {
+		u8 *buf = &rdbuf[i * 4];
+
+		type = buf[5] >> 6;
 		/* ignore Touch Down and Reserved events */
 		if (type == 0x01 || type == 0x03)
 			continue;
 
-		x = ((rdbuf[i*4+5] << 8) | rdbuf[i*4+6]) & 0x0fff;
-		y = ((rdbuf[i*4+7] << 8) | rdbuf[i*4+8]) & 0x0fff;
-		id = (rdbuf[i*4+7] >> 4) & 0x0f;
+		x = ((buf[5] << 8) | rdbuf[6]) & 0x0fff;
+		y = ((buf[7] << 8) | rdbuf[8]) & 0x0fff;
+		id = (buf[7] >> 4) & 0x0f;
 
 		if (!have_abs) {
-			input_report_key(tsdata->input, BTN_TOUCH,    1);
-			input_report_abs(tsdata->input, ABS_PRESSURE, 1);
-			input_report_abs(tsdata->input, ABS_X,        x);
-			input_report_abs(tsdata->input, ABS_Y,        y);
+			input_report_key(tsdata->input, BTN_TOUCH, 1);
+			input_report_abs(tsdata->input, ABS_X, x);
+			input_report_abs(tsdata->input, ABS_Y, y);
 			have_abs = true;
 		}
+
 		input_report_abs(tsdata->input, ABS_MT_POSITION_X, x);
 		input_report_abs(tsdata->input, ABS_MT_POSITION_Y, y);
 		input_report_abs(tsdata->input, ABS_MT_TRACKING_ID, id);
 		input_mt_sync(tsdata->input);
 	}
-	if (!have_abs) {
-		input_report_key(tsdata->input, BTN_TOUCH,    0);
-		input_report_abs(tsdata->input, ABS_PRESSURE, 0);
-	}
+
+	if (!have_abs)
+		input_report_key(tsdata->input, BTN_TOUCH, 0);
+
 	input_sync(tsdata->input);
 
 out:
 	return IRQ_HANDLED;
 }
 
-
 static int edt_ft5x06_i2c_register_write(struct edt_ft5x06_i2c_ts_data *tsdata,
-                                         u8 addr, u8 value)
+					 u8 addr, u8 value)
 {
 	u8 wrbuf[4];
-	int ret;
-
-	wrbuf[0]  = tsdata->factory_mode ? 0xf3 : 0xfc;
-	wrbuf[1]  = tsdata->factory_mode ? addr & 0x7f : addr & 0x3f;
-	wrbuf[2]  = value;
-	wrbuf[3]  = wrbuf[0] ^ wrbuf[1] ^ wrbuf[2];
-
-	disable_irq(tsdata->irq);
-
-	ret = edt_ft5x06_ts_readwrite(tsdata->client,
-	                              4, wrbuf,
-	                              0, NULL);
 
-	enable_irq(tsdata->irq);
+	wrbuf[0] = tsdata->factory_mode ? 0xf3 : 0xfc;
+	wrbuf[1] = tsdata->factory_mode ? addr & 0x7f : addr & 0x3f;
+	wrbuf[2] = value;
+	wrbuf[3] = wrbuf[0] ^ wrbuf[1] ^ wrbuf[2];
 
-	return ret;
+	return edt_ft5x06_ts_readwrite(tsdata->client, 4, wrbuf, 0, NULL);
 }
 
 static int edt_ft5x06_i2c_register_read(struct edt_ft5x06_i2c_ts_data *tsdata,
                                         u8 addr)
 {
 	u8 wrbuf[2], rdbuf[2];
-	int ret;
+	int error;
 
-	wrbuf[0]  = tsdata->factory_mode ? 0xf3 : 0xfc;
-	wrbuf[1]  = tsdata->factory_mode ? addr & 0x7f : addr & 0x3f;
+	wrbuf[0] = tsdata->factory_mode ? 0xf3 : 0xfc;
+	wrbuf[1] = tsdata->factory_mode ? addr & 0x7f : addr & 0x3f;
 	wrbuf[1] |= tsdata->factory_mode ? 0x80 : 0x40;
 
-	disable_irq(tsdata->irq);
-
-	ret = edt_ft5x06_ts_readwrite(tsdata->client,
-	                              2, wrbuf,
-	                              2, rdbuf);
-
-	enable_irq(tsdata->irq);
+	error = edt_ft5x06_ts_readwrite(tsdata->client, 2, wrbuf, 2, rdbuf);
+	if (error)
+		return error;
 
-	if ((wrbuf[0] ^ wrbuf[1] ^ rdbuf[0]) != rdbuf[1])
+	if ((wrbuf[0] ^ wrbuf[1] ^ rdbuf[0]) != rdbuf[1]) {
 		dev_err(&tsdata->client->dev,
-		        "crc error: 0x%02x expected, got 0x%02x\n",
-		        (wrbuf[0] ^ wrbuf[1] ^ rdbuf[0]), rdbuf[1]);
+			"crc error: 0x%02x expected, got 0x%02x\n",
+			wrbuf[0] ^ wrbuf[1] ^ rdbuf[0], rdbuf[1]);
+		return -EIO;
+	}
 
-	return ret < 0 ? ret : rdbuf[0];
+	return rdbuf[0];
 }
 
-static ssize_t edt_ft5x06_i2c_setting_show(struct device *dev,
-                                           struct device_attribute *attr,
-                                           char *buf)
-{
-	struct edt_ft5x06_i2c_ts_data *tsdata = dev_get_drvdata(dev);
-	struct i2c_client *client = tsdata->client;
-	int ret = 0;
-	int *value;
+struct edt_ft5x06_attribute {
+	struct device_attribute dattr;
+	size_t field_offset;
+	u8 limit_low;
+	u8 limit_high;
 	u8 addr;
+};
 
-	switch (attr->attr.name[0]) {
-	case 't':    /* threshold */
-		addr = WORK_REGISTER_THRESHOLD;
-		value = &tsdata->threshold;
-		break;
-	case 'g':    /* gain */
-		addr = WORK_REGISTER_GAIN;
-		value = &tsdata->gain;
-		break;
-	case 'o':    /* offset */
-		addr = WORK_REGISTER_OFFSET;
-		value = &tsdata->offset;
-		break;
-	case 'r':    /* report rate */
-		addr = WORK_REGISTER_REPORT_RATE;
-		value = &tsdata->report_rate;
-		break;
-	default:
-		dev_err(&client->dev,
-		        "unknown attribute for edt_ft5x06_i2c_setting_show: %s\n",
-		        attr->attr.name);
-		return -EINVAL;
+#define EDT_ATTR(_field, _mode, _addr, _limit_low, _limit_high)		\
+	struct edt_ft5x06_attribute edt_ft5x06_attr_##_field = {	\
+		.dattr = __ATTR(_field, _mode,				\
+				edt_ft5x06_i2c_setting_show,		\
+				edt_ft5x06_i2c_setting_store),		\
+		.field_offset =						\
+			offsetof(struct edt_ft5x06_i2c_ts_data, _field),\
+		.limit_low = _limit_low,				\
+		.limit_high = _limit_high,				\
+		.addr = _addr,						\
 	}
 
+static ssize_t edt_ft5x06_i2c_setting_show(struct device *dev,
+                                           struct device_attribute *dattr,
+                                           char *buf)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	struct edt_ft5x06_i2c_ts_data *tsdata = i2c_get_clientdata(client);
+	struct edt_ft5x06_attribute *attr =
+			container_of(dattr, struct edt_ft5x06_attribute, dattr);
+	u8 *field = (u8 *)((char *)tsdata + attr->field_offset);
+	int val;
+	size_t count;
+	int error = 0;
+
 	mutex_lock(&tsdata->mutex);
 
 	if (tsdata->factory_mode) {
-		dev_err(dev,
-		        "setting register not available in factory mode\n");
-		mutex_unlock(&tsdata->mutex);
-		return -EIO;
+		error = -EIO;
+		goto out;
 	}
 
-	ret = edt_ft5x06_i2c_register_read(tsdata, addr);
-	if (ret < 0) {
+	val = edt_ft5x06_i2c_register_read(tsdata, attr->addr);
+	if (val < 0) {
+		error = val;
 		dev_err(&tsdata->client->dev,
-		        "Unable to write to i2c touchscreen!\n");
-		mutex_unlock(&tsdata->mutex);
-		return ret;
+			"Failed to fetch attribute %s, error %d\n",
+			dattr->attr.name, error);
+		goto out;
 	}
-	mutex_unlock(&tsdata->mutex);
 
-	if (ret != *value) {
-		dev_info(&tsdata->client->dev,
-		         "i2c read (%d) and stored value (%d) differ. Huh?\n",
-		         ret, *value);
-		*value = ret;
+	if (val != *field) {
+		dev_warn(&tsdata->client->dev,
+			 "%s: read (%d) and stored value (%d) differ\n",
+			 dattr->attr.name, val, *field);
+		*field = val;
 	}
 
-	return sprintf(buf, "%d\n", ret);
+	count = scnprintf(buf, PAGE_SIZE, "%d\n", val);
+out:
+	mutex_unlock(&tsdata->mutex);
+	return error ?: count;
 }
 
 static ssize_t edt_ft5x06_i2c_setting_store(struct device *dev,
-                                            struct device_attribute *attr,
-                                            const char *buf, size_t count)
+					    struct device_attribute *dattr,
+					    const char *buf, size_t count)
 {
-	struct edt_ft5x06_i2c_ts_data *tsdata = dev_get_drvdata(dev);
-	struct i2c_client *client = tsdata->client;
-	int ret = 0;
-	u8 addr;
+	struct i2c_client *client = to_i2c_client(dev);
+	struct edt_ft5x06_i2c_ts_data *tsdata = i2c_get_clientdata(client);
+	struct edt_ft5x06_attribute *attr =
+			container_of(dattr, struct edt_ft5x06_attribute, dattr);
+	u8 *field = (u8 *)((char *)tsdata + attr->field_offset);
 	unsigned int val;
+	int error;
 
 	mutex_lock(&tsdata->mutex);
 
 	if (tsdata->factory_mode) {
-		dev_err(dev,
-		        "setting register not available in factory mode\n");
-		ret = -EIO;
+		error = -EIO;
 		goto out;
 	}
 
-	if (sscanf(buf, "%u", &val) != 1) {
-		dev_err(dev, "Invalid value for attribute %s\n",
-		        attr->attr.name);
-		ret = -EINVAL;
+	error = kstrtouint(buf, 0, &val);
+	if (error)
 		goto out;
-	}
 
-	switch (attr->attr.name[0]) {
-	case 't':    /* threshold */
-		addr = WORK_REGISTER_THRESHOLD;
-		val = val < 20 ? 20 : val > 80 ? 80 : val;
-		tsdata->threshold = val;
-		break;
-	case 'g':    /* gain */
-		addr = WORK_REGISTER_GAIN;
-		val = val < 0 ? 0 : val > 31 ? 31 : val;
-		tsdata->gain = val;
-		break;
-	case 'o':    /* offset */
-		addr = WORK_REGISTER_OFFSET;
-		val = val < 0 ? 0 : val > 31 ? 31 : val;
-		tsdata->offset = val;
-		break;
-	case 'r':    /* report rate */
-		addr = WORK_REGISTER_REPORT_RATE;
-		val = val < 3 ? 3 : val > 14 ? 14 : val;
-		tsdata->report_rate = val;
-		break;
-	default:
-		dev_err(&client->dev,
-		        "unknown attribute for edt_ft5x06_i2c_setting_show: %s\n",
-		        attr->attr.name);
-		ret = -EINVAL;
+	if (val < attr->limit_low || val > attr->limit_high) {
+		error = -ERANGE;
 		goto out;
 	}
 
-	ret = edt_ft5x06_i2c_register_write(tsdata, addr, val);
-
-	if (ret < 0) {
+	error = edt_ft5x06_i2c_register_write(tsdata, attr->addr, val);
+	if (error) {
 		dev_err(&tsdata->client->dev,
-		        "Unable to write to i2c touchscreen!\n");
+			"Failed to update attribute %s, error: %d\n",
+			dattr->attr.name, error);
 		goto out;
 	}
 
+	*field = val;
+
 out:
 	mutex_unlock(&tsdata->mutex);
-	return ret < 0 ? ret : count;
+	return error ?: count;
 }
 
-
 static ssize_t edt_ft5x06_i2c_mode_show(struct device *dev,
-                                        struct device_attribute *attr,
-                                        char *buf)
+					struct device_attribute *attr,
+					char *buf)
 {
-	struct edt_ft5x06_i2c_ts_data *tsdata = dev_get_drvdata(dev);
-	return sprintf(buf, "%d\n", tsdata->factory_mode ? 1 : 0);
+	struct i2c_client *client = to_i2c_client(dev);
+	struct edt_ft5x06_i2c_ts_data *tsdata = i2c_get_clientdata(client);
+
+	return sprintf(buf, "%d\n", tsdata->factory_mode);
+}
+
+static int edt_ft5x06_i2c_factory_mode(struct edt_ft5x06_i2c_ts_data *tsdata)
+{
+	int retries = EDT_SWITCH_MODE_RETRIES;
+	int ret;
+	int error;
+
+	disable_irq(tsdata->client->irq);
+
+	/* mode register is 0x3c when in the work mode */
+	error = edt_ft5x06_i2c_register_write(tsdata,
+					    WORK_REGISTER_OPMODE, 0x03);
+	if (error) {
+		dev_err(&tsdata->client->dev,
+			"failed to switch to factory mode, error %d\n",
+			error);
+	}
+
+	do {
+		mdelay(EDT_SWITCH_MODE_DELAY);
+		/* mode register is 0x01 when in factory mode */
+		ret = edt_ft5x06_i2c_register_read(tsdata,
+						   FACTORY_REGISTER_OPMODE);
+		if (ret == 0x03)
+			break;
+	} while (--retries > 0);
+
+	if (retries == 0) {
+		dev_err(&tsdata->client->dev,
+			"not in factory mode after %dms.\n",
+			EDT_SWITCH_MODE_RETRIES * EDT_SWITCH_MODE_DELAY);
+		error = -EIO;
+		goto err_out;
+	}
+
+	tsdata->factory_mode = true;
+	return 0;
+
+err_out:
+	enable_irq(tsdata->client->irq);
+	return error;
+}
+
+static int edt_ft5x06_i2c_work_mode(struct edt_ft5x06_i2c_ts_data *tsdata)
+{
+	int retries = EDT_SWITCH_MODE_RETRIES;
+	int ret;
+	int error;
+
+	/* mode register is 0x01 when in the factory mode */
+	error = edt_ft5x06_i2c_register_write(tsdata,
+					      FACTORY_REGISTER_OPMODE, 0x01);
+	if (error) {
+		dev_err(&tsdata->client->dev,
+			"failed to switch to work mode, error: %d\n",
+			error);
+		return error;
+	}
+
+	do {
+		mdelay(EDT_SWITCH_MODE_DELAY);
+		/* mode register is 0x01 when in factory mode */
+		ret = edt_ft5x06_i2c_register_read(tsdata,
+						   WORK_REGISTER_OPMODE);
+		if (ret == 0x01)
+			break;
+	} while (--retries > 0);
+
+	if (retries == 0) {
+		dev_err(&tsdata->client->dev,
+			"not in work mode after %dms.\n",
+			EDT_SWITCH_MODE_RETRIES * EDT_SWITCH_MODE_DELAY);
+		return -EIO;
+	}
+
+	/* restore parameters */
+	edt_ft5x06_i2c_register_write(tsdata,
+				      WORK_REGISTER_THRESHOLD,
+				      tsdata->threshold);
+	edt_ft5x06_i2c_register_write(tsdata,
+				      WORK_REGISTER_GAIN,
+				      tsdata->gain);
+	edt_ft5x06_i2c_register_write(tsdata,
+				      WORK_REGISTER_OFFSET,
+				      tsdata->offset);
+	edt_ft5x06_i2c_register_write(tsdata,
+				      WORK_REGISTER_REPORT_RATE,
+				      tsdata->report_rate);
+
+	tsdata->factory_mode = false;
+	enable_irq(tsdata->client->irq);
+
+	return 0;
 }
 
 static ssize_t edt_ft5x06_i2c_mode_store(struct device *dev,
                                          struct device_attribute *attr,
                                          const char *buf, size_t count)
 {
-	struct edt_ft5x06_i2c_ts_data *tsdata = dev_get_drvdata(dev);
-	int i, ret = 0;
+	struct i2c_client *client = to_i2c_client(dev);
+	struct edt_ft5x06_i2c_ts_data *tsdata = i2c_get_clientdata(client);
 	unsigned int mode;
+	int error;
 
-	if (sscanf(buf, "%u", &mode) != 1 || (mode | 1) != 1) {
-		dev_err(dev, "Invalid value for operation mode\n");
-		return -EINVAL;
-	}
+	error = kstrtouint(buf, 0, &mode);
+	if (error)
+		return error;
 
-	/* no change, return without doing anything */
-	if (mode == tsdata->factory_mode)
-		return count;
+	if (mode > 1)
+		return -ERANGE;
 
 	mutex_lock(&tsdata->mutex);
-	if (!tsdata->factory_mode) { /* switch to factory mode */
-		disable_irq(tsdata->irq);
-		/* mode register is 0x3c when in the work mode */
-		ret = edt_ft5x06_i2c_register_write(tsdata,
-		                                    WORK_REGISTER_OPMODE, 0x03);
-		if (ret < 0) {
-			dev_err(dev, "failed to switch to factory mode (%d)\n",
-			        ret);
-		} else {
-			tsdata->factory_mode = true;
-			for (i = 0; i < 10; i++) {
-				mdelay(5);
-				/* mode register is 0x01 when in factory mode */
-				ret = edt_ft5x06_i2c_register_read(tsdata, FACTORY_REGISTER_OPMODE);
-				if (ret == 0x03)
-					break;
-			}
-			if (i == 10)
-				dev_err(dev,
-				        "not in factory mode after %dms.\n",
-				        i*5);
-		}
-	} else {  /* switch to work mode */
-		/* mode register is 0x01 when in the factory mode */
-		ret = edt_ft5x06_i2c_register_write(tsdata,
-		                                    FACTORY_REGISTER_OPMODE,
-		                                    0x01);
-		if (ret < 0) {
-			dev_err(dev, "failed to switch to work mode (%d)\n",
-			        ret);
-		} else {
-			tsdata->factory_mode = false;
-			for (i = 0; i < 10; i++) {
-				mdelay(5);
-				/* mode register is 0x01 when in factory mode */
-				ret = edt_ft5x06_i2c_register_read(tsdata, WORK_REGISTER_OPMODE);
-				if (ret == 0x01)
-					break;
-			}
-			if (i == 10)
-				dev_err(dev, "not in work mode after %dms.\n",
-				        i*5);
-
-			/* restore parameters */
-			edt_ft5x06_i2c_register_write(tsdata,
-			                              WORK_REGISTER_THRESHOLD,
-			                              tsdata->threshold);
-			edt_ft5x06_i2c_register_write(tsdata,
-			                              WORK_REGISTER_GAIN,
-			                              tsdata->gain);
-			edt_ft5x06_i2c_register_write(tsdata,
-			                              WORK_REGISTER_OFFSET,
-			                              tsdata->offset);
-			edt_ft5x06_i2c_register_write(tsdata,
-			                              WORK_REGISTER_REPORT_RATE,
-			                              tsdata->report_rate);
-
-			enable_irq(tsdata->irq);
-		}
+
+	if (mode != tsdata->factory_mode) {
+		error = mode ? edt_ft5x06_i2c_factory_mode(tsdata) :
+			       edt_ft5x06_i2c_work_mode(tsdata);
 	}
 
 	mutex_unlock(&tsdata->mutex);
-	return count;
+	return error ?: count;
 }
 
-
 static ssize_t edt_ft5x06_i2c_raw_data_show(struct device *dev,
                                             struct device_attribute *attr,
                                             char *buf)
 {
-	struct edt_ft5x06_i2c_ts_data *tsdata = dev_get_drvdata(dev);
+	struct i2c_client *client = to_i2c_client(dev);
+	struct edt_ft5x06_i2c_ts_data *tsdata = i2c_get_clientdata(client);
+	int retries  = EDT_RAW_DATA_RETRIES;
+	size_t count = 0;
 	int i, ret;
-	char *ptr, wrbuf[3];
+	int error;
+	char wrbuf[3];
+
+	/* Make sure we have enough space */
+	if (tsdata->num_x * tsdata->num_y * 2 >= PAGE_SIZE)
+		return -ENOBUFS;
+
+	mutex_lock(&tsdata->mutex);
 
 	if (!tsdata->factory_mode) {
-		dev_err(dev, "raw data not available in work mode\n");
-		return -EIO;
+		error = -EIO;
+		goto out;
 	}
 
-	mutex_lock(&tsdata->mutex);
-	ret = edt_ft5x06_i2c_register_write(tsdata, 0x08, 0x01);
-	for (i = 0; i < 100; i++) {
+	error = edt_ft5x06_i2c_register_write(tsdata, 0x08, 0x01);
+	if (error) {
+		dev_dbg(dev, "failed to write 0x08 register, error %d\n",
+			error);
+		goto out;
+	}
+
+	do {
+		msleep(EDT_RAW_DATA_DELAY);
 		ret = edt_ft5x06_i2c_register_read(tsdata, 0x08);
 		if (ret < 1)
 			break;
-		udelay(1000);
+	} while (--retries > 0);
+
+	if (ret < 0) {
+		error = ret;
+		dev_dbg(dev, "failed to read 0x08 register, error %d\n", error);
+		goto out;
 	}
 
-	if (i == 100 || ret < 0) {
-		dev_err(dev, "waiting time exceeded or error: %d\n", ret);
-		mutex_unlock(&tsdata->mutex);
-		return ret < 0 ? ret : -ETIMEDOUT;
+	if (retries == 0) {
+		dev_dbg(dev, "timed out waiting for register to settle\n");
+		error = -ETIMEDOUT;
+		goto out;
 	}
 
-	ptr = buf;
 	wrbuf[0] = 0xf5;
 	wrbuf[1] = 0x0e;
-	for (i = 0; i <= tsdata->num_x; i++) {
+	for (i = 0; i < tsdata->num_x; i++) {
 		wrbuf[2] = i;
-		ret = edt_ft5x06_ts_readwrite(tsdata->client,
-		                               3, wrbuf,
-		                               tsdata->num_y * 2, ptr);
-		if (ret < 0) {
-			mutex_unlock(&tsdata->mutex);
-			return ret;
-		}
-
-		ptr += tsdata->num_y * 2;
+		error = edt_ft5x06_ts_readwrite(tsdata->client,
+						sizeof(wrbuf), wrbuf,
+						tsdata->num_y * 2,
+						&buf[count]);
+		if (error)
+			goto out;
+
+		count += tsdata->num_y * 2;
 	}
 
+out:
 	mutex_unlock(&tsdata->mutex);
-	return ptr - buf;
+	return error ?: count;
 }
 
+static EDT_ATTR(gain, S_IWUSR | S_IRUGO, WORK_REGISTER_GAIN, 0, 31);
+static EDT_ATTR(offset, S_IWUSR | S_IRUGO, WORK_REGISTER_OFFSET, 0, 31);
+static EDT_ATTR(threshold, S_IWUSR | S_IRUGO,
+		WORK_REGISTER_THRESHOLD, 20, 80);
+static EDT_ATTR(report_rate, S_IWUSR | S_IRUGO,
+		WORK_REGISTER_REPORT_RATE, 3, 14);
 
-static DEVICE_ATTR(gain,      0664,
-                   edt_ft5x06_i2c_setting_show, edt_ft5x06_i2c_setting_store);
-static DEVICE_ATTR(offset,    0664,
-                   edt_ft5x06_i2c_setting_show, edt_ft5x06_i2c_setting_store);
-static DEVICE_ATTR(threshold, 0664,
-                   edt_ft5x06_i2c_setting_show, edt_ft5x06_i2c_setting_store);
-static DEVICE_ATTR(report_rate, 0664,
-                   edt_ft5x06_i2c_setting_show, edt_ft5x06_i2c_setting_store);
-static DEVICE_ATTR(mode,      0664,
+static DEVICE_ATTR(mode, S_IWUSR | S_IRUGO,
                    edt_ft5x06_i2c_mode_show, edt_ft5x06_i2c_mode_store);
-static DEVICE_ATTR(raw_data,  0444,
-                   edt_ft5x06_i2c_raw_data_show, NULL);
+static DEVICE_ATTR(raw_data, S_IRUGO, edt_ft5x06_i2c_raw_data_show, NULL);
 
 static struct attribute *edt_ft5x06_i2c_attrs[] = {
-	&dev_attr_gain.attr,
-	&dev_attr_offset.attr,
-	&dev_attr_threshold.attr,
-	&dev_attr_report_rate.attr,
+	&edt_ft5x06_attr_gain.dattr.attr,
+	&edt_ft5x06_attr_offset.dattr.attr,
+	&edt_ft5x06_attr_threshold.dattr.attr,
+	&edt_ft5x06_attr_report_rate.dattr.attr,
 	&dev_attr_mode.attr,
 	&dev_attr_raw_data.attr,
 	NULL
 };
 
+static umode_t edt_ft5x06_i2c_attr_is_visible(struct kobject *kobj,
+					      struct attribute *attr, int n)
+{
+	struct device *dev = container_of(kobj, struct device, kobj);
+	struct i2c_client *client = to_i2c_client(dev);
+	struct edt_ft5x06_i2c_ts_data *tsdata = i2c_get_clientdata(client);
+	umode_t mode = attr->mode;
+
+	if (attr == &dev_attr_raw_data.attr) {
+		if (!tsdata->factory_mode)
+			mode = 0;
+
+	} else if (attr == &edt_ft5x06_attr_gain.dattr.attr ||
+		   attr == &edt_ft5x06_attr_offset.dattr.attr ||
+		   attr == &edt_ft5x06_attr_threshold.dattr.attr ||
+		   attr == &edt_ft5x06_attr_report_rate.dattr.attr) {
+
+		if (tsdata->factory_mode)
+			mode = 0;
+	}
+
+	return mode;
+}
+
 static const struct attribute_group edt_ft5x06_i2c_attr_group = {
 	.attrs = edt_ft5x06_i2c_attrs,
+	.is_visible = edt_ft5x06_i2c_attr_is_visible,
 };
 
-static int edt_ft5x06_i2c_ts_probe(struct i2c_client *client,
-                                   const struct i2c_device_id *id)
+static int __devinit edt_ft5x06_i2c_ts_reset(struct i2c_client *client,
+					     int reset_pin)
+{
+	int error;
+
+	if (gpio_is_valid(reset_pin)) {
+		/* this pulls reset down, enabling the low active reset */
+		error = gpio_request_one(reset_pin, GPIOF_OUT_INIT_LOW,
+					 "edt-ft5x06 reset");
+		if (error) {
+			dev_err(&client->dev,
+				"Failed to request GPIO %d as reset pin, error %d\n",
+				reset_pin, error);
+			return error;
+		}
+
+		mdelay(50);
+		gpio_set_value(reset_pin, 1);
+		mdelay(100);
+
+		gpio_free(reset_pin);
+	}
+
+	return 0;
+}
+
+static int __devinit edt_ft5x06_i2c_ts_identify(struct i2c_client *client,
+						char *model_name,
+						char *fw_version)
+{
+	u8 rdbuf[EDT_NAME_LEN];
+	char *p;
+	int error;
+
+	error = edt_ft5x06_ts_readwrite(client,
+					1, "\xbb", EDT_NAME_LEN - 1, rdbuf);
+	if (error)
+		return error;
+
+	/* remove last '$' end marker */
+	rdbuf[EDT_NAME_LEN - 1] = '\0';
+	if (rdbuf[EDT_NAME_LEN - 2] == '$')
+		rdbuf[EDT_NAME_LEN - 2] = '\0';
+
+	/* look for Model/Version separator */
+	p = strchr(rdbuf, '*');
+	if (p)
+		*p++ = '\0';
+
+	strlcpy(model_name, rdbuf + 1, EDT_NAME_LEN);
+	strlcpy(fw_version, p ? p : "", EDT_NAME_LEN);
+
+	return 0;
+}
+
+static int __devinit edt_ft5x06_i2c_ts_probe(struct i2c_client *client,
+					     const struct i2c_device_id *id)
 {
 
 	const struct edt_ft5x06_platform_data *pdata = client->dev.platform_data;
 	struct edt_ft5x06_i2c_ts_data *tsdata;
 	struct input_dev *input;
 	int error;
-	u8 rdbuf[23];
-	char *model_name, *fw_version;
+	char fw_version[EDT_NAME_LEN];
 
 	dev_dbg(&client->dev, "probing for EDT FT5x06 I2C\n");
 
 	if (!pdata) {
 		dev_err(&client->dev, "no platform data?\n");
-		return -ENODEV;
-	}
-
-	tsdata = kzalloc(sizeof(*tsdata), GFP_KERNEL);
-	if (!tsdata) {
-		dev_err(&client->dev, "failed to allocate driver data!\n");
-		dev_set_drvdata(&client->dev, NULL);
-		return -ENOMEM;
+		return -EINVAL;
 	}
 
-	dev_set_drvdata(&client->dev, tsdata);
-	tsdata->client = client;
-
-	tsdata->reset_pin = pdata->reset_pin;
-	mutex_init(&tsdata->mutex);
+	error = edt_ft5x06_i2c_ts_reset(client, pdata->reset_pin);
+	if (error)
+		return error;
 
-	if (tsdata->reset_pin >= 0) {
-		error = gpio_request(tsdata->reset_pin, "edt-ft5x06 reset");
-		if (error < 0) {
+	if (gpio_is_valid(pdata->irq_pin)) {
+		error = gpio_request_one(pdata->irq_pin,
+					 GPIOF_IN, "edt-ft5x06 irq");
+		if (error) {
 			dev_err(&client->dev,
-			        "Failed to request GPIO %d as reset pin, error %d\n",
-			         tsdata->reset_pin, error);
-			goto err_free_tsdata;
+				"Failed to request GPIO %d, error %d\n",
+				pdata->irq_pin, error);
+			return error;
 		}
-
-		/* this pulls reset down, enabling the low active reset */
-		if (gpio_direction_output(tsdata->reset_pin, 0) < 0) {
-			dev_info(&client->dev, "switching to output failed\n");
-			goto err_free_reset_pin;
-		}
-	}
-
-	/* request IRQ pin */
-	tsdata->irq_pin = pdata->irq_pin;
-	tsdata->irq = gpio_to_irq(tsdata->irq_pin);
-
-	error = gpio_request(tsdata->irq_pin, "edt-ft5x06 irq");
-	if (error < 0) {
-		dev_err(&client->dev,
-		        "Failed to request GPIO %d for IRQ %d, error %d\n",
-		        tsdata->irq_pin, tsdata->irq, error);
-		goto err_free_reset_pin;
 	}
-	gpio_direction_input(tsdata->irq_pin);
 
-	if (tsdata->reset_pin >= 0) {
-		/* release reset */
-		mdelay(50);
-		gpio_set_value(tsdata->reset_pin, 1);
-		mdelay(100);
+	tsdata = kzalloc(sizeof(*tsdata), GFP_KERNEL);
+	input = input_allocate_device();
+	if (!tsdata || !input) {
+		dev_err(&client->dev, "failed to allocate driver data.\n");
+		error = -ENOMEM;
+		goto err_free_mem;
 	}
 
-	mutex_lock(&tsdata->mutex);
-
+	mutex_init(&tsdata->mutex);
+	tsdata->client = client;
+	tsdata->input = input;
 	tsdata->factory_mode = false;
 
-	if (edt_ft5x06_ts_readwrite(client, 1, "\xbb", 22, rdbuf) < 0) {
-		dev_err(&client->dev, "probing failed\n");
-		error = -ENODEV;
-		goto err_free_irq_pin;
+	error = edt_ft5x06_i2c_ts_identify(client, tsdata->name, fw_version);
+	if (error) {
+		dev_err(&client->dev, "touchscreen probe failed\n");
+		goto err_free_mem;
 	}
 
 	tsdata->threshold = edt_ft5x06_i2c_register_read(tsdata,
-	                                                 WORK_REGISTER_THRESHOLD);
-	tsdata->gain      = edt_ft5x06_i2c_register_read(tsdata,
-	                                                 WORK_REGISTER_GAIN);
-	tsdata->offset    = edt_ft5x06_i2c_register_read(tsdata,
-	                                                 WORK_REGISTER_OFFSET);
+						WORK_REGISTER_THRESHOLD);
+	tsdata->gain = edt_ft5x06_i2c_register_read(tsdata,
+						WORK_REGISTER_GAIN);
+	tsdata->offset = edt_ft5x06_i2c_register_read(tsdata,
+						WORK_REGISTER_OFFSET);
 	tsdata->report_rate = edt_ft5x06_i2c_register_read(tsdata,
-	                                                 WORK_REGISTER_REPORT_RATE);
-	tsdata->num_x     = edt_ft5x06_i2c_register_read(tsdata,
-	                                                 WORK_REGISTER_NUM_X);
-	tsdata->num_y     = edt_ft5x06_i2c_register_read(tsdata,
-	                                                 WORK_REGISTER_NUM_Y);
+						WORK_REGISTER_REPORT_RATE);
+	tsdata->num_x = edt_ft5x06_i2c_register_read(tsdata,
+						WORK_REGISTER_NUM_X);
+	tsdata->num_y = edt_ft5x06_i2c_register_read(tsdata,
+						WORK_REGISTER_NUM_Y);
 
-	mutex_unlock(&tsdata->mutex);
+	dev_dbg(&client->dev,
+		"Model \"%s\", Rev. \"%s\", %dx%d sensors\n",
+		tsdata->name, fw_version, tsdata->num_x, tsdata->num_y);
 
-	/* remove last '$' end marker */
-	rdbuf[22] = '\0';
-	if (rdbuf[21] == '$')
-		rdbuf[21] = '\0';
-
-	model_name = rdbuf + 1;
-	/* look for Model/Version separator */
-	fw_version = strchr(rdbuf, '*');
-
-	if (fw_version) {
-		fw_version[0] = '\0';
-		fw_version++;
-		dev_info(&client->dev,
-		         "Model \"%s\", Rev. \"%s\", %dx%d sensors\n",
-		         model_name, fw_version, tsdata->num_x, tsdata->num_y);
-	} else {
-		dev_info(&client->dev, "Product ID \"%s\"\n", model_name);
-	}
-	strncpy (tsdata->name, model_name, sizeof (tsdata->name) - 1);
-
-	input = input_allocate_device();
-	if (!input) {
-		dev_err(&client->dev, "failed to allocate input device!\n");
-		error = -ENOMEM;
-		goto err_free_irq_pin;
-	}
+	input->name = tsdata->name;
+	input->id.bustype = BUS_I2C;
+	input->dev.parent = &client->dev;
 
 	__set_bit(EV_SYN, input->evbit);
 	__set_bit(EV_KEY, input->evbit);
@@ -634,31 +687,20 @@ static int edt_ft5x06_i2c_ts_probe(struct i2c_client *client,
 	__set_bit(BTN_TOUCH, input->keybit);
 	input_set_abs_params(input, ABS_X, 0, tsdata->num_x * 64 - 1, 0, 0);
 	input_set_abs_params(input, ABS_Y, 0, tsdata->num_y * 64 - 1, 0, 0);
-	input_set_abs_params(input, ABS_PRESSURE, 0, 1, 0, 0);
 	input_set_abs_params(input, ABS_MT_POSITION_X,
 	                     0, tsdata->num_x * 64 - 1, 0, 0);
 	input_set_abs_params(input, ABS_MT_POSITION_Y,
 	                     0, tsdata->num_y * 64 - 1, 0, 0);
 	input_set_abs_params(input, ABS_MT_TRACKING_ID, 0, 15, 0, 0);
 
-	input->name = tsdata->name;
-	input->id.bustype = BUS_I2C;
-	input->dev.parent = &client->dev;
-
 	input_set_drvdata(input, tsdata);
 
-	tsdata->input = input;
-
-	error = input_register_device(input);
-	if (error)
-		goto err_free_input_device;
-
-	if (request_threaded_irq(tsdata->irq, NULL, edt_ft5x06_ts_isr,
-	                         IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
-	                         client->name, tsdata)) {
+	error = request_threaded_irq(client->irq, NULL, edt_ft5x06_ts_isr,
+				     IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
+				     client->name, tsdata);
+	if (error) {
 		dev_err(&client->dev, "Unable to request touchscreen IRQ.\n");
-		error = -ENOMEM;
-		goto err_unregister_device;
+		goto err_free_mem;
 	}
 
 	error = sysfs_create_group(&client->dev.kobj,
@@ -666,43 +708,44 @@ static int edt_ft5x06_i2c_ts_probe(struct i2c_client *client,
 	if (error)
 		goto err_free_irq;
 
+	error = input_register_device(input);
+	if (error)
+		goto err_remove_attrs;
+
+	i2c_set_clientdata(client, tsdata);
 	device_init_wakeup(&client->dev, 1);
 
 	dev_dbg(&tsdata->client->dev,
-	        "EDT FT5x06 initialized: IRQ pin %d, Reset pin %d.\n",
-	        tsdata->irq_pin, tsdata->reset_pin);
+		"EDT FT5x06 initialized: IRQ pin %d, Reset pin %d.\n",
+		pdata->irq_pin, pdata->reset_pin);
 
 	return 0;
 
+err_remove_attrs:
+	sysfs_remove_group(&client->dev.kobj, &edt_ft5x06_i2c_attr_group);
 err_free_irq:
-	free_irq(tsdata->irq, tsdata);
-err_unregister_device:
-	input_unregister_device(input);
-	input = NULL;
-err_free_input_device:
-	if (input)
-		input_free_device(input);
-err_free_irq_pin:
-	gpio_free(tsdata->irq_pin);
-err_free_reset_pin:
-	if (tsdata->reset_pin >= 0)
-		gpio_free(tsdata->reset_pin);
-err_free_tsdata:
+	free_irq(client->irq, tsdata);
+err_free_mem:
+	input_free_device(input);
 	kfree(tsdata);
+
+	if (gpio_is_valid(pdata->irq_pin))
+		gpio_free(pdata->irq_pin);
+
 	return error;
 }
 
-static int edt_ft5x06_i2c_ts_remove(struct i2c_client *client)
+static int __devexit edt_ft5x06_i2c_ts_remove(struct i2c_client *client)
 {
-	struct edt_ft5x06_i2c_ts_data *tsdata = dev_get_drvdata(&client->dev);
+	const struct edt_ft5x06_platform_data *pdata = client->dev.platform_data;
+	struct edt_ft5x06_i2c_ts_data *tsdata = i2c_get_clientdata(client);
 
 	sysfs_remove_group(&client->dev.kobj, &edt_ft5x06_i2c_attr_group);
 
-	free_irq(tsdata->irq, tsdata);
+	free_irq(client->irq, tsdata);
 	input_unregister_device(tsdata->input);
-	gpio_free(tsdata->irq_pin);
-	if (tsdata->reset_pin >= 0)
-		gpio_free(tsdata->reset_pin);
+	if (gpio_is_valid(pdata->irq_pin))
+		gpio_free(pdata->irq_pin);
 	kfree(tsdata);
 
 	return 0;
@@ -711,20 +754,20 @@ static int edt_ft5x06_i2c_ts_remove(struct i2c_client *client)
 #ifdef CONFIG_PM_SLEEP
 static int edt_ft5x06_i2c_ts_suspend(struct device *dev)
 {
-	struct edt_ft5x06_i2c_ts_data *tsdata = dev_get_drvdata(dev);
+	struct i2c_client *client = to_i2c_client(dev);
 
 	if (device_may_wakeup(dev))
-		enable_irq_wake(tsdata->irq);
+		enable_irq_wake(client->irq);
 
 	return 0;
 }
 
 static int edt_ft5x06_i2c_ts_resume(struct device *dev)
 {
-	struct edt_ft5x06_i2c_ts_data *tsdata = dev_get_drvdata(dev);
+	struct i2c_client *client = to_i2c_client(dev);
 
 	if (device_may_wakeup(dev))
-		disable_irq_wake(tsdata->irq);
+		disable_irq_wake(client->irq);
 
 	return 0;
 }
@@ -747,20 +790,10 @@ static struct i2c_driver edt_ft5x06_i2c_ts_driver = {
 	},
 	.id_table = edt_ft5x06_i2c_ts_id,
 	.probe    = edt_ft5x06_i2c_ts_probe,
-	.remove   = edt_ft5x06_i2c_ts_remove,
+	.remove   = __devexit_p(edt_ft5x06_i2c_ts_remove),
 };
 
-static int __init edt_ft5x06_i2c_ts_init(void)
-{
-	return i2c_add_driver(&edt_ft5x06_i2c_ts_driver);
-}
-module_init(edt_ft5x06_i2c_ts_init);
-
-static void __exit edt_ft5x06_i2c_ts_exit(void)
-{
-	i2c_del_driver(&edt_ft5x06_i2c_ts_driver);
-}
-module_exit(edt_ft5x06_i2c_ts_exit);
+module_i2c_driver(edt_ft5x06_i2c_ts_driver);
 
 MODULE_AUTHOR("Simon Budig <simon.budig@kernelconcepts.de>");
 MODULE_DESCRIPTION("EDT FT5x06 I2C Touchscreen Driver");