Re: Another keyboard woes with 2.6.0...

[Andries Brouwer]

On Tue, Oct 14, 2003 at 08:16:06PM +0200, Petr Vandrovec wrote:

> Got it again. This time with detailed logging.

Excellent. This immediately shows another bug in the code.

> Oct 14 19:59:18 ppc kernel: i8042.c: e0 <- i8042 (interrupt, kbd, 1) [30115341]
> Oct 14 19:59:18 ppc kernel: i8042.c: ed -> i8042 (kbd-data) [30115342]
> Oct 14 19:59:18 ppc kernel: i8042.c: fa <- i8042 (interrupt, kbd, 1) [30115346]
> Oct 14 19:59:18 ppc kernel: atkbd.c: Unknown key released (translated set 2, code 0x165, data 0xfa, on isa0060/serio0).

The code (some version of 2.6.0-test6) says

	if (atkbd->translated) do {
		if (atkbd->emul != 1) {
			if (code == ATKBD_RET_ACK)
				break;
			...
		}
		if (code < 0x80) {
			code = atkbd_unxlate_table[code];
			break;
		}
		code = atkbd_unxlate_table[code & 0x7f];
	}

Here an e0 preceded, setting the atkbd->emul flag.
Now the acknowledge for the 0xed command was not recognized as
ATKBD_RET_ACK and untranslated as if it were a keystroke.

Yes, I hope to convince Vojtech that untranslating is evil.

The question is: what to do with a protocol scancode?
My answer from long ago was: view it as a protocol scancode only
when it is expected - after we send a command we expect an ACK.
And indeed, 2.4 still has

	if (reply_expected) {
		if (scancode == KBD_REPLY_ACK) {
			acknowledge = 1;
			reply_expected = 0;
	...

That is, the right way, or at least the way that worked since 1.1.54,
is to test only for KBD_REPLY_ACK when we just sent something.

The wrong solution follows below (not compiled or tested):

--- atkbd.c~    Mon Sep 29 09:12:26 2003
+++ atkbd.c     Tue Oct 14 23:15:57 2003
@@ -183,11 +183,19 @@
                atkbd->resend = 0;
 #endif
 
+       switch (code) {
+               case ATKBD_RET_ACK:
+                       atkbd->ack = 1;
+                       goto out;
+               case ATKBD_RET_NAK:
+                       atkbd->ack = -1;
+                       goto out;
+       }
+
        if (atkbd->translated) do {
 
                if (atkbd->emul != 1) {
-                       if (code == ATKBD_RET_EMUL0 || code == ATKBD_RET_EMUL1 ||
-                           code == ATKBD_RET_ACK || code == ATKBD_RET_NAK)
+                       if (code == ATKBD_RET_EMUL0 || code == ATKBD_RET_EMUL1)
                                break;
                        if (code == ATKBD_RET_BAT) {
                                if (!atkbd->bat_xl)
@@ -211,15 +219,6 @@
 
        } while (0);
 
-       switch (code) {
-               case ATKBD_RET_ACK:
-                       atkbd->ack = 1;
-                       goto out;
-               case ATKBD_RET_NAK:
-                       atkbd->ack = -1;
-                       goto out;
-       }
-
        if (atkbd->cmdcnt) {
                atkbd->cmdbuf[--atkbd->cmdcnt] = code;
                goto out;

(This is right for the great majority of people that does not
have fa,fe occur as non-protocol scancodes. In rare cases
some more surgery is needed. Left to Vojtech.)

Andries


[patch against some source similar to 2.6.0-test6]

-
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/