2021-05-02 23:30:21

by Emmanuel Gil Peyrot

[permalink] [raw]
Subject: [PATCH 0/4] RFC: HID: wiiu-drc: Add a driver for the Wii U gamepad

This driver is for the DRC (wireless gamepad) when plugged to the DRH of
the Wii U, a chip exposing it as a USB device.

I tried to use this driver on master over usbip on my laptop, but usbip
disconnects the device right after the driver created the
/dev/input/event* files, so instead I have only tested this driver on
the 4.19 branch of the linux-wiiu[1] downstream.

Other than that, pretty much all of the HID parts of the gamepad work,
it’s only missing microphone, camera and NFC input now but those are
mostly standard (read require quirks) and pertain to other subsystems,
so I felt like this can be upstreamed already.

I’ve still put the RFC tag on this pull request because of two known
problems in these patches (annotated with TODOs in the code):
- The magnetometer is exposed using non-sensical ABS_* values, it seems
most (all?) magnetometers are exposed in the iio subsystem instead,
should I go the same way despite it clearly being part of the same HID
device?
- The battery number is currently based on a static int being
incremented every time a new gamepad is “plugged in”, while I’d prefer
to reuse the interface number for that.

Thanks for your guidance. :)

[1] https://gitlab.com/linux-wiiu/linux-wiiu

Ash Logan (1):
HID: wiiu-drc: Add a driver for this gamepad

Emmanuel Gil Peyrot (3):
HID: wiiu-drc: Implement touch reports
HID: wiiu-drc: Add accelerometer, gyroscope and magnetometer readings
HID: wiiu-drc: Add battery reporting

drivers/hid/Kconfig | 7 +
drivers/hid/Makefile | 1 +
drivers/hid/hid-ids.h | 1 +
drivers/hid/hid-quirks.c | 3 +
drivers/hid/hid-wiiu-drc.c | 522 +++++++++++++++++++++++++++++++++++++
5 files changed, 534 insertions(+)
create mode 100644 drivers/hid/hid-wiiu-drc.c

--
2.31.1


2021-05-02 23:30:23

by Emmanuel Gil Peyrot

[permalink] [raw]
Subject: [PATCH 1/4] HID: wiiu-drc: Add a driver for this gamepad

From: Ash Logan <[email protected]>

This driver is for the DRC (wireless gamepad) when plugged to the DRH of
the Wii U, a chip exposing it as a USB device.

This first patch exposes the buttons and sticks of this device, so that
it can act as a plain game controller.

Signed-off-by: Ash Logan <[email protected]>
Signed-off-by: Emmanuel Gil Peyrot <[email protected]>
---
drivers/hid/Kconfig | 7 +
drivers/hid/Makefile | 1 +
drivers/hid/hid-ids.h | 1 +
drivers/hid/hid-quirks.c | 3 +
drivers/hid/hid-wiiu-drc.c | 270 +++++++++++++++++++++++++++++++++++++
5 files changed, 282 insertions(+)
create mode 100644 drivers/hid/hid-wiiu-drc.c

diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index 4bf263c2d61a..01116c315459 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -1105,6 +1105,13 @@ config HID_WACOM
To compile this driver as a module, choose M here: the
module will be called wacom.

+config HID_WIIU_DRC
+ tristate "Nintendo Wii U gamepad over internal DRH"
+ depends on HID
+ help
+ Support for the Wii U gamepad, when connected with the Wii U’s
+ internal DRH chip.
+
config HID_WIIMOTE
tristate "Nintendo Wii / Wii U peripherals"
depends on HID
diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
index 193431ec4db8..8fcaaeae4d65 100644
--- a/drivers/hid/Makefile
+++ b/drivers/hid/Makefile
@@ -134,6 +134,7 @@ wacom-objs := wacom_wac.o wacom_sys.o
obj-$(CONFIG_HID_WACOM) += wacom.o
obj-$(CONFIG_HID_WALTOP) += hid-waltop.o
obj-$(CONFIG_HID_WIIMOTE) += hid-wiimote.o
+obj-$(CONFIG_HID_WIIU_DRC) += hid-wiiu-drc.o
obj-$(CONFIG_HID_SENSOR_HUB) += hid-sensor-hub.o
obj-$(CONFIG_HID_SENSOR_CUSTOM_SENSOR) += hid-sensor-custom.o

diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 84b8da3e7d09..fbac0dd021f1 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -916,6 +916,7 @@
#define USB_VENDOR_ID_NINTENDO 0x057e
#define USB_DEVICE_ID_NINTENDO_WIIMOTE 0x0306
#define USB_DEVICE_ID_NINTENDO_WIIMOTE2 0x0330
+#define USB_DEVICE_ID_NINTENDO_WIIU_DRH 0x0341

#define USB_VENDOR_ID_NOVATEK 0x0603
#define USB_DEVICE_ID_NOVATEK_PCT 0x0600
diff --git a/drivers/hid/hid-quirks.c b/drivers/hid/hid-quirks.c
index 3dd6f15f2a67..af400177537e 100644
--- a/drivers/hid/hid-quirks.c
+++ b/drivers/hid/hid-quirks.c
@@ -513,6 +513,9 @@ static const struct hid_device_id hid_have_special_driver[] = {
{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_NINTENDO, USB_DEVICE_ID_NINTENDO_WIIMOTE) },
{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_NINTENDO, USB_DEVICE_ID_NINTENDO_WIIMOTE2) },
#endif
+#if IS_ENABLED(CONFIG_HID_WIIU_DRC)
+ { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_NINTENDO, USB_DEVICE_ID_NINTENDO_WIIU_DRH) },
+#endif
#if IS_ENABLED(CONFIG_HID_NTI)
{ HID_USB_DEVICE(USB_VENDOR_ID_NTI, USB_DEVICE_ID_USB_SUN) },
#endif
diff --git a/drivers/hid/hid-wiiu-drc.c b/drivers/hid/hid-wiiu-drc.c
new file mode 100644
index 000000000000..018cbdb53a2c
--- /dev/null
+++ b/drivers/hid/hid-wiiu-drc.c
@@ -0,0 +1,270 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * HID driver for Nintendo Wii U gamepad, connected via console-internal DRH
+ *
+ * Copyright (C) 2021 Emmanuel Gil Peyrot <[email protected]>
+ * Copyright (C) 2019 Ash Logan <[email protected]>
+ * Copyright (C) 2013 Mema Hacking
+ *
+ * Based on the excellent work at http://libdrc.org/docs/re/sc-input.html and
+ * https://bitbucket.org/memahaxx/libdrc/src/master/src/input-receiver.cpp .
+ * libdrc code is licensed under BSD 2-Clause.
+ * Driver based on hid-udraw-ps3.c.
+ */
+
+#include <linux/device.h>
+#include <linux/hid.h>
+#include <linux/module.h>
+#include "hid-ids.h"
+
+#define DEVICE_NAME "Nintendo Wii U gamepad"
+
+/* Button and stick constants */
+#define VOLUME_MIN 0
+#define VOLUME_MAX 255
+#define NUM_STICK_AXES 4
+#define STICK_MIN 900
+#define STICK_MAX 3200
+
+#define BUTTON_SYNC BIT(0)
+#define BUTTON_HOME BIT(1)
+#define BUTTON_MINUS BIT(2)
+#define BUTTON_PLUS BIT(3)
+#define BUTTON_R BIT(4)
+#define BUTTON_L BIT(5)
+#define BUTTON_ZR BIT(6)
+#define BUTTON_ZL BIT(7)
+#define BUTTON_DOWN BIT(8)
+#define BUTTON_UP BIT(9)
+#define BUTTON_RIGHT BIT(10)
+#define BUTTON_LEFT BIT(11)
+#define BUTTON_Y BIT(12)
+#define BUTTON_X BIT(13)
+#define BUTTON_B BIT(14)
+#define BUTTON_A BIT(15)
+
+#define BUTTON_TV BIT(21)
+#define BUTTON_R3 BIT(22)
+#define BUTTON_L3 BIT(23)
+
+#define BUTTON_POWER BIT(25)
+
+/*
+ * The device is setup with multiple input devices:
+ * - A joypad with the buttons and sticks.
+ */
+
+struct drc {
+ struct input_dev *joy_input_dev;
+ struct hid_device *hdev;
+};
+
+static int drc_raw_event(struct hid_device *hdev, struct hid_report *report,
+ u8 *data, int len)
+{
+ struct drc *drc = hid_get_drvdata(hdev);
+ int i;
+ u32 buttons;
+
+ if (len != 128)
+ return 0;
+
+ buttons = (data[4] << 24) | (data[80] << 16) | (data[2] << 8) | data[3];
+ /* joypad */
+ input_report_key(drc->joy_input_dev, BTN_DPAD_RIGHT, buttons & BUTTON_RIGHT);
+ input_report_key(drc->joy_input_dev, BTN_DPAD_DOWN, buttons & BUTTON_DOWN);
+ input_report_key(drc->joy_input_dev, BTN_DPAD_LEFT, buttons & BUTTON_LEFT);
+ input_report_key(drc->joy_input_dev, BTN_DPAD_UP, buttons & BUTTON_UP);
+
+ input_report_key(drc->joy_input_dev, BTN_EAST, buttons & BUTTON_A);
+ input_report_key(drc->joy_input_dev, BTN_SOUTH, buttons & BUTTON_B);
+ input_report_key(drc->joy_input_dev, BTN_NORTH, buttons & BUTTON_X);
+ input_report_key(drc->joy_input_dev, BTN_WEST, buttons & BUTTON_Y);
+
+ input_report_key(drc->joy_input_dev, BTN_TL, buttons & BUTTON_L);
+ input_report_key(drc->joy_input_dev, BTN_TL2, buttons & BUTTON_ZL);
+ input_report_key(drc->joy_input_dev, BTN_TR, buttons & BUTTON_R);
+ input_report_key(drc->joy_input_dev, BTN_TR2, buttons & BUTTON_ZR);
+
+ input_report_key(drc->joy_input_dev, BTN_Z, buttons & BUTTON_TV);
+ input_report_key(drc->joy_input_dev, BTN_THUMBL, buttons & BUTTON_L3);
+ input_report_key(drc->joy_input_dev, BTN_THUMBR, buttons & BUTTON_R3);
+
+ input_report_key(drc->joy_input_dev, BTN_SELECT, buttons & BUTTON_MINUS);
+ input_report_key(drc->joy_input_dev, BTN_START, buttons & BUTTON_PLUS);
+ input_report_key(drc->joy_input_dev, BTN_MODE, buttons & BUTTON_HOME);
+
+ input_report_key(drc->joy_input_dev, BTN_DEAD, buttons & BUTTON_POWER);
+
+ for (i = 0; i < NUM_STICK_AXES; i++) {
+ s16 val = (data[7 + 2*i] << 8) | data[6 + 2*i];
+ /* clamp */
+ if (val < STICK_MIN)
+ val = STICK_MIN;
+ if (val > STICK_MAX)
+ val = STICK_MAX;
+
+ switch (i) {
+ case 0:
+ input_report_abs(drc->joy_input_dev, ABS_X, val);
+ break;
+ case 1:
+ input_report_abs(drc->joy_input_dev, ABS_Y, val);
+ break;
+ case 2:
+ input_report_abs(drc->joy_input_dev, ABS_RX, val);
+ break;
+ case 3:
+ input_report_abs(drc->joy_input_dev, ABS_RY, val);
+ break;
+ default:
+ break;
+ }
+ }
+
+ input_report_abs(drc->joy_input_dev, ABS_VOLUME, data[14]);
+
+ input_sync(drc->joy_input_dev);
+
+ /* let hidraw and hiddev handle the report */
+ return 0;
+}
+
+static int drc_open(struct input_dev *dev)
+{
+ struct drc *drc = input_get_drvdata(dev);
+
+ return hid_hw_open(drc->hdev);
+}
+
+static void drc_close(struct input_dev *dev)
+{
+ struct drc *drc = input_get_drvdata(dev);
+
+ hid_hw_close(drc->hdev);
+}
+
+static struct input_dev *allocate_and_setup(struct hid_device *hdev,
+ const char *name)
+{
+ struct input_dev *input_dev;
+
+ input_dev = devm_input_allocate_device(&hdev->dev);
+ if (!input_dev)
+ return NULL;
+
+ input_dev->name = name;
+ input_dev->phys = hdev->phys;
+ input_dev->dev.parent = &hdev->dev;
+ input_dev->open = drc_open;
+ input_dev->close = drc_close;
+ input_dev->uniq = hdev->uniq;
+ input_dev->id.bustype = hdev->bus;
+ input_dev->id.vendor = hdev->vendor;
+ input_dev->id.product = hdev->product;
+ input_dev->id.version = hdev->version;
+ input_set_drvdata(input_dev, hid_get_drvdata(hdev));
+
+ return input_dev;
+}
+
+static bool drc_setup_joypad(struct drc *drc,
+ struct hid_device *hdev)
+{
+ struct input_dev *input_dev;
+
+ input_dev = allocate_and_setup(hdev, DEVICE_NAME " Joypad");
+ if (!input_dev)
+ return false;
+
+ input_dev->evbit[0] = BIT(EV_KEY) | BIT(EV_ABS);
+
+ set_bit(BTN_DPAD_RIGHT, input_dev->keybit);
+ set_bit(BTN_DPAD_DOWN, input_dev->keybit);
+ set_bit(BTN_DPAD_LEFT, input_dev->keybit);
+ set_bit(BTN_DPAD_UP, input_dev->keybit);
+ set_bit(BTN_EAST, input_dev->keybit);
+ set_bit(BTN_SOUTH, input_dev->keybit);
+ set_bit(BTN_NORTH, input_dev->keybit);
+ set_bit(BTN_WEST, input_dev->keybit);
+ set_bit(BTN_TL, input_dev->keybit);
+ set_bit(BTN_TL2, input_dev->keybit);
+ set_bit(BTN_TR, input_dev->keybit);
+ set_bit(BTN_TR2, input_dev->keybit);
+ set_bit(BTN_THUMBL, input_dev->keybit);
+ set_bit(BTN_THUMBR, input_dev->keybit);
+ set_bit(BTN_SELECT, input_dev->keybit);
+ set_bit(BTN_START, input_dev->keybit);
+ set_bit(BTN_MODE, input_dev->keybit);
+
+ /* These two buttons are actually TV control and Power. */
+ set_bit(BTN_Z, input_dev->keybit);
+ set_bit(BTN_DEAD, input_dev->keybit);
+
+ input_set_abs_params(input_dev, ABS_X, STICK_MIN, STICK_MAX, 0, 0);
+ input_set_abs_params(input_dev, ABS_Y, STICK_MIN, STICK_MAX, 0, 0);
+ input_set_abs_params(input_dev, ABS_RX, STICK_MIN, STICK_MAX, 0, 0);
+ input_set_abs_params(input_dev, ABS_RY, STICK_MIN, STICK_MAX, 0, 0);
+ input_set_abs_params(input_dev, ABS_VOLUME, VOLUME_MIN, VOLUME_MAX, 0, 0);
+
+ drc->joy_input_dev = input_dev;
+
+ return true;
+}
+
+static int drc_probe(struct hid_device *hdev, const struct hid_device_id *id)
+{
+ struct drc *drc;
+ int ret;
+
+ drc = devm_kzalloc(&hdev->dev, sizeof(struct drc), GFP_KERNEL);
+ if (!drc)
+ return -ENOMEM;
+
+ drc->hdev = hdev;
+
+ hid_set_drvdata(hdev, drc);
+
+ ret = hid_parse(hdev);
+ if (ret) {
+ hid_err(hdev, "parse failed\n");
+ return ret;
+ }
+
+ if (!drc_setup_joypad(drc, hdev)) {
+ hid_err(hdev, "could not allocate interface\n");
+ return -ENOMEM;
+ }
+
+ ret = input_register_device(drc->joy_input_dev);
+ if (ret) {
+ hid_err(hdev, "failed to register interface\n");
+ return ret;
+ }
+
+ ret = hid_hw_start(hdev, HID_CONNECT_HIDRAW | HID_CONNECT_DRIVER);
+ if (ret) {
+ hid_err(hdev, "hw start failed\n");
+ return ret;
+ }
+
+ return 0;
+}
+
+static const struct hid_device_id drc_devices[] = {
+ { HID_USB_DEVICE(USB_VENDOR_ID_NINTENDO, USB_DEVICE_ID_NINTENDO_WIIU_DRH) },
+ { }
+};
+MODULE_DEVICE_TABLE(hid, drc_devices);
+
+static struct hid_driver drc_driver = {
+ .name = "hid-wiiu-drc",
+ .id_table = drc_devices,
+ .raw_event = drc_raw_event,
+ .probe = drc_probe,
+};
+module_hid_driver(drc_driver);
+
+MODULE_AUTHOR("Ash Logan <[email protected]>");
+MODULE_DESCRIPTION("Nintendo Wii U gamepad driver");
+MODULE_LICENSE("GPL");
--
2.31.1

2021-05-02 23:30:29

by Emmanuel Gil Peyrot

[permalink] [raw]
Subject: [PATCH 2/4] HID: wiiu-drc: Implement touch reports

There is a 100×200 inaccessible border on each side, and the Y axis is
inverted, these are the two main quirks of this touch panel.

I’ve been testing with weston-simple-touch mostly, but it also with the
rest of Weston.

Signed-off-by: Ash Logan <[email protected]>
Signed-off-by: Emmanuel Gil Peyrot <[email protected]>
---
drivers/hid/hid-wiiu-drc.c | 83 +++++++++++++++++++++++++++++++++++---
1 file changed, 78 insertions(+), 5 deletions(-)

diff --git a/drivers/hid/hid-wiiu-drc.c b/drivers/hid/hid-wiiu-drc.c
index 018cbdb53a2c..77e70827c37d 100644
--- a/drivers/hid/hid-wiiu-drc.c
+++ b/drivers/hid/hid-wiiu-drc.c
@@ -49,13 +49,27 @@

#define BUTTON_POWER BIT(25)

+/* Touch constants */
+/* Resolution in pixels */
+#define RES_X 854
+#define RES_Y 480
+/* Display/touch size in mm */
+#define WIDTH 138
+#define HEIGHT 79
+#define NUM_TOUCH_POINTS 10
+#define MAX_TOUCH_RES (1 << 12)
+#define TOUCH_BORDER_X 100
+#define TOUCH_BORDER_Y 200
+
/*
* The device is setup with multiple input devices:
* - A joypad with the buttons and sticks.
+ * - The touch area which works as a touchscreen.
*/

struct drc {
struct input_dev *joy_input_dev;
+ struct input_dev *touch_input_dev;
struct hid_device *hdev;
};

