Re: [PATCH] input: multi-touch fix for ALPS touchpads ("SS4 plus" variant)
On Fri, Jan 12, 2018 at 01:02:55AM +, Masaki Ota wrote:
> Hi, Nir,
>
> Wow, thank you for fixing the bug.
> Your code is correct!
Great, I am putting you down as "Reviewed-by" then. Thanks!
>
> Best Regards,
> Masaki Ota
> -Original Message-
> From: Nir Perry [mailto:nirpe...@gmail.com]
> Sent: Saturday, January 06, 2018 8:55 PM
> To: 太田 真喜 Masaki Ota <masaki@jp.alps.com>; Dmitry Torokhov
> <dmitry.torok...@gmail.com>; Pali Rohár <pali.ro...@gmail.com>
> Cc: linux-kernel@vger.kernel.org; linux-in...@vger.kernel.org
> Subject: [PATCH] input: multi-touch fix for ALPS touchpads ("SS4 plus"
> variant)
>
> Hi all,
>
> I think a minor "typo" bug was accidentally introduced to ALPS touchpad
> driver by a previous bug-fix (commit
> 4a646580f793d19717f7e034c8d473b509c27d49, "Input: ALPS - fix two-finger
> scroll breakage in right side on ALPS touchpad").
> It breaks how multi-touch events are decoded on some ALPS touchpads, so for
> example tapping with three-fingers can no longer be used to emulate
> middle-mouse-button (the kernel doesn't recognize this as the proper event,
> and doesn't report it correctly to userspace).
> This affects touchpads that use SS4 "plus" protocol variant, like those found
> on Dell E7270 & E7470 laptops (tested on E7270).
>
> The cause of the problem
> --
> First, probably due to a typo, the code in alps_decode_ss4_v2() for case
> SS4_PACKET_ID_MULTI used inconsistent indices to "f->mt[]". You can see 0 & 1
> are used for the "if" part but 2 & 3 are used for the "else" part, which I
> believe is a typo.
> Second, in the previous patch, new macros were introduced to decode X
> coordinates specific to the SS4 "plus" variant, but the macro to define the
> maximum X value wasn't changed accordingly. The macros to decode X values for
> "plus" variant are effectively shifted right by 1 bit, but the max wasn't
> shifted too. This causes the driver to incorrectly handle "no data" cases,
> which also interfered with how multi-touch was handled. To fix it - I created
> new SS4 "plus" macros for the max value - SS4_PLUS_MFPACKET_NO_AX &
> SS4_PLUS_MFPACKET_NO_AX_BL. To make the change a little more readable, I
> moved also the Y-max lines so they are closer to the X-max lines.
> To get three-finger tap to work both changes are required.
>
> The included patch was generated against the mainline tree today, but was
> also tested against the 4.14 kernel branch. I've included in this e-mail the
> people involved with the old patch from August, plus Pali Rohár who is listed
> as the ALPS PS/2 touchpad driver reviewer (in the maintainers file).
>
> Fixes: 4a646580f793d19717f7e034c8d473b509c27d49 ("Input: ALPS - fix
> two-finger scroll breakage in right side on ALPS touchpad")
>
> Regards,
> Nir
>
> Signed-off-by: Nir Perry <nirpe...@gmail.com> diff --git
> a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c index
> 579b899..dbe57da 100644
> --- a/drivers/input/mouse/alps.c
> +++ b/drivers/input/mouse/alps.c
> @@ -1250,29 +1250,32 @@ static int alps_decode_ss4_v2(struct alps_fields *f,
> case SS4_PACKET_ID_MULTI:
> if (priv->flags & ALPS_BUTTONPAD) {
> if (IS_SS4PLUS_DEV(priv->dev_id)) {
> - f->mt[0].x = SS4_PLUS_BTL_MF_X_V2(p, 0);
> - f->mt[1].x = SS4_PLUS_BTL_MF_X_V2(p, 1);
> + f->mt[2].x = SS4_PLUS_BTL_MF_X_V2(p, 0);
> + f->mt[3].x = SS4_PLUS_BTL_MF_X_V2(p, 1);
> + no_data_x = SS4_PLUS_MFPACKET_NO_AX_BL;
> } else {
> f->mt[2].x = SS4_BTL_MF_X_V2(p, 0);
> f->mt[3].x = SS4_BTL_MF_X_V2(p, 1);
> + no_data_x = SS4_MFPACKET_NO_AX_BL;
> }
> + no_data_y = SS4_MFPACKET_NO_AY_BL;
>
> f->mt[2].y = SS4_BTL_MF_Y_V2(p, 0);
> f->mt[3].y = SS4_BTL_MF_Y_V2(p, 1);
> - no_data_x = SS4_MFPACKET_NO_AX_BL;
> - no_data_y = SS4_MFPACKET_NO_AY_BL;
> } else {
> if (IS_SS4PLUS_DEV(priv->dev_id)) {
> - f->mt[0].x = SS4_PLUS_STD_MF_X_V2(p, 0);
> - f->mt[1].x = SS4_PLUS_STD_MF_X_V2(p, 1);
> +
Re: [PATCH] input: multi-touch fix for ALPS touchpads ("SS4 plus" variant)
On Fri, Jan 12, 2018 at 01:02:55AM +, Masaki Ota wrote:
> Hi, Nir,
>
> Wow, thank you for fixing the bug.
> Your code is correct!
Great, I am putting you down as "Reviewed-by" then. Thanks!
>
> Best Regards,
> Masaki Ota
> -Original Message-
> From: Nir Perry [mailto:nirpe...@gmail.com]
> Sent: Saturday, January 06, 2018 8:55 PM
> To: 太田 真喜 Masaki Ota ; Dmitry Torokhov
> ; Pali Rohár
> Cc: linux-kernel@vger.kernel.org; linux-in...@vger.kernel.org
> Subject: [PATCH] input: multi-touch fix for ALPS touchpads ("SS4 plus"
> variant)
>
> Hi all,
>
> I think a minor "typo" bug was accidentally introduced to ALPS touchpad
> driver by a previous bug-fix (commit
> 4a646580f793d19717f7e034c8d473b509c27d49, "Input: ALPS - fix two-finger
> scroll breakage in right side on ALPS touchpad").
> It breaks how multi-touch events are decoded on some ALPS touchpads, so for
> example tapping with three-fingers can no longer be used to emulate
> middle-mouse-button (the kernel doesn't recognize this as the proper event,
> and doesn't report it correctly to userspace).
> This affects touchpads that use SS4 "plus" protocol variant, like those found
> on Dell E7270 & E7470 laptops (tested on E7270).
>
> The cause of the problem
> --
> First, probably due to a typo, the code in alps_decode_ss4_v2() for case
> SS4_PACKET_ID_MULTI used inconsistent indices to "f->mt[]". You can see 0 & 1
> are used for the "if" part but 2 & 3 are used for the "else" part, which I
> believe is a typo.
> Second, in the previous patch, new macros were introduced to decode X
> coordinates specific to the SS4 "plus" variant, but the macro to define the
> maximum X value wasn't changed accordingly. The macros to decode X values for
> "plus" variant are effectively shifted right by 1 bit, but the max wasn't
> shifted too. This causes the driver to incorrectly handle "no data" cases,
> which also interfered with how multi-touch was handled. To fix it - I created
> new SS4 "plus" macros for the max value - SS4_PLUS_MFPACKET_NO_AX &
> SS4_PLUS_MFPACKET_NO_AX_BL. To make the change a little more readable, I
> moved also the Y-max lines so they are closer to the X-max lines.
> To get three-finger tap to work both changes are required.
>
> The included patch was generated against the mainline tree today, but was
> also tested against the 4.14 kernel branch. I've included in this e-mail the
> people involved with the old patch from August, plus Pali Rohár who is listed
> as the ALPS PS/2 touchpad driver reviewer (in the maintainers file).
>
> Fixes: 4a646580f793d19717f7e034c8d473b509c27d49 ("Input: ALPS - fix
> two-finger scroll breakage in right side on ALPS touchpad")
>
> Regards,
> Nir
>
> Signed-off-by: Nir Perry diff --git
> a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c index
> 579b899..dbe57da 100644
> --- a/drivers/input/mouse/alps.c
> +++ b/drivers/input/mouse/alps.c
> @@ -1250,29 +1250,32 @@ static int alps_decode_ss4_v2(struct alps_fields *f,
> case SS4_PACKET_ID_MULTI:
> if (priv->flags & ALPS_BUTTONPAD) {
> if (IS_SS4PLUS_DEV(priv->dev_id)) {
> - f->mt[0].x = SS4_PLUS_BTL_MF_X_V2(p, 0);
> - f->mt[1].x = SS4_PLUS_BTL_MF_X_V2(p, 1);
> + f->mt[2].x = SS4_PLUS_BTL_MF_X_V2(p, 0);
> + f->mt[3].x = SS4_PLUS_BTL_MF_X_V2(p, 1);
> + no_data_x = SS4_PLUS_MFPACKET_NO_AX_BL;
> } else {
> f->mt[2].x = SS4_BTL_MF_X_V2(p, 0);
> f->mt[3].x = SS4_BTL_MF_X_V2(p, 1);
> + no_data_x = SS4_MFPACKET_NO_AX_BL;
> }
> + no_data_y = SS4_MFPACKET_NO_AY_BL;
>
> f->mt[2].y = SS4_BTL_MF_Y_V2(p, 0);
> f->mt[3].y = SS4_BTL_MF_Y_V2(p, 1);
> - no_data_x = SS4_MFPACKET_NO_AX_BL;
> - no_data_y = SS4_MFPACKET_NO_AY_BL;
> } else {
> if (IS_SS4PLUS_DEV(priv->dev_id)) {
> - f->mt[0].x = SS4_PLUS_STD_MF_X_V2(p, 0);
> - f->mt[1].x = SS4_PLUS_STD_MF_X_V2(p, 1);
> + f->mt[2].x = SS4_PLUS_STD_MF_X_V2(p, 0);
> + f->mt[3]
RE: [PATCH] input: multi-touch fix for ALPS touchpads ("SS4 plus" variant)
Hi, Nir,
Wow, thank you for fixing the bug.
Your code is correct!
Best Regards,
Masaki Ota
-Original Message-
From: Nir Perry [mailto:nirpe...@gmail.com]
Sent: Saturday, January 06, 2018 8:55 PM
To: 太田 真喜 Masaki Ota <masaki@jp.alps.com>; Dmitry Torokhov
<dmitry.torok...@gmail.com>; Pali Rohár <pali.ro...@gmail.com>
Cc: linux-kernel@vger.kernel.org; linux-in...@vger.kernel.org
Subject: [PATCH] input: multi-touch fix for ALPS touchpads ("SS4 plus" variant)
Hi all,
I think a minor "typo" bug was accidentally introduced to ALPS touchpad driver
by a previous bug-fix (commit 4a646580f793d19717f7e034c8d473b509c27d49, "Input:
ALPS - fix two-finger scroll breakage in right side on ALPS touchpad").
It breaks how multi-touch events are decoded on some ALPS touchpads, so for
example tapping with three-fingers can no longer be used to emulate
middle-mouse-button (the kernel doesn't recognize this as the proper event, and
doesn't report it correctly to userspace).
This affects touchpads that use SS4 "plus" protocol variant, like those found
on Dell E7270 & E7470 laptops (tested on E7270).
The cause of the problem
--
First, probably due to a typo, the code in alps_decode_ss4_v2() for case
SS4_PACKET_ID_MULTI used inconsistent indices to "f->mt[]". You can see 0 & 1
are used for the "if" part but 2 & 3 are used for the "else" part, which I
believe is a typo.
Second, in the previous patch, new macros were introduced to decode X
coordinates specific to the SS4 "plus" variant, but the macro to define the
maximum X value wasn't changed accordingly. The macros to decode X values for
"plus" variant are effectively shifted right by 1 bit, but the max wasn't
shifted too. This causes the driver to incorrectly handle "no data" cases,
which also interfered with how multi-touch was handled. To fix it - I created
new SS4 "plus" macros for the max value - SS4_PLUS_MFPACKET_NO_AX &
SS4_PLUS_MFPACKET_NO_AX_BL. To make the change a little more readable, I moved
also the Y-max lines so they are closer to the X-max lines.
To get three-finger tap to work both changes are required.
The included patch was generated against the mainline tree today, but was also
tested against the 4.14 kernel branch. I've included in this e-mail the people
involved with the old patch from August, plus Pali Rohár who is listed as the
ALPS PS/2 touchpad driver reviewer (in the maintainers file).
Fixes: 4a646580f793d19717f7e034c8d473b509c27d49 ("Input: ALPS - fix two-finger
scroll breakage in right side on ALPS touchpad")
Regards,
Nir
Signed-off-by: Nir Perry <nirpe...@gmail.com> diff --git
a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c index
579b899..dbe57da 100644
--- a/drivers/input/mouse/alps.c
+++ b/drivers/input/mouse/alps.c
@@ -1250,29 +1250,32 @@ static int alps_decode_ss4_v2(struct alps_fields *f,
case SS4_PACKET_ID_MULTI:
if (priv->flags & ALPS_BUTTONPAD) {
if (IS_SS4PLUS_DEV(priv->dev_id)) {
- f->mt[0].x = SS4_PLUS_BTL_MF_X_V2(p, 0);
- f->mt[1].x = SS4_PLUS_BTL_MF_X_V2(p, 1);
+ f->mt[2].x = SS4_PLUS_BTL_MF_X_V2(p, 0);
+ f->mt[3].x = SS4_PLUS_BTL_MF_X_V2(p, 1);
+ no_data_x = SS4_PLUS_MFPACKET_NO_AX_BL;
} else {
f->mt[2].x = SS4_BTL_MF_X_V2(p, 0);
f->mt[3].x = SS4_BTL_MF_X_V2(p, 1);
+ no_data_x = SS4_MFPACKET_NO_AX_BL;
}
+ no_data_y = SS4_MFPACKET_NO_AY_BL;
f->mt[2].y = SS4_BTL_MF_Y_V2(p, 0);
f->mt[3].y = SS4_BTL_MF_Y_V2(p, 1);
- no_data_x = SS4_MFPACKET_NO_AX_BL;
- no_data_y = SS4_MFPACKET_NO_AY_BL;
} else {
if (IS_SS4PLUS_DEV(priv->dev_id)) {
- f->mt[0].x = SS4_PLUS_STD_MF_X_V2(p, 0);
- f->mt[1].x = SS4_PLUS_STD_MF_X_V2(p, 1);
+ f->mt[2].x = SS4_PLUS_STD_MF_X_V2(p, 0);
+ f->mt[3].x = SS4_PLUS_STD_MF_X_V2(p, 1);
+ no_data_x = SS4_PLUS_MFPACKET_NO_AX;
} else {
- f->mt[0].x = SS4_STD_MF_X_V2(p, 0);
- f->mt[1].x = SS4_STD_MF_X_V2(p, 1);
+ f->mt[2].x = SS4_STD_MF_X_V2(p, 0);
+ f->mt[3].x = SS4_STD_MF_X_V2(p, 1);
+ no_data_x
RE: [PATCH] input: multi-touch fix for ALPS touchpads ("SS4 plus" variant)
Hi, Nir,
Wow, thank you for fixing the bug.
Your code is correct!
Best Regards,
Masaki Ota
-Original Message-
From: Nir Perry [mailto:nirpe...@gmail.com]
Sent: Saturday, January 06, 2018 8:55 PM
To: 太田 真喜 Masaki Ota ; Dmitry Torokhov
; Pali Rohár
Cc: linux-kernel@vger.kernel.org; linux-in...@vger.kernel.org
Subject: [PATCH] input: multi-touch fix for ALPS touchpads ("SS4 plus" variant)
Hi all,
I think a minor "typo" bug was accidentally introduced to ALPS touchpad driver
by a previous bug-fix (commit 4a646580f793d19717f7e034c8d473b509c27d49, "Input:
ALPS - fix two-finger scroll breakage in right side on ALPS touchpad").
It breaks how multi-touch events are decoded on some ALPS touchpads, so for
example tapping with three-fingers can no longer be used to emulate
middle-mouse-button (the kernel doesn't recognize this as the proper event, and
doesn't report it correctly to userspace).
This affects touchpads that use SS4 "plus" protocol variant, like those found
on Dell E7270 & E7470 laptops (tested on E7270).
The cause of the problem
--
First, probably due to a typo, the code in alps_decode_ss4_v2() for case
SS4_PACKET_ID_MULTI used inconsistent indices to "f->mt[]". You can see 0 & 1
are used for the "if" part but 2 & 3 are used for the "else" part, which I
believe is a typo.
Second, in the previous patch, new macros were introduced to decode X
coordinates specific to the SS4 "plus" variant, but the macro to define the
maximum X value wasn't changed accordingly. The macros to decode X values for
"plus" variant are effectively shifted right by 1 bit, but the max wasn't
shifted too. This causes the driver to incorrectly handle "no data" cases,
which also interfered with how multi-touch was handled. To fix it - I created
new SS4 "plus" macros for the max value - SS4_PLUS_MFPACKET_NO_AX &
SS4_PLUS_MFPACKET_NO_AX_BL. To make the change a little more readable, I moved
also the Y-max lines so they are closer to the X-max lines.
To get three-finger tap to work both changes are required.
The included patch was generated against the mainline tree today, but was also
tested against the 4.14 kernel branch. I've included in this e-mail the people
involved with the old patch from August, plus Pali Rohár who is listed as the
ALPS PS/2 touchpad driver reviewer (in the maintainers file).
Fixes: 4a646580f793d19717f7e034c8d473b509c27d49 ("Input: ALPS - fix two-finger
scroll breakage in right side on ALPS touchpad")
Regards,
Nir
Signed-off-by: Nir Perry diff --git
a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c index
579b899..dbe57da 100644
--- a/drivers/input/mouse/alps.c
+++ b/drivers/input/mouse/alps.c
@@ -1250,29 +1250,32 @@ static int alps_decode_ss4_v2(struct alps_fields *f,
case SS4_PACKET_ID_MULTI:
if (priv->flags & ALPS_BUTTONPAD) {
if (IS_SS4PLUS_DEV(priv->dev_id)) {
- f->mt[0].x = SS4_PLUS_BTL_MF_X_V2(p, 0);
- f->mt[1].x = SS4_PLUS_BTL_MF_X_V2(p, 1);
+ f->mt[2].x = SS4_PLUS_BTL_MF_X_V2(p, 0);
+ f->mt[3].x = SS4_PLUS_BTL_MF_X_V2(p, 1);
+ no_data_x = SS4_PLUS_MFPACKET_NO_AX_BL;
} else {
f->mt[2].x = SS4_BTL_MF_X_V2(p, 0);
f->mt[3].x = SS4_BTL_MF_X_V2(p, 1);
+ no_data_x = SS4_MFPACKET_NO_AX_BL;
}
+ no_data_y = SS4_MFPACKET_NO_AY_BL;
f->mt[2].y = SS4_BTL_MF_Y_V2(p, 0);
f->mt[3].y = SS4_BTL_MF_Y_V2(p, 1);
- no_data_x = SS4_MFPACKET_NO_AX_BL;
- no_data_y = SS4_MFPACKET_NO_AY_BL;
} else {
if (IS_SS4PLUS_DEV(priv->dev_id)) {
- f->mt[0].x = SS4_PLUS_STD_MF_X_V2(p, 0);
- f->mt[1].x = SS4_PLUS_STD_MF_X_V2(p, 1);
+ f->mt[2].x = SS4_PLUS_STD_MF_X_V2(p, 0);
+ f->mt[3].x = SS4_PLUS_STD_MF_X_V2(p, 1);
+ no_data_x = SS4_PLUS_MFPACKET_NO_AX;
} else {
- f->mt[0].x = SS4_STD_MF_X_V2(p, 0);
- f->mt[1].x = SS4_STD_MF_X_V2(p, 1);
+ f->mt[2].x = SS4_STD_MF_X_V2(p, 0);
+ f->mt[3].x = SS4_STD_MF_X_V2(p, 1);
+ no_data_x = SS4_MFPACKET_NO_AX;
}
+ no_data_y = SS4_MFPACKET_NO_AY;
+
[PATCH] input: multi-touch fix for ALPS touchpads ("SS4 plus" variant)
Hi all,
I think a minor "typo" bug was accidentally introduced to ALPS
touchpad driver by a previous bug-fix (commit
4a646580f793d19717f7e034c8d473b509c27d49, "Input: ALPS - fix
two-finger scroll breakage in right side on ALPS touchpad").
It breaks how multi-touch events are decoded on some ALPS touchpads,
so for example tapping with three-fingers can no longer be used to
emulate middle-mouse-button (the kernel doesn't recognize this as the
proper event, and doesn't report it correctly to userspace).
This affects touchpads that use SS4 "plus" protocol variant, like
those found on Dell E7270 & E7470 laptops (tested on E7270).
The cause of the problem
--
First, probably due to a typo, the code in alps_decode_ss4_v2() for
case SS4_PACKET_ID_MULTI used inconsistent indices to "f->mt[]". You
can see 0 & 1 are used for the "if" part but 2 & 3 are used for the
"else" part, which I believe is a typo.
Second, in the previous patch, new macros were introduced to decode X
coordinates specific to the SS4 "plus" variant, but the macro to
define the maximum X value wasn't changed accordingly. The macros to
decode X values for "plus" variant are effectively shifted right by 1
bit, but the max wasn't shifted too. This causes the driver to
incorrectly handle "no data" cases, which also interfered with how
multi-touch was handled. To fix it - I created new SS4 "plus" macros
for the max value - SS4_PLUS_MFPACKET_NO_AX &
SS4_PLUS_MFPACKET_NO_AX_BL. To make the change a little more readable,
I moved also the Y-max lines so they are closer to the X-max lines.
To get three-finger tap to work both changes are required.
The included patch was generated against the mainline tree today, but
was also tested against the 4.14 kernel branch. I've included in this
e-mail the people involved with the old patch from August, plus Pali
Rohár who is listed as the ALPS PS/2 touchpad driver reviewer (in the
maintainers file).
Fixes: 4a646580f793d19717f7e034c8d473b509c27d49 ("Input: ALPS - fix
two-finger scroll breakage in right side on ALPS touchpad")
Regards,
Nir
Signed-off-by: Nir Perry
diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c
index 579b899..dbe57da 100644
--- a/drivers/input/mouse/alps.c
+++ b/drivers/input/mouse/alps.c
@@ -1250,29 +1250,32 @@ static int alps_decode_ss4_v2(struct alps_fields *f,
case SS4_PACKET_ID_MULTI:
if (priv->flags & ALPS_BUTTONPAD) {
if (IS_SS4PLUS_DEV(priv->dev_id)) {
- f->mt[0].x = SS4_PLUS_BTL_MF_X_V2(p, 0);
- f->mt[1].x = SS4_PLUS_BTL_MF_X_V2(p, 1);
+ f->mt[2].x = SS4_PLUS_BTL_MF_X_V2(p, 0);
+ f->mt[3].x = SS4_PLUS_BTL_MF_X_V2(p, 1);
+ no_data_x = SS4_PLUS_MFPACKET_NO_AX_BL;
} else {
f->mt[2].x = SS4_BTL_MF_X_V2(p, 0);
f->mt[3].x = SS4_BTL_MF_X_V2(p, 1);
+ no_data_x = SS4_MFPACKET_NO_AX_BL;
}
+ no_data_y = SS4_MFPACKET_NO_AY_BL;
f->mt[2].y = SS4_BTL_MF_Y_V2(p, 0);
f->mt[3].y = SS4_BTL_MF_Y_V2(p, 1);
- no_data_x = SS4_MFPACKET_NO_AX_BL;
- no_data_y = SS4_MFPACKET_NO_AY_BL;
} else {
if (IS_SS4PLUS_DEV(priv->dev_id)) {
- f->mt[0].x = SS4_PLUS_STD_MF_X_V2(p, 0);
- f->mt[1].x = SS4_PLUS_STD_MF_X_V2(p, 1);
+ f->mt[2].x = SS4_PLUS_STD_MF_X_V2(p, 0);
+ f->mt[3].x = SS4_PLUS_STD_MF_X_V2(p, 1);
+ no_data_x = SS4_PLUS_MFPACKET_NO_AX;
} else {
- f->mt[0].x = SS4_STD_MF_X_V2(p, 0);
- f->mt[1].x = SS4_STD_MF_X_V2(p, 1);
+ f->mt[2].x = SS4_STD_MF_X_V2(p, 0);
+ f->mt[3].x = SS4_STD_MF_X_V2(p, 1);
+ no_data_x = SS4_MFPACKET_NO_AX;
}
+ no_data_y = SS4_MFPACKET_NO_AY;
+
f->mt[2].y = SS4_STD_MF_Y_V2(p, 0);
f->mt[3].y = SS4_STD_MF_Y_V2(p, 1);
- no_data_x = SS4_MFPACKET_NO_AX;
- no_data_y = SS4_MFPACKET_NO_AY;
}
f->first_mp = 0;
diff --git a/drivers/input/mouse/alps.h b/drivers/input/mouse/alps.h
index c80a7c7..3dfae83 100644
--- a/drivers/input/mouse/alps.h
+++ b/drivers/input/mouse/alps.h
@@ -145,6 +145,8 @@ enum SS4_PACKET_ID {
#define SS4_MFPACKET_NO_AY 4080/* Y-Coordinate value */
#define SS4_MFPACKET_NO_AX_BL 8176/* Buttonless X-Coordinate value */
[PATCH] input: multi-touch fix for ALPS touchpads ("SS4 plus" variant)
Hi all,
I think a minor "typo" bug was accidentally introduced to ALPS
touchpad driver by a previous bug-fix (commit
4a646580f793d19717f7e034c8d473b509c27d49, "Input: ALPS - fix
two-finger scroll breakage in right side on ALPS touchpad").
It breaks how multi-touch events are decoded on some ALPS touchpads,
so for example tapping with three-fingers can no longer be used to
emulate middle-mouse-button (the kernel doesn't recognize this as the
proper event, and doesn't report it correctly to userspace).
This affects touchpads that use SS4 "plus" protocol variant, like
those found on Dell E7270 & E7470 laptops (tested on E7270).
The cause of the problem
--
First, probably due to a typo, the code in alps_decode_ss4_v2() for
case SS4_PACKET_ID_MULTI used inconsistent indices to "f->mt[]". You
can see 0 & 1 are used for the "if" part but 2 & 3 are used for the
"else" part, which I believe is a typo.
Second, in the previous patch, new macros were introduced to decode X
coordinates specific to the SS4 "plus" variant, but the macro to
define the maximum X value wasn't changed accordingly. The macros to
decode X values for "plus" variant are effectively shifted right by 1
bit, but the max wasn't shifted too. This causes the driver to
incorrectly handle "no data" cases, which also interfered with how
multi-touch was handled. To fix it - I created new SS4 "plus" macros
for the max value - SS4_PLUS_MFPACKET_NO_AX &
SS4_PLUS_MFPACKET_NO_AX_BL. To make the change a little more readable,
I moved also the Y-max lines so they are closer to the X-max lines.
To get three-finger tap to work both changes are required.
The included patch was generated against the mainline tree today, but
was also tested against the 4.14 kernel branch. I've included in this
e-mail the people involved with the old patch from August, plus Pali
Rohár who is listed as the ALPS PS/2 touchpad driver reviewer (in the
maintainers file).
Fixes: 4a646580f793d19717f7e034c8d473b509c27d49 ("Input: ALPS - fix
two-finger scroll breakage in right side on ALPS touchpad")
Regards,
Nir
Signed-off-by: Nir Perry
diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c
index 579b899..dbe57da 100644
--- a/drivers/input/mouse/alps.c
+++ b/drivers/input/mouse/alps.c
@@ -1250,29 +1250,32 @@ static int alps_decode_ss4_v2(struct alps_fields *f,
case SS4_PACKET_ID_MULTI:
if (priv->flags & ALPS_BUTTONPAD) {
if (IS_SS4PLUS_DEV(priv->dev_id)) {
- f->mt[0].x = SS4_PLUS_BTL_MF_X_V2(p, 0);
- f->mt[1].x = SS4_PLUS_BTL_MF_X_V2(p, 1);
+ f->mt[2].x = SS4_PLUS_BTL_MF_X_V2(p, 0);
+ f->mt[3].x = SS4_PLUS_BTL_MF_X_V2(p, 1);
+ no_data_x = SS4_PLUS_MFPACKET_NO_AX_BL;
} else {
f->mt[2].x = SS4_BTL_MF_X_V2(p, 0);
f->mt[3].x = SS4_BTL_MF_X_V2(p, 1);
+ no_data_x = SS4_MFPACKET_NO_AX_BL;
}
+ no_data_y = SS4_MFPACKET_NO_AY_BL;
f->mt[2].y = SS4_BTL_MF_Y_V2(p, 0);
f->mt[3].y = SS4_BTL_MF_Y_V2(p, 1);
- no_data_x = SS4_MFPACKET_NO_AX_BL;
- no_data_y = SS4_MFPACKET_NO_AY_BL;
} else {
if (IS_SS4PLUS_DEV(priv->dev_id)) {
- f->mt[0].x = SS4_PLUS_STD_MF_X_V2(p, 0);
- f->mt[1].x = SS4_PLUS_STD_MF_X_V2(p, 1);
+ f->mt[2].x = SS4_PLUS_STD_MF_X_V2(p, 0);
+ f->mt[3].x = SS4_PLUS_STD_MF_X_V2(p, 1);
+ no_data_x = SS4_PLUS_MFPACKET_NO_AX;
} else {
- f->mt[0].x = SS4_STD_MF_X_V2(p, 0);
- f->mt[1].x = SS4_STD_MF_X_V2(p, 1);
+ f->mt[2].x = SS4_STD_MF_X_V2(p, 0);
+ f->mt[3].x = SS4_STD_MF_X_V2(p, 1);
+ no_data_x = SS4_MFPACKET_NO_AX;
}
+ no_data_y = SS4_MFPACKET_NO_AY;
+
f->mt[2].y = SS4_STD_MF_Y_V2(p, 0);
f->mt[3].y = SS4_STD_MF_Y_V2(p, 1);
- no_data_x = SS4_MFPACKET_NO_AX;
- no_data_y = SS4_MFPACKET_NO_AY;
}
f->first_mp = 0;
diff --git a/drivers/input/mouse/alps.h b/drivers/input/mouse/alps.h
index c80a7c7..3dfae83 100644
--- a/drivers/input/mouse/alps.h
+++ b/drivers/input/mouse/alps.h
@@ -145,6 +145,8 @@ enum SS4_PACKET_ID {
#define SS4_MFPACKET_NO_AY 4080/* Y-Coordinate value */
#define SS4_MFPACKET_NO_AX_BL 8176/* Buttonless X-Coordinate value */
#define
