All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH V2 0/3] Input: Cypress TTSP device driver
@ 2011-09-03  5:50 Javier Martinez Canillas
  2011-09-03  5:50 ` [PATCH V2 1/3] Input: cyttsp - Cypress TTSP capacitive multi-touch screen support Javier Martinez Canillas
                   ` (2 more replies)
  0 siblings, 3 replies; 13+ messages in thread
From: Javier Martinez Canillas @ 2011-09-03  5:50 UTC (permalink / raw)
  To: Kevin McNeely
  Cc: Dmitry Torokhov, Henrik Rydberg, Greg Kroah-Hartman, linux-input,
	linux-kernel

Cypress TrueTouch(tm) Standard Product controllers are found in
a wide range of embedded devices. This patchset 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 second version of the driver that fixes a few issues called out by
Dmitry Torokhov.

The patchset is composed of the patches:

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH V2 1/3] Input: cyttsp - Cypress TTSP capacitive multi-touch screen support
  2011-09-03  5:50 [PATCH V2 0/3] Input: Cypress TTSP device driver Javier Martinez Canillas
@ 2011-09-03  5:50 ` Javier Martinez Canillas
  2011-09-13  8:24   ` Henrik Rydberg
  2011-09-14 10:50   ` Mohan Pallaka
  2011-09-03  5:50 ` [PATCH V2 2/3] Input: cyttsp - add support for Cypress TTSP touchscreen I2C bus interface Javier Martinez Canillas
  2011-09-03  5:50 ` [PATCH V2 3/3] Input: cyttsp - add support for Cypress TTSP touchscreen SPI " Javier Martinez Canillas
  2 siblings, 2 replies; 13+ messages in thread
From: Javier Martinez Canillas @ 2011-09-03  5:50 UTC (permalink / raw)
  To: Kevin McNeely
  Cc: Dmitry Torokhov, Henrik Rydberg, Greg Kroah-Hartman, linux-input,
	linux-kernel, 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>
---
Changes since v1: 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

 drivers/input/touchscreen/Kconfig              |    2 +
 drivers/input/touchscreen/Makefile             |    1 +
 drivers/input/touchscreen/cyttsp/Kconfig       |    4 +
 drivers/input/touchscreen/cyttsp/Makefile      |    1 +
 drivers/input/touchscreen/cyttsp/cyttsp_core.c |  879 ++++++++++++++++++++++++
 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..6786680 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..945950a 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-$(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..7e8e0f4
--- /dev/null
+++ b/drivers/input/touchscreen/cyttsp/Kconfig
@@ -0,0 +1,4 @@
+config TOUCHSCREEN_CYTTSP_CORE
+	tristate "Cypress TTSP touchscreen core"
+	help
+	  Always activated for Cypress TTSP touchscreen
diff --git a/drivers/input/touchscreen/cyttsp/Makefile b/drivers/input/touchscreen/cyttsp/Makefile
new file mode 100644
index 0000000..d1b250f
--- /dev/null
+++ b/drivers/input/touchscreen/cyttsp/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_TOUCHSCREEN_CYTTSP_CORE)   += cyttsp_core.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..a145518
--- /dev/null
+++ b/drivers/input/touchscreen/cyttsp/cyttsp_core.c
@@ -0,0 +1,879 @@
+/*
+ * 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_SLOT_UNUSED              0
+
+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 bus_type *bus_type;
+	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_cnt;
+	int slots[CY_MAX_FINGER];
+	int slot_status[CY_MAX_FINGER];
+};
+
+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;
+	if (!buf || !length)
+		return -EINVAL;
+
+	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 {
+		msleep(CY_DELAY_DFLT);
+		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_get_slot_id(struct cyttsp *ts, int id)
+{
+	int i;
+
+	for (i = 0; i < CY_MAX_FINGER; i++)
+		if (ts->slots[i] == id)
+			return i;
+
+	return -1;
+}
+
+static int cyttsp_assign_next_slot(struct cyttsp *ts, int id)
+{
+	int i;
+
+	for (i = 0; i < CY_MAX_FINGER; i++)
+		if (ts->slots[i] == CY_SLOT_UNUSED) {
+			ts->slots[i] = id;
+			ts->slot_cnt++;
+			return i;
+		}
+
+	return -1;
+}
+
+static int cyttsp_xy_worker(struct cyttsp *ts)
+{
+	struct cyttsp_xydata xy_data;
+	u8 num_cur_tch;
+	int i;
+	int ids[4];
+	int slot = -1;
+	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 < CY_MAX_FINGER; i++)
+		ts->slot_status[i] = false;
+
+	for (i = 0; i < num_cur_tch; i++) {
+		slot = cyttsp_get_slot_id(ts, ids[i]);
+
+		if (slot < 0)
+			slot = cyttsp_assign_next_slot(ts, ids[i]);
+
+		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, slot, x, y, z);
+
+		ts->slot_status[slot] = true;
+	}
+
+	for (i = 0; i < CY_MAX_FINGER; i++)
+		if (ts->slot_status[i] == false &&
+		   ts->slots[i] != CY_SLOT_UNUSED) {
+			ts->slots[i] = CY_SLOT_UNUSED;
+			ts->slot_cnt--;
+			cyttsp_report_slot_empty(ts->input, i);
+		}
+
+	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_xy_worker(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->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 = CY_DEEP_SLEEP_MODE;
+		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;
+
+	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;
+	ts->bus_type = bus_ops->dev->bus;
+	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_FINGER);
+
+	for (i = 0; i < CY_MAX_FINGER; i++)
+		ts->slots[i] = CY_SLOT_UNUSED;
+
+	ts->slot_cnt = 0;
+
+	if (input_register_device(input_device)) {
+		dev_dbg(ts->dev, "%s: Error, failed to register input device\n",
+			__func__);
+		goto error_input_register_device;
+	}
+
+	goto no_error;
+
+error_input_register_device:
+	input_unregister_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


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH V2 2/3] Input: cyttsp - add support for Cypress TTSP touchscreen I2C bus interface
  2011-09-03  5:50 [PATCH V2 0/3] Input: Cypress TTSP device driver Javier Martinez Canillas
  2011-09-03  5:50 ` [PATCH V2 1/3] Input: cyttsp - Cypress TTSP capacitive multi-touch screen support Javier Martinez Canillas
