All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] Input: Wacom Stylus driver for I2C interface
@ 2012-03-23  5:40 Tatsunosuke Tobita
  2012-03-23 18:02 ` Shubhrajyoti
  2012-03-25 23:50 ` Dmitry Torokhov
  0 siblings, 2 replies; 6+ messages in thread
From: Tatsunosuke Tobita @ 2012-03-23  5:40 UTC (permalink / raw)
  To: linux-input; +Cc: Tatsunosuke Tobita

From: Tatsunosuke Tobita <tobita.tatsunosuke@wacom.co.jp>

This adds support for Wacom Stylus device with I2C interface.
The modification from the previous update is replacing module_init and
module_exit with module_i2c_driver.

Signed-off-by: Tatsunosuke Tobita <tobita.tatsunosuke@wacom.co.jp>
---
 drivers/input/touchscreen/wacom_i2c.c |  238 +++++++++++++++++++++++++++++++++
 drivers/input/touchscreen/wacom_i2c.h |   55 ++++++++
 2 files changed, 293 insertions(+), 0 deletions(-)
 create mode 100644 drivers/input/touchscreen/wacom_i2c.c
 create mode 100644 drivers/input/touchscreen/wacom_i2c.h

diff --git a/drivers/input/touchscreen/wacom_i2c.c b/drivers/input/touchscreen/wacom_i2c.c
new file mode 100644
index 0000000..f5a3845
--- /dev/null
+++ b/drivers/input/touchscreen/wacom_i2c.c
@@ -0,0 +1,238 @@
+/*
+ * Wacom Penabled Driver for I2C
+ *
+ * Copyright (c) 2011 Tatsunosuke Tobita, Wacom.
+ * <tobita.tatsunosuke@wacom.co.jp>
+ *
+ * This program is free software; you can redistribute it
+ * and/or modify it under the terms of the GNU General
+ * Public License as published by the Free Software
+ * Foundation; either version of 2 of the License,
+ * or (at your option) any later version.
+ */
+
+#include "wacom_i2c.h"
+
+static int wacom_send_query(struct wacom_i2c *wac_i2c)
+{
+	int ret;
+	u8 cmd[4] = {CMD_QUERY0, CMD_QUERY1, CMD_QUERY2, CMD_QUERY3};
+	u8 data[COM_COORD_NUM];
+
+	ret = i2c_master_send(wac_i2c->client, cmd, sizeof(cmd));
+	if (ret < 0) {
+		dev_printk(KERN_ERR, &wac_i2c->client->dev,
+			   "Sending 1st Query failed \n");
+		return -1;
+	}
+
+	cmd[0] = CMD_THROW0; cmd[1] = CMD_THROW1;
+	ret = i2c_master_send(wac_i2c->client, cmd, 2);
+	if (ret < 0) {
+		dev_printk(KERN_ERR, &wac_i2c->client->dev,
+			   "Sending 2nd Query failed \n");
+		return -1;
+	}
+
+	ret = i2c_master_recv(wac_i2c->client, data, QUERY_SIZE);
+	if (ret < 0) {
+		dev_printk(KERN_ERR, &wac_i2c->client->dev,
+			   "Receiving data failed 1\n");
+		return -1;
+	}
+
+	wac_i2c->wac_feature.x_max = get_unaligned_le16(&data[3]);
+	wac_i2c->wac_feature.y_max = get_unaligned_le16(&data[5]);
+	wac_i2c->wac_feature.pressure_max = get_unaligned_le16(&data[11]);
+	wac_i2c->wac_feature.fw_version = get_unaligned_le16(&data[13]);
+
+	dev_dbg(&wac_i2c->client->dev,
+		   "x_max:%d, y_max:%d, pressure:%d, fw:%d\n",
+		   wac_i2c->wac_feature.x_max, wac_i2c->wac_feature.y_max,
+		   wac_i2c->wac_feature.pressure_max, wac_i2c->wac_feature.fw_version);
+
+	return 0;
+}
+
+static irqreturn_t wacom_i2c_irq(int irqno, void *param)
+{
+	struct wacom_i2c *wac_i2c = param;
+       int ret;
+	unsigned int x, y, pressure;
+	unsigned char tsw, f1, f2, ers;
+	u8 data[COM_COORD_NUM];
+
+	do {
+		ret = i2c_master_recv(wac_i2c->client, data, QUERY_SIZE);
+	} while (gpio_get_value(irq_to_gpio(wac_i2c->client->irq)) == 0);
+
+	if (ret > 0) {
+		tsw = data[3]&0x01;
+		ers = data[3]&0x04;
+		f1 = data[3]&0x02;
+		f2 = data[3]&0x10;
+		x = le16_to_cpup((__le16 *)&data[4]);
+		y = le16_to_cpup((__le16 *)&data[6]);
+		pressure = le16_to_cpup((__le16 *)&data[8]);
+
+		input_report_abs(wac_i2c->input, ABS_X, x);
+		input_report_abs(wac_i2c->input, ABS_Y, y);
+		input_report_key(wac_i2c->input, BTN_TOOL_PEN, tsw);
+		input_report_key(wac_i2c->input, BTN_TOOL_RUBBER, ers);
+		input_report_abs(wac_i2c->input, ABS_PRESSURE, pressure);
+		input_report_key(wac_i2c->input, BTN_TOUCH, (tsw || ers));
+		input_report_key(wac_i2c->input, BTN_STYLUS, f1);
+		input_report_key(wac_i2c->input, BTN_STYLUS2, f2);
+
+		input_sync(wac_i2c->input);
+	}
+
+	return IRQ_HANDLED;
+}
+
+static int wacom_i2c_input_open(struct input_dev *dev)
+{
+	struct wacom_i2c *wac_i2c = input_get_drvdata(dev);
+	int ret;
+	ret = wacom_send_query(wac_i2c);
+	if (ret < 0)
+		return -EIO;
+
+	wac_i2c->input->id.version = wac_i2c->wac_feature.fw_version;
+	input_set_abs_params(wac_i2c->input, ABS_X, 0,
+			     wac_i2c->wac_feature.x_max, 0, 0);
+	input_set_abs_params(wac_i2c->input, ABS_Y, 0,
+			     wac_i2c->wac_feature.y_max, 0, 0);
+	input_set_abs_params(wac_i2c->input, ABS_PRESSURE, 0,
+			     wac_i2c->wac_feature.pressure_max, 0, 0);
+
+	return 0;
+}
+
+static void wacom_i2c_input_close(struct input_dev *dev)
+{
+}
+
+static int __devinit wacom_i2c_probe(struct i2c_client *client,
+			   const struct i2c_device_id *id)
+{
+	struct wacom_i2c *wac_i2c;
+	int err;
+	u8 data[COM_COORD_NUM];
+
+	if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
+		err = -EIO;
+		goto fail1;
+	}
+
+	wac_i2c = kzalloc(sizeof(*wac_i2c), GFP_KERNEL);
+	if (wac_i2c == NULL) {
+		err = -ENOMEM;
+		goto fail2;
+	}
+
+	wac_i2c->input = input_allocate_device();
+	if (wac_i2c->input == NULL) {
+		err = -ENOMEM;
+		goto fail2;
+	}
+
+	wac_i2c->client = client;
+	i2c_set_clientdata(client, wac_i2c);
+	mutex_init(&wac_i2c->lock);
+
+	__set_bit(BTN_TOOL_PEN, wac_i2c->input->keybit);
+	__set_bit(BTN_TOOL_RUBBER, wac_i2c->input->keybit);
+	__set_bit(BTN_STYLUS, wac_i2c->input->keybit);
+	__set_bit(BTN_STYLUS2, wac_i2c->input->keybit);
+	__set_bit(BTN_TOUCH, wac_i2c->input->keybit);
+
+	wac_i2c->input->name = "Wacom I2C Digitizer";
+	wac_i2c->input->id.bustype = BUS_I2C;
+	wac_i2c->input->id.vendor = 0x56a;
+	wac_i2c->input->dev.parent = &client->dev;
+	wac_i2c->input->open = wacom_i2c_input_open;
+	wac_i2c->input->close = wacom_i2c_input_close;
+	wac_i2c->input->evbit[0] |= BIT_MASK(EV_KEY) | BIT_MASK(EV_ABS);
+	input_set_drvdata(wac_i2c->input, wac_i2c);
+
+	if (input_register_device(wac_i2c->input)) {
+		dev_printk(KERN_ERR, &wac_i2c->client->dev, "Failed to register input device \n");
+		err = -ENODEV;
+		goto fail3;
+	}
+
+
+	err = request_threaded_irq(wac_i2c->client->irq, NULL,
+				   wacom_i2c_irq, IRQF_TRIGGER_FALLING, "WACOM_I2C_IRQ", wac_i2c);
+	if (err) {
+		dev_printk(KERN_ERR, &wac_i2c->client->dev, "Failed to enable IRQ for EMR \n");
+		goto fail3;
+	}
+
+	/*Clear the buffer in the firmware*/
+	do {
+		i2c_master_recv(wac_i2c->client, data, QUERY_SIZE);
+	} while (gpio_get_value(irq_to_gpio(wac_i2c->client->irq)) == 0);
+
+	return 0;
+
+ fail3:
+	input_free_device(wac_i2c->input);
+ fail2:
+	dev_err(&wac_i2c->client->dev, "Freeing device \n");
+ fail1:
+	return err;
+}
+
+static int __devexit wacom_i2c_remove(struct i2c_client *client)
+{
+	struct wacom_i2c *wac_i2c = i2c_get_clientdata(client);
+
+	free_irq(wac_i2c->client->irq, wac_i2c);
+	input_unregister_device(wac_i2c->input);
+	kfree(wac_i2c);
+
+	return 0;
+}
+
+static int wacom_i2c_suspend(struct device *dev)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	disable_irq(client->irq);
+
+	return 0;
+}
+
+static int wacom_i2c_resume(struct device *dev)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	enable_irq(client->irq);
+
+	return 0;
+}
+
+static SIMPLE_DEV_PM_OPS(wacom_i2c_pm, wacom_i2c_suspend, wacom_i2c_resume);
+
+static const struct i2c_device_id wacom_i2c_id[] = {
+	{WACNAME, 0},
+	{},
+};
+MODULE_DEVICE_TABLE(i2c, wacom_i2c_id);
+
+static struct i2c_driver wacom_i2c_driver = {
+	.driver = {
+		.name = "wacom_i2c",
+		.owner = THIS_MODULE,
+		.pm = &wacom_i2c_pm,
+	},
+
+	.probe = wacom_i2c_probe,
+	.remove = __devexit_p(wacom_i2c_remove),
+	.id_table = wacom_i2c_id,
+};
+module_i2c_driver(wacom_i2c_driver);
+
+MODULE_AUTHOR("Tatsunosuke Tobita <tobita.tatsunosuke@wacom.co.jp>");
+MODULE_DESCRIPTION("WACOM EMR I2C Driver");
+MODULE_LICENSE("GPL");
diff --git a/drivers/input/touchscreen/wacom_i2c.h b/drivers/input/touchscreen/wacom_i2c.h
new file mode 100644
index 0000000..b5c2d07
--- /dev/null
+++ b/drivers/input/touchscreen/wacom_i2c.h
@@ -0,0 +1,55 @@
+/*
+ * Wacom Penabled Driver for I2C
+ *
+ * Copyright (c) 2011 Tatsunosuke Tobita, Wacom.
+ * <tobita.tatsunosuke@wacom.co.jp>
+ *
+ * This program is free software; you can redistribute it
+ * and/or modify it under the terms of the GNU General
+ * Public License as published by the Free Software
+ * Foundation; either version of 2 of the License,
+ * or (at your option) any later version.
+ */
+
+#ifndef WACOM_I2C_H
+#define WACOM_I2C_H
+
+#include <linux/module.h>
+#include <linux/input.h>
+#include <linux/i2c.h>
+#include <linux/slab.h>
+#include <linux/irq.h>
+#include <linux/interrupt.h>
+#include <linux/gpio.h>
+#include <asm/unaligned.h>
+
+#define NAMEBUF 12
+#define WACNAME "WAC_I2C_EMR"
+
+/*Wacom Command*/
+#define COM_COORD_NUM      128
+#define CMD_QUERY0         0x04
+#define CMD_QUERY1         0x00
+#define CMD_QUERY2         0x33
+#define CMD_QUERY3         0x02
+#define CMD_THROW0         0x05
+#define CMD_THROW1         0x00
+#define QUERY_SIZE           19
+
+/*Parameters for wacom own features*/
+struct wacom_features {
+	int x_max;
+	int y_max;
+	int pressure_max;
+	char fw_version;
+};
+
+/*Parameters for i2c driver*/
+struct wacom_i2c {
+	struct i2c_client *client;
+	struct input_dev *input;
+	struct mutex lock;
+	struct wacom_features wac_feature;
+	int type;
+};
+#endif
-- 
1.7.1


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

