All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/4] qt602240_ts changes for Intel mid platform
@ 2010-11-16 20:41 Chris Leech
  2010-11-16 20:41 ` [PATCH 1/5] qt602240_ts: fix wrong sizeof in object table allocation Chris Leech
                   ` (4 more replies)
  0 siblings, 5 replies; 16+ messages in thread
From: Chris Leech @ 2010-11-16 20:41 UTC (permalink / raw)
  To: linux-input; +Cc: Joonyoung Shim

The following changes were made while getting this driver to work on an Intel
mid platform.

---

Chris Leech (4):
      qt602240_ts: fix wrong sizeof in object table allocation
      qt602240_ts: move clearing of pending interrupt closer to request_threaded_irq
      qt602240_ts: Trust factory configuration of touchscreen.
      qt602240_ts: add optional hooks for board specific reset logic


 drivers/input/touchscreen/qt602240_ts.c |  138 +++++++++++++++++++++++--------
 include/linux/i2c/qt602240_ts.h         |   12 +++
 2 files changed, 114 insertions(+), 36 deletions(-)

---

Here is an example of the platform device setup code that makes use of these
changes.  The platform code will be sent upstream via a different channel.

diff --git a/arch/x86/kernel/mrst.c b/arch/x86/kernel/mrst.c
index 9ed9e6b..d1ebdc8 100644
--- a/arch/x86/kernel/mrst.c
+++ b/arch/x86/kernel/mrst.c
@@ -34,6 +34,7 @@
 #include <linux/i2c/tc35894xbg.h>
 #include <linux/bh1770glc.h>
 #include <linux/leds-lp5523.h>
+#include <linux/i2c/qt602240_ts.h>
 
 #include <asm/setup.h>
 #include <asm/mpspec_def.h>
@@ -1032,6 +1033,76 @@ void *tc35894xbg_n_platform_data(void *info)
 		&tc35894xbg_ncdk_data);
 }
 
