[PATCH v3] Input: Elan HID-I2C device driver

[PATCH v3] Input: Elan HID-I2C device driver

[Tom Lin]

This patch adds driver for Elan I2C touchpad. These protocol of HID-I2C was
defined by Microsoft. The kernel driver would use the multi-touch protocol
format B.

Signed-off-by:Tom Lin(Lin Yen Yu) <tom_lin@emc.com.tw>
---
 drivers/input/mouse/Kconfig        |    8 +
 drivers/input/mouse/Makefile       |    1 +
 drivers/input/mouse/elantech_i2c.c |  504 ++++++++++++++++++++++++++++++++++++
 3 files changed, 513 insertions(+)
 create mode 100644 drivers/input/mouse/elantech_i2c.c

diff --git a/drivers/input/mouse/Kconfig b/drivers/input/mouse/Kconfig
index 9b8db82..345cf8c 100644
--- a/drivers/input/mouse/Kconfig
+++ b/drivers/input/mouse/Kconfig
@@ -339,4 +339,12 @@ config MOUSE_SYNAPTICS_USB
 	  To compile this driver as a module, choose M here: the
 	  module will be called synaptics_usb.
 
+config MOUSE_ELANTECH_I2C
+	tristate "Elan I2C Touchpad support"
+	depends on I2C
+	help
+	 Say Y here if you want to use a Elan HID-I2C touchpad.
+
+	 To compile this driver as a module, choose M here: the
+	 module will be called elantech_i2c.
 endif
diff --git a/drivers/input/mouse/Makefile b/drivers/input/mouse/Makefile
index 4718eff..6f092a3 100644
--- a/drivers/input/mouse/Makefile
+++ b/drivers/input/mouse/Makefile
@@ -20,6 +20,7 @@ obj-$(CONFIG_MOUSE_SERIAL)		+= sermouse.o
 obj-$(CONFIG_MOUSE_SYNAPTICS_I2C)	+= synaptics_i2c.o
 obj-$(CONFIG_MOUSE_SYNAPTICS_USB)	+= synaptics_usb.o
 obj-$(CONFIG_MOUSE_VSXXXAA)		+= vsxxxaa.o
+obj-$(CONFIG_MOUSE_ELANTECH_I2C)	+= elantech_i2c.o
 
 psmouse-objs := psmouse-base.o synaptics.o
 
diff --git a/drivers/input/mouse/elantech_i2c.c b/drivers/input/mouse/elantech_i2c.c
new file mode 100644
index 0000000..3e4b5d4
--- /dev/null
+++ b/drivers/input/mouse/elantech_i2c.c
@@ -0,0 +1,504 @@
+/*
+ * Elantech I2C Touchpad diver
+ *
+ * Copyright (c) 2011-2012 Tom Lin (Lin Yen Yu) <tom_lin@emc.com.tw>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published
+ * by the Free Software Foundation.
+ *
+ * Trademarks are the property of their respective owners.
+ */
+
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/gpio.h>
+#include <linux/i2c.h>
+#include <linux/init.h>
+#include <linux/input.h>
+#include <linux/input/mt.h>
+#include <linux/interrupt.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/workqueue.h>
+
+#define DRIVER_NAME		"elantech_i2c"
+#define ETP_FINGER_DATA_OFFSET	4
+#define ETP_FINGER_DATA_LEN	5
+#define ETP_I2C_COMMAND_TRIES	3
+#define ETP_I2C_COMMAND_DELAY	500
+/*length of Elan Touchpad information*/
+#define ETP_INF_LENGTH		2
+#define ETP_MAX_FINGERS		5
+#define ETP_PACKET_LENGTH	30
+#define ETP_REPORT_ID		0X5d
+#define HID_DES_LENGTH_OFFSET	1
+
+static bool elan_i2c_debug;
+module_param_named(debug, elan_i2c_debug, bool, 0600);
+MODULE_PARM_DESC(debug, "Turn Elan i2c TP  debugging mode on and off");
+
+#define elantech_debug(fmt, ...)				\
+	do {							\
+		if (elan_i2c_debug)				\
+			printk(KERN_DEBUG KBUILD_MODNAME	\
+					fmt, ##__VA_ARGS__);	\
+	} while (0)
+
+enum etd_i2c_command {
+	ETP_HID_DESCR_LENGTH_CMD,
+	ETP_HID_DESCR_CMD,
+	ETP_HID_WAKE_UP_CMD,
+	ETP_HID_RESET_CMD,
+	ETP_HID_REPORT_DESCR_CMD,
+	ETP_TRACE_NUM_CMD,
+	ETP_X_AXIS_MAX_CMD,
+	ETP_Y_AXIS_MAX_CMD,
+	ETP_DPI_VALUE_CMD,
+	ETP_ENABLE_ABS_CMD
+};
+
+static const u8 hid_descriptor[] = {0x01, 0x00};
+static const u8 hid_wake_up[] = {0x05, 0x00, 0x00, 0x08};
+static const u8 hid_reset_cmd[] = {0x05, 0x00, 0x00, 0x01};
+static const u8 hid_report_descriptor[] = {0x02, 0x00};
+static const u8 trace_num_xy[] = {0x05, 0x01};
+static const u8 x_axis_max[] = {0x06, 0x01};
+static const u8 y_axis_max[] = {0x07, 0x01};
+static const u8 dpi_value[] = {0x08, 0x01};
+static const u8 enable_abs[] = {0x00, 0x03, 0x01, 0x00};
+
+/* The main device structure */
+struct elantech_i2c {
+	struct i2c_client	*client;
+	struct input_dev	*input;
+	struct work_struct	work;
+	unsigned int		gpio;
+	unsigned int		max_x;
+	unsigned int		max_y;
+	unsigned int		width_x;
+	unsigned int		width_y;
+	int			irq;
+	u8			pre_recv[28];
+};
+
+static int elantech_i2c_command(struct i2c_client *client, int  command,
+				unsigned char *buf_recv, int data_len)
+{
+	const u8 *cmd = NULL;
+	unsigned char *rec_buf = buf_recv;
+	int ret;
+	int tries = ETP_I2C_COMMAND_TRIES;
+	int length = 0;
+	struct i2c_msg msg[2];
+	int msg_num = 0;
+
+	switch (command) {
+	case ETP_HID_DESCR_LENGTH_CMD:
+	case ETP_HID_DESCR_CMD:
+		cmd = hid_descriptor;
+		length = sizeof(hid_descriptor);
+		msg[1].len = data_len;
+		msg_num = 2;
+		break;
+	case ETP_HID_WAKE_UP_CMD:
+		cmd = hid_wake_up;
+		length = sizeof(hid_wake_up);
+		msg_num = 1;
+		break;
+	case ETP_HID_RESET_CMD:
+		cmd = hid_reset_cmd;
+		length = sizeof(hid_reset_cmd);
+		msg_num = 1;
+		break;
+	case ETP_HID_REPORT_DESCR_CMD:
+		cmd = hid_report_descriptor;
+		length = sizeof(hid_report_descriptor);
+		msg[1].len = data_len;
+		msg_num = 2;
+		break;
+	case ETP_TRACE_NUM_CMD:
+		cmd = trace_num_xy;
+		length = sizeof(trace_num_xy);
+		msg[1].len = data_len;
+		msg_num = 2;
+		break;
+	case ETP_X_AXIS_MAX_CMD:
+		cmd = x_axis_max;
+		length = sizeof(x_axis_max);
+		msg[1].len = data_len;
+		msg_num = 2;
+		break;
+	case ETP_Y_AXIS_MAX_CMD:
+		cmd = y_axis_max;
+		length = sizeof(y_axis_max);
+		msg[1].len = data_len;
+		msg_num = 2;
+		break;
+	case ETP_DPI_VALUE_CMD:
+		cmd = dpi_value;
+		length = sizeof(dpi_value);
+		msg[1].len = data_len;
+		msg_num = 2;
+		break;
+	case ETP_ENABLE_ABS_CMD:
+		cmd = enable_abs;
+		length = sizeof(enable_abs);
+		msg_num = 1;
+		break;
+	default:
+		elantech_debug("%s command=%d unknow.\n",
+			 __func__, command);
+		return -1;
+	}
+
+	msg[0].addr = client->addr;
+	msg[0].flags = client->flags & I2C_M_TEN;
+	msg[0].len = length;
+	msg[0].buf = (char *) cmd;
+	msg[1].addr = client->addr;
+	msg[1].flags = client->flags & I2C_M_TEN;
+	msg[1].flags |= I2C_M_RD;
+	msg[1].buf = rec_buf;
+
+	do {
+		ret = i2c_transfer(client->adapter, msg, msg_num);
+		if (ret != msg_num && elan_i2c_debug)
+			print_hex_dump_bytes("Elan I2C Touch data :",
+				DUMP_PREFIX_NONE, rec_buf, msg[1].len);
+		if (ret > 0)
+			break;
+		tries--;
+		elantech_debug("retrying elantech_i2c_command:%d (%d)\n",
+			command, tries);
+	} while (tries > 0);
+
+	return ret;
+}
+
+static bool elantech_i2c_hw_init(struct i2c_client *client)
+{
+	int rc;
+	uint8_t buf_recv[79];
+	int hid_descr_len;
+	int hid_report_len;
+
+	/*Fetch the length of HID description*/
+	rc = elantech_i2c_command(client, ETP_HID_DESCR_LENGTH_CMD, buf_recv,
+			HID_DES_LENGTH_OFFSET);
+	if (rc != 2)
+		return false;
+	else
+		hid_descr_len = buf_recv[0];
+
+	/*Fetch the lenght of HID report description*/
+	rc = elantech_i2c_command(client, ETP_HID_DESCR_CMD, buf_recv, hid_descr_len);
+	if (rc != 2)
+		return false;
+	else
+		hid_report_len = buf_recv[4];
+
+	rc = elantech_i2c_command(client, ETP_HID_WAKE_UP_CMD, buf_recv, 0);
+	if (rc != 1)
+		return false;
+
+	rc = elantech_i2c_command(client, ETP_HID_RESET_CMD, buf_recv, 0);
+	if (rc != 1)
+		return false;
+
+	msleep(ETP_I2C_COMMAND_DELAY);
+	rc = i2c_master_recv(client, buf_recv, 2);
+	if (rc != 2 || (buf_recv[0] != 0 && buf_recv[1] != 0))
+		return false;
+
+	rc = elantech_i2c_command(client, ETP_HID_REPORT_DESCR_CMD, buf_recv,
+		hid_report_len);
+	if (rc != 2)
+		return false;
+
+	return true;
+}
+
+static void elantech_i2c_report_absolute(struct elantech_i2c *touch,
+				 uint8_t *buf)
+{
+	unsigned char *packet = buf;
+	unsigned char *finger_data;
+	struct input_dev *input = touch->input;
+	int finger_status[5];
+	int i;
+	short dyn_data_i;
+	int position_x, position_y;
+	int mky_value, mkx_value, h_value;
+
+	dyn_data_i = 0;
+	for (i = 0 ; i < ETP_MAX_FINGERS ; i++) {
+		finger_status[i] = (packet[3] >> (3+i)) & 0x01;
+		if (finger_status[i]) {
+			finger_data = &packet[ETP_FINGER_DATA_OFFSET +
+				(dyn_data_i * ETP_FINGER_DATA_LEN)];
+			position_x = ((finger_data[0] & 0xf0) << 4) | finger_data[1];
+			position_y = touch->max_y -
+				(((finger_data[0] & 0x0f) << 8) | finger_data[2]);
+			mkx_value = (finger_data[3] & 0x0f) * touch->width_x;
+			mky_value = (finger_data[3] >> 4) * touch->width_y;
+			h_value = finger_data[4];
+			dyn_data_i++;
+
+			input_mt_slot(input, i);
+			input_mt_report_slot_state(input, MT_TOOL_FINGER, true);
+
+			input_report_abs(input, ABS_MT_POSITION_X, position_x);
+			input_report_abs(input, ABS_MT_POSITION_Y, position_y);
+			input_report_abs(input, ABS_MT_PRESSURE, h_value);
+			input_report_abs(input, ABS_MT_TOUCH_MAJOR, mkx_value);
+			input_report_abs(input, ABS_MT_TOUCH_MINOR, mky_value);
+		} else if (finger_status[i] == 0) {
+			input_mt_slot(input, i);
+			input_mt_report_slot_state(input, MT_TOOL_FINGER, false);
+		}
+	}
+	input_report_key(input, BTN_LEFT, ((packet[3] & 0x01) == 1));
+	input_report_key(input, BTN_RIGHT, ((packet[3] & 0x02) == 2));
+	input_report_key(input, BTN_MIDDLE, ((packet[3] & 0x04) == 4));
+	input_mt_report_pointer_emulation(input, true);
+	input_sync(input);
+}
+
+static irqreturn_t elantech_i2c_isr(int irq, void *dev_id)
+{
+	struct elantech_i2c *elantech_i2c_priv = dev_id;
+	unsigned char buf_recv[30] = {0};
+	int rc;
+	int length;
+
+	rc = i2c_master_recv(elantech_i2c_priv->client, buf_recv, ETP_PACKET_LENGTH);
+	if (rc == ETP_PACKET_LENGTH) {
+		length = (buf_recv[1] << 8) | buf_recv[0];
+		if (!memcmp(elantech_i2c_priv->pre_recv, buf_recv, length))
+			goto elantech_isr_end;
+
+		memcpy(elantech_i2c_priv->pre_recv, buf_recv, length);
+		/*Packet check*/
+		if (buf_recv[2] == ETP_REPORT_ID && buf_recv[length - 1] == 0x00)
+			elantech_i2c_report_absolute(elantech_i2c_priv, buf_recv);
+		else if (elan_i2c_debug)
+			print_hex_dump_bytes("Elan I2C Touch data :",
+				DUMP_PREFIX_NONE, buf_recv, length);
+	}
+
+elantech_isr_end:
+	return IRQ_HANDLED;
+}
+
+static struct elantech_i2c *elantech_i2c_priv_create(struct i2c_client *client)
+{
+	struct elantech_i2c *elantech_i2c_priv;
+	struct input_dev *dev;
+	unsigned int gpio;
+	const char *label = "elantech_i2c";
+	int rc = 0;
+	int Pressure_value_data;
+	unsigned char buf_recv[2];
+
+	elantech_i2c_priv = kzalloc(sizeof(struct elantech_i2c), GFP_KERNEL);
+	if (!elantech_i2c_priv)
+		return NULL;
+
+	elantech_i2c_priv->client = client;
+	elantech_i2c_priv->irq = client->irq;
+	elantech_i2c_priv->input = input_allocate_device();
+	if (elantech_i2c_priv->input == NULL)
+		goto err_input_allocate_device;
+
+	elantech_i2c_priv->input->name = "ELANTECH_I2C";
+	elantech_i2c_priv->input->phys = client->adapter->name;
+	elantech_i2c_priv->input->id.bustype = BUS_I2C;
+
+	dev = elantech_i2c_priv->input;
+	__set_bit(EV_KEY, dev->evbit);
+	__set_bit(EV_ABS, dev->evbit);
+
+	__set_bit(BTN_LEFT, dev->keybit);
+	__set_bit(BTN_RIGHT, dev->keybit);
+	__set_bit(BTN_MIDDLE, dev->keybit);
+
+	__set_bit(BTN_TOUCH, dev->keybit);
+	__set_bit(BTN_TOOL_FINGER, dev->keybit);
+	__set_bit(BTN_TOOL_DOUBLETAP, dev->keybit);
+	__set_bit(BTN_TOOL_TRIPLETAP, dev->keybit);
+	__set_bit(BTN_TOOL_QUADTAP, dev->keybit);
+
+	elantech_i2c_command(client, ETP_X_AXIS_MAX_CMD, buf_recv, ETP_INF_LENGTH);
+	elantech_i2c_priv->max_x = (0x0f & buf_recv[1]) << 8 | buf_recv[0];
+	elantech_i2c_command(client, ETP_Y_AXIS_MAX_CMD, buf_recv, ETP_INF_LENGTH);
+	elantech_i2c_priv->max_y = (0x0f & buf_recv[1]) << 8 | buf_recv[0];
+	printk(KERN_INFO "%s max_x = %d\n", __func__, elantech_i2c_priv->max_x);
+	printk(KERN_INFO "%s max_y = %d\n", __func__, elantech_i2c_priv->max_y);
+	elantech_i2c_command(client, ETP_TRACE_NUM_CMD, buf_recv, ETP_INF_LENGTH);
+	elantech_i2c_priv->width_x = elantech_i2c_priv->max_x / (buf_recv[0] - 1);
+	elantech_i2c_priv->width_y = elantech_i2c_priv->max_y / (buf_recv[1] - 1);
+	Pressure_value_data = 0xff;
+
+	/* X Y Value*/
+	input_set_abs_params(dev, ABS_X, 0, elantech_i2c_priv->max_x, 0, 0);
+	input_set_abs_params(dev, ABS_Y, 0, elantech_i2c_priv->max_y, 0, 0);
+	/* Palme Value */
+	input_set_abs_params(dev, ABS_PRESSURE, 0, 255, 0, 0);
+
+	/* Finger Pressure value */
+	input_set_abs_params(dev, ABS_TOOL_WIDTH, 0, Pressure_value_data, 0, 0);
+
+	/* Finger Pressure value */
+	input_mt_init_slots(dev, ETP_MAX_FINGERS);
+	input_set_abs_params(dev, ABS_MT_POSITION_X, 0, elantech_i2c_priv->max_x, 0, 0);
+	input_set_abs_params(dev, ABS_MT_POSITION_Y, 0, elantech_i2c_priv->max_y, 0, 0);
+	input_set_abs_params(dev, ABS_MT_PRESSURE, 0, Pressure_value_data, 0, 0);
+	input_set_abs_params(dev, ABS_MT_TOUCH_MAJOR, 0, Pressure_value_data, 0, 0);
+	input_set_abs_params(dev, ABS_MT_TOUCH_MINOR, 0, Pressure_value_data, 0, 0);
+
+	/* Enable ABS mode*/
+	elantech_i2c_command(client, ETP_ENABLE_ABS_CMD, buf_recv, ETP_INF_LENGTH);
+	gpio = irq_to_gpio(client->irq);
+	rc = gpio_request(gpio, label);
+	if (rc < 0) {
+		dev_dbg(&client->dev, "gpio_request failed for input %d rc = %d\n", gpio, rc);
+		goto	err_gpio_request;
+	}
+	rc = gpio_direction_input(gpio);
+	if (rc < 0) {
+		dev_dbg(&client->dev, "gpio_direction_input failed for input %d\n", gpio);
+		goto	err_gpio_request;
+	}
+	elantech_i2c_priv->gpio = gpio;
+	rc = request_threaded_irq(client->irq, NULL, elantech_i2c_isr,
+			IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
+			client->name, elantech_i2c_priv);
+	 if (rc < 0) {
+		dev_dbg(&client->dev, "Could not register for %s interrupt, irq = %d, rc = %d\n",
+		label, client->irq, rc);
+		goto	err_gpio_request_irq_fail;
+	}
+
+	return elantech_i2c_priv;
+
+err_gpio_request_irq_fail:
+	gpio_free(gpio);
+err_gpio_request:
+	input_free_device(elantech_i2c_priv->input);
+err_input_allocate_device:
+	kfree(elantech_i2c_priv);
+	return NULL;
+}
+
+static int elantech_i2c_probe(struct i2c_client *client,
+			      const struct i2c_device_id *dev_id)
+
+{
+	int ret = 0;
+	struct elantech_i2c *elantech_i2c_priv;
+	unsigned int gpio;
+
+	if (!elantech_i2c_hw_init(client)) {
+		ret = -EINVAL;
+		goto err_hw_init;
+	}
+	elantech_i2c_priv = elantech_i2c_priv_create(client);
+	if (!elantech_i2c_priv) {
+		ret = -EINVAL;
+		goto err_elantech_i2c_priv;
+	}
+
+	ret = input_register_device(elantech_i2c_priv->input);
+	if (ret < 0)
+		goto err_input_register_device;
+
+	i2c_set_clientdata(client, elantech_i2c_priv);
+	device_init_wakeup(&client->dev, 1);
+
+	return 0;
+
+err_input_register_device:
+	gpio = elantech_i2c_priv->gpio;
+	gpio_free(gpio);
+	free_irq(elantech_i2c_priv->irq, elantech_i2c_priv);
+	input_free_device(elantech_i2c_priv->input);
+	kfree(elantech_i2c_priv);
+err_elantech_i2c_priv:
+err_hw_init:
+	return ret;
+}
+
+static int elantech_i2c_remove(struct i2c_client *client)
+{
+	struct elantech_i2c *elantech_i2c_priv = i2c_get_clientdata(client);
+
+	if (elantech_i2c_priv) {
+		if (elantech_i2c_priv->gpio > 0)
+			gpio_free(elantech_i2c_priv->gpio);
+		free_irq(elantech_i2c_priv->irq, elantech_i2c_priv);
+		input_unregister_device(elantech_i2c_priv->input);
+		kfree(elantech_i2c_priv);
+	}
+
+	return 0;
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int elantech_i2c_suspend(struct device *dev)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+
+	if (device_may_wakeup(&client->dev))
+		enable_irq_wake(client->irq);
+
+	return 0;
+}
+
+static int elantech_i2c_resume(struct device *dev)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+
+	if (device_may_wakeup(&client->dev))
+		enable_irq_wake(client->irq);
+
+	return 0;
+}
+#endif
+
+static SIMPLE_DEV_PM_OPS(elan_i2c_touchpad_pm_ops,
+		elantech_i2c_suspend, elantech_i2c_resume);
+
+static const struct i2c_device_id elantech_i2c_id_table[] = {
+	{ "elantech_i2c", 0 },
+	{ },
+};
+MODULE_DEVICE_TABLE(i2c, elantech_i2c_id_table);
+
+static struct i2c_driver elantech_i2c_driver = {
+	.driver = {
+		.name	= DRIVER_NAME,
+		.owner	= THIS_MODULE,
+		.pm	= &elan_i2c_touchpad_pm_ops,
+	},
+	.probe		= elantech_i2c_probe,
+	.remove		= __devexit_p(elantech_i2c_remove),
+	.id_table	= elantech_i2c_id_table,
+};
+
+static int __init elantech_i2c_init(void)
+{
+	return i2c_add_driver(&elantech_i2c_driver);
+}
+
+static void __exit elantech_i2c_exit(void)
+{
+	i2c_del_driver(&elantech_i2c_driver);
+}
+
+module_init(elantech_i2c_init);
+module_exit(elantech_i2c_exit);
+
+MODULE_AUTHOR("Tom Lin (Lin Yen Yu) <tom_lin@emc.com.tw>");
+MODULE_DESCRIPTION("Elan I2C Touch Pad driver");
+MODULE_LICENSE("GPL");
-- 
1.7.9.2

Re: [PATCH v3] Input: Elan HID-I2C device driver

[Dmitry Torokhov]

Hi Tom,

On Tue, Apr 10, 2012 at 08:42:46PM +0800, Tom Lin wrote:
> This patch adds driver for Elan I2C touchpad. These protocol of HID-I2C was
> defined by Microsoft. The kernel driver would use the multi-touch protocol
> format B.
> 

If this device truly supports HID protocol shouldn't we define i2c-hid
transport, similar to usbhid?

Thanks.

-- 
Dmitry

Re: [PATCH v3] Input: Elan HID-I2C device driver

[林彥佑]

Hi Daniel


On 04/11/2012 03:12 PM, Dmitry Torokhov wrote:
> Hi Tom,
>
> On Tue, Apr 10, 2012 at 08:42:46PM +0800, Tom Lin wrote:
>> This patch adds driver for Elan I2C touchpad. These protocol of HID-I2C was
>> defined by Microsoft. The kernel driver would use the multi-touch protocol
>> format B.
>>
>
> If this device truly supports HID protocol shouldn't we define i2c-hid
> transport, similar to usbhid?
I merely follow defined by Microsoft.
It was called "HID OVER I2C" by Microsoft.
(http://msdn.microsoft.com/en-us/library/windows/hardware/hh825917(v=vs.85).aspx)
But I agree to change from hid i2c to i2chid.
Do you have any better opinions?
>
> Thanks.
>

Re: [PATCH v3] Input: Elan HID-I2C device driver

[Daniel Kurtz]

On Tue, Apr 10, 2012 at 8:42 PM, Tom Lin <tom_lin@emc.com.tw> wrote:
> This patch adds driver for Elan I2C touchpad. These protocol of HID-I2C was
> defined by Microsoft. The kernel driver would use the multi-touch protocol
> format B.
>
> Signed-off-by:Tom Lin(Lin Yen Yu) <tom_lin@emc.com.tw>
> ---
>  drivers/input/mouse/Kconfig        |    8 +
>  drivers/input/mouse/Makefile       |    1 +
>  drivers/input/mouse/elantech_i2c.c |  504 ++++++++++++++++++++++++++++++++++++
>  3 files changed, 513 insertions(+)
>  create mode 100644 drivers/input/mouse/elantech_i2c.c
>
> diff --git a/drivers/input/mouse/Kconfig b/drivers/input/mouse/Kconfig
> index 9b8db82..345cf8c 100644
> --- a/drivers/input/mouse/Kconfig
> +++ b/drivers/input/mouse/Kconfig
> @@ -339,4 +339,12 @@ config MOUSE_SYNAPTICS_USB
>          To compile this driver as a module, choose M here: the
>          module will be called synaptics_usb.
>
> +config MOUSE_ELANTECH_I2C

I think the KConfig is supposed to be alphabetized...  but the Kconfig
file looks like it is already pretty jumbled.  I'll leave it to the
experts to express the official opinion.

> +       tristate "Elan I2C Touchpad support"
> +       depends on I2C
> +       help
> +        Say Y here if you want to use a Elan HID-I2C touchpad.
> +
> +        To compile this driver as a module, choose M here: the
> +        module will be called elantech_i2c.
>  endif
> diff --git a/drivers/input/mouse/Makefile b/drivers/input/mouse/Makefile
> index 4718eff..6f092a3 100644
> --- a/drivers/input/mouse/Makefile
> +++ b/drivers/input/mouse/Makefile
> @@ -20,6 +20,7 @@ obj-$(CONFIG_MOUSE_SERIAL)            += sermouse.o
>  obj-$(CONFIG_MOUSE_SYNAPTICS_I2C)      += synaptics_i2c.o
>  obj-$(CONFIG_MOUSE_SYNAPTICS_USB)      += synaptics_usb.o
>  obj-$(CONFIG_MOUSE_VSXXXAA)            += vsxxxaa.o
> +obj-$(CONFIG_MOUSE_ELANTECH_I2C)       += elantech_i2c.o

Please alphabetize.

>
>  psmouse-objs := psmouse-base.o synaptics.o
>
> diff --git a/drivers/input/mouse/elantech_i2c.c b/drivers/input/mouse/elantech_i2c.c
> new file mode 100644
> index 0000000..3e4b5d4
> --- /dev/null
> +++ b/drivers/input/mouse/elantech_i2c.c
> @@ -0,0 +1,504 @@
> +/*
> + * Elantech I2C Touchpad diver
> + *
> + * Copyright (c) 2011-2012 Tom Lin (Lin Yen Yu) <tom_lin@emc.com.tw>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License version 2 as published
> + * by the Free Software Foundation.
> + *
> + * Trademarks are the property of their respective owners.
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/device.h>
> +#include <linux/gpio.h>
> +#include <linux/i2c.h>
> +#include <linux/init.h>
> +#include <linux/input.h>
> +#include <linux/input/mt.h>
> +#include <linux/interrupt.h>
> +#include <linux/module.h>
> +#include <linux/slab.h>
> +#include <linux/workqueue.h>
> +
> +#define DRIVER_NAME            "elantech_i2c"
> +#define ETP_FINGER_DATA_OFFSET 4
> +#define ETP_FINGER_DATA_LEN    5
> +#define ETP_I2C_COMMAND_TRIES  3
> +#define ETP_I2C_COMMAND_DELAY  500
> +/*length of Elan Touchpad information*/
> +#define ETP_INF_LENGTH         2
> +#define ETP_MAX_FINGERS                5
> +#define ETP_PACKET_LENGTH      30
> +#define ETP_REPORT_ID          0X5d
> +#define HID_DES_LENGTH_OFFSET  1
> +
> +static bool elan_i2c_debug;
> +module_param_named(debug, elan_i2c_debug, bool, 0600);
> +MODULE_PARM_DESC(debug, "Turn Elan i2c TP  debugging mode on and off");
> +
> +#define elantech_debug(fmt, ...)                               \
> +       do {                                                    \
> +               if (elan_i2c_debug)                             \
> +                       printk(KERN_DEBUG KBUILD_MODNAME        \
> +                                       fmt, ##__VA_ARGS__);    \

Why not use dev_dbg()

> +       } while (0)
> +
> +enum etd_i2c_command {
> +       ETP_HID_DESCR_LENGTH_CMD,
> +       ETP_HID_DESCR_CMD,
> +       ETP_HID_WAKE_UP_CMD,
> +       ETP_HID_RESET_CMD,
> +       ETP_HID_REPORT_DESCR_CMD,
> +       ETP_TRACE_NUM_CMD,
> +       ETP_X_AXIS_MAX_CMD,
> +       ETP_Y_AXIS_MAX_CMD,
> +       ETP_DPI_VALUE_CMD,
> +       ETP_ENABLE_ABS_CMD
> +};
> +
> +static const u8 hid_descriptor[] = {0x01, 0x00};
> +static const u8 hid_wake_up[] = {0x05, 0x00, 0x00, 0x08};
> +static const u8 hid_reset_cmd[] = {0x05, 0x00, 0x00, 0x01};
> +static const u8 hid_report_descriptor[] = {0x02, 0x00};
> +static const u8 trace_num_xy[] = {0x05, 0x01};
> +static const u8 x_axis_max[] = {0x06, 0x01};
> +static const u8 y_axis_max[] = {0x07, 0x01};
> +static const u8 dpi_value[] = {0x08, 0x01};
> +static const u8 enable_abs[] = {0x00, 0x03, 0x01, 0x00};

If these are all etc commands, start their names with etc_ and end with _cmd.
In fact, just use a lower case version of the corresponding value from the enum.

> +
> +/* The main device structure */
> +struct elantech_i2c {
> +       struct i2c_client       *client;
> +       struct input_dev        *input;
> +       struct work_struct      work;
> +       unsigned int            gpio;
> +       unsigned int            max_x;
> +       unsigned int            max_y;
> +       unsigned int            width_x;
> +       unsigned int            width_y;
> +       int                     irq;
> +       u8                      pre_recv[28];
> +};
> +
> +static int elantech_i2c_command(struct i2c_client *client, int  command,
> +                               unsigned char *buf_recv, int data_len)
> +{
> +       const u8 *cmd = NULL;
> +       unsigned char *rec_buf = buf_recv;
> +       int ret;
> +       int tries = ETP_I2C_COMMAND_TRIES;
> +       int length = 0;
> +       struct i2c_msg msg[2];
> +       int msg_num = 0;
> +
> +       switch (command) {
> +       case ETP_HID_DESCR_LENGTH_CMD:
> +       case ETP_HID_DESCR_CMD:
> +               cmd = hid_descriptor;
> +               length = sizeof(hid_descriptor);
> +               msg[1].len = data_len;
> +               msg_num = 2;
> +               break;
> +       case ETP_HID_WAKE_UP_CMD:
> +               cmd = hid_wake_up;
> +               length = sizeof(hid_wake_up);
> +               msg_num = 1;
> +               break;
> +       case ETP_HID_RESET_CMD:
> +               cmd = hid_reset_cmd;
> +               length = sizeof(hid_reset_cmd);
> +               msg_num = 1;
> +               break;
> +       case ETP_HID_REPORT_DESCR_CMD:
> +               cmd = hid_report_descriptor;
> +               length = sizeof(hid_report_descriptor);
> +               msg[1].len = data_len;
> +               msg_num = 2;
> +               break;
> +       case ETP_TRACE_NUM_CMD:
> +               cmd = trace_num_xy;
> +               length = sizeof(trace_num_xy);
> +               msg[1].len = data_len;
> +               msg_num = 2;
> +               break;
> +       case ETP_X_AXIS_MAX_CMD:
> +               cmd = x_axis_max;
> +               length = sizeof(x_axis_max);
> +               msg[1].len = data_len;
> +               msg_num = 2;
> +               break;
> +       case ETP_Y_AXIS_MAX_CMD:
> +               cmd = y_axis_max;
> +               length = sizeof(y_axis_max);
> +               msg[1].len = data_len;
> +               msg_num = 2;
> +               break;
> +       case ETP_DPI_VALUE_CMD:
> +               cmd = dpi_value;
> +               length = sizeof(dpi_value);
> +               msg[1].len = data_len;
> +               msg_num = 2;
> +               break;
> +       case ETP_ENABLE_ABS_CMD:
> +               cmd = enable_abs;
> +               length = sizeof(enable_abs);
> +               msg_num = 1;
> +               break;
> +       default:
> +               elantech_debug("%s command=%d unknow.\n",
> +                        __func__, command);
> +               return -1;
> +       }
> +
> +       msg[0].addr = client->addr;
> +       msg[0].flags = client->flags & I2C_M_TEN;
> +       msg[0].len = length;
> +       msg[0].buf = (char *) cmd;
> +       msg[1].addr = client->addr;
> +       msg[1].flags = client->flags & I2C_M_TEN;
> +       msg[1].flags |= I2C_M_RD;
> +       msg[1].buf = rec_buf;
> +
> +       do {
> +               ret = i2c_transfer(client->adapter, msg, msg_num);
> +               if (ret != msg_num && elan_i2c_debug)
> +                       print_hex_dump_bytes("Elan I2C Touch data :",
> +                               DUMP_PREFIX_NONE, rec_buf, msg[1].len);
> +               if (ret > 0)
> +                       break;
> +               tries--;
> +               elantech_debug("retrying elantech_i2c_command:%d (%d)\n",
> +                       command, tries);
> +       } while (tries > 0);
> +
> +       return ret;
> +}
> +
> +static bool elantech_i2c_hw_init(struct i2c_client *client)
> +{
> +       int rc;
> +       uint8_t buf_recv[79];

Why 79?  A named constant would be better.

> +       int hid_descr_len;
> +       int hid_report_len;
> +
> +       /*Fetch the length of HID description*/
/* Fetch HID descriptor length */

Note the space after /* and before the trailing */.  This applies to
all of your comments.

> +       rc = elantech_i2c_command(client, ETP_HID_DESCR_LENGTH_CMD, buf_recv,
> +                       HID_DES_LENGTH_OFFSET);
> +       if (rc != 2)
> +               return false;
> +       else

else not needed

> +               hid_descr_len = buf_recv[0];
> +
> +       /*Fetch the lenght of HID report description*/

/* Fetch HID descriptor and parse HID report descriptor form length
from byte 4 */

Why ignore the rest if the HID descriptor?
What does it contain?
I am not familiar with the HID spec.  Perhaps this is well defined elsewhere.

> +       rc = elantech_i2c_command(client, ETP_HID_DESCR_CMD, buf_recv, hid_descr_len);
> +       if (rc != 2)
> +               return false;
> +       else
else not needed

> +               hid_report_len = buf_recv[4];
> +
> +       rc = elantech_i2c_command(client, ETP_HID_WAKE_UP_CMD, buf_recv, 0);
> +       if (rc != 1)
> +               return false;
> +
> +       rc = elantech_i2c_command(client, ETP_HID_RESET_CMD, buf_recv, 0);
> +       if (rc != 1)
> +               return false;
> +
> +       msleep(ETP_I2C_COMMAND_DELAY);

It is generally not a good idea to put a 500 ms msleep in the middle
of a driver probe.  This directly affects boot time.
The rest of this initialization should be done in a workqueue in
response to the interrupt that should occur after reset (according to
the HID-I2C doc that you referenced).


> +       rc = i2c_master_recv(client, buf_recv, 2);
> +       if (rc != 2 || (buf_recv[0] != 0 && buf_recv[1] != 0))
> +               return false;
> +
> +       rc = elantech_i2c_command(client, ETP_HID_REPORT_DESCR_CMD, buf_recv,
> +               hid_report_len);
> +       if (rc != 2)
> +               return false;
> +
> +       return true;
> +}
> +
> +static void elantech_i2c_report_absolute(struct elantech_i2c *touch,
> +                                uint8_t *buf)
> +{
> +       unsigned char *packet = buf;
Why do you need a separate variable?

> +       unsigned char *finger_data;
            uint8_t *finger_data = &packet[ETP_FINGER_DATA_OFFSET];
> +       struct input_dev *input = touch->input;
> +       int finger_status[5];

  bool finger_status;

No need for an array

> +       int i;
> +       int position_x, position_y;
> +       int mky_value, mkx_value, h_value;
> +
> +       dyn_data_i = 0;

> +       for (i = 0 ; i < ETP_MAX_FINGERS ; i++) {
> +               finger_status[i] = (packet[3] >> (3+i)) & 0x01;
> +               if (finger_status[i]) {

> +                       position_x = ((finger_data[0] & 0xf0) << 4) | finger_data[1];
> +                       position_y = touch->max_y -
> +                               (((finger_data[0] & 0x0f) << 8) | finger_data[2]);
> +                       mkx_value = (finger_data[3] & 0x0f) * touch->width_x;
> +                       mky_value = (finger_data[3] >> 4) * touch->width_y;
> +                       h_value = finger_data[4];

                            finger_data += ETP_FINGER_DATA_LEN;

> +                       input_mt_slot(input, i);
> +                       input_mt_report_slot_state(input, MT_TOOL_FINGER, true);
> +
> +                       input_report_abs(input, ABS_MT_POSITION_X, position_x);
> +                       input_report_abs(input, ABS_MT_POSITION_Y, position_y);
> +                       input_report_abs(input, ABS_MT_PRESSURE, h_value);
> +                       input_report_abs(input, ABS_MT_TOUCH_MAJOR, mkx_value);
> +                       input_report_abs(input, ABS_MT_TOUCH_MINOR, mky_value);
> +               } else if (finger_status[i] == 0) {

} else {

> +                       input_mt_slot(input, i);
> +                       input_mt_report_slot_state(input, MT_TOOL_FINGER, false);
> +               }
> +       }
> +       input_report_key(input, BTN_LEFT, ((packet[3] & 0x01) == 1));
> +       input_report_key(input, BTN_RIGHT, ((packet[3] & 0x02) == 2));
> +       input_report_key(input, BTN_MIDDLE, ((packet[3] & 0x04) == 4));
> +       input_mt_report_pointer_emulation(input, true);
> +       input_sync(input);
> +}
> +
> +static irqreturn_t elantech_i2c_isr(int irq, void *dev_id)
> +{
> +       struct elantech_i2c *elantech_i2c_priv = dev_id;
> +       unsigned char buf_recv[30] = {0};

uint8_t buf_recv[ETP_PACKET_LENGTH];

> +       int rc;
> +       int length;
> +
> +       rc = i2c_master_recv(elantech_i2c_priv->client, buf_recv, ETP_PACKET_LENGTH);

  if (rc != ETP_PACKET_LENGTH)
    goto elantech_isr_end;
  ...

> +       if (rc == ETP_PACKET_LENGTH) {
> +               length = (buf_recv[1] << 8) | buf_recv[0];

If you are sanity checking the packet, perhaps you should check this
length, too?

> +               if (!memcmp(elantech_i2c_priv->pre_recv, buf_recv, length))
> +                       goto elantech_isr_end;

Why are you checking that the packet is the same as before and exiting if it is?
Let the evdev layer handle event compression.

> +
> +               memcpy(elantech_i2c_priv->pre_recv, buf_recv, length);
> +               /*Packet check*/
> +               if (buf_recv[2] == ETP_REPORT_ID && buf_recv[length - 1] == 0x00)
> +                       elantech_i2c_report_absolute(elantech_i2c_priv, buf_recv);
> +               else if (elan_i2c_debug)
> +                       print_hex_dump_bytes("Elan I2C Touch data :",
> +                               DUMP_PREFIX_NONE, buf_recv, length);
> +       }
> +
> +elantech_isr_end:
> +       return IRQ_HANDLED;
> +}
> +
> +static struct elantech_i2c *elantech_i2c_priv_create(struct i2c_client *client)
> +{
> +       struct elantech_i2c *elantech_i2c_priv;
> +       struct input_dev *dev;
> +       unsigned int gpio;
> +       const char *label = "elantech_i2c";
> +       int rc = 0;
> +       int Pressure_value_data;

use lower case pressure

> +       unsigned char buf_recv[2];
> +
> +       elantech_i2c_priv = kzalloc(sizeof(struct elantech_i2c), GFP_KERNEL);
> +       if (!elantech_i2c_priv)
> +               return NULL;
> +
> +       elantech_i2c_priv->client = client;
> +       elantech_i2c_priv->irq = client->irq;
> +       elantech_i2c_priv->input = input_allocate_device();
> +       if (elantech_i2c_priv->input == NULL)
> +               goto err_input_allocate_device;
> +
> +       elantech_i2c_priv->input->name = "ELANTECH_I2C";
> +       elantech_i2c_priv->input->phys = client->adapter->name;
> +       elantech_i2c_priv->input->id.bustype = BUS_I2C;
> +
> +       dev = elantech_i2c_priv->input;
> +       __set_bit(EV_KEY, dev->evbit);
> +       __set_bit(EV_ABS, dev->evbit);
> +
> +       __set_bit(BTN_LEFT, dev->keybit);
> +       __set_bit(BTN_RIGHT, dev->keybit);
> +       __set_bit(BTN_MIDDLE, dev->keybit);
> +
> +       __set_bit(BTN_TOUCH, dev->keybit);
> +       __set_bit(BTN_TOOL_FINGER, dev->keybit);
> +       __set_bit(BTN_TOOL_DOUBLETAP, dev->keybit);
> +       __set_bit(BTN_TOOL_TRIPLETAP, dev->keybit);
> +       __set_bit(BTN_TOOL_QUADTAP, dev->keybit);
> +
> +       elantech_i2c_command(client, ETP_X_AXIS_MAX_CMD, buf_recv, ETP_INF_LENGTH);
> +       elantech_i2c_priv->max_x = (0x0f & buf_recv[1]) << 8 | buf_recv[0];
> +       elantech_i2c_command(client, ETP_Y_AXIS_MAX_CMD, buf_recv, ETP_INF_LENGTH);
> +       elantech_i2c_priv->max_y = (0x0f & buf_recv[1]) << 8 | buf_recv[0];
> +       printk(KERN_INFO "%s max_x = %d\n", __func__, elantech_i2c_priv->max_x);

dev_info()

> +       printk(KERN_INFO "%s max_y = %d\n", __func__, elantech_i2c_priv->max_y);
> +       elantech_i2c_command(client, ETP_TRACE_NUM_CMD, buf_recv, ETP_INF_LENGTH);
> +       elantech_i2c_priv->width_x = elantech_i2c_priv->max_x / (buf_recv[0] - 1);
> +       elantech_i2c_priv->width_y = elantech_i2c_priv->max_y / (buf_recv[1] - 1);
> +       Pressure_value_data = 0xff;
> +
> +       /* X Y Value*/
> +       input_set_abs_params(dev, ABS_X, 0, elantech_i2c_priv->max_x, 0, 0);
> +       input_set_abs_params(dev, ABS_Y, 0, elantech_i2c_priv->max_y, 0, 0);
> +       /* Palme Value */
> +       input_set_abs_params(dev, ABS_PRESSURE, 0, 255, 0, 0);
> +
> +       /* Finger Pressure value */
> +       input_set_abs_params(dev, ABS_TOOL_WIDTH, 0, Pressure_value_data, 0, 0);
> +
> +       /* Finger Pressure value */
> +       input_mt_init_slots(dev, ETP_MAX_FINGERS);
> +       input_set_abs_params(dev, ABS_MT_POSITION_X, 0, elantech_i2c_priv->max_x, 0, 0);
> +       input_set_abs_params(dev, ABS_MT_POSITION_Y, 0, elantech_i2c_priv->max_y, 0, 0);
> +       input_set_abs_params(dev, ABS_MT_PRESSURE, 0, Pressure_value_data, 0, 0);
> +       input_set_abs_params(dev, ABS_MT_TOUCH_MAJOR, 0, Pressure_value_data, 0, 0);
> +       input_set_abs_params(dev, ABS_MT_TOUCH_MINOR, 0, Pressure_value_data, 0, 0);
> +
> +       /* Enable ABS mode*/
> +       elantech_i2c_command(client, ETP_ENABLE_ABS_CMD, buf_recv, ETP_INF_LENGTH);
> +       gpio = irq_to_gpio(client->irq);
> +       rc = gpio_request(gpio, label);
> +       if (rc < 0) {
> +               dev_dbg(&client->dev, "gpio_request failed for input %d rc = %d\n", gpio, rc);
> +               goto    err_gpio_request;
> +       }
> +       rc = gpio_direction_input(gpio);
> +       if (rc < 0) {
> +               dev_dbg(&client->dev, "gpio_direction_input failed for input %d\n", gpio);
> +               goto    err_gpio_request;
> +       }
> +       elantech_i2c_priv->gpio = gpio;
> +       rc = request_threaded_irq(client->irq, NULL, elantech_i2c_isr,
> +                       IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
> +                       client->name, elantech_i2c_priv);
> +        if (rc < 0) {
> +               dev_dbg(&client->dev, "Could not register for %s interrupt, irq = %d, rc = %d\n",
> +               label, client->irq, rc);
> +               goto    err_gpio_request_irq_fail;
> +       }
> +
> +       return elantech_i2c_priv;
> +
> +err_gpio_request_irq_fail:
> +       gpio_free(gpio);
> +err_gpio_request:
> +       input_free_device(elantech_i2c_priv->input);
> +err_input_allocate_device:

if you get here via the goto err_input_allocate_device,
elantech_i2c_priv is already NULL so you don't need to free it.

> +       kfree(elantech_i2c_priv);
> +       return NULL;
> +}
> +
> +static int elantech_i2c_probe(struct i2c_client *client,
> +                             const struct i2c_device_id *dev_id)
> +
> +{
> +       int ret = 0;
> +       struct elantech_i2c *elantech_i2c_priv;
> +       unsigned int gpio;
> +
> +       if (!elantech_i2c_hw_init(client)) {
> +               ret = -EINVAL;
> +               goto err_hw_init;
> +       }
> +       elantech_i2c_priv = elantech_i2c_priv_create(client);
> +       if (!elantech_i2c_priv) {
> +               ret = -EINVAL;
> +               goto err_elantech_i2c_priv;
> +       }
> +
> +       ret = input_register_device(elantech_i2c_priv->input);
> +       if (ret < 0)
> +               goto err_input_register_device;
> +
> +       i2c_set_clientdata(client, elantech_i2c_priv);
> +       device_init_wakeup(&client->dev, 1);
> +
> +       return 0;
> +
> +err_input_register_device:
> +       gpio = elantech_i2c_priv->gpio;
> +       gpio_free(gpio);
> +       free_irq(elantech_i2c_priv->irq, elantech_i2c_priv);
> +       input_free_device(elantech_i2c_priv->input);
> +       kfree(elantech_i2c_priv);
> +err_elantech_i2c_priv:

Remove redundant label.

> +err_hw_init:
> +       return ret;
> +}
> +
> +static int elantech_i2c_remove(struct i2c_client *client)
> +{
> +       struct elantech_i2c *elantech_i2c_priv = i2c_get_clientdata(client);
> +
> +       if (elantech_i2c_priv) {
> +               if (elantech_i2c_priv->gpio > 0)
> +                       gpio_free(elantech_i2c_priv->gpio);
> +               free_irq(elantech_i2c_priv->irq, elantech_i2c_priv);
> +               input_unregister_device(elantech_i2c_priv->input);
> +               kfree(elantech_i2c_priv);
> +       }
> +
> +       return 0;
> +}
> +
> +#ifdef CONFIG_PM_SLEEP
> +static int elantech_i2c_suspend(struct device *dev)
> +{
> +       struct i2c_client *client = to_i2c_client(dev);
> +
> +       if (device_may_wakeup(&client->dev))
> +               enable_irq_wake(client->irq);
> +
> +       return 0;
> +}
> +
> +static int elantech_i2c_resume(struct device *dev)
> +{
> +       struct i2c_client *client = to_i2c_client(dev);
> +
> +       if (device_may_wakeup(&client->dev))
> +               enable_irq_wake(client->irq);
> +
> +       return 0;
> +}
> +#endif
> +
> +static SIMPLE_DEV_PM_OPS(elan_i2c_touchpad_pm_ops,
> +               elantech_i2c_suspend, elantech_i2c_resume);
> +
> +static const struct i2c_device_id elantech_i2c_id_table[] = {
> +       { "elantech_i2c", 0 },
> +       { },
> +};
> +MODULE_DEVICE_TABLE(i2c, elantech_i2c_id_table);
> +
> +static struct i2c_driver elantech_i2c_driver = {
> +       .driver = {
> +               .name   = DRIVER_NAME,
> +               .owner  = THIS_MODULE,
> +               .pm     = &elan_i2c_touchpad_pm_ops,
> +       },
> +       .probe          = elantech_i2c_probe,
> +       .remove         = __devexit_p(elantech_i2c_remove),
> +       .id_table       = elantech_i2c_id_table,
> +};
> +
> +static int __init elantech_i2c_init(void)
> +{
> +       return i2c_add_driver(&elantech_i2c_driver);
> +}
> +
> +static void __exit elantech_i2c_exit(void)
> +{
> +       i2c_del_driver(&elantech_i2c_driver);
> +}
> +
> +module_init(elantech_i2c_init);
> +module_exit(elantech_i2c_exit);
> +
> +MODULE_AUTHOR("Tom Lin (Lin Yen Yu) <tom_lin@emc.com.tw>");
> +MODULE_DESCRIPTION("Elan I2C Touch Pad driver");
> +MODULE_LICENSE("GPL");
> --
> 1.7.9.2
>

Re: [PATCH v3] Input: Elan HID-I2C device driver

[Dmitry Torokhov]

On Wednesday, April 11, 2012 06:09:03 PM 林彥佑 wrote:
> Hi Daniel
> 
> On 04/11/2012 03:12 PM, Dmitry Torokhov wrote:
> > Hi Tom,
> > 
> > On Tue, Apr 10, 2012 at 08:42:46PM +0800, Tom Lin wrote:
> >> This patch adds driver for Elan I2C touchpad. These protocol of HID-I2C
> >> was
> >> defined by Microsoft. The kernel driver would use the multi-touch
> >> protocol
> >> format B.
> > 
> > If this device truly supports HID protocol shouldn't we define i2c-hid
> > transport, similar to usbhid?
> 
> I merely follow defined by Microsoft.
> It was called "HID OVER I2C" by Microsoft.
> (http://msdn.microsoft.com/en-us/library/windows/hardware/hh825917(v=vs.85).
> aspx) But I agree to change from hid i2c to i2chid.
> Do you have any better opinions?

I am not sure we are talking about same thing... I do not really care
whether it is named "hid i2c" or "i2chid". What I meant if the device
really speaks HID protocol, except that the transport is not USB or
bluetooth but I2C, then instead of writing a custom driver for this
particular part we need to add a new HID transport into
drivers/hid/i2c-hid that is similar to drivers/hid/usbhid/ and reuse the
rest of HID framework along with hid- multitouch.c which handles
HID-compliant multitouch devices.

I am CCing Jiri (HID maintainer) for additional input.

Thanks.

-- 
Dmitry

Re: [PATCH v3] Input: Elan HID-I2C device driver

[Ping Cheng]

On Wed, Apr 11, 2012 at 9:25 AM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> On Wednesday, April 11, 2012 06:09:03 PM 林彥佑 wrote:
>> Hi Daniel
>>
>> On 04/11/2012 03:12 PM, Dmitry Torokhov wrote:
>> > Hi Tom,
>> >
>> > On Tue, Apr 10, 2012 at 08:42:46PM +0800, Tom Lin wrote:
>> >> This patch adds driver for Elan I2C touchpad. These protocol of HID-I2C
>> >> was
>> >> defined by Microsoft. The kernel driver would use the multi-touch
>> >> protocol
>> >> format B.
>> >
>> > If this device truly supports HID protocol shouldn't we define i2c-hid
>> > transport, similar to usbhid?
>>
>> I merely follow defined by Microsoft.
>> It was called "HID OVER I2C" by Microsoft.
>> (http://msdn.microsoft.com/en-us/library/windows/hardware/hh825917(v=vs.85).
>> aspx) But I agree to change from hid i2c to i2chid.
>> Do you have any better opinions?
>
> I am not sure we are talking about same thing... I do not really care
> whether it is named "hid i2c" or "i2chid". What I meant if the device
> really speaks HID protocol, except that the transport is not USB or
> bluetooth but I2C, then instead of writing a custom driver for this
> particular part we need to add a new HID transport into
> drivers/hid/i2c-hid that is similar to drivers/hid/usbhid/

I fully agree with Dmitry's suggestion. The new i2chid driver would
cover all hid-compliant i2c devices.

Ping

> and reuse the rest of HID framework along with hid- multitouch.c which handles
> HID-compliant multitouch devices.
>
> I am CCing Jiri (HID maintainer) for additional input.
>
> Thanks.
>
> --
> Dmitry

Re: [PATCH v3] Input: Elan HID-I2C device driver

[Ping Cheng]

On Wed, Apr 11, 2012 at 9:25 AM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> On Wednesday, April 11, 2012 06:09:03 PM 林彥佑 wrote:
>> Hi Daniel
>>
>> On 04/11/2012 03:12 PM, Dmitry Torokhov wrote:
>> > Hi Tom,
>> >
>> > On Tue, Apr 10, 2012 at 08:42:46PM +0800, Tom Lin wrote:
>> >> This patch adds driver for Elan I2C touchpad. These protocol of HID-I2C
>> >> was
>> >> defined by Microsoft. The kernel driver would use the multi-touch
>> >> protocol
>> >> format B.
>> >
>> > If this device truly supports HID protocol shouldn't we define i2c-hid
>> > transport, similar to usbhid?
>>
>> I merely follow defined by Microsoft.
>> It was called "HID OVER I2C" by Microsoft.
>> (http://msdn.microsoft.com/en-us/library/windows/hardware/hh825917(v=vs.85).
>> aspx) But I agree to change from hid i2c to i2chid.
>> Do you have any better opinions?
>
> I am not sure we are talking about same thing... I do not really care
> whether it is named "hid i2c" or "i2chid". What I meant if the device
> really speaks HID protocol, except that the transport is not USB or
> bluetooth but I2C, then instead of writing a custom driver for this
> particular part we need to add a new HID transport into
> drivers/hid/i2c-hid that is similar to drivers/hid/usbhid/

I fully agree with Dmitry's suggestion. The new i2chid driver would
cover all hid-compliant i2c devices.

Ping

> and reuse the rest of HID framework along with hid- multitouch.c which handles
> HID-compliant multitouch devices.
>
> I am CCing Jiri (HID maintainer) for additional input.
>
> Thanks.
>
> --
> Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3] Input: Elan HID-I2C device driver

[Jiri Kosina]

On Wed, 11 Apr 2012, Dmitry Torokhov wrote:

> > >> This patch adds driver for Elan I2C touchpad. These protocol of 
> > >> HID-I2C was defined by Microsoft. The kernel driver would use the 
> > >> multi-touch protocol format B.
> > > 
> > > If this device truly supports HID protocol shouldn't we define i2c-hid
> > > transport, similar to usbhid?
> > 
> > I merely follow defined by Microsoft.
> > It was called "HID OVER I2C" by Microsoft.
> > (http://msdn.microsoft.com/en-us/library/windows/hardware/hh825917(v=vs.85).
> > aspx) But I agree to change from hid i2c to i2chid.
> > Do you have any better opinions?
> 
> I am not sure we are talking about same thing... I do not really care
> whether it is named "hid i2c" or "i2chid". What I meant if the device
> really speaks HID protocol, except that the transport is not USB or
> bluetooth but I2C, then instead of writing a custom driver for this
> particular part we need to add a new HID transport into
> drivers/hid/i2c-hid that is similar to drivers/hid/usbhid/ and reuse the
> rest of HID framework along with hid- multitouch.c which handles
> HID-compliant multitouch devices.
> 
> I am CCing Jiri (HID maintainer) for additional input.

Yes, please.

In case you'd be looking into a bit less bloated example of how transport 
driver for HID subsystem looks like, see the Bluetooth one in 
net/bluetooth/hidp/ (and we don't currently have any other examples than 
these two :) ).

I'd be happy to merge it once you convert to to proper HID transport 
driver.

Could the event processing part then be integrated into hid-multitouch?

Thanks,

-- 
Jiri Kosina
SUSE Labs

Re: [PATCH v3] Input: Elan HID-I2C device driver

[林彥佑]

Hi Dmitry

On 04/12/2012 12:25 AM, Dmitry Torokhov wrote:
> On Wednesday, April 11, 2012 06:09:03 PM 林彥佑 wrote:
>> Hi Daniel
>>
>> On 04/11/2012 03:12 PM, Dmitry Torokhov wrote:
>>> Hi Tom,
>>>
>>> On Tue, Apr 10, 2012 at 08:42:46PM +0800, Tom Lin wrote:
>>>> This patch adds driver for Elan I2C touchpad. These protocol of HID-I2C
>>>> was
>>>> defined by Microsoft. The kernel driver would use the multi-touch
>>>> protocol
>>>> format B.
>>>
>>> If this device truly supports HID protocol shouldn't we define i2c-hid
>>> transport, similar to usbhid?
>>
>> I merely follow defined by Microsoft.
>> It was called "HID OVER I2C" by Microsoft.
>> (http://msdn.microsoft.com/en-us/library/windows/hardware/hh825917(v=vs.85).
>> aspx) But I agree to change from hid i2c to i2chid.
>> Do you have any better opinions?
>
> I am not sure we are talking about same thing... I do not really care
> whether it is named "hid i2c" or "i2chid". What I meant if the device
> really speaks HID protocol, except that the transport is not USB or
> bluetooth but I2C, then instead of writing a custom driver for this
> particular part we need to add a new HID transport into
> drivers/hid/i2c-hid that is similar to drivers/hid/usbhid/ and reuse the
> rest of HID framework along with hid- multitouch.c which handles
> HID-compliant multitouch devices.
>
> I am CCing Jiri (HID maintainer) for additional input.
Sorry! I misunderstood what you mean.
I fully agree with your suggestion. The I2CHID device will be more and 
more in the future. Need the new i2chid-driver that would cover all 
hid-compliant i2c-devices.
Thank you

Tom-
>
> Thanks.
>

Re: [PATCH v3] Input: Elan HID-I2C device driver

[Benjamin Tissoires]

Hi Tom,

I read Microsoft's things about HID over I2C few days ago. And from
what I understood, the report descriptors and the enumeration of the
devices are stored in the ACPI table and not in the device itself. Can
you confirm this?

I was thinking about making hid-multitouch independent from the usb
stack and was thinking about doing the i2c-hid bridge. However, I did
not start writing a line yet ;-)

Cheers,
Benjamin

On Thu, Apr 12, 2012 at 03:37, 林彥佑 <tom_lin@emc.com.tw> wrote:
> Hi Dmitry
>
>
> On 04/12/2012 12:25 AM, Dmitry Torokhov wrote:
>>
>> On Wednesday, April 11, 2012 06:09:03 PM 林彥佑 wrote:
>>>
>>> Hi Daniel
>>>
>>> On 04/11/2012 03:12 PM, Dmitry Torokhov wrote:
>>>>
>>>> Hi Tom,
>>>>
>>>> On Tue, Apr 10, 2012 at 08:42:46PM +0800, Tom Lin wrote:
>>>>>
>>>>> This patch adds driver for Elan I2C touchpad. These protocol of HID-I2C
>>>>> was
>>>>> defined by Microsoft. The kernel driver would use the multi-touch
>>>>> protocol
>>>>> format B.
>>>>
>>>>
>>>> If this device truly supports HID protocol shouldn't we define i2c-hid
>>>> transport, similar to usbhid?
>>>
>>>
>>> I merely follow defined by Microsoft.
>>> It was called "HID OVER I2C" by Microsoft.
>>>
>>> (http://msdn.microsoft.com/en-us/library/windows/hardware/hh825917(v=vs.85).
>>> aspx) But I agree to change from hid i2c to i2chid.
>>> Do you have any better opinions?
>>
>>
>> I am not sure we are talking about same thing... I do not really care
>> whether it is named "hid i2c" or "i2chid". What I meant if the device
>> really speaks HID protocol, except that the transport is not USB or
>> bluetooth but I2C, then instead of writing a custom driver for this
>> particular part we need to add a new HID transport into
>> drivers/hid/i2c-hid that is similar to drivers/hid/usbhid/ and reuse the
>> rest of HID framework along with hid- multitouch.c which handles
>> HID-compliant multitouch devices.
>>
>> I am CCing Jiri (HID maintainer) for additional input.
>
> Sorry! I misunderstood what you mean.
> I fully agree with your suggestion. The I2CHID device will be more and more
> in the future. Need the new i2chid-driver that would cover all hid-compliant
> i2c-devices.
> Thank you
>
> Tom-
>>
>>
>> Thanks.
>>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-input" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3] Input: Elan HID-I2C device driver

[Benjamin Tissoires]

Hi Tom,

I read Microsoft's things about HID over I2C few days ago. And from
what I understood, the report descriptors and the enumeration of the
devices are stored in the ACPI table and not in the device itself. Can
you confirm this?

I was thinking about making hid-multitouch independent from the usb
stack and was thinking about doing the i2c-hid bridge. However, I did
not start writing a line yet ;-)

Cheers,
Benjamin

On Thu, Apr 12, 2012 at 03:37, 林彥佑 <tom_lin@emc.com.tw> wrote:
> Hi Dmitry
>
>
> On 04/12/2012 12:25 AM, Dmitry Torokhov wrote:
>>
>> On Wednesday, April 11, 2012 06:09:03 PM 林彥佑 wrote:
>>>
>>> Hi Daniel
>>>
>>> On 04/11/2012 03:12 PM, Dmitry Torokhov wrote:
>>>>
>>>> Hi Tom,
>>>>
>>>> On Tue, Apr 10, 2012 at 08:42:46PM +0800, Tom Lin wrote:
>>>>>
>>>>> This patch adds driver for Elan I2C touchpad. These protocol of HID-I2C
>>>>> was
>>>>> defined by Microsoft. The kernel driver would use the multi-touch
>>>>> protocol
>>>>> format B.
>>>>
>>>>
>>>> If this device truly supports HID protocol shouldn't we define i2c-hid
>>>> transport, similar to usbhid?
>>>
>>>
>>> I merely follow defined by Microsoft.
>>> It was called "HID OVER I2C" by Microsoft.
>>>
>>> (http://msdn.microsoft.com/en-us/library/windows/hardware/hh825917(v=vs.85).
>>> aspx) But I agree to change from hid i2c to i2chid.
>>> Do you have any better opinions?
>>
>>
>> I am not sure we are talking about same thing... I do not really care
>> whether it is named "hid i2c" or "i2chid". What I meant if the device
>> really speaks HID protocol, except that the transport is not USB or
>> bluetooth but I2C, then instead of writing a custom driver for this
>> particular part we need to add a new HID transport into
>> drivers/hid/i2c-hid that is similar to drivers/hid/usbhid/ and reuse the
>> rest of HID framework along with hid- multitouch.c which handles
>> HID-compliant multitouch devices.
>>
>> I am CCing Jiri (HID maintainer) for additional input.
>
> Sorry! I misunderstood what you mean.
> I fully agree with your suggestion. The I2CHID device will be more and more
> in the future. Need the new i2chid-driver that would cover all hid-compliant
> i2c-devices.
> Thank you
>
> Tom-
>>
>>
>> Thanks.
>>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-input" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3] Input: Elan HID-I2C device driver

