* Re: [PATCH] Input: xpad - ignore short Xbox One packets
From: sashiko-bot @ 2026-07-08 14:19 UTC (permalink / raw)
To: Guangshuo Li; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260708135939.774433-1-lgs201920130244@gmail.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Atomic context violation and double URB submission via xpad_start_input
--
--- Patch [1]: [PATCH] Input: xpad - ignore short Xbox One packets ---
commit d7d9f4eb601031ea325de5332a0575c0310240ad
Author: Guangshuo Li <lgs201920130244@gmail.com>
Input: xpad - ignore short Xbox One packets
This commit adds length checks for fixed-offset Xbox One packet fields and
skips optional extended fields when the corresponding bytes are not present.
This prevents parsing stale data left in the transfer buffer from previous
longer transfers when a malformed device sends a short packet.
> diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c
> index feb8f368f834e..649f59b3a162f 100644
> --- a/drivers/input/joystick/xpad.c
> +++ b/drivers/input/joystick/xpad.c
[ ... ]
> @@ -1079,6 +1090,9 @@ static void xpadone_process_packet(struct usb_xpad *xpad, u16 cmd, unsigned char
> error);
[Severity: High]
This isn't a bug introduced by this patch, but is it safe to call
xpad_start_input() from here?
Since xpadone_process_packet() is called from the URB completion handler,
it runs in atomic softirq context. However, xpad_start_input() appears to
perform sleeping operations:
xpadone_process_packet()
xpad_start_input()
usb_submit_urb(xpad->irq_in, GFP_KERNEL);
It also calls xpad_start_xbox_one(), which uses synchronous USB control
messages that can sleep:
xpad_start_input()
xpad_start_xbox_one()
usb_set_interface(xpad->udev, GIP_WIRED_INTF_AUDIO, 0);
Could this sequence trigger might_sleep() warnings or cause a deadlock under
memory pressure? Additionally, could submitting xpad->irq_in while its
completion handler is still running cause a double-submission error?
> }
> } else if (data[0] == GIP_CMD_INPUT) { /* The main valid packet type for inputs */
> + if (len < 18)
> + return;
> +
> /* menu/view buttons */
> input_report_key(dev, BTN_START, data[4] & BIT(2));
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260708135939.774433-1-lgs201920130244@gmail.com?part=1
^ permalink raw reply
* [PATCH] Input: xpad - ignore short Xbox One packets
From: Guangshuo Li @ 2026-07-08 13:59 UTC (permalink / raw)
To: Dmitry Torokhov, Sanjay Govind, Vicki Pfau, Kees Cook,
Antheas Kapenekakis, Dmitriy Zharov, Qbeliw Tanaka, Guangshuo Li,
Elliot Tester, Ted Mielczarek, linux-input, linux-kernel
xpadone_process_packet() parses Xbox One reports from fixed offsets in
the interrupt transfer buffer.
A malformed device can complete an interrupt transfer with a short
actual_length. The transfer buffer is reused across URBs, so bytes beyond
actual_length can still contain data left by a previous, longer transfer.
Without checking the received length, the parser can report stale button,
axis or paddle state as if it belonged to the new packet.
A previous fix added bounds checking for the len-relative Share button
access. Add length checks for the fixed-offset Xbox One packet fields as
well, and skip optional extended fields when the corresponding bytes are
not present.
Fixes: 1a48ff81b391 ("Input: xpad - add support for Xbox One controllers")
Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
---
drivers/input/joystick/xpad.c | 27 +++++++++++++++++++++++++--
1 file changed, 25 insertions(+), 2 deletions(-)
diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c
index feb8f368f834..649f59b3a162 100644
--- a/drivers/input/joystick/xpad.c
+++ b/drivers/input/joystick/xpad.c
@@ -1035,8 +1035,14 @@ static void xpadone_process_packet(struct usb_xpad *xpad, u16 cmd, unsigned char
struct input_dev *dev = xpad->dev;
bool do_sync = false;
+ if (!len)
+ return;
+
/* the xbox button has its own special report */
if (data[0] == GIP_CMD_VIRTUAL_KEY) {
+ if (len < 3)
+ return;
+
/*
* The Xbox One S controller requires these reports to be
* acked otherwise it continues sending them forever and
@@ -1044,6 +1050,8 @@ static void xpadone_process_packet(struct usb_xpad *xpad, u16 cmd, unsigned char
*/
if (data[1] == (GIP_OPT_ACK | GIP_OPT_INTERNAL))
xpadone_ack_mode_report(xpad, data[2]);
+ if (len < 5)
+ return;
input_report_key(dev, BTN_MODE, data[4] & GENMASK(1, 0));
input_sync(dev);
@@ -1052,6 +1060,9 @@ static void xpadone_process_packet(struct usb_xpad *xpad, u16 cmd, unsigned char
} else if (data[0] == GIP_CMD_FIRMWARE) {
/* Some packet formats force us to use this separate to poll paddle inputs */
if (xpad->packet_type == PKT_XBE2_FW_5_11) {
+ if (len < 20)
+ return;
+
/* Mute paddles if controller is in a custom profile slot
* Checked by looking at the active profile slot to
* verify it's the default slot
@@ -1079,6 +1090,9 @@ static void xpadone_process_packet(struct usb_xpad *xpad, u16 cmd, unsigned char
error);
}
} else if (data[0] == GIP_CMD_INPUT) { /* The main valid packet type for inputs */
+ if (len < 18)
+ return;
+
/* menu/view buttons */
input_report_key(dev, BTN_START, data[4] & BIT(2));
input_report_key(dev, BTN_SELECT, data[4] & BIT(3));
@@ -1145,13 +1159,16 @@ static void xpadone_process_packet(struct usb_xpad *xpad, u16 cmd, unsigned char
}
/* Profile button has a value of 0-3, so it is reported as an axis */
- if (xpad->mapping & MAP_PROFILE_BUTTON)
+ if ((xpad->mapping & MAP_PROFILE_BUTTON) && len > 34)
input_report_abs(dev, ABS_PROFILE, data[34]);
/* paddle handling */
/* based on SDL's SDL_hidapi_xboxone.c */
if (xpad->mapping & MAP_PADDLES) {
if (xpad->packet_type == PKT_XBE1) {
+ if (len < 33)
+ goto no_paddles;
+
/* Mute paddles if controller has a custom mapping applied.
* Checked by comparing the current mapping
* config against the factory mapping config
@@ -1165,6 +1182,9 @@ static void xpadone_process_packet(struct usb_xpad *xpad, u16 cmd, unsigned char
input_report_key(dev, BTN_GRIPL, data[32] & BIT(0));
input_report_key(dev, BTN_GRIPL2, data[32] & BIT(2));
} else if (xpad->packet_type == PKT_XBE2_FW_OLD) {
+ if (len < 20)
+ goto no_paddles;
+
/* Mute paddles if controller has a custom mapping applied.
* Checked by comparing the current mapping
* config against the factory mapping config
@@ -1178,6 +1198,9 @@ static void xpadone_process_packet(struct usb_xpad *xpad, u16 cmd, unsigned char
input_report_key(dev, BTN_GRIPL, data[18] & BIT(2));
input_report_key(dev, BTN_GRIPL2, data[18] & BIT(3));
} else if (xpad->packet_type == PKT_XBE2_FW_5_EARLY) {
+ if (len < 24)
+ goto no_paddles;
+
/* Mute paddles if controller has a custom mapping applied.
* Checked by comparing the current mapping
* config against the factory mapping config
@@ -1194,7 +1217,7 @@ static void xpadone_process_packet(struct usb_xpad *xpad, u16 cmd, unsigned char
input_report_key(dev, BTN_GRIPL2, data[22] & BIT(3));
}
}
-
+no_paddles:
do_sync = true;
}
--
2.43.0
^ permalink raw reply related
* Re: [PATCH v2] iio: temperature: hid-sensor-temperature: switch to non-devm iio_device_register()
From: Sanjay Chitroda @ 2026-07-08 13:50 UTC (permalink / raw)
To: jikos, jic23, srinivas.pandruvada
Cc: dlechner, nuno.sa, andy, hongyan.song, linux-input, linux-iio,
linux-kernel
In-Reply-To: <20260625121611.2003780-1-sanjayembedded@gmail.com>
On 25 June 2026 5:46:11 pm IST, Sanjay Chitroda <sanjayembeddedse@gmail.com> wrote:
>From: Sanjay Chitroda <sanjayembeddedse@gmail.com>
>
>Avoid using devm_iio_device_register(), as this driver requires explicit
>error handling and teardown ordering.
>
>With devm_iio_device_register(), IIO device remains registered until the
>devres cleanup phase. However, driver's remove() callback removes the
>sensor hub callback and trigger support. This can create a race window
>where IIO device is still visible and read_raw() requests are issued.
>These requests might call sensor_hub_input_attr_get_raw_value(), which
>waits up to 5 seconds for a response from the sensor hub callback that
>has already been removed.
>
>Add an explicit iio_device_unregister() call in the teardown path to
>ensure deterministic cleanup, so that userspace can no longer access the
>device once backend resources begin to be dismantled.
>
>Fixes: 59d0f2da3569 ("iio: hid: Add temperature sensor support")
>Cc: stable@vger.kernel.org
>Reviewed-by: Maxwell Doose <m32285159@gmail.com>
>Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
>Signed-off-by: Sanjay Chitroda <sanjayembeddedse@gmail.com>
>
Hi Jonathan,
There are two pre-existing issue highlighted by Sashiko which can be analysed and fix in separate change if required.
Requesting your valuable input on this fix. also, this is pre-requisite for HID IIO devm conversation series.
Thanks, Sanjay
>---
>Changes in v2:
>- Added review tags of Maxwell and Andy along with stable Cc
>- Based on input from Srinivas and investigation use-after-free
> explanation was not sufficiently justified so updated the commit
> message with appropriate information and kept Fixes tag
>- Link to v1: https://lore.kernel.org/all/20260622052135.1804135-1-sanjayembedded@gmail.com
>---
> drivers/iio/temperature/hid-sensor-temperature.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
>diff --git a/drivers/iio/temperature/hid-sensor-temperature.c b/drivers/iio/temperature/hid-sensor-temperature.c
>index a8d3a15f9c53..ab6ec8f659b8 100644
>--- a/drivers/iio/temperature/hid-sensor-temperature.c
>+++ b/drivers/iio/temperature/hid-sensor-temperature.c
>@@ -244,7 +244,7 @@ static int hid_temperature_probe(struct platform_device *pdev)
> if (ret)
> goto error_remove_trigger;
>
>- ret = devm_iio_device_register(indio_dev->dev.parent, indio_dev);
>+ ret = iio_device_register(indio_dev);
> if (ret)
> goto error_remove_callback;
>
>@@ -264,6 +264,7 @@ static void hid_temperature_remove(struct platform_device *pdev)
> struct iio_dev *indio_dev = platform_get_drvdata(pdev);
> struct temperature_state *temp_st = iio_priv(indio_dev);
>
>+ iio_device_unregister(indio_dev);
> sensor_hub_remove_callback(hsdev, HID_USAGE_SENSOR_TEMPERATURE);
> hid_sensor_remove_trigger(indio_dev, &temp_st->common_attributes);
> }
^ permalink raw reply
* [RFC] Input: matrix_keypad: fix interrupt regression introduced by commit 01c84b03d80a
From: Alexander Kochetkov @ 2026-07-08 13:30 UTC (permalink / raw)
To: linux-input; +Cc: Dmitry Torokhov, linux-kernel, Siarhei Volkau
Hi everyone,
I found a commit [1] that breaks the matrix_keypad behavior on the Allwinner A64.
I have a PINE A64-LTS board with a connected keypad polled by matrix_keypad.
This commit caused interrupts to stop arriving at the matrix_keypad driver entirely.
An explicit call to gpiod_direction_input() disables interrupt reception on all chips
where the GPIO input mode and GPIO interrupt input mode are configured via
the pinmux register. Interrupt reception is guaranteed to break on Allwinner
(A64, H3, H6, etc.), Broadcom (BCM2835/2711), and some Rockchip SoCs. It
does not break on NXP i.MX (i.MX6, i.MX8), STMicroelectronics (STM32MP1),
TI Sitara (AM335x), or Intel/AMD.
Furthermore, the assumption that enable_row_irqs() restores the interrupt mode is
also specific to the Ingenic pinctrl. In the vast majority of drivers, enable_row_irqs() is
supposed to simply set the interrupt enable mask without changing the pinmux.
Commit [1] was introduced to work around a hardware limitation in Ingenic's JZ4755.
This specific behavior is unique to Ingenic and a small number of specialized chips.
The majority of SoCs (around 90%) allow reading a GPIO input that is currently configured
as an interrupt source.
In my opinion, the correct approach would be to revert this commit and fix the behavior of
ingenic_gpio_get_value() inside pinctrl-ingenic.c instead. However, I do not own an Ingenic
board, so I won't be able to test such a patch.
Alternatively, I could introduce a DTS property like read-gpio-quirk in matrix_keypad to restore
the original driver behavior by default, and add this property to qi_lb60.dts so as not to break
the Ingenic platform.
What do you think? What is the best way to proceed here?
Best regards,
Alexander Kochetkov
[1] commit 01c84b03d80aab9f04c4e3e1f9085f4202ff7c29 ("Input: matrix_keypad - force switch rows to input mode")
^ permalink raw reply
* Re: [PATCH] dt-bindings: input: Convert TI Keypad Controller to DT schema
From: sashiko-bot @ 2026-07-08 12:41 UTC (permalink / raw)
To: Eduard Bostina; +Cc: dmitry.torokhov, conor+dt, linux-input, devicetree, robh
In-Reply-To: <20260708123252.1768355-1-egbostina@gmail.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Properties strictly required by the hardware and driver ('keypad,num-rows', 'keypad,num-columns', 'interrupts', and 'linux,keymap') are omitted from the 'required:' list, allowing non-functional device trees to silently pass validation.
--
--- Patch [1]: [PATCH] dt-bindings: input: Convert TI Keypad Controller to DT schema ---
commit 31e1662350e854e7b96beb7e7de83fc2b0d265e2
Author: Eduard Bostina <egbostina@gmail.com>
dt-bindings: input: Convert TI Keypad Controller to DT schema
This commit converts the Texas Instruments Keypad Controller bindings to DT
schema format. It updates the autorepeat property name, adds the reg-names
property, and omits the keypad row and column properties from the required
list to prevent validation failures.
> diff --git a/Documentation/devicetree/bindings/input/ti,omap4-keypad.yaml b/Documentation/devicetree/bindings/input/ti,omap4-keypad.yaml
> new file mode 100644
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/ti,omap4-keypad.yaml
[ ... ]
> +required:
> + - compatible
> + - reg
[Severity: Medium]
The commit message mentions that keypad,num-rows and keypad,num-columns were
omitted from this required list because enforcing them causes validation
failures. Should these properties, along with interrupts and linux,keymap,
be included here to accurately reflect the driver's probing requirements?
Omitting them allows non-functional device tree nodes to silently pass
schema validation. At runtime, the omap4-keypad driver will unconditionally
fail to probe via platform_get_irq(), matrix_keypad_parse_properties(), and
matrix_keypad_build_keymap() when these properties are missing.
Would it be better to enforce the strict hardware and driver requirements in
the schema, and instead fix the incomplete device tree nodes that are
failing validation?
> +
> +unevaluatedProperties: false
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260708123252.1768355-1-egbostina@gmail.com?part=1
^ permalink raw reply
* [PATCH] dt-bindings: input: Convert TI Keypad Controller to DT schema
From: Eduard Bostina @ 2026-07-08 12:32 UTC (permalink / raw)
To: Conor Dooley, devicetree, Dmitry Torokhov, Eduard Bostina,
Krzysztof Kozlowski, linux-input, linux-kernel, Rob Herring
Cc: daniel.baluta, simona.toaca, goledhruva, m-chawdhry
Convert the Texas Instruments Keypad Controller bindings
to DT schema.
During the conversion, the following updates were made:
- Corrected the documented property 'linux,keypad-no-autorepeat'
to 'linux,input-no-autorepeat'. The old text binding documented
the property incorrectly. The standard input subsystem property is
'linux,input-no-autorepeat', which is actively used in device
trees and parsed by the kernel.
- Added the 'reg-names' property ("mpu"), which was omitted from
the original text binding but is actively used in devide trees.
- Omitted 'keypad,num-rows' and 'keypad,num-columns' from the
required list. The original text binding stated these were
required but enforcing them causes validation failures.
Signed-off-by: Eduard Bostina <egbostina@gmail.com>
---
.../devicetree/bindings/input/omap-keypad.txt | 28 ---------
.../bindings/input/ti,omap4-keypad.yaml | 57 +++++++++++++++++++
2 files changed, 57 insertions(+), 28 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/input/omap-keypad.txt
create mode 100644 Documentation/devicetree/bindings/input/ti,omap4-keypad.yaml
diff --git a/Documentation/devicetree/bindings/input/omap-keypad.txt b/Documentation/devicetree/bindings/input/omap-keypad.txt
deleted file mode 100644
index 34ed1c60ff95..000000000000
--- a/Documentation/devicetree/bindings/input/omap-keypad.txt
+++ /dev/null
@@ -1,28 +0,0 @@
-* TI's Keypad Controller device tree bindings
-
-TI's Keypad controller is used to interface a SoC with a matrix-type
-keypad device. The keypad controller supports multiple row and column lines.
-A key can be placed at each intersection of a unique row and a unique column.
-The keypad controller can sense a key-press and key-release and report the
-event using a interrupt to the cpu.
-
-This binding is based on the matrix-keymap binding with the following
-changes:
-
-keypad,num-rows and keypad,num-columns are required.
-
-Required SoC Specific Properties:
-- compatible: should be one of the following
- - "ti,omap4-keypad": For controllers compatible with omap4 keypad
- controller.
-
-Optional Properties specific to linux:
-- linux,keypad-no-autorepeat: do no enable autorepeat feature.
-
-Example:
- keypad@4ae1c000{
- compatible = "ti,omap4-keypad";
- keypad,num-rows = <2>;
- keypad,num-columns = <8>;
- linux,keypad-no-autorepeat;
- };
diff --git a/Documentation/devicetree/bindings/input/ti,omap4-keypad.yaml b/Documentation/devicetree/bindings/input/ti,omap4-keypad.yaml
new file mode 100644
index 000000000000..5647b4a24228
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/ti,omap4-keypad.yaml
@@ -0,0 +1,57 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/input/ti,omap4-keypad.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Texas Instruments Keypad Controller
+
+maintainers:
+ - Eduard Bostina <egbostina@gmail.com>
+
+description: |
+ TI's Keypad controller is used to interface a SoC with a matrix-type
+ keypad device. The keypad controller supports multiple row and column lines.
+ A key can be placed at each intersection of a unique row and a unique column.
+ The keypad controller can sense a key-press and key-release and report the
+ event using a interrupt to the cpu.
+
+allOf:
+ - $ref: /schemas/input/matrix-keymap.yaml#
+
+properties:
+ compatible:
+ const: ti,omap4-keypad
+
+ reg:
+ maxItems: 1
+
+ reg-names:
+ const: mpu
+
+ interrupts:
+ maxItems: 1
+
+ linux,input-no-autorepeat:
+ type: boolean
+ description: Do not enable autorepeat feature.
+
+required:
+ - compatible
+ - reg
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+
+ keypad@4ae1c000 {
+ compatible = "ti,omap4-keypad";
+ reg = <0x4ae1c000 0x400>;
+ reg-names = "mpu";
+ interrupts = <GIC_SPI 120 IRQ_TYPE_LEVEL_HIGH>;
+ keypad,num-rows = <2>;
+ keypad,num-columns = <8>;
+ linux,input-no-autorepeat;
+ };
--
2.43.0
^ permalink raw reply related
* Re: [PATCH] hwmon: prevent packets from going to driver for probe
From: Guenter Roeck @ 2026-07-08 0:50 UTC (permalink / raw)
To: Edward Adam Davis
Cc: syzbot+9eebf5f6544c5e873858, bentiss, jikos, linux-input,
linux-kernel, linux-usb, syzkaller-bugs
In-Reply-To: <tencent_BB7C33EB9EA41B7B4B5F1B8B25C0BA13BB08@qq.com>
On Tue, Apr 28, 2026 at 12:12:26PM +0800, Edward Adam Davis wrote:
> A race condition exists between hid_input_report() and the point
> immediately following the execution of hid_device_io_start() within
> corsairpsu_probe(). If the probe operation fails after "io start" has
> been initiated, this race condition will result in a uaf vulnerability
> [1].
>
> CPU0 CPU1
> ==== ====
> corsairpsu_probe()
> hid_device_io_start()
> ... unlock driver_input_lock
> hid_hw_stop()
> kfree(hidraw) __hid_input_report()
> ... acquire driver_input_lock
> hid_report_raw_event()
> hidraw_report_event()
> ... access hidraw's list_lock // trigger uaf
>
> Consequently, when corsairpsu_probe() fails and hid_hw_stop() needs to
> be executed, the io_started flag is first cleared while holding the
> driver_input_lock to prevent potential race conditions involving input
> reports.
>
> [1]
> BUG: KASAN: slab-use-after-free in rt_spin_lock+0x83/0x400 kernel/locking/spinlock_rt.c:56
> Call Trace:
> hidraw_report_event+0x5d/0x3a0 drivers/hid/hidraw.c:577
> hid_report_raw_event+0x311/0x1730 drivers/hid/hid-core.c:2076
> __hid_input_report drivers/hid/hid-core.c:2152 [inline]
> hid_input_report+0x44e/0x580 drivers/hid/hid-core.c:2174
> hid_irq_in+0x47e/0x6d0 drivers/hid/usbhid/hid-core.c:286
> __usb_hcd_giveback_urb+0x3b3/0x5e0 drivers/usb/core/hcd.c:1657
> dummy_timer+0x8a9/0x47d0 drivers/usb/gadget/udc/dummy_hcd.c:2005
>
> Allocated by task 10:
> hidraw_connect+0x57/0x430 drivers/hid/hidraw.c:606
> hid_connect+0x5bf/0x19d0 drivers/hid/hid-core.c:2277
> hid_hw_start+0xa8/0x120 drivers/hid/hid-core.c:2387
> corsairpsu_probe+0xd9/0x3c0 drivers/hwmon/corsair-psu.c:782
>
> Freed by task 10:
> hidraw_disconnect+0x4f/0x60 drivers/hid/hidraw.c:662
> hid_disconnect drivers/hid/hid-core.c:2362 [inline]
> hid_hw_stop+0x101/0x1e0 drivers/hid/hid-core.c:2407
> corsairpsu_probe+0x327/0x3c0 drivers/hwmon/corsair-psu.c:826
>
> Fixes: d115b51e0e56 ("hwmon: add Corsair PSU HID controller driver")
> Reported-by: syzbot+9eebf5f6544c5e873858@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=9eebf5f6544c5e873858
> Tested-by: syzbot+9eebf5f6544c5e873858@syzkaller.appspotmail.com
> Signed-off-by: Edward Adam Davis <eadavis@qq.com>
Applied.
Note that it would help to copy subsystem mailing lists and maintainers
on patches like this.
Thanks,
Guenter
^ permalink raw reply
* [PATCH v2] HID: i2c-hid: Add a quirk for a Cirque I2C device.
From: Vadim Klishko @ 2026-07-08 0:31 UTC (permalink / raw)
To: jikos
Cc: bentiss, dianders, treapking, raoxu, superm1, kenkinming2002,
linux-input, linux-kernel, AJ_Cheng, kevinchou, andy.hung,
andy.hung, CindyXT_Wang, frank.chen, Vadim Klishko
In-Reply-To: <20260703144726.14808-1-vadim@cirque.com>
Cirque touchpads with PID D0C1 generate an error when probed
by the I2C HID driver, resulting in no hidraw device created.
Adding I2C_HID_QUIRK_NO_IRQ_AFTER_RESET fixes the issue.
Signed-off-by: Vadim Klishko <vadim@cirque.com>
---
Changes in v2:
- No code changes.
- Added subsystem to the subject line.
- Revised the explanation.
- Link to v1: https://lore.kernel.org/all/20260703144726.14808-1-vadim@cirque.com/
drivers/hid/hid-ids.h | 1 +
drivers/hid/i2c-hid/i2c-hid-core.c | 2 ++
2 files changed, 3 insertions(+)
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 1059922baaac..496589277bd5 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -334,6 +334,7 @@
#define I2C_VENDOR_ID_CIRQUE 0x0488
#define I2C_PRODUCT_ID_CIRQUE_1063 0x1063
+#define I2C_PRODUCT_ID_CIRQUE_D0C1 0xD0C1
#define USB_VENDOR_ID_CJTOUCH 0x24b8
#define USB_DEVICE_ID_CJTOUCH_MULTI_TOUCH_0020 0x0020
diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c b/drivers/hid/i2c-hid/i2c-hid-core.c
index 3adb16366e93..63004bcbfa71 100644
--- a/drivers/hid/i2c-hid/i2c-hid-core.c
+++ b/drivers/hid/i2c-hid/i2c-hid-core.c
@@ -136,6 +136,8 @@ static const struct i2c_hid_quirks {
I2C_HID_QUIRK_BAD_INPUT_SIZE },
{ I2C_VENDOR_ID_CIRQUE, I2C_PRODUCT_ID_CIRQUE_1063,
I2C_HID_QUIRK_NO_SLEEP_ON_SUSPEND },
+ { I2C_VENDOR_ID_CIRQUE, I2C_PRODUCT_ID_CIRQUE_D0C1,
+ I2C_HID_QUIRK_NO_IRQ_AFTER_RESET },
/*
* Without additional power on command, at least some QTEC devices send garbage
*/
--
2.34.1
^ permalink raw reply related
* Re: [PATCH v11 2/3] HID: nintendo: Add rumble support for Switch 2 controllers
From: Vicki Pfau @ 2026-07-08 0:01 UTC (permalink / raw)
To: Silvan Jegen
Cc: Dmitry Torokhov, Jiri Kosina, Benjamin Tissoires, linux-input
In-Reply-To: <39MSC7NQF6IO8.2E4BHGFFF82H4@homearch.localdomain>
Hi Silvan,
On 7/5/26 7:13 AM, Silvan Jegen wrote:
> Heyhey!
>
> Just one comment below.
>
> Vicki Pfau <vi@endrift.com> wrote:
>> This adds rumble support for both the "HD Rumble" linear resonant actuator
>> type as used in the Joy-Cons and Pro Controller, as well as the eccentric
>> rotating mass type used in the GameCube controller. Note that since there's
>> currently no API for exposing full control of LRAs with evdev, it only
>> simulates a basic rumble for now.
>>
>> Signed-off-by: Vicki Pfau <vi@endrift.com>
>> ---
>> drivers/hid/Kconfig | 8 +-
>> drivers/hid/hid-nintendo.c | 211 ++++++++++++++++++++++++++++++++++++-
>> 2 files changed, 213 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
>> index 19c77c323ec9..851eed76c236 100644
>> --- a/drivers/hid/Kconfig
>> +++ b/drivers/hid/Kconfig
>> @@ -859,10 +859,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.
>>
>> config HID_NTI
>> tristate "NTI keyboard adapters"
>> diff --git a/drivers/hid/hid-nintendo.c b/drivers/hid/hid-nintendo.c
>> index e21c36921832..a36f4fd9a1da 100644
>> --- a/drivers/hid/hid-nintendo.c
>> +++ b/drivers/hid/hid-nintendo.c
>> @@ -37,6 +37,7 @@
>> #include <linux/unaligned.h>
>> #include <linux/delay.h>
>> #include <linux/device.h>
>> +#include <linux/devm-helpers.h>
>> #include <linux/kernel.h>
>> #include <linux/hid.h>
>> #include <linux/idr.h>
>> @@ -2989,6 +2990,7 @@ enum switch2_init_step {
>> NS2_INIT_READ_USER_SECONDARY_CALIB,
>> NS2_INIT_SET_FEATURE_MASK,
>> NS2_INIT_ENABLE_FEATURES,
>> + NS2_INIT_ENABLE_RUMBLE,
>> NS2_INIT_GRIP_BUTTONS,
>> NS2_INIT_REPORT_FORMAT,
>> NS2_INIT_INPUT,
>> @@ -3020,6 +3022,18 @@ struct switch2_stick_calibration {
>> struct switch2_axis_calibration y;
>> };
>>
>> +struct switch2_hd_rumble {
>> + uint16_t hi_freq : 10;
>> + uint16_t hi_amp : 10;
>> + uint16_t lo_freq : 10;
>> + uint16_t lo_amp : 10;
>> +} __packed;
>> +
>> +struct switch2_erm_rumble {
>> + uint16_t error;
>> + uint16_t amplitude;
>> +};
>> +
>> struct switch2_controller {
>> struct hid_device *hdev;
>> struct switch2_cfg_intf *cfg;
>> @@ -3043,8 +3057,45 @@ struct switch2_controller {
>>
>> uint32_t player_id;
>> struct led_classdev *leds;
>> +
>> +#if IS_ENABLED(CONFIG_NINTENDO_FF)
>> + spinlock_t rumble_lock;
>> + uint8_t rumble_seq;
>> + union {
>> + struct switch2_hd_rumble hd;
>> + struct switch2_erm_rumble sd;
>> + } rumble;
>> + uint64_t last_rumble_work;
>> + struct delayed_work rumble_work;
>> + uint8_t *rumble_buffer;
>> +#endif
>> };
>>
>> +enum gc_rumble {
>> + GC_RUMBLE_OFF = 0,
>> + GC_RUMBLE_ON = 1,
>> + GC_RUMBLE_STOP = 2,
>> +};
>> +
>> +/*
>> + * The highest rumble level for "HD Rumble" is strong enough to potentially damage the controller,
>> + * and also leaves your hands feeling like melted jelly, so we set a semi-arbitrary scaling factor
>> + * to artificially limit the maximum for safety and comfort. It is currently unknown if the Switch
>> + * 2 itself does something similar, but it's quite likely.
>> + *
>> + * This value must be between 0 and 1024, otherwise the math below will overflow.
>> + */
>> +#define RUMBLE_MAX 450u
>> +
>> +/*
>> + * Semi-arbitrary values used to simulate the "rumble" sensation of an eccentric rotating
>> + * mass type haptic motor on the Switch 2 controllers' linear resonant actuator type haptics.
>> + *
>> + * The units used are unknown, but the values must be between 0 and 1023.
>> + */
>> +#define RUMBLE_HI_FREQ 0x187
>> +#define RUMBLE_LO_FREQ 0x112
>> +
>> static DEFINE_MUTEX(switch2_controllers_lock);
>> static LIST_HEAD(switch2_controllers);
>>
>> @@ -3136,7 +3187,7 @@ static const uint8_t switch2_init_cmd_data[] = {
>> static const uint8_t switch2_one_data[] = { 0x01, 0x00, 0x00, 0x00 };
>>
>> static const uint8_t switch2_feature_mask[] = {
>> - NS2_FEATURE_BUTTONS | NS2_FEATURE_ANALOG | NS2_FEATURE_IMU,
>> + NS2_FEATURE_BUTTONS | NS2_FEATURE_ANALOG | NS2_FEATURE_IMU | NS2_FEATURE_RUMBLE,
>> 0x00, 0x00, 0x00
>> };
>>
>> @@ -3209,6 +3260,125 @@ static void switch2_kref_put(struct kref *refcount)
>> kfree(ns2);
>> }
>>
>> +#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);
>
> We might want to check ns2 for NULL here (like we do in
> switch2_player_led_brightness_set).
Good call.
>
> Cheers,
> Silvan
>
>> + unsigned long flags;
>> +
>> + if (effect->type != FF_RUMBLE)
>> + return 0;
>> +
>> + spin_lock_irqsave(&ns2->rumble_lock, flags);
>> + if (ns2->ctlr_type == NS2_CTLR_TYPE_GC) {
>> + ns2->rumble.sd.amplitude = max(effect->u.rumble.strong_magnitude,
>> + (uint16_t) (effect->u.rumble.weak_magnitude >> 1));
>> + } else {
>> + ns2->rumble.hd.hi_freq = RUMBLE_HI_FREQ;
>> + ns2->rumble.hd.lo_freq = RUMBLE_LO_FREQ;
>> + 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;
>> + }
>> + spin_unlock_irqrestore(&ns2->rumble_lock, flags);
>> +
>> + 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 flags;
>> + bool active;
>> + int ret = 0;
>> +
>> + spin_lock_irqsave(&ns2->rumble_lock, flags);
>> + ns2->rumble_buffer[0x1] = 0x50 | ns2->rumble_seq;
>> + if (ns2->ctlr_type == NS2_CTLR_TYPE_GC) {
>> + ns2->rumble_buffer[0] = 3;
>> + if (ns2->rumble.sd.amplitude == 0) {
>> + ns2->rumble_buffer[2] = GC_RUMBLE_STOP;
>> + ns2->rumble.sd.error = 0;
>> + active = false;
>> + } else {
>> + if (ns2->rumble.sd.error < ns2->rumble.sd.amplitude) {
>> + ns2->rumble_buffer[2] = GC_RUMBLE_ON;
>> + ns2->rumble.sd.error += U16_MAX - ns2->rumble.sd.amplitude;
>> + } else {
>> + ns2->rumble_buffer[2] = GC_RUMBLE_OFF;
>> + ns2->rumble.sd.error -= ns2->rumble.sd.amplitude;
>> + }
>> + active = true;
>> + }
>> + } else {
>> + ns2->rumble_buffer[0] = 1;
>> + switch2_encode_rumble(&ns2->rumble.hd, &ns2->rumble_buffer[0x2]);
>> + active = ns2->rumble.hd.hi_amp || ns2->rumble.hd.lo_amp;
>> + if (ns2->ctlr_type == NS2_CTLR_TYPE_PRO) {
>> + /*
>> + * The Pro Controller contains separate LRAs on each
>> + * side that can be controlled individually.
>> + */
>> + ns2->rumble_buffer[0] = 2;
>> + ns2->rumble_buffer[0x11] = 0x50 | ns2->rumble_seq;
>> + switch2_encode_rumble(&ns2->rumble.hd, &ns2->rumble_buffer[0x12]);
>> + }
>> + }
>> + ns2->rumble_seq = (ns2->rumble_seq + 1) & 0xF;
>> + spin_unlock_irqrestore(&ns2->rumble_lock, flags);
>> +
>> + if (active) {
>> + unsigned long interval = msecs_to_jiffies(4);
>> + uint64_t current_jiffies = get_jiffies_64();
>> +
>> + if (!ns2->last_rumble_work)
>> + ns2->last_rumble_work = current_jiffies;
>> + else
>> + ns2->last_rumble_work += interval;
>> +
>> + /* Reschedule a little early to make sure the buffer never underruns */
>> + interval -= msecs_to_jiffies(2);
>> + if (ns2->last_rumble_work + interval >= current_jiffies)
>> + schedule_delayed_work(&ns2->rumble_work,
>> + ns2->last_rumble_work + interval - current_jiffies);
>> + else
>> + schedule_delayed_work(&ns2->rumble_work, 0);
>> + } else {
>> + ns2->last_rumble_work = 0;
>> + }
>> +
>> + mutex_lock(&ns2->lock);
>> + if (!ns2->hdev) {
>> + cancel_delayed_work(&ns2->rumble_work);
>> + } else {
>> + ret = hid_hw_output_report(ns2->hdev, ns2->rumble_buffer, 64);
>> + /*
>> + * Don't log on ENODEV, ESHUTDOWN, or EPROTO, which can happen
>> + * mid-hotplug. Also cancel any further work on ENODEV or
>> + * ESHUTDOWN as they're clear indications that the endpoint
>> + * is dead.
>> + */
>> + if (ret == -ENODEV || ret == -ESHUTDOWN)
>> + cancel_delayed_work(&ns2->rumble_work);
>> + else if (ret < 0 && ret != -EPROTO)
>> + hid_warn_ratelimited(ns2->hdev,
>> + "Failed to send output report ret=%d\n", ret);
>> + }
>> + mutex_unlock(&ns2->lock);
>> +}
>> +#endif
>> +
>> static int switch2_set_leds(struct switch2_controller *ns2)
>> {
>> int i;
>> @@ -3332,6 +3502,26 @@ static int switch2_init_input(struct switch2_controller *ns2)
>> return -EINVAL;
>> }
>>
>> +#if IS_ENABLED(CONFIG_NINTENDO_FF)
>> + ns2->rumble_buffer = devm_kzalloc(&input->dev, 64, GFP_KERNEL);
>> + if (!ns2->rumble_buffer) {
>> + input_free_device(input);
>> + return -ENOMEM;
>> + }
>> + ret = devm_delayed_work_autocancel(&input->dev, &ns2->rumble_work, switch2_rumble_work);
>> + if (ret < 0) {
>> + input_free_device(input);
>> + return ret;
>> + }
>> +
>> + input_set_capability(input, EV_FF, FF_RUMBLE);
>> + ret = input_ff_create_memless(input, NULL, switch2_play_effect);
>> + if (ret) {
>> + input_free_device(input);
>> + return ret;
>> + }
>> +#endif
>> +
>> hid_info(ns2->hdev, "Firmware version %u.%u.%u (type %i)\n", ns2->version.major,
>> ns2->version.minor, ns2->version.patch, ns2->version.ctlr_type);
>> if (ns2->version.dsp_type >= 0)
>> @@ -3765,7 +3955,16 @@ int switch2_init_controller(struct switch2_controller *ns2)
>> return ns2->cfg->send_command(NS2_CMD_FEATSEL, NS2_SUBCMD_FEATSEL_SET_MASK,
>> switch2_feature_mask, sizeof(switch2_feature_mask), ns2->cfg);
>> case NS2_INIT_ENABLE_FEATURES:
>> - return switch2_features_enable(ns2, NS2_FEATURE_BUTTONS | NS2_FEATURE_ANALOG);
>> + return switch2_features_enable(ns2, NS2_FEATURE_BUTTONS |
>> + NS2_FEATURE_ANALOG | NS2_FEATURE_RUMBLE);
>> + case NS2_INIT_ENABLE_RUMBLE:
>> + /*
>> + * It is unclear what this packet is supposed to be for, but it
>> + * appears to be needed for rumble to work reliably. The reply
>> + * data indicates it might be a query of some sort, but we
>> + * ignore the reply so long as it doesn't return an error.
>> + */
>> + return ns2->cfg->send_command(0x11, 1, NULL, 0, ns2->cfg);
>> case NS2_INIT_GRIP_BUTTONS:
>> if (!switch2_ctlr_is_joycon(ns2->ctlr_type)) {
>> switch2_init_step_done(ns2, ns2->init_step);
>> @@ -3878,6 +4077,10 @@ int switch2_receive_command(struct switch2_controller *ns2,
>> switch2_init_step_done(ns2, NS2_INIT_GET_FIRMWARE_INFO);
>> }
>> break;
>> + case 0x11:
>> + if (header->subcommand == 1)
>> + switch2_init_step_done(ns2, NS2_INIT_ENABLE_RUMBLE);
>> + break;
>> default:
>> break;
>> }
>> @@ -3997,6 +4200,10 @@ static int switch2_probe(struct hid_device *hdev, const struct hid_device_id *id
>> else
>> ns2->player_id = ret;
>>
>> +#if IS_ENABLED(CONFIG_NINTENDO_FF)
>> + spin_lock_init(&ns2->rumble_lock);
>> +#endif
>> +
>> ret = hid_hw_start(hdev, HID_CONNECT_HIDRAW);
>> if (ret) {
>> hid_err(hdev, "hw_start failed %d\n", ret);
>
>
Vicki
^ permalink raw reply
* Re: [PATCH v11 1/3] HID: nintendo: Add preliminary Switch 2 controller driver
From: Vicki Pfau @ 2026-07-08 0:00 UTC (permalink / raw)
To: Silvan Jegen
Cc: Dmitry Torokhov, Jiri Kosina, Benjamin Tissoires, linux-input
In-Reply-To: <2RB9AEPZHD3VD.3OBFQALQF980M@homearch.localdomain>
Hi Silvan,
On 7/4/26 12:36 PM, Silvan Jegen wrote:
> Heyhey!
>
> Some comments and questions below.
>
> Vicki Pfau <vi@endrift.com> wrote:
>> 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. This is the case on both USB and Bluetooth, so this new driver
>> uses a split-driver design with the HID interface being the "main" driver
>> and the configuration interface is a secondary driver that looks up to the
>> HID interface, sharing resources on a common struct.
>>
>> Due to using a non-standard pairing interface as well as Bluetooth
>> communications being extremely limited in the kernel, a custom interface
>> between userspace and the kernel will need to be designed, along with
>> bringup in BlueZ. That is beyond the scope of this initial patch, which
>> only contains the generic HID and USB configuration interface drivers.
>>
>> This initial work supports general input for the Joy-Con 2, Pro Controller
>> 2, and GameCube NSO controllers. IMU, rumble and battery support is not yet
>> present.
>>
>> Signed-off-by: Vicki Pfau <vi@endrift.com>
>> ---
>> MAINTAINERS | 1 +
>> drivers/hid/Kconfig | 11 +-
>> drivers/hid/hid-ids.h | 4 +
>> drivers/hid/hid-nintendo.c | 1278 ++++++++++++++++-
>> drivers/hid/hid-nintendo.h | 72 +
>> drivers/input/joystick/Kconfig | 11 +
>> drivers/input/joystick/Makefile | 1 +
>> drivers/input/joystick/nintendo-switch2-usb.c | 468 ++++++
>> 8 files changed, 1836 insertions(+), 10 deletions(-)
>> create mode 100644 drivers/hid/hid-nintendo.h
>> create mode 100644 drivers/input/joystick/nintendo-switch2-usb.c
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 4ecd282f8f52..778982ab298e 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -19051,6 +19051,7 @@ F: drivers/scsi/nsp32*
>>
>> NINTENDO HID DRIVER
>> M: Daniel J. Ogorchock <djogorchock@gmail.com>
>> +M: Vicki Pfau <vi@endrift.com>
>> L: linux-input@vger.kernel.org
>> S: Maintained
>> F: drivers/hid/hid-nintendo*
>> diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
>> index f9bcaeb66385..19c77c323ec9 100644
>> --- a/drivers/hid/Kconfig
>> +++ b/drivers/hid/Kconfig
>> @@ -843,10 +843,13 @@ config HID_NINTENDO
>> depends on LEDS_CLASS
>> select POWER_SUPPLY
>> help
>> - Adds support for the Nintendo Switch Joy-Cons, NSO, Pro Controller.
>> - All controllers support bluetooth, and the Pro Controller also supports
>> - its USB mode. This also includes support for the Nintendo Switch Online
>> - Controllers which include the NES, Genesis, SNES, and N64 controllers.
>> + Adds support for the Nintendo Switch Joy-Cons, NSO, Pro Controller, as
>> + well as Nintendo Switch 2 Joy-Cons, Pro Controller, and NSO GameCube
>> + controllers. All Switch controllers support bluetooth, and the Pro
>> + Controller also supports its USB mode. This also includes support for
>> + the Nintendo Switch Online Controllers which include the NES, Genesis,
>> + SNES, and N64 controllers. Switch 2 controllers currently only support
>> + USB mode.
>>
>> To compile this driver as a module, choose M here: the
>> module will be called hid-nintendo.
>> diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
>> index 1059922baaac..9ba62b8fb894 100644
>> --- a/drivers/hid/hid-ids.h
>> +++ b/drivers/hid/hid-ids.h
>> @@ -1103,6 +1103,10 @@
>> #define USB_DEVICE_ID_NINTENDO_SNESCON 0x2017
>> #define USB_DEVICE_ID_NINTENDO_GENCON 0x201e
>> #define USB_DEVICE_ID_NINTENDO_N64CON 0x2019
>> +#define USB_DEVICE_ID_NINTENDO_NS2_JOYCONR 0x2066
>> +#define USB_DEVICE_ID_NINTENDO_NS2_JOYCONL 0x2067
>> +#define USB_DEVICE_ID_NINTENDO_NS2_PROCON 0x2069
>> +#define USB_DEVICE_ID_NINTENDO_NS2_GCCON 0x2073
>>
>> #define USB_VENDOR_ID_NOVATEK 0x0603
>> #define USB_DEVICE_ID_NOVATEK_PCT 0x0600
>> diff --git a/drivers/hid/hid-nintendo.c b/drivers/hid/hid-nintendo.c
>> index e7302ec01ff1..e21c36921832 100644
>> --- a/drivers/hid/hid-nintendo.c
>> +++ b/drivers/hid/hid-nintendo.c
>> @@ -1,11 +1,13 @@
>> // SPDX-License-Identifier: GPL-2.0+
>> /*
>> - * HID driver for Nintendo Switch Joy-Cons and Pro Controllers
>> + * HID driver for Nintendo Switch Joy-Cons and Pro Controllers, as well as
>> + * Nintendo Switch 2 Joy-Cons, Pro Controller, and GameCube Controller
>> *
>> * Copyright (c) 2019-2021 Daniel J. Ogorchock <djogorchock@gmail.com>
>> * Portions Copyright (c) 2020 Nadia Holmquist Pedersen <nadia@nhp.sh>
>> * Copyright (c) 2022 Emily Strickland <linux@emily.st>
>> * Copyright (c) 2023 Ryan McClelland <rymcclel@gmail.com>
>> + * Copyright (c) 2026 Valve Software
>> *
>> * The following resources/projects were referenced for this driver:
>> * https://github.com/dekuNukem/Nintendo_Switch_Reverse_Engineering
>> @@ -13,6 +15,8 @@
>> * https://github.com/FrotBot/SwitchProConLinuxUSB
>> * https://github.com/MTCKC/ProconXInput
>> * https://github.com/Davidobot/BetterJoyForCemu
>> + * https://gist.github.com/shinyquagsire23/66f006b46c56216acbaac6c1e2279b64
>> + * https://github.com/ndeadly/switch2_controller_research
>> * hid-wiimote kernel hid driver
>> * hid-logitech-hidpp driver
>> * hid-sony driver
>> @@ -29,6 +33,7 @@
>> */
>>
>> #include "hid-ids.h"
>> +#include "hid-nintendo.h"
>> #include <linux/unaligned.h>
>> #include <linux/delay.h>
>> #include <linux/device.h>
>> @@ -41,6 +46,8 @@
>> #include <linux/module.h>
>> #include <linux/power_supply.h>
>> #include <linux/spinlock.h>
>> +#include <linux/usb.h>
>> +#include "usbhid/usbhid.h"
>>
>> /*
>> * Reference the url below for the following HID report defines:
>> @@ -2662,7 +2669,7 @@ static int joycon_ctlr_handle_event(struct joycon_ctlr *ctlr, u8 *data,
>> return ret;
>> }
>>
>> -static int nintendo_hid_event(struct hid_device *hdev,
>> +static int joycon_event(struct hid_device *hdev,
>> struct hid_report *report, u8 *raw_data, int size)
>> {
>> struct joycon_ctlr *ctlr = hid_get_drvdata(hdev);
>> @@ -2673,7 +2680,7 @@ static int nintendo_hid_event(struct hid_device *hdev,
>> return joycon_ctlr_handle_event(ctlr, raw_data, size);
>> }
>>
>> -static int nintendo_hid_probe(struct hid_device *hdev,
>> +static int joycon_probe(struct hid_device *hdev,
>> const struct hid_device_id *id)
>> {
>> int ret;
>> @@ -2777,7 +2784,7 @@ static int nintendo_hid_probe(struct hid_device *hdev,
>> return ret;
>> }
>>
>> -static void nintendo_hid_remove(struct hid_device *hdev)
>> +static void joycon_remove(struct hid_device *hdev)
>> {
>> struct joycon_ctlr *ctlr = hid_get_drvdata(hdev);
>> unsigned long flags;
>> @@ -2796,7 +2803,7 @@ static void nintendo_hid_remove(struct hid_device *hdev)
>> hid_hw_stop(hdev);
>> }
>>
>> -static int nintendo_hid_resume(struct hid_device *hdev)
>> +static int joycon_resume(struct hid_device *hdev)
>> {
>> struct joycon_ctlr *ctlr = hid_get_drvdata(hdev);
>> int ret;
>> @@ -2819,7 +2826,7 @@ static int nintendo_hid_resume(struct hid_device *hdev)
>> return ret;
>> }
>>
>> -static int nintendo_hid_suspend(struct hid_device *hdev, pm_message_t message)
>> +static int joycon_suspend(struct hid_device *hdev, pm_message_t message)
>> {
>> struct joycon_ctlr *ctlr = hid_get_drvdata(hdev);
>>
>> @@ -2838,7 +2845,1208 @@ static int nintendo_hid_suspend(struct hid_device *hdev, pm_message_t message)
>> return 0;
>> }
>>
>> +/*
>> + * =============================================================================
>> + * Switch 2 support
>> + * =============================================================================
>> + */
>> +#define NS2_BTNR_B BIT(0)
>> +#define NS2_BTNR_A BIT(1)
>> +#define NS2_BTNR_Y BIT(2)
>> +#define NS2_BTNR_X BIT(3)
>> +#define NS2_BTNR_R BIT(4)
>> +#define NS2_BTNR_ZR BIT(5)
>> +#define NS2_BTNR_PLUS BIT(6)
>> +#define NS2_BTNR_RS BIT(7)
>> +
>> +#define NS2_BTNL_DOWN BIT(0)
>> +#define NS2_BTNL_RIGHT BIT(1)
>> +#define NS2_BTNL_LEFT BIT(2)
>> +#define NS2_BTNL_UP BIT(3)
>> +#define NS2_BTNL_L BIT(4)
>> +#define NS2_BTNL_ZL BIT(5)
>> +#define NS2_BTNL_MINUS BIT(6)
>> +#define NS2_BTNL_LS BIT(7)
>> +
>> +#define NS2_BTN3_C BIT(4)
>> +#define NS2_BTN3_SR BIT(6)
>> +#define NS2_BTN3_SL BIT(7)
>> +
>> +#define NS2_BTN_JCR_HOME BIT(0)
>> +#define NS2_BTN_JCR_GR BIT(2)
>> +#define NS2_BTN_JCR_C NS2_BTN3_C
>> +#define NS2_BTN_JCR_SR NS2_BTN3_SR
>> +#define NS2_BTN_JCR_SL NS2_BTN3_SL
>> +
>> +#define NS2_BTN_JCL_CAPTURE BIT(0)
>> +#define NS2_BTN_JCL_GL BIT(2)
>> +#define NS2_BTN_JCL_SR NS2_BTN3_SR
>> +#define NS2_BTN_JCL_SL NS2_BTN3_SL
>> +
>> +#define NS2_BTN_PRO_HOME BIT(0)
>> +#define NS2_BTN_PRO_CAPTURE BIT(1)
>> +#define NS2_BTN_PRO_GR BIT(2)
>> +#define NS2_BTN_PRO_GL BIT(3)
>> +#define NS2_BTN_PRO_C NS2_BTN3_C
>> +
>> +#define NS2_BTN_GC_HOME BIT(0)
>> +#define NS2_BTN_GC_CAPTURE BIT(1)
>> +#define NS2_BTN_GC_C NS2_BTN3_C
>> +
>> +#define NS2_TRIGGER_RANGE 4095
>> +#define NS2_AXIS_MIN -32768
>> +#define NS2_AXIS_MAX 32767
>> +
>> +#define NS2_MAX_PLAYER_ID 8
>> +
>> +#define NS2_MAX_INIT_RETRIES 4
>> +
>> +#define NS2_FLASH_ADDR_SERIAL 0x13002
>> +#define NS2_FLASH_ADDR_FACTORY_PRIMARY_CALIB 0x130a8
>> +#define NS2_FLASH_ADDR_FACTORY_SECONDARY_CALIB 0x130e8
>> +#define NS2_FLASH_ADDR_FACTORY_TRIGGER_CALIB 0x13140
>> +#define NS2_FLASH_ADDR_USER_PRIMARY_CALIB 0x1fc040
>> +#define NS2_FLASH_ADDR_USER_SECONDARY_CALIB 0x1fc080
>> +
>> +#define NS2_FLASH_SIZE_SERIAL 0x10
>> +#define NS2_FLASH_SIZE_FACTORY_AXIS_CALIB 9
>> +#define NS2_FLASH_SIZE_FACTORY_TRIGGER_CALIB 2
>> +#define NS2_FLASH_SIZE_USER_AXIS_CALIB 11
>> +
>> +#define NS2_USER_CALIB_MAGIC 0xa1b2
>> +
>> +#define NS2_FEATURE_BUTTONS BIT(0)
>> +#define NS2_FEATURE_ANALOG BIT(1)
>> +#define NS2_FEATURE_IMU BIT(2)
>
> Two tabs instead of one?
>
Yes, Linux uses a tabstop of 8 and this is how it lines up with that ts.
>
>> +#define NS2_FEATURE_MOUSE BIT(4)
>
> This one doesn't seem to be used (yet). Not sure if that is intended
> or not.
I don't see that as a problem. It's code as documentation, even if it's not used yet.
>
>> +#define NS2_FEATURE_RUMBLE BIT(5)
>> +#define NS2_FEATURE_MAGNETO BIT(7)
>
> Ditto.
>
>> +
>> +enum switch2_subcmd_flash {
>> + NS2_SUBCMD_FLASH_READ_BLOCK = 0x01,
>> + NS2_SUBCMD_FLASH_WRITE_BLOCK = 0x02,
>> + NS2_SUBCMD_FLASH_ERASE_BLOCK = 0x03,
>> + NS2_SUBCMD_FLASH_READ = 0x04,
>> + NS2_SUBCMD_FLASH_WRITE = 0x05,
>> +};
>> +
>> +enum switch2_subcmd_init {
>> + NS2_SUBCMD_INIT_SELECT_REPORT = 0xa,
>> + NS2_SUBCMD_INIT_USB = 0xd,
>> +};
>> +
>> +enum switch2_subcmd_feature_select {
>> + NS2_SUBCMD_FEATSEL_GET_INFO = 0x1,
>> + NS2_SUBCMD_FEATSEL_SET_MASK = 0x2,
>> + NS2_SUBCMD_FEATSEL_CLEAR_MASK = 0x3,
>> + NS2_SUBCMD_FEATSEL_ENABLE = 0x4,
>> + NS2_SUBCMD_FEATSEL_DISABLE = 0x5,
>> +};
>> +
>> +enum switch2_subcmd_grip {
>> + NS2_SUBCMD_GRIP_GET_INFO = 0x1,
>> + NS2_SUBCMD_GRIP_ENABLE_BUTTONS = 0x2,
>> + NS2_SUBCMD_GRIP_GET_INFO_EXT = 0x3,
>> +};
>> +
>> +enum switch2_subcmd_led {
>> + NS2_SUBCMD_LED_P1 = 0x1,
>> + NS2_SUBCMD_LED_P2 = 0x2,
>> + NS2_SUBCMD_LED_P3 = 0x3,
>> + NS2_SUBCMD_LED_P4 = 0x4,
>> + NS2_SUBCMD_LED_ALL_ON = 0x5,
>> + NS2_SUBCMD_LED_ALL_OFF = 0x6,
>> + NS2_SUBCMD_LED_PATTERN = 0x7,
>> + NS2_SUBCMD_LED_BLINK = 0x8,
>> +};
>> +
>> +enum switch2_subcmd_fw_info {
>> + NS2_SUBCMD_FW_INFO_GET = 0x1,
>> +};
>> +
>> +enum switch2_ctlr_type {
>> + NS2_CTLR_TYPE_JCL = 0x00,
>> + NS2_CTLR_TYPE_JCR = 0x01,
>> + NS2_CTLR_TYPE_PRO = 0x02,
>> + NS2_CTLR_TYPE_GC = 0x03,
>> +};
>> +
>> +enum switch2_report_id {
>> + NS2_REPORT_UNIFIED = 0x05,
>> + NS2_REPORT_JCL = 0x07,
>> + NS2_REPORT_JCR = 0x08,
>> + NS2_REPORT_PRO = 0x09,
>> + NS2_REPORT_GC = 0x0a,
>> +};
>> +
>> +enum switch2_init_step {
>> + NS2_INIT_READ_SERIAL,
>> + NS2_INIT_GET_FIRMWARE_INFO,
>> + NS2_INIT_READ_FACTORY_PRIMARY_CALIB,
>> + NS2_INIT_READ_FACTORY_SECONDARY_CALIB,
>> + NS2_INIT_READ_FACTORY_TRIGGER_CALIB,
>> + NS2_INIT_READ_USER_PRIMARY_CALIB,
>> + NS2_INIT_READ_USER_SECONDARY_CALIB,
>> + NS2_INIT_SET_FEATURE_MASK,
>> + NS2_INIT_ENABLE_FEATURES,
>> + NS2_INIT_GRIP_BUTTONS,
>> + NS2_INIT_REPORT_FORMAT,
>> + NS2_INIT_INPUT,
>> + NS2_INIT_SET_PLAYER_LEDS,
>> + NS2_INIT_FINISH,
>> + NS2_INIT_DONE,
>> +};
>> +
>> +struct switch2_version_info {
>> + uint8_t major;
>> + uint8_t minor;
>> + uint8_t patch;
>> + uint8_t ctlr_type;
>> + __le32 unk;
>> + int8_t dsp_major;
>> + int8_t dsp_minor;
>> + int8_t dsp_patch;
>> + int8_t dsp_type;
>> +};
>> +
>> +struct switch2_axis_calibration {
>> + uint16_t neutral;
>> + uint16_t negative;
>> + uint16_t positive;
>> +};
>> +
>> +struct switch2_stick_calibration {
>> + struct switch2_axis_calibration x;
>> + struct switch2_axis_calibration y;
>> +};
>> +
>> +struct switch2_controller {
>> + struct hid_device *hdev;
>> + struct switch2_cfg_intf *cfg;
>> + struct kref refcount;
>> +
>> + char name[64];
>> + char phys[64];
>> + struct list_head entry;
>> + struct mutex lock;
>> +
>> + enum switch2_ctlr_type ctlr_type;
>> + enum switch2_init_step init_step;
>> + int init_retries;
>> + struct input_dev __rcu *input;
>> + char serial[NS2_FLASH_SIZE_SERIAL + 1];
>> + struct switch2_version_info version;
>> +
>> + struct switch2_stick_calibration stick_calib[2];
>> + uint8_t lt_zero;
>> + uint8_t rt_zero;
>> +
>> + uint32_t player_id;
>> + struct led_classdev *leds;
>> +};
>> +
>> +static DEFINE_MUTEX(switch2_controllers_lock);
>> +static LIST_HEAD(switch2_controllers);
>> +
>> +struct switch2_ctlr_button_mapping {
>> + uint32_t code;
>> + int byte;
>> + uint32_t bit;
>> +};
>> +
>> +static const struct switch2_ctlr_button_mapping ns2_left_joycon_button_mappings[] = {
>> + { BTN_DPAD_LEFT, 0, NS2_BTNL_LEFT, },
>> + { BTN_DPAD_UP, 0, NS2_BTNL_UP, },
>> + { BTN_DPAD_DOWN, 0, NS2_BTNL_DOWN, },
>> + { BTN_DPAD_RIGHT, 0, NS2_BTNL_RIGHT, },
>> + { BTN_TL, 0, NS2_BTNL_L, },
>> + { BTN_TL2, 0, NS2_BTNL_ZL, },
>> + { BTN_SELECT, 0, NS2_BTNL_MINUS, },
>> + { BTN_THUMBL, 0, NS2_BTNL_LS, },
>> + { KEY_RECORD, 1, NS2_BTN_JCL_CAPTURE, },
>> + { BTN_GRIPR, 1, NS2_BTN_JCL_SL, },
>> + { BTN_GRIPR2, 1, NS2_BTN_JCL_SR, },
>> + { BTN_GRIPL, 1, NS2_BTN_JCL_GL, },
>> + { /* sentinel */ },
>> +};
>> +
>> +static const struct switch2_ctlr_button_mapping ns2_right_joycon_button_mappings[] = {
>> + { BTN_SOUTH, 0, NS2_BTNR_A, },
>> + { BTN_EAST, 0, NS2_BTNR_B, },
>> + { BTN_NORTH, 0, NS2_BTNR_X, },
>> + { BTN_WEST, 0, NS2_BTNR_Y, },
>> + { BTN_TR, 0, NS2_BTNR_R, },
>> + { BTN_TR2, 0, NS2_BTNR_ZR, },
>> + { BTN_START, 0, NS2_BTNR_PLUS, },
>> + { BTN_THUMBR, 0, NS2_BTNR_RS, },
>> + { BTN_C, 1, NS2_BTN_JCR_C, },
>> + { BTN_MODE, 1, NS2_BTN_JCR_HOME, },
>> + { BTN_GRIPL2, 1, NS2_BTN_JCR_SL, },
>> + { BTN_GRIPL, 1, NS2_BTN_JCR_SR, },
>> + { BTN_GRIPR, 1, NS2_BTN_JCR_GR, },
>> + { /* sentinel */ },
>> +};
>> +
>> +static const struct switch2_ctlr_button_mapping ns2_procon_mappings[] = {
>> + { BTN_SOUTH, 0, NS2_BTNR_A, },
>> + { BTN_EAST, 0, NS2_BTNR_B, },
>> + { BTN_NORTH, 0, NS2_BTNR_X, },
>> + { BTN_WEST, 0, NS2_BTNR_Y, },
>> + { BTN_TL, 1, NS2_BTNL_L, },
>> + { BTN_TR, 0, NS2_BTNR_R, },
>> + { BTN_TL2, 1, NS2_BTNL_ZL, },
>> + { BTN_TR2, 0, NS2_BTNR_ZR, },
>> + { BTN_SELECT, 1, NS2_BTNL_MINUS, },
>> + { BTN_START, 0, NS2_BTNR_PLUS, },
>> + { BTN_THUMBL, 1, NS2_BTNL_LS, },
>> + { BTN_THUMBR, 0, NS2_BTNR_RS, },
>> + { BTN_MODE, 2, NS2_BTN_PRO_HOME },
>> + { KEY_RECORD, 2, NS2_BTN_PRO_CAPTURE },
>> + { BTN_GRIPR, 2, NS2_BTN_PRO_GR },
>> + { BTN_GRIPL, 2, NS2_BTN_PRO_GL },
>> + { BTN_C, 2, NS2_BTN_PRO_C },
>> + { /* sentinel */ },
>> +};
>> +
>> +static const struct switch2_ctlr_button_mapping ns2_gccon_mappings[] = {
>> + { BTN_SOUTH, 0, NS2_BTNR_A, },
>> + { BTN_EAST, 0, NS2_BTNR_B, },
>> + { BTN_NORTH, 0, NS2_BTNR_X, },
>> + { BTN_WEST, 0, NS2_BTNR_Y, },
>> + { BTN_TL2, 1, NS2_BTNL_L, },
>> + { BTN_TR2, 0, NS2_BTNR_R, },
>> + { BTN_TL, 1, NS2_BTNL_ZL, },
>> + { BTN_TR, 0, NS2_BTNR_ZR, },
>> + { BTN_SELECT, 1, NS2_BTNL_MINUS, },
>> + { BTN_START, 0, NS2_BTNR_PLUS, },
>> + { BTN_MODE, 2, NS2_BTN_GC_HOME },
>> + { KEY_RECORD, 2, NS2_BTN_GC_CAPTURE },
>> + { BTN_C, 2, NS2_BTN_GC_C },
>> + { /* sentinel */ },
>> +};
>> +
>> +static const uint8_t switch2_init_cmd_data[] = {
>> + /*
>> + * The last 6 bytes of this packet are the MAC address of
>> + * the console, but we don't need that for USB
>> + */
>> + 0x01, 0x00, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF
>> +};
>> +
>> +static const uint8_t switch2_one_data[] = { 0x01, 0x00, 0x00, 0x00 };
>> +
>> +static const uint8_t switch2_feature_mask[] = {
>> + NS2_FEATURE_BUTTONS | NS2_FEATURE_ANALOG | NS2_FEATURE_IMU,
>> + 0x00, 0x00, 0x00
>> +};
>> +
>> +static int switch2_init_controller(struct switch2_controller *ns2);
>> +
>> +static void switch2_init_step_done(struct switch2_controller *ns2, enum switch2_init_step step)
>> +{
>> + if (ns2->init_step != step)
>> + return;
>> +
>> + ns2->init_retries = 0;
>> + ns2->init_step++;
>> +}
>> +
>> +static inline bool switch2_ctlr_is_joycon(enum switch2_ctlr_type type)
>> +{
>> + return type == NS2_CTLR_TYPE_JCL || type == NS2_CTLR_TYPE_JCR;
>> +}
>> +
>> +static struct switch2_controller *switch2_get_controller(const char *phys)
>> +{
>> + struct switch2_controller *ns2;
>> +
>> + guard(mutex)(&switch2_controllers_lock);
>> + list_for_each_entry(ns2, &switch2_controllers, entry) {
>> + if (strncmp(ns2->phys, phys, sizeof(ns2->phys)) == 0) {
>> + if (kref_get_unless_zero(&ns2->refcount))
>> + return ns2;
>> + }
>> + }
>> + ns2 = kzalloc(sizeof(*ns2), GFP_KERNEL);
>> + if (!ns2)
>> + return ERR_PTR(-ENOMEM);
>> +
>> + kref_init(&ns2->refcount);
>> + mutex_init(&ns2->lock);
>> + INIT_LIST_HEAD(&ns2->entry);
>> + list_add(&ns2->entry, &switch2_controllers);
>> + strscpy(ns2->phys, phys, sizeof(ns2->phys));
>> + return ns2;
>> +}
>> +
>> +static void switch2_controller_put(struct switch2_controller *ns2)
>> +{
>> + struct input_dev *input;
>> +
>> + mutex_lock(&ns2->lock);
>> + rcu_read_lock();
>> + input = rcu_dereference(ns2->input);
>> + rcu_read_unlock();
>> +
>> + rcu_assign_pointer(ns2->input, NULL);
>> + synchronize_rcu();
>> +
>> + ns2->init_step = 0;
>> + mutex_unlock(&ns2->lock);
>> +
>> + if (input)
>> + input_unregister_device(input);
>> +}
>> +
>> +static void switch2_kref_put(struct kref *refcount)
>> +{
>> + struct switch2_controller *ns2 = container_of(refcount,
>> + struct switch2_controller, refcount);
>> +
>> + guard(mutex)(&switch2_controllers_lock);
>> + list_del_init(&ns2->entry);
>> + mutex_destroy(&ns2->lock);
>> + kfree(ns2);
>> +}
>> +
>> +static int switch2_set_leds(struct switch2_controller *ns2)
>> +{
>> + int i;
>> + uint8_t message[8] = { 0 };
>> +
>> + for (i = 0; i < JC_NUM_LEDS; i++)
>> + message[0] |= (!!ns2->leds[i].brightness) << i;
>> +
>> + if (!ns2->cfg)
>> + return -ENOTCONN;
>> + return ns2->cfg->send_command(NS2_CMD_LED, NS2_SUBCMD_LED_PATTERN,
>> + &message, sizeof(message),
>> + ns2->cfg);
>> +}
>> +
>> +static int switch2_player_led_brightness_set(struct led_classdev *led,
>> + enum led_brightness brightness)
>> +{
>> + struct device *dev = led->dev->parent;
>> + struct input_dev *input = to_input_dev(dev);
>> + struct switch2_controller *ns2 = input_get_drvdata(input);
>> +
>> + if (!ns2)
>> + return -ENODEV;
>> +
>> + guard(mutex)(&ns2->lock);
>> + return switch2_set_leds(ns2);
>> +}
>> +
>> +static void switch2_config_buttons(struct input_dev *idev,
>> + const struct switch2_ctlr_button_mapping button_mappings[])
>> +{
>> + const struct switch2_ctlr_button_mapping *button;
>> +
>> + for (button = button_mappings; button->code; button++)
>> + input_set_capability(idev, EV_KEY, button->code);
>> +}
>> +
>> +static int switch2_input_ref(struct input_dev *input)
>> +{
>> + struct switch2_controller *ns2 = input_get_drvdata(input);
>> +
>> + kref_get(&ns2->refcount);
>> +
>> + return 0;
>> +}
>> +
>> +static void switch2_input_deref(struct input_dev *input)
>> +{
>> + struct switch2_controller *ns2 = input_get_drvdata(input);
>> +
>> + kref_put(&ns2->refcount, switch2_kref_put);
>> +}
>> +
>> +static int switch2_init_input(struct switch2_controller *ns2)
>> +{
>> + struct input_dev *input;
>> + struct hid_device *hdev = ns2->hdev;
>> + int player_led_pattern;
>> + int i;
>> + int ret;
>> +
>> + rcu_read_lock();
>> + input = rcu_dereference(ns2->input);
>> + rcu_read_unlock();
>> +
>> + if (input) {
>> + switch2_init_step_done(ns2, NS2_INIT_INPUT);
>> + return 0;
>> + }
>> +
>> + input = input_allocate_device();
>> + if (!input)
>> + return -ENOMEM;
>> +
>> + input_set_drvdata(input, ns2);
>> + input->open = switch2_input_ref;
>> + input->close = switch2_input_deref;
>> + input->dev.parent = &hdev->dev;
>> + input->id.bustype = hdev->bus;
>> + input->id.vendor = hdev->vendor;
>> + input->id.product = hdev->product;
>> + input->id.version = hdev->version;
>> + input->uniq = ns2->serial;
>> + input->name = ns2->name;
>> + input->phys = hdev->phys;
>> +
>> + switch (ns2->ctlr_type) {
>> + case NS2_CTLR_TYPE_JCL:
>> + input_set_abs_params(input, ABS_X, NS2_AXIS_MIN, NS2_AXIS_MAX, 32, 128);
>> + input_set_abs_params(input, ABS_Y, NS2_AXIS_MIN, NS2_AXIS_MAX, 32, 128);
>> + switch2_config_buttons(input, ns2_left_joycon_button_mappings);
>> + break;
>> + case NS2_CTLR_TYPE_JCR:
>> + input_set_abs_params(input, ABS_X, NS2_AXIS_MIN, NS2_AXIS_MAX, 32, 128);
>> + input_set_abs_params(input, ABS_Y, NS2_AXIS_MIN, NS2_AXIS_MAX, 32, 128);
>> + switch2_config_buttons(input, ns2_right_joycon_button_mappings);
>> + break;
>> + case NS2_CTLR_TYPE_GC:
>> + input_set_abs_params(input, ABS_X, NS2_AXIS_MIN, NS2_AXIS_MAX, 32, 128);
>> + input_set_abs_params(input, ABS_Y, NS2_AXIS_MIN, NS2_AXIS_MAX, 32, 128);
>> + input_set_abs_params(input, ABS_RX, NS2_AXIS_MIN, NS2_AXIS_MAX, 32, 128);
>> + input_set_abs_params(input, ABS_RY, NS2_AXIS_MIN, NS2_AXIS_MAX, 32, 128);
>> + input_set_abs_params(input, ABS_Z, 0, NS2_TRIGGER_RANGE, 32, 128);
>> + input_set_abs_params(input, ABS_RZ, 0, NS2_TRIGGER_RANGE, 32, 128);
>> + input_set_abs_params(input, ABS_HAT0X, -1, 1, 0, 0);
>> + input_set_abs_params(input, ABS_HAT0Y, -1, 1, 0, 0);
>> + switch2_config_buttons(input, ns2_gccon_mappings);
>> + break;
>> + case NS2_CTLR_TYPE_PRO:
>> + input_set_abs_params(input, ABS_X, NS2_AXIS_MIN, NS2_AXIS_MAX, 32, 128);
>> + input_set_abs_params(input, ABS_Y, NS2_AXIS_MIN, NS2_AXIS_MAX, 32, 128);
>> + input_set_abs_params(input, ABS_RX, NS2_AXIS_MIN, NS2_AXIS_MAX, 32, 128);
>> + input_set_abs_params(input, ABS_RY, NS2_AXIS_MIN, NS2_AXIS_MAX, 32, 128);
>> + input_set_abs_params(input, ABS_HAT0X, -1, 1, 0, 0);
>> + input_set_abs_params(input, ABS_HAT0Y, -1, 1, 0, 0);
>> + switch2_config_buttons(input, ns2_procon_mappings);
>> + break;
>> + default:
>> + input_free_device(input);
>> + return -EINVAL;
>> + }
>> +
>> + hid_info(ns2->hdev, "Firmware version %u.%u.%u (type %i)\n", ns2->version.major,
>> + ns2->version.minor, ns2->version.patch, ns2->version.ctlr_type);
>> + if (ns2->version.dsp_type >= 0)
>> + hid_info(ns2->hdev, "DSP version %u.%u.%u\n", ns2->version.dsp_major,
>> + ns2->version.dsp_minor, ns2->version.dsp_patch);
>> +
>> + ret = input_register_device(input);
>> + if (ret < 0) {
>> + hid_err(ns2->hdev, "Failed to register input; ret=%d\n", ret);
>> + input_free_device(input);
>> + return ret;
>> + }
>> +
>> + player_led_pattern = ns2->player_id % JC_NUM_LED_PATTERNS;
>> + hid_dbg(hdev, "assigned player %d led pattern", player_led_pattern + 1);
>> +
>> + ns2->leds = devm_kcalloc(&input->dev, JC_NUM_LEDS, sizeof(*ns2->leds), GFP_KERNEL);
>> + if (!ns2->leds) {
>> + hid_err(ns2->hdev, "Failed to allocate LEDs\n");
>> + input_unregister_device(input);
>> + return -ENOMEM;
>> + }
>> +
>> + for (i = 0; i < JC_NUM_LEDS; i++) {
>> + struct led_classdev *led = &ns2->leds[i];
>> +
>> + led->brightness = joycon_player_led_patterns[player_led_pattern][i];
>> + led->max_brightness = 1;
>> + led->brightness_set_blocking = switch2_player_led_brightness_set;
>> + led->flags = LED_CORE_SUSPENDRESUME | LED_HW_PLUGGABLE | LED_RETAIN_AT_SHUTDOWN;
>> + char *name = devm_kasprintf(&input->dev, GFP_KERNEL, "%s:%s:%s",
>> + dev_name(&input->dev),
>> + "green",
>> + joycon_player_led_names[i]);
>> +
>> + if (!name) {
>> + dev_err(&input->dev, "Failed to allocate name for player %d LED; ret=%d\n",
>> + i + 1, ret);
>> + break;
>> + }
>> +
>> + led->name = name;
>> + ret = devm_led_classdev_register(&input->dev, led);
>> + if (ret < 0) {
>> + dev_err(&input->dev, "Failed to register player %d LED; ret=%d\n",
>> + i + 1, ret);
>> + break;
>> + }
>> + }
>> +
>> + rcu_assign_pointer(ns2->input, input);
>> + synchronize_rcu();
>> +
>> + switch2_init_step_done(ns2, NS2_INIT_INPUT);
>> + return switch2_init_controller(ns2);
>> +}
>> +
>> +static bool switch2_parse_stick_calibration(struct switch2_stick_calibration *calib,
>> + const uint8_t *data)
>> +{
>> + static const uint8_t UNCALIBRATED[9] = {
>> + 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF
>> + };
>> + if (memcmp(UNCALIBRATED, data, sizeof(UNCALIBRATED)) == 0)
>> + return false;
>> +
>> + calib->x.neutral = data[0];
>> + calib->x.neutral |= (data[1] & 0x0F) << 8;
>> +
>> + calib->y.neutral = data[1] >> 4;
>> + calib->y.neutral |= data[2] << 4;
>> +
>> + calib->x.positive = data[3];
>> + calib->x.positive |= (data[4] & 0x0F) << 8;
>> +
>> + calib->y.positive = data[4] >> 4;
>> + calib->y.positive |= data[5] << 4;
>> +
>> + calib->x.negative = data[6];
>> + calib->x.negative |= (data[7] & 0x0F) << 8;
>> +
>> + calib->y.negative = data[7] >> 4;
>> + calib->y.negative |= data[8] << 4;
>> +
>> + return true;
>> +}
>> +
>> +static void switch2_handle_flash_read(struct switch2_controller *ns2, uint8_t size,
>> + uint32_t address, const uint8_t *data)
>> +{
>> + bool ok;
>> +
>> + switch (address) {
>> + case NS2_FLASH_ADDR_SERIAL:
>> + if (size != NS2_FLASH_SIZE_SERIAL)
>> + return;
>> + memcpy(ns2->serial, data, size);
>> + switch2_init_step_done(ns2, NS2_INIT_READ_SERIAL);
>> + break;
>> + case NS2_FLASH_ADDR_FACTORY_PRIMARY_CALIB:
>> + if (size != NS2_FLASH_SIZE_FACTORY_AXIS_CALIB)
>> + return;
>> + switch2_init_step_done(ns2, NS2_INIT_READ_FACTORY_PRIMARY_CALIB);
>> + ok = switch2_parse_stick_calibration(&ns2->stick_calib[0], data);
>> + if (ns2->hdev) {
>> + if (ok) {
>> + hid_dbg(ns2->hdev, "Got factory primary stick calibration:\n");
>> + hid_dbg(ns2->hdev, "Left max: %i, neutral: %i, right max: %i\n",
>> + ns2->stick_calib[0].x.negative,
>> + ns2->stick_calib[0].x.neutral,
>> + ns2->stick_calib[0].x.positive);
>> + hid_dbg(ns2->hdev, "Down max: %i, neutral: %i, up max: %i\n",
>> + ns2->stick_calib[0].y.negative,
>> + ns2->stick_calib[0].y.neutral,
>> + ns2->stick_calib[0].y.positive);
>> + } else {
>> + hid_dbg(ns2->hdev, "Factory primary stick calibration not present\n");
>> + }
>> + }
>> + break;
>> + case NS2_FLASH_ADDR_FACTORY_SECONDARY_CALIB:
>> + if (size != NS2_FLASH_SIZE_FACTORY_AXIS_CALIB)
>> + return;
>> + switch2_init_step_done(ns2, NS2_INIT_READ_FACTORY_SECONDARY_CALIB);
>> + ok = switch2_parse_stick_calibration(&ns2->stick_calib[1], data);
>> + if (ns2->hdev) {
>> + if (ok) {
>> + hid_dbg(ns2->hdev, "Got factory secondary stick calibration:\n");
>> + hid_dbg(ns2->hdev, "Left max: %i, neutral: %i, right max: %i\n",
>> + ns2->stick_calib[1].x.negative,
>> + ns2->stick_calib[1].x.neutral,
>> + ns2->stick_calib[1].x.positive);
>> + hid_dbg(ns2->hdev, "Down max: %i, neutral: %i, up max: %i\n",
>> + ns2->stick_calib[1].y.negative,
>> + ns2->stick_calib[1].y.neutral,
>> + ns2->stick_calib[1].y.positive);
>> + } else {
>> + hid_dbg(ns2->hdev, "Factory secondary stick calibration not present\n");
>> + }
>> + }
>> + break;
>> + case NS2_FLASH_ADDR_FACTORY_TRIGGER_CALIB:
>> + if (size != NS2_FLASH_SIZE_FACTORY_TRIGGER_CALIB)
>> + return;
>> + switch2_init_step_done(ns2, NS2_INIT_READ_FACTORY_TRIGGER_CALIB);
>> + if (data[0] != 0xFF && data[1] != 0xFF) {
>> + ns2->lt_zero = data[0];
>> + ns2->rt_zero = data[1];
>> +
>> + if (ns2->hdev) {
>> + hid_dbg(ns2->hdev, "Got factory trigger calibration:\n");
>> + hid_dbg(ns2->hdev, "Left zero point: %i\n", ns2->lt_zero);
>> + hid_dbg(ns2->hdev, "Right zero point: %i\n", ns2->rt_zero);
>> + }
>> + } else if (ns2->hdev) {
>> + hid_dbg(ns2->hdev, "Factory trigger calibration not present\n");
>> + }
>> + break;
>> + case NS2_FLASH_ADDR_USER_PRIMARY_CALIB:
>> + if (size != NS2_FLASH_SIZE_USER_AXIS_CALIB)
>> + return;
>> + switch2_init_step_done(ns2, NS2_INIT_READ_USER_PRIMARY_CALIB);
>> + if (get_unaligned_le16((__le16 *)data) != NS2_USER_CALIB_MAGIC) {
>> + if (ns2->hdev)
>> + hid_dbg(ns2->hdev, "No user primary stick calibration present\n");
>> + break;
>> + }
>> +
>> + ok = switch2_parse_stick_calibration(&ns2->stick_calib[0], &data[2]);
>> + if (ns2->hdev) {
>> + if (ok) {
>> + hid_dbg(ns2->hdev, "Got user primary stick calibration:\n");
>> + hid_dbg(ns2->hdev, "Left max: %i, neutral: %i, right max: %i\n",
>> + ns2->stick_calib[0].x.negative,
>> + ns2->stick_calib[0].x.neutral,
>> + ns2->stick_calib[0].x.positive);
>> + hid_dbg(ns2->hdev, "Down max: %i, neutral: %i, up max: %i\n",
>> + ns2->stick_calib[0].y.negative,
>> + ns2->stick_calib[0].y.neutral,
>> + ns2->stick_calib[0].y.positive);
>> + } else {
>> + hid_dbg(ns2->hdev, "No user primary stick calibration present\n");
>> + }
>> + }
>> + break;
>> + case NS2_FLASH_ADDR_USER_SECONDARY_CALIB:
>> + if (size != NS2_FLASH_SIZE_USER_AXIS_CALIB)
>> + return;
>> + switch2_init_step_done(ns2, NS2_INIT_READ_USER_SECONDARY_CALIB);
>> + if (get_unaligned_le16((__le16 *)data) != NS2_USER_CALIB_MAGIC) {
>> + if (ns2->hdev)
>> + hid_dbg(ns2->hdev, "No user secondary stick calibration present\n");
>> + break;
>> + }
>> +
>> + ok = switch2_parse_stick_calibration(&ns2->stick_calib[1], &data[2]);
>> + if (ns2->hdev) {
>> + if (ok) {
>> + hid_dbg(ns2->hdev, "Got user secondary stick calibration:\n");
>> + hid_dbg(ns2->hdev, "Left max: %i, neutral: %i, right max: %i\n",
>> + ns2->stick_calib[1].x.negative,
>> + ns2->stick_calib[1].x.neutral,
>> + ns2->stick_calib[1].x.positive);
>> + hid_dbg(ns2->hdev, "Down max: %i, neutral: %i, up max: %i\n",
>> + ns2->stick_calib[1].y.negative,
>> + ns2->stick_calib[1].y.neutral,
>> + ns2->stick_calib[1].y.positive);
>> + } else {
>> + hid_dbg(ns2->hdev, "No user secondary stick calibration present\n");
>> + }
>> + }
>> + break;
>
> nit: this break is redundant and could be removed.
I always put breaks at the end of switch statements, for consistency. I'm pretty sure that's common with C coders, and plenty of kernel code does this.>
>> + }
>> +}
>> +
>> +static void switch2_report_buttons(struct input_dev *input, const uint8_t *bytes,
>> + const struct switch2_ctlr_button_mapping button_mappings[])
>> +{
>> + const struct switch2_ctlr_button_mapping *button;
>> +
>> + for (button = button_mappings; button->code; button++)
>> + input_report_key(input, button->code, bytes[button->byte] & button->bit);
>> +}
>> +
>> +static void switch2_report_axis(struct input_dev *input, struct switch2_axis_calibration *calib,
>> + int axis, bool invert, int value)
>> +{
>> + if (calib && calib->neutral && calib->negative && calib->positive) {
>> + value -= calib->neutral;
>> + value *= NS2_AXIS_MAX + 1;
>> + if (value < 0)
>> + value /= calib->negative;
>
> We might want to make sure that calib->{negative,positive} are not zero
> (at calibration parsing time) in order to avoid a div-by-zero issue
I could go either way on this. Sure, it'd clean up this side a bit, but I don't think it's important enough to iterate this just for that.
>
>> + else
>> + value /= calib->positive;
>> + } else {
>> + value = (value - 2048) * 16;
>> + }
>> +
>> + if (invert)
>> + value = -value;
>> + input_report_abs(input, axis,
>> + clamp(value, NS2_AXIS_MIN, NS2_AXIS_MAX));
>> +}
>> +
>> +static void switch2_report_stick(struct input_dev *input, struct switch2_stick_calibration *calib,
>> + int x, bool invert_x, int y, bool invert_y, const uint8_t *data)
>> +{
>> + switch2_report_axis(input, &calib->x, x, invert_x, data[0] | ((data[1] & 0x0F) << 8));
>> + switch2_report_axis(input, &calib->y, y, invert_y, (data[1] >> 4) | (data[2] << 4));
>> +}
>> +
>> +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);
>> +
>> + if (zero != 232)
>> + value /= (232 - zero);
>> + input_report_abs(input, abs, clamp(value, 0, NS2_TRIGGER_RANGE));
>> +}
>> +
>> +static int switch2_event(struct hid_device *hdev, struct hid_report *report, uint8_t *raw_data,
>> + int size)
>> +{
>> + struct switch2_controller *ns2 = hid_get_drvdata(hdev);
>> + struct input_dev *input;
>> +
>> + if (report->type != HID_INPUT_REPORT)
>> + return 0;
>> +
>> + if (size < 15)
>> + return -EINVAL;
>> +
>> + guard(rcu)();
>> + input = rcu_dereference(ns2->input);
>> +
>> + if (!input)
>> + return 0;
>> +
>> + 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.
>> + */
>> + break;
>> + case NS2_REPORT_JCL:
>> + switch2_report_stick(input, &ns2->stick_calib[0], ABS_X, false,
>> + ABS_Y, true, &raw_data[6]);
>> + switch2_report_buttons(input, &raw_data[3], ns2_left_joycon_button_mappings);
>> + break;
>> + case NS2_REPORT_JCR:
>> + switch2_report_stick(input, &ns2->stick_calib[0], ABS_X, false,
>> + ABS_Y, true, &raw_data[6]);
>> + switch2_report_buttons(input, &raw_data[3], ns2_right_joycon_button_mappings);
>> + break;
>> + case NS2_REPORT_GC:
>> + input_report_abs(input, ABS_HAT0X,
>> + !!(raw_data[4] & NS2_BTNL_RIGHT) -
>> + !!(raw_data[4] & NS2_BTNL_LEFT));
>> + input_report_abs(input, ABS_HAT0Y,
>> + !!(raw_data[4] & NS2_BTNL_DOWN) -
>> + !!(raw_data[4] & NS2_BTNL_UP));
>> + switch2_report_buttons(input, &raw_data[3], ns2_gccon_mappings);
>> + switch2_report_stick(input, &ns2->stick_calib[0], ABS_X, false,
>> + ABS_Y, true, &raw_data[6]);
>> + switch2_report_stick(input, &ns2->stick_calib[1], ABS_RX, false,
>> + ABS_RY, true, &raw_data[9]);
>> + switch2_report_trigger(input, ns2->lt_zero, ABS_Z, raw_data[13]);
>> + switch2_report_trigger(input, ns2->rt_zero, ABS_RZ, raw_data[14]);
>> + break;
>> + case NS2_REPORT_PRO:
>> + input_report_abs(input, ABS_HAT0X,
>> + !!(raw_data[4] & NS2_BTNL_RIGHT) -
>> + !!(raw_data[4] & NS2_BTNL_LEFT));
>> + input_report_abs(input, ABS_HAT0Y,
>> + !!(raw_data[4] & NS2_BTNL_DOWN) -
>> + !!(raw_data[4] & NS2_BTNL_UP));
>> + switch2_report_buttons(input, &raw_data[3], ns2_procon_mappings);
>> + switch2_report_stick(input, &ns2->stick_calib[0], ABS_X, false,
>> + ABS_Y, true, &raw_data[6]);
>> + switch2_report_stick(input, &ns2->stick_calib[1], ABS_RX, false,
>> + ABS_RY, true, &raw_data[9]);
>> + break;
>> + default:
>> + return -EINVAL;
>> + }
>> +
>> + input_sync(input);
>> + return 0;
>> +}
>> +
>> +static int switch2_features_enable(struct switch2_controller *ns2, int features)
>> +{
>> + __le32 feature_bits = __cpu_to_le32(features);
>> +
>> + if (!ns2->cfg)
>
> From what I can tell switch2_features_enable is only called from
> switch2_init_controller, which is already checking that ns2->cfg is not
> NULL (while holding the ns2->lock). So I would assume that this check is
> not necessary
For now this is the case. In the future we might use it from other places to save battery when used wirelessly, e.g. disabling the IMU when it's not in use.
>
>> + return -ENOTCONN;
>> + return ns2->cfg->send_command(NS2_CMD_FEATSEL, NS2_SUBCMD_FEATSEL_ENABLE,
>> + &feature_bits, sizeof(feature_bits),
>> + ns2->cfg);
>> +}
>> +
>> +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;
>
> See above.
Above wouldn't be the case here. I guess it is redundant, but it's a bit of a case of better safe than sorry if it ever does get reused. I'd imagine the compiler optimizes it out anyway, as this function is small enough to get inlined and have the check eaten by common subexpression elimination.
>
>> + put_unaligned_le32(address, &message[4]);
>> + return ns2->cfg->send_command(NS2_CMD_FLASH, NS2_SUBCMD_FLASH_READ, message,
>> + sizeof(message), ns2->cfg);
>> +}
>> +
>> +static int switch2_set_player_id(struct switch2_controller *ns2, uint32_t player_id)
>> +{
>> + int i;
>> + int player_led_pattern = player_id % JC_NUM_LED_PATTERNS;
>> +
>> + for (i = 0; i < JC_NUM_LEDS; i++)
>> + ns2->leds[i].brightness = joycon_player_led_patterns[player_led_pattern][i];
>> +
>> + return switch2_set_leds(ns2);
>> +}
>> +
>> +static int switch2_set_report_format(struct switch2_controller *ns2, enum switch2_report_id fmt)
>> +{
>> + __le32 format_id = __cpu_to_le32(fmt);
>> +
>> + if (!ns2->cfg)
>> + return -ENOTCONN;
>> + return ns2->cfg->send_command(NS2_CMD_INIT, NS2_SUBCMD_INIT_SELECT_REPORT,
>> + &format_id, sizeof(format_id),
>> + ns2->cfg);
>> +}
>> +
>> +int switch2_init_controller(struct switch2_controller *ns2)
>> +{
>> + if (ns2->init_step == NS2_INIT_DONE)
>> + return 0;
>> +
>> + if (!ns2->cfg)
>> + return -ENOTCONN;
>> +
>> + if (ns2->init_retries > NS2_MAX_INIT_RETRIES) {
>> + if (ns2->init_retries == NS2_MAX_INIT_RETRIES + 1) {
>
> Couldn't we get rid of this if-clause and the ns2->init_retries++ after
> it? That would mean we would potentially log the error message earlier
> but I don't think that would be an issue
The point of this is to avoid log spam: this function can get called repeatedly after init_retries has exceeded the limit, but we only want to log it once. We then increment it one more time to indicate it's already been logged so we don't printit further times. The other option would be to move the check to the other place where init_retries gets incremented, but it's functionally equivalent.
>
>
>> + if (ns2->cfg)
>
> We check this a few lines up so unless this somehow can get set
> to NULL again (even though we seem to be holding ns2->lock whenever
> switch2_init_controller is called) I don't think we need to check this
> a second time
Yeah this is redundant, you're right. I probably got overly paranoid while dealing with sashiko's nonsense.
>
>> + dev_err(ns2->cfg->dev, "Failed to configure controller\n");
>> + ns2->init_retries++;
>> + }
>> + return -EIO;
>> + }
>> +
>> + ns2->init_retries++;
>> + switch (ns2->init_step) {
>> + case NS2_INIT_READ_SERIAL:
>> + return switch2_read_flash(ns2, NS2_FLASH_ADDR_SERIAL,
>> + NS2_FLASH_SIZE_SERIAL);
>> + case NS2_INIT_GET_FIRMWARE_INFO:
>> + return ns2->cfg->send_command(NS2_CMD_FW_INFO, NS2_SUBCMD_FW_INFO_GET,
>> + NULL, 0, ns2->cfg);
>> + case NS2_INIT_READ_FACTORY_PRIMARY_CALIB:
>> + return switch2_read_flash(ns2, NS2_FLASH_ADDR_FACTORY_PRIMARY_CALIB,
>> + NS2_FLASH_SIZE_FACTORY_AXIS_CALIB);
>> + case NS2_INIT_READ_FACTORY_SECONDARY_CALIB:
>> + if (switch2_ctlr_is_joycon(ns2->ctlr_type)) {
>> + switch2_init_step_done(ns2, ns2->init_step);
>> + return switch2_init_controller(ns2);
>> + }
>> + return switch2_read_flash(ns2, NS2_FLASH_ADDR_FACTORY_SECONDARY_CALIB,
>> + NS2_FLASH_SIZE_FACTORY_AXIS_CALIB);
>> + case NS2_INIT_READ_FACTORY_TRIGGER_CALIB:
>> + if (ns2->ctlr_type != NS2_CTLR_TYPE_GC) {
>> + switch2_init_step_done(ns2, ns2->init_step);
>> + return switch2_init_controller(ns2);
>> + }
>> + return switch2_read_flash(ns2, NS2_FLASH_ADDR_FACTORY_TRIGGER_CALIB,
>> + NS2_FLASH_SIZE_FACTORY_TRIGGER_CALIB);
>> + case NS2_INIT_READ_USER_PRIMARY_CALIB:
>> + return switch2_read_flash(ns2, NS2_FLASH_ADDR_USER_PRIMARY_CALIB,
>> + NS2_FLASH_SIZE_USER_AXIS_CALIB);
>> + case NS2_INIT_READ_USER_SECONDARY_CALIB:
>> + if (switch2_ctlr_is_joycon(ns2->ctlr_type)) {
>> + switch2_init_step_done(ns2, ns2->init_step);
>> + return switch2_init_controller(ns2);
>> + }
>> + return switch2_read_flash(ns2, NS2_FLASH_ADDR_USER_SECONDARY_CALIB,
>> + NS2_FLASH_SIZE_USER_AXIS_CALIB);
>> + case NS2_INIT_SET_FEATURE_MASK:
>> + return ns2->cfg->send_command(NS2_CMD_FEATSEL, NS2_SUBCMD_FEATSEL_SET_MASK,
>> + switch2_feature_mask, sizeof(switch2_feature_mask), ns2->cfg);
>> + case NS2_INIT_ENABLE_FEATURES:
>> + return switch2_features_enable(ns2, NS2_FEATURE_BUTTONS | NS2_FEATURE_ANALOG);
>> + case NS2_INIT_GRIP_BUTTONS:
>> + if (!switch2_ctlr_is_joycon(ns2->ctlr_type)) {
>> + switch2_init_step_done(ns2, ns2->init_step);
>> + return switch2_init_controller(ns2);
>> + }
>> + return ns2->cfg->send_command(NS2_CMD_GRIP, NS2_SUBCMD_GRIP_ENABLE_BUTTONS,
>> + switch2_one_data, sizeof(switch2_one_data),
>> + ns2->cfg);
>> + case NS2_INIT_REPORT_FORMAT:
>> + switch (ns2->ctlr_type) {
>> + case NS2_CTLR_TYPE_JCL:
>> + return switch2_set_report_format(ns2, NS2_REPORT_JCL);
>> + case NS2_CTLR_TYPE_JCR:
>> + return switch2_set_report_format(ns2, NS2_REPORT_JCR);
>> + case NS2_CTLR_TYPE_PRO:
>> + return switch2_set_report_format(ns2, NS2_REPORT_PRO);
>> + case NS2_CTLR_TYPE_GC:
>> + return switch2_set_report_format(ns2, NS2_REPORT_GC);
>> + default:
>> + switch2_init_step_done(ns2, ns2->init_step);
>> + return switch2_init_controller(ns2);
>> + }
>> + case NS2_INIT_INPUT:
>> + if (ns2->hdev)
>> + return switch2_init_input(ns2);
>> + break;
>> + case NS2_INIT_SET_PLAYER_LEDS:
>> + return switch2_set_player_id(ns2, ns2->player_id);
>> + case NS2_INIT_FINISH:
>> + return ns2->cfg->send_command(NS2_CMD_INIT, NS2_SUBCMD_INIT_USB,
>> + switch2_init_cmd_data, sizeof(switch2_init_cmd_data), ns2->cfg);
>> + default:
>> + WARN_ON_ONCE(1);
>> + break;
>> + }
>> + return 0;
>> +}
>> +
>> +int switch2_receive_command(struct switch2_controller *ns2,
>> + const uint8_t *message, size_t length)
>> +{
>> + const struct switch2_cmd_header *header;
>> + int ret = 0;
>> +
>> + if (length < 8)
>
> Shouldn't this check for 'length < 9' as otherwise
>
> message = &message[8];
>
> would index the ninth byte which is out of range (i.e. off-by-one issue)?
No, 8 byte messages are valid. &message[8] would point to an empty message as a result. message[8] is technically the ninth byte if present, but when length is 0 it's functionally just an empty array.
>
>
>> + return -EINVAL;
>> +
>> + print_hex_dump_debug("got cmd: ", DUMP_PREFIX_OFFSET, 16, 1, message, length, false);
>> +
>> + mutex_lock(&ns2->lock);
>> +
>> + header = (const struct switch2_cmd_header *)message;
>> + if (!(header->flags & NS2_FLAG_OK)) {
>> + if (ns2->cfg)
>> + dev_warn(ns2->cfg->dev, "Packet error %02x replying to command %x:%x",
>> + header->flags, header->command, header->subcommand);
>> + ret = -EIO;
>> + goto exit;
>> + }
>> + message = &message[8];
>> + length -= 8;
>
> Shouldn't this be 'length -= 9' (as arrays are zero-indexed)?
No, see above. Length becomes 0, which is valid.
>
>> +
>> + switch (header->command) {
>> + case NS2_CMD_FLASH:
>> + if (header->subcommand == NS2_SUBCMD_FLASH_READ) {
>> + uint8_t read_size;
>> + uint32_t read_address;
>> +
>> + if (length < 8) {
>
> Shouldn't this be 'if (length < 9)' as well?
No, the original length would be 16 so the message minus the header is still at least 8 bytes.
>
>> + ret = -EINVAL;
>> + goto exit;
>> + }
>> + read_size = message[0];
>> + read_address = get_unaligned_le32(&message[4]);
>> + if (length < read_size + 8) {
>
> Should it check for
>
> if (length < read_size + 9) {
>
> as we are taking a pointer to the ninth byte with '&message[8]'?
Still no.
>
>
>> + ret = -EINVAL;
>> + goto exit;
>> + }
>> + switch2_handle_flash_read(ns2, read_size, read_address, &message[8]);
>> + }
>> + break;
>> + case NS2_CMD_INIT:
>> + if (header->subcommand == NS2_SUBCMD_INIT_USB)
>> + switch2_init_step_done(ns2, NS2_INIT_FINISH);
>> + else if (header->subcommand == NS2_SUBCMD_INIT_SELECT_REPORT)
>> + switch2_init_step_done(ns2, NS2_INIT_REPORT_FORMAT);
>> + break;
>> + case NS2_CMD_GRIP:
>> + if (header->subcommand == NS2_SUBCMD_GRIP_ENABLE_BUTTONS)
>> + switch2_init_step_done(ns2, NS2_INIT_GRIP_BUTTONS);
>> + break;
>> + case NS2_CMD_LED:
>> + if (header->subcommand == NS2_SUBCMD_LED_PATTERN)
>> + switch2_init_step_done(ns2, NS2_INIT_SET_PLAYER_LEDS);
>> + break;
>> + case NS2_CMD_FEATSEL:
>> + if (header->subcommand == NS2_SUBCMD_FEATSEL_SET_MASK)
>> + switch2_init_step_done(ns2, NS2_INIT_SET_FEATURE_MASK);
>> + else if (header->subcommand == NS2_SUBCMD_FEATSEL_ENABLE)
>> + switch2_init_step_done(ns2, NS2_INIT_ENABLE_FEATURES);
>> + break;
>> + 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));
>> + 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);
>> +
>> + mutex_unlock(&ns2->lock);
>> + return ret;
>> +}
>> +EXPORT_SYMBOL_GPL(switch2_receive_command);
>> +
>> +int switch2_controller_attach_cfg(const char *phys, struct switch2_cfg_intf *cfg)
>> +{
>> + struct switch2_controller *ns2 = switch2_get_controller(phys);
>> + int ret = 0;
>> +
>> + if (IS_ERR(ns2))
>> + return PTR_ERR(ns2);
>> +
>> + mutex_lock(&ns2->lock);
>> + if (ns2->cfg) {
>> + ret = -EBUSY;
>> + goto out;
>> + }
>> + cfg->parent = ns2;
>> + ns2->cfg = cfg;
>> +
>> + if (ns2->hdev)
>> + ret = switch2_init_controller(ns2);
>> +
>> + if (ret < 0) {
>> + cfg->parent = NULL;
>> + ns2->cfg = NULL;
>> + }
>> +
>> +out:
>> + mutex_unlock(&ns2->lock);
>> +
>> + if (ret < 0)
>> + kref_put(&ns2->refcount, switch2_kref_put);
>> + return ret;
>> +}
>> +EXPORT_SYMBOL_GPL(switch2_controller_attach_cfg);
>> +
>> +void switch2_controller_detach_cfg(struct switch2_controller *ns2)
>> +{
>> + mutex_lock(&ns2->lock);
>> + if (!ns2->cfg || WARN_ON(ns2 != ns2->cfg->parent)) {
>> + mutex_unlock(&ns2->lock);
>> + return;
>> + }
>> + ns2->cfg->parent = NULL;
>> + ns2->cfg = NULL;
>> + mutex_unlock(&ns2->lock);
>> + switch2_controller_put(ns2);
>> + kref_put(&ns2->refcount, switch2_kref_put);
>> +}
>> +EXPORT_SYMBOL_GPL(switch2_controller_detach_cfg);
>> +
>> +static int switch2_probe(struct hid_device *hdev, const struct hid_device_id *id)
>> +{
>> + struct switch2_controller *ns2;
>> + struct usb_device *udev;
>> + char phys[64];
>> + int ret;
>> +
>> + if (!hid_is_usb(hdev))
>> + return -ENODEV;
>> +
>> + udev = hid_to_usb_dev(hdev);
>> + if (usb_make_path(udev, phys, sizeof(phys)) < 0)
>> + return -EINVAL;
>> +
>> + ret = hid_parse(hdev);
>> + if (ret) {
>> + hid_err(hdev, "parse failed %d\n", ret);
>> + return ret;
>> + }
>> +
>> + ns2 = switch2_get_controller(phys);
>> + if (IS_ERR(ns2))
>> + return PTR_ERR(ns2);
>> +
>> + mutex_lock(&ns2->lock);
>> + if (ns2->hdev) {
>> + mutex_unlock(&ns2->lock);
>> + hid_err(hdev,
>> + "Second hdev tried to claim same controller, first=%p vs second=%p\n",
>> + ns2->hdev, hdev);
>> + kref_put(&ns2->refcount, switch2_kref_put);
>> + return -EBUSY;
>> + }
>> + ns2->hdev = hdev;
>> + hid_set_drvdata(hdev, ns2);
>> +
>> + switch (hdev->product | (hdev->vendor << 16)) {
>> + default:
>> + strscpy(ns2->name, hdev->name, sizeof(ns2->name));
>> + break;
>> + /* Some controllers have slightly wrong names so we override them */
>> + case USB_DEVICE_ID_NINTENDO_NS2_JOYCONR | (USB_VENDOR_ID_NINTENDO << 16):
>> + /* Missing the "2" in the name */
>> + strscpy(ns2->name, "Nintendo Joy-Con 2 (R)", sizeof(ns2->name));
>> + break;
>> + case USB_DEVICE_ID_NINTENDO_NS2_GCCON | (USB_VENDOR_ID_NINTENDO << 16):
>> + /* Has "Nintendo" in the name twice */
>> + strscpy(ns2->name, "Nintendo GameCube Controller", sizeof(ns2->name));
>> + break;
>> + }
>> +
>> + ns2->player_id = U32_MAX;
>> + ret = ida_alloc(&nintendo_player_id_allocator, GFP_KERNEL);
>> + if (ret < 0)
>> + hid_warn(hdev, "Failed to allocate player ID, skipping; ret=%d\n", ret);
>> + else
>> + ns2->player_id = ret;
>> +
>> + ret = hid_hw_start(hdev, HID_CONNECT_HIDRAW);
>> + if (ret) {
>> + hid_err(hdev, "hw_start failed %d\n", ret);
>> + goto err_cleanup;
>> + }
>> +
>> + ret = hid_hw_open(hdev);
>> + if (ret) {
>> + hid_err(hdev, "hw_open failed %d\n", ret);
>> + goto err_stop;
>> + }
>> +
>> + ret = 0;
>> + if (ns2->cfg)
>> + ret = switch2_init_controller(ns2);
>> +
>> + if (!ret) {
>> + mutex_unlock(&ns2->lock);
>> + return 0;
>> + }
>> +
>> + hid_hw_close(hdev);
>> +err_stop:
>> + hid_hw_stop(hdev);
>> +err_cleanup:
>> + ida_free(&nintendo_player_id_allocator, ns2->player_id);
>> + ns2->hdev = NULL;
>> + mutex_unlock(&ns2->lock);
>> + switch2_controller_put(ns2);
>> + kref_put(&ns2->refcount, switch2_kref_put);
>> +
>> + return ret;
>> +}
>> +
>> +static void switch2_remove(struct hid_device *hdev)
>> +{
>> + struct switch2_controller *ns2 = hid_get_drvdata(hdev);
>> +
>> + switch2_controller_put(ns2);
>> + mutex_lock(&ns2->lock);
>> + ns2->hdev = NULL;
>> + ida_free(&nintendo_player_id_allocator, ns2->player_id);
>> + mutex_unlock(&ns2->lock);
>> + kref_put(&ns2->refcount, switch2_kref_put);
>> + hid_hw_close(hdev);
>> + hid_hw_stop(hdev);
>> +}
>> +
>> static const struct hid_device_id nintendo_hid_devices[] = {
>> + /* Switch devices */
>> { HID_USB_DEVICE(USB_VENDOR_ID_NINTENDO,
>> USB_DEVICE_ID_NINTENDO_PROCON) },
>> { HID_USB_DEVICE(USB_VENDOR_ID_NINTENDO,
>> @@ -2863,10 +4071,67 @@ static const struct hid_device_id nintendo_hid_devices[] = {
>> USB_DEVICE_ID_NINTENDO_N64CON) },
>> { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_HORI,
>> USB_DEVICE_ID_HORI_WIRELESS_SWITCH_PAD) },
>> + /* Switch 2 devices */
>> + { HID_USB_DEVICE(USB_VENDOR_ID_NINTENDO,
>> + USB_DEVICE_ID_NINTENDO_NS2_JOYCONL) },
>> + { HID_USB_DEVICE(USB_VENDOR_ID_NINTENDO,
>> + USB_DEVICE_ID_NINTENDO_NS2_JOYCONR) },
>> + { HID_USB_DEVICE(USB_VENDOR_ID_NINTENDO,
>> + USB_DEVICE_ID_NINTENDO_NS2_PROCON) },
>> + { HID_USB_DEVICE(USB_VENDOR_ID_NINTENDO,
>> + USB_DEVICE_ID_NINTENDO_NS2_GCCON) },
>> { }
>> };
>> MODULE_DEVICE_TABLE(hid, nintendo_hid_devices);
>>
>> +static bool nintendo_is_switch2(struct hid_device *hdev)
>> +{
>> + return hdev->vendor == USB_VENDOR_ID_NINTENDO &&
>> + hdev->product >= USB_DEVICE_ID_NINTENDO_NS2_JOYCONR;
>> +}
>> +
>> +static void nintendo_hid_remove(struct hid_device *hdev)
>> +{
>> + if (nintendo_is_switch2(hdev))
>> + switch2_remove(hdev);
>> + else
>> + joycon_remove(hdev);
>> +}
>> +
>> +static int nintendo_hid_event(struct hid_device *hdev,
>> + struct hid_report *report, u8 *raw_data, int size)
>> +{
>> + if (nintendo_is_switch2(hdev))
>> + return switch2_event(hdev, report, raw_data, size);
>> + else
>> + return joycon_event(hdev, report, raw_data, size);
>> +}
>> +
>> +static int nintendo_hid_probe(struct hid_device *hdev,
>> + const struct hid_device_id *id)
>> +{
>> + if (nintendo_is_switch2(hdev))
>> + return switch2_probe(hdev, id);
>> + else
>> + return joycon_probe(hdev, id);
>> +}
>> +
>> +static int nintendo_hid_resume(struct hid_device *hdev)
>> +{
>> + if (nintendo_is_switch2(hdev))
>> + return 0;
>> + else
>> + return joycon_resume(hdev);
>> +}
>> +
>> +static int nintendo_hid_suspend(struct hid_device *hdev, pm_message_t message)
>> +{
>> + if (nintendo_is_switch2(hdev))
>> + return 0;
>> + else
>> + return joycon_suspend(hdev, message);
>> +}
>> +
>> static struct hid_driver nintendo_hid_driver = {
>> .name = "nintendo",
>> .id_table = nintendo_hid_devices,
>> @@ -2894,4 +4159,5 @@ MODULE_LICENSE("GPL");
>> MODULE_AUTHOR("Ryan McClelland <rymcclel@gmail.com>");
>> MODULE_AUTHOR("Emily Strickland <linux@emily.st>");
>> MODULE_AUTHOR("Daniel J. Ogorchock <djogorchock@gmail.com>");
>> +MODULE_AUTHOR("Vicki Pfau <vi@endrift.com>");
>> MODULE_DESCRIPTION("Driver for Nintendo Switch Controllers");
>> diff --git a/drivers/hid/hid-nintendo.h b/drivers/hid/hid-nintendo.h
>> new file mode 100644
>> index 000000000000..7aff22f30266
>> --- /dev/null
>> +++ b/drivers/hid/hid-nintendo.h
>> @@ -0,0 +1,72 @@
>> +/* SPDX-License-Identifier: GPL-2.0+ */
>> +/*
>> + * HID driver for Nintendo Switch 2 controllers
>> + *
>> + * Copyright (c) 2025 Valve Software
>> + *
>> + * This driver is based on the following work:
>> + * https://gist.github.com/shinyquagsire23/66f006b46c56216acbaac6c1e2279b64
>> + * https://github.com/ndeadly/switch2_controller_research
>> + */
>> +
>> +#ifndef __HID_NINTENDO_H
>> +#define __HID_NINTENDO_H
>> +
>> +#include <linux/bits.h>
>> +
>> +#define NS2_FLAG_OK BIT(0)
>> +#define NS2_FLAG_NACK BIT(2)
>> +
>> +enum switch2_cmd {
>> + NS2_CMD_NFC = 0x01,
>> + NS2_CMD_FLASH = 0x02,
>> + NS2_CMD_INIT = 0x03,
>> + NS2_CMD_GRIP = 0x08,
>> + NS2_CMD_LED = 0x09,
>> + NS2_CMD_VIBRATE = 0x0a,
>> + NS2_CMD_BATTERY = 0x0b,
>> + NS2_CMD_FEATSEL = 0x0c,
>> + NS2_CMD_FW_UPD = 0x0d,
>> + NS2_CMD_FW_INFO = 0x10,
>> + NS2_CMD_BT_PAIR = 0x15,
>> +};
>> +
>> +enum switch2_direction {
>> + NS2_DIR_IN = 0x00,
>> + NS2_DIR_OUT = 0x90,
>> +};
>> +
>> +enum switch2_transport {
>> + NS2_TRANS_USB = 0x00,
>> + NS2_TRANS_BT = 0x01,
>> +};
>> +
>> +struct switch2_cmd_header {
>> + uint8_t command;
>> + uint8_t flags;
>> + uint8_t transport;
>> + uint8_t subcommand;
>> + uint8_t unk1;
>> + uint8_t length;
>> + uint16_t unk2;
>> +};
>> +static_assert(sizeof(struct switch2_cmd_header) == 8);
>> +
>> +struct device;
>> +struct switch2_controller;
>> +struct switch2_cfg_intf {
>> + struct switch2_controller *parent;
>> + struct device *dev;
>> +
>> + int (*send_command)(enum switch2_cmd command, uint8_t subcommand,
>> + const void *message, size_t length,
>> + struct switch2_cfg_intf *intf);
>> +};
>> +
>> +int switch2_controller_attach_cfg(const char *phys, struct switch2_cfg_intf *cfg);
>> +void switch2_controller_detach_cfg(struct switch2_controller *controller);
>> +
>> +int switch2_receive_command(struct switch2_controller *controller,
>> + const uint8_t *message, size_t length);
>> +
>> +#endif
>> diff --git a/drivers/input/joystick/Kconfig b/drivers/input/joystick/Kconfig
>> index 7755e5b454d2..868262c6ccd9 100644
>> --- a/drivers/input/joystick/Kconfig
>> +++ b/drivers/input/joystick/Kconfig
>> @@ -422,4 +422,15 @@ config JOYSTICK_SEESAW
>> To compile this driver as a module, choose M here: the module will be
>> called adafruit-seesaw.
>>
>> +config JOYSTICK_NINTENDO_SWITCH2_USB
>> + tristate "Wired Nintendo Switch 2 controller support"
>> + depends on HID_NINTENDO
>> + depends on USB
>> + help
>> + Say Y here if you want to enable support for wired Nintendo Switch 2
>> + controllers.
>> +
>> + To compile this driver as a module, choose M here: the
>> + module will be called nintendo-switch2-usb.
>> +
>> endif
>> diff --git a/drivers/input/joystick/Makefile b/drivers/input/joystick/Makefile
>> index 9976f596a920..8f92900ae885 100644
>> --- a/drivers/input/joystick/Makefile
>> +++ b/drivers/input/joystick/Makefile
>> @@ -34,6 +34,7 @@ obj-$(CONFIG_JOYSTICK_SIDEWINDER) += sidewinder.o
>> obj-$(CONFIG_JOYSTICK_SPACEBALL) += spaceball.o
>> obj-$(CONFIG_JOYSTICK_SPACEORB) += spaceorb.o
>> obj-$(CONFIG_JOYSTICK_STINGER) += stinger.o
>> +obj-$(CONFIG_JOYSTICK_NINTENDO_SWITCH2_USB) += nintendo-switch2-usb.o
>> obj-$(CONFIG_JOYSTICK_TMDC) += tmdc.o
>> obj-$(CONFIG_JOYSTICK_TURBOGRAFX) += turbografx.o
>> obj-$(CONFIG_JOYSTICK_TWIDJOY) += twidjoy.o
>> diff --git a/drivers/input/joystick/nintendo-switch2-usb.c b/drivers/input/joystick/nintendo-switch2-usb.c
>> new file mode 100644
>> index 000000000000..a6999a0a26ae
>> --- /dev/null
>> +++ b/drivers/input/joystick/nintendo-switch2-usb.c
>> @@ -0,0 +1,468 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +/*
>> + * USB driver for Nintendo Switch 2 controllers configuration interface
>> + *
>> + * Copyright (c) 2025 Valve Software
>> + *
>> + * This driver is based on the following work:
>> + * https://gist.github.com/shinyquagsire23/66f006b46c56216acbaac6c1e2279b64
>> + * https://github.com/ndeadly/switch2_controller_research
>> + */
>> +
>> +#include "../../hid/hid-ids.h"
>> +#include "../../hid/hid-nintendo.h"
>> +#include <linux/module.h>
>> +#include <linux/usb/input.h>
>> +
>> +#define NS2_BULK_SIZE 64
>> +#define NS2_IN_URBS 2
>> +#define NS2_OUT_URBS 4
>> +
>> +static struct usb_driver switch2_usb;
>> +
>> +enum switch2_urb_state {
>> + NS2_URB_FREE,
>> + NS2_URB_OUT,
>> + NS2_URB_IN,
>> +};
>> +
>> +struct switch2_urb {
>> + struct urb *urb;
>> + uint8_t *data;
>> + enum switch2_urb_state state;
>> +};
>> +
>> +struct switch2_usb {
>> + struct switch2_cfg_intf cfg;
>> + struct usb_device *udev;
>> +
>> + struct switch2_urb bulk_in[NS2_IN_URBS];
>> + struct usb_anchor bulk_in_anchor;
>> + bool shutdown;
>> + spinlock_t bulk_in_lock;
>> +
>> + struct switch2_urb bulk_out[NS2_OUT_URBS];
>> + struct usb_anchor bulk_out_anchor;
>> + spinlock_t bulk_out_lock;
>> +
>> + struct work_struct message_in_work;
>> +};
>> +
>> +static void switch2_bulk_in(struct urb *urb)
>> +{
>> + struct switch2_usb *ns2_usb = urb->context;
>> + int i;
>> + bool schedule = false;
>> + unsigned long flags;
>> +
>> + switch (urb->status) {
>> + case 0:
>> + schedule = true;
>> + break;
>> + case -ECONNRESET:
>> + case -ENOENT:
>> + case -ESHUTDOWN:
>> + case -EPIPE:
>> + break;
>> + default:
>> + dev_dbg(&ns2_usb->udev->dev, "unknown input urb status: %d\n", urb->status);
>> + break;
>> + }
>> +
>> + 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) {
>> + if (schedule) {
>> + ns2_usb->bulk_in[i].state = NS2_URB_IN;
>> + continue;
>> + } else {
>
> nit: this else is redundant as we are continue-ing above anyways.
Below you mean? I guess technically, yeah.
>
>> + ns2_usb->bulk_in[i].state = NS2_URB_FREE;
>> + }
>> + }
>> +
>> + if (ns2_usb->bulk_in[i].state != NS2_URB_FREE)
>> + continue;
>> +
>> + /*
>> + * We want exactly one bulk in URB scheduled at a time, so only
>> + * reschedule this immediately if nothing else is scheduled
>> + * currently.
>> + */
>> + if (!usb_anchor_empty(&ns2_usb->bulk_in_anchor) || ns2_usb->shutdown)
>> + continue;
>> +
>> + ns2_urb = &ns2_usb->bulk_in[i];
>> + if (!ns2_urb)
>
> AFAICT ns2_urb will never be NULL here so we can remove this check.
This is kind of a result of dealing with sashiko's obsession with hypothetical concurrency bugs. I would have to think way too hard to figure out if it's conceivably possible compared to whatever sashiko spits out so if I can get it to shut up it's worth it to me. Worst case scenario it will never trigger, so it's harmless.
>
>> + continue;
>> +
>> + usb_anchor_urb(ns2_urb->urb, &ns2_usb->bulk_in_anchor);
>> + err = usb_submit_urb(ns2_urb->urb, GFP_ATOMIC);
>> + if (err) {
>> + usb_unanchor_urb(ns2_urb->urb);
>> + dev_dbg(&ns2_usb->udev->dev, "failed to queue input urb: %d\n", err);
>> + } else {
>> + ns2_urb->state = NS2_URB_OUT;
>> + }
>> + }
>> + spin_unlock_irqrestore(&ns2_usb->bulk_in_lock, flags);
>> +
>> + if (schedule)
>> + schedule_work(&ns2_usb->message_in_work);
>> +}
>> +
>> +static void switch2_bulk_out(struct urb *urb)
>> +{
>> + struct switch2_usb *ns2_usb = urb->context;
>> + int i;
>> +
>> + guard(spinlock_irqsave)(&ns2_usb->bulk_out_lock);
>> +
>> + switch (urb->status) {
>> + case 0:
>> + break;
>> + case -ECONNRESET:
>> + case -ENOENT:
>> + case -ESHUTDOWN:
>> + case -EPIPE:
>> + break;
>> + default:
>> + dev_dbg(&ns2_usb->udev->dev, "unknown output urb status: %d\n", urb->status);
>> + break;
>> + }
>> +
>> + for (i = 0; i < NS2_OUT_URBS; i++) {
>> + if (ns2_usb->bulk_out[i].urb != urb)
>> + continue;
>> +
>> + ns2_usb->bulk_out[i].state = NS2_URB_FREE;
>> + break;
>> + }
>> +}
>> +
>> +static int switch2_usb_send_cmd(enum switch2_cmd command, uint8_t subcommand,
>> + const void *message, size_t size, struct switch2_cfg_intf *cfg)
>> +{
>> + struct switch2_usb *ns2_usb = (struct switch2_usb *)cfg;
>> + struct switch2_urb *urb = NULL;
>> + int i;
>> + int ret;
>> + unsigned long flags;
>> +
>> + struct switch2_cmd_header header = {
>> + command, NS2_DIR_OUT | NS2_FLAG_OK, NS2_TRANS_USB, subcommand, 0, size
>> + };
>> +
>> + if (WARN_ON(size > 56))
>> + return -EINVAL;
>> +
>> + spin_lock_irqsave(&ns2_usb->bulk_out_lock, flags);
>> + for (i = 0; i < NS2_OUT_URBS; i++) {
>> + if (ns2_usb->bulk_out[i].state != NS2_URB_FREE)
>> + continue;
>> +
>> + urb = &ns2_usb->bulk_out[i];
>> + urb->state = NS2_URB_OUT;
>> + break;
>> + }
>> + spin_unlock_irqrestore(&ns2_usb->bulk_out_lock, flags);
>> +
>> + if (!urb) {
>> + dev_warn(&ns2_usb->udev->dev, "output queue full, dropping message\n");
>> + return -ENOBUFS;
>> + }
>> +
>> + memcpy(urb->data, &header, sizeof(header));
>> + if (message && size)
>> + memcpy(&urb->data[8], message, size);
>> + urb->urb->transfer_buffer_length = size + sizeof(header);
>> +
>> + print_hex_dump_debug("sending cmd: ", DUMP_PREFIX_OFFSET, 16, 1, urb->data,
>> + size + sizeof(header), false);
>> +
>> + usb_anchor_urb(urb->urb, &ns2_usb->bulk_out_anchor);
>> + ret = usb_submit_urb(urb->urb, GFP_KERNEL);
>> + if (ret) {
>> + if (ret != -ENODEV)
>> + dev_warn(&ns2_usb->udev->dev, "failed to submit output urb: %i", ret);
>> + spin_lock_irqsave(&ns2_usb->bulk_out_lock, flags);
>> + urb->state = NS2_URB_FREE;
>> + spin_unlock_irqrestore(&ns2_usb->bulk_out_lock, flags);
>> + usb_unanchor_urb(urb->urb);
>> + return ret;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static void switch2_usb_message_in_work(struct work_struct *work)
>> +{
>> + struct switch2_usb *ns2_usb = container_of(work, struct switch2_usb, message_in_work);
>> + struct switch2_urb *urb;
>> + int err;
>> + int i;
>> + unsigned long flags;
>> +
>> + spin_lock_irqsave(&ns2_usb->bulk_in_lock, flags);
>> + for (i = 0; i < NS2_IN_URBS; i++) {
>> + urb = &ns2_usb->bulk_in[i];
>> + if (urb->state != NS2_URB_IN)
>> + continue;
>> + spin_unlock_irqrestore(&ns2_usb->bulk_in_lock, flags);
>> +
>> + if (ns2_usb->cfg.parent) {
>> + err = switch2_receive_command(ns2_usb->cfg.parent,
>> + urb->urb->transfer_buffer, urb->urb->actual_length);
>> + if (err)
>> + dev_dbg(&ns2_usb->udev->dev, "receive command failed: %d\n", err);
>> + } else {
>> + dev_err(&ns2_usb->udev->dev,
>> + "Got message before controller is fully set up; discarding\n");
>> + }
>> +
>> + spin_lock_irqsave(&ns2_usb->bulk_in_lock, flags);
>> + urb->state = NS2_URB_FREE;
>> + /*
>> + * We want exactly one bulk in URB scheduled at a time, so only
>> + * reschedule this immediately if nothing else is scheduled
>> + * currently.
>> + */
>> + if (!usb_anchor_empty(&ns2_usb->bulk_in_anchor) || ns2_usb->shutdown)
>> + continue;
>> +
>> + usb_anchor_urb(urb->urb, &ns2_usb->bulk_in_anchor);
>> + err = usb_submit_urb(urb->urb, GFP_ATOMIC);
>> + if (err) {
>> + usb_unanchor_urb(urb->urb);
>> + dev_dbg(&ns2_usb->udev->dev,
>> + "failed to queue input urb: %d\n", err);
>> + } else {
>> + urb->state = NS2_URB_OUT;
>> + }
>> + }
>> + spin_unlock_irqrestore(&ns2_usb->bulk_in_lock, flags);
>> +}
>> +
>> +static int switch2_usb_probe(struct usb_interface *intf, const struct usb_device_id *id)
>> +{
>> + struct switch2_usb *ns2_usb;
>> + struct usb_device *udev;
>> + struct usb_endpoint_descriptor *bulk_in, *bulk_out;
>> + struct urb *urb;
>> + uint8_t *data;
>> + char phys[64];
>> + int ret;
>> + int i;
>> + unsigned long flags;
>> +
>> + udev = interface_to_usbdev(intf);
>> + if (usb_make_path(udev, phys, sizeof(phys)) < 0)
>> + return -EINVAL;
>> +
>> + ret = usb_find_common_endpoints(intf->cur_altsetting, &bulk_in, &bulk_out, NULL, NULL);
>> + if (ret) {
>> + dev_err(&intf->dev, "failed to find bulk EPs\n");
>> + return ret;
>> + }
>> +
>> + ns2_usb = devm_kzalloc(&intf->dev, sizeof(*ns2_usb), GFP_KERNEL);
>> + if (!ns2_usb)
>> + return -ENOMEM;
>> +
>> + 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);
>> +
>> + ns2_usb->udev = udev;
>> + for (i = 0; i < NS2_IN_URBS; i++) {
>> + urb = usb_alloc_urb(0, GFP_KERNEL);
>> + if (!urb) {
>> + ret = -ENOMEM;
>> + goto err_free_in;
>> + }
>> +
>> + data = usb_alloc_coherent(udev, NS2_BULK_SIZE, GFP_KERNEL,
>> + &urb->transfer_dma);
>> + if (!data) {
>> + usb_free_urb(urb);
>> + ret = -ENOMEM;
>> + goto err_free_in;
>> + }
>> +
>> + spin_lock_irqsave(&ns2_usb->bulk_in_lock, flags);
>> + usb_fill_bulk_urb(urb, udev,
>> + usb_rcvbulkpipe(udev, bulk_in->bEndpointAddress),
>> + data, NS2_BULK_SIZE, switch2_bulk_in, ns2_usb);
>> + urb->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
>> +
>> + ns2_usb->bulk_in[i].urb = urb;
>> + ns2_usb->bulk_in[i].data = data;
>> + spin_unlock_irqrestore(&ns2_usb->bulk_in_lock, flags);
>> + }
>> +
>> + for (i = 0; i < NS2_OUT_URBS; i++) {
>> + urb = usb_alloc_urb(0, GFP_KERNEL);
>> + if (!urb) {
>> + ret = -ENOMEM;
>> + goto err_free_out;
>> + }
>> +
>> + data = usb_alloc_coherent(udev, NS2_BULK_SIZE, GFP_KERNEL,
>> + &urb->transfer_dma);
>> + if (!data) {
>> + usb_free_urb(urb);
>> + ret = -ENOMEM;
>> + goto err_free_out;
>> + }
>> +
>> + spin_lock_irqsave(&ns2_usb->bulk_out_lock, flags);
>> + usb_fill_bulk_urb(urb, udev,
>> + usb_sndbulkpipe(udev, bulk_out->bEndpointAddress),
>> + data, NS2_BULK_SIZE, switch2_bulk_out, ns2_usb);
>> + urb->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
>> +
>> + ns2_usb->bulk_out[i].urb = urb;
>> + ns2_usb->bulk_out[i].data = data;
>> + spin_unlock_irqrestore(&ns2_usb->bulk_out_lock, flags);
>> + }
>> +
>> + usb_set_intfdata(intf, ns2_usb);
>> +
>> + ns2_usb->cfg.dev = &ns2_usb->udev->dev;
>> + ns2_usb->cfg.send_command = switch2_usb_send_cmd;
>> +
>> + spin_lock_irqsave(&ns2_usb->bulk_in_lock, flags);
>> + ns2_usb->bulk_in[0].state = NS2_URB_OUT;
>> + usb_anchor_urb(ns2_usb->bulk_in[0].urb, &ns2_usb->bulk_in_anchor);
>> + ret = usb_submit_urb(ns2_usb->bulk_in[0].urb, GFP_ATOMIC);
>> + spin_unlock_irqrestore(&ns2_usb->bulk_in_lock, flags);
>> +
>> + if (ret < 0)
>> + goto err_free_out;
>> +
>> + ret = switch2_controller_attach_cfg(phys, &ns2_usb->cfg);
>> + if (ret < 0)
>> + goto err_free_out;
>> +
>> + return 0;
>> +
>> +err_free_out:
>> + usb_kill_anchored_urbs(&ns2_usb->bulk_out_anchor);
>> + for (i = 0; i < NS2_OUT_URBS; i++) {
>> + spin_lock_irqsave(&ns2_usb->bulk_out_lock, flags);
>> + urb = ns2_usb->bulk_out[i].urb;
>> + data = ns2_usb->bulk_out[i].data;
>> + if (!urb) {
>> + spin_unlock_irqrestore(&ns2_usb->bulk_out_lock, flags);
>> + continue;
>> + }
>> +
>> + ns2_usb->bulk_out[i].urb = NULL;
>> + ns2_usb->bulk_out[i].data = NULL;
>> + spin_unlock_irqrestore(&ns2_usb->bulk_out_lock, flags);
>> +
>> + usb_free_coherent(ns2_usb->udev, NS2_BULK_SIZE, data, urb->transfer_dma);
>> + usb_free_urb(urb);
>> + }
>> +err_free_in:
>> + spin_lock_irqsave(&ns2_usb->bulk_in_lock, flags);
>> + ns2_usb->shutdown = true;
>> + spin_unlock_irqrestore(&ns2_usb->bulk_in_lock, flags);
>> +
>> + usb_kill_anchored_urbs(&ns2_usb->bulk_in_anchor);
>> + cancel_work_sync(&ns2_usb->message_in_work);
>> + for (i = 0; i < NS2_IN_URBS; i++) {
>> + spin_lock_irqsave(&ns2_usb->bulk_in_lock, flags);
>> + urb = ns2_usb->bulk_in[i].urb;
>> + data = ns2_usb->bulk_in[i].data;
>> + if (!urb) {
>> + spin_unlock_irqrestore(&ns2_usb->bulk_in_lock, flags);
>> + continue;
>> + }
>> +
>> + ns2_usb->bulk_in[i].urb = NULL;
>> + ns2_usb->bulk_in[i].data = NULL;
>> + spin_unlock_irqrestore(&ns2_usb->bulk_in_lock, flags);
>> +
>> + usb_free_coherent(ns2_usb->udev, NS2_BULK_SIZE, data, urb->transfer_dma);
>> + usb_free_urb(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;
>> + struct urb *urb;
>> + uint8_t *data;
>> + int i;
>> +
>> + /* Prevent any further IN URBs from being scheduled */
>> + spin_lock_irqsave(&ns2_usb->bulk_in_lock, flags);
>> + ns2_usb->shutdown = true;
>> + spin_unlock_irqrestore(&ns2_usb->bulk_in_lock, flags);
>> +
>> + usb_kill_anchored_urbs(&ns2_usb->bulk_in_anchor);
>> + cancel_work_sync(&ns2_usb->message_in_work);
>> + for (i = 0; i < NS2_IN_URBS; i++) {
>> + spin_lock_irqsave(&ns2_usb->bulk_in_lock, flags);
>> + urb = ns2_usb->bulk_in[i].urb;
>
> In other clean-up cases we are checking this to be NULL. We might have
> to do so here as well unless we know that it will never be NULL on
> switch2_usb_disconnect (otherwise we will have a NULL pointer deref a
> few lines down).
This is the only place where it can become NULL and it's not called more than once. In the cleanup cases in probe it's because they hadn't become not NULL yet, but if they do they will never become NULL again until here. We only go through this list once (it's freed after this returns due to devres allocation), so it's not an issue.
>
>> + data = ns2_usb->bulk_in[i].data;
>> + ns2_usb->bulk_in[i].urb = NULL;
>> + ns2_usb->bulk_in[i].data = NULL;
>> + spin_unlock_irqrestore(&ns2_usb->bulk_in_lock, flags);
>> +
>> + usb_free_coherent(ns2_usb->udev, NS2_BULK_SIZE, data, urb->transfer_dma);
>> + usb_free_urb(urb);
>> + }
>> +
>> + /*
>> + * We need to detach *before* we kill the out URBs to make sure no
>> + * further URBs get scheduled by the HID endpoint in the meantime.
>> + */
>> + switch2_controller_detach_cfg(ns2_usb->cfg.parent);
>> +
>> + usb_kill_anchored_urbs(&ns2_usb->bulk_out_anchor);
>> + for (i = 0; i < NS2_OUT_URBS; i++) {
>> + spin_lock_irqsave(&ns2_usb->bulk_out_lock, flags);
>> + urb = ns2_usb->bulk_out[i].urb;
>
> See above.
See above.
>
>> + data = ns2_usb->bulk_out[i].data;
>> + ns2_usb->bulk_out[i].urb = NULL;
>> + ns2_usb->bulk_out[i].data = NULL;
>> + spin_unlock_irqrestore(&ns2_usb->bulk_out_lock, flags);
>> +
>> + usb_free_coherent(ns2_usb->udev, NS2_BULK_SIZE, data, urb->transfer_dma);
>> + usb_free_urb(urb);
>> + }
>> +}
>> +
>> +#define SWITCH2_CONTROLLER(vend, prod) \
>> + USB_DEVICE_AND_INTERFACE_INFO(vend, prod, USB_CLASS_VENDOR_SPEC, 0, 0)
>> +
>> +static const struct usb_device_id switch2_usb_devices[] = {
>> + { SWITCH2_CONTROLLER(USB_VENDOR_ID_NINTENDO, USB_DEVICE_ID_NINTENDO_NS2_JOYCONL) },
>> + { SWITCH2_CONTROLLER(USB_VENDOR_ID_NINTENDO, USB_DEVICE_ID_NINTENDO_NS2_JOYCONR) },
>> + { SWITCH2_CONTROLLER(USB_VENDOR_ID_NINTENDO, USB_DEVICE_ID_NINTENDO_NS2_PROCON) },
>> + { SWITCH2_CONTROLLER(USB_VENDOR_ID_NINTENDO, USB_DEVICE_ID_NINTENDO_NS2_GCCON) },
>> + { }
>> +};
>> +MODULE_DEVICE_TABLE(usb, switch2_usb_devices);
>> +
>> +static struct usb_driver switch2_usb = {
>> + .name = "nintendo-switch2",
>> + .id_table = switch2_usb_devices,
>> + .probe = switch2_usb_probe,
>> + .disconnect = switch2_usb_disconnect,
>> +};
>> +module_usb_driver(switch2_usb);
>> +
>> +MODULE_LICENSE("GPL");
>> +MODULE_AUTHOR("Vicki Pfau <vi@endrift.com>");
>> +MODULE_DESCRIPTION("Driver for Nintendo Switch 2 Controllers");
>
> Thanks for all the work on this!
>
> Cheers,
> Silvan
Vicki
^ permalink raw reply
* Re: [PATCH v2 06/10] HID: steam: Fully unregister controller when hidraw is opened
From: sashiko-bot @ 2026-07-07 23:52 UTC (permalink / raw)
To: Vicki Pfau; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260707233241.2644971-7-vi@endrift.com>
Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
New issues:
- [Medium] Memory leak of devm-allocated battery name if power_supply_register() fails.
Pre-existing issues:
- [High] Race condition in steam_remove leading to Use-After-Free of steam_device.
- [High] List corruption and Memory Leak when steam_register fails during probe.
- [High] Race condition in hidraw open/close callbacks leading to Use-After-Free.
- [High] TOCTOU race in steam_raw_event accessing steam->client_hdev during device removal.
--
--- Patch [6]: [PATCH v2 06/10] HID: steam: Fully unregister controller when hidraw is opened ---
commit d361e686d46a1745e3d9d699aff5bb87aea155b9
Author: Vicki Pfau <vi@endrift.com>
HID: steam: Fully unregister controller when hidraw is opened
This patch changes the driver to fully unregister the controller internally
when hidraw is opened, rather than just detaching the evdev nodes. This
also unifies the unregister and connect callbacks.
> diff --git a/drivers/hid/hid-steam.c b/drivers/hid/hid-steam.c
> index 5537488139011..3f1dbdcad23c8 100644
> --- a/drivers/hid/hid-steam.c
> +++ b/drivers/hid/hid-steam.c
[ ... ]
> @@ -1077,6 +1076,7 @@ static void steam_battery_unregister(struct steam_device *steam)
> RCU_INIT_POINTER(steam->battery, NULL);
> synchronize_rcu();
> power_supply_unregister(battery);
> + devm_kfree(&steam->hdev->dev, steam->battery_desc.name);
> }
[Severity: Medium]
If power_supply_register() fails in steam_battery_register(), steam->battery
is not set. When this unregister function is called, an early return on
!battery could skip this newly added devm_kfree():
steam_battery_unregister()
{
...
if (!battery)
return;
...
devm_kfree(&steam->hdev->dev, steam->battery_desc.name);
}
Since this unregister function is now dynamically triggered every time the
hidraw node is opened, could an unprivileged user repeatedly open and close
the hidraw node to continuously leak the devm-allocated string?
[ ... ]
> @@ -1277,7 +1255,7 @@ static int steam_client_ll_open(struct hid_device *hdev)
> steam->client_opened++;
> spin_unlock_irqrestore(&steam->lock, flags);
>
> - schedule_work(&steam->unregister_work);
> + schedule_work(&steam->work_connect);
>
> return 0;
> }
[Severity: High]
This isn't a bug introduced by this patch, but releasing the lock here before
scheduling work appears to open a race window:
steam_client_ll_open()
{
...
spin_unlock_irqrestore(&steam->lock, flags);
schedule_work(&steam->work_connect);
...
}
If the thread is preempted immediately after releasing the lock, and the
physical device is removed concurrently, steam_remove() could execute, free
the steam structure, and leave the resumed schedule_work() call to access
freed memory. Is there a way to safely synchronize this against device
removal?
[ ... ]
> @@ -1389,7 +1367,6 @@ static int steam_probe(struct hid_device *hdev,
> steam->sensor_update_rate_us = 4000;
> else
> steam->sensor_update_rate_us = 9000;
> - INIT_WORK(&steam->unregister_work, steam_work_unregister_cb);
[Severity: High]
This is a pre-existing issue, but I wanted to ask about the error handling
path in steam_probe() since it is closely related to driver registration:
steam_probe()
{
...
ret = steam_register(steam);
if (ret) {
hid_err(hdev, ...);
goto err_hw_close;
}
...
}
If steam_register() adds the device to the steam_devices list, but a
subsequent step like steam_input_register() fails, steam_register() returns
an error but leaves the device on the global list.
Since the err_hw_close path bypasses unregistering, could this lead to a
use-after-free when the lizard_mode module parameter later traverses the
steam_devices list?
[ ... ]
> @@ -1470,7 +1446,6 @@ static void steam_remove(struct hid_device *hdev)
> cancel_work_sync(&steam->work_connect);
> cancel_work_sync(&steam->rumble_work);
> cancel_delayed_work_sync(&steam->coalesce_rumble_work);
> - cancel_work_sync(&steam->unregister_work);
> steam->client_hdev = NULL;
> steam->client_opened = 0;
[Severity: High]
This is a pre-existing issue, but does calling cancel_work_sync() here before
stopping the hardware leave a race window?
steam_remove()
{
...
cancel_work_sync(&steam->work_connect);
...
} // hid_hw_stop() is called later
Because the HID hardware is still active, could an incoming connection event
interrupt trigger steam_raw_event() and re-schedule work_connect after it was
canceled?
Once steam_remove() completes and frees the steam object, this might cause the
workqueue to execute the asynchronous callback using the freed steam structure.
[Severity: High]
This is also a pre-existing issue in the raw event handler, but could this
unlocked check lead to a time-of-check to time-of-use race?
steam_raw_event()
{
...
if (steam->client_opened)
hid_input_report(steam->client_hdev, ...);
...
}
If the thread is preempted after reading a non-zero client_opened value, and
the physical device is concurrently unplugged, steam_remove() could run and
destroy client_hdev.
When steam_raw_event() resumes, will it attempt to call hid_input_report()
with the freed steam->client_hdev pointer?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260707233241.2644971-1-vi@endrift.com?part=6
^ permalink raw reply
* Re: [PATCH v2 04/10] HID: steam: Add support for sensor events on the Steam Controller (2015)
From: sashiko-bot @ 2026-07-07 23:50 UTC (permalink / raw)
To: Vicki Pfau; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260707233241.2644971-5-vi@endrift.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] If `steam_register()` fails during probe for a wired device, the `steam` struct is left on the global `steam_devices` list but its memory is freed, leading to a Use-After-Free.
--
--- Patch [4]: [PATCH v2 04/10] HID: steam: Add support for sensor events on the Steam Controller (2015) ---
commit d6e009f462fb9aec26d7157d6389455fb438d8e1
Author: Vicki Pfau <vi@endrift.com>
HID: steam: Add support for sensor events on the Steam Controller (2015)
Sensor support was added for the Steam Deck previously, but Steam Controller
sensor events were never added. This adds that missing support, bringing Steam
Controller support much closer to feature parity with things like SDL and Steam
itself.
> diff --git a/drivers/hid/hid-steam.c b/drivers/hid/hid-steam.c
> index add9f4f27fff9..6029c055a6cf3 100644
> --- a/drivers/hid/hid-steam.c
> +++ b/drivers/hid/hid-steam.c
[ ... ]
> @@ -968,6 +1077,12 @@ static int steam_register(struct steam_device *steam)
> strscpy(steam->serial_no, "XXXXXXXXXX",
> sizeof(steam->serial_no));
>
> + ret = steam_get_attributes(steam);
> + if (ret < 0)
> + hid_err(steam->hdev,
> + "%s:steam_get_attributes failed with error %d\n",
> + __func__, ret);
> +
> hid_info(steam->hdev, "Steam Controller '%s' connected",
> steam->serial_no);
>
[Severity: High]
This is a pre-existing issue, but can the error path in steam_probe() lead to
a use-after-free if steam_register() fails?
In steam_register(), the device is added to the global steam_devices list:
mutex_lock(&steam_devices_lock);
if (list_empty(&steam->list))
list_add(&steam->list, &steam_devices);
mutex_unlock(&steam_devices_lock);
If a subsequent registration step (like steam_input_register() or
steam_sensors_register()) fails, steam_register() returns an error without
removing the device from the list.
Then, back in steam_probe(), if steam_register() fails, it jumps to
err_hw_close, bypassing the unregister cleanup:
err_steam_unregister:
if (steam->connected)
steam_unregister(steam);
err_hw_close:
hid_hw_close(hdev);
Because the steam device was allocated with devm_kzalloc(), devres will
automatically free the structure upon probe failure. Does this leave a dangling
pointer in the global steam_devices list, which could cause memory corruption
when the list is later iterated (e.g., when setting the lizard_mode module
parameter)?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260707233241.2644971-1-vi@endrift.com?part=4
^ permalink raw reply
* Re: [PATCH v2 10/10] HID: steam: Retry send/recv reports if stale
From: sashiko-bot @ 2026-07-07 23:48 UTC (permalink / raw)
To: Vicki Pfau; +Cc: linux-input, dmitry.torokhov
In-Reply-To: <20260707233241.2644971-11-vi@endrift.com>
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] Log spam in expected retry path due to unconditional `hid_err` in `steam_recv_report`.
- [Medium] Out-of-bounds stack read in `hid_dbg` when the device report exceeds the caller's buffer.
- [Medium] `steam_recv_report` reads uninitialized stack memory on short hardware reads.
--
--- Patch [10]: [PATCH v2 10/10] HID: steam: Retry send/recv reports if stale ---
commit 181ac85173536f44d1a324b3bc0042d43fa8238e
Author: Vicki Pfau <vi@endrift.com>
HID: steam: Retry send/recv reports if stale
Sometimes recv report will reply with a stale result from a previous send
report. Instead of failing out, we should retry them, as they generally
reply correctly after three tries, give or take.
> diff --git a/drivers/hid/hid-steam.c b/drivers/hid/hid-steam.c
> index 49f6733d89359..ba2dd40a79763 100644
> --- a/drivers/hid/hid-steam.c
> +++ b/drivers/hid/hid-steam.c
> @@ -497,6 +497,43 @@ static int steam_write_settings(struct steam_device *steam,
> return steam_recv_report(steam, cmd, 2 + cmd[1]);
> }
>
> +static int steam_exchange_report(struct steam_device *steam, u8 *cmd, int csize,
> + u8 *reply, int rsize)
> +{
> + unsigned int retries = 5;
> + int ret;
> +
> + guard(mutex)(&steam->report_mutex);
> + do {
> + ret = steam_send_report(steam, cmd, csize);
> + if (ret < 0)
> + return ret;
> + ret = steam_recv_report(steam, reply, rsize);
[Severity: Medium]
Does this call cause an out-of-bounds stack read in steam_recv_report()?
When a device sends a padded report that is larger than the caller's buffer
(like the 36-byte reply buffer in steam_get_serial()), steam_recv_report()
copies the correct size but logs using the unbounded ret length:
steam_recv_report() {
...
size = min(size, ret);
memcpy(data, buf + 1, size);
...
hid_dbg(steam->hdev, "Received report %*ph\n", ret, data);
...
}
Since ret can be larger than size, won't hid_dbg() read past the end of the
reply buffer on the stack?
[Severity: Medium]
Can this also result in reading uninitialized stack memory on short reads?
If the hardware sends a short report where hid_hw_raw_request() returns
0 or 1, memcpy() copies 0 bytes, leaving the caller's reply buffer
uninitialized.
steam_recv_report() {
...
if (ret < 2 || ret < data[1] + 2) {
hid_err(steam->hdev, "%s: expected %u bytes, read %i\n",
__func__, data[1] + 2, ret);
return -EPROTO;
}
}
When ret < 2, data[1] has not been populated. Won't this read and log an
uninitialized byte from the stack?
> + /*
> + * Sometimes this can fail on the first few tries on the Steam
> + * Controller (2015). It appears to be a firmware bug, and Steam
> + * itself just retries, so we should also retry a few times to
> + * see if we get it.
> + */
> + if (ret == -EPROTO)
[Severity: Medium]
Will this retry logic cause log spam during normal operation?
Because steam_recv_report() unconditionally calls hid_err() before returning
-EPROTO:
steam_recv_report() {
...
if (ret < 2 || ret < data[1] + 2) {
hid_err(steam->hdev, "%s: expected %u bytes, read %i\n",
__func__, data[1] + 2, ret);
return -EPROTO;
}
}
Won't every expected retry here flood the kernel log with error messages?
> + continue;
> + if (ret < 0) {
> + hid_err(steam->hdev, "%s: error reading reply (%*ph)\n",
> + __func__, csize, cmd);
> + return ret;
> + }
> + if (reply[0] == cmd[0] && reply[1] >= 1)
> + break;
> + if (retries > 0)
> + continue;
> + hid_err(steam->hdev, "%s: invalid reply (%*ph)\n", __func__,
> + rsize, reply);
> + return -EPROTO;
> + } while (retries--);
> +
> + return ret;
> +}
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260707233241.2644971-1-vi@endrift.com?part=10
^ permalink raw reply
* Re: [PATCH v2 08/10] HID: steam: Improve logging and other cleanup
From: sashiko-bot @ 2026-07-07 23:47 UTC (permalink / raw)
To: Vicki Pfau; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260707233241.2644971-9-vi@endrift.com>
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
New issues:
- [High] Uninitialized kernel stack memory leaked to dmesg via short reports in steam_get_serial()
- [High] Use-After-Free due to bypassed workqueue cancellation in steam_probe() error paths
Pre-existing issues:
- [High] Use-After-Free via global steam_devices list due to bypassed unregister in probe error paths
--
--- Patch [8]: [PATCH v2 08/10] HID: steam: Improve logging and other cleanup ---
commit 5aa379e88890315b198ed7d8da2c352e3a8ce6a8
Author: Vicki Pfau <vi@endrift.com>
HID: steam: Improve logging and other cleanup
This commit adds more logging where appropriate and reindents an enum to
match the surrounding coding style. It also cleans up some places where
guard() can be used for lock management instead of manually calling locking
and unlocking functions.
> diff --git a/drivers/hid/hid-steam.c b/drivers/hid/hid-steam.c
> index 296ced2989a42..1f2cf15d40831 100644
> --- a/drivers/hid/hid-steam.c
> +++ b/drivers/hid/hid-steam.c
[ ... ]
> @@ -481,22 +490,21 @@ static int steam_get_serial(struct steam_device *steam)
> u8 cmd[] = {ID_GET_STRING_ATTRIBUTE, sizeof(steam->serial_no), ATTRIB_STR_UNIT_SERIAL};
> u8 reply[3 + STEAM_SERIAL_LEN + 1];
>
> - mutex_lock(&steam->report_mutex);
> + guard(mutex)(&steam->report_mutex);
> ret = steam_send_report(steam, cmd, sizeof(cmd));
> if (ret < 0)
> - goto out;
> + return ret;
> ret = steam_recv_report(steam, reply, sizeof(reply));
> if (ret < 0)
> - goto out;
> + return ret;
> if (reply[0] != ID_GET_STRING_ATTRIBUTE || reply[1] < 1 ||
> reply[1] > sizeof(steam->serial_no) || reply[2] != ATTRIB_STR_UNIT_SERIAL) {
> - ret = -EIO;
> - goto out;
> + hid_err(steam->hdev, "%s: invalid reply (%*ph)\n", __func__,
> + (int)sizeof(reply), reply);
[Severity: High]
Does this code risk exposing uninitialized kernel stack memory to the kernel
log?
When steam_recv_report() reads a short report, it might copy fewer bytes than
sizeof(reply) into the uninitialized reply buffer. The new hid_err() call
unconditionally dumps the entire buffer capacity, which could leak stack
memory if the device responds with an invalid or short report.
> + return -EIO;
> }
> reply[3 + STEAM_SERIAL_LEN] = 0;
> strscpy(steam->serial_no, reply + 3, reply[1]);
> -out:
> - mutex_unlock(&steam->report_mutex);
> return ret;
> }
[ ... ]
While reviewing this series, I also noticed a couple of issues in the
error handling paths of steam_probe() from earlier patches or existing code:
[Severity: High]
This is a pre-existing issue, but does this code leave a dangling pointer in
the global steam_devices list on registration failure?
In steam_probe():
} else {
/* A wired connection is always present */
steam->connected = true;
ret = steam_register(steam);
if (ret) {
hid_err(hdev,
"%s:steam_register failed with error %d\n",
__func__, ret);
goto err_hw_close;
}
}
If steam_register() fails, the device is already added to the global
steam_devices list, but the error path jumps to err_hw_close. This bypasses
err_steam_unregister where it would normally be removed.
Once probe fails, devm frees the steam structure, which can lead to a
use-after-free if the lizard_mode sysfs parameter is subsequently accessed.
[Severity: High]
Does the following sequence result in a use-after-free if probe fails late?
In a previous patch in this series, the cancellation of work_connect was
moved from err_cancel_work to err_steam_unregister:
err_steam_unregister:
spin_lock_irqsave(&steam->lock, flags);
steam->client_opened = 0;
spin_unlock_irqrestore(&steam->lock, flags);
cancel_work_sync(&steam->work_connect);
if (steam->connected)
steam_unregister(steam);
err_hw_close:
hid_hw_close(hdev);
err_hw_stop:
hid_hw_stop(hdev);
err_cancel_work:
cancel_delayed_work_sync(&steam->mode_switch);
cancel_work_sync(&steam->rumble_work);
cancel_delayed_work_sync(&steam->coalesce_rumble_work);
If an ID_CONTROLLER_WIRELESS event is received early asynchronously,
work_connect could be queued. If hid_hw_open() then fails and jumps to
err_hw_stop, err_steam_unregister is bypassed.
The devm subsystem will free the steam structure, and the still-queued work
item could crash when executed.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260707233241.2644971-1-vi@endrift.com?part=8
^ permalink raw reply
* Re: [PATCH v2 09/10] HID: steam: Reject short reads
From: sashiko-bot @ 2026-07-07 23:47 UTC (permalink / raw)
To: Vicki Pfau; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260707233241.2644971-10-vi@endrift.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Out-of-bounds stack memory read in `steam_recv_report` when logging debug information.
- [Medium] Uninitialized kernel stack memory read on the error path for short reports.
--
--- Patch [9]: [PATCH v2 09/10] HID: steam: Reject short reads ---
commit ea3743d28300ad972f70af0939882012d7dbf763
Author: Vicki Pfau <vi@endrift.com>
HID: steam: Reject short reads
Steam Controller FEATURE reports encode the size of the message in the
message itself. Previously we were trusting that the size reported matched
the size we actually read, leading to a potential issue with short reads.
Instead, we should actually verify the length of the read.
Fixes: c164d6abf384 ("HID: add driver for Valve Steam Controller")
Reported-by: syzbot+75f3f9bff8c510602d36@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=75f3f9bff8c510602d36
> diff --git a/drivers/hid/hid-steam.c b/drivers/hid/hid-steam.c
> index 1f2cf15d40831..49f6733d89359 100644
> --- a/drivers/hid/hid-steam.c
> +++ b/drivers/hid/hid-steam.c
[ ... ]
> @@ -380,8 +387,10 @@ static int steam_recv_report(struct steam_device *steam,
> buf, hid_report_len(r) + 1,
> HID_FEATURE_REPORT, HID_REQ_GET_REPORT);
> if (ret > 0) {
> - ret = min(size, ret - 1);
> - memcpy(data, buf + 1, ret);
> + /* Remove the report ID from the return buffer */
> + ret--;
> + size = min(size, ret);
> + memcpy(data, buf + 1, size);
> }
> kfree(buf);
>
> @@ -389,7 +398,15 @@ static int steam_recv_report(struct steam_device *steam,
> hid_err(steam->hdev, "%s: error %d\n", __func__, ret);
> else
> hid_dbg(steam->hdev, "Received report %*ph\n", ret, data);
[Severity: Medium]
Since the clamping of ret to size was removed above, ret can now be larger
than the caller-provided buffer size.
If a caller provides a small stack buffer (e.g. 25 bytes in
steam_get_serial()) and the device returns a larger report, will hid_dbg()
read out-of-bounds stack memory from the data buffer when dynamic debug is
enabled?
> - return ret;
> + if (ret < 0)
> + return ret;
> +
> + if (ret < 2 || ret < data[1] + 2) {
> + hid_err(steam->hdev, "%s: expected %u bytes, read %i\n",
> + __func__, data[1] + 2, ret);
[Severity: Medium]
If the device returns a short read of 0 or 1 bytes, memcpy() copies less
than 2 bytes to data, leaving data[1] uninitialized.
Since hid_err() arguments are unconditionally evaluated before the function
is called, could this read uninitialized stack memory from data[1] if
ret < 2 evaluates to true?
> + return -EPROTO;
> + }
> + return size;
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260707233241.2644971-1-vi@endrift.com?part=9
^ permalink raw reply
* Re: [PATCH v2 05/10] HID: steam: Coalesce rumble packets
From: sashiko-bot @ 2026-07-07 23:46 UTC (permalink / raw)
To: Vicki Pfau; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260707233241.2644971-6-vi@endrift.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The rate-limiting logic inadvertently creates a continuous polling loop that actively induces the haptic discontinuities it was intended to fix.
--
--- Patch [5]: [PATCH v2 05/10] HID: steam: Coalesce rumble packets ---
commit 57ea23e2a38271a41e31886938cdef03c430a55f
Author: Vicki Pfau <vi@endrift.com>
HID: steam: Coalesce rumble packets
This commit adds rate limiting to Steam Deck haptic rumble packets to
avoid resetting the haptic pattern too frequently when userspace sends
many rapid updates. It introduces a delayed work queue to throttle updates
to 20Hz.
> diff --git a/drivers/hid/hid-steam.c b/drivers/hid/hid-steam.c
> index 6029c055a6cf3..5537488139011 100644
> --- a/drivers/hid/hid-steam.c
> +++ b/drivers/hid/hid-steam.c
[ ... ]
> @@ -603,10 +604,24 @@ static void steam_haptic_rumble_cb(struct work_struct *work)
> {
> struct steam_device *steam = container_of(work, struct steam_device,
> rumble_work);
> +
> steam_haptic_rumble(steam, 0, steam->rumble_left,
> steam->rumble_right, 2, 0);
> }
>
> +static void steam_coalesce_rumble_cb(struct work_struct *work)
> +{
> + struct steam_device *steam = container_of(to_delayed_work(work),
> + struct steam_device,
> + coalesce_rumble_work);
> +
> + steam_haptic_rumble(steam, 0, steam->rumble_left,
> + steam->rumble_right, 2, 0);
> +
> + if (steam->rumble_left || steam->rumble_right)
> + schedule_delayed_work(&steam->coalesce_rumble_work, HZ / 20);
> +}
[Severity: High]
Does this logic inadvertently create a continuous loop that actively induces
the haptic discontinuities it was intended to fix?
Since the callback unconditionally reschedules itself every 50ms as long as
the stored rumble values are non-zero, it will continuously transmit identical
rumble packets to the device at 20Hz, even when userspace has stopped
updating the effect.
Because the commit message mentions the Steam Deck resets its haptic pattern
on every packet, won't this perpetual loop guarantee the pattern resets every
50ms and unnecessarily spam the USB bus?
> +
> #ifdef CONFIG_STEAM_FF
> static int steam_play_effect(struct input_dev *dev, void *data,
> struct ff_effect *effect)
> @@ -616,6 +631,14 @@ static int steam_play_effect(struct input_dev *dev, void *data,
> steam->rumble_left = effect->u.rumble.strong_magnitude;
> steam->rumble_right = effect->u.rumble.weak_magnitude;
>
> + /*
> + * The interface gets somewhat overloaded when too many rumble
> + * packets are sent in a row, so Steam throttles it to 20 Hz
> + */
> + if (delayed_work_pending(&steam->coalesce_rumble_work))
> + return 0;
[Severity: High]
Could this condition unnecessarily stall valid userspace updates?
Because the loop in steam_coalesce_rumble_cb() keeps the delayed work pending
continuously while the effect is active, valid userspace updates may be
stalled by up to 50ms instead of executing immediately when the system is
otherwise idle.
> +
> + schedule_delayed_work(&steam->coalesce_rumble_work, HZ / 20);
> return schedule_work(&steam->rumble_work);
> }
> #endif
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260707233241.2644971-1-vi@endrift.com?part=5
^ permalink raw reply
* [PATCH v2 10/10] HID: steam: Retry send/recv reports if stale
From: Vicki Pfau @ 2026-07-07 23:32 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, linux-input; +Cc: Vicki Pfau, Yousef Alhouseen
In-Reply-To: <20260707233241.2644971-1-vi@endrift.com>
Sometimes recv report will reply with a stale result from a previous send
report. Instead of failing out, we should retry them, as they generally
reply correctly after three tries, give or take.
Signed-off-by: Vicki Pfau <vi@endrift.com>
---
drivers/hid/hid-steam.c | 54 +++++++++++++++++++++++++++++++----------
1 file changed, 41 insertions(+), 13 deletions(-)
diff --git a/drivers/hid/hid-steam.c b/drivers/hid/hid-steam.c
index 49f6733d8935..ba2dd40a7976 100644
--- a/drivers/hid/hid-steam.c
+++ b/drivers/hid/hid-steam.c
@@ -497,6 +497,43 @@ static int steam_write_settings(struct steam_device *steam,
return steam_recv_report(steam, cmd, 2 + cmd[1]);
}
+static int steam_exchange_report(struct steam_device *steam, u8 *cmd, int csize,
+ u8 *reply, int rsize)
+{
+ unsigned int retries = 5;
+ int ret;
+
+ guard(mutex)(&steam->report_mutex);
+ do {
+ ret = steam_send_report(steam, cmd, csize);
+ if (ret < 0)
+ return ret;
+ ret = steam_recv_report(steam, reply, rsize);
+ /*
+ * Sometimes this can fail on the first few tries on the Steam
+ * Controller (2015). It appears to be a firmware bug, and Steam
+ * itself just retries, so we should also retry a few times to
+ * see if we get it.
+ */
+ if (ret == -EPROTO)
+ continue;
+ if (ret < 0) {
+ hid_err(steam->hdev, "%s: error reading reply (%*ph)\n",
+ __func__, csize, cmd);
+ return ret;
+ }
+ if (reply[0] == cmd[0] && reply[1] >= 1)
+ break;
+ if (retries > 0)
+ continue;
+ hid_err(steam->hdev, "%s: invalid reply (%*ph)\n", __func__,
+ rsize, reply);
+ return -EPROTO;
+ } while (retries--);
+
+ return ret;
+}
+
static int steam_get_serial(struct steam_device *steam)
{
/*
@@ -507,15 +544,10 @@ static int steam_get_serial(struct steam_device *steam)
u8 cmd[] = {ID_GET_STRING_ATTRIBUTE, sizeof(steam->serial_no), ATTRIB_STR_UNIT_SERIAL};
u8 reply[3 + STEAM_SERIAL_LEN + 1];
- guard(mutex)(&steam->report_mutex);
- ret = steam_send_report(steam, cmd, sizeof(cmd));
+ ret = steam_exchange_report(steam, cmd, sizeof(cmd), reply, sizeof(reply));
if (ret < 0)
return ret;
- ret = steam_recv_report(steam, reply, sizeof(reply));
- if (ret < 0)
- return ret;
- if (reply[0] != ID_GET_STRING_ATTRIBUTE || reply[1] < 1 ||
- reply[1] > sizeof(steam->serial_no) || reply[2] != ATTRIB_STR_UNIT_SERIAL) {
+ if (reply[1] > sizeof(steam->serial_no) || reply[2] != ATTRIB_STR_UNIT_SERIAL) {
hid_err(steam->hdev, "%s: invalid reply (%*ph)\n", __func__,
(int)sizeof(reply), reply);
return -EIO;
@@ -534,14 +566,10 @@ static int steam_get_attributes(struct steam_device *steam)
int i;
struct steam_controller_attribute *attr;
- guard(mutex)(&steam->report_mutex);
- ret = steam_send_report(steam, cmd, sizeof(cmd));
- if (ret < 0)
- return ret;
- ret = steam_recv_report(steam, reply, sizeof(reply));
+ ret = steam_exchange_report(steam, cmd, sizeof(cmd), reply, sizeof(reply));
if (ret < 0)
return ret;
- if (reply[0] != ID_GET_ATTRIBUTES_VALUES || reply[1] < 2) {
+ if (reply[1] < 2) {
hid_err(steam->hdev, "%s: invalid reply (%*ph)\n", __func__,
(int)sizeof(reply), reply);
return -EIO;
--
2.54.0
^ permalink raw reply related
* [PATCH v2 09/10] HID: steam: Reject short reads
From: Vicki Pfau @ 2026-07-07 23:32 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, linux-input
Cc: Vicki Pfau, Yousef Alhouseen, syzbot+75f3f9bff8c510602d36
In-Reply-To: <20260707233241.2644971-1-vi@endrift.com>
Steam Controller FEATURE reports encode the size of the message in the
message itself. Previously we were trusting that the size reported matched
the size we actually read, leading to a potential issue with short reads.
Instead, we should actually verify the length of the read.
Fixes: c164d6abf384 ("HID: add driver for Valve Steam Controller")
Reported-by: syzbot+75f3f9bff8c510602d36@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=75f3f9bff8c510602d36
Signed-off-by: Vicki Pfau <vi@endrift.com>
---
drivers/hid/hid-steam.c | 23 ++++++++++++++++++++---
1 file changed, 20 insertions(+), 3 deletions(-)
diff --git a/drivers/hid/hid-steam.c b/drivers/hid/hid-steam.c
index 1f2cf15d4083..49f6733d8935 100644
--- a/drivers/hid/hid-steam.c
+++ b/drivers/hid/hid-steam.c
@@ -357,6 +357,13 @@ static int steam_recv_report(struct steam_device *steam,
u8 *buf;
int ret;
+ /*
+ * All reports start with a two byte header.
+ * We must read at least two bytes to get a sensible output.
+ */
+ if (size < 2)
+ return -EINVAL;
+
r = steam->hdev->report_enum[HID_FEATURE_REPORT].report_id_hash[0];
if (!r) {
hid_err(steam->hdev, "No HID_FEATURE_REPORT submitted - nothing to read\n");
@@ -380,8 +387,10 @@ static int steam_recv_report(struct steam_device *steam,
buf, hid_report_len(r) + 1,
HID_FEATURE_REPORT, HID_REQ_GET_REPORT);
if (ret > 0) {
- ret = min(size, ret - 1);
- memcpy(data, buf + 1, ret);
+ /* Remove the report ID from the return buffer */
+ ret--;
+ size = min(size, ret);
+ memcpy(data, buf + 1, size);
}
kfree(buf);
@@ -389,7 +398,15 @@ static int steam_recv_report(struct steam_device *steam,
hid_err(steam->hdev, "%s: error %d\n", __func__, ret);
else
hid_dbg(steam->hdev, "Received report %*ph\n", ret, data);
- return ret;
+ if (ret < 0)
+ return ret;
+
+ if (ret < 2 || ret < data[1] + 2) {
+ hid_err(steam->hdev, "%s: expected %u bytes, read %i\n",
+ __func__, data[1] + 2, ret);
+ return -EPROTO;
+ }
+ return size;
}
static int steam_send_report(struct steam_device *steam,
--
2.54.0
^ permalink raw reply related
* [PATCH v2 08/10] HID: steam: Improve logging and other cleanup
From: Vicki Pfau @ 2026-07-07 23:32 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, linux-input; +Cc: Vicki Pfau, Yousef Alhouseen
In-Reply-To: <20260707233241.2644971-1-vi@endrift.com>
Adds more logging as appropriate, reindents an enum to match surrounding
style, as well as cleaning up some places where we can use guard() instead
of doing locking and unlocking manually.
Signed-off-by: Vicki Pfau <vi@endrift.com>
---
drivers/hid/hid-steam.c | 56 ++++++++++++++++++++++++-----------------
1 file changed, 33 insertions(+), 23 deletions(-)
diff --git a/drivers/hid/hid-steam.c b/drivers/hid/hid-steam.c
index 296ced2989a4..1f2cf15d4083 100644
--- a/drivers/hid/hid-steam.c
+++ b/drivers/hid/hid-steam.c
@@ -246,14 +246,14 @@ enum {
/* Input report identifiers */
enum
{
- ID_CONTROLLER_STATE = 1,
- ID_CONTROLLER_DEBUG = 2,
- ID_CONTROLLER_WIRELESS = 3,
- ID_CONTROLLER_STATUS = 4,
- ID_CONTROLLER_DEBUG2 = 5,
- ID_CONTROLLER_SECONDARY_STATE = 6,
- ID_CONTROLLER_BLE_STATE = 7,
- ID_CONTROLLER_DECK_STATE = 9
+ ID_CONTROLLER_STATE = 1,
+ ID_CONTROLLER_DEBUG = 2,
+ ID_CONTROLLER_WIRELESS = 3,
+ ID_CONTROLLER_STATUS = 4,
+ ID_CONTROLLER_DEBUG2 = 5,
+ ID_CONTROLLER_SECONDARY_STATE = 6,
+ ID_CONTROLLER_BLE_STATE = 7,
+ ID_CONTROLLER_DECK_STATE = 9,
};
/* Read-only attributes */
@@ -379,9 +379,16 @@ static int steam_recv_report(struct steam_device *steam,
ret = hid_hw_raw_request(steam->hdev, 0x00,
buf, hid_report_len(r) + 1,
HID_FEATURE_REPORT, HID_REQ_GET_REPORT);
- if (ret > 0)
- memcpy(data, buf + 1, min(size, ret - 1));
+ if (ret > 0) {
+ ret = min(size, ret - 1);
+ memcpy(data, buf + 1, ret);
+ }
kfree(buf);
+
+ if (ret < 0)
+ hid_err(steam->hdev, "%s: error %d\n", __func__, ret);
+ else
+ hid_dbg(steam->hdev, "Received report %*ph\n", ret, data);
return ret;
}
@@ -409,6 +416,8 @@ static int steam_send_report(struct steam_device *steam,
/* The report ID is always 0 */
memcpy(buf + 1, cmd, size);
+ hid_dbg(steam->hdev, "Sending report %*ph\n", size, cmd);
+
/*
* Sometimes the wireless controller fails with EPIPE
* when sending a feature report.
@@ -481,22 +490,21 @@ static int steam_get_serial(struct steam_device *steam)
u8 cmd[] = {ID_GET_STRING_ATTRIBUTE, sizeof(steam->serial_no), ATTRIB_STR_UNIT_SERIAL};
u8 reply[3 + STEAM_SERIAL_LEN + 1];
- mutex_lock(&steam->report_mutex);
+ guard(mutex)(&steam->report_mutex);
ret = steam_send_report(steam, cmd, sizeof(cmd));
if (ret < 0)
- goto out;
+ return ret;
ret = steam_recv_report(steam, reply, sizeof(reply));
if (ret < 0)
- goto out;
+ return ret;
if (reply[0] != ID_GET_STRING_ATTRIBUTE || reply[1] < 1 ||
reply[1] > sizeof(steam->serial_no) || reply[2] != ATTRIB_STR_UNIT_SERIAL) {
- ret = -EIO;
- goto out;
+ hid_err(steam->hdev, "%s: invalid reply (%*ph)\n", __func__,
+ (int)sizeof(reply), reply);
+ return -EIO;
}
reply[3 + STEAM_SERIAL_LEN] = 0;
strscpy(steam->serial_no, reply + 3, reply[1]);
-out:
- mutex_unlock(&steam->report_mutex);
return ret;
}
@@ -516,8 +524,11 @@ static int steam_get_attributes(struct steam_device *steam)
ret = steam_recv_report(steam, reply, sizeof(reply));
if (ret < 0)
return ret;
- if (reply[0] != ID_GET_ATTRIBUTES_VALUES || reply[1] < 2)
+ if (reply[0] != ID_GET_ATTRIBUTES_VALUES || reply[1] < 2) {
+ hid_err(steam->hdev, "%s: invalid reply (%*ph)\n", __func__,
+ (int)sizeof(reply), reply);
return -EIO;
+ }
size = min(reply[1], sizeof(reply) - 2);
for (i = 0; i + sizeof(*attr) <= size; i += sizeof(*attr)) {
@@ -539,11 +550,8 @@ static int steam_get_attributes(struct steam_device *steam)
*/
static inline int steam_request_conn_status(struct steam_device *steam)
{
- int ret;
- mutex_lock(&steam->report_mutex);
- ret = steam_send_report_byte(steam, ID_DONGLE_GET_WIRELESS_STATE);
- mutex_unlock(&steam->report_mutex);
- return ret;
+ guard(mutex)(&steam->report_mutex);
+ return steam_send_report_byte(steam, ID_DONGLE_GET_WIRELESS_STATE);
}
/*
@@ -1192,6 +1200,7 @@ static void steam_mode_switch_cb(struct work_struct *work)
return;
steam->gamepad_mode = !steam->gamepad_mode;
+ hid_dbg(steam->hdev, "%s: switching gamepad mode to %i\n", __func__, steam->gamepad_mode);
if (steam->gamepad_mode)
steam_set_lizard_mode(steam, false);
else {
@@ -1840,6 +1849,7 @@ static void steam_do_deck_input_event(struct steam_device *steam,
steam->did_mode_switch = false;
cancel_delayed_work(&steam->mode_switch);
} else if (!steam->client_opened && start_pressed && !steam->did_mode_switch) {
+ hid_dbg(steam->hdev, "%s: doing mode switch\n", __func__);
steam->did_mode_switch = true;
schedule_delayed_work(&steam->mode_switch, 45 * HZ / 100);
}
--
2.54.0
^ permalink raw reply related
* [PATCH v2 07/10] HID: steam: Rearrange deinitialization sequence
From: Vicki Pfau @ 2026-07-07 23:32 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, linux-input; +Cc: Vicki Pfau, Yousef Alhouseen
In-Reply-To: <20260707233241.2644971-1-vi@endrift.com>
This fixes a narrow window during the deinitialization where callbacks
could still be scheduled during cleanup that would then have a dangling
pointer to the now-freed steam struct.
Signed-off-by: Vicki Pfau <vi@endrift.com>
---
drivers/hid/hid-steam.c | 21 ++++++++++++++-------
1 file changed, 14 insertions(+), 7 deletions(-)
diff --git a/drivers/hid/hid-steam.c b/drivers/hid/hid-steam.c
index 3f1dbdcad23c..296ced2989a4 100644
--- a/drivers/hid/hid-steam.c
+++ b/drivers/hid/hid-steam.c
@@ -1327,6 +1327,7 @@ static int steam_probe(struct hid_device *hdev,
{
struct steam_device *steam;
int ret;
+ unsigned long flags;
ret = hid_parse(hdev);
if (ret) {
@@ -1417,6 +1418,10 @@ static int steam_probe(struct hid_device *hdev,
err_destroy:
hid_destroy_device(steam->client_hdev);
err_steam_unregister:
+ spin_lock_irqsave(&steam->lock, flags);
+ steam->client_opened = 0;
+ spin_unlock_irqrestore(&steam->lock, flags);
+ cancel_work_sync(&steam->work_connect);
if (steam->connected)
steam_unregister(steam);
err_hw_close:
@@ -1424,7 +1429,6 @@ static int steam_probe(struct hid_device *hdev,
err_hw_stop:
hid_hw_stop(hdev);
err_cancel_work:
- cancel_work_sync(&steam->work_connect);
cancel_delayed_work_sync(&steam->mode_switch);
cancel_work_sync(&steam->rumble_work);
cancel_delayed_work_sync(&steam->coalesce_rumble_work);
@@ -1435,6 +1439,7 @@ static int steam_probe(struct hid_device *hdev,
static void steam_remove(struct hid_device *hdev)
{
struct steam_device *steam = hid_get_drvdata(hdev);
+ unsigned long flags;
if (!steam || hdev->group == HID_GROUP_STEAM) {
hid_hw_stop(hdev);
@@ -1442,18 +1447,20 @@ static void steam_remove(struct hid_device *hdev)
}
hid_destroy_device(steam->client_hdev);
- cancel_delayed_work_sync(&steam->mode_switch);
- cancel_work_sync(&steam->work_connect);
- cancel_work_sync(&steam->rumble_work);
- cancel_delayed_work_sync(&steam->coalesce_rumble_work);
steam->client_hdev = NULL;
+ hid_hw_close(hdev);
+ spin_lock_irqsave(&steam->lock, flags);
steam->client_opened = 0;
+ spin_unlock_irqrestore(&steam->lock, flags);
+ cancel_work_sync(&steam->work_connect);
if (steam->quirks & STEAM_QUIRK_WIRELESS) {
hid_info(hdev, "Steam wireless receiver disconnected");
}
- hid_hw_close(hdev);
- hid_hw_stop(hdev);
steam_unregister(steam);
+ cancel_work_sync(&steam->rumble_work);
+ cancel_delayed_work_sync(&steam->mode_switch);
+ cancel_delayed_work_sync(&steam->coalesce_rumble_work);
+ hid_hw_stop(hdev);
}
static void steam_do_connect_event(struct steam_device *steam, bool connected)
--
2.54.0
^ permalink raw reply related
* [PATCH v2 06/10] HID: steam: Fully unregister controller when hidraw is opened
From: Vicki Pfau @ 2026-07-07 23:32 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, linux-input; +Cc: Vicki Pfau, Yousef Alhouseen
In-Reply-To: <20260707233241.2644971-1-vi@endrift.com>
To avoid conflicts between anything touching the hidraw and the driver we
had previously detached the evdev nodes when the hidraw is opened. However,
this isn't sufficient to avoid FEATURE reports from conflicting, so we
change to fully unregistering the controller internally, leaving only the
hidraw active until it's closed.
This also unifies the unregister and connect callbacks, as now the logic
between these two callbacks is identical.
Signed-off-by: Vicki Pfau <vi@endrift.com>
---
drivers/hid/hid-steam.c | 59 ++++++++++++-----------------------------
1 file changed, 17 insertions(+), 42 deletions(-)
diff --git a/drivers/hid/hid-steam.c b/drivers/hid/hid-steam.c
index 553748813901..3f1dbdcad23c 100644
--- a/drivers/hid/hid-steam.c
+++ b/drivers/hid/hid-steam.c
@@ -348,7 +348,6 @@ struct steam_device {
u16 rumble_right;
unsigned int sensor_timestamp_us;
unsigned int sensor_update_rate_us;
- struct work_struct unregister_work;
};
static int steam_recv_report(struct steam_device *steam,
@@ -1077,6 +1076,7 @@ static void steam_battery_unregister(struct steam_device *steam)
RCU_INIT_POINTER(steam->battery, NULL);
synchronize_rcu();
power_supply_unregister(battery);
+ devm_kfree(&steam->hdev->dev, steam->battery_desc.name);
}
static int steam_register(struct steam_device *steam)
@@ -1142,38 +1142,41 @@ static int steam_register(struct steam_device *steam)
static void steam_unregister(struct steam_device *steam)
{
+ if (!steam->serial_no[0])
+ return;
+
+ hid_info(steam->hdev, "Steam Controller '%s' disconnected",
+ steam->serial_no);
steam_battery_unregister(steam);
steam_sensors_unregister(steam);
steam_input_unregister(steam);
- if (steam->serial_no[0]) {
- hid_info(steam->hdev, "Steam Controller '%s' disconnected",
- steam->serial_no);
- mutex_lock(&steam_devices_lock);
- list_del_init(&steam->list);
- mutex_unlock(&steam_devices_lock);
- steam->serial_no[0] = 0;
- }
+ mutex_lock(&steam_devices_lock);
+ list_del_init(&steam->list);
+ mutex_unlock(&steam_devices_lock);
+ steam->serial_no[0] = 0;
}
static void steam_work_connect_cb(struct work_struct *work)
{
struct steam_device *steam = container_of(work, struct steam_device,
work_connect);
+
unsigned long flags;
bool connected;
+ bool opened;
int ret;
spin_lock_irqsave(&steam->lock, flags);
+ opened = steam->client_opened;
connected = steam->connected;
spin_unlock_irqrestore(&steam->lock, flags);
- if (connected) {
+ if (connected && !opened) {
ret = steam_register(steam);
- if (ret) {
+ if (ret)
hid_err(steam->hdev,
"%s:steam_register failed with error %d\n",
__func__, ret);
- }
} else {
steam_unregister(steam);
}
@@ -1207,31 +1210,6 @@ static void steam_mode_switch_cb(struct work_struct *work)
}
}
-static void steam_work_unregister_cb(struct work_struct *work)
-{
- struct steam_device *steam = container_of(work, struct steam_device,
- unregister_work);
- unsigned long flags;
- bool connected;
- bool opened;
-
- spin_lock_irqsave(&steam->lock, flags);
- opened = steam->client_opened;
- connected = steam->connected;
- spin_unlock_irqrestore(&steam->lock, flags);
-
- if (connected) {
- if (opened) {
- steam_sensors_unregister(steam);
- steam_input_unregister(steam);
- } else {
- steam_set_lizard_mode(steam, lizard_mode);
- steam_input_register(steam);
- steam_sensors_register(steam);
- }
- }
-}
-
static bool steam_is_valve_interface(struct hid_device *hdev)
{
struct hid_report_enum *rep_enum;
@@ -1277,7 +1255,7 @@ static int steam_client_ll_open(struct hid_device *hdev)
steam->client_opened++;
spin_unlock_irqrestore(&steam->lock, flags);
- schedule_work(&steam->unregister_work);
+ schedule_work(&steam->work_connect);
return 0;
}
@@ -1292,7 +1270,7 @@ static void steam_client_ll_close(struct hid_device *hdev)
steam->client_opened--;
spin_unlock_irqrestore(&steam->lock, flags);
- schedule_work(&steam->unregister_work);
+ schedule_work(&steam->work_connect);
}
static int steam_client_ll_raw_request(struct hid_device *hdev,
@@ -1389,7 +1367,6 @@ static int steam_probe(struct hid_device *hdev,
steam->sensor_update_rate_us = 4000;
else
steam->sensor_update_rate_us = 9000;
- INIT_WORK(&steam->unregister_work, steam_work_unregister_cb);
/*
* With the real steam controller interface, do not connect hidraw.
@@ -1451,7 +1428,6 @@ static int steam_probe(struct hid_device *hdev,
cancel_delayed_work_sync(&steam->mode_switch);
cancel_work_sync(&steam->rumble_work);
cancel_delayed_work_sync(&steam->coalesce_rumble_work);
- cancel_work_sync(&steam->unregister_work);
return ret;
}
@@ -1470,7 +1446,6 @@ static void steam_remove(struct hid_device *hdev)
cancel_work_sync(&steam->work_connect);
cancel_work_sync(&steam->rumble_work);
cancel_delayed_work_sync(&steam->coalesce_rumble_work);
- cancel_work_sync(&steam->unregister_work);
steam->client_hdev = NULL;
steam->client_opened = 0;
if (steam->quirks & STEAM_QUIRK_WIRELESS) {
--
2.54.0
^ permalink raw reply related
* [PATCH v2 05/10] HID: steam: Coalesce rumble packets
From: Vicki Pfau @ 2026-07-07 23:32 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, linux-input; +Cc: Vicki Pfau, Yousef Alhouseen
In-Reply-To: <20260707233241.2644971-1-vi@endrift.com>
The Steam Deck resets the haptic pattern every time it receives a rumble
packet, leading to weird discontinuities or sometimes cutting out entirely.
Instead of overloading the interface, Steam interally rate-limits sending
these packets, so we should too.
Signed-off-by: Vicki Pfau <vi@endrift.com>
---
drivers/hid/hid-steam.c | 26 ++++++++++++++++++++++++++
1 file changed, 26 insertions(+)
diff --git a/drivers/hid/hid-steam.c b/drivers/hid/hid-steam.c
index 6029c055a6cf..553748813901 100644
--- a/drivers/hid/hid-steam.c
+++ b/drivers/hid/hid-steam.c
@@ -343,6 +343,7 @@ struct steam_device {
bool did_mode_switch;
bool gamepad_mode;
struct work_struct rumble_work;
+ struct delayed_work coalesce_rumble_work;
u16 rumble_left;
u16 rumble_right;
unsigned int sensor_timestamp_us;
@@ -603,10 +604,24 @@ static void steam_haptic_rumble_cb(struct work_struct *work)
{
struct steam_device *steam = container_of(work, struct steam_device,
rumble_work);
+
steam_haptic_rumble(steam, 0, steam->rumble_left,
steam->rumble_right, 2, 0);
}
+static void steam_coalesce_rumble_cb(struct work_struct *work)
+{
+ struct steam_device *steam = container_of(to_delayed_work(work),
+ struct steam_device,
+ coalesce_rumble_work);
+
+ steam_haptic_rumble(steam, 0, steam->rumble_left,
+ steam->rumble_right, 2, 0);
+
+ if (steam->rumble_left || steam->rumble_right)
+ schedule_delayed_work(&steam->coalesce_rumble_work, HZ / 20);
+}
+
#ifdef CONFIG_STEAM_FF
static int steam_play_effect(struct input_dev *dev, void *data,
struct ff_effect *effect)
@@ -616,6 +631,14 @@ static int steam_play_effect(struct input_dev *dev, void *data,
steam->rumble_left = effect->u.rumble.strong_magnitude;
steam->rumble_right = effect->u.rumble.weak_magnitude;
+ /*
+ * The interface gets somewhat overloaded when too many rumble
+ * packets are sent in a row, so Steam throttles it to 20 Hz
+ */
+ if (delayed_work_pending(&steam->coalesce_rumble_work))
+ return 0;
+
+ schedule_delayed_work(&steam->coalesce_rumble_work, HZ / 20);
return schedule_work(&steam->rumble_work);
}
#endif
@@ -1360,6 +1383,7 @@ static int steam_probe(struct hid_device *hdev,
INIT_DELAYED_WORK(&steam->mode_switch, steam_mode_switch_cb);
INIT_LIST_HEAD(&steam->list);
INIT_WORK(&steam->rumble_work, steam_haptic_rumble_cb);
+ INIT_DELAYED_WORK(&steam->coalesce_rumble_work, steam_coalesce_rumble_cb);
steam->sensor_timestamp_us = 0;
if (steam->quirks & STEAM_QUIRK_DECK)
steam->sensor_update_rate_us = 4000;
@@ -1426,6 +1450,7 @@ static int steam_probe(struct hid_device *hdev,
cancel_work_sync(&steam->work_connect);
cancel_delayed_work_sync(&steam->mode_switch);
cancel_work_sync(&steam->rumble_work);
+ cancel_delayed_work_sync(&steam->coalesce_rumble_work);
cancel_work_sync(&steam->unregister_work);
return ret;
@@ -1444,6 +1469,7 @@ static void steam_remove(struct hid_device *hdev)
cancel_delayed_work_sync(&steam->mode_switch);
cancel_work_sync(&steam->work_connect);
cancel_work_sync(&steam->rumble_work);
+ cancel_delayed_work_sync(&steam->coalesce_rumble_work);
cancel_work_sync(&steam->unregister_work);
steam->client_hdev = NULL;
steam->client_opened = 0;
--
2.54.0
^ permalink raw reply related
* [PATCH v2 04/10] HID: steam: Add support for sensor events on the Steam Controller (2015)
From: Vicki Pfau @ 2026-07-07 23:32 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, linux-input; +Cc: Vicki Pfau, Yousef Alhouseen
In-Reply-To: <20260707233241.2644971-1-vi@endrift.com>
Sensor support was added for the Steam Deck previously, but Steam
Controller sensor events were never added. This adds that missing support,
bringing Steam Controller support much closer to feature parity with things
like SDL and Steam itself.
Signed-off-by: Vicki Pfau <vi@endrift.com>
---
drivers/hid/hid-steam.c | 214 ++++++++++++++++++++++++++++++++--------
1 file changed, 175 insertions(+), 39 deletions(-)
diff --git a/drivers/hid/hid-steam.c b/drivers/hid/hid-steam.c
index add9f4f27fff..6029c055a6cf 100644
--- a/drivers/hid/hid-steam.c
+++ b/drivers/hid/hid-steam.c
@@ -70,13 +70,14 @@ static LIST_HEAD(steam_devices);
/* Joystick runs are about 5 mm and 32768 units */
#define STEAM_DECK_JOYSTICK_RESOLUTION 6553
/* Accelerometer has 16 bit resolution and a range of +/- 2g */
-#define STEAM_DECK_ACCEL_RES_PER_G 16384
-#define STEAM_DECK_ACCEL_RANGE 32768
+#define STEAM_ACCEL_RES_PER_G 16384
+#define STEAM_ACCEL_RANGE 32768
+#define STEAM_ACCEL_FUZZ 128
#define STEAM_DECK_ACCEL_FUZZ 32
/* Gyroscope has 16 bit resolution and a range of +/- 2000 dps */
-#define STEAM_DECK_GYRO_RES_PER_DPS 16
-#define STEAM_DECK_GYRO_RANGE 32768
-#define STEAM_DECK_GYRO_FUZZ 1
+#define STEAM_GYRO_RES_PER_DPS 16
+#define STEAM_GYRO_RANGE 32768
+#define STEAM_GYRO_FUZZ 0
#define STEAM_PAD_FUZZ 256
@@ -255,6 +256,31 @@ enum
ID_CONTROLLER_DECK_STATE = 9
};
+/* Read-only attributes */
+enum {
+ ATTRIB_UNIQUE_ID, // deprecated
+ ATTRIB_PRODUCT_ID,
+ ATTRIB_PRODUCT_REVISON, // deprecated
+ ATTRIB_CAPABILITIES = ATTRIB_PRODUCT_REVISON, // intentional aliasing
+ ATTRIB_FIRMWARE_VERSION, // deprecated
+ ATTRIB_FIRMWARE_BUILD_TIME,
+ ATTRIB_RADIO_FIRMWARE_BUILD_TIME,
+ ATTRIB_RADIO_DEVICE_ID0,
+ ATTRIB_RADIO_DEVICE_ID1,
+ ATTRIB_DONGLE_FIRMWARE_BUILD_TIME,
+ ATTRIB_HW_ID, // AKA BOARD_REVISION,
+ ATTRIB_BOOTLOADER_BUILD_TIME,
+ ATTRIB_CONNECTION_INTERVAL_IN_US,
+ ATTRIB_SECONDARY_FIRMWARE_BUILD_TIME,
+ ATTRIB_SECONDARY_BOOTLOADER_BUILD_TIME,
+ ATTRIB_SECONDARY_HW_ID, // AKA BOARD_REVISION,
+ ATTRIB_STREAMING,
+ ATTRIB_TRACKPAD_ID,
+ ATTRIB_SECONDARY_TRACKPAD_ID,
+
+ ATTRIB_COUNT
+};
+
/* String attribute identifiers */
enum {
ATTRIB_STR_BOARD_SERIAL,
@@ -284,6 +310,11 @@ enum {
TRACKPAD_GESTURE_KEYBOARD,
};
+struct steam_controller_attribute {
+ unsigned char tag;
+ __le32 value;
+} __packed;
+
/* Pad identifiers for the deck */
#define STEAM_PAD_LEFT 0
#define STEAM_PAD_RIGHT 1
@@ -315,6 +346,7 @@ struct steam_device {
u16 rumble_left;
u16 rumble_right;
unsigned int sensor_timestamp_us;
+ unsigned int sensor_update_rate_us;
struct work_struct unregister_work;
};
@@ -468,6 +500,38 @@ static int steam_get_serial(struct steam_device *steam)
return ret;
}
+static int steam_get_attributes(struct steam_device *steam)
+{
+ int ret = 0;
+ u8 cmd[] = {ID_GET_ATTRIBUTES_VALUES, 0};
+ u8 reply[64] = {};
+ u8 size;
+ int i;
+ struct steam_controller_attribute *attr;
+
+ guard(mutex)(&steam->report_mutex);
+ ret = steam_send_report(steam, cmd, sizeof(cmd));
+ if (ret < 0)
+ return ret;
+ ret = steam_recv_report(steam, reply, sizeof(reply));
+ if (ret < 0)
+ return ret;
+ if (reply[0] != ID_GET_ATTRIBUTES_VALUES || reply[1] < 2)
+ return -EIO;
+
+ size = min(reply[1], sizeof(reply) - 2);
+ for (i = 0; i + sizeof(*attr) <= size; i += sizeof(*attr)) {
+ attr = (struct steam_controller_attribute *)&reply[i + 2];
+ if (attr->tag == ATTRIB_CONNECTION_INTERVAL_IN_US) {
+ steam->sensor_update_rate_us = get_unaligned_le32(&attr->value);
+ hid_dbg(steam->hdev, "Sensor update rate: %uus\n",
+ steam->sensor_update_rate_us);
+ }
+ }
+
+ return 0;
+}
+
/*
* This command requests the wireless adaptor to post an event
* with the connection status. Useful if this driver is loaded when
@@ -626,6 +690,42 @@ static void steam_input_close(struct input_dev *dev)
}
}
+static int steam_sensor_open(struct input_dev *dev)
+{
+ struct steam_device *steam = input_get_drvdata(dev);
+ unsigned long flags;
+ bool client_opened;
+
+ spin_lock_irqsave(&steam->lock, flags);
+ client_opened = steam->client_opened;
+ spin_unlock_irqrestore(&steam->lock, flags);
+ if (client_opened)
+ return 0;
+
+ guard(mutex)(&steam->report_mutex);
+ steam_write_settings(steam, SETTING_IMU_MODE,
+ SETTING_IMU_MODE_SEND_RAW_ACCEL | SETTING_IMU_MODE_SEND_RAW_GYRO,
+ 0);
+
+ return 0;
+}
+
+static void steam_sensor_close(struct input_dev *dev)
+{
+ struct steam_device *steam = input_get_drvdata(dev);
+ unsigned long flags;
+ bool client_opened;
+
+ spin_lock_irqsave(&steam->lock, flags);
+ client_opened = steam->client_opened;
+ spin_unlock_irqrestore(&steam->lock, flags);
+ if (client_opened)
+ return;
+
+ guard(mutex)(&steam->report_mutex);
+ steam_write_settings(steam, SETTING_IMU_MODE, 0, 0);
+}
+
static enum power_supply_property steam_battery_props[] = {
POWER_SUPPLY_PROP_PRESENT,
POWER_SUPPLY_PROP_SCOPE,
@@ -839,9 +939,6 @@ static int steam_sensors_register(struct steam_device *steam)
struct input_dev *sensors;
int ret;
- if (!(steam->quirks & STEAM_QUIRK_DECK))
- return 0;
-
rcu_read_lock();
sensors = rcu_dereference(steam->sensors);
rcu_read_unlock();
@@ -856,8 +953,14 @@ static int steam_sensors_register(struct steam_device *steam)
input_set_drvdata(sensors, steam);
sensors->dev.parent = &hdev->dev;
+ if (!(steam->quirks & STEAM_QUIRK_DECK)) {
+ sensors->open = steam_sensor_open;
+ sensors->close = steam_sensor_close;
+ }
- sensors->name = "Steam Deck Motion Sensors";
+ sensors->name = steam->quirks & STEAM_QUIRK_DECK ?
+ "Steam Deck Motion Sensors" :
+ "Steam Controller Motion Sensors";
sensors->phys = hdev->phys;
sensors->uniq = steam->serial_no;
sensors->id.bustype = hdev->bus;
@@ -869,25 +972,34 @@ static int steam_sensors_register(struct steam_device *steam)
__set_bit(EV_MSC, sensors->evbit);
__set_bit(MSC_TIMESTAMP, sensors->mscbit);
- input_set_abs_params(sensors, ABS_X, -STEAM_DECK_ACCEL_RANGE,
- STEAM_DECK_ACCEL_RANGE, STEAM_DECK_ACCEL_FUZZ, 0);
- input_set_abs_params(sensors, ABS_Y, -STEAM_DECK_ACCEL_RANGE,
- STEAM_DECK_ACCEL_RANGE, STEAM_DECK_ACCEL_FUZZ, 0);
- input_set_abs_params(sensors, ABS_Z, -STEAM_DECK_ACCEL_RANGE,
- STEAM_DECK_ACCEL_RANGE, STEAM_DECK_ACCEL_FUZZ, 0);
- input_abs_set_res(sensors, ABS_X, STEAM_DECK_ACCEL_RES_PER_G);
- input_abs_set_res(sensors, ABS_Y, STEAM_DECK_ACCEL_RES_PER_G);
- input_abs_set_res(sensors, ABS_Z, STEAM_DECK_ACCEL_RES_PER_G);
-
- input_set_abs_params(sensors, ABS_RX, -STEAM_DECK_GYRO_RANGE,
- STEAM_DECK_GYRO_RANGE, STEAM_DECK_GYRO_FUZZ, 0);
- input_set_abs_params(sensors, ABS_RY, -STEAM_DECK_GYRO_RANGE,
- STEAM_DECK_GYRO_RANGE, STEAM_DECK_GYRO_FUZZ, 0);
- input_set_abs_params(sensors, ABS_RZ, -STEAM_DECK_GYRO_RANGE,
- STEAM_DECK_GYRO_RANGE, STEAM_DECK_GYRO_FUZZ, 0);
- input_abs_set_res(sensors, ABS_RX, STEAM_DECK_GYRO_RES_PER_DPS);
- input_abs_set_res(sensors, ABS_RY, STEAM_DECK_GYRO_RES_PER_DPS);
- input_abs_set_res(sensors, ABS_RZ, STEAM_DECK_GYRO_RES_PER_DPS);
+ if (steam->quirks & STEAM_QUIRK_DECK) {
+ input_set_abs_params(sensors, ABS_X, -STEAM_ACCEL_RANGE,
+ STEAM_ACCEL_RANGE, STEAM_DECK_ACCEL_FUZZ, 0);
+ input_set_abs_params(sensors, ABS_Y, -STEAM_ACCEL_RANGE,
+ STEAM_ACCEL_RANGE, STEAM_DECK_ACCEL_FUZZ, 0);
+ input_set_abs_params(sensors, ABS_Z, -STEAM_ACCEL_RANGE,
+ STEAM_ACCEL_RANGE, STEAM_DECK_ACCEL_FUZZ, 0);
+ } else {
+ input_set_abs_params(sensors, ABS_X, -STEAM_ACCEL_RANGE,
+ STEAM_ACCEL_RANGE, STEAM_ACCEL_FUZZ, 0);
+ input_set_abs_params(sensors, ABS_Y, -STEAM_ACCEL_RANGE,
+ STEAM_ACCEL_RANGE, STEAM_ACCEL_FUZZ, 0);
+ input_set_abs_params(sensors, ABS_Z, -STEAM_ACCEL_RANGE,
+ STEAM_ACCEL_RANGE, STEAM_ACCEL_FUZZ, 0);
+ }
+ input_abs_set_res(sensors, ABS_X, STEAM_ACCEL_RES_PER_G);
+ input_abs_set_res(sensors, ABS_Y, STEAM_ACCEL_RES_PER_G);
+ input_abs_set_res(sensors, ABS_Z, STEAM_ACCEL_RES_PER_G);
+
+ input_set_abs_params(sensors, ABS_RX, -STEAM_GYRO_RANGE,
+ STEAM_GYRO_RANGE, STEAM_GYRO_FUZZ, 0);
+ input_set_abs_params(sensors, ABS_RY, -STEAM_GYRO_RANGE,
+ STEAM_GYRO_RANGE, STEAM_GYRO_FUZZ, 0);
+ input_set_abs_params(sensors, ABS_RZ, -STEAM_GYRO_RANGE,
+ STEAM_GYRO_RANGE, STEAM_GYRO_FUZZ, 0);
+ input_abs_set_res(sensors, ABS_RX, STEAM_GYRO_RES_PER_DPS);
+ input_abs_set_res(sensors, ABS_RY, STEAM_GYRO_RES_PER_DPS);
+ input_abs_set_res(sensors, ABS_RZ, STEAM_GYRO_RES_PER_DPS);
ret = input_register_device(sensors);
if (ret)
@@ -918,9 +1030,6 @@ static void steam_sensors_unregister(struct steam_device *steam)
{
struct input_dev *sensors;
- if (!(steam->quirks & STEAM_QUIRK_DECK))
- return;
-
rcu_read_lock();
sensors = rcu_dereference(steam->sensors);
rcu_read_unlock();
@@ -968,6 +1077,12 @@ static int steam_register(struct steam_device *steam)
strscpy(steam->serial_no, "XXXXXXXXXX",
sizeof(steam->serial_no));
+ ret = steam_get_attributes(steam);
+ if (ret < 0)
+ hid_err(steam->hdev,
+ "%s:steam_get_attributes failed with error %d\n",
+ __func__, ret);
+
hid_info(steam->hdev, "Steam Controller '%s' connected",
steam->serial_no);
@@ -1246,6 +1361,10 @@ static int steam_probe(struct hid_device *hdev,
INIT_LIST_HEAD(&steam->list);
INIT_WORK(&steam->rumble_work, steam_haptic_rumble_cb);
steam->sensor_timestamp_us = 0;
+ if (steam->quirks & STEAM_QUIRK_DECK)
+ steam->sensor_update_rate_us = 4000;
+ else
+ steam->sensor_update_rate_us = 9000;
INIT_WORK(&steam->unregister_work, steam_work_unregister_cb);
/*
@@ -1491,6 +1610,16 @@ static const struct steam_axis_mapping steam_controller_axis_mappings[] = {
{ /* sentinel */ },
};
+static const struct steam_axis_mapping steam_controller_imu_mappings[] = {
+ { ABS_X, 1, 28 },
+ { ABS_Z, -1, 30 },
+ { ABS_Y, 1, 32 },
+ { ABS_RX, 1, 34 },
+ { ABS_RZ, 1, 36 },
+ { ABS_RY, 1, 38 },
+ { /* sentinel */ },
+};
+
static void steam_do_input_event(struct steam_device *steam,
struct input_dev *input, u8 *data)
{
@@ -1533,6 +1662,17 @@ static void steam_do_input_event(struct steam_device *steam,
input_sync(input);
}
+static void steam_do_sensors_event(struct steam_device *steam,
+ struct input_dev *sensors, u8 *data)
+{
+ steam->sensor_timestamp_us += steam->sensor_update_rate_us;
+
+ input_event(sensors, EV_MSC, MSC_TIMESTAMP, steam->sensor_timestamp_us);
+ steam_map_axes(sensors, steam_controller_imu_mappings, data);
+
+ input_sync(sensors);
+}
+
/*
* The size for this message payload is 56.
* The known values are:
@@ -1727,14 +1867,7 @@ static void steam_do_deck_input_event(struct steam_device *steam,
static void steam_do_deck_sensors_event(struct steam_device *steam,
struct input_dev *sensors, u8 *data)
{
- /*
- * The deck input report is received every 4 ms on average,
- * with a jitter of +/- 4 ms even though the USB descriptor claims
- * that it uses 1 kHz.
- * Since the HID report does not include a sensor timestamp,
- * use a fixed increment here.
- */
- steam->sensor_timestamp_us += 4000;
+ steam->sensor_timestamp_us += steam->sensor_update_rate_us;
if (!steam->gamepad_mode && lizard_mode)
return;
@@ -1819,6 +1952,9 @@ static int steam_raw_event(struct hid_device *hdev,
input = rcu_dereference(steam->input);
if (likely(input))
steam_do_input_event(steam, input, data);
+ sensors = rcu_dereference(steam->sensors);
+ if (likely(sensors))
+ steam_do_sensors_event(steam, sensors, data);
rcu_read_unlock();
break;
case ID_CONTROLLER_DECK_STATE:
--
2.54.0
^ permalink raw reply related
* [PATCH v2 02/10] HID: steam: Refactor and clean up report parsing
From: Vicki Pfau @ 2026-07-07 23:32 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, linux-input; +Cc: Vicki Pfau, Yousef Alhouseen
In-Reply-To: <20260707233241.2644971-1-vi@endrift.com>
This switches from a parsing style where each button or axis is parsed
individually out of a report using !!(byte & BIT(x)) style. This commit
switches it to a mostly unified approach of defining a list of individual
mappings in an array and passing it to a function that handles all of the
extraction. Theoretically this is more lines, but in practice it results in
(subjectively) cleaner code. Some exceptions still need to be made for
things like handling the lizard mode toggle key, but in general there's a
lot less manual code.
Signed-off-by: Vicki Pfau <vi@endrift.com>
---
drivers/hid/hid-steam.c | 211 ++++++++++++++++++++++++----------------
1 file changed, 128 insertions(+), 83 deletions(-)
diff --git a/drivers/hid/hid-steam.c b/drivers/hid/hid-steam.c
index a854d6360a0e..1b0367c98eac 100644
--- a/drivers/hid/hid-steam.c
+++ b/drivers/hid/hid-steam.c
@@ -43,6 +43,7 @@
#include <linux/rcupdate.h>
#include <linux/delay.h>
#include <linux/power_supply.h>
+#include <linux/unaligned.h>
#include "hid-ids.h"
MODULE_DESCRIPTION("HID driver for Valve Steam Controller");
@@ -1355,13 +1356,45 @@ static void steam_do_connect_event(struct steam_device *steam, bool connected)
* Clamp the values to 32767..-32767 so that the range is
* symmetrical and can be negated safely.
*/
-static inline s16 steam_le16(u8 *data)
+static inline s16 steam_le16(const u8 *data)
{
- s16 x = (s16) le16_to_cpup((__le16 *)data);
+ s16 x = (s16) get_unaligned_le16((__le16 *)data);
return x == -32768 ? -32767 : x;
}
+struct steam_button_mapping {
+ int code;
+ u8 byte;
+ u8 bit;
+};
+
+struct steam_axis_mapping {
+ int code;
+ s8 sign;
+ u8 byte;
+};
+
+static void steam_map_buttons(struct input_dev *input,
+ const struct steam_button_mapping *mappings, const u8 *data)
+{
+ const struct steam_button_mapping *mapping;
+
+ for (mapping = mappings; mapping->code; mapping++)
+ input_report_key(input, mapping->code,
+ data[mapping->byte] & BIT(mapping->bit));
+}
+
+static void steam_map_axes(struct input_dev *input,
+ const struct steam_axis_mapping *mappings, const u8 *data)
+{
+ const struct steam_axis_mapping *mapping;
+
+ for (mapping = mappings; mapping->sign; mapping++)
+ input_report_abs(input, mapping->code,
+ mapping->sign * steam_le16(&data[mapping->byte]));
+}
+
/*
* The size for this message payload is 60.
* The known values are:
@@ -1428,18 +1461,42 @@ static inline s16 steam_le16(u8 *data)
* 10.7 | -- | lpad_and_joy
*/
+static const struct steam_button_mapping steam_controller_button_mappings[] = {
+ { BTN_TR2, 8, 0 },
+ { BTN_TL2, 8, 1 },
+ { BTN_TR, 8, 2 },
+ { BTN_TL, 8, 3 },
+ { BTN_Y, 8, 4 },
+ { BTN_B, 8, 5 },
+ { BTN_X, 8, 6 },
+ { BTN_A, 8, 7 },
+ { BTN_SELECT, 9, 4 },
+ { BTN_MODE, 9, 5 },
+ { BTN_START, 9, 6 },
+ { BTN_GRIPL, 9, 7 },
+ { BTN_GRIPR, 10, 0 },
+ { BTN_THUMBR, 10, 2 },
+ { BTN_THUMBL, 10, 6 },
+ { BTN_THUMB2, 10, 4 },
+ { BTN_DPAD_UP, 9, 0 },
+ { BTN_DPAD_RIGHT, 9, 1 },
+ { BTN_DPAD_LEFT, 9, 2 },
+ { BTN_DPAD_DOWN, 9, 3 },
+ { /* sentinel */ },
+};
+
+static const struct steam_axis_mapping steam_controller_axis_mappings[] = {
+ { ABS_RX, 1, 20 },
+ { ABS_RY, -1, 22 },
+ { /* sentinel */ },
+};
+
static void steam_do_input_event(struct steam_device *steam,
struct input_dev *input, u8 *data)
{
- /* 24 bits of buttons */
- u8 b8, b9, b10;
s16 x, y;
bool lpad_touched, lpad_and_joy;
- b8 = data[8];
- b9 = data[9];
- b10 = data[10];
-
input_report_abs(input, ABS_HAT2Y, data[11]);
input_report_abs(input, ABS_HAT2X, data[12]);
@@ -1451,8 +1508,8 @@ static void steam_do_input_event(struct steam_device *steam,
* joystick values.
* (lpad_touched || lpad_and_joy) tells if the lpad is really touched.
*/
- lpad_touched = b10 & BIT(3);
- lpad_and_joy = b10 & BIT(7);
+ lpad_touched = data[10] & BIT(3);
+ lpad_and_joy = data[10] & BIT(7);
x = steam_le16(data + 16);
y = -steam_le16(data + 18);
@@ -1468,31 +1525,10 @@ static void steam_do_input_event(struct steam_device *steam,
input_report_abs(input, ABS_HAT0X, 0);
input_report_abs(input, ABS_HAT0Y, 0);
}
+ input_report_key(input, BTN_THUMB, lpad_touched || lpad_and_joy);
- input_report_abs(input, ABS_RX, steam_le16(data + 20));
- input_report_abs(input, ABS_RY, -steam_le16(data + 22));
-
- input_event(input, EV_KEY, BTN_TR2, !!(b8 & BIT(0)));
- input_event(input, EV_KEY, BTN_TL2, !!(b8 & BIT(1)));
- input_event(input, EV_KEY, BTN_TR, !!(b8 & BIT(2)));
- input_event(input, EV_KEY, BTN_TL, !!(b8 & BIT(3)));
- input_event(input, EV_KEY, BTN_Y, !!(b8 & BIT(4)));
- input_event(input, EV_KEY, BTN_B, !!(b8 & BIT(5)));
- input_event(input, EV_KEY, BTN_X, !!(b8 & BIT(6)));
- input_event(input, EV_KEY, BTN_A, !!(b8 & BIT(7)));
- input_event(input, EV_KEY, BTN_SELECT, !!(b9 & BIT(4)));
- input_event(input, EV_KEY, BTN_MODE, !!(b9 & BIT(5)));
- input_event(input, EV_KEY, BTN_START, !!(b9 & BIT(6)));
- input_event(input, EV_KEY, BTN_GRIPL, !!(b9 & BIT(7)));
- input_event(input, EV_KEY, BTN_GRIPR, !!(b10 & BIT(0)));
- input_event(input, EV_KEY, BTN_THUMBR, !!(b10 & BIT(2)));
- input_event(input, EV_KEY, BTN_THUMBL, !!(b10 & BIT(6)));
- input_event(input, EV_KEY, BTN_THUMB, lpad_touched || lpad_and_joy);
- input_event(input, EV_KEY, BTN_THUMB2, !!(b10 & BIT(4)));
- input_event(input, EV_KEY, BTN_DPAD_UP, !!(b9 & BIT(0)));
- input_event(input, EV_KEY, BTN_DPAD_RIGHT, !!(b9 & BIT(1)));
- input_event(input, EV_KEY, BTN_DPAD_LEFT, !!(b9 & BIT(2)));
- input_event(input, EV_KEY, BTN_DPAD_DOWN, !!(b9 & BIT(3)));
+ steam_map_buttons(input, steam_controller_button_mappings, data);
+ steam_map_axes(input, steam_controller_axis_mappings, data);
input_sync(input);
}
@@ -1595,23 +1631,67 @@ static void steam_do_input_event(struct steam_device *steam,
* 15.6 | -- | unknown
* 15.7 | -- | unknown
*/
+
+static const struct steam_button_mapping steam_deck_button_mappings[] = {
+ { BTN_TR2, 8, 0 },
+ { BTN_TL2, 8, 1 },
+ { BTN_TR, 8, 2 },
+ { BTN_TL, 8, 3 },
+ { BTN_Y, 8, 4 },
+ { BTN_B, 8, 5 },
+ { BTN_X, 8, 6 },
+ { BTN_A, 8, 7 },
+ { BTN_SELECT, 9, 4 },
+ { BTN_MODE, 9, 5 },
+ { BTN_START, 9, 6 },
+ { BTN_GRIPL2, 9, 7 },
+ { BTN_GRIPR2, 10, 0 },
+ { BTN_THUMBL, 10, 6 },
+ { BTN_THUMBR, 11, 2 },
+ { BTN_DPAD_UP, 9, 0 },
+ { BTN_DPAD_RIGHT, 9, 1 },
+ { BTN_DPAD_LEFT, 9, 2 },
+ { BTN_DPAD_DOWN, 9, 3 },
+ { BTN_THUMB, 10, 1 },
+ { BTN_THUMB2, 10, 2 },
+ { BTN_GRIPL, 13, 1 },
+ { BTN_GRIPR, 13, 2 },
+ { BTN_BASE, 14, 2 },
+ { /* sentinel */ },
+};
+
+static const struct steam_axis_mapping steam_deck_axis_mappings[] = {
+ { ABS_X, 1, 48 },
+ { ABS_Y, -1, 50 },
+ { ABS_RX, 1, 52 },
+ { ABS_RY, -1, 54 },
+ { ABS_HAT2Y, 1, 44 },
+ { ABS_HAT2X, 1, 46 },
+ { /* sentinel */ },
+};
+
+static const struct steam_axis_mapping steam_deck_imu_mappings[] = {
+ { ABS_X, 1, 24 },
+ { ABS_Z, -1, 26 },
+ { ABS_Y, 1, 28 },
+ { ABS_RX, 1, 30 },
+ { ABS_RZ, -1, 32 },
+ { ABS_RY, 1, 34 },
+ { /* sentinel */ },
+};
+
static void steam_do_deck_input_event(struct steam_device *steam,
struct input_dev *input, u8 *data)
{
- u8 b8, b9, b10, b11, b13, b14;
+ bool start_pressed;
bool lpad_touched, rpad_touched;
- b8 = data[8];
- b9 = data[9];
- b10 = data[10];
- b11 = data[11];
- b13 = data[13];
- b14 = data[14];
+ start_pressed = data[9] & BIT(6);
- if (!(b9 & BIT(6)) && steam->did_mode_switch) {
+ if (!start_pressed && steam->did_mode_switch) {
steam->did_mode_switch = false;
cancel_delayed_work(&steam->mode_switch);
- } else if (!steam->client_opened && (b9 & BIT(6)) && !steam->did_mode_switch) {
+ } else if (!steam->client_opened && start_pressed && !steam->did_mode_switch) {
steam->did_mode_switch = true;
schedule_delayed_work(&steam->mode_switch, 45 * HZ / 100);
}
@@ -1619,8 +1699,8 @@ static void steam_do_deck_input_event(struct steam_device *steam,
if (!steam->gamepad_mode && lizard_mode)
return;
- lpad_touched = b10 & BIT(3);
- rpad_touched = b10 & BIT(4);
+ lpad_touched = data[10] & BIT(3);
+ rpad_touched = data[10] & BIT(4);
if (lpad_touched) {
input_report_abs(input, ABS_HAT0X, steam_le16(data + 16));
@@ -1638,38 +1718,8 @@ static void steam_do_deck_input_event(struct steam_device *steam,
input_report_abs(input, ABS_HAT1Y, 0);
}
- input_report_abs(input, ABS_X, steam_le16(data + 48));
- input_report_abs(input, ABS_Y, -steam_le16(data + 50));
- input_report_abs(input, ABS_RX, steam_le16(data + 52));
- input_report_abs(input, ABS_RY, -steam_le16(data + 54));
-
- input_report_abs(input, ABS_HAT2Y, steam_le16(data + 44));
- input_report_abs(input, ABS_HAT2X, steam_le16(data + 46));
-
- input_event(input, EV_KEY, BTN_TR2, !!(b8 & BIT(0)));
- input_event(input, EV_KEY, BTN_TL2, !!(b8 & BIT(1)));
- input_event(input, EV_KEY, BTN_TR, !!(b8 & BIT(2)));
- input_event(input, EV_KEY, BTN_TL, !!(b8 & BIT(3)));
- input_event(input, EV_KEY, BTN_Y, !!(b8 & BIT(4)));
- input_event(input, EV_KEY, BTN_B, !!(b8 & BIT(5)));
- input_event(input, EV_KEY, BTN_X, !!(b8 & BIT(6)));
- input_event(input, EV_KEY, BTN_A, !!(b8 & BIT(7)));
- input_event(input, EV_KEY, BTN_SELECT, !!(b9 & BIT(4)));
- input_event(input, EV_KEY, BTN_MODE, !!(b9 & BIT(5)));
- input_event(input, EV_KEY, BTN_START, !!(b9 & BIT(6)));
- input_event(input, EV_KEY, BTN_GRIPL2, !!(b9 & BIT(7)));
- input_event(input, EV_KEY, BTN_GRIPR2, !!(b10 & BIT(0)));
- input_event(input, EV_KEY, BTN_THUMBL, !!(b10 & BIT(6)));
- input_event(input, EV_KEY, BTN_THUMBR, !!(b11 & BIT(2)));
- input_event(input, EV_KEY, BTN_DPAD_UP, !!(b9 & BIT(0)));
- input_event(input, EV_KEY, BTN_DPAD_RIGHT, !!(b9 & BIT(1)));
- input_event(input, EV_KEY, BTN_DPAD_LEFT, !!(b9 & BIT(2)));
- input_event(input, EV_KEY, BTN_DPAD_DOWN, !!(b9 & BIT(3)));
- input_event(input, EV_KEY, BTN_THUMB, !!(b10 & BIT(1)));
- input_event(input, EV_KEY, BTN_THUMB2, !!(b10 & BIT(2)));
- input_event(input, EV_KEY, BTN_GRIPL, !!(b13 & BIT(1)));
- input_event(input, EV_KEY, BTN_GRIPR, !!(b13 & BIT(2)));
- input_event(input, EV_KEY, BTN_BASE, !!(b14 & BIT(2)));
+ steam_map_buttons(input, steam_deck_button_mappings, data);
+ steam_map_axes(input, steam_deck_axis_mappings, data);
input_sync(input);
}
@@ -1690,12 +1740,7 @@ static void steam_do_deck_sensors_event(struct steam_device *steam,
return;
input_event(sensors, EV_MSC, MSC_TIMESTAMP, steam->sensor_timestamp_us);
- input_report_abs(sensors, ABS_X, steam_le16(data + 24));
- input_report_abs(sensors, ABS_Z, -steam_le16(data + 26));
- input_report_abs(sensors, ABS_Y, steam_le16(data + 28));
- input_report_abs(sensors, ABS_RX, steam_le16(data + 30));
- input_report_abs(sensors, ABS_RZ, -steam_le16(data + 32));
- input_report_abs(sensors, ABS_RY, steam_le16(data + 34));
+ steam_map_axes(sensors, steam_deck_imu_mappings, data);
input_sync(sensors);
}
--
2.54.0
^ permalink raw reply related
* [PATCH v2 03/10] HID: steam: Rename some constants that got renamed upstream
From: Vicki Pfau @ 2026-07-07 23:32 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, linux-input; +Cc: Vicki Pfau, Yousef Alhouseen
In-Reply-To: <20260707233241.2644971-1-vi@endrift.com>
SETTING_MOUSE_POINTER_ENABLED was renamed to SETTING_LIZARD_MODE upstream.
SETTING_GYRO_MODE was renamed to SETTING_IMU_MODE in an older commit, but
the associated enum was overlooked.
Signed-off-by: Vicki Pfau <vi@endrift.com>
---
drivers/hid/hid-steam.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/hid/hid-steam.c b/drivers/hid/hid-steam.c
index 1b0367c98eac..add9f4f27fff 100644
--- a/drivers/hid/hid-steam.c
+++ b/drivers/hid/hid-steam.c
@@ -151,7 +151,7 @@ enum {
SETTING_USB_DEBUG_MODE,
SETTING_LEFT_TRACKPAD_MODE,
SETTING_RIGHT_TRACKPAD_MODE,
- SETTING_MOUSE_POINTER_ENABLED,
+ SETTING_LIZARD_MODE,
/* 10 */
SETTING_DPAD_DEADZONE,
@@ -261,14 +261,14 @@ enum {
ATTRIB_STR_UNIT_SERIAL,
};
-/* Values for GYRO_MODE (bitmask) */
+/* Values for IMU_MODE (bitmask) */
enum {
- SETTING_GYRO_MODE_OFF = 0,
- SETTING_GYRO_MODE_STEERING = BIT(0),
- SETTING_GYRO_MODE_TILT = BIT(1),
- SETTING_GYRO_MODE_SEND_ORIENTATION = BIT(2),
- SETTING_GYRO_MODE_SEND_RAW_ACCEL = BIT(3),
- SETTING_GYRO_MODE_SEND_RAW_GYRO = BIT(4),
+ SETTING_IMU_MODE_OFF = 0,
+ SETTING_IMU_MODE_STEERING = BIT(0),
+ SETTING_IMU_MODE_TILT = BIT(1),
+ SETTING_IMU_MODE_SEND_ORIENTATION = BIT(2),
+ SETTING_IMU_MODE_SEND_RAW_ACCEL = BIT(3),
+ SETTING_IMU_MODE_SEND_RAW_GYRO = BIT(4),
};
/* Trackpad modes */
--
2.54.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