[PATCH V3 0/3] Input: Cypress TTSP device driver

[PATCH V3 0/3] Input: Cypress TTSP device driver

[Javier Martinez Canillas]

Cypress TrueTouch(tm) Standard Product controllers are found in
a wide range of embedded devices. This patch-set adds support for a
variety of TTSP controllers.

The original author of the driver is Kevin McNeely <kev@cypress.com>

Since the hardware is capable of tracking identifiable contacts and the
original driver used multi-touch protocol type A (stateless), I've added
multi-touch protocol type B (stateful) support.

The driver is composed of a core driver that process the data sent by
the contacts (fingers) and two bus specific interface modules (I2C and SPI).

This is a third version of the driver that fixes issues called out by
Dmitry Torokhov, Henrik Rydberg and Mohan Pallaka

The patchset is composed of the patches:

[PATCH V3 1/3] Input: cyttsp - Cypress TTSP capacitive multi-touch screen support
[PATCH V3 2/3] Input: cyttsp - add support for Cypress TTSP touchscreen I2C bus interface
[PATCH V3 3/3] Input: cyttsp - add support for Cypress TTSP touchscreen SPI bus interface

[PATCH V3 1/3] Input: cyttsp - Cypress TTSP capacitive multi-touch screen support

[Javier Martinez Canillas]

Cypress TrueTouch(tm) Standard Product controllers are found in
a wide range of embedded devices. This driver add support for a
variety of TTSP controllers.

The driver is composed of a core driver that process the data sent by
the contacts and a set of bus specific interface modules. This patch
adds the base core TTSP driver.

The original author of the driver is Kevin McNeely <kev@cypress.com>

Since the hardware is capable of tracking identifiable contacts and the
original driver used multi-touch protocol type A (stateless), multi-touch
protocol type B (stateful) support was added by Javier Martinez Canillas.

Signed-off-by: Javier Martinez Canillas <martinez.javier@gmail.com>
---
V2: Fix issues called out by Dmitry Torokhov
     - Add msleep() delays between retries for read and write operations
     - Change cyttsp_core_init() to receive the IRQ from the client data 
       instead of obtaining from the platform_data

V3: Fix issues called out by Henrik Rydberg and Mohan Pallaka
    - Map each possible track id to a multitouch input slot
    - Remove bus type info since it is not used
    - Add retry logic to ttsp_write_block_data()
    - ttsp_read_block_data() already msleep() remove the sleep in the caller
    - cyttsp_xy_worker() sounds as if it's a workqueue, change the function name
    - Check if handle is NULL in cyttsp_resume()
    - Use platform data's use_sleep to decide to go deep sleep or low power mode
    - input_register_device() error path has to call input_free_device()

 drivers/input/touchscreen/Kconfig              |    2 +
 drivers/input/touchscreen/Makefile             |    1 +
 drivers/input/touchscreen/cyttsp/Kconfig       |   28 +
 drivers/input/touchscreen/cyttsp/Makefile      |    3 +
 drivers/input/touchscreen/cyttsp/cyttsp_core.c |  853 ++++++++++++++++++++++++
 drivers/input/touchscreen/cyttsp/cyttsp_core.h |   56 ++
 include/linux/input/cyttsp.h                   |   68 ++
 7 files changed, 1011 insertions(+), 0 deletions(-)
 create mode 100644 drivers/input/touchscreen/cyttsp/Kconfig
 create mode 100644 drivers/input/touchscreen/cyttsp/Makefile
 create mode 100644 drivers/input/touchscreen/cyttsp/cyttsp_core.c
 create mode 100644 drivers/input/touchscreen/cyttsp/cyttsp_core.h
 create mode 100644 include/linux/input/cyttsp.h

diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
index cabd9e5..6933823 100644
--- a/drivers/input/touchscreen/Kconfig
+++ b/drivers/input/touchscreen/Kconfig
@@ -726,4 +726,6 @@ config TOUCHSCREEN_TPS6507X
 	  To compile this driver as a module, choose M here: the
 	  module will be called tps6507x_ts.
 
+source "drivers/input/touchscreen/cyttsp/Kconfig"
+
 endif
diff --git a/drivers/input/touchscreen/Makefile b/drivers/input/touchscreen/Makefile
index 282d6f7..ff7ad47 100644
--- a/drivers/input/touchscreen/Makefile
+++ b/drivers/input/touchscreen/Makefile
@@ -60,3 +60,4 @@ obj-$(CONFIG_TOUCHSCREEN_WM97XX_MAINSTONE)	+= mainstone-wm97xx.o
 obj-$(CONFIG_TOUCHSCREEN_WM97XX_ZYLONITE)	+= zylonite-wm97xx.o
 obj-$(CONFIG_TOUCHSCREEN_W90X900)	+= w90p910_ts.o
 obj-$(CONFIG_TOUCHSCREEN_TPS6507X)	+= tps6507x-ts.o
+obj-S(CONFIG_TOUCHSCREEN_CYTTSP_CORE)	+= cyttsp/
diff --git a/drivers/input/touchscreen/cyttsp/Kconfig b/drivers/input/touchscreen/cyttsp/Kconfig
new file mode 100644
index 0000000..e72c66c
--- /dev/null
+++ b/drivers/input/touchscreen/cyttsp/Kconfig
@@ -0,0 +1,28 @@
+config TOUCHSCREEN_CYTTSP_CORE
+	tristate "Cypress TTSP touchscreen core"
+	help
+	  Always activated for Cypress TTSP touchscreen
+
+config TOUCHSCREEN_CYTTSP_I2C
+	tristate "Cypress TTSP i2c touchscreen"
+	depends on I2C && TOUCHSCREEN_CYTTSP_CORE
+	help
+	  Say Y here if you have a Cypress TTSP touchscreen
+	  connected to your system with an I2C interface.
+
+	  If unsure, say N.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called cyttsp-i2c.
+
+config TOUCHSCREEN_CYTTSP_SPI
+	tristate "Cypress TTSP spi touchscreen"
+	depends on SPI_MASTER && TOUCHSCREEN_CYTTSP_CORE
+	help
+	  Say Y here if you have a Cypress TTSP touchscreen
+	  connected to your  with an SPI interface.
+
+	  If unsure, say N.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called cyttsp-spi.
diff --git a/drivers/input/touchscreen/cyttsp/Makefile b/drivers/input/touchscreen/cyttsp/Makefile
new file mode 100644
index 0000000..687eeaa
--- /dev/null
+++ b/drivers/input/touchscreen/cyttsp/Makefile
@@ -0,0 +1,3 @@
+obj-$(CONFIG_TOUCHSCREEN_CYTTSP_CORE)   += cyttsp_core.o
+obj-$(CONFIG_TOUCHSCREEN_CYTTSP_I2C)    += cyttsp_i2c.o
+obj-$(CONFIG_TOUCHSCREEN_CYTTSP_SPI)    += cyttsp_spi.o
diff --git a/drivers/input/touchscreen/cyttsp/cyttsp_core.c b/drivers/input/touchscreen/cyttsp/cyttsp_core.c
new file mode 100644
index 0000000..8e7bd66
--- /dev/null
+++ b/drivers/input/touchscreen/cyttsp/cyttsp_core.c
@@ -0,0 +1,853 @@
+/*
+ * Core Source for:
+ * Cypress TrueTouch(TM) Standard Product (TTSP) touchscreen drivers.
+ * For use with Cypress Txx3xx parts.
+ * Supported parts include:
+ * CY8CTST341
+ * CY8CTMA340
+ *
+ * Copyright (C) 2009, 2010, 2011 Cypress Semiconductor, Inc.
+ * Copyright (C) 2011 Javier Martinez Canillas <martinez.javier@gmail.com>
+ *
+ * Added multi-touch protocol type B support by Javier Martinez Canillas
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2, and only version 2, as published by the
+ * Free Software Foundation.
+ *
+ * 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 program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+ *
+ * Contact Cypress Semiconductor at www.cypress.com <kev@cypress.com>
+ *
+ */
+
+#include "cyttsp_core.h"
+
+#include <linux/delay.h>
+#include <linux/input.h>
+#include <linux/input/mt.h>
+#include <linux/gpio.h>
+#include <linux/interrupt.h>
+#include <linux/slab.h>
+
+/* Bootloader number of command keys */
+#define CY_NUM_BL_KEYS    8
+
+/* helpers */
+#define GET_NUM_TOUCHES(x)          ((x) & 0x0F)
+#define IS_LARGE_AREA(x)            (((x) & 0x10) >> 4)
+#define IS_BAD_PKT(x)               ((x) & 0x20)
+#define IS_VALID_APP(x)             ((x) & 0x01)
+#define IS_OPERATIONAL_ERR(x)       ((x) & 0x3F)
+#define GET_HSTMODE(reg)            ((reg & 0x70) >> 4)
+#define GET_BOOTLOADERMODE(reg)     ((reg & 0x10) >> 4)
+
+#define CY_REG_BASE                 0x00
+#define CY_REG_ACT_DIST             0x1E
+#define CY_REG_ACT_INTRVL           0x1D
+#define CY_REG_TCH_TMOUT            (CY_REG_ACT_INTRVL+1)
+#define CY_REG_LP_INTRVL            (CY_REG_TCH_TMOUT+1)
+#define CY_MAXZ                     255
+#define CY_DELAY_DFLT               20 /* ms */
+#define CY_DELAY_MAX                (500/CY_DELAY_DFLT) /* half second */
+#define CY_ACT_DIST_DFLT            0xF8
+#define CY_HNDSHK_BIT               0x80
+/* device mode bits */
+#define CY_OPERATE_MODE             0x00
+#define CY_SYSINFO_MODE             0x10
+/* power mode select bits */
+#define CY_SOFT_RESET_MODE          0x01 /* return to Bootloader mode */
+#define CY_DEEP_SLEEP_MODE          0x02
+#define CY_LOW_POWER_MODE           0x04
+
+/* Slots management */
+#define CY_MAX_FINGER               4
+#define CY_UNUSED                   0
+#define CY_USED                     1
+#define CY_MAX_ID                   15
+
+struct cyttsp_tch {
+	__be16 x, y;
+	u8 z;
+} __packed;
+
+/* TrueTouch Standard Product Gen3 interface definition */
+struct cyttsp_xydata {
+	u8 hst_mode;
+	u8 tt_mode;
+	u8 tt_stat;
+	struct cyttsp_tch tch1;
+	u8 touch12_id;
+	struct cyttsp_tch tch2;
+	u8 gest_cnt;
+	u8 gest_id;
+	struct cyttsp_tch tch3;
+	u8 touch34_id;
+	struct cyttsp_tch tch4;
+	u8 tt_undef[3];
+	u8 act_dist;
+	u8 tt_reserved;
+} __packed;
+
+/* TTSP System Information interface definition */
+struct cyttsp_sysinfo_data {
+	u8 hst_mode;
+	u8 mfg_cmd;
+	u8 mfg_stat;
+	u8 cid[3];
+	u8 tt_undef1;
+	u8 uid[8];
+	u8 bl_verh;
+	u8 bl_verl;
+	u8 tts_verh;
+	u8 tts_verl;
+	u8 app_idh;
+	u8 app_idl;
+	u8 app_verh;
+	u8 app_verl;
+	u8 tt_undef[5];
+	u8 scn_typ;
+	u8 act_intrvl;
+	u8 tch_tmout;
+	u8 lp_intrvl;
+};
+
+/* TTSP Bootloader Register Map interface definition */
+#define CY_BL_CHKSUM_OK 0x01
+struct cyttsp_bootloader_data {
+	u8 bl_file;
+	u8 bl_status;
+	u8 bl_error;
+	u8 blver_hi;
+	u8 blver_lo;
+	u8 bld_blver_hi;
+	u8 bld_blver_lo;
+	u8 ttspver_hi;
+	u8 ttspver_lo;
+	u8 appid_hi;
+	u8 appid_lo;
+	u8 appver_hi;
+	u8 appver_lo;
+	u8 cid_0;
+	u8 cid_1;
+	u8 cid_2;
+};
+
+struct cyttsp {
+	struct device *dev;
+	int irq;
+	struct input_dev *input;
+	char phys[32];
+	const struct cyttsp_platform_data *platform_data;
+	struct cyttsp_bus_ops *bus_ops;
+	struct cyttsp_bootloader_data bl_data;
+	struct cyttsp_sysinfo_data sysinfo_data;
+	struct completion bl_ready;
+	enum cyttsp_powerstate power_state;
+	int slot_curr[CY_MAX_ID];
+	int slot_prev[CY_MAX_ID];
+};
+
+static const u8 bl_command[] = {
+	0x00,			/* file offset */
+	0xFF,			/* command */
+	0xA5,			/* exit bootloader command */
+	0, 1, 2, 3, 4, 5, 6, 7	/* default keys */
+};
+
+static int ttsp_read_block_data(struct cyttsp *ts, u8 command,
+	u8 length, void *buf)
+{
+	int retval;
+	int tries;
+
+	if (!buf || !length)
+		return -EINVAL;
+
+	for (tries = 0, retval = -1;
+	     tries < CY_NUM_RETRY && (retval < 0);
+	     tries++) {
+		retval = ts->bus_ops->read(ts->bus_ops, command, length, buf);
+		if (retval)
+			msleep(CY_DELAY_DFLT);
+	}
+
+	return retval;
+}
+
+static int ttsp_write_block_data(struct cyttsp *ts, u8 command,
+	u8 length, void *buf)
+{
+	int retval;
+	int tries;
+
+	if (!buf || !length)
+		return -EINVAL;
+
+	for (tries = 0, retval = -1;
+	     tries < CY_NUM_RETRY && (retval < 0);
+	     tries++) {
+		retval = ts->bus_ops->write(ts->bus_ops, command, length, buf);
+		if (retval)
+			msleep(CY_DELAY_DFLT);
+	}
+
+	return retval;
+}
+
+static int ttsp_tch_ext(struct cyttsp *ts, void *buf)
+{
+	int retval;
+
+	if (!buf)
+		return -EIO;
+
+	retval = ts->bus_ops->ext(ts->bus_ops, buf);
+
+	return retval;
+}
+
+static int cyttsp_load_bl_regs(struct cyttsp *ts)
+{
+	int retval;
+
+	memset(&(ts->bl_data), 0, sizeof(struct cyttsp_bootloader_data));
+
+	retval =  ttsp_read_block_data(ts, CY_REG_BASE,
+		sizeof(ts->bl_data), &(ts->bl_data));
+
+	return retval;
+}
+
+static int cyttsp_bl_app_valid(struct cyttsp *ts)
+{
+	int retval;
+
+	retval = cyttsp_load_bl_regs(ts);
+
+	if (retval < 0)
+		return -ENODEV;
+
+	if (GET_BOOTLOADERMODE(ts->bl_data.bl_status)) {
+		if (IS_VALID_APP(ts->bl_data.bl_status)) {
+			dev_dbg(ts->dev, "%s: App found; normal boot\n",
+				__func__);
+			return 0;
+		} else {
+			dev_dbg(ts->dev, "%s: NO APP; load firmware!!\n",
+				__func__);
+			return -ENODEV;
+		}
+	} else if (GET_HSTMODE(ts->bl_data.bl_file) == CY_OPERATE_MODE) {
+		if (!(IS_OPERATIONAL_ERR(ts->bl_data.bl_status))) {
+			dev_dbg(ts->dev, "%s: Operational\n",
+				__func__);
+			return 1;
+		} else {
+			dev_dbg(ts->dev, "%s: Operational failure\n",
+				__func__);
+			return -ENODEV;
+		}
+	} else {
+		dev_dbg(ts->dev, "%s: Non-Operational failure\n",
+			__func__);
+			return -ENODEV;
+	}
+
+}
+
+static int cyttsp_exit_bl_mode(struct cyttsp *ts)
+{
+	int retval;
+	int tries;
+	u8 bl_cmd[sizeof(bl_command)];
+
+	memcpy(bl_cmd, bl_command, sizeof(bl_command));
+	if (ts->platform_data->bl_keys)
+		memcpy(&bl_cmd[sizeof(bl_command) - CY_NUM_BL_KEYS],
+			ts->platform_data->bl_keys, sizeof(bl_command));
+
+	dev_dbg(ts->dev,
+		"%s: bl_cmd= "
+		"%02X %02X %02X %02X %02X %02X %02X %02X %02X %02X %02X\n",
+		__func__, bl_cmd[0], bl_cmd[1], bl_cmd[2],
+		bl_cmd[3], bl_cmd[4], bl_cmd[5], bl_cmd[6],
+		bl_cmd[7], bl_cmd[8], bl_cmd[9], bl_cmd[10]);
+
+	retval = ttsp_write_block_data(ts, CY_REG_BASE,
+		sizeof(bl_cmd), (void *)bl_cmd);
+	if (retval < 0)
+		return retval;
+
+	/* wait for TTSP Device to complete switch to Operational mode */
+	tries = 0;
+	do {
+		msleep(CY_DELAY_DFLT);
+		retval = cyttsp_load_bl_regs(ts);
+	} while (!((retval == 0) &&
+		!GET_BOOTLOADERMODE(ts->bl_data.bl_status)) &&
+		(tries++ < CY_DELAY_MAX));
+
+	dev_dbg(ts->dev, "%s: check bl ready tries=%d ret=%d stat=%02X\n",
+		__func__, tries, retval, ts->bl_data.bl_status);
+
+	if (retval < 0)
+		return retval;
+	else if (GET_BOOTLOADERMODE(ts->bl_data.bl_status))
+		return -ENODEV;
+	else
+		return 0;
+}
+
+static int cyttsp_set_operational_mode(struct cyttsp *ts)
+{
+	struct cyttsp_xydata xy_data;
+	int retval;
+	int tries;
+	u8 cmd = CY_OPERATE_MODE;
+
+	retval = ttsp_write_block_data(ts, CY_REG_BASE, sizeof(cmd), &cmd);
+
+	if (retval < 0)
+		return retval;
+
+	/* wait for TTSP Device to complete switch to Operational mode */
+	tries = 0;
+	do {
+		retval = ttsp_read_block_data(ts, CY_REG_BASE,
+			sizeof(xy_data), &(xy_data));
+	} while (!((retval == 0) &&
+		(xy_data.act_dist == CY_ACT_DIST_DFLT)) &&
+		(tries++ < CY_DELAY_MAX));
+
+	dev_dbg(ts->dev, "%s: check op ready tries=%d ret=%d dist=%02X\n",
+		__func__, tries, retval, xy_data.act_dist);
+
+	return retval;
+}
+
+static int cyttsp_set_sysinfo_mode(struct cyttsp *ts)
+{
+	int retval;
+	int tries;
+	u8 cmd = CY_SYSINFO_MODE;
+
+	memset(&(ts->sysinfo_data), 0, sizeof(struct cyttsp_sysinfo_data));
+
+	/* switch to sysinfo mode */
+	retval = ttsp_write_block_data(ts, CY_REG_BASE, sizeof(cmd), &cmd);
+	if (retval < 0)
+		return retval;
+
+	/* read sysinfo registers */
+	tries = 0;
+	do {
+		msleep(CY_DELAY_DFLT);
+		retval = ttsp_read_block_data(ts, CY_REG_BASE,
+			sizeof(ts->sysinfo_data), &(ts->sysinfo_data));
+	} while (!((retval == 0) &&
+		!((ts->sysinfo_data.tts_verh == 0) &&
+		(ts->sysinfo_data.tts_verl == 0))) &&
+		(tries++ < CY_DELAY_MAX));
+
+	dev_dbg(ts->dev, "%s: check sysinfo ready tries=%d ret=%d\n",
+		__func__, tries, retval);
+
+	dev_info(ts->dev, "%s: tv=%02X%02X ai=0x%02X%02X "
+		"av=0x%02X%02X ci=0x%02X%02X%02X\n", "cyttsp",
+		ts->sysinfo_data.tts_verh, ts->sysinfo_data.tts_verl,
+		ts->sysinfo_data.app_idh, ts->sysinfo_data.app_idl,
+		ts->sysinfo_data.app_verh, ts->sysinfo_data.app_verl,
+		ts->sysinfo_data.cid[0], ts->sysinfo_data.cid[1],
+		ts->sysinfo_data.cid[2]);
+
+	return retval;
+}
+
+static int cyttsp_set_sysinfo_regs(struct cyttsp *ts)
+{
+	int retval = 0;
+
+	if (ts->platform_data->act_intrvl != CY_ACT_INTRVL_DFLT ||
+		ts->platform_data->tch_tmout != CY_TCH_TMOUT_DFLT ||
+		ts->platform_data->lp_intrvl != CY_LP_INTRVL_DFLT) {
+
+		u8 intrvl_ray[3];
+
+		intrvl_ray[0] = ts->platform_data->act_intrvl;
+		intrvl_ray[1] = ts->platform_data->tch_tmout;
+		intrvl_ray[2] = ts->platform_data->lp_intrvl;
+
+		/* set intrvl registers */
+		retval = ttsp_write_block_data(ts,
+				CY_REG_ACT_INTRVL,
+				sizeof(intrvl_ray), intrvl_ray);
+
+		msleep(CY_DELAY_DFLT);
+	}
+
+	return retval;
+}
+
+static int cyttsp_soft_reset(struct cyttsp *ts)
+{
+	int retval;
+	u8 cmd = CY_SOFT_RESET_MODE;
+
+	retval = ttsp_write_block_data(ts, CY_REG_BASE, sizeof(cmd), &cmd);
+	if (retval < 0)
+		return retval;
+
+	/* wait for interrupt to set ready completion */
+	INIT_COMPLETION(ts->bl_ready);
+
+	retval = wait_for_completion_interruptible_timeout(&ts->bl_ready,
+		msecs_to_jiffies(CY_DELAY_DFLT * CY_DELAY_MAX));
+
+	if (retval > 0)
+		retval = 0;
+
+	return retval;
+}
+
+static int cyttsp_act_dist_setup(struct cyttsp *ts)
+{
+	int retval;
+	u8 act_dist_setup;
+
+	/* Init gesture; active distance setup */
+	act_dist_setup = ts->platform_data->act_dist;
+	retval = ttsp_write_block_data(ts, CY_REG_ACT_DIST,
+		sizeof(act_dist_setup), &act_dist_setup);
+
+	return retval;
+}
+
+static int cyttsp_hndshk(struct cyttsp *ts, u8 hst_mode)
+{
+	int retval;
+	u8 cmd;
+
+	cmd = hst_mode & CY_HNDSHK_BIT ?
+		hst_mode & ~CY_HNDSHK_BIT :
+		hst_mode | CY_HNDSHK_BIT;
+
+	retval = ttsp_write_block_data(ts, CY_REG_BASE,
+		sizeof(cmd), (u8 *)&cmd);
+
+	return retval;
+}
+
+static void cyttsp_report_slot(struct input_dev *dev, int slot,
+			       int x, int y, int z)
+{
+	input_mt_slot(dev, slot);
+	input_mt_report_slot_state(dev, MT_TOOL_FINGER, true);
+	input_report_abs(dev, ABS_MT_POSITION_X, x);
+	input_report_abs(dev, ABS_MT_POSITION_Y, y);
+	input_report_abs(dev, ABS_MT_TOUCH_MAJOR, z);
+}
+
+static void cyttsp_report_slot_empty(struct input_dev *dev, int slot)
+{
+	input_mt_slot(dev, slot);
+	input_mt_report_slot_state(dev, MT_TOOL_FINGER, false);
+}
+
+static void cyttsp_extract_track_ids(struct cyttsp_xydata *xy_data, int *ids)
+{
+	ids[0] = xy_data->touch12_id >> 4;
+	ids[1] = xy_data->touch12_id & 0xF;
+	ids[2] = xy_data->touch34_id >> 4;
+	ids[3] = xy_data->touch34_id & 0xF;
+}
+
+static void cyttsp_get_tch(struct cyttsp_xydata *xy_data, int idx,
+			   struct cyttsp_tch **tch)
+{
+	switch (idx) {
+	case 0:
+		*tch = &xy_data->tch1;
+		break;
+	case 1:
+		*tch = &xy_data->tch2;
+		break;
+	case 2:
+		*tch = &xy_data->tch3;
+		break;
+	case 3:
+		*tch = &xy_data->tch4;
+		break;
+	}
+}
+
+static int cyttsp_handle_tchdata(struct cyttsp *ts)
+{
+	struct cyttsp_xydata xy_data;
+	u8 num_cur_tch;
+	int i;
+	int ids[4];
+	struct cyttsp_tch *tch = NULL;
+	int x, y, z;
+
+	/* Get touch data from CYTTSP device */
+	if (ttsp_read_block_data(ts,
+		CY_REG_BASE, sizeof(struct cyttsp_xydata), &xy_data))
+		return 0;
+
+	/* touch extension handling */
+	if (ttsp_tch_ext(ts, &xy_data))
+		return 0;
+
+	/* provide flow control handshake */
+	if (ts->platform_data->use_hndshk)
+		if (cyttsp_hndshk(ts, xy_data.hst_mode))
+			return 0;
+
+	/* determine number of currently active touches */
+	num_cur_tch = GET_NUM_TOUCHES(xy_data.tt_stat);
+
+	/* check for any error conditions */
+	if (ts->power_state == CY_IDLE_STATE)
+		return 0;
+	else if (GET_BOOTLOADERMODE(xy_data.tt_mode)) {
+		return -1;
+	} else if (IS_LARGE_AREA(xy_data.tt_stat) == 1) {
+		/* terminate all active tracks */
+		num_cur_tch = 0;
+		dev_dbg(ts->dev, "%s: Large area detected\n", __func__);
+	} else if (num_cur_tch > CY_MAX_FINGER) {
+		/* terminate all active tracks */
+		num_cur_tch = 0;
+		dev_dbg(ts->dev, "%s: Num touch error detected\n", __func__);
+	} else if (IS_BAD_PKT(xy_data.tt_mode)) {
+		/* terminate all active tracks */
+		num_cur_tch = 0;
+		dev_dbg(ts->dev, "%s: Invalid buffer detected\n", __func__);
+	}
+
+	cyttsp_extract_track_ids(&xy_data, ids);
+
+	for (i = 0; i < num_cur_tch; i++) {
+		ts->slot_curr[ids[i] - 1] = CY_USED;
+
+		cyttsp_get_tch(&xy_data, i, &tch);
+
+		x = be16_to_cpu(tch->x);
+		y = be16_to_cpu(tch->y);
+		z = tch->z;
+
+		cyttsp_report_slot(ts->input, ids[i] - 1, x, y, z);
+	}
+
+	for (i = 0; i < CY_MAX_ID; i++) {
+		if (ts->slot_prev[i] == CY_USED &&
+		    ts->slot_curr[i] == CY_UNUSED)
+			cyttsp_report_slot_empty(ts->input, i);
+		ts->slot_prev[i] = ts->slot_curr[i];
+		ts->slot_curr[i] = CY_UNUSED;
+	}
+
+	input_sync(ts->input);
+
+	return 0;
+}
+
+static void cyttsp_pr_state(struct cyttsp *ts)
+{
+	static char *cyttsp_powerstate_string[] = {
+		"IDLE",
+		"ACTIVE",
+		"LOW_PWR",
+		"SLEEP",
+		"BOOTLOADER",
+		"INVALID"
+	};
+
+	dev_info(ts->dev, "%s: %s\n", __func__,
+		ts->power_state < CY_INVALID_STATE ?
+		cyttsp_powerstate_string[ts->power_state] :
+		"INVALID");
+}
+
+static irqreturn_t cyttsp_irq(int irq, void *handle)
+{
+	struct cyttsp *ts = handle;
+	int retval;
+
+	if (ts->power_state == CY_BL_STATE)
+		complete(&ts->bl_ready);
+	else {
+		/* process the touches */
+		retval = cyttsp_handle_tchdata(ts);
+
+		if (retval < 0) {
+			/*
+			 * TTSP device has reset back to bootloader mode.
+			 * Restore to operational mode.
+			 */
+			retval = cyttsp_exit_bl_mode(ts);
+			if (retval)
+				ts->power_state = CY_IDLE_STATE;
+			else
+				ts->power_state = CY_ACTIVE_STATE;
+			cyttsp_pr_state(ts);
+		}
+	}
+
+	return IRQ_HANDLED;
+}
+
+static int cyttsp_power_on(struct cyttsp *ts)
+{
+	int retval = 0;
+
+	if (!ts)
+		return -ENOMEM;
+
+	ts->power_state = CY_BL_STATE;
+
+	/* enable interrupts */
+	retval = request_threaded_irq(ts->irq, NULL, cyttsp_irq,
+		IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
+		ts->platform_data->name, ts);
+	if (retval < 0)
+		goto bypass;
+
+	retval = cyttsp_soft_reset(ts);
+	if (retval < 0)
+		goto bypass;
+
+	retval = cyttsp_bl_app_valid(ts);
+	if (retval < 0)
+		goto bypass;
+	else if (retval > 0)
+		goto no_bl_bypass;
+
+	retval = cyttsp_exit_bl_mode(ts);
+
+	if (retval < 0)
+		goto bypass;
+
+	ts->power_state = CY_IDLE_STATE;
+
+no_bl_bypass:
+	retval = cyttsp_set_sysinfo_mode(ts);
+	if (retval < 0)
+		goto bypass;
+
+	retval = cyttsp_set_sysinfo_regs(ts);
+	if (retval < 0)
+		goto bypass;
+
+	retval = cyttsp_set_operational_mode(ts);
+	if (retval < 0)
+		goto bypass;
+
+	/* init active distance */
+	retval = cyttsp_act_dist_setup(ts);
+	if (retval < 0)
+		goto bypass;
+
+	ts->power_state = CY_ACTIVE_STATE;
+	retval = 0;
+
+bypass:
+	cyttsp_pr_state(ts);
+	return retval;
+}
+
+#ifdef CONFIG_PM
+int cyttsp_resume(void *handle)
+{
+	struct cyttsp *ts = handle;
+	int retval = 0;
+	struct cyttsp_xydata xydata;
+
+	if (ts) {
+		if (ts->platform_data->use_sleep && (ts->power_state !=
+						     CY_ACTIVE_STATE)) {
+			if (ts->platform_data->wakeup) {
+				retval = ts->platform_data->wakeup();
+				if (retval < 0)
+					dev_dbg(ts->dev, "%s: Error, wakeup failed!\n",
+						__func__);
+			} else {
+				dev_dbg(ts->dev, "%s: Error, wakeup not implemented "
+					"(check board file).\n", __func__);
+				retval = -ENOSYS;
+			}
+			if (!(retval < 0)) {
+				retval = ttsp_read_block_data(ts, CY_REG_BASE,
+							      sizeof(xydata), &xydata);
+				if (!(retval < 0) && !GET_HSTMODE(xydata.hst_mode))
+					ts->power_state = CY_ACTIVE_STATE;
+			}
+		}
+		dev_dbg(ts->dev, "%s: Wake Up %s\n", __func__,
+			(retval < 0) ? "FAIL" : "PASS");
+	}
+	return retval;
+}
+EXPORT_SYMBOL_GPL(cyttsp_resume);
+
+int cyttsp_suspend(void *handle)
+{
+	struct cyttsp *ts = handle;
+	u8 sleep_mode = 0;
+	int retval = 0;
+
+	if (ts->platform_data->use_sleep &&
+		(ts->power_state == CY_ACTIVE_STATE)) {
+		sleep_mode = ts->platform_data->use_sleep;
+		retval = ttsp_write_block_data(ts,
+			CY_REG_BASE, sizeof(sleep_mode), &sleep_mode);
+		if (!(retval < 0))
+			ts->power_state = CY_SLEEP_STATE;
+	}
+	dev_dbg(ts->dev, "%s: Sleep Power state is %s\n", __func__,
+		(ts->power_state == CY_ACTIVE_STATE) ?
+		"ACTIVE" :
+		((ts->power_state == CY_SLEEP_STATE) ?
+		"SLEEP" : "LOW POWER"));
+	return retval;
+}
+EXPORT_SYMBOL_GPL(cyttsp_suspend);
+#endif
+
+static int cyttsp_open(struct input_dev *dev)
+{
+	struct cyttsp *ts = input_get_drvdata(dev);
+
+	return cyttsp_power_on(ts);
+}
+
+void cyttsp_core_release(void *handle)
+{
+	struct cyttsp *ts = handle;
+
+	if (ts) {
+		free_irq(ts->irq, ts);
+		input_unregister_device(ts->input);
+		if (ts->platform_data->exit)
+			ts->platform_data->exit();
+		input_mt_destroy_slots(ts->input);
+		kfree(ts);
+	}
+}
+EXPORT_SYMBOL_GPL(cyttsp_core_release);
+
+static void cyttsp_close(struct input_dev *dev)
+{
+	struct cyttsp *ts = input_get_drvdata(dev);
+
+	free_irq(ts->irq, ts);
+}
+
+void *cyttsp_core_init(struct cyttsp_bus_ops *bus_ops, struct device *dev, int irq)
+{
+	struct input_dev *input_device;
+	int i;
+	int ret;
+
+	struct cyttsp *ts = kzalloc(sizeof(*ts), GFP_KERNEL);
+
+	if (!ts) {
+		pr_err("%s: Error, kzalloc\n", __func__);
+		goto error_alloc_data;
+	}
+
+	if (dev == NULL || bus_ops == NULL) {
+		kfree(ts);
+		goto error_alloc_data;
+	}
+
+	ts->dev = dev;
+	ts->platform_data = dev->platform_data;
+	ts->bus_ops = bus_ops;
+	init_completion(&ts->bl_ready);
+
+	if (ts->platform_data->init) {
+		if (ts->platform_data->init()) {
+			dev_dbg(ts->dev, "%s: Error, platform init failed!\n",
+				__func__);
+			goto error_init;
+		}
+	}
+
+	ts->irq = irq;
+	if (ts->irq <= 0) {
+		dev_dbg(ts->dev, "%s: Error, failed to allocate irq\n",
+			__func__);
+			goto error_init;
+	}
+
+	/* Create the input device and register it. */
+	input_device = input_allocate_device();
+	if (!input_device) {
+		dev_dbg(ts->dev, "%s: Error, failed to allocate input device\n",
+			__func__);
+		goto error_input_allocate_device;
+	}
+
+	ts->input = input_device;
+	input_device->name = ts->platform_data->name;
+	snprintf(ts->phys, sizeof(ts->phys), "%s", dev_name(dev));
+	input_device->phys = ts->phys;
+	input_device->dev.parent = ts->dev;
+	input_device->open = cyttsp_open;
+	input_device->close = cyttsp_close;
+	input_set_drvdata(input_device, ts);
+
+	__set_bit(EV_SYN, input_device->evbit);
+	__set_bit(EV_KEY, input_device->evbit);
+	__set_bit(EV_ABS, input_device->evbit);
+
+	input_set_abs_params(input_device, ABS_MT_POSITION_X,
+			     0, ts->platform_data->maxx, 0, 0);
+	input_set_abs_params(input_device, ABS_MT_POSITION_Y,
+			     0, ts->platform_data->maxy, 0, 0);
+	input_set_abs_params(input_device, ABS_MT_TOUCH_MAJOR,
+			     0, CY_MAXZ, 0, 0);
+
+	input_mt_init_slots(input_device, CY_MAX_ID);
+
+	for (i = 0; i < CY_MAX_ID; i++) {
+		ts->slot_prev[i] = CY_UNUSED;
+		ts->slot_curr[i] = CY_UNUSED;
+	}
+
+	ret = input_register_device(input_device);
+	if (ret) {
+		dev_err(ts->dev, "%s: Error, failed to register input device: %d\n",
+			__func__, ret);
+		goto error_input_register_device;
+	}
+
+	goto no_error;
+
+error_input_register_device:
+	input_free_device(input_device);
+error_input_allocate_device:
+	if (ts->platform_data->exit)
+		ts->platform_data->exit();
+error_init:
+	kfree(ts);
+error_alloc_data:
+no_error:
+	return ts;
+}
+EXPORT_SYMBOL_GPL(cyttsp_core_init);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("Cypress TrueTouch(R) Standard touchscreen driver core");
+MODULE_AUTHOR("Cypress");
+
diff --git a/drivers/input/touchscreen/cyttsp/cyttsp_core.h b/drivers/input/touchscreen/cyttsp/cyttsp_core.h
new file mode 100644
index 0000000..e72fecf
--- /dev/null
+++ b/drivers/input/touchscreen/cyttsp/cyttsp_core.h
@@ -0,0 +1,56 @@
+/*
+ * Header file for:
+ * Cypress TrueTouch(TM) Standard Product (TTSP) touchscreen drivers.
+ * For use with Cypress Txx3xx parts.
+ * Supported parts include:
+ * CY8CTST341
+ * CY8CTMA340
+ *
+ * Copyright (C) 2009, 2010, 2011 Cypress Semiconductor, Inc.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2, and only version 2, as published by the
+ * Free Software Foundation.
+ *
+ * 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 program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+ *
+ * Contact Cypress Semiconductor at www.cypress.com <kev@cypress.com>
+ *
+ */
+
+
+#ifndef __CYTTSP_CORE_H__
+#define __CYTTSP_CORE_H__
+
+#include <linux/kernel.h>
+#include <linux/err.h>
+#include <linux/module.h>
+#include <linux/input/cyttsp.h>
+
+#define CY_NUM_RETRY                4 /* max number of retries for read ops */
+
+
+struct cyttsp_bus_ops {
+	s32 (*write)(void *handle, u8 addr, u8 length, const void *values);
+	s32 (*read)(void *handle, u8 addr, u8 length, void *values);
+	s32 (*ext)(void *handle, void *values);
+	struct device *dev;
+};
+
+void *cyttsp_core_init(struct cyttsp_bus_ops *bus_ops, struct device *dev, int irq);
+
+void cyttsp_core_release(void *handle);
+#ifdef CONFIG_PM
+int cyttsp_resume(void *handle);
+int cyttsp_suspend(void *handle);
+#endif
+
+#endif /* __CYTTSP_CORE_H__ */
diff --git a/include/linux/input/cyttsp.h b/include/linux/input/cyttsp.h
new file mode 100644
index 0000000..3907bfc
--- /dev/null
+++ b/include/linux/input/cyttsp.h
@@ -0,0 +1,68 @@
+/*
+ * Header file for:
+ * Cypress TrueTouch(TM) Standard Product (TTSP) touchscreen drivers.
+ * For use with Cypress Txx3xx parts.
+ * Supported parts include:
+ * CY8CTST341
+ * CY8CTMA340
+ *
+ * Copyright (C) 2009, 2010, 2011 Cypress Semiconductor, Inc.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2, and only version 2, as published by the
+ * Free Software Foundation.
+ *
+ * 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 program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+ *
+ * Contact Cypress Semiconductor at www.cypress.com (kev@cypress.com)
+ *
+ */
+#ifndef _CYTTSP_H_
+#define _CYTTSP_H_
+
+#define CY_SPI_NAME "cyttsp-spi"
+#define CY_I2C_NAME "cyttsp-i2c"
+/* Active Power state scanning/processing refresh interval */
+#define CY_ACT_INTRVL_DFLT 0x00 /* ms */
+/* touch timeout for the Active power */
+#define CY_TCH_TMOUT_DFLT 0xFF /* ms */
+/* Low Power state scanning/processing refresh interval */
+#define CY_LP_INTRVL_DFLT 0x0A /* ms */
+/* Active distance in pixels for a gesture to be reported */
+#define CY_ACT_DIST_DFLT 0xF8 /* pixels */
+
+enum cyttsp_powerstate {
+	CY_IDLE_STATE,
+	CY_ACTIVE_STATE,
+	CY_LOW_PWR_STATE,
+	CY_SLEEP_STATE,
+	CY_BL_STATE,
+	CY_INVALID_STATE	/* always last in the list */
+};
+
+struct cyttsp_platform_data {
+	u32 maxx;
+	u32 maxy;
+	bool use_hndshk;
+	bool use_sleep;
+	u8 act_dist;	/* Active distance */
+	u8 act_intrvl;  /* Active refresh interval; ms */
+	u8 tch_tmout;   /* Active touch timeout; ms */
+	u8 lp_intrvl;   /* Low power refresh interval; ms */
+	int (*wakeup)(void);
+	int (*init)(void);
+	void (*exit)(void);
+	char *name;
+	s16 irq_gpio;
+	u8 *bl_keys;
+};
+
+#endif /* _CYTTSP_H_ */
-- 
1.7.4.1

[PATCH V3 2/3] Input: cyttsp - add support for Cypress TTSP touchscreen I2C bus interface

[Javier Martinez Canillas]

The driver is composed of a core driver that process the data sent by
the contacts and a set of bus specific interface modules.

This patch add supports for the Cypress TTSP I2C bus interface.

The original author of the driver is Kevin McNeely <kev@cypress.com>

Signed-off-by: Javier Martinez Canillas <martinez.javier@gmail.com>
---
V2: Fix issues called out by Dmitry Torokhov
	- Extract the IRQ from the i2c client data and pass to cyttsp_core_init

V3: Fix issues called out by Henrik Rydberg and Mohan Pallaka
    - Remove bus type info since it is not used

 drivers/input/touchscreen/cyttsp/cyttsp_i2c.c |  187 +++++++++++++++++++++++++
 1 files changed, 187 insertions(+), 0 deletions(-)
 create mode 100644 drivers/input/touchscreen/cyttsp/cyttsp_i2c.c

diff --git a/drivers/input/touchscreen/cyttsp/cyttsp_i2c.c b/drivers/input/touchscreen/cyttsp/cyttsp_i2c.c
new file mode 100644
index 0000000..9c953d2
--- /dev/null
+++ b/drivers/input/touchscreen/cyttsp/cyttsp_i2c.c
@@ -0,0 +1,187 @@
+/*
+ * Source for:
+ * Cypress TrueTouch(TM) Standard Product (TTSP) I2C touchscreen driver.
+ * For use with Cypress Txx3xx parts.
+ * Supported parts include:
+ * CY8CTST341
+ * CY8CTMA340
+ *
+ * Copyright (C) 2009, 2010, 2011 Cypress Semiconductor, Inc.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2, and only version 2, as published by the
+ * Free Software Foundation.
+ *
+ * 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 program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+ *
+ * Contact Cypress Semiconductor at www.cypress.com <kev@cypress.com>
+ *
+ */
+
+#include "cyttsp_core.h"
+
+#include <linux/i2c.h>
+#include <linux/slab.h>
+
+#define CY_I2C_DATA_SIZE  128
+
+struct cyttsp_i2c {
+	struct cyttsp_bus_ops ops;
+	struct i2c_client *client;
+	void *ttsp_client;
+	u8 wr_buf[CY_I2C_DATA_SIZE];
+};
+
+static s32 ttsp_i2c_read_block_data(void *handle, u8 addr,
+	u8 length, void *values)
+{
+	struct cyttsp_i2c *ts = container_of(handle, struct cyttsp_i2c, ops);
+	int retval = 0;
+
+	retval = i2c_master_send(ts->client, &addr, 1);
+	if (retval < 0)
+		return retval;
+
+	retval = i2c_master_recv(ts->client, values, length);
+
+	return (retval < 0) ? retval : 0;
+}
+
+static s32 ttsp_i2c_write_block_data(void *handle, u8 addr,
+	u8 length, const void *values)
+{
+	struct cyttsp_i2c *ts = container_of(handle, struct cyttsp_i2c, ops);
+	int retval;
+
+	ts->wr_buf[0] = addr;
+	memcpy(&ts->wr_buf[1], values, length);
+
+	retval = i2c_master_send(ts->client, ts->wr_buf, length+1);
+
+	return (retval < 0) ? retval : 0;
+}
+
+static s32 ttsp_i2c_tch_ext(void *handle, void *values)
+{
+	struct cyttsp_i2c *ts = container_of(handle, struct cyttsp_i2c, ops);
+	int retval = 0;
+
+	/*
+	 * TODO: Add custom touch extension handling code here
+	 * set: retval < 0 for any returned system errors,
+	 *	retval = 0 if normal touch handling is required,
+	 *	retval > 0 if normal touch handling is *not* required
+	 */
+	if (!ts || !values)
+		retval = -EINVAL;
+
+	return retval;
+}
+
+static int __devinit cyttsp_i2c_probe(struct i2c_client *client,
+	const struct i2c_device_id *id)
+{
+	struct cyttsp_i2c *ts;
+
+	if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C))
+		return -EIO;
+
+	/* allocate and clear memory */
+	ts = kzalloc(sizeof(*ts), GFP_KERNEL);
+	if (!ts) {
+		dev_dbg(&client->dev, "%s: Error, kzalloc.\n", __func__);
+		return -ENOMEM;
+	}
+
+	/* register driver_data */
+	ts->client = client;
+	i2c_set_clientdata(client, ts);
+	ts->ops.write = ttsp_i2c_write_block_data;
+	ts->ops.read = ttsp_i2c_read_block_data;
+	ts->ops.ext = ttsp_i2c_tch_ext;
+	ts->ops.dev = &client->dev;
+
+	ts->ttsp_client = cyttsp_core_init(&ts->ops, &client->dev, client->irq);
+	if (IS_ERR(ts->ttsp_client)) {
+		int retval = PTR_ERR(ts->ttsp_client);
+		kfree(ts);
+		return retval;
+	}
+
+	dev_dbg(ts->ops.dev, "%s: Registration complete\n", __func__);
+
+	return 0;
+}
+
+
+/* registered in driver struct */
+static int __devexit cyttsp_i2c_remove(struct i2c_client *client)
+{
+	struct cyttsp_i2c *ts;
+
+	ts = i2c_get_clientdata(client);
+	cyttsp_core_release(ts->ttsp_client);
+	kfree(ts);
+	return 0;
+}
+
+#ifdef CONFIG_PM
+static int cyttsp_i2c_suspend(struct i2c_client *client, pm_message_t message)
+{
+	struct cyttsp_i2c *ts = i2c_get_clientdata(client);
+
+	return cyttsp_suspend(ts->ttsp_client);
+}
+
+static int cyttsp_i2c_resume(struct i2c_client *client)
+{
+	struct cyttsp_i2c *ts = i2c_get_clientdata(client);
+
+	return cyttsp_resume(ts->ttsp_client);
+}
+#endif
+
+static const struct i2c_device_id cyttsp_i2c_id[] = {
+	{ CY_I2C_NAME, 0 },  { }
+};
+
+static struct i2c_driver cyttsp_i2c_driver = {
+	.driver = {
+		.name = CY_I2C_NAME,
+		.owner = THIS_MODULE,
+	},
+	.probe = cyttsp_i2c_probe,
+	.remove = __devexit_p(cyttsp_i2c_remove),
+	.id_table = cyttsp_i2c_id,
+#ifdef CONFIG_PM
+	.suspend = cyttsp_i2c_suspend,
+	.resume = cyttsp_i2c_resume,
+#endif
+};
+
+static int __init cyttsp_i2c_init(void)
+{
+	return i2c_add_driver(&cyttsp_i2c_driver);
+}
+
+static void __exit cyttsp_i2c_exit(void)
+{
+	return i2c_del_driver(&cyttsp_i2c_driver);
+}
+
+module_init(cyttsp_i2c_init);
+module_exit(cyttsp_i2c_exit);
+
+MODULE_ALIAS("i2c:cyttsp");
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("Cypress TrueTouch(R) Standard Product (TTSP) I2C driver");
+MODULE_AUTHOR("Cypress");
+MODULE_DEVICE_TABLE(i2c, cyttsp_i2c_id);
-- 
1.7.4.1

[PATCH V3 3/3] Input: cyttsp - add support for Cypress TTSP touchscreen SPI bus interface

[Javier Martinez Canillas]

The driver is composed of a core driver that process the data sent by
the contacts and a set of bus specific interface modules.

This patch add supports for the Cypress TTSP SPI bus interface.

The original author of the driver is Kevin McNeely <kev@cypress.com>

Signed-off-by: Javier Martinez Canillas <martinez.javier@gmail.com>
---
V2: Fix issues called out by Dmitry Torokhov
	- Extract the IRQ from the spi client data and pass to cyttsp_core_init
	- Remove the extra retries and limit the retries to the cyttsp_core.c
	  read/write block functions.
	- Cleanup cyttsp_spi_xfer(), check ACK in write operation and fix special
	  EIO case to show its meaning.

V3: Fix issues called out by Henrik Rydberg and Mohan Pallaka
    - Remove bus type info since it is not used

 drivers/input/touchscreen/cyttsp/cyttsp_spi.c |  292 +++++++++++++++++++++++++
 1 files changed, 292 insertions(+), 0 deletions(-)
 create mode 100644 drivers/input/touchscreen/cyttsp/cyttsp_spi.c

diff --git a/drivers/input/touchscreen/cyttsp/cyttsp_spi.c b/drivers/input/touchscreen/cyttsp/cyttsp_spi.c
new file mode 100644
index 0000000..5567859
--- /dev/null
+++ b/drivers/input/touchscreen/cyttsp/cyttsp_spi.c
@@ -0,0 +1,292 @@
+/*
+ * Source for:
+ * Cypress TrueTouch(TM) Standard Product (TTSP) SPI touchscreen driver.
+ * For use with Cypress Txx3xx parts.
+ * Supported parts include:
+ * CY8CTST341
+ * CY8CTMA340
+ *
+ * Copyright (C) 2009, 2010, 2011 Cypress Semiconductor, Inc.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2, and only version 2, as published by the
+ * Free Software Foundation.
+ *
+ * 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 program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+ *
+ * Contact Cypress Semiconductor at www.cypress.com <kev@cypress.com>
+ *
+ */
+
+#include "cyttsp_core.h"
+
+#include <linux/spi/spi.h>
+#include <linux/delay.h>
+
+#define CY_SPI_WR_OP      0x00 /* r/~w */
+#define CY_SPI_RD_OP      0x01
+#define CY_SPI_CMD_BYTES  4
+#define CY_SPI_SYNC_BYTE  2
+#define CY_SPI_SYNC_ACK1  0x62 /* from protocol v.2 */
+#define CY_SPI_SYNC_ACK2  0x9D /* from protocol v.2 */
+#define CY_SPI_DATA_SIZE  128
+#define CY_SPI_DATA_BUF_SIZE (CY_SPI_CMD_BYTES + CY_SPI_DATA_SIZE)
+#define CY_SPI_BITS_PER_WORD 8
+
+struct cyttsp_spi {
+	struct cyttsp_bus_ops bus_ops;
+	struct spi_device *spi_client;
+	void *ttsp_client;
+	u8 wr_buf[CY_SPI_DATA_BUF_SIZE];
+	u8 rd_buf[CY_SPI_DATA_BUF_SIZE];
+};
+
+static int cyttsp_spi_xfer(u8 op, struct cyttsp_spi *ts,
+			   u8 reg, u8 *buf, int length)
+{
+	struct spi_message msg;
+	struct spi_transfer xfer[2];
+	u8 *wr_buf = ts->wr_buf;
+	u8 *rd_buf = ts->rd_buf;
+	int retval;
+
+	if (length > CY_SPI_DATA_SIZE) {
+		dev_dbg(ts->bus_ops.dev,
+			"%s: length %d is too big.\n",
+			__func__, length);
+		return -EINVAL;
+	}
+
+	memset(wr_buf, 0, CY_SPI_DATA_BUF_SIZE);
+	memset(rd_buf, 0, CY_SPI_DATA_BUF_SIZE);
+
+	wr_buf[0] = 0x00; /* header byte 0 */
+	wr_buf[1] = 0xFF; /* header byte 1 */
+	wr_buf[2] = reg;  /* reg index */
+	wr_buf[3] = op;   /* r/~w */
+	if (op == CY_SPI_WR_OP)
+		memcpy(wr_buf + CY_SPI_CMD_BYTES, buf, length);
+
+	memset((void *)xfer, 0, sizeof(xfer));
+	spi_message_init(&msg);
+
+	/*
+	  We set both TX and RX buffers because Cypress TTSP
+	  requires full duplex operation.
+	*/
+	xfer[0].tx_buf = wr_buf;
+	xfer[0].rx_buf = rd_buf;
+	if (op == CY_SPI_WR_OP) {
+		xfer[0].len = length + CY_SPI_CMD_BYTES;
+		spi_message_add_tail(&xfer[0], &msg);
+	} else if (op == CY_SPI_RD_OP) {
+		xfer[0].len = CY_SPI_CMD_BYTES;
+		spi_message_add_tail(&xfer[0], &msg);
+
+		xfer[1].rx_buf = buf;
+		xfer[1].len = length;
+		spi_message_add_tail(&xfer[1], &msg);
+	}
+
+	retval = spi_sync(ts->spi_client, &msg);
+	if (retval < 0) {
+		dev_dbg(ts->bus_ops.dev,
+			"%s: spi_sync() error %d, len=%d, op=%d\n",
+			__func__, retval, xfer[1].len, op);
+
+		/*
+		 * do not return here since was a bad ACK sequence
+		 * let the following ACK check handle any errors and
+		 * allow silent retries
+		 */
+	}
+
+	if ((rd_buf[CY_SPI_SYNC_BYTE] == CY_SPI_SYNC_ACK1) &&
+	    (rd_buf[CY_SPI_SYNC_BYTE+1] == CY_SPI_SYNC_ACK2))
+		retval = 0;
+	else {
+		int i;
+		for (i = 0; i < (CY_SPI_CMD_BYTES); i++)
+			dev_dbg(ts->bus_ops.dev,
+				"%s: test rd_buf[%d]:0x%02x\n",
+				__func__, i, rd_buf[i]);
+		for (i = 0; i < (length); i++)
+			dev_dbg(ts->bus_ops.dev,
+				"%s: test buf[%d]:0x%02x\n",
+				__func__, i, buf[i]);
+
+		/* signal ACK error so silent retry */
+		retval = 1;
+	}
+
+	return retval;
+}
+
+static s32 ttsp_spi_read_block_data(void *handle, u8 addr,
+				    u8 length, void *data)
+{
+	struct cyttsp_spi *ts =
+		container_of(handle, struct cyttsp_spi, bus_ops);
+	int retval;
+
+	retval = cyttsp_spi_xfer(CY_SPI_RD_OP, ts, addr, data, length);
+	if (retval < 0)
+		pr_err("%s: ttsp_spi_read_block_data failed\n",
+		       __func__);
+
+	/*
+	 * Do not print the above error if the data sync bytes were not found.
+	 * This is a normal condition for the bootloader loader startup and need
+	 * to retry until data sync bytes are found.
+	 */
+	if (retval > 0)
+		retval = -EIO;  /* now signal fail; so retry can be done */
+
+	return retval;
+}
+
+static s32 ttsp_spi_write_block_data(void *handle, u8 addr,
+				     u8 length, const void *data)
+{
+	struct cyttsp_spi *ts =
+		container_of(handle, struct cyttsp_spi, bus_ops);
+	int retval;
+
+	retval = cyttsp_spi_xfer(CY_SPI_WR_OP, ts, addr, (void *)data, length);
+	if (retval < 0)
+		pr_err("%s: ttsp_spi_write_block_data failed\n",
+		       __func__);
+
+	/*
+	 * Do not print the above error if the data sync bytes were not found.
+	 * This is a normal condition for the bootloader loader startup and need
+	 * to retry until data sync bytes are found.
+	 */
+	if (retval > 0)
+		retval = -EIO;  /* now signal fail; so retry can be done */
+
+	return retval;
+}
+
+static s32 ttsp_spi_tch_ext(void *handle, void *values)
+{
+	struct cyttsp_spi *ts =
+		container_of(handle, struct cyttsp_spi, bus_ops);
+	int retval = 0;
+
+	/*
+	 * TODO: Add custom touch extension handling code here
+	 * set: retval < 0 for any returned system errors,
+	 *	retval = 0 if normal touch handling is required,
+	 *	retval > 0 if normal touch handling is *not* required
+	 */
+
+	if (!ts || !values)
+		retval = -EINVAL;
+
+	return retval;
+}
+
+static int __devinit cyttsp_spi_probe(struct spi_device *spi)
+{
+	struct cyttsp_spi *ts;
+	int retval;
+
+	/* Set up SPI*/
+	spi->bits_per_word = CY_SPI_BITS_PER_WORD;
+	spi->mode = SPI_MODE_0;
+	retval = spi_setup(spi);
+	if (retval < 0) {
+		dev_dbg(&spi->dev, "%s: SPI setup error %d\n",
+			__func__, retval);
+		return retval;
+	}
+
+	ts = kzalloc(sizeof(*ts), GFP_KERNEL);
+	if (!ts) {
+		dev_dbg(&spi->dev, "%s: Error, kzalloc\n", __func__);
+		return -ENOMEM;
+	}
+
+	ts->spi_client = spi;
+	dev_set_drvdata(&spi->dev, ts);
+	ts->bus_ops.write = ttsp_spi_write_block_data;
+	ts->bus_ops.read = ttsp_spi_read_block_data;
+	ts->bus_ops.ext = ttsp_spi_tch_ext;
+	ts->bus_ops.dev = &spi->dev;
+
+	ts->ttsp_client = cyttsp_core_init(&ts->bus_ops, &spi->dev, spi->irq);
+	if (IS_ERR(ts->ttsp_client)) {
+		int retval = PTR_ERR(ts->ttsp_client);
+		kfree(ts);
+		return retval;
+	}
+
+	dev_dbg(ts->bus_ops.dev, "%s: Registration complete\n", __func__);
+
+	return 0;
+}
+
+static int __devexit cyttsp_spi_remove(struct spi_device *spi)
+{
+	struct cyttsp_spi *ts = dev_get_drvdata(&spi->dev);
+
+	cyttsp_core_release(ts->ttsp_client);
+	kfree(ts);
+	return 0;
+}
+
+#ifdef CONFIG_PM
+static int cyttsp_spi_suspend(struct spi_device *spi, pm_message_t message)
+{
+	struct cyttsp_spi *ts = dev_get_drvdata(&spi->dev);
+
+	return cyttsp_suspend(ts->ttsp_client);
+}
+
+static int cyttsp_spi_resume(struct spi_device *spi)
+{
+	struct cyttsp_spi *ts = dev_get_drvdata(&spi->dev);
+
+	return cyttsp_resume(ts->ttsp_client);
+}
+#endif
+
+static struct spi_driver cyttsp_spi_driver = {
+	.driver = {
+		.name = CY_SPI_NAME,
+		.owner = THIS_MODULE,
+	},
+	.probe = cyttsp_spi_probe,
+	.remove = __devexit_p(cyttsp_spi_remove),
+#ifdef CONFIG_PM
+	.suspend = cyttsp_spi_suspend,
+	.resume = cyttsp_spi_resume,
+#endif
+};
+
+static int __init cyttsp_spi_init(void)
+{
+	return spi_register_driver(&cyttsp_spi_driver);
+}
+module_init(cyttsp_spi_init);
+
+static void __exit cyttsp_spi_exit(void)
+{
+	spi_unregister_driver(&cyttsp_spi_driver);
+}
+module_exit(cyttsp_spi_exit);
+
+MODULE_ALIAS("spi:cyttsp");
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("Cypress TrueTouch(R) Standard Product (TTSP) SPI driver");
+MODULE_AUTHOR("Cypress");
+
-- 
1.7.4.1

Re: [PATCH V3 0/3] Input: Cypress TTSP device driver

[Javier Martinez Canillas]

On Sun, Sep 18, 2011 at 4:01 AM, Javier Martinez Canillas
<martinez.javier@gmail.com> wrote:
> Cypress TrueTouch(tm) Standard Product controllers are found in
> a wide range of embedded devices. This patch-set adds support for a
> variety of TTSP controllers.
>
> The original author of the driver is Kevin McNeely <kev@cypress.com>
>
> Since the hardware is capable of tracking identifiable contacts and the
> original driver used multi-touch protocol type A (stateless), I've added
> multi-touch protocol type B (stateful) support.
>
> The driver is composed of a core driver that process the data sent by
> the contacts (fingers) and two bus specific interface modules (I2C and SPI).
>
> This is a third version of the driver that fixes issues called out by
> Dmitry Torokhov, Henrik Rydberg and Mohan Pallaka
>
> The patchset is composed of the patches:
>
> [PATCH V3 1/3] Input: cyttsp - Cypress TTSP capacitive multi-touch screen support
> [PATCH V3 2/3] Input: cyttsp - add support for Cypress TTSP touchscreen I2C bus interface
> [PATCH V3 3/3] Input: cyttsp - add support for Cypress TTSP touchscreen SPI bus interface
>

Hello,

Any feedback for this patch-set?

Best regards,

-- 
Javier Martínez Canillas
(+34) 682 39 81 69
Barcelona, Spain

Re: [PATCH V3 0/3] Input: Cypress TTSP device driver

[Javier Martinez Canillas]

On Sun, Sep 18, 2011 at 4:01 AM, Javier Martinez Canillas
<martinez.javier@gmail.com> wrote:
> Cypress TrueTouch(tm) Standard Product controllers are found in
> a wide range of embedded devices. This patch-set adds support for a
> variety of TTSP controllers.
>
> The original author of the driver is Kevin McNeely <kev@cypress.com>
>
> Since the hardware is capable of tracking identifiable contacts and the
> original driver used multi-touch protocol type A (stateless), I've added
> multi-touch protocol type B (stateful) support.
>
> The driver is composed of a core driver that process the data sent by
> the contacts (fingers) and two bus specific interface modules (I2C and SPI).
>
> This is a third version of the driver that fixes issues called out by
> Dmitry Torokhov, Henrik Rydberg and Mohan Pallaka
>
> The patchset is composed of the patches:
>
> [PATCH V3 1/3] Input: cyttsp - Cypress TTSP capacitive multi-touch screen support
> [PATCH V3 2/3] Input: cyttsp - add support for Cypress TTSP touchscreen I2C bus interface
> [PATCH V3 3/3] Input: cyttsp - add support for Cypress TTSP touchscreen SPI bus interface
>

Hello,

Any feedback for this patch-set?

Best regards,

-- 
Javier Martínez Canillas
(+34) 682 39 81 69
Barcelona, Spain
--
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

Re: [PATCH V3 1/3] Input: cyttsp - Cypress TTSP capacitive multi-touch screen support

[Henrik Rydberg]

Hi Javier,

> Cypress TrueTouch(tm) Standard Product controllers are found in
> a wide range of embedded devices. This driver add support for a
> variety of TTSP controllers.

please find some comments below.

> diff --git a/drivers/input/touchscreen/cyttsp/cyttsp_core.c b/drivers/input/touchscreen/cyttsp/cyttsp_core.c
> new file mode 100644
> index 0000000..8e7bd66
> --- /dev/null
> +++ b/drivers/input/touchscreen/cyttsp/cyttsp_core.c
> @@ -0,0 +1,853 @@
> +/*
> + * Core Source for:
> + * Cypress TrueTouch(TM) Standard Product (TTSP) touchscreen drivers.
> + * For use with Cypress Txx3xx parts.
> + * Supported parts include:
> + * CY8CTST341
> + * CY8CTMA340
> + *
> + * Copyright (C) 2009, 2010, 2011 Cypress Semiconductor, Inc.
> + * Copyright (C) 2011 Javier Martinez Canillas <martinez.javier@gmail.com>
> + *
> + * Added multi-touch protocol type B support by Javier Martinez Canillas
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * version 2, and only version 2, as published by the
> + * Free Software Foundation.
> + *
> + * 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 program; if not, write to the Free Software Foundation, Inc.,
> + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
> + *
> + * Contact Cypress Semiconductor at www.cypress.com <kev@cypress.com>
> + *
> + */
> +
> +#include "cyttsp_core.h"
> +
> +#include <linux/delay.h>
> +#include <linux/input.h>
> +#include <linux/input/mt.h>
> +#include <linux/gpio.h>
> +#include <linux/interrupt.h>
> +#include <linux/slab.h>
> +
> +/* Bootloader number of command keys */
> +#define CY_NUM_BL_KEYS    8
> +
> +/* helpers */
> +#define GET_NUM_TOUCHES(x)          ((x) & 0x0F)
> +#define IS_LARGE_AREA(x)            (((x) & 0x10) >> 4)
> +#define IS_BAD_PKT(x)               ((x) & 0x20)
> +#define IS_VALID_APP(x)             ((x) & 0x01)
> +#define IS_OPERATIONAL_ERR(x)       ((x) & 0x3F)
> +#define GET_HSTMODE(reg)            ((reg & 0x70) >> 4)
> +#define GET_BOOTLOADERMODE(reg)     ((reg & 0x10) >> 4)
> +
> +#define CY_REG_BASE                 0x00
> +#define CY_REG_ACT_DIST             0x1E
> +#define CY_REG_ACT_INTRVL           0x1D
> +#define CY_REG_TCH_TMOUT            (CY_REG_ACT_INTRVL+1)
> +#define CY_REG_LP_INTRVL            (CY_REG_TCH_TMOUT+1)
> +#define CY_MAXZ                     255
> +#define CY_DELAY_DFLT               20 /* ms */
> +#define CY_DELAY_MAX                (500/CY_DELAY_DFLT) /* half second */
> +#define CY_ACT_DIST_DFLT            0xF8
> +#define CY_HNDSHK_BIT               0x80
> +/* device mode bits */
> +#define CY_OPERATE_MODE             0x00
> +#define CY_SYSINFO_MODE             0x10
> +/* power mode select bits */
> +#define CY_SOFT_RESET_MODE          0x01 /* return to Bootloader mode */
> +#define CY_DEEP_SLEEP_MODE          0x02
> +#define CY_LOW_POWER_MODE           0x04
> +
> +/* Slots management */
> +#define CY_MAX_FINGER               4
> +#define CY_UNUSED                   0
> +#define CY_USED                     1

These two look like bool, please use as bool instead.

> +#define CY_MAX_ID                   15

Why not 16 here?

> +struct cyttsp_tch {
> +	__be16 x, y;
> +	u8 z;
> +} __packed;
> +
> +/* TrueTouch Standard Product Gen3 interface definition */
> +struct cyttsp_xydata {
> +	u8 hst_mode;
> +	u8 tt_mode;
> +	u8 tt_stat;
> +	struct cyttsp_tch tch1;
> +	u8 touch12_id;
> +	struct cyttsp_tch tch2;
> +	u8 gest_cnt;
> +	u8 gest_id;
> +	struct cyttsp_tch tch3;
> +	u8 touch34_id;
> +	struct cyttsp_tch tch4;
> +	u8 tt_undef[3];
> +	u8 act_dist;
> +	u8 tt_reserved;
> +} __packed;

Too bad the touches are not an array here...

> +
> +/* TTSP System Information interface definition */
> +struct cyttsp_sysinfo_data {
> +	u8 hst_mode;
> +	u8 mfg_cmd;
> +	u8 mfg_stat;
> +	u8 cid[3];
> +	u8 tt_undef1;
> +	u8 uid[8];
> +	u8 bl_verh;
> +	u8 bl_verl;
> +	u8 tts_verh;
> +	u8 tts_verl;
> +	u8 app_idh;
> +	u8 app_idl;
> +	u8 app_verh;
> +	u8 app_verl;
> +	u8 tt_undef[5];
> +	u8 scn_typ;
> +	u8 act_intrvl;
> +	u8 tch_tmout;
> +	u8 lp_intrvl;
> +};
> +
> +/* TTSP Bootloader Register Map interface definition */
> +#define CY_BL_CHKSUM_OK 0x01
> +struct cyttsp_bootloader_data {
> +	u8 bl_file;
> +	u8 bl_status;
> +	u8 bl_error;
> +	u8 blver_hi;
> +	u8 blver_lo;
> +	u8 bld_blver_hi;
> +	u8 bld_blver_lo;
> +	u8 ttspver_hi;
> +	u8 ttspver_lo;
> +	u8 appid_hi;
> +	u8 appid_lo;
> +	u8 appver_hi;
> +	u8 appver_lo;
> +	u8 cid_0;
> +	u8 cid_1;
> +	u8 cid_2;
> +};
> +
> +struct cyttsp {
> +	struct device *dev;
> +	int irq;
> +	struct input_dev *input;
> +	char phys[32];
> +	const struct cyttsp_platform_data *platform_data;
> +	struct cyttsp_bus_ops *bus_ops;
> +	struct cyttsp_bootloader_data bl_data;
> +	struct cyttsp_sysinfo_data sysinfo_data;
> +	struct completion bl_ready;
> +	enum cyttsp_powerstate power_state;
> +	int slot_curr[CY_MAX_ID];
> +	int slot_prev[CY_MAX_ID];

These two are not needed; the input core will take care of duplicates,
so slot_prev can be removed. Also, the set of current slots can be
tracked using a temporary bitmask, so slot_curr can be removed as well.

<snip>

> +static void cyttsp_report_slot(struct input_dev *dev, int slot,
> +			       int x, int y, int z)
> +{
> +	input_mt_slot(dev, slot);
> +	input_mt_report_slot_state(dev, MT_TOOL_FINGER, true);
> +	input_report_abs(dev, ABS_MT_POSITION_X, x);
> +	input_report_abs(dev, ABS_MT_POSITION_Y, y);
> +	input_report_abs(dev, ABS_MT_TOUCH_MAJOR, z);
> +}
> +
> +static void cyttsp_report_slot_empty(struct input_dev *dev, int slot)
> +{
> +	input_mt_slot(dev, slot);
> +	input_mt_report_slot_state(dev, MT_TOOL_FINGER, false);
> +}
> +
> +static void cyttsp_extract_track_ids(struct cyttsp_xydata *xy_data, int *ids)
> +{
> +	ids[0] = xy_data->touch12_id >> 4;
> +	ids[1] = xy_data->touch12_id & 0xF;
> +	ids[2] = xy_data->touch34_id >> 4;
> +	ids[3] = xy_data->touch34_id & 0xF;
> +}
> +
> +static void cyttsp_get_tch(struct cyttsp_xydata *xy_data, int idx,
> +			   struct cyttsp_tch **tch)
> +{
> +	switch (idx) {
> +	case 0:
> +		*tch = &xy_data->tch1;
> +		break;
> +	case 1:
> +		*tch = &xy_data->tch2;
> +		break;
> +	case 2:
> +		*tch = &xy_data->tch3;
> +		break;
> +	case 3:
> +		*tch = &xy_data->tch4;
> +		break;
> +	}
> +}

How about returning a const struct cyttsp_tch* here instead.

> +static int cyttsp_handle_tchdata(struct cyttsp *ts)
> +{
> +	struct cyttsp_xydata xy_data;
> +	u8 num_cur_tch;
> +	int i;
> +	int ids[4];
> +	struct cyttsp_tch *tch = NULL;
> +	int x, y, z;
> +
> +	/* Get touch data from CYTTSP device */
> +	if (ttsp_read_block_data(ts,
> +		CY_REG_BASE, sizeof(struct cyttsp_xydata), &xy_data))
> +		return 0;
> +
> +	/* touch extension handling */
> +	if (ttsp_tch_ext(ts, &xy_data))
> +		return 0;
> +
> +	/* provide flow control handshake */
> +	if (ts->platform_data->use_hndshk)
> +		if (cyttsp_hndshk(ts, xy_data.hst_mode))
> +			return 0;
> +
> +	/* determine number of currently active touches */
> +	num_cur_tch = GET_NUM_TOUCHES(xy_data.tt_stat);
> +
> +	/* check for any error conditions */
> +	if (ts->power_state == CY_IDLE_STATE)
> +		return 0;
> +	else if (GET_BOOTLOADERMODE(xy_data.tt_mode)) {
> +		return -1;
> +	} else if (IS_LARGE_AREA(xy_data.tt_stat) == 1) {
> +		/* terminate all active tracks */
> +		num_cur_tch = 0;
> +		dev_dbg(ts->dev, "%s: Large area detected\n", __func__);
> +	} else if (num_cur_tch > CY_MAX_FINGER) {
> +		/* terminate all active tracks */
> +		num_cur_tch = 0;
> +		dev_dbg(ts->dev, "%s: Num touch error detected\n", __func__);
> +	} else if (IS_BAD_PKT(xy_data.tt_mode)) {
> +		/* terminate all active tracks */
> +		num_cur_tch = 0;
> +		dev_dbg(ts->dev, "%s: Invalid buffer detected\n", __func__);
> +	}
> +
> +	cyttsp_extract_track_ids(&xy_data, ids);
> +
> +	for (i = 0; i < num_cur_tch; i++) {
> +		ts->slot_curr[ids[i] - 1] = CY_USED;

Why the -1 here? Since the values are provably between 0 and 15, there
is no need to change them further.

Also, the above line could be replaced by "used |= 1 << ids[i]", for instance.

> +
> +		cyttsp_get_tch(&xy_data, i, &tch);
> +
> +		x = be16_to_cpu(tch->x);
> +		y = be16_to_cpu(tch->y);
> +		z = tch->z;
> +
> +		cyttsp_report_slot(ts->input, ids[i] - 1, x, y, z);

Ditto, -1.

> +	}
> +
> +	for (i = 0; i < CY_MAX_ID; i++) {
> +		if (ts->slot_prev[i] == CY_USED &&
> +		    ts->slot_curr[i] == CY_UNUSED)
> +			cyttsp_report_slot_empty(ts->input, i);
> +		ts->slot_prev[i] = ts->slot_curr[i];
> +		ts->slot_curr[i] = CY_UNUSED;
> +	}

Input core handles duplicate calls, so the above could be simplified.

> +
> +	input_sync(ts->input);
> +
> +	return 0;
> +}

<snip>

> +void *cyttsp_core_init(struct cyttsp_bus_ops *bus_ops, struct device *dev, int irq)
> +{
> +	struct input_dev *input_device;
> +	int i;
> +	int ret;
> +
> +	struct cyttsp *ts = kzalloc(sizeof(*ts), GFP_KERNEL);
> +
> +	if (!ts) {
> +		pr_err("%s: Error, kzalloc\n", __func__);
> +		goto error_alloc_data;
> +	}
> +
> +	if (dev == NULL || bus_ops == NULL) {
> +		kfree(ts);
> +		goto error_alloc_data;
> +	}
> +
> +	ts->dev = dev;
> +	ts->platform_data = dev->platform_data;
> +	ts->bus_ops = bus_ops;
> +	init_completion(&ts->bl_ready);
> +
> +	if (ts->platform_data->init) {
> +		if (ts->platform_data->init()) {
> +			dev_dbg(ts->dev, "%s: Error, platform init failed!\n",
> +				__func__);
> +			goto error_init;
> +		}
> +	}
> +
> +	ts->irq = irq;
> +	if (ts->irq <= 0) {
> +		dev_dbg(ts->dev, "%s: Error, failed to allocate irq\n",
> +			__func__);
> +			goto error_init;
> +	}
> +
> +	/* Create the input device and register it. */
> +	input_device = input_allocate_device();
> +	if (!input_device) {
> +		dev_dbg(ts->dev, "%s: Error, failed to allocate input device\n",
> +			__func__);
> +		goto error_input_allocate_device;
> +	}
> +
> +	ts->input = input_device;
> +	input_device->name = ts->platform_data->name;
> +	snprintf(ts->phys, sizeof(ts->phys), "%s", dev_name(dev));
> +	input_device->phys = ts->phys;
> +	input_device->dev.parent = ts->dev;
> +	input_device->open = cyttsp_open;
> +	input_device->close = cyttsp_close;
> +	input_set_drvdata(input_device, ts);
> +
> +	__set_bit(EV_SYN, input_device->evbit);
> +	__set_bit(EV_KEY, input_device->evbit);
> +	__set_bit(EV_ABS, input_device->evbit);
> +
> +	input_set_abs_params(input_device, ABS_MT_POSITION_X,
> +			     0, ts->platform_data->maxx, 0, 0);
> +	input_set_abs_params(input_device, ABS_MT_POSITION_Y,
> +			     0, ts->platform_data->maxy, 0, 0);
> +	input_set_abs_params(input_device, ABS_MT_TOUCH_MAJOR,
> +			     0, CY_MAXZ, 0, 0);
> +
> +	input_mt_init_slots(input_device, CY_MAX_ID);
> +
> +	for (i = 0; i < CY_MAX_ID; i++) {
> +		ts->slot_prev[i] = CY_UNUSED;
> +		ts->slot_curr[i] = CY_UNUSED;
> +	}

Could be removed.

> +	ret = input_register_device(input_device);
> +	if (ret) {
> +		dev_err(ts->dev, "%s: Error, failed to register input device: %d\n",
> +			__func__, ret);
> +		goto error_input_register_device;
> +	}
> +
> +	goto no_error;
> +
> +error_input_register_device:
> +	input_free_device(input_device);
> +error_input_allocate_device:
> +	if (ts->platform_data->exit)
> +		ts->platform_data->exit();
> +error_init:
> +	kfree(ts);
> +error_alloc_data:
> +no_error:
> +	return ts;
> +}

Thanks,
Henrik

Re: [PATCH V3 1/3] Input: cyttsp - Cypress TTSP capacitive multi-touch screen support

[Javier Martinez Canillas]

On Tue, Sep 27, 2011 at 1:52 PM, Henrik Rydberg <rydberg@euromail.se> wrote:
> Hi Javier,
>
>> Cypress TrueTouch(tm) Standard Product controllers are found in
>> a wide range of embedded devices. This driver add support for a
>> variety of TTSP controllers.
>
> please find some comments below.
>

Hello Henrik,

Thanks a lot for the review

>> +/* Slots management */
>> +#define CY_MAX_FINGER               4
>> +#define CY_UNUSED                   0
>> +#define CY_USED                     1
>
> These two look like bool, please use as bool instead.
>
>> +#define CY_MAX_ID                   15
>
> Why not 16 here?
>

Because as far as I know there are only 14 possible tracking ids
(1-14), not 16. 15 and 0 are not valid track ids.

When a contact lifts off the hardware reports a magic number (15) for
it. Also, the sequence always start with 1 which is the track id that
gets the first contact reported by the touch.

>> +
>> +/* TrueTouch Standard Product Gen3 interface definition */
>> +struct cyttsp_xydata {
>> +     u8 hst_mode;
>> +     u8 tt_mode;
>> +     u8 tt_stat;
>> +     struct cyttsp_tch tch1;
>> +     u8 touch12_id;
>> +     struct cyttsp_tch tch2;
>> +     u8 gest_cnt;
>> +     u8 gest_id;
>> +     struct cyttsp_tch tch3;
>> +     u8 touch34_id;
>> +     struct cyttsp_tch tch4;
>> +     u8 tt_undef[3];
>> +     u8 act_dist;
>> +     u8 tt_reserved;
>> +} __packed;
>
> Too bad the touches are not an array here...
>

Yes, the problem is that the offset between touches is not uniform.
For tch1 and tch2 is only 8 bits but the offset between tch2 and tch3
is 16 (u8 * 2) for example.

>> +     int slot_curr[CY_MAX_ID];
>> +     int slot_prev[CY_MAX_ID];
>
> These two are not needed; the input core will take care of duplicates,
> so slot_prev can be removed. Also, the set of current slots can be
> tracked using a temporary bitmask, so slot_curr can be removed as well.

Perfect, I'll remove both. I didn't know that the input core took care
of duplicates, sorry for not getting this from the docs.

>> +
>> +static void cyttsp_get_tch(struct cyttsp_xydata *xy_data, int idx,
>> +                        struct cyttsp_tch **tch)
>> +{
>> +     switch (idx) {
>> +     case 0:
>> +             *tch = &xy_data->tch1;
>> +             break;
>> +     case 1:
>> +             *tch = &xy_data->tch2;
>> +             break;
>> +     case 2:
>> +             *tch = &xy_data->tch3;
>> +             break;
>> +     case 3:
>> +             *tch = &xy_data->tch4;
>> +             break;
>> +     }
>> +}
>
> How about returning a const struct cyttsp_tch* here instead.
>

Ok, will return a struct pointer instead.

>> +
>> +     cyttsp_extract_track_ids(&xy_data, ids);
>> +
>> +     for (i = 0; i < num_cur_tch; i++) {
>> +             ts->slot_curr[ids[i] - 1] = CY_USED;
>
> Why the -1 here? Since the values are provably between 0 and 15, there
> is no need to change them further.

As I told you before is a HW thing. I looked at the track ids reported
by the touchscreen and their values are always between 1 and 14. So I
did the -1 to avoid having and unused element in the array. (I don't
have the HW datasheet so maybe I'm wrong with this)

Kevin?

>
> Also, the above line could be replaced by "used |= 1 << ids[i]", for instance.
>
>> +
>> +             cyttsp_get_tch(&xy_data, i, &tch);
>> +
>> +             x = be16_to_cpu(tch->x);
>> +             y = be16_to_cpu(tch->y);
>> +             z = tch->z;
>> +
>> +             cyttsp_report_slot(ts->input, ids[i] - 1, x, y, z);
>
> Ditto, -1.
>
>> +     }
>> +
>> +     for (i = 0; i < CY_MAX_ID; i++) {
>> +             if (ts->slot_prev[i] == CY_USED &&
>> +                 ts->slot_curr[i] == CY_UNUSED)
>> +                     cyttsp_report_slot_empty(ts->input, i);
>> +             ts->slot_prev[i] = ts->slot_curr[i];
>> +             ts->slot_curr[i] = CY_UNUSED;
>> +     }
>
> Input core handles duplicate calls, so the above could be simplified.
>

Perfect, will clean this code.

>> +
>> +     for (i = 0; i < CY_MAX_ID; i++) {
>> +             ts->slot_prev[i] = CY_UNUSED;
>> +             ts->slot_curr[i] = CY_UNUSED;
>> +     }
>
> Could be removed.
>

Yes, I'll do that.

>
> Thanks,
> Henrik
>

Thanks for your comments, I will work on these issues and resubmit a
new version of the patch-set.

Best regards,

-- 
Javier Martínez Canillas
(+34) 682 39 81 69
Barcelona, Spain

RE: [PATCH V3 1/3] Input: cyttsp - Cypress TTSP capacitive multi-touch screen support

[Kevin McNeely]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 5499 bytes --]

Hi Javier,

Yes, the hardware reports touches in the range 1 to 14.

Best regards,
Kevin


> -----Original Message-----
> From: Javier Martinez Canillas [mailto:martinez.javier@gmail.com]
> Sent: Wednesday, September 28, 2011 4:23 PM
> To: Henrik Rydberg
> Cc: Dmitry Torokhov; Greg Kroah-Hartman; linux-input@vger.kernel.org;
> linux-kernel@vger.kernel.org; Kevin McNeely
> Subject: Re: [PATCH V3 1/3] Input: cyttsp - Cypress TTSP capacitive multi-
> touch screen support
>
> On Tue, Sep 27, 2011 at 1:52 PM, Henrik Rydberg <rydberg@euromail.se>
> wrote:
> > Hi Javier,
> >
> >> Cypress TrueTouch(tm) Standard Product controllers are found in a
> >> wide range of embedded devices. This driver add support for a variety
> >> of TTSP controllers.
> >
> > please find some comments below.
> >
>
> Hello Henrik,
>
> Thanks a lot for the review
>
> >> +/* Slots management */
> >> +#define CY_MAX_FINGER               4 #define CY_UNUSED
> >> +0 #define CY_USED                     1
> >
> > These two look like bool, please use as bool instead.
> >
> >> +#define CY_MAX_ID                   15
> >
> > Why not 16 here?
> >
>
> Because as far as I know there are only 14 possible tracking ids (1-14), not 16.
> 15 and 0 are not valid track ids.
>
> When a contact lifts off the hardware reports a magic number (15) for it.
> Also, the sequence always start with 1 which is the track id that gets the first
> contact reported by the touch.
>
> >> +
> >> +/* TrueTouch Standard Product Gen3 interface definition */ struct
> >> +cyttsp_xydata {
> >> +     u8 hst_mode;
> >> +     u8 tt_mode;
> >> +     u8 tt_stat;
> >> +     struct cyttsp_tch tch1;
> >> +     u8 touch12_id;
> >> +     struct cyttsp_tch tch2;
> >> +     u8 gest_cnt;
> >> +     u8 gest_id;
> >> +     struct cyttsp_tch tch3;
> >> +     u8 touch34_id;
> >> +     struct cyttsp_tch tch4;
> >> +     u8 tt_undef[3];
> >> +     u8 act_dist;
> >> +     u8 tt_reserved;
> >> +} __packed;
> >
> > Too bad the touches are not an array here...
> >
>
> Yes, the problem is that the offset between touches is not uniform.
> For tch1 and tch2 is only 8 bits but the offset between tch2 and tch3 is 16 (u8
> * 2) for example.
>
> >> +     int slot_curr[CY_MAX_ID];
> >> +     int slot_prev[CY_MAX_ID];
> >
> > These two are not needed; the input core will take care of duplicates,
> > so slot_prev can be removed. Also, the set of current slots can be
> > tracked using a temporary bitmask, so slot_curr can be removed as well.
>
> Perfect, I'll remove both. I didn't know that the input core took care of
> duplicates, sorry for not getting this from the docs.
>
> >> +
> >> +static void cyttsp_get_tch(struct cyttsp_xydata *xy_data, int idx,
> >> +                        struct cyttsp_tch **tch) {
> >> +     switch (idx) {
> >> +     case 0:
> >> +             *tch = &xy_data->tch1;
> >> +             break;
> >> +     case 1:
> >> +             *tch = &xy_data->tch2;
> >> +             break;
> >> +     case 2:
> >> +             *tch = &xy_data->tch3;
> >> +             break;
> >> +     case 3:
> >> +             *tch = &xy_data->tch4;
> >> +             break;
> >> +     }
> >> +}
> >
> > How about returning a const struct cyttsp_tch* here instead.
> >
>
> Ok, will return a struct pointer instead.
>
> >> +
> >> +     cyttsp_extract_track_ids(&xy_data, ids);
> >> +
> >> +     for (i = 0; i < num_cur_tch; i++) {
> >> +             ts->slot_curr[ids[i] - 1] = CY_USED;
> >
> > Why the -1 here? Since the values are provably between 0 and 15, there
> > is no need to change them further.
>
> As I told you before is a HW thing. I looked at the track ids reported by the
> touchscreen and their values are always between 1 and 14. So I did the -1 to
> avoid having and unused element in the array. (I don't have the HW
> datasheet so maybe I'm wrong with this)
>
> Kevin?
>
> >
> > Also, the above line could be replaced by "used |= 1 << ids[i]", for instance.
> >
> >> +
> >> +             cyttsp_get_tch(&xy_data, i, &tch);
> >> +
> >> +             x = be16_to_cpu(tch->x);
> >> +             y = be16_to_cpu(tch->y);
> >> +             z = tch->z;
> >> +
> >> +             cyttsp_report_slot(ts->input, ids[i] - 1, x, y, z);
> >
> > Ditto, -1.
> >
> >> +     }
> >> +
> >> +     for (i = 0; i < CY_MAX_ID; i++) {
> >> +             if (ts->slot_prev[i] == CY_USED &&
> >> +                 ts->slot_curr[i] == CY_UNUSED)
> >> +                     cyttsp_report_slot_empty(ts->input, i);
> >> +             ts->slot_prev[i] = ts->slot_curr[i];
> >> +             ts->slot_curr[i] = CY_UNUSED;
> >> +     }
> >
> > Input core handles duplicate calls, so the above could be simplified.
> >
>
> Perfect, will clean this code.
>
> >> +
> >> +     for (i = 0; i < CY_MAX_ID; i++) {
> >> +             ts->slot_prev[i] = CY_UNUSED;
> >> +             ts->slot_curr[i] = CY_UNUSED;
> >> +     }
> >
> > Could be removed.
> >
>
> Yes, I'll do that.
>
> >
> > Thanks,
> > Henrik
> >
>
> Thanks for your comments, I will work on these issues and resubmit a new
> version of the patch-set.
>
> Best regards,
>
> --
> Javier Martínez Canillas
> (+34) 682 39 81 69
> Barcelona, Spain

This message and any attachments may contain Cypress (or its subsidiaries) confidential information. If it has been received in error, please advise the sender and immediately delete this message.
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

RE: [PATCH V3 1/3] Input: cyttsp - Cypress TTSP capacitive multi-touch screen support

[Kevin McNeely]

Hi Javier,

Yes, the hardware reports touches in the range 1 to 14.

Best regards,
Kevin


> -----Original Message-----
> From: Javier Martinez Canillas [mailto:martinez.javier@gmail.com]
> Sent: Wednesday, September 28, 2011 4:23 PM
> To: Henrik Rydberg
> Cc: Dmitry Torokhov; Greg Kroah-Hartman; linux-input@vger.kernel.org;
> linux-kernel@vger.kernel.org; Kevin McNeely
> Subject: Re: [PATCH V3 1/3] Input: cyttsp - Cypress TTSP capacitive multi-
> touch screen support
>
> On Tue, Sep 27, 2011 at 1:52 PM, Henrik Rydberg <rydberg@euromail.se>
> wrote:
> > Hi Javier,
> >
> >> Cypress TrueTouch(tm) Standard Product controllers are found in a
> >> wide range of embedded devices. This driver add support for a variety
> >> of TTSP controllers.
> >
> > please find some comments below.
> >
>
> Hello Henrik,
>
> Thanks a lot for the review
>
> >> +/* Slots management */
> >> +#define CY_MAX_FINGER               4 #define CY_UNUSED
> >> +0 #define CY_USED                     1
> >
> > These two look like bool, please use as bool instead.
> >
> >> +#define CY_MAX_ID                   15
> >
> > Why not 16 here?
> >
>
> Because as far as I know there are only 14 possible tracking ids (1-14), not 16.
> 15 and 0 are not valid track ids.
>
> When a contact lifts off the hardware reports a magic number (15) for it.
> Also, the sequence always start with 1 which is the track id that gets the first
> contact reported by the touch.
>
> >> +
> >> +/* TrueTouch Standard Product Gen3 interface definition */ struct
> >> +cyttsp_xydata {
> >> +     u8 hst_mode;
> >> +     u8 tt_mode;
> >> +     u8 tt_stat;
> >> +     struct cyttsp_tch tch1;
> >> +     u8 touch12_id;
> >> +     struct cyttsp_tch tch2;
> >> +     u8 gest_cnt;
> >> +     u8 gest_id;
> >> +     struct cyttsp_tch tch3;
> >> +     u8 touch34_id;
> >> +     struct cyttsp_tch tch4;
> >> +     u8 tt_undef[3];
> >> +     u8 act_dist;
> >> +     u8 tt_reserved;
> >> +} __packed;
> >
> > Too bad the touches are not an array here...
> >
>
> Yes, the problem is that the offset between touches is not uniform.
> For tch1 and tch2 is only 8 bits but the offset between tch2 and tch3 is 16 (u8
> * 2) for example.
>
> >> +     int slot_curr[CY_MAX_ID];
> >> +     int slot_prev[CY_MAX_ID];
> >
> > These two are not needed; the input core will take care of duplicates,
> > so slot_prev can be removed. Also, the set of current slots can be
> > tracked using a temporary bitmask, so slot_curr can be removed as well.
>
> Perfect, I'll remove both. I didn't know that the input core took care of
> duplicates, sorry for not getting this from the docs.
>
> >> +
> >> +static void cyttsp_get_tch(struct cyttsp_xydata *xy_data, int idx,
> >> +                        struct cyttsp_tch **tch) {
> >> +     switch (idx) {
> >> +     case 0:
> >> +             *tch = &xy_data->tch1;
> >> +             break;
> >> +     case 1:
> >> +             *tch = &xy_data->tch2;
> >> +             break;
> >> +     case 2:
> >> +             *tch = &xy_data->tch3;
> >> +             break;
> >> +     case 3:
> >> +             *tch = &xy_data->tch4;
> >> +             break;
> >> +     }
> >> +}
> >
> > How about returning a const struct cyttsp_tch* here instead.
> >
>
> Ok, will return a struct pointer instead.
>
> >> +
> >> +     cyttsp_extract_track_ids(&xy_data, ids);
> >> +
> >> +     for (i = 0; i < num_cur_tch; i++) {
> >> +             ts->slot_curr[ids[i] - 1] = CY_USED;
> >
> > Why the -1 here? Since the values are provably between 0 and 15, there
> > is no need to change them further.
>
> As I told you before is a HW thing. I looked at the track ids reported by the
> touchscreen and their values are always between 1 and 14. So I did the -1 to
> avoid having and unused element in the array. (I don't have the HW
> datasheet so maybe I'm wrong with this)
>
> Kevin?
>
> >
> > Also, the above line could be replaced by "used |= 1 << ids[i]", for instance.
> >
> >> +
> >> +             cyttsp_get_tch(&xy_data, i, &tch);
> >> +
> >> +             x = be16_to_cpu(tch->x);
> >> +             y = be16_to_cpu(tch->y);
> >> +             z = tch->z;
> >> +
> >> +             cyttsp_report_slot(ts->input, ids[i] - 1, x, y, z);
> >
> > Ditto, -1.
> >
> >> +     }
> >> +
> >> +     for (i = 0; i < CY_MAX_ID; i++) {
> >> +             if (ts->slot_prev[i] == CY_USED &&
> >> +                 ts->slot_curr[i] == CY_UNUSED)
> >> +                     cyttsp_report_slot_empty(ts->input, i);
> >> +             ts->slot_prev[i] = ts->slot_curr[i];
> >> +             ts->slot_curr[i] = CY_UNUSED;
> >> +     }
> >
> > Input core handles duplicate calls, so the above could be simplified.
> >
>
> Perfect, will clean this code.
>
> >> +
> >> +     for (i = 0; i < CY_MAX_ID; i++) {
> >> +             ts->slot_prev[i] = CY_UNUSED;
> >> +             ts->slot_curr[i] = CY_UNUSED;
> >> +     }
> >
> > Could be removed.
> >
>
> Yes, I'll do that.
>
> >
> > Thanks,
> > Henrik
> >
>
> Thanks for your comments, I will work on these issues and resubmit a new
> version of the patch-set.
>
> Best regards,
>
> --
> Javier Martínez Canillas
> (+34) 682 39 81 69
> Barcelona, Spain

This message and any attachments may contain Cypress (or its subsidiaries) confidential information. If it has been received in error, please advise the sender and immediately delete this message.

Re: [PATCH V3 1/3] Input: cyttsp - Cypress TTSP capacitive multi-touch screen support

[Javier Martinez Canillas]

On Tue, Sep 27, 2011 at 1:52 PM, Henrik Rydberg <rydberg@euromail.se> wrote:
> Hi Javier,
>
>> Cypress TrueTouch(tm) Standard Product controllers are found in
>> a wide range of embedded devices. This driver add support for a
>> variety of TTSP controllers.
>
> please find some comments below.
>

Hello Henrik,

I solved most of the issues you point me out, I have just a small
question about one of them:

>> +     }
>> +
>> +     for (i = 0; i < CY_MAX_ID; i++) {
>> +             if (ts->slot_prev[i] == CY_USED &&
>> +                 ts->slot_curr[i] == CY_UNUSED)
>> +                     cyttsp_report_slot_empty(ts->input, i);
>> +             ts->slot_prev[i] = ts->slot_curr[i];
>> +             ts->slot_curr[i] = CY_UNUSED;
>> +     }
>
> Input core handles duplicate calls, so the above could be simplified.
>

That means that the input MT is able to handle duplicates and I can
send the state of all the contacts and it will be able to identify the
deltas to send to user-space? Or do I have to keep the deltas and only
call input_mt_report_slot_state() when I detect that a finger that was
previously down was lifted?

To be more precise, the correct approach is:

	for (i = 0; i < CY_MAX_ID; i++)
		if (!(used & (1 << i)))
			cyttsp_report_slot_empty(ts->input, i);

or

	for (i = 0; i < CY_MAX_ID; i++)
		if (previous & (1 << i) &&
		    !(used & (1 << i)))
			cyttsp_report_slot_empty(ts->input, i);

where used is a bitmask to store each contact state for this event and
previous is a bitmask to store the state for the previous event.

I looked at input_mt_report_slot_state() and it seems that it only
checks whether the active parameter to send an ABS_MT_TRACKING_ID -1
input event, so I think that I should use the latter.

Thank you and best regards,

-- 
Javier Martínez Canillas
(+34) 682 39 81 69
Barcelona, Spain
--
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

Re: [PATCH V3 1/3] Input: cyttsp - Cypress TTSP capacitive multi-touch screen support

[Henrik Rydberg]

> To be more precise, the correct approach is:
> 
> 	for (i = 0; i < CY_MAX_ID; i++)
> 		if (!(used & (1 << i)))
> 			cyttsp_report_slot_empty(ts->input, i);
> 
> or
> 
> 	for (i = 0; i < CY_MAX_ID; i++)
> 		if (previous & (1 << i) &&
> 		    !(used & (1 << i)))
> 			cyttsp_report_slot_empty(ts->input, i);
> 
> where used is a bitmask to store each contact state for this event and
> previous is a bitmask to store the state for the previous event.
> 
> I looked at input_mt_report_slot_state() and it seems that it only
> checks whether the active parameter to send an ABS_MT_TRACKING_ID -1
> input event, so I think that I should use the latter.

Please implement the first version; the event never gets sent unless
there is an actual change, which you can see by following the code
deeper. If/when there are more drivers in need of a similar approach,
I might integrate the logic in the input-mt core instead.

Thanks,
Henrik