[Jiri Kosina]

On Thu, 12 Apr 2012, Benjamin Tissoires wrote:

> I read Microsoft's things about HID over I2C few days ago. And from
> what I understood, the report descriptors and the enumeration of the
> devices are stored in the ACPI table and not in the device itself. Can
> you confirm this?
> 
> I was thinking about making hid-multitouch independent from the usb
> stack and was thinking about doing the i2c-hid bridge. However, I did
> not start writing a line yet ;-)

I guess you were planning to write that as a HID transport (ll) driver, 
right?

Thanks,

-- 
Jiri Kosina
SUSE Labs

Re: [PATCH v3] Input: Elan HID-I2C device driver

[Henrik Rydberg]

On Fri, Apr 13, 2012 at 02:43:14PM +0200, Jiri Kosina wrote:
> On Thu, 12 Apr 2012, Benjamin Tissoires wrote:
> 
> > I read Microsoft's things about HID over I2C few days ago. And from
> > what I understood, the report descriptors and the enumeration of the
> > devices are stored in the ACPI table and not in the device itself. Can
> > you confirm this?
> > 
> > I was thinking about making hid-multitouch independent from the usb
> > stack and was thinking about doing the i2c-hid bridge. However, I did
> > not start writing a line yet ;-)
> 
> I guess you were planning to write that as a HID transport (ll) driver, 
> right?

Sounds great. The recent investigations into the properties of the hid
device identity for multitouch suggests that the ll driver could be
seen as the owner of the hid device itself, and should take on the
responsibility of both report fixups and determining the device
identify (read modalias). Using generic code (hid-owner?) of course. I
am experimenting with a couple of patches for it. Would you all agree
with this assessment?

Henrik