[PATCH] alps: Non interleaved V2 dualpoint has separate stick button bits

[PATCH] alps: Non interleaved V2 dualpoint has separate stick button bits

[Hans de Goede]

Non interleaved V2 dualpoint touchpad / stick combos have separate stick
button bits in the touchpad packets, if we do not check these then the
trackpoint buttons will not work when using the touchpad, and when pressed
when the user starts using the touchpad will report a release event even
though the button is still pressed.

This commit fixes this by checking the separate bits, note that we simply
combine the buttons, since the hardware does the same when using the touchpad
buttons with the trackpoint, so we do not have enough information to properly
separate them.

Reported-by: Hans de Bruin <jmdebruin@xmsnet.nl>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/input/mouse/alps.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c
index 6962c26..58987b5 100644
--- a/drivers/input/mouse/alps.c
+++ b/drivers/input/mouse/alps.c
@@ -243,6 +243,14 @@ static void alps_process_packet_v1_v2(struct psmouse *psmouse)
 		return;
 	}
 
+	/* Non interleaved V2 dualpoint has separate stick button bits */
+	if (priv->proto_version == ALPS_PROTO_V2 &&
+	    priv->flags == (ALPS_PASS | ALPS_DUALPOINT)) {
+		left |= packet[0] & 1;
+		right |= packet[0] & 2;
+		middle |= packet[0] & 4;
+	}
+
 	alps_report_buttons(dev, dev2, left, right, middle);
 
 	/* Convert hardware tap to a reasonable Z value */
-- 
2.3.4

Re: [PATCH] alps: Non interleaved V2 dualpoint has separate stick button bits

[Dmitry Torokhov]

On Wed, Apr 08, 2015 at 06:04:52PM +0200, Hans de Goede wrote:
> Non interleaved V2 dualpoint touchpad / stick combos have separate stick
> button bits in the touchpad packets, if we do not check these then the
> trackpoint buttons will not work when using the touchpad, and when pressed
> when the user starts using the touchpad will report a release event even
> though the button is still pressed.
> 
> This commit fixes this by checking the separate bits, note that we simply
> combine the buttons, since the hardware does the same when using the touchpad
> buttons with the trackpoint, so we do not have enough information to properly
> separate them.

Hmm, if they are designated as trackstick buttons why don't we report
them as such (i.e. send button events out of dev2 without movement)?

Also, should we make note of this in alps.txt?

Thanks!

> 
> Reported-by: Hans de Bruin <jmdebruin@xmsnet.nl>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  drivers/input/mouse/alps.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c
> index 6962c26..58987b5 100644
> --- a/drivers/input/mouse/alps.c
> +++ b/drivers/input/mouse/alps.c
> @@ -243,6 +243,14 @@ static void alps_process_packet_v1_v2(struct psmouse *psmouse)
>  		return;
>  	}
>  
> +	/* Non interleaved V2 dualpoint has separate stick button bits */
> +	if (priv->proto_version == ALPS_PROTO_V2 &&
> +	    priv->flags == (ALPS_PASS | ALPS_DUALPOINT)) {
> +		left |= packet[0] & 1;
> +		right |= packet[0] & 2;
> +		middle |= packet[0] & 4;
> +	}
> +
>  	alps_report_buttons(dev, dev2, left, right, middle);
>  
>  	/* Convert hardware tap to a reasonable Z value */
> -- 
> 2.3.4
> 

-- 
Dmitry

Re: [PATCH] alps: Non interleaved V2 dualpoint has separate stick button bits

[Hans de Goede]

Hi,

On 08-04-15 18:24, Dmitry Torokhov wrote:
> On Wed, Apr 08, 2015 at 06:04:52PM +0200, Hans de Goede wrote:
>> Non interleaved V2 dualpoint touchpad / stick combos have separate stick
>> button bits in the touchpad packets, if we do not check these then the
>> trackpoint buttons will not work when using the touchpad, and when pressed
>> when the user starts using the touchpad will report a release event even
>> though the button is still pressed.
>>
>> This commit fixes this by checking the separate bits, note that we simply
>> combine the buttons, since the hardware does the same when using the touchpad
>> buttons with the trackpoint, so we do not have enough information to properly
>> separate them.
>
> Hmm, if they are designated as trackstick buttons why don't we report
> them as such (i.e. send button events out of dev2 without movement)?

Because we may then end up with having a button pressed on both
the touchpad and trackstick evdev nodes, and if we then switch to
getting trackstick packets, which have one combined bit for both, and
the bit becomes 0 we end up releasing only one, which is what the commit
message tries to say with: "we do not have enough information to properly
separate them", I actually had an earlier version which did as you suggested,
but that become ugly pretty quickly.

> Also, should we make note of this in alps.txt?

Ack, I'll do a v2 when we've agreement on your first point (and I've
some time).

Regards,

Hans



>
> Thanks!
>
>>
>> Reported-by: Hans de Bruin <jmdebruin@xmsnet.nl>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>>   drivers/input/mouse/alps.c | 8 ++++++++
>>   1 file changed, 8 insertions(+)
>>
>> diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c
>> index 6962c26..58987b5 100644
>> --- a/drivers/input/mouse/alps.c
>> +++ b/drivers/input/mouse/alps.c
>> @@ -243,6 +243,14 @@ static void alps_process_packet_v1_v2(struct psmouse *psmouse)
>>   		return;
>>   	}
>>
>> +	/* Non interleaved V2 dualpoint has separate stick button bits */
>> +	if (priv->proto_version == ALPS_PROTO_V2 &&
>> +	    priv->flags == (ALPS_PASS | ALPS_DUALPOINT)) {
>> +		left |= packet[0] & 1;
>> +		right |= packet[0] & 2;
>> +		middle |= packet[0] & 4;
>> +	}
>> +
>>   	alps_report_buttons(dev, dev2, left, right, middle);
>>
>>   	/* Convert hardware tap to a reasonable Z value */
>> --
>> 2.3.4
>>
>

Re: [PATCH] alps: Non interleaved V2 dualpoint has separate stick button bits

[Dmitry Torokhov]

On Wed, Apr 08, 2015 at 07:05:17PM +0200, Hans de Goede wrote:
> Hi,
> 
> On 08-04-15 18:24, Dmitry Torokhov wrote:
> >On Wed, Apr 08, 2015 at 06:04:52PM +0200, Hans de Goede wrote:
> >>Non interleaved V2 dualpoint touchpad / stick combos have separate stick
> >>button bits in the touchpad packets, if we do not check these then the
> >>trackpoint buttons will not work when using the touchpad, and when pressed
> >>when the user starts using the touchpad will report a release event even
> >>though the button is still pressed.
> >>
> >>This commit fixes this by checking the separate bits, note that we simply
> >>combine the buttons, since the hardware does the same when using the touchpad
> >>buttons with the trackpoint, so we do not have enough information to properly
> >>separate them.
> >
> >Hmm, if they are designated as trackstick buttons why don't we report
> >them as such (i.e. send button events out of dev2 without movement)?
> 
> Because we may then end up with having a button pressed on both
> the touchpad and trackstick evdev nodes, and if we then switch to
> getting trackstick packets, which have one combined bit for both, and
> the bit becomes 0 we end up releasing only one, which is what the commit
> message tries to say with: "we do not have enough information to properly
> separate them", I actually had an earlier version which did as you suggested,
> but that become ugly pretty quickly.

Oh, right... Do we get touchpad packets if both trackpoint and touchpad
buttons are pressed and trackpoint is moved (bit no contacts on the pad)?

Anyway, I guess it is older hardware and just mixing buttons is fine, no
need to be super-fancy.

> 
> >Also, should we make note of this in alps.txt?
> 
> Ack, I'll do a v2 when we've agreement on your first point (and I've
> some time).
> 
> Regards,
> 
> Hans
> 
> 
> 
> >
> >Thanks!
> >
> >>
> >>Reported-by: Hans de Bruin <jmdebruin@xmsnet.nl>
> >>Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> >>---
> >>  drivers/input/mouse/alps.c | 8 ++++++++
> >>  1 file changed, 8 insertions(+)
> >>
> >>diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c
> >>index 6962c26..58987b5 100644
> >>--- a/drivers/input/mouse/alps.c
> >>+++ b/drivers/input/mouse/alps.c
> >>@@ -243,6 +243,14 @@ static void alps_process_packet_v1_v2(struct psmouse *psmouse)
> >>  		return;
> >>  	}
> >>
> >>+	/* Non interleaved V2 dualpoint has separate stick button bits */
> >>+	if (priv->proto_version == ALPS_PROTO_V2 &&
> >>+	    priv->flags == (ALPS_PASS | ALPS_DUALPOINT)) {
> >>+		left |= packet[0] & 1;
> >>+		right |= packet[0] & 2;
> >>+		middle |= packet[0] & 4;
> >>+	}
> >>+
> >>  	alps_report_buttons(dev, dev2, left, right, middle);
> >>
> >>  	/* Convert hardware tap to a reasonable Z value */
> >>--
> >>2.3.4
> >>
> >

-- 
Dmitry

Re: [PATCH] alps: Non interleaved V2 dualpoint has separate stick button bits

[Hans de Goede]

Hi,

On 08-04-15 19:38, Dmitry Torokhov wrote:
> On Wed, Apr 08, 2015 at 07:05:17PM +0200, Hans de Goede wrote:
>> Hi,
>>
>> On 08-04-15 18:24, Dmitry Torokhov wrote:
>>> On Wed, Apr 08, 2015 at 06:04:52PM +0200, Hans de Goede wrote:
>>>> Non interleaved V2 dualpoint touchpad / stick combos have separate stick
>>>> button bits in the touchpad packets, if we do not check these then the
>>>> trackpoint buttons will not work when using the touchpad, and when pressed
>>>> when the user starts using the touchpad will report a release event even
>>>> though the button is still pressed.
>>>>
>>>> This commit fixes this by checking the separate bits, note that we simply
>>>> combine the buttons, since the hardware does the same when using the touchpad
>>>> buttons with the trackpoint, so we do not have enough information to properly
>>>> separate them.
>>>
>>> Hmm, if they are designated as trackstick buttons why don't we report
>>> them as such (i.e. send button events out of dev2 without movement)?
>>
>> Because we may then end up with having a button pressed on both
>> the touchpad and trackstick evdev nodes, and if we then switch to
>> getting trackstick packets, which have one combined bit for both, and
>> the bit becomes 0 we end up releasing only one, which is what the commit
>> message tries to say with: "we do not have enough information to properly
>> separate them", I actually had an earlier version which did as you suggested,
>> but that become ugly pretty quickly.
>
> Oh, right... Do we get touchpad packets if both trackpoint and touchpad
> buttons are pressed and trackpoint is moved (bit no contacts on the pad)?

No, if the user does:

1) Press touchpad button
2) Move trackpoint

We get:

1) Touchpad packet with touchpad button pressed bit set
2) Trackpoint packets with "a button pressed" bit set and
movement

> Anyway, I guess it is older hardware and just mixing buttons is fine, no
> need to be super-fancy.

Ack, I'll do a v2 adding the requested documentation update tomorrow.

Regards,

Hans

Re: [PATCH] alps: Non interleaved V2 dualpoint has separate stick button bits

[Dmitry Torokhov]

On Thu, Apr 09, 2015 at 02:49:10PM +0200, Hans de Goede wrote:
> Hi,
> 
> On 08-04-15 19:38, Dmitry Torokhov wrote:
> >On Wed, Apr 08, 2015 at 07:05:17PM +0200, Hans de Goede wrote:
> >>Hi,
> >>
> >>On 08-04-15 18:24, Dmitry Torokhov wrote:
> >>>On Wed, Apr 08, 2015 at 06:04:52PM +0200, Hans de Goede wrote:
> >>>>Non interleaved V2 dualpoint touchpad / stick combos have separate stick
> >>>>button bits in the touchpad packets, if we do not check these then the
> >>>>trackpoint buttons will not work when using the touchpad, and when pressed
> >>>>when the user starts using the touchpad will report a release event even
> >>>>though the button is still pressed.
> >>>>
> >>>>This commit fixes this by checking the separate bits, note that we simply
> >>>>combine the buttons, since the hardware does the same when using the touchpad
> >>>>buttons with the trackpoint, so we do not have enough information to properly
> >>>>separate them.
> >>>
> >>>Hmm, if they are designated as trackstick buttons why don't we report
> >>>them as such (i.e. send button events out of dev2 without movement)?
> >>
> >>Because we may then end up with having a button pressed on both
> >>the touchpad and trackstick evdev nodes, and if we then switch to
> >>getting trackstick packets, which have one combined bit for both, and
> >>the bit becomes 0 we end up releasing only one, which is what the commit
> >>message tries to say with: "we do not have enough information to properly
> >>separate them", I actually had an earlier version which did as you suggested,
> >>but that become ugly pretty quickly.
> >
> >Oh, right... Do we get touchpad packets if both trackpoint and touchpad
> >buttons are pressed and trackpoint is moved (bit no contacts on the pad)?
> 
> No, if the user does:
> 
> 1) Press touchpad button
> 2) Move trackpoint
> 
> We get:
> 
> 1) Touchpad packet with touchpad button pressed bit set
> 2) Trackpoint packets with "a button pressed" bit set and
> movement
> 
> >Anyway, I guess it is older hardware and just mixing buttons is fine, no
> >need to be super-fancy.
> 
> Ack, I'll do a v2 adding the requested documentation update tomorrow.

Hans, I applied the patch as is so please just send the doc update.

Thanks!

-- 
Dmitry