[PATCH] input:Adding support for Wacom Stylus with I2c interface

[PATCH] input:Adding support for Wacom Stylus with I2c interface

[Tatsunosuke Tobita]

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

This driver has the same pen input process logic as wacom_w8001.c does.
Also some modifications for structures were applied.

Signed-off-by: Tatsunosuke Tobita <tobita.tatsunosuke@wacom.co.jp>
---
 drivers/input/touchscreen/wacom_i2c.c      |  216 ++++++++++++++++++++++++++++
 drivers/input/touchscreen/wacom_i2c.h      |   66 +++++++++
 drivers/input/touchscreen/wacom_i2c_func.c |   96 ++++++++++++
 drivers/input/touchscreen/wacom_i2c_func.h |    8 +
 4 files changed, 386 insertions(+), 0 deletions(-)
 create mode 100644 drivers/input/touchscreen/wacom_i2c.c
 create mode 100644 drivers/input/touchscreen/wacom_i2c.h
 create mode 100644 drivers/input/touchscreen/wacom_i2c_func.c
 create mode 100644 drivers/input/touchscreen/wacom_i2c_func.h

diff --git a/drivers/input/touchscreen/wacom_i2c.c b/drivers/input/touchscreen/wacom_i2c.c
new file mode 100644
index 0000000..88a232b
--- /dev/null
+++ b/drivers/input/touchscreen/wacom_i2c.c
@@ -0,0 +1,216 @@
+/*
+ * 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.
+ *
+ * Note: Wacom Penabled Driver for I2C obtains data by polling.
+ * If you use irq, you must use the edge based rising
+ * interruption with low level pin.
+ */
+
+#include <linux/device.h>
+#include "wacom_i2c_func.h"
+
+const unsigned short addr[2];
+
+static void wacom_i2c_workqueue(struct work_struct *workq)
+{
+	struct wacom_i2c *wac_i2c = container_of(workq, struct wacom_i2c, workq);
+	wacom_set_coordination(wac_i2c);
+}
+
+static enum hrtimer_restart wacom_i2c_timer(struct hrtimer *timer)
+{
+	struct wacom_i2c *wac_i2c = container_of(timer, struct wacom_i2c, timer);
+
+	queue_work(wacom_i2c_wq, &wac_i2c->workq);
+	hrtimer_start(&wac_i2c->timer, ktime_set(0, POLL_RATE), HRTIMER_MODE_REL);
+
+	return HRTIMER_NORESTART;
+}
+
+static int wacom_i2c_input_open(struct input_dev *dev)
+{
+	struct wacom_i2c *wac_i2c = input_get_drvdata(dev);
+
+	mutex_lock(&wac_i2c->lock);
+	wac_i2c->dev->open = true;
+	mutex_unlock(&wac_i2c->lock);
+
+	return 0;
+}
+
+static void wacom_i2c_input_close(struct input_dev *dev)
+{
+	struct wacom_i2c *wac_i2c = input_get_drvdata(dev);
+
+	mutex_lock(&wac_i2c->lock);
+	wac_i2c->dev->open = false;
+	mutex_unlock(&wac_i2c->lock);
+}
+
+static void wacom_i2c_set_input_values(struct i2c_client *client,
+				       struct wacom_i2c *wac_i2c, struct input_dev *dev)
+{
+	dev->name = "WACOM_I2C_DIGITIZER";
+	dev->id.bustype = BUS_I2C;
+	dev->id.vendor = 0x56a;
+	dev->dev.parent = &client->dev;
+	dev->open = wacom_i2c_input_open;
+	dev->close = wacom_i2c_input_close;
+	dev->evbit[0] |= BIT_MASK(EV_KEY) | BIT_MASK(EV_ABS);
+
+	__set_bit(ABS_X, dev->absbit);
+	__set_bit(ABS_Y, dev->absbit);
+	__set_bit(ABS_PRESSURE, dev->absbit);
+	__set_bit(BTN_TOOL_PEN, dev->keybit);
+	__set_bit(BTN_TOOL_RUBBER, dev->keybit);
+	__set_bit(BTN_STYLUS, dev->keybit);
+	__set_bit(BTN_STYLUS2, dev->keybit);
+	__set_bit(BTN_TOUCH, dev->keybit);
+}
+
+static int wacom_i2c_probe(struct i2c_client *client,
+			   const struct i2c_device_id *id)
+{
+	struct wacom_i2c *wac_i2c;
+	int i, ret;
+	i = ret = 0;
+
+	if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C))
+		goto err2;
+
+	wac_i2c = kzalloc(sizeof(struct wacom_i2c), GFP_KERNEL);
+	wac_i2c->wac_feature = wacom_feature_EMR;
+
+	mutex_init(&wac_i2c->lock);
+
+	wac_i2c->dev = input_allocate_device();
+	if (wac_i2c == NULL || wac_i2c->dev == NULL)
+		goto fail;
+	wacom_i2c_set_input_values(client, wac_i2c, wac_i2c->dev);
+
+	wac_i2c->client = client;
+
+	INIT_WORK(&wac_i2c->workq, wacom_i2c_workqueue);
+
+	ret = wacom_send_query(wac_i2c);
+	if (ret < 0)
+		goto err1;
+
+	wac_i2c->dev->id.version = wac_i2c->wac_feature.fw_version;
+	input_set_abs_params(wac_i2c->dev, ABS_X, 0,
+			     wac_i2c->wac_feature.x_max, 0, 0);
+	input_set_abs_params(wac_i2c->dev, ABS_Y, 0,
+			     wac_i2c->wac_feature.y_max, 0, 0);
+	input_set_abs_params(wac_i2c->dev, ABS_PRESSURE, 0,
+			     wac_i2c->wac_feature.pressure_max, 0, 0);
+	input_set_drvdata(wac_i2c->dev, wac_i2c);
+
+	i2c_set_clientdata(client, wac_i2c);
+	if (input_register_device(wac_i2c->dev))
+		goto err1;
+
+	hrtimer_init(&wac_i2c->timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
+	wac_i2c->timer.function = wacom_i2c_timer;
+	hrtimer_start(&wac_i2c->timer, ktime_set(1, 0), HRTIMER_MODE_REL);
+	printk(KERN_INFO "WACOM_I2C_DIGITIZER: initialized and started \n");
+
+	return 0;
+
+ err2:
+	printk(KERN_ERR "wacom_i2c:No I2C functionality found\n");
+	return -ENODEV;
+
+ err1:
+	printk(KERN_ERR "wacom_i2c:err1 occured(num:%d)\n", ret);
+	input_free_device(wac_i2c->dev);
+	wac_i2c->dev = NULL;
+	return -EIO;
+
+ fail:
+	printk(KERN_ERR "wacom_i2c:fail occured\n");
+	return -ENOMEM;
+}
+
+static int wacom_i2c_remove(struct i2c_client *client)
+{
+	struct wacom_i2c *wac_i2c = i2c_get_clientdata(client);
+
+	input_unregister_device(wac_i2c->dev);
+	hrtimer_cancel(&wac_i2c->timer);
+	kfree(wac_i2c);
+
+	return 0;
+}
+
+static int wacom_i2c_suspend(struct i2c_client *client, pm_message_t mesg)
+{
+	int ret;
+	struct wacom_i2c *wac_i2c = i2c_get_clientdata(client);
+
+	hrtimer_cancel(&wac_i2c->timer);
+	cancel_work_sync(&wac_i2c->workq);
+	ret = wac_i2c->power(0);
+
+	return 0;
+}
+
+static int wacom_i2c_resume(struct i2c_client *client)
+{
+	struct wacom_i2c *wac_i2c = i2c_get_clientdata(client);
+
+	hrtimer_start(&wac_i2c->timer, ktime_set(1, 0), HRTIMER_MODE_REL);
+
+	return 0;
+}
+
+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",
+	},
+
+	.probe = wacom_i2c_probe,
+	.remove = wacom_i2c_remove,
+	.suspend = wacom_i2c_suspend,
+	.resume = wacom_i2c_resume,
+	.id_table = wacom_i2c_id,
+};
+
+static int __init wacom_i2c_init()
+{
+
+	wacom_i2c_wq = create_singlethread_workqueue("wacom_i2c_wq");
+	if (!wacom_i2c_wq)
+		return -ENOMEM;
+
+	return i2c_add_driver(&wacom_i2c_driver);
+}
+
+static void __exit wacom_i2c_exit()
+{
+	if (wacom_i2c_wq)
+		destroy_workqueue(wacom_i2c_wq);
+
+	i2c_del_driver(&wacom_i2c_driver);
+}
+
+module_init(wacom_i2c_init);
+module_exit(wacom_i2c_exit);
+
+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..390b765
--- /dev/null
+++ b/drivers/input/touchscreen/wacom_i2c.h
@@ -0,0 +1,66 @@
+#ifndef WACOM_I2C_H
+#define WAOCM_I2C_H
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/input.h>
+#include <linux/i2c.h>
+#include <linux/slab.h>
+#include <linux/hrtimer.h>
+#include <linux/delay.h>
+#include <linux/uaccess.h>
+#include <asm/unaligned.h>
+
+#define NAMEBUF 12
+#define WACNAME "WAC_I2C_EMR"
+
+/*Wacom Command*/
+#define COM_COORD_NUM      256
+#define COM_SAMPLERATE_40  0x33
+#define COM_SAMPLERATE_80  0x32
+#define COM_SAMPLERATE_133 0x31
+#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
+
+/*I2C address for digitizer*/
+#define WACOM_I2C_ADDR   0x09
+
+/*Polling Rate*/
+#define POLL_RATE 7500000
+
+static struct workqueue_struct *wacom_i2c_wq;
+
+/*Parameters for wacom own features*/
+struct wacom_features {
+	int x_max;
+	int y_max;
+	int pressure_max;
+	char fw_version;
+};
+
+static struct wacom_features wacom_feature_EMR = {
+	16128,
+	8448,
+	256,
+	0x10,
+};
+
+/*Parameters for i2c driver*/
+struct wacom_i2c {
+	struct i2c_client *client;
+	struct input_dev *dev;
+	struct mutex lock;
+	struct work_struct workq;
+	struct hrtimer timer;
+	struct wacom_features wac_feature;
+	int (*power)(int on);
+	int type;
+	const char name[NAMEBUF];
+};
+#endif
diff --git a/drivers/input/touchscreen/wacom_i2c_func.c b/drivers/input/touchscreen/wacom_i2c_func.c
new file mode 100644
index 0000000..97f086c
--- /dev/null
+++ b/drivers/input/touchscreen/wacom_i2c_func.c
@@ -0,0 +1,96 @@
+#include "wacom_i2c_func.h"
+
+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) {
+		printk(KERN_ERR "wacom_i2c: Sending 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) {
+		printk(KERN_ERR "wacom_i2c: Sending Query failed \n");
+		return -1;
+	}
+
+	ret = i2c_master_recv(wac_i2c->client, data, QUERY_SIZE);
+	if (ret < 0) {
+		printk(KERN_ERR "wacom_i2c: Receiving Query failed\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]);
+
+	printk(KERN_INFO "wacom_i2c: 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;
+}
+
+int wacom_set_coordination(struct wacom_i2c *wac_i2c)
+{
+	int ret = 0;
+	unsigned int x, y, pressure;
+	unsigned char rdy, tsw, f1, f2, ers;
+	u8 data[COM_COORD_NUM];
+
+	ret = i2c_master_recv(wac_i2c->client, data, QUERY_SIZE);
+	if (ret >= 0) {
+		tsw = data[3]&0x01;
+		ers = data[3]&0x04;
+		f1 = data[3]&0x02;
+		f2 = data[3]&0x10;
+		rdy = data[3]&0x20;
+		x = le16_to_cpup((__le16 *)&data[4]);
+		y = le16_to_cpup((__le16 *)&data[6]);
+		pressure = le16_to_cpup((__le16 *)&data[8]);
+
+		switch (wac_i2c->type) {
+		case BTN_TOOL_RUBBER:
+			if (!f2) {
+				input_report_abs(wac_i2c->dev, ABS_PRESSURE, 0);
+				input_report_key(wac_i2c->dev, BTN_TOUCH, 0);
+				input_report_key(wac_i2c->dev, BTN_STYLUS, 0);
+				input_report_key(wac_i2c->dev, BTN_STYLUS2, 0);
+				input_report_key(wac_i2c->dev, BTN_TOOL_RUBBER, 0);
+				input_sync(wac_i2c->dev);
+				wac_i2c->type = BTN_TOOL_PEN;
+			}
+			break;
+
+		case KEY_RESERVED:
+			wac_i2c->type = f2 ? BTN_TOOL_RUBBER : BTN_TOOL_PEN;
+			break;
+
+		default:
+			input_report_key(wac_i2c->dev, BTN_STYLUS2, f2);
+			break;
+		}
+
+		input_report_abs(wac_i2c->dev, ABS_X, x);
+		input_report_abs(wac_i2c->dev, ABS_Y, y);
+		input_report_abs(wac_i2c->dev, ABS_PRESSURE, pressure);
+		input_report_key(wac_i2c->dev, BTN_TOUCH, (tsw || ers));
+		input_report_key(wac_i2c->dev, BTN_STYLUS, f1);
+
+		input_sync(wac_i2c->dev);
+
+		if (!rdy)
+			wac_i2c->type = KEY_RESERVED;
+
+	} else {
+		return -1;
+	}
+
+	return 0;
+}
diff --git a/drivers/input/touchscreen/wacom_i2c_func.h b/drivers/input/touchscreen/wacom_i2c_func.h
new file mode 100644
index 0000000..cce35c4
--- /dev/null
+++ b/drivers/input/touchscreen/wacom_i2c_func.h
@@ -0,0 +1,8 @@
+#ifndef WACOM_I2C_FUNC_H
+#define WACOM_I2C_FUNC_H
+
+#include "wacom_i2c.h"
+
+int wacom_set_coordination(struct wacom_i2c *wac_i2c);
+int wacom_send_query(struct wacom_i2c *wac_i2c);
+#endif
-- 
1.7.1

Re: [PATCH] input:Adding support for Wacom Stylus with I2c interface

[Oliver Neukum]

Am Dienstag, 4. Oktober 2011, 09:44:23 schrieb Tatsunosuke Tobita:

Hi,

> +static int wacom_i2c_probe(struct i2c_client *client,
> +                          const struct i2c_device_id *id)
> +{
> +       struct wacom_i2c *wac_i2c;
> +       int i, ret;
> +       i = ret = 0;
> +
> +       if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C))
> +               goto err2;
> +
> +       wac_i2c = kzalloc(sizeof(struct wacom_i2c), GFP_KERNEL);
> +       wac_i2c->wac_feature = wacom_feature_EMR;

potentially following the NULL pointer

> +
> +       mutex_init(&wac_i2c->lock);
> +
> +       wac_i2c->dev = input_allocate_device();
> +       if (wac_i2c == NULL || wac_i2c->dev == NULL)
> +               goto fail;

You must test before you use

> +       wacom_i2c_set_input_values(client, wac_i2c, wac_i2c->dev);
> +
> +       wac_i2c->client = client;
> +
> +       INIT_WORK(&wac_i2c->workq, wacom_i2c_workqueue);
> +
> +       ret = wacom_send_query(wac_i2c);
> +       if (ret < 0)
> +               goto err1;
> +
> +       wac_i2c->dev->id.version = wac_i2c->wac_feature.fw_version;
> +       input_set_abs_params(wac_i2c->dev, ABS_X, 0,
> +                            wac_i2c->wac_feature.x_max, 0, 0);
> +       input_set_abs_params(wac_i2c->dev, ABS_Y, 0,
> +                            wac_i2c->wac_feature.y_max, 0, 0);
> +       input_set_abs_params(wac_i2c->dev, ABS_PRESSURE, 0,
> +                            wac_i2c->wac_feature.pressure_max, 0, 0);
> +       input_set_drvdata(wac_i2c->dev, wac_i2c);
> +
> +       i2c_set_clientdata(client, wac_i2c);
> +       if (input_register_device(wac_i2c->dev))
> +               goto err1;
> +
> +       hrtimer_init(&wac_i2c->timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
> +       wac_i2c->timer.function = wacom_i2c_timer;
> +       hrtimer_start(&wac_i2c->timer, ktime_set(1, 0), HRTIMER_MODE_REL);

I think you want to initialize this before you register the input device to prevent
a race condition.

> +       printk(KERN_INFO "WACOM_I2C_DIGITIZER: initialized and started \n");
> +
> +       return 0;
> +
> + err2:
> +       printk(KERN_ERR "wacom_i2c:No I2C functionality found\n");
> +       return -ENODEV;
> +
> + err1:
> +       printk(KERN_ERR "wacom_i2c:err1 occured(num:%d)\n", ret);
> +       input_free_device(wac_i2c->dev);
> +       wac_i2c->dev = NULL;
> +       return -EIO;
> +
> + fail:
> +       printk(KERN_ERR "wacom_i2c:fail occured\n");
> +       return -ENOMEM;
> +}
> +

	Regards
		Oliver

Re: [PATCH] input:Adding support for Wacom Stylus with I2c interface

[Chris Bagwell]

On Tue, Oct 4, 2011 at 2:50 AM, Tobita Tatsunosuke
<tobita.tatsunosuke@wacom.co.jp> wrote:
> all
>
> I modified the original code as Chris suggested.
>
> below codes were modified:
>> +               input_report_abs(wac_i2c->dev, ABS_X, x);
>> +               input_report_abs(wac_i2c->dev, ABS_Y, y);
>> +               input_report_abs(wac_i2c->dev, ABS_PRESSURE,
>> + pressure);
>> +               input_report_key(wac_i2c->dev, BTN_TOOL_PEN, tsw);
>> +               input_report_key(wac_i2c->dev, BTN_STYLUS, f1);
>> +               input_report_key(wac_i2c->dev, BTN_STYLUS2, f2);
>> +               input_report_key(wac_i2c->dev, BTN_TOOL_RUBBER, ers);
>> +               input_report_key(wac_i2c->dev, BTN_TOUCH, (tsw ||
>> + ers));
>> +
>> +               input_sync(wac_i2c->dev);
>

Hi again, Tats.  Looking at your code again, I see now that the f2
status and ers status are not shared by single bit like wacom_8001.

I am sorry to have led you towards that code.  I think probably your
original code was closer to what you need.  But it depends on bit
means.  Can this HW tell difference between tool in proximity and tool
touching for both pen and eraser?

My guess based on wacom_8001 behavior is bits mean this:

rdy&&!tsw&&!ers = pen in proximity but not touching
rdy&&tsw&&!ers = pen in proximity and touching
rdy&&!tsw&&ers = eraser in proximity but not touching
rdy&&tsw&&ers = eraser in proximity and touching

If thats true then your original code is better with following tool changes:

               input_report_key(wac_i2c->dev, BTN_TOOL_PEN, rdy && !ers);
               input_report_key(wac_i2c->dev, BTN_TOOL_RUBBER, rdy && ers);

> and the applied codes after the modification are:
>
> +               switch (wac_i2c->type) {
> +               case BTN_TOOL_RUBBER:
> +                       if (!f2) {
> +                               input_report_abs(wac_i2c->dev, ABS_PRESSURE,
> 0);
> +                               input_report_key(wac_i2c->dev, BTN_TOUCH,
> 0);
> +                               input_report_key(wac_i2c->dev, BTN_STYLUS,
> 0);
> +                               input_report_key(wac_i2c->dev, BTN_STYLUS2,
> 0);
> +                               input_report_key(wac_i2c->dev,
> BTN_TOOL_RUBBER, 0);
> +                               input_sync(wac_i2c->dev);
> +                               wac_i2c->type = BTN_TOOL_PEN;
> +                       }
> +                       break;
> +
> +               case KEY_RESERVED:
> +                       wac_i2c->type = f2 ? BTN_TOOL_RUBBER : BTN_TOOL_PEN;
> +                       break;
> +
> +               default:
> +                       input_report_key(wac_i2c->dev, BTN_STYLUS2, f2);
> +                       break;
> +               }
> +
> +               input_report_abs(wac_i2c->dev, ABS_X, x);
> +               input_report_abs(wac_i2c->dev, ABS_Y, y);
> +               input_report_abs(wac_i2c->dev, ABS_PRESSURE, pressure);
> +               input_report_key(wac_i2c->dev, BTN_TOUCH, (tsw || ers));
> +               input_report_key(wac_i2c->dev, BTN_STYLUS, f1);

Its hard to comment on this without knowing bit meanings but at
minimum you need these two items.  The above code though is based
around f2 and ers being same bit so probably needs to be cleaned up.

input_report_key(wac_i2c->dev, BTN_STYLUS2, f2);
input_report_key(wac_i2c->dev, wac_i2c->type, rdy);

Chris

> +
> +               input_sync(wac_i2c->dev);
> +
> +               if (!rdy)
> +                       wac_i2c->type = KEY_RESERVED;
>
> If you guys have another suggestion, please let me know.
>
> Thanks
>
> Tats
>
> -----Original Message-----
> From: linux-input-owner@vger.kernel.org
> [mailto:linux-input-owner@vger.kernel.org] On Behalf Of Tatsunosuke Tobita
> Sent: Tuesday, October 04, 2011 4:44 PM
> To: linux-input@vger.kernel.org
> Cc: Tatsunosuke Tobita
> Subject: [PATCH] input:Adding support for Wacom Stylus with I2c interface
>
> From: Tatsunosuke Tobita <tobita.tatsunosuke@wacom.co.jp>
>
> This driver has the same pen input process logic as wacom_w8001.c does.
> Also some modifications for structures were applied.
>
> Signed-off-by: Tatsunosuke Tobita <tobita.tatsunosuke@wacom.co.jp>
> ---
>  drivers/input/touchscreen/wacom_i2c.c      |  216
> ++++++++++++++++++++++++++++
>  drivers/input/touchscreen/wacom_i2c.h      |   66 +++++++++
>  drivers/input/touchscreen/wacom_i2c_func.c |   96 ++++++++++++
>  drivers/input/touchscreen/wacom_i2c_func.h |    8 +
>  4 files changed, 386 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/input/touchscreen/wacom_i2c.c
>  create mode 100644 drivers/input/touchscreen/wacom_i2c.h
>  create mode 100644 drivers/input/touchscreen/wacom_i2c_func.c
>  create mode 100644 drivers/input/touchscreen/wacom_i2c_func.h
>
> diff --git a/drivers/input/touchscreen/wacom_i2c.c
> b/drivers/input/touchscreen/wacom_i2c.c
> new file mode 100644
> index 0000000..88a232b
> --- /dev/null
> +++ b/drivers/input/touchscreen/wacom_i2c.c
> @@ -0,0 +1,216 @@
> +/*
> + * 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.
> + *
> + * Note: Wacom Penabled Driver for I2C obtains data by polling.
> + * If you use irq, you must use the edge based rising
> + * interruption with low level pin.
> + */
> +
> +#include <linux/device.h>
> +#include "wacom_i2c_func.h"
> +
> +const unsigned short addr[2];
> +
> +static void wacom_i2c_workqueue(struct work_struct *workq)
> +{
> +       struct wacom_i2c *wac_i2c = container_of(workq, struct wacom_i2c,
> workq);
> +       wacom_set_coordination(wac_i2c);
> +}
> +
> +static enum hrtimer_restart wacom_i2c_timer(struct hrtimer *timer)
> +{
> +       struct wacom_i2c *wac_i2c = container_of(timer, struct wacom_i2c,
> timer);
> +
> +       queue_work(wacom_i2c_wq, &wac_i2c->workq);
> +       hrtimer_start(&wac_i2c->timer, ktime_set(0, POLL_RATE),
> HRTIMER_MODE_REL);
> +
> +       return HRTIMER_NORESTART;
> +}
> +
> +static int wacom_i2c_input_open(struct input_dev *dev)
> +{
> +       struct wacom_i2c *wac_i2c = input_get_drvdata(dev);
> +
> +       mutex_lock(&wac_i2c->lock);
> +       wac_i2c->dev->open = true;
> +       mutex_unlock(&wac_i2c->lock);
> +
> +       return 0;
> +}
> +
> +static void wacom_i2c_input_close(struct input_dev *dev)
> +{
> +       struct wacom_i2c *wac_i2c = input_get_drvdata(dev);
> +
> +       mutex_lock(&wac_i2c->lock);
> +       wac_i2c->dev->open = false;
> +       mutex_unlock(&wac_i2c->lock);
> +}
> +
> +static void wacom_i2c_set_input_values(struct i2c_client *client,
> +                                      struct wacom_i2c *wac_i2c, struct
> input_dev *dev)
> +{
> +       dev->name = "WACOM_I2C_DIGITIZER";
> +       dev->id.bustype = BUS_I2C;
> +       dev->id.vendor = 0x56a;
> +       dev->dev.parent = &client->dev;
> +       dev->open = wacom_i2c_input_open;
> +       dev->close = wacom_i2c_input_close;
> +       dev->evbit[0] |= BIT_MASK(EV_KEY) | BIT_MASK(EV_ABS);
> +
> +       __set_bit(ABS_X, dev->absbit);
> +       __set_bit(ABS_Y, dev->absbit);
> +       __set_bit(ABS_PRESSURE, dev->absbit);
> +       __set_bit(BTN_TOOL_PEN, dev->keybit);
> +       __set_bit(BTN_TOOL_RUBBER, dev->keybit);
> +       __set_bit(BTN_STYLUS, dev->keybit);
> +       __set_bit(BTN_STYLUS2, dev->keybit);
> +       __set_bit(BTN_TOUCH, dev->keybit);
> +}
> +
> +static int wacom_i2c_probe(struct i2c_client *client,
> +                          const struct i2c_device_id *id)
> +{
> +       struct wacom_i2c *wac_i2c;
> +       int i, ret;
> +       i = ret = 0;
> +
> +       if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C))
> +               goto err2;
> +
> +       wac_i2c = kzalloc(sizeof(struct wacom_i2c), GFP_KERNEL);
> +       wac_i2c->wac_feature = wacom_feature_EMR;
> +
> +       mutex_init(&wac_i2c->lock);
> +
> +       wac_i2c->dev = input_allocate_device();
> +       if (wac_i2c == NULL || wac_i2c->dev == NULL)
> +               goto fail;
> +       wacom_i2c_set_input_values(client, wac_i2c, wac_i2c->dev);
> +
> +       wac_i2c->client = client;
> +
> +       INIT_WORK(&wac_i2c->workq, wacom_i2c_workqueue);
> +
> +       ret = wacom_send_query(wac_i2c);
> +       if (ret < 0)
> +               goto err1;
> +
> +       wac_i2c->dev->id.version = wac_i2c->wac_feature.fw_version;
> +       input_set_abs_params(wac_i2c->dev, ABS_X, 0,
> +                            wac_i2c->wac_feature.x_max, 0, 0);
> +       input_set_abs_params(wac_i2c->dev, ABS_Y, 0,
> +                            wac_i2c->wac_feature.y_max, 0, 0);
> +       input_set_abs_params(wac_i2c->dev, ABS_PRESSURE, 0,
> +                            wac_i2c->wac_feature.pressure_max, 0, 0);
> +       input_set_drvdata(wac_i2c->dev, wac_i2c);
> +
> +       i2c_set_clientdata(client, wac_i2c);
> +       if (input_register_device(wac_i2c->dev))
> +               goto err1;
> +
> +       hrtimer_init(&wac_i2c->timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
> +       wac_i2c->timer.function = wacom_i2c_timer;
> +       hrtimer_start(&wac_i2c->timer, ktime_set(1, 0), HRTIMER_MODE_REL);
> +       printk(KERN_INFO "WACOM_I2C_DIGITIZER: initialized and started \n");
> +
> +       return 0;
> +
> + err2:
> +       printk(KERN_ERR "wacom_i2c:No I2C functionality found\n");
> +       return -ENODEV;
> +
> + err1:
> +       printk(KERN_ERR "wacom_i2c:err1 occured(num:%d)\n", ret);
> +       input_free_device(wac_i2c->dev);
> +       wac_i2c->dev = NULL;
> +       return -EIO;
> +
> + fail:
> +       printk(KERN_ERR "wacom_i2c:fail occured\n");
> +       return -ENOMEM;
> +}
> +
> +static int wacom_i2c_remove(struct i2c_client *client)
> +{
> +       struct wacom_i2c *wac_i2c = i2c_get_clientdata(client);
> +
> +       input_unregister_device(wac_i2c->dev);
> +       hrtimer_cancel(&wac_i2c->timer);
> +       kfree(wac_i2c);
> +
> +       return 0;
> +}
> +
> +static int wacom_i2c_suspend(struct i2c_client *client, pm_message_t mesg)
> +{
> +       int ret;
> +       struct wacom_i2c *wac_i2c = i2c_get_clientdata(client);
> +
> +       hrtimer_cancel(&wac_i2c->timer);
> +       cancel_work_sync(&wac_i2c->workq);
> +       ret = wac_i2c->power(0);
> +
> +       return 0;
> +}
> +
> +static int wacom_i2c_resume(struct i2c_client *client)
> +{
> +       struct wacom_i2c *wac_i2c = i2c_get_clientdata(client);
> +
> +       hrtimer_start(&wac_i2c->timer, ktime_set(1, 0), HRTIMER_MODE_REL);
> +
> +       return 0;
> +}
> +
> +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",
> +       },
> +
> +       .probe = wacom_i2c_probe,
> +       .remove = wacom_i2c_remove,
> +       .suspend = wacom_i2c_suspend,
> +       .resume = wacom_i2c_resume,
> +       .id_table = wacom_i2c_id,
> +};
> +
> +static int __init wacom_i2c_init()
> +{
> +
> +       wacom_i2c_wq = create_singlethread_workqueue("wacom_i2c_wq");
> +       if (!wacom_i2c_wq)
> +               return -ENOMEM;
> +
> +       return i2c_add_driver(&wacom_i2c_driver);
> +}
> +
> +static void __exit wacom_i2c_exit()
> +{
> +       if (wacom_i2c_wq)
> +               destroy_workqueue(wacom_i2c_wq);
> +
> +       i2c_del_driver(&wacom_i2c_driver);
> +}
> +
> +module_init(wacom_i2c_init);
> +module_exit(wacom_i2c_exit);
> +
> +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..390b765
> --- /dev/null
> +++ b/drivers/input/touchscreen/wacom_i2c.h
> @@ -0,0 +1,66 @@
> +#ifndef WACOM_I2C_H
> +#define WAOCM_I2C_H
> +
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/kernel.h>
> +#include <linux/input.h>
> +#include <linux/i2c.h>
> +#include <linux/slab.h>
> +#include <linux/hrtimer.h>
> +#include <linux/delay.h>
> +#include <linux/uaccess.h>
> +#include <asm/unaligned.h>
> +
> +#define NAMEBUF 12
> +#define WACNAME "WAC_I2C_EMR"
> +
> +/*Wacom Command*/
> +#define COM_COORD_NUM      256
> +#define COM_SAMPLERATE_40  0x33
> +#define COM_SAMPLERATE_80  0x32
> +#define COM_SAMPLERATE_133 0x31
> +#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
> +
> +/*I2C address for digitizer*/
> +#define WACOM_I2C_ADDR   0x09
> +
> +/*Polling Rate*/
> +#define POLL_RATE 7500000
> +
> +static struct workqueue_struct *wacom_i2c_wq;
> +
> +/*Parameters for wacom own features*/
> +struct wacom_features {
> +       int x_max;
> +       int y_max;
> +       int pressure_max;
> +       char fw_version;
> +};
> +
> +static struct wacom_features wacom_feature_EMR = {
> +       16128,
> +       8448,
> +       256,
> +       0x10,
> +};
> +
> +/*Parameters for i2c driver*/
> +struct wacom_i2c {
> +       struct i2c_client *client;
> +       struct input_dev *dev;
> +       struct mutex lock;
> +       struct work_struct workq;
> +       struct hrtimer timer;
> +       struct wacom_features wac_feature;
> +       int (*power)(int on);
> +       int type;
> +       const char name[NAMEBUF];
> +};
> +#endif
> diff --git a/drivers/input/touchscreen/wacom_i2c_func.c
> b/drivers/input/touchscreen/wacom_i2c_func.c
> new file mode 100644
> index 0000000..97f086c
> --- /dev/null
> +++ b/drivers/input/touchscreen/wacom_i2c_func.c
> @@ -0,0 +1,96 @@
> +#include "wacom_i2c_func.h"
> +
> +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) {
> +               printk(KERN_ERR "wacom_i2c: Sending 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) {
> +               printk(KERN_ERR "wacom_i2c: Sending Query failed \n");
> +               return -1;
> +       }
> +
> +       ret = i2c_master_recv(wac_i2c->client, data, QUERY_SIZE);
> +       if (ret < 0) {
> +               printk(KERN_ERR "wacom_i2c: Receiving Query failed\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]);
> +
> +       printk(KERN_INFO "wacom_i2c: 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;
> +}
> +
> +int wacom_set_coordination(struct wacom_i2c *wac_i2c)
> +{
> +       int ret = 0;
> +       unsigned int x, y, pressure;
> +       unsigned char rdy, tsw, f1, f2, ers;
> +       u8 data[COM_COORD_NUM];
> +
> +       ret = i2c_master_recv(wac_i2c->client, data, QUERY_SIZE);
> +       if (ret >= 0) {
> +               tsw = data[3]&0x01;
> +               ers = data[3]&0x04;
> +               f1 = data[3]&0x02;
> +               f2 = data[3]&0x10;
> +               rdy = data[3]&0x20;
> +               x = le16_to_cpup((__le16 *)&data[4]);
> +               y = le16_to_cpup((__le16 *)&data[6]);
> +               pressure = le16_to_cpup((__le16 *)&data[8]);
> +
> +               switch (wac_i2c->type) {
> +               case BTN_TOOL_RUBBER:
> +                       if (!f2) {
> +                               input_report_abs(wac_i2c->dev, ABS_PRESSURE,
> 0);
> +                               input_report_key(wac_i2c->dev, BTN_TOUCH,
> 0);
> +                               input_report_key(wac_i2c->dev, BTN_STYLUS,
> 0);
> +                               input_report_key(wac_i2c->dev, BTN_STYLUS2,
> 0);
> +                               input_report_key(wac_i2c->dev,
> BTN_TOOL_RUBBER, 0);
> +                               input_sync(wac_i2c->dev);
> +                               wac_i2c->type = BTN_TOOL_PEN;
> +                       }
> +                       break;
> +
> +               case KEY_RESERVED:
> +                       wac_i2c->type = f2 ? BTN_TOOL_RUBBER : BTN_TOOL_PEN;
> +                       break;
> +
> +               default:
> +                       input_report_key(wac_i2c->dev, BTN_STYLUS2, f2);
> +                       break;
> +               }
> +
> +               input_report_abs(wac_i2c->dev, ABS_X, x);
> +               input_report_abs(wac_i2c->dev, ABS_Y, y);
> +               input_report_abs(wac_i2c->dev, ABS_PRESSURE, pressure);
> +               input_report_key(wac_i2c->dev, BTN_TOUCH, (tsw || ers));
> +               input_report_key(wac_i2c->dev, BTN_STYLUS, f1);
> +
> +               input_sync(wac_i2c->dev);
> +
> +               if (!rdy)
> +                       wac_i2c->type = KEY_RESERVED;
> +
> +       } else {
> +               return -1;
> +       }
> +
> +       return 0;
> +}
> diff --git a/drivers/input/touchscreen/wacom_i2c_func.h
> b/drivers/input/touchscreen/wacom_i2c_func.h
> new file mode 100644
> index 0000000..cce35c4
> --- /dev/null
> +++ b/drivers/input/touchscreen/wacom_i2c_func.h
> @@ -0,0 +1,8 @@
> +#ifndef WACOM_I2C_FUNC_H
> +#define WACOM_I2C_FUNC_H
> +
> +#include "wacom_i2c.h"
> +
> +int wacom_set_coordination(struct wacom_i2c *wac_i2c);
> +int wacom_send_query(struct wacom_i2c *wac_i2c);
> +#endif
> --
> 1.7.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-input" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] input:Adding support for Wacom Stylus with I2c interface

[Dmitry Torokhov]

Hi Tatsunosuke,

On Tue, Oct 04, 2011 at 04:44:23PM +0900, Tatsunosuke Tobita wrote:
> From: Tatsunosuke Tobita <tobita.tatsunosuke@wacom.co.jp>
> 
> This driver has the same pen input process logic as wacom_w8001.c does.
> Also some modifications for structures were applied.
> 
> Signed-off-by: Tatsunosuke Tobita <tobita.tatsunosuke@wacom.co.jp>
> ---
>  drivers/input/touchscreen/wacom_i2c.c      |  216 ++++++++++++++++++++++++++++
>  drivers/input/touchscreen/wacom_i2c.h      |   66 +++++++++
>  drivers/input/touchscreen/wacom_i2c_func.c |   96 ++++++++++++
>  drivers/input/touchscreen/wacom_i2c_func.h |    8 +
>  4 files changed, 386 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/input/touchscreen/wacom_i2c.c
>  create mode 100644 drivers/input/touchscreen/wacom_i2c.h
>  create mode 100644 drivers/input/touchscreen/wacom_i2c_func.c
>  create mode 100644 drivers/input/touchscreen/wacom_i2c_func.h
> 
> diff --git a/drivers/input/touchscreen/wacom_i2c.c b/drivers/input/touchscreen/wacom_i2c.c
> new file mode 100644
> index 0000000..88a232b
> --- /dev/null
> +++ b/drivers/input/touchscreen/wacom_i2c.c
> @@ -0,0 +1,216 @@
> +/*
> + * 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.
> + *
> + * Note: Wacom Penabled Driver for I2C obtains data by polling.
> + * If you use irq, you must use the edge based rising
> + * interruption with low level pin.
> + */
> +
> +#include <linux/device.h>
> +#include "wacom_i2c_func.h"
> +
> +const unsigned short addr[2];
> +
> +static void wacom_i2c_workqueue(struct work_struct *workq)
> +{
> +	struct wacom_i2c *wac_i2c = container_of(workq, struct wacom_i2c, workq);
> +	wacom_set_coordination(wac_i2c);
> +}
> +
> +static enum hrtimer_restart wacom_i2c_timer(struct hrtimer *timer)
> +{
> +	struct wacom_i2c *wac_i2c = container_of(timer, struct wacom_i2c, timer);
> +
> +	queue_work(wacom_i2c_wq, &wac_i2c->workq);
> +	hrtimer_start(&wac_i2c->timer, ktime_set(0, POLL_RATE), HRTIMER_MODE_REL);
> +

So we have a timer and a workqueue. Why not use delayed_work?

> +	return HRTIMER_NORESTART;
> +}
> +
> +static int wacom_i2c_input_open(struct input_dev *dev)
> +{
> +	struct wacom_i2c *wac_i2c = input_get_drvdata(dev);
> +
> +	mutex_lock(&wac_i2c->lock);
> +	wac_i2c->dev->open = true;

Did you try actually compiling this?

> +	mutex_unlock(&wac_i2c->lock);
> +
> +	return 0;
> +}
> +
> +static void wacom_i2c_input_close(struct input_dev *dev)
> +{
> +	struct wacom_i2c *wac_i2c = input_get_drvdata(dev);
> +
> +	mutex_lock(&wac_i2c->lock);
> +	wac_i2c->dev->open = false;
> +	mutex_unlock(&wac_i2c->lock);
> +}
> +
> +static void wacom_i2c_set_input_values(struct i2c_client *client,
> +				       struct wacom_i2c *wac_i2c, struct input_dev *dev)
> +{
> +	dev->name = "WACOM_I2C_DIGITIZER";
> +	dev->id.bustype = BUS_I2C;
> +	dev->id.vendor = 0x56a;
> +	dev->dev.parent = &client->dev;
> +	dev->open = wacom_i2c_input_open;
> +	dev->close = wacom_i2c_input_close;
> +	dev->evbit[0] |= BIT_MASK(EV_KEY) | BIT_MASK(EV_ABS);
> +
> +	__set_bit(ABS_X, dev->absbit);
> +	__set_bit(ABS_Y, dev->absbit);
> +	__set_bit(ABS_PRESSURE, dev->absbit);
> +	__set_bit(BTN_TOOL_PEN, dev->keybit);
> +	__set_bit(BTN_TOOL_RUBBER, dev->keybit);
> +	__set_bit(BTN_STYLUS, dev->keybit);
> +	__set_bit(BTN_STYLUS2, dev->keybit);
> +	__set_bit(BTN_TOUCH, dev->keybit);

Just put this into probe().

> +}
> +
> +static int wacom_i2c_probe(struct i2c_client *client,
> +			   const struct i2c_device_id *id)
> +{
> +	struct wacom_i2c *wac_i2c;
> +	int i, ret;
> +	i = ret = 0;
> +
> +	if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C))
> +		goto err2;
> +
> +	wac_i2c = kzalloc(sizeof(struct wacom_i2c), GFP_KERNEL);
> +	wac_i2c->wac_feature = wacom_feature_EMR;

Should we get this from i2c_device_id, platform data, device tree or
something else instead of hard-coding?

> +
> +	mutex_init(&wac_i2c->lock);
> +
> +	wac_i2c->dev = input_allocate_device();
> +	if (wac_i2c == NULL || wac_i2c->dev == NULL)
> +		goto fail;
> +	wacom_i2c_set_input_values(client, wac_i2c, wac_i2c->dev);
> +
> +	wac_i2c->client = client;
> +
> +	INIT_WORK(&wac_i2c->workq, wacom_i2c_workqueue);
> +
> +	ret = wacom_send_query(wac_i2c);
> +	if (ret < 0)
> +		goto err1;
> +
> +	wac_i2c->dev->id.version = wac_i2c->wac_feature.fw_version;
> +	input_set_abs_params(wac_i2c->dev, ABS_X, 0,
> +			     wac_i2c->wac_feature.x_max, 0, 0);
> +	input_set_abs_params(wac_i2c->dev, ABS_Y, 0,
> +			     wac_i2c->wac_feature.y_max, 0, 0);
> +	input_set_abs_params(wac_i2c->dev, ABS_PRESSURE, 0,
> +			     wac_i2c->wac_feature.pressure_max, 0, 0);
> +	input_set_drvdata(wac_i2c->dev, wac_i2c);
> +
> +	i2c_set_clientdata(client, wac_i2c);
> +	if (input_register_device(wac_i2c->dev))
> +		goto err1;

You should report error returned by input_register_device().

> +
> +	hrtimer_init(&wac_i2c->timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
> +	wac_i2c->timer.function = wacom_i2c_timer;
> +	hrtimer_start(&wac_i2c->timer, ktime_set(1, 0), HRTIMER_MODE_REL);

Why start timer if nobody has the device opened yet?

> +	printk(KERN_INFO "WACOM_I2C_DIGITIZER: initialized and started \n");
> +
> +	return 0;
> +
> + err2:
> +	printk(KERN_ERR "wacom_i2c:No I2C functionality found\n");
> +	return -ENODEV;
> +
> + err1:
> +	printk(KERN_ERR "wacom_i2c:err1 occured(num:%d)\n", ret);
> +	input_free_device(wac_i2c->dev);
> +	wac_i2c->dev = NULL;
> +	return -EIO;
> +
> + fail:
> +	printk(KERN_ERR "wacom_i2c:fail occured\n");
> +	return -ENOMEM;
> +}
> +
> +static int wacom_i2c_remove(struct i2c_client *client)
> +{
> +	struct wacom_i2c *wac_i2c = i2c_get_clientdata(client);
> +
> +	input_unregister_device(wac_i2c->dev);

Timer fires here -> *BOOM*. You should cancel the timer in close().

Also, polling-only mode is hardly useful for a touchscreen driver. Is
there a chance you could switch to interrupt-driven mode instead?

> +	hrtimer_cancel(&wac_i2c->timer);
> +	kfree(wac_i2c);
> +
> +	return 0;
> +}
> +
> +static int wacom_i2c_suspend(struct i2c_client *client, pm_message_t mesg)
> +{
> +	int ret;
> +	struct wacom_i2c *wac_i2c = i2c_get_clientdata(client);
> +
> +	hrtimer_cancel(&wac_i2c->timer);
> +	cancel_work_sync(&wac_i2c->workq);
> +	ret = wac_i2c->power(0);
> +
> +	return 0;
> +}
> +
> +static int wacom_i2c_resume(struct i2c_client *client)
> +{
> +	struct wacom_i2c *wac_i2c = i2c_get_clientdata(client);
> +
> +	hrtimer_start(&wac_i2c->timer, ktime_set(1, 0), HRTIMER_MODE_REL);
> +
> +	return 0;
> +}
> +
> +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",
> +	},
> +
> +	.probe = wacom_i2c_probe,
> +	.remove = wacom_i2c_remove,
> +	.suspend = wacom_i2c_suspend,
> +	.resume = wacom_i2c_resume,

Please use dev_pm_ops.

> +	.id_table = wacom_i2c_id,
> +};
> +
> +static int __init wacom_i2c_init()
> +{
> +
> +	wacom_i2c_wq = create_singlethread_workqueue("wacom_i2c_wq");
> +	if (!wacom_i2c_wq)
> +		return -ENOMEM;

There is no reason for dedicated workqueue in mainline.

> +
> +	return i2c_add_driver(&wacom_i2c_driver);
> +}
> +
> +static void __exit wacom_i2c_exit()
> +{
> +	if (wacom_i2c_wq)

The test is useless.

> +		destroy_workqueue(wacom_i2c_wq);
> +
> +	i2c_del_driver(&wacom_i2c_driver);
> +}
> +
> +module_init(wacom_i2c_init);
> +module_exit(wacom_i2c_exit);
> +
> +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..390b765
> --- /dev/null
> +++ b/drivers/input/touchscreen/wacom_i2c.h
> @@ -0,0 +1,66 @@
> +#ifndef WACOM_I2C_H
> +#define WAOCM_I2C_H
> +
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/kernel.h>
> +#include <linux/input.h>
> +#include <linux/i2c.h>
> +#include <linux/slab.h>
> +#include <linux/hrtimer.h>
> +#include <linux/delay.h>
> +#include <linux/uaccess.h>
> +#include <asm/unaligned.h>
> +
> +#define NAMEBUF 12
> +#define WACNAME "WAC_I2C_EMR"
> +
> +/*Wacom Command*/
> +#define COM_COORD_NUM      256
> +#define COM_SAMPLERATE_40  0x33
> +#define COM_SAMPLERATE_80  0x32
> +#define COM_SAMPLERATE_133 0x31
> +#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
> +
> +/*I2C address for digitizer*/
> +#define WACOM_I2C_ADDR   0x09
> +
> +/*Polling Rate*/
> +#define POLL_RATE 7500000
> +
> +static struct workqueue_struct *wacom_i2c_wq;

There shouldn't be static variables in header files.

> +
> +/*Parameters for wacom own features*/
> +struct wacom_features {
> +	int x_max;
> +	int y_max;
> +	int pressure_max;
> +	char fw_version;
> +};
> +
> +static struct wacom_features wacom_feature_EMR = {
> +	16128,
> +	8448,
> +	256,
> +	0x10,
> +};
> +
> +/*Parameters for i2c driver*/
> +struct wacom_i2c {
> +	struct i2c_client *client;
> +	struct input_dev *dev;
> +	struct mutex lock;
> +	struct work_struct workq;
> +	struct hrtimer timer;
> +	struct wacom_features wac_feature;
> +	int (*power)(int on);
> +	int type;
> +	const char name[NAMEBUF];
> +};
> +#endif
> diff --git a/drivers/input/touchscreen/wacom_i2c_func.c b/drivers/input/touchscreen/wacom_i2c_func.c
> new file mode 100644
> index 0000000..97f086c
> --- /dev/null
> +++ b/drivers/input/touchscreen/wacom_i2c_func.c
> @@ -0,0 +1,96 @@
> +#include "wacom_i2c_func.h"
> +

Why is is split? Also, what license does this code have?

> +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) {
> +		printk(KERN_ERR "wacom_i2c: Sending 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) {
> +		printk(KERN_ERR "wacom_i2c: Sending Query failed \n");
> +		return -1;
> +	}
> +
> +	ret = i2c_master_recv(wac_i2c->client, data, QUERY_SIZE);
> +	if (ret < 0) {
> +		printk(KERN_ERR "wacom_i2c: Receiving Query failed\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]);
> +
> +	printk(KERN_INFO "wacom_i2c: 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);
> +

Please use dev_xxx() for reporting.

> +	return 0;
> +}
> +
> +int wacom_set_coordination(struct wacom_i2c *wac_i2c)
> +{
> +	int ret = 0;
> +	unsigned int x, y, pressure;
> +	unsigned char rdy, tsw, f1, f2, ers;
> +	u8 data[COM_COORD_NUM];
> +
> +	ret = i2c_master_recv(wac_i2c->client, data, QUERY_SIZE);
> +	if (ret >= 0) {
> +		tsw = data[3]&0x01;
> +		ers = data[3]&0x04;
> +		f1 = data[3]&0x02;
> +		f2 = data[3]&0x10;
> +		rdy = data[3]&0x20;
> +		x = le16_to_cpup((__le16 *)&data[4]);
> +		y = le16_to_cpup((__le16 *)&data[6]);
> +		pressure = le16_to_cpup((__le16 *)&data[8]);
> +
> +		switch (wac_i2c->type) {
> +		case BTN_TOOL_RUBBER:
> +			if (!f2) {
> +				input_report_abs(wac_i2c->dev, ABS_PRESSURE, 0);
> +				input_report_key(wac_i2c->dev, BTN_TOUCH, 0);
> +				input_report_key(wac_i2c->dev, BTN_STYLUS, 0);
> +				input_report_key(wac_i2c->dev, BTN_STYLUS2, 0);
> +				input_report_key(wac_i2c->dev, BTN_TOOL_RUBBER, 0);
> +				input_sync(wac_i2c->dev);
> +				wac_i2c->type = BTN_TOOL_PEN;
> +			}
> +			break;
> +
> +		case KEY_RESERVED:
> +			wac_i2c->type = f2 ? BTN_TOOL_RUBBER : BTN_TOOL_PEN;
> +			break;
> +
> +		default:
> +			input_report_key(wac_i2c->dev, BTN_STYLUS2, f2);
> +			break;
> +		}
> +
> +		input_report_abs(wac_i2c->dev, ABS_X, x);
> +		input_report_abs(wac_i2c->dev, ABS_Y, y);
> +		input_report_abs(wac_i2c->dev, ABS_PRESSURE, pressure);
> +		input_report_key(wac_i2c->dev, BTN_TOUCH, (tsw || ers));
> +		input_report_key(wac_i2c->dev, BTN_STYLUS, f1);
> +
> +		input_sync(wac_i2c->dev);
> +
> +		if (!rdy)
> +			wac_i2c->type = KEY_RESERVED;
> +
> +	} else {
> +		return -1;
> +	}
> +
> +	return 0;
> +}
> diff --git a/drivers/input/touchscreen/wacom_i2c_func.h b/drivers/input/touchscreen/wacom_i2c_func.h
> new file mode 100644
> index 0000000..cce35c4
> --- /dev/null
> +++ b/drivers/input/touchscreen/wacom_i2c_func.h
> @@ -0,0 +1,8 @@
> +#ifndef WACOM_I2C_FUNC_H
> +#define WACOM_I2C_FUNC_H
> +
> +#include "wacom_i2c.h"
> +
> +int wacom_set_coordination(struct wacom_i2c *wac_i2c);
> +int wacom_send_query(struct wacom_i2c *wac_i2c);
> +#endif

Thanks.

-- 
Dmitry

Re: [PATCH] input:Adding support for Wacom Stylus with I2c interface

[Dmitry Torokhov]

On Fri, Oct 07, 2011 at 08:30:03PM +0900, Tobita Tatsunosuke wrote:
> Hi Dmitry
> 
> -----Original Message-----
> From: Dmitry Torokhov [mailto:dmitry.torokhov@gmail.com] 
> Sent: Wednesday, October 05, 2011 4:11 PM
> To: Tatsunosuke Tobita
> Cc: linux-input@vger.kernel.org; Tatsunosuke Tobita
> Subject: Re: [PATCH] input:Adding support for Wacom Stylus with I2c
> interface
> 
> Hi Tatsunosuke,
> 
> On Tue, Oct 04, 2011 at 04:44:23PM +0900, Tatsunosuke Tobita wrote:
> > From: Tatsunosuke Tobita <tobita.tatsunosuke@wacom.co.jp>
> > 
> > This driver has the same pen input process logic as wacom_w8001.c does.
> > Also some modifications for structures were applied.
> > 
> > Signed-off-by: Tatsunosuke Tobita <tobita.tatsunosuke@wacom.co.jp>
> > ---
> >  drivers/input/touchscreen/wacom_i2c.c      |  216
> ++++++++++++++++++++++++++++
> >  drivers/input/touchscreen/wacom_i2c.h      |   66 +++++++++
> >  drivers/input/touchscreen/wacom_i2c_func.c |   96 ++++++++++++
> >  drivers/input/touchscreen/wacom_i2c_func.h |    8 +
> >  4 files changed, 386 insertions(+), 0 deletions(-)
> >  create mode 100644 drivers/input/touchscreen/wacom_i2c.c
> >  create mode 100644 drivers/input/touchscreen/wacom_i2c.h
> >  create mode 100644 drivers/input/touchscreen/wacom_i2c_func.c
> >  create mode 100644 drivers/input/touchscreen/wacom_i2c_func.h
> > 
> > diff --git a/drivers/input/touchscreen/wacom_i2c.c
> b/drivers/input/touchscreen/wacom_i2c.c
> > new file mode 100644
> > index 0000000..88a232b
> > --- /dev/null
> > +++ b/drivers/input/touchscreen/wacom_i2c.c
> > @@ -0,0 +1,216 @@
> > +/*
> > + * 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.
> > + *
> > + * Note: Wacom Penabled Driver for I2C obtains data by polling.
> > + * If you use irq, you must use the edge based rising
> > + * interruption with low level pin.
> > + */
> > +
> > +#include <linux/device.h>
> > +#include "wacom_i2c_func.h"
> > +
> > +const unsigned short addr[2];
> > +
> > +static void wacom_i2c_workqueue(struct work_struct *workq)
> > +{
> > +	struct wacom_i2c *wac_i2c = container_of(workq, struct wacom_i2c,
> workq);
> > +	wacom_set_coordination(wac_i2c);
> > +}
> > +
> > +static enum hrtimer_restart wacom_i2c_timer(struct hrtimer *timer)
> > +{
> > +	struct wacom_i2c *wac_i2c = container_of(timer, struct wacom_i2c,
> timer);
> > +
> > +	queue_work(wacom_i2c_wq, &wac_i2c->workq);
> > +	hrtimer_start(&wac_i2c->timer, ktime_set(0, POLL_RATE),
> HRTIMER_MODE_REL);
> > +
> 
> >>So we have a timer and a workqueue. Why not use delayed_work?
> When I was starting developing these codes, I looked at several text books
> about writing drivers
> and "workqueue" is mostly recommended to use when running such amount of
> work in kernel context.
> I haven't known "delayed work". But, as you suggested, it definitely looks
> that the "delayed work" is more useful combination
> for both polling and workqueue. I'll work on changing this to delayed_work.
> 
> > +	return HRTIMER_NORESTART;
> > +}
> > +
> > +static int wacom_i2c_input_open(struct input_dev *dev)
> > +{
> > +	struct wacom_i2c *wac_i2c = input_get_drvdata(dev);
> > +
> > +	mutex_lock(&wac_i2c->lock);
> > +	wac_i2c->dev->open = true;
> 
> >>Did you try actually compiling this?
> Yes, I did, I compiled it in bunch of times and checked it working.

Here is what I get when trying to compile the driver:

[dtor@hammer work]$ make drivers/input/touchscreen/wacom_i2c.o 
  CHK     include/linux/version.h
  CHK     include/generated/utsrelease.h
  CALL    scripts/checksyscalls.sh
  CC      drivers/input/touchscreen/wacom_i2c.o
drivers/input/touchscreen/wacom_i2c.c: In function
‘wacom_i2c_input_open’:
drivers/input/touchscreen/wacom_i2c.c:44:21: warning: assignment makes
pointer from integer without a cast [enabled by default]
drivers/input/touchscreen/wacom_i2c.c: At top level:
drivers/input/touchscreen/wacom_i2c.c:193:122: warning: function
declaration isn’t a prototype [-Wstrict-prototypes]
drivers/input/touchscreen/wacom_i2c.c:203:149: warning: function
declaration isn’t a prototype [-Wstrict-prototypes]

The reason for the first warning is because you do 

	wac_i2c->dev->open = true;

but "open" here is not a flag, it is a method that is called when first
user opens the input device. In fact, it is the method that is currently
running, wacom_i2c_input_open().

So when you close the device and open it again, you will get an oops.

> If there's some suspect-able statement which may affect the system seriously
> and
> output the errors when compiling, please let me know.
> I'll try to get rid of that as much as possible.

You can not only act on errors and ignore warnings. There should not be
a single warning produced when compiling a kernel driver It is also
advisable to pass the patch through scripst/checkpath.pl and also check
it with sparse (just install it and then do 'make C=2
drivers/input/touchscreen/synaptics_i2c.o').

> 
> > +	mutex_unlock(&wac_i2c->lock);
> > +
> > +	return 0;
> > +}
> > +
> > +static void wacom_i2c_input_close(struct input_dev *dev)
> > +{
> > +	struct wacom_i2c *wac_i2c = input_get_drvdata(dev);
> > +
> > +	mutex_lock(&wac_i2c->lock);
> > +	wac_i2c->dev->open = false;
> > +	mutex_unlock(&wac_i2c->lock);
> > +}
> > +
> > +static void wacom_i2c_set_input_values(struct i2c_client *client,
> > +				       struct wacom_i2c *wac_i2c, struct
> input_dev *dev)
> > +{
> > +	dev->name = "WACOM_I2C_DIGITIZER";
> > +	dev->id.bustype = BUS_I2C;
> > +	dev->id.vendor = 0x56a;
> > +	dev->dev.parent = &client->dev;
> > +	dev->open = wacom_i2c_input_open;
> > +	dev->close = wacom_i2c_input_close;
> > +	dev->evbit[0] |= BIT_MASK(EV_KEY) | BIT_MASK(EV_ABS);
> > +
> > +	__set_bit(ABS_X, dev->absbit);
> > +	__set_bit(ABS_Y, dev->absbit);
> > +	__set_bit(ABS_PRESSURE, dev->absbit);
> > +	__set_bit(BTN_TOOL_PEN, dev->keybit);
> > +	__set_bit(BTN_TOOL_RUBBER, dev->keybit);
> > +	__set_bit(BTN_STYLUS, dev->keybit);
> > +	__set_bit(BTN_STYLUS2, dev->keybit);
> > +	__set_bit(BTN_TOUCH, dev->keybit);
> 
> >>Just put this into probe().
> I thought it was too long to see the order of the codes in probe if I put
> this into it. I thought this way is more easy to read what happens in the
> code
> for people trying to customize it.
> However, if this caused some trouble when probing, I would gladly remove it
> and place
> the same statement in probe(). So, could you just give me the advice for
> this change?

probe() is quite simple and most of the drivers initialize input devices
there. I guess it is just my preference.

> 
> > +}
> > +
> > +static int wacom_i2c_probe(struct i2c_client *client,
> > +			   const struct i2c_device_id *id)
> > +{
> > +	struct wacom_i2c *wac_i2c;
> > +	int i, ret;
> > +	i = ret = 0;
> > +
> > +	if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C))
> > +		goto err2;
> > +
> > +	wac_i2c = kzalloc(sizeof(struct wacom_i2c), GFP_KERNEL);
> > +	wac_i2c->wac_feature = wacom_feature_EMR;
> 
> >>Should we get this from i2c_device_id, platform data, device tree or
> >>something else instead of hard-coding?
> Right, there's no reason for leaving this hard-coding here anymore,
> I forget it. This is just useful for us to test or check; My apology.
> I will find another way to initialize it.
> 
> > +
> > +	mutex_init(&wac_i2c->lock);
> > +
> > +	wac_i2c->dev = input_allocate_device();
> > +	if (wac_i2c == NULL || wac_i2c->dev == NULL)
> > +		goto fail;
> > +	wacom_i2c_set_input_values(client, wac_i2c, wac_i2c->dev);
> > +
> > +	wac_i2c->client = client;
> > +
> > +	INIT_WORK(&wac_i2c->workq, wacom_i2c_workqueue);
> > +
> > +	ret = wacom_send_query(wac_i2c);
> > +	if (ret < 0)
> > +		goto err1;
> > +
> > +	wac_i2c->dev->id.version = wac_i2c->wac_feature.fw_version;
> > +	input_set_abs_params(wac_i2c->dev, ABS_X, 0,
> > +			     wac_i2c->wac_feature.x_max, 0, 0);
> > +	input_set_abs_params(wac_i2c->dev, ABS_Y, 0,
> > +			     wac_i2c->wac_feature.y_max, 0, 0);
> > +	input_set_abs_params(wac_i2c->dev, ABS_PRESSURE, 0,
> > +			     wac_i2c->wac_feature.pressure_max, 0, 0);
> > +	input_set_drvdata(wac_i2c->dev, wac_i2c);
> > +
> > +	i2c_set_clientdata(client, wac_i2c);
> > +	if (input_register_device(wac_i2c->dev))
> > +		goto err1;
> 
> 
> >>You should report error returned by input_register_device().
> I understand that, I could do it.
> 
> > +
> > +	hrtimer_init(&wac_i2c->timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
> > +	wac_i2c->timer.function = wacom_i2c_timer;
> > +	hrtimer_start(&wac_i2c->timer, ktime_set(1, 0), HRTIMER_MODE_REL);
> 
> >>Why start timer if nobody has the device opened yet?
> Do your "device" mean "input" subsystem?

By device I mean instance of struct input_dev.

> I thought, at this point, the i2c driver was completed the initialization
> and started probing.
> The reason I put this here is because I thought the device has almost been
> ready to work as a device and opened.
> I guess I made a mistake in the order of the running these i2c and input
> sybsystem.
> Can you tell more in detail, which initializes first, input or i2c?
> Thanks.

It does not really matter, but we do know that by the time this probe()
runs I2C core, I2C controller and input core should be fully functional.
However that does not mean that there are consumers for the input events
generated by the touchscreen, so we should not rush and start polling
right now. We'll get notified when first users appears - input core will
call our open() method and then we can start our polling. Also, input
core will notify us when last user closes the device by calling our
close() method to give us chance to stop polling or otherwise quiesce
the device.

> 
> > +	printk(KERN_INFO "WACOM_I2C_DIGITIZER: initialized and started \n");
> > +
> > +	return 0;
> > +
> > + err2:
> > +	printk(KERN_ERR "wacom_i2c:No I2C functionality found\n");
> > +	return -ENODEV;
> > +
> > + err1:
> > +	printk(KERN_ERR "wacom_i2c:err1 occured(num:%d)\n", ret);
> > +	input_free_device(wac_i2c->dev);
> > +	wac_i2c->dev = NULL;
> > +	return -EIO;
> > +
> > + fail:
> > +	printk(KERN_ERR "wacom_i2c:fail occured\n");
> > +	return -ENOMEM;
> > +}
> > +
> > +static int wacom_i2c_remove(struct i2c_client *client)
> > +{
> > +	struct wacom_i2c *wac_i2c = i2c_get_clientdata(client);
> > +
> > +	input_unregister_device(wac_i2c->dev);
> 
> >>Timer fires here -> *BOOM*. You should cancel the timer in close().
> Thanks for letting me know that. I will definitely put the cancel in
> close().
> 
> >>Also, polling-only mode is hardly useful for a touchscreen driver. Is
> >>there a chance you could switch to interrupt-driven mode instead?
> Be honest with you, we are OEM vendors and found the setting of the i2c's
> irq lines depends(relays) on customers implementation and
> there's no single way to use i2c's irq.
> For example, one customer simply implements the i2c's irq line by gpio and
> set their address and pins on it in a single
> code for gpio, but the others may have the settings and the initialization
> before enabling its i2c's irq on their system in 
> different or separated codes.
> Then, these customer's settings sometimes affect to irq handler in this code
> in different way.
> I mean I may also change the codes in irq handler differently for each
> customers.
> This happens because each customer produces their own individual system for
> i2c's irq lines.

Setting up interrupts should happen in board code, by the time we get to
the driver binding it shoudl simply use irq line specified in i2c client
structure. If special processing is needed we could think about adding
some platform data with necessary methods at a later point, however for
now just drop the polling and switch to threaded IRQ model for the
driver.

> However, the most of the customers just want to know how our device works
> with the driver.
> Since the code is under GPL, that means the customers freely customize the
> codes; that means they can implement
> interrupt cods by themselves for their own devices.
> So, polling would give them easier way to see if just our device works on
> the screen.
> I guess this complex happens because i2c controller doesn't have irq line on
> itself but relies on the system io port.
> 
> However, I understand what you concern as well, so I will try to add
> something for irq on the next patching release;
> If you are not clear, please let me know; maybe my explanation is enough.
> 

As I mention, please switch to pure interrupt model for the driver -
polling is not suitable for production use and mainline kernel is
supposed to have production-quality code.

> > +	hrtimer_cancel(&wac_i2c->timer);
> > +	kfree(wac_i2c);
> > +
> > +	return 0;
> > +}
> > +
> > +static int wacom_i2c_suspend(struct i2c_client *client, pm_message_t
> mesg)
> > +{
> > +	int ret;
> > +	struct wacom_i2c *wac_i2c = i2c_get_clientdata(client);
> > +
> > +	hrtimer_cancel(&wac_i2c->timer);
> > +	cancel_work_sync(&wac_i2c->workq);
> > +	ret = wac_i2c->power(0);
> > +
> > +	return 0;
> > +}
> > +
> > +static int wacom_i2c_resume(struct i2c_client *client)
> > +{
> > +	struct wacom_i2c *wac_i2c = i2c_get_clientdata(client);
> > +
> > +	hrtimer_start(&wac_i2c->timer, ktime_set(1, 0), HRTIMER_MODE_REL);
> > +
> > +	return 0;
> > +}
> > +
> > +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",
> > +	},
> > +
> > +	.probe = wacom_i2c_probe,
> > +	.remove = wacom_i2c_remove,
> > +	.suspend = wacom_i2c_suspend,
> > +	.resume = wacom_i2c_resume,
> 
> >>Please use dev_pm_ops.
> I understand, deve_pm_ops is commonly and safely used for suspending,
> resuming, or other power management.
> 
> > +	.id_table = wacom_i2c_id,
> > +};
> > +
> > +static int __init wacom_i2c_init()
> > +{
> > +
> > +	wacom_i2c_wq = create_singlethread_workqueue("wacom_i2c_wq");
> > +	if (!wacom_i2c_wq)
> > +		return -ENOMEM;
> 
> >>There is no reason for dedicated workqueue in mainline.
> I'll work on it and I'm thinking replace this in probe() as well.
> 
> > +
> > +	return i2c_add_driver(&wacom_i2c_driver);
> > +}
> > +
> > +static void __exit wacom_i2c_exit()
> > +{
> > +	if (wacom_i2c_wq)
> 
> >>The test is useless.
> Right, I'll take this away.
> 
> > +		destroy_workqueue(wacom_i2c_wq);
> > +
> > +	i2c_del_driver(&wacom_i2c_driver);
> > +}
> > +
> > +module_init(wacom_i2c_init);
> > +module_exit(wacom_i2c_exit);
> > +
> > +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..390b765
> > --- /dev/null
> > +++ b/drivers/input/touchscreen/wacom_i2c.h
> > @@ -0,0 +1,66 @@
> > +#ifndef WACOM_I2C_H
> > +#define WAOCM_I2C_H
> > +
> > +#include <linux/module.h>
> > +#include <linux/init.h>
> > +#include <linux/kernel.h>
> > +#include <linux/input.h>
> > +#include <linux/i2c.h>
> > +#include <linux/slab.h>
> > +#include <linux/hrtimer.h>
> > +#include <linux/delay.h>
> > +#include <linux/uaccess.h>
> > +#include <asm/unaligned.h>
> > +
> > +#define NAMEBUF 12
> > +#define WACNAME "WAC_I2C_EMR"
> > +
> > +/*Wacom Command*/
> > +#define COM_COORD_NUM      256
> > +#define COM_SAMPLERATE_40  0x33
> > +#define COM_SAMPLERATE_80  0x32
> > +#define COM_SAMPLERATE_133 0x31
> > +#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
> > +
> > +/*I2C address for digitizer*/
> > +#define WACOM_I2C_ADDR   0x09
> > +
> > +/*Polling Rate*/
> > +#define POLL_RATE 7500000
> > +
> > +static struct workqueue_struct *wacom_i2c_wq;
> 
> >>There shouldn't be static variables in header files.
> I understand it. I'll find another proper space for it.
> 
> > +
> > +/*Parameters for wacom own features*/
> > +struct wacom_features {
> > +	int x_max;
> > +	int y_max;
> > +	int pressure_max;
> > +	char fw_version;
> > +};
> > +
> > +static struct wacom_features wacom_feature_EMR = {
> > +	16128,
> > +	8448,
> > +	256,
> > +	0x10,
> > +};
> > +
> > +/*Parameters for i2c driver*/
> > +struct wacom_i2c {
> > +	struct i2c_client *client;
> > +	struct input_dev *dev;
> > +	struct mutex lock;
> > +	struct work_struct workq;
> > +	struct hrtimer timer;
> > +	struct wacom_features wac_feature;
> > +	int (*power)(int on);
> > +	int type;
> > +	const char name[NAMEBUF];
> > +};
> > +#endif
> > diff --git a/drivers/input/touchscreen/wacom_i2c_func.c
> b/drivers/input/touchscreen/wacom_i2c_func.c
> > new file mode 100644
> > index 0000000..97f086c
> > --- /dev/null
> > +++ b/drivers/input/touchscreen/wacom_i2c_func.c
> > @@ -0,0 +1,96 @@
> > +#include "wacom_i2c_func.h"
> > +
> 
> >>Why is is split? Also, what license does this code have?
> Do you mean "why did I have two codes, wac_i2c.c and wac_i2c_func.c ?".
> If you do so, I will answer that I split up because it is clear to see
> the difference between the driver initialization and the other
> functionalities which
> are mostly influenced by our device such as querying or contacting to our
> firmware when
> the device gets ready for the users to operate the digitizers.
> So, wac_i2c is mostly dedicated to the driver initialization and
> wac_i2c_func is dedicated to querying and getting the coordination or other
> stuffs related
> to our firmware.

The driver is too small for it to be split in several source files. It
just obscures what is happening.

> wac_i2c_func.c has the license as GPL, the same as wac_i2c.c.
> I should add the license also in it at the last line, right?

No, you should add a comment at the top with the license note. There
should only be one MODULE_LICENSE() per kernel module and you already
have it.

> 
> > +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) {
> > +		printk(KERN_ERR "wacom_i2c: Sending 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) {
> > +		printk(KERN_ERR "wacom_i2c: Sending Query failed \n");
> > +		return -1;
> > +	}
> > +
> > +	ret = i2c_master_recv(wac_i2c->client, data, QUERY_SIZE);
> > +	if (ret < 0) {
> > +		printk(KERN_ERR "wacom_i2c: Receiving Query failed\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]);
> > +
> > +	printk(KERN_INFO "wacom_i2c: 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);
> > +
> 
> >>Please use dev_xxx() for reporting.
> I'll check it and change it to dev_xxx() as you requested for all reporting.
> 
> > +	return 0;
> > +}
> > +
> > +int wacom_set_coordination(struct wacom_i2c *wac_i2c)
> > +{
> > +	int ret = 0;
> > +	unsigned int x, y, pressure;
> > +	unsigned char rdy, tsw, f1, f2, ers;
> > +	u8 data[COM_COORD_NUM];
> > +
> > +	ret = i2c_master_recv(wac_i2c->client, data, QUERY_SIZE);
> > +	if (ret >= 0) {
> > +		tsw = data[3]&0x01;
> > +		ers = data[3]&0x04;
> > +		f1 = data[3]&0x02;
> > +		f2 = data[3]&0x10;
> > +		rdy = data[3]&0x20;
> > +		x = le16_to_cpup((__le16 *)&data[4]);
> > +		y = le16_to_cpup((__le16 *)&data[6]);
> > +		pressure = le16_to_cpup((__le16 *)&data[8]);
> > +
> > +		switch (wac_i2c->type) {
> > +		case BTN_TOOL_RUBBER:
> > +			if (!f2) {
> > +				input_report_abs(wac_i2c->dev, ABS_PRESSURE,
> 0);
> > +				input_report_key(wac_i2c->dev, BTN_TOUCH,
> 0);
> > +				input_report_key(wac_i2c->dev, BTN_STYLUS,
> 0);
> > +				input_report_key(wac_i2c->dev, BTN_STYLUS2,
> 0);
> > +				input_report_key(wac_i2c->dev,
> BTN_TOOL_RUBBER, 0);
> > +				input_sync(wac_i2c->dev);
> > +				wac_i2c->type = BTN_TOOL_PEN;
> > +			}
> > +			break;
> > +
> > +		case KEY_RESERVED:
> > +			wac_i2c->type = f2 ? BTN_TOOL_RUBBER : BTN_TOOL_PEN;
> > +			break;
> > +
> > +		default:
> > +			input_report_key(wac_i2c->dev, BTN_STYLUS2, f2);
> > +			break;
> > +		}
> > +
> > +		input_report_abs(wac_i2c->dev, ABS_X, x);
> > +		input_report_abs(wac_i2c->dev, ABS_Y, y);
> > +		input_report_abs(wac_i2c->dev, ABS_PRESSURE, pressure);
> > +		input_report_key(wac_i2c->dev, BTN_TOUCH, (tsw || ers));
> > +		input_report_key(wac_i2c->dev, BTN_STYLUS, f1);
> > +
> > +		input_sync(wac_i2c->dev);
> > +
> > +		if (!rdy)
> > +			wac_i2c->type = KEY_RESERVED;
> > +
> > +	} else {
> > +		return -1;
> > +	}
> > +
> > +	return 0;
> > +}
> > diff --git a/drivers/input/touchscreen/wacom_i2c_func.h
> b/drivers/input/touchscreen/wacom_i2c_func.h
> > new file mode 100644
> > index 0000000..cce35c4
> > --- /dev/null
> > +++ b/drivers/input/touchscreen/wacom_i2c_func.h
> > @@ -0,0 +1,8 @@
> > +#ifndef WACOM_I2C_FUNC_H
> > +#define WACOM_I2C_FUNC_H
> > +
> > +#include "wacom_i2c.h"
> > +
> > +int wacom_set_coordination(struct wacom_i2c *wac_i2c);
> > +int wacom_send_query(struct wacom_i2c *wac_i2c);
> > +#endif
> 
> Thank you for taking your time to read such long comments.
> I will be looking forward to seeing your reply.
> Please let me know if there's something that you are not sure or doesn't
> make sense.

I will sure do so when you send the updated version of the driver ;)

Thanks.

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

[PATCH] Input:Adding support for Wacom Stylus with I2C interface

[Tatsunosuke Tobita]

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


Signed-off-by: Tatsunosuke Tobita <tobita.tatsunosuke@wacom.co.jp>
---
 drivers/input/touchscreen/wacom_i2c.c      |  216 ++++++++++++++++++++++++++++
 drivers/input/touchscreen/wacom_i2c.h      |   66 +++++++++
 drivers/input/touchscreen/wacom_i2c_func.c |   81 +++++++++++
 drivers/input/touchscreen/wacom_i2c_func.h |    8 +
 4 files changed, 371 insertions(+), 0 deletions(-)
 create mode 100644 drivers/input/touchscreen/wacom_i2c.c
 create mode 100644 drivers/input/touchscreen/wacom_i2c.h
 create mode 100644 drivers/input/touchscreen/wacom_i2c_func.c
 create mode 100644 drivers/input/touchscreen/wacom_i2c_func.h

diff --git a/drivers/input/touchscreen/wacom_i2c.c b/drivers/input/touchscreen/wacom_i2c.c
new file mode 100644
index 0000000..4f5910a
--- /dev/null
+++ b/drivers/input/touchscreen/wacom_i2c.c
@@ -0,0 +1,216 @@
+/*
+ * 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.
+ *
+ * Note: Wacom Penabled Driver for I2C obtains data by polling.
+ * If you use irq, you must use the edge based rising
+ * interruption with low level pin.
+ */
+
+#include <linux/device.h>
+#include "wacom_i2c_func.h"
+
+const unsigned short addr[2];
+
+static void wacom_i2c_workqueue(struct work_struct *workq)
+{
+	struct wacom_i2c *wac_i2c = container_of(workq, struct wacom_i2c, workq);
+	wacom_set_coordination(wac_i2c);
+}
+
+static enum hrtimer_restart wacom_i2c_timer(struct hrtimer *timer)
+{
+	struct wacom_i2c *wac_i2c = container_of(timer, struct wacom_i2c, timer);
+
+	queue_work(wacom_i2c_wq, &wac_i2c->workq);
+	hrtimer_start(&wac_i2c->timer, ktime_set(0, POLL_RATE), HRTIMER_MODE_REL);
+
+	return HRTIMER_NORESTART;
+}
+
+static int wacom_i2c_input_open(struct input_dev *dev)
+{
+	struct wacom_i2c *wac_i2c = input_get_drvdata(dev);
+
+	mutex_lock(&wac_i2c->lock);
+	wac_i2c->dev->open = true;
+	mutex_unlock(&wac_i2c->lock);
+
+	return 0;
+}
+
+static void wacom_i2c_input_close(struct input_dev *dev)
+{
+	struct wacom_i2c *wac_i2c = input_get_drvdata(dev);
+
+	mutex_lock(&wac_i2c->lock);
+	wac_i2c->dev->open = false;
+	mutex_unlock(&wac_i2c->lock);
+}
+
+static void wacom_i2c_set_input_values(struct i2c_client *client,
+				       struct wacom_i2c *wac_i2c, struct input_dev *dev)
+{
+	dev->name = "WACOM_I2C_DIGITIZER";
+	dev->id.bustype = BUS_I2C;
+	dev->id.vendor = 0x56a;
+	dev->dev.parent = &client->dev;
+	dev->open = wacom_i2c_input_open;
+	dev->close = wacom_i2c_input_close;
+	dev->evbit[0] |= BIT_MASK(EV_KEY) | BIT_MASK(EV_ABS);
+
+	__set_bit(ABS_X, dev->absbit);
+	__set_bit(ABS_Y, dev->absbit);
+	__set_bit(ABS_PRESSURE, dev->absbit);
+	__set_bit(BTN_TOOL_PEN, dev->keybit);
+	__set_bit(BTN_TOOL_RUBBER, dev->keybit);
+	__set_bit(BTN_STYLUS, dev->keybit);
+	__set_bit(BTN_STYLUS2, dev->keybit);
+	__set_bit(BTN_TOUCH, dev->keybit);
+}
+
+static int wacom_i2c_probe(struct i2c_client *client,
+			   const struct i2c_device_id *id)
+{
+	struct wacom_i2c *wac_i2c;
+	int i, ret;
+	i = ret = 0;
+
+	if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C))
+		goto err2;
+
+	wac_i2c = kzalloc(sizeof(struct wacom_i2c), GFP_KERNEL);
+	wac_i2c->wac_feature = &wacom_feature_EMR;
+
+	mutex_init(&wac_i2c->lock);
+
+	wac_i2c->dev = input_allocate_device();
+	if (wac_i2c == NULL || wac_i2c->dev == NULL)
+		goto fail;
+	wacom_i2c_set_input_values(client, wac_i2c, wac_i2c->dev);
+
+	wac_i2c->client = client;
+
+	INIT_WORK(&wac_i2c->workq, wacom_i2c_workqueue);
+
+	ret = wacom_send_query(wac_i2c);
+	if (ret < 0)
+		goto err1;
+
+	wac_i2c->dev->id.version = wac_i2c->wac_feature->fw_version;
+	input_set_abs_params(wac_i2c->dev, ABS_X, 0,
+			     wac_i2c->wac_feature->x_max, 0, 0);
+	input_set_abs_params(wac_i2c->dev, ABS_Y, 0,
+			     wac_i2c->wac_feature->y_max, 0, 0);
+	input_set_abs_params(wac_i2c->dev, ABS_PRESSURE, 0,
+			     wac_i2c->wac_feature->pressure_max, 0, 0);
+	input_set_drvdata(wac_i2c->dev, wac_i2c);
+
+	i2c_set_clientdata(client, wac_i2c);
+	if (input_register_device(wac_i2c->dev))
+		goto err1;
+
+	hrtimer_init(&wac_i2c->timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
+	wac_i2c->timer.function = wacom_i2c_timer;
+	hrtimer_start(&wac_i2c->timer, ktime_set(1, 0), HRTIMER_MODE_REL);
+	printk(KERN_INFO "WACOM_I2C_DIGITIZER: initialized and started \n");
+
+	return 0;
+
+ err2:
+	printk(KERN_ERR "wacom_i2c:No I2C functionality found\n");
+	return -ENODEV;
+
+ err1:
+	printk(KERN_ERR "wacom_i2c:err1 occured(num:%d)\n", ret);
+	input_free_device(wac_i2c->dev);
+	wac_i2c->dev = NULL;
+	return -EIO;
+
+ fail:
+	printk(KERN_ERR "wacom_i2c:fail occured\n");
+	return -ENOMEM;
+}
+
+static int wacom_i2c_remove(struct i2c_client *client)
+{
+	struct wacom_i2c *wac_i2c = i2c_get_clientdata(client);
+
+	input_unregister_device(wac_i2c->dev);
+	hrtimer_cancel(&wac_i2c->timer);
+	kfree(wac_i2c);
+
+	return 0;
+}
+
+static int wacom_i2c_suspend(struct i2c_client *client, pm_message_t mesg)
+{
+	int ret;
+	struct wacom_i2c *wac_i2c = i2c_get_clientdata(client);
+
+	hrtimer_cancel(&wac_i2c->timer);
+	cancel_work_sync(&wac_i2c->workq);
+	ret = wac_i2c->power(0);
+
+	return 0;
+}
+
+static int wacom_i2c_resume(struct i2c_client *client)
+{
+	struct wacom_i2c *wac_i2c = i2c_get_clientdata(client);
+
+	hrtimer_start(&wac_i2c->timer, ktime_set(1, 0), HRTIMER_MODE_REL);
+
+	return 0;
+}
+
+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",
+	},
+
+	.probe = wacom_i2c_probe,
+	.remove = wacom_i2c_remove,
+	.suspend = wacom_i2c_suspend,
+	.resume = wacom_i2c_resume,
+	.id_table = wacom_i2c_id,
+};
+
+static int __init wacom_i2c_init()
+{
+
+	wacom_i2c_wq = create_singlethread_workqueue("wacom_i2c_wq");
+	if (!wacom_i2c_wq)
+		return -ENOMEM;
+
+	return i2c_add_driver(&wacom_i2c_driver);
+}
+
+static void __exit wacom_i2c_exit()
+{
+	if (wacom_i2c_wq)
+		destroy_workqueue(wacom_i2c_wq);
+
+	i2c_del_driver(&wacom_i2c_driver);
+}
+
+module_init(wacom_i2c_init);
+module_exit(wacom_i2c_exit);
+
+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..4c5f79f
--- /dev/null
+++ b/drivers/input/touchscreen/wacom_i2c.h
@@ -0,0 +1,66 @@
+#ifndef WACOM_I2C_H
+#define WAOCM_I2C_H
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/input.h>
+#include <linux/i2c.h>
+#include <linux/slab.h>
+#include <linux/hrtimer.h>
+#include <linux/delay.h>
+#include <linux/uaccess.h>
+#include <asm/unaligned.h>
+
+#define NAMEBUF 12
+#define WACNAME "WAC_I2C_EMR"
+
+/*Wacom Command*/
+#define COM_COORD_NUM      256
+#define COM_SAMPLERATE_40  0x33
+#define COM_SAMPLERATE_80  0x32
+#define COM_SAMPLERATE_133 0x31
+#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
+
+/*I2C address for digitizer*/
+#define WACOM_I2C_ADDR   0x09
+
+/*Polling Rate*/
+#define POLL_RATE 7500000
+
+static struct workqueue_struct *wacom_i2c_wq;
+
+/*Parameters for wacom own features*/
+struct wacom_features {
+	int x_max;
+	int y_max;
+	int pressure_max;
+	char fw_version;
+	u8 data[COM_COORD_NUM];
+};
+
+static struct wacom_features wacom_feature_EMR = {
+	16128,
+	8448,
+	256,
+	0x10
+};
+
+/*Parameters for i2c driver*/
+struct wacom_i2c {
+	struct i2c_client *client;
+	struct input_dev *dev;
+	struct mutex lock;
+	struct work_struct workq;
+	struct hrtimer timer;
+	struct wacom_features *wac_feature;
+	int (*power)(int on);
+	const char name[NAMEBUF];
+};
+#endif
diff --git a/drivers/input/touchscreen/wacom_i2c_func.c b/drivers/input/touchscreen/wacom_i2c_func.c
new file mode 100644
index 0000000..51e6f4f
--- /dev/null
+++ b/drivers/input/touchscreen/wacom_i2c_func.c
@@ -0,0 +1,81 @@
+#include "wacom_i2c_func.h"
+
+int wacom_send_query(struct wacom_i2c *wac_i2c)
+{
+	struct wacom_features *wac_feature = wac_i2c->wac_feature;
+	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) {
+		printk(KERN_ERR "wacom_i2c: Sending 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) {
+		printk(KERN_ERR "wacom_i2c: Sending Query failed \n");
+		return -1;
+	}
+
+	ret = i2c_master_recv(wac_i2c->client, data, QUERY_SIZE);
+	if (ret < 0) {
+		printk(KERN_ERR "wacom_i2c: Receiving Query failed\n");
+		return -1;
+	}
+
+	wac_feature->x_max = get_unaligned_le16(&data[3]);
+	wac_feature->y_max = get_unaligned_le16(&data[5]);
+	wac_feature->pressure_max = get_unaligned_le16(&data[11]);
+	wac_feature->fw_version = get_unaligned_le16(&data[13]);
+
+	printk(KERN_INFO "wacom_i2c: x_max:%d, y_max:%d, pressure:%d, fw:%d\n",
+	       wac_feature->x_max, wac_feature->y_max, wac_feature->pressure_max,
+	       wac_feature->fw_version);
+
+	return 0;
+}
+
+int wacom_set_coordination(struct wacom_i2c *wac_i2c)
+{
+	int ret = 0;
+	unsigned int x_max, y_max;
+	unsigned int x, y, pressure;
+	unsigned char rdy, tsw, f1, f2, ers;
+	u8 *data;
+
+	data = wac_i2c->wac_feature->data;
+	x_max = wac_i2c->wac_feature->x_max;
+	y_max = wac_i2c->wac_feature->y_max;
+
+	ret = i2c_master_recv(wac_i2c->client, data, QUERY_SIZE);
+	if (ret >= 0) {
+		tsw = data[3]&0x01;
+		ers = data[3]&0x04;
+		f1 = data[3]&0x02;
+		f2 = data[3]&0x10;
+		rdy = data[3]&0x20;
+
+		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->dev, ABS_X, x);
+		input_report_abs(wac_i2c->dev, ABS_Y, y);
+		input_report_abs(wac_i2c->dev, ABS_PRESSURE, pressure);
+		input_report_key(wac_i2c->dev, BTN_TOOL_PEN, tsw);
+		input_report_key(wac_i2c->dev, BTN_STYLUS, f1);
+		input_report_key(wac_i2c->dev, BTN_STYLUS2, f2);
+		input_report_key(wac_i2c->dev, BTN_TOOL_RUBBER, ers);
+		input_report_key(wac_i2c->dev, BTN_TOUCH, (tsw || ers));
+
+		input_sync(wac_i2c->dev);
+
+	} else {
+		return -1;
+	}
+
+	return 0;
+}
diff --git a/drivers/input/touchscreen/wacom_i2c_func.h b/drivers/input/touchscreen/wacom_i2c_func.h
new file mode 100644
index 0000000..cce35c4
--- /dev/null
+++ b/drivers/input/touchscreen/wacom_i2c_func.h
@@ -0,0 +1,8 @@
+#ifndef WACOM_I2C_FUNC_H
+#define WACOM_I2C_FUNC_H
+
+#include "wacom_i2c.h"
+
+int wacom_set_coordination(struct wacom_i2c *wac_i2c);
+int wacom_send_query(struct wacom_i2c *wac_i2c);
+#endif
-- 
1.7.1

Re: [PATCH] Input:Adding support for Wacom Stylus with I2C interface

[Chris Bagwell]

Hi Tatsunosuke,

I am not qualified to comment on driver framework but I have a comment
on the pen packet processing below.

On Mon, Oct 3, 2011 at 3:27 AM, Tatsunosuke Tobita
<junkpainting@gmail.com> wrote:
> From: Tatsunosuke Tobita <tobita.tatsunosuke@wacom.co.jp>
>
>
> Signed-off-by: Tatsunosuke Tobita <tobita.tatsunosuke@wacom.co.jp>
> ---
>  drivers/input/touchscreen/wacom_i2c.c      |  216 ++++++++++++++++++++++++++++
>  drivers/input/touchscreen/wacom_i2c.h      |   66 +++++++++
>  drivers/input/touchscreen/wacom_i2c_func.c |   81 +++++++++++
>  drivers/input/touchscreen/wacom_i2c_func.h |    8 +
>  4 files changed, 371 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/input/touchscreen/wacom_i2c.c
>  create mode 100644 drivers/input/touchscreen/wacom_i2c.h
>  create mode 100644 drivers/input/touchscreen/wacom_i2c_func.c
>  create mode 100644 drivers/input/touchscreen/wacom_i2c_func.h
>
> diff --git a/drivers/input/touchscreen/wacom_i2c.c b/drivers/input/touchscreen/wacom_i2c.c
> new file mode 100644
> index 0000000..4f5910a
> --- /dev/null
> +++ b/drivers/input/touchscreen/wacom_i2c.c
> @@ -0,0 +1,216 @@
> +/*
> + * 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.
> + *
> + * Note: Wacom Penabled Driver for I2C obtains data by polling.
> + * If you use irq, you must use the edge based rising
> + * interruption with low level pin.
> + */
> +
> +#include <linux/device.h>
> +#include "wacom_i2c_func.h"
> +
> +const unsigned short addr[2];
> +
> +static void wacom_i2c_workqueue(struct work_struct *workq)
> +{
> +       struct wacom_i2c *wac_i2c = container_of(workq, struct wacom_i2c, workq);
> +       wacom_set_coordination(wac_i2c);
> +}
> +
> +static enum hrtimer_restart wacom_i2c_timer(struct hrtimer *timer)
> +{
> +       struct wacom_i2c *wac_i2c = container_of(timer, struct wacom_i2c, timer);
> +
> +       queue_work(wacom_i2c_wq, &wac_i2c->workq);
> +       hrtimer_start(&wac_i2c->timer, ktime_set(0, POLL_RATE), HRTIMER_MODE_REL);
> +
> +       return HRTIMER_NORESTART;
> +}
> +
> +static int wacom_i2c_input_open(struct input_dev *dev)
> +{
> +       struct wacom_i2c *wac_i2c = input_get_drvdata(dev);
> +
> +       mutex_lock(&wac_i2c->lock);
> +       wac_i2c->dev->open = true;
> +       mutex_unlock(&wac_i2c->lock);
> +
> +       return 0;
> +}
> +
> +static void wacom_i2c_input_close(struct input_dev *dev)
> +{
> +       struct wacom_i2c *wac_i2c = input_get_drvdata(dev);
> +
> +       mutex_lock(&wac_i2c->lock);
> +       wac_i2c->dev->open = false;
> +       mutex_unlock(&wac_i2c->lock);
> +}
> +
> +static void wacom_i2c_set_input_values(struct i2c_client *client,
> +                                      struct wacom_i2c *wac_i2c, struct input_dev *dev)
> +{
> +       dev->name = "WACOM_I2C_DIGITIZER";
> +       dev->id.bustype = BUS_I2C;
> +       dev->id.vendor = 0x56a;
> +       dev->dev.parent = &client->dev;
> +       dev->open = wacom_i2c_input_open;
> +       dev->close = wacom_i2c_input_close;
> +       dev->evbit[0] |= BIT_MASK(EV_KEY) | BIT_MASK(EV_ABS);
> +
> +       __set_bit(ABS_X, dev->absbit);
> +       __set_bit(ABS_Y, dev->absbit);
> +       __set_bit(ABS_PRESSURE, dev->absbit);
> +       __set_bit(BTN_TOOL_PEN, dev->keybit);
> +       __set_bit(BTN_TOOL_RUBBER, dev->keybit);
> +       __set_bit(BTN_STYLUS, dev->keybit);
> +       __set_bit(BTN_STYLUS2, dev->keybit);
> +       __set_bit(BTN_TOUCH, dev->keybit);
> +}
> +
> +static int wacom_i2c_probe(struct i2c_client *client,
> +                          const struct i2c_device_id *id)
> +{
> +       struct wacom_i2c *wac_i2c;
> +       int i, ret;
> +       i = ret = 0;
> +
> +       if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C))
> +               goto err2;
> +
> +       wac_i2c = kzalloc(sizeof(struct wacom_i2c), GFP_KERNEL);
> +       wac_i2c->wac_feature = &wacom_feature_EMR;
> +
> +       mutex_init(&wac_i2c->lock);
> +
> +       wac_i2c->dev = input_allocate_device();
> +       if (wac_i2c == NULL || wac_i2c->dev == NULL)
> +               goto fail;
> +       wacom_i2c_set_input_values(client, wac_i2c, wac_i2c->dev);
> +
> +       wac_i2c->client = client;
> +
> +       INIT_WORK(&wac_i2c->workq, wacom_i2c_workqueue);
> +
> +       ret = wacom_send_query(wac_i2c);
> +       if (ret < 0)
> +               goto err1;
> +
> +       wac_i2c->dev->id.version = wac_i2c->wac_feature->fw_version;
> +       input_set_abs_params(wac_i2c->dev, ABS_X, 0,
> +                            wac_i2c->wac_feature->x_max, 0, 0);
> +       input_set_abs_params(wac_i2c->dev, ABS_Y, 0,
> +                            wac_i2c->wac_feature->y_max, 0, 0);
> +       input_set_abs_params(wac_i2c->dev, ABS_PRESSURE, 0,
> +                            wac_i2c->wac_feature->pressure_max, 0, 0);
> +       input_set_drvdata(wac_i2c->dev, wac_i2c);
> +
> +       i2c_set_clientdata(client, wac_i2c);
> +       if (input_register_device(wac_i2c->dev))
> +               goto err1;
> +
> +       hrtimer_init(&wac_i2c->timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
> +       wac_i2c->timer.function = wacom_i2c_timer;
> +       hrtimer_start(&wac_i2c->timer, ktime_set(1, 0), HRTIMER_MODE_REL);
> +       printk(KERN_INFO "WACOM_I2C_DIGITIZER: initialized and started \n");
> +
> +       return 0;
> +
> + err2:
> +       printk(KERN_ERR "wacom_i2c:No I2C functionality found\n");
> +       return -ENODEV;
> +
> + err1:
> +       printk(KERN_ERR "wacom_i2c:err1 occured(num:%d)\n", ret);
> +       input_free_device(wac_i2c->dev);
> +       wac_i2c->dev = NULL;
> +       return -EIO;
> +
> + fail:
> +       printk(KERN_ERR "wacom_i2c:fail occured\n");
> +       return -ENOMEM;
> +}
> +
> +static int wacom_i2c_remove(struct i2c_client *client)
> +{
> +       struct wacom_i2c *wac_i2c = i2c_get_clientdata(client);
> +
> +       input_unregister_device(wac_i2c->dev);
> +       hrtimer_cancel(&wac_i2c->timer);
> +       kfree(wac_i2c);
> +
> +       return 0;
> +}
> +
> +static int wacom_i2c_suspend(struct i2c_client *client, pm_message_t mesg)
> +{
> +       int ret;
> +       struct wacom_i2c *wac_i2c = i2c_get_clientdata(client);
> +
> +       hrtimer_cancel(&wac_i2c->timer);
> +       cancel_work_sync(&wac_i2c->workq);
> +       ret = wac_i2c->power(0);
> +
> +       return 0;
> +}
> +
> +static int wacom_i2c_resume(struct i2c_client *client)
> +{
> +       struct wacom_i2c *wac_i2c = i2c_get_clientdata(client);
> +
> +       hrtimer_start(&wac_i2c->timer, ktime_set(1, 0), HRTIMER_MODE_REL);
> +
> +       return 0;
> +}
> +
> +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",
> +       },
> +
> +       .probe = wacom_i2c_probe,
> +       .remove = wacom_i2c_remove,
> +       .suspend = wacom_i2c_suspend,
> +       .resume = wacom_i2c_resume,
> +       .id_table = wacom_i2c_id,
> +};
> +
> +static int __init wacom_i2c_init()
> +{
> +
> +       wacom_i2c_wq = create_singlethread_workqueue("wacom_i2c_wq");
> +       if (!wacom_i2c_wq)
> +               return -ENOMEM;
> +
> +       return i2c_add_driver(&wacom_i2c_driver);
> +}
> +
> +static void __exit wacom_i2c_exit()
> +{
> +       if (wacom_i2c_wq)
> +               destroy_workqueue(wacom_i2c_wq);
> +
> +       i2c_del_driver(&wacom_i2c_driver);
> +}
> +
> +module_init(wacom_i2c_init);
> +module_exit(wacom_i2c_exit);
> +
> +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..4c5f79f
> --- /dev/null
> +++ b/drivers/input/touchscreen/wacom_i2c.h
> @@ -0,0 +1,66 @@
> +#ifndef WACOM_I2C_H
> +#define WAOCM_I2C_H
> +
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/kernel.h>
> +#include <linux/input.h>
> +#include <linux/i2c.h>
> +#include <linux/slab.h>
> +#include <linux/hrtimer.h>
> +#include <linux/delay.h>
> +#include <linux/uaccess.h>
> +#include <asm/unaligned.h>
> +
> +#define NAMEBUF 12
> +#define WACNAME "WAC_I2C_EMR"
> +
> +/*Wacom Command*/
> +#define COM_COORD_NUM      256
> +#define COM_SAMPLERATE_40  0x33
> +#define COM_SAMPLERATE_80  0x32
> +#define COM_SAMPLERATE_133 0x31
> +#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
> +
> +/*I2C address for digitizer*/
> +#define WACOM_I2C_ADDR   0x09
> +
> +/*Polling Rate*/
> +#define POLL_RATE 7500000
> +
> +static struct workqueue_struct *wacom_i2c_wq;
> +
> +/*Parameters for wacom own features*/
> +struct wacom_features {
> +       int x_max;
> +       int y_max;
> +       int pressure_max;
> +       char fw_version;
> +       u8 data[COM_COORD_NUM];
> +};
> +
> +static struct wacom_features wacom_feature_EMR = {
> +       16128,
> +       8448,
> +       256,
> +       0x10
> +};
> +
> +/*Parameters for i2c driver*/
> +struct wacom_i2c {
> +       struct i2c_client *client;
> +       struct input_dev *dev;
> +       struct mutex lock;
> +       struct work_struct workq;
> +       struct hrtimer timer;
> +       struct wacom_features *wac_feature;
> +       int (*power)(int on);
> +       const char name[NAMEBUF];
> +};
> +#endif
> diff --git a/drivers/input/touchscreen/wacom_i2c_func.c b/drivers/input/touchscreen/wacom_i2c_func.c
> new file mode 100644
> index 0000000..51e6f4f
> --- /dev/null
> +++ b/drivers/input/touchscreen/wacom_i2c_func.c
> @@ -0,0 +1,81 @@
> +#include "wacom_i2c_func.h"
> +
> +int wacom_send_query(struct wacom_i2c *wac_i2c)
> +{
> +       struct wacom_features *wac_feature = wac_i2c->wac_feature;
> +       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) {
> +               printk(KERN_ERR "wacom_i2c: Sending 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) {
> +               printk(KERN_ERR "wacom_i2c: Sending Query failed \n");
> +               return -1;
> +       }
> +
> +       ret = i2c_master_recv(wac_i2c->client, data, QUERY_SIZE);
> +       if (ret < 0) {
> +               printk(KERN_ERR "wacom_i2c: Receiving Query failed\n");
> +               return -1;
> +       }
> +
> +       wac_feature->x_max = get_unaligned_le16(&data[3]);
> +       wac_feature->y_max = get_unaligned_le16(&data[5]);
> +       wac_feature->pressure_max = get_unaligned_le16(&data[11]);
> +       wac_feature->fw_version = get_unaligned_le16(&data[13]);
> +
> +       printk(KERN_INFO "wacom_i2c: x_max:%d, y_max:%d, pressure:%d, fw:%d\n",
> +              wac_feature->x_max, wac_feature->y_max, wac_feature->pressure_max,
> +              wac_feature->fw_version);
> +
> +       return 0;
> +}
> +
> +int wacom_set_coordination(struct wacom_i2c *wac_i2c)
> +{
> +       int ret = 0;
> +       unsigned int x_max, y_max;
> +       unsigned int x, y, pressure;
> +       unsigned char rdy, tsw, f1, f2, ers;
> +       u8 *data;
> +
> +       data = wac_i2c->wac_feature->data;
> +       x_max = wac_i2c->wac_feature->x_max;
> +       y_max = wac_i2c->wac_feature->y_max;

Unreferenced variables?

> +
> +       ret = i2c_master_recv(wac_i2c->client, data, QUERY_SIZE);
> +       if (ret >= 0) {
> +               tsw = data[3]&0x01;
> +               ers = data[3]&0x04;
> +               f1 = data[3]&0x02;
> +               f2 = data[3]&0x10;
> +               rdy = data[3]&0x20;

rdy appears not to be referenced below so I would think that pen only
works while touching screen in this driver.

The packet format looks the same as that used over serial link by
wacom_w8001.c driver.  That driver addresses the complexity in
detecting pen versus eraser and works while hovering.  Looks like you
can copy that logic.

Chris

> +
> +               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->dev, ABS_X, x);
> +               input_report_abs(wac_i2c->dev, ABS_Y, y);
> +               input_report_abs(wac_i2c->dev, ABS_PRESSURE, pressure);
> +               input_report_key(wac_i2c->dev, BTN_TOOL_PEN, tsw);
> +               input_report_key(wac_i2c->dev, BTN_STYLUS, f1);
> +               input_report_key(wac_i2c->dev, BTN_STYLUS2, f2);
> +               input_report_key(wac_i2c->dev, BTN_TOOL_RUBBER, ers);
> +               input_report_key(wac_i2c->dev, BTN_TOUCH, (tsw || ers));
> +
> +               input_sync(wac_i2c->dev);
> +
> +       } else {
> +               return -1;
> +       }
> +
> +       return 0;
> +}
> diff --git a/drivers/input/touchscreen/wacom_i2c_func.h b/drivers/input/touchscreen/wacom_i2c_func.h
> new file mode 100644
> index 0000000..cce35c4
> --- /dev/null
> +++ b/drivers/input/touchscreen/wacom_i2c_func.h
> @@ -0,0 +1,8 @@
> +#ifndef WACOM_I2C_FUNC_H
> +#define WACOM_I2C_FUNC_H
> +
> +#include "wacom_i2c.h"
> +
> +int wacom_set_coordination(struct wacom_i2c *wac_i2c);
> +int wacom_send_query(struct wacom_i2c *wac_i2c);
> +#endif
> --
> 1.7.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-input" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html