[PATCH FIX for 4.0 0/2] alps: Report alps v2 Dualpoint Stick events via the right event node

[PATCH FIX for 4.0 0/2] alps: Report alps v2 Dualpoint Stick events via the right event node

[Hans de Goede]

Hi Dmitry & Pali,

While working on some libinput code to deal with trackpoints of different
model laptops having quite different speed / sensitivity ootb, I noticed that
with the current 4.0-rc# kernels the trackpoint events on laptops with a
PROTO_V2 alps touchpad are no longer being send by the "Dualpoint Stick"
event node, instead a new "ALPS PS/2 Mouse" node gets created and sends
the trackpoint events.

The cause of this is that the stick on these devices sends bare ps/2 packets
as data.

Although this does not really break anything (atm, it does break my libinput
work), it is still wrong, esp. also since the "Dualpoint Stick" node has
the POINTING_STICK property set, where as the "ALPS PS/2 Mouse" node which
is actually sending the stick events does not.

If still possible I would like to see this fixes added to 4.0, if not we should
queue them up for stable.

Regards,

Hans

[PATCH 1/2] alps: Report bare packets coming from alps_handle_interleaved_ps2 via dev3

[Hans de Goede]

Bare packets should be reported via the same evdev device independent on
whether they are detected on the beginning of a packet or in the middle
of a packet.

This has been tested on a Dell Latitude E6400, where the DualPoint Stick
reports bare packets, which when the touchpad is idle get reported via
dev3, and when the touchpad ans tick are used simulateniously via dev2.

This commit fixes this inconsistency by always reporting bare packets via
dev3. Note that since the come from a DualPoint Stick they really should be
reported via dev2, this gets fixed in a later commit.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/input/mouse/alps.c | 32 ++++++++++++++++++--------------
 1 file changed, 18 insertions(+), 14 deletions(-)

diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c
index 1bd15eb..f2c5545 100644
--- a/drivers/input/mouse/alps.c
+++ b/drivers/input/mouse/alps.c
@@ -1154,10 +1154,23 @@ out:
 	mutex_unlock(&alps_mutex);
 }
 
-static void alps_report_bare_ps2_packet(struct input_dev *dev,
+static void alps_report_bare_ps2_packet(struct psmouse *psmouse,
 					unsigned char packet[],
 					bool report_buttons)
 {
+	struct alps_data *priv = psmouse->private;
+	struct input_dev *dev;
+
+	if (unlikely(IS_ERR_OR_NULL(priv->dev3))) {
+		/* Register dev3 mouse if we received PS/2 packet first time */
+		if (!IS_ERR(priv->dev3))
+			psmouse_queue_work(psmouse, &priv->dev3_register_work,
+					   0);
+		return;
+	} else {
+		dev = priv->dev3;
+	}
+
 	if (report_buttons)
 		alps_report_buttons(dev, NULL,
 				packet[0] & 1, packet[0] & 2, packet[0] & 4);
@@ -1232,8 +1245,8 @@ static psmouse_ret_t alps_handle_interleaved_ps2(struct psmouse *psmouse)
 		 * de-synchronization.
 		 */
 
-		alps_report_bare_ps2_packet(priv->dev2,
-					    &psmouse->packet[3], false);
+		alps_report_bare_ps2_packet(psmouse, &psmouse->packet[3],
+					    false);
 
 		/*
 		 * Continue with the standard ALPS protocol handling,
@@ -1289,18 +1302,9 @@ static psmouse_ret_t alps_process_byte(struct psmouse *psmouse)
 	 * properly we only do this if the device is fully synchronized.
 	 */
 	if (!psmouse->out_of_sync_cnt && (psmouse->packet[0] & 0xc8) == 0x08) {
-
-		/* Register dev3 mouse if we received PS/2 packet first time */
-		if (unlikely(!priv->dev3))
-			psmouse_queue_work(psmouse,
-					   &priv->dev3_register_work, 0);
-
 		if (psmouse->pktcnt == 3) {
-			/* Once dev3 mouse device is registered report data */
-			if (likely(!IS_ERR_OR_NULL(priv->dev3)))
-				alps_report_bare_ps2_packet(priv->dev3,
-							    psmouse->packet,
-							    true);
+			alps_report_bare_ps2_packet(psmouse, psmouse->packet,
+						    true);
 			return PSMOUSE_FULL_PACKET;
 		}
 		return PSMOUSE_GOOD_DATA;
-- 
2.3.4

[PATCH 2/2] alps: Report alps v2 Dualpoint Stick events via the right evdev node

[Hans de Goede]

On V2 devices the DualPoint Stick reports bare packets, these should be
reported via the "AlpsPS/2 ALPS DualPoint Stick" dev2 evdev node, which also
has the INPUT_PROP_POINTING_STICK propbit set.

Note that since there is no way to distuingish these packets from an external
ps/2 mouse (in sofar as these laptops have an external ps/2 port) this means
that we will be reporting ps/2 mouse events via this evdev node too, as we've
been doing in kernel 3.19 and older.

This has been tested on a Dell Latitude D620 and a Dell Latitude E6400,
which both have a V2 touchpad + a DualPoint Stick which reports bare packets.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 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 f2c5545..80b6eca 100644
--- a/drivers/input/mouse/alps.c
+++ b/drivers/input/mouse/alps.c
@@ -1161,7 +1161,12 @@ static void alps_report_bare_ps2_packet(struct psmouse *psmouse,
 	struct alps_data *priv = psmouse->private;
 	struct input_dev *dev;
 
-	if (unlikely(IS_ERR_OR_NULL(priv->dev3))) {
+	/* Figure out which device to use to report the bare packet */
+	if (priv->proto_version == ALPS_PROTO_V2 &&
+	    (priv->flags & ALPS_DUALPOINT)) {
+		/* On V2 devices the DualPoint Stick reports bare packets */
+		dev = priv->dev2;
+	} else if (unlikely(IS_ERR_OR_NULL(priv->dev3))) {
 		/* Register dev3 mouse if we received PS/2 packet first time */
 		if (!IS_ERR(priv->dev3))
 			psmouse_queue_work(psmouse, &priv->dev3_register_work,
-- 
2.3.4

Re: [PATCH FIX for 4.0 0/2] alps: Report alps v2 Dualpoint Stick events via the right event node

[Pali Rohár]

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

On Wednesday 01 April 2015 17:44:04 Hans de Goede wrote:
> Hi Dmitry & Pali,
> 
> While working on some libinput code to deal with trackpoints
> of different model laptops having quite different speed /
> sensitivity ootb, I noticed that with the current 4.0-rc#
> kernels the trackpoint events on laptops with a PROTO_V2 alps
> touchpad are no longer being send by the "Dualpoint Stick"
> event node, instead a new "ALPS PS/2 Mouse" node gets created
> and sends the trackpoint events.
> 
> The cause of this is that the stick on these devices sends
> bare ps/2 packets as data.
> 
> Although this does not really break anything (atm, it does
> break my libinput work), it is still wrong, esp. also since
> the "Dualpoint Stick" node has the POINTING_STICK property
> set, where as the "ALPS PS/2 Mouse" node which is actually
> sending the stick events does not.
> 
> If still possible I would like to see this fixes added to 4.0,
> if not we should queue them up for stable.
> 
> Regards,
> 
> Hans

Hi Hans, thanks for testing!

I would like to see this observation also in alps protocol 
documentation, where is all information how are data from 
touchpads and tracksticks reported.

Also for me it it makes sense to squash both patches into one.

And can you check if your change is needed only for 
ALPS_PS2_INTERLEAVED devices? For me it looks like that only 
those alps devices could mix both 3-bytes and 6-bytes packets...

-- 
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 FIX for 4.0 0/2] alps: Report alps v2 Dualpoint Stick events via the right event node

[Hans de Goede]

Hi,

On 01-04-15 18:11, Pali Rohár wrote:
> On Wednesday 01 April 2015 17:44:04 Hans de Goede wrote:
>> Hi Dmitry & Pali,
>>
>> While working on some libinput code to deal with trackpoints
>> of different model laptops having quite different speed /
>> sensitivity ootb, I noticed that with the current 4.0-rc#
>> kernels the trackpoint events on laptops with a PROTO_V2 alps
>> touchpad are no longer being send by the "Dualpoint Stick"
>> event node, instead a new "ALPS PS/2 Mouse" node gets created
>> and sends the trackpoint events.
>>
>> The cause of this is that the stick on these devices sends
>> bare ps/2 packets as data.
>>
>> Although this does not really break anything (atm, it does
>> break my libinput work), it is still wrong, esp. also since
>> the "Dualpoint Stick" node has the POINTING_STICK property
>> set, where as the "ALPS PS/2 Mouse" node which is actually
>> sending the stick events does not.
>>
>> If still possible I would like to see this fixes added to 4.0,
>> if not we should queue them up for stable.
>>
>> Regards,
>>
>> Hans
>
> Hi Hans, thanks for testing!
>
> I would like to see this observation also in alps protocol
> documentation, where is all information how are data from
> touchpads and tracksticks reported.

Good point, for today I'm already a bit late with logging
off, but I'll do a follow up patch for this tomorrow.

> Also for me it it makes sense to squash both patches into one.

I think having them separate is slightly better, but either
way is fine with me,

> And can you check if your change is needed only for
> ALPS_PS2_INTERLEAVED devices? For me it looks like that only
> those alps devices could mix both 3-bytes and 6-bytes packets...

That is what I expected in the beginning too, but no that is
not the case, this happens on my Latitude D620 too, and that
one does not use / set ALPS_PS2_INTERLEAVED.

Regards,

Hans

p.s.

Talking about ALPS_PS2_INTERLEAVED did you ever try that on the
current troublesome models which sometime get out of sync?

Maybe they are actually interleaving things again ?

Regards,

Hans
--
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 FIX for 4.0 0/2] alps: Report alps v2 Dualpoint Stick events via the right event node

[Pali Rohár]

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

On Wednesday 01 April 2015 18:14:43 Hans de Goede wrote:
> Hi,
> 
> On 01-04-15 18:11, Pali Rohár wrote:
> > On Wednesday 01 April 2015 17:44:04 Hans de Goede wrote:
> >> Hi Dmitry & Pali,
> >> 
> >> While working on some libinput code to deal with
> >> trackpoints of different model laptops having quite
> >> different speed / sensitivity ootb, I noticed that with
> >> the current 4.0-rc# kernels the trackpoint events on
> >> laptops with a PROTO_V2 alps touchpad are no longer being
> >> send by the "Dualpoint Stick" event node, instead a new
> >> "ALPS PS/2 Mouse" node gets created and sends the
> >> trackpoint events.
> >> 
> >> The cause of this is that the stick on these devices sends
> >> bare ps/2 packets as data.
> >> 
> >> Although this does not really break anything (atm, it does
> >> break my libinput work), it is still wrong, esp. also since
> >> the "Dualpoint Stick" node has the POINTING_STICK property
> >> set, where as the "ALPS PS/2 Mouse" node which is actually
> >> sending the stick events does not.
> >> 
> >> If still possible I would like to see this fixes added to
> >> 4.0, if not we should queue them up for stable.
> >> 
> >> Regards,
> >> 
> >> Hans
> > 
> > Hi Hans, thanks for testing!
> > 
> > I would like to see this observation also in alps protocol
> > documentation, where is all information how are data from
> > touchpads and tracksticks reported.
> 
> Good point, for today I'm already a bit late with logging
> off, but I'll do a follow up patch for this tomorrow.
> 
> > Also for me it it makes sense to squash both patches into
> > one.
> 
> I think having them separate is slightly better, but either
> way is fine with me,
> 
> > And can you check if your change is needed only for
> > ALPS_PS2_INTERLEAVED devices? For me it looks like that only
> > those alps devices could mix both 3-bytes and 6-bytes
> > packets...
> 
> That is what I expected in the beginning too, but no that is
> not the case, this happens on my Latitude D620 too, and that
> one does not use / set ALPS_PS2_INTERLEAVED.
> 

Ok. It really needs documentation.

And I'm also for including fix to 4.0 stable kernel as this is 
regression for those dell models.

> Regards,
> 
> Hans
> 
> p.s.
> 
> Talking about ALPS_PS2_INTERLEAVED did you ever try that on
> the current troublesome models which sometime get out of
> sync?
> 
> Maybe they are actually interleaving things again ?
> 
> Regards,
> 
> Hans

No, there are no interleaved packets. All packets are 6-bytes 
length, but some have swapped bits... When I saw those lot of 
invalid packets I thought it is similar to ALPS_PS2_INTERLEAVED. 
But not.

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

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