[PATCH] Touch screen driver for the SuperH MigoR board

[PATCH] Touch screen driver for the SuperH MigoR board

[Magnus Damm]

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);

Re: [PATCH] Touch screen driver for the SuperH MigoR board

[Andrew Morton]

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.

Re: [PATCH] Touch screen driver for the SuperH MigoR board

[Magnus Damm]

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

Re: [PATCH] Touch screen driver for the SuperH MigoR board

[Mark Brown]

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."

Re: [PATCH] Touch screen driver for the SuperH MigoR board

[Nicholas Beck]

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

Re: [PATCH] Touch screen driver for the SuperH MigoR board

[Magnus Damm]

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=885c316d776b64728b4ed30e3af60d23c9e46825

> 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