* Re: [PATCH] Input: uinput - fix circular locking dependency with ff-core
From: Mikhail Gavrilov @ 2026-04-07 8:39 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input, linux-kernel
In-Reply-To: <adSSoVZBLs8b6I0J@google.com>
On Tue, Apr 7, 2026 at 10:19 AM Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
>
> I was talking about taking this new lock in both uinput_request_send()
> as well as in uinput_destroy_device() when updating the state. With that
> requests_lock will be taken only in uinput_request_alloc_id(),
> uinput_request_release_slot(), and uinput_flush_requests().
Hi Dmitry,
I've sent v2 as a separate thread with a dedicated state_lock
spinlock as you suggested, along with Fixes and Cc: stable tags.
https://lore.kernel.org/all/20260407075031.38351-1-mikhail.v.gavrilov@gmail.com/
--
Best Regards,
Mike Gavrilov.
^ permalink raw reply
* [PATCH v4] xpad: Overhaul device data for wireless devices
From: Sanjay Govind @ 2026-04-07 8:13 UTC (permalink / raw)
To: Dmitry Torokhov, Vicki Pfau, Nilton Perim Neto, Mario Limonciello,
Sanjay Govind
Cc: Antheas Kapenekakis, linux-input, linux-kernel
Xbox 360 wireless controllers expose information in the link and
capabilities reports.
Extract and use the vendor id for wireless controllers, and use
the subtype to build a nicer device name and product id.
Some xbox 360 controllers put a vid and pid into the stick capability
data, so check if this was done, and pull the vid, pid and revision from
there.
Signed-off-by: Sanjay Govind <sanjay.govind9@gmail.com>
---
v2: Delay marking device as present until after capabilities or timeout
v3: Fix issues when receiving incorrect or missing link and capabilities reports
v4: Clear wireless state when processing device prescence change
drivers/input/joystick/xpad.c | 179 ++++++++++++++++++++++++++++++++--
1 file changed, 170 insertions(+), 9 deletions(-)
diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c
index bf4accf3f581..3538c7e55c6d 100644
--- a/drivers/input/joystick/xpad.c
+++ b/drivers/input/joystick/xpad.c
@@ -68,6 +68,7 @@
#include <linux/slab.h>
#include <linux/stat.h>
#include <linux/module.h>
+#include <linux/unaligned.h>
#include <linux/usb/input.h>
#include <linux/usb/quirks.h>
@@ -94,6 +95,22 @@
#define XTYPE_XBOXONE 3
#define XTYPE_UNKNOWN 4
+#define FLAG_FORCE_FEEDBACK 0x01
+
+#define SUBTYPE_GAMEPAD 0x01
+#define SUBTYPE_WHEEL 0x02
+#define SUBTYPE_ARCADE_STICK 0x03
+#define SUBTYPE_FLIGHT_SICK 0x04
+#define SUBTYPE_DANCE_PAD 0x05
+#define SUBTYPE_GUITAR 0x06
+#define SUBTYPE_GUITAR_ALTERNATE 0x07
+#define SUBTYPE_DRUM_KIT 0x08
+#define SUBTYPE_GUITAR_BASS 0x0B
+#define SUBTYPE_RB_KEYBOARD 0x0F
+#define SUBTYPE_ARCADE_PAD 0x13
+#define SUBTYPE_TURNTABLE 0x17
+#define SUBTYPE_PRO_GUITAR 0x19
+
/* Send power-off packet to xpad360w after holding the mode button for this many
* seconds
*/
@@ -795,8 +812,13 @@ struct usb_xpad {
int xtype; /* type of xbox device */
int packet_type; /* type of the extended packet */
int pad_nr; /* the order x360 pads were attached */
+ u8 sub_type;
+ u16 flags;
+ u16 wireless_vid;
+ u16 wireless_pid;
+ u16 wireless_version;
const char *name; /* name of the device */
- struct work_struct work; /* init/remove device from callback */
+ struct delayed_work work; /* init/remove device from callback */
time64_t mode_btn_down_ts;
bool delay_init; /* init packets should be delayed */
bool delayed_init_done;
@@ -807,6 +829,8 @@ static void xpad_deinit_input(struct usb_xpad *xpad);
static int xpad_start_input(struct usb_xpad *xpad);
static void xpadone_ack_mode_report(struct usb_xpad *xpad, u8 seq_num);
static void xpad360w_poweroff_controller(struct usb_xpad *xpad);
+static int xpad_inquiry_pad_capabilities(struct usb_xpad *xpad);
+
/*
* xpad_process_packet
@@ -980,7 +1004,7 @@ static void xpad360_process_packet(struct usb_xpad *xpad, struct input_dev *dev,
static void xpad_presence_work(struct work_struct *work)
{
- struct usb_xpad *xpad = container_of(work, struct usb_xpad, work);
+ struct usb_xpad *xpad = container_of(work, struct usb_xpad, work.work);
int error;
if (xpad->pad_present) {
@@ -1017,7 +1041,7 @@ static void xpad_presence_work(struct work_struct *work)
* 01.1 - Pad state (Bytes 4+) valid
*
*/
-static void xpad360w_process_packet(struct usb_xpad *xpad, u16 cmd, unsigned char *data)
+static void xpad360w_process_packet(struct usb_xpad *xpad, u16 cmd, unsigned char *data, u32 len)
{
struct input_dev *dev;
bool present;
@@ -1028,7 +1052,67 @@ static void xpad360w_process_packet(struct usb_xpad *xpad, u16 cmd, unsigned cha
if (xpad->pad_present != present) {
xpad->pad_present = present;
- schedule_work(&xpad->work);
+ xpad->wireless_vid = 0;
+ xpad->wireless_pid = 0;
+ xpad->wireless_version = 0;
+ xpad->flags = 0;
+ xpad->sub_type = 0;
+ if (present) {
+ /*
+ * Delay marking device as present, so we can make sure
+ * we have received all the information from the capabilities
+ * report. Some devices don't send one, so the delay
+ * guarantees that these devices are still initialized.
+ */
+ schedule_delayed_work(&xpad->work, msecs_to_jiffies(500));
+ } else {
+ schedule_delayed_work(&xpad->work, 0);
+ }
+ }
+ }
+
+ /* Link report */
+ if (data[0] == 0x00 && data[1] == 0x0F && len >= 26) {
+ xpad->sub_type = data[25] & 0x7f;
+
+ /* Decode vendor id from link report */
+ xpad->wireless_vid = ((data[0x16] & 0xf) | data[0x18] << 4) << 8 | data[0x17];
+
+ /*
+ * If the link report doesn't provide a proper vid, it sets the vid to 1.
+ * In that case we zero out wireless_vid, so that we fall back to the vid
+ * from the receiver instead.
+ */
+ if (xpad->wireless_vid == 1)
+ xpad->wireless_vid = 0;
+ /*
+ * x360w controllers on windows put the subtype into the product
+ * for wheels and gamepads, but it makes sense to do it for all
+ * subtypes. This will be used if the capabilities report
+ * doesn't provide us with a product id later.
+ */
+ xpad->wireless_pid = 0x02a0 + xpad->sub_type;
+ xpad->wireless_version = 0;
+
+ if ((data[25] & 0x80) != 0)
+ xpad->flags |= FLAG_FORCE_FEEDBACK;
+
+ xpad_inquiry_pad_capabilities(xpad);
+ }
+
+ /* Capabilities report */
+ if (data[0] == 0x00 && data[1] == 0x05 && data[5] == 0x12 && len >= 21) {
+ xpad->flags |= data[20];
+ /*
+ * A bunch of vendors started putting vids and pids
+ * into capabilities data because they can't be
+ * retrieved by xinput easliy.
+ * Not all of them do though, so check the vids match
+ * before extracting that info.
+ */
+ if (get_unaligned_le16(data + 10) == xpad->wireless_vid) {
+ xpad->wireless_pid = get_unaligned_le16(data + 12);
+ xpad->wireless_version = get_unaligned_le16(data + 14);
}
}
@@ -1254,7 +1338,7 @@ static void xpad_irq_in(struct urb *urb)
xpad360_process_packet(xpad, xpad->dev, 0, xpad->idata);
break;
case XTYPE_XBOX360W:
- xpad360w_process_packet(xpad, 0, xpad->idata);
+ xpad360w_process_packet(xpad, 0, xpad->idata, urb->actual_length);
break;
case XTYPE_XBOXONE:
xpadone_process_packet(xpad, 0, xpad->idata, urb->actual_length);
@@ -1495,6 +1579,31 @@ static int xpad_inquiry_pad_presence(struct usb_xpad *xpad)
return xpad_try_sending_next_out_packet(xpad);
}
+static int xpad_inquiry_pad_capabilities(struct usb_xpad *xpad)
+{
+ struct xpad_output_packet *packet =
+ &xpad->out_packets[XPAD_OUT_CMD_IDX];
+
+ guard(spinlock_irqsave)(&xpad->odata_lock);
+
+ packet->data[0] = 0x00;
+ packet->data[1] = 0x00;
+ packet->data[2] = 0x02;
+ packet->data[3] = 0x80;
+ packet->data[4] = 0x00;
+ packet->data[5] = 0x00;
+ packet->data[6] = 0x00;
+ packet->data[7] = 0x00;
+ packet->data[8] = 0x00;
+ packet->data[9] = 0x00;
+ packet->data[10] = 0x00;
+ packet->data[11] = 0x00;
+ packet->len = 12;
+ packet->pending = true;
+
+ return xpad_try_sending_next_out_packet(xpad);
+}
+
static int xpad_start_xbox_one(struct usb_xpad *xpad)
{
int error;
@@ -1893,7 +2002,7 @@ static void xpad360w_stop_input(struct usb_xpad *xpad)
usb_kill_urb(xpad->irq_in);
/* Make sure we are done with presence work if it was scheduled */
- flush_work(&xpad->work);
+ flush_delayed_work(&xpad->work);
}
static int xpad_open(struct input_dev *dev)
@@ -1965,8 +2074,60 @@ static int xpad_init_input(struct usb_xpad *xpad)
usb_to_input_id(xpad->udev, &input_dev->id);
if (xpad->xtype == XTYPE_XBOX360W) {
- /* x360w controllers and the receiver have different ids */
- input_dev->id.product = 0x02a1;
+ if (xpad->wireless_vid)
+ input_dev->id.vendor = xpad->wireless_vid;
+ if (xpad->wireless_pid)
+ input_dev->id.product = xpad->wireless_pid;
+ else
+ /* Default product id for x360w controllers */
+ input_dev->id.product = 0x02a1;
+ if (xpad->wireless_version)
+ input_dev->id.version = xpad->wireless_version;
+ switch (xpad->sub_type) {
+ case SUBTYPE_GAMEPAD:
+ input_dev->name = "Xbox 360 Wireless Controller";
+ break;
+ case SUBTYPE_WHEEL:
+ input_dev->name = "Xbox 360 Wireless Wheel";
+ break;
+ case SUBTYPE_ARCADE_STICK:
+ input_dev->name = "Xbox 360 Wireless Arcade Stick";
+ break;
+ case SUBTYPE_FLIGHT_SICK:
+ input_dev->name = "Xbox 360 Wireless Flight Stick";
+ break;
+ case SUBTYPE_DANCE_PAD:
+ input_dev->name = "Xbox 360 Wireless Dance Pad";
+ break;
+ case SUBTYPE_GUITAR:
+ input_dev->name = "Xbox 360 Wireless Guitar";
+ break;
+ case SUBTYPE_GUITAR_ALTERNATE:
+ input_dev->name = "Xbox 360 Wireless Alternate Guitar";
+ break;
+ case SUBTYPE_GUITAR_BASS:
+ input_dev->name = "Xbox 360 Wireless Bass Guitar";
+ break;
+ case SUBTYPE_DRUM_KIT:
+ /* Vendors used force feedback flag to differentiate these */
+ if (xpad->flags & FLAG_FORCE_FEEDBACK)
+ input_dev->name = "Xbox 360 Wireless Guitar Hero Drum Kit";
+ else
+ input_dev->name = "Xbox 360 Wireless Rock Band Drum Kit";
+ break;
+ case SUBTYPE_RB_KEYBOARD:
+ input_dev->name = "Xbox 360 Wireless Rock Band Keyboard";
+ break;
+ case SUBTYPE_ARCADE_PAD:
+ input_dev->name = "Xbox 360 Wireless Arcade Pad";
+ break;
+ case SUBTYPE_TURNTABLE:
+ input_dev->name = "Xbox 360 Wireless DJ Hero Turntable";
+ break;
+ case SUBTYPE_PRO_GUITAR:
+ input_dev->name = "Xbox 360 Wireless Rock Band Pro Guitar";
+ break;
+ }
}
input_dev->dev.parent = &xpad->intf->dev;
@@ -2106,7 +2267,7 @@ static int xpad_probe(struct usb_interface *intf, const struct usb_device_id *id
xpad->delay_init = true;
xpad->packet_type = PKT_XB;
- INIT_WORK(&xpad->work, xpad_presence_work);
+ INIT_DELAYED_WORK(&xpad->work, xpad_presence_work);
if (xpad->xtype == XTYPE_UNKNOWN) {
if (intf->cur_altsetting->desc.bInterfaceClass == USB_CLASS_VENDOR_SPEC) {
--
2.53.0
^ permalink raw reply related
* Re: [PATCH v2 1/1] HID: add malicious HID device detection driver
From: Benjamin Tissoires @ 2026-04-07 7:59 UTC (permalink / raw)
To: Zubeyr Almaho; +Cc: Jiri Kosina, linux-input, linux-kernel, security
In-Reply-To: <20260404133746.80914-2-zybo1000@gmail.com>
Hi,
On Apr 04 2026, Zubeyr Almaho wrote:
> Add a passive HID driver that computes a suspicion score for keyboard-like
> USB devices based on:
>
> - low keystroke timing entropy,
> - immediate post-enumeration typing,
> - known suspicious VID/PID and descriptor anomalies.
>
> If the score exceeds a tunable threshold, the driver emits a warning and
> suggests userspace blocking (e.g. usbguard). The module never blocks or
> modifies HID events.
>
> Signed-off-by: Zubeyr Almaho <zybo1000@gmail.com>
As Greg said, a HID-BPF program would be far better in terms of scope
because there are numerous pitfal in your implementation.
The question will be how to notify userspace of the suspicious HID
device in HID-BPF, but here, you are also just emitting a dmesg warning,
so it's not doing much either.
Anyway, comments inline, but please reach out to the end where I explain
why the implementation can't work in it's current state.
> ---
> drivers/hid/hid-omg-detect.c | 435 ++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 435 insertions(+)
> create mode 100644 drivers/hid/hid-omg-detect.c
>
> diff --git a/drivers/hid/hid-omg-detect.c b/drivers/hid/hid-omg-detect.c
> new file mode 100644
> --- /dev/null
> +++ b/drivers/hid/hid-omg-detect.c
> @@ -0,0 +1,435 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * hid-omg-detect.c - Malicious HID Device Detection Kernel Module
> + *
> + * Detects O.MG Cable and similar BadUSB devices by combining:
> + * 1. Keystroke timing entropy analysis (human vs machine rhythm)
> + * 2. Plug-and-type detection (typing immediately after connect)
> + * 3. USB descriptor fingerprinting (known bad VID/PIDs + anomalies)
> + *
> + * When suspicion score >= threshold, logs a kernel warning and suggests
> + * blocking the device with usbguard.
> + *
> + * The driver is purely passive — it does not drop, modify, or delay any
> + * HID events.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/kernel.h>
> +#include <linux/hid.h>
> +#include <linux/usb.h>
> +#include <linux/ktime.h>
> +#include <linux/slab.h>
> +#include <linux/spinlock.h>
> +#include <linux/math64.h>
> +
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Zübeyr Almaho <zybo1000@gmail.com>");
> +MODULE_DESCRIPTION("O.MG Cable / Malicious HID Device Detector");
> +
> +/* ============================================================
> + * Tuneable parameters (pass as insmod args)
> + * ============================================================ */
I'm pretty sure checkpatch.pl would complain about those multiline
comments.
> +static int score_threshold = 70;
> +module_param(score_threshold, int, 0644);
> +MODULE_PARM_DESC(score_threshold,
> + "Suspicion score (0-100) to trigger alert (default: 70)");
> +
> +static int plug_type_ms = 500;
> +module_param(plug_type_ms, int, 0644);
> +MODULE_PARM_DESC(plug_type_ms,
> + "Max ms after connect before first key = plug-and-type (default: 500)");
> +
> +static int entropy_variance_low = 500;
> +module_param(entropy_variance_low, int, 0644);
> +MODULE_PARM_DESC(entropy_variance_low,
> + "Variance (us^2) below which typing is machine-like (default: 500)");
> +
> +/* ============================================================
> + * Constants
> + * ============================================================ */
> +#define DRIVER_NAME "hid_omg_detect"
> +#define MAX_TIMING_SAMPLES 64
> +#define MIN_SAMPLES 10
> +
> +/* ============================================================
> + * Known suspicious VID/PID pairs (O.MG Cable, Rubber Ducky, etc.)
> + * ============================================================ */
> +static const struct {
> + u16 vid;
> + u16 pid;
> + const char *name;
> +} suspicious_ids[] = {
> + { 0x16c0, 0x27db, "O.MG Cable (classic)" },
> + { 0x1209, 0xa000, "O.MG Cable (Elite)" },
> + { 0x16c0, 0x27dc, "USB Rubber Ducky" },
> + { 0x1b4f, 0x9208, "LilyPad Arduino BadUSB" },
> +};
> +
> +/* ============================================================
> + * Per-device state (allocated on probe, freed on remove)
> + * ============================================================ */
> +struct omg_device_state {
> + struct hid_device *hdev;
> +
> + /* --- connection timing --- */
> + ktime_t connect_time;
> + ktime_t first_key_time;
> + bool first_key_seen;
> +
> + /* --- keystroke interval circular buffer --- */
> + ktime_t last_key_time;
> + s64 intervals_us[MAX_TIMING_SAMPLES];
> + unsigned int sample_count;
> + unsigned int sample_head;
> +
> + /* --- USB info (immutable after probe) --- */
> + u16 vid;
> + u16 pid;
> + const char *vid_pid_match_name; /* non-NULL if VID/PID matched */
> + bool descriptor_anomaly;
> +
> + /* --- verdict --- */
> + int score;
> + bool alerted;
> +
> + spinlock_t lock;
> +};
> +
> +/* ============================================================
> + * Math helpers
> + * ============================================================ */
> +static void timing_stats(struct omg_device_state *s, s64 *mean, s64 *variance)
> +{
> + unsigned int i, n;
> + s64 sum = 0, sq = 0, m;
> +
> + n = min(s->sample_count, (unsigned int)MAX_TIMING_SAMPLES);
> +
> + if (n < 2) {
> + *mean = 0;
> + *variance = 0;
> + return;
> + }
> +
> + for (i = 0; i < n; i++)
> + sum += s->intervals_us[i];
> + m = div_s64(sum, n);
> +
> + for (i = 0; i < n; i++) {
> + s64 d = s->intervals_us[i] - m;
> +
> + sq += d * d;
> + }
> +
> + *mean = m;
> + *variance = div_s64(sq, n);
> +}
> +
> +/* ============================================================
> + * Signal 1: Timing entropy
> + *
> + * Humans: high variance (pauses, rhythm changes)
> + * Machines: low variance (constant clock)
> + *
> + * Also penalises extremely fast consistent typing (mean < 15 ms).
> + * ============================================================ */
> +static int score_entropy(struct omg_device_state *s)
> +{
> + s64 mean, var;
> + int pts = 0;
> +
> + if (s->sample_count < MIN_SAMPLES)
> + return 0;
> +
> + timing_stats(s, &mean, &var);
> +
> + if (var < (s64)entropy_variance_low)
> + pts += 35;
> + else if (var < (s64)entropy_variance_low * 10)
> + pts += 20;
> + else if (var < (s64)entropy_variance_low * 40)
> + pts += 8;
> +
> + if (mean > 0 && mean < 5000)
> + pts += 25;
> + else if (mean > 0 && mean < 15000)
> + pts += 10;
> +
> + return min(pts, 60);
> +}
> +
> +/* ============================================================
> + * Signal 2: Plug-and-type
> + *
> + * A legitimate keyboard sits idle for a while after connect.
> + * A script device starts injecting within milliseconds.
> + * ============================================================ */
> +static int score_plug_type(struct omg_device_state *s)
> +{
> + s64 delta_ms;
> +
> + if (!s->first_key_seen)
> + return 0;
> +
> + delta_ms = ktime_to_ms(ktime_sub(s->first_key_time, s->connect_time));
> +
> + if (delta_ms < 100)
> + return 30;
> + if (delta_ms < (s64)plug_type_ms)
> + return 15;
> + if (delta_ms < 2000)
> + return 5;
> +
> + return 0;
> +}
> +
> +/* ============================================================
> + * Signal 3: USB descriptor fingerprint
> + * ============================================================ */
> +static int score_descriptor(struct omg_device_state *s)
> +{
> + int pts = 0;
> +
> + if (s->vid_pid_match_name)
> + pts += 50;
> +
> + if (s->descriptor_anomaly)
> + pts += 20;
> +
> + return min(pts, 50);
> +}
> +
> +/* ============================================================
> + * Combine all signals and return per-signal breakdown.
> + * Caller is responsible for alerting outside any lock.
> + * ============================================================ */
> +struct omg_score_result {
> + int entropy;
> + int plug_type;
> + int descriptor;
> + int total;
> + bool newly_alerted;
> +};
> +
> +static void compute_score(struct omg_device_state *s,
> + struct omg_score_result *res)
> +{
> + res->entropy = score_entropy(s);
> + res->plug_type = score_plug_type(s);
> + res->descriptor = score_descriptor(s);
> + res->total = min(res->entropy + res->plug_type + res->descriptor,
> + 100);
> +
> + s->score = res->total;
> + res->newly_alerted = false;
> +
> + if (res->total >= score_threshold && !s->alerted) {
> + s->alerted = true;
> + res->newly_alerted = true;
> + }
> +}
> +
> +/* Emit alert outside of spinlock */
> +static void emit_alert(struct hid_device *hdev,
> + struct omg_device_state *s,
> + const struct omg_score_result *res)
> +{
> + hid_warn(hdev, "=============================================\n");
> + hid_warn(hdev, "!! SUSPICIOUS HID DEVICE DETECTED !!\n");
> + hid_warn(hdev, "Device : %s\n", hdev->name);
> + hid_warn(hdev, "VID/PID: %04x:%04x\n", s->vid, s->pid);
> + if (s->vid_pid_match_name)
> + hid_warn(hdev, "Match : %s\n", s->vid_pid_match_name);
> + hid_warn(hdev, "Score : %d/100 (threshold=%d)\n",
> + res->total, score_threshold);
> + hid_warn(hdev, " Entropy : %d pts\n", res->entropy);
> + hid_warn(hdev, " Plug-and-type: %d pts\n", res->plug_type);
> + hid_warn(hdev, " Descriptor : %d pts\n", res->descriptor);
> + hid_warn(hdev, "Action : sudo usbguard block-device <id>\n");
> + hid_warn(hdev, "=============================================\n");
> +}
> +
> +/* ============================================================
> + * HID raw_event hook — called for every incoming HID report
> + *
> + * This runs in softirq/BH context — no sleeping locks allowed.
> + * ============================================================ */
> +static int omg_raw_event(struct hid_device *hdev,
> + struct hid_report *report,
> + u8 *data, int size)
> +{
> + struct omg_device_state *s = hid_get_drvdata(hdev);
> + struct omg_score_result res;
> + ktime_t now;
> + unsigned long flags;
> + bool keystroke = false;
> + int i;
> +
> + if (!s)
> + return 0;
> +
> + if (report->type != HID_INPUT_REPORT)
> + return 0;
> +
> + /* Byte 0 = modifier, byte 1 = reserved, bytes 2-7 = keycodes */
That's a strong assumption. This actually depend on the report
descriptor of the device and what you said is generally true for USB
keyboards, but not all of HID devices behave like that:
- bluetooth keyboards usually have a report ID in byte 0
- some devices are reporting gyro data, so they are sending a stream of
events which would likely be tagged as "suspicious"
- some devices are reporting touch information, and as such they are
also reporting a constant stream of events as long as at least one
finger is touching the sensor
- yubikeys, other security keys and gaming devices are devices
specifically tailored to send a stream of events, being a keyboard
macro, or a one time password, or a stored passwored in the key
- not all devices are keyboards :)
> + for (i = 2; i < size && i < 8; i++) {
> + if (data[i] != 0) {
> + keystroke = true;
> + break;
> + }
> + }
> + if (size > 0 && data[0] != 0)
> + keystroke = true;
> +
> + if (!keystroke)
> + return 0;
> +
> + now = ktime_get();
> +
> + spin_lock_irqsave(&s->lock, flags);
Why spin_locking?
> +
> + if (!s->first_key_seen) {
> + s->first_key_seen = true;
> + s->first_key_time = now;
> + s->last_key_time = now;
> + } else {
> + s64 interval = ktime_to_us(ktime_sub(now, s->last_key_time));
> +
> + /* ignore idle gaps > 10 s */
> + if (interval < 10000000LL) {
> + s->intervals_us[s->sample_head] = interval;
> + s->sample_head =
> + (s->sample_head + 1) % MAX_TIMING_SAMPLES;
> + if (s->sample_count < MAX_TIMING_SAMPLES)
> + s->sample_count++;
> + }
> + s->last_key_time = now;
> + }
> +
> + compute_score(s, &res);
> +
> + spin_unlock_irqrestore(&s->lock, flags);
> +
> + /* Alert outside the spinlock — hid_warn may sleep */
> + if (res.newly_alerted)
> + emit_alert(hdev, s, &res);
> +
> + return 0;
> +}
> +
> +/* ============================================================
> + * HID probe — device connected
> + * ============================================================ */
> +static int omg_probe(struct hid_device *hdev, const struct hid_device_id *id)
> +{
> + struct omg_device_state *s;
> + struct usb_interface *intf;
> + struct usb_device *udev;
> + struct omg_score_result res;
> + int ret, i;
> +
> + if (hdev->bus != BUS_USB)
> + return -ENODEV;
Why restricting to BUS_USB only? Bluetooth could be a strong acttack
vector as well.
> +
> + s = kzalloc(sizeof(*s), GFP_KERNEL);
devm_kzalloc?
> + if (!s)
> + return -ENOMEM;
> +
> + spin_lock_init(&s->lock);
> + s->hdev = hdev;
> + s->connect_time = ktime_get();
> +
> + /* Extract USB descriptor info */
> + intf = to_usb_interface(hdev->dev.parent);
You can't call that function unless you are sure that you are talking to
ta USB device. If the user is using a uhid device (because they are
replaying another), the transport driver is not usbhid, and you'll end
up with a lot of issues.
There is a function for checking if the transport is usb: hid_is_usb()
> + udev = interface_to_usbdev(intf);
> + s->vid = le16_to_cpu(udev->descriptor.idVendor);
> + s->pid = le16_to_cpu(udev->descriptor.idProduct);
> +
> + /* Check known bad VID/PID table (once, at probe time) */
> + for (i = 0; i < ARRAY_SIZE(suspicious_ids); i++) {
> + if (s->vid == suspicious_ids[i].vid &&
> + s->pid == suspicious_ids[i].pid) {
> + s->vid_pid_match_name = suspicious_ids[i].name;
> + break;
> + }
> + }
> +
> + /*
> + * Anomaly: legitimate HID keyboards use bDeviceClass = 0x00
> + * (class defined at interface level). Any other value here
> + * for a device presenting as a keyboard is suspicious.
> + */
> + s->descriptor_anomaly =
> + (udev->descriptor.bDeviceClass != 0x00 &&
> + udev->descriptor.bDeviceClass != 0x03);
> +
> + hid_set_drvdata(hdev, s);
> +
> + ret = hid_parse(hdev);
> + if (ret) {
> + hid_err(hdev, "hid_parse failed: %d\n", ret);
> + goto err_free;
> + }
> +
> + ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
> + if (ret) {
> + hid_err(hdev, "hid_hw_start failed: %d\n", ret);
> + goto err_free;
> + }
> +
> + hid_info(hdev, "Monitoring %s (VID=%04x PID=%04x%s%s)\n",
> + hdev->name, s->vid, s->pid,
> + s->vid_pid_match_name ? " KNOWN-BAD:" : "",
> + s->vid_pid_match_name ? s->vid_pid_match_name : "");
> +
> + if (s->descriptor_anomaly)
> + hid_warn(hdev, "Descriptor anomaly: bDeviceClass=0x%02x\n",
> + udev->descriptor.bDeviceClass);
> +
> + /* Run initial descriptor-only score */
> + compute_score(s, &res);
> + if (res.newly_alerted)
> + emit_alert(hdev, s, &res);
> +
> + return 0;
> +
> +err_free:
> + hid_set_drvdata(hdev, NULL);
> + kfree(s);
no need to free when using devm_kzalloc.
> + return ret;
> +}
> +
> +/* ============================================================
> + * HID remove — device disconnected
> + * ============================================================ */
> +static void omg_remove(struct hid_device *hdev)
> +{
> + struct omg_device_state *s = hid_get_drvdata(hdev);
> +
> + hid_hw_stop(hdev);
> +
> + if (s) {
> + hid_info(hdev, "Removed %s (final score: %d/100)\n",
> + hdev->name, s->score);
> + hid_set_drvdata(hdev, NULL);
> + kfree(s);
> + }
> +}
this omg_remove() function can be removed entirely when using
devm_kzalloc.
> +
> +/* Match all USB HID devices — we inspect and score them all */
> +static const struct hid_device_id omg_table[] = {
> + { HID_USB_DEVICE(HID_ANY_ID, HID_ANY_ID) },
ouch. Very much ouch:
You are binding to any USB device, but you don't have implemented a
.match callback, meaning that you are racing with any other driver to
take the ownership of a USB device.
This is where your plan falls completely. There can only be one HID
driver attached to a specific HID device. And we have multiple HID
drivers in the subsystem. So either they get their fixes (the original
HID driver), either they'll get the new security driver but they can not
have both. And even worse, there is no guarantee one driver will be
loaded before the other.
hid-generic is a special case as this is a fallback driver. So
hid-generic is capable of unbinding itself to let device specific
drivers to take over the device. But here, that driver just takes
ownership and can't release it (otherwise it's pointless).
So definitively, addressing that problem at the driver level is the
wrong place:
- either make it a HID core feature, but it'll be harder to sell IMO
- either make it a separate module that HID core calls in: slightly
easier to sell, but still hard
- either work at the HID-BPF level: much better because you'll need some
userspace eventually and this way you control from userpsace both the
HID-BPF program and your userspace part
I've recently added the parsed report descriptor injection during probe
in the HID-BPF program (assuming you use udev-hid-bpf to load HID-BPF).
So in theory, you can create a HID-BPF which would matches on keyboards,
chose which bytes are interesting to look at and then bind on those *in
addition* to any kernel driver. It happens before any kernel processing
so you've got the raw event stream and can decide to block or not the
stream.
Anyway, even with the various issues I mentioned in the code, this
present version can't be merged, as long as you are using a HID driver.
Cheers,
Benjamin
> + { }
> +};
> +MODULE_DEVICE_TABLE(hid, omg_table);
> +
> +static struct hid_driver omg_driver = {
> + .name = DRIVER_NAME,
> + .id_table = omg_table,
> + .probe = omg_probe,
> + .remove = omg_remove,
> + .raw_event = omg_raw_event,
> +};
> +
> +module_hid_driver(omg_driver);
>
^ permalink raw reply
* [PATCH v2] Input: uinput - fix circular locking dependency with ff-core
From: Mikhail Gavrilov @ 2026-04-07 7:50 UTC (permalink / raw)
To: dmitry.torokhov; +Cc: linux-input, linux-kernel, stable, Mikhail Gavrilov
A lockdep circular locking dependency warning can be triggered
reproducibly when using a force-feedback gamepad with uinput (for
example, playing ELDEN RING under Wine with a Flydigi Vader 5
controller):
ff->mutex -> udev->mutex -> input_mutex -> dev->mutex -> ff->mutex
The cycle is caused by four lock acquisition paths:
1. ff upload: input_ff_upload() holds ff->mutex and calls
uinput_dev_upload_effect() -> uinput_request_submit() ->
uinput_request_send(), which acquires udev->mutex.
2. device create: uinput_ioctl_handler() holds udev->mutex and calls
uinput_create_device() -> input_register_device(), which acquires
input_mutex.
3. device register: input_register_device() holds input_mutex and
calls kbd_connect() -> input_register_handle(), which acquires
dev->mutex.
4. evdev release: evdev_release() calls input_flush_device() under
dev->mutex, which calls input_ff_flush() acquiring ff->mutex.
Fix this by introducing a new state_lock spinlock to protect
udev->state and udev->dev access in uinput_request_send() instead of
acquiring udev->mutex. The function only needs to atomically check
device state and queue an input event into the ring buffer via
uinput_dev_event() -- both operations are safe under a spinlock
(ktime_get_ts64() and wake_up_interruptible() do not sleep). This
breaks the ff->mutex -> udev->mutex link since a spinlock is a leaf in
the lock ordering and cannot form cycles with mutexes.
To keep state transitions visible to uinput_request_send(), protect
writes to udev->state in uinput_create_device() and
uinput_destroy_device() with the same state_lock spinlock.
Additionally, move init_completion(&request->done) from
uinput_request_send() to uinput_request_submit() before
uinput_request_reserve_slot(). Once the slot is allocated,
uinput_flush_requests() may call complete() on it at any time from
the destroy path, so the completion must be initialised before the
request becomes visible.
Lock ordering after the fix:
ff->mutex -> state_lock (spinlock, leaf)
udev->mutex -> state_lock (spinlock, leaf)
udev->mutex -> input_mutex -> dev->mutex -> ff->mutex (no back-edge)
Fixes: ff462551235d ("Input: uinput - switch to the new FF interface")
Cc: stable@vger.kernel.org
Link: https://lore.kernel.org/all/CABXGCsMoxag+kEwHhb7KqhuyxfmGGd0P=tHZyb1uKE0pLr8Hkg@mail.gmail.com/
Signed-off-by: Mikhail Gavrilov <mikhail.v.gavrilov@gmail.com>
---
Changes since v1:
https://lore.kernel.org/all/20260228223628.472208-1-mikhail.v.gavrilov@gmail.com/
- Use a dedicated state_lock spinlock instead of reusing requests_lock,
as suggested by Dmitry Torokhov
- Add Fixes and Cc: stable tags
drivers/input/misc/uinput.c | 28 +++++++++++++++++++++-------
1 file changed, 21 insertions(+), 7 deletions(-)
diff --git a/drivers/input/misc/uinput.c b/drivers/input/misc/uinput.c
index e589060db280..e24caf6fc8e8 100644
--- a/drivers/input/misc/uinput.c
+++ b/drivers/input/misc/uinput.c
@@ -57,6 +57,7 @@ struct uinput_device {
struct input_dev *dev;
struct mutex mutex;
enum uinput_state state;
+ spinlock_t state_lock;
wait_queue_head_t waitq;
unsigned char ready;
unsigned char head;
@@ -146,19 +147,15 @@ static void uinput_request_release_slot(struct uinput_device *udev,
static int uinput_request_send(struct uinput_device *udev,
struct uinput_request *request)
{
- int retval;
+ int retval = 0;
- retval = mutex_lock_interruptible(&udev->mutex);
- if (retval)
- return retval;
+ spin_lock(&udev->state_lock);
if (udev->state != UIST_CREATED) {
retval = -ENODEV;
goto out;
}
- init_completion(&request->done);
-
/*
* Tell our userspace application about this new request
* by queueing an input event.
@@ -166,7 +163,7 @@ static int uinput_request_send(struct uinput_device *udev,
uinput_dev_event(udev->dev, EV_UINPUT, request->code, request->id);
out:
- mutex_unlock(&udev->mutex);
+ spin_unlock(&udev->state_lock);
return retval;
}
@@ -175,6 +172,13 @@ static int uinput_request_submit(struct uinput_device *udev,
{
int retval;
+ /*
+ * Initialize completion before allocating the request slot.
+ * Once the slot is allocated, uinput_flush_requests() may
+ * complete it at any time, so it must be initialized first.
+ */
+ init_completion(&request->done);
+
retval = uinput_request_reserve_slot(udev, request);
if (retval)
return retval;
@@ -289,7 +293,14 @@ static void uinput_destroy_device(struct uinput_device *udev)
struct input_dev *dev = udev->dev;
enum uinput_state old_state = udev->state;
+ /*
+ * Update state under state_lock so that concurrent
+ * uinput_request_send() sees the state change before we
+ * flush pending requests and tear down the device.
+ */
+ spin_lock(&udev->state_lock);
udev->state = UIST_NEW_DEVICE;
+ spin_unlock(&udev->state_lock);
if (dev) {
name = dev->name;
@@ -366,7 +377,9 @@ static int uinput_create_device(struct uinput_device *udev)
if (error)
goto fail2;
+ spin_lock(&udev->state_lock);
udev->state = UIST_CREATED;
+ spin_unlock(&udev->state_lock);
return 0;
@@ -384,6 +397,7 @@ static int uinput_open(struct inode *inode, struct file *file)
return -ENOMEM;
mutex_init(&newdev->mutex);
+ spin_lock_init(&newdev->state_lock);
spin_lock_init(&newdev->requests_lock);
init_waitqueue_head(&newdev->requests_waitq);
init_waitqueue_head(&newdev->waitq);
--
2.53.0
^ permalink raw reply related
* [PATCH v3] xpad: Overhaul device data for wireless devices
From: Sanjay Govind @ 2026-04-07 6:57 UTC (permalink / raw)
To: Dmitry Torokhov, Vicki Pfau, Sanjay Govind, Mario Limonciello,
Nilton Perim Neto, Kees Cook
Cc: Antheas Kapenekakis, linux-input, linux-kernel
Xbox 360 wireless controllers expose information in the link and
capabilities reports.
Extract and use the vendor id for wireless controllers, and use
the subtype to build a nicer device name and product id.
Some xbox 360 controllers put a vid and pid into the stick capability
data, so check if this was done, and pull the vid, pid and revision from
there.
Signed-off-by: Sanjay Govind <sanjay.govind9@gmail.com>
---
v2: Delay marking device as present until after capabilities or timeout
v3: Fix issues when receiving incorrect or missing link and capabilities reports
drivers/input/joystick/xpad.c | 174 ++++++++++++++++++++++++++++++++--
1 file changed, 165 insertions(+), 9 deletions(-)
diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c
index bf4accf3f581..18c355aff80f 100644
--- a/drivers/input/joystick/xpad.c
+++ b/drivers/input/joystick/xpad.c
@@ -68,6 +68,7 @@
#include <linux/slab.h>
#include <linux/stat.h>
#include <linux/module.h>
+#include <linux/unaligned.h>
#include <linux/usb/input.h>
#include <linux/usb/quirks.h>
@@ -94,6 +95,22 @@
#define XTYPE_XBOXONE 3
#define XTYPE_UNKNOWN 4
+#define FLAG_FORCE_FEEDBACK 0x01
+
+#define SUBTYPE_GAMEPAD 0x01
+#define SUBTYPE_WHEEL 0x02
+#define SUBTYPE_ARCADE_STICK 0x03
+#define SUBTYPE_FLIGHT_SICK 0x04
+#define SUBTYPE_DANCE_PAD 0x05
+#define SUBTYPE_GUITAR 0x06
+#define SUBTYPE_GUITAR_ALTERNATE 0x07
+#define SUBTYPE_DRUM_KIT 0x08
+#define SUBTYPE_GUITAR_BASS 0x0B
+#define SUBTYPE_RB_KEYBOARD 0x0F
+#define SUBTYPE_ARCADE_PAD 0x13
+#define SUBTYPE_TURNTABLE 0x17
+#define SUBTYPE_PRO_GUITAR 0x19
+
/* Send power-off packet to xpad360w after holding the mode button for this many
* seconds
*/
@@ -795,8 +812,13 @@ struct usb_xpad {
int xtype; /* type of xbox device */
int packet_type; /* type of the extended packet */
int pad_nr; /* the order x360 pads were attached */
+ u8 sub_type;
+ u16 flags;
+ u16 wireless_vid;
+ u16 wireless_pid;
+ u16 wireless_version;
const char *name; /* name of the device */
- struct work_struct work; /* init/remove device from callback */
+ struct delayed_work work; /* init/remove device from callback */
time64_t mode_btn_down_ts;
bool delay_init; /* init packets should be delayed */
bool delayed_init_done;
@@ -807,6 +829,8 @@ static void xpad_deinit_input(struct usb_xpad *xpad);
static int xpad_start_input(struct usb_xpad *xpad);
static void xpadone_ack_mode_report(struct usb_xpad *xpad, u8 seq_num);
static void xpad360w_poweroff_controller(struct usb_xpad *xpad);
+static int xpad_inquiry_pad_capabilities(struct usb_xpad *xpad);
+
/*
* xpad_process_packet
@@ -980,7 +1004,7 @@ static void xpad360_process_packet(struct usb_xpad *xpad, struct input_dev *dev,
static void xpad_presence_work(struct work_struct *work)
{
- struct usb_xpad *xpad = container_of(work, struct usb_xpad, work);
+ struct usb_xpad *xpad = container_of(work, struct usb_xpad, work.work);
int error;
if (xpad->pad_present) {
@@ -1017,7 +1041,7 @@ static void xpad_presence_work(struct work_struct *work)
* 01.1 - Pad state (Bytes 4+) valid
*
*/
-static void xpad360w_process_packet(struct usb_xpad *xpad, u16 cmd, unsigned char *data)
+static void xpad360w_process_packet(struct usb_xpad *xpad, u16 cmd, unsigned char *data, u32 len)
{
struct input_dev *dev;
bool present;
@@ -1028,7 +1052,62 @@ static void xpad360w_process_packet(struct usb_xpad *xpad, u16 cmd, unsigned cha
if (xpad->pad_present != present) {
xpad->pad_present = present;
- schedule_work(&xpad->work);
+ if (present) {
+ /*
+ * Delay marking device as present, so we can make sure
+ * we have received all the information from the capabilities
+ * report. Some devices don't send one, so the delay
+ * guarantees that these devices are still initialized.
+ */
+ schedule_delayed_work(&xpad->work, msecs_to_jiffies(500));
+ } else {
+ schedule_delayed_work(&xpad->work, 0);
+ }
+ }
+ }
+
+ /* Link report */
+ if (data[0] == 0x00 && data[1] == 0x0F && len >= 26) {
+ xpad->sub_type = data[25] & 0x7f;
+
+ /* Decode vendor id from link report */
+ xpad->wireless_vid = ((data[0x16] & 0xf) | data[0x18] << 4) << 8 | data[0x17];
+
+ /*
+ * If the link report doesn't provide a proper vid, it sets the vid to 1.
+ * In that case we zero out wireless_vid, so that we fall back to the vid
+ * from the receiver instead.
+ */
+ if (xpad->wireless_vid == 1)
+ xpad->wireless_vid = 0;
+ /*
+ * x360w controllers on windows put the subtype into the product
+ * for wheels and gamepads, but it makes sense to do it for all
+ * subtypes. This will be used if the capabilities report
+ * doesn't provide us with a product id later.
+ */
+ xpad->wireless_pid = 0x02a0 + xpad->sub_type;
+ xpad->wireless_version = 0;
+
+ if ((data[25] & 0x80) != 0)
+ xpad->flags |= FLAG_FORCE_FEEDBACK;
+
+ xpad_inquiry_pad_capabilities(xpad);
+ }
+
+ /* Capabilities report */
+ if (data[0] == 0x00 && data[1] == 0x05 && data[5] == 0x12 && len >= 20) {
+ xpad->flags |= data[20];
+ /*
+ * A bunch of vendors started putting vids and pids
+ * into capabilities data because they can't be
+ * retrieved by xinput easliy.
+ * Not all of them do though, so check the vids match
+ * before extracting that info.
+ */
+ if (get_unaligned_le16(data + 10) == xpad->wireless_vid) {
+ xpad->wireless_pid = get_unaligned_le16(data + 12);
+ xpad->wireless_version = get_unaligned_le16(data + 14);
}
}
@@ -1254,7 +1333,7 @@ static void xpad_irq_in(struct urb *urb)
xpad360_process_packet(xpad, xpad->dev, 0, xpad->idata);
break;
case XTYPE_XBOX360W:
- xpad360w_process_packet(xpad, 0, xpad->idata);
+ xpad360w_process_packet(xpad, 0, xpad->idata, urb->actual_length);
break;
case XTYPE_XBOXONE:
xpadone_process_packet(xpad, 0, xpad->idata, urb->actual_length);
@@ -1495,6 +1574,31 @@ static int xpad_inquiry_pad_presence(struct usb_xpad *xpad)
return xpad_try_sending_next_out_packet(xpad);
}
+static int xpad_inquiry_pad_capabilities(struct usb_xpad *xpad)
+{
+ struct xpad_output_packet *packet =
+ &xpad->out_packets[XPAD_OUT_CMD_IDX];
+
+ guard(spinlock_irqsave)(&xpad->odata_lock);
+
+ packet->data[0] = 0x00;
+ packet->data[1] = 0x00;
+ packet->data[2] = 0x02;
+ packet->data[3] = 0x80;
+ packet->data[4] = 0x00;
+ packet->data[5] = 0x00;
+ packet->data[6] = 0x00;
+ packet->data[7] = 0x00;
+ packet->data[8] = 0x00;
+ packet->data[9] = 0x00;
+ packet->data[10] = 0x00;
+ packet->data[11] = 0x00;
+ packet->len = 12;
+ packet->pending = true;
+
+ return xpad_try_sending_next_out_packet(xpad);
+}
+
static int xpad_start_xbox_one(struct usb_xpad *xpad)
{
int error;
@@ -1893,7 +1997,7 @@ static void xpad360w_stop_input(struct usb_xpad *xpad)
usb_kill_urb(xpad->irq_in);
/* Make sure we are done with presence work if it was scheduled */
- flush_work(&xpad->work);
+ flush_delayed_work(&xpad->work);
}
static int xpad_open(struct input_dev *dev)
@@ -1965,8 +2069,60 @@ static int xpad_init_input(struct usb_xpad *xpad)
usb_to_input_id(xpad->udev, &input_dev->id);
if (xpad->xtype == XTYPE_XBOX360W) {
- /* x360w controllers and the receiver have different ids */
- input_dev->id.product = 0x02a1;
+ if (xpad->wireless_vid)
+ input_dev->id.vendor = xpad->wireless_vid;
+ if (xpad->wireless_pid)
+ input_dev->id.product = xpad->wireless_pid;
+ else
+ /* Default product id for x360w controllers */
+ input_dev->id.product = 0x02a1;
+ if (xpad->wireless_version)
+ input_dev->id.version = xpad->wireless_version;
+ switch (xpad->sub_type) {
+ case SUBTYPE_GAMEPAD:
+ input_dev->name = "Xbox 360 Wireless Controller";
+ break;
+ case SUBTYPE_WHEEL:
+ input_dev->name = "Xbox 360 Wireless Wheel";
+ break;
+ case SUBTYPE_ARCADE_STICK:
+ input_dev->name = "Xbox 360 Wireless Arcade Stick";
+ break;
+ case SUBTYPE_FLIGHT_SICK:
+ input_dev->name = "Xbox 360 Wireless Flight Stick";
+ break;
+ case SUBTYPE_DANCE_PAD:
+ input_dev->name = "Xbox 360 Wireless Dance Pad";
+ break;
+ case SUBTYPE_GUITAR:
+ input_dev->name = "Xbox 360 Wireless Guitar";
+ break;
+ case SUBTYPE_GUITAR_ALTERNATE:
+ input_dev->name = "Xbox 360 Wireless Alternate Guitar";
+ break;
+ case SUBTYPE_GUITAR_BASS:
+ input_dev->name = "Xbox 360 Wireless Bass Guitar";
+ break;
+ case SUBTYPE_DRUM_KIT:
+ /* Vendors used force feedback flag to differentiate these */
+ if (xpad->flags & FLAG_FORCE_FEEDBACK)
+ input_dev->name = "Xbox 360 Wireless Guitar Hero Drum Kit";
+ else
+ input_dev->name = "Xbox 360 Wireless Rock Band Drum Kit";
+ break;
+ case SUBTYPE_RB_KEYBOARD:
+ input_dev->name = "Xbox 360 Wireless Rock Band Keyboard";
+ break;
+ case SUBTYPE_ARCADE_PAD:
+ input_dev->name = "Xbox 360 Wireless Arcade Pad";
+ break;
+ case SUBTYPE_TURNTABLE:
+ input_dev->name = "Xbox 360 Wireless DJ Hero Turntable";
+ break;
+ case SUBTYPE_PRO_GUITAR:
+ input_dev->name = "Xbox 360 Wireless Rock Band Pro Guitar";
+ break;
+ }
}
input_dev->dev.parent = &xpad->intf->dev;
@@ -2106,7 +2262,7 @@ static int xpad_probe(struct usb_interface *intf, const struct usb_device_id *id
xpad->delay_init = true;
xpad->packet_type = PKT_XB;
- INIT_WORK(&xpad->work, xpad_presence_work);
+ INIT_DELAYED_WORK(&xpad->work, xpad_presence_work);
if (xpad->xtype == XTYPE_UNKNOWN) {
if (intf->cur_altsetting->desc.bInterfaceClass == USB_CLASS_VENDOR_SPEC) {
--
2.53.0
^ permalink raw reply related
* Re: [PATCH v2] xpad: Overhaul device data for wireless devices
From: Sanjay Govind @ 2026-04-07 6:36 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Vicki Pfau, Nilton Perim Neto, Mario Limonciello,
Antheas Kapenekakis, Pierre-Loup A. Griffais, linux-input,
linux-kernel
In-Reply-To: <adSJp5U47g1JnImj@google.com>
On Tue, Apr 7, 2026 at 4:46 PM Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> When will it be set to 1?
The link report itself sets it to 1 in some cases, but i think i will
just change this so that if we get a link report and the vid == 1, we
set wireless_vid to 0, so we can rely on it being a non zero value
instead
> Maybe this should be disable_delayed_work() instead...
For simplicity I'll just get rid of that, and we can just always wait
for it to expire instead of trying to shortcut it if we get the
capabilities report faster than 500ms.
> Can this also read stale data for the same reason if the packet is too short?
I'll add in length checks.
^ permalink raw reply
* Re: [PATCH] Input: uinput - fix circular locking dependency with ff-core
From: Dmitry Torokhov @ 2026-04-07 5:19 UTC (permalink / raw)
To: Mikhail Gavrilov; +Cc: linux-input, linux-kernel
In-Reply-To: <CABXGCsNwLTAKFdRDHnGBdF-h3fnKY0_ysQJevMTv-N7+MNwS1A@mail.gmail.com>
Sorry for disappearing again...
On Mon, Mar 23, 2026 at 10:39:29AM +0500, Mikhail Gavrilov wrote:
> On Mon, Mar 23, 2026 at 10:34 AM Dmitry Torokhov
> <dmitry.torokhov@gmail.com> wrote:
> >
> > Hmm, I am not sure I see the issue. We are not going to change state
> > back to UIST_CREATED until after uinput_destroy_device() returns so we
> > will not submit more requests...
> >
> > What am I missing?
>
> You are right, there is no lock ordering issue since the state
> transition is one-way.
>
> The reason I reused requests_lock is that uinput_request_send()
> needs to atomically check state and access udev->dev. If we use a
> separate state_lock and release it before calling
> uinput_dev_event(), uinput_destroy_device() could run in between,
> unregister the device, and we'd hit a use-after-free on udev->dev.
>
> A separate lock would need to be held across the same scope,
> making it functionally equivalent to reusing requests_lock.
I was talking about taking this new lock in both uinput_request_send()
as well as in uinput_destroy_device() when updating the state. With that
requests_lock will be taken only in uinput_request_alloc_id(),
uinput_request_release_slot(), and uinput_flush_requests().
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH v2] Input: aiptek: validate raw macro indices before updating state
From: Dmitry Torokhov @ 2026-04-07 5:12 UTC (permalink / raw)
To: Pengpeng Hou; +Cc: andriy.shevchenko, kees, linux-input, linux-kernel
In-Reply-To: <20260329001711.88076-1-pengpeng@iscas.ac.cn>
On Sun, Mar 29, 2026 at 08:17:11AM +0800, Pengpeng Hou wrote:
> aiptek_irq() derives macro key indices directly from tablet reports and
> then uses them to index macroKeyEvents[]. Report types 4 and 5 also save
> the derived value in aiptek->lastMacro and later use that state to
> release the previous key.
>
> Validate the raw macro index once before it enters that state machine, so
> lastMacro only ever stores an in-range macro key. Keep direct bounds
> checks for report type 6, which reads the macro number from the packet
> body and uses it immediately.
>
> Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
Applied, thank you.
--
Dmitry
^ permalink raw reply
* Re: [PATCH v2] Input: gf2k: skip invalid hat lookup values
From: Dmitry Torokhov @ 2026-04-07 5:12 UTC (permalink / raw)
To: Pengpeng Hou; +Cc: linux-input, linux-kernel, kees
In-Reply-To: <20260407120001.1-gf2k-v2-pengpeng@iscas.ac.cn>
On Tue, Apr 07, 2026 at 09:56:52AM +0800, Pengpeng Hou wrote:
> gf2k_read() decodes the hat position from a 4-bit field and uses it
> directly to index gf2k_hat_to_axis[]. The lookup table only has nine
> entries, so malformed packets can read past the end of the fixed table.
>
> Skip hat reporting when the decoded value falls outside the lookup
> table instead of forcing it to the neutral position. This keeps the
> fix local and avoids reporting a made-up axis state for malformed
> packets.
>
> Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
Applied, thank you.
--
Dmitry
^ permalink raw reply
* Re: [PATCH v8 1/7] dt-bindings: input: syna,rmi4: Document syna,rmi4-s3706b
From: Dmitry Torokhov @ 2026-04-07 4:48 UTC (permalink / raw)
To: David Heidelberg
Cc: Kaustabh Chakraborty, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Jason A. Donenfeld, Matthias Schiffer,
Vincent Huang, Casey Connolly, linux-input, devicetree,
linux-kernel, phone-devel, Krzysztof Kozlowski
In-Reply-To: <5630a4af-e18f-4daf-9b04-ea61091d9e51@ixit.cz>
On Wed, Mar 25, 2026 at 12:33:23PM +0100, David Heidelberg wrote:
> On 24/03/2026 20:42, Dmitry Torokhov wrote:
> > On Tue, Mar 24, 2026 at 08:40:34PM +0100, David Heidelberg via B4 Relay wrote:
> > > From: David Heidelberg <david@ixit.cz>
> > >
> > > Mostly irrelevant for authentic Synaptics touchscreens, but very important
> > > for applying workarounds to cheap TS knockoffs.
> > >
> > > These knockoffs work well with the downstream driver, and since the user
> > > has no way to distinguish them, later in this patch set, we introduce
> > > workarounds to ensure they function as well as possible.
> > >
> > > Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> > > Signed-off-by: David Heidelberg <david@ixit.cz>
> > > ---
> > > Documentation/devicetree/bindings/input/syna,rmi4.yaml | 11 ++++++++---
> > > 1 file changed, 8 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/Documentation/devicetree/bindings/input/syna,rmi4.yaml b/Documentation/devicetree/bindings/input/syna,rmi4.yaml
> > > index 8685ef4481f4a..fb4804ac3544d 100644
> > > --- a/Documentation/devicetree/bindings/input/syna,rmi4.yaml
> > > +++ b/Documentation/devicetree/bindings/input/syna,rmi4.yaml
> > > @@ -18,9 +18,14 @@ description: |
> > > properties:
> > > compatible:
> > > - enum:
> > > - - syna,rmi4-i2c
> > > - - syna,rmi4-spi
> > > + oneOf:
> > > + - enum:
> > > + - syna,rmi4-i2c
> > > + - syna,rmi4-spi
> > > + - items:
> > > + - enum:
> > > + - syna,rmi4-s3706b # OnePlus 6/6T
> >
> > I thought that all the workarounds will be keyed off this new
> > compatible, but I do not see that. What am I missing?
>
> The compatible is used for sequence in the
>
> Input: synaptics-rmi4 - support fallback values for PDT descriptor bytes
>
> where it is used to provide values missing for OP6 (and possible others in
> the future, when added).
>
> From my understanding the series, only two patches (1st and last) are
> specific for the OP6, rest will likely benefit various TS not implementing
> full Synaptics set. All measures apply only when touchscreen reports
> something wrong.
If the sensor does not implement RMI4 protocol properly it should not
use rmi4 compatibility. I will not apply any patches that work around
incomplete implementations unless they are triggered by a dedicated
compatible.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH v2] xpad: Overhaul device data for wireless devices
From: Dmitry Torokhov @ 2026-04-07 4:46 UTC (permalink / raw)
To: Sanjay Govind
Cc: Vicki Pfau, Nilton Perim Neto, Mario Limonciello,
Antheas Kapenekakis, Pierre-Loup A. Griffais, linux-input,
linux-kernel
In-Reply-To: <20260404083928.489966-3-sanjay.govind9@gmail.com>
Hi Sanjay,
On Sat, Apr 04, 2026 at 09:39:27PM +1300, Sanjay Govind wrote:
> Xbox 360 wireless controllers expose information in the link and
> capabilities reports.
>
> Extract and use the vendor id for wireless controllers, and use
> the subtype to build a nicer device name and product id.
>
> Some xbox 360 controllers put a vid and pid into the stick capability
> data, so check if this was done, and pull the vid, pid and revision from
> there.
Sashuko correctly identified issues areoud re-scheduling work that
already completed, please see:
https://sashiko.dev/#/patchset/20260404083928.489966-3-sanjay.govind9@gmail.com
>
> Signed-off-by: Sanjay Govind <sanjay.govind9@gmail.com>
> ---
> v2: Delay marking device as present until after capabilities or timeout
> drivers/input/joystick/xpad.c | 162 ++++++++++++++++++++++++++++++++--
> 1 file changed, 155 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c
> index bf4accf3f581..c35512c7c199 100644
> --- a/drivers/input/joystick/xpad.c
> +++ b/drivers/input/joystick/xpad.c
> @@ -68,6 +68,7 @@
> #include <linux/slab.h>
> #include <linux/stat.h>
> #include <linux/module.h>
> +#include <linux/unaligned.h>
> #include <linux/usb/input.h>
> #include <linux/usb/quirks.h>
>
> @@ -94,6 +95,22 @@
> #define XTYPE_XBOXONE 3
> #define XTYPE_UNKNOWN 4
>
> +#define FLAG_FORCE_FEEDBACK 0x01
> +
> +#define SUBTYPE_GAMEPAD 0x01
> +#define SUBTYPE_WHEEL 0x02
> +#define SUBTYPE_ARCADE_STICK 0x03
> +#define SUBTYPE_FLIGHT_SICK 0x04
> +#define SUBTYPE_DANCE_PAD 0x05
> +#define SUBTYPE_GUITAR 0x06
> +#define SUBTYPE_GUITAR_ALTERNATE 0x07
> +#define SUBTYPE_DRUM_KIT 0x08
> +#define SUBTYPE_GUITAR_BASS 0x0B
> +#define SUBTYPE_RB_KEYBOARD 0x0F
> +#define SUBTYPE_ARCADE_PAD 0x13
> +#define SUBTYPE_TURNTABLE 0x17
> +#define SUBTYPE_PRO_GUITAR 0x19
> +
> /* Send power-off packet to xpad360w after holding the mode button for this many
> * seconds
> */
> @@ -795,8 +812,13 @@ struct usb_xpad {
> int xtype; /* type of xbox device */
> int packet_type; /* type of the extended packet */
> int pad_nr; /* the order x360 pads were attached */
> + u8 sub_type;
> + u16 flags;
> + u16 wireless_vid;
> + u16 wireless_pid;
> + u16 wireless_version;
> const char *name; /* name of the device */
> - struct work_struct work; /* init/remove device from callback */
> + struct delayed_work work; /* init/remove device from callback */
> time64_t mode_btn_down_ts;
> bool delay_init; /* init packets should be delayed */
> bool delayed_init_done;
> @@ -807,6 +829,8 @@ static void xpad_deinit_input(struct usb_xpad *xpad);
> static int xpad_start_input(struct usb_xpad *xpad);
> static void xpadone_ack_mode_report(struct usb_xpad *xpad, u8 seq_num);
> static void xpad360w_poweroff_controller(struct usb_xpad *xpad);
> +static int xpad_inquiry_pad_capabilities(struct usb_xpad *xpad);
> +
>
> /*
> * xpad_process_packet
> @@ -980,7 +1004,7 @@ static void xpad360_process_packet(struct usb_xpad *xpad, struct input_dev *dev,
>
> static void xpad_presence_work(struct work_struct *work)
> {
> - struct usb_xpad *xpad = container_of(work, struct usb_xpad, work);
> + struct usb_xpad *xpad = container_of(work, struct usb_xpad, work.work);
> int error;
>
> if (xpad->pad_present) {
> @@ -1028,10 +1052,60 @@ static void xpad360w_process_packet(struct usb_xpad *xpad, u16 cmd, unsigned cha
>
> if (xpad->pad_present != present) {
> xpad->pad_present = present;
> - schedule_work(&xpad->work);
> + if (present) {
> + /*
> + * Delay marking device as present, so we can make sure
> + * we have received all the information from the capabilities
> + * report. Some devices don't send one, so the delay
> + * guarantees that these devices are still initialized.
> + */
> + schedule_delayed_work(&xpad->work, msecs_to_jiffies(500));
> + } else {
> + schedule_delayed_work(&xpad->work, 0);
schedule_delayed_work(&xpad->work,
present ? msecs_to_jiffies(500) : 0);
> + }
> }
> }
>
> + /* Link report */
> + if (data[0] == 0x00 && data[1] == 0x0F) {
> + xpad->sub_type = data[25] & 0x7f;
> +
> + /* Decode vendor id from link report */
> + xpad->wireless_vid = ((data[0x16] & 0xf) | data[0x18] << 4) << 8 | data[0x17];
> + /*
> + * x360w controllers on windows put the subtype into the product
> + * for wheels and gamepads, but it makes sense to do it for all
> + * subtypes. This will be used if the capabilities report
> + * doesn't provide us with a product id later.
> + */
> + xpad->wireless_pid = 0x02a0 + xpad->sub_type;
> + xpad->wireless_version = 0;
> +
> + if ((data[25] & 0x80) != 0)
> + xpad->flags |= FLAG_FORCE_FEEDBACK;
> +
> + xpad_inquiry_pad_capabilities(xpad);
> + }
> +
> + /* Capabilities report */
> + if (data[0] == 0x00 && data[1] == 0x05 && data[5] == 0x12) {
> + xpad->flags |= data[20];
> + /*
> + * A bunch of vendors started putting vids and pids
> + * into capabilities data because they can't be
> + * retrieved by xinput easliy.
> + * Not all of them do though, so check the vids match
> + * before extracting that info.
> + */
> + if (get_unaligned_le16(data + 10) == xpad->wireless_vid) {
> + xpad->wireless_pid = get_unaligned_le16(data + 12);
> + xpad->wireless_version = get_unaligned_le16(data + 14);
> + }
> + /* We got the capabilities report, so mark the device present now */
> + cancel_delayed_work(&xpad->work);
> + schedule_delayed_work(&xpad->work, 0);
mod_delayed_work(&xpad->work, 0);
> + }
> +
> /* Valid pad data */
> if (data[1] != 0x1)
> return;
> @@ -1495,6 +1569,31 @@ static int xpad_inquiry_pad_presence(struct usb_xpad *xpad)
> return xpad_try_sending_next_out_packet(xpad);
> }
>
> +static int xpad_inquiry_pad_capabilities(struct usb_xpad *xpad)
> +{
> + struct xpad_output_packet *packet =
> + &xpad->out_packets[XPAD_OUT_CMD_IDX];
> +
> + guard(spinlock_irqsave)(&xpad->odata_lock);
> +
> + packet->data[0] = 0x00;
> + packet->data[1] = 0x00;
> + packet->data[2] = 0x02;
> + packet->data[3] = 0x80;
> + packet->data[4] = 0x00;
> + packet->data[5] = 0x00;
> + packet->data[6] = 0x00;
> + packet->data[7] = 0x00;
> + packet->data[8] = 0x00;
> + packet->data[9] = 0x00;
> + packet->data[10] = 0x00;
> + packet->data[11] = 0x00;
> + packet->len = 12;
> + packet->pending = true;
> +
> + return xpad_try_sending_next_out_packet(xpad);
> +}
> +
> static int xpad_start_xbox_one(struct usb_xpad *xpad)
> {
> int error;
> @@ -1893,7 +1992,7 @@ static void xpad360w_stop_input(struct usb_xpad *xpad)
> usb_kill_urb(xpad->irq_in);
>
> /* Make sure we are done with presence work if it was scheduled */
> - flush_work(&xpad->work);
> + flush_delayed_work(&xpad->work);
Maybe this should be disable_delayed_work() instead...
> }
>
> static int xpad_open(struct input_dev *dev)
> @@ -1965,8 +2064,57 @@ static int xpad_init_input(struct usb_xpad *xpad)
> usb_to_input_id(xpad->udev, &input_dev->id);
>
> if (xpad->xtype == XTYPE_XBOX360W) {
> - /* x360w controllers and the receiver have different ids */
> - input_dev->id.product = 0x02a1;
> + /* If the Link report has provided a vid, it won't be set to 1 */
> + if (xpad->wireless_vid != 1)
When will it be set to 1?
Thanks.
--
Dmitry
^ permalink raw reply
* [PATCH] HID: playstation: Add DualSense Edge extra button support
From: Aaron Webster @ 2026-04-07 4:40 UTC (permalink / raw)
To: Roderick Colenbrander, Jiri Kosina, Benjamin Tissoires
Cc: linux-input, linux-kernel, Aaron Webster
The DualSense Edge controller (product ID 0x0df2) has four additional
buttons compared to the standard DualSense: two front function buttons
(Fn1 and Fn2) and two rear paddles (left and right). These are reported
in bits 4-7 of buttons[2] in the input report.
Map them to BTN_TRIGGER_HAPPY1 through BTN_TRIGGER_HAPPY4 so that
userspace applications can use these extra inputs. An is_edge flag
gates the extra button handling based on the product ID.
Signed-off-by: Aaron Webster <awebster@gmail.com>
---
Tested with a DualSense Edge controller (Hardware: 1000208, Firmware:
1000087 type 3, Fw version: 20 131082 6, Sw series: 68, Update version:
0213, build date Jul 4 2025) on Debian 13 (trixie) with kernel
6.12.74+deb13+1-amd64 (x86_64) via Bluetooth.
drivers/hid/hid-playstation.c | 31 +++++++++++++++++++++++++++++++
1 file changed, 31 insertions(+)
diff --git a/drivers/hid/hid-playstation.c b/drivers/hid/hid-playstation.c
index 3c0db8f93..962159239 100644
--- a/drivers/hid/hid-playstation.c
+++ b/drivers/hid/hid-playstation.c
@@ -112,6 +112,12 @@ struct ps_led_info {
#define DS_BUTTONS2_TOUCHPAD BIT(1)
#define DS_BUTTONS2_MIC_MUTE BIT(2)
+/* DualSense Edge extra buttons in buttons[2], bits 4-7. */
+#define DS_EDGE_BUTTONS_FN1 BIT(4)
+#define DS_EDGE_BUTTONS_FN2 BIT(5)
+#define DS_EDGE_BUTTONS_LEFT_PADDLE BIT(6)
+#define DS_EDGE_BUTTONS_RIGHT_PADDLE BIT(7)
+
/* Status fields of DualSense input report. */
#define DS_STATUS0_BATTERY_CAPACITY GENMASK(3, 0)
#define DS_STATUS0_CHARGING GENMASK(7, 4)
@@ -178,6 +184,9 @@ struct dualsense {
struct input_dev *touchpad;
struct input_dev *jack;
+ /* True if this is a DualSense Edge (product 0x0df2). */
+ bool is_edge;
+
/* Update version is used as a feature/capability version. */
u16 update_version;
@@ -1486,6 +1495,18 @@ static int dualsense_parse_report(struct ps_device *ps_dev, struct hid_report *r
input_report_key(ds->gamepad, BTN_THUMBL, ds_report->buttons[1] & DS_BUTTONS1_L3);
input_report_key(ds->gamepad, BTN_THUMBR, ds_report->buttons[1] & DS_BUTTONS1_R3);
input_report_key(ds->gamepad, BTN_MODE, ds_report->buttons[2] & DS_BUTTONS2_PS_HOME);
+
+ if (ds->is_edge) {
+ input_report_key(ds->gamepad, BTN_TRIGGER_HAPPY1,
+ ds_report->buttons[2] & DS_EDGE_BUTTONS_FN1);
+ input_report_key(ds->gamepad, BTN_TRIGGER_HAPPY2,
+ ds_report->buttons[2] & DS_EDGE_BUTTONS_FN2);
+ input_report_key(ds->gamepad, BTN_TRIGGER_HAPPY3,
+ ds_report->buttons[2] & DS_EDGE_BUTTONS_LEFT_PADDLE);
+ input_report_key(ds->gamepad, BTN_TRIGGER_HAPPY4,
+ ds_report->buttons[2] & DS_EDGE_BUTTONS_RIGHT_PADDLE);
+ }
+
input_sync(ds->gamepad);
/*
@@ -1785,6 +1806,7 @@ static struct ps_device *dualsense_create(struct hid_device *hdev)
ds->use_vibration_v2 = ds->update_version >= DS_FEATURE_VERSION(2, 21);
} else if (hdev->product == USB_DEVICE_ID_SONY_PS5_CONTROLLER_2) {
ds->use_vibration_v2 = true;
+ ds->is_edge = true;
}
ret = ps_devices_list_add(ps_dev);
@@ -1802,6 +1824,15 @@ static struct ps_device *dualsense_create(struct hid_device *hdev)
ret = PTR_ERR(ds->gamepad);
goto err;
}
+
+ /* Register DualSense Edge back paddle and Fn buttons. */
+ if (ds->is_edge) {
+ input_set_capability(ds->gamepad, EV_KEY, BTN_TRIGGER_HAPPY1);
+ input_set_capability(ds->gamepad, EV_KEY, BTN_TRIGGER_HAPPY2);
+ input_set_capability(ds->gamepad, EV_KEY, BTN_TRIGGER_HAPPY3);
+ input_set_capability(ds->gamepad, EV_KEY, BTN_TRIGGER_HAPPY4);
+ }
+
/* Use gamepad input device name as primary device name for e.g. LEDs */
ps_dev->input_dev_name = dev_name(&ds->gamepad->dev);
--
2.47.3
^ permalink raw reply related
* Re: [PATCH] Input: gpio-keys - add hibernation support
From: Dmitry Torokhov @ 2026-04-07 4:31 UTC (permalink / raw)
To: Armando De Leon; +Cc: Armando De Leon, linux-input, linux-kernel
In-Reply-To: <20260406203932.3391600-1-learmand@amazon.com>
Hi Armando,
On Mon, Apr 06, 2026 at 01:39:10PM -0700, Armando De Leon wrote:
> Hi Dmitry,
>
> Thanks for the guidance. I investigated further:
>
> The upstream GICv3 driver (irq-gic-v3.c) has no suspend/resume or
> syscore_ops for the distributor - it does not save or restore any
> per-IRQ configuration across power cycles. On platforms where the
> GIC is re-initialized by firmware during hibernate restore, all
> trigger type configurations set by consumer drivers during probe()
> are lost.
>
> The IRQ core does preserve the trigger type in irq_data
> (irqd_get_trigger_type), but resume_irq() in kernel/irq/pm.c only
> re-enables the interrupt without re-applying the trigger type to
> hardware.
>
> A fix in the IRQ PM resume path (having resume_irq() call
> irq_set_irq_type() using the saved trigger type) would fix all
> consumer drivers generically. However, such a change would have
> broad consequences and impact across all platforms and interrupt
> controllers, and would need very careful review and testing.
>
> Given that the gpio-keys fix is a single irq_set_irq_type() call
> in the .restore callback - minimal, self-contained, and low risk -
> would it be acceptable to handle it at the driver level for now?
> This avoids the risk of a sweeping IRQ core change while solving
> the immediate problem for gpio-keys users with hibernation support.
I am sorry but as I mentioned, the proper fix is in IRQ/irqchip code,
which will ensure that not only gpio-keys but also the rest of the
consumer drivers have consistent state post hibernate.
I understand that you might need your change for a product; I recommend
applying the patch to your internal tree while you are sorting out a
generic solution.
Thanks.
--
Dmitry
^ permalink raw reply
* [PATCH v2 5/5] HID: hid-oxp: Add Vibration Intensity Attributes
From: Derek J. Clark @ 2026-04-07 4:13 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: Pierre-Loup A . Griffais, Lambert Fan, Derek J . Clark,
linux-input, linux-doc, linux-kernel
In-Reply-To: <20260407041354.2283201-1-derekjohn.clark@gmail.com>
Adds attribute for setting the rumble intensity level. This setting must
be re-applied after the gamepad mode is set as doing so resets this to
the default value.
Signed-off-by: Derek J. Clark <derekjohn.clark@gmail.com>
---
drivers/hid/hid-oxp.c | 80 +++++++++++++++++++++++++++++++++++++++++--
1 file changed, 78 insertions(+), 2 deletions(-)
diff --git a/drivers/hid/hid-oxp.c b/drivers/hid/hid-oxp.c
index 1100f1f14f35..cad6973089a0 100644
--- a/drivers/hid/hid-oxp.c
+++ b/drivers/hid/hid-oxp.c
@@ -34,6 +34,7 @@ enum oxp_function_index {
OXP_FID_GEN1_RGB_SET = 0x07,
OXP_FID_GEN1_RGB_REPLY = 0x0f,
OXP_FID_GEN2_TOGGLE_MODE = 0xb2,
+ OXP_FID_GEN2_RUMBLE_SET = 0xb3,
OXP_FID_GEN2_KEY_STATE = 0xb4,
OXP_FID_GEN2_STATUS_EVENT = 0xb8,
};
@@ -178,6 +179,7 @@ static struct oxp_hid_cfg {
struct mutex cfg_mutex; /*ensure single synchronous output report*/
u8 rgb_brightness;
u8 gamepad_mode;
+ u8 rumble_intensity;
u8 rgb_effect;
u8 rgb_speed;
u8 rgb_en;
@@ -263,6 +265,11 @@ static const char *const oxp_rgb_effect_text[] = {
[OXP_EFFECT_MONO_LIST] = "monocolor",
};
+enum oxp_rumble_side_index {
+ OXP_RUMBLE_LEFT = 0x00,
+ OXP_RUMBLE_RIGHT,
+};
+
struct oxp_gen_1_rgb_report {
u8 report_id;
u8 message_id;
@@ -338,6 +345,7 @@ static int oxp_hid_raw_event_gen_1(struct hid_device *hdev,
static int oxp_gen_2_property_out(enum oxp_function_index fid, u8 *data, u8 data_size);
static int oxp_set_buttons(void);
+static int oxp_rumble_intensity_set(u8 intensity);
static void oxp_mcu_init_fn(struct work_struct *work)
{
@@ -365,6 +373,12 @@ static void oxp_mcu_init_fn(struct work_struct *work)
if (ret)
dev_err(&drvdata.hdev->dev,
"Error: Failed to set gamepad mode: %i\n", ret);
+
+ /* Set vibration level */
+ ret = oxp_rumble_intensity_set(drvdata.rumble_intensity);
+ if (ret)
+ dev_err(&drvdata.hdev->dev,
+ "Error: Failed to set rumble intensity: %i\n", ret);
}
static DECLARE_DELAYED_WORK(oxp_mcu_init, oxp_mcu_init_fn);
@@ -513,6 +527,14 @@ static ssize_t gamepad_mode_store(struct device *dev,
drvdata.gamepad_mode = data[0];
+ if (drvdata.gamepad_mode == OXP_GP_MODE_DEBUG)
+ return count;
+
+ /* Re-apply rumble settings as switching gamepad mode will override */
+ ret = oxp_rumble_intensity_set(drvdata.rumble_intensity);
+ if (ret)
+ return ret;
+
return count;
}
@@ -858,6 +880,59 @@ static ssize_t button_mapping_options_show(struct device *dev,
}
static DEVICE_ATTR_RO(button_mapping_options);
+static int oxp_rumble_intensity_set(u8 intensity)
+{
+ u8 header[15] = { 0x02, 0x38, 0x02, 0xe3, 0x39, 0xe3, 0x39, 0xe3,
+ 0x39, 0x01, intensity, 0x05, 0xe3, 0x39, 0xe3 };
+ u8 footer[9] = { 0x39, 0xe3, 0x39, 0xe3, 0xe3, 0x02, 0x04, 0x39, 0x39 };
+ size_t footer_size = ARRAY_SIZE(footer);
+ size_t header_size = ARRAY_SIZE(header);
+ u8 data[59] = { 0x0 };
+ size_t data_size = ARRAY_SIZE(data);
+
+ memcpy(data, header, header_size);
+ memcpy(data + data_size - footer_size, footer, footer_size);
+
+ return oxp_gen_2_property_out(OXP_FID_GEN2_RUMBLE_SET, data, data_size);
+}
+
+static ssize_t rumble_intensity_store(struct device *dev,
+ struct device_attribute *attr, const char *buf,
+ size_t count)
+{
+ int ret;
+ u8 val;
+
+ ret = kstrtou8(buf, 10, &val);
+ if (ret)
+ return ret;
+
+ if (val < 0 || val > 5)
+ return -EINVAL;
+
+ ret = oxp_rumble_intensity_set(val);
+ if (ret)
+ return ret;
+
+ drvdata.rumble_intensity = val;
+
+ return count;
+}
+
+static ssize_t rumble_intensity_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ return sysfs_emit(buf, "%i\n", drvdata.rumble_intensity);
+}
+static DEVICE_ATTR_RW(rumble_intensity);
+
+static ssize_t rumble_intensity_range_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ return sysfs_emit(buf, "0-5\n");
+}
+static DEVICE_ATTR_RO(rumble_intensity_range);
+
#define OXP_DEVICE_ATTR_RW(_name, _group) \
static ssize_t _name##_store(struct device *dev, \
struct device_attribute *attr, \
@@ -949,6 +1024,8 @@ static struct attribute *oxp_cfg_attrs[] = {
&dev_attr_gamepad_mode.attr,
&dev_attr_gamepad_mode_index.attr,
&dev_attr_reset_buttons.attr,
+ &dev_attr_rumble_intensity.attr,
+ &dev_attr_rumble_intensity_range.attr,
NULL,
};
@@ -1420,10 +1497,9 @@ static int oxp_cfg_probe(struct hid_device *hdev, u16 up)
drvdata.bmap_1 = bmap_1;
drvdata.bmap_2 = bmap_2;
+ drvdata.rumble_intensity = 5;
mod_delayed_work(system_wq, &oxp_mcu_init, msecs_to_jiffies(50));
- drvdata.gamepad_mode = OXP_GP_MODE_XINPUT;
-
ret = devm_device_add_group(&hdev->dev, &oxp_cfg_attrs_group);
if (ret)
return dev_err_probe(&hdev->dev, ret,
--
2.53.0
^ permalink raw reply related
* [PATCH v2 4/5] HID: hid-oxp: Add Button Mapping Interface
From: Derek J. Clark @ 2026-04-07 4:13 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: Pierre-Loup A . Griffais, Lambert Fan, Derek J . Clark,
linux-input, linux-doc, linux-kernel
In-Reply-To: <20260407041354.2283201-1-derekjohn.clark@gmail.com>
Adds button mapping interface for second generation OneXPlayer
configuration HID interfaces. This interface allows the MCU to swap
button mappings at the hardware level. The current state cannot be
retrieved, and the mappings may have been modified in Windows prior, so
we reset the button mapping at init and expose an attribute to allow
userspace to do this again at any time.
The interface requires two pages of button mapping data to be sent
before the settings will take place. Since the MCU requires a 200ms
delay after each message (total 400ms for these attributes) use the same
debounce work queue method we used for RGB. This will allow for
userspace or udev rules to rapidly map all buttons. The values will
be cached before the final write is finally sent to the device.
Signed-off-by: Derek J. Clark <derekjohn.clark@gmail.com>
---
v2:
- Add detection of post-suspend MCU init to trigger setting the button
map again.
---
drivers/hid/hid-oxp.c | 565 ++++++++++++++++++++++++++++++++++++++++++
1 file changed, 565 insertions(+)
diff --git a/drivers/hid/hid-oxp.c b/drivers/hid/hid-oxp.c
index c62952537d98..1100f1f14f35 100644
--- a/drivers/hid/hid-oxp.c
+++ b/drivers/hid/hid-oxp.c
@@ -34,10 +34,145 @@ enum oxp_function_index {
OXP_FID_GEN1_RGB_SET = 0x07,
OXP_FID_GEN1_RGB_REPLY = 0x0f,
OXP_FID_GEN2_TOGGLE_MODE = 0xb2,
+ OXP_FID_GEN2_KEY_STATE = 0xb4,
OXP_FID_GEN2_STATUS_EVENT = 0xb8,
};
+#define OXP_MAPPING_GAMEPAD 0x01
+#define OXP_MAPPING_KEYBOARD 0x02
+
+struct oxp_button_data {
+ u8 mode;
+ u8 index;
+ u8 key_id;
+ u8 padding[2];
+} __packed;
+
+struct oxp_button_entry {
+ struct oxp_button_data data;
+ const char *name;
+};
+
+static const struct oxp_button_entry oxp_button_table[] = {
+ /* Gamepad Buttons */
+ { { OXP_MAPPING_GAMEPAD, 0x01 }, "BTN_A" },
+ { { OXP_MAPPING_GAMEPAD, 0x02 }, "BTN_B" },
+ { { OXP_MAPPING_GAMEPAD, 0x03 }, "BTN_X" },
+ { { OXP_MAPPING_GAMEPAD, 0x04 }, "BTN_Y" },
+ { { OXP_MAPPING_GAMEPAD, 0x05 }, "BTN_LB" },
+ { { OXP_MAPPING_GAMEPAD, 0x06 }, "BTN_RB" },
+ { { OXP_MAPPING_GAMEPAD, 0x07 }, "BTN_LT" },
+ { { OXP_MAPPING_GAMEPAD, 0x08 }, "BTN_RT" },
+ { { OXP_MAPPING_GAMEPAD, 0x09 }, "BTN_START" },
+ { { OXP_MAPPING_GAMEPAD, 0x0a }, "BTN_SELECT" },
+ { { OXP_MAPPING_GAMEPAD, 0x0b }, "BTN_L3" },
+ { { OXP_MAPPING_GAMEPAD, 0x0c }, "BTN_R3" },
+ { { OXP_MAPPING_GAMEPAD, 0x0d }, "DPAD_UP" },
+ { { OXP_MAPPING_GAMEPAD, 0x0e }, "DPAD_DOWN" },
+ { { OXP_MAPPING_GAMEPAD, 0x0f }, "DPAD_LEFT" },
+ { { OXP_MAPPING_GAMEPAD, 0x10 }, "DPAD_RIGHT" },
+ { { OXP_MAPPING_GAMEPAD, 0x11 }, "JOY_L_UP" },
+ { { OXP_MAPPING_GAMEPAD, 0x12 }, "JOY_L_UP_RIGHT" },
+ { { OXP_MAPPING_GAMEPAD, 0x13 }, "JOY_L_RIGHT" },
+ { { OXP_MAPPING_GAMEPAD, 0x14 }, "JOY_L_DOWN_RIGHT" },
+ { { OXP_MAPPING_GAMEPAD, 0x15 }, "JOY_L_DOWN" },
+ { { OXP_MAPPING_GAMEPAD, 0x16 }, "JOY_L_DOWN_LEFT" },
+ { { OXP_MAPPING_GAMEPAD, 0x17 }, "JOY_L_LEFT" },
+ { { OXP_MAPPING_GAMEPAD, 0x18 }, "JOY_L_UP_LEFT" },
+ { { OXP_MAPPING_GAMEPAD, 0x19 }, "JOY_R_UP" },
+ { { OXP_MAPPING_GAMEPAD, 0x1a }, "JOY_R_UP_RIGHT" },
+ { { OXP_MAPPING_GAMEPAD, 0x1b }, "JOY_R_RIGHT" },
+ { { OXP_MAPPING_GAMEPAD, 0x1c }, "JOY_R_DOWN_RIGHT" },
+ { { OXP_MAPPING_GAMEPAD, 0x1d }, "JOY_R_DOWN" },
+ { { OXP_MAPPING_GAMEPAD, 0x1e }, "JOY_R_DOWN_LEFT" },
+ { { OXP_MAPPING_GAMEPAD, 0x1f }, "JOY_R_LEFT" },
+ { { OXP_MAPPING_GAMEPAD, 0x20 }, "JOY_R_UP_LEFT" },
+ { { OXP_MAPPING_GAMEPAD, 0x22 }, "BTN_GUIDE" },
+ /* Keyboard Keys */
+ { { OXP_MAPPING_KEYBOARD, 0x01, 0x5a }, "KEY_F1" },
+ { { OXP_MAPPING_KEYBOARD, 0x01, 0x5b }, "KEY_F2" },
+ { { OXP_MAPPING_KEYBOARD, 0x01, 0x5c }, "KEY_F3" },
+ { { OXP_MAPPING_KEYBOARD, 0x01, 0x5d }, "KEY_F4" },
+ { { OXP_MAPPING_KEYBOARD, 0x01, 0x5e }, "KEY_F5" },
+ { { OXP_MAPPING_KEYBOARD, 0x01, 0x5f }, "KEY_F6" },
+ { { OXP_MAPPING_KEYBOARD, 0x01, 0x60 }, "KEY_F7" },
+ { { OXP_MAPPING_KEYBOARD, 0x01, 0x61 }, "KEY_F8" },
+ { { OXP_MAPPING_KEYBOARD, 0x01, 0x62 }, "KEY_F9" },
+ { { OXP_MAPPING_KEYBOARD, 0x01, 0x63 }, "KEY_F10" },
+ { { OXP_MAPPING_KEYBOARD, 0x01, 0x64 }, "KEY_F11" },
+ { { OXP_MAPPING_KEYBOARD, 0x01, 0x65 }, "KEY_F12" },
+ { { OXP_MAPPING_KEYBOARD, 0x01, 0x66 }, "KEY_F13" },
+ { { OXP_MAPPING_KEYBOARD, 0x01, 0x67 }, "KEY_F14" },
+ { { OXP_MAPPING_KEYBOARD, 0x01, 0x68 }, "KEY_F15" },
+ { { OXP_MAPPING_KEYBOARD, 0x01, 0x69 }, "KEY_F16" },
+ { { OXP_MAPPING_KEYBOARD, 0x01, 0x6a }, "KEY_F17" },
+ { { OXP_MAPPING_KEYBOARD, 0x01, 0x6b }, "KEY_F18" },
+ { { OXP_MAPPING_KEYBOARD, 0x01, 0x6c }, "KEY_F19" },
+ { { OXP_MAPPING_KEYBOARD, 0x01, 0x6d }, "KEY_F20" },
+ { { OXP_MAPPING_KEYBOARD, 0x01, 0x6e }, "KEY_F21" },
+ { { OXP_MAPPING_KEYBOARD, 0x01, 0x6f }, "KEY_F22" },
+ { { OXP_MAPPING_KEYBOARD, 0x01, 0x70 }, "KEY_F23" },
+ { { OXP_MAPPING_KEYBOARD, 0x01, 0x71 }, "KEY_F24" },
+};
+
+enum oxp_joybutton_index {
+ BUTTON_A = 0x01,
+ BUTTON_B,
+ BUTTON_X,
+ BUTTON_Y,
+ BUTTON_LB,
+ BUTTON_RB,
+ BUTTON_LT,
+ BUTTON_RT,
+ BUTTON_START,
+ BUTTON_SELECT,
+ BUTTON_L3,
+ BUTTON_R3,
+ BUTTON_DUP,
+ BUTTON_DDOWN,
+ BUTTON_DLEFT,
+ BUTTON_DRIGHT,
+ BUTTON_M1 = 0x22,
+ BUTTON_M2,
+ /* These are unused currently, reserved for future devices */
+ BUTTON_M3,
+ BUTTON_M4,
+ BUTTON_M5,
+ BUTTON_M6,
+};
+
+struct oxp_button_idx {
+ enum oxp_joybutton_index button_idx;
+ u8 mapping_idx;
+} __packed;
+
+struct oxp_bmap_page_1 {
+ struct oxp_button_idx btn_a;
+ struct oxp_button_idx btn_b;
+ struct oxp_button_idx btn_x;
+ struct oxp_button_idx btn_y;
+ struct oxp_button_idx btn_lb;
+ struct oxp_button_idx btn_rb;
+ struct oxp_button_idx btn_lt;
+ struct oxp_button_idx btn_rt;
+ struct oxp_button_idx btn_start;
+} __packed;
+
+struct oxp_bmap_page_2 {
+ struct oxp_button_idx btn_select;
+ struct oxp_button_idx btn_l3;
+ struct oxp_button_idx btn_r3;
+ struct oxp_button_idx btn_dup;
+ struct oxp_button_idx btn_ddown;
+ struct oxp_button_idx btn_dleft;
+ struct oxp_button_idx btn_dright;
+ struct oxp_button_idx btn_m1;
+ struct oxp_button_idx btn_m2;
+} __packed;
+
static struct oxp_hid_cfg {
+ struct oxp_bmap_page_1 *bmap_1;
+ struct oxp_bmap_page_2 *bmap_2;
struct led_classdev_mc *led_mc;
struct hid_device *hdev;
struct mutex cfg_mutex; /*ensure single synchronous output report*/
@@ -48,6 +183,10 @@ static struct oxp_hid_cfg {
u8 rgb_en;
} drvdata;
+#define OXP_FILL_PAGE_SLOT(page, btn) \
+ { .button_idx = (page)->btn.button_idx, \
+ .mapping_idx = (page)->btn.mapping_idx }
+
enum oxp_gamepad_mode_index {
OXP_GP_MODE_XINPUT = 0x00,
OXP_GP_MODE_DEBUG = 0x03,
@@ -153,6 +292,10 @@ struct oxp_gen_2_rgb_report {
u8 effect;
} __packed;
+struct oxp_attr {
+ u8 index;
+};
+
static u16 get_usage_page(struct hid_device *hdev)
{
return hdev->collection[0].usage >> 16;
@@ -194,12 +337,19 @@ static int oxp_hid_raw_event_gen_1(struct hid_device *hdev,
}
static int oxp_gen_2_property_out(enum oxp_function_index fid, u8 *data, u8 data_size);
+static int oxp_set_buttons(void);
static void oxp_mcu_init_fn(struct work_struct *work)
{
u8 gp_mode_data[3] = { OXP_GP_MODE_DEBUG, 0x01, 0x02 };
int ret;
+ /* Re-apply the button mapping */
+ ret = oxp_set_buttons();
+ if (ret)
+ dev_err(&drvdata.hdev->dev,
+ "Error: Failed to set button mapping: %i\n", ret);
+
/* Cycle the gamepad mode */
ret = oxp_gen_2_property_out(OXP_FID_GEN2_TOGGLE_MODE, gp_mode_data, 3);
if (ret)
@@ -395,9 +545,410 @@ static ssize_t gamepad_mode_index_show(struct device *dev,
}
static DEVICE_ATTR_RO(gamepad_mode_index);
+static void oxp_set_defaults_bmap_1(struct oxp_bmap_page_1 *bmap)
+{
+ bmap->btn_a.button_idx = BUTTON_A;
+ bmap->btn_a.mapping_idx = 0;
+ bmap->btn_b.button_idx = BUTTON_B;
+ bmap->btn_b.mapping_idx = 1;
+ bmap->btn_x.button_idx = BUTTON_X;
+ bmap->btn_x.mapping_idx = 2;
+ bmap->btn_y.button_idx = BUTTON_Y;
+ bmap->btn_y.mapping_idx = 3;
+ bmap->btn_lb.button_idx = BUTTON_LB;
+ bmap->btn_lb.mapping_idx = 4;
+ bmap->btn_rb.button_idx = BUTTON_RB;
+ bmap->btn_rb.mapping_idx = 5;
+ bmap->btn_lt.button_idx = BUTTON_LT;
+ bmap->btn_lt.mapping_idx = 6;
+ bmap->btn_rt.button_idx = BUTTON_RT;
+ bmap->btn_rt.mapping_idx = 7;
+ bmap->btn_start.button_idx = BUTTON_START;
+ bmap->btn_start.mapping_idx = 8;
+}
+
+static void oxp_set_defaults_bmap_2(struct oxp_bmap_page_2 *bmap)
+{
+ bmap->btn_select.button_idx = BUTTON_SELECT;
+ bmap->btn_select.mapping_idx = 9;
+ bmap->btn_l3.button_idx = BUTTON_L3;
+ bmap->btn_l3.mapping_idx = 10;
+ bmap->btn_r3.button_idx = BUTTON_R3;
+ bmap->btn_r3.mapping_idx = 11;
+ bmap->btn_dup.button_idx = BUTTON_DUP;
+ bmap->btn_dup.mapping_idx = 12;
+ bmap->btn_ddown.button_idx = BUTTON_DDOWN;
+ bmap->btn_ddown.mapping_idx = 13;
+ bmap->btn_dleft.button_idx = BUTTON_DLEFT;
+ bmap->btn_dleft.mapping_idx = 14;
+ bmap->btn_dright.button_idx = BUTTON_DRIGHT;
+ bmap->btn_dright.mapping_idx = 15;
+ bmap->btn_m1.button_idx = BUTTON_M1;
+ bmap->btn_m1.mapping_idx = 48; /* KEY_F15 */
+ bmap->btn_m2.button_idx = BUTTON_M2;
+ bmap->btn_m2.mapping_idx = 49; /* KEY_F16 */
+}
+
+static void oxp_page_fill_data(char *buf, const struct oxp_button_idx *buttons,
+ size_t len)
+{
+ size_t offset_increment = sizeof(u8) + sizeof(struct oxp_button_idx);
+ size_t offset = 5;
+ unsigned int i;
+
+ for (i = 0; i < len; i++, offset += offset_increment) {
+ buf[offset] = (u8)buttons[i].button_idx;
+ memcpy(buf + offset + 1,
+ &oxp_button_table[buttons[i].mapping_idx].data,
+ sizeof(struct oxp_button_data));
+ }
+}
+
+static int oxp_set_buttons(void)
+{
+ u8 page_1[59] = { 0x02, 0x38, 0x20, 0x01, 0x01 };
+ u8 page_2[59] = { 0x02, 0x38, 0x20, 0x02, 0x01 };
+ u16 up = get_usage_page(drvdata.hdev);
+ int ret;
+
+ if (up != GEN2_USAGE_PAGE)
+ return -EINVAL;
+
+ const struct oxp_button_idx p1[] = {
+ OXP_FILL_PAGE_SLOT(drvdata.bmap_1, btn_a),
+ OXP_FILL_PAGE_SLOT(drvdata.bmap_1, btn_b),
+ OXP_FILL_PAGE_SLOT(drvdata.bmap_1, btn_x),
+ OXP_FILL_PAGE_SLOT(drvdata.bmap_1, btn_y),
+ OXP_FILL_PAGE_SLOT(drvdata.bmap_1, btn_lb),
+ OXP_FILL_PAGE_SLOT(drvdata.bmap_1, btn_rb),
+ OXP_FILL_PAGE_SLOT(drvdata.bmap_1, btn_lt),
+ OXP_FILL_PAGE_SLOT(drvdata.bmap_1, btn_rt),
+ OXP_FILL_PAGE_SLOT(drvdata.bmap_1, btn_start),
+ };
+
+ const struct oxp_button_idx p2[] = {
+ OXP_FILL_PAGE_SLOT(drvdata.bmap_2, btn_select),
+ OXP_FILL_PAGE_SLOT(drvdata.bmap_2, btn_l3),
+ OXP_FILL_PAGE_SLOT(drvdata.bmap_2, btn_r3),
+ OXP_FILL_PAGE_SLOT(drvdata.bmap_2, btn_dup),
+ OXP_FILL_PAGE_SLOT(drvdata.bmap_2, btn_ddown),
+ OXP_FILL_PAGE_SLOT(drvdata.bmap_2, btn_dleft),
+ OXP_FILL_PAGE_SLOT(drvdata.bmap_2, btn_dright),
+ OXP_FILL_PAGE_SLOT(drvdata.bmap_2, btn_m1),
+ OXP_FILL_PAGE_SLOT(drvdata.bmap_2, btn_m2),
+ };
+
+ oxp_page_fill_data(page_1, p1, ARRAY_SIZE(p1));
+ oxp_page_fill_data(page_2, p2, ARRAY_SIZE(p2));
+
+ ret = oxp_gen_2_property_out(OXP_FID_GEN2_KEY_STATE, page_1, ARRAY_SIZE(page_1));
+ if (ret)
+ return ret;
+
+ return oxp_gen_2_property_out(OXP_FID_GEN2_KEY_STATE, page_2, ARRAY_SIZE(page_2));
+}
+
+static int oxp_reset_buttons(void)
+{
+ oxp_set_defaults_bmap_1(drvdata.bmap_1);
+ oxp_set_defaults_bmap_2(drvdata.bmap_2);
+ return oxp_set_buttons();
+}
+
+static ssize_t reset_buttons_store(struct device *dev,
+ struct device_attribute *attr, const char *buf,
+ size_t count)
+{
+ int val, ret;
+
+ ret = kstrtoint(buf, 10, &val);
+ if (ret)
+ return ret;
+
+ if (val != 1)
+ return -EINVAL;
+
+ ret = oxp_reset_buttons();
+ if (ret)
+ return ret;
+
+ return count;
+}
+static DEVICE_ATTR_WO(reset_buttons);
+
+static void oxp_btn_queue_fn(struct work_struct *work)
+{
+ int ret;
+
+ ret = oxp_set_buttons();
+ if (ret)
+ dev_err(&drvdata.hdev->dev,
+ "Error: Failed to write button mapping: %i\n", ret);
+}
+
+static DECLARE_DELAYED_WORK(oxp_btn_queue, oxp_btn_queue_fn);
+
+static int oxp_button_idx_from_str(const char *buf)
+{
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(oxp_button_table); i++)
+ if (sysfs_streq(buf, oxp_button_table[i].name))
+ return i;
+
+ return -EINVAL;
+}
+
+static ssize_t map_button_store(struct device *dev,
+ struct device_attribute *attr, const char *buf,
+ size_t count, u8 index)
+{
+ int idx;
+
+ idx = oxp_button_idx_from_str(buf);
+ if (idx < 0)
+ return idx;
+
+ switch (index) {
+ case BUTTON_A:
+ drvdata.bmap_1->btn_a.mapping_idx = idx;
+ break;
+ case BUTTON_B:
+ drvdata.bmap_1->btn_b.mapping_idx = idx;
+ break;
+ case BUTTON_X:
+ drvdata.bmap_1->btn_x.mapping_idx = idx;
+ break;
+ case BUTTON_Y:
+ drvdata.bmap_1->btn_y.mapping_idx = idx;
+ break;
+ case BUTTON_LB:
+ drvdata.bmap_1->btn_lb.mapping_idx = idx;
+ break;
+ case BUTTON_RB:
+ drvdata.bmap_1->btn_rb.mapping_idx = idx;
+ break;
+ case BUTTON_LT:
+ drvdata.bmap_1->btn_lt.mapping_idx = idx;
+ break;
+ case BUTTON_RT:
+ drvdata.bmap_1->btn_rt.mapping_idx = idx;
+ break;
+ case BUTTON_START:
+ drvdata.bmap_1->btn_start.mapping_idx = idx;
+ break;
+ case BUTTON_SELECT:
+ drvdata.bmap_2->btn_select.mapping_idx = idx;
+ break;
+ case BUTTON_L3:
+ drvdata.bmap_2->btn_l3.mapping_idx = idx;
+ break;
+ case BUTTON_R3:
+ drvdata.bmap_2->btn_r3.mapping_idx = idx;
+ break;
+ case BUTTON_DUP:
+ drvdata.bmap_2->btn_dup.mapping_idx = idx;
+ break;
+ case BUTTON_DDOWN:
+ drvdata.bmap_2->btn_ddown.mapping_idx = idx;
+ break;
+ case BUTTON_DLEFT:
+ drvdata.bmap_2->btn_dleft.mapping_idx = idx;
+ break;
+ case BUTTON_DRIGHT:
+ drvdata.bmap_2->btn_dright.mapping_idx = idx;
+ break;
+ case BUTTON_M1:
+ drvdata.bmap_2->btn_m1.mapping_idx = idx;
+ break;
+ case BUTTON_M2:
+ drvdata.bmap_2->btn_m2.mapping_idx = idx;
+ break;
+ default:
+ return -EINVAL;
+ }
+ mod_delayed_work(system_wq, &oxp_btn_queue, msecs_to_jiffies(50));
+ return count;
+}
+
+static ssize_t map_button_show(struct device *dev,
+ struct device_attribute *attr, char *buf,
+ u8 index)
+{
+ u8 i;
+
+ switch (index) {
+ case BUTTON_A:
+ i = drvdata.bmap_1->btn_a.mapping_idx;
+ break;
+ case BUTTON_B:
+ i = drvdata.bmap_1->btn_b.mapping_idx;
+ break;
+ case BUTTON_X:
+ i = drvdata.bmap_1->btn_x.mapping_idx;
+ break;
+ case BUTTON_Y:
+ i = drvdata.bmap_1->btn_y.mapping_idx;
+ break;
+ case BUTTON_LB:
+ i = drvdata.bmap_1->btn_lb.mapping_idx;
+ break;
+ case BUTTON_RB:
+ i = drvdata.bmap_1->btn_rb.mapping_idx;
+ break;
+ case BUTTON_LT:
+ i = drvdata.bmap_1->btn_lt.mapping_idx;
+ break;
+ case BUTTON_RT:
+ i = drvdata.bmap_1->btn_rt.mapping_idx;
+ break;
+ case BUTTON_START:
+ i = drvdata.bmap_1->btn_start.mapping_idx;
+ break;
+ case BUTTON_SELECT:
+ i = drvdata.bmap_2->btn_select.mapping_idx;
+ break;
+ case BUTTON_L3:
+ i = drvdata.bmap_2->btn_l3.mapping_idx;
+ break;
+ case BUTTON_R3:
+ i = drvdata.bmap_2->btn_r3.mapping_idx;
+ break;
+ case BUTTON_DUP:
+ i = drvdata.bmap_2->btn_dup.mapping_idx;
+ break;
+ case BUTTON_DDOWN:
+ i = drvdata.bmap_2->btn_ddown.mapping_idx;
+ break;
+ case BUTTON_DLEFT:
+ i = drvdata.bmap_2->btn_dleft.mapping_idx;
+ break;
+ case BUTTON_DRIGHT:
+ i = drvdata.bmap_2->btn_dright.mapping_idx;
+ break;
+ case BUTTON_M1:
+ i = drvdata.bmap_2->btn_m1.mapping_idx;
+ break;
+ case BUTTON_M2:
+ i = drvdata.bmap_2->btn_m2.mapping_idx;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ if (i >= ARRAY_SIZE(oxp_button_table))
+ return -EINVAL;
+
+ return sysfs_emit(buf, "%s\n", oxp_button_table[i].name);
+}
+
+static ssize_t button_mapping_options_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ ssize_t count = 0;
+ unsigned int i;
+
+ for (i = 0; i < ARRAY_SIZE(oxp_button_table); i++)
+ count += sysfs_emit_at(buf, count, "%s ", oxp_button_table[i].name);
+
+ if (count)
+ buf[count - 1] = '\n';
+
+ return count;
+}
+static DEVICE_ATTR_RO(button_mapping_options);
+
+#define OXP_DEVICE_ATTR_RW(_name, _group) \
+ static ssize_t _name##_store(struct device *dev, \
+ struct device_attribute *attr, \
+ const char *buf, size_t count) \
+ { \
+ return _group##_store(dev, attr, buf, count, _name.index); \
+ } \
+ static ssize_t _name##_show(struct device *dev, \
+ struct device_attribute *attr, char *buf) \
+ { \
+ return _group##_show(dev, attr, buf, _name.index); \
+ } \
+ static DEVICE_ATTR_RW(_name)
+
+static struct oxp_attr button_a = { BUTTON_A };
+OXP_DEVICE_ATTR_RW(button_a, map_button);
+
+static struct oxp_attr button_b = { BUTTON_B };
+OXP_DEVICE_ATTR_RW(button_b, map_button);
+
+static struct oxp_attr button_x = { BUTTON_X };
+OXP_DEVICE_ATTR_RW(button_x, map_button);
+
+static struct oxp_attr button_y = { BUTTON_Y };
+OXP_DEVICE_ATTR_RW(button_y, map_button);
+
+static struct oxp_attr button_lb = { BUTTON_LB };
+OXP_DEVICE_ATTR_RW(button_lb, map_button);
+
+static struct oxp_attr button_rb = { BUTTON_RB };
+OXP_DEVICE_ATTR_RW(button_rb, map_button);
+
+static struct oxp_attr button_lt = { BUTTON_LT };
+OXP_DEVICE_ATTR_RW(button_lt, map_button);
+
+static struct oxp_attr button_rt = { BUTTON_RT };
+OXP_DEVICE_ATTR_RW(button_rt, map_button);
+
+static struct oxp_attr button_start = { BUTTON_START };
+OXP_DEVICE_ATTR_RW(button_start, map_button);
+
+static struct oxp_attr button_select = { BUTTON_SELECT };
+OXP_DEVICE_ATTR_RW(button_select, map_button);
+
+static struct oxp_attr button_l3 = { BUTTON_L3 };
+OXP_DEVICE_ATTR_RW(button_l3, map_button);
+
+static struct oxp_attr button_r3 = { BUTTON_R3 };
+OXP_DEVICE_ATTR_RW(button_r3, map_button);
+
+static struct oxp_attr button_d_up = { BUTTON_DUP };
+OXP_DEVICE_ATTR_RW(button_d_up, map_button);
+
+static struct oxp_attr button_d_down = { BUTTON_DDOWN };
+OXP_DEVICE_ATTR_RW(button_d_down, map_button);
+
+static struct oxp_attr button_d_left = { BUTTON_DLEFT };
+OXP_DEVICE_ATTR_RW(button_d_left, map_button);
+
+static struct oxp_attr button_d_right = { BUTTON_DRIGHT };
+OXP_DEVICE_ATTR_RW(button_d_right, map_button);
+
+static struct oxp_attr button_m1 = { BUTTON_M1 };
+OXP_DEVICE_ATTR_RW(button_m1, map_button);
+
+static struct oxp_attr button_m2 = { BUTTON_M2 };
+OXP_DEVICE_ATTR_RW(button_m2, map_button);
+
static struct attribute *oxp_cfg_attrs[] = {
+ &dev_attr_button_a.attr,
+ &dev_attr_button_b.attr,
+ &dev_attr_button_d_down.attr,
+ &dev_attr_button_d_left.attr,
+ &dev_attr_button_d_right.attr,
+ &dev_attr_button_d_up.attr,
+ &dev_attr_button_l3.attr,
+ &dev_attr_button_lb.attr,
+ &dev_attr_button_lt.attr,
+ &dev_attr_button_m1.attr,
+ &dev_attr_button_m2.attr,
+ &dev_attr_button_mapping_options.attr,
+ &dev_attr_button_r3.attr,
+ &dev_attr_button_rb.attr,
+ &dev_attr_button_rt.attr,
+ &dev_attr_button_select.attr,
+ &dev_attr_button_start.attr,
+ &dev_attr_button_x.attr,
+ &dev_attr_button_y.attr,
&dev_attr_gamepad_mode.attr,
&dev_attr_gamepad_mode_index.attr,
+ &dev_attr_reset_buttons.attr,
NULL,
};
@@ -823,6 +1374,8 @@ static bool oxp_hybrid_mcu_device(void)
static int oxp_cfg_probe(struct hid_device *hdev, u16 up)
{
+ struct oxp_bmap_page_1 *bmap_1;
+ struct oxp_bmap_page_2 *bmap_2;
int ret;
hid_set_drvdata(hdev, &drvdata);
@@ -855,6 +1408,18 @@ static int oxp_cfg_probe(struct hid_device *hdev, u16 up)
return 0;
skip_rgb:
+ bmap_1 = devm_kzalloc(&hdev->dev, sizeof(struct oxp_bmap_page_1), GFP_KERNEL);
+ if (!bmap_1)
+ return dev_err_probe(&hdev->dev, -ENOMEM,
+ "Unable to allocate button map page 1\n");
+
+ bmap_2 = devm_kzalloc(&hdev->dev, sizeof(struct oxp_bmap_page_2), GFP_KERNEL);
+ if (!bmap_2)
+ return dev_err_probe(&hdev->dev, -ENOMEM,
+ "Unable to allocate button map page 2\n");
+
+ drvdata.bmap_1 = bmap_1;
+ drvdata.bmap_2 = bmap_2;
mod_delayed_work(system_wq, &oxp_mcu_init, msecs_to_jiffies(50));
drvdata.gamepad_mode = OXP_GP_MODE_XINPUT;
--
2.53.0
^ permalink raw reply related
* [PATCH v2 3/5] HID: hid-oxp: Add Second Generation Gamepad Mode Switch
From: Derek J. Clark @ 2026-04-07 4:13 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: Pierre-Loup A . Griffais, Lambert Fan, Derek J . Clark,
linux-input, linux-doc, linux-kernel
In-Reply-To: <20260407041354.2283201-1-derekjohn.clark@gmail.com>
Adds "gamepad_mode" attribute to second generation OneXPlayer
configuration HID devices. This attribute initiates a mode shift in the
device MCU that puts it into a state where all events are routed to an
hidraw interface instead of the xpad evdev interface. This allows for
debugging the hardware input mapping added in the next patch.
Signed-off-by: Derek J. Clark <derekjohn.clark@gmail.com>
---
v2:
- Rename to gamepad_mode & show relevant gamepad modes instead of
using a debug enable/disable paradigm, to match other drivers.
---
drivers/hid/hid-oxp.c | 130 ++++++++++++++++++++++++++++++++++++++++++
1 file changed, 130 insertions(+)
diff --git a/drivers/hid/hid-oxp.c b/drivers/hid/hid-oxp.c
index 25214356163e..c62952537d98 100644
--- a/drivers/hid/hid-oxp.c
+++ b/drivers/hid/hid-oxp.c
@@ -33,6 +33,7 @@
enum oxp_function_index {
OXP_FID_GEN1_RGB_SET = 0x07,
OXP_FID_GEN1_RGB_REPLY = 0x0f,
+ OXP_FID_GEN2_TOGGLE_MODE = 0xb2,
OXP_FID_GEN2_STATUS_EVENT = 0xb8,
};
@@ -41,11 +42,22 @@ static struct oxp_hid_cfg {
struct hid_device *hdev;
struct mutex cfg_mutex; /*ensure single synchronous output report*/
u8 rgb_brightness;
+ u8 gamepad_mode;
u8 rgb_effect;
u8 rgb_speed;
u8 rgb_en;
} drvdata;
+enum oxp_gamepad_mode_index {
+ OXP_GP_MODE_XINPUT = 0x00,
+ OXP_GP_MODE_DEBUG = 0x03,
+};
+
+static const char *const oxp_gamepad_mode_text[] = {
+ [OXP_GP_MODE_XINPUT] = "xinput",
+ [OXP_GP_MODE_DEBUG] = "debug",
+};
+
enum oxp_feature_en_index {
OXP_FEAT_DISABLED,
OXP_FEAT_ENABLED,
@@ -181,6 +193,32 @@ static int oxp_hid_raw_event_gen_1(struct hid_device *hdev,
return 0;
}
+static int oxp_gen_2_property_out(enum oxp_function_index fid, u8 *data, u8 data_size);
+
+static void oxp_mcu_init_fn(struct work_struct *work)
+{
+ u8 gp_mode_data[3] = { OXP_GP_MODE_DEBUG, 0x01, 0x02 };
+ int ret;
+
+ /* Cycle the gamepad mode */
+ ret = oxp_gen_2_property_out(OXP_FID_GEN2_TOGGLE_MODE, gp_mode_data, 3);
+ if (ret)
+ dev_err(&drvdata.hdev->dev,
+ "Error: Failed to set gamepad mode: %i\n", ret);
+
+ /* Remainder only applies for xinput mode */
+ if (drvdata.gamepad_mode == OXP_GP_MODE_DEBUG)
+ return;
+
+ gp_mode_data[0] = OXP_GP_MODE_XINPUT;
+ ret = oxp_gen_2_property_out(OXP_FID_GEN2_TOGGLE_MODE, gp_mode_data, 3);
+ if (ret)
+ dev_err(&drvdata.hdev->dev,
+ "Error: Failed to set gamepad mode: %i\n", ret);
+}
+
+static DECLARE_DELAYED_WORK(oxp_mcu_init, oxp_mcu_init_fn);
+
static int oxp_hid_raw_event_gen_2(struct hid_device *hdev,
struct hid_report *report, u8 *data,
int size)
@@ -191,6 +229,14 @@ static int oxp_hid_raw_event_gen_2(struct hid_device *hdev,
if (data[0] != OXP_FID_GEN2_STATUS_EVENT)
return 0;
+ /* Sent ~6s after resume event, indicating the MCU has fully reset.
+ * Re-apply our settings after this has been received.
+ */
+ if (data[3] == OXP_EFFECT_MONO_TRUE) {
+ mod_delayed_work(system_wq, &oxp_mcu_init, msecs_to_jiffies(50));
+ return 0;
+ }
+
if (data[3] != OXP_GET_PROPERTY)
return 0;
@@ -288,6 +334,77 @@ static int oxp_gen_2_property_out(enum oxp_function_index fid, u8 *data,
footer_size);
}
+static ssize_t gamepad_mode_store(struct device *dev,
+ struct device_attribute *attr, const char *buf,
+ size_t count)
+{
+ u16 up = get_usage_page(drvdata.hdev);
+ u8 data[3] = { 0x00, 0x01, 0x02 };
+ int ret = -EINVAL;
+ int i;
+
+ if (up != GEN2_USAGE_PAGE)
+ return ret;
+
+ for (i = 0; i < ARRAY_SIZE(oxp_gamepad_mode_text); i++) {
+ if (oxp_gamepad_mode_text[i] && sysfs_streq(buf, oxp_gamepad_mode_text[i])) {
+ ret = i;
+ break;
+ }
+ }
+ if (ret < 0)
+ return ret;
+
+ data[0] = ret;
+
+ ret = oxp_gen_2_property_out(OXP_FID_GEN2_TOGGLE_MODE, data, 3);
+ if (ret)
+ return ret;
+
+ drvdata.gamepad_mode = data[0];
+
+ return count;
+}
+
+static ssize_t gamepad_mode_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ return sysfs_emit(buf, "%s\n", oxp_gamepad_mode_text[drvdata.gamepad_mode]);
+}
+static DEVICE_ATTR_RW(gamepad_mode);
+
+static ssize_t gamepad_mode_index_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ ssize_t count = 0;
+ unsigned int i;
+
+ for (i = 0; i < ARRAY_SIZE(oxp_gamepad_mode_text); i++) {
+ if (!oxp_gamepad_mode_text[i] ||
+ oxp_gamepad_mode_text[i][0] == '\0')
+ continue;
+
+ count += sysfs_emit_at(buf, count, "%s ", oxp_gamepad_mode_text[i]);
+ }
+
+ if (count)
+ buf[count - 1] = '\n';
+
+ return count;
+}
+static DEVICE_ATTR_RO(gamepad_mode_index);
+
+static struct attribute *oxp_cfg_attrs[] = {
+ &dev_attr_gamepad_mode.attr,
+ &dev_attr_gamepad_mode_index.attr,
+ NULL,
+};
+
+static const struct attribute_group oxp_cfg_attrs_group = {
+ .attrs = oxp_cfg_attrs,
+};
+
static int oxp_rgb_status_store(u8 enabled, u8 speed, u8 brightness)
{
u16 up = get_usage_page(drvdata.hdev);
@@ -733,7 +850,20 @@ static int oxp_cfg_probe(struct hid_device *hdev, u16 up)
dev_warn(drvdata.led_mc->led_cdev.dev,
"Failed to query RGB initial state: %i\n", ret);
+ /* Below features are only implemented in gen 2 */
+ if (up != GEN2_USAGE_PAGE)
+ return 0;
+
skip_rgb:
+ mod_delayed_work(system_wq, &oxp_mcu_init, msecs_to_jiffies(50));
+
+ drvdata.gamepad_mode = OXP_GP_MODE_XINPUT;
+
+ ret = devm_device_add_group(&hdev->dev, &oxp_cfg_attrs_group);
+ if (ret)
+ return dev_err_probe(&hdev->dev, ret,
+ "Failed to attach configuration attributes\n");
+
return 0;
}
--
2.53.0
^ permalink raw reply related
* [PATCH v2 2/5] HID: hid-oxp: Add Second Generation RGB Control
From: Derek J. Clark @ 2026-04-07 4:13 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: Pierre-Loup A . Griffais, Lambert Fan, Derek J . Clark,
linux-input, linux-doc, linux-kernel
In-Reply-To: <20260407041354.2283201-1-derekjohn.clark@gmail.com>
Adds support for the second generation of RGB Control for OneXPlayer
devices. The interface mirrors the first generation, with some
differences to how messages are formatted.
Some devices have both a GEN1 MCU for RGB control and a GEN2 MCU for
button mapping. To avoid conflicts, quirk these devices to skip RGB
setup for the GEN2_USAGE_PAGE.
Signed-off-by: Derek J. Clark <derekjohn.clark@gmail.com>
---
v2:
- Add DMI quirks table.
---
drivers/hid/Kconfig | 1 +
drivers/hid/hid-ids.h | 3 +
drivers/hid/hid-oxp.c | 151 ++++++++++++++++++++++++++++++++++++++++++
3 files changed, 155 insertions(+)
diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index 2deaec9f467d..b779088b80b6 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -924,6 +924,7 @@ config HID_OXP
depends on USB_HID
depends on LEDS_CLASS
depends on LEDS_CLASS_MULTICOLOR
+ depends on DMI
help
Say Y here if you would like to enable support for OneXPlayer handheld
devices that come with RGB LED rings around the joysticks and macro buttons.
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index dcc5a3a70eaf..0d1ff879e959 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -1134,6 +1134,9 @@
#define USB_VENDOR_ID_CRSC 0x1a2c
#define USB_DEVICE_ID_ONEXPLAYER_GEN1 0xb001
+#define USB_VENDOR_ID_WCH 0x1a86
+#define USB_DEVICE_ID_ONEXPLAYER_GEN2 0xfe00
+
#define USB_VENDOR_ID_ONTRAK 0x0a07
#define USB_DEVICE_ID_ONTRAK_ADU100 0x0064
diff --git a/drivers/hid/hid-oxp.c b/drivers/hid/hid-oxp.c
index c4219ecd8d71..25214356163e 100644
--- a/drivers/hid/hid-oxp.c
+++ b/drivers/hid/hid-oxp.c
@@ -10,6 +10,7 @@
#include <linux/delay.h>
#include <linux/dev_printk.h>
#include <linux/device.h>
+#include <linux/dmi.h>
#include <linux/hid.h>
#include <linux/jiffies.h>
#include <linux/kstrtox.h>
@@ -24,12 +25,15 @@
#define OXP_PACKET_SIZE 64
#define GEN1_MESSAGE_ID 0xff
+#define GEN2_MESSAGE_ID 0x3f
#define GEN1_USAGE_PAGE 0xff01
+#define GEN2_USAGE_PAGE 0xff00
enum oxp_function_index {
OXP_FID_GEN1_RGB_SET = 0x07,
OXP_FID_GEN1_RGB_REPLY = 0x0f,
+ OXP_FID_GEN2_STATUS_EVENT = 0xb8,
};
static struct oxp_hid_cfg {
@@ -121,6 +125,22 @@ struct oxp_gen_1_rgb_report {
u8 blue;
} __packed;
+struct oxp_gen_2_rgb_report {
+ u8 report_id;
+ u8 header_id;
+ u8 padding_2;
+ u8 message_id;
+ u8 padding_4[2];
+ u8 enabled;
+ u8 speed;
+ u8 brightness;
+ u8 red;
+ u8 green;
+ u8 blue;
+ u8 padding_12[3];
+ u8 effect;
+} __packed;
+
static u16 get_usage_page(struct hid_device *hdev)
{
return hdev->collection[0].usage >> 16;
@@ -161,6 +181,44 @@ static int oxp_hid_raw_event_gen_1(struct hid_device *hdev,
return 0;
}
+static int oxp_hid_raw_event_gen_2(struct hid_device *hdev,
+ struct hid_report *report, u8 *data,
+ int size)
+{
+ struct led_classdev_mc *led_mc = drvdata.led_mc;
+ struct oxp_gen_2_rgb_report *rgb_rep;
+
+ if (data[0] != OXP_FID_GEN2_STATUS_EVENT)
+ return 0;
+
+ if (data[3] != OXP_GET_PROPERTY)
+ return 0;
+
+ rgb_rep = (struct oxp_gen_2_rgb_report *)data;
+ /* Ensure we save monocolor as the list value */
+ drvdata.rgb_effect = rgb_rep->effect == OXP_EFFECT_MONO_TRUE ?
+ OXP_EFFECT_MONO_LIST :
+ rgb_rep->effect;
+ drvdata.rgb_speed = rgb_rep->speed;
+ drvdata.rgb_en = rgb_rep->enabled == 0 ? OXP_FEAT_DISABLED :
+ OXP_FEAT_ENABLED;
+ drvdata.rgb_brightness = rgb_rep->brightness;
+ led_mc->led_cdev.brightness = rgb_rep->brightness / 4 *
+ led_mc->led_cdev.max_brightness;
+ /* If monocolor had less than 100% brightness on the previous boot,
+ * there will be no reliable way to determine the real intensity.
+ * Since intensity scaling is used with a hardware brightness set at max,
+ * our brightness will always look like 100%. Use the last set value to
+ * prevent successive boots from lowering the brightness further.
+ * Brightness will be "wrong" but the effect will remain the same visually.
+ */
+ led_mc->subled_info[0].intensity = rgb_rep->red;
+ led_mc->subled_info[1].intensity = rgb_rep->green;
+ led_mc->subled_info[2].intensity = rgb_rep->blue;
+
+ return 0;
+}
+
static int oxp_hid_raw_event(struct hid_device *hdev, struct hid_report *report,
u8 *data, int size)
{
@@ -171,6 +229,8 @@ static int oxp_hid_raw_event(struct hid_device *hdev, struct hid_report *report,
switch (up) {
case GEN1_USAGE_PAGE:
return oxp_hid_raw_event_gen_1(hdev, report, data, size);
+ case GEN2_USAGE_PAGE:
+ return oxp_hid_raw_event_gen_2(hdev, report, data, size);
default:
break;
}
@@ -216,6 +276,18 @@ static int oxp_gen_1_property_out(enum oxp_function_index fid, u8 *data,
return mcu_property_out(header, header_size, data, data_size, NULL, 0);
}
+static int oxp_gen_2_property_out(enum oxp_function_index fid, u8 *data,
+ u8 data_size)
+{
+ u8 header[] = { fid, GEN2_MESSAGE_ID, 0x01 };
+ u8 footer[] = { GEN2_MESSAGE_ID, fid };
+ size_t header_size = ARRAY_SIZE(header);
+ size_t footer_size = ARRAY_SIZE(footer);
+
+ return mcu_property_out(header, header_size, data, data_size, footer,
+ footer_size);
+}
+
static int oxp_rgb_status_store(u8 enabled, u8 speed, u8 brightness)
{
u16 up = get_usage_page(drvdata.hdev);
@@ -230,6 +302,11 @@ static int oxp_rgb_status_store(u8 enabled, u8 speed, u8 brightness)
if (drvdata.rgb_effect == OXP_EFFECT_MONO_LIST)
data[3] = 0x04;
return oxp_gen_1_property_out(OXP_FID_GEN1_RGB_SET, data, 4);
+ case GEN2_USAGE_PAGE:
+ data = (u8[6]) { OXP_SET_PROPERTY, 0x00, 0x02, enabled, speed, brightness };
+ if (drvdata.rgb_effect == OXP_EFFECT_MONO_LIST)
+ data[5] = 0x04;
+ return oxp_gen_2_property_out(OXP_FID_GEN2_STATUS_EVENT, data, 6);
default:
return -ENODEV;
}
@@ -244,6 +321,9 @@ static ssize_t oxp_rgb_status_show(void)
case GEN1_USAGE_PAGE:
data = (u8[1]) { OXP_GET_PROPERTY };
return oxp_gen_1_property_out(OXP_FID_GEN1_RGB_SET, data, 1);
+ case GEN2_USAGE_PAGE:
+ data = (u8[3]) { OXP_GET_PROPERTY, 0x00, 0x02 };
+ return oxp_gen_2_property_out(OXP_FID_GEN2_STATUS_EVENT, data, 3);
default:
return -ENODEV;
}
@@ -274,6 +354,16 @@ static int oxp_rgb_color_set(void)
data[3 * i + 3] = blue;
}
return oxp_gen_1_property_out(OXP_FID_GEN1_RGB_SET, data, size);
+ case GEN2_USAGE_PAGE:
+ size = 57;
+ data = (u8[57]) { OXP_EFFECT_MONO_TRUE, 0x00, 0x02 };
+
+ for (i = 1; i < size / 3; i++) {
+ data[3 * i] = red;
+ data[3 * i + 1] = green;
+ data[3 * i + 2] = blue;
+ }
+ return oxp_gen_2_property_out(OXP_FID_GEN2_STATUS_EVENT, data, size);
default:
return -ENODEV;
}
@@ -310,6 +400,10 @@ static int oxp_rgb_effect_set(u8 effect)
data = (u8[1]) { effect };
ret = oxp_gen_1_property_out(OXP_FID_GEN1_RGB_SET, data, 1);
break;
+ case GEN2_USAGE_PAGE:
+ data = (u8[3]) { effect, 0x00, 0x02 };
+ ret = oxp_gen_2_property_out(OXP_FID_GEN2_STATUS_EVENT, data, 3);
+ break;
default:
ret = -ENODEV;
}
@@ -560,6 +654,56 @@ static struct led_classdev_mc oxp_cdev_rgb = {
.subled_info = oxp_rgb_subled_info,
};
+struct quirk_entry {
+ bool hybrid_mcu;
+};
+
+static struct quirk_entry quirk_hybrid_mcu = {
+ .hybrid_mcu = true,
+};
+
+static const struct dmi_system_id oxp_hybrid_mcu_list[] = {
+ {
+ .ident = "OneXPlayer Apex",
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "ONE-NETBOOK"),
+ DMI_MATCH(DMI_PRODUCT_NAME, "ONEXPLAYER APEX"),
+ },
+ .driver_data = &quirk_hybrid_mcu,
+ },
+ {
+ .ident = "OneXPlayer G1 AMD",
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "ONE-NETBOOK"),
+ DMI_MATCH(DMI_PRODUCT_NAME, "ONEXPLAYER G1 A"),
+ },
+ .driver_data = &quirk_hybrid_mcu,
+ },
+ {
+ .ident = "OneXPlayer G1 Intel",
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "ONE-NETBOOK"),
+ DMI_MATCH(DMI_PRODUCT_NAME, "ONEXPLAYER G1 i"),
+ },
+ .driver_data = &quirk_hybrid_mcu,
+ },
+ {},
+};
+
+static bool oxp_hybrid_mcu_device(void)
+{
+ const struct dmi_system_id *dmi_id;
+ struct quirk_entry *quirks;
+
+ dmi_id = dmi_first_match(oxp_hybrid_mcu_list);
+ if (!dmi_id)
+ return false;
+
+ quirks = dmi_id->driver_data;
+
+ return quirks->hybrid_mcu;
+}
+
static int oxp_cfg_probe(struct hid_device *hdev, u16 up)
{
int ret;
@@ -567,6 +711,10 @@ static int oxp_cfg_probe(struct hid_device *hdev, u16 up)
hid_set_drvdata(hdev, &drvdata);
mutex_init(&drvdata.cfg_mutex);
drvdata.hdev = hdev;
+
+ if (up == GEN2_USAGE_PAGE && oxp_hybrid_mcu_device())
+ goto skip_rgb;
+
drvdata.led_mc = &oxp_cdev_rgb;
ret = devm_led_classdev_multicolor_register(&hdev->dev, &oxp_cdev_rgb);
@@ -585,6 +733,7 @@ static int oxp_cfg_probe(struct hid_device *hdev, u16 up)
dev_warn(drvdata.led_mc->led_cdev.dev,
"Failed to query RGB initial state: %i\n", ret);
+skip_rgb:
return 0;
}
@@ -613,6 +762,7 @@ static int oxp_hid_probe(struct hid_device *hdev,
switch (up) {
case GEN1_USAGE_PAGE:
+ case GEN2_USAGE_PAGE:
ret = oxp_cfg_probe(hdev, up);
if (ret) {
hid_hw_close(hdev);
@@ -633,6 +783,7 @@ static void oxp_hid_remove(struct hid_device *hdev)
static const struct hid_device_id oxp_devices[] = {
{ HID_USB_DEVICE(USB_VENDOR_ID_CRSC, USB_DEVICE_ID_ONEXPLAYER_GEN1) },
+ { HID_USB_DEVICE(USB_VENDOR_ID_WCH, USB_DEVICE_ID_ONEXPLAYER_GEN2) },
{}
};
--
2.53.0
^ permalink raw reply related
* [PATCH v2 1/5] HID: hid-oxp: Add OneXPlayer configuration driver
From: Derek J. Clark @ 2026-04-07 4:13 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: Pierre-Loup A . Griffais, Lambert Fan, Derek J . Clark,
linux-input, linux-doc, linux-kernel
In-Reply-To: <20260407041354.2283201-1-derekjohn.clark@gmail.com>
Adds OneXPlayer HID configuration driver. In this initial driver patch,
add the RGB interface for the first generation of HID based RGB control.
This interface provides the following attributes:
- brightness: provided by the LED core, this works in a fairly unique
way on this device. The hardware accepts 5 brightness values (0-4),
which affects the brightness of the multicolor and animated effects
built into the MCU firmware. For monocolor settings, the device
expects the hardware brightness value to be pushed to maximum, then we
apply brightness adjustments mathematically based on % (0-100). This
leads to some odd conversion as we need the brightness slider to reach
the full range, but it has no affect when incrementing between the
division points for other effects.
- multi-intensity: provided by the LED core for red, green, and blue.
- effect: Allows the MCU to set 19 individual effects.
- effect_index: Lists the 19 valid effect names for the interface.
- enabled: Allows the MCU to toggle the RGB interface on/off.
- enabled_index: Lists the valid states for enabled.
- speed: Allows the MCU to set the animation rate for the various
effects.
- speed_range: Lists the valid range of speed (0-9).
The MCU also has a few odd quirks that make sending multiple synchronous
events challenging. It will essentially freeze if it receives another
message before it has finished processing the last command. It also will
not reply if you wait on it using a completion. To get around this, we
do a 200ms sleep inside a work queue thread and debounce all but the most
recent message using a 50ms mod_delayed_work. This will cache the last
write, queue the work, then return so userspace can release its write
thread. The work queue is only used for brightness/multi-intensity as
that is the path likely to receive rapid successive writes.
Signed-off-by: Derek J. Clark <derekjohn.clark@gmail.com>
---
MAINTAINERS | 6 +
drivers/hid/Kconfig | 12 +
drivers/hid/Makefile | 1 +
drivers/hid/hid-ids.h | 3 +
drivers/hid/hid-oxp.c | 651 ++++++++++++++++++++++++++++++++++++++++++
5 files changed, 673 insertions(+)
create mode 100644 drivers/hid/hid-oxp.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 6f6517bf4f97..dae814192fa4 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -19707,6 +19707,12 @@ S: Maintained
F: drivers/mtd/nand/onenand/
F: include/linux/mtd/onenand*.h
+ONEXPLAYER HID DRIVER
+M: Derek J. Clark <derekjohn.clark@gmail.com>
+L: linux-input@vger.kernel.org
+S: Maintained
+F: drivers/hid/hid-oxp.c
+
ONEXPLAYER PLATFORM EC DRIVER
M: Antheas Kapenekakis <lkml@antheas.dev>
M: Derek John Clark <derekjohn.clark@gmail.com>
diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index 3c034cd32fa8..2deaec9f467d 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -919,6 +919,18 @@ config HID_ORTEK
- Ortek WKB-2000
- Skycable wireless presenter
+config HID_OXP
+ tristate "OneXPlayer handheld controller configuration support"
+ depends on USB_HID
+ depends on LEDS_CLASS
+ depends on LEDS_CLASS_MULTICOLOR
+ help
+ Say Y here if you would like to enable support for OneXPlayer handheld
+ devices that come with RGB LED rings around the joysticks and macro buttons.
+
+ To compile this driver as a module, choose M here: the module will
+ be called hid-oxp.
+
config HID_PANTHERLORD
tristate "Pantherlord/GreenAsia game controller"
help
diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
index 03ef72ec4499..bda8a24c9257 100644
--- a/drivers/hid/Makefile
+++ b/drivers/hid/Makefile
@@ -99,6 +99,7 @@ obj-$(CONFIG_HID_NTI) += hid-nti.o
obj-$(CONFIG_HID_NTRIG) += hid-ntrig.o
obj-$(CONFIG_HID_NVIDIA_SHIELD) += hid-nvidia-shield.o
obj-$(CONFIG_HID_ORTEK) += hid-ortek.o
+obj-$(CONFIG_HID_OXP) += hid-oxp.o
obj-$(CONFIG_HID_PRODIKEYS) += hid-prodikeys.o
obj-$(CONFIG_HID_PANTHERLORD) += hid-pl.o
obj-$(CONFIG_HID_PENMOUNT) += hid-penmount.o
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 5bad81222c6e..dcc5a3a70eaf 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -1131,6 +1131,9 @@
#define USB_VENDOR_ID_NVIDIA 0x0955
#define USB_DEVICE_ID_NVIDIA_THUNDERSTRIKE_CONTROLLER 0x7214
+#define USB_VENDOR_ID_CRSC 0x1a2c
+#define USB_DEVICE_ID_ONEXPLAYER_GEN1 0xb001
+
#define USB_VENDOR_ID_ONTRAK 0x0a07
#define USB_DEVICE_ID_ONTRAK_ADU100 0x0064
diff --git a/drivers/hid/hid-oxp.c b/drivers/hid/hid-oxp.c
new file mode 100644
index 000000000000..c4219ecd8d71
--- /dev/null
+++ b/drivers/hid/hid-oxp.c
@@ -0,0 +1,651 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * HID driver for OneXPlayer gamepad configuration devices.
+ *
+ * Copyright (c) 2026 Valve Corporation
+ */
+
+#include <linux/array_size.h>
+#include <linux/cleanup.h>
+#include <linux/delay.h>
+#include <linux/dev_printk.h>
+#include <linux/device.h>
+#include <linux/hid.h>
+#include <linux/jiffies.h>
+#include <linux/kstrtox.h>
+#include <linux/led-class-multicolor.h>
+#include <linux/mutex.h>
+#include <linux/sysfs.h>
+#include <linux/types.h>
+#include <linux/workqueue.h>
+
+#include "hid-ids.h"
+
+#define OXP_PACKET_SIZE 64
+
+#define GEN1_MESSAGE_ID 0xff
+
+#define GEN1_USAGE_PAGE 0xff01
+
+enum oxp_function_index {
+ OXP_FID_GEN1_RGB_SET = 0x07,
+ OXP_FID_GEN1_RGB_REPLY = 0x0f,
+};
+
+static struct oxp_hid_cfg {
+ struct led_classdev_mc *led_mc;
+ struct hid_device *hdev;
+ struct mutex cfg_mutex; /*ensure single synchronous output report*/
+ u8 rgb_brightness;
+ u8 rgb_effect;
+ u8 rgb_speed;
+ u8 rgb_en;
+} drvdata;
+
+enum oxp_feature_en_index {
+ OXP_FEAT_DISABLED,
+ OXP_FEAT_ENABLED,
+};
+
+static const char *const oxp_feature_en_text[] = {
+ [OXP_FEAT_DISABLED] = "false",
+ [OXP_FEAT_ENABLED] = "true",
+};
+
+enum oxp_rgb_effect_index {
+ OXP_UNKNOWN,
+ OXP_EFFECT_AURORA,
+ OXP_EFFECT_BIRTHDAY,
+ OXP_EFFECT_FLOWING,
+ OXP_EFFECT_CHROMA_1,
+ OXP_EFFECT_NEON,
+ OXP_EFFECT_CHROMA_2,
+ OXP_EFFECT_DREAMY,
+ OXP_EFFECT_WARM,
+ OXP_EFFECT_CYBERPUNK,
+ OXP_EFFECT_SEA,
+ OXP_EFFECT_SUNSET,
+ OXP_EFFECT_COLORFUL,
+ OXP_EFFECT_MONSTER,
+ OXP_EFFECT_GREEN,
+ OXP_EFFECT_BLUE,
+ OXP_EFFECT_YELLOW,
+ OXP_EFFECT_TEAL,
+ OXP_EFFECT_PURPLE,
+ OXP_EFFECT_FOGGY,
+ OXP_EFFECT_MONO_LIST, /* placeholder for effect_index_show */
+};
+
+/* These belong to rgb_effect_index, but we want to hide them from
+ * rgb_effect_text
+ */
+
+#define OXP_GET_PROPERTY 0xfc
+#define OXP_SET_PROPERTY 0xfd
+#define OXP_EFFECT_MONO_TRUE 0xfe /* actual index for monocolor */
+
+static const char *const oxp_rgb_effect_text[] = {
+ [OXP_UNKNOWN] = "unknown",
+ [OXP_EFFECT_AURORA] = "aurora",
+ [OXP_EFFECT_BIRTHDAY] = "birthday_cake",
+ [OXP_EFFECT_FLOWING] = "flowing_light",
+ [OXP_EFFECT_CHROMA_1] = "chroma_popping",
+ [OXP_EFFECT_NEON] = "neon",
+ [OXP_EFFECT_CHROMA_2] = "chroma_breathing",
+ [OXP_EFFECT_DREAMY] = "dreamy",
+ [OXP_EFFECT_WARM] = "warm_sun",
+ [OXP_EFFECT_CYBERPUNK] = "cyberpunk",
+ [OXP_EFFECT_SEA] = "sea_foam",
+ [OXP_EFFECT_SUNSET] = "sunset_afterglow",
+ [OXP_EFFECT_COLORFUL] = "colorful",
+ [OXP_EFFECT_MONSTER] = "monster_woke",
+ [OXP_EFFECT_GREEN] = "green_breathing",
+ [OXP_EFFECT_BLUE] = "blue_breathing",
+ [OXP_EFFECT_YELLOW] = "yellow_breathing",
+ [OXP_EFFECT_TEAL] = "teal_breathing",
+ [OXP_EFFECT_PURPLE] = "purple_breathing",
+ [OXP_EFFECT_FOGGY] = "foggy_haze",
+ [OXP_EFFECT_MONO_LIST] = "monocolor",
+};
+
+struct oxp_gen_1_rgb_report {
+ u8 report_id;
+ u8 message_id;
+ u8 padding_2[2];
+ u8 effect;
+ u8 enabled;
+ u8 speed;
+ u8 brightness;
+ u8 red;
+ u8 green;
+ u8 blue;
+} __packed;
+
+static u16 get_usage_page(struct hid_device *hdev)
+{
+ return hdev->collection[0].usage >> 16;
+}
+
+static int oxp_hid_raw_event_gen_1(struct hid_device *hdev,
+ struct hid_report *report, u8 *data,
+ int size)
+{
+ struct led_classdev_mc *led_mc = drvdata.led_mc;
+ struct oxp_gen_1_rgb_report *rgb_rep;
+
+ if (data[1] != OXP_FID_GEN1_RGB_REPLY)
+ return 0;
+
+ rgb_rep = (struct oxp_gen_1_rgb_report *)data;
+ /* Ensure we save monocolor as the list value */
+ drvdata.rgb_effect = rgb_rep->effect == OXP_EFFECT_MONO_TRUE ?
+ OXP_EFFECT_MONO_LIST :
+ rgb_rep->effect;
+ drvdata.rgb_speed = rgb_rep->speed;
+ drvdata.rgb_en = rgb_rep->enabled == 0 ? OXP_FEAT_DISABLED :
+ OXP_FEAT_ENABLED;
+ drvdata.rgb_brightness = rgb_rep->brightness;
+ led_mc->led_cdev.brightness = rgb_rep->brightness / 4 *
+ led_mc->led_cdev.max_brightness;
+ /* If monocolor had less than 100% brightness on the previous boot,
+ * there will be no reliable way to determine the real intensity.
+ * Since intensity scaling is used with a hardware brightness set at max,
+ * our brightness will always look like 100%. Use the last set value to
+ * prevent successive boots from lowering the brightness further.
+ * Brightness will be "wrong" but the effect will remain the same visually.
+ */
+ led_mc->subled_info[0].intensity = rgb_rep->red;
+ led_mc->subled_info[1].intensity = rgb_rep->green;
+ led_mc->subled_info[2].intensity = rgb_rep->blue;
+
+ return 0;
+}
+
+static int oxp_hid_raw_event(struct hid_device *hdev, struct hid_report *report,
+ u8 *data, int size)
+{
+ u16 up = get_usage_page(hdev);
+
+ dev_dbg(&hdev->dev, "raw event data: [%*ph]\n", OXP_PACKET_SIZE, data);
+
+ switch (up) {
+ case GEN1_USAGE_PAGE:
+ return oxp_hid_raw_event_gen_1(hdev, report, data, size);
+ default:
+ break;
+ }
+
+ return 0;
+}
+
+static int mcu_property_out(u8 *header, size_t header_size, u8 *data,
+ size_t data_size, u8 *footer, size_t footer_size)
+{
+ unsigned char *dmabuf __free(kfree) = kzalloc(OXP_PACKET_SIZE, GFP_KERNEL);
+ int ret;
+
+ if (!dmabuf)
+ return -ENOMEM;
+
+ if (header_size + data_size + footer_size > OXP_PACKET_SIZE)
+ return -EINVAL;
+
+ guard(mutex)(&drvdata.cfg_mutex);
+ memcpy(dmabuf, header, header_size);
+ memcpy(dmabuf + header_size, data, data_size);
+ if (footer_size)
+ memcpy(dmabuf + OXP_PACKET_SIZE - footer_size, footer, footer_size);
+
+ dev_dbg(&drvdata.hdev->dev, "raw data: [%*ph]\n", OXP_PACKET_SIZE, dmabuf);
+
+ ret = hid_hw_output_report(drvdata.hdev, dmabuf, OXP_PACKET_SIZE);
+ if (ret < 0)
+ return ret;
+
+ /* MCU takes 200ms to be ready for another command. */
+ msleep(200);
+ return ret == OXP_PACKET_SIZE ? 0 : -EIO;
+}
+
+static int oxp_gen_1_property_out(enum oxp_function_index fid, u8 *data,
+ u8 data_size)
+{
+ u8 header[] = { fid, GEN1_MESSAGE_ID };
+ size_t header_size = ARRAY_SIZE(header);
+
+ return mcu_property_out(header, header_size, data, data_size, NULL, 0);
+}
+
+static int oxp_rgb_status_store(u8 enabled, u8 speed, u8 brightness)
+{
+ u16 up = get_usage_page(drvdata.hdev);
+ u8 *data;
+
+ /* Always default to max brightness and use intensity scaling when in
+ * monocolor mode.
+ */
+ switch (up) {
+ case GEN1_USAGE_PAGE:
+ data = (u8[4]) { OXP_SET_PROPERTY, enabled, speed, brightness };
+ if (drvdata.rgb_effect == OXP_EFFECT_MONO_LIST)
+ data[3] = 0x04;
+ return oxp_gen_1_property_out(OXP_FID_GEN1_RGB_SET, data, 4);
+ default:
+ return -ENODEV;
+ }
+}
+
+static ssize_t oxp_rgb_status_show(void)
+{
+ u16 up = get_usage_page(drvdata.hdev);
+ u8 *data;
+
+ switch (up) {
+ case GEN1_USAGE_PAGE:
+ data = (u8[1]) { OXP_GET_PROPERTY };
+ return oxp_gen_1_property_out(OXP_FID_GEN1_RGB_SET, data, 1);
+ default:
+ return -ENODEV;
+ }
+}
+
+static int oxp_rgb_color_set(void)
+{
+ u8 max_br = drvdata.led_mc->led_cdev.max_brightness;
+ u8 br = drvdata.led_mc->led_cdev.brightness;
+ u16 up = get_usage_page(drvdata.hdev);
+ u8 green, red, blue;
+ size_t size;
+ u8 *data;
+ int i;
+
+ red = br * drvdata.led_mc->subled_info[0].intensity / max_br;
+ green = br * drvdata.led_mc->subled_info[1].intensity / max_br;
+ blue = br * drvdata.led_mc->subled_info[2].intensity / max_br;
+
+ switch (up) {
+ case GEN1_USAGE_PAGE:
+ size = 55;
+ data = (u8[55]) { OXP_EFFECT_MONO_TRUE };
+
+ for (i = 0; i < (size - 1) / 3; i++) {
+ data[3 * i + 1] = red;
+ data[3 * i + 2] = green;
+ data[3 * i + 3] = blue;
+ }
+ return oxp_gen_1_property_out(OXP_FID_GEN1_RGB_SET, data, size);
+ default:
+ return -ENODEV;
+ }
+}
+
+static int oxp_rgb_effect_set(u8 effect)
+{
+ u16 up = get_usage_page(drvdata.hdev);
+ u8 *data;
+ int ret;
+
+ switch (effect) {
+ case OXP_EFFECT_AURORA:
+ case OXP_EFFECT_BIRTHDAY:
+ case OXP_EFFECT_FLOWING:
+ case OXP_EFFECT_CHROMA_1:
+ case OXP_EFFECT_NEON:
+ case OXP_EFFECT_CHROMA_2:
+ case OXP_EFFECT_DREAMY:
+ case OXP_EFFECT_WARM:
+ case OXP_EFFECT_CYBERPUNK:
+ case OXP_EFFECT_SEA:
+ case OXP_EFFECT_SUNSET:
+ case OXP_EFFECT_COLORFUL:
+ case OXP_EFFECT_MONSTER:
+ case OXP_EFFECT_GREEN:
+ case OXP_EFFECT_BLUE:
+ case OXP_EFFECT_YELLOW:
+ case OXP_EFFECT_TEAL:
+ case OXP_EFFECT_PURPLE:
+ case OXP_EFFECT_FOGGY:
+ switch (up) {
+ case GEN1_USAGE_PAGE:
+ data = (u8[1]) { effect };
+ ret = oxp_gen_1_property_out(OXP_FID_GEN1_RGB_SET, data, 1);
+ break;
+ default:
+ ret = -ENODEV;
+ }
+ break;
+ case OXP_EFFECT_MONO_LIST:
+ ret = oxp_rgb_color_set();
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ if (ret)
+ return ret;
+
+ drvdata.rgb_effect = effect;
+
+ return 0;
+}
+
+static ssize_t enabled_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ int ret;
+ u8 val;
+
+ ret = sysfs_match_string(oxp_feature_en_text, buf);
+ if (ret < 0)
+ return ret;
+ val = ret;
+
+ ret = oxp_rgb_status_store(val, drvdata.rgb_speed,
+ drvdata.rgb_brightness);
+ if (ret)
+ return ret;
+
+ drvdata.rgb_en = val;
+ return count;
+}
+
+static ssize_t enabled_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ int ret;
+
+ ret = oxp_rgb_status_show();
+ if (ret)
+ return ret;
+
+ if (drvdata.rgb_en >= ARRAY_SIZE(oxp_feature_en_text))
+ return -EINVAL;
+
+ return sysfs_emit(buf, "%s\n", oxp_feature_en_text[drvdata.rgb_en]);
+}
+static DEVICE_ATTR_RW(enabled);
+
+static ssize_t enabled_index_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ size_t count = 0;
+ unsigned int i;
+
+ for (i = 0; i < ARRAY_SIZE(oxp_feature_en_text); i++)
+ count += sysfs_emit_at(buf, count, "%s ", oxp_feature_en_text[i]);
+
+ if (count)
+ buf[count - 1] = '\n';
+
+ return count;
+}
+static DEVICE_ATTR_RO(enabled_index);
+
+static ssize_t effect_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ int ret;
+ u8 val;
+
+ ret = sysfs_match_string(oxp_rgb_effect_text, buf);
+ if (ret < 0)
+ return ret;
+
+ val = ret;
+
+ ret = oxp_rgb_status_store(drvdata.rgb_en, drvdata.rgb_speed,
+ drvdata.rgb_brightness);
+ if (ret)
+ return ret;
+
+ ret = oxp_rgb_effect_set(val);
+ if (ret)
+ return ret;
+
+ return count;
+}
+
+static ssize_t effect_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ int ret;
+
+ ret = oxp_rgb_status_show();
+ if (ret)
+ return ret;
+
+ if (drvdata.rgb_effect >= ARRAY_SIZE(oxp_rgb_effect_text))
+ return -EINVAL;
+
+ return sysfs_emit(buf, "%s\n", oxp_rgb_effect_text[drvdata.rgb_effect]);
+}
+
+static DEVICE_ATTR_RW(effect);
+
+static ssize_t effect_index_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ size_t count = 0;
+ unsigned int i;
+
+ for (i = 1; i < ARRAY_SIZE(oxp_rgb_effect_text); i++)
+ count += sysfs_emit_at(buf, count, "%s ", oxp_rgb_effect_text[i]);
+
+ if (count)
+ buf[count - 1] = '\n';
+
+ return count;
+}
+static DEVICE_ATTR_RO(effect_index);
+
+static ssize_t speed_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ int ret;
+ u8 val;
+
+ ret = kstrtou8(buf, 10, &val);
+ if (ret)
+ return ret;
+
+ if (val > 9)
+ return -EINVAL;
+
+ ret = oxp_rgb_status_store(drvdata.rgb_en, val, drvdata.rgb_brightness);
+ if (ret)
+ return ret;
+
+ drvdata.rgb_speed = val;
+ return count;
+}
+
+static ssize_t speed_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ int ret;
+
+ ret = oxp_rgb_status_show();
+ if (ret)
+ return ret;
+
+ if (drvdata.rgb_speed > 9)
+ return -EINVAL;
+
+ return sysfs_emit(buf, "%hhu\n", drvdata.rgb_speed);
+}
+static DEVICE_ATTR_RW(speed);
+
+static ssize_t speed_range_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ return sysfs_emit(buf, "0-9\n");
+}
+static DEVICE_ATTR_RO(speed_range);
+
+static void oxp_rgb_queue_fn(struct work_struct *work)
+{
+ unsigned int max_brightness = drvdata.led_mc->led_cdev.max_brightness;
+ unsigned int brightness = drvdata.led_mc->led_cdev.brightness;
+ u8 val = 4 * brightness / max_brightness;
+ int ret;
+
+ if (drvdata.rgb_brightness != val) {
+ ret = oxp_rgb_status_store(drvdata.rgb_en, drvdata.rgb_speed, val);
+ if (ret)
+ dev_err(drvdata.led_mc->led_cdev.dev,
+ "Error: Failed to write RGB Status: %i\n", ret);
+
+ drvdata.rgb_brightness = val;
+ }
+
+ if (drvdata.rgb_effect != OXP_EFFECT_MONO_LIST)
+ return;
+
+ ret = oxp_rgb_effect_set(drvdata.rgb_effect);
+ if (ret)
+ dev_err(drvdata.led_mc->led_cdev.dev, "Error: Failed to write RGB color: %i\n",
+ ret);
+}
+
+static DECLARE_DELAYED_WORK(oxp_rgb_queue, oxp_rgb_queue_fn);
+
+static void oxp_rgb_brightness_set(struct led_classdev *led_cdev,
+ enum led_brightness brightness)
+{
+ led_cdev->brightness = brightness;
+ mod_delayed_work(system_wq, &oxp_rgb_queue, msecs_to_jiffies(50));
+}
+
+static struct attribute *oxp_rgb_attrs[] = {
+ &dev_attr_effect.attr,
+ &dev_attr_effect_index.attr,
+ &dev_attr_enabled.attr,
+ &dev_attr_enabled_index.attr,
+ &dev_attr_speed.attr,
+ &dev_attr_speed_range.attr,
+ NULL,
+};
+
+static const struct attribute_group oxp_rgb_attr_group = {
+ .attrs = oxp_rgb_attrs,
+};
+
+static struct mc_subled oxp_rgb_subled_info[] = {
+ {
+ .color_index = LED_COLOR_ID_RED,
+ .intensity = 0x24,
+ .channel = 0x1,
+ },
+ {
+ .color_index = LED_COLOR_ID_GREEN,
+ .intensity = 0x22,
+ .channel = 0x2,
+ },
+ {
+ .color_index = LED_COLOR_ID_BLUE,
+ .intensity = 0x99,
+ .channel = 0x3,
+ },
+};
+
+static struct led_classdev_mc oxp_cdev_rgb = {
+ .led_cdev = {
+ .name = "oxp:rgb:joystick_rings",
+ .color = LED_COLOR_ID_RGB,
+ .brightness = 0x64,
+ .max_brightness = 0x64,
+ .brightness_set = oxp_rgb_brightness_set,
+ },
+ .num_colors = ARRAY_SIZE(oxp_rgb_subled_info),
+ .subled_info = oxp_rgb_subled_info,
+};
+
+static int oxp_cfg_probe(struct hid_device *hdev, u16 up)
+{
+ int ret;
+
+ hid_set_drvdata(hdev, &drvdata);
+ mutex_init(&drvdata.cfg_mutex);
+ drvdata.hdev = hdev;
+ drvdata.led_mc = &oxp_cdev_rgb;
+
+ ret = devm_led_classdev_multicolor_register(&hdev->dev, &oxp_cdev_rgb);
+ if (ret)
+ return dev_err_probe(&hdev->dev, ret,
+ "Failed to create RGB device\n");
+
+ ret = devm_device_add_group(drvdata.led_mc->led_cdev.dev,
+ &oxp_rgb_attr_group);
+ if (ret)
+ return dev_err_probe(drvdata.led_mc->led_cdev.dev, ret,
+ "Failed to create RGB configuration attributes\n");
+
+ ret = oxp_rgb_status_show();
+ if (ret)
+ dev_warn(drvdata.led_mc->led_cdev.dev,
+ "Failed to query RGB initial state: %i\n", ret);
+
+ return 0;
+}
+
+static int oxp_hid_probe(struct hid_device *hdev,
+ const struct hid_device_id *id)
+{
+ int ret;
+ u16 up;
+
+ ret = hid_parse(hdev);
+ if (ret)
+ return dev_err_probe(&hdev->dev, ret, "Failed to parse HID device\n");
+
+ ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
+ if (ret)
+ return dev_err_probe(&hdev->dev, ret, "Failed to start HID device\n");
+
+ ret = hid_hw_open(hdev);
+ if (ret) {
+ hid_hw_stop(hdev);
+ return dev_err_probe(&hdev->dev, ret, "Failed to open HID device\n");
+ }
+
+ up = get_usage_page(hdev);
+ dev_dbg(&hdev->dev, "Got usage page %04x\n", up);
+
+ switch (up) {
+ case GEN1_USAGE_PAGE:
+ ret = oxp_cfg_probe(hdev, up);
+ if (ret) {
+ hid_hw_close(hdev);
+ hid_hw_stop(hdev);
+ }
+
+ return ret;
+ default:
+ return 0;
+ }
+}
+
+static void oxp_hid_remove(struct hid_device *hdev)
+{
+ hid_hw_close(hdev);
+ hid_hw_stop(hdev);
+}
+
+static const struct hid_device_id oxp_devices[] = {
+ { HID_USB_DEVICE(USB_VENDOR_ID_CRSC, USB_DEVICE_ID_ONEXPLAYER_GEN1) },
+ {}
+};
+
+MODULE_DEVICE_TABLE(hid, oxp_devices);
+static struct hid_driver hid_oxp = {
+ .name = "hid-oxp",
+ .id_table = oxp_devices,
+ .probe = oxp_hid_probe,
+ .remove = oxp_hid_remove,
+ .raw_event = oxp_hid_raw_event,
+};
+module_hid_driver(hid_oxp);
+
+MODULE_AUTHOR("Derek J. Clark <derekjohn.clark@gmail.com>");
+MODULE_DESCRIPTION("Driver for OneXPlayer HID Interfaces");
+MODULE_LICENSE("GPL");
--
2.53.0
^ permalink raw reply related
* [PATCH v2 0/5] Add OneXPlayer Configuration HID Driver
From: Derek J. Clark @ 2026-04-07 4:13 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: Pierre-Loup A . Griffais, Lambert Fan, Derek J . Clark,
linux-input, linux-doc, linux-kernel
Adds an HID driver for OneXPlayer HID configuration devices. There are
currently 2 generations of OneXPlayer HID protocol. The first (OneXPlayer
F1 series) only provides an RGB control interface over HID. The Second
(X1 mini series, G1 series, AOKZOE A1X) also includes a hardware level
button mapping interface, vibration intensity settings, and the ability
to switch output between xinput and a debug mode that can be used to debug
the button mapping. Some devices (G1 Series, APEX) use a hybrid of Gen1
RGB control and Gen 2 controller settings. To ensure there is no conflicts
when the driver is loaded, we skip creating the RGB interface for Gen 2
devices if there is a DMI match.
I'll also add a note that Gen 1 devices also have an interface for
setting the key map and debug mode, but that is done entirely over a
serial TTY device so it is not able to be added to this driver. There
are also some "Gen 0" devices (OneXPlayer 2 Series) also use it, but
the TTY interface also handles the RGB control so no support is
provided by this driver for those devices.
Signed-off-by: Derel J. Clark <derekjohn.clark@gmail.com>
---
v2:
- Add DMI quirks for certain devices that ship with both GEN1 and GEN2
MCU to avoid clashing when initializing the RGB interface.
- Add left & right vibration intensity attributes.
- Add additional mappings for keyboard inputs.
- Add a delayed work trigger to re-apply settings after the MCU
completes initializing after a suspend/resume cycle.
v1: https://lore.kernel.org/linux-input/20260322031615.1524307-1-derekjohn.clark@gmail.com/
Derek J. Clark (5):
HID: hid-oxp: Add OneXPlayer configuration driver
HID: hid-oxp: Add Second Generation RGB Control
HID: hid-oxp: Add Second Generation Gamepad Mode Switch
HID: hid-oxp: Add Button Mapping Interface
HID: hid-oxp: Add Virbation Intenstity Attributes
MAINTAINERS | 6 +
drivers/hid/Kconfig | 13 +
drivers/hid/Makefile | 1 +
drivers/hid/hid-ids.h | 6 +
drivers/hid/hid-oxp.c | 1595 +++++++++++++++++++++++++++++++++++++++++
5 files changed, 1621 insertions(+)
create mode 100644 drivers/hid/hid-oxp.c
--
2.53.0
^ permalink raw reply
* [PATCH v2] Input: gf2k: skip invalid hat lookup values
From: Pengpeng Hou @ 2026-04-07 1:56 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input, linux-kernel, kees, pengpeng
In-Reply-To: <20260323074541.93413-1-pengpeng@iscas.ac.cn>
gf2k_read() decodes the hat position from a 4-bit field and uses it
directly to index gf2k_hat_to_axis[]. The lookup table only has nine
entries, so malformed packets can read past the end of the fixed table.
Skip hat reporting when the decoded value falls outside the lookup
table instead of forcing it to the neutral position. This keeps the
fix local and avoids reporting a made-up axis state for malformed
packets.
Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
---
Changes since v1:
- skip reporting invalid hat values instead of clamping them to the
neutral entry
drivers/input/joystick/gf2k.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/input/joystick/gf2k.c b/drivers/input/joystick/gf2k.c
index 5a1cdce0bc48..1d843115d674 100644
--- a/drivers/input/joystick/gf2k.c
+++ b/drivers/input/joystick/gf2k.c
@@ -165,8 +165,10 @@ static void gf2k_read(struct gf2k *gf2k, unsigned char *data)
t = GB(40,4,0);
- for (i = 0; i < gf2k_hats[gf2k->id]; i++)
- input_report_abs(dev, ABS_HAT0X + i, gf2k_hat_to_axis[t][i]);
+ if (t < ARRAY_SIZE(gf2k_hat_to_axis))
+ for (i = 0; i < gf2k_hats[gf2k->id]; i++)
+ input_report_abs(dev, ABS_HAT0X + i,
+ gf2k_hat_to_axis[t][i]);
t = GB(44,2,0) | GB(32,8,2) | GB(78,2,10);
--
2.50.1 (Apple Git-155)
^ permalink raw reply related
* Re: [PATCH] Input: gf2k: clamp hat values to the lookup table
From: Pengpeng Hou @ 2026-04-07 3:30 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input, linux-kernel, kees, pengpeng
In-Reply-To: <20260323074541.93413-1-pengpeng@iscas.ac.cn>
Hi Dmitry,
Thanks, that makes sense.
I have updated this to skip reporting hat axes when the decoded value
falls outside the lookup table instead of clamping it to the neutral
entry, and will resend it that way.
Thanks,
Pengpeng
^ permalink raw reply
* Re: [PATCH] Input: gpio-keys - add hibernation support
From: Armando De Leon @ 2026-04-06 20:39 UTC (permalink / raw)
To: dmitry.torokhov; +Cc: Armando De Leon, linux-input, linux-kernel
In-Reply-To: <adPdc2Z1SYxvqDmP@google.com>
Hi Dmitry,
Thanks for the guidance. I investigated further:
The upstream GICv3 driver (irq-gic-v3.c) has no suspend/resume or
syscore_ops for the distributor - it does not save or restore any
per-IRQ configuration across power cycles. On platforms where the
GIC is re-initialized by firmware during hibernate restore, all
trigger type configurations set by consumer drivers during probe()
are lost.
The IRQ core does preserve the trigger type in irq_data
(irqd_get_trigger_type), but resume_irq() in kernel/irq/pm.c only
re-enables the interrupt without re-applying the trigger type to
hardware.
A fix in the IRQ PM resume path (having resume_irq() call
irq_set_irq_type() using the saved trigger type) would fix all
consumer drivers generically. However, such a change would have
broad consequences and impact across all platforms and interrupt
controllers, and would need very careful review and testing.
Given that the gpio-keys fix is a single irq_set_irq_type() call
in the .restore callback - minimal, self-contained, and low risk -
would it be acceptable to handle it at the driver level for now?
This avoids the risk of a sweeping IRQ core change while solving
the immediate problem for gpio-keys users with hibernation support.
Thanks,
Armando
^ permalink raw reply
* Re: [PATCH 1/2] input: pc110pad: change PCI check to get rid of orphaned no_pci_devices
From: Dmitry Torokhov @ 2026-04-06 19:02 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Heiner Kallweit, Bjorn Helgaas, open list:HID CORE LAYER,
linux-pci@vger.kernel.org
In-Reply-To: <20260406172324.GA170281@bhelgaas>
On Mon, Apr 06, 2026 at 12:23:24PM -0500, Bjorn Helgaas wrote:
> On Fri, Apr 03, 2026 at 12:17:35AM +0200, Heiner Kallweit wrote:
> > As a prerequisite for removing no_pci_devices(), replace its usage here
> > with an equivalent check for presence of a PCI bus.
> >
> > Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> > ---
> > drivers/input/mouse/pc110pad.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/input/mouse/pc110pad.c b/drivers/input/mouse/pc110pad.c
> > index efa58049f..c7a6df210 100644
> > --- a/drivers/input/mouse/pc110pad.c
> > +++ b/drivers/input/mouse/pc110pad.c
> > @@ -91,7 +91,7 @@ static int __init pc110pad_init(void)
> > {
> > int err;
> >
> > - if (!no_pci_devices())
> > + if (pci_find_next_bus(NULL))
> > return -ENODEV;
>
> I'd certainly love to get rid of no_pci_devices(), although using
> pci_find_next_bus() is not really much better.
>
> I don't have much opinion about pc110pad.c change. It's essentially
> still the same kludge, which has been there at least since the
> beginning of git history.
>
> Dmitry, if you want to ack this, I can take both together.
Given "x86/cpu: Remove M486/M486SX/ELAN support" I'd just drop the
driver completely.
I tried it in 2024 but Maciej was insistent that keeping these old
drivers doe snot hurt. Now I think we can get rid of them.
You can grab the patch removing it from
https://lore.kernel.org/all/20240808172733.1194442-4-dmitry.torokhov@gmail.com/
as I think it should still apply.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH] HID: lg-g15: Add support for the Logitech G710/G710+ gaming keyboards
From: Hans de Goede @ 2026-04-06 18:50 UTC (permalink / raw)
To: Xavier Bestel, Jiri Kosina, Benjamin Tissoires; +Cc: linux-input, linux-kernel
In-Reply-To: <20260402075239.3829699-1-xav@bes.tel>
Hi Xavier,
Thank you for your patch, it is always nice to have people helping
with improving Linux support for devices like this.
On 2-Apr-26 09:52, Xavier Bestel wrote:
> Add support for the Logitech G710 and G710+ (USB ID 046d:c24d) gaming
> keyboards to the hid-lg-g15 driver.
>
> These keyboards have 6 G-keys (G1-G6), M-keys (M1-M3, MR), a game mode
> toggle, and two independent backlights: one for the main keyboard and one
> for the WASD keys, each with a physical button to cycle brightness through
> 5 levels (0-4).
>
> Key implementation details:
>
> - G-keys and M-keys are reported via HID input report 0x03 (4 bytes)
> using KEY_MACRO1-6, KEY_MACRO_PRESET1-3 and KEY_MACRO_RECORD_START.
>
> - The WASD backlight LED is registered as
> "g15::kbd_zoned_backlight-wasd" rather than with a
> "::kbd_backlight" suffix, because UPower currently only supports a
> single kbd_backlight LED per device. This follows the nomenclature
> from Documentation/leds/leds-class.rst
Actually that nomenclature is something which was suggested in the past
but has never been agreed upon.
Since the discussion about how to name the LED class devices for
multi-zone keyboard backlights keeps coming up I've submitted a patch
now to document how these should be named:
https://lore.kernel.org/linux-leds/20260406174638.320135-1-johannes.goede@oss.qualcomm.com/
Since the new userspace API for this needs to be agreed upon first, that
patch should be accepted upstream first, before we can move forward with
the G710 support from this patch.
>
> - The G710+ firmware GET_REPORT for the backlight feature (0x08)
> always returns the power-on default values, ignoring any changes
> made via SET_REPORT. To work around this, the backlight brightness
> is tracked in the driver cache and brightness_get returns the
> cached value. M-key and game mode LEDs read back correctly.
>
> - The physical brightness cycle buttons are handled following the
> same pattern as the G15/G15v2: no key events are sent, instead the
> driver cycles the cached brightness and calls
> led_classdev_notify_brightness_hw_changed() from a work function,
> which allows GNOME to show the brightness OSD.
>
> - The game mode toggle is handled entirely by the firmware (it
> disables the Super key and lights an indicator LED). The driver
> exposes a read-only LED "g15::gamemode" with the
> LED_BRIGHT_HW_CHANGED flag, and notifies userspace of state
> changes via led_classdev_notify_brightness_hw_changed() by reading
> back the actual firmware state on each toggle.
>
> - Both brightness LEDs are registered with the LED_BRIGHT_HW_CHANGED
> flag to enable the brightness_hw_changed sysfs attribute.
>
> - HID_QUIRK_NOGET is set because the keyboard has buggy GET_REPORT
> handling that causes timeouts on some feature reports.
>
> - The G-keys feature report (0x09) uses 2 bytes per key rather than
> 1 as on the G510, so the report size calculation is adjusted
> accordingly.
>
> - Also fix a pre-existing comment typo: "f000.0000" -> "ff00.0000"
> for the application report ID.
>
> - The loop bounds in lg_g15_led_set() and lg_g510_led_set() are
> tightened from "< LG_G15_LED_MAX" to "<= LG_G15_MACRO_RECORD" to
> avoid iterating over the new LG_G15_GAMEMODE enum value which does
> not apply to those keyboards.
>
> Signed-off-by: Xavier Bestel <xav@bes.tel>
> ---
> drivers/hid/hid-ids.h | 1 +
> drivers/hid/hid-lg-g15.c | 393 ++++++++++++++++++++++++++++++++++++++-
> 2 files changed, 384 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
> index afcee13bad61..7c0f930bd014 100644
> --- a/drivers/hid/hid-ids.h
> +++ b/drivers/hid/hid-ids.h
> @@ -904,6 +904,7 @@
> #define USB_DEVICE_ID_LOGITECH_G15_V2_LCD 0xc227
> #define USB_DEVICE_ID_LOGITECH_G510 0xc22d
> #define USB_DEVICE_ID_LOGITECH_G510_USB_AUDIO 0xc22e
> +#define USB_DEVICE_ID_LOGITECH_G710 0xc24d
> #define USB_DEVICE_ID_LOGITECH_G29_WHEEL 0xc24f
> #define USB_DEVICE_ID_LOGITECH_G920_WHEEL 0xc262
> #define USB_DEVICE_ID_LOGITECH_G923_XBOX_WHEEL 0xc26e
> diff --git a/drivers/hid/hid-lg-g15.c b/drivers/hid/hid-lg-g15.c
> index 1a88bc44ada4..8a4c4eb22c07 100644
> --- a/drivers/hid/hid-lg-g15.c
> +++ b/drivers/hid/hid-lg-g15.c
> @@ -29,6 +29,11 @@
> #define LG_G510_INPUT_MACRO_KEYS 0x03
> #define LG_G510_INPUT_KBD_BACKLIGHT 0x04
>
> +#define LG_G710_FEATURE_GAMEMODE 0x05
> +#define LG_G710_FEATURE_M_KEYS_LEDS 0x06
> +#define LG_G710_FEATURE_BACKLIGHT 0x08
> +#define LG_G710_FEATURE_EXTRA_KEYS 0x09
> +
> #define LG_G13_INPUT_REPORT 0x01
> #define LG_G13_FEATURE_M_KEYS_LEDS 0x05
> #define LG_G13_FEATURE_BACKLIGHT_RGB 0x07
> @@ -48,6 +53,7 @@ enum lg_g15_model {
> LG_G15_V2,
> LG_G510,
> LG_G510_USB_AUDIO,
> + LG_G710,
> LG_Z10,
> };
>
> @@ -59,6 +65,7 @@ enum lg_g15_led_type {
> LG_G15_MACRO_PRESET2,
> LG_G15_MACRO_PRESET3,
> LG_G15_MACRO_RECORD,
> + LG_G15_GAMEMODE,
> LG_G15_LED_MAX
> };
>
> @@ -91,7 +98,9 @@ struct lg_g15_data {
> enum lg_g15_model model;
> struct lg_g15_led leds[LG_G15_LED_MAX];
> bool game_mode_enabled;
> + u16 pressed_keys;
> bool backlight_disabled; /* true == HW backlight toggled *OFF* */
> + unsigned long brightness_changed; /* bitmask of LEDs hw-cycled */
> };
>
> /********* G13 LED functions ***********/
> @@ -334,7 +343,7 @@ static int lg_g15_led_set(struct led_classdev *led_cdev,
> g15->transfer_buf[1] = g15_led->led + 1;
> g15->transfer_buf[2] = brightness << (g15_led->led * 4);
> } else {
> - for (i = LG_G15_MACRO_PRESET1; i < LG_G15_LED_MAX; i++) {
> + for (i = LG_G15_MACRO_PRESET1; i <= LG_G15_MACRO_RECORD; i++) {
> if (i == g15_led->led)
> val = brightness;
> else
> @@ -567,7 +576,7 @@ static int lg_g510_mkey_led_set(struct led_classdev *led_cdev,
>
> mutex_lock(&g15->mutex);
>
> - for (i = LG_G15_MACRO_PRESET1; i < LG_G15_LED_MAX; i++) {
> + for (i = LG_G15_MACRO_PRESET1; i <= LG_G15_MACRO_RECORD; i++) {
> if (i == g15_led->led)
> val = brightness;
> else
> @@ -597,6 +606,239 @@ static int lg_g510_mkey_led_set(struct led_classdev *led_cdev,
> return ret;
> }
>
> +/******** G710 LED functions ********/
> +
> +static int lg_g710_update_game_led_brightness(struct lg_g15_data *g15)
> +{
> + int ret;
> +
> + ret = hid_hw_raw_request(g15->hdev, LG_G710_FEATURE_GAMEMODE,
> + g15->transfer_buf, 8,
> + HID_FEATURE_REPORT, HID_REQ_GET_REPORT);
> + if (ret != 8) {
> + hid_err(g15->hdev, "Error getting gamemode LED brightness: %d\n", ret);
> + return (ret < 0) ? ret : -EIO;
> + }
> +
> + g15->leds[LG_G15_GAMEMODE].brightness =
> + !!g15->transfer_buf[1];
> +
> + return 0;
> +}
> +
> +static int lg_g710_update_mkey_led_brightness(struct lg_g15_data *g15)
> +{
> + int ret;
> +
> + ret = hid_hw_raw_request(g15->hdev, LG_G710_FEATURE_M_KEYS_LEDS,
> + g15->transfer_buf, 2,
> + HID_FEATURE_REPORT, HID_REQ_GET_REPORT);
> + if (ret != 2) {
> + hid_err(g15->hdev, "Error getting macro LED brightness: %d\n", ret);
> + return (ret < 0) ? ret : -EIO;
> + }
> +
> + g15->leds[LG_G15_MACRO_PRESET1].brightness =
> + !!(g15->transfer_buf[1] & 0x10);
> + g15->leds[LG_G15_MACRO_PRESET2].brightness =
> + !!(g15->transfer_buf[1] & 0x20);
> + g15->leds[LG_G15_MACRO_PRESET3].brightness =
> + !!(g15->transfer_buf[1] & 0x40);
> + g15->leds[LG_G15_MACRO_RECORD].brightness =
> + !!(g15->transfer_buf[1] & 0x80);
> +
> + return 0;
> +}
> +
> +static int lg_g710_update_kbd_led_brightness(struct lg_g15_data *g15)
> +{
> + int ret;
> +
> + ret = hid_hw_raw_request(g15->hdev, LG_G710_FEATURE_BACKLIGHT,
> + g15->transfer_buf, 4,
> + HID_FEATURE_REPORT, HID_REQ_GET_REPORT);
> + if (ret != 4) {
> + hid_err(g15->hdev, "Error getting LED brightness: %d\n", ret);
> + return (ret < 0) ? ret : -EIO;
> + }
> +
> + g15->leds[LG_G15_KBD_BRIGHTNESS].brightness = 4 - g15->transfer_buf[2];
> + g15->leds[LG_G15_LCD_BRIGHTNESS].brightness = 4 - g15->transfer_buf[1];
I think this needs a range-check on the buf contents to avoid e.g.
negative brightness values when we somehow get unexpected values back from
the keyboard.
> +
> + return 0;
> +}
> +
> +static enum led_brightness lg_g710_led_get(struct led_classdev *led_cdev)
> +{
> + struct lg_g15_led *g15_led =
> + container_of(led_cdev, struct lg_g15_led, cdev);
> + struct lg_g15_data *g15 = dev_get_drvdata(led_cdev->dev->parent);
> + enum led_brightness brightness;
> +
> + mutex_lock(&g15->mutex);
> + /*
> + * The G710+ firmware's GET_REPORT for the backlight always returns
> + * the power-on default values, ignoring any changes made via
> + * SET_REPORT. Use the cached brightness which is kept in sync by
> + * the _set callbacks. M-key and gamemode LEDs read back correctly.
> + */
> + if (g15_led->led >= LG_G15_BRIGHTNESS_MAX && g15_led->led < LG_G15_GAMEMODE)
> + lg_g710_update_mkey_led_brightness(g15);
> + else if (g15_led->led >= LG_G15_GAMEMODE)
> + lg_g710_update_game_led_brightness(g15);
> + brightness = g15->leds[g15_led->led].brightness;
> + mutex_unlock(&g15->mutex);
> +
> + return brightness;
> +}
> +
> +static int lg_g710_mkey_led_set(struct led_classdev *led_cdev,
> + enum led_brightness brightness)
> +{
> + struct lg_g15_led *g15_led =
> + container_of(led_cdev, struct lg_g15_led, cdev);
> + struct lg_g15_data *g15 = dev_get_drvdata(led_cdev->dev->parent);
> + u8 val, mask = 0;
> + int i, ret;
> +
> + /* Ignore LED off on unregister / keyboard unplug */
> + if (led_cdev->flags & LED_UNREGISTERING)
> + return 0;
> +
> + mutex_lock(&g15->mutex);
> +
> + g15->transfer_buf[0] = LG_G710_FEATURE_M_KEYS_LEDS;
> +
> + for (i = LG_G15_MACRO_PRESET1; i <= LG_G15_MACRO_RECORD; i++) {
> + if (i == g15_led->led)
> + val = brightness;
> + else
> + val = g15->leds[i].brightness;
> +
> + if (val)
> + mask |= 1 << (i - LG_G15_MACRO_PRESET1 + 4);
> + }
> +
> + g15->transfer_buf[1] = mask;
> +
> + ret = hid_hw_raw_request(g15->hdev, LG_G710_FEATURE_M_KEYS_LEDS,
> + g15->transfer_buf, 2,
> + HID_FEATURE_REPORT, HID_REQ_SET_REPORT);
> + if (ret == 2) {
> + /* Success */
> + g15_led->brightness = brightness;
> + ret = 0;
> + } else {
> + hid_err(g15->hdev, "Error setting LED brightness: %d\n", ret);
> + ret = (ret < 0) ? ret : -EIO;
> + }
> +
> + mutex_unlock(&g15->mutex);
> +
> + return ret;
> +}
> +
> +static int lg_g710_kbd_led_set(struct led_classdev *led_cdev,
> + enum led_brightness brightness)
> +{
> + struct lg_g15_led *g15_led =
> + container_of(led_cdev, struct lg_g15_led, cdev);
> + struct lg_g15_data *g15 = dev_get_drvdata(led_cdev->dev->parent);
> + int ret;
> +
> + /* Ignore LED off on unregister / keyboard unplug */
> + if (led_cdev->flags & LED_UNREGISTERING)
> + return 0;
> +
> + mutex_lock(&g15->mutex);
> +
> + g15->transfer_buf[0] = LG_G710_FEATURE_BACKLIGHT;
> + g15->transfer_buf[3] = 0;
> +
> + if (g15_led->led == LG_G15_KBD_BRIGHTNESS) {
> + g15->transfer_buf[1] = 4 - g15->leds[LG_G15_LCD_BRIGHTNESS].brightness;
> + g15->transfer_buf[2] = 4 - brightness;
> + } else {
> + g15->transfer_buf[1] = 4 - brightness;
> + g15->transfer_buf[2] = 4 - g15->leds[LG_G15_KBD_BRIGHTNESS].brightness;
> + }
> +
> + ret = hid_hw_raw_request(g15->hdev, LG_G710_FEATURE_BACKLIGHT,
> + g15->transfer_buf, 4,
> + HID_FEATURE_REPORT, HID_REQ_SET_REPORT);
> + if (ret == 4) {
> + /* Success */
> + g15_led->brightness = brightness;
> + ret = 0;
> + } else {
> + hid_err(g15->hdev, "Error setting LED brightness: %d\n", ret);
> + ret = (ret < 0) ? ret : -EIO;
> + }
> +
> + mutex_unlock(&g15->mutex);
> +
> + return ret;
> +}
> +
> +/*
> + * The G710+ has separate physical keys for cycling the main keyboard backlight
> + * and the WASD backlight. The firmware handles the actual brightness change
> + * internally, but GET_REPORT always returns the power-on defaults regardless
> + * of any changes. So we must track the brightness in our cache and cycle it
> + * ourselves when a hardware brightness key press is detected.
> + *
> + * The firmware cycles brightness DOWN: 4 → 3 → 2 → 1 → 0 → 4 (in wire
> + * format where 0 = brightest, 4 = off). In user-facing terms (inverted):
> + * 4 → 3 → 2 → 1 → 0 → 4.
> + *
> + * The game mode toggle is also handled here: the firmware toggles game mode
> + * internally and updates the LED, so we read back the actual state via
> + * GET_REPORT and notify userspace of the change.
> + */
> +static void lg_g710_leds_changed_work(struct work_struct *work)
> +{
> + struct lg_g15_data *g15 = container_of(work, struct lg_g15_data, work);
> + enum led_brightness brightness[LG_G15_BRIGHTNESS_MAX];
> + bool changed[LG_G15_BRIGHTNESS_MAX] = {};
> + bool gamemode_changed;
This can be simplified a bit, drop the brightness[] array here as
well as the gamemode_changed bool and make the changed array
LG_G15_LED_MAX entries large.
> + int i;
> +
> + mutex_lock(&g15->mutex);
> + for (i = 0; i < LG_G15_BRIGHTNESS_MAX; i++) {
> + if (!test_and_clear_bit(i, &g15->brightness_changed))
> + continue;
> +
> + changed[i] = true;
> +
> + if (g15->leds[i].brightness > 0)
> + g15->leds[i].brightness--;
> + else
> + g15->leds[i].brightness =
> + g15->leds[i].cdev.max_brightness;
> +
> + brightness[i] = g15->leds[i].brightness;
No need to store in the brightness[i] array you can simply use
g15->leds[i].brightness below.
> + }
> +
> + gamemode_changed = test_and_clear_bit(LG_G15_GAMEMODE,
> + &g15->brightness_changed);
Use changed[LG_G15_GAMEMODE] here instead of the gamemode_changed bool,
replacing it twice both in the assignment and in the if below.
> + if (gamemode_changed)
> + lg_g710_update_game_led_brightness(g15);
> + mutex_unlock(&g15->mutex);
move this mutex_unlock to below the below for loop.
> +
> + for (i = 0; i < LG_G15_BRIGHTNESS_MAX; i++) {
and make this loop condition "i < LG_G15_LED_MAX"
> + if (!changed[i])
> + continue;
> +
> + led_classdev_notify_brightness_hw_changed(&g15->leds[i].cdev,
> + brightness[i]);
use g15->leds[i].brightness here instead of brightness[i]
> + }
mutex_unlock() ends up here.
> +
> + if (gamemode_changed)
> + led_classdev_notify_brightness_hw_changed(
> + &g15->leds[LG_G15_GAMEMODE].cdev,
> + g15->leds[LG_G15_GAMEMODE].brightness);
and this bit can be dropped since this is handled by the loop now :)
> +}
> +
> /******** Generic LED functions ********/
> static int lg_g15_get_initial_led_brightness(struct lg_g15_data *g15)
> {
> @@ -619,6 +861,16 @@ static int lg_g15_get_initial_led_brightness(struct lg_g15_data *g15)
> return ret;
>
> return lg_g510_update_mkey_led_brightness(g15);
> + case LG_G710:
> + ret = lg_g710_update_game_led_brightness(g15);
> + if (ret)
> + return ret;
> +
> + ret = lg_g710_update_mkey_led_brightness(g15);
> + if (ret)
> + return ret;
> +
> + return lg_g710_update_kbd_led_brightness(g15);
> case LG_Z10:
> /*
> * Getting the LCD backlight brightness is not supported.
> @@ -890,6 +1142,74 @@ static int lg_g510_leds_event(struct lg_g15_data *g15, u8 *data)
> return 0;
> }
>
> +static int lg_g710_event(struct lg_g15_data *g15, u8 *data, int size)
> +{
> + /*
> + * Bits 0-5: G1-G6 keys
> + * Bits 6-8: M1-M3 keys
> + * Bit 9: MR key
> + * Bit 10: WASD backlight cycle (handled as hw brightness change)
> + * Bit 11: Kbd backlight cycle (handled as hw brightness change)
> + * Bit 12: Game mode toggle (LED state change, handled by firmware)
> + */
> + static const u16 keymap[] = {
> + KEY_MACRO1,
> + KEY_MACRO2,
> + KEY_MACRO3,
> + KEY_MACRO4,
> + KEY_MACRO5,
> + KEY_MACRO6,
> + KEY_MACRO_PRESET1,
> + KEY_MACRO_PRESET2,
> + KEY_MACRO_PRESET3,
> + KEY_MACRO_RECORD_START,
> + 0, /* WASD illumination cycle - not a key event */
> + 0, /* Kbd illumination cycle - not a key event */
> + 0, /* Game mode toggle */
> + };
> + u16 pressed_keys, changed;
> + int i;
> +
> + if (size != 4 || data[0] != 3)
> + return 1;
> +
> + pressed_keys = (data[1] & 0x3f) | ((data[2] & 0xf0) << 2) |
> + ((data[3] & 0x7) << 10);
> + changed = pressed_keys ^ g15->pressed_keys;
> +
> + for (i = 0; i < ARRAY_SIZE(keymap); i++) {
> + if (keymap[i] && (changed & BIT(i)))
> + input_report_key(g15->input, keymap[i],
> + pressed_keys & BIT(i));
> + }
> + input_sync(g15->input);
There is no need for the changed thing / check here and thus also
no need to remember the previously pressed keys. The input subsystem
is smart enough to not send out events when keys are unchanged from
the previous input_sync() call.
> +
> + /*
> + * Detect brightness key presses (0->1 transition) and schedule
> + * the work function to cycle cached brightness and notify userspace.
> + * Bit 10 = WASD backlight (maps to LG_G15_LCD_BRIGHTNESS slot).
> + * Bit 11 = Kbd backlight (maps to LG_G15_KBD_BRIGHTNESS slot).
> + */
> + if ((changed & BIT(10)) && (pressed_keys & BIT(10))) {
> + set_bit(LG_G15_LCD_BRIGHTNESS, &g15->brightness_changed);
> + schedule_work(&g15->work);
> + }
> + if ((changed & BIT(11)) && (pressed_keys & BIT(11))) {
> + set_bit(LG_G15_KBD_BRIGHTNESS, &g15->brightness_changed);
> + schedule_work(&g15->work);
> + }
> +
> + /* Game mode toggle — bit 12 is a state bit, trigger on any change */
> + if (changed & BIT(12)) {
> + set_bit(LG_G15_GAMEMODE, &g15->brightness_changed);
> + schedule_work(&g15->work);
> + }
> +
> + g15->pressed_keys = pressed_keys;
> +
> + return 0;
> +}
> +
> static int lg_g15_raw_event(struct hid_device *hdev, struct hid_report *report,
> u8 *data, int size)
> {
> @@ -924,6 +1244,10 @@ static int lg_g15_raw_event(struct hid_device *hdev, struct hid_report *report,
> if (data[0] == LG_G510_INPUT_KBD_BACKLIGHT && size == 2)
> return lg_g510_leds_event(g15, data);
> break;
> + case LG_G710:
> + if (data[0] == 0x03 && size == 4)
> + return lg_g710_event(g15, data, size);
> + break;
> }
>
> return 0;
> @@ -1055,6 +1379,37 @@ static int lg_g15_register_led(struct lg_g15_data *g15, int i, const char *name)
> ret = devm_led_classdev_register(&g15->hdev->dev, &g15->leds[i].cdev);
> }
> break;
> + case LG_G710:
> + switch (i) {
> + case LG_G15_LCD_BRIGHTNESS:
> + /*
> + * The G710+ does not have a separate LCD brightness,
> + * but it does have a separate brightness for WASD keys.
> + * Do not use the ::kbd_backlight suffix here, UPower
> + * only supports one kbd_backlight LED per device.
> + */
> + g15->leds[i].cdev.name = "g15::kbd_zoned_backlight-wasd";
> + fallthrough;
> + case LG_G15_KBD_BRIGHTNESS:
> + g15->leds[i].cdev.brightness_set_blocking =
> + lg_g710_kbd_led_set;
> + g15->leds[i].cdev.brightness_get =
> + lg_g710_led_get;
> + g15->leds[i].cdev.max_brightness = 4;
> + g15->leds[i].cdev.flags = LED_BRIGHT_HW_CHANGED;
> + break;
> + default:
> + if (i != LG_G15_GAMEMODE)
> + g15->leds[i].cdev.brightness_set_blocking =
> + lg_g710_mkey_led_set;
> + g15->leds[i].cdev.brightness_get =
> + lg_g710_led_get;
> + g15->leds[i].cdev.max_brightness = 1;
> + if (i == LG_G15_GAMEMODE)
> + g15->leds[i].cdev.flags = LED_BRIGHT_HW_CHANGED;
> + }
> + ret = devm_led_classdev_register(&g15->hdev->dev, &g15->leds[i].cdev);
> + break;
> }
>
> return ret;
> @@ -1079,13 +1434,16 @@ static void lg_g15_init_input_dev_core(struct hid_device *hdev, struct input_dev
> static void lg_g15_init_input_dev(struct hid_device *hdev, struct input_dev *input,
> const char *name)
> {
> + struct lg_g15_data *g15 = hid_get_drvdata(hdev);
> int i;
>
> lg_g15_init_input_dev_core(hdev, input, name);
>
> - /* Keys below the LCD, intended for controlling a menu on the LCD */
> - for (i = 0; i < 5; i++)
> - input_set_capability(input, EV_KEY, KEY_KBD_LCD_MENU1 + i);
> + if (g15->model != LG_G710) {
> + /* Keys below the LCD, intended for controlling a menu on the LCD */
> + for (i = 0; i < 5; i++)
> + input_set_capability(input, EV_KEY, KEY_KBD_LCD_MENU1 + i);
> + }
Maybe instead add the following above the for-loop and
leave the for-loop as is? :
if (g15->model == LG_G710)
return;
> }
>
> static void lg_g13_init_input_dev(struct hid_device *hdev,
> @@ -1119,6 +1477,7 @@ static int lg_g15_probe(struct hid_device *hdev, const struct hid_device_id *id)
> "g15::macro_preset2",
> "g15::macro_preset3",
> "g15::macro_record",
> + "g15::gamemode",
> };
> u8 gkeys_settings_output_report = 0;
> u8 gkeys_settings_feature_report = 0;
> @@ -1137,8 +1496,8 @@ static int lg_g15_probe(struct hid_device *hdev, const struct hid_device_id *id)
> return ret;
>
> /*
> - * Some models have multiple interfaces, we want the interface with
> - * the f000.0000 application input report.
> + * Some models have multiple interfaces, we want the interface
> + * with the ff00.0000 application input report.
> */
> rep_enum = &hdev->report_enum[HID_INPUT_REPORT];
> list_for_each_entry(rep, &rep_enum->report_list, list) {
> @@ -1212,6 +1571,13 @@ static int lg_g15_probe(struct hid_device *hdev, const struct hid_device_id *id)
> case LG_Z10:
> connect_mask = HID_CONNECT_HIDRAW;
> break;
> + case LG_G710:
> + INIT_WORK(&g15->work, lg_g710_leds_changed_work);
> + hdev->quirks |= HID_QUIRK_NOGET;
> + connect_mask = HID_CONNECT_DEFAULT;
> + gkeys_settings_feature_report = LG_G710_FEATURE_EXTRA_KEYS;
> + gkeys = 6;
> + break;
> }
>
> ret = hid_hw_start(hdev, connect_mask);
> @@ -1234,11 +1600,14 @@ static int lg_g15_probe(struct hid_device *hdev, const struct hid_device_id *id)
> }
>
> if (gkeys_settings_feature_report) {
> + int report_size = ((g15->model == LG_G710) ?
> + gkeys * 2 : gkeys) + 1;
> +
The [0] byte in the transfer buffer is the report-number,
so this should be:
int report_size = (g15->model == LG_G710) ? gkeys * 2 : gkeys;
> g15->transfer_buf[0] = gkeys_settings_feature_report;
> - memset(g15->transfer_buf + 1, 0, gkeys);
> + memset(g15->transfer_buf + 1, 0, report_size - 1);
and drop the - 1 here.
> ret = hid_hw_raw_request(g15->hdev,
> gkeys_settings_feature_report,
> - g15->transfer_buf, gkeys + 1,
> + g15->transfer_buf, report_size,
and re-add / keep the + 1 here.
> HID_FEATURE_REPORT, HID_REQ_SET_REPORT);
> }
>
> @@ -1327,7 +1696,7 @@ static int lg_g15_probe(struct hid_device *hdev, const struct hid_device_id *id)
> goto error_hw_stop;
>
> /* Register LED devices */
> - for (i = 0; i < LG_G15_LED_MAX; i++) {
> + for (i = 0; i < LG_G15_LED_MAX - (g15->model != LG_G710); i++) {
> ret = lg_g15_register_led(g15, i, led_names[i]);
> if (ret)
> goto error_hw_stop;
The " - (g15->model != LG_G710)" you add here is a bit convoluted.
I would prefer if you change the for condition to:
"i <= LG_G15_MACRO_RECORD" like you've done in other places and then below
the loop add:
if (g15->model == LG_G710) {
ret = lg_g15_register_led(g15, LG_G15_GAMEMODE, "g15::gamemode");
if (ret)
goto error_hw_stop;
}
You can then also drop the adding of "g15::gamemode" to led_names[].
This is somewhat more code, but a lot easier to parse / read so IMHO
this is better in the long run keeping readability of the code in mind.
Regards,
Hans
> @@ -1366,6 +1735,10 @@ static const struct hid_device_id lg_g15_devices[] = {
> { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH,
> USB_DEVICE_ID_LOGITECH_G510_USB_AUDIO),
> .driver_data = LG_G510_USB_AUDIO },
> + /* G710 or G710+ */
> + { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH,
> + USB_DEVICE_ID_LOGITECH_G710),
> + .driver_data = LG_G710 },
> /* Z-10 speakers */
> { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH,
> USB_DEVICE_ID_LOGITECH_Z_10_SPK),
^ permalink raw reply
* Re: [PATCH] Input: gpio-keys - add hibernation support
From: Dmitry Torokhov @ 2026-04-06 18:18 UTC (permalink / raw)
To: Armando De Leon; +Cc: Armando De Leon, linux-input, linux-kernel
In-Reply-To: <20260406165833.3137128-1-learmand@amazon.com>
On Mon, Apr 06, 2026 at 09:58:06AM -0700, Armando De Leon wrote:
> Hi Dmitry,
>
> Thank you for the review.
>
> The interrupt controller (GICv3) is re-initialized by platform
> firmware during hibernate restore - this is expected behavior, not
> a bug. The IRQ trigger type (EDGE_BOTH) was originally configured
> by devm_request_any_context_irq() during probe(), which does not
> run again after hibernate restore.
>
> The TLMM/pinctrl registers are correctly saved and restored by the
> platform's syscore_ops - I verified this with register dumps. The
> issue is specifically that the IRQ trigger type configured at the
> GIC level during probe is lost and not re-applied.
>
> Should the generic IRQ core be responsible for restoring trigger
> types across hibernate? Otherwise, consumer drivers like gpio-keys need to handle
> this in their .restore callback.
I am not sure if it is job of IRQ core vs. particular interrupt
controller, but they should restore the trigger types along with
entirety of the interrupts and pins states to exactly the same condition
that they were before entering hibernation.
>
> Either way, gpio-keys currently lacks .freeze/.restore callbacks
> entirely, which is needed for proper hibernation support.
Drivers only need dedicated freeze and restore handlers if the behavior
should be different between suspend-to-ram vs suspend-to-disk. For the
vast majority of the drivers they behave exactly the same and I do not
see why it would be different for gpio-keys.
Thanks.
--
Dmitry
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox