[PATCH] Add support for the Mid 2011 MacBook Air keyboard layout

[PATCH] Add support for the Mid 2011 MacBook Air keyboard layout

[Dirk Hohndel]

When adding support for the MacBookAir4,x models in commit
5d922baa631058c7e37ae33e81c4d3e6437f8d1d Joshua Dillon overlooked
that they have a slightly different keyboard layout compared to the older
MacBook Air models. With this patch the function keys all work as
advertized, including keyboard brightness, etc.

Signed-off-by: Dirk Hohndel <hohndel@infradead.org>
---
 drivers/hid/hid-apple.c |   26 +++++++++++++++++++++++++-
 1 files changed, 25 insertions(+), 1 deletions(-)

diff --git a/drivers/hid/hid-apple.c b/drivers/hid/hid-apple.c
index 8cdb4b4..57489a2 100644
--- a/drivers/hid/hid-apple.c
+++ b/drivers/hid/hid-apple.c
@@ -82,6 +82,30 @@ static const struct apple_key_translation macbookair_fn_keys[] = {
 	{ }
 };
 
+/* MacBook Air Mid2011 
+ * (dmidecode identifies as MacBookAir4,1 (11") and 4,2 (13")) */
+static const struct apple_key_translation macbookair4_x_fn_keys[] = {
+	{ KEY_BACKSPACE, KEY_DELETE },
+	{ KEY_ENTER,	KEY_INSERT },
+	{ KEY_F1,	KEY_BRIGHTNESSDOWN, APPLE_FLAG_FKEY },
+	{ KEY_F2,	KEY_BRIGHTNESSUP,   APPLE_FLAG_FKEY },
+	{ KEY_F3,	KEY_SCALE,          APPLE_FLAG_FKEY },
+	{ KEY_F4,	KEY_DASHBOARD,      APPLE_FLAG_FKEY },
+	{ KEY_F5,	KEY_KBDILLUMDOWN,   APPLE_FLAG_FKEY },
+	{ KEY_F6,	KEY_KBDILLUMUP,     APPLE_FLAG_FKEY },
+	{ KEY_F7,	KEY_PREVIOUSSONG,   APPLE_FLAG_FKEY },
+	{ KEY_F8,	KEY_PLAYPAUSE,      APPLE_FLAG_FKEY },
+	{ KEY_F9,	KEY_NEXTSONG,       APPLE_FLAG_FKEY },
+	{ KEY_F10,	KEY_MUTE,           APPLE_FLAG_FKEY },
+	{ KEY_F11,	KEY_VOLUMEDOWN,     APPLE_FLAG_FKEY },
+	{ KEY_F12,	KEY_VOLUMEUP,       APPLE_FLAG_FKEY },
+	{ KEY_UP,	KEY_PAGEUP },
+	{ KEY_DOWN,	KEY_PAGEDOWN },
+	{ KEY_LEFT,	KEY_HOME },
+	{ KEY_RIGHT,	KEY_END },
+	{ }
+};
+
 static const struct apple_key_translation apple_fn_keys[] = {
 	{ KEY_BACKSPACE, KEY_DELETE },
 	{ KEY_ENTER,	KEY_INSERT },
@@ -185,7 +209,7 @@ static int hidinput_apple_event(struct hid_device *hid, struct input_dev *input,
 			table = macbookair_fn_keys;
 		else if (hid->product >= USB_DEVICE_ID_APPLE_WELLSPRING6_ANSI &&
 				hid->product <= USB_DEVICE_ID_APPLE_WELLSPRING6_JIS)
-			table = macbookair_fn_keys;
+			table = macbookair4_x_fn_keys;
 		else if (hid->product < 0x21d || hid->product >= 0x300)
 			table = powerbook_fn_keys;
 		else
-- 
1.7.7

Re: [PATCH] Add support for the Mid 2011 MacBook Air keyboard layout

[Linus Torvalds]

On Sun, Nov 6, 2011 at 9:16 AM, Dirk Hohndel <hohndel@infradead.org> wrote:
>
> When adding support for the MacBookAir4,x models in commit
> 5d922baa631058c7e37ae33e81c4d3e6437f8d1d Joshua Dillon overlooked
> that they have a slightly different keyboard layout compared to the older
> MacBook Air models. With this patch the function keys all work as
> advertized, including keyboard brightness, etc.
>
> Signed-off-by: Dirk Hohndel <hohndel@infradead.org>

Hmm. I think this is wrong.

Why?

Because it works unmodified for me on my MacBook Air.

Now, I have the 11" one which apparently has a different keyboard ID,
but the *reason* it works for me is that my keyboard ID is apparently
0x0249, which is USB_DEVICE_ID_APPLE_WELLSPRING6A_ANSI.

And because that one isn't known by the broken quirks at all, it falls
back on the standard apple_fn_keys[] translation.

Which is the *right* translation for me, and judging by your patch,
it's the right translation for you too.

So I suspect that the *right* thing to do is to just remove the broken
WELLSPRING6 cases. Exactly the same way that da617c7cb915 ("HID:
consolidate MacbookAir 4,1 mappings") did for the WELLSPRING6A ones.

So I suspect the only case that should use the "macbookair_fn_keys"
thing is the *old* macbook air ("WELLSPRING4"). They were added in
commit 5d922baa6310, and that commit implies that it was never tested
all that much ("reportedly unchanged from the 3,2 predecessor and
seems to work well.")

So I think the right patch is just this one:

     drivers/hid/hid-apple.c |    3 ---
     1 files changed, 0 insertions(+), 3 deletions(-)

    diff --git a/drivers/hid/hid-apple.c b/drivers/hid/hid-apple.c
    index 8cdb4b45b30a..299d23871122 100644
    --- a/drivers/hid/hid-apple.c
    +++ b/drivers/hid/hid-apple.c
    @@ -183,9 +183,6 @@ static int hidinput_apple_event(struct
hid_device *hid, struct input_dev *input,
     		if (hid->product >= USB_DEVICE_ID_APPLE_WELLSPRING4_ANSI &&
     				hid->product <= USB_DEVICE_ID_APPLE_WELLSPRING4A_JIS)
     			table = macbookair_fn_keys;
    -		else if (hid->product >= USB_DEVICE_ID_APPLE_WELLSPRING6_ANSI &&
    -				hid->product <= USB_DEVICE_ID_APPLE_WELLSPRING6_JIS)
    -			table = macbookair_fn_keys;
     		else if (hid->product < 0x21d || hid->product >= 0x300)
     			table = powerbook_fn_keys;
     		else

instead. Hmm? Does that work for you?

               Linus

Re: [PATCH] Add support for the Mid 2011 MacBook Air keyboard layout

[Dirk Hohndel]

On Sun, 6 Nov 2011 12:12:49 -0800, Linus Torvalds <torvalds@linux-foundation.org> wrote:
> Now, I have the 11" one which apparently has a different keyboard ID,
> but the *reason* it works for me is that my keyboard ID is apparently
> 0x0249, which is USB_DEVICE_ID_APPLE_WELLSPRING6A_ANSI.
> 
> And because that one isn't known by the broken quirks at all, it falls
> back on the standard apple_fn_keys[] translation.
> 
> Which is the *right* translation for me, and judging by your patch,
> it's the right translation for you too.

Correct. 
 
> So I suspect that the *right* thing to do is to just remove the broken
> WELLSPRING6 cases. Exactly the same way that da617c7cb915 ("HID:
> consolidate MacbookAir 4,1 mappings") did for the WELLSPRING6A ones.
> 
> So I suspect the only case that should use the "macbookair_fn_keys"
> thing is the *old* macbook air ("WELLSPRING4"). They were added in
> commit 5d922baa6310, and that commit implies that it was never tested
> all that much ("reportedly unchanged from the 3,2 predecessor and
> seems to work well.")
> 
> So I think the right patch is just this one:
> 
>      drivers/hid/hid-apple.c |    3 ---
>      1 files changed, 0 insertions(+), 3 deletions(-)
> 
>     diff --git a/drivers/hid/hid-apple.c b/drivers/hid/hid-apple.c
>     index 8cdb4b45b30a..299d23871122 100644
>     --- a/drivers/hid/hid-apple.c
>     +++ b/drivers/hid/hid-apple.c
>     @@ -183,9 +183,6 @@ static int hidinput_apple_event(struct
> hid_device *hid, struct input_dev *input,
>      		if (hid->product >= USB_DEVICE_ID_APPLE_WELLSPRING4_ANSI &&
>      				hid->product <= USB_DEVICE_ID_APPLE_WELLSPRING4A_JIS)
>      			table = macbookair_fn_keys;
>     -		else if (hid->product >= USB_DEVICE_ID_APPLE_WELLSPRING6_ANSI &&
>     -				hid->product <= USB_DEVICE_ID_APPLE_WELLSPRING6_JIS)
>     -			table = macbookair_fn_keys;
>      		else if (hid->product < 0x21d || hid->product >= 0x300)
>      			table = powerbook_fn_keys;
>      		else
> 
> instead. Hmm? Does that work for you?

Tested-by: Dirk Hohndel <hohndel@infradead.org>

/D