From: Andries.Brouwer@cwi.nl

I just checked my old keyboard code, looking at pc_keyb.c here,
and the resend variable is used only to react to keyboard controller
requests for resend. There is no reaction to a timeout or parity
error from the keyboard controller, other than to throw the data
byte out as invalid.

In the new code we do
   serio_write(serio, ATKBD_CMD_RESEND);
when an error occurs.

I think that is bad for several reasons.
(i) It crashes the kernel with interrupt within interrupt :-)
(ii) Retrying is almost always bad.
Retrying is meaningful at the very lowest level.
With usb-storage devices I have seen the kernel retrying for
a quarter of an hour because all layers did a dozen or so retries
on error, and the total number of retries grew exponentially with
the number of layers involved.
(iii) My docs on the PS/2 keyboard controller say:
"the 0xfe resend command to the keyboard is meant for use by
the keyboard controller; it is not used by the BIOS".
(iv) You send the same commands to any connected device.
But sending 0xfe to a non-keyboard, e.g. a (multiplexed) mouse,
may provoke undefined behaviour, even hangs.

So, I would propose to remove the resend field from struct atkbd,
and to replace

    if ((flags & (SERIO_FRAME | SERIO_PARITY)) && (~flags & SERIO_TIMEOUT) \
         && !atkbd->resend && atkbd->write) {
            printk("atkbd.c: frame/parity error: %02x\n", flags);
            serio_write(serio, ATKBD_CMD_RESEND);
            atkbd->resend = 1;
            goto out;
    }

    if (!flags)
            atkbd->resend = 0;

by something like

    if ((flags & (SERIO_FRAME | SERIO_PARITY)) && (~flags & SERIO_TIMEOUT)) {
	    printk("atkbd.c: frame/parity error: %02x\n", flags);
	    goto out;
    }

I seem to recall that I had to #ifdef out the printk in the end,
because too many people had bad keyboards, or switches that produced
some garbage at a switchover.
We can leave it for the moment, for debugging purposes, or give the
message at most five times or so.



 drivers/input/keyboard/atkbd.c |   17 +++++++----------
 1 files changed, 7 insertions(+), 10 deletions(-)

diff -puN drivers/input/keyboard/atkbd.c~keyboard-resend-fix drivers/input/keyboard/atkbd.c
--- 25/drivers/input/keyboard/atkbd.c~keyboard-resend-fix	2003-07-30 11:35:37.000000000 -0700
+++ 25-akpm/drivers/input/keyboard/atkbd.c	2003-07-30 11:35:37.000000000 -0700
@@ -34,7 +34,7 @@ static int atkbd_reset = 1;
 
 /*
  * Scancode to keycode tables. These are just the default setting, and
- * are loadable via an userland utility.
+ * are loadable via a userland utility.
  */
 
 static unsigned char atkbd_set2_keycode[512] = {
@@ -124,7 +124,6 @@ struct atkbd {
 	unsigned char emul;
 	unsigned short id;
 	unsigned char write;
-	unsigned char resend;
 };
 
 /*
@@ -142,16 +141,12 @@ static irqreturn_t atkbd_interrupt(struc
 	printk(KERN_DEBUG "atkbd.c: Received %02x flags %02x\n", data, flags);
 #endif
 
-	if ((flags & (SERIO_FRAME | SERIO_PARITY)) && (~flags & SERIO_TIMEOUT) && !atkbd->resend && atkbd->write) {
+	if ((flags & (SERIO_FRAME | SERIO_PARITY)) &&
+	    (~flags & SERIO_TIMEOUT)) {
 		printk("atkbd.c: frame/parity error: %02x\n", flags);
-		serio_write(serio, ATKBD_CMD_RESEND);
-		atkbd->resend = 1;
 		goto out;
 	}
 	
-	if (!flags)
-		atkbd->resend = 0;
-
 	switch (code) {
 		case ATKBD_RET_ACK:
 			atkbd->ack = 1;
@@ -214,7 +209,7 @@ out:
 
 static int atkbd_sendbyte(struct atkbd *atkbd, unsigned char byte)
 {
-	int timeout = 10000; /* 100 msec */
+	int timeout = 20000; /* 200 msec */
 	atkbd->ack = 0;
 
 #ifdef ATKBD_DEBUG
@@ -399,7 +394,9 @@ static int atkbd_probe(struct atkbd *atk
 
 	if (atkbd_reset)
 		if (atkbd_command(atkbd, NULL, ATKBD_CMD_RESET_BAT)) 
-			printk(KERN_WARNING "atkbd.c: keyboard reset failed on %s\n", atkbd->serio->phys);
+			printk(KERN_WARNING
+			       "atkbd.c: keyboard reset failed on %s\n",
+			       atkbd->serio->phys);
 
 /*
  * Then we check the keyboard ID. We should get 0xab83 under normal conditions.

_