linux-input.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] input: qt602240 - Add ATMEL QT602240 touchscreen driver
@ 2010-06-25  1:54 Joonyoung Shim
  2010-06-25 14:08 ` Henrik Rydberg
  0 siblings, 1 reply; 21+ messages in thread
From: Joonyoung Shim @ 2010-06-25  1:54 UTC (permalink / raw)
  To: dmitry.torokhov; +Cc: linux-input, rydberg, kyungmin.park

This chip full name is AT42QT602240 or ATMXT224. This is the capacitive
touchscreen supporting 10 multitouch and uses I2C interface.

Signed-off-by: Joonyoung Shim <jy0922.shim@samsung.com>
---
Changes since v1:
- Splitting long functions up
- Use threaded IRQs
- Add open() and close() methods for the input device
- Code cleanup from patch v1 feedback

 drivers/input/touchscreen/Kconfig       |   12 +
 drivers/input/touchscreen/Makefile      |    1 +
 drivers/input/touchscreen/qt602240_ts.c | 1442 +++++++++++++++++++++++++++++++
 include/linux/i2c/qt602240_ts.h         |   40 +
 4 files changed, 1495 insertions(+), 0 deletions(-)
 create mode 100644 drivers/input/touchscreen/qt602240_ts.c
 create mode 100644 include/linux/i2c/qt602240_ts.h

diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
index 3b9d5e2..99c8c7e 100644
--- a/drivers/input/touchscreen/Kconfig
+++ b/drivers/input/touchscreen/Kconfig
@@ -603,4 +603,16 @@ config TOUCHSCREEN_TPS6507X
 	  To compile this driver as a module, choose M here: the
 	  module will be called tps6507x_ts.
 
+config TOUCHSCREEN_QT602240
+	tristate "QT602240 I2C Touchscreen"
+	depends on I2C
+	help
+	  Say Y here if you have the AT42QT602240/ATMXT224 I2C touchscreen
+	  connected to your system.
+
+	  If unsure, say N.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called qt602240_ts.
+
 endif
diff --git a/drivers/input/touchscreen/Makefile b/drivers/input/touchscreen/Makefile
index 497964a..4f3760b 100644
--- a/drivers/input/touchscreen/Makefile
+++ b/drivers/input/touchscreen/Makefile
@@ -47,3 +47,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_QT602240)	+= qt602240_ts.o
diff --git a/drivers/input/touchscreen/qt602240_ts.c b/drivers/input/touchscreen/qt602240_ts.c
new file mode 100644
index 0000000..10a88dc
--- /dev/null
+++ b/drivers/input/touchscreen/qt602240_ts.c
@@ -0,0 +1,1442 @@
+/*
+ * qt602240_ts.c - AT42QT602240/ATMXT224 Touchscreen driver
+ *
+ * Copyright (C) 2010 Samsung Electronics Co.Ltd
+ * Author: Joonyoung Shim <jy0922.shim@samsung.com>
+ *
+ *  This program is free software; you can redistribute  it and/or modify it
+ *  under  the terms of  the GNU General  Public License as published by the
+ *  Free Software Foundation;  either version 2 of the  License, or (at your
+ *  option) any later version.
+ *
+ */
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/delay.h>
+#include <linux/firmware.h>
+#include <linux/i2c.h>
+#include <linux/i2c/qt602240_ts.h>
+#include <linux/input.h>
+#include <linux/interrupt.h>
+#include <linux/slab.h>
+
+/* Version */
+#define QT602240_VER_20			20
+#define QT602240_VER_21			21
+#define QT602240_VER_22			22
+
+/* Slave addresses */
+#define QT602240_APP_LOW		0x4a
+#define QT602240_APP_HIGH		0x4b
+#define QT602240_BOOT_LOW		0x24
+#define QT602240_BOOT_HIGH		0x25
+
+/* Firmware */
+#define QT602240_FW_NAME		"qt602240.fw"
+
+/* Registers */
+#define QT602240_FAMILY_ID		0x00
+#define QT602240_VARIANT_ID		0x01
+#define QT602240_VERSION		0x02
+#define QT602240_BUILD			0x03
+#define QT602240_MATRIX_X_SIZE		0x04
+#define QT602240_MATRIX_Y_SIZE		0x05
+#define QT602240_OBJECT_NUM		0x06
+#define QT602240_OBJECT_START		0x07
+
+#define QT602240_OBJECT_SIZE		6
+
+/* Object types */
+#define QT602240_DEBUG_DIAGNOSTIC	37
+#define QT602240_GEN_MESSAGE		5
+#define QT602240_GEN_COMMAND		6
+#define QT602240_GEN_POWER		7
+#define QT602240_GEN_ACQUIRE		8
+#define QT602240_TOUCH_MULTI		9
+#define QT602240_TOUCH_KEYARRAY		15
+#define QT602240_TOUCH_PROXIMITY	23
+#define QT602240_PROCI_GRIPFACE		20
+#define QT602240_PROCG_NOISE		22
+#define QT602240_PROCI_ONETOUCH		24
+#define QT602240_PROCI_TWOTOUCH		27
+#define QT602240_SPT_COMMSCONFIG	18	/* firmware ver 21 over */
+#define QT602240_SPT_GPIOPWM		19
+#define QT602240_SPT_SELFTEST		25
+#define QT602240_SPT_CTECONFIG		28
+#define QT602240_SPT_USERDATA		38	/* firmware ver 21 over */
+
+/* QT602240_GEN_COMMAND field */
+#define QT602240_COMMAND_RESET		0
+#define QT602240_COMMAND_BACKUPNV	1
+#define QT602240_COMMAND_CALIBRATE	2
+#define QT602240_COMMAND_REPORTALL	3
+#define QT602240_COMMAND_DIAGNOSTIC	5
+
+/* QT602240_GEN_POWER field */
+#define QT602240_POWER_IDLEACQINT	0
+#define QT602240_POWER_ACTVACQINT	1
+#define QT602240_POWER_ACTV2IDLETO	2
+
+/* QT602240_GEN_ACQUIRE field */
+#define QT602240_ACQUIRE_CHRGTIME	0
+#define QT602240_ACQUIRE_TCHDRIFT	2
+#define QT602240_ACQUIRE_DRIFTST	3
+#define QT602240_ACQUIRE_TCHAUTOCAL	4
+#define QT602240_ACQUIRE_SYNC		5
+#define QT602240_ACQUIRE_ATCHCALST	6
+#define QT602240_ACQUIRE_ATCHCALSTHR	7
+
+/* QT602240_TOUCH_MULTI field */
+#define QT602240_TOUCH_CTRL		0
+#define QT602240_TOUCH_XORIGIN		1
+#define QT602240_TOUCH_YORIGIN		2
+#define QT602240_TOUCH_XSIZE		3
+#define QT602240_TOUCH_YSIZE		4
+#define QT602240_TOUCH_BLEN		6
+#define QT602240_TOUCH_TCHTHR		7
+#define QT602240_TOUCH_TCHDI		8
+#define QT602240_TOUCH_ORIENT		9
+#define QT602240_TOUCH_MOVHYSTI		11
+#define QT602240_TOUCH_MOVHYSTN		12
+#define QT602240_TOUCH_NUMTOUCH		14
+#define QT602240_TOUCH_MRGHYST		15
+#define QT602240_TOUCH_MRGTHR		16
+#define QT602240_TOUCH_AMPHYST		17
+#define QT602240_TOUCH_XRANGE_LSB	18
+#define QT602240_TOUCH_XRANGE_MSB	19
+#define QT602240_TOUCH_YRANGE_LSB	20
+#define QT602240_TOUCH_YRANGE_MSB	21
+#define QT602240_TOUCH_XLOCLIP		22
+#define QT602240_TOUCH_XHICLIP		23
+#define QT602240_TOUCH_YLOCLIP		24
+#define QT602240_TOUCH_YHICLIP		25
+#define QT602240_TOUCH_XEDGECTRL	26
+#define QT602240_TOUCH_XEDGEDIST	27
+#define QT602240_TOUCH_YEDGECTRL	28
+#define QT602240_TOUCH_YEDGEDIST	29
+#define QT602240_TOUCH_JUMPLIMIT	30	/* firmware ver 22 over */
+
+/* QT602240_PROCI_GRIPFACE field */
+#define QT602240_GRIPFACE_CTRL		0
+#define QT602240_GRIPFACE_XLOGRIP	1
+#define QT602240_GRIPFACE_XHIGRIP	2
+#define QT602240_GRIPFACE_YLOGRIP	3
+#define QT602240_GRIPFACE_YHIGRIP	4
+#define QT602240_GRIPFACE_MAXTCHS	5
+#define QT602240_GRIPFACE_SZTHR1	7
+#define QT602240_GRIPFACE_SZTHR2	8
+#define QT602240_GRIPFACE_SHPTHR1	9
+#define QT602240_GRIPFACE_SHPTHR2	10
+#define QT602240_GRIPFACE_SUPEXTTO	11
+
+/* QT602240_PROCI_NOISE field */
+#define QT602240_NOISE_CTRL		0
+#define QT602240_NOISE_OUTFLEN		1
+#define QT602240_NOISE_GCAFUL_LSB	3
+#define QT602240_NOISE_GCAFUL_MSB	4
+#define QT602240_NOISE_GCAFLL_LSB	5
+#define QT602240_NOISE_GCAFLL_MSB	6
+#define QT602240_NOISE_ACTVGCAFVALID	7
+#define QT602240_NOISE_NOISETHR		8
+#define QT602240_NOISE_FREQHOPSCALE	10
+#define QT602240_NOISE_FREQ0		11
+#define QT602240_NOISE_FREQ1		12
+#define QT602240_NOISE_FREQ2		13
+#define QT602240_NOISE_FREQ3		14
+#define QT602240_NOISE_FREQ4		15
+#define QT602240_NOISE_IDLEGCAFVALID	16
+
+/* QT602240_SPT_COMMSCONFIG */
+#define QT602240_COMMS_CTRL		0
+#define QT602240_COMMS_CMD		1
+
+/* QT602240_SPT_CTECONFIG field */
+#define QT602240_CTE_CTRL		0
+#define QT602240_CTE_CMD		1
+#define QT602240_CTE_MODE		2
+#define QT602240_CTE_IDLEGCAFDEPTH	3
+#define QT602240_CTE_ACTVGCAFDEPTH	4
+#define QT602240_CTE_VOLTAGE		5	/* firmware ver 21 over */
+
+#define QT602240_VOLTAGE_DEFAULT	2700000
+#define QT602240_VOLTAGE_STEP		10000
+
+/* Define for QT602240_GEN_COMMAND */
+#define QT602240_BOOT_VALUE		0xa5
+#define QT602240_BACKUP_VALUE		0x55
+#define QT602240_BACKUP_TIME		25	/* msec */
+#define QT602240_RESET_TIME		65	/* msec */
+
+#define QT602240_FWRESET_TIME		175	/* msec */
+
+/* Command to unlock bootloader */
+#define QT602240_UNLOCK_CMD_MSB		0xaa
+#define QT602240_UNLOCK_CMD_LSB		0xdc
+
+/* Bootloader mode status */
+#define QT602240_WAITING_BOOTLOAD_CMD	0xc0	/* valid 7 6 bit only */
+#define QT602240_WAITING_FRAME_DATA	0x80	/* valid 7 6 bit only */
+#define QT602240_FRAME_CRC_CHECK	0x02
+#define QT602240_FRAME_CRC_FAIL		0x03
+#define QT602240_FRAME_CRC_PASS		0x04
+#define QT602240_APP_CRC_FAIL		0x40	/* valid 7 8 bit only */
+#define QT602240_BOOT_STATUS_MASK	0x3f
+
+/* Touch status */
+#define QT602240_SUPPRESS		(1 << 1)
+#define QT602240_AMP			(1 << 2)
+#define QT602240_VECTOR			(1 << 3)
+#define QT602240_MOVE			(1 << 4)
+#define QT602240_RELEASE		(1 << 5)
+#define QT602240_PRESS			(1 << 6)
+#define QT602240_DETECT			(1 << 7)
+
+/* Touchscreen absolute values */
+#define QT602240_MAX_XC			0x3ff
+#define QT602240_MAX_YC			0x3ff
+#define QT602240_MAX_AREA		0xff
+
+#define QT602240_MAX_FINGER		10
+
+/* Initial register values recommended from chip vendor */
+static const u8 init_vals_ver_20[] = {
+	/* QT602240_GEN_COMMAND(6) */
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	/* QT602240_GEN_POWER(7) */
+	0x20, 0xff, 0x32,
+	/* QT602240_GEN_ACQUIRE(8) */
+	0x08, 0x05, 0x05, 0x00, 0x00, 0x00, 0x05, 0x14,
+	/* QT602240_TOUCH_MULTI(9) */
+	0x00, 0x00, 0x00, 0x11, 0x0a, 0x00, 0x00, 0x00, 0x02, 0x00,
+	0x00, 0x01, 0x01, 0x0e, 0x0a, 0x0a, 0x0a, 0x0a, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x88, 0x64,
+	/* QT602240_TOUCH_KEYARRAY(15) */
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00,
+	/* QT602240_SPT_GPIOPWM(19) */
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x00,
+	/* QT602240_PROCI_GRIPFACE(20) */
+	0x00, 0x64, 0x64, 0x64, 0x64, 0x00, 0x00, 0x1e, 0x14, 0x04,
+	0x1e, 0x00,
+	/* QT602240_PROCG_NOISE(22) */
+	0x05, 0x00, 0x00, 0x19, 0x00, 0xe7, 0xff, 0x04, 0x32, 0x00,
+	0x01, 0x0a, 0x0f, 0x14, 0x00, 0x00, 0xe8,
+	/* QT602240_TOUCH_PROXIMITY(23) */
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x00,
+	/* QT602240_PROCI_ONETOUCH(24) */
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	/* QT602240_SPT_SELFTEST(25) */
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x00,
+	/* QT602240_PROCI_TWOTOUCH(27) */
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	/* QT602240_SPT_CTECONFIG(28) */
+	0x00, 0x00, 0x00, 0x04, 0x08,
+};
+
+static const u8 init_vals_ver_21[] = {
+	/* QT602240_GEN_COMMAND(6) */
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	/* QT602240_GEN_POWER(7) */
+	0x20, 0xff, 0x32,
+	/* QT602240_GEN_ACQUIRE(8) */
+	0x07, 0x00, 0x05, 0x00, 0x00, 0x00, 0x09, 0x23,
+	/* QT602240_TOUCH_MULTI(9) */
+	0x00, 0x00, 0x00, 0x13, 0x0b, 0x00, 0x00, 0x00, 0x02, 0x00,
+	0x00, 0x01, 0x01, 0x0e, 0x0a, 0x0a, 0x0a, 0x0a, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	/* QT602240_TOUCH_KEYARRAY(15) */
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00,
+	/* QT602240_SPT_GPIOPWM(19) */
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	/* QT602240_PROCI_GRIPFACE(20) */
+	0x07, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x50, 0x28, 0x04,
+	0x0f, 0x0a,
+	/* QT602240_PROCG_NOISE(22) */
+	0x05, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x03, 0x23, 0x00,
+	0x00, 0x05, 0x0f, 0x19, 0x23, 0x2d, 0x03,
+	/* QT602240_TOUCH_PROXIMITY(23) */
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x00,
+	/* QT602240_PROCI_ONETOUCH(24) */
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	/* QT602240_SPT_SELFTEST(25) */
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x00,
+	/* QT602240_PROCI_TWOTOUCH(27) */
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	/* QT602240_SPT_CTECONFIG(28) */
+	0x00, 0x00, 0x00, 0x08, 0x10, 0x00,
+};
+
+static const u8 init_vals_ver_22[] = {
+	/* QT602240_GEN_COMMAND(6) */
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	/* QT602240_GEN_POWER(7) */
+	0x20, 0xff, 0x32,
+	/* QT602240_GEN_ACQUIRE(8) */
+	0x07, 0x00, 0x05, 0x00, 0x00, 0x00, 0x09, 0x23,
+	/* QT602240_TOUCH_MULTI(9) */
+	0x00, 0x00, 0x00, 0x13, 0x0b, 0x00, 0x00, 0x00, 0x02, 0x00,
+	0x00, 0x01, 0x01, 0x0e, 0x0a, 0x0a, 0x0a, 0x0a, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00,
+	/* QT602240_TOUCH_KEYARRAY(15) */
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00,
+	/* QT602240_SPT_GPIOPWM(19) */
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	/* QT602240_PROCI_GRIPFACE(20) */
+	0x07, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x50, 0x28, 0x04,
+	0x0f, 0x0a,
+	/* QT602240_PROCG_NOISE(22) */
+	0x05, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x03, 0x23, 0x00,
+	0x00, 0x05, 0x0f, 0x19, 0x23, 0x2d, 0x03,
+	/* QT602240_TOUCH_PROXIMITY(23) */
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x00, 0x00,
+	/* QT602240_PROCI_ONETOUCH(24) */
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	/* QT602240_SPT_SELFTEST(25) */
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x00,
+	/* QT602240_PROCI_TWOTOUCH(27) */
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	/* QT602240_SPT_CTECONFIG(28) */
+	0x00, 0x00, 0x00, 0x08, 0x10, 0x00,
+};
+
+struct qt602240_info {
+	u8 family_id;
+	u8 variant_id;
+	u8 version;
+	u8 build;
+	u8 matrix_xsize;
+	u8 matrix_ysize;
+	u8 object_num;
+};
+
+struct qt602240_object {
+	u8 type;
+	u16 start_address;
+	u8 size;
+	u8 instances;
+	u8 num_report_ids;
+
+	/* to map object and message */
+	u8 max_reportid;
+};
+
+struct qt602240_message {
+	u8 reportid;
+	u8 message[7];
+	u8 checksum;
+};
+
+struct qt602240_finger {
+	int status;
+	int x;
+	int y;
+	int area;
+};
+
+/* Each client has this additional data */
+struct qt602240_data {
+	struct i2c_client *client;
+	struct input_dev *input_dev;
+	struct qt602240_platform_data *pdata;
+	struct qt602240_object *object_table;
+	struct qt602240_info info;
+	struct qt602240_finger finger[QT602240_MAX_FINGER];
+	struct work_struct ts_resume_work;
+	unsigned int irq;
+};
+
+static int qt602240_object_readable(unsigned int type)
+{
+	switch (type) {
+	case QT602240_GEN_MESSAGE:
+	case QT602240_GEN_COMMAND:
+	case QT602240_GEN_POWER:
+	case QT602240_GEN_ACQUIRE:
+	case QT602240_TOUCH_MULTI:
+	case QT602240_TOUCH_KEYARRAY:
+	case QT602240_TOUCH_PROXIMITY:
+	case QT602240_PROCI_GRIPFACE:
+	case QT602240_PROCG_NOISE:
+	case QT602240_PROCI_ONETOUCH:
+	case QT602240_PROCI_TWOTOUCH:
+	case QT602240_SPT_COMMSCONFIG:
+	case QT602240_SPT_GPIOPWM:
+	case QT602240_SPT_SELFTEST:
+	case QT602240_SPT_CTECONFIG:
+	case QT602240_SPT_USERDATA:
+		return 1;
+	default:
+		return 0;
+	}
+}
+
+static int qt602240_object_writable(unsigned int type)
+{
+	switch (type) {
+	case QT602240_GEN_COMMAND:
+	case QT602240_GEN_POWER:
+	case QT602240_GEN_ACQUIRE:
+	case QT602240_TOUCH_MULTI:
+	case QT602240_TOUCH_KEYARRAY:
+	case QT602240_TOUCH_PROXIMITY:
+	case QT602240_PROCI_GRIPFACE:
+	case QT602240_PROCG_NOISE:
+	case QT602240_PROCI_ONETOUCH:
+	case QT602240_PROCI_TWOTOUCH:
+	case QT602240_SPT_GPIOPWM:
+	case QT602240_SPT_SELFTEST:
+	case QT602240_SPT_CTECONFIG:
+		return 1;
+	default:
+		return 0;
+	}
+}
+
+static void qt602240_dump_message(struct device *dev,
+		struct qt602240_message *message)
+{
+	dev_dbg(dev, "reportid:\t0x%x\n", message->reportid);
+	dev_dbg(dev, "message1:\t0x%x\n", message->message[0]);
+	dev_dbg(dev, "message2:\t0x%x\n", message->message[1]);
+	dev_dbg(dev, "message3:\t0x%x\n", message->message[2]);
+	dev_dbg(dev, "message4:\t0x%x\n", message->message[3]);
+	dev_dbg(dev, "message5:\t0x%x\n", message->message[4]);
+	dev_dbg(dev, "message6:\t0x%x\n", message->message[5]);
+	dev_dbg(dev, "message7:\t0x%x\n", message->message[6]);
+	dev_dbg(dev, "checksum:\t0x%x\n", message->checksum);
+}
+
+static int qt602240_check_bootloader(struct i2c_client *client,
+		unsigned int state)
+{
+	u8 val;
+
+recheck:
+	if (i2c_master_recv(client, &val, 1) != 1) {
+		dev_err(&client->dev, "%s: i2c recv failed\n", __func__);
+		return -EIO;
+	}
+
+	switch (state) {
+	case QT602240_WAITING_BOOTLOAD_CMD:
+	case QT602240_WAITING_FRAME_DATA:
+		val &= ~QT602240_BOOT_STATUS_MASK;
+		break;
+	case QT602240_FRAME_CRC_PASS:
+		if (val == QT602240_FRAME_CRC_CHECK)
+			goto recheck;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	if (val != state) {
+		dev_err(&client->dev, "Unvalid bootloader mode state\n");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int qt602240_unlock_bootloader(struct i2c_client *client)
+{
+	u8 buf[2];
+
+	buf[0] = QT602240_UNLOCK_CMD_LSB;
+	buf[1] = QT602240_UNLOCK_CMD_MSB;
+
+	if (i2c_master_send(client, buf, 2) != 2) {
+		dev_err(&client->dev, "%s: i2c send failed\n", __func__);
+		return -EIO;
+	}
+
+	return 0;
+}
+
+static int qt602240_fw_write(struct i2c_client *client, const u8 *data,
+		unsigned int frame_size)
+{
+	if (i2c_master_send(client, data, frame_size) != frame_size) {
+		dev_err(&client->dev, "%s: i2c send failed\n", __func__);
+		return -EIO;
+	}
+
+	return 0;
+}
+
+static int qt602240_read_reg(struct i2c_client *client, u16 reg)
+{
+	u8 buf[2];
+	u8 val;
+
+	buf[0] = reg & 0xff;
+	buf[1] = (reg >> 8) & 0xff;
+
+	if (i2c_master_send(client, buf, 2) != 2) {
+		dev_err(&client->dev, "%s: i2c send failed\n", __func__);
+		return -EIO;
+	}
+
+	if (i2c_master_recv(client, &val, 1) != 1) {
+		dev_err(&client->dev, "%s: i2c recv failed\n", __func__);
+		return -EIO;
+	}
+
+	return val;
+}
+
+static int qt602240_write_reg(struct i2c_client *client, u16 reg, u8 val)
+{
+	u8 buf[3];
+
+	buf[0] = reg & 0xff;
+	buf[1] = (reg >> 8) & 0xff;
+	buf[2] = val;
+
+	if (i2c_master_send(client, buf, 3) != 3) {
+		dev_err(&client->dev, "%s: i2c send failed\n", __func__);
+		return -EIO;
+	}
+
+	return 0;
+}
+
+static int qt602240_read_object_table(struct i2c_client *client, u16 reg,
+		u8 *object_buf)
+{
+	u8 buf[2];
+
+	buf[0] = reg & 0xff;
+	buf[1] = (reg >> 8) & 0xff;
+
+	if (i2c_master_send(client, buf, 2) != 2) {
+		dev_err(&client->dev, "%s: i2c send failed\n", __func__);
+		return -EIO;
+	}
+
+	if (i2c_master_recv(client, object_buf, 6) != 6) {
+		dev_err(&client->dev, "%s: i2c recv failed\n", __func__);
+		return -EIO;
+	}
+
+	return 0;
+}
+
+static struct qt602240_object *
+qt602240_get_object(struct qt602240_data *data, u8 type)
+{
+	struct qt602240_object *object;
+	int i;
+
+	for (i = 0; i < data->info.object_num; i++) {
+		object = data->object_table + i;
+		if (object->type == type)
+			return object;
+	}
+
+	dev_err(&data->client->dev, "invalid type\n");
+	return NULL;
+}
+
+static int qt602240_read_message(struct qt602240_data *data,
+		struct qt602240_message *message)
+{
+	struct i2c_client *client = data->client;
+	struct qt602240_object *object;
+	u16 reg;
+	u8 buf[2];
+
+	object = qt602240_get_object(data, QT602240_GEN_MESSAGE);
+	if (!object)
+		return -EINVAL;
+
+	reg = object->start_address;
+
+	buf[0] = reg & 0xff;
+	buf[1] = (reg >> 8) & 0xff;
+
+	if (i2c_master_send(client, buf, 2) != 2) {
+		dev_err(&client->dev, "%s: i2c send failed\n", __func__);
+		return -EIO;
+	}
+
+	if (i2c_master_recv(client, (u8 *)message, 9) != 9) {
+		dev_err(&client->dev, "%s: i2c recv failed\n", __func__);
+		return -EIO;
+	}
+
+	return 0;
+}
+
+static int qt602240_read_object(struct qt602240_data *data, u8 type, u8 offset)
+{
+	struct qt602240_object *object;
+	u16 reg;
+
+	object = qt602240_get_object(data, type);
+	if (!object)
+		return -EINVAL;
+
+	reg = object->start_address;
+
+	return qt602240_read_reg(data->client, reg + offset);
+}
+
+static int qt602240_write_object(struct qt602240_data *data, u8 type,
+		u8 offset, u8 val)
+{
+	struct qt602240_object *object;
+	u16 reg;
+
+	object = qt602240_get_object(data, type);
+	if (!object)
+		return -EINVAL;
+
+	reg = object->start_address;
+	return qt602240_write_reg(data->client, reg + offset, val);
+}
+
+static void qt602240_check_config_error(struct qt602240_data *data,
+		struct qt602240_message *message)
+{
+	struct qt602240_object *object;
+	u8 reportid = message->reportid;
+	u8 min_reportid;
+
+	object = qt602240_get_object(data, QT602240_GEN_COMMAND);
+	if (!object)
+		return;
+
+	min_reportid = object->max_reportid - object->num_report_ids + 1;
+
+	/* Check if configration error */
+	if ((reportid == min_reportid) && (message->message[0] & 0x8)) {
+		/* Touch disable */
+		qt602240_write_object(data, QT602240_TOUCH_MULTI,
+				QT602240_TOUCH_CTRL, 0);
+
+		/* Backup to memory */
+		qt602240_write_object(data, QT602240_GEN_COMMAND,
+				QT602240_COMMAND_BACKUPNV,
+				QT602240_BACKUP_VALUE);
+		msleep(QT602240_BACKUP_TIME);
+
+		/* Soft reset */
+		qt602240_write_object(data, QT602240_GEN_COMMAND,
+				QT602240_COMMAND_RESET, 1);
+		msleep(QT602240_RESET_TIME);
+
+		/* Touch enable */
+		qt602240_write_object(data, QT602240_TOUCH_MULTI,
+				QT602240_TOUCH_CTRL, 0x83);
+	}
+}
+
+static void qt602240_input_report(struct qt602240_data *data, int single_id)
+{
+	struct qt602240_finger *finger = data->finger;
+	struct input_dev *input_dev = data->input_dev;
+	int finger_num = 0;
+	int id;
+
+	for (id = 0; id < QT602240_MAX_FINGER; id++) {
+		if (!finger[id].status)
+			continue;
+
+		input_report_abs(input_dev, ABS_MT_TOUCH_MAJOR,
+				finger[id].area);
+
+		if (finger[id].status == QT602240_RELEASE)
+			finger[id].status = 0;
+		else {
+			input_report_abs(input_dev, ABS_MT_POSITION_X,
+					finger[id].x);
+			input_report_abs(input_dev, ABS_MT_POSITION_Y,
+					finger[id].y);
+			finger_num++;
+		}
+
+		input_mt_sync(input_dev);
+	}
+
+	input_report_key(input_dev, BTN_TOUCH, finger_num > 0);
+
+	if (finger[single_id].status != QT602240_RELEASE) {
+		input_report_abs(input_dev, ABS_X, finger[single_id].x);
+		input_report_abs(input_dev, ABS_Y, finger[single_id].y);
+	}
+
+	input_sync(input_dev);
+}
+
+static void qt602240_input_touchevent(struct qt602240_data *data,
+		struct qt602240_message *message, int id)
+{
+	struct qt602240_finger *finger = data->finger;
+	struct device *dev = &data->client->dev;
+	u8 status = message->message[0];
+	int x;
+	int y;
+	int area;
+
+	/* Check the touch is present on the screen */
+	if (!(status & QT602240_DETECT)) {
+		if (status & QT602240_RELEASE) {
+			dev_dbg(dev, "[%d] released\n", id);
+
+			finger[id].status = QT602240_RELEASE;
+			finger[id].area = 0;
+
+			qt602240_input_report(data, id);
+		}
+		return;
+	}
+
+	/* Check only AMP detection */
+	if (!(status & (QT602240_PRESS | QT602240_MOVE)))
+		return;
+
+	x = (message->message[1] << 2) | ((message->message[3] & ~0x3f) >> 6);
+	y = (message->message[2] << 2) | ((message->message[3] & ~0xf3) >> 2);
+	area = message->message[4];
+
+	dev_dbg(dev, "[%d] %s x: %d, y: %d, area: %d\n", id,
+			status & QT602240_MOVE ? "moved" : "pressed",
+			x, y, area);
+
+	finger[id].status = status & QT602240_MOVE ?
+		QT602240_MOVE : QT602240_PRESS;
+	finger[id].x = x;
+	finger[id].y = y;
+	finger[id].area = area;
+
+	qt602240_input_report(data, id);
+}
+
+static irqreturn_t qt602240_interrupt(int irq, void *dev_id)
+{
+	struct qt602240_data *data = dev_id;
+	struct qt602240_message message;
+	struct qt602240_object *object;
+	struct device *dev = &data->client->dev;
+	int id;
+	u8 reportid;
+	u8 max_reportid;
+	u8 min_reportid;
+
+	do {
+		if (qt602240_read_message(data, &message)) {
+			dev_err(dev, "Failed to read message\n");
+			goto end;
+		}
+
+		reportid = message.reportid;
+
+		/* whether reportid is thing of QT602240_TOUCH_MULTI */
+		object = qt602240_get_object(data, QT602240_TOUCH_MULTI);
+		if (!object)
+			goto end;
+
+		max_reportid = object->max_reportid;
+		min_reportid = max_reportid - object->num_report_ids + 1;
+		id = reportid - min_reportid;
+
+		if ((reportid >= min_reportid) && (reportid <= max_reportid))
+			qt602240_input_touchevent(data, &message, id);
+		else {
+			qt602240_dump_message(dev, &message);
+			qt602240_check_config_error(data, &message);
+		}
+	} while (reportid != 0xff);
+
+end:
+	return IRQ_HANDLED;
+}
+
+static void qt602240_resume_worker(struct work_struct *work)
+{
+	struct qt602240_data *data = container_of(work,
+			struct qt602240_data, ts_resume_work);
+
+	/* Soft reset */
+	qt602240_write_object(data, QT602240_GEN_COMMAND,
+			QT602240_COMMAND_RESET, 1);
+
+	msleep(QT602240_RESET_TIME);
+
+	if (data->input_dev->users)
+		/* Touch enable */
+		qt602240_write_object(data, QT602240_TOUCH_MULTI,
+				QT602240_TOUCH_CTRL, 0x83);
+}
+
+static int qt602240_check_reg_init(struct qt602240_data *data)
+{
+	struct qt602240_object *object;
+	struct device *dev = &data->client->dev;
+	int index = 0;
+	int i, j;
+	u8 version = data->info.version;
+	u8 *init_vals;
+
+	switch (version) {
+	case QT602240_VER_20:
+		init_vals = (u8 *)init_vals_ver_20;
+		break;
+	case QT602240_VER_21:
+		init_vals = (u8 *)init_vals_ver_21;
+		break;
+	case QT602240_VER_22:
+		init_vals = (u8 *)init_vals_ver_22;
+		break;
+	default:
+		dev_err(dev, "Firmware version %d doesn't support\n", version);
+		return -EINVAL;
+	}
+
+	for (i = 0; i < data->info.object_num; i++) {
+		object = data->object_table + i;
+
+		if (!qt602240_object_writable(object->type))
+			continue;
+
+		for (j = 0; j < object->size + 1; j++)
+			qt602240_write_object(data, object->type, j,
+					init_vals[index + j]);
+
+		index += object->size + 1;
+	}
+
+	return 0;
+}
+
+static int qt602240_check_matrix_size(struct qt602240_data *data)
+{
+	const struct qt602240_platform_data *pdata = data->pdata;
+	struct device *dev = &data->client->dev;
+	int val;
+	int mode = 0;
+
+	dev_dbg(dev, "Number of X lines: %d\n", pdata->x_line);
+	dev_dbg(dev, "Number of Y lines: %d\n", pdata->y_line);
+
+	switch (pdata->x_line) {
+	case 0 ... 15:
+		if (pdata->y_line <= 14)
+			mode = 0;
+		break;
+	case 16:
+		if (pdata->y_line <= 12)
+			mode = 1;
+		if (pdata->y_line == 13 || pdata->y_line == 14)
+			mode = 0;
+		break;
+	case 17:
+		if (pdata->y_line <= 11)
+			mode = 2;
+		if (pdata->y_line == 12 || pdata->y_line == 13)
+			mode = 1;
+		break;
+	case 18:
+		if (pdata->y_line <= 10)
+			mode = 3;
+		if (pdata->y_line == 11 || pdata->y_line == 12)
+			mode = 2;
+		break;
+	case 19:
+		if (pdata->y_line <= 9)
+			mode = 4;
+		if (pdata->y_line == 10 || pdata->y_line == 11)
+			mode = 3;
+		break;
+	case 20:
+		mode = 4;
+		break;
+	default:
+		dev_err(dev, "Invalid X/Y lines\n");
+		return -EINVAL;
+	}
+
+	val = qt602240_read_object(data, QT602240_SPT_CTECONFIG,
+			QT602240_CTE_MODE);
+	if (val < 0)
+		return -EIO;
+
+	if (mode == val)
+		return 0;
+
+	/* Change the CTE configuration */
+	qt602240_write_object(data, QT602240_SPT_CTECONFIG,
+			QT602240_CTE_CTRL, 1);
+	qt602240_write_object(data, QT602240_SPT_CTECONFIG,
+			QT602240_CTE_MODE, mode);
+	qt602240_write_object(data, QT602240_SPT_CTECONFIG,
+			QT602240_CTE_CTRL, 0);
+
+	return 0;
+}
+
+static void qt602240_handle_pdata(struct qt602240_data *data)
+{
+	const struct qt602240_platform_data *pdata = data->pdata;
+	u8 voltage;
+
+	/* Set touchscreen lines */
+	qt602240_write_object(data, QT602240_TOUCH_MULTI, QT602240_TOUCH_XSIZE,
+			pdata->x_line);
+	qt602240_write_object(data, QT602240_TOUCH_MULTI, QT602240_TOUCH_YSIZE,
+			pdata->y_line);
+
+	/* Set touchscreen orient */
+	qt602240_write_object(data, QT602240_TOUCH_MULTI, QT602240_TOUCH_ORIENT,
+			pdata->orient);
+
+	/* Set touchscreen burst length */
+	qt602240_write_object(data, QT602240_TOUCH_MULTI,
+			QT602240_TOUCH_BLEN, pdata->blen);
+
+	/* Set touchscreen threshold */
+	qt602240_write_object(data, QT602240_TOUCH_MULTI,
+			QT602240_TOUCH_TCHTHR, pdata->threshold);
+
+	/* Set touchscreen detect integration */
+	if (pdata->tchdi)
+		qt602240_write_object(data, QT602240_TOUCH_MULTI,
+				QT602240_TOUCH_TCHDI, pdata->tchdi);
+
+	/* Set touchscreen resolution */
+	qt602240_write_object(data, QT602240_TOUCH_MULTI,
+			QT602240_TOUCH_XRANGE_LSB, (pdata->x_size - 1) & 0xff);
+	qt602240_write_object(data, QT602240_TOUCH_MULTI,
+			QT602240_TOUCH_XRANGE_MSB, (pdata->x_size - 1) >> 8);
+	qt602240_write_object(data, QT602240_TOUCH_MULTI,
+			QT602240_TOUCH_YRANGE_LSB, (pdata->y_size - 1) & 0xff);
+	qt602240_write_object(data, QT602240_TOUCH_MULTI,
+			QT602240_TOUCH_YRANGE_MSB, (pdata->y_size - 1) >> 8);
+
+	/* Set touchscreen voltage */
+	if (data->info.version >= QT602240_VER_21 && pdata->voltage) {
+		if (pdata->voltage < QT602240_VOLTAGE_DEFAULT) {
+			voltage = (QT602240_VOLTAGE_DEFAULT - pdata->voltage) /
+				QT602240_VOLTAGE_STEP;
+			voltage = 0xff - voltage + 1;
+		} else
+			voltage = (pdata->voltage - QT602240_VOLTAGE_DEFAULT) /
+				QT602240_VOLTAGE_STEP;
+
+		qt602240_write_object(data, QT602240_SPT_CTECONFIG,
+				QT602240_CTE_VOLTAGE, voltage);
+	}
+}
+
+static int qt602240_get_info(struct qt602240_data *data)
+{
+	struct i2c_client *client = data->client;
+	struct qt602240_info *info = &data->info;
+	int val;
+
+	val = qt602240_read_reg(client, QT602240_FAMILY_ID);
+	if (val < 0)
+		return -EIO;
+	info->family_id = val;
+
+	val = qt602240_read_reg(client, QT602240_VARIANT_ID);
+	if (val < 0)
+		return -EIO;
+	info->variant_id = val;
+
+	val = qt602240_read_reg(client, QT602240_VERSION);
+	if (val < 0)
+		return -EIO;
+	info->version = val;
+
+	val = qt602240_read_reg(client, QT602240_BUILD);
+	if (val < 0)
+		return -EIO;
+	info->build = val;
+
+	val = qt602240_read_reg(client, QT602240_OBJECT_NUM);
+	if (val < 0)
+		return -EIO;
+	info->object_num = val;
+
+	return 0;
+}
+
+static int qt602240_get_object_table(struct qt602240_data *data)
+{
+	int ret;
+	int i;
+	u16 reg;
+	u8 reportid = 0;
+	u8 buf[QT602240_OBJECT_SIZE];
+
+	for (i = 0; i < data->info.object_num; i++) {
+		struct qt602240_object *object = data->object_table + i;
+
+		reg = QT602240_OBJECT_START + QT602240_OBJECT_SIZE * i;
+		ret = qt602240_read_object_table(data->client, reg, buf);
+		if (ret)
+			return ret;
+
+		object->type = buf[0];
+		object->start_address = (buf[2] << 8) | buf[1];
+		object->size = buf[3];
+		object->instances = buf[4];
+		object->num_report_ids = buf[5];
+
+		if (object->num_report_ids) {
+			reportid += object->num_report_ids *
+				(object->instances + 1);
+			object->max_reportid = reportid;
+		}
+	}
+
+	return 0;
+}
+
+static int qt602240_initialize(struct qt602240_data *data)
+{
+	struct i2c_client *client = data->client;
+	struct qt602240_info *info = &data->info;
+	int ret;
+
+	ret = qt602240_get_info(data);
+	if (ret < 0)
+		return ret;
+
+	data->object_table =
+		kzalloc(sizeof(struct qt602240_data) * info->object_num,
+				GFP_KERNEL);
+	if (!data->object_table) {
+		dev_err(&client->dev, "Failed to allocate memory\n");
+		return -ENOMEM;
+	}
+
+	/* Get object table information */
+	ret = qt602240_get_object_table(data);
+	if (ret < 0)
+		return ret;
+
+	/* Check register init values */
+	ret = qt602240_check_reg_init(data);
+	if (ret < 0)
+		return ret;
+
+	/* Check X/Y matrix size */
+	ret = qt602240_check_matrix_size(data);
+	if (ret < 0)
+		return ret;
+
+	/* Read dummy message to make high CHG pin */
+	do {
+		ret = qt602240_read_object(data, QT602240_GEN_MESSAGE, 0);
+		if (ret < 0)
+			return ret;
+	} while (ret != 0xff);
+
+	qt602240_handle_pdata(data);
+
+	/* Backup to memory */
+	qt602240_write_object(data, QT602240_GEN_COMMAND,
+			QT602240_COMMAND_BACKUPNV,
+			QT602240_BACKUP_VALUE);
+	msleep(QT602240_BACKUP_TIME);
+
+	/* Soft reset */
+	qt602240_write_object(data, QT602240_GEN_COMMAND,
+			QT602240_COMMAND_RESET, 1);
+	msleep(QT602240_RESET_TIME);
+
+	/* Update matrix size at info struct */
+	ret = qt602240_read_reg(client, QT602240_MATRIX_X_SIZE);
+	if (ret < 0)
+		return ret;
+	info->matrix_xsize = ret;
+
+	ret = qt602240_read_reg(client, QT602240_MATRIX_Y_SIZE);
+	if (ret < 0)
+		return ret;
+	info->matrix_ysize = ret;
+
+	dev_info(&client->dev,
+			"Family ID: %d Variant ID: %d Version: %d Build: %d\n",
+			info->family_id, info->variant_id, info->version,
+			info->build);
+
+	dev_info(&client->dev,
+			"Matrix X Size: %d Matrix Y Size: %d Object Num: %d\n",
+			info->matrix_xsize, info->matrix_ysize,
+			info->object_num);
+
+	return 0;
+}
+
+static ssize_t qt602240_object_show(struct device *dev,
+		struct device_attribute *attr, char *buf)
+{
+	struct qt602240_data *data = dev_get_drvdata(dev);
+	struct qt602240_object *object;
+	int val;
+	int count = 0;
+	int i, j;
+
+	if (!data) {
+		dev_err(dev, "The data is NULL\n");
+		return -EFAULT;
+	}
+
+	for (i = 0; i < data->info.object_num; i++) {
+		object = data->object_table + i;
+
+		count += sprintf(buf + count,
+				"Object Table Element %d(Type %d)\n",
+				i + 1, object->type);
+
+		if (!qt602240_object_readable(object->type)) {
+			count += sprintf(buf + count, "\n");
+			continue;
+		}
+
+		for (j = 0; j < object->size + 1; j++) {
+			val = qt602240_read_object(data, object->type, j);
+			if (val < 0)
+				return count;
+
+			count += sprintf(buf + count,
+					"  Byte %d: 0x%x (%d)\n", j, val, val);
+		}
+
+		count += sprintf(buf + count, "\n");
+	}
+
+	return count;
+}
+
+static int qt602240_load_fw(struct device *dev, const char *fn)
+{
+	struct qt602240_data *data = dev_get_drvdata(dev);
+	struct i2c_client *client = data->client;
+	const struct firmware *fw = NULL;
+	unsigned int frame_size;
+	unsigned int pos = 0;
+	int ret;
+
+	if (!data) {
+		dev_err(dev, "The data is NULL\n");
+		return -EFAULT;
+	}
+
+	ret = request_firmware(&fw, fn, dev);
+	if (ret < 0) {
+		dev_err(dev, "Unable to open firmware %s\n", fn);
+		return -ENOMEM;
+	}
+
+	/* Change to the bootloader mode */
+	qt602240_write_object(data, QT602240_GEN_COMMAND,
+			QT602240_COMMAND_RESET, QT602240_BOOT_VALUE);
+	msleep(QT602240_RESET_TIME);
+
+	/* Change to slave address of bootloader */
+	if (client->addr == QT602240_APP_LOW)
+		client->addr = QT602240_BOOT_LOW;
+	else
+		client->addr = QT602240_BOOT_HIGH;
+
+	ret = qt602240_check_bootloader(client, QT602240_WAITING_BOOTLOAD_CMD);
+	if (ret < 0)
+		goto err_fw;
+
+	/* Unlock bootloader */
+	qt602240_unlock_bootloader(client);
+
+	while (pos < fw->size) {
+		ret = qt602240_check_bootloader(client,
+				QT602240_WAITING_FRAME_DATA);
+		if (ret < 0)
+			goto err_fw;
+
+		frame_size = ((*(fw->data + pos) << 8) | *(fw->data + pos + 1));
+
+		/* We should add 2 at frame size as the the firmware data is not
+		 * included the CRC bytes.
+		 */
+		frame_size += 2;
+
+		/* Write one frame to device */
+		qt602240_fw_write(client, fw->data + pos, frame_size);
+
+		ret = qt602240_check_bootloader(client,
+				QT602240_FRAME_CRC_PASS);
+		if (ret < 0)
+			goto err_fw;
+
+		pos += frame_size;
+
+		dev_dbg(dev, "Updated %zd bytes / %zd bytes\n", pos, fw->size);
+	}
+
+err_fw:
+	/* Change to slave address of application */
+	if (client->addr == QT602240_BOOT_LOW)
+		client->addr = QT602240_APP_LOW;
+	else
+		client->addr = QT602240_APP_HIGH;
+
+	return ret;
+}
+
+static ssize_t qt602240_update_fw_store(struct device *dev,
+		struct device_attribute *attr, const char *buf, size_t count)
+{
+	struct qt602240_data *data = dev_get_drvdata(dev);
+	unsigned int version;
+	int ret;
+
+	if (sscanf(buf, "%u", &version) != 1) {
+		dev_err(dev, "Invalid values\n");
+		return -EINVAL;
+	}
+
+	/* Firmware update supports from version 21 */
+	if ((data->info.version < QT602240_VER_21) ||
+			(version < QT602240_VER_21)) {
+		dev_err(dev, "Firmware update supports from version 21\n");
+		return -EINVAL;
+	}
+
+	/* Interrupt disable */
+	disable_irq(data->irq);
+
+	ret = qt602240_load_fw(dev, QT602240_FW_NAME);
+	if (ret < 0)
+		dev_err(dev, "The firmware update failed(%d)\n", ret);
+	else {
+		dev_dbg(dev, "The firmware update successed\n");
+
+		/* Wait for reset */
+		msleep(QT602240_FWRESET_TIME);
+
+		kfree(data->object_table);
+
+		qt602240_initialize(data);
+	}
+
+	/* Interrupt enable */
+	enable_irq(data->irq);
+
+	return count;
+}
+
+static DEVICE_ATTR(object, 0444, qt602240_object_show, NULL);
+static DEVICE_ATTR(update_fw, 0664, NULL, qt602240_update_fw_store);
+
+static struct attribute *qt602240_attrs[] = {
+	&dev_attr_object.attr,
+	&dev_attr_update_fw.attr,
+	NULL
+};
+
+static const struct attribute_group qt602240_attr_group = {
+	.attrs = qt602240_attrs,
+};
+
+static int qt602240_input_open(struct input_dev *dev)
+{
+	struct qt602240_data *data = input_get_drvdata(dev);
+
+	/* Touch enable */
+	qt602240_write_object(data, QT602240_TOUCH_MULTI, QT602240_TOUCH_CTRL,
+			0x83);
+
+	return 0;
+}
+
+static void qt602240_input_close(struct input_dev *dev)
+{
+	struct qt602240_data *data = input_get_drvdata(dev);
+
+	/* Touch disable */
+	qt602240_write_object(data, QT602240_TOUCH_MULTI, QT602240_TOUCH_CTRL,
+			0);
+}
+
+static int __devinit qt602240_probe(struct i2c_client *client,
+		const struct i2c_device_id *id)
+{
+	struct qt602240_data *data;
+	struct input_dev *input_dev;
+	int ret;
+
+	if (!client->dev.platform_data)
+		return -EINVAL;
+
+	data = kzalloc(sizeof(struct qt602240_data), GFP_KERNEL);
+
+	input_dev = input_allocate_device();
+	if (!data || !input_dev) {
+		dev_err(&client->dev, "Failed to allocate memory\n");
+		ret = -ENOMEM;
+		goto err_free_mem;
+	}
+
+	input_dev->name = "AT42QT602240/ATMXT224 Touchscreen";
+	input_dev->id.bustype = BUS_I2C;
+	input_dev->dev.parent = &client->dev;
+	input_dev->open = qt602240_input_open;
+	input_dev->close = qt602240_input_close;
+
+	__set_bit(EV_ABS, input_dev->evbit);
+	__set_bit(EV_KEY, input_dev->evbit);
+	__set_bit(BTN_TOUCH, input_dev->keybit);
+
+	/* For single touch */
+	input_set_abs_params(input_dev, ABS_X, 0, QT602240_MAX_XC, 0,
+			0);
+	input_set_abs_params(input_dev, ABS_Y, 0, QT602240_MAX_YC, 0,
+			0);
+
+	/* For multi touch */
+	input_set_abs_params(input_dev, ABS_MT_TOUCH_MAJOR, 0,
+			QT602240_MAX_AREA, 0, 0);
+	input_set_abs_params(input_dev, ABS_MT_POSITION_X, 0,
+			QT602240_MAX_XC, 0, 0);
+	input_set_abs_params(input_dev, ABS_MT_POSITION_Y, 0,
+			QT602240_MAX_YC, 0, 0);
+
+	input_set_drvdata(input_dev, data);
+
+	INIT_WORK(&data->ts_resume_work, qt602240_resume_worker);
+	data->client = client;
+	data->input_dev = input_dev;
+	data->pdata = client->dev.platform_data;
+	data->irq = client->irq;
+
+	i2c_set_clientdata(client, data);
+
+	ret = qt602240_initialize(data);
+	if (ret < 0)
+		goto err_free_object;
+
+	ret = request_threaded_irq(client->irq, NULL, qt602240_interrupt,
+			IRQF_TRIGGER_FALLING, client->dev.driver->name, data);
+
+	if (ret < 0) {
+		dev_err(&client->dev, "Failed to register interrupt\n");
+		goto err_free_object;
+	}
+
+	ret = input_register_device(input_dev);
+	if (ret < 0)
+		goto err_free_irq;
+
+	ret = sysfs_create_group(&client->dev.kobj, &qt602240_attr_group);
+	if (ret)
+		goto err_unregister_device;
+
+	return 0;
+
+err_unregister_device:
+	input_unregister_device(data->input_dev);
+err_free_irq:
+	free_irq(client->irq, data);
+err_free_object:
+	kfree(data->object_table);
+err_free_mem:
+	input_free_device(input_dev);
+	kfree(data);
+	return ret;
+}
+
+static int __devexit qt602240_remove(struct i2c_client *client)
+{
+	struct qt602240_data *data = i2c_get_clientdata(client);
+
+	free_irq(data->irq, data);
+	cancel_work_sync(&data->ts_resume_work);
+	input_unregister_device(data->input_dev);
+	kfree(data->object_table);
+	kfree(data);
+
+	sysfs_remove_group(&client->dev.kobj, &qt602240_attr_group);
+
+	return 0;
+}
+
+#ifdef CONFIG_PM
+static int qt602240_suspend(struct i2c_client *client, pm_message_t mesg)
+{
+	struct qt602240_data *data = i2c_get_clientdata(client);
+
+	if (data->input_dev->users)
+		/* Touch disable */
+		qt602240_write_object(data, QT602240_TOUCH_MULTI,
+				QT602240_TOUCH_CTRL, 0);
+
+	return 0;
+}
+
+static int qt602240_resume(struct i2c_client *client)
+{
+	struct qt602240_data *data = i2c_get_clientdata(client);
+
+	schedule_work(&data->ts_resume_work);
+
+	return 0;
+}
+#else
+#define qt602240_suspend	NULL
+#define qt602240_resume		NULL
+#endif
+
+static const struct i2c_device_id qt602240_id[] = {
+	{ "qt602240_ts", 0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, qt602240_id);
+
+static struct i2c_driver qt602240_driver = {
+	.driver = {
+		.name = "qt602240_ts",
+	},
+	.probe		= qt602240_probe,
+	.remove		= __devexit_p(qt602240_remove),
+	.suspend	= qt602240_suspend,
+	.resume		= qt602240_resume,
+	.id_table	= qt602240_id,
+};
+
+static int __init qt602240_init(void)
+{
+	return i2c_add_driver(&qt602240_driver);
+}
+
+static void __exit qt602240_exit(void)
+{
+	i2c_del_driver(&qt602240_driver);
+}
+
+module_init(qt602240_init);
+module_exit(qt602240_exit);
+
+/* Module information */
+MODULE_AUTHOR("Joonyoung Shim <jy0922.shim@samsung.com>");
+MODULE_DESCRIPTION("AT42QT602240/ATMXT224 Touchscreen driver");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/i2c/qt602240_ts.h b/include/linux/i2c/qt602240_ts.h
new file mode 100644
index 0000000..3a8bf19
--- /dev/null
+++ b/include/linux/i2c/qt602240_ts.h
@@ -0,0 +1,40 @@
+/*
+ * qt602240_ts.h - AT42QT602240/ATMXT224 Touchscreen driver
+ *
+ * Copyright (C) 2010 Samsung Electronics Co.Ltd
+ * Author: Joonyoung Shim <jy0922.shim@samsung.com>
+ *
+ *  This program is free software; you can redistribute  it and/or modify it
+ *  under  the terms of  the GNU General  Public License as published by the
+ *  Free Software Foundation;  either version 2 of the  License, or (at your
+ *  option) any later version.
+ *
+ */
+
+#ifndef __LINUX_QT602240_TS_H
+#define __LINUX_QT602240_TS_H
+
+/* Orient */
+#define QT602240_NORMAL			0x0
+#define QT602240_DIAGONAL		0x1
+#define QT602240_HORIZONTAL_FLIP	0x2
+#define QT602240_ROTATED_90_COUNTER	0x3
+#define QT602240_VERTICAL_FLIP		0x4
+#define QT602240_ROTATED_90		0x5
+#define QT602240_ROTATED_180		0x6
+#define QT602240_DIAGONAL_COUNTER	0x7
+
+/* The platform data for the AT42QT602240/ATMXT224 touchscreen driver */
+struct qt602240_platform_data {
+	unsigned int x_line;
+	unsigned int y_line;
+	unsigned int x_size;
+	unsigned int y_size;
+	unsigned int blen;
+	unsigned int threshold;
+	unsigned int tchdi;
+	unsigned int voltage;
+	unsigned char orient;
+};
+
+#endif /* __LINUX_QT602240_TS_H */
-- 
1.7.0.4


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

* Re: [PATCH v2] input: qt602240 - Add ATMEL QT602240 touchscreen driver
  2010-06-25  1:54 [PATCH v2] input: qt602240 - Add ATMEL QT602240 touchscreen driver Joonyoung Shim
@ 2010-06-25 14:08 ` Henrik Rydberg
  2010-06-28  5:16   ` Joonyoung Shim
  0 siblings, 1 reply; 21+ messages in thread
From: Henrik Rydberg @ 2010-06-25 14:08 UTC (permalink / raw)
  To: Joonyoung Shim; +Cc: dmitry.torokhov, linux-input, kyungmin.park

Hi Joonyoung,

some follow-up comments on the MT events.

> +static void qt602240_input_report(struct qt602240_data *data, int single_id)
> +{
> +	struct qt602240_finger *finger = data->finger;
> +	struct input_dev *input_dev = data->input_dev;
> +	int finger_num = 0;
> +	int id;
> +
> +	for (id = 0; id < QT602240_MAX_FINGER; id++) {
> +		if (!finger[id].status)
> +			continue;
> +
> +		input_report_abs(input_dev, ABS_MT_TOUCH_MAJOR,
> +				finger[id].area);
> +
> +		if (finger[id].status == QT602240_RELEASE)
> +			finger[id].status = 0;
> +		else {
> +			input_report_abs(input_dev, ABS_MT_POSITION_X,
> +					finger[id].x);
> +			input_report_abs(input_dev, ABS_MT_POSITION_Y,
> +					finger[id].y);
> +			finger_num++;
> +		}
> +
> +		input_mt_sync(input_dev);
> +	}
> +
> +	input_report_key(input_dev, BTN_TOUCH, finger_num > 0);
> +
> +	if (finger[single_id].status != QT602240_RELEASE) {
> +		input_report_abs(input_dev, ABS_X, finger[single_id].x);
> +		input_report_abs(input_dev, ABS_Y, finger[single_id].y);
> +	}
> +
> +	input_sync(input_dev);
> +}

The problem still persists here. I will try to explain in code form instead:

for (id = 0; id < QT602240_MAX_FINGER; id++) {
	if (!finger[id].status)
		continue;

	input_report_abs(input_dev, ABS_MT_TOUCH_MAJOR,
		finger[id].status != QT602240_RELEASE ? finger[id].area : 0);
	input_report_abs(input_dev, ABS_MT_POSITION_X,
			finger[id].x);
	input_report_abs(input_dev, ABS_MT_POSITION_Y,
			finger[id].y);
	input_mt_sync(input_dev);

	if (finger[id].status == QT602240_RELEASE)
		finger[id].status = 0;
	else
		finger_num++;
}

Regarding the single_id, I can understand the need for it, but the logic for a
single touch is slightly confusing. If single_id is to be interpreted as the
contact currently being tracked as the single pointer, and that single_id
changes as the number of fingers on the pad is reduced, until there is only one
left, then it would still be clearer logically to do something like this:

if (finger_num > 0) {
	input_report_key(input_dev, BTN_TOUCH, 1);
	input_report_abs(input_dev, ABS_X, finger[single_id].x);
	input_report_abs(input_dev, ABS_Y, finger[single_id].y);
} else {
	input_report_key(input_dev, BTN_TOUCH, 0);
}
input_sync(input_dev);

Would it not?

Thanks,
Henrik

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

* Re: [PATCH v2] input: qt602240 - Add ATMEL QT602240 touchscreen driver
  2010-06-25 14:08 ` Henrik Rydberg
@ 2010-06-28  5:16   ` Joonyoung Shim
  2010-06-28  7:37     ` Henrik Rydberg
  0 siblings, 1 reply; 21+ messages in thread
From: Joonyoung Shim @ 2010-06-28  5:16 UTC (permalink / raw)
  To: Henrik Rydberg; +Cc: dmitry.torokhov, linux-input, kyungmin.park

On 6/25/2010 11:08 PM, Henrik Rydberg wrote:
> Hi Joonyoung,
> 
> some follow-up comments on the MT events.
> 
>> +static void qt602240_input_report(struct qt602240_data *data, int single_id)
>> +{
>> +	struct qt602240_finger *finger = data->finger;
>> +	struct input_dev *input_dev = data->input_dev;
>> +	int finger_num = 0;
>> +	int id;
>> +
>> +	for (id = 0; id < QT602240_MAX_FINGER; id++) {
>> +		if (!finger[id].status)
>> +			continue;
>> +
>> +		input_report_abs(input_dev, ABS_MT_TOUCH_MAJOR,
>> +				finger[id].area);
>> +
>> +		if (finger[id].status == QT602240_RELEASE)
>> +			finger[id].status = 0;
>> +		else {
>> +			input_report_abs(input_dev, ABS_MT_POSITION_X,
>> +					finger[id].x);
>> +			input_report_abs(input_dev, ABS_MT_POSITION_Y,
>> +					finger[id].y);
>> +			finger_num++;
>> +		}
>> +
>> +		input_mt_sync(input_dev);
>> +	}
>> +
>> +	input_report_key(input_dev, BTN_TOUCH, finger_num > 0);
>> +
>> +	if (finger[single_id].status != QT602240_RELEASE) {
>> +		input_report_abs(input_dev, ABS_X, finger[single_id].x);
>> +		input_report_abs(input_dev, ABS_Y, finger[single_id].y);
>> +	}
>> +
>> +	input_sync(input_dev);
>> +}
> 
> The problem still persists here. I will try to explain in code form instead:
> 
> for (id = 0; id < QT602240_MAX_FINGER; id++) {
> 	if (!finger[id].status)
> 		continue;
> 
> 	input_report_abs(input_dev, ABS_MT_TOUCH_MAJOR,
> 		finger[id].status != QT602240_RELEASE ? finger[id].area : 0);
> 	input_report_abs(input_dev, ABS_MT_POSITION_X,
> 			finger[id].x);
> 	input_report_abs(input_dev, ABS_MT_POSITION_Y,
> 			finger[id].y);

Hmm, is it OK to report ABS_MT_POSITION_X/Y when releases?

> 	input_mt_sync(input_dev);
> 
> 	if (finger[id].status == QT602240_RELEASE)
> 		finger[id].status = 0;
> 	else
> 		finger_num++;
> }
> 
> Regarding the single_id, I can understand the need for it, but the logic for a
> single touch is slightly confusing. If single_id is to be interpreted as the
> contact currently being tracked as the single pointer, and that single_id
> changes as the number of fingers on the pad is reduced, until there is only one
> left, then it would still be clearer logically to do something like this:
> 
> if (finger_num > 0) {
> 	input_report_key(input_dev, BTN_TOUCH, 1);
> 	input_report_abs(input_dev, ABS_X, finger[single_id].x);
> 	input_report_abs(input_dev, ABS_Y, finger[single_id].y);
> } else {
> 	input_report_key(input_dev, BTN_TOUCH, 0);
> }
> input_sync(input_dev);
> 
> Would it not?
> 

There is a case which fingers more than one are touched and current
status of single_id finger is release, then this will report ABS_X/Y 
events even if it is the release event.

How about codes like this? :

+       int status = finger[single_id].status;

[snip]

+       if (finger_num > 0) {
+               if (status != QT602240_RELEASE) {
+                       input_report_key(input_dev, BTN_TOUCH, 1);
+                       input_report_abs(input_dev, ABS_X, finger[single_id].x);
+                       input_report_abs(input_dev, ABS_Y, finger[single_id].y);
+               }
+       } else
+               input_report_key(input_dev, BTN_TOUCH, 0);

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

* Re: [PATCH v2] input: qt602240 - Add ATMEL QT602240 touchscreen driver
  2010-06-28  5:16   ` Joonyoung Shim
@ 2010-06-28  7:37     ` Henrik Rydberg
  2010-06-28  8:17       ` Joonyoung Shim
  0 siblings, 1 reply; 21+ messages in thread
From: Henrik Rydberg @ 2010-06-28  7:37 UTC (permalink / raw)
  To: Joonyoung Shim; +Cc: dmitry.torokhov, linux-input, kyungmin.park

Joonyoung Shim wrote:
> On 6/25/2010 11:08 PM, Henrik Rydberg wrote:
>> Hi Joonyoung,
>>
>> some follow-up comments on the MT events.
>>
>>> +static void qt602240_input_report(struct qt602240_data *data, int single_id)
>>> +{
>>> +	struct qt602240_finger *finger = data->finger;
>>> +	struct input_dev *input_dev = data->input_dev;
>>> +	int finger_num = 0;
>>> +	int id;
>>> +
>>> +	for (id = 0; id < QT602240_MAX_FINGER; id++) {
>>> +		if (!finger[id].status)
>>> +			continue;
>>> +
>>> +		input_report_abs(input_dev, ABS_MT_TOUCH_MAJOR,
>>> +				finger[id].area);
>>> +
>>> +		if (finger[id].status == QT602240_RELEASE)
>>> +			finger[id].status = 0;
>>> +		else {
>>> +			input_report_abs(input_dev, ABS_MT_POSITION_X,
>>> +					finger[id].x);
>>> +			input_report_abs(input_dev, ABS_MT_POSITION_Y,
>>> +					finger[id].y);
>>> +			finger_num++;
>>> +		}
>>> +
>>> +		input_mt_sync(input_dev);
>>> +	}
>>> +
>>> +	input_report_key(input_dev, BTN_TOUCH, finger_num > 0);
>>> +
>>> +	if (finger[single_id].status != QT602240_RELEASE) {
>>> +		input_report_abs(input_dev, ABS_X, finger[single_id].x);
>>> +		input_report_abs(input_dev, ABS_Y, finger[single_id].y);
>>> +	}
>>> +
>>> +	input_sync(input_dev);
>>> +}
>> The problem still persists here. I will try to explain in code form instead:
>>
>> for (id = 0; id < QT602240_MAX_FINGER; id++) {
>> 	if (!finger[id].status)
>> 		continue;
>>
>> 	input_report_abs(input_dev, ABS_MT_TOUCH_MAJOR,
>> 		finger[id].status != QT602240_RELEASE ? finger[id].area : 0);
>> 	input_report_abs(input_dev, ABS_MT_POSITION_X,
>> 			finger[id].x);
>> 	input_report_abs(input_dev, ABS_MT_POSITION_Y,
>> 			finger[id].y);
> 
> Hmm, is it OK to report ABS_MT_POSITION_X/Y when releases?

Yes. The position should be the position where the finger left the surface. It
is different from the ABS_X/Y case simply because the type A protocol exchanges
data statelessly.

>> 	input_mt_sync(input_dev);
>>
>> 	if (finger[id].status == QT602240_RELEASE)
>> 		finger[id].status = 0;
>> 	else
>> 		finger_num++;
>> }
>>
>> Regarding the single_id, I can understand the need for it, but the logic for a
>> single touch is slightly confusing. If single_id is to be interpreted as the
>> contact currently being tracked as the single pointer, and that single_id
>> changes as the number of fingers on the pad is reduced, until there is only one
>> left, then it would still be clearer logically to do something like this:
>>
>> if (finger_num > 0) {
>> 	input_report_key(input_dev, BTN_TOUCH, 1);
>> 	input_report_abs(input_dev, ABS_X, finger[single_id].x);
>> 	input_report_abs(input_dev, ABS_Y, finger[single_id].y);
>> } else {
>> 	input_report_key(input_dev, BTN_TOUCH, 0);
>> }
>> input_sync(input_dev);
>>
>> Would it not?
>>
> 
> There is a case which fingers more than one are touched and current
> status of single_id finger is release, then this will report ABS_X/Y 
> events even if it is the release event.

I see. And you want BTN_TOUCH to follow the logic for the single touch? I think
that is the main issue here. We can have _one_ of the following definitions, but
not both:

1. input_report_key(input_dev, BTN_TOUCH, finger_num > 0);

2. input_report_key(input_dev, BTN_TOUCH,
                    finger[single_id].status != QT602240_RELEASE);

If you use the latter, there should be another event to denote the finger_num ==
0 case. This line at the end should do it:

if (finger_num == 0)
	input_mt_sync(input_dev);

Thanks,
Henrik


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

* Re: [PATCH v2] input: qt602240 - Add ATMEL QT602240 touchscreen driver
  2010-06-28  7:37     ` Henrik Rydberg
@ 2010-06-28  8:17       ` Joonyoung Shim
  2010-06-28  8:34         ` Henrik Rydberg
  0 siblings, 1 reply; 21+ messages in thread
From: Joonyoung Shim @ 2010-06-28  8:17 UTC (permalink / raw)
  To: Henrik Rydberg; +Cc: dmitry.torokhov, linux-input, kyungmin.park

On 6/28/2010 4:37 PM, Henrik Rydberg wrote:
> Joonyoung Shim wrote:
>> On 6/25/2010 11:08 PM, Henrik Rydberg wrote:
>>> Hi Joonyoung,
>>>
>>> some follow-up comments on the MT events.
>>>
>>>> +static void qt602240_input_report(struct qt602240_data *data, int single_id)
>>>> +{
>>>> +	struct qt602240_finger *finger = data->finger;
>>>> +	struct input_dev *input_dev = data->input_dev;
>>>> +	int finger_num = 0;
>>>> +	int id;
>>>> +
>>>> +	for (id = 0; id < QT602240_MAX_FINGER; id++) {
>>>> +		if (!finger[id].status)
>>>> +			continue;
>>>> +
>>>> +		input_report_abs(input_dev, ABS_MT_TOUCH_MAJOR,
>>>> +				finger[id].area);
>>>> +
>>>> +		if (finger[id].status == QT602240_RELEASE)
>>>> +			finger[id].status = 0;
>>>> +		else {
>>>> +			input_report_abs(input_dev, ABS_MT_POSITION_X,
>>>> +					finger[id].x);
>>>> +			input_report_abs(input_dev, ABS_MT_POSITION_Y,
>>>> +					finger[id].y);
>>>> +			finger_num++;
>>>> +		}
>>>> +
>>>> +		input_mt_sync(input_dev);
>>>> +	}
>>>> +
>>>> +	input_report_key(input_dev, BTN_TOUCH, finger_num > 0);
>>>> +
>>>> +	if (finger[single_id].status != QT602240_RELEASE) {
>>>> +		input_report_abs(input_dev, ABS_X, finger[single_id].x);
>>>> +		input_report_abs(input_dev, ABS_Y, finger[single_id].y);
>>>> +	}
>>>> +
>>>> +	input_sync(input_dev);
>>>> +}
>>> The problem still persists here. I will try to explain in code form instead:
>>>
>>> for (id = 0; id < QT602240_MAX_FINGER; id++) {
>>> 	if (!finger[id].status)
>>> 		continue;
>>>
>>> 	input_report_abs(input_dev, ABS_MT_TOUCH_MAJOR,
>>> 		finger[id].status != QT602240_RELEASE ? finger[id].area : 0);
>>> 	input_report_abs(input_dev, ABS_MT_POSITION_X,
>>> 			finger[id].x);
>>> 	input_report_abs(input_dev, ABS_MT_POSITION_Y,
>>> 			finger[id].y);
>> Hmm, is it OK to report ABS_MT_POSITION_X/Y when releases?
> 
> Yes. The position should be the position where the finger left the surface. It
> is different from the ABS_X/Y case simply because the type A protocol exchanges
> data statelessly.
> 

I see, but i have something wondering at your document.

This is your patch of "Document the MT event slot protocol"

+Protocol Example A
+------------------
+
+Here is what a minimal event sequence for a two-contact touch would look
+like for a type A device:
+
+   ABS_MT_POSITION_X x[0]
+   ABS_MT_POSITION_Y y[0]
+   SYN_MT_REPORT
+   ABS_MT_POSITION_X x[1]
+   ABS_MT_POSITION_Y y[1]
+   SYN_MT_REPORT
+   SYN_REPORT

+The sequence after moving one of the contacts looks exactly the same; the
+raw data for all present contacts are sent between every synchronization
+with SYN_REPORT.

-Usage
------
+Here is the sequence after lifting the first contact:
+
+   ABS_MT_POSITION_X x[1]
+   ABS_MT_POSITION_Y y[1]
+   SYN_MT_REPORT
+   SYN_REPORT
+
+And here is the sequence after lifting the second contact:
+
+   SYN_MT_REPORT
+   SYN_REPORT
+

Here, there is no reporting for ABS_MT_POSITION_X/Y event, because that
is the last contact?
Then, the coordinates of the first contact are x[1] and y[1], right? If 
yes, it is some confusing, i think they are x[0] and y[0].

>>> 	input_mt_sync(input_dev);
>>>
>>> 	if (finger[id].status == QT602240_RELEASE)
>>> 		finger[id].status = 0;
>>> 	else
>>> 		finger_num++;
>>> }
>>>
>>> Regarding the single_id, I can understand the need for it, but the logic for a
>>> single touch is slightly confusing. If single_id is to be interpreted as the
>>> contact currently being tracked as the single pointer, and that single_id
>>> changes as the number of fingers on the pad is reduced, until there is only one
>>> left, then it would still be clearer logically to do something like this:
>>>
>>> if (finger_num > 0) {
>>> 	input_report_key(input_dev, BTN_TOUCH, 1);
>>> 	input_report_abs(input_dev, ABS_X, finger[single_id].x);
>>> 	input_report_abs(input_dev, ABS_Y, finger[single_id].y);
>>> } else {
>>> 	input_report_key(input_dev, BTN_TOUCH, 0);
>>> }
>>> input_sync(input_dev);
>>>
>>> Would it not?
>>>
>> There is a case which fingers more than one are touched and current
>> status of single_id finger is release, then this will report ABS_X/Y 
>> events even if it is the release event.
> 
> I see. And you want BTN_TOUCH to follow the logic for the single touch? I think
> that is the main issue here. We can have _one_ of the following definitions, but
> not both:
> 
> 1. input_report_key(input_dev, BTN_TOUCH, finger_num > 0);
> 

OK, i will use this. This was original code.

> 2. input_report_key(input_dev, BTN_TOUCH,
>                     finger[single_id].status != QT602240_RELEASE);
> 
> If you use the latter, there should be another event to denote the finger_num ==
> 0 case. This line at the end should do it:
> 
> if (finger_num == 0)
> 	input_mt_sync(input_dev);
> 

I don't know why this needs?


Here, there are fixed codes. How about do you think?

static void qt602240_input_report(struct qt602240_data *data, int single_id)
{
	struct qt602240_finger *finger = data->finger;
	struct input_dev *input_dev = data->input_dev;
	int status = finger[single_id].status;
	int finger_num = 0;
	int id;

	for (id = 0; id < QT602240_MAX_FINGER; id++) {
		if (!finger[id].status)
			continue;

		input_report_abs(input_dev, ABS_MT_TOUCH_MAJOR,
				finger[id].status != QT602240_RELEASE ?
				finger[id].area : 0);
		input_report_abs(input_dev, ABS_MT_POSITION_X,
				finger[id].x);
		input_report_abs(input_dev, ABS_MT_POSITION_Y,
				finger[id].y);
		input_mt_sync(input_dev);

		if (finger[id].status == QT602240_RELEASE)
			finger[id].status = 0;
		else
			finger_num++;
	}

	input_report_key(input_dev, BTN_TOUCH, finger_num > 0);

	if (status != QT602240_RELEASE) {
		input_report_abs(input_dev, ABS_X, finger[single_id].x);
		input_report_abs(input_dev, ABS_Y, finger[single_id].y);
	}

	input_sync(input_dev);
}


Thanks.

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

* Re: [PATCH v2] input: qt602240 - Add ATMEL QT602240 touchscreen driver
  2010-06-28  8:17       ` Joonyoung Shim
@ 2010-06-28  8:34         ` Henrik Rydberg
  2010-06-28  8:42           ` Dmitry Torokhov
  2010-06-28  9:05           ` Joonyoung Shim
  0 siblings, 2 replies; 21+ messages in thread
From: Henrik Rydberg @ 2010-06-28  8:34 UTC (permalink / raw)
  To: Joonyoung Shim; +Cc: dmitry.torokhov, linux-input, kyungmin.park

Joonyoung Shim wrote:
[...]
> I see, but i have something wondering at your document.
> 
> This is your patch of "Document the MT event slot protocol"
> 
> +Protocol Example A
> +------------------
> +
> +Here is what a minimal event sequence for a two-contact touch would look
> +like for a type A device:
> +
> +   ABS_MT_POSITION_X x[0]
> +   ABS_MT_POSITION_Y y[0]
> +   SYN_MT_REPORT
> +   ABS_MT_POSITION_X x[1]
> +   ABS_MT_POSITION_Y y[1]
> +   SYN_MT_REPORT
> +   SYN_REPORT
> 
> +The sequence after moving one of the contacts looks exactly the same; the
> +raw data for all present contacts are sent between every synchronization
> +with SYN_REPORT.
> 
> -Usage
> ------
> +Here is the sequence after lifting the first contact:
> +
> +   ABS_MT_POSITION_X x[1]
> +   ABS_MT_POSITION_Y y[1]
> +   SYN_MT_REPORT
> +   SYN_REPORT
> +
> +And here is the sequence after lifting the second contact:
> +
> +   SYN_MT_REPORT
> +   SYN_REPORT
> +
> 
> Here, there is no reporting for ABS_MT_POSITION_X/Y event, because that
> is the last contact?
> Then, the coordinates of the first contact are x[1] and y[1], right? If 
> yes, it is some confusing, i think they are x[0] and y[0].

It is a bit confusing I agree, but the document is correct. The empty
input_mt_sync() is used when there is no data to report, no lifted fingers,
nothing. Just imagine a device which gets polled periodically.

[...]
>> I see. And you want BTN_TOUCH to follow the logic for the single touch? I think
>> that is the main issue here. We can have _one_ of the following definitions, but
>> not both:
>>
>> 1. input_report_key(input_dev, BTN_TOUCH, finger_num > 0);
>>
> 
> OK, i will use this. This was original code.
> 
>> 2. input_report_key(input_dev, BTN_TOUCH,
>>                     finger[single_id].status != QT602240_RELEASE);
>>
>> If you use the latter, there should be another event to denote the finger_num ==
>> 0 case. This line at the end should do it:
>>
>> if (finger_num == 0)
>> 	input_mt_sync(input_dev);
>>
> 
> I don't know why this needs?

The general reason is the one given above. Since you are going with the first
option, it won't be needed.

> Here, there are fixed codes. How about do you think?
> 
> static void qt602240_input_report(struct qt602240_data *data, int single_id)
> {
> 	struct qt602240_finger *finger = data->finger;
> 	struct input_dev *input_dev = data->input_dev;
> 	int status = finger[single_id].status;
> 	int finger_num = 0;
> 	int id;
> 
> 	for (id = 0; id < QT602240_MAX_FINGER; id++) {
> 		if (!finger[id].status)
> 			continue;
> 
> 		input_report_abs(input_dev, ABS_MT_TOUCH_MAJOR,
> 				finger[id].status != QT602240_RELEASE ?
> 				finger[id].area : 0);
> 		input_report_abs(input_dev, ABS_MT_POSITION_X,
> 				finger[id].x);
> 		input_report_abs(input_dev, ABS_MT_POSITION_Y,
> 				finger[id].y);
> 		input_mt_sync(input_dev);
> 
> 		if (finger[id].status == QT602240_RELEASE)
> 			finger[id].status = 0;
> 		else
> 			finger_num++;
> 	}
> 
> 	input_report_key(input_dev, BTN_TOUCH, finger_num > 0);
> 
> 	if (status != QT602240_RELEASE) {
> 		input_report_abs(input_dev, ABS_X, finger[single_id].x);
> 		input_report_abs(input_dev, ABS_Y, finger[single_id].y);
> 	}
> 
> 	input_sync(input_dev);
> }
> 
> 
> Thanks.

This looks like it will work.

Thanks,
Henrik

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

* Re: [PATCH v2] input: qt602240 - Add ATMEL QT602240 touchscreen driver
  2010-06-28  8:34         ` Henrik Rydberg
@ 2010-06-28  8:42           ` Dmitry Torokhov
  2010-06-28  8:46             ` Henrik Rydberg
  2010-06-28  9:05           ` Joonyoung Shim
  1 sibling, 1 reply; 21+ messages in thread
From: Dmitry Torokhov @ 2010-06-28  8:42 UTC (permalink / raw)
  To: Henrik Rydberg; +Cc: Joonyoung Shim, linux-input, kyungmin.park

On Mon, Jun 28, 2010 at 10:34:13AM +0200, Henrik Rydberg wrote:
> 
> It is a bit confusing I agree, but the document is correct. The empty
> input_mt_sync() is used when there is no data to report, no lifted fingers,
> nothing. Just imagine a device which gets polled periodically.

If there is no new data to report why we need to call input_mt_sync() at
all? You can send input_sync() but input core will filter it out...

-- 
Dmitry

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

* Re: [PATCH v2] input: qt602240 - Add ATMEL QT602240 touchscreen driver
  2010-06-28  8:42           ` Dmitry Torokhov
@ 2010-06-28  8:46             ` Henrik Rydberg
  2010-06-28  9:01               ` Dmitry Torokhov
  0 siblings, 1 reply; 21+ messages in thread
From: Henrik Rydberg @ 2010-06-28  8:46 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: Joonyoung Shim, linux-input, kyungmin.park

Dmitry Torokhov wrote:
> On Mon, Jun 28, 2010 at 10:34:13AM +0200, Henrik Rydberg wrote:
>> It is a bit confusing I agree, but the document is correct. The empty
>> input_mt_sync() is used when there is no data to report, no lifted fingers,
>> nothing. Just imagine a device which gets polled periodically.
> 
> If there is no new data to report why we need to call input_mt_sync() at
> all? You can send input_sync() but input core will filter it out...
> 

Yep, that is the reason. Admittedly, this is a corner cases that type A does not
handle very gracefully, but it works. The only device so far where this seems
useful is for the magic mouse, which does not want to report BTN_TOUCH at all.

Henrik

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

* Re: [PATCH v2] input: qt602240 - Add ATMEL QT602240 touchscreen driver
  2010-06-28  8:46             ` Henrik Rydberg
@ 2010-06-28  9:01               ` Dmitry Torokhov
  2010-06-28  9:10                 ` Henrik Rydberg
  0 siblings, 1 reply; 21+ messages in thread
From: Dmitry Torokhov @ 2010-06-28  9:01 UTC (permalink / raw)
  To: Henrik Rydberg; +Cc: Joonyoung Shim, linux-input, kyungmin.park

On Mon, Jun 28, 2010 at 10:46:59AM +0200, Henrik Rydberg wrote:
> Dmitry Torokhov wrote:
> > On Mon, Jun 28, 2010 at 10:34:13AM +0200, Henrik Rydberg wrote:
> >> It is a bit confusing I agree, but the document is correct. The empty
> >> input_mt_sync() is used when there is no data to report, no lifted fingers,
> >> nothing. Just imagine a device which gets polled periodically.
> > 
> > If there is no new data to report why we need to call input_mt_sync() at
> > all? You can send input_sync() but input core will filter it out...
> > 
> 
> Yep, that is the reason. Admittedly, this is a corner cases that type A does not
> handle very gracefully, but it works. The only device so far where this seems
> useful is for the magic mouse, which does not want to report BTN_TOUCH at all.
> 

So what if we filter everything out? Userspace will never know that we
got an interrupt and will be just fine...

-- 
Dmitry

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

* Re: [PATCH v2] input: qt602240 - Add ATMEL QT602240 touchscreen driver
  2010-06-28  8:34         ` Henrik Rydberg
  2010-06-28  8:42           ` Dmitry Torokhov
@ 2010-06-28  9:05           ` Joonyoung Shim
  2010-06-28  9:17             ` Henrik Rydberg
  1 sibling, 1 reply; 21+ messages in thread
From: Joonyoung Shim @ 2010-06-28  9:05 UTC (permalink / raw)
  To: Henrik Rydberg; +Cc: dmitry.torokhov, linux-input, kyungmin.park

On 6/28/2010 5:34 PM, Henrik Rydberg wrote:
> Joonyoung Shim wrote:
> [...]
>> I see, but i have something wondering at your document.
>>
>> This is your patch of "Document the MT event slot protocol"
>>
>> +Protocol Example A
>> +------------------
>> +
>> +Here is what a minimal event sequence for a two-contact touch would look
>> +like for a type A device:
>> +
>> +   ABS_MT_POSITION_X x[0]
>> +   ABS_MT_POSITION_Y y[0]
>> +   SYN_MT_REPORT
>> +   ABS_MT_POSITION_X x[1]
>> +   ABS_MT_POSITION_Y y[1]
>> +   SYN_MT_REPORT
>> +   SYN_REPORT
>>
>> +The sequence after moving one of the contacts looks exactly the same; the
>> +raw data for all present contacts are sent between every synchronization
>> +with SYN_REPORT.
>>
>> -Usage
>> ------
>> +Here is the sequence after lifting the first contact:
>> +
>> +   ABS_MT_POSITION_X x[1]
>> +   ABS_MT_POSITION_Y y[1]
>> +   SYN_MT_REPORT
>> +   SYN_REPORT
>> +
>> +And here is the sequence after lifting the second contact:
>> +
>> +   SYN_MT_REPORT
>> +   SYN_REPORT
>> +
>>
>> Here, there is no reporting for ABS_MT_POSITION_X/Y event, because that
>> is the last contact?
>> Then, the coordinates of the first contact are x[1] and y[1], right? If 
>> yes, it is some confusing, i think they are x[0] and y[0].
> 
> It is a bit confusing I agree, but the document is correct. The empty
> input_mt_sync() is used when there is no data to report, no lifted fingers,
> nothing. Just imagine a device which gets polled periodically.
> 

The thing i wondering is why reports x[1] and y[1] instead of x[0] and 
y[0] after lifting the first contact. I have understood the first 
contact are x[0] and y[0] and the second contact are x[1] and y[1].

> [...]
>>> I see. And you want BTN_TOUCH to follow the logic for the single touch? I think
>>> that is the main issue here. We can have _one_ of the following definitions, but
>>> not both:
>>>
>>> 1. input_report_key(input_dev, BTN_TOUCH, finger_num > 0);
>>>
>> OK, i will use this. This was original code.
>>
>>> 2. input_report_key(input_dev, BTN_TOUCH,
>>>                     finger[single_id].status != QT602240_RELEASE);
>>>
>>> If you use the latter, there should be another event to denote the finger_num ==
>>> 0 case. This line at the end should do it:
>>>
>>> if (finger_num == 0)
>>> 	input_mt_sync(input_dev);
>>>
>> I don't know why this needs?
> 
> The general reason is the one given above. Since you are going with the first
> option, it won't be needed.
> 

But, input_mt_sync is reported already with reporting of ABS_MT_POSITION_X/Y.
I meant the case of single touch reporting.

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

* Re: [PATCH v2] input: qt602240 - Add ATMEL QT602240 touchscreen driver
  2010-06-28  9:01               ` Dmitry Torokhov
@ 2010-06-28  9:10                 ` Henrik Rydberg
  0 siblings, 0 replies; 21+ messages in thread
From: Henrik Rydberg @ 2010-06-28  9:10 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: Joonyoung Shim, linux-input, kyungmin.park

Dmitry Torokhov wrote:
> On Mon, Jun 28, 2010 at 10:46:59AM +0200, Henrik Rydberg wrote:
>> Dmitry Torokhov wrote:
>>> On Mon, Jun 28, 2010 at 10:34:13AM +0200, Henrik Rydberg wrote:
>>>> It is a bit confusing I agree, but the document is correct. The empty
>>>> input_mt_sync() is used when there is no data to report, no lifted fingers,
>>>> nothing. Just imagine a device which gets polled periodically.
>>> If there is no new data to report why we need to call input_mt_sync() at
>>> all? You can send input_sync() but input core will filter it out...
>>>
>> Yep, that is the reason. Admittedly, this is a corner cases that type A does not
>> handle very gracefully, but it works. The only device so far where this seems
>> useful is for the magic mouse, which does not want to report BTN_TOUCH at all.
>>
> 
> So what if we filter everything out? Userspace will never know that we
> got an interrupt and will be just fine...
> 

Userspace would still want to know if that last finger stayed on the pad the
whole time, or if it was actually removed in between.

Henrik

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

* Re: [PATCH v2] input: qt602240 - Add ATMEL QT602240 touchscreen driver
  2010-06-28  9:05           ` Joonyoung Shim
@ 2010-06-28  9:17             ` Henrik Rydberg
  2010-06-28 10:00               ` Joonyoung Shim
  0 siblings, 1 reply; 21+ messages in thread
From: Henrik Rydberg @ 2010-06-28  9:17 UTC (permalink / raw)
  To: Joonyoung Shim; +Cc: dmitry.torokhov, linux-input, kyungmin.park

Joonyoung Shim wrote:
> On 6/28/2010 5:34 PM, Henrik Rydberg wrote:
>> Joonyoung Shim wrote:
>> [...]
>>> I see, but i have something wondering at your document.
>>>
>>> This is your patch of "Document the MT event slot protocol"
>>>
>>> +Protocol Example A
>>> +------------------
>>> +
>>> +Here is what a minimal event sequence for a two-contact touch would look
>>> +like for a type A device:
>>> +
>>> +   ABS_MT_POSITION_X x[0]
>>> +   ABS_MT_POSITION_Y y[0]
>>> +   SYN_MT_REPORT
>>> +   ABS_MT_POSITION_X x[1]
>>> +   ABS_MT_POSITION_Y y[1]
>>> +   SYN_MT_REPORT
>>> +   SYN_REPORT
>>>
>>> +The sequence after moving one of the contacts looks exactly the same; the
>>> +raw data for all present contacts are sent between every synchronization
>>> +with SYN_REPORT.
>>>
>>> -Usage
>>> ------
>>> +Here is the sequence after lifting the first contact:
>>> +
>>> +   ABS_MT_POSITION_X x[1]
>>> +   ABS_MT_POSITION_Y y[1]
>>> +   SYN_MT_REPORT
>>> +   SYN_REPORT
>>> +
>>> +And here is the sequence after lifting the second contact:
>>> +
>>> +   SYN_MT_REPORT
>>> +   SYN_REPORT
>>> +
>>>
>>> Here, there is no reporting for ABS_MT_POSITION_X/Y event, because that
>>> is the last contact?
>>> Then, the coordinates of the first contact are x[1] and y[1], right? If 
>>> yes, it is some confusing, i think they are x[0] and y[0].
>> It is a bit confusing I agree, but the document is correct. The empty
>> input_mt_sync() is used when there is no data to report, no lifted fingers,
>> nothing. Just imagine a device which gets polled periodically.
>>
> 
> The thing i wondering is why reports x[1] and y[1] instead of x[0] and 
> y[0] after lifting the first contact. I have understood the first 
> contact are x[0] and y[0] and the second contact are x[1] and y[1].

Yes, after lifting the first contact, what remains is the second contact, which
is the one getting reported. Again, stateless protocol. ;-)

> 
>> [...]
>>>> I see. And you want BTN_TOUCH to follow the logic for the single touch? I think
>>>> that is the main issue here. We can have _one_ of the following definitions, but
>>>> not both:
>>>>
>>>> 1. input_report_key(input_dev, BTN_TOUCH, finger_num > 0);
>>>>
>>> OK, i will use this. This was original code.
>>>
>>>> 2. input_report_key(input_dev, BTN_TOUCH,
>>>>                     finger[single_id].status != QT602240_RELEASE);
>>>>
>>>> If you use the latter, there should be another event to denote the finger_num ==
>>>> 0 case. This line at the end should do it:
>>>>
>>>> if (finger_num == 0)
>>>> 	input_mt_sync(input_dev);
>>>>
>>> I don't know why this needs?
>> The general reason is the one given above. Since you are going with the first
>> option, it won't be needed.
>>
> 
> But, input_mt_sync is reported already with reporting of ABS_MT_POSITION_X/Y.
> I meant the case of single touch reporting.

Yes. I meant there should be at least one event when all fingers are up. Since
you report ABS_MT_TOUCH_MAJOR = 0 for every finger going up, this is already
guaranteed, and you can disregard my last comment.

Thanks,
Henrik

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

* Re: [PATCH v2] input: qt602240 - Add ATMEL QT602240 touchscreen driver
  2010-06-28  9:17             ` Henrik Rydberg
@ 2010-06-28 10:00               ` Joonyoung Shim
  2010-06-28 10:22                 ` Henrik Rydberg
  0 siblings, 1 reply; 21+ messages in thread
From: Joonyoung Shim @ 2010-06-28 10:00 UTC (permalink / raw)
  To: Henrik Rydberg; +Cc: dmitry.torokhov, linux-input, kyungmin.park

On 6/28/2010 6:17 PM, Henrik Rydberg wrote:
> Joonyoung Shim wrote:
>> On 6/28/2010 5:34 PM, Henrik Rydberg wrote:
>>> Joonyoung Shim wrote:
>>> [...]
>>>> I see, but i have something wondering at your document.
>>>>
>>>> This is your patch of "Document the MT event slot protocol"
>>>>
>>>> +Protocol Example A
>>>> +------------------
>>>> +
>>>> +Here is what a minimal event sequence for a two-contact touch would look
>>>> +like for a type A device:
>>>> +
>>>> +   ABS_MT_POSITION_X x[0]
>>>> +   ABS_MT_POSITION_Y y[0]
>>>> +   SYN_MT_REPORT
>>>> +   ABS_MT_POSITION_X x[1]
>>>> +   ABS_MT_POSITION_Y y[1]
>>>> +   SYN_MT_REPORT
>>>> +   SYN_REPORT
>>>>
>>>> +The sequence after moving one of the contacts looks exactly the same; the
>>>> +raw data for all present contacts are sent between every synchronization
>>>> +with SYN_REPORT.
>>>>
>>>> -Usage
>>>> ------
>>>> +Here is the sequence after lifting the first contact:
>>>> +
>>>> +   ABS_MT_POSITION_X x[1]
>>>> +   ABS_MT_POSITION_Y y[1]
>>>> +   SYN_MT_REPORT
>>>> +   SYN_REPORT
>>>> +
>>>> +And here is the sequence after lifting the second contact:
>>>> +
>>>> +   SYN_MT_REPORT
>>>> +   SYN_REPORT
>>>> +
>>>>
>>>> Here, there is no reporting for ABS_MT_POSITION_X/Y event, because that
>>>> is the last contact?
>>>> Then, the coordinates of the first contact are x[1] and y[1], right? If 
>>>> yes, it is some confusing, i think they are x[0] and y[0].
>>> It is a bit confusing I agree, but the document is correct. The empty
>>> input_mt_sync() is used when there is no data to report, no lifted fingers,
>>> nothing. Just imagine a device which gets polled periodically.
>>>
>> The thing i wondering is why reports x[1] and y[1] instead of x[0] and 
>> y[0] after lifting the first contact. I have understood the first 
>> contact are x[0] and y[0] and the second contact are x[1] and y[1].
> 
> Yes, after lifting the first contact, what remains is the second contact, which
> is the one getting reported. Again, stateless protocol. ;-)
> 

Do you mean to report the coordinates of contact __remaining__? You told 
me at first, "The position should be the position where the finger left
the surface", so i am confusing.

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

* Re: [PATCH v2] input: qt602240 - Add ATMEL QT602240 touchscreen driver
  2010-06-28 10:00               ` Joonyoung Shim
@ 2010-06-28 10:22                 ` Henrik Rydberg
  2010-06-28 11:12                   ` Joonyoung Shim
  0 siblings, 1 reply; 21+ messages in thread
From: Henrik Rydberg @ 2010-06-28 10:22 UTC (permalink / raw)
  To: Joonyoung Shim; +Cc: dmitry.torokhov, linux-input, kyungmin.park

Joonyoung Shim wrote:
[...]
> Do you mean to report the coordinates of contact __remaining__? You told 
> me at first, "The position should be the position where the finger left
> the surface", so i am confusing.

The two comments do not apply to the same situation. The latter comment was made
in the context of a tracking-capable device which sends one last event for a
finger going away. Transformed to the stateless type A protocol, that results in
(touch = 0, x = last value, y = last value). The former comment was in the
general context of the type A protocol, which has no notion of anything going
away. After you lift a finger and look at the state, what you see is the
remaining set of fingers.

Thank you,
Henrik


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

* Re: [PATCH v2] input: qt602240 - Add ATMEL QT602240 touchscreen driver
  2010-06-28 10:22                 ` Henrik Rydberg
@ 2010-06-28 11:12                   ` Joonyoung Shim
  2010-06-28 12:23                     ` Henrik Rydberg
  0 siblings, 1 reply; 21+ messages in thread
From: Joonyoung Shim @ 2010-06-28 11:12 UTC (permalink / raw)
  To: Henrik Rydberg; +Cc: dmitry.torokhov, linux-input, kyungmin.park

On 6/28/2010 7:22 PM, Henrik Rydberg wrote:
> Joonyoung Shim wrote:
> [...]
>> Do you mean to report the coordinates of contact __remaining__? You told 
>> me at first, "The position should be the position where the finger left
>> the surface", so i am confusing.
> 
> The two comments do not apply to the same situation. The latter comment was made
> in the context of a tracking-capable device which sends one last event for a
> finger going away. Transformed to the stateless type A protocol, that results in
> (touch = 0, x = last value, y = last value). 
> The former comment was in the
> general context of the type A protocol, which has no notion of anything going
> away. After you lift a finger and look at the state, what you see is the
> remaining set of fingers.
> 

OK, i see, but i think it needs to add the latter comment on MT protocol 
document to prevent some confusion.


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

* Re: [PATCH v2] input: qt602240 - Add ATMEL QT602240 touchscreen driver
  2010-06-28 11:12                   ` Joonyoung Shim
@ 2010-06-28 12:23                     ` Henrik Rydberg
  2010-06-28 12:58                       ` Joonyoung Shim
  0 siblings, 1 reply; 21+ messages in thread
From: Henrik Rydberg @ 2010-06-28 12:23 UTC (permalink / raw)
  To: Joonyoung Shim; +Cc: dmitry.torokhov, linux-input, kyungmin.park

Joonyoung Shim wrote:
> On 6/28/2010 7:22 PM, Henrik Rydberg wrote:
>> Joonyoung Shim wrote:
>> [...]
>>> Do you mean to report the coordinates of contact __remaining__? You told 
>>> me at first, "The position should be the position where the finger left
>>> the surface", so i am confusing.
>> The two comments do not apply to the same situation. The latter comment was made
>> in the context of a tracking-capable device which sends one last event for a
>> finger going away. Transformed to the stateless type A protocol, that results in
>> (touch = 0, x = last value, y = last value). 
>> The former comment was in the
>> general context of the type A protocol, which has no notion of anything going
>> away. After you lift a finger and look at the state, what you see is the
>> remaining set of fingers.
>>
> 
> OK, i see, but i think it needs to add the latter comment on MT protocol 
> document to prevent some confusion.
> 

Something like this? Dmitry, would you please consider squashing this patch with
the 5b version, if Joonyoung concurs?

Thanks,
Henrik

---

diff --git a/Documentation/input/multi-touch-protocol.txt b/Documentation/input/
index 3ab038d..bdcba15 100644
--- a/Documentation/input/multi-touch-protocol.txt
+++ b/Documentation/input/multi-touch-protocol.txt
@@ -43,15 +43,16 @@ input_sync() function. This instructs the receiver to act up
 accumulated since last EV_SYN/SYN_REPORT and prepare to receive a new set
 of events/packets.

-The main difference between the raw type A protocol and the higher level
-type B slot protocol lies in the usage of identifiable contacts. The slot
-protocol requires the use of the ABS_MT_TRACKING_ID, either provided by the
-hardware or computed from the raw data [5].
+The main difference between the stateless type A protocol and the stateful
+type B slot protocol lies in the usage of identifiable contacts to reduce
+the amount of data sent to userspace. The slot protocol requires the use of
+the ABS_MT_TRACKING_ID, either provided by the hardware or computed from
+the raw data [5].

 For type A devices, the kernel driver should generate an arbitrary
-enumeration of the set of anonymous contacts currently on the surface. The
-order in which the packets appear in the event stream is not important.
-Event filtering and finger tracking is left to user space [3].
+enumeration of the full set of anonymous contacts currently on the
+surface. The order in which the packets appear in the event stream is not
+important.  Event filtering and finger tracking is left to user space [3].

 For type B devices, the kernel driver should associate a slot with each
 identified contact, and use that slot to propagate changes for the contact.

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

* Re: [PATCH v2] input: qt602240 - Add ATMEL QT602240 touchscreen driver
  2010-06-28 12:23                     ` Henrik Rydberg
@ 2010-06-28 12:58                       ` Joonyoung Shim
  2010-06-28 13:24                         ` Henrik Rydberg
  0 siblings, 1 reply; 21+ messages in thread
From: Joonyoung Shim @ 2010-06-28 12:58 UTC (permalink / raw)
  To: Henrik Rydberg; +Cc: dmitry.torokhov, linux-input, kyungmin.park

On 6/28/2010 9:23 PM, Henrik Rydberg wrote:
> Joonyoung Shim wrote:
>> On 6/28/2010 7:22 PM, Henrik Rydberg wrote:
>>> Joonyoung Shim wrote:
>>> [...]
>>>> Do you mean to report the coordinates of contact __remaining__? You told 
>>>> me at first, "The position should be the position where the finger left
>>>> the surface", so i am confusing.
>>> The two comments do not apply to the same situation. The latter comment was made
>>> in the context of a tracking-capable device which sends one last event for a
>>> finger going away. Transformed to the stateless type A protocol, that results in
>>> (touch = 0, x = last value, y = last value). 
>>> The former comment was in the
>>> general context of the type A protocol, which has no notion of anything going
>>> away. After you lift a finger and look at the state, what you see is the
>>> remaining set of fingers.
>>>
>> OK, i see, but i think it needs to add the latter comment on MT protocol 
>> document to prevent some confusion.
>>
> 
> Something like this? Dmitry, would you please consider squashing this patch with
> the 5b version, if Joonyoung concurs?
> 
> Thanks,
> Henrik
> 
> ---
> 
> diff --git a/Documentation/input/multi-touch-protocol.txt b/Documentation/input/
> index 3ab038d..bdcba15 100644
> --- a/Documentation/input/multi-touch-protocol.txt
> +++ b/Documentation/input/multi-touch-protocol.txt
> @@ -43,15 +43,16 @@ input_sync() function. This instructs the receiver to act up
>  accumulated since last EV_SYN/SYN_REPORT and prepare to receive a new set
>  of events/packets.
> 
> -The main difference between the raw type A protocol and the higher level
> -type B slot protocol lies in the usage of identifiable contacts. The slot
> -protocol requires the use of the ABS_MT_TRACKING_ID, either provided by the
> -hardware or computed from the raw data [5].
> +The main difference between the stateless type A protocol and the stateful
> +type B slot protocol lies in the usage of identifiable contacts to reduce
> +the amount of data sent to userspace. The slot protocol requires the use of
> +the ABS_MT_TRACKING_ID, either provided by the hardware or computed from
> +the raw data [5].
> 
>  For type A devices, the kernel driver should generate an arbitrary
> -enumeration of the set of anonymous contacts currently on the surface. The
> -order in which the packets appear in the event stream is not important.
> -Event filtering and finger tracking is left to user space [3].
> +enumeration of the full set of anonymous contacts currently on the
> +surface. The order in which the packets appear in the event stream is not
> +important.  Event filtering and finger tracking is left to user space [3].
> 

What is the full set of anonymous contacts? It needs the description of
case sending event for a finger going away.

>  For type B devices, the kernel driver should associate a slot with each
>  identified contact, and use that slot to propagate changes for the contact.
> 



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

* Re: [PATCH v2] input: qt602240 - Add ATMEL QT602240 touchscreen driver
  2010-06-28 12:58                       ` Joonyoung Shim
@ 2010-06-28 13:24                         ` Henrik Rydberg
  2010-06-28 13:41                           ` Joonyoung Shim
  0 siblings, 1 reply; 21+ messages in thread
From: Henrik Rydberg @ 2010-06-28 13:24 UTC (permalink / raw)
  To: Joonyoung Shim; +Cc: dmitry.torokhov, linux-input, kyungmin.park

Joonyoung Shim wrote:
[...]
> What is the full set of anonymous contacts? It needs the description of
> case sending event for a finger going away.

I disagree. Protocol A deals with devices that do not necessarily know what
going away means. There is a whole new protocol devoted to your kind of
hardware, type B, and using protocol A is simply not encouraged.

Thanks,
Henrik

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

* Re: [PATCH v2] input: qt602240 - Add ATMEL QT602240 touchscreen driver
  2010-06-28 13:24                         ` Henrik Rydberg
@ 2010-06-28 13:41                           ` Joonyoung Shim
       [not found]                             ` <AANLkTikNh-LTClIgXGAIEkegLCmWNnN2Zk05XhZWOumb@mail.gmail.com>
  0 siblings, 1 reply; 21+ messages in thread
From: Joonyoung Shim @ 2010-06-28 13:41 UTC (permalink / raw)
  To: Henrik Rydberg
  Cc: Joonyoung Shim, dmitry.torokhov, linux-input, kyungmin.park

2010/6/28 Henrik Rydberg <rydberg@euromail.se>:
> Joonyoung Shim wrote:
> [...]
>> What is the full set of anonymous contacts? It needs the description of
>> case sending event for a finger going away.
>
> I disagree. Protocol A deals with devices that do not necessarily know what
> going away means. There is a whole new protocol devoted to your kind of
> hardware, type B, and using protocol A is simply not encouraged.
>

If so, i'm fine.

Thanks.


-- 
- Joonyoung Shim

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

* Re: [PATCH v2] input: qt602240 - Add ATMEL QT602240 touchscreen driver
       [not found]                             ` <AANLkTikNh-LTClIgXGAIEkegLCmWNnN2Zk05XhZWOumb@mail.gmail.com>
@ 2010-06-28 14:33                               ` Dan Murphy
  2010-06-29  3:33                               ` Joonyoung Shim
  1 sibling, 0 replies; 21+ messages in thread
From: Dan Murphy @ 2010-06-28 14:33 UTC (permalink / raw)
  To: Joonyoung Shim
  Cc: Henrik Rydberg, Joonyoung Shim, dmitry.torokhov, linux-input,
	kyungmin.park

On Mon, Jun 28, 2010 at 9:26 AM, Dan Murphy <murpdj72@gmail.com> wrote:
>
> I don't see any error handling from the IC here.
>
> Generally this IC should give a few error messages if a X/Y line is broken, the chip goes into reset, or the chip is missed configured.
>
> This is a bad situation here
> +       /* Read dummy message to make high CHG pin */
> +       do {
> +               ret = qt602240_read_object(data, QT602240_GEN_MESSAGE, 0);
> +               if (ret < 0)
> +                       return ret;
> +       } while (ret != 0xff);
>
> The chip may have an error and constantly send you the message which means you will never get out of this loop.  This can be bad as your probe or load_fw may never return.
>
>
>
> Dan
>
> On Mon, Jun 28, 2010 at 8:41 AM, Joonyoung Shim <dofmind@gmail.com> wrote:
>>
>> 2010/6/28 Henrik Rydberg <rydberg@euromail.se>:
>> > Joonyoung Shim wrote:
>> > [...]
>> >> What is the full set of anonymous contacts? It needs the description of
>> >> case sending event for a finger going away.
>> >
>> > I disagree. Protocol A deals with devices that do not necessarily know what
>> > going away means. There is a whole new protocol devoted to your kind of
>> > hardware, type B, and using protocol A is simply not encouraged.
>> >
>>
>> If so, i'm fine.
>>
>> Thanks.
>>
>>
>> --
>> - Joonyoung Shim
>> --
>> 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
>
--
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] 21+ messages in thread

* Re: [PATCH v2] input: qt602240 - Add ATMEL QT602240 touchscreen driver
       [not found]                             ` <AANLkTikNh-LTClIgXGAIEkegLCmWNnN2Zk05XhZWOumb@mail.gmail.com>
  2010-06-28 14:33                               ` Dan Murphy
@ 2010-06-29  3:33                               ` Joonyoung Shim
  1 sibling, 0 replies; 21+ messages in thread
From: Joonyoung Shim @ 2010-06-29  3:33 UTC (permalink / raw)
  To: Dan Murphy
  Cc: Joonyoung Shim, Henrik Rydberg, dmitry.torokhov, linux-input,
	kyungmin.park

Hi,

On 6/28/2010 11:26 PM, Dan Murphy wrote:
> I don't see any error handling from the IC here.
> 
> Generally this IC should give a few error messages if a X/Y line is broken,
> the chip goes into reset, or the chip is missed configured.
> 

The configuration error can occur and if the chip detect it, we get
messages from message object in interrupt handler and can see it. If 
needs, i can add error print.

> This is a bad situation here
> +       /* Read dummy message to make high CHG pin */
> +       do {
> +               ret = qt602240_read_object(data, QT602240_GEN_MESSAGE, 0);
> +               if (ret < 0)
> +                       return ret;
> +       } while (ret != 0xff);
> 
> The chip may have an error and constantly send you the message which means
> you will never get out of this loop.  This can be bad as your probe or
> load_fw may never return.
> 

Hmm, this chip reports always 0xff value if the message is nothing, but
i will add count to prevent unlikely loop.

Thanks.

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

end of thread, other threads:[~2010-06-29  3:33 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-06-25  1:54 [PATCH v2] input: qt602240 - Add ATMEL QT602240 touchscreen driver Joonyoung Shim
2010-06-25 14:08 ` Henrik Rydberg
2010-06-28  5:16   ` Joonyoung Shim
2010-06-28  7:37     ` Henrik Rydberg
2010-06-28  8:17       ` Joonyoung Shim
2010-06-28  8:34         ` Henrik Rydberg
2010-06-28  8:42           ` Dmitry Torokhov
2010-06-28  8:46             ` Henrik Rydberg
2010-06-28  9:01               ` Dmitry Torokhov
2010-06-28  9:10                 ` Henrik Rydberg
2010-06-28  9:05           ` Joonyoung Shim
2010-06-28  9:17             ` Henrik Rydberg
2010-06-28 10:00               ` Joonyoung Shim
2010-06-28 10:22                 ` Henrik Rydberg
2010-06-28 11:12                   ` Joonyoung Shim
2010-06-28 12:23                     ` Henrik Rydberg
2010-06-28 12:58                       ` Joonyoung Shim
2010-06-28 13:24                         ` Henrik Rydberg
2010-06-28 13:41                           ` Joonyoung Shim
     [not found]                             ` <AANLkTikNh-LTClIgXGAIEkegLCmWNnN2Zk05XhZWOumb@mail.gmail.com>
2010-06-28 14:33                               ` Dan Murphy
2010-06-29  3:33                               ` Joonyoung Shim

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).