linux-input.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] HID: gembird: add new driver to fix Gembird JPD-DualForce 2
@ 2015-08-11 15:35 Benjamin Tissoires
  2015-08-12  1:32 ` Orivej Desh
  0 siblings, 1 reply; 5+ messages in thread
From: Benjamin Tissoires @ 2015-08-11 15:35 UTC (permalink / raw)
  To: Jiri Kosina, Orivej Desh; +Cc: linux-kernel, linux-input, Benjamin Tissoires

This gamepad advertise 5 absolute axis while 4 are actually used.
The second Z axis shows some garbage, so it has to be ignored by HID.
The first Z axis and the Rz one are actually Rx and Ry. Remap them.

We could also just remap and ignore the axis in .input_mapping(). I
went ahead with .report_fixup() first, so here it is.

Reported-by: Orivej Desh <orivej@gmx.fr>
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---

Hi,

so here are the fixes I mentioned yesterday. If you could give a quick shot
and a "tested-by" that would be even better.

Jiri, I am not sure if the .input_mapping() alternative would not be better.
Both solutions are valid, so if you do not like my memcpys, I can re-write the
patch the other way.

Cheers,
Benjamin

 drivers/hid/Kconfig       |   6 +++
 drivers/hid/Makefile      |   1 +
 drivers/hid/hid-core.c    |   1 +
 drivers/hid/hid-gembird.c | 116 ++++++++++++++++++++++++++++++++++++++++++++++
 drivers/hid/hid-ids.h     |   3 ++
 5 files changed, 127 insertions(+)
 create mode 100644 drivers/hid/hid-gembird.c

diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index 57c94d7..6ab51ae 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -251,6 +251,12 @@ config HID_EZKEY
 	---help---
 	Support for Ezkey BTC 8193 keyboard.
 
+config HID_GEMBIRD
+	tristate "Gembird Joypad"
+	depends on HID
+	---help---
+	Support for Gembird JPD-DualForce 2.
+
 config HID_HOLTEK
 	tristate "Holtek HID devices"
 	depends on USB_HID
diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
index 2f8a41d..e6441bc 100644
--- a/drivers/hid/Makefile
+++ b/drivers/hid/Makefile
@@ -36,6 +36,7 @@ obj-$(CONFIG_HID_EMS_FF)	+= hid-emsff.o
 obj-$(CONFIG_HID_ELECOM)	+= hid-elecom.o
 obj-$(CONFIG_HID_ELO)		+= hid-elo.o
 obj-$(CONFIG_HID_EZKEY)		+= hid-ezkey.o
+obj-$(CONFIG_HID_GEMBIRD)	+= hid-gembird.o
 obj-$(CONFIG_HID_GT683R)	+= hid-gt683r.o
 obj-$(CONFIG_HID_GYRATION)	+= hid-gyration.o
 obj-$(CONFIG_HID_HOLTEK)	+= hid-holtek-kbd.o
diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index e982e70..22afab9 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -1844,6 +1844,7 @@ static const struct hid_device_id hid_have_special_driver[] = {
 	{ HID_USB_DEVICE(USB_VENDOR_ID_EZKEY, USB_DEVICE_ID_BTC_8193) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_GAMERON, USB_DEVICE_ID_GAMERON_DUAL_PSX_ADAPTOR) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_GAMERON, USB_DEVICE_ID_GAMERON_DUAL_PCS_ADAPTOR) },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_GEMBIRD, USB_DEVICE_ID_GEMBIRD_JPD_DUALFORCE2) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_GREENASIA, 0x0003) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_GREENASIA, 0x0012) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_GYRATION, USB_DEVICE_ID_GYRATION_REMOTE) },
diff --git a/drivers/hid/hid-gembird.c b/drivers/hid/hid-gembird.c
new file mode 100644
index 0000000..cefc394
--- /dev/null
+++ b/drivers/hid/hid-gembird.c
@@ -0,0 +1,116 @@
+/*
+ *  HID driver for Gembird Joypad, "PC Game Controller"
+ *
+ *  Copyright (c) 2015 Red Hat, Inc
+ *  Copyright (c) 2015 Benjamin Tissoires
+ */
+
+/*
+ * 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/device.h>
+#include <linux/hid.h>
+#include <linux/module.h>
+
+#include "hid-ids.h"
+
+#define GEMBIRD_START_FAULTY_RDESC	8
+
+static const __u8 gembird_jpd_faulty_rdesc[] = {
+	0x75, 0x08,			/*   Report Size (8)		*/
+	0x95, 0x05,			/*   Report Count (5)		*/
+	0x15, 0x00,			/*   Logical Minimum (0)	*/
+	0x26, 0xff, 0x00,		/*   Logical Maximum (255)	*/
+	0x35, 0x00,			/*   Physical Minimum (0)	*/
+	0x46, 0xff, 0x00,		/*   Physical Maximum (255)	*/
+	0x09, 0x30,			/*   Usage (X)			*/
+	0x09, 0x31,			/*   Usage (Y)			*/
+	0x09, 0x32,			/*   Usage (Z)			*/
+	0x09, 0x32,			/*   Usage (Z)			*/
+	0x09, 0x35,			/*   Usage (Rz)			*/
+	0x81, 0x02,			/*   Input (Data,Var,Abs)	*/
+};
+
+/*
+ * we fix the report descriptor by:
+ * - re-assigning X to the original Z axis (it will get ignored this way
+ * - assign the original second Z to Rx
+ * - assign the original Rz to Ry
+ */
+static const __u8 gembird_jpd_fixed_rdesc[] = {
+	0x75, 0x08,			/*   Report Size (8)		*/
+	0x95, 0x02,			/*   Report Count (2)		*/
+	0x15, 0x00,			/*   Logical Minimum (0)	*/
+	0x26, 0xff, 0x00,		/*   Logical Maximum (255)	*/
+	0x35, 0x00,			/*   Physical Minimum (0)	*/
+	0x46, 0xff, 0x00,		/*   Physical Maximum (255)	*/
+	0x09, 0x30,			/*   Usage (X)			*/
+	0x09, 0x31,			/*   Usage (Y)			*/
+	0x81, 0x02,			/*   Input (Data,Var,Abs)	*/
+	0x95, 0x01,			/*   Report Count (1)		*/
+	0x09, 0x32,			/*   Usage (Z)			*/
+	0x81, 0x01,			/*   Input (Cnst,Arr,Abs)	*/
+	0x95, 0x02,			/*   Report Count (2)		*/
+	0x09, 0x33,			/*   Usage (Rx)			*/
+	0x09, 0x34,			/*   Usage (Ry)			*/
+	0x81, 0x02,			/*   Input (Data,Var,Abs)	*/
+};
+
+static __u8 *gembird_report_fixup(struct hid_device *hdev, __u8 *rdesc,
+		unsigned int *rsize)
+{
+	__u8 *new_rdesc;
+	/* delta_size is > 0 */
+	size_t delta_size = sizeof(gembird_jpd_fixed_rdesc) -
+			    sizeof(gembird_jpd_faulty_rdesc);
+	size_t new_size = *rsize + delta_size;
+
+	if (*rsize >= 31 && !memcmp(&rdesc[GEMBIRD_START_FAULTY_RDESC],
+				    gembird_jpd_faulty_rdesc,
+				    sizeof(gembird_jpd_faulty_rdesc))) {
+		new_rdesc = devm_kzalloc(&hdev->dev, new_size, GFP_KERNEL);
+		if (new_rdesc == NULL)
+			return rdesc;
+
+		dev_info(&hdev->dev,
+			 "fixing Gembird JPD-DualForce 2 report descriptor.\n");
+
+		/* start by copying the end of the rdesc */
+		memcpy(new_rdesc + delta_size, rdesc, *rsize);
+
+		/* add the correct beginning */
+		memcpy(new_rdesc, rdesc, GEMBIRD_START_FAULTY_RDESC);
+
+		/* replace the faulty part with the fixed one */
+		memcpy(new_rdesc + GEMBIRD_START_FAULTY_RDESC,
+		       gembird_jpd_fixed_rdesc,
+		       sizeof(gembird_jpd_fixed_rdesc));
+
+		*rsize = new_size;
+		rdesc = new_rdesc;
+	}
+
+	return rdesc;
+}
+
+static const struct hid_device_id gembird_devices[] = {
+	{ HID_USB_DEVICE(USB_VENDOR_ID_GEMBIRD,
+			 USB_DEVICE_ID_GEMBIRD_JPD_DUALFORCE2) },
+	{ }
+};
+MODULE_DEVICE_TABLE(hid, gembird_devices);
+
+static struct hid_driver gembird_driver = {
+	.name = "gembird",
+	.id_table = gembird_devices,
+	.report_fixup = gembird_report_fixup,
+};
+module_hid_driver(gembird_driver);
+
+MODULE_AUTHOR("Benjamin Tissoires <benjamin.tissoires@gmail.com>");
+MODULE_DESCRIPTION("HID Gembird joypad driver");
+MODULE_LICENSE("GPL");
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 602f2b7..720d5e3 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -363,6 +363,9 @@
 #define USB_DEVICE_ID_GAMERON_DUAL_PSX_ADAPTOR	0x0001
 #define USB_DEVICE_ID_GAMERON_DUAL_PCS_ADAPTOR	0x0002
 
+#define USB_VENDOR_ID_GEMBIRD			0x11ff
+#define USB_DEVICE_ID_GEMBIRD_JPD_DUALFORCE2	0x3331
+
 #define USB_VENDOR_ID_GENERAL_TOUCH	0x0dfc
 #define USB_DEVICE_ID_GENERAL_TOUCH_WIN7_TWOFINGERS 0x0003
 #define USB_DEVICE_ID_GENERAL_TOUCH_WIN8_PWT_TENFINGERS 0x0100
-- 
2.4.3


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

* Re: [PATCH] HID: gembird: add new driver to fix Gembird JPD-DualForce 2
  2015-08-11 15:35 [PATCH] HID: gembird: add new driver to fix Gembird JPD-DualForce 2 Benjamin Tissoires
@ 2015-08-12  1:32 ` Orivej Desh
  2015-08-12 14:40   ` Benjamin Tissoires
  0 siblings, 1 reply; 5+ messages in thread
From: Orivej Desh @ 2015-08-12  1:32 UTC (permalink / raw)
  To: Benjamin Tissoires; +Cc: Jiri Kosina, linux-kernel, linux-input

This gamepad advertises 5 absolute axis while 4 are actually used.
The first Z axis shows some garbage, so it has to be ignored by HID.
The second Z axis and the Rz one are actually Rx and Ry. Remap them.

We could also just remap and ignore the axis in .input_mapping(). I
went ahead with .report_fixup() first, so here it is.

Reported-by: Orivej Desh <orivej@gmx.fr>
Tested-by: Orivej Desh <orivej@gmx.fr>
---

* Benjamin Tissoires
> so here are the fixes I mentioned yesterday. If you could give a quick shot
> and a "tested-by" that would be even better.

I tested your patch and it works as advertised, thank you!  But your
comment about the implementation («re-assigning X to the original Z
axis») did not match the implementation, so I simplified the
implementation according to the comment, and tested it too.  Since the
patch has changed, I think you need to sign it off again.

> Jiri, I am not sure if the .input_mapping() alternative would not be better.
> Both solutions are valid, so if you do not like my memcpys, I can re-write the
> patch the other way.

 drivers/hid/Kconfig       |  6 ++++
 drivers/hid/Makefile      |  1 +
 drivers/hid/hid-core.c    |  1 +
 drivers/hid/hid-gembird.c | 77 +++++++++++++++++++++++++++++++++++++++++++++++
 drivers/hid/hid-ids.h     |  3 ++
 5 files changed, 88 insertions(+)
 create mode 100644 drivers/hid/hid-gembird.c

diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index 57c94d7..6ab51ae 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -251,6 +251,12 @@ config HID_EZKEY
 	---help---
 	Support for Ezkey BTC 8193 keyboard.
 
+config HID_GEMBIRD
+	tristate "Gembird Joypad"
+	depends on HID
+	---help---
+	Support for Gembird JPD-DualForce 2.
+
 config HID_HOLTEK
 	tristate "Holtek HID devices"
 	depends on USB_HID
diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
index 2f8a41d..e6441bc 100644
--- a/drivers/hid/Makefile
+++ b/drivers/hid/Makefile
@@ -36,6 +36,7 @@ obj-$(CONFIG_HID_EMS_FF)	+= hid-emsff.o
 obj-$(CONFIG_HID_ELECOM)	+= hid-elecom.o
 obj-$(CONFIG_HID_ELO)		+= hid-elo.o
 obj-$(CONFIG_HID_EZKEY)		+= hid-ezkey.o
+obj-$(CONFIG_HID_GEMBIRD)	+= hid-gembird.o
 obj-$(CONFIG_HID_GT683R)	+= hid-gt683r.o
 obj-$(CONFIG_HID_GYRATION)	+= hid-gyration.o
 obj-$(CONFIG_HID_HOLTEK)	+= hid-holtek-kbd.o
diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index e982e70..22afab9 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -1844,6 +1844,7 @@ static const struct hid_device_id hid_have_special_driver[] = {
 	{ HID_USB_DEVICE(USB_VENDOR_ID_EZKEY, USB_DEVICE_ID_BTC_8193) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_GAMERON, USB_DEVICE_ID_GAMERON_DUAL_PSX_ADAPTOR) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_GAMERON, USB_DEVICE_ID_GAMERON_DUAL_PCS_ADAPTOR) },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_GEMBIRD, USB_DEVICE_ID_GEMBIRD_JPD_DUALFORCE2) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_GREENASIA, 0x0003) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_GREENASIA, 0x0012) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_GYRATION, USB_DEVICE_ID_GYRATION_REMOTE) },
diff --git a/drivers/hid/hid-gembird.c b/drivers/hid/hid-gembird.c
new file mode 100644
index 0000000..cefc394
--- /dev/null
+++ b/drivers/hid/hid-gembird.c
@@ -0,0 +1,77 @@
+/*
+ *  HID driver for Gembird Joypad, "PC Game Controller"
+ *
+ *  Copyright (c) 2015 Red Hat, Inc
+ *  Copyright (c) 2015 Benjamin Tissoires
+ */
+
+/*
+ * 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/device.h>
+#include <linux/hid.h>
+#include <linux/module.h>
+
+#include "hid-ids.h"
+
+#define GEMBIRD_START_FAULTY_RDESC	22
+
+/*
+ * This gamepad advertises 5 absolute axis while 4 are actually used.
+ * The first Z axis shows some garbage, so it has to be ignored by HID.
+ * The second Z axis and the Rz one are actually Rx and Ry. Remap them.
+ */
+
+static const __u8 gembird_jpd_faulty_rdesc[] = {
+	0x09, 0x30,			/*   Usage (X)			*/
+	0x09, 0x31,			/*   Usage (Y)			*/
+	0x09, 0x32,			/*   Usage (Z)			*/
+	0x09, 0x32,			/*   Usage (Z)			*/
+	0x09, 0x35,			/*   Usage (Rz)			*/
+};
+static const __u8 gembird_jpd_fixed_rdesc[] = {
+	0x09, 0x30,			/*   Usage (X)			*/
+	0x09, 0x31,			/*   Usage (Y)			*/
+	0x09, 0x30,			/*   Usage (X) (ignored)	*/
+	0x09, 0x33,			/*   Usage (Rx)			*/
+	0x09, 0x34,			/*   Usage (Ry)			*/
+};
+
+static __u8 *gembird_report_fixup(struct hid_device *hdev, __u8 *rdesc,
+		unsigned int *rsize)
+{
+	if (*rsize >= 32 && !memcmp(&rdesc[GEMBIRD_START_FAULTY_RDESC],
+				    gembird_jpd_faulty_rdesc,
+				    sizeof(gembird_jpd_faulty_rdesc))) {
+		dev_info(&hdev->dev,
+			 "fixing Gembird JPD-DualForce 2 report descriptor.\n");
+
+		memcpy(rdesc + GEMBIRD_START_FAULTY_RDESC,
+		       gembird_jpd_fixed_rdesc,
+		       sizeof(gembird_jpd_fixed_rdesc));
+	}
+
+	return rdesc;
+}
+
+static const struct hid_device_id gembird_devices[] = {
+	{ HID_USB_DEVICE(USB_VENDOR_ID_GEMBIRD,
+			 USB_DEVICE_ID_GEMBIRD_JPD_DUALFORCE2) },
+	{ }
+};
+MODULE_DEVICE_TABLE(hid, gembird_devices);
+
+static struct hid_driver gembird_driver = {
+	.name = "gembird",
+	.id_table = gembird_devices,
+	.report_fixup = gembird_report_fixup,
+};
+module_hid_driver(gembird_driver);
+
+MODULE_AUTHOR("Benjamin Tissoires <benjamin.tissoires@gmail.com>");
+MODULE_DESCRIPTION("HID Gembird joypad driver");
+MODULE_LICENSE("GPL");
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 602f2b7..720d5e3 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -363,6 +363,9 @@
 #define USB_DEVICE_ID_GAMERON_DUAL_PSX_ADAPTOR	0x0001
 #define USB_DEVICE_ID_GAMERON_DUAL_PCS_ADAPTOR	0x0002
 
+#define USB_VENDOR_ID_GEMBIRD			0x11ff
+#define USB_DEVICE_ID_GEMBIRD_JPD_DUALFORCE2	0x3331
+
 #define USB_VENDOR_ID_GENERAL_TOUCH	0x0dfc
 #define USB_DEVICE_ID_GENERAL_TOUCH_WIN7_TWOFINGERS 0x0003
 #define USB_DEVICE_ID_GENERAL_TOUCH_WIN8_PWT_TENFINGERS 0x0100
-- 
2.4.3

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

* Re: [PATCH] HID: gembird: add new driver to fix Gembird JPD-DualForce 2
  2015-08-12  1:32 ` Orivej Desh
