* [PATCH v5] HID: steelseries: Add MSI Raider A18 HX A9WJG RGB support
From: David Glushkov @ 2026-05-31 23:39 UTC (permalink / raw)
To: Jiri Kosina; +Cc: Benjamin Tissoires, linux-input, linux-kernel
In-Reply-To: <20260531215204.172030-1-david.glushkov@sntiq.com>
The MSI Raider A18 HX A9WJG exposes two internal SteelSeries USB HID
devices for RGB lighting: KLC (1038:1122) for the keyboard and ALC
(1038:1161) for the lightbar/logo zones.
Add DMI-gated support for these devices and expose them as multicolor
LED class devices. The driver sends the same HID class SET_REPORT
control transfer as the tested userspace implementation for this
machine and writes a uniform RGB value to all known keyboard keys or
ALC zones.
Limit RGB support to USB interface 0 and the tested DMI system because
the KLC product ID is shared across MSI laptop designs and the key
layout mapping is model-specific. If the DMI or interface check does
not match, keep the device bound as a regular HID device instead of
failing probe.
Tested on MSI Raider A18 HX A9WJG. Both internal SteelSeries ALC
(1038:1161) and KLC (1038:1122) HID devices bind on interface 0 and
create steelseries::lightbar and steelseries::kbd_backlight. Setting
multi_intensity and brightness changes the keyboard and lightbar
colors.
Signed-off-by: David Glushkov <david.glushkov@sntiq.com>
---
v5:
- Drop pm_ret handling and ignore PM_HINT_NORMAL cleanup errors.
- Fix LED registration error handling to clean up rgb_buf on failure.
- Fix trailing whitespaces and formatting style nits.
- Update commit message to accurately reflect the DMI fallback behavior.
v4:
- Fix literal \n typo in C code.
- Remove unused steelseries_msi_rgb_free_buf from #else block.
- Do not fail probe on unsupported DMI/interface; clear MSI RGB quirk and continue normal HID initialization.
- Do not fail probe when RGB LED registration fails; keep the HID device usable without RGB LED support.
- Add explicit linux/slab.h include for kzalloc/kfree.
v3:
- Fix build failure (added missing err_close label to steelseries_probe).
- Fix DMA API violation (use kzalloc instead of devm_kzalloc for usb transfer buffer).
- Fix C syntax declaration-after-statement warning.
- Fix type confusion for SRWS1 (add early check in raw_event before hid_get_drvdata).
- Fix delayed_work crash (add battery_registered flag).
v2:
- Fixed unsafe to_usb_interface cast by checking hid_is_usb() first.
- Fixed uninitialized delayed_work warning by restricting cancel_delayed_work_sync to headset devices.
- Fixed error path leaks in probe (hid_hw_stop / hid_hw_close).
- Added hid_hw_power PM wrappers around direct usb_control_msg transfers.
drivers/hid/hid-ids.h | 2 +
drivers/hid/hid-steelseries.c | 277 ++++++++++++++++++++++++++++++++--
2 files changed, 270 insertions(+), 9 deletions(-)
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 4657d96fb..4af4397b8 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -1367,6 +1367,8 @@
#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_MSI_KLC 0x1122
+#define USB_DEVICE_ID_STEELSERIES_MSI_ALC 0x1161
#define USB_VENDOR_ID_SUN 0x0430
#define USB_DEVICE_ID_RARITAN_KVM_DONGLE 0xcdab
diff --git a/drivers/hid/hid-steelseries.c b/drivers/hid/hid-steelseries.c
index f98435631..c269db06d 100644
--- a/drivers/hid/hid-steelseries.c
+++ b/drivers/hid/hid-steelseries.c
@@ -10,16 +10,26 @@
*/
#include <linux/device.h>
+#include <linux/dmi.h>
#include <linux/hid.h>
#include <linux/module.h>
#include <linux/usb.h>
#include <linux/leds.h>
+#include <linux/led-class-multicolor.h>
+#include <linux/slab.h>
#include "hid-ids.h"
#define STEELSERIES_SRWS1 BIT(0)
#define STEELSERIES_ARCTIS_1 BIT(1)
#define STEELSERIES_ARCTIS_9 BIT(2)
+#define STEELSERIES_MSI_RGB BIT(3)
+
+#define STEELSERIES_MSI_RGB_REPORT_LEN 524
+
+#define STEELSERIES_HAS_LEDS_MULTICOLOR \
+ (IS_BUILTIN(CONFIG_LEDS_CLASS_MULTICOLOR) || \
+ (IS_MODULE(CONFIG_LEDS_CLASS_MULTICOLOR) && IS_MODULE(CONFIG_HID_STEELSERIES)))
struct steelseries_device {
struct hid_device *hdev;
@@ -34,6 +44,14 @@ struct steelseries_device {
uint8_t battery_capacity;
bool headset_connected;
bool battery_charging;
+ bool battery_registered;
+
+#if STEELSERIES_HAS_LEDS_MULTICOLOR
+ struct led_classdev_mc mc_cdev;
+ struct mc_subled subled_info[3];
+ struct mutex rgb_lock; /* protects rgb_buf */
+ u8 *rgb_buf;
+#endif
};
#if IS_BUILTIN(CONFIG_LEDS_CLASS) || \
@@ -510,6 +528,7 @@ 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);
+ sd->battery_registered = true;
steelseries_headset_fetch_battery(sd->hdev);
if (sd->quirks & STEELSERIES_ARCTIS_9) {
@@ -528,6 +547,196 @@ static bool steelseries_is_vendor_usage_page(struct hid_device *hdev, uint8_t us
hdev->rdesc[2] == 0xff;
}
+static const struct dmi_system_id steelseries_msi_rgb_dmi_table[] = {
+ {
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "Micro-Star International Co., Ltd."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "Raider A18 HX A9WJG"),
+ DMI_MATCH(DMI_BOARD_NAME, "MS-182L"),
+ },
+ },
+ { }
+};
+
+static struct usb_interface *steelseries_hid_to_usb_intf(struct hid_device *hdev)
+{
+ if (!hid_is_usb(hdev))
+ return NULL;
+
+ return to_usb_interface(hdev->dev.parent);
+}
+
+static struct usb_device *steelseries_hid_to_usb_dev(struct hid_device *hdev)
+{
+ struct usb_interface *intf = steelseries_hid_to_usb_intf(hdev);
+
+ if (!intf)
+ return NULL;
+
+ return interface_to_usbdev(intf);
+}
+
+static bool steelseries_msi_rgb_is_interface0(struct hid_device *hdev)
+{
+ struct usb_interface *intf = steelseries_hid_to_usb_intf(hdev);
+ struct usb_device *udev;
+
+ if (!intf)
+ return false;
+
+ udev = interface_to_usbdev(intf);
+
+ return intf == usb_ifnum_to_if(udev, 0);
+}
+
+#if STEELSERIES_HAS_LEDS_MULTICOLOR
+
+static int steelseries_msi_rgb_set_blocking(struct led_classdev *led_cdev,
+ enum led_brightness brightness)
+{
+ struct led_classdev_mc *mc_cdev = lcdev_to_mccdev(led_cdev);
+ struct steelseries_device *sd = container_of(mc_cdev,
+ struct steelseries_device,
+ mc_cdev);
+ struct hid_device *hdev = sd->hdev;
+ struct usb_device *udev = steelseries_hid_to_usb_dev(hdev);
+ int i, ret;
+ u8 r, g, b;
+
+ static const u8 keys[] = {
+ 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, 0x0a, 0x0b,
+ 0x0c, 0x0d, 0x0e, 0x0f, 0x10, 0x11, 0x12, 0x13,
+ 0x14, 0x15, 0x16, 0x17, 0x18, 0x19, 0x1a, 0x1b,
+ 0x1c, 0x1d, 0x1e, 0x1f, 0x20, 0x21, 0x22, 0x23,
+ 0x24, 0x25, 0x26, 0x27, 0x28, 0x29, 0x2a, 0x2b,
+ 0x2c, 0x2d, 0x2e, 0x2f, 0x30, 0x31, 0x33, 0x34,
+ 0x35, 0x36, 0x37, 0x38, 0x39, 0x3a, 0x3b, 0x3c,
+ 0x3d, 0x3e, 0x3f, 0x40, 0x41, 0x42, 0x43, 0x44,
+ 0x45, 0x46, 0x47, 0x49, 0x4b, 0x4c, 0x4e, 0x4f,
+ 0x50, 0x51, 0x52, 0x53, 0x54, 0x55, 0x56, 0x57,
+ 0x58, 0x59, 0x5a, 0x5b, 0x5c, 0x5d, 0x5e, 0x5f,
+ 0x60, 0x61, 0x62, 0x63, 0x64, 0x66, 0xe0, 0xe1,
+ 0xe2, 0xe3, 0xe4, 0xe5, 0xe6, 0xf0
+ };
+ static const u8 alc_zones[] = { 0x00, 0x01, 0x02, 0x03, 0x04, 0x05 };
+
+ if (!udev)
+ return -ENODEV;
+
+ mutex_lock(&sd->rgb_lock);
+
+ led_mc_calc_color_components(mc_cdev, brightness);
+
+ r = mc_cdev->subled_info[0].brightness;
+ g = mc_cdev->subled_info[1].brightness;
+ b = mc_cdev->subled_info[2].brightness;
+
+ memset(sd->rgb_buf, 0, STEELSERIES_MSI_RGB_REPORT_LEN);
+ sd->rgb_buf[0] = 0x0c;
+ sd->rgb_buf[1] = 0x00;
+ sd->rgb_buf[3] = 0x00;
+
+ if (hdev->product == USB_DEVICE_ID_STEELSERIES_MSI_KLC) {
+ sd->rgb_buf[2] = 0x66;
+ for (i = 0; i < ARRAY_SIZE(keys); i++) {
+ sd->rgb_buf[4 + i * 4] = keys[i];
+ sd->rgb_buf[5 + i * 4] = r;
+ sd->rgb_buf[6 + i * 4] = g;
+ sd->rgb_buf[7 + i * 4] = b;
+ }
+ } else {
+ sd->rgb_buf[2] = 0x06;
+ for (i = 0; i < ARRAY_SIZE(alc_zones); i++) {
+ sd->rgb_buf[4 + i * 4] = alc_zones[i];
+ sd->rgb_buf[5 + i * 4] = r;
+ sd->rgb_buf[6 + i * 4] = g;
+ sd->rgb_buf[7 + i * 4] = b;
+ }
+ }
+
+ ret = hid_hw_power(hdev, PM_HINT_FULLON);
+ if (ret < 0)
+ goto out_unlock;
+
+ ret = usb_control_msg(udev, usb_sndctrlpipe(udev, 0),
+ HID_REQ_SET_REPORT,
+ USB_DIR_OUT | USB_TYPE_CLASS | USB_RECIP_INTERFACE,
+ 0x0300, 0,
+ sd->rgb_buf, STEELSERIES_MSI_RGB_REPORT_LEN,
+ USB_CTRL_SET_TIMEOUT);
+
+ hid_hw_power(hdev, PM_HINT_NORMAL);
+
+out_unlock:
+ mutex_unlock(&sd->rgb_lock);
+ return ret < 0 ? ret : 0;
+}
+
+static void steelseries_msi_rgb_free_buf(void *data)
+{
+ kfree(data);
+}
+
+static int steelseries_msi_rgb_register(struct steelseries_device *sd)
+{
+ struct hid_device *hdev = sd->hdev;
+ struct led_classdev *led_cdev;
+ int ret;
+
+ sd->rgb_buf = kzalloc(STEELSERIES_MSI_RGB_REPORT_LEN, GFP_KERNEL);
+ if (!sd->rgb_buf)
+ return -ENOMEM;
+
+ ret = devm_add_action_or_reset(&hdev->dev,
+ steelseries_msi_rgb_free_buf,
+ sd->rgb_buf);
+ if (ret) {
+ sd->rgb_buf = NULL;
+ return ret;
+ }
+
+ mutex_init(&sd->rgb_lock);
+
+ sd->subled_info[0].color_index = LED_COLOR_ID_RED;
+ sd->subled_info[1].color_index = LED_COLOR_ID_GREEN;
+ sd->subled_info[2].color_index = LED_COLOR_ID_BLUE;
+ sd->subled_info[0].intensity = 255;
+ sd->subled_info[1].intensity = 255;
+ sd->subled_info[2].intensity = 255;
+ sd->subled_info[0].channel = 0;
+ sd->subled_info[1].channel = 1;
+ sd->subled_info[2].channel = 2;
+
+ sd->mc_cdev.subled_info = sd->subled_info;
+ sd->mc_cdev.num_colors = 3;
+
+ led_cdev = &sd->mc_cdev.led_cdev;
+ if (hdev->product == USB_DEVICE_ID_STEELSERIES_MSI_KLC)
+ led_cdev->name = "steelseries::kbd_backlight";
+ else
+ led_cdev->name = "steelseries::lightbar";
+
+ led_cdev->max_brightness = 255;
+ led_cdev->brightness_set_blocking = steelseries_msi_rgb_set_blocking;
+
+ ret = devm_led_classdev_multicolor_register(&hdev->dev, &sd->mc_cdev);
+ if (ret) {
+ devm_remove_action(&hdev->dev, steelseries_msi_rgb_free_buf,
+ sd->rgb_buf);
+ kfree(sd->rgb_buf);
+ sd->rgb_buf = NULL;
+ return ret;
+ }
+
+ return 0;
+}
+#else
+static int steelseries_msi_rgb_register(struct steelseries_device *sd)
+{
+ return -ENODEV;
+}
+#endif
+
static int steelseries_probe(struct hid_device *hdev, const struct hid_device_id *id)
{
struct steelseries_device *sd;
@@ -549,6 +758,14 @@ static int steelseries_probe(struct hid_device *hdev, const struct hid_device_id
sd->hdev = hdev;
sd->quirks = id->driver_data;
+ if (sd->quirks & STEELSERIES_MSI_RGB) {
+ if (!dmi_check_system(steelseries_msi_rgb_dmi_table) ||
+ !steelseries_msi_rgb_is_interface0(hdev)) {
+ hid_dbg(hdev, "MSI RGB quirk not applicable, using generic HID path\n");
+ sd->quirks &= ~STEELSERIES_MSI_RGB;
+ }
+ }
+
ret = hid_parse(hdev);
if (ret)
return ret;
@@ -565,12 +782,28 @@ static int steelseries_probe(struct hid_device *hdev, const struct hid_device_id
ret = hid_hw_open(hdev);
if (ret)
- return ret;
+ goto err_stop;
+
+ if (sd->quirks & STEELSERIES_MSI_RGB) {
+ ret = steelseries_msi_rgb_register(sd);
+ if (ret) {
+ hid_warn(hdev,
+ "Failed to register MSI RGB LEDs: %d, continuing without RGB support\n",
+ ret);
+ sd->quirks &= ~STEELSERIES_MSI_RGB;
+ }
+ return 0;
+ }
- if (steelseries_headset_battery_register(sd) < 0)
+ if (sd->quirks & (STEELSERIES_ARCTIS_1 | STEELSERIES_ARCTIS_9) &&
+ steelseries_headset_battery_register(sd) < 0)
hid_err(sd->hdev,
"Failed to register battery for headset\n");
+ return 0;
+
+err_stop:
+ hid_hw_stop(hdev);
return ret;
}
@@ -588,12 +821,15 @@ static void steelseries_remove(struct hid_device *hdev)
}
sd = hid_get_drvdata(hdev);
+ if (!sd)
+ return;
spin_lock_irqsave(&sd->lock, flags);
sd->removed = true;
spin_unlock_irqrestore(&sd->lock, flags);
- cancel_delayed_work_sync(&sd->battery_work);
+ if (sd->battery_registered)
+ cancel_delayed_work_sync(&sd->battery_work);
hid_hw_close(hdev);
hid_hw_stop(hdev);
@@ -624,20 +860,33 @@ static uint8_t steelseries_headset_map_capacity(uint8_t capacity, uint8_t min_in
return (capacity - min_in) * 100 / (max_in - min_in);
}
+static bool steelseries_is_headset(struct hid_device *hdev)
+{
+ return hdev->product == USB_DEVICE_ID_STEELSERIES_ARCTIS_1 ||
+ hdev->product == USB_DEVICE_ID_STEELSERIES_ARCTIS_9;
+}
+
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;
+ struct steelseries_device *sd;
+ int capacity;
+ bool connected;
+ bool charging;
unsigned long flags;
- /* Not a headset */
- if (hdev->product == USB_DEVICE_ID_STEELSERIES_SRWS1)
+ if (!steelseries_is_headset(hdev))
+ return 0;
+
+ sd = hid_get_drvdata(hdev);
+ if (!sd || !sd->battery_registered)
return 0;
+ capacity = sd->battery_capacity;
+ connected = sd->headset_connected;
+ charging = sd->battery_charging;
+
if (hdev->product == USB_DEVICE_ID_STEELSERIES_ARCTIS_1) {
hid_dbg(sd->hdev,
"Parsing raw event for Arctis 1 headset (%*ph)\n", size, read_buf);
@@ -732,6 +981,16 @@ static const struct hid_device_id steelseries_devices[] = {
HID_USB_DEVICE(USB_VENDOR_ID_STEELSERIES, USB_DEVICE_ID_STEELSERIES_ARCTIS_9),
.driver_data = STEELSERIES_ARCTIS_9 },
+#if STEELSERIES_HAS_LEDS_MULTICOLOR
+ { /* MSI Raider A18 KLC */
+ HID_USB_DEVICE(USB_VENDOR_ID_STEELSERIES, USB_DEVICE_ID_STEELSERIES_MSI_KLC),
+ .driver_data = STEELSERIES_MSI_RGB },
+
+ { /* MSI Raider A18 ALC */
+ HID_USB_DEVICE(USB_VENDOR_ID_STEELSERIES, USB_DEVICE_ID_STEELSERIES_MSI_ALC),
+ .driver_data = STEELSERIES_MSI_RGB },
+#endif
+
{ }
};
MODULE_DEVICE_TABLE(hid, steelseries_devices);
--
2.54.0
^ permalink raw reply related
* RE: [PATCH] HID: intel-thc-hid: intel-quickspi: reset touch IC on system resume
From: Xu, Even @ 2026-06-01 3:24 UTC (permalink / raw)
To: d3z-the-dev, Sun, Xinpeng, Jiri Kosina, Benjamin Tissoires
Cc: linux-input@vger.kernel.org, linux-kernel@vger.kernel.org,
Abhishek Tamboli, Sakari Ailus
In-Reply-To: <20260529222210.1532876-1-d3z.the.dev@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 4832 bytes --]
Hi,
Sorry, I cannot get your exact name from this patch email.
From your issue description, I suppose Surface Pro 10 uses "Suspend-to-RAM" instead of "Standby" or "Suspend-to-Idle" for suspend/resume.
As this documents mentioned, driver needs to take care the suspend type then handle resume flow carefully.
Actually, I had a patch for this "Suspend-to-RAM" support several months ago, please find attached file for the patch. This patch is still under our internal final validation.
It will be very appreciated if you can help test attached patch on your Surface device.
If it works, then everything is fine, I will submit this patch with your name after our final validation.
If it doesn't work for your case, then we need to future debug on your surface device to refine your patch to let it more standard.
Thanks!
Best Regards,
Even Xu
> -----Original Message-----
> From: d3z-the-dev <d3z.the.dev@gmail.com>
> Sent: Saturday, May 30, 2026 6:22 AM
> To: Xu, Even <even.xu@intel.com>; Sun, Xinpeng <xinpeng.sun@intel.com>; Jiri
> Kosina <jikos@kernel.org>; Benjamin Tissoires <bentiss@kernel.org>
> Cc: linux-input@vger.kernel.org; linux-kernel@vger.kernel.org; Abhishek Tamboli
> <abhishektamboli9@gmail.com>; Sakari Ailus <sakari.ailus@linux.intel.com>;
> d3z-the-dev <d3z.the.dev@gmail.com>
> Subject: [PATCH] HID: intel-thc-hid: intel-quickspi: reset touch IC on system
> resume
>
> On the Surface Pro 10 (Meteor Lake) the touchscreen stops working after a
> suspend/resume cycle and only recovers after a reboot. The driver logs
> "GET_DEVICE_INFO: recv failed: -11" on resume.
>
> The touch IC loses power during system suspend (s2idle) on this platform, the
> same way it does across hibernation. quickspi_resume() only restores the THC
> port, interrupts and DMA and sends a HIDSPI_ON command, assuming the touch
> IC kept its power and state. When it has actually lost power the HIDSPI_ON
> command is never acknowledged and the descriptor read fails, leaving the
> touchscreen dead until the module is reloaded.
>
> quickspi_restore() already handles this for hibernation by running
> reset_tic() and reconfiguring the THC SPI/LTR settings. Make
> quickspi_resume() do the same: quiesce interrupts, re-select the THC port,
> reconfigure the SPI input/output addresses and read/write parameters, run
> reset_tic() to re-enumerate the device and restore the LTR configuration.
>
> Tested on a Surface Pro 10 across multiple s2idle suspend/resume cycles.
>
> Link: https://github.com/linux-surface/linux-surface/issues/1799
>
> Signed-off-by: d3z-the-dev <d3z.the.dev@gmail.com>
> ---
> .../intel-quickspi/pci-quickspi.c | 38 +++++++++++++++++--
> 1 file changed, 34 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c
> b/drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c
> index f669235f1883..d59278a3e5c1 100644
> --- a/drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c
> +++ b/drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c
> @@ -780,24 +780,54 @@ static int quickspi_resume(struct device *device)
> if (!qsdev)
> return -ENODEV;
>
> + ret = thc_interrupt_quiesce(qsdev->thc_hw, true);
> + if (ret)
> + return ret;
> +
> ret = thc_port_select(qsdev->thc_hw, THC_PORT_TYPE_SPI);
> if (ret)
> return ret;
>
> + thc_spi_input_output_address_config(qsdev->thc_hw,
> + qsdev->input_report_hdr_addr,
> + qsdev->input_report_bdy_addr,
> + qsdev->output_report_addr);
> +
> + ret = thc_spi_read_config(qsdev->thc_hw, qsdev->spi_freq_val,
> + qsdev->spi_read_io_mode,
> + qsdev->spi_read_opcode,
> + qsdev->spi_packet_size);
> + if (ret)
> + return ret;
> +
> + ret = thc_spi_write_config(qsdev->thc_hw, qsdev->spi_freq_val,
> + qsdev->spi_write_io_mode,
> + qsdev->spi_write_opcode,
> + qsdev->spi_packet_size,
> + qsdev->performance_limit);
> + if (ret)
> + return ret;
> +
> thc_interrupt_config(qsdev->thc_hw);
>
> thc_interrupt_enable(qsdev->thc_hw, true);
>
> - ret = thc_dma_configure(qsdev->thc_hw);
> + /* The TIC may lose power across system suspend, reset it to recover */
> + ret = reset_tic(qsdev);
> if (ret)
> return ret;
>
> - ret = thc_interrupt_quiesce(qsdev->thc_hw, false);
> + ret = thc_dma_configure(qsdev->thc_hw);
> if (ret)
> return ret;
>
> - if (!device_may_wakeup(qsdev->dev))
> - return quickspi_set_power(qsdev, HIDSPI_ON);
> + thc_ltr_config(qsdev->thc_hw,
> + qsdev->active_ltr_val,
> + qsdev->low_power_ltr_val);
> +
> + thc_change_ltr_mode(qsdev->thc_hw, THC_LTR_MODE_ACTIVE);
> +
> + qsdev->state = QUICKSPI_ENABLED;
>
> return 0;
> }
> --
> 2.54.0
[-- Attachment #2: 0001-Hid-Intel-thc-hid-Intel-quickspi-Fix-non-functional-.patch --]
[-- Type: application/octet-stream, Size: 3337 bytes --]
From 939ec2c491f789b5b740a58936ff0e04cb71d2a8 Mon Sep 17 00:00:00 2001
From: Even Xu <even.xu@intel.com>
Date: Mon, 19 Jan 2026 16:16:12 +0800
Subject: [PATCH] Hid: Intel-thc-hid: Intel-quickspi: Fix non-functional touch
after S3 resume
THC hardware registers lose their content during S3 suspend. Add hardware
reconfiguration on resume from S3 to restore touch functionality.
Signed-off-by: Even Xu <even.xu@intel.com>
---
.../intel-quickspi/pci-quickspi.c | 32 +++++++++++++++++++
.../intel-quickspi/quickspi-dev.h | 4 +++
2 files changed, 36 insertions(+)
diff --git a/drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c b/drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c
index 20e8dbf1d313..0d67a0bb2ee7 100644
--- a/drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c
+++ b/drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c
@@ -10,6 +10,7 @@
#include <linux/irqreturn.h>
#include <linux/pci.h>
#include <linux/pm_runtime.h>
+#include <linux/suspend.h>
#include <linux/gpio/consumer.h>
@@ -765,6 +766,8 @@ static int quickspi_suspend(struct device *device)
thc_dma_unconfigure(qsdev->thc_hw);
+ qsdev->last_suspend_state = pm_suspend_target_state;
+
return 0;
}
@@ -782,6 +785,35 @@ static int quickspi_resume(struct device *device)
if (ret)
return ret;
+ /* If it's resume from S3, need to re-configure SPI port */
+ if (qsdev->last_suspend_state == PM_SUSPEND_MEM) {
+ thc_spi_input_output_address_config(qsdev->thc_hw,
+ qsdev->input_report_hdr_addr,
+ qsdev->input_report_bdy_addr,
+ qsdev->output_report_addr);
+
+ ret = thc_spi_read_config(qsdev->thc_hw, qsdev->spi_freq_val,
+ qsdev->spi_read_io_mode,
+ qsdev->spi_read_opcode,
+ qsdev->spi_packet_size);
+ if (ret)
+ return ret;
+
+ ret = thc_spi_write_config(qsdev->thc_hw, qsdev->spi_freq_val,
+ qsdev->spi_write_io_mode,
+ qsdev->spi_write_opcode,
+ qsdev->spi_packet_size,
+ qsdev->performance_limit);
+ if (ret)
+ return ret;
+
+ thc_ltr_config(qsdev->thc_hw,
+ qsdev->active_ltr_val,
+ qsdev->low_power_ltr_val);
+
+ thc_change_ltr_mode(qsdev->thc_hw, THC_LTR_MODE_ACTIVE);
+ }
+
thc_interrupt_config(qsdev->thc_hw);
thc_interrupt_enable(qsdev->thc_hw, true);
diff --git a/drivers/hid/intel-thc-hid/intel-quickspi/quickspi-dev.h b/drivers/hid/intel-thc-hid/intel-quickspi/quickspi-dev.h
index ef433fe1b45d..3beeec62dd09 100644
--- a/drivers/hid/intel-thc-hid/intel-quickspi/quickspi-dev.h
+++ b/drivers/hid/intel-thc-hid/intel-quickspi/quickspi-dev.h
@@ -7,6 +7,7 @@
#include <linux/bits.h>
#include <linux/hid-over-spi.h>
#include <linux/sizes.h>
+#include <linux/suspend.h>
#include <linux/wait.h>
#include "quickspi-protocol.h"
@@ -124,6 +125,7 @@ struct acpi_device;
* @get_feature_cmpl: indicate get feature received or not
* @set_feature_cmpl_wq: workqueue for waiting set feature to device
* @set_feature_cmpl: indicate set feature send complete or not
+ * @last_suspend_state: save last system suspend state
*/
struct quickspi_device {
struct device *dev;
@@ -171,6 +173,8 @@ struct quickspi_device {
wait_queue_head_t set_report_cmpl_wq;
bool set_report_cmpl;
+
+ suspend_state_t last_suspend_state;
};
#endif /* _QUICKSPI_DEV_H_ */
--
2.43.0
^ permalink raw reply related
* RE: [PATCH] HID: intel-thc-hid: intel-quickspi: reset touch IC on system resume
From: Xu, Even @ 2026-06-01 3:26 UTC (permalink / raw)
To: d3z-the-dev, Sun, Xinpeng, Jiri Kosina, Benjamin Tissoires
Cc: linux-input@vger.kernel.org, linux-kernel@vger.kernel.org,
Abhishek Tamboli, Sakari Ailus
In-Reply-To: <IA1PR11MB60987CB2D72FD1701951DFA4F4152@IA1PR11MB6098.namprd11.prod.outlook.com>
Sorry, missed the doc link: https://docs.kernel.org/admin-guide/pm/sleep-states.html
> -----Original Message-----
> From: Xu, Even
> Sent: Monday, June 1, 2026 11:25 AM
> To: 'd3z-the-dev' <d3z.the.dev@gmail.com>; Sun, Xinpeng
> <Xinpeng.Sun@intel.com>; Jiri Kosina <jikos@kernel.org>; Benjamin Tissoires
> <bentiss@kernel.org>
> Cc: linux-input@vger.kernel.org; linux-kernel@vger.kernel.org; Abhishek Tamboli
> <abhishektamboli9@gmail.com>; Sakari Ailus <sakari.ailus@linux.intel.com>
> Subject: RE: [PATCH] HID: intel-thc-hid: intel-quickspi: reset touch IC on system
> resume
>
> Hi,
>
> Sorry, I cannot get your exact name from this patch email.
>
> From your issue description, I suppose Surface Pro 10 uses "Suspend-to-RAM"
> instead of "Standby" or "Suspend-to-Idle" for suspend/resume.
> As this documents mentioned, driver needs to take care the suspend type then
> handle resume flow carefully.
>
> Actually, I had a patch for this "Suspend-to-RAM" support several months ago,
> please find attached file for the patch. This patch is still under our internal final
> validation.
>
> It will be very appreciated if you can help test attached patch on your Surface
> device.
> If it works, then everything is fine, I will submit this patch with your name after
> our final validation.
> If it doesn't work for your case, then we need to future debug on your surface
> device to refine your patch to let it more standard.
>
> Thanks!
>
> Best Regards,
> Even Xu
>
> > -----Original Message-----
> > From: d3z-the-dev <d3z.the.dev@gmail.com>
> > Sent: Saturday, May 30, 2026 6:22 AM
> > To: Xu, Even <even.xu@intel.com>; Sun, Xinpeng
> > <xinpeng.sun@intel.com>; Jiri Kosina <jikos@kernel.org>; Benjamin
> > Tissoires <bentiss@kernel.org>
> > Cc: linux-input@vger.kernel.org; linux-kernel@vger.kernel.org;
> > Abhishek Tamboli <abhishektamboli9@gmail.com>; Sakari Ailus
> > <sakari.ailus@linux.intel.com>; d3z-the-dev <d3z.the.dev@gmail.com>
> > Subject: [PATCH] HID: intel-thc-hid: intel-quickspi: reset touch IC on
> > system resume
> >
> > On the Surface Pro 10 (Meteor Lake) the touchscreen stops working
> > after a suspend/resume cycle and only recovers after a reboot. The
> > driver logs
> > "GET_DEVICE_INFO: recv failed: -11" on resume.
> >
> > The touch IC loses power during system suspend (s2idle) on this
> > platform, the same way it does across hibernation. quickspi_resume()
> > only restores the THC port, interrupts and DMA and sends a HIDSPI_ON
> > command, assuming the touch IC kept its power and state. When it has
> > actually lost power the HIDSPI_ON command is never acknowledged and
> > the descriptor read fails, leaving the touchscreen dead until the module is
> reloaded.
> >
> > quickspi_restore() already handles this for hibernation by running
> > reset_tic() and reconfiguring the THC SPI/LTR settings. Make
> > quickspi_resume() do the same: quiesce interrupts, re-select the THC
> > port, reconfigure the SPI input/output addresses and read/write
> > parameters, run
> > reset_tic() to re-enumerate the device and restore the LTR configuration.
> >
> > Tested on a Surface Pro 10 across multiple s2idle suspend/resume cycles.
> >
> > Link: https://github.com/linux-surface/linux-surface/issues/1799
> >
> > Signed-off-by: d3z-the-dev <d3z.the.dev@gmail.com>
> > ---
> > .../intel-quickspi/pci-quickspi.c | 38 +++++++++++++++++--
> > 1 file changed, 34 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c
> > b/drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c
> > index f669235f1883..d59278a3e5c1 100644
> > --- a/drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c
> > +++ b/drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c
> > @@ -780,24 +780,54 @@ static int quickspi_resume(struct device *device)
> > if (!qsdev)
> > return -ENODEV;
> >
> > + ret = thc_interrupt_quiesce(qsdev->thc_hw, true);
> > + if (ret)
> > + return ret;
> > +
> > ret = thc_port_select(qsdev->thc_hw, THC_PORT_TYPE_SPI);
> > if (ret)
> > return ret;
> >
> > + thc_spi_input_output_address_config(qsdev->thc_hw,
> > + qsdev->input_report_hdr_addr,
> > + qsdev->input_report_bdy_addr,
> > + qsdev->output_report_addr);
> > +
> > + ret = thc_spi_read_config(qsdev->thc_hw, qsdev->spi_freq_val,
> > + qsdev->spi_read_io_mode,
> > + qsdev->spi_read_opcode,
> > + qsdev->spi_packet_size);
> > + if (ret)
> > + return ret;
> > +
> > + ret = thc_spi_write_config(qsdev->thc_hw, qsdev->spi_freq_val,
> > + qsdev->spi_write_io_mode,
> > + qsdev->spi_write_opcode,
> > + qsdev->spi_packet_size,
> > + qsdev->performance_limit);
> > + if (ret)
> > + return ret;
> > +
> > thc_interrupt_config(qsdev->thc_hw);
> >
> > thc_interrupt_enable(qsdev->thc_hw, true);
> >
> > - ret = thc_dma_configure(qsdev->thc_hw);
> > + /* The TIC may lose power across system suspend, reset it to recover */
> > + ret = reset_tic(qsdev);
> > if (ret)
> > return ret;
> >
> > - ret = thc_interrupt_quiesce(qsdev->thc_hw, false);
> > + ret = thc_dma_configure(qsdev->thc_hw);
> > if (ret)
> > return ret;
> >
> > - if (!device_may_wakeup(qsdev->dev))
> > - return quickspi_set_power(qsdev, HIDSPI_ON);
> > + thc_ltr_config(qsdev->thc_hw,
> > + qsdev->active_ltr_val,
> > + qsdev->low_power_ltr_val);
> > +
> > + thc_change_ltr_mode(qsdev->thc_hw, THC_LTR_MODE_ACTIVE);
> > +
> > + qsdev->state = QUICKSPI_ENABLED;
> >
> > return 0;
> > }
> > --
> > 2.54.0
^ permalink raw reply
* [bug report] Potential atomicity bug in drivers/input/joydev.c, between joydev_0x_read() and joydev_ioctl_common()
From: Ginger @ 2026-06-01 7:07 UTC (permalink / raw)
To: dmitry.torokhov; +Cc: linux-input
Dear Linux kernel maintainers,
My research-based static analyzer found a potential atomicity bug
within the 'drivers/input' subsystem, more specifically, in
'drivers/input/joydev.c'.
This potential issue is present as of git commit
eb3f4b7426cfd2b79d65b7d37155480b32259a11 of the mainline kernel.
Potential concurrent triggering executions:
T0:
joydev_0x_read
--> spin_lock_irq(&input->event_lock);
--> read from joydev->abs
--> spin_unlock_irq(&input->event_lock);
T1:
joydev_ioctl_common
--> case JSIOCSCORR:
--> write to joydev->abs[i] (no unlocked)
The above trace is meant to demonstrate an illustrative example of the issue:
IMHO, in 'joydev_0x_read', the 'input->event_lock' is adopted to
serialize the read
accesses to joydev's fields like 'abs' and 'keypam' or input's fields
like 'input->key.
However, in either case, the write-side accesses to these fields are
not similarly
serialized.
Please kindly check at your convenience. Thank you for your time and
consideration.
Best regards,
Ginger
^ permalink raw reply
* Re: [PATCH 2/4] HID: core: introduce hid_safe_input_report()
From: Benjamin Tissoires @ 2026-06-01 7:30 UTC (permalink / raw)
To: Carlos Llamas
Cc: Benjamin Tissoires, Bastien Nocera, Jiri Kosina,
Filipe Laíns, Ping Cheng, Jason Gerecke, Viresh Kumar,
Johan Hovold, Alex Elder, Greg Kroah-Hartman, Lee Jones,
linux-input, linux-kernel, greybus-dev, linux-staging, linux-usb,
stable
In-Reply-To: <ahsh0UtTX6e0ZeHa@google.com>
Hi Carlos,
On May 30 2026, Carlos Llamas wrote:
> On Thu, Apr 16, 2026 at 04:46:28PM +0200, Benjamin Tissoires wrote:
> > On Thu, Apr 16, 2026 at 11:41 AM Bastien Nocera <hadess@hadess.net> wrote:
> > >
> > > On Wed, 2026-04-15 at 11:38 +0200, Benjamin Tissoires wrote:
> > > > hid_input_report() is used in too many places to have a commit that
> > > > doesn't cross subsystem borders. Instead of changing the API,
> > > > introduce
> > > > a new one when things matters in the transport layers:
> > > > - usbhid
> > > > - i2chid
> > > >
> > > > This effectively revert to the old behavior for those two transport
> > > > layers.
> > > >
> > > > Fixes: 0a3fe972a7cb ("HID: core: Mitigate potential OOB by removing
> > > > bogus memset()")
> > > > Cc: stable@vger.kernel.org
> > > > Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
> > > > ---
[...]
>
> Hi Benjamin, our CI started failing with commit 0a3fe972a7cb ("HID:
> core: Mitigate potential OOB by removing bogus memset()"), so I was
> hoping your patchset would fix this.
>
> However, I just realized our call path goes through uhid precisely,
> which still triggers the EINVAL error since uhid as not converted to
> hid_safe_input_report().
>
> My vague understanding though, is that uhid_event uses a static buffer
> in ev->data[UHID_DATA_MAX], so maybe we can use that through
> uhid_dev_input{2}()?
>
> I ran the following path through our CI and it fixed our issue, so I
> wanted to get your thoughts on this.
Oh, yes, you are correct. Sorry with all the back and forth on this
paritcular topic, my brain assumed that uhid was only allocating the
useful part of the payload and was not safe.
For the future me: the problem with uhid was that we were emultaing
devices that would trigger a bug elsewhere in the stack not in
uhid_dev_input*().
Patch looks good, please send it normally to the ML with your SoB :)
Cheers,
Benjamin
>
> Carlos Llamas
>
> ---
> drivers/hid/uhid.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/hid/uhid.c b/drivers/hid/uhid.c
> index 524b53a3c87b..37b60c3aaf66 100644
> --- a/drivers/hid/uhid.c
> +++ b/drivers/hid/uhid.c
> @@ -595,8 +595,8 @@ static int uhid_dev_input(struct uhid_device *uhid, struct uhid_event *ev)
> if (!READ_ONCE(uhid->running))
> return -EINVAL;
>
> - hid_input_report(uhid->hid, HID_INPUT_REPORT, ev->u.input.data,
> - min_t(size_t, ev->u.input.size, UHID_DATA_MAX), 0);
> + hid_safe_input_report(uhid->hid, HID_INPUT_REPORT, ev->u.input.data, UHID_DATA_MAX,
> + min_t(size_t, ev->u.input.size, UHID_DATA_MAX), 0);
>
> return 0;
> }
> @@ -606,8 +606,8 @@ static int uhid_dev_input2(struct uhid_device *uhid, struct uhid_event *ev)
> if (!READ_ONCE(uhid->running))
> return -EINVAL;
>
> - hid_input_report(uhid->hid, HID_INPUT_REPORT, ev->u.input2.data,
> - min_t(size_t, ev->u.input2.size, UHID_DATA_MAX), 0);
> + hid_safe_input_report(uhid->hid, HID_INPUT_REPORT, ev->u.input2.data, UHID_DATA_MAX,
> + min_t(size_t, ev->u.input2.size, UHID_DATA_MAX), 0);
>
> return 0;
> }
>
^ permalink raw reply
* [linux-6.12.y 1/4] HID: core: Add printk_ratelimited variants to hid_warn() etc
From: Lee Jones @ 2026-06-01 8:36 UTC (permalink / raw)
To: lee, Jiri Kosina, Benjamin Tissoires, Filipe Laíns,
Bastien Nocera, Ping Cheng, Jason Gerecke, Viresh Kumar,
Johan Hovold, Alex Elder, Greg Kroah-Hartman, Sasha Levin,
Mario Limonciello (AMD), Kwok Kin Ming, linux-input, linux-kernel,
linux-usb, greybus-dev, linux-staging, bpf
Cc: stable, Vicki Pfau, Jiri Kosina
From: Vicki Pfau <vi@endrift.com>
hid_warn_ratelimited() is needed. Add the others as part of the block.
Signed-off-by: Vicki Pfau <vi@endrift.com>
Signed-off-by: Jiri Kosina <jkosina@suse.com>
(cherry picked from commit 1d64624243af8329b4b219d8c39e28ea448f9929)
Signed-off-by: Lee Jones <lee@kernel.org>
---
include/linux/hid.h | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/include/linux/hid.h b/include/linux/hid.h
index 7d8d09318fa9..bef017d6b440 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -1245,4 +1245,15 @@ void hid_quirks_exit(__u16 bus);
#define hid_dbg_once(hid, fmt, ...) \
dev_dbg_once(&(hid)->dev, fmt, ##__VA_ARGS__)
+#define hid_err_ratelimited(hid, fmt, ...) \
+ dev_err_ratelimited(&(hid)->dev, fmt, ##__VA_ARGS__)
+#define hid_notice_ratelimited(hid, fmt, ...) \
+ dev_notice_ratelimited(&(hid)->dev, fmt, ##__VA_ARGS__)
+#define hid_warn_ratelimited(hid, fmt, ...) \
+ dev_warn_ratelimited(&(hid)->dev, fmt, ##__VA_ARGS__)
+#define hid_info_ratelimited(hid, fmt, ...) \
+ dev_info_ratelimited(&(hid)->dev, fmt, ##__VA_ARGS__)
+#define hid_dbg_ratelimited(hid, fmt, ...) \
+ dev_dbg_ratelimited(&(hid)->dev, fmt, ##__VA_ARGS__)
+
#endif
--
2.54.0.823.g6e5bcc1fc9-goog
^ permalink raw reply related
* [linux-6.12.y 2/4] HID: pass the buffer size to hid_report_raw_event
From: Lee Jones @ 2026-06-01 8:36 UTC (permalink / raw)
To: lee, Jiri Kosina, Benjamin Tissoires, Filipe Laíns,
Bastien Nocera, Ping Cheng, Jason Gerecke, Viresh Kumar,
Johan Hovold, Alex Elder, Greg Kroah-Hartman, Sasha Levin,
Kwok Kin Ming, Mario Limonciello (AMD), linux-input, linux-kernel,
linux-usb, greybus-dev, linux-staging, bpf
Cc: stable, Jiri Kosina
In-Reply-To: <20260601083642.908433-1-lee@kernel.org>
From: Benjamin Tissoires <bentiss@kernel.org>
[ Upstream commit 2c85c61d1332e1e16f020d76951baf167dcb6f7a ]
commit 0a3fe972a7cb ("HID: core: Mitigate potential OOB by removing
bogus memset()") enforced the provided data to be at least the size of
the declared buffer in the report descriptor to prevent a buffer
overflow. However, we can try to be smarter by providing both the buffer
size and the data size, meaning that hid_report_raw_event() can make
better decision whether we should plaining reject the buffer (buffer
overflow attempt) or if we can safely memset it to 0 and pass it to the
rest of the stack.
Fixes: 0a3fe972a7cb ("HID: core: Mitigate potential OOB by removing bogus memset()")
Cc: stable@vger.kernel.org
Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
Acked-by: Johan Hovold <johan@kernel.org>
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Jiri Kosina <jkosina@suse.com>
Stable-dep-of: 206342541fc8 ("HID: core: introduce hid_safe_input_report()")
Signed-off-by: Sasha Levin <sashal@kernel.org>
(cherry picked from commit 509c2605065004fc4cd86ee50a9350d402785307)
[Lee: Backported to linux-6.12.y and beyond]
Signed-off-by: Lee Jones <lee@kernel.org>
---
drivers/hid/bpf/hid_bpf_dispatch.c | 6 +++--
drivers/hid/hid-core.c | 37 +++++++++++++++++++++---------
drivers/hid/hid-gfrm.c | 4 ++--
drivers/hid/hid-logitech-hidpp.c | 2 +-
drivers/hid/hid-multitouch.c | 2 +-
drivers/hid/hid-primax.c | 2 +-
drivers/hid/hid-vivaldi-common.c | 2 +-
drivers/hid/wacom_sys.c | 6 ++---
drivers/staging/greybus/hid.c | 2 +-
include/linux/hid.h | 4 ++--
include/linux/hid_bpf.h | 14 +++++++----
11 files changed, 51 insertions(+), 30 deletions(-)
diff --git a/drivers/hid/bpf/hid_bpf_dispatch.c b/drivers/hid/bpf/hid_bpf_dispatch.c
index 284861c166d9..b711d83dfde1 100644
--- a/drivers/hid/bpf/hid_bpf_dispatch.c
+++ b/drivers/hid/bpf/hid_bpf_dispatch.c
@@ -24,7 +24,8 @@ EXPORT_SYMBOL(hid_ops);
u8 *
dispatch_hid_bpf_device_event(struct hid_device *hdev, enum hid_report_type type, u8 *data,
- u32 *size, int interrupt, u64 source, bool from_bpf)
+ size_t *buf_size, u32 *size, int interrupt, u64 source,
+ bool from_bpf)
{
struct hid_bpf_ctx_kern ctx_kern = {
.ctx = {
@@ -74,6 +75,7 @@ dispatch_hid_bpf_device_event(struct hid_device *hdev, enum hid_report_type type
*size = ret;
}
+ *buf_size = ctx_kern.ctx.allocated_size;
return ctx_kern.data;
}
EXPORT_SYMBOL_GPL(dispatch_hid_bpf_device_event);
@@ -514,7 +516,7 @@ __hid_bpf_input_report(struct hid_bpf_ctx *ctx, enum hid_report_type type, u8 *b
if (ret)
return ret;
- return hid_ops->hid_input_report(ctx->hid, type, buf, size, 0, (u64)(long)ctx, true,
+ return hid_ops->hid_input_report(ctx->hid, type, buf, size, size, 0, (u64)(long)ctx, true,
lock_already_taken);
}
diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index 294a25330ed0..ceff91722c3c 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -1998,24 +1998,32 @@ int __hid_request(struct hid_device *hid, struct hid_report *report,
}
EXPORT_SYMBOL_GPL(__hid_request);
-int hid_report_raw_event(struct hid_device *hid, enum hid_report_type type, u8 *data, u32 size,
- int interrupt)
+int hid_report_raw_event(struct hid_device *hid, enum hid_report_type type, u8 *data,
+ size_t bufsize, u32 size, int interrupt)
{
struct hid_report_enum *report_enum = hid->report_enum + type;
struct hid_report *report;
struct hid_driver *hdrv;
int max_buffer_size = HID_MAX_BUFFER_SIZE;
u32 rsize, csize = size;
+ size_t bsize = bufsize;
u8 *cdata = data;
int ret = 0;
report = hid_get_report(report_enum, data);
if (!report)
- goto out;
+ return 0;
+
+ if (unlikely(bsize < csize)) {
+ hid_warn_ratelimited(hid, "Event data for report %d is incorrect (%d vs %ld)\n",
+ report->id, csize, bsize);
+ return -EINVAL;
+ }
if (report_enum->numbered) {
cdata++;
csize--;
+ bsize--;
}
rsize = hid_compute_report_size(report);
@@ -2028,9 +2036,15 @@ int hid_report_raw_event(struct hid_device *hid, enum hid_report_type type, u8 *
else if (rsize > max_buffer_size)
rsize = max_buffer_size;
+ if (bsize < rsize) {
+ hid_warn_ratelimited(hid, "Event data for report %d was too short (%d vs %ld)\n",
+ report->id, rsize, bsize);
+ return -EINVAL;
+ }
+
if (csize < rsize) {
dbg_hid("report %d is too short, (%d < %d)\n", report->id,
- csize, rsize);
+ csize, rsize);
memset(cdata + csize, 0, rsize - csize);
}
@@ -2039,7 +2053,7 @@ int hid_report_raw_event(struct hid_device *hid, enum hid_report_type type, u8 *
if (hid->claimed & HID_CLAIMED_HIDRAW) {
ret = hidraw_report_event(hid, data, size);
if (ret)
- goto out;
+ return ret;
}
if (hid->claimed != HID_CLAIMED_HIDRAW && report->maxfield) {
@@ -2051,15 +2065,15 @@ int hid_report_raw_event(struct hid_device *hid, enum hid_report_type type, u8 *
if (hid->claimed & HID_CLAIMED_INPUT)
hidinput_report_event(hid, report);
-out:
+
return ret;
}
EXPORT_SYMBOL_GPL(hid_report_raw_event);
static int __hid_input_report(struct hid_device *hid, enum hid_report_type type,
- u8 *data, u32 size, int interrupt, u64 source, bool from_bpf,
- bool lock_already_taken)
+ u8 *data, size_t bufsize, u32 size, int interrupt, u64 source,
+ bool from_bpf, bool lock_already_taken)
{
struct hid_report_enum *report_enum;
struct hid_driver *hdrv;
@@ -2084,7 +2098,8 @@ static int __hid_input_report(struct hid_device *hid, enum hid_report_type type,
report_enum = hid->report_enum + type;
hdrv = hid->driver;
- data = dispatch_hid_bpf_device_event(hid, type, data, &size, interrupt, source, from_bpf);
+ data = dispatch_hid_bpf_device_event(hid, type, data, &bufsize, &size, interrupt,
+ source, from_bpf);
if (IS_ERR(data)) {
ret = PTR_ERR(data);
goto unlock;
@@ -2113,7 +2128,7 @@ static int __hid_input_report(struct hid_device *hid, enum hid_report_type type,
goto unlock;
}
- ret = hid_report_raw_event(hid, type, data, size, interrupt);
+ ret = hid_report_raw_event(hid, type, data, bufsize, size, interrupt);
unlock:
if (!lock_already_taken)
@@ -2135,7 +2150,7 @@ static int __hid_input_report(struct hid_device *hid, enum hid_report_type type,
int hid_input_report(struct hid_device *hid, enum hid_report_type type, u8 *data, u32 size,
int interrupt)
{
- return __hid_input_report(hid, type, data, size, interrupt, 0,
+ return __hid_input_report(hid, type, data, size, size, interrupt, 0,
false, /* from_bpf */
false /* lock_already_taken */);
}
diff --git a/drivers/hid/hid-gfrm.c b/drivers/hid/hid-gfrm.c
index 699186ff2349..d2a56bf92b41 100644
--- a/drivers/hid/hid-gfrm.c
+++ b/drivers/hid/hid-gfrm.c
@@ -66,7 +66,7 @@ static int gfrm_raw_event(struct hid_device *hdev, struct hid_report *report,
switch (data[1]) {
case GFRM100_SEARCH_KEY_DOWN:
ret = hid_report_raw_event(hdev, HID_INPUT_REPORT, search_key_dn,
- sizeof(search_key_dn), 1);
+ sizeof(search_key_dn), sizeof(search_key_dn), 1);
break;
case GFRM100_SEARCH_KEY_AUDIO_DATA:
@@ -74,7 +74,7 @@ static int gfrm_raw_event(struct hid_device *hdev, struct hid_report *report,
case GFRM100_SEARCH_KEY_UP:
ret = hid_report_raw_event(hdev, HID_INPUT_REPORT, search_key_up,
- sizeof(search_key_up), 1);
+ sizeof(search_key_up), sizeof(search_key_up), 1);
break;
default:
diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index d60cd4379e86..858ac2ab46bd 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -3691,7 +3691,7 @@ static int hidpp10_consumer_keys_raw_event(struct hidpp_device *hidpp,
memcpy(&consumer_report[1], &data[3], 4);
/* We are called from atomic context */
hid_report_raw_event(hidpp->hid_dev, HID_INPUT_REPORT,
- consumer_report, 5, 1);
+ consumer_report, sizeof(consumer_report), 5, 1);
return 1;
}
diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index fcf9a806f109..760f9db44c9e 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -500,7 +500,7 @@ static void mt_get_feature(struct hid_device *hdev, struct hid_report *report)
}
ret = hid_report_raw_event(hdev, HID_FEATURE_REPORT, buf,
- size, 0);
+ size, size, 0);
if (ret)
dev_warn(&hdev->dev, "failed to report feature\n");
}
diff --git a/drivers/hid/hid-primax.c b/drivers/hid/hid-primax.c
index e44d79dff8de..8db054280afb 100644
--- a/drivers/hid/hid-primax.c
+++ b/drivers/hid/hid-primax.c
@@ -44,7 +44,7 @@ static int px_raw_event(struct hid_device *hid, struct hid_report *report,
data[0] |= (1 << (data[idx] - 0xE0));
data[idx] = 0;
}
- hid_report_raw_event(hid, HID_INPUT_REPORT, data, size, 0);
+ hid_report_raw_event(hid, HID_INPUT_REPORT, data, size, size, 0);
return 1;
default: /* unknown report */
diff --git a/drivers/hid/hid-vivaldi-common.c b/drivers/hid/hid-vivaldi-common.c
index bf734055d4b6..b12bb5cc091a 100644
--- a/drivers/hid/hid-vivaldi-common.c
+++ b/drivers/hid/hid-vivaldi-common.c
@@ -85,7 +85,7 @@ void vivaldi_feature_mapping(struct hid_device *hdev,
}
ret = hid_report_raw_event(hdev, HID_FEATURE_REPORT, report_data,
- report_len, 0);
+ report_len, report_len, 0);
if (ret) {
dev_warn(&hdev->dev, "failed to report feature %d\n",
field->report->id);
diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index 1b1112772777..ffcf65dcf713 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -74,7 +74,7 @@ static void wacom_wac_queue_flush(struct hid_device *hdev,
int err;
size = kfifo_out(fifo, buf, sizeof(buf));
- err = hid_report_raw_event(hdev, HID_INPUT_REPORT, buf, size, false);
+ err = hid_report_raw_event(hdev, HID_INPUT_REPORT, buf, size, size, false);
if (err) {
hid_warn(hdev, "%s: unable to flush event due to error %d\n",
__func__, err);
@@ -319,7 +319,7 @@ static void wacom_feature_mapping(struct hid_device *hdev,
data, n, WAC_CMD_RETRIES);
if (ret == n && features->type == HID_GENERIC) {
ret = hid_report_raw_event(hdev,
- HID_FEATURE_REPORT, data, n, 0);
+ HID_FEATURE_REPORT, data, n, n, 0);
} else if (ret == 2 && features->type != HID_GENERIC) {
features->touch_max = data[1];
} else {
@@ -380,7 +380,7 @@ static void wacom_feature_mapping(struct hid_device *hdev,
data, n, WAC_CMD_RETRIES);
if (ret == n) {
ret = hid_report_raw_event(hdev, HID_FEATURE_REPORT,
- data, n, 0);
+ data, n, n, 0);
} else {
hid_warn(hdev, "%s: could not retrieve sensor offsets\n",
__func__);
diff --git a/drivers/staging/greybus/hid.c b/drivers/staging/greybus/hid.c
index 63c77a3df591..afa78c96ede8 100644
--- a/drivers/staging/greybus/hid.c
+++ b/drivers/staging/greybus/hid.c
@@ -201,7 +201,7 @@ static void gb_hid_init_report(struct gb_hid *ghid, struct hid_report *report)
* we just need to setup the input fields, so using
* hid_report_raw_event is safe.
*/
- hid_report_raw_event(ghid->hid, report->type, ghid->inbuf, size, 1);
+ hid_report_raw_event(ghid->hid, report->type, ghid->inbuf, ghid->bufsize, size, 1);
}
static void gb_hid_init_reports(struct gb_hid *ghid)
diff --git a/include/linux/hid.h b/include/linux/hid.h
index bef017d6b440..fdd401e4ebde 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -1213,8 +1213,8 @@ static inline u32 hid_report_len(struct hid_report *report)
return DIV_ROUND_UP(report->size, 8) + (report->id > 0);
}
-int hid_report_raw_event(struct hid_device *hid, enum hid_report_type type, u8 *data, u32 size,
- int interrupt);
+int hid_report_raw_event(struct hid_device *hid, enum hid_report_type type, u8 *data,
+ size_t bufsize, u32 size, int interrupt);
/* HID quirks API */
unsigned long hid_lookup_quirk(const struct hid_device *hdev);
diff --git a/include/linux/hid_bpf.h b/include/linux/hid_bpf.h
index 6a47223e6460..aa87513acbcd 100644
--- a/include/linux/hid_bpf.h
+++ b/include/linux/hid_bpf.h
@@ -72,8 +72,8 @@ struct hid_ops {
int (*hid_hw_output_report)(struct hid_device *hdev, __u8 *buf, size_t len,
u64 source, bool from_bpf);
int (*hid_input_report)(struct hid_device *hid, enum hid_report_type type,
- u8 *data, u32 size, int interrupt, u64 source, bool from_bpf,
- bool lock_already_taken);
+ u8 *data, size_t bufsize, u32 size, int interrupt, u64 source,
+ bool from_bpf, bool lock_already_taken);
struct module *owner;
const struct bus_type *bus_type;
};
@@ -200,7 +200,8 @@ struct hid_bpf {
#ifdef CONFIG_HID_BPF
u8 *dispatch_hid_bpf_device_event(struct hid_device *hid, enum hid_report_type type, u8 *data,
- u32 *size, int interrupt, u64 source, bool from_bpf);
+ size_t *buf_size, u32 *size, int interrupt, u64 source,
+ bool from_bpf);
int dispatch_hid_bpf_raw_requests(struct hid_device *hdev,
unsigned char reportnum, __u8 *buf,
u32 size, enum hid_report_type rtype,
@@ -215,8 +216,11 @@ int hid_bpf_device_init(struct hid_device *hid);
u8 *call_hid_bpf_rdesc_fixup(struct hid_device *hdev, const u8 *rdesc, unsigned int *size);
#else /* CONFIG_HID_BPF */
static inline u8 *dispatch_hid_bpf_device_event(struct hid_device *hid, enum hid_report_type type,
- u8 *data, u32 *size, int interrupt,
- u64 source, bool from_bpf) { return data; }
+ u8 *data, size_t *buf_size, u32 *size,
+ int interrupt, u64 source, bool from_bpf)
+{
+ return data;
+}
static inline int dispatch_hid_bpf_raw_requests(struct hid_device *hdev,
unsigned char reportnum, u8 *buf,
u32 size, enum hid_report_type rtype,
--
2.54.0.823.g6e5bcc1fc9-goog
^ permalink raw reply related
* [linux-6.12.y 3/4] HID: core: introduce hid_safe_input_report()
From: Lee Jones @ 2026-06-01 8:36 UTC (permalink / raw)
To: lee, Jiri Kosina, Benjamin Tissoires, Filipe Laíns,
Bastien Nocera, Ping Cheng, Jason Gerecke, Viresh Kumar,
Johan Hovold, Alex Elder, Greg Kroah-Hartman, Sasha Levin,
Mario Limonciello (AMD), Kwok Kin Ming, linux-input, linux-kernel,
linux-usb, greybus-dev, linux-staging, bpf
Cc: stable, Jiri Kosina
In-Reply-To: <20260601083642.908433-1-lee@kernel.org>
From: Benjamin Tissoires <bentiss@kernel.org>
[ Upstream commit 206342541fc887ae919774a43942dc883161fece ]
hid_input_report() is used in too many places to have a commit that
doesn't cross subsystem borders. Instead of changing the API, introduce
a new one when things matters in the transport layers:
- usbhid
- i2chid
This effectively revert to the old behavior for those two transport
layers.
Fixes: 0a3fe972a7cb ("HID: core: Mitigate potential OOB by removing bogus memset()")
Cc: stable@vger.kernel.org
Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
Signed-off-by: Jiri Kosina <jkosina@suse.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
(cherry picked from commit 301338b8edadc67a42b1c86add975091e66768d9)
Signed-off-by: Lee Jones <lee@kernel.org>
---
drivers/hid/hid-core.c | 25 +++++++++++++++++++++++++
drivers/hid/i2c-hid/i2c-hid-core.c | 7 ++++---
drivers/hid/usbhid/hid-core.c | 11 ++++++-----
include/linux/hid.h | 2 ++
4 files changed, 37 insertions(+), 8 deletions(-)
diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index ceff91722c3c..d9ea99cdb68e 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -2146,6 +2146,7 @@ static int __hid_input_report(struct hid_device *hid, enum hid_report_type type,
* @interrupt: distinguish between interrupt and control transfers
*
* This is data entry for lower layers.
+ * Legacy, please use hid_safe_input_report() instead.
*/
int hid_input_report(struct hid_device *hid, enum hid_report_type type, u8 *data, u32 size,
int interrupt)
@@ -2156,6 +2157,30 @@ int hid_input_report(struct hid_device *hid, enum hid_report_type type, u8 *data
}
EXPORT_SYMBOL_GPL(hid_input_report);
+/**
+ * hid_safe_input_report - report data from lower layer (usb, bt...)
+ *
+ * @hid: hid device
+ * @type: HID report type (HID_*_REPORT)
+ * @data: report contents
+ * @bufsize: allocated size of the data buffer
+ * @size: useful size of data parameter
+ * @interrupt: distinguish between interrupt and control transfers
+ *
+ * This is data entry for lower layers.
+ * Please use this function instead of the non safe version because we provide
+ * here the size of the buffer, allowing hid-core to make smarter decisions
+ * regarding the incoming buffer.
+ */
+int hid_safe_input_report(struct hid_device *hid, enum hid_report_type type, u8 *data,
+ size_t bufsize, u32 size, int interrupt)
+{
+ return __hid_input_report(hid, type, data, bufsize, size, interrupt, 0,
+ false, /* from_bpf */
+ false /* lock_already_taken */);
+}
+EXPORT_SYMBOL_GPL(hid_safe_input_report);
+
bool hid_match_one_id(const struct hid_device *hdev,
const struct hid_device_id *id)
{
diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c b/drivers/hid/i2c-hid/i2c-hid-core.c
index cf8ae0df0cda..8ce0535fc42d 100644
--- a/drivers/hid/i2c-hid/i2c-hid-core.c
+++ b/drivers/hid/i2c-hid/i2c-hid-core.c
@@ -568,9 +568,10 @@ static void i2c_hid_get_input(struct i2c_hid *ihid)
if (ihid->hid->group != HID_GROUP_RMI)
pm_wakeup_event(&ihid->client->dev, 0);
- hid_input_report(ihid->hid, HID_INPUT_REPORT,
- ihid->inbuf + sizeof(__le16),
- ret_size - sizeof(__le16), 1);
+ hid_safe_input_report(ihid->hid, HID_INPUT_REPORT,
+ ihid->inbuf + sizeof(__le16),
+ ihid->bufsize - sizeof(__le16),
+ ret_size - sizeof(__le16), 1);
}
return;
diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
index f14b46ce00cb..336ad7cf3d48 100644
--- a/drivers/hid/usbhid/hid-core.c
+++ b/drivers/hid/usbhid/hid-core.c
@@ -283,9 +283,9 @@ static void hid_irq_in(struct urb *urb)
break;
usbhid_mark_busy(usbhid);
if (!test_bit(HID_RESUME_RUNNING, &usbhid->iofl)) {
- hid_input_report(urb->context, HID_INPUT_REPORT,
- urb->transfer_buffer,
- urb->actual_length, 1);
+ hid_safe_input_report(urb->context, HID_INPUT_REPORT,
+ urb->transfer_buffer, urb->transfer_buffer_length,
+ urb->actual_length, 1);
/*
* autosuspend refused while keys are pressed
* because most keyboards don't wake up when
@@ -482,9 +482,10 @@ static void hid_ctrl(struct urb *urb)
switch (status) {
case 0: /* success */
if (usbhid->ctrl[usbhid->ctrltail].dir == USB_DIR_IN)
- hid_input_report(urb->context,
+ hid_safe_input_report(urb->context,
usbhid->ctrl[usbhid->ctrltail].report->type,
- urb->transfer_buffer, urb->actual_length, 0);
+ urb->transfer_buffer, urb->transfer_buffer_length,
+ urb->actual_length, 0);
break;
case -ESHUTDOWN: /* unplug */
unplug = 1;
diff --git a/include/linux/hid.h b/include/linux/hid.h
index fdd401e4ebde..7d05b1edacd8 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -949,6 +949,8 @@ struct hid_field *hid_find_field(struct hid_device *hdev, unsigned int report_ty
int hid_set_field(struct hid_field *, unsigned, __s32);
int hid_input_report(struct hid_device *hid, enum hid_report_type type, u8 *data, u32 size,
int interrupt);
+int hid_safe_input_report(struct hid_device *hid, enum hid_report_type type, u8 *data,
+ size_t bufsize, u32 size, int interrupt);
struct hid_field *hidinput_get_led_field(struct hid_device *hid);
unsigned int hidinput_count_leds(struct hid_device *hid);
__s32 hidinput_calc_abs_res(const struct hid_field *field, __u16 code);
--
2.54.0.823.g6e5bcc1fc9-goog
^ permalink raw reply related
* [linux-6.12.y 4/4] HID: core: Fix size_t specifier in hid_report_raw_event()
From: Lee Jones @ 2026-06-01 8:36 UTC (permalink / raw)
To: lee, Jiri Kosina, Benjamin Tissoires, Filipe Laíns,
Bastien Nocera, Ping Cheng, Jason Gerecke, Viresh Kumar,
Johan Hovold, Alex Elder, Greg Kroah-Hartman, Sasha Levin,
Mario Limonciello (AMD), Kwok Kin Ming, linux-input, linux-kernel,
linux-usb, greybus-dev, linux-staging, bpf
Cc: stable, Nathan Chancellor, Miguel Ojeda, Linus Torvalds
In-Reply-To: <20260601083642.908433-1-lee@kernel.org>
From: Nathan Chancellor <nathan@kernel.org>
[ Upstream commit 4d3a2a466b8d68d852a1f3bbf11204b718428dc4 ]
When building for 32-bit platforms, for which 'size_t' is
'unsigned int', there are warnings around using the incorrect format
specifier to print bsize in hid_report_raw_event():
drivers/hid/hid-core.c:2054:29: error: format specifies type 'long' but the argument has type 'size_t' (aka 'unsigned int') [-Werror,-Wformat]
2053 | hid_warn_ratelimited(hid, "Event data for report %d is incorrect (%d vs %ld)\n",
| ~~~
| %zu
2054 | report->id, csize, bsize);
| ^~~~~
drivers/hid/hid-core.c:2076:29: error: format specifies type 'long' but the argument has type 'size_t' (aka 'unsigned int') [-Werror,-Wformat]
2075 | hid_warn_ratelimited(hid, "Event data for report %d was too short (%d vs %ld)\n",
| ~~~
| %zu
2076 | report->id, rsize, bsize);
| ^~~~~
Use the proper 'size_t' format specifier, '%zu', to clear up the
warnings.
Cc: stable@vger.kernel.org
Fixes: 2c85c61d1332 ("HID: pass the buffer size to hid_report_raw_event")
Reported-by: Miguel Ojeda <ojeda@kernel.org>
Closes: https://lore.kernel.org/20260516020430.110135-1-ojeda@kernel.org/
Signed-off-by: Nathan Chancellor <nathan@kernel.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
(cherry picked from commit 3ab135238832446399614e7a4bb796d620717806)
Signed-off-by: Lee Jones <lee@kernel.org>
---
drivers/hid/hid-core.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index d9ea99cdb68e..87d990ada868 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -2015,7 +2015,7 @@ int hid_report_raw_event(struct hid_device *hid, enum hid_report_type type, u8 *
return 0;
if (unlikely(bsize < csize)) {
- hid_warn_ratelimited(hid, "Event data for report %d is incorrect (%d vs %ld)\n",
+ hid_warn_ratelimited(hid, "Event data for report %d is incorrect (%d vs %zu)\n",
report->id, csize, bsize);
return -EINVAL;
}
@@ -2037,7 +2037,7 @@ int hid_report_raw_event(struct hid_device *hid, enum hid_report_type type, u8 *
rsize = max_buffer_size;
if (bsize < rsize) {
- hid_warn_ratelimited(hid, "Event data for report %d was too short (%d vs %ld)\n",
+ hid_warn_ratelimited(hid, "Event data for report %d was too short (%d vs %zu)\n",
report->id, rsize, bsize);
return -EINVAL;
}
--
2.54.0.823.g6e5bcc1fc9-goog
^ permalink raw reply related
* Re: [linux-6.12.y 3/4] HID: core: introduce hid_safe_input_report()
From: sashiko-bot @ 2026-06-01 8:46 UTC (permalink / raw)
To: Lee Jones; +Cc: linux-input, bpf, dmitry.torokhov
In-Reply-To: <20260601083642.908433-3-lee@kernel.org>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues:
- [Low] The patch introduces a hard build failure by passing 9 arguments to a function expecting 8.
Pre-existing issues:
- [High] Integer underflow in `i2c_hid_get_input()` when computing the remaining buffer size for bogus HID descriptors.
--
commit d54718e72e548c203a2c42b46b69bf352c081a00
Author: Benjamin Tissoires <bentiss@kernel.org>
HID: core: introduce hid_safe_input_report()
This patch introduces hid_safe_input_report() to be used in transport
layers like usbhid and i2chid. This provides the buffer size allowing
the hid-core to make smarter decisions regarding incoming buffers.
> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
> index 294a25330ed03..2c5583818421e 100644
> --- a/drivers/hid/hid-core.c
> +++ b/drivers/hid/hid-core.c
[ ... ]
> @@ -2141,6 +2142,30 @@ int hid_input_report(struct hid_device *hid, enum hid_report_type type, u8 *data
> }
> EXPORT_SYMBOL_GPL(hid_input_report);
>
> +/**
> + * hid_safe_input_report - report data from lower layer (usb, bt...)
> + *
> + * @hid: hid device
> + * @type: HID report type (HID_*_REPORT)
> + * @data: report contents
> + * @bufsize: allocated size of the data buffer
> + * @size: useful size of data parameter
> + * @interrupt: distinguish between interrupt and control transfers
> + *
> + * This is data entry for lower layers.
> + * Please use this function instead of the non safe version because we provide
> + * here the size of the buffer, allowing hid-core to make smarter decisions
> + * regarding the incoming buffer.
> + */
> +int hid_safe_input_report(struct hid_device *hid, enum hid_report_type type, u8 *data,
> + size_t bufsize, u32 size, int interrupt)
> +{
> + return __hid_input_report(hid, type, data, bufsize, size, interrupt, 0,
> + false, /* from_bpf */
> + false /* lock_already_taken */);
> +}
[Severity: Low]
Does this cause a build failure? Looking at the definition of
__hid_input_report() in this file, it only accepts 8 arguments:
static int __hid_input_report(struct hid_device *hid, enum hid_report_type type,
u8 *data, u32 size, int interrupt, u64 source,
bool from_bpf, bool lock_already_taken)
This call passes 9 arguments, including the new bufsize parameter. Were some
prerequisite upstream patches updating the signature of __hid_input_report()
omitted from this backport?
> +EXPORT_SYMBOL_GPL(hid_safe_input_report);
> +
> bool hid_match_one_id(const struct hid_device *hdev,
> const struct hid_device_id *id)
[ ... ]
> diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c b/drivers/hid/i2c-hid/i2c-hid-core.c
> index cf8ae0df0cda9..8ce0535fc42d6 100644
> --- a/drivers/hid/i2c-hid/i2c-hid-core.c
> +++ b/drivers/hid/i2c-hid/i2c-hid-core.c
> @@ -568,9 +568,10 @@ static void i2c_hid_get_input(struct i2c_hid *ihid)
> if (ihid->hid->group != HID_GROUP_RMI)
> pm_wakeup_event(&ihid->client->dev, 0);
>
> - hid_input_report(ihid->hid, HID_INPUT_REPORT,
> - ihid->inbuf + sizeof(__le16),
> - ret_size - sizeof(__le16), 1);
> + hid_safe_input_report(ihid->hid, HID_INPUT_REPORT,
> + ihid->inbuf + sizeof(__le16),
> + ihid->bufsize - sizeof(__le16),
> + ret_size - sizeof(__le16), 1);
[Severity: High]
This is a pre-existing issue, but can this calculation underflow and pass a
massive size to the HID core?
If a device has the I2C_HID_QUIRK_BAD_INPUT_SIZE quirk and provides a bogus
descriptor with wMaxInputLength < 2, the size variable will be < 2, and the
code earlier in i2c_hid_get_input() forces ret_size = size.
When ret_size and ihid->bufsize are less than sizeof(__le16) (which is 2),
subtracting sizeof(__le16) will result in an unsigned integer underflow.
This large underflowed value would then be passed into the HID core here and
could lead to out-of-bounds reads in functions like hid_dump_report().
> }
>
> return;
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260601083642.908433-3-lee@kernel.org?part=1
^ permalink raw reply
* Re: [linux-6.12.y 2/4] HID: pass the buffer size to hid_report_raw_event
From: sashiko-bot @ 2026-06-01 8:50 UTC (permalink / raw)
To: Lee Jones; +Cc: dmitry.torokhov, linux-input, bpf
In-Reply-To: <20260601083642.908433-2-lee@kernel.org>
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
New issues:
- [High] The patch fails to resolve the short-report regression for standard transport drivers because APIs do not propagate the real buffer size, bypassing the newly introduced padding logic.
- [Low] Format string mismatch for `size_t` variable.
Pre-existing issues:
- [Critical] Heap buffer overflow in `dispatch_hid_bpf_device_event` when processing incoming events.
- [High] A BPF program calling the `bpf_hid_input_report` helper causes unhandled re-entrancy that silently corrupts the shared per-device BPF data buffer.
--
commit e46292c21905a8e10fe2c446a47f211679b0d355
Author: Benjamin Tissoires <bentiss@kernel.org>
HID: pass the buffer size to hid_report_raw_event
This commit attempts to fix an OOB mitigation by passing both the buffer
capacity and the actual data size to hid_report_raw_event(). The goal is
to allow safe zero-padding of short reports if the backing buffer is large
enough, rather than unconditionally rejecting them.
[ Upstream commit 2c85c61d1332e1e16f020d76951baf167dcb6f7a ]
[Lee: Backported to linux-6.12.y and beyond]
> diff --git a/drivers/hid/bpf/hid_bpf_dispatch.c b/drivers/hid/bpf/hid_bpf_dispatch.c
> index 284861c166d9c..b711d83dfde1d 100644
> --- a/drivers/hid/bpf/hid_bpf_dispatch.c
> +++ b/drivers/hid/bpf/hid_bpf_dispatch.c
> @@ -24,7 +24,8 @@ EXPORT_SYMBOL(hid_ops);
>
> u8 *
> dispatch_hid_bpf_device_event(struct hid_device *hdev, enum hid_report_type type, u8 *data,
> - u32 *size, int interrupt, u64 source, bool from_bpf)
> + size_t *buf_size, u32 *size, int interrupt, u64 source,
> + bool from_bpf)
> {
[Severity: Critical]
This is a pre-existing issue, but does dispatch_hid_bpf_device_event() lack
bounds checking on the event size?
When a HID event arrives, the function unconditionally copies the data into the
BPF context buffer:
memset(ctx_kern.data, 0, hdev->bpf.allocated_data);
memcpy(ctx_kern.data, data, *size);
If a malicious or compromised HID device (or userspace via uhid) sends an
event where *size is larger than hdev->bpf.allocated_data, could this cause a
heap buffer overflow?
[Severity: High]
This is also a pre-existing issue, but does this code suffer from unhandled
re-entrancy corrupting shared state?
If a BPF program injects an input report by calling the bpf_hid_input_report()
helper, execution re-enters dispatch_hid_bpf_device_event(). The inner call
will unconditionally execute the memset() and memcpy() block shown above on
the exact same shared hdev->bpf.device_data buffer.
If the injected report uses ctx->data as its buffer, the memset wipes the data
before it can be copied. Will this silently corrupt the execution state of the
outer BPF program upon return?
[ ... ]
> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
> index 294a25330ed03..ceff91722c3c8 100644
> --- a/drivers/hid/hid-core.c
> +++ b/drivers/hid/hid-core.c
[ ... ]
> @@ -2028,9 +2036,15 @@ int hid_report_raw_event(struct hid_device *hid, enum hid_report_type type, u8 *
> else if (rsize > max_buffer_size)
> rsize = max_buffer_size;
>
> + if (bsize < rsize) {
> + hid_warn_ratelimited(hid, "Event data for report %d was too short (%d vs %ld)\n",
> + report->id, rsize, bsize);
[Severity: Low]
Can we use the %zu format specifier instead of %ld for bsize?
bsize is a size_t, which might trigger a compiler warning on 32-bit platforms
where size_t is unsigned int.
> + return -EINVAL;
> + }
> +
[ ... ]
> @@ -2135,7 +2150,7 @@ static int __hid_input_report(struct hid_device *hid, enum hid_report_type type,
> int hid_input_report(struct hid_device *hid, enum hid_report_type type, u8 *data, u32 size,
> int interrupt)
> {
> - return __hid_input_report(hid, type, data, size, interrupt, 0,
> + return __hid_input_report(hid, type, data, size, size, interrupt, 0,
[Severity: High]
Does this leave the short-report regression unfixed for standard transport
drivers?
By blindly passing the received data size as both the size and bufsize capacity
arguments to __hid_input_report(), bsize will always equal size.
If the report is short (size < rsize), the new bsize < rsize condition in
hid_report_raw_event() evaluates to true and the event is dropped, bypassing
the padding logic completely.
> false, /* from_bpf */
> false /* lock_already_taken */);
> }
[ ... ]
> diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
> index 1b1112772777c..ffcf65dcf7134 100644
> --- a/drivers/hid/wacom_sys.c
> +++ b/drivers/hid/wacom_sys.c
> @@ -74,7 +74,7 @@ static void wacom_wac_queue_flush(struct hid_device *hdev,
> int err;
>
> size = kfifo_out(fifo, buf, sizeof(buf));
> - err = hid_report_raw_event(hdev, HID_INPUT_REPORT, buf, size, false);
> + err = hid_report_raw_event(hdev, HID_INPUT_REPORT, buf, size, size, false);
[Severity: High]
Does passing size instead of sizeof(buf) for the buffer capacity prevent safe
padding for wacom devices?
Similar to the issue above, because bsize becomes equal to size, any short
report will fail the bsize < rsize check in hid_report_raw_event() and be
rejected instead of zeroes being safely padded to the end of the buffer.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260601083642.908433-2-lee@kernel.org?part=1
^ permalink raw reply
* Re: [PATCH v5] HID: steelseries: Add MSI Raider A18 HX A9WJG RGB support
From: kernel test robot @ 2026-06-01 9:04 UTC (permalink / raw)
To: David Glushkov, Jiri Kosina
Cc: oe-kbuild-all, Benjamin Tissoires, linux-input, linux-kernel
In-Reply-To: <20260531233903.208321-1-david.glushkov@sntiq.com>
Hi David,
kernel test robot noticed the following build warnings:
[auto build test WARNING on hid/for-next]
[also build test WARNING on linus/master v7.1-rc6 next-20260529]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/David-Glushkov/HID-steelseries-Add-MSI-Raider-A18-HX-A9WJG-RGB-support/20260601-074317
base: https://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git for-next
patch link: https://lore.kernel.org/r/20260531233903.208321-1-david.glushkov%40sntiq.com
patch subject: [PATCH v5] HID: steelseries: Add MSI Raider A18 HX A9WJG RGB support
config: x86_64-rhel-9.4-kunit (https://download.01.org/0day-ci/archive/20260601/202606011654.dFHta3Dw-lkp@intel.com/config)
compiler: gcc-14 (Debian 14.2.0-19) 14.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260601/202606011654.dFHta3Dw-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202606011654.dFHta3Dw-lkp@intel.com/
All warnings (new ones prefixed by >>):
>> drivers/hid/hid-steelseries.c:569:27: warning: 'steelseries_hid_to_usb_dev' defined but not used [-Wunused-function]
569 | static struct usb_device *steelseries_hid_to_usb_dev(struct hid_device *hdev)
| ^~~~~~~~~~~~~~~~~~~~~~~~~~
vim +/steelseries_hid_to_usb_dev +569 drivers/hid/hid-steelseries.c
568
> 569 static struct usb_device *steelseries_hid_to_usb_dev(struct hid_device *hdev)
570 {
571 struct usb_interface *intf = steelseries_hid_to_usb_intf(hdev);
572
573 if (!intf)
574 return NULL;
575
576 return interface_to_usbdev(intf);
577 }
578
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* [PATCH v6] HID: steelseries: Add MSI Raider A18 HX A9WJG RGB support
From: David Glushkov @ 2026-06-01 12:11 UTC (permalink / raw)
To: Jiri Kosina
Cc: Benjamin Tissoires, linux-input, linux-kernel, kernel test robot
In-Reply-To: <20260531215204.172030-1-david.glushkov@sntiq.com>
The MSI Raider A18 HX A9WJG exposes two internal SteelSeries USB HID
devices for RGB lighting: KLC (1038:1122) for the keyboard and ALC
(1038:1161) for the lightbar/logo zones.
Add DMI-gated support for these devices and expose them as multicolor
LED class devices. The driver sends the same HID class SET_REPORT
control transfer as the tested userspace implementation for this
machine and writes a uniform RGB value to all known keyboard keys or
ALC zones.
Limit RGB support to USB interface 0 and the tested DMI system because
the KLC product ID is shared across MSI laptop designs and the key
layout mapping is model-specific. If the DMI or interface check does
not match, keep the device bound as a regular HID device instead of
failing probe.
Tested on MSI Raider A18 HX A9WJG. Both internal SteelSeries ALC
(1038:1161) and KLC (1038:1122) HID devices bind on interface 0 and
create steelseries::lightbar and steelseries::kbd_backlight. Setting
multi_intensity and brightness changes the keyboard and lightbar
colors.
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202606010709.X0QYNjFZ-lkp@intel.com/
Signed-off-by: David Glushkov <david.glushkov@sntiq.com>
---
v6:
- Fix W=1 build warning when CONFIG_LEDS_CLASS_MULTICOLOR is disabled by
moving steelseries_hid_to_usb_dev() into the multicolor LED code path.
v5:
- Drop pm_ret handling and ignore PM_HINT_NORMAL cleanup errors.
- Fix LED registration error handling to clean up rgb_buf on failure.
- Fix trailing whitespaces and formatting style nits.
- Update commit message to accurately reflect the DMI fallback behavior.
v4:
- Fix literal \n typo in C code.
- Remove unused steelseries_msi_rgb_free_buf from #else block.
- Do not fail probe on unsupported DMI/interface; clear MSI RGB quirk and continue normal HID initialization.
- Do not fail probe when RGB LED registration fails; keep the HID device usable without RGB LED support.
- Add explicit linux/slab.h include for kzalloc/kfree.
v3:
- Fix build failure (added missing err_close label to steelseries_probe).
- Fix DMA API violation (use kzalloc instead of devm_kzalloc for usb transfer buffer).
- Fix C syntax declaration-after-statement warning.
- Fix type confusion for SRWS1 (add early check in raw_event before hid_get_drvdata).
- Fix delayed_work crash (add battery_registered flag).
v2:
- Fixed unsafe to_usb_interface cast by checking hid_is_usb() first.
- Fixed uninitialized delayed_work warning by restricting cancel_delayed_work_sync to headset devices.
- Fixed error path leaks in probe (hid_hw_stop / hid_hw_close).
- Added hid_hw_power PM wrappers around direct usb_control_msg transfers.
drivers/hid/hid-ids.h | 2 +
drivers/hid/hid-steelseries.c | 277 ++++++++++++++++++++++++++++++++--
2 files changed, 270 insertions(+), 9 deletions(-)
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 4657d96fb..4af4397b8 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -1367,6 +1367,8 @@
#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_MSI_KLC 0x1122
+#define USB_DEVICE_ID_STEELSERIES_MSI_ALC 0x1161
#define USB_VENDOR_ID_SUN 0x0430
#define USB_DEVICE_ID_RARITAN_KVM_DONGLE 0xcdab
diff --git a/drivers/hid/hid-steelseries.c b/drivers/hid/hid-steelseries.c
index f98435631..4915cf6eb 100644
--- a/drivers/hid/hid-steelseries.c
+++ b/drivers/hid/hid-steelseries.c
@@ -10,16 +10,26 @@
*/
#include <linux/device.h>
+#include <linux/dmi.h>
#include <linux/hid.h>
#include <linux/module.h>
#include <linux/usb.h>
#include <linux/leds.h>
+#include <linux/led-class-multicolor.h>
+#include <linux/slab.h>
#include "hid-ids.h"
#define STEELSERIES_SRWS1 BIT(0)
#define STEELSERIES_ARCTIS_1 BIT(1)
#define STEELSERIES_ARCTIS_9 BIT(2)
+#define STEELSERIES_MSI_RGB BIT(3)
+
+#define STEELSERIES_MSI_RGB_REPORT_LEN 524
+
+#define STEELSERIES_HAS_LEDS_MULTICOLOR \
+ (IS_BUILTIN(CONFIG_LEDS_CLASS_MULTICOLOR) || \
+ (IS_MODULE(CONFIG_LEDS_CLASS_MULTICOLOR) && IS_MODULE(CONFIG_HID_STEELSERIES)))
struct steelseries_device {
struct hid_device *hdev;
@@ -34,6 +44,14 @@ struct steelseries_device {
uint8_t battery_capacity;
bool headset_connected;
bool battery_charging;
+ bool battery_registered;
+
+#if STEELSERIES_HAS_LEDS_MULTICOLOR
+ struct led_classdev_mc mc_cdev;
+ struct mc_subled subled_info[3];
+ struct mutex rgb_lock; /* protects rgb_buf */
+ u8 *rgb_buf;
+#endif
};
#if IS_BUILTIN(CONFIG_LEDS_CLASS) || \
@@ -510,6 +528,7 @@ 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);
+ sd->battery_registered = true;
steelseries_headset_fetch_battery(sd->hdev);
if (sd->quirks & STEELSERIES_ARCTIS_9) {
@@ -528,6 +547,196 @@ static bool steelseries_is_vendor_usage_page(struct hid_device *hdev, uint8_t us
hdev->rdesc[2] == 0xff;
}
+static const struct dmi_system_id steelseries_msi_rgb_dmi_table[] = {
+ {
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "Micro-Star International Co., Ltd."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "Raider A18 HX A9WJG"),
+ DMI_MATCH(DMI_BOARD_NAME, "MS-182L"),
+ },
+ },
+ { }
+};
+
+static struct usb_interface *steelseries_hid_to_usb_intf(struct hid_device *hdev)
+{
+ if (!hid_is_usb(hdev))
+ return NULL;
+
+ return to_usb_interface(hdev->dev.parent);
+}
+
+static bool steelseries_msi_rgb_is_interface0(struct hid_device *hdev)
+{
+ struct usb_interface *intf = steelseries_hid_to_usb_intf(hdev);
+ struct usb_device *udev;
+
+ if (!intf)
+ return false;
+
+ udev = interface_to_usbdev(intf);
+
+ return intf == usb_ifnum_to_if(udev, 0);
+}
+
+#if STEELSERIES_HAS_LEDS_MULTICOLOR
+
+static struct usb_device *steelseries_hid_to_usb_dev(struct hid_device *hdev)
+{
+ struct usb_interface *intf = steelseries_hid_to_usb_intf(hdev);
+
+ if (!intf)
+ return NULL;
+
+ return interface_to_usbdev(intf);
+}
+
+static int steelseries_msi_rgb_set_blocking(struct led_classdev *led_cdev,
+ enum led_brightness brightness)
+{
+ struct led_classdev_mc *mc_cdev = lcdev_to_mccdev(led_cdev);
+ struct steelseries_device *sd = container_of(mc_cdev,
+ struct steelseries_device,
+ mc_cdev);
+ struct hid_device *hdev = sd->hdev;
+ struct usb_device *udev = steelseries_hid_to_usb_dev(hdev);
+ int i, ret;
+ u8 r, g, b;
+
+ static const u8 keys[] = {
+ 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, 0x0a, 0x0b,
+ 0x0c, 0x0d, 0x0e, 0x0f, 0x10, 0x11, 0x12, 0x13,
+ 0x14, 0x15, 0x16, 0x17, 0x18, 0x19, 0x1a, 0x1b,
+ 0x1c, 0x1d, 0x1e, 0x1f, 0x20, 0x21, 0x22, 0x23,
+ 0x24, 0x25, 0x26, 0x27, 0x28, 0x29, 0x2a, 0x2b,
+ 0x2c, 0x2d, 0x2e, 0x2f, 0x30, 0x31, 0x33, 0x34,
+ 0x35, 0x36, 0x37, 0x38, 0x39, 0x3a, 0x3b, 0x3c,
+ 0x3d, 0x3e, 0x3f, 0x40, 0x41, 0x42, 0x43, 0x44,
+ 0x45, 0x46, 0x47, 0x49, 0x4b, 0x4c, 0x4e, 0x4f,
+ 0x50, 0x51, 0x52, 0x53, 0x54, 0x55, 0x56, 0x57,
+ 0x58, 0x59, 0x5a, 0x5b, 0x5c, 0x5d, 0x5e, 0x5f,
+ 0x60, 0x61, 0x62, 0x63, 0x64, 0x66, 0xe0, 0xe1,
+ 0xe2, 0xe3, 0xe4, 0xe5, 0xe6, 0xf0
+ };
+ static const u8 alc_zones[] = { 0x00, 0x01, 0x02, 0x03, 0x04, 0x05 };
+
+ if (!udev)
+ return -ENODEV;
+
+ mutex_lock(&sd->rgb_lock);
+
+ led_mc_calc_color_components(mc_cdev, brightness);
+
+ r = mc_cdev->subled_info[0].brightness;
+ g = mc_cdev->subled_info[1].brightness;
+ b = mc_cdev->subled_info[2].brightness;
+
+ memset(sd->rgb_buf, 0, STEELSERIES_MSI_RGB_REPORT_LEN);
+ sd->rgb_buf[0] = 0x0c;
+ sd->rgb_buf[1] = 0x00;
+ sd->rgb_buf[3] = 0x00;
+
+ if (hdev->product == USB_DEVICE_ID_STEELSERIES_MSI_KLC) {
+ sd->rgb_buf[2] = 0x66;
+ for (i = 0; i < ARRAY_SIZE(keys); i++) {
+ sd->rgb_buf[4 + i * 4] = keys[i];
+ sd->rgb_buf[5 + i * 4] = r;
+ sd->rgb_buf[6 + i * 4] = g;
+ sd->rgb_buf[7 + i * 4] = b;
+ }
+ } else {
+ sd->rgb_buf[2] = 0x06;
+ for (i = 0; i < ARRAY_SIZE(alc_zones); i++) {
+ sd->rgb_buf[4 + i * 4] = alc_zones[i];
+ sd->rgb_buf[5 + i * 4] = r;
+ sd->rgb_buf[6 + i * 4] = g;
+ sd->rgb_buf[7 + i * 4] = b;
+ }
+ }
+
+ ret = hid_hw_power(hdev, PM_HINT_FULLON);
+ if (ret < 0)
+ goto out_unlock;
+
+ ret = usb_control_msg(udev, usb_sndctrlpipe(udev, 0),
+ HID_REQ_SET_REPORT,
+ USB_DIR_OUT | USB_TYPE_CLASS | USB_RECIP_INTERFACE,
+ 0x0300, 0,
+ sd->rgb_buf, STEELSERIES_MSI_RGB_REPORT_LEN,
+ USB_CTRL_SET_TIMEOUT);
+
+ hid_hw_power(hdev, PM_HINT_NORMAL);
+
+out_unlock:
+ mutex_unlock(&sd->rgb_lock);
+ return ret < 0 ? ret : 0;
+}
+
+static void steelseries_msi_rgb_free_buf(void *data)
+{
+ kfree(data);
+}
+
+static int steelseries_msi_rgb_register(struct steelseries_device *sd)
+{
+ struct hid_device *hdev = sd->hdev;
+ struct led_classdev *led_cdev;
+ int ret;
+
+ sd->rgb_buf = kzalloc(STEELSERIES_MSI_RGB_REPORT_LEN, GFP_KERNEL);
+ if (!sd->rgb_buf)
+ return -ENOMEM;
+
+ ret = devm_add_action_or_reset(&hdev->dev,
+ steelseries_msi_rgb_free_buf,
+ sd->rgb_buf);
+ if (ret) {
+ sd->rgb_buf = NULL;
+ return ret;
+ }
+
+ mutex_init(&sd->rgb_lock);
+
+ sd->subled_info[0].color_index = LED_COLOR_ID_RED;
+ sd->subled_info[1].color_index = LED_COLOR_ID_GREEN;
+ sd->subled_info[2].color_index = LED_COLOR_ID_BLUE;
+ sd->subled_info[0].intensity = 255;
+ sd->subled_info[1].intensity = 255;
+ sd->subled_info[2].intensity = 255;
+ sd->subled_info[0].channel = 0;
+ sd->subled_info[1].channel = 1;
+ sd->subled_info[2].channel = 2;
+
+ sd->mc_cdev.subled_info = sd->subled_info;
+ sd->mc_cdev.num_colors = 3;
+
+ led_cdev = &sd->mc_cdev.led_cdev;
+ if (hdev->product == USB_DEVICE_ID_STEELSERIES_MSI_KLC)
+ led_cdev->name = "steelseries::kbd_backlight";
+ else
+ led_cdev->name = "steelseries::lightbar";
+
+ led_cdev->max_brightness = 255;
+ led_cdev->brightness_set_blocking = steelseries_msi_rgb_set_blocking;
+
+ ret = devm_led_classdev_multicolor_register(&hdev->dev, &sd->mc_cdev);
+ if (ret) {
+ devm_remove_action(&hdev->dev, steelseries_msi_rgb_free_buf,
+ sd->rgb_buf);
+ kfree(sd->rgb_buf);
+ sd->rgb_buf = NULL;
+ return ret;
+ }
+
+ return 0;
+}
+#else
+static int steelseries_msi_rgb_register(struct steelseries_device *sd)
+{
+ return -ENODEV;
+}
+#endif
+
static int steelseries_probe(struct hid_device *hdev, const struct hid_device_id *id)
{
struct steelseries_device *sd;
@@ -549,6 +758,14 @@ static int steelseries_probe(struct hid_device *hdev, const struct hid_device_id
sd->hdev = hdev;
sd->quirks = id->driver_data;
+ if (sd->quirks & STEELSERIES_MSI_RGB) {
+ if (!dmi_check_system(steelseries_msi_rgb_dmi_table) ||
+ !steelseries_msi_rgb_is_interface0(hdev)) {
+ hid_dbg(hdev, "MSI RGB quirk not applicable, using generic HID path\n");
+ sd->quirks &= ~STEELSERIES_MSI_RGB;
+ }
+ }
+
ret = hid_parse(hdev);
if (ret)
return ret;
@@ -565,12 +782,28 @@ static int steelseries_probe(struct hid_device *hdev, const struct hid_device_id
ret = hid_hw_open(hdev);
if (ret)
- return ret;
+ goto err_stop;
+
+ if (sd->quirks & STEELSERIES_MSI_RGB) {
+ ret = steelseries_msi_rgb_register(sd);
+ if (ret) {
+ hid_warn(hdev,
+ "Failed to register MSI RGB LEDs: %d, continuing without RGB support\n",
+ ret);
+ sd->quirks &= ~STEELSERIES_MSI_RGB;
+ }
+ return 0;
+ }
- if (steelseries_headset_battery_register(sd) < 0)
+ if (sd->quirks & (STEELSERIES_ARCTIS_1 | STEELSERIES_ARCTIS_9) &&
+ steelseries_headset_battery_register(sd) < 0)
hid_err(sd->hdev,
"Failed to register battery for headset\n");
+ return 0;
+
+err_stop:
+ hid_hw_stop(hdev);
return ret;
}
@@ -588,12 +821,15 @@ static void steelseries_remove(struct hid_device *hdev)
}
sd = hid_get_drvdata(hdev);
+ if (!sd)
+ return;
spin_lock_irqsave(&sd->lock, flags);
sd->removed = true;
spin_unlock_irqrestore(&sd->lock, flags);
- cancel_delayed_work_sync(&sd->battery_work);
+ if (sd->battery_registered)
+ cancel_delayed_work_sync(&sd->battery_work);
hid_hw_close(hdev);
hid_hw_stop(hdev);
@@ -624,20 +860,33 @@ static uint8_t steelseries_headset_map_capacity(uint8_t capacity, uint8_t min_in
return (capacity - min_in) * 100 / (max_in - min_in);
}
+static bool steelseries_is_headset(struct hid_device *hdev)
+{
+ return hdev->product == USB_DEVICE_ID_STEELSERIES_ARCTIS_1 ||
+ hdev->product == USB_DEVICE_ID_STEELSERIES_ARCTIS_9;
+}
+
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;
+ struct steelseries_device *sd;
+ int capacity;
+ bool connected;
+ bool charging;
unsigned long flags;
- /* Not a headset */
- if (hdev->product == USB_DEVICE_ID_STEELSERIES_SRWS1)
+ if (!steelseries_is_headset(hdev))
+ return 0;
+
+ sd = hid_get_drvdata(hdev);
+ if (!sd || !sd->battery_registered)
return 0;
+ capacity = sd->battery_capacity;
+ connected = sd->headset_connected;
+ charging = sd->battery_charging;
+
if (hdev->product == USB_DEVICE_ID_STEELSERIES_ARCTIS_1) {
hid_dbg(sd->hdev,
"Parsing raw event for Arctis 1 headset (%*ph)\n", size, read_buf);
@@ -732,6 +981,16 @@ static const struct hid_device_id steelseries_devices[] = {
HID_USB_DEVICE(USB_VENDOR_ID_STEELSERIES, USB_DEVICE_ID_STEELSERIES_ARCTIS_9),
.driver_data = STEELSERIES_ARCTIS_9 },
+#if STEELSERIES_HAS_LEDS_MULTICOLOR
+ { /* MSI Raider A18 KLC */
+ HID_USB_DEVICE(USB_VENDOR_ID_STEELSERIES, USB_DEVICE_ID_STEELSERIES_MSI_KLC),
+ .driver_data = STEELSERIES_MSI_RGB },
+
+ { /* MSI Raider A18 ALC */
+ HID_USB_DEVICE(USB_VENDOR_ID_STEELSERIES, USB_DEVICE_ID_STEELSERIES_MSI_ALC),
+ .driver_data = STEELSERIES_MSI_RGB },
+#endif
+
{ }
};
MODULE_DEVICE_TABLE(hid, steelseries_devices);
--
2.54.0
^ permalink raw reply related
* Re: [PATCH v6] HID: steelseries: Add MSI Raider A18 HX A9WJG RGB support
From: sashiko-bot @ 2026-06-01 12:27 UTC (permalink / raw)
To: David Glushkov; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260601121118.8546-1-david.glushkov@sntiq.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Missing memory barrier when publishing `sd->battery_registered`, leading to a potential race condition and kernel panic in `raw_event`.
--
commit cc3464e1bd82d31b73852e983e4e3aef990bd1d8
Author: David Glushkov <david.glushkov@sntiq.com>
HID: steelseries: Add MSI Raider A18 HX A9WJG RGB support
This patch adds DMI-gated support for internal SteelSeries USB HID devices
for RGB lighting on the MSI Raider A18 HX A9WJG laptop. It exposes them as
multicolor LED class devices.
Link: https://lore.kernel.org/oe-kbuild-all/202606010709.X0QYNjFZ-lkp@intel.com/
> diff --git a/drivers/hid/hid-steelseries.c b/drivers/hid/hid-steelseries.c
> --- a/drivers/hid/hid-steelseries.c
> +++ b/drivers/hid/hid-steelseries.c
> @@ -509,6 +528,7 @@ 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);
> + sd->battery_registered = true;
[Severity: High]
Since hid_hw_start() is called earlier in the steelseries_probe() sequence,
could the store to sd->battery_registered be reordered before the
initialization of sd->battery_work by the compiler or on weakly-ordered CPUs?
If this reordering occurs, the interrupt-driven steelseries_headset_raw_event
handler might locklessly observe battery_registered as true prematurely. This
would lead to schedule_delayed_work() being called on a partially initialized
work_struct, which could cause a kernel panic in the timer core.
Would it be safer to publish this flag using smp_store_release() to ensure
the delayed work initialization is fully visible before the flag is set?
> steelseries_headset_fetch_battery(sd->hdev);
>
> if (sd->quirks & STEELSERIES_ARCTIS_9) {
[ ... ]
> @@ -855,6 +885,9 @@ static int steelseries_headset_raw_event(struct hid_device *hdev,
> if (!steelseries_is_headset(hdev))
> return 0;
>
> + sd = hid_get_drvdata(hdev);
> + if (!sd || !sd->battery_registered)
> + return 0;
> +
> capacity = sd->battery_capacity;
[Severity: High]
To safely pair with the suggested smp_store_release() in the initialization
path, should this lockless read of sd->battery_registered be done using
smp_load_acquire()?
This check happens directly in the raw_event handler, so it needs memory
barriers to guarantee that if it reads true, the fully initialized work_struct
is visible to this CPU before we subsequently call schedule_delayed_work() on
it later in this function.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260601121118.8546-1-david.glushkov@sntiq.com?part=1
^ permalink raw reply
* [PATCH v7] HID: steelseries: Add MSI Raider A18 HX A9WJG RGB support
From: David Glushkov @ 2026-06-01 12:37 UTC (permalink / raw)
To: Jiri Kosina
Cc: Benjamin Tissoires, linux-input, linux-kernel, kernel test robot
In-Reply-To: <20260531215204.172030-1-david.glushkov@sntiq.com>
The MSI Raider A18 HX A9WJG exposes two internal SteelSeries USB HID
devices for RGB lighting: KLC (1038:1122) for the keyboard and ALC
(1038:1161) for the lightbar/logo zones.
Add DMI-gated support for these devices and expose them as multicolor
LED class devices. The driver sends the same HID class SET_REPORT
control transfer as the tested userspace implementation for this
machine and writes a uniform RGB value to all known keyboard keys or
ALC zones.
Limit RGB support to USB interface 0 and the tested DMI system because
the KLC product ID is shared across MSI laptop designs and the key
layout mapping is model-specific. If the DMI or interface check does
not match, keep the device bound as a regular HID device instead of
failing probe.
Tested on MSI Raider A18 HX A9WJG. Both internal SteelSeries ALC
(1038:1161) and KLC (1038:1122) HID devices bind on interface 0 and
create steelseries::lightbar and steelseries::kbd_backlight. Setting
multi_intensity and brightness changes the keyboard and lightbar
colors.
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202606010709.X0QYNjFZ-lkp@intel.com/
Signed-off-by: David Glushkov <david.glushkov@sntiq.com>
---
v7:
- Use smp_store_release()/smp_load_acquire() when publishing and reading
battery_registered, so raw_event cannot observe the flag before the
delayed work initialization is visible.
v6:
- Fix W=1 build warning when CONFIG_LEDS_CLASS_MULTICOLOR is disabled by
moving steelseries_hid_to_usb_dev() into the multicolor LED code path.
v5:
- Drop pm_ret handling and ignore PM_HINT_NORMAL cleanup errors.
- Fix LED registration error handling to clean up rgb_buf on failure.
- Fix trailing whitespaces and formatting style nits.
- Update commit message to accurately reflect the DMI fallback behavior.
v4:
- Fix literal \n typo in C code.
- Remove unused steelseries_msi_rgb_free_buf from #else block.
- Do not fail probe on unsupported DMI/interface; clear MSI RGB quirk and continue normal HID initialization.
- Do not fail probe when RGB LED registration fails; keep the HID device usable without RGB LED support.
- Add explicit linux/slab.h include for kzalloc/kfree.
v3:
- Fix build failure (added missing err_close label to steelseries_probe).
- Fix DMA API violation (use kzalloc instead of devm_kzalloc for usb transfer buffer).
- Fix C syntax declaration-after-statement warning.
- Fix type confusion for SRWS1 (add early check in raw_event before hid_get_drvdata).
- Fix delayed_work crash (add battery_registered flag).
v2:
- Fixed unsafe to_usb_interface cast by checking hid_is_usb() first.
- Fixed uninitialized delayed_work warning by restricting cancel_delayed_work_sync to headset devices.
- Fixed error path leaks in probe (hid_hw_stop / hid_hw_close).
- Added hid_hw_power PM wrappers around direct usb_control_msg transfers.
drivers/hid/hid-ids.h | 2 +
drivers/hid/hid-steelseries.c | 280 ++++++++++++++++++++++++++++++++--
2 files changed, 273 insertions(+), 9 deletions(-)
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 4657d96fb..4af4397b8 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -1367,6 +1367,8 @@
#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_MSI_KLC 0x1122
+#define USB_DEVICE_ID_STEELSERIES_MSI_ALC 0x1161
#define USB_VENDOR_ID_SUN 0x0430
#define USB_DEVICE_ID_RARITAN_KVM_DONGLE 0xcdab
diff --git a/drivers/hid/hid-steelseries.c b/drivers/hid/hid-steelseries.c
index f98435631..370ed11a6 100644
--- a/drivers/hid/hid-steelseries.c
+++ b/drivers/hid/hid-steelseries.c
@@ -10,16 +10,26 @@
*/
#include <linux/device.h>
+#include <linux/dmi.h>
#include <linux/hid.h>
#include <linux/module.h>
#include <linux/usb.h>
#include <linux/leds.h>
+#include <linux/led-class-multicolor.h>
+#include <linux/slab.h>
#include "hid-ids.h"
#define STEELSERIES_SRWS1 BIT(0)
#define STEELSERIES_ARCTIS_1 BIT(1)
#define STEELSERIES_ARCTIS_9 BIT(2)
+#define STEELSERIES_MSI_RGB BIT(3)
+
+#define STEELSERIES_MSI_RGB_REPORT_LEN 524
+
+#define STEELSERIES_HAS_LEDS_MULTICOLOR \
+ (IS_BUILTIN(CONFIG_LEDS_CLASS_MULTICOLOR) || \
+ (IS_MODULE(CONFIG_LEDS_CLASS_MULTICOLOR) && IS_MODULE(CONFIG_HID_STEELSERIES)))
struct steelseries_device {
struct hid_device *hdev;
@@ -34,6 +44,14 @@ struct steelseries_device {
uint8_t battery_capacity;
bool headset_connected;
bool battery_charging;
+ bool battery_registered;
+
+#if STEELSERIES_HAS_LEDS_MULTICOLOR
+ struct led_classdev_mc mc_cdev;
+ struct mc_subled subled_info[3];
+ struct mutex rgb_lock; /* protects rgb_buf */
+ u8 *rgb_buf;
+#endif
};
#if IS_BUILTIN(CONFIG_LEDS_CLASS) || \
@@ -510,6 +528,8 @@ 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);
+ /* Pairs with smp_load_acquire() in raw_event and remove */
+ smp_store_release(&sd->battery_registered, true);
steelseries_headset_fetch_battery(sd->hdev);
if (sd->quirks & STEELSERIES_ARCTIS_9) {
@@ -528,6 +548,196 @@ static bool steelseries_is_vendor_usage_page(struct hid_device *hdev, uint8_t us
hdev->rdesc[2] == 0xff;
}
+static const struct dmi_system_id steelseries_msi_rgb_dmi_table[] = {
+ {
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "Micro-Star International Co., Ltd."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "Raider A18 HX A9WJG"),
+ DMI_MATCH(DMI_BOARD_NAME, "MS-182L"),
+ },
+ },
+ { }
+};
+
+static struct usb_interface *steelseries_hid_to_usb_intf(struct hid_device *hdev)
+{
+ if (!hid_is_usb(hdev))
+ return NULL;
+
+ return to_usb_interface(hdev->dev.parent);
+}
+
+static bool steelseries_msi_rgb_is_interface0(struct hid_device *hdev)
+{
+ struct usb_interface *intf = steelseries_hid_to_usb_intf(hdev);
+ struct usb_device *udev;
+
+ if (!intf)
+ return false;
+
+ udev = interface_to_usbdev(intf);
+
+ return intf == usb_ifnum_to_if(udev, 0);
+}
+
+#if STEELSERIES_HAS_LEDS_MULTICOLOR
+
+static struct usb_device *steelseries_hid_to_usb_dev(struct hid_device *hdev)
+{
+ struct usb_interface *intf = steelseries_hid_to_usb_intf(hdev);
+
+ if (!intf)
+ return NULL;
+
+ return interface_to_usbdev(intf);
+}
+
+static int steelseries_msi_rgb_set_blocking(struct led_classdev *led_cdev,
+ enum led_brightness brightness)
+{
+ struct led_classdev_mc *mc_cdev = lcdev_to_mccdev(led_cdev);
+ struct steelseries_device *sd = container_of(mc_cdev,
+ struct steelseries_device,
+ mc_cdev);
+ struct hid_device *hdev = sd->hdev;
+ struct usb_device *udev = steelseries_hid_to_usb_dev(hdev);
+ int i, ret;
+ u8 r, g, b;
+
+ static const u8 keys[] = {
+ 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, 0x0a, 0x0b,
+ 0x0c, 0x0d, 0x0e, 0x0f, 0x10, 0x11, 0x12, 0x13,
+ 0x14, 0x15, 0x16, 0x17, 0x18, 0x19, 0x1a, 0x1b,
+ 0x1c, 0x1d, 0x1e, 0x1f, 0x20, 0x21, 0x22, 0x23,
+ 0x24, 0x25, 0x26, 0x27, 0x28, 0x29, 0x2a, 0x2b,
+ 0x2c, 0x2d, 0x2e, 0x2f, 0x30, 0x31, 0x33, 0x34,
+ 0x35, 0x36, 0x37, 0x38, 0x39, 0x3a, 0x3b, 0x3c,
+ 0x3d, 0x3e, 0x3f, 0x40, 0x41, 0x42, 0x43, 0x44,
+ 0x45, 0x46, 0x47, 0x49, 0x4b, 0x4c, 0x4e, 0x4f,
+ 0x50, 0x51, 0x52, 0x53, 0x54, 0x55, 0x56, 0x57,
+ 0x58, 0x59, 0x5a, 0x5b, 0x5c, 0x5d, 0x5e, 0x5f,
+ 0x60, 0x61, 0x62, 0x63, 0x64, 0x66, 0xe0, 0xe1,
+ 0xe2, 0xe3, 0xe4, 0xe5, 0xe6, 0xf0
+ };
+ static const u8 alc_zones[] = { 0x00, 0x01, 0x02, 0x03, 0x04, 0x05 };
+
+ if (!udev)
+ return -ENODEV;
+
+ mutex_lock(&sd->rgb_lock);
+
+ led_mc_calc_color_components(mc_cdev, brightness);
+
+ r = mc_cdev->subled_info[0].brightness;
+ g = mc_cdev->subled_info[1].brightness;
+ b = mc_cdev->subled_info[2].brightness;
+
+ memset(sd->rgb_buf, 0, STEELSERIES_MSI_RGB_REPORT_LEN);
+ sd->rgb_buf[0] = 0x0c;
+ sd->rgb_buf[1] = 0x00;
+ sd->rgb_buf[3] = 0x00;
+
+ if (hdev->product == USB_DEVICE_ID_STEELSERIES_MSI_KLC) {
+ sd->rgb_buf[2] = 0x66;
+ for (i = 0; i < ARRAY_SIZE(keys); i++) {
+ sd->rgb_buf[4 + i * 4] = keys[i];
+ sd->rgb_buf[5 + i * 4] = r;
+ sd->rgb_buf[6 + i * 4] = g;
+ sd->rgb_buf[7 + i * 4] = b;
+ }
+ } else {
+ sd->rgb_buf[2] = 0x06;
+ for (i = 0; i < ARRAY_SIZE(alc_zones); i++) {
+ sd->rgb_buf[4 + i * 4] = alc_zones[i];
+ sd->rgb_buf[5 + i * 4] = r;
+ sd->rgb_buf[6 + i * 4] = g;
+ sd->rgb_buf[7 + i * 4] = b;
+ }
+ }
+
+ ret = hid_hw_power(hdev, PM_HINT_FULLON);
+ if (ret < 0)
+ goto out_unlock;
+
+ ret = usb_control_msg(udev, usb_sndctrlpipe(udev, 0),
+ HID_REQ_SET_REPORT,
+ USB_DIR_OUT | USB_TYPE_CLASS | USB_RECIP_INTERFACE,
+ 0x0300, 0,
+ sd->rgb_buf, STEELSERIES_MSI_RGB_REPORT_LEN,
+ USB_CTRL_SET_TIMEOUT);
+
+ hid_hw_power(hdev, PM_HINT_NORMAL);
+
+out_unlock:
+ mutex_unlock(&sd->rgb_lock);
+ return ret < 0 ? ret : 0;
+}
+
+static void steelseries_msi_rgb_free_buf(void *data)
+{
+ kfree(data);
+}
+
+static int steelseries_msi_rgb_register(struct steelseries_device *sd)
+{
+ struct hid_device *hdev = sd->hdev;
+ struct led_classdev *led_cdev;
+ int ret;
+
+ sd->rgb_buf = kzalloc(STEELSERIES_MSI_RGB_REPORT_LEN, GFP_KERNEL);
+ if (!sd->rgb_buf)
+ return -ENOMEM;
+
+ ret = devm_add_action_or_reset(&hdev->dev,
+ steelseries_msi_rgb_free_buf,
+ sd->rgb_buf);
+ if (ret) {
+ sd->rgb_buf = NULL;
+ return ret;
+ }
+
+ mutex_init(&sd->rgb_lock);
+
+ sd->subled_info[0].color_index = LED_COLOR_ID_RED;
+ sd->subled_info[1].color_index = LED_COLOR_ID_GREEN;
+ sd->subled_info[2].color_index = LED_COLOR_ID_BLUE;
+ sd->subled_info[0].intensity = 255;
+ sd->subled_info[1].intensity = 255;
+ sd->subled_info[2].intensity = 255;
+ sd->subled_info[0].channel = 0;
+ sd->subled_info[1].channel = 1;
+ sd->subled_info[2].channel = 2;
+
+ sd->mc_cdev.subled_info = sd->subled_info;
+ sd->mc_cdev.num_colors = 3;
+
+ led_cdev = &sd->mc_cdev.led_cdev;
+ if (hdev->product == USB_DEVICE_ID_STEELSERIES_MSI_KLC)
+ led_cdev->name = "steelseries::kbd_backlight";
+ else
+ led_cdev->name = "steelseries::lightbar";
+
+ led_cdev->max_brightness = 255;
+ led_cdev->brightness_set_blocking = steelseries_msi_rgb_set_blocking;
+
+ ret = devm_led_classdev_multicolor_register(&hdev->dev, &sd->mc_cdev);
+ if (ret) {
+ devm_remove_action(&hdev->dev, steelseries_msi_rgb_free_buf,
+ sd->rgb_buf);
+ kfree(sd->rgb_buf);
+ sd->rgb_buf = NULL;
+ return ret;
+ }
+
+ return 0;
+}
+#else
+static int steelseries_msi_rgb_register(struct steelseries_device *sd)
+{
+ return -ENODEV;
+}
+#endif
+
static int steelseries_probe(struct hid_device *hdev, const struct hid_device_id *id)
{
struct steelseries_device *sd;
@@ -549,6 +759,14 @@ static int steelseries_probe(struct hid_device *hdev, const struct hid_device_id
sd->hdev = hdev;
sd->quirks = id->driver_data;
+ if (sd->quirks & STEELSERIES_MSI_RGB) {
+ if (!dmi_check_system(steelseries_msi_rgb_dmi_table) ||
+ !steelseries_msi_rgb_is_interface0(hdev)) {
+ hid_dbg(hdev, "MSI RGB quirk not applicable, using generic HID path\n");
+ sd->quirks &= ~STEELSERIES_MSI_RGB;
+ }
+ }
+
ret = hid_parse(hdev);
if (ret)
return ret;
@@ -565,12 +783,28 @@ static int steelseries_probe(struct hid_device *hdev, const struct hid_device_id
ret = hid_hw_open(hdev);
if (ret)
- return ret;
+ goto err_stop;
+
+ if (sd->quirks & STEELSERIES_MSI_RGB) {
+ ret = steelseries_msi_rgb_register(sd);
+ if (ret) {
+ hid_warn(hdev,
+ "Failed to register MSI RGB LEDs: %d, continuing without RGB support\n",
+ ret);
+ sd->quirks &= ~STEELSERIES_MSI_RGB;
+ }
+ return 0;
+ }
- if (steelseries_headset_battery_register(sd) < 0)
+ if (sd->quirks & (STEELSERIES_ARCTIS_1 | STEELSERIES_ARCTIS_9) &&
+ steelseries_headset_battery_register(sd) < 0)
hid_err(sd->hdev,
"Failed to register battery for headset\n");
+ return 0;
+
+err_stop:
+ hid_hw_stop(hdev);
return ret;
}
@@ -588,12 +822,16 @@ static void steelseries_remove(struct hid_device *hdev)
}
sd = hid_get_drvdata(hdev);
+ if (!sd)
+ return;
spin_lock_irqsave(&sd->lock, flags);
sd->removed = true;
spin_unlock_irqrestore(&sd->lock, flags);
- cancel_delayed_work_sync(&sd->battery_work);
+ /* Pairs with smp_store_release() in battery_register */
+ if (smp_load_acquire(&sd->battery_registered))
+ cancel_delayed_work_sync(&sd->battery_work);
hid_hw_close(hdev);
hid_hw_stop(hdev);
@@ -624,20 +862,34 @@ static uint8_t steelseries_headset_map_capacity(uint8_t capacity, uint8_t min_in
return (capacity - min_in) * 100 / (max_in - min_in);
}
+static bool steelseries_is_headset(struct hid_device *hdev)
+{
+ return hdev->product == USB_DEVICE_ID_STEELSERIES_ARCTIS_1 ||
+ hdev->product == USB_DEVICE_ID_STEELSERIES_ARCTIS_9;
+}
+
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;
+ struct steelseries_device *sd;
+ int capacity;
+ bool connected;
+ bool charging;
unsigned long flags;
- /* Not a headset */
- if (hdev->product == USB_DEVICE_ID_STEELSERIES_SRWS1)
+ if (!steelseries_is_headset(hdev))
+ return 0;
+
+ sd = hid_get_drvdata(hdev);
+ /* Pairs with smp_store_release() in battery_register */
+ if (!sd || !smp_load_acquire(&sd->battery_registered))
return 0;
+ capacity = sd->battery_capacity;
+ connected = sd->headset_connected;
+ charging = sd->battery_charging;
+
if (hdev->product == USB_DEVICE_ID_STEELSERIES_ARCTIS_1) {
hid_dbg(sd->hdev,
"Parsing raw event for Arctis 1 headset (%*ph)\n", size, read_buf);
@@ -732,6 +984,16 @@ static const struct hid_device_id steelseries_devices[] = {
HID_USB_DEVICE(USB_VENDOR_ID_STEELSERIES, USB_DEVICE_ID_STEELSERIES_ARCTIS_9),
.driver_data = STEELSERIES_ARCTIS_9 },
+#if STEELSERIES_HAS_LEDS_MULTICOLOR
+ { /* MSI Raider A18 KLC */
+ HID_USB_DEVICE(USB_VENDOR_ID_STEELSERIES, USB_DEVICE_ID_STEELSERIES_MSI_KLC),
+ .driver_data = STEELSERIES_MSI_RGB },
+
+ { /* MSI Raider A18 ALC */
+ HID_USB_DEVICE(USB_VENDOR_ID_STEELSERIES, USB_DEVICE_ID_STEELSERIES_MSI_ALC),
+ .driver_data = STEELSERIES_MSI_RGB },
+#endif
+
{ }
};
MODULE_DEVICE_TABLE(hid, steelseries_devices);
--
2.54.0
^ permalink raw reply related
* Re: [PATCH v2 0/4] MediaTek MT6735+MT6328 SoC/PMIC pair base support
From: Mark Brown @ 2026-06-01 13:16 UTC (permalink / raw)
To: y.oudjana
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Matthias Brugger,
AngeloGioacchino Del Regno, Flora Fu, Alexandre Mergnat,
Liam Girdwood, Dmitry Torokhov, devicetree, linux-kernel,
linux-arm-kernel, linux-mediatek, linux-input
In-Reply-To: <20260531-mt6735-pwrap-mt6328-pmic-v2-0-dac37bf92894@protonmail.com>
[-- Attachment #1: Type: text/plain, Size: 484 bytes --]
On Sun, May 31, 2026 at 11:10:41AM +0200, Yassine Oudjana via B4 Relay wrote:
> These patches are part of a larger effort to support the MT6735 SoC family in
> mainline Linux. More patches (unsent or sent and pending review or revision)
> can be found here[1].
Please send patches for unrelated subsystems as separate serieses to
each subsystem unless there's a depdendency, making everything into a
cross subsystem series just makes it harder to work out what's supposed
to happen.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* [PATCH v2 0/2] HID: wacom: fix sleeping in atomic context in wacom_wac_queue_flush()
From: Jinmo Yang @ 2026-06-01 13:41 UTC (permalink / raw)
To: linux-input, dmitry.torokhov
Cc: jikos, benjamin.tissoires, stable, jinmo44.yang
In-Reply-To: <ahu2oxLwkgMlwXu7@google.com>
wacom_wac_queue_flush() uses GFP_KERNEL for kzalloc, but it can be
called from atomic context via the .raw_event callback path. Patch 1
fixes this by switching to GFP_ATOMIC, and patch 2 converts the buffer
management to use __free(kfree) cleanup as suggested by Dmitry.
Changes since v1:
- Replaced Suggested-by with Reported-by for Sashiko-bot
- Added patch 2 to use __free(kfree) cleanup facility (Dmitry)
Jinmo Yang (2):
HID: wacom: use GFP_ATOMIC in wacom_wac_queue_flush()
HID: wacom: use cleanup.h for wacom_wac_queue_flush() buffer
management
drivers/hid/wacom_sys.c | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
--
2.53.0
^ permalink raw reply
* [PATCH v2 1/2] HID: wacom: use GFP_ATOMIC in wacom_wac_queue_flush()
From: Jinmo Yang @ 2026-06-01 13:41 UTC (permalink / raw)
To: linux-input, dmitry.torokhov
Cc: jikos, benjamin.tissoires, stable, jinmo44.yang, Sashiko-bot
In-Reply-To: <20260601134124.1560886-1-jinmo44.yang@gmail.com>
wacom_wac_queue_flush() is called via the .raw_event callback
(wacom_raw_event → wacom_wac_pen_serial_enforce → wacom_wac_queue_flush).
For USB HID devices, this callback is invoked from hid_irq_in(), which
is a URB completion handler running in atomic context. Using GFP_KERNEL
in this path can sleep, leading to a "scheduling while atomic" bug.
Use GFP_ATOMIC instead. The existing code already handles allocation
failure by skipping the fifo entry and continuing.
Reported-by: Sashiko-bot <sashiko-bot@kernel.org>
Fixes: 5e013ad20689 ("HID: wacom: Remove static WACOM_PKGLEN_MAX limit")
Cc: stable@vger.kernel.org
Reviewed-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Signed-off-by: Jinmo Yang <jinmo44.yang@gmail.com>
---
drivers/hid/wacom_sys.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index a32320b35..2e237bdd2 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -74,7 +74,7 @@ static void wacom_wac_queue_flush(struct hid_device *hdev,
unsigned int count;
int err;
- buf = kzalloc(size, GFP_KERNEL);
+ buf = kzalloc(size, GFP_ATOMIC);
if (!buf) {
kfifo_skip(fifo);
continue;
--
2.53.0
^ permalink raw reply related
* [PATCH v2 2/2] HID: wacom: use cleanup.h for wacom_wac_queue_flush() buffer management
From: Jinmo Yang @ 2026-06-01 13:41 UTC (permalink / raw)
To: linux-input, dmitry.torokhov
Cc: jikos, benjamin.tissoires, stable, jinmo44.yang
In-Reply-To: <20260601134124.1560886-1-jinmo44.yang@gmail.com>
Use __free(kfree) cleanup facility for the temporary buffer in
wacom_wac_queue_flush() to simplify error paths and ensure the buffer
is freed automatically when it goes out of scope.
Signed-off-by: Jinmo Yang <jinmo44.yang@gmail.com>
---
drivers/hid/wacom_sys.c | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index 2e237bdd2..edc24fe2e 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -70,11 +70,10 @@ static void wacom_wac_queue_flush(struct hid_device *hdev,
{
while (!kfifo_is_empty(fifo)) {
int size = kfifo_peek_len(fifo);
- u8 *buf;
+ u8 *buf __free(kfree) = kzalloc(size, GFP_ATOMIC);
unsigned int count;
int err;
- buf = kzalloc(size, GFP_ATOMIC);
if (!buf) {
kfifo_skip(fifo);
continue;
@@ -87,7 +86,6 @@ static void wacom_wac_queue_flush(struct hid_device *hdev,
// to flush seems reasonable enough, however.
hid_warn(hdev, "%s: removed fifo entry with unexpected size\n",
__func__);
- kfree(buf);
continue;
}
err = hid_report_raw_event(hdev, HID_INPUT_REPORT, buf, size, size, false);
@@ -95,8 +93,6 @@ static void wacom_wac_queue_flush(struct hid_device *hdev,
hid_warn(hdev, "%s: unable to flush event due to error %d\n",
__func__, err);
}
-
- kfree(buf);
}
}
--
2.53.0
^ permalink raw reply related
* [PATCH v2 1/2] HID: wacom: Fix Use-After-Free in wacom_intuos_pad
From: Lee Jones @ 2026-06-01 14:08 UTC (permalink / raw)
To: lee, Ping Cheng, Jason Gerecke, Jiri Kosina, Benjamin Tissoires,
Dmitry Torokhov, Peter Hutterer, linux-input, linux-kernel
wacom_intuos_pad() accesses wacom->shared->touch_input locklessly inside the
interrupt handler context. If the Touch sibling device is disconnected,
wacom_remove_shared_data() clears 'touch_input' outside any lock, creating a
Time-of-Check to Time-of-Use (TOCTOU) race condition where a preempted reader in
interrupt context dereferences the freed pointer, leading to a Use-After-Free.
Resolve this by introducing RCU protection for the touch_input pointer:
- Annotate 'touch_input' in wacom_shared struct with __rcu
- Wrap lockless readers in wacom_wac.c with rcu_read_lock() and rcu_dereference()
- Update writers in wacom_sys.c using rcu_assign_pointer()
- Call synchronize_rcu() in wacom_remove_shared_data() to ensure all active RCU
readers have finished before the input device is freed
To ensure the devres release order is correct and wacom_remove_shared_data()
completes its RCU synchronization BEFORE the input device is freed, move the
wacom_add_shared_data() call to the very end of wacom_parse_and_register().
Since devres actions are executed in LIFO order, this guarantees the shared
reference is cleared and synchronized before the input device resource is destroyed.
Also wrap wacom_set_shared_values() and touch/pen assignments in
wacom_add_shared_data() inside the wacom_udev_list_lock to serialize concurrent
probe assignments, and verify that 'shared->touch == hdev' before setting
touch_input to prevent concurrent sibling probe state desynchronization.
Fixes: 961794a00eab ("Input: wacom - add reporting of SW_MUTE_DEVICE events")
Signed-off-by: Lee Jones <lee@kernel.org>
---
v1 -> v2: Split and use RCU as per Dmitry's review
drivers/hid/wacom_sys.c | 49 +++++++++++++++++++++++++++--------------
drivers/hid/wacom_wac.c | 17 +++++++++-----
drivers/hid/wacom_wac.h | 2 +-
3 files changed, 45 insertions(+), 23 deletions(-)
diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index 2220168bf116..26fce6d3198a 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -877,10 +877,16 @@ static void wacom_remove_shared_data(void *res)
data = container_of(wacom_wac->shared, struct wacom_hdev_data,
shared);
- if (wacom_wac->shared->touch == wacom->hdev)
+ mutex_lock(&wacom_udev_list_lock);
+ if (wacom_wac->shared->touch == wacom->hdev) {
wacom_wac->shared->touch = NULL;
- else if (wacom_wac->shared->pen == wacom->hdev)
+ rcu_assign_pointer(wacom_wac->shared->touch_input, NULL);
+ } else if (wacom_wac->shared->pen == wacom->hdev) {
wacom_wac->shared->pen = NULL;
+ }
+ mutex_unlock(&wacom_udev_list_lock);
+
+ synchronize_rcu();
kref_put(&data->kref, wacom_release_shared_data);
wacom_wac->shared = NULL;
@@ -909,6 +915,11 @@ static int wacom_add_shared_data(struct hid_device *hdev)
list_add_tail(&data->list, &wacom_udev_list);
}
+ if (wacom_wac->features.device_type & WACOM_DEVICETYPE_TOUCH)
+ data->shared.touch = hdev;
+ else if (wacom_wac->features.device_type & WACOM_DEVICETYPE_PEN)
+ data->shared.pen = hdev;
+
mutex_unlock(&wacom_udev_list_lock);
wacom_wac->shared = &data->shared;
@@ -917,11 +928,6 @@ static int wacom_add_shared_data(struct hid_device *hdev)
if (retval)
return retval;
- if (wacom_wac->features.device_type & WACOM_DEVICETYPE_TOUCH)
- wacom_wac->shared->touch = hdev;
- else if (wacom_wac->features.device_type & WACOM_DEVICETYPE_PEN)
- wacom_wac->shared->pen = hdev;
-
return retval;
}
@@ -2345,9 +2351,15 @@ static void wacom_release_resources(struct wacom *wacom)
static void wacom_set_shared_values(struct wacom_wac *wacom_wac)
{
+ struct wacom *wacom = container_of(wacom_wac, struct wacom, wacom_wac);
+
+ mutex_lock(&wacom_udev_list_lock);
+
if (wacom_wac->features.device_type & WACOM_DEVICETYPE_TOUCH) {
- wacom_wac->shared->type = wacom_wac->features.type;
- wacom_wac->shared->touch_input = wacom_wac->touch_input;
+ if (wacom_wac->shared->touch == wacom->hdev) {
+ wacom_wac->shared->type = wacom_wac->features.type;
+ rcu_assign_pointer(wacom_wac->shared->touch_input, wacom_wac->touch_input);
+ }
}
if (wacom_wac->has_mute_touch_switch) {
@@ -2362,11 +2374,14 @@ static void wacom_set_shared_values(struct wacom_wac *wacom_wac)
}
if (wacom_wac->shared->has_mute_touch_switch &&
- wacom_wac->shared->touch_input) {
- set_bit(EV_SW, wacom_wac->shared->touch_input->evbit);
- input_set_capability(wacom_wac->shared->touch_input, EV_SW,
- SW_MUTE_DEVICE);
+ rcu_access_pointer(wacom_wac->shared->touch_input)) {
+ struct input_dev *touch_input = rcu_dereference_protected(wacom_wac->shared->touch_input,
+ lockdep_is_held(&wacom_udev_list_lock));
+ set_bit(EV_SW, touch_input->evbit);
+ input_set_capability(touch_input, EV_SW, SW_MUTE_DEVICE);
}
+
+ mutex_unlock(&wacom_udev_list_lock);
}
static int wacom_parse_and_register(struct wacom *wacom, bool wireless)
@@ -2442,10 +2457,6 @@ static int wacom_parse_and_register(struct wacom *wacom, bool wireless)
goto fail;
}
- error = wacom_add_shared_data(hdev);
- if (error)
- goto fail;
-
error = wacom_setup_inputs(wacom);
if (error)
goto fail;
@@ -2496,6 +2507,10 @@ static int wacom_parse_and_register(struct wacom *wacom, bool wireless)
}
}
+ error = wacom_add_shared_data(hdev);
+ if (error)
+ goto fail_quirks;
+
wacom_set_shared_values(wacom_wac);
devres_close_group(&hdev->dev, wacom);
diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
index da1f0ea85625..c7cf1aaab903 100644
--- a/drivers/hid/wacom_wac.c
+++ b/drivers/hid/wacom_wac.c
@@ -650,11 +650,18 @@ static int wacom_intuos_pad(struct wacom_wac *wacom)
input_report_key(input, KEY_CONTROLPANEL, menu);
input_report_key(input, KEY_INFO, info);
- if (wacom->shared && wacom->shared->touch_input) {
- input_report_switch(wacom->shared->touch_input,
- SW_MUTE_DEVICE,
- !wacom->shared->is_touch_on);
- input_sync(wacom->shared->touch_input);
+ if (wacom->shared) {
+ struct input_dev *touch_input;
+
+ rcu_read_lock();
+ touch_input = rcu_dereference(wacom->shared->touch_input);
+ if (touch_input) {
+ input_report_switch(touch_input,
+ SW_MUTE_DEVICE,
+ !wacom->shared->is_touch_on);
+ input_sync(touch_input);
+ }
+ rcu_read_unlock();
}
input_report_abs(input, ABS_RX, strip1);
diff --git a/drivers/hid/wacom_wac.h b/drivers/hid/wacom_wac.h
index 126bec6e5c0c..a8bbba4a6f37 100644
--- a/drivers/hid/wacom_wac.h
+++ b/drivers/hid/wacom_wac.h
@@ -285,7 +285,7 @@ struct wacom_shared {
/* for wireless device to access USB interfaces */
unsigned touch_max;
int type;
- struct input_dev *touch_input;
+ struct input_dev __rcu *touch_input;
struct hid_device *pen;
struct hid_device *touch;
bool has_mute_touch_switch;
--
2.54.0.823.g6e5bcc1fc9-goog
^ permalink raw reply related
* [PATCH v2 2/2] HID: wacom: Redesign shared sibling data lifecycle
From: Lee Jones @ 2026-06-01 14:08 UTC (permalink / raw)
To: lee, Ping Cheng, Jason Gerecke, Jiri Kosina, Benjamin Tissoires,
Dmitry Torokhov, Peter Hutterer, linux-input, linux-kernel
In-Reply-To: <20260601140845.1237227-1-lee@kernel.org>
The Wacom driver coordinates state between sibling interfaces of the same
physical device using a shared structure 'wacom_shared' inside
'wacom_hdev_data'. The driver kept a volatile representative pointer
'data->dev' pointing to a sibling 'hid_device' for physical path comparisons
during sibling matching.
This pointer management is fragile. When the representative device is
disconnected, wacom_remove_shared_data() failed to clear/update
'data->dev', leading to a Use-After-Free vulnerability when subsequent
sibling probes dereference the dangling 'data->dev' pointer.
Resolve this issue by redesigning the sibling data lifecycle:
- Eliminate the volatile 'data->dev' representative pointer completely
- Redesign 'wacom_hdev_data' to store stable static copies of the required
attributes upon first allocation: 'phys' path string, 'vendor', 'product
IDs and the sibling's 'device_type'
- Use these static attributes for stable sibling matching in
wacom_are_sibling() and wacom_get_hdev_data()
This ensures sibling matching remains safe and stable even if individual
siblings are dynamically added or removed.
To secure the lifecycle against concurrent probe/disconnect races:
- Switch kref_put() to kref_put_mutex() in wacom_remove_shared_data() to
serialize refcount drops with list traversal and lookup
- Modify wacom_release_shared_data() to assume the list lock is already held
Also, do not accumulate the 'device_type' capability flag during subsequent
sibling probes. Keeping only the first probed sibling's device_type exactly
preserves the original sibling matching behavior without introducing side
effects.
Fixes: 4492efffffeb ("Input: wacom - share pen info with touch of the same ID")
Signed-off-by: Lee Jones <lee@kernel.org>
---
v1 -> v2: Split and use RCU as per Dmitry's review
drivers/hid/wacom_sys.c | 63 +++++++++++++++++++++++++----------------
1 file changed, 39 insertions(+), 24 deletions(-)
diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index 26fce6d3198a..7cd0b0ff3128 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -759,27 +759,40 @@ static void wacom_retrieve_hid_descriptor(struct hid_device *hdev,
struct wacom_hdev_data {
struct list_head list;
struct kref kref;
- struct hid_device *dev;
+ char phys[64];
+ __u32 vendor;
+ __u32 product;
+ __u32 device_type;
struct wacom_shared shared;
};
+static bool wacom_compare_device_paths(struct hid_device *hdev_a,
+ const char *phys_b, char separator)
+{
+ int n1 = strrchr(hdev_a->phys, separator) - hdev_a->phys;
+ int n2 = strrchr(phys_b, separator) - phys_b;
+
+ if (n1 != n2 || n1 <= 0 || n2 <= 0)
+ return false;
+
+ return !strncmp(hdev_a->phys, phys_b, n1);
+}
+
static LIST_HEAD(wacom_udev_list);
static DEFINE_MUTEX(wacom_udev_list_lock);
static bool wacom_are_sibling(struct hid_device *hdev,
- struct hid_device *sibling)
+ struct wacom_hdev_data *data)
{
struct wacom *wacom = hid_get_drvdata(hdev);
struct wacom_features *features = &wacom->wacom_wac.features;
- struct wacom *sibling_wacom = hid_get_drvdata(sibling);
- struct wacom_features *sibling_features = &sibling_wacom->wacom_wac.features;
__u32 oVid = features->oVid ? features->oVid : hdev->vendor;
__u32 oPid = features->oPid ? features->oPid : hdev->product;
/* The defined oVid/oPid must match that of the sibling */
- if (features->oVid != HID_ANY_ID && sibling->vendor != oVid)
+ if (features->oVid != HID_ANY_ID && data->vendor != oVid)
return false;
- if (features->oPid != HID_ANY_ID && sibling->product != oPid)
+ if (features->oPid != HID_ANY_ID && data->product != oPid)
return false;
/*
@@ -787,11 +800,11 @@ static bool wacom_are_sibling(struct hid_device *hdev,
* device path, while those with different VID/PID must share
* the same physical parent device path.
*/
- if (hdev->vendor == sibling->vendor && hdev->product == sibling->product) {
- if (!hid_compare_device_paths(hdev, sibling, '/'))
+ if (hdev->vendor == data->vendor && hdev->product == data->product) {
+ if (!wacom_compare_device_paths(hdev, data->phys, '/'))
return false;
} else {
- if (!hid_compare_device_paths(hdev, sibling, '.'))
+ if (!wacom_compare_device_paths(hdev, data->phys, '.'))
return false;
}
@@ -804,7 +817,7 @@ static bool wacom_are_sibling(struct hid_device *hdev,
* devices.
*/
if ((features->device_type & WACOM_DEVICETYPE_DIRECT) &&
- !(sibling_features->device_type & WACOM_DEVICETYPE_DIRECT))
+ !(data->device_type & WACOM_DEVICETYPE_DIRECT))
return false;
/*
@@ -812,17 +825,17 @@ static bool wacom_are_sibling(struct hid_device *hdev,
* devices.
*/
if (!(features->device_type & WACOM_DEVICETYPE_DIRECT) &&
- (sibling_features->device_type & WACOM_DEVICETYPE_DIRECT))
+ (data->device_type & WACOM_DEVICETYPE_DIRECT))
return false;
/* Pen devices may only be siblings of touch devices */
if ((features->device_type & WACOM_DEVICETYPE_PEN) &&
- !(sibling_features->device_type & WACOM_DEVICETYPE_TOUCH))
+ !(data->device_type & WACOM_DEVICETYPE_TOUCH))
return false;
/* Touch devices may only be siblings of pen devices */
if ((features->device_type & WACOM_DEVICETYPE_TOUCH) &&
- !(sibling_features->device_type & WACOM_DEVICETYPE_PEN))
+ !(data->device_type & WACOM_DEVICETYPE_PEN))
return false;
/*
@@ -838,7 +851,7 @@ static struct wacom_hdev_data *wacom_get_hdev_data(struct hid_device *hdev)
/* Try to find an already-probed interface from the same device */
list_for_each_entry(data, &wacom_udev_list, list) {
- if (hid_compare_device_paths(hdev, data->dev, '/')) {
+ if (wacom_compare_device_paths(hdev, data->phys, '/')) {
kref_get(&data->kref);
return data;
}
@@ -846,7 +859,7 @@ static struct wacom_hdev_data *wacom_get_hdev_data(struct hid_device *hdev)
/* Fallback to finding devices that appear to be "siblings" */
list_for_each_entry(data, &wacom_udev_list, list) {
- if (wacom_are_sibling(hdev, data->dev)) {
+ if (wacom_are_sibling(hdev, data)) {
kref_get(&data->kref);
return data;
}
@@ -860,35 +873,34 @@ static void wacom_release_shared_data(struct kref *kref)
struct wacom_hdev_data *data =
container_of(kref, struct wacom_hdev_data, kref);
- mutex_lock(&wacom_udev_list_lock);
list_del(&data->list);
- mutex_unlock(&wacom_udev_list_lock);
-
kfree(data);
}
static void wacom_remove_shared_data(void *res)
{
- struct wacom *wacom = res;
+ struct wacom *res_wacom = res;
struct wacom_hdev_data *data;
- struct wacom_wac *wacom_wac = &wacom->wacom_wac;
+ struct wacom_wac *wacom_wac = &res_wacom->wacom_wac;
if (wacom_wac->shared) {
data = container_of(wacom_wac->shared, struct wacom_hdev_data,
shared);
mutex_lock(&wacom_udev_list_lock);
- if (wacom_wac->shared->touch == wacom->hdev) {
+ if (wacom_wac->shared->touch == res_wacom->hdev) {
wacom_wac->shared->touch = NULL;
rcu_assign_pointer(wacom_wac->shared->touch_input, NULL);
- } else if (wacom_wac->shared->pen == wacom->hdev) {
+ } else if (wacom_wac->shared->pen == res_wacom->hdev) {
wacom_wac->shared->pen = NULL;
}
mutex_unlock(&wacom_udev_list_lock);
synchronize_rcu();
- kref_put(&data->kref, wacom_release_shared_data);
+ if (kref_put_mutex(&data->kref, wacom_release_shared_data, &wacom_udev_list_lock))
+ mutex_unlock(&wacom_udev_list_lock);
+
wacom_wac->shared = NULL;
}
}
@@ -911,7 +923,10 @@ static int wacom_add_shared_data(struct hid_device *hdev)
}
kref_init(&data->kref);
- data->dev = hdev;
+ strncpy(data->phys, hdev->phys, sizeof(data->phys) - 1);
+ data->vendor = hdev->vendor;
+ data->product = hdev->product;
+ data->device_type = wacom_wac->features.device_type;
list_add_tail(&data->list, &wacom_udev_list);
}
--
2.54.0.823.g6e5bcc1fc9-goog
^ permalink raw reply related
* Re: [PATCH v2 1/2] HID: wacom: Fix Use-After-Free in wacom_intuos_pad
From: sashiko-bot @ 2026-06-01 14:26 UTC (permalink / raw)
To: Lee Jones; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260601140845.1237227-1-lee@kernel.org>
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
New issues:
- [Critical] Moving wacom_add_shared_data() to the end of wacom_parse_and_register() causes an unconditional NULL pointer dereference during device probe.
- [High] Moving wacom_add_shared_data() to the end of the probe function creates a window where wacom_wac->shared is NULL while hardware interrupts are active, leading to an asynchronous NULL pointer dereference.
Pre-existing issues:
- [High] The patch incompletely fixes a Time-of-Check to Time-of-Use (TOCTOU) race condition. It successfully converts wacom_intuos_pad() to use RCU, but leaves identical lockless accesses unprotected in wacom_wac_pad_event(), wacom_wireless_irq(), and wacom_status_irq().
- [High] Modifying input capabilities on the sibling touch device after it has already been registered with the input core.
--
commit 5bec6e8c12bc84f46d120679cc4bef944e215dc2
Author: Lee Jones <lee@kernel.org>
HID: wacom: Fix Use-After-Free in wacom_intuos_pad
This commit resolves a Time-of-Check to Time-of-Use race condition by
introducing RCU protection for the touch_input pointer. It also delays the
call to wacom_add_shared_data() to ensure correct devres release order.
> diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
> --- a/drivers/hid/wacom_sys.c
> +++ b/drivers/hid/wacom_sys.c
[ ... ]
> @@ -2362,11 +2374,14 @@ static void wacom_set_shared_values(struct wacom_wac *wacom_wac)
> }
>
> if (wacom_wac->shared->has_mute_touch_switch &&
> - wacom_wac->shared->touch_input) {
> - set_bit(EV_SW, wacom_wac->shared->touch_input->evbit);
> - input_set_capability(wacom_wac->shared->touch_input, EV_SW,
> - SW_MUTE_DEVICE);
> + rcu_access_pointer(wacom_wac->shared->touch_input)) {
> + struct input_dev *touch_input = rcu_dereference_protected(wacom_wac->shared->touch_input,
> + lockdep_is_held(&wacom_udev_list_lock));
> + set_bit(EV_SW, touch_input->evbit);
> + input_set_capability(touch_input, EV_SW, SW_MUTE_DEVICE);
> }
> +
> + mutex_unlock(&wacom_udev_list_lock);
> }
[Severity: High]
This is a pre-existing issue, but does this capability modification happen
too late?
In wacom_set_shared_values(), input capabilities are modified on the sibling
touch device. Because the Pen interface probes after the Touch interface,
this occurs on a sibling's touch_input device that was already fully probed
and registered.
The input subsystem requires that all capabilities must be fully initialized
before calling input_register_device(). Modifying capabilities
post-registration means the ADD uevent has already been emitted, and
userspace components miss the capability changes and might fail to process
events like SW_MUTE_DEVICE.
[ ... ]
> @@ -2442,10 +2457,6 @@ static int wacom_parse_and_register(struct wacom *wacom, bool wireless)
> goto fail;
> }
>
> - error = wacom_add_shared_data(hdev);
> - if (error)
> - goto fail;
> -
> error = wacom_setup_inputs(wacom);
> if (error)
> goto fail;
[Severity: Critical]
Does this change introduce a regression during device probe?
By removing wacom_add_shared_data() from this earlier location,
wacom_wac->shared is left uninitialized (NULL). However, wacom_setup_inputs()
is called immediately after:
wacom_setup_inputs()
wacom_setup_touch_input_capabilities()
if (wacom_wac->shared->touch->product == 0x361) {
Because wacom_wac->shared is unconditionally dereferenced for some devices
like INTUOSP2_BT and WACOM_27QHDT inside wacom_setup_touch_input_capabilities(),
could this cause a NULL pointer dereference during initialization?
[ ... ]
> @@ -2496,6 +2507,10 @@ static int wacom_parse_and_register(struct wacom *wacom, bool wireless)
> }
> }
>
> + error = wacom_add_shared_data(hdev);
> + if (error)
> + goto fail_quirks;
> +
> wacom_set_shared_values(wacom_wac);
> devres_close_group(&hdev->dev, wacom);
[Severity: High]
Is there a race condition here since wacom_add_shared_data() is now called
at the very end of the probe function?
Before reaching this point, wacom_parse_and_register() has already called
hid_hw_start() and wacom_register_inputs(). hid_hw_start() enables incoming
HID reports and wacom_register_inputs() exposes the device to userspace.
If a hardware interrupt fires right after hid_hw_start(), the interrupt
handlers can execute while wacom_wac->shared is still NULL:
wacom_bpt_pen()
wacom->shared->stylus_in_proximity = true;
Could this cause an asynchronous NULL pointer dereference if a report
arrives before the shared data is allocated?
> diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
> --- a/drivers/hid/wacom_wac.c
> +++ b/drivers/hid/wacom_wac.c
[ ... ]
> @@ -650,11 +650,18 @@ static int wacom_intuos_pad(struct wacom_wac *wacom)
> input_report_key(input, KEY_CONTROLPANEL, menu);
> input_report_key(input, KEY_INFO, info);
>
> - if (wacom->shared && wacom->shared->touch_input) {
> - input_report_switch(wacom->shared->touch_input,
> - SW_MUTE_DEVICE,
> - !wacom->shared->is_touch_on);
> - input_sync(wacom->shared->touch_input);
> + if (wacom->shared) {
> + struct input_dev *touch_input;
> +
> + rcu_read_lock();
> + touch_input = rcu_dereference(wacom->shared->touch_input);
> + if (touch_input) {
> + input_report_switch(touch_input,
> + SW_MUTE_DEVICE,
> + !wacom->shared->is_touch_on);
> + input_sync(touch_input);
> + }
> + rcu_read_unlock();
> }
[Severity: High]
This is a pre-existing issue, but did this patch miss a few other places
where touch_input is accessed locklessly?
This update successfully converts wacom_intuos_pad() to use RCU, but there
are other HID event handlers in wacom_wac.c that still directly dereference
wacom_wac->shared->touch_input without holding rcu_read_lock().
For example, in wacom_wac_pad_event():
if (wacom_wac->shared->touch_input) {
...
input_report_switch(wacom_wac->shared->touch_input, ...);
Similar direct accesses happen in wacom_wireless_irq() and
wacom_status_irq(). Because these readers do not establish an RCU read-side
critical section, synchronize_rcu() in wacom_remove_shared_data() will not
wait for them.
If the sibling touch device is unbound, could the input_dev be freed while
these handlers are still running, resulting in the same use-after-free issue?
Direct access to an __rcu pointer may also trigger Sparse warnings.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260601140845.1237227-1-lee@kernel.org?part=1
^ permalink raw reply
* Re: [PATCH v2 1/2] HID: wacom: Fix Use-After-Free in wacom_intuos_pad
From: Markus Elfring @ 2026-06-01 14:48 UTC (permalink / raw)
To: Lee Jones, linux-input, Benjamin Tissoires, Dmitry Torokhov,
Jason Gerecke, Jiri Kosina, Peter Hutterer, Ping Cheng
Cc: LKML
In-Reply-To: <20260601140845.1237227-1-lee@kernel.org>
…
> +++ b/drivers/hid/wacom_sys.c
> @@ -877,10 +877,16 @@ static void wacom_remove_shared_data(void *res)
> data = container_of(wacom_wac->shared, struct wacom_hdev_data,
> shared);
>
> - if (wacom_wac->shared->touch == wacom->hdev)
> + mutex_lock(&wacom_udev_list_lock);
> + if (wacom_wac->shared->touch == wacom->hdev) {
> wacom_wac->shared->touch = NULL;
…
> + }
> + mutex_unlock(&wacom_udev_list_lock);
> +
> + synchronize_rcu();
>
> kref_put(&data->kref, wacom_release_shared_data);
…
Under which circumstances would you dare to apply another lock guard?
https://elixir.bootlin.com/linux/v7.1-rc5/source/include/linux/mutex.h#L253
Regards,
Markus
^ permalink raw reply
* Re: [PATCH v3] HID: Expose LattePanda IOTA UPS as a power_supply device
From: Benjamin Tissoires @ 2026-06-01 15:24 UTC (permalink / raw)
To: Andrew Maney; +Cc: jikos, linux-kernel, linux-input
In-Reply-To: <CAO1jzHj6AAB_hkitjsGJDXeN5iiSWcrEdCMeZeuruXmhfwZzYQ@mail.gmail.com>
On May 21 2026, Andrew Maney wrote:
> On Thu, May 21, 2026 at 2:54 AM Benjamin Tissoires <bentiss@kernel.org> wrote:
> >
> > On May 20 2026, Andrew Maney wrote:
> > > This driver exposes the DFRobot LattePanda IOTA UPS board as a standard
> > > power_supply device, allowing desktop environments and power management
> > > tools such as UPower and systemd-logind to display battery status,
> > > remaining capacity, and charging status without any special
> > > configuration. It also enables automatic suspend or shutdown on low
> > > battery and power profile configuration via any tool that supports the
> > > standard power_supply interface.
> > >
> > > The UPS presents itself as an Arduino Leonardo HID device running custom
> > > firmware (VID 0x2341, PID 0x8036). It reports status and capacity via
> > > HID reports 0x07 and 0x0C respectively.
> >
> > As sashiko detected, you are using a standard Arduino Leonardo and not
> > making any specific detections. So I was thinking that maybe we should
> > implement that feature as a HID-BPF program on top of the generic HID
> > handling.
> >
> > However, the handling of battery supplies in the HID generic core is not
> > entirely filling all of the requirements here, but that's something I
> > wanted to do for a couple of month but I have been swamped with other
> > projects.
> >
> > Anyway, I wanted to understand why this product was using a generic
> > Leonardo PID, and: https://wiki.dfrobot.com/dfr1247#tech_specs
> >
> > "it leverages the standard HID-UPS protocol to be natively recognized by
> > Windows as a battery-powered device"
> >
> > So. Instead of working on a custom driver, why not simply implement (or
> > finish the generic implementation) of HID-UPS in hid-input.c?
> >
> > Cheers,
> > Benjamin
> >
>
> Hi Benjamin,
>
> Thank you for the feedback! I'll look into the HID-UPS implementation or a
> HID-BPF program.
>
> I should mention that the kernel does already detect the IOTA UPS via the
> generic HID-UPS support, but the updates that it receives are so slow that
> it becomes practically unusable for a device with smaller capacity batteries
> like this. Changes can take several minutes or more to be detected. I
> originally went with a driver that reads the HID reports directly because it
> was providing updates far faster.
>
> I implemented a check in driver v4 that verifies that the device's HID reports
> are what's expected before binding to it. In theory, that should prevent the
> driver from binding to other Arduino Leonardo devices, but I agree that fixing
> the generic HID-UPS implementation would be a better long-term solution.
>
> Which approach do you think would be more suitable for this, completing the
> HID-UPS implementation or a HID-BPF program? I'm happy to work on either,
> though as a first-time kernel contributor I'd appreciate any pointers
> on where to
> start.
Honestly, if the device is compliant with HID-UPS, you should work in
the kernel implementation. A HID-BPF would be used to fix device issues,
but if the kernel is not correct enough you can't do much in just a
HID-BPF.
If I'm not wrong, the HID-UPS implementation we have currently is the
hid_battery_* functions. And that implementation assumes a HID connected
device is a separate device, not a power supply of the main machine. So
you'll have to deal with a few new HID fields.
Also, I've started the cleanup work on the hid_battery code a couple of
months ago and never got the chance to send it. Maybe I should send it
so you can have a better environment with testing.
Cheers,
Benjamin
>
> Thanks,
> Andrew
>
> > >
> > > The charge limit (80% or 100%) is configured via a physical DIP switch
> > > on the UPS board and cannot be detected automatically. Userspace can
> > > inform the driver of the configured limit via
> > > charge_control_end_threshold.
> > >
> > > ---
> > >
> > > Changes in v3:
> > > - Deferred power_supply registration to workqueue to avoid blocking probe
> > > - Fixed kernel panic when instantiated via uhid by checking hid_is_usb()
> > > before dereferencing USB-specific structures
> > >
> > > - Fixed ERR_PTR dereference in raw_event by only assigning ups->psu on
> > > successful registration
> > >
> > > - Fixed data race on ups->charge_limit using spin_lock_irqsave()
> > > - Removed TIME_TO_EMPTY_NOW and TIME_TO_FULL_NOW properties to avoid
> > > spurious shutdowns
> > >
> > > - Changed plugged-in but not charging state from FULL to NOT_CHARGING
> > > - Used devm_kasprintf() for a unique sysfs name in order to support
> > > multiple devices
> > >
> > > - Added POWER_SUPPLY and HIDRAW dependencies to Kconfig
> > > - Used %pe for more human-readable error messages
> > >
> > > Changes in v2:
> > > - Rebased on top of the current tree
> > > - Moved vendor and device IDs to drivers/hid/hid-ids.h
> > > - Added Kconfig entry under HID bus support -> Special HID drivers
> > > - Added build rule to drivers/hid/Makefile
> > >
> > > Signed-off-by: Andrew Maney <andrewmaney05@gmail.com>
> > > ---
> > > MAINTAINERS | 6 +
> > > drivers/hid/Kconfig | 10 +
> > > drivers/hid/Makefile | 1 +
> > > drivers/hid/hid-ids.h | 3 +
> > > drivers/hid/hid-lattepanda-iota-ups.c | 409 ++++++++++++++++++++++++++
> > > 5 files changed, 429 insertions(+)
> > > create mode 100644 drivers/hid/hid-lattepanda-iota-ups.c
> > >
> > > diff --git a/MAINTAINERS b/MAINTAINERS
> > > index 10e825318..d80721c2c 100644
> > > --- a/MAINTAINERS
> > > +++ b/MAINTAINERS
> > > @@ -11416,6 +11416,12 @@ F: include/uapi/linux/hid*
> > > F: samples/hid/
> > > F: tools/testing/selftests/hid/
> > >
> > > +HID LATTEPANDA IOTA UPS DRIVER
> > > +M: Andrew Maney <andrewmaney05@gmail.com>
> > > +L: linux-input@vger.kernel.org
> > > +S: Maintained
> > > +F: drivers/hid/hid-lattepanda-iota-ups.c
> > > +
> > > HID LOGITECH DRIVERS
> > > R: Filipe Laíns <lains@riseup.net>
> > > L: linux-input@vger.kernel.org
> > > diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
> > > index ff2f580b6..21ffc2fd0 100644
> > > --- a/drivers/hid/Kconfig
> > > +++ b/drivers/hid/Kconfig
> > > @@ -510,6 +510,16 @@ config HID_KYSONA
> > > Say Y here if you have a Kysona M600 mouse
> > > and want to be able to read its battery capacity.
> > >
> > > +config HID_LATTEPANDA_IOTA_UPS
> > > + tristate "LattePanda IOTA UPS"
> > > + depends on USB_HID && USB_HIDDEV && X86 && POWER_SUPPLY
> > > + help
> > > + Support for the LattePanda IOTA UPS (DFRobot, VID 0x2341 PID 0x8036).
> > > + Exposes the battery status and capacity via the power_supply interface.
> > > +
> > > + To compile as a module, choose M here: the module will be
> > > + called hid-lattepanda-iota-ups.
> > > +
> > > config HID_UCLOGIC
> > > tristate "UC-Logic"
> > > depends on USB_HID
> > > diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
> > > index 0597fd6a4..d7ad3fc8f 100644
> > > --- a/drivers/hid/Makefile
> > > +++ b/drivers/hid/Makefile
> > > @@ -74,6 +74,7 @@ obj-$(CONFIG_HID_KENSINGTON) += hid-kensington.o
> > > obj-$(CONFIG_HID_KEYTOUCH) += hid-keytouch.o
> > > obj-$(CONFIG_HID_KYE) += hid-kye.o
> > > obj-$(CONFIG_HID_KYSONA) += hid-kysona.o
> > > +obj-$(CONFIG_HID_LATTEPANDA_IOTA_UPS) += hid-lattepanda-iota-ups.o
> > > obj-$(CONFIG_HID_LCPOWER) += hid-lcpower.o
> > > obj-$(CONFIG_HID_LENOVO) += hid-lenovo.o
> > > obj-$(CONFIG_HID_LENOVO_GO) += hid-lenovo-go.o
> > > diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
> > > index 4657d96fb..6ded2c943 100644
> > > --- a/drivers/hid/hid-ids.h
> > > +++ b/drivers/hid/hid-ids.h
> > > @@ -859,6 +859,9 @@
> > > #define USB_DEVICE_ID_LD_HYBRID 0x2090
> > > #define USB_DEVICE_ID_LD_HEATCONTROL 0x20A0
> > >
> > > +#define USB_VENDOR_ID_LATTEPANDA_IOTA 0x2341
> > > +#define USB_DEVICE_ID_LATTEPANDA_IOTA_UPS 0x8036
> > > +
> > > #define USB_VENDOR_ID_LENOVO 0x17ef
> > > #define USB_DEVICE_ID_LENOVO_TPKBD 0x6009
> > > #define USB_DEVICE_ID_LENOVO_CUSBKBD 0x6047
> > > diff --git a/drivers/hid/hid-lattepanda-iota-ups.c b/drivers/hid/hid-lattepanda-iota-ups.c
> > > new file mode 100644
> > > index 000000000..f5d522695
> > > --- /dev/null
> > > +++ b/drivers/hid/hid-lattepanda-iota-ups.c
> > > @@ -0,0 +1,409 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +#include <linux/power_supply.h>
> > > +#include <linux/completion.h>
> > > +#include <linux/workqueue.h>
> > > +#include <linux/module.h>
> > > +#include <linux/spinlock.h>
> > > +#include <linux/hid.h>
> > > +#include <linux/usb.h>
> > > +#include "hid-ids.h"
> > > +
> > > +#define REPORT_ID_CAPACITY 0x0C
> > > +#define REPORT_ID_STATUS 0x07
> > > +
> > > +#define STATUS_DISCHARGING BIT(1)
> > > +#define STATUS_PLUGGED_IN BIT(0)
> > > +#define STATUS_CHARGING BIT(2)
> > > +
> > > +MODULE_AUTHOR("Andrew Maney");
> > > +MODULE_DESCRIPTION("LattePanda IOTA UPS power supply driver");
> > > +MODULE_LICENSE("GPL");
> > > +
> > > +struct iota_ups {
> > > + struct power_supply_desc psu_desc;
> > > + struct power_supply *psu;
> > > + struct hid_device *hiddev;
> > > + spinlock_t lock; /* Protects cached HID report values */
> > > +
> > > + /* Cached values that are updated from HID reports */
> > > + bool plugged_in;
> > > + char serial[64];
> > > + int charge_limit;
> > > + int psu_status;
> > > + int capacity;
> > > +
> > > + /*
> > > + * Wait for both status and capacity reports before registering
> > > + * with the power_supply core, so initial values are correct and
> > > + * not erroneous.
> > > + */
> > > + struct completion got_initial_data;
> > > + struct work_struct register_work;
> > > + bool got_capacity;
> > > + bool data_ready;
> > > + bool got_status;
> > > +};
> > > +
> > > +static enum power_supply_property iota_ups_properties[] = {
> > > + POWER_SUPPLY_PROP_CHARGE_CONTROL_END_THRESHOLD,
> > > + POWER_SUPPLY_PROP_SERIAL_NUMBER,
> > > + POWER_SUPPLY_PROP_MANUFACTURER,
> > > + POWER_SUPPLY_PROP_MODEL_NAME,
> > > + POWER_SUPPLY_PROP_TECHNOLOGY,
> > > + POWER_SUPPLY_PROP_CAPACITY,
> > > + POWER_SUPPLY_PROP_PRESENT,
> > > + POWER_SUPPLY_PROP_ONLINE,
> > > + POWER_SUPPLY_PROP_STATUS,
> > > + POWER_SUPPLY_PROP_SCOPE,
> > > +};
> > > +
> > > +static const struct hid_device_id iota_ups_devices[] = {
> > > + { HID_USB_DEVICE(USB_VENDOR_ID_LATTEPANDA_IOTA,
> > > + USB_DEVICE_ID_LATTEPANDA_IOTA_UPS) },
> > > + { }
> > > +};
> > > +MODULE_DEVICE_TABLE(hid, iota_ups_devices);
> > > +
> > > +static int iota_ups_get_property(struct power_supply *supply,
> > > + enum power_supply_property psp,
> > > + union power_supply_propval *val)
> > > +{
> > > + struct iota_ups *ups = power_supply_get_drvdata(supply);
> > > + unsigned long flags;
> > > +
> > > + spin_lock_irqsave(&ups->lock, flags);
> > > +
> > > + switch (psp) {
> > > + case POWER_SUPPLY_PROP_STATUS:
> > > + val->intval = ups->psu_status;
> > > + break;
> > > +
> > > + /* Remaining capacity as a percentage from 0 to 100 */
> > > + case POWER_SUPPLY_PROP_CAPACITY:
> > > + val->intval = ups->capacity;
> > > + break;
> > > +
> > > + /* The UPS is always present if the driver is loaded */
> > > + case POWER_SUPPLY_PROP_PRESENT:
> > > + val->intval = 1;
> > > + break;
> > > +
> > > + /* Whether mains power is connected */
> > > + case POWER_SUPPLY_PROP_ONLINE:
> > > + val->intval = ups->plugged_in ? 1 : 0;
> > > + break;
> > > +
> > > + /*
> > > + * The UPS board supplies power to the IOTA and any
> > > + * peripherals connected to it, therefore its scope
> > > + * is system-wide.
> > > + */
> > > + case POWER_SUPPLY_PROP_SCOPE:
> > > + val->intval = POWER_SUPPLY_SCOPE_SYSTEM;
> > > + break;
> > > +
> > > + /* V1.0 only accepts 18650 Li-ion cells */
> > > + case POWER_SUPPLY_PROP_TECHNOLOGY:
> > > + val->intval = POWER_SUPPLY_TECHNOLOGY_LION;
> > > + break;
> > > +
> > > + /* 80% or 100%, configured via a DIP switch on the UPS board */
> > > + case POWER_SUPPLY_PROP_CHARGE_CONTROL_END_THRESHOLD:
> > > + val->intval = ups->charge_limit;
> > > + break;
> > > +
> > > + case POWER_SUPPLY_PROP_MANUFACTURER:
> > > + val->strval = "DFRobot";
> > > + break;
> > > +
> > > + case POWER_SUPPLY_PROP_MODEL_NAME:
> > > + val->strval = "LattePanda IOTA UPS";
> > > + break;
> > > +
> > > + /* Retrieved from the USB descriptor */
> > > + case POWER_SUPPLY_PROP_SERIAL_NUMBER:
> > > + val->strval = ups->serial;
> > > + break;
> > > +
> > > + default:
> > > + spin_unlock_irqrestore(&ups->lock, flags);
> > > + return -EINVAL;
> > > + }
> > > +
> > > + spin_unlock_irqrestore(&ups->lock, flags);
> > > + return 0;
> > > +}
> > > +
> > > +static int iota_ups_set_property(struct power_supply *supply,
> > > + enum power_supply_property psp,
> > > + const union power_supply_propval *val)
> > > +{
> > > + struct iota_ups *ups = power_supply_get_drvdata(supply);
> > > +
> > > + if (psp == POWER_SUPPLY_PROP_CHARGE_CONTROL_END_THRESHOLD) {
> > > + unsigned long flags;
> > > +
> > > + /*
> > > + * V1.0 supports 80% and 100% charge limits only, which is
> > > + * set via a DIP switch on the board. This property allows
> > > + * userspace to inform the driver which limit is configured.
> > > + */
> > > + if (val->intval != 80 && val->intval != 100)
> > > + return -EINVAL;
> > > +
> > > + spin_lock_irqsave(&ups->lock, flags);
> > > + ups->charge_limit = val->intval;
> > > + spin_unlock_irqrestore(&ups->lock, flags);
> > > + return 0;
> > > + }
> > > +
> > > + return -EINVAL;
> > > +}
> > > +
> > > +static int iota_ups_property_is_writable(struct power_supply *supply,
> > > + enum power_supply_property psp)
> > > +{
> > > + return psp == POWER_SUPPLY_PROP_CHARGE_CONTROL_END_THRESHOLD;
> > > +}
> > > +
> > > +static int iota_ups_raw_event(struct hid_device *hdev,
> > > + struct hid_report *report,
> > > + u8 *data, int size)
> > > +{
> > > + struct iota_ups *ups = hid_get_drvdata(hdev);
> > > + unsigned long flags;
> > > + bool changed = false;
> > > +
> > > + /* All of the UPS's reports are at least 2 bytes */
> > > + if (size < 2)
> > > + return 0;
> > > +
> > > + spin_lock_irqsave(&ups->lock, flags);
> > > +
> > > + switch (data[0]) {
> > > + case REPORT_ID_STATUS: {
> > > + u8 status = data[1];
> > > + int new_status;
> > > + bool plugged_in = !!(status & STATUS_PLUGGED_IN);
> > > +
> > > + /*
> > > + * The UPS status is determined as follows:
> > > + * Battery full:
> > > + * UPS is plugged in
> > > + * Battery is at full capacity
> > > + *
> > > + * Battery charging:
> > > + * UPS is plugged in
> > > + * Battery is not at full capacity
> > > + *
> > > + * Battery discharging:
> > > + * UPS is not plugged in
> > > + *
> > > + * Battery not charging:
> > > + * UPS is plugged in
> > > + * UPS has halted charging for some reason
> > > + *
> > > + * Unknown:
> > > + * None of the above conditions are met
> > > + */
> > > + if (status & STATUS_CHARGING) {
> > > + if (ups->capacity >= ups->charge_limit)
> > > + new_status = POWER_SUPPLY_STATUS_FULL;
> > > + else
> > > + new_status = POWER_SUPPLY_STATUS_CHARGING;
> > > +
> > > + } else if (status & STATUS_DISCHARGING) {
> > > + new_status = POWER_SUPPLY_STATUS_DISCHARGING;
> > > +
> > > + } else if (plugged_in) {
> > > + new_status = POWER_SUPPLY_STATUS_NOT_CHARGING;
> > > +
> > > + } else {
> > > + new_status = POWER_SUPPLY_STATUS_UNKNOWN;
> > > + }
> > > +
> > > + if (new_status != ups->psu_status ||
> > > + plugged_in != ups->plugged_in) {
> > > + ups->plugged_in = plugged_in;
> > > + ups->psu_status = new_status;
> > > + changed = true;
> > > + }
> > > +
> > > + ups->got_status = true;
> > > + break;
> > > + }
> > > +
> > > + case REPORT_ID_CAPACITY: {
> > > + int new_cap = clamp((int)data[1], 0, 100);
> > > +
> > > + if (new_cap != ups->capacity) {
> > > + ups->capacity = new_cap;
> > > + changed = true;
> > > + }
> > > +
> > > + ups->got_capacity = true;
> > > + break;
> > > + }
> > > + }
> > > +
> > > + /*
> > > + * Signal that the UPS is ready to be registered because we have
> > > + * received both capacity and status reports.
> > > + */
> > > + if (!ups->data_ready && ups->got_status && ups->got_capacity) {
> > > + ups->data_ready = true;
> > > + complete(&ups->got_initial_data);
> > > + }
> > > +
> > > + spin_unlock_irqrestore(&ups->lock, flags);
> > > +
> > > + /*
> > > + * Notify the power_supply core outside the spinlock to avoid
> > > + * a deadlock; power_supply_changed() may call back into
> > > + * get_property() which acquires the same lock.
> > > + */
> > > + if (changed && ups->psu)
> > > + power_supply_changed(ups->psu);
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static void iota_ups_register_work(struct work_struct *work)
> > > +{
> > > + struct iota_ups *ups = container_of(work, struct iota_ups, register_work);
> > > + struct power_supply_config psu_config = {};
> > > + struct power_supply *psu;
> > > +
> > > + /*
> > > + * Wait for both status and capacity reports before registering.
> > > + * The device sends reports every ~1 second, so 3 seconds is safe.
> > > + * We wait here in order to prevent registration in an unknown
> > > + * state, since this could cause emergency shutdowns or other
> > > + * undesired effects.
> > > + */
> > > + wait_for_completion_timeout(&ups->got_initial_data,
> > > + msecs_to_jiffies(3000));
> > > +
> > > + /* Configure the UPS's power supply properties */
> > > + ups->psu_desc.name = devm_kasprintf(&ups->hiddev->dev, GFP_KERNEL,
> > > + "lattepanda-iota-ups.%s",
> > > + dev_name(&ups->hiddev->dev));
> > > +
> > > + if (!ups->psu_desc.name) {
> > > + hid_err(ups->hiddev, "failed to allocate power supply name\n");
> > > + return;
> > > + }
> > > +
> > > + ups->psu_desc.property_is_writeable = iota_ups_property_is_writable;
> > > + ups->psu_desc.num_properties = ARRAY_SIZE(iota_ups_properties);
> > > + ups->psu_desc.get_property = iota_ups_get_property;
> > > + ups->psu_desc.set_property = iota_ups_set_property;
> > > + ups->psu_desc.properties = iota_ups_properties;
> > > + ups->psu_desc.type = POWER_SUPPLY_TYPE_BATTERY;
> > > + psu_config.drv_data = ups;
> > > +
> > > + /* Register the UPS as a power_supply device */
> > > + psu = devm_power_supply_register(&ups->hiddev->dev, &ups->psu_desc, &psu_config);
> > > + if (IS_ERR(psu)) {
> > > + hid_err(ups->hiddev, "power supply registration failed: %pe\n", psu);
> > > + return;
> > > + }
> > > +
> > > + /*
> > > + * Finally, notify the power_supply core so userspace reads the correct
> > > + * initial state immediately after registration.
> > > + */
> > > + ups->psu = psu;
> > > + power_supply_changed(ups->psu);
> > > + hid_info(ups->hiddev, "LattePanda IOTA UPS registered as a power_supply device\n");
> > > +}
> > > +
> > > +static int iota_ups_probe(struct hid_device *hdev,
> > > + const struct hid_device_id *id)
> > > +{
> > > + struct iota_ups *ups;
> > > + int ret;
> > > +
> > > + ups = devm_kzalloc(&hdev->dev, sizeof(*ups), GFP_KERNEL);
> > > + if (!ups)
> > > + return -ENOMEM;
> > > +
> > > + ups->hiddev = hdev;
> > > + ups->psu_status = POWER_SUPPLY_STATUS_UNKNOWN;
> > > +
> > > + /* 50% is a safe default if wait_for_completion_timeout() times out. */
> > > + ups->capacity = 50;
> > > +
> > > + /*
> > > + * Default to 100% to prevent unexpected shutdowns.
> > > + * Userspace can update this via charge_control_end_threshold.
> > > + */
> > > + ups->charge_limit = 100;
> > > +
> > > + init_completion(&ups->got_initial_data);
> > > + spin_lock_init(&ups->lock);
> > > + hid_set_drvdata(hdev, ups);
> > > +
> > > + /*
> > > + * Retrieve the UPS's serial number from the USB descriptor. If the device is not
> > > + * a USB device, we can use the unique device identifier as the serial number.
> > > + */
> > > + if (hid_is_usb(hdev)) {
> > > + struct usb_device *udev = to_usb_device(hdev->dev.parent->parent);
> > > +
> > > + if (udev->serial)
> > > + strscpy(ups->serial, udev->serial, sizeof(ups->serial));
> > > + else
> > > + strscpy(ups->serial, "Unknown", sizeof(ups->serial));
> > > + } else {
> > > + if (*hdev->uniq)
> > > + strscpy(ups->serial, hdev->uniq, sizeof(ups->serial));
> > > + else
> > > + strscpy(ups->serial, "Unknown", sizeof(ups->serial));
> > > + }
> > > +
> > > + ret = hid_parse(hdev);
> > > + if (ret) {
> > > + hid_err(hdev, "HID parse failed: %pe\n", ERR_PTR(ret));
> > > + return ret;
> > > + }
> > > +
> > > + ret = hid_hw_start(hdev, HID_CONNECT_HIDRAW);
> > > + if (ret) {
> > > + hid_err(hdev, "HID hw start failed: %pe\n", ERR_PTR(ret));
> > > + return ret;
> > > + }
> > > +
> > > + ret = hid_hw_open(hdev);
> > > + if (ret) {
> > > + hid_err(hdev, "HID hw open failed: %pe\n", ERR_PTR(ret));
> > > + goto err_stop;
> > > + }
> > > +
> > > + /* Probe for the UPS in a worker queue so we don't halt the enumeration thread */
> > > + INIT_WORK(&ups->register_work, iota_ups_register_work);
> > > + schedule_work(&ups->register_work);
> > > + return 0;
> > > +
> > > +err_stop:
> > > + hid_hw_stop(hdev);
> > > + return ret;
> > > +}
> > > +
> > > +static void iota_ups_remove(struct hid_device *hdev)
> > > +{
> > > + struct iota_ups *ups = hid_get_drvdata(hdev);
> > > +
> > > + cancel_work_sync(&ups->register_work);
> > > + hid_hw_close(hdev);
> > > + hid_hw_stop(hdev);
> > > +}
> > > +
> > > +static struct hid_driver iota_ups_driver = {
> > > + .name = "lattepanda-iota-ups",
> > > + .id_table = iota_ups_devices,
> > > + .probe = iota_ups_probe,
> > > + .remove = iota_ups_remove,
> > > + .raw_event = iota_ups_raw_event,
> > > +};
> > > +module_hid_driver(iota_ups_driver);
> > > --
> > > 2.54.0
> > >
> > >
>
^ permalink raw reply
* Re: [PATCH v4 00/36] HID: iio: basic clean up and introduce devm_ API for HID sensors
From: srinivas pandruvada @ 2026-06-01 15:55 UTC (permalink / raw)
To: Jonathan Cameron, Sanjay Chitroda
Cc: jikos, dlechner, nuno.sa, andy, sakari.ailus, linux-input,
linux-iio, linux-kernel
In-Reply-To: <20260526165639.42d6e98e@jic23-huawei>
On Tue, 2026-05-26 at 16:56 +0100, Jonathan Cameron wrote:
> On Mon, 25 May 2026 00:50:23 +0530
> Sanjay Chitroda <sanjayembeddedse@gmail.com> wrote:
>
> > From: Sanjay Chitroda <sanjayembeddedse@gmail.com>
> >
> > Key highlights:
> > - 0000-0024: General cleanup and kernel few coding style fixes
> > across HID IIO drivers
> > - 0025: Remove unused iio_dev argument from HID IIO sensor helper
> > - 0026: Introduce devm_hid_sensor_setup_trigger() device-managed
> > API
> > - 0027-0036: Convert HID IIO sensor drivers to use the new devm-
> > based trigger setup
> >
> > changes in v4:
> > - Extend the series to cover remaining HID IIO drivers with devm
> > API usage
> > - Reorder patches to place cleanup and warning fix at beginning
> > and,
> > devm-related changes toward the end based on feedback from David
> > - v3 series ->
> > https://lore.kernel.org/all/20260509101040.791404-1-sanjayembedded@gmail.com/
> > changes in v3:
> > - Added cleanup and prepratory changes before adding devm_ API
> > conversion based on self review: 0002, 0004, 0006, 0007 and 0008
> > - Address andy's review comment on commit message and coding style
> > - v2 series ->
> > https://lore.kernel.org/all/20260429175918.2541914-1-sanjayembedded@gmail.com/
> > changes in v2:
> > - Following input from Jonathan and Andy, squash initial patch v1
> > series in single change as individual change should not break
> > anything
> > - Add devm API support and two driver using the same
> > - v1 series ->
> > https://lore.kernel.org/all/20260428071613.1134053-1-sanjayembedded@gmail.com/
> >
> > Testing:
> > - Compiled with W=1 for each patch in series
> > - Build-tested on QEMU x86_64
> >
> > P.S.
> > - Sashiko reported an issue in a different driver and noted that it
> > is not
> > introduced by this series. I have taken this feedback into
> > account and
> > will address the actual issue in a separate series focus on that
> > driver.
> > - Once this series is merged into the IIO tree, a number of HID IIO
> > drivers will become available to fully converted to devm API
> > usage.
> > - The changes are organized across drivers to keep similar
> > modifications
> > grouped together for consistency, making the series easier to
> > review,
> > rather than grouping all changes per driver.
> >
> The series is mostly fine, though not sure what happened with how you
> sent
> it as I have it in seperate threads of 10 patches at a time. If you
> are having
> email troubles with big series, use b4 to send them instead
> (instructions on kernel.org)
>
> My main concern is this is a lot of churn on a critical driver. I'd
> like therefore
> to be able to see the full benefit of those devm patches rather than
> get us
> part way as this does.
>
I am not able to apply this patchset cleanly on 7.1-rc6. This series
does formatting changes, u32 changes and devm changes. Atleast three
different series will be better.
Thanks,
Srinivas
> Jonathan
>
> > Thanks,
> > Sanjay Chitroda
> >
> > Sanjay Chitroda (36):
> > iio: hid-sensors: add missing blank line after declarations
> > iio: gyro: hid-sensor-gyro-3d: use u32 instead of unsigned
> > iio: accel: hid-sensor-accel-3d: use u32 instead of unsigned
> > iio: light: hid-sensor-als: use u32 instead of unsigned
> > iio: light: hid-sensor-prox: use u32 instead of unsigned
> > iio: orientation: hid-sensor-incl-3d: use u32 instead of unsigned
> > iio: orientation: hid-sensor-rotation: use u32 instead of
> > unsigned
> > iio: pressure: hid-sensor-press: use u32 instead of unsigned
> > iio: humidity: hid-sensor-humidity: align parenthesis for
> > readability
> > iio: gyro: hid-sensor-gyro-3d: align parenthesis for readability
> > iio: magnetometer: hid-sensor-magn-3d: align parenthesis for
> > readability
> > iio: humidity: hid-sensor-humidity: use common device for devres
> > iio: position: hid-sensor-custom-intel-hinge: use common device
> > for
> > devres
> > iio: temperature: hid-sensor-temperature: use common device for
> > devres
> > iio: humidity: hid-sensor-humidity: use local struct device
> > iio: gyro: hid-sensor-gyro-3d: use local struct device
> > iio: accel: hid-sensor-accel-3d: use local struct device
> > iio: light: hid-sensor-als: use local struct device
> > iio: light: hid-sensor-prox: use local struct device
> > iio: magnetometer: hid-sensor-magn-3d: use local struct device
> > iio: orientation: hid-sensor-incl-3d: use local struct device
> > iio: orientation: hid-sensor-rotation: use local struct device
> > iio: position: hid-sensor-custom-intel-hinge: use local struct
> > device
> > iio: pressure: hid-sensor-press: use local struct device
> > iio: hid-sensors: remove unused iio_dev argument
> > iio: hid-sensors: introduce device managed API
> > iio: gyro: hid-sensor-gyro-3d: drop hid_sensor_remove_trigger()
> > using
> > devm API
> > iio: humidity: hid-sensor-humidity: drop
> > hid_sensor_remove_trigger()
> > using devm API
> > iio: light: hid-sensor-prox: drop hid_sensor_remove_trigger()
> > using
> > devm API
> > iio: light: hid-sensor-als: drop hid_sensor_remove_trigger()
> > using
> > devm API
> > iio: magnetometer: hid-sensor-magn-3d: drop
> > hid_sensor_remove_trigger() using devm API
> > iio: orientation: hid-sensor-incl-3d: drop
> > hid_sensor_remove_trigger()
> > using devm API
> > iio: orientation: hid-sensor-rotation: drop
> > hid_sensor_remove_trigger() using devm API
> > iio: position: hid-sensor-custom-intel-hinge: drop
> > hid_sensor_remove_trigger() using devm API
> > iio: pressure: hid-sensor-press: drop hid_sensor_remove_trigger()
> > using devm API
> > iio: temperature: hid-sensor-temperature: drop
> > hid_sensor_remove_trigger() using devm API
> >
> > drivers/iio/accel/hid-sensor-accel-3d.c | 30 ++---
> > .../common/hid-sensors/hid-sensor-trigger.c | 24 +++-
> > .../common/hid-sensors/hid-sensor-trigger.h | 5 +-
> > drivers/iio/gyro/hid-sensor-gyro-3d.c | 96 ++++++++------
> > -
> > drivers/iio/humidity/hid-sensor-humidity.c | 61 +++++-----
> > drivers/iio/light/hid-sensor-als.c | 31 +++--
> > drivers/iio/light/hid-sensor-prox.c | 30 ++---
> > drivers/iio/magnetometer/hid-sensor-magn-3d.c | 112 +++++++++-----
> > ----
> > drivers/iio/orientation/hid-sensor-incl-3d.c | 36 +++---
> > drivers/iio/orientation/hid-sensor-rotation.c | 38 +++---
> > .../position/hid-sensor-custom-intel-hinge.c | 27 ++---
> > drivers/iio/pressure/hid-sensor-press.c | 36 +++---
> > .../iio/temperature/hid-sensor-temperature.c | 15 +--
> > 13 files changed, 264 insertions(+), 277 deletions(-)
> >
> >
> > base-commit: 08297ca8422541dde6c8b7e6b1d68bd4aa4568ef
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox