From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
To: "Pali Rohár" <pali.rohar@gmail.com>
Cc: Hans de Goede <hdegoede@redhat.com>,
Yunkang Tang <yunkang.tang@cn.alps.com>,
Tommy Will <tommywill2011@gmail.com>,
linux-input@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3 1/4] input: alps: Do not try to parse data as 3 bytes packet when driver is out of sync
Date: Sat, 8 Nov 2014 12:52:47 -0800 [thread overview]
Message-ID: <20141108205247.GB40319@dtor-ws> (raw)
In-Reply-To: <1414884310-19842-2-git-send-email-pali.rohar@gmail.com>
On Sun, Nov 02, 2014 at 12:25:07AM +0100, Pali Rohár wrote:
> 5th and 6th byte of ALPS trackstick V3 protocol match condition for first byte
> of PS/2 3 bytes packet. When driver enters out of sync state and ALPS trackstick
> is sending data then driver match 5th, 6th and next 1st bytes as PS/2.
>
> It basically means if user is using trackstick when driver is in out of sync
> state driver will never resync. Processing these bytes as 3 bytes PS/2 data
> cause total mess (random cursor movements, random clicks) and make trackstick
> unusable until psmouse driver decide to do full device reset.
>
> Lot of users reported problems with ALPS devices on Dell Latitude E6440, E6540
> and E7440 laptops. ALPS device or Dell EC for unknown reason send some invalid
> ALPS PS/2 bytes which cause driver out of sync. It looks like that i8042 and
> psmouse/alps driver always receive group of 6 bytes packets so there are no
> missing bytes and no bytes were inserted between valid once.
>
> This patch does not fix root of problem with ALPS devices found in Dell Latitude
> laptops but it does not allow to process some (invalid) subsequence of 6 bytes
> ALPS packets as 3 bytes PS/2 when driver is out of sync.
>
> So with this patch trackstick input device does not report bogus data when
> also driver is out of sync, so trackstick should be usable on those machines.
>
> Unknown is also information which ALPS devices send 3 bytes packets and why
> ALPS driver needs to handle also bare PS/2 packets. According to git (and plus
> historic tree from bitkeeper) code for processing 3 bytes bare PS/2 packets
> is there since first version of alps.c existence (since 2.6.9-rc2).
I believe it was to handle external devices plugged into PS/2 ports on
Dell laptops (maybe?). Dell laptops do not use active multiplexing
controller so everything comes mixed into single data stream.
>
> We do not want to break some older ALPS devices. And disabling processing bare
> PS/2 packets when driver is out of sync should not break it.
>
> Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
> Tested-by: Pali Rohár <pali.rohar@gmail.com>
> Cc: stable@vger.kernel.org
Applied, thank you.
> ---
> drivers/input/mouse/alps.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c
> index 2b0ae8c..a772745 100644
> --- a/drivers/input/mouse/alps.c
> +++ b/drivers/input/mouse/alps.c
> @@ -1156,7 +1156,9 @@ static psmouse_ret_t alps_process_byte(struct psmouse *psmouse)
> {
> struct alps_data *priv = psmouse->private;
>
> - if ((psmouse->packet[0] & 0xc8) == 0x08) { /* PS/2 packet */
> + /* FIXME: Could we receive bare PS/2 packets from DualPoint devices?? */
> + if (!psmouse->out_of_sync_cnt &&
> + (psmouse->packet[0] & 0xc8) == 0x08) { /* PS/2 packet */
> if (psmouse->pktcnt == 3) {
> alps_report_bare_ps2_packet(psmouse, psmouse->packet,
> true);
> --
> 1.7.9.5
>
--
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
WARNING: multiple messages have this Message-ID (diff)
From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
To: "Pali Rohár" <pali.rohar@gmail.com>
Cc: Hans de Goede <hdegoede@redhat.com>,
Yunkang Tang <yunkang.tang@cn.alps.com>,
Tommy Will <tommywill2011@gmail.com>,
linux-input@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3 1/4] input: alps: Do not try to parse data as 3 bytes packet when driver is out of sync
Date: Sat, 8 Nov 2014 12:52:47 -0800 [thread overview]
Message-ID: <20141108205247.GB40319@dtor-ws> (raw)
In-Reply-To: <1414884310-19842-2-git-send-email-pali.rohar@gmail.com>
On Sun, Nov 02, 2014 at 12:25:07AM +0100, Pali Rohár wrote:
> 5th and 6th byte of ALPS trackstick V3 protocol match condition for first byte
> of PS/2 3 bytes packet. When driver enters out of sync state and ALPS trackstick
> is sending data then driver match 5th, 6th and next 1st bytes as PS/2.
>
> It basically means if user is using trackstick when driver is in out of sync
> state driver will never resync. Processing these bytes as 3 bytes PS/2 data
> cause total mess (random cursor movements, random clicks) and make trackstick
> unusable until psmouse driver decide to do full device reset.
>
> Lot of users reported problems with ALPS devices on Dell Latitude E6440, E6540
> and E7440 laptops. ALPS device or Dell EC for unknown reason send some invalid
> ALPS PS/2 bytes which cause driver out of sync. It looks like that i8042 and
> psmouse/alps driver always receive group of 6 bytes packets so there are no
> missing bytes and no bytes were inserted between valid once.
>
> This patch does not fix root of problem with ALPS devices found in Dell Latitude
> laptops but it does not allow to process some (invalid) subsequence of 6 bytes
> ALPS packets as 3 bytes PS/2 when driver is out of sync.
>
> So with this patch trackstick input device does not report bogus data when
> also driver is out of sync, so trackstick should be usable on those machines.
>
> Unknown is also information which ALPS devices send 3 bytes packets and why
> ALPS driver needs to handle also bare PS/2 packets. According to git (and plus
> historic tree from bitkeeper) code for processing 3 bytes bare PS/2 packets
> is there since first version of alps.c existence (since 2.6.9-rc2).
I believe it was to handle external devices plugged into PS/2 ports on
Dell laptops (maybe?). Dell laptops do not use active multiplexing
controller so everything comes mixed into single data stream.
>
> We do not want to break some older ALPS devices. And disabling processing bare
> PS/2 packets when driver is out of sync should not break it.
>
> Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
> Tested-by: Pali Rohár <pali.rohar@gmail.com>
> Cc: stable@vger.kernel.org
Applied, thank you.
> ---
> drivers/input/mouse/alps.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c
> index 2b0ae8c..a772745 100644
> --- a/drivers/input/mouse/alps.c
> +++ b/drivers/input/mouse/alps.c
> @@ -1156,7 +1156,9 @@ static psmouse_ret_t alps_process_byte(struct psmouse *psmouse)
> {
> struct alps_data *priv = psmouse->private;
>
> - if ((psmouse->packet[0] & 0xc8) == 0x08) { /* PS/2 packet */
> + /* FIXME: Could we receive bare PS/2 packets from DualPoint devices?? */
> + if (!psmouse->out_of_sync_cnt &&
> + (psmouse->packet[0] & 0xc8) == 0x08) { /* PS/2 packet */
> if (psmouse->pktcnt == 3) {
> alps_report_bare_ps2_packet(psmouse, psmouse->packet,
> true);
> --
> 1.7.9.5
>
--
Dmitry
next prev parent reply other threads:[~2014-11-08 20:52 UTC|newest]
Thread overview: 64+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-10-03 9:43 [PATCH 0/3] input: alps: Fixes for ALPS driver Pali Rohár
2014-10-03 9:43 ` Pali Rohár
2014-10-03 9:43 ` [PATCH 1/3] input: alps: Reset mouse before identifying it Pali Rohár
2014-10-03 9:47 ` Hans de Goede
2014-10-03 9:47 ` Hans de Goede
2014-10-14 6:08 ` Dmitry Torokhov
2014-10-14 6:08 ` Dmitry Torokhov
2014-10-15 12:53 ` Pali Rohár
2014-10-15 17:43 ` Dmitry Torokhov
2014-10-15 17:43 ` Dmitry Torokhov
2014-10-15 17:57 ` Pali Rohár
2014-10-15 18:00 ` Dmitry Torokhov
2014-10-15 18:00 ` Dmitry Torokhov
2014-10-15 18:10 ` Pali Rohár
2014-10-15 18:22 ` Dmitry Torokhov
2014-10-15 18:22 ` Dmitry Torokhov
2014-10-19 11:07 ` Pali Rohár
2014-10-23 15:44 ` Dmitry Torokhov
2014-10-23 15:44 ` Dmitry Torokhov
2014-11-01 23:29 ` Pali Rohár
2014-10-03 9:43 ` [PATCH 2/3] input: alps: For protocol V3, do not process data when last packet's bit7 is set Pali Rohár
2014-10-03 9:51 ` Hans de Goede
2014-10-03 9:51 ` Hans de Goede
2014-10-03 9:58 ` Pali Rohár
2014-10-03 10:01 ` Hans de Goede
2014-10-03 10:01 ` Hans de Goede
2014-10-03 9:43 ` [PATCH 3/3] input: alps: Reset mouse and ALPS driver immediately after first invalid packet Pali Rohár
2014-10-03 9:43 ` Pali Rohár
2014-10-03 9:55 ` Hans de Goede
2014-10-03 9:55 ` Hans de Goede
2014-10-03 10:05 ` Pali Rohár
2014-10-03 10:18 ` Hans de Goede
2014-10-03 10:18 ` Hans de Goede
2014-10-03 10:23 ` Pali Rohár
2014-10-03 11:03 ` Hans de Goede
2014-10-03 11:03 ` Hans de Goede
2014-10-03 12:04 ` Hans de Goede
2014-10-03 12:04 ` Hans de Goede
2014-11-01 23:25 ` [PATCH v3 0/4] Fixes for ALPS driver Pali Rohár
2014-11-01 23:25 ` Pali Rohár
2014-11-01 23:25 ` [PATCH v3 1/4] input: alps: Do not try to parse data as 3 bytes packet when driver is out of sync Pali Rohár
2014-11-01 23:25 ` Pali Rohár
2014-11-08 20:52 ` Dmitry Torokhov [this message]
2014-11-08 20:52 ` Dmitry Torokhov
2014-11-01 23:25 ` [PATCH v3 2/4] input: alps: Allow 2 invalid packets without resetting device Pali Rohár
2014-11-08 21:00 ` Dmitry Torokhov
2014-11-08 21:00 ` Dmitry Torokhov
2014-11-01 23:25 ` [PATCH v3 3/4] input: alps: For protocol V3, do not process data when last packet's bit7 is set Pali Rohár
2014-11-01 23:25 ` Pali Rohár
2014-11-09 7:50 ` Dmitry Torokhov
2014-11-09 7:50 ` Dmitry Torokhov
2014-11-09 11:22 ` Pali Rohár
2014-11-09 20:34 ` Dmitry Torokhov
2014-11-09 20:34 ` Dmitry Torokhov
2014-11-10 9:18 ` Pali Rohár
2014-11-01 23:25 ` [PATCH v3 4/4] input: alps: Fix trackstick detection Pali Rohár
2014-11-01 23:25 ` Pali Rohár
2014-11-09 8:05 ` Dmitry Torokhov
2014-11-09 11:30 ` Pali Rohár
2014-11-14 11:22 ` Pali Rohár
2014-11-14 19:41 ` Pali Rohár
2014-11-02 14:14 ` [PATCH v3 0/4] Fixes for ALPS driver Hans de Goede
2014-11-02 14:14 ` Hans de Goede
2014-11-06 17:46 ` Pali Rohár
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20141108205247.GB40319@dtor-ws \
--to=dmitry.torokhov@gmail.com \
--cc=hdegoede@redhat.com \
--cc=linux-input@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=pali.rohar@gmail.com \
--cc=tommywill2011@gmail.com \
--cc=yunkang.tang@cn.alps.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.