hid, 2.6.23 - add support for new macbook and apple aluminum keyboard "fn" key

PATCH In review Chris Nolan 3 participants 11 messages Feb 12, 2008 – Mar 14, 2008

Subsystems (3)

HID libata USB HID/hidbp drivers (USB keyboards, mice, remote controls, ...)

lore

Share & tools

lore

Chris Nolan

author

2008-02-12 19:40 UTC #0 lore raw

PATCH 1/1 hid, 2.6.23 - add support for new macbook and apple aluminum keyboard "fn" key

Hello list.

Seeking feedback for this patch which adds support for the "fn" key on the new macbook (3,1) and apple aluminum keyboard.

I have tested this with kernel 2.6.23 (both 2.6.23.9 and 2.6.23.15) under Fedora 8 and it seems to work for me.

The original patch was created by Martin Szulecki (of mactel-linux) and
Chris Irwin. I can't take any credit - I've just found that it works
and would like to see it tested and committed upstream if possible.

I am linking to the patch because a) its quite long, and b) I am not sure how to prevent thunderbird from wrapping the lines and I'd hate to annoy the list with some malformed garbage.

Patch is here:

https://bugzilla.redhat.com/attachment.cgi?id=294650

Regards
Chris

Jiri Kosina

2008-02-12 20:48 UTC #1 lore raw

Quoted 4 lines · Chris Nolan

Seeking feedback for this patch which adds support for the "fn" key on the new macbook (3,1) and apple aluminum keyboard. I have tested this with kernel 2.6.23 (both 2.6.23.9 and 2.6.23.15) under Fedora 8 and it seems to work for me.

Hi Chris,

Some part of the patch is already in 2.6.25-rc1 -- please see commit a45d82d19a6c2a717bcc33cff243199b77fa0082, which adds support for Apple Aluminium keyboards. So the patch would need some changes that it would apply on top of current tree.

Also, if you could get Signed-off-by for the new patch from the author(s) of the original patch, that would be great.

Quoted 8 lines

The original patch was created by Martin Szulecki (of mactel-linux) and Chris Irwin. I can't take any credit - I've just found that it works and would like to see it tested and committed upstream if possible.
I am linking to the patch because a) its quite long, and b) I am not sure how to prevent thunderbird from wrapping the lines and I'd hate to annoy the list with some malformed garbage.
Patch is here:
https://bugzilla.redhat.com/attachment.cgi?id=294650

Also, Tobias (added to CC) seems to be currently working on adding support for a subset of keyboards that this patch adds. Tobias -- it seems like this patch is handling a superset of what your patch does, right?

It would be great if you could put this all together into one condensated patch which I could merge into my tree.

Thanks guys!

Signature · 2 lines

Jiri Kosina
SUSE Labs

Tobias Müller

2008-02-13 12:45 UTC #2 lore raw

Quoted 1 line · Jiri Kosina

Some part of the patch is already in 2.6.25-rc1 -- please see commit a45d82d19a6c2a717bcc33cff243199b77fa0082, which adds support for Apple Aluminium keyboards. So the patch would need some changes that it would apply on top of current tree.

It seems there is almost everything implemented except for some product ids.

Quoted 1 line

Also, Tobias (added to CC) seems to be currently working on adding support for a subset of keyboards that this patch adds. Tobias -- it seems like this patch is handling a superset of what your patch does, right?

Yes

Quoted 1 line

It would be great if you could put this all together into one condensated patch which I could merge into my tree.

I'm working on that, but I found a problem in the new 2.6.25-rc1 code.

Until 2.6.24 hidinput_apple_event which handles the apple specials was called by hidinput_hid_event and if hidinput_apple_event changed some code, hidinput_hid_event returned and didn't send any events.

In 2.6.25 hidinput_apple_event is called in hidinput_event_quirks which is called by hidinput_hid_event. But if hidinput_event_quirks sent events, hidinput_hid_event doesn't know of this and send the events too! So if I press some keys on my keyboard, there are
2 keycodes sent.

This seems to be a greater change in 2.6.25 and I don't want to change this, because I think someone made this change for some good reason.

Regards
Tobias

Jiri Kosina

2008-02-13 12:57 UTC #3 lore raw

Quoted 4 lines · Tobias Müller

In 2.6.25 hidinput_apple_event is called in hidinput_event_quirks which is called by hidinput_hid_event. But if hidinput_event_quirks sent events, hidinput_hid_event doesn't know of this and send the events too! So if I press some keys on my keyboard, there are 2 keycodes sent.

Hi Tobias,

yes, this is a bug, for which I already have fix in my tree -- see
http://git.kernel.org/?p=linux/kernel/git/jikos/hid.git;a=commit;h=68a1f2cc8676f22a6fd49f344f99e326eb7f5117

This restores the previous behavior. I just haven't pushed this to Linus yet.

Signature · 2 lines

Jiri Kosina
SUSE Labs

Tobias Müller

2008-02-13 15:50 UTC #4 lore raw

Quoted 3 lines · Jiri Kosina

On Tue, 12 Fe
It would be great if you could put this all together into one condensated patch which I could merge into my tree.

So, this is the updated version. It only adds some product-ids and some keyboards mappings.

Regards
Tobias

drivers/hid/hid-input.c +7 -2

index 5325d98..6a100e2 100644
@@ -97,10 +97,11 @@ struct hidinput_key_translation {
 #define APPLE_FLAG_FKEY 0x01
 
 static struct hidinput_key_translation apple_fn_keys[] = {
+       { KEY_BACKSPACE, KEY_DELETE },                                  /* Needed for MacBook keyboards without delete */
        { KEY_F1,       KEY_BRIGHTNESSDOWN,     APPLE_FLAG_FKEY },
        { KEY_F2,       KEY_BRIGHTNESSUP,       APPLE_FLAG_FKEY },
-       { KEY_F3,       KEY_CYCLEWINDOWS,       APPLE_FLAG_FKEY }, /* Exposé */
-       { KEY_F4,       KEY_FN_F4,              APPLE_FLAG_FKEY }, /* Dashboard */
+       { KEY_F3,       KEY_CYCLEWINDOWS,       APPLE_FLAG_FKEY },      /* Exposé */
+       { KEY_F4,       KEY_FN_F4,              APPLE_FLAG_FKEY },      /* Dashboard */
        { KEY_F5,       KEY_FN_F5 },
        { KEY_F6,       KEY_FN_F6 },
        { KEY_F7,       KEY_BACK,               APPLE_FLAG_FKEY },

@@ -109,6 +110,10 @@ static struct hidinput_key_translation apple_fn_keys[] = {
        { 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 },                                   /* Needed for MacBook keyboards without page-up */
+       { KEY_DOWN,     KEY_PAGEDOWN },                                 /* Needed for MacBook keyboards without page-down */
+       { KEY_LEFT,     KEY_HOME },                                     /* Needed for MacBook keyboards without home */
+       { KEY_RIGHT,    KEY_END },                                      /* Needed for MacBook keyboards without end */
        { }
 };

drivers/hid/usbhid/hid-quirks.c +12 -0

index b77b61e..73a5aca 100644
@@ -66,6 +66,12 @@
 #define USB_DEVICE_ID_APPLE_ALU_ANSI   0x0220
 #define USB_DEVICE_ID_APPLE_ALU_ISO    0x0221
 #define USB_DEVICE_ID_APPLE_ALU_JIS    0x0222
+#define USB_DEVICE_ID_APPLE_GEYSER4_HF_ANSI    0x0229
+#define USB_DEVICE_ID_APPLE_GEYSER4_HF_ISO     0x022a
+#define USB_DEVICE_ID_APPLE_GEYSER4_HF_JIS     0x022b
+#define USB_DEVICE_ID_APPLE_ALU_WIRELESS_ANSI  0x022c
+#define USB_DEVICE_ID_APPLE_ALU_WIRELESS_ISO   0x022d
+#define USB_DEVICE_ID_APPLE_ALU_WIRELESS_JIS   0x022e
 #define USB_DEVICE_ID_APPLE_FOUNTAIN_TP_ONLY   0x030a
 #define USB_DEVICE_ID_APPLE_GEYSER1_TP_ONLY    0x030b
 #define USB_DEVICE_ID_APPLE_IRCONTROL4 0x8242

@@ -593,6 +599,12 @@ static const struct hid_blacklist {
        { USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_ALU_ANSI, HID_QUIRK_APPLE_HAS_FN },
        { USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_ALU_ISO, HID_QUIRK_APPLE_HAS_FN | HID_QUIRK_APPLE_ISO_KEYBOARD },
        { USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_ALU_JIS, HID_QUIRK_APPLE_HAS_FN },
+       { USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER4_HF_ANSI, HID_QUIRK_APPLE_HAS_FN | HID_QUIRK_IGNORE_MOUSE},
+       { USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER4_HF_ISO, HID_QUIRK_APPLE_HAS_FN | HID_QUIRK_IGNORE_MOUSE | HID_QUIRK_APPLE_ISO_KEYBOARD},
+       { USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER4_HF_JIS, HID_QUIRK_APPLE_HAS_FN | HID_QUIRK_IGNORE_MOUSE},
+       { USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_ALU_WIRELESS_ANSI, HID_QUIRK_APPLE_HAS_FN},
+       { USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_ALU_WIRELESS_ISO, HID_QUIRK_APPLE_HAS_FN | HID_QUIRK_APPLE_ISO_KEYBOARD},
+       { USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_ALU_WIRELESS_JIS, HID_QUIRK_APPLE_HAS_FN},
        { USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_FOUNTAIN_TP_ONLY, HID_QUIRK_APPLE_HAS_FN | HID_QUIRK_IGNORE_MOUSE },
        { USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER1_TP_ONLY, HID_QUIRK_APPLE_HAS_FN | HID_QUIRK_IGNORE_MOUSE },

Jiri Kosina

2008-02-13 16:06 UTC #5 lore raw

Quoted 4 lines · Tobias Müller

Quoted 2 lines
It would be great if you could put this all together into one condensated patch which I could merge into my tree.
So, this is the updated version. It only adds some product-ids and some keyboards mappings.

The patch had mangled whitespaces again, but I have cleaned that up manually and applied to my tree.

Thanks,

Signature · 2 lines

Jiri Kosina
SUSE Labs

Tobias Müller

2008-03-14 16:41 UTC #6 lore raw

Hi!

I just tested the rc5 on my pc and macbook and found a small mistake:

drivers/hid/usbhid/hid-quirks.c +1 -1

index e6d05f6..de8c5a9 100644
@@ -624,7 +624,7 @@ static const struct hid_blacklist {
        { USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_ALU_ISO, HID_QUIRK_APPLE_HAS_FN | HID_QUIRK_APPLE_ISO_KEYBOARD },
        { USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_ALU_JIS, HID_QUIRK_APPLE_HAS_FN },
        { USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER4_HF_ANSI, HID_QUIRK_APPLE_HAS_FN | HID_QUIRK_IGNORE_MOUSE },
- 
      { USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER4_HF_ISO, HID_QUIRK_APPLE_HAS_FN | HID_QUIRK_IGNORE_MOUSE | HID_QUIRK_APPLE_ISO_KEYBOARD },
+ 
      { USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER4_HF_ISO, HID_QUIRK_APPLE_HAS_FN | HID_QUIRK_IGNORE_MOUSE },
        { USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER4_HF_JIS, HID_QUIRK_APPLE_HAS_FN | HID_QUIRK_IGNORE_MOUSE },

{ USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_ALU_WIRELESS_ANSI, HID_QUIRK_APPLE_HAS_FN },
{ USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_ALU_WIRELESS_ISO, HID_QUIRK_APPLE_HAS_FN | HID_QUIRK_APPLE_ISO_KEYBOARD },

Regards
Tobias

Jiri Kosina

2008-03-14 16:52 UTC #7 lore raw

Quoted 1 line · Tobias Müller

I just tested the rc5 on my pc and macbook and found a small mistake:

Hi Tobias,

the patch has been horribly mangled by your mailer, could you please resend?

Also, a small changelog and Signed-off-by line would be needed.

Thanks,

Signature · 2 lines

Jiri Kosina
SUSE Labs

Tobias Müller

2008-03-14 17:12 UTC #8 lore raw

Hi!

Quoted 1 line

the patch has been horribly mangled by your mailer, could you please resend?

I'm sorry. My thunderbird is keen on scrambling mails... :/ Hope this time it's fine.

Regards
Tobias

-----------------------

4th=generation MacBook (product id 0x022a) from Apple doesn't need HID_QUIRK_APPLE_ISO_KEYBOARD fix anymore.

Signed-off-by Tobias Mueller <[email protected]>

drivers/hid/usbhid/hid-quirks.c +1 -1

index e6d05f6..de8c5a9 100644
@@ -624,7 +624,7 @@ static const struct hid_blacklist {
       { USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_ALU_ISO, HID_QUIRK_APPLE_HAS_FN | HID_QUIRK_APPLE_ISO_KEYBOARD },
       { USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_ALU_JIS, HID_QUIRK_APPLE_HAS_FN },
       { USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER4_HF_ANSI, HID_QUIRK_APPLE_HAS_FN | HID_QUIRK_IGNORE_MOUSE },
-       { USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER4_HF_ISO, HID_QUIRK_APPLE_HAS_FN | HID_QUIRK_IGNORE_MOUSE | HID_QUIRK_APPLE_ISO_KEYBOARD },
+       { USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER4_HF_ISO, HID_QUIRK_APPLE_HAS_FN | HID_QUIRK_IGNORE_MOUSE },
       { USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER4_HF_JIS, HID_QUIRK_APPLE_HAS_FN | HID_QUIRK_IGNORE_MOUSE },
       { USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_ALU_WIRELESS_ANSI, HID_QUIRK_APPLE_HAS_FN },
       { USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_ALU_WIRELESS_ISO, HID_QUIRK_APPLE_HAS_FN | HID_QUIRK_APPLE_ISO_KEYBOARD },

Tobias Müller

2008-03-14 20:52 UTC #10 lore raw

In reply to Jiri Kosina

Hi!

Quoted 1 line

the patch has been horribly mangled by your mailer, could you please resend?

I'm sorry. My thunderbird is keen on scrambling mails... :/ Hope this time it's fine.

Regards
Tobias

-----------------------

4th=generation MacBook (product id 0x022a) from Apple doesn't need HID_QUIRK_APPLE_ISO_KEYBOARD fix anymore.

Signed-off-by Tobias Mueller <[email protected]>

drivers/hid/usbhid/hid-quirks.c +1 -1

index e6d05f6..de8c5a9 100644
@@ -624,7 +624,7 @@ static const struct hid_blacklist {
       { USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_ALU_ISO, HID_QUIRK_APPLE_HAS_FN | HID_QUIRK_APPLE_ISO_KEYBOARD },
       { USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_ALU_JIS, HID_QUIRK_APPLE_HAS_FN },
       { USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER4_HF_ANSI, HID_QUIRK_APPLE_HAS_FN | HID_QUIRK_IGNORE_MOUSE },
-       { USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER4_HF_ISO, HID_QUIRK_APPLE_HAS_FN | HID_QUIRK_IGNORE_MOUSE | HID_QUIRK_APPLE_ISO_KEYBOARD },
+       { USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER4_HF_ISO, HID_QUIRK_APPLE_HAS_FN | HID_QUIRK_IGNORE_MOUSE },
       { USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER4_HF_JIS, HID_QUIRK_APPLE_HAS_FN | HID_QUIRK_IGNORE_MOUSE },
       { USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_ALU_WIRELESS_ANSI, HID_QUIRK_APPLE_HAS_FN },
       { USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_ALU_WIRELESS_ISO, HID_QUIRK_APPLE_HAS_FN | HID_QUIRK_APPLE_ISO_KEYBOARD },

Jiri Kosina

2008-03-14 22:26 UTC #9 lore raw

In reply to Tobias Müller

Quoted 3 lines · Tobias Müller

Quoted 1 line
the patch has been horribly mangled by your mailer, could you please resend?
I'm sorry. My thunderbird is keen on scrambling mails... :/ Hope this time it's fine.

No, it is still mangled. Also, you seem to have sent the mail to linux-ide instead of linus-input :)

Never mind, as this is oneliner I will fix it by hand, just to let you know in case you will be submitting any more sophisticated patches.

Thanks,

Signature · 2 lines

Jiri Kosina
SUSE Labs