All of lore.kernel.org
 help / color / mirror / Atom feed
* [RESEND][PATCH] input: Added TSC2003
@ 2009-06-15 15:09 Richard Röjfors
  2009-06-15 18:10   ` Trilok Soni
  0 siblings, 1 reply; 12+ messages in thread
From: Richard Röjfors @ 2009-06-15 15:09 UTC (permalink / raw)
  To: linux-input; +Cc: Linux Kernel Mailing List, Andrew Morton

Supplied is a driver for the TSC2003. There is actually a driver for TSC2007 which can be used in some cases.
The platform struct is reused from the TSC2007.
There is a big difference in the implementation between the drivers, this one does not use HR timers.
The TSC2007 driver performs synchronous I2C in the timer callback (SW IRQ context) which is bad when
the I2C driver is interrupt driven.


Signed-off-by: Richard Röjfors <richard.rojfors.ext@mocean-labs.com>
---
Index: linux-2.6.30-rc7/drivers/input/touchscreen/tsc2003.c
===================================================================
--- linux-2.6.30-rc7/drivers/input/touchscreen/tsc2003.c	(revision 0)
+++ linux-2.6.30-rc7/drivers/input/touchscreen/tsc2003.c	(revision 869)
@@ -0,0 +1,387 @@
+/*
+ * tsc2003.c Driver for TI TSC2003 touch screen controller
+ * Copyright (c) 2009 Intel Corporation
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+ */
+
+/* Supports:
+ * TI TSC2003
+ *
+ * Inspired by tsc2007, Copyright (c) 2008 MtekVision Co., Ltd.
+ */
+#include <linux/module.h>
+#include <linux/input.h>
+#include <linux/interrupt.h>
+#include <linux/i2c.h>
+#include <linux/i2c/tsc2007.h>
+#include <linux/kthread.h>
+#include <linux/semaphore.h>
+
+#define TSC2003_DRIVER_NAME		"tsc2003"
+
+#define TS_POLL_PERIOD	20		/* ms delay between samples */
+
+#define TSC2003_MEASURE_TEMP0		(0x0 << 4)
+#define TSC2003_MEASURE_AUX		(0x2 << 4)
+#define TSC2003_MEASURE_TEMP1		(0x4 << 4)
+#define TSC2003_ACTIVATE_XN		(0x8 << 4)
+#define TSC2003_ACTIVATE_YN		(0x9 << 4)
+#define TSC2003_ACTIVATE_YP_XN		(0xa << 4)
+#define TSC2003_SETUP			(0xb << 4)
+#define TSC2003_MEASURE_X		(0xc << 4)
+#define TSC2003_MEASURE_Y		(0xd << 4)
+#define TSC2003_MEASURE_Z1		(0xe << 4)
+#define TSC2003_MEASURE_Z2		(0xf << 4)
+
+#define TSC2003_POWER_OFF_IRQ_EN	(0x0 << 2)
+#define TSC2003_ADC_ON_IRQ_DIS0		(0x1 << 2)
+#define TSC2003_ADC_OFF_IRQ_EN		(0x2 << 2)
+#define TSC2003_ADC_ON_IRQ_DIS1		(0x3 << 2)
+
+#define TSC2003_12BIT			(0x0 << 1)
+#define TSC2003_8BIT			(0x1 << 1)
+
+#define	MAX_12BIT			((1 << 12) - 1)
+
+#define ADC_ON_12BIT	(TSC2003_12BIT | TSC2003_ADC_ON_IRQ_DIS0)
+
+#define READ_Y		(ADC_ON_12BIT | TSC2003_MEASURE_Y)
+#define READ_Z1		(ADC_ON_12BIT | TSC2003_MEASURE_Z1)
+#define READ_Z2		(ADC_ON_12BIT | TSC2003_MEASURE_Z2)
+#define READ_X		(ADC_ON_12BIT | TSC2003_MEASURE_X)
+#define PWRDOWN		(TSC2003_12BIT | TSC2003_POWER_OFF_IRQ_EN)
+
+struct ts_event {
+	int	x;
+	int	y;
+	int	z1, z2;
+};
+
+struct tsc2003 {
+	struct input_dev	*input;
+	char			phys[32];
+	struct task_struct	*task;
+	struct ts_event		tc;
+	struct completion 	penirq_completion;
+
+	struct i2c_client	*client;
+
+	u16			model;
+	u16			x_plate_ohms;
+
+	unsigned		pendown;
+};
+
+static inline int tsc2003_xfer(struct tsc2003 *tsc, u8 cmd)
+{
+	s32 data;
+	u16 val;
+
+	data = i2c_smbus_read_word_data(tsc->client, cmd);
+	if (data < 0) {
+		dev_err(&tsc->client->dev, "i2c io error: %d\n", data);
+		return data;
+	}
+
+	/* The protocol and raw data format from i2c interface:
+	 * S Addr Wr [A] Comm [A] S Addr Rd [A] [DataLow] A [DataHigh] NA P
+	 * Where DataLow has [D11-D4], DataHigh has [D3-D0 << 4 | Dummy 4bit].
+	 */
+	val = swab16(data) >> 4;
+
+	dev_dbg(&tsc->client->dev, "data: 0x%x, val: 0x%x\n", data, val);
+
+	return val;
+}
+
+static void tsc2003_send_event(void *tsc)
+{
+	struct tsc2003	*ts = tsc;
+	struct input_dev *input = ts->input;
+	u32		rt = 0;
+	u16		x, y, z1, z2;
+
+	x = ts->tc.x;
+	y = ts->tc.y;
+	z1 = ts->tc.z1;
+	z2 = ts->tc.z2;
+
+	/* range filtering */
+	if (x == MAX_12BIT)
+		x = 0;
+
+	if (likely(x && z1)) {
+		/* compute touch pressure resistance using equation #1 */
+		rt = z2;
+		rt -= z1;
+		rt *= x;
+		rt *= ts->x_plate_ohms;
+		rt /= z1;
+		rt = (rt + 2047) >> 12;
+	}
+
+	/* Sample found inconsistent by debouncing or pressure is beyond
+	 * the maximum. Don't report it to user space, repeat at least
+	 * once more the measurement
+	 */
+	if (rt > MAX_12BIT)
+		return;
+
+	/* NOTE: We can't rely on the pressure to determine the pen down
+	 * state, even this controller has a pressure sensor.  The pressure
+	 * value can fluctuate for quite a while after lifting the pen and
+	 * in some cases may not even settle at the expected value.
+	 *
+	 * The only safe way to check for the pen up condition is in the
+	 * timer by reading the pen signal state (it's a GPIO _and_ IRQ).
+	 */
+	if (rt) {
+		if (!ts->pendown) {
+			dev_dbg(&ts->client->dev, "DOWN\n");
+
+			input_report_key(input, BTN_TOUCH, 1);
+			ts->pendown = 1;
+		}
+
+		input_report_abs(input, ABS_X, x);
+		input_report_abs(input, ABS_Y, y);
+		input_report_abs(input, ABS_PRESSURE, rt);
+
+		input_sync(input);
+
+		dev_dbg(&ts->client->dev, "point(%4d,%4d), pressure (%4u)\n",
+			x, y, rt);
+	} else if (ts->pendown) {
+		/* pen up */
+		dev_dbg(&ts->client->dev, "UP\n");
+		input_report_key(input, BTN_TOUCH, 0);
+		input_report_abs(input, ABS_PRESSURE, 0);
+		input_sync(input);
+
+		ts->pendown = 0;
+	}
+}
+
+static int tsc2003_power_off_irq_en(struct tsc2003 *tsc)
+{
+	/* power down */
+	return tsc2003_xfer(tsc, PWRDOWN);
+}
+
+static int tsc2003_read_values(struct tsc2003 *tsc)
+{
+	/* y- still on; turn on only y+ (and ADC) */
+	tsc->tc.y = tsc2003_xfer(tsc, READ_Y);
+	if (tsc->tc.y < 0)
+		return tsc->tc.y;
+
+	/* turn y- off, x+ on, then leave in lowpower */
+	tsc->tc.x = tsc2003_xfer(tsc, READ_X);
+	if (tsc->tc.x < 0)
+		return tsc->tc.x;
+
+	/* turn y+ off, x- on; we'll use formula #1 */
+	tsc->tc.z1 = tsc2003_xfer(tsc, READ_Z1);
+	if (tsc->tc.z1 < 0)
+		return tsc->tc.z1;
+
+	tsc->tc.z2 = tsc2003_xfer(tsc, READ_Z2);
+	if (tsc->tc.z2 < 0)
+		return tsc->tc.z2;
+
+	return 0;
+}
+
+
+static irqreturn_t tsc2003_irq(int irq, void *handle)
+{
+	struct tsc2003 *ts = handle;
+
+	/* do not call the synced version -> deadlock */
+	disable_irq_nosync(irq);
+	/* signal the thread to continue */
+	complete(&ts->penirq_completion);
+
+	return IRQ_HANDLED;
+}
+
+static int tsc2003_thread(void *d)
+{
+	struct tsc2003 *ts = (struct tsc2003 *)d;
+	int ret;
+
+	allow_signal(SIGKILL);
+
+	while (!signal_pending(current)) {
+		/* power down and wait for interrupt */
+		do {
+			/* loop because the I2C bus might be busy */
+			ret = msleep_interruptible(TS_POLL_PERIOD);
+			if (!ret)
+				ret = tsc2003_power_off_irq_en(ts);
+		} while (ret == -EAGAIN && !signal_pending(current));
+
+		if (signal_pending(current))
+			break;
+
+		ret = wait_for_completion_interruptible(&ts->penirq_completion);
+		if (!ret) {
+			int first = 1;
+			/* got IRQ, start poll, until pen is up */
+			while (!ret && !signal_pending(current)
+				&& (first || ts->pendown)) {
+				ret = tsc2003_read_values(ts);
+				if (!ret)
+					tsc2003_send_event(ts);
+				ret = msleep_interruptible(TS_POLL_PERIOD);
+				first = 0;
+			}
+
+			/* we re enable the interrupt */
+			if (!signal_pending(current))
+				enable_irq(ts->client->irq);
+		}
+	}
+
+	return 0;
+}
+
+static int tsc2003_probe(struct i2c_client *client,
+			const struct i2c_device_id *id)
+{
+	struct tsc2003 *ts;
+	struct tsc2007_platform_data *pdata = client->dev.platform_data;
+	struct input_dev *input_dev;
+	int err;
+
+	if (!pdata) {
+		dev_err(&client->dev, "platform data is required!\n");
+		return -EINVAL;
+	}
+
+	if (!i2c_check_functionality(client->adapter,
+				     I2C_FUNC_SMBUS_READ_WORD_DATA))
+		return -EIO;
+
+	ts = kzalloc(sizeof(struct tsc2003), GFP_KERNEL);
+	input_dev = input_allocate_device();
+	if (!ts || !input_dev) {
+		err = -ENOMEM;
+		goto err_free_mem;
+	}
+
+	ts->client = client;
+	i2c_set_clientdata(client, ts);
+
+	ts->input = input_dev;
+
+	ts->model             = pdata->model;
+	ts->x_plate_ohms      = pdata->x_plate_ohms;
+
+	snprintf(ts->phys, sizeof(ts->phys),
+		 "%s/input0", dev_name(&client->dev));
+
+	input_dev->name = TSC2003_DRIVER_NAME" Touchscreen";
+	input_dev->phys = ts->phys;
+	input_dev->id.bustype = BUS_I2C;
+
+	input_dev->evbit[0] = BIT_MASK(EV_KEY) | BIT_MASK(EV_ABS);
+	input_dev->keybit[BIT_WORD(BTN_TOUCH)] = BIT_MASK(BTN_TOUCH);
+
+	input_set_abs_params(input_dev, ABS_X, 0, MAX_12BIT, 0, 0);
+	input_set_abs_params(input_dev, ABS_Y, 0, MAX_12BIT, 0, 0);
+	input_set_abs_params(input_dev, ABS_PRESSURE, 0, MAX_12BIT, 0, 0);
+
+	init_completion(&ts->penirq_completion);
+
+	ts->task = kthread_run(tsc2003_thread, ts, TSC2003_DRIVER_NAME);
+	if (IS_ERR(ts->task)) {
+		err = PTR_ERR(ts->task);
+		goto err_free_mem;
+	}
+
+	err = request_irq(client->irq, tsc2003_irq, 0,
+			client->dev.driver->name, ts);
+	if (err < 0) {
+		dev_err(&client->dev, "irq %d busy?\n", client->irq);
+		goto err_free_thread;
+	}
+
+	err = input_register_device(input_dev);
+	if (err)
+		goto err_free_irq;
+
+	dev_info(&client->dev, "registered with irq (%d)\n", client->irq);
+
+	return 0;
+
+ err_free_irq:
+	free_irq(client->irq, ts);
+ err_free_thread:
+	kthread_stop(ts->task);
+ err_free_mem:
+	input_free_device(input_dev);
+	kfree(ts);
+	return err;
+}
+
+static int tsc2003_remove(struct i2c_client *client)
+{
+	struct tsc2003 *ts = i2c_get_clientdata(client);
+
+	free_irq(client->irq, ts);
+	send_sig(SIGKILL, ts->task, 1);
+	kthread_stop(ts->task);
+	input_unregister_device(ts->input);
+	kfree(ts);
+
+	return 0;
+}
+
+static struct i2c_device_id tsc2003_idtable[] = {
+	{ TSC2003_DRIVER_NAME, 0 },
+	{ }
+};
+
+MODULE_DEVICE_TABLE(i2c, tsc2003_idtable);
+
+static struct i2c_driver tsc2003_driver = {
+	.driver = {
+		.owner	= THIS_MODULE,
+		.name	= TSC2003_DRIVER_NAME,
+		.bus = &i2c_bus_type,
+	},
+	.id_table	= tsc2003_idtable,
+	.probe		= tsc2003_probe,
+	.remove		= tsc2003_remove,
+};
+
+static int __init tsc2003_init(void)
+{
+	return i2c_add_driver(&tsc2003_driver);
+}
+
+static void __exit tsc2003_exit(void)
+{
+	i2c_del_driver(&tsc2003_driver);
+}
+
+module_init(tsc2003_init);
+module_exit(tsc2003_exit);
+
+MODULE_AUTHOR("Mocean Laboratories <info@mocean-labs.com>");
+MODULE_DESCRIPTION("TSC2003 TouchScreen Driver");
+MODULE_LICENSE("GPL v2");
+
Index: linux-2.6.30-rc7/drivers/input/touchscreen/Kconfig
===================================================================
--- linux-2.6.30-rc7/drivers/input/touchscreen/Kconfig	(revision 861)
+++ linux-2.6.30-rc7/drivers/input/touchscreen/Kconfig	(revision 869)
@@ -455,6 +455,17 @@
 	  To compile this driver as a module, choose M here: the
 	  module will be called touchit213.

+config TOUCHSCREEN_TSC2003
+	tristate "TSC2003 based touchscreens"
+	depends on I2C
+	help
+	  Say Y here if you have a TSC2003 based touchscreen.
+
+	  If unsure, say N.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called tsc2003.
+
 config TOUCHSCREEN_TSC2007
 	tristate "TSC2007 based touchscreens"
 	depends on I2C
Index: linux-2.6.30-rc7/drivers/input/touchscreen/Makefile
===================================================================
--- linux-2.6.30-rc7/drivers/input/touchscreen/Makefile	(revision 861)
+++ linux-2.6.30-rc7/drivers/input/touchscreen/Makefile	(revision 869)
@@ -27,6 +27,7 @@
 obj-$(CONFIG_TOUCHSCREEN_TOUCHIT213)	+= touchit213.o
 obj-$(CONFIG_TOUCHSCREEN_TOUCHRIGHT)	+= touchright.o
 obj-$(CONFIG_TOUCHSCREEN_TOUCHWIN)	+= touchwin.o
+obj-$(CONFIG_TOUCHSCREEN_TSC2003)	+= tsc2003.o
 obj-$(CONFIG_TOUCHSCREEN_TSC2007)	+= tsc2007.o
 obj-$(CONFIG_TOUCHSCREEN_UCB1400)	+= ucb1400_ts.o
 obj-$(CONFIG_TOUCHSCREEN_WACOM_W8001)	+= wacom_w8001.o

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

* Re: [RESEND][PATCH] input: Added TSC2003
  2009-06-15 15:09 [RESEND][PATCH] input: Added TSC2003 Richard Röjfors
@ 2009-06-15 18:10   ` Trilok Soni
  0 siblings, 0 replies; 12+ messages in thread