@ 2011-09-03  5:50 ` Javier Martinez Canillas
  2011-09-03  5:50 ` [PATCH V2 3/3] Input: cyttsp - add support for Cypress TTSP touchscreen SPI " Javier Martinez Canillas
  2 siblings, 0 replies; 13+ messages in thread
From: Javier Martinez Canillas @ 2011-09-03  5:50 UTC (permalink / raw)
  To: Kevin McNeely
  Cc: Dmitry Torokhov, Henrik Rydberg, Greg Kroah-Hartman, linux-input,
	linux-kernel, 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>
---
Changes since v1: Fix issues called out by Dmitry Torokhov
	- Extract the IRQ from the i2c client data and pass to cyttsp_core_init

 drivers/input/touchscreen/cyttsp/Kconfig      |   12 ++
 drivers/input/touchscreen/cyttsp/Makefile     |    1 +
 drivers/input/touchscreen/cyttsp/cyttsp_i2c.c |  188 +++++++++++++++++++++++++
 3 files changed, 201 insertions(+), 0 deletions(-)
 create mode 100644 drivers/input/touchscreen/cyttsp/cyttsp_i2c.c

diff --git a/drivers/input/touchscreen/cyttsp/Kconfig b/drivers/input/touchscreen/cyttsp/Kconfig
index 7e8e0f4..256642d 100644
--- a/drivers/input/touchscreen/cyttsp/Kconfig
+++ b/drivers/input/touchscreen/cyttsp/Kconfig
@@ -2,3 +2,15 @@ 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.
diff --git a/drivers/input/touchscreen/cyttsp/Makefile b/drivers/input/touchscreen/cyttsp/Makefile
index d1b250f..9101dc9 100644
--- a/drivers/input/touchscreen/cyttsp/Makefile
+++ b/drivers/input/touchscreen/cyttsp/Makefile
@@ -1 +1,2 @@
 obj-$(CONFIG_TOUCHSCREEN_CYTTSP_CORE)   += cyttsp_core.o
