linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] fix HID quirks for aluminium apple wireless keyboards
@ 2008-06-12 12:26 Paul Collins
  2008-06-12 12:57 ` [PATCH] [RESEND] " Paul Collins
  0 siblings, 1 reply; 12+ messages in thread
From: Paul Collins @ 2008-06-12 12:26 UTC (permalink / raw)
  To: marcel, jkosina; +Cc: linux-input, linux-kernel, linux-usb

Hi Marcel and Jiri,

I noticed that the quirks for Bluetooth Apple keyboards seem to have
been incorrectly added to the USB HID, thus rendering them ineffective.
Here is a patch that moves them to the Bluetooth HID.  With this patch
applied the Fn key on my Apple wireless keyboard now works as expected.

I also took the liberty of adding defines for the vendor and for the
existing Mighty Mouse quirk.

Signed-off-by: Paul Collins <paul@ondioline.org

diff --git a/drivers/hid/usbhid/hid-quirks.c b/drivers/hid/usbhid/hid-quirks.c
index 1df832a..ff41c07 100644
--- a/drivers/hid/usbhid/hid-quirks.c
+++ b/drivers/hid/usbhid/hid-quirks.c
@@ -72,9 +72,6 @@
 #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
@@ -639,9 +636,6 @@ static const struct hid_blacklist {
 	{ USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER4_HF_ANSI, HID_QUIRK_APPLE_NUMLOCK_EMULATION | HID_QUIRK_APPLE_HAS_FN | HID_QUIRK_IGNORE_MOUSE },
 	{ USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER4_HF_ISO, HID_QUIRK_APPLE_NUMLOCK_EMULATION | HID_QUIRK_APPLE_HAS_FN | HID_QUIRK_IGNORE_MOUSE },
 	{ USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER4_HF_JIS, HID_QUIRK_APPLE_NUMLOCK_EMULATION | HID_QUIRK_APPLE_HAS_FN | HID_QUIRK_IGNORE_MOUSE },
-	{ USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_ALU_WIRELESS_ANSI, HID_QUIRK_APPLE_NUMLOCK_EMULATION | HID_QUIRK_APPLE_HAS_FN },
-	{ USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_ALU_WIRELESS_ISO, HID_QUIRK_APPLE_NUMLOCK_EMULATION | HID_QUIRK_APPLE_HAS_FN | HID_QUIRK_APPLE_ISO_KEYBOARD },
-	{ USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_ALU_WIRELESS_JIS, HID_QUIRK_APPLE_NUMLOCK_EMULATION | HID_QUIRK_APPLE_HAS_FN },
 	{ USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_FOUNTAIN_TP_ONLY, HID_QUIRK_APPLE_NUMLOCK_EMULATION | HID_QUIRK_APPLE_HAS_FN | HID_QUIRK_IGNORE_MOUSE },
 	{ USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER1_TP_ONLY, HID_QUIRK_APPLE_NUMLOCK_EMULATION | HID_QUIRK_APPLE_HAS_FN | HID_QUIRK_IGNORE_MOUSE },
 
diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c
index 519cdb9..f0762e5 100644
--- a/net/bluetooth/hidp/core.c
+++ b/net/bluetooth/hidp/core.c
@@ -671,14 +671,26 @@ static void hidp_close(struct hid_device *hid)
 {
 }
 
+#define BT_VENDOR_ID_APPLE				0x05ac
+
+#define BT_DEVICE_ID_APPLE_ALU_WIRELESS_ANSI		0x022c
+#define BT_DEVICE_ID_APPLE_ALU_WIRELESS_ISO		0x022d
+#define BT_DEVICE_ID_APPLE_ALU_WIRELESS_JIS		0x022e
+#define BT_DEVICE_ID_APPLE_MIGHTY_MOUSE_WIRELESS	0x030c
+
 static const struct {
 	__u16 idVendor;
 	__u16 idProduct;
 	unsigned quirks;
 } hidp_blacklist[] = {
-	/* Apple wireless Mighty Mouse */
-	{ 0x05ac, 0x030c, HID_QUIRK_MIGHTYMOUSE | HID_QUIRK_INVERT_HWHEEL },
-
+	{ BT_VENDOR_ID_APPLE, BT_DEVICE_ID_APPLE_ALU_WIRELESS_ANSI,
+	  HID_QUIRK_APPLE_NUMLOCK_EMULATION | HID_QUIRK_APPLE_HAS_FN },
+	{ BT_VENDOR_ID_APPLE, BT_DEVICE_ID_APPLE_ALU_WIRELESS_ISO,
+	  HID_QUIRK_APPLE_NUMLOCK_EMULATION | HID_QUIRK_APPLE_HAS_FN | HID_QUIRK_APPLE_ISO_KEYBOARD },
+	{ BT_VENDOR_ID_APPLE, BT_DEVICE_ID_APPLE_ALU_WIRELESS_JIS,
+	  HID_QUIRK_APPLE_NUMLOCK_EMULATION | HID_QUIRK_APPLE_HAS_FN },
+	{ BT_VENDOR_ID_APPLE, BT_DEVICE_ID_APPLE_MIGHTY_MOUSE_WIRELESS,
+	  HID_QUIRK_MIGHTYMOUSE | HID_QUIRK_INVERT_HWHEEL },
 	{ }	/* Terminating entry */
 };
 

-- 
Paul Collins
Wellington, New Zealand

Dag vijandelijk luchtschip de huismeester is dood

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH] [RESEND] fix HID quirks for aluminium apple wireless keyboards
  2008-06-12 12:26 [PATCH] fix HID quirks for aluminium apple wireless keyboards Paul Collins
@ 2008-06-12 12:57 ` Paul Collins
  2008-06-12 13:17   ` Phil Endecott
  2008-06-12 20:08   ` Marcel Holtmann
  0 siblings, 2 replies; 12+ messages in thread
From: Paul Collins @ 2008-06-12 12:57 UTC (permalink / raw)
  To: marcel, jkosina; +Cc: linux-input, linux-kernel, linux-usb

Hi Marcel and Jiri,

I noticed that the quirks for Bluetooth Apple keyboards seem to have
been incorrectly added to the USB HID, thus rendering them ineffective.
Here is a patch that moves them to the Bluetooth HID.  With this patch
applied the Fn key on my Apple wireless keyboard now works as expected.

I also took the liberty of adding defines for the vendor and for the
existing Mighty Mouse quirk.

Signed-off-by: Paul Collins <paul@ondioline.org>

---
Here's a new version with corrected Signed-off-by line.  Also the
previous one was truncated by gmane; with any luck this makes it.

diff --git a/drivers/hid/usbhid/hid-quirks.c b/drivers/hid/usbhid/hid-quirks.c
index 1df832a..ff41c07 100644
--- a/drivers/hid/usbhid/hid-quirks.c
+++ b/drivers/hid/usbhid/hid-quirks.c
@@ -72,9 +72,6 @@
 #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
@@ -639,9 +636,6 @@ static const struct hid_blacklist {
 	{ USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER4_HF_ANSI, HID_QUIRK_APPLE_NUMLOCK_EMULATION | HID_QUIRK_APPLE_HAS_FN | HID_QUIRK_IGNORE_MOUSE },
 	{ USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER4_HF_ISO, HID_QUIRK_APPLE_NUMLOCK_EMULATION | HID_QUIRK_APPLE_HAS_FN | HID_QUIRK_IGNORE_MOUSE },
 	{ USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER4_HF_JIS, HID_QUIRK_APPLE_NUMLOCK_EMULATION | HID_QUIRK_APPLE_HAS_FN | HID_QUIRK_IGNORE_MOUSE },
-	{ USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_ALU_WIRELESS_ANSI, HID_QUIRK_APPLE_NUMLOCK_EMULATION | HID_QUIRK_APPLE_HAS_FN },
-	{ USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_ALU_WIRELESS_ISO, HID_QUIRK_APPLE_NUMLOCK_EMULATION | HID_QUIRK_APPLE_HAS_FN | HID_QUIRK_APPLE_ISO_KEYBOARD },
-	{ USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_ALU_WIRELESS_JIS, HID_QUIRK_APPLE_NUMLOCK_EMULATION | HID_QUIRK_APPLE_HAS_FN },
 	{ USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_FOUNTAIN_TP_ONLY, HID_QUIRK_APPLE_NUMLOCK_EMULATION | HID_QUIRK_APPLE_HAS_FN | HID_QUIRK_IGNORE_MOUSE },
 	{ USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER1_TP_ONLY, HID_QUIRK_APPLE_NUMLOCK_EMULATION | HID_QUIRK_APPLE_HAS_FN | HID_QUIRK_IGNORE_MOUSE },
 
diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c
index 519cdb9..f0762e5 100644
--- a/net/bluetooth/hidp/core.c
+++ b/net/bluetooth/hidp/core.c
@@ -671,14 +671,26 @@ static void hidp_close(struct hid_device *hid)
 {
 }
 
+#define BT_VENDOR_ID_APPLE				0x05ac
+
+#define BT_DEVICE_ID_APPLE_ALU_WIRELESS_ANSI		0x022c
+#define BT_DEVICE_ID_APPLE_ALU_WIRELESS_ISO		0x022d
+#define BT_DEVICE_ID_APPLE_ALU_WIRELESS_JIS		0x022e
+#define BT_DEVICE_ID_APPLE_MIGHTY_MOUSE_WIRELESS	0x030c
+
 static const struct {
 	__u16 idVendor;
 	__u16 idProduct;
 	unsigned quirks;
 } hidp_blacklist[] = {
-	/* Apple wireless Mighty Mouse */
-	{ 0x05ac, 0x030c, HID_QUIRK_MIGHTYMOUSE | HID_QUIRK_INVERT_HWHEEL },
-
+	{ BT_VENDOR_ID_APPLE, BT_DEVICE_ID_APPLE_ALU_WIRELESS_ANSI,
+	  HID_QUIRK_APPLE_NUMLOCK_EMULATION | HID_QUIRK_APPLE_HAS_FN },
+	{ BT_VENDOR_ID_APPLE, BT_DEVICE_ID_APPLE_ALU_WIRELESS_ISO,
+	  HID_QUIRK_APPLE_NUMLOCK_EMULATION | HID_QUIRK_APPLE_HAS_FN | HID_QUIRK_APPLE_ISO_KEYBOARD },
+	{ BT_VENDOR_ID_APPLE, BT_DEVICE_ID_APPLE_ALU_WIRELESS_JIS,
+	  HID_QUIRK_APPLE_NUMLOCK_EMULATION | HID_QUIRK_APPLE_HAS_FN },
+	{ BT_VENDOR_ID_APPLE, BT_DEVICE_ID_APPLE_MIGHTY_MOUSE_WIRELESS,
+	  HID_QUIRK_MIGHTYMOUSE | HID_QUIRK_INVERT_HWHEEL },
 	{ }	/* Terminating entry */
 };
 

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH] [RESEND] fix HID quirks for aluminium apple wireless keyboards
  2008-06-12 12:57 ` [PATCH] [RESEND] " Paul Collins
@ 2008-06-12 13:17   ` Phil Endecott
  2008-06-12 14:55     ` Paul Collins
  2008-06-12 20:08   ` Marcel Holtmann
  1 sibling, 1 reply; 12+ messages in thread
From: Phil Endecott @ 2008-06-12 13:17 UTC (permalink / raw)
  To: Paul Collins; +Cc: linux-input, linux-kernel, linux-usb, marcel, jkosina

Paul Collins wrote:
> Hi Marcel and Jiri,
>
> I noticed that the quirks for Bluetooth Apple keyboards seem to have
> been incorrectly added to the USB HID, thus rendering them ineffective.
> Here is a patch that moves them to the Bluetooth HID.  With this patch
> applied the Fn key on my Apple wireless keyboard now works as expected.
>
> I also took the liberty of adding defines for the vendor and for the
> existing Mighty Mouse quirk.
>
> Signed-off-by: Paul Collins <paul@ondioline.org>
>
> ---
> Here's a new version with corrected Signed-off-by line.  Also the
> previous one was truncated by gmane; with any luck this makes it.
>
> diff --git a/drivers/hid/usbhid/hid-quirks.c b/drivers/hid/usbhid/hid-quirks.c
> index 1df832a..ff41c07 100644
> --- a/drivers/hid/usbhid/hid-quirks.c
> +++ b/drivers/hid/usbhid/hid-quirks.c
> @@ -72,9 +72,6 @@
>  #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
> @@ -639,9 +636,6 @@ static const struct hid_blacklist {
>  	{ USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER4_HF_ANSI, HID_QUIRK_APPLE_NUMLOCK_EMULATION | HID_QUIRK_APPLE_HAS_FN | HID_QUIRK_IGNORE_MOUSE },
>  	{ USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER4_HF_ISO, HID_QUIRK_APPLE_NUMLOCK_EMULATION | HID_QUIRK_APPLE_HAS_FN | HID_QUIRK_IGNORE_MOUSE },
>  	{ USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER4_HF_JIS, HID_QUIRK_APPLE_NUMLOCK_EMULATION | HID_QUIRK_APPLE_HAS_FN | HID_QUIRK_IGNORE_MOUSE },
> -	{ USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_ALU_WIRELESS_ANSI, HID_QUIRK_APPLE_NUMLOCK_EMULATION | HID_QUIRK_APPLE_HAS_FN },
> -	{ USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_ALU_WIRELESS_ISO, HID_QUIRK_APPLE_NUMLOCK_EMULATION | HID_QUIRK_APPLE_HAS_FN | HID_QUIRK_APPLE_ISO_KEYBOARD },
> -	{ USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_ALU_WIRELESS_JIS, HID_QUIRK_APPLE_NUMLOCK_EMULATION | HID_QUIRK_APPLE_HAS_FN },
>  	{ USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_FOUNTAIN_TP_ONLY, HID_QUIRK_APPLE_NUMLOCK_EMULATION | HID_QUIRK_APPLE_HAS_FN | HID_QUIRK_IGNORE_MOUSE },
>  	{ USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER1_TP_ONLY, HID_QUIRK_APPLE_NUMLOCK_EMULATION | HID_QUIRK_APPLE_HAS_FN | HID_QUIRK_IGNORE_MOUSE },
>  
> diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c
> index 519cdb9..f0762e5 100644
> --- a/net/bluetooth/hidp/core.c
> +++ b/net/bluetooth/hidp/core.c
> @@ -671,14 +671,26 @@ static void hidp_close(struct hid_device *hid)
>  {
>  }
>  
> +#define BT_VENDOR_ID_APPLE				0x05ac
> +
> +#define BT_DEVICE_ID_APPLE_ALU_WIRELESS_ANSI		0x022c
> +#define BT_DEVICE_ID_APPLE_ALU_WIRELESS_ISO		0x022d
> +#define BT_DEVICE_ID_APPLE_ALU_WIRELESS_JIS		0x022e
> +#define BT_DEVICE_ID_APPLE_MIGHTY_MOUSE_WIRELESS	0x030c
> +
>  static const struct {
>  	__u16 idVendor;
>  	__u16 idProduct;
>  	unsigned quirks;
>  } hidp_blacklist[] = {
> -	/* Apple wireless Mighty Mouse */
> -	{ 0x05ac, 0x030c, HID_QUIRK_MIGHTYMOUSE | HID_QUIRK_INVERT_HWHEEL },
> -
> +	{ BT_VENDOR_ID_APPLE, BT_DEVICE_ID_APPLE_ALU_WIRELESS_ANSI,
> +	  HID_QUIRK_APPLE_NUMLOCK_EMULATION | HID_QUIRK_APPLE_HAS_FN },
> +	{ BT_VENDOR_ID_APPLE, BT_DEVICE_ID_APPLE_ALU_WIRELESS_ISO,
> +	  HID_QUIRK_APPLE_NUMLOCK_EMULATION | HID_QUIRK_APPLE_HAS_FN | HID_QUIRK_APPLE_ISO_KEYBOARD },
> +	{ BT_VENDOR_ID_APPLE, BT_DEVICE_ID_APPLE_ALU_WIRELESS_JIS,
> +	  HID_QUIRK_APPLE_NUMLOCK_EMULATION | HID_QUIRK_APPLE_HAS_FN },
> +	{ BT_VENDOR_ID_APPLE, BT_DEVICE_ID_APPLE_MIGHTY_MOUSE_WIRELESS,
> +	  HID_QUIRK_MIGHTYMOUSE | HID_QUIRK_INVERT_HWHEEL },
>  	{ }	/* Terminating entry */
>  };
>  

Hi Paul,

Did you see my messages about this a few weeks ago?  
http://thread.gmane.org/gmane.linux.kernel.input/4984

I didn't post a patch because I believe that this stuff has all moved as a 
result of Jiri Slaby's patch "HID: move apple quirks" posted to linux-input 
on 2008-05-16.  One of us should prepare a patch against the tree after 
that patch.

Does HID_QUIRK_APPLE_NUMLOCK_EMULATION do anything useful on these keyboards?  
I have not enabled it.

Do you know what happens if you have a USB bluetooth dongle with HID proxy 
mode?  My assumption was that the vendor and product IDs from the keyboard 
would then appear to the kernel as USB IDs.  If this is true, then you should 
keep the existing entries for these devices in the USB quirks tables.  Does 
anyone know if HID proxy dongles actually do this?  In any case, leaving 
the existing entries in the USB quirks table can't do any harm.

I wrote this up at http://chezphil.org/apple-alu-bluetooth-kb-linux/ and I'll 
just post that URL again to help the search engines...

Cheers,

Phil.


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] [RESEND] fix HID quirks for aluminium apple wireless keyboards
  2008-06-12 13:17   ` Phil Endecott
@ 2008-06-12 14:55     ` Paul Collins
  0 siblings, 0 replies; 12+ messages in thread
From: Paul Collins @ 2008-06-12 14:55 UTC (permalink / raw)
  To: Phil Endecott; +Cc: linux-input, linux-kernel, linux-usb, marcel, jkosina

"Phil Endecott" <phil_tuhck_endecott@chezphil.org> writes:

> Did you see my messages about this a few weeks ago?  
> http://thread.gmane.org/gmane.linux.kernel.input/4984

Alas, no!  That would have been very helpful.

> I didn't post a patch because I believe that this stuff has all moved
> as a result of Jiri Slaby's patch "HID: move apple quirks" posted to
> linux-input on 2008-05-16.  One of us should prepare a patch against
> the tree after that patch.

I'm happy to do this, although the only tree I track closely is Linus's.

> Do you know what happens if you have a USB bluetooth dongle with HID
> proxy mode?  My assumption was that the vendor and product IDs from
> the keyboard would then appear to the kernel as USB IDs.  If this is
> true, then you should keep the existing entries for these devices in
> the USB quirks tables.  Does anyone know if HID proxy dongles actually
> do this?  In any case, leaving the existing entries in the USB quirks
> table can't do any harm.

Hmm, no idea.  Absent confirmation I guess I'll drop the USB HID changes
to be on the safe side.

> I wrote this up at http://chezphil.org/apple-alu-bluetooth-kb-linux/
> and I'll just post that URL again to help the search engines...

Nice writeup!  Small correction: when the quirk is active, you can
generate forward delete by pressing Fn-backspace.

-- 
Paul Collins
Wellington, New Zealand

Dag vijandelijk luchtschip de huismeester is dood

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] [RESEND] fix HID quirks for aluminium apple wireless keyboards
  2008-06-12 12:57 ` [PATCH] [RESEND] " Paul Collins
  2008-06-12 13:17   ` Phil Endecott
@ 2008-06-12 20:08   ` Marcel Holtmann
  2008-06-14  8:03     ` Paul Collins
  1 sibling, 1 reply; 12+ messages in thread
From: Marcel Holtmann @ 2008-06-12 20:08 UTC (permalink / raw)
  To: Paul Collins; +Cc: jkosina, linux-input, linux-kernel, linux-usb

Hi Paul,

> I noticed that the quirks for Bluetooth Apple keyboards seem to have
> been incorrectly added to the USB HID, thus rendering them ineffective.
> Here is a patch that moves them to the Bluetooth HID.  With this patch
> applied the Fn key on my Apple wireless keyboard now works as expected.
> 
> I also took the liberty of adding defines for the vendor and for the
> existing Mighty Mouse quirk.

I prefer not to do this. I don't see any benefit or readability coming
out of it. Leave it as it is. Add the numeric ids and then a line on top
of it describing the devices.

Regards

Marcel



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] [RESEND] fix HID quirks for aluminium apple wireless keyboards
  2008-06-12 20:08   ` Marcel Holtmann
@ 2008-06-14  8:03     ` Paul Collins
  2008-06-14 12:22       ` Marcel Holtmann
                         ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: Paul Collins @ 2008-06-14  8:03 UTC (permalink / raw)
  To: Marcel Holtmann; +Cc: jkosina, linux-input, linux-kernel, linux-usb

Marcel Holtmann <marcel@holtmann.org> writes:

>> I also took the liberty of adding defines for the vendor and for the
>> existing Mighty Mouse quirk.
>
> I prefer not to do this. I don't see any benefit or readability coming
> out of it. Leave it as it is. Add the numeric ids and then a line on top
> of it describing the devices.

Here is a new version of the patch that just adds the entries to
hidp_blacklist using your suggested style.  I'm leaving the USB HID's
table alone based on Phil Endecott's remarks regarding HID proxy mode,
so now this patch only touches Bluetooth code.


bluetooth: fix Fn on Apple Wireless Keyboard

Enable the HID quirks for Apple Wireless Keyboards, based on the quirk
table entries in the USB HID.  With this patch applied the Fn key on my
keyboard now functions as expected.

diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c
index 519cdb9..62f174a 100644
--- a/net/bluetooth/hidp/core.c
+++ b/net/bluetooth/hidp/core.c
@@ -676,6 +676,12 @@ static const struct {
 	__u16 idProduct;
 	unsigned quirks;
 } hidp_blacklist[] = {
+	/* Apple Wireless Keyboard, ANSI layout */
+	{ 0x05ac, 0x022c, HID_QUIRK_APPLE_NUMLOCK_EMULATION | HID_QUIRK_APPLE_HAS_FN },
+	/* Apple Wireless Keyboard, ISO layout */
+	{ 0x05ac, 0x022d, HID_QUIRK_APPLE_NUMLOCK_EMULATION | HID_QUIRK_APPLE_HAS_FN | HID_QUIRK_APPLE_ISO_KEYBOARD },
+	/* Apple Wireless Keyboard, JIS layout */
+	{ 0x05ac, 0x022e, HID_QUIRK_APPLE_NUMLOCK_EMULATION | HID_QUIRK_APPLE_HAS_FN },
 	/* Apple wireless Mighty Mouse */
 	{ 0x05ac, 0x030c, HID_QUIRK_MIGHTYMOUSE | HID_QUIRK_INVERT_HWHEEL },
 


-- 
Paul Collins
Wellington, New Zealand

Dag vijandelijk luchtschip de huismeester is dood

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH] [RESEND] fix HID quirks for aluminium apple wireless keyboards
  2008-06-14  8:03     ` Paul Collins
@ 2008-06-14 12:22       ` Marcel Holtmann
  2008-06-17 13:54         ` Jiri Kosina
  2008-06-14 12:51       ` Phil Endecott
  2008-06-27 16:32       ` Jan Scholz
  2 siblings, 1 reply; 12+ messages in thread
From: Marcel Holtmann @ 2008-06-14 12:22 UTC (permalink / raw)
  To: Paul Collins; +Cc: jkosina, linux-input, linux-kernel, linux-usb

Hi Paul,

> >> I also took the liberty of adding defines for the vendor and for the
> >> existing Mighty Mouse quirk.
> >
> > I prefer not to do this. I don't see any benefit or readability coming
> > out of it. Leave it as it is. Add the numeric ids and then a line on top
> > of it describing the devices.
> 
> Here is a new version of the patch that just adds the entries to
> hidp_blacklist using your suggested style.  I'm leaving the USB HID's
> table alone based on Phil Endecott's remarks regarding HID proxy mode,
> so now this patch only touches Bluetooth code.
> 
> 
> bluetooth: fix Fn on Apple Wireless Keyboard
> 
> Enable the HID quirks for Apple Wireless Keyboards, based on the quirk
> table entries in the USB HID.  With this patch applied the Fn key on my
> keyboard now functions as expected.
> 
> diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c
> index 519cdb9..62f174a 100644
> --- a/net/bluetooth/hidp/core.c
> +++ b/net/bluetooth/hidp/core.c
> @@ -676,6 +676,12 @@ static const struct {
>  	__u16 idProduct;
>  	unsigned quirks;
>  } hidp_blacklist[] = {
> +	/* Apple Wireless Keyboard, ANSI layout */
> +	{ 0x05ac, 0x022c, HID_QUIRK_APPLE_NUMLOCK_EMULATION | HID_QUIRK_APPLE_HAS_FN },
> +	/* Apple Wireless Keyboard, ISO layout */
> +	{ 0x05ac, 0x022d, HID_QUIRK_APPLE_NUMLOCK_EMULATION | HID_QUIRK_APPLE_HAS_FN | HID_QUIRK_APPLE_ISO_KEYBOARD },
> +	/* Apple Wireless Keyboard, JIS layout */
> +	{ 0x05ac, 0x022e, HID_QUIRK_APPLE_NUMLOCK_EMULATION | HID_QUIRK_APPLE_HAS_FN },
>  	/* Apple wireless Mighty Mouse */
>  	{ 0x05ac, 0x030c, HID_QUIRK_MIGHTYMOUSE | HID_QUIRK_INVERT_HWHEEL },

Acked-by: Marcel Holtmann <marcel@holtmann.org>

However depending on if Jiri's changes for the HID bus are ready, this
patch might become obsolete.

Regards

Marcel



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] [RESEND] fix HID quirks for aluminium apple wireless keyboards
  2008-06-14  8:03     ` Paul Collins
  2008-06-14 12:22       ` Marcel Holtmann
@ 2008-06-14 12:51       ` Phil Endecott
  2008-06-18 10:19         ` Paul Collins
  2008-06-27 16:32       ` Jan Scholz
  2 siblings, 1 reply; 12+ messages in thread
From: Phil Endecott @ 2008-06-14 12:51 UTC (permalink / raw)
  To: Paul Collins
  Cc: jkosina, linux-input, linux-kernel, linux-usb, Marcel Holtmann

Paul Collins wrote:
> Here is a new version of the patch that just adds the entries to
> hidp_blacklist using your suggested style.  I'm leaving the USB HID's
> table alone based on Phil Endecott's remarks regarding HID proxy mode,
> so now this patch only touches Bluetooth code.
>
>
> bluetooth: fix Fn on Apple Wireless Keyboard
>
> Enable the HID quirks for Apple Wireless Keyboards, based on the quirk
> table entries in the USB HID.  With this patch applied the Fn key on my
> keyboard now functions as expected.
>
> diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c
> index 519cdb9..62f174a 100644
> --- a/net/bluetooth/hidp/core.c
> +++ b/net/bluetooth/hidp/core.c
> @@ -676,6 +676,12 @@ static const struct {
>  	__u16 idProduct;
>  	unsigned quirks;
>  } hidp_blacklist[] = {
> +	/* Apple Wireless Keyboard, ANSI layout */
> +	{ 0x05ac, 0x022c, HID_QUIRK_APPLE_NUMLOCK_EMULATION | HID_QUIRK_APPLE_HAS_FN },
> +	/* Apple Wireless Keyboard, ISO layout */
> +	{ 0x05ac, 0x022d, HID_QUIRK_APPLE_NUMLOCK_EMULATION | HID_QUIRK_APPLE_HAS_FN | HID_QUIRK_APPLE_ISO_KEYBOARD },
> +	/* Apple Wireless Keyboard, JIS layout */
> +	{ 0x05ac, 0x022e, HID_QUIRK_APPLE_NUMLOCK_EMULATION | HID_QUIRK_APPLE_HAS_FN },
>  	/* Apple wireless Mighty Mouse */
>  	{ 0x05ac, 0x030c, HID_QUIRK_MIGHTYMOUSE | HID_QUIRK_INVERT_HWHEEL },

Why do you set HID_QUIRK_APPLE_NUMLOCK_EMULATION ?  I don't know what 
that quirk really does, but since these keyboards have neither a 
numeric keypad nor a numlock key I can't see any benefit.


Phil.




^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] [RESEND] fix HID quirks for aluminium apple wireless keyboards
  2008-06-14 12:22       ` Marcel Holtmann
@ 2008-06-17 13:54         ` Jiri Kosina
  0 siblings, 0 replies; 12+ messages in thread
From: Jiri Kosina @ 2008-06-17 13:54 UTC (permalink / raw)
  To: Marcel Holtmann; +Cc: Paul Collins, linux-input, linux-kernel, linux-usb

On Sat, 14 Jun 2008, Marcel Holtmann wrote:

> > diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c
> > index 519cdb9..62f174a 100644
> > --- a/net/bluetooth/hidp/core.c
> > +++ b/net/bluetooth/hidp/core.c
> > @@ -676,6 +676,12 @@ static const struct {
> >  	__u16 idProduct;
> >  	unsigned quirks;
> >  } hidp_blacklist[] = {
> > +	/* Apple Wireless Keyboard, ANSI layout */
> > +	{ 0x05ac, 0x022c, HID_QUIRK_APPLE_NUMLOCK_EMULATION | HID_QUIRK_APPLE_HAS_FN },
> > +	/* Apple Wireless Keyboard, ISO layout */
> > +	{ 0x05ac, 0x022d, HID_QUIRK_APPLE_NUMLOCK_EMULATION | HID_QUIRK_APPLE_HAS_FN | HID_QUIRK_APPLE_ISO_KEYBOARD },
> > +	/* Apple Wireless Keyboard, JIS layout */
> > +	{ 0x05ac, 0x022e, HID_QUIRK_APPLE_NUMLOCK_EMULATION | HID_QUIRK_APPLE_HAS_FN },
> >  	/* Apple wireless Mighty Mouse */
> >  	{ 0x05ac, 0x030c, HID_QUIRK_MIGHTYMOUSE | HID_QUIRK_INVERT_HWHEEL },
> However depending on if Jiri's changes for the HID bus are ready, this
> patch might become obsolete.

This is however still not merged yet -- there is still some minor work to 
do. So I am now merging the incoming stuff as it comes, and hidbus patches 
will then be easily rebased.

So please, proceed with this patch through your/DaveM's tree, if you like 
to.

Thanks,

-- 
Jiri Kosina
SUSE Labs

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] [RESEND] fix HID quirks for aluminium apple wireless keyboards
  2008-06-14 12:51       ` Phil Endecott
@ 2008-06-18 10:19         ` Paul Collins
  2008-06-18 10:47           ` Phil Endecott
  0 siblings, 1 reply; 12+ messages in thread
From: Paul Collins @ 2008-06-18 10:19 UTC (permalink / raw)
  To: Phil Endecott
  Cc: jkosina, linux-input, linux-kernel, linux-usb, Marcel Holtmann

"Phil Endecott" <phil_tuhck_endecott@chezphil.org> writes:

> Why do you set HID_QUIRK_APPLE_NUMLOCK_EMULATION ?  I don't know what
> that quirk really does, but since these keyboards have neither a
> numeric keypad nor a numlock key I can't see any benefit.

>From a quick skim of the code (drivers/hid/hid-input.c) it looks like
it's supposed to allow a laptop-style numeric keymap mapping to be
enabled with F6, but I can't make it do anything here.  Perhaps it is
best removed after the hid-bus merge, otherwise there'll be different
quirks in different places for the same devices.

-- 
Paul Collins
Wellington, New Zealand

Dag vijandelijk luchtschip de huismeester is dood

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] [RESEND] fix HID quirks for aluminium apple wireless keyboards
  2008-06-18 10:19         ` Paul Collins
@ 2008-06-18 10:47           ` Phil Endecott
  0 siblings, 0 replies; 12+ messages in thread
From: Phil Endecott @ 2008-06-18 10:47 UTC (permalink / raw)
  To: Paul Collins
  Cc: jkosina, linux-input, linux-kernel, linux-usb, Marcel Holtmann,
	Phil Endecott

Paul Collins wrote:
> "Phil Endecott" <phil_tuhck_endecott@chezphil.org> writes:
>
>> Why do you set HID_QUIRK_APPLE_NUMLOCK_EMULATION ?  I don't know what
>> that quirk really does, but since these keyboards have neither a
>> numeric keypad nor a numlock key I can't see any benefit.
>
> From a quick skim of the code (drivers/hid/hid-input.c) it looks like
> it's supposed to allow a laptop-style numeric keymap mapping to be
> enabled with F6, but I can't make it do anything here.  Perhaps it is
> best removed after the hid-bus merge, otherwise there'll be different
> quirks in different places for the same devices.

The general policy seems to be to make these keyboards behave the same 
on Linux as they do with Macs.  My guess is that they don't have any 
numeric keypad emulation feature on Macs - can anyone confirm?  In that 
case it shouldn't be enabled.  Personally, I wouldn't like to 
accidentally press F6 (e.g. pressing all the fn keys in turn looking 
for the "right one"), and then discover that half of the 
letter/punctuation keys were broken, and have no idea how to "fix" it.


Phil.





^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] [RESEND] fix HID quirks for aluminium apple wireless keyboards
  2008-06-14  8:03     ` Paul Collins
  2008-06-14 12:22       ` Marcel Holtmann
  2008-06-14 12:51       ` Phil Endecott
@ 2008-06-27 16:32       ` Jan Scholz
  2 siblings, 0 replies; 12+ messages in thread
From: Jan Scholz @ 2008-06-27 16:32 UTC (permalink / raw)
  To: paul; +Cc: marcel, jkosina, linux-kernel

Hi Paul,

I applied your patch from http://lkml.org/lkml/2008/6/14/26 but
without HID_QUIRK_APPLE_NUMLOCK_EMULATION to v2.6.25, but on my G4-ppc
iBook the quirk is not found by hidp_setup_quirks.
If I remove the calls to le16_to_cpu from that function it
works on my ppc, looks like some endianness issue, see the second patch. 


diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c
index 519cdb9..f96fb6e 100644
--- a/net/bluetooth/hidp/core.c
+++ b/net/bluetooth/hidp/core.c
@@ -676,6 +676,12 @@ static const struct {
 	__u16 idProduct;
 	unsigned quirks;
 } hidp_blacklist[] = {
+	/* Apple Wireless Keyboard, ANSI layout */
+	{ 0x05ac, 0x022c, HID_QUIRK_APPLE_HAS_FN },
+	/* Apple Wireless Keyboard, ISO layout */
+	{ 0x05ac, 0x022d, HID_QUIRK_APPLE_HAS_FN | HID_QUIRK_APPLE_ISO_KEYBOARD },
+	/* Apple Wireless Keyboard, JIS layout */
+	{ 0x05ac, 0x022e, HID_QUIRK_APPLE_HAS_FN },
 	/* Apple wireless Mighty Mouse */
 	{ 0x05ac, 0x030c, HID_QUIRK_MIGHTYMOUSE | HID_QUIRK_INVERT_HWHEEL },
 


diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c
index f96fb6e..46f5958 100644
--- a/net/bluetooth/hidp/core.c
+++ b/net/bluetooth/hidp/core.c
@@ -693,8 +693,8 @@ static void hidp_setup_quirks(struct hid_device *hid)
 	unsigned int n;
 
 	for (n = 0; hidp_blacklist[n].idVendor; n++)
-		if (hidp_blacklist[n].idVendor == le16_to_cpu(hid->vendor) &&
-				hidp_blacklist[n].idProduct == le16_to_cpu(hid->product))
+		if (hidp_blacklist[n].idVendor == hid->vendor &&
+				hidp_blacklist[n].idProduct == hid->product)
 			hid->quirks = hidp_blacklist[n].quirks;
 }
 

--
 Jan Scholz

^ permalink raw reply related	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2008-06-27 17:01 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-06-12 12:26 [PATCH] fix HID quirks for aluminium apple wireless keyboards Paul Collins
2008-06-12 12:57 ` [PATCH] [RESEND] " Paul Collins
2008-06-12 13:17   ` Phil Endecott
2008-06-12 14:55     ` Paul Collins
2008-06-12 20:08   ` Marcel Holtmann
2008-06-14  8:03     ` Paul Collins
2008-06-14 12:22       ` Marcel Holtmann
2008-06-17 13:54         ` Jiri Kosina
2008-06-14 12:51       ` Phil Endecott
2008-06-18 10:19         ` Paul Collins
2008-06-18 10:47           ` Phil Endecott
2008-06-27 16:32       ` Jan Scholz

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).