[PATCH v2] input: mouse: add qci touchpad driver

[PATCH v2] input: mouse: add qci touchpad driver

[Neil Leeder]

This driver is for the QCI trackpad used on Quanta smartbooks

Signed-off-by: Horace Fu <horace.fu@quantatw.com>
Signed-off-by: Mandeep Singh Baines <msb@chromium.org>
[nleeder@codeaurora.org: cleanup i2c calls, address review comments etc]
Signed-off-by: Neil Leeder <nleeder@codeaurora.org>
---
 drivers/input/mouse/Kconfig        |   12 ++
 drivers/input/mouse/Makefile       |    1 +
 drivers/input/mouse/qci_touchpad.c |  270 ++++++++++++++++++++++++++++++++++++
 include/linux/input/qci_touchpad.h |   25 ++++
 4 files changed, 308 insertions(+), 0 deletions(-)
 create mode 100644 drivers/input/mouse/qci_touchpad.c
 create mode 100644 include/linux/input/qci_touchpad.h

diff --git a/drivers/input/mouse/Kconfig b/drivers/input/mouse/Kconfig
index c714ca2..32a88b4 100644
--- a/drivers/input/mouse/Kconfig
+++ b/drivers/input/mouse/Kconfig
@@ -303,6 +303,18 @@ config MOUSE_MAPLE
 	  To compile this driver as a module choose M here: the module will be
 	  called maplemouse.
 
+config MOUSE_QCITP
+	tristate "Quanta Computer Inc. Touchpad"
+	depends on I2C
+	help
+	  This driver supports the touchpad on Quanta smartbook devices.
+
+	  Say Y here if you have a Quanta-based smartboot or notepad
+	  device and want to use the Quanta touchpad driver.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called qci_touchpad.
+
 config MOUSE_SYNAPTICS_I2C
 	tristate "Synaptics I2C Touchpad support"
 	depends on I2C
diff --git a/drivers/input/mouse/Makefile b/drivers/input/mouse/Makefile
index 570c84a..6eda35d 100644
--- a/drivers/input/mouse/Makefile
+++ b/drivers/input/mouse/Makefile
@@ -15,6 +15,7 @@ obj-$(CONFIG_MOUSE_MAPLE)		+= maplemouse.o
 obj-$(CONFIG_MOUSE_PC110PAD)		+= pc110pad.o
 obj-$(CONFIG_MOUSE_PS2)			+= psmouse.o
 obj-$(CONFIG_MOUSE_PXA930_TRKBALL)	+= pxa930_trkball.o
+obj-$(CONFIG_MOUSE_QCITP)		+= qci_touchpad.o
 obj-$(CONFIG_MOUSE_RISCPC)		+= rpcmouse.o
 obj-$(CONFIG_MOUSE_SERIAL)		+= sermouse.o
 obj-$(CONFIG_MOUSE_SYNAPTICS_I2C)	+= synaptics_i2c.o
diff --git a/drivers/input/mouse/qci_touchpad.c b/drivers/input/mouse/qci_touchpad.c
new file mode 100644
index 0000000..746cbaa
--- /dev/null
+++ b/drivers/input/mouse/qci_touchpad.c
@@ -0,0 +1,270 @@
+/* Quanta I2C Touchpad Driver
+ *
+ * Copyright (C) 2009 Quanta Computer Inc.
+ * Copyright (c) 2010, Code Aurora Forum. All rights reserved.
+ * Author: Hsin Wu <hsin.wu@quantatw.com>
+ * Author: Austin Lai <austin.lai@quantatw.com>
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ */
+
+/*
+ * Driver communicates over i2c to nuvoTon WPCE775x Embedded Controller,
+ * which has touchpad attached through PS/2 interface.
+ */
+#define pr_fmt(fmt) KBUILD_BASENAME ": " fmt
+
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/i2c.h>
+#include <linux/interrupt.h>
+#include <linux/input.h>
+#include <linux/gpio.h>
+#include <linux/delay.h>
+#include <linux/input/qci_touchpad.h>
+
+#define TOUCHPAD_ID_NAME          "qci-i2cpad"
+#define TOUCHPAD_NAME             "QCI Touchpad"
+#define TOUCHPAD_DEVICE           "/qci_touchpad/input0"
+#define TOUCHPAD_CMD_ENABLE       0xF4
+#define TOUCHPAD_READ_DATA_LEN    3
+
+struct i2ctpad_drv_data {
+	struct i2c_client *ti2c_client;
+	struct input_dev *qcitp_dev;
+	unsigned int qcitp_gpio;
+	unsigned int qcitp_irq;
+	char ecdata[8];
+	bool opened;
+};
+
+#ifdef CONFIG_PM
+static int qcitp_suspend(struct device *_dev)
+{
+	struct i2c_client *client =
+		container_of(_dev, struct i2c_client, dev);
+	struct i2ctpad_drv_data *context = i2c_get_clientdata(client);
+
+	disable_irq(context->qcitp_irq);
+	return 0;
+}
+
+static int qcitp_resume(struct device *_dev)
+{
+	struct i2c_client *client =
+		container_of(_dev, struct i2c_client, dev);
+	struct i2ctpad_drv_data *context = i2c_get_clientdata(client);
+
+	enable_irq(context->qcitp_irq);
+	return 0;
+}
+#endif
+
+static const struct i2c_device_id qcitp_idtable[] = {
+	{ "wpce775-touchpad", 0 },
+	{ }
+};
+
+MODULE_DEVICE_TABLE(i2c, qcitp_idtable);
+
+#ifdef CONFIG_PM
+static const struct dev_pm_ops qcitp_pm_ops = {
+	.suspend  = qcitp_suspend,
+	.resume   = qcitp_resume,
+};
+#endif
+
+static void qcitp_report_key(struct input_dev *tpad_dev, char *ec_data)
+{
+	int dx = 0;
+	int dy = 0;
+
+	if (ec_data[1])
+		dx = (int) ec_data[1] -
+		     (int) ((ec_data[0] << 4) & 0x100);
+
+	if (ec_data[2])
+		dy = (int) ((ec_data[0] << 3) & 0x100) -
+		     (int) ec_data[2];
+
+	input_report_key(tpad_dev, BTN_LEFT, ec_data[0] & 0x01);
+	input_report_key(tpad_dev, BTN_RIGHT, ec_data[0] & 0x02);
+	input_report_key(tpad_dev, BTN_MIDDLE, ec_data[0] & 0x04);
+	input_report_rel(tpad_dev, REL_X, dx);
+	input_report_rel(tpad_dev, REL_Y, dy);
+	input_sync(tpad_dev);
+}
+
+static irqreturn_t qcitp_interrupt(int irq, void *dev_id)
+{
+	struct i2ctpad_drv_data *context = dev_id;
+	int rc;
+
+	rc = i2c_master_recv(context->ti2c_client, context->ecdata,
+			     TOUCHPAD_READ_DATA_LEN);
+	if (rc == TOUCHPAD_READ_DATA_LEN)
+		qcitp_report_key(context->qcitp_dev, context->ecdata);
+
+	return IRQ_HANDLED;
+}
+
+static int qcitp_open(struct input_dev *input)
+{
+	struct i2ctpad_drv_data *context = input_get_drvdata(input);
+	u8 buf[1];
+	int err = 0;
+
+	if (context->opened)
+		return 0;
+	context->opened = 1;
+
+	buf[0] = TOUCHPAD_CMD_ENABLE;
+	err = i2c_master_send(context->ti2c_client, buf, 1);
+	if (err < 0)
+		goto i2c_fail;
+	msleep(100);
+	err = i2c_master_recv(context->ti2c_client, buf, 1);
+
+i2c_fail:
+	return err;
+}
+
+static int __devinit qcitp_probe(struct i2c_client *client,
+				    const struct i2c_device_id *id)
+{
+	int err;
+	struct i2ctpad_drv_data *context;
+	struct qci_touchpad_platform_data *pd;
+	int irq_trigger_type;
+
+	if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
+		pr_err("i2c functionality failed\n");
+		return -ENODEV;
+	}
+
+	context = kzalloc(sizeof(struct i2ctpad_drv_data), GFP_KERNEL);
+	if (!context)
+		return -ENOMEM;
+	i2c_set_clientdata(client, context);
+	context->ti2c_client = client;
+	context->qcitp_gpio = client->irq;
+
+	pd = client->dev.platform_data;
+	if (pd)
+		irq_trigger_type = pd->irq_trigger_type;
+	else
+		irq_trigger_type = IRQF_TRIGGER_FALLING;
+
+	context->qcitp_dev = input_allocate_device();
+	if (!context->qcitp_dev) {
+		pr_err("allocating memory failed\n");
+		err = -ENOMEM;
+		goto allocate_fail;
+	}
+	context->qcitp_dev->name       = TOUCHPAD_NAME;
+	context->qcitp_dev->phys       = TOUCHPAD_DEVICE;
+	context->qcitp_dev->id.bustype = BUS_I2C;
+	context->qcitp_dev->id.vendor  = 0x1050;
+	context->qcitp_dev->id.product = 0x1;
+	context->qcitp_dev->id.version = 0x1;
+	context->qcitp_dev->evbit[0]  = BIT_MASK(EV_KEY) |
+					BIT_MASK(EV_REL);
+	context->qcitp_dev->relbit[0] = BIT_MASK(REL_X) |
+					BIT_MASK(REL_Y);
+	context->qcitp_dev->keybit[BIT_WORD(BTN_LEFT)] = BIT_MASK(BTN_LEFT) |
+							 BIT_MASK(BTN_MIDDLE) |
+							 BIT_MASK(BTN_RIGHT);
+
+	input_set_drvdata(context->qcitp_dev, context);
+	context->qcitp_dev->open = qcitp_open;
+	err = input_register_device(context->qcitp_dev);
+	if (err) {
+		pr_err("register device failed\n");
+		goto register_fail;
+	}
+
+	err = gpio_request(context->qcitp_gpio, "qci-pad");
+	if (err) {
+		pr_err("gpio request failed\n");
+		goto gpio_request_fail;
+	}
+	gpio_direction_input(context->qcitp_gpio);
+
+	context->qcitp_irq = gpio_to_irq(context->qcitp_gpio);
+	err = request_threaded_irq(context->qcitp_irq, NULL, qcitp_interrupt,
+				   irq_trigger_type, client->name, context);
+	if (err) {
+		pr_err("request threaded irq failed\n");
+		goto request_irq_fail;
+	}
+
+	return 0;
+
+request_irq_fail:
+	gpio_free(context->qcitp_gpio);
+
+gpio_request_fail:
+	input_unregister_device(context->qcitp_dev);
+	context->qcitp_dev = NULL;
+
+register_fail:
+	input_free_device(context->qcitp_dev);
+
+allocate_fail:
+	kfree(context);
+	return err;
+}
+
+static int __devexit qcitp_remove(struct i2c_client *client)
+{
+	struct i2ctpad_drv_data *context = i2c_get_clientdata(client);
+
+	free_irq(context->qcitp_irq, context);
+	gpio_free(context->qcitp_gpio);
+	input_unregister_device(context->qcitp_dev);
+	kfree(context);
+
+	return 0;
+}
+
+static struct i2c_driver i2ctp_driver = {
+	.driver = {
+		.owner = THIS_MODULE,
+		.name  = TOUCHPAD_ID_NAME,
+#ifdef CONFIG_PM
+		.pm = &qcitp_pm_ops,
+#endif
+	},
+	.probe	  = qcitp_probe,
+	.remove	  = __devexit_p(qcitp_remove),
+	.id_table = qcitp_idtable,
+};
+
+static int __init qcitp_init(void)
+{
+	return i2c_add_driver(&i2ctp_driver);
+}
+
+
+static void __exit qcitp_exit(void)
+{
+	i2c_del_driver(&i2ctp_driver);
+}
+
+module_init(qcitp_init);
+module_exit(qcitp_exit);
+
+MODULE_AUTHOR("Quanta Computer Inc.");
+MODULE_DESCRIPTION("Quanta Embedded Controller I2C Touchpad Driver");
+MODULE_ALIAS("platform:qci-touchpad");
+MODULE_LICENSE("GPL v2");
diff --git a/include/linux/input/qci_touchpad.h b/include/linux/input/qci_touchpad.h
new file mode 100644
index 0000000..8e266e4
--- /dev/null
+++ b/include/linux/input/qci_touchpad.h
@@ -0,0 +1,25 @@
+/* Copyright (c) 2010, Code Aurora Forum. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
+ * 02110-1301, USA.
+ */
+
+#ifndef __QCI_TOUCHPAD_H
+#define __QCI_TOUCHPAD_H
+
+struct qci_touchpad_platform_data {
+	unsigned long irq_trigger_type;
+};
+
+#endif
-- 
1.7.0
--
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

Re: [PATCH v2] input: mouse: add qci touchpad driver

[Stepan Moskovchenko]

  On 8/12/2010 9:49 AM, Neil Leeder wrote:
> This driver is for the QCI trackpad used on Quanta smartbooks
>
> Signed-off-by: Horace Fu<horace.fu@quantatw.com>
> Signed-off-by: Mandeep Singh Baines<msb@chromium.org>
> [nleeder@codeaurora.org: cleanup i2c calls, address review comments etc]
> Signed-off-by: Neil Leeder<nleeder@codeaurora.org>
> ---
>   drivers/input/mouse/Kconfig        |   12 ++
>   drivers/input/mouse/Makefile       |    1 +
>   drivers/input/mouse/qci_touchpad.c |  270 ++++++++++++++++++++++++++++++++++++
>   include/linux/input/qci_touchpad.h |   25 ++++
>   4 files changed, 308 insertions(+), 0 deletions(-)
>   create mode 100644 drivers/input/mouse/qci_touchpad.c
>   create mode 100644 include/linux/input/qci_touchpad.h
>
> diff --git a/drivers/input/mouse/Kconfig b/drivers/input/mouse/Kconfig
> index c714ca2..32a88b4 100644
> --- a/drivers/input/mouse/Kconfig
> +++ b/drivers/input/mouse/Kconfig
> @@ -303,6 +303,18 @@ config MOUSE_MAPLE
>   	  To compile this driver as a module choose M here: the module will be
>   	  called maplemouse.
>
> +config MOUSE_QCITP
> +	tristate "Quanta Computer Inc. Touchpad"
> +	depends on I2C
> +	help
> +	  This driver supports the touchpad on Quanta smartbook devices.
> +
> +	  Say Y here if you have a Quanta-based smartboot or notepad
Sorry, very minor point - do you mean "smartbook" here?

A lot of machines are made by Quanta. Should this be given a description 
/ option name that is more specific, ie, "Quanta [chip name] Touchpad" 
or even "Quanta I2C Touchpad" ?  What you have might be fine, however...

Steve

Re: [PATCH v2] input: mouse: add qci touchpad driver

[Neil Leeder]

On 8/12/2010 1:25 PM, Stepan Moskovchenko wrote:
> On 8/12/2010 9:49 AM, Neil Leeder wrote:
>> +config MOUSE_QCITP
>> + tristate "Quanta Computer Inc. Touchpad"
>> + depends on I2C
>> + help
>> + This driver supports the touchpad on Quanta smartbook devices.
>> +
>> + Say Y here if you have a Quanta-based smartboot or notepad
> Sorry, very minor point - do you mean "smartbook" here?
>
> A lot of machines are made by Quanta. Should this be given a description
> / option name that is more specific, ie, "Quanta [chip name] Touchpad"
> or even "Quanta I2C Touchpad" ? What you have might be fine, however...
>

Good catch on the typo. I can update the Kconfig item to include I2C as 
you suggested.

--
Neil
-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

Re: [PATCH v2] input: mouse: add qci touchpad driver

[Dmitry Torokhov]

On Thu, Aug 12, 2010 at 01:58:18PM -0400, Neil Leeder wrote:
> On 8/12/2010 1:25 PM, Stepan Moskovchenko wrote:
> >On 8/12/2010 9:49 AM, Neil Leeder wrote:
> >>+config MOUSE_QCITP
> >>+ tristate "Quanta Computer Inc. Touchpad"
> >>+ depends on I2C
> >>+ help
> >>+ This driver supports the touchpad on Quanta smartbook devices.
> >>+
> >>+ Say Y here if you have a Quanta-based smartboot or notepad
> >Sorry, very minor point - do you mean "smartbook" here?
> >
> >A lot of machines are made by Quanta. Should this be given a description
> >/ option name that is more specific, ie, "Quanta [chip name] Touchpad"
> >or even "Quanta I2C Touchpad" ? What you have might be fine, however...
> >
> 
> Good catch on the typo. I can update the Kconfig item to include I2C
> as you suggested.
> 

Actually, since this is not a new touchpad but simply a PS/2 interface
it should be implemented as a serio driver, not input device driver.

Could you please tell me if the following works for you? Note that it
expects IRQ to be set up properly (edge vs. level trigger) by the
platform code

Thanks.

-- 
Dmitry

Input: wpce775x-serio - add driver for WPCE775x PS/2 interface

From: Dmitry Torokhov <dmitry.torokhov@gmail.com>

This is a driver for PS/2 interface part of Nuvoton WPCE775x family of
Advanced Embedded Controllers.

Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
---

 drivers/input/serio/Kconfig          |   12 +
 drivers/input/serio/Makefile         |    1 
 drivers/input/serio/wpce775x_serio.c |  377 ++++++++++++++++++++++++++++++++++
 3 files changed, 389 insertions(+), 1 deletions(-)
 create mode 100644 drivers/input/serio/wpce775x_serio.c


diff --git a/drivers/input/serio/Kconfig b/drivers/input/serio/Kconfig
index 3bfe8fa..259e550 100644
--- a/drivers/input/serio/Kconfig
+++ b/drivers/input/serio/Kconfig
@@ -215,7 +215,7 @@ config SERIO_AMS_DELTA
 	depends on MACH_AMS_DELTA
 	default y
 	select AMS_DELTA_FIQ
-	---help---
+	help
 	  Say Y here if you have an E3 and want to use its mailboard,
 	  or any standard AT keyboard connected to the mailboard port.
 
@@ -226,4 +226,14 @@ config SERIO_AMS_DELTA
 	  To compile this driver as a module, choose M here;
 	  the module will be called ams_delta_serio.
 
+config SERIO_WPCE775X
+	tristate "WPCE775x EC PS/2 interface support"
+	depends on I2C
+	help
+	  Say Y here if you have a system with Nuvoton WPCE775x Advanced
+	  Embedded Controller chip and want to use its PS/2 interface part.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called wpce775x_serio.
+
 endif
diff --git a/drivers/input/serio/Makefile b/drivers/input/serio/Makefile
index 84c80bf..e442550 100644
--- a/drivers/input/serio/Makefile
+++ b/drivers/input/serio/Makefile
@@ -24,3 +24,4 @@ obj-$(CONFIG_SERIO_RAW)		+= serio_raw.o
 obj-$(CONFIG_SERIO_AMS_DELTA)	+= ams_delta_serio.o
 obj-$(CONFIG_SERIO_XILINX_XPS_PS2)	+= xilinx_ps2.o
 obj-$(CONFIG_SERIO_ALTERA_PS2)	+= altera_ps2.o
+obj-$(CONFIG_SERIO_WPCE775X)	+= wpce775x_serio.o
diff --git a/drivers/input/serio/wpce775x_serio.c b/drivers/input/serio/wpce775x_serio.c
new file mode 100644
index 0000000..6c8dcee
--- /dev/null
+++ b/drivers/input/serio/wpce775x_serio.c
@@ -0,0 +1,377 @@
+/*
+ * Driver for PS/2 Interface part of WPCE775x Embedded Controller
+ *
+ * Copyright (c) 2010 Dmitry Torokhov
+ * Copyright (C) 2009 Quanta Computer Inc.
+ * Copyright (c) 2010, Code Aurora Forum. All rights reserved.
+ * Author: Hsin Wu <hsin.wu@quantatw.com>
+ * Author: Austin Lai <austin.lai@quantatw.com>
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/i2c.h>
+#include <linux/gpio.h>
+#include <linux/interrupt.h>
+#include <linux/workqueue.h>
+#include <linux/serio.h>
+#include <linux/delay.h>
+
+struct wpce775x_ps2if {
+	struct i2c_client *client;
+	struct serio *serio;
+	unsigned int gpio;
+	unsigned int irq;
+	struct work_struct recv_work;
+	struct work_struct send_work;
+	spinlock_t send_lock;
+	unsigned char data_in[16];
+	unsigned char data_queued[8];
+	unsigned char data_out[8];
+	unsigned int out_len;
+	struct mutex pm_mutex;
+	bool opened;		/* protected by pm_mutex */
+	bool suspended;		/* protected by pm_mutex */
+};
+
+static DEFINE_MUTEX(wpce775x_wq_mutex);
+static struct workqueue_struct *wpce775x_wq;
+static int wpce775x_count;
+
+static void wpce775x_recv(struct work_struct *work)
+{
+	struct wpce775x_ps2if *ps2if =
+			container_of(work, struct wpce775x_ps2if, recv_work);
+	int count;
+	int i;
+
+	count = i2c_master_recv(ps2if->client, ps2if->data_in,
+				sizeof(ps2if->data_in));
+	if (count < 0) {
+		dev_err(&ps2if->client->dev,
+			"failed to read data, error %d", count);
+	} else {
+		for (i = 0; i < count; i++)
+			serio_interrupt(ps2if->serio, ps2if->data_in[1], 0);
+	}
+}
+
+static void wpce775x_send(struct work_struct *work)
+{
+	struct wpce775x_ps2if *ps2if =
+			container_of(work, struct wpce775x_ps2if, send_work);
+	unsigned long flags;
+	unsigned int len;
+	int error;
+
+	spin_lock_irqsave(&ps2if->send_lock, flags);
+
+	len = ps2if->out_len;
+	memcpy(ps2if->data_queued, ps2if->data_out, len);
+	ps2if->out_len = 0;
+
+	spin_unlock_irqrestore(&ps2if->send_lock, flags);
+
+	error = i2c_master_send(ps2if->client, ps2if->data_queued, len);
+	if (error < 0)
+		dev_err(&ps2if->client->dev,
+			"failed to send data, error: %d", error);
+}
+
+static int wpce775x_write(struct serio *serio, unsigned char data)
+{
+	struct wpce775x_ps2if *ps2if = serio->port_data;
+	unsigned long flags;
+
+	spin_lock_irqsave(&ps2if->send_lock, flags);
+
+	ps2if->data_out[ps2if->out_len++] = data;
+	queue_work(wpce775x_wq, &ps2if->send_work);
+
+	spin_unlock_irqrestore(&ps2if->send_lock, flags);
+
+	return 0;
+}
+
+static irqreturn_t wpce775x_interrupt(int irq, void *dev_id)
+{
+	struct wpce775x_ps2if *ps2if = dev_id;
+
+	queue_work(wpce775x_wq, &ps2if->recv_work);
+
+	return IRQ_HANDLED;
+}
+
+static int wpce775x_start_wq(struct wpce775x_ps2if *ps2if)
+{
+	int retval;
+
+	retval = mutex_lock_interruptible(&wpce775x_wq_mutex);
+	if (retval)
+		return retval;
+
+	if (wpce775x_count == 0) {
+		wpce775x_wq = create_singlethread_workqueue("wpce775x-serio");
+		if (!wpce775x_wq) {
+			dev_err(&ps2if->client->dev,
+				"Failed to create wpce775x-serio workqueue\n");
+			retval = -ENOMEM;
+			goto out;
+		}
+	}
+
+	wpce775x_count++;
+
+out:
+	mutex_unlock(&wpce775x_wq_mutex);
+	return retval;
+}
+
+static void wpce775x_stop_wq(struct wpce775x_ps2if *ps2if)
+{
+	mutex_lock(&wpce775x_wq_mutex);
+
+	if (!--wpce775x_count)
+		destroy_workqueue(wpce775x_wq);
+
+	mutex_unlock(&wpce775x_wq_mutex);
+}
+
+static void wpce775x_enable(struct wpce775x_ps2if *ps2if)
+{
+	enable_irq(ps2if->irq);
+}
+
+static void wpce775x_disable(struct wpce775x_ps2if *ps2if)
+{
+	disable_irq(ps2if->irq);
+
+	cancel_work_sync(&ps2if->recv_work);
+	cancel_work_sync(&ps2if->send_work);
+}
+
+static int wpce775x_open(struct serio *serio)
+{
+	struct wpce775x_ps2if *ps2if = serio->port_data;
+	int retval;
+
+	retval = mutex_lock_interruptible(&ps2if->pm_mutex);
+	if (retval)
+		return retval;
+
+	retval = wpce775x_start_wq(ps2if);
+	if (retval)
+		goto out;
+
+	if (!ps2if->suspended)
+		wpce775x_enable(ps2if);
+
+	ps2if->opened = true;
+
+out:
+	mutex_unlock(&ps2if->pm_mutex);
+	return retval;
+}
+
+static void wpce775x_close(struct serio *serio)
+{
+	struct wpce775x_ps2if *ps2if = serio->port_data;
+
+	mutex_lock(&ps2if->pm_mutex);
+
+	if (!ps2if->suspended)
+		wpce775x_disable(ps2if);
+
+	wpce775x_stop_wq(ps2if);
+
+	ps2if->opened = false;
+
+	mutex_unlock(&ps2if->pm_mutex);
+}
+
+#ifdef CONFIG_PM
+static int wpce775x_suspend(struct device *dev)
+{
+	struct i2c_client *client = container_of(dev, struct i2c_client, dev);
+	struct wpce775x_ps2if *ps2if = i2c_get_clientdata(client);
+
+	mutex_lock(&ps2if->pm_mutex);
+
+	if (!ps2if->suspended && ps2if->opened)
+		wpce775x_disable(ps2if);
+
+	ps2if->suspended = true;
+
+	mutex_unlock(&ps2if->pm_mutex);
+
+	return 0;
+}
+
+static int wpce775x_resume(struct device *dev)
+{
+	struct i2c_client *client = container_of(dev, struct i2c_client, dev);
+	struct wpce775x_ps2if *ps2if = i2c_get_clientdata(client);
+
+	mutex_lock(&ps2if->pm_mutex);
+
+	if (ps2if->suspended && ps2if->opened)
+		wpce775x_enable(ps2if);
+
+	ps2if->suspended = false;
+
+	mutex_unlock(&ps2if->pm_mutex);
+
+	return 0;
+}
+
+static const struct dev_pm_ops wpce775x_pm_ops = {
+	.suspend  = wpce775x_suspend,
+	.resume   = wpce775x_resume,
+};
+
+#endif
+
+static int __devinit wpce775x_probe(struct i2c_client *client,
+				    const struct i2c_device_id *id)
+{
+	struct wpce775x_ps2if *ps2if;
+	struct serio *serio;
+	int irq;
+	int error;
+
+	if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
+		dev_err(&client->dev, "missing required i2c functionality\n");
+		return -ENODEV;
+	}
+
+	ps2if = kzalloc(sizeof(struct wpce775x_ps2if), GFP_KERNEL);
+	serio = kzalloc(sizeof(struct serio), GFP_KERNEL);
+	if (!ps2if || !serio) {
+		error = -ENOMEM;
+		goto err_free_mem;
+	}
+
+	serio->id.type = SERIO_8042;
+	serio->write = wpce775x_write;
+	serio->open = wpce775x_open;
+	serio->close = wpce775x_close;
+	serio->port_data = ps2if;
+	serio->dev.parent = &client->dev;
+
+	strlcpy(serio->name, "WPCE775x PS/2 Interface", sizeof(serio->name));
+
+	ps2if->client = client;
+	ps2if->gpio = client->irq;
+	ps2if->serio = serio;
+
+	INIT_WORK(&ps2if->send_work, wpce775x_send);
+	INIT_WORK(&ps2if->recv_work, wpce775x_recv);
+	spin_lock_init(&ps2if->send_lock);
+	mutex_init(&ps2if->pm_mutex);
+
+	error = gpio_request(ps2if->gpio, "wpce775x-serio");
+	if (error) {
+		dev_err(&client->dev,
+			"Failed to request GPIO %d\n", ps2if->gpio);
+		goto err_free_mem;
+	}
+
+	error = gpio_direction_input(ps2if->gpio);
+	if (error) {
+		dev_err(&client->dev,
+			"Failed to configure GPIO %d as input\n", ps2if->gpio);
+		goto err_free_gpio;
+	}
+
+	irq = gpio_to_irq(ps2if->gpio);
+	if (irq < 0) {
+		error = irq;
+		dev_err(&client->dev,
+			"Unable to get irq number for GPIO %d\n", ps2if->gpio);
+		goto err_free_gpio;
+	}
+
+	ps2if->irq = irq;
+
+	error = request_irq(ps2if->irq, wpce775x_interrupt, 0,
+				client->name, ps2if);
+	if (error) {
+		dev_err(&client->dev, "Unable to claim IRQ %d\n", ps2if->irq);
+		goto err_free_gpio;
+	}
+
+	disable_irq(client->irq);
+
+	dev_info(&client->dev, "WPCE775x PS/2 interface, irq %d\n", ps2if->irq);
+
+	serio_register_port(ps2if->serio);
+	i2c_set_clientdata(client, ps2if);
+
+	return 0;
+
+err_free_gpio:
+	gpio_free(ps2if->gpio);
+err_free_mem:
+	kfree(serio);
+	kfree(ps2if);
+	return error;
+}
+
+static int __devexit wpce775x_remove(struct i2c_client *client)
+{
+	struct wpce775x_ps2if *ps2if = i2c_get_clientdata(client);
+
+	serio_unregister_port(ps2if->serio);
+	free_irq(ps2if->irq, ps2if);
+	gpio_free(ps2if->gpio);
+	kfree(ps2if);
+
+	return 0;
+}
+
+static const struct i2c_device_id wpce775x_idtable[] = {
+	{ "wpce775x-ps2" , 0 },
+	{ }
+};
+
+MODULE_DEVICE_TABLE(i2c, wpce775x_idtable);
+
+static struct i2c_driver i2ctp_driver = {
+	.driver = {
+		.owner = THIS_MODULE,
+		.name  = "wpce775x-ps2",
+#ifdef CONFIG_PM
+		.pm = &wpce775x_pm_ops,
+#endif
+	},
+	.probe	  = wpce775x_probe,
+	.remove	  = __devexit_p(wpce775x_remove),
+	.id_table = wpce775x_idtable,
+};
+
+static int __init wpce775x_init(void)
+{
+	return i2c_add_driver(&i2ctp_driver);
+}
+module_init(wpce775x_init);
+
+static void __exit wpce775x_exit(void)
+{
+	i2c_del_driver(&i2ctp_driver);
+}
+module_exit(wpce775x_exit);
+
+MODULE_AUTHOR("Dmitry Torokhov <dtor@mail.ru>");
+MODULE_DESCRIPTION("Nuvoton WPCE775x Embedded Controller driver (PS/2 interface part)");
+MODULE_LICENSE("GPL v2");

RE: [PATCH v2] input: mouse: add qci touchpad driver

[Datta, Shubhrajyoti]

Minor comments feel free to ignore if you feel so.

> -----Original Message-----
> From: linux-input-owner@vger.kernel.org [mailto:linux-input-
> owner@vger.kernel.org] On Behalf Of Neil Leeder
> Sent: Thursday, August 12, 2010 10:19 PM
> To: Dmitry Torokhov
> Cc: linux-input@vger.kernel.org; linux-arm-msm@vger.kernel.org; linux-
> kernel@vger.kernel.org; Neil Leeder; Horace Fu; Mandeep Singh Baines
> Subject: [PATCH v2] input: mouse: add qci touchpad driver
> 
> This driver is for the QCI trackpad used on Quanta smartbooks
Would you like to mention the part number.
> 
> Signed-off-by: Horace Fu <horace.fu@quantatw.com>
> Signed-off-by: Mandeep Singh Baines <msb@chromium.org>
> [nleeder@codeaurora.org: cleanup i2c calls, address review comments etc]
> Signed-off-by: Neil Leeder <nleeder@codeaurora.org>
> ---
>  drivers/input/mouse/Kconfig        |   12 ++
>  drivers/input/mouse/Makefile       |    1 +
>  drivers/input/mouse/qci_touchpad.c |  270
> ++++++++++++++++++++++++++++++++++++
>  include/linux/input/qci_touchpad.h |   25 ++++
>  4 files changed, 308 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/input/mouse/qci_touchpad.c
>  create mode 100644 include/linux/input/qci_touchpad.h
> 
> diff --git a/drivers/input/mouse/Kconfig b/drivers/input/mouse/Kconfig
> index c714ca2..32a88b4 100644
> --- a/drivers/input/mouse/Kconfig
> +++ b/drivers/input/mouse/Kconfig
> @@ -303,6 +303,18 @@ config MOUSE_MAPLE
>  	  To compile this driver as a module choose M here: the module will
> be
>  	  called maplemouse.
> 
> +config MOUSE_QCITP
> +	tristate "Quanta Computer Inc. Touchpad"
> +	depends on I2C
> +	help
> +	  This driver supports the touchpad on Quanta smartbook devices.
> +
> +	  Say Y here if you have a Quanta-based smartboot or notepad
> +	  device and want to use the Quanta touchpad driver.
> +
> +	  To compile this driver as a module, choose M here: the
> +	  module will be called qci_touchpad.
> +
>  config MOUSE_SYNAPTICS_I2C
>  	tristate "Synaptics I2C Touchpad support"
>  	depends on I2C
> diff --git a/drivers/input/mouse/Makefile b/drivers/input/mouse/Makefile
> index 570c84a..6eda35d 100644
> --- a/drivers/input/mouse/Makefile
> +++ b/drivers/input/mouse/Makefile
> @@ -15,6 +15,7 @@ obj-$(CONFIG_MOUSE_MAPLE)		+= maplemouse.o
>  obj-$(CONFIG_MOUSE_PC110PAD)		+= pc110pad.o
>  obj-$(CONFIG_MOUSE_PS2)			+= psmouse.o
>  obj-$(CONFIG_MOUSE_PXA930_TRKBALL)	+= pxa930_trkball.o
> +obj-$(CONFIG_MOUSE_QCITP)		+= qci_touchpad.o
>  obj-$(CONFIG_MOUSE_RISCPC)		+= rpcmouse.o
>  obj-$(CONFIG_MOUSE_SERIAL)		+= sermouse.o
>  obj-$(CONFIG_MOUSE_SYNAPTICS_I2C)	+= synaptics_i2c.o
> diff --git a/drivers/input/mouse/qci_touchpad.c
> b/drivers/input/mouse/qci_touchpad.c
> new file mode 100644
> index 0000000..746cbaa
> --- /dev/null
> +++ b/drivers/input/mouse/qci_touchpad.c
> @@ -0,0 +1,270 @@
> +/* Quanta I2C Touchpad Driver
> + *
> + * Copyright (C) 2009 Quanta Computer Inc.
> + * Copyright (c) 2010, Code Aurora Forum. All rights reserved.
> + * Author: Hsin Wu <hsin.wu@quantatw.com>
> + * Author: Austin Lai <austin.lai@quantatw.com>
> + *
> + * This software is licensed under the terms of the GNU General Public
> + * License version 2, as published by the Free Software Foundation, and
> + * may be copied, distributed, and modified under those terms.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + */
> +
> +/*
> + * Driver communicates over i2c to nuvoTon WPCE775x Embedded Controller,
> + * which has touchpad attached through PS/2 interface.
> + */
> +#define pr_fmt(fmt) KBUILD_BASENAME ": " fmt
> +
> +#include <linux/kernel.h>
> +#include <linux/init.h>
> +#include <linux/module.h>
> +#include <linux/slab.h>
> +#include <linux/i2c.h>
> +#include <linux/interrupt.h>
> +#include <linux/input.h>
> +#include <linux/gpio.h>
> +#include <linux/delay.h>
> +#include <linux/input/qci_touchpad.h>
> +
> +#define TOUCHPAD_ID_NAME          "qci-i2cpad"
> +#define TOUCHPAD_NAME             "QCI Touchpad"
> +#define TOUCHPAD_DEVICE           "/qci_touchpad/input0"
> +#define TOUCHPAD_CMD_ENABLE       0xF4
> +#define TOUCHPAD_READ_DATA_LEN    3
> +
> +struct i2ctpad_drv_data {
> +	struct i2c_client *ti2c_client;
> +	struct input_dev *qcitp_dev;
> +	unsigned int qcitp_gpio;
> +	unsigned int qcitp_irq;
> +	char ecdata[8];
> +	bool opened;
> +};
> +
> +#ifdef CONFIG_PM
> +static int qcitp_suspend(struct device *_dev)
> +{
> +	struct i2c_client *client =
> +		container_of(_dev, struct i2c_client, dev);
> +	struct i2ctpad_drv_data *context = i2c_get_clientdata(client);
> +
> +	disable_irq(context->qcitp_irq);
> +	return 0;
> +}
> +
> +static int qcitp_resume(struct device *_dev)
> +{
> +	struct i2c_client *client =
> +		container_of(_dev, struct i2c_client, dev);
> +	struct i2ctpad_drv_data *context = i2c_get_clientdata(client);
> +
> +	enable_irq(context->qcitp_irq);
> +	return 0;
> +}
> +#endif
> +
> +static const struct i2c_device_id qcitp_idtable[] = {
> +	{ "wpce775-touchpad", 0 },
> +	{ }
> +};
> +
> +MODULE_DEVICE_TABLE(i2c, qcitp_idtable);
> +
> +#ifdef CONFIG_PM
> +static const struct dev_pm_ops qcitp_pm_ops = {
> +	.suspend  = qcitp_suspend,
> +	.resume   = qcitp_resume,
> +};
> +#endif
> +
> +static void qcitp_report_key(struct input_dev *tpad_dev, char *ec_data)
> +{
> +	int dx = 0;
> +	int dy = 0;
> +
> +	if (ec_data[1])
> +		dx = (int) ec_data[1] -
> +		     (int) ((ec_data[0] << 4) & 0x100);
> +
> +	if (ec_data[2])
> +		dy = (int) ((ec_data[0] << 3) & 0x100) -
> +		     (int) ec_data[2];
> +
> +	input_report_key(tpad_dev, BTN_LEFT, ec_data[0] & 0x01);
> +	input_report_key(tpad_dev, BTN_RIGHT, ec_data[0] & 0x02);
> +	input_report_key(tpad_dev, BTN_MIDDLE, ec_data[0] & 0x04);
> +	input_report_rel(tpad_dev, REL_X, dx);
> +	input_report_rel(tpad_dev, REL_Y, dy);
> +	input_sync(tpad_dev);
> +}
> +
> +static irqreturn_t qcitp_interrupt(int irq, void *dev_id)
> +{
> +	struct i2ctpad_drv_data *context = dev_id;
> +	int rc;
> +
> +	rc = i2c_master_recv(context->ti2c_client, context->ecdata,
> +			     TOUCHPAD_READ_DATA_LEN);
> +	if (rc == TOUCHPAD_READ_DATA_LEN)
> +		qcitp_report_key(context->qcitp_dev, context->ecdata);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static int qcitp_open(struct input_dev *input)
> +{
> +	struct i2ctpad_drv_data *context = input_get_drvdata(input);
> +	u8 buf[1];
> +	int err = 0;
> +
> +	if (context->opened)
> +		return 0;
> +	context->opened = 1;
> +
> +	buf[0] = TOUCHPAD_CMD_ENABLE;
> +	err = i2c_master_send(context->ti2c_client, buf, 1);
> +	if (err < 0)
> +		goto i2c_fail;
> +	msleep(100);
Could you explain the time or have a #define XXX 

> +	err = i2c_master_recv(context->ti2c_client, buf, 1);
> +
> +i2c_fail:
> +	return err;
> +}
> +
> +static int __devinit qcitp_probe(struct i2c_client *client,
> +				    const struct i2c_device_id *id)
> +{
> +	int err;
> +	struct i2ctpad_drv_data *context;
> +	struct qci_touchpad_platform_data *pd;
> +	int irq_trigger_type;
> +
> +	if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
> +		pr_err("i2c functionality failed\n");
> +		return -ENODEV;
> +	}
> +
> +	context = kzalloc(sizeof(struct i2ctpad_drv_data), GFP_KERNEL);
> +	if (!context)
> +		return -ENOMEM;
> +	i2c_set_clientdata(client, context);
> +	context->ti2c_client = client;
> +	context->qcitp_gpio = client->irq;
> +
> +	pd = client->dev.platform_data;
> +	if (pd)
> +		irq_trigger_type = pd->irq_trigger_type;
> +	else
> +		irq_trigger_type = IRQF_TRIGGER_FALLING;
> +
> +	context->qcitp_dev = input_allocate_device();
> +	if (!context->qcitp_dev) {
> +		pr_err("allocating memory failed\n");
> +		err = -ENOMEM;
> +		goto allocate_fail;
> +	}
> +	context->qcitp_dev->name       = TOUCHPAD_NAME;
> +	context->qcitp_dev->phys       = TOUCHPAD_DEVICE;
> +	context->qcitp_dev->id.bustype = BUS_I2C;
> +	context->qcitp_dev->id.vendor  = 0x1050;
> +	context->qcitp_dev->id.product = 0x1;
> +	context->qcitp_dev->id.version = 0x1;
> +	context->qcitp_dev->evbit[0]  = BIT_MASK(EV_KEY) |
> +					BIT_MASK(EV_REL);
> +	context->qcitp_dev->relbit[0] = BIT_MASK(REL_X) |
> +					BIT_MASK(REL_Y);
> +	context->qcitp_dev->keybit[BIT_WORD(BTN_LEFT)] = BIT_MASK(BTN_LEFT)
> |
> +							 BIT_MASK(BTN_MIDDLE) |
> +							 BIT_MASK(BTN_RIGHT);
> +
> +	input_set_drvdata(context->qcitp_dev, context);
> +	context->qcitp_dev->open = qcitp_open;
> +	err = input_register_device(context->qcitp_dev);
> +	if (err) {
> +		pr_err("register device failed\n");
> +		goto register_fail;
> +	}
> +
> +	err = gpio_request(context->qcitp_gpio, "qci-pad");
> +	if (err) {
> +		pr_err("gpio request failed\n");
> +		goto gpio_request_fail;
> +	}
> +	gpio_direction_input(context->qcitp_gpio);
The result is not checked here.

> +
> +	context->qcitp_irq = gpio_to_irq(context->qcitp_gpio);
> +	err = request_threaded_irq(context->qcitp_irq, NULL,
> qcitp_interrupt,
> +				   irq_trigger_type, client->name, context);
> +	if (err) {
> +		pr_err("request threaded irq failed\n");
> +		goto request_irq_fail;
> +	}
> +
> +	return 0;
> +
> +request_irq_fail:
> +	gpio_free(context->qcitp_gpio);
> +
> +gpio_request_fail:
> +	input_unregister_device(context->qcitp_dev);
> +	context->qcitp_dev = NULL;
> +
> +register_fail:
> +	input_free_device(context->qcitp_dev);
You may like to revisit the use of free after unregister.

> +
> +allocate_fail:
> +	kfree(context);
> +	return err;
> +}
> +
> +static int __devexit qcitp_remove(struct i2c_client *client)
> +{
> +	struct i2ctpad_drv_data *context = i2c_get_clientdata(client);
> +
> +	free_irq(context->qcitp_irq, context);
> +	gpio_free(context->qcitp_gpio);
> +	input_unregister_device(context->qcitp_dev);
> +	kfree(context);
> +
> +	return 0;
> +}
> +
> +static struct i2c_driver i2ctp_driver = {
> +	.driver = {
> +		.owner = THIS_MODULE,
> +		.name  = TOUCHPAD_ID_NAME,
> +#ifdef CONFIG_PM
> +		.pm = &qcitp_pm_ops,
> +#endif
> +	},
> +	.probe	  = qcitp_probe,
> +	.remove	  = __devexit_p(qcitp_remove),
> +	.id_table = qcitp_idtable,
> +};
> +
> +static int __init qcitp_init(void)
> +{
> +	return i2c_add_driver(&i2ctp_driver);
> +}
> +
> +
> +static void __exit qcitp_exit(void)
> +{
> +	i2c_del_driver(&i2ctp_driver);
> +}
> +
> +module_init(qcitp_init);
> +module_exit(qcitp_exit);
> +
> +MODULE_AUTHOR("Quanta Computer Inc.");
> +MODULE_DESCRIPTION("Quanta Embedded Controller I2C Touchpad Driver");
> +MODULE_ALIAS("platform:qci-touchpad");
> +MODULE_LICENSE("GPL v2");
> diff --git a/include/linux/input/qci_touchpad.h
> b/include/linux/input/qci_touchpad.h
> new file mode 100644
> index 0000000..8e266e4
> --- /dev/null
> +++ b/include/linux/input/qci_touchpad.h
> @@ -0,0 +1,25 @@
> +/* Copyright (c) 2010, Code Aurora Forum. All rights reserved.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 and
> + * only version 2 as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
> + * 02110-1301, USA.
> + */
> +
> +#ifndef __QCI_TOUCHPAD_H
> +#define __QCI_TOUCHPAD_H
> +
> +struct qci_touchpad_platform_data {
> +	unsigned long irq_trigger_type;
> +};
> +
> +#endif
> --
> 1.7.0
> --
> Sent by an employee of the Qualcomm Innovation Center, Inc.
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
> --
> 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 v2] input: mouse: add qci touchpad driver

[Trilok Soni]

Hi Shubhrajyoti,

>> +
>> +gpio_request_fail:
>> +	input_unregister_device(context->qcitp_dev);
>> +	context->qcitp_dev = NULL;
>> +
>> +register_fail:
>> +	input_free_device(context->qcitp_dev);
> You may like to revisit the use of free after unregister.
> 

That looks OK, because qcitp_dev is set to NULL before this call.

---Trilok Soni

-- 
Sent by a consultant of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

Re: [PATCH v2] input: mouse: add qci touchpad driver

[Neil Leeder]

On 8/12/2010 10:49 PM, Dmitry Torokhov wrote:
> On Thu, Aug 12, 2010 at 01:58:18PM -0400, Neil Leeder wrote:
> Actually, since this is not a new touchpad but simply a PS/2 interface
> it should be implemented as a serio driver, not input device driver.
>
Dmitri,

Thanks for supplying that serio driver. I just have a couple of questions.

Even though the interface on the wpce775x EC happens to be a PS/2 
interface, this is completely hidden by the firmware on that device. To 
the linux driver it looks like a dedicated i2c connection directly to 
the touchpad. You can't substitute any other device on that PS/2 
interface without rewriting the firmware in the EC - it's not a generic 
interface.  A manufacturer could even move the touchpad from the PS/2 
interface to say GPIOs, re-write the firmware and the linux driver 
couldn't tell the difference. Does that change the rationale for using a 
serio driver?