@ 2015-08-12 14:40   ` Benjamin Tissoires
  2015-08-12 16:51     ` Orivej Desh
  0 siblings, 1 reply; 5+ messages in thread
From: Benjamin Tissoires @ 2015-08-12 14:40 UTC (permalink / raw)
  To: Orivej Desh; +Cc: Jiri Kosina, linux-kernel, linux-input

On Aug 12 2015 or thereabouts, Orivej Desh wrote:
> This gamepad advertises 5 absolute axis while 4 are actually used.
> The first Z axis shows some garbage, so it has to be ignored by HID.
> The second Z axis and the Rz one are actually Rx and Ry. Remap them.
> 
> We could also just remap and ignore the axis in .input_mapping(). I
> went ahead with .report_fixup() first, so here it is.
> 
> Reported-by: Orivej Desh <orivej@gmx.fr>
> Tested-by: Orivej Desh <orivej@gmx.fr>
> ---
> 
> * Benjamin Tissoires
> > so here are the fixes I mentioned yesterday. If you could give a quick shot
> > and a "tested-by" that would be even better.
> 
> I tested your patch and it works as advertised, thank you!  But your
> comment about the implementation («re-assigning X to the original Z
> axis») did not match the implementation, so I simplified the
> implementation according to the comment, and tested it too.  Since the
> patch has changed, I think you need to sign it off again.

Hmm... This was actually my first shot, and I noticed that the
flickering original Z axis was now taking over the correct X axis. So
that's why I added some complexity to ignore at the HID level this axis.
I just forgot to update the comment :)

If you think this patch is good enough (you will get flickering X), we
can carry on this one, but I think the one I submitted (with the comment
edited) would provide a better experience.

Cheers,
Benjamin

> 
> > Jiri, I am not sure if the .input_mapping() alternative would not be better.
> > Both solutions are valid, so if you do not like my memcpys, I can re-write the
> > patch the other way.
> 
>  drivers/hid/Kconfig       |  6 ++++
>  drivers/hid/Makefile      |  1 +
>  drivers/hid/hid-core.c    |  1 +
>  drivers/hid/hid-gembird.c | 77 +++++++++++++++++++++++++++++++++++++++++++++++
>  drivers/hid/hid-ids.h     |  3 ++
>  5 files changed, 88 insertions(+)
>  create mode 100644 drivers/hid/hid-gembird.c
> 
> diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
> index 57c94d7..6ab51ae 100644
> --- a/drivers/hid/Kconfig
> +++ b/drivers/hid/Kconfig
> @@ -251,6 +251,12 @@ config HID_EZKEY
>  	---help---
>  	Support for Ezkey BTC 8193 keyboard.
>  
> +config HID_GEMBIRD
> +	tristate "Gembird Joypad"
> +	depends on HID
> +	---help---
> +	Support for Gembird JPD-DualForce 2.
> +
>  config HID_HOLTEK
>  	tristate "Holtek HID devices"
>  	depends on USB_HID
> diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
> index 2f8a41d..e6441bc 100644
> --- a/drivers/hid/Makefile
> +++ b/drivers/hid/Makefile
> @@ -36,6 +36,7 @@ obj-$(CONFIG_HID_EMS_FF)	+= hid-emsff.o
>  obj-$(CONFIG_HID_ELECOM)	+= hid-elecom.o
>  obj-$(CONFIG_HID_ELO)		+= hid-elo.o
>  obj-$(CONFIG_HID_EZKEY)		+= hid-ezkey.o
> +obj-$(CONFIG_HID_GEMBIRD)	+= hid-gembird.o
>  obj-$(CONFIG_HID_GT683R)	+= hid-gt683r.o
>  obj-$(CONFIG_HID_GYRATION)	+= hid-gyration.o
>  obj-$(CONFIG_HID_HOLTEK)	+= hid-holtek-kbd.o
> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
> index e982e70..22afab9 100644
> --- a/drivers/hid/hid-core.c
> +++ b/drivers/hid/hid-core.c
> @@ -1844,6 +1844,7 @@ static const struct hid_device_id hid_have_special_driver[] = {
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_EZKEY, USB_DEVICE_ID_BTC_8193) },
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_GAMERON, USB_DEVICE_ID_GAMERON_DUAL_PSX_ADAPTOR) },
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_GAMERON, USB_DEVICE_ID_GAMERON_DUAL_PCS_ADAPTOR) },
> +	{ HID_USB_DEVICE(USB_VENDOR_ID_GEMBIRD, USB_DEVICE_ID_GEMBIRD_JPD_DUALFORCE2) },
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_GREENASIA, 0x0003) },
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_GREENASIA, 0x0012) },
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_GYRATION, USB_DEVICE_ID_GYRATION_REMOTE) },
> diff --git a/drivers/hid/hid-gembird.c b/drivers/hid/hid-gembird.c
> new file mode 100644
> index 0000000..cefc394
> --- /dev/null
> +++ b/drivers/hid/hid-gembird.c
> @@ -0,0 +1,77 @@
> +/*
> + *  HID driver for Gembird Joypad, "PC Game Controller"
> + *
> + *  Copyright (c) 2015 Red Hat, Inc
> + *  Copyright (c) 2015 Benjamin Tissoires
> + */
> +
> +/*
> + * 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/device.h>
> +#include <linux/hid.h>
> +#include <linux/module.h>
> +
> +#include "hid-ids.h"
> +
> +#define GEMBIRD_START_FAULTY_RDESC	22
> +
> +/*
> + * This gamepad advertises 5 absolute axis while 4 are actually used.
> + * The first Z axis shows some garbage, so it has to be ignored by HID.
> + * The second Z axis and the Rz one are actually Rx and Ry. Remap them.
> + */
> +
> +static const __u8 gembird_jpd_faulty_rdesc[] = {
> +	0x09, 0x30,			/*   Usage (X)			*/
> +	0x09, 0x31,			/*   Usage (Y)			*/
> +	0x09, 0x32,			/*   Usage (Z)			*/
> +	0x09, 0x32,			/*   Usage (Z)			*/
> +	0x09, 0x35,			/*   Usage (Rz)			*/
> +};
> +static const __u8 gembird_jpd_fixed_rdesc[] = {
> +	0x09, 0x30,			/*   Usage (X)			*/
> +	0x09, 0x31,			/*   Usage (Y)			*/
> +	0x09, 0x30,			/*   Usage (X) (ignored)	*/
> +	0x09, 0x33,			/*   Usage (Rx)			*/
> +	0x09, 0x34,			/*   Usage (Ry)			*/
> +};
> +
> +static __u8 *gembird_report_fixup(struct hid_device *hdev, __u8 *rdesc,
> +		unsigned int *rsize)
> +{
> +	if (*rsize >= 32 && !memcmp(&rdesc[GEMBIRD_START_FAULTY_RDESC],
> +				    gembird_jpd_faulty_rdesc,
> +				    sizeof(gembird_jpd_faulty_rdesc))) {
> +		dev_info(&hdev->dev,
> +			 "fixing Gembird JPD-DualForce 2 report descriptor.\n");
> +
> +		memcpy(rdesc + GEMBIRD_START_FAULTY_RDESC,
> +		       gembird_jpd_fixed_rdesc,
> +		       sizeof(gembird_jpd_fixed_rdesc));
> +	}
> +
> +	return rdesc;
> +}
> +
> +static const struct hid_device_id gembird_devices[] = {
> +	{ HID_USB_DEVICE(USB_VENDOR_ID_GEMBIRD,
> +			 USB_DEVICE_ID_GEMBIRD_JPD_DUALFORCE2) },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(hid, gembird_devices);
> +
> +static struct hid_driver gembird_driver = {
> +	.name = "gembird",
> +	.id_table = gembird_devices,
> +	.report_fixup = gembird_report_fixup,
> +};
> +module_hid_driver(gembird_driver);
> +
> +MODULE_AUTHOR("Benjamin Tissoires <benjamin.tissoires@gmail.com>");
> +MODULE_DESCRIPTION("HID Gembird joypad driver");
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
> index 602f2b7..720d5e3 100644
> --- a/drivers/hid/hid-ids.h
> +++ b/drivers/hid/hid-ids.h
> @@ -363,6 +363,9 @@
>  #define USB_DEVICE_ID_GAMERON_DUAL_PSX_ADAPTOR	0x0001
>  #define USB_DEVICE_ID_GAMERON_DUAL_PCS_ADAPTOR	0x0002
>  
> +#define USB_VENDOR_ID_GEMBIRD			0x11ff
> +#define USB_DEVICE_ID_GEMBIRD_JPD_DUALFORCE2	0x3331
> +
>  #define USB_VENDOR_ID_GENERAL_TOUCH	0x0dfc
>  #define USB_DEVICE_ID_GENERAL_TOUCH_WIN7_TWOFINGERS 0x0003
>  #define USB_DEVICE_ID_GENERAL_TOUCH_WIN8_PWT_TENFINGERS 0x0100
> -- 
> 2.4.3
> 
--
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

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

* Re: [PATCH] HID: gembird: add new driver to fix Gembird JPD-DualForce 2
  2015-08-12 14:40   ` Benjamin Tissoires
@ 2015-08-12 16:51     ` Orivej Desh
  2015-08-12 17:34       ` Benjamin Tissoires
  0 siblings, 1 reply; 5+ messages in thread
From: Orivej Desh @ 2015-08-12 16:51 UTC (permalink / raw)
  To: Benjamin Tissoires; +Cc: linux-input

-linux-kernel@vger.kernel.org
-jkosina@suse.com

* Benjamin Tissoires
> Hmm... This was actually my first shot, and I noticed that the
> flickering original Z axis was now taking over the correct X axis. So
> that's why I added some complexity to ignore at the HID level this axis.
> I just forgot to update the comment :)

How did you notice that?  Is there a tool in hid-replay?  I did not test
my edition of the patch extensively because I did not expect such a
subtle difference.  Yet I did not observe flickering when joypad was
idle.

I assumed that the commit that hid Rx axis in 3.18 by calling
"map_abs_clear(usage->hid & 0xf);" effectively removed any influence of
repeated axes in report description, but now it seems I am wrong.  Would
you explain what it really did?

> If you think this patch is good enough (you will get flickering X), we
> can carry on this one, but I think the one I submitted (with the comment
> edited) would provide a better experience.

In this case I certainly prefer your original patch.

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

* Re: [PATCH] HID: gembird: add new driver to fix Gembird JPD-DualForce 2
  2015-08-12 16:51     ` Orivej Desh
@ 2015-08-12 17:34       ` Benjamin Tissoires
  0 siblings, 0 replies; 5+ messages in thread
From: Benjamin Tissoires @ 2015-08-12 17:34 UTC (permalink / raw)
  To: Orivej Desh; +Cc: linux-input

On Aug 12 2015 or thereabouts, Orivej Desh wrote:
> -linux-kernel@vger.kernel.org
> -jkosina@suse.com
> 
> * Benjamin Tissoires
> > Hmm... This was actually my first shot, and I noticed that the
> > flickering original Z axis was now taking over the correct X axis. So
> > that's why I added some complexity to ignore at the HID level this axis.
> > I just forgot to update the comment :)
> 
> How did you notice that?  Is there a tool in hid-replay?  I did not test

I used evemu-record (package evemu) to see at the raw outputs from the
kernel.

> my edition of the patch extensively because I did not expect such a
> subtle difference.  Yet I did not observe flickering when joypad was
> idle.

There might be filtering on the userspace side for X/Y, that would
explain.

> 
> I assumed that the commit that hid Rx axis in 3.18 by calling
> "map_abs_clear(usage->hid & 0xf);" effectively removed any influence of
> repeated axes in report description, but now it seems I am wrong.  Would
> you explain what it really did?

It did remove the repeated axes. When you call map_abs(), it tries to
map the given HID axis to the corresponding evdev axis. But if this axis
is already mapped to an existing HID axis, it tries the next one. This
works OK for miscellaneous axes, but not for the ones that are actually
used and have a true semantic. By calling map_abs_clear(), the evdev
axis is left untouched, so the next time it is mapped, it does not try
the next one.

> 
> > If you think this patch is good enough (you will get flickering X), we
> > can carry on this one, but I think the one I submitted (with the comment
> > edited) would provide a better experience.
> 
> In this case I certainly prefer your original patch.

OK. I'll amend the comment in the patch and resubmit then.

Cheers,
Benjamin


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

end of thread, other threads:[~2015-08-12 17:34 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-08-11 15:35 [PATCH] HID: gembird: add new driver to fix Gembird JPD-DualForce 2 Benjamin Tissoires
2015-08-12  1:32 ` Orivej Desh
2015-08-12 14:40   ` Benjamin Tissoires
2015-08-12 16:51     ` Orivej Desh
2015-08-12 17:34       ` Benjamin Tissoires

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).