public inbox for linux-sh@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] Touch screen driver for the SuperH MigoR board
@ 2008-03-21 11:16 Magnus Damm
  2008-03-21 22:55 ` Andrew Morton
                   ` (2 more replies)
  0 siblings, 3 replies; 13+ messages in thread
From: Magnus Damm @ 2008-03-21 11:16 UTC (permalink / raw)
  To: linux-input; +Cc: Magnus Damm, lethal, dmitry.torokhov, akpm, linux-sh

This patch adds a touch screen driver for the MigoR board. The chip we
interface to is unfortunately a custom designed microcontroller speaking
some undocumented protocol over i2c.

The board specific code is expected to register this device as an i2c
chip using struct i2c_board_info [] and i2c_register_board_info().

Signed-off-by: Magnus Damm <damm@igel.co.jp>
---

 drivers/input/touchscreen/Kconfig    |   12 ++
 drivers/input/touchscreen/Makefile   |    1 
 drivers/input/touchscreen/migor_ts.c |  196 ++++++++++++++++++++++++++++++++++
 3 files changed, 209 insertions(+)

--- 0001/drivers/input/touchscreen/Kconfig
+++ work/drivers/input/touchscreen/Kconfig	2008-03-21 18:57:02.000000000 +0900
@@ -134,6 +134,18 @@ config TOUCHSCREEN_HP7XX
 	  To compile this driver as a module, choose M here: the
 	  module will be called jornada720_ts.
 
+config TOUCHSCREEN_MIGOR
+	tristate "Renesas MIGO-R touchscreen"
+	depends on SH_MIGOR
+	default n
+	help
+	  Say Y here to enable MIGO-R touchscreen support.
+
+	  If unsure, say N.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called migor_ts.
+
 config TOUCHSCREEN_PENMOUNT
 	tristate "Penmount serial touchscreen"
 	select SERIO
--- 0001/drivers/input/touchscreen/Makefile
+++ work/drivers/input/touchscreen/Makefile	2008-03-21 18:57:02.000000000 +0900
@@ -19,3 +19,4 @@ obj-$(CONFIG_TOUCHSCREEN_PENMOUNT)	+= pe
 obj-$(CONFIG_TOUCHSCREEN_TOUCHRIGHT)	+= touchright.o
 obj-$(CONFIG_TOUCHSCREEN_TOUCHWIN)	+= touchwin.o
 obj-$(CONFIG_TOUCHSCREEN_UCB1400)	+= ucb1400_ts.o
+obj-$(CONFIG_TOUCHSCREEN_MIGOR)		+= migor_ts.o
--- /dev/null
+++ work/drivers/input/touchscreen/migor_ts.c	2008-03-21 19:03:05.000000000 +0900
@@ -0,0 +1,196 @@
+/*
+ * Touch Screen driver for Renesas MIGO-R Platform
+ *
+ * Copyright (c) 2008 Magnus Damm
+ * Copyright (c) 2007 Ujjwal Pande <ujjwal@kenati.com>,
+ *  Kenati Technologies Pvt Ltd.
+ *
+ * This file 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 2 of the License, or (at your option) any later version.
+ *
+ * This file 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 library; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ */
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/input.h>
+#include <linux/interrupt.h>
+#include <asm/io.h>
+#include <linux/i2c.h>
+#include <linux/timer.h>
+
+#define EVENT_PENDOWN 1
+#define EVENT_REPEAT  2
+#define EVENT_PENUP   3
+
+struct migor_ts_priv {
+	struct i2c_client *client;
+	struct input_dev *input;
+	struct delayed_work work;
+	int irq;
+};
+
+static const u_int8_t migor_ts_ena_seq[17] = { 0x33, 0x22, 0x11,
+					       0x01, 0x06, 0x07, };
+static const u_int8_t migor_ts_dis_seq[17] = { };
+
+static void migor_ts_poscheck(struct work_struct *work)
+{
+	struct migor_ts_priv *priv = container_of(work,
+						  struct migor_ts_priv,
+						  work.work);
+	unsigned short xpos, ypos;
+	unsigned char event;
+	u_int8_t buf[16];
+
+	memset(buf, 0, sizeof(buf));
+
+	/* Set Index 0 */
+	buf[0] = 0;
+	if (i2c_master_send(priv->client, buf, 1) != 1) {
+		dev_err(&priv->client->dev, "Unable to write i2c index\n");
+		goto out;
+	}
+
+	/* Now do Page Read */
+	if (i2c_master_recv(priv->client, buf, sizeof(buf)) != sizeof(buf)) {
+		dev_err(&priv->client->dev, "Unable to read i2c page\n");
+		goto out;
+	}
+
+	ypos = ((buf[9] & 0x03) << 8 | buf[8]);
+	xpos = ((buf[11] & 0x03) << 8 | buf[10]);
+	event = buf[12];
+
+	if ((event = EVENT_PENDOWN) || (event = EVENT_REPEAT)) {
+		input_report_key(priv->input, BTN_TOUCH, 1);
+		input_report_abs(priv->input, ABS_X, ypos); /*X-Y swap*/
+		input_report_abs(priv->input, ABS_Y, xpos);
+		input_report_abs(priv->input, ABS_PRESSURE, 120);
+		input_sync(priv->input);
+	} else if (event = EVENT_PENUP) {
+		input_report_abs(priv->input, ABS_PRESSURE, 0);
+		input_sync(priv->input);
+	}
+ out:
+	enable_irq(priv->irq);
+}
+
+static irqreturn_t migor_ts_isr(int irq, void *dev_id)
+{
+	struct migor_ts_priv *priv = dev_id;
+
+	disable_irq_nosync(irq);
+	schedule_delayed_work(&priv->work, HZ / 20);
+	return IRQ_HANDLED;
+}
+
+static int migor_ts_probe(struct i2c_client *client)
+{
+	struct migor_ts_priv *priv;
+	int res = 0;
+
+	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
+	if (priv = NULL) {
+		dev_err(&client->dev, "failed to allocate driver data\n");
+		res = -ENOMEM;
+		goto err0;
+	}
+
+	dev_set_drvdata(&client->dev, priv);
+	priv->client = client;
+	INIT_DELAYED_WORK(&priv->work, migor_ts_poscheck);
+	priv->irq = client->irq;
+
+	priv->input = input_allocate_device();
+	if (!priv->input) {
+		dev_err(&client->dev, "Failed to allocate input device.\n");
+		res = -ENOMEM;
+		goto err1;
+	}
+
+	priv->input->evbit[0] = BIT_MASK(EV_KEY) | BIT_MASK(EV_ABS);
+	priv->input->keybit[BIT_WORD(BTN_TOUCH)] = BIT_MASK(BTN_TOUCH);
+
+	input_set_abs_params(priv->input, ABS_X, 95, 955, 0, 0);
+	input_set_abs_params(priv->input, ABS_Y, 85, 935, 0, 0);
+	input_set_abs_params(priv->input, ABS_PRESSURE, 0, 0, 0, 0);
+
+	priv->input->name = client->driver_name;
+	priv->input->phys = "input/event0";
+	priv->input->id.bustype = BUS_ISA;
+
+	res = input_register_device(priv->input);
+	if (res) {
+		input_free_device(priv->input);
+		goto err1;
+	}
+
+	if (request_irq(priv->irq, migor_ts_isr, IRQF_TRIGGER_LOW,
+			client->driver_name, priv)) {
+		dev_err(&client->dev, "Unable to request touchscreen IRQ.\n");
+		res = -EBUSY;
+		goto err2;
+	}
+
+	/* enable controller */
+	if (i2c_master_send(client, migor_ts_ena_seq, sizeof(migor_ts_ena_seq))
+	    = sizeof(migor_ts_ena_seq))
+		return 0;
+
+	dev_err(&client->dev, "Unable to enable touchscreen.\n");
+
+	free_irq(priv->irq, priv);
+ err2:
+	input_unregister_device(priv->input);
+ err1:
+	kfree(priv);
+ err0:
+	return res;
+}
+
+static int migor_ts_remove(struct i2c_client *client)
+{
+	struct migor_ts_priv *priv = dev_get_drvdata(&client->dev);
+
+	/* disable controller */
+	i2c_master_send(client, migor_ts_dis_seq, sizeof(migor_ts_dis_seq));
+
+	free_irq(priv->irq, priv);
+	input_unregister_device(priv->input);
+	kfree(priv);
+	return 0;
+}
+
+static struct i2c_driver migor_ts_driver = {
+	.driver = {
+		.name = "migor_ts",
+	},
+	.probe = migor_ts_probe,
+	.remove = migor_ts_remove,
+};
+
+static int __init migor_ts_init(void)
+{
+	return i2c_add_driver(&migor_ts_driver);
+}
+
+static void __exit migor_ts_exit(void)
+{
+	i2c_del_driver(&migor_ts_driver);
+}
+
+MODULE_DESCRIPTION("MigoR Touchscreen driver");
+MODULE_AUTHOR("Magnus Damm <damm@opensource.se>");
+MODULE_LICENSE("GPL");
+
+module_init(migor_ts_init);
+module_exit(migor_ts_exit);

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

* Re: [PATCH] Touch screen driver for the SuperH MigoR board
  2008-03-21 11:16 [PATCH] Touch screen driver for the SuperH MigoR board Magnus Damm
@ 2008-03-21 22:55 ` Andrew Morton
  2008-03-26  4:03   ` Magnus Damm
  2008-03-28  9:51 ` [PATCH] Touch screen driver for the SuperH MigoR board V2 Magnus Damm
  2008-07-01 10:33 ` [PATCH] Touch screen driver for the SuperH MigoR board Nicholas Beck
  2 siblings, 1 reply; 13+ messages in thread
From: Andrew Morton @ 2008-03-21 22:55 UTC (permalink / raw)
  Cc: linux-input, magnus.damm, lethal, dmitry.torokhov, linux-sh

On Fri, 21 Mar 2008 20:16:30 +0900
Magnus Damm <magnus.damm@gmail.com> wrote:

> This patch adds a touch screen driver for the MigoR board. The chip we
> interface to is unfortunately a custom designed microcontroller speaking
> some undocumented protocol over i2c.
> 
> The board specific code is expected to register this device as an i2c
> chip using struct i2c_board_info [] and i2c_register_board_info().
> 
> ...
>
> +static void migor_ts_poscheck(struct work_struct *work)
> +{
> +	struct migor_ts_priv *priv = container_of(work,
> +						  struct migor_ts_priv,
> +						  work.work);
> +	unsigned short xpos, ypos;
> +	unsigned char event;
> +	u_int8_t buf[16];
> +
> +	memset(buf, 0, sizeof(buf));
> +
> +	/* Set Index 0 */
> +	buf[0] = 0;
> +	if (i2c_master_send(priv->client, buf, 1) != 1) {
> +		dev_err(&priv->client->dev, "Unable to write i2c index\n");
> +		goto out;
> +	}
> +
> +	/* Now do Page Read */
> +	if (i2c_master_recv(priv->client, buf, sizeof(buf)) != sizeof(buf)) {
> +		dev_err(&priv->client->dev, "Unable to read i2c page\n");
> +		goto out;
> +	}
> +
> +	ypos = ((buf[9] & 0x03) << 8 | buf[8]);
> +	xpos = ((buf[11] & 0x03) << 8 | buf[10]);
> +	event = buf[12];
> +
> +	if ((event = EVENT_PENDOWN) || (event = EVENT_REPEAT)) {
> +		input_report_key(priv->input, BTN_TOUCH, 1);
> +		input_report_abs(priv->input, ABS_X, ypos); /*X-Y swap*/
> +		input_report_abs(priv->input, ABS_Y, xpos);
> +		input_report_abs(priv->input, ABS_PRESSURE, 120);
> +		input_sync(priv->input);
> +	} else if (event = EVENT_PENUP) {
> +		input_report_abs(priv->input, ABS_PRESSURE, 0);
> +		input_sync(priv->input);
> +	}
> + out:
> +	enable_irq(priv->irq);
> +}
> +
> +static irqreturn_t migor_ts_isr(int irq, void *dev_id)
> +{
> +	struct migor_ts_priv *priv = dev_id;
> +
> +	disable_irq_nosync(irq);
> +	schedule_delayed_work(&priv->work, HZ / 20);
> +	return IRQ_HANDLED;
> +}

eww.  Doing a disable_irq() on each interrupt and reenabling interrupts
later isn't very nice.  And it might cause havoc if that interrupt is
shared.

Why is this code like this?

No, don't answer that question.  If I wanted to know this when reading
the code, then others will want to know it also.  It needs to be fully
explained in code comments, please.

Can we not pull the data out of the hardware at interrupt time and then
queue that data up for the keventd processing? 

> +static int migor_ts_remove(struct i2c_client *client)
> +{
> +	struct migor_ts_priv *priv = dev_get_drvdata(&client->dev);
> +
> +	/* disable controller */
> +	i2c_master_send(client, migor_ts_dis_seq, sizeof(migor_ts_dis_seq));
> +
> +	free_irq(priv->irq, priv);
> +	input_unregister_device(priv->input);
> +	kfree(priv);
> +	return 0;
> +}

Might a delayed work still be scheduled when we run this?  A
cancel_delayed_work_sync() would set minds at rest.



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

* Re: [PATCH] Touch screen driver for the SuperH MigoR board
  2008-03-21 22:55 ` Andrew Morton
@ 2008-03-26  4:03   ` Magnus Damm
  2008-03-26 11:29     ` Mark Brown
  0 siblings, 1 reply; 13+ messages in thread
From: Magnus Damm @ 2008-03-26  4:03 UTC (permalink / raw)
  To: Andrew Morton; +Cc: linux-input, lethal, dmitry.torokhov, linux-sh

On Sat, Mar 22, 2008 at 7:55 AM, Andrew Morton
<akpm@linux-foundation.org> wrote:
> On Fri, 21 Mar 2008 20:16:30 +0900
>  Magnus Damm <magnus.damm@gmail.com> wrote:
>
>  > This patch adds a touch screen driver for the MigoR board. The chip we
>  > interface to is unfortunately a custom designed microcontroller speaking
>  > some undocumented protocol over i2c.

[snip]

>  > +static irqreturn_t migor_ts_isr(int irq, void *dev_id)
>  > +{
>  > +     struct migor_ts_priv *priv = dev_id;
>  > +
>  > +     disable_irq_nosync(irq);
>  > +     schedule_delayed_work(&priv->work, HZ / 20);
>  > +     return IRQ_HANDLED;
>  > +}
>
>  eww.  Doing a disable_irq() on each interrupt and reenabling interrupts
>  later isn't very nice.  And it might cause havoc if that interrupt is
>  shared.
>
>  Why is this code like this?

The code is more or less doing the same thing as the ads7846 driver,
except this one is communicating over i2c instead of spi. This means
is because of great hardware design with interrupts that needs to be
acked over an extremely slow serial bus. So we just disable the
interrupt and handle everything from a different context. Apart from
avoiding serious latency issues, doing things from interrupt context
simply won't work since the i2c bus driver may be interrupt driven and
may sleep while waiting for the serial bus data to be sent.

And you are right, this won't work with shared interrupts.

>  No, don't answer that question.  If I wanted to know this when reading
>  the code, then others will want to know it also.  It needs to be fully
>  explained in code comments, please.

Yeah, having a comment in there would certainly help.

>  Can we not pull the data out of the hardware at interrupt time and then
>  queue that data up for the keventd processing?

No can do, the serial bus is too slow.

>  > +static int migor_ts_remove(struct i2c_client *client)
>  > +{
>  > +     struct migor_ts_priv *priv = dev_get_drvdata(&client->dev);
>  > +
>  > +     /* disable controller */
>  > +     i2c_master_send(client, migor_ts_dis_seq, sizeof(migor_ts_dis_seq));
>  > +
>  > +     free_irq(priv->irq, priv);
>  > +     input_unregister_device(priv->input);
>  > +     kfree(priv);
>  > +     return 0;
>  > +}
>
>  Might a delayed work still be scheduled when we run this?  A
>  cancel_delayed_work_sync() would set minds at rest.

Good catch. I'll fix this up and repost.

Thanks!

/ magnus

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

* Re: [PATCH] Touch screen driver for the SuperH MigoR board
  2008-03-26  4:03   ` Magnus Damm