+static int qt602240_reset;
+static int qt602240_intr;
+
+static int qt602240_hw_setup(struct i2c_client *client)
+{
+	int err;
+	int intr;
+
+	err = gpio_request(qt602240_intr, "maXTouch-intr");
+	if (err) {
+		dev_err(&client->dev, "GPIO-%d request failed (err %d)\n",
+			qt602240_intr, err);
+		goto err_request_intr;
+	}
+
+	err = gpio_request(qt602240_reset, "maXTouch-reset");
+	if (err) {
+		dev_err(&client->dev, "GPIO-%d request failed (err %d)\n",
+			qt602240_reset, err);
+		goto err_request_reset;
+	}
+
+	err = gpio_direction_output(qt602240_reset, 0);
+	if (err) {
+		dev_err(&client->dev, "GPIO-%d direction set failed (err %d)\n",
+			qt602240_reset, err);
+		goto err_reset_direction;
+	}
+
+	gpio_set_value(qt602240_reset, 1);
+	/* maXTouch wants 40mSec minimum after reset to get organized */
+	/* double that to be safe as a maximum required delay */
+	msleep(80);
+	return 0;
+
+err_reset_direction:
+	gpio_free(qt602240_reset);
+err_request_reset:
+	gpio_free(qt602240_intr);
+err_request_intr:
+	return -EIO;
+}
+
+static void qt602240_hw_teardown(struct i2c_client *client)
+{
+	gpio_set_value(qt602240_reset, 0);
+	gpio_free(qt602240_reset);
+	gpio_free(qt602240_intr);
+}
+
+static void *qt602240_platform_data(void *info)
+{
+	struct i2c_board_info *i2c_info = info;
+	static struct qt602240_platform_data pdata = {
+		.trust_nvm = 1,
+		.hw_setup = qt602240_hw_setup,
+		.hw_teardown = qt602240_hw_teardown,
+	};
+
+	qt602240_reset = get_gpio_by_name("TS0-reset");
+	qt602240_intr = get_gpio_by_name("TS0-intr");
+	if (qt602240_reset < 0  ||  qt602240_intr < 0) {
+		printk(KERN_ERR "Touchscreen-0 GPIOs not in SFI table\n");
+		return NULL;
+	}
+
+	i2c_info->irq = qt602240_intr + MRST_IRQ_OFFSET;
+	return &pdata;
+}
+
 static const struct devs_id device_ids[] = {
 	{"pmic_gpio", SFI_DEV_TYPE_SPI, 1, &pmic_gpio_platform_data},
 	{"pmic_gpio", SFI_DEV_TYPE_IPC, 1, &pmic_gpio_platform_data},
@@ -1053,6 +1124,7 @@ static const struct devs_id device_ids[] = {
 	{"pmic_audio", SFI_DEV_TYPE_IPC, 1, &no_platform_data},
 	{"msic_audio", SFI_DEV_TYPE_SPI, 1, &no_platform_data},
 	{"msic_audio", SFI_DEV_TYPE_IPC, 1, &no_platform_data},
+	{"mXT224", SFI_DEV_TYPE_I2C, 0, &qt602240_platform_data},
 	{"i2c_TC35894-nEB1", SFI_DEV_TYPE_I2C, 0, &tc35894xbg_n_platform_data},
 	{"i2c_TC35894-i", SFI_DEV_TYPE_I2C, 0, &tc35894xbg_i_platform_data},
 	{"bh1770glc", SFI_DEV_TYPE_I2C, 0, &bh1770glc_platform_data_init},


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

* [PATCH 1/5] qt602240_ts: fix wrong sizeof in object table allocation
  2010-11-16 20:41 [PATCH 0/4] qt602240_ts changes for Intel mid platform Chris Leech
@ 2010-11-16 20:41 ` Chris Leech
  2010-11-18 11:16   ` Joonyoung Shim
  2010-11-16 20:41 ` [PATCH 2/5] qt602240_ts: move clearing of pending interrupt closer to request_threaded_irq Chris Leech
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 16+ messages in thread
From: Chris Leech @ 2010-11-16 20:41 UTC (permalink / raw)
  To: linux-input; +Cc: Joonyoung Shim

The kcalloc call for the object table is using sizeof(struct qt602240_data)
when it should be using sizeof(struct qt6602240_object), resulting in a larger
allocation than is required.

Signed-off-by: Chris Leech <christopher.leech@linux.intel.com>
---
 drivers/input/touchscreen/qt602240_ts.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/input/touchscreen/qt602240_ts.c b/drivers/input/touchscreen/qt602240_ts.c
index 66b26ad..0b92c9d 100644
--- a/drivers/input/touchscreen/qt602240_ts.c
+++ b/drivers/input/touchscreen/qt602240_ts.c
@@ -969,7 +969,7 @@ static int qt602240_initialize(struct qt602240_data *data)
 		return error;
 
 	data->object_table = kcalloc(info->object_num,
-				     sizeof(struct qt602240_data),
+				     sizeof(struct qt602240_object),
 				     GFP_KERNEL);
 	if (!data->object_table) {
 		dev_err(&client->dev, "Failed to allocate memory\n");


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

* [PATCH 2/5] qt602240_ts: move clearing of pending interrupt closer to request_threaded_irq
  2010-11-16 20:41 [PATCH 0/4] qt602240_ts changes for Intel mid platform Chris Leech
  2010-11-16 20:41 ` [PATCH 1/5] qt602240_ts: fix wrong sizeof in object table allocation Chris Leech
@ 2010-11-16 20:41 ` Chris Leech
  2010-11-18 12:53   ` Joonyoung Shim
  2010-11-16 20:42 ` [PATCH 3/5] qt602240_ts: Trust factory configuration of touchscreen Chris Leech
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 16+ messages in thread
From: Chris Leech @ 2010-11-16 20:41 UTC (permalink / raw)
  To: linux-input; +Cc: Joonyoung Shim

I've seen interrupts asserted on the CHG pin between the call to make_highcgh()
during initialization and registering the interrupt handler, leaving CHG low
and no events get passed up.

This moves the clearing of pending messages to right before the call to
request_threaded_irq().  I still think there's a race here that could leave CHG
stuck low, but worry about clearing the message queue while the interrupt
handler is registered without some sort of additional locking.

Signed-off-by: Chris Leech <christopher.leech@linux.intel.com>
---
 drivers/input/touchscreen/qt602240_ts.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/input/touchscreen/qt602240_ts.c b/drivers/input/touchscreen/qt602240_ts.c
index 0b92c9d..95496ec 100644
--- a/drivers/input/touchscreen/qt602240_ts.c
+++ b/drivers/input/touchscreen/qt602240_ts.c
@@ -991,10 +991,6 @@ static int qt602240_initialize(struct qt602240_data *data)
 	if (error)
 		return error;
 
-	error = qt602240_make_highchg(data);
-	if (error)
-		return error;
-
 	qt602240_handle_pdata(data);
 
 	/* Backup to memory */
@@ -1280,6 +1276,10 @@ static int __devinit qt602240_probe(struct i2c_client *client,
 	if (error)
 		goto err_free_object;
 
+	error = qt602240_make_highchg(data);
+	if (error)
+		goto err_free_object;
+
 	error = request_threaded_irq(client->irq, NULL, qt602240_interrupt,
 			IRQF_TRIGGER_FALLING, client->dev.driver->name, data);
 	if (error) {


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

* [PATCH 3/5] qt602240_ts: Trust factory configuration of touchscreen.
  2010-11-16 20:41 [PATCH 0/4] qt602240_ts changes for Intel mid platform Chris Leech
  2010-11-16 20:41 ` [PATCH 1/5] qt602240_ts: fix wrong sizeof in object table allocation Chris Leech
  2010-11-16 20:41 ` [PATCH 2/5] qt602240_ts: move clearing of pending interrupt closer to request_threaded_irq Chris Leech
@ 2010-11-16 20:42 ` Chris Leech
  2010-11-18 13:09   ` Joonyoung Shim
  2010-11-16 20:42 ` [PATCH 4/5] qt602240_ts: add optional hooks for board specific reset logic Chris Leech
  2010-11-16 20:42 ` [PATCH 5/5] qt602240_ts: add mXT224 identifier to id_table, to match Intel mid firmware identifier Chris Leech
  4 siblings, 1 reply; 16+ messages in thread
From: Chris Leech @ 2010-11-16 20:42 UTC (permalink / raw)
  To: linux-input; +Cc: Joonyoung Shim

The maXTouch 224 touchscreen controller has non-volatile storage for system
configuration and calibration settings.  Make it a platform option to trust
those values, instead of reconfiguring everything.  The configuration settings
currently being applied are determined from the firmware version, but are not
appropriate for all systems.  This should be a compatible change with existing
users of this driver.

Signed-off-by: Chris Leech <christopher.leech@linux.intel.com>
---
 drivers/input/touchscreen/qt602240_ts.c |  121 +++++++++++++++++++++++--------
 include/linux/i2c/qt602240_ts.h         |    8 ++
 2 files changed, 98 insertions(+), 31 deletions(-)

diff --git a/drivers/input/touchscreen/qt602240_ts.c b/drivers/input/touchscreen/qt602240_ts.c
index 95496ec..11055ec 100644
--- a/drivers/input/touchscreen/qt602240_ts.c
+++ b/drivers/input/touchscreen/qt602240_ts.c
@@ -353,7 +353,7 @@ struct qt602240_finger {
 struct qt602240_data {
 	struct i2c_client *client;
 	struct input_dev *input_dev;
-	const struct qt602240_platform_data *pdata;
+	struct qt602240_platform_data *pdata;
 	struct qt602240_object *object_table;
 	struct qt602240_info info;
 	struct qt602240_finger finger[QT602240_MAX_FINGER];
@@ -754,7 +754,7 @@ static int qt602240_check_reg_init(struct qt602240_data *data)
 
 static int qt602240_check_matrix_size(struct qt602240_data *data)
 {
-	const struct qt602240_platform_data *pdata = data->pdata;
+	struct qt602240_platform_data *pdata = data->pdata;
 	struct device *dev = &data->client->dev;
 	int mode = -1;
 	int error;
@@ -844,7 +844,7 @@ static int qt602240_make_highchg(struct qt602240_data *data)
 
 static void qt602240_handle_pdata(struct qt602240_data *data)
 {
-	const struct qt602240_platform_data *pdata = data->pdata;
+	struct qt602240_platform_data *pdata = data->pdata;
 	u8 voltage;
 
 	/* Set touchscreen lines */
@@ -890,6 +890,48 @@ static void qt602240_handle_pdata(struct qt602240_data *data)
 	}
 }
 
+static void qt602240_read_config(struct qt602240_data *data)
+{
+	struct qt602240_platform_data *pdata = data->pdata;
+	u8 val;
+	u8 high, low;
+
+	/* touchscreen lines */
+	qt602240_read_object(data, QT602240_TOUCH_MULTI, QT602240_TOUCH_XSIZE,
+			&val);
+		pdata->x_line = val;
+	qt602240_read_object(data, QT602240_TOUCH_MULTI, QT602240_TOUCH_YSIZE,
+			&val);
+		pdata->x_line = val;
+
+	/* touchscreen orient */
+	qt602240_read_object(data, QT602240_TOUCH_MULTI, QT602240_TOUCH_ORIENT,
+			&val);
+		pdata->orient = val;
+
+	/* touchscreen burst length */
+	qt602240_read_object(data, QT602240_TOUCH_MULTI, QT602240_TOUCH_BLEN,
+			&val);
+		pdata->blen = val;
+
+	/* touchscreen threshold */
+	qt602240_read_object(data, QT602240_TOUCH_MULTI, QT602240_TOUCH_TCHTHR,
+			&val);
+		pdata->threshold = val;
+
+	/* touchscreen resolution */
+	qt602240_read_object(data, QT602240_TOUCH_MULTI,
+			QT602240_TOUCH_XRANGE_LSB, &low);
+	qt602240_read_object(data, QT602240_TOUCH_MULTI,
+			QT602240_TOUCH_XRANGE_MSB, &high);
+	pdata->x_size = (high << 8) | (low + 1);
+	qt602240_read_object(data, QT602240_TOUCH_MULTI,
+			QT602240_TOUCH_YRANGE_LSB, &low);
+	qt602240_read_object(data, QT602240_TOUCH_MULTI,
+			QT602240_TOUCH_YRANGE_MSB, &high);
+	pdata->y_size = (high << 8) | (low + 1);
+}
+
 static int qt602240_get_info(struct qt602240_data *data)
 {
 	struct i2c_client *client = data->client;
@@ -981,23 +1023,28 @@ static int qt602240_initialize(struct qt602240_data *data)
 	if (error)
 		return error;
 
-	/* Check register init values */
-	error = qt602240_check_reg_init(data);
-	if (error)
-		return error;
+	if (data->pdata->trust_nvm) {
+		/* read configuration from device */
+		qt602240_read_config(data);
+	} else {
+		/* Check register init values */
+		error = qt602240_check_reg_init(data);
+		if (error)
+			return error;
 
-	/* Check X/Y matrix size */
-	error = qt602240_check_matrix_size(data);
-	if (error)
-		return error;
+		/* Check X/Y matrix size */
+		error = qt602240_check_matrix_size(data);
+		if (error)
+			return error;
 
-	qt602240_handle_pdata(data);
+		qt602240_handle_pdata(data);
 
-	/* Backup to memory */
-	qt602240_write_object(data, QT602240_GEN_COMMAND,
-			QT602240_COMMAND_BACKUPNV,
-			QT602240_BACKUP_VALUE);
-	msleep(QT602240_BACKUP_TIME);
+		/* 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,
@@ -1227,6 +1274,8 @@ static int __devinit qt602240_probe(struct i2c_client *client,
 	struct qt602240_data *data;
 	struct input_dev *input_dev;
 	int error;
+	u16 x_size;
+	u16 y_size;
 
 	if (!client->dev.platform_data)
 		return -EINVAL;
@@ -1249,20 +1298,6 @@ static int __devinit qt602240_probe(struct i2c_client *client,
 	__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);
 
 	data->client = client;
@@ -1276,6 +1311,30 @@ static int __devinit qt602240_probe(struct i2c_client *client,
 	if (error)
 		goto err_free_object;
 
+	/*
+	 * Bit 0 of TOUCH_ORIENT is the X/Y swap configuration.
+	 * If the axises are swapped the reporting will change, and in order to
+	 * get the scaling correct we need to swap the maximum range values
+	 * reported to the input layer.
+	 */
+	if (data->pdata->orient & 1) {
+		x_size = data->pdata->y_size;
+		y_size = data->pdata->x_size;
+	} else {
+		x_size = data->pdata->x_size;
+		y_size = data->pdata->y_size;
+	}
+
+	/* For single touch */
+	input_set_abs_params(input_dev, ABS_X, 0, x_size, 0, 0);
+	input_set_abs_params(input_dev, ABS_Y, 0, y_size, 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, x_size, 0, 0);
+	input_set_abs_params(input_dev, ABS_MT_POSITION_Y, 0, y_size, 0, 0);
+
 	error = qt602240_make_highchg(data);
 	if (error)
 		goto err_free_object;
diff --git a/include/linux/i2c/qt602240_ts.h b/include/linux/i2c/qt602240_ts.h
index c5033e1..d2aa1b6 100644
--- a/include/linux/i2c/qt602240_ts.h
+++ b/include/linux/i2c/qt602240_ts.h
@@ -33,6 +33,14 @@ struct qt602240_platform_data {
 	unsigned int threshold;
 	unsigned int voltage;
 	unsigned char orient;
+	/*
+	 * trust_nvm: 1 to trust HW config, 0 for software reconfiguration
+	 * If trust_nvm is set, all of the above values will be read from the
+	 * device (presumably loaded from non-volatile memory at reset),
+	 * instead of the other way around (configuring the device based on
+	 * platform_data).
+	 */
+	unsigned char trust_nvm;
 };
 
 #endif /* __LINUX_QT602240_TS_H */


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

* [PATCH 4/5] qt602240_ts: add optional hooks for board specific reset logic
  2010-11-16 20:41 [PATCH 0/4] qt602240_ts changes for Intel mid platform Chris Leech
                   ` (2 preceding siblings ...)
  2010-11-16 20:42 ` [PATCH 3/5] qt602240_ts: Trust factory configuration of touchscreen Chris Leech
@ 2010-11-16 20:42 ` Chris Leech
  2010-11-18 11:32   ` Joonyoung Shim
  2010-11-16 20:42 ` [PATCH 5/5] qt602240_ts: add mXT224 identifier to id_table, to match Intel mid firmware identifier Chris Leech
  4 siblings, 1 reply; 16+ messages in thread
From: Chris Leech @ 2010-11-16 20:42 UTC (permalink / raw)
  To: linux-input; +Cc: Joonyoung Shim

Hooks for board specific setup and tear-down, to be used if the controllers
reset is wired to a GPIO pin.

Signed-off-by: Chris Leech <christopher.leech@linux.intel.com>
---
 drivers/input/touchscreen/qt602240_ts.c |    7 +++++++
 include/linux/i2c/qt602240_ts.h         |    4 ++++
 2 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/drivers/input/touchscreen/qt602240_ts.c b/drivers/input/touchscreen/qt602240_ts.c
index 11055ec..efe975e 100644
--- a/drivers/input/touchscreen/qt602240_ts.c
+++ b/drivers/input/touchscreen/qt602240_ts.c
@@ -1307,6 +1307,11 @@ static int __devinit qt602240_probe(struct i2c_client *client,
 
 	i2c_set_clientdata(client, data);
 
+	if (data->pdata->hw_setup) {
+		error = data->pdata->hw_setup(client);
+		if (error)
+			goto err_free_object;
+	}
 	error = qt602240_initialize(data);
 	if (error)
 		goto err_free_object;
@@ -1376,6 +1381,8 @@ static int __devexit qt602240_remove(struct i2c_client *client)
 	sysfs_remove_group(&client->dev.kobj, &qt602240_attr_group);
 	free_irq(data->irq, data);
 	input_unregister_device(data->input_dev);
+	if (data->pdata->hw_teardown)
+		data->pdata->hw_teardown(client);
 	kfree(data->object_table);
 	kfree(data);
 
diff --git a/include/linux/i2c/qt602240_ts.h b/include/linux/i2c/qt602240_ts.h
index d2aa1b6..764dd45 100644
--- a/include/linux/i2c/qt602240_ts.h
+++ b/include/linux/i2c/qt602240_ts.h
@@ -41,6 +41,10 @@ struct qt602240_platform_data {
 	 * platform_data).
 	 */
 	unsigned char trust_nvm;
+
+	/* Board-specific HW hooks; NULL if not needed */
+	int (*hw_setup) (struct i2c_client *client);
+	void (*hw_teardown) (struct i2c_client *client);
 };
 
 #endif /* __LINUX_QT602240_TS_H */


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

* [PATCH 5/5] qt602240_ts: add mXT224 identifier to id_table, to match Intel mid firmware identifier
  2010-11-16 20:41 [PATCH 0/4] qt602240_ts changes for Intel mid platform Chris Leech
                   ` (3 preceding siblings ...)
  2010-11-16 20:42 ` [PATCH 4/5] qt602240_ts: add optional hooks for board specific reset logic Chris Leech
@ 2010-11-16 20:42 ` Chris Leech
  2010-11-18 11:34   ` Joonyoung Shim
  4 siblings, 1 reply; 16+ messages in thread
From: Chris Leech @ 2010-11-16 20:42 UTC (permalink / raw)
  To: linux-input; +Cc: Joonyoung Shim

"mXT224" is used in the SFI tables to identify the presence of this I2C device.

Signed-off-by: Chris Leech <christopher.leech@linux.intel.com>
---
 drivers/input/touchscreen/qt602240_ts.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/input/touchscreen/qt602240_ts.c b/drivers/input/touchscreen/qt602240_ts.c
index efe975e..cdd5a8b 100644
--- a/drivers/input/touchscreen/qt602240_ts.c
+++ b/drivers/input/touchscreen/qt602240_ts.c
@@ -1432,6 +1432,7 @@ static int qt602240_resume(struct i2c_client *client)
 
 static const struct i2c_device_id qt602240_id[] = {
 	{ "qt602240_ts", 0 },
+	{ "mXT224", 0 },
 	{ }
 };
 MODULE_DEVICE_TABLE(i2c, qt602240_id);


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

* Re: [PATCH 1/5] qt602240_ts: fix wrong sizeof in object table allocation
  2010-11-16 20:41 ` [PATCH 1/5] qt602240_ts: fix wrong sizeof in object table allocation Chris Leech
@ 2010-11-18 11:16   ` Joonyoung Shim
  2010-11-18 11:30     ` Joonyoung Shim
  0 siblings, 1 reply; 16+ messages in thread
From: Joonyoung Shim @ 2010-11-18 11:16 UTC (permalink / raw)
  To: Chris Leech; +Cc: linux-input

Hi, Chris.

On 2010-11-17 오전 5:41, Chris Leech wrote:
> The kcalloc call for the object table is using sizeof(struct qt602240_data)
> when it should be using sizeof(struct qt6602240_object), resulting in a larger
> allocation than is required.
>
> Signed-off-by: Chris Leech<christopher.leech@linux.intel.com>

Acked-by: Joonyoung Shim <jy0922@samsung.com>

Thanks.

> ---
>   drivers/input/touchscreen/qt602240_ts.c |    2 +-
>   1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/input/touchscreen/qt602240_ts.c b/drivers/input/touchscreen/qt602240_ts.c
> index 66b26ad..0b92c9d 100644
> --- a/drivers/input/touchscreen/qt602240_ts.c
> +++ b/drivers/input/touchscreen/qt602240_ts.c
> @@ -969,7 +969,7 @@ static int qt602240_initialize(struct qt602240_data *data)
>   		return error;
>
>   	data->object_table = kcalloc(info->object_num,
> -				     sizeof(struct qt602240_data),
> +				     sizeof(struct qt602240_object),
>   				     GFP_KERNEL);
>   	if (!data->object_table) {
>   		dev_err(&client->dev, "Failed to allocate memory\n");
>
> --
> 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] 16+ messages in thread

* Re: [PATCH 1/5] qt602240_ts: fix wrong sizeof in object table allocation
  2010-11-18 11:16   ` Joonyoung Shim
@ 2010-11-18 11:30     ` Joonyoung Shim
  0 siblings, 0 replies; 16+ messages in thread
From: Joonyoung Shim @ 2010-11-18 11:30 UTC (permalink / raw)
  To: Chris Leech; +Cc: linux-input

On 2010-11-18 오후 8:16, Joonyoung Shim wrote:
> Hi, Chris.
>
> On 2010-11-17 오전 5:41, Chris Leech wrote:
>> The kcalloc call for the object table is using sizeof(struct
>> qt602240_data)
>> when it should be using sizeof(struct qt6602240_object), resulting in
>> a larger
>> allocation than is required.
>>
>> Signed-off-by: Chris Leech<christopher.leech@linux.intel.com>
>
> Acked-by: Joonyoung Shim <jy0922@samsung.com>

Wrong e-mail address.

Acked-by: Joonyoung Shim <jy0922.shim@samsung.com>

>
> Thanks.
>
>> ---
>> drivers/input/touchscreen/qt602240_ts.c | 2 +-
>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/drivers/input/touchscreen/qt602240_ts.c
>> b/drivers/input/touchscreen/qt602240_ts.c
>> index 66b26ad..0b92c9d 100644
>> --- a/drivers/input/touchscreen/qt602240_ts.c
>> +++ b/drivers/input/touchscreen/qt602240_ts.c
>> @@ -969,7 +969,7 @@ static int qt602240_initialize(struct
>> qt602240_data *data)
>> return error;
>>
>> data->object_table = kcalloc(info->object_num,
>> - sizeof(struct qt602240_data),
>> + sizeof(struct qt602240_object),
>> GFP_KERNEL);
>> if (!data->object_table) {
>> dev_err(&client->dev, "Failed to allocate memory\n");
>>
>> --
>> 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
>

--
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] 16+ messages in thread

* Re: [PATCH 4/5] qt602240_ts: add optional hooks for board specific reset logic
  2010-11-16 20:42 ` [PATCH 4/5] qt602240_ts: add optional hooks for board specific reset logic Chris Leech
@ 2010-11-18 11:32   ` Joonyoung Shim
  2010-11-18 19:46     ` Chris Leech
  0 siblings, 1 reply; 16+ messages in thread
From: Joonyoung Shim @ 2010-11-18 11:32 UTC (permalink / raw)
  To: Chris Leech; +Cc: linux-input

Hi, Chris.

On 2010-11-17 오전 5:42, Chris Leech wrote:
> Hooks for board specific setup and tear-down, to be used if the controllers
> reset is wired to a GPIO pin.
>
> Signed-off-by: Chris Leech<christopher.leech@linux.intel.com>
> ---
>   drivers/input/touchscreen/qt602240_ts.c |    7 +++++++
>   include/linux/i2c/qt602240_ts.h         |    4 ++++
>   2 files changed, 11 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/input/touchscreen/qt602240_ts.c b/drivers/input/touchscreen/qt602240_ts.c
> index 11055ec..efe975e 100644
> --- a/drivers/input/touchscreen/qt602240_ts.c
> +++ b/drivers/input/touchscreen/qt602240_ts.c
> @@ -1307,6 +1307,11 @@ static int __devinit qt602240_probe(struct i2c_client *client,
>
>   	i2c_set_clientdata(client, data);
>
> +	if (data->pdata->hw_setup) {
> +		error = data->pdata->hw_setup(client);
> +		if (error)
> +			goto err_free_object;
> +	}
>   	error = qt602240_initialize(data);
>   	if (error)
>   		goto err_free_object;
> @@ -1376,6 +1381,8 @@ static int __devexit qt602240_remove(struct i2c_client *client)
>   	sysfs_remove_group(&client->dev.kobj,&qt602240_attr_group);
>   	free_irq(data->irq, data);
>   	input_unregister_device(data->input_dev);
> +	if (data->pdata->hw_teardown)
> +		data->pdata->hw_teardown(client);
>   	kfree(data->object_table);
>   	kfree(data);
>
> diff --git a/include/linux/i2c/qt602240_ts.h b/include/linux/i2c/qt602240_ts.h
> index d2aa1b6..764dd45 100644
> --- a/include/linux/i2c/qt602240_ts.h
> +++ b/include/linux/i2c/qt602240_ts.h
> @@ -41,6 +41,10 @@ struct qt602240_platform_data {
>   	 * platform_data).
>   	 */
>   	unsigned char trust_nvm;
> +
> +	/* Board-specific HW hooks; NULL if not needed */
> +	int (*hw_setup) (struct i2c_client *client);
> +	void (*hw_teardown) (struct i2c_client *client);

Why does client need?
--
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] 16+ messages in thread

* Re: [PATCH 5/5] qt602240_ts: add mXT224 identifier to id_table, to match Intel mid firmware identifier
  2010-11-16 20:42 ` [PATCH 5/5] qt602240_ts: add mXT224 identifier to id_table, to match Intel mid firmware identifier Chris Leech
@ 2010-11-18 11:34   ` Joonyoung Shim
  0 siblings, 0 replies; 16+ messages in thread
From: Joonyoung Shim @ 2010-11-18 11:34 UTC (permalink / raw)
  To: Chris Leech; +Cc: linux-input

On 2010-11-17 오전 5:42, Chris Leech wrote:
> "mXT224" is used in the SFI tables to identify the presence of this I2C device.
>
> Signed-off-by: Chris Leech<christopher.leech@linux.intel.com>

Acked-by: Joonyoung Shim <jy0922.shim@samsung.com>

Thanks.

> ---
>   drivers/input/touchscreen/qt602240_ts.c |    1 +
>   1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/input/touchscreen/qt602240_ts.c b/drivers/input/touchscreen/qt602240_ts.c
> index efe975e..cdd5a8b 100644
> --- a/drivers/input/touchscreen/qt602240_ts.c
> +++ b/drivers/input/touchscreen/qt602240_ts.c
> @@ -1432,6 +1432,7 @@ static int qt602240_resume(struct i2c_client *client)
>
>   static const struct i2c_device_id qt602240_id[] = {
>   	{ "qt602240_ts", 0 },
> +	{ "mXT224", 0 },
>   	{ }
>   };
>   MODULE_DEVICE_TABLE(i2c, qt602240_id);
>
>

--
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] 16+ messages in thread

* Re: [PATCH 2/5] qt602240_ts: move clearing of pending interrupt closer to request_threaded_irq
  2010-11-16 20:41 ` [PATCH 2/5] qt602240_ts: move clearing of pending interrupt closer to request_threaded_irq Chris Leech
@ 2010-11-18 12:53   ` Joonyoung Shim
  2010-11-18 19:29     ` Chris Leech
  0 siblings, 1 reply; 16+ messages in thread
From: Joonyoung Shim @ 2010-11-18 12:53 UTC (permalink / raw)
  To: Chris Leech; +Cc: linux-input

Hi, Chris.

On 2010-11-17 오전 5:41, Chris Leech wrote:
> I've seen interrupts asserted on the CHG pin between the call to make_highcgh()
> during initialization and registering the interrupt handler, leaving CHG low
> and no events get passed up.
>

The CHG pin can be affected by control of gpios connected to touch
chip. Could you check it? I think it doesn't matter where
make_highchg() exists actually.

Thanks.

> This moves the clearing of pending messages to right before the call to
> request_threaded_irq().  I still think there's a race here that could leave CHG
> stuck low, but worry about clearing the message queue while the interrupt
> handler is registered without some sort of additional locking.
>
> Signed-off-by: Chris Leech<christopher.leech@linux.intel.com>
> ---
>   drivers/input/touchscreen/qt602240_ts.c |    8 ++++----
>   1 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/input/touchscreen/qt602240_ts.c b/drivers/input/touchscreen/qt602240_ts.c
> index 0b92c9d..95496ec 100644
> --- a/drivers/input/touchscreen/qt602240_ts.c
> +++ b/drivers/input/touchscreen/qt602240_ts.c
> @@ -991,10 +991,6 @@ static int qt602240_initialize(struct qt602240_data *data)
>   	if (error)
>   		return error;
>
> -	error = qt602240_make_highchg(data);
> -	if (error)
> -		return error;
> -
>   	qt602240_handle_pdata(data);
>
>   	/* Backup to memory */
> @@ -1280,6 +1276,10 @@ static int __devinit qt602240_probe(struct i2c_client *client,
>   	if (error)
>   		goto err_free_object;
>
> +	error = qt602240_make_highchg(data);
> +	if (error)
> +		goto err_free_object;
> +
>   	error = request_threaded_irq(client->irq, NULL, qt602240_interrupt,
>   			IRQF_TRIGGER_FALLING, client->dev.driver->name, data);
>   	if (error) {
>
> --
> 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] 16+ messages in thread

* Re: [PATCH 3/5] qt602240_ts: Trust factory configuration of touchscreen.
  2010-11-16 20:42 ` [PATCH 3/5] qt602240_ts: Trust factory configuration of touchscreen Chris Leech
@ 2010-11-18 13:09   ` Joonyoung Shim
  2010-11-18 19:41     ` Chris Leech
  0 siblings, 1 reply; 16+ messages in thread
From: Joonyoung Shim @ 2010-11-18 13:09 UTC (permalink / raw)
  To: Chris Leech; +Cc: linux-input

Hi, Chris.

On 2010-11-17 오전 5:42, Chris Leech wrote:
> The maXTouch 224 touchscreen controller has non-volatile storage for system
> configuration and calibration settings.  Make it a platform option to trust
> those values, instead of reconfiguring everything.  The configuration settings
> currently being applied are determined from the firmware version, but are not
> appropriate for all systems.  This should be a compatible change with existing
> users of this driver.
>
> Signed-off-by: Chris Leech<christopher.leech@linux.intel.com>
> ---
>   drivers/input/touchscreen/qt602240_ts.c |  121 +++++++++++++++++++++++--------
>   include/linux/i2c/qt602240_ts.h         |    8 ++
>   2 files changed, 98 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/input/touchscreen/qt602240_ts.c b/drivers/input/touchscreen/qt602240_ts.c
> index 95496ec..11055ec 100644
> --- a/drivers/input/touchscreen/qt602240_ts.c
> +++ b/drivers/input/touchscreen/qt602240_ts.c
> @@ -353,7 +353,7 @@ struct qt602240_finger {
>   struct qt602240_data {
>   	struct i2c_client *client;
>   	struct input_dev *input_dev;
> -	const struct qt602240_platform_data *pdata;
> +	struct qt602240_platform_data *pdata;
>   	struct qt602240_object *object_table;
>   	struct qt602240_info info;
>   	struct qt602240_finger finger[QT602240_MAX_FINGER];
> @@ -754,7 +754,7 @@ static int qt602240_check_reg_init(struct qt602240_data *data)
>
>   static int qt602240_check_matrix_size(struct qt602240_data *data)
>   {
> -	const struct qt602240_platform_data *pdata = data->pdata;
> +	struct qt602240_platform_data *pdata = data->pdata;
>   	struct device *dev =&data->client->dev;
>   	int mode = -1;
>   	int error;
> @@ -844,7 +844,7 @@ static int qt602240_make_highchg(struct qt602240_data *data)
>
>   static void qt602240_handle_pdata(struct qt602240_data *data)
>   {
> -	const struct qt602240_platform_data *pdata = data->pdata;
> +	struct qt602240_platform_data *pdata = data->pdata;
>   	u8 voltage;
>
>   	/* Set touchscreen lines */
> @@ -890,6 +890,48 @@ static void qt602240_handle_pdata(struct qt602240_data *data)
>   	}
>   }
>
> +static void qt602240_read_config(struct qt602240_data *data)
> +{
> +	struct qt602240_platform_data *pdata = data->pdata;
> +	u8 val;
> +	u8 high, low;
> +
> +	/* touchscreen lines */
> +	qt602240_read_object(data, QT602240_TOUCH_MULTI, QT602240_TOUCH_XSIZE,
> +			&val);
> +		pdata->x_line = val;
> +	qt602240_read_object(data, QT602240_TOUCH_MULTI, QT602240_TOUCH_YSIZE,
> +			&val);
> +		pdata->x_line = val;
> +
> +	/* touchscreen orient */
> +	qt602240_read_object(data, QT602240_TOUCH_MULTI, QT602240_TOUCH_ORIENT,
> +			&val);
> +		pdata->orient = val;
> +
> +	/* touchscreen burst length */
> +	qt602240_read_object(data, QT602240_TOUCH_MULTI, QT602240_TOUCH_BLEN,
> +			&val);
> +		pdata->blen = val;
> +
> +	/* touchscreen threshold */
> +	qt602240_read_object(data, QT602240_TOUCH_MULTI, QT602240_TOUCH_TCHTHR,
> +			&val);
> +		pdata->threshold = val;
> +
> +	/* touchscreen resolution */
> +	qt602240_read_object(data, QT602240_TOUCH_MULTI,
> +			QT602240_TOUCH_XRANGE_LSB,&low);
> +	qt602240_read_object(data, QT602240_TOUCH_MULTI,
> +			QT602240_TOUCH_XRANGE_MSB,&high);
> +	pdata->x_size = (high<<  8) | (low + 1);
> +	qt602240_read_object(data, QT602240_TOUCH_MULTI,
> +			QT602240_TOUCH_YRANGE_LSB,&low);
> +	qt602240_read_object(data, QT602240_TOUCH_MULTI,
> +			QT602240_TOUCH_YRANGE_MSB,&high);
> +	pdata->y_size = (high<<  8) | (low + 1);
> +}
> +

Please don't modify pdata, the pdata means platform specific data from
machine file. The pdata of struct qt602240_data be used only in
qt602240_initialize function when driver is probed, so we need to
remove pdata from struct qt602240_data.

>   static int qt602240_get_info(struct qt602240_data *data)
>   {
>   	struct i2c_client *client = data->client;
> @@ -981,23 +1023,28 @@ static int qt602240_initialize(struct qt602240_data *data)
>   	if (error)
>   		return error;
>
> -	/* Check register init values */
> -	error = qt602240_check_reg_init(data);
> -	if (error)
> -		return error;
> +	if (data->pdata->trust_nvm) {
> +		/* read configuration from device */
> +		qt602240_read_config(data);
> +	} else {
> +		/* Check register init values */
> +		error = qt602240_check_reg_init(data);
> +		if (error)
> +			return error;
>
> -	/* Check X/Y matrix size */
> -	error = qt602240_check_matrix_size(data);
> -	if (error)
> -		return error;
> +		/* Check X/Y matrix size */
> +		error = qt602240_check_matrix_size(data);
> +		if (error)
> +			return error;
>
> -	qt602240_handle_pdata(data);
> +		qt602240_handle_pdata(data);
>
> -	/* Backup to memory */
> -	qt602240_write_object(data, QT602240_GEN_COMMAND,
> -			QT602240_COMMAND_BACKUPNV,
> -			QT602240_BACKUP_VALUE);
> -	msleep(QT602240_BACKUP_TIME);
> +		/* 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,
> @@ -1227,6 +1274,8 @@ static int __devinit qt602240_probe(struct i2c_client *client,
>   	struct qt602240_data *data;
>   	struct input_dev *input_dev;
>   	int error;
> +	u16 x_size;
> +	u16 y_size;
>
>   	if (!client->dev.platform_data)
>   		return -EINVAL;
> @@ -1249,20 +1298,6 @@ static int __devinit qt602240_probe(struct i2c_client *client,
>   	__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);
>
>   	data->client = client;
> @@ -1276,6 +1311,30 @@ static int __devinit qt602240_probe(struct i2c_client *client,
>   	if (error)
>   		goto err_free_object;
>
> +	/*
> +	 * Bit 0 of TOUCH_ORIENT is the X/Y swap configuration.
> +	 * If the axises are swapped the reporting will change, and in order to
> +	 * get the scaling correct we need to swap the maximum range values
> +	 * reported to the input layer.
> +	 */
> +	if (data->pdata->orient&  1) {
> +		x_size = data->pdata->y_size;
> +		y_size = data->pdata->x_size;
> +	} else {
> +		x_size = data->pdata->x_size;
> +		y_size = data->pdata->y_size;
> +	}
> +
> +	/* For single touch */
> +	input_set_abs_params(input_dev, ABS_X, 0, x_size, 0, 0);
> +	input_set_abs_params(input_dev, ABS_Y, 0, y_size, 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, x_size, 0, 0);
> +	input_set_abs_params(input_dev, ABS_MT_POSITION_Y, 0, y_size, 0, 0);
> +

I'm not sure but i know this max is maximum value to be supported by
touch chip.

>   	error = qt602240_make_highchg(data);
>   	if (error)
>   		goto err_free_object;
> diff --git a/include/linux/i2c/qt602240_ts.h b/include/linux/i2c/qt602240_ts.h
> index c5033e1..d2aa1b6 100644
> --- a/include/linux/i2c/qt602240_ts.h
> +++ b/include/linux/i2c/qt602240_ts.h
> @@ -33,6 +33,14 @@ struct qt602240_platform_data {
>   	unsigned int threshold;
>   	unsigned int voltage;
>   	unsigned char orient;
> +	/*
> +	 * trust_nvm: 1 to trust HW config, 0 for software reconfiguration
> +	 * If trust_nvm is set, all of the above values will be read from the
> +	 * device (presumably loaded from non-volatile memory at reset),
> +	 * instead of the other way around (configuring the device based on
> +	 * platform_data).
> +	 */
> +	unsigned char trust_nvm;
>   };
>
>   #endif /* __LINUX_QT602240_TS_H */
>
>

Thanks.
--
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] 16+ messages in thread

* Re: [PATCH 2/5] qt602240_ts: move clearing of pending interrupt closer to request_threaded_irq
  2010-11-18 12:53   ` Joonyoung Shim
@ 2010-11-18 19:29     ` Chris Leech
  2010-11-18 22:54       ` Dmitry Torokhov
  0 siblings, 1 reply; 16+ messages in thread
From: Chris Leech @ 2010-11-18 19:29 UTC (permalink / raw)
  To: Joonyoung Shim; +Cc: linux-input

On Thu, Nov 18, 2010 at 09:53:13PM +0900, Joonyoung Shim wrote:
> Hi, Chris.
>
> On 2010-11-17 오전 5:41, Chris Leech wrote:
>> I've seen interrupts asserted on the CHG pin between the call to make_highcgh()
>> during initialization and registering the interrupt handler, leaving CHG low
>> and no events get passed up.
>>
>
> The CHG pin can be affected by control of gpios connected to touch
> chip. Could you check it? I think it doesn't matter where
> make_highchg() exists actually.

My understanding is that CHG is to be used to trigger interrupts, and
the attached gpio should be set as an input.  I'm not sure trying to set
the value from the gpio side is going to work.

The problem I was seeing was the attached gpio configured as a
falling-edge-triggered interrupt stuck in the low state, and
qt602240_interrupt is never called.  What that says to me is that CHG
transitioned low before the interrupt handler was registered.  From what
I can see, the only way to reset CHG is to clear the queue of pending
messages, which is what make_highchg does.

- Chris

> Thanks.
>
>> This moves the clearing of pending messages to right before the call to
>> request_threaded_irq().  I still think there's a race here that could leave CHG
>> stuck low, but worry about clearing the message queue while the interrupt
>> handler is registered without some sort of additional locking.
>>
>> Signed-off-by: Chris Leech<christopher.leech@linux.intel.com>
>> ---
>>   drivers/input/touchscreen/qt602240_ts.c |    8 ++++----
>>   1 files changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/input/touchscreen/qt602240_ts.c b/drivers/input/touchscreen/qt602240_ts.c
>> index 0b92c9d..95496ec 100644
>> --- a/drivers/input/touchscreen/qt602240_ts.c
>> +++ b/drivers/input/touchscreen/qt602240_ts.c
>> @@ -991,10 +991,6 @@ static int qt602240_initialize(struct qt602240_data *data)
>>   	if (error)
>>   		return error;
>>
>> -	error = qt602240_make_highchg(data);
>> -	if (error)
>> -		return error;
>> -
>>   	qt602240_handle_pdata(data);
>>
>>   	/* Backup to memory */
>> @@ -1280,6 +1276,10 @@ static int __devinit qt602240_probe(struct i2c_client *client,
>>   	if (error)
>>   		goto err_free_object;
>>
>> +	error = qt602240_make_highchg(data);
>> +	if (error)
>> +		goto err_free_object;
>> +
>>   	error = request_threaded_irq(client->irq, NULL, qt602240_interrupt,
>>   			IRQF_TRIGGER_FALLING, client->dev.driver->name, data);
>>   	if (error) {
>>
>> --
>> 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] 16+ messages in thread

* Re: [PATCH 3/5] qt602240_ts: Trust factory configuration of touchscreen.
  2010-11-18 13:09   ` Joonyoung Shim
@ 2010-11-18 19:41     ` Chris Leech
  0 siblings, 0 replies; 16+ messages in thread
From: Chris Leech @ 2010-11-18 19:41 UTC (permalink / raw)
  To: Joonyoung Shim; +Cc: linux-input

On Thu, Nov 18, 2010 at 10:09:35PM +0900, Joonyoung Shim wrote:
> Hi, Chris.
> On 2010-11-17 오전 5:42, Chris Leech wrote:

>> +static void qt602240_read_config(struct qt602240_data *data)
>> +{
>> +	struct qt602240_platform_data *pdata = data->pdata;
>> +	u8 val;
>> +	u8 high, low;
>> +
>> +	/* touchscreen lines */
>> +	qt602240_read_object(data, QT602240_TOUCH_MULTI, QT602240_TOUCH_XSIZE,
>> +			&val);
>> +		pdata->x_line = val;
>> +	qt602240_read_object(data, QT602240_TOUCH_MULTI, QT602240_TOUCH_YSIZE,
>> +			&val);
>> +		pdata->x_line = val;
>> +
>> +	/* touchscreen orient */
>> +	qt602240_read_object(data, QT602240_TOUCH_MULTI, QT602240_TOUCH_ORIENT,
>> +			&val);
>> +		pdata->orient = val;
>> +
>> +	/* touchscreen burst length */
>> +	qt602240_read_object(data, QT602240_TOUCH_MULTI, QT602240_TOUCH_BLEN,
>> +			&val);
>> +		pdata->blen = val;
>> +
>> +	/* touchscreen threshold */
>> +	qt602240_read_object(data, QT602240_TOUCH_MULTI, QT602240_TOUCH_TCHTHR,
>> +			&val);
>> +		pdata->threshold = val;
>> +
>> +	/* touchscreen resolution */
>> +	qt602240_read_object(data, QT602240_TOUCH_MULTI,
>> +			QT602240_TOUCH_XRANGE_LSB,&low);
>> +	qt602240_read_object(data, QT602240_TOUCH_MULTI,
>> +			QT602240_TOUCH_XRANGE_MSB,&high);
>> +	pdata->x_size = (high<<  8) | (low + 1);
>> +	qt602240_read_object(data, QT602240_TOUCH_MULTI,
>> +			QT602240_TOUCH_YRANGE_LSB,&low);
>> +	qt602240_read_object(data, QT602240_TOUCH_MULTI,
>> +			QT602240_TOUCH_YRANGE_MSB,&high);
>> +	pdata->y_size = (high<<  8) | (low + 1);
>> +}
>> +
>
> Please don't modify pdata, the pdata means platform specific data from
> machine file. The pdata of struct qt602240_data be used only in
> qt602240_initialize function when driver is probed, so we need to
> remove pdata from struct qt602240_data.

OK. If I add the relevant fields to qt602240_data, and either copy over
from pdata or read from the hardware configuration based on trust_nvm,
does that approach seem acceptable to you?

>> +	/*
>> +	 * Bit 0 of TOUCH_ORIENT is the X/Y swap configuration.
>> +	 * If the axises are swapped the reporting will change, and in order to
>> +	 * get the scaling correct we need to swap the maximum range values
>> +	 * reported to the input layer.
>> +	 */
>> +	if (data->pdata->orient&  1) {
>> +		x_size = data->pdata->y_size;
>> +		y_size = data->pdata->x_size;
>> +	} else {
>> +		x_size = data->pdata->x_size;
>> +		y_size = data->pdata->y_size;
>> +	}
>> +
>> +	/* For single touch */
>> +	input_set_abs_params(input_dev, ABS_X, 0, x_size, 0, 0);
>> +	input_set_abs_params(input_dev, ABS_Y, 0, y_size, 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, x_size, 0, 0);
>> +	input_set_abs_params(input_dev, ABS_MT_POSITION_Y, 0, y_size, 0, 0);
>> +
>
> I'm not sure but i know this max is maximum value to be supported by
> touch chip.

I think the maxtouch scales it's output based on the TOUCH_XRANGE and
TOUCH_YRANGE configuration, so there should be the maximum values that
will ever be reported.

I can test this again, but if these values are wrong I get messed up
scaling when they events are translated to windowing coordinates.

- Chris
--
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] 16+ messages in thread

* Re: [PATCH 4/5] qt602240_ts: add optional hooks for board specific reset logic
  2010-11-18 11:32   ` Joonyoung Shim
@ 2010-11-18 19:46     ` Chris Leech
  0 siblings, 0 replies; 16+ messages in thread
From: Chris Leech @ 2010-11-18 19:46 UTC (permalink / raw)
  To: Joonyoung Shim; +Cc: linux-input

On Thu, Nov 18, 2010 at 08:32:39PM +0900, Joonyoung Shim wrote:
> Hi, Chris.
>
> On 2010-11-17 오전 5:42, Chris Leech wrote:
>> Hooks for board specific setup and tear-down, to be used if the controllers
>> reset is wired to a GPIO pin.
>>
>> Signed-off-by: Chris Leech<christopher.leech@linux.intel.com>
>> ---
>>   drivers/input/touchscreen/qt602240_ts.c |    7 +++++++
>>   include/linux/i2c/qt602240_ts.h         |    4 ++++
>>   2 files changed, 11 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/input/touchscreen/qt602240_ts.c b/drivers/input/touchscreen/qt602240_ts.c
>> index 11055ec..efe975e 100644
>> --- a/drivers/input/touchscreen/qt602240_ts.c
>> +++ b/drivers/input/touchscreen/qt602240_ts.c
>> @@ -1307,6 +1307,11 @@ static int __devinit qt602240_probe(struct i2c_client *client,
>>
>>   	i2c_set_clientdata(client, data);
>>
>> +	if (data->pdata->hw_setup) {
>> +		error = data->pdata->hw_setup(client);
>> +		if (error)
>> +			goto err_free_object;
>> +	}
>>   	error = qt602240_initialize(data);
>>   	if (error)
>>   		goto err_free_object;
>> @@ -1376,6 +1381,8 @@ static int __devexit qt602240_remove(struct i2c_client *client)
>>   	sysfs_remove_group(&client->dev.kobj,&qt602240_attr_group);
>>   	free_irq(data->irq, data);
>>   	input_unregister_device(data->input_dev);
>> +	if (data->pdata->hw_teardown)
>> +		data->pdata->hw_teardown(client);
>>   	kfree(data->object_table);
>>   	kfree(data);
>>
>> diff --git a/include/linux/i2c/qt602240_ts.h b/include/linux/i2c/qt602240_ts.h
>> index d2aa1b6..764dd45 100644
>> --- a/include/linux/i2c/qt602240_ts.h
>> +++ b/include/linux/i2c/qt602240_ts.h
>> @@ -41,6 +41,10 @@ struct qt602240_platform_data {
>>   	 * platform_data).
>>   	 */
>>   	unsigned char trust_nvm;
>> +
>> +	/* Board-specific HW hooks; NULL if not needed */
>> +	int (*hw_setup) (struct i2c_client *client);
>> +	void (*hw_teardown) (struct i2c_client *client);
>
> Why does client need?

I'm not sure I understand your question.

If it's why are these hooks needed, I want to make sure that if the
reset pin is hooked up to a gpio the device is taken out of reset
properly.

If it's why the client argument is passed, it seemed like there should
be some way to identify the device.  You could have a platform with
multiple touchscreens.

- Chris
--
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] 16+ messages in thread

* Re: [PATCH 2/5] qt602240_ts: move clearing of pending interrupt closer to request_threaded_irq
  2010-11-18 19:29     ` Chris Leech
@ 2010-11-18 22:54       ` Dmitry Torokhov
  0 siblings, 0 replies; 16+ messages in thread
From: Dmitry Torokhov @ 2010-11-18 22:54 UTC (permalink / raw)
  To: Chris Leech; +Cc: Joonyoung Shim, linux-input

On Thu, Nov 18, 2010 at 11:29:02AM -0800, Chris Leech wrote:
> On Thu, Nov 18, 2010 at 09:53:13PM +0900, Joonyoung Shim wrote:
> > Hi, Chris.
> >
> > On 2010-11-17 오전 5:41, Chris Leech wrote:
> >> I've seen interrupts asserted on the CHG pin between the call to make_highcgh()
> >> during initialization and registering the interrupt handler, leaving CHG low
> >> and no events get passed up.
> >>
> >
> > The CHG pin can be affected by control of gpios connected to touch
> > chip. Could you check it? I think it doesn't matter where
> > make_highchg() exists actually.
> 
> My understanding is that CHG is to be used to trigger interrupts, and
> the attached gpio should be set as an input.  I'm not sure trying to set
> the value from the gpio side is going to work.
> 
> The problem I was seeing was the attached gpio configured as a
> falling-edge-triggered interrupt stuck in the low state, and
> qt602240_interrupt is never called.  What that says to me is that CHG
> transitioned low before the interrupt handler was registered.  From what
> I can see, the only way to reset CHG is to clear the queue of pending
> messages, which is what make_highchg does.
> 

I wonder if it should actually go into qt602240_start().

-- 
Dmitry
--
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] 16+ messages in thread

end of thread, other threads:[~2010-11-18 22:54 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-11-16 20:41 [PATCH 0/4] qt602240_ts changes for Intel mid platform Chris Leech
2010-11-16 20:41 ` [PATCH 1/5] qt602240_ts: fix wrong sizeof in object table allocation Chris Leech
2010-11-18 11:16   ` Joonyoung Shim
2010-11-18 11:30     ` Joonyoung Shim
2010-11-16 20:41 ` [PATCH 2/5] qt602240_ts: move clearing of pending interrupt closer to request_threaded_irq Chris Leech
2010-11-18 12:53   ` Joonyoung Shim
2010-11-18 19:29     ` Chris Leech
2010-11-18 22:54       ` Dmitry Torokhov
2010-11-16 20:42 ` [PATCH 3/5] qt602240_ts: Trust factory configuration of touchscreen Chris Leech
2010-11-18 13:09   ` Joonyoung Shim
2010-11-18 19:41     ` Chris Leech
2010-11-16 20:42 ` [PATCH 4/5] qt602240_ts: add optional hooks for board specific reset logic Chris Leech
2010-11-18 11:32   ` Joonyoung Shim
2010-11-18 19:46     ` Chris Leech
2010-11-16 20:42 ` [PATCH 5/5] qt602240_ts: add mXT224 identifier to id_table, to match Intel mid firmware identifier Chris Leech
2010-11-18 11:34   ` Joonyoung Shim

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