Re: Toshiba keyboard workaroun

Re: Toshiba keyboard workaroun

[Pete Zaitcev]

> --- clean/drivers/char/keyboard.c	2003-02-15 18:51:18.000000000 +0100
> +++ linux/drivers/char/keyboard.c	2003-02-15 19:19:45.000000000 +0100
> @@ -1020,6 +1041,23 @@
>  	struct tty_struct *tty;
>  	int shift_final;
>  
> +        /*
> +         * Fix for Toshiba Satellites. Toshiba's like to repeat 
> +	 * "key down" event for A in combinations like shift-A.
> +	 * Thanx to Andrei Pitis <pink@roedu.net>.
> +         */
> +        static int prev_scancode = 0;
> +        static int stop_jiffies = 0;
> +
> +        /* new scancode, trigger delay */
> +        if (keycode != prev_scancode) 	       stop_jiffies = jiffies;
> +        else if (jiffies - stop_jiffies >= 10) stop_jiffies = 0;
> +        else {
> +	    printk( "Keyboard glitch detected, ignoring keypress\n" );
> +            return;
> +	}
> +        prev_scancode = keycode;
> +
>  	if (down != 2)
>  		add_keyboard_randomness((keycode << 1) ^ down);

This is incredibly broken, on many layers.

First, formatting does not respect the original code. Pavel, please,
I do not care what crap you write in softsuspend, but this is a
generic piece of code. Be kind to those who come next.

Second, no HZ or other way to specify a wall clock interval.
What if I run with HZ=4000? How do you protect against a
jiffies wraparound?

Third, I do not see how this is supposed to work at all.
What if I hit two letters like in a word "Fool"? The up event
is filtered already by this time, so, won't this code eat
the second 'o'?

-- Pete

Re: Toshiba keyboard workaroun

[Pavel Machek]

Hi!

> > --- clean/drivers/char/keyboard.c	2003-02-15 18:51:18.000000000 +0100
> > +++ linux/drivers/char/keyboard.c	2003-02-15 19:19:45.000000000 +0100
> > @@ -1020,6 +1041,23 @@
> >  	struct tty_struct *tty;
> >  	int shift_final;
> >  
> > +        /*
> > +         * Fix for Toshiba Satellites. Toshiba's like to repeat 
> > +	 * "key down" event for A in combinations like shift-A.
> > +	 * Thanx to Andrei Pitis <pink@roedu.net>.
> > +         */
> > +        static int prev_scancode = 0;
> > +        static int stop_jiffies = 0;
> > +
> > +        /* new scancode, trigger delay */
> > +        if (keycode != prev_scancode) 	       stop_jiffies = jiffies;
> > +        else if (jiffies - stop_jiffies >= 10) stop_jiffies = 0;
> > +        else {
> > +	    printk( "Keyboard glitch detected, ignoring keypress\n" );
> > +            return;
> > +	}
> > +        prev_scancode = keycode;
> > +
> >  	if (down != 2)
> >  		add_keyboard_randomness((keycode << 1) ^ down);
> 
> This is incredibly broken, on many layers.
> 
> First, formatting does not respect the original code. Pavel, please,
> I do not care what crap you write in softsuspend, but this is a
> generic piece of code. Be kind to those who come next.

Its not originally mine, I should have reformated it. Sorry.

> Second, no HZ or other way to specify a wall clock interval.
> What if I run with HZ=4000? How do you protect against a
> jiffies wraparound?
> 
> Third, I do not see how this is supposed to work at all.
> What if I hit two letters like in a word "Fool"? The up event
> is filtered already by this time, so, won't this code eat
> the second 'o'?

The up event is not filtered, AFAICS. Parameter down carries that.

							Pavel

-- 
Casualities in World Trade Center: ~3k dead inside the building,
cryptography in U.S.A. and free speech in Czech Republic.

Re: Toshiba keyboard workaroun

[Pavel Machek]

Hi!

> First, formatting does not respect the original code. Pavel, please,
> I do not care what crap you write in softsuspend, but this is a

Do you like this better?

								Pavel
PS: Oh, and, what crap in swsusp are you talking about?

--- clean/drivers/char/keyboard.c	2003-02-15 18:51:18.000000000 +0100
+++ linux/drivers/char/keyboard.c	2003-02-18 23:20:33.000000000 +0100
@@ -1020,6 +1041,26 @@
 	struct tty_struct *tty;
 	int shift_final;
 
+        /*
+         * Fix for Toshiba Satellites. Toshiba's like to repeat 
+	 * "key down" event for A in combinations like shift-A.
+	 * Thanx to Andrei Pitis <pink@roedu.net>.
+         */
+        static int prev_scancode = 0;
+        static int stop_jiffies = 0;
+
+        /* new scancode, trigger delay */
+        if (keycode != prev_scancode)
+		stop_jiffies = jiffies;
+        else 
+		if (time_after(jiffies, stop_jiffies + HZ/10))
+			prev_scancode = -1;
+        else {
+		printk( "Keyboard glitch detected, ignoring keypress\n" );
+		return;
+	}
+        prev_scancode = keycode;
+
 	if (down != 2)
 		add_keyboard_randomness((keycode << 1) ^ down);
 

-- 
Casualities in World Trade Center: ~3k dead inside the building,
cryptography in U.S.A. and free speech in Czech Republic.

Toshiba keyboard workaroun

[Pavel Machek]

Hi!

You said that you'll submit toshiba keyboard fix for 2.4 to
marcelo... Here goes 2.5 version, will you submit it to Linus? ;-).

--- clean/drivers/char/keyboard.c	2003-02-15 18:51:18.000000000 +0100
+++ linux/drivers/char/keyboard.c	2003-02-15 19:19:45.000000000 +0100
@@ -1020,6 +1041,23 @@
 	struct tty_struct *tty;
 	int shift_final;
 
+        /*
+         * Fix for Toshiba Satellites. Toshiba's like to repeat 
+	 * "key down" event for A in combinations like shift-A.
+	 * Thanx to Andrei Pitis <pink@roedu.net>.
+         */
+        static int prev_scancode = 0;
+        static int stop_jiffies = 0;
+
+        /* new scancode, trigger delay */
+        if (keycode != prev_scancode) 	       stop_jiffies = jiffies;
+        else if (jiffies - stop_jiffies >= 10) stop_jiffies = 0;
+        else {
+	    printk( "Keyboard glitch detected, ignoring keypress\n" );
+            return;
+	}
+        prev_scancode = keycode;
+
 	if (down != 2)
 		add_keyboard_randomness((keycode << 1) ^ down);
 

-- 
When do you have a heart between your knees?
[Johanka's followup: and *two* hearts?]

Re: Toshiba keyboard workaroun

[Vojtech Pavlik]

On Tue, Feb 18, 2003 at 10:19:40PM +0100, Pavel Machek wrote:

> You said that you'll submit toshiba keyboard fix for 2.4 to
> marcelo... Here goes 2.5 version, will you submit it to Linus? ;-).

This belongs into drivers/input/keyboard/atkbd.c, not here!

Subsequent keypresses are already ignored - autorepeat is done in
software, however if you get a very quick press-release-press of the
same key that means the keyboard controller didn't do proper debouncing
and you probably can ignore the later release and press. But you must
ignore the release as well. Further - comparing to 10 jiffies isn't a
good idea - jiffies speed is different on different archs.

> --- clean/drivers/char/keyboard.c	2003-02-15 18:51:18.000000000 +0100
> +++ linux/drivers/char/keyboard.c	2003-02-15 19:19:45.000000000 +0100
> @@ -1020,6 +1041,23 @@
>  	struct tty_struct *tty;
>  	int shift_final;
>  
> +        /*
> +         * Fix for Toshiba Satellites. Toshiba's like to repeat 
> +	 * "key down" event for A in combinations like shift-A.
> +	 * Thanx to Andrei Pitis <pink@roedu.net>.
> +         */
> +        static int prev_scancode = 0;
> +        static int stop_jiffies = 0;
> +
> +        /* new scancode, trigger delay */
> +        if (keycode != prev_scancode) 	       stop_jiffies = jiffies;
> +        else if (jiffies - stop_jiffies >= 10) stop_jiffies = 0;
> +        else {
> +	    printk( "Keyboard glitch detected, ignoring keypress\n" );
> +            return;
> +	}
> +        prev_scancode = keycode;
> +
>  	if (down != 2)
>  		add_keyboard_randomness((keycode << 1) ^ down);

-- 
Vojtech Pavlik
SuSE Labs

Re: Toshiba keyboard workaroun

[Pavel Machek]

Hi!

> > You said that you'll submit toshiba keyboard fix for 2.4 to
> > marcelo... Here goes 2.5 version, will you submit it to Linus? ;-).
> 
> This belongs into drivers/input/keyboard/atkbd.c, not here!
> 
> Subsequent keypresses are already ignored - autorepeat is done in
> software, however if you get a very quick press-release-press of the

No, it did not do releases.

Guess I should really try 2.5. without that patch, because vojtech's
patches might have had fixed that as a side effect. Alan, please don't
push this patch to Linus until this is fixed. [2.4 version is still
neccessary, through].
								Pavel

> same key that means the keyboard controller didn't do proper debouncing
> and you probably can ignore the later release and press. But you must
> ignore the release as well. Further - comparing to 10 jiffies isn't a
> good idea - jiffies speed is different on different archs.



> > --- clean/drivers/char/keyboard.c	2003-02-15 18:51:18.000000000 +0100
> > +++ linux/drivers/char/keyboard.c	2003-02-15 19:19:45.000000000 +0100
> > @@ -1020,6 +1041,23 @@
> >  	struct tty_struct *tty;
> >  	int shift_final;
> >  
> > +        /*
> > +         * Fix for Toshiba Satellites. Toshiba's like to repeat 
> > +	 * "key down" event for A in combinations like shift-A.
> > +	 * Thanx to Andrei Pitis <pink@roedu.net>.
> > +         */
> > +        static int prev_scancode = 0;
> > +        static int stop_jiffies = 0;
> > +
> > +        /* new scancode, trigger delay */
> > +        if (keycode != prev_scancode) 	       stop_jiffies = jiffies;
> > +        else if (jiffies - stop_jiffies >= 10) stop_jiffies = 0;
> > +        else {
> > +	    printk( "Keyboard glitch detected, ignoring keypress\n" );
> > +            return;
> > +	}
> > +        prev_scancode = keycode;
> > +
> >  	if (down != 2)
> >  		add_keyboard_randomness((keycode << 1) ^ down);
> 

-- 
Horseback riding is like software...
...vgf orggre jura vgf serr.

Re: Toshiba keyboard workaroun

[Christoph Hellwig]

On Tue, Feb 18, 2003 at 10:19:40PM +0100, Pavel Machek wrote:
> +        /*
> +         * Fix for Toshiba Satellites. Toshiba's like to repeat 
> +	 * "key down" event for A in combinations like shift-A.
> +	 * Thanx to Andrei Pitis <pink@roedu.net>.
> +         */
> +        static int prev_scancode = 0;
> +        static int stop_jiffies = 0;
> +
> +        /* new scancode, trigger delay */
> +        if (keycode != prev_scancode) 	       stop_jiffies = jiffies;
> +        else if (jiffies - stop_jiffies >= 10) stop_jiffies = 0;
> +        else {
> +	    printk( "Keyboard glitch detected, ignoring keypress\n" );
> +            return;
> +	}
> +        prev_scancode = keycode;
> +

That codingstyle is not acceptable.  Please reformat to match
Documentation/CodingStyle.  Also there are macros for jiffie overflow
handling you might want to use, see include/linux/jiffies.h