If the request to use a serio driver is still valid, then it seems that 
the workqueue from the interrupt handler sends each byte of data 
received over i2c in a separate serio_interrupt() call to the touchpad 
driver. Touchpad data comes in 3-byte packets, so the touchpad driver 
will have to re-assemble the packet from 3 separate interrupts. Is that 
the intended use?

> Could you please tell me if the following works for you? Note that it
> expects IRQ to be set up properly (edge vs. level trigger) by the
> platform code

The i2c_board_info that supplies the irq # from platform code doesn't 
have a way to set i2c flags, and can't set_irq_type() until after 
irq_request(). This may require another platform_data struct to pass the 
trigger level in.

Thanks.

--
Neil
-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

Re: [PATCH v2] input: mouse: add qci touchpad driver

[Dmitry Torokhov]

On Friday, August 13, 2010 02:56:41 pm Neil Leeder wrote:
> On 8/12/2010 10:49 PM, Dmitry Torokhov wrote:
> > On Thu, Aug 12, 2010 at 01:58:18PM -0400, Neil Leeder wrote:
> > Actually, since this is not a new touchpad but simply a PS/2 interface
> > it should be implemented as a serio driver, not input device driver.
> 
> Dmitri,
> 
> Thanks for supplying that serio driver. I just have a couple of questions.
> 
> Even though the interface on the wpce775x EC happens to be a PS/2
> interface, this is completely hidden by the firmware on that device. To
> the linux driver it looks like a dedicated i2c connection directly to
> the touchpad. You can't substitute any other device on that PS/2
> interface without rewriting the firmware in the EC - it's not a generic
> interface.  A manufacturer could even move the touchpad from the PS/2
> interface to say GPIOs, re-write the firmware and the linux driver
> couldn't tell the difference.

Hmm, according to the following diagram from  Nuvoton trhe chip does in
fact has a distinct PS/2 interface (3 of them actually):

http://www.nuvoton.com/NuvotonMOSS/Community/ProductInfo.aspx?tp_GUID=d2a1e761-c93d-4bf0-b292-acb80b38cfaf

Also it is not a simple coincidence that to enable device you send 0xf4
to it (which is PSMOUSE_CMD_ENABLE - standard PS/2 command). This tends
to suggest that interface is not actually hidden and that the device
(touchpad) could be replaced with other kinds of devices.

Anyway, please try the driver (you  may need to hardcode the IRQ trigger
type for now) and see if psmouse is able to talk to the touchpad. If it
is then serio is the proper solution.

> Does that change the rationale for using a
> serio driver?
> 
> If the request to use a serio driver is still valid, then it seems that
> the workqueue from the interrupt handler sends each byte of data
> received over i2c in a separate serio_interrupt() call to the touchpad
> driver. Touchpad data comes in 3-byte packets, so the touchpad driver
> will have to re-assemble the packet from 3 separate interrupts. Is that
> the intended use?

Yes, serio ports are byte-oriented devices. psmouse and atkbd drivers should
be able to work with such data.

Thanks.

-- 
Dmitry

Re: [PATCH v2] input: mouse: add qci touchpad driver

[Neil Leeder]

On 8/13/2010 8:54 PM, Dmitry Torokhov wrote:
> Also it is not a simple coincidence that to enable device you send 0xf4
> to it (which is PSMOUSE_CMD_ENABLE - standard PS/2 command). This tends
> to suggest that interface is not actually hidden and that the device
> (touchpad) could be replaced with other kinds of devices.
>
> Anyway, please try the driver (you  may need to hardcode the IRQ trigger
> type for now) and see if psmouse is able to talk to the touchpad. If it
> is then serio is the proper solution.

Dmitri,
I fixed one line in the wpce775x_serio driver which looked like a typo - 
hope I got that right:

	for (i = 0; i < count; i++)
-		serio_interrupt(ps2if->serio, ps2if->data_in[1], 0);
+		serio_interrupt(ps2if->serio, ps2if->data_in[i], 0);
	}

I tried running with psmouse but it didn't work. The touchpad was never 
detected as a synaptics touchpad. I looked at the dataflow from the 
device and it wasn't responding to the commands that synaptics_detect() 
was sending it. It eventually showed up as an unknown logitech mouse. I 
can see data being passed through the serio driver but the logitech 
driver can't process it.

--
Neil
-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

Re: [PATCH v2] input: mouse: add qci touchpad driver

[Dmitry Torokhov]

Hi Neil,

On Aug 17, 2010, at 10:14 AM, Neil Leeder <nleeder@codeaurora.org>  
wrote:

> On 8/13/2010 8:54 PM, Dmitry Torokhov wrote:
>> Also it is not a simple coincidence that to enable device you send  
>> 0xf4
>> to it (which is PSMOUSE_CMD_ENABLE - standard PS/2 command). This  
>> tends
>> to suggest that interface is not actually hidden and that the device
>> (touchpad) could be replaced with other kinds of devices.
>>
>> Anyway, please try the driver (you  may need to hardcode the IRQ  
>> trigger
>> type for now) and see if psmouse is able to talk to the touchpad.  
>> If it
>> is then serio is the proper solution.
>
> Dmitri,
> I fixed one line in the wpce775x_serio driver which looked like a  
> typo - hope I got that right:
>
>    for (i = 0; i < count; i++)
> -        serio_interrupt(ps2if->serio, ps2if->data_in[1], 0);
> +        serio_interrupt(ps2if->serio, ps2if->data_in[i], 0);
>    }
>

Yep, that was a typo.

> I tried running with psmouse but it didn't work. The touchpad was  
> never detected as a synaptics touchpad. I looked at the dataflow  
> from the device and it wasn't responding to the commands that  
> synaptics_detect() was sending it. It eventually showed up as an  
> unknown logitech mouse.


That suggests that the device responds to at least basic PS/2 queries.

> I can see data being passed through the serio driver but the  
> logitech driver can't process it.
>

What about loading psmouse module with proto=bare?

Could you also dump the data received from touchpad during probing?


-- 
Dmitry

Re: [PATCH v2] input: mouse: add qci touchpad driver

[Neil Leeder]

Hi Dmitry,

On 8/17/2010 2:05 PM, Dmitry Torokhov wrote:
> That suggests that the device responds to at least basic PS/2 queries.
>
>> I can see data being passed through the serio driver but the logitech
>> driver can't process it.
>>
>
> What about loading psmouse module with proto=bare?
>
> Could you also dump the data received from touchpad during probing?
>

It appears the controller does somewhat respond as a PS/2 device. The 
first problem is it only responds to CMD_GETID correctly about 50% of 
the time. Sometimes it responds well with a 0x00, other times with 0xfa, 
which causes psmouse_probe to not recognize it as a pointing device.

[    1.037223] wpce775x_send len=1 data=f2
[    1.054412] wpce775x_recv ... data_in=fa fa 64 0 0 0 0 0 fa 0 0 0 0 0 
0 0

It also doesn't respond well to CMD_GETINFO, which is why it gets 
mis-detected as the wrong device type. As an aside, I'm not sure if 
combining separate commands into one i2c packet is correct, but 
separating them didn't seem to produce better results either.

[    1.723943] wpce775x_send len=3 data=e8 3 e6
[    1.735666] wpce775x_recv ... data_in=fa fa 64 0 0 0 0 0 fa 0 0 0 0 0 
0 0
[    1.735892] wpce775x_send len=3 data=e6 e6 e9
[    1.748281] wpce775x_recv ... data_in=fe fa 64 0 0 0 0 0 fa 0 0 0 0 0 
0 0
[    1.748500] wpce775x_send len=3 data=e8 0 e6
[    1.760233] wpce775x_recv ... data_in=fa fa 64 0 0 0 0 0 fa 0 0 0 0 0 
0 0
[    1.760457] wpce775x_send len=3 data=e6 e6 e9
[    1.772876] wpce775x_recv ... data_in=fe fa 64 0 0 0 0 0 fa 0 0 0 0 0 
0 0

Using proto=bare gets around the GETINFO failure, but doesn't help with 
the more important GETID failure.

The other issue is that when the serio driver requests up to 16 bytes 
over i2c, the controller responds with 16 bytes of data, which appear to 
be the 3 flags/X/Y regs plus 13 bytes of irrelevant data. This all gets 
passed to psmouse which interprets it 3 bytes at a time as movement 
data. Not only does the extra data get interpreted as movement info, but 
it causes subsequent real data to be mis-aligned and wrongly interpreted.

[  115.915558] wpce775x_recv ... data_in=28 1 ff 0 0 0 0 0 9c 0 0 0 0 0 0 0
[  115.915649] psmouse_process_byte data=28 01 ff rel_x=1 rel_y=1 
<--- OK
[  115.915688] psmouse_process_byte data=00 00 00 rel_x=0 rel_y=0
[  115.915714] psmouse_process_byte data=00 00 9c rel_x=0 rel_y=-156 
<--- bad
[  115.915745] psmouse_process_byte data=00 00 00 rel_x=0 rel_y=0
[  115.915771] psmouse_process_byte data=00 00 00 rel_x=0 rel_y=0
[  115.928269] wpce775x_recv ... data_in=28 1 fe 0 0 0 0 0 9c 0 0 0 0 0 0 0
[  115.928357] psmouse_process_byte data=00 28 01 rel_x=40 rel_y=-1 
<--- wrong
[  115.928396] psmouse_process_byte data=fe 00 00 rel_x=0 rel_y=0 
<--- wrong
[  115.928426] psmouse_process_byte data=00 00 00 rel_x=0 rel_y=0
[  115.928457] psmouse_process_byte data=9c 00 00 rel_x=0 rel_y=0
[  115.928484] psmouse_process_byte data=00 00 00 rel_x=0 rel_y=0
[  115.939728] wpce775x_recv ... data_in=28 1 ff 0 0 0 0 0 9c 0 0 0 0 0 0 0
[  115.939818] psmouse_process_byte data=00 00 28 rel_x=0 rel_y=-40 
<--- wrong
[  115.939857] psmouse_process_byte data=01 ff 00 rel_x=255 rel_y=0 
<--- wrong
[  115.939889] psmouse_process_byte data=00 00 00 rel_x=0 rel_y=0
[  115.939916] psmouse_process_byte data=00 9c 00 rel_x=156 rel_y=0 
<--- bad
[  115.939947] psmouse_process_byte data=00 00 00 rel_x=0 rel_y=0
[  115.939972] psmouse_process_byte data=00 00 00 rel_x=0 rel_y=0

Is there a way to get the serio driver to only read 3 bytes of data per 
interrupt?

Thanks.

--
Neil
-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

Re: [PATCH v2] input: mouse: add qci touchpad driver

[Dmitry Torokhov]

On Wed, Aug 18, 2010 at 05:38:38PM -0400, Neil Leeder wrote:
> Hi Dmitry,
> 
> On 8/17/2010 2:05 PM, Dmitry Torokhov wrote:
> >That suggests that the device responds to at least basic PS/2 queries.
> >
> >>I can see data being passed through the serio driver but the logitech
> >>driver can't process it.
> >>
> >
> >What about loading psmouse module with proto=bare?
> >
> >Could you also dump the data received from touchpad during probing?
> >
> 
> It appears the controller does somewhat respond as a PS/2 device.
> The first problem is it only responds to CMD_GETID correctly about
> 50% of the time. Sometimes it responds well with a 0x00, other times
> with 0xfa, which causes psmouse_probe to not recognize it as a
> pointing device.
> 
> [    1.037223] wpce775x_send len=1 data=f2
> [    1.054412] wpce775x_recv ... data_in=fa fa 64 0 0 0 0 0 fa 0 0 0
> 0 0 0 0
> 
> It also doesn't respond well to CMD_GETINFO, which is why it gets
> mis-detected as the wrong device type. As an aside, I'm not sure if
> combining separate commands into one i2c packet is correct, but
> separating them didn't seem to produce better results either.
> 
> [    1.723943] wpce775x_send len=3 data=e8 3 e6
> [    1.735666] wpce775x_recv ... data_in=fa fa 64 0 0 0 0 0 fa 0 0 0
> 0 0 0 0
> [    1.735892] wpce775x_send len=3 data=e6 e6 e9
> [    1.748281] wpce775x_recv ... data_in=fe fa 64 0 0 0 0 0 fa 0 0 0
> 0 0 0 0
> [    1.748500] wpce775x_send len=3 data=e8 0 e6
> [    1.760233] wpce775x_recv ... data_in=fa fa 64 0 0 0 0 0 fa 0 0 0
> 0 0 0 0
> [    1.760457] wpce775x_send len=3 data=e6 e6 e9
> [    1.772876] wpce775x_recv ... data_in=fe fa 64 0 0 0 0 0 fa 0 0 0
> 0 0 0 0
> 
> Using proto=bare gets around the GETINFO failure, but doesn't help
> with the more important GETID failure.

Does it help if you change write() to transmit (and read) 1 byte at a time?

> 
> The other issue is that when the serio driver requests up to 16
> bytes over i2c, the controller responds with 16 bytes of data, which
> appear to be the 3 flags/X/Y regs plus 13 bytes of irrelevant data.
> This all gets passed to psmouse which interprets it 3 bytes at a
> time as movement data. Not only does the extra data get interpreted
> as movement info, but it causes subsequent real data to be
> mis-aligned and wrongly interpreted.
> 
> [  115.915558] wpce775x_recv ... data_in=28 1 ff 0 0 0 0 0 9c 0 0 0 0 0 0 0
> [  115.915649] psmouse_process_byte data=28 01 ff rel_x=1 rel_y=1
> <--- OK
> [  115.915688] psmouse_process_byte data=00 00 00 rel_x=0 rel_y=0
> [  115.915714] psmouse_process_byte data=00 00 9c rel_x=0 rel_y=-156
> <--- bad
> [  115.915745] psmouse_process_byte data=00 00 00 rel_x=0 rel_y=0
> [  115.915771] psmouse_process_byte data=00 00 00 rel_x=0 rel_y=0
> [  115.928269] wpce775x_recv ... data_in=28 1 fe 0 0 0 0 0 9c 0 0 0 0 0 0 0
> [  115.928357] psmouse_process_byte data=00 28 01 rel_x=40 rel_y=-1
> <--- wrong
> [  115.928396] psmouse_process_byte data=fe 00 00 rel_x=0 rel_y=0
> <--- wrong
> [  115.928426] psmouse_process_byte data=00 00 00 rel_x=0 rel_y=0
> [  115.928457] psmouse_process_byte data=9c 00 00 rel_x=0 rel_y=0
> [  115.928484] psmouse_process_byte data=00 00 00 rel_x=0 rel_y=0
> [  115.939728] wpce775x_recv ... data_in=28 1 ff 0 0 0 0 0 9c 0 0 0 0 0 0 0
> [  115.939818] psmouse_process_byte data=00 00 28 rel_x=0 rel_y=-40
> <--- wrong
> [  115.939857] psmouse_process_byte data=01 ff 00 rel_x=255 rel_y=0
> <--- wrong
> [  115.939889] psmouse_process_byte data=00 00 00 rel_x=0 rel_y=0
> [  115.939916] psmouse_process_byte data=00 9c 00 rel_x=156 rel_y=0
> <--- bad
> [  115.939947] psmouse_process_byte data=00 00 00 rel_x=0 rel_y=0
> [  115.939972] psmouse_process_byte data=00 00 00 rel_x=0 rel_y=0
> 
> Is there a way to get the serio driver to only read 3 bytes of data
> per interrupt?
> 

One way would be to look for PSMOUSE_CMD_ENABLE/PSMOUSE_CMD_DISABLE
(0xf4/0xf5) in ->write() method to switch between 1 and 3-byte transfers.

Thanks.

-- 
Dmitry

Re: [PATCH v2] input: mouse: add qci touchpad driver

[Neil Leeder]

Hi Dmitry,

On 8/19/2010 1:33 AM, Dmitry Torokhov wrote:
> On Wed, Aug 18, 2010 at 05:38:38PM -0400, Neil Leeder wrote:
>> Using proto=bare gets around the GETINFO failure, but doesn't help
>> with the more important GETID failure.
>
> Does it help if you change write() to transmit (and read) 1 byte at a time?
>

Thanks for the suggestions. Nothing I change seems able to get a 
consistent valid response to GETID. I think this is the first time 
anyone has written anything other than 0xF4 to the firmware in the EC 
and it's just not passing through the other PS/2 commands and responses 
reliably.

> One way would be to look for PSMOUSE_CMD_ENABLE/PSMOUSE_CMD_DISABLE
> (0xf4/0xf5) in ->write() method to switch between 1 and 3-byte transfers.

If I force my way past GETID and set the receive length to 3 based on 
seeing F4/F5 being written then the touchpad does work. However the 
serio driver is now a touchpad-specific driver rather than a generic 
PS/2 one.  I have a keyboard on the same device. I'm not sure if it will 
work with serio, but if so it will probably have its own requirements 
for the length of data sent on responses, probably not 3.

BTW, using 1 as the receive length for commands has its own problems as 
some commands require more than one byte response (GETID, GETINFO). I 
can work around it, but it's not particularly clean and getting further 
away from a generic driver.

At this point I'm thinking that the interface is close to being able to 
work with serio/psmouse, but just not close enough. The unreliability of 
responding to basic commands as well as the length of data problems 
indicates some custom driver is going to be needed. That could either be 
Quanta's original touchpad driver I posted, or a modification of your 
serio driver. I'd lean towards not having a serio driver which includes 
workarounds for a specific device, but I'd appreciate hearing your opinion.

Thanks.
--
Neil
-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

Re: [PATCH v2] input: mouse: add qci touchpad driver

[Matthew Garrett]

On Thu, Aug 19, 2010 at 06:19:10PM -0400, Neil Leeder wrote:

> At this point I'm thinking that the interface is close to being able to  
> work with serio/psmouse, but just not close enough. The unreliability of  
> responding to basic commands as well as the length of data problems  
> indicates some custom driver is going to be needed. That could either be  
> Quanta's original touchpad driver I posted, or a modification of your  
> serio driver. I'd lean towards not having a serio driver which includes  
> workarounds for a specific device, but I'd appreciate hearing your 
> opinion.

By any incredible stroke of luck, does it have a unique PNPID? 
/sys/bus/pnp/devices/*/id should give you some insight.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH v2] input: mouse: add qci touchpad driver

[Neil Leeder]

On 8/19/2010 7:35 PM, Matthew Garrett wrote:
> By any incredible stroke of luck, does it have a unique PNPID?
> /sys/bus/pnp/devices/*/id should give you some insight.
>

Thanks but I'm running this on an ARM netbook which doesn't have 
/sys/bus/pnp.

--
Neil
-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

Re: [PATCH v2] input: mouse: add qci touchpad driver

[Neil Leeder]

On 8/19/2010 6:19 PM, Neil Leeder wrote:
> Hi Dmitry,
>
> On 8/19/2010 1:33 AM, Dmitry Torokhov wrote:
>> On Wed, Aug 18, 2010 at 05:38:38PM -0400, Neil Leeder wrote:
>>> Using proto=bare gets around the GETINFO failure, but doesn't help
>>> with the more important GETID failure.
>>
>> Does it help if you change write() to transmit (and read) 1 byte at a
>> time?
>>
>
> Thanks for the suggestions. Nothing I change seems able to get a
> consistent valid response to GETID. I think this is the first time
> anyone has written anything other than 0xF4 to the firmware in the EC
> and it's just not passing through the other PS/2 commands and responses
> reliably.
>
>> One way would be to look for PSMOUSE_CMD_ENABLE/PSMOUSE_CMD_DISABLE
>> (0xf4/0xf5) in ->write() method to switch between 1 and 3-byte transfers.
>
> If I force my way past GETID and set the receive length to 3 based on
> seeing F4/F5 being written then the touchpad does work. However the
> serio driver is now a touchpad-specific driver rather than a generic
> PS/2 one. I have a keyboard on the same device. I'm not sure if it will
> work with serio, but if so it will probably have its own requirements
> for the length of data sent on responses, probably not 3.
>
> BTW, using 1 as the receive length for commands has its own problems as
> some commands require more than one byte response (GETID, GETINFO). I
> can work around it, but it's not particularly clean and getting further
> away from a generic driver.
>
> At this point I'm thinking that the interface is close to being able to
> work with serio/psmouse, but just not close enough. The unreliability of
> responding to basic commands as well as the length of data problems
> indicates some custom driver is going to be needed. That could either be
> Quanta's original touchpad driver I posted, or a modification of your
> serio driver. I'd lean towards not having a serio driver which includes
> workarounds for a specific device, but I'd appreciate hearing your opinion.
>
> Thanks.

Hi Dmitry,

Any comment on the above alternatives?

A third suggestion might be to have a fairly general serio driver, and 
rewrite the original Quanta driver to use serio rather than using i2c 
directly. I think that has its own set of problems, but thought I'd 
mention it anyway. The nuvoTon EC doesn't present a fully-working and 
reliable PS/2 interface to the driver so I'd lean toward using the 
Quanta touchpad driver by itself.

Thanks.

--
Neil
-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

Re: [PATCH v2] input: mouse: add qci touchpad driver

[Dmitry Torokhov]

Hi Neil,

On Wed, Aug 25, 2010 at 02:26:17PM -0400, Neil Leeder wrote:
> 
> Any comment on the above alternatives?
> 
> A third suggestion might be to have a fairly general serio driver,
> and rewrite the original Quanta driver to use serio rather than
> using i2c directly. I think that has its own set of problems, but
> thought I'd mention it anyway. The nuvoTon EC doesn't present a
> fully-working and reliable PS/2 interface to the driver so I'd lean
> toward using the Quanta touchpad driver by itself.
> 

I concur that since EC firmware does not seem to support full PS/2
protocol we should got with the original driver creating an input device
rather than serio.

Please send me the latest version of your driver and I will apply it to
my .37 queue.

Thanks.

-- 
Dmitry

Re: [PATCH v2] input: mouse: add qci touchpad driver

[Neil Leeder]

On 8/13/2010 4:36 AM, Datta, Shubhrajyoti wrote:
>
> Minor comments feel free to ignore if you feel so.

Thanks for reviewing this, I'll address these in the next patch.

>> +	err = i2c_master_send(context->ti2c_client, buf, 1);
>> +	if (err<  0)
>> +		goto i2c_fail;
>> +	msleep(100);
> Could you explain the time or have a #define XXX
>
Will add #define

>> +	gpio_direction_input(context->qcitp_gpio);
> The result is not checked here.

I'll check the result.

--
Neil
-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

Re: [PATCH v2] input: mouse: add qci touchpad driver

[Neil Leeder]

On 8/26/2010 10:49 AM, Dmitry Torokhov wrote:
>
> I concur that since EC firmware does not seem to support full PS/2
> protocol we should got with the original driver creating an input device
> rather than serio.
>
> Please send me the latest version of your driver and I will apply it to
> my .37 queue.

Thanks - I'll incorporate review comments and post an updated patch.

--
Neil
-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.