* [PATCH v13 2/4] HID: hid-msi: Add M-key mapping attributes
From: Derek J. Clark @ 2026-07-20 3:15 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: Pierre-Loup A . Griffais, Denis Benato, Zhouwang Huang,
Derek J . Clark, linux-input, linux-doc, linux-kernel
In-Reply-To: <20260720031549.2272658-1-derekjohn.clark@gmail.com>
Adds attributes that allow for remapping the M-keys with up to 5 values
when in macro mode. There are 2 mappable buttons on the rear of the
device, M1 on the right and M2 on the left. When mapped, the events will
fire from one of three event devices: gamepad buttons will fire from the
device handled by xpad, while keyboard and mouse events will fire from
respectively typed evdevs provided by the input core. Names of each
mapping have been kept as close to the event that will fire from the evdev
as possible, with context added to the ABS_ events on the direction of the
movement.
Initial reverse-engineering and implementation of this feature was done
by Zhouwang Huang. I refactored the overall format to conform to kernel
driver best practices and style guides. Claude was used as an initial
reviewer of this patch.
Assisted-by: Claude:claude-sonnet-4-6
Co-developed-by: Zhouwang Huang <honjow311@gmail.com>
Signed-off-by: Zhouwang Huang <honjow311@gmail.com>
Link: https://patch.msgid.link/20260529072111.7565-3-derekjohn.clark@gmail.com
Signed-off-by: Derek J. Clark <derekjohn.clark@gmail.com>
---
v12:
- On address mismatch, assume stale message return and keep waiting for
correct message.
- Use spinlock_irqsave for raw_event-reachable locks since completion
context isn't guaranteed softirq-only across all HCDs.
v10:
- Remove additional gamepad_registered variable left over after rename
to gp_registered.
v8:
- Wrap all branches under single cmd_lock guard in claw_raw_event.
- Reject generic ACK in claw_raw_event if waiting_cmd is for another
branch.
v7:
- Use smp_[store_release|load_acquire] pattern for checking
gamepad_registered to avoid possible races during teardown.
- Add profile_lock for read/write profile_pending.
- Match on write address for mkey reports to prevent late ACK
from causing synchronization errors.
- Use struct for mkey reports.
v6:
- Make all timeouts 25ms to ensure at least 2 jiffies in a 100Hz
config.
- Gate all attribute show/store functions with gamepad_registered.
- Remove duplicated argv_free macro.
v5:
- Ensure adding "DISABLED" key to valid entries is done in the correct
patch.
- Re-enable sending an empty string to clear button mappings in
addition to setting DISABLED.
v4:
- Change dev_warn to dev_dbg in claw_profile_event.
- use __free with DEFINE_FREE macro for argv instead of manually
running argv_free, cleaining up scoped_guard goto.
v3:
- Use scoped_guard where necessary.
v2:
- Add mutex for SYNC_TO_ROM commands to ensure every SYNC is completed
before more data is written to the MCU volatile memory.
- Add mutex for profile_pending to ensure every profile action
response is serialized to the generating command.
---
drivers/hid/hid-msi.c | 446 +++++++++++++++++++++++++++++++++++++++++-
1 file changed, 445 insertions(+), 1 deletion(-)
diff --git a/drivers/hid/hid-msi.c b/drivers/hid/hid-msi.c
index 6687e4579faa..023d63cee6b0 100644
--- a/drivers/hid/hid-msi.c
+++ b/drivers/hid/hid-msi.c
@@ -43,6 +43,8 @@
#define CLAW_DINPUT_CFG_INTF_IN 0x82
#define CLAW_XINPUT_CFG_INTF_IN 0x83
+#define CLAW_KEYS_MAX 5
+
enum claw_command_index {
CLAW_COMMAND_TYPE_NONE = 0x00,
CLAW_COMMAND_TYPE_READ_PROFILE = 0x04,
@@ -68,6 +70,17 @@ static const char * const claw_gamepad_mode_text[] = {
[CLAW_GAMEPAD_MODE_DESKTOP] = "desktop",
};
+enum claw_profile_ack_pending {
+ CLAW_NO_PENDING,
+ CLAW_M1_PENDING,
+ CLAW_M2_PENDING,
+};
+
+enum claw_key_index {
+ CLAW_KEY_M1,
+ CLAW_KEY_M2,
+};
+
enum claw_mkeys_function_index {
CLAW_MKEY_FUNCTION_MACRO,
CLAW_MKEY_FUNCTION_DISABLED,
@@ -85,6 +98,155 @@ static const char * const claw_mkeys_function_text[] = {
[CLAW_MKEY_FUNCTION_COMBO] = "combination",
};
+static const struct {
+ u8 code;
+ const char *name;
+} claw_button_mapping_key_map[] = {
+ /* Gamepad buttons */
+ { 0x01, "ABS_HAT0Y_UP" },
+ { 0x02, "ABS_HAT0Y_DOWN" },
+ { 0x03, "ABS_HAT0X_LEFT" },
+ { 0x04, "ABS_HAT0X_RIGHT" },
+ { 0x05, "BTN_TL" },
+ { 0x06, "BTN_TR" },
+ { 0x07, "BTN_THUMBL" },
+ { 0x08, "BTN_THUMBR" },
+ { 0x09, "BTN_SOUTH" },
+ { 0x0a, "BTN_EAST" },
+ { 0x0b, "BTN_NORTH" },
+ { 0x0c, "BTN_WEST" },
+ { 0x0d, "BTN_MODE" },
+ { 0x0e, "BTN_SELECT" },
+ { 0x0f, "BTN_START" },
+ { 0x13, "BTN_TL2"},
+ { 0x14, "BTN_TR2"},
+ { 0x15, "ABS_Y_UP"},
+ { 0x16, "ABS_Y_DOWN"},
+ { 0x17, "ABS_X_LEFT"},
+ { 0x18, "ABS_X_RIGHT"},
+ { 0x19, "ABS_RY_UP"},
+ { 0x1a, "ABS_RY_DOWN"},
+ { 0x1b, "ABS_RX_LEFT"},
+ { 0x1c, "ABS_RX_RIGHT"},
+ /* Keyboard keys */
+ { 0x32, "KEY_ESC" },
+ { 0x33, "KEY_F1" },
+ { 0x34, "KEY_F2" },
+ { 0x35, "KEY_F3" },
+ { 0x36, "KEY_F4" },
+ { 0x37, "KEY_F5" },
+ { 0x38, "KEY_F6" },
+ { 0x39, "KEY_F7" },
+ { 0x3a, "KEY_F8" },
+ { 0x3b, "KEY_F9" },
+ { 0x3c, "KEY_F10" },
+ { 0x3d, "KEY_F11" },
+ { 0x3e, "KEY_F12" },
+ { 0x3f, "KEY_GRAVE" },
+ { 0x40, "KEY_1" },
+ { 0x41, "KEY_2" },
+ { 0x42, "KEY_3" },
+ { 0x43, "KEY_4" },
+ { 0x44, "KEY_5" },
+ { 0x45, "KEY_6" },
+ { 0x46, "KEY_7" },
+ { 0x47, "KEY_8" },
+ { 0x48, "KEY_9" },
+ { 0x49, "KEY_0" },
+ { 0x4a, "KEY_MINUS" },
+ { 0x4b, "KEY_EQUAL" },
+ { 0x4c, "KEY_BACKSPACE" },
+ { 0x4d, "KEY_TAB" },
+ { 0x4e, "KEY_Q" },
+ { 0x4f, "KEY_W" },
+ { 0x50, "KEY_E" },
+ { 0x51, "KEY_R" },
+ { 0x52, "KEY_T" },
+ { 0x53, "KEY_Y" },
+ { 0x54, "KEY_U" },
+ { 0x55, "KEY_I" },
+ { 0x56, "KEY_O" },
+ { 0x57, "KEY_P" },
+ { 0x58, "KEY_LEFTBRACE" },
+ { 0x59, "KEY_RIGHTBRACE" },
+ { 0x5a, "KEY_BACKSLASH" },
+ { 0x5b, "KEY_CAPSLOCK" },
+ { 0x5c, "KEY_A" },
+ { 0x5d, "KEY_S" },
+ { 0x5e, "KEY_D" },
+ { 0x5f, "KEY_F" },
+ { 0x60, "KEY_G" },
+ { 0x61, "KEY_H" },
+ { 0x62, "KEY_J" },
+ { 0x63, "KEY_K" },
+ { 0x64, "KEY_L" },
+ { 0x65, "KEY_SEMICOLON" },
+ { 0x66, "KEY_APOSTROPHE" },
+ { 0x67, "KEY_ENTER" },
+ { 0x68, "KEY_LEFTSHIFT" },
+ { 0x69, "KEY_Z" },
+ { 0x6a, "KEY_X" },
+ { 0x6b, "KEY_C" },
+ { 0x6c, "KEY_V" },
+ { 0x6d, "KEY_B" },
+ { 0x6e, "KEY_N" },
+ { 0x6f, "KEY_M" },
+ { 0x70, "KEY_COMMA" },
+ { 0x71, "KEY_DOT" },
+ { 0x72, "KEY_SLASH" },
+ { 0x73, "KEY_RIGHTSHIFT" },
+ { 0x74, "KEY_LEFTCTRL" },
+ { 0x75, "KEY_LEFTMETA" },
+ { 0x76, "KEY_LEFTALT" },
+ { 0x77, "KEY_SPACE" },
+ { 0x78, "KEY_RIGHTALT" },
+ { 0x79, "KEY_RIGHTCTRL" },
+ { 0x7a, "KEY_INSERT" },
+ { 0x7b, "KEY_HOME" },
+ { 0x7c, "KEY_PAGEUP" },
+ { 0x7d, "KEY_DELETE" },
+ { 0x7e, "KEY_END" },
+ { 0x7f, "KEY_PAGEDOWN" },
+ { 0x8a, "KEY_KPENTER" },
+ { 0x8b, "KEY_KP0" },
+ { 0x8c, "KEY_KP1" },
+ { 0x8d, "KEY_KP2" },
+ { 0x8e, "KEY_KP3" },
+ { 0x8f, "KEY_KP4" },
+ { 0x90, "KEY_KP5" },
+ { 0x91, "KEY_KP6" },
+ { 0x92, "KEY_KP7" },
+ { 0x93, "KEY_KP8" },
+ { 0x94, "KEY_KP9" },
+ { 0x95, "MD_PLAY" },
+ { 0x96, "MD_STOP" },
+ { 0x97, "MD_NEXT" },
+ { 0x98, "MD_PREV" },
+ { 0x99, "MD_VOL_UP" },
+ { 0x9a, "MD_VOL_DOWN" },
+ { 0x9b, "MD_VOL_MUTE" },
+ { 0x9c, "KEY_F23" },
+ /* Mouse events */
+ { 0xc8, "BTN_LEFT" },
+ { 0xc9, "BTN_MIDDLE" },
+ { 0xca, "BTN_RIGHT" },
+ { 0xcb, "BTN_SIDE" },
+ { 0xcc, "BTN_EXTRA" },
+ { 0xcd, "REL_WHEEL_UP" },
+ { 0xce, "REL_WHEEL_DOWN" },
+ { 0xff, "DISABLED" },
+};
+
+static const u16 button_mapping_addr_old[] = {
+ 0x007a, /* M1 */
+ 0x011f, /* M2 */
+};
+
+static const u16 button_mapping_addr_new[] = {
+ 0x00bb, /* M1 */
+ 0x0164, /* M2 */
+};
+
struct claw_command_report {
u8 report_id;
u8 padding[2];
@@ -93,26 +255,48 @@ struct claw_command_report {
u8 data[59];
} __packed;
+struct claw_profile_report {
+ u8 profile;
+ __be16 read_addr;
+} __packed;
+
+struct claw_mkey_report {
+ struct claw_profile_report;
+ u8 padding_0;
+ u8 padding_1;
+ u8 padding_2;
+ u8 codes[5];
+} __packed;
+
struct claw_drvdata {
/* MCU General Variables */
+ enum claw_profile_ack_pending profile_pending;
struct completion orphan_ack_complete;
struct completion send_cmd_complete;
struct delayed_work cfg_resume;
struct delayed_work cfg_setup;
spinlock_t registration_lock; /* Lock for registration read/write */
+ struct mutex profile_mutex; /* mutex for profile_pending calls */
+ spinlock_t profile_lock; /* Lock for profile_pending read/write */
struct hid_device *hdev;
bool orphan_ack_pending;
struct mutex cfg_mutex; /* mutex for synchronous data */
+ struct mutex rom_mutex; /* mutex for SYNC_TO_ROM calls */
spinlock_t cmd_lock; /* Lock for cmd data read/write */
u8 waiting_cmd;
int cmd_status;
+ u16 bcd_device;
u8 ep;
/* Gamepad Variables */
enum claw_mkeys_function_index mkeys_function;
enum claw_gamepad_mode_index gamepad_mode;
+ u8 m1_codes[CLAW_KEYS_MAX];
+ u8 m2_codes[CLAW_KEYS_MAX];
+ const u16 *bmap_addr;
spinlock_t mode_lock; /* Lock for mode data read/write */
bool gp_registered;
+ bool bmap_support;
};
static int get_endpoint_address(struct hid_device *hdev)
@@ -144,6 +328,39 @@ static int claw_gamepad_mode_event(struct claw_drvdata *drvdata,
return 0;
}
+static int claw_profile_event(struct claw_drvdata *drvdata, struct claw_command_report *cmd_rep)
+{
+ enum claw_profile_ack_pending profile;
+ struct claw_mkey_report *mkeys;
+ u8 *codes, key;
+ int i;
+
+ scoped_guard(spinlock_irqsave, &drvdata->profile_lock)
+ profile = drvdata->profile_pending;
+
+ switch (profile) {
+ case CLAW_M1_PENDING:
+ case CLAW_M2_PENDING:
+ key = (profile == CLAW_M1_PENDING) ? CLAW_KEY_M1 : CLAW_KEY_M2;
+ mkeys = (struct claw_mkey_report *)cmd_rep->data;
+ if (be16_to_cpu(mkeys->read_addr) != drvdata->bmap_addr[key])
+ return -EAGAIN;
+ codes = (profile == CLAW_M1_PENDING) ? drvdata->m1_codes : drvdata->m2_codes;
+ for (i = 0; i < CLAW_KEYS_MAX; i++)
+ codes[i] = (mkeys->codes[i]);
+ break;
+ default:
+ dev_dbg(&drvdata->hdev->dev,
+ "Got profile event without changes pending from command: %x\n",
+ cmd_rep->cmd);
+ return -EINVAL;
+ }
+ scoped_guard(spinlock_irqsave, &drvdata->profile_lock)
+ drvdata->profile_pending = CLAW_NO_PENDING;
+
+ return 0;
+}
+
static int claw_raw_event(struct claw_drvdata *drvdata, struct hid_report *report,
u8 *data, int size)
{
@@ -170,6 +387,17 @@ static int claw_raw_event(struct claw_drvdata *drvdata, struct hid_report *repor
complete(&drvdata->send_cmd_complete);
}
+ break;
+ case CLAW_COMMAND_TYPE_READ_PROFILE_ACK:
+ ret = claw_profile_event(drvdata, cmd_rep);
+ /* Stale address received, ignore and keep waiting */
+ if (ret == -EAGAIN)
+ return 0;
+ if (drvdata->waiting_cmd == CLAW_COMMAND_TYPE_READ_PROFILE) {
+ drvdata->cmd_status = ret;
+ complete(&drvdata->send_cmd_complete);
+ }
+
break;
case CLAW_COMMAND_TYPE_ACK:
if (drvdata->orphan_ack_pending) {
@@ -499,6 +727,177 @@ static ssize_t reset_store(struct device *dev, struct device_attribute *attr,
}
static DEVICE_ATTR_WO(reset);
+static int mkey_mapping_name_to_code(const char *name)
+{
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(claw_button_mapping_key_map); i++) {
+ if (!strcmp(name, claw_button_mapping_key_map[i].name))
+ return claw_button_mapping_key_map[i].code;
+ }
+
+ return -EINVAL;
+}
+
+static const char *mkey_mapping_code_to_name(u8 code)
+{
+ int i;
+
+ if (code == 0xff)
+ return NULL;
+
+ for (i = 0; i < ARRAY_SIZE(claw_button_mapping_key_map); i++) {
+ if (claw_button_mapping_key_map[i].code == code)
+ return claw_button_mapping_key_map[i].name;
+ }
+
+ return NULL;
+}
+
+static int claw_mkey_store(struct device *dev, const char *buf, u8 mkey)
+{
+ struct hid_device *hdev = to_hid_device(dev);
+ struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
+ struct claw_mkey_report report = { {0x01, cpu_to_be16(drvdata->bmap_addr[mkey])},
+ 0x07, 0x04, 0x00, {0xff, 0xff, 0xff, 0xff, 0xff} };
+ char **raw_keys __free(argv_free) = NULL;
+ int ret, key_count, i;
+
+ scoped_guard(spinlock_irqsave, &drvdata->registration_lock) {
+ /* Pairs with smp_store_release from cfg_setup_fn in system_wq context */
+ if (!smp_load_acquire(&drvdata->gp_registered))
+ return -ENODEV;
+ }
+
+ raw_keys = argv_split(GFP_KERNEL, buf, &key_count);
+ if (!raw_keys)
+ return -ENOMEM;
+
+ if (key_count > CLAW_KEYS_MAX)
+ return -EINVAL;
+
+ if (key_count == 0)
+ goto set_buttons;
+
+ for (i = 0; i < key_count; i++) {
+ ret = mkey_mapping_name_to_code(raw_keys[i]);
+ if (ret < 0)
+ return ret;
+
+ report.codes[i] = ret;
+ }
+
+set_buttons:
+ scoped_guard(mutex, &drvdata->rom_mutex) {
+ ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_WRITE_PROFILE_DATA,
+ (u8 *)&report, sizeof(report), 25);
+ if (ret)
+ return ret;
+ /* MCU will not send ACK until the USB transaction completes. ACK is sent
+ * immediately after and will hit the stale state machine, before the next
+ * command re-arms the state machine. Timeout 0 ensures no deadlock waiting
+ * for ACK that ill never come.
+ */
+ ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_SYNC_TO_ROM, NULL, 0, 0);
+ }
+
+ return ret;
+}
+
+static int claw_mkey_show(struct device *dev, char *buf, enum claw_key_index m_key)
+{
+ struct hid_device *hdev = to_hid_device(dev);
+ struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
+ struct claw_mkey_report report = { {0x01, cpu_to_be16(drvdata->bmap_addr[m_key])}, 0x07 };
+ int i, ret, count = 0;
+ const char *name;
+ u8 *codes;
+
+ scoped_guard(spinlock_irqsave, &drvdata->registration_lock) {
+ /* Pairs with smp_store_release from cfg_setup_fn in system_wq context */
+ if (!smp_load_acquire(&drvdata->gp_registered))
+ return -ENODEV;
+ }
+
+ codes = (m_key == CLAW_KEY_M1) ? drvdata->m1_codes : drvdata->m2_codes;
+
+ guard(mutex)(&drvdata->profile_mutex);
+ scoped_guard(spinlock_irqsave, &drvdata->profile_lock)
+ drvdata->profile_pending = (m_key == CLAW_KEY_M1) ? CLAW_M1_PENDING
+ : CLAW_M2_PENDING;
+
+ ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_READ_PROFILE,
+ (u8 *)&report, sizeof(report), 25);
+ if (ret)
+ return ret;
+
+ for (i = 0; i < CLAW_KEYS_MAX; i++) {
+ name = mkey_mapping_code_to_name(codes[i]);
+ if (name)
+ count += sysfs_emit_at(buf, count, "%s ", name);
+ }
+
+ if (!count)
+ return sysfs_emit(buf, "(not set)\n");
+
+ buf[count - 1] = '\n';
+
+ return count;
+}
+
+static ssize_t button_m1_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ int ret;
+
+ ret = claw_mkey_store(dev, buf, CLAW_KEY_M1);
+ if (ret)
+ return ret;
+
+ return count;
+}
+
+static ssize_t button_m1_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ return claw_mkey_show(dev, buf, CLAW_KEY_M1);
+}
+static DEVICE_ATTR_RW(button_m1);
+
+static ssize_t button_m2_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ int ret;
+
+ ret = claw_mkey_store(dev, buf, CLAW_KEY_M2);
+ if (ret)
+ return ret;
+
+ return count;
+}
+
+static ssize_t button_m2_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ return claw_mkey_show(dev, buf, CLAW_KEY_M2);
+}
+static DEVICE_ATTR_RW(button_m2);
+
+static ssize_t button_mapping_options_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ int i, count = 0;
+
+ for (i = 0; i < ARRAY_SIZE(claw_button_mapping_key_map); i++)
+ count += sysfs_emit_at(buf, count, "%s ", claw_button_mapping_key_map[i].name);
+
+ if (count)
+ buf[count - 1] = '\n';
+
+ return count;
+}
+static DEVICE_ATTR_RO(button_mapping_options);
+
static umode_t claw_gamepad_attr_is_visible(struct kobject *kobj, struct attribute *attr,
int n)
{
@@ -511,10 +910,22 @@ static umode_t claw_gamepad_attr_is_visible(struct kobject *kobj, struct attribu
return 0;
}
- return attr->mode;
+ /* Always show attrs available on all firmware */
+ if (attr == &dev_attr_gamepad_mode.attr ||
+ attr == &dev_attr_gamepad_mode_index.attr ||
+ attr == &dev_attr_mkeys_function.attr ||
+ attr == &dev_attr_mkeys_function_index.attr ||
+ attr == &dev_attr_reset.attr)
+ return attr->mode;
+
+ /* Hide button mapping attrs if it isn't supported */
+ return drvdata->bmap_support ? attr->mode : 0;
}
static struct attribute *claw_gamepad_attrs[] = {
+ &dev_attr_button_m1.attr,
+ &dev_attr_button_m2.attr,
+ &dev_attr_button_mapping_options.attr,
&dev_attr_gamepad_mode.attr,
&dev_attr_gamepad_mode_index.attr,
&dev_attr_mkeys_function.attr,
@@ -567,8 +978,31 @@ static void cfg_resume_fn(struct work_struct *work)
schedule_delayed_work(&drvdata->cfg_setup, msecs_to_jiffies(500));
}
+static void claw_features_supported(struct claw_drvdata *drvdata)
+{
+ u8 major = (drvdata->bcd_device >> 8) & 0xff;
+ u8 minor = drvdata->bcd_device & 0xff;
+
+ if (major == 0x01) {
+ drvdata->bmap_support = true;
+ if (minor >= 0x66)
+ drvdata->bmap_addr = button_mapping_addr_new;
+ else
+ drvdata->bmap_addr = button_mapping_addr_old;
+ return;
+ }
+
+ if ((major == 0x02 && minor >= 0x17) || major >= 0x03) {
+ drvdata->bmap_support = true;
+ drvdata->bmap_addr = button_mapping_addr_new;
+ return;
+ }
+}
+
static int claw_probe(struct hid_device *hdev, u8 ep)
{
+ struct usb_interface *intf = to_usb_interface(hdev->dev.parent);
+ struct usb_device *udev = interface_to_usbdev(intf);
struct claw_drvdata *drvdata;
int ret;
@@ -580,10 +1014,20 @@ static int claw_probe(struct hid_device *hdev, u8 ep)
drvdata->hdev = hdev;
drvdata->ep = ep;
+ /* Determine feature level from firmware version */
+ drvdata->bcd_device = le16_to_cpu(udev->descriptor.bcdDevice);
+ claw_features_supported(drvdata);
+
+ if (!drvdata->bmap_support)
+ dev_dbg(&hdev->dev, "M-Key mapping is not supported. Update firmware to enable.\n");
+
mutex_init(&drvdata->cfg_mutex);
+ mutex_init(&drvdata->profile_mutex);
+ mutex_init(&drvdata->rom_mutex);
spin_lock_init(&drvdata->registration_lock);
spin_lock_init(&drvdata->cmd_lock);
spin_lock_init(&drvdata->mode_lock);
+ spin_lock_init(&drvdata->profile_lock);
init_completion(&drvdata->orphan_ack_complete);
init_completion(&drvdata->send_cmd_complete);
INIT_DELAYED_WORK(&drvdata->cfg_resume, &cfg_resume_fn);
--
2.54.0
^ permalink raw reply related
* [PATCH v13 3/4] HID: hid-msi: Add RGB control interface
From: Derek J. Clark @ 2026-07-20 3:15 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: Pierre-Loup A . Griffais, Denis Benato, Zhouwang Huang,
Derek J . Clark, linux-input, linux-doc, linux-kernel
In-Reply-To: <20260720031549.2272658-1-derekjohn.clark@gmail.com>
Adds RGB control interface for MSI Claw devices. The MSI Claw uses a
fairly unique RGB interface. It has 9 total zones (4 per joystick ring
and 1 for the ABXY buttons), and supports up to 8 sequential frames of
RGB zone data. Each frame is written to a specific area of MCU memory by
the profile command, the value of which changes based on the firmware of
the device. Unlike other devices (such as the Legion Go or the OneXPlayer
devices), there are no hard coded effects built into the MCU. Instead,
the basic effects are provided as a series of frame data. I have
mirrored the effects available in Windows in this driver, while keeping
the effect names consistent with the Lenovo drivers for the effects that
are similar.
Initial reverse-engineering and implementation of this feature was done
by Zhouwang Huang. I refactored the overall format to conform to kernel
driver best practices and style guides. Claude was used as an initial
reviewer of this patch.
Assisted-by: Claude:claude-sonnet-4-6
Co-developed-by: Zhouwang Huang <honjow311@gmail.com>
Signed-off-by: Zhouwang Huang <honjow311@gmail.com>
Link: https://patch.msgid.link/20260529072111.7565-4-derekjohn.clark@gmail.com
Signed-off-by: Derek J. Clark <derekjohn.clark@gmail.com>
---
v13:
- Add profile lock to rgb attributes store functions when mutating
read varaibles.
- Only use drvdata->hdev->dev for errors instead of
drvdata->led_mc.led_cdev.dev, preventing use-after-free in rare races.
v12:
- On remove, cancel the rgb_queue last to avoid a re-arming, and in
rgb_queue_fn check rgb_registered to avoid running after the sysfs
group has been removed.
- On address mismatch, assume stale message return and keep waiting for
correct message.
- Use spinlock_irqsave for raw_event-reachable locks since completion
context isn't guaranteed softirq-only across all HCDs.
- Add note to led_cdev.name to (hopefully) silence sashiko-bot about
an impossible double device collision.
- Explicitly check if the current address is less than the rgb_address
to avoid invalid overflow math.
v9:
- Limit guard usage in cfg_setup_fn to avoid holding a lock during
registration and add group events.
- Don't use devm_ functions in cfg_setup_fn. Manually setup/teardown
devices and groups to prevent use after free.
- Ensure rgb_queue work is canceled during suspend.
- Check for drvdata in rgb_queue_fn to avoid use after free during
teardown.
v8:
- Ensure led_classdev is unregistered if adding attribute group fails.
- Reorder remove actions to ensure no use-after free or rearming cleared
flags.
v7:
- Use smp_[store_release|load_acquire] pattern for checking
rgb_registered to avoid possible races during teardown.
- Add gating to cfg_setup_fn, allowing either gamepad settings or rgb
settings to populate if the other fails for any reason.
- Use spinlock when writing profile_pending.
v6:
- Make all timeouts 25ms to ensure at least 2 jiffies in a 100Hz
config.
- Gate all attribute show/store functions with rgb_registered,
enabling use of devm_device_add_group.
v5:
- Move adding the RGB device into cfg_setup to prevent led core
attributes from being written to prior to setup completing.
- Ensure frame_lock is properly init.
- Change variable names in RGB functions from frame and zone to f and
z respectively to fit all scoped_guard actions in 100 columns.
v4:
- Fix frame_calc validity check to use >=.
- USe spinlock instead of mutex in raw_event and related attribute
_store function.
- Ensure delayed work is canceled in suspend & canceled before sysfs
attribute removal.
v3:
- Add mutex for read/write of rgb frame data.
- Remove setting rgb_frame_count when reading rgb profiles as it always
returns garbage data.
- Ensure rgb_speed is getting drvdata from a valid lookup (not hdev).
v2:
- Use pending_profile mutex
---
drivers/hid/hid-msi.c | 681 +++++++++++++++++++++++++++++++++++++++++-
1 file changed, 666 insertions(+), 15 deletions(-)
diff --git a/drivers/hid/hid-msi.c b/drivers/hid/hid-msi.c
index 023d63cee6b0..5cb85fc2f053 100644
--- a/drivers/hid/hid-msi.c
+++ b/drivers/hid/hid-msi.c
@@ -22,6 +22,7 @@
#include <linux/device.h>
#include <linux/hid.h>
#include <linux/kobject.h>
+#include <linux/led-class-multicolor.h>
#include <linux/leds.h>
#include <linux/module.h>
#include <linux/mutex.h>
@@ -45,6 +46,10 @@
#define CLAW_KEYS_MAX 5
+#define CLAW_RGB_ZONES 9
+#define CLAW_RGB_MAX_FRAMES 8
+#define CLAW_RGB_FRAME_OFFSET 0x24
+
enum claw_command_index {
CLAW_COMMAND_TYPE_NONE = 0x00,
CLAW_COMMAND_TYPE_READ_PROFILE = 0x04,
@@ -74,6 +79,7 @@ enum claw_profile_ack_pending {
CLAW_NO_PENDING,
CLAW_M1_PENDING,
CLAW_M2_PENDING,
+ CLAW_RGB_PENDING,
};
enum claw_key_index {
@@ -237,6 +243,22 @@ static const struct {
{ 0xff, "DISABLED" },
};
+enum claw_rgb_effect_index {
+ CLAW_RGB_EFFECT_MONOCOLOR,
+ CLAW_RGB_EFFECT_BREATHE,
+ CLAW_RGB_EFFECT_CHROMA,
+ CLAW_RGB_EFFECT_RAINBOW,
+ CLAW_RGB_EFFECT_FROSTFIRE,
+};
+
+static const char * const claw_rgb_effect_text[] = {
+ [CLAW_RGB_EFFECT_MONOCOLOR] = "monocolor",
+ [CLAW_RGB_EFFECT_BREATHE] = "breathe",
+ [CLAW_RGB_EFFECT_CHROMA] = "chroma",
+ [CLAW_RGB_EFFECT_RAINBOW] = "rainbow",
+ [CLAW_RGB_EFFECT_FROSTFIRE] = "frostfire",
+};
+
static const u16 button_mapping_addr_old[] = {
0x007a, /* M1 */
0x011f, /* M2 */
@@ -247,6 +269,9 @@ static const u16 button_mapping_addr_new[] = {
0x0164, /* M2 */
};
+static const u16 rgb_addr_old = 0x01fa;
+static const u16 rgb_addr_new = 0x024a;
+
struct claw_command_report {
u8 report_id;
u8 padding[2];
@@ -268,6 +293,27 @@ struct claw_mkey_report {
u8 codes[5];
} __packed;
+struct rgb_zone {
+ u8 red;
+ u8 green;
+ u8 blue;
+};
+
+struct rgb_frame {
+ struct rgb_zone zone[CLAW_RGB_ZONES];
+};
+
+struct claw_rgb_report {
+ struct claw_profile_report;
+ u8 frame_bytes;
+ u8 padding;
+ u8 frame_count;
+ u8 state; /* Always 0x09 */
+ u8 speed;
+ u8 brightness;
+ struct rgb_frame zone_data;
+} __packed;
+
struct claw_drvdata {
/* MCU General Variables */
enum claw_profile_ack_pending profile_pending;
@@ -293,10 +339,22 @@ struct claw_drvdata {
enum claw_gamepad_mode_index gamepad_mode;
u8 m1_codes[CLAW_KEYS_MAX];
u8 m2_codes[CLAW_KEYS_MAX];
- const u16 *bmap_addr;
spinlock_t mode_lock; /* Lock for mode data read/write */
+ const u16 *bmap_addr;
bool gp_registered;
bool bmap_support;
+
+ /* RGB Variables */
+ struct rgb_frame rgb_frames[CLAW_RGB_MAX_FRAMES];
+ enum claw_rgb_effect_index rgb_effect;
+ struct led_classdev_mc led_mc;
+ struct delayed_work rgb_queue;
+ spinlock_t frame_lock; /* lock for rgb_frames read/write */
+ bool rgb_registered;
+ u8 rgb_frame_count;
+ bool rgb_enabled;
+ u8 rgb_speed;
+ u16 rgb_addr;
};
static int get_endpoint_address(struct hid_device *hdev)
@@ -332,7 +390,10 @@ static int claw_profile_event(struct claw_drvdata *drvdata, struct claw_command_
{
enum claw_profile_ack_pending profile;
struct claw_mkey_report *mkeys;
- u8 *codes, key;
+ struct claw_rgb_report *frame;
+ u16 rgb_addr, read_addr;
+ u8 *codes, key, f_idx;
+ u16 frame_calc;
int i;
scoped_guard(spinlock_irqsave, &drvdata->profile_lock)
@@ -349,6 +410,38 @@ static int claw_profile_event(struct claw_drvdata *drvdata, struct claw_command_
for (i = 0; i < CLAW_KEYS_MAX; i++)
codes[i] = (mkeys->codes[i]);
break;
+ case CLAW_RGB_PENDING:
+ frame = (struct claw_rgb_report *)cmd_rep->data;
+ rgb_addr = drvdata->rgb_addr;
+ read_addr = be16_to_cpu(frame->read_addr);
+
+ if (read_addr < drvdata->rgb_addr)
+ return -EAGAIN;
+
+ frame_calc = (read_addr - rgb_addr) / CLAW_RGB_FRAME_OFFSET;
+ if (frame_calc >= CLAW_RGB_MAX_FRAMES) {
+ dev_err(&drvdata->hdev->dev, "Got unsupported frame index: %x\n",
+ frame_calc);
+ return -EAGAIN;
+ }
+ f_idx = frame_calc;
+
+ scoped_guard(spinlock_irqsave, &drvdata->frame_lock) {
+ memcpy(&drvdata->rgb_frames[f_idx], &frame->zone_data,
+ sizeof(struct rgb_frame));
+
+ /* Only use frame 0 for remaining variable assignment */
+ if (f_idx != 0)
+ break;
+
+ drvdata->rgb_speed = frame->speed;
+ drvdata->led_mc.led_cdev.brightness = frame->brightness;
+ drvdata->led_mc.subled_info[0].intensity = frame->zone_data.zone[0].red;
+ drvdata->led_mc.subled_info[1].intensity = frame->zone_data.zone[0].green;
+ drvdata->led_mc.subled_info[2].intensity = frame->zone_data.zone[0].blue;
+ }
+
+ break;
default:
dev_dbg(&drvdata->hdev->dev,
"Got profile event without changes pending from command: %x\n",
@@ -939,32 +1032,555 @@ static const struct attribute_group claw_gamepad_attr_group = {
.is_visible = claw_gamepad_attr_is_visible,
};
+/* Read RGB config from device */
+static int claw_read_rgb_config(struct hid_device *hdev)
+{
+ u8 data[4] = { 0x01, 0x00, 0x00, CLAW_RGB_FRAME_OFFSET };
+ struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
+ u16 read_addr = drvdata->rgb_addr;
+ size_t len = ARRAY_SIZE(data);
+ int ret, i;
+
+ if (!drvdata->rgb_addr)
+ return -ENODEV;
+
+ /* Loop through all 8 pages of RGB data */
+ guard(mutex)(&drvdata->profile_mutex);
+ for (i = 0; i < CLAW_RGB_MAX_FRAMES; i++) {
+ scoped_guard(spinlock_irqsave, &drvdata->profile_lock)
+ drvdata->profile_pending = CLAW_RGB_PENDING;
+ data[1] = (read_addr >> 8) & 0xff;
+ data[2] = read_addr & 0x00ff;
+ ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_READ_PROFILE, data, len, 25);
+ if (ret)
+ return ret;
+
+ read_addr += CLAW_RGB_FRAME_OFFSET;
+ }
+
+ return 0;
+}
+
+/* Send RGB configuration to device */
+static int claw_write_rgb_state(struct claw_drvdata *drvdata)
+{
+ struct claw_rgb_report report = { {0x01, 0}, CLAW_RGB_FRAME_OFFSET, 0x00,
+ drvdata->rgb_frame_count, 0x09, drvdata->rgb_speed,
+ drvdata->led_mc.led_cdev.brightness };
+ u16 write_addr = drvdata->rgb_addr;
+ int f, ret;
+
+ if (!drvdata->rgb_addr)
+ return -ENODEV;
+
+ if (!drvdata->rgb_frame_count)
+ return -EINVAL;
+
+ guard(mutex)(&drvdata->rom_mutex);
+ /* Loop through (up to) 8 pages of RGB data */
+ for (f = 0; f < drvdata->rgb_frame_count; f++) {
+ scoped_guard(spinlock_irqsave, &drvdata->frame_lock)
+ report.zone_data = drvdata->rgb_frames[f];
+
+ /* Set the MCU address to write the frame data to */
+ report.read_addr = cpu_to_be16(write_addr);
+
+ /* Serialize the rgb_report and write it to MCU */
+ ret = claw_hw_output_report(drvdata->hdev, CLAW_COMMAND_TYPE_WRITE_PROFILE_DATA,
+ (u8 *)&report, sizeof(report), 25);
+ if (ret)
+ return ret;
+
+ /* Increment the write addr by the offset for the next frame */
+ write_addr += CLAW_RGB_FRAME_OFFSET;
+ }
+
+ /* MCU will not send ACK until the USB transaction completes. ACK is sent
+ * immediately after and will hit the stale state machine, before the next
+ * command re-arms the state machine. Timeout 0 ensures no deadlock waiting
+ * for ACK that ill never come.
+ */
+ ret = claw_hw_output_report(drvdata->hdev, CLAW_COMMAND_TYPE_SYNC_TO_ROM, NULL, 0, 0);
+
+ return ret;
+}
+
+/* Fill all zones with the same color */
+static void claw_frame_fill_solid(struct rgb_frame *frame, struct rgb_zone zone)
+{
+ int z;
+
+ for (z = 0; z < CLAW_RGB_ZONES; z++)
+ frame->zone[z] = zone;
+}
+
+/* Apply solid effect (1 frame, no color) */
+static int claw_apply_disabled(struct claw_drvdata *drvdata)
+{
+ struct rgb_zone off = { 0x00, 0x00, 0x00};
+
+ scoped_guard(spinlock_irqsave, &drvdata->frame_lock) {
+ drvdata->rgb_frame_count = 1;
+ claw_frame_fill_solid(&drvdata->rgb_frames[0], off);
+ }
+
+ return claw_write_rgb_state(drvdata);
+}
+
+/* Apply solid effect (1 frame, all zones same color) */
+static int claw_apply_monocolor(struct claw_drvdata *drvdata)
+{
+ struct mc_subled *subleds = drvdata->led_mc.subled_info;
+ struct rgb_zone zone = { subleds[0].intensity, subleds[1].intensity,
+ subleds[2].intensity };
+
+ scoped_guard(spinlock_irqsave, &drvdata->frame_lock) {
+ drvdata->rgb_frame_count = 1;
+ claw_frame_fill_solid(&drvdata->rgb_frames[0], zone);
+ }
+
+ return claw_write_rgb_state(drvdata);
+}
+
+/* Apply breathe effect (2 frames: color -> off) */
+static int claw_apply_breathe(struct claw_drvdata *drvdata)
+{
+ struct mc_subled *subleds = drvdata->led_mc.subled_info;
+ struct rgb_zone zone = { subleds[0].intensity, subleds[1].intensity,
+ subleds[2].intensity };
+ static const struct rgb_zone off = { 0, 0, 0 };
+
+ scoped_guard(spinlock_irqsave, &drvdata->frame_lock) {
+ drvdata->rgb_frame_count = 2;
+ claw_frame_fill_solid(&drvdata->rgb_frames[0], zone);
+ claw_frame_fill_solid(&drvdata->rgb_frames[1], off);
+ }
+
+ return claw_write_rgb_state(drvdata);
+}
+
+/* Apply chroma effect (6 frames: rainbow cycle, all zones sync) */
+static int claw_apply_chroma(struct claw_drvdata *drvdata)
+{
+ static const struct rgb_zone colors[] = {
+ {255, 0, 0}, /* red */
+ {255, 255, 0}, /* yellow */
+ { 0, 255, 0}, /* green */
+ { 0, 255, 255}, /* cyan */
+ { 0, 0, 255}, /* blue */
+ {255, 0, 255}, /* magenta */
+ };
+ u8 frame_count = ARRAY_SIZE(colors);
+ int f;
+
+ scoped_guard(spinlock_irqsave, &drvdata->frame_lock) {
+ drvdata->rgb_frame_count = frame_count;
+
+ for (f = 0; f < frame_count; f++)
+ claw_frame_fill_solid(&drvdata->rgb_frames[f], colors[f]);
+ }
+
+ return claw_write_rgb_state(drvdata);
+}
+
+/* Apply rainbow effect (4 frames: rotating colors around joysticks) */
+static int claw_apply_rainbow(struct claw_drvdata *drvdata)
+{
+ static const struct rgb_zone colors[] = {
+ {255, 0, 0}, /* red */
+ { 0, 255, 0}, /* green */
+ { 0, 255, 255}, /* cyan */
+ { 0, 0, 255}, /* blue */
+ };
+ u8 frame_count = ARRAY_SIZE(colors);
+ int f, z;
+
+ scoped_guard(spinlock_irqsave, &drvdata->frame_lock) {
+ drvdata->rgb_frame_count = frame_count;
+
+ for (f = 0; f < frame_count; f++) {
+ for (z = 0; z < 4; z++) {
+ drvdata->rgb_frames[f].zone[z] = colors[(z + f) % 4];
+ drvdata->rgb_frames[f].zone[z + 4] = colors[(z + f) % 4];
+ }
+ drvdata->rgb_frames[f].zone[8] = colors[f];
+ }
+ }
+
+ return claw_write_rgb_state(drvdata);
+}
+
+/*
+ * Apply frostfire effect (4 frames: fire vs ice rotating)
+ * Right joystick: fire red -> dark -> ice blue -> dark (clockwise)
+ * Left joystick: ice blue -> dark -> fire red -> dark (counter-clockwise)
+ * ABXY: fire red -> dark -> ice blue -> dark
+ */
+static int claw_apply_frostfire(struct claw_drvdata *drvdata)
+{
+ static const struct rgb_zone colors[] = {
+ {255, 0, 0}, /* fire red */
+ { 0, 0, 0}, /* dark */
+ { 0, 0, 255}, /* ice blue */
+ { 0, 0, 0}, /* dark */
+ };
+ u8 frame_count = ARRAY_SIZE(colors);
+ int f, z;
+
+ scoped_guard(spinlock_irqsave, &drvdata->frame_lock) {
+ drvdata->rgb_frame_count = frame_count;
+
+ for (f = 0; f < frame_count; f++) {
+ for (z = 0; z < 4; z++) {
+ drvdata->rgb_frames[f].zone[z] = colors[(z + f) % 4];
+ drvdata->rgb_frames[f].zone[z + 4] = colors[(z - f + 6) % 4];
+ }
+ drvdata->rgb_frames[f].zone[8] = colors[f];
+ }
+ }
+
+ return claw_write_rgb_state(drvdata);
+}
+
+/* Apply current state to device */
+static int claw_apply_rgb_state(struct claw_drvdata *drvdata)
+{
+ if (!drvdata->rgb_enabled)
+ return claw_apply_disabled(drvdata);
+
+ switch (drvdata->rgb_effect) {
+ case CLAW_RGB_EFFECT_MONOCOLOR:
+ return claw_apply_monocolor(drvdata);
+ case CLAW_RGB_EFFECT_BREATHE:
+ return claw_apply_breathe(drvdata);
+ case CLAW_RGB_EFFECT_CHROMA:
+ return claw_apply_chroma(drvdata);
+ case CLAW_RGB_EFFECT_RAINBOW:
+ return claw_apply_rainbow(drvdata);
+ case CLAW_RGB_EFFECT_FROSTFIRE:
+ return claw_apply_frostfire(drvdata);
+ default:
+ dev_err(&drvdata->hdev->dev, "No supported rgb_effect selected\n");
+ return -EINVAL;
+ }
+}
+
+static void claw_rgb_queue_fn(struct work_struct *work)
+{
+ struct delayed_work *dwork = container_of(work, struct delayed_work, work);
+ struct claw_drvdata *drvdata = container_of(dwork, struct claw_drvdata, rgb_queue);
+ int ret;
+
+ if (!drvdata)
+ return;
+
+ scoped_guard(spinlock_irqsave, &drvdata->registration_lock) {
+ /* Pairs with smp_store_release from cfg_setup_fn in system_wq context */
+ if (!smp_load_acquire(&drvdata->rgb_registered))
+ return;
+ }
+
+ ret = claw_apply_rgb_state(drvdata);
+ if (ret)
+ dev_err(&drvdata->hdev->dev, "Failed to apply RGB state: %d\n", ret);
+}
+
+static ssize_t effect_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct led_classdev *led_cdev = dev_get_drvdata(dev);
+ struct led_classdev_mc *led_mc = container_of(led_cdev, struct led_classdev_mc, led_cdev);
+ struct claw_drvdata *drvdata = container_of(led_mc, struct claw_drvdata, led_mc);
+ int ret;
+
+ scoped_guard(spinlock_irqsave, &drvdata->registration_lock) {
+ /* Pairs with smp_store_release from cfg_setup_fn in system_wq context */
+ if (!smp_load_acquire(&drvdata->rgb_registered))
+ return -ENODEV;
+ }
+
+ ret = sysfs_match_string(claw_rgb_effect_text, buf);
+ if (ret < 0)
+ return ret;
+
+ scoped_guard(spinlock_irqsave, &drvdata->profile_lock)
+ drvdata->rgb_effect = ret;
+
+ mod_delayed_work(system_wq, &drvdata->rgb_queue, msecs_to_jiffies(50));
+
+ return count;
+}
+
+static ssize_t effect_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct led_classdev *led_cdev = dev_get_drvdata(dev);
+ struct led_classdev_mc *led_mc = container_of(led_cdev, struct led_classdev_mc, led_cdev);
+ struct claw_drvdata *drvdata = container_of(led_mc, struct claw_drvdata, led_mc);
+
+ scoped_guard(spinlock_irqsave, &drvdata->registration_lock) {
+ /* Pairs with smp_store_release from cfg_setup_fn in system_wq context */
+ if (!smp_load_acquire(&drvdata->rgb_registered))
+ return -ENODEV;
+ }
+
+ if (drvdata->rgb_effect >= ARRAY_SIZE(claw_rgb_effect_text))
+ return -EINVAL;
+
+ return sysfs_emit(buf, "%s\n", claw_rgb_effect_text[drvdata->rgb_effect]);
+}
+
+static DEVICE_ATTR_RW(effect);
+
+static ssize_t effect_index_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ int i, count = 0;
+
+ for (i = 0; i < ARRAY_SIZE(claw_rgb_effect_text); i++)
+ count += sysfs_emit_at(buf, count, "%s ", claw_rgb_effect_text[i]);
+
+ if (count)
+ buf[count - 1] = '\n';
+
+ return count;
+}
+static DEVICE_ATTR_RO(effect_index);
+
+static ssize_t enabled_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct led_classdev *led_cdev = dev_get_drvdata(dev);
+ struct led_classdev_mc *led_mc = container_of(led_cdev, struct led_classdev_mc, led_cdev);
+ struct claw_drvdata *drvdata = container_of(led_mc, struct claw_drvdata, led_mc);
+ bool val;
+ int ret;
+
+ scoped_guard(spinlock_irqsave, &drvdata->registration_lock) {
+ /* Pairs with smp_store_release from cfg_setup_fn in system_wq context */
+ if (!smp_load_acquire(&drvdata->rgb_registered))
+ return -ENODEV;
+ }
+
+ ret = kstrtobool(buf, &val);
+ if (ret)
+ return ret;
+
+ scoped_guard(spinlock_irqsave, &drvdata->profile_lock)
+ drvdata->rgb_enabled = val;
+
+ mod_delayed_work(system_wq, &drvdata->rgb_queue, msecs_to_jiffies(50));
+
+ return count;
+}
+
+static ssize_t enabled_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct led_classdev *led_cdev = dev_get_drvdata(dev);
+ struct led_classdev_mc *led_mc = container_of(led_cdev, struct led_classdev_mc, led_cdev);
+ struct claw_drvdata *drvdata = container_of(led_mc, struct claw_drvdata, led_mc);
+
+ scoped_guard(spinlock_irqsave, &drvdata->registration_lock) {
+ /* Pairs with smp_store_release from cfg_setup_fn in system_wq context */
+ if (!smp_load_acquire(&drvdata->rgb_registered))
+ return -ENODEV;
+ }
+
+ return sysfs_emit(buf, "%s\n", drvdata->rgb_enabled ? "true" : "false");
+}
+static DEVICE_ATTR_RW(enabled);
+
+static ssize_t enabled_index_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ return sysfs_emit(buf, "true false\n");
+}
+static DEVICE_ATTR_RO(enabled_index);
+
+static ssize_t speed_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct led_classdev *led_cdev = dev_get_drvdata(dev);
+ struct led_classdev_mc *led_mc = container_of(led_cdev, struct led_classdev_mc, led_cdev);
+ struct claw_drvdata *drvdata = container_of(led_mc, struct claw_drvdata, led_mc);
+ unsigned int val, speed;
+ int ret;
+
+ scoped_guard(spinlock_irqsave, &drvdata->registration_lock) {
+ /* Pairs with smp_store_release from cfg_setup_fn in system_wq context */
+ if (!smp_load_acquire(&drvdata->rgb_registered))
+ return -ENODEV;
+ }
+
+ ret = kstrtouint(buf, 10, &val);
+ if (ret)
+ return ret;
+
+ if (val > 20)
+ return -EINVAL;
+
+ /* 0 is fastest, invert value for intuitive userspace speed */
+ speed = 20 - val;
+
+ scoped_guard(spinlock_irqsave, &drvdata->profile_lock)
+ drvdata->rgb_speed = speed;
+
+ mod_delayed_work(system_wq, &drvdata->rgb_queue, msecs_to_jiffies(50));
+
+ return count;
+}
+
+static ssize_t speed_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ struct led_classdev *led_cdev = dev_get_drvdata(dev);
+ struct led_classdev_mc *led_mc = container_of(led_cdev, struct led_classdev_mc, led_cdev);
+ struct claw_drvdata *drvdata = container_of(led_mc, struct claw_drvdata, led_mc);
+ u8 speed = 20 - drvdata->rgb_speed;
+
+ scoped_guard(spinlock_irqsave, &drvdata->registration_lock) {
+ /* Pairs with smp_store_release from cfg_setup_fn in system_wq context */
+ if (!smp_load_acquire(&drvdata->rgb_registered))
+ return -ENODEV;
+ }
+
+ return sysfs_emit(buf, "%u\n", speed);
+}
+static DEVICE_ATTR_RW(speed);
+
+static ssize_t speed_range_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ return sysfs_emit(buf, "0-20\n");
+}
+static DEVICE_ATTR_RO(speed_range);
+
+static void claw_led_brightness_set(struct led_classdev *led_cdev,
+ enum led_brightness _brightness)
+{
+ struct led_classdev_mc *led_mc = container_of(led_cdev, struct led_classdev_mc, led_cdev);
+ struct claw_drvdata *drvdata = container_of(led_mc, struct claw_drvdata, led_mc);
+
+ scoped_guard(spinlock_irqsave, &drvdata->registration_lock) {
+ /* Pairs with smp_store_release from cfg_setup_fn in system_wq context */
+ if (!smp_load_acquire(&drvdata->rgb_registered))
+ return;
+ }
+
+ mod_delayed_work(system_wq, &drvdata->rgb_queue, msecs_to_jiffies(50));
+}
+
+static struct attribute *claw_rgb_attrs[] = {
+ &dev_attr_effect.attr,
+ &dev_attr_effect_index.attr,
+ &dev_attr_enabled.attr,
+ &dev_attr_enabled_index.attr,
+ &dev_attr_speed.attr,
+ &dev_attr_speed_range.attr,
+ NULL,
+};
+
+static const struct attribute_group claw_rgb_attr_group = {
+ .attrs = claw_rgb_attrs,
+};
+
+static struct mc_subled claw_rgb_subled_info[] = {
+ {
+ .color_index = LED_COLOR_ID_RED,
+ .channel = 0x1,
+ },
+ {
+ .color_index = LED_COLOR_ID_GREEN,
+ .channel = 0x2,
+ },
+ {
+ .color_index = LED_COLOR_ID_BLUE,
+ .channel = 0x3,
+ },
+};
+
static void cfg_setup_fn(struct work_struct *work)
{
struct delayed_work *dwork = container_of(work, struct delayed_work, work);
struct claw_drvdata *drvdata = container_of(dwork, struct claw_drvdata, cfg_setup);
+ bool gamepad_ready = false, rgb_ready = false, gp_registered, rgb_registered;
int ret;
ret = claw_hw_output_report(drvdata->hdev, CLAW_COMMAND_TYPE_READ_GAMEPAD_MODE,
NULL, 0, 25);
if (ret) {
dev_err(&drvdata->hdev->dev,
- "Failed to setup device, can't read gamepad mode: %d\n", ret);
- return;
+ "Failed to read gamepad mode: %d\n", ret);
+ goto prep_rgb;
}
+ gamepad_ready = true;
- /* Add sysfs attributes after we get the device state */
- ret = device_add_group(&drvdata->hdev->dev, &claw_gamepad_attr_group);
+prep_rgb:
+ ret = claw_read_rgb_config(drvdata->hdev);
if (ret) {
dev_err(&drvdata->hdev->dev,
- "Failed to setup device, can't create gamepad attrs: %d\n", ret);
- return;
+ "Failed to read RGB config: %d\n", ret);
+ goto try_gamepad;
}
+ rgb_ready = true;
+
+ /* Add sysfs attributes after we get the device state */
+try_gamepad:
scoped_guard(spinlock_irqsave, &drvdata->registration_lock)
- /* Pairs with smp_load_acquire in attribute show/store functions */
- smp_store_release(&drvdata->gp_registered, true);
+ /* Pairs with smp_store_release from below */
+ gp_registered = smp_load_acquire(&drvdata->gp_registered);
+
+ if (!gp_registered && gamepad_ready) {
+ ret = device_add_group(&drvdata->hdev->dev, &claw_gamepad_attr_group);
+ if (ret) {
+ dev_err(&drvdata->hdev->dev,
+ "Failed to create gamepad attrs: %d\n", ret);
+ goto try_rgb;
+ }
+
+ scoped_guard(spinlock_irqsave, &drvdata->registration_lock) {
+ /* Pairs with smp_load_acquire in attribute show/store functions */
+ smp_store_release(&drvdata->gp_registered, true);
+ gp_registered = true;
+ }
+ }
- kobject_uevent(&drvdata->hdev->dev.kobj, KOBJ_CHANGE);
+try_rgb:
+ /* Add and enable RGB interface once we have the device state */
+ scoped_guard(spinlock_irqsave, &drvdata->registration_lock)
+ /* Pairs with smp_store_release from below */
+ rgb_registered = smp_load_acquire(&drvdata->rgb_registered);
+
+ if (!rgb_registered && rgb_ready) {
+ ret = led_classdev_multicolor_register(&drvdata->hdev->dev,
+ &drvdata->led_mc);
+ if (ret) {
+ dev_err(&drvdata->hdev->dev, "Failed to create led device: %d\n", ret);
+ goto update_kobjects;
+ }
+
+ ret = device_add_group(drvdata->led_mc.led_cdev.dev, &claw_rgb_attr_group);
+ if (ret) {
+ dev_err(&drvdata->hdev->dev, "Failed to create RGB attrs: %d\n", ret);
+ led_classdev_multicolor_unregister(&drvdata->led_mc);
+ goto update_kobjects;
+ }
+
+ scoped_guard(spinlock_irqsave, &drvdata->registration_lock) {
+ /* Pairs with smp_load_acquire in attribute show/store functions */
+ smp_store_release(&drvdata->rgb_registered, true);
+ rgb_registered = true;
+ }
+ }
+
+update_kobjects:
+ if (gp_registered)
+ kobject_uevent(&drvdata->hdev->dev.kobj, KOBJ_CHANGE);
+ if (rgb_registered)
+ kobject_uevent(&drvdata->led_mc.led_cdev.dev->kobj, KOBJ_CHANGE);
}
static void cfg_resume_fn(struct work_struct *work)
@@ -973,8 +1589,10 @@ static void cfg_resume_fn(struct work_struct *work)
struct claw_drvdata *drvdata = container_of(dwork, struct claw_drvdata, cfg_resume);
guard(spinlock_irqsave)(&drvdata->registration_lock);
- /* Pairs with smp_store_release from cfg_setup_fn in system_wq context */
- if (!smp_load_acquire(&drvdata->gp_registered))
+ /* Pairs with smp_store_release from cfg_setup_fn in system_wq context */
+ if (!smp_load_acquire(&drvdata->gp_registered) ||
+ /* Pairs with smp_store_release from cfg_setup_fn in system_wq context */
+ !smp_load_acquire(&drvdata->rgb_registered))
schedule_delayed_work(&drvdata->cfg_setup, msecs_to_jiffies(500));
}
@@ -985,18 +1603,24 @@ static void claw_features_supported(struct claw_drvdata *drvdata)
if (major == 0x01) {
drvdata->bmap_support = true;
- if (minor >= 0x66)
+ if (minor >= 0x66) {
drvdata->bmap_addr = button_mapping_addr_new;
- else
+ drvdata->rgb_addr = rgb_addr_new;
+ } else {
drvdata->bmap_addr = button_mapping_addr_old;
+ drvdata->rgb_addr = rgb_addr_old;
+ }
return;
}
if ((major == 0x02 && minor >= 0x17) || major >= 0x03) {
drvdata->bmap_support = true;
drvdata->bmap_addr = button_mapping_addr_new;
+ drvdata->rgb_addr = rgb_addr_new;
return;
}
+
+ drvdata->rgb_addr = rgb_addr_old;
}
static int claw_probe(struct hid_device *hdev, u8 ep)
@@ -1011,6 +1635,7 @@ static int claw_probe(struct hid_device *hdev, u8 ep)
return -ENOMEM;
drvdata->gamepad_mode = CLAW_GAMEPAD_MODE_XINPUT;
+ drvdata->rgb_enabled = true;
drvdata->hdev = hdev;
drvdata->ep = ep;
@@ -1021,6 +1646,18 @@ static int claw_probe(struct hid_device *hdev, u8 ep)
if (!drvdata->bmap_support)
dev_dbg(&hdev->dev, "M-Key mapping is not supported. Update firmware to enable.\n");
+ /* Device is hardwired and name is guaranteed to be unique */
+ drvdata->led_mc.led_cdev.name = "msi_claw:rgb:joystick_rings";
+ drvdata->led_mc.led_cdev.brightness = 0x50;
+ drvdata->led_mc.led_cdev.max_brightness = 0x64;
+ drvdata->led_mc.led_cdev.color = LED_COLOR_ID_RGB;
+ drvdata->led_mc.led_cdev.brightness_set = claw_led_brightness_set;
+ drvdata->led_mc.num_colors = 3;
+ drvdata->led_mc.subled_info = devm_kmemdup(&hdev->dev, claw_rgb_subled_info,
+ sizeof(claw_rgb_subled_info), GFP_KERNEL);
+ if (!drvdata->led_mc.subled_info)
+ return -ENOMEM;
+
mutex_init(&drvdata->cfg_mutex);
mutex_init(&drvdata->profile_mutex);
mutex_init(&drvdata->rom_mutex);
@@ -1028,10 +1665,12 @@ static int claw_probe(struct hid_device *hdev, u8 ep)
spin_lock_init(&drvdata->cmd_lock);
spin_lock_init(&drvdata->mode_lock);
spin_lock_init(&drvdata->profile_lock);
+ spin_lock_init(&drvdata->frame_lock);
init_completion(&drvdata->orphan_ack_complete);
init_completion(&drvdata->send_cmd_complete);
INIT_DELAYED_WORK(&drvdata->cfg_resume, &cfg_resume_fn);
INIT_DELAYED_WORK(&drvdata->cfg_setup, &cfg_setup_fn);
+ INIT_DELAYED_WORK(&drvdata->rgb_queue, &claw_rgb_queue_fn);
/* For control interface: open the HID transport for sending commands. */
ret = hid_hw_open(hdev);
@@ -1088,6 +1727,7 @@ static void claw_remove(struct hid_device *hdev)
{
struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
bool gp_registered;
+ bool rgb_registered;
if (!drvdata)
return;
@@ -1100,11 +1740,21 @@ static void claw_remove(struct hid_device *hdev)
gp_registered = smp_load_acquire(&drvdata->gp_registered);
/* Pairs with smp_load_acquire in attribute show/store functions */
smp_store_release(&drvdata->gp_registered, false);
+ /* Pairs with smp_store_release from cfg_setup_fn in system_wq context */
+ rgb_registered = smp_load_acquire(&drvdata->rgb_registered);
+ /* Pairs with smp_load_acquire in attribute show/store functions */
+ smp_store_release(&drvdata->rgb_registered, false);
}
if (gp_registered)
device_remove_group(&hdev->dev, &claw_gamepad_attr_group);
+ if (rgb_registered) {
+ device_remove_group(drvdata->led_mc.led_cdev.dev, &claw_rgb_attr_group);
+ led_classdev_multicolor_unregister(&drvdata->led_mc);
+ }
+ cancel_delayed_work_sync(&drvdata->rgb_queue);
+
hid_hw_close(hdev);
}
@@ -1164,6 +1814,7 @@ static int claw_suspend(struct hid_device *hdev)
cancel_delayed_work_sync(&drvdata->cfg_resume);
cancel_delayed_work_sync(&drvdata->cfg_setup);
+ cancel_delayed_work_sync(&drvdata->rgb_queue);
return 0;
}
--
2.54.0
^ permalink raw reply related
* [PATCH v13 4/4] HID: hid-msi: Add Rumble Intensity Attributes
From: Derek J. Clark @ 2026-07-20 3:15 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: Pierre-Loup A . Griffais, Denis Benato, Zhouwang Huang,
Derek J . Clark, linux-input, linux-doc, linux-kernel
In-Reply-To: <20260720031549.2272658-1-derekjohn.clark@gmail.com>
Adds intensity adjustment for the left and right rumble motors.
Claude was used during the reverse-engineering data gathering for this
feature done by Zhouwang Huang. As the code had already been affected,
I used Claude to create the initial framing for the feature, then did
manual cleanup of the _show and _store functions afterwards to fix bugs
and keep the coding style consistent. Claude was also used as an initial
reviewer of this patch.
Assisted-by: Claude:claude-sonnet-4-6
Co-developed-by: Zhouwang Huang <honjow311@gmail.com>
Signed-off-by: Zhouwang Huang <honjow311@gmail.com>
Link: https://patch.msgid.link/20260529072111.7565-5-derekjohn.clark@gmail.com
Signed-off-by: Derek J. Clark <derekjohn.clark@gmail.com>
---
v12:
- On address mismatch, assume stale message return and keep waiting for
correct message.
- Use spinlock_irqsave for raw_event-reachable locks since completion
context isn't guaranteed softirq-only across all HCDs.
v11:
- Restore dropped changes from v10.
v7:
- Match on write address for rumble reports to prevent late ACK
from causing synchronization errors.
- Use spinlock for read/write profile_pending.
- Use smp_[store_release|load_acquire] pattern for checking
gamepad_registered to avoid possible races during teardown.
- Use struct for rumble reports.
v6:
- Make all timeouts 25ms to ensure at least 2 jiffies in a 100Hz
config.
- Add spinlock_irqsave for read/write access on rumble_intensity
variables.
- Gate all attribute show/store functions with gamepad_registered.
v5:
- Remove mkey related changes.
v2:
- Use pending_profile and sync to rom mutexes.
---
drivers/hid/hid-msi.c | 202 +++++++++++++++++++++++++++++++++++++++++-
1 file changed, 201 insertions(+), 1 deletion(-)
diff --git a/drivers/hid/hid-msi.c b/drivers/hid/hid-msi.c
index 5cb85fc2f053..3225d3bf6125 100644
--- a/drivers/hid/hid-msi.c
+++ b/drivers/hid/hid-msi.c
@@ -80,6 +80,8 @@ enum claw_profile_ack_pending {
CLAW_M1_PENDING,
CLAW_M2_PENDING,
CLAW_RGB_PENDING,
+ CLAW_RUMBLE_LEFT_PENDING,
+ CLAW_RUMBLE_RIGHT_PENDING,
};
enum claw_key_index {
@@ -272,6 +274,11 @@ static const u16 button_mapping_addr_new[] = {
static const u16 rgb_addr_old = 0x01fa;
static const u16 rgb_addr_new = 0x024a;
+static const u16 rumble_addr[] = {
+ 0x0022, /* left */
+ 0x0023, /* right */
+};
+
struct claw_command_report {
u8 report_id;
u8 padding[2];
@@ -314,6 +321,12 @@ struct claw_rgb_report {
struct rgb_frame zone_data;
} __packed;
+struct claw_rumble_report {
+ struct claw_profile_report;
+ u8 padding;
+ u8 intensity;
+} __packed;
+
struct claw_drvdata {
/* MCU General Variables */
enum claw_profile_ack_pending profile_pending;
@@ -339,8 +352,12 @@ struct claw_drvdata {
enum claw_gamepad_mode_index gamepad_mode;
u8 m1_codes[CLAW_KEYS_MAX];
u8 m2_codes[CLAW_KEYS_MAX];
- spinlock_t mode_lock; /* Lock for mode data read/write */
+ u8 rumble_intensity_right;
+ u8 rumble_intensity_left;
const u16 *bmap_addr;
+ spinlock_t rumble_lock; /* lock for rumble_intensity read/write */
+ spinlock_t mode_lock; /* Lock for mode data read/write */
+ bool rumble_support;
bool gp_registered;
bool bmap_support;
@@ -389,6 +406,7 @@ static int claw_gamepad_mode_event(struct claw_drvdata *drvdata,
static int claw_profile_event(struct claw_drvdata *drvdata, struct claw_command_report *cmd_rep)
{
enum claw_profile_ack_pending profile;
+ struct claw_rumble_report *rumble;
struct claw_mkey_report *mkeys;
struct claw_rgb_report *frame;
u16 rgb_addr, read_addr;
@@ -442,6 +460,20 @@ static int claw_profile_event(struct claw_drvdata *drvdata, struct claw_command_
}
break;
+ case CLAW_RUMBLE_LEFT_PENDING:
+ rumble = (struct claw_rumble_report *)cmd_rep->data;
+ if (be16_to_cpu(rumble->read_addr) != rumble_addr[0])
+ return -EAGAIN;
+ scoped_guard(spinlock_irqsave, &drvdata->rumble_lock)
+ drvdata->rumble_intensity_left = rumble->intensity;
+ break;
+ case CLAW_RUMBLE_RIGHT_PENDING:
+ rumble = (struct claw_rumble_report *)cmd_rep->data;
+ if (be16_to_cpu(rumble->read_addr) != rumble_addr[1])
+ return -EAGAIN;
+ scoped_guard(spinlock_irqsave, &drvdata->rumble_lock)
+ drvdata->rumble_intensity_right = rumble->intensity;
+ break;
default:
dev_dbg(&drvdata->hdev->dev,
"Got profile event without changes pending from command: %x\n",
@@ -991,6 +1023,162 @@ static ssize_t button_mapping_options_show(struct device *dev,
}
static DEVICE_ATTR_RO(button_mapping_options);
+static ssize_t rumble_intensity_left_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct claw_rumble_report report = { {0x01, cpu_to_be16(rumble_addr[0])}, 0x01 };
+ struct hid_device *hdev = to_hid_device(dev);
+ struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
+ u8 val;
+ int ret;
+
+ scoped_guard(spinlock_irqsave, &drvdata->registration_lock) {
+ /* Pairs with smp_store_release from cfg_setup_fn in system_wq context */
+ if (!smp_load_acquire(&drvdata->gp_registered))
+ return -ENODEV;
+ }
+
+ ret = kstrtou8(buf, 10, &val);
+ if (ret)
+ return ret;
+
+ if (val > 100)
+ return -EINVAL;
+
+ report.intensity = val;
+
+ guard(mutex)(&drvdata->rom_mutex);
+ ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_WRITE_PROFILE_DATA,
+ (u8 *)&report, sizeof(report), 25);
+ if (ret)
+ return ret;
+
+ /* MCU will not send ACK until the USB transaction completes. ACK is sent
+ * immediately after and will hit the stale state machine, before the next
+ * command re-arms the state machine. Timeout 0 ensures no deadlock waiting
+ * for ACK that ill never come.
+ */
+ ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_SYNC_TO_ROM, NULL, 0, 0);
+ if (ret)
+ return ret;
+
+ return count;
+}
+
+static ssize_t rumble_intensity_left_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct claw_rumble_report report = { {0x01, cpu_to_be16(rumble_addr[0])}, 0x01 };
+ struct hid_device *hdev = to_hid_device(dev);
+ struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
+ int ret;
+ u8 val;
+
+ scoped_guard(spinlock_irqsave, &drvdata->registration_lock) {
+ /* Pairs with smp_store_release from cfg_setup_fn in system_wq context */
+ if (!smp_load_acquire(&drvdata->gp_registered))
+ return -ENODEV;
+ }
+
+ guard(mutex)(&drvdata->profile_mutex);
+ scoped_guard(spinlock_irqsave, &drvdata->profile_lock)
+ drvdata->profile_pending = CLAW_RUMBLE_LEFT_PENDING;
+ ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_READ_PROFILE,
+ (u8 *)&report, sizeof(report), 25);
+ if (ret)
+ return ret;
+
+ scoped_guard(spinlock_irqsave, &drvdata->rumble_lock)
+ val = drvdata->rumble_intensity_left;
+
+ return sysfs_emit(buf, "%u\n", val);
+}
+static DEVICE_ATTR_RW(rumble_intensity_left);
+
+static ssize_t rumble_intensity_right_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct claw_rumble_report report = { {0x01, cpu_to_be16(rumble_addr[1])}, 0x01 };
+ struct hid_device *hdev = to_hid_device(dev);
+ struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
+ u8 val;
+ int ret;
+
+ scoped_guard(spinlock_irqsave, &drvdata->registration_lock) {
+ /* Pairs with smp_store_release from cfg_setup_fn in system_wq context */
+ if (!smp_load_acquire(&drvdata->gp_registered))
+ return -ENODEV;
+ }
+
+ ret = kstrtou8(buf, 10, &val);
+ if (ret)
+ return ret;
+
+ if (val > 100)
+ return -EINVAL;
+
+ report.intensity = val;
+
+ guard(mutex)(&drvdata->rom_mutex);
+ ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_WRITE_PROFILE_DATA,
+ (u8 *)&report, sizeof(report), 25);
+ if (ret)
+ return ret;
+
+ /* MCU will not send ACK until the USB transaction completes. ACK is sent
+ * immediately after and will hit the stale state machine, before the next
+ * command re-arms the state machine. Timeout 0 ensures no deadlock waiting
+ * for ACK that ill never come.
+ */
+ ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_SYNC_TO_ROM, NULL, 0, 0);
+ if (ret)
+ return ret;
+
+ return count;
+}
+
+static ssize_t rumble_intensity_right_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct claw_rumble_report report = { {0x01, cpu_to_be16(rumble_addr[1])}, 0x01 };
+ struct hid_device *hdev = to_hid_device(dev);
+ struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
+ int ret;
+ u8 val;
+
+ scoped_guard(spinlock_irqsave, &drvdata->registration_lock) {
+ /* Pairs with smp_store_release from cfg_setup_fn in system_wq context */
+ if (!smp_load_acquire(&drvdata->gp_registered))
+ return -ENODEV;
+ }
+
+ guard(mutex)(&drvdata->profile_mutex);
+ scoped_guard(spinlock_irqsave, &drvdata->profile_lock)
+ drvdata->profile_pending = CLAW_RUMBLE_RIGHT_PENDING;
+ ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_READ_PROFILE,
+ (u8 *)&report, sizeof(report), 25);
+ if (ret)
+ return ret;
+
+ scoped_guard(spinlock_irqsave, &drvdata->rumble_lock)
+ val = drvdata->rumble_intensity_right;
+
+ return sysfs_emit(buf, "%u\n", val);
+}
+static DEVICE_ATTR_RW(rumble_intensity_right);
+
+static ssize_t rumble_intensity_range_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ return sysfs_emit(buf, "0-100\n");
+}
+static DEVICE_ATTR_RO(rumble_intensity_range);
+
static umode_t claw_gamepad_attr_is_visible(struct kobject *kobj, struct attribute *attr,
int n)
{
@@ -1011,6 +1199,12 @@ static umode_t claw_gamepad_attr_is_visible(struct kobject *kobj, struct attribu
attr == &dev_attr_reset.attr)
return attr->mode;
+ /* Hide rumble attrs if not supported */
+ if (attr == &dev_attr_rumble_intensity_left.attr ||
+ attr == &dev_attr_rumble_intensity_right.attr ||
+ attr == &dev_attr_rumble_intensity_range.attr)
+ return drvdata->rumble_support ? attr->mode : 0;
+
/* Hide button mapping attrs if it isn't supported */
return drvdata->bmap_support ? attr->mode : 0;
}
@@ -1024,6 +1218,9 @@ static struct attribute *claw_gamepad_attrs[] = {
&dev_attr_mkeys_function.attr,
&dev_attr_mkeys_function_index.attr,
&dev_attr_reset.attr,
+ &dev_attr_rumble_intensity_left.attr,
+ &dev_attr_rumble_intensity_right.attr,
+ &dev_attr_rumble_intensity_range.attr,
NULL,
};
@@ -1605,6 +1802,7 @@ static void claw_features_supported(struct claw_drvdata *drvdata)
drvdata->bmap_support = true;
if (minor >= 0x66) {
drvdata->bmap_addr = button_mapping_addr_new;
+ drvdata->rumble_support = true;
drvdata->rgb_addr = rgb_addr_new;
} else {
drvdata->bmap_addr = button_mapping_addr_old;
@@ -1616,6 +1814,7 @@ static void claw_features_supported(struct claw_drvdata *drvdata)
if ((major == 0x02 && minor >= 0x17) || major >= 0x03) {
drvdata->bmap_support = true;
drvdata->bmap_addr = button_mapping_addr_new;
+ drvdata->rumble_support = true;
drvdata->rgb_addr = rgb_addr_new;
return;
}
@@ -1666,6 +1865,7 @@ static int claw_probe(struct hid_device *hdev, u8 ep)
spin_lock_init(&drvdata->mode_lock);
spin_lock_init(&drvdata->profile_lock);
spin_lock_init(&drvdata->frame_lock);
+ spin_lock_init(&drvdata->rumble_lock);
init_completion(&drvdata->orphan_ack_complete);
init_completion(&drvdata->send_cmd_complete);
INIT_DELAYED_WORK(&drvdata->cfg_resume, &cfg_resume_fn);
--
2.54.0
^ permalink raw reply related
* [PATCH v5] Input: gunze: replace deprecated APIs and fix warning style
From: Bivash Kumar Singh @ 2026-07-20 3:24 UTC (permalink / raw)
To: linux-input; +Cc: dmitry.torokhov, Bivash Kumar Singh
In-Reply-To: <20260718190040.10613-1-bivashraj750@gmail.com>
Replace printk(KERN_WARNING) with dev_warn_ratelimited() using the
serio device, which is the correct logging style for driver code and
prevents log spam on noisy serial lines.
Replace simple_strtoul() with sscanf() to parse the comma-separated
coordinate data. NUL-terminate the receive buffer in gunze_interrupt()
before calling gunze_process_packet() so sscanf() and the warning
message can safely treat the data as a string.
Signed-off-by: Bivash Kumar Singh <bivashraj750@gmail.com>
---
Changes in v5:
- Switch from kstrtoul() to sscanf() as suggested by Dmitry Torokhov.
sscanf() naturally handles comma-separated coordinate data without
needing a local buffer copy or manual NUL termination tricks.
- NUL-terminate receive buffer in gunze_interrupt() before processing.
- Use dev_warn_ratelimited() to prevent log spam on noisy serial lines.
Changes in v4:
- Remove early return on kstrtoul() failure to prevent touch state
getting permanently stuck if a release packet contains noisy data.
Initialize x and y to 0 as safe fallback values instead.
(reported by Sashiko AI review)
Changes in v3:
- Fix comment style: add space after /* and use NUL instead of NULL
- Add missing Changes section that was absent in v2
Changes in v2:
- Copy packet data to a local NUL-terminated buffer before calling
kstrtoul(), so the comma separator does not cause parsing to fail
on every valid touch event. (reported by Sashiko AI review)
---
drivers/input/touchscreen/gunze.c | 16 +++++++++++-----
1 file changed, 11 insertions(+), 5 deletions(-)
diff --git a/drivers/input/touchscreen/gunze.c b/drivers/input/touchscreen/gunze.c
index 2baeb4f3b941..7a92052eba80 100644
--- a/drivers/input/touchscreen/gunze.c
+++ b/drivers/input/touchscreen/gunze.c
@@ -41,15 +41,20 @@ struct gunze {
static void gunze_process_packet(struct gunze *gunze)
{
struct input_dev *dev = gunze->dev;
+ unsigned int x, y;
- if (gunze->idx != GUNZE_MAX_LENGTH || gunze->data[5] != ',' ||
- (gunze->data[0] != 'T' && gunze->data[0] != 'R')) {
- printk(KERN_WARNING "gunze.c: bad packet: >%.*s<\n", GUNZE_MAX_LENGTH, gunze->data);
+ if (gunze->data[0] != 'T' && gunze->data[0] != 'R') {
+ dev_warn_ratelimited(&gunze->serio->dev, "bad packet: >%s<\n", gunze->data);
return;
}
- input_report_abs(dev, ABS_X, simple_strtoul(gunze->data + 1, NULL, 10));
- input_report_abs(dev, ABS_Y, 1024 - simple_strtoul(gunze->data + 6, NULL, 10));
+ if (sscanf(gunze->data + 1, "%4u,%4u", &x, &y) != 2) {
+ dev_warn_ratelimited(&gunze->serio->dev, "bad packet: >%s<\n", gunze->data);
+ return;
+ }
+
+ input_report_abs(dev, ABS_X, x);
+ input_report_abs(dev, ABS_Y, 1024 - y);
input_report_key(dev, BTN_TOUCH, gunze->data[0] == 'T');
input_sync(dev);
}
@@ -60,6 +65,7 @@ static irqreturn_t gunze_interrupt(struct serio *serio,
struct gunze *gunze = serio_get_drvdata(serio);
if (data == '\r') {
+ gunze->data[gunze->idx] = '\0';
gunze_process_packet(gunze);
gunze->idx = 0;
} else {
--
2.53.0
^ permalink raw reply related
* Re: [PATCH v13 1/4] HID: hid-msi: Add MSI Claw configuration driver
From: sashiko-bot @ 2026-07-20 3:27 UTC (permalink / raw)
To: Derek J. Clark; +Cc: linux-input, dmitry.torokhov
In-Reply-To: <20260720031549.2272658-2-derekjohn.clark@gmail.com>
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] Delayed generic ACKs from a previous timed-out command can falsely complete a subsequent command expecting a specific ACK, leading to silent state corruption.
- [Medium] Mixing `goto`-based error handling with scope-based `__free()` cleanup in the same function.
- [Medium] Sysfs attributes can spuriously return `-ENODEV` immediately after creation because they are exposed before `gp_registered` is set to true.
--
--- Patch [1]: [PATCH v13 1/4] HID: hid-msi: Add MSI Claw configuration driver ---
commit 12a4a941e97290e1a15080092de490d5782eef1c
Author: Derek J. Clark <derekjohn.clark@gmail.com>
HID: hid-msi: Add MSI Claw configuration driver
Adds configuration HID driver for the MSI Claw series of handheld PC's.
In this initial patch add the initial driver outline and attributes for
changing the gamepad mode, M-key behavior, and add a WO reset function.
> diff --git a/drivers/hid/hid-msi.c b/drivers/hid/hid-msi.c
> new file mode 100644
> --- /dev/null
> +++ b/drivers/hid/hid-msi.c
[ ... ]
> +static int claw_raw_event(struct claw_drvdata *drvdata, struct hid_report *report,
> + u8 *data, int size)
> +{
[ ... ]
> + switch (cmd_rep->cmd) {
> + case CLAW_COMMAND_TYPE_GAMEPAD_MODE_ACK:
> + ret = claw_gamepad_mode_event(drvdata, cmd_rep);
> + if (drvdata->waiting_cmd == CLAW_COMMAND_TYPE_READ_GAMEPAD_MODE) {
> + drvdata->cmd_status = ret;
> + complete(&drvdata->send_cmd_complete);
> + }
> +
> + break;
> + case CLAW_COMMAND_TYPE_ACK:
> + if (drvdata->orphan_ack_pending) {
> + drvdata->orphan_ack_pending = false;
> + complete(&drvdata->orphan_ack_complete);
> + break;
> + }
> +
> + if (drvdata->waiting_cmd == CLAW_COMMAND_TYPE_NONE) {
> + dev_warn(&drvdata->hdev->dev, "Got unexpected ACK from MCU, ignoring\n");
> + break;
> + }
> +
> + drvdata->cmd_status = 0;
> + complete(&drvdata->send_cmd_complete);
[Severity: Medium]
When claw_raw_event() receives a generic ACK, it only verifies that
waiting_cmd is not CLAW_COMMAND_TYPE_NONE before calling
complete(&drvdata->send_cmd_complete).
If a previous command with timeout == 0 timed out, __claw_hw_output_report()
clears orphan_ack_pending.
If the generic ACK for that command arrives later while a new command is
waiting (e.g., waiting for GAMEPAD_MODE_ACK), this logic will accept the
generic ACK and falsely complete the new command.
Could this lead to silent state corruption where commands finish early without
receiving their actual payloads?
[ ... ]
> +static int __claw_hw_output_report(struct hid_device *hdev, u8 index, u8 *data,
> + size_t len, unsigned int timeout)
> +{
> + unsigned char *dmabuf __free(kfree) = NULL;
[ ... ]
> + ret = hid_hw_output_report(hdev, dmabuf, CLAW_PACKET_SIZE);
> + if (ret < 0)
> + goto err;
> +
> + ret = ret == CLAW_PACKET_SIZE ? 0 : -EIO;
> + if (ret)
> + goto err;
[Severity: Medium]
This function uses the scope-based __free(kfree) cleanup for dmabuf, but also
uses manual goto err labels for error handling.
The cleanup subsystem guidelines specify that using goto and cleanup helpers
like __free() in the same function creates confusing ownership semantics and is
an anti-pattern.
Could we use a scope-based guard for the state machine cleanup, or stick
entirely to manual goto cleanup?
[ ... ]
> +static void cfg_setup_fn(struct work_struct *work)
> +{
[ ... ]
> + /* Add sysfs attributes after we get the device state */
> + ret = device_add_group(&drvdata->hdev->dev, &claw_gamepad_attr_group);
> + if (ret) {
> + dev_err(&drvdata->hdev->dev,
> + "Failed to setup device, can't create gamepad attrs: %d\n", ret);
> + return;
> + }
> + scoped_guard(spinlock_irqsave, &drvdata->registration_lock)
> + /* Pairs with smp_load_acquire in attribute show/store functions */
> + smp_store_release(&drvdata->gp_registered, true);
[Severity: Medium]
The sysfs attributes are exposed to userspace by device_add_group() before
gp_registered is set to true.
If an automated userspace tool reacts to the sysfs creation event and accesses
the attributes in this small timing window, the attribute show/store functions
will observe gp_registered == false and return -ENODEV.
Would it be better to set gp_registered to true before calling
device_add_group(), and revert it to false if the addition fails?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260720031549.2272658-1-derekjohn.clark@gmail.com?part=1
^ permalink raw reply
* Re: [PATCH v13 3/4] HID: hid-msi: Add RGB control interface
From: sashiko-bot @ 2026-07-20 3:30 UTC (permalink / raw)
To: Derek J. Clark; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260720031549.2272658-4-derekjohn.clark@gmail.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues:
- [Medium] Data races and locking mismatches when accessing RGB configuration variables and LED core state.
Pre-existing issues:
- [High] Use-after-free on device teardown due to missing cancellation of delayed works.
--
--- Patch [3]: [PATCH v13 3/4] HID: hid-msi: Add RGB control interface ---
commit d67de53a026c42fe9260a66fca23392240a52c26
Author: Derek J. Clark <derekjohn.clark@gmail.com>
HID: hid-msi: Add RGB control interface
This commit adds an RGB control interface for MSI Claw devices. It provides
support for configuring up to 9 LED zones via an MCU memory interface and
sets up the corresponding sysfs attributes and LED class device.
Link: https://patch.msgid.link/20260529072111.7565-4-derekjohn.clark@gmail.com
> diff --git a/drivers/hid/hid-msi.c b/drivers/hid/hid-msi.c
> index 023d63cee6b06..5cb85fc2f0534 100644
> --- a/drivers/hid/hid-msi.c
> +++ b/drivers/hid/hid-msi.c
[ ... ]
> @@ -349,6 +410,38 @@ static int claw_profile_event(struct claw_drvdata *drvdata, struct claw_command_
> for (i = 0; i < CLAW_KEYS_MAX; i++)
> codes[i] = (mkeys->codes[i]);
> break;
> + case CLAW_RGB_PENDING:
> + frame = (struct claw_rgb_report *)cmd_rep->data;
[ ... ]
> + scoped_guard(spinlock_irqsave, &drvdata->frame_lock) {
> + memcpy(&drvdata->rgb_frames[f_idx], &frame->zone_data,
> + sizeof(struct rgb_frame));
> +
> + /* Only use frame 0 for remaining variable assignment */
> + if (f_idx != 0)
> + break;
> +
> + drvdata->rgb_speed = frame->speed;
> + drvdata->led_mc.led_cdev.brightness = frame->brightness;
> + drvdata->led_mc.subled_info[0].intensity = frame->zone_data.zone[0].red;
> + drvdata->led_mc.subled_info[1].intensity = frame->zone_data.zone[0].green;
> + drvdata->led_mc.subled_info[2].intensity = frame->zone_data.zone[0].blue;
> + }
> +
> + break;
[Severity: Medium]
Is there a locking mismatch for rgb_speed? Here in claw_profile_event(),
rgb_speed is updated while holding frame_lock. However, in speed_store(),
it is updated while holding profile_lock.
Also, does this direct modification of the LED class device internal state
need synchronization? Updating brightness and subled_info[x].intensity here
is done without holding the standard led_access lock used by the LED core,
which could cause data races with concurrent sysfs accesses.
[ ... ]
> +/* Send RGB configuration to device */
> +static int claw_write_rgb_state(struct claw_drvdata *drvdata)
> +{
> + struct claw_rgb_report report = { {0x01, 0}, CLAW_RGB_FRAME_OFFSET, 0x00,
> + drvdata->rgb_frame_count, 0x09, drvdata->rgb_speed,
> + drvdata->led_mc.led_cdev.brightness };
[Severity: Medium]
Does claw_write_rgb_state() need to hold a lock when reading rgb_speed?
It appears to be read locklessly here, but is updated with a lock elsewhere.
This also applies to other variables like rgb_effect and rgb_enabled read
locklessly in claw_apply_rgb_state() and sysfs show functions.
[ ... ]
> +static ssize_t speed_store(struct device *dev, struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
[ ... ]
> + /* 0 is fastest, invert value for intuitive userspace speed */
> + speed = 20 - val;
> +
> + scoped_guard(spinlock_irqsave, &drvdata->profile_lock)
> + drvdata->rgb_speed = speed;
> +
> + mod_delayed_work(system_wq, &drvdata->rgb_queue, msecs_to_jiffies(50));
> +
> + return count;
> +}
[Severity: Medium]
This is where rgb_speed is written using profile_lock instead of the
frame_lock used in claw_profile_event().
[ ... ]
> @@ -1088,6 +1727,7 @@ static void claw_remove(struct hid_device *hdev)
> {
> struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
> bool gp_registered;
> + bool rgb_registered;
>
> if (!drvdata)
> return;
[ ... ]
> if (gp_registered)
> device_remove_group(&hdev->dev, &claw_gamepad_attr_group);
>
> + if (rgb_registered) {
> + device_remove_group(drvdata->led_mc.led_cdev.dev, &claw_rgb_attr_group);
> + led_classdev_multicolor_unregister(&drvdata->led_mc);
> + }
> + cancel_delayed_work_sync(&drvdata->rgb_queue);
> +
> hid_hw_close(hdev);
> }
[Severity: High]
This is a pre-existing issue, but does this teardown path leak delayed works?
The driver initializes the cfg_setup and cfg_resume delayed works during
probe, but they don't appear to be cancelled here in claw_remove().
If the device is removed while either of those works is pending, could they
execute after the devres-allocated drvdata is freed, causing a
use-after-free crash?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260720031549.2272658-1-derekjohn.clark@gmail.com?part=3
^ permalink raw reply
* Re: [PATCH v13 4/4] HID: hid-msi: Add Rumble Intensity Attributes
From: sashiko-bot @ 2026-07-20 3:30 UTC (permalink / raw)
To: Derek J. Clark; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260720031549.2272658-5-derekjohn.clark@gmail.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The function `__claw_hw_output_report` violates the cleanup subsystem rules by mixing `goto`-based error handling with scope-based cleanup (`__free(kfree)`).
--
--- Patch [4]: [PATCH v13 4/4] HID: hid-msi: Add Rumble Intensity Attributes ---
commit 9f513aab0b37d6357132f5ca7a76ad690510bbc7
Author: Derek J. Clark <derekjohn.clark@gmail.com>
HID: hid-msi: Add Rumble Intensity Attributes
This commit adds intensity adjustment attributes for the left and right rumble
motors on the MSI Claw device. It introduces sysfs store and show functions
to read and write rumble profile data to the hardware.
Link: https://patch.msgid.link/20260529072111.7565-5-derekjohn.clark@gmail.com
> diff --git a/drivers/hid/hid-msi.c b/drivers/hid/hid-msi.c
> index 5cb85fc2f0534..3225d3bf61254 100644
> --- a/drivers/hid/hid-msi.c
> +++ b/drivers/hid/hid-msi.c
[ ... ]
> @@ -991,6 +1023,162 @@ static ssize_t button_mapping_options_show(struct device *dev,
> }
> static DEVICE_ATTR_RO(button_mapping_options);
>
> +static ssize_t rumble_intensity_left_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
[ ... ]
> + ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_WRITE_PROFILE_DATA,
> + (u8 *)&report, sizeof(report), 25);
> + if (ret)
> + return ret;
[Severity: Low]
Does __claw_hw_output_report() mix goto-based error handling with scope-based
cleanup?
Looking at __claw_hw_output_report(), which is called via
claw_hw_output_report() here:
unsigned char *dmabuf __free(kfree) = NULL;
...
ret = hid_hw_output_report(hdev, dmabuf, CLAW_PACKET_SIZE);
if (ret < 0)
goto err;
...
err:
scoped_guard(spinlock_irqsave, &drvdata->cmd_lock) {
drvdata->waiting_cmd = CLAW_COMMAND_TYPE_NONE;
drvdata->orphan_ack_pending = false;
}
return ret;
The cleanup subsystem guidelines in include/linux/cleanup.h state that the
expectation is to never mix "goto" and cleanup helpers in the same function.
Is it possible to convert the error path to use scope-based cleanup entirely?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260720031549.2272658-1-derekjohn.clark@gmail.com?part=4
^ permalink raw reply
* [PATCH] Input: byd - synchronize timer deletion before freeing private data
From: Linmao Li @ 2026-07-20 3:30 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Kees Cook, Vivek BalachandharTN, Richard Pospesel, Chris Diamand,
linux-input, linux-kernel, Linmao Li, stable
byd_disconnect() uses timer_delete() before freeing the driver's private
data. This does not wait for a running byd_clear_touch() callback, which
dereferences the private data and its psmouse pointer. A callback racing
with disconnect can therefore access the private data after it has been
freed.
Use timer_delete_sync() to wait for any running callback before releasing
the private data.
Fixes: 2d5f5611dd0d ("Input: byd - enable absolute mode")
Cc: stable@vger.kernel.org
Signed-off-by: Linmao Li <lilinmao@kylinos.cn>
---
drivers/input/mouse/byd.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/input/mouse/byd.c b/drivers/input/mouse/byd.c
index f5770a3af2f1..7a69724c1ac6 100644
--- a/drivers/input/mouse/byd.c
+++ b/drivers/input/mouse/byd.c
@@ -423,7 +423,7 @@ static void byd_disconnect(struct psmouse *psmouse)
struct byd_data *priv = psmouse->private;
if (priv) {
- timer_delete(&priv->timer);
+ timer_delete_sync(&priv->timer);
kfree(psmouse->private);
psmouse->private = NULL;
}
--
2.25.1
^ permalink raw reply related
* Re: [PATCH v5] Input: gunze: replace deprecated APIs and fix warning style
From: sashiko-bot @ 2026-07-20 3:32 UTC (permalink / raw)
To: Bivash Kumar Singh; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260720032407.4644-1-bivashraj750@gmail.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Off-by-one buffer overflow when NUL-terminating the receive buffer corrupts the device physical path.
- [High] Early return on coordinate parsing failure drops malformed release packets, permanently sticking the touch state.
--
--- Patch [1]: [PATCH v5] Input: gunze: replace deprecated APIs and fix warning style ---
commit e46d9a0baa3ae281ee5b9c02aad4e7f63a5cf33f
Author: Bivash Kumar Singh <bivashraj750@gmail.com>
Input: gunze: replace deprecated APIs and fix warning style
This commit updates the driver to use dev_warn_ratelimited() instead of
printk() and replaces simple_strtoul() with sscanf() for parsing coordinate
data. It also NUL-terminates the receive buffer to allow safe string parsing.
> diff --git a/drivers/input/touchscreen/gunze.c b/drivers/input/touchscreen/gunze.c
> index 2baeb4f3b941c..7a92052eba805 100644
> --- a/drivers/input/touchscreen/gunze.c
> +++ b/drivers/input/touchscreen/gunze.c
> @@ -41,15 +41,20 @@ struct gunze {
> static void gunze_process_packet(struct gunze *gunze)
> {
> struct input_dev *dev = gunze->dev;
> + unsigned int x, y;
>
> - if (gunze->idx != GUNZE_MAX_LENGTH || gunze->data[5] != ',' ||
> - (gunze->data[0] != 'T' && gunze->data[0] != 'R')) {
> - printk(KERN_WARNING "gunze.c: bad packet: >%.*s<\n", GUNZE_MAX_LENGTH, gunze->data);
> + if (gunze->data[0] != 'T' && gunze->data[0] != 'R') {
> + dev_warn_ratelimited(&gunze->serio->dev, "bad packet: >%s<\n", gunze->data);
> return;
> }
>
> - input_report_abs(dev, ABS_X, simple_strtoul(gunze->data + 1, NULL, 10));
> - input_report_abs(dev, ABS_Y, 1024 - simple_strtoul(gunze->data + 6, NULL, 10));
> + if (sscanf(gunze->data + 1, "%4u,%4u", &x, &y) != 2) {
> + dev_warn_ratelimited(&gunze->serio->dev, "bad packet: >%s<\n", gunze->data);
> + return;
> + }
[Severity: High]
Does this early return risk leaving the touch state permanently stuck?
If a release packet (starting with 'R') is received but the coordinate data
contains noise causing sscanf() to fail, this will return early and skip the
input_report_key() call below, dropping the release event.
> +
> + input_report_abs(dev, ABS_X, x);
> + input_report_abs(dev, ABS_Y, 1024 - y);
> input_report_key(dev, BTN_TOUCH, gunze->data[0] == 'T');
> input_sync(dev);
> }
> @@ -60,6 +65,7 @@ static irqreturn_t gunze_interrupt(struct serio *serio,
> struct gunze *gunze = serio_get_drvdata(serio);
>
> if (data == '\r') {
> + gunze->data[gunze->idx] = '\0';
[Severity: High]
Can this write past the end of the data array and corrupt the physical path?
The data array in struct gunze is sized exactly to GUNZE_MAX_LENGTH (10):
struct gunze {
...
unsigned char data[GUNZE_MAX_LENGTH];
char phys[32];
};
When a full-length valid touch packet is received followed by a '\r',
gunze->idx will be 10. Writing '\0' at this index will write out of bounds
and overwrite the adjacent phys array. Could the array size be increased to
accommodate the NUL terminator?
> gunze_process_packet(gunze);
> gunze->idx = 0;
> } else {
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260720032407.4644-1-bivashraj750@gmail.com?part=1
^ permalink raw reply
* Re: [PATCH v13 2/4] HID: hid-msi: Add M-key mapping attributes
From: sashiko-bot @ 2026-07-20 3:32 UTC (permalink / raw)
To: Derek J. Clark; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260720031549.2272658-3-derekjohn.clark@gmail.com>
Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
New issues:
- [High] Race condition in `claw_profile_event` causes stale data to be returned for profile reads.
- [Low] Compilation error due to invalid C syntax for embedded tagged struct.
- [Low] Subsystem guide violation: Mixing `goto`-based cleanup labels with `__free()` automated cleanup.
Pre-existing issues:
- [High] Generic ACK responses improperly complete commands waiting for specific data payloads.
- [High] Gamepad sysfs attributes are permanently hidden and never created.
--
--- Patch [2]: [PATCH v13 2/4] HID: hid-msi: Add M-key mapping attributes ---
commit 4c48f895b57157a8a075f47f1218b8052e864f1b
Author: Derek J. Clark <derekjohn.clark@gmail.com>
HID: hid-msi: Add M-key mapping attributes
This patch adds attributes that allow for remapping the M-keys on MSI
devices when in macro mode. It routes mapped buttons to appropriate event
devices for gamepad, keyboard, or mouse events.
Link: https://patch.msgid.link/20260529072111.7565-3-derekjohn.clark@gmail.com
> diff --git a/drivers/hid/hid-msi.c b/drivers/hid/hid-msi.c
> index 6687e4579faa1..023d63cee6b06 100644
> --- a/drivers/hid/hid-msi.c
> +++ b/drivers/hid/hid-msi.c
> @@ -255,6 +255,15 @@ struct claw_command_report {
> u8 data[59];
> } __packed;
>
> +struct claw_profile_report {
> + u8 profile;
> + __be16 read_addr;
> +} __packed;
> +
> +struct claw_mkey_report {
> + struct claw_profile_report;
> + u8 padding_0;
[Severity: Low]
Will this compile successfully, or does this introduce a regression?
Standard C11 only permits anonymous structs if they do not have a tag. Using
a tagged struct (like struct claw_profile_report;) without a field name acts
as a forward declaration.
This makes padding_0 the first member, causing the initialization in
claw_mkey_store() to fail, and attempts to access mkeys->read_addr in
claw_profile_event() will fail with a missing member error.
[ ... ]
> +static int claw_profile_event(struct claw_drvdata *drvdata, struct claw_command_report *cmd_rep)
> +{
> + enum claw_profile_ack_pending profile;
> + struct claw_mkey_report *mkeys;
> + u8 *codes, key;
> + int i;
> +
> + scoped_guard(spinlock_irqsave, &drvdata->profile_lock)
> + profile = drvdata->profile_pending;
> +
> + switch (profile) {
> + case CLAW_M1_PENDING:
[ ... ]
> + default:
> + dev_dbg(&drvdata->hdev->dev,
> + "Got profile event without changes pending from command: %x\n",
> + cmd_rep->cmd);
> + return -EINVAL;
> + }
> + scoped_guard(spinlock_irqsave, &drvdata->profile_lock)
> + drvdata->profile_pending = CLAW_NO_PENDING;
> +
> + return 0;
> +}
[Severity: High]
Is there a race condition here that introduces a regression where stale
data is returned?
The lock is dropped after reading profile_pending, opening a race window
where another thread could update it. When this thread resumes and
unconditionally writes CLAW_NO_PENDING, it could destroy pending profile
requests initiated during the race window.
For example, if Thread 1 times out leaving CLAW_M1_PENDING, and Thread 2
sets CLAW_M2_PENDING, a delayed hardware response for M1 could cause this
function to overwrite Thread 2's state with CLAW_NO_PENDING. This would
cause Thread 2 to return unmodified M2 data.
[ ... ]
> static int claw_raw_event(struct claw_drvdata *drvdata, struct hid_report *report,
> u8 *data, int size)
> {
[ ... ]
> case CLAW_COMMAND_TYPE_ACK:
> if (drvdata->orphan_ack_pending) {
> drvdata->orphan_ack_pending = false;
> complete(&drvdata->orphan_ack_complete);
> break;
> }
>
> if (drvdata->waiting_cmd == CLAW_COMMAND_TYPE_NONE) {
> dev_warn(&drvdata->hdev->dev, "Got unexpected ACK from MCU, ignoring\n");
> break;
> }
>
> drvdata->cmd_status = 0;
> complete(&drvdata->send_cmd_complete);
[Severity: High]
This is a pre-existing issue, but do generic ACKs improperly complete
commands waiting for specific data payloads?
If a previous command (like SYNC_TO_ROM with a timeout of 0) takes longer
than the timeout to send its ACK, the orphan_ack_pending flag might clear
before the generic ACK arrives.
When the delayed generic ACK finally arrives while waiting_cmd is set for
a new command (like READ_PROFILE), this branch skips the orphan check and
unconditionally triggers complete(&drvdata->send_cmd_complete). This
prematurely completes the new command's wait, potentially returning stale
or uninitialized data.
[ ... ]
> static int __claw_hw_output_report(struct hid_device *hdev, u8 index, u8 *data,
> size_t len, unsigned int timeout)
> {
> unsigned char *dmabuf __free(kfree) = NULL;
[ ... ]
> err:
> scoped_guard(spinlock_irqsave, &drvdata->cmd_lock) {
> drvdata->waiting_cmd = CLAW_COMMAND_TYPE_NONE;
> drvdata->orphan_ack_pending = false;
> }
> return ret;
> }
[Severity: Low]
This is a pre-existing issue, but does this function mix goto-based cleanup
labels with __free() automated cleanup?
The function declares unsigned char *dmabuf __free(kfree) = NULL; but also
utilizes a goto err; label for state cleanup. The cleanup and guard
subsystem guidelines state that functions should either convert all
resources to scope-based cleanup or convert none of them.
[ ... ]
> static umode_t claw_gamepad_attr_is_visible(struct kobject *kobj, struct attribute *attr,
> int n)
> {
> struct hid_device *hdev = to_hid_device(kobj_to_dev(kobj));
> struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
>
> scoped_guard(spinlock_irqsave, &drvdata->registration_lock) {
> /* Pairs with smp_store_release from cfg_setup_fn in system_wq context */
> if (!smp_load_acquire(&drvdata->gp_registered))
> return 0;
> }
[Severity: High]
This is a pre-existing issue, but are the gamepad sysfs attributes ever
created and made visible?
In cfg_setup_fn(), device_add_group() is called before gp_registered is
set to true. During sysfs group creation, this callback evaluates
!smp_load_acquire(&drvdata->gp_registered) and returns 0.
Because it returns 0 during creation, the files are never created. Even
though gp_registered is set to true immediately afterward, the attributes
will remain permanently hidden since sysfs_update_group is not called.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260720031549.2272658-1-derekjohn.clark@gmail.com?part=2
^ permalink raw reply
* Re: [PATCH] Input: byd - synchronize timer deletion before freeing private data
From: sashiko-bot @ 2026-07-20 3:48 UTC (permalink / raw)
To: Linmao Li; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260720033030.1580891-1-lilinmao@kylinos.cn>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues:
- [High] Use-After-Free or NULL pointer dereference in byd_process_byte() due to missing RX synchronization during disconnect.
- [High] Improper timer teardown API allows the timer to be re-armed after sync, causing a delayed Use-After-Free.
--
--- Patch [1]: [PATCH] Input: byd - synchronize timer deletion before freeing private data ---
commit eb1fe8255d54e5e900b504d5aed98e3fff1721fb
Author: Linmao Li <lilinmao@kylinos.cn>
Input: byd - synchronize timer deletion before freeing private data
byd_disconnect() uses timer_delete() before freeing the driver's private
data. This does not wait for a running byd_clear_touch() callback, which
dereferences the private data and its psmouse pointer. A callback racing
with disconnect can therefore access the private data after it has been
freed.
Use timer_delete_sync() to wait for any running callback before releasing
the private data.
Fixes: 2d5f5611dd0d ("Input: byd - enable absolute mode")
Cc: stable@vger.kernel.org
Signed-off-by: Linmao Li <lilinmao@kylinos.cn>
> diff --git a/drivers/input/mouse/byd.c b/drivers/input/mouse/byd.c
> index f5770a3af2f12..7a69724c1ac6b 100644
> --- a/drivers/input/mouse/byd.c
> +++ b/drivers/input/mouse/byd.c
> @@ -423,7 +423,7 @@ static void byd_disconnect(struct psmouse *psmouse)
> struct byd_data *priv = psmouse->private;
>
> if (priv) {
> - timer_delete(&priv->timer);
> + timer_delete_sync(&priv->timer);
[Severity: High]
This isn't a bug introduced by this patch, but does timer_delete_sync()
prevent the timer from being re-armed concurrently?
Because device interrupts are not disabled during byd_disconnect(),
byd_process_byte() can execute immediately after timer_delete_sync()
returns but before psmouse->private is freed.
If byd_process_byte() processes a touch event, it can call mod_timer()
and successfully re-arm the timer on the priv structure:
drivers/input/mouse/byd.c:byd_process_byte() {
...
mod_timer(&priv->timer, jiffies + BYD_TOUCH_TIMEOUT);
...
}
When byd_disconnect() subsequently frees priv, won't the timer callback
execute on the freed memory?
Should this use timer_shutdown_sync() instead to permanently prevent the timer
from being re-armed during teardown?
> kfree(psmouse->private);
> psmouse->private = NULL;
[Severity: High]
This is a pre-existing issue, but is it possible for an interrupt to trigger
byd_process_byte() concurrently with or immediately after freeing this memory?
The core psmouse_disconnect() function invokes the driver's disconnect callback
without first stopping device interrupts (such as by using serio_pause_rx()).
If the hardware sends a byte during device teardown, ps2_interrupt() can
route the data to the active protocol handler byd_process_byte(). This
unconditionally dereferences the freed or NULL psmouse->private pointer:
drivers/input/mouse/byd.c:byd_process_byte() {
struct byd_data *priv = psmouse->private;
...
}
Does the receive path need to be paused or synchronized before freeing the
private data to prevent a use-after-free or NULL pointer dereference?
> }
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260720033030.1580891-1-lilinmao@kylinos.cn?part=1
^ permalink raw reply
* Re: [PATCH v4 3/3] input: misc: Add Qualcomm SPMI PMIC haptics driver
From: Fenglin Wu @ 2026-07-20 4:33 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: linux-arm-msm, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Lee Jones, Stephen Boyd, Bjorn Andersson, Konrad Dybcio,
David Collins, Subbaraman Narayanamurthy, Kamal Wadhwa,
linux-input, devicetree, linux-kernel
In-Reply-To: <alpWEL1hJgQ18I7d@google.com>
On 7/18/2026 12:51 AM, Dmitry Torokhov wrote:
...
Thank you for reviewing the change!
>> +
>> +static void haptics_fifo_irq_enable(struct qcom_haptics *h, bool enable)
>> +{
>> + if (h->irq_enabled == enable)
>> + return;
>
> Should t you know if given code runs with interrupts disabled or
> enabled? I believe this tracking and the wrapper should be removed.
>
>> +
>> + if (enable)
>> + enable_irq(h->fifo_empty_irq);
>> + else
>> + disable_irq(h->fifo_empty_irq);
>> +
>> + h->irq_enabled = enable;
>> +}
>> +
In normal handling, the code should be able to track the IRQ status. The
IRQ is not auto enabled after the registration. It's only enabled when
the upload data doesn't fit in the initial FIFO fill and requires to
refill based on the interrupt. And after play is done, the IRQ can be
disabled.
There are error paths or concurrency cases which might cause the IRQ
being disabled multiple times, for example:
1) IRQ is enabled when playing an effect with a long FIFO data
2) During the FIFO refill, if any SPMI write errors, and if the SPMI bus
issue persists, there would be an IRQ storm as the IRQ is still kept as
enabled and the HW FIFO is still 'empty'. So I need to disable the IRQ
to avoid the IRQ storming before the play is stopped in step 3) below.
3) Stop the play and disable the IRQ, either when the play is done, or
when userspace issues a stop command.
There are potential multiple times of IRQ disabling in such cases, I
created this helper function to track the IRQ status to prevent
disabling the IRQ permanently.
BTW, Sashiko AI flagged a deadlock issue on the 'disable_irq()' usage,
when a stop() command came 1st and acquired the 'fifo_lock' and then the
IRQ thread is scheduled, in 'disable_irq()" the IRQ handler would
compete the 'fifo_lock' and cause a deadlock. I will need to change it
to use the _nosync() version consider how to prevent the races of FIFO
resources being used by the IRQ handler after it is freed in stop().
>> +
>> +/*
>> + * haptics_fifo_empty_irq: Threaded IRQ handler for the FIFO-empty interrupt.
>> + *
>> + * While a FIFO play is in progress the hardware fires this interrupt when
>> + * the number of samples in the FIFO drops below the programmed threshold.
>> + * The handler refills the FIFO from the effect's data buffer. When all
>> + * samples have been written the threshold is set to zero. The HW would
>> + * stop the play automatically after all of the samples in FIFO memory are
>> + * played out.
>> + */
>> +static irqreturn_t haptics_fifo_empty_irq(int irq, void *dev_id)
>> +{
>> + struct qcom_haptics *h = dev_id;
>> + u32 sts, to_write;
>> + int ret;
>> +
>> + ret = regmap_read(h->regmap,
>> + h->cfg_base + HAP_CFG_INT_RT_STS_REG, &sts);
>> + if (ret || !(sts & FIFO_EMPTY_BIT))
>> + return IRQ_HANDLED;
>> +
>> + guard(mutex)(&h->fifo_lock);
>> +
>> + if (!h->fifo_data)
>> + return IRQ_HANDLED;
>> +
>> + /* Refill: write the next chunk */
>> + to_write = min_t(u32, h->data_len - h->data_written,
>> + h->fifo_len - FIFO_EMPTY_THRESH);
>> + ret = haptics_write_fifo_chunk(h, &h->fifo_data[h->data_written], to_write);
>> + if (ret) {
>> + dev_err(h->dev, "refill FIFO samples failed, ret=%d\n", ret);
>> + /*
>> + * If data refilling is failed,stop the HW play and disable the
>> + * IRQ to prevent the FIFO empty IRQ being fired continuously.
>> + */
>
> Is recovery possible after this?
It depends on how serious the problem is. Ideally, the SPMI write access
error above should only indicate a bus transaction fault but the haptics
HW should still work on its FIFO data which has already been programmed.
If the SPMI write fault just happens temporarily, it might only cause
the haptics driving waveform distortion and impact on the vibration
effect. If the SPMI write fault is caused by SPMI bus hung, then the
haptics module won't be functional anymore as all of the commands are
SPMI based. For safety, I stop the play (doesn't work in the latter
case) and disable the IRQ to prevent the IRQ being fired continuously.
Thanks
Fenglin
^ permalink raw reply
* Re: [PATCH] Input: byd - synchronize timer deletion before freeing private data
From: Dmitry Torokhov @ 2026-07-20 5:18 UTC (permalink / raw)
To: sashiko-reviews; +Cc: Linmao Li, linux-input
In-Reply-To: <20260720034855.07D8C1F000E9@smtp.kernel.org>
On Mon, Jul 20, 2026 at 03:48:54AM +0000, sashiko-bot@kernel.org wrote:
>
> Should this use timer_shutdown_sync() instead to permanently prevent the timer
> from being re-armed during teardown?
This is a good suggestion.
>
> > kfree(psmouse->private);
> > psmouse->private = NULL;
>
> [Severity: High]
> This is a pre-existing issue, but is it possible for an interrupt to trigger
> byd_process_byte() concurrently with or immediately after freeing this memory?
This has to be solved in psmouse core, I'll take a look.
Thanks.
--
Dmitry
^ permalink raw reply
* [PATCH v2] Input: byd - synchronize timer deletion before freeing private data
From: Linmao Li @ 2026-07-20 6:12 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Kees Cook, Vivek BalachandharTN, Richard Pospesel, Chris Diamand,
linux-input, linux-kernel, Linmao Li, stable
In-Reply-To: <20260720033030.1580891-1-lilinmao@kylinos.cn>
byd_disconnect() uses timer_delete() before freeing the driver's private
data. This does not wait for a running byd_clear_touch() callback, which
dereferences the private data and its psmouse pointer. A callback racing
with disconnect can therefore access the private data after it has been
freed. The timer can also still be re-armed by byd_process_byte() while
the disconnect is in progress.
Use timer_shutdown_sync() before freeing the private data: it waits for
a running callback and turns any later re-arm attempt into a no-op.
Fixes: 2d5f5611dd0d ("Input: byd - enable absolute mode")
Cc: stable@vger.kernel.org
Signed-off-by: Linmao Li <lilinmao@kylinos.cn>
---
v2:
- use timer_shutdown_sync() instead of timer_delete_sync() so the
timer cannot be re-armed by byd_process_byte() while disconnect is
in progress (Sashiko review, Dmitry)
drivers/input/mouse/byd.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/input/mouse/byd.c b/drivers/input/mouse/byd.c
index f5770a3af2f1..5fc3c629590a 100644
--- a/drivers/input/mouse/byd.c
+++ b/drivers/input/mouse/byd.c
@@ -423,7 +423,7 @@ static void byd_disconnect(struct psmouse *psmouse)
struct byd_data *priv = psmouse->private;
if (priv) {
- timer_delete(&priv->timer);
+ timer_shutdown_sync(&priv->timer);
kfree(psmouse->private);
psmouse->private = NULL;
}
--
2.25.1
^ permalink raw reply related
* Re: [PATCH v2] Input: byd - synchronize timer deletion before freeing private data
From: sashiko-bot @ 2026-07-20 6:26 UTC (permalink / raw)
To: Linmao Li; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260720061259.1601281-1-lilinmao@kylinos.cn>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Concurrent execution of the protocol handler `byd_process_byte()` during or after `byd_disconnect()` can lead to a Use-After-Free or NULL pointer dereference of the driver's private data.
--
--- Patch [1]: [PATCH v2] Input: byd - synchronize timer deletion before freeing private data ---
commit e08a94a088b6cedd4010692fa9ab4cd3e5287e71
Author: Linmao Li <lilinmao@kylinos.cn>
Input: byd - synchronize timer deletion before freeing private data
byd_disconnect() uses timer_delete() before freeing the driver's private
data. This does not wait for a running byd_clear_touch() callback, which
dereferences the private data and its psmouse pointer. A callback racing
with disconnect can therefore access the private data after it has been
freed. The timer can also still be re-armed by byd_process_byte() while
the disconnect is in progress.
Use timer_shutdown_sync() before freeing the private data: it waits for
a running callback and turns any later re-arm attempt into a no-op.
> diff --git a/drivers/input/mouse/byd.c b/drivers/input/mouse/byd.c
> index f5770a3af2f12..5fc3c629590ae 100644
> --- a/drivers/input/mouse/byd.c
> +++ b/drivers/input/mouse/byd.c
> @@ -423,7 +423,7 @@ static void byd_disconnect(struct psmouse *psmouse)
> struct byd_data *priv = psmouse->private;
>
> if (priv) {
> - timer_delete(&priv->timer);
> + timer_shutdown_sync(&priv->timer);
> kfree(psmouse->private);
^^^^^^^^^^^^^^^^^^^^^^^
[Severity: High]
This is a pre-existing issue, but could this free lead to a use-after-free
or null pointer dereference if a hardware interrupt occurs concurrently?
During disconnect, the psmouse core sets the software state to
PSMOUSE_CMD_MODE in psmouse_disconnect(), which still allows the interrupt
handler to process incoming bytes.
If a full packet is received, the protocol handler byd_process_byte() is
called and unconditionally dereferences psmouse->private, which may have
just been freed or set to NULL:
drivers/input/mouse/byd.c:byd_process_byte() {
struct byd_data *priv = psmouse->private;
...
switch (pkt[3]) {
case BYD_PACKET_ABSOLUTE:
if (!priv->touch) {
...
}
Should the state be set to PSMOUSE_IGNORE or the RX queue otherwise paused
before we free the private data here?
> psmouse->private = NULL;
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260720061259.1601281-1-lilinmao@kylinos.cn?part=1
^ permalink raw reply
* Re: [PATCH v2 1/3] ARM: dts: ti: am335x-guardian: Drop unused isink-en property
From: Krzysztof Kozlowski @ 2026-07-20 6:49 UTC (permalink / raw)
To: Eduard Bostina
Cc: Aaro Koskinen, Andreas Kemnade, Conor Dooley, Daniel Thompson,
devicetree, Dmitry Torokhov, dri-devel, Jingoo Han, Kevin Hilman,
Krzysztof Kozlowski, Lee Jones, linux-input, linux-kernel,
linux-leds, linux-omap, Pavel Machek, Rob Herring, Roger Quadros,
Tony Lindgren, daniel.baluta, simona.toaca, goledhruva,
m-chawdhry
In-Reply-To: <20260718215632.3295049-2-egbostina@gmail.com>
On Sat, Jul 18, 2026 at 09:56:30PM +0000, Eduard Bostina wrote:
> The 'isink-en' property is not read by any driver, so remove it from the
"...is not documented and not read by any driver..."
> TPS65217 backlight node.
With this change:
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v2 2/3] dt-bindings: input: Convert TI TPS65217 power button to DT schema
From: Krzysztof Kozlowski @ 2026-07-20 6:51 UTC (permalink / raw)
To: Eduard Bostina
Cc: Aaro Koskinen, Andreas Kemnade, Conor Dooley, Daniel Thompson,
devicetree, Dmitry Torokhov, dri-devel, Jingoo Han, Kevin Hilman,
Krzysztof Kozlowski, Lee Jones, linux-input, linux-kernel,
linux-leds, linux-omap, Pavel Machek, Rob Herring, Roger Quadros,
Tony Lindgren, daniel.baluta, simona.toaca, goledhruva,
m-chawdhry
In-Reply-To: <20260718215632.3295049-3-egbostina@gmail.com>
On Sat, Jul 18, 2026 at 09:56:31PM +0000, Eduard Bostina wrote:
> Convert the Texas Instruments TPS65217 and TPS65218 Power Button
> bindings to DT schema.
>
> Signed-off-by: Eduard Bostina <egbostina@gmail.com>
> ---
> .../bindings/input/ti,tps65217-pwrbutton.yaml | 42 +++++++++++++++++++
> .../bindings/input/tps65218-pwrbutton.txt | 30 -------------
> 2 files changed, 42 insertions(+), 30 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/input/ti,tps65217-pwrbutton.yaml
> delete mode 100644 Documentation/devicetree/bindings/input/tps65218-pwrbutton.txt
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v2 3/3] dt-bindings: leds: backlight: Convert TPS65217 to DT schema
From: Krzysztof Kozlowski @ 2026-07-20 6:54 UTC (permalink / raw)
To: Eduard Bostina
Cc: Aaro Koskinen, Andreas Kemnade, Conor Dooley, Daniel Thompson,
devicetree, Dmitry Torokhov, dri-devel, Jingoo Han, Kevin Hilman,
Krzysztof Kozlowski, Lee Jones, linux-input, linux-kernel,
linux-leds, linux-omap, Pavel Machek, Rob Herring, Roger Quadros,
Tony Lindgren, daniel.baluta, simona.toaca, goledhruva,
m-chawdhry
In-Reply-To: <20260718215632.3295049-4-egbostina@gmail.com>
On Sat, Jul 18, 2026 at 09:56:32PM +0000, Eduard Bostina wrote:
+
> + patternProperties:
> + "^regulator@[0-6]$":
> + type: object
> + $ref: /schemas/regulator/regulator.yaml#
> + unevaluatedProperties: false
> +
> + properties:
> + reg:
> + maxItems: 1
> +
> + regulator-compatible:
> + $ref: /schemas/types.yaml#/definitions/string
You should not need this property, it is already in regulator.yaml.
> +
> + charger:
> + $ref: /schemas/power/supply/tps65217-charger.yaml#
unevaluatedProperties: false
> +
> + pwrbutton:
> + $ref: /schemas/input/ti,tps65217-pwrbutton.yaml#
unevaluatedProperties: false
> +
> +
> +required:
> + - compatible
> + - reg
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + i2c {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + tps@24 {
pmic@24
Please make the example mostly complete - add rest of properties and nodes.
> + reg = <0x24>;
> + compatible = "ti,tps65217";
> +
> + backlight {
> + isel = <1>; /* 1 - ISET1, 2 ISET2 */
> + fdim = <100>; /* TPS65217_BL_FDIM_100HZ */
> + default-brightness = <50>;
> + };
> + };
Best regards,
Krzysztof
^ permalink raw reply
* [PATCH v3 0/4] HID: Remove redundant error messages on IRQ request failure
From: Pan Chuang @ 2026-07-20 8:42 UTC (permalink / raw)
To: Basavaraj Natikar, Jiri Kosina, Benjamin Tissoires,
Srinivas Pandruvada, Even Xu, Xinpeng Sun, Zhang Lixu,
Andy Shevchenko, Steven Rostedt, Pan Chuang, Vineeth Pillai,
Sakari Ailus, Abhishek Tamboli, Danny D.,
open list:AMD SENSOR FUSION HUB DRIVER, open list
Commit 55b48e23f5c4 ("genirq/devres: Add error handling in
devm_request_*_irq()") added automatic error logging to
devm_request_threaded_irq() and devm_request_any_context_irq()
via the new devm_request_result() helper, which prints device
name, IRQ number, handler functions, and error code on failure.
Since devm_request_irq() is a static inline wrapper around
devm_request_threaded_irq(), it also benefits from this
automatic logging.
Remove the now-redundant dev_err() and dev_err_probe() calls
in hid drivers that follow these devm_request_*_irq()
functions, as the core now provides more detailed diagnostic
information on failure.
v1 -> v2:
- Added the same change for the intel-quicki2c driver
v2 -> v3:
- amd_sfh: directly return devm_request_irq() result
Pan Chuang (4):
HID: amd_sfh: Remove redundant dev_err()
HID: hid-goodix: Remove redundant dev_err()
HID: intel-ish-hid: ipc: Remove redundant dev_err()
HID: Intel-thc-hid: Remove redundant dev_err()
drivers/hid/amd-sfh-hid/amd_sfh_pcie.c | 13 ++-----------
drivers/hid/hid-goodix-spi.c | 5 +----
drivers/hid/intel-ish-hid/ipc/pci-ish.c | 4 +---
.../hid/intel-thc-hid/intel-quicki2c/pci-quicki2c.c | 5 +----
.../hid/intel-thc-hid/intel-quickspi/pci-quickspi.c | 5 +----
5 files changed, 6 insertions(+), 26 deletions(-)
--
2.34.1
^ permalink raw reply
* [PATCH v3 1/4] HID: amd_sfh: Remove redundant dev_err()
From: Pan Chuang @ 2026-07-20 8:42 UTC (permalink / raw)
To: Basavaraj Natikar, Jiri Kosina, Benjamin Tissoires,
open list:AMD SENSOR FUSION HUB DRIVER, open list
Cc: Pan Chuang, Basavaraj Natikar
In-Reply-To: <20260720084310.210976-1-panchuang@vivo.com>
Since commit 55b48e23f5c4 ("genirq/devres: Add error handling in
devm_request_*_irq()"), devm_request_irq() automatically logs
detailed error messages on failure. Remove the now-redundant
driver-specific dev_err() calls.
Signed-off-by: Pan Chuang <panchuang@vivo.com>
Acked-by: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
---
drivers/hid/amd-sfh-hid/amd_sfh_pcie.c | 13 ++-----------
1 file changed, 2 insertions(+), 11 deletions(-)
diff --git a/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c b/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c
index 4b81cebdc335..4d0a95fbc4e4 100644
--- a/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c
+++ b/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c
@@ -122,19 +122,10 @@ static irqreturn_t amd_sfh_irq_handler(int irq, void *data)
int amd_sfh_irq_init_v2(struct amd_mp2_dev *privdata)
{
- int rc;
-
pcim_intx(privdata->pdev, true);
- rc = devm_request_irq(&privdata->pdev->dev, privdata->pdev->irq,
- amd_sfh_irq_handler, 0, DRIVER_NAME, privdata);
- if (rc) {
- dev_err(&privdata->pdev->dev, "failed to request irq %d err=%d\n",
- privdata->pdev->irq, rc);
- return rc;
- }
-
- return 0;
+ return devm_request_irq(&privdata->pdev->dev, privdata->pdev->irq,
+ amd_sfh_irq_handler, 0, DRIVER_NAME, privdata);
}
static int amd_sfh_dis_sts_v2(struct amd_mp2_dev *privdata)
--
2.34.1
^ permalink raw reply related
* [PATCH v3 2/4] HID: hid-goodix: Remove redundant dev_err()
From: Pan Chuang @ 2026-07-20 8:43 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, open list:HID CORE LAYER,
open list
Cc: Pan Chuang
In-Reply-To: <20260720084310.210976-1-panchuang@vivo.com>
Since commit 55b48e23f5c4 ("genirq/devres: Add error handling in
devm_request_*_irq()"), devm_request_threaded_irq() automatically logs
detailed error messages on failure. Remove the now-redundant
driver-specific dev_err() calls.
Signed-off-by: Pan Chuang <panchuang@vivo.com>
---
drivers/hid/hid-goodix-spi.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/hid/hid-goodix-spi.c b/drivers/hid/hid-goodix-spi.c
index 288cb827e9d6..03d549efbdce 100644
--- a/drivers/hid/hid-goodix-spi.c
+++ b/drivers/hid/hid-goodix-spi.c
@@ -722,11 +722,8 @@ static int goodix_spi_probe(struct spi_device *spi)
error = devm_request_threaded_irq(&ts->spi->dev, ts->spi->irq,
NULL, goodix_hid_irq, IRQF_ONESHOT,
"goodix_spi_hid", ts);
- if (error) {
- dev_err(ts->dev, "could not register interrupt, irq = %d, %d",
- ts->spi->irq, error);
+ if (error)
goto err_destroy_hid;
- }
return 0;
--
2.34.1
^ permalink raw reply related
* [PATCH v3 3/4] HID: intel-ish-hid: ipc: Remove redundant dev_err()
From: Pan Chuang @ 2026-07-20 8:43 UTC (permalink / raw)
To: Srinivas Pandruvada, Jiri Kosina, Benjamin Tissoires, Zhang Lixu,
Andy Shevchenko, Pan Chuang, Vineeth Pillai,
open list:INTEL INTEGRATED SENSOR HUB DRIVER, open list
In-Reply-To: <20260720084310.210976-1-panchuang@vivo.com>
Since commit 55b48e23f5c4 ("genirq/devres: Add error handling in
devm_request_*_irq()"), devm_request_irq() automatically logs
detailed error messages on failure. Remove the now-redundant
driver-specific dev_err() calls.
Signed-off-by: Pan Chuang <panchuang@vivo.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
---
drivers/hid/intel-ish-hid/ipc/pci-ish.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/hid/intel-ish-hid/ipc/pci-ish.c b/drivers/hid/intel-ish-hid/ipc/pci-ish.c
index 8d36ae96a3ee..e7196e429c8b 100644
--- a/drivers/hid/intel-ish-hid/ipc/pci-ish.c
+++ b/drivers/hid/intel-ish-hid/ipc/pci-ish.c
@@ -232,10 +232,8 @@ static int ish_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
ret = devm_request_irq(dev, pdev->irq, ish_irq_handler,
irq_flag, KBUILD_MODNAME, ishtp);
- if (ret) {
- dev_err(dev, "ISH: request IRQ %d failed\n", pdev->irq);
+ if (ret)
return ret;
- }
dev_set_drvdata(ishtp->devc, ishtp);
--
2.34.1
^ permalink raw reply related
* [PATCH v3 4/4] HID: Intel-thc-hid: Remove redundant dev_err()
From: Pan Chuang @ 2026-07-20 8:43 UTC (permalink / raw)
To: Even Xu, Xinpeng Sun, Jiri Kosina, Benjamin Tissoires,
Sakari Ailus, Pan Chuang, Danny D., Abhishek Tamboli,
open list:HID CORE LAYER, open list
In-Reply-To: <20260720084310.210976-1-panchuang@vivo.com>
Since commit 55b48e23f5c4 ("genirq/devres: Add error handling in
devm_request_*_irq()"), devm_request_threaded_irq() automatically logs
detailed error messages on failure. Remove the now-redundant
driver-specific dev_err() calls.
Signed-off-by: Pan Chuang <panchuang@vivo.com>
Reviewed-by: Even Xu <even.xu@intel.com>
---
drivers/hid/intel-thc-hid/intel-quicki2c/pci-quicki2c.c | 5 +----
drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c | 5 +----
2 files changed, 2 insertions(+), 8 deletions(-)
diff --git a/drivers/hid/intel-thc-hid/intel-quicki2c/pci-quicki2c.c b/drivers/hid/intel-thc-hid/intel-quicki2c/pci-quicki2c.c
index 46d3e9a01999..59f500345acb 100644
--- a/drivers/hid/intel-thc-hid/intel-quicki2c/pci-quicki2c.c
+++ b/drivers/hid/intel-thc-hid/intel-quicki2c/pci-quicki2c.c
@@ -682,11 +682,8 @@ static int quicki2c_probe(struct pci_dev *pdev, const struct pci_device_id *id)
quicki2c_irq_thread_handler,
IRQF_ONESHOT, KBUILD_MODNAME,
qcdev);
- if (ret) {
- dev_err_once(&pdev->dev,
- "Failed to request threaded IRQ, irq = %d.\n", pdev->irq);
+ if (ret)
goto dev_deinit;
- }
ret = quicki2c_get_device_descriptor(qcdev);
if (ret) {
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 4ae2e1718b30..504ef3c842ab 100644
--- a/drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c
+++ b/drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c
@@ -636,11 +636,8 @@ static int quickspi_probe(struct pci_dev *pdev,
quickspi_irq_thread_handler,
IRQF_ONESHOT, KBUILD_MODNAME,
qsdev);
- if (ret) {
- dev_err(&pdev->dev,
- "Failed to request threaded IRQ, irq = %d.\n", pdev->irq);
+ if (ret)
goto dev_deinit;
- }
ret = reset_tic(qsdev);
if (ret) {
--
2.34.1
^ permalink raw reply related
* [PATCH 1/2] Input: snvs_pwrkey: Improve probe error handling
From: phucduc.bui @ 2026-07-20 8:50 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input, linux-kernel, bui duc phuc
From: bui duc phuc <phucduc.bui@gmail.com>
Use dev_err_probe() for devm_clk_get_optional_enabled() to handle
deferred probe correctly.
Return the original error from platform_get_irq() instead of replacing
it with -EINVAL.
Drop the redundant error message after devm_request_irq(), as the
helper already reports request failures internally.
Signed-off-by: bui duc phuc <phucduc.bui@gmail.com>
---
drivers/input/keyboard/snvs_pwrkey.c | 13 +++++--------
1 file changed, 5 insertions(+), 8 deletions(-)
diff --git a/drivers/input/keyboard/snvs_pwrkey.c b/drivers/input/keyboard/snvs_pwrkey.c
index 954055aaf6e2..fa08ac561bf2 100644
--- a/drivers/input/keyboard/snvs_pwrkey.c
+++ b/drivers/input/keyboard/snvs_pwrkey.c
@@ -142,16 +142,15 @@ static int imx_snvs_pwrkey_probe(struct platform_device *pdev)
}
clk = devm_clk_get_optional_enabled(&pdev->dev, NULL);
- if (IS_ERR(clk)) {
- dev_err(&pdev->dev, "Failed to get snvs clock (%pe)\n", clk);
- return PTR_ERR(clk);
- }
+ if (IS_ERR(clk))
+ return dev_err_probe(&pdev->dev, PTR_ERR(clk),
+ "Failed to get snvs clock\n");
pdata->wakeup = of_property_read_bool(np, "wakeup-source");
pdata->irq = platform_get_irq(pdev, 0);
if (pdata->irq < 0)
- return -EINVAL;
+ return pdata->irq;
error = of_property_read_u32(np, "power-off-time-sec", &val);
if (!error) {
@@ -209,10 +208,8 @@ static int imx_snvs_pwrkey_probe(struct platform_device *pdev)
error = devm_request_irq(&pdev->dev, pdata->irq,
imx_snvs_pwrkey_interrupt,
0, pdev->name, pdev);
- if (error) {
- dev_err(&pdev->dev, "interrupt not available.\n");
+ if (error)
return error;
- }
error = input_register_device(input);
if (error < 0) {
--
2.43.0
^ permalink raw reply related
* [PATCH 2/2] Input: st-keyscan: Improve probe error handling
From: phucduc.bui @ 2026-07-20 8:50 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input, linux-kernel, bui duc phuc
In-Reply-To: <20260720085041.224896-1-phucduc.bui@gmail.com>
From: bui duc phuc <phucduc.bui@gmail.com>
Use dev_err_probe() for devm_clk_get() so deferred probe errors are
handled correctly.
Drop the redundant error message after devm_request_irq(), since the
helper already reports request failures internally.
Signed-off-by: bui duc phuc <phucduc.bui@gmail.com>
---
drivers/input/keyboard/st-keyscan.c | 11 ++++-------
1 file changed, 4 insertions(+), 7 deletions(-)
diff --git a/drivers/input/keyboard/st-keyscan.c b/drivers/input/keyboard/st-keyscan.c
index e53ef4c670e4..d86f6f16ae1b 100644
--- a/drivers/input/keyboard/st-keyscan.c
+++ b/drivers/input/keyboard/st-keyscan.c
@@ -173,10 +173,9 @@ static int keyscan_probe(struct platform_device *pdev)
return PTR_ERR(keypad_data->base);
keypad_data->clk = devm_clk_get(&pdev->dev, NULL);
- if (IS_ERR(keypad_data->clk)) {
- dev_err(&pdev->dev, "cannot get clock\n");
- return PTR_ERR(keypad_data->clk);
- }
+ if (IS_ERR(keypad_data->clk))
+ return dev_err_probe(&pdev->dev, PTR_ERR(keypad_data->clk),
+ "cannot get clock\n");
error = clk_enable(keypad_data->clk);
if (error) {
@@ -192,10 +191,8 @@ static int keyscan_probe(struct platform_device *pdev)
error = devm_request_irq(&pdev->dev, keypad_data->irq, keyscan_isr, 0,
pdev->name, keypad_data);
- if (error) {
- dev_err(&pdev->dev, "failed to request IRQ\n");
+ if (error)
return error;
- }
error = input_register_device(input_dev);
if (error) {
--
2.43.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox