linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/3] new driver for Valve Steam Controller
@ 2018-02-25 18:52 Rodrigo Rivas Costa
  2018-02-25 18:52 ` [PATCH v3 1/3] HID: add " Rodrigo Rivas Costa
                   ` (3 more replies)
  0 siblings, 4 replies; 7+ messages in thread
From: Rodrigo Rivas Costa @ 2018-02-25 18:52 UTC (permalink / raw)
  To: Jiri Kosina, Benjamin Tissoires, Pierre-Loup A. Griffais,
	Cameron Gutman, Clément VUCHENER, linux-kernel, linux-input
  Cc: Rodrigo Rivas Costa

This patchset implements a driver for Valve Steam Controller, based on a
reverse analysis by myself.

This is reroll v3, codenamed "lazy lizard". Changes from v2:
 * Use RCU to do the dynamic connec/disconnect of wireless devices. Please,
   to anybody that knows their way around RCU, review.
 * Remove entries in hid-quirks.c as they are no longer needed. This allows
   this module to be blacklisted without side effects.
 * Do not bypass the virtual keyboard/mouse HID devices to avoid breaking
   existing use cases (lizard mode). A user-space tool to do that is
   linked.
 * Fully separated axes for joystick and left-pad. As it happens, there are
   people with too many fingers.
 * Add fuzz values for left/right pad axes, they are a little wiggly.

Notable changes from patchset v1:
 * Remove references to USB. Now the interesting interfaces are selected by
   looking for the ones with feature reports.
 * Feature reports buffers are allocated with hid_alloc_report_buf().
 * Feature report length is checked, to avoid overflows in case of
   corrupt/malicius USB devices.
 * Resolution added to the ABS axes.
 * A lot of minor cleanups.

Rodrigo Rivas Costa (3):
  HID: add driver for Valve Steam Controller
  HID: steam: add serial number information.
  HID: steam: add battery device.

 drivers/hid/Kconfig     |   8 +
 drivers/hid/Makefile    |   1 +
 drivers/hid/hid-ids.h   |   4 +
 drivers/hid/hid-steam.c | 777 ++++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 790 insertions(+)
 create mode 100644 drivers/hid/hid-steam.c

-- 
2.16.2

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

* [PATCH v3 1/3] HID: add driver for Valve Steam Controller
  2018-02-25 18:52 [PATCH v3 0/3] new driver for Valve Steam Controller Rodrigo Rivas Costa
@ 2018-02-25 18:52 ` Rodrigo Rivas Costa
  2018-02-25 18:52 ` [PATCH v3 2/3] HID: steam: add serial number information Rodrigo Rivas Costa
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 7+ messages in thread
From: Rodrigo Rivas Costa @ 2018-02-25 18:52 UTC (permalink / raw)
  To: Jiri Kosina, Benjamin Tissoires, Pierre-Loup A. Griffais,
	Cameron Gutman, Clément VUCHENER, linux-kernel, linux-input
  Cc: Rodrigo Rivas Costa

There are two ways to connect the Steam Controller: directly to the USB
or with the USB wireless adapter.  Both methods are similar, but the
wireless adapter can connect up to 4 devices at the same time.

The wired device will appear as 3 interfaces: a virtual mouse, a virtual
keyboard and a custom HID device.

The wireless device will appear as 5 interfaces: a virtual keyboard and
4 custom HID devices, that will remain silent until a device is actually
connected.

The custom HID device has a report descriptor with all vendor specific
usages, so the hid-generic is not very useful. In a PC/SteamBox Valve
Steam Client provices a software translation by using direct USB access
and a creates a uinput virtual gamepad.

This driver was reverse engineered to provide direct kernel support in
case you cannot, or do not want to, use Valve Steam Client. It disables
the virtual keyboard and mouse, as they are not so useful when you have
a working gamepad.

Working: buttons, axes, pads, wireless connect/disconnect.

TO-DO: Battery, force-feedback, accelerometer/gyro, led, beeper...

Signed-off-by: Rodrigo Rivas Costa <rodrigorivascosta@gmail.com>
---
 drivers/hid/Kconfig     |   8 +
 drivers/hid/Makefile    |   1 +
 drivers/hid/hid-ids.h   |   4 +
 drivers/hid/hid-steam.c | 539 ++++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 552 insertions(+)
 create mode 100644 drivers/hid/hid-steam.c

diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index 19c499f5623d..6e80fbf04e03 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -823,6 +823,14 @@ config HID_SPEEDLINK
 	---help---
 	Support for Speedlink Vicious and Divine Cezanne mouse.
 
+config HID_STEAM
+	tristate "Steam Controller support"
+	depends on HID
+	---help---
+	Say Y here if you have a Steam Controller if you want to use it
+	without running the Steam Client. It supports both the wired and
+	the wireless adaptor.
+
 config HID_STEELSERIES
 	tristate "Steelseries SRW-S1 steering wheel support"
 	depends on HID
diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
index eb13b9e92d85..60a8abf84682 100644
--- a/drivers/hid/Makefile
+++ b/drivers/hid/Makefile
@@ -95,6 +95,7 @@ obj-$(CONFIG_HID_SAMSUNG)	+= hid-samsung.o
 obj-$(CONFIG_HID_SMARTJOYPLUS)	+= hid-sjoy.o
 obj-$(CONFIG_HID_SONY)		+= hid-sony.o
 obj-$(CONFIG_HID_SPEEDLINK)	+= hid-speedlink.o
+obj-$(CONFIG_HID_STEAM)		+= hid-steam.o
 obj-$(CONFIG_HID_STEELSERIES)	+= hid-steelseries.o
 obj-$(CONFIG_HID_SUNPLUS)	+= hid-sunplus.o
 obj-$(CONFIG_HID_GREENASIA)	+= hid-gaff.o
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 43ddcdfbd0da..be31a3c20818 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -988,6 +988,10 @@
 #define USB_VENDOR_ID_STANTUM_SITRONIX		0x1403
 #define USB_DEVICE_ID_MTP_SITRONIX		0x5001
 
+#define USB_VENDOR_ID_VALVE			0x28de
+#define USB_DEVICE_ID_STEAM_CONTROLLER		0x1102
+#define USB_DEVICE_ID_STEAM_CONTROLLER_WIRELESS	0x1142
+
 #define USB_VENDOR_ID_STEELSERIES	0x1038
 #define USB_DEVICE_ID_STEELSERIES_SRWS1	0x1410
 
diff --git a/drivers/hid/hid-steam.c b/drivers/hid/hid-steam.c
new file mode 100644
index 000000000000..59dbcb95f349
--- /dev/null
+++ b/drivers/hid/hid-steam.c
@@ -0,0 +1,539 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * HID driver for Valve Steam Controller
+ *
+ * Copyright (c) 2018 Rodrigo Rivas Costa <rodrigorivascosta@gmail.com>
+ *
+ * Supports both the wired and wireless interfaces.
+ *
+ * For additional functions, such as disabling the virtual mouse/keyboard or
+ * changing the right-pad margin you can use the user-space tool at:
+ *
+ *   https://github.com/rodrigorc/steamctrl
+ */
+
+#include <linux/device.h>
+#include <linux/input.h>
+#include <linux/hid.h>
+#include <linux/module.h>
+#include <linux/workqueue.h>
+#include "hid-ids.h"
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Rodrigo Rivas Costa <rodrigorivascosta@gmail.com>");
+
+#define STEAM_QUIRK_WIRELESS		BIT(0)
+
+/* Touch pads are 40 mm in diameter and 65535 units */
+#define STEAM_PAD_RESOLUTION 1638
+/* Trigger runs are about 5 mm and 256 units */
+#define STEAM_TRIGGER_RESOLUTION 51
+/* Joystick runs are about 5 mm and 256 units */
+#define STEAM_JOYSTICK_RESOLUTION 51
+
+#define STEAM_PAD_FUZZ 128
+
+struct steam_device {
+	spinlock_t lock;
+	struct hid_device *hdev;
+	struct input_dev __rcu *input;
+	unsigned long quirks;
+	struct work_struct work_connect;
+	bool connected;
+};
+
+static int steam_input_open(struct input_dev *dev)
+{
+	struct steam_device *steam = input_get_drvdata(dev);
+
+	return hid_hw_open(steam->hdev);
+}
+
+static void steam_input_close(struct input_dev *dev)
+{
+	struct steam_device *steam = input_get_drvdata(dev);
+
+	hid_hw_close(steam->hdev);
+}
+
+static int steam_register(struct steam_device *steam)
+{
+	struct hid_device *hdev = steam->hdev;
+	struct input_dev *input;
+	int ret;
+
+	rcu_read_lock();
+	input = rcu_dereference(steam->input);
+	rcu_read_unlock();
+	if (input) {
+		dbg_hid("%s: already connected\n", __func__);
+		return -ENODEV;
+	}
+
+	hid_info(hdev, "Steam Controller connected");
+
+	input = input_allocate_device();
+	if (!input)
+		return -ENOMEM;
+
+	input_set_drvdata(input, steam);
+	input->dev.parent = &hdev->dev;
+	input->open = steam_input_open;
+	input->close = steam_input_close;
+
+	input->name = (steam->quirks & STEAM_QUIRK_WIRELESS) ?
+		"Wireless Steam Controller" :
+		"Steam Controller";
+	input->phys = hdev->phys;
+	input->uniq = hdev->uniq;
+	input->id.bustype = hdev->bus;
+	input->id.vendor = hdev->vendor;
+	input->id.product = hdev->product;
+	input->id.version = hdev->version;
+
+	input_set_capability(input, EV_KEY, BTN_TR2);
+	input_set_capability(input, EV_KEY, BTN_TL2);
+	input_set_capability(input, EV_KEY, BTN_TR);
+	input_set_capability(input, EV_KEY, BTN_TL);
+	input_set_capability(input, EV_KEY, BTN_Y);
+	input_set_capability(input, EV_KEY, BTN_B);
+	input_set_capability(input, EV_KEY, BTN_X);
+	input_set_capability(input, EV_KEY, BTN_A);
+	input_set_capability(input, EV_KEY, BTN_SELECT);
+	input_set_capability(input, EV_KEY, BTN_MODE);
+	input_set_capability(input, EV_KEY, BTN_START);
+	input_set_capability(input, EV_KEY, BTN_GEAR_DOWN);
+	input_set_capability(input, EV_KEY, BTN_GEAR_UP);
+	input_set_capability(input, EV_KEY, BTN_THUMBR);
+	input_set_capability(input, EV_KEY, BTN_THUMBL);
+
+	input_set_abs_params(input, ABS_Z, 0, 255, 0, 0);
+	input_set_abs_params(input, ABS_RZ, 0, 255, 0, 0);
+	input_set_abs_params(input, ABS_X, -32767, 32767, 0, 0);
+	input_set_abs_params(input, ABS_Y, -32767, 32767, 0, 0);
+	input_set_abs_params(input, ABS_RX, -32767, 32767,
+			STEAM_PAD_FUZZ, 0);
+	input_set_abs_params(input, ABS_RY, -32767, 32767,
+			STEAM_PAD_FUZZ, 0);
+	input_set_abs_params(input, ABS_HAT1X, -32767, 32767,
+			STEAM_PAD_FUZZ, 0);
+	input_set_abs_params(input, ABS_HAT1Y, -32767, 32767,
+			STEAM_PAD_FUZZ, 0);
+	input_set_abs_params(input, ABS_HAT0X, -1, 1, 0, 0);
+	input_set_abs_params(input, ABS_HAT0Y, -1, 1, 0, 0);
+	input_abs_set_res(input, ABS_X, STEAM_JOYSTICK_RESOLUTION);
+	input_abs_set_res(input, ABS_Y, STEAM_JOYSTICK_RESOLUTION);
+	input_abs_set_res(input, ABS_RX, STEAM_PAD_RESOLUTION);
+	input_abs_set_res(input, ABS_RY, STEAM_PAD_RESOLUTION);
+	input_abs_set_res(input, ABS_HAT1X, STEAM_PAD_RESOLUTION);
+	input_abs_set_res(input, ABS_HAT1Y, STEAM_PAD_RESOLUTION);
+	input_abs_set_res(input, ABS_Z, STEAM_TRIGGER_RESOLUTION);
+	input_abs_set_res(input, ABS_RZ, STEAM_TRIGGER_RESOLUTION);
+
+	ret = input_register_device(input);
+	if (ret)
+		goto input_register_fail;
+
+	rcu_assign_pointer(steam->input, input);
+
+	return 0;
+
+input_register_fail:
+	input_free_device(input);
+	return ret;
+}
+
+static void steam_unregister(struct steam_device *steam)
+{
+	struct input_dev *input;
+
+	rcu_read_lock();
+	input = rcu_dereference(steam->input);
+	rcu_read_unlock();
+
+	if (input) {
+		RCU_INIT_POINTER(steam->input, NULL);
+		synchronize_rcu();
+		hid_info(steam->hdev, "Steam Controller disconnected");
+		input_unregister_device(input);
+	}
+}
+
+static void steam_work_connect_cb(struct work_struct *work)
+{
+	struct steam_device *steam = container_of(work, struct steam_device,
+							work_connect);
+	unsigned long flags;
+	bool connected;
+	int ret;
+
+	spin_lock_irqsave(&steam->lock, flags);
+	connected = steam->connected;
+	spin_unlock_irqrestore(&steam->lock, flags);
+
+	if (connected) {
+		ret = steam_register(steam);
+		if (ret) {
+			hid_err(steam->hdev,
+				"%s:steam_register failed with error %d\n",
+				__func__, ret);
+			return;
+		}
+	} else {
+		steam_unregister(steam);
+	}
+}
+
+static bool steam_is_valve_interface(struct hid_device *hdev)
+{
+	struct hid_report_enum *rep_enum;
+	struct hid_report *hreport;
+
+	/*
+	 * The wired device creates 3 interfaces:
+	 *  0: emulated mouse.
+	 *  1: emulated keyboard.
+	 *  2: the real game pad.
+	 * The wireless device creates 5 interfaces:
+	 *  0: emulated keyboard.
+	 *  1-4: slots where up to 4 real game pads will be connected to.
+	 * We know which one is the real gamepad interface because they are the
+	 * only ones with a feature report.
+	 */
+	rep_enum = &hdev->report_enum[HID_FEATURE_REPORT];
+	list_for_each_entry(hreport, &rep_enum->report_list, list) {
+		/* should we check hreport->id == 0? */
+		return true;
+	}
+	return false;
+}
+
+static int steam_probe(struct hid_device *hdev,
+				const struct hid_device_id *id)
+{
+	struct steam_device *steam;
+	int ret;
+
+	ret = hid_parse(hdev);
+	if (ret) {
+		hid_err(hdev,
+			"%s:parse of hid interface failed\n", __func__);
+		return ret;
+	}
+
+	/*
+	 * Connect the HID device so that Steam Controller keeps on working
+	 * without changes.
+	 */
+	ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
+	if (ret) {
+		hid_err(hdev,
+			"%s:hid_hw_start failed with error %d\n",
+			__func__, ret);
+		return ret;
+	}
+
+	if (!steam_is_valve_interface(hdev))
+		return 0;
+
+	/*
+	 * From this point on, if anything fails return 0 and ignores
+	 * the error, so that the default HID devices are still bound.
+	 */
+	steam = devm_kzalloc(&hdev->dev,
+			sizeof(struct steam_device), GFP_KERNEL);
+	if (!steam) {
+		ret = -ENOMEM;
+		goto mem_fail;
+	}
+
+	spin_lock_init(&steam->lock);
+	steam->hdev = hdev;
+	hid_set_drvdata(hdev, steam);
+	steam->quirks = id->driver_data;
+	INIT_WORK(&steam->work_connect, steam_work_connect_cb);
+
+	if (steam->quirks & STEAM_QUIRK_WIRELESS) {
+		ret = hid_hw_open(hdev);
+		if (ret) {
+			hid_err(hdev,
+				"%s:hid_hw_open for wireless\n",
+				__func__);
+			goto hid_hw_open_fail;
+		}
+		hid_info(hdev, "Steam wireless receiver connected");
+	} else {
+		ret = steam_register(steam);
+		if (ret) {
+			hid_err(hdev,
+				"%s:steam_register failed with error %d\n",
+				__func__, ret);
+			goto input_register_fail;
+		}
+	}
+
+	return 0;
+
+input_register_fail:
+hid_hw_open_fail:
+	cancel_work_sync(&steam->work_connect);
+	hid_set_drvdata(hdev, NULL);
+mem_fail:
+	hid_err(hdev, "%s: failed with error %d\n",
+			__func__, ret);
+	return 0;
+}
+
+static void steam_remove(struct hid_device *hdev)
+{
+	struct steam_device *steam = hid_get_drvdata(hdev);
+
+	if (steam && (steam->quirks & STEAM_QUIRK_WIRELESS)) {
+		hid_info(hdev, "Steam wireless receiver disconnected");
+		hid_hw_close(hdev);
+	}
+
+	hid_hw_stop(hdev);
+
+	if (steam) {
+		cancel_work_sync(&steam->work_connect);
+		steam_unregister(steam);
+		hid_set_drvdata(hdev, NULL);
+	}
+}
+
+static void steam_do_connect_event(struct steam_device *steam, bool connected)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&steam->lock, flags);
+	steam->connected = connected;
+	spin_unlock_irqrestore(&steam->lock, flags);
+
+	if (schedule_work(&steam->work_connect) == 0)
+		dbg_hid("%s: connected=%d event already queued\n",
+				__func__, connected);
+}
+
+/*
+ * The size for this message payload is 60.
+ * The known values are:
+ *  (* values are not sent through wireless)
+ *  (* accelerator/gyro is disabled by default)
+ *  Offset| Type  | Mapped to |Meaning
+ * -------+-------+-----------+--------------------------
+ *  4-7   | u32   | --        | sequence number
+ *  8-10  | 24bit | see below | buttons
+ *  11    | u8    | ABS_Z     | left trigger
+ *  12    | u8    | ABS_RZ    | right trigger
+ *  13-15 | --    | --        | always 0
+ *  16-17 | s16   | ABS_X     | X value
+ *  18-19 | s16   | ABS_Y     | Y value
+ *  20-21 | s16   | ABS_RX    | right-pad X value
+ *  22-23 | s16   | ABS_RY    | right-pad Y value
+ *  24-25 | s16   | --        | * left trigger
+ *  26-27 | s16   | --        | * right trigger
+ *  28-29 | s16   | --        | * accelerometer X value
+ *  30-31 | s16   | --        | * accelerometer Y value
+ *  32-33 | s16   | --        | * accelerometer Z value
+ *  34-35 | s16   | --        | gyro X value
+ *  36-36 | s16   | --        | gyro Y value
+ *  38-39 | s16   | --        | gyro Z value
+ *  40-41 | s16   | --        | quaternion W value
+ *  42-43 | s16   | --        | quaternion X value
+ *  44-45 | s16   | --        | quaternion Y value
+ *  46-47 | s16   | --        | quaternion Z value
+ *  48-49 | --    | --        | always 0
+ *  50-51 | s16   | --        | * left trigger (uncalibrated)
+ *  52-53 | s16   | --        | * right trigger (uncalibrated)
+ *  54-55 | s16   | --        | * joystick X value (uncalibrated)
+ *  56-57 | s16   | --        | * joystick Y value (uncalibrated)
+ *  58-59 | s16   | --        | * left-pad X value
+ *  60-61 | s16   | --        | * left-pad Y value
+ *  62-63 | u16   | --        | * battery voltage
+ *
+ * The buttons are:
+ *  Bit  | Mapped to  | Description
+ * ------+------------+--------------------------------
+ *  8.0  | BTN_TR2    | right trigger fully pressed
+ *  8.1  | BTN_TL2    | left trigger fully pressed
+ *  8.2  | BTN_TR     | right shoulder
+ *  8.3  | BTN_TL     | left shoulder
+ *  8.4  | BTN_Y      | button Y
+ *  8.5  | BTN_B      | button B
+ *  8.6  | BTN_X      | button X
+ *  8.7  | BTN_A      | button A
+ *  9.0  | -ABS_HAT0Y | lef-pad up
+ *  9.1  | +ABS_HAT0X | lef-pad right
+ *  9.2  | -ABS_HAT0X | lef-pad left
+ *  9.3  | +ABS_HAT0Y | lef-pad down
+ *  9.4  | BTN_SELECT | menu left
+ *  9.5  | BTN_MODE   | steam logo
+ *  9.6  | BTN_START  | menu right
+ *  9.7  | BTN_GEAR_DOWN | left back lever
+ * 10.0  | BTN_GEAR_UP   | right back lever
+ * 10.1  | --         | left-pad clicked
+ * 10.2  | BTN_THUMBR | right-pad clicked
+ * 10.3  | --         | left-pad touched (but see explanation below)
+ * 10.4  | --         | right-pad touched
+ * 10.5  | --         | unknown
+ * 10.6  | BTN_THUMBL | joystick clicked
+ * 10.7  | --         | lpad_and_joy
+ */
+
+static void steam_do_input_event(struct steam_device *steam,
+		struct input_dev *input, u8 *data)
+{
+	/* 24 bits of buttons */
+	u8 b8, b9, b10;
+	bool lpad_touched, lpad_and_joy;
+
+	b8 = data[8];
+	b9 = data[9];
+	b10 = data[10];
+
+	input_report_abs(input, ABS_Z, data[11]);
+	input_report_abs(input, ABS_RZ, data[12]);
+
+	/*
+	 * These two bits tells how to interpret the values X and Y.
+	 * lpad_and_joy tells that the joystick and the lpad are used at the
+	 * same time.
+	 * lpad_touched tells whether X/Y are to be read as lpad coord or
+	 * joystick values.
+	 * (lpad_touched || lpad_and_joy) tells if the lpad is really touched.
+	 */
+	lpad_touched = b10 & 0x08;
+	lpad_and_joy = b10 & 0x80;
+	input_report_abs(input, lpad_touched ? ABS_HAT1X : ABS_X,
+			(s16) le16_to_cpup((__le16 *)(data + 16)));
+	input_report_abs(input, lpad_touched ? ABS_HAT1Y : ABS_Y,
+			-(s16) le16_to_cpup((__le16 *)(data + 18)));
+	/* Check if joystick is centered */
+	if (lpad_touched && !lpad_and_joy) {
+		input_report_abs(input, ABS_X, 0);
+		input_report_abs(input, ABS_Y, 0);
+	}
+	/* Check if lpad is untouched */
+	if (!(lpad_touched || lpad_and_joy)) {
+		input_report_abs(input, ABS_HAT1X, 0);
+		input_report_abs(input, ABS_HAT1Y, 0);
+	}
+
+	input_report_abs(input, ABS_RX,
+			(s16) le16_to_cpup((__le16 *)(data + 20)));
+	input_report_abs(input, ABS_RY,
+			-(s16) le16_to_cpup((__le16 *)(data + 22)));
+
+	input_event(input, EV_KEY, BTN_TR2, !!(b8 & 0x01));
+	input_event(input, EV_KEY, BTN_TL2, !!(b8 & 0x02));
+	input_event(input, EV_KEY, BTN_TR, !!(b8 & 0x04));
+	input_event(input, EV_KEY, BTN_TL, !!(b8 & 0x08));
+	input_event(input, EV_KEY, BTN_Y, !!(b8 & 0x10));
+	input_event(input, EV_KEY, BTN_B, !!(b8 & 0x20));
+	input_event(input, EV_KEY, BTN_X, !!(b8 & 0x40));
+	input_event(input, EV_KEY, BTN_A, !!(b8 & 0x80));
+	input_event(input, EV_KEY, BTN_SELECT, !!(b9 & 0x10));
+	input_event(input, EV_KEY, BTN_MODE, !!(b9 & 0x20));
+	input_event(input, EV_KEY, BTN_START, !!(b9 & 0x40));
+	input_event(input, EV_KEY, BTN_GEAR_DOWN, !!(b9 & 0x80));
+	input_event(input, EV_KEY, BTN_GEAR_UP, !!(b10 & 0x01));
+	input_event(input, EV_KEY, BTN_THUMBR, !!(b10 & 0x04));
+	input_event(input, EV_KEY, BTN_THUMBL, !!(b10 & 0x40));
+
+	input_report_abs(input, ABS_HAT0X,
+			!!(b9 & 0x02) - !!(b9 & 0x04));
+	input_report_abs(input, ABS_HAT0Y,
+			!!(b9 & 0x08) - !!(b9 & 0x01));
+
+	input_sync(input);
+}
+
+static int steam_raw_event(struct hid_device *hdev,
+			struct hid_report *report, u8 *data,
+			int size)
+{
+	struct steam_device *steam = hid_get_drvdata(hdev);
+	struct input_dev *input;
+
+	if (!steam)
+		return 0;
+
+	/*
+	 * All messages are size=64, all values little-endian.
+	 * The format is:
+	 *  Offset| Meaning
+	 * -------+--------------------------------------------
+	 *  0-1   | always 0x01, 0x00, maybe protocol version?
+	 *  2     | type of message
+	 *  3     | length of the real payload (not checked)
+	 *  4-n   | payload data, depends on the type
+	 *
+	 * There are these known types of message:
+	 *  0x01: input data (60 bytes)
+	 *  0x03: wireless connect/disconnect (1 byte)
+	 *  0x04: battery status (11 bytes)
+	 */
+
+	if (size != 64 || data[0] != 1 || data[1] != 0)
+		return 0;
+
+	switch (data[2]) {
+	case 0x01:
+		rcu_read_lock();
+		input = rcu_dereference(steam->input);
+		if (likely(input)) {
+			steam_do_input_event(steam, input, data);
+		} else {
+			dbg_hid("%s: input data without connect event\n",
+					__func__);
+			steam_do_connect_event(steam, true);
+		}
+		rcu_read_unlock();
+		break;
+	case 0x03:
+		/*
+		 * The payload of this event is a single byte:
+		 *  0x01: disconnected.
+		 *  0x02: connected.
+		 */
+		switch (data[4]) {
+		case 0x01:
+			steam_do_connect_event(steam, false);
+			break;
+		case 0x02:
+			steam_do_connect_event(steam, true);
+			break;
+		}
+		break;
+	case 0x04:
+		/* TODO battery status */
+		break;
+	}
+	return 0;
+}
+
+static const struct hid_device_id steam_controllers[] = {
+	{ /* Wired Steam Controller */
+	  HID_USB_DEVICE(USB_VENDOR_ID_VALVE,
+		USB_DEVICE_ID_STEAM_CONTROLLER)
+	},
+	{ /* Wireless Steam Controller */
+	  HID_USB_DEVICE(USB_VENDOR_ID_VALVE,
+		USB_DEVICE_ID_STEAM_CONTROLLER_WIRELESS),
+	  .driver_data = STEAM_QUIRK_WIRELESS
+	},
+	{}
+};
+
+MODULE_DEVICE_TABLE(hid, steam_controllers);
+
+static struct hid_driver steam_controller_driver = {
+	.name = "hid-steam",
+	.id_table = steam_controllers,
+	.probe = steam_probe,
+	.remove = steam_remove,
+	.raw_event = steam_raw_event,
+};
+
+module_hid_driver(steam_controller_driver);
-- 
2.16.2

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

* [PATCH v3 2/3] HID: steam: add serial number information.
  2018-02-25 18:52 [PATCH v3 0/3] new driver for Valve Steam Controller Rodrigo Rivas Costa
  2018-02-25 18:52 ` [PATCH v3 1/3] HID: add " Rodrigo Rivas Costa
@ 2018-02-25 18:52 ` Rodrigo Rivas Costa
  2018-02-25 18:52 ` [PATCH v3 3/3] HID: steam: add battery device Rodrigo Rivas Costa
  2018-02-26  9:50 ` [PATCH v3 0/3] new driver for Valve Steam Controller Benjamin Tissoires
  3 siblings, 0 replies; 7+ messages in thread
From: Rodrigo Rivas Costa @ 2018-02-25 18:52 UTC (permalink / raw)
  To: Jiri Kosina, Benjamin Tissoires, Pierre-Loup A. Griffais,
	Cameron Gutman, Clément VUCHENER, linux-kernel, linux-input
  Cc: Rodrigo Rivas Costa

This device has a feature report to send and receive commands.
Use it to get the serial number and set the device's uniq value.

Signed-off-by: Rodrigo Rivas Costa <rodrigorivascosta@gmail.com>
---
 drivers/hid/hid-steam.c | 105 ++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 102 insertions(+), 3 deletions(-)

diff --git a/drivers/hid/hid-steam.c b/drivers/hid/hid-steam.c
index 59dbcb95f349..405fae2d2e4b 100644
--- a/drivers/hid/hid-steam.c
+++ b/drivers/hid/hid-steam.c
@@ -17,6 +17,8 @@
 #include <linux/hid.h>
 #include <linux/module.h>
 #include <linux/workqueue.h>
+#include <linux/delay.h>
+#include <linux/rcupdate.h>
 #include "hid-ids.h"
 
 MODULE_LICENSE("GPL");
@@ -40,8 +42,99 @@ struct steam_device {
 	unsigned long quirks;
 	struct work_struct work_connect;
 	bool connected;
+	char serial_no[11];
 };
 
+static int steam_recv_report(struct steam_device *steam,
+		u8 *data, int size)
+{
+	struct hid_report *r;
+	u8 *buf;
+	int ret;
+
+	r = steam->hdev->report_enum[HID_FEATURE_REPORT].report_id_hash[0];
+	if (hid_report_len(r) < 64)
+		return -EINVAL;
+	buf = hid_alloc_report_buf(r, GFP_KERNEL);
+	if (!buf)
+		return -ENOMEM;
+
+	/*
+	 * The report ID is always 0, so strip the first byte from the output.
+	 * hid_report_len() is not counting the report ID, so +1 to the length
+	 * or else we get a EOVERFLOW. We are safe from a buffer overflow
+	 * because hid_alloc_report_buf() allocates +7 bytes.
+	 */
+	ret = hid_hw_raw_request(steam->hdev, 0x00,
+			buf, hid_report_len(r) + 1,
+			HID_FEATURE_REPORT, HID_REQ_GET_REPORT);
+	if (ret > 0)
+		memcpy(data, buf + 1, min(size, ret - 1));
+	kfree(buf);
+	return ret;
+}
+
+static int steam_send_report(struct steam_device *steam,
+		u8 *cmd, int size)
+{
+	struct hid_report *r;
+	u8 *buf;
+	int retry;
+	int ret;
+
+	r = steam->hdev->report_enum[HID_FEATURE_REPORT].report_id_hash[0];
+	if (hid_report_len(r) < 64)
+		return -EINVAL;
+	buf = hid_alloc_report_buf(r, GFP_KERNEL);
+	if (!buf)
+		return -ENOMEM;
+
+	/* The report ID is always 0 */
+	memcpy(buf + 1, cmd, size);
+
+	/*
+	 * Sometimes the wireless controller fails with EPIPE
+	 * when sending a feature report.
+	 * Doing a HID_REQ_GET_REPORT and waiting for a while
+	 * seems to fix that.
+	 */
+	for (retry = 0; retry < 10; ++retry) {
+		ret = hid_hw_raw_request(steam->hdev, 0,
+				buf, size + 1,
+				HID_FEATURE_REPORT, HID_REQ_SET_REPORT);
+		if (ret != -EPIPE)
+			break;
+		steam_recv_report(steam, NULL, 0);
+		msleep(50);
+	}
+	kfree(buf);
+	if (ret < 0)
+		hid_err(steam->hdev, "%s: error %d (%*ph)\n", __func__,
+				ret, size, cmd);
+	return ret;
+}
+
+static int steam_get_serial(struct steam_device *steam)
+{
+	/*
+	 * Send: 0xae 0x15 0x01
+	 * Recv: 0xae 0x15 0x01 serialnumber (10 chars)
+	 */
+	int ret;
+	u8 cmd[] = {0xae, 0x15, 0x01};
+	u8 reply[14];
+
+	ret = steam_send_report(steam, cmd, sizeof(cmd));
+	if (ret < 0)
+		return ret;
+	ret = steam_recv_report(steam, reply, sizeof(reply));
+	if (ret < 0)
+		return ret;
+	reply[13] = 0;
+	strcpy(steam->serial_no, reply + 3);
+	return 0;
+}
+
 static int steam_input_open(struct input_dev *dev)
 {
 	struct steam_device *steam = input_get_drvdata(dev);
@@ -70,7 +163,12 @@ static int steam_register(struct steam_device *steam)
 		return -ENODEV;
 	}
 
-	hid_info(hdev, "Steam Controller connected");
+	ret = steam_get_serial(steam);
+	if (ret)
+		return ret;
+
+	hid_info(hdev, "Steam Controller '%s' connected",
+			steam->serial_no);
 
 	input = input_allocate_device();
 	if (!input)
@@ -85,7 +183,7 @@ static int steam_register(struct steam_device *steam)
 		"Wireless Steam Controller" :
 		"Steam Controller";
 	input->phys = hdev->phys;
-	input->uniq = hdev->uniq;
+	input->uniq = steam->serial_no;
 	input->id.bustype = hdev->bus;
 	input->id.vendor = hdev->vendor;
 	input->id.product = hdev->product;
@@ -154,7 +252,8 @@ static void steam_unregister(struct steam_device *steam)
 	if (input) {
 		RCU_INIT_POINTER(steam->input, NULL);
 		synchronize_rcu();
-		hid_info(steam->hdev, "Steam Controller disconnected");
+		hid_info(steam->hdev, "Steam Controller '%s' disconnected",
+				steam->serial_no);
 		input_unregister_device(input);
 	}
 }
-- 
2.16.2

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

* [PATCH v3 3/3] HID: steam: add battery device.
  2018-02-25 18:52 [PATCH v3 0/3] new driver for Valve Steam Controller Rodrigo Rivas Costa
  2018-02-25 18:52 ` [PATCH v3 1/3] HID: add " Rodrigo Rivas Costa
  2018-02-25 18:52 ` [PATCH v3 2/3] HID: steam: add serial number information Rodrigo Rivas Costa
@ 2018-02-25 18:52 ` Rodrigo Rivas Costa
  2018-02-26  9:50 ` [PATCH v3 0/3] new driver for Valve Steam Controller Benjamin Tissoires
  3 siblings, 0 replies; 7+ messages in thread
From: Rodrigo Rivas Costa @ 2018-02-25 18:52 UTC (permalink / raw)
  To: Jiri Kosina, Benjamin Tissoires, Pierre-Loup A. Griffais,
	Cameron Gutman, Clément VUCHENER, linux-kernel, linux-input
  Cc: Rodrigo Rivas Costa

The wireless Steam Controller is battery operated, so add the battery
device and power information.

Signed-off-by: Rodrigo Rivas Costa <rodrigorivascosta@gmail.com>
---
 drivers/hid/hid-steam.c | 141 +++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 140 insertions(+), 1 deletion(-)

diff --git a/drivers/hid/hid-steam.c b/drivers/hid/hid-steam.c
index 405fae2d2e4b..6af828ecafe9 100644
--- a/drivers/hid/hid-steam.c
+++ b/drivers/hid/hid-steam.c
@@ -19,6 +19,7 @@
 #include <linux/workqueue.h>
 #include <linux/delay.h>
 #include <linux/rcupdate.h>
+#include <linux/power_supply.h>
 #include "hid-ids.h"
 
 MODULE_LICENSE("GPL");
@@ -43,6 +44,10 @@ struct steam_device {
 	struct work_struct work_connect;
 	bool connected;
 	char serial_no[11];
+	struct power_supply_desc battery_desc;
+	struct power_supply __rcu *battery;
+	u8 battery_charge;
+	u16 voltage;
 };
 
 static int steam_recv_report(struct steam_device *steam,
@@ -149,6 +154,85 @@ static void steam_input_close(struct input_dev *dev)
 	hid_hw_close(steam->hdev);
 }
 
+static enum power_supply_property steam_battery_props[] = {
+	POWER_SUPPLY_PROP_PRESENT,
+	POWER_SUPPLY_PROP_SCOPE,
+	POWER_SUPPLY_PROP_VOLTAGE_NOW,
+	POWER_SUPPLY_PROP_CAPACITY,
+};
+
+static int steam_battery_get_property(struct power_supply *psy,
+				enum power_supply_property psp,
+				union power_supply_propval *val)
+{
+	struct steam_device *steam = power_supply_get_drvdata(psy);
+	unsigned long flags;
+	s16 volts;
+	u8 batt;
+	int ret = 0;
+
+	spin_lock_irqsave(&steam->lock, flags);
+	volts = steam->voltage;
+	batt = steam->battery_charge;
+	spin_unlock_irqrestore(&steam->lock, flags);
+
+	switch (psp) {
+	case POWER_SUPPLY_PROP_PRESENT:
+		val->intval = 1;
+		break;
+	case POWER_SUPPLY_PROP_SCOPE:
+		val->intval = POWER_SUPPLY_SCOPE_DEVICE;
+		break;
+	case POWER_SUPPLY_PROP_VOLTAGE_NOW:
+		val->intval = volts * 1000; /* mV -> uV */
+		break;
+	case POWER_SUPPLY_PROP_CAPACITY:
+		val->intval = batt;
+		break;
+	default:
+		ret = -EINVAL;
+		break;
+	}
+	return ret;
+}
+
+static int steam_battery_register(struct steam_device *steam)
+{
+	struct power_supply *battery;
+	struct power_supply_config battery_cfg = { .drv_data = steam, };
+	unsigned long flags;
+	int ret;
+
+	steam->battery_desc.type = POWER_SUPPLY_TYPE_BATTERY;
+	steam->battery_desc.properties = steam_battery_props;
+	steam->battery_desc.num_properties = ARRAY_SIZE(steam_battery_props);
+	steam->battery_desc.get_property = steam_battery_get_property;
+	steam->battery_desc.name = devm_kasprintf(&steam->hdev->dev,
+			GFP_KERNEL, "steam-controller-%s-battery",
+			steam->serial_no);
+	if (!steam->battery_desc.name)
+		return -ENOMEM;
+
+	/* avoid the warning of 0% battery while waiting for the first info */
+	spin_lock_irqsave(&steam->lock, flags);
+	steam->voltage = 3000;
+	steam->battery_charge = 100;
+	spin_unlock_irqrestore(&steam->lock, flags);
+
+	battery = power_supply_register(&steam->hdev->dev,
+			&steam->battery_desc, &battery_cfg);
+	if (IS_ERR(battery)) {
+		ret = PTR_ERR(battery);
+		hid_err(steam->hdev,
+				"%s:power_supply_register failed with error %d\n",
+				__func__, ret);
+		return ret;
+	}
+	rcu_assign_pointer(steam->battery, battery);
+	power_supply_powers(battery, &steam->hdev->dev);
+	return 0;
+}
+
 static int steam_register(struct steam_device *steam)
 {
 	struct hid_device *hdev = steam->hdev;
@@ -234,6 +318,10 @@ static int steam_register(struct steam_device *steam)
 
 	rcu_assign_pointer(steam->input, input);
 
+	/* ignore battery errors, we can live without it */
+	if (steam->quirks & STEAM_QUIRK_WIRELESS)
+		steam_battery_register(steam);
+
 	return 0;
 
 input_register_fail:
@@ -244,11 +332,18 @@ static int steam_register(struct steam_device *steam)
 static void steam_unregister(struct steam_device *steam)
 {
 	struct input_dev *input;
+	struct power_supply *battery;
 
 	rcu_read_lock();
 	input = rcu_dereference(steam->input);
+	battery = rcu_dereference(steam->battery);
 	rcu_read_unlock();
 
+	if (battery) {
+		RCU_INIT_POINTER(steam->battery, NULL);
+		synchronize_rcu();
+		power_supply_unregister(battery);
+	}
 	if (input) {
 		RCU_INIT_POINTER(steam->input, NULL);
 		synchronize_rcu();
@@ -548,12 +643,44 @@ static void steam_do_input_event(struct steam_device *steam,
 	input_sync(input);
 }
 
+/*
+ * The size for this message payload is 11.
+ * The known values are:
+ *  Offset| Type  | Meaning
+ * -------+-------+---------------------------
+ *  4-7   | u32   | sequence number
+ *  8-11  | --    | always 0
+ *  12-13 | u16   | voltage (mV)
+ *  14    | u8    | battery percent
+ */
+static void steam_do_battery_event(struct steam_device *steam,
+		struct power_supply *battery, u8 *data)
+{
+	unsigned long flags;
+
+	s16 volts = le16_to_cpup((__le16 *)(data + 12));
+	u8 batt = data[14];
+
+	/* Creating the battery may have failed */
+	rcu_read_lock();
+	battery = rcu_dereference(steam->battery);
+	if (likely(battery)) {
+		spin_lock_irqsave(&steam->lock, flags);
+		steam->voltage = volts;
+		steam->battery_charge = batt;
+		spin_unlock_irqrestore(&steam->lock, flags);
+		power_supply_changed(battery);
+	}
+	rcu_read_unlock();
+}
+
 static int steam_raw_event(struct hid_device *hdev,
 			struct hid_report *report, u8 *data,
 			int size)
 {
 	struct steam_device *steam = hid_get_drvdata(hdev);
 	struct input_dev *input;
+	struct power_supply *battery;
 
 	if (!steam)
 		return 0;
@@ -606,7 +733,19 @@ static int steam_raw_event(struct hid_device *hdev,
 		}
 		break;
 	case 0x04:
-		/* TODO battery status */
+		if (steam->quirks & STEAM_QUIRK_WIRELESS) {
+			rcu_read_lock();
+			battery = rcu_dereference(steam->battery);
+			if (likely(battery)) {
+				steam_do_battery_event(steam, battery, data);
+			} else {
+				dbg_hid(
+				  "%s: battery data without connect event\n",
+				  __func__);
+				steam_do_connect_event(steam, true);
+			}
+			rcu_read_unlock();
+		}
 		break;
 	}
 	return 0;
-- 
2.16.2

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

* Re: [PATCH v3 0/3] new driver for Valve Steam Controller
  2018-02-25 18:52 [PATCH v3 0/3] new driver for Valve Steam Controller Rodrigo Rivas Costa
                   ` (2 preceding siblings ...)
  2018-02-25 18:52 ` [PATCH v3 3/3] HID: steam: add battery device Rodrigo Rivas Costa
@ 2018-02-26  9:50 ` Benjamin Tissoires
  2018-02-26 11:24   ` Clément VUCHENER
  3 siblings, 1 reply; 7+ messages in thread
From: Benjamin Tissoires @ 2018-02-26  9:50 UTC (permalink / raw)
  To: Rodrigo Rivas Costa
  Cc: Jiri Kosina, Pierre-Loup A. Griffais, Cameron Gutman,
	Clément VUCHENER, lkml, linux-input

Hi Rodrigo,

On Sun, Feb 25, 2018 at 7:52 PM, Rodrigo Rivas Costa
<rodrigorivascosta@gmail.com> wrote:
> This patchset implements a driver for Valve Steam Controller, based on a
> reverse analysis by myself.

To me, the code looks OK now. I haven't got the time to do a better
review, so giving my:
Acked-by: Bnejamin Tissoires <benjamin.tissoires@redhat.com>

However, before we include it, I'd like to have the ACK from
Pierre-Loup and Clément. They should be more qualified than me to say
if this will interfere with the official Steam client (I think we are
good now, but that's just my opinion).

Cheers,
Benjamin

>
> This is reroll v3, codenamed "lazy lizard". Changes from v2:
>  * Use RCU to do the dynamic connec/disconnect of wireless devices. Please,
>    to anybody that knows their way around RCU, review.
>  * Remove entries in hid-quirks.c as they are no longer needed. This allows
>    this module to be blacklisted without side effects.
>  * Do not bypass the virtual keyboard/mouse HID devices to avoid breaking
>    existing use cases (lizard mode). A user-space tool to do that is
>    linked.
>  * Fully separated axes for joystick and left-pad. As it happens, there are
>    people with too many fingers.
>  * Add fuzz values for left/right pad axes, they are a little wiggly.
>
> Notable changes from patchset v1:
>  * Remove references to USB. Now the interesting interfaces are selected by
>    looking for the ones with feature reports.
>  * Feature reports buffers are allocated with hid_alloc_report_buf().
>  * Feature report length is checked, to avoid overflows in case of
>    corrupt/malicius USB devices.
>  * Resolution added to the ABS axes.
>  * A lot of minor cleanups.
>
> Rodrigo Rivas Costa (3):
>   HID: add driver for Valve Steam Controller
>   HID: steam: add serial number information.
>   HID: steam: add battery device.
>
>  drivers/hid/Kconfig     |   8 +
>  drivers/hid/Makefile    |   1 +
>  drivers/hid/hid-ids.h   |   4 +
>  drivers/hid/hid-steam.c | 777 ++++++++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 790 insertions(+)
>  create mode 100644 drivers/hid/hid-steam.c
>
> --
> 2.16.2
>

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

* Re: [PATCH v3 0/3] new driver for Valve Steam Controller
  2018-02-26  9:50 ` [PATCH v3 0/3] new driver for Valve Steam Controller Benjamin Tissoires
@ 2018-02-26 11:24   ` Clément VUCHENER
  2018-02-26 14:38     ` Rodrigo Rivas Costa
  0 siblings, 1 reply; 7+ messages in thread
From: Clément VUCHENER @ 2018-02-26 11:24 UTC (permalink / raw)
  To: Benjamin Tissoires
  Cc: Rodrigo Rivas Costa, Jiri Kosina, Pierre-Loup A. Griffais,
	Cameron Gutman, lkml, linux-input

2018-02-26 10:50 GMT+01:00 Benjamin Tissoires <benjamin.tissoires@redhat.com>:
> Hi Rodrigo,
>
> On Sun, Feb 25, 2018 at 7:52 PM, Rodrigo Rivas Costa
> <rodrigorivascosta@gmail.com> wrote:
>> This patchset implements a driver for Valve Steam Controller, based on a
>> reverse analysis by myself.
>
> To me, the code looks OK now. I haven't got the time to do a better
> review, so giving my:
> Acked-by: Bnejamin Tissoires <benjamin.tissoires@redhat.com>
>
> However, before we include it, I'd like to have the ACK from
> Pierre-Loup and Clément. They should be more qualified than me to say
> if this will interfere with the official Steam client (I think we are
> good now, but that's just my opinion).

I am not qualified to speak about the steam client, it has been a long
time since I have a look at how it uses the controller and I think it
changed a lot.

I checked the code and I think you are not handling wireless
connection correctly. If a wireless controller is already connected
when the driver is loaded, you will not receive a connection event and
forget to register the controller. You can send a 0xb4 request with
empty parameters (0xb4, 0x00, ... ) to force the receiver to send its
connection status (as the type 3 event you are already parsing). Since
user-space program may also send this request, you should make sure
the driver works if it receives a (dis)connected event when it is
already (dis)connected.

>
> Cheers,
> Benjamin
>
>>
>> This is reroll v3, codenamed "lazy lizard". Changes from v2:
>>  * Use RCU to do the dynamic connec/disconnect of wireless devices. Please,
>>    to anybody that knows their way around RCU, review.
>>  * Remove entries in hid-quirks.c as they are no longer needed. This allows
>>    this module to be blacklisted without side effects.
>>  * Do not bypass the virtual keyboard/mouse HID devices to avoid breaking
>>    existing use cases (lizard mode). A user-space tool to do that is
>>    linked.
>>  * Fully separated axes for joystick and left-pad. As it happens, there are
>>    people with too many fingers.
>>  * Add fuzz values for left/right pad axes, they are a little wiggly.
>>
>> Notable changes from patchset v1:
>>  * Remove references to USB. Now the interesting interfaces are selected by
>>    looking for the ones with feature reports.
>>  * Feature reports buffers are allocated with hid_alloc_report_buf().
>>  * Feature report length is checked, to avoid overflows in case of
>>    corrupt/malicius USB devices.
>>  * Resolution added to the ABS axes.
>>  * A lot of minor cleanups.
>>
>> Rodrigo Rivas Costa (3):
>>   HID: add driver for Valve Steam Controller
>>   HID: steam: add serial number information.
>>   HID: steam: add battery device.
>>
>>  drivers/hid/Kconfig     |   8 +
>>  drivers/hid/Makefile    |   1 +
>>  drivers/hid/hid-ids.h   |   4 +
>>  drivers/hid/hid-steam.c | 777 ++++++++++++++++++++++++++++++++++++++++++++++++
>>  4 files changed, 790 insertions(+)
>>  create mode 100644 drivers/hid/hid-steam.c
>>
>> --
>> 2.16.2
>>

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

* Re: [PATCH v3 0/3] new driver for Valve Steam Controller
  2018-02-26 11:24   ` Clément VUCHENER
@ 2018-02-26 14:38     ` Rodrigo Rivas Costa
  0 siblings, 0 replies; 7+ messages in thread
From: Rodrigo Rivas Costa @ 2018-02-26 14:38 UTC (permalink / raw)
  To: Clément VUCHENER
  Cc: Benjamin Tissoires, Jiri Kosina, Pierre-Loup A. Griffais,
	Cameron Gutman, lkml, linux-input

On Mon, Feb 26, 2018 at 12:24:21PM +0100, Clément VUCHENER wrote:
> 2018-02-26 10:50 GMT+01:00 Benjamin Tissoires <benjamin.tissoires@redhat.com>:
> > Hi Rodrigo,
> >
> > On Sun, Feb 25, 2018 at 7:52 PM, Rodrigo Rivas Costa
> > <rodrigorivascosta@gmail.com> wrote:
> >> This patchset implements a driver for Valve Steam Controller, based on a
> >> reverse analysis by myself.
> >
> > To me, the code looks OK now. I haven't got the time to do a better
> > review, so giving my:
> > Acked-by: Bnejamin Tissoires <benjamin.tissoires@redhat.com>
> >
> > However, before we include it, I'd like to have the ACK from
> > Pierre-Loup and Clément. They should be more qualified than me to say
> > if this will interfere with the official Steam client (I think we are
> > good now, but that's just my opinion).
> 
> I am not qualified to speak about the steam client, it has been a long
> time since I have a look at how it uses the controller and I think it
> changed a lot.
> 
> I checked the code and I think you are not handling wireless
> connection correctly. If a wireless controller is already connected
> when the driver is loaded, you will not receive a connection event and
> forget to register the controller. You can send a 0xb4 request with
> empty parameters (0xb4, 0x00, ... ) to force the receiver to send its
> connection status (as the type 3 event you are already parsing). Since
> user-space program may also send this request, you should make sure
> the driver works if it receives a (dis)connected event when it is
> already (dis)connected.

You are partially right. If a wireless controller is already connected
when the driver is loaded, it will not create the input device. But
eventually it will receive a input or battery report. Then it will know
that it is not yet registered and it will do it. Since the wireless
device sends one battery status per second, that is not a big deal. That
is in patch 3/3, though, without that you will have to touch a button to
get the connection.

The redundant disconnection is handled properly, I think. It will just
call steam_unregister() several times, but that will do nothing if the
input device/battery is not created.

Anyway, I agree that the 0xb4 request is a much better solution, I
didn't know about that command. I'll add it to the driver and check it
from user-land to see if it breaks anything.

Thanks for the tip!
Rodrigo

> 
> >
> > Cheers,
> > Benjamin
> >
> >>
> >> This is reroll v3, codenamed "lazy lizard". Changes from v2:
> >>  * Use RCU to do the dynamic connec/disconnect of wireless devices. Please,
> >>    to anybody that knows their way around RCU, review.
> >>  * Remove entries in hid-quirks.c as they are no longer needed. This allows
> >>    this module to be blacklisted without side effects.
> >>  * Do not bypass the virtual keyboard/mouse HID devices to avoid breaking
> >>    existing use cases (lizard mode). A user-space tool to do that is
> >>    linked.
> >>  * Fully separated axes for joystick and left-pad. As it happens, there are
> >>    people with too many fingers.
> >>  * Add fuzz values for left/right pad axes, they are a little wiggly.
> >>
> >> Notable changes from patchset v1:
> >>  * Remove references to USB. Now the interesting interfaces are selected by
> >>    looking for the ones with feature reports.
> >>  * Feature reports buffers are allocated with hid_alloc_report_buf().
> >>  * Feature report length is checked, to avoid overflows in case of
> >>    corrupt/malicius USB devices.
> >>  * Resolution added to the ABS axes.
> >>  * A lot of minor cleanups.
> >>
> >> Rodrigo Rivas Costa (3):
> >>   HID: add driver for Valve Steam Controller
> >>   HID: steam: add serial number information.
> >>   HID: steam: add battery device.
> >>
> >>  drivers/hid/Kconfig     |   8 +
> >>  drivers/hid/Makefile    |   1 +
> >>  drivers/hid/hid-ids.h   |   4 +
> >>  drivers/hid/hid-steam.c | 777 ++++++++++++++++++++++++++++++++++++++++++++++++
> >>  4 files changed, 790 insertions(+)
> >>  create mode 100644 drivers/hid/hid-steam.c
> >>
> >> --
> >> 2.16.2
> >>

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

end of thread, other threads:[~2018-02-26 14:38 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-02-25 18:52 [PATCH v3 0/3] new driver for Valve Steam Controller Rodrigo Rivas Costa
2018-02-25 18:52 ` [PATCH v3 1/3] HID: add " Rodrigo Rivas Costa
2018-02-25 18:52 ` [PATCH v3 2/3] HID: steam: add serial number information Rodrigo Rivas Costa
2018-02-25 18:52 ` [PATCH v3 3/3] HID: steam: add battery device Rodrigo Rivas Costa
2018-02-26  9:50 ` [PATCH v3 0/3] new driver for Valve Steam Controller Benjamin Tissoires
2018-02-26 11:24   ` Clément VUCHENER
2018-02-26 14:38     ` Rodrigo Rivas Costa

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