OLPC: touchpad driver (take 2)

This adds support for OLPC's touchpad. It has lots of neat features, none of which are enabled because the hardware is too buggy. Instead, we use it like a normal touchpad, but with a number of workarounds in place to deal with the frequent hardware spasms. Humidity changes, sweat, tinfoil underwear, plugging in AC, drinks, evil felines.. All tend to cause the touchpad to freak out.

Stuff like this keeps me from getting bored.

We make use of psmouse_set_state and psmouse_queue_work quite a bit.

(also, extension renamed from olpc.c to hgpk.c)

Signed-off-by Andres Salomon <[email protected]>

---
 drivers/input/mouse/Kconfig        |   10 +
 drivers/input/mouse/Makefile       |    1 +
 drivers/input/mouse/hgpk.c         |  492 ++++++++++++++++++++++++++++++++++++
 drivers/input/mouse/hgpk.h         |   48 ++++
 drivers/input/mouse/psmouse-base.c |   23 ++-
 drivers/input/mouse/psmouse.h      |    2 +-
 6 files changed, 574 insertions(+), 2 deletions(-)
 create mode 100644 drivers/input/mouse/hgpk.c
 create mode 100644 drivers/input/mouse/hgpk.h

drivers/input/mouse/Kconfig +10 -0

index 7bbea09..fff0253 100644
@@ -96,6 +96,16 @@ config MOUSE_PS2_TOUCHKIT
 
 	  If unsure, say N.
 
+config MOUSE_PS2_OLPC
+	bool "OLPC PS/2 mouse protocol extension"
+	depends on MOUSE_PS2 && OLPC
+	help
+	  Say Y here if you have an OLPC XO-1 laptop (with built-in
+	  PS/2 touchpad/tablet device).  The manufacturer calls the
+	  touchpad an HGPK.
+
+	  If unsure, say N.
+
 config MOUSE_SERIAL
 	tristate "Serial mouse"
 	select SERIO

drivers/input/mouse/Makefile +1 -0

index 9e6e363..799aa3f 100644
@@ -24,3 +24,4 @@ psmouse-$(CONFIG_MOUSE_PS2_LOGIPS2PP)	+= logips2pp.o
 psmouse-$(CONFIG_MOUSE_PS2_LIFEBOOK)	+= lifebook.o
 psmouse-$(CONFIG_MOUSE_PS2_TRACKPOINT)	+= trackpoint.o
 psmouse-$(CONFIG_MOUSE_PS2_TOUCHKIT)	+= touchkit_ps2.o
+psmouse-$(CONFIG_MOUSE_PS2_OLPC)	+= hgpk.o

drivers/input/mouse/hgpk.c new file +84 -0

new file mode 100644
index 0000000..ac7c2a8
@@ -0,0 +1,492 @@
+/*
+ * OLPC HGPK (XO-1) touchpad PS/2 mouse driver
+ *
+ * Copyright (c) 2006-2008 One Laptop Per Child
+ * Authors:
+ *   Zephaniah E. Hull
+ *   Andres Salomon <[email protected]>
+ *
+ * This driver is partly based on the ALPS driver, which is:
+ *
+ * Copyright (c) 2003 Neil Brown <[email protected]>
+ * Copyright (c) 2003-2005 Peter Osterlund <[email protected]>
+ * Copyright (c) 2004 Dmitry Torokhov <[email protected]>
+ * Copyright (c) 2005 Vojtech Pavlik <[email protected]>
+ *
+ * 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.
+ */
+
+/*
+ * The spec from ALPS is available from
+ * <http://wiki.laptop.org/go/Touch_Pad/Tablet>.  It refers to this
+ * device as HGPK (Hybrid GS, PT, and Keymatrix).
+ *
+ * The earliest versions of the device had simultaneous reporting; that
+ * was removed.  After that, the device used the Advanced Mode GS/PT streaming
+ * stuff.  That turned out to be too buggy to support, so we've finally
+ * switched to Mouse Mode (which utilizes only the center 1/3 of the touchpad).
+ */
+
+#define DEBUG
+#include <linux/input.h>
+#include <linux/serio.h>
+#include <linux/libps2.h>
+#include <linux/delay.h>
+#include <asm/olpc.h>
+
+#include "psmouse.h"
+#include "hgpk.h"
+
+static int tpdebug;
+module_param(tpdebug, int, 0644);
+MODULE_PARM_DESC(tpdebug, "enable debugging, dumping packets to KERN_DEBUG.");
+
+static int recalib_delta = 100;
+module_param(recalib_delta, int, 0644);
+MODULE_PARM_DESC(recalib_delta,
+	"packets containing a delta this large will cause a recalibration.");
+
+/*
+ * When the touchpad gets ultra-sensitive, one can keep their finger 1/2"
+ * above the pad and still have it send packets.  This causes a jump cursor
+ * when one places their finger on the pad.  We can probably detect the
+ * jump as we see a large deltas (>= 100px).  In mouse mode, I've been
+ * unable to even come close to 100px deltas during normal usage, so I think
+ * this threshold is safe.  If a large delta occurs, trigger a recalibration.
+ */
+static void hgpk_jumpy_hack(struct psmouse *psmouse, int x, int y)
+{
+	struct hgpk_data *priv = psmouse->private;
+
+	if (abs(x) > recalib_delta || abs(y) > recalib_delta) {
+		hgpk_err(psmouse, ">%dpx jump detected (%d,%d)\n",
+				recalib_delta, x, y);
+		/* My car gets forty rods to the hogshead and that's the
+		 * way I likes it! */
+		psmouse_queue_work(psmouse, &priv->recalib_wq,
+				msecs_to_jiffies(1000));
+	}
+}
+
+/*
+ * We have no idea why this particular hardware bug occurs.  The touchpad
+ * will randomly start spewing packets without anything touching the
+ * pad.  This wouldn't necessarily be bad, but it's indicative of a
+ * severely miscalibrated pad; attempting to use the touchpad while it's
+ * spewing means the cursor will jump all over the place, and act "drunk".
+ *
+ * The packets that are spewed tend to all have deltas between -2 and 2, and
+ * the cursor will move around without really going very far.  It will
+ * tend to end up in the same location; if we tally up the changes over
+ * 100 packets, we end up w/ a final delta of close to 0.  This happens
+ * pretty regularly when the touchpad is spewing, and is pretty hard to

drivers/input/mouse/hgpk.c new file +27 -0

+}
+
+static ssize_t hgpk_show_powered(struct device *dev,
+		struct device_attribute *attr, char *buf)
+{
+	struct serio *serio = to_serio_port(dev);
+	struct psmouse *psmouse;
+	struct hgpk_data *priv;
+	int retval;
+
+	retval = serio_pin_driver(serio);
+	if (retval)
+		return retval;
+
+	psmouse = serio_get_drvdata(serio);
+	priv = psmouse->private;
+
+	retval = sprintf(buf, "%d\n", priv->powered);
+	serio_unpin_driver(serio);
+	return retval;
+}
+
+static ssize_t hgpk_set_powered(struct device *dev,
+		struct device_attribute *attr, const char *buf, size_t count)
+{
+	struct serio *serio = to_serio_port(dev);
+	struct psmouse *psmouse;

drivers/input/mouse/hgpk.c new file +13 -0

+
+	retval = serio_pin_driver(serio);
+	if (retval)
+		return retval;
+
+	psmouse = serio_get_drvdata(serio);
+	priv = psmouse->private;
+
+	if (val == priv->powered)
+		goto done;
+
+	/* hgpk_toggle_power will deal w/ state so we're not racing w/ irq */
+	retval = hgpk_toggle_power(psmouse, val);

drivers/input/mouse/hgpk.c new file +5 -0

+	kfree(priv);
+}
+
 
+static void hgpk_recalib_work(struct work_struct *work)
+{

Hi Andres,

Quoted 29 lines

drivers/input/mouse/hgpk.c new file +3 -0

+
+/*
+ * We have no idea why this particular hardware bug occurs.  The

touchpad
+ * will randomly start spewing packets without anything touching the
+ * pad. This wouldn't necessarily be bad, but it's indicative of a
+ * severely miscalibrated pad; attempting to use the touchpad while it's
+ * spewing means the cursor will jump all over the place, and act "drunk".
+ *
+ * The packets that are spewed tend to all have deltas between -2 and 2, and
+ * the cursor will move around without really going very far. It will
+ * tend to end up in the same location; if we tally up the changes over
+ * 100 packets, we end up w/ a final delta of close to 0. This happens
+ * pretty regularly when the touchpad is spewing, and is pretty hard to
+ * manually trigger (at least for *my* fingers). So, it makes a perfect
+ * scheme for detecting spews.
+ */
+static void hgpk_spewing_hack(struct psmouse *psmouse, int l, int r, int x,

drivers/input/mouse/hgpk.c new file +1 -0

+		int y)

drivers/input/mouse/hgpk.c line 94

Quoted 5 lines

drivers/input/mouse/hgpk.c new file +5 -0

+{
+	struct hgpk_data *priv = psmouse->private;
+	static int count;
+	static int x_tally;
+	static int y_tally;

Hmm, I guess we'll never see 2 such devices in one box, but still moving it to psmouse->private woudl be a good practice.

drivers/input/mouse/hgpk.c line 305

Quoted 12 lines

drivers/input/mouse/hgpk.c new file +5 -0

+
+static int hgpk_reconnect(struct psmouse *psmouse)
+{
+	if (olpc_board_at_least(olpc_board(0xb2)))
+		if

(psmouse->ps2dev.serio->dev.power.power_state.event !=

drivers/input/mouse/hgpk.c new file +5 -0

+				PM_EVENT_ON)
+			return 0;
+
+	psmouse_reset(psmouse);
+	return 0;

We should not return success if we can't detect the same hardware. I think if olpc_board_at_least(olpc_board(0xb2) fails we need to return -1;

Quoted 31 lines

drivers/input/mouse/hgpk.c new file +24 -0

+}
+
+static ssize_t hgpk_show_powered(struct device *dev,
+		struct device_attribute *attr, char *buf)
+{
+	struct serio *serio = to_serio_port(dev);
+	struct psmouse *psmouse;
+	struct hgpk_data *priv;
+	int retval;
+
+	retval = serio_pin_driver(serio);
+	if (retval)
+		return retval;
+
+	psmouse = serio_get_drvdata(serio);
+	priv = psmouse->private;
+
+	retval = sprintf(buf, "%d\n", priv->powered);
+	serio_unpin_driver(serio);
+	return retval;
+}
+
+static ssize_t hgpk_set_powered(struct device *dev,
+		struct device_attribute *attr, const char *buf,

size_t count) +{

drivers/input/mouse/hgpk.c new file +6 -0

+	struct serio *serio = to_serio_port(dev);
+	struct psmouse *psmouse;
+	struct hgpk_data *priv;
+	unsigned long val;
+	int retval;
+

drivers/input/mouse/hgpk.c line 342

Quoted 6 lines

drivers/input/mouse/hgpk.c new file +6 -0

+	if (*buf == '1')
+		val = 1;
+	else if (*buf == '0')
+		val = 0;
+	else
+		return -EINVAL;

I'd use strict_strtoul() and checked for val <= 1.

Quoted 19 lines

drivers/input/mouse/hgpk.c new file +12 -0

+
+	retval = serio_pin_driver(serio);
+	if (retval)
+		return retval;
+
+	psmouse = serio_get_drvdata(serio);
+	priv = psmouse->private;
+
+	if (val == priv->powered)
+		goto done;
+
+	/* hgpk_toggle_power will deal w/ state so we're not

racing w/ irq */

drivers/input/mouse/hgpk.c new file +6 -0

+	retval = hgpk_toggle_power(psmouse, val);
+	if (!retval)
+		priv->powered = val;
+
+done:
+	serio_unpin_driver(serio);

drivers/input/mouse/hgpk.c line 365

Quoted 6 lines

drivers/input/mouse/hgpk.c new file +5 -0

+	return retval ? retval : count;
+}
+
+static DEVICE_ATTR(powered, S_IWUSR | S_IRUGO, hgpk_show_powered,
+		hgpk_set_powered);

Any particular reason you didn't use PSMOUSE_DEFINE_ATTR? Then you

would not need to bother with pinning the driver and provode mutual exclusion with other things. Btw, what do we do if device is powered

down an user tries to change some settings via generic attributes defined in psmouse-base?

+

drivers/input/mouse/hgpk.c line 373

Quoted 8 lines

drivers/input/mouse/hgpk.c new file +5 -0

+static void hgpk_disconnect(struct psmouse *psmouse)
+{
+	struct hgpk_data *priv = psmouse->private;
+
+	device_remove_file(&psmouse->ps2dev.serio->dev,

&dev_attr_powered);

drivers/input/mouse/hgpk.c new file +2 -0

+	psmouse_reset(psmouse);
+	flush_scheduled_work();

What does this flush protect from? We are using specialized workqueue. Plus psmouse_disconnect should take care of this, shouldn't it?

Quoted 15 lines

drivers/input/mouse/hgpk.c new file +6 -0

+	kfree(priv);
+}
+
 
+static void hgpk_recalib_work(struct work_struct *work)
+{
+	struct delayed_work *w = container_of(work, struct

delayed_work, work);

(quoted diff) +1 -0

+	struct hgpk_data *priv = container_of(w, struct hgpk_data,

recalib_wq);

drivers/input/mouse/hgpk.c new file +5 -0

+	struct psmouse *psmouse = priv->psmouse;
+
+	hgpk_dbg(psmouse, "recalibrating touchpad..\n");
+
+	if (hgpk_force_recalibrate(psmouse))

drivers/input/mouse/hgpk.c line 388

Quoted 3 lines

drivers/input/mouse/hgpk.c new file +3 -0

+		hgpk_err(psmouse, "recalibration failed!\n");
+}
+

Overall I am concerned about mutual exclusions between operations, such as settin power state and adjusting genericmouse properties (repeaqt rate, resolution, etc). Psmouse-base usues psmouse_mutex for that, mayne olps should do that too...

Quoted 11 lines

drivers/input/mouse/hgpk.c line 94

Quoted 8 lines

+static void hgpk_spewing_hack(struct psmouse *psmouse, int l, int r, int x,

drivers/input/mouse/hgpk.c new file +6 -0

+		int y)
+{
+	struct hgpk_data *priv = psmouse->private;
+	static int count;
+	static int x_tally;
+	static int y_tally;

Hmm, I guess we'll never see 2 such devices in one box, but still moving it to psmouse->private woudl be a good practice.

Can you please clarify this comment?

Hi Andres,

On Wed, Aug 13, 2008 at 03:24:59PM -0400, Andres Salomon wrote:

Quoted 24 lines

drivers/input/mouse/hgpk.c new file +12 -0

+
+	retval = serio_pin_driver(serio);
+	if (retval)
+		return retval;
+
+	psmouse = serio_get_drvdata(serio);
+	priv = psmouse->private;
+
+	if (val == priv->powered)
+		goto done;
+
+	/* hgpk_toggle_power will deal w/ state so we're not

racing w/ irq */

drivers/input/mouse/hgpk.c new file +10 -0

+	retval = hgpk_toggle_power(psmouse, val);
+	if (!retval)
+		priv->powered = val;
+
+done:
+	serio_unpin_driver(serio);
+	return retval ? retval : count;
+}
+
+static DEVICE_ATTR(powered, S_IWUSR | S_IRUGO, hgpk_show_powered,

drivers/input/mouse/hgpk.c line 365

Quoted 2 lines

drivers/input/mouse/hgpk.c new file +1 -0

+		hgpk_set_powered);

Any particular reason you didn't use PSMOUSE_DEFINE_ATTR? Then you would not need to bother with pinning the driver and provode mutual exclusion with other things. Btw, what do we do if device is powered down an user tries to change some settings via generic attributes defined in psmouse-base?

What do you think about either changing PSMOUSE_DEFINE_ATTR to take an additional 'raw' argument (meaning psmouse->state is not checked, and psmouse_deactivate/psmouse_activate are not called), or alternatively adding new helper functions that just handle the locking (__PSMOUSE_DEFINE_ATTR() and __psmouse_attr_{set,show}_helper())? I'd prefer the raw argument.

From f009e1d3da0266ec99437c243482026c96c9103c Mon Sep 17 00:00:00 2001

On Wed, 10 Sep 2008 17:55:13 -0400
Andres Salomon <[email protected]> wrote:
[...]

Quoted 9 lines

What do you think about either changing PSMOUSE_DEFINE_ATTR to take an additional 'raw' argument (meaning psmouse->state is not checked, and psmouse_deactivate/psmouse_activate are not called), or alternatively adding new helper functions that just handle the locking (__PSMOUSE_DEFINE_ATTR() and __psmouse_attr_{set,show}_helper())? I'd prefer the raw argument.

Ie, something like this (I'll resend all patches later if you approve).

And here's the current touchpad driver after updating based upon your comments.

Quoted 4 lines · Andres Salomon

And here's the current touchpad driver after updating based upon your comments.

Looks good. Care to respin it all thogether based on the attribute change I requested so I can be sure I'm applying the latest version of everything? Ummm....

drivers/input/mouse/Kconfig config MOUSE_PS2_TOUCHKIT · line 101

Quoted 11 lines

(quoted diff) +0 -0

b/drivers/input/mouse/Kconfig index 7bbea09..fff0253 100644

drivers/input/mouse/Kconfig +3 -0

@@ -96,6 +96,16 @@ config MOUSE_PS2_TOUCHKIT
 
 	  If unsure, say N.
 
+config MOUSE_PS2_OLPC
+	bool "OLPC PS/2 mouse protocol extension"
+	depends on MOUSE_PS2 && OLPC

Are you sure it shoudl depend on OLPC? It looks like driver should work on any box with PS/2 port. I understand that only OLPC has the hardware installed but can this change?

Thanks!

Quoted 4 lines · Andres Salomon

And here's the current touchpad driver after updating based upon your comments.

Looks good. Care to respin it all thogether based on the attribute change I requested so I can be sure I'm applying the latest version of everything? Ummm....

Quoted 6 lines · On Thu, 11 Sep 2008 09:05:22 -0400 Dmitry Torokhov <[email protected]>…

Quoted 4 lines · Andres Salomon

And here's the current touchpad driver after updating based upon your comments.

Quoted 4 lines

Looks good. Care to respin it all thogether based on the attribute change I requested so I can be sure I'm applying the latest version of everything? Ummm....

Here's the patch. This is on top of the other 2 psmouse patches that were previously sent.

drivers/input/mouse/psmouse.h +1 -1

@@ -114,7 +115,7 @@ ssize_t psmouse_attr_show_helper(struct device *dev, struct device_attribute *at
 ssize_t psmouse_attr_set_helper(struct device *dev, struct device_attribute *attr,
 				const char *buf, size_t count);
 
-#define PSMOUSE_DEFINE_ATTR(_name, _mode, _data, _show, _set)			\
+#define PSMOUSE_DEFINE_ATTR(_name, _mode, _data, _protected_set, _show, _set)	\
 static ssize_t _show(struct psmouse *, void *data, char *);			\
 static ssize_t _set(struct psmouse *, void *data, const char *, size_t);	\
 static struct psmouse_attribute psmouse_attr_##_name = {			\