[PATCH 00/15] atmel_mxt_ts - device tree, bootloader, etc

[PATCH 00/15] atmel_mxt_ts - device tree, bootloader, etc

[nick.dyer]

Hi Dimitry-

Here is another set of atmel_mxt_ts patches for upstream. There are some
really useful new features, but I hope nothing too controversial.

thanks

Nick Dyer

[PATCH 01/15] Input: atmel_mxt_ts - initialise IRQ before probing

[nick.dyer]

From: Nick Dyer <nick.dyer@itdev.co.uk>

The maXTouch chips use the CHG line to generate status events in bootloader
mode, and during configuration download, before there is enough information to
configure the input device. Therefore set up the interrupt handler earlier.

However, this introduces states where parts of the interrupt processing must
not run. Use data->object_table as a way to tell whether the chip information
is valid, and data->input_dev as a way to tell whether it is valid to generate
input report.

Signed-off-by: Nick Dyer <nick.dyer@itdev.co.uk>
Acked-by: Benson Leung <bleung@chromium.org>
Acked-by: Yufeng Shen <miletus@chromium.org>
---
 drivers/input/touchscreen/atmel_mxt_ts.c | 101 +++++++++++++++++++------------
 1 file changed, 62 insertions(+), 39 deletions(-)

diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index 6e0b4a2..02c374d 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -764,6 +764,12 @@ static irqreturn_t mxt_process_messages_until_invalid(struct mxt_data *data)
 
 			if (status & MXT_T6_STATUS_RESET)
 				complete(&data->reset_completion);
+		} else if (!data->input_dev) {
+			/*
+			 * do not report events if input device
+			 * is not yet registered
+			 */
+			mxt_dump_message(dev, &message);
 		} else if (mxt_is_T9_message(data, &message)) {
 			int id = reportid - data->T9_reportid_min;
 			mxt_input_touchevent(data, &message, id);
@@ -792,6 +798,9 @@ static irqreturn_t mxt_interrupt(int irq, void *dev_id)
 		return IRQ_HANDLED;
 	}
 
+	if (!data->object_table)
+		return IRQ_HANDLED;
+
 	return mxt_process_messages_until_invalid(data);
 }
 
@@ -942,6 +951,19 @@ static int mxt_make_highchg(struct mxt_data *data)
 	return 0;
 }
 
+static int mxt_acquire_irq(struct mxt_data *data)
+{
+	int error;
+
+	enable_irq(data->irq);
+
+	error = mxt_make_highchg(data);
+	if (error)
+		return error;
+
+	return 0;
+}
+
 static int mxt_get_info(struct mxt_data *data)
 {
 	struct i2c_client *client = data->client;
@@ -960,20 +982,29 @@ static int mxt_get_object_table(struct mxt_data *data)
 {
 	struct i2c_client *client = data->client;
 	size_t table_size;
+	struct mxt_object *object_table;
 	int error;
 	int i;
 	u8 reportid;
 
 	table_size = data->info.object_num * sizeof(struct mxt_object);
+	object_table = kzalloc(table_size, GFP_KERNEL);
+	if (!object_table) {
+		dev_err(&data->client->dev, "Failed to allocate memory\n");
+		return -ENOMEM;
+	}
+
 	error = __mxt_read_reg(client, MXT_OBJECT_START, table_size,
-			data->object_table);
-	if (error)
+			object_table);
+	if (error) {
+		kfree(object_table);
 		return error;
+	}
 
 	/* Valid Report IDs start counting from 1 */
 	reportid = 1;
 	for (i = 0; i < data->info.object_num; i++) {
-		struct mxt_object *object = data->object_table + i;
+		struct mxt_object *object = object_table + i;
 		u8 min_id, max_id;
 
 		le16_to_cpus(&object->start_address);
@@ -1009,6 +1040,8 @@ static int mxt_get_object_table(struct mxt_data *data)
 		}
 	}
 
+	data->object_table = object_table;
+
 	return 0;
 }
 
@@ -1080,21 +1113,17 @@ static int mxt_initialize(struct mxt_data *data)
 	if (error)
 		return error;
 
-	data->object_table = kcalloc(info->object_num,
-				     sizeof(struct mxt_object),
-				     GFP_KERNEL);
-	if (!data->object_table) {
-		dev_err(&client->dev, "Failed to allocate memory\n");
-		return -ENOMEM;
-	}
-
 	/* Get object table information */
 	error = mxt_get_object_table(data);
 	if (error) {
 		dev_err(&client->dev, "Error %d reading object table\n", error);
-		goto err_free_object_table;
+		return error;
 	}
 
+	mxt_acquire_irq(data);
+	if (error)
+		goto err_free_object_table;
+
 	/* Check register init values */
 	error = mxt_check_reg_init(data);
 	if (error) {
@@ -1345,11 +1374,7 @@ static ssize_t mxt_update_fw_store(struct device *dev,
 
 		mxt_free_object_table(data);
 
-		mxt_initialize(data);
-
-		enable_irq(data->irq);
-
-		error = mxt_make_highchg(data);
+		error = mxt_initialize(data);
 		if (error)
 			return error;
 	}
@@ -1446,9 +1471,26 @@ static int mxt_probe(struct i2c_client *client,
 	init_completion(&data->reset_completion);
 	init_completion(&data->crc_completion);
 
+	error = request_threaded_irq(client->irq, NULL, mxt_interrupt,
+				     pdata->irqflags | IRQF_ONESHOT,
+				     client->name, data);
+	if (error) {
+		dev_err(&client->dev, "Failed to register interrupt\n");
+		goto err_free_mem;
+	}
+
+	disable_irq(client->irq);
+
+	error = input_register_device(input_dev);
+	if (error) {
+		dev_err(&client->dev, "Error %d registering input device\n",
+			error);
+		goto err_free_irq;
+	}
+
 	error = mxt_initialize(data);
 	if (error)
-		goto err_free_mem;
+		goto err_unregister_device;
 
 	__set_bit(EV_ABS, input_dev->evbit);
 	__set_bit(EV_KEY, input_dev->evbit);
@@ -1499,25 +1541,6 @@ static int mxt_probe(struct i2c_client *client,
 	input_set_drvdata(input_dev, data);
 	i2c_set_clientdata(client, data);
 
-	error = request_threaded_irq(client->irq, NULL, mxt_interrupt,
-				     pdata->irqflags | IRQF_ONESHOT,
-				     client->name, data);
-	if (error) {
-		dev_err(&client->dev, "Failed to register interrupt\n");
-		goto err_free_object;
-	}
-
-	error = mxt_make_highchg(data);
-	if (error)
-		goto err_free_irq;
-
-	error = input_register_device(input_dev);
-	if (error) {
-		dev_err(&client->dev, "Error %d registering input device\n",
-			error);
-		goto err_free_irq;
-	}
-
 	error = sysfs_create_group(&client->dev.kobj, &mxt_attr_group);
 	if (error) {
 		dev_err(&client->dev, "Failure %d creating sysfs group\n",
@@ -1530,10 +1553,10 @@ static int mxt_probe(struct i2c_client *client,
 err_unregister_device:
 	input_unregister_device(input_dev);
 	input_dev = NULL;
-err_free_irq:
-	free_irq(client->irq, data);
 err_free_object:
 	kfree(data->object_table);
+err_free_irq:
+	free_irq(client->irq, data);
 err_free_mem:
 	input_free_device(input_dev);
 	kfree(data);
-- 
2.0.1

[PATCH 02/15] Input: atmel_mxt_ts - move input device init into separate function

[nick.dyer]

From: Nick Dyer <nick.dyer@itdev.co.uk>

It is useful to initialise the input device later:
- Screen parameters may not be not known yet, for instance if waiting for
  firmware loader to return.
- Device may be in bootloader mode on probe (but could still be recovered by
  firmware download).

In addition, later devices have a different touchscreen object (T100) which
requires handling differently.

This also reduces the complexity of the probe function.

Signed-off-by: Nick Dyer <nick.dyer@itdev.co.uk>
Acked-by: Yufeng Shen <miletus@chromium.org>
---
 drivers/input/touchscreen/atmel_mxt_ts.c | 191 +++++++++++++++++--------------
 1 file changed, 107 insertions(+), 84 deletions(-)

diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index 02c374d..a248414 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -1047,6 +1047,9 @@ static int mxt_get_object_table(struct mxt_data *data)
 
 static void mxt_free_object_table(struct mxt_data *data)
 {
+	input_unregister_device(data->input_dev);
+	data->input_dev = NULL;
+
 	kfree(data->object_table);
 	data->object_table = NULL;
 	data->T6_reportid = 0;
@@ -1103,6 +1106,102 @@ static int mxt_read_t9_resolution(struct mxt_data *data)
 	return 0;
 }
 
+static int mxt_input_open(struct input_dev *dev);
+static void mxt_input_close(struct input_dev *dev);
+
+static int mxt_initialize_t9_input_device(struct mxt_data *data)
+{
+	struct device *dev = &data->client->dev;
+	const struct mxt_platform_data *pdata = data->pdata;
+	struct input_dev *input_dev;
+	int error;
+	unsigned int num_mt_slots;
+	unsigned int mt_flags = 0;
+	int i;
+
+	error = mxt_read_t9_resolution(data);
+	if (error)
+		dev_warn(dev, "Failed to initialize T9 resolution\n");
+
+	input_dev = input_allocate_device();
+	if (!input_dev) {
+		dev_err(dev, "Failed to allocate memory\n");
+		return -ENOMEM;
+	}
+
+	input_dev->name = "Atmel maXTouch Touchscreen";
+	input_dev->phys = data->phys;
+	input_dev->id.bustype = BUS_I2C;
+	input_dev->dev.parent = dev;
+	input_dev->open = mxt_input_open;
+	input_dev->close = mxt_input_close;
+
+	__set_bit(EV_ABS, input_dev->evbit);
+	__set_bit(EV_KEY, input_dev->evbit);
+	__set_bit(BTN_TOUCH, input_dev->keybit);
+
+	if (pdata->t19_num_keys) {
+		__set_bit(INPUT_PROP_BUTTONPAD, input_dev->propbit);
+
+		for (i = 0; i < pdata->t19_num_keys; i++)
+			if (pdata->t19_keymap[i] != KEY_RESERVED)
+				input_set_capability(input_dev, EV_KEY,
+						     pdata->t19_keymap[i]);
+
+		mt_flags |= INPUT_MT_POINTER;
+
+		input_abs_set_res(input_dev, ABS_X, MXT_PIXELS_PER_MM);
+		input_abs_set_res(input_dev, ABS_Y, MXT_PIXELS_PER_MM);
+		input_abs_set_res(input_dev, ABS_MT_POSITION_X,
+				  MXT_PIXELS_PER_MM);
+		input_abs_set_res(input_dev, ABS_MT_POSITION_Y,
+				  MXT_PIXELS_PER_MM);
+
+		input_dev->name = "Atmel maXTouch Touchpad";
+	}
+
+	/* For single touch */
+	input_set_abs_params(input_dev, ABS_X,
+			     0, data->max_x, 0, 0);
+	input_set_abs_params(input_dev, ABS_Y,
+			     0, data->max_y, 0, 0);
+	input_set_abs_params(input_dev, ABS_PRESSURE,
+			     0, 255, 0, 0);
+
+	/* For multi touch */
+	num_mt_slots = data->T9_reportid_max - data->T9_reportid_min + 1;
+	error = input_mt_init_slots(input_dev, num_mt_slots, mt_flags);
+	if (error) {
+		dev_err(dev, "Error %d initialising slots\n", error);
+		goto err_free_mem;
+	}
+
+	input_set_abs_params(input_dev, ABS_MT_TOUCH_MAJOR,
+			     0, MXT_MAX_AREA, 0, 0);
+	input_set_abs_params(input_dev, ABS_MT_POSITION_X,
+			     0, data->max_x, 0, 0);
+	input_set_abs_params(input_dev, ABS_MT_POSITION_Y,
+			     0, data->max_y, 0, 0);
+	input_set_abs_params(input_dev, ABS_MT_PRESSURE,
+			     0, 255, 0, 0);
+
+	input_set_drvdata(input_dev, data);
+
+	error = input_register_device(input_dev);
+	if (error) {
+		dev_err(dev, "Error %d registering input device\n", error);
+		goto err_free_mem;
+	}
+
+	data->input_dev = input_dev;
+
+	return 0;
+
+err_free_mem:
+	input_free_device(input_dev);
+	return error;
+}
+
 static int mxt_initialize(struct mxt_data *data)
 {
 	struct i2c_client *client = data->client;
@@ -1132,11 +1231,9 @@ static int mxt_initialize(struct mxt_data *data)
 		goto err_free_object_table;
 	}
 
-	error = mxt_read_t9_resolution(data);
-	if (error) {
-		dev_err(&client->dev, "Failed to initialize T9 resolution\n");
+	error = mxt_initialize_t9_input_device(data);
+	if (error)
 		goto err_free_object_table;
-	}
 
 	dev_info(&client->dev,
 		 "Family: %u Variant: %u Firmware V%u.%u.%02X Objects: %u\n",
@@ -1432,40 +1529,26 @@ static void mxt_input_close(struct input_dev *dev)
 static int mxt_probe(struct i2c_client *client,
 		const struct i2c_device_id *id)
 {
-	const struct mxt_platform_data *pdata = dev_get_platdata(&client->dev);
 	struct mxt_data *data;
-	struct input_dev *input_dev;
+	const struct mxt_platform_data *pdata = dev_get_platdata(&client->dev);
 	int error;
-	unsigned int num_mt_slots;
-	unsigned int mt_flags = 0;
-	int i;
 
 	if (!pdata)
 		return -EINVAL;
 
 	data = kzalloc(sizeof(struct mxt_data), GFP_KERNEL);
-	input_dev = input_allocate_device();
-	if (!data || !input_dev) {
+	if (!data) {
 		dev_err(&client->dev, "Failed to allocate memory\n");
-		error = -ENOMEM;
-		goto err_free_mem;
+		return -ENOMEM;
 	}
 
-	input_dev->name = "Atmel maXTouch Touchscreen";
 	snprintf(data->phys, sizeof(data->phys), "i2c-%u-%04x/input0",
 		 client->adapter->nr, client->addr);
 
-	input_dev->phys = data->phys;
-
-	input_dev->id.bustype = BUS_I2C;
-	input_dev->dev.parent = &client->dev;
-	input_dev->open = mxt_input_open;
-	input_dev->close = mxt_input_close;
-
 	data->client = client;
-	data->input_dev = input_dev;
 	data->pdata = pdata;
 	data->irq = client->irq;
+	i2c_set_clientdata(client, data);
 
 	init_completion(&data->bl_completion);
 	init_completion(&data->reset_completion);
@@ -1481,84 +1564,24 @@ static int mxt_probe(struct i2c_client *client,
 
 	disable_irq(client->irq);
 
-	error = input_register_device(input_dev);
-	if (error) {
-		dev_err(&client->dev, "Error %d registering input device\n",
-			error);
-		goto err_free_irq;
-	}
-
 	error = mxt_initialize(data);
 	if (error)
-		goto err_unregister_device;
-
-	__set_bit(EV_ABS, input_dev->evbit);
-	__set_bit(EV_KEY, input_dev->evbit);
-	__set_bit(BTN_TOUCH, input_dev->keybit);
-
-	if (pdata->t19_num_keys) {
-		__set_bit(INPUT_PROP_BUTTONPAD, input_dev->propbit);
-
-		for (i = 0; i < pdata->t19_num_keys; i++)
-			if (pdata->t19_keymap[i] != KEY_RESERVED)
-				input_set_capability(input_dev, EV_KEY,
-						     pdata->t19_keymap[i]);
-
-		mt_flags |= INPUT_MT_POINTER;
-
-		input_abs_set_res(input_dev, ABS_X, MXT_PIXELS_PER_MM);
-		input_abs_set_res(input_dev, ABS_Y, MXT_PIXELS_PER_MM);
-		input_abs_set_res(input_dev, ABS_MT_POSITION_X,
-				  MXT_PIXELS_PER_MM);
-		input_abs_set_res(input_dev, ABS_MT_POSITION_Y,
-				  MXT_PIXELS_PER_MM);
-
-		input_dev->name = "Atmel maXTouch Touchpad";
-	}
-
-	/* For single touch */
-	input_set_abs_params(input_dev, ABS_X,
-			     0, data->max_x, 0, 0);
-	input_set_abs_params(input_dev, ABS_Y,
-			     0, data->max_y, 0, 0);
-	input_set_abs_params(input_dev, ABS_PRESSURE,
-			     0, 255, 0, 0);
-
-	/* For multi touch */
-	num_mt_slots = data->T9_reportid_max - data->T9_reportid_min + 1;
-	error = input_mt_init_slots(input_dev, num_mt_slots, mt_flags);
-	if (error)
-		goto err_free_object;
-	input_set_abs_params(input_dev, ABS_MT_TOUCH_MAJOR,
-			     0, MXT_MAX_AREA, 0, 0);
-	input_set_abs_params(input_dev, ABS_MT_POSITION_X,
-			     0, data->max_x, 0, 0);
-	input_set_abs_params(input_dev, ABS_MT_POSITION_Y,
-			     0, data->max_y, 0, 0);
-	input_set_abs_params(input_dev, ABS_MT_PRESSURE,
-			     0, 255, 0, 0);
-
-	input_set_drvdata(input_dev, data);
-	i2c_set_clientdata(client, data);
+		goto err_free_irq;
 
 	error = sysfs_create_group(&client->dev.kobj, &mxt_attr_group);
 	if (error) {
 		dev_err(&client->dev, "Failure %d creating sysfs group\n",
 			error);
-		goto err_unregister_device;
+		goto err_free_object;
 	}
 
 	return 0;
 
-err_unregister_device:
-	input_unregister_device(input_dev);
-	input_dev = NULL;
 err_free_object:
 	kfree(data->object_table);
 err_free_irq:
 	free_irq(client->irq, data);
 err_free_mem:
-	input_free_device(input_dev);
 	kfree(data);
 	return error;
 }
-- 
2.0.1

[PATCH 03/15] Input: atmel_mxt_ts - set pointer emulation on touchpads

[nick.dyer]

From: Benson Leung <bleung@chromium.org>

Touchpads are pointers, so make sure to pass the correct values to
input_mt_report_pointer_emulation(). Without this, tap-to-click doesn't
work.

Signed-off-by: Benson Leung <bleung@chromium.org>
Signed-off-by: Stephen Warren <swarren@nvidia.com>
Signed-off-by: Nick Dyer <nick.dyer@itdev.co.uk>
---
 drivers/input/touchscreen/atmel_mxt_ts.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index a248414..31113c3 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -654,10 +654,11 @@ static void mxt_input_button(struct mxt_data *data, struct mxt_message *message)
 	}
 }
 
-static void mxt_input_sync(struct input_dev *input_dev)
+static void mxt_input_sync(struct mxt_data *data)
 {
-	input_mt_report_pointer_emulation(input_dev, false);
-	input_sync(input_dev);
+	input_mt_report_pointer_emulation(data->input_dev,
+					  data->pdata->t19_num_keys);
+	input_sync(data->input_dev);
 }
 
 static void mxt_input_touchevent(struct mxt_data *data,
@@ -707,7 +708,7 @@ static void mxt_input_touchevent(struct mxt_data *data,
 		if (status & MXT_T9_RELEASE) {
 			input_mt_report_slot_state(input_dev,
 						   MT_TOOL_FINGER, 0);
-			mxt_input_sync(input_dev);
+			mxt_input_sync(data);
 		}
 
 		/* Touch active */
@@ -783,7 +784,7 @@ static irqreturn_t mxt_process_messages_until_invalid(struct mxt_data *data)
 	} while (reportid != 0xff);
 
 	if (update_input)
-		mxt_input_sync(data->input_dev);
+		mxt_input_sync(data);
 
 	return IRQ_HANDLED;
 }
-- 
2.0.1

[PATCH 04/15] Input: atmel_mxt_ts - implement device tree support

[nick.dyer]

From: Stephen Warren <swarren@nvidia.com>

Signed-off-by: Stephen Warren <swarren@nvidia.com>
Signed-off-by: Nick Dyer <nick.dyer@itdev.co.uk>
---
 .../devicetree/bindings/input/atmel,maxtouch.txt   | 25 +++++++++++
 drivers/input/touchscreen/atmel_mxt_ts.c           | 48 ++++++++++++++++++++++
 2 files changed, 73 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/input/atmel,maxtouch.txt

diff --git a/Documentation/devicetree/bindings/input/atmel,maxtouch.txt b/Documentation/devicetree/bindings/input/atmel,maxtouch.txt
new file mode 100644
index 0000000..60d6339
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/atmel,maxtouch.txt
@@ -0,0 +1,25 @@
+Atmel maXTouch touchscreen/touchpad
+
+Required properties:
+- compatible:
+    atmel,maxtouch
+
+- reg: The I2C address of the device
+
+- interrupts: The sink for the touchpad's IRQ output
+    See ../interrupt-controller/interrupts.txt
+
+Optional properties for main touchpad device:
+
+- linux,gpio-keymap: An array of up to 4 entries indicating the Linux
+    keycode generated by each GPIO. Linux keycodes are defined in
+    <dt-bindings/input/input.h>.
+
+Example:
+
+	touch@4b {
+		compatible = "atmel,maxtouch";
+		reg = <0x4b>;
+		interrupt-parent = <&gpio>;
+		interrupts = <TEGRA_GPIO(W, 3) GPIO_ACTIVE_HIGH>;
+	};
diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index 31113c3..5a095f6 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -22,6 +22,7 @@
 #include <linux/i2c/atmel_mxt_ts.h>
 #include <linux/input/mt.h>
 #include <linux/interrupt.h>
+#include <linux/of.h>
 #include <linux/slab.h>
 
 /* Version */
@@ -1527,6 +1528,42 @@ static void mxt_input_close(struct input_dev *dev)
 	mxt_stop(data);
 }
 
+#ifdef CONFIG_OF
+static struct mxt_platform_data *mxt_parse_dt(struct i2c_client *client)
+{
+	struct mxt_platform_data *pdata;
+	struct property *prop;
+	unsigned int *keymap;
+	int proplen, i, ret;
+	u32 keycode;
+
+	pdata = devm_kzalloc(&client->dev, sizeof(*pdata), GFP_KERNEL);
+	if (!pdata)
+		return NULL;
+
+	prop = of_find_property(client->dev.of_node, "linux,gpio-keymap",
+				&proplen);
+	if (prop) {
+		pdata->t19_num_keys = proplen / sizeof(u32);
+
+		keymap = devm_kzalloc(&client->dev,
+			pdata->t19_num_keys * sizeof(u32), GFP_KERNEL);
+		if (!keymap)
+			return NULL;
+		pdata->t19_keymap = keymap;
+		for (i = 0; i < pdata->t19_num_keys; i++) {
+			ret = of_property_read_u32_index(client->dev.of_node,
+					"linux,gpio-keymap", i, &keycode);
+			if (ret)
+				keycode = 0;
+			keymap[i] = keycode;
+		}
+	}
+
+	return pdata;
+}
+#endif
+
 static int mxt_probe(struct i2c_client *client,
 		const struct i2c_device_id *id)
 {
@@ -1534,6 +1571,10 @@ static int mxt_probe(struct i2c_client *client,
 	const struct mxt_platform_data *pdata = dev_get_platdata(&client->dev);
 	int error;
 
+#ifdef CONFIG_OF
+	if (!pdata && client->dev.of_node)
+		pdata = mxt_parse_dt(client);
+#endif
 	if (!pdata)
 		return -EINVAL;
 
@@ -1638,6 +1679,12 @@ static int mxt_resume(struct device *dev)
 
 static SIMPLE_DEV_PM_OPS(mxt_pm_ops, mxt_suspend, mxt_resume);
 
+static const struct of_device_id mxt_of_match[] = {
+	{ .compatible = "atmel,maxtouch", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, mxt_of_match);
+
 static const struct i2c_device_id mxt_id[] = {
 	{ "qt602240_ts", 0 },
 	{ "atmel_mxt_ts", 0 },
@@ -1651,6 +1698,7 @@ static struct i2c_driver mxt_driver = {
 	.driver = {
 		.name	= "atmel_mxt_ts",
 		.owner	= THIS_MODULE,
+		.of_match_table = of_match_ptr(mxt_of_match),
 		.pm	= &mxt_pm_ops,
 	},
 	.probe		= mxt_probe,
-- 
2.0.1

Re: [PATCH 04/15] Input: atmel_mxt_ts - implement device tree support

[Stephen Warren]

On 07/03/2014 09:01 AM, nick.dyer@itdev.co.uk wrote:
> From: Stephen Warren <swarren@nvidia.com>

> diff --git a/Documentation/devicetree/bindings/input/atmel,maxtouch.txt b/Documentation/devicetree/bindings/input/atmel,maxtouch.txt

> +	touch@4b {
> +		compatible = "atmel,maxtouch";
> +		reg = <0x4b>;
> +		interrupt-parent = <&gpio>;
> +		interrupts = <TEGRA_GPIO(W, 3) GPIO_ACTIVE_HIGH>;

Oops. s/GPIO_ACTIVE_HIGH/IRQ_TYPE_LEVEL_LOW/ there; I evidently typed
something stupid into the doc when I wrote it!

Re: [PATCH 04/15] Input: atmel_mxt_ts - implement device tree support

[Nick Dyer]

On 22/07/14 21:37, Stephen Warren wrote:
> On 07/03/2014 09:01 AM, nick.dyer@itdev.co.uk wrote:
>> From: Stephen Warren <swarren@nvidia.com>
> 
>> diff --git a/Documentation/devicetree/bindings/input/atmel,maxtouch.txt b/Documentation/devicetree/bindings/input/atmel,maxtouch.txt
> 
>> +	touch@4b {
>> +		compatible = "atmel,maxtouch";
>> +		reg = <0x4b>;
>> +		interrupt-parent = <&gpio>;
>> +		interrupts = <TEGRA_GPIO(W, 3) GPIO_ACTIVE_HIGH>;
> 
> Oops. s/GPIO_ACTIVE_HIGH/IRQ_TYPE_LEVEL_LOW/ there; I evidently typed
> something stupid into the doc when I wrote it!

OK, fixed this. I thought it was some tegra quirk I wasn't aware of!

Re: [PATCH 04/15] Input: atmel_mxt_ts - implement device tree support

[Stephen Warren]

On 07/03/2014 09:01 AM, nick.dyer@itdev.co.uk wrote:
> From: Stephen Warren <swarren@nvidia.com>
> 
> Signed-off-by: Stephen Warren <swarren@nvidia.com>
> Signed-off-by: Nick Dyer <nick.dyer@itdev.co.uk>

Hmm. It looks like the version of this patch that was actually applied
varies quite a bit from what was posted, in particular in the
implementation and call site of mxt_parse_dt(). It'd be nice if the
commit log had been adjusted to mention this, so it didn't look like *I*
caused all those compile errors and warnings that the 0-day builder just
found:-(

Re: [PATCH 04/15] Input: atmel_mxt_ts - implement device tree support

[Nick Dyer]

On 23/07/14 22:36, Stephen Warren wrote:
> Hmm. It looks like the version of this patch that was actually applied
> varies quite a bit from what was posted, in particular in the
> implementation and call site of mxt_parse_dt(). It'd be nice if the
> commit log had been adjusted to mention this, so it didn't look like *I*
> caused all those compile errors and warnings that the 0-day builder just
> found:-(

Apologies for my part in this. It doesn't help that this patch went through
several revisions. Is there a best practice for when to remove
signed-off-by or change authorship documented anywhere?

Re: [PATCH 04/15] Input: atmel_mxt_ts - implement device tree support

[Stephen Warren]

On 07/24/2014 09:10 AM, Nick Dyer wrote:
> On 23/07/14 22:36, Stephen Warren wrote:
>> Hmm. It looks like the version of this patch that was actually applied
>> varies quite a bit from what was posted, in particular in the
>> implementation and call site of mxt_parse_dt(). It'd be nice if the
>> commit log had been adjusted to mention this, so it didn't look like *I*
>> caused all those compile errors and warnings that the 0-day builder just
>> found:-(
> 
> Apologies for my part in this. It doesn't help that this patch went through
> several revisions. Is there a best practice for when to remove
> signed-off-by or change authorship documented anywhere?

My comment was more directed at the diff between what you posted and
what was applied.

I believe the usual practice is:

* If completely re-writing a patch using another as inspiration, change
the author and credit them in free-form text, or perhaps with a
Based-on-work-by: tag.

* If generally keeping the patch, but just changing a few things, make a
note of it in between the s-o-b lines, e.g.:

Signed-off-by: Stephen Warren <swarren@nvidia.com>
Signed-off-by: Nick Dyer <nick.dyer@itdev.co.uk>
[assign directly to pdata->t19_keymap to avoid a temporary, description
of other changes ...]
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

[PATCH 05/15] Input: atmel_mxt_ts - download device config using firmware loader

[nick.dyer]

From: Nick Dyer <nick.dyer@itdev.co.uk>

The existing implementation which encodes the configuration as a binary
blob in platform data is unsatisfactory since it requires a kernel
recompile for the configuration to be changed, and it doesn't deal well
with firmware changes that move values around on the chip.

Atmel define an ASCII format for the configuration which can be exported
from their tools. This patch implements a parser for that format which
loads the configuration via the firmware loader and sends it to the MXT
chip.

Signed-off-by: Nick Dyer <nick.dyer@itdev.co.uk>
Acked-by: Benson Leung <bleung@chromium.org>
Acked-by: Yufeng Shen <miletus@chromium.org>
---
 drivers/input/touchscreen/atmel_mxt_ts.c  | 278 ++++++++++++++++++++++--------
 drivers/platform/chrome/chromeos_laptop.c |   4 -
 include/linux/i2c/atmel_mxt_ts.h          |   3 -
 3 files changed, 204 insertions(+), 81 deletions(-)

diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index 5a095f6..e0bd2c1 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -2,6 +2,7 @@
  * Atmel maXTouch Touchscreen driver
  *
  * Copyright (C) 2010 Samsung Electronics Co.Ltd
+ * Copyright (C) 2011-2014 Atmel Corporation
  * Copyright (C) 2012 Google, Inc.
  *
  * Author: Joonyoung Shim <jy0922.shim@samsung.com>
@@ -30,8 +31,10 @@
 #define MXT_VER_21		21
 #define MXT_VER_22		22
 
-/* Firmware */
+/* Firmware files */
 #define MXT_FW_NAME		"maxtouch.fw"
+#define MXT_CFG_NAME		"maxtouch.cfg"
+#define MXT_CFG_MAGIC		"OBP_RAW V1"
 
 /* Registers */
 #define MXT_INFO		0x00
@@ -298,37 +301,6 @@ static bool mxt_object_readable(unsigned int type)
 	}
 }
 
-static bool mxt_object_writable(unsigned int type)
-{
-	switch (type) {
-	case MXT_GEN_COMMAND_T6:
-	case MXT_GEN_POWER_T7:
-	case MXT_GEN_ACQUIRE_T8:
-	case MXT_TOUCH_MULTI_T9:
-	case MXT_TOUCH_KEYARRAY_T15:
-	case MXT_TOUCH_PROXIMITY_T23:
-	case MXT_TOUCH_PROXKEY_T52:
-	case MXT_PROCI_GRIPFACE_T20:
-	case MXT_PROCG_NOISE_T22:
-	case MXT_PROCI_ONETOUCH_T24:
-	case MXT_PROCI_TWOTOUCH_T27:
-	case MXT_PROCI_GRIP_T40:
-	case MXT_PROCI_PALM_T41:
-	case MXT_PROCI_TOUCHSUPPRESSION_T42:
-	case MXT_PROCI_STYLUS_T47:
-	case MXT_PROCG_NOISESUPPRESSION_T48:
-	case MXT_SPT_COMMSCONFIG_T18:
-	case MXT_SPT_GPIOPWM_T19:
-	case MXT_SPT_SELFTEST_T25:
-	case MXT_SPT_CTECONFIG_T28:
-	case MXT_SPT_DIGITIZER_T43:
-	case MXT_SPT_CTECONFIG_T46:
-		return true;
-	default:
-		return false;
-	}
-}
-
 static void mxt_dump_message(struct device *dev,
 			     struct mxt_message *message)
 {
@@ -606,7 +578,7 @@ mxt_get_object(struct mxt_data *data, u8 type)
 			return object;
 	}
 
-	dev_err(&data->client->dev, "Invalid object type T%u\n", type);
+	dev_warn(&data->client->dev, "Invalid object type T%u\n", type);
 	return NULL;
 }
 
@@ -877,58 +849,197 @@ static void mxt_update_crc(struct mxt_data *data, u8 cmd, u8 value)
 	mxt_wait_for_completion(data, &data->crc_completion, MXT_CRC_TIMEOUT);
 }
 
-static int mxt_check_reg_init(struct mxt_data *data)
+/*
+ * mxt_update_cfg - download configuration to chip
+ *
+ * Atmel Raw Config File Format
+ *
+ * The first four lines of the raw config file contain:
+ *  1) Version
+ *  2) Chip ID Information (first 7 bytes of device memory)
+ *  3) Chip Information Block 24-bit CRC Checksum
+ *  4) Chip Configuration 24-bit CRC Checksum
+ *
+ * The rest of the file consists of one line per object instance:
+ *   <TYPE> <INSTANCE> <SIZE> <CONTENTS>
+ *
+ *   <TYPE> - 2-byte object type as hex
+ *   <INSTANCE> - 2-byte object instance number as hex
+ *   <SIZE> - 2-byte object size as hex
+ *   <CONTENTS> - array of <SIZE> 1-byte hex values
+ */
+static int mxt_update_cfg(struct mxt_data *data, const struct firmware *cfg)
 {
-	const struct mxt_platform_data *pdata = data->pdata;
-	struct mxt_object *object;
 	struct device *dev = &data->client->dev;
-	int index = 0;
-	int i, size;
+	struct mxt_info cfg_info;
+	struct mxt_object *object;
 	int ret;
+	int offset;
+	int pos;
+	int i;
+	u32 info_crc, config_crc;
+	unsigned int type, instance, size;
+	u8 val;
+	u16 reg;
 
-	if (!pdata->config) {
-		dev_dbg(dev, "No cfg data defined, skipping reg init\n");
-		return 0;
+	mxt_update_crc(data, MXT_COMMAND_REPORTALL, 1);
+
+	if (strncmp(cfg->data, MXT_CFG_MAGIC, strlen(MXT_CFG_MAGIC))) {
+		dev_err(dev, "Unrecognised config file\n");
+		ret = -EINVAL;
+		goto release;
 	}
 
-	mxt_update_crc(data, MXT_COMMAND_REPORTALL, 1);
+	pos = strlen(MXT_CFG_MAGIC);
+
+	/* Load information block and check */
+	for (i = 0; i < sizeof(struct mxt_info); i++) {
+		ret = sscanf(cfg->data + pos, "%hhx%n",
+			     (unsigned char *)&cfg_info + i,
+			     &offset);
+		if (ret != 1) {
+			dev_err(dev, "Bad format\n");
+			ret = -EINVAL;
+			goto release;
+		}
 
-	if (data->config_crc == pdata->config_crc) {
-		dev_info(dev, "Config CRC 0x%06X: OK\n", data->config_crc);
-		return 0;
+		pos += offset;
+	}
+
+	if (cfg_info.family_id != data->info.family_id) {
+		dev_err(dev, "Family ID mismatch!\n");
+		ret = -EINVAL;
+		goto release;
 	}
 
-	dev_info(dev, "Config CRC 0x%06X: does not match 0x%06X\n",
-		 data->config_crc, pdata->config_crc);
+	if (cfg_info.variant_id != data->info.variant_id) {
+		dev_err(dev, "Variant ID mismatch!\n");
+		ret = -EINVAL;
+		goto release;
+	}
 
-	for (i = 0; i < data->info.object_num; i++) {
-		object = data->object_table + i;
+	if (cfg_info.version != data->info.version)
+		dev_err(dev, "Warning: version mismatch!\n");
 
-		if (!mxt_object_writable(object->type))
+	if (cfg_info.build != data->info.build)
+		dev_err(dev, "Warning: build num mismatch!\n");
+
+	ret = sscanf(cfg->data + pos, "%x%n", &info_crc, &offset);
+	if (ret != 1) {
+		dev_err(dev, "Bad format: failed to parse Info CRC\n");
+		ret = -EINVAL;
+		goto release;
+	}
+	pos += offset;
+
+	/* Check config CRC */
+	ret = sscanf(cfg->data + pos, "%x%n", &config_crc, &offset);
+	if (ret != 1) {
+		dev_err(dev, "Bad format: failed to parse Config CRC\n");
+		ret = -EINVAL;
+		goto release;
+	}
+	pos += offset;
+
+	if (data->config_crc == config_crc) {
+		dev_dbg(dev, "Config CRC 0x%06X: OK\n", config_crc);
+		ret = 0;
+		goto release;
+	}
+
+	dev_info(dev, "Config CRC 0x%06X: does not match file 0x%06X\n",
+		 data->config_crc, config_crc);
+
+	while (pos < cfg->size) {
+		/* Read type, instance, length */
+		ret = sscanf(cfg->data + pos, "%x %x %x%n",
+			     &type, &instance, &size, &offset);
+		if (ret == 0) {
+			/* EOF */
+			ret = 1;
+			goto release;
+		} else if (ret != 3) {
+			dev_err(dev, "Bad format: failed to parse object\n");
+			ret = -EINVAL;
+			goto release;
+		}
+		pos += offset;
+
+		object = mxt_get_object(data, type);
+		if (!object) {
+			/* Skip object */
+			for (i = 0; i < size; i++) {
+				ret = sscanf(cfg->data + pos, "%hhx%n",
+					     &val,
+					     &offset);
+				pos += offset;
+			}
 			continue;
+		}
 
-		size = mxt_obj_size(object) * mxt_obj_instances(object);
-		if (index + size > pdata->config_length) {
-			dev_err(dev, "Not enough config data!\n");
-			return -EINVAL;
+		if (size > mxt_obj_size(object)) {
+			dev_err(dev, "Discarding %zu byte(s) in T%u\n",
+				size - mxt_obj_size(object), type);
 		}
 
-		ret = __mxt_write_reg(data->client, object->start_address,
-				size, &pdata->config[index]);
-		if (ret)
-			return ret;
-		index += size;
+		if (instance >= mxt_obj_instances(object)) {
+			dev_err(dev, "Object instances exceeded!\n");
+			ret = -EINVAL;
+			goto release;
+		}
+
+		reg = object->start_address + mxt_obj_size(object) * instance;
+
+		for (i = 0; i < size; i++) {
+			ret = sscanf(cfg->data + pos, "%hhx%n",
+				     &val,
+				     &offset);
+			if (ret != 1) {
+				dev_err(dev, "Bad format in T%d\n", type);
+				ret = -EINVAL;
+				goto release;
+			}
+			pos += offset;
+
+			if (i > mxt_obj_size(object))
+				continue;
+
+			ret = mxt_write_reg(data->client, reg + i, val);
+			if (ret)
+				goto release;
+
+		}
+
+		/*
+		 * If firmware is upgraded, new bytes may be added to end of
+		 * objects. It is generally forward compatible to zero these
+		 * bytes - previous behaviour will be retained. However
+		 * this does invalidate the CRC and will force a config
+		 * download every time until the configuration is updated.
+		 */
+		if (size < mxt_obj_size(object)) {
+			dev_info(dev, "Zeroing %zu byte(s) in T%d\n",
+				 mxt_obj_size(object) - size, type);
+
+			for (i = size + 1; i < mxt_obj_size(object); i++) {
+				ret = mxt_write_reg(data->client, reg + i, 0);
+				if (ret)
+					goto release;
+			}
+		}
 	}
 
 	mxt_update_crc(data, MXT_COMMAND_BACKUPNV, MXT_BACKUP_VALUE);
 
 	ret = mxt_soft_reset(data);
 	if (ret)
-		return ret;
+		goto release;
 
 	dev_info(dev, "Config successfully updated\n");
 
-	return 0;
+release:
+	release_firmware(cfg);
+	return ret;
 }
 
 static int mxt_make_highchg(struct mxt_data *data)
@@ -1204,10 +1315,17 @@ err_free_mem:
 	return error;
 }
 
+static int mxt_configure_objects(struct mxt_data *data,
+				 const struct firmware *cfg);
+
+static void mxt_config_cb(const struct firmware *cfg, void *ctx)
+{
+	mxt_configure_objects(ctx, cfg);
+}
+
 static int mxt_initialize(struct mxt_data *data)
 {
 	struct i2c_client *client = data->client;
-	struct mxt_info *info = &data->info;
 	int error;
 
 	error = mxt_get_info(data);
@@ -1225,28 +1343,40 @@ static int mxt_initialize(struct mxt_data *data)
 	if (error)
 		goto err_free_object_table;
 
-	/* Check register init values */
-	error = mxt_check_reg_init(data);
-	if (error) {
-		dev_err(&client->dev, "Error %d initializing configuration\n",
-			error);
-		goto err_free_object_table;
+	request_firmware_nowait(THIS_MODULE, true, MXT_CFG_NAME,
+				&data->client->dev, GFP_KERNEL, data,
+				mxt_config_cb);
+
+	return 0;
+
+err_free_object_table:
+	mxt_free_object_table(data);
+	return error;
+}
+
+static int mxt_configure_objects(struct mxt_data *data,
+				 const struct firmware *cfg)
+{
+	struct device *dev = &data->client->dev;
+	struct mxt_info *info = &data->info;
+	int error;
+
+	if (cfg) {
+		error = mxt_update_cfg(data, cfg);
+		if (error)
+			dev_warn(dev, "Error %d updating config\n", error);
 	}
 
 	error = mxt_initialize_t9_input_device(data);
 	if (error)
-		goto err_free_object_table;
+		return error;
 
-	dev_info(&client->dev,
+	dev_info(dev,
 		 "Family: %u Variant: %u Firmware V%u.%u.%02X Objects: %u\n",
 		 info->family_id, info->variant_id, info->version >> 4,
 		 info->version & 0xf, info->build, info->object_num);
 
 	return 0;
-
-err_free_object_table:
-	mxt_free_object_table(data);
-	return error;
 }
 
 /* Firmware Version is returned as Major.Minor.Build */
diff --git a/drivers/platform/chrome/chromeos_laptop.c b/drivers/platform/chrome/chromeos_laptop.c
index 7f1a2e2..67b316b 100644
--- a/drivers/platform/chrome/chromeos_laptop.c
+++ b/drivers/platform/chrome/chromeos_laptop.c
@@ -97,8 +97,6 @@ static struct mxt_platform_data atmel_224s_tp_platform_data = {
 	.irqflags		= IRQF_TRIGGER_FALLING,
 	.t19_num_keys		= ARRAY_SIZE(mxt_t19_keys),
 	.t19_keymap		= mxt_t19_keys,
-	.config			= NULL,
-	.config_length		= 0,
 };
 
 static struct i2c_board_info atmel_224s_tp_device = {
@@ -109,8 +107,6 @@ static struct i2c_board_info atmel_224s_tp_device = {
 
 static struct mxt_platform_data atmel_1664s_platform_data = {
 	.irqflags		= IRQF_TRIGGER_FALLING,
-	.config			= NULL,
-	.config_length		= 0,
 };
 
 static struct i2c_board_info atmel_1664s_device = {
diff --git a/include/linux/i2c/atmel_mxt_ts.h b/include/linux/i2c/atmel_mxt_ts.h
index 3891dc1..02bf6ea 100644
--- a/include/linux/i2c/atmel_mxt_ts.h
+++ b/include/linux/i2c/atmel_mxt_ts.h
@@ -17,9 +17,6 @@
 
 /* The platform data for the Atmel maXTouch touchscreen driver */
 struct mxt_platform_data {
-	const u8 *config;
-	size_t config_length;
-	u32 config_crc;
 	unsigned long irqflags;
 	u8 t19_num_keys;
 	const unsigned int *t19_keymap;
-- 
2.0.1

[PATCH 06/15] Input: atmel_mxt_ts - calculate and check CRC in config file

[nick.dyer]

From: Nick Dyer <nick.dyer@itdev.co.uk>

By validating the checksum, we can identify if the configuration is
corrupt.  In addition, this patch writes the configuration in a short
series of block writes rather than as many individual values.

Signed-off-by: Nick Dyer <nick.dyer@itdev.co.uk>
Acked-by: Benson Leung <bleung@chromium.org>
Acked-by: Yufeng Shen <miletus@chromium.org>
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/touchscreen/atmel_mxt_ts.c | 232 +++++++++++++++++++++++--------
 1 file changed, 177 insertions(+), 55 deletions(-)

diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index e0bd2c1..90c2c4b 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -48,6 +48,8 @@
 #define MXT_OBJECT_START	0x07
 
 #define MXT_OBJECT_SIZE		6
+#define MXT_INFO_CHECKSUM_SIZE	3
+#define MXT_MAX_BLOCK_WRITE	256
 
 /* Object types */
 #define MXT_DEBUG_DIAGNOSTIC_T37	37
@@ -238,12 +240,15 @@ struct mxt_data {
 	unsigned int max_x;
 	unsigned int max_y;
 	bool in_bootloader;
+	u16 mem_size;
 	u32 config_crc;
+	u32 info_crc;
 	u8 bootloader_addr;
 
 	/* Cached parameters from object table */
 	u8 T6_reportid;
 	u16 T6_address;
+	u16 T7_address;
 	u8 T9_reportid_min;
 	u8 T9_reportid_max;
 	u8 T19_reportid;
@@ -849,6 +854,45 @@ static void mxt_update_crc(struct mxt_data *data, u8 cmd, u8 value)
 	mxt_wait_for_completion(data, &data->crc_completion, MXT_CRC_TIMEOUT);
 }
 
+static void mxt_calc_crc24(u32 *crc, u8 firstbyte, u8 secondbyte)
+{
+	static const unsigned int crcpoly = 0x80001B;
+	u32 result;
+	u32 data_word;
+
+	data_word = (secondbyte << 8) | firstbyte;
+	result = ((*crc << 1) ^ data_word);
+
+	if (result & 0x1000000)
+		result ^= crcpoly;
+
+	*crc = result;
+}
+
+static u32 mxt_calculate_crc(u8 *base, off_t start_off, off_t end_off)
+{
+	u32 crc = 0;
+	u8 *ptr = base + start_off;
+	u8 *last_val = base + end_off - 1;
+
+	if (end_off < start_off)
+		return -EINVAL;
+
+	while (ptr < last_val) {
+		mxt_calc_crc24(&crc, *ptr, *(ptr + 1));
+		ptr += 2;
+	}
+
+	/* if len is odd, fill the last byte with 0 */
+	if (ptr == last_val)
+		mxt_calc_crc24(&crc, *ptr, 0);
+
+	/* Mask to 24-bit */
+	crc &= 0x00FFFFFF;
+
+	return crc;
+}
+
 /*
  * mxt_update_cfg - download configuration to chip
  *
@@ -875,9 +919,13 @@ static int mxt_update_cfg(struct mxt_data *data, const struct firmware *cfg)
 	struct mxt_object *object;
 	int ret;
 	int offset;
-	int pos;
+	int data_pos;
+	int byte_offset;
 	int i;
-	u32 info_crc, config_crc;
+	int cfg_start_ofs;
+	u32 info_crc, config_crc, calculated_crc;
+	u8 *config_mem;
+	size_t config_mem_size;
 	unsigned int type, instance, size;
 	u8 val;
 	u16 reg;
@@ -890,11 +938,11 @@ static int mxt_update_cfg(struct mxt_data *data, const struct firmware *cfg)
 		goto release;
 	}
 
-	pos = strlen(MXT_CFG_MAGIC);
+	data_pos = strlen(MXT_CFG_MAGIC);
 
 	/* Load information block and check */
 	for (i = 0; i < sizeof(struct mxt_info); i++) {
-		ret = sscanf(cfg->data + pos, "%hhx%n",
+		ret = sscanf(cfg->data + data_pos, "%hhx%n",
 			     (unsigned char *)&cfg_info + i,
 			     &offset);
 		if (ret != 1) {
@@ -903,7 +951,7 @@ static int mxt_update_cfg(struct mxt_data *data, const struct firmware *cfg)
 			goto release;
 		}
 
-		pos += offset;
+		data_pos += offset;
 	}
 
 	if (cfg_info.family_id != data->info.family_id) {
@@ -918,125 +966,188 @@ static int mxt_update_cfg(struct mxt_data *data, const struct firmware *cfg)
 		goto release;
 	}
 
-	if (cfg_info.version != data->info.version)
-		dev_err(dev, "Warning: version mismatch!\n");
-
-	if (cfg_info.build != data->info.build)
-		dev_err(dev, "Warning: build num mismatch!\n");
-
-	ret = sscanf(cfg->data + pos, "%x%n", &info_crc, &offset);
+	/* Read CRCs */
+	ret = sscanf(cfg->data + data_pos, "%x%n", &info_crc, &offset);
 	if (ret != 1) {
 		dev_err(dev, "Bad format: failed to parse Info CRC\n");
 		ret = -EINVAL;
 		goto release;
 	}
-	pos += offset;
+	data_pos += offset;
 
-	/* Check config CRC */
-	ret = sscanf(cfg->data + pos, "%x%n", &config_crc, &offset);
+	ret = sscanf(cfg->data + data_pos, "%x%n", &config_crc, &offset);
 	if (ret != 1) {
 		dev_err(dev, "Bad format: failed to parse Config CRC\n");
 		ret = -EINVAL;
 		goto release;
 	}
-	pos += offset;
+	data_pos += offset;
 
-	if (data->config_crc == config_crc) {
-		dev_dbg(dev, "Config CRC 0x%06X: OK\n", config_crc);
-		ret = 0;
-		goto release;
+	/*
+	 * The Info Block CRC is calculated over mxt_info and the object
+	 * table. If it does not match then we are trying to load the
+	 * configuration from a different chip or firmware version, so
+	 * the configuration CRC is invalid anyway.
+	 */
+	if (info_crc == data->info_crc) {
+		if (config_crc == 0 || data->config_crc == 0) {
+			dev_info(dev, "CRC zero, attempting to apply config\n");
+		} else if (config_crc == data->config_crc) {
+			dev_dbg(dev, "Config CRC 0x%06X: OK\n",
+				 data->config_crc);
+			ret = 0;
+			goto release;
+		} else {
+			dev_info(dev, "Config CRC 0x%06X: does not match file 0x%06X\n",
+				 data->config_crc, config_crc);
+		}
+	} else {
+		dev_warn(dev,
+			 "Warning: Info CRC error - device=0x%06X file=0x%06X\n",
+			 data->info_crc, info_crc);
 	}
 
-	dev_info(dev, "Config CRC 0x%06X: does not match file 0x%06X\n",
-		 data->config_crc, config_crc);
+	/* Malloc memory to store configuration */
+	cfg_start_ofs = MXT_OBJECT_START +
+			data->info.object_num * sizeof(struct mxt_object) +
+			MXT_INFO_CHECKSUM_SIZE;
+	config_mem_size = data->mem_size - cfg_start_ofs;
+	config_mem = kzalloc(config_mem_size, GFP_KERNEL);
+	if (!config_mem) {
+		dev_err(dev, "Failed to allocate memory\n");
+		ret = -ENOMEM;
+		goto release;
+	}
 
-	while (pos < cfg->size) {
+	while (data_pos < cfg->size) {
 		/* Read type, instance, length */
-		ret = sscanf(cfg->data + pos, "%x %x %x%n",
+		ret = sscanf(cfg->data + data_pos, "%x %x %x%n",
 			     &type, &instance, &size, &offset);
 		if (ret == 0) {
 			/* EOF */
-			ret = 1;
-			goto release;
+			break;
 		} else if (ret != 3) {
 			dev_err(dev, "Bad format: failed to parse object\n");
 			ret = -EINVAL;
-			goto release;
+			goto release_mem;
 		}
-		pos += offset;
+		data_pos += offset;
 
 		object = mxt_get_object(data, type);
 		if (!object) {
 			/* Skip object */
 			for (i = 0; i < size; i++) {
-				ret = sscanf(cfg->data + pos, "%hhx%n",
+				ret = sscanf(cfg->data + data_pos, "%hhx%n",
 					     &val,
 					     &offset);
-				pos += offset;
+				data_pos += offset;
 			}
 			continue;
 		}
 
 		if (size > mxt_obj_size(object)) {
-			dev_err(dev, "Discarding %zu byte(s) in T%u\n",
-				size - mxt_obj_size(object), type);
+			/*
+			 * Either we are in fallback mode due to wrong
+			 * config or config from a later fw version,
+			 * or the file is corrupt or hand-edited.
+			 */
+			dev_warn(dev, "Discarding %zu byte(s) in T%u\n",
+				 size - mxt_obj_size(object), type);
+		} else if (mxt_obj_size(object) > size) {
+			/*
+			 * If firmware is upgraded, new bytes may be added to
+			 * end of objects. It is generally forward compatible
+			 * to zero these bytes - previous behaviour will be
+			 * retained. However this does invalidate the CRC and
+			 * will force fallback mode until the configuration is
+			 * updated. We warn here but do nothing else - the
+			 * malloc has zeroed the entire configuration.
+			 */
+			dev_warn(dev, "Zeroing %zu byte(s) in T%d\n",
+				 mxt_obj_size(object) - size, type);
 		}
 
 		if (instance >= mxt_obj_instances(object)) {
 			dev_err(dev, "Object instances exceeded!\n");
 			ret = -EINVAL;
-			goto release;
+			goto release_mem;
 		}
 
 		reg = object->start_address + mxt_obj_size(object) * instance;
 
 		for (i = 0; i < size; i++) {
-			ret = sscanf(cfg->data + pos, "%hhx%n",
+			ret = sscanf(cfg->data + data_pos, "%hhx%n",
 				     &val,
 				     &offset);
 			if (ret != 1) {
 				dev_err(dev, "Bad format in T%d\n", type);
 				ret = -EINVAL;
-				goto release;
+				goto release_mem;
 			}
-			pos += offset;
+			data_pos += offset;
 
 			if (i > mxt_obj_size(object))
 				continue;
 
-			ret = mxt_write_reg(data->client, reg + i, val);
-			if (ret)
-				goto release;
+			byte_offset = reg + i - cfg_start_ofs;
 
+			if ((byte_offset >= 0)
+			    && (byte_offset <= config_mem_size)) {
+				*(config_mem + byte_offset) = val;
+			} else {
+				dev_err(dev, "Bad object: reg:%d, T%d, ofs=%d\n",
+					reg, object->type, byte_offset);
+				ret = -EINVAL;
+				goto release_mem;
+			}
 		}
+	}
 
-		/*
-		 * If firmware is upgraded, new bytes may be added to end of
-		 * objects. It is generally forward compatible to zero these
-		 * bytes - previous behaviour will be retained. However
-		 * this does invalidate the CRC and will force a config
-		 * download every time until the configuration is updated.
-		 */
-		if (size < mxt_obj_size(object)) {
-			dev_info(dev, "Zeroing %zu byte(s) in T%d\n",
-				 mxt_obj_size(object) - size, type);
+	/* Calculate crc of the received configs (not the raw config file) */
+	if (data->T7_address < cfg_start_ofs) {
+		dev_err(dev, "Bad T7 address, T7addr = %x, config offset %x\n",
+			data->T7_address, cfg_start_ofs);
+		ret = 0;
+		goto release_mem;
+	}
 
-			for (i = size + 1; i < mxt_obj_size(object); i++) {
-				ret = mxt_write_reg(data->client, reg + i, 0);
-				if (ret)
-					goto release;
-			}
+	calculated_crc = mxt_calculate_crc(config_mem,
+					   data->T7_address - cfg_start_ofs,
+					   config_mem_size);
+
+	if (config_crc > 0 && (config_crc != calculated_crc))
+		dev_warn(dev, "Config CRC error, calculated=%06X, file=%06X\n",
+			 calculated_crc, config_crc);
+
+	/* Write configuration as blocks */
+	byte_offset = 0;
+	while (byte_offset < config_mem_size) {
+		size = config_mem_size - byte_offset;
+
+		if (size > MXT_MAX_BLOCK_WRITE)
+			size = MXT_MAX_BLOCK_WRITE;
+
+		ret = __mxt_write_reg(data->client,
+				      cfg_start_ofs + byte_offset,
+				      size, config_mem + byte_offset);
+		if (ret != 0) {
+			dev_err(dev, "Config write error, ret=%d\n", ret);
+			goto release_mem;
 		}
+
+		byte_offset += size;
 	}
 
 	mxt_update_crc(data, MXT_COMMAND_BACKUPNV, MXT_BACKUP_VALUE);
 
 	ret = mxt_soft_reset(data);
 	if (ret)
-		goto release;
+		goto release_mem;
 
 	dev_info(dev, "Config successfully updated\n");
 
+release_mem:
+	kfree(config_mem);
 release:
 	release_firmware(cfg);
 	return ret;
@@ -1099,6 +1210,7 @@ static int mxt_get_object_table(struct mxt_data *data)
 	int error;
 	int i;
 	u8 reportid;
+	u16 end_address;
 
 	table_size = data->info.object_num * sizeof(struct mxt_object);
 	object_table = kzalloc(table_size, GFP_KERNEL);
@@ -1116,6 +1228,7 @@ static int mxt_get_object_table(struct mxt_data *data)
 
 	/* Valid Report IDs start counting from 1 */
 	reportid = 1;
+	data->mem_size = 0;
 	for (i = 0; i < data->info.object_num; i++) {
 		struct mxt_object *object = object_table + i;
 		u8 min_id, max_id;
@@ -1143,6 +1256,9 @@ static int mxt_get_object_table(struct mxt_data *data)
 			data->T6_reportid = min_id;
 			data->T6_address = object->start_address;
 			break;
+		case MXT_GEN_POWER_T7:
+			data->T7_address = object->start_address;
+			break;
 		case MXT_TOUCH_MULTI_T9:
 			data->T9_reportid_min = min_id;
 			data->T9_reportid_max = max_id;
@@ -1151,6 +1267,12 @@ static int mxt_get_object_table(struct mxt_data *data)
 			data->T19_reportid = min_id;
 			break;
 		}
+
+		end_address = object->start_address
+			+ mxt_obj_size(object) * mxt_obj_instances(object) - 1;
+
+		if (end_address >= data->mem_size)
+			data->mem_size = end_address + 1;
 	}
 
 	data->object_table = object_table;
-- 
2.0.1

[PATCH 07/15] Input: atmel_mxt_ts - use deep sleep mode when stopped

[nick.dyer]

From: Nick Dyer <nick.dyer@itdev.co.uk>

By writing zero to both the active and idle cycle times the maXTouch device
is put into a deep sleep mode when it consumes minimal power. It is
unnecessary to change the configuration of any other objects (for example
to disable T9 touchscreen).

It is counterproductive to reset the chip on resume, it will result in a
long delay. However it is necessary to issue a calibrate command after the
chip has spent any time in deep sleep.

This patch also deals with the situation where the power configuration is
zero on probe, which would mean that the device never wakes up to execute
commands.

Signed-off-by: Nick Dyer <nick.dyer@itdev.co.uk>
Acked-by: Benson Leung <bleung@chromium.org>
Acked-by: Yufeng Shen <miletus@chromium.org>
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/touchscreen/atmel_mxt_ts.c | 99 +++++++++++++++++++++++---------
 1 file changed, 73 insertions(+), 26 deletions(-)

diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index 90c2c4b..9b0cccd 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -91,9 +91,13 @@
 #define MXT_T6_STATUS_RESET	(1 << 7)
 
 /* MXT_GEN_POWER_T7 field */
-#define MXT_POWER_IDLEACQINT	0
-#define MXT_POWER_ACTVACQINT	1
-#define MXT_POWER_ACTV2IDLETO	2
+struct t7_config {
+	u8 idle;
+	u8 active;
+} __packed;
+
+#define MXT_POWER_CFG_RUN		0
+#define MXT_POWER_CFG_DEEPSLEEP		1
 
 /* MXT_GEN_ACQUIRE_T8 field */
 #define MXT_ACQUIRE_CHRGTIME	0
@@ -105,7 +109,6 @@
 #define MXT_ACQUIRE_ATCHCALSTHR	7
 
 /* MXT_TOUCH_MULTI_T9 field */
-#define MXT_TOUCH_CTRL		0
 #define MXT_T9_ORIENT		9
 #define MXT_T9_RANGE		18
 
@@ -244,6 +247,7 @@ struct mxt_data {
 	u32 config_crc;
 	u32 info_crc;
 	u8 bootloader_addr;
+	struct t7_config t7_cfg;
 
 	/* Cached parameters from object table */
 	u8 T6_reportid;
@@ -602,20 +606,6 @@ static int mxt_read_message(struct mxt_data *data,
 			sizeof(struct mxt_message), message);
 }
 
-static int mxt_write_object(struct mxt_data *data,
-				 u8 type, u8 offset, u8 val)
-{
-	struct mxt_object *object;
-	u16 reg;
-
-	object = mxt_get_object(data, type);
-	if (!object || offset >= mxt_obj_size(object))
-		return -EINVAL;
-
-	reg = object->start_address;
-	return mxt_write_reg(data->client, reg + offset, val);
-}
-
 static void mxt_input_button(struct mxt_data *data, struct mxt_message *message)
 {
 	struct input_dev *input = data->input_dev;
@@ -1153,6 +1143,60 @@ release:
 	return ret;
 }
 
+static int mxt_set_t7_power_cfg(struct mxt_data *data, u8 sleep)
+{
+	struct device *dev = &data->client->dev;
+	int error;
+	struct t7_config *new_config;
+	struct t7_config deepsleep = { .active = 0, .idle = 0 };
+
+	if (sleep == MXT_POWER_CFG_DEEPSLEEP)
+		new_config = &deepsleep;
+	else
+		new_config = &data->t7_cfg;
+
+	error = __mxt_write_reg(data->client, data->T7_address,
+				sizeof(data->t7_cfg), new_config);
+	if (error)
+		return error;
+
+	dev_dbg(dev, "Set T7 ACTV:%d IDLE:%d\n",
+		new_config->active, new_config->idle);
+
+	return 0;
+}
+
+static int mxt_init_t7_power_cfg(struct mxt_data *data)
+{
+	struct device *dev = &data->client->dev;
+	int error;
+	bool retry = false;
+
+recheck:
+	error = __mxt_read_reg(data->client, data->T7_address,
+				sizeof(data->t7_cfg), &data->t7_cfg);
+	if (error)
+		return error;
+
+	if (data->t7_cfg.active == 0 || data->t7_cfg.idle == 0) {
+		if (!retry) {
+			dev_dbg(dev, "T7 cfg zero, resetting\n");
+			mxt_soft_reset(data);
+			retry = true;
+			goto recheck;
+		} else {
+			dev_dbg(dev, "T7 cfg zero after reset, overriding\n");
+			data->t7_cfg.active = 20;
+			data->t7_cfg.idle = 100;
+			return mxt_set_t7_power_cfg(data, MXT_POWER_CFG_RUN);
+		}
+	}
+
+	dev_dbg(dev, "Initialized power cfg: ACTV %d, IDLE %d\n",
+		data->t7_cfg.active, data->t7_cfg.idle);
+	return 0;
+}
+
 static int mxt_make_highchg(struct mxt_data *data)
 {
 	struct device *dev = &data->client->dev;
@@ -1489,6 +1533,12 @@ static int mxt_configure_objects(struct mxt_data *data,
 			dev_warn(dev, "Error %d updating config\n", error);
 	}
 
+	error = mxt_init_t7_power_cfg(data);
+	if (error) {
+		dev_err(dev, "Failed to initialize power cfg\n");
+		return error;
+	}
+
 	error = mxt_initialize_t9_input_device(data);
 	if (error)
 		return error;
@@ -1752,16 +1802,15 @@ static const struct attribute_group mxt_attr_group = {
 
 static void mxt_start(struct mxt_data *data)
 {
-	/* Touch enable */
-	mxt_write_object(data,
-			MXT_TOUCH_MULTI_T9, MXT_TOUCH_CTRL, 0x83);
+	mxt_set_t7_power_cfg(data, MXT_POWER_CFG_RUN);
+
+	/* Recalibrate since chip has been in deep sleep */
+	mxt_t6_command(data, MXT_COMMAND_CALIBRATE, 1, false);
 }
 
 static void mxt_stop(struct mxt_data *data)
 {
-	/* Touch disable */
-	mxt_write_object(data,
-			MXT_TOUCH_MULTI_T9, MXT_TOUCH_CTRL, 0);
+	mxt_set_t7_power_cfg(data, MXT_POWER_CFG_DEEPSLEEP);
 }
 
 static int mxt_input_open(struct input_dev *dev)
@@ -1916,8 +1965,6 @@ static int mxt_resume(struct device *dev)
 	struct mxt_data *data = i2c_get_clientdata(client);
 	struct input_dev *input_dev = data->input_dev;
 
-	mxt_soft_reset(data);
-
 	mutex_lock(&input_dev->mutex);
 
 	if (input_dev->users)
-- 
2.0.1

[PATCH 08/15] Input: atmel_mxt_ts - handle APP_CRC_FAIL on startup

[nick.dyer]

From: Nick Dyer <nick.dyer@itdev.co.uk>

If the bootloader on the touchscreen controller fails to initialise the
firmware image, it stays in bootloader mode and reports a failure. It is
possible to reflash a working firmware image from this state.

Signed-off-by: Nick Dyer <nick.dyer@itdev.co.uk>
Acked-by: Benson Leung <bleung@chromium.org>
Acked-by: Yufeng Shen <miletus@chromium.org>
---
 drivers/input/touchscreen/atmel_mxt_ts.c | 53 ++++++++++++++++++++++++++------
 1 file changed, 43 insertions(+), 10 deletions(-)

diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index 9b0cccd..aa957d2 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -408,6 +408,30 @@ static int mxt_lookup_bootloader_address(struct mxt_data *data)
 	return 0;
 }
 
+static int mxt_probe_bootloader(struct mxt_data *data)
+{
+	struct device *dev = &data->client->dev;
+	int ret;
+	u8 val;
+	bool crc_failure;
+
+	ret = mxt_lookup_bootloader_address(data);
+	if (ret)
+		return ret;
+
+	ret = mxt_bootloader_read(data, &val, 1);
+	if (ret)
+		return ret;
+
+	/* Check app crc fail mode */
+	crc_failure = (val & ~MXT_BOOT_STATUS_MASK) == MXT_APP_CRC_FAIL;
+
+	dev_err(dev, "Detected bootloader, status:%02X%s\n",
+			val, crc_failure ? ", APP_CRC_FAIL" : "");
+
+	return 0;
+}
+
 static u8 mxt_get_bootloader_version(struct mxt_data *data, u8 val)
 {
 	struct device *dev = &data->client->dev;
@@ -467,6 +491,7 @@ recheck:
 	switch (state) {
 	case MXT_WAITING_BOOTLOAD_CMD:
 	case MXT_WAITING_FRAME_DATA:
+	case MXT_APP_CRC_FAIL:
 		val &= ~MXT_BOOT_STATUS_MASK;
 		break;
 	case MXT_FRAME_CRC_PASS:
@@ -1495,8 +1520,14 @@ static int mxt_initialize(struct mxt_data *data)
 	int error;
 
 	error = mxt_get_info(data);
-	if (error)
-		return error;
+	if (error) {
+		error = mxt_probe_bootloader(data);
+		if (error)
+			return error;
+
+		data->in_bootloader = true;
+		return 0;
+	}
 
 	/* Get object table information */
 	error = mxt_get_object_table(data);
@@ -1680,15 +1711,19 @@ static int mxt_load_fw(struct device *dev, const char *fn)
 	if (ret)
 		goto release_firmware;
 
-	/* Change to the bootloader mode */
-	data->in_bootloader = true;
+	if (!data->in_bootloader) {
+		/* Change to the bootloader mode */
+		data->in_bootloader = true;
 
-	ret = mxt_t6_command(data, MXT_COMMAND_RESET, MXT_BOOT_VALUE, false);
-	if (ret)
-		goto release_firmware;
+		ret = mxt_t6_command(data, MXT_COMMAND_RESET,
+				     MXT_BOOT_VALUE, false);
+		if (ret)
+			goto release_firmware;
 
-	msleep(MXT_RESET_TIME);
+		msleep(MXT_RESET_TIME);
+	}
 
+	mxt_free_object_table(data);
 	reinit_completion(&data->bl_completion);
 
 	ret = mxt_check_bootloader(data, MXT_WAITING_BOOTLOAD_CMD);
@@ -1773,8 +1808,6 @@ static ssize_t mxt_update_fw_store(struct device *dev,
 	} else {
 		dev_info(dev, "The firmware update succeeded\n");
 
-		mxt_free_object_table(data);
-
 		error = mxt_initialize(data);
 		if (error)
 			return error;
-- 
2.0.1

[PATCH 09/15] Input: atmel_mxt_ts - handle bootloader previously unlocked

[nick.dyer]

From: Nick Dyer <nick.dyer@itdev.co.uk>

On a warm probe, the device might be in a state where an flash operation was
not completed.

Signed-off-by: Nick Dyer <nick.dyer@itdev.co.uk>
Acked-by: Benson Leung <bleung@chromium.org>
Acked-by: Yufeng Shen <miletus@chromium.org>
---
 drivers/input/touchscreen/atmel_mxt_ts.c | 27 ++++++++++++++++++---------
 1 file changed, 18 insertions(+), 9 deletions(-)

diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index aa957d2..f9db3e1 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -453,14 +453,15 @@ static u8 mxt_get_bootloader_version(struct mxt_data *data, u8 val)
 	}
 }
 
-static int mxt_check_bootloader(struct mxt_data *data, unsigned int state)
+static int mxt_check_bootloader(struct mxt_data *data, unsigned int state,
+				bool wait)
 {
 	struct device *dev = &data->client->dev;
 	u8 val;
 	int ret;
 
 recheck:
-	if (state != MXT_WAITING_BOOTLOAD_CMD) {
+	if (wait) {
 		/*
 		 * In application update mode, the interrupt
 		 * line signals state transitions. We must wait for the
@@ -1726,15 +1727,23 @@ static int mxt_load_fw(struct device *dev, const char *fn)
 	mxt_free_object_table(data);
 	reinit_completion(&data->bl_completion);
 
-	ret = mxt_check_bootloader(data, MXT_WAITING_BOOTLOAD_CMD);
-	if (ret)
-		goto disable_irq;
+	ret = mxt_check_bootloader(data, MXT_WAITING_BOOTLOAD_CMD, false);
+	if (ret) {
+		/* Bootloader may still be unlocked from previous attempt */
+		ret = mxt_check_bootloader(data, MXT_WAITING_FRAME_DATA, false);
+		if (ret)
+			goto disable_irq;
+	} else {
+		dev_info(dev, "Unlocking bootloader\n");
 
-	/* Unlock bootloader */
-	mxt_unlock_bootloader(data);
+		/* Unlock bootloader */
+		ret = mxt_unlock_bootloader(data);
+		if (ret)
+			goto disable_irq;
+	}
 
 	while (pos < fw->size) {
-		ret = mxt_check_bootloader(data, MXT_WAITING_FRAME_DATA);
+		ret = mxt_check_bootloader(data, MXT_WAITING_FRAME_DATA, true);
 		if (ret)
 			goto disable_irq;
 
@@ -1748,7 +1757,7 @@ static int mxt_load_fw(struct device *dev, const char *fn)
 		if (ret)
 			goto disable_irq;
 
-		ret = mxt_check_bootloader(data, MXT_FRAME_CRC_PASS);
+		ret = mxt_check_bootloader(data, MXT_FRAME_CRC_PASS, true);
 		if (ret) {
 			retry++;
 
-- 
2.0.1

[PATCH 10/15] Input: atmel_mxt_ts - add bootloader addresses for new chips

[nick.dyer]

From: Nick Dyer <nick.dyer@itdev.co.uk>

Later chips (for example mXT1664S) different mappings for bootloader addresses.
This means that we must look at the family ID to determine which address to
use.

Signed-off-by: Nick Dyer <nick.dyer@itdev.co.uk>
Acked-by: Benson Leung <bleung@chromium.org>
Acked-by: Yufeng Shen <miletus@chromium.org>
---
 drivers/input/touchscreen/atmel_mxt_ts.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index f9db3e1..5fe3285 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -391,6 +391,12 @@ static int mxt_lookup_bootloader_address(struct mxt_data *data)
 	switch (appmode) {
 	case 0x4a:
 	case 0x4b:
+		/* Chips after 1664S use different scheme */
+		if (data->info.family_id >= 0xa2) {
+			bootloader = appmode - 0x24;
+			break;
+		}
+		/* Fall through for normal case */
 	case 0x4c:
 	case 0x4d:
 	case 0x5a:
-- 
2.0.1

[PATCH 11/15] Input: atmel_mxt_ts - recover from bootloader on probe

[nick.dyer]

From: Nick Dyer <nick.dyer@itdev.co.uk>

The MXT device may be in bootloader mode on probe, due to:
1) APP CRC failure, either:
  a) flash corruption
  b) bad power or other intermittent problem while checking CRC
2) If the device has been reset 10 or more times without accessing comms
3) Warm probe, device was in bootloader mode already

This code attempts to recover from 1(b) and 3.

There is an additional complexity: we have to try two possible bootloader
addresses because the mapping is not one-to-one and we don't know the exact
model yet.

Signed-off-by: Nick Dyer <nick.dyer@itdev.co.uk>
Acked-by: Yufeng Shen <miletus@chromium.org>
---
 drivers/input/touchscreen/atmel_mxt_ts.c | 67 ++++++++++++++++++++++++--------
 1 file changed, 50 insertions(+), 17 deletions(-)

diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index 5fe3285..8bcf21c 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -383,7 +383,7 @@ static int mxt_bootloader_write(struct mxt_data *data,
 	return ret;
 }
 
-static int mxt_lookup_bootloader_address(struct mxt_data *data)
+static int mxt_lookup_bootloader_address(struct mxt_data *data, bool retry)
 {
 	u8 appmode = data->client->addr;
 	u8 bootloader;
@@ -392,7 +392,7 @@ static int mxt_lookup_bootloader_address(struct mxt_data *data)
 	case 0x4a:
 	case 0x4b:
 		/* Chips after 1664S use different scheme */
-		if (data->info.family_id >= 0xa2) {
+		if (retry || data->info.family_id >= 0xa2) {
 			bootloader = appmode - 0x24;
 			break;
 		}
@@ -414,14 +414,14 @@ static int mxt_lookup_bootloader_address(struct mxt_data *data)
 	return 0;
 }
 
-static int mxt_probe_bootloader(struct mxt_data *data)
+static int mxt_probe_bootloader(struct mxt_data *data, bool retry)
 {
 	struct device *dev = &data->client->dev;
 	int ret;
 	u8 val;
 	bool crc_failure;
 
-	ret = mxt_lookup_bootloader_address(data);
+	ret = mxt_lookup_bootloader_address(data, retry);
 	if (ret)
 		return ret;
 
@@ -522,13 +522,18 @@ recheck:
 	return 0;
 }
 
-static int mxt_unlock_bootloader(struct mxt_data *data)
+static int mxt_send_bootloader_cmd(struct mxt_data *data, bool unlock)
 {
 	int ret;
 	u8 buf[2];
 
-	buf[0] = MXT_UNLOCK_CMD_LSB;
-	buf[1] = MXT_UNLOCK_CMD_MSB;
+	if (unlock) {
+		buf[0] = MXT_UNLOCK_CMD_LSB;
+		buf[1] = MXT_UNLOCK_CMD_MSB;
+	} else {
+		buf[0] = 0x01;
+		buf[1] = 0x01;
+	}
 
 	ret = mxt_bootloader_write(data, buf, 2);
 	if (ret)
@@ -1525,15 +1530,40 @@ static int mxt_initialize(struct mxt_data *data)
 {
 	struct i2c_client *client = data->client;
 	int error;
+	bool alt_bootloader_addr = false;
+	bool retry = false;
 
+retry_info:
 	error = mxt_get_info(data);
 	if (error) {
-		error = mxt_probe_bootloader(data);
-		if (error)
-			return error;
+retry_bootloader:
+		error = mxt_probe_bootloader(data, alt_bootloader_addr);
+		if (error) {
+			if (alt_bootloader_addr) {
+				/* Chip is not in appmode or bootloader mode */
+				return error;
+			}
 
-		data->in_bootloader = true;
-		return 0;
+			dev_info(&client->dev, "Trying alternate bootloader address\n");
+			alt_bootloader_addr = true;
+			goto retry_bootloader;
+		} else {
+			if (retry) {
+				dev_err(&client->dev, "Could not recover from bootloader mode\n");
+				/*
+				 * We can reflash from this state, so do not
+				 * abort init
+				 */
+				data->in_bootloader = true;
+				return 0;
+			}
+
+			/* Attempt to exit bootloader into app mode */
+			mxt_send_bootloader_cmd(data, false);
+			msleep(MXT_FW_RESET_TIME);
+			retry = true;
+			goto retry_info;
+		}
 	}
 
 	/* Get object table information */
@@ -1714,10 +1744,6 @@ static int mxt_load_fw(struct device *dev, const char *fn)
 	if (ret)
 		goto release_firmware;
 
-	ret = mxt_lookup_bootloader_address(data);
-	if (ret)
-		goto release_firmware;
-
 	if (!data->in_bootloader) {
 		/* Change to the bootloader mode */
 		data->in_bootloader = true;
@@ -1728,6 +1754,13 @@ static int mxt_load_fw(struct device *dev, const char *fn)
 			goto release_firmware;
 
 		msleep(MXT_RESET_TIME);
+
+		/* Do not need to scan since we know family ID */
+		ret = mxt_lookup_bootloader_address(data, 0);
+		if (ret)
+			goto release_firmware;
+	} else {
+		enable_irq(data->irq);
 	}
 
 	mxt_free_object_table(data);
@@ -1743,7 +1776,7 @@ static int mxt_load_fw(struct device *dev, const char *fn)
 		dev_info(dev, "Unlocking bootloader\n");
 
 		/* Unlock bootloader */
-		ret = mxt_unlock_bootloader(data);
+		ret = mxt_send_bootloader_cmd(data, true);
 		if (ret)
 			goto disable_irq;
 	}
-- 
2.0.1

[PATCH 12/15] Input: atmel_mxt_ts - add support for dynamic message size

[nick.dyer]

From: Nick Dyer <nick.dyer@itdev.co.uk>

The T5 object may have various sizes depending on the objects used on the
particular maXTouch chip and firmware version, therefore it can't be hardcoded
in the driver. Allocate a buffer on probe instead.

Signed-off-by: Nick Dyer <nick.dyer@itdev.co.uk>
Acked-by: Benson Leung <bleung@chromium.org>
Acked-by: Yufeng Shen <miletus@chromium.org>
---
 drivers/input/touchscreen/atmel_mxt_ts.c | 120 +++++++++++++++++--------------
 1 file changed, 68 insertions(+), 52 deletions(-)

diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index 8bcf21c..e4520c9 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -80,6 +80,9 @@
 #define MXT_SPT_MESSAGECOUNT_T44	44
 #define MXT_SPT_CTECONFIG_T46		46
 
+/* MXT_GEN_MESSAGE_T5 object */
+#define MXT_RPTID_NOMSG		0xff
+
 /* MXT_GEN_COMMAND_T6 field */
 #define MXT_COMMAND_RESET	0
 #define MXT_COMMAND_BACKUPNV	1
@@ -226,11 +229,6 @@ struct mxt_object {
 	u8 num_report_ids;
 } __packed;
 
-struct mxt_message {
-	u8 reportid;
-	u8 message[7];
-};
-
 /* Each client has this additional data */
 struct mxt_data {
 	struct i2c_client *client;
@@ -248,8 +246,10 @@ struct mxt_data {
 	u32 info_crc;
 	u8 bootloader_addr;
 	struct t7_config t7_cfg;
+	u8 *msg_buf;
 
 	/* Cached parameters from object table */
+	u8 T5_msg_size;
 	u8 T6_reportid;
 	u16 T6_address;
 	u16 T7_address;
@@ -310,11 +310,10 @@ static bool mxt_object_readable(unsigned int type)
 	}
 }
 
-static void mxt_dump_message(struct device *dev,
-			     struct mxt_message *message)
+static void mxt_dump_message(struct mxt_data *data, u8 *message)
 {
-	dev_dbg(dev, "reportid: %u\tmessage: %*ph\n",
-		message->reportid, 7, message->message);
+	dev_dbg(&data->client->dev, "message: %*ph\n",
+		data->T5_msg_size, message);
 }
 
 static int mxt_wait_for_completion(struct mxt_data *data,
@@ -628,8 +627,7 @@ mxt_get_object(struct mxt_data *data, u8 type)
 	return NULL;
 }
 
-static int mxt_read_message(struct mxt_data *data,
-				 struct mxt_message *message)
+static int mxt_read_message(struct mxt_data *data, u8 *message)
 {
 	struct mxt_object *object;
 	u16 reg;
@@ -640,10 +638,10 @@ static int mxt_read_message(struct mxt_data *data,
 
 	reg = object->start_address;
 	return __mxt_read_reg(data->client, reg,
-			sizeof(struct mxt_message), message);
+			data->T5_msg_size, message);
 }
 
-static void mxt_input_button(struct mxt_data *data, struct mxt_message *message)
+static void mxt_input_button(struct mxt_data *data, u8 *message)
 {
 	struct input_dev *input = data->input_dev;
 	const struct mxt_platform_data *pdata = data->pdata;
@@ -654,7 +652,7 @@ static void mxt_input_button(struct mxt_data *data, struct mxt_message *message)
 	for (i = 0; i < pdata->t19_num_keys; i++) {
 		if (pdata->t19_keymap[i] == KEY_RESERVED)
 			continue;
-		button = !(message->message[0] & (1 << i));
+		button = !(message[1] & (1 << i));
 		input_report_key(input, pdata->t19_keymap[i], button);
 	}
 }
@@ -666,19 +664,21 @@ static void mxt_input_sync(struct mxt_data *data)
 	input_sync(data->input_dev);
 }
 
-static void mxt_input_touchevent(struct mxt_data *data,
-				      struct mxt_message *message, int id)
+static void mxt_input_touchevent(struct mxt_data *data, u8 *message)
 {
 	struct device *dev = &data->client->dev;
-	u8 status = message->message[0];
 	struct input_dev *input_dev = data->input_dev;
+	int id;
+	u8 status;
 	int x;
 	int y;
 	int area;
 	int amplitude;
 
-	x = (message->message[1] << 4) | ((message->message[3] >> 4) & 0xf);
-	y = (message->message[2] << 4) | ((message->message[3] & 0xf));
+	id = message[0] - data->T9_reportid_min;
+	status = message[1];
+	x = (message[2] << 4) | ((message[4] >> 4) & 0xf);
+	y = (message[3] << 4) | ((message[4] & 0xf));
 
 	/* Handle 10/12 bit switching */
 	if (data->max_x < 1024)
@@ -686,8 +686,8 @@ static void mxt_input_touchevent(struct mxt_data *data,
 	if (data->max_y < 1024)
 		y >>= 2;
 
-	area = message->message[4];
-	amplitude = message->message[5];
+	area = message[5];
+	amplitude = message[6];
 
 	dev_dbg(dev,
 		"[%u] %c%c%c%c%c%c%c%c x: %5u y: %5u area: %3u amp: %3u\n",
@@ -733,28 +733,28 @@ static u16 mxt_extract_T6_csum(const u8 *csum)
 	return csum[0] | (csum[1] << 8) | (csum[2] << 16);
 }
 
-static bool mxt_is_T9_message(struct mxt_data *data, struct mxt_message *msg)
+static bool mxt_is_T9_message(struct mxt_data *data, u8 *msg)
 {
-	u8 id = msg->reportid;
+	u8 id = msg[0];
 	return (id >= data->T9_reportid_min && id <= data->T9_reportid_max);
 }
 
 static irqreturn_t mxt_process_messages_until_invalid(struct mxt_data *data)
 {
-	struct mxt_message message;
-	const u8 *payload = &message.message[0];
+	u8 *message = &data->msg_buf[0];
+	const u8 *payload = &data->msg_buf[1];
 	struct device *dev = &data->client->dev;
 	u8 reportid;
 	bool update_input = false;
 	u32 crc;
 
 	do {
-		if (mxt_read_message(data, &message)) {
+		if (mxt_read_message(data, message)) {
 			dev_err(dev, "Failed to read message\n");
 			return IRQ_NONE;
 		}
 
-		reportid = message.reportid;
+		reportid = message[0];
 
 		if (reportid == data->T6_reportid) {
 			u8 status = payload[0];
@@ -775,18 +775,17 @@ static irqreturn_t mxt_process_messages_until_invalid(struct mxt_data *data)
 			 * do not report events if input device
 			 * is not yet registered
 			 */
-			mxt_dump_message(dev, &message);
-		} else if (mxt_is_T9_message(data, &message)) {
-			int id = reportid - data->T9_reportid_min;
-			mxt_input_touchevent(data, &message, id);
+			mxt_dump_message(data, message);
+		} else if (mxt_is_T9_message(data, message)) {
+			mxt_input_touchevent(data, message);
 			update_input = true;
-		} else if (message.reportid == data->T19_reportid) {
-			mxt_input_button(data, &message);
+		} else if (reportid == data->T19_reportid) {
+			mxt_input_button(data, message);
 			update_input = true;
 		} else {
-			mxt_dump_message(dev, &message);
+			mxt_dump_message(data, message);
 		}
-	} while (reportid != 0xff);
+	} while (reportid != MXT_RPTID_NOMSG);
 
 	if (update_input)
 		mxt_input_sync(data);
@@ -1237,16 +1236,15 @@ recheck:
 static int mxt_make_highchg(struct mxt_data *data)
 {
 	struct device *dev = &data->client->dev;
-	struct mxt_message message;
 	int count = 10;
 	int error;
 
 	/* Read dummy message to make high CHG pin */
 	do {
-		error = mxt_read_message(data, &message);
+		error = mxt_read_message(data, data->msg_buf);
 		if (error)
 			return error;
-	} while (message.reportid != 0xff && --count);
+	} while (data->msg_buf[0] != MXT_RPTID_NOMSG && --count);
 
 	if (!count) {
 		dev_err(dev, "CHG pin isn't cleared\n");
@@ -1283,6 +1281,23 @@ static int mxt_get_info(struct mxt_data *data)
 	return 0;
 }
 
+static void mxt_free_object_table(struct mxt_data *data)
+{
+	input_unregister_device(data->input_dev);
+	data->input_dev = NULL;
+
+	kfree(data->object_table);
+	data->object_table = NULL;
+	kfree(data->msg_buf);
+	data->msg_buf = NULL;
+	data->T5_msg_size = 0;
+	data->T6_reportid = 0;
+	data->T7_address = 0;
+	data->T9_reportid_min = 0;
+	data->T9_reportid_max = 0;
+	data->T19_reportid = 0;
+}
+
 static int mxt_get_object_table(struct mxt_data *data)
 {
 	struct i2c_client *client = data->client;
@@ -1333,6 +1348,9 @@ static int mxt_get_object_table(struct mxt_data *data)
 			min_id, max_id);
 
 		switch (object->type) {
+		case MXT_GEN_MESSAGE_T5:
+			/* CRC not enabled, therefore don't read last byte */
+			data->T5_msg_size = mxt_obj_size(object) - 1;
 		case MXT_GEN_COMMAND_T6:
 			data->T6_reportid = min_id;
 			data->T6_address = object->start_address;
@@ -1356,22 +1374,20 @@ static int mxt_get_object_table(struct mxt_data *data)
 			data->mem_size = end_address + 1;
 	}
 
+	data->msg_buf = kzalloc(data->T5_msg_size, GFP_KERNEL);
+	if (!data->msg_buf) {
+		dev_err(&client->dev, "Failed to allocate message buffer\n");
+		error = -ENOMEM;
+		goto free_object_table;
+	}
+
 	data->object_table = object_table;
 
 	return 0;
-}
 
-static void mxt_free_object_table(struct mxt_data *data)
-{
-	input_unregister_device(data->input_dev);
-	data->input_dev = NULL;
-
-	kfree(data->object_table);
-	data->object_table = NULL;
-	data->T6_reportid = 0;
-	data->T9_reportid_min = 0;
-	data->T9_reportid_max = 0;
-	data->T19_reportid = 0;
+free_object_table:
+	mxt_free_object_table(data);
+	return error;
 }
 
 static int mxt_read_t9_resolution(struct mxt_data *data)
@@ -2002,7 +2018,7 @@ static int mxt_probe(struct i2c_client *client,
 	return 0;
 
 err_free_object:
-	kfree(data->object_table);
+	mxt_free_object_table(data);
 err_free_irq:
 	free_irq(client->irq, data);
 err_free_mem:
@@ -2017,7 +2033,7 @@ static int mxt_remove(struct i2c_client *client)
 	sysfs_remove_group(&client->dev.kobj, &mxt_attr_group);
 	free_irq(data->irq, data);
 	input_unregister_device(data->input_dev);
-	kfree(data->object_table);
+	mxt_free_object_table(data);
 	kfree(data);
 
 	return 0;
-- 
2.0.1

[PATCH 13/15] Input: atmel_mxt_ts - decode T6 status messages

[nick.dyer]

From: Nick Dyer <nick.dyer@itdev.co.uk>

By storing the previous T6 status byte multiple debug output of the same
status can be suppressed (for example CFGERR).

Signed-off-by: Nick Dyer <nick.dyer@itdev.co.uk>
Acked-by: Benson Leung <bleung@chromium.org>
Acked-by: Yufeng Shen <miletus@chromium.org>
---
 drivers/input/touchscreen/atmel_mxt_ts.c | 60 +++++++++++++++++++++-----------
 1 file changed, 40 insertions(+), 20 deletions(-)

diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index e4520c9..c409753 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -92,6 +92,11 @@
 
 /* Define for T6 status byte */
 #define MXT_T6_STATUS_RESET	(1 << 7)
+#define MXT_T6_STATUS_OFL	(1 << 6)
+#define MXT_T6_STATUS_SIGERR	(1 << 5)
+#define MXT_T6_STATUS_CAL	(1 << 4)
+#define MXT_T6_STATUS_CFGERR	(1 << 3)
+#define MXT_T6_STATUS_COMSERR	(1 << 2)
 
 /* MXT_GEN_POWER_T7 field */
 struct t7_config {
@@ -247,6 +252,7 @@ struct mxt_data {
 	u8 bootloader_addr;
 	struct t7_config t7_cfg;
 	u8 *msg_buf;
+	u8 t6_status;
 
 	/* Cached parameters from object table */
 	u8 T5_msg_size;
@@ -627,6 +633,39 @@ mxt_get_object(struct mxt_data *data, u8 type)
 	return NULL;
 }
 
+static void mxt_proc_t6_messages(struct mxt_data *data, u8 *msg)
+{
+	struct device *dev = &data->client->dev;
+	u8 status = msg[1];
+	u32 crc = msg[2] | (msg[3] << 8) | (msg[4] << 16);
+
+	complete(&data->crc_completion);
+
+	if (crc != data->config_crc) {
+		data->config_crc = crc;
+		dev_dbg(dev, "T6 Config Checksum: 0x%06X\n", crc);
+	}
+
+	/* Detect reset */
+	if (status & MXT_T6_STATUS_RESET)
+		complete(&data->reset_completion);
+
+	/* Output debug if status has changed */
+	if (status != data->t6_status)
+		dev_dbg(dev, "T6 Status 0x%02X%s%s%s%s%s%s%s\n",
+			status,
+			status == 0 ? " OK" : "",
+			status & MXT_T6_STATUS_RESET ? " RESET" : "",
+			status & MXT_T6_STATUS_OFL ? " OFL" : "",
+			status & MXT_T6_STATUS_SIGERR ? " SIGERR" : "",
+			status & MXT_T6_STATUS_CAL ? " CAL" : "",
+			status & MXT_T6_STATUS_CFGERR ? " CFGERR" : "",
+			status & MXT_T6_STATUS_COMSERR ? " COMSERR" : "");
+
+	/* Save current status */
+	data->t6_status = status;
+}
+
 static int mxt_read_message(struct mxt_data *data, u8 *message)
 {
 	struct mxt_object *object;
@@ -728,11 +767,6 @@ static void mxt_input_touchevent(struct mxt_data *data, u8 *message)
 	}
 }
 
-static u16 mxt_extract_T6_csum(const u8 *csum)
-{
-	return csum[0] | (csum[1] << 8) | (csum[2] << 16);
-}
-
 static bool mxt_is_T9_message(struct mxt_data *data, u8 *msg)
 {
 	u8 id = msg[0];
@@ -742,11 +776,9 @@ static bool mxt_is_T9_message(struct mxt_data *data, u8 *msg)
 static irqreturn_t mxt_process_messages_until_invalid(struct mxt_data *data)
 {
 	u8 *message = &data->msg_buf[0];
-	const u8 *payload = &data->msg_buf[1];
 	struct device *dev = &data->client->dev;
 	u8 reportid;
 	bool update_input = false;
-	u32 crc;
 
 	do {
 		if (mxt_read_message(data, message)) {
@@ -757,19 +789,7 @@ static irqreturn_t mxt_process_messages_until_invalid(struct mxt_data *data)
 		reportid = message[0];
 
 		if (reportid == data->T6_reportid) {
-			u8 status = payload[0];
-
-			crc = mxt_extract_T6_csum(&payload[1]);
-			if (crc != data->config_crc) {
-				data->config_crc = crc;
-				complete(&data->crc_completion);
-			}
-
-			dev_dbg(dev, "Status: %02x Config Checksum: %06x\n",
-				status, data->config_crc);
-
-			if (status & MXT_T6_STATUS_RESET)
-				complete(&data->reset_completion);
+			mxt_proc_t6_messages(data, message);
 		} else if (!data->input_dev) {
 			/*
 			 * do not report events if input device
-- 
2.0.1

[PATCH 14/15] Input: atmel_mxt_ts - split message handler into separate functions

[nick.dyer]

From: Nick Dyer <nick.dyer@itdev.co.uk>

This is in preparation for support of the T44 message count object.

Also, cache T5 address to avoid lookup on every interrupt cycle.

Signed-off-by: Nick Dyer <nick.dyer@itdev.co.uk>
Acked-by: Benson Leung <bleung@chromium.org>
Acked-by: Yufeng Shen <miletus@chromium.org>
---
 drivers/input/touchscreen/atmel_mxt_ts.c | 123 ++++++++++++++++---------------
 1 file changed, 64 insertions(+), 59 deletions(-)

diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index c409753..c3eee0f 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -253,8 +253,10 @@ struct mxt_data {
 	struct t7_config t7_cfg;
 	u8 *msg_buf;
 	u8 t6_status;
+	bool update_input;
 
 	/* Cached parameters from object table */
+	u16 T5_address;
 	u8 T5_msg_size;
 	u8 T6_reportid;
 	u16 T6_address;
@@ -666,20 +668,6 @@ static void mxt_proc_t6_messages(struct mxt_data *data, u8 *msg)
 	data->t6_status = status;
 }
 
-static int mxt_read_message(struct mxt_data *data, u8 *message)
-{
-	struct mxt_object *object;
-	u16 reg;
-
-	object = mxt_get_object(data, MXT_GEN_MESSAGE_T5);
-	if (!object)
-		return -EINVAL;
-
-	reg = object->start_address;
-	return __mxt_read_reg(data->client, reg,
-			data->T5_msg_size, message);
-}
-
 static void mxt_input_button(struct mxt_data *data, u8 *message)
 {
 	struct input_dev *input = data->input_dev;
@@ -703,7 +691,7 @@ static void mxt_input_sync(struct mxt_data *data)
 	input_sync(data->input_dev);
 }
 
-static void mxt_input_touchevent(struct mxt_data *data, u8 *message)
+static void mxt_proc_t9_message(struct mxt_data *data, u8 *message)
 {
 	struct device *dev = &data->client->dev;
 	struct input_dev *input_dev = data->input_dev;
@@ -765,50 +753,67 @@ static void mxt_input_touchevent(struct mxt_data *data, u8 *message)
 		/* Touch no longer active, close out slot */
 		input_mt_report_slot_state(input_dev, MT_TOOL_FINGER, 0);
 	}
+
+	data->update_input = true;
 }
 
-static bool mxt_is_T9_message(struct mxt_data *data, u8 *msg)
+static int mxt_proc_message(struct mxt_data *data, u8 *message)
 {
-	u8 id = msg[0];
-	return (id >= data->T9_reportid_min && id <= data->T9_reportid_max);
+	u8 report_id = message[0];
+
+	if (report_id == MXT_RPTID_NOMSG)
+		return 0;
+
+	if (report_id == data->T6_reportid) {
+		mxt_proc_t6_messages(data, message);
+	} else if (!data->input_dev) {
+		/*
+		 * do not report events if input device
+		 * is not yet registered
+		 */
+		mxt_dump_message(data, message);
+	} else if (report_id >= data->T9_reportid_min
+	    && report_id <= data->T9_reportid_max) {
+		mxt_proc_t9_message(data, message);
+	} else if (report_id == data->T19_reportid) {
+		mxt_input_button(data, message);
+		data->update_input = true;
+	} else {
+		mxt_dump_message(data, message);
+	}
+
+	return 1;
 }
 
-static irqreturn_t mxt_process_messages_until_invalid(struct mxt_data *data)
+static int mxt_read_and_process_message(struct mxt_data *data)
 {
-	u8 *message = &data->msg_buf[0];
 	struct device *dev = &data->client->dev;
-	u8 reportid;
-	bool update_input = false;
+	int ret;
 
-	do {
-		if (mxt_read_message(data, message)) {
-			dev_err(dev, "Failed to read message\n");
-			return IRQ_NONE;
-		}
+	ret = __mxt_read_reg(data->client, data->T5_address,
+				data->T5_msg_size, data->msg_buf);
+	if (ret) {
+		dev_err(dev, "Error %d reading message\n", ret);
+		return ret;
+	}
 
-		reportid = message[0];
+	return mxt_proc_message(data, data->msg_buf);
+}
 
-		if (reportid == data->T6_reportid) {
-			mxt_proc_t6_messages(data, message);
-		} else if (!data->input_dev) {
-			/*
-			 * do not report events if input device
-			 * is not yet registered
-			 */
-			mxt_dump_message(data, message);
-		} else if (mxt_is_T9_message(data, message)) {
-			mxt_input_touchevent(data, message);
-			update_input = true;
-		} else if (reportid == data->T19_reportid) {
-			mxt_input_button(data, message);
-			update_input = true;
-		} else {
-			mxt_dump_message(data, message);
-		}
-	} while (reportid != MXT_RPTID_NOMSG);
+static irqreturn_t mxt_process_messages_until_invalid(struct mxt_data *data)
+{
+	int ret;
+
+	do {
+		ret = mxt_read_and_process_message(data);
+		if (ret < 0)
+			return IRQ_NONE;
+	} while (ret > 0);
 
-	if (update_input)
+	if (data->update_input) {
 		mxt_input_sync(data);
+		data->update_input = false;
+	}
 
 	return IRQ_HANDLED;
 }
@@ -1257,21 +1262,19 @@ static int mxt_make_highchg(struct mxt_data *data)
 {
 	struct device *dev = &data->client->dev;
 	int count = 10;
-	int error;
+	int ret;
 
-	/* Read dummy message to make high CHG pin */
+	/* Read messages until we force an invalid */
 	do {
-		error = mxt_read_message(data, data->msg_buf);
-		if (error)
-			return error;
-	} while (data->msg_buf[0] != MXT_RPTID_NOMSG && --count);
-
-	if (!count) {
-		dev_err(dev, "CHG pin isn't cleared\n");
-		return -EBUSY;
-	}
+		ret = mxt_read_and_process_message(data);
+		if (ret == 0)
+			return 0;
+		else if (ret < 0)
+			return ret;
+	} while (--count);
 
-	return 0;
+	dev_err(dev, "CHG pin isn't cleared\n");
+	return -EBUSY;
 }
 
 static int mxt_acquire_irq(struct mxt_data *data)
@@ -1310,6 +1313,7 @@ static void mxt_free_object_table(struct mxt_data *data)
 	data->object_table = NULL;
 	kfree(data->msg_buf);
 	data->msg_buf = NULL;
+	data->T5_address = 0;
 	data->T5_msg_size = 0;
 	data->T6_reportid = 0;
 	data->T7_address = 0;
@@ -1371,6 +1375,7 @@ static int mxt_get_object_table(struct mxt_data *data)
 		case MXT_GEN_MESSAGE_T5:
 			/* CRC not enabled, therefore don't read last byte */
 			data->T5_msg_size = mxt_obj_size(object) - 1;
+			data->T5_address = object->start_address;
 		case MXT_GEN_COMMAND_T6:
 			data->T6_reportid = min_id;
 			data->T6_address = object->start_address;
-- 
2.0.1

[PATCH 15/15] Input: atmel_mxt_ts - implement T44 message handling

[nick.dyer]

From: Nick Dyer <nick.dyer@itdev.co.uk>

maXTouch chips allow the reading of multiple messages in a single I2C
transaction, which reduces bus overhead and improves performance/latency. The
number of messages available to be read is given by the value in the T44
object which is located directly before the T5 object.

Signed-off-by: Nick Dyer <nick.dyer@itdev.co.uk>
Acked-by: Benson Leung <bleung@chromium.org>
Acked-by: Yufeng Shen <miletus@chromium.org>
---
 drivers/input/touchscreen/atmel_mxt_ts.c | 191 +++++++++++++++++++++++++------
 1 file changed, 159 insertions(+), 32 deletions(-)

diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index c3eee0f..25cfbba 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -247,6 +247,7 @@ struct mxt_data {
 	unsigned int max_y;
 	bool in_bootloader;
 	u16 mem_size;
+	u8 max_reportid;
 	u32 config_crc;
 	u32 info_crc;
 	u8 bootloader_addr;
@@ -254,6 +255,8 @@ struct mxt_data {
 	u8 *msg_buf;
 	u8 t6_status;
 	bool update_input;
+	u8 last_message_count;
+	u8 num_touchids;
 
 	/* Cached parameters from object table */
 	u16 T5_address;
@@ -264,6 +267,7 @@ struct mxt_data {
 	u8 T9_reportid_min;
 	u8 T9_reportid_max;
 	u8 T19_reportid;
+	u16 T44_address;
 
 	/* for fw update in bootloader */
 	struct completion bl_completion;
@@ -785,30 +789,142 @@ static int mxt_proc_message(struct mxt_data *data, u8 *message)
 	return 1;
 }
 
-static int mxt_read_and_process_message(struct mxt_data *data)
+static int mxt_read_and_process_messages(struct mxt_data *data, u8 count)
 {
 	struct device *dev = &data->client->dev;
 	int ret;
+	int i;
+	u8 num_valid = 0;
+
+	/* Safety check for msg_buf */
+	if (count > data->max_reportid)
+		return -EINVAL;
 
+	/* Process remaining messages if necessary */
 	ret = __mxt_read_reg(data->client, data->T5_address,
-				data->T5_msg_size, data->msg_buf);
+				data->T5_msg_size * count, data->msg_buf);
 	if (ret) {
-		dev_err(dev, "Error %d reading message\n", ret);
+		dev_err(dev, "Failed to read %u messages (%d)\n", count, ret);
 		return ret;
 	}
 
-	return mxt_proc_message(data, data->msg_buf);
+	for (i = 0;  i < count; i++) {
+		ret = mxt_proc_message(data,
+			data->msg_buf + data->T5_msg_size * i);
+
+		if (ret == 1)
+			num_valid++;
+	}
+
+	/* return number of messages read */
+	return num_valid;
 }
 
-static irqreturn_t mxt_process_messages_until_invalid(struct mxt_data *data)
+static irqreturn_t mxt_process_messages_t44(struct mxt_data *data)
 {
+	struct device *dev = &data->client->dev;
 	int ret;
+	u8 count, num_left;
 
-	do {
-		ret = mxt_read_and_process_message(data);
+	/* Read T44 and T5 together */
+	ret = __mxt_read_reg(data->client, data->T44_address,
+		data->T5_msg_size + 1, data->msg_buf);
+	if (ret) {
+		dev_err(dev, "Failed to read T44 and T5 (%d)\n", ret);
+		return IRQ_NONE;
+	}
+
+	count = data->msg_buf[0];
+
+	if (count == 0) {
+		dev_warn(dev, "Interrupt triggered but zero messages\n");
+		return IRQ_NONE;
+	} else if (count > data->max_reportid) {
+		dev_err(dev, "T44 count %d exceeded max report id\n", count);
+		count = data->max_reportid;
+	}
+
+	/* Process first message */
+	ret = mxt_proc_message(data, data->msg_buf + 1);
+	if (ret < 0) {
+		dev_warn(dev, "Unexpected invalid message\n");
+		return IRQ_NONE;
+	}
+
+	num_left = count - 1;
+
+	/* Process remaining messages if necessary */
+	if (num_left) {
+		ret = mxt_read_and_process_messages(data, num_left);
 		if (ret < 0)
+			goto end;
+		else if (ret != num_left)
+			dev_warn(dev, "Unexpected invalid message\n");
+	}
+
+end:
+	if (data->update_input) {
+		mxt_input_sync(data);
+		data->update_input = false;
+	}
+
+	return IRQ_HANDLED;
+}
+
+static int mxt_process_messages_until_invalid(struct mxt_data *data)
+{
+	struct device *dev = &data->client->dev;
+	int count, read;
+	u8 tries = 2;
+
+	count = data->max_reportid;
+
+	/* Read messages until we force an invalid */
+	do {
+		read = mxt_read_and_process_messages(data, count);
+		if (read < count)
+			return 0;
+	} while (--tries);
+
+	if (data->update_input) {
+		mxt_input_sync(data);
+		data->update_input = false;
+	}
+
+	dev_err(dev, "CHG pin isn't cleared\n");
+	return -EBUSY;
+}
+
+static irqreturn_t mxt_process_messages(struct mxt_data *data)
+{
+	int total_handled, num_handled;
+	u8 count = data->last_message_count;
+
+	if (count < 1 || count > data->max_reportid)
+		count = 1;
+
+	/* include final invalid message */
+	total_handled = mxt_read_and_process_messages(data, count + 1);
+	if (total_handled < 0)
+		return IRQ_NONE;
+	/* if there were invalid messages, then we are done */
+	else if (total_handled <= count)
+		goto update_count;
+
+	/* keep reading two msgs until one is invalid or reportid limit */
+	do {
+		num_handled = mxt_read_and_process_messages(data, 2);
+		if (num_handled < 0)
 			return IRQ_NONE;
-	} while (ret > 0);
+
+		total_handled += num_handled;
+
+		if (num_handled < 2)
+			break;
+	} while (total_handled < data->num_touchids);
+
+update_count:
+	data->last_message_count = total_handled;
 
 	if (data->update_input) {
 		mxt_input_sync(data);
@@ -831,7 +947,11 @@ static irqreturn_t mxt_interrupt(int irq, void *dev_id)
 	if (!data->object_table)
 		return IRQ_HANDLED;
 
-	return mxt_process_messages_until_invalid(data);
+	if (data->T44_address) {
+		return mxt_process_messages_t44(data);
+	} else {
+		return mxt_process_messages(data);
+	}
 }
 
 static int mxt_t6_command(struct mxt_data *data, u16 cmd_offset,
@@ -1258,32 +1378,13 @@ recheck:
 	return 0;
 }
 
-static int mxt_make_highchg(struct mxt_data *data)
-{
-	struct device *dev = &data->client->dev;
-	int count = 10;
-	int ret;
-
-	/* Read messages until we force an invalid */
-	do {
-		ret = mxt_read_and_process_message(data);
-		if (ret == 0)
-			return 0;
-		else if (ret < 0)
-			return ret;
-	} while (--count);
-
-	dev_err(dev, "CHG pin isn't cleared\n");
-	return -EBUSY;
-}
-
 static int mxt_acquire_irq(struct mxt_data *data)
 {
 	int error;
 
 	enable_irq(data->irq);
 
-	error = mxt_make_highchg(data);
+	error = mxt_process_messages_until_invalid(data);
 	if (error)
 		return error;
 
@@ -1320,6 +1421,8 @@ static void mxt_free_object_table(struct mxt_data *data)
 	data->T9_reportid_min = 0;
 	data->T9_reportid_max = 0;
 	data->T19_reportid = 0;
+	data->T44_address = 0;
+	data->max_reportid = 0;
 }
 
 static int mxt_get_object_table(struct mxt_data *data)
@@ -1373,8 +1476,16 @@ static int mxt_get_object_table(struct mxt_data *data)
 
 		switch (object->type) {
 		case MXT_GEN_MESSAGE_T5:
-			/* CRC not enabled, therefore don't read last byte */
-			data->T5_msg_size = mxt_obj_size(object) - 1;
+			if (data->info.family_id == 0x80) {
+				/*
+				 * On mXT224 read and discard unused CRC byte
+				 * otherwise DMA reads are misaligned
+				 */
+				data->T5_msg_size = mxt_obj_size(object);
+			} else {
+				/* CRC not enabled, so skip last byte */
+				data->T5_msg_size = mxt_obj_size(object) - 1;
+			}
 			data->T5_address = object->start_address;
 		case MXT_GEN_COMMAND_T6:
 			data->T6_reportid = min_id;
@@ -1386,6 +1497,11 @@ static int mxt_get_object_table(struct mxt_data *data)
 		case MXT_TOUCH_MULTI_T9:
 			data->T9_reportid_min = min_id;
 			data->T9_reportid_max = max_id;
+			data->num_touchids = object->num_report_ids
+						* mxt_obj_instances(object);
+			break;
+		case MXT_SPT_MESSAGECOUNT_T44:
+			data->T44_address = object->start_address;
 			break;
 		case MXT_SPT_GPIOPWM_T19:
 			data->T19_reportid = min_id;
@@ -1399,7 +1515,18 @@ static int mxt_get_object_table(struct mxt_data *data)
 			data->mem_size = end_address + 1;
 	}
 
-	data->msg_buf = kzalloc(data->T5_msg_size, GFP_KERNEL);
+	/* Store maximum reportid */
+	data->max_reportid = reportid;
+
+	/* If T44 exists, T5 position has to be directly after */
+	if (data->T44_address && (data->T5_address != data->T44_address + 1)) {
+		dev_err(&client->dev, "Invalid T44 position\n");
+		error = -EINVAL;
+		goto free_object_table;
+	}
+
+	data->msg_buf = kcalloc(data->max_reportid,
+				data->T5_msg_size, GFP_KERNEL);
 	if (!data->msg_buf) {
 		dev_err(&client->dev, "Failed to allocate message buffer\n");
 		error = -ENOMEM;
-- 
2.0.1

Re: [PATCH 00/15] atmel_mxt_ts - device tree, bootloader, etc

[Sekhar Nori]

Hi Nick,

On Thursday 03 July 2014 08:31 PM, nick.dyer@itdev.co.uk wrote:
> Hi Dimitry-
> 
> Here is another set of atmel_mxt_ts patches for upstream. There are some
> really useful new features, but I hope nothing too controversial.

I was unable to get the touchscreen working on my board after applying 
just these patches. It does work correctly with your for-next branch so 
I guess I need to wait for you to post the rest of your patches too.

Here are the relevant messages at boot. Full boot log is available here 
(in case you want to have a look): http://paste.ubuntu.com/7759703/

[    2.315717] atmel_mxt_ts 0-004a: Direct firmware load failed with error -2
[    2.322949] atmel_mxt_ts 0-004a: Falling back to user helper
[    5.934924] atmel_mxt_ts 0-004a: Wait for completion timed out.
[    5.941237] atmel_mxt_ts 0-004a: Warning: Info CRC error - device=0x000000 file=0x8EE45C
[    7.294769] atmel_mxt_ts 0-004a: Wait for completion timed out.
[    7.300976] atmel_mxt_ts 0-004a: Resetting chip
[   10.574729] atmel_mxt_ts 0-004a: Wait for completion timed out.
[   10.581010] atmel_mxt_ts 0-004a: Error -110 updating config
[   10.626788] atmel_mxt_ts 0-004a: Family: 128 Variant: 1 Firmware V1.6.AB Objects: 17

One key difference is that these patches try to load the config at 
probe where as with your -next branch that is avoided in the DT case. 
This is also missing the new update_cfg sysfs interface (which I guess 
you will post as follow-on patches).

The wait_for_completion() times out because the interrupt never 
arrives. Even later when testing using evtest, I do not see interrupts 
coming. There are only two interrupts that arrive during boot and it 
stays that way. There is something going on with the way interrupts are 
handled. I havent debugged further yet. This problem is not there with 
your for-next branch.

I used your mxt-app to dump configuration in both cases and its exactly
the same.

Thanks,
Sekhar

Re: [PATCH 00/15] atmel_mxt_ts - device tree, bootloader, etc

[Sekhar Nori]

Hi Nick,

On Thursday 03 July 2014 08:31 PM, nick.dyer@itdev.co.uk wrote:
> Hi Dimitry-
> 
> Here is another set of atmel_mxt_ts patches for upstream. There are some
> really useful new features, but I hope nothing too controversial.

I was unable to get the touchscreen working on my board after applying 
just these patches. It does work correctly with your for-next branch so 
I guess I need to wait for you to post the rest of your patches too.

Here are the relevant messages at boot. Full boot log is available here 
(in case you want to have a look): http://paste.ubuntu.com/7759703/

[    2.315717] atmel_mxt_ts 0-004a: Direct firmware load failed with error -2
[    2.322949] atmel_mxt_ts 0-004a: Falling back to user helper
[    5.934924] atmel_mxt_ts 0-004a: Wait for completion timed out.
[    5.941237] atmel_mxt_ts 0-004a: Warning: Info CRC error - device=0x000000 file=0x8EE45C
[    7.294769] atmel_mxt_ts 0-004a: Wait for completion timed out.
[    7.300976] atmel_mxt_ts 0-004a: Resetting chip
[   10.574729] atmel_mxt_ts 0-004a: Wait for completion timed out.
[   10.581010] atmel_mxt_ts 0-004a: Error -110 updating config
[   10.626788] atmel_mxt_ts 0-004a: Family: 128 Variant: 1 Firmware V1.6.AB Objects: 17

One key difference is that these patches try to load the config at 
probe where as with your -next branch that is avoided in the DT case. 
This is also missing the new update_cfg sysfs interface (which I guess 
you will post as follow-on patches).

The wait_for_completion() times out because the interrupt never 
arrives. Even later when testing using evtest, I do not see interrupts 
coming. There are only two interrupts that arrive during boot and it 
stays that way. There is something going on with the way interrupts are 
handled. I havent debugged further yet. This problem is not there with 
your for-next branch.

I used your mxt-app to dump configuration in both cases and its exactly
the same.

Thanks,
Sekhar

Re: [PATCH 00/15] atmel_mxt_ts - device tree, bootloader, etc

[Nick Dyer]

On 07/07/14 12:21, Sekhar Nori wrote:
> I was unable to get the touchscreen working on my board after applying 
> just these patches. It does work correctly with your for-next branch so 
> I guess I need to wait for you to post the rest of your patches too.
> 
> Here are the relevant messages at boot. Full boot log is available here 
> (in case you want to have a look): http://paste.ubuntu.com/7759703/
> 
> [    2.315717] atmel_mxt_ts 0-004a: Direct firmware load failed with error -2
> [    2.322949] atmel_mxt_ts 0-004a: Falling back to user helper
> [    5.934924] atmel_mxt_ts 0-004a: Wait for completion timed out.
> [    5.941237] atmel_mxt_ts 0-004a: Warning: Info CRC error - device=0x000000 file=0x8EE45C
> [    7.294769] atmel_mxt_ts 0-004a: Wait for completion timed out.
> [    7.300976] atmel_mxt_ts 0-004a: Resetting chip
> [   10.574729] atmel_mxt_ts 0-004a: Wait for completion timed out.
> [   10.581010] atmel_mxt_ts 0-004a: Error -110 updating config
> [   10.626788] atmel_mxt_ts 0-004a: Family: 128 Variant: 1 Firmware V1.6.AB Objects: 17
> 
> One key difference is that these patches try to load the config at 
> probe where as with your -next branch that is avoided in the DT case. 
> This is also missing the new update_cfg sysfs interface (which I guess 
> you will post as follow-on patches).
> 
> The wait_for_completion() times out because the interrupt never 
> arrives. Even later when testing using evtest, I do not see interrupts 
> coming. There are only two interrupts that arrive during boot and it 
> stays that way. There is something going on with the way interrupts are 
> handled. I havent debugged further yet. This problem is not there with 
> your for-next branch.

Thanks for trying this, apologies that it didn't work.

Looking at it, I've a feeling that this is the solution for the issue you see:
https://github.com/ndyer/linux/commit/4e8f8d56361b09a2a

Although, the fact that it says the device is reporting the Info CRC to be
0x000000 is rather odd, as well.

It would be useful if you could turn on all the debug in the driver and
re-test (#define DEBUG 1 at the top). However, if you don't have time, I
will try and reproduce on my test system.

cheers

Nick

Re: [PATCH 00/15] atmel_mxt_ts - device tree, bootloader, etc

[Sekhar Nori]

On Monday 07 July 2014 05:08 PM, Nick Dyer wrote:
> On 07/07/14 12:21, Sekhar Nori wrote:
>> I was unable to get the touchscreen working on my board after applying 
>> just these patches. It does work correctly with your for-next branch so 
>> I guess I need to wait for you to post the rest of your patches too.
>>
>> Here are the relevant messages at boot. Full boot log is available here 
>> (in case you want to have a look): http://paste.ubuntu.com/7759703/
>>
>> [    2.315717] atmel_mxt_ts 0-004a: Direct firmware load failed with error -2
>> [    2.322949] atmel_mxt_ts 0-004a: Falling back to user helper
>> [    5.934924] atmel_mxt_ts 0-004a: Wait for completion timed out.
>> [    5.941237] atmel_mxt_ts 0-004a: Warning: Info CRC error - device=0x000000 file=0x8EE45C
>> [    7.294769] atmel_mxt_ts 0-004a: Wait for completion timed out.
>> [    7.300976] atmel_mxt_ts 0-004a: Resetting chip
>> [   10.574729] atmel_mxt_ts 0-004a: Wait for completion timed out.
>> [   10.581010] atmel_mxt_ts 0-004a: Error -110 updating config
>> [   10.626788] atmel_mxt_ts 0-004a: Family: 128 Variant: 1 Firmware V1.6.AB Objects: 17
>>
>> One key difference is that these patches try to load the config at 
>> probe where as with your -next branch that is avoided in the DT case. 
>> This is also missing the new update_cfg sysfs interface (which I guess 
>> you will post as follow-on patches).
>>
>> The wait_for_completion() times out because the interrupt never 
>> arrives. Even later when testing using evtest, I do not see interrupts 
>> coming. There are only two interrupts that arrive during boot and it 
>> stays that way. There is something going on with the way interrupts are 
>> handled. I havent debugged further yet. This problem is not there with 
>> your for-next branch.
> 
> Thanks for trying this, apologies that it didn't work.
> 
> Looking at it, I've a feeling that this is the solution for the issue you see:
> https://github.com/ndyer/linux/commit/4e8f8d56361b09a2a

Okay, I rebased that patch onto this series, but it did not solve the 
problem.

> 
> Although, the fact that it says the device is reporting the Info CRC to be
> 0x000000 is rather odd, as well.

Thats because the device was not configured at factory (I was using the
board for the first time). Once I run the kernel from your for-next branch,
this messages disappear since I guess the configuration is successfully
flashed by that kernel.

> It would be useful if you could turn on all the debug in the driver and
> re-test (#define DEBUG 1 at the top). However, if you don't have time, I
> will try and reproduce on my test system.

Here are the messages I see:

[    2.257611] atmel_mxt_ts 0-004a: T5 Start:242 Size:9 Instances:1 Report IDs:0-0
[    2.265261] atmel_mxt_ts 0-004a: T6 Start:251 Size:6 Instances:1 Report IDs:1-1
[    2.272966] atmel_mxt_ts 0-004a: T38 Start:257 Size:8 Instances:1 Report IDs:0-0
[    2.280726] atmel_mxt_ts 0-004a: T7 Start:265 Size:3 Instances:1 Report IDs:0-0
[    2.288389] atmel_mxt_ts 0-004a: T8 Start:268 Size:8 Instances:1 Report IDs:0-0
[    2.296045] atmel_mxt_ts 0-004a: T9 Start:276 Size:31 Instances:1 Report IDs:2-11
[    2.303905] atmel_mxt_ts 0-004a: T15 Start:307 Size:11 Instances:1 Report IDs:12-12
[    2.311935] atmel_mxt_ts 0-004a: T18 Start:318 Size:2 Instances:1 Report IDs:0-0
[    2.319695] atmel_mxt_ts 0-004a: T19 Start:320 Size:16 Instances:1 Report IDs:13-13
[    2.327723] atmel_mxt_ts 0-004a: T20 Start:336 Size:12 Instances:1 Report IDs:14-14
[    2.335732] atmel_mxt_ts 0-004a: T22 Start:348 Size:17 Instances:1 Report IDs:15-15
[    2.343761] atmel_mxt_ts 0-004a: T23 Start:365 Size:15 Instances:1 Report IDs:16-16
[    2.351789] atmel_mxt_ts 0-004a: T24 Start:380 Size:19 Instances:1 Report IDs:17-20
[    2.359812] atmel_mxt_ts 0-004a: T25 Start:399 Size:14 Instances:1 Report IDs:21-21
[    2.367844] atmel_mxt_ts 0-004a: T27 Start:413 Size:7 Instances:1 Report IDs:22-22
[    2.375763] atmel_mxt_ts 0-004a: T28 Start:420 Size:6 Instances:1 Report IDs:23-23
[    2.383705] atmel_mxt_ts 0-004a: T37 Start:112 Size:130 Instances:1 Report IDs:0-0
[    2.401277] atmel_mxt_ts 0-004a: T6 Config Checksum: 0x6BC1AB
[    2.407301] atmel_mxt_ts 0-004a: T6 Status 0x90 RESET CAL
[    2.413026] atmel_mxt_ts 0-004a: T6 Status 0x00 OK
[    2.424854] atmel_mxt_ts 0-004a: Direct firmware load failed with error -2
[    2.462113] atmel_mxt_ts 0-004a: Falling back to user helper
[    5.698524] atmel_mxt_ts 0-004a: Initialized power cfg: ACTV 255, IDLE 32
[    5.712014] atmel_mxt_ts 0-004a: Touchscreen size X799Y479
[    5.742180] atmel_mxt_ts 0-004a: Family: 128 Variant: 1 Firmware V1.6.AB Objects: 17

Full testing log here: http://paste.ubuntu.com/7765352/

Thanks,
Sekhar

Re: [PATCH 00/15] atmel_mxt_ts - device tree, bootloader, etc

[Sekhar Nori]

On Monday 07 July 2014 05:08 PM, Nick Dyer wrote:
> On 07/07/14 12:21, Sekhar Nori wrote:
>> I was unable to get the touchscreen working on my board after applying 
>> just these patches. It does work correctly with your for-next branch so 
>> I guess I need to wait for you to post the rest of your patches too.
>>
>> Here are the relevant messages at boot. Full boot log is available here 
>> (in case you want to have a look): http://paste.ubuntu.com/7759703/
>>
>> [    2.315717] atmel_mxt_ts 0-004a: Direct firmware load failed with error -2
>> [    2.322949] atmel_mxt_ts 0-004a: Falling back to user helper
>> [    5.934924] atmel_mxt_ts 0-004a: Wait for completion timed out.
>> [    5.941237] atmel_mxt_ts 0-004a: Warning: Info CRC error - device=0x000000 file=0x8EE45C
>> [    7.294769] atmel_mxt_ts 0-004a: Wait for completion timed out.
>> [    7.300976] atmel_mxt_ts 0-004a: Resetting chip
>> [   10.574729] atmel_mxt_ts 0-004a: Wait for completion timed out.
>> [   10.581010] atmel_mxt_ts 0-004a: Error -110 updating config
>> [   10.626788] atmel_mxt_ts 0-004a: Family: 128 Variant: 1 Firmware V1.6.AB Objects: 17
>>
>> One key difference is that these patches try to load the config at 
>> probe where as with your -next branch that is avoided in the DT case. 
>> This is also missing the new update_cfg sysfs interface (which I guess 
>> you will post as follow-on patches).
>>
>> The wait_for_completion() times out because the interrupt never 
>> arrives. Even later when testing using evtest, I do not see interrupts 
>> coming. There are only two interrupts that arrive during boot and it 
>> stays that way. There is something going on with the way interrupts are 
>> handled. I havent debugged further yet. This problem is not there with 
>> your for-next branch.
> 
> Thanks for trying this, apologies that it didn't work.
> 
> Looking at it, I've a feeling that this is the solution for the issue you see:
> https://github.com/ndyer/linux/commit/4e8f8d56361b09a2a

Okay, I rebased that patch onto this series, but it did not solve the 
problem.

> 
> Although, the fact that it says the device is reporting the Info CRC to be
> 0x000000 is rather odd, as well.

Thats because the device was not configured at factory (I was using the
board for the first time). Once I run the kernel from your for-next branch,
this messages disappear since I guess the configuration is successfully
flashed by that kernel.

> It would be useful if you could turn on all the debug in the driver and
> re-test (#define DEBUG 1 at the top). However, if you don't have time, I
> will try and reproduce on my test system.

Here are the messages I see:

[    2.257611] atmel_mxt_ts 0-004a: T5 Start:242 Size:9 Instances:1 Report IDs:0-0
[    2.265261] atmel_mxt_ts 0-004a: T6 Start:251 Size:6 Instances:1 Report IDs:1-1
[    2.272966] atmel_mxt_ts 0-004a: T38 Start:257 Size:8 Instances:1 Report IDs:0-0
[    2.280726] atmel_mxt_ts 0-004a: T7 Start:265 Size:3 Instances:1 Report IDs:0-0
[    2.288389] atmel_mxt_ts 0-004a: T8 Start:268 Size:8 Instances:1 Report IDs:0-0
[    2.296045] atmel_mxt_ts 0-004a: T9 Start:276 Size:31 Instances:1 Report IDs:2-11
[    2.303905] atmel_mxt_ts 0-004a: T15 Start:307 Size:11 Instances:1 Report IDs:12-12
[    2.311935] atmel_mxt_ts 0-004a: T18 Start:318 Size:2 Instances:1 Report IDs:0-0
[    2.319695] atmel_mxt_ts 0-004a: T19 Start:320 Size:16 Instances:1 Report IDs:13-13
[    2.327723] atmel_mxt_ts 0-004a: T20 Start:336 Size:12 Instances:1 Report IDs:14-14
[    2.335732] atmel_mxt_ts 0-004a: T22 Start:348 Size:17 Instances:1 Report IDs:15-15
[    2.343761] atmel_mxt_ts 0-004a: T23 Start:365 Size:15 Instances:1 Report IDs:16-16
[    2.351789] atmel_mxt_ts 0-004a: T24 Start:380 Size:19 Instances:1 Report IDs:17-20
[    2.359812] atmel_mxt_ts 0-004a: T25 Start:399 Size:14 Instances:1 Report IDs:21-21
[    2.367844] atmel_mxt_ts 0-004a: T27 Start:413 Size:7 Instances:1 Report IDs:22-22
[    2.375763] atmel_mxt_ts 0-004a: T28 Start:420 Size:6 Instances:1 Report IDs:23-23
[    2.383705] atmel_mxt_ts 0-004a: T37 Start:112 Size:130 Instances:1 Report IDs:0-0
[    2.401277] atmel_mxt_ts 0-004a: T6 Config Checksum: 0x6BC1AB
[    2.407301] atmel_mxt_ts 0-004a: T6 Status 0x90 RESET CAL
[    2.413026] atmel_mxt_ts 0-004a: T6 Status 0x00 OK
[    2.424854] atmel_mxt_ts 0-004a: Direct firmware load failed with error -2
[    2.462113] atmel_mxt_ts 0-004a: Falling back to user helper
[    5.698524] atmel_mxt_ts 0-004a: Initialized power cfg: ACTV 255, IDLE 32
[    5.712014] atmel_mxt_ts 0-004a: Touchscreen size X799Y479
[    5.742180] atmel_mxt_ts 0-004a: Family: 128 Variant: 1 Firmware V1.6.AB Objects: 17

Full testing log here: http://paste.ubuntu.com/7765352/

Thanks,
Sekhar

Re: [PATCH 00/15] atmel_mxt_ts - device tree, bootloader, etc

[Stephen Warren]

On 07/03/2014 09:01 AM, nick.dyer@itdev.co.uk wrote:
> Hi Dimitry-
> 
> Here is another set of atmel_mxt_ts patches for upstream. There are some
> really useful new features, but I hope nothing too controversial.

Unfortunately, I still can't get these to work on my system.

Per your "Re: atmel_mxt_ts: defaulting irqflags to
IRQF_TRIGGER_FALLING", I set up the IRQ type in the Tegra DT file, and
then applied this series on top of next-20140721. The driver appears to
initialize OK, but neither X nor evtest see any events from the device.
The IRQ count in /proc/interrupts doesn't increase when I touch the
touchpad, but does when I press it hard enough to trigger the physical
button. A boot log with debug enabled follows. No additional kernel log
messages are generated by touches or clicks. Do you have any idea what I
should try?

> [    1.634289] atmel_mxt_ts 1-004b: T37 Start:142 Size:130 Instances:1 Report IDs:0-0
> [    1.641889] atmel_mxt_ts 1-004b: T44 Start:272 Size:1 Instances:1 Report IDs:0-0
> [    1.649316] atmel_mxt_ts 1-004b: T5 Start:273 Size:9 Instances:1 Report IDs:0-0
> [    1.656631] atmel_mxt_ts 1-004b: T6 Start:282 Size:6 Instances:1 Report IDs:1-1
> [    1.663960] atmel_mxt_ts 1-004b: T38 Start:288 Size:8 Instances:1 Report IDs:0-0
> [    1.671376] atmel_mxt_ts 1-004b: T7 Start:296 Size:4 Instances:1 Report IDs:0-0
> [    1.678703] atmel_mxt_ts 1-004b: T8 Start:300 Size:10 Instances:1 Report IDs:0-0
> [    1.686104] atmel_mxt_ts 1-004b: T9 Start:310 Size:36 Instances:1 Report IDs:2-11
> [    1.693605] atmel_mxt_ts 1-004b: T15 Start:346 Size:11 Instances:1 Report IDs:12-12
> [    1.701286] atmel_mxt_ts 1-004b: T18 Start:357 Size:2 Instances:1 Report IDs:0-0
> [    1.708701] atmel_mxt_ts 1-004b: T19 Start:359 Size:6 Instances:1 Report IDs:13-13
> [    1.716276] atmel_mxt_ts 1-004b: T23 Start:365 Size:15 Instances:1 Report IDs:14-14
> [    1.723950] atmel_mxt_ts 1-004b: T25 Start:380 Size:15 Instances:1 Report IDs:15-15
> [    1.731622] atmel_mxt_ts 1-004b: T40 Start:395 Size:5 Instances:1 Report IDs:0-0
> [    1.739037] atmel_mxt_ts 1-004b: T42 Start:400 Size:10 Instances:1 Report IDs:16-16
> [    1.746697] atmel_mxt_ts 1-004b: T46 Start:410 Size:9 Instances:1 Report IDs:17-17
> [    1.754282] atmel_mxt_ts 1-004b: T47 Start:419 Size:13 Instances:1 Report IDs:0-0
> [    1.761783] atmel_mxt_ts 1-004b: T55 Start:432 Size:6 Instances:1 Report IDs:0-0
> [    1.769195] atmel_mxt_ts 1-004b: T56 Start:438 Size:40 Instances:1 Report IDs:18-18
> [    1.776855] atmel_mxt_ts 1-004b: T57 Start:478 Size:3 Instances:1 Report IDs:19-19
> [    1.784441] atmel_mxt_ts 1-004b: T61 Start:481 Size:5 Instances:2 Report IDs:20-21
> [    1.792028] atmel_mxt_ts 1-004b: T62 Start:491 Size:54 Instances:1 Report IDs:22-22
> [    1.821133] atmel_mxt_ts 1-004b: T6 Config Checksum: 0xC4CC1B
> [    1.826899] atmel_mxt_ts 1-004b: T6 Status 0x80 RESET
> [    1.831983] atmel_mxt_ts 1-004b: message: 00 20 00 00 00 00 00 00
> [    1.831998] atmel_mxt_ts 1-004b: Interrupt triggered but zero messages
> [    1.844743] atmel_mxt_ts 1-004b: Direct firmware load for maxtouch.cfg failed with error -2
> [    1.854145] atmel_mxt_ts 1-004b: Initialized power cfg: ACTV 16, IDLE 32
> [    1.862567] atmel_mxt_ts 1-004b: Touchscreen size X2040Y1360
> [    1.863232] input: Atmel maXTouch Touchpad as /devices/soc0/7000c400.i2c/i2c-1/1-004b/input/input1
> [    1.888976] atmel_mxt_ts 1-004b: Family: 130 Variant: 1 Firmware V1.0.AA Objects: 22
...
> [    8.591036] atmel_mxt_ts 1-004b: Set T7 ACTV:16 IDLE:32
> [    8.629419] atmel_mxt_ts 1-004b: Set T7 ACTV:0 IDLE:0
> [    8.669210] atmel_mxt_ts 1-004b: Set T7 ACTV:16 IDLE:32

Re: [PATCH 00/15] atmel_mxt_ts - device tree, bootloader, etc

[Nick Dyer]

On 22/07/14 21:34, Stephen Warren wrote:
> Unfortunately, I still can't get these to work on my system.
> 
> Per your "Re: atmel_mxt_ts: defaulting irqflags to
> IRQF_TRIGGER_FALLING", I set up the IRQ type in the Tegra DT file, and
> then applied this series on top of next-20140721. The driver appears to
> initialize OK, but neither X nor evtest see any events from the device.
> The IRQ count in /proc/interrupts doesn't increase when I touch the
> touchpad, but does when I press it hard enough to trigger the physical
> button.

You're using the T19/GPIO support, then? In which case, there appears to be
something wrong on the touch controller rather than the driver itself.

> A boot log with debug enabled follows. No additional kernel log
> messages are generated by touches or clicks.

Perhaps I should add some debug to mxt_input_button() - currently it will
not debug the fact that a click is received, although I guess that you will
see it in getevent.

> Do you have any idea what I should try?

I am suspicious that it may be that the power sequencing isn't quite right,
which sometimes leads to parts of the chip not working properly (eg GPIO
buttons working, but no touch).

The patch "use deep sleep when stopped" removes the reset on every resume
(which would otherwise kill resume performance). But that reset tends to
paper over a device which hasn't been powered up properly in the first place.

Could you try issuing a manual reset and see if the touch starts working? I
would normally do this by compiling our obp-utils software from
https://github.com/atmel-maxtouch/obp-utils using ndk-build and doing
something like:

mxt-app -d i2c-dev:1-004b --reset

(you need CONFIG_I2C_DEBUG to make /dev/i2c-1 appear)

If this works, then we need to adjust the board file that powers it on, the
correct power on sequence is contained in this later patch:
https://github.com/ndyer/linux/commit/97e938fa5863

>> [    1.831998] atmel_mxt_ts 1-004b: Interrupt triggered but zero messages

FWIW, this warning generally means you should be using the CHG line mode 1
in T18 COMMSCONFIG. It's benign, though.

Re: [PATCH 00/15] atmel_mxt_ts - device tree, bootloader, etc

[Stephen Warren]

On 07/23/2014 09:30 AM, Nick Dyer wrote:
> On 22/07/14 21:34, Stephen Warren wrote:
>> Unfortunately, I still can't get these to work on my system.
>>
>> Per your "Re: atmel_mxt_ts: defaulting irqflags to
>> IRQF_TRIGGER_FALLING", I set up the IRQ type in the Tegra DT file, and
>> then applied this series on top of next-20140721. The driver appears to
>> initialize OK, but neither X nor evtest see any events from the device.
>> The IRQ count in /proc/interrupts doesn't increase when I touch the
>> touchpad, but does when I press it hard enough to trigger the physical
>> button.
> 
> You're using the T19/GPIO support, then? In which case, there appears to be
> something wrong on the touch controller rather than the driver itself.

I assume I'm using T19, since there's a physical click action on the
touchpad along with the normal touch detection.

>> A boot log with debug enabled follows. No additional kernel log
>> messages are generated by touches or clicks.
> 
> Perhaps I should add some debug to mxt_input_button() - currently it will
> not debug the fact that a click is received, although I guess that you will
> see it in getevent.
> 
>> Do you have any idea what I should try?
> 
> I am suspicious that it may be that the power sequencing isn't quite right,
> which sometimes leads to parts of the chip not working properly (eg GPIO
> buttons working, but no touch).
> 
> The patch "use deep sleep when stopped" removes the reset on every resume
> (which would otherwise kill resume performance). But that reset tends to
> paper over a device which hasn't been powered up properly in the first place.
> 
> Could you try issuing a manual reset and see if the touch starts working? I
> would normally do this by compiling our obp-utils software from
> https://github.com/atmel-maxtouch/obp-utils using ndk-build and doing
> something like:
> 
> mxt-app -d i2c-dev:1-004b --reset
> 
> (you need CONFIG_I2C_DEBUG to make /dev/i2c-1 appear)

That didn't make any difference.

I also tried the tool interactively. the "Display raw (M)essages" option
never displayed anything, and the couple of self-tests I tried just
timed out. "Read (I)nfo block" did display some values that seemed like
they might be correct rather than random data.

Interestingly though, I did bisect the series and found "Input:
atmel_mxt_ts - use deep sleep mode when stopped" causes the problem. If
I apply the whole series and revert that one patch, the touchpad works
for mouse movement, but interestingly not for taps or physical clicks.

...
>>> [    1.831998] atmel_mxt_ts 1-004b: Interrupt triggered but zero messages
> 
> FWIW, this warning generally means you should be using the CHG line mode 1
> in T18 COMMSCONFIG. It's benign, though.

I'm not sure how I would adjust this; all firmware/config/... was
flashed into the touchpad device itself before I received the system,
and I don't have any firmware files on my host system.

Re: [PATCH 00/15] atmel_mxt_ts - device tree, bootloader, etc

[Dmitry Torokhov]

On Wed, Jul 23, 2014 at 11:22:54AM -0600, Stephen Warren wrote:
> On 07/23/2014 09:30 AM, Nick Dyer wrote:
> > On 22/07/14 21:34, Stephen Warren wrote:
> >> Unfortunately, I still can't get these to work on my system.
> >>
> >> Per your "Re: atmel_mxt_ts: defaulting irqflags to
> >> IRQF_TRIGGER_FALLING", I set up the IRQ type in the Tegra DT file, and
> >> then applied this series on top of next-20140721. The driver appears to
> >> initialize OK, but neither X nor evtest see any events from the device.
> >> The IRQ count in /proc/interrupts doesn't increase when I touch the
> >> touchpad, but does when I press it hard enough to trigger the physical
> >> button.
> > 
> > You're using the T19/GPIO support, then? In which case, there appears to be
> > something wrong on the touch controller rather than the driver itself.
> 
> I assume I'm using T19, since there's a physical click action on the
> touchpad along with the normal touch detection.
> 
> >> A boot log with debug enabled follows. No additional kernel log
> >> messages are generated by touches or clicks.
> > 
> > Perhaps I should add some debug to mxt_input_button() - currently it will
> > not debug the fact that a click is received, although I guess that you will
> > see it in getevent.
> > 
> >> Do you have any idea what I should try?
> > 
> > I am suspicious that it may be that the power sequencing isn't quite right,
> > which sometimes leads to parts of the chip not working properly (eg GPIO
> > buttons working, but no touch).
> > 
> > The patch "use deep sleep when stopped" removes the reset on every resume
> > (which would otherwise kill resume performance). But that reset tends to
> > paper over a device which hasn't been powered up properly in the first place.
> > 
> > Could you try issuing a manual reset and see if the touch starts working? I
> > would normally do this by compiling our obp-utils software from
> > https://github.com/atmel-maxtouch/obp-utils using ndk-build and doing
> > something like:
> > 
> > mxt-app -d i2c-dev:1-004b --reset
> > 
> > (you need CONFIG_I2C_DEBUG to make /dev/i2c-1 appear)
> 
> That didn't make any difference.
> 
> I also tried the tool interactively. the "Display raw (M)essages" option
> never displayed anything, and the couple of self-tests I tried just
> timed out. "Read (I)nfo block" did display some values that seemed like
> they might be correct rather than random data.
> 
> Interestingly though, I did bisect the series and found "Input:
> atmel_mxt_ts - use deep sleep mode when stopped" causes the problem. If
> I apply the whole series and revert that one patch, the touchpad works
> for mouse movement, but interestingly not for taps or physical clicks.

I ended up applying everything but the "deep sleep" patch. I wonder if
you have any keys defined to indicate that it is a touchpad and activate
tap-to-click support in userspace.

Thanks.

-- 
Dmitry

Re: [PATCH 00/15] atmel_mxt_ts - device tree, bootloader, etc

[Stephen Warren]

On 07/23/2014 02:29 PM, Dmitry Torokhov wrote:
> On Wed, Jul 23, 2014 at 11:22:54AM -0600, Stephen Warren wrote:
>> On 07/23/2014 09:30 AM, Nick Dyer wrote:
>>> On 22/07/14 21:34, Stephen Warren wrote:
>>>> Unfortunately, I still can't get these to work on my system.
>>>>
>>>> Per your "Re: atmel_mxt_ts: defaulting irqflags to
>>>> IRQF_TRIGGER_FALLING", I set up the IRQ type in the Tegra DT file, and
>>>> then applied this series on top of next-20140721. The driver appears to
>>>> initialize OK, but neither X nor evtest see any events from the device.
>>>> The IRQ count in /proc/interrupts doesn't increase when I touch the
>>>> touchpad, but does when I press it hard enough to trigger the physical
>>>> button.
>>>
>>> You're using the T19/GPIO support, then? In which case, there appears to be
>>> something wrong on the touch controller rather than the driver itself.
>>
>> I assume I'm using T19, since there's a physical click action on the
>> touchpad along with the normal touch detection.
>>
>>>> A boot log with debug enabled follows. No additional kernel log
>>>> messages are generated by touches or clicks.
>>>
>>> Perhaps I should add some debug to mxt_input_button() - currently it will
>>> not debug the fact that a click is received, although I guess that you will
>>> see it in getevent.
>>>
>>>> Do you have any idea what I should try?
>>>
>>> I am suspicious that it may be that the power sequencing isn't quite right,
>>> which sometimes leads to parts of the chip not working properly (eg GPIO
>>> buttons working, but no touch).
>>>
>>> The patch "use deep sleep when stopped" removes the reset on every resume
>>> (which would otherwise kill resume performance). But that reset tends to
>>> paper over a device which hasn't been powered up properly in the first place.
>>>
>>> Could you try issuing a manual reset and see if the touch starts working? I
>>> would normally do this by compiling our obp-utils software from
>>> https://github.com/atmel-maxtouch/obp-utils using ndk-build and doing
>>> something like:
>>>
>>> mxt-app -d i2c-dev:1-004b --reset
>>>
>>> (you need CONFIG_I2C_DEBUG to make /dev/i2c-1 appear)
>>
>> That didn't make any difference.
>>
>> I also tried the tool interactively. the "Display raw (M)essages" option
>> never displayed anything, and the couple of self-tests I tried just
>> timed out. "Read (I)nfo block" did display some values that seemed like
>> they might be correct rather than random data.
>>
>> Interestingly though, I did bisect the series and found "Input:
>> atmel_mxt_ts - use deep sleep mode when stopped" causes the problem. If
>> I apply the whole series and revert that one patch, the touchpad works
>> for mouse movement, but interestingly not for taps or physical clicks.
> 
> I ended up applying everything but the "deep sleep" patch. I wonder if
> you have any keys defined to indicate that it is a touchpad and activate
> tap-to-click support in userspace.

Yes, I have one GPIO defined in the keymap, for the physical
push-to-click button:

>                 trackpad@4b {
>                         compatible = "atmel,maxtouch";
>                         reg = <0x4b>;
>                         interrupt-parent = <&gpio>;
>                         interrupts = <TEGRA_GPIO(W, 3) IRQ_TYPE_LEVEL_LOW>;
>                         linux,gpio-keymap = <0 0 0 BTN_LEFT>;
>                 };

(at some point long ago, this did work fine, just by adding my DT
binding/parsing patch on top of what was in linux-next).

Re: [PATCH 00/15] atmel_mxt_ts - device tree, bootloader, etc

[Nick Dyer]

On 23/07/14 18:22, Stephen Warren wrote:
> That didn't make any difference.
> 
> I also tried the tool interactively. the "Display raw (M)essages" option
> never displayed anything, and the couple of self-tests I tried just
> timed out. "Read (I)nfo block" did display some values that seemed like
> they might be correct rather than random data.
> 
> Interestingly though, I did bisect the series and found "Input:
> atmel_mxt_ts - use deep sleep mode when stopped" causes the problem. If
> I apply the whole series and revert that one patch, the touchpad works
> for mouse movement, but interestingly not for taps or physical clicks.

Could you dump out the config for me (when it's in the failed state)? This
can be done by running:

mxt-app [device] --save fail.xcfg

>> FWIW, this warning generally means you should be using the CHG line mode 1
>> in T18 COMMSCONFIG. It's benign, though.
> 
> I'm not sure how I would adjust this; all firmware/config/... was
> flashed into the touchpad device itself before I received the system,
> and I don't have any firmware files on my host system.

It can be done via mxt-app. I would probably keep a known good config (via
--save/--load) if you're going to start making changes, though. Something like:

Write correct COMMSCONFIG settings:
mxt-app [device] -W -T18 44

Backup config to NVRAM:
mxt-app [device] --backup

I have on my TODO list to make the driver adjust this automatically.

Re: [PATCH 00/15] atmel_mxt_ts - device tree, bootloader, etc

[Stephen Warren]

On 07/24/2014 07:47 AM, Nick Dyer wrote:
> On 23/07/14 18:22, Stephen Warren wrote:
>> That didn't make any difference.
>>
>> I also tried the tool interactively. the "Display raw (M)essages" option
>> never displayed anything, and the couple of self-tests I tried just
>> timed out. "Read (I)nfo block" did display some values that seemed like
>> they might be correct rather than random data.
>>
>> Interestingly though, I did bisect the series and found "Input:
>> atmel_mxt_ts - use deep sleep mode when stopped" causes the problem. If
>> I apply the whole series and revert that one patch, the touchpad works
>> for mouse movement, but interestingly not for taps or physical clicks.
>
> Could you dump out the config for me (when it's in the failed state)? This
> can be done by running:
>
> mxt-app [device] --save fail.xcfg

That command always experiences a timeout error, but still seems to dump 
something out. Is this timeout error an issue? I'm a little hesitant to 
modify the COMMSCONFIG settings until I know load/save are really working.

root@localhost:~/obp-utils# ./mxt-app -d i2c-dev:1-004b --save 
~/mxt-save-no-movement.xml
Version:1.16-65-g0a4c
Registered i2c-dev adapter:1 address:0x4b
Opening config file /root/mxt-save-no-movement.xml...
REPORTALL command issued
Timeout

I've uploaded 2 logs to:

http://avon.wwwdotorg.org/downloads/mxt-logs/
(note there's no directory indexing, so manually add the filenames below 
to the URL)

mxt-save-no-movement.xml

This is with the whole series applied. Neither mouse movement nor clicks 
works. I tried mxt-app --reset and it made no difference to the dump 
results.

mxt-save-move-ok-no-clicking.xml

This is with "Input: atmel_mxt_ts - use deep sleep mode when stopped" 
reverted; mouse movement works, but clicking doesn't.

Re: [PATCH 00/15] atmel_mxt_ts - device tree, bootloader, etc

[Nick Dyer]

On 24/07/14 22:19, Stephen Warren wrote:
>> mxt-app [device] --save fail.xcfg
> 
> That command always experiences a timeout error, but still seems to dump
> something out. Is this timeout error an issue? I'm a little hesitant to
> modify the COMMSCONFIG settings until I know load/save are really working.
>
> root@localhost:~/obp-utils# ./mxt-app -d i2c-dev:1-004b --save
> ~/mxt-save-no-movement.xml
> Version:1.16-65-g0a4c
> Registered i2c-dev adapter:1 address:0x4b
> Opening config file /root/mxt-save-no-movement.xml...
> REPORTALL command issued
> Timeout

The timeout is benign. In the mode that we're using it here, it is unable
to query the chip to query the configuration checksum, hence in the file it
says "CHECKSUM=0x000000". This won't affect --load/--save.

> I've uploaded 2 logs to:
> 
> http://avon.wwwdotorg.org/downloads/mxt-logs/
> (note there's no directory indexing, so manually add the filenames below to
> the URL)
> 
> mxt-save-no-movement.xml
> 
> This is with the whole series applied. Neither mouse movement nor clicks
> works. I tried mxt-app --reset and it made no difference to the dump results.
> 
> mxt-save-move-ok-no-clicking.xml
> 
> This is with "Input: atmel_mxt_ts - use deep sleep mode when stopped"
> reverted; mouse movement works, but clicking doesn't.

Great, this has identified the issue with mouse movement (touch).

The config programmed into the NVRAM on your touch controller has the first
byte of the T9 touchscreen object set to zero. This is the CTRL byte which
enables/disables the touch object and what it reports. It is relying on
this to enable the touchscreen on resume:

https://github.com/dtor/input/blob/9d8dc3e529/drivers/input/touchscreen/atmel_mxt_ts.c#L2005-L2006

My "use deep sleep mode when stopped" patch stops the driver writing to the
T9.CTRL byte, so whatever config you have in NVRAM for that byte will be
used (ie zero, disabled). Going forward, deep sleep is more generic.
Indeed, newer chips do not have T9 at all, or they might be using other
touch objects. The deep sleep mode is a lower power state to be in, and is
what Atmel recommends.

However, it does mean changing the maxtouch cfg - you can write the 0x83 to
the first byte of T9 and save it to NVRAM, by doing:

mxt-app [device] -W -T9 83
mxt-app [device] --backup

It should still work fine with the older driver, it will just be enabled
for an additional short time during bootup before the first call to
mxt_input_open().

About the clicking - what does getevent -lp show? It should show the
BTN_LEFT key. If that is working correctly, then the driver isn't parsing
the messages correctly, it would be useful if you could add a
mxt_dump_message() call to mxt_input_button() and capture some dmesg output
of pressing the button.

Thanks for your patience in debugging this.

Re: [PATCH 00/15] atmel_mxt_ts - device tree, bootloader, etc

[Stephen Warren]

On 07/25/2014 08:10 AM, Nick Dyer wrote:
> On 24/07/14 22:19, Stephen Warren wrote:
...
>> I've uploaded 2 logs to:
>>
>> http://avon.wwwdotorg.org/downloads/mxt-logs/
>> (note there's no directory indexing, so manually add the filenames below to
>> the URL)
>>
>> mxt-save-no-movement.xml
>>
>> This is with the whole series applied. Neither mouse movement nor clicks
>> works. I tried mxt-app --reset and it made no difference to the dump results.
>>
>> mxt-save-move-ok-no-clicking.xml
>>
>> This is with "Input: atmel_mxt_ts - use deep sleep mode when stopped"
>> reverted; mouse movement works, but clicking doesn't.
>
> Great, this has identified the issue with mouse movement (touch).
>
> The config programmed into the NVRAM on your touch controller has the first
> byte of the T9 touchscreen object set to zero. This is the CTRL byte which
> enables/disables the touch object and what it reports. It is relying on
> this to enable the touchscreen on resume:
>
> https://github.com/dtor/input/blob/9d8dc3e529/drivers/input/touchscreen/atmel_mxt_ts.c#L2005-L2006
>
> My "use deep sleep mode when stopped" patch stops the driver writing to the
> T9.CTRL byte, so whatever config you have in NVRAM for that byte will be
> used (ie zero, disabled). Going forward, deep sleep is more generic.
> Indeed, newer chips do not have T9 at all, or they might be using other
> touch objects. The deep sleep mode is a lower power state to be in, and is
> what Atmel recommends.
>
> However, it does mean changing the maxtouch cfg - you can write the 0x83 to
> the first byte of T9 and save it to NVRAM, by doing:
>
> mxt-app [device] -W -T9 83
> mxt-app [device] --backup

If I do that, then both mouse movement and "touch" clicks work:-)

(Dmitry, I guess that means it's fine to go ahead and apply "Input: 
atmel_mxt_ts - use deep sleep mode when stopped".)

I wonder why the configuration NVRAM in my device was incorrect though? 
I'm CCing a few ChromeOS people to try and find out any relevant history 
for the touchpad NVRAM settings on Venice2. Perhaps this is simply 
something that wasn't noticed because the driver used to initialize this 
configuration bit, so nobody realized the config in NVRAM was wrong.

...
> About the clicking - what does getevent -lp show? It should show the
> BTN_LEFT key. If that is working correctly, then the driver isn't parsing
> the messages correctly, it would be useful if you could add a
> mxt_dump_message() call to mxt_input_button() and capture some dmesg output
> of pressing the button.

I'm not sure what getevent is, but I think I tracked down the issue anyway.

With the NVRAM config fix you mentioned, "touch" clicks work OK. 
However, as such as I do a "physical" click (push the touchpad down), 
all kinds of click stop working. I think the answer lies in evtest logs:

First "touch" click (with touch pressure/position events removed):

> Event: time 1406318070.891773, type 1 (EV_KEY), code 330 (BTN_TOUCH), value 1
> Event: time 1406318070.891773, type 1 (EV_KEY), code 325 (BTN_TOOL_FINGER), value 1
...
> Event: time 1406318070.941752, type 1 (EV_KEY), code 330 (BTN_TOUCH), value 0
> Event: time 1406318070.941752, type 1 (EV_KEY), code 325 (BTN_TOOL_FINGER), value 0

The second "touch" click generates the same events. Note how all the 
events are correctly mirrored between down/up events.

Now, the first "physical" click:

> Event: time 1406318085.303424, type 1 (EV_KEY), code 272 (BTN_LEFT), value 1
...
> Event: time 1406318085.319818, type 1 (EV_KEY), code 330 (BTN_TOUCH), value 1
> Event: time 1406318085.319818, type 1 (EV_KEY), code 325 (BTN_TOOL_FINGER), value 1
...
> Event: time 1406318085.515763, type 1 (EV_KEY), code 330 (BTN_TOUCH), value 0
> Event: time 1406318085.515763, type 1 (EV_KEY), code 325 (BTN_TOOL_FINGER), value 0

Note the extra BTN_LEFT down event, which has no corresponding "up" 
event. After this, subsequent "physical" clicks don't generate any more 
BTN_LEFT events (down or up) at all, and a USB mouse's BTN_LEFT events 
are ignored, I suppose since the system still thinks the touchpad's left 
button is being held down.

Is this a driver bug (not generating the correct BTN_LEFT events), or 
some other touchpad HW/NVRAM configuration problem?

Re: [PATCH 00/15] atmel_mxt_ts - device tree, bootloader, etc

[Stephen Warren]

On 07/25/2014 08:10 AM, Nick Dyer wrote:
> On 24/07/14 22:19, Stephen Warren wrote:
...
>> I've uploaded 2 logs to:
>>
>> http://avon.wwwdotorg.org/downloads/mxt-logs/
>> (note there's no directory indexing, so manually add the filenames below to
>> the URL)
>>
>> mxt-save-no-movement.xml
>>
>> This is with the whole series applied. Neither mouse movement nor clicks
>> works. I tried mxt-app --reset and it made no difference to the dump results.
>>
>> mxt-save-move-ok-no-clicking.xml
>>
>> This is with "Input: atmel_mxt_ts - use deep sleep mode when stopped"
>> reverted; mouse movement works, but clicking doesn't.
>
> Great, this has identified the issue with mouse movement (touch).
>
> The config programmed into the NVRAM on your touch controller has the first
> byte of the T9 touchscreen object set to zero. This is the CTRL byte which
> enables/disables the touch object and what it reports. It is relying on
> this to enable the touchscreen on resume:
>
> https://github.com/dtor/input/blob/9d8dc3e529/drivers/input/touchscreen/atmel_mxt_ts.c#L2005-L2006
>
> My "use deep sleep mode when stopped" patch stops the driver writing to the
> T9.CTRL byte, so whatever config you have in NVRAM for that byte will be
> used (ie zero, disabled). Going forward, deep sleep is more generic.
> Indeed, newer chips do not have T9 at all, or they might be using other
> touch objects. The deep sleep mode is a lower power state to be in, and is
> what Atmel recommends.
>
> However, it does mean changing the maxtouch cfg - you can write the 0x83 to
> the first byte of T9 and save it to NVRAM, by doing:
>
> mxt-app [device] -W -T9 83
> mxt-app [device] --backup

If I do that, then both mouse movement and "touch" clicks work:-)

(Dmitry, I guess that means it's fine to go ahead and apply "Input: 
atmel_mxt_ts - use deep sleep mode when stopped".)

I wonder why the configuration NVRAM in my device was incorrect though? 
I'm CCing a few ChromeOS people to try and find out any relevant history 
for the touchpad NVRAM settings on Venice2. Perhaps this is simply 
something that wasn't noticed because the driver used to initialize this 
configuration bit, so nobody realized the config in NVRAM was wrong.

...
> About the clicking - what does getevent -lp show? It should show the
> BTN_LEFT key. If that is working correctly, then the driver isn't parsing
> the messages correctly, it would be useful if you could add a
> mxt_dump_message() call to mxt_input_button() and capture some dmesg output
> of pressing the button.

I'm not sure what getevent is, but I think I tracked down the issue anyway.

With the NVRAM config fix you mentioned, "touch" clicks work OK. 
However, as such as I do a "physical" click (push the touchpad down), 
all kinds of click stop working. I think the answer lies in evtest logs:

First "touch" click (with touch pressure/position events removed):

> Event: time 1406318070.891773, type 1 (EV_KEY), code 330 (BTN_TOUCH), value 1
> Event: time 1406318070.891773, type 1 (EV_KEY), code 325 (BTN_TOOL_FINGER), value 1
...
> Event: time 1406318070.941752, type 1 (EV_KEY), code 330 (BTN_TOUCH), value 0
> Event: time 1406318070.941752, type 1 (EV_KEY), code 325 (BTN_TOOL_FINGER), value 0

The second "touch" click generates the same events. Note how all the 
events are correctly mirrored between down/up events.

Now, the first "physical" click:

> Event: time 1406318085.303424, type 1 (EV_KEY), code 272 (BTN_LEFT), value 1
...
> Event: time 1406318085.319818, type 1 (EV_KEY), code 330 (BTN_TOUCH), value 1
> Event: time 1406318085.319818, type 1 (EV_KEY), code 325 (BTN_TOOL_FINGER), value 1
...
> Event: time 1406318085.515763, type 1 (EV_KEY), code 330 (BTN_TOUCH), value 0
> Event: time 1406318085.515763, type 1 (EV_KEY), code 325 (BTN_TOOL_FINGER), value 0

Note the extra BTN_LEFT down event, which has no corresponding "up" 
event. After this, subsequent "physical" clicks don't generate any more 
BTN_LEFT events (down or up) at all, and a USB mouse's BTN_LEFT events 
are ignored, I suppose since the system still thinks the touchpad's left 
button is being held down.

Is this a driver bug (not generating the correct BTN_LEFT events), or 
some other touchpad HW/NVRAM configuration problem?

Re: [PATCH 00/15] atmel_mxt_ts - device tree, bootloader, etc

[Dmitry Torokhov]

On Fri, Jul 25, 2014 at 02:06:40PM -0600, Stephen Warren wrote:
> On 07/25/2014 08:10 AM, Nick Dyer wrote:
> >On 24/07/14 22:19, Stephen Warren wrote:
> ...
> >>I've uploaded 2 logs to:
> >>
> >>http://avon.wwwdotorg.org/downloads/mxt-logs/
> >>(note there's no directory indexing, so manually add the filenames below to
> >>the URL)
> >>
> >>mxt-save-no-movement.xml
> >>
> >>This is with the whole series applied. Neither mouse movement nor clicks
> >>works. I tried mxt-app --reset and it made no difference to the dump results.
> >>
> >>mxt-save-move-ok-no-clicking.xml
> >>
> >>This is with "Input: atmel_mxt_ts - use deep sleep mode when stopped"
> >>reverted; mouse movement works, but clicking doesn't.
> >
> >Great, this has identified the issue with mouse movement (touch).
> >
> >The config programmed into the NVRAM on your touch controller has the first
> >byte of the T9 touchscreen object set to zero. This is the CTRL byte which
> >enables/disables the touch object and what it reports. It is relying on
> >this to enable the touchscreen on resume:
> >
> >https://github.com/dtor/input/blob/9d8dc3e529/drivers/input/touchscreen/atmel_mxt_ts.c#L2005-L2006
> >
> >My "use deep sleep mode when stopped" patch stops the driver writing to the
> >T9.CTRL byte, so whatever config you have in NVRAM for that byte will be
> >used (ie zero, disabled). Going forward, deep sleep is more generic.
> >Indeed, newer chips do not have T9 at all, or they might be using other
> >touch objects. The deep sleep mode is a lower power state to be in, and is
> >what Atmel recommends.
> >
> >However, it does mean changing the maxtouch cfg - you can write the 0x83 to
> >the first byte of T9 and save it to NVRAM, by doing:
> >
> >mxt-app [device] -W -T9 83
> >mxt-app [device] --backup
> 
> If I do that, then both mouse movement and "touch" clicks work:-)
> 
> (Dmitry, I guess that means it's fine to go ahead and apply "Input:
> atmel_mxt_ts - use deep sleep mode when stopped".)

OK, I've applied it (but because it now last I had to resolve a few
conflicts so if somebody could take a peek at my next branch that would
be great).

Thanks.

-- 
Dmitry

Re: [PATCH 00/15] atmel_mxt_ts - device tree, bootloader, etc

[Yufeng Shen]

On Fri, Jul 25, 2014 at 4:06 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
>
> On 07/25/2014 08:10 AM, Nick Dyer wrote:
>>
>> On 24/07/14 22:19, Stephen Warren wrote:
>
> ...
>
>>> I've uploaded 2 logs to:
>>>
>>> http://avon.wwwdotorg.org/downloads/mxt-logs/
>>> (note there's no directory indexing, so manually add the filenames below to
>>> the URL)
>>>
>>> mxt-save-no-movement.xml
>>>
>>> This is with the whole series applied. Neither mouse movement nor clicks
>>> works. I tried mxt-app --reset and it made no difference to the dump results.
>>>
>>> mxt-save-move-ok-no-clicking.xml
>>>
>>> This is with "Input: atmel_mxt_ts - use deep sleep mode when stopped"
>>> reverted; mouse movement works, but clicking doesn't.
>>
>>
>> Great, this has identified the issue with mouse movement (touch).
>>
>> The config programmed into the NVRAM on your touch controller has the first
>> byte of the T9 touchscreen object set to zero. This is the CTRL byte which
>> enables/disables the touch object and what it reports. It is relying on
>> this to enable the touchscreen on resume:
>>
>> https://github.com/dtor/input/blob/9d8dc3e529/drivers/input/touchscreen/atmel_mxt_ts.c#L2005-L2006
>>
>> My "use deep sleep mode when stopped" patch stops the driver writing to the
>> T9.CTRL byte, so whatever config you have in NVRAM for that byte will be
>> used (ie zero, disabled). Going forward, deep sleep is more generic.
>> Indeed, newer chips do not have T9 at all, or they might be using other
>> touch objects. The deep sleep mode is a lower power state to be in, and is
>> what Atmel recommends.
>>
>> However, it does mean changing the maxtouch cfg - you can write the 0x83 to
>> the first byte of T9 and save it to NVRAM, by doing:
>>
>> mxt-app [device] -W -T9 83
>> mxt-app [device] --backup
>
>
> If I do that, then both mouse movement and "touch" clicks work:-)
>
> (Dmitry, I guess that means it's fine to go ahead and apply "Input: atmel_mxt_ts - use deep sleep mode when stopped".)
>
> I wonder why the configuration NVRAM in my device was incorrect though? I'm CCing a few ChromeOS people to try and find out any relevant history for the touchpad NVRAM settings on Venice2. Perhaps this is simply something that wasn't noticed because the driver used to initialize this configuration bit, so nobody realized the config in NVRAM was wrong.
>

Where did you get the configuration file ? It is possible that we rely
too much on mxt_start to turn on the T9.CTRL bit and have neglected
its setting in the config file.
If you can tell me where you get the config file I can do a check.


> ...
>
>> About the clicking - what does getevent -lp show? It should show the
>> BTN_LEFT key. If that is working correctly, then the driver isn't parsing
>> the messages correctly, it would be useful if you could add a
>> mxt_dump_message() call to mxt_input_button() and capture some dmesg output
>> of pressing the button.
>
>
> I'm not sure what getevent is, but I think I tracked down the issue anyway.
>
> With the NVRAM config fix you mentioned, "touch" clicks work OK. However, as such as I do a "physical" click (push the touchpad down), all kinds of click stop working. I think the answer lies in evtest logs:
>
> First "touch" click (with touch pressure/position events removed):
>
>> Event: time 1406318070.891773, type 1 (EV_KEY), code 330 (BTN_TOUCH), value 1
>> Event: time 1406318070.891773, type 1 (EV_KEY), code 325 (BTN_TOOL_FINGER), value 1
>
> ...
>>
>> Event: time 1406318070.941752, type 1 (EV_KEY), code 330 (BTN_TOUCH), value 0
>> Event: time 1406318070.941752, type 1 (EV_KEY), code 325 (BTN_TOOL_FINGER), value 0
>
>
> The second "touch" click generates the same events. Note how all the events are correctly mirrored between down/up events.
>
> Now, the first "physical" click:
>
>> Event: time 1406318085.303424, type 1 (EV_KEY), code 272 (BTN_LEFT), value 1
>
> ...
>>
>> Event: time 1406318085.319818, type 1 (EV_KEY), code 330 (BTN_TOUCH), value 1
>> Event: time 1406318085.319818, type 1 (EV_KEY), code 325 (BTN_TOOL_FINGER), value 1
>
> ...
>>
>> Event: time 1406318085.515763, type 1 (EV_KEY), code 330 (BTN_TOUCH), value 0
>> Event: time 1406318085.515763, type 1 (EV_KEY), code 325 (BTN_TOOL_FINGER), value 0
>
>
> Note the extra BTN_LEFT down event, which has no corresponding "up" event. After this, subsequent "physical" clicks don't generate any more BTN_LEFT events (down or up) at all, and a USB mouse's BTN_LEFT events are ignored, I suppose since the system still thinks the touchpad's left button is being held down.
>
> Is this a driver bug (not generating the correct BTN_LEFT events), or some other touchpad HW/NVRAM configuration problem?

Re: [PATCH 00/15] atmel_mxt_ts - device tree, bootloader, etc

[Stephen Warren]

On 07/28/2014 02:20 PM, Yufeng Shen wrote:
> On Fri, Jul 25, 2014 at 4:06 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
>>
>> On 07/25/2014 08:10 AM, Nick Dyer wrote:
>>>
>>> On 24/07/14 22:19, Stephen Warren wrote:
>>
>> ...
>>
>>>> I've uploaded 2 logs to:
>>>>
>>>> http://avon.wwwdotorg.org/downloads/mxt-logs/
>>>> (note there's no directory indexing, so manually add the filenames below to
>>>> the URL)
>>>>
>>>> mxt-save-no-movement.xml
>>>>
>>>> This is with the whole series applied. Neither mouse movement nor clicks
>>>> works. I tried mxt-app --reset and it made no difference to the dump results.
>>>>
>>>> mxt-save-move-ok-no-clicking.xml
>>>>
>>>> This is with "Input: atmel_mxt_ts - use deep sleep mode when stopped"
>>>> reverted; mouse movement works, but clicking doesn't.
>>>
>>>
>>> Great, this has identified the issue with mouse movement (touch).
>>>
>>> The config programmed into the NVRAM on your touch controller has the first
>>> byte of the T9 touchscreen object set to zero. This is the CTRL byte which
>>> enables/disables the touch object and what it reports. It is relying on
>>> this to enable the touchscreen on resume:
>>>
>>> https://github.com/dtor/input/blob/9d8dc3e529/drivers/input/touchscreen/atmel_mxt_ts.c#L2005-L2006
>>>
>>> My "use deep sleep mode when stopped" patch stops the driver writing to the
>>> T9.CTRL byte, so whatever config you have in NVRAM for that byte will be
>>> used (ie zero, disabled). Going forward, deep sleep is more generic.
>>> Indeed, newer chips do not have T9 at all, or they might be using other
>>> touch objects. The deep sleep mode is a lower power state to be in, and is
>>> what Atmel recommends.
>>>
>>> However, it does mean changing the maxtouch cfg - you can write the 0x83 to
>>> the first byte of T9 and save it to NVRAM, by doing:
>>>
>>> mxt-app [device] -W -T9 83
>>> mxt-app [device] --backup
>>
>>
>> If I do that, then both mouse movement and "touch" clicks work:-)
>>
>> (Dmitry, I guess that means it's fine to go ahead and apply "Input: atmel_mxt_ts - use deep sleep mode when stopped".)
>>
>> I wonder why the configuration NVRAM in my device was incorrect though? I'm CCing a few ChromeOS people to try and find out any relevant history for the touchpad NVRAM settings on Venice2. Perhaps this is simply something that wasn't noticed because the driver used to initialize this configuration bit, so nobody realized the config in NVRAM was wrong.
>
> Where did you get the configuration file ? It is possible that we rely
> too much on mxt_start to turn on the T9.CTRL bit and have neglected
> its setting in the config file.
> If you can tell me where you get the config file I can do a check.

It was already flashed into the touchpad when I received the board. I 
did try to track down the firmware/config files a few months ago, but 
didn't manage to; I was told since they were already flashed so I didn't 
need them. The board is Venice2.

Re: [PATCH 00/15] atmel_mxt_ts - device tree, bootloader, etc

[Stephen Warren]

On 07/28/2014 03:23 PM, Stephen Warren wrote:
> On 07/28/2014 02:20 PM, Yufeng Shen wrote:
...
>> Where did you get the configuration file ? It is possible that we rely
>> too much on mxt_start to turn on the T9.CTRL bit and have neglected
>> its setting in the config file.
>> If you can tell me where you get the config file I can do a check.
>
> It was already flashed into the touchpad when I received the board. I
> did try to track down the firmware/config files a few months ago, but
> didn't manage to; I was told since they were already flashed so I didn't
> need them. The board is Venice2.

OK, I received the configuration and firmware file that's supposed to be 
in the touchpad.

I can see that the config file I was given has the "83" byte in the T9 
configuration, and in fact /almost/ exactly matches the configuration I 
have. I don't know why my T9 configuration was wrong before, but I 
suspect it's not worth trying to track that down.

Anyway, here's the diff between the two config files:

> # diff -u mxt-save-after-t9-83-write.xml 224sl.raw
> --- mxt-save-after-t9-83-write.xml	2014-07-25 19:41:45.000000000 +0000
> +++ 224sl.raw	2014-07-28 23:25:49.000000000 +0000
> @@ -1,8 +1,7 @@
>  OBP_RAW V1
>  82 01 10 AA 12 0C 16
>  F5AF33
> -000000
> -0025 0000 0082 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> +E21E65
>  0026 0000 0008 00 00 00 00 00 00 00 00
>  0007 0000 0004 20 10 32 00
>  0008 0000 000A 1E 00 28 28 00 00 00 00 00 00

It seems that the T25(?) entry is missing in the new/expected 
configuration file. I figured I'd try out the new/expected configuration 
file, so did:

# ./obp-utils/mxt-app -d i2c-dev:1-004b --load 224sl.raw
# ./obp-utils/mxt-app -d i2c-dev:1-004b --save 
mxt-save-after-loading-224sl.raw.xml

At this point, mxt-save-after-loading-224sl.raw.xml contains identical 
content to mxt-save-after-t9-83-write.xml (my previous backup). It looks 
like the new configuration isn't being loaded correctly, or perhaps 
configuration loading doesn't delete entries that are simply not in the 
new configuration file?

I subsequently did the following in case --save is reading from the 
NVRAM rather than RAM:

# ./obp-utils/mxt-app -d i2c-dev:1-004b --backup
# ./obp-utils/mxt-app -d i2c-dev:1-004b --save 
mxt-save-after-loading-224sl.raw.xml

... but that made no difference.

I haven't yet tried upgrading or otherwise using the new firmware image. 
I'd like to make sure config load/save is fully working first, in case 
there's any common problem between the two.

Re: [PATCH 00/15] atmel_mxt_ts - device tree, bootloader, etc

[Yufeng Shen]

On Mon, Jul 28, 2014 at 7:42 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 07/28/2014 03:23 PM, Stephen Warren wrote:
>>
>> On 07/28/2014 02:20 PM, Yufeng Shen wrote:
>
> ...
>
>>> Where did you get the configuration file ? It is possible that we rely
>>> too much on mxt_start to turn on the T9.CTRL bit and have neglected
>>> its setting in the config file.
>>> If you can tell me where you get the config file I can do a check.
>>
>>
>> It was already flashed into the touchpad when I received the board. I
>> did try to track down the firmware/config files a few months ago, but
>> didn't manage to; I was told since they were already flashed so I didn't
>> need them. The board is Venice2.
>
>
> OK, I received the configuration and firmware file that's supposed to be in
> the touchpad.
>
> I can see that the config file I was given has the "83" byte in the T9
> configuration, and in fact /almost/ exactly matches the configuration I
> have. I don't know why my T9 configuration was wrong before, but I suspect
> it's not worth trying to track that down.
>
> Anyway, here's the diff between the two config files:
>
>> # diff -u mxt-save-after-t9-83-write.xml 224sl.raw
>> --- mxt-save-after-t9-83-write.xml      2014-07-25 19:41:45.000000000
>> +0000
>> +++ 224sl.raw   2014-07-28 23:25:49.000000000 +0000
>> @@ -1,8 +1,7 @@
>>  OBP_RAW V1
>>  82 01 10 AA 12 0C 16
>>  F5AF33
>> -000000
>> -0025 0000 0082 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>> 00 00 00 00 00 00 00 00 00 00 00
>> +E21E65
>>  0026 0000 0008 00 00 00 00 00 00 00 00
>>  0007 0000 0004 20 10 32 00
>>  0008 0000 000A 1E 00 28 28 00 00 00 00 00 00
>
>
> It seems that the T25(?) entry is missing in the new/expected configuration
> file. I figured I'd try out the new/expected configuration file, so did:
>

T37 (0x25) is DEBUG_DIAGNOSTIC object which the host can read debugging info
from. It is not useful to have a initial config for it so usually CrOS
system would just
don't include configuration for this object.

Nick, I want to confirm with you that does T37 contribute to config
checksum computation ?

> # ./obp-utils/mxt-app -d i2c-dev:1-004b --load 224sl.raw
> # ./obp-utils/mxt-app -d i2c-dev:1-004b --save
> mxt-save-after-loading-224sl.raw.xml
>
> At this point, mxt-save-after-loading-224sl.raw.xml contains identical
> content to mxt-save-after-t9-83-write.xml (my previous backup). It looks
> like the new configuration isn't being loaded correctly, or perhaps
> configuration loading doesn't delete entries that are simply not in the new
> configuration file?
>

Yeah, I would guess since T37 is not in the config, so whatever in the NVRAM
stays the same and when you --save its original value gets dumped.

> I subsequently did the following in case --save is reading from the NVRAM
> rather than RAM:
>
> # ./obp-utils/mxt-app -d i2c-dev:1-004b --backup
> # ./obp-utils/mxt-app -d i2c-dev:1-004b --save
> mxt-save-after-loading-224sl.raw.xml
>
> ... but that made no difference.
>
> I haven't yet tried upgrading or otherwise using the new firmware image. I'd
> like to make sure config load/save is fully working first, in case there's
> any common problem between the two.

Re: [PATCH 00/15] atmel_mxt_ts - device tree, bootloader, etc

[Stephen Warren]

On 07/28/2014 06:10 PM, Yufeng Shen wrote:
> On Mon, Jul 28, 2014 at 7:42 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
>> On 07/28/2014 03:23 PM, Stephen Warren wrote:
>>> On 07/28/2014 02:20 PM, Yufeng Shen wrote:
>>
>> ...
>>
>>>> Where did you get the configuration file ? It is possible that we rely
>>>> too much on mxt_start to turn on the T9.CTRL bit and have neglected
>>>> its setting in the config file.
>>>> If you can tell me where you get the config file I can do a check.
>>>
>>>
>>> It was already flashed into the touchpad when I received the board. I
>>> did try to track down the firmware/config files a few months ago, but
>>> didn't manage to; I was told since they were already flashed so I didn't
>>> need them. The board is Venice2.
>>
>> OK, I received the configuration and firmware file that's supposed to be in
>> the touchpad.
>>
>> I can see that the config file I was given has the "83" byte in the T9
>> configuration, and in fact /almost/ exactly matches the configuration I
>> have. I don't know why my T9 configuration was wrong before, but I suspect
>> it's not worth trying to track that down.
>>
>> Anyway, here's the diff between the two config files:
>>
>>> # diff -u mxt-save-after-t9-83-write.xml 224sl.raw
>>> --- mxt-save-after-t9-83-write.xml      2014-07-25 19:41:45.000000000
>>> +0000
>>> +++ 224sl.raw   2014-07-28 23:25:49.000000000 +0000
>>> @@ -1,8 +1,7 @@
>>>   OBP_RAW V1
>>>   82 01 10 AA 12 0C 16
>>>   F5AF33
>>> -000000
>>> -0025 0000 0082 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>>> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>>> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>>> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>>> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>>> 00 00 00 00 00 00 00 00 00 00 00
>>> +E21E65
>>>   0026 0000 0008 00 00 00 00 00 00 00 00
>>>   0007 0000 0004 20 10 32 00
>>>   0008 0000 000A 1E 00 28 28 00 00 00 00 00 00
>>
>>
>> It seems that the T25(?) entry is missing in the new/expected configuration
>> file. I figured I'd try out the new/expected configuration file, so did:
>
> T37 (0x25) is DEBUG_DIAGNOSTIC object which the host can read debugging info
> from. It is not useful to have a initial config for it so usually CrOS
> system would just don't include configuration for this object.

OK, that makes sense.

I also tested mxt-app --zero to clear the config, the dumped it with 
--save to verify it was cleared, then --load 224sl.raw and then --save 
to verify it was programmed back correctly. That seemed to all work fine.

I then tried updating the firmware. This didn't work at all.

First I tried via mxt-app:

> root@localhost:~# ./obp-utils/mxt-app -d i2c-dev:1-004b --flash 130.1_1.0.170.bin
> Version:1.16-65-g0a4c
> Opening firmware file 130.1_1.0.170.bin
> Registered i2c-dev adapter:1 address:0x4b
> Chip detected
> Current firmware version: 1.0.AA
> Skipping version check
> Resetting in bootloader mode
> Registered i2c-dev adapter:1 address:0x25
> Error Remote I/O error (121) reading from i2c
> Bootloader read failure
> Bootloader not found

Then I power-cycled and tried via the atmel_mxt_ts modules' sysfs files:

> root@localhost:~# echo 1 > /sys/devices/soc0/7000c400.i2c/i2c-1/1-004b/update_fw
> [   38.495420] atmel_mxt_ts 1-004b: mxt_bootloader_read: i2c recv failed (-121)
> [   38.506208] atmel_mxt_ts 1-004b: mxt_bootloader_read: i2c recv failed (-121)
> [   38.513836] atmel_mxt_ts 1-004b: The firmware update failed(-121)
> -bash: echo: write error: Remote I/O error

I also found that removing the module (even without attempting a FW 
update) yields:

After attempted FW update via sysfs:

> root@localhost:~# rmmod ./atmel_mxt_ts.ko
> [   81.995672] Unable to handle kernel NULL pointer dereference at virtual address 00000364
> [   82.003828] pgd = e8cd0000
> [   82.006548] [00000364] *pgd=00000000
> [   82.010221] Internal error: Oops: 5 [#1] PREEMPT SMP ARM
> [   82.015537] Modules linked in: atmel_mxt_ts(-)
> [   82.020007] CPU: 0 PID: 836 Comm: rmmod Not tainted 3.16.0-rc7-next-20140729-00011-gead0778e710c-dirty #7
> [   82.029559] task: e98ba140 ti: e8cc8000 task.ti: e8cc8000
> [   82.034961] PC is at input_unregister_device+0x8/0x70
> [   82.040010] LR is at mxt_remove+0x28/0x44 [atmel_mxt_ts]
> [   82.045315] pc : [<c039de7c>]    lr : [<bf000410>]    psr: 60000113
> [   82.045315] sp : e8cc9f08  ip : e97c7900  fp : 00000800
> [   82.056774] r10: 00000000  r9 : e8cc8000  r8 : c000e924
> [   82.061990] r7 : 00000081  r6 : ea1a7a54  r5 : bf003660  r4 : 00000000
> [   82.068505] r3 : 0000000c  r2 : 0000000a  r1 : 00000000  r0 : 00000000
> [   82.075024] Flags: nZCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment user
> [   82.082146] Control: 10c5387d  Table: a8cd006a  DAC: 00000015
> [   82.087882] Process rmmod (pid: 836, stack limit = 0xe8cc8240)
> [   82.093704] Stack: (0xe8cc9f08 to 0xe8cca000)
> [   82.098055] 9f00:                   e9a4e040 bf000410 ea1a7a20 c03bbe24 c03bbde0 c02dc12c
> [   82.106221] 9f20: bf003660 ea1a7a20 bf003660 c02dc910 bf003660 bf003704 00000800 c02dbfbc
> [   82.114386] 9f40: 00000000 c0081cdc e9f29018 00000000 bf003704 00000800 e8cc9f4c 656d7461
> [   82.122550] 9f60: 786d5f6c 73745f74 00000000 e98ba63c 00000000 c08c0d74 00000800 c0039d60
> [   82.130716] 9f80: e8c964c0 e8cc8000 e8cc8000 e8cc8000 c000e924 00010ef0 b6f3dd08 00000002
> [   82.138881] 9fa0: 00000000 c000e7a0 b6f3dd08 00000002 b6f3dd38 00000800 0cadcf00 0cadcf00
> [   82.147046] 9fc0: b6f3dd08 00000002 00000000 00000081 b6f3dd08 b6f3d008 beeac848 00000800
> [   82.155211] 9fe0: b6e65070 beeac5c4 b6ee02e9 b6e6507c 80000010 b6f3dd38 00000000 00000000
> [   82.163392] [<c039de7c>] (input_unregister_device) from [<bf000410>] (mxt_remove+0x28/0x44 [atmel_mxt_ts])
> [   82.173042] [<bf000410>] (mxt_remove [atmel_mxt_ts]) from [<c03bbe24>] (i2c_device_remove+0x44/0x5c)
> [   82.182171] [<c03bbe24>] (i2c_device_remove) from [<c02dc12c>] (__device_release_driver+0x70/0xc4)
> [   82.191122] [<c02dc12c>] (__device_release_driver) from [<c02dc910>] (driver_detach+0xac/0xb0)
> [   82.199726] [<c02dc910>] (driver_detach) from [<c02dbfbc>] (bus_remove_driver+0x4c/0x90)
> [   82.207810] [<c02dbfbc>] (bus_remove_driver) from [<c0081cdc>] (SyS_delete_module+0x108/0x194)
> [   82.216417] [<c0081cdc>] (SyS_delete_module) from [<c000e7a0>] (ret_fast_syscall+0x0/0x30)
> [   82.224672] Code: c089ecf0 c0786790 e92d4010 e1a04000 (e5d03364)
> [   82.231059] ---[ end trace e485a1b642f0d1d1 ]---
> Segmentation fault

After nothing but insmod:

> root@localhost:~# rmmod atmel_mxt_ts
> [   25.499625] Alignment trap: not handling instruction e1923f9f at [<c05ec6d8>]
> [   25.506763] Unhandled fault: alignment exception (0x001) at 0x6b6b6cc7
> [   25.513298] Internal error: : 1 [#1] PREEMPT SMP ARM
> [   25.518260] Modules linked in: atmel_mxt_ts(-)
> [   25.522724] CPU: 0 PID: 831 Comm: rmmod Not tainted 3.16.0-rc7-next-20140729-00011-gead0778e710c-dirty #7
> [   25.532277] task: ea205380 ti: e97d0000 task.ti: e97d0000
> [   25.537674] PC is at _raw_spin_lock_irqsave+0x2c/0x64
> [   25.542724] LR is at devres_remove+0x20/0x80
> [   25.546988] pc : [<c05ec6dc>]    lr : [<c02dec90>]    psr: 20000193
> [   25.546988] sp : e97d1ed0  ip : e9b5b5c0  fp : 00000800
> [   25.558446] r10: c039dee4  r9 : e97d0000  r8 : c039c278
> [   25.563662] r7 : e9a7d400  r6 : ea1a7a54  r5 : 6b6b6cc7  r4 : 6b6b6b6b
> [   25.570178] r3 : e97d0000  r2 : 6b6b6cc7  r1 : 00000001  r0 : 20000113
> [   25.576696] Flags: nzCv  IRQs off  FIQs on  Mode SVC_32  ISA ARM  Segment user
> [   25.583905] Control: 10c5387d  Table: a996406a  DAC: 00000015
> [   25.589641] Process rmmod (pid: 831, stack limit = 0xe97d0240)
> [   25.595464] Stack: (0xe97d1ed0 to 0xe97d2000)
> [   25.599816] 1ec0:                                     e9a7d400 e9a7d400 00000000 ea1a7a54
> [   25.607983] 1ee0: 00000081 c000e924 00000000 c02df508 e9a7d400 c039de9c e8c16b80 bf0003a4
> [   25.616148] 1f00: 00000019 e8c16b80 bf003660 bf000418 ea1a7a20 c03bbe24 c03bbde0 c02dc12c
> [   25.624313] 1f20: bf003660 ea1a7a20 bf003660 c02dc910 bf003660 bf003704 00000800 c02dbfbc
> [   25.632478] 1f40: 00000000 c0081cdc e9fe0e78 00000000 bf003704 00000800 e97d1f4c 656d7461
> [   25.640644] 1f60: 786d5f6c 73745f74 00000000 ea20587c 00000000 c08c0d74 00000800 c0039d60
> [   25.648808] 1f80: e9b8d880 e97d0000 e97d0000 e97d0000 c000e924 00010ef0 b6fe7d08 00000002
> [   25.656973] 1fa0: 00000000 c000e7a0 b6fe7d08 00000002 b6fe7d38 00000800 7a392d00 7a392d00
> [   25.665138] 1fc0: b6fe7d08 00000002 00000000 00000081 b6fe7d08 b6fe7008 be857858 00000800
> [   25.673303] 1fe0: b6f0f070 be8575d4 b6f8a2e9 b6f0f07c 80000010 b6fe7d38 00000000 00000000
> [   25.681479] [<c05ec6dc>] (_raw_spin_lock_irqsave) from [<c02dec90>] (devres_remove+0x20/0x80)
> [   25.689999] [<c02dec90>] (devres_remove) from [<c02df508>] (devres_destroy+0x8/0x24)
> [   25.697738] [<c02df508>] (devres_destroy) from [<c039de9c>] (input_unregister_device+0x28/0x70)
> [   25.706435] [<c039de9c>] (input_unregister_device) from [<bf0003a4>] (mxt_free_object_table+0x14/0x58 [atmel_mxt_ts])
> [   25.717037] [<bf0003a4>] (mxt_free_object_table [atmel_mxt_ts]) from [<bf000418>] (mxt_remove+0x30/0x44 [atmel_mxt_ts])
> [   25.727813] [<bf000418>] (mxt_remove [atmel_mxt_ts]) from [<c03bbe24>] (i2c_device_remove+0x44/0x5c)
> [   25.736940] [<c03bbe24>] (i2c_device_remove) from [<c02dc12c>] (__device_release_driver+0x70/0xc4)
> [   25.745891] [<c02dc12c>] (__device_release_driver) from [<c02dc910>] (driver_detach+0xac/0xb0)
> [   25.754494] [<c02dc910>] (driver_detach) from [<c02dbfbc>] (bus_remove_driver+0x4c/0x90)
> [   25.762579] [<c02dbfbc>] (bus_remove_driver) from [<c0081cdc>] (SyS_delete_module+0x108/0x194)
> [   25.771184] [<c0081cdc>] (SyS_delete_module) from [<c000e7a0>] (ret_fast_syscall+0x0/0x30)
> [   25.779438] Code: e2811001 e5831004 f592f000 e1923f9f (e2831801)
> [   25.785524] ---[ end trace fd2f70b3c6f48889 ]---
> [   25.790136] note: rmmod[831] exited with preempt_count 1
> Segmentation fault

Re: [PATCH 00/15] atmel_mxt_ts - device tree, bootloader, etc

[Nick Dyer]

On 29/07/14 17:16, Stephen Warren wrote:
> I then tried updating the firmware. This didn't work at all.
> 
> First I tried via mxt-app:
> 
>> root@localhost:~# ./obp-utils/mxt-app -d i2c-dev:1-004b --flash
>> 130.1_1.0.170.bin
>> Version:1.16-65-g0a4c
>> Opening firmware file 130.1_1.0.170.bin
>> Registered i2c-dev adapter:1 address:0x4b
>> Chip detected
>> Current firmware version: 1.0.AA
>> Skipping version check
>> Resetting in bootloader mode
>> Registered i2c-dev adapter:1 address:0x25
>> Error Remote I/O error (121) reading from i2c
>> Bootloader read failure
>> Bootloader not found
> 
> Then I power-cycled and tried via the atmel_mxt_ts modules' sysfs files:
> 
>> root@localhost:~# echo 1 >
>> /sys/devices/soc0/7000c400.i2c/i2c-1/1-004b/update_fw
>> [   38.495420] atmel_mxt_ts 1-004b: mxt_bootloader_read: i2c recv failed
>> (-121)
>> [   38.506208] atmel_mxt_ts 1-004b: mxt_bootloader_read: i2c recv failed
>> (-121)
>> [   38.513836] atmel_mxt_ts 1-004b: The firmware update failed(-121)
>> -bash: echo: write error: Remote I/O error

OK - that's the same error in both cases, it has tried to switch the device
into bootloader mode, however it is not responding on the bootloader I2C
address.

Couple of things to check:
- is the device in deep sleep mode when you run the mxt-app command? can
you try doing "mxt-app [device] -W -T7 FFFF" which will make sure it is
definitely not sleeping first.
- if you do "mxt-app [device] -i" straight after the --flash failure, does
it respond (which would mean it hasn't actioned the reset-into-bootloader
command)

> I also found that removing the module (even without attempting a FW update)
> yields:
> 
> After attempted FW update via sysfs:
> 
>> root@localhost:~# rmmod ./atmel_mxt_ts.ko
>> [   81.995672] Unable to handle kernel NULL pointer dereference at virtual address 00000364

Not good. It's trying to free an input device which isn't registered at
that point. In a later patch in my series I add a guard around that
(mxt_free_input_device):
https://github.com/ndyer/linux/commit/bb4800ff8c185

Re: [PATCH 00/15] atmel_mxt_ts - device tree, bootloader, etc

[Stephen Warren]

On 07/29/2014 11:06 AM, Nick Dyer wrote:
> On 29/07/14 17:16, Stephen Warren wrote:
>> I then tried updating the firmware. This didn't work at all.
>>
>> First I tried via mxt-app:
>>
>>> root@localhost:~# ./obp-utils/mxt-app -d i2c-dev:1-004b --flash
>>> 130.1_1.0.170.bin
>>> Version:1.16-65-g0a4c
>>> Opening firmware file 130.1_1.0.170.bin
>>> Registered i2c-dev adapter:1 address:0x4b
>>> Chip detected
>>> Current firmware version: 1.0.AA
>>> Skipping version check
>>> Resetting in bootloader mode
>>> Registered i2c-dev adapter:1 address:0x25
>>> Error Remote I/O error (121) reading from i2c
>>> Bootloader read failure
>>> Bootloader not found
>>
>> Then I power-cycled and tried via the atmel_mxt_ts modules' sysfs files:
>>
>>> root@localhost:~# echo 1 >
>>> /sys/devices/soc0/7000c400.i2c/i2c-1/1-004b/update_fw
>>> [   38.495420] atmel_mxt_ts 1-004b: mxt_bootloader_read: i2c recv failed
>>> (-121)
>>> [   38.506208] atmel_mxt_ts 1-004b: mxt_bootloader_read: i2c recv failed
>>> (-121)
>>> [   38.513836] atmel_mxt_ts 1-004b: The firmware update failed(-121)
>>> -bash: echo: write error: Remote I/O error
>
> OK - that's the same error in both cases, it has tried to switch the device
> into bootloader mode, however it is not responding on the bootloader I2C
> address.
>
> Couple of things to check:
> - is the device in deep sleep mode when you run the mxt-app command? can
> you try doing "mxt-app [device] -W -T7 FFFF" which will make sure it is
> definitely not sleeping first.

That didn't hekp.

> - if you do "mxt-app [device] -i" straight after the --flash failure, does
> it respond (which would mean it hasn't actioned the reset-into-bootloader
> command)

That still works, so I assume it wasn't reset into bootloader mode. I 
also tried mxt-app --reset-bootloader, and mxt-app -i still works after 
that too.

>> I also found that removing the module (even without attempting a FW update)
>> yields:
>>
>> After attempted FW update via sysfs:
>>
>>> root@localhost:~# rmmod ./atmel_mxt_ts.ko
>>> [   81.995672] Unable to handle kernel NULL pointer dereference at virtual address 00000364
>
> Not good. It's trying to free an input device which isn't registered at
> that point. In a later patch in my series I add a guard around that
> (mxt_free_input_device):
> https://github.com/ndyer/linux/commit/bb4800ff8c185

It looks like 8d01a84b86b0 "Input: atmel_mxt_ts - implement T100 touch 
object support" from that branch is what solves the issue.

Ah, in linux-next, it's simply a double-unregister; both mxt_remove() 
and mxt_free_object_table() call 
input_unregister_device(data->input_dev). I think it would make sense to 
send the following patch for 3.17. Do you agree?

diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c 
b/drivers/input/touchscreen/atmel_mxt_ts.c
index 03b85711cb70..da497dbf9735 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -1353,8 +1353,10 @@ static int mxt_get_info(struct mxt_data *data)

  static void mxt_free_object_table(struct mxt_data *data)
  {
-	input_unregister_device(data->input_dev);
-	data->input_dev = NULL;
+	if (data->input_dev) {
+		input_unregister_device(data->input_dev);
+		data->input_dev = NULL;
+	}

  	kfree(data->object_table);
  	data->object_table = NULL;
@@ -2194,7 +2196,6 @@ static int mxt_remove(struct i2c_client *client)

  	sysfs_remove_group(&client->dev.kobj, &mxt_attr_group);
  	free_irq(data->irq, data);
-	input_unregister_device(data->input_dev);
  	mxt_free_object_table(data);
  	kfree(data);

Re: [PATCH 00/15] atmel_mxt_ts - device tree, bootloader, etc

[Stephen Warren]

On 07/29/2014 01:26 PM, Stephen Warren wrote:
> On 07/29/2014 11:06 AM, Nick Dyer wrote:
>> On 29/07/14 17:16, Stephen Warren wrote:
...
>>> I also found that removing the module (even without attempting a FW
>>> update) yields:
>>>
>>> After attempted FW update via sysfs:
>>>
>>>> root@localhost:~# rmmod ./atmel_mxt_ts.ko
>>>> [   81.995672] Unable to handle kernel NULL pointer dereference at
>>>> virtual address 00000364
>>
>> Not good. It's trying to free an input device which isn't registered at
>> that point. In a later patch in my series I add a guard around that
>> (mxt_free_input_device):
>> https://github.com/ndyer/linux/commit/bb4800ff8c185
>
> It looks like 8d01a84b86b0 "Input: atmel_mxt_ts - implement T100 touch
> object support" from that branch is what solves the issue.
>
> Ah, in linux-next, it's simply a double-unregister; both mxt_remove()
> and mxt_free_object_table() call
> input_unregister_device(data->input_dev). I think it would make sense to
> send the following patch for 3.17. Do you agree?

Nick, I see the fix below isn't in linux-next as of next-20140829. IIRC, 
you'd sent the patch below but there had been some comments on it by 
Dmitry. Unfortunately, I don't recall the patch subject to search for 
it. Anyway, are you planning on re-spinning the patch so it can be applied?

> diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c
> b/drivers/input/touchscreen/atmel_mxt_ts.c
> index 03b85711cb70..da497dbf9735 100644
> --- a/drivers/input/touchscreen/atmel_mxt_ts.c
> +++ b/drivers/input/touchscreen/atmel_mxt_ts.c
> @@ -1353,8 +1353,10 @@ static int mxt_get_info(struct mxt_data *data)
>
>   static void mxt_free_object_table(struct mxt_data *data)
>   {
> -    input_unregister_device(data->input_dev);
> -    data->input_dev = NULL;
> +    if (data->input_dev) {
> +        input_unregister_device(data->input_dev);
> +        data->input_dev = NULL;
> +    }
>
>       kfree(data->object_table);
>       data->object_table = NULL;
> @@ -2194,7 +2196,6 @@ static int mxt_remove(struct i2c_client *client)
>
>       sysfs_remove_group(&client->dev.kobj, &mxt_attr_group);
>       free_irq(data->irq, data);
> -    input_unregister_device(data->input_dev);
>       mxt_free_object_table(data);
>       kfree(data);

Re: [PATCH 00/15] atmel_mxt_ts - device tree, bootloader, etc

[Nick Dyer]

On 29/07/14 01:10, Yufeng Shen wrote:
> T37 (0x25) is DEBUG_DIAGNOSTIC object which the host can read debugging info
> from. It is not useful to have a initial config for it so usually CrOS
> system would just
> don't include configuration for this object.
> 
> Nick, I want to confirm with you that does T37 contribute to config
> checksum computation?

You are correct.

>> At this point, mxt-save-after-loading-224sl.raw.xml contains identical
>> content to mxt-save-after-t9-83-write.xml (my previous backup). It looks
>> like the new configuration isn't being loaded correctly, or perhaps
>> configuration loading doesn't delete entries that are simply not in the new
>> configuration file?
> 
> Yeah, I would guess since T37 is not in the config, so whatever in the NVRAM
> stays the same and when you --save its original value gets dumped.

Yes. You need to call --zero to get a truly blank config, before loading.

Re: [PATCH 00/15] atmel_mxt_ts - device tree, bootloader, etc

[Nick Dyer]

On 29/07/14 00:42, Stephen Warren wrote:
> Anyway, here's the diff between the two config files:
> 
>> # diff -u mxt-save-after-t9-83-write.xml 224sl.raw
>> --- mxt-save-after-t9-83-write.xml    2014-07-25 19:41:45.000000000 +0000
>> +++ 224sl.raw    2014-07-28 23:25:49.000000000 +0000
>> @@ -1,8 +1,7 @@
>>  OBP_RAW V1
>>  82 01 10 AA 12 0C 16
>>  F5AF33
>> -000000
>> -0025 0000 0082 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>> +E21E65
>>  0026 0000 0008 00 00 00 00 00 00 00 00
>>  0007 0000 0004 20 10 32 00
>>  0008 0000 000A 1E 00 28 28 00 00 00 00 00 00
> 
> It seems that the T25(?) entry is missing in the new/expected configuration
> file. I figured I'd try out the new/expected configuration file, so did:

0x25 = 37. T37 is the diagnostic debug object. A change was made fairly
recently to the tools to save this object into the config files, which is
the reason why it is missing in one of your files. But the difference is
essentially meaningless for your purposes, and writing those zeros to the
chip won't affect anything.

> # ./obp-utils/mxt-app -d i2c-dev:1-004b --load 224sl.raw
> # ./obp-utils/mxt-app -d i2c-dev:1-004b --save
> mxt-save-after-loading-224sl.raw.xml
> 
> At this point, mxt-save-after-loading-224sl.raw.xml contains identical
> content to mxt-save-after-t9-83-write.xml (my previous backup). It looks
> like the new configuration isn't being loaded correctly, or perhaps
> configuration loading doesn't delete entries that are simply not in the new
> configuration file?
> 
> I subsequently did the following in case --save is reading from the NVRAM
> rather than RAM:

The --save command reads from RAM. There's no way of reading from NVRAM.