+obj-$(CONFIG_TOUCHSCREEN_CYTTSP_I2C)    += cyttsp_i2c.o
diff --git a/drivers/input/touchscreen/cyttsp/cyttsp_i2c.c b/drivers/input/touchscreen/cyttsp/cyttsp_i2c.c
new file mode 100644
index 0000000..89d1fdf
--- /dev/null
+++ b/drivers/input/touchscreen/cyttsp/cyttsp_i2c.c
@@ -0,0 +1,188 @@
+/*
+ * 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->ops.dev->bus = &i2c_bus_type;
+
+	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


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH V2 3/3] Input: cyttsp - add support for Cypress TTSP touchscreen SPI bus interface
  2011-09-03  5:50 [PATCH V2 0/3] Input: Cypress TTSP device driver Javier Martinez Canillas
  2011-09-03  5:50 ` [PATCH V2 1/3] Input: cyttsp - Cypress TTSP capacitive multi-touch screen support Javier Martinez Canillas
  2011-09-03  5:50 ` [PATCH V2 2/3] Input: cyttsp - add support for Cypress TTSP touchscreen I2C bus interface Javier Martinez Canillas
@ 2011-09-03  5:50 ` Javier Martinez Canillas
  2 siblings, 0 replies; 13+ messages in thread
From: Javier Martinez Canillas @ 2011-09-03  5:50 UTC (permalink / raw)
  To: Kevin McNeely
  Cc: Dmitry Torokhov, Henrik Rydberg, Greg Kroah-Hartman, linux-input,
	linux-kernel, 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>
---
Changes since v1: 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.

 drivers/input/touchscreen/cyttsp/Kconfig      |   12 +
 drivers/input/touchscreen/cyttsp/Makefile     |    1 +
 drivers/input/touchscreen/cyttsp/cyttsp_spi.c |  293 +++++++++++++++++++++++++
 3 files changed, 306 insertions(+), 0 deletions(-)
 create mode 100644 drivers/input/touchscreen/cyttsp/cyttsp_spi.c

diff --git a/drivers/input/touchscreen/cyttsp/Kconfig b/drivers/input/touchscreen/cyttsp/Kconfig
index 256642d..e72c66c 100644
--- a/drivers/input/touchscreen/cyttsp/Kconfig
+++ b/drivers/input/touchscreen/cyttsp/Kconfig
@@ -14,3 +14,15 @@ config TOUCHSCREEN_CYTTSP_I2C
 
 	  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
index 9101dc9..687eeaa 100644
--- a/drivers/input/touchscreen/cyttsp/Makefile
+++ b/drivers/input/touchscreen/cyttsp/Makefile
@@ -1,2 +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_spi.c b/drivers/input/touchscreen/cyttsp/cyttsp_spi.c
new file mode 100644
index 0000000..71a586a
--- /dev/null
+++ b/drivers/input/touchscreen/cyttsp/cyttsp_spi.c
@@ -0,0 +1,293 @@
+/*
+ * 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,
+		.bus = &spi_bus_type,
+		.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


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH V2 1/3] Input: cyttsp - Cypress TTSP capacitive multi-touch screen support
  2011-09-03  5:50 ` [PATCH V2 1/3] Input: cyttsp - Cypress TTSP capacitive multi-touch screen support Javier Martinez Canillas
@ 2011-09-13  8:24   ` Henrik Rydberg
  2011-09-14  7:14       ` Javier Martinez Canillas
  2011-09-14 10:50   ` Mohan Pallaka
  1 sibling, 1 reply; 13+ messages in thread
From: Henrik Rydberg @ 2011-09-13  8:24 UTC (permalink / raw)
  To: Javier Martinez Canillas
  Cc: Kevin McNeely, Dmitry Torokhov, Greg Kroah-Hartman, linux-input,
	linux-kernel

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.
> 
> 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>
> ---
> Changes since v1: 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

Compared to the staging version you sent earlier, this version seems
to be a step back towards the original version regarding the MT
implementation. Is this the right patch?

Thanks,
Henrik

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH V2 1/3] Input: cyttsp - Cypress TTSP capacitive multi-touch screen support
  2011-09-13  8:24   ` Henrik Rydberg
@ 2011-09-14  7:14       ` Javier Martinez Canillas
  0 siblings, 0 replies; 13+ messages in thread
From: Javier Martinez Canillas @ 2011-09-14  7:14 UTC (permalink / raw)
  To: Henrik Rydberg
  Cc: Kevin McNeely, Dmitry Torokhov, Greg Kroah-Hartman, linux-input,
	linux-kernel

On Tue, Sep 13, 2011 at 10:24 AM, 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.
>>
>> 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>
>> ---
>> Changes since v1: 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
>
> Compared to the staging version you sent earlier, this version seems
> to be a step back towards the original version regarding the MT
> implementation. Is this the right patch?
>
> Thanks,
> Henrik
>

Hello Henrik,

Yes it is the right patch, or at least the patch I meant to send. The
staging version I send earlier didn't support multi-touch protocol
type A.

Since Cypress HW can do finger (contact) tracking, a requirement was
to add multi-touch protocol type B support to the driver. This
patch-set does that and also fixes some issues that Dmitry had with
this patch-set in its V1.

Even when Cypress touchscreen can keep track of each contact, it
doesn't preserve the contact index. So if I have 3 fingers that were
pressed in order and the touchscreen assigned it the values:

10 11 12

And then I lift the first finger (10), the hardware reports:

11 12

Maybe that is why the driver seems complex, you have to keep a copy of
the hardware state and iterate over all previous contacts to see if
there are new fingers and if the old ones have been lifted.

Best regards,

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH V2 1/3] Input: cyttsp - Cypress TTSP capacitive multi-touch screen support
@ 2011-09-14  7:14       ` Javier Martinez Canillas
  0 siblings, 0 replies; 13+ messages in thread
From: Javier Martinez Canillas @ 2011-09-14  7:14 UTC (permalink / raw)
  To: Henrik Rydberg
  Cc: Kevin McNeely, Dmitry Torokhov, Greg Kroah-Hartman, linux-input,
	linux-kernel

On Tue, Sep 13, 2011 at 10:24 AM, 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.
>>
>> 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>
>> ---
>> Changes since v1: 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
>
> Compared to the staging version you sent earlier, this version seems
> to be a step back towards the original version regarding the MT
> implementation. Is this the right patch?
>
> Thanks,
> Henrik
>

Hello Henrik,

Yes it is the right patch, or at least the patch I meant to send. The
staging version I send earlier didn't support multi-touch protocol
type A.

Since Cypress HW can do finger (contact) tracking, a requirement was
to add multi-touch protocol type B support to the driver. This
patch-set does that and also fixes some issues that Dmitry had with
this patch-set in its V1.

Even when Cypress touchscreen can keep track of each contact, it
doesn't preserve the contact index. So if I have 3 fingers that were
pressed in order and the touchscreen assigned it the values:

10 11 12

And then I lift the first finger (10), the hardware reports:

11 12

Maybe that is why the driver seems complex, you have to keep a copy of
the hardware state and iterate over all previous contacts to see if
there are new fingers and if the old ones have been lifted.

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH V2 1/3] Input: cyttsp - Cypress TTSP capacitive multi-touch screen support
  2011-09-14  7:14       ` Javier Martinez Canillas
  (?)
@ 2011-09-14  7:46       ` Henrik Rydberg
  2011-09-14 21:03           ` Javier Martinez Canillas
  -1 siblings, 1 reply; 13+ messages in thread
From: Henrik Rydberg @ 2011-09-14  7:46 UTC (permalink / raw)
  To: Javier Martinez Canillas
  Cc: Kevin McNeely, Dmitry Torokhov, Greg Kroah-Hartman, linux-input,
	linux-kernel

> > Compared to the staging version you sent earlier, this version seems
> > to be a step back towards the original version regarding the MT
> > implementation. Is this the right patch?
> 
> Hello Henrik,
> 
> Yes it is the right patch, or at least the patch I meant to send. The
> staging version I send earlier didn't support multi-touch protocol
> type A.

I see - the cyttsp_xydata struct seems to be the old version,
though. Any particular reason for that? Also, the similarity to the
problem found here, https://lkml.org/lkml/2011/8/25/516, suggests your
16 tracking ids could be mapped directly to slots instead, using the
example code. It ought to reduce the present patch significantly.

Thanks,
Henrik

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH V2 1/3] Input: cyttsp - Cypress TTSP capacitive multi-touch screen support
  2011-09-03  5:50 ` [PATCH V2 1/3] Input: cyttsp - Cypress TTSP capacitive multi-touch screen support Javier Martinez Canillas
  2011-09-13  8:24   ` Henrik Rydberg
@ 2011-09-14 10:50   ` Mohan Pallaka
  2011-09-14 22:32       ` Javier Martinez Canillas
  1 sibling, 1 reply; 13+ messages in thread
From: Mohan Pallaka @ 2011-09-14 10:50 UTC (permalink / raw)
  To: Javier Martinez Canillas
  Cc: Kevin McNeely, Dmitry Torokhov, Henrik Rydberg,
	Greg Kroah-Hartman, linux-input, linux-kernel

  Hi Javier,

Please find my review comments.

On 9/3/2011 11:20 AM, Javier Martinez Canillas wrote:
> 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>
> ---
> Changes since v1: 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
>
>   drivers/input/touchscreen/Kconfig              |    2 +
>   drivers/input/touchscreen/Makefile             |    1 +
>   drivers/input/touchscreen/cyttsp/Kconfig       |    4 +
>   drivers/input/touchscreen/cyttsp/Makefile      |    1 +
>   drivers/input/touchscreen/cyttsp/cyttsp_core.c |  879 ++++++++++++++++++++++++
>   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..6786680 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..945950a 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-$(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..7e8e0f4
> --- /dev/null
> +++ b/drivers/input/touchscreen/cyttsp/Kconfig
> @@ -0,0 +1,4 @@
> +config TOUCHSCREEN_CYTTSP_CORE
> +	tristate "Cypress TTSP touchscreen core"
> +	help
> +	  Always activated for Cypress TTSP touchscreen
> diff --git a/drivers/input/touchscreen/cyttsp/Makefile b/drivers/input/touchscreen/cyttsp/Makefile
> new file mode 100644
> index 0000000..d1b250f
> --- /dev/null
> +++ b/drivers/input/touchscreen/cyttsp/Makefile
> @@ -0,0 +1 @@
> +obj-$(CONFIG_TOUCHSCREEN_CYTTSP_CORE)   += cyttsp_core.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..a145518
> --- /dev/null
> +++ b/drivers/input/touchscreen/cyttsp/cyttsp_core.c
> @@ -0,0 +1,879 @@
> +/*
> + * 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_SLOT_UNUSED              0
> +
> +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 bus_type *bus_type;
Do we need to know the bus type? Rest of the code doesn't seem to
be using this information.
> +	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_cnt;
> +	int slots[CY_MAX_FINGER];
> +	int slot_status[CY_MAX_FINGER];
> +};
> +
> +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;
> +	if (!buf || !length)
> +		return -EINVAL;
> +
> +	retval = ts->bus_ops->write(ts->bus_ops, command, length, buf);
> +	if (retval)
> +		msleep(CY_DELAY_DFLT);
Don't we need to retry if write fails?
> +
> +	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 {
> +		msleep(CY_DELAY_DFLT);
> +		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))&&
what is the need for this check,"xy_data.act_dist == CY_ACT_DIST_DFLT"?
we can pass a different value for act_dist from pdata. Another point to 
concerns me
is that in case of i2c/spi failures "ttsp_read_block_data" will it self 
try for NUM_TRY
and this particular code block tries till CY_DELAY_MAX, I think we are 
overdoing
in this case.  How about keeping the "ttsp_read_block_dat" simple and 
let the
code that uses do the retry job.
> +		(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_get_slot_id(struct cyttsp *ts, int id)
> +{
> +	int i;
> +
> +	for (i = 0; i<  CY_MAX_FINGER; i++)
> +		if (ts->slots[i] == id)
> +			return i;
> +
> +	return -1;
> +}
> +
> +static int cyttsp_assign_next_slot(struct cyttsp *ts, int id)
> +{
> +	int i;
> +
> +	for (i = 0; i<  CY_MAX_FINGER; i++)
> +		if (ts->slots[i] == CY_SLOT_UNUSED) {
> +			ts->slots[i] = id;
> +			ts->slot_cnt++;
> +			return i;
> +		}
> +
> +	return -1;
> +}
> +
> +static int cyttsp_xy_worker(struct cyttsp *ts)
> +{
> +	struct cyttsp_xydata xy_data;
> +	u8 num_cur_tch;
> +	int i;
> +	int ids[4];
> +	int slot = -1;
> +	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<  CY_MAX_FINGER; i++)
> +		ts->slot_status[i] = false;
> +
> +	for (i = 0; i<  num_cur_tch; i++) {
> +		slot = cyttsp_get_slot_id(ts, ids[i]);
> +
> +		if (slot<  0)
> +			slot = cyttsp_assign_next_slot(ts, ids[i]);
> +
> +		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, slot, x, y, z);
> +
> +		ts->slot_status[slot] = true;
> +	}
> +
> +	for (i = 0; i<  CY_MAX_FINGER; i++)
> +		if (ts->slot_status[i] == false&&
> +		   ts->slots[i] != CY_SLOT_UNUSED) {
> +			ts->slots[i] = CY_SLOT_UNUSED;
> +			ts->slot_cnt--;
> +			cyttsp_report_slot_empty(ts->input, i);
> +		}
> +
> +	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_xy_worker(ts);
can we have a different name than cyttsp_xy_worker as we can 
misinterpret as a workqueue,
like cyttsp_handle_tchdata. Also, we seem to be doing good amount of 
work in this function,
how about deferring it to a workqueue to make irq handler fast to not to 
miss any irqs.
> +
> +		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;
"handle" is passed from a different path, so it is possible to be a 
"NULL" which
could crash the machine. so I think we can have an extra check here like 
you
did for core_release.
> +	int retval = 0;
> +	struct cyttsp_xydata xydata;
> +
> +	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;
I think we need to have a error msg when it fails to resume. Also, say 
after resume if it goes to
"bootloader"(which happens when the chip is reset) mode then touch will 
not work. So, we
can get it out of the bootloader mode in cyttsp_xy_worker as interrupts 
are fired even in bootloader
mode.
> +		}
> +	}
> +	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 = CY_DEEP_SLEEP_MODE;
> +		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"));
This code will not let the chip to go to low power mode since we are 
setting
CY_DEEP_SLEEP_MODE directly. So, lets use platform_data's "use_sleep" 
flag to
determine if we want to go to deep sleep or low power mode.
> +	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;
> +
> +	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;
> +	ts->bus_type = bus_ops->dev->bus;
> +	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_FINGER);
> +
> +	for (i = 0; i<  CY_MAX_FINGER; i++)
> +		ts->slots[i] = CY_SLOT_UNUSED;
> +
> +	ts->slot_cnt = 0;
> +
> +	if (input_register_device(input_device)) {
Please avoid this style of check, we will not know why it is failed.
> +		dev_dbg(ts->dev, "%s: Error, failed to register input device\n",
> +			__func__);
> +		goto error_input_register_device;
> +	}
> +
> +	goto no_error;
> +
> +error_input_register_device:
> +	input_unregister_device(input_device);
Here we should use input_free_device() rather than unregister since it 
failed in
registration.
> +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_ */
--Mohan.

--
Sent by a consultant of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
Forum.



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH V2 1/3] Input: cyttsp - Cypress TTSP capacitive multi-touch screen support
  2011-09-14  7:46       ` Henrik Rydberg
@ 2011-09-14 21:03           ` Javier Martinez Canillas
  0 siblings, 0 replies; 13+ messages in thread
From: Javier Martinez Canillas @ 2011-09-14 21:03 UTC (permalink / raw)
  To: Henrik Rydberg
  Cc: Kevin McNeely, Dmitry Torokhov, Greg Kroah-Hartman, linux-input,
	linux-kernel

On Wed, Sep 14, 2011 at 9:46 AM, Henrik Rydberg <rydberg@euromail.se> wrote:
>>
>> Yes it is the right patch, or at least the patch I meant to send. The
>> staging version I send earlier didn't support multi-touch protocol
>> type A.
>
> I see - the cyttsp_xydata struct seems to be the old version,
> though. Any particular reason for that? Also, the similarity to the
> problem found here, https://lkml.org/lkml/2011/8/25/516, suggests your
> 16 tracking ids could be mapped directly to slots instead, using the
> example code. It ought to reduce the present patch significantly.
>
> Thanks,
> Henrik
>

Hello Henrik,

Thank you for your recommendations.

I didn't know that there was a newer version of cyttsp_xydata struct.
I thought I based my work on the last version of Cypress driver.
Didn't know about Dudley Du's work either. It seems that both drivers
are for the same device family.

I'll take a look at Dudley's patch-set and also contact him to join efforts.

Best regards,

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH V2 1/3] Input: cyttsp - Cypress TTSP capacitive multi-touch screen support
@ 2011-09-14 21:03           ` Javier Martinez Canillas
  0 siblings, 0 replies; 13+ messages in thread
From: Javier Martinez Canillas @ 2011-09-14 21:03 UTC (permalink / raw)
  To: Henrik Rydberg
  Cc: Kevin McNeely, Dmitry Torokhov, Greg Kroah-Hartman, linux-input,
	linux-kernel

On Wed, Sep 14, 2011 at 9:46 AM, Henrik Rydberg <rydberg@euromail.se> wrote:
>>
>> Yes it is the right patch, or at least the patch I meant to send. The
>> staging version I send earlier didn't support multi-touch protocol
>> type A.
>
> I see - the cyttsp_xydata struct seems to be the old version,
> though. Any particular reason for that? Also, the similarity to the
> problem found here, https://lkml.org/lkml/2011/8/25/516, suggests your
> 16 tracking ids could be mapped directly to slots instead, using the
> example code. It ought to reduce the present patch significantly.
>
> Thanks,
> Henrik
>

Hello Henrik,

Thank you for your recommendations.

I didn't know that there was a newer version of cyttsp_xydata struct.
I thought I based my work on the last version of Cypress driver.
Didn't know about Dudley Du's work either. It seems that both drivers
are for the same device family.

I'll take a look at Dudley's patch-set and also contact him to join efforts.

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH V2 1/3] Input: cyttsp - Cypress TTSP capacitive multi-touch screen support
  2011-09-14 10:50   ` Mohan Pallaka
@ 2011-09-14 22:32       ` Javier Martinez Canillas
  0 siblings, 0 replies; 13+ messages in thread
From: Javier Martinez Canillas @ 2011-09-14 22:32 UTC (permalink / raw)
  To: Mohan Pallaka
  Cc: Kevin McNeely, Dmitry Torokhov, Henrik Rydberg,
	Greg Kroah-Hartman, linux-input, linux-kernel

On Wed, Sep 14, 2011 at 12:50 PM, Mohan Pallaka <mpallaka@codeaurora.org> wrote:
>  Hi Javier,
>
> Please find my review comments.
>

Hello Mohan,

Thank you very much for the review and your comments.

> On 9/3/2011 11:20 AM, Javier Martinez Canillas wrote:
>> +
>> +struct cyttsp {
>> +       struct device *dev;
>> +       int irq;
>> +       struct input_dev *input;
>> +       char phys[32];
>> +       const struct bus_type *bus_type;
>
> Do we need to know the bus type? Rest of the code doesn't seem to
> be using this information.

You are right, will remove it.

>> +
>> +static int ttsp_write_block_data(struct cyttsp *ts, u8 command,
>> +       u8 length, void *buf)
>> +{
>> +       int retval;
>> +       if (!buf || !length)
>> +               return -EINVAL;
>> +
>> +       retval = ts->bus_ops->write(ts->bus_ops, command, length, buf);
>> +       if (retval)
>> +               msleep(CY_DELAY_DFLT);
>
> Don't we need to retry if write fails?

Yes, I'll add the retry logic also to ttsp_write_block_data().

>> +
>> +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 {
>> +               msleep(CY_DELAY_DFLT);
>> +               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))&&
>
> what is the need for this check,"xy_data.act_dist == CY_ACT_DIST_DFLT"?
> we can pass a different value for act_dist from pdata. Another point to
> concerns me
> is that in case of i2c/spi failures "ttsp_read_block_data" will it self try
> for NUM_TRY
> and this particular code block tries till CY_DELAY_MAX, I think we are
> overdoing
> in this case.  How about keeping the "ttsp_read_block_dat" simple and let
> the
> code that uses do the retry job.

Yes, there are too many msleep() and retry here, will clean this code
and let ttsp_read_block_data() do the retry.

>> +
>> +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_xy_worker(ts);
>
> can we have a different name than cyttsp_xy_worker as we can misinterpret as
> a workqueue,
> like cyttsp_handle_tchdata. Also, we seem to be doing good amount of work in
> this function,
> how about deferring it to a workqueue to make irq handler fast to not to
> miss any irqs.

Yes, your are right worker sounds like a workqueue. Will change the
name of the handler function.

Well cyttsp_xy_worker() is calling from cyttsp_irq() which is
initialized has a threaded irq with request_threaded_irq(). I thought
it was the way to go for new drivers, isn't using a workqueue inside a
threaded irq overkill?

>> +
>> +#ifdef CONFIG_PM
>> +int cyttsp_resume(void *handle)
>> +{
>> +       struct cyttsp *ts = handle;
>
> "handle" is passed from a different path, so it is possible to be a "NULL"
> which
> could crash the machine. so I think we can have an extra check here like you
> did for core_release.

Ok, will add the check.

>>
>> +       int retval = 0;
>> +       struct cyttsp_xydata xydata;
>> +
>> +       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;
>
> I think we need to have a error msg when it fails to resume. Also, say after
> resume if it goes to
> "bootloader"(which happens when the chip is reset) mode then touch will not
> work. So, we
> can get it out of the bootloader mode in cyttsp_xy_worker as interrupts are
> fired even in bootloader
> mode.

Yes, will add the error msg and add the logic in the irq handler to go
out from bootloader mode.

>> +
>> +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 = CY_DEEP_SLEEP_MODE;
>> +               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"));
>
> This code will not let the chip to go to low power mode since we are setting
> CY_DEEP_SLEEP_MODE directly. So, lets use platform_data's "use_sleep" flag
> to
> determine if we want to go to deep sleep or low power mode.

Ok, will change to check platform_data's to take the decision.


>> +
>> +       if (input_register_device(input_device)) {
>
> Please avoid this style of check, we will not know why it is failed.

Ok, will get the error value to know what happened.

>>
>> +               dev_dbg(ts->dev, "%s: Error, failed to register input
>> device\n",
>> +                       __func__);
>> +               goto error_input_register_device;
>> +       }
>> +
>> +       goto no_error;
>> +
>> +error_input_register_device:
>> +       input_unregister_device(input_device);
>
> Here we should use input_free_device() rather than unregister since it
> failed in
> registration.

Ok, will use input_free_device() instead.

>
> --Mohan.
>
> --
> Sent by a consultant of the Qualcomm Innovation Center, Inc.
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
> Forum.
>
>
>

Again, thank you very much for taking the time to look at the code and
for your suggestions. Will work on this and resend.

Best regards,

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH V2 1/3] Input: cyttsp - Cypress TTSP capacitive multi-touch screen support
@ 2011-09-14 22:32       ` Javier Martinez Canillas
  0 siblings, 0 replies; 13+ messages in thread
From: Javier Martinez Canillas @ 2011-09-14 22:32 UTC (permalink / raw)
  To: Mohan Pallaka
  Cc: Kevin McNeely, Dmitry Torokhov, Henrik Rydberg,
	Greg Kroah-Hartman, linux-input, linux-kernel

On Wed, Sep 14, 2011 at 12:50 PM, Mohan Pallaka <mpallaka@codeaurora.org> wrote:
>  Hi Javier,
>
> Please find my review comments.
>

Hello Mohan,

Thank you very much for the review and your comments.

> On 9/3/2011 11:20 AM, Javier Martinez Canillas wrote:
>> +
>> +struct cyttsp {
>> +       struct device *dev;
>> +       int irq;
>> +       struct input_dev *input;
>> +       char phys[32];
>> +       const struct bus_type *bus_type;
>
> Do we need to know the bus type? Rest of the code doesn't seem to
> be using this information.

You are right, will remove it.

>> +
>> +static int ttsp_write_block_data(struct cyttsp *ts, u8 command,
>> +       u8 length, void *buf)
>> +{
>> +       int retval;
>> +       if (!buf || !length)
>> +               return -EINVAL;
>> +
>> +       retval = ts->bus_ops->write(ts->bus_ops, command, length, buf);
>> +       if (retval)
>> +               msleep(CY_DELAY_DFLT);
>
> Don't we need to retry if write fails?

Yes, I'll add the retry logic also to ttsp_write_block_data().

>> +
>> +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 {
>> +               msleep(CY_DELAY_DFLT);
>> +               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))&&
>
> what is the need for this check,"xy_data.act_dist == CY_ACT_DIST_DFLT"?
> we can pass a different value for act_dist from pdata. Another point to
> concerns me
> is that in case of i2c/spi failures "ttsp_read_block_data" will it self try
> for NUM_TRY
> and this particular code block tries till CY_DELAY_MAX, I think we are
> overdoing
> in this case.  How about keeping the "ttsp_read_block_dat" simple and let
> the
> code that uses do the retry job.

Yes, there are too many msleep() and retry here, will clean this code
and let ttsp_read_block_data() do the retry.

>> +
>> +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_xy_worker(ts);
>
> can we have a different name than cyttsp_xy_worker as we can misinterpret as
> a workqueue,
> like cyttsp_handle_tchdata. Also, we seem to be doing good amount of work in
> this function,
> how about deferring it to a workqueue to make irq handler fast to not to
> miss any irqs.

Yes, your are right worker sounds like a workqueue. Will change the
name of the handler function.

Well cyttsp_xy_worker() is calling from cyttsp_irq() which is
initialized has a threaded irq with request_threaded_irq(). I thought
it was the way to go for new drivers, isn't using a workqueue inside a
threaded irq overkill?

>> +
>> +#ifdef CONFIG_PM
>> +int cyttsp_resume(void *handle)
>> +{
>> +       struct cyttsp *ts = handle;
>
> "handle" is passed from a different path, so it is possible to be a "NULL"
> which
> could crash the machine. so I think we can have an extra check here like you
> did for core_release.

Ok, will add the check.

>>
>> +       int retval = 0;
>> +       struct cyttsp_xydata xydata;
>> +
>> +       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;
>
> I think we need to have a error msg when it fails to resume. Also, say after
> resume if it goes to
> "bootloader"(which happens when the chip is reset) mode then touch will not
> work. So, we
> can get it out of the bootloader mode in cyttsp_xy_worker as interrupts are
> fired even in bootloader
> mode.

Yes, will add the error msg and add the logic in the irq handler to go
out from bootloader mode.

>> +
>> +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 = CY_DEEP_SLEEP_MODE;
>> +               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"));
>
> This code will not let the chip to go to low power mode since we are setting
> CY_DEEP_SLEEP_MODE directly. So, lets use platform_data's "use_sleep" flag
> to
> determine if we want to go to deep sleep or low power mode.

Ok, will change to check platform_data's to take the decision.


>> +
>> +       if (input_register_device(input_device)) {
>
> Please avoid this style of check, we will not know why it is failed.

Ok, will get the error value to know what happened.

>>
>> +               dev_dbg(ts->dev, "%s: Error, failed to register input
>> device\n",
>> +                       __func__);
>> +               goto error_input_register_device;
>> +       }
>> +
>> +       goto no_error;
>> +
>> +error_input_register_device:
>> +       input_unregister_device(input_device);
>
> Here we should use input_free_device() rather than unregister since it
> failed in
> registration.

Ok, will use input_free_device() instead.

>
> --Mohan.
>
> --
> Sent by a consultant of the Qualcomm Innovation Center, Inc.
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
> Forum.
>
>
>

Again, thank you very much for taking the time to look at the code and
for your suggestions. Will work on this and resend.

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2011-09-14 22:33 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-09-03  5:50 [PATCH V2 0/3] Input: Cypress TTSP device driver Javier Martinez Canillas
2011-09-03  5:50 ` [PATCH V2 1/3] Input: cyttsp - Cypress TTSP capacitive multi-touch screen support Javier Martinez Canillas
2011-09-13  8:24   ` Henrik Rydberg
2011-09-14  7:14     ` Javier Martinez Canillas
2011-09-14  7:14       ` Javier Martinez Canillas
2011-09-14  7:46       ` Henrik Rydberg
2011-09-14 21:03         ` Javier Martinez Canillas
2011-09-14 21:03           ` Javier Martinez Canillas
2011-09-14 10:50   ` Mohan Pallaka
2011-09-14 22:32     ` Javier Martinez Canillas
2011-09-14 22:32       ` Javier Martinez Canillas
2011-09-03  5:50 ` [PATCH V2 2/3] Input: cyttsp - add support for Cypress TTSP touchscreen I2C bus interface Javier Martinez Canillas
2011-09-03  5:50 ` [PATCH V2 3/3] Input: cyttsp - add support for Cypress TTSP touchscreen SPI " Javier Martinez Canillas

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.