Linux Input/HID development
 help / color / mirror / Atom feed
* [PATCH 01/15] Input: atmel_mxt_ts - initialise IRQ before probing
From: nick.dyer @ 2014-07-03 15:01 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Yufeng Shen, Daniel Kurtz, Henrik Rydberg, Joonyoung Shim,
	Alan Bowens, linux-input, linux-kernel, Peter Meerwald,
	Benson Leung, Olof Johansson, Sekhar Nori, Nick Dyer
In-Reply-To: <1404399697-26484-1-git-send-email-nick.dyer@itdev.co.uk>

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


^ permalink raw reply related

* [PATCH 00/15] atmel_mxt_ts - device tree, bootloader, etc
From: nick.dyer @ 2014-07-03 15:01 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Yufeng Shen, Daniel Kurtz, Henrik Rydberg, Joonyoung Shim,
	Alan Bowens, linux-input, linux-kernel, Peter Meerwald,
	Benson Leung, Olof Johansson, Sekhar Nori

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


^ permalink raw reply

* [PATCH 04/15] Input: atmel_mxt_ts - implement device tree support
From: nick.dyer @ 2014-07-03 15:01 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Yufeng Shen, Daniel Kurtz, Henrik Rydberg, Joonyoung Shim,
	Alan Bowens, linux-input, linux-kernel, Peter Meerwald,
	Benson Leung, Olof Johansson, Sekhar Nori, Stephen Warren,
	Nick Dyer
In-Reply-To: <1404399697-26484-1-git-send-email-nick.dyer@itdev.co.uk>

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


^ permalink raw reply related

* [PATCH 03/15] Input: atmel_mxt_ts - set pointer emulation on touchpads
From: nick.dyer @ 2014-07-03 15:01 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Yufeng Shen, Daniel Kurtz, Henrik Rydberg, Joonyoung Shim,
	Alan Bowens, linux-input, linux-kernel, Peter Meerwald,
	Benson Leung, Olof Johansson, Sekhar Nori, Stephen Warren,
	Nick Dyer
In-Reply-To: <1404399697-26484-1-git-send-email-nick.dyer@itdev.co.uk>

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


^ permalink raw reply related

* [PATCH 02/15] Input: atmel_mxt_ts - move input device init into separate function
From: nick.dyer @ 2014-07-03 15:01 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Yufeng Shen, Daniel Kurtz, Henrik Rydberg, Joonyoung Shim,
	Alan Bowens, linux-input, linux-kernel, Peter Meerwald,
	Benson Leung, Olof Johansson, Sekhar Nori, Nick Dyer
In-Reply-To: <1404399697-26484-1-git-send-email-nick.dyer@itdev.co.uk>

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


^ permalink raw reply related

* [PATCH 06/15] Input: atmel_mxt_ts - calculate and check CRC in config file
From: nick.dyer @ 2014-07-03 15:01 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Yufeng Shen, Daniel Kurtz, Henrik Rydberg, Joonyoung Shim,
	Alan Bowens, linux-input, linux-kernel, Peter Meerwald,
	Benson Leung, Olof Johansson, Sekhar Nori, Nick Dyer
In-Reply-To: <1404399697-26484-1-git-send-email-nick.dyer@itdev.co.uk>

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


^ permalink raw reply related

* [PATCH 05/15] Input: atmel_mxt_ts - download device config using firmware loader
From: nick.dyer @ 2014-07-03 15:01 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Yufeng Shen, Daniel Kurtz, Henrik Rydberg, Joonyoung Shim,
	Alan Bowens, linux-input, linux-kernel, Peter Meerwald,
	Benson Leung, Olof Johansson, Sekhar Nori, Nick Dyer
In-Reply-To: <1404399697-26484-1-git-send-email-nick.dyer@itdev.co.uk>

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


^ permalink raw reply related

* [PATCH 08/15] Input: atmel_mxt_ts - handle APP_CRC_FAIL on startup
From: nick.dyer @ 2014-07-03 15:01 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Yufeng Shen, Daniel Kurtz, Henrik Rydberg, Joonyoung Shim,
	Alan Bowens, linux-input, linux-kernel, Peter Meerwald,
	Benson Leung, Olof Johansson, Sekhar Nori, Nick Dyer
In-Reply-To: <1404399697-26484-1-git-send-email-nick.dyer@itdev.co.uk>

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


^ permalink raw reply related

* [PATCH 07/15] Input: atmel_mxt_ts - use deep sleep mode when stopped
From: nick.dyer @ 2014-07-03 15:01 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Yufeng Shen, Daniel Kurtz, Henrik Rydberg, Joonyoung Shim,
	Alan Bowens, linux-input, linux-kernel, Peter Meerwald,
	Benson Leung, Olof Johansson, Sekhar Nori, Nick Dyer
In-Reply-To: <1404399697-26484-1-git-send-email-nick.dyer@itdev.co.uk>

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


^ permalink raw reply related

* [PATCH 10/15] Input: atmel_mxt_ts - add bootloader addresses for new chips
From: nick.dyer @ 2014-07-03 15:01 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Yufeng Shen, Daniel Kurtz, Henrik Rydberg, Joonyoung Shim,
	Alan Bowens, linux-input, linux-kernel, Peter Meerwald,
	Benson Leung, Olof Johansson, Sekhar Nori, Nick Dyer
In-Reply-To: <1404399697-26484-1-git-send-email-nick.dyer@itdev.co.uk>

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


^ permalink raw reply related

* [PATCH 09/15] Input: atmel_mxt_ts - handle bootloader previously unlocked
From: nick.dyer @ 2014-07-03 15:01 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Yufeng Shen, Daniel Kurtz, Henrik Rydberg, Joonyoung Shim,
	Alan Bowens, linux-input, linux-kernel, Peter Meerwald,
	Benson Leung, Olof Johansson, Sekhar Nori, Nick Dyer
In-Reply-To: <1404399697-26484-1-git-send-email-nick.dyer@itdev.co.uk>

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


^ permalink raw reply related

* [PATCH 11/15] Input: atmel_mxt_ts - recover from bootloader on probe
From: nick.dyer @ 2014-07-03 15:01 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Yufeng Shen, Daniel Kurtz, Henrik Rydberg, Joonyoung Shim,
	Alan Bowens, linux-input, linux-kernel, Peter Meerwald,
	Benson Leung, Olof Johansson, Sekhar Nori, Nick Dyer
In-Reply-To: <1404399697-26484-1-git-send-email-nick.dyer@itdev.co.uk>

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


^ permalink raw reply related

* [PATCH 12/15] Input: atmel_mxt_ts - add support for dynamic message size
From: nick.dyer @ 2014-07-03 15:01 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Yufeng Shen, Daniel Kurtz, Henrik Rydberg, Joonyoung Shim,
	Alan Bowens, linux-input, linux-kernel, Peter Meerwald,
	Benson Leung, Olof Johansson, Sekhar Nori, Nick Dyer
In-Reply-To: <1404399697-26484-1-git-send-email-nick.dyer@itdev.co.uk>

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


^ permalink raw reply related

* [PATCH 13/15] Input: atmel_mxt_ts - decode T6 status messages
From: nick.dyer @ 2014-07-03 15:01 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Yufeng Shen, Daniel Kurtz, Henrik Rydberg, Joonyoung Shim,
	Alan Bowens, linux-input, linux-kernel, Peter Meerwald,
	Benson Leung, Olof Johansson, Sekhar Nori, Nick Dyer
In-Reply-To: <1404399697-26484-1-git-send-email-nick.dyer@itdev.co.uk>

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


^ permalink raw reply related

* [PATCH 14/15] Input: atmel_mxt_ts - split message handler into separate functions
From: nick.dyer @ 2014-07-03 15:01 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Yufeng Shen, Daniel Kurtz, Henrik Rydberg, Joonyoung Shim,
	Alan Bowens, linux-input, linux-kernel, Peter Meerwald,
	Benson Leung, Olof Johansson, Sekhar Nori, Nick Dyer
In-Reply-To: <1404399697-26484-1-git-send-email-nick.dyer@itdev.co.uk>

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


^ permalink raw reply related

* [PATCH 15/15] Input: atmel_mxt_ts - implement T44 message handling
From: nick.dyer @ 2014-07-03 15:01 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Yufeng Shen, Daniel Kurtz, Henrik Rydberg, Joonyoung Shim,
	Alan Bowens, linux-input, linux-kernel, Peter Meerwald,
	Benson Leung, Olof Johansson, Sekhar Nori, Nick Dyer
In-Reply-To: <1404399697-26484-1-git-send-email-nick.dyer@itdev.co.uk>

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


^ permalink raw reply related

* [PATCH 2/2 v6] HID: gt683r: move mode attribute to led-class devices
From: Janne Kanniainen @ 2014-07-03 17:17 UTC (permalink / raw)
  To: johan
  Cc: cooloney, jkosina, greg, bjorn, linux-kernel, linux-leds,
	linux-usb, linux-input, Janne Kanniainen
In-Reply-To: <1404407829-1815-1-git-send-email-janne.kanniainen@gmail.com>

Move led_mode attribute from HID device to led-class devices and rename
it msi_mode. This will also fix race condition by using
attribute-groups.

Signed-off-by: Janne Kanniainen <janne.kanniainen@gmail.com>
---

Changes in v3:
	- Style fixes
	- Rename sysfs-class-hid-driver-gt683r to sysfs-class-leds-driver-gt683r

Changes in v4:
	- Rename sysfs-class-leds-driver-gt683r to sysfs-class-leds-gt683r
	- Change "What: " to /sys/class/leds/<led>/gt683r/mode
	- Change .name from gt683r_led to gt683r

Changes in v5:
	- Move mode attribute to gt683r/mode

Changes in v6:
	- Fix subject and commit message

 .../ABI/testing/sysfs-class-hid-driver-gt683r      | 14 ---------
 Documentation/ABI/testing/sysfs-class-leds-gt683r  | 16 ++++++++++
 drivers/hid/hid-gt683r.c                           | 36 ++++++++++++++--------
 3 files changed, 40 insertions(+), 26 deletions(-)
 delete mode 100644 Documentation/ABI/testing/sysfs-class-hid-driver-gt683r
 create mode 100644 Documentation/ABI/testing/sysfs-class-leds-gt683r

diff --git a/Documentation/ABI/testing/sysfs-class-hid-driver-gt683r b/Documentation/ABI/testing/sysfs-class-hid-driver-gt683r
deleted file mode 100644
index 317e9d5..0000000
--- a/Documentation/ABI/testing/sysfs-class-hid-driver-gt683r
+++ /dev/null
@@ -1,14 +0,0 @@
-What:		/sys/class/hidraw/<hidraw>/device/leds_mode
-Date:		Jun 2014
-KernelVersion:	3.17
-Contact:	Janne Kanniainen <janne.kanniainen@gmail.com>
-Description:
-		Set the mode of LEDs
-
-		0 - normal
-		1 - audio
-		2 - breathing
-
-		Normal: LEDs are fully on when enabled
-		Audio:  LEDs brightness depends on sound level
-		Breathing: LEDs brightness varies at human breathing rate
\ No newline at end of file
diff --git a/Documentation/ABI/testing/sysfs-class-leds-gt683r b/Documentation/ABI/testing/sysfs-class-leds-gt683r
new file mode 100644
index 0000000..e4fae60
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-class-leds-gt683r
@@ -0,0 +1,16 @@
+What:		/sys/class/leds/<led>/gt683r/mode
+Date:		Jun 2014
+KernelVersion:	3.17
+Contact:	Janne Kanniainen <janne.kanniainen@gmail.com>
+Description:
+		Set the mode of LEDs. You should notice that changing the mode
+		of one LED will update the mode of its two sibling devices as
+		well.
+
+		0 - normal
+		1 - audio
+		2 - breathing
+
+		Normal: LEDs are fully on when enabled
+		Audio:  LEDs brightness depends on sound level
+		Breathing: LEDs brightness varies at human breathing rate
\ No newline at end of file
diff --git a/drivers/hid/hid-gt683r.c b/drivers/hid/hid-gt683r.c
index 073bd80..0d6f135 100644
--- a/drivers/hid/hid-gt683r.c
+++ b/drivers/hid/hid-gt683r.c
@@ -84,12 +84,13 @@ static void gt683r_brightness_set(struct led_classdev *led_cdev,
 	}
 }
 
-static ssize_t leds_mode_show(struct device *dev,
+static ssize_t mode_show(struct device *dev,
 				struct device_attribute *attr,
 				char *buf)
 {
 	u8 sysfs_mode;
-	struct hid_device *hdev = container_of(dev, struct hid_device, dev);
+	struct hid_device *hdev = container_of(dev->parent,
+					struct hid_device, dev);
 	struct gt683r_led *led = hid_get_drvdata(hdev);
 
 	if (led->mode == GT683R_LED_NORMAL)
@@ -102,12 +103,13 @@ static ssize_t leds_mode_show(struct device *dev,
 	return scnprintf(buf, PAGE_SIZE, "%u\n", sysfs_mode);
 }
 
-static ssize_t leds_mode_store(struct device *dev,
+static ssize_t mode_store(struct device *dev,
 				struct device_attribute *attr,
 				const char *buf, size_t count)
 {
 	u8 sysfs_mode;
-	struct hid_device *hdev = container_of(dev, struct hid_device, dev);
+	struct hid_device *hdev = container_of(dev->parent,
+					struct hid_device, dev);
 	struct gt683r_led *led = hid_get_drvdata(hdev);
 
 
@@ -212,7 +214,22 @@ fail:
 	mutex_unlock(&led->lock);
 }
 
-static DEVICE_ATTR_RW(leds_mode);
+static DEVICE_ATTR_RW(mode);
+
+static struct attribute *gt683r_led_attrs[] = {
+	&dev_attr_mode.attr,
+	NULL
+};
+
+static const struct attribute_group gt683r_led_group = {
+	.name = "gt683r",
+	.attrs = gt683r_led_attrs,
+};
+
+static const struct attribute_group *gt683r_led_groups[] = {
+	&gt683r_led_group,
+	NULL
+};
 
 static int gt683r_led_probe(struct hid_device *hdev,
 			const struct hid_device_id *id)
@@ -261,6 +278,8 @@ static int gt683r_led_probe(struct hid_device *hdev,
 		led->led_devs[i].name = name;
 		led->led_devs[i].max_brightness = 1;
 		led->led_devs[i].brightness_set = gt683r_brightness_set;
+		led->led_devs[i].groups = gt683r_led_groups;
+
 		ret = led_classdev_register(&hdev->dev, &led->led_devs[i]);
 		if (ret) {
 			hid_err(hdev, "could not register led device\n");
@@ -268,12 +287,6 @@ static int gt683r_led_probe(struct hid_device *hdev,
 		}
 	}
 
-	ret = device_create_file(&led->hdev->dev, &dev_attr_leds_mode);
-	if (ret) {
-		hid_err(hdev, "could not make mode attribute file\n");
-		goto fail;
-	}
-
 	return 0;
 
 fail:
@@ -288,7 +301,6 @@ static void gt683r_led_remove(struct hid_device *hdev)
 	int i;
 	struct gt683r_led *led = hid_get_drvdata(hdev);
 
-	device_remove_file(&hdev->dev, &dev_attr_leds_mode);
 	for (i = 0; i < GT683R_LED_COUNT; i++)
 		led_classdev_unregister(&led->led_devs[i]);
 	flush_work(&led->work);
-- 
1.9.2

^ permalink raw reply related

* [PATCH 1/2 v6] HID: gt683r: fix race condition
From: Janne Kanniainen @ 2014-07-03 17:17 UTC (permalink / raw)
  To: johan
  Cc: cooloney, jkosina, greg, bjorn, linux-kernel, linux-leds,
	linux-usb, linux-input, Janne Kanniainen
In-Reply-To: <20140703082830.GB16590@localhost>

This will fix race condition noticed by Oliver Neukum. Sysfs files are
created before mutex and work are initialized.

Signed-off-by: Janne Kanniainen <janne.kanniainen@gmail.com>
---
 drivers/hid/hid-gt683r.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/hid/hid-gt683r.c b/drivers/hid/hid-gt683r.c
index 077f7a1..073bd80 100644
--- a/drivers/hid/hid-gt683r.c
+++ b/drivers/hid/hid-gt683r.c
@@ -227,6 +227,9 @@ static int gt683r_led_probe(struct hid_device *hdev,
 	if (!led)
 		return -ENOMEM;
 
+	mutex_init(&led->lock);
+	INIT_WORK(&led->work, gt683r_led_work);
+
 	led->mode = GT683R_LED_NORMAL;
 	led->hdev = hdev;
 	hid_set_drvdata(hdev, led);
@@ -271,9 +274,6 @@ static int gt683r_led_probe(struct hid_device *hdev,
 		goto fail;
 	}
 
-	mutex_init(&led->lock);
-	INIT_WORK(&led->work, gt683r_led_work);
-
 	return 0;
 
 fail:
-- 
1.9.2


^ permalink raw reply related

* Re: [PATCH 1/2 v6] HID: gt683r: fix race condition
From: Johan Hovold @ 2014-07-03 17:34 UTC (permalink / raw)
  To: Janne Kanniainen
  Cc: johan, cooloney, jkosina, greg, bjorn, linux-kernel, linux-leds,
	linux-usb, linux-input
In-Reply-To: <1404407829-1815-1-git-send-email-janne.kanniainen@gmail.com>

On Thu, Jul 03, 2014 at 08:17:08PM +0300, Janne Kanniainen wrote:
> This will fix race condition noticed by Oliver Neukum. Sysfs files are
> created before mutex and work are initialized.
> 
> Signed-off-by: Janne Kanniainen <janne.kanniainen@gmail.com>

Reviewed-by: Johan Hovold <johan@kernel.org>

> ---
>  drivers/hid/hid-gt683r.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/hid/hid-gt683r.c b/drivers/hid/hid-gt683r.c
> index 077f7a1..073bd80 100644
> --- a/drivers/hid/hid-gt683r.c
> +++ b/drivers/hid/hid-gt683r.c
> @@ -227,6 +227,9 @@ static int gt683r_led_probe(struct hid_device *hdev,
>  	if (!led)
>  		return -ENOMEM;
>  
> +	mutex_init(&led->lock);
> +	INIT_WORK(&led->work, gt683r_led_work);
> +
>  	led->mode = GT683R_LED_NORMAL;
>  	led->hdev = hdev;
>  	hid_set_drvdata(hdev, led);
> @@ -271,9 +274,6 @@ static int gt683r_led_probe(struct hid_device *hdev,
>  		goto fail;
>  	}
>  
> -	mutex_init(&led->lock);
> -	INIT_WORK(&led->work, gt683r_led_work);
> -
>  	return 0;
>  
>  fail:

^ permalink raw reply

* Re: [PATCH 2/2 v6] HID: gt683r: move mode attribute to led-class devices
From: Johan Hovold @ 2014-07-03 17:40 UTC (permalink / raw)
  To: Janne Kanniainen
  Cc: johan, cooloney, jkosina, greg, bjorn, linux-kernel, linux-leds,
	linux-usb, linux-input
In-Reply-To: <1404407829-1815-2-git-send-email-janne.kanniainen@gmail.com>

On Thu, Jul 03, 2014 at 08:17:09PM +0300, Janne Kanniainen wrote:
> Move led_mode attribute from HID device to led-class devices and rename
> it msi_mode. This will also fix race condition by using

There's a typo here (s/msi_mode/mode) but perhaps Bryan can just fix
that up before applying?

> attribute-groups.
> 
> Signed-off-by: Janne Kanniainen <janne.kanniainen@gmail.com>

Reviewed-by: Johan Hovold <johan@kernel.org>

Otherwise both patches (v6) are ready to be merged, Bryan.

Thanks, Janne!

Johan

> ---
> 
> Changes in v3:
> 	- Style fixes
> 	- Rename sysfs-class-hid-driver-gt683r to sysfs-class-leds-driver-gt683r
> 
> Changes in v4:
> 	- Rename sysfs-class-leds-driver-gt683r to sysfs-class-leds-gt683r
> 	- Change "What: " to /sys/class/leds/<led>/gt683r/mode
> 	- Change .name from gt683r_led to gt683r
> 
> Changes in v5:
> 	- Move mode attribute to gt683r/mode
> 
> Changes in v6:
> 	- Fix subject and commit message
> 
>  .../ABI/testing/sysfs-class-hid-driver-gt683r      | 14 ---------
>  Documentation/ABI/testing/sysfs-class-leds-gt683r  | 16 ++++++++++
>  drivers/hid/hid-gt683r.c                           | 36 ++++++++++++++--------
>  3 files changed, 40 insertions(+), 26 deletions(-)
>  delete mode 100644 Documentation/ABI/testing/sysfs-class-hid-driver-gt683r
>  create mode 100644 Documentation/ABI/testing/sysfs-class-leds-gt683r
> 
> diff --git a/Documentation/ABI/testing/sysfs-class-hid-driver-gt683r b/Documentation/ABI/testing/sysfs-class-hid-driver-gt683r
> deleted file mode 100644
> index 317e9d5..0000000
> --- a/Documentation/ABI/testing/sysfs-class-hid-driver-gt683r
> +++ /dev/null
> @@ -1,14 +0,0 @@
> -What:		/sys/class/hidraw/<hidraw>/device/leds_mode
> -Date:		Jun 2014
> -KernelVersion:	3.17
> -Contact:	Janne Kanniainen <janne.kanniainen@gmail.com>
> -Description:
> -		Set the mode of LEDs
> -
> -		0 - normal
> -		1 - audio
> -		2 - breathing
> -
> -		Normal: LEDs are fully on when enabled
> -		Audio:  LEDs brightness depends on sound level
> -		Breathing: LEDs brightness varies at human breathing rate
> \ No newline at end of file
> diff --git a/Documentation/ABI/testing/sysfs-class-leds-gt683r b/Documentation/ABI/testing/sysfs-class-leds-gt683r
> new file mode 100644
> index 0000000..e4fae60
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-class-leds-gt683r
> @@ -0,0 +1,16 @@
> +What:		/sys/class/leds/<led>/gt683r/mode
> +Date:		Jun 2014
> +KernelVersion:	3.17
> +Contact:	Janne Kanniainen <janne.kanniainen@gmail.com>
> +Description:
> +		Set the mode of LEDs. You should notice that changing the mode
> +		of one LED will update the mode of its two sibling devices as
> +		well.
> +
> +		0 - normal
> +		1 - audio
> +		2 - breathing
> +
> +		Normal: LEDs are fully on when enabled
> +		Audio:  LEDs brightness depends on sound level
> +		Breathing: LEDs brightness varies at human breathing rate
> \ No newline at end of file
> diff --git a/drivers/hid/hid-gt683r.c b/drivers/hid/hid-gt683r.c
> index 073bd80..0d6f135 100644
> --- a/drivers/hid/hid-gt683r.c
> +++ b/drivers/hid/hid-gt683r.c
> @@ -84,12 +84,13 @@ static void gt683r_brightness_set(struct led_classdev *led_cdev,
>  	}
>  }
>  
> -static ssize_t leds_mode_show(struct device *dev,
> +static ssize_t mode_show(struct device *dev,
>  				struct device_attribute *attr,
>  				char *buf)
>  {
>  	u8 sysfs_mode;
> -	struct hid_device *hdev = container_of(dev, struct hid_device, dev);
> +	struct hid_device *hdev = container_of(dev->parent,
> +					struct hid_device, dev);
>  	struct gt683r_led *led = hid_get_drvdata(hdev);
>  
>  	if (led->mode == GT683R_LED_NORMAL)
> @@ -102,12 +103,13 @@ static ssize_t leds_mode_show(struct device *dev,
>  	return scnprintf(buf, PAGE_SIZE, "%u\n", sysfs_mode);
>  }
>  
> -static ssize_t leds_mode_store(struct device *dev,
> +static ssize_t mode_store(struct device *dev,
>  				struct device_attribute *attr,
>  				const char *buf, size_t count)
>  {
>  	u8 sysfs_mode;
> -	struct hid_device *hdev = container_of(dev, struct hid_device, dev);
> +	struct hid_device *hdev = container_of(dev->parent,
> +					struct hid_device, dev);
>  	struct gt683r_led *led = hid_get_drvdata(hdev);
>  
>  
> @@ -212,7 +214,22 @@ fail:
>  	mutex_unlock(&led->lock);
>  }
>  
> -static DEVICE_ATTR_RW(leds_mode);
> +static DEVICE_ATTR_RW(mode);
> +
> +static struct attribute *gt683r_led_attrs[] = {
> +	&dev_attr_mode.attr,
> +	NULL
> +};
> +
> +static const struct attribute_group gt683r_led_group = {
> +	.name = "gt683r",
> +	.attrs = gt683r_led_attrs,
> +};
> +
> +static const struct attribute_group *gt683r_led_groups[] = {
> +	&gt683r_led_group,
> +	NULL
> +};
>  
>  static int gt683r_led_probe(struct hid_device *hdev,
>  			const struct hid_device_id *id)
> @@ -261,6 +278,8 @@ static int gt683r_led_probe(struct hid_device *hdev,
>  		led->led_devs[i].name = name;
>  		led->led_devs[i].max_brightness = 1;
>  		led->led_devs[i].brightness_set = gt683r_brightness_set;
> +		led->led_devs[i].groups = gt683r_led_groups;
> +
>  		ret = led_classdev_register(&hdev->dev, &led->led_devs[i]);
>  		if (ret) {
>  			hid_err(hdev, "could not register led device\n");
> @@ -268,12 +287,6 @@ static int gt683r_led_probe(struct hid_device *hdev,
>  		}
>  	}
>  
> -	ret = device_create_file(&led->hdev->dev, &dev_attr_leds_mode);
> -	if (ret) {
> -		hid_err(hdev, "could not make mode attribute file\n");
> -		goto fail;
> -	}
> -
>  	return 0;
>  
>  fail:
> @@ -288,7 +301,6 @@ static void gt683r_led_remove(struct hid_device *hdev)
>  	int i;
>  	struct gt683r_led *led = hid_get_drvdata(hdev);
>  
> -	device_remove_file(&hdev->dev, &dev_attr_leds_mode);
>  	for (i = 0; i < GT683R_LED_COUNT; i++)
>  		led_classdev_unregister(&led->led_devs[i]);
>  	flush_work(&led->work);

^ permalink raw reply

* Re: [PATCH v2 2/6] input: touchscreen: imx25 tcq driver
From: Jonathan Cameron @ 2014-07-03 17:57 UTC (permalink / raw)
  To: Russell King - ARM Linux
  Cc: Fabio Estevam, Lars-Peter Clausen, Samuel Ortiz, Eric Bénard,
	linux-iio-u79uwXL29TY76Z2rM5mHXA, Dmitry Torokhov, Denis Carikli,
	Sascha Hauer, linux-input-u79uwXL29TY76Z2rM5mHXA, Markus Pargmann,
	Shawn Guo, Lee Jones,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
In-Reply-To: <20140629114901.GO32514-l+eeeJia6m9vn6HldHNs0ANdhmdF6hFW@public.gmane.org>

On 29/06/14 12:49, Russell King - ARM Linux wrote:
> On Sun, Jun 29, 2014 at 12:00:20PM +0100, Jonathan Cameron wrote:
>> On 21/06/14 20:55, Fabio Estevam wrote:
>>> On Sat, Jun 21, 2014 at 7:18 AM, Jonathan Cameron <jic23-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
>>>>> +
>>>>> +       res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>>>
>>>> Could this not in theory fail?
>>>
>>> Yes, but devm_ioremap_resource will take care of it, so we don't need
>>> to check for error here.
>> Nothing like counter intuitive behaviour.  Ah well.
>>
>> Thanks for explaining this!
>
> Maybe it would be better to have:
>
> 	mem = platform_ioremap_resource(pdev, IORESOURCE_MEM, 0);
>
> which wraps up both the platform_get_resource() call with
> devm_ioremap_resource() so driver writers don't have to be concerned
> about this aspect?
>
Good idea.

Something like that would certainly make more sense to me.

^ permalink raw reply

* Re: [PATCH 1/2 v6] HID: gt683r: fix race condition
From: Bryan Wu @ 2014-07-03 18:13 UTC (permalink / raw)
  To: Johan Hovold
  Cc: Janne Kanniainen, Jiri Kosina, Greg Kroah-Hartman,
	Bjørn Mork, lkml, Linux LED Subsystem, linux-usb,
	linux-input@vger.kernel.org
In-Reply-To: <20140703173421.GO16590@localhost>

On Thu, Jul 3, 2014 at 10:34 AM, Johan Hovold <johan@kernel.org> wrote:
> On Thu, Jul 03, 2014 at 08:17:08PM +0300, Janne Kanniainen wrote:
>> This will fix race condition noticed by Oliver Neukum. Sysfs files are
>> created before mutex and work are initialized.
>>
>> Signed-off-by: Janne Kanniainen <janne.kanniainen@gmail.com>
>
> Reviewed-by: Johan Hovold <johan@kernel.org>
>

Good, merged to my tree.
-Bryan

>> ---
>>  drivers/hid/hid-gt683r.c | 6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/hid/hid-gt683r.c b/drivers/hid/hid-gt683r.c
>> index 077f7a1..073bd80 100644
>> --- a/drivers/hid/hid-gt683r.c
>> +++ b/drivers/hid/hid-gt683r.c
>> @@ -227,6 +227,9 @@ static int gt683r_led_probe(struct hid_device *hdev,
>>       if (!led)
>>               return -ENOMEM;
>>
>> +     mutex_init(&led->lock);
>> +     INIT_WORK(&led->work, gt683r_led_work);
>> +
>>       led->mode = GT683R_LED_NORMAL;
>>       led->hdev = hdev;
>>       hid_set_drvdata(hdev, led);
>> @@ -271,9 +274,6 @@ static int gt683r_led_probe(struct hid_device *hdev,
>>               goto fail;
>>       }
>>
>> -     mutex_init(&led->lock);
>> -     INIT_WORK(&led->work, gt683r_led_work);
>> -
>>       return 0;
>>
>>  fail:

^ permalink raw reply

* Re: [PATCH 2/2 v6] HID: gt683r: move mode attribute to led-class devices
From: Bryan Wu @ 2014-07-03 18:17 UTC (permalink / raw)
  To: Johan Hovold
  Cc: Janne Kanniainen, Jiri Kosina, Greg Kroah-Hartman,
	Bjørn Mork, lkml, Linux LED Subsystem, linux-usb,
	linux-input@vger.kernel.org
In-Reply-To: <20140703174017.GP16590@localhost>

On Thu, Jul 3, 2014 at 10:40 AM, Johan Hovold <johan@kernel.org> wrote:
> On Thu, Jul 03, 2014 at 08:17:09PM +0300, Janne Kanniainen wrote:
>> Move led_mode attribute from HID device to led-class devices and rename
>> it msi_mode. This will also fix race condition by using
>
> There's a typo here (s/msi_mode/mode) but perhaps Bryan can just fix
> that up before applying?
>
>> attribute-groups.
>>
>> Signed-off-by: Janne Kanniainen <janne.kanniainen@gmail.com>
>
> Reviewed-by: Johan Hovold <johan@kernel.org>
>
> Otherwise both patches (v6) are ready to be merged, Bryan.
>
> Thanks, Janne!
>

No problem. I fixed the typo and merged it.

Thanks,
-Bryan



>> ---
>>
>> Changes in v3:
>>       - Style fixes
>>       - Rename sysfs-class-hid-driver-gt683r to sysfs-class-leds-driver-gt683r
>>
>> Changes in v4:
>>       - Rename sysfs-class-leds-driver-gt683r to sysfs-class-leds-gt683r
>>       - Change "What: " to /sys/class/leds/<led>/gt683r/mode
>>       - Change .name from gt683r_led to gt683r
>>
>> Changes in v5:
>>       - Move mode attribute to gt683r/mode
>>
>> Changes in v6:
>>       - Fix subject and commit message
>>
>>  .../ABI/testing/sysfs-class-hid-driver-gt683r      | 14 ---------
>>  Documentation/ABI/testing/sysfs-class-leds-gt683r  | 16 ++++++++++
>>  drivers/hid/hid-gt683r.c                           | 36 ++++++++++++++--------
>>  3 files changed, 40 insertions(+), 26 deletions(-)
>>  delete mode 100644 Documentation/ABI/testing/sysfs-class-hid-driver-gt683r
>>  create mode 100644 Documentation/ABI/testing/sysfs-class-leds-gt683r
>>
>> diff --git a/Documentation/ABI/testing/sysfs-class-hid-driver-gt683r b/Documentation/ABI/testing/sysfs-class-hid-driver-gt683r
>> deleted file mode 100644
>> index 317e9d5..0000000
>> --- a/Documentation/ABI/testing/sysfs-class-hid-driver-gt683r
>> +++ /dev/null
>> @@ -1,14 +0,0 @@
>> -What:                /sys/class/hidraw/<hidraw>/device/leds_mode
>> -Date:                Jun 2014
>> -KernelVersion:       3.17
>> -Contact:     Janne Kanniainen <janne.kanniainen@gmail.com>
>> -Description:
>> -             Set the mode of LEDs
>> -
>> -             0 - normal
>> -             1 - audio
>> -             2 - breathing
>> -
>> -             Normal: LEDs are fully on when enabled
>> -             Audio:  LEDs brightness depends on sound level
>> -             Breathing: LEDs brightness varies at human breathing rate
>> \ No newline at end of file
>> diff --git a/Documentation/ABI/testing/sysfs-class-leds-gt683r b/Documentation/ABI/testing/sysfs-class-leds-gt683r
>> new file mode 100644
>> index 0000000..e4fae60
>> --- /dev/null
>> +++ b/Documentation/ABI/testing/sysfs-class-leds-gt683r
>> @@ -0,0 +1,16 @@
>> +What:                /sys/class/leds/<led>/gt683r/mode
>> +Date:                Jun 2014
>> +KernelVersion:       3.17
>> +Contact:     Janne Kanniainen <janne.kanniainen@gmail.com>
>> +Description:
>> +             Set the mode of LEDs. You should notice that changing the mode
>> +             of one LED will update the mode of its two sibling devices as
>> +             well.
>> +
>> +             0 - normal
>> +             1 - audio
>> +             2 - breathing
>> +
>> +             Normal: LEDs are fully on when enabled
>> +             Audio:  LEDs brightness depends on sound level
>> +             Breathing: LEDs brightness varies at human breathing rate
>> \ No newline at end of file
>> diff --git a/drivers/hid/hid-gt683r.c b/drivers/hid/hid-gt683r.c
>> index 073bd80..0d6f135 100644
>> --- a/drivers/hid/hid-gt683r.c
>> +++ b/drivers/hid/hid-gt683r.c
>> @@ -84,12 +84,13 @@ static void gt683r_brightness_set(struct led_classdev *led_cdev,
>>       }
>>  }
>>
>> -static ssize_t leds_mode_show(struct device *dev,
>> +static ssize_t mode_show(struct device *dev,
>>                               struct device_attribute *attr,
>>                               char *buf)
>>  {
>>       u8 sysfs_mode;
>> -     struct hid_device *hdev = container_of(dev, struct hid_device, dev);
>> +     struct hid_device *hdev = container_of(dev->parent,
>> +                                     struct hid_device, dev);
>>       struct gt683r_led *led = hid_get_drvdata(hdev);
>>
>>       if (led->mode == GT683R_LED_NORMAL)
>> @@ -102,12 +103,13 @@ static ssize_t leds_mode_show(struct device *dev,
>>       return scnprintf(buf, PAGE_SIZE, "%u\n", sysfs_mode);
>>  }
>>
>> -static ssize_t leds_mode_store(struct device *dev,
>> +static ssize_t mode_store(struct device *dev,
>>                               struct device_attribute *attr,
>>                               const char *buf, size_t count)
>>  {
>>       u8 sysfs_mode;
>> -     struct hid_device *hdev = container_of(dev, struct hid_device, dev);
>> +     struct hid_device *hdev = container_of(dev->parent,
>> +                                     struct hid_device, dev);
>>       struct gt683r_led *led = hid_get_drvdata(hdev);
>>
>>
>> @@ -212,7 +214,22 @@ fail:
>>       mutex_unlock(&led->lock);
>>  }
>>
>> -static DEVICE_ATTR_RW(leds_mode);
>> +static DEVICE_ATTR_RW(mode);
>> +
>> +static struct attribute *gt683r_led_attrs[] = {
>> +     &dev_attr_mode.attr,
>> +     NULL
>> +};
>> +
>> +static const struct attribute_group gt683r_led_group = {
>> +     .name = "gt683r",
>> +     .attrs = gt683r_led_attrs,
>> +};
>> +
>> +static const struct attribute_group *gt683r_led_groups[] = {
>> +     &gt683r_led_group,
>> +     NULL
>> +};
>>
>>  static int gt683r_led_probe(struct hid_device *hdev,
>>                       const struct hid_device_id *id)
>> @@ -261,6 +278,8 @@ static int gt683r_led_probe(struct hid_device *hdev,
>>               led->led_devs[i].name = name;
>>               led->led_devs[i].max_brightness = 1;
>>               led->led_devs[i].brightness_set = gt683r_brightness_set;
>> +             led->led_devs[i].groups = gt683r_led_groups;
>> +
>>               ret = led_classdev_register(&hdev->dev, &led->led_devs[i]);
>>               if (ret) {
>>                       hid_err(hdev, "could not register led device\n");
>> @@ -268,12 +287,6 @@ static int gt683r_led_probe(struct hid_device *hdev,
>>               }
>>       }
>>
>> -     ret = device_create_file(&led->hdev->dev, &dev_attr_leds_mode);
>> -     if (ret) {
>> -             hid_err(hdev, "could not make mode attribute file\n");
>> -             goto fail;
>> -     }
>> -
>>       return 0;
>>
>>  fail:
>> @@ -288,7 +301,6 @@ static void gt683r_led_remove(struct hid_device *hdev)
>>       int i;
>>       struct gt683r_led *led = hid_get_drvdata(hdev);
>>
>> -     device_remove_file(&hdev->dev, &dev_attr_leds_mode);
>>       for (i = 0; i < GT683R_LED_COUNT; i++)
>>               led_classdev_unregister(&led->led_devs[i]);
>>       flush_work(&led->work);

^ permalink raw reply

* Re: [PATCH 2/2 v6] HID: gt683r: move mode attribute to led-class devices
From: Janne Kanniainen @ 2014-07-03 18:28 UTC (permalink / raw)
  To: Bryan Wu
  Cc: Johan Hovold, Jiri Kosina, Greg Kroah-Hartman, Bjørn Mork,
	lkml, Linux LED Subsystem, linux-usb, linux-input@vger.kernel.org
In-Reply-To: <CAK5ve-LBU72Z_dnV9XUyMTVgFF2C1xRhKNhC_cziO5GMx__zhA@mail.gmail.com>

2014-07-03 21:17 GMT+03:00 Bryan Wu <cooloney@gmail.com>:
> On Thu, Jul 3, 2014 at 10:40 AM, Johan Hovold <johan@kernel.org> wrote:
>> On Thu, Jul 03, 2014 at 08:17:09PM +0300, Janne Kanniainen wrote:
>>> Move led_mode attribute from HID device to led-class devices and rename
>>> it msi_mode. This will also fix race condition by using
>>
>> There's a typo here (s/msi_mode/mode) but perhaps Bryan can just fix
>> that up before applying?
>>
>>> attribute-groups.
>>>
>>> Signed-off-by: Janne Kanniainen <janne.kanniainen@gmail.com>
>>
>> Reviewed-by: Johan Hovold <johan@kernel.org>
>>
>> Otherwise both patches (v6) are ready to be merged, Bryan.
>>
>> Thanks, Janne!
>>
>
> No problem. I fixed the typo and merged it.
>
> Thanks,
> -Bryan

Thanks again to both of you!

Janne

^ permalink raw reply

* Re: [PATCH v3] Input: evdev - add event-mask API
From: David Herrmann @ 2014-07-04  5:16 UTC (permalink / raw)
  To: open list:HID CORE LAYER, Dmitry Torokhov; +Cc: Peter Hutterer, David Herrmann
In-Reply-To: <1403070158-31489-1-git-send-email-dh.herrmann@gmail.com>

Hi Dmitry

On Wed, Jun 18, 2014 at 7:42 AM, David Herrmann <dh.herrmann@gmail.com> wrote:
> Hardware manufacturers group keys in the weirdest way possible. This may
> cause a power-key to be grouped together with normal keyboard keys and
> thus be reported on the same kernel interface.
>
> However, user-space is often only interested in specific sets of events.
> For instance, daemons dealing with system-reboot (like systemd-logind)
> listen for KEY_POWER, but are not interested in any main keyboard keys.
> Usually, power keys are reported via separate interfaces, however,
> some i8042 boards report it in the AT matrix. To avoid waking up those
> system daemons on each key-press, we had two ideas:
>  - split off KEY_POWER into a separate interface unconditionally
>  - allow filtering a specific set of events on evdev FDs
>
> Splitting of KEY_POWER is a rather weird way to deal with this and may
> break backwards-compatibility. It is also specific to KEY_POWER and might
> be required for other stuff, too. Moreover, we might end up with a huge
> set of input-devices just to have them properly split.
>
> Hence, this patchset implements the second idea: An event-mask to specify
> which events you're interested in. Two ioctls allow setting this mask for
> each event-type. If not set, all events are reported. The type==0 entry is
> used same as in EVIOCGBIT to set the actual EV_* mask of filtered events.
> This way, you have a two-level filter.
>
> We are heavily forward-compatible to new event-types and event-codes. So
> new user-space will be able to run on an old kernel which doesn't know the
> given event-codes or event-types.
>
> Acked-by: Peter Hutterer <peter.hutterer@who-t.net>
> Signed-off-by: David Herrmann <dh.herrmann@gmail.com>

Dmitry, any further comments on this one? We really want this for
proper power/suspend-key handling.

Thanks
David

> ---
> v3:
>  - rewording (Peter)
>
>  drivers/input/evdev.c      | 156 ++++++++++++++++++++++++++++++++++++++++++++-
>  include/uapi/linux/input.h |  56 ++++++++++++++++
>  2 files changed, 210 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/input/evdev.c b/drivers/input/evdev.c
> index fd325ec..6386882 100644
> --- a/drivers/input/evdev.c
> +++ b/drivers/input/evdev.c
> @@ -51,10 +51,130 @@ struct evdev_client {
>         struct list_head node;
>         int clkid;
>         bool revoked;
> +       unsigned long *evmasks[EV_CNT];
>         unsigned int bufsize;
>         struct input_event buffer[];
>  };
>
> +static size_t evdev_get_mask_cnt(unsigned int type)
> +{
> +       static size_t counts[EV_CNT] = {
> +               /* EV_SYN==0 is EV_CNT, _not_ SYN_CNT, see EVIOCGBIT */
> +               [EV_SYN] = EV_CNT,
> +               [EV_KEY] = KEY_CNT,
> +               [EV_REL] = REL_CNT,
> +               [EV_ABS] = ABS_CNT,
> +               [EV_MSC] = MSC_CNT,
> +               [EV_SW] = SW_CNT,
> +               [EV_LED] = LED_CNT,
> +               [EV_SND] = SND_CNT,
> +               [EV_FF] = FF_CNT,
> +       };
> +
> +       return (type < EV_CNT) ? counts[type] : 0;
> +}
> +
> +/* must be called with evdev-mutex held */
> +static int evdev_set_mask(struct evdev_client *client,
> +                         unsigned int type,
> +                         const void __user *codes,
> +                         u32 codes_size)
> +{
> +       unsigned long flags, *mask, *oldmask;
> +       size_t cnt, size;
> +
> +       /* unknown masks are simply ignored for forward-compat */
> +       cnt = evdev_get_mask_cnt(type);
> +       if (!cnt)
> +               return 0;
> +
> +       /* we allow 'codes_size > size' for forward-compat */
> +       size = sizeof(unsigned long) * BITS_TO_LONGS(cnt);
> +
> +       mask = kzalloc(size, GFP_KERNEL);
> +       if (!mask)
> +               return -ENOMEM;
> +
> +       if (copy_from_user(mask, codes, min_t(size_t, codes_size, size))) {
> +               kfree(mask);
> +               return -EFAULT;
> +       }
> +
> +       spin_lock_irqsave(&client->buffer_lock, flags);
> +       oldmask = client->evmasks[type];
> +       client->evmasks[type] = mask;
> +       spin_unlock_irqrestore(&client->buffer_lock, flags);
> +
> +       kfree(oldmask);
> +
> +       return 0;
> +}
> +
> +/* must be called with evdev-mutex held */
> +static int evdev_get_mask(struct evdev_client *client,
> +                         unsigned int type,
> +                         void __user *codes,
> +                         u32 codes_size)
> +{
> +       unsigned long *mask;
> +       size_t cnt, size, min, i;
> +       u8 __user *out;
> +
> +       /* we allow unknown types and 'codes_size > size' for forward-compat */
> +       cnt = evdev_get_mask_cnt(type);
> +       size = sizeof(unsigned long) * BITS_TO_LONGS(cnt);
> +       min = min_t(size_t, codes_size, size);
> +
> +       if (cnt > 0) {
> +               mask = client->evmasks[type];
> +               if (mask) {
> +                       if (copy_to_user(codes, mask, min))
> +                               return -EFAULT;
> +               } else {
> +                       /* fake mask with all bits set */
> +                       out = (u8 __user*)codes;
> +                       for (i = 0; i < min; ++i) {
> +                               if (put_user((u8)0xff,  out + i))
> +                                       return -EFAULT;
> +                       }
> +               }
> +       }
> +
> +       codes = (u8*)codes + min;
> +       codes_size -= min;
> +
> +       if (codes_size > 0 && clear_user(codes, codes_size))
> +               return -EFAULT;
> +
> +       return 0;
> +}
> +
> +/* requires the buffer lock to be held */
> +static bool __evdev_is_filtered(struct evdev_client *client,
> +                               unsigned int type,
> +                               unsigned int code)
> +{
> +       unsigned long *mask;
> +       size_t cnt;
> +
> +       /* EV_SYN and unknown codes are never filtered */
> +       if (type == EV_SYN || type >= EV_CNT)
> +               return false;
> +
> +       /* first test whether the type is filtered */
> +       mask = client->evmasks[0];
> +       if (mask && !test_bit(type, mask))
> +               return true;
> +
> +       /* unknown values are never filtered */
> +       cnt = evdev_get_mask_cnt(type);
> +       if (!cnt || code >= cnt)
> +               return false;
> +
> +       mask = client->evmasks[type];
> +       return mask && !test_bit(code, mask);
> +}
> +
>  /* flush queued events of type @type, caller must hold client->buffer_lock */
>  static void __evdev_flush_queue(struct evdev_client *client, unsigned int type)
>  {
> @@ -177,12 +297,21 @@ static void evdev_pass_values(struct evdev_client *client,
>         spin_lock(&client->buffer_lock);
>
>         for (v = vals; v != vals + count; v++) {
> +               if (__evdev_is_filtered(client, v->type, v->code))
> +                       continue;
> +
> +               if (v->type == EV_SYN && v->code == SYN_REPORT) {
> +                       /* drop empty SYN_REPORT */
> +                       if (client->packet_head == client->head)
> +                               continue;
> +
> +                       wakeup = true;
> +               }
> +
>                 event.type = v->type;
>                 event.code = v->code;
>                 event.value = v->value;
>                 __pass_event(client, &event);
> -               if (v->type == EV_SYN && v->code == SYN_REPORT)
> -                       wakeup = true;
>         }
>
>         spin_unlock(&client->buffer_lock);
> @@ -365,6 +494,7 @@ static int evdev_release(struct inode *inode, struct file *file)
>  {
>         struct evdev_client *client = file->private_data;
>         struct evdev *evdev = client->evdev;
> +       unsigned int i;
>
>         mutex_lock(&evdev->mutex);
>         evdev_ungrab(evdev, client);
> @@ -372,6 +502,9 @@ static int evdev_release(struct inode *inode, struct file *file)
>
>         evdev_detach_client(evdev, client);
>
> +       for (i = 0; i < EV_CNT; ++i)
> +               kfree(client->evmasks[i]);
> +
>         if (is_vmalloc_addr(client))
>                 vfree(client);
>         else
> @@ -811,6 +944,7 @@ static long evdev_do_ioctl(struct file *file, unsigned int cmd,
>         struct evdev *evdev = client->evdev;
>         struct input_dev *dev = evdev->handle.dev;
>         struct input_absinfo abs;
> +       struct input_mask mask;
>         struct ff_effect effect;
>         int __user *ip = (int __user *)p;
>         unsigned int i, t, u, v;
> @@ -872,6 +1006,24 @@ static long evdev_do_ioctl(struct file *file, unsigned int cmd,
>                 else
>                         return evdev_revoke(evdev, client, file);
>
> +       case EVIOCGMASK:
> +               if (copy_from_user(&mask, p, sizeof(mask)))
> +                       return -EFAULT;
> +
> +               return evdev_get_mask(client,
> +                                     mask.type,
> +                                     (void*)(long)mask.codes_ptr,
> +                                     mask.codes_size);
> +
> +       case EVIOCSMASK:
> +               if (copy_from_user(&mask, p, sizeof(mask)))
> +                       return -EFAULT;
> +
> +               return evdev_set_mask(client,
> +                                     mask.type,
> +                                     (const void*)(long)mask.codes_ptr,
> +                                     mask.codes_size);
> +
>         case EVIOCSCLOCKID:
>                 if (copy_from_user(&i, p, sizeof(unsigned int)))
>                         return -EFAULT;
> diff --git a/include/uapi/linux/input.h b/include/uapi/linux/input.h
> index 19df18c..f6ace0e 100644
> --- a/include/uapi/linux/input.h
> +++ b/include/uapi/linux/input.h
> @@ -97,6 +97,12 @@ struct input_keymap_entry {
>         __u8  scancode[32];
>  };
>
> +struct input_mask {
> +       __u32 type;
> +       __u32 codes_size;
> +       __u64 codes_ptr;
> +};
> +
>  #define EVIOCGVERSION          _IOR('E', 0x01, int)                    /* get driver version */
>  #define EVIOCGID               _IOR('E', 0x02, struct input_id)        /* get device ID */
>  #define EVIOCGREP              _IOR('E', 0x03, unsigned int[2])        /* get repeat settings */
> @@ -154,6 +160,56 @@ struct input_keymap_entry {
>  #define EVIOCGRAB              _IOW('E', 0x90, int)                    /* Grab/Release device */
>  #define EVIOCREVOKE            _IOW('E', 0x91, int)                    /* Revoke device access */
>
> +/**
> + * EVIOCGMASK - Retrieve current event-mask
> + *
> + * This retrieves the current event-mask for a specific event-type. The
> + * argument must be of type "struct input_mask" and specifies the event-type to
> + * query, the receive buffer and the size of the receive buffer.
> + *
> + * The event-mask is a per-client mask that specifies which events are forwarded
> + * to the client. Each event-code is represented by a single bit in the
> + * event-mask. If the bit is set, the event is passed to the client normally.
> + * Otherwise, the event is filtered and and will never be queued on the
> + * client's receive buffer.
> + * Event-masks do not affect global state of an input-device. They only affect
> + * the open-file they're applied on. Each open-file (i.e, file-description) can
> + * have a different event-mask.
> + *
> + * The default event-mask for a client has all bits set, i.e. all events are
> + * forwarded to the client. If a kernel is queried for an unknown event-type
> + * or if the receive buffer is larger than the number of event-codes known to
> + * the kernel, the kernel returns all zeroes for those codes.
> + *
> + * At maximum, codes_size bytes are copied.
> + *
> + * This ioctl may fail with ENODEV in case the file is revoked, EFAULT
> + * if the receive-buffer points to invalid memory, or EINVAL if the kernel
> + * does not implement the ioctl.
> + */
> +#define EVIOCGMASK             _IOR('E', 0x92, struct input_mask)      /* Get event-masks */
> +
> +/**
> + * EVIOCSMASK - Set event-mask
> + *
> + * This is the counterpart to EVIOCGMASK. Instead of receiving the current
> + * event-mask, this changes the client's event-mask for a specific type. See
> + * EVIOCGMASK for a description of event-masks and the argument-type.
> + *
> + * This ioctl provides full forward-compatibility. If the passed event-type is
> + * unknown to the kernel, or if the number of codes is bigger than known to the
> + * kernel, the ioctl is still accepted and applied. However, any unknown codes
> + * are left untouched and stay cleared. That means, the kernel always filters
> + * unknown codes regardless of what the client requests.
> + * If the new mask doesn't cover all known event-codes, all remaining codes are
> + * automatically cleared and thus filtered.
> + *
> + * This ioctl may fail with ENODEV in case the file is revoked. EFAULT is
> + * returned if the receive-buffer points to invalid memory. EINVAL is returned
> + * if the kernel does not implement the ioctl.
> + */
> +#define EVIOCSMASK             _IOW('E', 0x93, struct input_mask)      /* Set event-masks */
> +
>  #define EVIOCSCLOCKID          _IOW('E', 0xa0, int)                    /* Set clockid to be used for timestamps */
>
>  /*
> --
> 2.0.0
>

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox