linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Re: Re:Re: Linux 2.6.0-test4
@ 2003-08-31 16:22 Chris Heath
  2003-09-01 16:01 ` Ralf Hildebrandt
  2003-09-02  8:07 ` Ralf Hildebrandt
  0 siblings, 2 replies; 21+ messages in thread
From: Chris Heath @ 2003-08-31 16:22 UTC (permalink / raw)
  To: Ralf.Hildebrandt; +Cc: linux-kernel

> Aug 27 18:53:41 hummus2 kernel: atkbd.c: Unknown key (set 2, scancode 0x9d, on isa0060/serio0) pressed.
> Aug 27 19:15:14 hummus2 kernel: atkbd.c: Unknown key (set 2, scancode 0xb9, on isa0060/serio0) pressed.
> Aug 27 19:42:50 hummus2 kernel: atkbd.c: Unknown key (set 2, scancode 0x9d, on isa0060/serio0) pressed.
> Aug 28 10:14:14 hummus2 kernel: atkbd.c: Unknown key (set 2, scancode 0x9d, on isa0060/serio0) pressed.
> 
> Basically, CTRL was stuck. Even when I switched to X11.

Well, this completely baffles me.  I thought X11 maintains its own
keydown array.

Anyway, I've included a patch that should hopefully give us better
debugging information.  When you get an unknown key error, it will also
dump the last 16 bytes that were sent from the keyboard.  Be careful
with this one.  If you post any errors to the list, make sure it doesn't
contain any sensitive passwords. :-)

Chris


--- a/drivers/input/serio/i8042.c	2003-08-09 11:58:10.000000000 -0400
+++ b/drivers/input/serio/i8042.c	2003-08-31 10:16:55.000000000 -0400
@@ -62,6 +62,7 @@
 static unsigned char i8042_last_release;
 static unsigned char i8042_mux_open;
 struct timer_list i8042_timer;
+unsigned char i8042_history[16];
 
 /*
  * Shared IRQ's require a device pointer, but this driver doesn't support
@@ -334,6 +335,14 @@
 static char i8042_mux_short[4][16];
 static char i8042_mux_phys[4][32];
 
+void dump_i8042_history(void) {
+	int i;
+	printk(KERN_WARNING "i8042 history: ");
+	for (i=0; i<sizeof(i8042_history); i++)
+		printk("%02x ", i8042_history[i]);
+	printk("\n");
+}
+
 /*
  * i8042_interrupt() is the most important function in this driver -
  * it handles the interrupts from the i8042, and sends incoming bytes
@@ -405,6 +414,8 @@
 			continue;
 		}
 
+		memmove(i8042_history, &i8042_history[1], sizeof(i8042_history)-1);
+		i8042_history[sizeof(i8042_history)-1] = data;
 		if (data > 0x7f) {
 			unsigned char index = (data & 0x7f) | (i8042_last_e0 << 7);
 			/* work around hardware that doubles key releases */
--- a/drivers/input/keyboard/atkbd.c	2003-06-22 18:45:06.000000000 -0400
+++ b/drivers/input/keyboard/atkbd.c	2003-08-31 10:11:51.000000000 -0400
@@ -131,6 +131,7 @@
  * atkbd_interrupt(). Here takes place processing of data received from
  * the keyboard into events.
  */
+void dump_i8042_history(void);
 
 static irqreturn_t atkbd_interrupt(struct serio *serio, unsigned char data,
 			unsigned int flags, struct pt_regs *regs)
@@ -193,6 +194,7 @@
 		case ATKBD_KEY_UNKNOWN:
 			printk(KERN_WARNING "atkbd.c: Unknown key (set %d, scancode %#x, on %s) %s.\n",
 				atkbd->set, code, serio->phys, atkbd->release ? "released" : "pressed");
+			dump_i8042_history();
 			break;
 		default:
 			input_regs(&atkbd->dev, regs);


^ permalink raw reply	[flat|nested] 21+ messages in thread
* Re: keyboard - was: Re: Linux 2.6.0-test4
@ 2003-09-04  4:09 Chris Heath
  2003-09-04 20:48 ` Jamie Lokier
  0 siblings, 1 reply; 21+ messages in thread
From: Chris Heath @ 2003-09-04  4:09 UTC (permalink / raw)
  To: aebr; +Cc: linux-kernel, vojtech, Ralf Hildebrandt

> > Right now, my CTRL key is totally "stuck" on the fbconsole. I can't
> > release it, not even by switching between the consoles and/or X11.
> > 
> > > > atkbd.c: Unknown key (set 2, scancode 0x9c, on isa0060/serio0) pressed.
> > > > i8042 history: e0 d0 1c 9c 2e ae 10 90 e0 50 e0 d0 e0 d0 1c 9c
> 
> Well, that shows that this particular problem was solved, but there are
> more problems. No doubt we'll understand everything eventually.

Quite right.  When we get a repeated key release, say e0 d0 e0 d0, we
were ignoring the last d0, whereas actually we should have ignored the
second e0 too.  As you can imagine, this caused stuck CTRL keys among
other things.

> > But now I don't get any messages like the one below (yes, the special
> > code generating this output is still active...)

Since then, Ralf has reported to me in a private email that he got the
following (rather troubling) error:

Sep  2 14:43:51 hummus2 kernel: atkbd.c: Unknown key (set 2, scancode 0xb2, on isa0060/serio0) pressed.
Sep  2 14:43:51 hummus2 kernel: i8042 history: e0 48 e0 c8 22 a2 13 93 93 17 97 32 31 b2 b1 b2

So it seems it is occasionally possible to have a repeated key release
(b2) with another key release (b1) in between.  Ouch!

The patch below is supposed to solve both these problems.  It makes some 
assumptions, which I have listed below.  (It probably wouldn't be hard to 
remove any of these assumptions, but I don't feel like hacking this
code any more than I have to.)

1. I assume it is OK to defer processing the code e0 until the following
byte arrives.  Are there any keyboards out there with e0 in the keyboard
ID?  This could break the atkbd probe for such keyboards.  (The
alternative would be to pass the repeated key releases through to atkbd,
and let that layer remove the duplicates.)

2. The event that appears between two repeated key releases is always a
key release and not a key press.

3. There will only be at most one key event in between two repeated key 
releases.

Chris


--- linux-2.6.0-test4-bk5/drivers/input/serio/i8042.c	2003-08-09 11:58:10.000000000 -0400
+++ linux-2.6.0-test4-cdh1/drivers/input/serio/i8042.c	2003-09-03 22:30:52.000000000 -0400
@@ -59,7 +59,7 @@
 static unsigned char i8042_initial_ctr;
 static unsigned char i8042_ctr;
 static unsigned char i8042_last_e0;
-static unsigned char i8042_last_release;
+static unsigned char i8042_last_release[2];
 static unsigned char i8042_mux_open;
 struct timer_list i8042_timer;
 
@@ -343,7 +343,7 @@
 static irqreturn_t i8042_interrupt(int irq, void *dev_id, struct pt_regs *regs)
 {
 	unsigned long flags;
-	unsigned char str, data;
+	unsigned char str, data, index;
 	unsigned int dfl;
 	struct {
 		int data;
@@ -405,30 +405,39 @@
 			continue;
 		}
 
+		index = (data & 0x7f) | (i8042_last_e0 << 7);
+
+		/* work around hardware that doubles key releases */
+		if (data > 0x7f && (index == i8042_last_release[0]
+				|| index == i8042_last_release[1])) {
+			dbg("i8042 skipped double release (%d)\n", index);
+			i8042_last_e0 = 0;
+			continue;
+		}
+
+		if (i8042_last_e0)
+			serio_interrupt(&i8042_kbd_port, 0xe0, dfl, regs);
 		if (data > 0x7f) {
-			unsigned char index = (data & 0x7f) | (i8042_last_e0 << 7);
-			/* work around hardware that doubles key releases */
-			if (index == i8042_last_release) {
-				dbg("i8042 skipped double release (%d)\n", index);
-				i8042_last_e0 = 0;
-				continue;
-			}
 			if (index == 0xaa || index == 0xb6)
 				set_bit(index, i8042_unxlate_seen);
 			if (test_and_clear_bit(index, i8042_unxlate_seen)) {
 				serio_interrupt(&i8042_kbd_port, 0xf0, dfl, regs);
 				data = i8042_unxlate_table[data & 0x7f];
-				i8042_last_release = index;
+				i8042_last_release[1] = i8042_last_release[0];
+				i8042_last_release[0] = index;
 			}
 		} else {
-			set_bit(data | (i8042_last_e0 << 7), i8042_unxlate_seen);
+			set_bit(index, i8042_unxlate_seen);
 			data = i8042_unxlate_table[data];
-			i8042_last_release = 0;
+			i8042_last_release[1] = 0;
+			i8042_last_release[0] = 0;
 		}
 
 		i8042_last_e0 = (data == 0xe0);
 
-		serio_interrupt(&i8042_kbd_port, data, dfl, regs);
+		/* defer sending e0 in case it's part of a double key release */
+		if (!i8042_last_e0)
+			serio_interrupt(&i8042_kbd_port, data, dfl, regs);
 	}
 
 	/* FIXME - was it really ours? */


^ permalink raw reply	[flat|nested] 21+ messages in thread
* Re: keyboard - was: Re: Linux 2.6.0-test4
@ 2003-09-05 13:46 Norman Diamond
  0 siblings, 0 replies; 21+ messages in thread
From: Norman Diamond @ 2003-09-05 13:46 UTC (permalink / raw)
  To: linux-kernel

Although I don't have time to keep up with the list, I saw this posting from
"Chris Heath" <chris@heathens.co.nz>.  Reordered by importance:

> At this late stage, I don't think it is a good idea to completely
> rewrite the untranslate algorithm.  So we continue to hack it and hack
> it until it works.  :-/

Surely not.  Some keyboards which worked since 2.0 and probably 1.something
are broken in 2.6.  Other keyboards which worked before 2.2.something
(without USB drivers but with BIOS emulation) and resumed working since
2.4.something (with fixed USB dirvers) are broken in 2.6.  Haven't some
lessons been learned from a 2.4.something-dontuse and a few other
2.4.somethings which also should have been -dontuse?  If 2.6.0-dontuse gets
released, Linux will really be as bad as some other famous operating
systems.  Surely it is better to rewrite the untranslate algorithm.

Of course there's some power management and other problems which are in the
same situation.  The keyboard is not the only reason why it would be foolish
to release 2.6.0 before it starts working.

> However, the bytes that come from the i8042 are a mixture of Set 1 and
> Set 2.  Set 1 because the key releases have their 8th bits set, and Set
> 2 because we get the non-XT keys escaped with E0.

I wonder if it's really that simple.  Though today I experimented on a
desktop machine which might have a real i8042 maybe.  Under a combination of
2.6.0-test4 and X11, showkey -s produced the same results which showkey -s
used to produce under 2.4.something on a plain text console.  Maybe this
proves that X11 still accesses the keyboard at a sufficiently low level that
it doesn't suffer from the breakage that was added in 2.6.0-test4 keyboard
drivers.

> I guess the keyboard is sending Set 2 and the BIOS is translating the set
> 2 codes to set 1 for "compatibility with XT software".

I'm pretty sure that this isn't that simple.  The BIOS fails to translate
some keys.  I hacked grub enough to make it possible to type from a Japanese
 keyboard into grub.  Fortunately grub doesn't use every key that the BIOS
understands, so I was able to swap some scan codes in grub's interrupt
handler, let the BIOS translate the ones it likes, and then translate the
results again in grub's higher-level translator.

(Now why does such ugly stuff make people want to puke on their keyboards
when it's really the BIOS's fault  :-?)


^ permalink raw reply	[flat|nested] 21+ messages in thread
* Re: keyboard - was: Re: Linux 2.6.0-test4
@ 2003-09-05 14:57 John Bradford
  2003-09-05 23:45 ` Andries Brouwer
  0 siblings, 1 reply; 21+ messages in thread
From: John Bradford @ 2003-09-05 14:57 UTC (permalink / raw)
  To: linux-kernel, ndiamond

> > At this late stage, I don't think it is a good idea to completely
> > rewrite the untranslate algorithm.  So we continue to hack it and hack
> > it until it works.  :-/
>
> Surely not.  Some keyboards which worked since 2.0 and probably 1.something
> are broken in 2.6.

Aren't we over-complicating this?

The vast majority of keyboards are usable with translated Set 2, and
no workarounds.  Some workarounds may break other keyboards, possibly
ones we don't even know about yet.

There are advantages to using untranslated Set 2, and Set 3, and some
keyboards need to be used in those modes them to work correctly.

So, why not:

* Default to translated Set 2 with no work arounds, unless the
keyboard is known to work fine in Set 3.

Either:

1. It works perfectly.
2. It doesn't.

In case 1, if the user is happy with translated Set 2, there is no
problem.  If they are interested in seeing whether the technically
neater untranslated Set 2, or Set 3 work for them they can test them
and see.

In case 2, the user can boot with a kernel parameter enabling
workarounds for broken keyboards.  Conflicting workarounds can be
moved in to different sets, I.E. boot with inputworkarounds=1 for the
most common ones, and inputworkarounds=2 for the less common ones.

This should get almost all keyboards working by default.  Most of
those that don't work due to critical errors such as keys bouncing and
excessive auto-repeat, and those that don't work fully, should work
fully with workarounds enabled.

My keyboard which requires Set 3 to operate fully sends a distinctive
ID, so can be identified, and set to Set 3 automatically.

John.

^ permalink raw reply	[flat|nested] 21+ messages in thread
end of thread, other threads:[~2003-09-05 23:45 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2003-08-31 16:22 Re:Re: Linux 2.6.0-test4 Chris Heath
2003-09-01 16:01 ` Ralf Hildebrandt
2003-09-02  0:16   ` [PATCH] keyboard - was: " Andries Brouwer
2003-09-02  5:39     ` Ralf Hildebrandt
2003-09-02  8:07 ` Ralf Hildebrandt
2003-09-02 10:47   ` keyboard - was: " Andries Brouwer
2003-09-02 11:18     ` Ralf Hildebrandt
2003-09-02 12:32     ` Ralf Hildebrandt
2003-09-02 21:41       ` Andries Brouwer
2003-09-03  7:45         ` Ralf Hildebrandt
2003-09-03 13:25         ` Ralf Hildebrandt
2003-09-04  4:09 Chris Heath
2003-09-04 20:48 ` Jamie Lokier
2003-09-04 22:34   ` Andries Brouwer
2003-09-04 23:00     ` Jamie Lokier
2003-09-05  0:19       ` Andries Brouwer
2003-09-05  3:41         ` Chris Heath
2003-09-05 10:08           ` Andries Brouwer
2003-09-05 13:46 Norman Diamond
2003-09-05 14:57 John Bradford
2003-09-05 23:45 ` Andries Brouwer

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).