@@ -63,7 +77,7 @@ static int drc_raw_event(struct hid_device *hdev, struct hid_report *report,
u8 *data, int len)
{
struct drc *drc = hid_get_drvdata(hdev);
- int i;
+ int i, x, y, pressure, base;
u32 buttons;

if (len != 128)
@@ -126,6 +140,37 @@ static int drc_raw_event(struct hid_device *hdev, struct hid_report *report,

input_sync(drc->joy_input_dev);

+ /* touch */
+ /* Average touch points for improved accuracy. */
+ x = y = 0;
+ for (i = 0; i < NUM_TOUCH_POINTS; i++) {
+ base = 36 + 4 * i;
+
+ x += ((data[base + 1] & 0xF) << 8) | data[base];
+ y += ((data[base + 3] & 0xF) << 8) | data[base + 2];
+ }
+ x /= NUM_TOUCH_POINTS;
+ y /= NUM_TOUCH_POINTS;
+
+ /* Pressure reporting isn’t properly understood, so we don’t report it yet. */
+ pressure = 0;
+ pressure |= ((data[37] >> 4) & 7) << 0;
+ pressure |= ((data[39] >> 4) & 7) << 3;
+ pressure |= ((data[41] >> 4) & 7) << 6;
+ pressure |= ((data[43] >> 4) & 7) << 9;
+
+ if (pressure != 0) {
+ input_report_key(drc->touch_input_dev, BTN_TOUCH, 1);
+ input_report_key(drc->touch_input_dev, BTN_TOOL_FINGER, 1);
+
+ input_report_abs(drc->touch_input_dev, ABS_X, x);
+ input_report_abs(drc->touch_input_dev, ABS_Y, MAX_TOUCH_RES - y);
+ } else {
+ input_report_key(drc->touch_input_dev, BTN_TOUCH, 0);
+ input_report_key(drc->touch_input_dev, BTN_TOOL_FINGER, 0);
+ }
+ input_sync(drc->touch_input_dev);
+
/* let hidraw and hiddev handle the report */
return 0;
}
@@ -168,6 +213,32 @@ static struct input_dev *allocate_and_setup(struct hid_device *hdev,
return input_dev;
}

+static bool drc_setup_touch(struct drc *drc,
+ struct hid_device *hdev)
+{
+ struct input_dev *input_dev;
+
+ input_dev = allocate_and_setup(hdev, DEVICE_NAME " Touch");
+ if (!input_dev)
+ return false;
+
+ input_dev->evbit[0] = BIT(EV_ABS) | BIT(EV_KEY);
+
+ input_set_abs_params(input_dev, ABS_X, TOUCH_BORDER_X, MAX_TOUCH_RES - TOUCH_BORDER_X, 20, 0);
+ input_abs_set_res(input_dev, ABS_X, RES_X / WIDTH);
+ input_set_abs_params(input_dev, ABS_Y, TOUCH_BORDER_Y, MAX_TOUCH_RES - TOUCH_BORDER_Y, 20, 0);
+ input_abs_set_res(input_dev, ABS_Y, RES_Y / HEIGHT);
+
+ set_bit(BTN_TOUCH, input_dev->keybit);
+ set_bit(BTN_TOOL_FINGER, input_dev->keybit);
+
+ set_bit(INPUT_PROP_DIRECT, input_dev->propbit);
+
+ drc->touch_input_dev = input_dev;
+
+ return true;
+}
+
static bool drc_setup_joypad(struct drc *drc,
struct hid_device *hdev)
{
@@ -231,14 +302,16 @@ static int drc_probe(struct hid_device *hdev, const struct hid_device_id *id)
return ret;
}

- if (!drc_setup_joypad(drc, hdev)) {
- hid_err(hdev, "could not allocate interface\n");
+ if (!drc_setup_joypad(drc, hdev) ||
+ !drc_setup_touch(drc, hdev)) {
+ hid_err(hdev, "could not allocate interfaces\n");
return -ENOMEM;
}

- ret = input_register_device(drc->joy_input_dev);
+ ret = input_register_device(drc->joy_input_dev) ||
+ input_register_device(drc->touch_input_dev);
if (ret) {
- hid_err(hdev, "failed to register interface\n");
+ hid_err(hdev, "failed to register interfaces\n");
return ret;
}

--
2.31.1

2021-05-02 23:32:29

by Emmanuel Gil Peyrot

[permalink] [raw]
Subject: [PATCH 3/4] HID: wiiu-drc: Add accelerometer, gyroscope and magnetometer readings

These are mostly untested so far, because I have no idea which userland
to test against, but evtest seems to give at least sensible values.

The magnetometer doesn’t have dedicated INPUT_PROP_ACCELEROMETER
buttons, so I used three clearly invalid absolute values, in the hope
that someone will fix that over time. Another solution might be to go
for the iio subsystem instead, but it wouldn’t be tied to the HID any
longer and I would feel uneasy about that. Especially because multiple
such gamepads could be connected to a single computer.

Signed-off-by: Ash Logan <[email protected]>
Signed-off-by: Emmanuel Gil Peyrot <[email protected]>
---
drivers/hid/hid-wiiu-drc.c | 78 ++++++++++++++++++++++++++++++++++++--
1 file changed, 75 insertions(+), 3 deletions(-)

diff --git a/drivers/hid/hid-wiiu-drc.c b/drivers/hid/hid-wiiu-drc.c
index 77e70827c37d..80faaaad2bb5 100644
--- a/drivers/hid/hid-wiiu-drc.c
+++ b/drivers/hid/hid-wiiu-drc.c
@@ -61,15 +61,25 @@
#define TOUCH_BORDER_X 100
#define TOUCH_BORDER_Y 200

+/* Accelerometer, gyroscope and magnetometer constants */
+#define ACCEL_MIN -(1 << 15)
+#define ACCEL_MAX ((1 << 15) - 1)
+#define GYRO_MIN -(1 << 23)
+#define GYRO_MAX ((1 << 23) - 1)
+#define MAGNET_MIN ACCEL_MIN
+#define MAGNET_MAX ACCEL_MAX
+
/*
* The device is setup with multiple input devices:
* - A joypad with the buttons and sticks.
* - The touch area which works as a touchscreen.
+ * - An accelerometer + gyroscope + magnetometer device.
*/

struct drc {
struct input_dev *joy_input_dev;
struct input_dev *touch_input_dev;
+ struct input_dev *accel_input_dev;
struct hid_device *hdev;
};

@@ -77,7 +87,7 @@ static int drc_raw_event(struct hid_device *hdev, struct hid_report *report,
u8 *data, int len)
{
struct drc *drc = hid_get_drvdata(hdev);
- int i, x, y, pressure, base;
+ int i, x, y, z, pressure, base;
u32 buttons;

if (len != 128)
@@ -171,6 +181,31 @@ static int drc_raw_event(struct hid_device *hdev, struct hid_report *report,
}
input_sync(drc->touch_input_dev);

+ /* accelerometer */
+ x = (data[16] << 8) | data[15];
+ y = (data[18] << 8) | data[17];
+ z = (data[20] << 8) | data[19];
+ input_report_abs(drc->accel_input_dev, ABS_X, (int16_t)x);
+ input_report_abs(drc->accel_input_dev, ABS_Y, (int16_t)y);
+ input_report_abs(drc->accel_input_dev, ABS_Z, (int16_t)z);
+
+ /* gyroscope */
+ x = (data[23] << 24) | (data[22] << 16) | (data[21] << 8);
+ y = (data[26] << 24) | (data[25] << 16) | (data[24] << 8);
+ z = (data[29] << 24) | (data[28] << 16) | (data[27] << 8);
+ input_report_abs(drc->accel_input_dev, ABS_RX, x >> 8);
+ input_report_abs(drc->accel_input_dev, ABS_RY, y >> 8);
+ input_report_abs(drc->accel_input_dev, ABS_RZ, z >> 8);
+
+ /* magnetometer */
+ x = (data[31] << 8) | data[30];
+ y = (data[33] << 8) | data[32];
+ z = (data[35] << 8) | data[34];
+ input_report_abs(drc->accel_input_dev, ABS_THROTTLE, (int16_t)x);
+ input_report_abs(drc->accel_input_dev, ABS_RUDDER, (int16_t)y);
+ input_report_abs(drc->accel_input_dev, ABS_WHEEL, (int16_t)z);
+ input_sync(drc->accel_input_dev);
+
/* let hidraw and hiddev handle the report */
return 0;
}
@@ -239,6 +274,41 @@ static bool drc_setup_touch(struct drc *drc,
return true;
}

+static bool drc_setup_accel(struct drc *drc,
+ struct hid_device *hdev)
+{
+ struct input_dev *input_dev;
+
+ input_dev = allocate_and_setup(hdev, DEVICE_NAME " Accelerometer");
+ if (!input_dev)
+ return false;
+
+ input_dev->evbit[0] = BIT(EV_ABS);
+
+ /* 1G accel is reported as about -7600 */
+ input_set_abs_params(input_dev, ABS_X, ACCEL_MIN, ACCEL_MAX, 0, 0);
+ input_set_abs_params(input_dev, ABS_Y, ACCEL_MIN, ACCEL_MAX, 0, 0);
+ input_set_abs_params(input_dev, ABS_Z, ACCEL_MIN, ACCEL_MAX, 0, 0);
+
+ /* gyroscope */
+ input_set_abs_params(input_dev, ABS_RX, GYRO_MIN, GYRO_MAX, 0, 0);
+ input_set_abs_params(input_dev, ABS_RY, GYRO_MIN, GYRO_MAX, 0, 0);
+ input_set_abs_params(input_dev, ABS_RZ, GYRO_MIN, GYRO_MAX, 0, 0);
+
+ /* magnetometer */
+ /* TODO: Figure out which ABS_* would make more sense to expose, or
+ * maybe go for the iio subsystem? */
+ input_set_abs_params(input_dev, ABS_THROTTLE, MAGNET_MIN, MAGNET_MAX, 0, 0);
+ input_set_abs_params(input_dev, ABS_RUDDER, MAGNET_MIN, MAGNET_MAX, 0, 0);
+ input_set_abs_params(input_dev, ABS_WHEEL, MAGNET_MIN, MAGNET_MAX, 0, 0);
+
+ set_bit(INPUT_PROP_ACCELEROMETER, input_dev->propbit);
+
+ drc->accel_input_dev = input_dev;
+
+ return true;
+}
+
static bool drc_setup_joypad(struct drc *drc,
struct hid_device *hdev)
{
@@ -303,13 +373,15 @@ static int drc_probe(struct hid_device *hdev, const struct hid_device_id *id)
}

if (!drc_setup_joypad(drc, hdev) ||
- !drc_setup_touch(drc, hdev)) {
+ !drc_setup_touch(drc, hdev) ||
+ !drc_setup_accel(drc, hdev)) {
hid_err(hdev, "could not allocate interfaces\n");
return -ENOMEM;
}

ret = input_register_device(drc->joy_input_dev) ||
- input_register_device(drc->touch_input_dev);
+ input_register_device(drc->touch_input_dev) ||
+ input_register_device(drc->accel_input_dev);
if (ret) {
hid_err(hdev, "failed to register interfaces\n");
return ret;
--
2.31.1

2021-05-02 23:33:32

by Emmanuel Gil Peyrot

[permalink] [raw]
Subject: [PATCH 4/4] HID: wiiu-drc: Add battery reporting

On my DRC the values only go between 142 (battery LED blinking red
before shutdown) and 178 (charge LED stopping), it seems to be the same
on other units according to other testers.

A spinlock is used to avoid the battery level and status from being
reported unsynchronised.

Signed-off-by: Emmanuel Gil Peyrot <[email protected]>
---
drivers/hid/hid-wiiu-drc.c | 107 +++++++++++++++++++++++++++++++++++++
1 file changed, 107 insertions(+)

diff --git a/drivers/hid/hid-wiiu-drc.c b/drivers/hid/hid-wiiu-drc.c
index 80faaaad2bb5..119d55542e31 100644
--- a/drivers/hid/hid-wiiu-drc.c
+++ b/drivers/hid/hid-wiiu-drc.c
@@ -15,6 +15,8 @@
#include <linux/device.h>
#include <linux/hid.h>
#include <linux/module.h>
+#include <linux/power_supply.h>
+#include <linux/spinlock.h>
#include "hid-ids.h"

#define DEVICE_NAME "Nintendo Wii U gamepad"
@@ -69,6 +71,11 @@
#define MAGNET_MIN ACCEL_MIN
#define MAGNET_MAX ACCEL_MAX

+/* Battery constants */
+#define BATTERY_MIN 142
+#define BATTERY_MAX 178
+#define BATTERY_CAPACITY(val) ((val - BATTERY_MIN) * 100 / (BATTERY_MAX - BATTERY_MIN))
+
/*
* The device is setup with multiple input devices:
* - A joypad with the buttons and sticks.
@@ -77,10 +84,17 @@
*/

struct drc {
+ spinlock_t lock;
+
struct input_dev *joy_input_dev;
struct input_dev *touch_input_dev;
struct input_dev *accel_input_dev;
struct hid_device *hdev;
+ struct power_supply *battery;
+ struct power_supply_desc battery_desc;
+
+ u8 battery_energy;
+ int battery_status;
};

static int drc_raw_event(struct hid_device *hdev, struct hid_report *report,
@@ -89,6 +103,7 @@ static int drc_raw_event(struct hid_device *hdev, struct hid_report *report,
struct drc *drc = hid_get_drvdata(hdev);
int i, x, y, z, pressure, base;
u32 buttons;
+ unsigned long flags;

if (len != 128)
return 0;
@@ -206,6 +221,17 @@ static int drc_raw_event(struct hid_device *hdev, struct hid_report *report,
input_report_abs(drc->accel_input_dev, ABS_WHEEL, (int16_t)z);
input_sync(drc->accel_input_dev);

+ /* battery */
+ spin_lock_irqsave(&drc->lock, flags);
+ drc->battery_energy = data[5];
+ if (drc->battery_energy == BATTERY_MAX)
+ drc->battery_status = POWER_SUPPLY_STATUS_FULL;
+ else if ((data[4] & 0x40) != 0)
+ drc->battery_status = POWER_SUPPLY_STATUS_CHARGING;
+ else
+ drc->battery_status = POWER_SUPPLY_STATUS_DISCHARGING;
+ spin_unlock_irqrestore(&drc->lock, flags);
+
/* let hidraw and hiddev handle the report */
return 0;
}
@@ -309,10 +335,67 @@ static bool drc_setup_accel(struct drc *drc,
return true;
}

+static enum power_supply_property drc_battery_props[] = {
+ POWER_SUPPLY_PROP_PRESENT,
+ POWER_SUPPLY_PROP_CAPACITY,
+ POWER_SUPPLY_PROP_SCOPE,
+ POWER_SUPPLY_PROP_STATUS,
+ POWER_SUPPLY_PROP_ENERGY_NOW,
+ POWER_SUPPLY_PROP_ENERGY_EMPTY,
+ POWER_SUPPLY_PROP_ENERGY_FULL,
+};
+
+static int drc_battery_get_property(struct power_supply *psy,
+ enum power_supply_property psp,
+ union power_supply_propval *val)
+{
+ struct drc *drc = power_supply_get_drvdata(psy);
+ unsigned long flags;
+ int ret = 0;
+ u8 battery_energy;
+ int battery_status;
+
+ spin_lock_irqsave(&drc->lock, flags);
+ battery_energy = drc->battery_energy;
+ battery_status = drc->battery_status;
+ spin_unlock_irqrestore(&drc->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_CAPACITY:
+ val->intval = BATTERY_CAPACITY(battery_energy);
+ break;
+ case POWER_SUPPLY_PROP_STATUS:
+ val->intval = battery_status;
+ break;
+ case POWER_SUPPLY_PROP_ENERGY_NOW:
+ val->intval = battery_energy;
+ break;
+ case POWER_SUPPLY_PROP_ENERGY_EMPTY:
+ val->intval = BATTERY_MIN;
+ break;
+ case POWER_SUPPLY_PROP_ENERGY_FULL:
+ val->intval = BATTERY_MAX;
+ break;
+ default:
+ ret = -EINVAL;
+ break;
+ }
+ return ret;
+}
+
static bool drc_setup_joypad(struct drc *drc,
struct hid_device *hdev)
{
struct input_dev *input_dev;
+ struct power_supply_config psy_cfg = { .drv_data = drc, };
+ int ret;
+ static uint8_t drc_num = 0;

input_dev = allocate_and_setup(hdev, DEVICE_NAME " Joypad");
if (!input_dev)
@@ -350,6 +433,30 @@ static bool drc_setup_joypad(struct drc *drc,

drc->joy_input_dev = input_dev;

+ drc->battery_desc.properties = drc_battery_props;
+ drc->battery_desc.num_properties = ARRAY_SIZE(drc_battery_props);
+ drc->battery_desc.get_property = drc_battery_get_property;
+ drc->battery_desc.type = POWER_SUPPLY_TYPE_BATTERY;
+ drc->battery_desc.use_for_apm = 0;
+
+ /*
+ * TODO: It might be better to use the interface number as the drc_num,
+ * but I don’t know how to fetch it from here. In userland it is
+ * /sys/devices/platform/latte/d140000.usb/usb3/3-1/3-1:1.?/bInterfaceNumber
+ */
+ drc->battery_desc.name = devm_kasprintf(&hdev->dev, GFP_KERNEL, "wiiu-drc-%i-battery", drc_num++);
+ if (!drc->battery_desc.name)
+ return -ENOMEM;
+
+ drc->battery = devm_power_supply_register(&hdev->dev, &drc->battery_desc, &psy_cfg);
+ if (IS_ERR(drc->battery)) {
+ ret = PTR_ERR(drc->battery);
+ hid_err(hdev, "Unable to register battery device\n");
+ return ret;
+ }
+
+ power_supply_powers(drc->battery, &hdev->dev);
+
return true;
}

--
2.31.1

2021-05-05 22:43:37

by Jonathan Neuschäfer

[permalink] [raw]
Subject: Re: [PATCH 1/4] HID: wiiu-drc: Add a driver for this gamepad

Hi,

some mostly trivial remarks and questions of curiosity below, because
I'm not very qualified to review the input subsystem side of things.


On Mon, May 03, 2021 at 01:28:32AM +0200, Emmanuel Gil Peyrot wrote:
> From: Ash Logan <[email protected]>
>
> This driver is for the DRC (wireless gamepad) when plugged to the DRH of
> the Wii U, a chip exposing it as a USB device.

s/plugged/wirelessly connected/, rather

>
> This first patch exposes the buttons and sticks of this device, so that
> it can act as a plain game controller.
>
> Signed-off-by: Ash Logan <[email protected]>
> Signed-off-by: Emmanuel Gil Peyrot <[email protected]>
> ---

Out of curiosity:

Do the HID reports travel over the wireless link from DRC to DRH, or are
they formed in DRH firmware?

Is there a reference of the device-specific HID format? I briefly looked
at https://libdrc.org/docs/index.html but couldn't find it there.


> drivers/hid/Kconfig | 7 +
> drivers/hid/Makefile | 1 +
> drivers/hid/hid-ids.h | 1 +
> drivers/hid/hid-quirks.c | 3 +
> drivers/hid/hid-wiiu-drc.c | 270 +++++++++++++++++++++++++++++++++++++
> 5 files changed, 282 insertions(+)
> create mode 100644 drivers/hid/hid-wiiu-drc.c
>
> diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
> index 4bf263c2d61a..01116c315459 100644
> --- a/drivers/hid/Kconfig
> +++ b/drivers/hid/Kconfig
> @@ -1105,6 +1105,13 @@ config HID_WACOM
> To compile this driver as a module, choose M here: the
> module will be called wacom.
>
> +config HID_WIIU_DRC
> + tristate "Nintendo Wii U gamepad over internal DRH"

gamepad (DRC)

... so it's clearer where the "DRC" name comes from.

> +#if IS_ENABLED(CONFIG_HID_WIIU_DRC)
> + { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_NINTENDO, USB_DEVICE_ID_NINTENDO_WIIU_DRH) },
> +#endif

Is the DRC connection the only USB function that the DRH provides?


> +++ b/drivers/hid/hid-wiiu-drc.c
> @@ -0,0 +1,270 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * HID driver for Nintendo Wii U gamepad, connected via console-internal DRH

gamepad (DRC)


> +static int drc_raw_event(struct hid_device *hdev, struct hid_report *report,
> + u8 *data, int len)
> +{
> + struct drc *drc = hid_get_drvdata(hdev);
> + int i;
> + u32 buttons;
> +
> + if (len != 128)
> + return 0;

From include/linux/hid.h:

* raw_event and event should return negative on error, any other value will
* pass the event on to .event() typically return 0 for success.

Not sure if returning 0 as you do above is appropriate.


> +static bool drc_setup_joypad(struct drc *drc,
> + struct hid_device *hdev)
> +{
> + struct input_dev *input_dev;
> +
> + input_dev = allocate_and_setup(hdev, DEVICE_NAME " Joypad");

"Nintendo Wii U gamepad Joypad" looks a bit sub-optimal, but I'm not
sure about the conventions here.


> +
> + /* These two buttons are actually TV control and Power. */
> + set_bit(BTN_Z, input_dev->keybit);
> + set_bit(BTN_DEAD, input_dev->keybit);

Hmm... from what I've deen the TV control button opens a menu on the
gamepad itself. Does it send the input event in addition to that?
Or is there a mode where it opens the TV menu, and a mode where it
forwards the button press to the Wii U?


> +MODULE_AUTHOR("Ash Logan <[email protected]>");

Since you're submitting the driver, rather than Ash, maybe adjust the
author field here? (totally your choice.)



Thanks,
Jonathan


Attachments:
(No filename) (3.52 kB)
signature.asc (849.00 B)
Download all attachments

2021-05-05 22:45:22

by Jonathan Neuschäfer

[permalink] [raw]
Subject: Re: [PATCH 2/4] HID: wiiu-drc: Implement touch reports

Hi,

some more comments below. Enjoy :)

On Mon, May 03, 2021 at 01:28:33AM +0200, Emmanuel Gil Peyrot wrote:
> There is a 100×200 inaccessible border on each side, and the Y axis is
> inverted, these are the two main quirks of this touch panel.

Does that mean 100 px borders left and right, and 200 px borders top and
bottom?

100×200 evokes the image of a rectangle of that size, which I found
confusing for a moment.

>
> I’ve been testing with weston-simple-touch mostly, but it also with the
> rest of Weston.
>
> Signed-off-by: Ash Logan <[email protected]>
> Signed-off-by: Emmanuel Gil Peyrot <[email protected]>
> ---
> drivers/hid/hid-wiiu-drc.c | 83 +++++++++++++++++++++++++++++++++++---
> 1 file changed, 78 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/hid/hid-wiiu-drc.c b/drivers/hid/hid-wiiu-drc.c
> index 018cbdb53a2c..77e70827c37d 100644
> --- a/drivers/hid/hid-wiiu-drc.c
> +++ b/drivers/hid/hid-wiiu-drc.c
> @@ -49,13 +49,27 @@
>
> #define BUTTON_POWER BIT(25)
>
> +/* Touch constants */
> +/* Resolution in pixels */
> +#define RES_X 854
> +#define RES_Y 480
> +/* Display/touch size in mm */
> +#define WIDTH 138
> +#define HEIGHT 79
> +#define NUM_TOUCH_POINTS 10
> +#define MAX_TOUCH_RES (1 << 12)
> +#define TOUCH_BORDER_X 100
> +#define TOUCH_BORDER_Y 200

[...]
> + /* touch */
> + /* Average touch points for improved accuracy. */
> + x = y = 0;
> + for (i = 0; i < NUM_TOUCH_POINTS; i++) {
> + base = 36 + 4 * i;
> +
> + x += ((data[base + 1] & 0xF) << 8) | data[base];
> + y += ((data[base + 3] & 0xF) << 8) | data[base + 2];
> + }
> + x /= NUM_TOUCH_POINTS;
> + y /= NUM_TOUCH_POINTS;

Given that there are 10 possible touch points: Does the gamepad actually
support multitouch (usefully)?

If so, I think it would be better to report all touch points
individually to userspace, to allow for multitouch gestures;
userspace can still implement averaging if desired.



Thanks,
Jonathan


Attachments:
(No filename) (1.97 kB)
signature.asc (849.00 B)
Download all attachments

2021-05-06 11:50:56

by Emmanuel Gil Peyrot

[permalink] [raw]
Subject: Re: [PATCH 1/4] HID: wiiu-drc: Add a driver for this gamepad

On Wed, May 05, 2021 at 10:33:15PM +0000, Jonathan Neuschäfer wrote:
> Hi,

Hi,

>
> some mostly trivial remarks and questions of curiosity below, because
> I'm not very qualified to review the input subsystem side of things.

Thanks for the questions anyway, I can probably make things clearer in
the patch thanks to them. :)

[…]
> Out of curiosity:
>
> Do the HID reports travel over the wireless link from DRC to DRH, or are
> they formed in DRH firmware?

This HID report is a 1:1 copy of what the DRC sends, with no
modification that I could find.

>
> Is there a reference of the device-specific HID format? I briefly looked
> at https://libdrc.org/docs/index.html but couldn't find it there.

You were very close, the input report is described here:
https://libdrc.org/docs/re/sc-input.html

This project wrote a userland driver for using the DRC without the DRH,
but it requires a very specific wifi chip which makes it quite
cumbersome to use.

>
>
> > drivers/hid/Kconfig | 7 +
> > drivers/hid/Makefile | 1 +
> > drivers/hid/hid-ids.h | 1 +
> > drivers/hid/hid-quirks.c | 3 +
> > drivers/hid/hid-wiiu-drc.c | 270 +++++++++++++++++++++++++++++++++++++
> > 5 files changed, 282 insertions(+)
> > create mode 100644 drivers/hid/hid-wiiu-drc.c
> >
> > diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
> > index 4bf263c2d61a..01116c315459 100644
> > --- a/drivers/hid/Kconfig
> > +++ b/drivers/hid/Kconfig
> > @@ -1105,6 +1105,13 @@ config HID_WACOM
> > To compile this driver as a module, choose M here: the
> > module will be called wacom.
> >
> > +config HID_WIIU_DRC
> > + tristate "Nintendo Wii U gamepad over internal DRH"
>
> gamepad (DRC)
>
> ... so it's clearer where the "DRC" name comes from.

Will do in v2.

>
> > +#if IS_ENABLED(CONFIG_HID_WIIU_DRC)
> > + { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_NINTENDO, USB_DEVICE_ID_NINTENDO_WIIU_DRH) },
> > +#endif
>
> Is the DRC connection the only USB function that the DRH provides?

As far as I know, yes.

But the DRC also sends microphone and camera data, which gets exposed by
the DRH, but juuuuuust not quite standard enough to work as is using
snd_usb_audio or uvcvideo. There is also a NFC reader which no one has
reversed yet to my knowledge.

There are two DRCs exposed by the DRH, despite only one of them being
bundled with each Wii U, and no game ever making use of more.

>
>
> > +++ b/drivers/hid/hid-wiiu-drc.c
> > @@ -0,0 +1,270 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later
> > +/*
> > + * HID driver for Nintendo Wii U gamepad, connected via console-internal DRH
>
> gamepad (DRC)

Ack, will be fixed in v2.

>
>
> > +static int drc_raw_event(struct hid_device *hdev, struct hid_report *report,
> > + u8 *data, int len)
> > +{
> > + struct drc *drc = hid_get_drvdata(hdev);
> > + int i;
> > + u32 buttons;
> > +
> > + if (len != 128)
> > + return 0;
>
> From include/linux/hid.h:
>
> * raw_event and event should return negative on error, any other value will
> * pass the event on to .event() typically return 0 for success.
>
> Not sure if returning 0 as you do above is appropriate.

Oops, thanks for noticing, this will be fixed in v2.

>
>
> > +static bool drc_setup_joypad(struct drc *drc,
> > + struct hid_device *hdev)
> > +{
> > + struct input_dev *input_dev;
> > +
> > + input_dev = allocate_and_setup(hdev, DEVICE_NAME " Joypad");
>
> "Nintendo Wii U gamepad Joypad" looks a bit sub-optimal, but I'm not
> sure about the conventions here.

"Nintendo Wii U gamepad buttons and sticks" would be better I think.

>
>
> > +
> > + /* These two buttons are actually TV control and Power. */
> > + set_bit(BTN_Z, input_dev->keybit);
> > + set_bit(BTN_DEAD, input_dev->keybit);
>
> Hmm... from what I've deen the TV control button opens a menu on the
> gamepad itself. Does it send the input event in addition to that?
> Or is there a mode where it opens the TV menu, and a mode where it
> forwards the button press to the Wii U?

It does draw a line of text near the bottom of the screen, saying “TV
Remote can be configured in System Settings.”, but also sends the button
as a normal button in the report. It could be possible to change its
behaviour (in System Settings perhaps?) but so far I’ve been avoiding
interacting with the proprietary OS.

The power button also has a special behaviour: when it is held for four
seconds, it will power off the DRC.

>
>
> > +MODULE_AUTHOR("Ash Logan <[email protected]>");
>
> Since you're submitting the driver, rather than Ash, maybe adjust the
> author field here? (totally your choice.)

I’ll ask them, I’m perfectly fine with becoming the author, but they
wrote most of that code, I only fixed the last few missing pieces and
did some cleanup.

>
>
>
> Thanks,
> Jonathan

Thanks!

--
Emmanuel Gil Peyrot


Attachments:
(No filename) (4.96 kB)
signature.asc (499.00 B)
Download all attachments

2021-05-06 11:54:14

by Emmanuel Gil Peyrot

[permalink] [raw]
Subject: Re: [PATCH 2/4] HID: wiiu-drc: Implement touch reports

On Wed, May 05, 2021 at 10:43:55PM +0000, Jonathan Neuschäfer wrote:
> Hi,
>
> some more comments below. Enjoy :)
>
> On Mon, May 03, 2021 at 01:28:33AM +0200, Emmanuel Gil Peyrot wrote:
> > There is a 100×200 inaccessible border on each side, and the Y axis is
> > inverted, these are the two main quirks of this touch panel.
>
> Does that mean 100 px borders left and right, and 200 px borders top and
> bottom?

Correct, I’ll reformulate in v2. :)

>
> 100×200 evokes the image of a rectangle of that size, which I found
> confusing for a moment.
>
> >
> > I’ve been testing with weston-simple-touch mostly, but it also with the
> > rest of Weston.
> >
> > Signed-off-by: Ash Logan <[email protected]>
> > Signed-off-by: Emmanuel Gil Peyrot <[email protected]>
> > ---
> > drivers/hid/hid-wiiu-drc.c | 83 +++++++++++++++++++++++++++++++++++---
> > 1 file changed, 78 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/hid/hid-wiiu-drc.c b/drivers/hid/hid-wiiu-drc.c
> > index 018cbdb53a2c..77e70827c37d 100644
> > --- a/drivers/hid/hid-wiiu-drc.c
> > +++ b/drivers/hid/hid-wiiu-drc.c
> > @@ -49,13 +49,27 @@
> >
> > #define BUTTON_POWER BIT(25)
> >
> > +/* Touch constants */
> > +/* Resolution in pixels */
> > +#define RES_X 854
> > +#define RES_Y 480
> > +/* Display/touch size in mm */
> > +#define WIDTH 138
> > +#define HEIGHT 79
> > +#define NUM_TOUCH_POINTS 10
> > +#define MAX_TOUCH_RES (1 << 12)
> > +#define TOUCH_BORDER_X 100
> > +#define TOUCH_BORDER_Y 200
>
> [...]
> > + /* touch */
> > + /* Average touch points for improved accuracy. */
> > + x = y = 0;
> > + for (i = 0; i < NUM_TOUCH_POINTS; i++) {
> > + base = 36 + 4 * i;
> > +
> > + x += ((data[base + 1] & 0xF) << 8) | data[base];
> > + y += ((data[base + 3] & 0xF) << 8) | data[base + 2];
> > + }
> > + x /= NUM_TOUCH_POINTS;
> > + y /= NUM_TOUCH_POINTS;
>
> Given that there are 10 possible touch points: Does the gamepad actually
> support multitouch (usefully)?
>
> If so, I think it would be better to report all touch points
> individually to userspace, to allow for multitouch gestures;
> userspace can still implement averaging if desired.

Sadly no, in my testing all ten reports are always within a few units
from each other, even if I press two (or more) different points on the
touchscreen at the same time.

My guess would be, the firmware and report format got written before
Nintendo decided whether to go for a capacitive or resistive touch
panel, and they didn’t get changed once the final decision was made to
go for a non-multitouch-aware resistive panel.

I’ll add some factual comment about this in v2.

>
>
>
> Thanks,
> Jonathan

Thanks!

--
Emmanuel Gil Peyrot


Attachments:
(No filename) (2.74 kB)
signature.asc (499.00 B)
Download all attachments

2021-05-06 12:14:09

by Barnabás Pőcze

[permalink] [raw]
Subject: Re: [PATCH 4/4] HID: wiiu-drc: Add battery reporting

Hi


2021. május 3., hétfő 1:28 keltezéssel, Emmanuel Gil Peyrot írta:

> On my DRC the values only go between 142 (battery LED blinking red
> before shutdown) and 178 (charge LED stopping), it seems to be the same
> on other units according to other testers.
>
> A spinlock is used to avoid the battery level and status from being
> reported unsynchronised.
>
> Signed-off-by: Emmanuel Gil Peyrot <[email protected]>
> ---
> drivers/hid/hid-wiiu-drc.c | 107 +++++++++++++++++++++++++++++++++++++
> 1 file changed, 107 insertions(+)
>
> diff --git a/drivers/hid/hid-wiiu-drc.c b/drivers/hid/hid-wiiu-drc.c
> index 80faaaad2bb5..119d55542e31 100644
> --- a/drivers/hid/hid-wiiu-drc.c
> +++ b/drivers/hid/hid-wiiu-drc.c
> @@ -15,6 +15,8 @@
> #include <linux/device.h>
> #include <linux/hid.h>
> #include <linux/module.h>
> +#include <linux/power_supply.h>
> +#include <linux/spinlock.h>
> #include "hid-ids.h"
>
> #define DEVICE_NAME "Nintendo Wii U gamepad"
> @@ -69,6 +71,11 @@
> #define MAGNET_MIN ACCEL_MIN
> #define MAGNET_MAX ACCEL_MAX
>
> +/* Battery constants */
> +#define BATTERY_MIN 142
> +#define BATTERY_MAX 178
> +#define BATTERY_CAPACITY(val) ((val - BATTERY_MIN) * 100 / (BATTERY_MAX - BATTERY_MIN))

There's `fixp_linear_interpolate()` in linux/fixp-arithmetic.h,
you could use that.


> +
> /*
> * The device is setup with multiple input devices:
> * - A joypad with the buttons and sticks.
> @@ -77,10 +84,17 @@
> */
>
> struct drc {
> + spinlock_t lock;

I believe a `spin_lock_init()` call is missing from the code.


> +
> struct input_dev *joy_input_dev;
> struct input_dev *touch_input_dev;
> struct input_dev *accel_input_dev;
> struct hid_device *hdev;
> + struct power_supply *battery;
> + struct power_supply_desc battery_desc;
> +
> + u8 battery_energy;
> + int battery_status;
> };
>
> static int drc_raw_event(struct hid_device *hdev, struct hid_report *report,
> @@ -89,6 +103,7 @@ static int drc_raw_event(struct hid_device *hdev, struct hid_report *report,
> struct drc *drc = hid_get_drvdata(hdev);
> int i, x, y, z, pressure, base;
> u32 buttons;
> + unsigned long flags;
>
> if (len != 128)
> return 0;
> @@ -206,6 +221,17 @@ static int drc_raw_event(struct hid_device *hdev, struct hid_report *report,
> input_report_abs(drc->accel_input_dev, ABS_WHEEL, (int16_t)z);
> input_sync(drc->accel_input_dev);
>
> + /* battery */
> + spin_lock_irqsave(&drc->lock, flags);
> + drc->battery_energy = data[5];
> + if (drc->battery_energy == BATTERY_MAX)
> + drc->battery_status = POWER_SUPPLY_STATUS_FULL;
> + else if ((data[4] & 0x40) != 0)

Maybe `if (data[4] & BIT(BATTERY_CHARGING))` or `if (data[4] & BATTERY_CHARGING_BIT)`
would be better.


> + drc->battery_status = POWER_SUPPLY_STATUS_CHARGING;
> + else
> + drc->battery_status = POWER_SUPPLY_STATUS_DISCHARGING;
> + spin_unlock_irqrestore(&drc->lock, flags);
> +
> /* let hidraw and hiddev handle the report */
> return 0;
> }
> @@ -309,10 +335,67 @@ static bool drc_setup_accel(struct drc *drc,
> return true;
> }
>
> +static enum power_supply_property drc_battery_props[] = {
> + POWER_SUPPLY_PROP_PRESENT,
> + POWER_SUPPLY_PROP_CAPACITY,
> + POWER_SUPPLY_PROP_SCOPE,
> + POWER_SUPPLY_PROP_STATUS,
> + POWER_SUPPLY_PROP_ENERGY_NOW,
> + POWER_SUPPLY_PROP_ENERGY_EMPTY,
> + POWER_SUPPLY_PROP_ENERGY_FULL,
> +};
> +
> +static int drc_battery_get_property(struct power_supply *psy,
> + enum power_supply_property psp,
> + union power_supply_propval *val)
> +{
> + struct drc *drc = power_supply_get_drvdata(psy);
> + unsigned long flags;
> + int ret = 0;
> + u8 battery_energy;
> + int battery_status;
> +
> + spin_lock_irqsave(&drc->lock, flags);
> + battery_energy = drc->battery_energy;
> + battery_status = drc->battery_status;
> + spin_unlock_irqrestore(&drc->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_CAPACITY:
> + val->intval = BATTERY_CAPACITY(battery_energy);
> + break;
> + case POWER_SUPPLY_PROP_STATUS:
> + val->intval = battery_status;
> + break;
> + case POWER_SUPPLY_PROP_ENERGY_NOW:
> + val->intval = battery_energy;
> + break;
> + case POWER_SUPPLY_PROP_ENERGY_EMPTY:
> + val->intval = BATTERY_MIN;
> + break;
> + case POWER_SUPPLY_PROP_ENERGY_FULL:
> + val->intval = BATTERY_MAX;
> + break;
> + default:
> + ret = -EINVAL;
> + break;
> + }
> + return ret;
> +}
> +
> static bool drc_setup_joypad(struct drc *drc,
> struct hid_device *hdev)
> {
> struct input_dev *input_dev;
> + struct power_supply_config psy_cfg = { .drv_data = drc, };
> + int ret;
> + static uint8_t drc_num = 0;

You probably need an atomic integer here and use `atomic_fetch_inc()`
in the `devm_kasprintf()` call.


>
> input_dev = allocate_and_setup(hdev, DEVICE_NAME " Joypad");
> if (!input_dev)
> @@ -350,6 +433,30 @@ static bool drc_setup_joypad(struct drc *drc,
>
> drc->joy_input_dev = input_dev;
>
> + drc->battery_desc.properties = drc_battery_props;
> + drc->battery_desc.num_properties = ARRAY_SIZE(drc_battery_props);
> + drc->battery_desc.get_property = drc_battery_get_property;
> + drc->battery_desc.type = POWER_SUPPLY_TYPE_BATTERY;
> + drc->battery_desc.use_for_apm = 0;
> +
> + /*
> + * TODO: It might be better to use the interface number as the drc_num,
> + * but I don’t know how to fetch it from here. In userland it is
> + * /sys/devices/platform/latte/d140000.usb/usb3/3-1/3-1:1.?/bInterfaceNumber
> + */

The interface number is not globally unique in any way as far as I can tell,
maybe the combination of the bus, port, device, interface numbers is.


> + drc->battery_desc.name = devm_kasprintf(&hdev->dev, GFP_KERNEL, "wiiu-drc-%i-battery", drc_num++);
> + if (!drc->battery_desc.name)
> + return -ENOMEM;

The function returns `bool`. You might want to change it to `int` and return
a proper errno.


> +
> + drc->battery = devm_power_supply_register(&hdev->dev, &drc->battery_desc, &psy_cfg);
> + if (IS_ERR(drc->battery)) {
> + ret = PTR_ERR(drc->battery);
> + hid_err(hdev, "Unable to register battery device\n");
> + return ret;
> + }
> +
> + power_supply_powers(drc->battery, &hdev->dev);
> +
> return true;
> }
>
> --
> 2.31.1


Regards,
Barnabás Pőcze

2021-05-06 12:15:41

by Barnabás Pőcze

[permalink] [raw]
Subject: Re: [PATCH 1/4] HID: wiiu-drc: Add a driver for this gamepad

Hi


2021. május 3., hétfő 1:28 keltezéssel, Emmanuel Gil Peyrot írta:

> From: Ash Logan <[email protected]>
>
> This driver is for the DRC (wireless gamepad) when plugged to the DRH of
> the Wii U, a chip exposing it as a USB device.
>
> This first patch exposes the buttons and sticks of this device, so that
> it can act as a plain game controller.
>
> Signed-off-by: Ash Logan <[email protected]>
> Signed-off-by: Emmanuel Gil Peyrot <[email protected]>
> ---
> drivers/hid/Kconfig | 7 +
> drivers/hid/Makefile | 1 +
> drivers/hid/hid-ids.h | 1 +
> drivers/hid/hid-quirks.c | 3 +
> drivers/hid/hid-wiiu-drc.c | 270 +++++++++++++++++++++++++++++++++++++
> 5 files changed, 282 insertions(+)
> create mode 100644 drivers/hid/hid-wiiu-drc.c
>
> diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
> index 4bf263c2d61a..01116c315459 100644
> --- a/drivers/hid/Kconfig
> +++ b/drivers/hid/Kconfig
> @@ -1105,6 +1105,13 @@ config HID_WACOM
> To compile this driver as a module, choose M here: the
> module will be called wacom.
>
> +config HID_WIIU_DRC
> + tristate "Nintendo Wii U gamepad over internal DRH"
> + depends on HID
> + help
> + Support for the Wii U gamepad, when connected with the Wii U’s
> + internal DRH chip.
> +
> config HID_WIIMOTE
> tristate "Nintendo Wii / Wii U peripherals"
> depends on HID
> diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
> index 193431ec4db8..8fcaaeae4d65 100644
> --- a/drivers/hid/Makefile
> +++ b/drivers/hid/Makefile
> @@ -134,6 +134,7 @@ wacom-objs := wacom_wac.o wacom_sys.o
> obj-$(CONFIG_HID_WACOM) += wacom.o
> obj-$(CONFIG_HID_WALTOP) += hid-waltop.o
> obj-$(CONFIG_HID_WIIMOTE) += hid-wiimote.o
> +obj-$(CONFIG_HID_WIIU_DRC) += hid-wiiu-drc.o
> obj-$(CONFIG_HID_SENSOR_HUB) += hid-sensor-hub.o
> obj-$(CONFIG_HID_SENSOR_CUSTOM_SENSOR) += hid-sensor-custom.o
>
> diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
> index 84b8da3e7d09..fbac0dd021f1 100644
> --- a/drivers/hid/hid-ids.h
> +++ b/drivers/hid/hid-ids.h
> @@ -916,6 +916,7 @@
> #define USB_VENDOR_ID_NINTENDO 0x057e
> #define USB_DEVICE_ID_NINTENDO_WIIMOTE 0x0306
> #define USB_DEVICE_ID_NINTENDO_WIIMOTE2 0x0330
> +#define USB_DEVICE_ID_NINTENDO_WIIU_DRH 0x0341
>
> #define USB_VENDOR_ID_NOVATEK 0x0603
> #define USB_DEVICE_ID_NOVATEK_PCT 0x0600
> diff --git a/drivers/hid/hid-quirks.c b/drivers/hid/hid-quirks.c
> index 3dd6f15f2a67..af400177537e 100644
> --- a/drivers/hid/hid-quirks.c
> +++ b/drivers/hid/hid-quirks.c
> @@ -513,6 +513,9 @@ static const struct hid_device_id hid_have_special_driver[] = {
> { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_NINTENDO, USB_DEVICE_ID_NINTENDO_WIIMOTE) },
> { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_NINTENDO, USB_DEVICE_ID_NINTENDO_WIIMOTE2) },
> #endif
> +#if IS_ENABLED(CONFIG_HID_WIIU_DRC)
> + { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_NINTENDO, USB_DEVICE_ID_NINTENDO_WIIU_DRH) },
> +#endif
> #if IS_ENABLED(CONFIG_HID_NTI)
> { HID_USB_DEVICE(USB_VENDOR_ID_NTI, USB_DEVICE_ID_USB_SUN) },
> #endif
> diff --git a/drivers/hid/hid-wiiu-drc.c b/drivers/hid/hid-wiiu-drc.c
> new file mode 100644
> index 000000000000..018cbdb53a2c
> --- /dev/null
> +++ b/drivers/hid/hid-wiiu-drc.c
> @@ -0,0 +1,270 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * HID driver for Nintendo Wii U gamepad, connected via console-internal DRH
> + *
> + * Copyright (C) 2021 Emmanuel Gil Peyrot <[email protected]>
> + * Copyright (C) 2019 Ash Logan <[email protected]>
> + * Copyright (C) 2013 Mema Hacking
> + *
> + * Based on the excellent work at http://libdrc.org/docs/re/sc-input.html and
> + * https://bitbucket.org/memahaxx/libdrc/src/master/src/input-receiver.cpp .
> + * libdrc code is licensed under BSD 2-Clause.
> + * Driver based on hid-udraw-ps3.c.
> + */
> +
> +#include <linux/device.h>
> +#include <linux/hid.h>
> +#include <linux/module.h>
> +#include "hid-ids.h"

It's usually best if you don't rely indirect includes. Include everything you use.
E.g. here linux/input.h is missing.


> +
> +#define DEVICE_NAME "Nintendo Wii U gamepad"
> +
> +/* Button and stick constants */
> +#define VOLUME_MIN 0
> +#define VOLUME_MAX 255
> +#define NUM_STICK_AXES 4
> +#define STICK_MIN 900
> +#define STICK_MAX 3200
> +
> +#define BUTTON_SYNC BIT(0)
> +#define BUTTON_HOME BIT(1)
> +#define BUTTON_MINUS BIT(2)
> +#define BUTTON_PLUS BIT(3)
> +#define BUTTON_R BIT(4)
> +#define BUTTON_L BIT(5)
> +#define BUTTON_ZR BIT(6)
> +#define BUTTON_ZL BIT(7)
> +#define BUTTON_DOWN BIT(8)
> +#define BUTTON_UP BIT(9)
> +#define BUTTON_RIGHT BIT(10)
> +#define BUTTON_LEFT BIT(11)
> +#define BUTTON_Y BIT(12)
> +#define BUTTON_X BIT(13)
> +#define BUTTON_B BIT(14)
> +#define BUTTON_A BIT(15)
> +
> +#define BUTTON_TV BIT(21)
> +#define BUTTON_R3 BIT(22)
> +#define BUTTON_L3 BIT(23)
> +
> +#define BUTTON_POWER BIT(25)
> +
> +/*
> + * The device is setup with multiple input devices:
> + * - A joypad with the buttons and sticks.
> + */
> +
> +struct drc {
> + struct input_dev *joy_input_dev;
> + struct hid_device *hdev;
> +};
> +
> +static int drc_raw_event(struct hid_device *hdev, struct hid_report *report,
> + u8 *data, int len)
> +{
> + struct drc *drc = hid_get_drvdata(hdev);
> + int i;
> + u32 buttons;
> +
> + if (len != 128)
> + return 0;
> +
> + buttons = (data[4] << 24) | (data[80] << 16) | (data[2] << 8) | data[3];
> + /* joypad */
> + input_report_key(drc->joy_input_dev, BTN_DPAD_RIGHT, buttons & BUTTON_RIGHT);
> + input_report_key(drc->joy_input_dev, BTN_DPAD_DOWN, buttons & BUTTON_DOWN);
> + input_report_key(drc->joy_input_dev, BTN_DPAD_LEFT, buttons & BUTTON_LEFT);
> + input_report_key(drc->joy_input_dev, BTN_DPAD_UP, buttons & BUTTON_UP);
> +
> + input_report_key(drc->joy_input_dev, BTN_EAST, buttons & BUTTON_A);
> + input_report_key(drc->joy_input_dev, BTN_SOUTH, buttons & BUTTON_B);
> + input_report_key(drc->joy_input_dev, BTN_NORTH, buttons & BUTTON_X);
> + input_report_key(drc->joy_input_dev, BTN_WEST, buttons & BUTTON_Y);
> +
> + input_report_key(drc->joy_input_dev, BTN_TL, buttons & BUTTON_L);
> + input_report_key(drc->joy_input_dev, BTN_TL2, buttons & BUTTON_ZL);
> + input_report_key(drc->joy_input_dev, BTN_TR, buttons & BUTTON_R);
> + input_report_key(drc->joy_input_dev, BTN_TR2, buttons & BUTTON_ZR);
> +
> + input_report_key(drc->joy_input_dev, BTN_Z, buttons & BUTTON_TV);
> + input_report_key(drc->joy_input_dev, BTN_THUMBL, buttons & BUTTON_L3);
> + input_report_key(drc->joy_input_dev, BTN_THUMBR, buttons & BUTTON_R3);
> +
> + input_report_key(drc->joy_input_dev, BTN_SELECT, buttons & BUTTON_MINUS);
> + input_report_key(drc->joy_input_dev, BTN_START, buttons & BUTTON_PLUS);
> + input_report_key(drc->joy_input_dev, BTN_MODE, buttons & BUTTON_HOME);
> +
> + input_report_key(drc->joy_input_dev, BTN_DEAD, buttons & BUTTON_POWER);
> +
> + for (i = 0; i < NUM_STICK_AXES; i++) {
> + s16 val = (data[7 + 2*i] << 8) | data[6 + 2*i];
> + /* clamp */
> + if (val < STICK_MIN)
> + val = STICK_MIN;
> + if (val > STICK_MAX)
> + val = STICK_MAX;

There's `clamp()` in linux/minmax.h, you might want to use that.


> +
> + switch (i) {
> + case 0:
> + input_report_abs(drc->joy_input_dev, ABS_X, val);
> + break;
> + case 1:
> + input_report_abs(drc->joy_input_dev, ABS_Y, val);
> + break;
> + case 2:
> + input_report_abs(drc->joy_input_dev, ABS_RX, val);
> + break;
> + case 3:
> + input_report_abs(drc->joy_input_dev, ABS_RY, val);
> + break;
> + default:
> + break;
> + }
> + }
> +
> + input_report_abs(drc->joy_input_dev, ABS_VOLUME, data[14]);
> +
> + input_sync(drc->joy_input_dev);
> +
> + /* let hidraw and hiddev handle the report */
> + return 0;
> +}
> +
> +static int drc_open(struct input_dev *dev)
> +{
> + struct drc *drc = input_get_drvdata(dev);
> +
> + return hid_hw_open(drc->hdev);
> +}
> +
> +static void drc_close(struct input_dev *dev)
> +{
> + struct drc *drc = input_get_drvdata(dev);
> +
> + hid_hw_close(drc->hdev);
> +}
> +
> +static struct input_dev *allocate_and_setup(struct hid_device *hdev,
> + const char *name)
> +{
> + struct input_dev *input_dev;
> +
> + input_dev = devm_input_allocate_device(&hdev->dev);
> + if (!input_dev)
> + return NULL;
> +
> + input_dev->name = name;
> + input_dev->phys = hdev->phys;
> + input_dev->dev.parent = &hdev->dev;
> + input_dev->open = drc_open;
> + input_dev->close = drc_close;
> + input_dev->uniq = hdev->uniq;
> + input_dev->id.bustype = hdev->bus;
> + input_dev->id.vendor = hdev->vendor;
> + input_dev->id.product = hdev->product;
> + input_dev->id.version = hdev->version;
> + input_set_drvdata(input_dev, hid_get_drvdata(hdev));
> +
> + return input_dev;
> +}
> +
> +static bool drc_setup_joypad(struct drc *drc,
> + struct hid_device *hdev)
> +{
> + struct input_dev *input_dev;
> +
> + input_dev = allocate_and_setup(hdev, DEVICE_NAME " Joypad");
> + if (!input_dev)
> + return false;
> +
> + input_dev->evbit[0] = BIT(EV_KEY) | BIT(EV_ABS);

`input_set_abs_params()` already sets EV_ABS.


> +
> + set_bit(BTN_DPAD_RIGHT, input_dev->keybit);
> + set_bit(BTN_DPAD_DOWN, input_dev->keybit);
> + set_bit(BTN_DPAD_LEFT, input_dev->keybit);
> + set_bit(BTN_DPAD_UP, input_dev->keybit);
> + set_bit(BTN_EAST, input_dev->keybit);
> + set_bit(BTN_SOUTH, input_dev->keybit);
> + set_bit(BTN_NORTH, input_dev->keybit);
> + set_bit(BTN_WEST, input_dev->keybit);
> + set_bit(BTN_TL, input_dev->keybit);
> + set_bit(BTN_TL2, input_dev->keybit);
> + set_bit(BTN_TR, input_dev->keybit);
> + set_bit(BTN_TR2, input_dev->keybit);
> + set_bit(BTN_THUMBL, input_dev->keybit);
> + set_bit(BTN_THUMBR, input_dev->keybit);
> + set_bit(BTN_SELECT, input_dev->keybit);
> + set_bit(BTN_START, input_dev->keybit);
> + set_bit(BTN_MODE, input_dev->keybit);
> +
> + /* These two buttons are actually TV control and Power. */
> + set_bit(BTN_Z, input_dev->keybit);
> + set_bit(BTN_DEAD, input_dev->keybit);

You could use `input_set_capability(device, EV_KEY, ...)` (potentially in a loop)
instead of manually setting the bits. And then `input_dev->evbit[0] = BIT(EV_KEY) | BIT(EV_ABS);`
would be unnecessary.


> +
> + input_set_abs_params(input_dev, ABS_X, STICK_MIN, STICK_MAX, 0, 0);
> + input_set_abs_params(input_dev, ABS_Y, STICK_MIN, STICK_MAX, 0, 0);
> + input_set_abs_params(input_dev, ABS_RX, STICK_MIN, STICK_MAX, 0, 0);
> + input_set_abs_params(input_dev, ABS_RY, STICK_MIN, STICK_MAX, 0, 0);
> + input_set_abs_params(input_dev, ABS_VOLUME, VOLUME_MIN, VOLUME_MAX, 0, 0);
> +
> + drc->joy_input_dev = input_dev;
> +
> + return true;
> +}
> +
> +static int drc_probe(struct hid_device *hdev, const struct hid_device_id *id)
> +{
> + struct drc *drc;
> + int ret;
> +
> + drc = devm_kzalloc(&hdev->dev, sizeof(struct drc), GFP_KERNEL);
> + if (!drc)
> + return -ENOMEM;
> +
> + drc->hdev = hdev;
> +
> + hid_set_drvdata(hdev, drc);
> +
> + ret = hid_parse(hdev);
> + if (ret) {
> + hid_err(hdev, "parse failed\n");
> + return ret;
> + }
> +
> + if (!drc_setup_joypad(drc, hdev)) {
> + hid_err(hdev, "could not allocate interface\n");
> + return -ENOMEM;
> + }
> +
> + ret = input_register_device(drc->joy_input_dev);
> + if (ret) {
> + hid_err(hdev, "failed to register interface\n");
> + return ret;
> + }
> +
> + ret = hid_hw_start(hdev, HID_CONNECT_HIDRAW | HID_CONNECT_DRIVER);

As far as I'm aware, `hid_hw_start()` should be called before `hid_hw_open()`.
Since you register the input device first, I think it is possible that `hid_hw_open()`
will be called first.


> + if (ret) {
> + hid_err(hdev, "hw start failed\n");
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static const struct hid_device_id drc_devices[] = {
> + { HID_USB_DEVICE(USB_VENDOR_ID_NINTENDO, USB_DEVICE_ID_NINTENDO_WIIU_DRH) },
> + { }
> +};
> +MODULE_DEVICE_TABLE(hid, drc_devices);
> +
> +static struct hid_driver drc_driver = {
> + .name = "hid-wiiu-drc",
> + .id_table = drc_devices,
> + .raw_event = drc_raw_event,
> + .probe = drc_probe,
> +};
> +module_hid_driver(drc_driver);
> +
> +MODULE_AUTHOR("Ash Logan <[email protected]>");
> +MODULE_DESCRIPTION("Nintendo Wii U gamepad driver");
> +MODULE_LICENSE("GPL");
> --
> 2.31.1


Regards,
Barnabás Pőcze

2021-05-06 18:10:26

by Jonathan Neuschäfer

[permalink] [raw]
Subject: Re: [PATCH 1/4] HID: wiiu-drc: Add a driver for this gamepad

On Thu, May 06, 2021 at 12:07:05PM +0200, Emmanuel Gil Peyrot wrote:
> On Wed, May 05, 2021 at 10:33:15PM +0000, Jonathan Neuschäfer wrote:
[...]
> > Is there a reference of the device-specific HID format? I briefly looked
> > at https://libdrc.org/docs/index.html but couldn't find it there.
>
> You were very close, the input report is described here:
> https://libdrc.org/docs/re/sc-input.html

Ok, I think this link would be somewhat useful to have in the commit
message or a comment.

> This project wrote a userland driver for using the DRC without the DRH,
> but it requires a very specific wifi chip which makes it quite
> cumbersome to use.

I see.


Thanks,
Jonathan


Attachments:
(No filename) (700.00 B)
signature.asc (849.00 B)
Download all attachments