Subject: [RPC] OLPC tablet input driver.

The OLPC will ship with a somewhat unique input device made by ALPS,
connected via PS/2 and speaking a protocol only loosely based on that
spoken by other ALPS devices.

This is required by the noticeable different between this device and
others made by alps, specificly that it is very wide, with the center
1/3rd usable with the GS sensor, and the entire area usable with the PT
sensor, with support for using both at once.

The protocol differs enough that I split the driver for this off from
the ALPS driver.

The patch is below, but there are a few things of note.

1: Cosmetic: Some line lengths, and outputs with debugging enabled, are
over 80 columns wide. Will be fixed in the next version of this patch.

2: Cosmetic: Input device IDs need to be decided on, some feedback on
the best values to use here would be appreciated.

3: Patch stuff: Because the protocol uses 9 byte packets I had to
increase the size of the buffer in struct psmouse. Should this be split
off into a separate patch?

4: Technical/policy: Buttons are currently sent to both of the input
devices we generate, I don't see any way to avoid this that is not a
policy decision on which buttons belong to which device, but I'm open to
suggestions.

5: Technical: Min/max on absolute values are currently reported as the
protocol limits (10 bits on GS X, GS Y, and PT Y. 11 bits on PT X. 7
bits on GS pressure). Until we get samples based on the newer design
and do some testing to see how big the variations are, we just don't
have any numbers to put here.

6: Technical, maybe: The early samples I have that speak this protocol
are doing some odd things with this driver. Mostly in the realm of
sample rate and pressure reporting. I'm fairly sure that this is
hardware related, but it's worth mentioning.


That said, here the patch is for comments.
(And possibly for the OLPC kernel tree for others with samples to play
with.)


Signed-off-by: Zephaniah E. Hull <[email protected]>

diff --git a/drivers/input/mouse/Makefile b/drivers/input/mouse/Makefile
index 21a1de6..6218e5a 100644
--- a/drivers/input/mouse/Makefile
+++ b/drivers/input/mouse/Makefile
@@ -14,4 +14,4 @@ obj-$(CONFIG_MOUSE_SERIAL) += sermouse.o
obj-$(CONFIG_MOUSE_HIL) += hil_ptr.o
obj-$(CONFIG_MOUSE_VSXXXAA) += vsxxxaa.o

-psmouse-objs := psmouse-base.o alps.o logips2pp.o synaptics.o lifebook.o trackpoint.o
+psmouse-objs := psmouse-base.o alps.o logips2pp.o synaptics.o lifebook.o trackpoint.o olpc.o
diff --git a/drivers/input/mouse/olpc.c b/drivers/input/mouse/olpc.c
new file mode 100644
index 0000000..245f29e
--- /dev/null
+++ b/drivers/input/mouse/olpc.c
@@ -0,0 +1,327 @@
+/*
+ * OLPC touchpad PS/2 mouse driver
+ *
+ * Copyright (c) 2006 One Laptop Per Child, inc.
+ * Author Zephaniah E. Hull.
+ *
+ * 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 touchpad on the OLPC is fairly wide, with the entire area usable
+ * as a tablet, and the center 1/3rd also usable as a touchpad.
+ *
+ * The device has simultanious reporting, so that both can be used at once.
+ *
+ * The PT+GS protocol is similar to the base ALPS protocol, in that the
+ * GS data is where the ALPS parser would expect to find it, however
+ * there are several additional bytes, the button bits are in a
+ * different byte, and the bits used for finger and gesture indication
+ * are replaced by two bits which indicate if it is reporting PT or GS
+ * coordinate data in that packet.
+ */
+
+#include <linux/input.h>
+#include <linux/serio.h>
+#include <linux/libps2.h>
+
+#include "psmouse.h"
+#include "olpc.h"
+
+#undef DEBUG
+#ifdef DEBUG
+#define dbg(format, arg...) printk(KERN_INFO "olpc.c(%d): " format "\n", __LINE__, ## arg)
+#else
+#define dbg(format, arg...) do {} while (0)
+#endif
+
+#define OLPC_PT 0x01
+#define OLPC_GS 0x02
+#define OLPC_PTGS 0x04
+
+static struct olpc_model_info olpc_model_data[] = {
+ { { 0x67, 0x00, 0x0a }, 0xeb, 0xff, OLPC_PTGS }, /* OLPC in PT+GS mode. */
+};
+
+/*
+ * OLPC absolute Mode - new format
+ *
+ * byte 0: 1 ? ? ? 1 ? ? ?
+ * byte 1: 0 x6 x5 x4 x3 x2 x1 x0
+ * byte 2: 0 x10 x9 x8 x7 ? fin ges
+ * byte 3: 0 y9 y8 y7 1 0 R L
+ * byte 4: 0 y6 y5 y4 y3 y2 y1 y0
+ * byte 5: 0 z6 z5 z4 z3 z2 z1 z0
+ *
+ * ?'s can have different meanings on different models,
+ * such as wheel rotation, extra buttons, stick buttons
+ * on a dualpoint, etc.
+ */
+
+static void olpc_process_packet(struct psmouse *psmouse, struct pt_regs *regs)
+{
+ struct olpc_data *priv = psmouse->private;
+ unsigned char *packet = psmouse->packet;
+ struct input_dev *dev = psmouse->dev;
+ struct input_dev *dev2 = priv->dev2;
+ int px, py, gx, gy, gz, gs_down, pt_down, left, right;
+
+ input_regs(dev, regs);
+
+ left = packet[6] & 1;
+ right = packet[6] & 2;
+ gx = packet[1] | ((packet[2] & 0x78) << (7 - 3));
+ gy = packet[4] | ((packet[3] & 0x70) << (7 - 4));
+ gz = packet[5];
+ px = packet[8] | ((packet[2] & 0x7) << 7);
+ py = packet[7] | ((packet[6] & 0x70) << (7 - 4));
+
+ pt_down = packet[3] & 1;
+ gs_down = packet[3] & 2;
+
+ input_report_key(dev, BTN_LEFT, left);
+ input_report_key(dev2, BTN_LEFT, left);
+ input_report_key(dev, BTN_RIGHT, right);
+ input_report_key(dev2, BTN_RIGHT, right);
+
+ input_report_key(dev, BTN_TOUCH, pt_down);
+ input_report_key(dev, BTN_TOOL_PEN, pt_down);
+ input_report_key(dev2, BTN_TOUCH, gs_down);
+ input_report_key(dev2, BTN_TOOL_FINGER, gs_down);
+
+ if (gs_down) {
+ input_report_abs(dev2, ABS_X, gx);
+ input_report_abs(dev2, ABS_Y, gy);
+ }
+ input_report_abs(dev2, ABS_PRESSURE, gz);
+
+ if (pt_down) {
+ input_report_abs(dev, ABS_X, px);
+ input_report_abs(dev, ABS_Y, py);
+ }
+
+ input_sync(dev);
+}
+
+static psmouse_ret_t olpc_process_byte(struct psmouse *psmouse, struct pt_regs *regs)
+{
+ struct olpc_data *priv = psmouse->private;
+ psmouse_ret_t ret = PSMOUSE_BAD_DATA;
+
+ if ((psmouse->packet[0] & priv->i->mask0) != priv->i->byte0) {
+ ret = PSMOUSE_BAD_DATA;
+ goto out;
+ }
+
+ /* Bytes 2 - 6 should have 0 in the highest bit */
+ if (psmouse->pktcnt >= 2 && psmouse->pktcnt <= 9 &&
+ (psmouse->packet[psmouse->pktcnt - 1] & 0x80)) {
+ ret = PSMOUSE_BAD_DATA;
+ goto out;
+ }
+
+ if ((psmouse->pktcnt == 4 || psmouse->pktcnt == 7) &&
+ ((psmouse->packet[psmouse->pktcnt - 1] & 0x88) != 8)) {
+ ret = PSMOUSE_BAD_DATA;
+ goto out;
+ }
+
+ if (psmouse->pktcnt == 9) {
+ olpc_process_packet(psmouse, regs);
+
+ ret = PSMOUSE_FULL_PACKET;
+ goto out;
+ }
+
+ ret = PSMOUSE_GOOD_DATA;
+out:
+ if (ret != PSMOUSE_GOOD_DATA)
+ dbg("ret: %d, len: %u, data: %2.2x %2.2x %2.2x %2.2x %2.2x %2.2x %2.2x %2.2x %2.2x", ret, psmouse->pktcnt,
+ psmouse->packet[0], psmouse->packet[1], psmouse->packet[2],
+ psmouse->packet[3], psmouse->packet[4], psmouse->packet[5],
+ psmouse->packet[6], psmouse->packet[7], psmouse->packet[8]);
+ return ret;
+}
+
+static struct olpc_model_info *olpc_get_model(struct psmouse *psmouse)
+{
+ struct ps2dev *ps2dev = &psmouse->ps2dev;
+ unsigned char param[4];
+ int i;
+
+ /*
+ * Now try "E7 report". Allowed responses are in
+ * olpc_model_data[].signature
+ */
+ if (ps2_command(ps2dev, NULL, PSMOUSE_CMD_SETSCALE21) ||
+ ps2_command(ps2dev, NULL, PSMOUSE_CMD_SETSCALE21) ||
+ ps2_command(ps2dev, NULL, PSMOUSE_CMD_SETSCALE21))
+ return NULL;
+
+ param[0] = param[1] = param[2] = 0xff;
+ if (ps2_command(ps2dev, param, PSMOUSE_CMD_GETINFO))
+ return NULL;
+
+ dbg("E7 report: %2.2x %2.2x %2.2x", param[0], param[1], param[2]);
+
+ for (i = 0; i < ARRAY_SIZE(olpc_model_data); i++)
+ if (!memcmp(param, olpc_model_data[i].signature, sizeof(olpc_model_data[i].signature)))
+ return olpc_model_data + i;
+
+ return NULL;
+}
+
+static int olpc_absolute_mode(struct psmouse *psmouse)
+{
+ struct ps2dev *ps2dev = &psmouse->ps2dev;
+ unsigned char param;
+
+ /* Switch to 'Advanced mode.', four disables in a row. */
+ if (ps2_command(ps2dev, NULL, PSMOUSE_CMD_DISABLE) ||
+ ps2_command(ps2dev, NULL, PSMOUSE_CMD_DISABLE) ||
+ ps2_command(ps2dev, NULL, PSMOUSE_CMD_DISABLE) ||
+ ps2_command(ps2dev, NULL, PSMOUSE_CMD_DISABLE))
+ return -1;
+
+ /*
+ * Switch to simultanious mode, F2 (GETID) three times with no
+ * arguments or reply, followed by SETRES with an argument of 2.
+ */
+ ps2_command(ps2dev, NULL, 0xF2);
+ ps2_command(ps2dev, NULL, 0xF2);
+ ps2_command(ps2dev, NULL, 0xF2);
+ param = 0x02;
+ ps2_command(ps2dev, &param, PSMOUSE_CMD_SETRES);
+
+ return 0;
+}
+
+/*
+ * olpc_poll() - poll the touchpad for current motion packet.
+ * Used in resync.
+ */
+static int olpc_poll(struct psmouse *psmouse)
+{
+ /*
+ * FIXME: We can't poll, find a way to make resync work better.
+ */
+ return 0;
+}
+
+static int olpc_reconnect(struct psmouse *psmouse)
+{
+ struct olpc_data *priv = psmouse->private;
+
+ psmouse_reset(psmouse);
+
+ if (!(priv->i = olpc_get_model(psmouse)))
+ return -1;
+
+ if (olpc_absolute_mode(psmouse)) {
+ printk(KERN_ERR "olpc.c: Failed to reenable absolute mode\n");
+ return -1;
+ }
+
+ return 0;
+}
+
+static void olpc_disconnect(struct psmouse *psmouse)
+{
+ struct olpc_data *priv = psmouse->private;
+
+ psmouse_reset(psmouse);
+ input_unregister_device(priv->dev2);
+ kfree(priv);
+}
+
+int olpc_init(struct psmouse *psmouse)
+{
+ struct olpc_data *priv;
+ struct input_dev *dev = psmouse->dev;
+ struct input_dev *dev2;
+
+ psmouse->private = priv = kzalloc(sizeof(struct olpc_data), GFP_KERNEL);
+ dev2 = input_allocate_device();
+ if (!priv || !dev2)
+ goto init_fail;
+
+ priv->dev2 = dev2;
+
+ if (!(priv->i = olpc_get_model(psmouse)))
+ goto init_fail;
+
+ if (olpc_absolute_mode(psmouse)) {
+ printk(KERN_ERR "olpc.c: Failed to enable absolute mode\n");
+ goto init_fail;
+ }
+
+ dev->evbit[LONG(EV_KEY)] |= BIT(EV_KEY);
+ dev->keybit[LONG(BTN_TOUCH)] |= BIT(BTN_TOUCH);
+ dev->keybit[LONG(BTN_TOOL_PEN)] |= BIT(BTN_TOOL_PEN);
+ dev->keybit[LONG(BTN_LEFT)] |= BIT(BTN_LEFT) | BIT(BTN_RIGHT);
+
+ dev->evbit[LONG(EV_ABS)] |= BIT(EV_ABS);
+ input_set_abs_params(dev, ABS_X, 0, 1023, 0, 0);
+ input_set_abs_params(dev, ABS_Y, 0, 1023, 0, 0);
+
+ snprintf(priv->phys, sizeof(priv->phys), "%s/input1", psmouse->ps2dev.serio->phys);
+ dev2->phys = priv->phys;
+ dev2->name = "OLPC OLPC GlideSensor";
+ dev2->id.bustype = BUS_I8042;
+ dev2->id.vendor = 0x0002;
+ dev2->id.product = PSMOUSE_OLPC;
+ dev2->id.version = 0x0000;
+
+ dev2->evbit[LONG(EV_KEY)] |= BIT(EV_KEY);
+ dev2->evbit[LONG(EV_ABS)] |= BIT(EV_ABS);
+ input_set_abs_params(dev2, ABS_X, 0, 2047, 0, 0);
+ input_set_abs_params(dev2, ABS_Y, 0, 1023, 0, 0);
+ input_set_abs_params(dev2, ABS_PRESSURE, 0, 63, 0, 0);
+ dev2->keybit[LONG(BTN_TOUCH)] |= BIT(BTN_TOUCH);
+ dev2->keybit[LONG(BTN_TOOL_FINGER)] |= BIT(BTN_TOOL_FINGER);
+ dev2->keybit[LONG(BTN_LEFT)] |= BIT(BTN_LEFT) | BIT(BTN_RIGHT);
+
+ input_register_device(priv->dev2);
+
+
+ psmouse->protocol_handler = olpc_process_byte;
+ psmouse->poll = olpc_poll;
+ psmouse->disconnect = olpc_disconnect;
+ psmouse->reconnect = olpc_reconnect;
+ psmouse->pktsize = 9;
+
+ /* We are having trouble resyncing OLPC touchpads so disable it for now */
+ psmouse->resync_time = 0;
+
+ return 0;
+
+init_fail:
+ input_free_device(dev2);
+ kfree(priv);
+ return -1;
+}
+
+int olpc_detect(struct psmouse *psmouse, int set_properties)
+{
+ struct olpc_model_info *model;
+
+ if (!(model = olpc_get_model(psmouse)))
+ return -1;
+
+ if (set_properties) {
+ psmouse->vendor = "OLPC";
+ psmouse->name = "PenTablet";
+ psmouse->model = 0;
+ }
+ return 0;
+}
+
diff --git a/drivers/input/mouse/olpc.h b/drivers/input/mouse/olpc.h
new file mode 100644
index 0000000..49f4e3e
--- /dev/null
+++ b/drivers/input/mouse/olpc.h
@@ -0,0 +1,37 @@
+/*
+ * OLPC touchpad PS/2 mouse driver
+ *
+ * Copyright (c) 2006 One Laptop Per Child, inc.
+ *
+ * This driver is partly based on the ALPS driver.
+ * Copyright (c) 2003 Peter Osterlund <[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.
+ */
+
+#ifndef _OLPC_H
+#define _OLPC_H
+
+int olpc_detect(struct psmouse *psmouse, int set_properties);
+int olpc_init(struct psmouse *psmouse);
+
+struct olpc_model_info {
+ unsigned char signature[3];
+ unsigned char byte0, mask0;
+ unsigned char flags;
+};
+
+struct olpc_data {
+ struct input_dev *dev2; /* Relative device */
+ char name[32]; /* Name */
+ char phys[32]; /* Phys */
+ struct olpc_model_info *i; /* Info */
+ int prev_fin_pt; /* Finger bit from previous packet */
+ int prev_fin_gs; /* Finger bit from previous packet */
+};
+
+
+#endif
diff --git a/drivers/input/mouse/psmouse-base.c b/drivers/input/mouse/psmouse-base.c
index 8bc9f51..20060b0 100644
--- a/drivers/input/mouse/psmouse-base.c
+++ b/drivers/input/mouse/psmouse-base.c
@@ -26,6 +26,7 @@
#include "synaptics.h"
#include "logips2pp.h"
#include "alps.h"
+#include "olpc.h"
#include "lifebook.h"
#include "trackpoint.h"