* Re: [PATCH] Input: Wacom Stylus driver for I2C interface
  2012-03-23  5:40 [PATCH] Input: Wacom Stylus driver for I2C interface Tatsunosuke Tobita
@ 2012-03-23 18:02 ` Shubhrajyoti
  2012-03-26  2:26   ` Tatsunosuke Tobita
  2012-03-25 23:50 ` Dmitry Torokhov
  1 sibling, 1 reply; 6+ messages in thread
From: Shubhrajyoti @ 2012-03-23 18:02 UTC (permalink / raw)
  To: Tatsunosuke Tobita; +Cc: linux-input, Tatsunosuke Tobita

Hi ,
Some comments / doubts.

On Friday 23 March 2012 11:10 AM, Tatsunosuke Tobita wrote:
> From: Tatsunosuke Tobita <tobita.tatsunosuke@wacom.co.jp>
>
> This adds support for Wacom Stylus device with I2C interface.
> The modification from the previous update is replacing module_init and
> module_exit with module_i2c_driver.
>
> Signed-off-by: Tatsunosuke Tobita <tobita.tatsunosuke@wacom.co.jp>
> ---
>  drivers/input/touchscreen/wacom_i2c.c |  238 +++++++++++++++++++++++++++++++++
>  drivers/input/touchscreen/wacom_i2c.h |   55 ++++++++
>  2 files changed, 293 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/input/touchscreen/wacom_i2c.c
>  create mode 100644 drivers/input/touchscreen/wacom_i2c.h
>
> diff --git a/drivers/input/touchscreen/wacom_i2c.c b/drivers/input/touchscreen/wacom_i2c.c
> new file mode 100644
> index 0000000..f5a3845
> --- /dev/null
> +++ b/drivers/input/touchscreen/wacom_i2c.c
> @@ -0,0 +1,238 @@
> +/*
> + * Wacom Penabled Driver for I2C
> + *
> + * Copyright (c) 2011 Tatsunosuke Tobita, Wacom.
> + * <tobita.tatsunosuke@wacom.co.jp>
> + *
> + * This program is free software; you can redistribute it
> + * and/or modify it under the terms of the GNU General
> + * Public License as published by the Free Software
> + * Foundation; either version of 2 of the License,
> + * or (at your option) any later version.
> + */
> +
> +#include "wacom_i2c.h"
> +
> +static int wacom_send_query(struct wacom_i2c *wac_i2c)
> +{
> +	int ret;
> +	u8 cmd[4] = {CMD_QUERY0, CMD_QUERY1, CMD_QUERY2, CMD_QUERY3};
> +	u8 data[COM_COORD_NUM];
> +
> +	ret = i2c_master_send(wac_i2c->client, cmd, sizeof(cmd));
> +	if (ret < 0) {
> +		dev_printk(KERN_ERR, &wac_i2c->client->dev,
> +			   "Sending 1st Query failed \n");
May be use dev_err . Feel free to ignore if you feel so. Just a suggestion
> +		return -1;

Why mask the error?
> +	}
> +
> +	cmd[0] = CMD_THROW0; cmd[1] = CMD_THROW1;
> +	ret = i2c_master_send(wac_i2c->client, cmd, 2);
> +	if (ret < 0) {
> +		dev_printk(KERN_ERR, &wac_i2c->client->dev,
> +			   "Sending 2nd Query failed \n");
> +		return -1;
same
> +	}
> +
> +	ret = i2c_master_recv(wac_i2c->client, data, QUERY_SIZE);
> +	if (ret < 0) {
> +		dev_printk(KERN_ERR, &wac_i2c->client->dev,
> +			   "Receiving data failed 1\n");
> +		return -1;
same
> +	}
> +
> +	wac_i2c->wac_feature.x_max = get_unaligned_le16(&data[3]);
> +	wac_i2c->wac_feature.y_max = get_unaligned_le16(&data[5]);
> +	wac_i2c->wac_feature.pressure_max = get_unaligned_le16(&data[11]);
> +	wac_i2c->wac_feature.fw_version = get_unaligned_le16(&data[13]);
> +
> +	dev_dbg(&wac_i2c->client->dev,
> +		   "x_max:%d, y_max:%d, pressure:%d, fw:%d\n",
> +		   wac_i2c->wac_feature.x_max, wac_i2c->wac_feature.y_max,
> +		   wac_i2c->wac_feature.pressure_max, wac_i2c->wac_feature.fw_version);
> +
> +	return 0;
> +}
> +
> +static irqreturn_t wacom_i2c_irq(int irqno, void *param)
> +{
> +	struct wacom_i2c *wac_i2c = param;
> +       int ret;
> +	unsigned int x, y, pressure;
> +	unsigned char tsw, f1, f2, ers;
> +	u8 data[COM_COORD_NUM];
> +
> +	do {
> +		ret = i2c_master_recv(wac_i2c->client, data, QUERY_SIZE);
> +	} while (gpio_get_value(irq_to_gpio(wac_i2c->client->irq)) == 0);
> +
> +	if (ret > 0) {
> +		tsw = data[3]&0x01;
> +		ers = data[3]&0x04;
> +		f1 = data[3]&0x02;
> +		f2 = data[3]&0x10;
> +		x = le16_to_cpup((__le16 *)&data[4]);
> +		y = le16_to_cpup((__le16 *)&data[6]);
> +		pressure = le16_to_cpup((__le16 *)&data[8]);
> +
> +		input_report_abs(wac_i2c->input, ABS_X, x);
> +		input_report_abs(wac_i2c->input, ABS_Y, y);
> +		input_report_key(wac_i2c->input, BTN_TOOL_PEN, tsw);
> +		input_report_key(wac_i2c->input, BTN_TOOL_RUBBER, ers);
> +		input_report_abs(wac_i2c->input, ABS_PRESSURE, pressure);
> +		input_report_key(wac_i2c->input, BTN_TOUCH, (tsw || ers));
> +		input_report_key(wac_i2c->input, BTN_STYLUS, f1);
> +		input_report_key(wac_i2c->input, BTN_STYLUS2, f2);
> +
> +		input_sync(wac_i2c->input);
> +	}
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static int wacom_i2c_input_open(struct input_dev *dev)
> +{
> +	struct wacom_i2c *wac_i2c = input_get_drvdata(dev);
> +	int ret;
> +	ret = wacom_send_query(wac_i2c);
> +	if (ret < 0)
> +		return -EIO;
> +
> +	wac_i2c->input->id.version = wac_i2c->wac_feature.fw_version;
> +	input_set_abs_params(wac_i2c->input, ABS_X, 0,
> +			     wac_i2c->wac_feature.x_max, 0, 0);
> +	input_set_abs_params(wac_i2c->input, ABS_Y, 0,
> +			     wac_i2c->wac_feature.y_max, 0, 0);
> +	input_set_abs_params(wac_i2c->input, ABS_PRESSURE, 0,
> +			     wac_i2c->wac_feature.pressure_max, 0, 0);
> +
> +	return 0;
> +}
> +
> +static void wacom_i2c_input_close(struct input_dev *dev)
> +{
> +}
May be this could be cleaned up?
> +
> +static int __devinit wacom_i2c_probe(struct i2c_client *client,
> +			   const struct i2c_device_id *id)
> +{
> +	struct wacom_i2c *wac_i2c;
> +	int err;
> +	u8 data[COM_COORD_NUM];
> +
> +	if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
> +		err = -EIO;
> +		goto fail1;
> +	}
> +
> +	wac_i2c = kzalloc(sizeof(*wac_i2c), GFP_KERNEL);
> +	if (wac_i2c == NULL) {
> +		err = -ENOMEM;
> +		goto fail2;
> +	}
> +
> +	wac_i2c->input = input_allocate_device();
> +	if (wac_i2c->input == NULL) {
> +		err = -ENOMEM;
> +		goto fail2;
We are leaking wac_i2c here ?
> +	}
> +
> +	wac_i2c->client = client;
> +	i2c_set_clientdata(client, wac_i2c);
> +	mutex_init(&wac_i2c->lock);
> +
> +	__set_bit(BTN_TOOL_PEN, wac_i2c->input->keybit);
> +	__set_bit(BTN_TOOL_RUBBER, wac_i2c->input->keybit);
> +	__set_bit(BTN_STYLUS, wac_i2c->input->keybit);
> +	__set_bit(BTN_STYLUS2, wac_i2c->input->keybit);
> +	__set_bit(BTN_TOUCH, wac_i2c->input->keybit);
> +
> +	wac_i2c->input->name = "Wacom I2C Digitizer";
> +	wac_i2c->input->id.bustype = BUS_I2C;
> +	wac_i2c->input->id.vendor = 0x56a;
> +	wac_i2c->input->dev.parent = &client->dev;
> +	wac_i2c->input->open = wacom_i2c_input_open;
> +	wac_i2c->input->close = wacom_i2c_input_close;
> +	wac_i2c->input->evbit[0] |= BIT_MASK(EV_KEY) | BIT_MASK(EV_ABS);
> +	input_set_drvdata(wac_i2c->input, wac_i2c);
> +
> +	if (input_register_device(wac_i2c->input)) {
> +		dev_printk(KERN_ERR, &wac_i2c->client->dev, "Failed to register input device \n");
> +		err = -ENODEV;
> +		goto fail3;
> +	}
> +
> +
> +	err = request_threaded_irq(wac_i2c->client->irq, NULL,
> +				   wacom_i2c_irq, IRQF_TRIGGER_FALLING, "WACOM_I2C_IRQ", wac_i2c);
> +	if (err) {
> +		dev_printk(KERN_ERR, &wac_i2c->client->dev, "Failed to enable IRQ for EMR \n");
> +		goto fail3;
> +	}
> +
> +	/*Clear the buffer in the firmware*/
> +	do {
> +		i2c_master_recv(wac_i2c->client, data, QUERY_SIZE);
> +	} while (gpio_get_value(irq_to_gpio(wac_i2c->client->irq)) == 0);
> +
> +	return 0;
> +
> + fail3:
> +	input_free_device(wac_i2c->input);
> + fail2:
> +	dev_err(&wac_i2c->client->dev, "Freeing device \n");
> + fail1:
> +	return err;
> +}
> +
> +static int __devexit wacom_i2c_remove(struct i2c_client *client)
> +{
> +	struct wacom_i2c *wac_i2c = i2c_get_clientdata(client);
> +
> +	free_irq(wac_i2c->client->irq, wac_i2c);
> +	input_unregister_device(wac_i2c->input);
> +	kfree(wac_i2c);
> +
> +	return 0;
> +}
> +
> +static int wacom_i2c_suspend(struct device *dev)
> +{
> +	struct i2c_client *client = to_i2c_client(dev);
> +	disable_irq(client->irq);
Why is this disable needed?
> +
> +	return 0;
> +}
> +
> +static int wacom_i2c_resume(struct device *dev)
> +{
> +	struct i2c_client *client = to_i2c_client(dev);
> +	enable_irq(client->irq);
> +
> +	return 0;
> +}
> +
> +static SIMPLE_DEV_PM_OPS(wacom_i2c_pm, wacom_i2c_suspend, wacom_i2c_resume);
> +
> +static const struct i2c_device_id wacom_i2c_id[] = {
> +	{WACNAME, 0},
> +	{},
> +};
> +MODULE_DEVICE_TABLE(i2c, wacom_i2c_id);
> +
> +static struct i2c_driver wacom_i2c_driver = {
> +	.driver = {
> +		.name = "wacom_i2c",
> +		.owner = THIS_MODULE,
> +		.pm = &wacom_i2c_pm,
> +	},
> +
> +	.probe = wacom_i2c_probe,
> +	.remove = __devexit_p(wacom_i2c_remove),
> +	.id_table = wacom_i2c_id,
> +};
> +module_i2c_driver(wacom_i2c_driver);
> +
> +MODULE_AUTHOR("Tatsunosuke Tobita <tobita.tatsunosuke@wacom.co.jp>");
> +MODULE_DESCRIPTION("WACOM EMR I2C Driver");
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/input/touchscreen/wacom_i2c.h b/drivers/input/touchscreen/wacom_i2c.h
> new file mode 100644
> index 0000000..b5c2d07
> --- /dev/null
> +++ b/drivers/input/touchscreen/wacom_i2c.h
> @@ -0,0 +1,55 @@
> +/*
> + * Wacom Penabled Driver for I2C
> + *
> + * Copyright (c) 2011 Tatsunosuke Tobita, Wacom.
> + 
nit : 2012?

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

* Re: [PATCH] Input: Wacom Stylus driver for I2C interface
  2012-03-23  5:40 [PATCH] Input: Wacom Stylus driver for I2C interface Tatsunosuke Tobita
  2012-03-23 18:02 ` Shubhrajyoti
@ 2012-03-25 23:50 ` Dmitry Torokhov
  2012-03-26  3:05   ` Tatsunosuke Tobita
       [not found]   ` <4f755be3.42d1440a.31fc.3511SMTPIN_ADDED@mx.google.com>
  1 sibling, 2 replies; 6+ messages in thread
From: Dmitry Torokhov @ 2012-03-25 23:50 UTC (permalink / raw)
  To: Tatsunosuke Tobita; +Cc: linux-input, Tatsunosuke Tobita

Hi Tatsunosuke,

On Fri, Mar 23, 2012 at 02:40:07PM +0900, Tatsunosuke Tobita wrote:
> From: Tatsunosuke Tobita <tobita.tatsunosuke@wacom.co.jp>
> 
> This adds support for Wacom Stylus device with I2C interface.
> The modification from the previous update is replacing module_init and
> module_exit with module_i2c_driver.
> 
> Signed-off-by: Tatsunosuke Tobita <tobita.tatsunosuke@wacom.co.jp>
> ---
>  drivers/input/touchscreen/wacom_i2c.c |  238 +++++++++++++++++++++++++++++++++
>  drivers/input/touchscreen/wacom_i2c.h |   55 ++++++++
>  2 files changed, 293 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/input/touchscreen/wacom_i2c.c
>  create mode 100644 drivers/input/touchscreen/wacom_i2c.h
> 
> diff --git a/drivers/input/touchscreen/wacom_i2c.c b/drivers/input/touchscreen/wacom_i2c.c
> new file mode 100644
> index 0000000..f5a3845
> --- /dev/null
> +++ b/drivers/input/touchscreen/wacom_i2c.c
> @@ -0,0 +1,238 @@
> +/*
> + * Wacom Penabled Driver for I2C
> + *
> + * Copyright (c) 2011 Tatsunosuke Tobita, Wacom.
> + * <tobita.tatsunosuke@wacom.co.jp>
> + *
> + * This program is free software; you can redistribute it
> + * and/or modify it under the terms of the GNU General
> + * Public License as published by the Free Software
> + * Foundation; either version of 2 of the License,
> + * or (at your option) any later version.
> + */
> +
> +#include "wacom_i2c.h"
> +
> +static int wacom_send_query(struct wacom_i2c *wac_i2c)
> +{
> +	int ret;
> +	u8 cmd[4] = {CMD_QUERY0, CMD_QUERY1, CMD_QUERY2, CMD_QUERY3};
> +	u8 data[COM_COORD_NUM];
> +
> +	ret = i2c_master_send(wac_i2c->client, cmd, sizeof(cmd));
> +	if (ret < 0) {
> +		dev_printk(KERN_ERR, &wac_i2c->client->dev,
> +			   "Sending 1st Query failed \n");
> +		return -1;

As Shubhrajyoti already mentioned you should not clobber errors returned
by i2c_master_send with a generic -1 (-EPERM).

> +	}
> +
> +	cmd[0] = CMD_THROW0; cmd[1] = CMD_THROW1;
> +	ret = i2c_master_send(wac_i2c->client, cmd, 2);
> +	if (ret < 0) {
> +		dev_printk(KERN_ERR, &wac_i2c->client->dev,
> +			   "Sending 2nd Query failed \n");
> +		return -1;
> +	}
> +
> +	ret = i2c_master_recv(wac_i2c->client, data, QUERY_SIZE);
> +	if (ret < 0) {
> +		dev_printk(KERN_ERR, &wac_i2c->client->dev,
> +			   "Receiving data failed 1\n");
> +		return -1;
> +	}

I wonder if all this could be converted into a single i2c_transfer().

> +
> +	wac_i2c->wac_feature.x_max = get_unaligned_le16(&data[3]);
> +	wac_i2c->wac_feature.y_max = get_unaligned_le16(&data[5]);
> +	wac_i2c->wac_feature.pressure_max = get_unaligned_le16(&data[11]);
> +	wac_i2c->wac_feature.fw_version = get_unaligned_le16(&data[13]);
> +
> +	dev_dbg(&wac_i2c->client->dev,
> +		   "x_max:%d, y_max:%d, pressure:%d, fw:%d\n",
> +		   wac_i2c->wac_feature.x_max, wac_i2c->wac_feature.y_max,
> +		   wac_i2c->wac_feature.pressure_max, wac_i2c->wac_feature.fw_version);
> +
> +	return 0;
> +}
> +
> +static irqreturn_t wacom_i2c_irq(int irqno, void *param)
> +{
> +	struct wacom_i2c *wac_i2c = param;
> +       int ret;
> +	unsigned int x, y, pressure;
> +	unsigned char tsw, f1, f2, ers;
> +	u8 data[COM_COORD_NUM];
> +
> +	do {
> +		ret = i2c_master_recv(wac_i2c->client, data, QUERY_SIZE);
> +	} while (gpio_get_value(irq_to_gpio(wac_i2c->client->irq)) == 0);

irq_to_gpio() may fail. Also it does not make sense to do the conversion
in interrupt handler if it can be done once, at probe() time.

> +
> +	if (ret > 0) {
> +		tsw = data[3]&0x01;
> +		ers = data[3]&0x04;
> +		f1 = data[3]&0x02;
> +		f2 = data[3]&0x10;
> +		x = le16_to_cpup((__le16 *)&data[4]);
> +		y = le16_to_cpup((__le16 *)&data[6]);
> +		pressure = le16_to_cpup((__le16 *)&data[8]);
> +
> +		input_report_abs(wac_i2c->input, ABS_X, x);
> +		input_report_abs(wac_i2c->input, ABS_Y, y);
> +		input_report_key(wac_i2c->input, BTN_TOOL_PEN, tsw);
> +		input_report_key(wac_i2c->input, BTN_TOOL_RUBBER, ers);
> +		input_report_abs(wac_i2c->input, ABS_PRESSURE, pressure);
> +		input_report_key(wac_i2c->input, BTN_TOUCH, (tsw || ers));
> +		input_report_key(wac_i2c->input, BTN_STYLUS, f1);
> +		input_report_key(wac_i2c->input, BTN_STYLUS2, f2);
> +
> +		input_sync(wac_i2c->input);
> +	}
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static int wacom_i2c_input_open(struct input_dev *dev)
> +{
> +	struct wacom_i2c *wac_i2c = input_get_drvdata(dev);
> +	int ret;
> +	ret = wacom_send_query(wac_i2c);
> +	if (ret < 0)
> +		return -EIO;
> +
> +	wac_i2c->input->id.version = wac_i2c->wac_feature.fw_version;
> +	input_set_abs_params(wac_i2c->input, ABS_X, 0,
> +			     wac_i2c->wac_feature.x_max, 0, 0);
> +	input_set_abs_params(wac_i2c->input, ABS_Y, 0,
> +			     wac_i2c->wac_feature.y_max, 0, 0);
> +	input_set_abs_params(wac_i2c->input, ABS_PRESSURE, 0,
> +			     wac_i2c->wac_feature.pressure_max, 0, 0);

This should be done when registering input device.

> +
> +	return 0;
> +}
> +
> +static void wacom_i2c_input_close(struct input_dev *dev)
> +{
> +}

Empty methods are not needed.

> +
> +static int __devinit wacom_i2c_probe(struct i2c_client *client,
> +			   const struct i2c_device_id *id)
> +{
> +	struct wacom_i2c *wac_i2c;
> +	int err;
> +	u8 data[COM_COORD_NUM];
> +
> +	if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
> +		err = -EIO;
> +		goto fail1;
> +	}
> +
> +	wac_i2c = kzalloc(sizeof(*wac_i2c), GFP_KERNEL);
> +	if (wac_i2c == NULL) {
> +		err = -ENOMEM;
> +		goto fail2;
> +	}
> +
> +	wac_i2c->input = input_allocate_device();
> +	if (wac_i2c->input == NULL) {
> +		err = -ENOMEM;
> +		goto fail2;
> +	}
> +
> +	wac_i2c->client = client;
> +	i2c_set_clientdata(client, wac_i2c);
> +	mutex_init(&wac_i2c->lock);

What is this lock used for?

> +
> +	__set_bit(BTN_TOOL_PEN, wac_i2c->input->keybit);
> +	__set_bit(BTN_TOOL_RUBBER, wac_i2c->input->keybit);
> +	__set_bit(BTN_STYLUS, wac_i2c->input->keybit);
> +	__set_bit(BTN_STYLUS2, wac_i2c->input->keybit);
> +	__set_bit(BTN_TOUCH, wac_i2c->input->keybit);
> +
> +	wac_i2c->input->name = "Wacom I2C Digitizer";
> +	wac_i2c->input->id.bustype = BUS_I2C;
> +	wac_i2c->input->id.vendor = 0x56a;
> +	wac_i2c->input->dev.parent = &client->dev;
> +	wac_i2c->input->open = wacom_i2c_input_open;
> +	wac_i2c->input->close = wacom_i2c_input_close;
> +	wac_i2c->input->evbit[0] |= BIT_MASK(EV_KEY) | BIT_MASK(EV_ABS);
> +	input_set_drvdata(wac_i2c->input, wac_i2c);
> +
> +	if (input_register_device(wac_i2c->input)) {
> +		dev_printk(KERN_ERR, &wac_i2c->client->dev, "Failed to register input device \n");

dev_err() and dev_info() throughout.

> +		err = -ENODEV;

The proper error is ENOMEM.

> +		goto fail3;
> +	}
> +
> +
> +	err = request_threaded_irq(wac_i2c->client->irq, NULL,
> +				   wacom_i2c_irq, IRQF_TRIGGER_FALLING, "WACOM_I2C_IRQ", wac_i2c);
> +	if (err) {
> +		dev_printk(KERN_ERR, &wac_i2c->client->dev, "Failed to enable IRQ for EMR \n");
> +		goto fail3;
> +	}
> +
> +	/*Clear the buffer in the firmware*/
> +	do {
> +		i2c_master_recv(wac_i2c->client, data, QUERY_SIZE);
> +	} while (gpio_get_value(irq_to_gpio(wac_i2c->client->irq)) == 0);

This should be done at open() time.

> +
> +	return 0;
> +
> + fail3:
> +	input_free_device(wac_i2c->input);
> + fail2:
> +	dev_err(&wac_i2c->client->dev, "Freeing device \n");
> + fail1:
> +	return err;
> +}
> +
> +static int __devexit wacom_i2c_remove(struct i2c_client *client)
> +{
> +	struct wacom_i2c *wac_i2c = i2c_get_clientdata(client);
> +
> +	free_irq(wac_i2c->client->irq, wac_i2c);
> +	input_unregister_device(wac_i2c->input);
> +	kfree(wac_i2c);
> +
> +	return 0;
> +}
> +
> +static int wacom_i2c_suspend(struct device *dev)
> +{
> +	struct i2c_client *client = to_i2c_client(dev);
> +	disable_irq(client->irq);

I guess disabling IRQ is OK for now, but don't you want to use it as a
wakeup source?

> +
> +	return 0;
> +}
> +
> +static int wacom_i2c_resume(struct device *dev)
> +{
> +	struct i2c_client *client = to_i2c_client(dev);
> +	enable_irq(client->irq);
> +
> +	return 0;
> +}

Suspend and resume should be guarded by #ifdef CONFIG_PM_SLEEP

> +
> +static SIMPLE_DEV_PM_OPS(wacom_i2c_pm, wacom_i2c_suspend, wacom_i2c_resume);
> +
> +static const struct i2c_device_id wacom_i2c_id[] = {
> +	{WACNAME, 0},
> +	{},
> +};
> +MODULE_DEVICE_TABLE(i2c, wacom_i2c_id);
> +
> +static struct i2c_driver wacom_i2c_driver = {
> +	.driver = {
> +		.name = "wacom_i2c",
> +		.owner = THIS_MODULE,
> +		.pm = &wacom_i2c_pm,
> +	},
> +
> +	.probe = wacom_i2c_probe,
> +	.remove = __devexit_p(wacom_i2c_remove),
> +	.id_table = wacom_i2c_id,
> +};
> +module_i2c_driver(wacom_i2c_driver);
> +
> +MODULE_AUTHOR("Tatsunosuke Tobita <tobita.tatsunosuke@wacom.co.jp>");
> +MODULE_DESCRIPTION("WACOM EMR I2C Driver");
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/input/touchscreen/wacom_i2c.h b/drivers/input/touchscreen/wacom_i2c.h
> new file mode 100644
> index 0000000..b5c2d07
> --- /dev/null
> +++ b/drivers/input/touchscreen/wacom_i2c.h
> @@ -0,0 +1,55 @@
> +/*
> + * Wacom Penabled Driver for I2C
> + *
> + * Copyright (c) 2011 Tatsunosuke Tobita, Wacom.
> + * <tobita.tatsunosuke@wacom.co.jp>
> + *
> + * This program is free software; you can redistribute it
> + * and/or modify it under the terms of the GNU General
> + * Public License as published by the Free Software
> + * Foundation; either version of 2 of the License,
> + * or (at your option) any later version.
> + */
> +
> +#ifndef WACOM_I2C_H
> +#define WACOM_I2C_H
> +
> +#include <linux/module.h>
> +#include <linux/input.h>
> +#include <linux/i2c.h>
> +#include <linux/slab.h>
> +#include <linux/irq.h>
> +#include <linux/interrupt.h>
> +#include <linux/gpio.h>
> +#include <asm/unaligned.h>
> +
> +#define NAMEBUF 12

Not used.

> +#define WACNAME "WAC_I2C_EMR"
> +
> +/*Wacom Command*/
> +#define COM_COORD_NUM      128
> +#define CMD_QUERY0         0x04
> +#define CMD_QUERY1         0x00
> +#define CMD_QUERY2         0x33
> +#define CMD_QUERY3         0x02
> +#define CMD_THROW0         0x05
> +#define CMD_THROW1         0x00
> +#define QUERY_SIZE           19
> +
> +/*Parameters for wacom own features*/
> +struct wacom_features {
> +	int x_max;
> +	int y_max;
> +	int pressure_max;
> +	char fw_version;
> +};
> +
> +/*Parameters for i2c driver*/
> +struct wacom_i2c {
> +	struct i2c_client *client;
> +	struct input_dev *input;
> +	struct mutex lock;

Not used.

> +	struct wacom_features wac_feature;

No need to carry this around, you only use it once.

> +	int type;

Not needed.

> +};
> +#endif

I also do not see why you need a separate header file and why you split
this into 2 patches - one with driver and another with Kconfig/Makefile
adjustments.

Could you please try the version below and let me know if it works for
you. Thanks!

-- 
Dmitry


Input: add support for Wacom Stylus device with I2C interface

From: Tatsunosuke Tobita <tobita.tatsunosuke@wacom.co.jp>

This adds support for Wacom Stylus device with I2C interface.

Signed-off-by: Tatsunosuke Tobita <tobita.tatsunosuke@wacom.co.jp>
Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
---

 drivers/input/touchscreen/Kconfig     |   12 +
 drivers/input/touchscreen/Makefile    |    1 
 drivers/input/touchscreen/wacom_i2c.c |  338 +++++++++++++++++++++++++++++++++
 3 files changed, 351 insertions(+), 0 deletions(-)
 create mode 100644 drivers/input/touchscreen/wacom_i2c.c


diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
index 3594591..1773001 100644
--- a/drivers/input/touchscreen/Kconfig
+++ b/drivers/input/touchscreen/Kconfig
@@ -323,6 +323,18 @@ config TOUCHSCREEN_WACOM_W8001
 	  To compile this driver as a module, choose M here: the
 	  module will be called wacom_w8001.
 
+config TOUCHSCREEN_WACOM_I2C
+	tristate "Wacom Tablet support (I2C)"
+	depends on I2C
+	help
+	  Say Y here if you want to use the I2C version of the Wacom
+	  Pen Tablet.
+
+	  If unsure, say N.
+
+	  To compile this driver as a module, choose M here: the module
+	  will be called wacom_i2c.
+
 config TOUCHSCREEN_LPC32XX
 	tristate "LPC32XX touchscreen controller"
 	depends on ARCH_LPC32XX
diff --git a/drivers/input/touchscreen/Makefile b/drivers/input/touchscreen/Makefile
index b6c746e0..eb8bfe1 100644
--- a/drivers/input/touchscreen/Makefile
+++ b/drivers/input/touchscreen/Makefile
@@ -60,6 +60,7 @@ obj-$(CONFIG_TOUCHSCREEN_TSC2005)	+= tsc2005.o
 obj-$(CONFIG_TOUCHSCREEN_TSC2007)	+= tsc2007.o
 obj-$(CONFIG_TOUCHSCREEN_UCB1400)	+= ucb1400_ts.o
 obj-$(CONFIG_TOUCHSCREEN_WACOM_W8001)	+= wacom_w8001.o
+obj-$(CONFIG_TOUCHSCREEN_WACOM_I2C)	+= wacom_i2c.o
 obj-$(CONFIG_TOUCHSCREEN_WM831X)	+= wm831x-ts.o
 obj-$(CONFIG_TOUCHSCREEN_WM97XX)	+= wm97xx-ts.o
 wm97xx-ts-$(CONFIG_TOUCHSCREEN_WM9705)	+= wm9705.o
diff --git a/drivers/input/touchscreen/wacom_i2c.c b/drivers/input/touchscreen/wacom_i2c.c
new file mode 100644
index 0000000..88e0800
--- /dev/null
+++ b/drivers/input/touchscreen/wacom_i2c.c
@@ -0,0 +1,338 @@
+/*
+ * Wacom Penabled Driver for I2C
+ *
+ * Copyright (c) 2011 Tatsunosuke Tobita, Wacom.
+ * <tobita.tatsunosuke@wacom.co.jp>
+ *
+ * This program is free software; you can redistribute it
+ * and/or modify it under the terms of the GNU General
+ * Public License as published by the Free Software
+ * Foundation; either version of 2 of the License,
+ * or (at your option) any later version.
+ */
+
+#include <linux/module.h>
+#include <linux/input.h>
+#include <linux/i2c.h>
+#include <linux/slab.h>
+#include <linux/irq.h>
+#include <linux/interrupt.h>
+#include <linux/gpio.h>
+#include <asm/unaligned.h>
+
+#define WACOM_CMD_QUERY0	0x04
+#define WACOM_CMD_QUERY1	0x00
+#define WACOM_CMD_QUERY2	0x33
+#define WACOM_CMD_QUERY3	0x02
+#define WACOM_CMD_THROW0	0x05
+#define WACOM_CMD_THROW1	0x00
+#define WACOM_QUERY_SIZE	19
+#define WACOM_RETRY_CNT		100
+
+struct wacom_features {
+	int x_max;
+	int y_max;
+	int pressure_max;
+	char fw_version;
+};
+
+struct wacom_i2c {
+	struct i2c_client *client;
+	struct input_dev *input;
+	unsigned int gpio;
+	u8 data[WACOM_QUERY_SIZE];
+};
+
+static int wacom_query_device(struct i2c_client *client,
+			      struct wacom_features *features)
+{
+	int ret;
+	u8 cmd1[] = { WACOM_CMD_QUERY0, WACOM_CMD_QUERY1,
+			WACOM_CMD_QUERY2, WACOM_CMD_QUERY3 };
+	u8 cmd2[] = { WACOM_CMD_THROW0, WACOM_CMD_THROW1 };
+	u8 data[WACOM_QUERY_SIZE];
+#if 1
+	struct i2c_msg msgs[] = {
+		{
+			.addr = client->addr,
+			.flags = 0,
+			.len = sizeof(cmd1),
+			.buf = cmd1,
+		},
+		{
+			.addr = client->addr,
+			.flags = 0,
+			.len = sizeof(cmd2),
+			.buf = cmd2,
+		},
+		{
+			.addr = client->addr,
+			.flags = I2C_M_RD,
+			.len = sizeof(data),
+			.buf = data,
+		},
+	};
+
+	ret = i2c_transfer(client->adapter, msgs, ARRAY_SIZE(msgs));
+	if (ret < 0)
+		return ret;
+	if (ret != ARRAY_SIZE(msgs))
+		return -EIO;
+#else
+	ret = i2c_master_send(client, cmd1, sizeof(cmd1));
+	if (ret < 0) {
+		dev_err(&client->dev,
+			"Sending 1st Query failed, error: %d\n", ret);
+		return ret;
+	}
+
+	ret = i2c_master_send(client, cmd2, sizeof(cmd2));
+	if (ret < 0) {
+		dev_err(&client->dev,
+			"Sending 2nd Query failed, error: %d\n", ret);
+		return ret;
+	}
+
+	ret = i2c_master_recv(client, data, sizeof(data));
+	if (ret < 0) {
+		dev_err(&client->dev,
+			"Receiving data failed, error: %d\n", ret);
+		return ret;
+	}
+#endif
+
+	features->x_max = get_unaligned_le16(&data[3]);
+	features->y_max = get_unaligned_le16(&data[5]);
+	features->pressure_max = get_unaligned_le16(&data[11]);
+	features->fw_version = get_unaligned_le16(&data[13]);
+
+	dev_dbg(&client->dev,
+		"x_max:%d, y_max:%d, pressure:%d, fw:%d\n",
+		features->x_max, features->y_max,
+		features->pressure_max, features->fw_version);
+
+	return 0;
+}
+
+static int wacom_i2c_fetch_data(struct wacom_i2c *wac_i2c)
+{
+	int retries = 0;
+	int ret;
+
+	do {
+		ret = i2c_master_recv(wac_i2c->client,
+				      wac_i2c->data, sizeof(wac_i2c->data));
+	} while (gpio_get_value(wac_i2c->gpio) == 0 &&
+		 retries++ < WACOM_RETRY_CNT);
+
+	if (retries >= WACOM_RETRY_CNT)
+		ret = -EIO;
+
+	return ret < 0 ? ret : 0;
+}
+
+static irqreturn_t wacom_i2c_irq(int irq, void *dev_id)
+{
+	struct wacom_i2c *wac_i2c = dev_id;
+	struct input_dev *input = wac_i2c->input;
+	u8 *data = wac_i2c->data;
+	unsigned int x, y, pressure;
+	unsigned char tsw, f1, f2, ers;
+	int error;
+
+	error = wacom_i2c_fetch_data(wac_i2c);
+	if (error)
+		goto out;
+
+	tsw = data[3] & 0x01;
+	ers = data[3] & 0x04;
+	f1 = data[3] & 0x02;
+	f2 = data[3] & 0x10;
+	x = le16_to_cpup((__le16 *)&data[4]);
+	y = le16_to_cpup((__le16 *)&data[6]);
+	pressure = le16_to_cpup((__le16 *)&data[8]);
+
+	input_report_key(input, BTN_TOUCH, tsw || ers);
+	input_report_key(input, BTN_TOOL_PEN, tsw);
+	input_report_key(input, BTN_TOOL_RUBBER, ers);
+	input_report_key(input, BTN_STYLUS, f1);
+	input_report_key(input, BTN_STYLUS2, f2);
+	input_report_abs(input, ABS_X, x);
+	input_report_abs(input, ABS_Y, y);
+	input_report_abs(input, ABS_PRESSURE, pressure);
+
+	input_sync(wac_i2c->input);
+out:
+	return IRQ_HANDLED;
+}
+
+static int wacom_i2c_open(struct input_dev *dev)
+{
+	struct wacom_i2c *wac_i2c = input_get_drvdata(dev);
+	struct i2c_client *client = wac_i2c->client;
+	int error;
+
+	/* Clear the device buffer */
+	error = wacom_i2c_fetch_data(wac_i2c);
+	if (error)
+		return error;
+
+	enable_irq(client->irq);
+
+	return 0;
+}
+
+static void wacom_i2c_close(struct input_dev *dev)
+{
+	struct wacom_i2c *wac_i2c = input_get_drvdata(dev);
+	struct i2c_client *client = wac_i2c->client;
+
+	disable_irq(client->irq);
+}
+
+static int __devinit wacom_i2c_probe(struct i2c_client *client,
+				     const struct i2c_device_id *id)
+{
+	struct wacom_i2c *wac_i2c;
+	struct input_dev *input;
+	struct wacom_features features;
+	int gpio;
+	int error;
+
+	if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
+		dev_err(&client->dev, "i2c_check_functionality error\n");
+		return -EIO;
+	}
+
+	gpio = irq_to_gpio(client->irq);
+	if (gpio < 0) {
+		error = gpio;
+		dev_err(&client->dev,
+			"irq_to_gpio() failed, error: %d\n", error);
+		return error;
+	}
+
+	error = wacom_query_device(client, &features);
+	if (error)
+		return error;
+
+	wac_i2c = kzalloc(sizeof(*wac_i2c), GFP_KERNEL);
+	input = input_allocate_device();
+	if (!wac_i2c || !input) {
+		error = -ENOMEM;
+		goto err_free_mem;
+	}
+
+	wac_i2c->client = client;
+	wac_i2c->input = input;
+	wac_i2c->gpio = gpio;
+
+	input->name = "Wacom I2C Digitizer";
+	input->id.bustype = BUS_I2C;
+	input->id.vendor = 0x56a;
+	input->id.version = features.fw_version;
+	input->dev.parent = &client->dev;
+	input->open = wacom_i2c_open;
+	input->close = wacom_i2c_close;
+
+	input->evbit[0] |= BIT_MASK(EV_KEY) | BIT_MASK(EV_ABS);
+
+	__set_bit(BTN_TOOL_PEN, input->keybit);
+	__set_bit(BTN_TOOL_RUBBER, input->keybit);
+	__set_bit(BTN_STYLUS, input->keybit);
+	__set_bit(BTN_STYLUS2, input->keybit);
+	__set_bit(BTN_TOUCH, input->keybit);
+
+	input_set_abs_params(input, ABS_X, 0, features.x_max, 0, 0);
+	input_set_abs_params(input, ABS_Y, 0, features.y_max, 0, 0);
+	input_set_abs_params(input, ABS_PRESSURE,
+			     0, features.pressure_max, 0, 0);
+
+	input_set_drvdata(input, wac_i2c);
+
+	error = request_threaded_irq(client->irq, NULL, wacom_i2c_irq,
+				     IRQF_TRIGGER_FALLING,
+				     "wacom_i2c", wac_i2c);
+	if (error) {
+		dev_err(&client->dev,
+			"Failed to enable IRQ, error: %d\n", error);
+		goto err_free_mem;
+	}
+
+	/* Disable the IRQ, we'll enable it in wac_i2c_open() */
+	disable_irq(client->irq);
+
+	error = input_register_device(wac_i2c->input);
+	if (error) {
+		dev_err(&client->dev,
+			"Failed to register input device, error: %d\n", error);
+		goto err_free_irq;
+	}
+
+	i2c_set_clientdata(client, wac_i2c);
+	return 0;
+
+err_free_irq:
+	free_irq(client->irq, wac_i2c);
+err_free_mem:
+	input_free_device(wac_i2c->input);
+	kfree(wac_i2c);
+
+	return error;
+}
+
+static int __devexit wacom_i2c_remove(struct i2c_client *client)
+{
+	struct wacom_i2c *wac_i2c = i2c_get_clientdata(client);
+
+	free_irq(client->irq, wac_i2c);
+	input_unregister_device(wac_i2c->input);
+	kfree(wac_i2c);
+
+	return 0;
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int wacom_i2c_suspend(struct device *dev)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+
+	disable_irq(client->irq);
+
+	return 0;
+}
+
+static int wacom_i2c_resume(struct device *dev)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+
+	enable_irq(client->irq);
+
+	return 0;
+}
+#endif
+
+static SIMPLE_DEV_PM_OPS(wacom_i2c_pm, wacom_i2c_suspend, wacom_i2c_resume);
+
+static const struct i2c_device_id wacom_i2c_id[] = {
+	{ "WAC_I2C_EMR", 0 },
+	{ },
+};
+MODULE_DEVICE_TABLE(i2c, wacom_i2c_id);
+
+static struct i2c_driver wacom_i2c_driver = {
+	.driver	= {
+		.name	= "wacom_i2c",
+		.owner	= THIS_MODULE,
+		.pm	= &wacom_i2c_pm,
+	},
+
+	.probe		= wacom_i2c_probe,
+	.remove		= __devexit_p(wacom_i2c_remove),
+	.id_table	= wacom_i2c_id,
+};
+module_i2c_driver(wacom_i2c_driver);
+
+MODULE_AUTHOR("Tatsunosuke Tobita <tobita.tatsunosuke@wacom.co.jp>");
+MODULE_DESCRIPTION("WACOM EMR I2C Driver");
+MODULE_LICENSE("GPL");

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

* Re: [PATCH] Input: Wacom Stylus driver for I2C interface
  2012-03-23 18:02 ` Shubhrajyoti