From: Trilok Soni @ 2009-06-15 18:10 UTC (permalink / raw)
  To: Richard Röjfors
  Cc: linux-input, Linux Kernel Mailing List, Andrew Morton,
	linux-omap, Thierry Reding

Hi,

On Mon, Jun 15, 2009 at 8:39 PM, Richard
Röjfors<richard.rojfors.ext@mocean-labs.com> wrote:
> Supplied is a driver for the TSC2003. There is actually a driver for TSC2007 which can be used in some cases.
> The platform struct is reused from the TSC2007.
> There is a big difference in the implementation between the drivers, this one does not use HR timers.
> The TSC2007 driver performs synchronous I2C in the timer callback (SW IRQ context) which is bad when
> the I2C driver is interrupt driven.
>

Meaning? I think I2C transaction can sleep. I don't see (right now)
need for two drivers. TSC2003 id was added to TSC2007. If you think
TSC2007 doesn't provide enough support for it, then please add that in
TSC2007 driver itself. We don't need create another driver it seems.

I have added Thierry who can further comment on it, as he has access
to device with TSC2003.

>
> Signed-off-by: Richard Röjfors <richard.rojfors.ext@mocean-labs.com>
> ---
> Index: linux-2.6.30-rc7/drivers/input/touchscreen/tsc2003.c
> ===================================================================
> --- linux-2.6.30-rc7/drivers/input/touchscreen/tsc2003.c        (revision 0)
> +++ linux-2.6.30-rc7/drivers/input/touchscreen/tsc2003.c        (revision 869)
> @@ -0,0 +1,387 @@
> +/*
> + * tsc2003.c Driver for TI TSC2003 touch screen controller
> + * Copyright (c) 2009 Intel Corporation
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
> + */
> +
> +/* Supports:
> + * TI TSC2003
> + *
> + * Inspired by tsc2007, Copyright (c) 2008 MtekVision Co., Ltd.
> + */
> +#include <linux/module.h>
> +#include <linux/input.h>
> +#include <linux/interrupt.h>
> +#include <linux/i2c.h>
> +#include <linux/i2c/tsc2007.h>
> +#include <linux/kthread.h>
> +#include <linux/semaphore.h>
> +
> +#define TSC2003_DRIVER_NAME            "tsc2003"
> +
> +#define TS_POLL_PERIOD 20              /* ms delay between samples */
> +
> +#define TSC2003_MEASURE_TEMP0          (0x0 << 4)
> +#define TSC2003_MEASURE_AUX            (0x2 << 4)
> +#define TSC2003_MEASURE_TEMP1          (0x4 << 4)
> +#define TSC2003_ACTIVATE_XN            (0x8 << 4)
> +#define TSC2003_ACTIVATE_YN            (0x9 << 4)
> +#define TSC2003_ACTIVATE_YP_XN         (0xa << 4)
> +#define TSC2003_SETUP                  (0xb << 4)
> +#define TSC2003_MEASURE_X              (0xc << 4)
> +#define TSC2003_MEASURE_Y              (0xd << 4)
> +#define TSC2003_MEASURE_Z1             (0xe << 4)
> +#define TSC2003_MEASURE_Z2             (0xf << 4)
> +
> +#define TSC2003_POWER_OFF_IRQ_EN       (0x0 << 2)
> +#define TSC2003_ADC_ON_IRQ_DIS0                (0x1 << 2)
> +#define TSC2003_ADC_OFF_IRQ_EN         (0x2 << 2)
> +#define TSC2003_ADC_ON_IRQ_DIS1                (0x3 << 2)
> +
> +#define TSC2003_12BIT                  (0x0 << 1)
> +#define TSC2003_8BIT                   (0x1 << 1)
> +
> +#define        MAX_12BIT                       ((1 << 12) - 1)
> +
> +#define ADC_ON_12BIT   (TSC2003_12BIT | TSC2003_ADC_ON_IRQ_DIS0)
> +
> +#define READ_Y         (ADC_ON_12BIT | TSC2003_MEASURE_Y)
> +#define READ_Z1                (ADC_ON_12BIT | TSC2003_MEASURE_Z1)
> +#define READ_Z2                (ADC_ON_12BIT | TSC2003_MEASURE_Z2)
> +#define READ_X         (ADC_ON_12BIT | TSC2003_MEASURE_X)
> +#define PWRDOWN                (TSC2003_12BIT | TSC2003_POWER_OFF_IRQ_EN)
> +
> +struct ts_event {
> +       int     x;
> +       int     y;
> +       int     z1, z2;
> +};
> +
> +struct tsc2003 {
> +       struct input_dev        *input;
> +       char                    phys[32];
> +       struct task_struct      *task;
> +       struct ts_event         tc;
> +       struct completion       penirq_completion;
> +
> +       struct i2c_client       *client;
> +
> +       u16                     model;
> +       u16                     x_plate_ohms;
> +
> +       unsigned                pendown;
> +};
> +
> +static inline int tsc2003_xfer(struct tsc2003 *tsc, u8 cmd)
> +{
> +       s32 data;
> +       u16 val;
> +
> +       data = i2c_smbus_read_word_data(tsc->client, cmd);
> +       if (data < 0) {
> +               dev_err(&tsc->client->dev, "i2c io error: %d\n", data);
> +               return data;
> +       }
> +
> +       /* The protocol and raw data format from i2c interface:
> +        * S Addr Wr [A] Comm [A] S Addr Rd [A] [DataLow] A [DataHigh] NA P
> +        * Where DataLow has [D11-D4], DataHigh has [D3-D0 << 4 | Dummy 4bit].
> +        */
> +       val = swab16(data) >> 4;
> +
> +       dev_dbg(&tsc->client->dev, "data: 0x%x, val: 0x%x\n", data, val);
> +
> +       return val;
> +}
> +
> +static void tsc2003_send_event(void *tsc)
> +{
> +       struct tsc2003  *ts = tsc;
> +       struct input_dev *input = ts->input;
> +       u32             rt = 0;
> +       u16             x, y, z1, z2;
> +
> +       x = ts->tc.x;
> +       y = ts->tc.y;
> +       z1 = ts->tc.z1;
> +       z2 = ts->tc.z2;
> +
> +       /* range filtering */
> +       if (x == MAX_12BIT)
> +               x = 0;
> +
> +       if (likely(x && z1)) {
> +               /* compute touch pressure resistance using equation #1 */
> +               rt = z2;
> +               rt -= z1;
> +               rt *= x;
> +               rt *= ts->x_plate_ohms;
> +               rt /= z1;
> +               rt = (rt + 2047) >> 12;
> +       }
> +
> +       /* Sample found inconsistent by debouncing or pressure is beyond
> +        * the maximum. Don't report it to user space, repeat at least
> +        * once more the measurement
> +        */
> +       if (rt > MAX_12BIT)
> +               return;
> +
> +       /* NOTE: We can't rely on the pressure to determine the pen down
> +        * state, even this controller has a pressure sensor.  The pressure
> +        * value can fluctuate for quite a while after lifting the pen and
> +        * in some cases may not even settle at the expected value.
> +        *
> +        * The only safe way to check for the pen up condition is in the
> +        * timer by reading the pen signal state (it's a GPIO _and_ IRQ).
> +        */
> +       if (rt) {
> +               if (!ts->pendown) {
> +                       dev_dbg(&ts->client->dev, "DOWN\n");
> +
> +                       input_report_key(input, BTN_TOUCH, 1);
> +                       ts->pendown = 1;
> +               }
> +
> +               input_report_abs(input, ABS_X, x);
> +               input_report_abs(input, ABS_Y, y);
> +               input_report_abs(input, ABS_PRESSURE, rt);
> +
> +               input_sync(input);
> +
> +               dev_dbg(&ts->client->dev, "point(%4d,%4d), pressure (%4u)\n",
> +                       x, y, rt);
> +       } else if (ts->pendown) {
> +               /* pen up */
> +               dev_dbg(&ts->client->dev, "UP\n");
> +               input_report_key(input, BTN_TOUCH, 0);
> +               input_report_abs(input, ABS_PRESSURE, 0);
> +               input_sync(input);
> +
> +               ts->pendown = 0;
> +       }
> +}
> +
> +static int tsc2003_power_off_irq_en(struct tsc2003 *tsc)
> +{
> +       /* power down */
> +       return tsc2003_xfer(tsc, PWRDOWN);
> +}
> +
> +static int tsc2003_read_values(struct tsc2003 *tsc)
> +{
> +       /* y- still on; turn on only y+ (and ADC) */
> +       tsc->tc.y = tsc2003_xfer(tsc, READ_Y);
> +       if (tsc->tc.y < 0)
> +               return tsc->tc.y;
> +
> +       /* turn y- off, x+ on, then leave in lowpower */
> +       tsc->tc.x = tsc2003_xfer(tsc, READ_X);
> +       if (tsc->tc.x < 0)
> +               return tsc->tc.x;
> +
> +       /* turn y+ off, x- on; we'll use formula #1 */
> +       tsc->tc.z1 = tsc2003_xfer(tsc, READ_Z1);
> +       if (tsc->tc.z1 < 0)
> +               return tsc->tc.z1;
> +
> +       tsc->tc.z2 = tsc2003_xfer(tsc, READ_Z2);
> +       if (tsc->tc.z2 < 0)
> +               return tsc->tc.z2;
> +
> +       return 0;
> +}
> +
> +
> +static irqreturn_t tsc2003_irq(int irq, void *handle)
> +{
> +       struct tsc2003 *ts = handle;
> +
> +       /* do not call the synced version -> deadlock */
> +       disable_irq_nosync(irq);
> +       /* signal the thread to continue */
> +       complete(&ts->penirq_completion);
> +
> +       return IRQ_HANDLED;
> +}
> +
> +static int tsc2003_thread(void *d)
> +{
> +       struct tsc2003 *ts = (struct tsc2003 *)d;
> +       int ret;
> +
> +       allow_signal(SIGKILL);
> +
> +       while (!signal_pending(current)) {
> +               /* power down and wait for interrupt */
> +               do {
> +                       /* loop because the I2C bus might be busy */
> +                       ret = msleep_interruptible(TS_POLL_PERIOD);
> +                       if (!ret)
> +                               ret = tsc2003_power_off_irq_en(ts);
> +               } while (ret == -EAGAIN && !signal_pending(current));
> +
> +               if (signal_pending(current))
> +                       break;
> +
> +               ret = wait_for_completion_interruptible(&ts->penirq_completion);
> +               if (!ret) {
> +                       int first = 1;
> +                       /* got IRQ, start poll, until pen is up */
> +                       while (!ret && !signal_pending(current)
> +                               && (first || ts->pendown)) {
> +                               ret = tsc2003_read_values(ts);
> +                               if (!ret)
> +                                       tsc2003_send_event(ts);
> +                               ret = msleep_interruptible(TS_POLL_PERIOD);
> +                               first = 0;
> +                       }
> +
> +                       /* we re enable the interrupt */
> +                       if (!signal_pending(current))
> +                               enable_irq(ts->client->irq);
> +               }
> +       }
> +
> +       return 0;
> +}
> +
> +static int tsc2003_probe(struct i2c_client *client,
> +                       const struct i2c_device_id *id)
> +{
> +       struct tsc2003 *ts;
> +       struct tsc2007_platform_data *pdata = client->dev.platform_data;
> +       struct input_dev *input_dev;
> +       int err;
> +
> +       if (!pdata) {
> +               dev_err(&client->dev, "platform data is required!\n");
> +               return -EINVAL;
> +       }
> +
> +       if (!i2c_check_functionality(client->adapter,
> +                                    I2C_FUNC_SMBUS_READ_WORD_DATA))
> +               return -EIO;
> +
> +       ts = kzalloc(sizeof(struct tsc2003), GFP_KERNEL);
> +       input_dev = input_allocate_device();
> +       if (!ts || !input_dev) {
> +               err = -ENOMEM;
> +               goto err_free_mem;
> +       }
> +
> +       ts->client = client;
> +       i2c_set_clientdata(client, ts);
> +
> +       ts->input = input_dev;
> +
> +       ts->model             = pdata->model;
> +       ts->x_plate_ohms      = pdata->x_plate_ohms;
> +
> +       snprintf(ts->phys, sizeof(ts->phys),
> +                "%s/input0", dev_name(&client->dev));
> +
> +       input_dev->name = TSC2003_DRIVER_NAME" Touchscreen";
> +       input_dev->phys = ts->phys;
> +       input_dev->id.bustype = BUS_I2C;
> +
> +       input_dev->evbit[0] = BIT_MASK(EV_KEY) | BIT_MASK(EV_ABS);
> +       input_dev->keybit[BIT_WORD(BTN_TOUCH)] = BIT_MASK(BTN_TOUCH);
> +
> +       input_set_abs_params(input_dev, ABS_X, 0, MAX_12BIT, 0, 0);
> +       input_set_abs_params(input_dev, ABS_Y, 0, MAX_12BIT, 0, 0);
> +       input_set_abs_params(input_dev, ABS_PRESSURE, 0, MAX_12BIT, 0, 0);
> +
> +       init_completion(&ts->penirq_completion);
> +
> +       ts->task = kthread_run(tsc2003_thread, ts, TSC2003_DRIVER_NAME);
> +       if (IS_ERR(ts->task)) {
> +               err = PTR_ERR(ts->task);
> +               goto err_free_mem;
> +       }
> +
> +       err = request_irq(client->irq, tsc2003_irq, 0,
> +                       client->dev.driver->name, ts);
> +       if (err < 0) {
> +               dev_err(&client->dev, "irq %d busy?\n", client->irq);
> +               goto err_free_thread;
> +       }
> +
> +       err = input_register_device(input_dev);
> +       if (err)
> +               goto err_free_irq;
> +
> +       dev_info(&client->dev, "registered with irq (%d)\n", client->irq);
> +
> +       return 0;
> +
> + err_free_irq:
> +       free_irq(client->irq, ts);
> + err_free_thread:
> +       kthread_stop(ts->task);
> + err_free_mem:
> +       input_free_device(input_dev);
> +       kfree(ts);
> +       return err;
> +}
> +
> +static int tsc2003_remove(struct i2c_client *client)
> +{
> +       struct tsc2003 *ts = i2c_get_clientdata(client);
> +
> +       free_irq(client->irq, ts);
> +       send_sig(SIGKILL, ts->task, 1);
> +       kthread_stop(ts->task);
> +       input_unregister_device(ts->input);
> +       kfree(ts);
> +
> +       return 0;
> +}
> +
> +static struct i2c_device_id tsc2003_idtable[] = {
> +       { TSC2003_DRIVER_NAME, 0 },
> +       { }
> +};
> +
> +MODULE_DEVICE_TABLE(i2c, tsc2003_idtable);
> +
> +static struct i2c_driver tsc2003_driver = {
> +       .driver = {
> +               .owner  = THIS_MODULE,
> +               .name   = TSC2003_DRIVER_NAME,
> +               .bus = &i2c_bus_type,
> +       },
> +       .id_table       = tsc2003_idtable,
> +       .probe          = tsc2003_probe,
> +       .remove         = tsc2003_remove,
> +};
> +
> +static int __init tsc2003_init(void)
> +{
> +       return i2c_add_driver(&tsc2003_driver);
> +}
> +
> +static void __exit tsc2003_exit(void)
> +{
> +       i2c_del_driver(&tsc2003_driver);
> +}
> +
> +module_init(tsc2003_init);
> +module_exit(tsc2003_exit);
> +
> +MODULE_AUTHOR("Mocean Laboratories <info@mocean-labs.com>");
> +MODULE_DESCRIPTION("TSC2003 TouchScreen Driver");
> +MODULE_LICENSE("GPL v2");
> +
> Index: linux-2.6.30-rc7/drivers/input/touchscreen/Kconfig
> ===================================================================
> --- linux-2.6.30-rc7/drivers/input/touchscreen/Kconfig  (revision 861)
> +++ linux-2.6.30-rc7/drivers/input/touchscreen/Kconfig  (revision 869)
> @@ -455,6 +455,17 @@
>          To compile this driver as a module, choose M here: the
>          module will be called touchit213.
>
> +config TOUCHSCREEN_TSC2003
> +       tristate "TSC2003 based touchscreens"
> +       depends on I2C
> +       help
> +         Say Y here if you have a TSC2003 based touchscreen.
> +
> +         If unsure, say N.
> +
> +         To compile this driver as a module, choose M here: the
> +         module will be called tsc2003.
> +
>  config TOUCHSCREEN_TSC2007
>        tristate "TSC2007 based touchscreens"
>        depends on I2C
> Index: linux-2.6.30-rc7/drivers/input/touchscreen/Makefile
> ===================================================================
> --- linux-2.6.30-rc7/drivers/input/touchscreen/Makefile (revision 861)
> +++ linux-2.6.30-rc7/drivers/input/touchscreen/Makefile (revision 869)
> @@ -27,6 +27,7 @@
>  obj-$(CONFIG_TOUCHSCREEN_TOUCHIT213)   += touchit213.o
>  obj-$(CONFIG_TOUCHSCREEN_TOUCHRIGHT)   += touchright.o
>  obj-$(CONFIG_TOUCHSCREEN_TOUCHWIN)     += touchwin.o
> +obj-$(CONFIG_TOUCHSCREEN_TSC2003)      += tsc2003.o
>  obj-$(CONFIG_TOUCHSCREEN_TSC2007)      += tsc2007.o
>  obj-$(CONFIG_TOUCHSCREEN_UCB1400)      += ucb1400_ts.o
>  obj-$(CONFIG_TOUCHSCREEN_WACOM_W8001)  += wacom_w8001.o
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>



