Patchwork HID: Add special driver for Jabra devices

login
register
mail settings
Submitter nolsen@jabra.com
Date Sept. 1, 2017, 12:43 p.m.
Message ID <1504269832-23142-1-git-send-email-nolsen@jabra.com>
Download mbox | patch
Permalink /patch/9934051/
State New
Headers show

Comments

nolsen@jabra.com - Sept. 1, 2017, 12:43 p.m.
From: Niels Skou Olsen <nolsen@jabra.com>

Add a hid-jabra driver to the list of special drivers in hid-core. The
driver prevents vendor defined HID usages (FF00-FFFF) in Jabra devices
from being mapped to input events that become unintended mouse events
in the X11 server.

Signed-off-by: Niels Skou Olsen <nolsen@jabra.com>
---
 drivers/hid/Kconfig     | 11 ++++++++++
 drivers/hid/Makefile    |  1 +
 drivers/hid/hid-core.c  |  3 +++
 drivers/hid/hid-jabra.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 73 insertions(+)
 create mode 100644 drivers/hid/hid-jabra.c
Jiri Kosina - Sept. 6, 2017, 9:25 a.m.
On Fri, 1 Sep 2017, nolsen@jabra.com wrote:

> From: Niels Skou Olsen <nolsen@jabra.com>
> 
> Add a hid-jabra driver to the list of special drivers in hid-core. The
> driver prevents vendor defined HID usages (FF00-FFFF) in Jabra devices
> from being mapped to input events that become unintended mouse events
> in the X11 server.
> 
> Signed-off-by: Niels Skou Olsen <nolsen@jabra.com>

Do you plan to eventually map the vendor specific devices to anything 
meaningful?

Thanks,
nolsen@jabra.com - Sept. 6, 2017, 12:10 p.m.
On Thu, 6 Sep 2017, jikos@kernel.org wrote:

> On Fri, 1 Sep 2017, nolsen@jabra.com wrote:
>
> > From: Niels Skou Olsen <nolsen@jabra.com>
> >
> > Add a hid-jabra driver to the list of special drivers in hid-core. The
> > driver prevents vendor defined HID usages (FF00-FFFF) in Jabra devices
> > from being mapped to input events that become unintended mouse events
> > in the X11 server.
> >
> > Signed-off-by: Niels Skou Olsen <nolsen@jabra.com>
>
> Do you plan to eventually map the vendor specific devices to anything meaningful?

Currently there is no need, but we might need to in future devices/firmware versions.

The standard usages (Consumer, Button, etc) are handled appropriately by the default mapping in hidinput_configure_usage(). But unfortunately some of our vendor defined usages are mapped to events that do not make sense.

As an example, here's an excerpt of the sysfs rdesc for one of our devices. The vendor defined usages FF30.xxxx are mapped like this:

ff30.0020 ---> Key.Btn0
ff30.0097 ---> Key.Btn1
ff30.002f ---> Key.Btn2
ff30.0021 ---> Key.Btn3
ff30.0024 ---> Key.Btn4
ff30.fffd ---> Key.Btn5
ff30.0050 ---> Key.Btn6
ff30.00b0 ---> Absolute.Misc
ff30.00b1 ---> Absolute.?
ff30.00b2 ---> Absolute.?
ff30.00b3 ---> Absolute.?
ff30.00b4 ---> Absolute.?
ff30.00b5 ---> Absolute.?
ff30.00b6 ---> Absolute.?
ff30.00b7 ---> Absolute.?
ff30.00b8 ---> Absolute.MTMajor
ff30.00b9 ---> Absolute.MTMinor
ff30.00ba ---> Absolute.MTMajorW
ff30.00bb ---> Absolute.MTMinorW

All of these should be ignored by the input mapping.

Thanks,
Niels
**** GN GROUP NOTICE - AUTOMATICALLY INSERTED**** The information in this e-mail (including attachments, if any) is considered confidential and is intended only for the recipient(s) listed above. Any review, use, disclosure, distribution or copying of this e-mail is prohibited except by or on behalf of the intended recipient. If you have received this email in error, please notify me immediately by reply e-mail, delete this e-mail, and do not disclose its contents to anyone. Any opinions expressed in this e-mail are those of the individual and not necessarily the GN group. Thank you. ******************** DISCLAIMER END ************************
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jiri Kosina - Sept. 8, 2017, 12:10 p.m.
On Fri, 1 Sep 2017, nolsen@jabra.com wrote:

> From: Niels Skou Olsen <nolsen@jabra.com>
> 
> Add a hid-jabra driver to the list of special drivers in hid-core. The
> driver prevents vendor defined HID usages (FF00-FFFF) in Jabra devices
> from being mapped to input events that become unintended mouse events
> in the X11 server.
> 
> Signed-off-by: Niels Skou Olsen <nolsen@jabra.com>
> ---
>  drivers/hid/Kconfig     | 11 ++++++++++
>  drivers/hid/Makefile    |  1 +
>  drivers/hid/hid-core.c  |  3 +++
>  drivers/hid/hid-jabra.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 73 insertions(+)
>  create mode 100644 drivers/hid/hid-jabra.c
> 
> diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
> index 0a3117c..d9d2843 100644
> --- a/drivers/hid/Kconfig
> +++ b/drivers/hid/Kconfig
> @@ -395,6 +395,17 @@ config HID_ITE
>  	---help---
>  	Support for ITE devices not fully compliant with HID standard.
>  
> +config HID_JABRA
> +	tristate "Jabra USB HID Driver"
> +	depends on HID
> +	---help---
> +	Support for Jabra USB HID devices.
> +
> +	Prevents mapping of vendor defined HID usages to input events. Without
> +	this driver HID	reports from Jabra devices may incorrectly be seen as
> +	mouse button events.
> +	Say M here if you may ever plug in a Jabra USB device.
> +
>  config HID_TWINHAN
>  	tristate "Twinhan IR remote control"
>  	depends on HID
> diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
> index 8659d7e..d2563e5 100644
> --- a/drivers/hid/Makefile
> +++ b/drivers/hid/Makefile
> @@ -51,6 +51,7 @@ obj-$(CONFIG_HID_HOLTEK)	+= hid-holtekff.o
>  obj-$(CONFIG_HID_HYPERV_MOUSE)	+= hid-hyperv.o
>  obj-$(CONFIG_HID_ICADE)		+= hid-icade.o
>  obj-$(CONFIG_HID_ITE)		+= hid-ite.o
> +obj-$(CONFIG_HID_JABRA)		+= hid-jabra.o
>  obj-$(CONFIG_HID_KENSINGTON)	+= hid-kensington.o
>  obj-$(CONFIG_HID_KEYTOUCH)	+= hid-keytouch.o
>  obj-$(CONFIG_HID_KYE)		+= hid-kye.o
> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
> index 9bc9116..0603012 100644
> --- a/drivers/hid/hid-core.c
> +++ b/drivers/hid/hid-core.c
> @@ -2077,6 +2077,9 @@ static const struct hid_device_id hid_have_special_driver[] = {
>  #if IS_ENABLED(CONFIG_HID_ITE)
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_ITE, USB_DEVICE_ID_ITE8595) },
>  #endif
> +#if IS_ENABLED(CONFIG_HID_JABRA)
> +	{ HID_USB_DEVICE(USB_VENDOR_ID_JABRA, HID_ANY_ID) },
> +#endif
>  #if IS_ENABLED(CONFIG_HID_KENSINGTON)
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_KENSINGTON, USB_DEVICE_ID_KS_SLIMBLADE) },
>  #endif
> diff --git a/drivers/hid/hid-jabra.c b/drivers/hid/hid-jabra.c
> new file mode 100644
> index 0000000..1f52daf
> --- /dev/null
> +++ b/drivers/hid/hid-jabra.c
> @@ -0,0 +1,58 @@
> +/*
> + *  Jabra USB HID Driver
> + *
> + *  Copyright (c) 2017 Niels Skou Olsen <nolsen@jabra.com>
> + */
> +
> +/*
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by the Free
> + * Software Foundation; either version 2 of the License, or (at your option)
> + * any later version.
> + */
> +
> +#include <linux/hid.h>
> +#include <linux/module.h>
> +
> +#include "hid-ids.h"
> +
> +#define HID_UP_VENDOR_DEFINED_MIN	0xff000000
> +#define HID_UP_VENDOR_DEFINED_MAX	0xffff0000
> +
> +static int jabra_input_mapping(struct hid_device *hdev,
> +			       struct hid_input *hi,
> +			       struct hid_field *field,
> +			       struct hid_usage *usage,
> +			       unsigned long **bit, int *max)
> +{
> +	int is_vendor_defined =
> +		((usage->hid & HID_USAGE_PAGE) >= HID_UP_VENDOR_DEFINED_MIN &&
> +		 (usage->hid & HID_USAGE_PAGE) <= HID_UP_VENDOR_DEFINED_MAX);
> +
> +	dbg_hid("hid=0x%08x appl=0x%08x coll_idx=0x%02x usage_idx=0x%02x: %s\n",
> +		usage->hid,
> +		field->application,
> +		usage->collection_index,
> +		usage->usage_index,
> +		is_vendor_defined ? "ignored" : "defaulted");
> +
> +	/* Ignore vendor defined usages, default map standard usages */
> +	return is_vendor_defined ? -1 : 0;
> +}
> +
> +static const struct hid_device_id jabra_devices[] = {
> +	{ HID_USB_DEVICE(USB_VENDOR_ID_JABRA, HID_ANY_ID) },

You also need to update hid-core so that generic driver doesn't bind to 
those device. See hid_have_special_driver[] in drivers/hid/hid-core.c

Thanks,
nolsen@jabra.com - Sept. 8, 2017, 12:58 p.m.
On Fri, 8 Sep 2017, jikos@kernel.org write:

> On Fri, 1 Sep 2017, nolsen@jabra.com wrote:
>
> > From: Niels Skou Olsen <nolsen@jabra.com>
> >
> > Add a hid-jabra driver to the list of special drivers in hid-core. The
> > driver prevents vendor defined HID usages (FF00-FFFF) in Jabra devices
> > from being mapped to input events that become unintended mouse events
> > in the X11 server.
> >
> > Signed-off-by: Niels Skou Olsen <nolsen@jabra.com>
> > ---
> >  drivers/hid/Kconfig     | 11 ++++++++++
> >  drivers/hid/Makefile    |  1 +
> >  drivers/hid/hid-core.c  |  3 +++
> >  drivers/hid/hid-jabra.c | 58
> > +++++++++++++++++++++++++++++++++++++++++++++++++
> >  4 files changed, 73 insertions(+)
> >  create mode 100644 drivers/hid/hid-jabra.c
> >
> > diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig index
> > 0a3117c..d9d2843 100644
> > --- a/drivers/hid/Kconfig
> > +++ b/drivers/hid/Kconfig
> > @@ -395,6 +395,17 @@ config HID_ITE
> >  ---help---
> >  Support for ITE devices not fully compliant with HID standard.
> >
> > +config HID_JABRA
> > +tristate "Jabra USB HID Driver"
> > +depends on HID
> > +---help---
> > +Support for Jabra USB HID devices.
> > +
> > +Prevents mapping of vendor defined HID usages to input events.
> Without
> > +this driver HIDreports from Jabra devices may incorrectly be seen as
> > +mouse button events.
> > +Say M here if you may ever plug in a Jabra USB device.
> > +
> >  config HID_TWINHAN
> >  tristate "Twinhan IR remote control"
> >  depends on HID
> > diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile index
> > 8659d7e..d2563e5 100644
> > --- a/drivers/hid/Makefile
> > +++ b/drivers/hid/Makefile
> > @@ -51,6 +51,7 @@ obj-$(CONFIG_HID_HOLTEK)+= hid-holtekff.o
> >  obj-$(CONFIG_HID_HYPERV_MOUSE)+= hid-hyperv.o
> >  obj-$(CONFIG_HID_ICADE)+= hid-icade.o
> >  obj-$(CONFIG_HID_ITE)+= hid-ite.o
> > +obj-$(CONFIG_HID_JABRA)+= hid-jabra.o
> >  obj-$(CONFIG_HID_KENSINGTON)+= hid-kensington.o
> >  obj-$(CONFIG_HID_KEYTOUCH)+= hid-keytouch.o
> >  obj-$(CONFIG_HID_KYE)+= hid-kye.o
> > diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c index
> > 9bc9116..0603012 100644
> > --- a/drivers/hid/hid-core.c
> > +++ b/drivers/hid/hid-core.c
> > @@ -2077,6 +2077,9 @@ static const struct hid_device_id
> > hid_have_special_driver[] = {  #if IS_ENABLED(CONFIG_HID_ITE)
> >  { HID_USB_DEVICE(USB_VENDOR_ID_ITE,
> USB_DEVICE_ID_ITE8595) },
> > #endif
> > +#if IS_ENABLED(CONFIG_HID_JABRA)
> > +{ HID_USB_DEVICE(USB_VENDOR_ID_JABRA, HID_ANY_ID) },
> #endif
> >  #if IS_ENABLED(CONFIG_HID_KENSINGTON)
> >  { HID_USB_DEVICE(USB_VENDOR_ID_KENSINGTON,
> > USB_DEVICE_ID_KS_SLIMBLADE) },  #endif diff --git
> > a/drivers/hid/hid-jabra.c b/drivers/hid/hid-jabra.c new file mode
> > 100644 index 0000000..1f52daf
> > --- /dev/null
> > +++ b/drivers/hid/hid-jabra.c
> > @@ -0,0 +1,58 @@
> > +/*
> > + *  Jabra USB HID Driver
> > + *
> > + *  Copyright (c) 2017 Niels Skou Olsen <nolsen@jabra.com>  */
> > +
> > +/*
> > + * This program is free software; you can redistribute it and/or
> > +modify it
> > + * under the terms of the GNU General Public License as published by
> > +the Free
> > + * Software Foundation; either version 2 of the License, or (at your
> > +option)
> > + * any later version.
> > + */
> > +
> > +#include <linux/hid.h>
> > +#include <linux/module.h>
> > +
> > +#include "hid-ids.h"
> > +
> > +#define HID_UP_VENDOR_DEFINED_MIN0xff000000
> > +#define HID_UP_VENDOR_DEFINED_MAX0xffff0000
> > +
> > +static int jabra_input_mapping(struct hid_device *hdev,
> > +       struct hid_input *hi,
> > +       struct hid_field *field,
> > +       struct hid_usage *usage,
> > +       unsigned long **bit, int *max) {
> > +int is_vendor_defined =
> > +((usage->hid & HID_USAGE_PAGE) >=
> HID_UP_VENDOR_DEFINED_MIN &&
> > + (usage->hid & HID_USAGE_PAGE) <=
> HID_UP_VENDOR_DEFINED_MAX);
> > +
> > +dbg_hid("hid=0x%08x appl=0x%08x coll_idx=0x%02x
> usage_idx=0x%02x: %s\n",
> > +usage->hid,
> > +field->application,
> > +usage->collection_index,
> > +usage->usage_index,
> > +is_vendor_defined ? "ignored" : "defaulted");
> > +
> > +/* Ignore vendor defined usages, default map standard usages */
> > +return is_vendor_defined ? -1 : 0;
> > +}
> > +
> > +static const struct hid_device_id jabra_devices[] = {
> > +{ HID_USB_DEVICE(USB_VENDOR_ID_JABRA, HID_ANY_ID) },
>
> You also need to update hid-core so that generic driver doesn't bind to those device.
> See hid_have_special_driver[] in drivers/hid/hid-core.c

The original patch already has a hunk that patches hid_have_special_driver[] -- did you miss it, or do I need to do something else also?

Thanks,
Niels
**** GN GROUP NOTICE - AUTOMATICALLY INSERTED**** The information in this e-mail (including attachments, if any) is considered confidential and is intended only for the recipient(s) listed above. Any review, use, disclosure, distribution or copying of this e-mail is prohibited except by or on behalf of the intended recipient. If you have received this email in error, please notify me immediately by reply e-mail, delete this e-mail, and do not disclose its contents to anyone. Any opinions expressed in this e-mail are those of the individual and not necessarily the GN group. Thank you. ******************** DISCLAIMER END ************************
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
nolsen@jabra.com - Sept. 18, 2017, 6:02 a.m.
On Fri, 8 Sep 2017, jikos@kernel.org wrote:

> On Fri, 1 Sep 2017, nolsen@jabra.com wrote:
>
> > From: Niels Skou Olsen <nolsen@jabra.com>
> >
> > diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c index
> > 9bc9116..0603012 100644
> > --- a/drivers/hid/hid-core.c
> > +++ b/drivers/hid/hid-core.c
> > @@ -2077,6 +2077,9 @@ static const struct hid_device_id hid_have_special_driver[] = {
> > +#if IS_ENABLED(CONFIG_HID_JABRA)
> > +{ HID_USB_DEVICE(USB_VENDOR_ID_JABRA, HID_ANY_ID) },
> > +#endif
>
> You also need to update hid-core so that generic driver doesn't bind to those device.
> See hid_have_special_driver[] in drivers/hid/hid-core.c

Isn't above addition to hid_have_special_driver[] enough? It is in the original patch.

Thanks,
Niels

**** GN GROUP NOTICE - AUTOMATICALLY INSERTED**** The information in this e-mail (including attachments, if any) is considered confidential and is intended only for the recipient(s) listed above. Any review, use, disclosure, distribution or copying of this e-mail is prohibited except by or on behalf of the intended recipient. If you have received this email in error, please notify me immediately by reply e-mail, delete this e-mail, and do not disclose its contents to anyone. Any opinions expressed in this e-mail are those of the individual and not necessarily the GN group. Thank you. ******************** DISCLAIMER END ************************
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index 0a3117c..d9d2843 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -395,6 +395,17 @@  config HID_ITE
 	---help---
 	Support for ITE devices not fully compliant with HID standard.
 
+config HID_JABRA
+	tristate "Jabra USB HID Driver"
+	depends on HID
+	---help---
+	Support for Jabra USB HID devices.
+
+	Prevents mapping of vendor defined HID usages to input events. Without
+	this driver HID	reports from Jabra devices may incorrectly be seen as
+	mouse button events.
+	Say M here if you may ever plug in a Jabra USB device.
+
 config HID_TWINHAN
 	tristate "Twinhan IR remote control"
 	depends on HID
diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
index 8659d7e..d2563e5 100644
--- a/drivers/hid/Makefile
+++ b/drivers/hid/Makefile
@@ -51,6 +51,7 @@  obj-$(CONFIG_HID_HOLTEK)	+= hid-holtekff.o
 obj-$(CONFIG_HID_HYPERV_MOUSE)	+= hid-hyperv.o
 obj-$(CONFIG_HID_ICADE)		+= hid-icade.o
 obj-$(CONFIG_HID_ITE)		+= hid-ite.o
+obj-$(CONFIG_HID_JABRA)		+= hid-jabra.o
 obj-$(CONFIG_HID_KENSINGTON)	+= hid-kensington.o
 obj-$(CONFIG_HID_KEYTOUCH)	+= hid-keytouch.o
 obj-$(CONFIG_HID_KYE)		+= hid-kye.o
diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index 9bc9116..0603012 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -2077,6 +2077,9 @@  static const struct hid_device_id hid_have_special_driver[] = {
 #if IS_ENABLED(CONFIG_HID_ITE)
 	{ HID_USB_DEVICE(USB_VENDOR_ID_ITE, USB_DEVICE_ID_ITE8595) },
 #endif
+#if IS_ENABLED(CONFIG_HID_JABRA)
+	{ HID_USB_DEVICE(USB_VENDOR_ID_JABRA, HID_ANY_ID) },
+#endif
 #if IS_ENABLED(CONFIG_HID_KENSINGTON)
 	{ HID_USB_DEVICE(USB_VENDOR_ID_KENSINGTON, USB_DEVICE_ID_KS_SLIMBLADE) },
 #endif
diff --git a/drivers/hid/hid-jabra.c b/drivers/hid/hid-jabra.c
new file mode 100644
index 0000000..1f52daf
--- /dev/null
+++ b/drivers/hid/hid-jabra.c
@@ -0,0 +1,58 @@ 
+/*
+ *  Jabra USB HID Driver
+ *
+ *  Copyright (c) 2017 Niels Skou Olsen <nolsen@jabra.com>
+ */
+
+/*
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the Free
+ * Software Foundation; either version 2 of the License, or (at your option)
+ * any later version.
+ */
+
+#include <linux/hid.h>
+#include <linux/module.h>
+
+#include "hid-ids.h"
+
+#define HID_UP_VENDOR_DEFINED_MIN	0xff000000
+#define HID_UP_VENDOR_DEFINED_MAX	0xffff0000
+
+static int jabra_input_mapping(struct hid_device *hdev,
+			       struct hid_input *hi,
+			       struct hid_field *field,
+			       struct hid_usage *usage,
+			       unsigned long **bit, int *max)
+{
+	int is_vendor_defined =
+		((usage->hid & HID_USAGE_PAGE) >= HID_UP_VENDOR_DEFINED_MIN &&
+		 (usage->hid & HID_USAGE_PAGE) <= HID_UP_VENDOR_DEFINED_MAX);
+
+	dbg_hid("hid=0x%08x appl=0x%08x coll_idx=0x%02x usage_idx=0x%02x: %s\n",
+		usage->hid,
+		field->application,
+		usage->collection_index,
+		usage->usage_index,
+		is_vendor_defined ? "ignored" : "defaulted");
+
+	/* Ignore vendor defined usages, default map standard usages */
+	return is_vendor_defined ? -1 : 0;
+}
+
+static const struct hid_device_id jabra_devices[] = {
+	{ HID_USB_DEVICE(USB_VENDOR_ID_JABRA, HID_ANY_ID) },
+	{ }
+};
+MODULE_DEVICE_TABLE(hid, jabra_devices);
+
+static struct hid_driver jabra_driver = {
+	.name = "jabra",
+	.id_table = jabra_devices,
+	.input_mapping = jabra_input_mapping,
+};
+module_hid_driver(jabra_driver);
+
+MODULE_AUTHOR("Niels Skou Olsen <nolsen@jabra.com>");
+MODULE_DESCRIPTION("Jabra USB HID Driver");
+MODULE_LICENSE("GPL");