* [PATCH] hid-asus-ally: Add full gamepad support
@ 2024-08-06 8:12 Luke D. Jones
2024-08-06 21:34 ` kernel test robot
` (2 more replies)
0 siblings, 3 replies; 13+ messages in thread
From: Luke D. Jones @ 2024-08-06 8:12 UTC (permalink / raw)
To: linux-input; +Cc: linux-kernel, bentiss, jikos, Luke D. Jones
This driver adds full support of the ASUS ROG Ally X gamepad and support
for the Ally 1 xpad configuration options.
- dinput is translated to XBox controller (Ally-X only)
- default mode has the QAM buttons mapped (Ally-X only)
* left is XBox button
* right is an XBox + A combo for steam QAM
- force feedback is supported (Ally-X only)
- LED brightness control (0-3)
- LED multicolor class support
- Support all configuration
The configuration options available are:
- Gamepad mode (game, wasd, mouse)
- Remapping each button, plus macro map (hold macro btn and press other)
- Joystrick and trigger deadzones
- Gamepad vibration intensity
- Leds (using multicolor class)
- Button turbo abilities (per button)
- Joystick repsonse curves
- Joystick anti-deadzones
The attribute path tree looks like this:
- `./sys/../<USB HID>/`
- `joystick_left/
- `deadzone`
- `mapping` (mouse, wasd, custom)
- `anti_deadzone`
- `response_curve`
- `calibration`
- `calibration_reset`
- `trigger_left/
- `deadzone`
- `response_curve`
- `calibration`
- `calibration_reset`
- `gamepad_mode`
- `button_mapping`
- `A`
- `B`
- `dpad_left`
- etc
No settings are applied until `apply_all` is written to. The exception is
for calibrations.
While there is calibration ability, it can be difficult to get correct
and is heavily device dependent, as such it is set when written and not
when `apply_all` is written to. On driver load the set calibrations are
retrieved - this may be what you've set in Linux, Windows, or factory
defaults.
This patch provides an excellent default experience for the Ally X
device while the Ally 1 is a little more limited due to Xinput being
used.
Signed-off-by: Luke D. Jones <luke@ljones.dev>
---
drivers/hid/Kconfig | 9 +
drivers/hid/Makefile | 1 +
drivers/hid/hid-asus-ally.c | 2281 +++++++++++++++++++++++++++++++++++
drivers/hid/hid-asus-ally.h | 544 +++++++++
drivers/hid/hid-asus.c | 20 +-
5 files changed, 2853 insertions(+), 2 deletions(-)
create mode 100644 drivers/hid/hid-asus-ally.c
create mode 100644 drivers/hid/hid-asus-ally.h
diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index 08446c89eff6..ea0dbe9111c4 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -164,6 +164,15 @@ config HID_ASUS
- GL553V series
- GL753V series
+config HID_ASUS_ALLY
+ tristate "Asus Ally gamepad configuration support"
+ depends on USB_HID
+ depends on LEDS_CLASS
+ depends on LEDS_CLASS_MULTICOLOR
+ select POWER_SUPPLY
+ help
+ Support for configuring the Asus ROG Ally gamepad using attributes.
+
config HID_AUREAL
tristate "Aureal"
help
diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
index e40f1ddebbb7..2b2aa8b6f828 100644
--- a/drivers/hid/Makefile
+++ b/drivers/hid/Makefile
@@ -31,6 +31,7 @@ obj-$(CONFIG_HID_APPLE) += hid-apple.o
obj-$(CONFIG_HID_APPLEIR) += hid-appleir.o
obj-$(CONFIG_HID_CREATIVE_SB0540) += hid-creative-sb0540.o
obj-$(CONFIG_HID_ASUS) += hid-asus.o
+obj-$(CONFIG_HID_ASUS_ALLY) += hid-asus-ally.o
obj-$(CONFIG_HID_AUREAL) += hid-aureal.o
obj-$(CONFIG_HID_BELKIN) += hid-belkin.o
obj-$(CONFIG_HID_BETOP_FF) += hid-betopff.o
diff --git a/drivers/hid/hid-asus-ally.c b/drivers/hid/hid-asus-ally.c
new file mode 100644
index 000000000000..9e8007fc3c19
--- /dev/null
+++ b/drivers/hid/hid-asus-ally.c
@@ -0,0 +1,2281 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * HID driver for Asus ROG laptops and Ally
+ *
+ * Copyright (c) 2023 Luke Jones <luke@ljones.dev>
+ */
+
+#include "linux/delay.h"
+#include "linux/device.h"
+#include "linux/err.h"
+#include "linux/input-event-codes.h"
+#include "linux/kstrtox.h"
+#include "linux/printk.h"
+#include "linux/slab.h"
+#include "linux/stddef.h"
+#include "linux/sysfs.h"
+#include <linux/hid.h>
+#include <linux/types.h>
+#include <linux/usb.h>
+#include <linux/leds.h>
+#include <linux/led-class-multicolor.h>
+
+#include "hid-ids.h"
+#include "hid-asus-ally.h"
+
+#define READY_MAX_TRIES 3
+#define FEATURE_REPORT_ID 0x0d
+#define FEATURE_ROG_ALLY_REPORT_ID 0x5a
+#define FEATURE_ROG_ALLY_CODE_PAGE 0xD1
+#define FEATURE_ROG_ALLY_REPORT_SIZE 64
+#define ALLY_X_INPUT_REPORT_USB 0x0B
+#define ALLY_X_INPUT_REPORT_USB_SIZE 16
+
+#define ALLY_CFG_INTF_IN_ADDRESS 0x83
+#define ALLY_CFG_INTF_OUT_ADDRESS 0x04
+#define ALLY_X_INTERFACE_ADDRESS 0x87
+
+#define FEATURE_KBD_LED_REPORT_ID1 0x5d
+#define FEATURE_KBD_LED_REPORT_ID2 0x5e
+
+enum ROG_ALLY_TYPE {
+ ROG_ALLY_TYPE,
+ ROG_ALLY_TYPE_X,
+};
+
+static const struct hid_device_id rog_ally_devices[] = {
+ { HID_USB_DEVICE(USB_VENDOR_ID_ASUSTEK, USB_DEVICE_ID_ASUSTEK_ROG_NKEY_ALLY),
+ .driver_data = ROG_ALLY_TYPE },
+ { HID_USB_DEVICE(USB_VENDOR_ID_ASUSTEK, USB_DEVICE_ID_ASUSTEK_ROG_NKEY_ALLY_X),
+ .driver_data = ROG_ALLY_TYPE_X },
+ {}
+};
+
+struct KeyCode {
+ const char *label;
+ u8 code;
+};
+
+static const struct KeyCode gamepad_codes[] = {
+ { "PAD_A", 0x01 }, { "PAD_B", 0x02 }, { "PAD_X", 0x03 },
+ { "PAD_Y", 0x04 }, { "PAD_LB", 0x05 }, { "PAD_RB", 0x06 },
+ { "PAD_LS", 0x07 }, { "PAD_RS", 0x08 }, { "PAD_DPAD_UP", 0x09 },
+ { "PAD_DPAD_DOWN", 0x0a }, { "PAD_DPAD_LEFT", 0x0b }, { "PAD_DPAD_RIGHT", 0x0c },
+ { "PAD_VIEW", 0x11 }, { "PAD_MENU", 0x12 }, { "PAD_XBOX", 0x13 }
+};
+
+static const struct KeyCode keyboard_codes[] = { { "KB_M1", 0x8f },
+ { "KB_M2", 0x8e },
+ { "KB_ESC", 0x76 },
+ { "KB_F1", 0x50 },
+ { "KB_F2", 0x60 },
+ { "KB_F3", 0x40 },
+ { "KB_F4", 0x0c },
+ { "KB_F5", 0x03 },
+ { "KB_F6", 0x0b },
+ { "KB_F7", 0x80 },
+ { "KB_F8", 0x0a },
+ { "KB_F9", 0x01 },
+ { "KB_F10", 0x09 },
+ { "KB_F11", 0x78 },
+ { "KB_F12", 0x07 },
+ { "KB_F14", 0x10 },
+ { "KB_F15", 0x18 },
+ { "KB_BACKTICK", 0x0e },
+ { "KB_1", 0x16 },
+ { "KB_2", 0x1e },
+ { "KB_3", 0x26 },
+ { "KB_4", 0x25 },
+ { "KB_5", 0x2e },
+ { "KB_6", 0x36 },
+ { "KB_7", 0x3d },
+ { "KB_8", 0x3e },
+ { "KB_9", 0x46 },
+ { "KB_0", 0x45 },
+ { "KB_HYPHEN", 0x4e },
+ { "KB_EQUALS", 0x55 },
+ { "KB_BACKSPACE", 0x66 },
+ { "KB_TAB", 0x0d },
+ { "KB_Q", 0x15 },
+ { "KB_W", 0x1d },
+ { "KB_E", 0x24 },
+ { "KB_R", 0x2d },
+ { "KB_T", 0x2d },
+ { "KB_Y", 0x35 },
+ { "KB_U", 0x3c },
+ { "KB_I", 0x43 },
+ { "KB_O", 0x44 },
+ { "KB_P", 0x4d },
+ { "KB_LBRACKET", 0x54 },
+ { "KB_RBRACKET", 0x5b },
+ { "KB_BACKSLASH", 0x5d },
+ { "KB_CAPS", 0x58 },
+ { "KB_A", 0x1c },
+ { "KB_S", 0x1b },
+ { "KB_D", 0x23 },
+ { "KB_F", 0x2b },
+ { "KB_G", 0x34 },
+ { "KB_H", 0x33 },
+ { "KB_J", 0x3b },
+ { "KB_K", 0x42 },
+ { "KB_L", 0x4b },
+ { "KB_SEMI", 0x4c },
+ { "KB_QUOTE", 0x52 },
+ { "KB_RET", 0x5a },
+ { "KB_LSHIFT", 0x88 },
+ { "KB_Z", 0x1a },
+ { "KB_X", 0x22 },
+ { "KB_C", 0x21 },
+ { "KB_V", 0x2a },
+ { "KB_B", 0x32 },
+ { "KB_N", 0x31 },
+ { "KB_M", 0x3a },
+ { "KB_COMMA", 0x41 },
+ { "KB_PERIOD", 0x49 },
+ { "KB_FWDSLASH", 0x4a },
+ { "KB_RSHIFT", 0x89 },
+ { "KB_LCTL", 0x8c },
+ { "KB_META", 0x82 },
+ { "KB_LALT", 0xba },
+ { "KB_SPACE", 0x29 },
+ { "KB_RALT", 0x8b },
+ { "KB_MENU", 0x84 },
+ { "KB_RCTL", 0x8d },
+ { "KB_PRNTSCN", 0xc3 },
+ { "KB_SCRLCK", 0x7e },
+ { "KB_PAUSE", 0x91 },
+ { "KB_INS", 0xc2 },
+ { "KB_HOME", 0x94 },
+ { "KB_PGUP", 0x96 },
+ { "KB_DEL", 0xc0 },
+ { "KB_END", 0x95 },
+ { "KB_PGDWN", 0x97 },
+ { "KB_UP_ARROW", 0x99 },
+ { "KB_DOWN_ARROW", 0x98 },
+ { "KB_LEFT_ARROW", 0x91 },
+ { "KB_RIGHT_ARROW", 0x9b },
+ { "NUMPAD_LOCK", 0x77 },
+ { "NUMPAD_FWDSLASH", 0x90 },
+ { "NUMPAD_ASTERISK", 0x7c },
+ { "NUMPAD_HYPHEN", 0x7b },
+ { "NUMPAD_0", 0x70 },
+ { "NUMPAD_1", 0x69 },
+ { "NUMPAD_2", 0x72 },
+ { "NUMPAD_3", 0x7a },
+ { "NUMPAD_4", 0x6b },
+ { "NUMPAD_5", 0x73 },
+ { "NUMPAD_6", 0x74 },
+ { "NUMPAD_7", 0x6c },
+ { "NUMPAD_8", 0x75 },
+ { "NUMPAD_9", 0x7d },
+ { "NUMPAD_PLUS", 0x79 },
+ { "NUMPAD_ENTER", 0x81 },
+ { "NUMPAD_PERIOD", 0x71 } };
+
+static const struct KeyCode mouse_codes[] = { { "MOUSE_LCLICK", 0x01 },
+ { "MOUSE_RCLICK", 0x02 },
+ { "MOUSE_MCLICK", 0x03 },
+ { "MOUSE_WHEEL_UP", 0x04 },
+ { "MOUSE_WHEEL_DOWN", 0x05 } };
+
+static const struct KeyCode media_codes[] = {
+ { "MEDIA_SCREENSHOT", 0x16 }, { "MEDIA_SHOW_KEYBOARD", 0x19 },
+ { "MEDIA_SHOW_DESKTOP", 0x1c }, { "MEDIA_START_RECORDING", 0x1e },
+ { "MEDIA_MIC_OFF", 0x01 }, { "MEDIA_VOL_DOWN", 0x02 },
+ { "MEDIA_VOL_UP", 0x03 }
+};
+
+/* The hatswitch outputs integers, we use them to index this X|Y pair */
+static const int hat_values[][2] = {
+ { 0, 0 }, { 0, -1 }, { 1, -1 }, { 1, 0 }, { 1, 1 },
+ { 0, 1 }, { -1, 1 }, { -1, 0 }, { -1, -1 },
+};
+
+/* rumble packet structure */
+struct ff_data {
+ u8 enable;
+ u8 magnitude_left;
+ u8 magnitude_right;
+ u8 magnitude_strong;
+ u8 magnitude_weak;
+ u8 pulse_sustain_10ms;
+ u8 pulse_release_10ms;
+ u8 loop_count;
+} __packed;
+
+struct ff_report {
+ u8 report_id;
+ struct ff_data ff;
+} __packed;
+
+struct ally_x_input_report {
+ uint16_t x, y;
+ uint16_t rx, ry;
+ uint16_t z, rz;
+ uint8_t buttons[4];
+} __packed;
+
+struct ally_x_device {
+ struct input_dev *input;
+ struct hid_device *hdev;
+ spinlock_t lock;
+
+ struct ff_report *ff_packet;
+ struct work_struct output_worker;
+ bool output_worker_initialized;
+ /* Prevent multiple queued event due to the enforced delay in worker */
+ bool update_qam_btn;
+ /* Set if the QAM and AC buttons emit Xbox and Xbox+A */
+ bool qam_btns_steam_mode;
+ bool update_ff;
+};
+
+struct ally_rgb_leds {
+ struct hid_device *hdev;
+ /* Need two dev here to enable the 3 step brightness */
+ struct led_classdev led_bright_dev;
+ struct led_classdev_mc led_rgb_dev;
+ struct work_struct work;
+ spinlock_t lock;
+
+ bool removed;
+
+ /* Update the main brightness 0-2 using a single raw write */
+ bool update_bright;
+ unsigned int brightness;
+
+ /* Update the RGB only to keep write efficient */
+ bool update_rgb;
+ uint8_t gamepad_red;
+ uint8_t gamepad_green;
+ uint8_t gamepad_blue;
+};
+
+/* ROG Ally has many settings related to the gamepad, all using the same n-key endpoint */
+struct ally_gamepad_cfg {
+ struct hid_device *hdev;
+ struct input_dev *input;
+
+ enum xpad_mode mode;
+ /*
+ * index: [joysticks/triggers][left(2 bytes), right(2 bytes)]
+ * joysticks: 2 bytes: inner, outer
+ * triggers: 2 bytes: lower, upper
+ * min/max: 0-64
+ */
+ u8 deadzones[xpad_mode_mouse][2][4];
+ /*
+ * index: left, right
+ * max: 64
+ */
+ u8 vibration_intensity[xpad_mode_mouse][2];
+ /*
+ * index: [joysticks][2 byte stepping per point]
+ * - 4 points of 2 bytes each
+ * - byte 0 of pair = stick move %
+ * - byte 1 of pair = stick response %
+ * - min/max: 1-63
+ */
+ bool supports_response_curves;
+ u8 response_curve[xpad_mode_mouse][2][8];
+ /*
+ * left = byte 0, right = byte 1
+ */
+ bool supports_anti_deadzones;
+ u8 anti_deadzones[xpad_mode_mouse][2];
+ /*
+ * index: [mode][phys pair][b1, b1 secondary, b2, b2 secondary, blocks of 11]
+ */
+ u8 key_mapping[xpad_mode_mouse][btn_pair_lt_rt][MAPPING_BLOCK_LEN];
+ /*
+ * index: [mode][button index]
+ */
+ u8 turbo_btns[xpad_mode_mouse][TURBO_BLOCK_LEN];
+ /*
+ * index: [joystick side][Y-stable, Y-min, Y-max, X-stable, X-min, X-max]
+ */
+ u32 js_calibrations[2][6];
+ /*
+ * index: [trigger side][stable, max]
+ */
+ u32 tr_calibrations[2][2];
+};
+
+static struct ally_drvdata {
+ struct hid_device *hdev;
+ struct ally_x_device *ally_x;
+ struct ally_gamepad_cfg *gamepad_cfg;
+ struct ally_rgb_leds *led_rgb;
+} drvdata;
+
+static int asus_dev_get_report(struct hid_device *hdev, u8 *out_buf, size_t out_buf_size)
+{
+ return hid_hw_raw_request(hdev, FEATURE_REPORT_ID, out_buf, out_buf_size,
+ HID_FEATURE_REPORT, HID_REQ_GET_REPORT);
+}
+
+static int asus_dev_set_report(struct hid_device *hdev, const u8 *buf, size_t buf_size)
+{
+ unsigned char *dmabuf;
+ int ret;
+
+ dmabuf = kmemdup(buf, buf_size, GFP_KERNEL);
+ if (!dmabuf)
+ return -ENOMEM;
+
+ ret = hid_hw_raw_request(hdev, buf[0], dmabuf, buf_size, HID_FEATURE_REPORT,
+ HID_REQ_SET_REPORT);
+ kfree(dmabuf);
+
+ return ret;
+}
+
+/**************************************************************************************************/
+/* ROG Ally gamepad i/o and force-feedback */
+/**************************************************************************************************/
+static int ally_x_raw_event(struct ally_x_device *ally_x, struct hid_report *report, u8 *data,
+ int size)
+{
+ struct ally_x_input_report *in_report;
+ unsigned long flags;
+ u8 byte;
+
+ if (data[0] == 0x0B) {
+ in_report = (struct ally_x_input_report *)&data[1];
+
+ input_report_abs(ally_x->input, ABS_X, in_report->x);
+ input_report_abs(ally_x->input, ABS_Y, in_report->y);
+ input_report_abs(ally_x->input, ABS_RX, in_report->rx);
+ input_report_abs(ally_x->input, ABS_RY, in_report->ry);
+ input_report_abs(ally_x->input, ABS_Z, in_report->z);
+ input_report_abs(ally_x->input, ABS_RZ, in_report->rz);
+
+ byte = in_report->buttons[0];
+ input_report_key(ally_x->input, BTN_A, byte & BIT(0));
+ input_report_key(ally_x->input, BTN_B, byte & BIT(1));
+ input_report_key(ally_x->input, BTN_X, byte & BIT(2));
+ input_report_key(ally_x->input, BTN_Y, byte & BIT(3));
+ input_report_key(ally_x->input, BTN_TL, byte & BIT(4));
+ input_report_key(ally_x->input, BTN_TR, byte & BIT(5));
+ input_report_key(ally_x->input, BTN_SELECT, byte & BIT(6));
+ input_report_key(ally_x->input, BTN_START, byte & BIT(7));
+
+ byte = in_report->buttons[1];
+ input_report_key(ally_x->input, BTN_THUMBL, byte & BIT(0));
+ input_report_key(ally_x->input, BTN_THUMBR, byte & BIT(1));
+ input_report_key(ally_x->input, BTN_MODE, byte & BIT(2));
+
+ byte = in_report->buttons[2];
+ input_report_abs(ally_x->input, ABS_HAT0X, hat_values[byte][0]);
+ input_report_abs(ally_x->input, ABS_HAT0Y, hat_values[byte][1]);
+ }
+ /*
+ * The MCU used on Ally provides many devices: gamepad, keyboord, mouse, other.
+ * The AC and QAM buttons route through another interface making it difficult to
+ * use the events unless we grab those and use them here. Only works for Ally X.
+ */
+ else if (data[0] == 0x5A) {
+ if (ally_x->qam_btns_steam_mode) {
+ spin_lock_irqsave(&ally_x->lock, flags);
+ if (data[1] == 0x38 && !ally_x->update_qam_btn) {
+ ally_x->update_qam_btn = true;
+ if (ally_x->output_worker_initialized)
+ schedule_work(&ally_x->output_worker);
+ }
+ spin_unlock_irqrestore(&ally_x->lock, flags);
+ /* Left/XBox button. Long press does ctrl+alt+del which we can't catch */
+ input_report_key(ally_x->input, BTN_MODE, data[1] == 0xA6);
+ } else {
+ input_report_key(ally_x->input, KEY_F16, data[1] == 0xA6);
+ input_report_key(ally_x->input, KEY_PROG1, data[1] == 0x38);
+ }
+ /* QAM long press */
+ input_report_key(ally_x->input, KEY_F17, data[1] == 0xA7);
+ /* QAM long press released */
+ input_report_key(ally_x->input, KEY_F18, data[1] == 0xA8);
+ }
+
+ input_sync(ally_x->input);
+
+ return 0;
+}
+
+static struct input_dev *ally_x_alloc_input_dev(struct hid_device *hdev,
+ const char *name_suffix)
+{
+ struct input_dev *input_dev;
+
+ input_dev = devm_input_allocate_device(&hdev->dev);
+ if (!input_dev)
+ return ERR_PTR(-ENOMEM);
+
+ 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_dev->uniq = hdev->uniq;
+ input_dev->name = "ASUS ROG Ally X Gamepad";
+
+ input_set_drvdata(input_dev, hdev);
+
+ return input_dev;
+}
+
+static int ally_x_play_effect(struct input_dev *idev, void *data, struct ff_effect *effect)
+{
+ struct ally_x_device *ally_x = drvdata.ally_x;
+ unsigned long flags;
+
+ if (effect->type != FF_RUMBLE)
+ return 0;
+
+ spin_lock_irqsave(&ally_x->lock, flags);
+ ally_x->ff_packet->ff.magnitude_strong = effect->u.rumble.strong_magnitude / 512;
+ ally_x->ff_packet->ff.magnitude_weak = effect->u.rumble.weak_magnitude / 512;
+ ally_x->update_ff = true;
+ spin_unlock_irqrestore(&ally_x->lock, flags);
+
+ if (ally_x->output_worker_initialized)
+ schedule_work(&ally_x->output_worker);
+
+ return 0;
+}
+
+static void ally_x_work(struct work_struct *work)
+{
+ struct ally_x_device *ally_x = container_of(work, struct ally_x_device, output_worker);
+ struct ff_report *ff_report = NULL;
+ bool update_qam = false;
+ bool update_ff = false;
+ unsigned long flags;
+
+ spin_lock_irqsave(&ally_x->lock, flags);
+ update_ff = ally_x->update_ff;
+ if (ally_x->update_ff) {
+ ff_report = kmemdup(ally_x->ff_packet, sizeof(*ally_x->ff_packet), GFP_KERNEL);
+ ally_x->update_ff = false;
+ }
+ update_qam = ally_x->update_qam_btn;
+ spin_unlock_irqrestore(&ally_x->lock, flags);
+
+ if (update_ff && ff_report) {
+ ff_report->ff.magnitude_left = ff_report->ff.magnitude_strong;
+ ff_report->ff.magnitude_right = ff_report->ff.magnitude_weak;
+ asus_dev_set_report(ally_x->hdev, (u8 *)ff_report, sizeof(*ff_report));
+ }
+ kfree(ff_report);
+
+ if (update_qam) {
+ /*
+ * The sleeps here are required to allow steam to register the button combo.
+ */
+ usleep_range(1000, 2000);
+ input_report_key(ally_x->input, BTN_MODE, 1);
+ input_sync(ally_x->input);
+
+ msleep(80);
+ input_report_key(ally_x->input, BTN_A, 1);
+ input_sync(ally_x->input);
+
+ msleep(80);
+ input_report_key(ally_x->input, BTN_A, 0);
+ input_sync(ally_x->input);
+
+ msleep(80);
+ input_report_key(ally_x->input, BTN_MODE, 0);
+ input_sync(ally_x->input);
+
+ spin_lock_irqsave(&ally_x->lock, flags);
+ ally_x->update_qam_btn = false;
+ spin_unlock_irqrestore(&ally_x->lock, flags);
+ }
+}
+
+static struct input_dev *ally_x_setup_input(struct hid_device *hdev)
+{
+ int ret, abs_min = 0, js_abs_max = 65535, tr_abs_max = 1023;
+ struct input_dev *input;
+
+ input = ally_x_alloc_input_dev(hdev, NULL);
+ if (IS_ERR(input))
+ return ERR_CAST(input);
+
+ input_set_abs_params(input, ABS_X, abs_min, js_abs_max, 0, 0);
+ input_set_abs_params(input, ABS_Y, abs_min, js_abs_max, 0, 0);
+ input_set_abs_params(input, ABS_RX, abs_min, js_abs_max, 0, 0);
+ input_set_abs_params(input, ABS_RY, abs_min, js_abs_max, 0, 0);
+ input_set_abs_params(input, ABS_Z, abs_min, tr_abs_max, 0, 0);
+ input_set_abs_params(input, ABS_RZ, abs_min, tr_abs_max, 0, 0);
+ input_set_abs_params(input, ABS_HAT0X, -1, 1, 0, 0);
+ input_set_abs_params(input, ABS_HAT0Y, -1, 1, 0, 0);
+ input_set_capability(input, EV_KEY, BTN_A);
+ input_set_capability(input, EV_KEY, BTN_B);
+ input_set_capability(input, EV_KEY, BTN_X);
+ input_set_capability(input, EV_KEY, BTN_Y);
+ input_set_capability(input, EV_KEY, BTN_TL);
+ input_set_capability(input, EV_KEY, BTN_TR);
+ input_set_capability(input, EV_KEY, BTN_SELECT);
+ input_set_capability(input, EV_KEY, BTN_START);
+ input_set_capability(input, EV_KEY, BTN_MODE);
+ input_set_capability(input, EV_KEY, BTN_THUMBL);
+ input_set_capability(input, EV_KEY, BTN_THUMBR);
+
+ input_set_capability(input, EV_KEY, KEY_PROG1);
+ input_set_capability(input, EV_KEY, KEY_F16);
+ input_set_capability(input, EV_KEY, KEY_F17);
+ input_set_capability(input, EV_KEY, KEY_F18);
+
+ input_set_capability(input, EV_FF, FF_RUMBLE);
+ input_ff_create_memless(input, NULL, ally_x_play_effect);
+
+ ret = input_register_device(input);
+ if (ret)
+ return ERR_PTR(ret);
+
+ return input;
+}
+
+static ssize_t ally_x_qam_mode_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ struct ally_x_device *ally_x = drvdata.ally_x;
+
+ return sysfs_emit(buf, "%d\n", ally_x->qam_btns_steam_mode);
+}
+
+static ssize_t ally_x_qam_mode_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct ally_x_device *ally_x = drvdata.ally_x;
+ bool val;
+ int ret;
+
+ ret = kstrtobool(buf, &val);
+ if (ret < 0)
+ return ret;
+
+ ally_x->qam_btns_steam_mode = val;
+
+ return count;
+}
+ALLY_DEVICE_ATTR_RW(ally_x_qam_mode, qam_mode);
+
+static struct ally_x_device *ally_x_create(struct hid_device *hdev)
+{
+ uint8_t max_output_report_size;
+ struct ally_x_device *ally_x;
+ struct ff_report *report;
+ int ret;
+
+ ally_x = devm_kzalloc(&hdev->dev, sizeof(*ally_x), GFP_KERNEL);
+ if (!ally_x)
+ return ERR_PTR(-ENOMEM);
+
+ ally_x->hdev = hdev;
+ INIT_WORK(&ally_x->output_worker, ally_x_work);
+ spin_lock_init(&ally_x->lock);
+ ally_x->output_worker_initialized = true;
+ ally_x->qam_btns_steam_mode =
+ true; /* Always default to steam mode, it can be changed by userspace attr */
+
+ max_output_report_size = sizeof(struct ally_x_input_report);
+ report = devm_kzalloc(&hdev->dev, sizeof(*report), GFP_KERNEL);
+ if (!report) {
+ ret = -ENOMEM;
+ goto free_ally_x;
+ }
+
+ /* None of these bytes will change for the FF command for now */
+ report->report_id = 0x0D;
+ report->ff.enable = 0x0F; /* Enable all by default */
+ report->ff.pulse_sustain_10ms = 0xFF; /* Duration */
+ report->ff.pulse_release_10ms = 0x00; /* Start Delay */
+ report->ff.loop_count = 0xEB; /* Loop Count */
+ ally_x->ff_packet = report;
+
+ ally_x->input = ally_x_setup_input(hdev);
+ if (IS_ERR(ally_x->input)) {
+ ret = PTR_ERR(ally_x->input);
+ goto free_ff_packet;
+ }
+
+ if (sysfs_create_file(&hdev->dev.kobj, &dev_attr_ally_x_qam_mode.attr)) {
+ ret = -ENODEV;
+ goto unregister_input;
+ }
+
+ ally_x->update_ff = true;
+ if (ally_x->output_worker_initialized)
+ schedule_work(&ally_x->output_worker);
+
+ hid_info(hdev, "Registered Ally X controller using %s\n",
+ dev_name(&ally_x->input->dev));
+ return ally_x;
+
+unregister_input:
+ input_unregister_device(ally_x->input);
+free_ff_packet:
+ kfree(ally_x->ff_packet);
+free_ally_x:
+ kfree(ally_x);
+ return ERR_PTR(ret);
+}
+
+static void ally_x_remove(struct hid_device *hdev)
+{
+ struct ally_x_device *ally_x = drvdata.ally_x;
+ unsigned long flags;
+
+ spin_lock_irqsave(&ally_x->lock, flags);
+ ally_x->output_worker_initialized = false;
+ spin_unlock_irqrestore(&ally_x->lock, flags);
+ cancel_work_sync(&ally_x->output_worker);
+ sysfs_remove_file(&hdev->dev.kobj, &dev_attr_ally_x_qam_mode.attr);
+}
+
+/**************************************************************************************************/
+/* ROG Ally configuration */
+/**************************************************************************************************/
+static int __gamepad_write_all_to_mcu(struct hid_device *hdev,
+ struct ally_gamepad_cfg *ally_cfg);
+
+static int process_key_code(const struct KeyCode *codes, int code_count, const char *buf_copy,
+ u8 *out, int out_idx)
+{
+ for (int i = 0; i < code_count; i++) {
+ if (strcmp(buf_copy, codes[i].label) == 0) {
+ out[out_idx] = codes[i].code;
+ return 0; // Found
+ }
+ }
+ return -EINVAL; // Not found
+}
+
+static int __string_to_key_code(const char *buf, u8 *out, int out_len)
+{
+ char buf_copy[32];
+ u8 *save_buf;
+
+ if (out_len != BTN_CODE_LEN)
+ return -EINVAL;
+
+ save_buf = kzalloc(out_len, GFP_KERNEL);
+ if (!save_buf)
+ return -ENOMEM;
+ memcpy(save_buf, out, out_len);
+ memset(out, 0, out_len); /* always clear before adjusting */
+
+ strscpy(buf_copy, buf);
+ buf_copy[strcspn(buf_copy, "\n")] = 0;
+
+ /* Gamepad group */
+ out[0] = 0x01;
+ if (process_key_code(gamepad_codes, ARRAY_SIZE(gamepad_codes), buf_copy, out, 1) == 0)
+ goto success;
+
+ /* Keyboard group */
+ out[0] = 0x02;
+ if (process_key_code(keyboard_codes, ARRAY_SIZE(keyboard_codes), buf_copy, out, 2) == 0)
+ goto success;
+
+ /* Mouse group */
+ out[0] = 0x03;
+ if (process_key_code(mouse_codes, ARRAY_SIZE(mouse_codes), buf_copy, out, 4) == 0)
+ goto success;
+
+ /* Media group */
+ out[0] = 0x05;
+ if (process_key_code(media_codes, ARRAY_SIZE(media_codes), buf_copy, out, 3) == 0)
+ goto success;
+
+ /* Restore bytes if invalid input */
+ memcpy(out, save_buf, out_len);
+ kfree(save_buf);
+ return -EINVAL;
+
+success:
+ kfree(save_buf);
+ return 0;
+}
+
+static const char *key_code_to_string(const struct KeyCode *codes, int code_count, u8 code)
+{
+ for (int i = 0; i < code_count; i++) {
+ if (codes[i].code == code)
+ return codes[i].label;
+ }
+ return "";
+}
+
+static u8 *__get_btn_block(struct ally_gamepad_cfg *ally_cfg, enum btn_pair pair,
+ enum btn_pair_side side, bool secondary)
+{
+ int offs;
+
+ offs = side ? MAPPING_BLOCK_LEN / 2 : 0;
+ offs = secondary ? offs + BTN_CODE_LEN : offs;
+ return ally_cfg->key_mapping[ally_cfg->mode - 1][pair - 1] + offs;
+}
+
+static const char *__btn_map_to_string(struct ally_gamepad_cfg *ally_cfg, enum btn_pair pair,
+ enum btn_pair_side side, bool secondary)
+{
+ u8 *out_arg = __get_btn_block(ally_cfg, pair, side, secondary);
+
+ switch (out_arg[0]) {
+ case 0x01: // Gamepad buttons
+ return key_code_to_string(gamepad_codes, ARRAY_SIZE(gamepad_codes), out_arg[1]);
+ case 0x02: // Keyboard keys
+ return key_code_to_string(keyboard_codes, ARRAY_SIZE(keyboard_codes),
+ out_arg[2]);
+ case 0x03: // Mouse buttons
+ return key_code_to_string(mouse_codes, ARRAY_SIZE(mouse_codes), out_arg[4]);
+ case 0x05: // Media controls
+ return key_code_to_string(media_codes, ARRAY_SIZE(media_codes), out_arg[3]);
+ default:
+ return "";
+ }
+}
+
+/* ASUS ROG Ally device specific attributes */
+
+/* This should be called before any attempts to set device functions */
+static int __gamepad_check_ready(struct hid_device *hdev)
+{
+ int ret, count;
+ u8 *hidbuf;
+
+ hidbuf = kzalloc(FEATURE_ROG_ALLY_REPORT_SIZE, GFP_KERNEL);
+ if (!hidbuf)
+ return -ENOMEM;
+
+ ret = 0;
+ for (count = 0; count < READY_MAX_TRIES; count++) {
+ hidbuf[0] = FEATURE_ROG_ALLY_REPORT_ID;
+ hidbuf[1] = FEATURE_ROG_ALLY_CODE_PAGE;
+ hidbuf[2] = xpad_cmd_check_ready;
+ hidbuf[3] = 01;
+ ret = asus_dev_set_report(hdev, hidbuf, FEATURE_ROG_ALLY_REPORT_SIZE);
+ if (ret < 0)
+ hid_warn(hdev, "ROG Ally check failed set report: %d\n", ret);
+
+ hidbuf[0] = hidbuf[1] = hidbuf[2] = hidbuf[3] = 0;
+ ret = asus_dev_get_report(hdev, hidbuf, FEATURE_ROG_ALLY_REPORT_SIZE);
+ if (ret < 0)
+ hid_warn(hdev, "ROG Ally check failed get report: %d\n", ret);
+
+ ret = hidbuf[2] == xpad_cmd_check_ready;
+ if (ret)
+ break;
+ usleep_range(
+ 1000,
+ 2000); /* don't spam the entire loop in less than USB response time */
+ }
+
+ if (count == READY_MAX_TRIES)
+ hid_warn(hdev, "ROG Ally never responded with a ready\n");
+
+ kfree(hidbuf);
+ return ret;
+}
+
+/* BUTTON REMAPPING *******************************************************************************/
+static void __btn_pair_to_pkt(struct ally_gamepad_cfg *ally_cfg, enum btn_pair pair, u8 *out,
+ int out_len)
+{
+ out[0] = FEATURE_ROG_ALLY_REPORT_ID;
+ out[1] = FEATURE_ROG_ALLY_CODE_PAGE;
+ out[2] = xpad_cmd_set_mapping;
+ out[3] = pair;
+ out[4] = xpad_cmd_len_mapping;
+ memcpy(&out[5], &ally_cfg->key_mapping[ally_cfg->mode - 1][pair - 1],
+ MAPPING_BLOCK_LEN);
+}
+
+/* Store the button setting in driver data. Does not apply to device until __gamepad_set_mapping */
+static int __gamepad_mapping_store(struct ally_gamepad_cfg *ally_cfg, const char *buf,
+ enum btn_pair pair, int side, bool secondary)
+{
+ u8 *out_arg;
+
+ out_arg = __get_btn_block(ally_cfg, pair, side, secondary);
+
+ return __string_to_key_code(buf, out_arg, BTN_CODE_LEN);
+}
+
+/* Apply the mapping pair to the device */
+static int __gamepad_set_mapping(struct hid_device *hdev, struct ally_gamepad_cfg *ally_cfg,
+ enum btn_pair pair)
+{
+ u8 *hidbuf;
+ int ret;
+
+ ret = __gamepad_check_ready(hdev);
+ if (ret < 0)
+ return ret;
+
+ hidbuf = kzalloc(FEATURE_ROG_ALLY_REPORT_SIZE, GFP_KERNEL);
+ if (!hidbuf)
+ return -ENOMEM;
+
+ __btn_pair_to_pkt(ally_cfg, pair, hidbuf, FEATURE_ROG_ALLY_REPORT_SIZE);
+ ret = asus_dev_set_report(hdev, hidbuf, FEATURE_ROG_ALLY_REPORT_SIZE);
+
+ kfree(hidbuf);
+
+ return ret;
+}
+
+static ssize_t btn_mapping_apply_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct ally_gamepad_cfg *ally_cfg = drvdata.gamepad_cfg;
+ struct hid_device *hdev = to_hid_device(dev);
+ int ret;
+
+ if (!drvdata.gamepad_cfg)
+ return -ENODEV;
+
+ ret = __gamepad_write_all_to_mcu(hdev, ally_cfg);
+ if (ret < 0)
+ return ret;
+
+ return count;
+}
+ALLY_DEVICE_ATTR_WO(btn_mapping_apply, apply_all);
+
+/* BUTTON TURBO ***********************************************************************************/
+static int __btn_turbo_index(enum btn_pair pair, int side)
+{
+ return (pair - 1) * (2 * TURBO_BLOCK_STEP) + (side * TURBO_BLOCK_STEP);
+};
+
+static int __gamepad_turbo_show(struct device *dev, enum btn_pair pair, int side)
+{
+ struct ally_gamepad_cfg *ally_cfg = drvdata.gamepad_cfg;
+
+ if (!drvdata.gamepad_cfg)
+ return -ENODEV;
+
+ return ally_cfg->turbo_btns[ally_cfg->mode - 1][__btn_turbo_index(pair, side)];
+};
+
+static int __gamepad_turbo_store(struct device *dev, const char *buf, enum btn_pair pair,
+ int side)
+{
+ struct ally_gamepad_cfg *ally_cfg = drvdata.gamepad_cfg;
+ int ret, val;
+
+ if (!drvdata.gamepad_cfg)
+ return -ENODEV;
+
+ ret = kstrtoint(buf, 0, &val);
+ if (ret)
+ return ret;
+ if (val < 0 || val > 16)
+ return -EINVAL;
+
+ ally_cfg->turbo_btns[ally_cfg->mode - 1][__btn_turbo_index(pair, side)] = val;
+
+ return 0;
+};
+
+/* button map attributes, regular and macro*/
+ALLY_BTN_MAPPING(m2, btn_pair_m1_m2, btn_pair_side_left);
+ALLY_BTN_MAPPING(m1, btn_pair_m1_m2, btn_pair_side_right);
+ALLY_BTN_MAPPING(a, btn_pair_a_b, btn_pair_side_left);
+ALLY_BTN_MAPPING(b, btn_pair_a_b, btn_pair_side_right);
+ALLY_BTN_MAPPING(x, btn_pair_x_y, btn_pair_side_left);
+ALLY_BTN_MAPPING(y, btn_pair_x_y, btn_pair_side_right);
+ALLY_BTN_MAPPING(lb, btn_pair_lb_rb, btn_pair_side_left);
+ALLY_BTN_MAPPING(rb, btn_pair_lb_rb, btn_pair_side_right);
+ALLY_BTN_MAPPING(ls, btn_pair_ls_rs, btn_pair_side_left);
+ALLY_BTN_MAPPING(rs, btn_pair_ls_rs, btn_pair_side_right);
+ALLY_BTN_MAPPING(lt, btn_pair_lt_rt, btn_pair_side_left);
+ALLY_BTN_MAPPING(rt, btn_pair_lt_rt, btn_pair_side_right);
+ALLY_BTN_MAPPING(dpad_u, btn_pair_dpad_u_d, btn_pair_side_left);
+ALLY_BTN_MAPPING(dpad_d, btn_pair_dpad_u_d, btn_pair_side_right);
+ALLY_BTN_MAPPING(dpad_l, btn_pair_dpad_l_r, btn_pair_side_left);
+ALLY_BTN_MAPPING(dpad_r, btn_pair_dpad_l_r, btn_pair_side_right);
+ALLY_BTN_MAPPING(view, btn_pair_view_menu, btn_pair_side_left);
+ALLY_BTN_MAPPING(menu, btn_pair_view_menu, btn_pair_side_right);
+
+static void __gamepad_mapping_xpad_default(struct ally_gamepad_cfg *ally_cfg)
+{
+ memcpy(&ally_cfg->key_mapping[0][0], &XPAD_DEF1, MAPPING_BLOCK_LEN);
+ memcpy(&ally_cfg->key_mapping[0][1], &XPAD_DEF2, MAPPING_BLOCK_LEN);
+ memcpy(&ally_cfg->key_mapping[0][2], &XPAD_DEF3, MAPPING_BLOCK_LEN);
+ memcpy(&ally_cfg->key_mapping[0][3], &XPAD_DEF4, MAPPING_BLOCK_LEN);
+ memcpy(&ally_cfg->key_mapping[0][4], &XPAD_DEF5, MAPPING_BLOCK_LEN);
+ memcpy(&ally_cfg->key_mapping[0][5], &XPAD_DEF6, MAPPING_BLOCK_LEN);
+ memcpy(&ally_cfg->key_mapping[0][6], &XPAD_DEF7, MAPPING_BLOCK_LEN);
+ memcpy(&ally_cfg->key_mapping[0][7], &XPAD_DEF8, MAPPING_BLOCK_LEN);
+ memcpy(&ally_cfg->key_mapping[0][8], &XPAD_DEF9, MAPPING_BLOCK_LEN);
+}
+
+static void __gamepad_mapping_wasd_default(struct ally_gamepad_cfg *ally_cfg)
+{
+ memcpy(&ally_cfg->key_mapping[1][0], &WASD_DEF1, MAPPING_BLOCK_LEN);
+ memcpy(&ally_cfg->key_mapping[1][1], &WASD_DEF2, MAPPING_BLOCK_LEN);
+ memcpy(&ally_cfg->key_mapping[1][2], &WASD_DEF3, MAPPING_BLOCK_LEN);
+ memcpy(&ally_cfg->key_mapping[1][3], &WASD_DEF4, MAPPING_BLOCK_LEN);
+ memcpy(&ally_cfg->key_mapping[1][4], &WASD_DEF5, MAPPING_BLOCK_LEN);
+ memcpy(&ally_cfg->key_mapping[1][5], &WASD_DEF6, MAPPING_BLOCK_LEN);
+ memcpy(&ally_cfg->key_mapping[1][6], &WASD_DEF7, MAPPING_BLOCK_LEN);
+ memcpy(&ally_cfg->key_mapping[1][7], &WASD_DEF8, MAPPING_BLOCK_LEN);
+ memcpy(&ally_cfg->key_mapping[1][8], &WASD_DEF9, MAPPING_BLOCK_LEN);
+}
+
+static ssize_t btn_mapping_reset_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct ally_gamepad_cfg *ally_cfg = drvdata.gamepad_cfg;
+
+ if (!drvdata.gamepad_cfg)
+ return -ENODEV;
+
+ switch (ally_cfg->mode) {
+ case xpad_mode_game:
+ __gamepad_mapping_xpad_default(ally_cfg);
+ break;
+ case xpad_mode_wasd:
+ __gamepad_mapping_wasd_default(ally_cfg);
+ break;
+ default:
+ __gamepad_mapping_xpad_default(ally_cfg);
+ break;
+ }
+
+ return count;
+}
+
+ALLY_DEVICE_ATTR_WO(btn_mapping_reset, reset_btn_mapping);
+
+/* GAMEPAD MODE ***********************************************************************************/
+static ssize_t __gamepad_set_mode(struct hid_device *hdev, struct ally_gamepad_cfg *ally_cfg,
+ int val)
+{
+ u8 *hidbuf;
+ int ret;
+
+ ret = __gamepad_check_ready(hdev);
+ if (ret < 0)
+ return ret;
+
+ hidbuf = kzalloc(FEATURE_ROG_ALLY_REPORT_SIZE, GFP_KERNEL);
+ if (!hidbuf)
+ return -ENOMEM;
+
+ hidbuf[0] = FEATURE_ROG_ALLY_REPORT_ID;
+ hidbuf[1] = FEATURE_ROG_ALLY_CODE_PAGE;
+ hidbuf[2] = xpad_cmd_set_mode;
+ hidbuf[3] = xpad_cmd_len_mode;
+ hidbuf[4] = val;
+
+ ret = __gamepad_check_ready(hdev);
+ if (ret < 0)
+ goto report_fail;
+
+ ret = asus_dev_set_report(hdev, hidbuf, FEATURE_ROG_ALLY_REPORT_SIZE);
+ if (ret < 0)
+ goto report_fail;
+
+ ret = __gamepad_write_all_to_mcu(hdev, ally_cfg);
+ if (ret < 0)
+ goto report_fail;
+
+report_fail:
+ kfree(hidbuf);
+ return ret;
+}
+
+static ssize_t gamepad_mode_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+ struct ally_gamepad_cfg *ally_cfg = drvdata.gamepad_cfg;
+
+ if (!drvdata.gamepad_cfg)
+ return -ENODEV;
+
+ return sysfs_emit(buf, "%d\n", ally_cfg->mode);
+}
+
+static ssize_t gamepad_mode_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct hid_device *hdev = to_hid_device(dev);
+ struct ally_gamepad_cfg *ally_cfg = drvdata.gamepad_cfg;
+ int ret, val;
+
+ if (!drvdata.gamepad_cfg)
+ return -ENODEV;
+
+ ret = kstrtoint(buf, 0, &val);
+ if (ret)
+ return ret;
+
+ if (val < xpad_mode_game || val > xpad_mode_mouse)
+ return -EINVAL;
+
+ ally_cfg->mode = val;
+
+ ret = __gamepad_set_mode(hdev, ally_cfg, val);
+ if (ret < 0)
+ return ret;
+
+ return count;
+}
+
+DEVICE_ATTR_RW(gamepad_mode);
+
+/* VIBRATION INTENSITY ****************************************************************************/
+static ssize_t gamepad_vibration_intensity_index_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ return sysfs_emit(buf, "left right\n");
+}
+
+ALLY_DEVICE_ATTR_RO(gamepad_vibration_intensity_index, vibration_intensity_index);
+
+static ssize_t __gamepad_write_vibe_intensity_to_mcu(struct hid_device *hdev,
+ struct ally_gamepad_cfg *ally_cfg)
+{
+ u8 *hidbuf;
+ int ret;
+
+ ret = __gamepad_check_ready(hdev);
+ if (ret < 0)
+ return ret;
+
+ hidbuf = kzalloc(FEATURE_ROG_ALLY_REPORT_SIZE, GFP_KERNEL);
+ if (!hidbuf)
+ return -ENOMEM;
+
+ hidbuf[0] = FEATURE_ROG_ALLY_REPORT_ID;
+ hidbuf[1] = FEATURE_ROG_ALLY_CODE_PAGE;
+ hidbuf[2] = xpad_cmd_set_vibe_intensity;
+ hidbuf[3] = xpad_cmd_len_vibe_intensity;
+ hidbuf[4] = ally_cfg->vibration_intensity[ally_cfg->mode - 1][btn_pair_side_left];
+ hidbuf[5] = ally_cfg->vibration_intensity[ally_cfg->mode - 1][btn_pair_side_right];
+
+ ret = __gamepad_check_ready(hdev);
+ if (ret < 0)
+ goto report_fail;
+
+ ret = asus_dev_set_report(hdev, hidbuf, FEATURE_ROG_ALLY_REPORT_SIZE);
+ if (ret < 0)
+ goto report_fail;
+
+report_fail:
+ kfree(hidbuf);
+ return ret;
+}
+
+static ssize_t gamepad_vibration_intensity_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct ally_gamepad_cfg *ally_cfg = drvdata.gamepad_cfg;
+
+ if (!drvdata.gamepad_cfg)
+ return -ENODEV;
+
+ return sysfs_emit(
+ buf, "%d %d\n",
+ ally_cfg->vibration_intensity[ally_cfg->mode - 1][btn_pair_side_left],
+ ally_cfg->vibration_intensity[ally_cfg->mode - 1][btn_pair_side_right]);
+}
+
+static ssize_t gamepad_vibration_intensity_store(struct device *dev,
+ struct device_attribute *attr, const char *buf,
+ size_t count)
+{
+ struct hid_device *hdev = to_hid_device(dev);
+ struct ally_gamepad_cfg *ally_cfg = drvdata.gamepad_cfg;
+ u32 left, right;
+ int ret;
+
+ if (!drvdata.gamepad_cfg)
+ return -ENODEV;
+
+ if (sscanf(buf, "%d %d", &left, &right) != 2)
+ return -EINVAL;
+
+ if (left > 64 || right > 64)
+ return -EINVAL;
+
+ ally_cfg->vibration_intensity[ally_cfg->mode - 1][btn_pair_side_left] = left;
+ ally_cfg->vibration_intensity[ally_cfg->mode - 1][btn_pair_side_right] = right;
+
+ ret = __gamepad_write_vibe_intensity_to_mcu(hdev, ally_cfg);
+ if (ret < 0)
+ return ret;
+
+ return count;
+}
+
+ALLY_DEVICE_ATTR_RW(gamepad_vibration_intensity, vibration_intensity);
+
+/* ROOT LEVEL ATTRS *******************************************************************************/
+static struct attribute *gamepad_device_attrs[] = {
+ &dev_attr_gamepad_mode.attr,
+ &dev_attr_btn_mapping_reset.attr,
+ &dev_attr_btn_mapping_apply.attr,
+ &dev_attr_gamepad_vibration_intensity.attr,
+ &dev_attr_gamepad_vibration_intensity_index.attr,
+ NULL
+};
+
+static const struct attribute_group ally_controller_attr_group = {
+ .attrs = gamepad_device_attrs,
+};
+
+/* ANALOGUE DEADZONES *****************************************************************************/
+static ssize_t __gamepad_set_deadzones(struct hid_device *hdev,
+ struct ally_gamepad_cfg *ally_cfg)
+{
+ u8 *hidbuf;
+ int ret;
+
+ ret = __gamepad_check_ready(hdev);
+ if (ret < 0)
+ return ret;
+
+ hidbuf = kzalloc(FEATURE_ROG_ALLY_REPORT_SIZE, GFP_KERNEL);
+ if (!hidbuf)
+ return -ENOMEM;
+
+ hidbuf[0] = FEATURE_ROG_ALLY_REPORT_ID;
+ hidbuf[1] = FEATURE_ROG_ALLY_CODE_PAGE;
+ hidbuf[2] = xpad_cmd_set_js_dz;
+ hidbuf[3] = xpad_cmd_len_deadzone;
+ hidbuf[4] = ally_cfg->deadzones[ally_cfg->mode - 1][0][0];
+ hidbuf[5] = ally_cfg->deadzones[ally_cfg->mode - 1][0][1];
+ hidbuf[6] = ally_cfg->deadzones[ally_cfg->mode - 1][0][2];
+ hidbuf[7] = ally_cfg->deadzones[ally_cfg->mode - 1][0][3];
+
+ ret = asus_dev_set_report(hdev, hidbuf, FEATURE_ROG_ALLY_REPORT_SIZE);
+ if (ret < 0)
+ goto end;
+
+ hidbuf[2] = xpad_cmd_set_tr_dz;
+ hidbuf[4] = ally_cfg->deadzones[ally_cfg->mode - 1][1][0];
+ hidbuf[5] = ally_cfg->deadzones[ally_cfg->mode - 1][1][1];
+ hidbuf[6] = ally_cfg->deadzones[ally_cfg->mode - 1][1][2];
+ hidbuf[7] = ally_cfg->deadzones[ally_cfg->mode - 1][1][3];
+
+ ret = asus_dev_set_report(hdev, hidbuf, FEATURE_ROG_ALLY_REPORT_SIZE);
+ if (ret < 0)
+ goto end;
+
+end:
+ kfree(hidbuf);
+ return ret;
+}
+
+static ssize_t __gamepad_store_deadzones(struct ally_gamepad_cfg *ally_cfg, enum xpad_axis axis,
+ const char *buf)
+{
+ int cmd, side, is_tr;
+ u32 inner, outer;
+
+ if (sscanf(buf, "%d %d", &inner, &outer) != 2)
+ return -EINVAL;
+
+ if (inner > 64 || outer > 64 || inner > outer)
+ return -EINVAL;
+
+ is_tr = axis > xpad_axis_xy_right;
+ side = axis == xpad_axis_xy_right || axis == xpad_axis_z_right ? 2 : 0;
+ cmd = is_tr ? xpad_cmd_set_js_dz : xpad_cmd_set_tr_dz;
+
+ ally_cfg->deadzones[ally_cfg->mode - 1][is_tr][side] = inner;
+ ally_cfg->deadzones[ally_cfg->mode - 1][is_tr][side + 1] = outer;
+
+ return 0;
+}
+
+static ssize_t axis_xyz_deadzone_index_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ return sysfs_emit(buf, "inner outer\n");
+}
+
+ALLY_DEVICE_ATTR_RO(axis_xyz_deadzone_index, deadzone_index);
+
+ALLY_AXIS_DEADZONE(xpad_axis_xy_left, deadzone);
+ALLY_AXIS_DEADZONE(xpad_axis_xy_right, deadzone);
+ALLY_AXIS_DEADZONE(xpad_axis_z_left, deadzone);
+ALLY_AXIS_DEADZONE(xpad_axis_z_right, deadzone);
+
+/* ANTI-DEADZONES *********************************************************************************/
+static ssize_t __gamepad_write_js_ADZ_to_mcu(struct hid_device *hdev,
+ struct ally_gamepad_cfg *ally_cfg)
+{
+ u8 *hidbuf;
+ int ret;
+
+ ret = __gamepad_check_ready(hdev);
+ if (ret < 0)
+ return ret;
+
+ hidbuf = kzalloc(FEATURE_ROG_ALLY_REPORT_SIZE, GFP_KERNEL);
+ if (!hidbuf)
+ return -ENOMEM;
+
+ hidbuf[0] = FEATURE_ROG_ALLY_REPORT_ID;
+ hidbuf[1] = FEATURE_ROG_ALLY_CODE_PAGE;
+ hidbuf[2] = xpad_cmd_set_adz;
+ hidbuf[3] = xpad_cmd_len_adz;
+ hidbuf[4] = ally_cfg->anti_deadzones[ally_cfg->mode - 1][btn_pair_side_left];
+ hidbuf[5] = ally_cfg->anti_deadzones[ally_cfg->mode - 1][btn_pair_side_right];
+
+ ret = __gamepad_check_ready(hdev);
+ if (ret < 0)
+ goto report_fail;
+
+ ret = asus_dev_set_report(hdev, hidbuf, FEATURE_ROG_ALLY_REPORT_SIZE);
+ if (ret < 0)
+ goto report_fail;
+
+report_fail:
+ kfree(hidbuf);
+ return ret;
+}
+
+static ssize_t __gamepad_js_ADZ_store(struct device *dev, const char *buf,
+ enum btn_pair_side side)
+{
+ struct ally_gamepad_cfg *ally_cfg = drvdata.gamepad_cfg;
+ int ret, val;
+
+ if (!drvdata.gamepad_cfg)
+ return -ENODEV;
+
+ ret = kstrtoint(buf, 0, &val);
+ if (ret)
+ return ret;
+
+ if (val < 0 || val > 32)
+ return -EINVAL;
+
+ ally_cfg->anti_deadzones[ally_cfg->mode - 1][side] = val;
+
+ return ret;
+}
+
+static ssize_t xpad_axis_xy_left_ADZ_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ struct ally_gamepad_cfg *ally_cfg = drvdata.gamepad_cfg;
+
+ if (!drvdata.gamepad_cfg)
+ return -ENODEV;
+
+ return sysfs_emit(buf, "%d\n",
+ ally_cfg->anti_deadzones[ally_cfg->mode - 1][btn_pair_side_left]);
+}
+
+static ssize_t xpad_axis_xy_left_ADZ_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ int ret = __gamepad_js_ADZ_store(dev, buf, btn_pair_side_left);
+
+ if (ret)
+ return ret;
+
+ return count;
+}
+
+ALLY_DEVICE_ATTR_RW(xpad_axis_xy_left_ADZ, anti_deadzone);
+
+static ssize_t xpad_axis_xy_right_ADZ_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ struct ally_gamepad_cfg *ally_cfg = drvdata.gamepad_cfg;
+
+ if (!drvdata.gamepad_cfg)
+ return -ENODEV;
+
+ return sysfs_emit(buf, "%d\n",
+ ally_cfg->anti_deadzones[ally_cfg->mode - 1][btn_pair_side_right]);
+}
+
+static ssize_t xpad_axis_xy_right_ADZ_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ int ret = __gamepad_js_ADZ_store(dev, buf, btn_pair_side_right);
+
+ if (ret)
+ return ret;
+
+ return count;
+}
+
+ALLY_DEVICE_ATTR_RW(xpad_axis_xy_right_ADZ, anti_deadzone);
+
+/* JS RESPONSE CURVES *****************************************************************************/
+static ssize_t rc_point_index_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+ return sysfs_emit(buf, "move response\n");
+}
+
+ALLY_DEVICE_ATTR_RO(rc_point_index, rc_point_index);
+
+static ssize_t __gamepad_write_response_curves_to_mcu(struct hid_device *hdev,
+ struct ally_gamepad_cfg *ally_cfg)
+{
+ u8 *hidbuf;
+ int ret;
+
+ ret = __gamepad_check_ready(hdev);
+ if (ret < 0)
+ return ret;
+
+ hidbuf = kzalloc(FEATURE_ROG_ALLY_REPORT_SIZE, GFP_KERNEL);
+ if (!hidbuf)
+ return -ENOMEM;
+
+ hidbuf[0] = FEATURE_ROG_ALLY_REPORT_ID;
+ hidbuf[1] = FEATURE_ROG_ALLY_CODE_PAGE;
+ hidbuf[2] = xpad_cmd_set_response_curve;
+ hidbuf[3] = xpad_cmd_len_response_curve;
+ hidbuf[4] = 0x01;
+ memcpy(&hidbuf[5], &ally_cfg->response_curve[ally_cfg->mode - 1][btn_pair_side_left],
+ 8);
+
+ ret = __gamepad_check_ready(hdev);
+ if (ret < 0)
+ goto report_fail;
+
+ hidbuf[4] = 0x02;
+ memcpy(&hidbuf[5], &ally_cfg->response_curve[ally_cfg->mode - 1][btn_pair_side_right],
+ 8);
+
+ ret = __gamepad_check_ready(hdev);
+ if (ret < 0)
+ goto report_fail;
+
+ ret = asus_dev_set_report(hdev, hidbuf, FEATURE_ROG_ALLY_REPORT_SIZE);
+ if (ret < 0)
+ goto report_fail;
+
+report_fail:
+ kfree(hidbuf);
+ return ret;
+}
+
+static ssize_t __gamepad_store_response_curve(struct device *dev, const char *buf,
+ enum btn_pair_side side, int point)
+{
+ struct ally_gamepad_cfg *ally_cfg = drvdata.gamepad_cfg;
+ u32 move, response;
+ int idx;
+
+ idx = (point - 1) * 2;
+
+ if (!drvdata.gamepad_cfg)
+ return -ENODEV;
+
+ if (sscanf(buf, "%d %d", &move, &response) != 2)
+ return -EINVAL;
+
+ if (move > 64 || response > 64)
+ return -EINVAL;
+
+ ally_cfg->response_curve[ally_cfg->mode - 1][side][idx] = move;
+ ally_cfg->response_curve[ally_cfg->mode - 1][side][idx + 1] = response;
+
+ return 0;
+}
+
+ALLY_JS_RC_POINT(left, 1, rc_point_);
+ALLY_JS_RC_POINT(left, 2, rc_point_);
+ALLY_JS_RC_POINT(left, 3, rc_point_);
+ALLY_JS_RC_POINT(left, 4, rc_point_);
+
+ALLY_JS_RC_POINT(right, 1, rc_point_);
+ALLY_JS_RC_POINT(right, 2, rc_point_);
+ALLY_JS_RC_POINT(right, 3, rc_point_);
+ALLY_JS_RC_POINT(right, 4, rc_point_);
+
+/* CALIBRATIONS ***********************************************************************************/
+static int __gamepad_get_calibration(struct hid_device *hdev)
+{
+ struct ally_gamepad_cfg *ally_cfg = drvdata.gamepad_cfg;
+ u8 *hidbuf;
+ int ret, i;
+
+ if (!drvdata.gamepad_cfg)
+ return -ENODEV;
+
+ hidbuf = kzalloc(FEATURE_ROG_ALLY_REPORT_SIZE, GFP_KERNEL);
+ if (!hidbuf)
+ return -ENOMEM;
+
+ for (i = 0; i < 2; i++) {
+ hidbuf[0] = FEATURE_ROG_ALLY_REPORT_ID;
+ hidbuf[1] = 0xD0;
+ hidbuf[2] = 0x03;
+ hidbuf[3] = i + 1; // 0x01 JS, 0x02 TR
+ hidbuf[4] = 0x20;
+
+ ret = asus_dev_set_report(hdev, hidbuf, FEATURE_ROG_ALLY_REPORT_SIZE);
+ if (ret < 0) {
+ hid_warn(hdev, "ROG Ally check failed set report: %d\n", ret);
+ goto cleanup;
+ }
+
+ memset(hidbuf, 0, FEATURE_ROG_ALLY_REPORT_SIZE);
+ ret = asus_dev_get_report(hdev, hidbuf, FEATURE_ROG_ALLY_REPORT_SIZE);
+ if (ret < 0 || hidbuf[5] != 1) {
+ hid_warn(hdev, "ROG Ally check failed get report: %d\n", ret);
+ goto cleanup;
+ }
+
+ if (i == 0) {
+ /* Joystick calibration */
+ /* [left][index] is Y: stable, min, max. X: stable, min, max */
+ ally_cfg->js_calibrations[0][3] = (hidbuf[6] << 8) | hidbuf[7];
+ ally_cfg->js_calibrations[0][4] = (hidbuf[8] << 8) | hidbuf[9];
+ ally_cfg->js_calibrations[0][5] = (hidbuf[10] << 8) | hidbuf[11];
+ ally_cfg->js_calibrations[0][0] = (hidbuf[12] << 8) | hidbuf[13];
+ ally_cfg->js_calibrations[0][1] = (hidbuf[14] << 8) | hidbuf[15];
+ ally_cfg->js_calibrations[0][2] = (hidbuf[16] << 8) | hidbuf[17];
+ /* [right][index] is Y: stable, min, max. X: stable, min, max */
+ ally_cfg->js_calibrations[1][0] = (hidbuf[24] << 8) | hidbuf[25];
+ ally_cfg->js_calibrations[1][1] = (hidbuf[26] << 8) | hidbuf[27];
+ ally_cfg->js_calibrations[1][2] = (hidbuf[28] << 8) | hidbuf[29];
+ ally_cfg->js_calibrations[1][3] = (hidbuf[18] << 8) | hidbuf[19];
+ ally_cfg->js_calibrations[1][4] = (hidbuf[20] << 8) | hidbuf[21];
+ ally_cfg->js_calibrations[1][5] = (hidbuf[22] << 8) | hidbuf[23];
+ } else {
+ /* Trigger calibration */
+ /* [left/right][stable/max] */
+ ally_cfg->tr_calibrations[0][0] = (hidbuf[6] << 8) | hidbuf[7];
+ ally_cfg->tr_calibrations[0][1] = (hidbuf[8] << 8) | hidbuf[9];
+ ally_cfg->tr_calibrations[1][0] = (hidbuf[10] << 8) | hidbuf[11];
+ ally_cfg->tr_calibrations[1][1] = (hidbuf[12] << 8) | hidbuf[13];
+ }
+ }
+
+cleanup:
+ kfree(hidbuf);
+ return ret;
+}
+
+static ssize_t __gamepad_write_cal_to_mcu(struct device *dev, enum xpad_axis axis)
+{
+ struct hid_device *hdev = to_hid_device(dev);
+ struct ally_gamepad_cfg *ally_cfg = drvdata.gamepad_cfg;
+ u8 *c, side, pkt_len, data_len;
+ int ret, cal, checksum = 0;
+ u8 *hidbuf;
+ int *head;
+
+ if (!drvdata.gamepad_cfg)
+ return -ENODEV;
+
+ ret = __gamepad_check_ready(hdev);
+ if (ret < 0)
+ return ret;
+
+ hidbuf = kzalloc(FEATURE_ROG_ALLY_REPORT_SIZE, GFP_KERNEL);
+ if (!hidbuf)
+ return -ENOMEM;
+
+ side = axis == xpad_axis_xy_right || axis == xpad_axis_z_right ? 1 : 0;
+ pkt_len = axis > xpad_axis_xy_right ? 0x06 : 0x0E;
+ data_len = axis > xpad_axis_xy_right ? 2 : 6;
+ head = axis > xpad_axis_xy_right ? ally_cfg->tr_calibrations[side] :
+ ally_cfg->js_calibrations[side];
+
+ hidbuf[0] = FEATURE_ROG_ALLY_REPORT_ID;
+ hidbuf[1] = FEATURE_ROG_ALLY_CODE_PAGE;
+ hidbuf[2] = xpad_cmd_set_calibration;
+ hidbuf[3] = pkt_len;
+ hidbuf[4] = 0x01; /* second command (write calibration) */
+ hidbuf[5] = axis;
+ c = &hidbuf[6]; /* pointer to data start */
+
+ for (size_t i = 0; i < data_len; i++) {
+ cal = head[i];
+ *c = (u8)((cal & 0xff00) >> 8);
+ checksum += *c;
+ c += 1;
+ *c = (u8)(cal & 0xff);
+ checksum += *c;
+ c += 1;
+ }
+
+ hidbuf[6 + data_len * 2] = checksum;
+
+ ret = asus_dev_set_report(hdev, hidbuf, FEATURE_ROG_ALLY_REPORT_SIZE);
+ if (ret < 0)
+ goto report_fail;
+
+ memset(hidbuf, 0, FEATURE_ROG_ALLY_REPORT_SIZE);
+ hidbuf[0] = FEATURE_ROG_ALLY_REPORT_ID;
+ hidbuf[1] = FEATURE_ROG_ALLY_CODE_PAGE;
+ hidbuf[2] = xpad_cmd_set_calibration;
+ hidbuf[3] = xpad_cmd_len_calibration3;
+ hidbuf[4] = 0x03; /* second command (apply the calibration that was written) */
+
+ ret = asus_dev_set_report(hdev, hidbuf, FEATURE_ROG_ALLY_REPORT_SIZE);
+ if (ret < 0)
+ goto report_fail;
+
+report_fail:
+ kfree(hidbuf);
+ return ret;
+}
+
+static ssize_t __gamepad_cal_store(struct device *dev, const char *buf, enum xpad_axis axis)
+{
+ struct ally_gamepad_cfg *ally_cfg = drvdata.gamepad_cfg;
+ u32 x_stable, x_min, x_max, y_stable, y_min, y_max, side;
+
+ if (!drvdata.gamepad_cfg)
+ return -ENODEV;
+
+ if (axis == xpad_axis_xy_left || axis == xpad_axis_xy_right) {
+ if (sscanf(buf, "%d %d %d %d %d %d", &x_stable, &x_min, &x_max, &y_stable,
+ &y_min, &y_max) != 6)
+ return -EINVAL;
+
+ side = axis == xpad_axis_xy_right || axis == xpad_axis_z_right ? 1 : 0;
+ /* stored in reverse order for easy copy to packet */
+ ally_cfg->js_calibrations[side][0] = y_stable;
+ ally_cfg->js_calibrations[side][1] = y_min;
+ ally_cfg->js_calibrations[side][2] = y_max;
+ ally_cfg->js_calibrations[side][3] = x_stable;
+ ally_cfg->js_calibrations[side][4] = x_min;
+ ally_cfg->js_calibrations[side][5] = x_max;
+
+ return __gamepad_write_cal_to_mcu(dev, axis);
+ }
+ if (sscanf(buf, "%d %d", &x_stable, &x_max) != 2)
+ return -EINVAL;
+
+ side = axis == xpad_axis_xy_right || axis == xpad_axis_z_right ? 1 : 0;
+ /* stored in reverse order for easy copy to packet */
+ ally_cfg->tr_calibrations[side][0] = x_stable;
+ ally_cfg->tr_calibrations[side][1] = x_max;
+
+ return __gamepad_write_cal_to_mcu(dev, axis);
+}
+
+static ssize_t __gamepad_cal_show(struct device *dev, char *buf, enum xpad_axis axis)
+{
+ struct ally_gamepad_cfg *ally_cfg = drvdata.gamepad_cfg;
+ int side = (axis == xpad_axis_xy_right || axis == xpad_axis_z_right) ? 1 : 0;
+
+ if (!drvdata.gamepad_cfg)
+ return -ENODEV;
+
+ if (axis == xpad_axis_xy_left || axis == xpad_axis_xy_right) {
+ return sysfs_emit(
+ buf, "%d %d %d %d %d %d\n", ally_cfg->js_calibrations[side][3],
+ ally_cfg->js_calibrations[side][4], ally_cfg->js_calibrations[side][5],
+ ally_cfg->js_calibrations[side][0], ally_cfg->js_calibrations[side][1],
+ ally_cfg->js_calibrations[side][2]);
+ }
+
+ return sysfs_emit(buf, "%d %d\n", ally_cfg->tr_calibrations[side][0],
+ ally_cfg->tr_calibrations[side][1]);
+}
+
+ALLY_CAL_ATTR(xpad_axis_xy_left_cal, xpad_axis_xy_left, calibration);
+ALLY_CAL_ATTR(xpad_axis_xy_right_cal, xpad_axis_xy_right, calibration);
+ALLY_CAL_ATTR(xpad_axis_z_left_cal, xpad_axis_z_left, calibration);
+ALLY_CAL_ATTR(xpad_axis_z_right_cal, xpad_axis_z_right, calibration);
+
+static ssize_t xpad_axis_xy_cal_index_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ return sysfs_emit(buf, "x_stable x_min x_max y_stable y_min y_max\n");
+}
+
+ALLY_DEVICE_ATTR_RO(xpad_axis_xy_cal_index, calibration_index);
+
+static ssize_t xpad_axis_z_cal_index_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ return sysfs_emit(buf, "z_stable z_max\n");
+}
+
+ALLY_DEVICE_ATTR_RO(xpad_axis_z_cal_index, calibration_index);
+
+static ssize_t __gamepad_cal_reset(struct device *dev, const char *buf, enum xpad_axis axis)
+{
+ struct hid_device *hdev = to_hid_device(dev);
+ u8 *hidbuf;
+ int ret;
+
+ ret = __gamepad_check_ready(hdev);
+ if (ret < 0)
+ return ret;
+
+ hidbuf = kzalloc(FEATURE_ROG_ALLY_REPORT_SIZE, GFP_KERNEL);
+ if (!hidbuf)
+ return -ENOMEM;
+
+ /* Write the reset value, then apply it */
+ for (u8 cmd = 0x02; cmd <= 0x03; cmd++) {
+ memset(hidbuf, 0, FEATURE_ROG_ALLY_REPORT_SIZE);
+ hidbuf[0] = FEATURE_ROG_ALLY_REPORT_ID;
+ hidbuf[1] = FEATURE_ROG_ALLY_CODE_PAGE;
+ hidbuf[2] = xpad_cmd_set_calibration;
+ hidbuf[3] = (cmd == 0x02) ? xpad_cmd_len_calibration2 :
+ xpad_cmd_len_calibration3;
+ hidbuf[4] = cmd;
+ hidbuf[5] = axis;
+
+ ret = asus_dev_set_report(hdev, hidbuf, FEATURE_ROG_ALLY_REPORT_SIZE);
+ if (ret < 0)
+ break;
+ }
+
+ __gamepad_get_calibration(hdev);
+
+ kfree(hidbuf);
+ return ret;
+}
+
+ALLY_CAL_RESET_ATTR(xpad_axis_xy_left_cal_reset, xpad_axis_xy_left, calibration_reset);
+ALLY_CAL_RESET_ATTR(xpad_axis_xy_right_cal_reset, xpad_axis_xy_right, calibration_reset);
+ALLY_CAL_RESET_ATTR(xpad_axis_z_left_cal_reset, xpad_axis_z_left, calibration_reset);
+ALLY_CAL_RESET_ATTR(xpad_axis_z_right_cal_reset, xpad_axis_z_right, calibration_reset);
+
+static struct attribute *gamepad_axis_xy_left_attrs[] = {
+ &dev_attr_xpad_axis_xy_left_deadzone.attr,
+ &dev_attr_axis_xyz_deadzone_index.attr,
+ &dev_attr_xpad_axis_xy_left_ADZ.attr,
+ &dev_attr_xpad_axis_xy_left_cal_reset.attr,
+ &dev_attr_xpad_axis_xy_left_cal.attr,
+ &dev_attr_xpad_axis_xy_cal_index.attr,
+ &dev_attr_rc_point_left_1.attr,
+ &dev_attr_rc_point_left_2.attr,
+ &dev_attr_rc_point_left_3.attr,
+ &dev_attr_rc_point_left_4.attr,
+ &dev_attr_rc_point_index.attr,
+ NULL
+};
+static const struct attribute_group ally_controller_axis_xy_left_attr_group = {
+ .name = "axis_xy_left",
+ .attrs = gamepad_axis_xy_left_attrs,
+};
+
+static struct attribute *gamepad_axis_xy_right_attrs[] = {
+ &dev_attr_xpad_axis_xy_right_deadzone.attr,
+ &dev_attr_axis_xyz_deadzone_index.attr,
+ &dev_attr_xpad_axis_xy_right_ADZ.attr,
+ &dev_attr_xpad_axis_xy_right_cal_reset.attr,
+ &dev_attr_xpad_axis_xy_right_cal.attr,
+ &dev_attr_xpad_axis_xy_cal_index.attr,
+ &dev_attr_rc_point_right_1.attr,
+ &dev_attr_rc_point_right_2.attr,
+ &dev_attr_rc_point_right_3.attr,
+ &dev_attr_rc_point_right_4.attr,
+ &dev_attr_rc_point_index.attr,
+ NULL
+};
+static const struct attribute_group ally_controller_axis_xy_right_attr_group = {
+ .name = "axis_xy_right",
+ .attrs = gamepad_axis_xy_right_attrs,
+};
+
+static struct attribute *gamepad_axis_z_left_attrs[] = {
+ &dev_attr_xpad_axis_z_left_deadzone.attr, &dev_attr_axis_xyz_deadzone_index.attr,
+ &dev_attr_xpad_axis_z_left_cal.attr, &dev_attr_xpad_axis_z_cal_index.attr,
+ &dev_attr_xpad_axis_z_left_cal_reset.attr, NULL
+};
+static const struct attribute_group ally_controller_axis_z_left_attr_group = {
+ .name = "axis_z_left",
+ .attrs = gamepad_axis_z_left_attrs,
+};
+
+static struct attribute *gamepad_axis_z_right_attrs[] = {
+ &dev_attr_xpad_axis_z_right_deadzone.attr, &dev_attr_axis_xyz_deadzone_index.attr,
+ &dev_attr_xpad_axis_z_right_cal.attr, &dev_attr_xpad_axis_z_cal_index.attr,
+ &dev_attr_xpad_axis_z_right_cal_reset.attr, NULL
+};
+static const struct attribute_group ally_controller_axis_z_right_attr_group = {
+ .name = "axis_z_right",
+ .attrs = gamepad_axis_z_right_attrs,
+};
+
+static const struct attribute_group *gamepad_device_attr_groups[] = {
+ &ally_controller_attr_group,
+ &ally_controller_axis_xy_left_attr_group,
+ &ally_controller_axis_xy_right_attr_group,
+ &ally_controller_axis_z_left_attr_group,
+ &ally_controller_axis_z_right_attr_group,
+ &btn_mapping_m1_attr_group,
+ &btn_mapping_m2_attr_group,
+ &btn_mapping_a_attr_group,
+ &btn_mapping_b_attr_group,
+ &btn_mapping_x_attr_group,
+ &btn_mapping_y_attr_group,
+ &btn_mapping_lb_attr_group,
+ &btn_mapping_rb_attr_group,
+ &btn_mapping_ls_attr_group,
+ &btn_mapping_rs_attr_group,
+ &btn_mapping_dpad_u_attr_group,
+ &btn_mapping_dpad_d_attr_group,
+ &btn_mapping_dpad_l_attr_group,
+ &btn_mapping_dpad_r_attr_group,
+ &btn_mapping_view_attr_group,
+ &btn_mapping_menu_attr_group,
+ NULL
+};
+
+static int __gamepad_write_all_to_mcu(struct hid_device *hdev,
+ struct ally_gamepad_cfg *ally_cfg)
+{
+ u8 *hidbuf;
+ int ret;
+
+ ret = __gamepad_set_mapping(hdev, ally_cfg, btn_pair_dpad_u_d);
+ if (ret < 0)
+ return ret;
+ ret = __gamepad_set_mapping(hdev, ally_cfg, btn_pair_dpad_l_r);
+ if (ret < 0)
+ return ret;
+ ret = __gamepad_set_mapping(hdev, ally_cfg, btn_pair_ls_rs);
+ if (ret < 0)
+ return ret;
+ ret = __gamepad_set_mapping(hdev, ally_cfg, btn_pair_lb_rb);
+ if (ret < 0)
+ return ret;
+ ret = __gamepad_set_mapping(hdev, ally_cfg, btn_pair_a_b);
+ if (ret < 0)
+ return ret;
+ ret = __gamepad_set_mapping(hdev, ally_cfg, btn_pair_x_y);
+ if (ret < 0)
+ return ret;
+ ret = __gamepad_set_mapping(hdev, ally_cfg, btn_pair_view_menu);
+ if (ret < 0)
+ return ret;
+ ret = __gamepad_set_mapping(hdev, ally_cfg, btn_pair_m1_m2);
+ if (ret < 0)
+ return ret;
+ __gamepad_set_mapping(hdev, ally_cfg, btn_pair_lt_rt);
+ if (ret < 0)
+ return ret;
+ __gamepad_set_deadzones(hdev, ally_cfg);
+ if (ret < 0)
+ return ret;
+ __gamepad_write_js_ADZ_to_mcu(hdev, ally_cfg);
+ if (ret < 0)
+ return ret;
+ __gamepad_write_vibe_intensity_to_mcu(hdev, ally_cfg);
+ if (ret < 0)
+ return ret;
+ __gamepad_write_response_curves_to_mcu(hdev, ally_cfg);
+ if (ret < 0)
+ return ret;
+ ret = __gamepad_check_ready(hdev);
+ if (ret < 0)
+ return ret;
+
+ /* set turbo */
+ hidbuf = kzalloc(FEATURE_ROG_ALLY_REPORT_SIZE, GFP_KERNEL);
+ if (!hidbuf)
+ return -ENOMEM;
+ hidbuf[0] = FEATURE_ROG_ALLY_REPORT_ID;
+ hidbuf[1] = FEATURE_ROG_ALLY_CODE_PAGE;
+ hidbuf[2] = xpad_cmd_set_turbo;
+ hidbuf[3] = xpad_cmd_len_turbo;
+ memcpy(&hidbuf[4], ally_cfg->turbo_btns[ally_cfg->mode - 1], TURBO_BLOCK_LEN);
+ ret = asus_dev_set_report(hdev, hidbuf, FEATURE_ROG_ALLY_REPORT_SIZE);
+
+ kfree(hidbuf);
+ return ret;
+}
+
+static struct ally_gamepad_cfg *ally_gamepad_cfg_create(struct hid_device *hdev)
+{
+ struct ally_gamepad_cfg *ally_cfg;
+ struct input_dev *input_dev;
+ int i, err;
+
+ ally_cfg = devm_kzalloc(&hdev->dev, sizeof(*ally_cfg), GFP_KERNEL);
+ if (!ally_cfg)
+ return ERR_PTR(-ENOMEM);
+ ally_cfg->hdev = hdev;
+
+ input_dev = devm_input_allocate_device(&hdev->dev);
+ if (!input_dev) {
+ err = -ENOMEM;
+ goto free_ally_cfg;
+ }
+ ally_cfg->input = input_dev;
+
+ 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_dev->uniq = hdev->uniq;
+ input_dev->name = "ASUS ROG Ally Config";
+ input_set_capability(input_dev, EV_KEY, KEY_PROG1);
+ input_set_capability(input_dev, EV_KEY, KEY_F16);
+ input_set_capability(input_dev, EV_KEY, KEY_F17);
+ input_set_capability(input_dev, EV_KEY, KEY_F18);
+
+ input_set_drvdata(input_dev, hdev);
+
+ err = input_register_device(input_dev);
+ if (err)
+ goto free_input_dev;
+
+ ally_cfg->mode = xpad_mode_game;
+ for (i = 0; i < xpad_mode_mouse; i++) {
+ ally_cfg->deadzones[i][0][1] = 64;
+ ally_cfg->deadzones[i][0][3] = 64;
+ ally_cfg->deadzones[i][1][1] = 64;
+ ally_cfg->deadzones[i][1][3] = 64;
+ ally_cfg->response_curve[i][0][0] = 0x14;
+ ally_cfg->response_curve[i][0][1] = 0x14;
+ ally_cfg->response_curve[i][0][2] = 0x28;
+ ally_cfg->response_curve[i][0][3] = 0x28;
+ ally_cfg->response_curve[i][0][4] = 0x3c;
+ ally_cfg->response_curve[i][0][5] = 0x3c;
+ ally_cfg->response_curve[i][0][6] = 0x63;
+ ally_cfg->response_curve[i][0][7] = 0x63;
+ ally_cfg->response_curve[i][1][0] = 0x14;
+ ally_cfg->response_curve[i][1][1] = 0x14;
+ ally_cfg->response_curve[i][1][2] = 0x28;
+ ally_cfg->response_curve[i][1][3] = 0x28;
+ ally_cfg->response_curve[i][1][4] = 0x3c;
+ ally_cfg->response_curve[i][1][5] = 0x3c;
+ ally_cfg->response_curve[i][1][6] = 0x63;
+ ally_cfg->response_curve[i][1][7] = 0x63;
+ ally_cfg->vibration_intensity[i][0] = 64;
+ ally_cfg->vibration_intensity[i][1] = 64;
+ }
+ drvdata.gamepad_cfg = ally_cfg;
+
+ /* ignore all errors for this as they are related to USB HID I/O */
+ __gamepad_mapping_xpad_default(ally_cfg);
+ __gamepad_mapping_wasd_default(ally_cfg);
+ /* these calls will never error so ignore the return */
+ __gamepad_mapping_store(ally_cfg, KB_M2, btn_pair_m1_m2, btn_pair_side_left, false);
+ __gamepad_mapping_store(ally_cfg, KB_M1, btn_pair_m1_m2, btn_pair_side_right, false);
+ __gamepad_set_mode(hdev, ally_cfg, xpad_mode_game);
+ __gamepad_set_mapping(hdev, ally_cfg, btn_pair_m1_m2);
+ /* ensure we have data for users to start from */
+ __gamepad_get_calibration(hdev);
+
+ if (sysfs_create_groups(&hdev->dev.kobj, gamepad_device_attr_groups)) {
+ err = -ENODEV;
+ goto unregister_input_dev;
+ }
+
+ return ally_cfg;
+
+unregister_input_dev:
+ input_unregister_device(input_dev);
+ ally_cfg->input = NULL; // Prevent double free when kfree(ally_cfg) happens
+
+free_input_dev:
+ devm_kfree(&hdev->dev, input_dev);
+
+free_ally_cfg:
+ devm_kfree(&hdev->dev, ally_cfg);
+ return ERR_PTR(err);
+}
+
+static void ally_cfg_remove(struct hid_device *hdev)
+{
+ __gamepad_set_mode(hdev, drvdata.gamepad_cfg, xpad_mode_mouse);
+ sysfs_remove_groups(&hdev->dev.kobj, gamepad_device_attr_groups);
+}
+
+/**************************************************************************************************/
+/* ROG Ally LED control */
+/**************************************************************************************************/
+static void ally_schedule_work(struct ally_rgb_leds *led)
+{
+ unsigned long flags;
+
+ spin_lock_irqsave(&led->lock, flags);
+ if (!led->removed)
+ schedule_work(&led->work);
+ spin_unlock_irqrestore(&led->lock, flags);
+ pr_warn("5");
+}
+
+static void ally_led_do_brightness(struct work_struct *work)
+{
+ struct ally_rgb_leds *led = container_of(work, struct ally_rgb_leds, work);
+ u8 buf[] = { FEATURE_ROG_ALLY_REPORT_ID, 0xba, 0xc5, 0xc4, 0x00 };
+ unsigned long flags;
+
+ spin_lock_irqsave(&led->lock, flags);
+ if (!led->update_bright) {
+ spin_unlock_irqrestore(&led->lock, flags);
+ return;
+ }
+ led->update_bright = false;
+ buf[4] = led->brightness;
+ spin_unlock_irqrestore(&led->lock, flags);
+
+ if (asus_dev_set_report(led->hdev, buf, sizeof(buf)) < 0)
+ hid_err(led->hdev, "Ally failed to set backlight\n");
+}
+
+static void ally_led_do_rgb(struct work_struct *work)
+{
+ struct ally_rgb_leds *led = container_of(work, struct ally_rgb_leds, work);
+ unsigned long flags;
+ int ret;
+
+ u8 buf[16] = { [0] = FEATURE_ROG_ALLY_REPORT_ID,
+ [1] = FEATURE_ROG_ALLY_CODE_PAGE,
+ [2] = xpad_cmd_set_leds,
+ [3] = xpad_cmd_len_leds };
+
+ spin_lock_irqsave(&led->lock, flags);
+ if (!led->update_rgb) {
+ spin_unlock_irqrestore(&led->lock, flags);
+ return;
+ }
+ for (int i = 0; i < 4; i++) {
+ buf[4 + i * 3] = led->gamepad_red;
+ buf[5 + i * 3] = led->gamepad_green;
+ buf[6 + i * 3] = led->gamepad_blue;
+ }
+ led->update_rgb = false;
+ spin_unlock_irqrestore(&led->lock, flags);
+
+ ret = asus_dev_set_report(led->hdev, buf, sizeof(buf));
+ if (ret < 0)
+ hid_err(led->hdev, "Ally failed to set gamepad backlight: %d\n", ret);
+}
+
+static void ally_led_work(struct work_struct *work)
+{
+ ally_led_do_brightness(work);
+ ally_led_do_rgb(work);
+}
+
+static void ally_backlight_set(struct led_classdev *led_cdev, enum led_brightness brightness)
+{
+ struct ally_rgb_leds *led =
+ container_of(led_cdev, struct ally_rgb_leds, led_bright_dev);
+ unsigned long flags;
+
+ spin_lock_irqsave(&led->lock, flags);
+ led->update_bright = true;
+ led->brightness = brightness;
+ spin_unlock_irqrestore(&led->lock, flags);
+
+ ally_schedule_work(led);
+}
+
+static enum led_brightness ally_backlight_get(struct led_classdev *led_cdev)
+{
+ struct ally_rgb_leds *led =
+ container_of(led_cdev, struct ally_rgb_leds, led_bright_dev);
+ enum led_brightness brightness;
+ unsigned long flags;
+
+ spin_lock_irqsave(&led->lock, flags);
+ brightness = led->brightness;
+ spin_unlock_irqrestore(&led->lock, flags);
+
+ return brightness;
+}
+
+static void ally_set_rgb_brightness(struct led_classdev *cdev, enum led_brightness brightness)
+{
+ struct led_classdev_mc *mc_cdev = lcdev_to_mccdev(cdev);
+ struct ally_rgb_leds *led = container_of(mc_cdev, struct ally_rgb_leds, led_rgb_dev);
+ unsigned long flags;
+
+ led_mc_calc_color_components(mc_cdev, brightness);
+ spin_lock_irqsave(&led->lock, flags);
+ led->update_rgb = true;
+ led->gamepad_red = mc_cdev->subled_info[0].brightness;
+ led->gamepad_green = mc_cdev->subled_info[1].brightness;
+ led->gamepad_blue = mc_cdev->subled_info[2].brightness;
+ spin_unlock_irqrestore(&led->lock, flags);
+
+ ally_schedule_work(led);
+}
+
+static int ally_gamepad_register_brightness(struct hid_device *hdev,
+ struct ally_rgb_leds *led_rgb)
+{
+ struct led_classdev *led_cdev;
+
+ led_cdev = &led_rgb->led_bright_dev;
+ led_cdev->name = "ally:kbd_backlight"; /* Let a desktop control it also */
+ led_cdev->max_brightness = 3;
+ led_cdev->brightness_set = ally_backlight_set;
+ led_cdev->brightness_get = ally_backlight_get;
+
+ return devm_led_classdev_register(&hdev->dev, &led_rgb->led_bright_dev);
+}
+
+static int ally_gamepad_register_rgb_leds(struct hid_device *hdev,
+ struct ally_rgb_leds *led_rgb)
+{
+ struct mc_subled *mc_led_info;
+ struct led_classdev *led_cdev;
+
+ mc_led_info = devm_kmalloc_array(&hdev->dev, 3, sizeof(*mc_led_info),
+ GFP_KERNEL | __GFP_ZERO);
+ if (!mc_led_info)
+ return -ENOMEM;
+
+ mc_led_info[0].color_index = LED_COLOR_ID_RED;
+ mc_led_info[1].color_index = LED_COLOR_ID_GREEN;
+ mc_led_info[2].color_index = LED_COLOR_ID_BLUE;
+
+ led_rgb->led_rgb_dev.subled_info = mc_led_info;
+ led_rgb->led_rgb_dev.num_colors = 3;
+
+ led_cdev = &led_rgb->led_rgb_dev.led_cdev;
+ led_cdev->name = "ally:rgb:gamepad";
+ led_cdev->brightness = 128;
+ led_cdev->max_brightness = 255;
+ led_cdev->brightness_set = ally_set_rgb_brightness;
+
+ return devm_led_classdev_multicolor_register(&hdev->dev, &led_rgb->led_rgb_dev);
+}
+
+static struct ally_rgb_leds *ally_gamepad_rgb_create(struct hid_device *hdev)
+{
+ struct ally_rgb_leds *led_rgb;
+ int ret;
+
+ led_rgb = devm_kzalloc(&hdev->dev, sizeof(struct ally_rgb_leds), GFP_KERNEL);
+ if (!led_rgb)
+ return ERR_PTR(-ENOMEM);
+
+ ret = ally_gamepad_register_rgb_leds(hdev, led_rgb);
+ if (ret < 0) {
+ cancel_work_sync(&led_rgb->work);
+ devm_kfree(&hdev->dev, led_rgb);
+ return ERR_PTR(ret);
+ }
+
+ ret = ally_gamepad_register_brightness(hdev, led_rgb);
+ if (ret < 0) {
+ cancel_work_sync(&led_rgb->work);
+ devm_kfree(&hdev->dev, led_rgb);
+ return ERR_PTR(ret);
+ }
+
+ led_rgb->hdev = hdev;
+ led_rgb->brightness = 3;
+ led_rgb->removed = false;
+
+ INIT_WORK(&led_rgb->work, ally_led_work);
+ spin_lock_init(&led_rgb->lock);
+
+ return led_rgb;
+}
+
+static void ally_rgb_remove(struct hid_device *hdev)
+{
+ struct ally_rgb_leds *led_rgb = drvdata.led_rgb;
+ unsigned long flags;
+
+ spin_lock_irqsave(&led_rgb->lock, flags);
+ led_rgb->removed = true;
+ spin_unlock_irqrestore(&led_rgb->lock, flags);
+ cancel_work_sync(&led_rgb->work);
+}
+
+/**************************************************************************************************/
+/* ROG Ally driver init */
+/**************************************************************************************************/
+static int ally_raw_event(struct hid_device *hdev, struct hid_report *report, u8 *data,
+ int size)
+{
+ struct ally_gamepad_cfg *cfg = drvdata.gamepad_cfg;
+ struct ally_x_device *ally_x = drvdata.ally_x;
+
+ if (ally_x) {
+ if ((hdev->bus == BUS_USB && report->id == ALLY_X_INPUT_REPORT_USB &&
+ size == ALLY_X_INPUT_REPORT_USB_SIZE) ||
+ (cfg && data[0] == 0x5A)) {
+ ally_x_raw_event(ally_x, report, data, size);
+ } else {
+ return -1;
+ }
+ }
+ if (cfg && !ally_x) {
+ input_report_key(cfg->input, KEY_PROG1, data[1] == 0x38);
+ input_report_key(cfg->input, KEY_F16, data[1] == 0xA6);
+ input_report_key(cfg->input, KEY_F17, data[1] == 0xA7);
+ input_report_key(cfg->input, KEY_F18, data[1] == 0xA8);
+ input_sync(cfg->input);
+ }
+
+ return 0;
+}
+
+static int ally_gamepad_init(struct hid_device *hdev, u8 report_id)
+{
+ const u8 buf[] = { report_id, 0x41, 0x53, 0x55, 0x53, 0x20, 0x54, 0x65,
+ 0x63, 0x68, 0x2e, 0x49, 0x6e, 0x63, 0x2e, 0x00 };
+ int ret;
+
+ ret = asus_dev_set_report(hdev, buf, sizeof(buf));
+ if (ret < 0)
+ hid_err(hdev, "Ally failed to send init command: %d\n", ret);
+
+ return ret;
+}
+
+static int ally_probe(struct hid_device *hdev, const struct hid_device_id *id)
+{
+ struct usb_interface *intf = to_usb_interface(hdev->dev.parent);
+ struct usb_host_endpoint *ep = intf->cur_altsetting->endpoint;
+ int ret;
+
+ if (ep->desc.bEndpointAddress != ALLY_CFG_INTF_IN_ADDRESS &&
+ ep->desc.bEndpointAddress != ALLY_X_INTERFACE_ADDRESS)
+ return -ENODEV;
+
+ ret = hid_parse(hdev);
+ if (ret) {
+ hid_err(hdev, "Parse failed\n");
+ return ret;
+ }
+
+ ret = hid_hw_start(hdev, HID_CONNECT_HIDRAW);
+ if (ret) {
+ hid_err(hdev, "Failed to start HID device\n");
+ return ret;
+ }
+
+ ret = hid_hw_open(hdev);
+ if (ret) {
+ hid_err(hdev, "Failed to open HID device\n");
+ goto err_stop;
+ }
+
+ /* Initialize MCU even before alloc */
+ ret = ally_gamepad_init(hdev, FEATURE_REPORT_ID);
+ if (ret < 0)
+ return ret;
+
+ ret = ally_gamepad_init(hdev, FEATURE_KBD_LED_REPORT_ID1);
+ if (ret < 0)
+ return ret;
+
+ ret = ally_gamepad_init(hdev, FEATURE_KBD_LED_REPORT_ID2);
+ if (ret < 0)
+ return ret;
+
+ drvdata.hdev = hdev;
+ hid_set_drvdata(hdev, &drvdata);
+
+ /* This should almost always exist */
+ if (ep->desc.bEndpointAddress == ALLY_CFG_INTF_IN_ADDRESS) {
+ drvdata.led_rgb = ally_gamepad_rgb_create(hdev);
+ if (IS_ERR(drvdata.led_rgb))
+ hid_err(hdev, "Failed to create Ally gamepad LEDs.\n");
+ else
+ hid_info(hdev, "Created Ally RGB LED controls.\n");
+
+ ally_gamepad_cfg_create(hdev); // assigns self
+ if (IS_ERR(drvdata.gamepad_cfg))
+ hid_err(hdev, "Failed to create Ally gamepad attributes.\n");
+ else
+ hid_info(hdev, "Created Ally gamepad attributes.\n");
+
+ if (IS_ERR(drvdata.led_rgb) && IS_ERR(drvdata.gamepad_cfg))
+ goto err_close;
+ }
+
+ /* May or may not exist */
+ if (ep->desc.bEndpointAddress == ALLY_X_INTERFACE_ADDRESS) {
+ drvdata.ally_x = ally_x_create(hdev);
+ if (IS_ERR(drvdata.ally_x)) {
+ hid_err(hdev, "Failed to create Ally X gamepad.\n");
+ drvdata.ally_x = NULL;
+ goto err_close;
+ }
+ hid_info(hdev, "Created Ally X controller.\n");
+
+ // Not required since we send this inputs ep through the gamepad input dev
+ if (drvdata.gamepad_cfg && drvdata.gamepad_cfg->input) {
+ input_unregister_device(drvdata.gamepad_cfg->input);
+ hid_info(hdev, "Ally X removed unrequired input dev.\n");
+ }
+ }
+
+ return 0;
+
+err_close:
+ hid_hw_close(hdev);
+err_stop:
+ hid_hw_stop(hdev);
+ return ret;
+}
+
+static void ally_remove(struct hid_device *hdev)
+{
+ if (drvdata.ally_x)
+ ally_x_remove(hdev);
+ if (drvdata.led_rgb)
+ ally_rgb_remove(hdev);
+ if (drvdata.gamepad_cfg)
+ ally_cfg_remove(hdev);
+ hid_hw_close(hdev);
+ hid_hw_stop(hdev);
+}
+
+static int ally_resume(struct hid_device *hdev)
+{
+ int ret;
+
+ ret = ally_gamepad_init(hdev, FEATURE_REPORT_ID);
+ if (ret < 0)
+ return ret;
+
+ ret = ally_gamepad_init(hdev, FEATURE_KBD_LED_REPORT_ID1);
+ if (ret < 0)
+ return ret;
+
+ ret = ally_gamepad_init(hdev, FEATURE_KBD_LED_REPORT_ID2);
+ if (ret < 0)
+ return ret;
+
+ if (drvdata.ally_x && drvdata.ally_x->output_worker_initialized)
+ schedule_work(&drvdata.ally_x->output_worker);
+
+ return ret;
+}
+
+MODULE_DEVICE_TABLE(hid, rog_ally_devices);
+
+static struct hid_driver rog_ally_cfg = {
+ .name = "asus_rog_ally",
+ .id_table = rog_ally_devices,
+ .probe = ally_probe,
+ .remove = ally_remove,
+ .raw_event = ally_raw_event,
+ .resume = ally_resume,
+};
+
+static int __init rog_ally_cfg_init(void)
+{
+ return hid_register_driver(&rog_ally_cfg);
+}
+
+static void __exit rog_ally_cfg_exit(void)
+{
+ hid_unregister_driver(&rog_ally_cfg);
+}
+
+module_init(rog_ally_cfg_init);
+module_exit(rog_ally_cfg_exit);
+
+MODULE_AUTHOR("Luke D. Jones");
+MODULE_DESCRIPTION("HID Driver for ASUS ROG Ally gamepad configuration.");
+MODULE_LICENSE("GPL");
diff --git a/drivers/hid/hid-asus-ally.h b/drivers/hid/hid-asus-ally.h
new file mode 100644
index 000000000000..252d9f126e32
--- /dev/null
+++ b/drivers/hid/hid-asus-ally.h
@@ -0,0 +1,544 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later
+ *
+ * HID driver for Asus ROG laptops and Ally
+ *
+ * Copyright (c) 2023 Luke Jones <luke@ljones.dev>
+ */
+
+#include <linux/hid.h>
+#include <linux/types.h>
+
+#define ALLY_X_INTERFACE_ADDRESS 0x87
+
+#define BTN_CODE_LEN 11
+#define MAPPING_BLOCK_LEN 44
+
+#define TURBO_BLOCK_LEN 32
+#define TURBO_BLOCK_STEP 2
+
+#define PAD_A "pad_a"
+#define PAD_B "pad_b"
+#define PAD_X "pad_x"
+#define PAD_Y "pad_y"
+#define PAD_LB "pad_lb"
+#define PAD_RB "pad_rb"
+#define PAD_LS "pad_ls"
+#define PAD_RS "pad_rs"
+#define PAD_DPAD_UP "pad_dpad_up"
+#define PAD_DPAD_DOWN "pad_dpad_down"
+#define PAD_DPAD_LEFT "pad_dpad_left"
+#define PAD_DPAD_RIGHT "pad_dpad_right"
+#define PAD_VIEW "pad_view"
+#define PAD_MENU "pad_menu"
+#define PAD_XBOX "pad_xbox"
+
+#define KB_M1 "kb_m1"
+#define KB_M2 "kb_m2"
+#define KB_ESC "kb_esc"
+#define KB_F1 "kb_f1"
+#define KB_F2 "kb_f2"
+#define KB_F3 "kb_f3"
+#define KB_F4 "kb_f4"
+#define KB_F5 "kb_f5"
+#define KB_F6 "kb_f6"
+#define KB_F7 "kb_f7"
+#define KB_F8 "kb_f8"
+#define KB_F9 "kb_f9"
+#define KB_F10 "kb_f10"
+#define KB_F11 "kb_f11"
+#define KB_F12 "kb_f12"
+#define KB_F14 "kb_f14"
+#define KB_F15 "kb_f15"
+
+#define KB_BACKTICK "kb_backtick"
+#define KB_1 "kb_1"
+#define KB_2 "kb_2"
+#define KB_3 "kb_3"
+#define KB_4 "kb_4"
+#define KB_5 "kb_5"
+#define KB_6 "kb_6"
+#define KB_7 "kb_7"
+#define KB_8 "kb_8"
+#define KB_9 "kb_9"
+#define KB_0 "kb_0"
+#define KB_HYPHEN "kb_hyphen"
+#define KB_EQUALS "kb_equals"
+#define KB_BACKSPACE "kb_backspace"
+
+#define KB_TAB "kb_tab"
+#define KB_Q "kb_q"
+#define KB_W "kb_w"
+#define KB_E "kb_e"
+#define KB_R "kb_r"
+#define KB_T "kb_t"
+#define KB_Y "kb_y"
+#define KB_U "kb_u"
+#define KB_I "kb_i"
+#define KB_O "kb_o"
+#define KB_P "kb_p"
+#define KB_LBRACKET "kb_lbracket"
+#define KB_RBRACKET "kb_rbracket"
+#define KB_BACKSLASH "kb_bkslash"
+
+#define KB_CAPS "kb_caps"
+#define KB_A "kb_a"
+#define KB_S "kb_s"
+#define KB_D "kb_d"
+#define KB_F "kb_f"
+#define KB_G "kb_g"
+#define KB_H "kb_h"
+#define KB_J "kb_j"
+#define KB_K "kb_k"
+#define KB_L "kb_l"
+#define KB_SEMI "kb_semicolon"
+#define KB_QUOTE "kb_quote"
+#define KB_RET "kb_enter"
+
+#define KB_LSHIFT "kb_lshift"
+#define KB_Z "kb_z"
+#define KB_X "kb_x"
+#define KB_C "kb_c"
+#define KB_V "kb_v"
+#define KB_B "kb_b"
+#define KB_N "kb_n"
+#define KB_M "kb_m"
+#define KB_COMMA "kb_comma"
+#define KB_PERIOD "kb_period"
+#define KB_FWDSLASH "kb_fwdslash"
+#define KB_RSHIFT "kb_rshift"
+
+#define KB_LCTL "kb_lctl"
+#define KB_META "kb_meta"
+#define KB_LALT "kb_lalt"
+#define KB_SPACE "kb_space"
+#define KB_RALT "kb_ralt"
+#define KB_MENU "kb_menu"
+#define KB_RCTL "kb_rctl"
+
+#define KB_PRNTSCN "kb_prntscn"
+#define KB_SCRLCK "kb_scrlck"
+#define KB_PAUSE "kb_pause"
+#define KB_INS "kb_ins"
+#define KB_HOME "kb_home"
+#define KB_PGUP "kb_pgup"
+#define KB_DEL "kb_del"
+#define KB_END "kb_end"
+#define KB_PGDWN "kb_pgdwn"
+
+#define KB_UP_ARROW "kb_up_arrow"
+#define KB_DOWN_ARROW "kb_down_arrow"
+#define KB_LEFT_ARROW "kb_left_arrow"
+#define KB_RIGHT_ARROW "kb_right_arrow"
+
+#define NUMPAD_LOCK "numpad_lock"
+#define NUMPAD_FWDSLASH "numpad_fwdslash"
+#define NUMPAD_ASTERISK "numpad_asterisk"
+#define NUMPAD_HYPHEN "numpad_hyphen"
+#define NUMPAD_0 "numpad_0"
+#define NUMPAD_1 "numpad_1"
+#define NUMPAD_2 "numpad_2"
+#define NUMPAD_3 "numpad_3"
+#define NUMPAD_4 "numpad_4"
+#define NUMPAD_5 "numpad_5"
+#define NUMPAD_6 "numpad_6"
+#define NUMPAD_7 "numpad_7"
+#define NUMPAD_8 "numpad_8"
+#define NUMPAD_9 "numpad_9"
+#define NUMPAD_PLUS "numpad_plus"
+#define NUMPAD_ENTER "numpad_enter"
+#define NUMPAD_PERIOD "numpad_."
+
+#define MOUSE_LCLICK "rat_lclick"
+#define MOUSE_RCLICK "rat_rclick"
+#define MOUSE_MCLICK "rat_mclick"
+#define MOUSE_WHEEL_UP "rat_wheel_up"
+#define MOUSE_WHEEL_DOWN "rat_wheel_down"
+
+#define MEDIA_SCREENSHOT "media_screenshot"
+#define MEDIA_SHOW_KEYBOARD "media_show_keyboard"
+#define MEDIA_SHOW_DESKTOP "media_show_desktop"
+#define MEDIA_START_RECORDING "media_start_recording"
+#define MEDIA_MIC_OFF "media_mic_off"
+#define MEDIA_VOL_DOWN "media_vol_down"
+#define MEDIA_VOL_UP "media_vol_up"
+
+/* required so we can have nested attributes with same name but different functions */
+#define ALLY_DEVICE_ATTR_RW(_name, _sysfs_name) \
+ struct device_attribute dev_attr_##_name = \
+ __ATTR(_sysfs_name, 0644, _name##_show, _name##_store)
+
+#define ALLY_DEVICE_ATTR_RO(_name, _sysfs_name) \
+ struct device_attribute dev_attr_##_name = \
+ __ATTR(_sysfs_name, 0444, _name##_show, NULL)
+
+#define ALLY_DEVICE_ATTR_WO(_name, _sysfs_name) \
+ struct device_attribute dev_attr_##_name = \
+ __ATTR(_sysfs_name, 0200, NULL, _name##_store)
+
+/* response curve macros */
+#define ALLY_RESP_CURVE_SHOW(_name, _point_n) \
+ static ssize_t _name##_show(struct device *dev, \
+ struct device_attribute *attr, char *buf) \
+ { \
+ struct ally_gamepad_cfg *ally_cfg = drvdata.gamepad_cfg; \
+ int idx = (_point_n - 1) * 2; \
+ if (!drvdata.gamepad_cfg) \
+ return -ENODEV; \
+ return sysfs_emit( \
+ buf, "%d %d\n", \
+ ally_cfg->response_curve[ally_cfg->mode] \
+ [btn_pair_side_left][idx], \
+ ally_cfg->response_curve[ally_cfg->mode] \
+ [btn_pair_side_right] \
+ [idx + 1]); \
+ }
+
+#define ALLY_RESP_CURVE_STORE(_name, _side, _point_n) \
+ static ssize_t _name##_store(struct device *dev, \
+ struct device_attribute *attr, \
+ const char *buf, size_t count) \
+ { \
+ int ret = __gamepad_store_response_curve( \
+ dev, buf, btn_pair_side_##_side, _point_n); \
+ if (ret < 0) \
+ return ret; \
+ return count; \
+ }
+
+/* _point_n must start at 1 */
+#define ALLY_JS_RC_POINT(_side, _point_n, _sysfs_label) \
+ ALLY_RESP_CURVE_SHOW(rc_point_##_side##_##_point_n, _point_n); \
+ ALLY_RESP_CURVE_STORE(rc_point_##_side##_##_point_n, _side, _point_n); \
+ ALLY_DEVICE_ATTR_RW(rc_point_##_side##_##_point_n, \
+ _sysfs_label##_point_n)
+
+/* deadzone macros */
+#define ALLY_AXIS_DEADZONE_SHOW(_axis) \
+ static ssize_t _axis##_deadzone_show( \
+ struct device *dev, struct device_attribute *attr, char *buf) \
+ { \
+ struct ally_gamepad_cfg *ally_cfg = drvdata.gamepad_cfg; \
+ int side, is_tr; \
+ if (!drvdata.gamepad_cfg) \
+ return -ENODEV; \
+ is_tr = _axis > xpad_axis_xy_right; \
+ side = _axis == xpad_axis_xy_right || \
+ _axis == xpad_axis_z_right ? \
+ 2 : \
+ 0; \
+ return sysfs_emit( \
+ buf, "%d %d\n", \
+ ally_cfg->deadzones[ally_cfg->mode][is_tr][side], \
+ ally_cfg->deadzones[ally_cfg->mode][is_tr][side + 1]); \
+ }
+
+#define ALLY_AXIS_DEADZONE_STORE(_axis) \
+ static ssize_t _axis##_deadzone_store(struct device *dev, \
+ struct device_attribute *attr, \
+ const char *buf, size_t count) \
+ { \
+ struct ally_gamepad_cfg *ally_cfg = drvdata.gamepad_cfg; \
+ if (!drvdata.gamepad_cfg) \
+ return -ENODEV; \
+ int ret = __gamepad_store_deadzones(ally_cfg, _axis, buf); \
+ if (ret < 0) \
+ return ret; \
+ return count; \
+ }
+
+#define ALLY_AXIS_DEADZONE(_axis, _sysfs_label) \
+ ALLY_AXIS_DEADZONE_SHOW(_axis); \
+ ALLY_AXIS_DEADZONE_STORE(_axis); \
+ ALLY_DEVICE_ATTR_RW(_axis##_deadzone, _sysfs_label)
+
+/* button specific macros */
+#define ALLY_BTN_SHOW(_fname, _pair, _side, _secondary) \
+ static ssize_t _fname##_show(struct device *dev, \
+ struct device_attribute *attr, char *buf) \
+ { \
+ struct ally_gamepad_cfg *ally_cfg = drvdata.gamepad_cfg; \
+ if (!drvdata.gamepad_cfg) \
+ return -ENODEV; \
+ return sysfs_emit(buf, "%s\n", \
+ __btn_map_to_string(ally_cfg, _pair, _side, \
+ _secondary)); \
+ }
+
+#define ALLY_BTN_STORE(_fname, _pair, _side, _secondary) \
+ static ssize_t _fname##_store(struct device *dev, \
+ struct device_attribute *attr, \
+ const char *buf, size_t count) \
+ { \
+ struct ally_gamepad_cfg *ally_cfg = drvdata.gamepad_cfg; \
+ if (!drvdata.gamepad_cfg) \
+ return -ENODEV; \
+ int ret = __gamepad_mapping_store(ally_cfg, buf, _pair, _side, \
+ _secondary); \
+ if (ret < 0) \
+ return ret; \
+ return count; \
+ }
+
+#define ALLY_BTN_TURBO_SHOW(_fname, _pair, _side) \
+ static ssize_t _fname##_turbo_show( \
+ struct device *dev, struct device_attribute *attr, char *buf) \
+ { \
+ return sysfs_emit(buf, "%d\n", \
+ __gamepad_turbo_show(dev, _pair, _side)); \
+ }
+
+#define ALLY_BTN_TURBO_STORE(_fname, _pair, _side) \
+ static ssize_t _fname##_turbo_store(struct device *dev, \
+ struct device_attribute *attr, \
+ const char *buf, size_t count) \
+ { \
+ int ret = __gamepad_turbo_store(dev, buf, _pair, _side); \
+ if (ret < 0) \
+ return ret; \
+ return count; \
+ }
+
+#define ALLY_BTN_ATTRS_GROUP(_name, _fname) \
+ static struct attribute *_fname##_attrs[] = { \
+ &dev_attr_##_fname.attr, &dev_attr_##_fname##_macro.attr, \
+ &dev_attr_##_fname##_turbo.attr, NULL \
+ }; \
+ static const struct attribute_group _fname##_attr_group = { \
+ .name = __stringify(_name), \
+ .attrs = _fname##_attrs, \
+ }
+
+#define ALLY_BTN_MAPPING(_fname, _pair, _side) \
+ ALLY_BTN_SHOW(btn_mapping_##_fname, _pair, _side, false); \
+ ALLY_BTN_STORE(btn_mapping_##_fname, _pair, _side, false); \
+ ALLY_BTN_SHOW(btn_mapping_##_fname##_macro, _pair, _side, true); \
+ ALLY_BTN_STORE(btn_mapping_##_fname##_macro, _pair, _side, true); \
+ ALLY_BTN_TURBO_SHOW(btn_mapping_##_fname, _pair, _side); \
+ ALLY_BTN_TURBO_STORE(btn_mapping_##_fname, _pair, _side); \
+ ALLY_DEVICE_ATTR_RW(btn_mapping_##_fname, remap); \
+ ALLY_DEVICE_ATTR_RW(btn_mapping_##_fname##_macro, macro_remap); \
+ ALLY_DEVICE_ATTR_RW(btn_mapping_##_fname##_turbo, turbo); \
+ ALLY_BTN_ATTRS_GROUP(btn_##_fname, btn_mapping_##_fname)
+
+/* calibration macros */
+#define ALLY_CAL_STORE(_fname, _axis) \
+ static ssize_t _fname##_store(struct device *dev, \
+ struct device_attribute *attr, \
+ const char *buf, size_t count) \
+ { \
+ int ret = __gamepad_cal_store(dev, buf, _axis); \
+ if (ret < 0) \
+ return ret; \
+ return count; \
+ }
+
+#define ALLY_CAL_SHOW(_fname, _axis) \
+ static ssize_t _fname##_show(struct device *dev, \
+ struct device_attribute *attr, char *buf) \
+ { \
+ return __gamepad_cal_show(dev, buf, _axis); \
+ }
+
+#define ALLY_CAL_ATTR(_fname, _axis, _sysfs_label) \
+ ALLY_CAL_STORE(_fname, _axis); \
+ ALLY_CAL_SHOW(_fname, _axis); \
+ ALLY_DEVICE_ATTR_RW(_fname, _sysfs_label)
+
+#define ALLY_CAL_RESET_STORE(_fname, _axis) \
+ static ssize_t _fname##_store(struct device *dev, \
+ struct device_attribute *attr, \
+ const char *buf, size_t count) \
+ { \
+ int ret = __gamepad_cal_reset(dev, buf, _axis); \
+ if (ret < 0) \
+ return ret; \
+ return count; \
+ }
+
+#define ALLY_CAL_RESET_ATTR(_fname, _axis, _sysfs_label) \
+ ALLY_CAL_RESET_STORE(_fname, _axis); \
+ ALLY_DEVICE_ATTR_WO(_fname, _sysfs_label)
+
+/*
+ * The following blocks of packets exist to make setting a default boot config
+ * easier. They were directly captured from setting the gamepad up.
+ */
+
+/* Default blocks for the xpad mode */
+static const u8 XPAD_DEF1[MAPPING_BLOCK_LEN] = {
+ 0x01, 0x09, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x05, 0x00, 0x00, 0x19, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x01, 0x0a, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x04, 0x00, 0x00, 0x00, 0x00, 0x03, 0x8c, 0x88, 0x76, 0x00, 0x00
+};
+static const u8 XPAD_DEF2[MAPPING_BLOCK_LEN] = {
+ 0x01, 0x0b, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x04, 0x00, 0x00, 0x00, 0x00, 0x02, 0x82, 0x23, 0x00, 0x00, 0x00,
+ 0x01, 0x0c, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x04, 0x00, 0x00, 0x00, 0x00, 0x02, 0x82, 0x0d, 0x00, 0x00, 0x00
+};
+static const u8 XPAD_DEF3[MAPPING_BLOCK_LEN] = {
+ 0x01, 0x07, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x01, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
+};
+static const u8 XPAD_DEF4[MAPPING_BLOCK_LEN] = {
+ 0x01, 0x05, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x01, 0x06, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
+};
+static const u8 XPAD_DEF5[MAPPING_BLOCK_LEN] = {
+ 0x01, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x05, 0x00, 0x00, 0x16, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x01, 0x02, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x04, 0x00, 0x00, 0x00, 0x00, 0x02, 0x82, 0x31, 0x00, 0x00, 0x00
+};
+static const u8 XPAD_DEF6[MAPPING_BLOCK_LEN] = {
+ 0x01, 0x03, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x04, 0x00, 0x00, 0x00, 0x00, 0x02, 0x82, 0x4d, 0x00, 0x00, 0x00,
+ 0x01, 0x04, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x05, 0x00, 0x00, 0x1e, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
+};
+static const u8 XPAD_DEF7[MAPPING_BLOCK_LEN] = {
+ 0x01, 0x11, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x01, 0x12, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
+};
+static const u8 XPAD_DEF8[MAPPING_BLOCK_LEN] = {
+ 0x02, 0x00, 0x8e, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x02, 0x00, 0x8e, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x02, 0x00, 0x8f, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x02, 0x00, 0x8f, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
+};
+static const u8 XPAD_DEF9[MAPPING_BLOCK_LEN] = {
+ 0x01, 0x0d, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x01, 0x0e, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
+};
+
+/* default blocks for the wasd mode */
+static const u8 WASD_DEF1[MAPPING_BLOCK_LEN] = {
+ 0x02, 0x00, 0x98, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x05, 0x00, 0x00, 0x19, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x02, 0x00, 0x99, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x04, 0x00, 0x00, 0x00, 0x00, 0x03, 0x8c, 0x88, 0x76, 0x00, 0x00
+};
+static const u8 WASD_DEF2[MAPPING_BLOCK_LEN] = {
+ 0x02, 0x00, 0x9a, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x04, 0x00, 0x00, 0x00, 0x00, 0x02, 0x82, 0x23, 0x00, 0x00, 0x00,
+ 0x02, 0x00, 0x9b, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x04, 0x00, 0x00, 0x00, 0x00, 0x02, 0x82, 0x0d, 0x00, 0x00, 0x00
+};
+static const u8 WASD_DEF3[MAPPING_BLOCK_LEN] = {
+ 0x02, 0x00, 0x88, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x03, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
+};
+static const u8 WASD_DEF4[MAPPING_BLOCK_LEN] = {
+ 0x02, 0x00, 0x0d, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x03, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
+};
+static const u8 WASD_DEF5[MAPPING_BLOCK_LEN] = {
+ 0x02, 0x00, 0x5a, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x05, 0x00, 0x00, 0x16, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x02, 0x00, 0x76, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x04, 0x00, 0x00, 0x00, 0x00, 0x02, 0x82, 0x31, 0x00, 0x00, 0x00
+};
+static const u8 WASD_DEF6[MAPPING_BLOCK_LEN] = {
+ 0x02, 0x00, 0x97, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x04, 0x00, 0x00, 0x00, 0x00, 0x02, 0x82, 0x4d, 0x00, 0x00, 0x00,
+ 0x02, 0x00, 0x96, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x05, 0x00, 0x00, 0x1e, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
+};
+static const u8 WASD_DEF7[MAPPING_BLOCK_LEN] = {
+ 0x01, 0x11, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x01, 0x12, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
+};
+static const u8 WASD_DEF8[MAPPING_BLOCK_LEN] = {
+ 0x02, 0x00, 0x8e, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x02, 0x00, 0x8e, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x02, 0x00, 0x8f, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x02, 0x00, 0x8f, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
+};
+static const u8 WASD_DEF9[MAPPING_BLOCK_LEN] = {
+ 0x04, 0x00, 0x00, 0x00, 0x00, 0x02, 0x88, 0x0d, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x03, 0x00, 0x00, 0x00, 0x02, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
+};
+
+/*
+ * the xpad_mode is used inside the mode setting packet and is used
+ * for indexing (xpad_mode - 1)
+ */
+enum xpad_mode {
+ xpad_mode_game = 0x01,
+ xpad_mode_wasd = 0x02,
+ xpad_mode_mouse = 0x03,
+};
+
+/* the xpad_cmd determines which feature is set or queried */
+enum xpad_cmd {
+ xpad_cmd_set_mode = 0x01,
+ xpad_cmd_set_mapping = 0x02,
+ xpad_cmd_set_js_dz = 0x04, /* deadzones */
+ xpad_cmd_set_tr_dz = 0x05, /* deadzones */
+ xpad_cmd_set_vibe_intensity = 0x06,
+ xpad_cmd_set_leds = 0x08,
+ xpad_cmd_check_ready = 0x0A,
+ xpad_cmd_set_calibration = 0x0D,
+ xpad_cmd_set_turbo = 0x0F,
+ xpad_cmd_set_response_curve = 0x13,
+ xpad_cmd_set_adz = 0x18,
+};
+
+/* the xpad_cmd determines which feature is set or queried */
+enum xpad_cmd_len {
+ xpad_cmd_len_mode = 0x01,
+ xpad_cmd_len_mapping = 0x2c,
+ xpad_cmd_len_deadzone = 0x04,
+ xpad_cmd_len_vibe_intensity = 0x02,
+ xpad_cmd_len_leds = 0x0C,
+ xpad_cmd_len_calibration2 = 0x01,
+ xpad_cmd_len_calibration3 = 0x01,
+ xpad_cmd_len_turbo = 0x20,
+ xpad_cmd_len_response_curve = 0x09,
+ xpad_cmd_len_adz = 0x02,
+};
+
+/*
+ * the xpad_mode is used in various set and query HID packets and is
+ * used for indexing (xpad_axis - 1)
+ */
+enum xpad_axis {
+ xpad_axis_xy_left = 0x01,
+ xpad_axis_xy_right = 0x02,
+ xpad_axis_z_left = 0x03,
+ xpad_axis_z_right = 0x04,
+};
+
+enum btn_pair {
+ btn_pair_dpad_u_d = 0x01,
+ btn_pair_dpad_l_r = 0x02,
+ btn_pair_ls_rs = 0x03,
+ btn_pair_lb_rb = 0x04,
+ btn_pair_a_b = 0x05,
+ btn_pair_x_y = 0x06,
+ btn_pair_view_menu = 0x07,
+ btn_pair_m1_m2 = 0x08,
+ btn_pair_lt_rt = 0x09,
+};
+
+enum btn_pair_side {
+ btn_pair_side_left = 0x00,
+ btn_pair_side_right = 0x01,
+};
diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index 659cf9c96e26..528c210f9e8c 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -52,6 +52,10 @@ MODULE_DESCRIPTION("Asus HID Keyboard and TouchPad");
#define FEATURE_KBD_LED_REPORT_ID1 0x5d
#define FEATURE_KBD_LED_REPORT_ID2 0x5e
+#define ALLY_CFG_INTF_IN_ADDRESS 0x83
+#define ALLY_CFG_INTF_OUT_ADDRESS 0x04
+#define ALLY_X_INTERFACE_ADDRESS 0x87
+
#define SUPPORT_KBD_BACKLIGHT BIT(0)
#define MAX_TOUCH_MAJOR 8
@@ -84,6 +88,7 @@ MODULE_DESCRIPTION("Asus HID Keyboard and TouchPad");
#define QUIRK_MEDION_E1239T BIT(10)
#define QUIRK_ROG_NKEY_KEYBOARD BIT(11)
#define QUIRK_ROG_CLAYMORE_II_KEYBOARD BIT(12)
+#define QUIRK_ROG_ALLY_XPAD BIT(13)
#define I2C_KEYBOARD_QUIRKS (QUIRK_FIX_NOTEBOOK_REPORT | \
QUIRK_NO_INIT_REPORTS | \
@@ -1003,6 +1008,17 @@ static int asus_probe(struct hid_device *hdev, const struct hid_device_id *id)
drvdata->quirks = id->driver_data;
+ /* Ignore these endpoints as they will be used by other drivers */
+ if (drvdata->quirks & QUIRK_ROG_ALLY_XPAD) {
+ struct usb_interface *intf = to_usb_interface(hdev->dev.parent);
+ struct usb_host_endpoint *ep = intf->cur_altsetting->endpoint;
+
+ if (ep->desc.bEndpointAddress == ALLY_X_INTERFACE_ADDRESS ||
+ ep->desc.bEndpointAddress == ALLY_CFG_INTF_IN_ADDRESS ||
+ ep->desc.bEndpointAddress == ALLY_CFG_INTF_OUT_ADDRESS)
+ return -ENODEV;
+ }
+
/*
* T90CHI's keyboard dock returns same ID values as T100CHI's dock.
* Thus, identify T90CHI dock with product name string.
@@ -1254,10 +1270,10 @@ static const struct hid_device_id asus_devices[] = {
QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD },
{ HID_USB_DEVICE(USB_VENDOR_ID_ASUSTEK,
USB_DEVICE_ID_ASUSTEK_ROG_NKEY_ALLY),
- QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD },
+ QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD | QUIRK_ROG_ALLY_XPAD},
{ HID_USB_DEVICE(USB_VENDOR_ID_ASUSTEK,
USB_DEVICE_ID_ASUSTEK_ROG_NKEY_ALLY_X),
- QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD },
+ QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD | QUIRK_ROG_ALLY_XPAD },
{ HID_USB_DEVICE(USB_VENDOR_ID_ASUSTEK,
USB_DEVICE_ID_ASUSTEK_ROG_CLAYMORE_II_KEYBOARD),
QUIRK_ROG_CLAYMORE_II_KEYBOARD },
--
2.45.2
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH] hid-asus-ally: Add full gamepad support
2024-08-06 8:12 Luke D. Jones
@ 2024-08-06 21:34 ` kernel test robot
2024-08-06 22:20 ` Luke Jones
2024-08-07 9:50 ` kernel test robot
2024-08-07 11:43 ` kernel test robot
2 siblings, 1 reply; 13+ messages in thread
From: kernel test robot @ 2024-08-06 21:34 UTC (permalink / raw)
To: Luke D. Jones, linux-input
Cc: llvm, oe-kbuild-all, linux-kernel, bentiss, jikos, Luke D. Jones
Hi Luke,
kernel test robot noticed the following build warnings:
[auto build test WARNING on hid/for-next]
[also build test WARNING on next-20240806]
[cannot apply to linus/master v6.11-rc2]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Luke-D-Jones/hid-asus-ally-Add-full-gamepad-support/20240806-170850
base: https://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git for-next
patch link: https://lore.kernel.org/r/20240806081212.56860-1-luke%40ljones.dev
patch subject: [PATCH] hid-asus-ally: Add full gamepad support
config: hexagon-allmodconfig (https://download.01.org/0day-ci/archive/20240807/202408070520.r1N2Tpys-lkp@intel.com/config)
compiler: clang version 20.0.0git (https://github.com/llvm/llvm-project 423aec6573df4424f90555468128e17073ddc69e)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240807/202408070520.r1N2Tpys-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202408070520.r1N2Tpys-lkp@intel.com/
All warnings (new ones prefixed by >>):
In file included from drivers/hid/hid-asus-ally.c:17:
In file included from include/linux/hid.h:29:
In file included from include/linux/hid_bpf.h:6:
In file included from include/linux/bpf.h:21:
In file included from include/linux/kallsyms.h:13:
In file included from include/linux/mm.h:2228:
include/linux/vmstat.h:514:36: warning: arithmetic between different enumeration types ('enum node_stat_item' and 'enum lru_list') [-Wenum-enum-conversion]
514 | return node_stat_name(NR_LRU_BASE + lru) + 3; // skip "nr_"
| ~~~~~~~~~~~ ^ ~~~
In file included from drivers/hid/hid-asus-ally.c:17:
In file included from include/linux/hid.h:29:
In file included from include/linux/hid_bpf.h:6:
In file included from include/linux/bpf.h:31:
In file included from include/linux/memcontrol.h:13:
In file included from include/linux/cgroup.h:25:
In file included from include/linux/kernel_stat.h:8:
In file included from include/linux/interrupt.h:11:
In file included from include/linux/hardirq.h:11:
In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1:
In file included from include/asm-generic/hardirq.h:17:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:14:
In file included from arch/hexagon/include/asm/io.h:328:
include/asm-generic/io.h:548:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
548 | val = __raw_readb(PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:561:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
561 | val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
| ~~~~~~~~~~ ^
include/uapi/linux/byteorder/little_endian.h:37:51: note: expanded from macro '__le16_to_cpu'
37 | #define __le16_to_cpu(x) ((__force __u16)(__le16)(x))
| ^
In file included from drivers/hid/hid-asus-ally.c:17:
In file included from include/linux/hid.h:29:
In file included from include/linux/hid_bpf.h:6:
In file included from include/linux/bpf.h:31:
In file included from include/linux/memcontrol.h:13:
In file included from include/linux/cgroup.h:25:
In file included from include/linux/kernel_stat.h:8:
In file included from include/linux/interrupt.h:11:
In file included from include/linux/hardirq.h:11:
In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1:
In file included from include/asm-generic/hardirq.h:17:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:14:
In file included from arch/hexagon/include/asm/io.h:328:
include/asm-generic/io.h:574:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
574 | val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
| ~~~~~~~~~~ ^
include/uapi/linux/byteorder/little_endian.h:35:51: note: expanded from macro '__le32_to_cpu'
35 | #define __le32_to_cpu(x) ((__force __u32)(__le32)(x))
| ^
In file included from drivers/hid/hid-asus-ally.c:17:
In file included from include/linux/hid.h:29:
In file included from include/linux/hid_bpf.h:6:
In file included from include/linux/bpf.h:31:
In file included from include/linux/memcontrol.h:13:
In file included from include/linux/cgroup.h:25:
In file included from include/linux/kernel_stat.h:8:
In file included from include/linux/interrupt.h:11:
In file included from include/linux/hardirq.h:11:
In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1:
In file included from include/asm-generic/hardirq.h:17:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:14:
In file included from arch/hexagon/include/asm/io.h:328:
include/asm-generic/io.h:585:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
585 | __raw_writeb(value, PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:595:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
595 | __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:605:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
605 | __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
drivers/hid/hid-asus-ally.c:565:10: warning: variable 'max_output_report_size' set but not used [-Wunused-but-set-variable]
565 | uint8_t max_output_report_size;
| ^
drivers/hid/hid-asus-ally.c:1177:6: warning: variable 'cmd' set but not used [-Wunused-but-set-variable]
1177 | int cmd, side, is_tr;
| ^
>> drivers/hid/hid-asus-ally.c:894:1: warning: unused variable 'btn_mapping_lt_attr_group' [-Wunused-const-variable]
894 | ALLY_BTN_MAPPING(lt, btn_pair_lt_rt, btn_pair_side_left);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/hid/hid-asus-ally.h:321:2: note: expanded from macro 'ALLY_BTN_MAPPING'
321 | ALLY_BTN_ATTRS_GROUP(btn_##_fname, btn_mapping_##_fname)
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/hid/hid-asus-ally.h:306:38: note: expanded from macro 'ALLY_BTN_ATTRS_GROUP'
306 | static const struct attribute_group _fname##_attr_group = { \
| ^~~~~~~~~~~~~~~~~~~
<scratch space>:125:1: note: expanded from here
125 | btn_mapping_lt_attr_group
| ^~~~~~~~~~~~~~~~~~~~~~~~~
>> drivers/hid/hid-asus-ally.c:895:1: warning: unused variable 'btn_mapping_rt_attr_group' [-Wunused-const-variable]
895 | ALLY_BTN_MAPPING(rt, btn_pair_lt_rt, btn_pair_side_right);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/hid/hid-asus-ally.h:321:2: note: expanded from macro 'ALLY_BTN_MAPPING'
321 | ALLY_BTN_ATTRS_GROUP(btn_##_fname, btn_mapping_##_fname)
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/hid/hid-asus-ally.h:306:38: note: expanded from macro 'ALLY_BTN_ATTRS_GROUP'
306 | static const struct attribute_group _fname##_attr_group = { \
| ^~~~~~~~~~~~~~~~~~~
<scratch space>:167:1: note: expanded from here
167 | btn_mapping_rt_attr_group
| ^~~~~~~~~~~~~~~~~~~~~~~~~
11 warnings generated.
vim +/btn_mapping_lt_attr_group +894 drivers/hid/hid-asus-ally.c
882
883 /* button map attributes, regular and macro*/
884 ALLY_BTN_MAPPING(m2, btn_pair_m1_m2, btn_pair_side_left);
885 ALLY_BTN_MAPPING(m1, btn_pair_m1_m2, btn_pair_side_right);
886 ALLY_BTN_MAPPING(a, btn_pair_a_b, btn_pair_side_left);
887 ALLY_BTN_MAPPING(b, btn_pair_a_b, btn_pair_side_right);
888 ALLY_BTN_MAPPING(x, btn_pair_x_y, btn_pair_side_left);
889 ALLY_BTN_MAPPING(y, btn_pair_x_y, btn_pair_side_right);
890 ALLY_BTN_MAPPING(lb, btn_pair_lb_rb, btn_pair_side_left);
891 ALLY_BTN_MAPPING(rb, btn_pair_lb_rb, btn_pair_side_right);
892 ALLY_BTN_MAPPING(ls, btn_pair_ls_rs, btn_pair_side_left);
893 ALLY_BTN_MAPPING(rs, btn_pair_ls_rs, btn_pair_side_right);
> 894 ALLY_BTN_MAPPING(lt, btn_pair_lt_rt, btn_pair_side_left);
> 895 ALLY_BTN_MAPPING(rt, btn_pair_lt_rt, btn_pair_side_right);
896 ALLY_BTN_MAPPING(dpad_u, btn_pair_dpad_u_d, btn_pair_side_left);
897 ALLY_BTN_MAPPING(dpad_d, btn_pair_dpad_u_d, btn_pair_side_right);
898 ALLY_BTN_MAPPING(dpad_l, btn_pair_dpad_l_r, btn_pair_side_left);
899 ALLY_BTN_MAPPING(dpad_r, btn_pair_dpad_l_r, btn_pair_side_right);
900 ALLY_BTN_MAPPING(view, btn_pair_view_menu, btn_pair_side_left);
901 ALLY_BTN_MAPPING(menu, btn_pair_view_menu, btn_pair_side_right);
902
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] hid-asus-ally: Add full gamepad support
2024-08-06 21:34 ` kernel test robot
@ 2024-08-06 22:20 ` Luke Jones
0 siblings, 0 replies; 13+ messages in thread
From: Luke Jones @ 2024-08-06 22:20 UTC (permalink / raw)
To: kernel test robot, linux-input
Cc: llvm, oe-kbuild-all, linux-kernel, Benjamin Tissoires,
Jiri Kosina
On Wed, 7 Aug 2024, at 9:34 AM, kernel test robot wrote:
> Hi Luke,
>
> kernel test robot noticed the following build warnings:
>
> [auto build test WARNING on hid/for-next]
> [also build test WARNING on next-20240806]
> [cannot apply to linus/master v6.11-rc2]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch#_base_tree_information]
>
> url: https://github.com/intel-lab-lkp/linux/commits/Luke-D-Jones/hid-asus-ally-Add-full-gamepad-support/20240806-170850
> base: https://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git for-next
> patch link: https://lore.kernel.org/r/20240806081212.56860-1-luke%40ljones.dev
> patch subject: [PATCH] hid-asus-ally: Add full gamepad support
> config: hexagon-allmodconfig (https://download.01.org/0day-ci/archive/20240807/202408070520.r1N2Tpys-lkp@intel.com/config)
> compiler: clang version 20.0.0git (https://github.com/llvm/llvm-project 423aec6573df4424f90555468128e17073ddc69e)
> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240807/202408070520.r1N2Tpys-lkp@intel.com/reproduce)
>
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Closes: https://lore.kernel.org/oe-kbuild-all/202408070520.r1N2Tpys-lkp@intel.com/
>
> All warnings (new ones prefixed by >>):
>
> In file included from drivers/hid/hid-asus-ally.c:17:
> In file included from include/linux/hid.h:29:
> In file included from include/linux/hid_bpf.h:6:
> In file included from include/linux/bpf.h:21:
> In file included from include/linux/kallsyms.h:13:
> In file included from include/linux/mm.h:2228:
> include/linux/vmstat.h:514:36: warning: arithmetic between different enumeration types ('enum node_stat_item' and 'enum lru_list') [-Wenum-enum-conversion]
> 514 | return node_stat_name(NR_LRU_BASE + lru) + 3; // skip "nr_"
> | ~~~~~~~~~~~ ^ ~~~
> In file included from drivers/hid/hid-asus-ally.c:17:
> In file included from include/linux/hid.h:29:
> In file included from include/linux/hid_bpf.h:6:
> In file included from include/linux/bpf.h:31:
> In file included from include/linux/memcontrol.h:13:
> In file included from include/linux/cgroup.h:25:
> In file included from include/linux/kernel_stat.h:8:
> In file included from include/linux/interrupt.h:11:
> In file included from include/linux/hardirq.h:11:
> In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1:
> In file included from include/asm-generic/hardirq.h:17:
> In file included from include/linux/irq.h:20:
> In file included from include/linux/io.h:14:
> In file included from arch/hexagon/include/asm/io.h:328:
> include/asm-generic/io.h:548:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
> 548 | val = __raw_readb(PCI_IOBASE + addr);
> | ~~~~~~~~~~ ^
> include/asm-generic/io.h:561:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
> 561 | val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
> | ~~~~~~~~~~ ^
> include/uapi/linux/byteorder/little_endian.h:37:51: note: expanded from macro '__le16_to_cpu'
> 37 | #define __le16_to_cpu(x) ((__force __u16)(__le16)(x))
> | ^
> In file included from drivers/hid/hid-asus-ally.c:17:
> In file included from include/linux/hid.h:29:
> In file included from include/linux/hid_bpf.h:6:
> In file included from include/linux/bpf.h:31:
> In file included from include/linux/memcontrol.h:13:
> In file included from include/linux/cgroup.h:25:
> In file included from include/linux/kernel_stat.h:8:
> In file included from include/linux/interrupt.h:11:
> In file included from include/linux/hardirq.h:11:
> In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1:
> In file included from include/asm-generic/hardirq.h:17:
> In file included from include/linux/irq.h:20:
> In file included from include/linux/io.h:14:
> In file included from arch/hexagon/include/asm/io.h:328:
> include/asm-generic/io.h:574:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
> 574 | val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
> | ~~~~~~~~~~ ^
> include/uapi/linux/byteorder/little_endian.h:35:51: note: expanded from macro '__le32_to_cpu'
> 35 | #define __le32_to_cpu(x) ((__force __u32)(__le32)(x))
> | ^
> In file included from drivers/hid/hid-asus-ally.c:17:
> In file included from include/linux/hid.h:29:
> In file included from include/linux/hid_bpf.h:6:
> In file included from include/linux/bpf.h:31:
> In file included from include/linux/memcontrol.h:13:
> In file included from include/linux/cgroup.h:25:
> In file included from include/linux/kernel_stat.h:8:
> In file included from include/linux/interrupt.h:11:
> In file included from include/linux/hardirq.h:11:
> In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1:
> In file included from include/asm-generic/hardirq.h:17:
> In file included from include/linux/irq.h:20:
> In file included from include/linux/io.h:14:
> In file included from arch/hexagon/include/asm/io.h:328:
> include/asm-generic/io.h:585:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
> 585 | __raw_writeb(value, PCI_IOBASE + addr);
> | ~~~~~~~~~~ ^
> include/asm-generic/io.h:595:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
> 595 | __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
> | ~~~~~~~~~~ ^
> include/asm-generic/io.h:605:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
> 605 | __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
> | ~~~~~~~~~~ ^
> drivers/hid/hid-asus-ally.c:565:10: warning: variable 'max_output_report_size' set but not used [-Wunused-but-set-variable]
> 565 | uint8_t max_output_report_size;
> | ^
> drivers/hid/hid-asus-ally.c:1177:6: warning: variable 'cmd' set but not used [-Wunused-but-set-variable]
> 1177 | int cmd, side, is_tr;
> | ^
> >> drivers/hid/hid-asus-ally.c:894:1: warning: unused variable 'btn_mapping_lt_attr_group' [-Wunused-const-variable]
> 894 | ALLY_BTN_MAPPING(lt, btn_pair_lt_rt, btn_pair_side_left);
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> drivers/hid/hid-asus-ally.h:321:2: note: expanded from macro 'ALLY_BTN_MAPPING'
> 321 | ALLY_BTN_ATTRS_GROUP(btn_##_fname, btn_mapping_##_fname)
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> drivers/hid/hid-asus-ally.h:306:38: note: expanded from macro 'ALLY_BTN_ATTRS_GROUP'
> 306 | static const struct attribute_group _fname##_attr_group = { \
> | ^~~~~~~~~~~~~~~~~~~
> <scratch space>:125:1: note: expanded from here
> 125 | btn_mapping_lt_attr_group
> | ^~~~~~~~~~~~~~~~~~~~~~~~~
> >> drivers/hid/hid-asus-ally.c:895:1: warning: unused variable 'btn_mapping_rt_attr_group' [-Wunused-const-variable]
> 895 | ALLY_BTN_MAPPING(rt, btn_pair_lt_rt, btn_pair_side_right);
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> drivers/hid/hid-asus-ally.h:321:2: note: expanded from macro 'ALLY_BTN_MAPPING'
> 321 | ALLY_BTN_ATTRS_GROUP(btn_##_fname, btn_mapping_##_fname)
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> drivers/hid/hid-asus-ally.h:306:38: note: expanded from macro 'ALLY_BTN_ATTRS_GROUP'
> 306 | static const struct attribute_group _fname##_attr_group = { \
> | ^~~~~~~~~~~~~~~~~~~
> <scratch space>:167:1: note: expanded from here
> 167 | btn_mapping_rt_attr_group
> | ^~~~~~~~~~~~~~~~~~~~~~~~~
> 11 warnings generated.
>
>
> vim +/btn_mapping_lt_attr_group +894 drivers/hid/hid-asus-ally.c
>
> 882
> 883 /* button map attributes, regular and macro*/
> 884 ALLY_BTN_MAPPING(m2, btn_pair_m1_m2, btn_pair_side_left);
> 885 ALLY_BTN_MAPPING(m1, btn_pair_m1_m2, btn_pair_side_right);
> 886 ALLY_BTN_MAPPING(a, btn_pair_a_b, btn_pair_side_left);
> 887 ALLY_BTN_MAPPING(b, btn_pair_a_b, btn_pair_side_right);
> 888 ALLY_BTN_MAPPING(x, btn_pair_x_y, btn_pair_side_left);
> 889 ALLY_BTN_MAPPING(y, btn_pair_x_y, btn_pair_side_right);
> 890 ALLY_BTN_MAPPING(lb, btn_pair_lb_rb, btn_pair_side_left);
> 891 ALLY_BTN_MAPPING(rb, btn_pair_lb_rb, btn_pair_side_right);
> 892 ALLY_BTN_MAPPING(ls, btn_pair_ls_rs, btn_pair_side_left);
> 893 ALLY_BTN_MAPPING(rs, btn_pair_ls_rs, btn_pair_side_right);
> > 894 ALLY_BTN_MAPPING(lt, btn_pair_lt_rt, btn_pair_side_left);
> > 895 ALLY_BTN_MAPPING(rt, btn_pair_lt_rt, btn_pair_side_right);
> 896 ALLY_BTN_MAPPING(dpad_u, btn_pair_dpad_u_d, btn_pair_side_left);
> 897 ALLY_BTN_MAPPING(dpad_d, btn_pair_dpad_u_d, btn_pair_side_right);
> 898 ALLY_BTN_MAPPING(dpad_l, btn_pair_dpad_l_r, btn_pair_side_left);
> 899 ALLY_BTN_MAPPING(dpad_r, btn_pair_dpad_l_r, btn_pair_side_right);
> 900 ALLY_BTN_MAPPING(view, btn_pair_view_menu, btn_pair_side_left);
> 901 ALLY_BTN_MAPPING(menu, btn_pair_view_menu, btn_pair_side_right);
> 902
>
> --
> 0-DAY CI Kernel Test Service
> https://github.com/intel/lkp-tests/wiki
I like this service. The above warnings will be fixed in next version after I get some review.
Regards,
Luke.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] hid-asus-ally: Add full gamepad support
2024-08-06 8:12 Luke D. Jones
2024-08-06 21:34 ` kernel test robot
@ 2024-08-07 9:50 ` kernel test robot
2024-08-07 11:43 ` kernel test robot
2 siblings, 0 replies; 13+ messages in thread
From: kernel test robot @ 2024-08-07 9:50 UTC (permalink / raw)
To: Luke D. Jones, linux-input
Cc: oe-kbuild-all, linux-kernel, bentiss, jikos, Luke D. Jones
Hi Luke,
kernel test robot noticed the following build warnings:
[auto build test WARNING on hid/for-next]
[also build test WARNING on next-20240807]
[cannot apply to linus/master v6.11-rc2]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Luke-D-Jones/hid-asus-ally-Add-full-gamepad-support/20240806-170850
base: https://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git for-next
patch link: https://lore.kernel.org/r/20240806081212.56860-1-luke%40ljones.dev
patch subject: [PATCH] hid-asus-ally: Add full gamepad support
config: parisc-allmodconfig (https://download.01.org/0day-ci/archive/20240807/202408071743.00IxSKrf-lkp@intel.com/config)
compiler: hppa-linux-gcc (GCC) 14.1.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240807/202408071743.00IxSKrf-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202408071743.00IxSKrf-lkp@intel.com/
All warnings (new ones prefixed by >>):
drivers/hid/hid-asus-ally.c: In function 'ally_x_create':
>> drivers/hid/hid-asus-ally.c:565:17: warning: variable 'max_output_report_size' set but not used [-Wunused-but-set-variable]
565 | uint8_t max_output_report_size;
| ^~~~~~~~~~~~~~~~~~~~~~
drivers/hid/hid-asus-ally.c: In function '__gamepad_store_deadzones':
>> drivers/hid/hid-asus-ally.c:1177:13: warning: variable 'cmd' set but not used [-Wunused-but-set-variable]
1177 | int cmd, side, is_tr;
| ^~~
In file included from drivers/hid/hid-asus-ally.c:24:
drivers/hid/hid-asus-ally.c: At top level:
>> drivers/hid/hid-asus-ally.h:321:44: warning: 'btn_mapping_rt_attr_group' defined but not used [-Wunused-const-variable=]
321 | ALLY_BTN_ATTRS_GROUP(btn_##_fname, btn_mapping_##_fname)
| ^~~~~~~~~~~~
drivers/hid/hid-asus-ally.h:306:45: note: in definition of macro 'ALLY_BTN_ATTRS_GROUP'
306 | static const struct attribute_group _fname##_attr_group = { \
| ^~~~~~
drivers/hid/hid-asus-ally.c:895:1: note: in expansion of macro 'ALLY_BTN_MAPPING'
895 | ALLY_BTN_MAPPING(rt, btn_pair_lt_rt, btn_pair_side_right);
| ^~~~~~~~~~~~~~~~
>> drivers/hid/hid-asus-ally.h:321:44: warning: 'btn_mapping_lt_attr_group' defined but not used [-Wunused-const-variable=]
321 | ALLY_BTN_ATTRS_GROUP(btn_##_fname, btn_mapping_##_fname)
| ^~~~~~~~~~~~
drivers/hid/hid-asus-ally.h:306:45: note: in definition of macro 'ALLY_BTN_ATTRS_GROUP'
306 | static const struct attribute_group _fname##_attr_group = { \
| ^~~~~~
drivers/hid/hid-asus-ally.c:894:1: note: in expansion of macro 'ALLY_BTN_MAPPING'
894 | ALLY_BTN_MAPPING(lt, btn_pair_lt_rt, btn_pair_side_left);
| ^~~~~~~~~~~~~~~~
vim +/max_output_report_size +565 drivers/hid/hid-asus-ally.c
562
563 static struct ally_x_device *ally_x_create(struct hid_device *hdev)
564 {
> 565 uint8_t max_output_report_size;
566 struct ally_x_device *ally_x;
567 struct ff_report *report;
568 int ret;
569
570 ally_x = devm_kzalloc(&hdev->dev, sizeof(*ally_x), GFP_KERNEL);
571 if (!ally_x)
572 return ERR_PTR(-ENOMEM);
573
574 ally_x->hdev = hdev;
575 INIT_WORK(&ally_x->output_worker, ally_x_work);
576 spin_lock_init(&ally_x->lock);
577 ally_x->output_worker_initialized = true;
578 ally_x->qam_btns_steam_mode =
579 true; /* Always default to steam mode, it can be changed by userspace attr */
580
581 max_output_report_size = sizeof(struct ally_x_input_report);
582 report = devm_kzalloc(&hdev->dev, sizeof(*report), GFP_KERNEL);
583 if (!report) {
584 ret = -ENOMEM;
585 goto free_ally_x;
586 }
587
588 /* None of these bytes will change for the FF command for now */
589 report->report_id = 0x0D;
590 report->ff.enable = 0x0F; /* Enable all by default */
591 report->ff.pulse_sustain_10ms = 0xFF; /* Duration */
592 report->ff.pulse_release_10ms = 0x00; /* Start Delay */
593 report->ff.loop_count = 0xEB; /* Loop Count */
594 ally_x->ff_packet = report;
595
596 ally_x->input = ally_x_setup_input(hdev);
597 if (IS_ERR(ally_x->input)) {
598 ret = PTR_ERR(ally_x->input);
599 goto free_ff_packet;
600 }
601
602 if (sysfs_create_file(&hdev->dev.kobj, &dev_attr_ally_x_qam_mode.attr)) {
603 ret = -ENODEV;
604 goto unregister_input;
605 }
606
607 ally_x->update_ff = true;
608 if (ally_x->output_worker_initialized)
609 schedule_work(&ally_x->output_worker);
610
611 hid_info(hdev, "Registered Ally X controller using %s\n",
612 dev_name(&ally_x->input->dev));
613 return ally_x;
614
615 unregister_input:
616 input_unregister_device(ally_x->input);
617 free_ff_packet:
618 kfree(ally_x->ff_packet);
619 free_ally_x:
620 kfree(ally_x);
621 return ERR_PTR(ret);
622 }
623
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] hid-asus-ally: Add full gamepad support
2024-08-06 8:12 Luke D. Jones
2024-08-06 21:34 ` kernel test robot
2024-08-07 9:50 ` kernel test robot
@ 2024-08-07 11:43 ` kernel test robot
2 siblings, 0 replies; 13+ messages in thread
From: kernel test robot @ 2024-08-07 11:43 UTC (permalink / raw)
To: Luke D. Jones, linux-input
Cc: oe-kbuild-all, linux-kernel, bentiss, jikos, Luke D. Jones
Hi Luke,
kernel test robot noticed the following build warnings:
[auto build test WARNING on hid/for-next]
[also build test WARNING on next-20240807]
[cannot apply to linus/master v6.11-rc2]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Luke-D-Jones/hid-asus-ally-Add-full-gamepad-support/20240806-170850
base: https://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git for-next
patch link: https://lore.kernel.org/r/20240806081212.56860-1-luke%40ljones.dev
patch subject: [PATCH] hid-asus-ally: Add full gamepad support
config: csky-randconfig-r113-20240807 (https://download.01.org/0day-ci/archive/20240807/202408071932.E7Cxqi0x-lkp@intel.com/config)
compiler: csky-linux-gcc (GCC) 14.1.0
reproduce: (https://download.01.org/0day-ci/archive/20240807/202408071932.E7Cxqi0x-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202408071932.E7Cxqi0x-lkp@intel.com/
sparse warnings: (new ones prefixed by >>)
>> drivers/hid/hid-asus-ally.c:561:1: sparse: sparse: symbol 'dev_attr_ally_x_qam_mode' was not declared. Should it be static?
>> drivers/hid/hid-asus-ally.c:845:1: sparse: sparse: symbol 'dev_attr_btn_mapping_apply' was not declared. Should it be static?
>> drivers/hid/hid-asus-ally.c:884:1: sparse: sparse: symbol 'dev_attr_btn_mapping_m2' was not declared. Should it be static?
>> drivers/hid/hid-asus-ally.c:884:1: sparse: sparse: symbol 'dev_attr_btn_mapping_m2_macro' was not declared. Should it be static?
>> drivers/hid/hid-asus-ally.c:884:1: sparse: sparse: symbol 'dev_attr_btn_mapping_m2_turbo' was not declared. Should it be static?
>> drivers/hid/hid-asus-ally.c:885:1: sparse: sparse: symbol 'dev_attr_btn_mapping_m1' was not declared. Should it be static?
>> drivers/hid/hid-asus-ally.c:885:1: sparse: sparse: symbol 'dev_attr_btn_mapping_m1_macro' was not declared. Should it be static?
>> drivers/hid/hid-asus-ally.c:885:1: sparse: sparse: symbol 'dev_attr_btn_mapping_m1_turbo' was not declared. Should it be static?
>> drivers/hid/hid-asus-ally.c:886:1: sparse: sparse: symbol 'dev_attr_btn_mapping_a' was not declared. Should it be static?
>> drivers/hid/hid-asus-ally.c:886:1: sparse: sparse: symbol 'dev_attr_btn_mapping_a_macro' was not declared. Should it be static?
>> drivers/hid/hid-asus-ally.c:886:1: sparse: sparse: symbol 'dev_attr_btn_mapping_a_turbo' was not declared. Should it be static?
>> drivers/hid/hid-asus-ally.c:887:1: sparse: sparse: symbol 'dev_attr_btn_mapping_b' was not declared. Should it be static?
>> drivers/hid/hid-asus-ally.c:887:1: sparse: sparse: symbol 'dev_attr_btn_mapping_b_macro' was not declared. Should it be static?
>> drivers/hid/hid-asus-ally.c:887:1: sparse: sparse: symbol 'dev_attr_btn_mapping_b_turbo' was not declared. Should it be static?
>> drivers/hid/hid-asus-ally.c:888:1: sparse: sparse: symbol 'dev_attr_btn_mapping_x' was not declared. Should it be static?
>> drivers/hid/hid-asus-ally.c:888:1: sparse: sparse: symbol 'dev_attr_btn_mapping_x_macro' was not declared. Should it be static?
>> drivers/hid/hid-asus-ally.c:888:1: sparse: sparse: symbol 'dev_attr_btn_mapping_x_turbo' was not declared. Should it be static?
>> drivers/hid/hid-asus-ally.c:889:1: sparse: sparse: symbol 'dev_attr_btn_mapping_y' was not declared. Should it be static?
>> drivers/hid/hid-asus-ally.c:889:1: sparse: sparse: symbol 'dev_attr_btn_mapping_y_macro' was not declared. Should it be static?
>> drivers/hid/hid-asus-ally.c:889:1: sparse: sparse: symbol 'dev_attr_btn_mapping_y_turbo' was not declared. Should it be static?
>> drivers/hid/hid-asus-ally.c:890:1: sparse: sparse: symbol 'dev_attr_btn_mapping_lb' was not declared. Should it be static?
>> drivers/hid/hid-asus-ally.c:890:1: sparse: sparse: symbol 'dev_attr_btn_mapping_lb_macro' was not declared. Should it be static?
>> drivers/hid/hid-asus-ally.c:890:1: sparse: sparse: symbol 'dev_attr_btn_mapping_lb_turbo' was not declared. Should it be static?
>> drivers/hid/hid-asus-ally.c:891:1: sparse: sparse: symbol 'dev_attr_btn_mapping_rb' was not declared. Should it be static?
>> drivers/hid/hid-asus-ally.c:891:1: sparse: sparse: symbol 'dev_attr_btn_mapping_rb_macro' was not declared. Should it be static?
>> drivers/hid/hid-asus-ally.c:891:1: sparse: sparse: symbol 'dev_attr_btn_mapping_rb_turbo' was not declared. Should it be static?
>> drivers/hid/hid-asus-ally.c:892:1: sparse: sparse: symbol 'dev_attr_btn_mapping_ls' was not declared. Should it be static?
>> drivers/hid/hid-asus-ally.c:892:1: sparse: sparse: symbol 'dev_attr_btn_mapping_ls_macro' was not declared. Should it be static?
>> drivers/hid/hid-asus-ally.c:892:1: sparse: sparse: symbol 'dev_attr_btn_mapping_ls_turbo' was not declared. Should it be static?
>> drivers/hid/hid-asus-ally.c:893:1: sparse: sparse: symbol 'dev_attr_btn_mapping_rs' was not declared. Should it be static?
>> drivers/hid/hid-asus-ally.c:893:1: sparse: sparse: symbol 'dev_attr_btn_mapping_rs_macro' was not declared. Should it be static?
>> drivers/hid/hid-asus-ally.c:893:1: sparse: sparse: symbol 'dev_attr_btn_mapping_rs_turbo' was not declared. Should it be static?
>> drivers/hid/hid-asus-ally.c:894:1: sparse: sparse: symbol 'dev_attr_btn_mapping_lt' was not declared. Should it be static?
>> drivers/hid/hid-asus-ally.c:894:1: sparse: sparse: symbol 'dev_attr_btn_mapping_lt_macro' was not declared. Should it be static?
>> drivers/hid/hid-asus-ally.c:894:1: sparse: sparse: symbol 'dev_attr_btn_mapping_lt_turbo' was not declared. Should it be static?
>> drivers/hid/hid-asus-ally.c:895:1: sparse: sparse: symbol 'dev_attr_btn_mapping_rt' was not declared. Should it be static?
>> drivers/hid/hid-asus-ally.c:895:1: sparse: sparse: symbol 'dev_attr_btn_mapping_rt_macro' was not declared. Should it be static?
>> drivers/hid/hid-asus-ally.c:895:1: sparse: sparse: symbol 'dev_attr_btn_mapping_rt_turbo' was not declared. Should it be static?
>> drivers/hid/hid-asus-ally.c:896:1: sparse: sparse: symbol 'dev_attr_btn_mapping_dpad_u' was not declared. Should it be static?
>> drivers/hid/hid-asus-ally.c:896:1: sparse: sparse: symbol 'dev_attr_btn_mapping_dpad_u_macro' was not declared. Should it be static?
>> drivers/hid/hid-asus-ally.c:896:1: sparse: sparse: symbol 'dev_attr_btn_mapping_dpad_u_turbo' was not declared. Should it be static?
>> drivers/hid/hid-asus-ally.c:897:1: sparse: sparse: symbol 'dev_attr_btn_mapping_dpad_d' was not declared. Should it be static?
>> drivers/hid/hid-asus-ally.c:897:1: sparse: sparse: symbol 'dev_attr_btn_mapping_dpad_d_macro' was not declared. Should it be static?
>> drivers/hid/hid-asus-ally.c:897:1: sparse: sparse: symbol 'dev_attr_btn_mapping_dpad_d_turbo' was not declared. Should it be static?
>> drivers/hid/hid-asus-ally.c:898:1: sparse: sparse: symbol 'dev_attr_btn_mapping_dpad_l' was not declared. Should it be static?
>> drivers/hid/hid-asus-ally.c:898:1: sparse: sparse: symbol 'dev_attr_btn_mapping_dpad_l_macro' was not declared. Should it be static?
>> drivers/hid/hid-asus-ally.c:898:1: sparse: sparse: symbol 'dev_attr_btn_mapping_dpad_l_turbo' was not declared. Should it be static?
>> drivers/hid/hid-asus-ally.c:899:1: sparse: sparse: symbol 'dev_attr_btn_mapping_dpad_r' was not declared. Should it be static?
>> drivers/hid/hid-asus-ally.c:899:1: sparse: sparse: symbol 'dev_attr_btn_mapping_dpad_r_macro' was not declared. Should it be static?
>> drivers/hid/hid-asus-ally.c:899:1: sparse: sparse: symbol 'dev_attr_btn_mapping_dpad_r_turbo' was not declared. Should it be static?
>> drivers/hid/hid-asus-ally.c:900:1: sparse: sparse: symbol 'dev_attr_btn_mapping_view' was not declared. Should it be static?
>> drivers/hid/hid-asus-ally.c:900:1: sparse: sparse: symbol 'dev_attr_btn_mapping_view_macro' was not declared. Should it be static?
>> drivers/hid/hid-asus-ally.c:900:1: sparse: sparse: symbol 'dev_attr_btn_mapping_view_turbo' was not declared. Should it be static?
>> drivers/hid/hid-asus-ally.c:901:1: sparse: sparse: symbol 'dev_attr_btn_mapping_menu' was not declared. Should it be static?
>> drivers/hid/hid-asus-ally.c:901:1: sparse: sparse: symbol 'dev_attr_btn_mapping_menu_macro' was not declared. Should it be static?
>> drivers/hid/hid-asus-ally.c:901:1: sparse: sparse: symbol 'dev_attr_btn_mapping_menu_turbo' was not declared. Should it be static?
>> drivers/hid/hid-asus-ally.c:952:1: sparse: sparse: symbol 'dev_attr_btn_mapping_reset' was not declared. Should it be static?
>> drivers/hid/hid-asus-ally.c:1028:1: sparse: sparse: symbol 'dev_attr_gamepad_mode' was not declared. Should it be static?
>> drivers/hid/hid-asus-ally.c:1037:1: sparse: sparse: symbol 'dev_attr_gamepad_vibration_intensity_index' was not declared. Should it be static?
>> drivers/hid/hid-asus-ally.c:1115:1: sparse: sparse: symbol 'dev_attr_gamepad_vibration_intensity' was not declared. Should it be static?
>> drivers/hid/hid-asus-ally.c:1202:1: sparse: sparse: symbol 'dev_attr_axis_xyz_deadzone_index' was not declared. Should it be static?
>> drivers/hid/hid-asus-ally.c:1204:1: sparse: sparse: symbol 'dev_attr_xpad_axis_xy_left_deadzone' was not declared. Should it be static?
>> drivers/hid/hid-asus-ally.c:1205:1: sparse: sparse: symbol 'dev_attr_xpad_axis_xy_right_deadzone' was not declared. Should it be static?
>> drivers/hid/hid-asus-ally.c:1206:1: sparse: sparse: symbol 'dev_attr_xpad_axis_z_left_deadzone' was not declared. Should it be static?
>> drivers/hid/hid-asus-ally.c:1207:1: sparse: sparse: symbol 'dev_attr_xpad_axis_z_right_deadzone' was not declared. Should it be static?
>> drivers/hid/hid-asus-ally.c:1288:1: sparse: sparse: symbol 'dev_attr_xpad_axis_xy_left_ADZ' was not declared. Should it be static?
>> drivers/hid/hid-asus-ally.c:1313:1: sparse: sparse: symbol 'dev_attr_xpad_axis_xy_right_ADZ' was not declared. Should it be static?
>> drivers/hid/hid-asus-ally.c:1321:1: sparse: sparse: symbol 'dev_attr_rc_point_index' was not declared. Should it be static?
>> drivers/hid/hid-asus-ally.c:1390:1: sparse: sparse: symbol 'dev_attr_rc_point_left_1' was not declared. Should it be static?
>> drivers/hid/hid-asus-ally.c:1391:1: sparse: sparse: symbol 'dev_attr_rc_point_left_2' was not declared. Should it be static?
>> drivers/hid/hid-asus-ally.c:1392:1: sparse: sparse: symbol 'dev_attr_rc_point_left_3' was not declared. Should it be static?
>> drivers/hid/hid-asus-ally.c:1393:1: sparse: sparse: symbol 'dev_attr_rc_point_left_4' was not declared. Should it be static?
>> drivers/hid/hid-asus-ally.c:1395:1: sparse: sparse: symbol 'dev_attr_rc_point_right_1' was not declared. Should it be static?
>> drivers/hid/hid-asus-ally.c:1396:1: sparse: sparse: symbol 'dev_attr_rc_point_right_2' was not declared. Should it be static?
>> drivers/hid/hid-asus-ally.c:1397:1: sparse: sparse: symbol 'dev_attr_rc_point_right_3' was not declared. Should it be static?
>> drivers/hid/hid-asus-ally.c:1398:1: sparse: sparse: symbol 'dev_attr_rc_point_right_4' was not declared. Should it be static?
>> drivers/hid/hid-asus-ally.c:1586:1: sparse: sparse: symbol 'dev_attr_xpad_axis_xy_left_cal' was not declared. Should it be static?
>> drivers/hid/hid-asus-ally.c:1587:1: sparse: sparse: symbol 'dev_attr_xpad_axis_xy_right_cal' was not declared. Should it be static?
>> drivers/hid/hid-asus-ally.c:1588:1: sparse: sparse: symbol 'dev_attr_xpad_axis_z_left_cal' was not declared. Should it be static?
>> drivers/hid/hid-asus-ally.c:1589:1: sparse: sparse: symbol 'dev_attr_xpad_axis_z_right_cal' was not declared. Should it be static?
>> drivers/hid/hid-asus-ally.c:1597:1: sparse: sparse: symbol 'dev_attr_xpad_axis_xy_cal_index' was not declared. Should it be static?
>> drivers/hid/hid-asus-ally.c:1605:1: sparse: sparse: symbol 'dev_attr_xpad_axis_z_cal_index' was not declared. Should it be static?
>> drivers/hid/hid-asus-ally.c:1643:1: sparse: sparse: symbol 'dev_attr_xpad_axis_xy_left_cal_reset' was not declared. Should it be static?
>> drivers/hid/hid-asus-ally.c:1644:1: sparse: sparse: symbol 'dev_attr_xpad_axis_xy_right_cal_reset' was not declared. Should it be static?
>> drivers/hid/hid-asus-ally.c:1645:1: sparse: sparse: symbol 'dev_attr_xpad_axis_z_left_cal_reset' was not declared. Should it be static?
>> drivers/hid/hid-asus-ally.c:1646:1: sparse: sparse: symbol 'dev_attr_xpad_axis_z_right_cal_reset' was not declared. Should it be static?
vim +/dev_attr_ally_x_qam_mode +561 drivers/hid/hid-asus-ally.c
545
546 static ssize_t ally_x_qam_mode_store(struct device *dev, struct device_attribute *attr,
547 const char *buf, size_t count)
548 {
549 struct ally_x_device *ally_x = drvdata.ally_x;
550 bool val;
551 int ret;
552
553 ret = kstrtobool(buf, &val);
554 if (ret < 0)
555 return ret;
556
557 ally_x->qam_btns_steam_mode = val;
558
559 return count;
560 }
> 561 ALLY_DEVICE_ATTR_RW(ally_x_qam_mode, qam_mode);
562
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] hid-asus-ally: Add full gamepad support
@ 2024-08-08 15:35 Antheas Kapenekakis
2024-08-11 9:22 ` Luke Jones
0 siblings, 1 reply; 13+ messages in thread
From: Antheas Kapenekakis @ 2024-08-08 15:35 UTC (permalink / raw)
To: luke; +Cc: bentiss, jikos, linux-input, linux-kernel
Hi Luke,
I reviewed this patch and leave some of my thoughts below.
I am the creator of the Handheld Daemon (HHD) (https://github.com/hhd-dev/hhd)
app, which offers a lot of this functionality using a userspace implementation,
and have thousands of handheld users, a lot of whom are on both Allies,
so I believe I have some valuable feedback on this.
First for some background:
The original Ally used a composite USB device with the following components:
a vendor interface for Armoury Crate, an XInput gamepad device, and a Keyboard
device for things such as WASD and Mouse mode support.
Armoury Crate uses the vendor interface to control device mappings and how the
device should act, including by sending faux commands to it for gyro emulation
using the Bosch gyro built into the screen. As such, no drivers are required for
it in Windows, and the same can be said of Linux. Although without Armoury Crate
in either Windows or Linux, the controller does not behave properly.
In Ally X, Asus seems to have been told by Microsoft that the XInput driver is
being deprecated and they've had to move on to a different one.
For Asus, this meant doing a HID implementation which seems to be
similar to the Xbox Adaptive Controller, and using a custom windows
driver for it
in Windows. Therefore, unlike the original Ally, which did not require Windows
drivers to have its controller work, Ally X does.
It is a similar situation in Linux, where without a kernel driver, the
controller
of the Ally reads as a HID Gamepad with a wrong mapping, and does not have
vibration support.
The patch I am replying to does three things (which ideally should be separated
into different patches): provide an in-kernel equivalent for Armoury
Crate options
that will configure both Ally and Ally X, a HID driver for the Ally X that
restores vibration functionality and a sane mapping to the controllers, and
finally, a cross-interface "hack" which only works on the X and pulls the vendor
"Armoury" and "ROG Crate" buttons into the new HID gamepad driver and turns them
into "BTN_GUIDE" and "BTN_GUIDE+BTN_SOUTH" (which is recognized by
Steam to open the
side menu and by now defunct yuzu to switch from windowed to full screen).
Let me add notes for each one.
## Gamepad HID Driver
For the HID gamepad driver: we are in agreement that such a driver should exist
for the X. It makes basic functionality accessible to users without a userspace
tool and restores mapping and vibration.
Your implementation seems straightforward and I only have a couple of
notes for it:
First, you are hardcoding `desc.bEndpointAddress`, which is error
prone and could
break in the future with an MCU/EC update. You should instead use the Usage Page
and Usage of the device, which in this case are unique. Referencing HHD, for the
HID gamepad, the usage page is 0xFF31 and the usage is 0x0080. And perhaps there
is a gamepad driver already that could support Ally X with a simple quirk.
Following, you attempted to mimic an Xinput device, but your driver does not set
the ABS values of the device properly. For your convenience, here are the values
from the Legion Go, which uses an XInput gamepad:
```
0x0000: ABS_X
> [val 0, min -32768, max 32767, fuzz 16, flat 128, res 0]
0x0001: ABS_Y
> [val -1, min -32768, max 32767, fuzz 16, flat 128, res 0]
0x0002: ABS_Z
> [val 0, min 0, max 255, fuzz 0, flat 0, res 0]
0x0003: ABS_RX
> [val 0, min -32768, max 32767, fuzz 16, flat 128, res 0]
0x0004: ABS_RY
> [val 0, min -32768, max 32767, fuzz 16, flat 128, res 0]
0x0005: ABS_RZ
> [val 0, min 0, max 255, fuzz 0, flat 0, res 0]
0x0010: ABS_HAT0X
> [val 0, min -1, max 1, fuzz 0, flat 0, res 0]
0x0011: ABS_HAT0Y
> [val 0, min -1, max 1, fuzz 0, flat 0, res 0]
```
Specifically, your ABS_X, ABS_Y, ABS_RX, ABS_RY values are set from 0
to ABS_MAX, which is
a deviation from XInput and more similar to DInput. This means that if I added
support for your patchset to HHD which supports over 30 XInput
devices, I would have to
quirk it only for the Ally X. If you break HHD, the chance you break
userspace in
general (e.g., random proton game) is very high.
## Armoury Crate Functionality
Let me move over to armoury crate functionality, which you first started in
December as the XPAD patchset, and as such I have a certain familiarity with it.
I will not reiterate the `desc.bEndpointAddress` quirk.
The vendor interface is a HID device and it is the case both in Linux
and Windows
that configuration is usually done by userspace programs.
I recall multiple examples of this in Linux as well, e.g., Solaar for Logitech,
asusctl (your daemon) for Asus, libratbag for mice, and OpenRGB for
rgb which includes
the Ally. Can you justify the deviation here and the need for a kernel driver?
I am afraid that it might be unreliable, and applying kernel patches
is something
very difficult that most users can not do. Furthermore, given the user chooses
to install a userspace program to manage their controller (e.g., HHD),
you should
ensure this driver functionality can be disabled, from boot, so that it does not
interfere with userspace programs.
For example, one problem I recall that before we removed the XPAD patchset
from the distribution Bazzite, I noticed during reviewing logs that your patch
regularly failed the ready check on certain allies, and after trying
the ready check
myself as part of HHD, I had to pull it after 10 hours because users
came out of
the woodwork to report their controller misbehaved.
Of course, in those cases HHD handled initialization, so your patch failing did
not affect functionality.
I know that some basic initialization is required regardless for the
gamepad to work
without e.g., HHD. In my experience changing the gamepad mode to
Gamepad (from Mouse
mode) with a single HID command should suffice. Then, the rest of the settings
are initialized to manufacturer defaults.
## Xbox/Xbox + A Quirk
As mentioned above, the vendor device of the Ally reports the front manufacturer
buttons as HID commands from the special vendor device. This allows Armoury
crate to open in windows. Of course, in Linux there is no Armoury crate, so
these buttons are unused by default. Userspace programs can fix this
quirk and your previous kernel patches do as much, by exposing the buttons as
F15, F16, F17, F18.
A final component to this patchset is taking this vendor device and making it
accessible from the gamepad driver. This is uniquely possible on the
Ally X, because
the gamepad happens to be HID and located on the same USB interface, so you can,
with a bit of trickery, receive the front button commands from the
gamepad driver.
This allows you to quirk them, and here specifically for steam, to
Xbox and Xbox+A
("BTN_GUIDE" and "BTN_GUIDE+BTN_SOUTH"), which have similar functionality to
the Steamdeck's steam and three dot buttons.
In my opinion, that borders a bit on being too opinionated, and perhaps e.g.,
Asus would also share this opinion. Imagine if Asus tried to add some sort of
official support to linux, only to find out the Linux driver is remapping the
buttons and they have to work around that. Also, combining two HID devices is
a bit unorthodox, and may cause other issues (see `desc.bEndpointAddress`).
So perhaps Asus needs to be consulted here?
Further, I have some advice for your button implementation.
80ms is too short for steam and is a delay I have tried before. It
will fail in low
device TDPs, as Steam will begin to lag and miss the commands. For
your reference,
HHD uses 150ms for this and I have tried 80ms in the past.
In general, I am trying to move away from Xbox+A as it tends to add delay and
since I prefer to multiplex the right vendor button for double
presses, it starts
to become noticable. It also does not work if the user presses the
Nintendo layout
button in the Steam UI.
In addition, you are freezing the kernel driver to send those commands for 240ms
which is around 100 reports.
I would advise that you store that the user has pressed the vendor button and
then when you process the following HID reports, you set the Xbox
button as 1 for
300ms after the button is pressed and the A button to 1 after 150ms until 300ms.
It is fine to release both buttons at the same time.
## Other issues
These are my overall thoughts. Since I went through the effort of
posting to lkml,
I will also draw your attention to 2 other patches you upstreamed and
cause issues
for the original Ally and now Ally X.
The first one is allowing users to enable mcu_powersave in February.
Beforehand, it was force disabled by the kernel by your previous patch.
This feature remains broken when the device is at low TDPs and unplugged.
We force disable it now as part of the distribution Bazzite now.
Applies to both Ally and Ally X.
The second occasion was 3 weeks ago when you pushed the Ally X quirk
and converted
`ally_mcu_usb_switch` into a DMI table. It seems that a DMI table does
not work in
this codepath, and it is something that we had tried over a week
before you posted the
patch and had reverted it. If you had asked me I would have told you as much.
After this patch got cherry picked by CachyOS and ChimeraOS, it ended
up breaking
both Allies, which is not a favorable user experience.
Moving forward, I will try to be more attentive in the kernel conversation and,
e.g., upstream patches. And perhaps you can be a bit more thorough
testing before you push your patches up to the kernel ;).
Good luck with this patch series!
Of course, once you fix the notes I have above and/or your patchset is
integrated
to the kernel, I will make sure HHD is forward compatible with it, while being
backwards compatible with older kernels.
Best Regards,
Antheas
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] hid-asus-ally: Add full gamepad support
2024-08-08 15:35 [PATCH] hid-asus-ally: Add full gamepad support Antheas Kapenekakis
@ 2024-08-11 9:22 ` Luke Jones
2024-08-11 16:10 ` Antheas Kapenekakis
0 siblings, 1 reply; 13+ messages in thread
From: Luke Jones @ 2024-08-11 9:22 UTC (permalink / raw)
To: Antheas Kapenekakis
Cc: Benjamin Tissoires, Jiri Kosina, linux-input, linux-kernel
On Fri, 9 Aug 2024, at 3:35 AM, Antheas Kapenekakis wrote:
> Hi Luke,
> I reviewed this patch and leave some of my thoughts below.
>
> I am the creator of the Handheld Daemon (HHD) (https://github.com/hhd-dev/hhd)
> app, which offers a lot of this functionality using a userspace implementation,
> and have thousands of handheld users, a lot of whom are on both Allies,
> so I believe I have some valuable feedback on this.
Then you are aware that the work I do in kernel is almost always targeting ASUS ROG. You will also be aware that my goal is to enable default user experience *without* the requirement of any userspace blobs - this includes moving things from my own daemon to kernel where possible so that users do not require a userspace app at all if they so desire.
> In Ally X, Asus seems to have been told by Microsoft that the XInput driver is
> being deprecated and they've had to move on to a different one.
> For Asus, this meant doing a HID implementation which seems to be
> similar to the Xbox Adaptive Controller, and using a custom windows
> driver for it
> in Windows. Therefore, unlike the original Ally, which did not require Windows
> drivers to have its controller work, Ally X does.
You're repeating information that has come directly from me.
> First, you are hardcoding `desc.bEndpointAddress`, which is error
> prone and could
> break in the future with an MCU/EC update.
I have many records from many MCU updates. It doesn't happen. This is even true right across the ROG laptop range for as long as I've been tracking these. ASUS is remarkably consistent.
> HID gamepad, the usage page is 0xFF31 and the usage is 0x0080. And perhaps there
> is a gamepad driver already that could support Ally X with a simple quirk.
There isn't. The ones that might support it are not being upstreamed. And that is also *not* my goal here (a complete and accessible default user experience with zero dependency).
> Following, you attempted to mimic an Xinput device, but your driver does not set
> the ABS values of the device properly. For your convenience, here are the values
> from the Legion Go, which uses an XInput gamepad:
> ```
> 0x0000: ABS_X
> > [val 0, min -32768, max 32767, fuzz 16, flat 128, res 0]
> 0x0001: ABS_Y
> > [val -1, min -32768, max 32767, fuzz 16, flat 128, res 0]
> 0x0002: ABS_Z
> > [val 0, min 0, max 255, fuzz 0, flat 0, res 0]
> 0x0003: ABS_RX
> > [val 0, min -32768, max 32767, fuzz 16, flat 128, res 0]
> 0x0004: ABS_RY
> > [val 0, min -32768, max 32767, fuzz 16, flat 128, res 0]
> 0x0005: ABS_RZ
> > [val 0, min 0, max 255, fuzz 0, flat 0, res 0]
> 0x0010: ABS_HAT0X
> > [val 0, min -1, max 1, fuzz 0, flat 0, res 0]
> 0x0011: ABS_HAT0Y
> > [val 0, min -1, max 1, fuzz 0, flat 0, res 0]
> ```
"Attempted"... I *did*. You've failed to notice that what I've set is what is reported by the HID report. When you use the event and js interfaces it is shown as above. I invite you to modify the driver as you think and see the results for yourself instead of suggesting I don't know what I'm doing here.
> quirk it only for the Ally X. If you break HHD, the chance you break
> userspace in
> general (e.g., random proton game) is very high.
I don't know what HHD is doing, but it sounds like it is not very compatible with anything anyone else does - particularly in kernel when functionality *is being added*. InputPlumber on the other hand was easily able to support this driver.
If functionality is missing for a device the proper thing to do is add that upstream to the kernel for *everyone* to benefit from, not keep it local to a userspace application where it benefits only users of that application and results in yet more duplicated effort, fragmentation, lack of or less peer review.
> I recall multiple examples of this in Linux as well, e.g., Solaar for Logitech,
> asusctl (your daemon) for Asus, libratbag for mice, and OpenRGB for
> rgb which includes
> the Ally. Can you justify the deviation here and the need for a kernel driver?
As I said, default user experience *without* requirement for userspace - the appropriate place for this is in the kernel, not in userspace. The job of the kernel is to handle hardware and expose safe interfaces for it (asusctl exists to expose a safe userspace dbus API for the functions I add to the kernel that doesn't require root. And I code for kernel first, asusctl second - the exception being RGB led because that is difficult to handle well with mcled_class when so many laptops have varying levels of function here - thus I await the outcome of a very long discussion on LKML regarding this).
Throw any plain distro on Ally with this patch and it should just work. And again InputPlumber was able to work with it without any fuss and I believe has gained a lot from it. Why can't you?
>
> I am afraid that it might be unreliable, and applying kernel patches
> is something
> very difficult that most users can not do. Furthermore, given the user chooses
> to install a userspace program to manage their controller (e.g., HHD),
> you should
> ensure this driver functionality can be disabled, from boot, so that it does not
> interfere with userspace programs.
It is so far more reliable than a python daemon and it all gets reviewed heavily before being accepted to mainstream. Most users do not apply patches, they use a distro that provides everything required.
> For example, one problem I recall that before we removed the XPAD patchset
> from the distribution Bazzite, I noticed during reviewing logs that your patch
> regularly failed the ready check on certain allies, and after trying
> the ready check
> myself as part of HHD, I had to pull it after 10 hours because users
> came out of
> the woodwork to report their controller misbehaved.
> Of course, in those cases HHD handled initialization, so your patch failing did
> not affect functionality.
I've had zero reports of this.
> ## Xbox/Xbox + A Quirk
> As mentioned above, the vendor device of the Ally reports the front manufacturer
> buttons as HID commands from the special vendor device. This allows Armoury
> crate to open in windows. Of course, in Linux there is no Armoury crate, so
> these buttons are unused by default. Userspace programs can fix this
> quirk and your previous kernel patches do as much, by exposing the buttons as
> F15, F16, F17, F18.
>
> A final component to this patchset is taking this vendor device and making it
> accessible from the gamepad driver. This is uniquely possible on the
> Ally X, because
> the gamepad happens to be HID and located on the same USB interface, so you can,
> with a bit of trickery, receive the front button commands from the
> gamepad driver.
There is no trickery. It very simply takes the event and puts it through the shared input device that was created - dead easy with a driver with global state.
> This allows you to quirk them, and here specifically for steam, to
> Xbox and Xbox+A
> ("BTN_GUIDE" and "BTN_GUIDE+BTN_SOUTH"), which have similar functionality to
> the Steamdeck's steam and three dot buttons.
>
> In my opinion, that borders a bit on being too opinionated, and perhaps e.g.,
> Asus would also share this opinion. Imagine if Asus tried to add some sort of
> official support to linux, only to find out the Linux driver is remapping the
> buttons and they have to work around that. Also, combining two HID devices is
> a bit unorthodox, and may cause other issues (see `desc.bEndpointAddress`).
> So perhaps Asus needs to be consulted here?
I talk to ASUS engineers regularly. I in fact have an agreement with them, and I'm under NDA for a lot of specifics.
> Further, I have some advice for your button implementation.
> 80ms is too short for steam and is a delay I have tried before. It
> will fail in low
> device TDPs, as Steam will begin to lag and miss the commands. For
> your reference,
> HHD uses 150ms for this and I have tried 80ms in the past.c.
It is a very different story in a kernel driver vs a python blob daemon subject to GC, userspace etc.
> In general, I am trying to move away from Xbox+A as it tends to add delay and
> since I prefer to multiplex the right vendor button for double
> presses, it starts
> to become noticable. It also does not work if the user presses the
> Nintendo layout
> button in the Steam UI.
>
> In addition, you are freezing the kernel driver to send those commands for 240ms
> which is around 100 reports.
It is done on a worker. It is not blocking the kernel....
> The first one is allowing users to enable mcu_powersave in February.
> Beforehand, it was force disabled by the kernel by your previous patch.
> This feature remains broken when the device is at low TDPs and unplugged.
> We force disable it now as part of the distribution Bazzite now.
> Applies to both Ally and Ally X.
This is not my experience on both Ally and Ally X. Please describe how you think it is broken? If you mean that the USB devices are removed and re-added - then it is up to you to account for this, just as other tools now do. The powersaving gained from enabling this for suspend can be significant and in the case of the Ally X it drops right down to a 2% use of 80Wh over 10 hours. ChimeraOS now enables it by default, and this was heavily tested.
>
> The second occasion was 3 weeks ago when you pushed the Ally X quirk
> and converted
> `ally_mcu_usb_switch` into a DMI table. It seems that a DMI table does
> not work in
> this codepath, and it is something that we had tried over a week
> before you posted the
> patch and had reverted it. If you had asked me I would have told you as much.
> After this patch got cherry picked by CachyOS and ChimeraOS, it ended
> up breaking
> both Allies, which is not a favorable user experience.
Does not work how? And what do you mean "Asked you?" I don't know you, and your work is not my target here - the default kernel is. This is what you need to be prepared to work with just like others are. Userspace needs to work with kernelspace, and hardware interaction is the purpose of the kernel - this driver exposes this functionality in a safe way *for* userspace to consume without requiring direct (raw) access to hardware.
> Moving forward, I will try to be more attentive in the kernel conversation and,
> e.g., upstream patches. And perhaps you can be a bit more thorough
> testing before you push your patches up to the kernel.
I test, mate. With the default kernel and empty userspace. This was also pulled in to ChimeraOS Unstable as part of that testing and validation - it has been quite thoroughly vetted at this point by myself and many others.
Luke.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] hid-asus-ally: Add full gamepad support
2024-08-11 9:22 ` Luke Jones
@ 2024-08-11 16:10 ` Antheas Kapenekakis
2024-08-11 20:20 ` Denis Benato
2024-08-11 23:14 ` Derek J. Clark
0 siblings, 2 replies; 13+ messages in thread
From: Antheas Kapenekakis @ 2024-08-11 16:10 UTC (permalink / raw)
To: Luke Jones; +Cc: Benjamin Tissoires, Jiri Kosina, linux-input, linux-kernel
Hi Luke,
thank you for taking the time to reply.
And everyone else, thank you for putting up with my broken line spacing,
because of Gmail. And perhaps the lack of in-reply-to because of mailto.
Hopefully in-reply-to works this time ;).
First off, understand that I do not mean to attack your work. I tried to
make my response helpful to you. In fact, I took a lot of time in writing it
and preemptively saved you a lot of work in testing your patchset so you do
not have to spend time reaching the same conclusions that I have had to.
Provided you heed my comments of course, which is not clear from your reply.
As I currently represent what is the largest Linux ROG Ally community, I hope
it is reasonable that it is in my and my community's best interest that an
unstable patch which could affect Linux Ally support should be vetted before
it becomes part of the mainline kernel. Once it becomes part of the kernel,
workarounds around it will become very hard. We have achieved near perfect
controller support through userspace, so I would hate to jeopardize that.
In my previous email, I gave you my preliminary thoughts without having time
to test your patchset. Of course, as I noted in that email, I will be testing
and integrating support for your patchset, personally, with an Ally X unit
I will be getting access to close to the end of August.
I do not think there is value in arguing, therefore I am not happy continuing
the discussion under this tone. Hopefully, in a few days you, will have
another look at my comments, with a level head, and address them.
I still believe that they are valid and that if they are fixed, I am more
than happy with your patchset merging into the kernel.
Since you raised some technical points in your response, let me disambiguate.
> You're repeating information that has come directly from me.
Indeed, this specific point (XInput being deprecated) came from you.
I am just bringing everyone up to speed, since I feel your patch missed
some important context.
> I have many records from many MCU updates. It doesn't happen. (referring to
> relying to the endpoint descriptor instead of HID Usages)
In my opinion, using the standard Usage Page and Usage the controller reports
remains the proper solution (this is what the Windows driver does).
Remember that if there is a breakage due to a firmware update, users will
become unable to use their device as they can not update the kernel.
Using `desc.bEndpointAddress` may be appropriate for a userspace tool or an
out-of-tree kernel driver, but perhaps not for the mainline kernel.
I am happy to be proven wrong.
> "Attempted"... I *did*. You've failed to notice that what I've set is
> what is reported by the HID report.
I am a bit confused here. I thought the purpose of your patch was to convert
the HID report into what xpad would export. That means respecting xpad, not
a random HID report.
In this case, the absinfo (with `input_set_abs_params`) needs to be set
according to what is set by xpad, which is signed and from -32768 to 32767
(referencing both the Linux Gamepad Specification and the out-of-tree driver
xpadneo which seems to be the prominent driver providing support for
controllers similar to Ally X, i.e., Xbox One bluetooth controllers).
I know from experience that Handheld Daemon will have a problem with this.
But alas, I was not referring to Handheld Daemon being the problem here:
it is simple enough to fix it in there and I will do it when I add
support for your patchset (would rather avoid doing it or doing it and having
to revert it of course). I was moreso referring to other userspace
applications without this privilege.
As for why I have to add support for your patchset, it is simply because
it being there changes the controller mappings, so I simply need to add
an if statement that uses the standard XInput mappings when it is available.
This is not to say that the end result will be as reliable as without your
driver, as Handheld Daemon will then be at the mercy of your kernel driver.
So please, do extended testing and I think with e.g., ChimeraOS 46.2 being
released yesterday with your patchset, you will get some valuable
feedback soon enough.
I know that I have spent multiple weeks already optimizing my implementation,
having released it close to a month ago. Which is also why I am not in a
rush to add support for your experimental patchset.
> It is a very different story in a kernel driver... (referring to the 80ms
> delay used for Xbox+A)
Please understand that I have spent weeks of effort debugging and optimizing
the Side Menu behavior of Handheld Daemon. In fact, it currently implements
three different ways of opening the Steam Side Menu (keyboard, extest through
the gamescope X11 socket, and as a last fallback as Xbox + A). When I say
80ms is not enough, I know it is not enough. Otherwise Handheld Daemon would
be using 80ms. The rest is conjecture.
> It is done on a worker. It is not blocking the kernel....
> (referring to xbox+a holding a spinlock to send the key combo with msleep)
Repeating myself:
> > In addition, you are freezing the kernel **driver** to send those
> > commands for 240ms which is around 100 reports.
Which in my opinion will become more like 300ms. Freezing the controller for
that long is not ideal (I know you are not freezing the kernel).
Please revisit this.
> Please describe how you think it is broken? (referring to `mcu_powersave`)
Quoting myself:
> > This feature remains broken when the device is at low TDPs and unplugged.
e.g., when the Ally is set on its quiet mode and or is below 12W, and is
suspended unplugged, with Steam and a game running. In this case,
the USB controller of the Ally simply does not wake up and RGB breaks.
The occurrence of this given those conditions is around 40% of the time.
This includes testing with or without your DMI table patch by the way.
> Does not work how? (referring to `ally_mcu_usb_switch`)
Seems like a DMI table always sets it to 0. I do not know why. However I do
know that as part of our validation on the distribution Bazzite which took
place prior to you submitting your patch, we tested both adding an or for
ally x and a dmi table, and the dmi table did not work. Post submitting your
patch, there was a 5 day brief period where both the unstable ChimeraOS
kernel and the CachyOs kernel integrated your patch before they also
integrated the patch I am replying to (which makes Handheld Daemon not work;
for now). During this period both the original ally and the ally x regressed
when `mcu_powersave` was disabled.
> I test, mate. With the default kernel and empty userspace.
Unfortunately, this is not how users interact with my kernel patches and
Handheld Daemon. They usually play games with SteamUI running in the
background. Often, this includes setting an aggressively low TDP, and multiple
suspends back-to-back while in game. This is the standard I hold myself to.
And I would expect no less from a mainline kernel driver.
I hope I replied to all your technical claims. If I missed any, I am
happy to clarify and expand where appropriate.
Again, good luck with your patchset. Hopefully, it will merge with 6.12 and
Ally X owners will get an acceptable result without the need for userspace
tools (albeit without gyro, back buttons, and RGB being part of the
controller).
Best regards,
Antheas
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] hid-asus-ally: Add full gamepad support
2024-08-11 16:10 ` Antheas Kapenekakis
@ 2024-08-11 20:20 ` Denis Benato
2024-08-11 23:14 ` Derek J. Clark
1 sibling, 0 replies; 13+ messages in thread
From: Denis Benato @ 2024-08-11 20:20 UTC (permalink / raw)
To: Antheas Kapenekakis, Luke Jones
Cc: Benjamin Tissoires, Jiri Kosina, linux-input, linux-kernel
On 11/08/24 18:10, Antheas Kapenekakis wrote:
> Hi Luke,
> thank you for taking the time to reply.
>
> And everyone else, thank you for putting up with my broken line spacing,
> because of Gmail. And perhaps the lack of in-reply-to because of mailto.
> Hopefully in-reply-to works this time ;).
>> First off, understand that I do not mean to attack your work. I tried to
> make my response helpful to you. In fact, I took a lot of time in writing it
> and preemptively saved you a lot of work in testing your patchset so you do
> not have to spend time reaching the same conclusions that I have had to.
>> Provided you heed my comments of course, which is not clear from your reply.
>
> As I currently represent what is the largest Linux ROG Ally community, I hope
> it is reasonable that it is in my and my community's best interest that an
> unstable patch which could affect Linux Ally support should be vetted before
> it becomes part of the mainline kernel. Once it becomes part of the kernel,
> workarounds around it will become very hard. We have achieved near perfect
> controller support through userspace, so I would hate to jeopardize that.
>
Hello,
I'm Denis and I want to say a few things:
first of all I have been following the ROG Ally scene from quite a while now and
I submitted one of the oldest patches regarding it specifically:
https://lore.kernel.org/all/20231117011556.13067-1-luke@ljones.dev/
and in no way I feel represented by you.
We have talked about this matter many times in an open-to-everybody discord server,
and all the records are still available for whoever wants to read those in the chimeraos
server.
We all have warned you about taking control of devices from userspace, many and
many times again. More than nine months ago, you decided to ignore us all and
write an application you knew it would have played bad with future a driver that would
have been submitted upstream.
To be clear to readers your application started as a clone of my own application, meant
as a temporary workaround for a support that we all knew was coming:
mine: https://github.com/NeroReflex/ROGueENEMY (first commit: 2 Nov 2023)
yours: https://github.com/hhd-dev/hhd (first commit: 30 Nov 2023)
plus yours was targeting lenovo hardware specifically and added ROG support
after some time... Both have been developed in the same discord server, there
is absolutely no way you could have missed those messages.
The first tool supporting the legion go console was a port of my own tool by another
user (that I helped) in our server and we all knew it was a very suboptimal (and temporary)
way of arriving at the final goal of having these handhelds devices in a reliable
ready-to-game state, including you.
> In my previous email, I gave you my preliminary thoughts without having time
> to test your patchset. Of course, as I noted in that email, I will be testing
> and integrating support for your patchset, personally, with an Ally X unit
> I will be getting access to close to the end of August.
>
> I do not think there is value in arguing, therefore I am not happy continuing
> the discussion under this tone. Hopefully, in a few days you, will have
> another look at my comments, with a level head, and address them.
> I still believe that they are valid and that if they are fixed, I am more
> than happy with your patchset merging into the kernel.
>
> Since you raised some technical points in your response, let me disambiguate.
>
>> You're repeating information that has come directly from me.
>
> Indeed, this specific point (XInput being deprecated) came from you.
> I am just bringing everyone up to speed, since I feel your patch missed
> some important context.
>
>> I have many records from many MCU updates. It doesn't happen. (referring to
>> relying to the endpoint descriptor instead of HID Usages)
>
> In my opinion, using the standard Usage Page and Usage the controller reports
> remains the proper solution (this is what the Windows driver does).
> Remember that if there is a breakage due to a firmware update, users will
> become unable to use their device as they can not update the kernel.
>
> Using `desc.bEndpointAddress` may be appropriate for a userspace tool or an
> out-of-tree kernel driver, but perhaps not for the mainline kernel.
> I am happy to be proven wrong.
>
>> "Attempted"... I *did*. You've failed to notice that what I've set is
>> what is reported by the HID report.
>
> I am a bit confused here. I thought the purpose of your patch was to convert
> the HID report into what xpad would export. That means respecting xpad, not
> a random HID report.
>
> In this case, the absinfo (with `input_set_abs_params`) needs to be set
> according to what is set by xpad, which is signed and from -32768 to 32767
> (referencing both the Linux Gamepad Specification and the out-of-tree driver
> xpadneo which seems to be the prominent driver providing support for
> controllers similar to Ally X, i.e., Xbox One bluetooth controllers).
>
> I know from experience that Handheld Daemon will have a problem with this.
> But alas, I was not referring to Handheld Daemon being the problem here:
> it is simple enough to fix it in there and I will do it when I add
> support for your patchset (would rather avoid doing it or doing it and having
> to revert it of course). I was moreso referring to other userspace
> applications without this privilege.
>
> As for why I have to add support for your patchset, it is simply because
> it being there changes the controller mappings, so I simply need to add
> an if statement that uses the standard XInput mappings when it is available.
>
> This is not to say that the end result will be as reliable as without your
> driver, as Handheld Daemon will then be at the mercy of your kernel driver.
> So please, do extended testing and I think with e.g., ChimeraOS 46.2 being
> released yesterday with your patchset, you will get some valuable
> feedback soon enough.
>
To be fair, the submitted work has been ongoing for quite some time now,
and it has been tested on both ally and ally x by at least five developers,
including me who tested even more scenarios than input, especially regarding s2idle.
Moreover users of other distributions tested the driver too.
Our distribution version was released as stable when we felt it was completed in all features
and we have finished testing what we could have possibly taught of (this happens to be yesterday),
but anyway I don't believe the release date of a distribution is a valid point of argument for a kernel driver.
If there are bugs please file an actual bug report them so we can fix them.
> I know that I have spent multiple weeks already optimizing my implementation,
> having released it close to a month ago. Which is also why I am not in a
> rush to add support for your experimental patchset.
>
Again, you were totally aware of what you were doing and what was going on,
so this is not "unfortunate accident".
>> It is a very different story in a kernel driver... (referring to the 80ms
>> delay used for Xbox+A)
>
> Please understand that I have spent weeks of effort debugging and optimizing
> the Side Menu behavior of Handheld Daemon. In fact, it currently implements
> three different ways of opening the Steam Side Menu (keyboard, extest through
> the gamescope X11 socket, and as a last fallback as Xbox + A). When I say
> 80ms is not enough, I know it is not enough. Otherwise Handheld Daemon would
> be using 80ms. The rest is conjecture.
>
I am not understanding the point here. If your userspace tool was working before all of these
additions why can't you simply ship a blacklist file for the new driver?
If you are relying in some initialization code done from the driver
I offer myself in adding a kernel argument to keep only that initialization code.
>> It is done on a worker. It is not blocking the kernel....
>> (referring to xbox+a holding a spinlock to send the key combo with msleep)
>
> Repeating myself:
>
>>> In addition, you are freezing the kernel **driver** to send those
>>> commands for 240ms which is around 100 reports.
>
> Which in my opinion will become more like 300ms. Freezing the controller for
> that long is not ideal (I know you are not freezing the kernel).
> Please revisit this.
>
>> Please describe how you think it is broken? (referring to `mcu_powersave`)
>
> Quoting myself:
>
>>> This feature remains broken when the device is at low TDPs and unplugged.
>
> e.g., when the Ally is set on its quiet mode and or is below 12W, and is
> suspended unplugged, with Steam and a game running. In this case,
> the USB controller of the Ally simply does not wake up and RGB breaks.
> The occurrence of this given those conditions is around 40% of the time.
> This includes testing with or without your DMI table patch by the way.
>
>> Does not work how? (referring to `ally_mcu_usb_switch`)
>
> Seems like a DMI table always sets it to 0. I do not know why. However I do
> know that as part of our validation on the distribution Bazzite which took
> place prior to you submitting your patch, we tested both adding an or for
> ally x and a dmi table, and the dmi table did not work. Post submitting your
> patch, there was a 5 day brief period where both the unstable ChimeraOS
> kernel and the CachyOs kernel integrated your patch before they also
> integrated the patch I am replying to (which makes Handheld Daemon not work;
> for now). During this period both the original ally and the ally x regressed
> when `mcu_powersave` was disabled.
>
>> I test, mate. With the default kernel and empty userspace.
MCU powersave is not even part of this driver. It directly affects the device to be
clear, but nobody can do anything about it: it is how hardware has been designed.
If you need said feature to be disabled you can use an udev rule, that will indeed
make the device consume more while in s2idle state and is therefore suboptimal,
otherwise please brings this up where it is relevant.
Again all of this is knowledge that has been around for about a year, and none of
this comes at a surprise to you.
>
> Unfortunately, this is not how users interact with my kernel patches and
> Handheld Daemon. They usually play games with SteamUI running in the
> background. Often, this includes setting an aggressively low TDP, and multiple
> suspends back-to-back while in game. This is the standard I hold myself to.
> And I would expect no less from a mainline kernel driver.
>
The vast majority of users are using a distribution meant for handheld hardware
because tradition distributions requires having a mouse and keyboard around,
however we have records in our server of people asking how to have their controllers
working in archlinux, so what you say is not an absolute truth and there are people who
actually wants being able to use theirs hardware without any of our userspace tooling:
"ours" means all of them, my unmaintained proof-of-concept project, your hdd and inputplumber.
Plus you are also talking about TDP here, and this is a totally separate topic from my understanding.
> I hope I replied to all your technical claims. If I missed any, I am
> happy to clarify and expand where appropriate.
>
> Again, good luck with your patchset. Hopefully, it will merge with 6.12 and
> Ally X owners will get an acceptable result without the need for userspace
> tools (albeit without gyro, back buttons, and RGB being part of the
> controller).
Gyroscope is on a different subsystem on the kernel and there is no way of having it part of
the input device without userspace drivers. This is true for every OS, including windows.
Back buttons are mappable and can be made part of the controller, and this is even documented:
it comes with some caveats, but if an user wants to have X and Y buttons mapped on back paddles
it will now be possible.
RGB support is once again another example of a peripheral that is impossible to be made
part of an input device, so I am not getting the point:
the fact that you control leds via the emulated controller because some other user
space drivers for that specific emulated device can does mean nothing in this context
and to be fair since you are sending raw hidraw commands to that device it means you are
already driving the current driver in an inconsistent state (reported state vs hardware state)
and therefore blacklisting it is what you should have done months ago, when we told you were
already conflicting with what was upstreamed already, before your work.
> Best regards,
> Antheas
Best regards,
Denis
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: Re: [PATCH] hid-asus-ally: Add full gamepad support
2024-08-11 16:10 ` Antheas Kapenekakis
2024-08-11 20:20 ` Denis Benato
@ 2024-08-11 23:14 ` Derek J. Clark
2024-08-12 8:31 ` Antheas Kapenekakis
1 sibling, 1 reply; 13+ messages in thread
From: Derek J. Clark @ 2024-08-11 23:14 UTC (permalink / raw)
To: lkml; +Cc: luke, bentiss, jikos, linux-input, linux-kernel, Derek J . Clark
Antheas,
Having been one of the many testers of this patch set, having been the one
who added support for it to InputPlumber, and being in the position of
actually having a device in hand to validate the patches in ChimeraOS, I am
fully qualified to address your "concerns".
> Hi Luke,
> thank you for taking the time to reply.
>
> And everyone else, thank you for putting up with my broken line spacing,
> because of Gmail. And perhaps the lack of in-reply-to because of mailto.
> Hopefully in-reply-to works this time ;).
>
> First off, understand that I do not mean to attack your work. I tried to
> make my response helpful to you. In fact, I took a lot of time in writing it
> and preemptively saved you a lot of work in testing your patchset so you do
> not have to spend time reaching the same conclusions that I have had to.
>
> Provided you heed my comments of course, which is not clear from your reply.
>
> As I currently represent what is the largest Linux ROG Ally community, I hope
> it is reasonable that it is in my and my community's best interest that an
> unstable patch which could affect Linux Ally support should be vetted before
> it becomes part of the mainline kernel. Once it becomes part of the kernel,
> workarounds around it will become very hard. We have achieved near perfect
> controller support through userspace, so I would hate to jeopardize that.
I think you might have a misunderstanding of the purpose of kernel space and
user space. Directly implementing many of the features that you have into HHD
as a user level systemd service violates the separation of the kernel and user
protection rings put in place by the kernel architecture. It is not incumbent on
kernel maintainers to respect that just because you have a lot of users,
especially considering that you have created this problem for yourself.
> In my previous email, I gave you my preliminary thoughts without having time
> to test your patchset. Of course, as I noted in that email, I will be testing
> and integrating support for your patchset, personally, with an Ally X unit
> I will be getting access to close to the end of August.
>
> I do not think there is value in arguing, therefore I am not happy continuing
> the discussion under this tone. Hopefully, in a few days you, will have
> another look at my comments, with a level head, and address them.
> I still believe that they are valid and that if they are fixed, I am more
> than happy with your patchset merging into the kernel.
>
> Since you raised some technical points in your response, let me disambiguate.
>
> > You're repeating information that has come directly from me.
>
> Indeed, this specific point (XInput being deprecated) came from you.
> I am just bringing everyone up to speed, since I feel your patch missed
> some important context.
>
> > I have many records from many MCU updates. It doesn't happen. (referring to
> > relying to the endpoint descriptor instead of HID Usages)
>
> In my opinion, using the standard Usage Page and Usage the controller reports
> remains the proper solution (this is what the Windows driver does).
> Remember that if there is a breakage due to a firmware update, users will
> become unable to use their device as they can not update the kernel.
>
> Using `desc.bEndpointAddress` may be appropriate for a userspace tool or an
> out-of-tree kernel driver, but perhaps not for the mainline kernel.
> I am happy to be proven wrong.
Multiple kernel drivers use desc.bEndpointAddress, with subsystems ranging from
i2c, isdn, and multiple usb drivers. If you want some concrete examples of HID
drivers, look at hid_thrustmaster or usbtouchscreen.
> > "Attempted"... I *did*. You've failed to notice that what I've set is
> > what is reported by the HID report.
>
> I am a bit confused here. I thought the purpose of your patch was to convert
> the HID report into what xpad would export. That means respecting xpad, not
> a random HID report.
>
> In this case, the absinfo (with `input_set_abs_params`) needs to be set
> according to what is set by xpad, which is signed and from -32768 to 32767
> (referencing both the Linux Gamepad Specification and the out-of-tree driver
> xpadneo which seems to be the prominent driver providing support for
> controllers similar to Ally X, i.e., Xbox One bluetooth controllers).
>
> I know from experience that Handheld Daemon will have a problem with this.
> But alas, I was not referring to Handheld Daemon being the problem here:
> it is simple enough to fix it in there and I will do it when I add
> support for your patchset (would rather avoid doing it or doing it and having
> to revert it of course). I was moreso referring to other userspace
> applications without this privilege.
Steam has no issues detecting a normal i16 axis range from the u16 provided by
this driver. It isn't an issue with InputPlumber either. This seems to be an
issue you with you not properly detecting and normalizing axes for translation
in your userspace implementation.
> As for why I have to add support for your patchset, it is simply because
> it being there changes the controller mappings, so I simply need to add
> an if statement that uses the standard XInput mappings when it is available.
>
> This is not to say that the end result will be as reliable as without your
> driver, as Handheld Daemon will then be at the mercy of your kernel driver.
> So please, do extended testing and I think with e.g., ChimeraOS 46.2 being
> released yesterday with your patchset, you will get some valuable
> feedback soon enough.
> I know that I have spent multiple weeks already optimizing my implementation,
> having released it close to a month ago. Which is also why I am not in a
> rush to add support for your experimental patchset.
I have dozens of hours of testing done with these patches on both the Ally X
and the original hardware. There aren't any issues that I have found that fall
under the scope of this patch set. Calling it "experimental" is expressly
unfounded, especially so considering you yourself admit you haven't actually
tested it.
> > It is a very different story in a kernel driver... (referring to the 80ms
> > delay used for Xbox+A)
>
> Please understand that I have spent weeks of effort debugging and optimizing
> the Side Menu behavior of Handheld Daemon. In fact, it currently implements
> three different ways of opening the Steam Side Menu (keyboard, extest through
> the gamescope X11 socket, and as a last fallback as Xbox + A). When I say
> 80ms is not enough, I know it is not enough. Otherwise Handheld Daemon would
> be using 80ms. The rest is conjecture.
This is 100% an issue with your software. I just completed an exhaustive battery
of testing at 8w STAPM/FPPT/SPPT on Quiet power profile with only 2 cores
active. The tests included using Steam by itself and the kernel implementation,
as well as running InputPlumber (which also has an 80ms delay) and an
OpenGamepadUI Overlay. In both the kernel implementation and the InputPlumber
implementation there was no difference between the low power mode and running
without restriction on either the Ally or Ally X.
> > It is done on a worker. It is not blocking the kernel....
> > (referring to xbox+a holding a spinlock to send the key combo with msleep)
>
> Repeating myself:
>
> > > In addition, you are freezing the kernel **driver** to send those
> > > commands for 240ms which is around 100 reports.
>
> Which in my opinion will become more like 300ms. Freezing the controller for
> that long is not ideal (I know you are not freezing the kernel).
> Please revisit this.
>
> > Please describe how you think it is broken? (referring to `mcu_powersave`)
>
> Quoting myself:
>
> > > This feature remains broken when the device is at low TDPs and unplugged.
>
> e.g., when the Ally is set on its quiet mode and or is below 12W, and is
> suspended unplugged, with Steam and a game running. In this case,
> the USB controller of the Ally simply does not wake up and RGB breaks.
> The occurrence of this given those conditions is around 40% of the time.
> This includes testing with or without your DMI table patch by the way.
Your RGB interface in HHD is another good example of bypassing the kernel
with userspace. Use the mc_led interface provided by this patch set. It works
the same as the out of tree Ayn and Ayaneo drivers I wrote that you're already
using.
> > Does not work how? (referring to `ally_mcu_usb_switch`)
> Seems like a DMI table always sets it to 0. I do not know why. However I do
> know that as part of our validation on the distribution Bazzite which took
> place prior to you submitting your patch, we tested both adding an or for
> ally x and a dmi table, and the dmi table did not work. Post submitting your
> patch, there was a 5 day brief period where both the unstable ChimeraOS
> kernel and the CachyOs kernel integrated your patch before they also
> integrated the patch I am replying to (which makes Handheld Daemon not work;
> for now). During this period both the original ally and the ally x regressed
> when `mcu_powersave` was disabled.
File a bug report. mcu_powersave is not relevant to this patch series. The LKML
is not a forum to voice grievances over any tangentially related issues you find.
> > I test, mate. With the default kernel and empty userspace.
>
> Unfortunately, this is not how users interact with my kernel patches and
> Handheld Daemon. They usually play games with SteamUI running in the
> background. Often, this includes setting an aggressively low TDP, and multiple
> suspends back-to-back while in game. This is the standard I hold myself to.
> And I would expect no less from a mainline kernel driver.
> I hope I replied to all your technical claims. If I missed any, I am
> happy to clarify and expand where appropriate.
>
> Again, good luck with your patchset. Hopefully, it will merge with 6.12 and
> Ally X owners will get an acceptable result without the need for userspace
> tools (albeit without gyro, back buttons, and RGB being part of the
> controller).
> Best regards,
> Antheas
Preeminent regards,
Derek J. Clark
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: Re: [PATCH] hid-asus-ally: Add full gamepad support
2024-08-11 23:14 ` Derek J. Clark
@ 2024-08-12 8:31 ` Antheas Kapenekakis
2024-08-12 12:49 ` Denis Benato
2024-08-12 20:12 ` Re: " Derek J. Clark
0 siblings, 2 replies; 13+ messages in thread
From: Antheas Kapenekakis @ 2024-08-12 8:31 UTC (permalink / raw)
To: Derek J. Clark
Cc: luke, bentiss, jikos, linux-input, linux-kernel, benato.denis96
Hi Derek and Denis,
Let us be civil. If I could have bug reported you I would have bug reported
you. However, for some weird coincidence, I do not have access to the
ShadowBlip bug tracker or the relevant communities. Nevertheless, this is
not relevant public discussion. Let us refrain from this discussion further,
including e.g., name-calling.
The MCU of the Ally is the embedded microcontroller that runs the RGB and the
controller of the Ally. Therefore, the discussion of the MCU powersave and
wake is relevant here. What is not proper is that I should also have replied
to the original patch. I admitted that much in my original email. However,
since you are now aware of it, I trust that you can block the patch for
merging until you review it.
If the patch does not function under normal operation, this is relevant here.
It means this extended patchset is built on reliance of the broken patch,
which raises questions. Nevertheless, calling the patchset "experimental"
is hearsay. Therefore, I will refer to it as ambitious from now on :).
> This is 100% an issue with your software. I just completed an exhaustive
> battery of testing at 8w STAPM/FPPT/SPPT on Quiet power profile with only
> 2 cores active. The tests included using Steam by itself and the kernel
> implementation, as well as running InputPlumber (which also has an
> 80ms delay).
Please refrain from name-calling. I was very specific to say that the issue
here is that under load when in a game, Steam will either let A leak through
to the game or not respond, sporadically. While in Steam UI the combination
always works, regardless of TDP. Since you did not test while in a game,
this renders your test invalid.
To save you some additional invalid testing for the other issues: using
ryzenadj on the Ally causes misbehavior, especially after suspend, where the
TDP will reset. In addition, modifying SMP and core parking can freeze the
Ally during suspend. Therefore, for further testing, I expect you to disable
your "PowerStation" application and instead use the low-power platform
profile, which is provided by asus-wmi, and is the vendor specific way for
setting TDP on the Ally. Or use asusctl, which does the same.
As for using direct HID commands to program RGB, asusctl does the same,
including many other userspace apps, and prior to this patchset there was
no way to do different, so I do not see the problem here.
Best,
Antheas
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] hid-asus-ally: Add full gamepad support
2024-08-12 8:31 ` Antheas Kapenekakis
@ 2024-08-12 12:49 ` Denis Benato
2024-08-12 20:12 ` Re: " Derek J. Clark
1 sibling, 0 replies; 13+ messages in thread
From: Denis Benato @ 2024-08-12 12:49 UTC (permalink / raw)
To: Antheas Kapenekakis, Derek J. Clark
Cc: luke, bentiss, jikos, linux-input, linux-kernel
On 12/08/24 10:31, Antheas Kapenekakis wrote:
> Hi Derek and Denis,
>
> Let us be civil. If I could have bug reported you I would have bug reported
> you. However, for some weird coincidence, I do not have access to the
> ShadowBlip bug tracker or the relevant communities. Nevertheless, this is
> not relevant public discussion. Let us refrain from this discussion further,
> including e.g., name-calling.
Hello,
First and foremost we have been civil, we vigorously stated the truth and
enriched this discussions with information and context you initially left out
(for reasons that at this point are very clear to everyone that has been reading)
and I can assure that we will remain civil as we are not going to make fools
out of ourselves.
We are discussing kernel drivers here, those userspace software are none of your
concerns as those are none of kernel maintainers concerns either.
>
> The MCU of the Ally is the embedded microcontroller that runs the RGB and the
> controller of the Ally. Therefore, the discussion of the MCU powersave and
> wake is relevant here. What is not proper is that I should also have replied
> to the original patch. I admitted that much in my original email. However,
> since you are now aware of it, I trust that you can block the patch for
> merging until you review it.
>
The MCU powersave is a feature driven by the APCI interface and here we are discussing
an USB driver: there is absolutely no reasons to block this patch for a problem you
claim you have found (but never reported) in a totally different driver that even lives
in a totally different subsystem, so if you want those issue solved file the proper bug
report where it is relevant (not here).
> If the patch does not function under normal operation, this is relevant here.
> It means this extended patchset is built on reliance of the broken patch,
> which raises questions. Nevertheless, calling the patchset "experimental"
> is hearsay. Therefore, I will refer to it as ambitious from now on :).
>
The patch does work under normal operation and it has been tested by many as stated already.
The device firmware performs the wake-up of the device and I don't
see any problem assuming the hardware works to submit a driver:
in fact this assumption is made by every kernel driver.
Again, you claim, without providing evidence, that the hardware is not properly waking up
when a certain ACPI attribute is set to one. Just set it to zero. Over these nine months
you had absolutely no problems is driving the hardware via acpi_call driver and tools
making SMU calls.
If you know a way that I can trigger this problem within my hardware
please, use the relevant mailing list reporting a bug so we can solve
it as we have solved other problems before.
Do not obstruct a fully working kernel driver for reasons unrelated to it.
>> This is 100% an issue with your software. I just completed an exhaustive
>> battery of testing at 8w STAPM/FPPT/SPPT on Quiet power profile with only
>> 2 cores active. The tests included using Steam by itself and the kernel
>> implementation, as well as running InputPlumber (which also has an
>> 80ms delay).
> > Please refrain from name-calling. I was very specific to say that the issue
> here is that under load when in a game, Steam will either let A leak through
> to the game or not respond, sporadically. While in Steam UI the combination
> always works, regardless of TDP. Since you did not test while in a game,
> this renders your test invalid.
>
I have opened the overlay in game with no problems.
Why should we refrain from name-calling? You started this discussion with the
name of your own software, and in the kernel no anonymous contributions are
allowed either.
Plus, if steam (another userspace software) has a problem with combo keys
recognition and timing we are not entitled to solve that in a kernel driver anyway.
> To save you some additional invalid testing for the other issues: using
> ryzenadj on the Ally causes misbehavior, especially after suspend, where the
> TDP will reset. In addition, modifying SMP and core parking can freeze the
> Ally during suspend. Therefore, for further testing, I expect you to disable
> your "PowerStation" application and instead use the low-power platform
> profile, which is provided by asus-wmi, and is the vendor specific way for
> setting TDP on the Ally. Or use asusctl, which does the same.
>
All of these information are given to you directly by us nine months ago,
while you were incorrectly driving the hardware via ryzenadj doing exactly
what we told you not to do, so you can expect we know this already.
Here you also make assumptions on how more than four people made their tests,
and while I don't speak for others I can ensure mine were done using the
firmware interface ASUS put in place and is exposed via asus-wmi platform driver:
the driver also in charge of handling the MCU powersave feature you are using as
an unreasonable point of arguments here.
In fact I have been telling you to use that interface for (ance again) more than nine months now,
and you only recently did so in your software because problems we told you were
going to manifest finally manifested, and you have been left with no other choice
but to use the proper way of driving the hardware in question.
> As for using direct HID commands to program RGB, asusctl does the same,
> including many other userspace apps, and prior to this patchset there was
> no way to do different, so I do not see the problem here.
>
There is no problem here: your software is driving the reported state inconsistent
with the hardware state and that's a you problem.
Our userspace tooling have been using what the kernel has been exposing, especially
Asusctl that is software written by the same person that contributes to said drivers
and is very well informed about other lkml discussion he partecipates.
> Best,
> Antheas
Best regards,
Denis
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: Re: Re: [PATCH] hid-asus-ally: Add full gamepad support
2024-08-12 8:31 ` Antheas Kapenekakis
2024-08-12 12:49 ` Denis Benato
@ 2024-08-12 20:12 ` Derek J. Clark
1 sibling, 0 replies; 13+ messages in thread
From: Derek J. Clark @ 2024-08-12 20:12 UTC (permalink / raw)
To: lkml; +Cc: luke, bentiss, jikos, linux-input, linux-kernel, Derek J . Clark
> Hi Derek and Denis,
>
> Let us be civil. If I could have bug reported you I would have bug reported
> you. However, for some weird coincidence, I do not have access to the
> ShadowBlip bug tracker or the relevant communities. Nevertheless, this is
> not relevant public discussion. Let us refrain from this discussion further,
> including e.g., name-calling.
I haven't called you any names or levied any personal attacks, so I will
continue not to. I'll also help you, here is the resource for reporting kernel
bugs. You will find the ShadowBlip GitHub is notably absent from it, so I'm not
sure why you brought that up.
https://www.kernel.org/doc/html/v4.19/admin-guide/reporting-bugs.html
> The MCU of the Ally is the embedded microcontroller that runs the RGB and the
> controller of the Ally. Therefore, the discussion of the MCU powersave and
> wake is relevant here. What is not proper is that I should also have replied
> to the original patch. I admitted that much in my original email. However,
> since you are now aware of it, I trust that you can block the patch for
> merging until you review it.
>
> If the patch does not function under normal operation, this is relevant here.
> It means this extended patchset is built on reliance of the broken patch,
> which raises questions. Nevertheless, calling the patchset "experimental"
> is hearsay. Therefore, I will refer to it as ambitious from now on :).
It is not relevant to the functionality provided by this patch, and the bug
exists with or without this patch. The bug cannot be resolved with this patch,
and it is not exacerbated by this patch. Therefore it follows that there is
no reason to block this patch because of the mcu_powersave issue. See above
for how to report the issue properly.
> > This is 100% an issue with your software. I just completed an exhaustive
> > battery of testing at 8w STAPM/FPPT/SPPT on Quiet power profile with only
> > 2 cores active. The tests included using Steam by itself and the kernel
> > implementation, as well as running InputPlumber (which also has an
> > 80ms delay).
>
> Please refrain from name-calling. I was very specific to say that the issue
> here is that under load when in a game, Steam will either let A leak through
> to the game or not respond, sporadically. While in Steam UI the combination
> always works, regardless of TDP. Since you did not test while in a game,
> this renders your test invalid.
>
> To save you some additional invalid testing for the other issues: using
> ryzenadj on the Ally causes misbehavior, especially after suspend, where the
> TDP will reset. In addition, modifying SMP and core parking can freeze the
> Ally during suspend. Therefore, for further testing, I expect you to disable
> your "PowerStation" application and instead use the low-power platform
> profile, which is provided by asus-wmi, and is the vendor specific way for
> setting TDP on the Ally. Or use asusctl, which does the same.
I should have been more clear about my testing as you have made a lot of
assumptions about how they were conducted. All of my tests were done multiple
times in different games. I chose not to list them all for brevity in the LKML.
PowerStation was disabled for all of tests and I used the sysfs devnodes at
/sys/class/firmware-attributes/asus-armoury/attributes/ to set STAPM/FPPT/SPPT
and asusctl to set the "quiet" power profile. ryzenadj -i was used only to
verify current setting of each PPT before and after sleep to ensure it did not
reset. Also, I only disabled cores to provide a “worst-case” scenario to ensure
that the test was as comprehensive as possible. These tests meet all of the
criteria you have specified so thank you for providing a second validation of my
methodology.
I will point out that the issue of Steam passing events through to a game can
occur when a user has their settings too low to run the game at a reasonable
framerate. In such cases it is recommended they lower their settings or adjust
their power profile to increase performance. Kernel drivers do not need to
account for misbehavior in userspace, especially when those issues are
driven by misconfiguration. In any case, this is a bug in Steam so you should
report it to Valve directly using their SteamForLinux GitHub, rather than on
unrelated kernel patches.
Furthermore, we both know you won't be using the default button combination
and will instead use the included configuration attributes to set the buttons
to the F14-F16 keys you already support so you can determine the discrete
events, just as InputPlumber is doing. This patch will not affect your ability
to use your multiple fallback methods and only provides a usable default
without the use of any userspace tools. Within the scope of the patch and its
purpose there is no issue here as it functions as intended.
> As for using direct HID commands to program RGB, asusctl does the same,
> including many other userspace apps, and prior to this patchset there was
> no way to do different, so I do not see the problem here.
>
> Best,
> Antheas
R/
Derek J. Clark
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2024-08-12 20:14 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-08-08 15:35 [PATCH] hid-asus-ally: Add full gamepad support Antheas Kapenekakis
2024-08-11 9:22 ` Luke Jones
2024-08-11 16:10 ` Antheas Kapenekakis
2024-08-11 20:20 ` Denis Benato
2024-08-11 23:14 ` Derek J. Clark
2024-08-12 8:31 ` Antheas Kapenekakis
2024-08-12 12:49 ` Denis Benato
2024-08-12 20:12 ` Re: " Derek J. Clark
-- strict thread matches above, loose matches on Subject: below --
2024-08-06 8:12 Luke D. Jones
2024-08-06 21:34 ` kernel test robot
2024-08-06 22:20 ` Luke Jones
2024-08-07 9:50 ` kernel test robot
2024-08-07 11:43 ` kernel test robot
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).