Bug 59698 - [keyboard] [patch] Rework of ukbd HID to AT code translation
Summary: [keyboard] [patch] Rework of ukbd HID to AT code translation
Status: Open
Alias: None
Product: Base System
Classification: Unclassified
Component: usb (show other bugs)
Version: Unspecified
Hardware: Any Any
: Normal Affects Only Me
Assignee: freebsd-usb (Nobody)
URL:
Keywords: patch
Depends on:
Blocks:
 
Reported: 2003-11-26 08:00 UTC by Andrew Hesford
Modified: 2022-10-17 12:38 UTC (History)
1 user (show)

See Also:


Attachments
file.diff (10.27 KB, patch)
2003-11-26 08:00 UTC, Andrew Hesford
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Andrew Hesford 2003-11-26 08:00:44 UTC
The existing USB keyboard implementation contains a haphazard mechanism
for translating USB HID codes to AT scancodes. In ukbd.c there is a
table mapping a subset of the HID codes to keycodes, and a function
converts these keycodes to AT scancodes for feeding into the keyboard
driver. Because it relies on keycodes, scancodes that don't map to an
existing keycode can not be created by the ukbd driver.

My new implementation removes this dependence on keycodes, and instead
relies on a table (pulled from a Microsoft URL in the NetBSD ukbd.c)
mapping USB HID codes directly to AT scancodes. As a result, things such
as the Macintosh keypad equal key now produces a valid scancode,
whereas before it generated no code at all. This table may be found at

	http://www.microsoft.com/HWDEV/TECH/input/Scancode.asp

Furthermore, this new implementation removes the keycode2scancode
function, which was responsible for back-converting keycodes to AT
scancodes. This saves a function call and some computation per keypress.

The implementation is not complete, but is meant to be a step towards a
fully-mapped USB keyboard. At present I only map the HID Usage IDs from
Usage Page 07. While this includes all they keys on a 104-key keyboard
and then some, certain useful keys (like multimedia keys) are unmapped.
When I have access to a USB keyboard with such keys, I can look into
supporting them.

I think this code is commit-ready even though it is not a full
implementation. The driver has greater functionality than before (for
example, the keypad equal key generates a scancode now) and increased
efficiency. From my testing, there are no new bugs, and maybe fewer old
ones.

Note: some keys that now generate valid scancodes still don't behave as
expected. This is a limitation of the AT keyboard driver, not the USB
driver. The keypad equal key I keep mentioning is supposed to generate
an AT scancode 0x59, and it does, but the AT keyboard driver maps the
keypad enter scan code (0xE0 0x1C) to the keycode 0x59. Hence, pressing
keypad equal is the same as pressing keypad enter as far as the console
is concerned. To correct this, the AT keyboard driver (and perhaps
syscons) will need to be updated.

How-To-Repeat: There is nothing to repeat.
Comment 1 Andrew Hesford 2003-11-26 22:42:52 UTC
Well, it would just so happen that I have to eat my words here.
Functionality WAS lost in my patch: the arrow and editing keys behaved
like the numeric keypad. That is, when numlock was enabled, they sent
numbers to the console. This was not the case in XFree86, leading me to
believe the problem was only present when the keyboard was not in K_RAW
or K_CODE mode.

This turned out to be the case. The call to genkbd_keyaction() in
ukbd_read_char() requires the keycode, not the scancode, so sending
codes with an 0xE0 prefix through caused problems.

Unfortunately, since atkbd maps the numeric keypad scancodes to lower
keycodes, there is no easy way around this. I simply added a switch on
my scancode variable, and manually replace the 0xE0-prefixed scancodes
with appropriate keycodes. Now everything really does seem to work as
well as the original implementation, but still without keycode2scancode.

The proper way to handle all of this is to overhaul atkbd. Scancodes of
the form 0xE0 0xYY, where YY is any hex number, should be mapped
directly to 0x1YY, rather than the odd mapping that exists now.
Scancodes without the 0xE0 prefix, of the form 0xWW, should be mapped
directly to keycode 0x0WW. This way, atkbd makes room for extra keys
(like the Macintosh keypad equal that started this whole thing) that
have low scancodes. Once atkbd has a straight-through keycode mapper
(would syscons need to be updated too?), calling genkbd_keyaction() from
ukbd would require only the scancode from the HID-PS2 translation table.
My switch block would become obsolete.

Unfortunately, I don't have the time to learn enough about atkbd and its
dependends to overhaul it. All I can say is that I'd be extremely
grateful to anybody who takes on the task. I want my Mac USB keyboard
working flawlessly on the console as well as in XFre86.

The following ukbd patch handles the 0xE0 prefixes so that the console
treats the numeric keypad differently than the editing keys:

--- ukbd.c.orig	Tue Nov 25 14:12:35 2003
+++ ukbd.c	Wed Nov 26 16:20:46 2003
@@ -275,54 +275,55 @@
 	{ MOD_WIN_R,	 0xe7 },
 };
 
-#define NN 0			/* no translation */
+#define NN 0		/* no translation */
 /*
  * Translate USB keycodes to AT keyboard scancodes.
  */
-/*
- * FIXME: Mac USB keyboard generates:
- * 0x53: keypad NumLock/Clear
- * 0x66: Power
- * 0x67: keypad =
- * 0x68: F13
- * 0x69: F14
- * 0x6a: F15
- */
-Static u_int8_t ukbd_trtab[256] = {
-	   0,   0,   0,   0,  30,  48,  46,  32, /* 00 - 07 */
-	  18,  33,  34,  35,  23,  36,  37,  38, /* 08 - 0F */
-	  50,  49,  24,  25,  16,  19,  31,  20, /* 10 - 17 */
-	  22,  47,  17,  45,  21,  44,   2,   3, /* 18 - 1F */
-	   4,   5,   6,   7,   8,   9,  10,  11, /* 20 - 27 */
-	  28,   1,  14,  15,  57,  12,  13,  26, /* 28 - 2F */
-	  27,  43,  43,  39,  40,  41,  51,  52, /* 30 - 37 */
-	  53,  58,  59,  60,  61,  62,  63,  64, /* 38 - 3F */
-	  65,  66,  67,  68,  87,  88,  92,  70, /* 40 - 47 */
-	 104, 102,  94,  96, 103,  99, 101,  98, /* 48 - 4F */
-	  97, 100,  95,  69,  91,  55,  74,  78, /* 50 - 57 */
-	  89,  79,  80,  81,  75,  76,  77,  71, /* 58 - 5F */
-          72,  73,  82,  83,  86, 107,  NN,  NN, /* 60 - 67 */
-          NN,  NN,  NN,  NN,  NN,  NN,  NN,  NN, /* 68 - 6F */
-          NN,  NN,  NN,  NN,  NN,  NN,  NN,  NN, /* 70 - 77 */
-          NN,  NN,  NN,  NN,  NN,  NN,  NN,  NN, /* 78 - 7F */
-          NN,  NN,  NN,  NN,  NN,  NN,  NN, 115, /* 80 - 87 */
-         112, 125, 121, 123,  NN,  NN,  NN,  NN, /* 88 - 8F */
-          NN,  NN,  NN,  NN,  NN,  NN,  NN,  NN, /* 90 - 97 */
-          NN,  NN,  NN,  NN,  NN,  NN,  NN,  NN, /* 98 - 9F */
-          NN,  NN,  NN,  NN,  NN,  NN,  NN,  NN, /* A0 - A7 */
-          NN,  NN,  NN,  NN,  NN,  NN,  NN,  NN, /* A8 - AF */
-          NN,  NN,  NN,  NN,  NN,  NN,  NN,  NN, /* B0 - B7 */
-          NN,  NN,  NN,  NN,  NN,  NN,  NN,  NN, /* B8 - BF */
-          NN,  NN,  NN,  NN,  NN,  NN,  NN,  NN, /* C0 - C7 */
-          NN,  NN,  NN,  NN,  NN,  NN,  NN,  NN, /* C8 - CF */
-          NN,  NN,  NN,  NN,  NN,  NN,  NN,  NN, /* D0 - D7 */
-          NN,  NN,  NN,  NN,  NN,  NN,  NN,  NN, /* D8 - DF */
-          29,  42,  56, 105,  90,  54,  93, 106, /* E0 - E7 */
-          NN,  NN,  NN,  NN,  NN,  NN,  NN,  NN, /* E8 - EF */
-          NN,  NN,  NN,  NN,  NN,  NN,  NN,  NN, /* F0 - F7 */
-          NN,  NN,  NN,  NN,  NN,  NN,  NN,  NN, /* F8 - FF */
+Static u_int16_t ukbd_trtab[256] = {
+	   NN,    NN,    NN,    NN, 0x01E, 0x030, 0x02E, 0x020, /* 00-07 */
+	0x012, 0x021, 0x022, 0x023, 0x017, 0x024, 0x025, 0x026, /* 08-0F */
+	0x032, 0x031, 0x018, 0x019, 0x010, 0x013, 0x01F, 0x014, /* 10-17 */
+	0x016, 0x02F, 0x011, 0x02D, 0x015, 0x02C, 0x002, 0x003, /* 18-1F */
+	0x004, 0x005, 0x006, 0x007, 0x008, 0x009, 0x00A, 0x00B, /* 20-27 */
+	0x01C, 0x001, 0x00E, 0x00F, 0x039, 0x00C, 0x00D, 0x01A, /* 28-2F */
+	0x01B, 0x02B, 0x02B, 0x027, 0x028, 0x029, 0x033, 0x034, /* 30-37 */
+	0x035, 0x03A, 0x03B, 0x03C, 0x03D, 0x03E, 0x03F, 0x040, /* 38-3F */
+	0x041, 0x042, 0x043, 0x044, 0x057, 0x058, 0x137, 0x046, /* 40-47 */
+	/* FIX BREAK, PAUSE (48): 0x0FF for handling later. */
+	0x0FF, 0x152, 0x147, 0x149, 0x153, 0x14F, 0x151, 0x14D, /* 48-4F */
+	0x14B, 0x150, 0x148, 0x045, 0x135, 0x037, 0x04A, 0x04E, /* 50-57 */
+	0x11C, 0x04F, 0x050, 0x051, 0x04B, 0x04C, 0x04D, 0x047, /* 58-5F */
+	0x048, 0x049, 0x052, 0x053, 0x056, 0x15D,    NN, 0x059, /* 60-67 */
+	0x05D, 0x05E, 0x05F,    NN,    NN,    NN,    NN,    NN, /* 68-6F */
+	   NN,    NN,    NN,    NN,    NN,    NN,    NN,    NN, /* 70-77 */
+	   NN,    NN,    NN,    NN,    NN,    NN,    NN,    NN, /* 78-7F */
+	   NN,    NN,    NN,    NN,    NN, 0x07E,    NN, 0x073, /* 80-87 */
+	0x070, 0x07D, 0x079, 0x07B, 0x05C,    NN,    NN,    NN, /* 88-8F */
+	/* FIX 90, 91: scancodes 0x0F? lost in translation to keycode.
+	 * Hence we just make them no-ops.
+	 */
+	   NN,    NN, 0x078, 0x077, 0x076,    NN,    NN,    NN, /* 90-97 */
+	   NN,    NN,    NN,    NN,    NN,    NN,    NN,    NN, /* 98-9F */
+	   NN,    NN,    NN,    NN,    NN,    NN,    NN,    NN, /* A0-A7 */
+	   NN,    NN,    NN,    NN,    NN,    NN,    NN,    NN, /* A8-AF */
+	   NN,    NN,    NN,    NN,    NN,    NN,    NN,    NN, /* B0-B7 */
+	   NN,    NN,    NN,    NN,    NN,    NN,    NN,    NN, /* B8-BF */
+	   NN,    NN,    NN,    NN,    NN,    NN,    NN,    NN, /* C0-C7 */
+	   NN,    NN,    NN,    NN,    NN,    NN,    NN,    NN, /* C8-CF */
+	   NN,    NN,    NN,    NN,    NN,    NN,    NN,    NN, /* D0-D7 */
+	   NN,    NN,    NN,    NN,    NN,    NN,    NN,    NN, /* D8-DF */
+	0x01D, 0x02A, 0x038, 0x15B, 0x11D, 0x036, 0x138, 0x15C, /* E0-E7 */
+	   NN,    NN,    NN,    NN,    NN,    NN,    NN,    NN, /* E8-EF */
+	   NN,    NN,    NN,    NN,    NN,    NN,    NN,    NN, /* F0-F7 */
+	   NN,    NN,    NN,    NN,    NN,    NN,    NN,    NN  /* F8-FF */
 };
 
+#define PAUSEFIX(c,d) if (((c) == 0x0FF) && !((d) & (MOD_CONTROL_L | MOD_CONTROL_R))) { \
+			(c) = 0x045 | SCAN_PREFIX_E1 | SCAN_PREFIX_CTL; }
+#define SHIFTFIX(c,d) if ((d) & (MOD_SHIFT_L | MOD_SHIFT_R)) { \
+			(c) &= ~SCAN_PREFIX_SHIFT; }
+#define KEYUP(c,d) (c) |= ((d) ? SCAN_RELEASE : SCAN_PRESS)
+
 typedef struct ukbd_state {
 	usbd_interface_handle ks_iface;	/* interface */
 	usbd_pipe_handle ks_intrpipe;	/* interrupt pipe */
@@ -413,9 +414,6 @@
 				      int flags);
 Static void		set_leds(ukbd_state_t *state, int leds);
 Static int		set_typematic(keyboard_t *kbd, int code);
-#ifdef UKBD_EMULATE_ATSCANCODE
-Static int		keycode2scancode(int keycode, int shift, int up);
-#endif
 
 /* local variables */
 
@@ -881,7 +879,6 @@
 	ukbd_state_t *state;
 	int usbcode;
 #ifdef UKBD_EMULATE_ATSCANCODE
-	int keycode;
 	int scancode;
 #endif
 
@@ -906,12 +903,14 @@
 		return -1;
 	++kbd->kb_count;
 #ifdef UKBD_EMULATE_ATSCANCODE
-	keycode = ukbd_trtab[KEY_INDEX(usbcode)];
-	if (keycode == NN)
+	scancode = ukbd_trtab[KEY_INDEX(usbcode)];
+	if (scancode == NN)
 		return -1;
 
-	scancode = keycode2scancode(keycode, state->ks_ndata.modifiers,
-				    usbcode & KEY_RELEASE);
+	PAUSEFIX(scancode,state->ks_ndata.modifiers);
+	SHIFTFIX(scancode,state->ks_ndata.modifiers);
+	KEYUP(scancode,usbcode & KEY_RELEASE);
+
 	if (scancode & SCAN_PREFIX) {
 		if (scancode & SCAN_PREFIX_CTL) {
 			state->ks_buffered_char[0] =
@@ -956,10 +955,7 @@
 	ukbd_state_t *state;
 	u_int action;
 	int usbcode;
-	int keycode;
-#ifdef UKBD_EMULATE_ATSCANCODE
 	int scancode;
-#endif
 
 	state = (ukbd_state_t *)kbd->kb_data;
 next_code:
@@ -1000,14 +996,15 @@
 
 #ifdef UKBD_EMULATE_ATSCANCODE
 	/* USB key index -> key code -> AT scan code */
-	keycode = ukbd_trtab[KEY_INDEX(usbcode)];
-	if (keycode == NN)
+	scancode = ukbd_trtab[KEY_INDEX(usbcode)];
+	if (scancode == NN)
 		return NOKEY;
 
 	/* return an AT scan code for the K_RAW mode */
 	if (state->ks_mode == K_RAW) {
-		scancode = keycode2scancode(keycode, state->ks_ndata.modifiers,
-					    usbcode & KEY_RELEASE);
+		PAUSEFIX(scancode,state->ks_ndata.modifiers);
+		SHIFTFIX(scancode,state->ks_ndata.modifiers);
+		KEYUP(scancode,usbcode&KEY_RELEASE);
 		if (scancode & SCAN_PREFIX) {
 			if (scancode & SCAN_PREFIX_CTL) {
 				state->ks_buffered_char[0] =
@@ -1034,12 +1031,12 @@
 		return usbcode;
 
 	/* USB key index -> key code */
-	keycode = ukbd_trtab[KEY_INDEX(usbcode)];
-	if (keycode == NN)
+	scancode = ukbd_trtab[KEY_INDEX(usbcode)];
+	if (scancode == NN)
 		return NOKEY;
 #endif /* UKBD_EMULATE_ATSCANCODE */
 
-	switch (keycode) {
+	switch (scancode) {
 	case 0x38:	/* left alt (compose key) */
 		if (usbcode & KEY_RELEASE) {
 			if (state->ks_flags & COMPOSE) {
@@ -1055,41 +1052,41 @@
 		}
 		break;
 	/* XXX: I don't like these... */
-	case 0x5c:	/* print screen */
+	case 0x137:	/* print screen */
 		if (state->ks_flags & ALTS)
-			keycode = 0x54;	/* sysrq */
+			scancode = 0x54;	/* sysrq */
 		break;
-	case 0x68:	/* pause/break */
+	case 0xFF:	/* pause/break */
 		if (state->ks_flags & CTLS)
-			keycode = 0x6c;	/* break */
+			scancode = 0x6c;	/* break */
 		break;
 	}
 
 	/* return the key code in the K_CODE mode */
 	if (usbcode & KEY_RELEASE)
-		keycode |= SCAN_RELEASE;
+		scancode |= SCAN_RELEASE;
 	if (state->ks_mode == K_CODE)
-		return keycode;
+		return scancode;
 
 	/* compose a character code */
 	if (state->ks_flags & COMPOSE) {
-		switch (keycode) {
+		switch (scancode) {
 		/* key pressed, process it */
 		case 0x47: case 0x48: case 0x49:	/* keypad 7,8,9 */
 			state->ks_composed_char *= 10;
-			state->ks_composed_char += keycode - 0x40;
+			state->ks_composed_char += scancode - 0x40;
 			if (state->ks_composed_char > UCHAR_MAX)
 				return ERRKEY;
 			goto next_code;
 		case 0x4B: case 0x4C: case 0x4D:	/* keypad 4,5,6 */
 			state->ks_composed_char *= 10;
-			state->ks_composed_char += keycode - 0x47;
+			state->ks_composed_char += scancode - 0x47;
 			if (state->ks_composed_char > UCHAR_MAX)
 				return ERRKEY;
 			goto next_code;
 		case 0x4F: case 0x50: case 0x51:	/* keypad 1,2,3 */
 			state->ks_composed_char *= 10;
-			state->ks_composed_char += keycode - 0x4E;
+			state->ks_composed_char += scancode - 0x4E;
 			if (state->ks_composed_char > UCHAR_MAX)
 				return ERRKEY;
 			goto next_code;
@@ -1126,8 +1123,75 @@
 	}
 
 	/* keycode to key action */
-	action = genkbd_keyaction(kbd, SCAN_CHAR(keycode),
-				  keycode & SCAN_RELEASE, &state->ks_state,
+	/* have to fix poor mapping of 0xE0 codes */
+	if (scancode & SCAN_PREFIX_E0) {
+		switch (scancode) {
+		case 0x11c:
+			scancode = 89;
+			break;
+		case 0x11d:
+			scancode = 90;
+			break;
+		case 0x135:
+			scancode = 91;
+			break;
+		case 0x137 | SCAN_PREFIX_SHIFT:
+			scancode = 92;
+			break;
+		case 0x138:
+			scancode = 93;
+			break;
+		case 0x147:
+			scancode = 94;
+			break;
+		case 0x148:
+			scancode = 95;
+			break;
+		case 0x149:
+			scancode = 96;
+			break;
+		case 0x14b:
+			scancode = 97;
+			break;
+		case 0x14d:
+			scancode = 98;
+			break;
+		case 0x14f:
+			scancode = 99;
+			break;
+		case 0x150:
+			scancode = 100;
+			break;
+		case 0x151:
+			scancode = 101;
+			break;
+		case 0x152:
+			scancode = 102;
+			break;
+		case 0x153:
+			scancode = 103;
+			break;
+		case 0x146:
+			scancode = 104;
+			break;
+		case 0x15b:
+			scancode = 105;
+			break;
+		case 0x15c:
+			scancode = 106;
+			break;
+		case 0x15d:
+			scancode = 107;
+			break;
+		default:
+			return NOKEY;
+			break;
+		}
+		if (usbcode & KEY_RELEASE)
+			scancode |= SCAN_RELEASE;
+	}
+	action = genkbd_keyaction(kbd, SCAN_CHAR(scancode),
+				  scancode & SCAN_RELEASE, &state->ks_state,
 				  &state->ks_accents);
 	if (action == NOKEY)
 		goto next_code;
@@ -1437,32 +1501,6 @@
 	kbd->kb_delay2 = rates[code & 0x1f];
 	return 0;
 }
-
-#ifdef UKBD_EMULATE_ATSCANCODE
-Static int
-keycode2scancode(int keycode, int shift, int up)
-{
-	static int scan[] = {
-		0x1c, 0x1d, 0x35,
-		0x37 | SCAN_PREFIX_SHIFT, /* PrintScreen */
-		0x38, 0x47, 0x48, 0x49, 0x4b, 0x4d, 0x4f,
-		0x50, 0x51, 0x52, 0x53,
-		0x46, 	/* XXX Pause/Break */
-		0x5b, 0x5c, 0x5d,
-	};
-	int scancode;
-
-	scancode = keycode;
-	if ((keycode >= 89) && (keycode < 89 + sizeof(scan)/sizeof(scan[0])))
-		scancode = scan[keycode - 89] | SCAN_PREFIX_E0;
-	/* Pause/Break */
-	if ((keycode == 104) && !(shift & (MOD_CONTROL_L | MOD_CONTROL_R)))
-		scancode = 0x45 | SCAN_PREFIX_E1 | SCAN_PREFIX_CTL;
-	if (shift & (MOD_SHIFT_L | MOD_SHIFT_R))
-		scancode &= ~SCAN_PREFIX_SHIFT;
-	return (scancode | (up ? SCAN_RELEASE : SCAN_PRESS));
-}
-#endif /* UKBD_EMULATE_ATSCANCODE */
 
 Static int
 ukbd_driver_load(module_t mod, int what, void *arg)
-- 
Andrew Hesford
hesford@uiuc.edu
Comment 2 Mark Linimon freebsd_committer freebsd_triage 2006-06-03 16:46:35 UTC
Responsible Changed
From-To: freebsd-bugs->freebsd-usb

This appears to be USB-related.
Comment 3 Eitan Adler freebsd_committer freebsd_triage 2011-07-24 23:39:53 UTC
Responsible Changed
From-To: freebsd-usb->mav

work
Comment 4 Eitan Adler freebsd_committer freebsd_triage 2017-12-31 08:00:55 UTC
For bugs matching the following criteria:

Status: In Progress Changed: (is less than) 2014-06-01

Reset to default assignee and clear in-progress tags.

Mail being skipped
Comment 5 Graham Perrin freebsd_committer freebsd_triage 2022-10-17 12:38:03 UTC
Keyword: 

    patch
or  patch-ready

– in lieu of summary line prefix: 

    [patch]

* bulk change for the keyword
* summary lines may be edited manually (not in bulk). 

Keyword descriptions and search interface: 

    <https://bugs.freebsd.org/bugzilla/describekeywords.cgi>