* Re: [RFC] Input: matrix_keypad: fix interrupt regression introduced by commit 01c84b03d80a
From: Siarhei Volkau @ 2026-07-08 15:59 UTC (permalink / raw)
To: Alexander Kochetkov
Cc: linux-input, Dmitry Torokhov, linux-kernel, Paul Cercueil
In-Reply-To: <4130C026-2A83-408B-836A-ECA044491C47@gmail.com>
Regarding JZ4755, I agree that the [1] patch can be reverted.
No device requiring this workaround has landed in the mainline.
qi,lb60 (Ben Nanonote) seems unaffected as it was there before
the patch was proposed.
However, if there are genuinely two hardware operating modes:
- "GPIO input mode" while scanning
- "GPIO interrupt input mode" while idle
then the keypad driver should be aware of this distinction.
I'd like to propose pinctrl state transitions as the mechanism for switching
these modes, wherever it makes sense (Allwinner et al).
CC Paul as the maintainer of Ingenic pinctrl driver.
BR,
Siarhei
ср, 8 июл. 2026 г. в 16:30, Alexander Kochetkov <al.kochet@gmail.com>:
>
> 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 v3 1/2] HID: sensor: custom: Fix use-after-free in enable_sensor
From: srinivas pandruvada @ 2026-07-08 17:58 UTC (permalink / raw)
To: Haoxiang Li, jikos, jic23, bentiss
Cc: linux-input, linux-iio, linux-kernel, Sashiko AI Review, stable
In-Reply-To: <20260707071545.3087073-2-haoxiang_li2024@163.com>
On Tue, 2026-07-07 at 15:15 +0800, Haoxiang Li wrote:
> enable_sensor_store() can call set_power_report_state(), which
> dereferences sensor_inst->power_state and sensor_inst->report_state.
> These pointers refer to entries in sensor_inst->fields.
>
> Create the field attributes before exposing the enable_sensor sysfs
> attribute, so enable_sensor cannot be accessed before the state it
> depends on has been initialized.
>
> On remove, delete enable_sensor before freeing the field attributes,
> so a concurrent sysfs write cannot dereference freed memory through
> power_state or report_state.
>
> Reported-by: Sashiko AI Review <sashiko-bot@kernel.org>
> Link:
> https://sashiko.dev/#/patchset/20260623021950.1736413-1-haoxiang_li2024@163.com?part=1
> Fixes: 4a7de0519df5 ("HID: sensor: Custom and Generic sensor
> support")
> Cc: stable@vger.kernel.org
> Signed-off-by: Haoxiang Li <haoxiang_li2024@163.com>
Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> ---
> drivers/hid/hid-sensor-custom.c | 17 +++++++++--------
> 1 file changed, 9 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/hid/hid-sensor-custom.c b/drivers/hid/hid-
> sensor-custom.c
> index afffea894021..6b0da2e0e1c9 100644
> --- a/drivers/hid/hid-sensor-custom.c
> +++ b/drivers/hid/hid-sensor-custom.c
> @@ -1005,26 +1005,26 @@ static int hid_sensor_custom_probe(struct
> platform_device *pdev)
> return ret;
> }
>
> - ret = sysfs_create_group(&sensor_inst->pdev->dev.kobj,
> - &enable_sensor_attr_group);
> + ret = hid_sensor_custom_add_attributes(sensor_inst);
> if (ret)
> goto err_remove_callback;
>
> - ret = hid_sensor_custom_add_attributes(sensor_inst);
> + ret = sysfs_create_group(&sensor_inst->pdev->dev.kobj,
> + &enable_sensor_attr_group);
> if (ret)
> - goto err_remove_group;
> + goto err_remove_attributes;
>
> ret = hid_sensor_custom_dev_if_add(sensor_inst);
> if (ret)
> - goto err_remove_attributes;
> + goto err_remove_group;
>
> return 0;
>
> -err_remove_attributes:
> - hid_sensor_custom_remove_attributes(sensor_inst);
> err_remove_group:
> sysfs_remove_group(&sensor_inst->pdev->dev.kobj,
> &enable_sensor_attr_group);
> +err_remove_attributes:
> + hid_sensor_custom_remove_attributes(sensor_inst);
> err_remove_callback:
> sensor_hub_remove_callback(hsdev, hsdev->usage);
>
> @@ -1042,9 +1042,10 @@ static void hid_sensor_custom_remove(struct
> platform_device *pdev)
> }
>
> hid_sensor_custom_dev_if_remove(sensor_inst);
> - hid_sensor_custom_remove_attributes(sensor_inst);
> + /* Remove enable_sensor first as it uses fields via
> power_state/report_state. */
> sysfs_remove_group(&sensor_inst->pdev->dev.kobj,
> &enable_sensor_attr_group);
> + hid_sensor_custom_remove_attributes(sensor_inst);
> sensor_hub_remove_callback(hsdev, hsdev->usage);
> }
>
^ permalink raw reply
* Re: [PATCH v3 2/2] HID: sensor: custom: Fix field sysfs group cleanup on failure
From: srinivas pandruvada @ 2026-07-08 17:58 UTC (permalink / raw)
To: Haoxiang Li, jikos, jic23, bentiss
Cc: linux-input, linux-iio, linux-kernel, stable
In-Reply-To: <20260707071545.3087073-3-haoxiang_li2024@163.com>
On Tue, 2026-07-07 at 15:15 +0800, Haoxiang Li wrote:
> hid_sensor_custom_add_attributes() creates one sysfs group for each
> custom sensor field. If sysfs_create_group() fails after some groups
> have already been created, the function returns the error without
> removing the previously created groups.
>
> Add a local unwind path to remove the groups that were already
> created.
> With enable_sensor exposed only after the field attributes are ready,
> this path can free sensor_inst->fields without leaving enable_sensor
> able to access pointers into that array.
>
> Fixes: 4a7de0519df5 ("HID: sensor: Custom and Generic sensor
> support")
> Cc: stable@vger.kernel.org
> Signed-off-by: Haoxiang Li <haoxiang_li2024@163.com>
Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> ---
> drivers/hid/hid-sensor-custom.c | 9 ++++++++-
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/hid/hid-sensor-custom.c b/drivers/hid/hid-
> sensor-custom.c
> index 6b0da2e0e1c9..c2b425afd951 100644
> --- a/drivers/hid/hid-sensor-custom.c
> +++ b/drivers/hid/hid-sensor-custom.c
> @@ -609,7 +609,7 @@ static int
> hid_sensor_custom_add_attributes(struct hid_sensor_custom
> &sensor_inst->fields[i].
>
> hid_custom_attribute_group);
> if (ret)
> - break;
> + goto err_remove_groups;
>
> /* For power or report field store indexes */
> if (sensor_inst->fields[i].attribute.attrib_id ==
> @@ -621,6 +621,13 @@ static int
> hid_sensor_custom_add_attributes(struct hid_sensor_custom
> }
>
> return ret;
> +
> +err_remove_groups:
> + while (--i >= 0)
> + sysfs_remove_group(&sensor_inst->pdev->dev.kobj,
> + &sensor_inst-
> >fields[i].hid_custom_attribute_group);
> + kfree(sensor_inst->fields);
> + return ret;
> }
>
> static void hid_sensor_custom_remove_attributes(struct
> hid_sensor_custom *
^ permalink raw reply
* Re: [PATCH v4 1/3] HID: wacom: Fix Use-After-Free in wacom_intuos_pad
From: Lee Jones @ 2026-07-08 18:41 UTC (permalink / raw)
To: Jason Gerecke
Cc: Ping Cheng, Jason Gerecke, Jiri Kosina, Benjamin Tissoires,
Peter Hutterer, Dmitry Torokhov, linux-input, linux-kernel
In-Reply-To: <CANRwn3SCQ58hLg00NU844nNxR2B5+f6UvuOYRgXR3x6q96FSgw@mail.gmail.com>
On Tue, 30 Jun 2026, Jason Gerecke wrote:
> I'm still working on testing and reviewing this set as a whole, but I
> can provide some immediate feedback on this first patch at least:
v5 posted. I look forward to your review.
--
Lee Jones
^ permalink raw reply
* Re: [PATCH v11 1/3] HID: nintendo: Add preliminary Switch 2 controller driver
From: Silvan Jegen @ 2026-07-08 20:08 UTC (permalink / raw)
To: Vicki Pfau; +Cc: Dmitry Torokhov, Jiri Kosina, Benjamin Tissoires, linux-input
In-Reply-To: <6e649d0b-a86c-4d63-859b-7fd89e71081f@endrift.com>
Vicki Pfau <vi@endrift.com> wrote:
> 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.
Then let's leave it as is!
> >
> >> +#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.
Works for me!
>
> >
> >> +#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.
It would need some kind of malicious or malfunctioning device reporting
these, so I don't think the risk is very high in practice.
>
> >
> >> + 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.
Fair enough!
>
> >
> >> + 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
If reducing the log spam is the goal, this approach seems fine to me.
> 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.
Right! When length is 0, no byte will be accessed anymore anyways and
this is fine. Please disregard!
>
> >
> >
> >> + 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.
It is somewhat confusing for the reader of the code, but I'm fine
either way.
>
> >
> >> + 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.
Thanks for the explanation!
>
> >
> >> + 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
with this, this series is
Reviewed-by: Silvan Jegen <s.jegen@gmail.com>
I have also tested the Switch 2 Pro controller functionality with
"evtest". So that part is also
Tested-by: Silvan Jegen <s.jegen@gmail.com>
I have tried to test the force-feedback functionality with "fftest" as
well. I only had partial success, but I assume this is due to my lack
of knowledge about how force-feedback and/or "fftest" works on Linux.
Cheers,
Silvan
^ permalink raw reply
* Re: [PATCH v2] iio: temperature: hid-sensor-temperature: switch to non-devm iio_device_register()
From: srinivas pandruvada @ 2026-07-08 23:25 UTC (permalink / raw)
To: Sanjay Chitroda, jikos, jic23
Cc: dlechner, nuno.sa, andy, hongyan.song, linux-input, linux-iio,
linux-kernel
In-Reply-To: <20260625121611.2003780-1-sanjayembedded@gmail.com>
On Thu, 2026-06-25 at 17:46 +0530, Sanjay Chitroda 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>
>
Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> ---
> 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
* [PATCH v6 0/9] HID: steelseries: split out Arctis driver and add Nova 5X/Nova 7 support
From: Sriman Achanta @ 2026-07-09 3:23 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: linux-input, linux-kernel, Bastien Nocera, Simon Wood,
Christian Mayer, Sriman Achanta
From: Sriman Achanta <srimanachanta@gmail.com>
This series moves the SteelSeries Arctis headset support out of
hid-steelseries.c into its own module, reworks it around a per-model
device_info table, hardens the battery power-supply lifetime, adds an
asynchronous status interface, and finally adds support for the Arctis
Nova 5X and Nova 7 families. After the split, hid-steelseries.c is left
handling only the SRW-S1 racing wheel.
Changes since v5:
- Nova: rename the Gen2-only parse function and device_info to
steelseries_arctis_nova_{parse_status,info} now that they are also
used by the Nova 5X and the 2026 refresh, not just Gen2.
- Nova: add device IDs and a device_info for the original (pre-Gen2)
Arctis Nova 7 family. It answers the same 0xb0/0xb7/0xb9/0xbb status
opcodes on the same interfaces as Gen2, but reports battery capacity
as a discrete 0-4 level rather than a raw percentage, so it gets its
own parse function that maps the level through
steelseries_map_capacity() instead of using it directly.
- Nova: add the Arctis Nova 5X, which shares the Gen2/2026 protocol.
- hid-ids.h: fix a whitespace-only bug where the new Nova 5X macro was
space-aligned instead of tab-aligned.
- Rebased on current master.
The Nova 5X protocol was verified against hardware by
sp4m.an.joker@gmail.com, and the original Nova 7 protocol was verified
against hardware by colin@1101b.com. Thanks to both for testing. I
previously ommited other devices in the v4 patch series due to being
unable to verify.
Sriman Achanta (9):
HID: steelseries: Fix ARCTIS_1_X device mislabeling
HID: steelseries: Split Arctis headset driver into separate module
HID: steelseries: Inline and simplify SRWS1 wheel driver
HID: steelseries: Refactor Arctis driver to use device_info framework
HID: steelseries: Report POWER_SUPPLY_STATUS_FULL when full
HID: steelseries: Correct Arctis 9 battery calibration range
HID: steelseries: Manage battery lifetime with refcounting
HID: steelseries: Add async status interface support
HID: steelseries: Add support for Arctis Nova 5X and Nova 7 families
drivers/hid/Makefile | 2 +-
drivers/hid/hid-ids.h | 18 +-
drivers/hid/hid-quirks.c | 16 +-
drivers/hid/hid-steelseries-arctis.c | 718 +++++++++++++++++++++++++++
drivers/hid/hid-steelseries.c | 406 +--------------
5 files changed, 759 insertions(+), 401 deletions(-)
create mode 100644 drivers/hid/hid-steelseries-arctis.c
base-commit: 7404ce51637231382873d0b55edabc2f3b841a9d
--
2.55.0
^ permalink raw reply
* [PATCH v6 1/9] HID: steelseries: Fix ARCTIS_1_X device mislabeling
From: Sriman Achanta @ 2026-07-09 3:23 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: linux-input, linux-kernel, Bastien Nocera, Simon Wood,
Christian Mayer, Sriman Achanta
In-Reply-To: <20260709032316.1908460-1-68172138+srimanachanta@users.noreply.github.com>
From: Sriman Achanta <srimanachanta@gmail.com>
The SteelSeries Arctis 1 Wireless for Xbox (0x12b6) was labelled as the
plain Arctis 1 Wireless. Rename USB_DEVICE_ID_STEELSERIES_ARCTIS_1 to
USB_DEVICE_ID_STEELSERIES_ARCTIS_1_X, along with the matching quirk flag
and device table entry. The device ID value is unchanged.
Signed-off-by: Sriman Achanta <srimanachanta@gmail.com>
---
drivers/hid/hid-ids.h | 4 ++--
drivers/hid/hid-quirks.c | 2 +-
drivers/hid/hid-steelseries.c | 10 +++++-----
3 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 1059922baaac..915e936cbf8b 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -1377,8 +1377,8 @@
#define USB_VENDOR_ID_STEELSERIES 0x1038
#define USB_DEVICE_ID_STEELSERIES_SRWS1 0x1410
-#define USB_DEVICE_ID_STEELSERIES_ARCTIS_1 0x12b6
-#define USB_DEVICE_ID_STEELSERIES_ARCTIS_9 0x12c2
+#define USB_DEVICE_ID_STEELSERIES_ARCTIS_1_X 0x12b6
+#define USB_DEVICE_ID_STEELSERIES_ARCTIS_9 0x12c2
#define USB_VENDOR_ID_SUN 0x0430
#define USB_DEVICE_ID_RARITAN_KVM_DONGLE 0xcdab
diff --git a/drivers/hid/hid-quirks.c b/drivers/hid/hid-quirks.c
index 57d8efdd9b89..f546179858c2 100644
--- a/drivers/hid/hid-quirks.c
+++ b/drivers/hid/hid-quirks.c
@@ -747,7 +747,7 @@ static const struct hid_device_id hid_have_special_driver[] = {
#endif
#if IS_ENABLED(CONFIG_HID_STEELSERIES)
{ HID_USB_DEVICE(USB_VENDOR_ID_STEELSERIES, USB_DEVICE_ID_STEELSERIES_SRWS1) },
- { HID_USB_DEVICE(USB_VENDOR_ID_STEELSERIES, USB_DEVICE_ID_STEELSERIES_ARCTIS_1) },
+ { HID_USB_DEVICE(USB_VENDOR_ID_STEELSERIES, USB_DEVICE_ID_STEELSERIES_ARCTIS_1_X) },
{ HID_USB_DEVICE(USB_VENDOR_ID_STEELSERIES, USB_DEVICE_ID_STEELSERIES_ARCTIS_9) },
#endif
#if IS_ENABLED(CONFIG_HID_SUNPLUS)
diff --git a/drivers/hid/hid-steelseries.c b/drivers/hid/hid-steelseries.c
index f98435631aa1..fd38ee3ea6fc 100644
--- a/drivers/hid/hid-steelseries.c
+++ b/drivers/hid/hid-steelseries.c
@@ -18,7 +18,7 @@
#include "hid-ids.h"
#define STEELSERIES_SRWS1 BIT(0)
-#define STEELSERIES_ARCTIS_1 BIT(1)
+#define STEELSERIES_ARCTIS_1_X BIT(1)
#define STEELSERIES_ARCTIS_9 BIT(2)
struct steelseries_device {
@@ -374,7 +374,7 @@ static void steelseries_headset_fetch_battery(struct hid_device *hdev)
{
int ret = 0;
- if (hdev->product == USB_DEVICE_ID_STEELSERIES_ARCTIS_1)
+ if (hdev->product == USB_DEVICE_ID_STEELSERIES_ARCTIS_1_X)
ret = steelseries_headset_request_battery(hdev,
arctis_1_battery_request, sizeof(arctis_1_battery_request));
else if (hdev->product == USB_DEVICE_ID_STEELSERIES_ARCTIS_9)
@@ -638,7 +638,7 @@ static int steelseries_headset_raw_event(struct hid_device *hdev,
if (hdev->product == USB_DEVICE_ID_STEELSERIES_SRWS1)
return 0;
- if (hdev->product == USB_DEVICE_ID_STEELSERIES_ARCTIS_1) {
+ if (hdev->product == USB_DEVICE_ID_STEELSERIES_ARCTIS_1_X) {
hid_dbg(sd->hdev,
"Parsing raw event for Arctis 1 headset (%*ph)\n", size, read_buf);
if (size < ARCTIS_1_BATTERY_RESPONSE_LEN ||
@@ -725,8 +725,8 @@ static const struct hid_device_id steelseries_devices[] = {
.driver_data = STEELSERIES_SRWS1 },
{ /* SteelSeries Arctis 1 Wireless for XBox */
- HID_USB_DEVICE(USB_VENDOR_ID_STEELSERIES, USB_DEVICE_ID_STEELSERIES_ARCTIS_1),
- .driver_data = STEELSERIES_ARCTIS_1 },
+ HID_USB_DEVICE(USB_VENDOR_ID_STEELSERIES, USB_DEVICE_ID_STEELSERIES_ARCTIS_1_X),
+ .driver_data = STEELSERIES_ARCTIS_1_X },
{ /* SteelSeries Arctis 9 Wireless for XBox */
HID_USB_DEVICE(USB_VENDOR_ID_STEELSERIES, USB_DEVICE_ID_STEELSERIES_ARCTIS_9),
--
2.55.0
^ permalink raw reply related
* [PATCH v6 2/9] HID: steelseries: Split Arctis headset driver into separate module
From: Sriman Achanta @ 2026-07-09 3:23 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: linux-input, linux-kernel, Bastien Nocera, Simon Wood,
Christian Mayer, Sriman Achanta
In-Reply-To: <20260709032316.1908460-1-68172138+srimanachanta@users.noreply.github.com>
From: Sriman Achanta <srimanachanta@gmail.com>
Move all Arctis headset code (battery monitoring, wireless status,
power supply registration, raw event handling) from hid-steelseries.c
into the new hid-steelseries-arctis.c driver module.
hid-steelseries.c now handles only the SRWS1 racing wheel, while
hid-steelseries-arctis.c handles the Arctis 1 (Xbox) and Arctis 9
wireless headsets with their own device table, probe, remove, and
raw_event implementations.
Signed-off-by: Sriman Achanta <srimanachanta@gmail.com>
---
drivers/hid/Makefile | 2 +-
drivers/hid/hid-steelseries-arctis.c | 404 +++++++++++++++++++++++++++
drivers/hid/hid-steelseries.c | 382 +------------------------
3 files changed, 408 insertions(+), 380 deletions(-)
create mode 100644 drivers/hid/hid-steelseries-arctis.c
diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
index 23e6e3dd0c56..4a172bd27b11 100644
--- a/drivers/hid/Makefile
+++ b/drivers/hid/Makefile
@@ -134,7 +134,7 @@ obj-$(CONFIG_HID_SMARTJOYPLUS) += hid-sjoy.o
obj-$(CONFIG_HID_SONY) += hid-sony.o
obj-$(CONFIG_HID_SPEEDLINK) += hid-speedlink.o
obj-$(CONFIG_HID_STEAM) += hid-steam.o
-obj-$(CONFIG_HID_STEELSERIES) += hid-steelseries.o
+obj-$(CONFIG_HID_STEELSERIES) += hid-steelseries.o hid-steelseries-arctis.o
obj-$(CONFIG_HID_SUNPLUS) += hid-sunplus.o
obj-$(CONFIG_HID_GREENASIA) += hid-gaff.o
obj-$(CONFIG_HID_THRUSTMASTER) += hid-tmff.o hid-thrustmaster.o
diff --git a/drivers/hid/hid-steelseries-arctis.c b/drivers/hid/hid-steelseries-arctis.c
new file mode 100644
index 000000000000..079504e6932a
--- /dev/null
+++ b/drivers/hid/hid-steelseries-arctis.c
@@ -0,0 +1,404 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * HID driver for Steelseries arctis headsets
+ *
+ * Copyright (c) 2023 Bastien Nocera
+ */
+
+#include <linux/device.h>
+#include <linux/hid.h>
+#include <linux/module.h>
+#include <linux/power_supply.h>
+#include <linux/spinlock.h>
+#include <linux/usb.h>
+#include <linux/workqueue.h>
+
+#include "hid-ids.h"
+
+#define STEELSERIES_ARCTIS_1_X BIT(0)
+#define STEELSERIES_ARCTIS_9 BIT(1)
+
+struct steelseries_device {
+ struct hid_device *hdev;
+ unsigned long quirks;
+
+ struct delayed_work battery_work;
+ spinlock_t lock;
+ bool removed;
+
+ struct power_supply_desc battery_desc;
+ struct power_supply *battery;
+ uint8_t battery_capacity;
+ bool headset_connected;
+ bool battery_charging;
+};
+
+#define STEELSERIES_HEADSET_BATTERY_TIMEOUT_MS 3000
+
+#define ARCTIS_1_BATTERY_RESPONSE_LEN 8
+#define ARCTIS_9_BATTERY_RESPONSE_LEN 64
+static const char arctis_1_battery_request[] = { 0x06, 0x12 };
+static const char arctis_9_battery_request[] = { 0x00, 0x20 };
+
+static int steelseries_headset_request_battery(struct hid_device *hdev,
+ const char *request, size_t len)
+{
+ u8 *write_buf;
+ int ret;
+
+ /* Request battery information */
+ write_buf = kmemdup(request, len, GFP_KERNEL);
+ if (!write_buf)
+ return -ENOMEM;
+
+ hid_dbg(hdev, "Sending battery request report");
+ ret = hid_hw_raw_request(hdev, request[0], write_buf, len,
+ HID_OUTPUT_REPORT, HID_REQ_SET_REPORT);
+ if (ret < (int)len) {
+ hid_err(hdev, "hid_hw_raw_request() failed with %d\n", ret);
+ ret = -ENODATA;
+ }
+
+ kfree(write_buf);
+ return ret;
+}
+
+static void steelseries_headset_fetch_battery(struct hid_device *hdev)
+{
+ int ret = 0;
+
+ if (hdev->product == USB_DEVICE_ID_STEELSERIES_ARCTIS_1_X)
+ ret = steelseries_headset_request_battery(hdev,
+ arctis_1_battery_request, sizeof(arctis_1_battery_request));
+ else if (hdev->product == USB_DEVICE_ID_STEELSERIES_ARCTIS_9)
+ ret = steelseries_headset_request_battery(hdev,
+ arctis_9_battery_request, sizeof(arctis_9_battery_request));
+
+ if (ret < 0)
+ hid_dbg(hdev,
+ "Battery query failed (err: %d)\n", ret);
+}
+
+static int battery_capacity_to_level(int capacity)
+{
+ if (capacity >= 50)
+ return POWER_SUPPLY_CAPACITY_LEVEL_NORMAL;
+ if (capacity >= 20)
+ return POWER_SUPPLY_CAPACITY_LEVEL_LOW;
+ return POWER_SUPPLY_CAPACITY_LEVEL_CRITICAL;
+}
+
+static void steelseries_headset_battery_timer_tick(struct work_struct *work)
+{
+ struct steelseries_device *sd = container_of(work,
+ struct steelseries_device, battery_work.work);
+ struct hid_device *hdev = sd->hdev;
+
+ steelseries_headset_fetch_battery(hdev);
+}
+
+#define STEELSERIES_PREFIX "SteelSeries "
+#define STEELSERIES_PREFIX_LEN strlen(STEELSERIES_PREFIX)
+
+static int steelseries_headset_battery_get_property(struct power_supply *psy,
+ enum power_supply_property psp,
+ union power_supply_propval *val)
+{
+ struct steelseries_device *sd = power_supply_get_drvdata(psy);
+ int ret = 0;
+
+ switch (psp) {
+ case POWER_SUPPLY_PROP_MODEL_NAME:
+ val->strval = sd->hdev->name;
+ while (!strncmp(val->strval, STEELSERIES_PREFIX, STEELSERIES_PREFIX_LEN))
+ val->strval += STEELSERIES_PREFIX_LEN;
+ break;
+ case POWER_SUPPLY_PROP_MANUFACTURER:
+ val->strval = "SteelSeries";
+ break;
+ case POWER_SUPPLY_PROP_PRESENT:
+ val->intval = 1;
+ break;
+ case POWER_SUPPLY_PROP_STATUS:
+ if (sd->headset_connected) {
+ val->intval = sd->battery_charging ?
+ POWER_SUPPLY_STATUS_CHARGING :
+ POWER_SUPPLY_STATUS_DISCHARGING;
+ } else
+ val->intval = POWER_SUPPLY_STATUS_UNKNOWN;
+ break;
+ case POWER_SUPPLY_PROP_SCOPE:
+ val->intval = POWER_SUPPLY_SCOPE_DEVICE;
+ break;
+ case POWER_SUPPLY_PROP_CAPACITY:
+ val->intval = sd->battery_capacity;
+ break;
+ case POWER_SUPPLY_PROP_CAPACITY_LEVEL:
+ val->intval = battery_capacity_to_level(sd->battery_capacity);
+ break;
+ default:
+ ret = -EINVAL;
+ break;
+ }
+ return ret;
+}
+
+static void
+steelseries_headset_set_wireless_status(struct hid_device *hdev,
+ bool connected)
+{
+ struct usb_interface *intf;
+
+ if (!hid_is_usb(hdev))
+ return;
+
+ intf = to_usb_interface(hdev->dev.parent);
+ usb_set_wireless_status(intf, connected ?
+ USB_WIRELESS_STATUS_CONNECTED :
+ USB_WIRELESS_STATUS_DISCONNECTED);
+}
+
+static enum power_supply_property steelseries_headset_battery_props[] = {
+ POWER_SUPPLY_PROP_MODEL_NAME,
+ POWER_SUPPLY_PROP_MANUFACTURER,
+ POWER_SUPPLY_PROP_PRESENT,
+ POWER_SUPPLY_PROP_STATUS,
+ POWER_SUPPLY_PROP_SCOPE,
+ POWER_SUPPLY_PROP_CAPACITY,
+ POWER_SUPPLY_PROP_CAPACITY_LEVEL,
+};
+
+static int steelseries_headset_battery_register(struct steelseries_device *sd)
+{
+ static atomic_t battery_no = ATOMIC_INIT(0);
+ struct power_supply_config battery_cfg = { .drv_data = sd, };
+ unsigned long n;
+ int ret;
+
+ sd->battery_desc.type = POWER_SUPPLY_TYPE_BATTERY;
+ sd->battery_desc.properties = steelseries_headset_battery_props;
+ sd->battery_desc.num_properties = ARRAY_SIZE(steelseries_headset_battery_props);
+ sd->battery_desc.get_property = steelseries_headset_battery_get_property;
+ sd->battery_desc.use_for_apm = 0;
+ n = atomic_inc_return(&battery_no) - 1;
+ sd->battery_desc.name = devm_kasprintf(&sd->hdev->dev, GFP_KERNEL,
+ "steelseries_headset_battery_%ld", n);
+ if (!sd->battery_desc.name)
+ return -ENOMEM;
+
+ /* avoid the warning of 0% battery while waiting for the first info */
+ steelseries_headset_set_wireless_status(sd->hdev, false);
+ sd->battery_capacity = 100;
+ sd->battery_charging = false;
+
+ sd->battery = devm_power_supply_register(&sd->hdev->dev,
+ &sd->battery_desc, &battery_cfg);
+ if (IS_ERR(sd->battery)) {
+ ret = PTR_ERR(sd->battery);
+ hid_err(sd->hdev,
+ "%s:power_supply_register failed with error %d\n",
+ __func__, ret);
+ return ret;
+ }
+ power_supply_powers(sd->battery, &sd->hdev->dev);
+
+ INIT_DELAYED_WORK(&sd->battery_work, steelseries_headset_battery_timer_tick);
+ steelseries_headset_fetch_battery(sd->hdev);
+
+ if (sd->quirks & STEELSERIES_ARCTIS_9) {
+ /* The first fetch_battery request can remain unanswered in some cases */
+ schedule_delayed_work(&sd->battery_work,
+ msecs_to_jiffies(STEELSERIES_HEADSET_BATTERY_TIMEOUT_MS));
+ }
+
+ return 0;
+}
+
+static bool steelseries_is_vendor_usage_page(struct hid_device *hdev, uint8_t usage_page)
+{
+ return hdev->rdesc[0] == 0x06 &&
+ hdev->rdesc[1] == usage_page &&
+ hdev->rdesc[2] == 0xff;
+}
+
+static int steelseries_arctis_probe(struct hid_device *hdev, const struct hid_device_id *id)
+{
+ struct steelseries_device *sd;
+ int ret;
+
+ sd = devm_kzalloc(&hdev->dev, sizeof(*sd), GFP_KERNEL);
+ if (!sd)
+ return -ENOMEM;
+ hid_set_drvdata(hdev, sd);
+ sd->hdev = hdev;
+ sd->quirks = id->driver_data;
+
+ ret = hid_parse(hdev);
+ if (ret)
+ return ret;
+
+ if (sd->quirks & STEELSERIES_ARCTIS_9 &&
+ !steelseries_is_vendor_usage_page(hdev, 0xc0))
+ return -ENODEV;
+
+ spin_lock_init(&sd->lock);
+
+ ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
+ if (ret)
+ return ret;
+
+ ret = hid_hw_open(hdev);
+ if (ret)
+ return ret;
+
+ if (steelseries_headset_battery_register(sd) < 0)
+ hid_err(sd->hdev,
+ "Failed to register battery for headset\n");
+
+ return ret;
+}
+
+static void steelseries_arctis_remove(struct hid_device *hdev)
+{
+ struct steelseries_device *sd;
+ unsigned long flags;
+
+ sd = hid_get_drvdata(hdev);
+
+ spin_lock_irqsave(&sd->lock, flags);
+ sd->removed = true;
+ spin_unlock_irqrestore(&sd->lock, flags);
+
+ cancel_delayed_work_sync(&sd->battery_work);
+
+ hid_hw_close(hdev);
+ hid_hw_stop(hdev);
+}
+
+static uint8_t steelseries_headset_map_capacity(uint8_t capacity, uint8_t min_in, uint8_t max_in)
+{
+ if (capacity >= max_in)
+ return 100;
+ if (capacity <= min_in)
+ return 0;
+ return (capacity - min_in) * 100 / (max_in - min_in);
+}
+
+static int steelseries_arctis_raw_event(struct hid_device *hdev,
+ struct hid_report *report, u8 *read_buf,
+ int size)
+{
+ struct steelseries_device *sd = hid_get_drvdata(hdev);
+ int capacity = sd->battery_capacity;
+ bool connected = sd->headset_connected;
+ bool charging = sd->battery_charging;
+ unsigned long flags;
+
+ if (hdev->product == USB_DEVICE_ID_STEELSERIES_ARCTIS_1_X) {
+ hid_dbg(sd->hdev,
+ "Parsing raw event for Arctis 1 headset (%*ph)\n", size, read_buf);
+ if (size < ARCTIS_1_BATTERY_RESPONSE_LEN ||
+ memcmp(read_buf, arctis_1_battery_request, sizeof(arctis_1_battery_request))) {
+ if (!delayed_work_pending(&sd->battery_work))
+ goto request_battery;
+ return 0;
+ }
+ if (read_buf[2] == 0x01) {
+ connected = false;
+ capacity = 100;
+ } else {
+ connected = true;
+ capacity = read_buf[3];
+ }
+ }
+
+ if (hdev->product == USB_DEVICE_ID_STEELSERIES_ARCTIS_9) {
+ hid_dbg(sd->hdev,
+ "Parsing raw event for Arctis 9 headset (%*ph)\n", size, read_buf);
+ if (size < ARCTIS_9_BATTERY_RESPONSE_LEN) {
+ if (!delayed_work_pending(&sd->battery_work))
+ goto request_battery;
+ return 0;
+ }
+
+ if (read_buf[0] == 0xaa && read_buf[1] == 0x01) {
+ connected = true;
+ charging = read_buf[4] == 0x01;
+
+ /*
+ * Found no official documentation about min and max.
+ * Values defined by testing.
+ */
+ capacity = steelseries_headset_map_capacity(read_buf[3], 0x68, 0x9d);
+ } else {
+ /*
+ * Device is off and sends the last known status read_buf[1] == 0x03 or
+ * there is no known status of the device read_buf[0] == 0x55
+ */
+ connected = false;
+ charging = false;
+ }
+ }
+
+ if (connected != sd->headset_connected) {
+ hid_dbg(sd->hdev,
+ "Connected status changed from %sconnected to %sconnected\n",
+ sd->headset_connected ? "" : "not ",
+ connected ? "" : "not ");
+ sd->headset_connected = connected;
+ steelseries_headset_set_wireless_status(hdev, connected);
+ }
+
+ if (capacity != sd->battery_capacity) {
+ hid_dbg(sd->hdev,
+ "Battery capacity changed from %d%% to %d%%\n",
+ sd->battery_capacity, capacity);
+ sd->battery_capacity = capacity;
+ power_supply_changed(sd->battery);
+ }
+
+ if (charging != sd->battery_charging) {
+ hid_dbg(sd->hdev,
+ "Battery charging status changed from %scharging to %scharging\n",
+ sd->battery_charging ? "" : "not ",
+ charging ? "" : "not ");
+ sd->battery_charging = charging;
+ power_supply_changed(sd->battery);
+ }
+
+request_battery:
+ spin_lock_irqsave(&sd->lock, flags);
+ if (!sd->removed)
+ schedule_delayed_work(&sd->battery_work,
+ msecs_to_jiffies(STEELSERIES_HEADSET_BATTERY_TIMEOUT_MS));
+ spin_unlock_irqrestore(&sd->lock, flags);
+
+ return 0;
+}
+
+static const struct hid_device_id steelseries_arctis_devices[] = {
+ { /* SteelSeries Arctis 1 Wireless for XBox */
+ HID_USB_DEVICE(USB_VENDOR_ID_STEELSERIES, USB_DEVICE_ID_STEELSERIES_ARCTIS_1_X),
+ .driver_data = STEELSERIES_ARCTIS_1_X },
+
+ { /* SteelSeries Arctis 9 Wireless for XBox */
+ HID_USB_DEVICE(USB_VENDOR_ID_STEELSERIES, USB_DEVICE_ID_STEELSERIES_ARCTIS_9),
+ .driver_data = STEELSERIES_ARCTIS_9 },
+
+ { }
+};
+MODULE_DEVICE_TABLE(hid, steelseries_arctis_devices);
+
+static struct hid_driver steelseries_arctis_driver = {
+ .name = "hid-steelseries-arctis",
+ .id_table = steelseries_arctis_devices,
+ .probe = steelseries_arctis_probe,
+ .remove = steelseries_arctis_remove,
+ .raw_event = steelseries_arctis_raw_event,
+};
+
+module_hid_driver(steelseries_arctis_driver);
+MODULE_DESCRIPTION("HID driver for Steelseries arctis headsets");
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Christian Mayer <git@mayer-bgk.de>");
+MODULE_AUTHOR("Bastien Nocera <hadess@hadess.net>");
diff --git a/drivers/hid/hid-steelseries.c b/drivers/hid/hid-steelseries.c
index fd38ee3ea6fc..5fca0d84d1e6 100644
--- a/drivers/hid/hid-steelseries.c
+++ b/drivers/hid/hid-steelseries.c
@@ -3,7 +3,6 @@
* HID driver for Steelseries devices
*
* Copyright (c) 2013 Simon Wood
- * Copyright (c) 2023 Bastien Nocera
*/
/*
@@ -12,29 +11,11 @@
#include <linux/device.h>
#include <linux/hid.h>
#include <linux/module.h>
-#include <linux/usb.h>
#include <linux/leds.h>
#include "hid-ids.h"
#define STEELSERIES_SRWS1 BIT(0)
-#define STEELSERIES_ARCTIS_1_X BIT(1)
-#define STEELSERIES_ARCTIS_9 BIT(2)
-
-struct steelseries_device {
- struct hid_device *hdev;
- unsigned long quirks;
-
- struct delayed_work battery_work;
- spinlock_t lock;
- bool removed;
-
- struct power_supply_desc battery_desc;
- struct power_supply *battery;
- uint8_t battery_capacity;
- bool headset_connected;
- bool battery_charging;
-};
#if IS_BUILTIN(CONFIG_LEDS_CLASS) || \
(IS_MODULE(CONFIG_LEDS_CLASS) && IS_MODULE(CONFIG_HID_STEELSERIES))
@@ -340,263 +321,22 @@ static int steelseries_srws1_probe(struct hid_device *hdev,
}
#endif
-#define STEELSERIES_HEADSET_BATTERY_TIMEOUT_MS 3000
-
-#define ARCTIS_1_BATTERY_RESPONSE_LEN 8
-#define ARCTIS_9_BATTERY_RESPONSE_LEN 64
-static const char arctis_1_battery_request[] = { 0x06, 0x12 };
-static const char arctis_9_battery_request[] = { 0x00, 0x20 };
-
-static int steelseries_headset_request_battery(struct hid_device *hdev,
- const char *request, size_t len)
-{
- u8 *write_buf;
- int ret;
-
- /* Request battery information */
- write_buf = kmemdup(request, len, GFP_KERNEL);
- if (!write_buf)
- return -ENOMEM;
-
- hid_dbg(hdev, "Sending battery request report");
- ret = hid_hw_raw_request(hdev, request[0], write_buf, len,
- HID_OUTPUT_REPORT, HID_REQ_SET_REPORT);
- if (ret < (int)len) {
- hid_err(hdev, "hid_hw_raw_request() failed with %d\n", ret);
- ret = -ENODATA;
- }
-
- kfree(write_buf);
- return ret;
-}
-
-static void steelseries_headset_fetch_battery(struct hid_device *hdev)
-{
- int ret = 0;
-
- if (hdev->product == USB_DEVICE_ID_STEELSERIES_ARCTIS_1_X)
- ret = steelseries_headset_request_battery(hdev,
- arctis_1_battery_request, sizeof(arctis_1_battery_request));
- else if (hdev->product == USB_DEVICE_ID_STEELSERIES_ARCTIS_9)
- ret = steelseries_headset_request_battery(hdev,
- arctis_9_battery_request, sizeof(arctis_9_battery_request));
-
- if (ret < 0)
- hid_dbg(hdev,
- "Battery query failed (err: %d)\n", ret);
-}
-
-static int battery_capacity_to_level(int capacity)
-{
- if (capacity >= 50)
- return POWER_SUPPLY_CAPACITY_LEVEL_NORMAL;
- if (capacity >= 20)
- return POWER_SUPPLY_CAPACITY_LEVEL_LOW;
- return POWER_SUPPLY_CAPACITY_LEVEL_CRITICAL;
-}
-
-static void steelseries_headset_battery_timer_tick(struct work_struct *work)
-{
- struct steelseries_device *sd = container_of(work,
- struct steelseries_device, battery_work.work);
- struct hid_device *hdev = sd->hdev;
-
- steelseries_headset_fetch_battery(hdev);
-}
-
-#define STEELSERIES_PREFIX "SteelSeries "
-#define STEELSERIES_PREFIX_LEN strlen(STEELSERIES_PREFIX)
-
-static int steelseries_headset_battery_get_property(struct power_supply *psy,
- enum power_supply_property psp,
- union power_supply_propval *val)
-{
- struct steelseries_device *sd = power_supply_get_drvdata(psy);
- int ret = 0;
-
- switch (psp) {
- case POWER_SUPPLY_PROP_MODEL_NAME:
- val->strval = sd->hdev->name;
- while (!strncmp(val->strval, STEELSERIES_PREFIX, STEELSERIES_PREFIX_LEN))
- val->strval += STEELSERIES_PREFIX_LEN;
- break;
- case POWER_SUPPLY_PROP_MANUFACTURER:
- val->strval = "SteelSeries";
- break;
- case POWER_SUPPLY_PROP_PRESENT:
- val->intval = 1;
- break;
- case POWER_SUPPLY_PROP_STATUS:
- if (sd->headset_connected) {
- val->intval = sd->battery_charging ?
- POWER_SUPPLY_STATUS_CHARGING :
- POWER_SUPPLY_STATUS_DISCHARGING;
- } else
- val->intval = POWER_SUPPLY_STATUS_UNKNOWN;
- break;
- case POWER_SUPPLY_PROP_SCOPE:
- val->intval = POWER_SUPPLY_SCOPE_DEVICE;
- break;
- case POWER_SUPPLY_PROP_CAPACITY:
- val->intval = sd->battery_capacity;
- break;
- case POWER_SUPPLY_PROP_CAPACITY_LEVEL:
- val->intval = battery_capacity_to_level(sd->battery_capacity);
- break;
- default:
- ret = -EINVAL;
- break;
- }
- return ret;
-}
-
-static void
-steelseries_headset_set_wireless_status(struct hid_device *hdev,
- bool connected)
-{
- struct usb_interface *intf;
-
- if (!hid_is_usb(hdev))
- return;
-
- intf = to_usb_interface(hdev->dev.parent);
- usb_set_wireless_status(intf, connected ?
- USB_WIRELESS_STATUS_CONNECTED :
- USB_WIRELESS_STATUS_DISCONNECTED);
-}
-
-static enum power_supply_property steelseries_headset_battery_props[] = {
- POWER_SUPPLY_PROP_MODEL_NAME,
- POWER_SUPPLY_PROP_MANUFACTURER,
- POWER_SUPPLY_PROP_PRESENT,
- POWER_SUPPLY_PROP_STATUS,
- POWER_SUPPLY_PROP_SCOPE,
- POWER_SUPPLY_PROP_CAPACITY,
- POWER_SUPPLY_PROP_CAPACITY_LEVEL,
-};
-
-static int steelseries_headset_battery_register(struct steelseries_device *sd)
-{
- static atomic_t battery_no = ATOMIC_INIT(0);
- struct power_supply_config battery_cfg = { .drv_data = sd, };
- unsigned long n;
- int ret;
-
- sd->battery_desc.type = POWER_SUPPLY_TYPE_BATTERY;
- sd->battery_desc.properties = steelseries_headset_battery_props;
- sd->battery_desc.num_properties = ARRAY_SIZE(steelseries_headset_battery_props);
- sd->battery_desc.get_property = steelseries_headset_battery_get_property;
- sd->battery_desc.use_for_apm = 0;
- n = atomic_inc_return(&battery_no) - 1;
- sd->battery_desc.name = devm_kasprintf(&sd->hdev->dev, GFP_KERNEL,
- "steelseries_headset_battery_%ld", n);
- if (!sd->battery_desc.name)
- return -ENOMEM;
-
- /* avoid the warning of 0% battery while waiting for the first info */
- steelseries_headset_set_wireless_status(sd->hdev, false);
- sd->battery_capacity = 100;
- sd->battery_charging = false;
-
- sd->battery = devm_power_supply_register(&sd->hdev->dev,
- &sd->battery_desc, &battery_cfg);
- if (IS_ERR(sd->battery)) {
- ret = PTR_ERR(sd->battery);
- hid_err(sd->hdev,
- "%s:power_supply_register failed with error %d\n",
- __func__, ret);
- return ret;
- }
- power_supply_powers(sd->battery, &sd->hdev->dev);
-
- INIT_DELAYED_WORK(&sd->battery_work, steelseries_headset_battery_timer_tick);
- steelseries_headset_fetch_battery(sd->hdev);
-
- if (sd->quirks & STEELSERIES_ARCTIS_9) {
- /* The first fetch_battery request can remain unanswered in some cases */
- schedule_delayed_work(&sd->battery_work,
- msecs_to_jiffies(STEELSERIES_HEADSET_BATTERY_TIMEOUT_MS));
- }
-
- return 0;
-}
-
-static bool steelseries_is_vendor_usage_page(struct hid_device *hdev, uint8_t usage_page)
-{
- return hdev->rdesc[0] == 0x06 &&
- hdev->rdesc[1] == usage_page &&
- hdev->rdesc[2] == 0xff;
-}
-
static int steelseries_probe(struct hid_device *hdev, const struct hid_device_id *id)
{
- struct steelseries_device *sd;
- int ret;
-
- if (hdev->product == USB_DEVICE_ID_STEELSERIES_SRWS1) {
#if IS_BUILTIN(CONFIG_LEDS_CLASS) || \
(IS_MODULE(CONFIG_LEDS_CLASS) && IS_MODULE(CONFIG_HID_STEELSERIES))
- return steelseries_srws1_probe(hdev, id);
+ return steelseries_srws1_probe(hdev, id);
#else
- return -ENODEV;
+ return -ENODEV;
#endif
- }
-
- sd = devm_kzalloc(&hdev->dev, sizeof(*sd), GFP_KERNEL);
- if (!sd)
- return -ENOMEM;
- hid_set_drvdata(hdev, sd);
- sd->hdev = hdev;
- sd->quirks = id->driver_data;
-
- ret = hid_parse(hdev);
- if (ret)
- return ret;
-
- if (sd->quirks & STEELSERIES_ARCTIS_9 &&
- !steelseries_is_vendor_usage_page(hdev, 0xc0))
- return -ENODEV;
-
- spin_lock_init(&sd->lock);
-
- ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
- if (ret)
- return ret;
-
- ret = hid_hw_open(hdev);
- if (ret)
- return ret;
-
- if (steelseries_headset_battery_register(sd) < 0)
- hid_err(sd->hdev,
- "Failed to register battery for headset\n");
-
- return ret;
}
static void steelseries_remove(struct hid_device *hdev)
{
- struct steelseries_device *sd;
- unsigned long flags;
-
- if (hdev->product == USB_DEVICE_ID_STEELSERIES_SRWS1) {
#if IS_BUILTIN(CONFIG_LEDS_CLASS) || \
(IS_MODULE(CONFIG_LEDS_CLASS) && IS_MODULE(CONFIG_HID_STEELSERIES))
- hid_hw_stop(hdev);
-#endif
- return;
- }
-
- sd = hid_get_drvdata(hdev);
-
- spin_lock_irqsave(&sd->lock, flags);
- sd->removed = true;
- spin_unlock_irqrestore(&sd->lock, flags);
-
- cancel_delayed_work_sync(&sd->battery_work);
-
- hid_hw_close(hdev);
hid_hw_stop(hdev);
+#endif
}
static const __u8 *steelseries_srws1_report_fixup(struct hid_device *hdev,
@@ -615,123 +355,10 @@ static const __u8 *steelseries_srws1_report_fixup(struct hid_device *hdev,
return rdesc;
}
-static uint8_t steelseries_headset_map_capacity(uint8_t capacity, uint8_t min_in, uint8_t max_in)
-{
- if (capacity >= max_in)
- return 100;
- if (capacity <= min_in)
- return 0;
- return (capacity - min_in) * 100 / (max_in - min_in);
-}
-
-static int steelseries_headset_raw_event(struct hid_device *hdev,
- struct hid_report *report, u8 *read_buf,
- int size)
-{
- struct steelseries_device *sd = hid_get_drvdata(hdev);
- int capacity = sd->battery_capacity;
- bool connected = sd->headset_connected;
- bool charging = sd->battery_charging;
- unsigned long flags;
-
- /* Not a headset */
- if (hdev->product == USB_DEVICE_ID_STEELSERIES_SRWS1)
- return 0;
-
- if (hdev->product == USB_DEVICE_ID_STEELSERIES_ARCTIS_1_X) {
- hid_dbg(sd->hdev,
- "Parsing raw event for Arctis 1 headset (%*ph)\n", size, read_buf);
- if (size < ARCTIS_1_BATTERY_RESPONSE_LEN ||
- memcmp(read_buf, arctis_1_battery_request, sizeof(arctis_1_battery_request))) {
- if (!delayed_work_pending(&sd->battery_work))
- goto request_battery;
- return 0;
- }
- if (read_buf[2] == 0x01) {
- connected = false;
- capacity = 100;
- } else {
- connected = true;
- capacity = read_buf[3];
- }
- }
-
- if (hdev->product == USB_DEVICE_ID_STEELSERIES_ARCTIS_9) {
- hid_dbg(sd->hdev,
- "Parsing raw event for Arctis 9 headset (%*ph)\n", size, read_buf);
- if (size < ARCTIS_9_BATTERY_RESPONSE_LEN) {
- if (!delayed_work_pending(&sd->battery_work))
- goto request_battery;
- return 0;
- }
-
- if (read_buf[0] == 0xaa && read_buf[1] == 0x01) {
- connected = true;
- charging = read_buf[4] == 0x01;
-
- /*
- * Found no official documentation about min and max.
- * Values defined by testing.
- */
- capacity = steelseries_headset_map_capacity(read_buf[3], 0x68, 0x9d);
- } else {
- /*
- * Device is off and sends the last known status read_buf[1] == 0x03 or
- * there is no known status of the device read_buf[0] == 0x55
- */
- connected = false;
- charging = false;
- }
- }
-
- if (connected != sd->headset_connected) {
- hid_dbg(sd->hdev,
- "Connected status changed from %sconnected to %sconnected\n",
- sd->headset_connected ? "" : "not ",
- connected ? "" : "not ");
- sd->headset_connected = connected;
- steelseries_headset_set_wireless_status(hdev, connected);
- }
-
- if (capacity != sd->battery_capacity) {
- hid_dbg(sd->hdev,
- "Battery capacity changed from %d%% to %d%%\n",
- sd->battery_capacity, capacity);
- sd->battery_capacity = capacity;
- power_supply_changed(sd->battery);
- }
-
- if (charging != sd->battery_charging) {
- hid_dbg(sd->hdev,
- "Battery charging status changed from %scharging to %scharging\n",
- sd->battery_charging ? "" : "not ",
- charging ? "" : "not ");
- sd->battery_charging = charging;
- power_supply_changed(sd->battery);
- }
-
-request_battery:
- spin_lock_irqsave(&sd->lock, flags);
- if (!sd->removed)
- schedule_delayed_work(&sd->battery_work,
- msecs_to_jiffies(STEELSERIES_HEADSET_BATTERY_TIMEOUT_MS));
- spin_unlock_irqrestore(&sd->lock, flags);
-
- return 0;
-}
-
static const struct hid_device_id steelseries_devices[] = {
{ HID_USB_DEVICE(USB_VENDOR_ID_STEELSERIES, USB_DEVICE_ID_STEELSERIES_SRWS1),
.driver_data = STEELSERIES_SRWS1 },
- { /* SteelSeries Arctis 1 Wireless for XBox */
- HID_USB_DEVICE(USB_VENDOR_ID_STEELSERIES, USB_DEVICE_ID_STEELSERIES_ARCTIS_1_X),
- .driver_data = STEELSERIES_ARCTIS_1_X },
-
- { /* SteelSeries Arctis 9 Wireless for XBox */
- HID_USB_DEVICE(USB_VENDOR_ID_STEELSERIES, USB_DEVICE_ID_STEELSERIES_ARCTIS_9),
- .driver_data = STEELSERIES_ARCTIS_9 },
-
{ }
};
MODULE_DEVICE_TABLE(hid, steelseries_devices);
@@ -742,12 +369,9 @@ static struct hid_driver steelseries_driver = {
.probe = steelseries_probe,
.remove = steelseries_remove,
.report_fixup = steelseries_srws1_report_fixup,
- .raw_event = steelseries_headset_raw_event,
};
module_hid_driver(steelseries_driver);
MODULE_DESCRIPTION("HID driver for Steelseries devices");
MODULE_LICENSE("GPL");
-MODULE_AUTHOR("Bastien Nocera <hadess@hadess.net>");
MODULE_AUTHOR("Simon Wood <simon@mungewell.org>");
-MODULE_AUTHOR("Christian Mayer <git@mayer-bgk.de>");
--
2.55.0
^ permalink raw reply related
* [PATCH v6 3/9] HID: steelseries: Inline and simplify SRWS1 wheel driver
From: Sriman Achanta @ 2026-07-09 3:23 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: linux-input, linux-kernel, Bastien Nocera, Simon Wood,
Christian Mayer, Sriman Achanta
In-Reply-To: <20260709032316.1908460-1-68172138+srimanachanta@users.noreply.github.com>
From: Sriman Achanta <srimanachanta@gmail.com>
Arctis headsets have their own driver now, so hid-steelseries.c only
handles the SRWS1 racing wheel. Remove the dispatch layer:
- Inline steelseries_srws1_probe() as steelseries_probe()
- Add a steelseries_remove() that calls hid_hw_stop()
- Drop the STEELSERIES_SRWS1 quirk bit, no longer needed
- Remove the vendor/product check in steelseries_srws1_report_fixup(),
since this driver only binds the SRWS1
Signed-off-by: Sriman Achanta <srimanachanta@gmail.com>
---
drivers/hid/hid-steelseries.c | 30 +++++++++---------------------
1 file changed, 9 insertions(+), 21 deletions(-)
diff --git a/drivers/hid/hid-steelseries.c b/drivers/hid/hid-steelseries.c
index 5fca0d84d1e6..5b3896643901 100644
--- a/drivers/hid/hid-steelseries.c
+++ b/drivers/hid/hid-steelseries.c
@@ -15,8 +15,6 @@
#include "hid-ids.h"
-#define STEELSERIES_SRWS1 BIT(0)
-
#if IS_BUILTIN(CONFIG_LEDS_CLASS) || \
(IS_MODULE(CONFIG_LEDS_CLASS) && IS_MODULE(CONFIG_HID_STEELSERIES))
#define SRWS1_NUMBER_LEDS 15
@@ -225,7 +223,7 @@ static enum led_brightness steelseries_srws1_led_get_brightness(struct led_class
return value ? LED_FULL : LED_OFF;
}
-static int steelseries_srws1_probe(struct hid_device *hdev,
+static int steelseries_probe(struct hid_device *hdev,
const struct hid_device_id *id)
{
int ret, i;
@@ -319,33 +317,24 @@ static int steelseries_srws1_probe(struct hid_device *hdev,
err:
return ret;
}
-#endif
-static int steelseries_probe(struct hid_device *hdev, const struct hid_device_id *id)
+static void steelseries_remove(struct hid_device *hdev)
{
-#if IS_BUILTIN(CONFIG_LEDS_CLASS) || \
- (IS_MODULE(CONFIG_LEDS_CLASS) && IS_MODULE(CONFIG_HID_STEELSERIES))
- return steelseries_srws1_probe(hdev, id);
+ hid_hw_stop(hdev);
+}
#else
+static int steelseries_probe(struct hid_device *hdev,
+ const struct hid_device_id *id)
+{
return -ENODEV;
-#endif
}
-static void steelseries_remove(struct hid_device *hdev)
-{
-#if IS_BUILTIN(CONFIG_LEDS_CLASS) || \
- (IS_MODULE(CONFIG_LEDS_CLASS) && IS_MODULE(CONFIG_HID_STEELSERIES))
- hid_hw_stop(hdev);
+static void steelseries_remove(struct hid_device *hdev) {}
#endif
-}
static const __u8 *steelseries_srws1_report_fixup(struct hid_device *hdev,
__u8 *rdesc, unsigned int *rsize)
{
- if (hdev->vendor != USB_VENDOR_ID_STEELSERIES ||
- hdev->product != USB_DEVICE_ID_STEELSERIES_SRWS1)
- return rdesc;
-
if (*rsize >= 115 && rdesc[11] == 0x02 && rdesc[13] == 0xc8
&& rdesc[29] == 0xbb && rdesc[40] == 0xc5) {
hid_info(hdev, "Fixing up Steelseries SRW-S1 report descriptor\n");
@@ -356,8 +345,7 @@ static const __u8 *steelseries_srws1_report_fixup(struct hid_device *hdev,
}
static const struct hid_device_id steelseries_devices[] = {
- { HID_USB_DEVICE(USB_VENDOR_ID_STEELSERIES, USB_DEVICE_ID_STEELSERIES_SRWS1),
- .driver_data = STEELSERIES_SRWS1 },
+ { HID_USB_DEVICE(USB_VENDOR_ID_STEELSERIES, USB_DEVICE_ID_STEELSERIES_SRWS1) },
{ }
};
--
2.55.0
^ permalink raw reply related
* [PATCH v6 4/9] HID: steelseries: Refactor Arctis driver to use device_info framework
From: Sriman Achanta @ 2026-07-09 3:23 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: linux-input, linux-kernel, Bastien Nocera, Simon Wood,
Christian Mayer, Sriman Achanta
In-Reply-To: <20260709032316.1908460-1-68172138+srimanachanta@users.noreply.github.com>
From: Sriman Achanta <srimanachanta@gmail.com>
Replace the per-product if/else quirk bitmap with a
steelseries_device_info struct. Each model provides its capabilities,
sync_interface, and request_status/parse_status callbacks. Report
sending is folded into steelseries_send_report() and the feature and
output wrappers, and the battery identifiers lose their per-model names.
This is mostly a refactor, but it changes two things:
- Battery status is polled from a periodic delayed work (status_work)
instead of being requested from raw_event(). The interval stays at
3s.
- Arctis 1 no longer clamps the reported capacity to 100% while
disconnected. The connection state already controls how this is
shown to userspace.
ARCTIS_1_X and ARCTIS_9 keep working. No new devices are added.
Signed-off-by: Sriman Achanta <srimanachanta@gmail.com>
---
drivers/hid/hid-steelseries-arctis.c | 468 +++++++++++++++------------
1 file changed, 267 insertions(+), 201 deletions(-)
diff --git a/drivers/hid/hid-steelseries-arctis.c b/drivers/hid/hid-steelseries-arctis.c
index 079504e6932a..c54c56db9ddd 100644
--- a/drivers/hid/hid-steelseries-arctis.c
+++ b/drivers/hid/hid-steelseries-arctis.c
@@ -3,6 +3,7 @@
* HID driver for Steelseries arctis headsets
*
* Copyright (c) 2023 Bastien Nocera
+ * Copyright (c) 2026 Sriman Achanta
*/
#include <linux/device.h>
@@ -15,70 +16,89 @@
#include "hid-ids.h"
-#define STEELSERIES_ARCTIS_1_X BIT(0)
-#define STEELSERIES_ARCTIS_9 BIT(1)
+#define SS_CAP_BATTERY BIT(0)
+
+struct steelseries_device;
+
+struct steelseries_device_info {
+ unsigned long capabilities;
+
+ u8 sync_interface;
+
+ int (*request_status)(struct hid_device *hdev);
+ void (*parse_status)(struct steelseries_device *sd, u8 *data, int size);
+};
struct steelseries_device {
struct hid_device *hdev;
- unsigned long quirks;
+ const struct steelseries_device_info *info;
- struct delayed_work battery_work;
- spinlock_t lock;
- bool removed;
+ struct delayed_work status_work;
struct power_supply_desc battery_desc;
struct power_supply *battery;
- uint8_t battery_capacity;
bool headset_connected;
+ u8 battery_capacity;
bool battery_charging;
-};
-#define STEELSERIES_HEADSET_BATTERY_TIMEOUT_MS 3000
+ spinlock_t lock;
+ bool removed;
+};
-#define ARCTIS_1_BATTERY_RESPONSE_LEN 8
-#define ARCTIS_9_BATTERY_RESPONSE_LEN 64
-static const char arctis_1_battery_request[] = { 0x06, 0x12 };
-static const char arctis_9_battery_request[] = { 0x00, 0x20 };
+/*
+ * Headset report helpers
+ */
-static int steelseries_headset_request_battery(struct hid_device *hdev,
- const char *request, size_t len)
+static int steelseries_send_report(struct hid_device *hdev, const u8 *data,
+ int len, enum hid_report_type type)
{
- u8 *write_buf;
+ u8 *buf;
int ret;
- /* Request battery information */
- write_buf = kmemdup(request, len, GFP_KERNEL);
- if (!write_buf)
+ buf = kmemdup(data, len, GFP_KERNEL);
+ if (!buf)
return -ENOMEM;
- hid_dbg(hdev, "Sending battery request report");
- ret = hid_hw_raw_request(hdev, request[0], write_buf, len,
- HID_OUTPUT_REPORT, HID_REQ_SET_REPORT);
- if (ret < (int)len) {
- hid_err(hdev, "hid_hw_raw_request() failed with %d\n", ret);
- ret = -ENODATA;
- }
+ ret = hid_hw_raw_request(hdev, data[0], buf, len, type,
+ HID_REQ_SET_REPORT);
+ kfree(buf);
- kfree(write_buf);
- return ret;
+ if (ret < 0)
+ return ret;
+ if (ret < len)
+ return -EIO;
+
+ return 0;
}
-static void steelseries_headset_fetch_battery(struct hid_device *hdev)
+static inline int steelseries_send_output_report(struct hid_device *hdev,
+ const u8 *data, int len)
{
- int ret = 0;
+ return steelseries_send_report(hdev, data, len, HID_OUTPUT_REPORT);
+}
+
+/*
+ * Headset status request functions
+ */
- if (hdev->product == USB_DEVICE_ID_STEELSERIES_ARCTIS_1_X)
- ret = steelseries_headset_request_battery(hdev,
- arctis_1_battery_request, sizeof(arctis_1_battery_request));
- else if (hdev->product == USB_DEVICE_ID_STEELSERIES_ARCTIS_9)
- ret = steelseries_headset_request_battery(hdev,
- arctis_9_battery_request, sizeof(arctis_9_battery_request));
+static int steelseries_arctis_1_request_status(struct hid_device *hdev)
+{
+ const u8 data[] = { 0x06, 0x12 };
- if (ret < 0)
- hid_dbg(hdev,
- "Battery query failed (err: %d)\n", ret);
+ return steelseries_send_output_report(hdev, data, sizeof(data));
}
+static int steelseries_arctis_9_request_status(struct hid_device *hdev)
+{
+ const u8 data[] = { 0x00, 0x20 };
+
+ return steelseries_send_output_report(hdev, data, sizeof(data));
+}
+
+/*
+ * Headset battery helpers
+ */
+
static int battery_capacity_to_level(int capacity)
{
if (capacity >= 50)
@@ -88,30 +108,101 @@ static int battery_capacity_to_level(int capacity)
return POWER_SUPPLY_CAPACITY_LEVEL_CRITICAL;
}
-static void steelseries_headset_battery_timer_tick(struct work_struct *work)
+static u8 steelseries_map_capacity(u8 capacity, u8 min_in, u8 max_in)
{
- struct steelseries_device *sd = container_of(work,
- struct steelseries_device, battery_work.work);
- struct hid_device *hdev = sd->hdev;
+ if (capacity >= max_in)
+ return 100;
+ if (capacity <= min_in)
+ return 0;
+ return (capacity - min_in) * 100 / (max_in - min_in);
+}
- steelseries_headset_fetch_battery(hdev);
+/*
+ * Headset status parse functions
+ */
+
+static void steelseries_arctis_1_parse_status(struct steelseries_device *sd,
+ u8 *data, int size)
+{
+ /* Only the battery status report echoes the request header. */
+ if (size < 8 || data[0] != 0x06 || data[1] != 0x12)
+ return;
+
+ sd->headset_connected = (data[2] != 0x01);
+ sd->battery_capacity = data[3];
+}
+
+static void steelseries_arctis_9_parse_status(struct steelseries_device *sd,
+ u8 *data, int size)
+{
+ if (size < 5)
+ return;
+
+ if (data[0] == 0xaa && data[1] == 0x01) {
+ sd->headset_connected = true;
+ sd->battery_charging = (data[4] == 0x01);
+ sd->battery_capacity = steelseries_map_capacity(data[3], 0x68, 0x9d);
+ } else {
+ /* Device off: 0x55 (no status) or 0x03 (stale status). */
+ sd->headset_connected = false;
+ sd->battery_charging = false;
+ }
+}
+
+/*
+ * Device info definitions
+ */
+
+static const struct steelseries_device_info arctis_1_info = {
+ .sync_interface = 3,
+ .capabilities = SS_CAP_BATTERY,
+ .request_status = steelseries_arctis_1_request_status,
+ .parse_status = steelseries_arctis_1_parse_status,
+};
+
+static const struct steelseries_device_info arctis_9_info = {
+ .sync_interface = 0,
+ .capabilities = SS_CAP_BATTERY,
+ .request_status = steelseries_arctis_9_request_status,
+ .parse_status = steelseries_arctis_9_parse_status,
+};
+
+/*
+ * Headset wireless status and battery infrastructure
+ */
+
+#define STEELSERIES_HEADSET_STATUS_TIMEOUT_MS 3000
+
+static void
+steelseries_headset_set_wireless_status(struct hid_device *hdev,
+ bool connected)
+{
+ struct usb_interface *intf;
+
+ if (!hid_is_usb(hdev))
+ return;
+
+ intf = to_usb_interface(hdev->dev.parent);
+ usb_set_wireless_status(intf, connected ?
+ USB_WIRELESS_STATUS_CONNECTED :
+ USB_WIRELESS_STATUS_DISCONNECTED);
}
#define STEELSERIES_PREFIX "SteelSeries "
-#define STEELSERIES_PREFIX_LEN strlen(STEELSERIES_PREFIX)
-static int steelseries_headset_battery_get_property(struct power_supply *psy,
+static int steelseries_battery_get_property(struct power_supply *psy,
enum power_supply_property psp,
union power_supply_propval *val)
{
struct steelseries_device *sd = power_supply_get_drvdata(psy);
+ size_t prefix_len;
int ret = 0;
switch (psp) {
case POWER_SUPPLY_PROP_MODEL_NAME:
val->strval = sd->hdev->name;
- while (!strncmp(val->strval, STEELSERIES_PREFIX, STEELSERIES_PREFIX_LEN))
- val->strval += STEELSERIES_PREFIX_LEN;
+ while ((prefix_len = str_has_prefix(val->strval, STEELSERIES_PREFIX)))
+ val->strval += prefix_len;
break;
case POWER_SUPPLY_PROP_MANUFACTURER:
val->strval = "SteelSeries";
@@ -120,12 +211,12 @@ static int steelseries_headset_battery_get_property(struct power_supply *psy,
val->intval = 1;
break;
case POWER_SUPPLY_PROP_STATUS:
- if (sd->headset_connected) {
- val->intval = sd->battery_charging ?
- POWER_SUPPLY_STATUS_CHARGING :
- POWER_SUPPLY_STATUS_DISCHARGING;
- } else
+ if (!sd->headset_connected)
val->intval = POWER_SUPPLY_STATUS_UNKNOWN;
+ else if (sd->battery_charging)
+ val->intval = POWER_SUPPLY_STATUS_CHARGING;
+ else
+ val->intval = POWER_SUPPLY_STATUS_DISCHARGING;
break;
case POWER_SUPPLY_PROP_SCOPE:
val->intval = POWER_SUPPLY_SCOPE_DEVICE;
@@ -143,22 +234,7 @@ static int steelseries_headset_battery_get_property(struct power_supply *psy,
return ret;
}
-static void
-steelseries_headset_set_wireless_status(struct hid_device *hdev,
- bool connected)
-{
- struct usb_interface *intf;
-
- if (!hid_is_usb(hdev))
- return;
-
- intf = to_usb_interface(hdev->dev.parent);
- usb_set_wireless_status(intf, connected ?
- USB_WIRELESS_STATUS_CONNECTED :
- USB_WIRELESS_STATUS_DISCONNECTED);
-}
-
-static enum power_supply_property steelseries_headset_battery_props[] = {
+static enum power_supply_property steelseries_battery_props[] = {
POWER_SUPPLY_PROP_MODEL_NAME,
POWER_SUPPLY_PROP_MANUFACTURER,
POWER_SUPPLY_PROP_PRESENT,
@@ -168,7 +244,26 @@ static enum power_supply_property steelseries_headset_battery_props[] = {
POWER_SUPPLY_PROP_CAPACITY_LEVEL,
};
-static int steelseries_headset_battery_register(struct steelseries_device *sd)
+/*
+ * Delayed work handlers for status polling
+ */
+
+static void steelseries_status_timer_work_handler(struct work_struct *work)
+{
+ struct steelseries_device *sd = container_of(
+ work, struct steelseries_device, status_work.work);
+ unsigned long flags;
+
+ sd->info->request_status(sd->hdev);
+
+ spin_lock_irqsave(&sd->lock, flags);
+ if (!sd->removed)
+ schedule_delayed_work(&sd->status_work,
+ msecs_to_jiffies(STEELSERIES_HEADSET_STATUS_TIMEOUT_MS));
+ spin_unlock_irqrestore(&sd->lock, flags);
+}
+
+static int steelseries_battery_register(struct steelseries_device *sd)
{
static atomic_t battery_no = ATOMIC_INIT(0);
struct power_supply_config battery_cfg = { .drv_data = sd, };
@@ -176,25 +271,27 @@ static int steelseries_headset_battery_register(struct steelseries_device *sd)
int ret;
sd->battery_desc.type = POWER_SUPPLY_TYPE_BATTERY;
- sd->battery_desc.properties = steelseries_headset_battery_props;
- sd->battery_desc.num_properties = ARRAY_SIZE(steelseries_headset_battery_props);
- sd->battery_desc.get_property = steelseries_headset_battery_get_property;
+ sd->battery_desc.properties = steelseries_battery_props;
+ sd->battery_desc.num_properties = ARRAY_SIZE(steelseries_battery_props);
+ sd->battery_desc.get_property = steelseries_battery_get_property;
sd->battery_desc.use_for_apm = 0;
n = atomic_inc_return(&battery_no) - 1;
sd->battery_desc.name = devm_kasprintf(&sd->hdev->dev, GFP_KERNEL,
- "steelseries_headset_battery_%ld", n);
+ "steelseries_headset_battery_%ld", n);
if (!sd->battery_desc.name)
return -ENOMEM;
/* avoid the warning of 0% battery while waiting for the first info */
- steelseries_headset_set_wireless_status(sd->hdev, false);
sd->battery_capacity = 100;
sd->battery_charging = false;
+ sd->headset_connected = false;
+ steelseries_headset_set_wireless_status(sd->hdev, false);
sd->battery = devm_power_supply_register(&sd->hdev->dev,
&sd->battery_desc, &battery_cfg);
if (IS_ERR(sd->battery)) {
ret = PTR_ERR(sd->battery);
+ sd->battery = NULL;
hid_err(sd->hdev,
"%s:power_supply_register failed with error %d\n",
__func__, ret);
@@ -202,59 +299,65 @@ static int steelseries_headset_battery_register(struct steelseries_device *sd)
}
power_supply_powers(sd->battery, &sd->hdev->dev);
- INIT_DELAYED_WORK(&sd->battery_work, steelseries_headset_battery_timer_tick);
- steelseries_headset_fetch_battery(sd->hdev);
-
- if (sd->quirks & STEELSERIES_ARCTIS_9) {
- /* The first fetch_battery request can remain unanswered in some cases */
- schedule_delayed_work(&sd->battery_work,
- msecs_to_jiffies(STEELSERIES_HEADSET_BATTERY_TIMEOUT_MS));
- }
-
return 0;
}
-static bool steelseries_is_vendor_usage_page(struct hid_device *hdev, uint8_t usage_page)
-{
- return hdev->rdesc[0] == 0x06 &&
- hdev->rdesc[1] == usage_page &&
- hdev->rdesc[2] == 0xff;
-}
-
-static int steelseries_arctis_probe(struct hid_device *hdev, const struct hid_device_id *id)
+static int steelseries_arctis_probe(struct hid_device *hdev,
+ const struct hid_device_id *id)
{
+ const struct steelseries_device_info *info =
+ (const struct steelseries_device_info *)id->driver_data;
struct steelseries_device *sd;
+ struct usb_interface *intf;
+ u8 interface_num;
int ret;
- sd = devm_kzalloc(&hdev->dev, sizeof(*sd), GFP_KERNEL);
- if (!sd)
- return -ENOMEM;
- hid_set_drvdata(hdev, sd);
- sd->hdev = hdev;
- sd->quirks = id->driver_data;
+ if (hid_is_usb(hdev)) {
+ intf = to_usb_interface(hdev->dev.parent);
+ interface_num = intf->cur_altsetting->desc.bInterfaceNumber;
+ } else {
+ return -ENODEV;
+ }
ret = hid_parse(hdev);
if (ret)
return ret;
- if (sd->quirks & STEELSERIES_ARCTIS_9 &&
- !steelseries_is_vendor_usage_page(hdev, 0xc0))
- return -ENODEV;
+ /* Let hid-generic handle non-sync interfaces */
+ if (interface_num != info->sync_interface)
+ return hid_hw_start(hdev, HID_CONNECT_DEFAULT);
+
+ sd = devm_kzalloc(&hdev->dev, sizeof(*sd), GFP_KERNEL);
+ if (!sd)
+ return -ENOMEM;
+ sd->hdev = hdev;
+ sd->info = info;
spin_lock_init(&sd->lock);
+ hid_set_drvdata(hdev, sd);
+
ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
if (ret)
return ret;
ret = hid_hw_open(hdev);
if (ret)
- return ret;
+ goto err_stop;
- if (steelseries_headset_battery_register(sd) < 0)
- hid_err(sd->hdev,
- "Failed to register battery for headset\n");
+ if (info->capabilities & SS_CAP_BATTERY) {
+ ret = steelseries_battery_register(sd);
+ if (ret < 0)
+ hid_warn(hdev, "Failed to register battery: %d\n", ret);
+ }
+
+ INIT_DELAYED_WORK(&sd->status_work, steelseries_status_timer_work_handler);
+ schedule_delayed_work(&sd->status_work, msecs_to_jiffies(100));
+ return 0;
+
+err_stop:
+ hid_hw_stop(hdev);
return ret;
}
@@ -262,130 +365,92 @@ static void steelseries_arctis_remove(struct hid_device *hdev)
{
struct steelseries_device *sd;
unsigned long flags;
+ struct usb_interface *intf;
+ u8 interface_num;
+
+ if (hid_is_usb(hdev)) {
+ intf = to_usb_interface(hdev->dev.parent);
+ interface_num = intf->cur_altsetting->desc.bInterfaceNumber;
+ } else {
+ return;
+ }
sd = hid_get_drvdata(hdev);
- spin_lock_irqsave(&sd->lock, flags);
- sd->removed = true;
- spin_unlock_irqrestore(&sd->lock, flags);
+ if (!sd) {
+ hid_hw_stop(hdev);
+ return;
+ }
- cancel_delayed_work_sync(&sd->battery_work);
+ if (interface_num == sd->info->sync_interface) {
+ spin_lock_irqsave(&sd->lock, flags);
+ sd->removed = true;
+ spin_unlock_irqrestore(&sd->lock, flags);
+
+ cancel_delayed_work_sync(&sd->status_work);
+ }
hid_hw_close(hdev);
hid_hw_stop(hdev);
}
-static uint8_t steelseries_headset_map_capacity(uint8_t capacity, uint8_t min_in, uint8_t max_in)
-{
- if (capacity >= max_in)
- return 100;
- if (capacity <= min_in)
- return 0;
- return (capacity - min_in) * 100 / (max_in - min_in);
-}
-
static int steelseries_arctis_raw_event(struct hid_device *hdev,
- struct hid_report *report, u8 *read_buf,
- int size)
+ struct hid_report *report, u8 *data, int size)
{
struct steelseries_device *sd = hid_get_drvdata(hdev);
- int capacity = sd->battery_capacity;
- bool connected = sd->headset_connected;
- bool charging = sd->battery_charging;
- unsigned long flags;
+ u8 old_capacity;
+ bool old_connected;
+ bool old_charging;
- if (hdev->product == USB_DEVICE_ID_STEELSERIES_ARCTIS_1_X) {
- hid_dbg(sd->hdev,
- "Parsing raw event for Arctis 1 headset (%*ph)\n", size, read_buf);
- if (size < ARCTIS_1_BATTERY_RESPONSE_LEN ||
- memcmp(read_buf, arctis_1_battery_request, sizeof(arctis_1_battery_request))) {
- if (!delayed_work_pending(&sd->battery_work))
- goto request_battery;
- return 0;
- }
- if (read_buf[2] == 0x01) {
- connected = false;
- capacity = 100;
- } else {
- connected = true;
- capacity = read_buf[3];
- }
- }
+ if (!sd)
+ return 0;
- if (hdev->product == USB_DEVICE_ID_STEELSERIES_ARCTIS_9) {
- hid_dbg(sd->hdev,
- "Parsing raw event for Arctis 9 headset (%*ph)\n", size, read_buf);
- if (size < ARCTIS_9_BATTERY_RESPONSE_LEN) {
- if (!delayed_work_pending(&sd->battery_work))
- goto request_battery;
- return 0;
- }
+ old_capacity = sd->battery_capacity;
+ old_connected = sd->headset_connected;
+ old_charging = sd->battery_charging;
- if (read_buf[0] == 0xaa && read_buf[1] == 0x01) {
- connected = true;
- charging = read_buf[4] == 0x01;
-
- /*
- * Found no official documentation about min and max.
- * Values defined by testing.
- */
- capacity = steelseries_headset_map_capacity(read_buf[3], 0x68, 0x9d);
- } else {
- /*
- * Device is off and sends the last known status read_buf[1] == 0x03 or
- * there is no known status of the device read_buf[0] == 0x55
- */
- connected = false;
- charging = false;
- }
- }
+ sd->info->parse_status(sd, data, size);
- if (connected != sd->headset_connected) {
- hid_dbg(sd->hdev,
+ if (sd->headset_connected != old_connected) {
+ hid_dbg(hdev,
"Connected status changed from %sconnected to %sconnected\n",
- sd->headset_connected ? "" : "not ",
- connected ? "" : "not ");
- sd->headset_connected = connected;
- steelseries_headset_set_wireless_status(hdev, connected);
+ old_connected ? "" : "not ",
+ sd->headset_connected ? "" : "not ");
+
+ if (sd->battery) {
+ steelseries_headset_set_wireless_status(sd->hdev,
+ sd->headset_connected);
+ power_supply_changed(sd->battery);
+ }
}
- if (capacity != sd->battery_capacity) {
- hid_dbg(sd->hdev,
- "Battery capacity changed from %d%% to %d%%\n",
- sd->battery_capacity, capacity);
- sd->battery_capacity = capacity;
- power_supply_changed(sd->battery);
+ if (sd->battery_capacity != old_capacity) {
+ hid_dbg(hdev, "Battery capacity changed from %d%% to %d%%\n",
+ old_capacity, sd->battery_capacity);
+ if (sd->battery)
+ power_supply_changed(sd->battery);
}
- if (charging != sd->battery_charging) {
- hid_dbg(sd->hdev,
+ if (sd->battery_charging != old_charging) {
+ hid_dbg(hdev,
"Battery charging status changed from %scharging to %scharging\n",
- sd->battery_charging ? "" : "not ",
- charging ? "" : "not ");
- sd->battery_charging = charging;
- power_supply_changed(sd->battery);
+ old_charging ? "" : "not ",
+ sd->battery_charging ? "" : "not ");
+ if (sd->battery)
+ power_supply_changed(sd->battery);
}
-request_battery:
- spin_lock_irqsave(&sd->lock, flags);
- if (!sd->removed)
- schedule_delayed_work(&sd->battery_work,
- msecs_to_jiffies(STEELSERIES_HEADSET_BATTERY_TIMEOUT_MS));
- spin_unlock_irqrestore(&sd->lock, flags);
-
return 0;
}
static const struct hid_device_id steelseries_arctis_devices[] = {
- { /* SteelSeries Arctis 1 Wireless for XBox */
- HID_USB_DEVICE(USB_VENDOR_ID_STEELSERIES, USB_DEVICE_ID_STEELSERIES_ARCTIS_1_X),
- .driver_data = STEELSERIES_ARCTIS_1_X },
-
- { /* SteelSeries Arctis 9 Wireless for XBox */
- HID_USB_DEVICE(USB_VENDOR_ID_STEELSERIES, USB_DEVICE_ID_STEELSERIES_ARCTIS_9),
- .driver_data = STEELSERIES_ARCTIS_9 },
-
- { }
+ { HID_USB_DEVICE(USB_VENDOR_ID_STEELSERIES,
+ USB_DEVICE_ID_STEELSERIES_ARCTIS_1_X),
+ .driver_data = (unsigned long)&arctis_1_info },
+ { HID_USB_DEVICE(USB_VENDOR_ID_STEELSERIES,
+ USB_DEVICE_ID_STEELSERIES_ARCTIS_9),
+ .driver_data = (unsigned long)&arctis_9_info },
+ {}
};
MODULE_DEVICE_TABLE(hid, steelseries_arctis_devices);
@@ -402,3 +467,4 @@ MODULE_DESCRIPTION("HID driver for Steelseries arctis headsets");
MODULE_LICENSE("GPL");
MODULE_AUTHOR("Christian Mayer <git@mayer-bgk.de>");
MODULE_AUTHOR("Bastien Nocera <hadess@hadess.net>");
+MODULE_AUTHOR("Sriman Achanta <srimanachanta@gmail.com>");
--
2.55.0
^ permalink raw reply related
* [PATCH v6 5/9] HID: steelseries: Report POWER_SUPPLY_STATUS_FULL when full
From: Sriman Achanta @ 2026-07-09 3:23 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: linux-input, linux-kernel, Bastien Nocera, Simon Wood,
Christian Mayer, Sriman Achanta
In-Reply-To: <20260709032316.1908460-1-68172138+srimanachanta@users.noreply.github.com>
From: Sriman Achanta <srimanachanta@gmail.com>
Report POWER_SUPPLY_STATUS_FULL when the headset is connected, charging,
and at 100% capacity. It reported CHARGING in that case before.
The charging flag doubles as a power-present signal: the headset keeps
reporting charging while it sits docked at 100%, and clears the flag as
soon as it runs on battery. Gating FULL on the charging flag therefore
avoids reporting FULL for an unplugged headset that merely happens to be
at 100%, which would otherwise hide the discharge state from userspace.
Signed-off-by: Sriman Achanta <srimanachanta@gmail.com>
---
drivers/hid/hid-steelseries-arctis.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/hid/hid-steelseries-arctis.c b/drivers/hid/hid-steelseries-arctis.c
index c54c56db9ddd..7b8f5f2623b0 100644
--- a/drivers/hid/hid-steelseries-arctis.c
+++ b/drivers/hid/hid-steelseries-arctis.c
@@ -214,7 +214,9 @@ static int steelseries_battery_get_property(struct power_supply *psy,
if (!sd->headset_connected)
val->intval = POWER_SUPPLY_STATUS_UNKNOWN;
else if (sd->battery_charging)
- val->intval = POWER_SUPPLY_STATUS_CHARGING;
+ val->intval = sd->battery_capacity >= 100 ?
+ POWER_SUPPLY_STATUS_FULL :
+ POWER_SUPPLY_STATUS_CHARGING;
else
val->intval = POWER_SUPPLY_STATUS_DISCHARGING;
break;
--
2.55.0
^ permalink raw reply related
* [PATCH v6 6/9] HID: steelseries: Correct Arctis 9 battery calibration range
From: Sriman Achanta @ 2026-07-09 3:23 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: linux-input, linux-kernel, Bastien Nocera, Simon Wood,
Christian Mayer, Sriman Achanta
In-Reply-To: <20260709032316.1908460-1-68172138+srimanachanta@users.noreply.github.com>
From: Sriman Achanta <srimanachanta@gmail.com>
Map the Arctis 9 raw battery value over 0x64 (empty) to 0x9a (full)
instead of 0x68 to 0x9d. These values match the HeadsetControl project
[1] and fit the calibration points from an independent reverse
engineering of the battery tray (about 25% at raw 112, 50% at raw 125)
[2].
I do not have this headset. The values come from those references and
were not measured directly.
[1] https://github.com/Sapd/HeadsetControl/blob/master/lib/devices/steelseries_arctis_9.hpp
[2] https://magnier.io/reverse-engineering-arctis-9-battery-tray/
Signed-off-by: Sriman Achanta <srimanachanta@gmail.com>
---
drivers/hid/hid-steelseries-arctis.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/hid/hid-steelseries-arctis.c b/drivers/hid/hid-steelseries-arctis.c
index 7b8f5f2623b0..b54f20f081a2 100644
--- a/drivers/hid/hid-steelseries-arctis.c
+++ b/drivers/hid/hid-steelseries-arctis.c
@@ -141,7 +141,7 @@ static void steelseries_arctis_9_parse_status(struct steelseries_device *sd,
if (data[0] == 0xaa && data[1] == 0x01) {
sd->headset_connected = true;
sd->battery_charging = (data[4] == 0x01);
- sd->battery_capacity = steelseries_map_capacity(data[3], 0x68, 0x9d);
+ sd->battery_capacity = steelseries_map_capacity(data[3], 0x64, 0x9a);
} else {
/* Device off: 0x55 (no status) or 0x03 (stale status). */
sd->headset_connected = false;
--
2.55.0
^ permalink raw reply related
* [PATCH v6 7/9] HID: steelseries: Manage battery lifetime with refcounting
From: Sriman Achanta @ 2026-07-09 3:23 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: linux-input, linux-kernel, Bastien Nocera, Simon Wood,
Christian Mayer, Sriman Achanta
In-Reply-To: <20260709032316.1908460-1-68172138+srimanachanta@users.noreply.github.com>
From: Sriman Achanta <srimanachanta@gmail.com>
The next change shares one steelseries_device between two HID
interfaces, so the state can outlive either interface. Stop using devm
for it. Reference count the struct with a kref and free it from
steelseries_device_release(). Register and unregister the power supply
explicitly, and clear sd->battery under sd->lock in remove() so it is
not touched after it is unregistered.
Drop the global atomic battery counter and name the power supply after
the device (hdev->uniq, or dev_name() when empty), as hid-input and the
other HID battery drivers do.
No functional change for the current single-interface devices.
Signed-off-by: Sriman Achanta <srimanachanta@gmail.com>
---
drivers/hid/hid-steelseries-arctis.c | 47 +++++++++++++++++++++-------
1 file changed, 36 insertions(+), 11 deletions(-)
diff --git a/drivers/hid/hid-steelseries-arctis.c b/drivers/hid/hid-steelseries-arctis.c
index b54f20f081a2..e534aa44e70a 100644
--- a/drivers/hid/hid-steelseries-arctis.c
+++ b/drivers/hid/hid-steelseries-arctis.c
@@ -8,6 +8,8 @@
#include <linux/device.h>
#include <linux/hid.h>
+#include <linux/kref.h>
+#include <linux/slab.h>
#include <linux/module.h>
#include <linux/power_supply.h>
#include <linux/spinlock.h>
@@ -30,6 +32,8 @@ struct steelseries_device_info {
};
struct steelseries_device {
+ struct kref refcnt;
+
struct hid_device *hdev;
const struct steelseries_device_info *info;
@@ -45,6 +49,14 @@ struct steelseries_device {
bool removed;
};
+static void steelseries_device_release(struct kref *ref)
+{
+ struct steelseries_device *sd =
+ container_of(ref, struct steelseries_device, refcnt);
+
+ kfree(sd);
+}
+
/*
* Headset report helpers
*/
@@ -267,9 +279,8 @@ static void steelseries_status_timer_work_handler(struct work_struct *work)
static int steelseries_battery_register(struct steelseries_device *sd)
{
- static atomic_t battery_no = ATOMIC_INIT(0);
struct power_supply_config battery_cfg = { .drv_data = sd, };
- unsigned long n;
+ struct power_supply *battery;
int ret;
sd->battery_desc.type = POWER_SUPPLY_TYPE_BATTERY;
@@ -277,9 +288,10 @@ static int steelseries_battery_register(struct steelseries_device *sd)
sd->battery_desc.num_properties = ARRAY_SIZE(steelseries_battery_props);
sd->battery_desc.get_property = steelseries_battery_get_property;
sd->battery_desc.use_for_apm = 0;
- n = atomic_inc_return(&battery_no) - 1;
sd->battery_desc.name = devm_kasprintf(&sd->hdev->dev, GFP_KERNEL,
- "steelseries_headset_battery_%ld", n);
+ "steelseries_headset_battery_%s",
+ sd->hdev->uniq[0] ? sd->hdev->uniq :
+ dev_name(&sd->hdev->dev));
if (!sd->battery_desc.name)
return -ENOMEM;
@@ -289,17 +301,19 @@ static int steelseries_battery_register(struct steelseries_device *sd)
sd->headset_connected = false;
steelseries_headset_set_wireless_status(sd->hdev, false);
- sd->battery = devm_power_supply_register(&sd->hdev->dev,
+ battery = power_supply_register(&sd->hdev->dev,
&sd->battery_desc, &battery_cfg);
- if (IS_ERR(sd->battery)) {
- ret = PTR_ERR(sd->battery);
- sd->battery = NULL;
+ if (IS_ERR(battery)) {
+ ret = PTR_ERR(battery);
hid_err(sd->hdev,
"%s:power_supply_register failed with error %d\n",
__func__, ret);
return ret;
}
- power_supply_powers(sd->battery, &sd->hdev->dev);
+ power_supply_powers(battery, &sd->hdev->dev);
+
+ /* Assign on success only, so a concurrent raw_event never sees an ERR_PTR. */
+ sd->battery = battery;
return 0;
}
@@ -329,10 +343,11 @@ static int steelseries_arctis_probe(struct hid_device *hdev,
if (interface_num != info->sync_interface)
return hid_hw_start(hdev, HID_CONNECT_DEFAULT);
- sd = devm_kzalloc(&hdev->dev, sizeof(*sd), GFP_KERNEL);
+ sd = kzalloc_obj(*sd, GFP_KERNEL);
if (!sd)
return -ENOMEM;
+ kref_init(&sd->refcnt);
sd->hdev = hdev;
sd->info = info;
spin_lock_init(&sd->lock);
@@ -341,7 +356,7 @@ static int steelseries_arctis_probe(struct hid_device *hdev,
ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
if (ret)
- return ret;
+ goto err_put;
ret = hid_hw_open(hdev);
if (ret)
@@ -360,12 +375,15 @@ static int steelseries_arctis_probe(struct hid_device *hdev,
err_stop:
hid_hw_stop(hdev);
+err_put:
+ kref_put(&sd->refcnt, steelseries_device_release);
return ret;
}
static void steelseries_arctis_remove(struct hid_device *hdev)
{
struct steelseries_device *sd;
+ struct power_supply *battery;
unsigned long flags;
struct usb_interface *intf;
u8 interface_num;
@@ -387,13 +405,20 @@ static void steelseries_arctis_remove(struct hid_device *hdev)
if (interface_num == sd->info->sync_interface) {
spin_lock_irqsave(&sd->lock, flags);
sd->removed = true;
+ battery = sd->battery;
+ sd->battery = NULL;
spin_unlock_irqrestore(&sd->lock, flags);
cancel_delayed_work_sync(&sd->status_work);
+
+ if (battery)
+ power_supply_unregister(battery);
}
hid_hw_close(hdev);
hid_hw_stop(hdev);
+
+ kref_put(&sd->refcnt, steelseries_device_release);
}
static int steelseries_arctis_raw_event(struct hid_device *hdev,
--
2.55.0
^ permalink raw reply related
* [PATCH v6 8/9] HID: steelseries: Add async status interface support
From: Sriman Achanta @ 2026-07-09 3:23 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: linux-input, linux-kernel, Bastien Nocera, Simon Wood,
Christian Mayer, Sriman Achanta
In-Reply-To: <20260709032316.1908460-1-68172138+srimanachanta@users.noreply.github.com>
From: Sriman Achanta <srimanachanta@gmail.com>
Some headsets expose a second HID interface that sends battery and
connection updates on its own. Watching that interface lets the driver
stop polling the sync interface. Add a
steelseries_device_info::async_interface field and the code to handle
it:
- The driver binds both the sync and async interfaces. The async
interface shares the steelseries_device created by the sync
interface. It finds the sibling with usb_ifnum_to_if(), and before
trusting its intfdata it rejects non-HID siblings by descriptor
class and holds the sibling's device lock across the lookup, so a
crafted device cannot cause a type-confused read and a concurrent
unbind cannot free the hid_device from under it. It then takes a
reference and returns -EPROBE_DEFER until the sync interface has
probed. If the sync interface never binds, the async interface
defers forever, which is fine here.
- raw_event() now holds sd->lock and re-checks sd->removed so events
on either interface are serialised against removal.
- status_work runs once for async devices instead of rearming. A
single status request is sent when the headset connects to get the
initial battery level.
No device sets async_interface yet. This is the infrastructure for the
next commit.
Signed-off-by: Sriman Achanta <srimanachanta@gmail.com>
---
drivers/hid/hid-steelseries-arctis.c | 158 ++++++++++++++++++++++-----
1 file changed, 131 insertions(+), 27 deletions(-)
diff --git a/drivers/hid/hid-steelseries-arctis.c b/drivers/hid/hid-steelseries-arctis.c
index e534aa44e70a..9960c0ec512b 100644
--- a/drivers/hid/hid-steelseries-arctis.c
+++ b/drivers/hid/hid-steelseries-arctis.c
@@ -26,6 +26,7 @@ struct steelseries_device_info {
unsigned long capabilities;
u8 sync_interface;
+ u8 async_interface;
int (*request_status)(struct hid_device *hdev);
void (*parse_status)(struct steelseries_device *sd, u8 *data, int size);
@@ -271,7 +272,8 @@ static void steelseries_status_timer_work_handler(struct work_struct *work)
sd->info->request_status(sd->hdev);
spin_lock_irqsave(&sd->lock, flags);
- if (!sd->removed)
+ /* Async devices push status events themselves; only poll once. */
+ if (!sd->removed && !sd->info->async_interface)
schedule_delayed_work(&sd->status_work,
msecs_to_jiffies(STEELSERIES_HEADSET_STATUS_TIMEOUT_MS));
spin_unlock_irqrestore(&sd->lock, flags);
@@ -318,6 +320,53 @@ static int steelseries_battery_register(struct steelseries_device *sd)
return 0;
}
+static struct hid_driver steelseries_arctis_driver;
+
+static struct steelseries_device *
+steelseries_get_sibling_sd(struct hid_device *hdev, int interface_num)
+{
+ struct usb_interface *intf = to_usb_interface(hdev->dev.parent);
+ struct usb_device *usb_dev = interface_to_usbdev(intf);
+ struct usb_interface *sibling_intf;
+ struct hid_device *sibling_hdev;
+ struct steelseries_device *sd = NULL;
+
+ sibling_intf = usb_ifnum_to_if(usb_dev, interface_num);
+ if (!sibling_intf)
+ return NULL;
+
+ /*
+ * usb_get_intfdata() only yields a hid_device when usbhid is bound;
+ * gate on the descriptor class so a non-HID sibling (e.g. a crafted
+ * device exposing storage or audio here) is never treated as one.
+ */
+ if (sibling_intf->cur_altsetting->desc.bInterfaceClass != USB_INTERFACE_CLASS_HID)
+ return NULL;
+
+ /*
+ * Take the sibling's device lock across the intfdata read and the
+ * kref_get so a concurrent unbind cannot free the hid_device underneath
+ * us; usbhid leaves intfdata dangling on disconnect, so dev.driver is
+ * the reliable "still bound" test under this lock. Use device_trylock()
+ * to stay off the lockdep chain of the interface being probed and let
+ * the caller retry via -EPROBE_DEFER if the sibling is momentarily busy.
+ */
+ if (!device_trylock(&sibling_intf->dev))
+ return NULL;
+ if (sibling_intf->dev.driver) {
+ sibling_hdev = usb_get_intfdata(sibling_intf);
+ if (sibling_hdev &&
+ sibling_hdev->driver == &steelseries_arctis_driver) {
+ sd = hid_get_drvdata(sibling_hdev);
+ if (sd)
+ kref_get(&sd->refcnt);
+ }
+ }
+ device_unlock(&sibling_intf->dev);
+
+ return sd;
+}
+
static int steelseries_arctis_probe(struct hid_device *hdev,
const struct hid_device_id *id)
{
@@ -339,43 +388,81 @@ static int steelseries_arctis_probe(struct hid_device *hdev,
if (ret)
return ret;
- /* Let hid-generic handle non-sync interfaces */
- if (interface_num != info->sync_interface)
+ /* Let hid-generic handle non-vendor or unknown interfaces */
+ if (interface_num != info->sync_interface &&
+ (!info->async_interface || interface_num != info->async_interface))
return hid_hw_start(hdev, HID_CONNECT_DEFAULT);
- sd = kzalloc_obj(*sd, GFP_KERNEL);
- if (!sd)
- return -ENOMEM;
-
- kref_init(&sd->refcnt);
- sd->hdev = hdev;
- sd->info = info;
- spin_lock_init(&sd->lock);
+ if (interface_num == info->sync_interface) {
+ sd = kzalloc_obj(*sd, GFP_KERNEL);
+ if (!sd)
+ return -ENOMEM;
+
+ kref_init(&sd->refcnt);
+ sd->hdev = hdev;
+ sd->info = info;
+ spin_lock_init(&sd->lock);
+ INIT_DELAYED_WORK(&sd->status_work, steelseries_status_timer_work_handler);
+
+ ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
+ if (ret)
+ goto err_free;
+
+ ret = hid_hw_open(hdev);
+ if (ret)
+ goto err_stop;
+
+ if (info->capabilities & SS_CAP_BATTERY) {
+ ret = steelseries_battery_register(sd);
+ if (ret < 0)
+ hid_warn(hdev, "Failed to register battery: %d\n", ret);
+ }
- hid_set_drvdata(hdev, sd);
+ /*
+ * Publish drvdata only once fully initialised: the async sibling
+ * attaches by reading it, so it must never observe a half-built or
+ * failed instance. A failed probe never gets here, so the error
+ * path below has nothing to unpublish.
+ */
+ hid_set_drvdata(hdev, sd);
+ schedule_delayed_work(&sd->status_work, msecs_to_jiffies(100));
- ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
- if (ret)
- goto err_put;
+ return 0;
+ }
- ret = hid_hw_open(hdev);
- if (ret)
- goto err_stop;
+ /*
+ * The async interface shares the steelseries_device created by the
+ * sync interface. Defer until the sync interface has probed and
+ * published its drvdata.
+ */
+ if (info->async_interface && interface_num == info->async_interface) {
+ sd = steelseries_get_sibling_sd(hdev, info->sync_interface);
+ if (!sd)
+ return -EPROBE_DEFER;
+
+ hid_set_drvdata(hdev, sd);
+
+ ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
+ if (ret) {
+ kref_put(&sd->refcnt, steelseries_device_release);
+ return ret;
+ }
- if (info->capabilities & SS_CAP_BATTERY) {
- ret = steelseries_battery_register(sd);
- if (ret < 0)
- hid_warn(hdev, "Failed to register battery: %d\n", ret);
+ ret = hid_hw_open(hdev);
+ if (ret) {
+ hid_hw_stop(hdev);
+ kref_put(&sd->refcnt, steelseries_device_release);
+ return ret;
+ }
+ return 0;
}
- INIT_DELAYED_WORK(&sd->status_work, steelseries_status_timer_work_handler);
- schedule_delayed_work(&sd->status_work, msecs_to_jiffies(100));
-
- return 0;
+ return -ENODEV;
err_stop:
hid_hw_stop(hdev);
-err_put:
+err_free:
+ /* drvdata is unpublished until full success, so no sibling can hold sd. */
kref_put(&sd->refcnt, steelseries_device_release);
return ret;
}
@@ -428,10 +515,21 @@ static int steelseries_arctis_raw_event(struct hid_device *hdev,
u8 old_capacity;
bool old_connected;
bool old_charging;
+ bool is_async_interface;
+ unsigned long flags;
if (!sd)
return 0;
+ is_async_interface = (hdev != sd->hdev);
+
+ spin_lock_irqsave(&sd->lock, flags);
+
+ if (sd->removed) {
+ spin_unlock_irqrestore(&sd->lock, flags);
+ return 0;
+ }
+
old_capacity = sd->battery_capacity;
old_connected = sd->headset_connected;
old_charging = sd->battery_charging;
@@ -444,6 +542,10 @@ static int steelseries_arctis_raw_event(struct hid_device *hdev,
old_connected ? "" : "not ",
sd->headset_connected ? "" : "not ");
+ if (sd->headset_connected && !old_connected &&
+ sd->info->async_interface && is_async_interface)
+ schedule_delayed_work(&sd->status_work, 0);
+
if (sd->battery) {
steelseries_headset_set_wireless_status(sd->hdev,
sd->headset_connected);
@@ -467,6 +569,8 @@ static int steelseries_arctis_raw_event(struct hid_device *hdev,
power_supply_changed(sd->battery);
}
+ spin_unlock_irqrestore(&sd->lock, flags);
+
return 0;
}
--
2.55.0
^ permalink raw reply related
* [PATCH v6 9/9] HID: steelseries: Add support for Arctis Nova 5X and Nova 7 families
From: Sriman Achanta @ 2026-07-09 3:23 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: linux-input, linux-kernel, Bastien Nocera, Simon Wood,
Christian Mayer, Sriman Achanta
In-Reply-To: <20260709032316.1908460-1-68172138+srimanachanta@users.noreply.github.com>
From: Sriman Achanta <srimanachanta@gmail.com>
The Arctis Nova 5X, Nova 7 Gen2, and Nova 7 2026 refresh headsets answer
status polls, but also send unsolicited battery and connection updates
on a second HID interface (interface 5). Use that interface through
async_interface so the driver does not have to poll. Add request and
parse helpers for the Nova status format (0xb0/0xb7/0xb9/0xbb opcodes).
The original (pre-Gen2) Arctis Nova 7 family answers the same status
opcodes on the same interfaces, but reports battery capacity as a
discrete 0-4 level instead of a raw percentage. Add a second parse
function that maps that level through steelseries_map_capacity(), and a
separate device_info so the two capacity encodings cannot get mixed up.
Add the fourteen USB product IDs that share these protocols:
0x2253 Arctis Nova 5X
0x2202 Arctis Nova 7
0x2206 Arctis Nova 7X
0x22a4 Arctis Nova 7X (alternate PID)
0x223a Arctis Nova 7 Diablo
0x227a Arctis Nova 7 World of Warcraft Edition
0x22a1 Arctis Nova 7 2026
0x22a7 Arctis Nova 7P 2026
0x22a5 Arctis Nova 7X 2026
0x22a9 Arctis Nova 7 Diablo 2026
0x227e Arctis Nova 7 Gen 2
0x2258 Arctis Nova 7X Gen 2
0x229e Arctis Nova 7X Gen 2 (alternate PID)
0x22ad Arctis Nova 7X Gen 2 (alternate PID)
Signed-off-by: Sriman Achanta <srimanachanta@gmail.com>
---
drivers/hid/hid-ids.h | 14 ++++
drivers/hid/hid-quirks.c | 14 ++++
drivers/hid/hid-steelseries-arctis.c | 117 +++++++++++++++++++++++++++
3 files changed, 145 insertions(+)
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 915e936cbf8b..92c685a6d3e5 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -1379,6 +1379,20 @@
#define USB_DEVICE_ID_STEELSERIES_SRWS1 0x1410
#define USB_DEVICE_ID_STEELSERIES_ARCTIS_1_X 0x12b6
#define USB_DEVICE_ID_STEELSERIES_ARCTIS_9 0x12c2
+#define USB_DEVICE_ID_STEELSERIES_ARCTIS_NOVA_5_X 0x2253
+#define USB_DEVICE_ID_STEELSERIES_ARCTIS_NOVA_7 0x2202
+#define USB_DEVICE_ID_STEELSERIES_ARCTIS_NOVA_7_X 0x2206
+#define USB_DEVICE_ID_STEELSERIES_ARCTIS_NOVA_7_X_2 0x22a4
+#define USB_DEVICE_ID_STEELSERIES_ARCTIS_NOVA_7_DIABLO 0x223a
+#define USB_DEVICE_ID_STEELSERIES_ARCTIS_NOVA_7_WOW 0x227a
+#define USB_DEVICE_ID_STEELSERIES_ARCTIS_NOVA_7_2026 0x22a1
+#define USB_DEVICE_ID_STEELSERIES_ARCTIS_NOVA_7_P_2026 0x22a7
+#define USB_DEVICE_ID_STEELSERIES_ARCTIS_NOVA_7_X_2026 0x22a5
+#define USB_DEVICE_ID_STEELSERIES_ARCTIS_NOVA_7_DIABLO_2026 0x22a9
+#define USB_DEVICE_ID_STEELSERIES_ARCTIS_NOVA_7_GEN2 0x227e
+#define USB_DEVICE_ID_STEELSERIES_ARCTIS_NOVA_7_X_GEN2 0x2258
+#define USB_DEVICE_ID_STEELSERIES_ARCTIS_NOVA_7_X_GEN2_2 0x229e
+#define USB_DEVICE_ID_STEELSERIES_ARCTIS_NOVA_7_X_GEN2_3 0x22ad
#define USB_VENDOR_ID_SUN 0x0430
#define USB_DEVICE_ID_RARITAN_KVM_DONGLE 0xcdab
diff --git a/drivers/hid/hid-quirks.c b/drivers/hid/hid-quirks.c
index f546179858c2..eef2c2106dd7 100644
--- a/drivers/hid/hid-quirks.c
+++ b/drivers/hid/hid-quirks.c
@@ -749,6 +749,20 @@ static const struct hid_device_id hid_have_special_driver[] = {
{ HID_USB_DEVICE(USB_VENDOR_ID_STEELSERIES, USB_DEVICE_ID_STEELSERIES_SRWS1) },
{ HID_USB_DEVICE(USB_VENDOR_ID_STEELSERIES, USB_DEVICE_ID_STEELSERIES_ARCTIS_1_X) },
{ HID_USB_DEVICE(USB_VENDOR_ID_STEELSERIES, USB_DEVICE_ID_STEELSERIES_ARCTIS_9) },
+ { HID_USB_DEVICE(USB_VENDOR_ID_STEELSERIES, USB_DEVICE_ID_STEELSERIES_ARCTIS_NOVA_5_X) },
+ { HID_USB_DEVICE(USB_VENDOR_ID_STEELSERIES, USB_DEVICE_ID_STEELSERIES_ARCTIS_NOVA_7) },
+ { HID_USB_DEVICE(USB_VENDOR_ID_STEELSERIES, USB_DEVICE_ID_STEELSERIES_ARCTIS_NOVA_7_X) },
+ { HID_USB_DEVICE(USB_VENDOR_ID_STEELSERIES, USB_DEVICE_ID_STEELSERIES_ARCTIS_NOVA_7_X_2) },
+ { HID_USB_DEVICE(USB_VENDOR_ID_STEELSERIES, USB_DEVICE_ID_STEELSERIES_ARCTIS_NOVA_7_DIABLO) },
+ { HID_USB_DEVICE(USB_VENDOR_ID_STEELSERIES, USB_DEVICE_ID_STEELSERIES_ARCTIS_NOVA_7_WOW) },
+ { HID_USB_DEVICE(USB_VENDOR_ID_STEELSERIES, USB_DEVICE_ID_STEELSERIES_ARCTIS_NOVA_7_2026) },
+ { HID_USB_DEVICE(USB_VENDOR_ID_STEELSERIES, USB_DEVICE_ID_STEELSERIES_ARCTIS_NOVA_7_P_2026) },
+ { HID_USB_DEVICE(USB_VENDOR_ID_STEELSERIES, USB_DEVICE_ID_STEELSERIES_ARCTIS_NOVA_7_X_2026) },
+ { HID_USB_DEVICE(USB_VENDOR_ID_STEELSERIES, USB_DEVICE_ID_STEELSERIES_ARCTIS_NOVA_7_DIABLO_2026) },
+ { HID_USB_DEVICE(USB_VENDOR_ID_STEELSERIES, USB_DEVICE_ID_STEELSERIES_ARCTIS_NOVA_7_GEN2) },
+ { HID_USB_DEVICE(USB_VENDOR_ID_STEELSERIES, USB_DEVICE_ID_STEELSERIES_ARCTIS_NOVA_7_X_GEN2) },
+ { HID_USB_DEVICE(USB_VENDOR_ID_STEELSERIES, USB_DEVICE_ID_STEELSERIES_ARCTIS_NOVA_7_X_GEN2_2) },
+ { HID_USB_DEVICE(USB_VENDOR_ID_STEELSERIES, USB_DEVICE_ID_STEELSERIES_ARCTIS_NOVA_7_X_GEN2_3) },
#endif
#if IS_ENABLED(CONFIG_HID_SUNPLUS)
{ HID_USB_DEVICE(USB_VENDOR_ID_SUNPLUS, USB_DEVICE_ID_SUNPLUS_WDESKTOP) },
diff --git a/drivers/hid/hid-steelseries-arctis.c b/drivers/hid/hid-steelseries-arctis.c
index 9960c0ec512b..23fb0cebd72a 100644
--- a/drivers/hid/hid-steelseries-arctis.c
+++ b/drivers/hid/hid-steelseries-arctis.c
@@ -108,6 +108,13 @@ static int steelseries_arctis_9_request_status(struct hid_device *hdev)
return steelseries_send_output_report(hdev, data, sizeof(data));
}
+static int steelseries_arctis_nova_request_status(struct hid_device *hdev)
+{
+ const u8 data[] = { 0x00, 0xb0 };
+
+ return steelseries_send_output_report(hdev, data, sizeof(data));
+}
+
/*
* Headset battery helpers
*/
@@ -162,6 +169,58 @@ static void steelseries_arctis_9_parse_status(struct steelseries_device *sd,
}
}
+static void steelseries_arctis_nova_parse_status(struct steelseries_device *sd,
+ u8 *data, int size)
+{
+ if (size < 2)
+ return;
+
+ switch (data[0]) {
+ case 0xb0:
+ if (size < 4)
+ return;
+ sd->headset_connected = (data[1] == 0x03);
+ sd->battery_capacity = data[2];
+ sd->battery_charging = (data[3] == 0x01);
+ break;
+ case 0xb7:
+ sd->battery_capacity = data[1];
+ break;
+ case 0xb9:
+ sd->headset_connected = (data[1] == 0x03);
+ break;
+ case 0xbb:
+ sd->battery_charging = (data[1] == 0x01);
+ break;
+ }
+}
+
+static void steelseries_arctis_nova_7_parse_status(struct steelseries_device *sd,
+ u8 *data, int size)
+{
+ if (size < 2)
+ return;
+
+ switch (data[0]) {
+ case 0xb0:
+ if (size < 4)
+ return;
+ sd->headset_connected = (data[1] == 0x03);
+ sd->battery_capacity = steelseries_map_capacity(data[2], 0, 4);
+ sd->battery_charging = (data[3] == 0x01);
+ break;
+ case 0xb7:
+ sd->battery_capacity = steelseries_map_capacity(data[1], 0, 4);
+ break;
+ case 0xb9:
+ sd->headset_connected = (data[1] == 0x03);
+ break;
+ case 0xbb:
+ sd->battery_charging = (data[1] == 0x01);
+ break;
+ }
+}
+
/*
* Device info definitions
*/
@@ -180,6 +239,22 @@ static const struct steelseries_device_info arctis_9_info = {
.parse_status = steelseries_arctis_9_parse_status,
};
+static const struct steelseries_device_info arctis_nova_info = {
+ .sync_interface = 3,
+ .async_interface = 5,
+ .capabilities = SS_CAP_BATTERY,
+ .request_status = steelseries_arctis_nova_request_status,
+ .parse_status = steelseries_arctis_nova_parse_status,
+};
+
+static const struct steelseries_device_info arctis_nova_7_info = {
+ .sync_interface = 3,
+ .async_interface = 5,
+ .capabilities = SS_CAP_BATTERY,
+ .request_status = steelseries_arctis_nova_request_status,
+ .parse_status = steelseries_arctis_nova_7_parse_status,
+};
+
/*
* Headset wireless status and battery infrastructure
*/
@@ -581,6 +656,48 @@ static const struct hid_device_id steelseries_arctis_devices[] = {
{ HID_USB_DEVICE(USB_VENDOR_ID_STEELSERIES,
USB_DEVICE_ID_STEELSERIES_ARCTIS_9),
.driver_data = (unsigned long)&arctis_9_info },
+ { HID_USB_DEVICE(USB_VENDOR_ID_STEELSERIES,
+ USB_DEVICE_ID_STEELSERIES_ARCTIS_NOVA_5_X),
+ .driver_data = (unsigned long)&arctis_nova_info },
+ { HID_USB_DEVICE(USB_VENDOR_ID_STEELSERIES,
+ USB_DEVICE_ID_STEELSERIES_ARCTIS_NOVA_7),
+ .driver_data = (unsigned long)&arctis_nova_7_info },
+ { HID_USB_DEVICE(USB_VENDOR_ID_STEELSERIES,
+ USB_DEVICE_ID_STEELSERIES_ARCTIS_NOVA_7_X),
+ .driver_data = (unsigned long)&arctis_nova_7_info },
+ { HID_USB_DEVICE(USB_VENDOR_ID_STEELSERIES,
+ USB_DEVICE_ID_STEELSERIES_ARCTIS_NOVA_7_X_2),
+ .driver_data = (unsigned long)&arctis_nova_7_info },
+ { HID_USB_DEVICE(USB_VENDOR_ID_STEELSERIES,
+ USB_DEVICE_ID_STEELSERIES_ARCTIS_NOVA_7_DIABLO),
+ .driver_data = (unsigned long)&arctis_nova_7_info },
+ { HID_USB_DEVICE(USB_VENDOR_ID_STEELSERIES,
+ USB_DEVICE_ID_STEELSERIES_ARCTIS_NOVA_7_WOW),
+ .driver_data = (unsigned long)&arctis_nova_7_info },
+ { HID_USB_DEVICE(USB_VENDOR_ID_STEELSERIES,
+ USB_DEVICE_ID_STEELSERIES_ARCTIS_NOVA_7_2026),
+ .driver_data = (unsigned long)&arctis_nova_info },
+ { HID_USB_DEVICE(USB_VENDOR_ID_STEELSERIES,
+ USB_DEVICE_ID_STEELSERIES_ARCTIS_NOVA_7_P_2026),
+ .driver_data = (unsigned long)&arctis_nova_info },
+ { HID_USB_DEVICE(USB_VENDOR_ID_STEELSERIES,
+ USB_DEVICE_ID_STEELSERIES_ARCTIS_NOVA_7_X_2026),
+ .driver_data = (unsigned long)&arctis_nova_info },
+ { HID_USB_DEVICE(USB_VENDOR_ID_STEELSERIES,
+ USB_DEVICE_ID_STEELSERIES_ARCTIS_NOVA_7_DIABLO_2026),
+ .driver_data = (unsigned long)&arctis_nova_info },
+ { HID_USB_DEVICE(USB_VENDOR_ID_STEELSERIES,
+ USB_DEVICE_ID_STEELSERIES_ARCTIS_NOVA_7_GEN2),
+ .driver_data = (unsigned long)&arctis_nova_info },
+ { HID_USB_DEVICE(USB_VENDOR_ID_STEELSERIES,
+ USB_DEVICE_ID_STEELSERIES_ARCTIS_NOVA_7_X_GEN2),
+ .driver_data = (unsigned long)&arctis_nova_info },
+ { HID_USB_DEVICE(USB_VENDOR_ID_STEELSERIES,
+ USB_DEVICE_ID_STEELSERIES_ARCTIS_NOVA_7_X_GEN2_2),
+ .driver_data = (unsigned long)&arctis_nova_info },
+ { HID_USB_DEVICE(USB_VENDOR_ID_STEELSERIES,
+ USB_DEVICE_ID_STEELSERIES_ARCTIS_NOVA_7_X_GEN2_3),
+ .driver_data = (unsigned long)&arctis_nova_info },
{}
};
MODULE_DEVICE_TABLE(hid, steelseries_arctis_devices);
--
2.55.0
^ permalink raw reply related
* Re: [PATCH v6 4/9] HID: steelseries: Refactor Arctis driver to use device_info framework
From: sashiko-bot @ 2026-07-09 3:33 UTC (permalink / raw)
To: Sriman Achanta; +Cc: linux-input, dmitry.torokhov
In-Reply-To: <20260709032316.1908460-5-68172138+srimanachanta@users.noreply.github.com>
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
New issues:
- [Medium] Wireless status updates are incorrectly gated by battery registration success, causing a feature regression.
- [Medium] Relaxing the Arctis 9 battery report size check may cause spurious headset disconnects.
Pre-existing issues:
- [High] A race condition exists during initialization where `sd->battery` can temporarily hold an `ERR_PTR`, leading to a potential kernel panic if `raw_event` triggers concurrently.
--
--- Patch [4]: [PATCH v6 4/9] HID: steelseries: Refactor Arctis driver to use device_info framework ---
commit 83accfc4c1be2885884d77d1f5dcd98d620ce34b
Author: Sriman Achanta <srimanachanta@gmail.com>
HID: steelseries: Refactor Arctis driver to use device_info framework
Replace the per-product if/else quirk bitmap with a
steelseries_device_info struct. Each model provides its capabilities,
sync_interface, and request_status/parse_status callbacks. Report
sending is folded into steelseries_send_report() and the feature and
output wrappers, and the battery identifiers lose their per-model names.
> diff --git a/drivers/hid/hid-steelseries-arctis.c b/drivers/hid/hid-steelseries-arctis.c
> index 079504e6932a1..c54c56db9ddd7 100644
> --- a/drivers/hid/hid-steelseries-arctis.c
> +++ b/drivers/hid/hid-steelseries-arctis.c
[ ... ]
> @@ -88,30 +108,101 @@ static int battery_capacity_to_level(int capacity)
> +static void steelseries_arctis_9_parse_status(struct steelseries_device *sd,
> + u8 *data, int size)
> +{
> + if (size < 5)
> + return;
> +
> + if (data[0] == 0xaa && data[1] == 0x01) {
> + sd->headset_connected = true;
> + sd->battery_charging = (data[4] == 0x01);
> + sd->battery_capacity = steelseries_map_capacity(data[3], 0x68, 0x9d);
> + } else {
> + /* Device off: 0x55 (no status) or 0x03 (stale status). */
> + sd->headset_connected = false;
> + sd->battery_charging = false;
> + }
> +}
[Severity: Medium]
Could this relaxed size check cause spurious disconnects?
The original code checked for ARCTIS_9_BATTERY_RESPONSE_LEN (64 bytes). By
relaxing the check to 5 bytes, if the headset sends any other non-battery
report between 5 and 63 bytes, it will bypass the filter, fail the signature
check, and fall into the else block.
Would that incorrectly mark the headset as disconnected?
[ ... ]
> @@ -168,7 +244,26 @@ static enum power_supply_property steelseries_headset_battery_props[] = {
[ ... ]
> +static int steelseries_battery_register(struct steelseries_device *sd)
> {
> static atomic_t battery_no = ATOMIC_INIT(0);
> struct power_supply_config battery_cfg = { .drv_data = sd, };
> @@ -176,25 +271,27 @@ static int steelseries_headset_battery_register(struct steelseries_device *sd)
> int ret;
[ ... ]
> sd->battery = devm_power_supply_register(&sd->hdev->dev,
> &sd->battery_desc, &battery_cfg);
> if (IS_ERR(sd->battery)) {
> ret = PTR_ERR(sd->battery);
> + sd->battery = NULL;
> hid_err(sd->hdev,
> "%s:power_supply_register failed with error %d\n",
> __func__, ret);
[Severity: High]
This is a pre-existing issue, but can this cause a kernel panic if a report
arrives concurrently?
Since hid_hw_start() is called earlier during probe, could raw_event()
trigger while sd->battery temporarily holds an ERR_PTR?
If raw_event() fires before sd->battery is set to NULL, it sees a non-NULL
pointer and passes it to power_supply_changed(), which might dereference the
error pointer and crash.
[ ... ]
> @@ -262,130 +365,92 @@ static void steelseries_arctis_remove(struct hid_device *hdev)
[ ... ]
> static int steelseries_arctis_raw_event(struct hid_device *hdev,
> - struct hid_report *report, u8 *read_buf,
> - int size)
> + struct hid_report *report, u8 *data, int size)
> {
[ ... ]
> - if (connected != sd->headset_connected) {
> - hid_dbg(sd->hdev,
> + if (sd->headset_connected != old_connected) {
> + hid_dbg(hdev,
> "Connected status changed from %sconnected to %sconnected\n",
> - sd->headset_connected ? "" : "not ",
> - connected ? "" : "not ");
> - sd->headset_connected = connected;
> - steelseries_headset_set_wireless_status(hdev, connected);
> + old_connected ? "" : "not ",
> + sd->headset_connected ? "" : "not ");
> +
> + if (sd->battery) {
> + steelseries_headset_set_wireless_status(sd->hdev,
> + sd->headset_connected);
> + power_supply_changed(sd->battery);
> + }
> }
[Severity: Medium]
Does this check unintentionally gate the wireless status update behind the
battery registration?
If battery registration fails (and sd->battery is NULL), it looks like the
USB device's wireless status will no longer be reported to userspace. Is
this expected behavior?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260709032316.1908460-1-68172138+srimanachanta@users.noreply.github.com?part=4
^ permalink raw reply
* Re: [PATCH v6 2/9] HID: steelseries: Split Arctis headset driver into separate module
From: sashiko-bot @ 2026-07-09 3:34 UTC (permalink / raw)
To: Sriman Achanta; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260709032316.1908460-3-68172138+srimanachanta@users.noreply.github.com>
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
Pre-existing issues:
- [High] If `steelseries_headset_battery_register()` fails, the probe function ignores the error and continues. This leaves `sd->battery` as an `ERR_PTR` and `sd->battery_work` uninitialized. Later, incoming events in `steelseries_arctis_raw_event()` call `power_supply_changed(sd->battery)` and `schedule_delayed_work(&sd->battery_work, ...)`, which will dereference the error pointer and operate on an uninitialized work structure, causing a kernel panic. Similarly, `steelseries_arctis_remove()` calls `cancel_delayed_work_sync(&sd->battery_work)` which will also crash.
- [High] The driver calls `hid_hw_start()` and `hid_hw_open()` before calling `steelseries_headset_battery_register()`. `hid_hw_open()` starts the URB polling, enabling asynchronous hardware events to trigger `steelseries_arctis_raw_event()` immediately. At this point, `sd->battery` is still `NULL` and `sd->battery_work` is uninitialized. If a report is received during this race window, the event handler will pass a `NULL` pointer to `power_supply_changed()` and call `schedule_delayed_work()` on uninitialized work, resulting in an immediate kernel panic.
- [High] Missing length check on the report descriptor before accessing payload offsets in `steelseries_is_vendor_usage_page()`. The function unconditionally accesses `hdev->rdesc[0]`, `hdev->rdesc[1]`, and `hdev->rdesc[2]`. Although `usbhid_parse()` guarantees `rsize > 0`, it does not guarantee `rsize >= 3`. If a malformed USB device presents a 1- or 2-byte descriptor, this reads out of bounds of the kmalloc'd `rdesc` buffer.
--
--- Patch [2]: [PATCH v6 2/9] HID: steelseries: Split Arctis headset driver into separate module ---
commit 5114a40b6cbe9d3f2ea95a54f76bb50da5fb5d42
Author: Sriman Achanta <srimanachanta@gmail.com>
HID: steelseries: Split Arctis headset driver into separate module
This patch moves the Arctis headset code from hid-steelseries.c into a
new hid-steelseries-arctis.c module. It separates the headset handling
from the SRWS1 racing wheel code.
> diff --git a/drivers/hid/hid-steelseries-arctis.c b/drivers/hid/hid-steelseries-arctis.c
> new file mode 100644
> index 0000000000000..079504e6932a1
> --- /dev/null
> +++ b/drivers/hid/hid-steelseries-arctis.c
[ ... ]
> +static bool steelseries_is_vendor_usage_page(struct hid_device *hdev, uint8_t usage_page)
> +{
> + return hdev->rdesc[0] == 0x06 &&
> + hdev->rdesc[1] == usage_page &&
> + hdev->rdesc[2] == 0xff;
> +}
[Severity: High]
This is a pre-existing issue, but does this function properly validate
the size of the report descriptor before accessing it?
The function unconditionally accesses the first 3 bytes of hdev->rdesc.
Although hid_parse() guarantees rsize > 0, if a malformed USB device
presents a 1- or 2-byte descriptor, wouldn't this read out of bounds
of the allocated rdesc buffer?
[ ... ]
> +static int steelseries_arctis_probe(struct hid_device *hdev, const struct hid_device_id *id)
> +{
[ ... ]
> + ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
> + if (ret)
> + return ret;
> +
> + ret = hid_hw_open(hdev);
> + if (ret)
> + return ret;
[Severity: High]
This isn't a bug introduced by this patch, but does calling hid_hw_open()
before steelseries_headset_battery_register() introduce a race condition?
Calling hid_hw_open() starts the hardware polling, which enables
asynchronous events to immediately trigger steelseries_arctis_raw_event().
At this point, sd->battery is still NULL and sd->battery_work is
uninitialized.
If a report is received during this race window, would the event handler
pass a NULL pointer to power_supply_changed() and schedule uninitialized
work, resulting in a crash?
> +
> + if (steelseries_headset_battery_register(sd) < 0)
> + hid_err(sd->hdev,
> + "Failed to register battery for headset\n");
> +
> + return ret;
> +}
[Severity: High]
This is a pre-existing issue, but is it safe to ignore the return value
of steelseries_headset_battery_register() here?
If registration fails (for example, due to a duplicate name), the probe
function prints an error but still succeeds. This leaves sd->battery as
an ERR_PTR and sd->battery_work uninitialized.
If a subsequent event is processed by steelseries_arctis_raw_event(),
it will call power_supply_changed() and dereference the error pointer.
Furthermore, steelseries_arctis_remove() will call
cancel_delayed_work_sync() on an uninitialized structure, which can
crash the workqueue.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260709032316.1908460-1-68172138+srimanachanta@users.noreply.github.com?part=2
^ permalink raw reply
* Re: [PATCH v6 1/9] HID: steelseries: Fix ARCTIS_1_X device mislabeling
From: sashiko-bot @ 2026-07-09 3:36 UTC (permalink / raw)
To: Sriman Achanta; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260709032316.1908460-2-68172138+srimanachanta@users.noreply.github.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues:
- [High] The probe function leaks hardware state and input devices if `hid_hw_open()` fails, leading to Use-After-Free.
- [High] Failure to register the power supply leaves an ERR_PTR and an uninitialized delayed_work, causing kernel panics on incoming events or device removal.
--
--- Patch [1]: [PATCH v6 1/9] HID: steelseries: Fix ARCTIS_1_X device mislabeling ---
commit c38516edb3d8e3340b9141595b245b3a3b7f06a6
Author: Sriman Achanta <srimanachanta@gmail.com>
HID: steelseries: Fix ARCTIS_1_X device mislabeling
The SteelSeries Arctis 1 Wireless for Xbox (0x12b6) was labelled as the
plain Arctis 1 Wireless. Rename USB_DEVICE_ID_STEELSERIES_ARCTIS_1 to
USB_DEVICE_ID_STEELSERIES_ARCTIS_1_X, along with the matching quirk flag
and device table entry. The device ID value is unchanged.
> diff --git a/drivers/hid/hid-steelseries.c b/drivers/hid/hid-steelseries.c
> index f98435631aa18..fd38ee3ea6fcb 100644
> --- a/drivers/hid/hid-steelseries.c
> +++ b/drivers/hid/hid-steelseries.c
[ ... ]
> @@ -638,7 +638,7 @@ static int steelseries_headset_raw_event(struct hid_device *hdev,
> if (hdev->product == USB_DEVICE_ID_STEELSERIES_SRWS1)
> return 0;
>
> - if (hdev->product == USB_DEVICE_ID_STEELSERIES_ARCTIS_1) {
> + if (hdev->product == USB_DEVICE_ID_STEELSERIES_ARCTIS_1_X) {
> hid_dbg(sd->hdev,
> "Parsing raw event for Arctis 1 headset (%*ph)\n", size, read_buf);
[Severity: High]
This is a pre-existing issue, but I noticed a potential problem in how
headset events are handled if battery registration fails.
Looking at steelseries_probe(), if the battery registration fails it logs an
error but returns success:
drivers/hid/hid-steelseries.c:steelseries_probe() {
...
if (steelseries_headset_battery_register(sd) < 0)
hid_err(sd->hdev,
"Failed to register battery for headset\n");
return ret;
}
This leaves sd->battery containing an error pointer, and skips initializing
the battery_work structure. When an event is received later, the headset raw
event handler proceeds to use these uninitialized members:
drivers/hid/hid-steelseries.c:steelseries_headset_raw_event() {
...
power_supply_changed(sd->battery);
...
schedule_delayed_work(&sd->battery_work,
msecs_to_jiffies(STEELSERIES_HEADSET_BATTERY_TIMEOUT_MS));
}
Could this cause a panic from dereferencing the error pointer in
power_supply_changed(), and trigger an oops when schedule_delayed_work()
runs on the uninitialized work structure?
[Severity: High]
This is another pre-existing issue, but does steelseries_probe() properly
clean up resources if hardware open fails?
drivers/hid/hid-steelseries.c:steelseries_probe() {
...
ret = hid_hw_open(hdev);
if (ret)
return ret;
...
}
If hid_hw_open() fails, the function returns directly. Would this bypass
calling hid_hw_stop() to unregister the devices allocated by the earlier
call to hid_hw_start()?
Could this leak the input devices and cause a use-after-free when the
parent device is subsequently removed?
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260709032316.1908460-1-68172138+srimanachanta@users.noreply.github.com?part=1
^ permalink raw reply
* Re: [PATCH v6 8/9] HID: steelseries: Add async status interface support
From: sashiko-bot @ 2026-07-09 3:37 UTC (permalink / raw)
To: Sriman Achanta; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260709032316.1908460-9-68172138+srimanachanta@users.noreply.github.com>
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] A concurrent unbinding of the sync interface's HID driver via sysfs can result in a Use-After-Free of the `steelseries_device` object during the async interface's probe.
- [High] Blindly casting `usb_get_intfdata()` to `struct hid_device *` causes a Type Confusion and Out-Of-Bounds Read if another driver is bound to the sibling interface.
- [Medium] The `async_interface` field treats `0` as a sentinel value for 'not present', which will break for devices where the async interface is actually at index 0.
--
--- Patch [8]: [PATCH v6 8/9] HID: steelseries: Add async status interface support ---
commit b3e51c178159f4ed7f8fb3b18ebfac675c1ff9b7
Author: Sriman Achanta <srimanachanta@gmail.com>
HID: steelseries: Add async status interface support
This patch adds support for a second HID interface that asynchronously sends
battery and connection updates. It binds both sync and async interfaces,
deferring the async one until the sync interface is ready.
> diff --git a/drivers/hid/hid-steelseries-arctis.c b/drivers/hid/hid-steelseries-arctis.c
> index e534aa44e70a0..9960c0ec512bf 100644
> --- a/drivers/hid/hid-steelseries-arctis.c
> +++ b/drivers/hid/hid-steelseries-arctis.c
[ ... ]
> @@ -318,6 +320,53 @@ static int steelseries_battery_register(struct steelseries_device *sd)
> return 0;
> }
>
> +static struct hid_driver steelseries_arctis_driver;
> +
> +static struct steelseries_device *
> +steelseries_get_sibling_sd(struct hid_device *hdev, int interface_num)
> +{
[ ... ]
> + if (!device_trylock(&sibling_intf->dev))
> + return NULL;
> + if (sibling_intf->dev.driver) {
> + sibling_hdev = usb_get_intfdata(sibling_intf);
> + if (sibling_hdev &&
> + sibling_hdev->driver == &steelseries_arctis_driver) {
[Severity: High]
Does casting usb_get_intfdata() directly to a hid_device risk a type
confusion and out-of-bounds read?
If another driver (like usbfs via userspace claiming the interface) is
bound to the sync USB interface, usb_get_intfdata() would return that
driver's data (e.g., a much smaller struct usb_dev_state).
Reading sibling_hdev->driver would then read far past the bounds of that
smaller struct.
> + sd = hid_get_drvdata(sibling_hdev);
> + if (sd)
> + kref_get(&sd->refcnt);
[Severity: High]
Is there a race condition here that could lead to a use-after-free of
the steelseries_device object?
The code acquires the parent USB interface lock
(device_trylock(&sibling_intf->dev)), but not the child hid_device lock.
If the HID driver of the sync interface is unbound via sysfs while the
async interface is actively probing, steelseries_arctis_remove() could be
executed, freeing 'sd'.
Is there a window before the driver core clears sibling_hdev->driver and
drvdata where this probing thread could call kref_get() on the freed 'sd'?
> + }
> + }
> + device_unlock(&sibling_intf->dev);
> +
> + return sd;
> +}
[ ... ]
> @@ -339,43 +388,81 @@ static int steelseries_arctis_probe(struct hid_device *hdev,
> if (ret)
> return ret;
>
> - /* Let hid-generic handle non-sync interfaces */
> - if (interface_num != info->sync_interface)
> + /* Let hid-generic handle non-vendor or unknown interfaces */
> + if (interface_num != info->sync_interface &&
> + (!info->async_interface || interface_num != info->async_interface))
[Severity: Medium]
Will this logic prevent devices from using interface 0 as the async
interface?
USB interfaces are zero-indexed, making 0 a valid interface number, but
evaluating !info->async_interface treats 0 as a sentinel "not present"
value.
> return hid_hw_start(hdev, HID_CONNECT_DEFAULT);
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260709032316.1908460-1-68172138+srimanachanta@users.noreply.github.com?part=8
^ permalink raw reply
* Re: [PATCH v6 7/9] HID: steelseries: Manage battery lifetime with refcounting
From: sashiko-bot @ 2026-07-09 3:38 UTC (permalink / raw)
To: Sriman Achanta; +Cc: linux-input, dmitry.torokhov
In-Reply-To: <20260709032316.1908460-8-68172138+srimanachanta@users.noreply.github.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Lockless access to `sd->battery` in `steelseries_arctis_raw_event()` creates a TOCTOU race with `remove()`, potentially causing a Use-After-Free.
--
--- Patch [7]: [PATCH v6 7/9] HID: steelseries: Manage battery lifetime with refcounting ---
commit acac88b3e3a741b01e199d1d54e37c2b85ec3b35
Author: Sriman Achanta <srimanachanta@gmail.com>
HID: steelseries: Manage battery lifetime with refcounting
This patch converts the steelseries_device structure to use a kref so its state
can outlive a single HID interface. It also registers and unregisters the power
supply explicitly instead of using devm, and clears sd->battery under a
spinlock in the remove function.
> diff --git a/drivers/hid/hid-steelseries-arctis.c b/drivers/hid/hid-steelseries-arctis.c
> --- a/drivers/hid/hid-steelseries-arctis.c
> +++ b/drivers/hid/hid-steelseries-arctis.c
[ ... ]
> @@ -387,13 +405,20 @@ static void steelseries_arctis_remove(struct hid_device *hdev)
> if (interface_num == sd->info->sync_interface) {
> spin_lock_irqsave(&sd->lock, flags);
> sd->removed = true;
> + battery = sd->battery;
> + sd->battery = NULL;
> spin_unlock_irqrestore(&sd->lock, flags);
>
> cancel_delayed_work_sync(&sd->status_work);
> +
> + if (battery)
> + power_supply_unregister(battery);
> }
>
> hid_hw_close(hdev);
> hid_hw_stop(hdev);
[Severity: High]
Can this lead to a use-after-free if a battery status report arrives just as
the device is being removed?
In steelseries_arctis_raw_event(), sd->battery is accessed locklessly:
if (sd->battery) {
steelseries_headset_set_wireless_status(sd->hdev,
sd->headset_connected);
power_supply_changed(sd->battery);
}
If a raw event checks sd->battery and then gets preempted, this remove() path
can clear sd->battery and unregister the power supply because hid_hw_stop()
hasn't been called yet to halt the delivery of USB URBs.
When raw_event() resumes, wouldn't it call power_supply_changed() on the newly
freed battery pointer?
> +
> + kref_put(&sd->refcnt, steelseries_device_release);
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260709032316.1908460-1-68172138+srimanachanta@users.noreply.github.com?part=7
^ permalink raw reply
* [PATCH] Input: joydev - replace ev_match boolean with goto for early exit
From: Ashton Warner @ 2026-07-09 4:18 UTC (permalink / raw)
To: dmitry.torokhov; +Cc: linux-input, linux-kernel
The joydev_dev_is_absolute_mouse runs unnecessary Event type checks to
determine whether a joydev device is an absolute mouse. Change ev_match
to a goto statement to exit early.
Signed-off-by: Ashton Warner <drflamemontgomery@gmail.com>
---
drivers/input/joydev.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/input/joydev.c b/drivers/input/joydev.c
index 32459fd8a7c1..a726ba0be05b 100644
--- a/drivers/input/joydev.c
+++ b/drivers/input/joydev.c
@@ -819,7 +819,6 @@ static bool joydev_dev_is_blacklisted(struct input_dev *dev)
static bool joydev_dev_is_absolute_mouse(struct input_dev *dev)
{
DECLARE_BITMAP(jd_scratch, KEY_CNT);
- bool ev_match = false;
BUILD_BUG_ON(ABS_CNT > KEY_CNT || EV_CNT > KEY_CNT);
@@ -855,20 +854,21 @@ static bool joydev_dev_is_absolute_mouse(struct input_dev *dev)
__set_bit(EV_KEY, jd_scratch);
__set_bit(EV_SYN, jd_scratch);
if (bitmap_equal(jd_scratch, dev->evbit, EV_CNT))
- ev_match = true;
+ goto ev_match_pass;
/* HP ILO2, AMI BMC firmware */
__set_bit(EV_MSC, jd_scratch);
if (bitmap_equal(jd_scratch, dev->evbit, EV_CNT))
- ev_match = true;
+ goto ev_match_pass;
/* VMware Virtual USB Mouse, QEMU USB Tablet, ATEN BMC firmware */
__set_bit(EV_REL, jd_scratch);
if (bitmap_equal(jd_scratch, dev->evbit, EV_CNT))
- ev_match = true;
+ goto ev_match_pass;
- if (!ev_match)
- return false;
+ return false;
+
+ev_match_pass:
bitmap_zero(jd_scratch, ABS_CNT);
__set_bit(ABS_X, jd_scratch);
--
2.55.0
^ permalink raw reply related
* [PATCH v2 00/11] Remove support for platform data from samsung keypad
From: Dmitry Torokhov @ 2026-07-09 4:52 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Peter Griffin,
Alim Akhtar, Russell King, Mark Brown, Linus Walleij,
Charles Keepax, Sam Protsenko, Arnd Bergmann, Bartosz Golaszewski
Cc: Krzysztof Kozlowski, linux-input, devicetree, linux-kernel,
linux-arm-kernel, linux-samsung-soc, patches, Krzysztof Kozlowski
Hi,
This is a reworked and extended version of the series previously posted
to convert the Samsung keypad driver and Cragganmore 6410 board to
generic device properties. The first 8 patches of the original series
(general cleanups) have been merged into mainline, and this version
focuses on the remaining keypad rework and extends the board conversion.
Specifically, we rework the Samsung keypad driver to stop using platform
data and instead rely on generic device properties, and convert the
Cragganmore board to use software nodes for all its peripherals, removing
legacy GPIO lookup tables.
We start by introducing a compact matrix keypad binding and implementing
it in the driver. To support referencing Samsung GPIO chips in board
properties, we add infrastructure to register and attach software nodes
to Samsung gpio_chips. We then switch the Cragganmore keypad to use
software properties and drop platform data support from the driver.
To convert the PMIC DVS regulator on Cragganmore, we add software node
support to the wm831x regulator driver, allowing DVS configuration via
device properties. Once the board is converted, we clean up the driver
by removing legacy DVS platform data fields.
Finally, we convert the remaining peripherals on Cragganmore (GPIO keys,
PMIC, WM1250, SPI0, basic-mmio-gpio, and LEDs) to software properties.
This allows us to eliminate all legacy GPIO lookup tables, which also
fixes incorrect GPIO chip names ("GPIO<N>" vs "GP<N>") from previous
descriptor conversions.
Mark, it would be great if you could give this a spin on Cragganmore.
Thanks!
Changes in v2:
- Patches 1-8 of v1 were merged into mainline, dropping them from this
series
- Rebased remaining patches (9-14 of v1) onto latest next
- Resolved conflicts in mach-crag6410.c
- Removed redundant "out of memory" error messages from the keypad
driver
- Added an infrastructure patch to register and attach software nodes
for Samsung gpio_chips
- Converted basic-mmio-gpio and LEDs on Cragganmore 6410 to software
properties in a new separate patch
- Addressed DT bindings feedback on the compact binding patch
- Fixed proximity switch event type for GPIO keys
- Added support for software nodes in wm831x regulator driver
- Converted PMIC DVS on Cragganmore 6410 to software properties using
the new driver support
- Removed legacy DVS platform data fields and cleaned up driver
fallback.
---
Dmitry Torokhov (11):
dt-bindings: input: samsung,s3c6410-keypad: introduce compact binding
Input: samsung-keypad - handle compact binding
ARM: s3c: register and attach software nodes for Samsung gpio_chips
ARM: s3c: crag6410: switch keypad device to software properties
Input: samsung-keypad - remove support for platform data
ARM: s3c: crag6410: use software nodes/properties to set up GPIO keys
regulator: wm831x: support software node in platform data
ARM: s3c: crag6410: convert PMIC to software properties
regulator: wm831x: remove legacy DVS platform data
ARM: s3c: crag6410: convert remaining GPIO lookup tables to property entries
ARM: s3c: crag6410: convert basic-mmio-gpio and LEDs to software properties
.../bindings/input/samsung,s3c6410-keypad.yaml | 53 ++-
arch/arm/mach-s3c/Kconfig | 5 -
arch/arm/mach-s3c/Kconfig.s3c64xx | 1 -
arch/arm/mach-s3c/Makefile.s3c64xx | 1 -
arch/arm/mach-s3c/devs.c | 62 ----
arch/arm/mach-s3c/devs.h | 2 -
arch/arm/mach-s3c/gpio-core.h | 3 +
arch/arm/mach-s3c/gpio-samsung-s3c64xx.h | 5 +
arch/arm/mach-s3c/gpio-samsung.c | 72 +++-
arch/arm/mach-s3c/keypad.h | 27 --
arch/arm/mach-s3c/mach-crag6410.c | 368 +++++++++++++--------
arch/arm/mach-s3c/setup-keypad-s3c64xx.c | 20 --
drivers/input/keyboard/samsung-keypad.c | 192 ++++-------
drivers/regulator/wm831x-dcdc.c | 24 +-
include/linux/input/samsung-keypad.h | 39 ---
include/linux/mfd/wm831x/pdata.h | 5 +-
16 files changed, 445 insertions(+), 434 deletions(-)
---
base-commit: 8e9685d3c41c35dd1b37df70d854137abcb2fbac
change-id: 20240819-samsung-kp-ceedd4f2f720
Thanks.
--
Dmitry
^ permalink raw reply
* [PATCH v2 01/11] dt-bindings: input: samsung,s3c6410-keypad: introduce compact binding
From: Dmitry Torokhov @ 2026-07-09 4:52 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Peter Griffin,
Alim Akhtar, Russell King, Mark Brown, Linus Walleij,
Charles Keepax, Sam Protsenko, Arnd Bergmann, Bartosz Golaszewski
Cc: Krzysztof Kozlowski, linux-input, devicetree, linux-kernel,
linux-arm-kernel, linux-samsung-soc, patches
In-Reply-To: <20260708-samsung-kp-v2-0-3c6ed4c9b3b6@gmail.com>
The binding with a sub-node per each key is very verbose and is hard to
use with static device properties. Allow standard matrix keymap binding
in addition to the verbose one.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
.../bindings/input/samsung,s3c6410-keypad.yaml | 53 ++++++++++++++++++++--
1 file changed, 50 insertions(+), 3 deletions(-)
diff --git a/Documentation/devicetree/bindings/input/samsung,s3c6410-keypad.yaml b/Documentation/devicetree/bindings/input/samsung,s3c6410-keypad.yaml
index a53569aa0ee7..2498c62444a1 100644
--- a/Documentation/devicetree/bindings/input/samsung,s3c6410-keypad.yaml
+++ b/Documentation/devicetree/bindings/input/samsung,s3c6410-keypad.yaml
@@ -37,6 +37,10 @@ properties:
wakeup-source: true
+ keypad,num-columns: true
+ keypad,num-rows: true
+ linux,keymap: true
+
linux,input-no-autorepeat:
type: boolean
description:
@@ -81,12 +85,33 @@ patternProperties:
- keypad,row
- linux,code
+dependencies:
+ linux,keymap: [ "keypad,num-columns", "keypad,num-rows" ]
+
required:
- compatible
- reg
- interrupts
- - samsung,keypad-num-columns
- - samsung,keypad-num-rows
+
+allOf:
+ - $ref: input.yaml#
+ - $ref: matrix-keymap.yaml#
+ - if:
+ required:
+ - linux,keymap
+ then:
+ properties:
+ samsung,keypad-num-columns: false
+ samsung,keypad-num-rows: false
+ patternProperties:
+ '^key-[0-9a-z]+$': false
+ else:
+ properties:
+ keypad,num-columns: false
+ keypad,num-rows: false
+ required:
+ - samsung,keypad-num-columns
+ - samsung,keypad-num-rows
additionalProperties: false
@@ -94,8 +119,9 @@ examples:
- |
#include <dt-bindings/clock/exynos4.h>
#include <dt-bindings/interrupt-controller/arm-gic.h>
+ #include <dt-bindings/input/input.h>
- keypad@100a0000 {
+ keypad1@100a0000 {
compatible = "samsung,s5pv210-keypad";
reg = <0x100a0000 0x100>;
interrupts = <GIC_SPI 109 IRQ_TYPE_LEVEL_HIGH>;
@@ -119,3 +145,24 @@ examples:
linux,code = <3>;
};
};
+ - |
+ #include <dt-bindings/clock/exynos4.h>
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+ #include <dt-bindings/input/input.h>
+
+ keypad2@100a0000 {
+ compatible = "samsung,s5pv210-keypad";
+ reg = <0x100a0000 0x100>;
+ interrupts = <GIC_SPI 109 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&clock CLK_KEYIF>;
+ clock-names = "keypad";
+
+ keypad,num-rows = <2>;
+ keypad,num-columns = <8>;
+ linux,keymap = <
+ MATRIX_KEY(0, 3, 2)
+ MATRIX_KEY(0, 4, 3)
+ >;
+ linux,input-no-autorepeat;
+ wakeup-source;
+ };
--
2.55.0.795.g602f6c329a-goog
^ permalink raw reply related
* [PATCH v2 02/11] Input: samsung-keypad - handle compact binding
From: Dmitry Torokhov @ 2026-07-09 4:53 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Peter Griffin,
Alim Akhtar, Russell King, Mark Brown, Linus Walleij,
Charles Keepax, Sam Protsenko, Arnd Bergmann, Bartosz Golaszewski
Cc: Krzysztof Kozlowski, linux-input, devicetree, linux-kernel,
linux-arm-kernel, linux-samsung-soc, patches
In-Reply-To: <20260708-samsung-kp-v2-0-3c6ed4c9b3b6@gmail.com>
Add support for standard matrix keymap binding (in addition to the
existing verbose binding with a sub-node for each key). This will
allow easier conversions from platform data to device properties when
using static device properties.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/input/keyboard/samsung-keypad.c | 130 ++++++++++++++++----------------
1 file changed, 66 insertions(+), 64 deletions(-)
diff --git a/drivers/input/keyboard/samsung-keypad.c b/drivers/input/keyboard/samsung-keypad.c
index 17127269e3f0..b9d4ea5f202a 100644
--- a/drivers/input/keyboard/samsung-keypad.c
+++ b/drivers/input/keyboard/samsung-keypad.c
@@ -231,83 +231,83 @@ static void samsung_keypad_close(struct input_dev *input_dev)
samsung_keypad_stop(keypad);
}
-#ifdef CONFIG_OF
-static struct samsung_keypad_platdata *
-samsung_keypad_parse_dt(struct device *dev)
+static const struct matrix_keymap_data *
+samsung_parse_verbose_keymap(struct device *dev)
{
- struct samsung_keypad_platdata *pdata;
struct matrix_keymap_data *keymap_data;
- uint32_t *keymap, num_rows = 0, num_cols = 0;
- struct device_node *np = dev->of_node, *key_np;
+ struct fwnode_handle *child;
+ u32 *keymap;
unsigned int key_count;
- if (!np) {
- dev_err(dev, "missing device tree data\n");
- return ERR_PTR(-EINVAL);
+ keymap_data = devm_kzalloc(dev, sizeof(*keymap_data), GFP_KERNEL);
+ if (!keymap_data)
+ return ERR_PTR(-ENOMEM);
+
+ key_count = device_get_child_node_count(dev);
+ keymap = devm_kcalloc(dev, key_count, sizeof(*keymap), GFP_KERNEL);
+ if (!keymap)
+ return ERR_PTR(-ENOMEM);
+
+ keymap_data->keymap_size = key_count;
+ keymap_data->keymap = keymap;
+
+ device_for_each_child_node(dev, child) {
+ u32 row, col, key_code;
+
+ fwnode_property_read_u32(child, "keypad,row", &row);
+ fwnode_property_read_u32(child, "keypad,column", &col);
+ fwnode_property_read_u32(child, "linux,code", &key_code);
+
+ *keymap++ = KEY(row, col, key_code);
}
+ return keymap_data;
+}
+
+static const struct samsung_keypad_platdata *
+samsung_keypad_parse_properties(struct device *dev)
+{
+ const struct matrix_keymap_data *keymap_data;
+ struct samsung_keypad_platdata *pdata;
+ u32 num_rows = 0, num_cols = 0;
+ int error;
+
pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
- if (!pdata) {
- dev_err(dev, "could not allocate memory for platform data\n");
+ if (!pdata)
return ERR_PTR(-ENOMEM);
- }
- of_property_read_u32(np, "samsung,keypad-num-rows", &num_rows);
- of_property_read_u32(np, "samsung,keypad-num-columns", &num_cols);
- if (!num_rows || !num_cols) {
- dev_err(dev, "number of keypad rows/columns not specified\n");
- return ERR_PTR(-EINVAL);
- }
+ device_property_read_u32(dev, "samsung,keypad-num-rows", &num_rows);
+ device_property_read_u32(dev, "samsung,keypad-num-columns", &num_cols);
+
+ error = matrix_keypad_parse_properties(dev, &num_rows, &num_cols);
+ if (error)
+ return ERR_PTR(error);
+
pdata->rows = num_rows;
pdata->cols = num_cols;
- keymap_data = devm_kzalloc(dev, sizeof(*keymap_data), GFP_KERNEL);
- if (!keymap_data) {
- dev_err(dev, "could not allocate memory for keymap data\n");
- return ERR_PTR(-ENOMEM);
- }
- pdata->keymap_data = keymap_data;
+ if (!device_property_present(dev, "linux,keymap")) {
+ keymap_data = samsung_parse_verbose_keymap(dev);
+ if (IS_ERR(keymap_data))
+ return ERR_CAST(keymap_data);
- key_count = of_get_child_count(np);
- keymap_data->keymap_size = key_count;
- keymap = devm_kcalloc(dev, key_count, sizeof(uint32_t), GFP_KERNEL);
- if (!keymap) {
- dev_err(dev, "could not allocate memory for keymap\n");
- return ERR_PTR(-ENOMEM);
+ pdata->keymap_data = keymap_data;
}
- keymap_data->keymap = keymap;
- for_each_child_of_node(np, key_np) {
- u32 row, col, key_code;
- of_property_read_u32(key_np, "keypad,row", &row);
- of_property_read_u32(key_np, "keypad,column", &col);
- of_property_read_u32(key_np, "linux,code", &key_code);
- *keymap++ = KEY(row, col, key_code);
- }
- pdata->no_autorepeat = of_property_read_bool(np, "linux,input-no-autorepeat");
+ pdata->no_autorepeat =
+ device_property_read_bool(dev, "linux,input-no-autorepeat");
- pdata->wakeup = of_property_read_bool(np, "wakeup-source") ||
+ pdata->wakeup = device_property_read_bool(dev, "wakeup-source") ||
/* legacy name */
- of_property_read_bool(np, "linux,input-wakeup");
-
+ device_property_read_bool(dev, "linux,input-wakeup");
return pdata;
}
-#else
-static struct samsung_keypad_platdata *
-samsung_keypad_parse_dt(struct device *dev)
-{
- dev_err(dev, "no platform data defined\n");
-
- return ERR_PTR(-EINVAL);
-}
-#endif
static int samsung_keypad_probe(struct platform_device *pdev)
{
const struct samsung_keypad_platdata *pdata;
- const struct matrix_keymap_data *keymap_data;
const struct platform_device_id *id;
struct samsung_keypad *keypad;
struct resource *res;
@@ -316,18 +316,17 @@ static int samsung_keypad_probe(struct platform_device *pdev)
int error;
pdata = dev_get_platdata(&pdev->dev);
- if (!pdata) {
- pdata = samsung_keypad_parse_dt(&pdev->dev);
+ if (pdata) {
+ if (!pdata->keymap_data) {
+ dev_err(&pdev->dev, "no keymap data defined\n");
+ return -EINVAL;
+ }
+ } else {
+ pdata = samsung_keypad_parse_properties(&pdev->dev);
if (IS_ERR(pdata))
return PTR_ERR(pdata);
}
- keymap_data = pdata->keymap_data;
- if (!keymap_data) {
- dev_err(&pdev->dev, "no keymap data defined\n");
- return -EINVAL;
- }
-
if (!pdata->rows || pdata->rows > SAMSUNG_MAX_ROWS)
return -EINVAL;
@@ -391,7 +390,7 @@ static int samsung_keypad_probe(struct platform_device *pdev)
input_dev->open = samsung_keypad_open;
input_dev->close = samsung_keypad_close;
- error = matrix_keypad_build_keymap(keymap_data, NULL,
+ error = matrix_keypad_build_keymap(pdata->keymap_data, NULL,
pdata->rows, pdata->cols,
keypad->keycodes, input_dev);
if (error) {
@@ -430,11 +429,14 @@ static int samsung_keypad_probe(struct platform_device *pdev)
if (error)
return error;
- if (pdev->dev.of_node) {
- devm_kfree(&pdev->dev, (void *)pdata->keymap_data->keymap);
- devm_kfree(&pdev->dev, (void *)pdata->keymap_data);
+ if (!dev_get_platdata(&pdev->dev)) {
+ if (pdata->keymap_data) {
+ devm_kfree(&pdev->dev, (void *)pdata->keymap_data->keymap);
+ devm_kfree(&pdev->dev, (void *)pdata->keymap_data);
+ }
devm_kfree(&pdev->dev, (void *)pdata);
}
+
return 0;
}
--
2.55.0.795.g602f6c329a-goog
^ 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