@ 2008-03-26 11:29     ` Mark Brown
  0 siblings, 0 replies; 13+ messages in thread
From: Mark Brown @ 2008-03-26 11:29 UTC (permalink / raw)
  To: Magnus Damm; +Cc: Andrew Morton, linux-input, lethal, dmitry.torokhov, linux-sh

On Wed, Mar 26, 2008 at 01:03:09PM +0900, Magnus Damm wrote:

> interrupt and handle everything from a different context. Apart from
> avoiding serious latency issues, doing things from interrupt context
> simply won't work since the i2c bus driver may be interrupt driven and
> may sleep while waiting for the serial bus data to be sent.

Given that having devices that generate interrupts that need to be
serviced over interrupt driven buses is a fairly common pattern it seems
like it would be worth adding some helper code - if nothing else, it
would help with the documentation.

-- 
"You grabbed my hand and we fell into it, like a daydream - or a fever."

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

* [PATCH] Touch screen driver for the SuperH MigoR board V2
  2008-03-21 11:16 [PATCH] Touch screen driver for the SuperH MigoR board Magnus Damm
  2008-03-21 22:55 ` Andrew Morton
@ 2008-03-28  9:51 ` Magnus Damm
  2008-04-02  6:08   ` Dmitry Torokhov
  2008-07-01 10:33 ` [PATCH] Touch screen driver for the SuperH MigoR board Nicholas Beck
  2 siblings, 1 reply; 13+ messages in thread
From: Magnus Damm @ 2008-03-28  9:51 UTC (permalink / raw)
  To: linux-input; +Cc: linux-sh, dmitry.torokhov, lethal, broonie, Magnus Damm, akpm

This is V2 of the MigoR touch screen driver. The chip we interface to
is unfortunately a custom designed microcontroller speaking some
undocumented protocol over i2c.

The board specific code is expected to register this device as an i2c
chip using struct i2c_board_info [] and i2c_register_board_info().

Changes since V1:
 - added cancel_delayed_work_sync()
 - added interrupt handler comment

Signed-off-by: Magnus Damm <damm@igel.co.jp>
---

 drivers/input/touchscreen/Kconfig    |   12 +
 drivers/input/touchscreen/Makefile   |    1 
 drivers/input/touchscreen/migor_ts.c |  213 ++++++++++++++++++++++++++++++++++
 3 files changed, 226 insertions(+)

--- 0001/drivers/input/touchscreen/Kconfig
+++ work/drivers/input/touchscreen/Kconfig	2008-03-28 15:19:11.000000000 +0900
@@ -134,6 +134,18 @@ config TOUCHSCREEN_HP7XX
 	  To compile this driver as a module, choose M here: the
 	  module will be called jornada720_ts.
 
+config TOUCHSCREEN_MIGOR
+	tristate "Renesas MIGO-R touchscreen"
+	depends on SH_MIGOR
+	default n
+	help
+	  Say Y here to enable MIGO-R touchscreen support.
+
+	  If unsure, say N.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called migor_ts.
+
 config TOUCHSCREEN_PENMOUNT
 	tristate "Penmount serial touchscreen"
 	select SERIO
--- 0001/drivers/input/touchscreen/Makefile
+++ work/drivers/input/touchscreen/Makefile	2008-03-28 15:19:11.000000000 +0900
@@ -19,3 +19,4 @@ obj-$(CONFIG_TOUCHSCREEN_PENMOUNT)	+= pe
 obj-$(CONFIG_TOUCHSCREEN_TOUCHRIGHT)	+= touchright.o
 obj-$(CONFIG_TOUCHSCREEN_TOUCHWIN)	+= touchwin.o
 obj-$(CONFIG_TOUCHSCREEN_UCB1400)	+= ucb1400_ts.o
+obj-$(CONFIG_TOUCHSCREEN_MIGOR)		+= migor_ts.o
--- /dev/null
+++ work/drivers/input/touchscreen/migor_ts.c	2008-03-28 16:19:02.000000000 +0900
@@ -0,0 +1,213 @@
+/*
+ * Touch Screen driver for Renesas MIGO-R Platform
+ *
+ * Copyright (c) 2008 Magnus Damm
+ * Copyright (c) 2007 Ujjwal Pande <ujjwal@kenati.com>,
+ *  Kenati Technologies Pvt Ltd.
+ *
+ * This file 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 2 of the License, or (at your option) any later version.
+ *
+ * This file 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 library; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ */
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/input.h>
+#include <linux/interrupt.h>
+#include <asm/io.h>
+#include <linux/i2c.h>
+#include <linux/timer.h>
+
+#define EVENT_PENDOWN 1
+#define EVENT_REPEAT  2
+#define EVENT_PENUP   3
+
+struct migor_ts_priv {
+	struct i2c_client *client;
+	struct input_dev *input;
+	struct delayed_work work;
+	int irq;
+};
+
+static const u_int8_t migor_ts_ena_seq[17] = { 0x33, 0x22, 0x11,
+					       0x01, 0x06, 0x07, };
+static const u_int8_t migor_ts_dis_seq[17] = { };
+
+static void migor_ts_poscheck(struct work_struct *work)
+{
+	struct migor_ts_priv *priv = container_of(work,
+						  struct migor_ts_priv,
+						  work.work);
+	unsigned short xpos, ypos;
+	unsigned char event;
+	u_int8_t buf[16];
+
+	memset(buf, 0, sizeof(buf));
+
+	/* Set Index 0 */
+	buf[0] = 0;
+	if (i2c_master_send(priv->client, buf, 1) != 1) {
+		dev_err(&priv->client->dev, "Unable to write i2c index\n");
+		goto out;
+	}
+
+	/* Now do Page Read */
+	if (i2c_master_recv(priv->client, buf, sizeof(buf)) != sizeof(buf)) {
+		dev_err(&priv->client->dev, "Unable to read i2c page\n");
+		goto out;
+	}
+
+	ypos = ((buf[9] & 0x03) << 8 | buf[8]);
+	xpos = ((buf[11] & 0x03) << 8 | buf[10]);
+	event = buf[12];
+
+	if ((event = EVENT_PENDOWN) || (event = EVENT_REPEAT)) {
+		input_report_key(priv->input, BTN_TOUCH, 1);
+		input_report_abs(priv->input, ABS_X, ypos); /*X-Y swap*/
+		input_report_abs(priv->input, ABS_Y, xpos);
+		input_report_abs(priv->input, ABS_PRESSURE, 120);
+		input_sync(priv->input);
+	} else if (event = EVENT_PENUP) {
+		input_report_abs(priv->input, ABS_PRESSURE, 0);
+		input_sync(priv->input);
+	}
+ out:
+	enable_irq(priv->irq);
+}
+
+static irqreturn_t migor_ts_isr(int irq, void *dev_id)
+{
+	struct migor_ts_priv *priv = dev_id;
+
+	/* the touch screen controller chip is hooked up to the cpu
+	 * using i2c and a single interrupt line. the interrupt line
+	 * is pulled low whenever someone taps the screen. to deassert
+	 * the interrupt line we need to acknowledge the interrupt by
+	 * communicating with the controller over the slow i2c bus.
+	 *
+	 * we can't acknowledge from interrupt context since the i2c
+	 * bus controller may sleep, so we just disable the interrupt
+	 * here and handle the acknowledge using delayed work.
+	 */
+
+	disable_irq_nosync(irq);
+	schedule_delayed_work(&priv->work, HZ / 20);
+	return IRQ_HANDLED;
+}
+
+static int migor_ts_probe(struct i2c_client *client)
+{
+	struct migor_ts_priv *priv;
+	int res = 0;
+
+	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
+	if (priv = NULL) {
+		dev_err(&client->dev, "failed to allocate driver data\n");
+		res = -ENOMEM;
+		goto err0;
+	}
+
+	dev_set_drvdata(&client->dev, priv);
+	priv->client = client;
+	INIT_DELAYED_WORK(&priv->work, migor_ts_poscheck);
+	priv->irq = client->irq;
+
+	priv->input = input_allocate_device();
+	if (!priv->input) {
+		dev_err(&client->dev, "Failed to allocate input device.\n");
+		res = -ENOMEM;
+		goto err1;
+	}
+
+	priv->input->evbit[0] = BIT_MASK(EV_KEY) | BIT_MASK(EV_ABS);
+	priv->input->keybit[BIT_WORD(BTN_TOUCH)] = BIT_MASK(BTN_TOUCH);
+
+	input_set_abs_params(priv->input, ABS_X, 95, 955, 0, 0);
+	input_set_abs_params(priv->input, ABS_Y, 85, 935, 0, 0);
+	input_set_abs_params(priv->input, ABS_PRESSURE, 0, 0, 0, 0);
+
+	priv->input->name = client->driver_name;
+	priv->input->phys = "input/event0";
+	priv->input->id.bustype = BUS_ISA;
+
+	res = input_register_device(priv->input);
+	if (res) {
+		input_free_device(priv->input);
+		goto err1;
+	}
+
+	if (request_irq(priv->irq, migor_ts_isr, IRQF_TRIGGER_LOW,
+			client->driver_name, priv)) {
+		dev_err(&client->dev, "Unable to request touchscreen IRQ.\n");
+		res = -EBUSY;
+		goto err2;
+	}
+
+	/* enable controller */
+	if (i2c_master_send(client, migor_ts_ena_seq, sizeof(migor_ts_ena_seq))
+	    = sizeof(migor_ts_ena_seq))
+		return 0;
+
+	dev_err(&client->dev, "Unable to enable touchscreen.\n");
+
+	free_irq(priv->irq, priv);
+ err2:
+	input_unregister_device(priv->input);
+ err1:
+	kfree(priv);
+ err0:
+	return res;
+}
+
+static int migor_ts_remove(struct i2c_client *client)
+{
+	struct migor_ts_priv *priv = dev_get_drvdata(&client->dev);
+	int ret;
+
+	/* cancel pending work and wait for migor_ts_poscheck() to finish */
+	ret = cancel_delayed_work_sync(&priv->work);
+	if (ret)
+		return ret;
+
+	/* disable controller */
+	i2c_master_send(client, migor_ts_dis_seq, sizeof(migor_ts_dis_seq));
+
+	free_irq(priv->irq, priv);
+	input_unregister_device(priv->input);
+	kfree(priv);
+	return ret;
+}
+
+static struct i2c_driver migor_ts_driver = {
+	.driver = {
+		.name = "migor_ts",
+	},
+	.probe = migor_ts_probe,
+	.remove = migor_ts_remove,
+};
+
+static int __init migor_ts_init(void)
+{
+	return i2c_add_driver(&migor_ts_driver);
+}
+
+static void __exit migor_ts_exit(void)
+{
+	i2c_del_driver(&migor_ts_driver);
+}
+
+MODULE_DESCRIPTION("MigoR Touchscreen driver");
+MODULE_AUTHOR("Magnus Damm <damm@opensource.se>");
+MODULE_LICENSE("GPL");
+
+module_init(migor_ts_init);
+module_exit(migor_ts_exit);

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

* Re: [PATCH] Touch screen driver for the SuperH MigoR board V2
  2008-03-28  9:51 ` [PATCH] Touch screen driver for the SuperH MigoR board V2 Magnus Damm
@ 2008-04-02  6:08   ` Dmitry Torokhov
  2008-04-04  8:21     ` Magnus Damm
  0 siblings, 1 reply; 13+ messages in thread
From: Dmitry Torokhov @ 2008-04-02  6:08 UTC (permalink / raw)
  To: Magnus Damm; +Cc: linux-input, linux-sh, lethal, broonie, akpm

Hi Magnus,

On Fri, Mar 28, 2008 at 06:51:01PM +0900, Magnus Damm wrote:
> This is V2 of the MigoR touch screen driver. The chip we interface to
> is unfortunately a custom designed microcontroller speaking some
> undocumented protocol over i2c.
> 

Thank you for the patch, I just have a couple of comments below...
 
> +config TOUCHSCREEN_MIGOR
> +	tristate "Renesas MIGO-R touchscreen"
> +	depends on SH_MIGOR
> +	default n

N is the default default so this line is not needed.

> +
> +	if ((event = EVENT_PENDOWN) || (event = EVENT_REPEAT)) {
> +		input_report_key(priv->input, BTN_TOUCH, 1);
> +		input_report_abs(priv->input, ABS_X, ypos); /*X-Y swap*/
> +		input_report_abs(priv->input, ABS_Y, xpos);
> +		input_report_abs(priv->input, ABS_PRESSURE, 120);
> +		input_sync(priv->input);
> +	} else if (event = EVENT_PENUP) {
> +		input_report_abs(priv->input, ABS_PRESSURE, 0);

Don't you need to signal BTN_TOUCH release here?

> +	input_set_abs_params(priv->input, ABS_X, 95, 955, 0, 0);
> +	input_set_abs_params(priv->input, ABS_Y, 85, 935, 0, 0);
> +	input_set_abs_params(priv->input, ABS_PRESSURE, 0, 0, 0, 0);

The device does not support pressure reporting so don't pretend
to send one. If this was done because of tslib please fix tslib
instead.

> +
> +	priv->input->name = client->driver_name;
> +	priv->input->phys = "input/event0";

Normally we encode bus slot or port in phys. If this data
is unavailable it is better to omit phys altogether.

What we can and should do is properly set up input device in
sysfs hierarchy by parenting it to the i2c client:

	input->dev.parent = &client->dev;

> +	priv->input->id.bustype = BUS_ISA;

Not BUS_i2C?

> +
> +	res = input_register_device(priv->input);
> +	if (res) {
> +		input_free_device(priv->input);
> +		goto err1;
> +	}
> +
> +	if (request_irq(priv->irq, migor_ts_isr, IRQF_TRIGGER_LOW,
> +			client->driver_name, priv)) {
> +		dev_err(&client->dev, "Unable to request touchscreen IRQ.\n");
> +		res = -EBUSY;
> +		goto err2;
> +	}
> +
> +	/* enable controller */
> +	if (i2c_master_send(client, migor_ts_ena_seq, sizeof(migor_ts_ena_seq))
> +	    = sizeof(migor_ts_ena_seq))
> +		return 0;
> +
> +	dev_err(&client->dev, "Unable to enable touchscreen.\n");
> +

Since you are not setting res here you will signal success the module
loader and bad things will happen.

I tried implementing my suggestions in the patch below, please let me
know if it still works for you and I will apply it.

Thanks!

-- 
Dmitry

Subject: Input: add support for SuperH MigoR touchscreen
From: Magnus Damm <magnus.damm@gmail.com>

Input: add support for SuperH MigoR touchscreen

This is V2 of the MigoR touch screen driver. The chip we interface to
is unfortunately a custom designed microcontroller speaking some
undocumented protocol over i2c.

The board specific code is expected to register this device as an i2c
chip using struct i2c_board_info [] and i2c_register_board_info().

Signed-off-by: Magnus Damm <damm@igel.co.jp>
Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
---
 drivers/input/touchscreen/Kconfig    |   11 +
 drivers/input/touchscreen/Makefile   |    1 
 drivers/input/touchscreen/migor_ts.c |  222 +++++++++++++++++++++++++++++++++++
 3 files changed, 234 insertions(+)

Index: work/drivers/input/touchscreen/Kconfig
=================================--- work.orig/drivers/input/touchscreen/Kconfig
+++ work/drivers/input/touchscreen/Kconfig
@@ -146,6 +146,17 @@ config TOUCHSCREEN_PENMOUNT
 	  To compile this driver as a module, choose M here: the
 	  module will be called penmount.
 
+config TOUCHSCREEN_MIGOR
+	tristate "Renesas MIGO-R touchscreen"
+	depends on SH_MIGOR
+	help
+	  Say Y here to enable MIGO-R touchscreen support.
+
+	  If unsure, say N.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called migor_ts.
+
 config TOUCHSCREEN_TOUCHRIGHT
 	tristate "Touchright serial touchscreen"
 	select SERIO
Index: work/drivers/input/touchscreen/Makefile
=================================--- work.orig/drivers/input/touchscreen/Makefile
+++ work/drivers/input/touchscreen/Makefile
@@ -10,6 +10,7 @@ obj-$(CONFIG_TOUCHSCREEN_CORGI)		+= corg
 obj-$(CONFIG_TOUCHSCREEN_GUNZE)		+= gunze.o
 obj-$(CONFIG_TOUCHSCREEN_ELO)		+= elo.o
 obj-$(CONFIG_TOUCHSCREEN_FUJITSU)	+= fujitsu_ts.o
+obj-$(CONFIG_TOUCHSCREEN_MIGOR)		+= migor_ts.o
 obj-$(CONFIG_TOUCHSCREEN_MTOUCH)	+= mtouch.o
 obj-$(CONFIG_TOUCHSCREEN_MK712)		+= mk712.o
 obj-$(CONFIG_TOUCHSCREEN_HP600)		+= hp680_ts_input.o
Index: work/drivers/input/touchscreen/migor_ts.c
=================================--- /dev/null
+++ work/drivers/input/touchscreen/migor_ts.c
@@ -0,0 +1,222 @@
+/*
+ * Touch Screen driver for Renesas MIGO-R Platform
+ *
+ * Copyright (c) 2008 Magnus Damm
+ * Copyright (c) 2007 Ujjwal Pande <ujjwal@kenati.com>,
+ *  Kenati Technologies Pvt Ltd.
+ *
+ * This file 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 2 of the License, or (at your option) any later version.
+ *
+ * This file 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 library; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ */
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/input.h>
+#include <linux/interrupt.h>
+#include <asm/io.h>
+#include <linux/i2c.h>
+#include <linux/timer.h>
+
+#define EVENT_PENDOWN 1
+#define EVENT_REPEAT  2
+#define EVENT_PENUP   3
+
+struct migor_ts_priv {
+	struct i2c_client *client;
+	struct input_dev *input;
+	struct delayed_work work;
+	int irq;
+};
+
+static const u_int8_t migor_ts_ena_seq[17] = { 0x33, 0x22, 0x11,
+					       0x01, 0x06, 0x07, };
+static const u_int8_t migor_ts_dis_seq[17] = { };
+
+static void migor_ts_poscheck(struct work_struct *work)
+{
+	struct migor_ts_priv *priv = container_of(work,
+						  struct migor_ts_priv,
+						  work.work);
+	unsigned short xpos, ypos;
+	unsigned char event;
+	u_int8_t buf[16];
+
+	memset(buf, 0, sizeof(buf));
+
+	/* Set Index 0 */
+	buf[0] = 0;
+	if (i2c_master_send(priv->client, buf, 1) != 1) {
+		dev_err(&priv->client->dev, "Unable to write i2c index\n");
+		goto out;
+	}
+
+	/* Now do Page Read */
+	if (i2c_master_recv(priv->client, buf, sizeof(buf)) != sizeof(buf)) {
+		dev_err(&priv->client->dev, "Unable to read i2c page\n");
+		goto out;
+	}
+
+	ypos = ((buf[9] & 0x03) << 8 | buf[8]);
+	xpos = ((buf[11] & 0x03) << 8 | buf[10]);
+	event = buf[12];
+
+	if ((event = EVENT_PENDOWN) || (event = EVENT_REPEAT)) {
+		input_report_key(priv->input, BTN_TOUCH, 1);
+		input_report_abs(priv->input, ABS_X, ypos); /*X-Y swap*/
+		input_report_abs(priv->input, ABS_Y, xpos);
+		input_sync(priv->input);
+	} else if (event = EVENT_PENUP) {
+		input_report_key(priv->input, BTN_TOUCH, 0);
+		input_sync(priv->input);
+	}
+ out:
+	enable_irq(priv->irq);
+}
+
+static irqreturn_t migor_ts_isr(int irq, void *dev_id)
+{
+	struct migor_ts_priv *priv = dev_id;
+
+	/* the touch screen controller chip is hooked up to the cpu
+	 * using i2c and a single interrupt line. the interrupt line
+	 * is pulled low whenever someone taps the screen. to deassert
+	 * the interrupt line we need to acknowledge the interrupt by
+	 * communicating with the controller over the slow i2c bus.
+	 *
+	 * we can't acknowledge from interrupt context since the i2c
+	 * bus controller may sleep, so we just disable the interrupt
+	 * here and handle the acknowledge using delayed work.
+	 */
+
+	disable_irq_nosync(irq);
+	schedule_delayed_work(&priv->work, HZ / 20);
+
+	return IRQ_HANDLED;
+}
+
+static int migor_ts_probe(struct i2c_client *client)
+{
+	struct migor_ts_priv *priv;
+	struct input_dev *input;
+	int count;
+	int error;
+
+	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
+	if (!priv) {
+		dev_err(&client->dev, "failed to allocate driver data\n");
+		error = -ENOMEM;
+		goto err0;
+	}
+
+	dev_set_drvdata(&client->dev, priv);
+
+	input = input_allocate_device();
+	if (!input) {
+		dev_err(&client->dev, "Failed to allocate input device.\n");
+		error = -ENOMEM;
+		goto err1;
+	}
+
+	input->evbit[0] = BIT_MASK(EV_KEY) | BIT_MASK(EV_ABS);
+	input->keybit[BIT_WORD(BTN_TOUCH)] = BIT_MASK(BTN_TOUCH);
+
+	input_set_abs_params(input, ABS_X, 95, 955, 0, 0);
+	input_set_abs_params(input, ABS_Y, 85, 935, 0, 0);
+
+	input->name = client->driver_name;
+	input->id.bustype = BUS_I2C;
+	input->dev.parent = &client->dev;
+
+	priv->client = client;
+	priv->input = input;
+	INIT_DELAYED_WORK(&priv->work, migor_ts_poscheck);
+	priv->irq = client->irq;
+
+	error = input_register_device(input);
+	if (error)
+		goto err1;
+
+	error = request_irq(priv->irq, migor_ts_isr, IRQF_TRIGGER_LOW,
+			    client->driver_name, priv);
+	if (error) {
+		dev_err(&client->dev, "Unable to request touchscreen IRQ.\n");
+		goto err2;
+	}
+
+	/* enable controller */
+	count = i2c_master_send(client, migor_ts_ena_seq,
+				sizeof(migor_ts_ena_seq));
+	if (count != sizeof(migor_ts_ena_seq)) {
+		dev_err(&client->dev, "Unable to enable touchscreen.\n");
+		error = -ENXIO;
+		goto err3;
+	}
+
+	return 0;
+
+ err3:
+	free_irq(priv->irq, priv);
+ err2:
+	input_unregister_device(input);
+	input = NULL; /* so we dont try to free it below */
+ err1:
+	input_free_device(input);
+	kfree(priv);
+ err0:
+	dev_set_drvdata(&client->dev, NULL);
+	return error;
+}
+
+static int migor_ts_remove(struct i2c_client *client)
+{
+	struct migor_ts_priv *priv = dev_get_drvdata(&client->dev);
+
+	/* cancel pending work and wait for migor_ts_poscheck() to finish */
+	cancel_delayed_work_sync(&priv->work);
+
+	/* disable controller */
+	i2c_master_send(client, migor_ts_dis_seq, sizeof(migor_ts_dis_seq));
+
+	free_irq(priv->irq, priv);
+	input_unregister_device(priv->input);
+	kfree(priv);
+
+	dev_set_drvdata(&client->dev, NULL);
+
+	return 0;
+}
+
+static struct i2c_driver migor_ts_driver = {
+	.driver = {
+		.name = "migor_ts",
+	},
+	.probe = migor_ts_probe,
+	.remove = migor_ts_remove,
+};
+
+static int __init migor_ts_init(void)
+{
+	return i2c_add_driver(&migor_ts_driver);
+}
+
+static void __exit migor_ts_exit(void)
+{
+	i2c_del_driver(&migor_ts_driver);
+}
+
+MODULE_DESCRIPTION("MigoR Touchscreen driver");
+MODULE_AUTHOR("Magnus Damm <damm@opensource.se>");
+MODULE_LICENSE("GPL");
+
+module_init(migor_ts_init);
+module_exit(migor_ts_exit);

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

* Re: [PATCH] Touch screen driver for the SuperH MigoR board V2
  2008-04-02  6:08   ` Dmitry Torokhov
@ 2008-04-04  8:21     ` Magnus Damm
  2008-04-04 16:14       ` Dmitry Torokhov
  0 siblings, 1 reply; 13+ messages in thread
From: Magnus Damm @ 2008-04-04  8:21 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: linux-input, linux-sh, lethal, broonie, akpm

Hi Dmitry,

On Wed, Apr 2, 2008 at 3:08 PM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
>  On Fri, Mar 28, 2008 at 06:51:01PM +0900, Magnus Damm wrote:
>  > This is V2 of the MigoR touch screen driver. The chip we interface to
>  > is unfortunately a custom designed microcontroller speaking some
>  > undocumented protocol over i2c.
>
>  Thank you for the patch, I just have a couple of comments below...

Your comments are much appreciated.

>  > +     if ((event = EVENT_PENDOWN) || (event = EVENT_REPEAT)) {
>  > +             input_report_key(priv->input, BTN_TOUCH, 1);
>  > +             input_report_abs(priv->input, ABS_X, ypos); /*X-Y swap*/
>  > +             input_report_abs(priv->input, ABS_Y, xpos);
>  > +             input_report_abs(priv->input, ABS_PRESSURE, 120);
>  > +             input_sync(priv->input);
>  > +     } else if (event = EVENT_PENUP) {
>  > +             input_report_abs(priv->input, ABS_PRESSURE, 0);
>
>  Don't you need to signal BTN_TOUCH release here?

Yes, you are correct. It is missing.

>  > +     input_set_abs_params(priv->input, ABS_X, 95, 955, 0, 0);
>  > +     input_set_abs_params(priv->input, ABS_Y, 85, 935, 0, 0);
>  > +     input_set_abs_params(priv->input, ABS_PRESSURE, 0, 0, 0, 0);
>
>  The device does not support pressure reporting so don't pretend
>  to send one. If this was done because of tslib please fix tslib
>  instead.

Right, there is no need to pretend.

>  > +     priv->input->name = client->driver_name;
>  > +     priv->input->phys = "input/event0";
>
>  Normally we encode bus slot or port in phys. If this data
>  is unavailable it is better to omit phys altogether.
>
>  What we can and should do is properly set up input device in
>  sysfs hierarchy by parenting it to the i2c client:
>
>         input->dev.parent = &client->dev;

Much better, thank you.

>  > +     priv->input->id.bustype = BUS_ISA;
>
>  Not BUS_i2C?

Yes please, BUS_I2C. =)

>  > +     if (request_irq(priv->irq, migor_ts_isr, IRQF_TRIGGER_LOW,
>  > +                     client->driver_name, priv)) {
>  > +             dev_err(&client->dev, "Unable to request touchscreen IRQ.\n");
>  > +             res = -EBUSY;
>  > +             goto err2;
>  > +     }
>  > +
>  > +     /* enable controller */
>  > +     if (i2c_master_send(client, migor_ts_ena_seq, sizeof(migor_ts_ena_seq))
>  > +         = sizeof(migor_ts_ena_seq))
>  > +             return 0;
>  > +
>  > +     dev_err(&client->dev, "Unable to enable touchscreen.\n");
>  > +
>
>  Since you are not setting res here you will signal success the module
>  loader and bad things will happen.

Yeah. Good catch.

>  I tried implementing my suggestions in the patch below, please let me
>  know if it still works for you and I will apply it.

I just tested using latest sh-2.6 git with evtest. Everything seems to
work well. The patch is much cleaner now. Please apply.

Thank you!

/ magnus

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

* Re: [PATCH] Touch screen driver for the SuperH MigoR board V2
  2008-04-04  8:21     ` Magnus Damm
@ 2008-04-04 16:14       ` Dmitry Torokhov
  2008-04-25 10:29         ` Magnus Damm
  0 siblings, 1 reply; 13+ messages in thread
From: Dmitry Torokhov @ 2008-04-04 16:14 UTC (permalink / raw)
  To: Magnus Damm; +Cc: linux-input, linux-sh, lethal, broonie, akpm

Hi Magnus,

On Fri, Apr 04, 2008 at 05:21:48PM +0900, Magnus Damm wrote:
> I just tested using latest sh-2.6 git with evtest. Everything seems to
> work well. The patch is much cleaner now. Please apply.
> 

I was just looking the driver over one more time before applying it
and I think there is a race in migor_ts_remove():

+       /* cancel pending work and wait for migor_ts_poscheck() to finish */
+       cancel_delayed_work_sync(&priv->work);
+

What if interrupt comes here, before we got a chance to shut off the
device? IRQ is still enabled and it will schedule migor_ts_poscheck()
again. I think we need to call disable_irq() before canceling the work.
Since the driver does not support sharing IRQs that should be allright.
What do you think?

+       /* disable controller */
+       i2c_master_send(client, migor_ts_dis_seq, sizeof(migor_ts_dis_seq));
+
+       free_irq(priv->irq, priv);

Thank you.

-- 
Dmitry

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

* Re: [PATCH] Touch screen driver for the SuperH MigoR board V2
  2008-04-04 16:14       ` Dmitry Torokhov
@ 2008-04-25 10:29         ` Magnus Damm
  2008-04-27  5:07           ` Dmitry Torokhov
  0 siblings, 1 reply; 13+ messages in thread
From: Magnus Damm @ 2008-04-25 10:29 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: linux-input, linux-sh, lethal, broonie, akpm

Hi Dmitry,

Thanks for all your help with this driver.

On Sat, Apr 5, 2008 at 1:14 AM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
>  On Fri, Apr 04, 2008 at 05:21:48PM +0900, Magnus Damm wrote:
>  > I just tested using latest sh-2.6 git with evtest. Everything seems to
>  > work well. The patch is much cleaner now. Please apply.
>  >
>
>  I was just looking the driver over one more time before applying it
>  and I think there is a race in migor_ts_remove():
>
>  +       /* cancel pending work and wait for migor_ts_poscheck() to finish */
>
> +       cancel_delayed_work_sync(&priv->work);
>  +
>
>  What if interrupt comes here, before we got a chance to shut off the
>  device? IRQ is still enabled and it will schedule migor_ts_poscheck()
>  again. I think we need to call disable_irq() before canceling the work.
>  Since the driver does not support sharing IRQs that should be allright.
>  What do you think?

I think you are right. Doing a disable_irq() before canceling the work
sounds good. Can you please add that, or do you want me to fix and
repost?

Thank you!

/ magnus

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

* Re: [PATCH] Touch screen driver for the SuperH MigoR board V2
  2008-04-25 10:29         ` Magnus Damm
@ 2008-04-27  5:07           ` Dmitry Torokhov
  2008-05-07 12:14             ` Magnus Damm
  0 siblings, 1 reply; 13+ messages in thread
From: Dmitry Torokhov @ 2008-04-27  5:07 UTC (permalink / raw)
  To: Magnus Damm; +Cc: linux-input, linux-sh, lethal, broonie, akpm

Hi Magnus,

On Fri, Apr 25, 2008 at 07:29:28PM +0900, Magnus Damm wrote:
> Hi Dmitry,
> 
> Thanks for all your help with this driver.
> 
> On Sat, Apr 5, 2008 at 1:14 AM, Dmitry Torokhov
> <dmitry.torokhov@gmail.com> wrote:
> >  On Fri, Apr 04, 2008 at 05:21:48PM +0900, Magnus Damm wrote:
> >  > I just tested using latest sh-2.6 git with evtest. Everything seems to
> >  > work well. The patch is much cleaner now. Please apply.
> >  >
> >
> >  I was just looking the driver over one more time before applying it
> >  and I think there is a race in migor_ts_remove():
> >
> >  +       /* cancel pending work and wait for migor_ts_poscheck() to finish */
> >
> > +       cancel_delayed_work_sync(&priv->work);
> >  +
> >
> >  What if interrupt comes here, before we got a chance to shut off the
> >  device? IRQ is still enabled and it will schedule migor_ts_poscheck()
> >  again. I think we need to call disable_irq() before canceling the work.
> >  Since the driver does not support sharing IRQs that should be allright.
> >  What do you think?
> 
> I think you are right. Doing a disable_irq() before canceling the work
> sounds good. Can you please add that, or do you want me to fix and
> repost?
>

Almost done ;) I also wanted to start the controller only when there are
users for the interface, by defining ->open and ->close methods for the
input device. Could you please take a look at the updated patch and if
it still works I will aplly it.

Thanks a lot.

-- 
Dmitry

From: Magnus Damm <magnus.damm@gmail.com>

Input: add support for SuperH MigoR touchscreen

This is V2 of the MigoR touch screen driver. The chip we interface to
is unfortunately a custom designed microcontroller speaking some
undocumented protocol over i2c.

The board specific code is expected to register this device as an i2c
chip using struct i2c_board_info [] and i2c_register_board_info().

Signed-off-by: Magnus Damm <damm@igel.co.jp>
Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
---
 drivers/input/touchscreen/Kconfig    |   11 +
 drivers/input/touchscreen/Makefile   |    1 
 drivers/input/touchscreen/migor_ts.c |  249 +++++++++++++++++++++++++++++++++++
 3 files changed, 261 insertions(+)

Index: work/drivers/input/touchscreen/Kconfig
=================================--- work.orig/drivers/input/touchscreen/Kconfig
+++ work/drivers/input/touchscreen/Kconfig
@@ -146,6 +146,17 @@ config TOUCHSCREEN_PENMOUNT
 	  To compile this driver as a module, choose M here: the
 	  module will be called penmount.
 
+config TOUCHSCREEN_MIGOR
+	tristate "Renesas MIGO-R touchscreen"
+	depends on SH_MIGOR
+	help
+	  Say Y here to enable MIGO-R touchscreen support.
+
+	  If unsure, say N.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called migor_ts.
+
 config TOUCHSCREEN_TOUCHRIGHT
 	tristate "Touchright serial touchscreen"
 	select SERIO