@@ -616,6 +617,15 @@ static int psmouse_extensions(struct psm
*/
max_proto = PSMOUSE_IMEX;
}
+ ps2_command(&psmouse->ps2dev, NULL, PSMOUSE_CMD_RESET_DIS);
+ if (olpc_detect(psmouse, set_properties) == 0) {
+ if (!set_properties || olpc_init(psmouse) == 0)
+ return PSMOUSE_OLPC;
+/*
+ * Init failed, try basic relative protocols
+ */
+ max_proto = PSMOUSE_IMEX;
+ }
}

if (max_proto > PSMOUSE_IMEX && genius_detect(psmouse, set_properties) == 0)
@@ -726,6 +736,13 @@ static struct psmouse_protocol psmouse_p
.detect = trackpoint_detect,
},
{
+ .type = PSMOUSE_OLPC,
+ .name = "OLPC",
+ .alias = "olpc",
+ .maxproto = 1,
+ .detect = olpc_detect,
+ },
+ {
.type = PSMOUSE_AUTO,
.name = "auto",
.alias = "any",
diff --git a/drivers/input/mouse/psmouse.h b/drivers/input/mouse/psmouse.h
index 4d9107f..f3d7199 100644
--- a/drivers/input/mouse/psmouse.h
+++ b/drivers/input/mouse/psmouse.h
@@ -42,7 +42,7 @@ struct psmouse {
struct work_struct resync_work;
char *vendor;
char *name;
- unsigned char packet[8];
+ unsigned char packet[9];
unsigned char badbyte;
unsigned char pktcnt;
unsigned char pktsize;
@@ -86,6 +86,7 @@ enum psmouse_type {
PSMOUSE_ALPS,
PSMOUSE_LIFEBOOK,
PSMOUSE_TRACKPOINT,
+ PSMOUSE_OLPC,
PSMOUSE_AUTO /* This one should always be last */
};

--
1024D/E65A7801 Zephaniah E. Hull <[email protected]>
92ED 94E4 B1E6 3624 226D 5727 4453 008B E65A 7801
CCs of replies from mailing lists are requested.


Attachments:
(No filename) (14.73 kB)
signature.asc (189.00 B)
Digital signature
Download all attachments

2006-08-29 08:10:58

by Arjan van de Ven

[permalink] [raw]
Subject: Re: [RPC] OLPC tablet input driver.

> +#undef DEBUG
> +#ifdef DEBUG
> +#define dbg(format, arg...) printk(KERN_INFO "olpc.c(%d): " format "\n", __LINE__, ## arg)
> +#else
> +#define dbg(format, arg...) do {} while (0)
> +#endif

why not use pr_debug or even dev_debug() ?
Those already have this ifdef included

> +
> +static struct olpc_model_info olpc_model_data[] = {
> + { { 0x67, 0x00, 0x0a }, 0xeb, 0xff, OLPC_PTGS }, /* OLPC in PT+GS mode. */
> +};

const?



also.. there's no locking visible anywhere in the driver... is this
right?

Greetings,
Arjan van de Ven

Subject: Re: [RPC] OLPC tablet input driver.

On Tue, Aug 29, 2006 at 10:10:19AM +0200, Arjan van de Ven wrote:
> > +#undef DEBUG
> > +#ifdef DEBUG
> > +#define dbg(format, arg...) printk(KERN_INFO "olpc.c(%d): " format "\n", __LINE__, ## arg)
> > +#else
> > +#define dbg(format, arg...) do {} while (0)
> > +#endif
>
> why not use pr_debug or even dev_debug() ?
> Those already have this ifdef included

I was not thinking of them at the time, however dev_dbg is not an option
because we do not have a struct device at hand when we want to print
some debugging lines.

pr_debug might work, but I would rather have file and line already
there.

Though, admittedly, that would be a better argument if it used __FILE__
there instead of hard coding it.

In any case, I don't think any of the debug prints will have to stick
around that much longer.
>
> > +
> > +static struct olpc_model_info olpc_model_data[] = {
> > + { { 0x67, 0x00, 0x0a }, 0xeb, 0xff, OLPC_PTGS }, /* OLPC in PT+GS mode. */
> > +};
>
> const?

Added.
(Along with associated changes so that it's kept const everywhere.)
>
> also.. there's no locking visible anywhere in the driver... is this
> right?

It looks like psmouse handles it with a mutex lock around freeing stuff
and calling the callback function pointers we set on init, so we
_should_ be safe unless I've missed something.

Add to it that none of the other psmouse drivers are doing locking on
their own, and I'm fairly sure that this is correct. (But if someone
knows better, please correct me.)


Thank you.

Zephaniah E. Hull.

--
1024D/E65A7801 Zephaniah E. Hull <[email protected]>
92ED 94E4 B1E6 3624 226D 5727 4453 008B E65A 7801
CCs of replies from mailing lists are requested.

Mike Sphar (Scary Devil Monastery):
>I am hired because I know what I am doing, not because I will do
>whatever I am told is a good idea. This might cost me bonuses, raises,
>promotions, and may even label me as "undesirable" by places I don't
>want to work at anyway, but I don't care. I will not compromise my own
>principles and judgement without putting up a fight. Of course, I
>won't always win, and I will sometimes be forced to do things I don't
>agree with, but if I am my objections will be known, and if I am shown
>to be right and problems later develop, I will shout "I told you so!"
>repeatedly, laugh hysterically, and do a small dance or jig as
>appropriate to my heritage.


Attachments:
(No filename) (2.33 kB)
signature.asc (189.00 B)
Digital signature
Download all attachments

2006-08-29 08:55:38

by Komal Shah

[permalink] [raw]
Subject: Re: [RPC] OLPC tablet input driver.

--- "Zephaniah E. Hull" <[email protected]> wrote:

>
>
> That said, here the patch is for comments.
> (And possibly for the OLPC kernel tree for others with samples to
> play
> with.)
>
>
> Signed-off-by: Zephaniah E. Hull <[email protected]>
>
> diff --git a/drivers/input/mouse/Makefile
> b/drivers/input/mouse/Makefile
> index 21a1de6..6218e5a 100644
> --- a/drivers/input/mouse/Makefile
> +++ b/drivers/input/mouse/Makefile
> @@ -14,4 +14,4 @@ obj-$(CONFIG_MOUSE_SERIAL) += sermouse.o
> obj-$(CONFIG_MOUSE_HIL) += hil_ptr.o
> obj-$(CONFIG_MOUSE_VSXXXAA) += vsxxxaa.o
>
> -psmouse-objs := psmouse-base.o alps.o logips2pp.o synaptics.o
> lifebook.o trackpoint.o
> +psmouse-objs := psmouse-base.o alps.o logips2pp.o synaptics.o
> lifebook.o trackpoint.o olpc.o

Where is KConfigurable entry ?

> diff --git a/drivers/input/mouse/olpc.c b/drivers/input/mouse/olpc.c
> new file mode 100644
> index 0000000..245f29e
> --- /dev/null
> +++ b/drivers/input/mouse/olpc.c
> @@ -0,0 +1,327 @@


> +/*
> + * OLPC touchpad PS/2 mouse driver
> + *
> +int olpc_init(struct psmouse *psmouse)
> +{
> + struct olpc_data *priv;
> + struct input_dev *dev = psmouse->dev;
> + struct input_dev *dev2;
> +
> + psmouse->private = priv = kzalloc(sizeof(struct olpc_data),
> GFP_KERNEL);

I think you should assign priv to private only if !NULL.

> + dev2 = input_allocate_device();
> + if (!priv || !dev2)
> + goto init_fail;
> +
> + priv->dev2 = dev2;
> +
> + if (!(priv->i = olpc_get_model(psmouse)))
> + goto init_fail;
> +
> + if (olpc_absolute_mode(psmouse)) {
> + printk(KERN_ERR "olpc.c: Failed to enable absolute mode\n");
> + goto init_fail;
> + }
> +
> + dev->evbit[LONG(EV_KEY)] |= BIT(EV_KEY);
> + dev->keybit[LONG(BTN_TOUCH)] |= BIT(BTN_TOUCH);
> + dev->keybit[LONG(BTN_TOOL_PEN)] |= BIT(BTN_TOOL_PEN);
> + dev->keybit[LONG(BTN_LEFT)] |= BIT(BTN_LEFT) | BIT(BTN_RIGHT);
> +
> + dev->evbit[LONG(EV_ABS)] |= BIT(EV_ABS);
> + input_set_abs_params(dev, ABS_X, 0, 1023, 0, 0);
> + input_set_abs_params(dev, ABS_Y, 0, 1023, 0, 0);
> +
> + snprintf(priv->phys, sizeof(priv->phys), "%s/input1",
> psmouse->ps2dev.serio->phys);
> + dev2->phys = priv->phys;
> + dev2->name = "OLPC OLPC GlideSensor";
> + dev2->id.bustype = BUS_I8042;
> + dev2->id.vendor = 0x0002;
> + dev2->id.product = PSMOUSE_OLPC;
> + dev2->id.version = 0x0000;
> +
> + dev2->evbit[LONG(EV_KEY)] |= BIT(EV_KEY);
> + dev2->evbit[LONG(EV_ABS)] |= BIT(EV_ABS);
> + input_set_abs_params(dev2, ABS_X, 0, 2047, 0, 0);
> + input_set_abs_params(dev2, ABS_Y, 0, 1023, 0, 0);
> + input_set_abs_params(dev2, ABS_PRESSURE, 0, 63, 0, 0);
> + dev2->keybit[LONG(BTN_TOUCH)] |= BIT(BTN_TOUCH);
> + dev2->keybit[LONG(BTN_TOOL_FINGER)] |= BIT(BTN_TOOL_FINGER);
> + dev2->keybit[LONG(BTN_LEFT)] |= BIT(BTN_LEFT) | BIT(BTN_RIGHT);
> +
> + input_register_device(priv->dev2);

Please check the return value of input_register_device and its friends.

> +
> +
> + psmouse->protocol_handler = olpc_process_byte;
> + psmouse->poll = olpc_poll;
> + psmouse->disconnect = olpc_disconnect;
> + psmouse->reconnect = olpc_reconnect;
> + psmouse->pktsize = 9;
> +
> + /* We are having trouble resyncing OLPC touchpads so disable it for
> now */
> + psmouse->resync_time = 0;
> +
> + return 0;
> +
> +init_fail:
> + input_free_device(dev2);
> + kfree(priv);
> + return -1;
> +}
> +


---Komal Shah
http://komalshah.blogspot.com/

__________________________________________________
Do You Yahoo!?
Tired of spam? Yahoo! Mail has the best spam protection around
http://mail.yahoo.com

2006-08-29 09:00:37

by Komal Shah

[permalink] [raw]
Subject: Re: [RPC] OLPC tablet input driver.

--- "Zephaniah E. Hull" <[email protected]> wrote:

>
>
> That said, here the patch is for comments.
> (And possibly for the OLPC kernel tree for others with samples to
> play
> with.)
>
>
> Signed-off-by: Zephaniah E. Hull <[email protected]>
>
> diff --git a/drivers/input/mouse/Makefile
> b/drivers/input/mouse/Makefile
> index 21a1de6..6218e5a 100644
> --- a/drivers/input/mouse/Makefile
> +++ b/drivers/input/mouse/Makefile
> @@ -14,4 +14,4 @@ obj-$(CONFIG_MOUSE_SERIAL) += sermouse.o
> obj-$(CONFIG_MOUSE_HIL) += hil_ptr.o
> obj-$(CONFIG_MOUSE_VSXXXAA) += vsxxxaa.o
>
> -psmouse-objs := psmouse-base.o alps.o logips2pp.o synaptics.o
> lifebook.o trackpoint.o
> +psmouse-objs := psmouse-base.o alps.o logips2pp.o synaptics.o
> lifebook.o trackpoint.o olpc.o

Where is KConfigurable entry ?

> diff --git a/drivers/input/mouse/olpc.c b/drivers/input/mouse/olpc.c
> new file mode 100644
> index 0000000..245f29e
> --- /dev/null
> +++ b/drivers/input/mouse/olpc.c
> @@ -0,0 +1,327 @@


> +/*
> + * OLPC touchpad PS/2 mouse driver
> + *
> +int olpc_init(struct psmouse *psmouse)
> +{
> + struct olpc_data *priv;
> + struct input_dev *dev = psmouse->dev;
> + struct input_dev *dev2;
> +
> + psmouse->private = priv = kzalloc(sizeof(struct olpc_data),
> GFP_KERNEL);

I think you should assign priv to private only if !NULL.

> + dev2 = input_allocate_device();
> + if (!priv || !dev2)
> + goto init_fail;
> +
> + priv->dev2 = dev2;
> +
> + if (!(priv->i = olpc_get_model(psmouse)))
> + goto init_fail;
> +
> + if (olpc_absolute_mode(psmouse)) {
> + printk(KERN_ERR "olpc.c: Failed to enable absolute mode\n");
> + goto init_fail;
> + }
> +
> + dev->evbit[LONG(EV_KEY)] |= BIT(EV_KEY);
> + dev->keybit[LONG(BTN_TOUCH)] |= BIT(BTN_TOUCH);
> + dev->keybit[LONG(BTN_TOOL_PEN)] |= BIT(BTN_TOOL_PEN);
> + dev->keybit[LONG(BTN_LEFT)] |= BIT(BTN_LEFT) | BIT(BTN_RIGHT);
> +
> + dev->evbit[LONG(EV_ABS)] |= BIT(EV_ABS);
> + input_set_abs_params(dev, ABS_X, 0, 1023, 0, 0);
> + input_set_abs_params(dev, ABS_Y, 0, 1023, 0, 0);
> +
> + snprintf(priv->phys, sizeof(priv->phys), "%s/input1",
> psmouse->ps2dev.serio->phys);
> + dev2->phys = priv->phys;
> + dev2->name = "OLPC OLPC GlideSensor";
> + dev2->id.bustype = BUS_I8042;
> + dev2->id.vendor = 0x0002;
> + dev2->id.product = PSMOUSE_OLPC;
> + dev2->id.version = 0x0000;
> +
> + dev2->evbit[LONG(EV_KEY)] |= BIT(EV_KEY);
> + dev2->evbit[LONG(EV_ABS)] |= BIT(EV_ABS);
> + input_set_abs_params(dev2, ABS_X, 0, 2047, 0, 0);
> + input_set_abs_params(dev2, ABS_Y, 0, 1023, 0, 0);
> + input_set_abs_params(dev2, ABS_PRESSURE, 0, 63, 0, 0);
> + dev2->keybit[LONG(BTN_TOUCH)] |= BIT(BTN_TOUCH);
> + dev2->keybit[LONG(BTN_TOOL_FINGER)] |= BIT(BTN_TOOL_FINGER);
> + dev2->keybit[LONG(BTN_LEFT)] |= BIT(BTN_LEFT) | BIT(BTN_RIGHT);
> +
> + input_register_device(priv->dev2);

Please check the return value of input_register_device and its friends.

> +
> +
> + psmouse->protocol_handler = olpc_process_byte;
> + psmouse->poll = olpc_poll;
> + psmouse->disconnect = olpc_disconnect;
> + psmouse->reconnect = olpc_reconnect;
> + psmouse->pktsize = 9;
> +
> + /* We are having trouble resyncing OLPC touchpads so disable it for
> now */
> + psmouse->resync_time = 0;
> +
> + return 0;
> +
> +init_fail:
> + input_free_device(dev2);
> + kfree(priv);
> + return -1;
> +}
> +


---Komal Shah
http://komalshah.blogspot.com/

__________________________________________________
Do You Yahoo!?
Tired of spam? Yahoo! Mail has the best spam protection around
http://mail.yahoo.com

Subject: Re: [RPC] OLPC tablet input driver.

On Tue, Aug 29, 2006 at 01:55:37AM -0700, Komal Shah wrote:
> --- "Zephaniah E. Hull" <[email protected]> wrote:
>
> >
> >
> > That said, here the patch is for comments.
> > (And possibly for the OLPC kernel tree for others with samples to
> > play
> > with.)
> >
> >
> > Signed-off-by: Zephaniah E. Hull <[email protected]>
> >
> > diff --git a/drivers/input/mouse/Makefile
> > b/drivers/input/mouse/Makefile
> > index 21a1de6..6218e5a 100644
> > --- a/drivers/input/mouse/Makefile
> > +++ b/drivers/input/mouse/Makefile
> > @@ -14,4 +14,4 @@ obj-$(CONFIG_MOUSE_SERIAL) += sermouse.o
> > obj-$(CONFIG_MOUSE_HIL) += hil_ptr.o
> > obj-$(CONFIG_MOUSE_VSXXXAA) += vsxxxaa.o
> >
> > -psmouse-objs := psmouse-base.o alps.o logips2pp.o synaptics.o
> > lifebook.o trackpoint.o
> > +psmouse-objs := psmouse-base.o alps.o logips2pp.o synaptics.o
> > lifebook.o trackpoint.o olpc.o
>
> Where is KConfigurable entry ?

It is a component of psmouse.o, which is a few lines up.

Breaking out the components of psmouse.o into separate configuration
items might be interesting, but it is quite a bit beyond the scope of
this patch.
>
> > diff --git a/drivers/input/mouse/olpc.c b/drivers/input/mouse/olpc.c
> > new file mode 100644
> > index 0000000..245f29e
> > --- /dev/null
> > +++ b/drivers/input/mouse/olpc.c
> > @@ -0,0 +1,327 @@
>
>
> > +/*
> > + * OLPC touchpad PS/2 mouse driver
> > + *
> > +int olpc_init(struct psmouse *psmouse)
> > +{
> > + struct olpc_data *priv;
> > + struct input_dev *dev = psmouse->dev;
> > + struct input_dev *dev2;
> > +
> > + psmouse->private = priv = kzalloc(sizeof(struct olpc_data),
> > GFP_KERNEL);
>
> I think you should assign priv to private only if !NULL.

Fixed.

It should not actually matter, as a failure to get a !NULL value causes
us to return false, which will fall over to other psmouse drivers which
will either set it themselves, or not use it at all, however.

It should be noted that alps.c contains the same issue.

> > + input_register_device(priv->dev2);
>
> Please check the return value of input_register_device and its friends.

Alright, added to my todo, should have it done by the next patch
revision.


Thank you very much.
Zephaniah E. Hull.

--
1024D/E65A7801 Zephaniah E. Hull <[email protected]>
92ED 94E4 B1E6 3624 226D 5727 4453 008B E65A 7801
CCs of replies from mailing lists are requested.

"And now, little kittens, we're going to run across red-hot
motherboards, with our bare feet." -- Buzh.


Attachments:
(No filename) (2.43 kB)
signature.asc (189.00 B)
Digital signature
Download all attachments

2006-08-29 12:26:49

by Dmitry Torokhov

[permalink] [raw]
Subject: Re: [RPC] OLPC tablet input driver.

On 8/29/06, Zephaniah E. Hull <[email protected]> wrote:
> On Tue, Aug 29, 2006 at 01:55:37AM -0700, Komal Shah wrote:
> > --- "Zephaniah E. Hull" <[email protected]> wrote:
> > >
> > > -psmouse-objs := psmouse-base.o alps.o logips2pp.o synaptics.o
> > > lifebook.o trackpoint.o
> > > +psmouse-objs := psmouse-base.o alps.o logips2pp.o synaptics.o
> > > lifebook.o trackpoint.o olpc.o
> >
> > Where is KConfigurable entry ?
>
> It is a component of psmouse.o, which is a few lines up.
>
> Breaking out the components of psmouse.o into separate configuration
> items might be interesting, but it is quite a bit beyond the scope of
> this patch.

Is there a chance that this device will be used on generally-available
hardware? If not then having Kconfig sub-option would be nice.

> >
> > > diff --git a/drivers/input/mouse/olpc.c b/drivers/input/mouse/olpc.c
> > > new file mode 100644
> > > index 0000000..245f29e
> > > --- /dev/null
> > > +++ b/drivers/input/mouse/olpc.c
> > > @@ -0,0 +1,327 @@
> >
> >
> > > +/*
> > > + * OLPC touchpad PS/2 mouse driver
> > > + *
> > > +int olpc_init(struct psmouse *psmouse)
> > > +{
> > > + struct olpc_data *priv;
> > > + struct input_dev *dev = psmouse->dev;
> > > + struct input_dev *dev2;
> > > +
> > > + psmouse->private = priv = kzalloc(sizeof(struct olpc_data),
> > > GFP_KERNEL);
> >
> > I think you should assign priv to private only if !NULL.
>
> Fixed.
>
> It should not actually matter, as a failure to get a !NULL value causes
> us to return false, which will fall over to other psmouse drivers which
> will either set it themselves, or not use it at all, however.
>
> It should be noted that alps.c contains the same issue.
>

I do not consider this is an issue. priv is just a shortcut or alias
for psmouse->private that saves some typing and allows the compiled to
generate better code.

--
Dmitry

2006-08-29 12:29:17

by Dmitry Torokhov

[permalink] [raw]
Subject: Re: [RPC] OLPC tablet input driver.

On 8/29/06, Zephaniah E. Hull <[email protected]> wrote:
> >
> > also.. there's no locking visible anywhere in the driver... is this
> > right?
>
> It looks like psmouse handles it with a mutex lock around freeing stuff
> and calling the callback function pointers we set on init, so we
> _should_ be safe unless I've missed something.
>
> Add to it that none of the other psmouse drivers are doing locking on
> their own, and I'm fairly sure that this is correct. (But if someone
> knows better, please correct me.)
>

Serio and psmouse cores should handle all necessary locking, no worries here.

--
Dmitry

2006-08-29 12:53:21

by Dmitry Torokhov

[permalink] [raw]
Subject: Re: [RPC] OLPC tablet input driver.

Hi,

On 8/29/06, Zephaniah E. Hull <[email protected]> wrote:
> The OLPC will ship with a somewhat unique input device made by ALPS,
> connected via PS/2 and speaking a protocol only loosely based on that
> spoken by other ALPS devices.
>

Do you have a formal programming spec for it?

> This is required by the noticeable different between this device and
> others made by alps, specificly that it is very wide, with the center
> 1/3rd usable with the GS sensor, and the entire area usable with the PT
> sensor, with support for using both at once.
>

Coudl youp please tell me what GS and PT stand for?

> The protocol differs enough that I split the driver for this off from
> the ALPS driver.
>
> The patch is below, but there are a few things of note.
>
> 1: Cosmetic: Some line lengths, and outputs with debugging enabled, are
> over 80 columns wide. Will be fixed in the next version of this patch.
>

If going to 80 colums will require monstrocities like this:

... do_bla(struct_b->
array_g[
index_i].
submember);

(and I seen quite a few such attempts to "improve" code) then please don't ;)

> 2: Cosmetic: Input device IDs need to be decided on, some feedback on
> the best values to use here would be appreciated.

I think what you've done is fine.

>
> 3: Patch stuff: Because the protocol uses 9 byte packets I had to
> increase the size of the buffer in struct psmouse. Should this be split
> off into a separate patch?
>

No.

> 4: Technical/policy: Buttons are currently sent to both of the input
> devices we generate, I don't see any way to avoid this that is not a
> policy decision on which buttons belong to which device, but I'm open to
> suggestions.
>

Is it not known how actual hardware wired?

> 5: Technical: Min/max on absolute values are currently reported as the
> protocol limits (10 bits on GS X, GS Y, and PT Y. 11 bits on PT X. 7
> bits on GS pressure). Until we get samples based on the newer design
> and do some testing to see how big the variations are, we just don't
> have any numbers to put here.
>

Using protocol limits is fine, I don't think anyone actually uses it.
Synaptics X driver allows users to tweak it to their preference.

> 6: Technical, maybe: The early samples I have that speak this protocol
> are doing some odd things with this driver. Mostly in the realm of
> sample rate and pressure reporting. I'm fairly sure that this is
> hardware related, but it's worth mentioning.
>
>
> That said, here the patch is for comments.
> (And possibly for the OLPC kernel tree for others with samples to play
> with.)
>
>
> Signed-off-by: Zephaniah E. Hull <[email protected]>
>
> diff --git a/drivers/input/mouse/Makefile b/drivers/input/mouse/Makefile
> index 21a1de6..6218e5a 100644
> --- a/drivers/input/mouse/Makefile
> +++ b/drivers/input/mouse/Makefile
> @@ -14,4 +14,4 @@ obj-$(CONFIG_MOUSE_SERIAL) += sermouse.o
> obj-$(CONFIG_MOUSE_HIL) += hil_ptr.o
> obj-$(CONFIG_MOUSE_VSXXXAA) += vsxxxaa.o
>
> -psmouse-objs := psmouse-base.o alps.o logips2pp.o synaptics.o lifebook.o trackpoint.o
> +psmouse-objs := psmouse-base.o alps.o logips2pp.o synaptics.o lifebook.o trackpoint.o olpc.o
> diff --git a/drivers/input/mouse/olpc.c b/drivers/input/mouse/olpc.c
> new file mode 100644
> index 0000000..245f29e
> --- /dev/null
> +++ b/drivers/input/mouse/olpc.c
> @@ -0,0 +1,327 @@
> +/*
> + * OLPC touchpad PS/2 mouse driver
> + *
> + * Copyright (c) 2006 One Laptop Per Child, inc.
> + * Author Zephaniah E. Hull.
> + *
> + * 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 touchpad on the OLPC is fairly wide, with the entire area usable
> + * as a tablet, and the center 1/3rd also usable as a touchpad.
> + *
> + * The device has simultanious reporting, so that both can be used at once.
> + *
> + * The PT+GS protocol is similar to the base ALPS protocol, in that the
> + * GS data is where the ALPS parser would expect to find it, however
> + * there are several additional bytes, the button bits are in a
> + * different byte, and the bits used for finger and gesture indication
> + * are replaced by two bits which indicate if it is reporting PT or GS
> + * coordinate data in that packet.
> + */
> +
> +#include <linux/input.h>
> +#include <linux/serio.h>
> +#include <linux/libps2.h>
> +
> +#include "psmouse.h"
> +#include "olpc.h"
> +
> +#undef DEBUG
> +#ifdef DEBUG
> +#define dbg(format, arg...) printk(KERN_INFO "olpc.c(%d): " format "\n", __LINE__, ## arg)
> +#else
> +#define dbg(format, arg...) do {} while (0)
> +#endif
> +
> +#define OLPC_PT 0x01
> +#define OLPC_GS 0x02
> +#define OLPC_PTGS 0x04
> +

Do you need a separate #define? I'd expect it to be OLPC_PT | OLPC_GS?

> +static struct olpc_model_info olpc_model_data[] = {
> + { { 0x67, 0x00, 0x0a }, 0xeb, 0xff, OLPC_PTGS }, /* OLPC in PT+GS mode. */
> +};
> +
> +/*
> + * OLPC absolute Mode - new format
> + *
> + * byte 0: 1 ? ? ? 1 ? ? ?
> + * byte 1: 0 x6 x5 x4 x3 x2 x1 x0
> + * byte 2: 0 x10 x9 x8 x7 ? fin ges
> + * byte 3: 0 y9 y8 y7 1 0 R L
> + * byte 4: 0 y6 y5 y4 y3 y2 y1 y0
> + * byte 5: 0 z6 z5 z4 z3 z2 z1 z0
> + *
> + * ?'s can have different meanings on different models,
> + * such as wheel rotation, extra buttons, stick buttons
> + * on a dualpoint, etc.
> + */
> +
> +static void olpc_process_packet(struct psmouse *psmouse, struct pt_regs *regs)
> +{
> + struct olpc_data *priv = psmouse->private;
> + unsigned char *packet = psmouse->packet;
> + struct input_dev *dev = psmouse->dev;
> + struct input_dev *dev2 = priv->dev2;
> + int px, py, gx, gy, gz, gs_down, pt_down, left, right;
> +
> + input_regs(dev, regs);
> +
> + left = packet[6] & 1;
> + right = packet[6] & 2;
> + gx = packet[1] | ((packet[2] & 0x78) << (7 - 3));
> + gy = packet[4] | ((packet[3] & 0x70) << (7 - 4));
> + gz = packet[5];
> + px = packet[8] | ((packet[2] & 0x7) << 7);
> + py = packet[7] | ((packet[6] & 0x70) << (7 - 4));
> +
> + pt_down = packet[3] & 1;
> + gs_down = packet[3] & 2;
> +
> + input_report_key(dev, BTN_LEFT, left);
> + input_report_key(dev2, BTN_LEFT, left);
> + input_report_key(dev, BTN_RIGHT, right);
> + input_report_key(dev2, BTN_RIGHT, right);
> +
> + input_report_key(dev, BTN_TOUCH, pt_down);
> + input_report_key(dev, BTN_TOOL_PEN, pt_down);
> + input_report_key(dev2, BTN_TOUCH, gs_down);
> + input_report_key(dev2, BTN_TOOL_FINGER, gs_down);
> +
> + if (gs_down) {
> + input_report_abs(dev2, ABS_X, gx);
> + input_report_abs(dev2, ABS_Y, gy);
> + }
> + input_report_abs(dev2, ABS_PRESSURE, gz);
> +
> + if (pt_down) {
> + input_report_abs(dev, ABS_X, px);
> + input_report_abs(dev, ABS_Y, py);
> + }
> +
> + input_sync(dev);
> +}
> +
> +static psmouse_ret_t olpc_process_byte(struct psmouse *psmouse, struct pt_regs *regs)
> +{
> + struct olpc_data *priv = psmouse->private;
> + psmouse_ret_t ret = PSMOUSE_BAD_DATA;
> +
> + if ((psmouse->packet[0] & priv->i->mask0) != priv->i->byte0) {
> + ret = PSMOUSE_BAD_DATA;
> + goto out;
> + }
> +
> + /* Bytes 2 - 6 should have 0 in the highest bit */
> + if (psmouse->pktcnt >= 2 && psmouse->pktcnt <= 9 &&
> + (psmouse->packet[psmouse->pktcnt - 1] & 0x80)) {
> + ret = PSMOUSE_BAD_DATA;
> + goto out;
> + }
> +
> + if ((psmouse->pktcnt == 4 || psmouse->pktcnt == 7) &&
> + ((psmouse->packet[psmouse->pktcnt - 1] & 0x88) != 8)) {
> + ret = PSMOUSE_BAD_DATA;
> + goto out;
> + }
> +
> + if (psmouse->pktcnt == 9) {
> + olpc_process_packet(psmouse, regs);
> +
> + ret = PSMOUSE_FULL_PACKET;
> + goto out;
> + }
> +
> + ret = PSMOUSE_GOOD_DATA;
> +out:
> + if (ret != PSMOUSE_GOOD_DATA)
> + dbg("ret: %d, len: %u, data: %2.2x %2.2x %2.2x %2.2x %2.2x %2.2x %2.2x %2.2x %2.2x", ret, psmouse->pktcnt,
> + psmouse->packet[0], psmouse->packet[1], psmouse->packet[2],
> + psmouse->packet[3], psmouse->packet[4], psmouse->packet[5],
> + psmouse->packet[6], psmouse->packet[7], psmouse->packet[8]);
> + return ret;
> +}
> +
> +static struct olpc_model_info *olpc_get_model(struct psmouse *psmouse)
> +{
> + struct ps2dev *ps2dev = &psmouse->ps2dev;
> + unsigned char param[4];
> + int i;
> +
> + /*
> + * Now try "E7 report". Allowed responses are in
> + * olpc_model_data[].signature
> + */
> + if (ps2_command(ps2dev, NULL, PSMOUSE_CMD_SETSCALE21) ||
> + ps2_command(ps2dev, NULL, PSMOUSE_CMD_SETSCALE21) ||
> + ps2_command(ps2dev, NULL, PSMOUSE_CMD_SETSCALE21))
> + return NULL;
> +
> + param[0] = param[1] = param[2] = 0xff;
> + if (ps2_command(ps2dev, param, PSMOUSE_CMD_GETINFO))
> + return NULL;
> +
> + dbg("E7 report: %2.2x %2.2x %2.2x", param[0], param[1], param[2]);
> +
> + for (i = 0; i < ARRAY_SIZE(olpc_model_data); i++)
> + if (!memcmp(param, olpc_model_data[i].signature, sizeof(olpc_model_data[i].signature)))
> + return olpc_model_data + i;
> +
> + return NULL;
> +}
> +
> +static int olpc_absolute_mode(struct psmouse *psmouse)
> +{
> + struct ps2dev *ps2dev = &psmouse->ps2dev;
> + unsigned char param;
> +
> + /* Switch to 'Advanced mode.', four disables in a row. */
> + if (ps2_command(ps2dev, NULL, PSMOUSE_CMD_DISABLE) ||
> + ps2_command(ps2dev, NULL, PSMOUSE_CMD_DISABLE) ||
> + ps2_command(ps2dev, NULL, PSMOUSE_CMD_DISABLE) ||
> + ps2_command(ps2dev, NULL, PSMOUSE_CMD_DISABLE))
> + return -1;
> +
> + /*
> + * Switch to simultanious mode, F2 (GETID) three times with no
> + * arguments or reply, followed by SETRES with an argument of 2.
> + */
> + ps2_command(ps2dev, NULL, 0xF2);
> + ps2_command(ps2dev, NULL, 0xF2);
> + ps2_command(ps2dev, NULL, 0xF2);
> + param = 0x02;
> + ps2_command(ps2dev, &param, PSMOUSE_CMD_SETRES);
> +
> + return 0;
> +}
> +
> +/*
> + * olpc_poll() - poll the touchpad for current motion packet.
> + * Used in resync.
> + */
> +static int olpc_poll(struct psmouse *psmouse)
> +{
> + /*
> + * FIXME: We can't poll, find a way to make resync work better.
> + */
> + return 0;
> +}

I'd expect it to return -1. It's OK that it can't poll, it looks like
it should resync fairly well on its own.

> +
> +static int olpc_reconnect(struct psmouse *psmouse)
> +{
> + struct olpc_data *priv = psmouse->private;
> +
> + psmouse_reset(psmouse);
> +
> + if (!(priv->i = olpc_get_model(psmouse)))
> + return -1;
> +
> + if (olpc_absolute_mode(psmouse)) {
> + printk(KERN_ERR "olpc.c: Failed to reenable absolute mode\n");
> + return -1;
> + }
> +
> + return 0;
> +}
> +
> +static void olpc_disconnect(struct psmouse *psmouse)
> +{
> + struct olpc_data *priv = psmouse->private;
> +
> + psmouse_reset(psmouse);
> + input_unregister_device(priv->dev2);
> + kfree(priv);
> +}
> +
> +int olpc_init(struct psmouse *psmouse)
> +{
> + struct olpc_data *priv;
> + struct input_dev *dev = psmouse->dev;
> + struct input_dev *dev2;
> +
> + psmouse->private = priv = kzalloc(sizeof(struct olpc_data), GFP_KERNEL);
> + dev2 = input_allocate_device();
> + if (!priv || !dev2)
> + goto init_fail;
> +
> + priv->dev2 = dev2;
> +
> + if (!(priv->i = olpc_get_model(psmouse)))
> + goto init_fail;
> +
> + if (olpc_absolute_mode(psmouse)) {
> + printk(KERN_ERR "olpc.c: Failed to enable absolute mode\n");
> + goto init_fail;
> + }
> +
> + dev->evbit[LONG(EV_KEY)] |= BIT(EV_KEY);
> + dev->keybit[LONG(BTN_TOUCH)] |= BIT(BTN_TOUCH);
> + dev->keybit[LONG(BTN_TOOL_PEN)] |= BIT(BTN_TOOL_PEN);
> + dev->keybit[LONG(BTN_LEFT)] |= BIT(BTN_LEFT) | BIT(BTN_RIGHT);
> +
> + dev->evbit[LONG(EV_ABS)] |= BIT(EV_ABS);
> + input_set_abs_params(dev, ABS_X, 0, 1023, 0, 0);
> + input_set_abs_params(dev, ABS_Y, 0, 1023, 0, 0);
> +
> + snprintf(priv->phys, sizeof(priv->phys), "%s/input1", psmouse->ps2dev.serio->phys);
> + dev2->phys = priv->phys;
> + dev2->name = "OLPC OLPC GlideSensor";

"OLPC OLPC"?

> + dev2->id.bustype = BUS_I8042;
> + dev2->id.vendor = 0x0002;
> + dev2->id.product = PSMOUSE_OLPC;
> + dev2->id.version = 0x0000;
> +
> + dev2->evbit[LONG(EV_KEY)] |= BIT(EV_KEY);
> + dev2->evbit[LONG(EV_ABS)] |= BIT(EV_ABS);
> + input_set_abs_params(dev2, ABS_X, 0, 2047, 0, 0);
> + input_set_abs_params(dev2, ABS_Y, 0, 1023, 0, 0);
> + input_set_abs_params(dev2, ABS_PRESSURE, 0, 63, 0, 0);
> + dev2->keybit[LONG(BTN_TOUCH)] |= BIT(BTN_TOUCH);
> + dev2->keybit[LONG(BTN_TOOL_FINGER)] |= BIT(BTN_TOOL_FINGER);
> + dev2->keybit[LONG(BTN_LEFT)] |= BIT(BTN_LEFT) | BIT(BTN_RIGHT);
> +
> + input_register_device(priv->dev2);
> +
> +
> + psmouse->protocol_handler = olpc_process_byte;
> + psmouse->poll = olpc_poll;
> + psmouse->disconnect = olpc_disconnect;
> + psmouse->reconnect = olpc_reconnect;
> + psmouse->pktsize = 9;
> +
> + /* We are having trouble resyncing OLPC touchpads so disable it for now */
> + psmouse->resync_time = 0;
> +
> + return 0;
> +
> +init_fail:
> + input_free_device(dev2);
> + kfree(priv);
> + return -1;
> +}
> +
> +int olpc_detect(struct psmouse *psmouse, int set_properties)
> +{
> + struct olpc_model_info *model;
> +
> + if (!(model = olpc_get_model(psmouse)))
> + return -1;
> +
> + if (set_properties) {
> + psmouse->vendor = "OLPC";
> + psmouse->name = "PenTablet";
> + psmouse->model = 0;
> + }
> + return 0;
> +}
> +
> diff --git a/drivers/input/mouse/olpc.h b/drivers/input/mouse/olpc.h
> new file mode 100644
> index 0000000..49f4e3e
> --- /dev/null
> +++ b/drivers/input/mouse/olpc.h
> @@ -0,0 +1,37 @@
> +/*
> + * OLPC touchpad PS/2 mouse driver
> + *
> + * Copyright (c) 2006 One Laptop Per Child, inc.
> + *
> + * This driver is partly based on the ALPS driver.
> + * Copyright (c) 2003 Peter Osterlund <[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.
> + */
> +
> +#ifndef _OLPC_H
> +#define _OLPC_H
> +
> +int olpc_detect(struct psmouse *psmouse, int set_properties);
> +int olpc_init(struct psmouse *psmouse);
> +
> +struct olpc_model_info {
> + unsigned char signature[3];
> + unsigned char byte0, mask0;
> + unsigned char flags;
> +};
> +
> +struct olpc_data {
> + struct input_dev *dev2; /* Relative device */
> + char name[32]; /* Name */
> + char phys[32]; /* Phys */
> + struct olpc_model_info *i; /* Info */
> + int prev_fin_pt; /* Finger bit from previous packet */
> + int prev_fin_gs; /* Finger bit from previous packet */

Bad (duplicate) comment?

> +};
> +
> +
> +#endif
> diff --git a/drivers/input/mouse/psmouse-base.c b/drivers/input/mouse/psmouse-base.c
> index 8bc9f51..20060b0 100644
> --- a/drivers/input/mouse/psmouse-base.c
> +++ b/drivers/input/mouse/psmouse-base.c
> @@ -26,6 +26,7 @@
> #include "synaptics.h"
> #include "logips2pp.h"
> #include "alps.h"
> +#include "olpc.h"
> #include "lifebook.h"
> #include "trackpoint.h"
>
> @@ -616,6 +617,15 @@ static int psmouse_extensions(struct psm
> */
> max_proto = PSMOUSE_IMEX;
> }
> + ps2_command(&psmouse->ps2dev, NULL, PSMOUSE_CMD_RESET_DIS);
> + if (olpc_detect(psmouse, set_properties) == 0) {
> + if (!set_properties || olpc_init(psmouse) == 0)
> + return PSMOUSE_OLPC;
> +/*
> + * Init failed, try basic relative protocols
> + */
> + max_proto = PSMOUSE_IMEX;
> + }
> }
>
> if (max_proto > PSMOUSE_IMEX && genius_detect(psmouse, set_properties) == 0)
> @@ -726,6 +736,13 @@ static struct psmouse_protocol psmouse_p
> .detect = trackpoint_detect,
> },
> {
> + .type = PSMOUSE_OLPC,
> + .name = "OLPC",
> + .alias = "olpc",
> + .maxproto = 1,
> + .detect = olpc_detect,
> + },
> + {
> .type = PSMOUSE_AUTO,
> .name = "auto",
> .alias = "any",
> diff --git a/drivers/input/mouse/psmouse.h b/drivers/input/mouse/psmouse.h
> index 4d9107f..f3d7199 100644
> --- a/drivers/input/mouse/psmouse.h
> +++ b/drivers/input/mouse/psmouse.h
> @@ -42,7 +42,7 @@ struct psmouse {
> struct work_struct resync_work;
> char *vendor;
> char *name;
> - unsigned char packet[8];
> + unsigned char packet[9];
> unsigned char badbyte;
> unsigned char pktcnt;
> unsigned char pktsize;
> @@ -86,6 +86,7 @@ enum psmouse_type {
> PSMOUSE_ALPS,
> PSMOUSE_LIFEBOOK,
> PSMOUSE_TRACKPOINT,
> + PSMOUSE_OLPC,
> PSMOUSE_AUTO /* This one should always be last */
> };
>
> --
> 1024D/E65A7801 Zephaniah E. Hull <[email protected]>
> 92ED 94E4 B1E6 3624 226D 5727 4453 008B E65A 7801
> CCs of replies from mailing lists are requested.
>
>
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.5 (GNU/Linux)
>
> iD8DBQFE8+3TRFMAi+ZaeAERAhiNAJ4t4uF3q9G+bdsGUAYDafNearwMUgCeN0kl
> se5meohSaoJEMhbRsrxtIOo=
> =Vd5o
> -----END PGP SIGNATURE-----
>
>
>


--
Dmitry

Subject: Re: [RPC] OLPC tablet input driver.

On Tue, Aug 29, 2006 at 08:53:17AM -0400, Dmitry Torokhov wrote:
> Hi,
>
> On 8/29/06, Zephaniah E. Hull <[email protected]> wrote:
> >The OLPC will ship with a somewhat unique input device made by ALPS,
> >connected via PS/2 and speaking a protocol only loosely based on that
> >spoken by other ALPS devices.
> >
>
> Do you have a formal programming spec for it?

Not that I can currently distribute.

Converting to html, trimming out hardware detail, and feeding it through
channels for ALPS to say that they are comfortable with the amount of
data being released is on my todo list, but behind a few other things.
>
> >This is required by the noticeable different between this device and
> >others made by alps, specificly that it is very wide, with the center
> >1/3rd usable with the GS sensor, and the entire area usable with the PT
> >sensor, with support for using both at once.
> >
>
> Coudl youp please tell me what GS and PT stand for?

Glide Sensor, and PenTablet.
>
> >The protocol differs enough that I split the driver for this off from
> >the ALPS driver.
> >
> >The patch is below, but there are a few things of note.
> >
> >1: Cosmetic: Some line lengths, and outputs with debugging enabled, are
> >over 80 columns wide. Will be fixed in the next version of this patch.
> >
>
> If going to 80 colums will require monstrocities like this:
>
> ... do_bla(struct_b->
> array_g[
> index_i].
> submember);
>
> (and I seen quite a few such attempts to "improve" code) then please don't
> ;)

Understood. :)
>
> >2: Cosmetic: Input device IDs need to be decided on, some feedback on
> >the best values to use here would be appreciated.
>
> I think what you've done is fine.
>
> >
> >3: Patch stuff: Because the protocol uses 9 byte packets I had to
> >increase the size of the buffer in struct psmouse. Should this be split
> >off into a separate patch?
> >
>
> No.
>
> >4: Technical/policy: Buttons are currently sent to both of the input
> >devices we generate, I don't see any way to avoid this that is not a
> >policy decision on which buttons belong to which device, but I'm open to
> >suggestions.
> >
>
> Is it not known how actual hardware wired?

Hardware is wired with one device, which is quite wide. The entire
width can be accessed via the PT sensor, and the middle 1/3rd with the
GS sensor.

I believe that the buttons will be one on each side, though I'm not
positive, and the PT data, the GS data, and the button data all arrive
in the same packet.

So really there is no 'right' way from the kernel driver's point of
view, the buttons belong equally to both.

The kernel driver that this will be matched with will probably leave it
as a user configuration setting as to which one it will throw button
presses away for.

> >+#define OLPC_PT 0x01
> >+#define OLPC_GS 0x02
> >+#define OLPC_PTGS 0x04
> >+
>
> Do you need a separate #define? I'd expect it to be OLPC_PT | OLPC_GS?

Actually it's going away, as the driver is simply not going to try to
handle PT or GS mode separate from PT+GS, after more thought there was
little to gain from it.

> >+/*
> >+ * olpc_poll() - poll the touchpad for current motion packet.
> >+ * Used in resync.
> >+ */
> >+static int olpc_poll(struct psmouse *psmouse)
> >+{
> >+ /*
> >+ * FIXME: We can't poll, find a way to make resync work better.
> >+ */
> >+ return 0;
> >+}
>
> I'd expect it to return -1. It's OK that it can't poll, it looks like
> it should resync fairly well on its own.

Alright, I'll give that a try.

> >+ dev2->name = "OLPC OLPC GlideSensor";
>
> "OLPC OLPC"?

To match the first one, with a protocol name of OLPC and a vendor of
OLPC we end up with 'OLPC OLPC' for the first one, this is, IMHO, rather
suboptimal, but I'm not sure what else to do here.

Suggestions are most welcome. :)


Thanks a ton for the review.
Zephaniah E. Hull.

--
1024D/E65A7801 Zephaniah E. Hull <[email protected]>
92ED 94E4 B1E6 3624 226D 5727 4453 008B E65A 7801
CCs of replies from mailing lists are requested.

I could imagine that there might be some GPL project out there that
_deserves_ getting sued(*) and it has nothing to do with Linux.

Linus

(*) "GNU Emacs, the defendent, did inefariously conspire to play
towers-of-hanoy, while under the guise of a harmless editor".


Attachments:
(No filename) (4.30 kB)
signature.asc (189.00 B)
Digital signature
Download all attachments

2006-08-29 15:12:49

by Dmitry Torokhov

[permalink] [raw]
Subject: Re: [RPC] OLPC tablet input driver.

On 8/29/06, Zephaniah E. Hull <[email protected]> wrote:
> On Tue, Aug 29, 2006 at 08:53:17AM -0400, Dmitry Torokhov wrote:
> > Hi,
> >
> > On 8/29/06, Zephaniah E. Hull <[email protected]> wrote:
> > >The OLPC will ship with a somewhat unique input device made by ALPS,
> > >connected via PS/2 and speaking a protocol only loosely based on that
> > >spoken by other ALPS devices.
> > >
> >
> > Do you have a formal programming spec for it?
>
> Not that I can currently distribute.
>
> Converting to html, trimming out hardware detail, and feeding it through
> channels for ALPS to say that they are comfortable with the amount of
> data being released is on my todo list, but behind a few other things.

I see. Well, if you have a decent contacts in ALPS could you ask them
if they could release any information on their other hardware?

> >
> > >4: Technical/policy: Buttons are currently sent to both of the input
> > >devices we generate, I don't see any way to avoid this that is not a
> > >policy decision on which buttons belong to which device, but I'm open to
> > >suggestions.
> > >
> >
> > Is it not known how actual hardware wired?
>
> Hardware is wired with one device, which is quite wide. The entire
> width can be accessed via the PT sensor, and the middle 1/3rd with the
> GS sensor.
>
> I believe that the buttons will be one on each side, though I'm not
> positive, and the PT data, the GS data, and the button data all arrive
> in the same packet.
>
> So really there is no 'right' way from the kernel driver's point of
> view, the buttons belong equally to both.
>
> The kernel driver that this will be matched with will probably leave it
> as a user configuration setting as to which one it will throw button
> presses away for.

OK.

>
> > >+ dev2->name = "OLPC OLPC GlideSensor";
> >
> > "OLPC OLPC"?
>
> To match the first one, with a protocol name of OLPC and a vendor of
> OLPC we end up with 'OLPC OLPC' for the first one, this is, IMHO, rather
> suboptimal, but I'm not sure what else to do here.
>

Should not vendor be still ALPS?

--
Dmitry

2006-08-30 04:47:10

by Greg KH

[permalink] [raw]
Subject: Re: [RPC] OLPC tablet input driver.

On Tue, Aug 29, 2006 at 04:44:43AM -0400, Zephaniah E. Hull wrote:
> On Tue, Aug 29, 2006 at 10:10:19AM +0200, Arjan van de Ven wrote:
> > > +#undef DEBUG
> > > +#ifdef DEBUG
> > > +#define dbg(format, arg...) printk(KERN_INFO "olpc.c(%d): " format "\n", __LINE__, ## arg)
> > > +#else
> > > +#define dbg(format, arg...) do {} while (0)
> > > +#endif
> >
> > why not use pr_debug or even dev_debug() ?
> > Those already have this ifdef included
>
> I was not thinking of them at the time, however dev_dbg is not an option
> because we do not have a struct device at hand when we want to print
> some debugging lines.

Then use it for the majority of the places where you do have it, and do
pr_debug() when you do not.

> pr_debug might work, but I would rather have file and line already
> there.
>
> Though, admittedly, that would be a better argument if it used __FILE__
> there instead of hard coding it.

__FILE__ will return you a full path, which is what I do not think you
want...

thanks,

greg k-h

Subject: [RFC] OLPC tablet input driver, take two.

Take two, with most of the items people commented about addressed.


The OLPC will ship with a somewhat unique input device made by ALPS,
connected via PS/2 and speaking a protocol only loosely based on that
spoken by other ALPS devices.

This is required by the noticeable different between this device and
others made by alps, specificly that it is very wide, with the center
1/3rd usable with the GS sensor, and the entire area usable with the PT
sensor, with support for using both at once.

It uses a 9 byte protocol that differs enough that I split the driver
for this off from the ALPS driver.

The patch is below, but there are a few things of note.

1: Cosmetic: Some line lengths, and outputs with debugging enabled, are
over 80 columns wide. I've fixed most of them, but it would just get
into ugly stuff to fix the last few remaining. Suggestions are always
welcome though.

2: Technical, maybe: We're seeing a very low sample rate, however we're
fairly sure that this is due to the clock on our hardware, should be
verified sometime tomorrow. It is doubtful that any changes to this
driver will be necessary.

3: Technical: At least the pressure range is a lot smaller then we are
reporting, leaving as is until others weigh in on if we need ALPS to
give a larger range.

4: Technical: I've not implemented the KCONFIG option for this driver
yet, it's on my todo list, but for after we get the sample rate stuff
figured out.


That said, here the patch is for comments.
(And possibly for the OLPC kernel tree for others with samples to play
with.)


Zephaniah E. Hull.


Signed-off-by: Zephaniah E. Hull <[email protected]>

diff --git a/drivers/input/mouse/Makefile b/drivers/input/mouse/Makefile
index 21a1de6..6218e5a 100644
--- a/drivers/input/mouse/Makefile
+++ b/drivers/input/mouse/Makefile
@@ -14,4 +14,4 @@ obj-$(CONFIG_MOUSE_SERIAL) += sermouse.o
obj-$(CONFIG_MOUSE_HIL) += hil_ptr.o
obj-$(CONFIG_MOUSE_VSXXXAA) += vsxxxaa.o

-psmouse-objs := psmouse-base.o alps.o logips2pp.o synaptics.o lifebook.o trackpoint.o
+psmouse-objs := psmouse-base.o alps.o logips2pp.o synaptics.o lifebook.o trackpoint.o olpc.o
diff --git a/drivers/input/mouse/olpc.c b/drivers/input/mouse/olpc.c
new file mode 100644
index 0000000..45a24ec
--- /dev/null
+++ b/drivers/input/mouse/olpc.c
@@ -0,0 +1,344 @@
+/*
+ * OLPC touchpad PS/2 mouse driver
+ *
+ * Copyright (c) 2006 One Laptop Per Child, inc.
+ * Author Zephaniah E. Hull.
+ *
+ * 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 touchpad on the OLPC is fairly wide, with the entire area usable
+ * as a tablet, and the center 1/3rd also usable as a touchpad.
+ *
+ * The device has simultanious reporting, so that both can be used at once.
+ *
+ * The PT+GS protocol is similar to the base ALPS protocol, in that the
+ * GS data is where the ALPS parser would expect to find it, however
+ * there are several additional bytes, the button bits are in a
+ * different byte, and the bits used for finger and gesture indication
+ * are replaced by two bits which indicate if it is reporting PT or GS
+ * coordinate data in that packet.
+ */
+
+#include <linux/input.h>
+#include <linux/serio.h>
+#include <linux/libps2.h>
+
+#include "psmouse.h"
+#include "olpc.h"
+
+static const struct olpc_model_info olpc_model_data[] = {
+ { { 0x67, 0x00, 0x0a }, 0xeb, 0xff, 0 }, /* OLPC in PT+GS mode. */
+};
+
+/*
+ * OLPC absolute Mode - simultanious format
+ *
+ * byte 0: 1 1 1 0 1 0 1 1
+ * byte 1: 0 gx6 gx5 gx4 gx3 gx2 gx1 gx0
+ * byte 2: 0 gx10 gx9 gx8 gx7 px9 px8 px7
+ * byte 3: 0 gy9 gy8 gy7 1 ? pt-dsw gs-dsw
+ * byte 4: 0 gy6 gy5 gy4 gy3 gy2 gy1 gy0
+ * byte 5: 0 gz6 gz5 gz4 gz3 gz2 gz1 gz0
+ * byte 6: 0 py9 py8 py7 1 ? swr swl
+ * byte 7: 0 py6 py5 py4 py3 py2 py1 py0
+ * byte 8: 0 px6 px5 px4 px3 px2 px1 px0
+ *
+ * ?'s are not defined in the protocol spec, may vary between models.
+ *
+ * gx/gy/gz is for the GlideSensor.
+ * px/py is for the PenTablet sensor.
+ *
+ * swr/swl are the left/right buttons.
+ *
+ * pt-dsw/gs-dsw indicate that the pt/gs sensor is detecting a
+ * pen/finger and is thus sending data this packet.
+ */
+
+static void olpc_process_packet(struct psmouse *psmouse, struct pt_regs *regs)
+{
+ struct olpc_data *priv = psmouse->private;
+ unsigned char *packet = psmouse->packet;
+ struct input_dev *dev = psmouse->dev;
+ struct input_dev *dev2 = priv->dev2;
+ int px, py, gx, gy, gz, gs_down, pt_down, left, right;
+
+ input_regs(dev, regs);
+
+ left = packet[6] & 1;
+ right = packet[6] & 2;
+ gx = packet[1] | ((packet[2] & 0x78) << (7 - 3));
+ gy = packet[4] | ((packet[3] & 0x70) << (7 - 4));
+ gz = packet[5];
+ px = packet[8] | ((packet[2] & 0x7) << 7);
+ py = packet[7] | ((packet[6] & 0x70) << (7 - 4));
+
+ pt_down = packet[3] & 1;
+ gs_down = packet[3] & 2;
+
+ input_report_key(dev, BTN_LEFT, left);
+ input_report_key(dev2, BTN_LEFT, left);
+ input_report_key(dev, BTN_RIGHT, right);
+ input_report_key(dev2, BTN_RIGHT, right);
+
+ input_report_key(dev, BTN_TOUCH, pt_down);
+ input_report_key(dev, BTN_TOOL_PEN, pt_down);
+ input_report_key(dev2, BTN_TOUCH, gs_down);
+ input_report_key(dev2, BTN_TOOL_FINGER, gs_down);
+
+ if (gs_down) {
+ input_report_abs(dev2, ABS_X, gx);
+ input_report_abs(dev2, ABS_Y, gy);
+ }
+ input_report_abs(dev2, ABS_PRESSURE, gz);
+
+ if (pt_down) {
+ input_report_abs(dev, ABS_X, px);
+ input_report_abs(dev, ABS_Y, py);
+ }
+
+ input_sync(dev);
+}
+
+static psmouse_ret_t olpc_process_byte(struct psmouse *psmouse, struct pt_regs *regs)
+{
+ struct olpc_data *priv = psmouse->private;
+ psmouse_ret_t ret = PSMOUSE_BAD_DATA;
+
+ if ((psmouse->packet[0] & priv->i->mask0) != priv->i->byte0) {
+ ret = PSMOUSE_BAD_DATA;
+ goto out;
+ }
+
+ /* Bytes 2 - 9 should have 0 in the highest bit */
+ if (psmouse->pktcnt >= 2 && psmouse->pktcnt <= 9 &&
+ (psmouse->packet[psmouse->pktcnt - 1] & 0x80)) {
+ ret = PSMOUSE_BAD_DATA;
+ goto out;
+ }
+
+ /*
+ * Bytes 4 and 7 should have 1 in the 4th bit, and the high bit unset.
+ */
+ if ((psmouse->pktcnt == 4 || psmouse->pktcnt == 7) &&
+ ((psmouse->packet[psmouse->pktcnt - 1] & 0x88) != 8)) {
+ ret = PSMOUSE_BAD_DATA;
+ goto out;
+ }
+
+ if (psmouse->pktcnt == 9) {
+ olpc_process_packet(psmouse, regs);
+
+ ret = PSMOUSE_FULL_PACKET;
+ goto out;
+ }
+
+ ret = PSMOUSE_GOOD_DATA;
+out:
+ if (ret != PSMOUSE_GOOD_DATA)
+ dev_dbg(psmouse->dev->cdev.dev, "olpc.c(%d): ret: %d, len: %u,"
+ "data: %2.2x %2.2x %2.2x %2.2x %2.2x %2.2x %2.2x %2.2x %2.2x",
+ __LINE__, ret, psmouse->pktcnt,
+ psmouse->packet[0], psmouse->packet[1],
+ psmouse->packet[2], psmouse->packet[3],
+ psmouse->packet[4], psmouse->packet[5],
+ psmouse->packet[6], psmouse->packet[7],
+ psmouse->packet[8]);
+ return ret;
+}
+
+static const struct olpc_model_info *olpc_get_model(struct psmouse *psmouse)
+{
+ struct ps2dev *ps2dev = &psmouse->ps2dev;
+ unsigned char param[4];
+ int i;
+
+ /*
+ * Now try "E7 report". Allowed responses are in
+ * olpc_model_data[].signature
+ */
+ if (ps2_command(ps2dev, NULL, PSMOUSE_CMD_SETSCALE21) ||
+ ps2_command(ps2dev, NULL, PSMOUSE_CMD_SETSCALE21) ||
+ ps2_command(ps2dev, NULL, PSMOUSE_CMD_SETSCALE21))
+ return NULL;
+
+ param[0] = param[1] = param[2] = 0xff;
+ if (ps2_command(ps2dev, param, PSMOUSE_CMD_GETINFO))
+ return NULL;
+
+ pr_debug("olpc.c(%d): E7 report: %2.2x %2.2x %2.2x",
+ __LINE__, param[0], param[1], param[2]);
+
+ for (i = 0; i < ARRAY_SIZE(olpc_model_data); i++)
+ if (!memcmp(param, olpc_model_data[i].signature,
+ sizeof(olpc_model_data[i].signature)))
+ return olpc_model_data + i;
+
+ return NULL;
+}
+
+static int olpc_absolute_mode(struct psmouse *psmouse)
+{
+ struct ps2dev *ps2dev = &psmouse->ps2dev;
+ unsigned char param;
+
+ /* Switch to 'Advanced mode.', four disables in a row. */
+ if (ps2_command(ps2dev, NULL, PSMOUSE_CMD_DISABLE) ||
+ ps2_command(ps2dev, NULL, PSMOUSE_CMD_DISABLE) ||
+ ps2_command(ps2dev, NULL, PSMOUSE_CMD_DISABLE) ||
+ ps2_command(ps2dev, NULL, PSMOUSE_CMD_DISABLE))
+ return -1;
+
+ /*
+ * Switch to simultanious mode, F2 (GETID) three times with no
+ * arguments or reply, followed by SETRES with an argument of 2.
+ */
+ ps2_command(ps2dev, NULL, 0xF2);
+ ps2_command(ps2dev, NULL, 0xF2);
+ ps2_command(ps2dev, NULL, 0xF2);
+ param = 0x02;
+ ps2_command(ps2dev, &param, PSMOUSE_CMD_SETRES);
+
+ ps2_command(ps2dev, NULL, PSMOUSE_CMD_ENABLE);
+ param = 100;
+ ps2_command(ps2dev, &param, PSMOUSE_CMD_SETRES);
+
+ return 0;
+}
+
+/*
+ * olpc_poll() - poll the touchpad for current motion packet.
+ * Used in resync.
+ * Note: We can't poll, so always return failure.
+ */
+static int olpc_poll(struct psmouse *psmouse)
+{
+ return -1;
+}
+
+static int olpc_reconnect(struct psmouse *psmouse)
+{
+ struct olpc_data *priv = psmouse->private;
+
+ psmouse_reset(psmouse);
+
+ if (!(priv->i = olpc_get_model(psmouse)))
+ return -1;
+
+ if (olpc_absolute_mode(psmouse)) {
+ printk(KERN_ERR "olpc.c: Failed to reenable absolute mode.\n");
+ return -1;
+ }
+
+ return 0;
+}
+
+static void olpc_disconnect(struct psmouse *psmouse)
+{
+ struct olpc_data *priv = psmouse->private;
+
+ psmouse_reset(psmouse);
+ input_unregister_device(priv->dev2);
+ kfree(priv);
+}
+
+int olpc_init(struct psmouse *psmouse)
+{
+ struct olpc_data *priv;
+ struct input_dev *dev = psmouse->dev;
+ struct input_dev *dev2;
+
+ priv = kzalloc(sizeof(struct olpc_data), GFP_KERNEL);
+ dev2 = input_allocate_device();
+ if (!priv || !dev2)
+ goto init_fail;
+
+ psmouse->private = priv;
+ priv->dev2 = dev2;
+
+ if (!(priv->i = olpc_get_model(psmouse)))
+ goto init_fail;
+
+ if (olpc_absolute_mode(psmouse)) {
+ printk(KERN_ERR "olpc.c: Failed to enable absolute mode\n");
+ goto init_fail;
+ }
+
+ /*
+ * Unset some of the default bits for things we don't have.
+ */
+ dev->evbit[LONG(EV_REL)] &= ~BIT(EV_REL);
+ dev->relbit[LONG(REL_X)] &= ~(BIT(REL_X) | BIT(REL_Y));
+ dev->keybit[LONG(BTN_MIDDLE)] &= ~BIT(BTN_MIDDLE);
+
+ dev->evbit[LONG(EV_KEY)] |= BIT(EV_KEY);
+ dev->keybit[LONG(BTN_TOUCH)] |= BIT(BTN_TOUCH);
+ dev->keybit[LONG(BTN_TOOL_PEN)] |= BIT(BTN_TOOL_PEN);
+ dev->keybit[LONG(BTN_LEFT)] |= BIT(BTN_LEFT) | BIT(BTN_RIGHT);
+
+ dev->evbit[LONG(EV_ABS)] |= BIT(EV_ABS);
+ input_set_abs_params(dev, ABS_X, 0, 1023, 0, 0);
+ input_set_abs_params(dev, ABS_Y, 0, 1023, 0, 0);
+
+ snprintf(priv->phys, sizeof(priv->phys),
+ "%s/input1", psmouse->ps2dev.serio->phys);
+ dev2->phys = priv->phys;
+ dev2->name = "OLPC ALPS GlideSensor";
+ dev2->id.bustype = BUS_I8042;
+ dev2->id.vendor = 0x0002;
+ dev2->id.product = PSMOUSE_OLPC;
+ dev2->id.version = 0x0000;
+
+ dev2->evbit[LONG(EV_KEY)] |= BIT(EV_KEY);
+ dev2->keybit[LONG(BTN_TOUCH)] |= BIT(BTN_TOUCH);
+ dev2->keybit[LONG(BTN_TOOL_FINGER)] |= BIT(BTN_TOOL_FINGER);
+ dev2->keybit[LONG(BTN_LEFT)] |= BIT(BTN_LEFT) | BIT(BTN_RIGHT);
+
+ dev2->evbit[LONG(EV_ABS)] |= BIT(EV_ABS);
+ input_set_abs_params(dev2, ABS_X, 0, 2047, 0, 0);
+ input_set_abs_params(dev2, ABS_Y, 0, 1023, 0, 0);
+ input_set_abs_params(dev2, ABS_PRESSURE, 0, 63, 0, 0);
+
+ if (input_register_device(dev2))
+ goto init_fail;
+
+ psmouse->protocol_handler = olpc_process_byte;
+ psmouse->poll = olpc_poll;
+ psmouse->disconnect = olpc_disconnect;
+ psmouse->reconnect = olpc_reconnect;
+ psmouse->pktsize = 9;
+
+ /* Disable the idle resync. */
+ psmouse->resync_time = 0;
+
+ return 0;
+
+init_fail:
+ input_free_device(dev2);
+ kfree(priv);
+ return -1;
+}
+
+int olpc_detect(struct psmouse *psmouse, int set_properties)
+{
+ if (!olpc_get_model(psmouse))
+ return -1;
+
+ if (set_properties) {
+ psmouse->vendor = "ALPS";
+ psmouse->name = "PenTablet";
+ psmouse->model = 0;
+ }
+ return 0;
+}
+
diff --git a/drivers/input/mouse/olpc.h b/drivers/input/mouse/olpc.h
new file mode 100644
index 0000000..8ed2e96
--- /dev/null
+++ b/drivers/input/mouse/olpc.h
@@ -0,0 +1,35 @@
+/*
+ * OLPC touchpad PS/2 mouse driver
+ *
+ * Copyright (c) 2006 One Laptop Per Child, inc.
+ *
+ * This driver is partly based on the ALPS driver.
+ * Copyright (c) 2003 Peter Osterlund <[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.
+ */
+
+#ifndef _OLPC_H
+#define _OLPC_H
+
+int olpc_detect(struct psmouse *psmouse, int set_properties);
+int olpc_init(struct psmouse *psmouse);
+
+struct olpc_model_info {
+ unsigned char signature[3];
+ unsigned char byte0, mask0;
+ unsigned char flags;
+};
+
+struct olpc_data {
+ struct input_dev *dev2; /* Relative device */
+ char name[32]; /* Name */
+ char phys[32]; /* Phys */
+ const struct olpc_model_info *i;/* Info */
+};
+
+
+#endif
diff --git a/drivers/input/mouse/psmouse-base.c b/drivers/input/mouse/psmouse-base.c
index 8bc9f51..20060b0 100644
--- a/drivers/input/mouse/psmouse-base.c
+++ b/drivers/input/mouse/psmouse-base.c
@@ -26,6 +26,7 @@
#include "synaptics.h"
#include "logips2pp.h"
#include "alps.h"
+#include "olpc.h"
#include "lifebook.h"
#include "trackpoint.h"

@@ -616,6 +617,15 @@ static int psmouse_extensions(struct psm
*/
max_proto = PSMOUSE_IMEX;
}
+ ps2_command(&psmouse->ps2dev, NULL, PSMOUSE_CMD_RESET_DIS);
+ if (olpc_detect(psmouse, set_properties) == 0) {
+ if (!set_properties || olpc_init(psmouse) == 0)
+ return PSMOUSE_OLPC;
+/*
+ * Init failed, try basic relative protocols
+ */
+ max_proto = PSMOUSE_IMEX;
+ }
}

if (max_proto > PSMOUSE_IMEX && genius_detect(psmouse, set_properties) == 0)
@@ -726,6 +736,13 @@ static struct psmouse_protocol psmouse_p
.detect = trackpoint_detect,
},
{
+ .type = PSMOUSE_OLPC,
+ .name = "OLPC",
+ .alias = "olpc",
+ .maxproto = 1,
+ .detect = olpc_detect,
+ },
+ {
.type = PSMOUSE_AUTO,
.name = "auto",
.alias = "any",
diff --git a/drivers/input/mouse/psmouse.h b/drivers/input/mouse/psmouse.h
index 4d9107f..f3d7199 100644
--- a/drivers/input/mouse/psmouse.h
+++ b/drivers/input/mouse/psmouse.h
@@ -42,7 +42,7 @@ struct psmouse {
struct work_struct resync_work;
char *vendor;
char *name;
- unsigned char packet[8];
+ unsigned char packet[9];
unsigned char badbyte;
unsigned char pktcnt;
unsigned char pktsize;
@@ -86,6 +86,7 @@ enum psmouse_type {
PSMOUSE_ALPS,
PSMOUSE_LIFEBOOK,
PSMOUSE_TRACKPOINT,
+ PSMOUSE_OLPC,
PSMOUSE_AUTO /* This one should always be last */
};


--
1024D/E65A7801 Zephaniah E. Hull <[email protected]>
92ED 94E4 B1E6 3624 226D 5727 4453 008B E65A 7801
CCs of replies from mailing lists are requested.

"First they came for the Jews, and I didn't speak out - because I
was not a jew. Then they came for the Communists, and I did not speak
out - because I was not a Communist. Then they came for the trade
unionists, and I did not speak out - because I was not a trade unionist.
Then they came for me and there was no one left to speak for me!"
- Pastor Niemoeller - victim of Hitler's Nazis


Attachments:
(No filename) (15.23 kB)
signature.asc (189.00 B)
Digital signature
Download all attachments

2006-09-10 22:19:36

by Dmitry Torokhov

[permalink] [raw]
Subject: Re: [RFC] OLPC tablet input driver, take two.

On Sunday 10 September 2006 16:10, Zephaniah E. Hull wrote:
> Take two, with most of the items people commented about addressed.
>

Hi Zephaniah,

I have couple more comments/requests:

>
> +
> + if (gs_down) {
> + input_report_abs(dev2, ABS_X, gx);
> + input_report_abs(dev2, ABS_Y, gy);
> + }
> + input_report_abs(dev2, ABS_PRESSURE, gz);
> +
> + if (pt_down) {
> + input_report_abs(dev, ABS_X, px);
> + input_report_abs(dev, ABS_Y, py);
> + }
> +
> + input_sync(dev);

Please add input_sync(dev2);

> +}
> +
> +static psmouse_ret_t olpc_process_byte(struct psmouse *psmouse, struct pt_regs *regs)
> +{
> + struct olpc_data *priv = psmouse->private;
> + psmouse_ret_t ret = PSMOUSE_BAD_DATA;
> +
> + if ((psmouse->packet[0] & priv->i->mask0) != priv->i->byte0) {
> + ret = PSMOUSE_BAD_DATA;

It looks like you can kill "ret = PSMOUSE_BAD_DATA" assignments since you
initialize ret with it.

> + goto out;
> + }
> +
> + /* Bytes 2 - 9 should have 0 in the highest bit */
> + if (psmouse->pktcnt >= 2 && psmouse->pktcnt <= 9 &&
> + (psmouse->packet[psmouse->pktcnt - 1] & 0x80)) {
> + ret = PSMOUSE_BAD_DATA;
> + goto out;
> + }

I'd like to have standard identation throughout the driver (and input
sybsystem in general).

> +
> +#ifndef _OLPC_H
> +#define _OLPC_H
> +
> +int olpc_detect(struct psmouse *psmouse, int set_properties);
> +int olpc_init(struct psmouse *psmouse);
> +
> +struct olpc_model_info {
> + unsigned char signature[3];
> + unsigned char byte0, mask0;
> + unsigned char flags;
> +};

Hard TABs for identation please.

> +
> +struct olpc_data {
> + struct input_dev *dev2; /* Relative device */
> + char name[32]; /* Name */
> + char phys[32]; /* Phys */
> + const struct olpc_model_info *i;/* Info */
> +};
> +
> +
> +#endif
> diff --git a/drivers/input/mouse/psmouse-base.c b/drivers/input/mouse/psmouse-base.c
> index 8bc9f51..20060b0 100644
> --- a/drivers/input/mouse/psmouse-base.c
> +++ b/drivers/input/mouse/psmouse-base.c
> @@ -26,6 +26,7 @@
> #include "synaptics.h"
> #include "logips2pp.h"
> #include "alps.h"
> +#include "olpc.h"
> #include "lifebook.h"
> #include "trackpoint.h"
>
> @@ -616,6 +617,15 @@ static int psmouse_extensions(struct psm
> */
> max_proto = PSMOUSE_IMEX;
> }
> + ps2_command(&psmouse->ps2dev, NULL, PSMOUSE_CMD_RESET_DIS);

Do we have to do 2nd reset here? Plus logic seems a bit fuzzy here -
if ALPS is detected but initizliztion fails it will start OLPC detection
which is probably not what you wanted...

> + if (olpc_detect(psmouse, set_properties) == 0) {
> + if (!set_properties || olpc_init(psmouse) == 0)
> + return PSMOUSE_OLPC;
> +/*
> + * Init failed, try basic relative protocols
> + */
> + max_proto = PSMOUSE_IMEX;
> + }
> }
>
> if (max_proto > PSMOUSE_IMEX && genius_detect(psmouse, set_properties) == 0)
> @@ -726,6 +736,13 @@ static struct psmouse_protocol psmouse_p
> .detect = trackpoint_detect,
> },
> {
> + .type = PSMOUSE_OLPC,
> + .name = "OLPC",
> + .alias = "olpc",
> + .maxproto = 1,

Do not set maxproto on speciality protocols. It is meant to limit highest
version of standard protocols to be probed/used by a device.

--
Dmitry

Subject: Re: [RFC] OLPC tablet input driver, take two.

On Sun, Sep 10, 2006 at 06:19:31PM -0400, Dmitry Torokhov wrote:
> On Sunday 10 September 2006 16:10, Zephaniah E. Hull wrote:
> > Take two, with most of the items people commented about addressed.
> >
>
> Hi Zephaniah,
>
> I have couple more comments/requests:
>
> >
> > +
> > + if (gs_down) {
> > + input_report_abs(dev2, ABS_X, gx);
> > + input_report_abs(dev2, ABS_Y, gy);
> > + }
> > + input_report_abs(dev2, ABS_PRESSURE, gz);
> > +
> > + if (pt_down) {
> > + input_report_abs(dev, ABS_X, px);
> > + input_report_abs(dev, ABS_Y, py);
> > + }
> > +
> > + input_sync(dev);
>
> Please add input_sync(dev2);

Whoops, bizarrely it still worked, but fixed.
>
> > +}
> > +
> > +static psmouse_ret_t olpc_process_byte(struct psmouse *psmouse, struct pt_regs *regs)
> > +{
> > + struct olpc_data *priv = psmouse->private;
> > + psmouse_ret_t ret = PSMOUSE_BAD_DATA;
> > +
> > + if ((psmouse->packet[0] & priv->i->mask0) != priv->i->byte0) {
> > + ret = PSMOUSE_BAD_DATA;
>
> It looks like you can kill "ret = PSMOUSE_BAD_DATA" assignments since you
> initialize ret with it.

Done.
>
> > + goto out;
> > + }
> > +
> > + /* Bytes 2 - 9 should have 0 in the highest bit */
> > + if (psmouse->pktcnt >= 2 && psmouse->pktcnt <= 9 &&
> > + (psmouse->packet[psmouse->pktcnt - 1] & 0x80)) {
> > + ret = PSMOUSE_BAD_DATA;
> > + goto out;
> > + }
>
> I'd like to have standard identation throughout the driver (and input
> sybsystem in general).

Whoops, fixed.
>
> > +
> > +#ifndef _OLPC_H
> > +#define _OLPC_H
> > +
> > +int olpc_detect(struct psmouse *psmouse, int set_properties);
> > +int olpc_init(struct psmouse *psmouse);
> > +
> > +struct olpc_model_info {
> > + unsigned char signature[3];
> > + unsigned char byte0, mask0;
> > + unsigned char flags;
> > +};
>
> Hard TABs for identation please.

Done.
>
> > +
> > +struct olpc_data {
> > + struct input_dev *dev2; /* Relative device */
> > + char name[32]; /* Name */
> > + char phys[32]; /* Phys */
> > + const struct olpc_model_info *i;/* Info */
> > +};
> > +
> > +
> > +#endif
> > diff --git a/drivers/input/mouse/psmouse-base.c b/drivers/input/mouse/psmouse-base.c
> > index 8bc9f51..20060b0 100644
> > --- a/drivers/input/mouse/psmouse-base.c
> > +++ b/drivers/input/mouse/psmouse-base.c
> > @@ -26,6 +26,7 @@
> > #include "synaptics.h"
> > #include "logips2pp.h"
> > #include "alps.h"
> > +#include "olpc.h"
> > #include "lifebook.h"
> > #include "trackpoint.h"
> >
> > @@ -616,6 +617,15 @@ static int psmouse_extensions(struct psm
> > */
> > max_proto = PSMOUSE_IMEX;
> > }
> > + ps2_command(&psmouse->ps2dev, NULL, PSMOUSE_CMD_RESET_DIS);
>
> Do we have to do 2nd reset here? Plus logic seems a bit fuzzy here -
> if ALPS is detected but initizliztion fails it will start OLPC detection
> which is probably not what you wanted...

Reset is _probably_ not necessary, I'll verify.

However the logic is the same as for all the others, if init succeeds,
it returns PSMOUSE_ALPS, if it doesn't then it continues on to the next,
which happens to be olpc, admittedly it would be more obvious that it's
doing the same thing if it was in its own if, but.
>
> > + if (olpc_detect(psmouse, set_properties) == 0) {
> > + if (!set_properties || olpc_init(psmouse) == 0)
> > + return PSMOUSE_OLPC;
> > +/*
> > + * Init failed, try basic relative protocols
> > + */
> > + max_proto = PSMOUSE_IMEX;
> > + }
> > }
> >
> > if (max_proto > PSMOUSE_IMEX && genius_detect(psmouse, set_properties) == 0)
> > @@ -726,6 +736,13 @@ static struct psmouse_protocol psmouse_p
> > .detect = trackpoint_detect,
> > },
> > {
> > + .type = PSMOUSE_OLPC,
> > + .name = "OLPC",
> > + .alias = "olpc",
> > + .maxproto = 1,
>
> Do not set maxproto on speciality protocols. It is meant to limit highest
> version of standard protocols to be probed/used by a device.

Fixed.


Thanks a ton, I have some extra testing to do and then I'll send out a
fixed copy.

Zephaniah E. Hull.
>
> --
> Dmitry
>

--
1024D/E65A7801 Zephaniah E. Hull <[email protected]>
92ED 94E4 B1E6 3624 226D 5727 4453 008B E65A 7801
CCs of replies from mailing lists are requested.

"I am ecstatic that some moron re-invented a 1995 windows fuckup."
-- Alan Cox


Attachments:
(No filename) (4.18 kB)
signature.asc (189.00 B)
Digital signature
Download all attachments

2006-09-11 19:01:38

by Dmitry Torokhov

[permalink] [raw]
Subject: Re: [RFC] OLPC tablet input driver, take two.

On 9/11/06, Zephaniah E. Hull <[email protected]> wrote:
> On Sun, Sep 10, 2006 at 06:19:31PM -0400, Dmitry Torokhov wrote:
> > >
> > > @@ -616,6 +617,15 @@ static int psmouse_extensions(struct psm
> > > */
> > > max_proto = PSMOUSE_IMEX;
> > > }
> > > + ps2_command(&psmouse->ps2dev, NULL, PSMOUSE_CMD_RESET_DIS);
> >
> > Do we have to do 2nd reset here? Plus logic seems a bit fuzzy here -
> > if ALPS is detected but initizliztion fails it will start OLPC detection
> > which is probably not what you wanted...
>
> Reset is _probably_ not necessary, I'll verify.
>
> However the logic is the same as for all the others, if init succeeds,
> it returns PSMOUSE_ALPS, if it doesn't then it continues on to the next,
> which happens to be olpc, admittedly it would be more obvious that it's
> doing the same thing if it was in its own if, but.

Not exactly. We have 2 types of protocols - some have only detect,
others have both detect and init. For protocols that have both detect
and init we expect detect to reliably identify whether the device is
of given type or not and once detect succeeds we do not try to probe
for other speciality protocols. For example if alps_detect succeeds
but alps_init fails we won't try Genius detection (we will only try
standard imex, exps and bare) and we should not try OLPC detection
either.

--
Dmitry

Subject: [RFC] OLPC tablet input driver, take three.

Take three, with most of the items people commented about addressed.


The OLPC will ship with a somewhat unique input device made by ALPS,
connected via PS/2 and speaking a protocol only loosely based on that
spoken by other ALPS devices.

This is required by the noticeable different between this device and
others made by alps, specificly that it is very wide, with the center
1/3rd usable with the GS sensor, and the entire area usable with the PT
sensor, with support for using both at once.

It uses a 9 byte protocol that differs enough that I split the driver
for this off from the ALPS driver.

The patch is below, but there are a few things of note.

1: Cosmetic: Some line lengths, and outputs with debugging enabled, are
over 80 columns wide. I've fixed most of them, but it would just get
into ugly stuff to fix the last few remaining. Suggestions are always
welcome though.

2: Technical, maybe: We're seeing a very low sample rate, however we're
fairly sure that this is due to the clock on our hardware, should be
verified sometime tomorrow. It is doubtful that any changes to this
driver will be necessary.

3: Technical: At least the pressure range is a lot smaller then we are
reporting, leaving as is until others weigh in on if we need ALPS to
give a larger range.

4: Technical: I've not implemented the KCONFIG option for this driver
yet, it's on my todo list, but for after we get the sample rate stuff
figured out.


That said, here the patch, it seems to work, and it's time to at least
get into the OLPC kernel tree, if not mainline.


Zephaniah E. Hull.


Signed-off-by: Zephaniah E. Hull <[email protected]>

diff --git a/drivers/input/mouse/Makefile b/drivers/input/mouse/Makefile
index 21a1de6..6218e5a 100644
--- a/drivers/input/mouse/Makefile
+++ b/drivers/input/mouse/Makefile
@@ -14,4 +14,4 @@ obj-$(CONFIG_MOUSE_SERIAL) += sermouse.o
obj-$(CONFIG_MOUSE_HIL) += hil_ptr.o
obj-$(CONFIG_MOUSE_VSXXXAA) += vsxxxaa.o

-psmouse-objs := psmouse-base.o alps.o logips2pp.o synaptics.o lifebook.o trackpoint.o
+psmouse-objs := psmouse-base.o alps.o logips2pp.o synaptics.o lifebook.o trackpoint.o olpc.o
diff --git a/drivers/input/mouse/olpc.c b/drivers/input/mouse/olpc.c
new file mode 100644
index 0000000..ef4093f
--- /dev/null
+++ b/drivers/input/mouse/olpc.c
@@ -0,0 +1,339 @@
+/*
+ * OLPC touchpad PS/2 mouse driver
+ *
+ * Copyright (c) 2006 One Laptop Per Child, inc.
+ * Author Zephaniah E. Hull.
+ *
+ * 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 touchpad on the OLPC is fairly wide, with the entire area usable
+ * as a tablet, and the center 1/3rd also usable as a touchpad.
+ *
+ * The device has simultanious reporting, so that both can be used at once.
+ *
+ * The PT+GS protocol is similar to the base ALPS protocol, in that the
+ * GS data is where the ALPS parser would expect to find it, however
+ * there are several additional bytes, the button bits are in a
+ * different byte, and the bits used for finger and gesture indication
+ * are replaced by two bits which indicate if it is reporting PT or GS
+ * coordinate data in that packet.
+ */
+
+#include <linux/input.h>
+#include <linux/serio.h>
+#include <linux/libps2.h>
+
+#include "psmouse.h"
+#include "olpc.h"
+
+static const struct olpc_model_info olpc_model_data[] = {
+ { { 0x67, 0x00, 0x0a }, 0xeb, 0xff, 0 }, /* OLPC in PT+GS mode. */
+};
+
+/*
+ * OLPC absolute Mode - simultanious format
+ *
+ * byte 0: 1 1 1 0 1 0 1 1
+ * byte 1: 0 gx6 gx5 gx4 gx3 gx2 gx1 gx0
+ * byte 2: 0 gx10 gx9 gx8 gx7 px9 px8 px7
+ * byte 3: 0 gy9 gy8 gy7 1 ? pt-dsw gs-dsw
+ * byte 4: 0 gy6 gy5 gy4 gy3 gy2 gy1 gy0
+ * byte 5: 0 gz6 gz5 gz4 gz3 gz2 gz1 gz0
+ * byte 6: 0 py9 py8 py7 1 ? swr swl
+ * byte 7: 0 py6 py5 py4 py3 py2 py1 py0
+ * byte 8: 0 px6 px5 px4 px3 px2 px1 px0
+ *
+ * ?'s are not defined in the protocol spec, may vary between models.
+ *
+ * gx/gy/gz is for the GlideSensor.
+ * px/py is for the PenTablet sensor.
+ *
+ * swr/swl are the left/right buttons.
+ *
+ * pt-dsw/gs-dsw indicate that the pt/gs sensor is detecting a
+ * pen/finger and is thus sending data this packet.
+ */
+
+static void olpc_process_packet(struct psmouse *psmouse, struct pt_regs *regs)
+{
+ struct olpc_data *priv = psmouse->private;
+ unsigned char *packet = psmouse->packet;
+ struct input_dev *dev = psmouse->dev;
+ struct input_dev *dev2 = priv->dev2;
+ int px, py, gx, gy, gz, gs_down, pt_down, left, right;
+
+ input_regs(dev, regs);
+
+ left = packet[6] & 1;
+ right = packet[6] & 2;
+ gx = packet[1] | ((packet[2] & 0x78) << (7 - 3));
+ gy = packet[4] | ((packet[3] & 0x70) << (7 - 4));
+ gz = packet[5];
+ px = packet[8] | ((packet[2] & 0x7) << 7);
+ py = packet[7] | ((packet[6] & 0x70) << (7 - 4));
+
+ pt_down = packet[3] & 1;
+ gs_down = packet[3] & 2;
+
+ input_report_key(dev, BTN_LEFT, left);
+ input_report_key(dev2, BTN_LEFT, left);
+ input_report_key(dev, BTN_RIGHT, right);
+ input_report_key(dev2, BTN_RIGHT, right);
+
+ input_report_key(dev, BTN_TOUCH, pt_down);
+ input_report_key(dev, BTN_TOOL_PEN, pt_down);
+ input_report_key(dev2, BTN_TOUCH, gs_down);
+ input_report_key(dev2, BTN_TOOL_FINGER, gs_down);
+
+ if (gs_down) {
+ input_report_abs(dev2, ABS_X, gx);
+ input_report_abs(dev2, ABS_Y, gy);
+ }
+ input_report_abs(dev2, ABS_PRESSURE, gz);
+
+ if (pt_down) {
+ input_report_abs(dev, ABS_X, px);
+ input_report_abs(dev, ABS_Y, py);
+ }
+
+ input_sync(dev);
+ input_sync(dev2);
+}
+
+static psmouse_ret_t olpc_process_byte(struct psmouse *psmouse, struct pt_regs *regs)
+{
+ struct olpc_data *priv = psmouse->private;
+ psmouse_ret_t ret = PSMOUSE_BAD_DATA;
+
+ if ((psmouse->packet[0] & priv->i->mask0) != priv->i->byte0)
+ goto out;
+
+ /* Bytes 2 - 9 should have 0 in the highest bit */
+ if (psmouse->pktcnt >= 2 && psmouse->pktcnt <= 9 &&
+ (psmouse->packet[psmouse->pktcnt - 1] & 0x80))
+ goto out;
+
+ /*
+ * Bytes 4 and 7 should have 1 in the 4th bit, and the high bit unset.
+ */
+ if ((psmouse->pktcnt == 4 || psmouse->pktcnt == 7) &&
+ ((psmouse->packet[psmouse->pktcnt - 1] & 0x88) != 8))
+ goto out;
+
+ if (psmouse->pktcnt == 9) {
+ olpc_process_packet(psmouse, regs);
+
+ ret = PSMOUSE_FULL_PACKET;
+ goto out;
+ }
+
+ ret = PSMOUSE_GOOD_DATA;
+out:
+ if (ret != PSMOUSE_GOOD_DATA)
+ dev_dbg(psmouse->dev->cdev.dev, "olpc.c(%d): ret: %d, len: %u,"
+ "data: %2.2x %2.2x %2.2x %2.2x %2.2x %2.2x %2.2x %2.2x %2.2x",
+ __LINE__, ret, psmouse->pktcnt,
+ psmouse->packet[0], psmouse->packet[1],
+ psmouse->packet[2], psmouse->packet[3],
+ psmouse->packet[4], psmouse->packet[5],
+ psmouse->packet[6], psmouse->packet[7],
+ psmouse->packet[8]);
+ return ret;
+}
+
+static const struct olpc_model_info *olpc_get_model(struct psmouse *psmouse)
+{
+ struct ps2dev *ps2dev = &psmouse->ps2dev;
+ unsigned char param[4];
+ int i;
+
+ /*
+ * Now try "E7 report". Allowed responses are in
+ * olpc_model_data[].signature
+ */
+ if (ps2_command(ps2dev, NULL, PSMOUSE_CMD_SETSCALE21) ||
+ ps2_command(ps2dev, NULL, PSMOUSE_CMD_SETSCALE21) ||
+ ps2_command(ps2dev, NULL, PSMOUSE_CMD_SETSCALE21))
+ return NULL;
+
+ param[0] = param[1] = param[2] = 0xff;
+ if (ps2_command(ps2dev, param, PSMOUSE_CMD_GETINFO))
+ return NULL;
+
+ pr_debug("olpc.c(%d): E7 report: %2.2x %2.2x %2.2x",
+ __LINE__, param[0], param[1], param[2]);
+
+ for (i = 0; i < ARRAY_SIZE(olpc_model_data); i++)
+ if (!memcmp(param, olpc_model_data[i].signature,
+ sizeof(olpc_model_data[i].signature)))
+ return olpc_model_data + i;
+
+ return NULL;
+}
+
+static int olpc_absolute_mode(struct psmouse *psmouse)
+{
+ struct ps2dev *ps2dev = &psmouse->ps2dev;
+ unsigned char param;
+
+ /* Switch to 'Advanced mode.', four disables in a row. */
+ if (ps2_command(ps2dev, NULL, PSMOUSE_CMD_DISABLE) ||
+ ps2_command(ps2dev, NULL, PSMOUSE_CMD_DISABLE) ||
+ ps2_command(ps2dev, NULL, PSMOUSE_CMD_DISABLE) ||
+ ps2_command(ps2dev, NULL, PSMOUSE_CMD_DISABLE))
+ return -1;
+
+ /*
+ * Switch to simultanious mode, F2 (GETID) three times with no
+ * arguments or reply, followed by SETRES with an argument of 2.
+ */
+ ps2_command(ps2dev, NULL, 0xF2);
+ ps2_command(ps2dev, NULL, 0xF2);
+ ps2_command(ps2dev, NULL, 0xF2);
+ param = 0x02;
+ ps2_command(ps2dev, &param, PSMOUSE_CMD_SETRES);
+
+ ps2_command(ps2dev, NULL, PSMOUSE_CMD_ENABLE);
+ param = 100;
+ ps2_command(ps2dev, &param, PSMOUSE_CMD_SETRES);
+
+ return 0;
+}
+
+/*
+ * olpc_poll() - poll the touchpad for current motion packet.
+ * Used in resync.
+ * Note: We can't poll, so always return failure.
+ */
+static int olpc_poll(struct psmouse *psmouse)
+{
+ return -1;
+}
+
+static int olpc_reconnect(struct psmouse *psmouse)
+{
+ struct olpc_data *priv = psmouse->private;
+
+ psmouse_reset(psmouse);
+
+ if (!(priv->i = olpc_get_model(psmouse)))
+ return -1;
+
+ if (olpc_absolute_mode(psmouse)) {
+ printk(KERN_ERR "olpc.c: Failed to reenable absolute mode.\n");
+ return -1;
+ }
+
+ return 0;
+}
+
+static void olpc_disconnect(struct psmouse *psmouse)
+{
+ struct olpc_data *priv = psmouse->private;
+
+ psmouse_reset(psmouse);
+ input_unregister_device(priv->dev2);
+ kfree(priv);
+}
+
+int olpc_init(struct psmouse *psmouse)
+{
+ struct olpc_data *priv;
+ struct input_dev *dev = psmouse->dev;
+ struct input_dev *dev2;
+
+ priv = kzalloc(sizeof(struct olpc_data), GFP_KERNEL);
+ dev2 = input_allocate_device();
+ if (!priv || !dev2)
+ goto init_fail;
+
+ psmouse->private = priv;
+ priv->dev2 = dev2;
+
+ if (!(priv->i = olpc_get_model(psmouse)))
+ goto init_fail;
+
+ if (olpc_absolute_mode(psmouse)) {
+ printk(KERN_ERR "olpc.c: Failed to enable absolute mode\n");
+ goto init_fail;
+ }
+
+ /*
+ * Unset some of the default bits for things we don't have.
+ */
+ dev->evbit[LONG(EV_REL)] &= ~BIT(EV_REL);
+ dev->relbit[LONG(REL_X)] &= ~(BIT(REL_X) | BIT(REL_Y));
+ dev->keybit[LONG(BTN_MIDDLE)] &= ~BIT(BTN_MIDDLE);
+
+ dev->evbit[LONG(EV_KEY)] |= BIT(EV_KEY);
+ dev->keybit[LONG(BTN_TOUCH)] |= BIT(BTN_TOUCH);
+ dev->keybit[LONG(BTN_TOOL_PEN)] |= BIT(BTN_TOOL_PEN);
+ dev->keybit[LONG(BTN_LEFT)] |= BIT(BTN_LEFT) | BIT(BTN_RIGHT);
+
+ dev->evbit[LONG(EV_ABS)] |= BIT(EV_ABS);
+ input_set_abs_params(dev, ABS_X, 0, 1023, 0, 0);
+ input_set_abs_params(dev, ABS_Y, 0, 1023, 0, 0);
+
+ snprintf(priv->phys, sizeof(priv->phys),
+ "%s/input1", psmouse->ps2dev.serio->phys);
+ dev2->phys = priv->phys;
+ dev2->name = "OLPC ALPS GlideSensor";
+ dev2->id.bustype = BUS_I8042;
+ dev2->id.vendor = 0x0002;
+ dev2->id.product = PSMOUSE_OLPC;
+ dev2->id.version = 0x0000;
+
+ dev2->evbit[LONG(EV_KEY)] |= BIT(EV_KEY);
+ dev2->keybit[LONG(BTN_TOUCH)] |= BIT(BTN_TOUCH);
+ dev2->keybit[LONG(BTN_TOOL_FINGER)] |= BIT(BTN_TOOL_FINGER);
+ dev2->keybit[LONG(BTN_LEFT)] |= BIT(BTN_LEFT) | BIT(BTN_RIGHT);
+
+ dev2->evbit[LONG(EV_ABS)] |= BIT(EV_ABS);
+ input_set_abs_params(dev2, ABS_X, 0, 2047, 0, 0);
+ input_set_abs_params(dev2, ABS_Y, 0, 1023, 0, 0);
+ input_set_abs_params(dev2, ABS_PRESSURE, 0, 63, 0, 0);
+
+ if (input_register_device(dev2))
+ goto init_fail;
+
+ psmouse->protocol_handler = olpc_process_byte;
+ psmouse->poll = olpc_poll;
+ psmouse->disconnect = olpc_disconnect;
+ psmouse->reconnect = olpc_reconnect;
+ psmouse->pktsize = 9;
+
+ /* Disable the idle resync. */
+ psmouse->resync_time = 0;
+
+ return 0;
+
+init_fail:
+ input_free_device(dev2);
+ kfree(priv);
+ return -1;
+}
+
+int olpc_detect(struct psmouse *psmouse, int set_properties)
+{
+ if (!olpc_get_model(psmouse))
+ return -1;
+
+ if (set_properties) {
+ psmouse->vendor = "ALPS";
+ psmouse->name = "PenTablet";
+ psmouse->model = 0;
+ }
+ return 0;
+}
+
diff --git a/drivers/input/mouse/olpc.h b/drivers/input/mouse/olpc.h
new file mode 100644
index 0000000..f61bc90
--- /dev/null
+++ b/drivers/input/mouse/olpc.h
@@ -0,0 +1,35 @@
+/*
+ * OLPC touchpad PS/2 mouse driver
+ *
+ * Copyright (c) 2006 One Laptop Per Child, inc.
+ *
+ * This driver is partly based on the ALPS driver.
+ * Copyright (c) 2003 Peter Osterlund <[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.
+ */
+
+#ifndef _OLPC_H
+#define _OLPC_H
+
+int olpc_detect(struct psmouse *psmouse, int set_properties);
+int olpc_init(struct psmouse *psmouse);
+
+struct olpc_model_info {
+ unsigned char signature[3];
+ unsigned char byte0, mask0;
+ unsigned char flags;
+};
+
+struct olpc_data {
+ struct input_dev *dev2; /* Relative device */
+ char name[32]; /* Name */
+ char phys[32]; /* Phys */
+ const struct olpc_model_info *i;/* Info */
+};
+
+
+#endif
diff --git a/drivers/input/mouse/psmouse-base.c b/drivers/input/mouse/psmouse-base.c
index 8bc9f51..30edbe7 100644
--- a/drivers/input/mouse/psmouse-base.c
+++ b/drivers/input/mouse/psmouse-base.c
@@ -26,6 +26,7 @@
#include "synaptics.h"
#include "logips2pp.h"
#include "alps.h"
+#include "olpc.h"
#include "lifebook.h"
#include "trackpoint.h"

@@ -618,6 +619,20 @@ static int psmouse_extensions(struct psm
}
}

+/*
+ * Try OLPC touchpad.
+ */
+ if (max_proto > PSMOUSE_IMEX) {
+ if (olpc_detect(psmouse, set_properties) == 0) {
+ if (!set_properties || olpc_init(psmouse) == 0)
+ return PSMOUSE_OLPC;
+/*
+ * Init failed, try basic relative protocols
+ */
+ max_proto = PSMOUSE_IMEX;
+ }
+ }
+
if (max_proto > PSMOUSE_IMEX && genius_detect(psmouse, set_properties) == 0)
return PSMOUSE_GENPS;

@@ -726,6 +741,12 @@ static struct psmouse_protocol psmouse_p
.detect = trackpoint_detect,
},
{
+ .type = PSMOUSE_OLPC,
+ .name = "OLPC",
+ .alias = "olpc",
+ .detect = olpc_detect,
+ },
+ {
.type = PSMOUSE_AUTO,
.name = "auto",
.alias = "any",
diff --git a/drivers/input/mouse/psmouse.h b/drivers/input/mouse/psmouse.h
index 4d9107f..f3d7199 100644
--- a/drivers/input/mouse/psmouse.h
+++ b/drivers/input/mouse/psmouse.h
@@ -42,7 +42,7 @@ struct psmouse {
struct work_struct resync_work;
char *vendor;
char *name;
- unsigned char packet[8];
+ unsigned char packet[9];
unsigned char badbyte;
unsigned char pktcnt;
unsigned char pktsize;
@@ -86,6 +86,7 @@ enum psmouse_type {
PSMOUSE_ALPS,
PSMOUSE_LIFEBOOK,
PSMOUSE_TRACKPOINT,
+ PSMOUSE_OLPC,
PSMOUSE_AUTO /* This one should always be last */
};


--
1024D/E65A7801 Zephaniah E. Hull <[email protected]>
92ED 94E4 B1E6 3624 226D 5727 4453 008B E65A 7801
CCs of replies from mailing lists are requested.

<nonlinear> .net is microsofts perverted version of a java networked
environment uglified for windows-specific crap
-- nonlinear in #opengl


Attachments:
(No filename) (14.86 kB)
signature.asc (189.00 B)
Digital signature
Download all attachments
Subject: Re: [RFC] OLPC tablet input driver, take two.

On Mon, Sep 11, 2006 at 03:01:35PM -0400, Dmitry Torokhov wrote:
> On 9/11/06, Zephaniah E. Hull <[email protected]> wrote:
> >On Sun, Sep 10, 2006 at 06:19:31PM -0400, Dmitry Torokhov wrote:
> >> >
> >> > @@ -616,6 +617,15 @@ static int psmouse_extensions(struct psm
> >> > */
> >> > max_proto = PSMOUSE_IMEX;
> >> > }
> >> > + ps2_command(&psmouse->ps2dev, NULL, PSMOUSE_CMD_RESET_DIS);
> >>
> >> Do we have to do 2nd reset here? Plus logic seems a bit fuzzy here -
> >> if ALPS is detected but initizliztion fails it will start OLPC detection
> >> which is probably not what you wanted...
> >
> >Reset is _probably_ not necessary, I'll verify.
> >
> >However the logic is the same as for all the others, if init succeeds,
> >it returns PSMOUSE_ALPS, if it doesn't then it continues on to the next,
> >which happens to be olpc, admittedly it would be more obvious that it's
> >doing the same thing if it was in its own if, but.
>
> Not exactly. We have 2 types of protocols - some have only detect,
> others have both detect and init. For protocols that have both detect
> and init we expect detect to reliably identify whether the device is
> of given type or not and once detect succeeds we do not try to probe
> for other speciality protocols. For example if alps_detect succeeds
> but alps_init fails we won't try Genius detection (we will only try
> standard imex, exps and bare) and we should not try OLPC detection
> either.

Ah, I had misread the if. Corrected in the patch that just went out.

Thank you.
Zephaniah E. Hull.
>
> --
> Dmitry
>

--
1024D/E65A7801 Zephaniah E. Hull <[email protected]>
92ED 94E4 B1E6 3624 226D 5727 4453 008B E65A 7801
CCs of replies from mailing lists are requested.

"This system operates under martial law. The constitution is suspended. You
have no rights except as declared by the area commander. Violators will be
shot. Repeat violators will be repeatedly shot...." -from "A_W_O_L"


Attachments:
(No filename) (1.95 kB)
signature.asc (189.00 B)
Digital signature
Download all attachments

2006-09-11 19:10:10

by Dmitry Torokhov

[permalink] [raw]
Subject: Re: [RFC] OLPC tablet input driver, take three.

On 9/11/06, Zephaniah E. Hull <[email protected]> wrote:
>
> 4: Technical: I've not implemented the KCONFIG option for this driver
> yet, it's on my todo list, but for after we get the sample rate stuff
> figured out.
>
>
> That said, here the patch, it seems to work, and it's time to at least
> get into the OLPC kernel tree, if not mainline.
>

Zephaniah,

What are the chances that commodity hardware will have OLPC device
present? If there are none (or extremely slim) I think we'd better
wait for Kconfig option to be implemented before adding this to
mainline because psmouse is already too fat.

Otherwise it looks good.

--
Dmitry

Subject: Re: [RFC] OLPC tablet input driver, take three.

On Mon, Sep 11, 2006 at 03:10:06PM -0400, Dmitry Torokhov wrote:
> On 9/11/06, Zephaniah E. Hull <[email protected]> wrote:
> >
> >4: Technical: I've not implemented the KCONFIG option for this driver
> >yet, it's on my todo list, but for after we get the sample rate stuff
> >figured out.
> >
> >
> >That said, here the patch, it seems to work, and it's time to at least
> >get into the OLPC kernel tree, if not mainline.
> >
>
> Zephaniah,
>
> What are the chances that commodity hardware will have OLPC device
> present? If there are none (or extremely slim) I think we'd better
> wait for Kconfig option to be implemented before adding this to
> mainline because psmouse is already too fat.

Extremely slim, the current generation of samples are 3.3V units instead
of 5V units.

When I go in and do this, would it make sense to split out most of the
external to psmouse-base.c drivers to be options, most tied to being on
unless CONFIG_EMBEDDED is enabled?

Zephaniah E. Hull.
>
> Otherwise it looks good.
>
> --
> Dmitry
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>

--
1024D/E65A7801 Zephaniah E. Hull <[email protected]>
92ED 94E4 B1E6 3624 226D 5727 4453 008B E65A 7801
CCs of replies from mailing lists are requested.

"I would rather spend 10 hours reading someone else's source code than
10 minutes listening to Musak waiting for technical support which
isn't."
(By Dr. Greg Wettstein, Roger Maris Cancer Center)


Attachments:
(No filename) (1.61 kB)
signature.asc (189.00 B)
Digital signature
Download all attachments

2006-09-12 19:58:45

by Dmitry Torokhov

[permalink] [raw]
Subject: Re: [RFC] OLPC tablet input driver, take three.

On 9/12/06, Zephaniah E. Hull <[email protected]> wrote:
> On Mon, Sep 11, 2006 at 03:10:06PM -0400, Dmitry Torokhov wrote:
> > On 9/11/06, Zephaniah E. Hull <[email protected]> wrote:
> > >
> > >4: Technical: I've not implemented the KCONFIG option for this driver
> > >yet, it's on my todo list, but for after we get the sample rate stuff
> > >figured out.
> > >
> > >
> > >That said, here the patch, it seems to work, and it's time to at least
> > >get into the OLPC kernel tree, if not mainline.
> > >
> >
> > Zephaniah,
> >
> > What are the chances that commodity hardware will have OLPC device
> > present? If there are none (or extremely slim) I think we'd better
> > wait for Kconfig option to be implemented before adding this to
> > mainline because psmouse is already too fat.
>
> Extremely slim, the current generation of samples are 3.3V units instead
> of 5V units.
>
> When I go in and do this, would it make sense to split out most of the
> external to psmouse-base.c drivers to be options, most tied to being on
> unless CONFIG_EMBEDDED is enabled?
>

Yes, that would be nice. Just keep in mind that we need to detect
synaptics touchpads even if driver support is disabled so that we can
properly reset them if they don't support imex or exps protocols.
Otherwise the trackpoint connected to the pass-through port stops
working.

--
Dmitry

2006-11-08 12:06:43

by Vojtech Pavlik

[permalink] [raw]
Subject: Re: [RPC] OLPC tablet input driver.

On Tue, Aug 29, 2006 at 03:33:39AM -0400, Zephaniah E. Hull wrote:

> 4: Technical/policy: Buttons are currently sent to both of the input
> devices we generate, I don't see any way to avoid this that is not a
> policy decision on which buttons belong to which device, but I'm open to
> suggestions.

I would put them on the touchpad device. This way you create a regular
looking touchpad and a regular looking tablet, and the user experience
will probably be best.

If you report them both, the GUI will get doubled click events (either
through /dev/input/mice or through the event X drivers), which can
result in doubleclicks where there were none when the click event is
very short.

So reporting on both is a bad idea.

> 5: Technical: Min/max on absolute values are currently reported as the
> protocol limits (10 bits on GS X, GS Y, and PT Y. 11 bits on PT X. 7
> bits on GS pressure). Until we get samples based on the newer design
> and do some testing to see how big the variations are, we just don't
> have any numbers to put here.

No big deal, they're informative only. However, they're defined as the
expected min/max of what the device will report, so updating them when
you get the actual hardware running makes sense.

> 6: Technical, maybe: The early samples I have that speak this protocol
> are doing some odd things with this driver. Mostly in the realm of
> sample rate and pressure reporting. I'm fairly sure that this is
> hardware related, but it's worth mentioning.

I would expect the sample rate to be somewhat limited by the fact that
the packets are very large and the serial communication will not be able
to keep up.

> + snprintf(priv->phys, sizeof(priv->phys), "%s/input1", psmouse->ps2dev.serio->phys);
> + dev2->phys = priv->phys;
> + dev2->name = "OLPC OLPC GlideSensor";

Why OLPC twice? Weren't you planning to say ALPS OLPC GlideSensor?

> + dev2->id.bustype = BUS_I8042;
> + dev2->id.vendor = 0x0002;
> + dev2->id.product = PSMOUSE_OLPC;
> + dev2->id.version = 0x0000;



--
Vojtech Pavlik
Director SuSE Labs