Discussion:
[PATCH 0/5] udev/hwdb: Add support for setting trackpoint properties
(too old to reply)
Hans de Goede
2015-04-02 09:48:46 UTC
Permalink
Hi All,

Testing has shown that the default experience of trackpoints on different
model laptops differs quite a lot, from almost unusably fast, to just plain
unusably slow on some models.

It seems that before libinput people where using the huge amount of
configurability Xorg gives them to work around this, this is not a
situation which we want to replicate with libinput. Instead the plan is
to where necessary add hwdb entries telling libinput apply a constant
(linear) acceleration factor to deltas from the trackpoint to normalize
the deltas.

This patch-set also adds some special handling for IBM/Lenovo trackpoints which
also have a sysfs driver setting which allows tweaking the sensitivity and on
some models we need to set a specific value to get the best user experience.

This patch-set has been tested (together with a matching libinput patch) on
5 different models sofar: Thinkpad X200s, Thinkpad T440s, Dell Latitude D620,
Dell Latitude E6400, Dell Latitude E6430.

Note the 2 ThinkPad models where specifically chosen as they are known to
have a bad ootb trackpoint experience, so unsurprisingly both end up with
hwdb entries, see:

https://bugzilla.redhat.com/show_bug.cgi?id=1200717
https://forums.lenovo.com/t5/X-Series-ThinkPad-Laptops/Trackpoint-quot-too-slow-quot-proglem-of-X1-Carbon-Gen-2/td-p/1603962

Regards,

Hans
Hans de Goede
2015-04-02 09:48:47 UTC
Permalink
The kernel has been setting the INPUT_PROP_POINTING_STICK property bit
on trackpoints for a while now, and this is useful information to have
in various places, so make input_id aware of this and make it set
ID_INPUT_POINTING_STICK on trackpoints.

While adding support for querying properties, also add support for the
recently added INPUT_PROP_ACCELEROMETER property bit.

Signed-off-by: Hans de Goede <***@redhat.com>
---
src/udev/udev-builtin-input_id.c | 21 +++++++++++++++++++--
1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/src/udev/udev-builtin-input_id.c b/src/udev/udev-builtin-input_id.c
index 46f1c53..5b1790c 100644
--- a/src/udev/udev-builtin-input_id.c
+++ b/src/udev/udev-builtin-input_id.c
@@ -33,6 +33,14 @@
#include "udev.h"
#include "util.h"

+#ifndef INPUT_PROP_POINTING_STICK
+#define INPUT_PROP_POINTING_STICK 0x05
+#endif
+
+#ifndef INPUT_PROP_ACCELEROMETER
+#define INPUT_PROP_ACCELEROMETER 0x06
+#endif
+
/* we must use this kernel-compatible implementation */
#define BITS_PER_LONG (sizeof(unsigned long) * 8)
#define NBITS(x) ((((x)-1)/BITS_PER_LONG)+1)
@@ -131,6 +139,7 @@ static void test_pointers (struct udev_device *dev,
const unsigned long* bitmask_abs,
const unsigned long* bitmask_key,
const unsigned long* bitmask_rel,
+ unsigned long prop,
bool test) {
int is_mouse = 0;
int is_touchpad = 0;
@@ -182,6 +191,10 @@ static void test_pointers (struct udev_device *dev,
udev_builtin_add_property(dev, test, "ID_INPUT_MOUSE", "1");
if (is_touchpad)
udev_builtin_add_property(dev, test, "ID_INPUT_TOUCHPAD", "1");
+ if (prop & (1 << INPUT_PROP_POINTING_STICK))
+ udev_builtin_add_property(dev, test, "ID_INPUT_TRACKPOINT", "1");
+ if (prop & (1 << INPUT_PROP_ACCELEROMETER))
+ udev_builtin_add_property(dev, test, "ID_INPUT_ACCELEROMETER", "1");
}

/* key like devices */
@@ -232,7 +245,8 @@ static int builtin_input_id(struct udev_device *dev, int argc, char *argv[], boo
unsigned long bitmask_abs[NBITS(ABS_MAX)];
unsigned long bitmask_key[NBITS(KEY_MAX)];
unsigned long bitmask_rel[NBITS(REL_MAX)];
- const char *sysname, *devnode;
+ unsigned long prop = 0;
+ const char *sysname, *devnode, *prop_str;

/* walk up the parental chain until we find the real input device; the
* argument is very likely a subdevice of this, like eventN */
@@ -248,7 +262,10 @@ static int builtin_input_id(struct udev_device *dev, int argc, char *argv[], boo
get_cap_mask(dev, pdev, "capabilities/abs", bitmask_abs, sizeof(bitmask_abs), test);
get_cap_mask(dev, pdev, "capabilities/rel", bitmask_rel, sizeof(bitmask_rel), test);
get_cap_mask(dev, pdev, "capabilities/key", bitmask_key, sizeof(bitmask_key), test);
- test_pointers(dev, bitmask_ev, bitmask_abs, bitmask_key, bitmask_rel, test);
+ prop_str = udev_device_get_property_value(pdev, "PROP");
+ if (prop_str)
+ prop = strtoul(prop_str, NULL, 16);
+ test_pointers(dev, bitmask_ev, bitmask_abs, bitmask_key, bitmask_rel, prop, test);
test_key(dev, bitmask_ev, bitmask_key, test);
}
--
2.3.4
Lennart Poettering
2015-04-02 10:21:08 UTC
Permalink
Post by Hans de Goede
The kernel has been setting the INPUT_PROP_POINTING_STICK property bit
on trackpoints for a while now, and this is useful information to have
in various places, so make input_id aware of this and make it set
ID_INPUT_POINTING_STICK on trackpoints.
While adding support for querying properties, also add support for the
recently added INPUT_PROP_ACCELEROMETER property bit.
---
src/udev/udev-builtin-input_id.c | 21 +++++++++++++++++++--
1 file changed, 19 insertions(+), 2 deletions(-)
diff --git a/src/udev/udev-builtin-input_id.c b/src/udev/udev-builtin-input_id.c
index 46f1c53..5b1790c 100644
--- a/src/udev/udev-builtin-input_id.c
+++ b/src/udev/udev-builtin-input_id.c
@@ -33,6 +33,14 @@
#include "udev.h"
#include "util.h"
+#ifndef INPUT_PROP_POINTING_STICK
+#define INPUT_PROP_POINTING_STICK 0x05
+#endif
+
+#ifndef INPUT_PROP_ACCELEROMETER
+#define INPUT_PROP_ACCELEROMETER 0x06
+#endif
Are these definitions normally defined in input.h? If so, please add
them to missing.h, where we try to centralize definitions of this kind.
Post by Hans de Goede
/* we must use this kernel-compatible implementation */
#define BITS_PER_LONG (sizeof(unsigned long) * 8)
#define NBITS(x) ((((x)-1)/BITS_PER_LONG)+1)
@@ -131,6 +139,7 @@ static void test_pointers (struct udev_device *dev,
const unsigned long* bitmask_abs,
const unsigned long* bitmask_key,
const unsigned long* bitmask_rel,
+ unsigned long prop,
"unsigned long"? Is this really necessary? Shouldn't we just use
uint64_t? here?
Post by Hans de Goede
bool test) {
int is_mouse = 0;
int is_touchpad = 0;
@@ -182,6 +191,10 @@ static void test_pointers (struct udev_device *dev,
udev_builtin_add_property(dev, test, "ID_INPUT_MOUSE", "1");
if (is_touchpad)
udev_builtin_add_property(dev, test, "ID_INPUT_TOUCHPAD", "1");
+ if (prop & (1 << INPUT_PROP_POINTING_STICK))
+ udev_builtin_add_property(dev, test, "ID_INPUT_TRACKPOINT", "1");
+ if (prop & (1 << INPUT_PROP_ACCELEROMETER))
+ udev_builtin_add_property(dev, test, "ID_INPUT_ACCELEROMETER", "1");
}
/* key like devices */
@@ -232,7 +245,8 @@ static int builtin_input_id(struct udev_device *dev, int argc, char *argv[], boo
unsigned long bitmask_abs[NBITS(ABS_MAX)];
unsigned long bitmask_key[NBITS(KEY_MAX)];
unsigned long bitmask_rel[NBITS(REL_MAX)];
- const char *sysname, *devnode;
+ unsigned long prop = 0;
+ const char *sysname, *devnode, *prop_str;
/* walk up the parental chain until we find the real input device; the
* argument is very likely a subdevice of this, like eventN */
@@ -248,7 +262,10 @@ static int builtin_input_id(struct udev_device *dev, int argc, char *argv[], boo
get_cap_mask(dev, pdev, "capabilities/abs", bitmask_abs, sizeof(bitmask_abs), test);
get_cap_mask(dev, pdev, "capabilities/rel", bitmask_rel, sizeof(bitmask_rel), test);
get_cap_mask(dev, pdev, "capabilities/key", bitmask_key, sizeof(bitmask_key), test);
- test_pointers(dev, bitmask_ev, bitmask_abs, bitmask_key, bitmask_rel, test);
+ prop_str = udev_device_get_property_value(pdev, "PROP");
+ if (prop_str)
+ prop = strtoul(prop_str, NULL, 16);
Hmm, we try to avoid direct invocations of strtoul() and friends, due
to the annoying error handling... Can't we use safe_atou64() here, or so?

Lennart
--
Lennart Poettering, Red Hat
Lennart Poettering
2015-04-02 10:27:18 UTC
Permalink
Post by Lennart Poettering
Post by Hans de Goede
The kernel has been setting the INPUT_PROP_POINTING_STICK property bit
on trackpoints for a while now, and this is useful information to have
in various places, so make input_id aware of this and make it set
ID_INPUT_POINTING_STICK on trackpoints.
While adding support for querying properties, also add support for the
recently added INPUT_PROP_ACCELEROMETER property bit.
---
src/udev/udev-builtin-input_id.c | 21 +++++++++++++++++++--
1 file changed, 19 insertions(+), 2 deletions(-)
diff --git a/src/udev/udev-builtin-input_id.c b/src/udev/udev-builtin-input_id.c
index 46f1c53..5b1790c 100644
--- a/src/udev/udev-builtin-input_id.c
+++ b/src/udev/udev-builtin-input_id.c
@@ -33,6 +33,14 @@
#include "udev.h"
#include "util.h"
+#ifndef INPUT_PROP_POINTING_STICK
+#define INPUT_PROP_POINTING_STICK 0x05
+#endif
+
+#ifndef INPUT_PROP_ACCELEROMETER
+#define INPUT_PROP_ACCELEROMETER 0x06
+#endif
Are these definitions normally defined in input.h? If so, please add
them to missing.h, where we try to centralize definitions of this kind.
Also, INPUT_PROP_POINTING_STICK is already introduced by Peter
Hutterer's most recent patch. He adds it to missing.h, can you sync
the two patche sets up, please?

Lennart
--
Lennart Poettering, Red Hat
Hans de Goede
2015-04-02 10:42:55 UTC
Permalink
Hi,
Post by Lennart Poettering
Post by Lennart Poettering
Post by Hans de Goede
The kernel has been setting the INPUT_PROP_POINTING_STICK property bit
on trackpoints for a while now, and this is useful information to have
in various places, so make input_id aware of this and make it set
ID_INPUT_POINTING_STICK on trackpoints.
While adding support for querying properties, also add support for the
recently added INPUT_PROP_ACCELEROMETER property bit.
---
src/udev/udev-builtin-input_id.c | 21 +++++++++++++++++++--
1 file changed, 19 insertions(+), 2 deletions(-)
diff --git a/src/udev/udev-builtin-input_id.c b/src/udev/udev-builtin-input_id.c
index 46f1c53..5b1790c 100644
--- a/src/udev/udev-builtin-input_id.c
+++ b/src/udev/udev-builtin-input_id.c
@@ -33,6 +33,14 @@
#include "udev.h"
#include "util.h"
+#ifndef INPUT_PROP_POINTING_STICK
+#define INPUT_PROP_POINTING_STICK 0x05
+#endif
+
+#ifndef INPUT_PROP_ACCELEROMETER
+#define INPUT_PROP_ACCELEROMETER 0x06
+#endif
Are these definitions normally defined in input.h? If so, please add
them to missing.h, where we try to centralize definitions of this kind.
Also, INPUT_PROP_POINTING_STICK is already introduced by Peter
Hutterer's most recent patch. He adds it to missing.h, can you sync
the two patche sets up, please?
Sure I'll base my set on top of Peters for V2.

Regards,

Hans
Hans de Goede
2015-04-03 09:51:59 UTC
Permalink
Hi,
Post by Lennart Poettering
Post by Lennart Poettering
Post by Hans de Goede
The kernel has been setting the INPUT_PROP_POINTING_STICK property bit
on trackpoints for a while now, and this is useful information to have
in various places, so make input_id aware of this and make it set
ID_INPUT_POINTING_STICK on trackpoints.
While adding support for querying properties, also add support for the
recently added INPUT_PROP_ACCELEROMETER property bit.
---
src/udev/udev-builtin-input_id.c | 21 +++++++++++++++++++--
1 file changed, 19 insertions(+), 2 deletions(-)
diff --git a/src/udev/udev-builtin-input_id.c b/src/udev/udev-builtin-input_id.c
index 46f1c53..5b1790c 100644
--- a/src/udev/udev-builtin-input_id.c
+++ b/src/udev/udev-builtin-input_id.c
@@ -33,6 +33,14 @@
#include "udev.h"
#include "util.h"
+#ifndef INPUT_PROP_POINTING_STICK
+#define INPUT_PROP_POINTING_STICK 0x05
+#endif
+
+#ifndef INPUT_PROP_ACCELEROMETER
+#define INPUT_PROP_ACCELEROMETER 0x06
+#endif
Are these definitions normally defined in input.h? If so, please add
them to missing.h, where we try to centralize definitions of this kind.
Also, INPUT_PROP_POINTING_STICK is already introduced by Peter
Hutterer's most recent patch. He adds it to missing.h, can you sync
the two patche sets up, please?
Actually Peter's patch pretty much does the same as my patch, except
that he does not add support for the recently added INPUT_PROP_ACCELEROMETER
bit.

So I'll completely drop this patch from v2 of my set.

Regards,

Hans

Hans de Goede
2015-04-02 10:39:23 UTC
Permalink
Hi,
Post by Lennart Poettering
Post by Hans de Goede
The kernel has been setting the INPUT_PROP_POINTING_STICK property bit
on trackpoints for a while now, and this is useful information to have
in various places, so make input_id aware of this and make it set
ID_INPUT_POINTING_STICK on trackpoints.
While adding support for querying properties, also add support for the
recently added INPUT_PROP_ACCELEROMETER property bit.
---
src/udev/udev-builtin-input_id.c | 21 +++++++++++++++++++--
1 file changed, 19 insertions(+), 2 deletions(-)
diff --git a/src/udev/udev-builtin-input_id.c b/src/udev/udev-builtin-input_id.c
index 46f1c53..5b1790c 100644
--- a/src/udev/udev-builtin-input_id.c
+++ b/src/udev/udev-builtin-input_id.c
@@ -33,6 +33,14 @@
#include "udev.h"
#include "util.h"
+#ifndef INPUT_PROP_POINTING_STICK
+#define INPUT_PROP_POINTING_STICK 0x05
+#endif
+
+#ifndef INPUT_PROP_ACCELEROMETER
+#define INPUT_PROP_ACCELEROMETER 0x06
+#endif
Are these definitions normally defined in input.h?
Yes.
Post by Lennart Poettering
If so, please add them to missing.h, where we try to centralize definitions of this kind.
Ok, will fix for v2.
Post by Lennart Poettering
Post by Hans de Goede
/* we must use this kernel-compatible implementation */
#define BITS_PER_LONG (sizeof(unsigned long) * 8)
#define NBITS(x) ((((x)-1)/BITS_PER_LONG)+1)
@@ -131,6 +139,7 @@ static void test_pointers (struct udev_device *dev,
const unsigned long* bitmask_abs,
const unsigned long* bitmask_key,
const unsigned long* bitmask_rel,
+ unsigned long prop,
"unsigned long"? Is this really necessary? Shouldn't we just use
uint64_t? here?
unsigned long matches what is used in the kernel, it is a bit field,
I do not know what type is preferred for bit fields in systemd / udev.
Post by Lennart Poettering
Post by Hans de Goede
bool test) {
int is_mouse = 0;
int is_touchpad = 0;
@@ -182,6 +191,10 @@ static void test_pointers (struct udev_device *dev,
udev_builtin_add_property(dev, test, "ID_INPUT_MOUSE", "1");
if (is_touchpad)
udev_builtin_add_property(dev, test, "ID_INPUT_TOUCHPAD", "1");
+ if (prop & (1 << INPUT_PROP_POINTING_STICK))
+ udev_builtin_add_property(dev, test, "ID_INPUT_TRACKPOINT", "1");
+ if (prop & (1 << INPUT_PROP_ACCELEROMETER))
+ udev_builtin_add_property(dev, test, "ID_INPUT_ACCELEROMETER", "1");
}
/* key like devices */
@@ -232,7 +245,8 @@ static int builtin_input_id(struct udev_device *dev, int argc, char *argv[], boo
unsigned long bitmask_abs[NBITS(ABS_MAX)];
unsigned long bitmask_key[NBITS(KEY_MAX)];
unsigned long bitmask_rel[NBITS(REL_MAX)];
- const char *sysname, *devnode;
+ unsigned long prop = 0;
+ const char *sysname, *devnode, *prop_str;
/* walk up the parental chain until we find the real input device; the
* argument is very likely a subdevice of this, like eventN */
@@ -248,7 +262,10 @@ static int builtin_input_id(struct udev_device *dev, int argc, char *argv[], boo
get_cap_mask(dev, pdev, "capabilities/abs", bitmask_abs, sizeof(bitmask_abs), test);
get_cap_mask(dev, pdev, "capabilities/rel", bitmask_rel, sizeof(bitmask_rel), test);
get_cap_mask(dev, pdev, "capabilities/key", bitmask_key, sizeof(bitmask_key), test);
- test_pointers(dev, bitmask_ev, bitmask_abs, bitmask_key, bitmask_rel, test);
+ prop_str = udev_device_get_property_value(pdev, "PROP");
+ if (prop_str)
+ prop = strtoul(prop_str, NULL, 16);
Hmm, we try to avoid direct invocations of strtoul() and friends, due
to the annoying error handling... Can't we use safe_atou64() here, or so?
If that can handle hex without a 0x prefix then yes.

Regards,

Hans
Lennart Poettering
2015-04-02 10:57:46 UTC
Permalink
Post by Hans de Goede
Post by Lennart Poettering
Post by Hans de Goede
/* we must use this kernel-compatible implementation */
#define BITS_PER_LONG (sizeof(unsigned long) * 8)
#define NBITS(x) ((((x)-1)/BITS_PER_LONG)+1)
@@ -131,6 +139,7 @@ static void test_pointers (struct udev_device *dev,
const unsigned long* bitmask_abs,
const unsigned long* bitmask_key,
const unsigned long* bitmask_rel,
+ unsigned long prop,
"unsigned long"? Is this really necessary? Shouldn't we just use
uint64_t? here?
unsigned long matches what is used in the kernel, it is a bit field,
I do not know what type is preferred for bit fields in systemd / udev.
Oh god, the kernel is stupid. Using variable size types for
kernel/userspace APIs is just stupid...

Anyway, if that's how it is, use unsigned long, and also use strtoul
then, as before...

Lennart
--
Lennart Poettering, Red Hat
Hans de Goede
2015-04-02 09:48:49 UTC
Permalink
The IBM / Lenovo trackpoints are special, they allow specifying a sensitivity
setting through a ps/2 command, which changes the range of the deltas send when
using the trackpoint. One some models with normal usage one only deltas
of 1 or 2 are send, resulting in there only being 2 mouse cursor movement
speeds, rather then the expected fluid scale.

Changing the sensitivity to a higher level then the bootup default fixes this,
the rule additions in this commit allows specifying a new sensitivity default
through hwdb giving a better ootb experience.

Signed-off-by: Hans de Goede <***@redhat.com>
---
hwdb/70-trackpoint.hwdb | 11 +++++++++++
rules/70-trackpoint.rules | 6 ++++++
2 files changed, 17 insertions(+)

diff --git a/hwdb/70-trackpoint.hwdb b/hwdb/70-trackpoint.hwdb
index f2f8364..3dd5100 100644
--- a/hwdb/70-trackpoint.hwdb
+++ b/hwdb/70-trackpoint.hwdb
@@ -54,6 +54,17 @@
# by how much to multiply deltas generated by the trackpoint to get
# normalized deltas.
#
+#########################################
+# TRACKPOINT_SENSITIVTY #
+#########################################
+#
+# TPPS/2 IBM TrackPoint driver sensitivity sysfs setting
+# TRACKPOINT_SENSITIVITY=<sensitivity>
+#
+# Where <sensitivity> is a number between 0 and 255, note this property
+# only applies to TPPS/2 IBM TrackPoint devices and only works with DMI
+# matches.
+#

#
# Sort by by brand, model
diff --git a/rules/70-trackpoint.rules b/rules/70-trackpoint.rules
index 8ecbde6..1ed234d 100644
--- a/rules/70-trackpoint.rules
+++ b/rules/70-trackpoint.rules
@@ -8,6 +8,12 @@ ENV{ID_INPUT_TRACKPOINT}=="", GOTO="trackpoint_end"
IMPORT{builtin}="hwdb --subsystem=input --lookup-prefix=trackpoint:", \
GOTO="trackpoint_end"

+# Same as below but also sets the TPPS/2 IBM TrackPoint driver sensitivity
+KERNELS=="input*", ATTR{device/name}=="TPPS/2 IBM TrackPoint", \
+ IMPORT{builtin}="hwdb 'trackpoint:name:$attr{device/name}:$attr{[dmi/id]modalias}'", \
+ RUN="/bin/sh -c 'echo -n $env{TRACKPOINT_SENSITIVTY} > /sys$env{DEVPATH}/../../../sensitivity'", \
+ GOTO="trackpoint_end"
+
# device matching the input device name and the machine's DMI data
KERNELS=="input*", \
IMPORT{builtin}="hwdb 'trackpoint:name:$attr{device/name}:$attr{[dmi/id]modalias}'", \
--
2.3.4
Lennart Poettering
2015-04-02 10:23:16 UTC
Permalink
Post by Hans de Goede
+# Same as below but also sets the TPPS/2 IBM TrackPoint driver sensitivity
+KERNELS=="input*", ATTR{device/name}=="TPPS/2 IBM TrackPoint", \
+ IMPORT{builtin}="hwdb 'trackpoint:name:$attr{device/name}:$attr{[dmi/id]modalias}'", \
+ RUN="/bin/sh -c 'echo -n $env{TRACKPOINT_SENSITIVTY} > /sys$env{DEVPATH}/../../../sensitivity'", \
+ GOTO="trackpoint_end"
Invoking a shell for writing an attribute is certainly not OK. In
particular since ATTR{} can be used to write attributes.

Also, did you test this? THere's a pretty obvious typo in "TRACKPOINT_SENSITIVTY"...

Lennart
--
Lennart Poettering, Red Hat
Hans de Goede
2015-04-02 10:42:27 UTC
Permalink
Hi,
Post by Lennart Poettering
Post by Hans de Goede
+# Same as below but also sets the TPPS/2 IBM TrackPoint driver sensitivity
+KERNELS=="input*", ATTR{device/name}=="TPPS/2 IBM TrackPoint", \
+ IMPORT{builtin}="hwdb 'trackpoint:name:$attr{device/name}:$attr{[dmi/id]modalias}'", \
+ RUN="/bin/sh -c 'echo -n $env{TRACKPOINT_SENSITIVTY} > /sys$env{DEVPATH}/../../../sensitivity'", \
+ GOTO="trackpoint_end"
Invoking a shell for writing an attribute is certainly not OK. In
particular since ATTR{} can be used to write attributes.
Ok, are there any examples / docs for this ?
Post by Lennart Poettering
Also, did you test this? THere's a pretty obvious typo in "TRACKPOINT_SENSITIVTY"...
Yes I tested this, I've copy and pasted the typo to various places so it all matches
up and thus works ... I'll fix both for v2 of this set.

Regards,

Hans
Lennart Poettering
2015-04-02 11:01:25 UTC
Permalink
Post by Hans de Goede
Hi,
Post by Lennart Poettering
Post by Hans de Goede
+# Same as below but also sets the TPPS/2 IBM TrackPoint driver sensitivity
+KERNELS=="input*", ATTR{device/name}=="TPPS/2 IBM TrackPoint", \
+ IMPORT{builtin}="hwdb 'trackpoint:name:$attr{device/name}:$attr{[dmi/id]modalias}'", \
+ RUN="/bin/sh -c 'echo -n $env{TRACKPOINT_SENSITIVTY} > /sys$env{DEVPATH}/../../../sensitivity'", \
+ GOTO="trackpoint_end"
Invoking a shell for writing an attribute is certainly not OK. In
particular since ATTR{} can be used to write attributes.
Ok, are there any examples / docs for this ?
See rules/ in the source tree. Grep for "ATTR{" and look for the uses
with "=" (in contrast to "=="). THose are attribute assignments. (And
the ones with == are attribute comparisons...)

It's tersely documented in udev's man page.

Lennart
--
Lennart Poettering, Red Hat
Hans de Goede
2015-04-02 09:48:48 UTC
Permalink
There is quite a wide spread in the delta events generated by trackpoints,
some generate deltas of 1-2 under normal use, while others generate deltas
from 1-20.

This commit adds a set of rules + a hwdb file which allows specifying a
per model TRACKPOINT_CONST_ACCEL value which can be used by the userspace
input stack to normalize the deltas so that all trackpoints get the same
speed/feeling ootb.

The hwdb matching is modelled after 60-keyboard.rules, rather then after
70-mouse.rules, as trackpoints are typically (but not always) found integrated
into laptops and the keyboard matching rules are a better match to this.

Signed-off-by: Hans de Goede <***@redhat.com>
---
hwdb/70-trackpoint.hwdb | 59 +++++++++++++++++++++++++++++++++++++++++++++++
rules/70-mouse.rules | 1 +
rules/70-trackpoint.rules | 16 +++++++++++++
3 files changed, 76 insertions(+)
create mode 100644 hwdb/70-trackpoint.hwdb
create mode 100644 rules/70-trackpoint.rules

diff --git a/hwdb/70-trackpoint.hwdb b/hwdb/70-trackpoint.hwdb
new file mode 100644
index 0000000..f2f8364
--- /dev/null
+++ b/hwdb/70-trackpoint.hwdb
@@ -0,0 +1,59 @@
+# This file is part of systemd.
+#
+# Trackpoint const-accel configuration, to make different brand / model
+# laptop trackpoints have the same speed / feel, and per model adjustment
+# of the IBM TrackPoint driver's sensitivty setting
+#
+# The lookup keys are composed in:
+# 70-trackpoint.rules
+#
+# Note: The format of the "trackpoint:" prefix match key is a
+# contract between the rules file and the hardware data, it might
+# change in later revisions to support more or better matches, it
+# is not necessarily expected to be a stable ABI.
+#
+# Supported hardware matches are:
+# - Generic input devices match:
+# trackpoint:input:bZZZZvYYYYpXXXXeWWWW-VVVV
+# This matches on the kernel modalias of the input-device, mainly:
+# ZZZZ is the bus-id (see /usr/include/linux/input.h BUS_*), YYYY, XXXX and
+# WWW are the 4-digit hex uppercase vendor, product and version ID and VVVV
+# is an arbitrary length input-modalias describing the device capabilities.
+#
+# - Input driver device name and DMI data match:
+# trackpoint:name:<input device name>:dmi:bvn*:bvr*:bd*:svn<vendor>:pn*
+# <input device name> is the name device specified by the driver,
+# <vendor> is the firmware-provided string from the kernel DMI modalias.
+#
+# To add local entries, create a new file
+# /etc/udev/hwdb.d/71-trackpoint-local.hwdb
+# and add your rules there. To load the new rules execute (as root):
+# udevadm hwdb --update
+# udevadm trigger /dev/input/eventXX
+# where /dev/input/eventXX is the trackpoint in question. If in
+# doubt, simply use /dev/input/event* to reload all input rules.
+#
+# If your changes are generally applicable, open a bug report on
+# http://bugs.freedesktop.org/enter_bug.cgi?product=systemd
+# and include your new rules, a description of the device, and the
+# output of
+# udevadm info /dev/input/eventXX
+# (or /dev/input/event*).
+#
+# Allowed properties are:
+# TRACKPOINT_CONST_ACCEL
+#
+#########################################
+# TRACKPOINT_CONST_ACCEL #
+#########################################
+#
+# Trackpoint const accel settings are specified as
+# TRACKPOINT_CONST_ACCEL=<accel>
+#
+# Where <accel> is a floating point number, using a '.' seperator, specifying
+# by how much to multiply deltas generated by the trackpoint to get
+# normalized deltas.
+#
+
+#
+# Sort by by brand, model
diff --git a/rules/70-mouse.rules b/rules/70-mouse.rules
index 3ea743a..c47a1b1 100644
--- a/rules/70-mouse.rules
+++ b/rules/70-mouse.rules
@@ -3,6 +3,7 @@
ACTION=="remove", GOTO="mouse_end"
KERNEL!="event*", GOTO="mouse_end"
ENV{ID_INPUT_MOUSE}=="", GOTO="mouse_end"
+ENV{ID_INPUT_TRACKPOINT}=="1", GOTO="mouse_end"

# mouse:<subsystem>:v<vid>p<pid>:name:<name>:*
KERNELS=="input*", ENV{ID_BUS}=="usb", \
diff --git a/rules/70-trackpoint.rules b/rules/70-trackpoint.rules
new file mode 100644
index 0000000..8ecbde6
--- /dev/null
+++ b/rules/70-trackpoint.rules
@@ -0,0 +1,16 @@
+# do not edit this file, it will be overwritten on update
+
+ACTION=="remove", GOTO="trackpoint_end"
+KERNEL!="event*", GOTO="trackpoint_end"
+ENV{ID_INPUT_TRACKPOINT}=="", GOTO="trackpoint_end"
+
+# skip later rules when we find something for this input device
+IMPORT{builtin}="hwdb --subsystem=input --lookup-prefix=trackpoint:", \
+ GOTO="trackpoint_end"
+
+# device matching the input device name and the machine's DMI data
+KERNELS=="input*", \
+ IMPORT{builtin}="hwdb 'trackpoint:name:$attr{device/name}:$attr{[dmi/id]modalias}'", \
+ GOTO="trackpoint_end"
+
+LABEL="trackpoint_end"
--
2.3.4
Hans de Goede
2015-04-02 09:48:50 UTC
Permalink
Lenovo has changed the sensitity of the trackpoint on the x240 / T440s / T540
generation of Thinkpads, making them somewhat unsensitive by default, add a
hwdb entry to tweak the sensitivity setting.

The ThinkPad X200s is way way too slow by default and unless you push the
trackpoint quite hard only sends delta events in the 1-2 range, tweak the
sensitivity to make it send a wider range of deltas and apply a const accel
factor to make it have a more reasonable speed by default.

Signed-off-by: Hans de Goede <***@redhat.com>
---
hwdb/70-trackpoint.hwdb | 14 ++++++++++++++
1 file changed, 14 insertions(+)

diff --git a/hwdb/70-trackpoint.hwdb b/hwdb/70-trackpoint.hwdb
index 3dd5100..0017d8b 100644
--- a/hwdb/70-trackpoint.hwdb
+++ b/hwdb/70-trackpoint.hwdb
@@ -68,3 +68,17 @@

#
# Sort by by brand, model
+
+#########################################
+# Lenovo
+#########################################
+
+# Lenovo Thinkpad X200s
+trackpoint:name:TPPS/2 IBM TrackPoint:dmi:bvn*:bvr*:bd*:svnLENOVO:pn*:pvrThinkPadX200s:*
+ TRACKPOINT_SENSITIVTY=200
+ TRACKPOINT_CONST_ACCEL=1.5
+
+# Lenovo Thinkpad T440s
+trackpoint:name:TPPS/2 IBM TrackPoint:dmi:bvn*:bvr*:bd*:svnLENOVO:pn*:pvrThinkPadT440s:*
+ TRACKPOINT_SENSITIVTY=200
+ TRACKPOINT_CONST_ACCEL=1.0
--
2.3.4
Hans de Goede
2015-04-02 09:48:51 UTC
Permalink
The trackpoint of the Dell Latitude E6400 is somewhat slow by default,
where as the trackpoint of the Dell Latitude D620 is much too fast by default,
set TRACKPOINT_CONST_ACCEL for both of them to adjust for this.

Signed-off-by: Hans de Goede <***@redhat.com>
---
hwdb/70-trackpoint.hwdb | 12 ++++++++++++
1 file changed, 12 insertions(+)

diff --git a/hwdb/70-trackpoint.hwdb b/hwdb/70-trackpoint.hwdb
index 0017d8b..30c22a3 100644
--- a/hwdb/70-trackpoint.hwdb
+++ b/hwdb/70-trackpoint.hwdb
@@ -70,6 +70,18 @@
# Sort by by brand, model

#########################################
+# Dell
+#########################################
+
+# Latitude D620
+trackpoint:name:*DualPoint Stick:dmi:bvn*:bvr*:bd*:svnDellInc.:pnLatitudeD620*:pvr*
+ TRACKPOINT_CONST_ACCEL=0.5
+
+# Latitude E6400
+trackpoint:name:*DualPoint Stick:dmi:bvn*:bvr*:bd*:svnDellInc.:pnLatitudeE6400*:pvr*
+ TRACKPOINT_CONST_ACCEL=1.5
+
+#########################################
# Lenovo
#########################################
--
2.3.4
Loading...