@ 2012-03-26  2:26   ` Tatsunosuke Tobita
  0 siblings, 0 replies; 6+ messages in thread
From: Tatsunosuke Tobita @ 2012-03-26  2:26 UTC (permalink / raw)
  To: Shubhrajyoti; +Cc: linux-input

Hi Shubhrajyoti

Thank you for your comment on this patch.

On Sat, Mar 24, 2012 at 3:02 AM, Shubhrajyoti <shubhrajyoti@ti.com> wrote:
> Hi ,
> Some comments / doubts.
>
> On Friday 23 March 2012 11:10 AM, Tatsunosuke Tobita wrote:
>> From: Tatsunosuke Tobita <tobita.tatsunosuke@wacom.co.jp>
>>
>> This adds support for Wacom Stylus device with I2C interface.
>> The modification from the previous update is replacing module_init and
>> module_exit with module_i2c_driver.
>>
>> Signed-off-by: Tatsunosuke Tobita <tobita.tatsunosuke@wacom.co.jp>
>> ---
>>  drivers/input/touchscreen/wacom_i2c.c |  238 +++++++++++++++++++++++++++++++++
>>  drivers/input/touchscreen/wacom_i2c.h |   55 ++++++++
>>  2 files changed, 293 insertions(+), 0 deletions(-)
>>  create mode 100644 drivers/input/touchscreen/wacom_i2c.c
>>  create mode 100644 drivers/input/touchscreen/wacom_i2c.h
>>
>> diff --git a/drivers/input/touchscreen/wacom_i2c.c b/drivers/input/touchscreen/wacom_i2c.c
>> new file mode 100644
>> index 0000000..f5a3845
>> --- /dev/null
>> +++ b/drivers/input/touchscreen/wacom_i2c.c
>> @@ -0,0 +1,238 @@
>> +/*
>> + * Wacom Penabled Driver for I2C
>> + *
>> + * Copyright (c) 2011 Tatsunosuke Tobita, Wacom.
>> + * <tobita.tatsunosuke@wacom.co.jp>
>> + *
>> + * This program is free software; you can redistribute it
>> + * and/or modify it under the terms of the GNU General
>> + * Public License as published by the Free Software
>> + * Foundation; either version of 2 of the License,
>> + * or (at your option) any later version.
>> + */
>> +
>> +#include "wacom_i2c.h"
>> +
>> +static int wacom_send_query(struct wacom_i2c *wac_i2c)
>> +{
>> +     int ret;
>> +     u8 cmd[4] = {CMD_QUERY0, CMD_QUERY1, CMD_QUERY2, CMD_QUERY3};
>> +     u8 data[COM_COORD_NUM];
>> +
>> +     ret = i2c_master_send(wac_i2c->client, cmd, sizeof(cmd));
>> +     if (ret < 0) {
>> +             dev_printk(KERN_ERR, &wac_i2c->client->dev,
>> +                        "Sending 1st Query failed \n");
> May be use dev_err . Feel free to ignore if you feel so. Just a suggestion
I guess I need to look up dev_err and decide which I should use; thanks.

>> +             return -1;
>
> Why mask the error?
Masking here is because whenever wacom_send_query returns the negative
number, that means
the connection and communication between the hardware and our device
is somehow missed.
The cause was always setting of I2C of our device our our developing
environment while we were testing.
So, I thought returning the -EIO
(Error for Input/Output) is understandable for us. But, I come to
realize that this should be not based on
our thought, but should be considered for someone else, so I will
modify it as more appropriate.

>> +     }
>> +
>> +     cmd[0] = CMD_THROW0; cmd[1] = CMD_THROW1;
>> +     ret = i2c_master_send(wac_i2c->client, cmd, 2);
>> +     if (ret < 0) {
>> +             dev_printk(KERN_ERR, &wac_i2c->client->dev,
>> +                        "Sending 2nd Query failed \n");
>> +             return -1;
> same
>> +     }
>> +
>> +     ret = i2c_master_recv(wac_i2c->client, data, QUERY_SIZE);
>> +     if (ret < 0) {
>> +             dev_printk(KERN_ERR, &wac_i2c->client->dev,
>> +                        "Receiving data failed 1\n");
>> +             return -1;
> same
>> +     }
>> +
>> +     wac_i2c->wac_feature.x_max = get_unaligned_le16(&data[3]);
>> +     wac_i2c->wac_feature.y_max = get_unaligned_le16(&data[5]);
>> +     wac_i2c->wac_feature.pressure_max = get_unaligned_le16(&data[11]);
>> +     wac_i2c->wac_feature.fw_version = get_unaligned_le16(&data[13]);
>> +
>> +     dev_dbg(&wac_i2c->client->dev,
>> +                "x_max:%d, y_max:%d, pressure:%d, fw:%d\n",
>> +                wac_i2c->wac_feature.x_max, wac_i2c->wac_feature.y_max,
>> +                wac_i2c->wac_feature.pressure_max, wac_i2c->wac_feature.fw_version);
>> +
>> +     return 0;
>> +}
>> +
>> +static irqreturn_t wacom_i2c_irq(int irqno, void *param)
>> +{
>> +     struct wacom_i2c *wac_i2c = param;
>> +       int ret;
>> +     unsigned int x, y, pressure;
>> +     unsigned char tsw, f1, f2, ers;
>> +     u8 data[COM_COORD_NUM];
>> +
>> +     do {
>> +             ret = i2c_master_recv(wac_i2c->client, data, QUERY_SIZE);
>> +     } while (gpio_get_value(irq_to_gpio(wac_i2c->client->irq)) == 0);
>> +
>> +     if (ret > 0) {
>> +             tsw = data[3]&0x01;
>> +             ers = data[3]&0x04;
>> +             f1 = data[3]&0x02;
>> +             f2 = data[3]&0x10;
>> +             x = le16_to_cpup((__le16 *)&data[4]);
>> +             y = le16_to_cpup((__le16 *)&data[6]);
>> +             pressure = le16_to_cpup((__le16 *)&data[8]);
>> +
>> +             input_report_abs(wac_i2c->input, ABS_X, x);
>> +             input_report_abs(wac_i2c->input, ABS_Y, y);
>> +             input_report_key(wac_i2c->input, BTN_TOOL_PEN, tsw);
>> +             input_report_key(wac_i2c->input, BTN_TOOL_RUBBER, ers);
>> +             input_report_abs(wac_i2c->input, ABS_PRESSURE, pressure);
>> +             input_report_key(wac_i2c->input, BTN_TOUCH, (tsw || ers));
>> +             input_report_key(wac_i2c->input, BTN_STYLUS, f1);
>> +             input_report_key(wac_i2c->input, BTN_STYLUS2, f2);
>> +
>> +             input_sync(wac_i2c->input);
>> +     }
>> +
>> +     return IRQ_HANDLED;
>> +}
>> +
>> +static int wacom_i2c_input_open(struct input_dev *dev)
>> +{
>> +     struct wacom_i2c *wac_i2c = input_get_drvdata(dev);
>> +     int ret;
>> +     ret = wacom_send_query(wac_i2c);
>> +     if (ret < 0)
>> +             return -EIO;
>> +
>> +     wac_i2c->input->id.version = wac_i2c->wac_feature.fw_version;
>> +     input_set_abs_params(wac_i2c->input, ABS_X, 0,
>> +                          wac_i2c->wac_feature.x_max, 0, 0);
>> +     input_set_abs_params(wac_i2c->input, ABS_Y, 0,
>> +                          wac_i2c->wac_feature.y_max, 0, 0);
>> +     input_set_abs_params(wac_i2c->input, ABS_PRESSURE, 0,
>> +                          wac_i2c->wac_feature.pressure_max, 0, 0);
>> +
>> +     return 0;
>> +}
>> +
>> +static void wacom_i2c_input_close(struct input_dev *dev)
>> +{
>> +}
> May be this could be cleaned up?
I see, so I shouldn't leave a function without any operation, right?
Leaving here is because the input_open and input_close are considered to
be registered as set. I will delete this function. Thanks!

>> +
>> +static int __devinit wacom_i2c_probe(struct i2c_client *client,
>> +                        const struct i2c_device_id *id)
>> +{
>> +     struct wacom_i2c *wac_i2c;
>> +     int err;
>> +     u8 data[COM_COORD_NUM];
>> +
>> +     if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
>> +             err = -EIO;
>> +             goto fail1;
>> +     }
>> +
>> +     wac_i2c = kzalloc(sizeof(*wac_i2c), GFP_KERNEL);
>> +     if (wac_i2c == NULL) {
>> +             err = -ENOMEM;
>> +             goto fail2;
>> +     }
>> +
>> +     wac_i2c->input = input_allocate_device();
>> +     if (wac_i2c->input == NULL) {
>> +             err = -ENOMEM;
>> +             goto fail2;
> We are leaking wac_i2c here ?
Sorry, can you tell more detail?
I guess you are talking about the registration of wac_i2c->input when
it errors, but not completely sure if
you mean either input registration or wac_i2c itself, or both.

>> +     }
>> +
>> +     wac_i2c->client = client;
>> +     i2c_set_clientdata(client, wac_i2c);
>> +     mutex_init(&wac_i2c->lock);
>> +
>> +     __set_bit(BTN_TOOL_PEN, wac_i2c->input->keybit);
>> +     __set_bit(BTN_TOOL_RUBBER, wac_i2c->input->keybit);
>> +     __set_bit(BTN_STYLUS, wac_i2c->input->keybit);
>> +     __set_bit(BTN_STYLUS2, wac_i2c->input->keybit);
>> +     __set_bit(BTN_TOUCH, wac_i2c->input->keybit);
>> +
>> +     wac_i2c->input->name = "Wacom I2C Digitizer";
>> +     wac_i2c->input->id.bustype = BUS_I2C;
>> +     wac_i2c->input->id.vendor = 0x56a;
>> +     wac_i2c->input->dev.parent = &client->dev;
>> +     wac_i2c->input->open = wacom_i2c_input_open;
>> +     wac_i2c->input->close = wacom_i2c_input_close;
>> +     wac_i2c->input->evbit[0] |= BIT_MASK(EV_KEY) | BIT_MASK(EV_ABS);
>> +     input_set_drvdata(wac_i2c->input, wac_i2c);
>> +
>> +     if (input_register_device(wac_i2c->input)) {
>> +             dev_printk(KERN_ERR, &wac_i2c->client->dev, "Failed to register input device \n");
>> +             err = -ENODEV;
>> +             goto fail3;
>> +     }
>> +
>> +
>> +     err = request_threaded_irq(wac_i2c->client->irq, NULL,
>> +                                wacom_i2c_irq, IRQF_TRIGGER_FALLING, "WACOM_I2C_IRQ", wac_i2c);
>> +     if (err) {
>> +             dev_printk(KERN_ERR, &wac_i2c->client->dev, "Failed to enable IRQ for EMR \n");
>> +             goto fail3;
>> +     }
>> +
>> +     /*Clear the buffer in the firmware*/
>> +     do {
>> +             i2c_master_recv(wac_i2c->client, data, QUERY_SIZE);
>> +     } while (gpio_get_value(irq_to_gpio(wac_i2c->client->irq)) == 0);
>> +
>> +     return 0;
>> +
>> + fail3:
>> +     input_free_device(wac_i2c->input);
>> + fail2:
>> +     dev_err(&wac_i2c->client->dev, "Freeing device \n");
>> + fail1:
>> +     return err;
>> +}
>> +
>> +static int __devexit wacom_i2c_remove(struct i2c_client *client)
>> +{
>> +     struct wacom_i2c *wac_i2c = i2c_get_clientdata(client);
>> +
>> +     free_irq(wac_i2c->client->irq, wac_i2c);
>> +     input_unregister_device(wac_i2c->input);
>> +     kfree(wac_i2c);
>> +
>> +     return 0;
>> +}
>> +
>> +static int wacom_i2c_suspend(struct device *dev)
>> +{
>> +     struct i2c_client *client = to_i2c_client(dev);
>> +     disable_irq(client->irq);
> Why is this disable needed?
Our device especially for the tablet PCs, sometimes we were asked to implement
 for entering the mode that reduces the cost of the electric current,
which is similar to sleep mode
when systems are in suspend. The waking up from this mode are easy;
just get our stylus close to the screen
even when the systems in suspend mode. However, the waking up
shouldn't happen when the systems in the suspend mode,
so disabling irq allows our device not to detect that our pen is
getting close to it since the irq is the line detecting our stylus's
signal.
We haven't used this mode a lot these days, but not sure what will be
in the future.
As a result, I leave the operation for disabling irq here.

>> +
>> +     return 0;
>> +}
>> +
>> +static int wacom_i2c_resume(struct device *dev)
>> +{
>> +     struct i2c_client *client = to_i2c_client(dev);
>> +     enable_irq(client->irq);
>> +
>> +     return 0;
>> +}
>> +
>> +static SIMPLE_DEV_PM_OPS(wacom_i2c_pm, wacom_i2c_suspend, wacom_i2c_resume);
>> +
>> +static const struct i2c_device_id wacom_i2c_id[] = {
>> +     {WACNAME, 0},
>> +     {},
>> +};
>> +MODULE_DEVICE_TABLE(i2c, wacom_i2c_id);
>> +
>> +static struct i2c_driver wacom_i2c_driver = {
>> +     .driver = {
>> +             .name = "wacom_i2c",
>> +             .owner = THIS_MODULE,
>> +             .pm = &wacom_i2c_pm,
>> +     },
>> +
>> +     .probe = wacom_i2c_probe,
>> +     .remove = __devexit_p(wacom_i2c_remove),
>> +     .id_table = wacom_i2c_id,
>> +};
>> +module_i2c_driver(wacom_i2c_driver);
>> +
>> +MODULE_AUTHOR("Tatsunosuke Tobita <tobita.tatsunosuke@wacom.co.jp>");
>> +MODULE_DESCRIPTION("WACOM EMR I2C Driver");
>> +MODULE_LICENSE("GPL");
>> diff --git a/drivers/input/touchscreen/wacom_i2c.h b/drivers/input/touchscreen/wacom_i2c.h
>> new file mode 100644
>> index 0000000..b5c2d07
>> --- /dev/null
>> +++ b/drivers/input/touchscreen/wacom_i2c.h
>> @@ -0,0 +1,55 @@
>> +/*
>> + * Wacom Penabled Driver for I2C
>> + *
>> + * Copyright (c) 2011 Tatsunosuke Tobita, Wacom.
>> +
> nit : 2012?
I almost forget I sign it last year.

Thanks!

Tats
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [PATCH] Input: Wacom Stylus driver for I2C interface
  2012-03-25 23:50 ` Dmitry Torokhov
