* RE: [PATCH v3 0/6] iio: hid-sensor: standardize scan_type initialization
From: Zhang, Lixu @ 2026-05-14 2:44 UTC (permalink / raw)
To: Jonathan Cameron, Natália Salvino André
Cc: andy@kernel.org, Lechner, David, jikos@kernel.org,
nuno.sa@analog.com, srinivas.pandruvada@linux.intel.com,
linux-iio@vger.kernel.org, linux-input@vger.kernel.org
In-Reply-To: <20260512154853.10aa2902@jic23-huawei>
>-----Original Message-----
>From: Zhang, Lixu
>Sent: Wednesday, May 13, 2026 11:27 AM
>To: Jonathan Cameron <jic23@kernel.org>; Natália Salvino André
><natalia.andre@ime.usp.br>
>Cc: andy@kernel.org; Lechner, David <dlechner@baylibre.com>;
>jikos@kernel.org; nuno.sa@analog.com; srinivas.pandruvada@linux.intel.com;
>linux-iio@vger.kernel.org; linux-input@vger.kernel.org
>Subject: RE: [PATCH v3 0/6] iio: hid-sensor: standardize scan_type initialization
>
>>-----Original Message-----
>>From: Jonathan Cameron <jic23@kernel.org>
>>Sent: Tuesday, May 12, 2026 10:49 PM
>>To: Natália Salvino André <natalia.andre@ime.usp.br>
>>Cc: andy@kernel.org; Lechner, David <dlechner@baylibre.com>;
>>jikos@kernel.org; nuno.sa@analog.com;
>>srinivas.pandruvada@linux.intel.com;
>>linux-iio@vger.kernel.org; linux-input@vger.kernel.org; Zhang, Lixu
>><lixu.zhang@intel.com>
>>Subject: Re: [PATCH v3 0/6] iio: hid-sensor: standardize scan_type
>>initialization
>>
>>On Mon, 11 May 2026 22:16:16 -0300
>>Natália Salvino André <natalia.andre@ime.usp.br> wrote:
>>
>>> This series refactors the HID sensor drivers to standardize the
>>> initialization of the iio_chan_spec scan_type structure.
>>> Direct initialization using compound literals was used.
>>>
>>> This change improves code readability and ensures that all fields of
>>> the scan_type structure are properly zero-initialized.
>>> Additionally, for the hid-sensor-accel-3d driver, the channel
>>> initialization loop was refactored to iterate directly over the scan
>>> indices, eliminating redundant logic.
>>
>>Series looks good to me but I'll leave it on the list a while to both
>>let sashiko catch up and anyone else who wants to can have time to
>>look. Also ideally get some testing.
>>
>>Lixu, if you don't mind, please can you give this a test on top of the
>>more radical refactor patches?
>
>Sure, I'll test this series on top of the refactor patches when I have some time.
Hi Jonathan,
I've done some basic testing on this series. Here are the results:
1. No issues seen for hid-sensor-gyro-3d, hid-sensor-als, hid-sensor-prox, hid-sensor-magn-3d.
2. Didn't test hid-sensor-press as I couldn't find a platform with the corresponding device.
3. There's an issue with hid-sensor-accel-3d. Sashiko AI review has already identified the problem and provided review comments.
Thanks,
Lixu
^ permalink raw reply
* [PATCH v2] Input: synaptics - add LEN2058 to SMBus passlist for ThinkPad E490
From: contacto @ 2026-05-14 1:35 UTC (permalink / raw)
To: dmitry.torokhov; +Cc: linux-input, linux-kernel, Nicolás Bazaes
In-Reply-To: <agTso46UcMSGaIYx@google.com>
From: Nicolás Bazaes <contacto@bazaes.cl>
The Lenovo ThinkPad E490 (PNP ID: LEN2058) has a Synaptics TM3471-020
touchpad that supports SMBus/RMI4 mode but is not listed in
smbus_pnp_ids[]. Without this entry, RMI4 over SMBus is not enabled
by default, and the touchpad falls back to PS/2 mode.
Adding LEN2058 to the passlist enables automatic RMI4 detection without
requiring the psmouse.synaptics_intertouch parameter, and matches
the behavior of similar ThinkPad models already in the list
(E480/LEN2054, E580/LEN2055).
Tested on ThinkPad E490 with kernel 7.0.5-zen1 and Arch Linux.
RMI4 over SMBus is confirmed working without any kernel parameters.
Signed-off-by: Nicolás Bazaes <contacto@bazaes.cl>
Assisted-by: Claude:claude-sonnet-4-6
---
drivers/input/mouse/synaptics.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
index 26071128f..c70502e24 100644
--- a/drivers/input/mouse/synaptics.c
+++ b/drivers/input/mouse/synaptics.c
@@ -190,6 +190,7 @@ static const char * const smbus_pnp_ids[] = {
"LEN2044", /* L470 */
"LEN2054", /* E480 */
"LEN2055", /* E580 */
+ "LEN2058", /* E490 */
"LEN2068", /* T14 Gen 1 */
"SYN1221", /* TUXEDO InfinityBook Pro 14 v5 */
"SYN3003", /* HP EliteBook 850 G1 */
--
2.54.0
^ permalink raw reply related
* Re: [PATCH] Input: synaptics - add LEN2058 to SMBus passlist for ThinkPad E490
From: sashiko-bot @ 2026-05-14 1:22 UTC (permalink / raw)
To: contacto; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260513045749.185969-1-contacto@bazaes.cl>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The commit message incorrectly states that without the passlist entry, psmouse ignores the `synaptics_intertouch` parameter and falls back to PS/2 mode even when explicitly requested.
--
commit 18925dd22ddb975fc9e545765ec54eb77a26f6f6
Author: Nicolás Bazaes <contacto@bazaes.cl>
Input: synaptics - add LEN2058 to SMBus passlist for ThinkPad E490
The Lenovo ThinkPad E490 has a Synaptics TM3471-020 touchpad that supports
SMBus/RMI4 mode but is not listed in smbus_pnp_ids[]. Adding LEN2058 to
the passlist enables RMI4 over SMBus, which provides better multitouch
support and matches the behavior of similar ThinkPad models.
> Without this entry, psmouse ignores the
> synaptics_intertouch parameter and falls back to PS/2 mode even when
> explicitly requested.
This isn't a bug, but is this description accurate?
Looking at synaptics_setup_intertouch(), the passlist is only checked if
synaptics_intertouch == SYNAPTICS_INTERTOUCH_NOT_SET:
drivers/input/mouse/synaptics.c:synaptics_setup_intertouch() {
...
if (synaptics_intertouch == SYNAPTICS_INTERTOUCH_NOT_SET) {
if (!psmouse_matches_pnp_id(psmouse, topbuttonpad_pnp_ids) &&
!psmouse_matches_pnp_id(psmouse, smbus_pnp_ids)) {
...
}
If a user explicitly requests SMBus mode by passing
psmouse.synaptics_intertouch=1, the condition is false, bypassing the
passlist check entirely and honoring the user's request.
Could the commit message be updated to clarify that the parameter is not
ignored when explicitly requested?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260513045749.185969-1-contacto@bazaes.cl?part=1
^ permalink raw reply
* Re: [PATCH v5 3/3] HID: nintendo: Add unified report format support
From: sashiko-bot @ 2026-05-14 1:06 UTC (permalink / raw)
To: Vicki Pfau; +Cc: linux-input, dmitry.torokhov
In-Reply-To: <20260512200051.2534081-4-vi@endrift.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Insufficient bounds checking for NS2_REPORT_UNIFIED payload leads to out-of-bounds reads.
- [High] Divide-by-zero kernel panic in switch2_report_trigger() caused by attacker-controlled calibration data.
--
commit 6e45b564fde8014fc6ade8452a1b793097b3bee4
Author: Vicki Pfau <vi@endrift.com>
Subject: HID: nintendo: Add unified report format support
This commit adds support for the "unified" report format to the nintendo hid
driver. It introduces new mappings for overlapping button and axis fields
across different controller types.
> diff --git a/drivers/hid/hid-nintendo.c b/drivers/hid/hid-nintendo.c
> index 9801799a90758..9e7eda7b70bfb 100644
> --- a/drivers/hid/hid-nintendo.c
> +++ b/drivers/hid/hid-nintendo.c
[ ... ]
> @@ -3690,11 +3791,48 @@ static int switch2_event(struct hid_device *hdev, struct hid_report *report, uin
>
> switch (report->id) {
> case NS2_REPORT_UNIFIED:
> - /*
> - * TODO
> - * This won't be sent unless the report type gets changed via command
> - * 03-0A, but we should support it at some point regardless.
> - */
> + switch (ns2->ctlr_type) {
> + case NS2_CTLR_TYPE_JCL:
> + switch2_report_stick(input, &ns2->stick_calib[0],
> + ABS_X, false, ABS_Y, true, &raw_data[11]);
> + switch2_report_buttons(input, &raw_data[5],
> + ns2_left_joycon_button_unified_mappings);
> + break;
> + case NS2_CTLR_TYPE_JCR:
> + switch2_report_stick(input, &ns2->stick_calib[0],
> + ABS_X, false, ABS_Y, true, &raw_data[14]);
Is this safe if the report size is exactly 15 bytes?
Earlier in switch2_event(), there is a global size check:
if (size < 15)
return -EINVAL;
However, switch2_report_stick() reads up to 3 bytes from the provided data
pointer (indices 0, 1, and 2). Passing &raw_data[14] means it will read
raw_data[14], raw_data[15], and raw_data[16].
Could this cause an out-of-bounds read if an undersized report is sent?
> + switch2_report_buttons(input, &raw_data[5],
> + ns2_right_joycon_button_unified_mappings);
> + break;
> + case NS2_CTLR_TYPE_GC:
> + input_report_abs(input, ABS_HAT0X,
> + !!(raw_data[7] & NS2_BTN_U3_RIGHT) -
> + !!(raw_data[7] & NS2_BTN_U3_LEFT));
> + input_report_abs(input, ABS_HAT0Y,
> + !!(raw_data[7] & NS2_BTN_U3_DOWN) -
> + !!(raw_data[7] & NS2_BTN_U3_UP));
> + switch2_report_buttons(input, &raw_data[5], ns2_gccon_unified_mappings);
> + switch2_report_stick(input, &ns2->stick_calib[0],
> + ABS_X, false, ABS_Y, true, &raw_data[11]);
> + switch2_report_stick(input, &ns2->stick_calib[1],
> + ABS_RX, false, ABS_RY, true, &raw_data[14]);
> + switch2_report_trigger(input, ns2->lt_zero, ABS_Z, raw_data[0x3d]);
> + switch2_report_trigger(input, ns2->rt_zero, ABS_RZ, raw_data[0x3e]);
Similarly, does this code read past the end of the buffer?
Accessing raw_data[0x3d] (index 61) and raw_data[0x3e] (index 62) requires
the report size to be at least 63 bytes, but the global check only guarantees
15 bytes.
Additionally, could this new path trigger a divide-by-zero panic?
While this issue pre-dates this commit, switch2_report_trigger() contains the
following calculation:
int value = (NS2_TRIGGER_RANGE + 1) * (data - zero) / (232 - zero);
The zero parameter here comes from the device's factory calibration data
(ns2->lt_zero and ns2->rt_zero). If a device advertises 232 as its zero
calibration point, this division evaluates to 0.
Since this patch exposes a new way to call switch2_report_trigger(), might it
be worth adding validation to prevent a zero divisor?
> + break;
> + case NS2_CTLR_TYPE_PRO:
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260512200051.2534081-1-vi@endrift.com?part=3
^ permalink raw reply
* Re: [PATCH v5 2/3] HID: nintendo: Add rumble support for Switch 2 controllers
From: sashiko-bot @ 2026-05-14 0:53 UTC (permalink / raw)
To: Vicki Pfau; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260512200051.2534081-3-vi@endrift.com>
Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [High] Mixing jiffies and milliseconds in delayed work scheduling logic causes an integer underflow, permanently stalling the haptic rumble.
- [High] A guaranteed NULL pointer dereference occurs in the error logging path of switch2_rumble_work, as well as a TOCTOU race condition.
- [Medium] `ns2->ctlr_type` is checked before it is dynamically initialized, causing union corruption for GameCube controllers.
- [Low] Implicit type promotion in the max() macro causes a compiler warning.
- [Low] Kconfig documentation claims individual motor control for the Pro Controller, but the implementation identicalizes the rumble data for both motors.
--
commit d72f0f8e0dadde2cacc27050b92088c2e5cbea0e
Author: Vicki Pfau <vi@endrift.com>
Subject: HID: nintendo: Add rumble support for Switch 2 controllers
This commit introduces rumble support for Switch 2 controllers, handling both HD
Rumble linear resonant actuators and eccentric rotating mass haptics. It adds
state machines for rumble scheduling and output reporting via hdev.
> diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
> index 26ef589f652c6..a7fe74d982475 100644
> --- a/drivers/hid/Kconfig
> +++ b/drivers/hid/Kconfig
> @@ -850,10 +850,10 @@ config NINTENDO_FF
> depends on HID_NINTENDO
> select INPUT_FF_MEMLESS
> help
> - Say Y here if you have a Nintendo Switch controller and want to enable
> - force feedback support for it. This works for both joy-cons, the pro
> - controller, and the NSO N64 controller. For the pro controller, both
> - rumble motors can be controlled individually.
> + Say Y here if you have a Nintendo Switch or Switch 2 controller and want
> + to enable force feedback support for it. This works for Joy-Cons, the Pro
> + Controllers, and the NSO N64 and GameCube controller. For the Pro
> + Controller, both rumble motors can be controlled individually.
This isn't a bug, but is the help text accurate regarding individual motor
control? The implementation in switch2_rumble_work() appears to encode the
exact same rumble configuration to both the left and right motor offsets,
which would make both motors vibrate identically.
> diff --git a/drivers/hid/hid-nintendo.c b/drivers/hid/hid-nintendo.c
> index 2193afcdefabc..9801799a90758 100644
> --- a/drivers/hid/hid-nintendo.c
> +++ b/drivers/hid/hid-nintendo.c
[ ... ]
> @@ -3103,6 +3155,107 @@ static inline bool switch2_ctlr_is_joycon(enum switch2_ctlr_type type)
> return type == NS2_CTLR_TYPE_JCL || type == NS2_CTLR_TYPE_JCR;
> }
>
> +#if IS_ENABLED(CONFIG_NINTENDO_FF)
> +static void switch2_encode_rumble(struct switch2_hd_rumble *rumble, uint8_t buffer[5])
> +{
> + buffer[0] = rumble->hi_freq;
> + buffer[1] = (rumble->hi_freq >> 8) | (rumble->hi_amp << 2);
> + buffer[2] = (rumble->hi_amp >> 6) | (rumble->lo_freq << 4);
> + buffer[3] = (rumble->lo_freq >> 4) | (rumble->lo_amp << 6);
> + buffer[4] = rumble->lo_amp >> 2;
> +}
> +
> +static int switch2_play_effect(struct input_dev *dev, void *data, struct ff_effect *effect)
> +{
> + struct switch2_controller *ns2 = input_get_drvdata(dev);
> +
> + if (effect->type != FF_RUMBLE)
> + return 0;
> +
> + guard(spinlock_irqsave)(&ns2->rumble_lock);
> + if (ns2->ctlr_type == NS2_CTLR_TYPE_GC) {
> + ns2->rumble.sd.amplitude = max(effect->u.rumble.strong_magnitude,
> + effect->u.rumble.weak_magnitude >> 1);
Will this max calculation trigger a compiler warning? The strict type checking
in the max macro might fail because effect->u.rumble.weak_magnitude >> 1
promotes to an int, while strong_magnitude is a u16.
> + } else {
> + ns2->rumble.hd.hi_amp = effect->u.rumble.weak_magnitude * RUMBLE_MAX >> 16;
> + ns2->rumble.hd.lo_amp = effect->u.rumble.strong_magnitude * RUMBLE_MAX >> 16;
> + }
> +
> + if (ns2->hdev)
> + schedule_delayed_work(&ns2->rumble_work, 0);
> +
> + return 0;
> +}
> +
> +static void switch2_rumble_work(struct work_struct *work)
> +{
> + struct switch2_controller *ns2 = container_of(to_delayed_work(work),
> + struct switch2_controller, rumble_work);
> + unsigned long current_ms = jiffies_to_msecs(get_jiffies_64());
> + unsigned long flags;
> + bool active;
> + int ret;
[ ... ]
> + if (active) {
> + unsigned long interval = msecs_to_jiffies(4);
> +
> + if (!ns2->last_rumble_work)
> + ns2->last_rumble_work = current_ms;
> + else
> + ns2->last_rumble_work += interval;
> + schedule_delayed_work(&ns2->rumble_work,
> + ns2->last_rumble_work + interval - current_ms);
Does this code underflow the scheduled delay?
The current_ms variable is retrieved in milliseconds using jiffies_to_msecs(),
but interval is calculated in jiffies using msecs_to_jiffies(). Since
ns2->last_rumble_work is initialized to milliseconds but incremented by
jiffies, subtracting current_ms could yield a negative value.
On systems where HZ is not 1000, could this underflow the delay parameter and
schedule the next work millions of jiffies in the future?
> + } else {
> + ns2->last_rumble_work = 0;
> + }
> + spin_unlock_irqrestore(&ns2->rumble_lock, flags);
> +
> + if (!ns2->hdev) {
> + cancel_delayed_work(&ns2->rumble_work);
> + ret = -ENODEV;
> + } else {
> + ret = hid_hw_output_report(ns2->hdev, ns2->rumble_buffer, 64);
> + }
> +
> + if (ret < 0)
> + hid_dbg(ns2->hdev, "Failed to send output report ret=%d\n", ret);
If ns2->hdev is NULL, we set ret = -ENODEV and then reach this hid_dbg call.
Could this result in a NULL pointer dereference, as the hid_dbg macro passes
ns2->hdev to dev_dbg()?
Also, since ns2->hdev is accessed locklessly and without READ_ONCE(), is there
a time-of-check to time-of-use race where the compiler might reload the pointer
between the check and hid_hw_output_report()?
[ ... ]
> @@ -3881,6 +4048,14 @@ static int switch2_probe(struct hid_device *hdev, const struct hid_device_id *id
>
> switch2_leds_create(ns2);
>
> +#if IS_ENABLED(CONFIG_NINTENDO_FF)
> + if (ns2->ctlr_type != NS2_CTLR_TYPE_GC) {
> + ns2->rumble.hd.hi_freq = RUMBLE_HI_FREQ;
> + ns2->rumble.hd.lo_freq = RUMBLE_LO_FREQ;
> + }
Are we checking ns2->ctlr_type before it is dynamically initialized?
During probe, ns2->ctlr_type appears to be 0 since the struct is freshly
allocated and the type is discovered later during the init state machine.
This would cause the high and low frequencies to always be written.
Since hd and sd share a union, could this corrupt the sd.error and
sd.amplitude fields for actual GameCube controllers?
> + spin_lock_init(&ns2->rumble_lock);
> + INIT_DELAYED_WORK(&ns2->rumble_work, switch2_rumble_work);
> +#endif
> hid_set_drvdata(hdev, ns2);
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260512200051.2534081-1-vi@endrift.com?part=2
^ permalink raw reply
* Re: [PATCH v5 1/3] HID: nintendo: Add preliminary Switch 2 controller driver
From: sashiko-bot @ 2026-05-14 0:15 UTC (permalink / raw)
To: Vicki Pfau; +Cc: linux-input, dmitry.torokhov
In-Reply-To: <20260512200051.2534081-2-vi@endrift.com>
Thank you for your contribution! Sashiko AI review found 11 potential issue(s) to consider:
- [High] Sleeping in atomic context and AB-BA deadlock in `switch2_usb_disconnect`
- [High] Race condition in IN URB tracking causes permanent starvation
- [High] Dangling pointers and use-after-free on probe failure
- [High] Memory corruption and double free due to incorrect devm input lifecycle management
- [High] NULL pointer dereference in USB probe error path
- [High] Division by zero crash from unvalidated trigger calibration data
- [High] BUG_ON kernel panic when `ida_free` is called with U32_MAX
- [High] Out-of-bounds read when parsing NS2_CMD_FW_INFO response
- [High] Race condition due to URB submission before initialization of synchronization primitives
- [Medium] Infinite USB retry storm on protocol errors
- [Medium] Unaligned memory access on byte array casts
--
commit 00cb7265ad5983144c6923702e2ee2c3c047d29e
Author: Vicki Pfau <vi@endrift.com>
Subject: HID: nintendo: Add preliminary Switch 2 controller driver
This adds a new driver for the Switch 2 controllers. The Switch 2 uses an
unusual split-interface design such that input and rumble occur on the main
HID interface, but all other communication occurs over a "configuration"
interface.
> diff --git a/drivers/hid/hid-nintendo.c b/drivers/hid/hid-nintendo.c
> index 29008c2cc5304..2193afcdefabc 100644
> --- a/drivers/hid/hid-nintendo.c
> +++ b/drivers/hid/hid-nintendo.c
[ ... ]
> +static int switch2_init_input(struct switch2_controller *ns2)
> +{
[ ... ]
> + input = devm_input_allocate_device(&hdev->dev);
> + if (!input)
> + return -ENOMEM;
[ ... ]
> + ret = input_register_device(input);
> + if (ret < 0) {
> + hid_err(ns2->hdev, "Failed to register input; ret=%d\n", ret);
> + input_free_device(input);
Calling input_free_device or input_unregister_device on a devm-managed
input device can cause double frees when devres cleanup fires. Can this
lead to memory corruption?
> + return ret;
> + }
[ ... ]
> +static void switch2_controller_put(struct switch2_controller *ns2)
> +{
> + struct input_dev *input;
> + bool do_free;
[ ... ]
> + if (input)
> + input_unregister_device(input);
Calling input_unregister_device on a devm-managed input device can cause
issues. If the input device remains alive (tied to the HID device devres)
and its child LEDs are not released, will reconnecting the USB cable
allocate a new input device and corrupt the LED lists by re-registering
the exact same ns2->leds structures?
> +
> + if (do_free) {
[ ... ]
> +static void switch2_report_trigger(struct input_dev *input, uint8_t zero, int abs, uint8_t data)
> +{
> + int value = (NS2_TRIGGER_RANGE + 1) * (data - zero) / (232 - zero);
The zero variable is loaded directly from the controller's factory calibration
flash. If a device reports a zero point of exactly 232, will this trigger a
division by zero exception and crash the kernel?
> +
> + input_report_abs(input, abs, clamp(value, 0, NS2_TRIGGER_RANGE));
> +}
[ ... ]
> +static int switch2_read_flash(struct switch2_controller *ns2, uint32_t address,
> + uint8_t size)
> +{
> + uint8_t message[8] = { size, 0x7e };
> +
> + if (!ns2->cfg)
> + return -ENOTCONN;
> + *(__le32 *)&message[4] = __cpu_to_le32(address);
The driver directly casts &message[4] to __le32. Since message is a uint8_t
array, it is only guaranteed to be 1-byte aligned. On strict architectures,
will this unaligned memory write trigger a hardware alignment fault?
> + return ns2->cfg->send_command(NS2_CMD_FLASH, NS2_SUBCMD_FLASH_READ, message,
> + sizeof(message), ns2->cfg);
> +}
[ ... ]
> +int switch2_receive_command(struct switch2_controller *ns2,
> + const uint8_t *message, size_t length)
> +{
[ ... ]
> + header = (const struct switch2_cmd_header *)message;
> + if (!(header->flags & NS2_FLAG_OK)) {
> + ret = -EIO;
> + goto exit;
> + }
> + message = &message[8];
> + switch (header->command) {
[ ... ]
> + case NS2_CMD_FW_INFO:
> + if (header->subcommand == NS2_SUBCMD_FW_INFO_GET) {
> + if (length < sizeof(ns2->version)) {
> + ret = -EINVAL;
> + goto exit;
> + }
> + memcpy(&ns2->version, message, sizeof(ns2->version));
The message pointer is advanced by 8 bytes to skip the header, but length
is not decremented. The bounds check compares length against
sizeof(ns2->version). If length is exactly sizeof(ns2->version), the check
passes, but the advanced message pointer only has length - 8 valid bytes
remaining. Will the memcpy over-read into uninitialized memory?
> + ns2->ctlr_type = ns2->version.ctlr_type;
> + switch2_init_step_done(ns2, NS2_INIT_GET_FIRMWARE_INFO);
> + }
> + break;
> + default:
> + break;
> + }
> +
> +exit:
> + if (ns2->init_step < NS2_INIT_DONE)
> + switch2_init_controller(ns2);
If a command response has the NACK flag set or encounters another error,
the code jumps to exit and invokes switch2_init_controller. Since
ns2->init_step has not advanced, the state machine will immediately re-send
the exact same command. Can this cause an infinite tight feedback loop of
failures and retries, flooding the USB bus?
> +
> + return ret;
> +}
[ ... ]
> +int switch2_controller_attach_cfg(const char *phys, struct switch2_cfg_intf *cfg)
> +{
[ ... ]
> + guard(mutex)(&ns2->lock);
> + WARN_ON(ns2->cfg);
> + ns2->cfg = cfg;
> +
> + if (ns2->hdev)
> + return switch2_init_controller(ns2);
If switch2_init_controller fails here or in switch2_probe, the error is
returned immediately without unrolling the state. Will ns2->cfg or ns2->hdev
remain set, potentially leading to a use-after-free when subsequent device
events occur?
> + return 0;
> +}
[ ... ]
> +static void switch2_remove(struct hid_device *hdev)
> +{
> + struct switch2_controller *ns2 = hid_get_drvdata(hdev);
> +
> + hid_hw_close(hdev);
> + mutex_lock(&ns2->lock);
> + WARN_ON(ns2->hdev != hdev);
> + ns2->hdev = NULL;
> + mutex_unlock(&ns2->lock);
> + ida_free(&nintendo_player_id_allocator, ns2->player_id);
If ida_alloc fails during probe, ns2->player_id remains at its default
value of U32_MAX. When ida_free is called, the IDA framework enforces a
check on the ID. Casting U32_MAX to a signed integer yields -1. Will this
trigger the BUG_ON and cause a kernel panic?
> + switch2_controller_put(ns2);
> + hid_hw_stop(hdev);
> +}
[ ... ]
> diff --git a/drivers/input/joystick/nintendo-switch2-usb.c b/drivers/input/joystick/nintendo-switch2-usb.c
> new file mode 100644
> index 0000000000000..ebd89d852e21a
> --- /dev/null
> +++ b/drivers/input/joystick/nintendo-switch2-usb.c
[ ... ]
> +static void switch2_bulk_in(struct urb *urb)
> +{
[ ... ]
> + spin_lock_irqsave(&ns2_usb->bulk_in_lock, flags);
> + for (i = 0; i < NS2_IN_URBS; i++) {
> + int err;
> + struct switch2_urb *ns2_urb;
> +
> + if (ns2_usb->bulk_in[i].urb == urb) {
> + ns2_usb->message_in = i;
If multiple URBs complete in quick succession before the work item executes,
the message_in index is overwritten. Will this cause the work item to only
process the last completed URB, leaving earlier URBs indefinitely active
and leading to starvation of the URB pool?
> + continue;
> + }
[ ... ]
> +static int switch2_usb_probe(struct usb_interface *intf, const struct usb_device_id *id)
> +{
[ ... ]
> + ns2_usb->bulk_in[0].active = true;
> + ret = usb_submit_urb(ns2_usb->bulk_in[0].urb, GFP_ATOMIC);
> + if (ret < 0)
> + goto err_free_out;
> +
> + init_usb_anchor(&ns2_usb->bulk_out_anchor);
> + spin_lock_init(&ns2_usb->bulk_out_lock);
> + init_usb_anchor(&ns2_usb->bulk_in_anchor);
> + spin_lock_init(&ns2_usb->bulk_in_lock);
> + INIT_WORK(&ns2_usb->message_in_work, switch2_usb_message_in_work);
usb_submit_urb is invoked for bulk_in[0] before the initialization of the
spinlocks, anchors, and work struct. If the URB completes immediately, the
completion handler will execute and attempt to use uninitialized
synchronization primitives. Can this cause a crash?
> +
> + usb_set_intfdata(intf, ns2_usb);
[ ... ]
> +err_kill_urb:
> + usb_kill_urb(ns2_usb->bulk_in[0].urb);
> +err_free_out:
> + for (i = 0; i < NS2_OUT_URBS; i++) {
> + usb_free_coherent(ns2_usb->udev, NS2_BULK_SIZE, ns2_usb->bulk_out[i].data,
> + ns2_usb->bulk_out[i].urb->transfer_dma);
> + usb_free_urb(ns2_usb->bulk_out[i].urb);
> + }
> +err_free_in:
> + for (i = 0; i < NS2_IN_URBS; i++) {
> + usb_free_coherent(ns2_usb->udev, NS2_BULK_SIZE, ns2_usb->bulk_in[i].data,
> + ns2_usb->bulk_in[i].urb->transfer_dma);
If usb_alloc_urb fails during the loop, it jumps to err_free_in. During
cleanup, it accesses ns2_usb->bulk_in[i].urb->transfer_dma. Since urb is
NULL for the failed index, will this trigger a NULL pointer dereference?
> + usb_free_urb(ns2_usb->bulk_in[i].urb);
> + }
> + devm_kfree(&intf->dev, ns2_usb);
> +
> + return ret;
> +}
> +
> +static void switch2_usb_disconnect(struct usb_interface *intf)
> +{
> + struct switch2_usb *ns2_usb = usb_get_intfdata(intf);
> + unsigned long flags;
> + int i;
> +
> + spin_lock_irqsave(&ns2_usb->bulk_out_lock, flags);
> + usb_kill_anchored_urbs(&ns2_usb->bulk_out_anchor);
Both usb_kill_anchored_urbs and cancel_work_sync can sleep. Is it safe to
call them while holding a spinlock?
> + for (i = 0; i < NS2_OUT_URBS; i++) {
> + usb_free_coherent(ns2_usb->udev, NS2_BULK_SIZE, ns2_usb->bulk_out[i].data,
> + ns2_usb->bulk_out[i].urb->transfer_dma);
> + usb_free_urb(ns2_usb->bulk_out[i].urb);
> + }
> + spin_unlock_irqrestore(&ns2_usb->bulk_out_lock, flags);
> +
> + spin_lock_irqsave(&ns2_usb->bulk_in_lock, flags);
> + usb_kill_anchored_urbs(&ns2_usb->bulk_in_anchor);
> + cancel_work_sync(&ns2_usb->message_in_work);
Furthermore, cancel_work_sync waits for message_in_work which attempts to
acquire bulk_in_lock. Does this cause a deadlock since the lock is already
held by the disconnecting thread?
> + for (i = 0; i < NS2_IN_URBS; i++) {
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260512200051.2534081-1-vi@endrift.com?part=1
^ permalink raw reply
* [PATCH v2 4/4] HID: hid-msi: Add Rumble Intensity Attributes
From: Derek J. Clark @ 2026-05-13 23:14 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: Pierre-Loup A . Griffais, Denis Benato, Zhouwang Huang,
Derek J . Clark, linux-input, linux-doc, linux-kernel
In-Reply-To: <20260513231445.3213501-1-derekjohn.clark@gmail.com>
Adds intensity adjustment for the left and right rumble motors.
Claude was used during the reverse-engineering data gathering for this
feature done by Zhouwang Huang. As the code had already been affected,
I used Claude to create the initial framing for the feature, then did
manual cleanup of the _show and _store functions afterwards to fix bugs
and keep the coding style consistent. Claude was also used as an initial
reviewer of this patch.
Assisted-by: Claude:claude-sonnet-4-6
Co-developed-by: Zhouwang Huang <honjow311@gmail.com>
Signed-off-by: Zhouwang Huang <honjow311@gmail.com>
Signed-off-by: Derek J. Clark <derekjohn.clark@gmail.com>
---
v2:
- Use pending_profile and sync to rom mutexes.
---
drivers/hid/hid-msi.c | 147 ++++++++++++++++++++++++++++++++++++++++++
1 file changed, 147 insertions(+)
diff --git a/drivers/hid/hid-msi.c b/drivers/hid/hid-msi.c
index a628b77bfb7b5..cffd6ed253ec9 100644
--- a/drivers/hid/hid-msi.c
+++ b/drivers/hid/hid-msi.c
@@ -76,6 +76,8 @@ enum claw_profile_ack_pending {
CLAW_M1_PENDING,
CLAW_M2_PENDING,
CLAW_RGB_PENDING,
+ CLAW_RUMBLE_LEFT_PENDING,
+ CLAW_RUMBLE_RIGHT_PENDING,
};
enum claw_key_index {
@@ -262,6 +264,11 @@ static const u16 button_mapping_addr_new[] = {
static const u16 rgb_addr_old = 0x01fa;
static const u16 rgb_addr_new = 0x024a;
+static const u16 rumble_addr[] = {
+ 0x0022, /* left */
+ 0x0023, /* right */
+};
+
struct claw_command_report {
u8 report_id;
u8 padding[2];
@@ -310,7 +317,10 @@ struct claw_drvdata {
enum claw_gamepad_mode_index gamepad_mode;
u8 m1_codes[CLAW_KEYS_MAX];
u8 m2_codes[CLAW_KEYS_MAX];
+ u8 rumble_intensity_right;
+ u8 rumble_intensity_left;
const u16 *bmap_addr;
+ bool rumble_support;
bool bmap_support;
/* RGB Variables */
@@ -402,6 +412,12 @@ static int claw_profile_event(struct claw_drvdata *drvdata, struct claw_command_
memcpy(&drvdata->rgb_frames[f_idx], &frame->zone_data,
sizeof(struct rgb_frame));
+ break;
+ case CLAW_RUMBLE_LEFT_PENDING:
+ drvdata->rumble_intensity_left = cmd_rep->data[4];
+ break;
+ case CLAW_RUMBLE_RIGHT_PENDING:
+ drvdata->rumble_intensity_right = cmd_rep->data[4];
break;
default:
dev_warn(&drvdata->hdev->dev,
@@ -815,6 +831,126 @@ static ssize_t button_mapping_options_show(struct device *dev,
}
static DEVICE_ATTR_RO(button_mapping_options);
+static ssize_t rumble_intensity_left_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ u8 data[] = { 0x01, (rumble_addr[0] >> 8) & 0xff, rumble_addr[0] & 0xff, 0x01, 0x00 };
+ struct hid_device *hdev = to_hid_device(dev);
+ struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
+ u8 val;
+ int ret;
+
+ ret = kstrtou8(buf, 10, &val);
+ if (ret)
+ return ret;
+
+ if (val > 100)
+ return -EINVAL;
+
+ data[4] = val;
+
+ guard(mutex)(&drvdata->rom_mutex);
+ ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_WRITE_PROFILE_DATA,
+ data, ARRAY_SIZE(data), 8);
+ if (ret)
+ return ret;
+
+ ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_SYNC_TO_ROM, NULL, 0, 8);
+ if (ret)
+ return ret;
+
+ drvdata->rumble_intensity_left = val;
+
+ return count;
+}
+
+static ssize_t rumble_intensity_left_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ u8 data[4] = { 0x01, (rumble_addr[0] >> 8) & 0xff, rumble_addr[0] & 0xff, 0x01 };
+ struct hid_device *hdev = to_hid_device(dev);
+ struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
+ int ret;
+
+ guard(mutex)(&drvdata->profile_mutex);
+ drvdata->profile_pending = CLAW_RUMBLE_LEFT_PENDING;
+ ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_READ_PROFILE, data,
+ ARRAY_SIZE(data), 8);
+ if (ret) {
+ drvdata->profile_pending = CLAW_NO_PENDING;
+ return ret;
+ }
+
+ return sysfs_emit(buf, "%u\n", drvdata->rumble_intensity_left);
+}
+static DEVICE_ATTR_RW(rumble_intensity_left);
+
+static ssize_t rumble_intensity_right_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ u8 data[] = { 0x01, (rumble_addr[1] >> 8) & 0xff, rumble_addr[1] & 0xff, 0x01, 0x00 };
+ struct hid_device *hdev = to_hid_device(dev);
+ struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
+ u8 val;
+ int ret;
+
+ ret = kstrtou8(buf, 10, &val);
+ if (ret)
+ return ret;
+
+ if (val > 100)
+ return -EINVAL;
+
+ data[4] = val;
+
+ guard(mutex)(&drvdata->rom_mutex);
+ ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_WRITE_PROFILE_DATA,
+ data, ARRAY_SIZE(data), 8);
+ if (ret)
+ return ret;
+
+ ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_SYNC_TO_ROM, NULL, 0, 8);
+ if (ret)
+ return ret;
+
+ drvdata->rumble_intensity_right = val;
+
+ return count;
+}
+
+static ssize_t rumble_intensity_right_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ u8 data[4] = { 0x01, (rumble_addr[1] >> 8) & 0xff, rumble_addr[1] & 0xff, 0x01 };
+ struct hid_device *hdev = to_hid_device(dev);
+ struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
+ int ret;
+
+ guard(mutex)(&drvdata->profile_mutex);
+ drvdata->profile_pending = CLAW_RUMBLE_RIGHT_PENDING;
+ ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_READ_PROFILE, data,
+ ARRAY_SIZE(data), 8);
+ if (ret) {
+ drvdata->profile_pending = CLAW_NO_PENDING;
+ return ret;
+ }
+
+ return sysfs_emit(buf, "%u\n", drvdata->rumble_intensity_right);
+}
+static DEVICE_ATTR_RW(rumble_intensity_right);
+
+static ssize_t rumble_intensity_range_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ return sysfs_emit(buf, "0-100\n");
+}
+static DEVICE_ATTR_RO(rumble_intensity_range);
+
static umode_t claw_gamepad_attr_is_visible(struct kobject *kobj, struct attribute *attr,
int n)
{
@@ -835,6 +971,12 @@ static umode_t claw_gamepad_attr_is_visible(struct kobject *kobj, struct attribu
attr == &dev_attr_reset.attr)
return attr->mode;
+ /* Hide rumble attrs if not supported */
+ if (attr == &dev_attr_rumble_intensity_left.attr ||
+ attr == &dev_attr_rumble_intensity_right.attr ||
+ attr == &dev_attr_rumble_intensity_range.attr)
+ return drvdata->rumble_support ? attr->mode : 0;
+
/* Hide button mapping attrs if it isn't supported */
return drvdata->bmap_support ? attr->mode : 0;
}
@@ -848,6 +990,9 @@ static struct attribute *claw_gamepad_attrs[] = {
&dev_attr_mkeys_function.attr,
&dev_attr_mkeys_function_index.attr,
&dev_attr_reset.attr,
+ &dev_attr_rumble_intensity_left.attr,
+ &dev_attr_rumble_intensity_right.attr,
+ &dev_attr_rumble_intensity_range.attr,
NULL,
};
@@ -1312,6 +1457,7 @@ static void claw_features_supported(struct claw_drvdata *drvdata)
drvdata->bmap_support = true;
if (minor >= 0x66) {
drvdata->bmap_addr = button_mapping_addr_new;
+ drvdata->rumble_support = true;
drvdata->rgb_addr = rgb_addr_new;
} else {
drvdata->bmap_addr = button_mapping_addr_old;
@@ -1323,6 +1469,7 @@ static void claw_features_supported(struct claw_drvdata *drvdata)
if ((major == 0x02 && minor >= 0x17) || major >= 0x03) {
drvdata->bmap_support = true;
drvdata->bmap_addr = button_mapping_addr_new;
+ drvdata->rumble_support = true;
drvdata->rgb_addr = rgb_addr_new;
return;
}
--
2.53.0
^ permalink raw reply related
* [PATCH v2 3/4] HID: hid-msi: Add RGB control interface
From: Derek J. Clark @ 2026-05-13 23:14 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: Pierre-Loup A . Griffais, Denis Benato, Zhouwang Huang,
Derek J . Clark, linux-input, linux-doc, linux-kernel
In-Reply-To: <20260513231445.3213501-1-derekjohn.clark@gmail.com>
Adds RGB control interface for MSI Claw devices. The MSI Claw uses a
fairly unique RGB interface. It has 9 total zones (4 per joystick ring
and 1 for the ABXY buttons), and supports up to 8 sequential frames of
RGB zone data. Each frame is written to a specific area of MCU memory by
the profile command, the value of which changes based on the firmware of
the device. Unlike other devices (such as the Legion Go or the OneXPlayer
devices), there are no hard coded effects built into the MCU. Instead,
the basic effects are provided as a series of frame data. I have
mirrored the effects available in Windows in this driver, while keeping
the effect names consistent with the Lenovo drivers for the effects that
are similar.
Initial reverse-engineering and implementation of this feature was done
by Zhouwang Huang. I refactored the overall format to conform to kernel
driver best practices and style guides. Claude was used as an initial
reviewer of this patch.
Assisted-by: Claude:claude-sonnet-4-6
Co-developed-by: Zhouwang Huang <honjow311@gmail.com>
Signed-off-by: Zhouwang Huang <honjow311@gmail.com>
Signed-off-by: Derek J. Clark <derekjohn.clark@gmail.com>
---
v2:
- Use pending_profile mutex
- Remove deadlock in cfg_setup, return on errors.
---
drivers/hid/hid-msi.c | 548 +++++++++++++++++++++++++++++++++++++++++-
1 file changed, 542 insertions(+), 6 deletions(-)
diff --git a/drivers/hid/hid-msi.c b/drivers/hid/hid-msi.c
index 13ba2747fdb66..a628b77bfb7b5 100644
--- a/drivers/hid/hid-msi.c
+++ b/drivers/hid/hid-msi.c
@@ -21,6 +21,7 @@
#include <linux/device.h>
#include <linux/hid.h>
#include <linux/kobject.h>
+#include <linux/led-class-multicolor.h>
#include <linux/leds.h>
#include <linux/module.h>
#include <linux/mutex.h>
@@ -42,6 +43,10 @@
#define CLAW_KEYS_MAX 5
+#define CLAW_RGB_ZONES 9
+#define CLAW_RGB_MAX_FRAMES 8
+#define CLAW_RGB_FRAME_OFFSET 0x24
+
enum claw_command_index {
CLAW_COMMAND_TYPE_READ_PROFILE = 0x04,
CLAW_COMMAND_TYPE_READ_PROFILE_ACK = 0x05,
@@ -70,6 +75,7 @@ enum claw_profile_ack_pending {
CLAW_NO_PENDING,
CLAW_M1_PENDING,
CLAW_M2_PENDING,
+ CLAW_RGB_PENDING,
};
enum claw_key_index {
@@ -227,6 +233,22 @@ static const struct {
{ 0xce, "REL_WHEEL_DOWN" },
};
+enum claw_rgb_effect_index {
+ CLAW_RGB_EFFECT_MONOCOLOR,
+ CLAW_RGB_EFFECT_BREATHE,
+ CLAW_RGB_EFFECT_CHROMA,
+ CLAW_RGB_EFFECT_RAINBOW,
+ CLAW_RGB_EFFECT_FROSTFIRE,
+};
+
+static const char * const claw_rgb_effect_text[] = {
+ [CLAW_RGB_EFFECT_MONOCOLOR] = "monocolor",
+ [CLAW_RGB_EFFECT_BREATHE] = "breathe",
+ [CLAW_RGB_EFFECT_CHROMA] = "chroma",
+ [CLAW_RGB_EFFECT_RAINBOW] = "rainbow",
+ [CLAW_RGB_EFFECT_FROSTFIRE] = "frostfire",
+};
+
static const u16 button_mapping_addr_old[] = {
0x007a, /* M1 */
0x011f, /* M2 */
@@ -237,6 +259,9 @@ static const u16 button_mapping_addr_new[] = {
0x0164, /* M2 */
};
+static const u16 rgb_addr_old = 0x01fa;
+static const u16 rgb_addr_new = 0x024a;
+
struct claw_command_report {
u8 report_id;
u8 padding[2];
@@ -245,6 +270,28 @@ struct claw_command_report {
u8 data[59];
} __packed;
+struct rgb_zone {
+ u8 red;
+ u8 green;
+ u8 blue;
+};
+
+struct rgb_frame {
+ struct rgb_zone zone[CLAW_RGB_ZONES];
+};
+
+struct rgb_report {
+ u8 profile;
+ __be16 read_addr;
+ u8 frame_bytes;
+ u8 padding;
+ u8 frame_count;
+ u8 state; /* Always 0x09 */
+ u8 speed;
+ u8 brightness;
+ struct rgb_frame zone_data;
+} __packed;
+
struct claw_drvdata {
/* MCU General Variables */
enum claw_profile_ack_pending profile_pending;
@@ -265,6 +312,16 @@ struct claw_drvdata {
u8 m2_codes[CLAW_KEYS_MAX];
const u16 *bmap_addr;
bool bmap_support;
+
+ /* RGB Variables */
+ struct rgb_frame rgb_frames[CLAW_RGB_MAX_FRAMES];
+ enum claw_rgb_effect_index rgb_effect;
+ struct led_classdev_mc led_mc;
+ struct delayed_work rgb_queue;
+ u8 rgb_frame_count;
+ bool rgb_enabled;
+ u8 rgb_speed;
+ u16 rgb_addr;
};
static int get_endpoint_address(struct hid_device *hdev)
@@ -296,8 +353,11 @@ static int claw_gamepad_mode_event(struct claw_drvdata *drvdata,
static int claw_profile_event(struct claw_drvdata *drvdata, struct claw_command_report *cmd_rep)
{
- u8 *codes;
- int i;
+ struct rgb_report *frame;
+ u16 rgb_addr, read_addr;
+ u8 *codes, f_idx;
+ u16 frame_calc;
+ int i, ret = 0;
switch (drvdata->profile_pending) {
case CLAW_M1_PENDING:
@@ -308,15 +368,52 @@ static int claw_profile_event(struct claw_drvdata *drvdata, struct claw_command_
for (i = 0; i < CLAW_KEYS_MAX; i++)
codes[i] = (cmd_rep->data[6 + i] != 0xff) ? cmd_rep->data[6 + i] : 0x00;
break;
+ case CLAW_RGB_PENDING:
+ frame = (struct rgb_report *)cmd_rep->data;
+ rgb_addr = drvdata->rgb_addr;
+ read_addr = be16_to_cpu(frame->read_addr);
+ frame_calc = (read_addr - rgb_addr) / CLAW_RGB_FRAME_OFFSET;
+ if (frame_calc > U8_MAX) {
+ dev_err(drvdata->led_mc.led_cdev.dev, "Got unsupported frame index: %x\n",
+ frame_calc);
+ ret = -EINVAL;
+ goto err_pending;
+ }
+ f_idx = frame_calc;
+
+ if (f_idx >= CLAW_RGB_MAX_FRAMES) {
+ dev_err(drvdata->led_mc.led_cdev.dev, "Got illegal frame index: %x\n",
+ f_idx);
+ ret = -EINVAL;
+ goto err_pending;
+ }
+
+ /* Always treat the first frame as the truth for these constants */
+ if (f_idx == 0) {
+ drvdata->rgb_frame_count = frame->frame_count;
+ /* Invert device speed (20-0) to sysfs speed (0-20) */
+ drvdata->rgb_speed = frame->speed;
+ drvdata->led_mc.led_cdev.brightness = frame->brightness;
+ drvdata->led_mc.subled_info[0].intensity = frame->zone_data.zone[0].red;
+ drvdata->led_mc.subled_info[1].intensity = frame->zone_data.zone[0].green;
+ drvdata->led_mc.subled_info[2].intensity = frame->zone_data.zone[0].blue;
+ }
+
+ memcpy(&drvdata->rgb_frames[f_idx], &frame->zone_data,
+ sizeof(struct rgb_frame));
+
+ break;
default:
dev_warn(&drvdata->hdev->dev,
"Got profile event without changes pending from command: %x\n",
cmd_rep->cmd);
- return -EINVAL;
+ ret = -EINVAL;
}
+
+err_pending:
drvdata->profile_pending = CLAW_NO_PENDING;
- return 0;
+ return ret;
}
static int claw_raw_event(struct claw_drvdata *drvdata, struct hid_report *report,
@@ -759,6 +856,397 @@ static const struct attribute_group claw_gamepad_attr_group = {
.is_visible = claw_gamepad_attr_is_visible,
};
+/* Read RGB config from device */
+static int claw_read_rgb_config(struct hid_device *hdev)
+{
+ u8 data[4] = { 0x01, 0x00, 0x00, CLAW_RGB_FRAME_OFFSET };
+ struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
+ u16 read_addr = drvdata->rgb_addr;
+ size_t len = ARRAY_SIZE(data);
+ int ret, i;
+
+ if (!drvdata->rgb_addr)
+ return -ENODEV;
+
+ /* Loop through all 8 pages of RGB data */
+ guard(mutex)(&drvdata->profile_mutex);
+ for (i = 0; i < 8; i++) {
+ drvdata->profile_pending = CLAW_RGB_PENDING;
+ data[1] = (read_addr >> 8) & 0xff;
+ data[2] = read_addr & 0x00ff;
+ ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_READ_PROFILE, data, len, 8);
+ if (ret) {
+ drvdata->profile_pending = CLAW_NO_PENDING;
+ return ret;
+ }
+ read_addr += CLAW_RGB_FRAME_OFFSET;
+ }
+
+ return 0;
+}
+
+/* Send RGB configuration to device */
+static int claw_write_rgb_state(struct claw_drvdata *drvdata)
+{
+ struct rgb_report report = { 0x01, 0x0000, CLAW_RGB_FRAME_OFFSET, 0x00,
+ drvdata->rgb_frame_count, 0x09, drvdata->rgb_speed,
+ drvdata->led_mc.led_cdev.brightness };
+ u16 write_addr = drvdata->rgb_addr;
+ size_t len = sizeof(report);
+ int f, ret;
+
+ if (!drvdata->rgb_addr)
+ return -ENODEV;
+
+ if (!drvdata->rgb_frame_count)
+ return -EINVAL;
+
+ guard(mutex)(&drvdata->rom_mutex);
+ /* Loop through (up to) 8 pages of RGB data */
+ for (f = 0; f < drvdata->rgb_frame_count; f++) {
+ report.zone_data = drvdata->rgb_frames[f];
+
+ /* Set the MCU address to write the frame data to */
+ report.read_addr = cpu_to_be16(write_addr);
+
+ /* Serialize the rgb_report and write it to MCU */
+ ret = claw_hw_output_report(drvdata->hdev, CLAW_COMMAND_TYPE_WRITE_PROFILE_DATA,
+ (u8 *)&report, len, 8);
+ if (ret)
+ return ret;
+
+ /* Increment the write addr by the offset for the next frame */
+ write_addr += CLAW_RGB_FRAME_OFFSET;
+ }
+
+ ret = claw_hw_output_report(drvdata->hdev, CLAW_COMMAND_TYPE_SYNC_TO_ROM, NULL, 0, 8);
+
+ return ret;
+}
+
+/* Fill all zones with the same color */
+static void claw_frame_fill_solid(struct rgb_frame *frame, struct rgb_zone zone)
+{
+ int z;
+
+ for (z = 0; z < CLAW_RGB_ZONES; z++)
+ frame->zone[z] = zone;
+}
+
+/* Apply solid effect (1 frame, all zones same color) */
+static int claw_apply_monocolor(struct claw_drvdata *drvdata)
+{
+ struct mc_subled *subleds = drvdata->led_mc.subled_info;
+ struct rgb_zone zone = { subleds[0].intensity, subleds[1].intensity,
+ subleds[2].intensity };
+
+ drvdata->rgb_frame_count = 1;
+ claw_frame_fill_solid(&drvdata->rgb_frames[0], zone);
+
+ return claw_write_rgb_state(drvdata);
+}
+
+/* Apply breathe effect (2 frames: color -> off) */
+static int claw_apply_breathe(struct claw_drvdata *drvdata)
+{
+ struct mc_subled *subleds = drvdata->led_mc.subled_info;
+ struct rgb_zone zone = { subleds[0].intensity, subleds[1].intensity,
+ subleds[2].intensity };
+ static const struct rgb_zone off = { 0, 0, 0 };
+
+ drvdata->rgb_frame_count = 2;
+ claw_frame_fill_solid(&drvdata->rgb_frames[0], zone);
+ claw_frame_fill_solid(&drvdata->rgb_frames[1], off);
+
+ return claw_write_rgb_state(drvdata);
+}
+
+/* Apply chroma effect (6 frames: rainbow cycle, all zones sync) */
+static int claw_apply_chroma(struct claw_drvdata *drvdata)
+{
+ static const struct rgb_zone colors[] = {
+ {255, 0, 0}, /* red */
+ {255, 255, 0}, /* yellow */
+ { 0, 255, 0}, /* green */
+ { 0, 255, 255}, /* cyan */
+ { 0, 0, 255}, /* blue */
+ {255, 0, 255}, /* magenta */
+ };
+ u8 frame_count = ARRAY_SIZE(colors);
+ int frame;
+
+ drvdata->rgb_frame_count = frame_count;
+
+ for (frame = 0; frame < frame_count; frame++)
+ claw_frame_fill_solid(&drvdata->rgb_frames[frame], colors[frame]);
+
+ return claw_write_rgb_state(drvdata);
+}
+
+/* Apply rainbow effect (4 frames: rotating colors around joysticks) */
+static int claw_apply_rainbow(struct claw_drvdata *drvdata)
+{
+ static const struct rgb_zone colors[] = {
+ {255, 0, 0}, /* red */
+ { 0, 255, 0}, /* green */
+ { 0, 255, 255}, /* cyan */
+ { 0, 0, 255}, /* blue */
+ };
+ u8 frame_count = ARRAY_SIZE(colors);
+ int frame, zone;
+
+ drvdata->rgb_frame_count = frame_count;
+
+ for (frame = 0; frame < frame_count; frame++) {
+ for (zone = 0; zone < 4; zone++) {
+ drvdata->rgb_frames[frame].zone[zone] = colors[(zone + frame) % 4];
+ drvdata->rgb_frames[frame].zone[zone + 4] = colors[(zone + frame) % 4];
+ }
+ drvdata->rgb_frames[frame].zone[8] = colors[frame];
+ }
+
+ return claw_write_rgb_state(drvdata);
+}
+
+/*
+ * Apply frostfire effect (4 frames: fire vs ice rotating)
+ * Right joystick: fire red -> dark -> ice blue -> dark (clockwise)
+ * Left joystick: ice blue -> dark -> fire red -> dark (counter-clockwise)
+ * ABXY: fire red -> dark -> ice blue -> dark
+ */
+static int claw_apply_frostfire(struct claw_drvdata *drvdata)
+{
+ static const struct rgb_zone colors[] = {
+ {255, 0, 0}, /* fire red */
+ { 0, 0, 0}, /* dark */
+ { 0, 0, 255}, /* ice blue */
+ { 0, 0, 0}, /* dark */
+ };
+ u8 frame_count = ARRAY_SIZE(colors);
+ int frame, zone;
+
+ drvdata->rgb_frame_count = frame_count;
+
+ for (frame = 0; frame < frame_count; frame++) {
+ for (zone = 0; zone < 4; zone++) {
+ drvdata->rgb_frames[frame].zone[zone] = colors[(zone + frame) % 4];
+ drvdata->rgb_frames[frame].zone[zone + 4] = colors[(zone - frame + 6) % 4];
+ }
+ drvdata->rgb_frames[frame].zone[8] = colors[frame];
+ }
+
+ return claw_write_rgb_state(drvdata);
+}
+
+/* Apply current state to device */
+static int claw_apply_rgb_state(struct claw_drvdata *drvdata)
+{
+ static const struct rgb_zone off = { 0, 0, 0 };
+
+ if (!drvdata->rgb_enabled) {
+ drvdata->rgb_frame_count = 1;
+ claw_frame_fill_solid(&drvdata->rgb_frames[0], off);
+ return claw_write_rgb_state(drvdata);
+ }
+
+ switch (drvdata->rgb_effect) {
+ case CLAW_RGB_EFFECT_MONOCOLOR:
+ return claw_apply_monocolor(drvdata);
+ case CLAW_RGB_EFFECT_BREATHE:
+ return claw_apply_breathe(drvdata);
+ case CLAW_RGB_EFFECT_CHROMA:
+ return claw_apply_chroma(drvdata);
+ case CLAW_RGB_EFFECT_RAINBOW:
+ return claw_apply_rainbow(drvdata);
+ case CLAW_RGB_EFFECT_FROSTFIRE:
+ return claw_apply_frostfire(drvdata);
+ default:
+ dev_err(drvdata->led_mc.led_cdev.dev,
+ "No supported rgb_effect selected\n");
+ return -EINVAL;
+ }
+}
+
+static void claw_rgb_queue_fn(struct work_struct *work)
+{
+ struct delayed_work *dwork = container_of(work, struct delayed_work, work);
+ struct claw_drvdata *drvdata = container_of(dwork, struct claw_drvdata, rgb_queue);
+ int ret;
+
+ ret = claw_apply_rgb_state(drvdata);
+ if (ret)
+ dev_err(drvdata->led_mc.led_cdev.dev,
+ "Failed to apply RGB state: %d\n", ret);
+}
+
+static ssize_t effect_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct led_classdev *led_cdev = dev_get_drvdata(dev);
+ struct led_classdev_mc *led_mc = container_of(led_cdev, struct led_classdev_mc, led_cdev);
+ struct claw_drvdata *drvdata = container_of(led_mc, struct claw_drvdata, led_mc);
+ int ret;
+
+ ret = sysfs_match_string(claw_rgb_effect_text, buf);
+ if (ret < 0)
+ return ret;
+
+ drvdata->rgb_effect = ret;
+ mod_delayed_work(system_wq, &drvdata->rgb_queue, msecs_to_jiffies(50));
+
+ return count;
+}
+
+static ssize_t effect_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct led_classdev *led_cdev = dev_get_drvdata(dev);
+ struct led_classdev_mc *led_mc = container_of(led_cdev, struct led_classdev_mc, led_cdev);
+ struct claw_drvdata *drvdata = container_of(led_mc, struct claw_drvdata, led_mc);
+
+ if (drvdata->rgb_effect >= ARRAY_SIZE(claw_rgb_effect_text))
+ return -EINVAL;
+
+ return sysfs_emit(buf, "%s\n", claw_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)
+{
+ int i, count = 0;
+
+ for (i = 0; i < ARRAY_SIZE(claw_rgb_effect_text); i++)
+ count += sysfs_emit_at(buf, count, "%s ", claw_rgb_effect_text[i]);
+
+ if (count)
+ buf[count - 1] = '\n';
+
+ return count;
+}
+static DEVICE_ATTR_RO(effect_index);
+
+static ssize_t enabled_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct led_classdev *led_cdev = dev_get_drvdata(dev);
+ struct led_classdev_mc *led_mc = container_of(led_cdev, struct led_classdev_mc, led_cdev);
+ struct claw_drvdata *drvdata = container_of(led_mc, struct claw_drvdata, led_mc);
+ bool val;
+ int ret;
+
+ ret = kstrtobool(buf, &val);
+ if (ret)
+ return ret;
+
+ drvdata->rgb_enabled = val;
+ mod_delayed_work(system_wq, &drvdata->rgb_queue, msecs_to_jiffies(50));
+
+ return count;
+}
+
+static ssize_t enabled_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct led_classdev *led_cdev = dev_get_drvdata(dev);
+ struct led_classdev_mc *led_mc = container_of(led_cdev, struct led_classdev_mc, led_cdev);
+ struct claw_drvdata *drvdata = container_of(led_mc, struct claw_drvdata, led_mc);
+
+ return sysfs_emit(buf, "%s\n", drvdata->rgb_enabled ? "true" : "false");
+}
+static DEVICE_ATTR_RW(enabled);
+
+static ssize_t enabled_index_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ return sysfs_emit(buf, "true false\n");
+}
+static DEVICE_ATTR_RO(enabled_index);
+
+static ssize_t speed_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct led_classdev *led_cdev = dev_get_drvdata(dev);
+ struct led_classdev_mc *led_mc = container_of(led_cdev, struct led_classdev_mc, led_cdev);
+ struct claw_drvdata *drvdata = container_of(led_mc, struct claw_drvdata, led_mc);
+ unsigned int val, speed;
+ int ret;
+
+ ret = kstrtouint(buf, 10, &val);
+ if (ret)
+ return ret;
+
+ if (val > 20)
+ return -EINVAL;
+
+ /* 0 is fastest, invert value for intuitive userspace speed */
+ speed = 20 - val;
+
+ drvdata->rgb_speed = speed;
+ mod_delayed_work(system_wq, &drvdata->rgb_queue, msecs_to_jiffies(50));
+
+ return count;
+}
+
+static ssize_t speed_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ struct hid_device *hdev = to_hid_device(dev);
+ struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
+ u8 speed = 20 - drvdata->rgb_speed;
+
+ return sysfs_emit(buf, "%u\n", 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-20\n");
+}
+static DEVICE_ATTR_RO(speed_range);
+
+static void claw_led_brightness_set(struct led_classdev *led_cdev,
+ enum led_brightness _brightness)
+{
+ struct led_classdev_mc *led_mc = container_of(led_cdev, struct led_classdev_mc, led_cdev);
+ struct claw_drvdata *drvdata = container_of(led_mc, struct claw_drvdata, led_mc);
+
+ mod_delayed_work(system_wq, &drvdata->rgb_queue, msecs_to_jiffies(50));
+}
+
+static struct attribute *claw_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 rgb_attr_group = {
+ .attrs = claw_rgb_attrs,
+};
+
+static struct mc_subled claw_rgb_subled_info[] = {
+ {
+ .color_index = LED_COLOR_ID_RED,
+ .channel = 0x1,
+ },
+ {
+ .color_index = LED_COLOR_ID_GREEN,
+ .channel = 0x2,
+ },
+ {
+ .color_index = LED_COLOR_ID_BLUE,
+ .channel = 0x3,
+ },
+};
+
static void cfg_setup_fn(struct work_struct *work)
{
struct delayed_work *dwork = container_of(work, struct delayed_work, work);
@@ -772,6 +1260,13 @@ static void cfg_setup_fn(struct work_struct *work)
return;
}
+ ret = claw_read_rgb_config(drvdata->hdev);
+ if (ret) {
+ dev_err(drvdata->led_mc.led_cdev.dev,
+ "Failed to setup device, can't read RGB config: %d\n", ret);
+ return;
+ }
+
/* Add sysfs attributes after we get the device state */
ret = devm_device_add_group(&drvdata->hdev->dev, &claw_gamepad_attr_group);
if (ret) {
@@ -780,7 +1275,15 @@ static void cfg_setup_fn(struct work_struct *work)
return;
}
+ ret = devm_device_add_group(drvdata->led_mc.led_cdev.dev, &rgb_attr_group);
+ if (ret) {
+ dev_err(&drvdata->hdev->dev,
+ "Failed to setup device, can't create led attributes: %d\n", ret);
+ return;
+ }
+
kobject_uevent(&drvdata->hdev->dev.kobj, KOBJ_CHANGE);
+ kobject_uevent(&drvdata->led_mc.led_cdev.dev->kobj, KOBJ_CHANGE);
}
static void cfg_resume_fn(struct work_struct *work)
@@ -790,6 +1293,10 @@ static void cfg_resume_fn(struct work_struct *work)
u8 data[2] = { drvdata->gamepad_mode, drvdata->mkeys_function };
int ret;
+ ret = claw_read_rgb_config(drvdata->hdev);
+ if (ret)
+ dev_err(drvdata->led_mc.led_cdev.dev, "Failed to read RGB config: %d\n", ret);
+
ret = claw_hw_output_report(drvdata->hdev, CLAW_COMMAND_TYPE_SWITCH_MODE, data,
ARRAY_SIZE(data), 0);
if (ret)
@@ -803,18 +1310,24 @@ static void claw_features_supported(struct claw_drvdata *drvdata)
if (major == 0x01) {
drvdata->bmap_support = true;
- if (minor >= 0x66)
+ if (minor >= 0x66) {
drvdata->bmap_addr = button_mapping_addr_new;
- else
+ drvdata->rgb_addr = rgb_addr_new;
+ } else {
drvdata->bmap_addr = button_mapping_addr_old;
+ drvdata->rgb_addr = rgb_addr_old;
+ }
return;
}
if ((major == 0x02 && minor >= 0x17) || major >= 0x03) {
drvdata->bmap_support = true;
drvdata->bmap_addr = button_mapping_addr_new;
+ drvdata->rgb_addr = rgb_addr_new;
return;
}
+
+ drvdata->rgb_addr = rgb_addr_old;
}
static int claw_probe(struct hid_device *hdev, u8 ep)
@@ -844,6 +1357,26 @@ static int claw_probe(struct hid_device *hdev, u8 ep)
if (!drvdata->bmap_support)
dev_dbg(&hdev->dev, "M-Key mapping is not supported. Update firmware to enable.\n");
+ /* Initialize RGB LED */
+ INIT_DELAYED_WORK(&drvdata->rgb_queue, &claw_rgb_queue_fn);
+
+ drvdata->led_mc.led_cdev.name = "msi_claw:rgb:joystick_rings";
+ drvdata->led_mc.led_cdev.brightness = 0x50;
+ drvdata->led_mc.led_cdev.max_brightness = 0x64;
+ drvdata->led_mc.led_cdev.color = LED_COLOR_ID_RGB;
+ drvdata->led_mc.led_cdev.brightness_set = claw_led_brightness_set;
+ drvdata->led_mc.num_colors = 3;
+ drvdata->led_mc.subled_info = devm_kmemdup(&hdev->dev, claw_rgb_subled_info,
+ sizeof(claw_rgb_subled_info), GFP_KERNEL);
+ if (!drvdata->led_mc.subled_info)
+ return -ENOMEM;
+
+ drvdata->rgb_enabled = true;
+
+ ret = devm_led_classdev_multicolor_register(&hdev->dev, &drvdata->led_mc);
+ if (ret)
+ return ret;
+
/* For control interface: open the HID transport for sending commands. */
ret = hid_hw_open(hdev);
if (ret)
@@ -905,6 +1438,9 @@ static void claw_remove(struct hid_device *hdev)
return;
}
+ /* Block writes to brightness/multi_intensity during teardown */
+ drvdata->led_mc.led_cdev.brightness_set = NULL;
+ cancel_delayed_work_sync(&drvdata->rgb_queue);
cancel_delayed_work_sync(&drvdata->cfg_setup);
cancel_delayed_work_sync(&drvdata->cfg_resume);
hid_hw_close(hdev);
--
2.53.0
^ permalink raw reply related
* [PATCH v2 2/4] HID: hid-msi: Add M-key mapping attributes
From: Derek J. Clark @ 2026-05-13 23:14 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: Pierre-Loup A . Griffais, Denis Benato, Zhouwang Huang,
Derek J . Clark, linux-input, linux-doc, linux-kernel
In-Reply-To: <20260513231445.3213501-1-derekjohn.clark@gmail.com>
Adds attributes that allow for remapping the M-keys with up to 5 values
when in macro mode. There are 2 mappable buttons on the rear of the
device, M1 on the right and M2 on the left. When mapped, the events will
fire from one of three event devices: gamepad buttons will fire from the
device handled by xpad, while keyboard and mouse events will fire from
respectively typed evdevs provided by the input core. Names of each
mapping have been kept as close to the event that will fire from the evdev
as possible, with context added to the ABS_ events on the direction of the
movement.
Initial reverse-engineering and implementation of this feature was done
by Zhouwang Huang. I refactored the overall format to conform to kernel
driver best practices and style guides. Claude was used as an initial
reviewer of this patch.
Assisted-by: Claude:claude-sonnet-4-6
Co-developed-by: Zhouwang Huang <honjow311@gmail.com>
Signed-off-by: Zhouwang Huang <honjow311@gmail.com>
Signed-off-by: Derek J. Clark <derekjohn.clark@gmail.com>
---
v2:
- Add mutex for SYNC_TO_ROM commands to ensure every SYNC is completed
before more data is written to the MCU volatile memory.
- Add mutex for profile_pending to ensure every profile action
response is serialized to the generating command.
---
drivers/hid/hid-msi.c | 398 +++++++++++++++++++++++++++++++++++++++++-
1 file changed, 397 insertions(+), 1 deletion(-)
diff --git a/drivers/hid/hid-msi.c b/drivers/hid/hid-msi.c
index 8915942af15e6..13ba2747fdb66 100644
--- a/drivers/hid/hid-msi.c
+++ b/drivers/hid/hid-msi.c
@@ -40,6 +40,8 @@
#define CLAW_DINPUT_CFG_INTF_IN 0x82
#define CLAW_XINPUT_CFG_INTF_IN 0x83
+#define CLAW_KEYS_MAX 5
+
enum claw_command_index {
CLAW_COMMAND_TYPE_READ_PROFILE = 0x04,
CLAW_COMMAND_TYPE_READ_PROFILE_ACK = 0x05,
@@ -64,6 +66,17 @@ static const char * const claw_gamepad_mode_text[] = {
[CLAW_GAMEPAD_MODE_DESKTOP] = "desktop",
};
+enum claw_profile_ack_pending {
+ CLAW_NO_PENDING,
+ CLAW_M1_PENDING,
+ CLAW_M2_PENDING,
+};
+
+enum claw_key_index {
+ CLAW_KEY_M1,
+ CLAW_KEY_M2,
+};
+
enum claw_mkeys_function_index {
CLAW_MKEY_FUNCTION_MACRO,
CLAW_MKEY_FUNCTION_COMBO,
@@ -76,6 +89,154 @@ static const char * const claw_mkeys_function_text[] = {
[CLAW_MKEY_FUNCTION_DISABLED] = "disabled",
};
+static const struct {
+ u8 code;
+ const char *name;
+} claw_button_mapping_key_map[] = {
+ /* Gamepad buttons */
+ { 0x01, "ABS_HAT0Y_UP" },
+ { 0x02, "ABS_HAT0Y_DOWN" },
+ { 0x03, "ABS_HAT0X_LEFT" },
+ { 0x04, "ABS_HAT0X_RIGHT" },
+ { 0x05, "BTN_TL" },
+ { 0x06, "BTN_TR" },
+ { 0x07, "BTN_THUMBL" },
+ { 0x08, "BTN_THUMBR" },
+ { 0x09, "BTN_SOUTH" },
+ { 0x0a, "BTN_EAST" },
+ { 0x0b, "BTN_NORTH" },
+ { 0x0c, "BTN_WEST" },
+ { 0x0d, "BTN_MODE" },
+ { 0x0e, "BTN_SELECT" },
+ { 0x0f, "BTN_START" },
+ { 0x13, "BTN_TL2"},
+ { 0x14, "BTN_TR2"},
+ { 0x15, "ABS_Y_UP"},
+ { 0x16, "ABS_Y_DOWN"},
+ { 0x17, "ABS_X_LEFT"},
+ { 0x18, "ABS_X_LEFT_RIGHT"},
+ { 0x19, "ABS_RY_UP"},
+ { 0x1a, "ABS_RY_DOWN"},
+ { 0x1b, "ABS_RX_LEFT"},
+ { 0x1c, "ABS_RX_RIGHT"},
+ /* Keyboard keys */
+ { 0x32, "KEY_ESC" },
+ { 0x33, "KEY_F1" },
+ { 0x34, "KEY_F2" },
+ { 0x35, "KEY_F3" },
+ { 0x36, "KEY_F4" },
+ { 0x37, "KEY_F5" },
+ { 0x38, "KEY_F6" },
+ { 0x39, "KEY_F7" },
+ { 0x3a, "KEY_F8" },
+ { 0x3b, "KEY_F9" },
+ { 0x3c, "KEY_F10" },
+ { 0x3d, "KEY_F11" },
+ { 0x3e, "KEY_F12" },
+ { 0x3f, "KEY_GRAVE" },
+ { 0x40, "KEY_1" },
+ { 0x41, "KEY_2" },
+ { 0x42, "KEY_3" },
+ { 0x43, "KEY_4" },
+ { 0x44, "KEY_5" },
+ { 0x45, "KEY_6" },
+ { 0x46, "KEY_7" },
+ { 0x47, "KEY_8" },
+ { 0x48, "KEY_9" },
+ { 0x49, "KEY_0" },
+ { 0x4a, "KEY_MINUS" },
+ { 0x4b, "KEY_EQUAL" },
+ { 0x4c, "KEY_BACKSPACE" },
+ { 0x4d, "KEY_TAB" },
+ { 0x4e, "KEY_Q" },
+ { 0x4f, "KEY_W" },
+ { 0x50, "KEY_E" },
+ { 0x51, "KEY_R" },
+ { 0x52, "KEY_T" },
+ { 0x53, "KEY_Y" },
+ { 0x54, "KEY_U" },
+ { 0x55, "KEY_I" },
+ { 0x56, "KEY_O" },
+ { 0x57, "KEY_P" },
+ { 0x58, "KEY_LEFTBRACE" },
+ { 0x59, "KEY_RIGHTBRACE" },
+ { 0x5a, "KEY_BACKSLASH" },
+ { 0x5b, "KEY_CAPSLOCK" },
+ { 0x5c, "KEY_A" },
+ { 0x5d, "KEY_S" },
+ { 0x5e, "KEY_D" },
+ { 0x5f, "KEY_F" },
+ { 0x60, "KEY_G" },
+ { 0x61, "KEY_H" },
+ { 0x62, "KEY_J" },
+ { 0x63, "KEY_K" },
+ { 0x64, "KEY_L" },
+ { 0x65, "KEY_SEMICOLON" },
+ { 0x66, "KEY_APOSTROPHE" },
+ { 0x67, "KEY_ENTER" },
+ { 0x68, "KEY_LEFTSHIFT" },
+ { 0x69, "KEY_Z" },
+ { 0x6a, "KEY_X" },
+ { 0x6b, "KEY_C" },
+ { 0x6c, "KEY_V" },
+ { 0x6d, "KEY_B" },
+ { 0x6e, "KEY_N" },
+ { 0x6f, "KEY_M" },
+ { 0x70, "KEY_COMMA" },
+ { 0x71, "KEY_DOT" },
+ { 0x72, "KEY_SLASH" },
+ { 0x73, "KEY_RIGHTSHIFT" },
+ { 0x74, "KEY_LEFTCTRL" },
+ { 0x75, "KEY_LEFTMETA" },
+ { 0x76, "KEY_LEFTALT" },
+ { 0x77, "KEY_SPACE" },
+ { 0x78, "KEY_RIGHTALT" },
+ { 0x79, "KEY_RIGHTCTRL" },
+ { 0x7a, "KEY_INSERT" },
+ { 0x7b, "KEY_HOME" },
+ { 0x7c, "KEY_PAGEUP" },
+ { 0x7d, "KEY_DELETE" },
+ { 0x7e, "KEY_END" },
+ { 0x7f, "KEY_PAGEDOWN" },
+ { 0x8a, "KEY_KPENTER" },
+ { 0x8b, "KEY_KP0" },
+ { 0x8c, "KEY_KP1" },
+ { 0x8d, "KEY_KP2" },
+ { 0x8e, "KEY_KP3" },
+ { 0x8f, "KEY_KP4" },
+ { 0x90, "KEY_KP5" },
+ { 0x91, "KEY_KP6" },
+ { 0x92, "KEY_KP7" },
+ { 0x93, "KEY_KP8" },
+ { 0x94, "KEY_KP9" },
+ { 0x95, "MD_PLAY" },
+ { 0x96, "MD_STOP" },
+ { 0x97, "MD_NEXT" },
+ { 0x98, "MD_PREV" },
+ { 0x99, "MD_VOL_UP" },
+ { 0x9a, "MD_VOL_DOWN" },
+ { 0x9b, "MD_VOL_MUTE" },
+ { 0x9c, "KEY_F23" },
+ /* Mouse events */
+ { 0xc8, "BTN_LEFT" },
+ { 0xc9, "BTN_MIDDLE" },
+ { 0xca, "BTN_RIGHT" },
+ { 0xcb, "BTN_SIDE" },
+ { 0xcc, "BTN_EXTRA" },
+ { 0xcd, "REL_WHEEL_UP" },
+ { 0xce, "REL_WHEEL_DOWN" },
+};
+
+static const u16 button_mapping_addr_old[] = {
+ 0x007a, /* M1 */
+ 0x011f, /* M2 */
+};
+
+static const u16 button_mapping_addr_new[] = {
+ 0x00bb, /* M1 */
+ 0x0164, /* M2 */
+};
+
struct claw_command_report {
u8 report_id;
u8 padding[2];
@@ -86,16 +247,24 @@ struct claw_command_report {
struct claw_drvdata {
/* MCU General Variables */
+ enum claw_profile_ack_pending profile_pending;
struct completion send_cmd_complete;
struct delayed_work cfg_resume;
struct delayed_work cfg_setup;
+ struct mutex profile_mutex; /* mutex for profile_pending calls */
struct hid_device *hdev;
struct mutex cfg_mutex; /* mutex for synchronous data */
+ struct mutex rom_mutex; /* mutex for SYNC_TO_ROM calls */
+ u16 bcd_device;
u8 ep;
/* Gamepad Variables */
enum claw_mkeys_function_index mkeys_function;
enum claw_gamepad_mode_index gamepad_mode;
+ u8 m1_codes[CLAW_KEYS_MAX];
+ u8 m2_codes[CLAW_KEYS_MAX];
+ const u16 *bmap_addr;
+ bool bmap_support;
};
static int get_endpoint_address(struct hid_device *hdev)
@@ -125,6 +294,31 @@ static int claw_gamepad_mode_event(struct claw_drvdata *drvdata,
return 0;
}
+static int claw_profile_event(struct claw_drvdata *drvdata, struct claw_command_report *cmd_rep)
+{
+ u8 *codes;
+ int i;
+
+ switch (drvdata->profile_pending) {
+ case CLAW_M1_PENDING:
+ case CLAW_M2_PENDING:
+ codes = (drvdata->profile_pending == CLAW_M1_PENDING) ?
+ drvdata->m1_codes : drvdata->m2_codes;
+ /* Extract key codes; replace disabled (0xff) with 0x00, which is (null) in _show */
+ for (i = 0; i < CLAW_KEYS_MAX; i++)
+ codes[i] = (cmd_rep->data[6 + i] != 0xff) ? cmd_rep->data[6 + i] : 0x00;
+ break;
+ default:
+ dev_warn(&drvdata->hdev->dev,
+ "Got profile event without changes pending from command: %x\n",
+ cmd_rep->cmd);
+ return -EINVAL;
+ }
+ drvdata->profile_pending = CLAW_NO_PENDING;
+
+ return 0;
+}
+
static int claw_raw_event(struct claw_drvdata *drvdata, struct hid_report *report,
u8 *data, int size)
{
@@ -146,6 +340,9 @@ static int claw_raw_event(struct claw_drvdata *drvdata, struct hid_report *repor
case CLAW_COMMAND_TYPE_GAMEPAD_MODE_ACK:
ret = claw_gamepad_mode_event(drvdata, cmd_rep);
break;
+ case CLAW_COMMAND_TYPE_READ_PROFILE_ACK:
+ ret = claw_profile_event(drvdata, cmd_rep);
+ break;
case CLAW_COMMAND_TYPE_ACK:
break;
default:
@@ -366,6 +563,161 @@ static ssize_t reset_store(struct device *dev, struct device_attribute *attr,
}
static DEVICE_ATTR_WO(reset);
+static int button_mapping_name_to_code(const char *name)
+{
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(claw_button_mapping_key_map); i++) {
+ if (!strcmp(name, claw_button_mapping_key_map[i].name))
+ return claw_button_mapping_key_map[i].code;
+ }
+
+ return -EINVAL;
+}
+
+static const char *button_mapping_code_to_name(u8 code)
+{
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(claw_button_mapping_key_map); i++) {
+ if (claw_button_mapping_key_map[i].code == code)
+ return claw_button_mapping_key_map[i].name;
+ }
+
+ return NULL;
+}
+
+static int claw_buttons_store(struct device *dev, const char *buf, u8 mkey_idx)
+{
+ struct hid_device *hdev = to_hid_device(dev);
+ struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
+ u8 data[] = { 0x01, (drvdata->bmap_addr[mkey_idx] >> 8) & 0xff,
+ drvdata->bmap_addr[mkey_idx] & 0xff, 0x07,
+ 0x04, 0x00, 0xff, 0xff, 0xff, 0xff, 0xff };
+ size_t len = ARRAY_SIZE(data);
+ int ret, key_count, i;
+ char **raw_keys;
+
+ raw_keys = argv_split(GFP_KERNEL, buf, &key_count);
+ if (!raw_keys)
+ return -ENOMEM;
+
+ guard(mutex)(&drvdata->rom_mutex); /* all err_free paths must be in scope */
+ if (key_count > CLAW_KEYS_MAX) {
+ ret = -EINVAL;
+ goto err_free;
+ }
+
+ if (key_count == 0)
+ goto set_buttons;
+
+ for (i = 0; i < key_count; i++) {
+ ret = button_mapping_name_to_code(raw_keys[i]);
+ if (ret < 0)
+ goto err_free;
+
+ data[6 + i] = ret;
+ }
+
+set_buttons:
+ ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_WRITE_PROFILE_DATA, data, len, 8);
+ if (ret < 0)
+ goto err_free;
+
+ ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_SYNC_TO_ROM, NULL, 0, 8);
+
+err_free:
+ argv_free(raw_keys);
+ return ret;
+}
+
+static int claw_buttons_show(struct device *dev, char *buf, enum claw_key_index m_key)
+{
+ struct hid_device *hdev = to_hid_device(dev);
+ struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
+ u8 data[] = { 0x01, (drvdata->bmap_addr[m_key] >> 8) & 0xff,
+ drvdata->bmap_addr[m_key] & 0xff, 0x07 };
+ size_t len = ARRAY_SIZE(data);
+ int i, ret, count = 0;
+ const char *name;
+ u8 *codes;
+
+ codes = (m_key == CLAW_KEY_M1) ? drvdata->m1_codes : drvdata->m2_codes;
+
+ guard(mutex)(&drvdata->profile_mutex);
+ drvdata->profile_pending = (m_key == CLAW_KEY_M1) ? CLAW_M1_PENDING : CLAW_M2_PENDING;
+
+ ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_READ_PROFILE, data, len, 8);
+ if (ret) {
+ drvdata->profile_pending = CLAW_NO_PENDING;
+ return ret;
+ }
+ for (i = 0; i < CLAW_KEYS_MAX; i++) {
+ name = button_mapping_code_to_name(codes[i]);
+ if (name)
+ count += sysfs_emit_at(buf, count, "%s ", name);
+ }
+
+ if (!count)
+ return sysfs_emit(buf, "(not set)\n");
+
+ buf[count - 1] = '\n';
+
+ return count;
+}
+
+static ssize_t button_m1_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ int ret;
+
+ ret = claw_buttons_store(dev, buf, CLAW_KEY_M1);
+ if (ret)
+ return ret;
+
+ return count;
+}
+
+static ssize_t button_m1_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ return claw_buttons_show(dev, buf, CLAW_KEY_M1);
+}
+static DEVICE_ATTR_RW(button_m1);
+
+static ssize_t button_m2_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ int ret;
+
+ ret = claw_buttons_store(dev, buf, CLAW_KEY_M2);
+ if (ret)
+ return ret;
+
+ return count;
+}
+
+static ssize_t button_m2_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ return claw_buttons_show(dev, buf, CLAW_KEY_M2);
+}
+static DEVICE_ATTR_RW(button_m2);
+
+static ssize_t button_mapping_options_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ int i, count = 0;
+
+ for (i = 0; i < ARRAY_SIZE(claw_button_mapping_key_map); i++)
+ count += sysfs_emit_at(buf, count, "%s ", claw_button_mapping_key_map[i].name);
+
+ buf[count - 1] = '\n';
+
+ return count;
+}
+static DEVICE_ATTR_RO(button_mapping_options);
+
static umode_t claw_gamepad_attr_is_visible(struct kobject *kobj, struct attribute *attr,
int n)
{
@@ -378,10 +730,22 @@ static umode_t claw_gamepad_attr_is_visible(struct kobject *kobj, struct attribu
return 0;
}
- return attr->mode;
+ /* Always show attrs available on all firmware */
+ if (attr == &dev_attr_gamepad_mode.attr ||
+ attr == &dev_attr_gamepad_mode_index.attr ||
+ attr == &dev_attr_mkeys_function.attr ||
+ attr == &dev_attr_mkeys_function_index.attr ||
+ attr == &dev_attr_reset.attr)
+ return attr->mode;
+
+ /* Hide button mapping attrs if it isn't supported */
+ return drvdata->bmap_support ? attr->mode : 0;
}
static struct attribute *claw_gamepad_attrs[] = {
+ &dev_attr_button_m1.attr,
+ &dev_attr_button_m2.attr,
+ &dev_attr_button_mapping_options.attr,
&dev_attr_gamepad_mode.attr,
&dev_attr_gamepad_mode_index.attr,
&dev_attr_mkeys_function.attr,
@@ -432,8 +796,31 @@ static void cfg_resume_fn(struct work_struct *work)
dev_err(&drvdata->hdev->dev, "Failed to set gamepad mode settings: %d\n", ret);
}
+static void claw_features_supported(struct claw_drvdata *drvdata)
+{
+ u8 major = (drvdata->bcd_device >> 8) & 0xff;
+ u8 minor = drvdata->bcd_device & 0xff;
+
+ if (major == 0x01) {
+ drvdata->bmap_support = true;
+ if (minor >= 0x66)
+ drvdata->bmap_addr = button_mapping_addr_new;
+ else
+ drvdata->bmap_addr = button_mapping_addr_old;
+ return;
+ }
+
+ if ((major == 0x02 && minor >= 0x17) || major >= 0x03) {
+ drvdata->bmap_support = true;
+ drvdata->bmap_addr = button_mapping_addr_new;
+ return;
+ }
+}
+
static int claw_probe(struct hid_device *hdev, u8 ep)
{
+ struct usb_interface *intf = to_usb_interface(hdev->dev.parent);
+ struct usb_device *udev = interface_to_usbdev(intf);
struct claw_drvdata *drvdata;
int ret;
@@ -446,8 +833,17 @@ static int claw_probe(struct hid_device *hdev, u8 ep)
drvdata->ep = ep;
mutex_init(&drvdata->cfg_mutex);
+ mutex_init(&drvdata->profile_mutex);
+ mutex_init(&drvdata->rom_mutex);
init_completion(&drvdata->send_cmd_complete);
+ /* Determine feature level from firmware version */
+ drvdata->bcd_device = le16_to_cpu(udev->descriptor.bcdDevice);
+ claw_features_supported(drvdata);
+
+ if (!drvdata->bmap_support)
+ dev_dbg(&hdev->dev, "M-Key mapping is not supported. Update firmware to enable.\n");
+
/* For control interface: open the HID transport for sending commands. */
ret = hid_hw_open(hdev);
if (ret)
--
2.53.0
^ permalink raw reply related
* [PATCH v2 1/4] HID: hid-msi: Add MSI Claw configuration driver
From: Derek J. Clark @ 2026-05-13 23:14 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: Pierre-Loup A . Griffais, Denis Benato, Zhouwang Huang,
Derek J . Clark, linux-input, linux-doc, linux-kernel
In-Reply-To: <20260513231445.3213501-1-derekjohn.clark@gmail.com>
Adds configuration HID driver for the MSI Claw series of handheld PC's.
In this initial patch add the initial driver outline and attributes for
changing the gamepad mode, M-key behavior, and add a WO reset function.
Sending the SWITCH_MODE and RESET commands causes a USB disconnect in
the device. The completion will therefore never get hit and would trigger
an -EIO. To avoid showing the user an error for every write to these
attrs a bypass for the completion handling is introduced when timeout ==
0.
The initial version of this patch was written by Denis Benato, which
contained the initial reverse-engineering and implementation for the
gamepad mode switching. This work was later expanded by Zhouwang Huang
to include more gamepad modes. Finally, I refactored the drivers data
in/out flow and overall format to conform to kernel driver best
practices and style guides. Claude was used as an initial reviewer of
this patch.
Assisted-by: Claude:claude-sonnet-4-6
Co-developed-by: Denis Benato <denis.benato@linux.dev>
Signed-off-by: Denis Benato <denis.benato@linux.dev>
Co-developed-by: Zhouwang Huang <honjow311@gmail.com>
Signed-off-by: Zhouwang Huang <honjow311@gmail.com>
Signed-off-by: Derek J. Clark <derekjohn.clark@gmail.com>
---
v2:
- Rename driver to hid-msi from hid-msi-claw.
- Rename reusable/generic functions to msi_* from claw_*, retaining
claw specific functions.
- Add generic entrypoints for probe, remove, and raw event that route
to claw specific functions.
- Remove deadlock in cfg_setup, return on errors.
---
MAINTAINERS | 6 +
drivers/hid/Kconfig | 12 +
drivers/hid/Makefile | 1 +
drivers/hid/hid-ids.h | 6 +
drivers/hid/hid-msi.c | 582 ++++++++++++++++++++++++++++++++++++++++++
5 files changed, 607 insertions(+)
create mode 100644 drivers/hid/hid-msi.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 6f6517bf4f970..8e2de98b768f7 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -17965,6 +17965,12 @@ S: Odd Fixes
F: Documentation/devicetree/bindings/net/ieee802154/mrf24j40.txt
F: drivers/net/ieee802154/mrf24j40.c
+MSI HID DRIVER
+M: Derek J. Clark <derekjohn.clark@gmail.com>
+L: linux-input@vger.kernel.org
+S: Maintained
+F: drivers/hid/hid-msi.c
+
MSI EC DRIVER
M: Nikita Kravets <teackot@gmail.com>
L: platform-driver-x86@vger.kernel.org
diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index 10c12d8e65579..af146691bd481 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -492,6 +492,18 @@ config HID_GT683R
Currently the following devices are know to be supported:
- MSI GT683R
+config HID_MSI
+ tristate "MSI Claw Gamepad Support"
+ depends on USB_HID
+ select LEDS_CLASS
+ select LEDS_CLASS_MULTICOLOR
+ help
+ Support for the MSI Claw RGB and controller configuration
+
+ Say Y here to include configuration interface support for the MSI Claw Line
+ of Handheld Console Controllers. Say M here to compile this driver as a
+ module. The module will be called hid-msi.
+
config HID_KEYTOUCH
tristate "Keytouch HID devices"
help
diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
index 07dfdb6a49c59..80925a17b059c 100644
--- a/drivers/hid/Makefile
+++ b/drivers/hid/Makefile
@@ -92,6 +92,7 @@ obj-$(CONFIG_HID_MAYFLASH) += hid-mf.o
obj-$(CONFIG_HID_MEGAWORLD_FF) += hid-megaworld.o
obj-$(CONFIG_HID_MICROSOFT) += hid-microsoft.o
obj-$(CONFIG_HID_MONTEREY) += hid-monterey.o
+obj-$(CONFIG_HID_MSI) += hid-msi.o
obj-$(CONFIG_HID_MULTITOUCH) += hid-multitouch.o
obj-$(CONFIG_HID_NINTENDO) += hid-nintendo.o
obj-$(CONFIG_HID_NTI) += hid-nti.o
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 933b7943bdb50..6d0d34806931f 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -1047,7 +1047,13 @@
#define USB_DEVICE_ID_MOZA_R16_R21_2 0x0010
#define USB_VENDOR_ID_MSI 0x1770
+#define USB_VENDOR_ID_MSI_2 0x0db0
#define USB_DEVICE_ID_MSI_GT683R_LED_PANEL 0xff00
+#define USB_DEVICE_ID_MSI_CLAW_XINPUT 0x1901
+#define USB_DEVICE_ID_MSI_CLAW_DINPUT 0x1902
+#define USB_DEVICE_ID_MSI_CLAW_DESKTOP 0x1903
+#define USB_DEVICE_ID_MSI_CLAW_BIOS 0x1904
+
#define USB_VENDOR_ID_NATIONAL_SEMICONDUCTOR 0x0400
#define USB_DEVICE_ID_N_S_HARMONY 0xc359
diff --git a/drivers/hid/hid-msi.c b/drivers/hid/hid-msi.c
new file mode 100644
index 0000000000000..8915942af15e6
--- /dev/null
+++ b/drivers/hid/hid-msi.c
@@ -0,0 +1,582 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * HID driver for MSI Claw Handheld PC gamepads.
+ *
+ * Provides configuration support for the MSI Claw series of handheld PC
+ * gamepads. Multiple iterations of the device firmware has led to some
+ * quirks for how certain attributes are handled. The original firmware
+ * did not support remapping of the M1 (right) and M2 (left) rear paddles.
+ * Additionally, the MCU RAM address for writing configuration data has
+ * changed twice. Checks are done during probe to enumerate these variances.
+ *
+ * Copyright (c) 2026 Zhouwang Huang <honjow311@gmail.com>
+ * Copyright (c) 2026 Denis Benato <denis.benato@linux.dev>
+ * Copyright (c) 2026 Valve Corporation
+ */
+
+#include <linux/array_size.h>
+#include <linux/cleanup.h>
+#include <linux/completion.h>
+#include <linux/container_of.h>
+#include <linux/device.h>
+#include <linux/hid.h>
+#include <linux/kobject.h>
+#include <linux/leds.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/sysfs.h>
+#include <linux/types.h>
+#include <linux/unaligned.h>
+#include <linux/usb.h>
+#include <linux/workqueue.h>
+
+#include "hid-ids.h"
+
+#define CLAW_OUTPUT_REPORT_ID 0x0f
+#define CLAW_INPUT_REPORT_ID 0x10
+
+#define CLAW_PACKET_SIZE 64
+
+#define CLAW_DINPUT_CFG_INTF_IN 0x82
+#define CLAW_XINPUT_CFG_INTF_IN 0x83
+
+enum claw_command_index {
+ CLAW_COMMAND_TYPE_READ_PROFILE = 0x04,
+ CLAW_COMMAND_TYPE_READ_PROFILE_ACK = 0x05,
+ CLAW_COMMAND_TYPE_ACK = 0x06,
+ CLAW_COMMAND_TYPE_WRITE_PROFILE_DATA = 0x21,
+ CLAW_COMMAND_TYPE_SYNC_TO_ROM = 0x22,
+ CLAW_COMMAND_TYPE_SWITCH_MODE = 0x24,
+ CLAW_COMMAND_TYPE_READ_GAMEPAD_MODE = 0x26,
+ CLAW_COMMAND_TYPE_GAMEPAD_MODE_ACK = 0x27,
+ CLAW_COMMAND_TYPE_RESET_DEVICE = 0x28,
+};
+
+enum claw_gamepad_mode_index {
+ CLAW_GAMEPAD_MODE_XINPUT = 0x01,
+ CLAW_GAMEPAD_MODE_DINPUT = 0x02,
+ CLAW_GAMEPAD_MODE_DESKTOP = 0x04,
+};
+
+static const char * const claw_gamepad_mode_text[] = {
+ [CLAW_GAMEPAD_MODE_XINPUT] = "xinput",
+ [CLAW_GAMEPAD_MODE_DINPUT] = "dinput",
+ [CLAW_GAMEPAD_MODE_DESKTOP] = "desktop",
+};
+
+enum claw_mkeys_function_index {
+ CLAW_MKEY_FUNCTION_MACRO,
+ CLAW_MKEY_FUNCTION_COMBO,
+ CLAW_MKEY_FUNCTION_DISABLED,
+};
+
+static const char * const claw_mkeys_function_text[] = {
+ [CLAW_MKEY_FUNCTION_MACRO] = "macro",
+ [CLAW_MKEY_FUNCTION_COMBO] = "combination",
+ [CLAW_MKEY_FUNCTION_DISABLED] = "disabled",
+};
+
+struct claw_command_report {
+ u8 report_id;
+ u8 padding[2];
+ u8 header_tail;
+ u8 cmd;
+ u8 data[59];
+} __packed;
+
+struct claw_drvdata {
+ /* MCU General Variables */
+ struct completion send_cmd_complete;
+ struct delayed_work cfg_resume;
+ struct delayed_work cfg_setup;
+ struct hid_device *hdev;
+ struct mutex cfg_mutex; /* mutex for synchronous data */
+ u8 ep;
+
+ /* Gamepad Variables */
+ enum claw_mkeys_function_index mkeys_function;
+ enum claw_gamepad_mode_index gamepad_mode;
+};
+
+static int get_endpoint_address(struct hid_device *hdev)
+{
+ struct usb_host_endpoint *ep;
+ struct usb_interface *intf;
+
+ intf = to_usb_interface(hdev->dev.parent);
+ ep = intf->cur_altsetting->endpoint;
+ if (ep)
+ return ep->desc.bEndpointAddress;
+
+ return -ENODEV;
+}
+
+static int claw_gamepad_mode_event(struct claw_drvdata *drvdata,
+ struct claw_command_report *cmd_rep)
+{
+ if (cmd_rep->data[0] >= ARRAY_SIZE(claw_gamepad_mode_text) ||
+ !claw_gamepad_mode_text[cmd_rep->data[0]] ||
+ cmd_rep->data[1] >= ARRAY_SIZE(claw_mkeys_function_text))
+ return -EINVAL;
+
+ drvdata->gamepad_mode = cmd_rep->data[0];
+ drvdata->mkeys_function = cmd_rep->data[1];
+
+ return 0;
+}
+
+static int claw_raw_event(struct claw_drvdata *drvdata, struct hid_report *report,
+ u8 *data, int size)
+{
+ struct claw_command_report *cmd_rep;
+ int ret = 0;
+
+ if (size != CLAW_PACKET_SIZE)
+ return 0;
+
+ cmd_rep = (struct claw_command_report *)data;
+
+ if (cmd_rep->report_id != CLAW_INPUT_REPORT_ID || cmd_rep->header_tail != 0x3c)
+ return 0;
+
+ dev_dbg(&drvdata->hdev->dev, "Rx data as raw input report: [%*ph]\n",
+ CLAW_PACKET_SIZE, data);
+
+ switch (cmd_rep->cmd) {
+ case CLAW_COMMAND_TYPE_GAMEPAD_MODE_ACK:
+ ret = claw_gamepad_mode_event(drvdata, cmd_rep);
+ break;
+ case CLAW_COMMAND_TYPE_ACK:
+ break;
+ default:
+ dev_dbg(&drvdata->hdev->dev, "Unknown command: %x\n", cmd_rep->cmd);
+ return 0;
+ }
+
+ complete(&drvdata->send_cmd_complete);
+
+ return ret;
+}
+
+static int msi_raw_event(struct hid_device *hdev, struct hid_report *report,
+ u8 *data, int size)
+{
+ struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
+
+ if (!drvdata || (drvdata->ep != CLAW_XINPUT_CFG_INTF_IN &&
+ drvdata->ep != CLAW_DINPUT_CFG_INTF_IN))
+ return 0;
+
+ return claw_raw_event(drvdata, report, data, size);
+}
+
+static int claw_hw_output_report(struct hid_device *hdev, u8 index, u8 *data,
+ size_t len, unsigned int timeout)
+{
+ unsigned char *dmabuf __free(kfree) = NULL;
+ u8 header[] = { CLAW_OUTPUT_REPORT_ID, 0, 0, 0x3c, index };
+ struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
+ size_t header_size = ARRAY_SIZE(header);
+ int ret;
+
+ if (header_size + len > CLAW_PACKET_SIZE)
+ return -EINVAL;
+
+ /* We can't use a devm_alloc reusable buffer without side effects during suspend */
+ dmabuf = kzalloc(CLAW_PACKET_SIZE, GFP_KERNEL);
+ if (!dmabuf)
+ return -ENOMEM;
+
+ memcpy(dmabuf, header, header_size);
+ if (data && len)
+ memcpy(dmabuf + header_size, data, len);
+
+ /* Don't hold a mutex when timeout=0, those commands cause USB disconnect */
+ if (timeout) {
+ guard(mutex)(&drvdata->cfg_mutex);
+ reinit_completion(&drvdata->send_cmd_complete);
+ }
+
+ dev_dbg(&hdev->dev, "Send data as raw output report: [%*ph]\n",
+ CLAW_PACKET_SIZE, dmabuf);
+
+ ret = hid_hw_output_report(hdev, dmabuf, CLAW_PACKET_SIZE);
+ if (ret < 0)
+ return ret;
+
+ ret = ret == CLAW_PACKET_SIZE ? 0 : -EIO;
+ if (ret)
+ return ret;
+
+ if (timeout) {
+ ret = wait_for_completion_interruptible_timeout(&drvdata->send_cmd_complete,
+ msecs_to_jiffies(timeout));
+
+ dev_dbg(&hdev->dev, "Remaining timeout: %u\n", ret);
+ if (ret >= 0) /* preserve errors */
+ ret = ret == 0 ? -EBUSY : 0; /* timeout occurred : time remained */
+ }
+
+ return ret;
+}
+
+static ssize_t gamepad_mode_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct hid_device *hdev = to_hid_device(dev);
+ struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
+ u8 data[2] = { 0x00, drvdata->mkeys_function };
+ int i, ret = -EINVAL;
+
+ for (i = 0; i < ARRAY_SIZE(claw_gamepad_mode_text); i++) {
+ if (claw_gamepad_mode_text[i] && sysfs_streq(buf, claw_gamepad_mode_text[i])) {
+ ret = i;
+ break;
+ }
+ }
+ if (ret < 0)
+ return ret;
+
+ data[0] = ret;
+
+ ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_SWITCH_MODE, data, ARRAY_SIZE(data), 0);
+ if (ret)
+ return ret;
+
+ return count;
+}
+
+static ssize_t gamepad_mode_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct hid_device *hdev = to_hid_device(dev);
+ struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
+ int ret, i;
+
+ ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_READ_GAMEPAD_MODE, NULL, 0, 8);
+ if (ret)
+ return ret;
+
+ i = drvdata->gamepad_mode;
+
+ if (!claw_gamepad_mode_text[i] || claw_gamepad_mode_text[i][0] == '\0')
+ return sysfs_emit(buf, "unsupported\n");
+
+ return sysfs_emit(buf, "%s\n", claw_gamepad_mode_text[i]);
+}
+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;
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(claw_gamepad_mode_text); i++) {
+ if (!claw_gamepad_mode_text[i] || claw_gamepad_mode_text[i][0] == '\0')
+ continue;
+ count += sysfs_emit_at(buf, count, "%s ", claw_gamepad_mode_text[i]);
+ }
+
+ buf[count - 1] = '\n';
+
+ return count;
+}
+static DEVICE_ATTR_RO(gamepad_mode_index);
+
+static ssize_t mkeys_function_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct hid_device *hdev = to_hid_device(dev);
+ struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
+ u8 data[2] = { drvdata->gamepad_mode, 0x00 };
+ int i, ret = -EINVAL;
+
+ for (i = 0; i < ARRAY_SIZE(claw_mkeys_function_text); i++) {
+ if (claw_mkeys_function_text[i] && sysfs_streq(buf, claw_mkeys_function_text[i])) {
+ ret = i;
+ break;
+ }
+ }
+ if (ret < 0)
+ return ret;
+
+ data[1] = ret;
+
+ ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_SWITCH_MODE, data, ARRAY_SIZE(data), 0);
+ if (ret)
+ return ret;
+
+ return count;
+}
+
+static ssize_t mkeys_function_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ struct hid_device *hdev = to_hid_device(dev);
+ struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
+ int ret, i;
+
+ ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_READ_GAMEPAD_MODE, NULL, 0, 8);
+ if (ret)
+ return ret;
+
+ i = drvdata->mkeys_function;
+
+ if (i >= ARRAY_SIZE(claw_mkeys_function_text))
+ return sysfs_emit(buf, "unsupported\n");
+
+ return sysfs_emit(buf, "%s\n", claw_mkeys_function_text[i]);
+}
+static DEVICE_ATTR_RW(mkeys_function);
+
+static ssize_t mkeys_function_index_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ int i, count = 0;
+
+ for (i = 0; i < ARRAY_SIZE(claw_mkeys_function_text); i++)
+ count += sysfs_emit_at(buf, count, "%s ", claw_mkeys_function_text[i]);
+
+ buf[count - 1] = '\n';
+
+ return count;
+}
+static DEVICE_ATTR_RO(mkeys_function_index);
+
+static ssize_t reset_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct hid_device *hdev = to_hid_device(dev);
+ bool val;
+ int ret;
+
+ ret = kstrtobool(buf, &val);
+ if (ret)
+ return ret;
+
+ if (!val)
+ return -EINVAL;
+
+ ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_RESET_DEVICE, NULL, 0, 0);
+ if (ret < 0)
+ return ret;
+
+ return count;
+}
+static DEVICE_ATTR_WO(reset);
+
+static umode_t claw_gamepad_attr_is_visible(struct kobject *kobj, struct attribute *attr,
+ int n)
+{
+ struct hid_device *hdev = to_hid_device(kobj_to_dev(kobj));
+ struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
+
+ if (!drvdata) {
+ dev_warn(&hdev->dev,
+ "Failed to get drvdata from kobj. Gamepad attributes are not available.\n");
+ return 0;
+ }
+
+ return attr->mode;
+}
+
+static struct attribute *claw_gamepad_attrs[] = {
+ &dev_attr_gamepad_mode.attr,
+ &dev_attr_gamepad_mode_index.attr,
+ &dev_attr_mkeys_function.attr,
+ &dev_attr_mkeys_function_index.attr,
+ &dev_attr_reset.attr,
+ NULL,
+};
+
+static const struct attribute_group claw_gamepad_attr_group = {
+ .attrs = claw_gamepad_attrs,
+ .is_visible = claw_gamepad_attr_is_visible,
+};
+
+static void cfg_setup_fn(struct work_struct *work)
+{
+ struct delayed_work *dwork = container_of(work, struct delayed_work, work);
+ struct claw_drvdata *drvdata = container_of(dwork, struct claw_drvdata, cfg_setup);
+ int ret;
+
+ ret = claw_hw_output_report(drvdata->hdev, CLAW_COMMAND_TYPE_READ_GAMEPAD_MODE, NULL, 0, 8);
+ if (ret) {
+ dev_err(&drvdata->hdev->dev,
+ "Failed to setup device, can't read gamepad mode: %d\n", ret);
+ return;
+ }
+
+ /* Add sysfs attributes after we get the device state */
+ ret = devm_device_add_group(&drvdata->hdev->dev, &claw_gamepad_attr_group);
+ if (ret) {
+ dev_err(&drvdata->hdev->dev,
+ "Failed to setup device, can't create gamepad attrs: %d\n", ret);
+ return;
+ }
+
+ kobject_uevent(&drvdata->hdev->dev.kobj, KOBJ_CHANGE);
+}
+
+static void cfg_resume_fn(struct work_struct *work)
+{
+ struct delayed_work *dwork = container_of(work, struct delayed_work, work);
+ struct claw_drvdata *drvdata = container_of(dwork, struct claw_drvdata, cfg_resume);
+ u8 data[2] = { drvdata->gamepad_mode, drvdata->mkeys_function };
+ int ret;
+
+ ret = claw_hw_output_report(drvdata->hdev, CLAW_COMMAND_TYPE_SWITCH_MODE, data,
+ ARRAY_SIZE(data), 0);
+ if (ret)
+ dev_err(&drvdata->hdev->dev, "Failed to set gamepad mode settings: %d\n", ret);
+}
+
+static int claw_probe(struct hid_device *hdev, u8 ep)
+{
+ struct claw_drvdata *drvdata;
+ int ret;
+
+ drvdata = devm_kzalloc(&hdev->dev, sizeof(*drvdata), GFP_KERNEL);
+ if (!drvdata)
+ return -ENOMEM;
+
+ hid_set_drvdata(hdev, drvdata);
+ drvdata->hdev = hdev;
+ drvdata->ep = ep;
+
+ mutex_init(&drvdata->cfg_mutex);
+ init_completion(&drvdata->send_cmd_complete);
+
+ /* For control interface: open the HID transport for sending commands. */
+ ret = hid_hw_open(hdev);
+ if (ret)
+ return ret;
+
+ INIT_DELAYED_WORK(&drvdata->cfg_resume, &cfg_resume_fn);
+ INIT_DELAYED_WORK(&drvdata->cfg_setup, &cfg_setup_fn);
+ schedule_delayed_work(&drvdata->cfg_setup, msecs_to_jiffies(500));
+
+ return 0;
+}
+
+static int msi_probe(struct hid_device *hdev, const struct hid_device_id *id)
+{
+ int ret;
+ u8 ep;
+
+ if (!hid_is_usb(hdev)) {
+ ret = -ENODEV;
+ goto err_probe;
+ }
+
+ ret = hid_parse(hdev);
+ if (ret)
+ goto err_probe;
+
+ /* Set quirk to create separate input devices per HID application */
+ hdev->quirks |= HID_QUIRK_INPUT_PER_APP | HID_QUIRK_MULTI_INPUT;
+ ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
+ if (ret)
+ goto err_probe;
+
+ /* For non-control interfaces (keyboard/mouse), allow userspace to grab the devices. */
+ ret = get_endpoint_address(hdev);
+ if (ret < 0)
+ goto err_stop_hw;
+
+ ep = ret;
+ if (ep == CLAW_XINPUT_CFG_INTF_IN || ep == CLAW_DINPUT_CFG_INTF_IN) {
+ ret = claw_probe(hdev, ep);
+ if (ret)
+ goto err_stop_hw;
+ }
+
+ return 0;
+
+err_stop_hw:
+ hid_hw_stop(hdev);
+err_probe:
+ return dev_err_probe(&hdev->dev, ret, "Failed to init device\n");
+}
+
+static void claw_remove(struct hid_device *hdev)
+{
+ struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
+
+ if (!drvdata) {
+ hid_hw_stop(hdev);
+ return;
+ }
+
+ cancel_delayed_work_sync(&drvdata->cfg_setup);
+ cancel_delayed_work_sync(&drvdata->cfg_resume);
+ hid_hw_close(hdev);
+}
+
+static void msi_remove(struct hid_device *hdev)
+{
+ int ret;
+ u8 ep;
+
+ ret = get_endpoint_address(hdev);
+ if (ret <= 0)
+ goto hw_stop;
+
+ ep = ret;
+ if (ep == CLAW_XINPUT_CFG_INTF_IN || ep == CLAW_DINPUT_CFG_INTF_IN)
+ claw_remove(hdev);
+
+hw_stop:
+ hid_hw_stop(hdev);
+}
+
+static int claw_resume(struct hid_device *hdev)
+{
+ struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
+
+ /* MCU can take up to 500ms to be ready after resume */
+ schedule_delayed_work(&drvdata->cfg_resume, msecs_to_jiffies(500));
+ return 0;
+}
+
+static int msi_resume(struct hid_device *hdev)
+{
+ int ret;
+ u8 ep;
+
+ ret = get_endpoint_address(hdev);
+ if (ret <= 0)
+ return 0;
+
+ ep = ret;
+ if (ep == CLAW_XINPUT_CFG_INTF_IN || ep == CLAW_DINPUT_CFG_INTF_IN)
+ return claw_resume(hdev);
+
+ return 0;
+}
+
+static const struct hid_device_id msi_devices[] = {
+ { HID_USB_DEVICE(USB_VENDOR_ID_MSI_2, USB_DEVICE_ID_MSI_CLAW_XINPUT) },
+ { HID_USB_DEVICE(USB_VENDOR_ID_MSI_2, USB_DEVICE_ID_MSI_CLAW_DINPUT) },
+ { HID_USB_DEVICE(USB_VENDOR_ID_MSI_2, USB_DEVICE_ID_MSI_CLAW_DESKTOP) },
+ { HID_USB_DEVICE(USB_VENDOR_ID_MSI_2, USB_DEVICE_ID_MSI_CLAW_BIOS) },
+ { }
+};
+MODULE_DEVICE_TABLE(hid, msi_devices);
+
+static struct hid_driver msi_driver = {
+ .name = "hid-msi",
+ .id_table = msi_devices,
+ .raw_event = msi_raw_event,
+ .probe = msi_probe,
+ .remove = msi_remove,
+ .resume = msi_resume,
+};
+module_hid_driver(msi_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Denis Benato <denis.benato@linux.dev>");
+MODULE_AUTHOR("Zhouwang Huang <honjow311@gmail.com>");
+MODULE_AUTHOR("Derek J. Clark <derekjohn.clark@gmail.com>");
+MODULE_DESCRIPTION("HID driver for MSI Claw Handheld PC gamepads");
--
2.53.0
^ permalink raw reply related
* [PATCH v2 0/4] Add MSI Claw HID Configuration Driver
From: Derek J. Clark @ 2026-05-13 23:14 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: Pierre-Loup A . Griffais, Denis Benato, Zhouwang Huang,
Derek J . Clark, linux-input, linux-doc, linux-kernel
This series adds an HID Configuration driver for the MSI Claw line of
Handheld Gaming PC's. The MSI Claw HID interface provides multiple
features, such as the ability to switch between xinput, dinput, and a
desktop mode, RGB control, rumble intensity, and mapping of the rear "M"
keys. There are additional gamepad modes that are not included in this
driver as they appear to be used in assembly line testing or are
incomplete in the firmware. During my testing I found them to be unstable.
The initial version of this driver was written by Denis Benato, which
contained the initial reverse-engineering and implementation for the
gamepad mode switching. This work was later expanded by Zhouwang Huang
to include more gamepad modes and additional features. Finally, I
refactored the entire driver, fixed multiple bugs, and refined the overall
format to conform to kernel driver best practices and style guide.
Claude was used initially by Zhouwang Huang to quickly parse HID captures
during the reverse-engineering of some of the features. Since Claude had
already been used, as a test of its capabilities I had it implement the
rumble intensity attribute after I had already rewritten most of the
driver, which I then manually edited to fix some mistakes. I also used
Claude to review the driver and these patches for any mistakes and bugs.
Assisted-by: Claude:claude-sonnet-4-6
Co-developed-by: Denis Benato <denis.benato@linux.dev>
Signed-off-by: Denis Benato <denis.benato@linux.dev>
Co-developed-by: Zhouwang Huang <honjow311@gmail.com>
Signed-off-by: Zhouwang Huang <honjow311@gmail.com>
Signed-off-by: Derek J. Clark <derekjohn.clark@gmail.com>
---
v2:
- Use mutexes to guard SYNC_TO_ROM calls and pending_profile calls.
- Rename driver to hid-msi and add generic entrypoints for
probe/resume/remove that call claw specific functions in order to
future proof the driver for other MSI HID interfaces.
- Fix various bugs and formatting issues.
Derek J. Clark (4):
HID: hid-msi: Add MSI Claw configuration driver
HID: hid-msi: Add M-key mapping attributes
HID: hid-msi: Add RGB control interface
HID: hid-msi: Add Rumble Intensity Attributes
MAINTAINERS | 6 +
drivers/hid/Kconfig | 12 +
drivers/hid/Makefile | 1 +
drivers/hid/hid-ids.h | 6 +
drivers/hid/hid-msi.c | 1663 +++++++++++++++++++++++++++++++++++++++++
5 files changed, 1688 insertions(+)
create mode 100644 drivers/hid/hid-msi.c
--
2.53.0
^ permalink raw reply
* Re: [PATCH v1] Input: atlas - Check ACPI_COMPANION() against NULL
From: sashiko-bot @ 2026-05-13 22:58 UTC (permalink / raw)
To: Rafael J. Wysocki; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <8696590.T7Z3S40VBb@rafael.j.wysocki>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Incomplete Fix for driver_override Abuse (Global State Corruption)
--
commit ef51cbfbb2dd379fb06abd9c22db66e86bb70eb3
Author: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Input: atlas - Check ACPI_COMPANION() against NULL
This commit adds a check to ensure the ACPI companion device is present in
the atlas_btns driver to prevent issues when a device is forced to match
via device_match_driver_override().
> diff --git a/drivers/input/misc/atlas_btns.c b/drivers/input/misc/atlas_btns.c
> index 47b31725e8507..835ad45a9d65c 100644
> --- a/drivers/input/misc/atlas_btns.c
> +++ b/drivers/input/misc/atlas_btns.c
> @@ -60,11 +60,15 @@ static acpi_status acpi_atlas_button_handler(u32 function,
>
> static int atlas_acpi_button_probe(struct platform_device *pdev)
> {
> - struct acpi_device *device = ACPI_COMPANION(&pdev->dev);
> + struct acpi_device *device;
> acpi_status status;
> int i;
> int err;
>
> + device = ACPI_COMPANION(&pdev->dev);
> + if (!device)
> + return -ENODEV;
> +
> input_dev = input_allocate_device();
The commit message mentions this check is to protect against
device_match_driver_override() forcing a match with unexpected devices.
If driver_override is used to force-bind this driver to multiple platform
devices that do have ACPI companions, could this cause issues with the global
input_dev variable?
Since input_dev is overwritten on each successful probe, it looks like earlier
allocated input devices would be leaked if multiple bindings succeed.
Additionally, if these devices are later unbound, or if a subsequent probe
fails after modifying input_dev, could this lead to a double-free or null
pointer dereference when the shared input_dev pointer is cleaned up?
Would it make sense to either reject multiple bindings or store the input
device in the platform device's private data using platform_set_drvdata()?
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/8696590.T7Z3S40VBb@rafael.j.wysocki?part=1
^ permalink raw reply
* Re: [PATCH] HID: lenovo: Restore Fn-lock LED state on resume for tp10ubkbd devices
From: sashiko-bot @ 2026-05-13 22:27 UTC (permalink / raw)
To: Kean; +Cc: linux-input, dmitry.torokhov
In-Reply-To: <20260512142229.4098386-1-rh_king@163.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Unprotected state modifications / TOCTOU Race leading to Deadlock
--
commit 7c663fefe9bb0e01bc7e377e27b713f9cd82c9d8
Author: Kean <rh_king@163.com>
HID: lenovo: Restore Fn-lock LED state on resume for tp10ubkbd devices
This commit updates the reset_resume callback to restore the Fn-lock LED
state for several ThinkPad tablet keyboards. It does this by reading the
driver's cached state and calling lenovo_led_set_tp10ubkbd to update the
hardware.
> diff --git a/drivers/hid/hid-lenovo.c b/drivers/hid/hid-lenovo.c
> --- a/drivers/hid/hid-lenovo.c
> +++ b/drivers/hid/hid-lenovo.c
[ ... ]
> @@ -1424,6 +1424,8 @@ static int lenovo_probe(struct hid_device *hdev,
>
> static int lenovo_reset_resume(struct hid_device *hdev)
> {
> + struct lenovo_drvdata *data;
> +
> switch (hdev->product) {
> case USB_DEVICE_ID_LENOVO_CUSBKBD:
> case USB_DEVICE_ID_LENOVO_TPIIUSBKBD:
> @@ -1431,6 +1433,17 @@ static int lenovo_reset_resume(struct hid_device *hdev)
> lenovo_features_set_cptkbd(hdev);
>
> break;
> + case USB_DEVICE_ID_LENOVO_X12_TAB:
> + case USB_DEVICE_ID_LENOVO_X12_TAB2:
> + case USB_DEVICE_ID_LENOVO_TP10UBKBD:
> + case USB_DEVICE_ID_LENOVO_X1_TAB:
> + case USB_DEVICE_ID_LENOVO_X1_TAB2:
> + case USB_DEVICE_ID_LENOVO_X1_TAB3:
> + data = hid_get_drvdata(hdev);
> + if (data)
> + lenovo_led_set_tp10ubkbd(hdev, TP10UBKBD_FN_LOCK_LED,
> + data->fn_lock ? LED_ON : LED_OFF);
Could this new call to lenovo_led_set_tp10ubkbd() cause a deadlock during
runtime resume?
When userspace writes to the fn_lock sysfs attribute, it eventually calls
lenovo_led_set_tp10ubkbd() which acquires led_report_mutex before issuing
the raw HID request:
lenovo_led_set_tp10ubkbd()
mutex_lock(&data->led_report_mutex);
...
hid_hw_raw_request(...)
If the device is runtime-suspended and requires a reset-resume,
hid_hw_raw_request() will trigger usb_autopm_get_interface() to
synchronously wake the device. This causes the USB core to execute
lenovo_reset_resume() in the same thread:
lenovo_reset_resume()
lenovo_led_set_tp10ubkbd()
mutex_lock(&data->led_report_mutex);
Since led_report_mutex is already held by the initial sysfs write call,
will this result in an immediate self-deadlock?
Additionally, if a resume is initiated by a remote wakeup via a PM worker
thread while a sysfs write is concurrently holding led_report_mutex, could
this lead to an ABBA deadlock between the sysfs thread and the PM worker
thread?
> + break;
> default:
> break;
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260512142229.4098386-1-rh_king@163.com?part=1
^ permalink raw reply
* Re: [PATCH] Input: synaptics - add LEN2058 to SMBus passlist for ThinkPad E490
From: Dmitry Torokhov @ 2026-05-13 21:27 UTC (permalink / raw)
To: contacto; +Cc: linux-input, linux-kernel
In-Reply-To: <20260513045749.185969-1-contacto@bazaes.cl>
On Wed, May 13, 2026 at 12:57:44AM -0400, contacto@bazaes.cl wrote:
> From: Nicolás Bazaes <contacto@bazaes.cl>
>
> The Lenovo ThinkPad E490 (PNP ID: LEN2058) has a Synaptics TM3471-020
> touchpad that supports SMBus/RMI4 mode but is not listed in
> smbus_pnp_ids[]. Without this entry, psmouse ignores the
> synaptics_intertouch parameter and falls back to PS/2 mode even when
> explicitly requested.
>
> Adding LEN2058 to the passlist enables RMI4 over SMBus, which provides
> better multitouch support and matches the behavior of similar ThinkPad
> models already in the list (E480/LEN2054, E580/LEN2055).
>
> Tested on ThinkPad E490 with kernel 7.0.5-zen1 and Arch Linux.
>
> Signed-off-by: Nicolás Bazaes <contacto@bazaes.cl>
> Assisted-by: Claude <claude.ai>
Thank you for the patch. From checkpatch.pl:
WARNING: Assisted-by expects 'AGENT_NAME:MODEL_VERSION [TOOL1] [TOOL2]' format
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH] Input: elan_i2c - prevent division by zero on invalid device parameters
From: Dmitry Torokhov @ 2026-05-13 21:00 UTC (permalink / raw)
To: Ranjan Kumar; +Cc: bleung, dusonlin, bentiss, linux-input, linux-kernel
In-Reply-To: <20260513073908.3326178-1-kumarranja@chromium.org>
Hi Ranjan,
On Wed, May 13, 2026 at 07:39:08AM +0000, Ranjan Kumar wrote:
> The Elan I2C touchpad driver queries the device for its physical
> dimensions and trace counts to calculate the device resolution and width.
> However, if the device firmware or device tree provides invalid zero
> values for x_traces, y_traces, x_mm, or y_mm, it results in a fatal
> division-by-zero exception leading to a kernel panic during device probe.
>
Neither crashing nor aborting probe with -EINVAL is great as we will not
be able to flash another firmware.
How do we get into this state? Is this something actually observed or
just theoretical?
> Add sanity checks to ensure these physical parameters are non-zero
> before performing the division. If invalid values are detected, log an
> error and return -EINVAL to gracefully abort the initialization and
> maintain system stability.
That is hardly "stability" for a laptop when its pointer is not working.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH v5 2/2] Input: isa1200 - new driver for Imagis ISA1200
From: sashiko-bot @ 2026-05-13 20:44 UTC (permalink / raw)
To: Svyatoslav Ryhel
Cc: linux-input, robh, krzk+dt, dmitry.torokhov, devicetree, conor+dt
In-Reply-To: <20260512102445.55372-3-clamor95@gmail.com>
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] Flawed interaction between isa1200_stop, suspend, and isa->level leads to lost haptic state on resume and power leaks.
- [High] Missing workqueue cancellation and timer cleanup on driver unbind causes Use-After-Free.
- [High] Data races around hardware state transitions lead to resource leaks and I2C corruption.
- [Medium] Stack out-of-bounds read due to unvalidated GPIO descriptor count.
--
commit 87a2f648deb0c974007e802b082c1ec62d4fb097
Author: Linus Walleij <linusw@kernel.org>
Input: isa1200 - new driver for Imagis ISA1200
This commit introduces a new input driver for the Imagis ISA1200 haptic
feedback unit, adding support for controlling its motors over I2C and
PWM interfaces.
> diff --git a/drivers/input/misc/isa1200.c b/drivers/input/misc/isa1200.c
> new file mode 100644
> index 0000000000000..ff82252a08e1b
> --- /dev/null
> +++ b/drivers/input/misc/isa1200.c
[ ... ]
> +static void isa1200_start(struct isa1200 *isa)
> +{
> + struct isa1200_config *config = &isa->config;
> + struct device *dev = &isa->input->dev;
> + struct pwm_state state;
> + u8 hctrl0 = 0, hctrl1 = 0;
> + DECLARE_BITMAP(values, ISA1200_EN_PINS_MAX);
> + int err;
> +
> + if (!isa->active) {
> + err = regulator_bulk_enable(ARRAY_SIZE(isa1200_supplies),
> + isa->supplies);
Are isa1200_start() and isa1200_stop() vulnerable to data races on the
isa->active state?
The active flag is read and modified here without holding any locks.
If the workqueue and PM resume callbacks execute concurrently, could both
threads observe isa->active as false? This might lead to double
initialization of the clocks and regulators, leaking their reference
counts when isa1200_stop() only disables them once.
[ ... ]
> +static void isa1200_stop(struct isa1200 *isa)
> +{
[ ... ]
> + isa->active = false;
> + isa->level = 0;
> +}
[ ... ]
> +static int isa1200_of_probe(struct i2c_client *client)
> +{
[ ... ]
> + isa->enable_gpios = devm_gpiod_get_array_optional(dev, "control",
> + GPIOD_OUT_LOW);
Should this validate the number of returned GPIO descriptors against
ISA1200_EN_PINS_MAX?
In isa1200_start() and isa1200_stop(), a local bitmap is declared on the
stack with DECLARE_BITMAP(values, ISA1200_EN_PINS_MAX). If a malformed
device tree provides more GPIOs than expected, could
gpiod_multi_set_value_cansleep() read past the stack bounds of the values
bitmap?
[ ... ]
> +static int isa1200_probe(struct i2c_client *client)
> +{
[ ... ]
> + INIT_WORK(&isa->play_work, isa1200_play_work);
The isa structure is allocated via devm_kzalloc() and will be freed
automatically when the device is unbound. Because there is no remove
callback or devm action to cancel isa->play_work, could the workqueue
execute after unbind and cause a use-after-free if an effect is playing
during unbind?
Similarly, the ff_memless timer is tied to the input_dev, which can
outlive the I2C device if userspace holds an open file descriptor. Does
the timer also need to be cleaned up on driver unbind to prevent it from
accessing freed memory?
[ ... ]
> +static int isa1200_suspend(struct device *dev)
> +{
> + struct isa1200 *isa = dev_get_drvdata(dev);
> +
> + guard(mutex)(&isa->input->mutex);
> +
> + if (input_device_enabled(isa->input)) {
> + cancel_work_sync(&isa->play_work);
> + if (isa->level)
> + isa1200_stop(isa);
> + }
> +
> + return 0;
> +}
If the user has already requested the motor to stop (level is 0), this
skips calling isa1200_stop(). If the workqueue is cancelled before it
gets a chance to shut down the hardware, would this leave the device
active and leak power throughout suspend?
Additionally, looking at the resume function:
[ ... ]
> +static int isa1200_resume(struct device *dev)
> +{
> + struct isa1200 *isa = dev_get_drvdata(dev);
> +
> + guard(mutex)(&isa->input->mutex);
> +
> + if (input_device_enabled(isa->input))
> + if (isa->level)
> + isa1200_start(isa);
> +
> + return 0;
> +}
Since isa1200_stop() unconditionally sets isa->level to 0 during suspend,
will this check always evaluate to false, causing the driver to permanently
lose its haptic state after resuming?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260512102445.55372-1-clamor95@gmail.com?part=2
^ permalink raw reply
* Re: [PATCH v5 1/2] dt-bindings: input: Document Imagis ISA1200 haptic motor driver
From: sashiko-bot @ 2026-05-13 20:15 UTC (permalink / raw)
To: Svyatoslav Ryhel
Cc: robh, linux-input, conor+dt, dmitry.torokhov, krzk+dt, devicetree
In-Reply-To: <20260512102445.55372-2-clamor95@gmail.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Missing scalar type definitions for `imagis,period-ns` and `imagis,duty-cycle-ns` cause schema bounds validation to be silently bypassed.
--
commit c2a7ef2ca3d62317c69bc5b6a450107172eeeac7
Author: Svyatoslav Ryhel <clamor95@gmail.com>
dt-bindings: input: Document Imagis ISA1200 haptic motor driver
This commit documents the Imagis ISA1200 haptic motor driver. It introduces
a new YAML binding schema for the driver, specifying properties like
control-gpios, clocks, and internal PWM configurations.
> diff --git a/Documentation/devicetree/bindings/input/imagis,isa1200.yaml b/Documentation/devicetree/bindings/input/imagis,isa1200.yaml
> new file mode 100644
> index 0000000000000..bbe6f99d39c19
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/imagis,isa1200.yaml
[ ... ]
> @@ -0,0 +1,140 @@
[ ... ]
> + imagis,mode:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + description: |
> + Defines the motor type isa1200 drives
> + 0 - LRA (Linear Resonant Actuator)
> + 1 - ERM (Eccentric Rotating Mass)
> + enum: [0, 1]
> + default: 0
> +
> + imagis,period-ns:
> + description:
> + Period of the internal PWM channel in nanoseconds.
> + minimum: 10000
> + maximum: 30000
> +
> + imagis,duty-cycle-ns:
> + description:
> + Duty cycle of the external/internal PWM channel in nanoseconds,
> + defaults to 50% of the channel's period
Will the minimum and maximum bounds on imagis,period-ns be silently ignored
during validation?
The core property-units.yaml schema automatically assigns an array type to
properties ending in -ns. Without an explicit reference to
/schemas/types.yaml#/definitions/uint32, these properties are evaluated as
arrays rather than scalar integers.
In standard JSON Schema, numeric constraints like minimum and maximum are
ignored when applied to an array instance. Adding the uint32 type reference
for both imagis,period-ns and imagis,duty-cycle-ns would ensure the
constraints are properly enforced on a single scalar value.
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260512102445.55372-1-clamor95@gmail.com?part=1
^ permalink raw reply
* Re: [PATCH] input/evdev: move kill_fasync() outside buffer_lock to fix SOFTIRQ deadlock
From: Dmitry Torokhov @ 2026-05-13 20:01 UTC (permalink / raw)
To: Rik van Riel; +Cc: linux-input, linux-kernel, kernel-team
In-Reply-To: <20260513115000.77ec5975@fangorn>
Hi Rik,
On Wed, May 13, 2026 at 11:50:00AM -0400, Rik van Riel wrote:
> buffer_lock is a SOFTIRQ-safe spinlock. kill_fasync() acquires fa_lock
> (SOFTIRQ-unsafe), creating a potential SOFTIRQ-safe->SOFTIRQ-unsafe lock
> ordering violation that lockdep flags as a deadlock.
>
> Fix by moving the kill_fasync() call to evdev_pass_values() after
> buffer_lock is released, alongside the existing wake_up_interruptible_poll().
>
> The wakeup condition check is the same in __pass_event() and
> evdev_pass_values()
Does this really fix anything? This code is running holding
input->event_lock with IRQs off...
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH v4 5/6 RESEND] mfd: motorola-cpcap: diverge configuration per-board
From: Svyatoslav Ryhel @ 2026-05-13 16:47 UTC (permalink / raw)
To: Lee Jones
Cc: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Pavel Machek, David Lechner, Tony Lindgren, linux-input,
devicetree, linux-kernel, linux-leds
In-Reply-To: <20260513140550.GD305027@google.com>
ср, 13 трав. 2026 р. о 17:05 Lee Jones <lee@kernel.org> пише:
>
> On Thu, 07 May 2026, Svyatoslav Ryhel wrote:
>
> > чт, 7 трав. 2026 р. о 17:05 Lee Jones <lee@kernel.org> пише:
> > >
> > > On Tue, 28 Apr 2026, Svyatoslav Ryhel wrote:
> > >
> > > > MFD have rigid subdevice structure which does not allow flexible dynamic
> > > > subdevice linking. Address this by diverging CPCAP subdevice composition
> > > > to take into account board specific configuration.
> > > >
> > > > Create a common default subdevice composition, rename existing subdevice
> > > > composition into cpcap_mapphone_mfd_devices since it targets mainly
> > > > Mapphone board.
> > > >
> > > > Removed st,6556002 as it is no longer applicable to all cases and
> > > > duplicates motorola,cpcap, which is used as the default composition.
> > > >
> > > > Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> > > > ---
> > >
> > > Changelog?
> > >
> >
> > Changelog is in the cover.
> >
> > > > drivers/mfd/motorola-cpcap.c | 101 ++++++++++++++++++++++++++++-------
> > > > 1 file changed, 83 insertions(+), 18 deletions(-)
> > > >
> > > > diff --git a/drivers/mfd/motorola-cpcap.c b/drivers/mfd/motorola-cpcap.c
> > > > index d8243b956f87..516d1e33affa 100644
> > > > --- a/drivers/mfd/motorola-cpcap.c
> > > > +++ b/drivers/mfd/motorola-cpcap.c
> > > > @@ -12,6 +12,7 @@
> > > > #include <linux/kernel.h>
> > > > #include <linux/module.h>
> > > > #include <linux/mod_devicetable.h>
> > > > +#include <linux/property.h>
> > > > #include <linux/regmap.h>
> > > > #include <linux/sysfs.h>
> > > >
> > > > @@ -24,10 +25,16 @@
> > > > #define CPCAP_REGISTER_SIZE 4
> > > > #define CPCAP_REGISTER_BITS 16
> > > >
> > > > +struct cpcap_chip_data {
> > > > + const struct mfd_cell *mfd_devices;
> > > > + unsigned int num_devices;
> > > > +};
> > >
> > > This is a red flag.
> > >
> > > > struct cpcap_ddata {
> > > > struct spi_device *spi;
> > > > struct regmap_irq *irqs;
> > > > struct regmap_irq_chip_data *irqdata[CPCAP_NR_IRQ_CHIPS];
> > > > + const struct cpcap_chip_data *cdata;
> > > > const struct regmap_config *regmap_conf;
> > > > struct regmap *regmap;
> > > > };
> > > > @@ -195,20 +202,6 @@ static int cpcap_init_irq(struct cpcap_ddata *cpcap)
> > > > return 0;
> > > > }
> > > >
> > > > -static const struct of_device_id cpcap_of_match[] = {
> > > > - { .compatible = "motorola,cpcap", },
> > > > - { .compatible = "st,6556002", },
> > > > - {},
> > > > -};
> > > > -MODULE_DEVICE_TABLE(of, cpcap_of_match);
> > > > -
> > > > -static const struct spi_device_id cpcap_spi_ids[] = {
> > > > - { .name = "cpcap", },
> > > > - { .name = "6556002", },
> > > > - {},
> > > > -};
> > > > -MODULE_DEVICE_TABLE(spi, cpcap_spi_ids);
> > > > -
> > > > static const struct regmap_config cpcap_regmap_config = {
> > > > .reg_bits = 16,
> > > > .reg_stride = 4,
> > > > @@ -241,7 +234,56 @@ static int cpcap_resume(struct device *dev)
> > > >
> > > > static DEFINE_SIMPLE_DEV_PM_OPS(cpcap_pm, cpcap_suspend, cpcap_resume);
> > > >
> > > > -static const struct mfd_cell cpcap_mfd_devices[] = {
> > > > +static const struct mfd_cell cpcap_default_mfd_devices[] = {
> > > > + {
> > > > + .name = "cpcap_adc",
> > > > + .of_compatible = "motorola,cpcap-adc",
> > > > + }, {
> > > > + .name = "cpcap_battery",
> > > > + .of_compatible = "motorola,cpcap-battery",
> > > > + }, {
> > > > + .name = "cpcap-regulator",
> > > > + .of_compatible = "motorola,cpcap-regulator",
> > > > + }, {
> > > > + .name = "cpcap-rtc",
> > > > + .of_compatible = "motorola,cpcap-rtc",
> > > > + }, {
> > > > + .name = "cpcap-pwrbutton",
> > > > + .of_compatible = "motorola,cpcap-pwrbutton",
> > > > + }, {
> > > > + .name = "cpcap-usb-phy",
> > > > + .of_compatible = "motorola,cpcap-usb-phy",
> > > > + }, {
> > > > + .name = "cpcap-led",
> > > > + .id = 0,
> > > > + .of_compatible = "motorola,cpcap-led-red",
> > > > + }, {
> > > > + .name = "cpcap-led",
> > > > + .id = 1,
> > > > + .of_compatible = "motorola,cpcap-led-green",
> > > > + }, {
> > > > + .name = "cpcap-led",
> > > > + .id = 2,
> > > > + .of_compatible = "motorola,cpcap-led-blue",
> > > > + }, {
> > > > + .name = "cpcap-led",
> > > > + .id = 3,
> > > > + .of_compatible = "motorola,cpcap-led-adl",
> > > > + }, {
> > > > + .name = "cpcap-led",
> > > > + .id = 4,
> > > > + .of_compatible = "motorola,cpcap-led-cp",
> > > > + }, {
> > > > + .name = "cpcap-codec",
> > > > + },
> > > > +};
> > > > +
> > > > +static const struct cpcap_chip_data cpcap_default_data = {
> > > > + .mfd_devices = cpcap_default_mfd_devices,
> > > > + .num_devices = ARRAY_SIZE(cpcap_default_mfd_devices),
> > > > +};
> > > > +
> > > > +static const struct mfd_cell cpcap_mapphone_mfd_devices[] = {
> > > > {
> > > > .name = "cpcap_adc",
> > > > .of_compatible = "motorola,mapphone-cpcap-adc",
> > > > @@ -285,7 +327,12 @@ static const struct mfd_cell cpcap_mfd_devices[] = {
> > > > .of_compatible = "motorola,cpcap-led-cp",
> > > > }, {
> > > > .name = "cpcap-codec",
> > > > - }
> > > > + },
> > > > +};
> > > > +
> > > > +static const struct cpcap_chip_data cpcap_mapphone_data = {
> > > > + .mfd_devices = cpcap_mapphone_mfd_devices,
> > > > + .num_devices = ARRAY_SIZE(cpcap_mapphone_mfd_devices),
> > > > };
> > > >
> > > > static int cpcap_probe(struct spi_device *spi)
> > > > @@ -297,9 +344,17 @@ static int cpcap_probe(struct spi_device *spi)
> > > > if (!cpcap)
> > > > return -ENOMEM;
> > > >
> > > > + cpcap->cdata = device_get_match_data(&spi->dev);
> > > > + if (!cpcap->cdata)
> > > > + return -ENODEV;
> > > > +
> > > > cpcap->spi = spi;
> > > > spi_set_drvdata(spi, cpcap);
> > > >
> > > > @@ -331,16 +382,24 @@ static int cpcap_probe(struct spi_device *spi)
> > > > spi->dev.coherent_dma_mask = 0;
> > > > spi->dev.dma_mask = &spi->dev.coherent_dma_mask;
> > > >
> > > > - return devm_mfd_add_devices(&spi->dev, 0, cpcap_mfd_devices,
> > > > - ARRAY_SIZE(cpcap_mfd_devices), NULL, 0, NULL);
> > > > + return devm_mfd_add_devices(&spi->dev, 0, cpcap->cdata->mfd_devices,
> > > > + cpcap->cdata->num_devices, NULL, 0, NULL);
> > > > }
> > > >
> > > > +static const struct of_device_id cpcap_of_match[] = {
> > > > + { .compatible = "motorola,cpcap", .data = &cpcap_default_data },
> > > > + { .compatible = "motorola,mapphone-cpcap", .data = &cpcap_mapphone_data },
> > >
> > > We don't allow data from one device registration API (MFD) to be passed
> > > through another (OF) because it tends to lead to all sorts of "creative
> > > solutions". Pass a value instead and match on that in a switch()
> > > statement like all of the other MFD drivers do.
> > >
> >
> > You don't allow this. I have not seen this enforced anywhere in the
> > kernel except the mfd subsystem. Fine, does not matter, if this makes
> > you happy I will adjust.
>
> Where else would this rule be applicable? I can't think of anywhere.
>
I have adjusted as you requested and already sent v5.
> --
> Lee Jones
^ permalink raw reply
* [PATCH 7/7] hid-asus: add prod-id, quirk for Zenbook Duo keyboard
From: Paolo Pisati @ 2026-05-13 16:32 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, linux-input
In-Reply-To: <20260513163248.16483-1-p.pisati@gmail.com>
From: Luke Jones <luke@ljones.dev>
The 2024/2025/2026 ASUS Zenbook Duo has a similar keyboard setup as many of the ROG
range of laptops, with the same init sequence and control for backlight.
Enable the keyboard control and backlight by:
- adding the product ID for 2024/2025/2026 Zenbook Duo keyboard MCU
- adding the Bluetooth Zenbook Duo keyboard IDs
- use Fn-lock on Zenbook Duo keyboards
Intended for the UX8406-style product line with the fully removable keyboard.
Signed-off-by: Luke Jones <luke@ljones.dev>
Signed-off-by: Joshua Leivenzon <hacker1024@users.sourceforge.net>
Signed-off-by: Joshua Leivenzon <hacker1024@users.sourceforge.net>
Signed-off-by: Joshua Leivenzon <hacker1024@users.sourceforge.net>
Signed-off-by: Paolo Pisati <p.pisati@gmail.com>
Signed-off-by: Ivan Levchenko <me@livan.pro>
---
drivers/hid/hid-asus.c | 18 ++++++++++++++++++
drivers/hid/hid-ids.h | 6 ++++++
2 files changed, 24 insertions(+)
diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index 37e3aec612b6d..ba093f11f802f 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -1583,6 +1583,24 @@ static const struct hid_device_id asus_devices[] = {
{ HID_USB_DEVICE(USB_VENDOR_ID_ASUSTEK,
USB_DEVICE_ID_ASUSTEK_ROG_Z13_LIGHTBAR),
QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD },
+ { HID_USB_DEVICE(USB_VENDOR_ID_ASUSTEK,
+ USB_DEVICE_ID_ASUSTEK_ZENBOOK_DUO_UX8406MA_KEYBOARD),
+ QUIRK_USE_KBD_BACKLIGHT | QUIRK_HID_FN_LOCK | QUIRK_ZENBOOK_DUO_KEYBOARD },
+ { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_ASUSTEK,
+ BT_DEVICE_ID_ASUSTEK_ZENBOOK_DUO_UX8406MA_KEYBOARD),
+ QUIRK_USE_KBD_BACKLIGHT | QUIRK_HID_FN_LOCK | QUIRK_ZENBOOK_DUO_KEYBOARD },
+ { HID_USB_DEVICE(USB_VENDOR_ID_ASUSTEK,
+ USB_DEVICE_ID_ASUSTEK_ZENBOOK_DUO_UX8406CA_KEYBOARD),
+ QUIRK_USE_KBD_BACKLIGHT | QUIRK_HID_FN_LOCK | QUIRK_ZENBOOK_DUO_KEYBOARD },
+ { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_ASUSTEK,
+ BT_DEVICE_ID_ASUSTEK_ZENBOOK_DUO_UX8406CA_KEYBOARD),
+ QUIRK_USE_KBD_BACKLIGHT | QUIRK_HID_FN_LOCK | QUIRK_ZENBOOK_DUO_KEYBOARD },
+ { HID_USB_DEVICE(USB_VENDOR_ID_ASUSTEK,
+ USB_DEVICE_ID_ASUSTEK_ZENBOOK_DUO_UX8407AA_KEYBOARD),
+ QUIRK_USE_KBD_BACKLIGHT | QUIRK_HID_FN_LOCK | QUIRK_ZENBOOK_DUO_KEYBOARD },
+ { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_ASUSTEK,
+ BT_DEVICE_ID_ASUSTEK_ZENBOOK_DUO_UX8407AA_KEYBOARD),
+ QUIRK_USE_KBD_BACKLIGHT | QUIRK_HID_FN_LOCK | QUIRK_ZENBOOK_DUO_KEYBOARD },
{ HID_USB_DEVICE(USB_VENDOR_ID_ASUSTEK,
USB_DEVICE_ID_ASUSTEK_ROG_NKEY_ALLY),
QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD | QUIRK_ROG_ALLY_XPAD},
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 0cf63742315bf..97dc451adac5a 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -227,6 +227,12 @@
#define USB_DEVICE_ID_ASUSTEK_ROG_NKEY_KEYBOARD 0x1866
#define USB_DEVICE_ID_ASUSTEK_ROG_NKEY_KEYBOARD2 0x19b6
#define USB_DEVICE_ID_ASUSTEK_ROG_Z13_FOLIO 0x1a30
+#define USB_DEVICE_ID_ASUSTEK_ZENBOOK_DUO_UX8406MA_KEYBOARD 0x1b2c
+#define BT_DEVICE_ID_ASUSTEK_ZENBOOK_DUO_UX8406MA_KEYBOARD 0x1b2d
+#define USB_DEVICE_ID_ASUSTEK_ZENBOOK_DUO_UX8406CA_KEYBOARD 0x1bf2
+#define BT_DEVICE_ID_ASUSTEK_ZENBOOK_DUO_UX8406CA_KEYBOARD 0x1bf3
+#define USB_DEVICE_ID_ASUSTEK_ZENBOOK_DUO_UX8407AA_KEYBOARD 0x1cd7
+#define BT_DEVICE_ID_ASUSTEK_ZENBOOK_DUO_UX8407AA_KEYBOARD 0x1cd8
#define USB_DEVICE_ID_ASUSTEK_ROG_Z13_LIGHTBAR 0x18c6
#define USB_DEVICE_ID_ASUSTEK_ROG_NKEY_ALLY 0x1abe
#define USB_DEVICE_ID_ASUSTEK_ROG_NKEY_ALLY_X 0x1b4c
--
2.53.0
^ permalink raw reply related
* [PATCH 6/7] hid-asus: Allow adding custom hotkey handler logic
From: Paolo Pisati @ 2026-05-13 16:32 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, linux-input
In-Reply-To: <20260513163248.16483-1-p.pisati@gmail.com>
From: Joshua Leivenzon <hacker1024@users.sourceforge.net>
Respond to platform profile cycle key:
Fn + F on UX8406MA.
May vary across devices.
Signed-off-by: Joshua Leivenzon <hacker1024@users.sourceforge.net>
---
drivers/hid/hid-asus.c | 18 ++++++++++++++++--
1 file changed, 16 insertions(+), 2 deletions(-)
diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index fe52fd63d61c6..37e3aec612b6d 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -29,6 +29,7 @@
#include <linux/input/mt.h>
#include <linux/usb.h> /* For to_usb_interface for T100 touchpad intf check */
#include <linux/power_supply.h>
+#include <linux/platform_profile.h>
#include <linux/leds.h>
#include "hid-ids.h"
@@ -362,8 +363,21 @@ static int asus_event(struct hid_device *hdev, struct hid_field *field,
if ((usage->hid & HID_USAGE_PAGE) == HID_UP_ASUSVENDOR &&
(usage->hid & HID_USAGE) != 0x00 &&
(usage->hid & HID_USAGE) != 0xff && !usage->type) {
- hid_warn(hdev, "Unmapped Asus vendor usagepage code 0x%02x\n",
- usage->hid & HID_USAGE);
+
+ /*
+ * Some reports do not map directly to standard keys, and need special
+ * handling.
+ */
+ switch (usage->hid & HID_USAGE) {
+ case 0x9d:
+ if (!value)
+ break;
+ return platform_profile_cycle();
+ break;
+ default:
+ hid_warn(hdev, "Unmapped Asus vendor usagepage code 0x%02x\n",
+ usage->hid & HID_USAGE);
+ }
}
if (usage->type == EV_KEY && value) {
--
2.53.0
^ permalink raw reply related
* [PATCH 5/7] hid-asus: Fix input mapping on dedicated vendor HID interfaces
From: Paolo Pisati @ 2026-05-13 16:32 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, linux-input
In-Reply-To: <20260513163248.16483-1-p.pisati@gmail.com>
From: Joshua Leivenzon <hacker1024@users.sourceforge.net>
Signed-off-by: Joshua Leivenzon <hacker1024@users.sourceforge.net>
---
drivers/hid/hid-asus.c | 44 ++++++++++++++++++++++++++++++++++++++++++
1 file changed, 44 insertions(+)
diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index cf9184ed2d3ec..fe52fd63d61c6 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -1376,6 +1376,30 @@ static void asus_remove(struct hid_device *hdev)
hid_hw_stop(hdev);
}
+/*
+ * Some USB keyboards, like the Zenbook Duo UX8406MA keyboard, have a dedicated
+ * USB interface for vendor-specific reports, separate to the generic HID
+ * keyboard or consumer control interfaces.
+ *
+ * The kernel does not register these vendor-specific interfaces as keyboards,
+ * or perform input mapping on them at all.
+ *
+ * To work around this, a fake keyboard input can be added to the
+ * vendor-specific interface's report descriptor. The kernel then combines it
+ * with the vendor-specific collections, and allows the interface to be used as
+ * a regular keyboard with our custom mappings.
+ */
+static const __u8 asus_fake_keyboard_rdesc[] = {
+ 0x05, 0x01, /* Usage Page (Generic Desktop) */
+ 0x09, 0x06, /* Usage (Keyboard) */
+ 0xa1, 0x01, /* Collection (Application) */
+ 0x85, 0x01, /* Report ID (1) */
+ 0x75, 0x08, /* Report Size (8) */
+ 0x95, 0x01, /* Report Count (1) */
+ 0x81, 0x00, /* Input (Data,Arr,Abs) */
+ 0xc0, /* End Collection */
+};
+
static const __u8 asus_g752_fixed_rdesc[] = {
0x19, 0x00, /* Usage Minimum (0x00) */
0x2A, 0xFF, 0x00, /* Usage Maximum (0xFF) */
@@ -1457,6 +1481,26 @@ static const __u8 *asus_report_fixup(struct hid_device *hdev, __u8 *rdesc,
}
}
+ if ((drvdata->quirks & QUIRK_ZENBOOK_DUO_KEYBOARD) &&
+ hid_is_usb(hdev) &&
+ to_usb_interface(hdev->dev.parent)->altsetting->desc.bInterfaceNumber == 4) {
+
+ __u8 *new_rdesc;
+ size_t new_size = *rsize + sizeof(asus_fake_keyboard_rdesc);
+
+ new_rdesc = devm_kzalloc(&hdev->dev, new_size, GFP_KERNEL);
+ if (new_rdesc == NULL)
+ return rdesc;
+
+ hid_info(hdev, "Injecting virtual Zenbook Duo keyboard usage page\n");
+
+ memcpy(new_rdesc, asus_fake_keyboard_rdesc, sizeof(asus_fake_keyboard_rdesc));
+ memcpy(new_rdesc + sizeof(asus_fake_keyboard_rdesc), rdesc, *rsize);
+
+ *rsize = new_size;
+ rdesc = new_rdesc;
+ }
+
if (drvdata->quirks & QUIRK_G752_KEYBOARD &&
*rsize == 75 && rdesc[61] == 0x15 && rdesc[62] == 0x00) {
/* report is missing usage minimum and maximum */
--
2.53.0
^ permalink raw reply related
* [PATCH 4/7] hid-asus: Remove more bogus zero bytes from some report descriptors
From: Paolo Pisati @ 2026-05-13 16:32 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, linux-input
In-Reply-To: <20260513163248.16483-1-p.pisati@gmail.com>
From: Joshua Leivenzon <hacker1024@users.sourceforge.net>
Signed-off-by: Joshua Leivenzon <hacker1024@users.sourceforge.net>
---
drivers/hid/hid-asus.c | 15 ++++++++++-----
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index 2a88ce695489d..cf9184ed2d3ec 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -1422,14 +1422,19 @@ static const __u8 *asus_report_fixup(struct hid_device *hdev, __u8 *rdesc,
/*
* Change Usage (76h) to Usage Minimum (00h), Usage Maximum
* (FFh) and clear the flags in the Input() byte.
- * Note the descriptor has a bogus 0 byte at the end so we
- * only need 1 extra byte.
*/
if (*rsize == rsize_orig &&
rdesc[offs] == 0x09 && rdesc[offs + 1] == 0x76) {
__u8 *new_rdesc;
+ unsigned int new_rsize = rsize_orig;
- new_rdesc = devm_kzalloc(&hdev->dev, rsize_orig + 1,
+ /* Clear bogus trailing zero bytes. */
+ while (rdesc[new_rsize + 1] == 0)
+ --new_rsize;
+ /* Make room for the added bytes. */
+ new_rsize += 2;
+
+ new_rdesc = devm_kzalloc(&hdev->dev, new_rsize,
GFP_KERNEL);
if (!new_rdesc)
return rdesc;
@@ -1439,8 +1444,8 @@ static const __u8 *asus_report_fixup(struct hid_device *hdev, __u8 *rdesc,
"T100CHI" : drvdata->quirks & QUIRK_T90CHI ?
"T90CHI" : "ZENBOOK DUO");
- memcpy(new_rdesc, rdesc, rsize_orig);
- *rsize = rsize_orig + 1;
+ memcpy(new_rdesc, rdesc, new_rsize);
+ *rsize = new_rsize;
rdesc = new_rdesc;
memmove(rdesc + offs + 4, rdesc + offs + 2, 12);
--
2.53.0
^ permalink raw reply related
* [PATCH 3/7] hid-asus: Add report descriptor fixup offsets for UX8406MA USB keyboard
From: Paolo Pisati @ 2026-05-13 16:32 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, linux-input
In-Reply-To: <20260513163248.16483-1-p.pisati@gmail.com>
From: Joshua Leivenzon <hacker1024@users.sourceforge.net>
Signed-off-by: Joshua Leivenzon <hacker1024@users.sourceforge.net>
---
drivers/hid/hid-asus.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index cba638b19465f..2a88ce695489d 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -1410,8 +1410,13 @@ static const __u8 *asus_report_fixup(struct hid_device *hdev, __u8 *rdesc,
rsize_orig = 306;
offs = 291;
} else if (drvdata->quirks & QUIRK_ZENBOOK_DUO_KEYBOARD) {
- rsize_orig = 257;
- offs = 176;
+ if (hid_is_usb(hdev)) {
+ rsize_orig = 90;
+ offs = 66;
+ } else /* Bluetooth */ {
+ rsize_orig = 257;
+ offs = 176;
+ }
}
/*
--
2.53.0
^ permalink raw reply related
* [PATCH 2/7] hid-asus: Add missing Zenbook Duo hotkeys
From: Paolo Pisati @ 2026-05-13 16:32 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, linux-input
In-Reply-To: <20260513163248.16483-1-p.pisati@gmail.com>
From: Joshua Leivenzon <hacker1024@users.sourceforge.net>
Signed-off-by: Joshua Leivenzon <hacker1024@users.sourceforge.net>
---
drivers/hid/hid-asus.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index ce246efba74d3..cba638b19465f 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -1046,6 +1046,7 @@ static int asus_input_mapping(struct hid_device *hdev,
case 0x6c: asus_map_key_clear(KEY_SLEEP); break;
case 0x7c: asus_map_key_clear(KEY_MICMUTE); break;
case 0x82: asus_map_key_clear(KEY_CAMERA); break;
+ case 0x86: asus_map_key_clear(KEY_CONTROLPANEL); break; /* MyASUS */
case 0x88: asus_map_key_clear(KEY_RFKILL); break;
case 0xb5: asus_map_key_clear(KEY_CALC); break;
case 0xc4: asus_map_key_clear(KEY_KBDILLUMUP); break;
@@ -1066,6 +1067,18 @@ static int asus_input_mapping(struct hid_device *hdev,
case 0xb3: asus_map_key_clear(KEY_PROG3); break; /* Fn+Left next aura */
case 0x6a: asus_map_key_clear(KEY_F13); break; /* Screenpad toggle */
case 0x4b: asus_map_key_clear(KEY_F14); break; /* Arrows/Pg-Up/Dn toggle */
+ case 0x9c: asus_map_key_clear(KEY_F19); break; /* Screen swap */
+
+ /* Mutually exclusive section: Special keys from different devices can
+ * bind to the same keycodes without concern, as they will never
+ * conflict.
+ *
+ * Note that devices with removable keyboards that connect over a
+ * standard interface (e.g. Zenbook Duo UX8406 with USB/Bluetooth) do
+ * not belong in this section, as such keyboards can be connected to
+ * another ASUS device that also uses this driver for its own keyboard.
+ */
+ /* ROG Ally */
case 0xa5: asus_map_key_clear(KEY_F15); break; /* ROG Ally left back */
case 0xa6: asus_map_key_clear(KEY_F16); break; /* ROG Ally QAM button */
case 0xa7: asus_map_key_clear(KEY_F17); break; /* ROG Ally ROG long-press */
--
2.53.0
^ permalink raw reply related
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