HID: Support for the Logitech G920 wheel

HID: Support for the Logitech G920 wheel

[Simon Wood]

This series of patches provide input support for the Logitech G920 gaming wheel.

This wheel is internally different from the other Logitech wheels; when first 
connected it is in X-Box mode and can instructed to switch to HID with a 'magic 
command' (1st patch). Once the wheel reconnects in HID mode it can communicate 
with the HID++ protocol, but using a 'very long' packet size (2nd patch).

Basic input operation is possible with adustment of the 'range' (the amount that 
the wheel turns) controlled via the '/sys' interface, same concept as the G25/G27/etc.

We also discovered that wheel uses some vendor specific pages, which confuse the 
HID system resulting in lots of additional axis reported. This is prevented by 
ignoring these pages (5th patch, thank you Elias).


Note: These patches are applied to Jiri's 'for-next' tree to work with the other 
HID++ changes already queued for 4.4.

[PATCH 1/6] INPUT: xpad: Add minimal support for Logitech G920 Wheel
[PATCH 2/6] HID: hid-logitech-hidpp: Add support for very long
[PATCH 3/6] HID: hid-logitech-hidpp: Add basic support for Logitech
[PATCH 4/6] HID: hid-logitech-hidpp: Add range sysfs for Logitech
[PATCH 5/6] HID: Add vendor specific usage pages for Logitech G920
[PATCH 6/6] HID: hid-logitech-hidpp: G920 remove deadzones


The future... as the internals of the wheel are considerably more 'capable' we 
are working on implementing Force Feedback using the forth-coming KLGD system.

[PATCH 1/6] INPUT: xpad: Add minimal support for Logitech G920 Wheel

[Simon Wood]

When plugged in the Logitech G920 wheel starts with USBID 046d:c261
and behaviors as a vendor specific class. If a 'magic' byte sequence
is sent the wheel will detach and reconnect as a HID device with the
USBID 046d:c262.

Signed-off-by: Simon Wood <simon@mungewell.org>
---
 drivers/input/joystick/xpad.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c
index f8850f9..af83f7e 100644
--- a/drivers/input/joystick/xpad.c
+++ b/drivers/input/joystick/xpad.c
@@ -93,6 +93,7 @@
 #define MAP_STICKS_TO_NULL		(1 << 2)
 #define DANCEPAD_MAP_CONFIG	(MAP_DPAD_TO_BUTTONS |			\
 				MAP_TRIGGERS_TO_BUTTONS | MAP_STICKS_TO_NULL)
+#define SWITCH_G920_TO_HID_MODE		(1 << 3)
 
 #define XTYPE_XBOX        0
 #define XTYPE_XBOX360     1
