[Patch-V2 0/6] HID: Support for the Logitech G920 Wheel

[Patch-V2 0/6] HID: Support for the Logitech G920 Wheel

[Simon Wood]

Patch-V2 tweaked as per Benjamin's requests.

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.

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-V2 1/6] INPUT: xpad: Add minimal support for Logitech G920
[Patch-V2 2/6] HID: hid-logitech-hidpp: Add support for very long
[Patch-V2 3/6] HID: hid-logitech-hidpp: Add basic support for
[Patch-V2 4/6] HID: hid-logitech-hidpp: Add range sysfs for Logitech
[Patch-V2 5/6] HID: Add vendor specific usage pages for Logitech G920
[Patch-V2 6/6] HID: hid-logitech-hidpp: G920 remove deadzones

[Patch-V2 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 fd4100d..338a3a4 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
@@ -134,6 +135,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 */
@@ -1048,6 +1051,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-V2 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 5fd9786..0f53dc8 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;
@@ -1347,6 +1376,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-V2 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.

Note: in 'hidpp_probe()' we have to start the hardware to get packets
flowing, the same might apply in future for other devices which don't
use the unifying protocol.

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 | 71 +++++++++++++++++++++++++++++++---------
 3 files changed, 57 insertions(+), 16 deletions(-)

diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index c6f7a69..190260c 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -1902,6 +1902,7 @@ static const struct hid_device_id hid_have_special_driver[] = {
 	{ 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_G29_WHEEL) },
+	{ 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 ac1feea..269e758 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -619,6 +619,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 0f53dc8..699a486 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,10 +1439,12 @@ static void hidpp_overwrite_name(struct hid_device *hdev, bool use_unifying)
 	else
 		name = hidpp_get_device_name(hidpp);
 
-	if (!name)
+	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);
 }
@@ -1596,6 +1607,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);
 
@@ -1604,8 +1634,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",
@@ -1618,19 +1647,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) {
@@ -1642,6 +1670,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);
@@ -1655,9 +1689,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[] = {
@@ -1685,6 +1721,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-V2 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 699a486..80ebd1c 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                                                      */
 /* -------------------------------------------------------------------------- */
@@ -1595,6 +1722,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);
@@ -1648,6 +1779,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 */
@@ -1673,6 +1808,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);
 	}
@@ -1689,8 +1825,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-V2 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 | 4 ++++
 include/linux/hid.h     | 2 ++
 2 files changed, 6 insertions(+)

diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
index 2ba6bf6..f4eeb6b 100644
--- a/drivers/hid/hid-input.c
+++ b/drivers/hid/hid-input.c
@@ -960,6 +960,10 @@ static void hidinput_configure_usage(struct hid_input *hidinput, struct hid_fiel
 		goto ignore;
 
 	case HID_UP_LOGIVENDOR:
+		/* intentional fallback */
+	case HID_UP_LOGIVENDOR2:
+		/* intentional fallback */
+	case HID_UP_LOGIVENDOR3:
 		goto ignore;
 
 	case HID_UP_PID:
diff --git a/include/linux/hid.h b/include/linux/hid.h
index 251a1d3..a6d7a3f 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-V2 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 | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index 80ebd1c..e235f3d 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -1441,6 +1441,25 @@ 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)
+{
+	struct hidpp_device *hidpp = hid_get_drvdata(hdev);
+
+	/* Ensure that Logitech G920 is not given a default fuzz/flat value */
+	if (hidpp->quirks & HIDPP_QUIRK_CLASS_G920) {
+		if (usage->type == EV_ABS && (usage->code == ABS_X ||
+				usage->code == ABS_Y || usage->code == ABS_Z ||
+				usage->code == ABS_RZ)) {
+			field->application = HID_GD_MULTIAXIS;
+		}
+	}
+
+	return 0;
+}
+
+
 static void hidpp_populate_input(struct hidpp_device *hidpp,
 		struct input_dev *input, bool origin_is_hid_core)
 {
@@ -1875,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-V2 0/6] HID: Support for the Logitech G920 Wheel

[Simon Wood]

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

Whoops, left this note in by mistake. These patches are against 4.4 HEAD.
Simon.

Re: [Patch-V2 0/6] HID: Support for the Logitech G920 Wheel

[Jiri Kosina]

On Thu, 12 Nov 2015, Simon Wood wrote:

> Patch-V2 tweaked as per Benjamin's requests.

Looking at the patches, I am happy with them, but still would like to have 
Benjamin's Ack/Reviewed-by before merging them.

Benjamin ... ?

Thanks,

-- 
Jiri Kosina
SUSE Labs

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

[Benjamin Tissoires]

On Nov 12 2015 or thereabouts, Simon Wood 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.
> 
> Note: in 'hidpp_probe()' we have to start the hardware to get packets
> flowing, the same might apply in future for other devices which don't
> use the unifying protocol.
> 
> 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 | 71 +++++++++++++++++++++++++++++++---------
>  3 files changed, 57 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
> index c6f7a69..190260c 100644
> --- a/drivers/hid/hid-core.c
> +++ b/drivers/hid/hid-core.c
> @@ -1902,6 +1902,7 @@ static const struct hid_device_id hid_have_special_driver[] = {
>  	{ 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_G29_WHEEL) },
> +	{ 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 ac1feea..269e758 100644
> --- a/drivers/hid/hid-ids.h
> +++ b/drivers/hid/hid-ids.h
> @@ -619,6 +619,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 0f53dc8..699a486 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,10 +1439,12 @@ static void hidpp_overwrite_name(struct hid_device *hdev, bool use_unifying)
>  	else
>  		name = hidpp_get_device_name(hidpp);
>  
> -	if (!name)
> +	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);
>  }
> @@ -1596,6 +1607,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;
> +

Sorry, I missed this one. But with the latest changes, the test should
be:
if (hidpp->quirks & HIDPP_QUIRK_NO_HIDINPUT)

see below:

> +	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);
>  
> @@ -1604,8 +1634,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",
> @@ -1618,19 +1647,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;
> -

This is the hunk that has been moved up and that has been recently
changed.

> -	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) {
> @@ -1642,6 +1670,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);
> @@ -1655,9 +1689,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[] = {
> @@ -1685,6 +1721,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
> 

Sorry for not spotting this earlier. Maybe Jiri can change it locally if
there are no other changes to make to the series?

Cheers,
Benjamin

Re: [Patch-V2 0/6] HID: Support for the Logitech G920 Wheel

[Benjamin Tissoires]

On Nov 19 2015 or thereabouts, Jiri Kosina wrote:
> On Thu, 12 Nov 2015, Simon Wood wrote:
> 
> > Patch-V2 tweaked as per Benjamin's requests.
> 
> Looking at the patches, I am happy with them, but still would like to have 
> Benjamin's Ack/Reviewed-by before merging them.
> 
> Benjamin ... ?

Sorry, I thought my previous review was enough and that I already gave
the rev-by.

I re-read the patches and found one small left over of a rebase. Maybe
you can just fix it locally if it's not too much to ask.

The series is:
Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>

I'll add to my TODO list the testing of this series with the HW coming
in the move I should receive tomorrow. I don't expect much to be found,
given that I must have tested the preliminary versions of the series.
So I'd be glad if you can pull this. I'll just send a fix if there is a
need to have one.

Cheers,
Benjamin

Re: [Patch-V2 1/6] INPUT: xpad: Add minimal support for Logitech G920 Wheel

[Jiri Kosina]

On Thu, 12 Nov 2015, Simon Wood wrote:

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

Adding Dmitry to CC.

Dmitry, I am planning to take this through my tree together with the rest 
of the actual HID support for that device if you Ack this.

Thanks,

-- 
Jiri Kosina
SUSE Labs

Re: [Patch-V2 1/6] INPUT: xpad: Add minimal support for Logitech G920 Wheel

[Dmitry Torokhov]

On Thu, Nov 19, 2015 at 02:50:51PM +0100, Jiri Kosina wrote:
> On Thu, 12 Nov 2015, Simon Wood wrote:
> 
> > 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>
> 
> Adding Dmitry to CC.
> 
> Dmitry, I am planning to take this through my tree together with the rest 
> of the actual HID support for that device if you Ack this.

Hmm, I have an incoming series for xbox that night clash with this... If
you'll put it in a clean branch off 4.3 I'd pull it and then get more
changes on top.

Can we also change the subject as it is not about adding a minimal
support. Something like "Input: xpad - switch Logitech G920 Wheel into
HID mode"

Otherwise:

Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

Thanks.

-- 
Dmitry

Re: [Patch-V2 1/6] INPUT: xpad: Add minimal support for Logitech G920 Wheel

[Simon Wood]

On Thu, November 19, 2015 11:31 am, Dmitry Torokhov wrote:
> On Thu, Nov 19, 2015 at 02:50:51PM +0100, Jiri Kosina wrote:
>
>> On Thu, 12 Nov 2015, Simon Wood wrote:
>>
>>
>>> 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>
>>>
>>
>> Adding Dmitry to CC.
>>
>>
>> Dmitry, I am planning to take this through my tree together with the
>> rest of the actual HID support for that device if you Ack this.
>
> Hmm, I have an incoming series for xbox that night clash with this... If
> you'll put it in a clean branch off 4.3 I'd pull it and then get more
> changes on top.
>
> Can we also change the subject as it is not about adding a minimal
> support. Something like "Input: xpad - switch Logitech G920 Wheel into HID
> mode"

Will spin a 'v3' with this and Benjamin's suggestion.

Cheers,
Simon.

Re: [Patch-V2 1/6] INPUT: xpad: Add minimal support for Logitech G920 Wheel

[Edwin]

On 19-11-15 19:35, Simon Wood wrote:
> On Thu, November 19, 2015 11:31 am, Dmitry Torokhov wrote:
> > On Thu, Nov 19, 2015 at 02:50:51PM +0100, Jiri Kosina wrote:
> >
> >> On Thu, 12 Nov 2015, Simon Wood wrote:
> >>
> >>
> >>> 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>
> >>>
> >>
> >> Adding Dmitry to CC.
> >>
> >>
> >> Dmitry, I am planning to take this through my tree together with the
> >> rest of the actual HID support for that device if you Ack this.
> >
> > Hmm, I have an incoming series for xbox that night clash with this... If
> > you'll put it in a clean branch off 4.3 I'd pull it and then get more
> > changes on top.
> >
> > Can we also change the subject as it is not about adding a minimal
> > support. Something like "Input: xpad - switch Logitech G920 Wheel into HID
> > mode"
>
> Will spin a 'v3' with this and Benjamin's suggestion.
>
> Cheers,
> Simon.
>

I'll put out the force feedback patch right (or as close as I can) after v3.

Edwin

Re: [Patch-V2 1/6] INPUT: xpad: Add minimal support for Logitech G920 Wheel

[Dmitry Torokhov]

On Thu, Nov 19, 2015 at 10:31 AM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> On Thu, Nov 19, 2015 at 02:50:51PM +0100, Jiri Kosina wrote:
>> On Thu, 12 Nov 2015, Simon Wood wrote:
>>
>> > 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>
>>
>> Adding Dmitry to CC.
>>
>> Dmitry, I am planning to take this through my tree together with the rest
>> of the actual HID support for that device if you Ack this.
>
> Hmm, I have an incoming series for xbox that night clash with this... If
> you'll put it in a clean branch off 4.3 I'd pull it and then get more
> changes on top.
>
> Can we also change the subject as it is not about adding a minimal
> support. Something like "Input: xpad - switch Logitech G920 Wheel into
> HID mode"
>
> Otherwise:
>
> Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

Hmm, looking sat this some more why are we waiting to switch device
mode until after userspace opens input device instead of when we are
executing driver probe()?

Thanks.

-- 
Dmitry

Re: [Patch-V2 1/6] INPUT: xpad: Add minimal support for Logitech G920 Wheel

[Dmitry Torokhov]

On Wed, Dec 9, 2015 at 5:23 PM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> On Thu, Nov 19, 2015 at 10:31 AM, Dmitry Torokhov
> <dmitry.torokhov@gmail.com> wrote:
>> On Thu, Nov 19, 2015 at 02:50:51PM +0100, Jiri Kosina wrote:
>>> On Thu, 12 Nov 2015, Simon Wood wrote:
>>>
>>> > 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>
>>>
>>> Adding Dmitry to CC.
>>>
>>> Dmitry, I am planning to take this through my tree together with the rest
>>> of the actual HID support for that device if you Ack this.
>>
>> Hmm, I have an incoming series for xbox that night clash with this... If
>> you'll put it in a clean branch off 4.3 I'd pull it and then get more
>> changes on top.
>>
>> Can we also change the subject as it is not about adding a minimal
>> support. Something like "Input: xpad - switch Logitech G920 Wheel into
>> HID mode"
>>
>> Otherwise:
>>
>> Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
>
> Hmm, looking sat this some more why are we waiting to switch device
> mode until after userspace opens input device instead of when we are
> executing driver probe()?
>

Actually, thinking about it even more, why do we want to have this in
xpad.c? Have HID module handle both IDs and switch to HID mode if we
want HID to handle this device. I think we should revert/drop this
patch.

Thanks.

-- 
Dmitry

Re: [Patch-V2 1/6] INPUT: xpad: Add minimal support for Logitech G920 Wheel

[Benjamin Tissoires]

On Dec 09 2015 or thereabouts, Dmitry Torokhov wrote:
> On Wed, Dec 9, 2015 at 5:23 PM, Dmitry Torokhov
> <dmitry.torokhov@gmail.com> wrote:
> > On Thu, Nov 19, 2015 at 10:31 AM, Dmitry Torokhov
> > <dmitry.torokhov@gmail.com> wrote:
> >> On Thu, Nov 19, 2015 at 02:50:51PM +0100, Jiri Kosina wrote:
> >>> On Thu, 12 Nov 2015, Simon Wood wrote:
> >>>
> >>> > 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>
> >>>
> >>> Adding Dmitry to CC.
> >>>
> >>> Dmitry, I am planning to take this through my tree together with the rest
> >>> of the actual HID support for that device if you Ack this.
> >>
> >> Hmm, I have an incoming series for xbox that night clash with this... If
> >> you'll put it in a clean branch off 4.3 I'd pull it and then get more
> >> changes on top.
> >>
> >> Can we also change the subject as it is not about adding a minimal
> >> support. Something like "Input: xpad - switch Logitech G920 Wheel into
> >> HID mode"
> >>
> >> Otherwise:
> >>
> >> Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> >
> > Hmm, looking sat this some more why are we waiting to switch device
> > mode until after userspace opens input device instead of when we are
> > executing driver probe()?
> >
> 
> Actually, thinking about it even more, why do we want to have this in
> xpad.c? Have HID module handle both IDs and switch to HID mode if we
> want HID to handle this device. I think we should revert/drop this
> patch.
> 

Hi Dmitry,

IIRC, last time I saw an XBox-like controller, it doesn't register as a
HID device at all. SO I think It will be hard to switch it into the HID
mode from HID directly.
Simon, can you confirm that the device does not contains any references
to HID while in the XBox mode (lsusb -v should give enough information).

Switching the device during probe in xpad.c makes a lot of sense
however.

Cheers,
Benjamin

Re: [Patch-V2 1/6] INPUT: xpad: Add minimal support for Logitech G920 Wheel

[Dmitry Torokhov]

On Thu, Dec 10, 2015 at 9:08 AM, Benjamin Tissoires
<benjamin.tissoires@redhat.com> wrote:
> On Dec 09 2015 or thereabouts, Dmitry Torokhov wrote:
>> On Wed, Dec 9, 2015 at 5:23 PM, Dmitry Torokhov
>> <dmitry.torokhov@gmail.com> wrote:
>> > On Thu, Nov 19, 2015 at 10:31 AM, Dmitry Torokhov
>> > <dmitry.torokhov@gmail.com> wrote:
>> >> On Thu, Nov 19, 2015 at 02:50:51PM +0100, Jiri Kosina wrote:
>> >>> On Thu, 12 Nov 2015, Simon Wood wrote:
>> >>>
>> >>> > 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>
>> >>>
>> >>> Adding Dmitry to CC.
>> >>>
>> >>> Dmitry, I am planning to take this through my tree together with the rest
>> >>> of the actual HID support for that device if you Ack this.
>> >>
>> >> Hmm, I have an incoming series for xbox that night clash with this... If
>> >> you'll put it in a clean branch off 4.3 I'd pull it and then get more
>> >> changes on top.
>> >>
>> >> Can we also change the subject as it is not about adding a minimal
>> >> support. Something like "Input: xpad - switch Logitech G920 Wheel into
>> >> HID mode"
>> >>
>> >> Otherwise:
>> >>
>> >> Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
>> >
>> > Hmm, looking sat this some more why are we waiting to switch device
>> > mode until after userspace opens input device instead of when we are
>> > executing driver probe()?
>> >
>>
>> Actually, thinking about it even more, why do we want to have this in
>> xpad.c? Have HID module handle both IDs and switch to HID mode if we
>> want HID to handle this device. I think we should revert/drop this
>> patch.
>>
>
> Hi Dmitry,

Hi Benjamin,

>
> IIRC, last time I saw an XBox-like controller, it doesn't register as a
> HID device at all. SO I think It will be hard to switch it into the HID
> mode from HID directly.
> Simon, can you confirm that the device does not contains any references
> to HID while in the XBox mode (lsusb -v should give enough information).
>
> Switching the device during probe in xpad.c makes a lot of sense
> however.

It makes as much sense doing it in xpad as doing it from a random USB
network driver. I mean the only reason we are doing it from xpad is
because of name and the fat that it has usb_driver structure. Nobody
stops you from creating a tiny USB stub driver in hid portion that
would probe the "non-hid" device and switch it over to hid.

Thanks.

-- 
Dmitry

Re: [Patch-V2 1/6] INPUT: xpad: Add minimal support for Logitech G920 Wheel

[Elias Vanderstuyft]

On Thu, Dec 10, 2015 at 6:08 PM, Benjamin Tissoires
<benjamin.tissoires@redhat.com> wrote:
> On Dec 09 2015 or thereabouts, Dmitry Torokhov wrote:
>> On Wed, Dec 9, 2015 at 5:23 PM, Dmitry Torokhov
>> <dmitry.torokhov@gmail.com> wrote:
>> > On Thu, Nov 19, 2015 at 10:31 AM, Dmitry Torokhov
>> > <dmitry.torokhov@gmail.com> wrote:
>> >> On Thu, Nov 19, 2015 at 02:50:51PM +0100, Jiri Kosina wrote:
>> >>> On Thu, 12 Nov 2015, Simon Wood wrote:
>> >>>
>> >>> > 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>
>> >>>
>> >>> Adding Dmitry to CC.
>> >>>
>> >>> Dmitry, I am planning to take this through my tree together with the rest
>> >>> of the actual HID support for that device if you Ack this.
>> >>
>> >> Hmm, I have an incoming series for xbox that night clash with this... If
>> >> you'll put it in a clean branch off 4.3 I'd pull it and then get more
>> >> changes on top.
>> >>
>> >> Can we also change the subject as it is not about adding a minimal
>> >> support. Something like "Input: xpad - switch Logitech G920 Wheel into
>> >> HID mode"
>> >>
>> >> Otherwise:
>> >>
>> >> Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
>> >
>> > Hmm, looking sat this some more why are we waiting to switch device
>> > mode until after userspace opens input device instead of when we are
>> > executing driver probe()?
>> >
>>
>> Actually, thinking about it even more, why do we want to have this in
>> xpad.c? Have HID module handle both IDs and switch to HID mode if we
>> want HID to handle this device. I think we should revert/drop this
>> patch.
>>
>
> Hi Dmitry,
>
> IIRC, last time I saw an XBox-like controller, it doesn't register as a
> HID device at all. SO I think It will be hard to switch it into the HID
> mode from HID directly.
> Simon, can you confirm that the device does not contains any references
> to HID while in the XBox mode (lsusb -v should give enough information).

This is probably not relevant anymore, but here is the "sudo lsusb -v"
output of my G920 in XBox mode:

Bus 001 Device 121: ID 046d:c261 Logitech, Inc.
Device Descriptor:
  bLength                18
  bDescriptorType         1
  bcdUSB               2.00
  bDeviceClass          255 Vendor Specific Class
  bDeviceSubClass       255 Vendor Specific Subclass
  bDeviceProtocol       255 Vendor Specific Protocol
  bMaxPacketSize0        64
  idVendor           0x046d Logitech, Inc.
  idProduct          0xc261
  bcdDevice           96.01
  iManufacturer           1 Logitech
  iProduct                2 G920 Driving Force Racing Wheel for Xbox One
  iSerial                 3 000072fb6b3b9f58
  bNumConfigurations      1
  Configuration Descriptor:
    bLength                 9
    bDescriptorType         2
    wTotalLength           32
    bNumInterfaces          1
    bConfigurationValue     1
    iConfiguration          0
    bmAttributes         0xa0
      (Bus Powered)
      Remote Wakeup
    MaxPower              100mA
    Interface Descriptor:
      bLength                 9
      bDescriptorType         4
      bInterfaceNumber        0
      bAlternateSetting       0
      bNumEndpoints           2
      bInterfaceClass       255 Vendor Specific Class
      bInterfaceSubClass     71
      bInterfaceProtocol    208
      iInterface              0
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x01  EP 1 OUT
        bmAttributes            3
          Transfer Type            Interrupt
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0040  1x 64 bytes
        bInterval               4
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x81  EP 1 IN
        bmAttributes            3
          Transfer Type            Interrupt
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0040  1x 64 bytes
        bInterval               4
Device Status:     0x0002
  (Bus Powered)
  Remote Wakeup Enabled

Re: [Patch-V2 1/6] INPUT: xpad: Add minimal support for Logitech G920 Wheel

[Benjamin Tissoires]

On Dec 10 2015 or thereabouts, Dmitry Torokhov wrote:
> On Thu, Dec 10, 2015 at 9:08 AM, Benjamin Tissoires
> <benjamin.tissoires@redhat.com> wrote:
> > On Dec 09 2015 or thereabouts, Dmitry Torokhov wrote:
> >> On Wed, Dec 9, 2015 at 5:23 PM, Dmitry Torokhov
> >> <dmitry.torokhov@gmail.com> wrote:
> >> > On Thu, Nov 19, 2015 at 10:31 AM, Dmitry Torokhov
> >> > <dmitry.torokhov@gmail.com> wrote:
> >> >> On Thu, Nov 19, 2015 at 02:50:51PM +0100, Jiri Kosina wrote:
> >> >>> On Thu, 12 Nov 2015, Simon Wood wrote:
> >> >>>
> >> >>> > 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>
> >> >>>
> >> >>> Adding Dmitry to CC.
> >> >>>
> >> >>> Dmitry, I am planning to take this through my tree together with the rest
> >> >>> of the actual HID support for that device if you Ack this.
> >> >>
> >> >> Hmm, I have an incoming series for xbox that night clash with this... If
> >> >> you'll put it in a clean branch off 4.3 I'd pull it and then get more
> >> >> changes on top.
> >> >>
> >> >> Can we also change the subject as it is not about adding a minimal
> >> >> support. Something like "Input: xpad - switch Logitech G920 Wheel into
> >> >> HID mode"
> >> >>
> >> >> Otherwise:
> >> >>
> >> >> Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> >> >
> >> > Hmm, looking sat this some more why are we waiting to switch device
> >> > mode until after userspace opens input device instead of when we are
> >> > executing driver probe()?
> >> >
> >>
> >> Actually, thinking about it even more, why do we want to have this in
> >> xpad.c? Have HID module handle both IDs and switch to HID mode if we
> >> want HID to handle this device. I think we should revert/drop this
> >> patch.
> >>
> >
> > Hi Dmitry,
> 
> Hi Benjamin,
> 
> >
> > IIRC, last time I saw an XBox-like controller, it doesn't register as a
> > HID device at all. SO I think It will be hard to switch it into the HID
> > mode from HID directly.
> > Simon, can you confirm that the device does not contains any references
> > to HID while in the XBox mode (lsusb -v should give enough information).
> >
> > Switching the device during probe in xpad.c makes a lot of sense
> > however.
> 
> It makes as much sense doing it in xpad as doing it from a random USB
> network driver. I mean the only reason we are doing it from xpad is
> because of name and the fat that it has usb_driver structure. Nobody
> stops you from creating a tiny USB stub driver in hid portion that
> would probe the "non-hid" device and switch it over to hid.
> 

Jiri, I *think* this commit still is in your next pull request for
Linus. We might want to drop it before it hits Linus' tree.

We can still keep the HID work in place even if the device is not
switched into the HID protocol at plug.

Simon, do you mind looking into Dmitry's suggestion of having a clean,
small usb device which loads itself when the G920 is plugged in and
switches it immediately into the HID mode?

Cheers,
Benjamin

Re: [Patch-V2 1/6] INPUT: xpad: Add minimal support for Logitech G920 Wheel

[madcatxster]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=utf-8, Size: 4030 bytes --]



