2003-09-18 23:44:19

by Peter Osterlund

[permalink] [raw]
Subject: Re: Broken synaptics mouse..

On Thu, 11 Sep 2003, Linus Torvalds wrote:

> Actually, I think I've changed my mind.
>
> How hard would it be to have the "event" driver _notice_ when somebody is
> trying to use it?
>
> In particular, what about something that
> - just keeps the touchpad in "ps/2 compatibility mode" by default
> - moves to absolute mode only if somebody actually uses the
> /dev/input/event0 thing for it (and that would obviously disable the
> ps/2 mode).
>
> That sounds like the simpler setup, especially since the touchpad does a
> pretty good job of PS/2 emulation on its own..

What do you think about the following patch? This patch makes the touchpad
stay in compatibility mode until user space explicitly asks for absolute
mode by sending a SYN_CONFIG event with value != 0 to the synaptics event
device.

I think this patch will apply on top of Vojtech's private tree, but I have
lost track of what patches he said he has applied. Anyway, the whole patch
series (14 patches) is available here:

http://w1.894.telia.com/~u89404340/patches/touchpad/2.6.0-test5-bk5/v1/

I dropped the "synaptics_optional" patch, because there should not be any
need for the config option now that we have backwards compatibility by
default.


linux-petero/drivers/input/mouse/psmouse-base.c | 2
linux-petero/drivers/input/mouse/psmouse.h | 1
linux-petero/drivers/input/mouse/synaptics.c | 62 ++++++++++++++++++++++--
linux-petero/drivers/input/mouse/synaptics.h | 4 +
4 files changed, 63 insertions(+), 6 deletions(-)

diff -puN drivers/input/mouse/synaptics.c~synaptics-absolute-on-demand drivers/input/mouse/synaptics.c
--- linux/drivers/input/mouse/synaptics.c~synaptics-absolute-on-demand 2003-09-19 01:05:43.000000000 +0200
+++ linux-petero/drivers/input/mouse/synaptics.c 2003-09-19 01:05:43.000000000 +0200
@@ -223,9 +223,11 @@ static int synaptics_set_mode(struct psm
mode |= SYN_BIT_DISABLE_GESTURE;
if (SYN_CAP_EXTENDED(priv->capabilities))
mode |= SYN_BIT_W_MODE;
- if (synaptics_mode_cmd(psmouse, mode))
- return -1;
+ priv->abs_mode_bits = mode;

+ if (priv->abs_mode_enabled)
+ if (synaptics_mode_cmd(psmouse, mode))
+ return -1;
return 0;
}

@@ -323,6 +325,33 @@ static inline void set_abs_params(struct
set_bit(axis, dev->absbit);
}

+static int synaptics_event(struct input_dev *dev, unsigned int type, unsigned int code, int value)
+{
+ struct psmouse *psmouse = dev->private;
+ struct synaptics_data *priv = psmouse->private;
+
+ if ((type != EV_SYN) || (code != SYN_CONFIG))
+ return -1;
+
+ if (!value) {
+ priv->abs_mode_enabled = 0;
+ printk(KERN_INFO "synaptics: Enable relative mode\n");
+ if (synaptics_mode_cmd(psmouse, 0)) {
+ printk(KERN_ERR "synaptics: Couldn't enable relative mode.\n");
+ return -1;
+ }
+ } else {
+ priv->abs_mode_enabled = 1;
+ printk(KERN_INFO "synaptics: Enable absolute mode\n");
+ if (synaptics_set_mode(psmouse, priv->abs_mode_bits)) {
+ printk(KERN_ERR "synaptics: Couldn't enable absolute mode.\n");
+ return -1;
+ }
+ }
+
+ return 0;
+}
+
static void set_input_params(struct input_dev *dev, struct synaptics_data *priv)
{
/*
@@ -340,6 +369,7 @@ static void set_input_params(struct inpu

set_bit(EV_KEY, dev->evbit);
set_bit(BTN_LEFT, dev->keybit);
+ set_bit(BTN_MIDDLE, dev->keybit);
set_bit(BTN_RIGHT, dev->keybit);
set_bit(BTN_FORWARD, dev->keybit);
set_bit(BTN_BACK, dev->keybit);
@@ -367,9 +397,13 @@ static void set_input_params(struct inpu
}
}

- clear_bit(EV_REL, dev->evbit);
- clear_bit(REL_X, dev->relbit);
- clear_bit(REL_Y, dev->relbit);
+ set_bit(EV_REL, dev->evbit);
+ set_bit(REL_X, dev->relbit);
+ set_bit(REL_Y, dev->relbit);
+
+ set_bit(BTN_TRIGGER, dev->keybit); /* Makes mousedev.c ignore ABS events */
+
+ dev->event = synaptics_event;
}

static void synaptics_disconnect(struct psmouse *psmouse)
@@ -433,6 +467,12 @@ int synaptics_init(struct psmouse *psmou
psmouse->disconnect = synaptics_disconnect;
psmouse->reconnect = synaptics_reconnect;

+ /*
+ * Now restore relative mode. To stay compatible with legacy user space
+ * drivers, absolute mode is only enabled when explicitly asked for.
+ */
+ synaptics_mode_cmd(psmouse, 0);
+
return 0;

init_fail:
@@ -597,6 +637,18 @@ void synaptics_process_byte(struct psmou
unsigned char data = psmouse->packet[psmouse->pktcnt - 1];
int newabs = SYN_MODEL_NEWABS(priv->model_id);

+ if (!priv->abs_mode_enabled) {
+ /*
+ * Touchpad is in relative mode. Parse packet using the standard
+ * PS/2 protocol.
+ */
+ if (psmouse->pktcnt == 3) {
+ psmouse_process_packet(psmouse, regs);
+ psmouse->pktcnt = 0;
+ }
+ return;
+ }
+
input_regs(dev, regs);

switch (psmouse->pktcnt) {
diff -puN drivers/input/mouse/psmouse-base.c~synaptics-absolute-on-demand drivers/input/mouse/psmouse-base.c
--- linux/drivers/input/mouse/psmouse-base.c~synaptics-absolute-on-demand 2003-09-19 01:05:43.000000000 +0200
+++ linux-petero/drivers/input/mouse/psmouse-base.c 2003-09-19 01:05:43.000000000 +0200
@@ -48,7 +48,7 @@ static char *psmouse_protocols[] = { "No
* reports relevant events to the input module.
*/

-static void psmouse_process_packet(struct psmouse *psmouse, struct pt_regs *regs)
+void psmouse_process_packet(struct psmouse *psmouse, struct pt_regs *regs)
{
struct input_dev *dev = &psmouse->dev;
unsigned char *packet = psmouse->packet;
diff -puN drivers/input/mouse/synaptics.h~synaptics-absolute-on-demand drivers/input/mouse/synaptics.h
--- linux/drivers/input/mouse/synaptics.h~synaptics-absolute-on-demand 2003-09-19 01:05:43.000000000 +0200
+++ linux-petero/drivers/input/mouse/synaptics.h 2003-09-19 01:05:43.000000000 +0200
@@ -100,6 +100,10 @@ struct synaptics_data {
unsigned long int ext_cap; /* Extended Capabilities */
unsigned long int identity; /* Identification */

+ int abs_mode_enabled; /* Non-zero when in absolute mode */
+ int abs_mode_bits; /* The mode bits to send to the touchpad
+ * when enabling absolute mode */
+
/* Data for normal processing */
unsigned int out_of_sync; /* # of packets out of sync */
int old_w; /* Previous w value */
diff -puN drivers/input/mouse/psmouse.h~synaptics-absolute-on-demand drivers/input/mouse/psmouse.h
--- linux/drivers/input/mouse/psmouse.h~synaptics-absolute-on-demand 2003-09-19 01:05:43.000000000 +0200
+++ linux-petero/drivers/input/mouse/psmouse.h 2003-09-19 01:05:43.000000000 +0200
@@ -64,6 +64,7 @@ struct psmouse {
#define PSMOUSE_IMEX 6
#define PSMOUSE_SYNAPTICS 7

+void psmouse_process_packet(struct psmouse *psmouse, struct pt_regs *regs);
int psmouse_command(struct psmouse *psmouse, unsigned char *param, int command);

extern int psmouse_smartscroll;

_

--
Peter Osterlund - [email protected]
http://w1.894.telia.com/~u89404340


2003-09-19 05:05:54

by Dmitry Torokhov

[permalink] [raw]
Subject: Re: Broken synaptics mouse..

On Thursday 18 September 2003 06:43 pm, Peter Osterlund wrote:
> On Thu, 11 Sep 2003, Linus Torvalds wrote:
> > Actually, I think I've changed my mind.
> >
> > How hard would it be to have the "event" driver _notice_ when somebody is
> > trying to use it?
> >
> > In particular, what about something that
> > - just keeps the touchpad in "ps/2 compatibility mode" by default
> > - moves to absolute mode only if somebody actually uses the
> > /dev/input/event0 thing for it (and that would obviously disable the
> > ps/2 mode).
> >
> > That sounds like the simpler setup, especially since the touchpad does a
> > pretty good job of PS/2 emulation on its own..
>
> What do you think about the following patch? This patch makes the touchpad
> stay in compatibility mode until user space explicitly asks for absolute
> mode by sending a SYN_CONFIG event with value != 0 to the synaptics event
> device.
>

This makes Synaptics lie about half of it's properties as it still reports
EV_REL after switching to absolute mode. Also, reporting that Synaptics
supports EV_TRIGGER will possibly confuse software as only joysticks (so far)
have it. I think this is wrong. Device should report only what it actually
has so userspace could reliable detect devices.

We also can't just emulate relative events as everything is multiplexed into
/dev/input/mice and I can see many people using Synaptics via
/dev/input/eventX and everything else via /dev/input/mice as it nicely handles
hot plugging (at least I use it this way).

Can't we just have compile option for Synaptics stating that new drivers are
needed and give links in help? By the time distributions pick up 2.6 they
will also package correct driver/GPM.

Dmitry

2003-09-19 11:53:52

by Vojtech Pavlik

[permalink] [raw]
Subject: Re: Broken synaptics mouse..

On Fri, Sep 19, 2003 at 01:43:11AM +0200, Peter Osterlund wrote:
> On Thu, 11 Sep 2003, Linus Torvalds wrote:
>
> > Actually, I think I've changed my mind.
> >
> > How hard would it be to have the "event" driver _notice_ when somebody is
> > trying to use it?
> >
> > In particular, what about something that
> > - just keeps the touchpad in "ps/2 compatibility mode" by default
> > - moves to absolute mode only if somebody actually uses the
> > /dev/input/event0 thing for it (and that would obviously disable the
> > ps/2 mode).
> >
> > That sounds like the simpler setup, especially since the touchpad does a
> > pretty good job of PS/2 emulation on its own..
>
> What do you think about the following patch? This patch makes the touchpad
> stay in compatibility mode until user space explicitly asks for absolute
> mode by sending a SYN_CONFIG event with value != 0 to the synaptics event
> device.
>
> I think this patch will apply on top of Vojtech's private tree, but I have
> lost track of what patches he said he has applied. Anyway, the whole patch
> series (14 patches) is available here:
>
> http://w1.894.telia.com/~u89404340/patches/touchpad/2.6.0-test5-bk5/v1/
>
> I dropped the "synaptics_optional" patch, because there should not be any
> need for the config option now that we have backwards compatibility by
> default.

The patch is nice and small, but I'm still not sure if we really want
input devices to have different modes, changing 'under hands' when one
process switches the mode while another is having the device open.
Things will then break.

I'd really prefer to contain the ugliness in mousedev.c, which then
could be removed completely in 2.8 or so, when XFree and GPM is already
well adapted to the event interface.

> linux-petero/drivers/input/mouse/psmouse-base.c | 2
> linux-petero/drivers/input/mouse/psmouse.h | 1
> linux-petero/drivers/input/mouse/synaptics.c | 62 ++++++++++++++++++++++--
> linux-petero/drivers/input/mouse/synaptics.h | 4 +
> 4 files changed, 63 insertions(+), 6 deletions(-)

--
Vojtech Pavlik
SuSE Labs, SuSE CR

2003-09-21 17:20:36

by Peter Osterlund

[permalink] [raw]
Subject: Re: Broken synaptics mouse..

Vojtech Pavlik <[email protected]> writes:

> On Fri, Sep 19, 2003 at 01:43:11AM +0200, Peter Osterlund wrote:
> > On Thu, 11 Sep 2003, Linus Torvalds wrote:
> >
> > > Actually, I think I've changed my mind.
> > >
> > > How hard would it be to have the "event" driver _notice_ when somebody is
> > > trying to use it?
> > >
> > > In particular, what about something that
> > > - just keeps the touchpad in "ps/2 compatibility mode" by default
> > > - moves to absolute mode only if somebody actually uses the
> > > /dev/input/event0 thing for it (and that would obviously disable the
> > > ps/2 mode).
> > >
> > > That sounds like the simpler setup, especially since the touchpad does a
> > > pretty good job of PS/2 emulation on its own..
> >
> > What do you think about the following patch? This patch makes the touchpad
> > stay in compatibility mode until user space explicitly asks for absolute
> > mode by sending a SYN_CONFIG event with value != 0 to the synaptics event
> > device.
>
> The patch is nice and small, but I'm still not sure if we really want
> input devices to have different modes, changing 'under hands' when one
> process switches the mode while another is having the device open.
> Things will then break.
>
> I'd really prefer to contain the ugliness in mousedev.c, which then
> could be removed completely in 2.8 or so, when XFree and GPM is already
> well adapted to the event interface.

That's certainly possible too. See patch below. Note though that this
patch has the disadvantage mentioned by Dmitry:

We also can't just emulate relative events as everything is
multiplexed into /dev/input/mice and I can see many people
using Synaptics via /dev/input/eventX and everything else via
/dev/input/mice as it nicely handles hot plugging (at least I
use it this way).

linux-petero/drivers/input/mousedev.c | 102 +++++++++++++++++++++++++---------
1 files changed, 76 insertions(+), 26 deletions(-)

diff -puN drivers/input/mousedev.c~mousedev-synaptics drivers/input/mousedev.c
--- linux/drivers/input/mousedev.c~mousedev-synaptics 2003-09-21 14:55:34.000000000 +0200
+++ linux-petero/drivers/input/mousedev.c 2003-09-21 18:58:47.000000000 +0200
@@ -26,6 +26,8 @@
#ifdef CONFIG_INPUT_MOUSEDEV_PSAUX
#include <linux/miscdevice.h>
#endif
+#include <linux/serio.h>
+#include "mouse/psmouse.h" /* For the PSMOUSE_SYNAPTICS definition */

MODULE_AUTHOR("Vojtech Pavlik <[email protected]>");
MODULE_DESCRIPTION("Mouse (ExplorerPS/2) device interfaces");
@@ -58,6 +60,7 @@ struct mousedev_list {
unsigned long buttons;
unsigned char ready, buffer, bufsiz;
unsigned char mode, imexseq, impsseq;
+ int finger;
};

#define MOUSEDEV_SEQ_LEN 6
@@ -73,12 +76,76 @@ static struct mousedev mousedev_mix;
static int xres = CONFIG_INPUT_MOUSEDEV_SCREEN_X;
static int yres = CONFIG_INPUT_MOUSEDEV_SCREEN_Y;

+static void mousedev_abs_event(struct input_handle *handle, struct mousedev_list *list, unsigned int code, int value)
+{
+ int size;
+
+ /* Ignore joysticks */
+ if (test_bit(BTN_TRIGGER, handle->dev->keybit))
+ return;
+
+ /* Handle touchpad data */
+ if (test_bit(ABS_PRESSURE, handle->dev->absbit) &&
+ (handle->dev->id.product == PSMOUSE_SYNAPTICS)) {
+ switch (code) {
+ case ABS_PRESSURE:
+ if (!list->finger) {
+ if (value > 30)
+ list->finger = 1;
+ } else {
+ if (value < 25)
+ list->finger = 0;
+ else if (list->finger < 3)
+ list->finger++;
+ }
+ break;
+ case ABS_X:
+ if (list->finger >= 3) {
+ list->dx += (value - list->oldx) / 8;
+ }
+ list->oldx = value;
+ break;
+ case ABS_Y:
+ if (list->finger >= 3) {
+ list->dy += (value - list->oldy) / 8;
+ }
+ list->oldy = value;
+ break;
+ }
+ return;
+ }
+
+ /* Handle tablet like devices */
+ switch (code) {
+ case ABS_X:
+ size = handle->dev->absmax[ABS_X] - handle->dev->absmin[ABS_X];
+ if (size != 0) {
+ list->dx += (value * xres - list->oldx) / size;
+ list->oldx += list->dx * size;
+ } else {
+ list->dx += value - list->oldx;
+ list->oldx += list->dx;
+ }
+ break;
+ case ABS_Y:
+ size = handle->dev->absmax[ABS_Y] - handle->dev->absmin[ABS_Y];
+ if (size != 0) {
+ list->dy -= (value * yres - list->oldy) / size;
+ list->oldy -= list->dy * size;
+ } else {
+ list->dy -= value - list->oldy;
+ list->oldy -= list->dy;
+ }
+ break;
+ }
+}
+
static void mousedev_event(struct input_handle *handle, unsigned int type, unsigned int code, int value)
{
struct mousedev *mousedevs[3] = { handle->private, &mousedev_mix, NULL };
struct mousedev **mousedev = mousedevs;
struct mousedev_list *list;
- int index, size, wake;
+ int index, wake;

while (*mousedev) {

@@ -87,31 +154,7 @@ static void mousedev_event(struct input_
list_for_each_entry(list, &(*mousedev)->list, node)
switch (type) {
case EV_ABS:
- if (test_bit(BTN_TRIGGER, handle->dev->keybit))
- break;
- switch (code) {
- case ABS_X:
- size = handle->dev->absmax[ABS_X] - handle->dev->absmin[ABS_X];
- if (size != 0) {
- list->dx += (value * xres - list->oldx) / size;
- list->oldx += list->dx * size;
- } else {
- list->dx += value - list->oldx;
- list->oldx += list->dx;
- }
- break;
-
- case ABS_Y:
- size = handle->dev->absmax[ABS_Y] - handle->dev->absmin[ABS_Y];
- if (size != 0) {
- list->dy -= (value * yres - list->oldy) / size;
- list->oldy -= list->dy * size;
- } else {
- list->dy -= value - list->oldy;
- list->oldy -= list->dy;
- }
- break;
- }
+ mousedev_abs_event(handle, list, code, value);
break;

case EV_REL:
@@ -473,6 +516,13 @@ static struct input_device_id mousedev_i
.absbit = { BIT(ABS_X) | BIT(ABS_Y) },
}, /* A tablet like device, at least touch detection, two absolute axes */

+ {
+ .flags = INPUT_DEVICE_ID_MATCH_EVBIT | INPUT_DEVICE_ID_MATCH_ABSBIT | INPUT_DEVICE_ID_MATCH_PRODUCT,
+ .evbit = { BIT(EV_KEY) | BIT(EV_ABS) },
+ .absbit = { BIT(ABS_X) | BIT(ABS_Y) | BIT(ABS_PRESSURE) },
+ .id.product = PSMOUSE_SYNAPTICS,
+ }, /* A synaptics touchpad */
+
{ }, /* Terminating entry */
};


_

--
Peter Osterlund - [email protected]
http://w1.894.telia.com/~u89404340

2003-09-21 17:28:16

by Vojtech Pavlik

[permalink] [raw]
Subject: Re: Broken synaptics mouse..

On Sun, Sep 21, 2003 at 07:20:09PM +0200, Peter Osterlund wrote:

> > I'd really prefer to contain the ugliness in mousedev.c, which then
> > could be removed completely in 2.8 or so, when XFree and GPM is already
> > well adapted to the event interface.
>
> That's certainly possible too. See patch below. Note though that this
> patch has the disadvantage mentioned by Dmitry:
>
> We also can't just emulate relative events as everything is
> multiplexed into /dev/input/mice and I can see many people
> using Synaptics via /dev/input/eventX and everything else via
> /dev/input/mice as it nicely handles hot plugging (at least I
> use it this way).

You can use EVIOCGRAB for the time being in the XFree86 synaptics
driver, this way you'll prevent its events coming into mousedev the
moment it's opened by XFree86, which is probably exactly what one wants.

For the future, I'd really like to get rid of /dev/input/mice, exactly
because it's not possible to configure which devices should be mixed in
there. A userspace solution might be better.

I'll get myself a Synaptics-equipped notebook on Monday, test this, and
if it looks OK, I'll merge it.

One thing I don't particularly like is the check for product ==
PSMOUSE_SYNAPTICS, since it's very non-generic. Also, ABS_PRESSURE is
defined on most tablets ...

We should define a standard set of features (ABS_*, BTN_*) that a
touchpad should have, and what they should mean. I fear the current
Synaptics driver gets some of it wrong. (Like the axis orientation,
which is defined to be the same as the USB HID spec, or the fact that
invalid ABS_* values should not be reported).

> linux-petero/drivers/input/mousedev.c | 102 +++++++++++++++++++++++++---------
> 1 files changed, 76 insertions(+), 26 deletions(-)
>
> diff -puN drivers/input/mousedev.c~mousedev-synaptics drivers/input/mousedev.c
> --- linux/drivers/input/mousedev.c~mousedev-synaptics 2003-09-21 14:55:34.000000000 +0200
> +++ linux-petero/drivers/input/mousedev.c 2003-09-21 18:58:47.000000000 +0200
> @@ -26,6 +26,8 @@
> #ifdef CONFIG_INPUT_MOUSEDEV_PSAUX
> #include <linux/miscdevice.h>
> #endif
> +#include <linux/serio.h>
> +#include "mouse/psmouse.h" /* For the PSMOUSE_SYNAPTICS definition */
>
> MODULE_AUTHOR("Vojtech Pavlik <[email protected]>");
> MODULE_DESCRIPTION("Mouse (ExplorerPS/2) device interfaces");
> @@ -58,6 +60,7 @@ struct mousedev_list {
> unsigned long buttons;
> unsigned char ready, buffer, bufsiz;
> unsigned char mode, imexseq, impsseq;
> + int finger;
> };
>
> #define MOUSEDEV_SEQ_LEN 6
> @@ -73,12 +76,76 @@ static struct mousedev mousedev_mix;
> static int xres = CONFIG_INPUT_MOUSEDEV_SCREEN_X;
> static int yres = CONFIG_INPUT_MOUSEDEV_SCREEN_Y;
>
> +static void mousedev_abs_event(struct input_handle *handle, struct mousedev_list *list, unsigned int code, int value)
> +{
> + int size;
> +
> + /* Ignore joysticks */
> + if (test_bit(BTN_TRIGGER, handle->dev->keybit))
> + return;
> +
> + /* Handle touchpad data */
> + if (test_bit(ABS_PRESSURE, handle->dev->absbit) &&
> + (handle->dev->id.product == PSMOUSE_SYNAPTICS)) {
> + switch (code) {
> + case ABS_PRESSURE:
> + if (!list->finger) {
> + if (value > 30)
> + list->finger = 1;
> + } else {
> + if (value < 25)
> + list->finger = 0;
> + else if (list->finger < 3)
> + list->finger++;
> + }
> + break;
> + case ABS_X:
> + if (list->finger >= 3) {
> + list->dx += (value - list->oldx) / 8;
> + }
> + list->oldx = value;
> + break;
> + case ABS_Y:
> + if (list->finger >= 3) {
> + list->dy += (value - list->oldy) / 8;
> + }
> + list->oldy = value;
> + break;
> + }
> + return;
> + }
> +
> + /* Handle tablet like devices */
> + switch (code) {
> + case ABS_X:
> + size = handle->dev->absmax[ABS_X] - handle->dev->absmin[ABS_X];
> + if (size != 0) {
> + list->dx += (value * xres - list->oldx) / size;
> + list->oldx += list->dx * size;
> + } else {
> + list->dx += value - list->oldx;
> + list->oldx += list->dx;
> + }
> + break;
> + case ABS_Y:
> + size = handle->dev->absmax[ABS_Y] - handle->dev->absmin[ABS_Y];
> + if (size != 0) {
> + list->dy -= (value * yres - list->oldy) / size;
> + list->oldy -= list->dy * size;
> + } else {
> + list->dy -= value - list->oldy;
> + list->oldy -= list->dy;
> + }
> + break;
> + }
> +}
> +
> static void mousedev_event(struct input_handle *handle, unsigned int type, unsigned int code, int value)
> {
> struct mousedev *mousedevs[3] = { handle->private, &mousedev_mix, NULL };
> struct mousedev **mousedev = mousedevs;
> struct mousedev_list *list;
> - int index, size, wake;
> + int index, wake;
>
> while (*mousedev) {
>
> @@ -87,31 +154,7 @@ static void mousedev_event(struct input_
> list_for_each_entry(list, &(*mousedev)->list, node)
> switch (type) {
> case EV_ABS:
> - if (test_bit(BTN_TRIGGER, handle->dev->keybit))
> - break;
> - switch (code) {
> - case ABS_X:
> - size = handle->dev->absmax[ABS_X] - handle->dev->absmin[ABS_X];
> - if (size != 0) {
> - list->dx += (value * xres - list->oldx) / size;
> - list->oldx += list->dx * size;
> - } else {
> - list->dx += value - list->oldx;
> - list->oldx += list->dx;
> - }
> - break;
> -
> - case ABS_Y:
> - size = handle->dev->absmax[ABS_Y] - handle->dev->absmin[ABS_Y];
> - if (size != 0) {
> - list->dy -= (value * yres - list->oldy) / size;
> - list->oldy -= list->dy * size;
> - } else {
> - list->dy -= value - list->oldy;
> - list->oldy -= list->dy;
> - }
> - break;
> - }
> + mousedev_abs_event(handle, list, code, value);
> break;
>
> case EV_REL:
> @@ -473,6 +516,13 @@ static struct input_device_id mousedev_i
> .absbit = { BIT(ABS_X) | BIT(ABS_Y) },
> }, /* A tablet like device, at least touch detection, two absolute axes */
>
> + {
> + .flags = INPUT_DEVICE_ID_MATCH_EVBIT | INPUT_DEVICE_ID_MATCH_ABSBIT | INPUT_DEVICE_ID_MATCH_PRODUCT,
> + .evbit = { BIT(EV_KEY) | BIT(EV_ABS) },
> + .absbit = { BIT(ABS_X) | BIT(ABS_Y) | BIT(ABS_PRESSURE) },
> + .id.product = PSMOUSE_SYNAPTICS,
> + }, /* A synaptics touchpad */
> +
> { }, /* Terminating entry */
> };
>
>
> _
>
> --
> Peter Osterlund - [email protected]
> http://w1.894.telia.com/~u89404340

--
Vojtech Pavlik
SuSE Labs, SuSE CR

2003-09-21 19:34:14

by Vojtech Pavlik

[permalink] [raw]
Subject: Re: Broken synaptics mouse..

On Sun, Sep 21, 2003 at 09:29:10PM +0200, Peter Osterlund wrote:

> OK, below is a new patch that splits the W value as has been suggested
> before. The synaptics driver now reports BTN_TOOL_FINGER,
> BTN_TOOL_DOUBLETAP and BTN_TOOL_TRIPLETAP for one, two and three
> fingers respectively, and it reports ABS_TOOL_WIDTH for the finger
> width value. These event types are also used by mousedev.c to decide
> if it is dealing with a touchpad.
>
> It should also gets the direction of the Y axis right.

At first glance, patch looks OK.

> One thing that it doesn't get right is the handling of invalid ABS_*
> values. How is this supposed to be handled? The driver doesn't know
> the exact limits for the X/Y values, and discarding values outside
> some guessed limits will only have the effect that some parts of the
> touchpad area becomes dead.

I think something like 'if the finger is lifted so much above surface
that X and Y are unreliable, don't report X and Y'. Is that doable?

--
Vojtech Pavlik
SuSE Labs, SuSE CR

2003-09-21 19:29:25

by Peter Osterlund

[permalink] [raw]
Subject: Re: Broken synaptics mouse..

Vojtech Pavlik <[email protected]> writes:

> On Sun, Sep 21, 2003 at 07:20:09PM +0200, Peter Osterlund wrote:
>
> > > I'd really prefer to contain the ugliness in mousedev.c, which then
> > > could be removed completely in 2.8 or so, when XFree and GPM is already
> > > well adapted to the event interface.
> >
> > That's certainly possible too. See patch below. Note though that this
> > patch has the disadvantage mentioned by Dmitry:
> >
> > We also can't just emulate relative events as everything is
> > multiplexed into /dev/input/mice and I can see many people
> > using Synaptics via /dev/input/eventX and everything else via
> > /dev/input/mice as it nicely handles hot plugging (at least I
> > use it this way).
>
> You can use EVIOCGRAB for the time being in the XFree86 synaptics
> driver, this way you'll prevent its events coming into mousedev the
> moment it's opened by XFree86, which is probably exactly what one wants.

OK, I'll try this, but I'll wait until we have worked out what events
we want the kernel driver to report.

> One thing I don't particularly like is the check for product ==
> PSMOUSE_SYNAPTICS, since it's very non-generic. Also, ABS_PRESSURE is
> defined on most tablets ...
>
> We should define a standard set of features (ABS_*, BTN_*) that a
> touchpad should have, and what they should mean. I fear the current
> Synaptics driver gets some of it wrong. (Like the axis orientation,
> which is defined to be the same as the USB HID spec, or the fact that
> invalid ABS_* values should not be reported).

OK, below is a new patch that splits the W value as has been suggested
before. The synaptics driver now reports BTN_TOOL_FINGER,
BTN_TOOL_DOUBLETAP and BTN_TOOL_TRIPLETAP for one, two and three
fingers respectively, and it reports ABS_TOOL_WIDTH for the finger
width value. These event types are also used by mousedev.c to decide
if it is dealing with a touchpad.

It should also gets the direction of the Y axis right.

One thing that it doesn't get right is the handling of invalid ABS_*
values. How is this supposed to be handled? The driver doesn't know
the exact limits for the X/Y values, and discarding values outside
some guessed limits will only have the effect that some parts of the
touchpad area becomes dead.

linux-petero/drivers/input/mouse/synaptics.c | 45 +++++++-----
linux-petero/drivers/input/mousedev.c | 100 +++++++++++++++++++--------
linux-petero/include/linux/input.h | 3
3 files changed, 103 insertions(+), 45 deletions(-)

diff -puN drivers/input/mouse/synaptics.c~mousedev-synaptics drivers/input/mouse/synaptics.c
--- linux/drivers/input/mouse/synaptics.c~mousedev-synaptics 2003-09-21 20:56:18.000000000 +0200
+++ linux-petero/drivers/input/mouse/synaptics.c 2003-09-21 20:56:18.000000000 +0200
@@ -334,11 +334,13 @@ static void set_input_params(struct inpu
set_abs_params(dev, ABS_X, 1472, 5472, 0, 0);
set_abs_params(dev, ABS_Y, 1408, 4448, 0, 0);
set_abs_params(dev, ABS_PRESSURE, 0, 255, 0, 0);
-
- set_bit(EV_MSC, dev->evbit);
- set_bit(MSC_GESTURE, dev->mscbit);
+ set_bit(ABS_TOOL_WIDTH, dev->absbit);

set_bit(EV_KEY, dev->evbit);
+ set_bit(BTN_TOOL_FINGER, dev->keybit);
+ set_bit(BTN_TOOL_DOUBLETAP, dev->keybit);
+ set_bit(BTN_TOOL_TRIPLETAP, dev->keybit);
+
set_bit(BTN_LEFT, dev->keybit);
set_bit(BTN_RIGHT, dev->keybit);
set_bit(BTN_FORWARD, dev->keybit);
@@ -528,42 +530,47 @@ static void synaptics_process_packet(str
struct input_dev *dev = &psmouse->dev;
struct synaptics_data *priv = psmouse->private;
struct synaptics_hw_state hw;
+ int num_fingers;
+ int finger_width;

synaptics_parse_hw_state(psmouse->packet, priv, &hw);

if (hw.z > 0) {
- int w_ok = 0;
- /*
- * Use capability bits to decide if the w value is valid.
- * If not, set it to 5, which corresponds to a finger of
- * normal width.
- */
+ num_fingers = 1;
+ finger_width = 5;
if (SYN_CAP_EXTENDED(priv->capabilities)) {
switch (hw.w) {
case 0 ... 1:
- w_ok = SYN_CAP_MULTIFINGER(priv->capabilities);
+ if (SYN_CAP_MULTIFINGER(priv->capabilities))
+ num_fingers = hw.w + 2;
break;
case 2:
- w_ok = SYN_MODEL_PEN(priv->model_id);
+ if (SYN_MODEL_PEN(priv->model_id))
+ ; /* Nothing, treat a pen as a single finger */
break;
case 4 ... 15:
- w_ok = SYN_CAP_PALMDETECT(priv->capabilities);
+ if (SYN_CAP_PALMDETECT(priv->capabilities))
+ finger_width = hw.w;
break;
}
}
- if (!w_ok)
- hw.w = 5;
+ } else {
+ num_fingers = 0;
+ finger_width = 0;
}

/* Post events */
input_report_abs(dev, ABS_X, hw.x);
- input_report_abs(dev, ABS_Y, hw.y);
+ if (SYN_MODEL_ROT180(priv->model_id))
+ input_report_abs(dev, ABS_Y, -hw.y);
+ else
+ input_report_abs(dev, ABS_Y, hw.y);
input_report_abs(dev, ABS_PRESSURE, hw.z);

- if (hw.w != priv->old_w) {
- input_event(dev, EV_MSC, MSC_GESTURE, hw.w);
- priv->old_w = hw.w;
- }
+ input_report_abs(dev, ABS_TOOL_WIDTH, finger_width);
+ input_report_key(dev, BTN_TOOL_FINGER, num_fingers == 1);
+ input_report_key(dev, BTN_TOOL_DOUBLETAP, num_fingers == 2);
+ input_report_key(dev, BTN_TOOL_TRIPLETAP, num_fingers == 3);

input_report_key(dev, BTN_LEFT, hw.left);
input_report_key(dev, BTN_RIGHT, hw.right);
diff -puN drivers/input/mousedev.c~mousedev-synaptics drivers/input/mousedev.c
--- linux/drivers/input/mousedev.c~mousedev-synaptics 2003-09-21 20:56:18.000000000 +0200
+++ linux-petero/drivers/input/mousedev.c 2003-09-21 20:56:18.000000000 +0200
@@ -58,6 +58,7 @@ struct mousedev_list {
unsigned long buttons;
unsigned char ready, buffer, bufsiz;
unsigned char mode, imexseq, impsseq;
+ int finger;
};

#define MOUSEDEV_SEQ_LEN 6
@@ -73,12 +74,77 @@ static struct mousedev mousedev_mix;
static int xres = CONFIG_INPUT_MOUSEDEV_SCREEN_X;
static int yres = CONFIG_INPUT_MOUSEDEV_SCREEN_Y;

+static void mousedev_abs_event(struct input_handle *handle, struct mousedev_list *list, unsigned int code, int value)
+{
+ int size;
+
+ /* Ignore joysticks */
+ if (test_bit(BTN_TRIGGER, handle->dev->keybit))
+ return;
+
+ /* Handle touchpad data */
+ if (test_bit(BTN_TOOL_FINGER, handle->dev->keybit) &&
+ test_bit(ABS_PRESSURE, handle->dev->absbit) &&
+ test_bit(ABS_TOOL_WIDTH, handle->dev->absbit)) {
+ switch (code) {
+ case ABS_PRESSURE:
+ if (!list->finger) {
+ if (value > 30)
+ list->finger = 1;
+ } else {
+ if (value < 25)
+ list->finger = 0;
+ else if (list->finger < 3)
+ list->finger++;
+ }
+ break;
+ case ABS_X:
+ if (list->finger >= 3) {
+ list->dx += (value - list->oldx) / 8;
+ }
+ list->oldx = value;
+ break;
+ case ABS_Y:
+ if (list->finger >= 3) {
+ list->dy -= (value - list->oldy) / 8;
+ }
+ list->oldy = value;
+ break;
+ }
+ return;
+ }
+
+ /* Handle tablet like devices */
+ switch (code) {
+ case ABS_X:
+ size = handle->dev->absmax[ABS_X] - handle->dev->absmin[ABS_X];
+ if (size != 0) {
+ list->dx += (value * xres - list->oldx) / size;
+ list->oldx += list->dx * size;
+ } else {
+ list->dx += value - list->oldx;
+ list->oldx += list->dx;
+ }
+ break;
+ case ABS_Y:
+ size = handle->dev->absmax[ABS_Y] - handle->dev->absmin[ABS_Y];
+ if (size != 0) {
+ list->dy -= (value * yres - list->oldy) / size;
+ list->oldy -= list->dy * size;
+ } else {
+ list->dy -= value - list->oldy;
+ list->oldy -= list->dy;
+ }
+ break;
+ }
+}
+
static void mousedev_event(struct input_handle *handle, unsigned int type, unsigned int code, int value)
{
struct mousedev *mousedevs[3] = { handle->private, &mousedev_mix, NULL };
struct mousedev **mousedev = mousedevs;
struct mousedev_list *list;
- int index, size, wake;
+ int index, wake;

while (*mousedev) {

@@ -87,31 +153,7 @@ static void mousedev_event(struct input_
list_for_each_entry(list, &(*mousedev)->list, node)
switch (type) {
case EV_ABS:
- if (test_bit(BTN_TRIGGER, handle->dev->keybit))
- break;
- switch (code) {
- case ABS_X:
- size = handle->dev->absmax[ABS_X] - handle->dev->absmin[ABS_X];
- if (size != 0) {
- list->dx += (value * xres - list->oldx) / size;
- list->oldx += list->dx * size;
- } else {
- list->dx += value - list->oldx;
- list->oldx += list->dx;
- }
- break;
-
- case ABS_Y:
- size = handle->dev->absmax[ABS_Y] - handle->dev->absmin[ABS_Y];
- if (size != 0) {
- list->dy -= (value * yres - list->oldy) / size;
- list->oldy -= list->dy * size;
- } else {
- list->dy -= value - list->oldy;
- list->oldy -= list->dy;
- }
- break;
- }
+ mousedev_abs_event(handle, list, code, value);
break;

case EV_REL:
@@ -472,6 +514,12 @@ static struct input_device_id mousedev_i
.keybit = { [LONG(BTN_TOUCH)] = BIT(BTN_TOUCH) },
.absbit = { BIT(ABS_X) | BIT(ABS_Y) },
}, /* A tablet like device, at least touch detection, two absolute axes */
+ {
+ .flags = INPUT_DEVICE_ID_MATCH_EVBIT | INPUT_DEVICE_ID_MATCH_KEYBIT | INPUT_DEVICE_ID_MATCH_ABSBIT,
+ .evbit = { BIT(EV_KEY) | BIT(EV_ABS) },
+ .keybit = { [LONG(BTN_TOOL_FINGER)] = BIT(BTN_TOOL_FINGER) },
+ .absbit = { BIT(ABS_X) | BIT(ABS_Y) | BIT(ABS_PRESSURE) | BIT(ABS_TOOL_WIDTH) },
+ }, /* A touchpad */

{ }, /* Terminating entry */
};
diff -puN include/linux/input.h~mousedev-synaptics include/linux/input.h
--- linux/include/linux/input.h~mousedev-synaptics 2003-09-21 20:56:18.000000000 +0200
+++ linux-petero/include/linux/input.h 2003-09-21 20:56:18.000000000 +0200
@@ -404,6 +404,8 @@ struct input_absinfo {
#define BTN_TOUCH 0x14a
#define BTN_STYLUS 0x14b
#define BTN_STYLUS2 0x14c
+#define BTN_TOOL_DOUBLETAP 0x14d
+#define BTN_TOOL_TRIPLETAP 0x14e

#define BTN_WHEEL 0x150
#define BTN_GEAR_DOWN 0x150
@@ -521,6 +523,7 @@ struct input_absinfo {
#define ABS_DISTANCE 0x19
#define ABS_TILT_X 0x1a
#define ABS_TILT_Y 0x1b
+#define ABS_TOOL_WIDTH 0x1c
#define ABS_VOLUME 0x20
#define ABS_MISC 0x28
#define ABS_MAX 0x3f

_

--
Peter Osterlund - [email protected]
http://w1.894.telia.com/~u89404340

2003-09-21 20:26:44

by Peter Osterlund

[permalink] [raw]
Subject: Re: Broken synaptics mouse..

Vojtech Pavlik <[email protected]> writes:

> On Sun, Sep 21, 2003 at 09:29:10PM +0200, Peter Osterlund wrote:
>
> > One thing that it doesn't get right is the handling of invalid ABS_*
> > values. How is this supposed to be handled? The driver doesn't know
> > the exact limits for the X/Y values, and discarding values outside
> > some guessed limits will only have the effect that some parts of the
> > touchpad area becomes dead.
>
> I think something like 'if the finger is lifted so much above surface
> that X and Y are unreliable, don't report X and Y'. Is that doable?

Yes, it should be sufficient to only report X/Y when Z>0. (I thought
invalid values referred to all values outside the limits defined by
dev->absmin and absmax, hence my previous comment.)

Here is a new patch:

linux-petero/drivers/input/mouse/synaptics.c | 68 +++++++++++-------
linux-petero/drivers/input/mousedev.c | 100 +++++++++++++++++++--------
linux-petero/include/linux/input.h | 3
3 files changed, 118 insertions(+), 53 deletions(-)

diff -puN drivers/input/mouse/synaptics.c~mousedev-synaptics drivers/input/mouse/synaptics.c
--- linux/drivers/input/mouse/synaptics.c~mousedev-synaptics 2003-09-21 20:56:18.000000000 +0200
+++ linux-petero/drivers/input/mouse/synaptics.c 2003-09-21 22:13:43.000000000 +0200
@@ -28,6 +28,16 @@
#include "psmouse.h"
#include "synaptics.h"

+/*
+ * The x/y limits are taken from the Synaptics TouchPad interfacing Guide,
+ * section 2.3.2, which says that they should be valid regardless of the
+ * actual size of the sensor.
+ */
+#define XMIN_NOMINAL 1472
+#define XMAX_NOMINAL 5472
+#define YMIN_NOMINAL 1408
+#define YMAX_NOMINAL 4448
+
/*****************************************************************************
* Synaptics communications functions
****************************************************************************/
@@ -325,20 +335,17 @@ static inline void set_abs_params(struct

static void set_input_params(struct input_dev *dev, struct synaptics_data *priv)
{
- /*
- * The x/y limits are taken from the Synaptics TouchPad interfacing Guide,
- * which says that they should be valid regardless of the actual size of
- * the sensor.
- */
set_bit(EV_ABS, dev->evbit);
- set_abs_params(dev, ABS_X, 1472, 5472, 0, 0);
- set_abs_params(dev, ABS_Y, 1408, 4448, 0, 0);
+ set_abs_params(dev, ABS_X, XMIN_NOMINAL, XMAX_NOMINAL, 0, 0);
+ set_abs_params(dev, ABS_Y, YMIN_NOMINAL, YMAX_NOMINAL, 0, 0);
set_abs_params(dev, ABS_PRESSURE, 0, 255, 0, 0);
-
- set_bit(EV_MSC, dev->evbit);
- set_bit(MSC_GESTURE, dev->mscbit);
+ set_bit(ABS_TOOL_WIDTH, dev->absbit);

set_bit(EV_KEY, dev->evbit);
+ set_bit(BTN_TOOL_FINGER, dev->keybit);
+ set_bit(BTN_TOOL_DOUBLETAP, dev->keybit);
+ set_bit(BTN_TOOL_TRIPLETAP, dev->keybit);
+
set_bit(BTN_LEFT, dev->keybit);
set_bit(BTN_RIGHT, dev->keybit);
set_bit(BTN_FORWARD, dev->keybit);
@@ -528,42 +535,49 @@ static void synaptics_process_packet(str
struct input_dev *dev = &psmouse->dev;
struct synaptics_data *priv = psmouse->private;
struct synaptics_hw_state hw;
+ int num_fingers;
+ int finger_width;

synaptics_parse_hw_state(psmouse->packet, priv, &hw);

if (hw.z > 0) {
- int w_ok = 0;
- /*
- * Use capability bits to decide if the w value is valid.
- * If not, set it to 5, which corresponds to a finger of
- * normal width.
- */
+ num_fingers = 1;
+ finger_width = 5;
if (SYN_CAP_EXTENDED(priv->capabilities)) {
switch (hw.w) {
case 0 ... 1:
- w_ok = SYN_CAP_MULTIFINGER(priv->capabilities);
+ if (SYN_CAP_MULTIFINGER(priv->capabilities))
+ num_fingers = hw.w + 2;
break;
case 2:
- w_ok = SYN_MODEL_PEN(priv->model_id);
+ if (SYN_MODEL_PEN(priv->model_id))
+ ; /* Nothing, treat a pen as a single finger */
break;
case 4 ... 15:
- w_ok = SYN_CAP_PALMDETECT(priv->capabilities);
+ if (SYN_CAP_PALMDETECT(priv->capabilities))
+ finger_width = hw.w;
break;
}
}
- if (!w_ok)
- hw.w = 5;
+ } else {
+ num_fingers = 0;
+ finger_width = 0;
}

/* Post events */
- input_report_abs(dev, ABS_X, hw.x);
- input_report_abs(dev, ABS_Y, hw.y);
+ if (hw.z > 0) {
+ input_report_abs(dev, ABS_X, hw.x);
+ if (SYN_MODEL_ROT180(priv->model_id))
+ input_report_abs(dev, ABS_Y, YMAX_NOMINAL + YMIN_NOMINAL - hw.y);
+ else
+ input_report_abs(dev, ABS_Y, hw.y);
+ }
input_report_abs(dev, ABS_PRESSURE, hw.z);

- if (hw.w != priv->old_w) {
- input_event(dev, EV_MSC, MSC_GESTURE, hw.w);
- priv->old_w = hw.w;
- }
+ input_report_abs(dev, ABS_TOOL_WIDTH, finger_width);
+ input_report_key(dev, BTN_TOOL_FINGER, num_fingers == 1);
+ input_report_key(dev, BTN_TOOL_DOUBLETAP, num_fingers == 2);
+ input_report_key(dev, BTN_TOOL_TRIPLETAP, num_fingers == 3);

input_report_key(dev, BTN_LEFT, hw.left);
input_report_key(dev, BTN_RIGHT, hw.right);
diff -puN drivers/input/mousedev.c~mousedev-synaptics drivers/input/mousedev.c
--- linux/drivers/input/mousedev.c~mousedev-synaptics 2003-09-21 20:56:18.000000000 +0200
+++ linux-petero/drivers/input/mousedev.c 2003-09-21 20:56:18.000000000 +0200
@@ -58,6 +58,7 @@ struct mousedev_list {
unsigned long buttons;
unsigned char ready, buffer, bufsiz;
unsigned char mode, imexseq, impsseq;
+ int finger;
};

#define MOUSEDEV_SEQ_LEN 6
@@ -73,12 +74,77 @@ static struct mousedev mousedev_mix;
static int xres = CONFIG_INPUT_MOUSEDEV_SCREEN_X;
static int yres = CONFIG_INPUT_MOUSEDEV_SCREEN_Y;

+static void mousedev_abs_event(struct input_handle *handle, struct mousedev_list *list, unsigned int code, int value)
+{
+ int size;
+
+ /* Ignore joysticks */
+ if (test_bit(BTN_TRIGGER, handle->dev->keybit))
+ return;
+
+ /* Handle touchpad data */
+ if (test_bit(BTN_TOOL_FINGER, handle->dev->keybit) &&
+ test_bit(ABS_PRESSURE, handle->dev->absbit) &&
+ test_bit(ABS_TOOL_WIDTH, handle->dev->absbit)) {
+ switch (code) {
+ case ABS_PRESSURE:
+ if (!list->finger) {
+ if (value > 30)
+ list->finger = 1;
+ } else {
+ if (value < 25)
+ list->finger = 0;
+ else if (list->finger < 3)
+ list->finger++;
+ }
+ break;
+ case ABS_X:
+ if (list->finger >= 3) {
+ list->dx += (value - list->oldx) / 8;
+ }
+ list->oldx = value;
+ break;
+ case ABS_Y:
+ if (list->finger >= 3) {
+ list->dy -= (value - list->oldy) / 8;
+ }
+ list->oldy = value;
+ break;
+ }
+ return;
+ }
+
+ /* Handle tablet like devices */
+ switch (code) {
+ case ABS_X:
+ size = handle->dev->absmax[ABS_X] - handle->dev->absmin[ABS_X];
+ if (size != 0) {
+ list->dx += (value * xres - list->oldx) / size;
+ list->oldx += list->dx * size;
+ } else {
+ list->dx += value - list->oldx;
+ list->oldx += list->dx;
+ }
+ break;
+ case ABS_Y:
+ size = handle->dev->absmax[ABS_Y] - handle->dev->absmin[ABS_Y];
+ if (size != 0) {
+ list->dy -= (value * yres - list->oldy) / size;
+ list->oldy -= list->dy * size;
+ } else {
+ list->dy -= value - list->oldy;
+ list->oldy -= list->dy;
+ }
+ break;
+ }
+}
+
static void mousedev_event(struct input_handle *handle, unsigned int type, unsigned int code, int value)
{
struct mousedev *mousedevs[3] = { handle->private, &mousedev_mix, NULL };
struct mousedev **mousedev = mousedevs;
struct mousedev_list *list;
- int index, size, wake;
+ int index, wake;

while (*mousedev) {

@@ -87,31 +153,7 @@ static void mousedev_event(struct input_
list_for_each_entry(list, &(*mousedev)->list, node)
switch (type) {
case EV_ABS:
- if (test_bit(BTN_TRIGGER, handle->dev->keybit))
- break;
- switch (code) {
- case ABS_X:
- size = handle->dev->absmax[ABS_X] - handle->dev->absmin[ABS_X];
- if (size != 0) {
- list->dx += (value * xres - list->oldx) / size;
- list->oldx += list->dx * size;
- } else {
- list->dx += value - list->oldx;
- list->oldx += list->dx;
- }
- break;
-
- case ABS_Y:
- size = handle->dev->absmax[ABS_Y] - handle->dev->absmin[ABS_Y];
- if (size != 0) {
- list->dy -= (value * yres - list->oldy) / size;
- list->oldy -= list->dy * size;
- } else {
- list->dy -= value - list->oldy;
- list->oldy -= list->dy;
- }
- break;
- }
+ mousedev_abs_event(handle, list, code, value);
break;

case EV_REL:
@@ -472,6 +514,12 @@ static struct input_device_id mousedev_i
.keybit = { [LONG(BTN_TOUCH)] = BIT(BTN_TOUCH) },
.absbit = { BIT(ABS_X) | BIT(ABS_Y) },
}, /* A tablet like device, at least touch detection, two absolute axes */
+ {
+ .flags = INPUT_DEVICE_ID_MATCH_EVBIT | INPUT_DEVICE_ID_MATCH_KEYBIT | INPUT_DEVICE_ID_MATCH_ABSBIT,
+ .evbit = { BIT(EV_KEY) | BIT(EV_ABS) },
+ .keybit = { [LONG(BTN_TOOL_FINGER)] = BIT(BTN_TOOL_FINGER) },
+ .absbit = { BIT(ABS_X) | BIT(ABS_Y) | BIT(ABS_PRESSURE) | BIT(ABS_TOOL_WIDTH) },
+ }, /* A touchpad */

{ }, /* Terminating entry */
};
diff -puN include/linux/input.h~mousedev-synaptics include/linux/input.h
--- linux/include/linux/input.h~mousedev-synaptics 2003-09-21 20:56:18.000000000 +0200
+++ linux-petero/include/linux/input.h 2003-09-21 20:56:18.000000000 +0200
@@ -404,6 +404,8 @@ struct input_absinfo {
#define BTN_TOUCH 0x14a
#define BTN_STYLUS 0x14b
#define BTN_STYLUS2 0x14c
+#define BTN_TOOL_DOUBLETAP 0x14d
+#define BTN_TOOL_TRIPLETAP 0x14e

#define BTN_WHEEL 0x150
#define BTN_GEAR_DOWN 0x150
@@ -521,6 +523,7 @@ struct input_absinfo {
#define ABS_DISTANCE 0x19
#define ABS_TILT_X 0x1a
#define ABS_TILT_Y 0x1b
+#define ABS_TOOL_WIDTH 0x1c
#define ABS_VOLUME 0x20
#define ABS_MISC 0x28
#define ABS_MAX 0x3f

_

--
Peter Osterlund - [email protected]
http://w1.894.telia.com/~u89404340

2003-09-21 20:42:57

by Vojtech Pavlik

[permalink] [raw]
Subject: Re: Broken synaptics mouse..

On Sun, Sep 21, 2003 at 10:26:17PM +0200, Peter Osterlund wrote:
> Vojtech Pavlik <[email protected]> writes:
>
> > On Sun, Sep 21, 2003 at 09:29:10PM +0200, Peter Osterlund wrote:
> >
> > > One thing that it doesn't get right is the handling of invalid ABS_*
> > > values. How is this supposed to be handled? The driver doesn't know
> > > the exact limits for the X/Y values, and discarding values outside
> > > some guessed limits will only have the effect that some parts of the
> > > touchpad area becomes dead.
> >
> > I think something like 'if the finger is lifted so much above surface
> > that X and Y are unreliable, don't report X and Y'. Is that doable?
>
> Yes, it should be sufficient to only report X/Y when Z>0. (I thought
> invalid values referred to all values outside the limits defined by
> dev->absmin and absmax, hence my previous comment.)
>
> Here is a new patch:
>
> linux-petero/drivers/input/mouse/synaptics.c | 68 +++++++++++-------
> linux-petero/drivers/input/mousedev.c | 100 +++++++++++++++++++--------
> linux-petero/include/linux/input.h | 3
> 3 files changed, 118 insertions(+), 53 deletions(-)

Yes, this now looks very nice. Applied.

--
Vojtech Pavlik
SuSE Labs, SuSE CR

2003-09-21 23:16:41

by Dmitry Torokhov

[permalink] [raw]
Subject: Re: Broken synaptics mouse..

On Sunday 21 September 2003 12:27 pm, Vojtech Pavlik wrote:
> On Sun, Sep 21, 2003 at 07:20:09PM +0200, Peter Osterlund wrote:
> > > I'd really prefer to contain the ugliness in mousedev.c, which then
> > > could be removed completely in 2.8 or so, when XFree and GPM is already
> > > well adapted to the event interface.
> >
> > That's certainly possible too. See patch below. Note though that this
> > patch has the disadvantage mentioned by Dmitry:
> >
> > We also can't just emulate relative events as everything is
> > multiplexed into /dev/input/mice and I can see many people
> > using Synaptics via /dev/input/eventX and everything else via
> > /dev/input/mice as it nicely handles hot plugging (at least I
> > use it this way).
>
> You can use EVIOCGRAB for the time being in the XFree86 synaptics
> driver, this way you'll prevent its events coming into mousedev the
> moment it's opened by XFree86, which is probably exactly what one wants.
>

Will that allow 2 processes to have access to the same event device
simultaneously? I am thinking about XFree and GPM. We just got away from
that mess caused by psaux providing only exclusive access to step into
the same problem again.

Dmitry

2003-09-22 05:31:30

by Vojtech Pavlik

[permalink] [raw]
Subject: Re: Broken synaptics mouse..

On Sun, Sep 21, 2003 at 06:16:36PM -0500, Dmitry Torokhov wrote:

> > You can use EVIOCGRAB for the time being in the XFree86 synaptics
> > driver, this way you'll prevent its events coming into mousedev the
> > moment it's opened by XFree86, which is probably exactly what one wants.
>
> Will that allow 2 processes to have access to the same event device
> simultaneously? I am thinking about XFree and GPM. We just got away from
> that mess caused by psaux providing only exclusive access to step into
> the same problem again.

No, it won't. Yes, it's a problem. The only solution I can propose here
is when you want GPM and XFree support simultaneously you have to
configure both to use either /dev/input/mice, or both /dev/input/event,
and not mix the two together.

The EVIOCGRAB thing could be optional in X.

--
Vojtech Pavlik
SuSE Labs, SuSE CR

2003-09-22 06:01:46

by Dmitry Torokhov

[permalink] [raw]
Subject: Re: Broken synaptics mouse..

On Monday 22 September 2003 12:31 am, Vojtech Pavlik wrote:
> On Sun, Sep 21, 2003 at 06:16:36PM -0500, Dmitry Torokhov wrote:
> > > You can use EVIOCGRAB for the time being in the XFree86 synaptics
> > > driver, this way you'll prevent its events coming into mousedev the
> > > moment it's opened by XFree86, which is probably exactly what one
> > > wants.
> >
> > Will that allow 2 processes to have access to the same event device
> > simultaneously? I am thinking about XFree and GPM. We just got away from
> > that mess caused by psaux providing only exclusive access to step into
> > the same problem again.
>
> No, it won't. Yes, it's a problem. The only solution I can propose here
> is when you want GPM and XFree support simultaneously you have to
> configure both to use either /dev/input/mice, or both /dev/input/event,
> and not mix the two together.
>

But in this case not only will I have to specify the event device Synaptics
is connected to but also explicitly specify _every other_ input device I use
(besides the touchpad I have a track-stick as a separate device and an USB
mouse in my docking station). I will also loose hot-plug capabilities I have
now for free.

All in all it just doesn't fly... I wonder if we could declare evdev the master
handler and do not propagate events to the secondary handlers if some process
has appropriate event device opened.

Dmitry

2003-09-22 06:09:50

by Vojtech Pavlik

[permalink] [raw]
Subject: Re: Broken synaptics mouse..

On Mon, Sep 22, 2003 at 12:58:48AM -0500, Dmitry Torokhov wrote:

> But in this case not only will I have to specify the event device Synaptics
> is connected to but also explicitly specify _every other_ input device I use
> (besides the touchpad I have a track-stick as a separate device and an USB
> mouse in my docking station). I will also loose hot-plug capabilities I have
> now for free.

The problem is that nothing comes for free. The hotplug capability you
have now comes at the cost of utter nonconfigurability, that is - it
gets input from every pointing devices.

The bright-future solution is to have GPM and XFree configured at
runtime by the /sbin/hotplug agent to open/close devices as they come
and go.

This again comes at a cost - more complex setup - but one that is
completely configurable as to which device goes where, stable in regard
of plug-in order, etc.

> All in all it just doesn't fly... I wonder if we could declare evdev
> the master handler and do not propagate events to the secondary
> handlers if some process has appropriate event device opened.

We could. Don't do a 'cat /dev/input/event0' if event0 is your keyboard
then, though. ;)

We also could change the EVIOCGRAB semantics to only grab in respect to
the handler, not just the single handle ...

But I still hope we'll be able to get to the /sbin/hotplug solution
without having to create too many intermediate solutions.

--
Vojtech Pavlik
SuSE Labs, SuSE CR