-- 
---Trilok Soni
http://triloksoni.wordpress.com
http://www.linkedin.com/in/triloksoni

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

* Re: [RESEND][PATCH] input: Added TSC2003
@ 2009-06-15 18:10   ` Trilok Soni
  0 siblings, 0 replies; 12+ messages in thread
From: Trilok Soni @ 2009-06-15 18:10 UTC (permalink / raw)
  To: Richard Röjfors
  Cc: linux-input, Linux Kernel Mailing List, Andrew Morton,
	linux-omap, Thierry Reding

Hi,

On Mon, Jun 15, 2009 at 8:39 PM, Richard
Röjfors<richard.rojfors.ext@mocean-labs.com> wrote:
> Supplied is a driver for the TSC2003. There is actually a driver for TSC2007 which can be used in some cases.
> The platform struct is reused from the TSC2007.
> There is a big difference in the implementation between the drivers, this one does not use HR timers.
> The TSC2007 driver performs synchronous I2C in the timer callback (SW IRQ context) which is bad when
> the I2C driver is interrupt driven.
>

Meaning? I think I2C transaction can sleep. I don't see (right now)
need for two drivers. TSC2003 id was added to TSC2007. If you think
TSC2007 doesn't provide enough support for it, then please add that in
TSC2007 driver itself. We don't need create another driver it seems.

I have added Thierry who can further comment on it, as he has access
to device with TSC2003.

>
> Signed-off-by: Richard Röjfors <richard.rojfors.ext@mocean-labs.com>
> ---
> Index: linux-2.6.30-rc7/drivers/input/touchscreen/tsc2003.c
> ===================================================================
> --- linux-2.6.30-rc7/drivers/input/touchscreen/tsc2003.c        (revision 0)
> +++ linux-2.6.30-rc7/drivers/input/touchscreen/tsc2003.c        (revision 869)
> @@ -0,0 +1,387 @@
> +/*
> + * tsc2003.c Driver for TI TSC2003 touch screen controller
> + * Copyright (c) 2009 Intel Corporation
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
> + */
> +
> +/* Supports:
> + * TI TSC2003
> + *
> + * Inspired by tsc2007, Copyright (c) 2008 MtekVision Co., Ltd.
> + */
> +#include <linux/module.h>
> +#include <linux/input.h>
> +#include <linux/interrupt.h>
> +#include <linux/i2c.h>
> +#include <linux/i2c/tsc2007.h>
> +#include <linux/kthread.h>
> +#include <linux/semaphore.h>
> +
> +#define TSC2003_DRIVER_NAME            "tsc2003"
> +
> +#define TS_POLL_PERIOD 20              /* ms delay between samples */
> +
> +#define TSC2003_MEASURE_TEMP0          (0x0 << 4)
> +#define TSC2003_MEASURE_AUX            (0x2 << 4)
> +#define TSC2003_MEASURE_TEMP1          (0x4 << 4)
> +#define TSC2003_ACTIVATE_XN            (0x8 << 4)
> +#define TSC2003_ACTIVATE_YN            (0x9 << 4)
> +#define TSC2003_ACTIVATE_YP_XN         (0xa << 4)
> +#define TSC2003_SETUP                  (0xb << 4)
> +#define TSC2003_MEASURE_X              (0xc << 4)
> +#define TSC2003_MEASURE_Y              (0xd << 4)
> +#define TSC2003_MEASURE_Z1             (0xe << 4)
> +#define TSC2003_MEASURE_Z2             (0xf << 4)
> +
> +#define TSC2003_POWER_OFF_IRQ_EN       (0x0 << 2)
> +#define TSC2003_ADC_ON_IRQ_DIS0                (0x1 << 2)
> +#define TSC2003_ADC_OFF_IRQ_EN         (0x2 << 2)
> +#define TSC2003_ADC_ON_IRQ_DIS1                (0x3 << 2)
> +
> +#define TSC2003_12BIT                  (0x0 << 1)
> +#define TSC2003_8BIT                   (0x1 << 1)
> +
> +#define        MAX_12BIT                       ((1 << 12) - 1)
> +
> +#define ADC_ON_12BIT   (TSC2003_12BIT | TSC2003_ADC_ON_IRQ_DIS0)
> +
> +#define READ_Y         (ADC_ON_12BIT | TSC2003_MEASURE_Y)
> +#define READ_Z1                (ADC_ON_12BIT | TSC2003_MEASURE_Z1)
> +#define READ_Z2                (ADC_ON_12BIT | TSC2003_MEASURE_Z2)
> +#define READ_X         (ADC_ON_12BIT | TSC2003_MEASURE_X)
> +#define PWRDOWN                (TSC2003_12BIT | TSC2003_POWER_OFF_IRQ_EN)
> +
> +struct ts_event {
> +       int     x;
> +       int     y;
> +       int     z1, z2;
> +};
> +
> +struct tsc2003 {
> +       struct input_dev        *input;
> +       char                    phys[32];
> +       struct task_struct      *task;
> +       struct ts_event         tc;
> +       struct completion       penirq_completion;
> +
> +       struct i2c_client       *client;
> +
> +       u16                     model;
> +       u16                     x_plate_ohms;
> +
> +       unsigned                pendown;
> +};
> +
> +static inline int tsc2003_xfer(struct tsc2003 *tsc, u8 cmd)
> +{
> +       s32 data;
> +       u16 val;
> +
> +       data = i2c_smbus_read_word_data(tsc->client, cmd);
> +       if (data < 0) {
> +               dev_err(&tsc->client->dev, "i2c io error: %d\n", data);
> +               return data;
> +       }
> +
> +       /* The protocol and raw data format from i2c interface:
> +        * S Addr Wr [A] Comm [A] S Addr Rd [A] [DataLow] A [DataHigh] NA P
> +        * Where DataLow has [D11-D4], DataHigh has [D3-D0 << 4 | Dummy 4bit].
> +        */
> +       val = swab16(data) >> 4;
> +
> +       dev_dbg(&tsc->client->dev, "data: 0x%x, val: 0x%x\n", data, val);
> +
> +       return val;
> +}
> +
> +static void tsc2003_send_event(void *tsc)
> +{
> +       struct tsc2003  *ts = tsc;
> +       struct input_dev *input = ts->input;
> +       u32             rt = 0;
> +       u16             x, y, z1, z2;
> +
> +       x = ts->tc.x;
> +       y = ts->tc.y;
> +       z1 = ts->tc.z1;
> +       z2 = ts->tc.z2;
> +
> +       /* range filtering */
> +       if (x == MAX_12BIT)
> +               x = 0;
> +
> +       if (likely(x && z1)) {
> +               /* compute touch pressure resistance using equation #1 */
> +               rt = z2;
> +               rt -= z1;
> +               rt *= x;
> +               rt *= ts->x_plate_ohms;
> +               rt /= z1;
> +               rt = (rt + 2047) >> 12;
> +       }
> +
> +       /* Sample found inconsistent by debouncing or pressure is beyond
> +        * the maximum. Don't report it to user space, repeat at least
> +        * once more the measurement
> +        */
> +       if (rt > MAX_12BIT)
> +               return;
> +
> +       /* NOTE: We can't rely on the pressure to determine the pen down
> +        * state, even this controller has a pressure sensor.  The pressure
> +        * value can fluctuate for quite a while after lifting the pen and
> +        * in some cases may not even settle at the expected value.
> +        *
> +        * The only safe way to check for the pen up condition is in the
> +        * timer by reading the pen signal state (it's a GPIO _and_ IRQ).
> +        */
> +       if (rt) {
> +               if (!ts->pendown) {
> +                       dev_dbg(&ts->client->dev, "DOWN\n");
> +
> +                       input_report_key(input, BTN_TOUCH, 1);
> +                       ts->pendown = 1;
> +               }
> +
> +               input_report_abs(input, ABS_X, x);
> +               input_report_abs(input, ABS_Y, y);
> +               input_report_abs(input, ABS_PRESSURE, rt);
> +
> +               input_sync(input);
> +
> +               dev_dbg(&ts->client->dev, "point(%4d,%4d), pressure (%4u)\n",
> +                       x, y, rt);
> +       } else if (ts->pendown) {
> +               /* pen up */
> +               dev_dbg(&ts->client->dev, "UP\n");
> +               input_report_key(input, BTN_TOUCH, 0);
> +               input_report_abs(input, ABS_PRESSURE, 0);
> +               input_sync(input);
> +
> +               ts->pendown = 0;
> +       }
> +}
> +
> +static int tsc2003_power_off_irq_en(struct tsc2003 *tsc)
> +{
> +       /* power down */
> +       return tsc2003_xfer(tsc, PWRDOWN);
> +}
> +
> +static int tsc2003_read_values(struct tsc2003 *tsc)
> +{
> +       /* y- still on; turn on only y+ (and ADC) */
> +       tsc->tc.y = tsc2003_xfer(tsc, READ_Y);
> +       if (tsc->tc.y < 0)
> +               return tsc->tc.y;
> +
> +       /* turn y- off, x+ on, then leave in lowpower */
> +       tsc->tc.x = tsc2003_xfer(tsc, READ_X);
> +       if (tsc->tc.x < 0)
> +               return tsc->tc.x;
> +
> +       /* turn y+ off, x- on; we'll use formula #1 */
> +       tsc->tc.z1 = tsc2003_xfer(tsc, READ_Z1);
> +       if (tsc->tc.z1 < 0)
> +               return tsc->tc.z1;
> +
> +       tsc->tc.z2 = tsc2003_xfer(tsc, READ_Z2);
> +       if (tsc->tc.z2 < 0)
> +               return tsc->tc.z2;
> +
> +       return 0;
> +}
> +
> +
> +static irqreturn_t tsc2003_irq(int irq, void *handle)
> +{
> +       struct tsc2003 *ts = handle;
> +
> +       /* do not call the synced version -> deadlock */
> +       disable_irq_nosync(irq);
> +       /* signal the thread to continue */
> +       complete(&ts->penirq_completion);
> +
> +       return IRQ_HANDLED;
> +}
> +
> +static int tsc2003_thread(void *d)
> +{
> +       struct tsc2003 *ts = (struct tsc2003 *)d;
> +       int ret;
> +
> +       allow_signal(SIGKILL);
> +
> +       while (!signal_pending(current)) {
> +               /* power down and wait for interrupt */
> +               do {
> +                       /* loop because the I2C bus might be busy */
> +                       ret = msleep_interruptible(TS_POLL_PERIOD);
> +                       if (!ret)
> +                               ret = tsc2003_power_off_irq_en(ts);
> +               } while (ret == -EAGAIN && !signal_pending(current));
> +
> +               if (signal_pending(current))
> +                       break;
> +
> +               ret = wait_for_completion_interruptible(&ts->penirq_completion);
> +               if (!ret) {
> +                       int first = 1;
> +                       /* got IRQ, start poll, until pen is up */
> +                       while (!ret && !signal_pending(current)
> +                               && (first || ts->pendown)) {
> +                               ret = tsc2003_read_values(ts);
> +                               if (!ret)
> +                                       tsc2003_send_event(ts);
> +                               ret = msleep_interruptible(TS_POLL_PERIOD);
> +                               first = 0;
> +                       }
> +
> +                       /* we re enable the interrupt */
> +                       if (!signal_pending(current))
> +                               enable_irq(ts->client->irq);
> +               }
> +       }
> +
> +       return 0;
> +}
> +
> +static int tsc2003_probe(struct i2c_client *client,
> +                       const struct i2c_device_id *id)
> +{
> +       struct tsc2003 *ts;
> +       struct tsc2007_platform_data *pdata = client->dev.platform_data;
> +       struct input_dev *input_dev;
> +       int err;
> +
> +       if (!pdata) {
> +               dev_err(&client->dev, "platform data is required!\n");
> +               return -EINVAL;
> +       }
> +
> +       if (!i2c_check_functionality(client->adapter,
> +                                    I2C_FUNC_SMBUS_READ_WORD_DATA))
> +               return -EIO;
> +
> +       ts = kzalloc(sizeof(struct tsc2003), GFP_KERNEL);
> +       input_dev = input_allocate_device();
> +       if (!ts || !input_dev) {
> +               err = -ENOMEM;
> +               goto err_free_mem;
> +       }
> +
> +       ts->client = client;
> +       i2c_set_clientdata(client, ts);
> +
> +       ts->input = input_dev;
> +
> +       ts->model             = pdata->model;
> +       ts->x_plate_ohms      = pdata->x_plate_ohms;
> +
> +       snprintf(ts->phys, sizeof(ts->phys),
> +                "%s/input0", dev_name(&client->dev));
> +
> +       input_dev->name = TSC2003_DRIVER_NAME" Touchscreen";
> +       input_dev->phys = ts->phys;
> +       input_dev->id.bustype = BUS_I2C;
> +
> +       input_dev->evbit[0] = BIT_MASK(EV_KEY) | BIT_MASK(EV_ABS);
> +       input_dev->keybit[BIT_WORD(BTN_TOUCH)] = BIT_MASK(BTN_TOUCH);
> +
> +       input_set_abs_params(input_dev, ABS_X, 0, MAX_12BIT, 0, 0);
> +       input_set_abs_params(input_dev, ABS_Y, 0, MAX_12BIT, 0, 0);
> +       input_set_abs_params(input_dev, ABS_PRESSURE, 0, MAX_12BIT, 0, 0);
> +
> +       init_completion(&ts->penirq_completion);
> +
> +       ts->task = kthread_run(tsc2003_thread, ts, TSC2003_DRIVER_NAME);
> +       if (IS_ERR(ts->task)) {
> +               err = PTR_ERR(ts->task);
> +               goto err_free_mem;
> +       }
> +
> +       err = request_irq(client->irq, tsc2003_irq, 0,
> +                       client->dev.driver->name, ts);
> +       if (err < 0) {
> +               dev_err(&client->dev, "irq %d busy?\n", client->irq);
> +               goto err_free_thread;
> +       }
> +
> +       err = input_register_device(input_dev);
> +       if (err)
> +               goto err_free_irq;
> +
> +       dev_info(&client->dev, "registered with irq (%d)\n", client->irq);
> +
> +       return 0;
> +
> + err_free_irq:
> +       free_irq(client->irq, ts);
> + err_free_thread:
> +       kthread_stop(ts->task);
> + err_free_mem:
> +       input_free_device(input_dev);
> +       kfree(ts);
> +       return err;
> +}
> +
> +static int tsc2003_remove(struct i2c_client *client)
> +{
> +       struct tsc2003 *ts = i2c_get_clientdata(client);
> +
> +       free_irq(client->irq, ts);
> +       send_sig(SIGKILL, ts->task, 1);
> +       kthread_stop(ts->task);
> +       input_unregister_device(ts->input);
> +       kfree(ts);
> +
> +       return 0;
> +}
> +
> +static struct i2c_device_id tsc2003_idtable[] = {
> +       { TSC2003_DRIVER_NAME, 0 },
> +       { }
> +};
> +
> +MODULE_DEVICE_TABLE(i2c, tsc2003_idtable);
> +
> +static struct i2c_driver tsc2003_driver = {
> +       .driver = {
> +               .owner  = THIS_MODULE,
> +               .name   = TSC2003_DRIVER_NAME,
> +               .bus = &i2c_bus_type,
> +       },
> +       .id_table       = tsc2003_idtable,
> +       .probe          = tsc2003_probe,
> +       .remove         = tsc2003_remove,
> +};
> +
> +static int __init tsc2003_init(void)
> +{
> +       return i2c_add_driver(&tsc2003_driver);
> +}
> +
> +static void __exit tsc2003_exit(void)
> +{
> +       i2c_del_driver(&tsc2003_driver);
> +}
> +
> +module_init(tsc2003_init);
> +module_exit(tsc2003_exit);
> +
> +MODULE_AUTHOR("Mocean Laboratories <info@mocean-labs.com>");
> +MODULE_DESCRIPTION("TSC2003 TouchScreen Driver");
> +MODULE_LICENSE("GPL v2");
> +
> Index: linux-2.6.30-rc7/drivers/input/touchscreen/Kconfig
> ===================================================================
> --- linux-2.6.30-rc7/drivers/input/touchscreen/Kconfig  (revision 861)
> +++ linux-2.6.30-rc7/drivers/input/touchscreen/Kconfig  (revision 869)
> @@ -455,6 +455,17 @@
>          To compile this driver as a module, choose M here: the
>          module will be called touchit213.
>
> +config TOUCHSCREEN_TSC2003
> +       tristate "TSC2003 based touchscreens"
> +       depends on I2C
> +       help
> +         Say Y here if you have a TSC2003 based touchscreen.
> +
> +         If unsure, say N.
> +
> +         To compile this driver as a module, choose M here: the
> +         module will be called tsc2003.
> +
>  config TOUCHSCREEN_TSC2007
>        tristate "TSC2007 based touchscreens"
>        depends on I2C
> Index: linux-2.6.30-rc7/drivers/input/touchscreen/Makefile
> ===================================================================
> --- linux-2.6.30-rc7/drivers/input/touchscreen/Makefile (revision 861)
> +++ linux-2.6.30-rc7/drivers/input/touchscreen/Makefile (revision 869)
> @@ -27,6 +27,7 @@
>  obj-$(CONFIG_TOUCHSCREEN_TOUCHIT213)   += touchit213.o
>  obj-$(CONFIG_TOUCHSCREEN_TOUCHRIGHT)   += touchright.o
>  obj-$(CONFIG_TOUCHSCREEN_TOUCHWIN)     += touchwin.o
> +obj-$(CONFIG_TOUCHSCREEN_TSC2003)      += tsc2003.o
>  obj-$(CONFIG_TOUCHSCREEN_TSC2007)      += tsc2007.o
>  obj-$(CONFIG_TOUCHSCREEN_UCB1400)      += ucb1400_ts.o
>  obj-$(CONFIG_TOUCHSCREEN_WACOM_W8001)  += wacom_w8001.o
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>



-- 
---Trilok Soni
http://triloksoni.wordpress.com
http://www.linkedin.com/in/triloksoni
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [RESEND][PATCH] input: Added TSC2003
  2009-06-15 18:10   ` Trilok Soni