Index: work/drivers/input/touchscreen/Makefile
=================================--- work.orig/drivers/input/touchscreen/Makefile
+++ work/drivers/input/touchscreen/Makefile
@@ -12,6 +12,7 @@ obj-$(CONFIG_TOUCHSCREEN_CORGI)		+= corg
 obj-$(CONFIG_TOUCHSCREEN_GUNZE)		+= gunze.o
 obj-$(CONFIG_TOUCHSCREEN_ELO)		+= elo.o
 obj-$(CONFIG_TOUCHSCREEN_FUJITSU)	+= fujitsu_ts.o
+obj-$(CONFIG_TOUCHSCREEN_MIGOR)		+= migor_ts.o
 obj-$(CONFIG_TOUCHSCREEN_MTOUCH)	+= mtouch.o
 obj-$(CONFIG_TOUCHSCREEN_MK712)		+= mk712.o
 obj-$(CONFIG_TOUCHSCREEN_HP600)		+= hp680_ts_input.o
Index: work/drivers/input/touchscreen/migor_ts.c
=================================--- /dev/null
+++ work/drivers/input/touchscreen/migor_ts.c
@@ -0,0 +1,249 @@
+/*
+ * Touch Screen driver for Renesas MIGO-R Platform
+ *
+ * Copyright (c) 2008 Magnus Damm
+ * Copyright (c) 2007 Ujjwal Pande <ujjwal@kenati.com>,
+ *  Kenati Technologies Pvt Ltd.
+ *
+ * This file 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 2 of the License, or (at your option) any later version.
+ *
+ * This file 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 library; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ */
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/input.h>
+#include <linux/interrupt.h>
+#include <asm/io.h>
+#include <linux/i2c.h>
+#include <linux/timer.h>
+
+#define EVENT_PENDOWN 1
+#define EVENT_REPEAT  2
+#define EVENT_PENUP   3
+
+struct migor_ts_priv {
+	struct i2c_client *client;
+	struct input_dev *input;
+	struct delayed_work work;
+	int irq;
+};
+
+static const u_int8_t migor_ts_ena_seq[17] = { 0x33, 0x22, 0x11,
+					       0x01, 0x06, 0x07, };
+static const u_int8_t migor_ts_dis_seq[17] = { };
+
+static void migor_ts_poscheck(struct work_struct *work)
+{
+	struct migor_ts_priv *priv = container_of(work,
+						  struct migor_ts_priv,
+						  work.work);
+	unsigned short xpos, ypos;
+	unsigned char event;
+	u_int8_t buf[16];
+
+	memset(buf, 0, sizeof(buf));
+
+	/* Set Index 0 */
+	buf[0] = 0;
+	if (i2c_master_send(priv->client, buf, 1) != 1) {
+		dev_err(&priv->client->dev, "Unable to write i2c index\n");
+		goto out;
+	}
+
+	/* Now do Page Read */
+	if (i2c_master_recv(priv->client, buf, sizeof(buf)) != sizeof(buf)) {
+		dev_err(&priv->client->dev, "Unable to read i2c page\n");
+		goto out;
+	}
+
+	ypos = ((buf[9] & 0x03) << 8 | buf[8]);
+	xpos = ((buf[11] & 0x03) << 8 | buf[10]);
+	event = buf[12];
+
+	if (event = EVENT_PENDOWN || event = EVENT_REPEAT) {
+		input_report_key(priv->input, BTN_TOUCH, 1);
+		input_report_abs(priv->input, ABS_X, ypos); /*X-Y swap*/
+		input_report_abs(priv->input, ABS_Y, xpos);
+		input_sync(priv->input);
+	} else if (event = EVENT_PENUP) {
+		input_report_key(priv->input, BTN_TOUCH, 0);
+		input_sync(priv->input);
+	}
+ out:
+	enable_irq(priv->irq);
+}
+
+static irqreturn_t migor_ts_isr(int irq, void *dev_id)
+{
+	struct migor_ts_priv *priv = dev_id;
+
+	/* the touch screen controller chip is hooked up to the cpu
+	 * using i2c and a single interrupt line. the interrupt line
+	 * is pulled low whenever someone taps the screen. to deassert
+	 * the interrupt line we need to acknowledge the interrupt by
+	 * communicating with the controller over the slow i2c bus.
+	 *
+	 * we can't acknowledge from interrupt context since the i2c
+	 * bus controller may sleep, so we just disable the interrupt
+	 * here and handle the acknowledge using delayed work.
+	 */
+
+	disable_irq_nosync(irq);
+	schedule_delayed_work(&priv->work, HZ / 20);
+
+	return IRQ_HANDLED;
+}
+
+
+static int migor_ts_open(struct input_dev *dev)
+{
+	struct migor_ts_priv *priv = input_get_drvdata(dev);
+	struct i2c_client *client = priv->client;
+	int count;
+
+	/* enable controller */
+	count = i2c_master_send(client, migor_ts_ena_seq,
+				sizeof(migor_ts_ena_seq));
+	if (count != sizeof(migor_ts_ena_seq)) {
+		dev_err(&client->dev, "Unable to enable touchscreen.\n");
+		return -ENXIO;
+	}
+
+	return 0;
+}
+
+static void migor_ts_close(struct input_dev *dev)
+{
+	struct migor_ts_priv *priv = input_get_drvdata(dev);
+	struct i2c_client *client = priv->client;
+
+	disable_irq(priv->irq);
+
+	/* cancel pending work and wait for migor_ts_poscheck() to finish */
+	if (cancel_delayed_work_sync(&priv->work)) {
+		/*
+		 * if migor_ts_poscheck was canceled we need to enable IRQ
+		 * here to balance disable done in migor_ts_isr.
+		 */
+		enable_irq(priv->irq);
+	}
+
+	/* disable controller */
+	i2c_master_send(client, migor_ts_dis_seq, sizeof(migor_ts_dis_seq));
+
+	enable_irq(priv->irq);
+}
+
+static int migor_ts_probe(struct i2c_client *client)
+{
+	struct migor_ts_priv *priv;
+	struct input_dev *input;
+	int error;
+
+	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
+	if (!priv) {
+		dev_err(&client->dev, "failed to allocate driver data\n");
+		error = -ENOMEM;
+		goto err0;
+	}
+
+	dev_set_drvdata(&client->dev, priv);
+
+	input = input_allocate_device();
+	if (!input) {
+		dev_err(&client->dev, "Failed to allocate input device.\n");
+		error = -ENOMEM;
+		goto err1;
+	}
+
+	input->evbit[0] = BIT_MASK(EV_KEY) | BIT_MASK(EV_ABS);
+	input->keybit[BIT_WORD(BTN_TOUCH)] = BIT_MASK(BTN_TOUCH);
+
+	input_set_abs_params(input, ABS_X, 95, 955, 0, 0);
+	input_set_abs_params(input, ABS_Y, 85, 935, 0, 0);
+
+	input->name = client->driver_name;
+	input->id.bustype = BUS_I2C;
+	input->dev.parent = &client->dev;
+
+	input->open = migor_ts_open;
+	input->close = migor_ts_close;
+
+	input_set_drvdata(input, priv);
+
+	priv->client = client;
+	priv->input = input;
+	INIT_DELAYED_WORK(&priv->work, migor_ts_poscheck);
+	priv->irq = client->irq;
+
+	error = input_register_device(input);
+	if (error)
+		goto err1;
+
+	error = request_irq(priv->irq, migor_ts_isr, IRQF_TRIGGER_LOW,
+			    client->driver_name, priv);
+	if (error) {
+		dev_err(&client->dev, "Unable to request touchscreen IRQ.\n");
+		goto err2;
+	}
+
+	return 0;
+
+ err2:
+	input_unregister_device(input);
+	input = NULL; /* so we dont try to free it below */
+ err1:
+	input_free_device(input);
+	kfree(priv);
+ err0:
+	dev_set_drvdata(&client->dev, NULL);
+	return error;
+}
+
+static int migor_ts_remove(struct i2c_client *client)
+{
+	struct migor_ts_priv *priv = dev_get_drvdata(&client->dev);
+
+	free_irq(priv->irq, priv);
+	input_unregister_device(priv->input);
+	kfree(priv);
+
+	dev_set_drvdata(&client->dev, NULL);
+
+	return 0;
+}
+
+static struct i2c_driver migor_ts_driver = {
+	.driver = {
+		.name = "migor_ts",
+	},
+	.probe = migor_ts_probe,
+	.remove = migor_ts_remove,
+};
+
+static int __init migor_ts_init(void)
+{
+	return i2c_add_driver(&migor_ts_driver);
+}
+
+static void __exit migor_ts_exit(void)
+{
+	i2c_del_driver(&migor_ts_driver);
+}
+
+MODULE_DESCRIPTION("MigoR Touchscreen driver");
+MODULE_AUTHOR("Magnus Damm <damm@opensource.se>");
+MODULE_LICENSE("GPL");
+
+module_init(migor_ts_init);
+module_exit(migor_ts_exit);

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

* Re: [PATCH] Touch screen driver for the SuperH MigoR board V2
  2008-04-27  5:07           ` Dmitry Torokhov
@ 2008-05-07 12:14             ` Magnus Damm
  0 siblings, 0 replies; 13+ messages in thread
From: Magnus Damm @ 2008-05-07 12:14 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: linux-input, linux-sh, lethal, broonie, akpm

Hi again Dmitry,

On Sun, Apr 27, 2008 at 2:07 PM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
>  Almost done ;) I also wanted to start the controller only when there are
>  users for the interface, by defining ->open and ->close methods for the
>  input device. Could you please take a look at the updated patch and if
>  it still works I will aplly it.

I just tested your improved version with evtest and it works just fine. Thanks!

/ magnus

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

* Re: [PATCH] Touch screen driver for the SuperH MigoR board
  2008-03-21 11:16 [PATCH] Touch screen driver for the SuperH MigoR board Magnus Damm
  2008-03-21 22:55 ` Andrew Morton
  2008-03-28  9:51 ` [PATCH] Touch screen driver for the SuperH MigoR board V2 Magnus Damm
@ 2008-07-01 10:33 ` Nicholas Beck
  2008-07-01 11:22   ` Magnus Damm
  2 siblings, 1 reply; 13+ messages in thread
