Patchwork rework keyboard driver

login
register
about
Submitter Stefan Reinauer
Date 2009-10-24 01:30:43
Message ID <4AE258C3.7030008@coresystems.de>
Download mbox | patch
Permalink /patch/474/
State Accepted
Headers show

Comments

Stefan Reinauer - 2009-10-24 01:30:43
See patch
Myles Watson - 2009-10-24 13:42:09
2009/10/23 Stefan Reinauer <stepan@coresystems.de>

> Index: src/pc80/keyboard.c
> ===================================================================
> --- src/pc80/keyboard.c (revision 4832)
> +++ src/pc80/keyboard.c (working copy)
> @@ -26,13 +26,47 @@
>  #include <arch/io.h>
>  #include <delay.h>
>
> +#define KBD_DATA       0x60
> +#define KBD_COMMAND    0x64
> +#define KBD_STATUS     0x64
>
>        if (!timeout) {
>                printk_err("Couldn't cleanup the keyboard controller
> buffers\n");
> -               printk_err("0x64: 0x%x, 0x60: 0x%x\n", inb(0x64),
> inb(0x60));
> +               printk_err("Status (0x64): 0x%x, Buffer (0x60): 0x%x\n",
>
could be printk_err("Status (0x%x)...
 KBD_STATUS, KBD_DATA,

I think we should have it be a number or a #define everywhere.

+                               inb(KBD_STATUS), inb(KBD_DATA));
>
>
> @@ -194,6 +247,8 @@
>  {
>        if ((port0 == 0x60) && (port1 == 0x64)) {
>
Same thing here, right?

Thanks,
Myles
Myles Watson - 2009-10-24 13:58:10
Acked-by: Myles Watson <mylesgw@gmail.com>

Thanks,
Myles
Stefan Reinauer - 2009-10-24 18:16:51
Myles Watson wrote:
>
>
> 2009/10/23 Stefan Reinauer <stepan@coresystems.de
> <mailto:stepan@coresystems.de>>
>
>     Index: src/pc80/keyboard.c
>     ===================================================================
>     --- src/pc80/keyboard.c (revision 4832)
>     +++ src/pc80/keyboard.c (working copy)
>     @@ -26,13 +26,47 @@
>      #include <arch/io.h>
>      #include <delay.h>
>
>     +#define KBD_DATA       0x60
>     +#define KBD_COMMAND    0x64
>     +#define KBD_STATUS     0x64
>
>            if (!timeout) {
>                    printk_err("Couldn't cleanup the keyboard
>     controller buffers\n");
>     -               printk_err("0x64: 0x%x, 0x60: 0x%x\n", inb(0x64),
>     inb(0x60));
>     +               printk_err("Status (0x64): 0x%x, Buffer (0x60):
>     0x%x\n",
>
> could be printk_err("Status (0x%x)...
>  KBD_STATUS, KBD_DATA,
>
> I think we should have it be a number or a #define everywhere.

Fixed.

>
>     +                               inb(KBD_STATUS), inb(KBD_DATA));
>
>
>     @@ -194,6 +247,8 @@
>      {
>            if ((port0 == 0x60) && (port1 == 0x64)) {
>
> Same thing here, right?
I like the idea, but ... Shouldn't we drop that whole function instead?
It's a runtime check for compile time settings, which generally sounds
like a bad idea.

Committed as r4842
Myles Watson - 2009-10-24 19:37:39
> >            if ((port0 == 0x60) && (port1 == 0x64)) {
> >
> > Same thing here, right?
> I like the idea, but ... Shouldn't we drop that whole function instead?
> It's a runtime check for compile time settings, which generally sounds
> like a bad idea.
Yes.  You're right.

Thanks,
Myles
Kevin O'Connor - 2009-10-24 22:28:11
On Sat, Oct 24, 2009 at 03:30:43AM +0200, Stefan Reinauer wrote:
> See patch

As a side note, it would be nice to move the ps2 keyboard
initialization out of coreboot and into libpayload.

The keyboard init takes a long time to run - several hundred
milliseconds with my keyboard.  Many payloads (eg, linux and seabios)
end up running their own keyboard initialization, so this time spent
in coreboot is often unneeded.

-Kevin
Peter Stuge - 2009-10-25 14:08:04
Kevin O'Connor wrote:
> As a side note, it would be nice to move the ps2 keyboard
> initialization out of coreboot and into libpayload.

If things still work, then definately yes!


> The keyboard init takes a long time to run - several hundred
> milliseconds with my keyboard.  Many payloads (eg, linux and
> seabios) end up running their own keyboard initialization,

It's not always enough though - I had keyboard problems with Linux as
payload in v3. The keyboard only worked in v2.

However, I don't know if it was caused by missing kbc init, or if the
problem was rather with setting up the chipset so that there _was_ a
kbc visible to begin with.


//Peter

Patch

Index: src/pc80/keyboard.c
===================================================================
--- src/pc80/keyboard.c	(revision 4832)
+++ src/pc80/keyboard.c	(working copy)
@@ -26,13 +26,47 @@ 
 #include <arch/io.h>
 #include <delay.h>
 
+#define KBD_DATA	0x60
+#define KBD_COMMAND	0x64
+#define KBD_STATUS	0x64
+#define   KBD_IBF	(1 << 1) // 1: input buffer full (data ready for ec)
+#define   KBD_OBF	(1 << 0) // 1: output buffer full (data ready for host)
+
+// Keyboard Controller Commands
+#define KBC_CMD_READ_COMMAND	0x20 // Read command byte
+#define KBC_CMD_WRITE_COMMAND	0x60 // Write command byte
+#define KBC_CMD_SELF_TEST	0xAA // Controller self-test
+
+/* The Keyboard controller command byte
+ *  BIT	| Description
+ *  ----+-------------------------------------------------------
+ *   7  | reserved, must be zero
+ *   6  | XT Translation, (1 = on, 0 = off)
+ *   5  | Disable Mouse Port (1 = disable, 0 = enable)
+ *   4  | Disable Keyboard Port (1 = disable, 0 = enable)
+ *   3  | reserved, must be zero
+ *   2  | System Flag (1 = self-test passed. DO NOT SET TO ZERO)
+ *   1  | Mouse Port Interrupts (1 = enable, 0 = disable)
+ *   0  | Keyboard Port Interrupts (1 = enable, 0 = disable)
+ */
+
+// Keyboard Controller Replies
+#define KBC_REPLY_SELFTEST_OK	0x55 // controller self-test succeeded
+
+//
+// Keyboard Replies
+//
+#define KBD_REPLY_POR		0xAA    // Power on reset
+#define KBD_REPLY_ACK		0xFA    // Command ACK
+#define KBD_REPLY_RESEND	0xFE    // Command NACK, send command again
+
 /* Wait 200ms for keyboard controller answers */
 #define KBC_TIMEOUT_IN_MS 200
 
 static int kbc_input_buffer_empty(void)
 {
 	u32 timeout;
-	for(timeout = KBC_TIMEOUT_IN_MS; timeout && (inb(0x64) & 0x02); timeout--) {
+	for(timeout = KBC_TIMEOUT_IN_MS; timeout && (inb(KBD_STATUS) & KBD_IBF); timeout--) {
 		mdelay(1);
 	}
 
@@ -46,7 +80,7 @@ 
 static int kbc_output_buffer_full(void)
 {
 	u32 timeout;
-	for(timeout = KBC_TIMEOUT_IN_MS; timeout && ((inb(0x64) & 0x01) == 0); timeout--) {
+	for(timeout = KBC_TIMEOUT_IN_MS; timeout && ((inb(KBD_STATUS) & KBD_OBF) == 0); timeout--) {
 		mdelay(1);
 	}
 
@@ -60,19 +94,51 @@ 
 static int kbc_cleanup_buffers(void)
 {
 	u32 timeout;
-	for(timeout = KBC_TIMEOUT_IN_MS; timeout && (inb(0x64) & 0x03); timeout--) {
+	for(timeout = KBC_TIMEOUT_IN_MS; timeout && (inb(KBD_STATUS) & (KBD_OBF | KBD_IBF)); timeout--) {
 		mdelay(1);
-		inb(0x60);
+		inb(KBD_DATA);
 	}
 
 	if (!timeout) {
 		printk_err("Couldn't cleanup the keyboard controller buffers\n");
-		printk_err("0x64: 0x%x, 0x60: 0x%x\n", inb(0x64), inb(0x60));
+		printk_err("Status (0x64): 0x%x, Buffer (0x60): 0x%x\n",
+				inb(KBD_STATUS), inb(KBD_DATA));
 	}
+
 	return !!timeout;
 }
 
+static int kbc_self_test(void)
+{
+	u8 self_test;
 
+	/* Clean up any junk that might have been in the KBC.
+	 * Both input and output buffers must be empty.
+	 */
+	if (!kbc_cleanup_buffers())
+		return 0;
+
+	/* reset/self test 8042 - send cmd 0xAA */
+	outb(KBC_CMD_SELF_TEST, KBD_COMMAND);
+
+	if (!kbc_output_buffer_full()) {
+		/* There probably is no keyboard controller. */
+		printk_err("Could not reset keyboard controller.\n");
+		return 0;
+	}
+
+	/* read self-test result, 0x55 is returned in the output buffer */
+	self_test = inb(KBD_DATA);
+
+	if (self_test != 0x55) {
+		printk_err("Keyboard Controller self-test failed: 0x%x\n",
+				self_test);
+		return 0;
+	}
+
+	return 1;
+}
+
 static u8 send_keyboard(u8 command)
 {
 	u8 regval = 0;
@@ -80,47 +146,33 @@ 
 
 	do {
 		if (!kbc_input_buffer_empty()) return 0;
-		outb(command, 0x60);
+		outb(command, KBD_DATA);
 		if (!kbc_output_buffer_full()) {
 			printk_err("Could not send keyboard command %02x\n",
 					command);
 			return 0;
 		}
-		regval = inb(0x60);
+		regval = inb(KBD_DATA);
 		--resend;
 	} while (regval == 0xFE && resend > 0);
 
 	return regval;
 }
 
-
 static void pc_keyboard_init(struct pc_keyboard *keyboard)
 {
 	u8 regval;
 	printk_debug("Keyboard init...\n");
 
-	/* clean up any junk that might have been in the kbc */
-	if (!kbc_cleanup_buffers()) return;
-
-	/* reset/self test 8042 - send cmd 0xAA */
-	if (!kbc_input_buffer_empty()) return;
-	outb(0xAA, 0x64);
-	if (!kbc_output_buffer_full()) {
-		printk_err("Could not reset keyboard controller.\n");
+	/* Run a keyboard controller self-test */
+	if (!kbc_self_test())
 		return;
-	}
 
-	/* read self-test result, 0x55 is returned in the output buffer (0x60) */
-	if ((regval = inb(0x60) != 0x55)) {
-		printk_err("Keyboard Controller self-test failed: 0x%x\n", regval);
-		return;
-	}
-
 	/* Enable keyboard interface - No IRQ */
 	if (!kbc_input_buffer_empty()) return;
-	outb(0x60, 0x64);
+	outb(0x60, KBD_COMMAND);
 	if (!kbc_input_buffer_empty()) return;
-	outb(0x20, 0x60);	/* send cmd: enable keyboard */
+	outb(0x20, KBD_DATA);	/* send cmd: enable keyboard */
 	if (!kbc_input_buffer_empty()) {
 		printk_info("Timeout while enabling keyboard\n");
 		return;
@@ -135,12 +187,13 @@ 
 		printk_err("Keyboard selftest failed ACK: 0x%x\n", regval);
 		return;
 	}
+
 	if (!kbc_output_buffer_full()) {
 		printk_err("Timeout waiting for keyboard after reset.\n");
 		return;
 	}
-		
-	regval = inb(0x60);
+
+	regval = inb(KBD_DATA);
 	if (regval != 0xAA) {
 		printk_err("Keyboard selftest failed: 0x%x\n", regval);
 		return;
@@ -180,10 +233,10 @@ 
 
 	/* All is well - enable keyboard interface */
 	if (!kbc_input_buffer_empty()) return;
-	outb(0x60, 0x64);
+	outb(0x60, KBD_COMMAND);
 	if (!kbc_input_buffer_empty()) return;
-	outb(0x61, 0x60);	/* send cmd: enable keyboard and IRQ 1 */
-	if (!kbc_input_buffer_empty()) {
+	outb(0x61, KBD_DATA);	/* send cmd: enable keyboard and IRQ 1 */
+	if (kbc_output_buffer_full()) {
 		printk_err("Timeout during final keyboard enable\n");
 		return;
 	}
@@ -194,6 +247,8 @@ 
 {
 	if ((port0 == 0x60) && (port1 == 0x64)) {
 		pc_keyboard_init(kbd);
+	} else {
+		printk_warning("Unsupported keyboard controller.\n");
 	}
 }
 
@@ -204,24 +259,16 @@ 
  */
 void set_kbc_ps2_mode(void)
 {
-	/* clean up any junk that might have been in the kbc */
-	if (!kbc_cleanup_buffers()) return;
-
-	/* reset/self test 8042 before we can do anything */
-	if (!kbc_input_buffer_empty()) return;
-	outb(0xAA, 0x64);
-	if (!kbc_output_buffer_full()) return;
-
-	/* read self-test result, 0x55 is returned in the output buffer (0x60) */
-	if ((inb(0x60) != 0x55)) {
-		printk_err("Keyboard Controller selftest failed\n");
+	/* Run a keyboard controller self-test */
+	if (!kbc_self_test())
 		return;
-	}
 
 	/* Support PS/2 mode */
 	if (!kbc_input_buffer_empty()) return;
-	outb(0xcb, 0x64);
+	outb(0xcb, KBD_COMMAND);
+
 	if (!kbc_input_buffer_empty()) return;
-	outb(0x01, 0x60);
+	outb(0x01, KBD_DATA);
+
 	kbc_cleanup_buffers();
 }