Re: [PATCH] input: handle bad parity PS/2 packets in mouse drivers better
From: Damjan Jovanovic
Date: Sat Apr 17 2010 - 05:33:43 EST
On Wed, Apr 14, 2010 at 10:59 PM, Dmitry Torokhov
<dmitry.torokhov@xxxxxxxxx> wrote:
> Hi Damjan,
>
> On Tue, Apr 13, 2010 at 01:29:20PM +0200, Damjan Jovanovic wrote:
>> This fixes a regression introduced in Linux 2.6.2 where mice that
>> sporadically produce bad parity go crazy and start jumping around and
>> clicking randomly, which never happens in any version of Windows
>> running on the same hardware. The bugzilla bug is
>> https://bugzilla.kernel.org/show_bug.cgi?id=6105
>>
>> The patch works by always accumulating a full PS/2 packet, then
>> ignoring the packet if any byte had a bad checksum. A month of testing
>> it against an affected mouse has revealed it works perfectly in
>> practice.
>>
>> This is the third resend, also CC'ed to lkml and Andrew Morton this
>> time, because the previous 2 emails to linux-input@xxxxxxxxxxxxxxx and
>> the input/mouse maintainers from 28 March 2010 were ignored.
>>
>
> I am not sure whether we can rely on the mouse and KBC combo to always
> finish transmitting entire packet when parity error is detected,
> regardless of the protocol involved. However I had a chanceto observe
> several versions of the $OTHER_OS in presence of parity errors and
> they (at least when used with stock mouse driver) appear to simply
> ignore errors and process motion data as usual. Therefore I propose
> instead of your patch the patch below.
>
> Could you please try it on your box and see if it helps?
>
> Thanks.
>
> --
> Dmitry
Hi Dmitry and others
This patch works.
Thank you
Damjan Jovanovic
>
> Input: psmouse - ignore parity error for basic protocols
>
> From: Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx>
>
> Observing behavior of the other OS it appears that parity errors reported
> by the keyboard controller are being ignored and the data is processed
> as usual. Let's do the same for standard PS/2 protocols (bare, Intellimouse
> and Intellimouse Explorer) to provide better compatibility. Thsi should fix
> teh following bug:
>
> https://bugzilla.kernel.org/show_bug.cgi?id=6105
>
> Thanks for Damjan Jovanovic for locating the source of issue and ideas
> for the patch.
>
> Signed-off-by: Dmitry Torokhov <dtor@xxxxxxx>
> ---
>
> drivers/input/mouse/psmouse-base.c | 18 +++++++++++++++---
> drivers/input/mouse/psmouse.h | 1 +
> 2 files changed, 16 insertions(+), 3 deletions(-)
>
>
> diff --git a/drivers/input/mouse/psmouse-base.c b/drivers/input/mouse/psmouse-base.c
> index d8c0c8d..cbc8072 100644
> --- a/drivers/input/mouse/psmouse-base.c
> +++ b/drivers/input/mouse/psmouse-base.c
> @@ -110,6 +110,7 @@ static struct workqueue_struct *kpsmoused_wq;
> struct psmouse_protocol {
> enum psmouse_type type;
> bool maxproto;
> + bool ignore_parity; /* Protocol should ignore parity errors from KBC */
> const char *name;
> const char *alias;
> int (*detect)(struct psmouse *, bool);
> @@ -288,7 +289,9 @@ static irqreturn_t psmouse_interrupt(struct serio *serio,
> if (psmouse->state == PSMOUSE_IGNORE)
> goto out;
>
> - if (flags & (SERIO_PARITY|SERIO_TIMEOUT)) {
> + if (unlikely((flags & SERIO_TIMEOUT) ||
> + ((flags & SERIO_PARITY) && !psmouse->ignore_parity))) {
> +
> if (psmouse->state == PSMOUSE_ACTIVATED)
> printk(KERN_WARNING "psmouse.c: bad data from KBC -%s%s\n",
> flags & SERIO_TIMEOUT ? " timeout" : "",
> @@ -759,6 +762,7 @@ static const struct psmouse_protocol psmouse_protocols[] = {
> .name = "PS/2",
> .alias = "bare",
> .maxproto = true,
> + .ignore_parity = true,
> .detect = ps2bare_detect,
> },
> #ifdef CONFIG_MOUSE_PS2_LOGIPS2PP
> @@ -786,6 +790,7 @@ static const struct psmouse_protocol psmouse_protocols[] = {
> .name = "ImPS/2",
> .alias = "imps",
> .maxproto = true,
> + .ignore_parity = true,
> .detect = intellimouse_detect,
> },
> {
> @@ -793,6 +798,7 @@ static const struct psmouse_protocol psmouse_protocols[] = {
> .name = "ImExPS/2",
> .alias = "exps",
> .maxproto = true,
> + .ignore_parity = true,
> .detect = im_explorer_detect,
> },
> #ifdef CONFIG_MOUSE_PS2_SYNAPTICS
> @@ -1222,6 +1228,7 @@ static void psmouse_disconnect(struct serio *serio)
> static int psmouse_switch_protocol(struct psmouse *psmouse,
> const struct psmouse_protocol *proto)
> {
> + const struct psmouse_protocol *selected_proto;
> struct input_dev *input_dev = psmouse->dev;
>
> input_dev->dev.parent = &psmouse->ps2dev.serio->dev;
> @@ -1245,9 +1252,14 @@ static int psmouse_switch_protocol(struct psmouse *psmouse,
> return -1;
>
> psmouse->type = proto->type;
> - } else
> + selected_proto = proto;
> + } else {
> psmouse->type = psmouse_extensions(psmouse,
> psmouse_max_proto, true);
> + selected_proto = psmouse_protocol_by_type(psmouse->type);
> + }
> +
> + psmouse->ignore_parity = selected_proto->ignore_parity;
>
> /*
> * If mouse's packet size is 3 there is no point in polling the
> @@ -1267,7 +1279,7 @@ static int psmouse_switch_protocol(struct psmouse *psmouse,
> psmouse->resync_time = 0;
>
> snprintf(psmouse->devname, sizeof(psmouse->devname), "%s %s %s",
> - psmouse_protocol_by_type(psmouse->type)->name, psmouse->vendor, psmouse->name);
> + selected_proto->name, psmouse->vendor, psmouse->name);
>
> input_dev->name = psmouse->devname;
> input_dev->phys = psmouse->phys;
> diff --git a/drivers/input/mouse/psmouse.h b/drivers/input/mouse/psmouse.h
> index e053bdd..593e910 100644
> --- a/drivers/input/mouse/psmouse.h
> +++ b/drivers/input/mouse/psmouse.h
> @@ -47,6 +47,7 @@ struct psmouse {
> unsigned char pktcnt;
> unsigned char pktsize;
> unsigned char type;
> + bool ignore_parity;
> bool acks_disable_command;
> unsigned int model;
> unsigned long last;
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/