From: Nicholas Beck @ 2008-07-01 10:33 UTC (permalink / raw)
  To: magnus.damm; +Cc: linux-sh, linux-input

Hi Magnus,

Firstly, thanks for the great work. I have one issue with the touch
screen driver for the MigoR board though.

I don't get any BTN_TOUCH release events using the touch screen driver
for the MigoR board. Do you experience the same problem?

I only see the first BTN_TOUCH down and then the absolute x-y pairs. The
input framework gobbles up the subsequent BTN_TOUCH down events until it
detects a BTN_TOUCH release event, as it should do. But the release
event does not occur at the moment (for me at least), so a tap cannot be
performed.

I have a patch that adds a timer to perform the BTN_TOUCH release event
(kind of like a WDT), that I can submit if this is of any interest?

Thanks in advance,
Nicholas Beck

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

* Re: [PATCH] Touch screen driver for the SuperH MigoR board
  2008-07-01 10:33 ` [PATCH] Touch screen driver for the SuperH MigoR board Nicholas Beck
@ 2008-07-01 11:22   ` Magnus Damm
  0 siblings, 0 replies; 13+ messages in thread
From: Magnus Damm @ 2008-07-01 11:22 UTC (permalink / raw)
  To: Nicholas Beck; +Cc: linux-sh, linux-input

Hi Nicholas,

Thanks for your email!

On Tue, Jul 1, 2008 at 7:33 PM, Nicholas Beck <nbeck@mpc-data.co.uk> wrote:
> Hi Magnus,
>
> Firstly, thanks for the great work. I have one issue with the touch
> screen driver for the MigoR board though.
>
> I don't get any BTN_TOUCH release events using the touch screen driver
> for the MigoR board. Do you experience the same problem?

This problem has been fixed by Dmitry Torokhov. The linux-input
archives seem gone though.

The updated patch below did not make it into 2.6.26, but it should be
included in 2.6.27.

http://git.kernel.org/?p=linux/kernel/git/dtor/input.git;a=commit;hˆ5c316d776b64728b4ed30e3af60d23c9e46825

> I have a patch that adds a timer to perform the BTN_TOUCH release event
> (kind of like a WDT), that I can submit if this is of any interest?

Does Dmitrys patch above solve your problem? If not, please post your
patch and I'll have a look.

Thank you!

/ magnus

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

end of thread, other threads:[~2008-07-01 11:22 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-03-21 11:16 [PATCH] Touch screen driver for the SuperH MigoR board Magnus Damm
2008-03-21 22:55 ` Andrew Morton
2008-03-26  4:03   ` Magnus Damm
2008-03-26 11:29     ` Mark Brown
2008-03-28  9:51 ` [PATCH] Touch screen driver for the SuperH MigoR board V2 Magnus Damm
2008-04-02  6:08   ` Dmitry Torokhov
2008-04-04  8:21     ` Magnus Damm
2008-04-04 16:14       ` Dmitry Torokhov
2008-04-25 10:29         ` Magnus Damm
2008-04-27  5:07           ` Dmitry Torokhov
2008-05-07 12:14             ` Magnus Damm
2008-07-01 10:33 ` [PATCH] Touch screen driver for the SuperH MigoR board Nicholas Beck
2008-07-01 11:22   ` Magnus Damm

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox