How to add support for Macbook Pro (USB) keyboard?

[Christoph Langguth]

Hi Hans Petter,

thanks for the quick reply!
I tried your patch but it doesn't work, all I get is:

May 16 14:00:01 marvin kernel: ukbd0: <Apple Internal Keyboard> on usbus5
May 16 14:00:01 marvin kernel: device_attach: ukbd0 attach returned 6

I don't know why this happens... I tried, but unfortunately I didn't 
quite understand what the patch is supposed to do :-)


In the meantime, I had continued writing up a slightly cleaner patch, 
that also allows to fully utilize the keyboard. Some considerations first:

- Quite a few apple keyboards (mine included) have an additional quirk 
in that they switch two keys. (On [1], that's all the ones having the 
APPLE_ISO_KEYBOARD flag). But of course, it's not all of them :-(

- To fully make use of the keyboard, one also needs to consider the Fn 
key ("part of" the 10th (9th payload) byte), so as to be able to be able 
to type "vital" keys like DEL, INS, PGUP etc. Judging from [1], 
everything from Apple that actually is a keyboard also has this FN key, 
so I would expect all of these keyboards to send that 9th byte

- It seems that some, but not all, of those keyboards, send this first 
byte (the one you called hid_id)


In light of the above, would it make sense to
- try to detect the HID ID byte in the manner that you suggested in your 
patch (assuming that I can figure out the logic, and what's going wrong 
there, I think it's the cleanest and vendor-independent solution)
- add an additional UKBD-specific quirk for the ISO_KEYBOARD problem 
(into usb/quirks/...)
- assume that any available 9th byte contains the Fn key information -- 
or would yet another quirk, listing all affected products, be more reliable?

[1]: https://www.linuxhq.com/kernel/v2.6/28-rc8/drivers/hid/hid-apple.c

In any case, I have attached the "full" patch that works for me -- as 
stated above, it's not as clean as it should be because it only works 
for my specific piece of hardware and doesn't consider the latest 
questions/proposed solution... but it already contains all the required 
logic so that you can see what I meant.

Again, thanks for your help!
Cheers
Chris

Hans Petter Selasky wrote:
> On Friday 15 May 2009, Christoph Langguth wrote:
>> Christoph Langguth
> 
> Hi,
> 
> Can you try the following patch on 8-current?
> 
> http://perforce.freebsd.org/chv.cgi?CH=162145
> 
> --HPS
> 

-------------- next part --------------
--- ukbd.c.org	2009-05-16 13:49:48.000000000 +0000
+++ ukbd.c	2009-05-16 14:39:35.000000000 +0000
@@ -162,6 +162,10 @@
 	uint8_t	sc_leds;		/* store for async led requests */
 	uint8_t	sc_iface_index;
 	uint8_t	sc_iface_no;
+
+	uint8_t quirks;
+#define UKBD_QUIRK_APPLE 1
+#define UKBD_QUIRK_SWAPKEYS 2
 };
 
 #define	KEY_ERROR	  0x01
@@ -464,23 +468,63 @@
 	}
 }
 
+static uint8_t
+apple_fn(uint8_t keycode) {
+	switch (keycode) {
+		case 0x2a: return 0x4c; // BACKSPACE -> DEL
+		case 0x2c: return 0x49; // SPACE -> INSERT
+		case 0x50: return 0x4a; // LEFT ARROW -> HOME
+		case 0x4f: return 0x4d; // RIGHT ARROW -> END
+		case 0x52: return 0x4b; // UP ARROW -> PGUP
+		case 0x51: return 0x4e; // DOWN ARROW -> PGDN
+		default: return keycode;
+	}
+}
+
 static void
 ukbd_intr_callback(struct usb2_xfer *xfer)
 {
 	struct ukbd_softc *sc = xfer->priv_sc;
 	uint16_t len = xfer->actlen;
 	uint8_t i;
+	uint8_t offset = 0;
+	uint8_t fnmode = 0;
+#define UKBD_FNMODE_FN 2
+#define UKBD_FNMODE_EJECT 1
 
 	switch (USB_GET_STATE(xfer)) {
 	case USB_ST_TRANSFERRED:
 		DPRINTF("actlen=%d bytes\n", len);
 
 		if (len > sizeof(sc->sc_ndata)) {
+			if (sc->quirks & UKBD_QUIRK_APPLE) {
+				offset = 1;
+				if (len >= 10) {
+					usb2_copy_out(xfer->frbuffers, 9, &fnmode, 1);
+				}
+			}
 			len = sizeof(sc->sc_ndata);
 		}
 		if (len) {
 			bzero(&sc->sc_ndata, sizeof(sc->sc_ndata));
-			usb2_copy_out(xfer->frbuffers, 0, &sc->sc_ndata, len);
+			usb2_copy_out(xfer->frbuffers, offset, &sc->sc_ndata, len);
+
+			if (sc->quirks & UKBD_QUIRK_APPLE) {
+				for (i = 0; i < UKBD_NKEYCODE; i++) {
+					if ((sc->quirks & UKBD_QUIRK_SWAPKEYS)
+					    && (sc->sc_ndata.keycode[i] == 0x35)) {
+						sc->sc_ndata.keycode[i] = 0x64;
+					}
+					else if ((sc->quirks & UKBD_QUIRK_SWAPKEYS)
+					    && (sc->sc_ndata.keycode[i] == 0x64)) {
+						sc->sc_ndata.keycode[i] = 0x35;
+					}
+					if (fnmode & UKBD_FNMODE_FN) {
+						sc->sc_ndata.keycode[i] = apple_fn(sc->sc_ndata.keycode[i]);
+					}
+				}
+			}
+
 #if USB_DEBUG
 			if (sc->sc_ndata.modifiers) {
 				DPRINTF("mod: 0x%04x\n", sc->sc_ndata.modifiers);
@@ -638,6 +682,12 @@
 	sc->sc_mode = K_XLATE;
 	sc->sc_iface = uaa->iface;
 
+	if ((uaa->info.idVendor == 0x5ac)
+	    && (uaa->info.idProduct == 0x231)) {
+		DPRINTF("Activating Apple kbd quirk\n");
+		sc->quirks = UKBD_QUIRK_APPLE | UKBD_QUIRK_SWAPKEYS;
+	}
+
 	usb2_callout_init_mtx(&sc->sc_callout, &Giant, 0);
 
 	err = usb2_transfer_setup(uaa->device,