@ 2009-06-15 19:32     ` Richard Röjfors
  -1 siblings, 0 replies; 12+ messages in thread
From: Richard Röjfors @ 2009-06-15 19:32 UTC (permalink / raw)
  To: Trilok Soni
  Cc: linux-input, Linux Kernel Mailing List, Andrew Morton,
	linux-omap, Thierry Reding

On 09-06-15 20.10, Trilok Soni wrote:
> Hi,
> 
> On Mon, Jun 15, 2009 at 8:39 PM, Richard
> Röjfors<richard.rojfors.ext@mocean-labs.com> wrote:
>> Supplied is a driver for the TSC2003. There is actually a driver for TSC2007 which can be used in some cases.
>> The platform struct is reused from the TSC2007.
>> There is a big difference in the implementation between the drivers, this one does not use HR timers.
>> The TSC2007 driver performs synchronous I2C in the timer callback (SW IRQ context) which is bad when
>> the I2C driver is interrupt driven.
>>
> 
> Meaning? I think I2C transaction can sleep.

Yes that's what it means, and that's bad in a HR timer callback.

> I don't see (right now)
> need for two drivers. TSC2003 id was added to TSC2007. If you think
> TSC2007 doesn't provide enough support for it, then please add that in
> TSC2007 driver itself. We don't need create another driver it seems.

The chips are actually compatible (when it comes to the touchscreen functionality).

So you are true, the tsc2007 can be modified, I didn't added support for
the other pins anyway.

Actually I don't have access to any tsc2007, so I can't verify it on the newer chip.
I will go on and propose a patch.

--Richard

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

* Re: [RESEND][PATCH] input: Added TSC2003
@ 2009-06-15 19:32     ` Richard Röjfors
  0 siblings, 0 replies; 12+ messages in thread
From: Richard Röjfors @ 2009-06-15 19:32 UTC (permalink / raw)
  To: Trilok Soni
  Cc: linux-input, Linux Kernel Mailing List, Andrew Morton,
	linux-omap, Thierry Reding

On 09-06-15 20.10, Trilok Soni wrote:
> Hi,
> 
> On Mon, Jun 15, 2009 at 8:39 PM, Richard
> Röjfors<richard.rojfors.ext@mocean-labs.com> wrote:
>> Supplied is a driver for the TSC2003. There is actually a driver for TSC2007 which can be used in some cases.
>> The platform struct is reused from the TSC2007.
>> There is a big difference in the implementation between the drivers, this one does not use HR timers.
>> The TSC2007 driver performs synchronous I2C in the timer callback (SW IRQ context) which is bad when
>> the I2C driver is interrupt driven.
>>
> 
> Meaning? I think I2C transaction can sleep.

Yes that's what it means, and that's bad in a HR timer callback.

> I don't see (right now)
> need for two drivers. TSC2003 id was added to TSC2007. If you think
> TSC2007 doesn't provide enough support for it, then please add that in
> TSC2007 driver itself. We don't need create another driver it seems.

The chips are actually compatible (when it comes to the touchscreen functionality).

So you are true, the tsc2007 can be modified, I didn't added support for
the other pins anyway.

Actually I don't have access to any tsc2007, so I can't verify it on the newer chip.
I will go on and propose a patch.

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

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

* Re: [RESEND][PATCH] input: Added TSC2003
  2009-06-15 19:32     ` Richard Röjfors
@ 2009-06-16  6:20       ` Thierry Reding
  -1 siblings, 0 replies; 12+ messages in thread
From: Thierry Reding @ 2009-06-16  6:20 UTC (permalink / raw)
  To: Richard Röjfors
  Cc: Trilok Soni, linux-input, Linux Kernel Mailing List,
	Andrew Morton, linux-omap

* Richard Röjfors wrote:
> On 09-06-15 20.10, Trilok Soni wrote:
> > Hi,
> > 
> > On Mon, Jun 15, 2009 at 8:39 PM, Richard
> > Röjfors<richard.rojfors.ext@mocean-labs.com> wrote:
> >> Supplied is a driver for the TSC2003. There is actually a driver for TSC2007 which can be used in some cases.
> >> The platform struct is reused from the TSC2007.
> >> There is a big difference in the implementation between the drivers, this one does not use HR timers.
> >> The TSC2007 driver performs synchronous I2C in the timer callback (SW IRQ context) which is bad when
> >> the I2C driver is interrupt driven.
> >>
> > 
> > Meaning? I think I2C transaction can sleep.
> 
> Yes that's what it means, and that's bad in a HR timer callback.

Note that my patch to the tsc2007 to support the tsc2003 exactly fixes this
problem. It moves the actual I2C transfers into a workqueue, so no sleeping
functions are called from the hrtimer callback.

Thierry

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

* Re: [RESEND][PATCH] input: Added TSC2003
@ 2009-06-16  6:20       ` Thierry Reding
  0 siblings, 0 replies; 12+ messages in thread
From: Thierry Reding @ 2009-06-16  6:20 UTC (permalink / raw)
  To: Richard Röjfors
  Cc: Trilok Soni, linux-input, Linux Kernel Mailing List,
	Andrew Morton, linux-omap

* Richard Röjfors wrote:
> On 09-06-15 20.10, Trilok Soni wrote:
> > Hi,
> > 
> > On Mon, Jun 15, 2009 at 8:39 PM, Richard
> > Röjfors<richard.rojfors.ext@mocean-labs.com> wrote:
> >> Supplied is a driver for the TSC2003. There is actually a driver for TSC2007 which can be used in some cases.
> >> The platform struct is reused from the TSC2007.
> >> There is a big difference in the implementation between the drivers, this one does not use HR timers.
> >> The TSC2007 driver performs synchronous I2C in the timer callback (SW IRQ context) which is bad when
> >> the I2C driver is interrupt driven.
> >>
> > 
> > Meaning? I think I2C transaction can sleep.
> 
> Yes that's what it means, and that's bad in a HR timer callback.

Note that my patch to the tsc2007 to support the tsc2003 exactly fixes this
problem. It moves the actual I2C transfers into a workqueue, so no sleeping
functions are called from the hrtimer callback.

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

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

* Re: [RESEND][PATCH] input: Added TSC2003
  2009-06-16  6:20       ` Thierry Reding
  (?)
@ 2009-06-16  7:40       ` Richard Röjfors
  2009-06-16  8:16           ` Thierry Reding
  -1 siblings, 1 reply; 12+ messages in thread
From: Richard Röjfors @ 2009-06-16  7:40 UTC (permalink / raw)
  To: Thierry Reding
  Cc: Trilok Soni, linux-input, Linux Kernel Mailing List,
	Andrew Morton, linux-omap

On 09-06-16 08.20, Thierry Reding wrote:
>>>>
>>> Meaning? I think I2C transaction can sleep.
>> Yes that's what it means, and that's bad in a HR timer callback.
> 
> Note that my patch to the tsc2007 to support the tsc2003 exactly fixes this
> problem. It moves the actual I2C transfers into a workqueue, so no sleeping
> functions are called from the hrtimer callback.

Ah good! The IRQ is disabled no sync too.
We actually don't have the possibility to implement a pen down state callback,
so I need to modify the code slightly to work even without one. (but not be
as accurate when the callback is not available)

Your patch where it schedules work rather than calling the I2C function directly
isn't in mainline.
I saw a patch where you added the work scheduling, and a later patch where you
fixed some spinlock stuff, have you resent the patch for the work scheduling?

--Richard

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

* Re: [RESEND][PATCH] input: Added TSC2003
  2009-06-16  7:40       ` Richard Röjfors
@ 2009-06-16  8:16           ` Thierry Reding
  0 siblings, 0 replies; 12+ messages in thread
From: Thierry Reding @ 2009-06-16  8:16 UTC (permalink / raw)
  To: Richard Röjfors
  Cc: Trilok Soni, linux-input, Linux Kernel Mailing List,
	Andrew Morton, linux-omap, Kwangwoo Lee

[Cc'ing Kwangwoo Lee]

* Richard Röjfors wrote:
> On 09-06-16 08.20, Thierry Reding wrote:
> >>>>
> >>> Meaning? I think I2C transaction can sleep.
> >> Yes that's what it means, and that's bad in a HR timer callback.
> > 
> > Note that my patch to the tsc2007 to support the tsc2003 exactly fixes this
> > problem. It moves the actual I2C transfers into a workqueue, so no sleeping
> > functions are called from the hrtimer callback.
> 
> Ah good! The IRQ is disabled no sync too.
> We actually don't have the possibility to implement a pen down state callback,
> so I need to modify the code slightly to work even without one. (but not be
> as accurate when the callback is not available)
> 
> Your patch where it schedules work rather than calling the I2C function directly
> isn't in mainline.
> I saw a patch where you added the work scheduling, and a later patch where you
> fixed some spinlock stuff, have you resent the patch for the work scheduling?

I think there were still some issues with that patch. Kwangwoo Lee was the
last to comment. This is from the previous thread:

> On Tue, May 12, 2009 at 12:41 AM, Dmitry Torokhov
> <dmitry.torokhov@gmail.com> wrote:
> > On Mon, May 11, 2009 at 08:38:09AM -0700, Dmitry Torokhov wrote:
> >> Hi,
> >> On Mon, May 11, 2009 at 08:44:00PM +0900, Kwangwoo Lee wrote:
> >> > From d5de0d22109de7564f9bf1df688acbe6b18f41db Mon Sep 17 00:00:00 2001
> >> > From: Kwangwoo Lee <kwangwoo.lee@gmail.com>
> >> > Date: Mon, 11 May 2009 20:05:50 +0900
> >> > Subject: [PATCH 2/2] Input: tsc2007: do I2C transfers in non-interrupt
> >> > context.
> >> >
> >> > This patch enhances pointer movements much smoother.
> >> > The original patch is written by Thierry.
> >> >
> >> > --- a/drivers/input/touchscreen/tsc2007.c
> >> > +++ b/drivers/input/touchscreen/tsc2007.c
> >> > @@ -70,6 +70,7 @@ struct ts_event {
> >> >  struct tsc2007 {
> >> >     struct input_dev        *input;
> >> >     char                    phys[32];
> >> > +   struct work_struct      work;
> >>
> >> Every time I see a work_struct in a driver and don't see
> >> cancel_work_sync() anywhere I know there are issues...
> >>
> 
> Thanks for your comment. I also missed that thing, sorry.
> 
> > Also, why do we need to chain irq->timer->work now? Surely we can bypass
> > the timer if we have to read in process context.
> 
> It's good point. I'll check again.
> Thanks.

So perhaps some more work is required to get this mainlined.

Thierry

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

* Re: [RESEND][PATCH] input: Added TSC2003
@ 2009-06-16  8:16           ` Thierry Reding
  0 siblings, 0 replies; 12+ messages in thread
From: Thierry Reding @ 2009-06-16  8:16 UTC (permalink / raw)
  To: Richard Röjfors
  Cc: Trilok Soni, linux-input, Linux Kernel Mailing List,
	Andrew Morton, linux-omap, Kwangwoo Lee

[Cc'ing Kwangwoo Lee]

* Richard Röjfors wrote:
> On 09-06-16 08.20, Thierry Reding wrote:
> >>>>
> >>> Meaning? I think I2C transaction can sleep.
> >> Yes that's what it means, and that's bad in a HR timer callback.
> > 
> > Note that my patch to the tsc2007 to support the tsc2003 exactly fixes this
> > problem. It moves the actual I2C transfers into a workqueue, so no sleeping
> > functions are called from the hrtimer callback.
> 
> Ah good! The IRQ is disabled no sync too.
> We actually don't have the possibility to implement a pen down state callback,
> so I need to modify the code slightly to work even without one. (but not be
> as accurate when the callback is not available)
> 
> Your patch where it schedules work rather than calling the I2C function directly
> isn't in mainline.
> I saw a patch where you added the work scheduling, and a later patch where you
> fixed some spinlock stuff, have you resent the patch for the work scheduling?

I think there were still some issues with that patch. Kwangwoo Lee was the
last to comment. This is from the previous thread:

> On Tue, May 12, 2009 at 12:41 AM, Dmitry Torokhov
> <dmitry.torokhov@gmail.com> wrote:
> > On Mon, May 11, 2009 at 08:38:09AM -0700, Dmitry Torokhov wrote:
> >> Hi,
> >> On Mon, May 11, 2009 at 08:44:00PM +0900, Kwangwoo Lee wrote:
> >> > From d5de0d22109de7564f9bf1df688acbe6b18f41db Mon Sep 17 00:00:00 2001
> >> > From: Kwangwoo Lee <kwangwoo.lee@gmail.com>
> >> > Date: Mon, 11 May 2009 20:05:50 +0900
> >> > Subject: [PATCH 2/2] Input: tsc2007: do I2C transfers in non-interrupt
> >> > context.
> >> >
> >> > This patch enhances pointer movements much smoother.
> >> > The original patch is written by Thierry.
> >> >
> >> > --- a/drivers/input/touchscreen/tsc2007.c
> >> > +++ b/drivers/input/touchscreen/tsc2007.c
> >> > @@ -70,6 +70,7 @@ struct ts_event {
> >> >  struct tsc2007 {
> >> >     struct input_dev        *input;
> >> >     char                    phys[32];
> >> > +   struct work_struct      work;
> >>
> >> Every time I see a work_struct in a driver and don't see
> >> cancel_work_sync() anywhere I know there are issues...
> >>
> 
> Thanks for your comment. I also missed that thing, sorry.
> 
> > Also, why do we need to chain irq->timer->work now? Surely we can bypass
> > the timer if we have to read in process context.
> 
> It's good point. I'll check again.
> Thanks.

So perhaps some more work is required to get this mainlined.

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

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

* Re: [RESEND][PATCH] input: Added TSC2003
  2009-06-16  8:16           ` Thierry Reding
@ 2009-06-19  0:50             ` Kwangwoo Lee
  -1 siblings, 0 replies; 12+ messages in thread
From: Kwangwoo Lee @ 2009-06-19  0:50 UTC (permalink / raw)
  To: Thierry Reding
  Cc: Richard Röjfors, Trilok Soni, linux-input,
	Linux Kernel Mailing List, Andrew Morton, linux-omap

Hi,

On Tue, Jun 16, 2009 at 5:16 PM, Thierry
Reding<thierry.reding@avionic-design.de> wrote:
> [Cc'ing Kwangwoo Lee]
>
> * Richard Röjfors wrote:
>> On 09-06-16 08.20, Thierry Reding wrote:
>> >>>>
>> >>> Meaning? I think I2C transaction can sleep.
>> >> Yes that's what it means, and that's bad in a HR timer callback.
>> >
>> > Note that my patch to the tsc2007 to support the tsc2003 exactly fixes this
>> > problem. It moves the actual I2C transfers into a workqueue, so no sleeping
>> > functions are called from the hrtimer callback.
>>
>> Ah good! The IRQ is disabled no sync too.
>> We actually don't have the possibility to implement a pen down state callback,
>> so I need to modify the code slightly to work even without one. (but not be
>> as accurate when the callback is not available)
>>
>> Your patch where it schedules work rather than calling the I2C function directly
>> isn't in mainline.
>> I saw a patch where you added the work scheduling, and a later patch where you
>> fixed some spinlock stuff, have you resent the patch for the work scheduling?
>
> I think there were still some issues with that patch. Kwangwoo Lee was the
> last to comment. This is from the previous thread:
>
>> On Tue, May 12, 2009 at 12:41 AM, Dmitry Torokhov
>> <dmitry.torokhov@gmail.com> wrote:
>> > On Mon, May 11, 2009 at 08:38:09AM -0700, Dmitry Torokhov wrote:
>> >> Hi,
>> >> On Mon, May 11, 2009 at 08:44:00PM +0900, Kwangwoo Lee wrote:
>> >> > From d5de0d22109de7564f9bf1df688acbe6b18f41db Mon Sep 17 00:00:00 2001
>> >> > From: Kwangwoo Lee <kwangwoo.lee@gmail.com>
>> >> > Date: Mon, 11 May 2009 20:05:50 +0900
>> >> > Subject: [PATCH 2/2] Input: tsc2007: do I2C transfers in non-interrupt
>> >> > context.
>> >> >
>> >> > This patch enhances pointer movements much smoother.
>> >> > The original patch is written by Thierry.
>> >> >
>> >> > --- a/drivers/input/touchscreen/tsc2007.c
>> >> > +++ b/drivers/input/touchscreen/tsc2007.c
>> >> > @@ -70,6 +70,7 @@ struct ts_event {
>> >> >  struct tsc2007 {
>> >> >     struct input_dev        *input;
>> >> >     char                    phys[32];
>> >> > +   struct work_struct      work;
>> >>
>> >> Every time I see a work_struct in a driver and don't see
>> >> cancel_work_sync() anywhere I know there are issues...
>> >>
>>
>> Thanks for your comment. I also missed that thing, sorry.
>>
>> > Also, why do we need to chain irq->timer->work now? Surely we can bypass
>> > the timer if we have to read in process context.
>>
>> It's good point. I'll check again.
>> Thanks.
>
> So perhaps some more work is required to get this mainlined.

Sorry for my late reply.

I think that the driver need to be fixed to send i2c command
asynchronously (if possible)
or replace hrtimer with delayed workqueue as Dmitry pointed out above.

Hmm.. I'm going to move to the other company, so I do not have the
testing environment anymore.
I can see the emails from the list, but can anyone else follow up the
driver, please?

Thanks.

-- 
Kwangwoo Lee <kwangwoo.lee@gmail.com>

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

* Re: [RESEND][PATCH] input: Added TSC2003
@ 2009-06-19  0:50             ` Kwangwoo Lee
  0 siblings, 0 replies; 12+ messages in thread
From: Kwangwoo Lee @ 2009-06-19  0:50 UTC (permalink / raw)
  To: Thierry Reding
  Cc: Richard Röjfors, Trilok Soni, linux-input,
	Linux Kernel Mailing List, Andrew Morton, linux-omap

Hi,

On Tue, Jun 16, 2009 at 5:16 PM, Thierry
Reding<thierry.reding@avionic-design.de> wrote:
> [Cc'ing Kwangwoo Lee]
>
> * Richard Röjfors wrote:
>> On 09-06-16 08.20, Thierry Reding wrote:
>> >>>>
>> >>> Meaning? I think I2C transaction can sleep.
>> >> Yes that's what it means, and that's bad in a HR timer callback.
>> >
>> > Note that my patch to the tsc2007 to support the tsc2003 exactly fixes this
>> > problem. It moves the actual I2C transfers into a workqueue, so no sleeping
>> > functions are called from the hrtimer callback.
>>
>> Ah good! The IRQ is disabled no sync too.
>> We actually don't have the possibility to implement a pen down state callback,
>> so I need to modify the code slightly to work even without one. (but not be
>> as accurate when the callback is not available)
>>
>> Your patch where it schedules work rather than calling the I2C function directly
>> isn't in mainline.
>> I saw a patch where you added the work scheduling, and a later patch where you
>> fixed some spinlock stuff, have you resent the patch for the work scheduling?
>
> I think there were still some issues with that patch. Kwangwoo Lee was the
> last to comment. This is from the previous thread:
>
>> On Tue, May 12, 2009 at 12:41 AM, Dmitry Torokhov
>> <dmitry.torokhov@gmail.com> wrote:
>> > On Mon, May 11, 2009 at 08:38:09AM -0700, Dmitry Torokhov wrote:
>> >> Hi,
>> >> On Mon, May 11, 2009 at 08:44:00PM +0900, Kwangwoo Lee wrote:
>> >> > From d5de0d22109de7564f9bf1df688acbe6b18f41db Mon Sep 17 00:00:00 2001
>> >> > From: Kwangwoo Lee <kwangwoo.lee@gmail.com>
>> >> > Date: Mon, 11 May 2009 20:05:50 +0900
>> >> > Subject: [PATCH 2/2] Input: tsc2007: do I2C transfers in non-interrupt
>> >> > context.
>> >> >
>> >> > This patch enhances pointer movements much smoother.
>> >> > The original patch is written by Thierry.
>> >> >
>> >> > --- a/drivers/input/touchscreen/tsc2007.c
>> >> > +++ b/drivers/input/touchscreen/tsc2007.c
>> >> > @@ -70,6 +70,7 @@ struct ts_event {
>> >> >  struct tsc2007 {
>> >> >     struct input_dev        *input;
>> >> >     char                    phys[32];
>> >> > +   struct work_struct      work;
>> >>
>> >> Every time I see a work_struct in a driver and don't see
>> >> cancel_work_sync() anywhere I know there are issues...
>> >>
>>
>> Thanks for your comment. I also missed that thing, sorry.
>>
>> > Also, why do we need to chain irq->timer->work now? Surely we can bypass
>> > the timer if we have to read in process context.
>>
>> It's good point. I'll check again.
>> Thanks.
>
> So perhaps some more work is required to get this mainlined.

Sorry for my late reply.

I think that the driver need to be fixed to send i2c command
asynchronously (if possible)
or replace hrtimer with delayed workqueue as Dmitry pointed out above.

Hmm.. I'm going to move to the other company, so I do not have the
testing environment anymore.
I can see the emails from the list, but can anyone else follow up the
driver, please?

Thanks.

-- 
Kwangwoo Lee <kwangwoo.lee@gmail.com>
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

end of thread, other threads:[~2009-06-19  0:50 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-06-15 15:09 [RESEND][PATCH] input: Added TSC2003 Richard Röjfors
2009-06-15 18:10 ` Trilok Soni
2009-06-15 18:10   ` Trilok Soni
2009-06-15 19:32   ` Richard Röjfors
2009-06-15 19:32     ` Richard Röjfors
2009-06-16  6:20     ` Thierry Reding
2009-06-16  6:20       ` Thierry Reding
2009-06-16  7:40       ` Richard Röjfors
2009-06-16  8:16         ` Thierry Reding
2009-06-16  8:16           ` Thierry Reding
2009-06-19  0:50           ` Kwangwoo Lee
2009-06-19  0:50             ` Kwangwoo Lee

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