@@ -133,6 +134,7 @@ static const struct xpad_device {
 	{ 0x046d, 0xc21e, "Logitech Gamepad F510", 0, XTYPE_XBOX360 },
 	{ 0x046d, 0xc21f, "Logitech Gamepad F710", 0, XTYPE_XBOX360 },
 	{ 0x046d, 0xc242, "Logitech Chillstream Controller", 0, XTYPE_XBOX360 },
+	{ 0x046d, 0xc261, "Logitech G920 Driving Force Racing Wheel", SWITCH_G920_TO_HID_MODE, XTYPE_XBOXONE },
 	{ 0x046d, 0xca84, "Logitech Xbox Cordless Controller", 0, XTYPE_XBOX },
 	{ 0x046d, 0xca88, "Logitech Compact Controller for Xbox", 0, XTYPE_XBOX },
 	{ 0x05fd, 0x1007, "Mad Catz Controller (unverified)", 0, XTYPE_XBOX },
@@ -299,6 +301,7 @@ static struct usb_device_id xpad_table[] = {
 	XPAD_XBOX360_VENDOR(0x045e),		/* Microsoft X-Box 360 controllers */
 	XPAD_XBOXONE_VENDOR(0x045e),		/* Microsoft X-Box One controllers */
 	XPAD_XBOX360_VENDOR(0x046d),		/* Logitech X-Box 360 style controllers */
+	XPAD_XBOXONE_VENDOR(0x046d),		/* Logitech X-Box One style controllers */
 	XPAD_XBOX360_VENDOR(0x0738),		/* Mad Catz X-Box 360 controllers */
 	{ USB_DEVICE(0x0738, 0x4540) },		/* Mad Catz Beat Pad */
 	XPAD_XBOX360_VENDOR(0x0e6f),		/* 0x0e6f X-Box 360 controllers */
@@ -1021,6 +1024,19 @@ static int xpad_open(struct input_dev *dev)
 	if (usb_submit_urb(xpad->irq_in, GFP_KERNEL))
 		return -EIO;
 
+	/* Logitect G920 wheel starts in XBOX mode, but is reconfigured to be HID  */
+	/* device with USBID of 046D:C262. Wheel will detach when 'magic' is sent. */
+	if (xpad->mapping & SWITCH_G920_TO_HID_MODE) {
+		xpad->odata[0] = 0x0F;
+		xpad->odata[1] = 0x00;
+		xpad->odata[2] = 0x01;
+		xpad->odata[3] = 0x01;
+		xpad->odata[4] = 0x42;
+		xpad->irq_out->transfer_buffer_length = 5;
+
+		return usb_submit_urb(xpad->irq_out, GFP_KERNEL);
+	}
+
 	if (xpad->xtype == XTYPE_XBOXONE) {
 		/* Xbox one controller needs to be initialized. */
 		xpad->odata[0] = 0x05;
-- 
2.1.4

[PATCH 2/6] HID: hid-logitech-hidpp: Add support for very long packets

[Simon Wood]

Patch add support for the 'very long' HID++ packets, which are
64 bytes in length.

Signed-off-by: Simon Wood <simon@mungewell.org>
---
 drivers/hid/hid-logitech-hidpp.c | 59 ++++++++++++++++++++++++++++++++--------
 1 file changed, 48 insertions(+), 11 deletions(-)

diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index 452e5d5..08e65e8 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -40,9 +40,11 @@ MODULE_PARM_DESC(disable_tap_to_click,
 
 #define REPORT_ID_HIDPP_SHORT			0x10
 #define REPORT_ID_HIDPP_LONG			0x11
+#define REPORT_ID_HIDPP_VERY_LONG		0x12
 
 #define HIDPP_REPORT_SHORT_LENGTH		7
 #define HIDPP_REPORT_LONG_LENGTH		20
+#define HIDPP_REPORT_VERY_LONG_LENGTH		64
 
 #define HIDPP_QUIRK_CLASS_WTP			BIT(0)
 #define HIDPP_QUIRK_CLASS_M560			BIT(1)
@@ -81,13 +83,13 @@ MODULE_PARM_DESC(disable_tap_to_click,
 struct fap {
 	u8 feature_index;
 	u8 funcindex_clientid;
-	u8 params[HIDPP_REPORT_LONG_LENGTH - 4U];
+	u8 params[HIDPP_REPORT_VERY_LONG_LENGTH - 4U];
 };
 
 struct rap {
 	u8 sub_id;
 	u8 reg_address;
-	u8 params[HIDPP_REPORT_LONG_LENGTH - 4U];
+	u8 params[HIDPP_REPORT_VERY_LONG_LENGTH - 4U];
 };
 
 struct hidpp_report {
@@ -153,6 +155,9 @@ static int __hidpp_send_report(struct hid_device *hdev,
 	case REPORT_ID_HIDPP_LONG:
 		fields_count = HIDPP_REPORT_LONG_LENGTH;
 		break;
+	case REPORT_ID_HIDPP_VERY_LONG:
+		fields_count = HIDPP_REPORT_VERY_LONG_LENGTH;
+		break;
 	default:
 		return -ENODEV;
 	}
@@ -217,8 +222,9 @@ static int hidpp_send_message_sync(struct hidpp_device *hidpp,
 		goto exit;
 	}
 
-	if (response->report_id == REPORT_ID_HIDPP_LONG &&
-	    response->fap.feature_index == HIDPP20_ERROR) {
+	if ((response->report_id == REPORT_ID_HIDPP_LONG ||
+			response->report_id == REPORT_ID_HIDPP_VERY_LONG) &&
+			response->fap.feature_index == HIDPP20_ERROR) {
 		ret = response->fap.params[1];
 		dbg_hid("%s:got hidpp 2.0 error %02X\n", __func__, ret);
 		goto exit;
@@ -243,7 +249,11 @@ static int hidpp_send_fap_command_sync(struct hidpp_device *hidpp,
 	message = kzalloc(sizeof(struct hidpp_report), GFP_KERNEL);
 	if (!message)
 		return -ENOMEM;
-	message->report_id = REPORT_ID_HIDPP_LONG;
+
+	if (param_count > (HIDPP_REPORT_LONG_LENGTH - 4))
+		message->report_id = REPORT_ID_HIDPP_VERY_LONG;
+	else
+		message->report_id = REPORT_ID_HIDPP_LONG;
 	message->fap.feature_index = feat_index;
 	message->fap.funcindex_clientid = funcindex_clientid;
 	memcpy(&message->fap.params, params, param_count);
@@ -258,13 +268,23 @@ static int hidpp_send_rap_command_sync(struct hidpp_device *hidpp_dev,
 	struct hidpp_report *response)
 {
 	struct hidpp_report *message;
-	int ret;
+	int ret, max_count;
 
-	if ((report_id != REPORT_ID_HIDPP_SHORT) &&
-	    (report_id != REPORT_ID_HIDPP_LONG))
+	switch (report_id) {
+	case REPORT_ID_HIDPP_SHORT:
+		max_count = HIDPP_REPORT_SHORT_LENGTH - 4;
+		break;
+	case REPORT_ID_HIDPP_LONG:
+		max_count = HIDPP_REPORT_LONG_LENGTH - 4;
+		break;
+	case REPORT_ID_HIDPP_VERY_LONG:
+		max_count = HIDPP_REPORT_VERY_LONG_LENGTH - 4;
+		break;
+	default:
 		return -EINVAL;
+	}
 
-	if (param_count > sizeof(message->rap.params))
+	if (param_count > max_count)
 		return -EINVAL;
 
 	message = kzalloc(sizeof(struct hidpp_report), GFP_KERNEL);
@@ -508,10 +528,19 @@ static int hidpp_devicenametype_get_device_name(struct hidpp_device *hidpp,
 	if (ret)
 		return ret;
 
-	if (response.report_id == REPORT_ID_HIDPP_LONG)
+	switch (response.report_id) {
+	case REPORT_ID_HIDPP_VERY_LONG:
+		count = HIDPP_REPORT_VERY_LONG_LENGTH - 4;
+		break;
+	case REPORT_ID_HIDPP_LONG:
 		count = HIDPP_REPORT_LONG_LENGTH - 4;
-	else
+		break;
+	case REPORT_ID_HIDPP_SHORT:
 		count = HIDPP_REPORT_SHORT_LENGTH - 4;
+		break;
+	default:
+		return -EPROTO;
+	}
 
 	if (len_buf < count)
 		count = len_buf;
@@ -1345,6 +1374,14 @@ static int hidpp_raw_event(struct hid_device *hdev, struct hid_report *report,
 
 	/* Generic HID++ processing. */
 	switch (data[0]) {
+	case REPORT_ID_HIDPP_VERY_LONG:
+		if (size != HIDPP_REPORT_VERY_LONG_LENGTH) {
+			hid_err(hdev, "received hid++ report of bad size (%d)",
+				size);
+			return 1;
+		}
+		ret = hidpp_raw_hidpp_event(hidpp, data, size);
+		break;
 	case REPORT_ID_HIDPP_LONG:
 		if (size != HIDPP_REPORT_LONG_LENGTH) {
 			hid_err(hdev, "received hid++ report of bad size (%d)",
-- 
2.1.4

[PATCH 3/6] HID: hid-logitech-hidpp: Add basic support for Logitech G920

[Simon Wood]

This patch adds basic support for the Logitech G920 wheel when in HID
mode. This wheel 'speaks' the HID++ protocol, and therefor is driven
with hid-logitech-hidpp.

At this stage the driver only shows that it can communicate with the
wheel by outputting the name discovered over HID++.

The normal HID functions work to give input functionality using
joystick/event interface.

Signed-off-by: Simon Wood <simon@mungewell.org>
---
 drivers/hid/hid-core.c           |  1 +
 drivers/hid/hid-ids.h            |  1 +
 drivers/hid/hid-logitech-hidpp.c | 69 +++++++++++++++++++++++++++++++---------
 3 files changed, 56 insertions(+), 15 deletions(-)

diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index bcd914a..60d564d 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -1896,6 +1896,7 @@ static const struct hid_device_id hid_have_special_driver[] = {
 	{ HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_RUMBLEPAD_CORD) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_RUMBLEPAD) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_RUMBLEPAD2_2) },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_G920_WHEEL) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_WINGMAN_F3D) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_WINGMAN_FFG ) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_FORCE3D_PRO) },
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index f769208..d3500c4 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -614,6 +614,7 @@
 #define USB_DEVICE_ID_LOGITECH_RUMBLEPAD2	0xc218
 #define USB_DEVICE_ID_LOGITECH_RUMBLEPAD2_2	0xc219
 #define USB_DEVICE_ID_LOGITECH_G29_WHEEL	0xc24f
+#define USB_DEVICE_ID_LOGITECH_G920_WHEEL	0xc262
 #define USB_DEVICE_ID_LOGITECH_WINGMAN_F3D	0xc283
 #define USB_DEVICE_ID_LOGITECH_FORCE3D_PRO	0xc286
 #define USB_DEVICE_ID_LOGITECH_FLIGHT_SYSTEM_G940	0xc287
diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index 08e65e8..dbb9ff3 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -49,11 +49,13 @@ MODULE_PARM_DESC(disable_tap_to_click,
 #define HIDPP_QUIRK_CLASS_WTP			BIT(0)
 #define HIDPP_QUIRK_CLASS_M560			BIT(1)
 #define HIDPP_QUIRK_CLASS_K400			BIT(2)
+#define HIDPP_QUIRK_CLASS_G920			BIT(3)
 
 /* bits 2..20 are reserved for classes */
 #define HIDPP_QUIRK_CONNECT_EVENTS		BIT(21)
 #define HIDPP_QUIRK_WTP_PHYSICAL_BUTTONS	BIT(22)
 #define HIDPP_QUIRK_NO_HIDINPUT			BIT(23)
+#define HIDPP_QUIRK_FORCE_OUTPUT_REPORTS	BIT(24)
 
 #define HIDPP_QUIRK_DELAYED_INIT		(HIDPP_QUIRK_NO_HIDINPUT | \
 						 HIDPP_QUIRK_CONNECT_EVENTS)
@@ -146,8 +148,11 @@ static void hidpp_connect_event(struct hidpp_device *hidpp_dev);
 static int __hidpp_send_report(struct hid_device *hdev,
 				struct hidpp_report *hidpp_report)
 {
+	struct hidpp_device *hidpp = hid_get_drvdata(hdev);
 	int fields_count, ret;
 
+	hidpp = hid_get_drvdata(hdev);
+
 	switch (hidpp_report->report_id) {
 	case REPORT_ID_HIDPP_SHORT:
 		fields_count = HIDPP_REPORT_SHORT_LENGTH;
@@ -168,9 +173,13 @@ static int __hidpp_send_report(struct hid_device *hdev,
 	 */
 	hidpp_report->device_index = 0xff;
 
-	ret = hid_hw_raw_request(hdev, hidpp_report->report_id,
-		(u8 *)hidpp_report, fields_count, HID_OUTPUT_REPORT,
-		HID_REQ_SET_REPORT);
+	if (hidpp->quirks & HIDPP_QUIRK_FORCE_OUTPUT_REPORTS) {
+		ret = hid_hw_output_report(hdev, (u8 *)hidpp_report, fields_count);
+	} else {
+		ret = hid_hw_raw_request(hdev, hidpp_report->report_id,
+			(u8 *)hidpp_report, fields_count, HID_OUTPUT_REPORT,
+			HID_REQ_SET_REPORT);
+	}
 
 	return ret == fields_count ? 0 : -1;
 }
@@ -1430,8 +1439,10 @@ static void hidpp_overwrite_name(struct hid_device *hdev, bool use_unifying)
 
 	if (!name)
 		hid_err(hdev, "unable to retrieve the name of the device");
-	else
+	else {
+		dbg_hid("HID++: Got name: %s\n", name);
 		snprintf(hdev->name, sizeof(hdev->name), "%s", name);
+	}
 
 	kfree(name);
 }
@@ -1594,6 +1605,25 @@ static int hidpp_probe(struct hid_device *hdev, const struct hid_device_id *id)
 		goto hid_parse_fail;
 	}
 
+	if (hidpp->quirks & HIDPP_QUIRK_DELAYED_INIT)
+		connect_mask &= ~HID_CONNECT_HIDINPUT;
+
+	if (hidpp->quirks & HIDPP_QUIRK_CLASS_G920) {
+		ret = hid_hw_start(hdev, connect_mask);
+		if (ret) {
+			hid_err(hdev, "hw start failed\n");
+			goto hid_hw_start_fail;
+		}
+		ret = hid_hw_open(hdev);
+		if (ret < 0) {
+			dev_err(&hdev->dev, "%s:hid_hw_open returned error:%d\n",
+				__func__, ret);
+			hid_hw_stop(hdev);
+			goto hid_hw_start_fail;
+		}
+	}
+
+
 	/* Allow incoming packets */
 	hid_device_io_start(hdev);
 
@@ -1602,8 +1632,7 @@ static int hidpp_probe(struct hid_device *hdev, const struct hid_device_id *id)
 		if (!connected) {
 			ret = -ENODEV;
 			hid_err(hdev, "Device not connected");
-			hid_device_io_stop(hdev);
-			goto hid_parse_fail;
+			goto hid_hw_open_failed;
 		}
 
 		hid_info(hdev, "HID++ %u.%u device connected.\n",
@@ -1616,19 +1645,18 @@ static int hidpp_probe(struct hid_device *hdev, const struct hid_device_id *id)
 	if (connected && (hidpp->quirks & HIDPP_QUIRK_CLASS_WTP)) {
 		ret = wtp_get_config(hidpp);
 		if (ret)
-			goto hid_parse_fail;
+			goto hid_hw_open_failed;
 	}
 
 	/* Block incoming packets */
 	hid_device_io_stop(hdev);
 
-	if (hidpp->quirks & HIDPP_QUIRK_NO_HIDINPUT)
-		connect_mask &= ~HID_CONNECT_HIDINPUT;
-
-	ret = hid_hw_start(hdev, connect_mask);
-	if (ret) {
-		hid_err(hdev, "%s:hid_hw_start returned error\n", __func__);
-		goto hid_hw_start_fail;
+	if (!(hidpp->quirks & HIDPP_QUIRK_CLASS_G920)) {
+		ret = hid_hw_start(hdev, connect_mask);
+		if (ret) {
+			hid_err(hdev, "%s:hid_hw_start returned error\n", __func__);
+			goto hid_hw_start_fail;
+		}
 	}
 
 	if (hidpp->quirks & HIDPP_QUIRK_CONNECT_EVENTS) {
@@ -1640,6 +1668,12 @@ static int hidpp_probe(struct hid_device *hdev, const struct hid_device_id *id)
 
 	return ret;
 
+hid_hw_open_failed:
+	hid_device_io_stop(hdev);
+	if (hidpp->quirks & HIDPP_QUIRK_CLASS_G920) {
+		hid_hw_close(hdev);
+		hid_hw_stop(hdev);
+	}
 hid_hw_start_fail:
 hid_parse_fail:
 	cancel_work_sync(&hidpp->work);
@@ -1653,9 +1687,11 @@ static void hidpp_remove(struct hid_device *hdev)
 {
 	struct hidpp_device *hidpp = hid_get_drvdata(hdev);
 
+	if (hidpp->quirks & HIDPP_QUIRK_CLASS_G920)
+		hid_hw_close(hdev);
+	hid_hw_stop(hdev);
 	cancel_work_sync(&hidpp->work);
 	mutex_destroy(&hidpp->send_mutex);
-	hid_hw_stop(hdev);
 }
 
 static const struct hid_device_id hidpp_devices[] = {
@@ -1683,6 +1719,9 @@ static const struct hid_device_id hidpp_devices[] = {
 
 	{ HID_DEVICE(BUS_USB, HID_GROUP_LOGITECH_DJ_DEVICE,
 		USB_VENDOR_ID_LOGITECH, HID_ANY_ID)},
+
+	{ HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_G920_WHEEL),
+		.driver_data = HIDPP_QUIRK_CLASS_G920 | HIDPP_QUIRK_FORCE_OUTPUT_REPORTS},
 	{}
 };
 
-- 
2.1.4

[PATCH 4/6] HID: hid-logitech-hidpp: Add range sysfs for Logitech G920

[Simon Wood]

The G920 can adjust the amount of 'turn' it permits, this patch adds
a sysfs file 'range' to control this.

Signed-off-by: Simon Wood <simon@mungewell.org>
---
 drivers/hid/hid-logitech-hidpp.c | 140 ++++++++++++++++++++++++++++++++++++++-
 1 file changed, 139 insertions(+), 1 deletion(-)

diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index dbb9ff3..03e01be 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -1295,6 +1295,133 @@ static int k400_connect(struct hid_device *hdev, bool connected)
 	return k400_disable_tap_to_click(hidpp);
 }
 
+/* ------------------------------------------------------------------------- */
+/* Logitech G920 Driving Force Racing Wheel for Xbox One                     */
+/* ------------------------------------------------------------------------- */
+
+#define HIDPP_PAGE_G920_FORCE_FEEDBACK			0x8123
+
+/* Using session ID = 1 */
+#define CMD_G920_FORCE_GET_APERTURE			0x51
+#define CMD_G920_FORCE_SET_APERTURE			0x61
+
+struct g920_private_data {
+	u8 force_feature;
+	u16 range;
+};
+
+#define to_hid_device(pdev) container_of(pdev, struct hid_device, dev)
+
+static ssize_t g920_range_show(struct device *dev, struct device_attribute *attr,
+				char *buf)
+{
+	struct hid_device *hid = to_hid_device(dev);
+	struct hidpp_device *hidpp = hid_get_drvdata(hid);
+	struct g920_private_data *pdata;
+
+	pdata = hidpp->private_data;
+	if (!pdata) {
+		hid_err(hid, "Private driver data not found!\n");
+		return -EINVAL;
+	}
+
+	return scnprintf(buf, PAGE_SIZE, "%u\n", pdata->range);
+}
+
+static ssize_t g920_range_store(struct device *dev, struct device_attribute *attr,
+				 const char *buf, size_t count)
+{
+	struct hid_device *hid = to_hid_device(dev);
+	struct hidpp_device *hidpp = hid_get_drvdata(hid);
+	struct g920_private_data *pdata;
+	struct hidpp_report response;
+	u8 params[2];
+	int ret;
+	u16 range = simple_strtoul(buf, NULL, 10);
+
+	pdata = hidpp->private_data;
+	if (!pdata) {
+		hid_err(hid, "Private driver data not found!\n");
+		return -EINVAL;
+	}
+
+	if (range < 180)
+		range = 180;
+	else if (range > 900)
+		range = 900;
+
+	params[0] = range >> 8;
+	params[1] = range & 0x00FF;
+
+	ret = hidpp_send_fap_command_sync(hidpp, pdata->force_feature,
+		CMD_G920_FORCE_SET_APERTURE, params, 2, &response);
+	if (ret)
+		return ret;
+
+	pdata->range = range;
+	return count;
+}
+
+static DEVICE_ATTR(range, S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH, g920_range_show, g920_range_store);
+
+static int g920_allocate(struct hid_device *hdev)
+{
+	struct hidpp_device *hidpp = hid_get_drvdata(hdev);
+	struct g920_private_data *pdata;
+
+	pdata = devm_kzalloc(&hdev->dev, sizeof(struct g920_private_data),
+			GFP_KERNEL);
+	if (!pdata)
+		return -ENOMEM;
+
+	hidpp->private_data = pdata;
+
+	return 0;
+}
+
+static int g920_get_config(struct hidpp_device *hidpp)
+{
+	struct g920_private_data *pdata = hidpp->private_data;
+	struct hidpp_report response;
+	u8 feature_type;
+	u8 feature_index;
+	int ret;
+
+	pdata = hidpp->private_data;
+	if (!pdata) {
+		hid_err(hidpp->hid_dev, "Private driver data not found!\n");
+		return -EINVAL;
+	}
+
+	/* Find feature and store for later use */
+	ret = hidpp_root_get_feature(hidpp, HIDPP_PAGE_G920_FORCE_FEEDBACK,
+		&feature_index, &feature_type);
+	if (ret)
+		return ret;
+
+	pdata->force_feature = feature_index;
+
+	/* Read current Range */
+	ret = hidpp_send_fap_command_sync(hidpp, feature_index,
+		CMD_G920_FORCE_GET_APERTURE, NULL, 0, &response);
+	if (ret > 0) {
+		hid_err(hidpp->hid_dev, "%s: received protocol error 0x%02x\n",
+			__func__, ret);
+		return -EPROTO;
+	}
+	if (ret)
+		return ret;
+
+	pdata->range = get_unaligned_be16(&response.fap.params[0]);
+
+	/* Create sysfs interface */
+	ret = device_create_file(&(hidpp->hid_dev->dev), &dev_attr_range);
+	if (ret)
+		hid_warn(hidpp->hid_dev, "Unable to create sysfs interface for \"range\", errno %d\n", ret);
+
+	return 0;
+}
+
 /* -------------------------------------------------------------------------- */
 /* Generic HID++ devices                                                      */
 /* -------------------------------------------------------------------------- */
@@ -1593,6 +1720,10 @@ static int hidpp_probe(struct hid_device *hdev, const struct hid_device_id *id)
 		ret = k400_allocate(hdev);
 		if (ret)
 			goto allocate_fail;
+	} else if (hidpp->quirks & HIDPP_QUIRK_CLASS_G920) {
+		ret = g920_allocate(hdev);
+		if (ret)
+			goto allocate_fail;
 	}
 
 	INIT_WORK(&hidpp->work, delayed_work_cb);
@@ -1646,6 +1777,10 @@ static int hidpp_probe(struct hid_device *hdev, const struct hid_device_id *id)
 		ret = wtp_get_config(hidpp);
 		if (ret)
 			goto hid_hw_open_failed;
+	} else if (connected && (hidpp->quirks & HIDPP_QUIRK_CLASS_G920)) {
+		ret = g920_get_config(hidpp);
+		if (ret)
+			goto hid_hw_open_failed;
 	}
 
 	/* Block incoming packets */
@@ -1671,6 +1806,7 @@ static int hidpp_probe(struct hid_device *hdev, const struct hid_device_id *id)
 hid_hw_open_failed:
 	hid_device_io_stop(hdev);
 	if (hidpp->quirks & HIDPP_QUIRK_CLASS_G920) {
+		device_remove_file(&hdev->dev, &dev_attr_range);
 		hid_hw_close(hdev);
 		hid_hw_stop(hdev);
 	}
@@ -1687,8 +1823,10 @@ static void hidpp_remove(struct hid_device *hdev)
 {
 	struct hidpp_device *hidpp = hid_get_drvdata(hdev);
 
-	if (hidpp->quirks & HIDPP_QUIRK_CLASS_G920)
+	if (hidpp->quirks & HIDPP_QUIRK_CLASS_G920) {
+		device_remove_file(&hdev->dev, &dev_attr_range);
 		hid_hw_close(hdev);
+	}
 	hid_hw_stop(hdev);
 	cancel_work_sync(&hidpp->work);
 	mutex_destroy(&hidpp->send_mutex);
-- 
2.1.4

[PATCH 5/6] HID: Add vendor specific usage pages for Logitech G920

[Simon Wood]

The Logitech G920 uses a couple of vendor specific usage pages,
which results in incorrect number of axis/buttons being detected.

This patch adds these pages to the 'ignore' list.

Reported-by: Elias Vanderstuyft <elias.vds@gmail.com>
Signed-off-by: Simon Wood <simon@mungewell.org>
---
 drivers/hid/hid-input.c | 2 +-
 include/linux/hid.h     | 2 ++
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
index 53aeaf6..c120be5 100644
--- a/drivers/hid/hid-input.c
+++ b/drivers/hid/hid-input.c
@@ -959,7 +959,7 @@ static void hidinput_configure_usage(struct hid_input *hidinput, struct hid_fiel
 		set_bit(EV_REP, input->evbit);
 		goto ignore;
 
-	case HID_UP_LOGIVENDOR:
+	case HID_UP_LOGIVENDOR: case HID_UP_LOGIVENDOR2: case HID_UP_LOGIVENDOR3:
 		goto ignore;
 
 	case HID_UP_PID:
diff --git a/include/linux/hid.h b/include/linux/hid.h
index f17980d..ce1d883 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -168,6 +168,8 @@ struct hid_item {
 #define HID_UP_MSVENDOR		0xff000000
 #define HID_UP_CUSTOM		0x00ff0000
 #define HID_UP_LOGIVENDOR	0xffbc0000
+#define HID_UP_LOGIVENDOR2   0xff090000
+#define HID_UP_LOGIVENDOR3   0xff430000
 #define HID_UP_LNVENDOR		0xffa00000
 #define HID_UP_SENSOR		0x00200000
 
-- 
2.1.4

[PATCH 6/6] HID: hid-logitech-hidpp: G920 remove deadzones

[Simon Wood]

Ensure that the G920 is not given the default deadzones.

Signed-off-by: Simon Wood <simon@mungewell.org>
---
 drivers/hid/hid-logitech-hidpp.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index 03e01be..853b9c2 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -1441,6 +1441,27 @@ static int hidpp_input_mapping(struct hid_device *hdev, struct hid_input *hi,
 	return 0;
 }
 
+static int hidpp_input_mapped(struct hid_device *hdev, struct hid_input *hi,
+		struct hid_field *field, struct hid_usage *usage,
+		unsigned long **bit, int *max)
+{
+	/* Ensure that Logitech G920 is not given a default fuzz/flat value */
+	if (usage->type == EV_ABS && (usage->code == ABS_X ||
+			usage->code == ABS_Y || usage->code == ABS_Z ||
+			usage->code == ABS_RZ)) {
+		switch (hdev->product) {
+		case USB_DEVICE_ID_LOGITECH_G920_WHEEL:
+			field->application = HID_GD_MULTIAXIS;
+			break;
+		default:
+			break;
+		}
+	}
+
+	return 0;
+}
+
+
 static void hidpp_populate_input(struct hidpp_device *hidpp,
 		struct input_dev *input, bool origin_is_hid_core)
 {
@@ -1873,6 +1894,7 @@ static struct hid_driver hidpp_driver = {
 	.raw_event = hidpp_raw_event,
 	.input_configured = hidpp_input_configured,
 	.input_mapping = hidpp_input_mapping,
+	.input_mapped = hidpp_input_mapped,
 };
 
 module_hid_driver(hidpp_driver);
-- 
2.1.4

Re: [PATCH 3/6] HID: hid-logitech-hidpp: Add basic support for Logitech G920

[Benjamin Tissoires]

Hi Simon,

On Sat, Nov 7, 2015 at 5:10 PM, Simon Wood <simon@mungewell.org> wrote:
> This patch adds basic support for the Logitech G920 wheel when in HID
> mode. This wheel 'speaks' the HID++ protocol, and therefor is driven
> with hid-logitech-hidpp.
>
> At this stage the driver only shows that it can communicate with the
> wheel by outputting the name discovered over HID++.
>
> The normal HID functions work to give input functionality using
> joystick/event interface.
>
> Signed-off-by: Simon Wood <simon@mungewell.org>

Just 2 nitpicks:

> ---
>  drivers/hid/hid-core.c           |  1 +
>  drivers/hid/hid-ids.h            |  1 +
>  drivers/hid/hid-logitech-hidpp.c | 69 +++++++++++++++++++++++++++++++---------
>  3 files changed, 56 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
> index bcd914a..60d564d 100644
> --- a/drivers/hid/hid-core.c
> +++ b/drivers/hid/hid-core.c
> @@ -1896,6 +1896,7 @@ static const struct hid_device_id hid_have_special_driver[] = {
>         { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_RUMBLEPAD_CORD) },
>         { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_RUMBLEPAD) },
>         { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_RUMBLEPAD2_2) },
> +       { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_G920_WHEEL) },
>         { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_WINGMAN_F3D) },
>         { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_WINGMAN_FFG ) },
>         { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_FORCE3D_PRO) },
> diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
> index f769208..d3500c4 100644
> --- a/drivers/hid/hid-ids.h
> +++ b/drivers/hid/hid-ids.h
> @@ -614,6 +614,7 @@
>  #define USB_DEVICE_ID_LOGITECH_RUMBLEPAD2      0xc218
>  #define USB_DEVICE_ID_LOGITECH_RUMBLEPAD2_2    0xc219
>  #define USB_DEVICE_ID_LOGITECH_G29_WHEEL       0xc24f
> +#define USB_DEVICE_ID_LOGITECH_G920_WHEEL      0xc262
>  #define USB_DEVICE_ID_LOGITECH_WINGMAN_F3D     0xc283
>  #define USB_DEVICE_ID_LOGITECH_FORCE3D_PRO     0xc286
>  #define USB_DEVICE_ID_LOGITECH_FLIGHT_SYSTEM_G940      0xc287
> diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
> index 08e65e8..dbb9ff3 100644
> --- a/drivers/hid/hid-logitech-hidpp.c
> +++ b/drivers/hid/hid-logitech-hidpp.c
> @@ -49,11 +49,13 @@ MODULE_PARM_DESC(disable_tap_to_click,
>  #define HIDPP_QUIRK_CLASS_WTP                  BIT(0)
>  #define HIDPP_QUIRK_CLASS_M560                 BIT(1)
>  #define HIDPP_QUIRK_CLASS_K400                 BIT(2)
> +#define HIDPP_QUIRK_CLASS_G920                 BIT(3)
>
>  /* bits 2..20 are reserved for classes */
>  #define HIDPP_QUIRK_CONNECT_EVENTS             BIT(21)
>  #define HIDPP_QUIRK_WTP_PHYSICAL_BUTTONS       BIT(22)
>  #define HIDPP_QUIRK_NO_HIDINPUT                        BIT(23)
> +#define HIDPP_QUIRK_FORCE_OUTPUT_REPORTS       BIT(24)
>
>  #define HIDPP_QUIRK_DELAYED_INIT               (HIDPP_QUIRK_NO_HIDINPUT | \
>                                                  HIDPP_QUIRK_CONNECT_EVENTS)
> @@ -146,8 +148,11 @@ static void hidpp_connect_event(struct hidpp_device *hidpp_dev);
>  static int __hidpp_send_report(struct hid_device *hdev,
>                                 struct hidpp_report *hidpp_report)
>  {
> +       struct hidpp_device *hidpp = hid_get_drvdata(hdev);
>         int fields_count, ret;
>
> +       hidpp = hid_get_drvdata(hdev);
> +
>         switch (hidpp_report->report_id) {
>         case REPORT_ID_HIDPP_SHORT:
>                 fields_count = HIDPP_REPORT_SHORT_LENGTH;
> @@ -168,9 +173,13 @@ static int __hidpp_send_report(struct hid_device *hdev,
>          */
>         hidpp_report->device_index = 0xff;
>
> -       ret = hid_hw_raw_request(hdev, hidpp_report->report_id,
> -               (u8 *)hidpp_report, fields_count, HID_OUTPUT_REPORT,
> -               HID_REQ_SET_REPORT);
> +       if (hidpp->quirks & HIDPP_QUIRK_FORCE_OUTPUT_REPORTS) {
> +               ret = hid_hw_output_report(hdev, (u8 *)hidpp_report, fields_count);
> +       } else {
> +               ret = hid_hw_raw_request(hdev, hidpp_report->report_id,
> +                       (u8 *)hidpp_report, fields_count, HID_OUTPUT_REPORT,
> +                       HID_REQ_SET_REPORT);
> +       }
>
>         return ret == fields_count ? 0 : -1;
>  }
> @@ -1430,8 +1439,10 @@ static void hidpp_overwrite_name(struct hid_device *hdev, bool use_unifying)
>
>         if (!name)
>                 hid_err(hdev, "unable to retrieve the name of the device");
> -       else

Put braces under the "if" statement too (kernel coding style IIRC).

> +       else {
> +               dbg_hid("HID++: Got name: %s\n", name);
>                 snprintf(hdev->name, sizeof(hdev->name), "%s", name);
> +       }
>
>         kfree(name);
>  }
> @@ -1594,6 +1605,25 @@ static int hidpp_probe(struct hid_device *hdev, const struct hid_device_id *id)
>                 goto hid_parse_fail;
>         }
>
> +       if (hidpp->quirks & HIDPP_QUIRK_DELAYED_INIT)
> +               connect_mask &= ~HID_CONNECT_HIDINPUT;
> +
> +       if (hidpp->quirks & HIDPP_QUIRK_CLASS_G920) {
> +               ret = hid_hw_start(hdev, connect_mask);
> +               if (ret) {
> +                       hid_err(hdev, "hw start failed\n");
> +                       goto hid_hw_start_fail;
> +               }
> +               ret = hid_hw_open(hdev);
> +               if (ret < 0) {
> +                       dev_err(&hdev->dev, "%s:hid_hw_open returned error:%d\n",
> +                               __func__, ret);
> +                       hid_hw_stop(hdev);
> +                       goto hid_hw_start_fail;
> +               }
> +       }
> +

This is fine, but please mention this in the commit message. We might
even need this for all USB devices not using the unifying protocol.
Better noting that in the commit message so that in the future we do
not enumerate each and every classes of USB devices not using
unifying.
For now, I think it is OK to keep it that way until we see more devices.


Cheers,
Benjamin

> +
>         /* Allow incoming packets */
>         hid_device_io_start(hdev);
>
> @@ -1602,8 +1632,7 @@ static int hidpp_probe(struct hid_device *hdev, const struct hid_device_id *id)
>                 if (!connected) {
>                         ret = -ENODEV;
>                         hid_err(hdev, "Device not connected");
> -                       hid_device_io_stop(hdev);
> -                       goto hid_parse_fail;
> +                       goto hid_hw_open_failed;
>                 }
>
>                 hid_info(hdev, "HID++ %u.%u device connected.\n",
> @@ -1616,19 +1645,18 @@ static int hidpp_probe(struct hid_device *hdev, const struct hid_device_id *id)
>         if (connected && (hidpp->quirks & HIDPP_QUIRK_CLASS_WTP)) {
>                 ret = wtp_get_config(hidpp);
>                 if (ret)
> -                       goto hid_parse_fail;
> +                       goto hid_hw_open_failed;
>         }
>
>         /* Block incoming packets */
>         hid_device_io_stop(hdev);
>
> -       if (hidpp->quirks & HIDPP_QUIRK_NO_HIDINPUT)
> -               connect_mask &= ~HID_CONNECT_HIDINPUT;
> -
> -       ret = hid_hw_start(hdev, connect_mask);
> -       if (ret) {
> -               hid_err(hdev, "%s:hid_hw_start returned error\n", __func__);
> -               goto hid_hw_start_fail;
> +       if (!(hidpp->quirks & HIDPP_QUIRK_CLASS_G920)) {
> +               ret = hid_hw_start(hdev, connect_mask);
> +               if (ret) {
> +                       hid_err(hdev, "%s:hid_hw_start returned error\n", __func__);
> +                       goto hid_hw_start_fail;
> +               }
>         }
>
>         if (hidpp->quirks & HIDPP_QUIRK_CONNECT_EVENTS) {
> @@ -1640,6 +1668,12 @@ static int hidpp_probe(struct hid_device *hdev, const struct hid_device_id *id)
>
>         return ret;
>
> +hid_hw_open_failed:
> +       hid_device_io_stop(hdev);
> +       if (hidpp->quirks & HIDPP_QUIRK_CLASS_G920) {
> +               hid_hw_close(hdev);
> +               hid_hw_stop(hdev);
> +       }
>  hid_hw_start_fail:
>  hid_parse_fail:
>         cancel_work_sync(&hidpp->work);
> @@ -1653,9 +1687,11 @@ static void hidpp_remove(struct hid_device *hdev)
>  {
>         struct hidpp_device *hidpp = hid_get_drvdata(hdev);
>
> +       if (hidpp->quirks & HIDPP_QUIRK_CLASS_G920)
> +               hid_hw_close(hdev);
> +       hid_hw_stop(hdev);
>         cancel_work_sync(&hidpp->work);
>         mutex_destroy(&hidpp->send_mutex);
> -       hid_hw_stop(hdev);
>  }
>
>  static const struct hid_device_id hidpp_devices[] = {
> @@ -1683,6 +1719,9 @@ static const struct hid_device_id hidpp_devices[] = {
>
>         { HID_DEVICE(BUS_USB, HID_GROUP_LOGITECH_DJ_DEVICE,
>                 USB_VENDOR_ID_LOGITECH, HID_ANY_ID)},
> +
> +       { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_G920_WHEEL),
> +               .driver_data = HIDPP_QUIRK_CLASS_G920 | HIDPP_QUIRK_FORCE_OUTPUT_REPORTS},
>         {}
>  };
>
> --
> 2.1.4
>
> --
> 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

Re: [PATCH 5/6] HID: Add vendor specific usage pages for Logitech G920

[Benjamin Tissoires]

On Sat, Nov 7, 2015 at 5:10 PM, Simon Wood <simon@mungewell.org> wrote:
> The Logitech G920 uses a couple of vendor specific usage pages,
> which results in incorrect number of axis/buttons being detected.
>
> This patch adds these pages to the 'ignore' list.
>
> Reported-by: Elias Vanderstuyft <elias.vds@gmail.com>
> Signed-off-by: Simon Wood <simon@mungewell.org>
> ---
>  drivers/hid/hid-input.c | 2 +-
>  include/linux/hid.h     | 2 ++
>  2 files changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
> index 53aeaf6..c120be5 100644
> --- a/drivers/hid/hid-input.c
> +++ b/drivers/hid/hid-input.c
> @@ -959,7 +959,7 @@ static void hidinput_configure_usage(struct hid_input *hidinput, struct hid_fiel
>                 set_bit(EV_REP, input->evbit);
>                 goto ignore;
>
> -       case HID_UP_LOGIVENDOR:
> +       case HID_UP_LOGIVENDOR: case HID_UP_LOGIVENDOR2: case HID_UP_LOGIVENDOR3:

One line per case, please. Also, IIRC it would be good to add /*
intentional fallback */ between each case to make sure we are not
ignoring usages that should not.

Cheers,
Benjamin

>                 goto ignore;
>
>         case HID_UP_PID:
> diff --git a/include/linux/hid.h b/include/linux/hid.h
> index f17980d..ce1d883 100644
> --- a/include/linux/hid.h
> +++ b/include/linux/hid.h
> @@ -168,6 +168,8 @@ struct hid_item {
>  #define HID_UP_MSVENDOR                0xff000000
>  #define HID_UP_CUSTOM          0x00ff0000
>  #define HID_UP_LOGIVENDOR      0xffbc0000
> +#define HID_UP_LOGIVENDOR2   0xff090000
> +#define HID_UP_LOGIVENDOR3   0xff430000
>  #define HID_UP_LNVENDOR                0xffa00000
>  #define HID_UP_SENSOR          0x00200000
>
> --
> 2.1.4
>
> --
> 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

Re: [PATCH 6/6] HID: hid-logitech-hidpp: G920 remove deadzones

[Benjamin Tissoires]

On Sat, Nov 7, 2015 at 5:10 PM, Simon Wood <simon@mungewell.org> wrote:
> Ensure that the G920 is not given the default deadzones.
>
> Signed-off-by: Simon Wood <simon@mungewell.org>
> ---
>  drivers/hid/hid-logitech-hidpp.c | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
>
> diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
> index 03e01be..853b9c2 100644
> --- a/drivers/hid/hid-logitech-hidpp.c
> +++ b/drivers/hid/hid-logitech-hidpp.c
> @@ -1441,6 +1441,27 @@ static int hidpp_input_mapping(struct hid_device *hdev, struct hid_input *hi,
>         return 0;
>  }
>
> +static int hidpp_input_mapped(struct hid_device *hdev, struct hid_input *hi,
> +               struct hid_field *field, struct hid_usage *usage,
> +               unsigned long **bit, int *max)
> +{
> +       /* Ensure that Logitech G920 is not given a default fuzz/flat value */
> +       if (usage->type == EV_ABS && (usage->code == ABS_X ||
> +                       usage->code == ABS_Y || usage->code == ABS_Z ||
> +                       usage->code == ABS_RZ)) {
> +               switch (hdev->product) {
> +               case USB_DEVICE_ID_LOGITECH_G920_WHEEL:
> +                       field->application = HID_GD_MULTIAXIS;
> +                       break;
> +               default:
> +                       break;
> +               }
> +       }

I'd rather see this as:

if (hidpp->quirks & HIDPP_QUIRK_CLASS_G920) {
  if (usage->type == EV_ABS && (usage->code == ABS_X ....
    field->application = HID_GD_MULTIAXIS;
  }
}

Just to make sure we do not enter this code path for other devices.

With this change, and the nitpicks in the other patches, the series is:
Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>

Cheers,
Benjamin

> +
> +       return 0;
> +}
> +
> +
>  static void hidpp_populate_input(struct hidpp_device *hidpp,
>                 struct input_dev *input, bool origin_is_hid_core)
>  {
> @@ -1873,6 +1894,7 @@ static struct hid_driver hidpp_driver = {
>         .raw_event = hidpp_raw_event,
>         .input_configured = hidpp_input_configured,
>         .input_mapping = hidpp_input_mapping,
> +       .input_mapped = hidpp_input_mapped,
>  };
>
>  module_hid_driver(hidpp_driver);
> --
> 2.1.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 6/6] HID: hid-logitech-hidpp: G920 remove deadzones

[Simon Wood]

> With this change, and the nitpicks in the other patches, the series is:
> Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>

Have Patch-V2 in progress. Other build problems stopped in last night but
trying again with new pull - hopefully they are fixed.

Simon

Re: [PATCH 5/6] HID: Add vendor specific usage pages for Logitech G920

[Elias Vanderstuyft]

On Mon, Nov 9, 2015 at 9:20 AM, Benjamin Tissoires
<benjamin.tissoires@gmail.com> wrote:
> On Sat, Nov 7, 2015 at 5:10 PM, Simon Wood <simon@mungewell.org> wrote:
>> The Logitech G920 uses a couple of vendor specific usage pages,
>> which results in incorrect number of axis/buttons being detected.
>>
>> This patch adds these pages to the 'ignore' list.
>>
>> Reported-by: Elias Vanderstuyft <elias.vds@gmail.com>
>> Signed-off-by: Simon Wood <simon@mungewell.org>
>> ---
>>  drivers/hid/hid-input.c | 2 +-
>>  include/linux/hid.h     | 2 ++
>>  2 files changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
>> index 53aeaf6..c120be5 100644
>> --- a/drivers/hid/hid-input.c
>> +++ b/drivers/hid/hid-input.c
>> @@ -959,7 +959,7 @@ static void hidinput_configure_usage(struct hid_input *hidinput, struct hid_fiel
>>                 set_bit(EV_REP, input->evbit);
>>                 goto ignore;
>>
>> -       case HID_UP_LOGIVENDOR:
>> +       case HID_UP_LOGIVENDOR: case HID_UP_LOGIVENDOR2: case HID_UP_LOGIVENDOR3:
>
> One line per case, please.

This is my fault, I followed the convention used some lines above:
"case HID_GD_SLIDER: case HID_GD_DIAL: case HID_GD_WHEEL:"
but we expected this would get comments ;-)

Cheers,
Elias