On Mon Jan 4 10:55:24 2016 GMT+0100, Benjamin Tissoires wrote:
> On Dec 10 2015 or thereabouts, Dmitry Torokhov wrote:
> > On Thu, Dec 10, 2015 at 9:08 AM, Benjamin Tissoires
> > <benjamin.tissoires@redhat.com> wrote:
> > > On Dec 09 2015 or thereabouts, Dmitry Torokhov wrote:
> > >> On Wed, Dec 9, 2015 at 5:23 PM, Dmitry Torokhov
> > >> <dmitry.torokhov@gmail.com> wrote:
> > >> > On Thu, Nov 19, 2015 at 10:31 AM, Dmitry Torokhov
> > >> > <dmitry.torokhov@gmail.com> wrote:
> > >> >> On Thu, Nov 19, 2015 at 02:50:51PM +0100, Jiri Kosina wrote:
> > >> >>> On Thu, 12 Nov 2015, Simon Wood wrote:
> > >> >>>
> > >> >>> > 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>
> > >> >>>
> > >> >>> Adding Dmitry to CC.
> > >> >>>
> > >> >>> Dmitry, I am planning to take this through my tree together with the rest
> > >> >>> of the actual HID support for that device if you Ack this.
> > >> >>
> > >> >> Hmm, I have an incoming series for xbox that night clash with this... If
> > >> >> you'll put it in a clean branch off 4.3 I'd pull it and then get more
> > >> >> changes on top.
> > >> >>
> > >> >> Can we also change the subject as it is not about adding a minimal
> > >> >> support. Something like "Input: xpad - switch Logitech G920 Wheel into
> > >> >> HID mode"
> > >> >>
> > >> >> Otherwise:
> > >> >>
> > >> >> Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > >> >
> > >> > Hmm, looking sat this some more why are we waiting to switch device
> > >> > mode until after userspace opens input device instead of when we are
> > >> > executing driver probe()?
> > >> >
> > >>
> > >> Actually, thinking about it even more, why do we want to have this in
> > >> xpad.c? Have HID module handle both IDs and switch to HID mode if we
> > >> want HID to handle this device. I think we should revert/drop this
> > >> patch.
> > >>
> > >
> > > Hi Dmitry,
> > 
> > Hi Benjamin,
> > 
> > >
> > > IIRC, last time I saw an XBox-like controller, it doesn't register as a
> > > HID device at all. SO I think It will be hard to switch it into the HID
> > > mode from HID directly.
> > > Simon, can you confirm that the device does not contains any references
> > > to HID while in the XBox mode (lsusb -v should give enough information).
> > >
> > > Switching the device during probe in xpad.c makes a lot of sense
> > > however.
> > 
> > It makes as much sense doing it in xpad as doing it from a random USB
> > network driver. I mean the only reason we are doing it from xpad is
> > because of name and the fat that it has usb_driver structure. Nobody
> > stops you from creating a tiny USB stub driver in hid portion that
> > would probe the "non-hid" device and switch it over to hid.
> > 
> 
> Jiri, I *think* this commit still is in your next pull request for
> Linus. We might want to drop it before it hits Linus' tree.
> 
> We can still keep the HID work in place even if the device is not
> switched into the HID protocol at plug.
> 
> Simon, do you mind looking into Dmitry's suggestion of having a clean,
> small usb device which loads itself when the G920 is plugged in and
> switches it immediately into the HID mode?
> 
> Cheers,
> Benjamin
> 

Hi guys,

since I feel pretty bad for not contributing to the project (too much PhD stuff) I'd gladly look into the USB stub driver is Simon is too buys or otherwise unable to do it himself. If I understand the issue the idea is to have a simple module that would pick up a device that at first appears as a generic USB device, do the necessary initialization and let the device-specific driver take over from there?

Michalÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

Re: [Patch-V2 1/6] INPUT: xpad: Add minimal support for Logitech G920 Wheel

[madcatxster]

On Mon Jan 4 10:55:24 2016 GMT+0100, Benjamin Tissoires wrote:
> On Dec 10 2015 or thereabouts, Dmitry Torokhov wrote:
> > On Thu, Dec 10, 2015 at 9:08 AM, Benjamin Tissoires
> > <benjamin.tissoires@redhat.com> wrote:
> > > On Dec 09 2015 or thereabouts, Dmitry Torokhov wrote:
> > >> On Wed, Dec 9, 2015 at 5:23 PM, Dmitry Torokhov
> > >> <dmitry.torokhov@gmail.com> wrote:
> > >> > On Thu, Nov 19, 2015 at 10:31 AM, Dmitry Torokhov
> > >> > <dmitry.torokhov@gmail.com> wrote:
> > >> >> On Thu, Nov 19, 2015 at 02:50:51PM +0100, Jiri Kosina wrote:
> > >> >>> On Thu, 12 Nov 2015, Simon Wood wrote:
> > >> >>>
> > >> >>> > 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>
> > >> >>>
> > >> >>> Adding Dmitry to CC.
> > >> >>>
> > >> >>> Dmitry, I am planning to take this through my tree together with the rest
> > >> >>> of the actual HID support for that device if you Ack this.
> > >> >>
> > >> >> Hmm, I have an incoming series for xbox that night clash with this... If
> > >> >> you'll put it in a clean branch off 4.3 I'd pull it and then get more
> > >> >> changes on top.
> > >> >>
> > >> >> Can we also change the subject as it is not about adding a minimal
> > >> >> support. Something like "Input: xpad - switch Logitech G920 Wheel into
> > >> >> HID mode"
> > >> >>
> > >> >> Otherwise:
> > >> >>
> > >> >> Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > >> >
> > >> > Hmm, looking sat this some more why are we waiting to switch device
> > >> > mode until after userspace opens input device instead of when we are
> > >> > executing driver probe()?
> > >> >
> > >>
> > >> Actually, thinking about it even more, why do we want to have this in
> > >> xpad.c? Have HID module handle both IDs and switch to HID mode if we
> > >> want HID to handle this device. I think we should revert/drop this
> > >> patch.
> > >>
> > >
> > > Hi Dmitry,
> > 
> > Hi Benjamin,
> > 
> > >
> > > IIRC, last time I saw an XBox-like controller, it doesn't register as a
> > > HID device at all. SO I think It will be hard to switch it into the HID
> > > mode from HID directly.
> > > Simon, can you confirm that the device does not contains any references
> > > to HID while in the XBox mode (lsusb -v should give enough information).
> > >
> > > Switching the device during probe in xpad.c makes a lot of sense
> > > however.
> > 
> > It makes as much sense doing it in xpad as doing it from a random USB
> > network driver. I mean the only reason we are doing it from xpad is
> > because of name and the fat that it has usb_driver structure. Nobody
> > stops you from creating a tiny USB stub driver in hid portion that
> > would probe the "non-hid" device and switch it over to hid.
> > 
> 
> Jiri, I *think* this commit still is in your next pull request for
> Linus. We might want to drop it before it hits Linus' tree.
> 
> We can still keep the HID work in place even if the device is not
> switched into the HID protocol at plug.
> 
> Simon, do you mind looking into Dmitry's suggestion of having a clean,
> small usb device which loads itself when the G920 is plugged in and
> switches it immediately into the HID mode?
> 
> Cheers,
> Benjamin
> 

Hi guys,

since I feel pretty bad for not contributing to the project (too much PhD stuff) I'd gladly look into the USB stub driver is Simon is too buys or otherwise unable to do it himself. If I understand the issue the idea is to have a simple module that would pick up a device that at first appears as a generic USB device, do the necessary initialization and let the device-specific driver take over from there?

Michal

Re: [Patch-V2 1/6] INPUT: xpad: Add minimal support for Logitech G920 Wheel

[Dmitry Torokhov]

Hi Simon,

On Mon, Jan 04, 2016 at 01:05:35PM -0700, Simon Wood wrote:
> 
> On Mon, 4 Jan 2016 02:55:24 -0700, Benjamin Tissoires wrote:
> your next pull request for
> > Linus. We might want to drop it before it hits Linus' tree.
> > 
> > We can still keep the HID work in place even if the device is not
> > switched into the HID protocol at plug.
> > 
> > Simon, do you mind looking into Dmitry's suggestion of having a clean,
> > small usb device which loads itself when the G920 is plugged in and
> > switches it immediately into the HID mode?
> 

> Hi,
>
> As noted 'xpad.c' sends the magic bytes to switch the G920 wheel into
> HID mode, the wheel then detaches and reconnects as a HID+ device
> handled by 'logitech-hidpp.c'.
> 
> I'd like to point that up to this point the wheel _is_ a Xbox One
> control device, that speaks some undocumented protocol from Microsoft.
> To my mind xpad is the correct place to send the magic bytes.

OTOH one can argue if it does not speak protocol that xpad understands
then it does not belong in xpad.

Currently xpad.ko "weighs" at 44K (in my configuration), this is steep
price for sending 1 packet to the device. I am certain that adding a
skeleton usb driver into hid module will be much cheaper.

> 
> It is possible that at some future time that the Xpad devs will
> understand the protocol enough sufficiently to do useful stuff with
> the wheel. In an earlier version of the patch I had a param to disable
> the 'switch to HID', and that might be resurrected.

This is way down the road. We may even end up with a brand new driver
for it, not xpad. And if/when that happens we'll be able to drop hid
potion.

Thanks.

-- 
Dmitry

Re: [Patch-V2 1/6] INPUT: xpad: Add minimal support for Logitech G920 Wheel

[Jiri Kosina]

On Mon, 4 Jan 2016, Benjamin Tissoires wrote:

> Jiri, I *think* this commit still is in your next pull request for
> Linus. We might want to drop it before it hits Linus' tree.

What exactly would be the reasoning for dropping it?

I am all for implementing the skeleton HID driver to take over the xpad.c 
heavylifting, but before that work gets done, this can stay, can't it?

Thanks,

-- 
Jiri Kosina
SUSE Labs

Re: [Patch-V2 1/6] INPUT: xpad: Add minimal support for Logitech G920 Wheel

[Dmitry Torokhov]

On Wed, Jan 06, 2016 at 03:36:57PM +0100, Jiri Kosina wrote:
> On Mon, 4 Jan 2016, Benjamin Tissoires wrote:
> 
> > Jiri, I *think* this commit still is in your next pull request for
> > Linus. We might want to drop it before it hits Linus' tree.
> 
> What exactly would be the reasoning for dropping it?

It is wrong. Aside form the fact that IMO xpad.c is the wrong place for
this code to be in, why are we waiting for the input device to be
opened by userspace before we do the switch instead of doing it
immediately?

Thanks.

-- 
Dmitry

Re: [Patch-V2 1/6] INPUT: xpad: Add minimal support for Logitech G920 Wheel

[Simon Wood]

On Wed, January 6, 2016 6:47 pm, Dmitry Torokhov wrote:

> It is wrong. Aside form the fact that IMO xpad.c is the wrong place for
> this code to be in, why are we waiting for the input device to be opened by
> userspace before we do the switch instead of doing it immediately?

The 'send magic' might be better in a probe() call, but I don't believe
that it requires userspace interaction as it stands. The wheel will
disconnect almost immediately without me doing anything other than plug it
in.
--
Jan  6 21:18:50 speedster kernel: [  439.604037] usb 5-1: new full-speed
USB device number 2 using uhci_hcd
Jan  6 21:18:50 speedster kernel: [  439.791153] usb 5-1: New USB device
found, idVendor=046d, idProduct=c261
Jan  6 21:18:50 speedster kernel: [  439.791160] usb 5-1: New USB device
strings: Mfr=1, Product=2, SerialNumber=3
Jan  6 21:18:50 speedster kernel: [  439.791164] usb 5-1: Product: G920
Driving Force Racing Wheel for Xbox One
Jan  6 21:18:50 speedster kernel: [  439.791167] usb 5-1: Manufacturer:
Logitech
Jan  6 21:18:50 speedster kernel: [  439.791170] usb 5-1: SerialNumber:
00005d1d5129cebe
Jan  6 21:18:50 speedster mtp-probe: checking bus 5, device 2:
"/sys/devices/pci0000:00/0000:00:1d.3/usb5/5-1"
Jan  6 21:18:50 speedster mtp-probe: bus: 5, device: 2 was not an MTP device
Jan  6 21:18:51 speedster kernel: [  440.815191] input: Logitech G920
Driving Force Racing Wheel as
/devices/pci0000:00/0000:00:1d.3/usb5/5-1/5-1:1.0/input/input4
Jan  6 21:18:51 speedster kernel: [  440.815310] usbcore: registered new
interface driver xpad
Jan  6 21:18:52 speedster kernel: [  441.340093] usb 5-1: USB disconnect,
device number 2
Jan  6 21:18:52 speedster kernel: [  442.052037] usb 5-1: new full-speed
USB device number 3 using uhci_hcd
Jan  6 21:18:52 speedster kernel: [  442.239129] usb 5-1: New USB device
found, idVendor=046d, idProduct=c262
Jan  6 21:18:52 speedster kernel: [  442.239136] usb 5-1: New USB device
strings: Mfr=1, Product=2, SerialNumber=3
Jan  6 21:18:52 speedster kernel: [  442.239139] usb 5-1: Product: G920
Driving Force Racing Wheel for Xbox One
Jan  6 21:18:52 speedster kernel: [  442.239142] usb 5-1: Manufacturer:
Logitech
Jan  6 21:18:52 speedster kernel: [  442.239145] usb 5-1: SerialNumber:
00005d1d5129cebe
Jan  6 21:18:52 speedster mtp-probe: checking bus 5, device 3:
"/sys/devices/pci0000:00/0000:00:1d.3/usb5/5-1"
Jan  6 21:18:52 speedster mtp-probe: bus: 5, device: 3 was not an MTP device
Jan  6 21:18:52 speedster kernel: [  442.267248] input: Logitech G920
Driving Force Racing Wheel for Xbox One as
/devices/pci0000:00/0000:00:1d.3/usb5/5-1/5-1:1.0/0003:046D:C262.0002/input/input5
Jan  6 21:18:52 speedster kernel: [  442.267510] logitech-hidpp-device
0003:046D:C262.0002: input,hiddev0,hidraw1: USB HID v1.11 Joystick
[Logitech G920 Driving Force Racing Wheel for Xbox One] on
usb-0000:00:1d.3-1/input0
Jan  6 21:18:53 speedster kernel: [  442.322160] logitech-hidpp-device
0003:046D:C262.0002: HID++ 4.2 device connected.
--

I also did a quick check with the 'send magic' disabled. Xpad creates the
js0 and populates buttons, but pressing buttons/turning wheel does not
result in any change in js0.

I don't disagree to it being a seperate module, but don't have the time to
implement/test at the moment. If some else does that would be good, can we
make sure that the kconfig/makefile stuff uses same/sensible HID_CONFIGs?
Simon

Re: [Patch-V2 1/6] INPUT: xpad: Add minimal support for Logitech G920 Wheel

[Michal Malý]

On Wed, 2016-01-06 at 17:47 -0800, Dmitry Torokhov wrote:
> On Wed, Jan 06, 2016 at 03:36:57PM +0100, Jiri Kosina wrote:
> > On Mon, 4 Jan 2016, Benjamin Tissoires wrote:
> > 
> > > Jiri, I *think* this commit still is in your next pull request
> > > for
> > > Linus. We might want to drop it before it hits Linus' tree.
> > 
> > What exactly would be the reasoning for dropping it?
> 
> It is wrong. Aside form the fact that IMO xpad.c is the wrong place
> for
> this code to be in, why are we waiting for the input device to be
> opened by userspace before we do the switch instead of doing it
> immediately?
> 

Hi all,

I have to disagree with the xpad driver being the wrong place to handle
this. The xpad driver matches devices it should handle by interface
class, subclass and protocol. When G920 first appears on the USB bus,
it for all intents and purposes looks like a Xbox One controller so the
xpad driver picks it up even if there is no G920-specific code in the
driver. Unless there is a way how to blacklist certain idProduct
values, the switch from XBone mode to HID mode will have to be done in
the xpad driver.

I'm pretty much done with the simple switching module but it will be of
no use if we cannot make the xpad module ignore G920 first.

Michal

Re: [Patch-V2 1/6] INPUT: xpad: Add minimal support for Logitech G920 Wheel

[Dmitry Torokhov]

On Thu, Jan 7, 2016 at 2:50 PM, Michal Malý
<madcatxster@devoid-pointer.net> wrote:
> On Wed, 2016-01-06 at 17:47 -0800, Dmitry Torokhov wrote:
>> On Wed, Jan 06, 2016 at 03:36:57PM +0100, Jiri Kosina wrote:
>> > On Mon, 4 Jan 2016, Benjamin Tissoires wrote:
>> >
>> > > Jiri, I *think* this commit still is in your next pull request
>> > > for
>> > > Linus. We might want to drop it before it hits Linus' tree.
>> >
>> > What exactly would be the reasoning for dropping it?
>>
>> It is wrong. Aside form the fact that IMO xpad.c is the wrong place
>> for
>> this code to be in, why are we waiting for the input device to be
>> opened by userspace before we do the switch instead of doing it
>> immediately?
>>
>
> Hi all,
>
> I have to disagree with the xpad driver being the wrong place to handle
> this. The xpad driver matches devices it should handle by interface
> class, subclass and protocol. When G920 first appears on the USB bus,
> it for all intents and purposes looks like a Xbox One controller so the
> xpad driver picks it up even if there is no G920-specific code in the
> driver. Unless there is a way how to blacklist certain idProduct
> values, the switch from XBone mode to HID mode will have to be done in
> the xpad driver.
>
> I'm pretty much done with the simple switching module but it will be of
> no use if we cannot make the xpad module ignore G920 first.

I see that Simon's patch added:

XPAD_XBOXONE_VENDOR(0x046d),

to the xpad driver. Are you saying that we latch onto the controller
even without this addition?

Thanks.

-- 
Dmitry

Re: [Patch-V2 1/6] INPUT: xpad: Add minimal support for Logitech G920 Wheel

[Dmitry Torokhov]

On Thu, Jan 7, 2016 at 2:50 PM, Michal Malý
<madcatxster@devoid-pointer.net> wrote:
> On Wed, 2016-01-06 at 17:47 -0800, Dmitry Torokhov wrote:
>> On Wed, Jan 06, 2016 at 03:36:57PM +0100, Jiri Kosina wrote:
>> > On Mon, 4 Jan 2016, Benjamin Tissoires wrote:
>> >
>> > > Jiri, I *think* this commit still is in your next pull request
>> > > for
>> > > Linus. We might want to drop it before it hits Linus' tree.
>> >
>> > What exactly would be the reasoning for dropping it?
>>
>> It is wrong. Aside form the fact that IMO xpad.c is the wrong place
>> for
>> this code to be in, why are we waiting for the input device to be
>> opened by userspace before we do the switch instead of doing it
>> immediately?
>>
>
> Hi all,
>
> I have to disagree with the xpad driver being the wrong place to handle
> this. The xpad driver matches devices it should handle by interface
> class, subclass and protocol. When G920 first appears on the USB bus,
> it for all intents and purposes looks like a Xbox One controller so the
> xpad driver picks it up even if there is no G920-specific code in the
> driver. Unless there is a way how to blacklist certain idProduct
> values, the switch from XBone mode to HID mode will have to be done in
> the xpad driver.
>
> I'm pretty much done with the simple switching module but it will be of
> no use if we cannot make the xpad module ignore G920 first.

I see that Simon's patch added:

XPAD_XBOXONE_VENDOR(0x046d),

to the xpad driver. Are you saying that we latch onto the controller
even without this addition?

Thanks.

-- 
Dmitry
--
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-V2 1/6] INPUT: xpad: Add minimal support for Logitech G920 Wheel

[Michal Malý]

On Thu, 2016-01-07 at 14:53 -0800, Dmitry Torokhov wrote:
> On Thu, Jan 7, 2016 at 2:50 PM, Michal Malý
> <madcatxster@devoid-pointer.net> wrote:
> > On Wed, 2016-01-06 at 17:47 -0800, Dmitry Torokhov wrote:
> > > On Wed, Jan 06, 2016 at 03:36:57PM +0100, Jiri Kosina wrote:
> > > > On Mon, 4 Jan 2016, Benjamin Tissoires wrote:
> > > > 
> > > > > Jiri, I *think* this commit still is in your next pull
> > > > > request
> > > > > for
> > > > > Linus. We might want to drop it before it hits Linus' tree.
> > > > 
> > > > What exactly would be the reasoning for dropping it?
> > > 
> > > It is wrong. Aside form the fact that IMO xpad.c is the wrong
> > > place
> > > for
> > > this code to be in, why are we waiting for the input device to be
> > > opened by userspace before we do the switch instead of doing it
> > > immediately?
> > > 
> > 
> > Hi all,
> > 
> > I have to disagree with the xpad driver being the wrong place to
> > handle
> > this. The xpad driver matches devices it should handle by interface
> > class, subclass and protocol. When G920 first appears on the USB
> > bus,
> > it for all intents and purposes looks like a Xbox One controller so
> > the
> > xpad driver picks it up even if there is no G920-specific code in
> > the
> > driver. Unless there is a way how to blacklist certain idProduct
> > values, the switch from XBone mode to HID mode will have to be done
> > in
> > the xpad driver.
> > 
> > I'm pretty much done with the simple switching module but it will
> > be of
> > no use if we cannot make the xpad module ignore G920 first.
> 
> I see that Simon's patch added:
> 
> XPAD_XBOXONE_VENDOR(0x046d),
> 
> to the xpad driver. Are you saying that we latch onto the controller
> even without this addition?
> 
> Thanks.

Sorry, my bad, I missed that change in the patch. Handling the switch
elsewhere should be no problem then.

Michal

Re: [Patch-V2 1/6] INPUT: xpad: Add minimal support for Logitech G920 Wheel

[Michal Malý]

On Thu, 2016-01-07 at 14:53 -0800, Dmitry Torokhov wrote:
> On Thu, Jan 7, 2016 at 2:50 PM, Michal Malý
> <madcatxster@devoid-pointer.net> wrote:
> > On Wed, 2016-01-06 at 17:47 -0800, Dmitry Torokhov wrote:
> > > On Wed, Jan 06, 2016 at 03:36:57PM +0100, Jiri Kosina wrote:
> > > > On Mon, 4 Jan 2016, Benjamin Tissoires wrote:
> > > > 
> > > > > Jiri, I *think* this commit still is in your next pull
> > > > > request
> > > > > for
> > > > > Linus. We might want to drop it before it hits Linus' tree.
> > > > 
> > > > What exactly would be the reasoning for dropping it?
> > > 
> > > It is wrong. Aside form the fact that IMO xpad.c is the wrong
> > > place
> > > for
> > > this code to be in, why are we waiting for the input device to be
> > > opened by userspace before we do the switch instead of doing it
> > > immediately?
> > > 
> > 
> > Hi all,
> > 
> > I have to disagree with the xpad driver being the wrong place to
> > handle
> > this. The xpad driver matches devices it should handle by interface
> > class, subclass and protocol. When G920 first appears on the USB
> > bus,
> > it for all intents and purposes looks like a Xbox One controller so
> > the
> > xpad driver picks it up even if there is no G920-specific code in
> > the
> > driver. Unless there is a way how to blacklist certain idProduct
> > values, the switch from XBone mode to HID mode will have to be done
> > in
> > the xpad driver.
> > 
> > I'm pretty much done with the simple switching module but it will
> > be of
> > no use if we cannot make the xpad module ignore G920 first.
> 
> I see that Simon's patch added:
> 
> XPAD_XBOXONE_VENDOR(0x046d),
> 
> to the xpad driver. Are you saying that we latch onto the controller
> even without this addition?
> 
> Thanks.

Sorry, my bad, I missed that change in the patch. Handling the switch
elsewhere should be no problem then.

Michal
--
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-V2 1/6] INPUT: xpad: Add minimal support for Logitech G920 Wheel

[Jiri Kosina]

Alright, so what I am planning to do is to drop 27b9d5a254d ("INPUT: xpad: 
switch Logitech G920 Wheel into HID mode") from the for-4.5/logitech 
branch, while keeping the rest of the G920 support in, so that it 
immediately starts working once proper HID-mode switching is implemented.

If anyone has objections to this aproach, please speak up.

Thanks,

-- 
Jiri Kosina
SUSE Labs