@ 2012-03-26  3:05   ` Tatsunosuke Tobita
       [not found]   ` <4f755be3.42d1440a.31fc.3511SMTPIN_ADDED@mx.google.com>
  1 sibling, 0 replies; 6+ messages in thread
From: Tatsunosuke Tobita @ 2012-03-26  3:05 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: linux-input, Shubhrajyoti

Hi Dmitry

On Mon, Mar 26, 2012 at 8:50 AM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> Hi Tatsunosuke,
>
> On Fri, Mar 23, 2012 at 02:40:07PM +0900, Tatsunosuke Tobita wrote:
>> From: Tatsunosuke Tobita <tobita.tatsunosuke@wacom.co.jp>
>>
>> This adds support for Wacom Stylus device with I2C interface.
>> The modification from the previous update is replacing module_init and
>> module_exit with module_i2c_driver.
>>
>> Signed-off-by: Tatsunosuke Tobita <tobita.tatsunosuke@wacom.co.jp>
>> ---
>>  drivers/input/touchscreen/wacom_i2c.c |  238 +++++++++++++++++++++++++++++++++
>>  drivers/input/touchscreen/wacom_i2c.h |   55 ++++++++
>>  2 files changed, 293 insertions(+), 0 deletions(-)
>>  create mode 100644 drivers/input/touchscreen/wacom_i2c.c
>>  create mode 100644 drivers/input/touchscreen/wacom_i2c.h
>>
>> diff --git a/drivers/input/touchscreen/wacom_i2c.c b/drivers/input/touchscreen/wacom_i2c.c
>> new file mode 100644
>> index 0000000..f5a3845
>> --- /dev/null
>> +++ b/drivers/input/touchscreen/wacom_i2c.c
>> @@ -0,0 +1,238 @@
>> +/*
>> + * Wacom Penabled Driver for I2C
>> + *
>> + * Copyright (c) 2011 Tatsunosuke Tobita, Wacom.
>> + * <tobita.tatsunosuke@wacom.co.jp>
>> + *
>> + * This program is free software; you can redistribute it
>> + * and/or modify it under the terms of the GNU General
>> + * Public License as published by the Free Software
>> + * Foundation; either version of 2 of the License,
>> + * or (at your option) any later version.
>> + */
>> +
>> +#include "wacom_i2c.h"
>> +
>> +static int wacom_send_query(struct wacom_i2c *wac_i2c)
>> +{
>> +     int ret;
>> +     u8 cmd[4] = {CMD_QUERY0, CMD_QUERY1, CMD_QUERY2, CMD_QUERY3};
>> +     u8 data[COM_COORD_NUM];
>> +
>> +     ret = i2c_master_send(wac_i2c->client, cmd, sizeof(cmd));
>> +     if (ret < 0) {
>> +             dev_printk(KERN_ERR, &wac_i2c->client->dev,
>> +                        "Sending 1st Query failed \n");
>> +             return -1;
>
> As Shubhrajyoti already mentioned you should not clobber errors returned
> by i2c_master_send with a generic -1 (-EPERM).
Right, I will check on this.

>
>> +     }
>> +
>> +     cmd[0] = CMD_THROW0; cmd[1] = CMD_THROW1;
>> +     ret = i2c_master_send(wac_i2c->client, cmd, 2);
>> +     if (ret < 0) {
>> +             dev_printk(KERN_ERR, &wac_i2c->client->dev,
>> +                        "Sending 2nd Query failed \n");
>> +             return -1;
>> +     }
>> +
>> +     ret = i2c_master_recv(wac_i2c->client, data, QUERY_SIZE);
>> +     if (ret < 0) {
>> +             dev_printk(KERN_ERR, &wac_i2c->client->dev,
>> +                        "Receiving data failed 1\n");
>> +             return -1;
>> +     }
>
> I wonder if all this could be converted into a single i2c_transfer().
The difficulty combining those into one is because our device allow to
receive the command as
the proper order. Therefore, I split it; sending the first command and
the second command, and receiving the information of the device as the
result.

>
>> +
>> +     wac_i2c->wac_feature.x_max = get_unaligned_le16(&data[3]);
>> +     wac_i2c->wac_feature.y_max = get_unaligned_le16(&data[5]);
>> +     wac_i2c->wac_feature.pressure_max = get_unaligned_le16(&data[11]);
>> +     wac_i2c->wac_feature.fw_version = get_unaligned_le16(&data[13]);
>> +
>> +     dev_dbg(&wac_i2c->client->dev,
>> +                "x_max:%d, y_max:%d, pressure:%d, fw:%d\n",
>> +                wac_i2c->wac_feature.x_max, wac_i2c->wac_feature.y_max,
>> +                wac_i2c->wac_feature.pressure_max, wac_i2c->wac_feature.fw_version);
>> +
>> +     return 0;
>> +}
>> +
>> +static irqreturn_t wacom_i2c_irq(int irqno, void *param)
>> +{
>> +     struct wacom_i2c *wac_i2c = param;
>> +       int ret;
>> +     unsigned int x, y, pressure;
>> +     unsigned char tsw, f1, f2, ers;
>> +     u8 data[COM_COORD_NUM];
>> +
>> +     do {
>> +             ret = i2c_master_recv(wac_i2c->client, data, QUERY_SIZE);
>> +     } while (gpio_get_value(irq_to_gpio(wac_i2c->client->irq)) == 0);
>
> irq_to_gpio() may fail. Also it does not make sense to do the conversion
> in interrupt handler if it can be done once, at probe() time.
Our device stores data as many as possible in buffer when the stylus
is in proximity and it
keeps an irq line low. So, we should check if all data is transfered;
otherwise, the device
keeps the irq line as low and no interrupt could not be detected after words.
So, in fact, this is implemented just in case. Should I use work-queue
here to do the same functionality?

>
>> +
>> +     if (ret > 0) {
>> +             tsw = data[3]&0x01;
>> +             ers = data[3]&0x04;
>> +             f1 = data[3]&0x02;
>> +             f2 = data[3]&0x10;
>> +             x = le16_to_cpup((__le16 *)&data[4]);
>> +             y = le16_to_cpup((__le16 *)&data[6]);
>> +             pressure = le16_to_cpup((__le16 *)&data[8]);
>> +
>> +             input_report_abs(wac_i2c->input, ABS_X, x);
>> +             input_report_abs(wac_i2c->input, ABS_Y, y);
>> +             input_report_key(wac_i2c->input, BTN_TOOL_PEN, tsw);
>> +             input_report_key(wac_i2c->input, BTN_TOOL_RUBBER, ers);
>> +             input_report_abs(wac_i2c->input, ABS_PRESSURE, pressure);
>> +             input_report_key(wac_i2c->input, BTN_TOUCH, (tsw || ers));
>> +             input_report_key(wac_i2c->input, BTN_STYLUS, f1);
>> +             input_report_key(wac_i2c->input, BTN_STYLUS2, f2);
>> +
>> +             input_sync(wac_i2c->input);
>> +     }
>> +
>> +     return IRQ_HANDLED;
>> +}
>> +
>> +static int wacom_i2c_input_open(struct input_dev *dev)
>> +{
>> +     struct wacom_i2c *wac_i2c = input_get_drvdata(dev);
>> +     int ret;
>> +     ret = wacom_send_query(wac_i2c);
>> +     if (ret < 0)
>> +             return -EIO;
>> +
>> +     wac_i2c->input->id.version = wac_i2c->wac_feature.fw_version;
>> +     input_set_abs_params(wac_i2c->input, ABS_X, 0,
>> +                          wac_i2c->wac_feature.x_max, 0, 0);
>> +     input_set_abs_params(wac_i2c->input, ABS_Y, 0,
>> +                          wac_i2c->wac_feature.y_max, 0, 0);
>> +     input_set_abs_params(wac_i2c->input, ABS_PRESSURE, 0,
>> +                          wac_i2c->wac_feature.pressure_max, 0, 0);
>
> This should be done when registering input device.
>
>> +
>> +     return 0;
>> +}
>> +
>> +static void wacom_i2c_input_close(struct input_dev *dev)
>> +{
>> +}
>
> Empty methods are not needed.
>
>> +
>> +static int __devinit wacom_i2c_probe(struct i2c_client *client,
>> +                        const struct i2c_device_id *id)
>> +{
>> +     struct wacom_i2c *wac_i2c;
>> +     int err;
>> +     u8 data[COM_COORD_NUM];
>> +
>> +     if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
>> +             err = -EIO;
>> +             goto fail1;
>> +     }
>> +
>> +     wac_i2c = kzalloc(sizeof(*wac_i2c), GFP_KERNEL);
>> +     if (wac_i2c == NULL) {
>> +             err = -ENOMEM;
>> +             goto fail2;
>> +     }
>> +
>> +     wac_i2c->input = input_allocate_device();
>> +     if (wac_i2c->input == NULL) {
>> +             err = -ENOMEM;
>> +             goto fail2;
>> +     }
>> +
>> +     wac_i2c->client = client;
>> +     i2c_set_clientdata(client, wac_i2c);
>> +     mutex_init(&wac_i2c->lock);
>
> What is this lock used for?
This should be  considered for the future when we need it at first, but
I understand that should be included when it is exactly needed.
I will get rid of this.
>
>> +
>> +     __set_bit(BTN_TOOL_PEN, wac_i2c->input->keybit);
>> +     __set_bit(BTN_TOOL_RUBBER, wac_i2c->input->keybit);
>> +     __set_bit(BTN_STYLUS, wac_i2c->input->keybit);
>> +     __set_bit(BTN_STYLUS2, wac_i2c->input->keybit);
>> +     __set_bit(BTN_TOUCH, wac_i2c->input->keybit);
>> +
>> +     wac_i2c->input->name = "Wacom I2C Digitizer";
>> +     wac_i2c->input->id.bustype = BUS_I2C;
>> +     wac_i2c->input->id.vendor = 0x56a;
>> +     wac_i2c->input->dev.parent = &client->dev;
>> +     wac_i2c->input->open = wacom_i2c_input_open;
>> +     wac_i2c->input->close = wacom_i2c_input_close;
>> +     wac_i2c->input->evbit[0] |= BIT_MASK(EV_KEY) | BIT_MASK(EV_ABS);
>> +     input_set_drvdata(wac_i2c->input, wac_i2c);
>> +
>> +     if (input_register_device(wac_i2c->input)) {
>> +             dev_printk(KERN_ERR, &wac_i2c->client->dev, "Failed to register input device \n");
>
> dev_err() and dev_info() throughout.
>
>> +             err = -ENODEV;
>
> The proper error is ENOMEM.
>
>> +             goto fail3;
>> +     }
>> +
>> +
>> +     err = request_threaded_irq(wac_i2c->client->irq, NULL,
>> +                                wacom_i2c_irq, IRQF_TRIGGER_FALLING, "WACOM_I2C_IRQ", wac_i2c);
>> +     if (err) {
>> +             dev_printk(KERN_ERR, &wac_i2c->client->dev, "Failed to enable IRQ for EMR \n");
>> +             goto fail3;
>> +     }
>> +
>> +     /*Clear the buffer in the firmware*/
>> +     do {
>> +             i2c_master_recv(wac_i2c->client, data, QUERY_SIZE);
>> +     } while (gpio_get_value(irq_to_gpio(wac_i2c->client->irq)) == 0);
>
> This should be done at open() time.
>
>> +
>> +     return 0;
>> +
>> + fail3:
>> +     input_free_device(wac_i2c->input);
>> + fail2:
>> +     dev_err(&wac_i2c->client->dev, "Freeing device \n");
>> + fail1:
>> +     return err;
>> +}
>> +
>> +static int __devexit wacom_i2c_remove(struct i2c_client *client)
>> +{
>> +     struct wacom_i2c *wac_i2c = i2c_get_clientdata(client);
>> +
>> +     free_irq(wac_i2c->client->irq, wac_i2c);
>> +     input_unregister_device(wac_i2c->input);
>> +     kfree(wac_i2c);
>> +
>> +     return 0;
>> +}
>> +
>> +static int wacom_i2c_suspend(struct device *dev)
>> +{
>> +     struct i2c_client *client = to_i2c_client(dev);
>> +     disable_irq(client->irq);
>
> I guess disabling IRQ is OK for now, but don't you want to use it as a
> wakeup source?
Can you tell more in detail? I don't understand exactly mean "wakeup source".
Thanks.

>
>> +
>> +     return 0;
>> +}
>> +
>> +static int wacom_i2c_resume(struct device *dev)
>> +{
>> +     struct i2c_client *client = to_i2c_client(dev);
>> +     enable_irq(client->irq);
>> +
>> +     return 0;
>> +}
>
> Suspend and resume should be guarded by #ifdef CONFIG_PM_SLEEP
>
>> +
>> +static SIMPLE_DEV_PM_OPS(wacom_i2c_pm, wacom_i2c_suspend, wacom_i2c_resume);
>> +
>> +static const struct i2c_device_id wacom_i2c_id[] = {
>> +     {WACNAME, 0},
>> +     {},
>> +};
>> +MODULE_DEVICE_TABLE(i2c, wacom_i2c_id);
>> +
>> +static struct i2c_driver wacom_i2c_driver = {
>> +     .driver = {
>> +             .name = "wacom_i2c",
>> +             .owner = THIS_MODULE,
>> +             .pm = &wacom_i2c_pm,
>> +     },
>> +
>> +     .probe = wacom_i2c_probe,
>> +     .remove = __devexit_p(wacom_i2c_remove),
>> +     .id_table = wacom_i2c_id,
>> +};
>> +module_i2c_driver(wacom_i2c_driver);
>> +
>> +MODULE_AUTHOR("Tatsunosuke Tobita <tobita.tatsunosuke@wacom.co.jp>");
>> +MODULE_DESCRIPTION("WACOM EMR I2C Driver");
>> +MODULE_LICENSE("GPL");
>> diff --git a/drivers/input/touchscreen/wacom_i2c.h b/drivers/input/touchscreen/wacom_i2c.h
>> new file mode 100644
>> index 0000000..b5c2d07
>> --- /dev/null
>> +++ b/drivers/input/touchscreen/wacom_i2c.h
>> @@ -0,0 +1,55 @@
>> +/*
>> + * Wacom Penabled Driver for I2C
>> + *
>> + * Copyright (c) 2011 Tatsunosuke Tobita, Wacom.
>> + * <tobita.tatsunosuke@wacom.co.jp>
>> + *
>> + * This program is free software; you can redistribute it
>> + * and/or modify it under the terms of the GNU General
>> + * Public License as published by the Free Software
>> + * Foundation; either version of 2 of the License,
>> + * or (at your option) any later version.
>> + */
>> +
>> +#ifndef WACOM_I2C_H
>> +#define WACOM_I2C_H
>> +
>> +#include <linux/module.h>
>> +#include <linux/input.h>
>> +#include <linux/i2c.h>
>> +#include <linux/slab.h>
>> +#include <linux/irq.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/gpio.h>
>> +#include <asm/unaligned.h>
>> +
>> +#define NAMEBUF 12
>
> Not used.
>
>> +#define WACNAME "WAC_I2C_EMR"
>> +
>> +/*Wacom Command*/
>> +#define COM_COORD_NUM      128
>> +#define CMD_QUERY0         0x04
>> +#define CMD_QUERY1         0x00
>> +#define CMD_QUERY2         0x33
>> +#define CMD_QUERY3         0x02
>> +#define CMD_THROW0         0x05
>> +#define CMD_THROW1         0x00
>> +#define QUERY_SIZE           19
>> +
>> +/*Parameters for wacom own features*/
>> +struct wacom_features {
>> +     int x_max;
>> +     int y_max;
>> +     int pressure_max;
>> +     char fw_version;
>> +};
>> +
>> +/*Parameters for i2c driver*/
>> +struct wacom_i2c {
>> +     struct i2c_client *client;
>> +     struct input_dev *input;
>> +     struct mutex lock;
>
> Not used.
>
>> +     struct wacom_features wac_feature;
>
> No need to carry this around, you only use it once.
>
>> +     int type;
>
> Not needed.
>
>> +};
>> +#endif
>
> I also do not see why you need a separate header file and why you split
> this into 2 patches - one with driver and another with Kconfig/Makefile
> adjustments.
Separating the codes may be clear to see, I thought.
The reason I made 2 patches is because when combining drivers and one
with Kconfig/Makefile,
it fails to patch in high percentage. On the other hand, when I make
two patched separately, it always
succeeds. That is the reason.

