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 [email protected] and
the input/mouse maintainers from 28 March 2010 were ignored.
Regards
Damjan Jovanovic
Signed-off-by: Damjan Jovanovic <[email protected]>
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 [email protected] 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
Input: psmouse - ignore parity error for basic protocols
From: Dmitry Torokhov <[email protected]>
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 <[email protected]>
---
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;
On Wed, Apr 14, 2010 at 10:59 PM, Dmitry Torokhov
<[email protected]> 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 [email protected] 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 <[email protected]>
>
> 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 <[email protected]>
> ---
>
> ?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;
>