[PATCH v2] Support for Alps SS5 touchpad

[PATCH v2] Support for Alps SS5 touchpad

[Ben Gamari]

Here is a patchset adding support for the Alps SS5 touchpad hardware shipped
with the Dell Latitude E7470. The protocol is similar to that used by the SS4
v2 devices, but with additional support for a touchstick.

The touchpad exhibits slightly inconsistent behavior when single-finger
contacts are released while a button is being held. This leads to extremely
unpleasant jumps in pointer position, especially during drag-and-drop
operations. This is resolved by patch 2/4.

One minor outstanding issue is the high speed of the touchstick. Previous
touchstick drivers have taken the extremely unfortunate approach of scaling the
input device space to work-around this (often sacrificing device resolution in
the process). I've started another thread on linux-input (see "Should
touchsticks really be relative input devices?") to discuss options for
resolving this.

Cheers,

- Ben


Changes:

v1:
 * Initial revision

v2:
 * Rework device detection
 * Fix contact release behavior of 1F events
 * Expose pressure of touchstick

[PATCH 1/4] input/alps: Add touchstick support for SS5 hardware

[Ben Gamari]

Add touchstick support for the so-called SS5 hardware, which uses a
variant of the SS4 protocol.
---
 drivers/input/mouse/alps.c | 64 ++++++++++++++++++++++++++++++++++++++--------
 drivers/input/mouse/alps.h |  2 ++
 2 files changed, 55 insertions(+), 11 deletions(-)

diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c
index 936f07a..b8454af 100644
--- a/drivers/input/mouse/alps.c
+++ b/drivers/input/mouse/alps.c
@@ -1156,15 +1156,27 @@ static unsigned char alps_get_pkt_id_ss4_v2(unsigned char *byte)
 {
 	unsigned char pkt_id = SS4_PACKET_ID_IDLE;
 
-	if (byte[0] == 0x18 && byte[1] == 0x10 && byte[2] == 0x00 &&
-	    (byte[3] & 0x88) == 0x08 && byte[4] == 0x10 && byte[5] == 0x00) {
-		pkt_id = SS4_PACKET_ID_IDLE;
-	} else if (!(byte[3] & 0x10)) {
-		pkt_id = SS4_PACKET_ID_ONE;
-	} else if (!(byte[3] & 0x20)) {
+	switch (byte[3] & 0x30) {
+	case 0x00:
+		if (byte[0] == 0x18 && byte[1] == 0x10 && byte[2] == 0x00 &&
+				(byte[3] & 0x88) == 0x08 && byte[4] == 0x10 && byte[5] == 0x00) {
+			pkt_id = SS4_PACKET_ID_IDLE;
+		} else {
+			pkt_id = SS4_PACKET_ID_ONE;
+		}
+		break;
+	case 0x10:
+		/* two-finger finger positions */
 		pkt_id = SS4_PACKET_ID_TWO;
-	} else {
+		break;
+	case 0x20:
+		/* stick pointer */
+		pkt_id = SS4_PACKET_ID_STICK;
+		break;
+	case 0x30:
+		/* third and fourth finger positions */
 		pkt_id = SS4_PACKET_ID_MULTI;
+		break;
 	}
 
 	return pkt_id;
@@ -1246,16 +1258,38 @@ static int alps_decode_ss4_v2(struct alps_fields *f,
 		}
 		break;
 
+	case SS4_PACKET_ID_STICK:
+		if (!(priv->flags & ALPS_DUALPOINT)) {
+			psmouse_warn(psmouse,
+				     "Rejected trackstick packet from non DualPoint device");
+		} else {
+			int x = (s8)(((p[0] & 1) << 7) | (p[1] & 0x7f));
+			int y = (s8)(((p[3] & 1) << 7) | (p[2] & 0x7f));
+
+			input_report_rel(priv->dev2, REL_X, x);
+			input_report_rel(priv->dev2, REL_Y, -y);
+		}
+		break;
+
 	case SS4_PACKET_ID_IDLE:
 	default:
 		memset(f, 0, sizeof(struct alps_fields));
 		break;
 	}
 
-	f->left = !!(SS4_BTN_V2(p) & 0x01);
-	if (!(priv->flags & ALPS_BUTTONPAD)) {
-		f->right = !!(SS4_BTN_V2(p) & 0x02);
-		f->middle = !!(SS4_BTN_V2(p) & 0x04);
+	/* handle buttons */
+	if (pkt_id == SS4_PACKET_ID_STICK) {
+		f->ts_left = !!(SS4_BTN_V2(p) & 0x01);
+		if (!(priv->flags & ALPS_BUTTONPAD)) {
+			f->ts_right = !!(SS4_BTN_V2(p) & 0x02);
+			f->ts_middle = !!(SS4_BTN_V2(p) & 0x04);
+		}
+	} else {
+		f->left = !!(SS4_BTN_V2(p) & 0x01);
+		if (!(priv->flags & ALPS_BUTTONPAD)) {
+			f->right = !!(SS4_BTN_V2(p) & 0x02);
+			f->middle = !!(SS4_BTN_V2(p) & 0x04);
+		}
 	}
 
 	return 0;
@@ -1266,6 +1300,7 @@ static void alps_process_packet_ss4_v2(struct psmouse *psmouse)
 	struct alps_data *priv = psmouse->private;
 	unsigned char *packet = psmouse->packet;
 	struct input_dev *dev = psmouse->dev;
+	struct input_dev *dev2 = priv->dev2;
 	struct alps_fields *f = &priv->f;
 
 	memset(f, 0, sizeof(struct alps_fields));
@@ -1311,6 +1346,13 @@ static void alps_process_packet_ss4_v2(struct psmouse *psmouse)
 
 	input_report_abs(dev, ABS_PRESSURE, f->pressure);
 	input_sync(dev);
+
+	if (priv->flags & ALPS_DUALPOINT) {
+		input_report_key(dev2, BTN_LEFT, f->ts_left);
+		input_report_key(dev2, BTN_RIGHT, f->ts_right);
+		input_report_key(dev2, BTN_MIDDLE, f->ts_middle);
+		input_sync(dev2);
+	}
 }
 
 static bool alps_is_valid_package_ss4_v2(struct psmouse *psmouse)
diff --git a/drivers/input/mouse/alps.h b/drivers/input/mouse/alps.h
index d37f814..b9417e2 100644
--- a/drivers/input/mouse/alps.h
+++ b/drivers/input/mouse/alps.h
@@ -37,12 +37,14 @@
  *  or there's button activities.
  * SS4_PACKET_ID_TWO: There's two or more fingers on touchpad
  * SS4_PACKET_ID_MULTI: There's three or more fingers on touchpad
+ * SS4_PACKET_ID_STICK: A stick pointer packet
 */
 enum SS4_PACKET_ID {
 	SS4_PACKET_ID_IDLE = 0,
 	SS4_PACKET_ID_ONE,
 	SS4_PACKET_ID_TWO,
 	SS4_PACKET_ID_MULTI,
+	SS4_PACKET_ID_STICK,
 };
 
 #define SS4_COUNT_PER_ELECTRODE		256
-- 
2.8.1

[PATCH 2/4] input/alps: Handle 0-pressure 1F events

[Ben Gamari]

While a button is held SS5 hardware will give us single-finger packets
with x, y, and pressure equal to zero. This causes annoying jumps in
pointer position if a touch is released while the button is held. Handle
this by claiming zero contacts to ensure that no position events are
provided to the user.
---
 drivers/input/mouse/alps.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c
index b8454af..7874f4f 100644
--- a/drivers/input/mouse/alps.c
+++ b/drivers/input/mouse/alps.c
@@ -1197,7 +1197,12 @@ static int alps_decode_ss4_v2(struct alps_fields *f,
 		f->mt[0].x = SS4_1F_X_V2(p);
 		f->mt[0].y = SS4_1F_Y_V2(p);
 		f->pressure = ((SS4_1F_Z_V2(p)) * 2) & 0x7f;
-		f->fingers = 1;
+		/*
+		 * When a button is held the device will give us events with x, y, and
+		 * pressure of 0. This causes annoying jumps if a touch is released while
+		 * the button is held. Handle this by claiming zero contacts.
+		 */
+		f->fingers = f->pressure > 0 ? 1 : 0;
 		f->first_mp = 0;
 		f->is_mp = 0;
 		break;
-- 
2.8.1

[PATCH 3/4] input/alps: Allow touchsticks to report pressure

[Ben Gamari]

The SS5 hardware can report this.
---
 drivers/input/mouse/alps.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c
index 7874f4f..25d2cad 100644
--- a/drivers/input/mouse/alps.c
+++ b/drivers/input/mouse/alps.c
@@ -103,6 +103,7 @@ static const struct alps_nibble_commands alps_v6_nibble_commands[] = {
 					   6-byte ALPS packet */
 #define ALPS_STICK_BITS		0x100	/* separate stick button bits */
 #define ALPS_BUTTONPAD		0x200	/* device is a clickpad */
+#define ALPS_DUALPOINT_WITH_PRESSURE		0x400	/* device can report trackpoint pressure */
 
 static const struct alps_model_info alps_model_data[] = {
 	{ { 0x32, 0x02, 0x14 }, 0x00, { ALPS_PROTO_V2, 0xf8, 0xf8, ALPS_PASS | ALPS_DUALPOINT } },	/* Toshiba Salellite Pro M10 */
@@ -1270,9 +1271,11 @@ static int alps_decode_ss4_v2(struct alps_fields *f,
 		} else {
 			int x = (s8)(((p[0] & 1) << 7) | (p[1] & 0x7f));
 			int y = (s8)(((p[3] & 1) << 7) | (p[2] & 0x7f));
+			int pressure = (s8)(p[4] & 0x7f);
 
 			input_report_rel(priv->dev2, REL_X, x);
 			input_report_rel(priv->dev2, REL_Y, -y);
+			input_report_abs(priv->dev2, ABS_PRESSURE, pressure);
 		}
 		break;
 
@@ -2996,6 +2999,10 @@ int alps_init(struct psmouse *psmouse)
 
 		input_set_capability(dev2, EV_REL, REL_X);
 		input_set_capability(dev2, EV_REL, REL_Y);
+		if (priv->flags & ALPS_DUALPOINT_WITH_PRESSURE) {
+			input_set_capability(dev2, EV_ABS, ABS_PRESSURE);
+			input_set_abs_params(dev2, ABS_PRESSURE, 0, 127, 0, 0);
+		}
 		input_set_capability(dev2, EV_KEY, BTN_LEFT);
 		input_set_capability(dev2, EV_KEY, BTN_RIGHT);
 		input_set_capability(dev2, EV_KEY, BTN_MIDDLE);
-- 
2.8.1

[PATCH 4/4] input/alps: Set DualPoint flag for 74 03 28 devices

[Ben Gamari]

Here we introduce logic in alps_identify to set the ALPS_DUALPOINT flag
for touchpad hardware responding to E7 report with 73 03 28, as is found
in the Dell Latitude E7470.
---
 drivers/input/mouse/alps.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c
index 25d2cad..431bc26 100644
--- a/drivers/input/mouse/alps.c
+++ b/drivers/input/mouse/alps.c
@@ -2776,6 +2776,7 @@ static int alps_identify(struct psmouse *psmouse, struct alps_data *priv)
 	const struct alps_protocol_info *protocol;
 	unsigned char e6[4], e7[4], ec[4];
 	int error;
+	int flags = 0;
 
 	/*
 	 * First try "E6 report".
@@ -2817,6 +2818,10 @@ static int alps_identify(struct psmouse *psmouse, struct alps_data *priv)
 		} else if (e7[0] == 0x73 && e7[1] == 0x03 &&
 			   e7[2] == 0x14 && ec[1] == 0x02) {
 			protocol = &alps_v8_protocol_data;
+		} else if (e7[0] == 0x73 && e7[1] == 0x03 &&
+			   e7[2] == 0x28 && ec[1] == 0x01) {
+			protocol = &alps_v8_protocol_data;
+			flags |= ALPS_DUALPOINT | ALPS_DUALPOINT_WITH_PRESSURE;
 		} else {
 			psmouse_dbg(psmouse,
 				    "Likely not an ALPS touchpad: E7=%3ph, EC=%3ph\n", e7, ec);
@@ -2830,6 +2835,7 @@ static int alps_identify(struct psmouse *psmouse, struct alps_data *priv)
 		error = alps_set_protocol(psmouse, priv, protocol);
 		if (error)
 			return error;
+		priv->flags |= flags;
 	}
 
 	return 0;
-- 
2.8.1

Re: [PATCH v2] Support for Alps SS5 touchpad

[Hans de Goede]

Hi,

On 28-05-16 14:00, Ben Gamari wrote:
> Here is a patchset adding support for the Alps SS5 touchpad hardware shipped
> with the Dell Latitude E7470. The protocol is similar to that used by the SS4
> v2 devices, but with additional support for a touchstick.
>
> The touchpad exhibits slightly inconsistent behavior when single-finger
> contacts are released while a button is being held. This leads to extremely
> unpleasant jumps in pointer position, especially during drag-and-drop
> operations. This is resolved by patch 2/4.
>
> One minor outstanding issue is the high speed of the touchstick. Previous
> touchstick drivers have taken the extremely unfortunate approach of scaling the
> input device space to work-around this (often sacrificing device resolution in
> the process). I've started another thread on linux-input (see "Should
> touchsticks really be relative input devices?") to discuss options for
> resolving this.

The problem with touchstick's is that they have a wildly varying sensitivity,
unfortunately this seems to be laptop model specific, e.g. one generation
of alps tracksticks can be slow on some models and fast on others.

We've entries in udev's hwdb for known troublesome models, see:
/lib/udev/hwdb.d/70-pointingstick.hwdb

On a modern Linux distro. If you add an entry for your laptop there, with a
slow-down factor and are using xf86-input-libinput as driver for the
touchstick, then things should work.

Note after updating the file you must run "sudo udevadm hwdb --update" and
then reboot (or trigger the relevant device).

Regards,

Hans

Re: [PATCH 3/4] input/alps: Allow touchsticks to report pressure

[Hans de Goede]

Hi,

On 28-05-16 14:01, Ben Gamari wrote:
> The SS5 hardware can report this.
> ---
>  drivers/input/mouse/alps.c | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c
> index 7874f4f..25d2cad 100644
> --- a/drivers/input/mouse/alps.c
> +++ b/drivers/input/mouse/alps.c
> @@ -103,6 +103,7 @@ static const struct alps_nibble_commands alps_v6_nibble_commands[] = {
>  					   6-byte ALPS packet */
>  #define ALPS_STICK_BITS		0x100	/* separate stick button bits */
>  #define ALPS_BUTTONPAD		0x200	/* device is a clickpad */
> +#define ALPS_DUALPOINT_WITH_PRESSURE		0x400	/* device can report trackpoint pressure */
>
>  static const struct alps_model_info alps_model_data[] = {
>  	{ { 0x32, 0x02, 0x14 }, 0x00, { ALPS_PROTO_V2, 0xf8, 0xf8, ALPS_PASS | ALPS_DUALPOINT } },	/* Toshiba Salellite Pro M10 */
> @@ -1270,9 +1271,11 @@ static int alps_decode_ss4_v2(struct alps_fields *f,
>  		} else {
>  			int x = (s8)(((p[0] & 1) << 7) | (p[1] & 0x7f));
>  			int y = (s8)(((p[3] & 1) << 7) | (p[2] & 0x7f));
> +			int pressure = (s8)(p[4] & 0x7f);
>
>  			input_report_rel(priv->dev2, REL_X, x);
>  			input_report_rel(priv->dev2, REL_Y, -y);
> +			input_report_abs(priv->dev2, ABS_PRESSURE, pressure);
>  		}
>  		break;
>
> @@ -2996,6 +2999,10 @@ int alps_init(struct psmouse *psmouse)
>
>  		input_set_capability(dev2, EV_REL, REL_X);
>  		input_set_capability(dev2, EV_REL, REL_Y);
> +		if (priv->flags & ALPS_DUALPOINT_WITH_PRESSURE) {
> +			input_set_capability(dev2, EV_ABS, ABS_PRESSURE);
> +			input_set_abs_params(dev2, ABS_PRESSURE, 0, 127, 0, 0);
> +		}
>  		input_set_capability(dev2, EV_KEY, BTN_LEFT);
>  		input_set_capability(dev2, EV_KEY, BTN_RIGHT);
>  		input_set_capability(dev2, EV_KEY, BTN_MIDDLE);

This seems wrong, reporting ABS_PRESSURE on a relative device. And yes, this
really is a relative device (it sends repeated delta events when you
keep pushing at the same force, rather then sending a single coordinate
value).

Maybe we need a REL_PRESSURE for cases like this ?

Regards,

Hans

Re: [PATCH v2] Support for Alps SS5 touchpad

[Ben Gamari]

[-- Attachment #1: Type: text/plain, Size: 3358 bytes --]

Hans de Goede <hdegoede@redhat.com> writes:

> Hi,
>
> On 28-05-16 14:00, Ben Gamari wrote:
>> Here is a patchset adding support for the Alps SS5 touchpad hardware shipped
>> with the Dell Latitude E7470. The protocol is similar to that used by the SS4
>> v2 devices, but with additional support for a touchstick.
>>
>> The touchpad exhibits slightly inconsistent behavior when single-finger
>> contacts are released while a button is being held. This leads to extremely
>> unpleasant jumps in pointer position, especially during drag-and-drop
>> operations. This is resolved by patch 2/4.
>>
>> One minor outstanding issue is the high speed of the touchstick. Previous
>> touchstick drivers have taken the extremely unfortunate approach of scaling the
>> input device space to work-around this (often sacrificing device resolution in
>> the process). I've started another thread on linux-input (see "Should
>> touchsticks really be relative input devices?") to discuss options for
>> resolving this.
>
> The problem with touchstick's is that they have a wildly varying sensitivity,
> unfortunately this seems to be laptop model specific, e.g. one generation
> of alps tracksticks can be slow on some models and fast on others.
>
Right.

> We've entries in udev's hwdb for known troublesome models, see:
> /lib/udev/hwdb.d/70-pointingstick.hwdb
>
Thanks, I'll submit a patch when I find a reasonable value.

> On a modern Linux distro. If you add an entry for your laptop there, with a
> slow-down factor and are using xf86-input-libinput as driver for the
> touchstick, then things should work.
>
Apologies for going slightly out of kernel-land but...

What exactly does this end up adjusting in libinput? I've been entirely
unable to get any control over the speed of the touchstick with xinput.
Currently the properties look like,

    $ xinput list-props 14
    Device 'AlpsPS/2 ALPS DualPoint Stick':
      Device Enabled (137):	1
      Coordinate Transformation Matrix (139):	0.010000, 0.000000, 0.000000, 0.000000, 0.010000, 0.000000, 0.000000, 0.000000, 0.010000
      libinput Accel Speed (631):	0.000010
      libinput Accel Speed Default (632):	0.000000
      libinput Accel Profiles Available (633):	1, 1
      libinput Accel Profile Enabled (634):	1, 0
      libinput Accel Profile Enabled Default (635):	1, 0
      libinput Natural Scrolling Enabled (636):	0
      libinput Natural Scrolling Enabled Default (637):	0
      libinput Send Events Modes Available (257):	1, 0
      libinput Send Events Mode Enabled (258):	0, 0
      libinput Send Events Mode Enabled Default (259):	0, 0
      libinput Left Handed Enabled (638):	0
      libinput Left Handed Enabled Default (639):	0
      libinput Scroll Methods Available (640):	0, 0, 1
      libinput Scroll Method Enabled (641):	0, 0, 1
      libinput Scroll Method Enabled Default (642):	0, 0, 1
      libinput Button Scrolling Button (643):	2
      libinput Button Scrolling Button Default (644):	274
      libinput Middle Emulation Enabled (645):	0
      libinput Middle Emulation Enabled Default (646):	0
      Device Node (260):	"/dev/input/event9"
      Device Product ID (261):	2, 8
      libinput Drag Lock Buttons (647):	<no items>
      libinput Horizonal Scroll Enabled (262):	1

Yet none of my fiddling seems to have had any effect on the sensitivity
of the device.

Cheers,

- Ben

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 472 bytes --]

Re: [PATCH v2] Support for Alps SS5 touchpad

[Hans de Goede]

Hi,

On 28-05-16 15:16, Ben Gamari wrote:
> Hans de Goede <hdegoede@redhat.com> writes:
>
>> Hi,
>>
>> On 28-05-16 14:00, Ben Gamari wrote:
>>> Here is a patchset adding support for the Alps SS5 touchpad hardware shipped
>>> with the Dell Latitude E7470. The protocol is similar to that used by the SS4
>>> v2 devices, but with additional support for a touchstick.
>>>
>>> The touchpad exhibits slightly inconsistent behavior when single-finger
>>> contacts are released while a button is being held. This leads to extremely
>>> unpleasant jumps in pointer position, especially during drag-and-drop
>>> operations. This is resolved by patch 2/4.
>>>
>>> One minor outstanding issue is the high speed of the touchstick. Previous
>>> touchstick drivers have taken the extremely unfortunate approach of scaling the
>>> input device space to work-around this (often sacrificing device resolution in
>>> the process). I've started another thread on linux-input (see "Should
>>> touchsticks really be relative input devices?") to discuss options for
>>> resolving this.
>>
>> The problem with touchstick's is that they have a wildly varying sensitivity,
>> unfortunately this seems to be laptop model specific, e.g. one generation
>> of alps tracksticks can be slow on some models and fast on others.
>>
> Right.
>
>> We've entries in udev's hwdb for known troublesome models, see:
>> /lib/udev/hwdb.d/70-pointingstick.hwdb
>>
> Thanks, I'll submit a patch when I find a reasonable value.
>
>> On a modern Linux distro. If you add an entry for your laptop there, with a
>> slow-down factor and are using xf86-input-libinput as driver for the
>> touchstick, then things should work.
>>
> Apologies for going slightly out of kernel-land but...
>
> What exactly does this end up adjusting in libinput?

It applies a constant factor to the delta-s before processing them
further, it does so using floating point, so as to not loose
precision as the kernel downscale solution does.

Note to slow down you will want to use a value of 0.x, e.g.
0.5 or even 0.25 .

> I've been entirely
> unable to get any control over the speed of the touchstick with xinput.
> Currently the properties look like,
>
>     $ xinput list-props 14
>     Device 'AlpsPS/2 ALPS DualPoint Stick':
>       Device Enabled (137):	1
>       Coordinate Transformation Matrix (139):	0.010000, 0.000000, 0.000000, 0.000000, 0.010000, 0.000000, 0.000000, 0.000000, 0.010000
>       libinput Accel Speed (631):	0.000010
>       libinput Accel Speed Default (632):	0.000000
>       libinput Accel Profiles Available (633):	1, 1
>       libinput Accel Profile Enabled (634):	1, 0
>       libinput Accel Profile Enabled Default (635):	1, 0
>       libinput Natural Scrolling Enabled (636):	0
>       libinput Natural Scrolling Enabled Default (637):	0
>       libinput Send Events Modes Available (257):	1, 0
>       libinput Send Events Mode Enabled (258):	0, 0
>       libinput Send Events Mode Enabled Default (259):	0, 0
>       libinput Left Handed Enabled (638):	0
>       libinput Left Handed Enabled Default (639):	0
>       libinput Scroll Methods Available (640):	0, 0, 1
>       libinput Scroll Method Enabled (641):	0, 0, 1
>       libinput Scroll Method Enabled Default (642):	0, 0, 1
>       libinput Button Scrolling Button (643):	2
>       libinput Button Scrolling Button Default (644):	274
>       libinput Middle Emulation Enabled (645):	0
>       libinput Middle Emulation Enabled Default (646):	0
>       Device Node (260):	"/dev/input/event9"
>       Device Product ID (261):	2, 8
>       libinput Drag Lock Buttons (647):	<no items>
>       libinput Horizonal Scroll Enabled (262):	1
>
> Yet none of my fiddling seems to have had any effect on the sensitivity
> of the device.

The range of "libinput Accel Speed" is from -1.0 to 1.0 to de-accel
the touchstick set it to a negative value.

But you really want to use the hwdb solution, so that 0.0 more
or less works nicely, so that we do not end up with all users
needing to tweak things manually.

Regards,

Hans

Re: [PATCH 4/4] input/alps: Set DualPoint flag for 74 03 28 devices

[Pali Rohár]

On Saturday 28 May 2016 14:01:02 Ben Gamari wrote:
> Here we introduce logic in alps_identify to set the ALPS_DUALPOINT flag
> for touchpad hardware responding to E7 report with 73 03 28, as is found
> in the Dell Latitude E7470.
> ---
>  drivers/input/mouse/alps.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c
> index 25d2cad..431bc26 100644
> --- a/drivers/input/mouse/alps.c
> +++ b/drivers/input/mouse/alps.c
> @@ -2776,6 +2776,7 @@ static int alps_identify(struct psmouse *psmouse, struct alps_data *priv)
>  	const struct alps_protocol_info *protocol;
>  	unsigned char e6[4], e7[4], ec[4];
>  	int error;
> +	int flags = 0;
>  
>  	/*
>  	 * First try "E6 report".
> @@ -2817,6 +2818,10 @@ static int alps_identify(struct psmouse *psmouse, struct alps_data *priv)
>  		} else if (e7[0] == 0x73 && e7[1] == 0x03 &&
>  			   e7[2] == 0x14 && ec[1] == 0x02) {
>  			protocol = &alps_v8_protocol_data;
> +		} else if (e7[0] == 0x73 && e7[1] == 0x03 &&
> +			   e7[2] == 0x28 && ec[1] == 0x01) {
> +			protocol = &alps_v8_protocol_data;
> +			flags |= ALPS_DUALPOINT | ALPS_DUALPOINT_WITH_PRESSURE;
>  		} else {
>  			psmouse_dbg(psmouse,
>  				    "Likely not an ALPS touchpad: E7=%3ph, EC=%3ph\n", e7, ec);
> @@ -2830,6 +2835,7 @@ static int alps_identify(struct psmouse *psmouse, struct alps_data *priv)
>  		error = alps_set_protocol(psmouse, priv, protocol);
>  		if (error)
>  			return error;
> +		priv->flags |= flags;
>  	}
>  
>  	return 0;

I would rather see setting flags in alps_set_protocol function. Where it
is done for other protocols. But do not know if it is easily possible
without rewriting lot of code...

-- 
Pali Rohár
pali.rohar@gmail.com
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] Support for Alps SS5 touchpad

[Pali Rohár]

Now when you are extending alps.c code for new touchpad, can you also
update documentation file Documentation/input/alps.txt to reflect V8
protcol? That would be great to have up-to-date documentation for
debugging and future purpose...

-- 
Pali Rohár
pali.rohar@gmail.com
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] Support for Alps SS5 touchpad

[Ben Gamari]

[-- Attachment #1: Type: text/plain, Size: 357 bytes --]

Pali Rohár <pali.rohar@gmail.com> writes:

> Now when you are extending alps.c code for new touchpad, can you also
> update documentation file Documentation/input/alps.txt to reflect V8
> protcol? That would be great to have up-to-date documentation for
> debugging and future purpose...
>
Indeed, it's been on my to-do list.

Cheers,

- Ben


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 472 bytes --]

Re: [PATCH 3/4] input/alps: Allow touchsticks to report pressure

[Ben Gamari]

[-- Attachment #1: Type: text/plain, Size: 1240 bytes --]

Hans de Goede <hdegoede@redhat.com> writes:

> Hi,
>>
>> @@ -2996,6 +2999,10 @@ int alps_init(struct psmouse *psmouse)
>>
>>  		input_set_capability(dev2, EV_REL, REL_X);
>>  		input_set_capability(dev2, EV_REL, REL_Y);
>> +		if (priv->flags & ALPS_DUALPOINT_WITH_PRESSURE) {
>> +			input_set_capability(dev2, EV_ABS, ABS_PRESSURE);
>> +			input_set_abs_params(dev2, ABS_PRESSURE, 0, 127, 0, 0);
>> +		}
>>  		input_set_capability(dev2, EV_KEY, BTN_LEFT);
>>  		input_set_capability(dev2, EV_KEY, BTN_RIGHT);
>>  		input_set_capability(dev2, EV_KEY, BTN_MIDDLE);
>
> This seems wrong, reporting ABS_PRESSURE on a relative device. And yes, this
> really is a relative device (it sends repeated delta events when you
> keep pushing at the same force, rather then sending a single coordinate
> value).
>
> Maybe we need a REL_PRESSURE for cases like this ?
>
Fair enough. However, if such a thing were added how would one specify
the range of the value (which seems necessary for the pressure to have
any meaning whatsoever)? It seems odd that the distinctions of
absolute/relative and bounded/unbounded values are conflated in the
event interface. It's not clear to me what input_set_abs_params isn't
instead input_set_params.

Cheers

- Ben

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 472 bytes --]

Re: [PATCH 3/4] input/alps: Allow touchsticks to report pressure

[Hans de Goede]

Hi,

On 10-06-16 12:21, Ben Gamari wrote:
> Hans de Goede <hdegoede@redhat.com> writes:
>
>> Hi,
>>>
>>> @@ -2996,6 +2999,10 @@ int alps_init(struct psmouse *psmouse)
>>>
>>>  		input_set_capability(dev2, EV_REL, REL_X);
>>>  		input_set_capability(dev2, EV_REL, REL_Y);
>>> +		if (priv->flags & ALPS_DUALPOINT_WITH_PRESSURE) {
>>> +			input_set_capability(dev2, EV_ABS, ABS_PRESSURE);
>>> +			input_set_abs_params(dev2, ABS_PRESSURE, 0, 127, 0, 0);
>>> +		}
>>>  		input_set_capability(dev2, EV_KEY, BTN_LEFT);
>>>  		input_set_capability(dev2, EV_KEY, BTN_RIGHT);
>>>  		input_set_capability(dev2, EV_KEY, BTN_MIDDLE);
>>
>> This seems wrong, reporting ABS_PRESSURE on a relative device. And yes, this
>> really is a relative device (it sends repeated delta events when you
>> keep pushing at the same force, rather then sending a single coordinate
>> value).
>>
>> Maybe we need a REL_PRESSURE for cases like this ?
>>
> Fair enough. However, if such a thing were added how would one specify
> the range of the value (which seems necessary for the pressure to have
> any meaning whatsoever)?

Good question, adding Peter Hutterer to the Cc.

Peter, some of the newer alps pointing sticks can also report pressure,
which is somewhat of an ill fit for the relative events interface
otherwise used with pointing sticks, do you have any suggestions on
how to deal with this ?

Regards,

Hans

Re: [PATCH 3/4] input/alps: Allow touchsticks to report pressure

[Peter Hutterer]

On Fri, Jun 10, 2016 at 12:57:12PM +0200, Hans de Goede wrote:
> Hi,
> 
> On 10-06-16 12:21, Ben Gamari wrote:
> > Hans de Goede <hdegoede@redhat.com> writes:
> > 
> > > Hi,
> > > > 
> > > > @@ -2996,6 +2999,10 @@ int alps_init(struct psmouse *psmouse)
> > > > 
> > > >  		input_set_capability(dev2, EV_REL, REL_X);
> > > >  		input_set_capability(dev2, EV_REL, REL_Y);
> > > > +		if (priv->flags & ALPS_DUALPOINT_WITH_PRESSURE) {
> > > > +			input_set_capability(dev2, EV_ABS, ABS_PRESSURE);
> > > > +			input_set_abs_params(dev2, ABS_PRESSURE, 0, 127, 0, 0);
> > > > +		}
> > > >  		input_set_capability(dev2, EV_KEY, BTN_LEFT);
> > > >  		input_set_capability(dev2, EV_KEY, BTN_RIGHT);
> > > >  		input_set_capability(dev2, EV_KEY, BTN_MIDDLE);
> > > 
> > > This seems wrong, reporting ABS_PRESSURE on a relative device. And yes, this
> > > really is a relative device (it sends repeated delta events when you
> > > keep pushing at the same force, rather then sending a single coordinate
> > > value).
> > > 
> > > Maybe we need a REL_PRESSURE for cases like this ?
> > > 
> > Fair enough. However, if such a thing were added how would one specify
> > the range of the value (which seems necessary for the pressure to have
> > any meaning whatsoever)?
> 
> Good question, adding Peter Hutterer to the Cc.
> 
> Peter, some of the newer alps pointing sticks can also report pressure,
> which is somewhat of an ill fit for the relative events interface
> otherwise used with pointing sticks, do you have any suggestions on
> how to deal with this ?

I think ABS_PRESSURE is fine here provided the ranges are set up correctly
and he device really sends an absolute value. It might confuse userspace at
first but there is no real requirement for a device to stick to one
interface only.

REL_PRESSURE would be a lot harder to handle because we don't have a
reference frame on what the permitted axis ranges could be and it would make
the actual value effectively meaningless (without a MOUSE_DPI-like database)

Cheers,
   Peter

Re: [PATCH 1/4] input/alps: Add touchstick support for SS5 hardware

[Pali Rohár]

On Saturday 28 May 2016 14:00:59 Ben Gamari wrote:
> Add touchstick support for the so-called SS5 hardware, which uses a
> variant of the SS4 protocol.
> ---
>  drivers/input/mouse/alps.c | 64 ++++++++++++++++++++++++++++++++++++++--------
>  drivers/input/mouse/alps.h |  2 ++
>  2 files changed, 55 insertions(+), 11 deletions(-)

Reviewed-by: Pali Rohár <pali.rohar@gmail.com>

-- 
Pali Rohár
pali.rohar@gmail.com
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/4] input/alps: Handle 0-pressure 1F events

[Pali Rohár]

On Saturday 28 May 2016 14:01:00 Ben Gamari wrote:
> While a button is held SS5 hardware will give us single-finger packets
> with x, y, and pressure equal to zero. This causes annoying jumps in
> pointer position if a touch is released while the button is held. Handle
> this by claiming zero contacts to ensure that no position events are
> provided to the user.
> ---
>  drivers/input/mouse/alps.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)

Reviewed-by: Pali Rohár <pali.rohar@gmail.com>

-- 
Pali Rohár
pali.rohar@gmail.com
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] Support for Alps SS5 touchpad

[Pali Rohár]

[-- Attachment #1: Type: Text/Plain, Size: 506 bytes --]

On Monday 30 May 2016 11:49:25 Ben Gamari wrote:
> Pali Rohár <pali.rohar@gmail.com> writes:
> > Now when you are extending alps.c code for new touchpad, can you
> > also update documentation file Documentation/input/alps.txt to
> > reflect V8 protcol? That would be great to have up-to-date
> > documentation for debugging and future purpose...
> 
> Indeed, it's been on my to-do list.
> 
> Cheers,
> 
> - Ben

Hi Ben! Are you planning to send V3?

-- 
Pali Rohár
pali.rohar@gmail.com

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH 4/4] input/alps: Set DualPoint flag for 74 03 28 devices

[Dmitry Torokhov]

On Mon, May 30, 2016 at 11:11:00AM +0200, Pali Rohár wrote:
> On Saturday 28 May 2016 14:01:02 Ben Gamari wrote:
> > Here we introduce logic in alps_identify to set the ALPS_DUALPOINT flag
> > for touchpad hardware responding to E7 report with 73 03 28, as is found
> > in the Dell Latitude E7470.
> > ---
> >  drivers/input/mouse/alps.c | 6 ++++++
> >  1 file changed, 6 insertions(+)
> > 
> > diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c
> > index 25d2cad..431bc26 100644
> > --- a/drivers/input/mouse/alps.c
> > +++ b/drivers/input/mouse/alps.c
> > @@ -2776,6 +2776,7 @@ static int alps_identify(struct psmouse *psmouse, struct alps_data *priv)
> >  	const struct alps_protocol_info *protocol;
> >  	unsigned char e6[4], e7[4], ec[4];
> >  	int error;
> > +	int flags = 0;
> >  
> >  	/*
> >  	 * First try "E6 report".
> > @@ -2817,6 +2818,10 @@ static int alps_identify(struct psmouse *psmouse, struct alps_data *priv)
> >  		} else if (e7[0] == 0x73 && e7[1] == 0x03 &&
> >  			   e7[2] == 0x14 && ec[1] == 0x02) {
> >  			protocol = &alps_v8_protocol_data;
> > +		} else if (e7[0] == 0x73 && e7[1] == 0x03 &&
> > +			   e7[2] == 0x28 && ec[1] == 0x01) {
> > +			protocol = &alps_v8_protocol_data;
> > +			flags |= ALPS_DUALPOINT | ALPS_DUALPOINT_WITH_PRESSURE;
> >  		} else {
> >  			psmouse_dbg(psmouse,
> >  				    "Likely not an ALPS touchpad: E7=%3ph, EC=%3ph\n", e7, ec);
> > @@ -2830,6 +2835,7 @@ static int alps_identify(struct psmouse *psmouse, struct alps_data *priv)
> >  		error = alps_set_protocol(psmouse, priv, protocol);
> >  		if (error)
> >  			return error;
> > +		priv->flags |= flags;
> >  	}
> >  
> >  	return 0;
> 
> I would rather see setting flags in alps_set_protocol function. Where it
> is done for other protocols. But do not know if it is easily possible
> without rewriting lot of code...

Could probably key it off of prov->fw_ver[1] (which is ec[1]) in
alps_set_protocol() in V8 case.

Thanks.

-- 
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] Support for Alps SS5 touchpad

[Ben Gamari]

[-- Attachment #1: Type: text/plain, Size: 666 bytes --]

Pali Rohár <pali.rohar@gmail.com> writes:

> [ Unknown signature status ]
> On Monday 30 May 2016 11:49:25 Ben Gamari wrote:
>> Pali Rohár <pali.rohar@gmail.com> writes:
>> > Now when you are extending alps.c code for new touchpad, can you
>> > also update documentation file Documentation/input/alps.txt to
>> > reflect V8 protcol? That would be great to have up-to-date
>> > documentation for debugging and future purpose...
>> 
>> Indeed, it's been on my to-do list.
>> 
>> Cheers,
>> 
>> - Ben
>
> Hi Ben! Are you planning to send V3?
>
Done! Thanks for the reminder; I had been sitting on this for a few too
many days.

Cheers,

- Ben

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 472 bytes --]

Re: [PATCH v2] Support for Alps SS5 touchpad

[Pali Rohár]

On Tuesday 21 June 2016 11:20:14 Ben Gamari wrote:
> Pali Rohár <pali.rohar@gmail.com> writes:
> 
> > [ Unknown signature status ]
> > On Monday 30 May 2016 11:49:25 Ben Gamari wrote:
> >> Pali Rohár <pali.rohar@gmail.com> writes:
> >> > Now when you are extending alps.c code for new touchpad, can you
> >> > also update documentation file Documentation/input/alps.txt to
> >> > reflect V8 protcol? That would be great to have up-to-date
> >> > documentation for debugging and future purpose...
> >> 
> >> Indeed, it's been on my to-do list.
> >> 
> >> Cheers,
> >> 
> >> - Ben
> >
> > Hi Ben! Are you planning to send V3?
> >
> Done! Thanks for the reminder; I had been sitting on this for a few too
> many days.

Have you sent it? Because I do not see it my mailbox...

-- 
Pali Rohár
pali.rohar@gmail.com
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] Support for Alps SS5 touchpad

[Ben Gamari]

[-- Attachment #1: Type: text/plain, Size: 1321 bytes --]

Pali Rohár <pali.rohar@gmail.com> writes:

> On Tuesday 21 June 2016 11:20:14 Ben Gamari wrote:
>> Pali Rohár <pali.rohar@gmail.com> writes:
>> 
>> > [ Unknown signature status ]
>> > On Monday 30 May 2016 11:49:25 Ben Gamari wrote:
>> >> Pali Rohár <pali.rohar@gmail.com> writes:
>> >> > Now when you are extending alps.c code for new touchpad, can you
>> >> > also update documentation file Documentation/input/alps.txt to
>> >> > reflect V8 protcol? That would be great to have up-to-date
>> >> > documentation for debugging and future purpose...
>> >> 
>> >> Indeed, it's been on my to-do list.
>> >> 
>> >> Cheers,
>> >> 
>> >> - Ben
>> >
>> > Hi Ben! Are you planning to send V3?
>> >
>> Done! Thanks for the reminder; I had been sitting on this for a few too
>> many days.
>
> Have you sent it? Because I do not see it my mailbox...
>
Very odd; I sent it [1] and indeed you were explicitly added to the
recipient list. You can find the patches on Patchwork here [2]. I
haven't receieved any reviews yet so perhaps the messages weren't
delivered for some reason. If so I could certainly send them again.

Sorry for the latency!

Cheers,

- Ben


[1] http://article.gmane.org/gmane.linux.kernel.input/50117/match=v3+alps+ss5
[2] https://patchwork.kernel.org/patch/9190015/

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 472 bytes --]

Re: [PATCH v2] Support for Alps SS5 touchpad

[Pali Rohár]

[-- Attachment #1: Type: Text/Plain, Size: 1509 bytes --]

On Sunday 17 July 2016 23:06:39 Ben Gamari wrote:
> Pali Rohár <pali.rohar@gmail.com> writes:
> > On Tuesday 21 June 2016 11:20:14 Ben Gamari wrote:
> >> Pali Rohár <pali.rohar@gmail.com> writes:
> >> > [ Unknown signature status ]
> >> > 
> >> > On Monday 30 May 2016 11:49:25 Ben Gamari wrote:
> >> >> Pali Rohár <pali.rohar@gmail.com> writes:
> >> >> > Now when you are extending alps.c code for new touchpad, can
> >> >> > you also update documentation file
> >> >> > Documentation/input/alps.txt to reflect V8 protcol? That
> >> >> > would be great to have up-to-date documentation for
> >> >> > debugging and future purpose...
> >> >> 
> >> >> Indeed, it's been on my to-do list.
> >> >> 
> >> >> Cheers,
> >> >> 
> >> >> - Ben
> >> > 
> >> > Hi Ben! Are you planning to send V3?
> >> 
> >> Done! Thanks for the reminder; I had been sitting on this for a
> >> few too many days.
> > 
> > Have you sent it? Because I do not see it my mailbox...
> 
> Very odd; I sent it [1] and indeed you were explicitly added to the
> recipient list. You can find the patches on Patchwork here [2]. I
> haven't receieved any reviews yet so perhaps the messages weren't
> delivered for some reason. If so I could certainly send them again.

Looks like I did not get patches to my mailbox... Maybe you probably 
forgot to CC emails to my address, as I'm not in To or Cc fields:
http://download.gmane.org/gmane.linux.kernel.input/50117/50118

-- 
Pali Rohár
pali.rohar@gmail.com

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH v2] Support for Alps SS5 touchpad

[Pali Rohár]

Hi! What happened with this patch series?

-- 
Pali Rohár
pali.rohar@gmail.com

Re: [PATCH v2] Support for Alps SS5 touchpad

[Ben Gamari]

[-- Attachment #1: Type: text/plain, Size: 227 bytes --]

Pali Rohár <pali.rohar@gmail.com> writes:

> Hi! What happened with this patch series?
>
Hi Pali,

v3 is still waiting for review [1].

Cheers,

- Ben


[1] http://www.spinics.net/lists/linux-input/msg45295.html

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 472 bytes --]

Re: [PATCH v2] Support for Alps SS5 touchpad

[Pali Rohár]

[-- Attachment #1: Type: Text/Plain, Size: 170 bytes --]

On Tuesday 06 September 2016 05:38:57 Ben Gamari wrote:
> v3 is still waiting for review [1].

Hi, please resend and CC me.

-- 
Pali Rohár
pali.rohar@gmail.com

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]