>
> Could you please try the version below and let me know if it works for
> you. Thanks!
Thank you for your support and help; this is much appreciated.
I will look through the code you created and learn more about your
idea; then compile
it and tell you the result!
Hope, it won't take long ;)

Tats

>
> --
> Dmitry
>
>
> Input: add support for Wacom Stylus device with I2C interface
>
> From: Tatsunosuke Tobita <tobita.tatsunosuke@wacom.co.jp>
>
> This adds support for Wacom Stylus device with I2C interface.
>
> Signed-off-by: Tatsunosuke Tobita <tobita.tatsunosuke@wacom.co.jp>
> Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
> ---
>
>  drivers/input/touchscreen/Kconfig     |   12 +
>  drivers/input/touchscreen/Makefile    |    1
>  drivers/input/touchscreen/wacom_i2c.c |  338 +++++++++++++++++++++++++++++++++
>  3 files changed, 351 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/input/touchscreen/wacom_i2c.c
>
>
> diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
> index 3594591..1773001 100644
> --- a/drivers/input/touchscreen/Kconfig
> +++ b/drivers/input/touchscreen/Kconfig
> @@ -323,6 +323,18 @@ config TOUCHSCREEN_WACOM_W8001
>          To compile this driver as a module, choose M here: the
>          module will be called wacom_w8001.
>
> +config TOUCHSCREEN_WACOM_I2C
> +       tristate "Wacom Tablet support (I2C)"
> +       depends on I2C
> +       help
> +         Say Y here if you want to use the I2C version of the Wacom
> +         Pen Tablet.
> +
> +         If unsure, say N.
> +
> +         To compile this driver as a module, choose M here: the module
> +         will be called wacom_i2c.
> +
>  config TOUCHSCREEN_LPC32XX
>        tristate "LPC32XX touchscreen controller"
>        depends on ARCH_LPC32XX
> diff --git a/drivers/input/touchscreen/Makefile b/drivers/input/touchscreen/Makefile
> index b6c746e0..eb8bfe1 100644
> --- a/drivers/input/touchscreen/Makefile
> +++ b/drivers/input/touchscreen/Makefile
> @@ -60,6 +60,7 @@ obj-$(CONFIG_TOUCHSCREEN_TSC2005)     += tsc2005.o
>  obj-$(CONFIG_TOUCHSCREEN_TSC2007)      += tsc2007.o
>  obj-$(CONFIG_TOUCHSCREEN_UCB1400)      += ucb1400_ts.o
>  obj-$(CONFIG_TOUCHSCREEN_WACOM_W8001)  += wacom_w8001.o
> +obj-$(CONFIG_TOUCHSCREEN_WACOM_I2C)    += wacom_i2c.o
>  obj-$(CONFIG_TOUCHSCREEN_WM831X)       += wm831x-ts.o
>  obj-$(CONFIG_TOUCHSCREEN_WM97XX)       += wm97xx-ts.o
>  wm97xx-ts-$(CONFIG_TOUCHSCREEN_WM9705) += wm9705.o
> diff --git a/drivers/input/touchscreen/wacom_i2c.c b/drivers/input/touchscreen/wacom_i2c.c
> new file mode 100644
> index 0000000..88e0800
> --- /dev/null
> +++ b/drivers/input/touchscreen/wacom_i2c.c
> @@ -0,0 +1,338 @@
> +/*
> + * Wacom Penabled Driver for I2C
> + *
> + * Copyright (c) 2011 Tatsunosuke Tobita, Wacom.
> + * <tobita.tatsunosuke@wacom.co.jp>
> + *
> + * This program is free software; you can redistribute it
> + * and/or modify it under the terms of the GNU General
> + * Public License as published by the Free Software
> + * Foundation; either version of 2 of the License,
> + * or (at your option) any later version.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/input.h>
> +#include <linux/i2c.h>
> +#include <linux/slab.h>
> +#include <linux/irq.h>
> +#include <linux/interrupt.h>
> +#include <linux/gpio.h>
> +#include <asm/unaligned.h>
> +
> +#define WACOM_CMD_QUERY0       0x04
> +#define WACOM_CMD_QUERY1       0x00
> +#define WACOM_CMD_QUERY2       0x33
> +#define WACOM_CMD_QUERY3       0x02
> +#define WACOM_CMD_THROW0       0x05
> +#define WACOM_CMD_THROW1       0x00
> +#define WACOM_QUERY_SIZE       19
> +#define WACOM_RETRY_CNT                100
> +
> +struct wacom_features {
> +       int x_max;
> +       int y_max;
> +       int pressure_max;
> +       char fw_version;
> +};
> +
> +struct wacom_i2c {
> +       struct i2c_client *client;
> +       struct input_dev *input;
> +       unsigned int gpio;
> +       u8 data[WACOM_QUERY_SIZE];
> +};
> +
> +static int wacom_query_device(struct i2c_client *client,
> +                             struct wacom_features *features)
> +{
> +       int ret;
> +       u8 cmd1[] = { WACOM_CMD_QUERY0, WACOM_CMD_QUERY1,
> +                       WACOM_CMD_QUERY2, WACOM_CMD_QUERY3 };
> +       u8 cmd2[] = { WACOM_CMD_THROW0, WACOM_CMD_THROW1 };
> +       u8 data[WACOM_QUERY_SIZE];
> +#if 1
> +       struct i2c_msg msgs[] = {
> +               {
> +                       .addr = client->addr,
> +                       .flags = 0,
> +                       .len = sizeof(cmd1),
> +                       .buf = cmd1,
> +               },
> +               {
> +                       .addr = client->addr,
> +                       .flags = 0,
> +                       .len = sizeof(cmd2),
> +                       .buf = cmd2,
> +               },
> +               {
> +                       .addr = client->addr,
> +                       .flags = I2C_M_RD,
> +                       .len = sizeof(data),
> +                       .buf = data,
> +               },
> +       };
> +
> +       ret = i2c_transfer(client->adapter, msgs, ARRAY_SIZE(msgs));
> +       if (ret < 0)
> +               return ret;
> +       if (ret != ARRAY_SIZE(msgs))
> +               return -EIO;
> +#else
> +       ret = i2c_master_send(client, cmd1, sizeof(cmd1));
> +       if (ret < 0) {
> +               dev_err(&client->dev,
> +                       "Sending 1st Query failed, error: %d\n", ret);
> +               return ret;
> +       }
> +
> +       ret = i2c_master_send(client, cmd2, sizeof(cmd2));
> +       if (ret < 0) {
> +               dev_err(&client->dev,
> +                       "Sending 2nd Query failed, error: %d\n", ret);
> +               return ret;
> +       }
> +
> +       ret = i2c_master_recv(client, data, sizeof(data));
> +       if (ret < 0) {
> +               dev_err(&client->dev,
> +                       "Receiving data failed, error: %d\n", ret);
> +               return ret;
> +       }
> +#endif
> +
> +       features->x_max = get_unaligned_le16(&data[3]);
> +       features->y_max = get_unaligned_le16(&data[5]);
> +       features->pressure_max = get_unaligned_le16(&data[11]);
> +       features->fw_version = get_unaligned_le16(&data[13]);
> +
> +       dev_dbg(&client->dev,
> +               "x_max:%d, y_max:%d, pressure:%d, fw:%d\n",
> +               features->x_max, features->y_max,
> +               features->pressure_max, features->fw_version);
> +
> +       return 0;
> +}
> +
> +static int wacom_i2c_fetch_data(struct wacom_i2c *wac_i2c)
> +{
> +       int retries = 0;
> +       int ret;
> +
> +       do {
> +               ret = i2c_master_recv(wac_i2c->client,
> +                                     wac_i2c->data, sizeof(wac_i2c->data));
> +       } while (gpio_get_value(wac_i2c->gpio) == 0 &&
> +                retries++ < WACOM_RETRY_CNT);
> +
> +       if (retries >= WACOM_RETRY_CNT)
> +               ret = -EIO;
> +
> +       return ret < 0 ? ret : 0;
> +}
> +
> +static irqreturn_t wacom_i2c_irq(int irq, void *dev_id)
> +{
> +       struct wacom_i2c *wac_i2c = dev_id;
> +       struct input_dev *input = wac_i2c->input;
> +       u8 *data = wac_i2c->data;
> +       unsigned int x, y, pressure;
> +       unsigned char tsw, f1, f2, ers;
> +       int error;
> +
> +       error = wacom_i2c_fetch_data(wac_i2c);
> +       if (error)
> +               goto out;
> +
> +       tsw = data[3] & 0x01;
> +       ers = data[3] & 0x04;
> +       f1 = data[3] & 0x02;
> +       f2 = data[3] & 0x10;
> +       x = le16_to_cpup((__le16 *)&data[4]);
> +       y = le16_to_cpup((__le16 *)&data[6]);
> +       pressure = le16_to_cpup((__le16 *)&data[8]);
> +
> +       input_report_key(input, BTN_TOUCH, tsw || ers);
> +       input_report_key(input, BTN_TOOL_PEN, tsw);
> +       input_report_key(input, BTN_TOOL_RUBBER, ers);
> +       input_report_key(input, BTN_STYLUS, f1);
> +       input_report_key(input, BTN_STYLUS2, f2);
> +       input_report_abs(input, ABS_X, x);
> +       input_report_abs(input, ABS_Y, y);
> +       input_report_abs(input, ABS_PRESSURE, pressure);
> +
> +       input_sync(wac_i2c->input);
> +out:
> +       return IRQ_HANDLED;
> +}
> +
> +static int wacom_i2c_open(struct input_dev *dev)
> +{
> +       struct wacom_i2c *wac_i2c = input_get_drvdata(dev);
> +       struct i2c_client *client = wac_i2c->client;
> +       int error;
> +
> +       /* Clear the device buffer */
> +       error = wacom_i2c_fetch_data(wac_i2c);
> +       if (error)
> +               return error;
> +
> +       enable_irq(client->irq);
> +
> +       return 0;
> +}
> +
> +static void wacom_i2c_close(struct input_dev *dev)
> +{
> +       struct wacom_i2c *wac_i2c = input_get_drvdata(dev);
> +       struct i2c_client *client = wac_i2c->client;
> +
> +       disable_irq(client->irq);
> +}
> +
> +static int __devinit wacom_i2c_probe(struct i2c_client *client,
> +                                    const struct i2c_device_id *id)
> +{
> +       struct wacom_i2c *wac_i2c;
> +       struct input_dev *input;
> +       struct wacom_features features;
> +       int gpio;
> +       int error;
> +
> +       if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
> +               dev_err(&client->dev, "i2c_check_functionality error\n");
> +               return -EIO;
> +       }
> +
> +       gpio = irq_to_gpio(client->irq);
> +       if (gpio < 0) {
> +               error = gpio;
> +               dev_err(&client->dev,
> +                       "irq_to_gpio() failed, error: %d\n", error);
> +               return error;
> +       }
> +
> +       error = wacom_query_device(client, &features);
> +       if (error)
> +               return error;
> +
> +       wac_i2c = kzalloc(sizeof(*wac_i2c), GFP_KERNEL);
> +       input = input_allocate_device();
> +       if (!wac_i2c || !input) {
> +               error = -ENOMEM;
> +               goto err_free_mem;
> +       }
> +
> +       wac_i2c->client = client;
> +       wac_i2c->input = input;
> +       wac_i2c->gpio = gpio;
> +
> +       input->name = "Wacom I2C Digitizer";
> +       input->id.bustype = BUS_I2C;
> +       input->id.vendor = 0x56a;
> +       input->id.version = features.fw_version;
> +       input->dev.parent = &client->dev;
> +       input->open = wacom_i2c_open;
> +       input->close = wacom_i2c_close;
> +
> +       input->evbit[0] |= BIT_MASK(EV_KEY) | BIT_MASK(EV_ABS);
> +
> +       __set_bit(BTN_TOOL_PEN, input->keybit);
> +       __set_bit(BTN_TOOL_RUBBER, input->keybit);
> +       __set_bit(BTN_STYLUS, input->keybit);
> +       __set_bit(BTN_STYLUS2, input->keybit);
> +       __set_bit(BTN_TOUCH, input->keybit);
> +
> +       input_set_abs_params(input, ABS_X, 0, features.x_max, 0, 0);
> +       input_set_abs_params(input, ABS_Y, 0, features.y_max, 0, 0);
> +       input_set_abs_params(input, ABS_PRESSURE,
> +                            0, features.pressure_max, 0, 0);
> +
> +       input_set_drvdata(input, wac_i2c);
> +
> +       error = request_threaded_irq(client->irq, NULL, wacom_i2c_irq,
> +                                    IRQF_TRIGGER_FALLING,
> +                                    "wacom_i2c", wac_i2c);
> +       if (error) {
> +               dev_err(&client->dev,
> +                       "Failed to enable IRQ, error: %d\n", error);
> +               goto err_free_mem;
> +       }
> +
> +       /* Disable the IRQ, we'll enable it in wac_i2c_open() */
> +       disable_irq(client->irq);
> +
> +       error = input_register_device(wac_i2c->input);
> +       if (error) {
> +               dev_err(&client->dev,
> +                       "Failed to register input device, error: %d\n", error);
> +               goto err_free_irq;
> +       }
> +
> +       i2c_set_clientdata(client, wac_i2c);
> +       return 0;
> +
> +err_free_irq:
> +       free_irq(client->irq, wac_i2c);
> +err_free_mem:
> +       input_free_device(wac_i2c->input);
> +       kfree(wac_i2c);
> +
> +       return error;
> +}
> +
> +static int __devexit wacom_i2c_remove(struct i2c_client *client)
> +{
> +       struct wacom_i2c *wac_i2c = i2c_get_clientdata(client);
> +
> +       free_irq(client->irq, wac_i2c);
> +       input_unregister_device(wac_i2c->input);
> +       kfree(wac_i2c);
> +
> +       return 0;
> +}
> +
> +#ifdef CONFIG_PM_SLEEP
> +static int wacom_i2c_suspend(struct device *dev)
> +{
> +       struct i2c_client *client = to_i2c_client(dev);
> +
> +       disable_irq(client->irq);
> +
> +       return 0;
> +}
> +
> +static int wacom_i2c_resume(struct device *dev)
> +{
> +       struct i2c_client *client = to_i2c_client(dev);
> +
> +       enable_irq(client->irq);
> +
> +       return 0;
> +}
> +#endif
> +
> +static SIMPLE_DEV_PM_OPS(wacom_i2c_pm, wacom_i2c_suspend, wacom_i2c_resume);
> +
> +static const struct i2c_device_id wacom_i2c_id[] = {
> +       { "WAC_I2C_EMR", 0 },
> +       { },
> +};
> +MODULE_DEVICE_TABLE(i2c, wacom_i2c_id);
> +
> +static struct i2c_driver wacom_i2c_driver = {
> +       .driver = {
> +               .name   = "wacom_i2c",
> +               .owner  = THIS_MODULE,
> +               .pm     = &wacom_i2c_pm,
> +       },
> +
> +       .probe          = wacom_i2c_probe,
> +       .remove         = __devexit_p(wacom_i2c_remove),
> +       .id_table       = wacom_i2c_id,
> +};
> +module_i2c_driver(wacom_i2c_driver);
> +
> +MODULE_AUTHOR("Tatsunosuke Tobita <tobita.tatsunosuke@wacom.co.jp>");
> +MODULE_DESCRIPTION("WACOM EMR I2C Driver");
> +MODULE_LICENSE("GPL");
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [PATCH] Input: Wacom Stylus driver for I2C interface
       [not found]   ` <4f755be3.42d1440a.31fc.3511SMTPIN_ADDED@mx.google.com>
@ 2012-03-30  7:42     ` Dmitry Torokhov
  0 siblings, 0 replies; 6+ messages in thread
