[PATCH] atkbd shaddup

[PATCH] atkbd shaddup

[Matthew Galgoci]

Andrew,

I can't be the only person to be annoyed by the "too many keys pressed" error message
that often gets spewed across the console when I am typing fast. This patch turns that
error message (and others) into info message. Also, one debug message was turned into
info, and a couple of warnings were turned into info where I thought it made sense.

Regards,

Matthew Galgoci

--- linux-2.6.4/drivers/input/keyboard/atkbd.c.orig	2004-03-12 12:21:13.595935024 -0500
+++ linux-2.6.4/drivers/input/keyboard/atkbd.c	2004-03-12 12:25:25.366938817 -0500
@@ -258,7 +258,7 @@
 			atkbd_report_key(&atkbd->dev, regs, KEY_HANJA, 3);
 			goto out;
 		case ATKBD_RET_ERR:
-			printk(KERN_WARNING "atkbd.c: Keyboard on %s reports too many keys pressed.\n", serio->phys);
+			printk(KERN_INFO "atkbd.c: Keyboard on %s reports too many keys pressed.\n", serio->phys);
 			goto out;
 	}
 
@@ -274,15 +274,15 @@
 		case ATKBD_KEY_NULL:
 			break;
 		case ATKBD_KEY_UNKNOWN:
-			printk(KERN_WARNING "atkbd.c: Unknown key %s (%s set %d, code %#x on %s).\n",
+			printk(KERN_INFO "atkbd.c: Unknown key %s (%s set %d, code %#x on %s).\n",
 				atkbd->release ? "released" : "pressed",
 				atkbd->translated ? "translated" : "raw", 
 				atkbd->set, code, serio->phys);
 			if (atkbd->translated && atkbd->set == 2 && code == 0x7a)
-				printk(KERN_WARNING "atkbd.c: This is an XFree86 bug. It shouldn't access"
+				printk(KERN_INFO "atkbd.c: This is an XFree86 bug. It shouldn't access"
 					" hardware directly.\n");
 			else
-				printk(KERN_WARNING "atkbd.c: Use 'setkeycodes %s%02x <keycode>' to make it known.\n",						code & 0x80 ? "e0" : "", code & 0x7f);
+				printk(KERN_INFO "atkbd.c: Use 'setkeycodes %s%02x <keycode>' to make it known.\n",						code & 0x80 ? "e0" : "", code & 0x7f);
 			break;
 		default:
 			value = atkbd->release ? 0 :
@@ -467,7 +467,7 @@
 
 	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_WARN "atkbd.c: keyboard reset failed on %s\n", atkbd->serio->phys);
 
 /*
  * Then we check the keyboard ID. We should get 0xab83 under normal conditions.
@@ -496,8 +496,8 @@
 	atkbd->id = (param[0] << 8) | param[1];
 
 	if (atkbd->id == 0xaca1 && atkbd->translated) {
-		printk(KERN_ERR "atkbd.c: NCD terminal keyboards are only supported on non-translating\n");
-		printk(KERN_ERR "atkbd.c: controllers. Use i8042.direct=1 to disable translation.\n");
+		printk(KERN_INFO "atkbd.c: NCD terminal keyboards are only supported on non-translating\n");
+		printk(KERN_INFO "atkbd.c: controllers. Use i8042.direct=1 to disable translation.\n");
 		return -1;
 	}
 
@@ -588,7 +588,7 @@
  */
 
 	if (atkbd_command(atkbd, NULL, ATKBD_CMD_ENABLE)) {
-		printk(KERN_ERR "atkbd.c: Failed to enable keyboard on %s\n",
+		printk(KERN_WARN "atkbd.c: Failed to enable keyboard on %s\n",
 			atkbd->serio->phys);
 		return -1;
 	}
@@ -744,7 +744,7 @@
 	int i;
 
         if (!dev) {
-                printk(KERN_DEBUG "atkbd: reconnect request, but serio is disconnected, ignoring...\n");
+                printk(KERN_INFO "atkbd.c: reconnect request, but serio is disconnected, ignoring...\n");
                 return -1;
         }
 

Re: [PATCH] atkbd shaddup

[Theodore Ts'o]

On Fri, Mar 12, 2004 at 12:37:06PM -0500, Matthew Galgoci wrote:
> 
> Andrew,
> 
> I can't be the only person to be annoyed by the "too many keys
> pressed" error message that often gets spewed across the console
> when I am typing fast. This patch turns that error message (and
> others) into info message. Also, one debug message was turned into
> info, and a couple of warnings were turned into info where I thought
> it made sense.

I'd go even further.  Do we need to print the "too many keys pressed"
message at *all*?  Why would anyone care?

						- Ted

Re: [PATCH] atkbd shaddup

[Matthew Galgoci]

On Fri, 12 Mar 2004, Theodore Ts'o wrote:

> On Fri, Mar 12, 2004 at 12:37:06PM -0500, Matthew Galgoci wrote:
> > 
> > Andrew,
> > 
> > I can't be the only person to be annoyed by the "too many keys
> > pressed" error message that often gets spewed across the console
> > when I am typing fast. This patch turns that error message (and
> > others) into info message. Also, one debug message was turned into
> > info, and a couple of warnings were turned into info where I thought
> > it made sense.
> 
> I'd go even further.  Do we need to print the "too many keys pressed"
> message at *all*?  Why would anyone care?

I wondered the same thing. Maybe it should be #ifdef DEBUG'd instead of toned
down.

-- 
Matthew Galgoci
System Administrator
Red Hat, Inc
919.754.3700 x44155

[PATCH - take 2] atkbd shaddup

[Matthew Galgoci]

> > Andrew,
> > 
> > I can't be the only person to be annoyed by the "too many keys
> > pressed" error message that often gets spewed across the console
> > when I am typing fast. This patch turns that error message (and
> > others) into info message. Also, one debug message was turned into
> > info, and a couple of warnings were turned into info where I thought
> > it made sense.
> 
> I'd go even further.  Do we need to print the "too many keys pressed"
> message at *all*?  Why would anyone care?

Take 2 - "too many keys pressed" has been ifdef ATKBD_DEBUG'd and changed to KERN_DEBUG.


--- linux-2.6.4/drivers/input/keyboard/atkbd.c.orig	2004-03-12 12:21:13.595935024 -0500
+++ linux-2.6.4/drivers/input/keyboard/atkbd.c	2004-03-12 14:51:03.935281846 -0500
@@ -257,9 +257,11 @@
 		case ATKBD_RET_HANJA:
 			atkbd_report_key(&atkbd->dev, regs, KEY_HANJA, 3);
 			goto out;
+#ifdef ATKBD_DEBUG
 		case ATKBD_RET_ERR:
-			printk(KERN_WARNING "atkbd.c: Keyboard on %s reports too many keys pressed.\n", serio->phys);
+			printk(KERN_DEBUG "atkbd.c: Keyboard on %s reports too many keys pressed.\n", serio->phys);
 			goto out;
+#endif /* ATKBD_DEBUG */
 	}
 
 	if (atkbd->set != 3)
@@ -274,15 +276,15 @@
 		case ATKBD_KEY_NULL:
 			break;
 		case ATKBD_KEY_UNKNOWN:
-			printk(KERN_WARNING "atkbd.c: Unknown key %s (%s set %d, code %#x on %s).\n",
+			printk(KERN_INFO "atkbd.c: Unknown key %s (%s set %d, code %#x on %s).\n",
 				atkbd->release ? "released" : "pressed",
 				atkbd->translated ? "translated" : "raw", 
 				atkbd->set, code, serio->phys);
 			if (atkbd->translated && atkbd->set == 2 && code == 0x7a)
-				printk(KERN_WARNING "atkbd.c: This is an XFree86 bug. It shouldn't access"
+				printk(KERN_INFO "atkbd.c: This is an XFree86 bug. It shouldn't access"
 					" hardware directly.\n");
 			else
-				printk(KERN_WARNING "atkbd.c: Use 'setkeycodes %s%02x <keycode>' to make it known.\n",						code & 0x80 ? "e0" : "", code & 0x7f);
+				printk(KERN_INFO "atkbd.c: Use 'setkeycodes %s%02x <keycode>' to make it known.\n",						code & 0x80 ? "e0" : "", code & 0x7f);
 			break;
 		default:
 			value = atkbd->release ? 0 :
@@ -467,7 +469,7 @@
 
 	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_WARN "atkbd.c: keyboard reset failed on %s\n", atkbd->serio->phys);
 
 /*
  * Then we check the keyboard ID. We should get 0xab83 under normal conditions.
@@ -496,8 +498,8 @@
 	atkbd->id = (param[0] << 8) | param[1];
 
 	if (atkbd->id == 0xaca1 && atkbd->translated) {
-		printk(KERN_ERR "atkbd.c: NCD terminal keyboards are only supported on non-translating\n");
-		printk(KERN_ERR "atkbd.c: controllers. Use i8042.direct=1 to disable translation.\n");
+		printk(KERN_INFO "atkbd.c: NCD terminal keyboards are only supported on non-translating\n");
+		printk(KERN_INFO "atkbd.c: controllers. Use i8042.direct=1 to disable translation.\n");
 		return -1;
 	}
 
@@ -588,7 +590,7 @@
  */
 
 	if (atkbd_command(atkbd, NULL, ATKBD_CMD_ENABLE)) {
-		printk(KERN_ERR "atkbd.c: Failed to enable keyboard on %s\n",
+		printk(KERN_WARN "atkbd.c: Failed to enable keyboard on %s\n",
 			atkbd->serio->phys);
 		return -1;
 	}
@@ -744,7 +746,7 @@
 	int i;
 
         if (!dev) {
-                printk(KERN_DEBUG "atkbd: reconnect request, but serio is disconnected, ignoring...\n");
+                printk(KERN_INFO "atkbd.c: reconnect request, but serio is disconnected, ignoring...\n");
                 return -1;
         }
 

[PATCH - take 3] atkbd shaddup

[Matthew Galgoci]

> > Andrew,
> > 
> > I can't be the only person to be annoyed by the "too many keys
> > pressed" error message that often gets spewed across the console
> > when I am typing fast. This patch turns that error message (and
> > others) into info message. Also, one debug message was turned into
> > info, and a couple of warnings were turned into info where I thought
> > it made sense.
> 
> I'd go even further.  Do we need to print the "too many keys pressed"
> message at *all*?  Why would anyone care?

atkbd shaddup - Take 3 - I suck again.

--- linux-2.6.4/drivers/input/keyboard/atkbd.c.orig	2004-03-12 20:13:30.000000000 -0500
+++ linux-2.6.4/drivers/input/keyboard/atkbd.c	2004-03-12 20:13:49.000000000 -0500
@@ -257,9 +257,11 @@
 		case ATKBD_RET_HANJA:
 			atkbd_report_key(&atkbd->dev, regs, KEY_HANJA, 3);
 			goto out;
+#ifdef ATKBD_DEBUG
 		case ATKBD_RET_ERR:
-			printk(KERN_WARNING "atkbd.c: Keyboard on %s reports too many keys pressed.\n", serio->phys);
+			printk(KERN_DEBUG "atkbd.c: Keyboard on %s reports too many keys pressed.\n", serio->phys);
 			goto out;
+#endif /* ATKBD_DEBUG */
 	}
 
 	if (atkbd->set != 3)
@@ -274,15 +276,15 @@
 		case ATKBD_KEY_NULL:
 			break;
 		case ATKBD_KEY_UNKNOWN:
-			printk(KERN_WARNING "atkbd.c: Unknown key %s (%s set %d, code %#x on %s).\n",
+			printk(KERN_INFO "atkbd.c: Unknown key %s (%s set %d, code %#x on %s).\n",
 				atkbd->release ? "released" : "pressed",
 				atkbd->translated ? "translated" : "raw", 
 				atkbd->set, code, serio->phys);
 			if (atkbd->translated && atkbd->set == 2 && code == 0x7a)
-				printk(KERN_WARNING "atkbd.c: This is an XFree86 bug. It shouldn't access"
+				printk(KERN_INFO "atkbd.c: This is an XFree86 bug. It shouldn't access"
 					" hardware directly.\n");
 			else
-				printk(KERN_WARNING "atkbd.c: Use 'setkeycodes %s%02x <keycode>' to make it known.\n",						code & 0x80 ? "e0" : "", code & 0x7f);
+				printk(KERN_INFO "atkbd.c: Use 'setkeycodes %s%02x <keycode>' to make it known.\n",						code & 0x80 ? "e0" : "", code & 0x7f);
 			break;
 		default:
 			value = atkbd->release ? 0 :
@@ -496,8 +498,8 @@
 	atkbd->id = (param[0] << 8) | param[1];
 
 	if (atkbd->id == 0xaca1 && atkbd->translated) {
-		printk(KERN_ERR "atkbd.c: NCD terminal keyboards are only supported on non-translating\n");
-		printk(KERN_ERR "atkbd.c: controllers. Use i8042.direct=1 to disable translation.\n");
+		printk(KERN_INFO "atkbd.c: NCD terminal keyboards are only supported on non-translating\n");
+		printk(KERN_INFO "atkbd.c: controllers. Use i8042.direct=1 to disable translation.\n");
 		return -1;
 	}
 
@@ -588,7 +590,7 @@
  */
 
 	if (atkbd_command(atkbd, NULL, ATKBD_CMD_ENABLE)) {
-		printk(KERN_ERR "atkbd.c: Failed to enable keyboard on %s\n",
+		printk(KERN_WARNING "atkbd.c: Failed to enable keyboard on %s\n",
 			atkbd->serio->phys);
 		return -1;
 	}
@@ -744,7 +746,7 @@
 	int i;
 
         if (!dev) {
-                printk(KERN_DEBUG "atkbd: reconnect request, but serio is disconnected, ignoring...\n");
+                printk(KERN_INFO "atkbd.c: reconnect request, but serio is disconnected, ignoring...\n");
                 return -1;
         }
 

Re: [PATCH - take 3] atkbd shaddup

[Dmitry Torokhov]

On Friday 12 March 2004 08:17 pm, Matthew Galgoci wrote:
> > > Andrew,
> > > 
> > > I can't be the only person to be annoyed by the "too many keys
> > > pressed" error message that often gets spewed across the console
> > > when I am typing fast. This patch turns that error message (and
> > > others) into info message. Also, one debug message was turned into
> > > info, and a couple of warnings were turned into info where I thought
> > > it made sense.
> > 
> > I'd go even further.  Do we need to print the "too many keys pressed"
> > message at *all*?  Why would anyone care?
> 
> atkbd shaddup - Take 3 - I suck again.
>

Hi, 

I agree that "too many keys pressed" message is annoying; I have some
objections to the rest of the changes:

>  	if (atkbd->id == 0xaca1 && atkbd->translated) {
> -		printk(KERN_ERR "atkbd.c: NCD terminal keyboards are only supported on non-translating\n");
> -		printk(KERN_ERR "atkbd.c: controllers. Use i8042.direct=1 to disable translation.\n");
> +		printk(KERN_INFO "atkbd.c: NCD terminal keyboards are only supported on non-translating\n");
> +		printk(KERN_INFO "atkbd.c: controllers. Use i8042.direct=1 to disable translation.\n");
>  		return -1;

This message has severity "error" since at this point because keyboard
initialization fails and you are left without a keyboard.

>  	if (atkbd_command(atkbd, NULL, ATKBD_CMD_ENABLE)) {
> -		printk(KERN_ERR "atkbd.c: Failed to enable keyboard on %s\n",
> +		printk(KERN_WARNING "atkbd.c: Failed to enable keyboard on %s\n",
>  			atkbd->serio->phys);
>  		return -1;
>  	}

Ditto. This is a hard error, not a warning, because keyboard will not be
available if atkbd fails to enable it.  

>  		case ATKBD_KEY_UNKNOWN:
> -			printk(KERN_WARNING "atkbd.c: Unknown key %s (%s set %d, code %#x on %s).\n",
> +			printk(KERN_INFO "atkbd.c: Unknown key %s (%s set %d, code %#x on %s).\n",
>  				atkbd->release ? "released" : "pressed",
>  				atkbd->translated ? "translated" : "raw", 
>  				atkbd->set, code, serio->phys);
>  			if (atkbd->translated && atkbd->set == 2 && code == 0x7a)
> -				printk(KERN_WARNING "atkbd.c: This is an XFree86 bug. It shouldn't access"
> +				printk(KERN_INFO "atkbd.c: This is an XFree86 bug. It shouldn't access"
>  					" hardware directly.\n");
>  			else
> -				printk(KERN_WARNING "atkbd.c: Use 'setkeycodes %s%02x <keycode>' to make it known.\n",						code & 0x80 ? "e0" : "", code & 0x7f);
> +				printk(KERN_INFO "atkbd.c: Use 'setkeycodes %s%02x <keycode>' to make it known.\n",						code & 0x80 ? "e0" : "", code & 0x7f);
>  			break;

Somebody either pokes directly in the keyboard controller guts or keymap is
incomplete - both deserve a warning, although wording could be better.

>  
>          if (!dev) {
> -                printk(KERN_DEBUG "atkbd: reconnect request, but serio is disconnected, ignoring...\n");
> +                 printk(KERN_INFO "atkbd.c: reconnect request, but serio is disconnected, ignoring...\n");
>                  return -1;
>          }

Might be useful while debugging reconnect code but otherwise normal situation
and is not worth mentioning.

-- 
Dmitry

Re: [PATCH] atkbd shaddup

[Pavel Machek]

Hi!

> > I can't be the only person to be annoyed by the "too many keys
> > pressed" error message that often gets spewed across the console
> > when I am typing fast. This patch turns that error message (and
> > others) into info message. Also, one debug message was turned into
> > info, and a couple of warnings were turned into info where I thought
> > it made sense.
> 
> I'd go even further.  Do we need to print the "too many keys pressed"
> message at *all*?  Why would anyone care?

You are typing too fast and key gets lost. You'll either think
that its your fault or that microswitch is failing. With
the message you know keyboard is miss-designed.
KERN_INFO seems fine...
-- 
64 bytes from 195.113.31.123: icmp_seq=28 ttl=51 time=448769.1 ms         

Re: [PATCH] atkbd shaddup

[Matthew Galgoci]

Ok, here is the revised patch based on Pavel's feedback:


--- linux-2.6.4/drivers/input/keyboard/atkbd.c.orig	2004-03-15 12:40:01.578423740 -0500
+++ linux-2.6.4/drivers/input/keyboard/atkbd.c	2004-03-15 12:45:02.545009523 -0500
@@ -197,7 +197,7 @@
 
 #if !defined(__i386__) && !defined (__x86_64__)
 	if ((flags & (SERIO_FRAME | SERIO_PARITY)) && (~flags & SERIO_TIMEOUT) && !atkbd->resend && atkbd->write) {
-		printk("atkbd.c: frame/parity error: %02x\n", flags);
+		printk(KERN_WARNING "atkbd.c: frame/parity error: %02x\n", flags);
 		serio_write(serio, ATKBD_CMD_RESEND);
 		atkbd->resend = 1;
 		goto out;
@@ -258,7 +258,7 @@
 			atkbd_report_key(&atkbd->dev, regs, KEY_HANJA, 3);
 			goto out;
 		case ATKBD_RET_ERR:
-			printk(KERN_WARNING "atkbd.c: Keyboard on %s reports too many keys pressed.\n", serio->phys);
+			printk(KERN_INFO "atkbd.c: Keyboard on %s reports too many keys pressed.\n", serio->phys);
 			goto out;
 	}
 

If there are no more objections, please apply.

Regards,

Matthew Galgoci


On Mon, 15 Mar 2004, Pavel Machek wrote:

> > Hi,
> > 
> > Pavel, how is this:
> > 
> > --- linux-2.6.4/drivers/input/keyboard/atkbd.c.orig	2004-03-15 12:40:01.578423740 -0500
> > +++ linux-2.6.4/drivers/input/keyboard/atkbd.c	2004-03-15 12:45:02.545009523 -0500
> > @@ -197,7 +197,7 @@
> >  
> >  #if !defined(__i386__) && !defined (__x86_64__)
> >  	if ((flags & (SERIO_FRAME | SERIO_PARITY)) && (~flags & SERIO_TIMEOUT) && !atkbd->resend && atkbd->write) {
> > -		printk("atkbd.c: frame/parity error: %02x\n", flags);
> > +		printk(KERN_WARNING "atkbd.c: frame/parity error: %02x\n", flags);
> >  		serio_write(serio, ATKBD_CMD_RESEND);
> >  		atkbd->resend = 1;
> >  		goto out;
> 
> Ok.
> 
> > @@ -258,7 +258,7 @@
> >  			atkbd_report_key(&atkbd->dev, regs, KEY_HANJA, 3);
> >  			goto out;
> >  		case ATKBD_RET_ERR:
> > -			printk(KERN_WARNING "atkbd.c: Keyboard on %s reports too many keys pressed.\n", serio->phys);
> > +			printk(KERN_INFO "atkbd.c: Keyboard on %s reports too many keys pressed.\n", serio->phys);
> >  			goto out;
> >  	}
> >  
> 
> Ok.
> 
> > @@ -274,15 +274,15 @@
> >  		case ATKBD_KEY_NULL:
> >  			break;
> >  		case ATKBD_KEY_UNKNOWN:
> > -			printk(KERN_WARNING "atkbd.c: Unknown key %s (%s set %d, code %#x on %s).\n",
> > +			printk(KERN_INFO "atkbd.c: Unknown key %s (%s set %d, code %#x on %s).\n",
> >  				atkbd->release ? "released" : "pressed",
> >  				atkbd->translated ? "translated" : "raw", 
> >  				atkbd->set, code, serio->phys);
> >  			if (atkbd->translated && atkbd->set == 2 && code == 0x7a)
> > -				printk(KERN_WARNING "atkbd.c: This is an XFree86 bug. It shouldn't access"
> > +				printk(KERN_INFO "atkbd.c: This is an XFree86 bug. It shouldn't access"
> >  					" hardware directly.\n");
> >  			else
> > -				printk(KERN_WARNING "atkbd.c: Use 'setkeycodes %s%02x <keycode>' to make it known.\n",						code & 0x80 ? "e0" : "", code & 0x7f);
> > +				printk(KERN_INFO "atkbd.c: Use 'setkeycodes %s%02x <keycode>' to make it known.\n",						code & 0x80 ? "e0" : "", code & 0x7f);
> >  			break;
> 
> I'd leave those at WARNING level.
> 
> >  		default:
> >  			value = atkbd->release ? 0 :
> > @@ -496,8 +496,8 @@
> >  	atkbd->id = (param[0] << 8) | param[1];
> >  
> >  	if (atkbd->id == 0xaca1 && atkbd->translated) {
> > -		printk(KERN_ERR "atkbd.c: NCD terminal keyboards are only supported on non-translating\n");
> > -		printk(KERN_ERR "atkbd.c: controllers. Use i8042.direct=1 to disable translation.\n");
> > +		printk(KERN_WARNING "atkbd.c: NCD terminal keyboards are only supported on non-translating\n");
> > +		printk(KERN_WARNING "atkbd.c: controllers. Use i8042.direct=1 to disable translation.\n");
> >  		return -1;
> >  	}
> >  
> 
> This is hard error. Leave it as such.
> 
> > @@ -588,7 +588,7 @@
> >   */
> >  
> >  	if (atkbd_command(atkbd, NULL, ATKBD_CMD_ENABLE)) {
> > -		printk(KERN_ERR "atkbd.c: Failed to enable keyboard on %s\n",
> > +		printk(KERN_WARNING "atkbd.c: Failed to enable keyboard on %s\n",
> >  			atkbd->serio->phys);
> >  		return -1;
> >  	}
> 
> Same here.
> 
> > @@ -744,7 +744,7 @@
> >  	int i;
> >  
> >          if (!dev) {
> > -                printk(KERN_DEBUG "atkbd: reconnect request, but serio is disconnected, ignoring...\n");
> > +                printk(KERN_WARNING "atkbd: reconnect request, but serio is disconnected, ignoring...\n");
> >                  return -1;
> >          }
> > 
> 
> I do not know about this one....
> 
> 									Pavel
> 
> 

-- 
Matthew Galgoci
System Administrator
Red Hat, Inc
919.754.3700 x44155