From: Dmitry Torokhov @ 2012-03-30  7:42 UTC (permalink / raw)
  To: Tobita Tatsunosuke; +Cc: linux-input

Hi Tats,

On Fri, Mar 30, 2012 at 04:08:12PM +0900, Tobita Tatsunosuke wrote:
> Hello Dmitry
> 
> Today, I compiled the code you had revised and it worked;
> it reported coordination, stylus button on/off, and other buttons assigned.
> I made a patch for this, so can I send this patch?

Did you have to make adjustments to the version of the patch I sent to
you? If it worked as is and you did not have to change anything then I
do not need a new patch, I will apply the one I already have.

Please let me know.

Thanks.

-- 
Dmitry

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

end of thread, other threads:[~2012-03-30  7:42 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-03-23  5:40 [PATCH] Input: Wacom Stylus driver for I2C interface Tatsunosuke Tobita
2012-03-23 18:02 ` Shubhrajyoti
2012-03-26  2:26   ` Tatsunosuke Tobita
2012-03-25 23:50 ` Dmitry Torokhov
2012-03-26  3:05   ` Tatsunosuke Tobita
     [not found]   ` <4f755be3.42d1440a.31fc.3511SMTPIN_ADDED@mx.google.com>
2012-03-30  7:42     ` Dmitry Torokhov

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