* [PATCH 4/6] tools/build: Allow versioning LLVM readelf
From: James Clark @ 2026-05-14 9:32 UTC (permalink / raw)
To: Nathan Chancellor, Nick Desaulniers, Bill Wendling, Justin Stitt,
Jiri Kosina, Benjamin Tissoires, Shuah Khan, Peter Zijlstra,
Ingo Molnar, Arnaldo Carvalho de Melo, Namhyung Kim, Mark Rutland,
Alexander Shishkin, Jiri Olsa, Ian Rogers, Adrian Hunter,
Andrii Nakryiko, Eduard Zingerman, Alexei Starovoitov,
Daniel Borkmann, Martin KaFai Lau, Kumar Kartikeya Dwivedi,
Song Liu, Yonghong Song
Cc: linux-kernel, llvm, linux-input, linux-kselftest, bpf,
linux-perf-users, James Clark, leo.yan
In-Reply-To: <20260514-james-perf-llvm-version-v1-0-6cac1a9a4c8d@linaro.org>
Documentation/kbuild/llvm.rst mentions that readelf is included in the
LLVM toolchain, but it's not currently included in this block.
Add it so that LLVM=... options also apply to readelf. Users in tools/
were Perf which was hardcoding it, and another was the BPF makefile.
Both already include Makefile.include so convert them to use the new
variable.
It also didn't have the cross compile prefix, so either readelf didn't
mind opening cross binaries, or it wasn't working for cross builds.
Signed-off-by: James Clark <james.clark@linaro.org>
---
tools/lib/bpf/Makefile | 8 ++++----
tools/perf/Makefile.perf | 1 -
tools/scripts/Makefile.include | 2 ++
3 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/tools/lib/bpf/Makefile b/tools/lib/bpf/Makefile
index 168140f8e646..180dca9c57c8 100644
--- a/tools/lib/bpf/Makefile
+++ b/tools/lib/bpf/Makefile
@@ -114,12 +114,12 @@ PC_FILE := $(addprefix $(OUTPUT),$(PC_FILE))
TAGS_PROG := $(if $(shell which etags 2>/dev/null),etags,ctags)
-GLOBAL_SYM_COUNT = $(shell readelf -s --wide $(BPF_IN_SHARED) | \
+GLOBAL_SYM_COUNT = $(shell $(READELF) -s --wide $(BPF_IN_SHARED) | \
cut -d "@" -f1 | sed 's/_v[0-9]_[0-9]_[0-9].*//' | \
sed 's/\[.*\]//' | \
awk '/GLOBAL/ && /DEFAULT/ && !/UND|ABS/ {print $$NF}' | \
sort -u | wc -l)
-VERSIONED_SYM_COUNT = $(shell readelf --dyn-syms --wide $(OUTPUT)libbpf.so | \
+VERSIONED_SYM_COUNT = $(shell $(READELF) --dyn-syms --wide $(OUTPUT)libbpf.so | \
sed 's/\[.*\]//' | \
awk '/GLOBAL/ && /DEFAULT/ && !/UND|ABS/ {print $$NF}' | \
grep -Eo '[^ ]+@LIBBPF_' | cut -d@ -f1 | sort -u | wc -l)
@@ -182,12 +182,12 @@ check_abi: $(OUTPUT)libbpf.so $(VERSION_SCRIPT)
"versioned symbols in $^ ($(VERSIONED_SYM_COUNT))." \
"Please make sure all LIBBPF_API symbols are" \
"versioned in $(VERSION_SCRIPT)." >&2; \
- readelf -s --wide $(BPF_IN_SHARED) | \
+ $(READELF) -s --wide $(BPF_IN_SHARED) | \
cut -d "@" -f1 | sed 's/_v[0-9]_[0-9]_[0-9].*//' | \
sed 's/\[.*\]//' | \
awk '/GLOBAL/ && /DEFAULT/ && !/UND/ {print $$NF}'| \
sort -u > $(OUTPUT)libbpf_global_syms.tmp; \
- readelf --dyn-syms --wide $(OUTPUT)libbpf.so | \
+ $(READELF) --dyn-syms --wide $(OUTPUT)libbpf.so | \
sed 's/\[.*\]//' | \
awk '/GLOBAL/ && /DEFAULT/ && !/UND|ABS/ {print $$NF}'| \
grep -Eo '[^ ]+@LIBBPF_' | cut -d@ -f1 | \
diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
index 0aba14f22a06..63276bf55856 100644
--- a/tools/perf/Makefile.perf
+++ b/tools/perf/Makefile.perf
@@ -215,7 +215,6 @@ FLEX ?= flex
BISON ?= bison
STRIP = strip
AWK = awk
-READELF ?= readelf
# include Makefile.config by default and rule out
# non-config cases
diff --git a/tools/scripts/Makefile.include b/tools/scripts/Makefile.include
index e81e5b479c56..380ad84ac51e 100644
--- a/tools/scripts/Makefile.include
+++ b/tools/scripts/Makefile.include
@@ -73,6 +73,7 @@ ifneq ($(LLVM),)
$(call allow-override,LLC,$(LLVM_PREFIX)llc$(LLVM_SUFFIX))
$(call allow-override,LLVM_CONFIG,$(LLVM_PREFIX)llvm-config$(LLVM_SUFFIX))
$(call allow-override,LLVM_OBJCOPY,$(LLVM_PREFIX)llvm-objcopy$(LLVM_SUFFIX))
+ $(call allow-override,READELF,$(LLVM_PREFIX)llvm-readelf$(LLVM_SUFFIX))
else
# Allow setting various cross-compile vars or setting CROSS_COMPILE as a prefix.
$(call allow-override,CC,$(CROSS_COMPILE)gcc)
@@ -80,6 +81,7 @@ else
$(call allow-override,LD,$(CROSS_COMPILE)ld)
$(call allow-override,CXX,$(CROSS_COMPILE)g++)
$(call allow-override,STRIP,$(CROSS_COMPILE)strip)
+ $(call allow-override,READELF,$(CROSS_COMPILE)readelf)
# Host versions aren't prefixed
$(call allow-override,HOSTAR,ar)
--
2.34.1
^ permalink raw reply related
* [PATCH 3/6] selftests: Remove unused LLD variable
From: James Clark @ 2026-05-14 9:32 UTC (permalink / raw)
To: Nathan Chancellor, Nick Desaulniers, Bill Wendling, Justin Stitt,
Jiri Kosina, Benjamin Tissoires, Shuah Khan, Peter Zijlstra,
Ingo Molnar, Arnaldo Carvalho de Melo, Namhyung Kim, Mark Rutland,
Alexander Shishkin, Jiri Olsa, Ian Rogers, Adrian Hunter,
Andrii Nakryiko, Eduard Zingerman, Alexei Starovoitov,
Daniel Borkmann, Martin KaFai Lau, Kumar Kartikeya Dwivedi,
Song Liu, Yonghong Song
Cc: linux-kernel, llvm, linux-input, linux-kselftest, bpf,
linux-perf-users, James Clark, leo.yan
In-Reply-To: <20260514-james-perf-llvm-version-v1-0-6cac1a9a4c8d@linaro.org>
This file was mostly copied from selftests/bpf/Makefile, but the LLD
variable is not used here. Also, this copied block didn't get the same
fixes as the original one did later.
Remove it to avoid confusion and so future fixes don't have to be in two
places.
Signed-off-by: James Clark <james.clark@linaro.org>
---
tools/testing/selftests/hid/Makefile | 7 -------
1 file changed, 7 deletions(-)
diff --git a/tools/testing/selftests/hid/Makefile b/tools/testing/selftests/hid/Makefile
index 50ec9e0406ab..96071b4800e8 100644
--- a/tools/testing/selftests/hid/Makefile
+++ b/tools/testing/selftests/hid/Makefile
@@ -105,13 +105,6 @@ $(MAKE_DIRS):
$(call msg,MKDIR,,$@)
$(Q)mkdir -p $@
-# LLVM's ld.lld doesn't support all the architectures, so use it only on x86
-ifeq ($(SRCARCH),x86)
-LLD := lld
-else
-LLD := ld
-endif
-
DEFAULT_BPFTOOL := $(HOST_SCRATCH_DIR)/sbin/bpftool
TEST_GEN_PROGS_EXTENDED += $(DEFAULT_BPFTOOL)
--
2.34.1
^ permalink raw reply related
* [PATCH 2/6] tools/build: Indent if else blocks
From: James Clark @ 2026-05-14 9:32 UTC (permalink / raw)
To: Nathan Chancellor, Nick Desaulniers, Bill Wendling, Justin Stitt,
Jiri Kosina, Benjamin Tissoires, Shuah Khan, Peter Zijlstra,
Ingo Molnar, Arnaldo Carvalho de Melo, Namhyung Kim, Mark Rutland,
Alexander Shishkin, Jiri Olsa, Ian Rogers, Adrian Hunter,
Andrii Nakryiko, Eduard Zingerman, Alexei Starovoitov,
Daniel Borkmann, Martin KaFai Lau, Kumar Kartikeya Dwivedi,
Song Liu, Yonghong Song
Cc: linux-kernel, llvm, linux-input, linux-kselftest, bpf,
linux-perf-users, James Clark, leo.yan
In-Reply-To: <20260514-james-perf-llvm-version-v1-0-6cac1a9a4c8d@linaro.org>
These blocks are quite big and unreadable without indentation. Indent
them.
No functional changes intended.
Signed-off-by: James Clark <james.clark@linaro.org>
---
tools/scripts/Makefile.include | 76 +++++++++++++++++++++---------------------
1 file changed, 38 insertions(+), 38 deletions(-)
diff --git a/tools/scripts/Makefile.include b/tools/scripts/Makefile.include
index 7022e78208a2..e81e5b479c56 100644
--- a/tools/scripts/Makefile.include
+++ b/tools/scripts/Makefile.include
@@ -52,46 +52,46 @@ define allow-override
endef
ifneq ($(LLVM),)
-ifneq ($(filter %/,$(LLVM)),)
-LLVM_PREFIX := $(LLVM)
-else ifneq ($(filter -%,$(LLVM)),)
-LLVM_SUFFIX := $(LLVM)
-else ifneq ($(LLVM),1)
-$(error Invalid value for LLVM, see Documentation/kbuild/llvm.rst)
-endif
+ ifneq ($(filter %/,$(LLVM)),)
+ LLVM_PREFIX := $(LLVM)
+ else ifneq ($(filter -%,$(LLVM)),)
+ LLVM_SUFFIX := $(LLVM)
+ else ifneq ($(LLVM),1)
+ $(error Invalid value for LLVM, see Documentation/kbuild/llvm.rst)
+ endif
-$(call allow-override,CC,$(LLVM_PREFIX)clang$(LLVM_SUFFIX))
-$(call allow-override,CLANG,$(LLVM_PREFIX)clang$(LLVM_SUFFIX))
-$(call allow-override,HOSTCC,$(LLVM_PREFIX)clang$(LLVM_SUFFIX))
-$(call allow-override,AR,$(LLVM_PREFIX)llvm-ar$(LLVM_SUFFIX))
-$(call allow-override,HOSTAR,$(LLVM_PREFIX)llvm-ar$(LLVM_SUFFIX))
-$(call allow-override,LD,$(LLVM_PREFIX)ld.lld$(LLVM_SUFFIX))
-$(call allow-override,HOSTLD,$(LLVM_PREFIX)ld.lld$(LLVM_SUFFIX))
-$(call allow-override,CXX,$(LLVM_PREFIX)clang++$(LLVM_SUFFIX))
-$(call allow-override,STRIP,$(LLVM_PREFIX)llvm-strip$(LLVM_SUFFIX))
-$(call allow-override,LLVM_STRIP,$(LLVM_PREFIX)llvm-strip$(LLVM_SUFFIX))
-$(call allow-override,LLC,$(LLVM_PREFIX)llc$(LLVM_SUFFIX))
-$(call allow-override,LLVM_CONFIG,$(LLVM_PREFIX)llvm-config$(LLVM_SUFFIX))
-$(call allow-override,LLVM_OBJCOPY,$(LLVM_PREFIX)llvm-objcopy$(LLVM_SUFFIX))
+ $(call allow-override,CC,$(LLVM_PREFIX)clang$(LLVM_SUFFIX))
+ $(call allow-override,CLANG,$(LLVM_PREFIX)clang$(LLVM_SUFFIX))
+ $(call allow-override,HOSTCC,$(LLVM_PREFIX)clang$(LLVM_SUFFIX))
+ $(call allow-override,AR,$(LLVM_PREFIX)llvm-ar$(LLVM_SUFFIX))
+ $(call allow-override,HOSTAR,$(LLVM_PREFIX)llvm-ar$(LLVM_SUFFIX))
+ $(call allow-override,LD,$(LLVM_PREFIX)ld.lld$(LLVM_SUFFIX))
+ $(call allow-override,HOSTLD,$(LLVM_PREFIX)ld.lld$(LLVM_SUFFIX))
+ $(call allow-override,CXX,$(LLVM_PREFIX)clang++$(LLVM_SUFFIX))
+ $(call allow-override,STRIP,$(LLVM_PREFIX)llvm-strip$(LLVM_SUFFIX))
+ $(call allow-override,LLVM_STRIP,$(LLVM_PREFIX)llvm-strip$(LLVM_SUFFIX))
+ $(call allow-override,LLC,$(LLVM_PREFIX)llc$(LLVM_SUFFIX))
+ $(call allow-override,LLVM_CONFIG,$(LLVM_PREFIX)llvm-config$(LLVM_SUFFIX))
+ $(call allow-override,LLVM_OBJCOPY,$(LLVM_PREFIX)llvm-objcopy$(LLVM_SUFFIX))
else
-# Allow setting various cross-compile vars or setting CROSS_COMPILE as a prefix.
-$(call allow-override,CC,$(CROSS_COMPILE)gcc)
-$(call allow-override,AR,$(CROSS_COMPILE)ar)
-$(call allow-override,LD,$(CROSS_COMPILE)ld)
-$(call allow-override,CXX,$(CROSS_COMPILE)g++)
-$(call allow-override,STRIP,$(CROSS_COMPILE)strip)
-
-# Host versions aren't prefixed
-$(call allow-override,HOSTAR,ar)
-$(call allow-override,HOSTCC,gcc)
-$(call allow-override,HOSTLD,ld)
-
-# Some tools still require Clang, LLC and/or LLVM utils
-$(call allow-override,CLANG,clang)
-$(call allow-override,LLC,llc)
-$(call allow-override,LLVM_CONFIG,llvm-config)
-$(call allow-override,LLVM_OBJCOPY,llvm-objcopy)
-$(call allow-override,LLVM_STRIP,llvm-strip)
+ # Allow setting various cross-compile vars or setting CROSS_COMPILE as a prefix.
+ $(call allow-override,CC,$(CROSS_COMPILE)gcc)
+ $(call allow-override,AR,$(CROSS_COMPILE)ar)
+ $(call allow-override,LD,$(CROSS_COMPILE)ld)
+ $(call allow-override,CXX,$(CROSS_COMPILE)g++)
+ $(call allow-override,STRIP,$(CROSS_COMPILE)strip)
+
+ # Host versions aren't prefixed
+ $(call allow-override,HOSTAR,ar)
+ $(call allow-override,HOSTCC,gcc)
+ $(call allow-override,HOSTLD,ld)
+
+ # Some tools still require Clang, LLC and/or LLVM utils
+ $(call allow-override,CLANG,clang)
+ $(call allow-override,LLC,llc)
+ $(call allow-override,LLVM_CONFIG,llvm-config)
+ $(call allow-override,LLVM_OBJCOPY,llvm-objcopy)
+ $(call allow-override,LLVM_STRIP,llvm-strip)
endif
CC_NO_CLANG := $(shell $(CC) -dM -E -x c /dev/null | grep -Fq "__clang__"; echo $$?)
--
2.34.1
^ permalink raw reply related
* [PATCH 1/6] tools/build: Allow versioning of all LLVM tools defined in Makefile.include
From: James Clark @ 2026-05-14 9:32 UTC (permalink / raw)
To: Nathan Chancellor, Nick Desaulniers, Bill Wendling, Justin Stitt,
Jiri Kosina, Benjamin Tissoires, Shuah Khan, Peter Zijlstra,
Ingo Molnar, Arnaldo Carvalho de Melo, Namhyung Kim, Mark Rutland,
Alexander Shishkin, Jiri Olsa, Ian Rogers, Adrian Hunter,
Andrii Nakryiko, Eduard Zingerman, Alexei Starovoitov,
Daniel Borkmann, Martin KaFai Lau, Kumar Kartikeya Dwivedi,
Song Liu, Yonghong Song
Cc: linux-kernel, llvm, linux-input, linux-kselftest, bpf,
linux-perf-users, James Clark, leo.yan
In-Reply-To: <20260514-james-perf-llvm-version-v1-0-6cac1a9a4c8d@linaro.org>
The version of LLVM tools can be given on the build command with
LLVM=-15, but this isn't applied to all tools. For example $(CC) gets
versioned, but $(CLANG) doesn't. This causes a Perf build with LTO=1 to
fail with an error about mixed clang versions:
ld.lld: error: libperf/core.o: Unknown attribute kind (86)
(Producer: 'LLVM18.1.8' Reader: 'LLVM 15.0.7')
This file has two "ifneq ($(LLVM),)" blocks adjacent to each other, so
merge these blocks making it obvious that all tools should be versioned
consistently and there is nothing special about each block.
This also reveals that ?= and "allow-override" are used inconsistently
between the blocks. "allow-override" is technically only required for
builtin variables, but isn't only used on them, and doesn't do any harm
if used on a non-builtin. Make them all "allow-override" for
consistency. The only functional difference this will cause is if there
is a file level definition of one of the variables followed by an
"#include of Makefile.include" which will now overwrite. But this isn't
done and in a later commit some of the duplicate definitions will be
removed for good measure.
There are also some other LLVM tools that are not defined here and will
be moved in a later commit.
Signed-off-by: James Clark <james.clark@linaro.org>
---
tools/scripts/Makefile.include | 37 ++++++++++++++++++++-----------------
1 file changed, 20 insertions(+), 17 deletions(-)
diff --git a/tools/scripts/Makefile.include b/tools/scripts/Makefile.include
index 41971a68972d..7022e78208a2 100644
--- a/tools/scripts/Makefile.include
+++ b/tools/scripts/Makefile.include
@@ -61,10 +61,18 @@ $(error Invalid value for LLVM, see Documentation/kbuild/llvm.rst)
endif
$(call allow-override,CC,$(LLVM_PREFIX)clang$(LLVM_SUFFIX))
+$(call allow-override,CLANG,$(LLVM_PREFIX)clang$(LLVM_SUFFIX))
+$(call allow-override,HOSTCC,$(LLVM_PREFIX)clang$(LLVM_SUFFIX))
$(call allow-override,AR,$(LLVM_PREFIX)llvm-ar$(LLVM_SUFFIX))
+$(call allow-override,HOSTAR,$(LLVM_PREFIX)llvm-ar$(LLVM_SUFFIX))
$(call allow-override,LD,$(LLVM_PREFIX)ld.lld$(LLVM_SUFFIX))
+$(call allow-override,HOSTLD,$(LLVM_PREFIX)ld.lld$(LLVM_SUFFIX))
$(call allow-override,CXX,$(LLVM_PREFIX)clang++$(LLVM_SUFFIX))
$(call allow-override,STRIP,$(LLVM_PREFIX)llvm-strip$(LLVM_SUFFIX))
+$(call allow-override,LLVM_STRIP,$(LLVM_PREFIX)llvm-strip$(LLVM_SUFFIX))
+$(call allow-override,LLC,$(LLVM_PREFIX)llc$(LLVM_SUFFIX))
+$(call allow-override,LLVM_CONFIG,$(LLVM_PREFIX)llvm-config$(LLVM_SUFFIX))
+$(call allow-override,LLVM_OBJCOPY,$(LLVM_PREFIX)llvm-objcopy$(LLVM_SUFFIX))
else
# Allow setting various cross-compile vars or setting CROSS_COMPILE as a prefix.
$(call allow-override,CC,$(CROSS_COMPILE)gcc)
@@ -72,26 +80,21 @@ $(call allow-override,AR,$(CROSS_COMPILE)ar)
$(call allow-override,LD,$(CROSS_COMPILE)ld)
$(call allow-override,CXX,$(CROSS_COMPILE)g++)
$(call allow-override,STRIP,$(CROSS_COMPILE)strip)
-endif
-
-CC_NO_CLANG := $(shell $(CC) -dM -E -x c /dev/null | grep -Fq "__clang__"; echo $$?)
-ifneq ($(LLVM),)
-HOSTAR ?= $(LLVM_PREFIX)llvm-ar$(LLVM_SUFFIX)
-HOSTCC ?= $(LLVM_PREFIX)clang$(LLVM_SUFFIX)
-HOSTLD ?= $(LLVM_PREFIX)ld.lld$(LLVM_SUFFIX)
-else
-HOSTAR ?= ar
-HOSTCC ?= gcc
-HOSTLD ?= ld
+# Host versions aren't prefixed
+$(call allow-override,HOSTAR,ar)
+$(call allow-override,HOSTCC,gcc)
+$(call allow-override,HOSTLD,ld)
+
+# Some tools still require Clang, LLC and/or LLVM utils
+$(call allow-override,CLANG,clang)
+$(call allow-override,LLC,llc)
+$(call allow-override,LLVM_CONFIG,llvm-config)
+$(call allow-override,LLVM_OBJCOPY,llvm-objcopy)
+$(call allow-override,LLVM_STRIP,llvm-strip)
endif
-# Some tools require Clang, LLC and/or LLVM utils
-CLANG ?= clang
-LLC ?= llc
-LLVM_CONFIG ?= llvm-config
-LLVM_OBJCOPY ?= llvm-objcopy
-LLVM_STRIP ?= llvm-strip
+CC_NO_CLANG := $(shell $(CC) -dM -E -x c /dev/null | grep -Fq "__clang__"; echo $$?)
# Some tools require bpftool
SYSTEM_BPFTOOL ?= bpftool
--
2.34.1
^ permalink raw reply related
* [PATCH 0/6] tools/build: Allow versioning of all LLVM tools
From: James Clark @ 2026-05-14 9:32 UTC (permalink / raw)
To: Nathan Chancellor, Nick Desaulniers, Bill Wendling, Justin Stitt,
Jiri Kosina, Benjamin Tissoires, Shuah Khan, Peter Zijlstra,
Ingo Molnar, Arnaldo Carvalho de Melo, Namhyung Kim, Mark Rutland,
Alexander Shishkin, Jiri Olsa, Ian Rogers, Adrian Hunter,
Andrii Nakryiko, Eduard Zingerman, Alexei Starovoitov,
Daniel Borkmann, Martin KaFai Lau, Kumar Kartikeya Dwivedi,
Song Liu, Yonghong Song
Cc: linux-kernel, llvm, linux-input, linux-kselftest, bpf,
linux-perf-users, James Clark, leo.yan
I ran into the build issue mentioned in the first commit, because using
an option like LLVM=-15 is a documented way to build [1], but only
some of the LLVM tools defined in tools/ support this.
After fixing that, a lot of other tidyups fell out. I didn't go too far,
for example some selftests include tools/testing/selftests/lib.mk which
reimplements some of the LLVM_SUFFIX stuff, but other selftests include
tools/scripts/Makefile.include and I stopped before touching the former.
Some other non build scripts hard code toolchains and I didn't touch
those either, this change is only focused on things that already
include Makefile.include.
[1]: Documentation/kbuild/llvm.rst
Signed-off-by: James Clark <james.clark@linaro.org>
---
James Clark (6):
tools/build: Allow versioning of all LLVM tools defined in Makefile.include
tools/build: Indent if else blocks
selftests: Remove unused LLD variable
tools/build: Allow versioning LLVM readelf
tools/build: selftests: Allow versioning LLVM lld
tools/build: selftests: Remove some duplicate toolchain definitions
tools/bpf/resolve_btfids/Makefile | 3 --
tools/lib/api/Makefile | 4 --
tools/lib/bpf/Makefile | 8 ++--
tools/lib/subcmd/Makefile | 4 --
tools/lib/symbol/Makefile | 4 --
tools/perf/Makefile.perf | 7 ----
tools/scripts/Makefile.include | 77 ++++++++++++++++++++----------------
tools/testing/selftests/bpf/Makefile | 9 ++---
tools/testing/selftests/hid/Makefile | 7 ----
9 files changed, 50 insertions(+), 73 deletions(-)
---
base-commit: 09d355618f7ccc27ffc7fc668b2e232872962079
change-id: 20260511-james-perf-llvm-version-43ab1cb5bc7d
Best regards,
--
James Clark <james.clark@linaro.org>
^ permalink raw reply
* Re: [PATCH 4/7] hid-asus: Remove more bogus zero bytes from some report descriptors
From: sashiko-bot @ 2026-05-14 8:57 UTC (permalink / raw)
To: Paolo Pisati; +Cc: linux-input, dmitry.torokhov
In-Reply-To: <20260513163248.16483-5-p.pisati@gmail.com>
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] The trailing zero removal loop reads past the end of the report descriptor buffer and lacks a lower bounds check.
- [High] The `memcpy` operation copies more bytes than are available in the source `rdesc` buffer.
- [High] Trimming trailing bytes can shrink the new buffer size below what is required for subsequent fixed-offset memory operations, causing a heap buffer overflow.
--
commit 1a97afa374b1aa0ec76a035ced7d0e6414476f71
Author: Joshua Leivenzon <hacker1024@users.sourceforge.net>
hid-asus: Remove more bogus zero bytes from some report descriptors
This commit attempts to remove bogus trailing zero bytes from specific
HID report descriptors. It adjusts the new report descriptor size by
stripping zeroes from the end and then allocates a new buffer.
> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
> --- a/drivers/hid/hid-asus.c
> +++ b/drivers/hid/hid-asus.c
[ ... ]
> if (*rsize == rsize_orig &&
> rdesc[offs] == 0x09 && rdesc[offs + 1] == 0x76) {
> __u8 *new_rdesc;
> + unsigned int new_rsize = rsize_orig;
>
> + /* Clear bogus trailing zero bytes. */
> + while (rdesc[new_rsize + 1] == 0)
> + --new_rsize;
Since the rdesc buffer contains exactly rsize_orig bytes, does evaluating
rdesc[new_rsize + 1] read two bytes past the end of the allocation?
Additionally, if the adjacent memory happens to contain zeroes, could
new_rsize decrement indefinitely and underflow since there is no lower
bound check?
> + /* Make room for the added bytes. */
> + new_rsize += 2;
> +
> + new_rdesc = devm_kzalloc(&hdev->dev, new_rsize,
> GFP_KERNEL);
> if (!new_rdesc)
> return rdesc;
[ ... ]
> - memcpy(new_rdesc, rdesc, rsize_orig);
> - *rsize = rsize_orig + 1;
> + memcpy(new_rdesc, rdesc, new_rsize);
If no trailing zeroes were stripped, new_rsize is now rsize_orig + 2. Does
this memcpy copy more bytes than are available in the original rdesc
buffer, leading to an out-of-bounds read?
> + *rsize = new_rsize;
> rdesc = new_rdesc;
>
> memmove(rdesc + offs + 4, rdesc + offs + 2, 12);
If trailing zeroes were successfully removed, could new_rsize drop below the
offsets used in this memmove and subsequent assignments?
For example, if new_rsize shrinks below offs + 15, will the fixed-offset
memmove and subsequent assignments write past the end of the newly allocated
new_rdesc buffer?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260513163248.16483-1-p.pisati@gmail.com?part=4
^ permalink raw reply
* Re: [PATCH] amd-sfh-hid: tablet mode switch and asus quirk
From: Helge Bahmann @ 2026-05-14 7:59 UTC (permalink / raw)
To: Jiri Kosina, linux-input
Cc: Nehal Bakulchandra Shah, Sandeep Singh, Basavaraj Natikar,
bentiss
In-Reply-To: <n0s5q66s-p747-612p-o743-pqr0q2r79448@xreary.bet>
On Tue, 12 May 2026, Jiri Kosina wrote:
> On Mon, 27 Apr 2026, Helge Bahmann wrote:
>
> > Add an input driver that interprets the "operation mode" sensor offered
> > by the amd sfh on some laptop models.
> >
> > Add a quirk to make the driver work again with the Asus VivoBook
> > VivoBook (turn off the "disable interrupts" flag).
> >
> > Expose the intr_disable flag as a module parameter in case it turns out
> > to be needed on further laptop models.
> >
> > Signed-off-by: Helge Bahmann <hcb@chaoticmind.net>
>
> Basavaraj, can you please review this one?
Some additional context, maybe helpful for review:
1. The numbers and behavior were extracted from the ACPI tables
(WMI driver of sorts) of the notebook; I don't have access to any
official AMD / ASUS docs or similar.
2. I have an alternate version of this change that is more indirect:
- create a HID driver providing an "abstract table mode" message
- have an input driver attaching to this newly defined HID driver
While that is keeping "more in line" with the current driver
architecture, I am not sure this indirection really helps. Particularly,
there is no "canonical" HID tablet mode switch message defined,
so it all remains completely bespoke. I am happy to change it if
you prefer, but would need your input.
3. Since this is based on Asus VivoBook and its ACPI tables,
there is a possibility that this "op sensor / tablet mode" behavior
is not as universal as I surmise. A point could be made to make this
entire behavior model-dependent (with a mod param to override
/ activate for other models). Happy to take input / advice.
Thanks for your work!
Helge
>
> Thanks,
>
>
^ permalink raw reply
* Re: [PATCH 2/7] hid-asus: Add missing Zenbook Duo hotkeys
From: sashiko-bot @ 2026-05-14 7:49 UTC (permalink / raw)
To: Paolo Pisati; +Cc: linux-input, dmitry.torokhov
In-Reply-To: <20260513163248.16483-3-p.pisati@gmail.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The 'MyASUS' key (0x86) mapping is inconsistent with the existing Asus WMI driver.
--
commit ee67c289461b2da1755d4534b96e25db841e41d8
Author: Joshua Leivenzon <hacker1024@users.sourceforge.net>
hid-asus: Add missing Zenbook Duo hotkeys
This commit adds input mappings for several missing Zenbook Duo hotkeys,
including the MyASUS key, screen swap, and ROG Ally buttons, handling
them through the hid-asus driver.
> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
> index ce246efba74d3..cba638b19465f 100644
> --- a/drivers/hid/hid-asus.c
> +++ b/drivers/hid/hid-asus.c
> @@ -1046,6 +1046,7 @@ static int asus_input_mapping(struct hid_device *hdev,
> case 0x6c: asus_map_key_clear(KEY_SLEEP); break;
> case 0x7c: asus_map_key_clear(KEY_MICMUTE); break;
> case 0x82: asus_map_key_clear(KEY_CAMERA); break;
> + case 0x86: asus_map_key_clear(KEY_CONTROLPANEL); break; /* MyASUS */
Is it intentional to map the MyASUS key to KEY_CONTROLPANEL here?
The existing asus-nb-wmi driver maps the identical 0x86 hardware event
for the MyASUS key to KEY_PROG1.
Mapping this to KEY_CONTROLPANEL in the HID driver might introduce an
inconsistent contract for user-space, where the exact same physical
button on Asus devices yields different keycodes depending on whether the
event is routed through the HID or WMI subsystem.
Could this cause issues for desktop environments attempting to bind
device-specific hotkeys consistently?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260513163248.16483-1-p.pisati@gmail.com?part=2
^ permalink raw reply
* Re: [PATCH] Input: wacom_w8001 - avoid double release of pen input device
From: Peter Hutterer @ 2026-05-14 7:10 UTC (permalink / raw)
To: Guangshuo Li
Cc: Dmitry Torokhov, Kees Cook, Benjamin Tissoires, linux-input,
linux-kernel
In-Reply-To: <20260430071311.451957-1-lgs201920130244@gmail.com>
On Thu, Apr 30, 2026 at 03:13:11PM +0800, Guangshuo Li wrote:
> When registering the touch input device fails after the pen input device
> has already been registered, w8001_connect() jumps to fail4 and
> unregisters w8001->pen_dev. It then falls through to fail1 where
> input_dev_pen is passed to input_free_device().
>
> Once input_register_device() has succeeded, the device must be released
> with input_unregister_device(), and input_free_device() must not be used
> on the same object afterwards. Since input_dev_pen still aliases
> w8001->pen_dev, this can result in a use-after-free or kref underflow.
>
> Clear the local and container aliases after unregistering the pen device
> so that the common cleanup path does not try to free it again.
>
> This issue was found by a static analysis tool I am developing.
>
> Fixes: e0361b70175f0 ("Input: wacom_w8001 - split the touch and pen devices into two devices")
> Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
thanks
Cheers,
Peter
> ---
> drivers/input/touchscreen/wacom_w8001.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/input/touchscreen/wacom_w8001.c b/drivers/input/touchscreen/wacom_w8001.c
> index 45930d731873..a3b283c59cdd 100644
> --- a/drivers/input/touchscreen/wacom_w8001.c
> +++ b/drivers/input/touchscreen/wacom_w8001.c
> @@ -665,8 +665,11 @@ static int w8001_connect(struct serio *serio, struct serio_driver *drv)
> return 0;
>
> fail4:
> - if (w8001->pen_dev)
> + if (w8001->pen_dev) {
> input_unregister_device(w8001->pen_dev);
> + input_dev_pen = NULL;
> + w8001->pen_dev = NULL;
> + }
> fail3:
> serio_close(serio);
> fail2:
> --
> 2.43.0
>
^ permalink raw reply
* Re: [PATCH] input/evdev: move kill_fasync() outside buffer_lock to fix SOFTIRQ deadlock
From: sashiko-bot @ 2026-05-14 7:06 UTC (permalink / raw)
To: Rik van Riel; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260513115000.77ec5975@fangorn>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Moving kill_fasync() outside of client->buffer_lock does not resolve the lock ordering violation because the function is still executing under dev->event_lock.
--
commit 803cd2c3ce0ada2c1fe4625fcbef594cb58290eb
Author: Rik van Riel <riel@surriel.com>
input/evdev: move kill_fasync() outside buffer_lock to fix SOFTIRQ deadlock
This commit attempts to fix a SOFTIRQ-safe to SOFTIRQ-unsafe lock inversion
by moving the kill_fasync() call outside of the client->buffer_lock.
> diff --git a/drivers/input/evdev.c b/drivers/input/evdev.c
> --- a/drivers/input/evdev.c
> +++ b/drivers/input/evdev.c
[ ... ]
> @@ -280,9 +278,11 @@ static void evdev_pass_values(struct evdev_client *client,
>
> spin_unlock(&client->buffer_lock);
>
> - if (wakeup)
> + if (wakeup) {
> + kill_fasync(&client->fasync, SIGIO, POLL_IN);
> wake_up_interruptible_poll(&client->wait,
> EPOLLIN | EPOLLOUT | EPOLLRDNORM | EPOLLWRNORM);
> + }
> }
Does this completely resolve the lock ordering violation?
While kill_fasync() is now outside of client->buffer_lock, evdev_pass_values()
is called from evdev_events(), which is invoked by the input core in
input_pass_values().
During this dispatch, the input core strictly holds dev->event_lock:
drivers/input/input.c:input_pass_values() {
...
rcu_read_lock();
spin_lock_irqsave(&dev->event_lock, flags);
...
handler->events(handle, vals, count);
...
spin_unlock_irqrestore(&dev->event_lock, flags);
}
Since dev->event_lock is also an IRQ/SOFTIRQ-safe spinlock, does this
just shift the problem so we still have a dev->event_lock -> fa_lock
inversion?
Would it make sense to defer the kill_fasync() call entirely outside
the event lock context, perhaps by using irq_work?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260513115000.77ec5975@fangorn?part=1
^ permalink raw reply
* Re: [PATCH] HID: Expose LattePanda IOTA UPS as a power_supply device
From: sashiko-bot @ 2026-05-14 6:50 UTC (permalink / raw)
To: Andrew Maney; +Cc: linux-input, dmitry.torokhov
In-Reply-To: <20260513155723.230707-1-andrewmaney05@gmail.com>
Thank you for your contribution! Sashiko AI review found 7 potential issue(s) to consider:
- [High] Kernel panic due to invalid type cast when transport is not USB (e.g. uhid).
- [High] Kernel panic from dereferencing an ERR_PTR in the raw event handler.
- [High] Exposing fabricated 0 values for unsupported TIME_TO_EMPTY properties causes immediate system shutdown.
- [Medium] Hardcoded sysfs name prevents multiple devices and causes probe failure.
- [Medium] Data race on `ups->charge_limit`.
- [Low] Missing `POWER_SUPPLY` dependency in Kconfig causes build failures.
- [Low] Driver synchronously blocks the probe path for 3 seconds.
--
commit 7d140ce589ec6c637bec5ce8b0c27bf8634322c6
Author: Andrew Maney <andrewmaney05@gmail.com>
Subject: HID: Expose LattePanda IOTA UPS as a power_supply device
This commit adds a driver for the LattePanda IOTA UPS. It exposes battery
status, capacity, and charging status via the power_supply subsystem by parsing
custom HID reports.
> diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
> --- a/drivers/hid/Kconfig
> +++ b/drivers/hid/Kconfig
> @@ -105,6 +105,16 @@ config HID_HAPTIC
>
> menu "Special HID drivers"
>
> +config HID_LATTEPANDA_IOTA_UPS
> + tristate "LattePanda IOTA UPS"
> + depends on USB_HID && X86
If the kernel is configured with CONFIG_POWER_SUPPLY=m and
CONFIG_HID_LATTEPANDA_IOTA_UPS=y, will the built-in HID driver fail to link
against the modular power supply core? Should this include a dependency on
POWER_SUPPLY?
> diff --git a/drivers/hid/hid-lattepanda-iota-ups.c b/drivers/hid/hid-lattepanda-iota-ups.c
> --- /dev/null
> +++ b/drivers/hid/hid-lattepanda-iota-ups.c
[ ... ]
> +static enum power_supply_property iota_ups_properties[] = {
> + POWER_SUPPLY_PROP_STATUS,
> + POWER_SUPPLY_PROP_CAPACITY,
> + POWER_SUPPLY_PROP_PRESENT,
> + POWER_SUPPLY_PROP_ONLINE,
> + POWER_SUPPLY_PROP_SCOPE,
> + POWER_SUPPLY_PROP_TECHNOLOGY,
> + POWER_SUPPLY_PROP_CHARGE_CONTROL_END_THRESHOLD,
> + POWER_SUPPLY_PROP_CAPACITY_LEVEL,
> + POWER_SUPPLY_PROP_TIME_TO_EMPTY_NOW,
> + POWER_SUPPLY_PROP_TIME_TO_FULL_NOW,
If the hardware doesn't support reporting time remaining, is it safe to
include these properties and return 0?
Returning 0 seconds remaining while discharging can signal to userspace power
daemons like UPower or systemd-logind that the battery is empty, which might
trigger an immediate emergency suspend or shutdown. Could these unsupported
properties be omitted from the array entirely?
[ ... ]
> +static int iota_ups_set_property(struct power_supply *supply,
> + enum power_supply_property psp,
> + const union power_supply_propval *val)
> +{
> + struct iota_ups *ups = power_supply_get_drvdata(supply);
> +
> + if (psp == POWER_SUPPLY_PROP_CHARGE_CONTROL_END_THRESHOLD) {
> + /*
> + * V1.0 supports 80% and 100% charge limits only, set via
> + * DIP switch on the board. This property allows userspace
> + * to inform the driver which limit is configured.
> + */
> + if (val->intval != 80 && val->intval != 100)
> + return -EINVAL;
> + ups->charge_limit = val->intval;
Does this introduce a data race on ups->charge_limit?
This field is modified here without holding ups->lock, but it is concurrently
read in iota_ups_raw_event() and iota_ups_get_property() where it is
protected by spin_lock_irqsave(). Should this assignment also be
protected by the spinlock?
[ ... ]
> +static int iota_ups_raw_event(struct hid_device *hdev,
> + struct hid_report *report,
> + u8 *data, int size)
> +{
> + struct iota_ups *ups = hid_get_drvdata(hdev);
[ ... ]
> + /*
> + * Notify the power_supply core outside the spinlock. This
> + * triggers UPower's PropertiesChanged signal with the new values.
> + */
> + if (changed && ups->psu)
> + power_supply_changed(ups->psu);
Could ups->psu contain an error pointer here?
In iota_ups_probe(), if devm_power_supply_register() fails, it returns an
error pointer like ERR_PTR(-ENOMEM), which gets directly assigned to ups->psu.
Because hid_hw_start() is called earlier, raw HID events can be processed
concurrently before the hardware is stopped in the error path. Since an error
pointer is not NULL, the check (changed && ups->psu) would pass, and
power_supply_changed() would be called with the error pointer, leading to a
crash.
Would it be safer to assign the return value of devm_power_supply_register()
to a local variable first, and only update ups->psu on success?
[ ... ]
> +static int iota_ups_probe(struct hid_device *hdev,
> + const struct hid_device_id *id)
> +{
[ ... ]
> + /* Retrieve serial number from the USB descriptor */
> + udev = to_usb_device(hdev->dev.parent->parent);
> + if (udev->serial)
> + strscpy(ups->serial, udev->serial, sizeof(ups->serial));
Can this cause a kernel panic if the HID device is instantiated via userspace
mock drivers like uhid?
When using uhid (which allows setting the bus type to BUS_USB), the parent
device is the uhid character device, not a usb_interface. Dereferencing the
resulting struct usb_device pointer would perform an out-of-bounds memory
read.
Should the driver verify the transport layer using hid_is_usb() before
dereferencing USB-specific structures, or simply use hdev->uniq instead?
[ ... ]
> + /*
> + * Wait for both status and capacity reports before registering.
> + * The device sends reports every ~1 second; 3 seconds is safe.
> + */
> + wait_for_completion_timeout(&ups->got_initial_data,
> + msecs_to_jiffies(3000));
Does blocking the probe path for 3 seconds cause issues for device
enumeration?
Calling wait_for_completion_timeout() synchronously stalls the kernel's
device enumeration thread for that bus, which can unnecessarily delay the
overall system boot process or stall the handling of other devices during
hotplug.
Could the registration be deferred, or could the device be registered
immediately with an unknown state instead of blocking?
> +
> + ups->psu_desc.name = "lattepanda-iota-ups";
Is it possible to have multiple of these UPS devices connected to a single
system?
If a second device is plugged in, it will attempt to register a sysfs
directory with the exact same hardcoded name, which fails with -EEXIST.
This probe failure would then trigger the error pointer panic mentioned
earlier. Should the driver dynamically construct a unique name using
devm_kasprintf() with dev_name(&hdev->dev)?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260513155723.230707-1-andrewmaney05@gmail.com?part=1
^ permalink raw reply
* Re: [PATCH 1/7] hid-asus: Fix up Zenbook Duo report descriptors
From: sashiko-bot @ 2026-05-14 6:50 UTC (permalink / raw)
To: Paolo Pisati; +Cc: linux-input, dmitry.torokhov
In-Reply-To: <20260513163248.16483-2-p.pisati@gmail.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The report descriptor fixup for the Zenbook Duo corrupts the descriptor because it incorrectly shifts only 12 bytes and allocates insufficient memory.
--
commit 6124d2bbfc4e5254d102730c30ac761f188e21b0
Author: Joshua Leivenzon <hacker1024@users.sourceforge.net>
hid-asus: Fix up Zenbook Duo report descriptors
This commit adds a quirk for the Zenbook Duo 2024+ keyboards to fix up an
unmapped Asus vendor usage page code. It reuses the existing report
descriptor fixup logic previously used for T100CHI and T90CHI devices.
> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
> index 3f5e96900b67a..ce246efba74d3 100644
> --- a/drivers/hid/hid-asus.c
> +++ b/drivers/hid/hid-asus.c
[ ... ]
> @@ -1384,17 +1385,20 @@ static const __u8 *asus_report_fixup(struct hid_device *hdev, __u8 *rdesc,
> hid_info(hdev, "Fixing up Asus T100 keyb report descriptor\n");
> rdesc[74] &= ~HID_MAIN_ITEM_CONSTANT;
> }
> - /* For the T100CHI/T90CHI keyboard dock */
> - if (drvdata->quirks & (QUIRK_T100CHI | QUIRK_T90CHI)) {
> + /* For the T100CHI/T90CHI keyboard dock and Zenbook Duo 2024+ keyboards */
> + if (drvdata->quirks & (QUIRK_T100CHI | QUIRK_T90CHI | QUIRK_ZENBOOK_DUO_KEYBOARD)) {
> int rsize_orig;
> int offs;
>
> if (drvdata->quirks & QUIRK_T100CHI) {
> rsize_orig = 403;
> offs = 388;
> - } else {
> + } else if (drvdata->quirks & QUIRK_T90CHI) {
> rsize_orig = 306;
> offs = 291;
> + } else if (drvdata->quirks & QUIRK_ZENBOOK_DUO_KEYBOARD) {
> + rsize_orig = 257;
> + offs = 176;
> }
Could this introduce a regression by corrupting the Zenbook Duo report
descriptor when failing to shift the remainder of the items correctly?
For the Zenbook Duo, the descriptor is 257 bytes long and the offset is 176,
leaving 79 bytes after the replaced item. However, the shared fixup logic
below this code hardcodes a memmove of exactly 12 bytes:
drivers/hid/hid-asus.c:asus_report_fixup() {
...
memcpy(new_rdesc, rdesc, rsize_orig);
*rsize = rsize_orig + 1;
rdesc = new_rdesc;
memmove(rdesc + offs + 4, rdesc + offs + 2, 12);
rdesc[offs] = 0x19;
...
}
Does shifting only 12 bytes overwrite the valid original bytes at offsets 190
and 191, and leave the remaining 67 bytes unshifted?
Additionally, does the new_rdesc allocation properly account for the shift?
Replacing a 2-byte item with a 4-byte item requires shifting the remaining
items forward by 2 bytes. Allocating only rsize_orig + 1 could lead to a
1-byte out-of-bounds write if all 79 bytes are shifted properly.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260513163248.16483-1-p.pisati@gmail.com?part=1
^ permalink raw reply
* RE: [PATCH v3 0/9] iio: introduce devm_ API for hid sensro setup and cleanup
From: Zhang, Lixu @ 2026-05-14 2:47 UTC (permalink / raw)
To: Sanjay Chitroda, jikos@kernel.org, jic23@kernel.org,
srinivas.pandruvada@linux.intel.com
Cc: Lechner, David, nuno.sa@analog.com, andy@kernel.org,
sakari.ailus@linux.intel.com, linux-input@vger.kernel.org,
linux-iio@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <20260509101040.791404-1-sanjayembedded@gmail.com>
Tested-by: Zhang Lixu <lixu.zhang@intel.com>
>-----Original Message-----
>From: Sanjay Chitroda <sanjayembeddedse@gmail.com>
>Sent: Saturday, May 9, 2026 6:11 PM
>To: jikos@kernel.org; jic23@kernel.org; srinivas.pandruvada@linux.intel.com
>Cc: Lechner, David <dlechner@baylibre.com>; nuno.sa@analog.com;
>andy@kernel.org; sanjayembeddedse@gmail.com;
>sakari.ailus@linux.intel.com; linux-input@vger.kernel.org; linux-
>iio@vger.kernel.org; linux-kernel@vger.kernel.org
>Subject: [PATCH v3 0/9] iio: introduce devm_ API for hid sensro setup and
>cleanup
>
>From: Sanjay Chitroda <sanjayembeddedse@gmail.com>
>
>Key highlights:
>- Prepare change as pre-requisite for devm conversion for HID IIO
> drivers by removing redundant argument
>- Add devm API to setup trigger and clenaup resource using
> devm_add_action_or_reset()
>- few cleanup and prepratory changes before updating driver for devm_
>- few sample driver update using devm conversion to auto release resource
>
>changes in v3:
>- Added cleanup and prepratory changes before adding devm_ API
> conversion based on self review: 0002, 0004, 0006, 0007 and 0008
>- Address andy's review comment on commit message and coding style
>- v2 series -> https://lore.kernel.org/all/20260429175918.2541914-1-
>sanjayembedded@gmail.com/
>changes in v2:
>- Following input from Jonathan and Andy, squash initial patch v1
> series in single change as individual change should not break anything
>- Add devm API support and two driver using the same
>- v1 series -> https://lore.kernel.org/all/20260428071613.1134053-1-
>sanjayembedded@gmail.com/
>
>Testing:
> - Compiled with W=1
> - Build-tested on QEMU x86_64
>
>Based on further feedback and reviews, I would extend this series to convert all
>HID IIO driver to use devm_* API.
>
>Thanks,
>Sanjay Chitroda
>
>
>Sanjay Chitroda (9):
> iio: hid-sensors: drop redundant iio_dev argument
> iio: hid-sensors: cleanup codestyle warning
> iio: hid-sensors: introduce device managed API
> iio: gyro: hid-sensor-gyro-3d: cleanup codestyle warning
> iio: gyro: hid-sensor-gyro-3d: drop hid_sensor_remove_trigger() using
> devm API
> iio: humidity: hid-sensor-humidity: cleanup codestyle check
> iio: humidity: hid-sensor-humidity: use common device for devres
> iio: humidity: hid-sensor-humidity: use local struct device
> iio: humidity: hid-sensor-humidity: drop hid_sensor_remove_trigger()
> using devm API
>
> drivers/iio/accel/hid-sensor-accel-3d.c | 4 +-
> .../common/hid-sensors/hid-sensor-trigger.c | 24 +++++++-
> .../common/hid-sensors/hid-sensor-trigger.h | 5 +-
> drivers/iio/gyro/hid-sensor-gyro-3d.c | 16 ++---
> drivers/iio/humidity/hid-sensor-humidity.c | 61 +++++++++----------
> drivers/iio/light/hid-sensor-als.c | 4 +-
> drivers/iio/light/hid-sensor-prox.c | 4 +-
> drivers/iio/magnetometer/hid-sensor-magn-3d.c | 4 +-
>drivers/iio/orientation/hid-sensor-incl-3d.c | 4 +- drivers/iio/orientation/hid-
>sensor-rotation.c | 4 +- .../position/hid-sensor-custom-intel-hinge.c | 4 +-
> drivers/iio/pressure/hid-sensor-press.c | 4 +-
> .../iio/temperature/hid-sensor-temperature.c | 4 +-
> 13 files changed, 78 insertions(+), 64 deletions(-)
>
>
>base-commit: 39b80c5c9830d12d2d6531059001301c4265322a
>--
>2.34.1
>
^ permalink raw reply
* RE: [PATCH v3 0/6] iio: hid-sensor: standardize scan_type initialization
From: Zhang, Lixu @ 2026-05-14 2:44 UTC (permalink / raw)
To: Jonathan Cameron, Natália Salvino André
Cc: andy@kernel.org, Lechner, David, jikos@kernel.org,
nuno.sa@analog.com, srinivas.pandruvada@linux.intel.com,
linux-iio@vger.kernel.org, linux-input@vger.kernel.org
In-Reply-To: <20260512154853.10aa2902@jic23-huawei>
>-----Original Message-----
>From: Zhang, Lixu
>Sent: Wednesday, May 13, 2026 11:27 AM
>To: Jonathan Cameron <jic23@kernel.org>; Natália Salvino André
><natalia.andre@ime.usp.br>
>Cc: andy@kernel.org; Lechner, David <dlechner@baylibre.com>;
>jikos@kernel.org; nuno.sa@analog.com; srinivas.pandruvada@linux.intel.com;
>linux-iio@vger.kernel.org; linux-input@vger.kernel.org
>Subject: RE: [PATCH v3 0/6] iio: hid-sensor: standardize scan_type initialization
>
>>-----Original Message-----
>>From: Jonathan Cameron <jic23@kernel.org>
>>Sent: Tuesday, May 12, 2026 10:49 PM
>>To: Natália Salvino André <natalia.andre@ime.usp.br>
>>Cc: andy@kernel.org; Lechner, David <dlechner@baylibre.com>;
>>jikos@kernel.org; nuno.sa@analog.com;
>>srinivas.pandruvada@linux.intel.com;
>>linux-iio@vger.kernel.org; linux-input@vger.kernel.org; Zhang, Lixu
>><lixu.zhang@intel.com>
>>Subject: Re: [PATCH v3 0/6] iio: hid-sensor: standardize scan_type
>>initialization
>>
>>On Mon, 11 May 2026 22:16:16 -0300
>>Natália Salvino André <natalia.andre@ime.usp.br> wrote:
>>
>>> This series refactors the HID sensor drivers to standardize the
>>> initialization of the iio_chan_spec scan_type structure.
>>> Direct initialization using compound literals was used.
>>>
>>> This change improves code readability and ensures that all fields of
>>> the scan_type structure are properly zero-initialized.
>>> Additionally, for the hid-sensor-accel-3d driver, the channel
>>> initialization loop was refactored to iterate directly over the scan
>>> indices, eliminating redundant logic.
>>
>>Series looks good to me but I'll leave it on the list a while to both
>>let sashiko catch up and anyone else who wants to can have time to
>>look. Also ideally get some testing.
>>
>>Lixu, if you don't mind, please can you give this a test on top of the
>>more radical refactor patches?
>
>Sure, I'll test this series on top of the refactor patches when I have some time.
Hi Jonathan,
I've done some basic testing on this series. Here are the results:
1. No issues seen for hid-sensor-gyro-3d, hid-sensor-als, hid-sensor-prox, hid-sensor-magn-3d.
2. Didn't test hid-sensor-press as I couldn't find a platform with the corresponding device.
3. There's an issue with hid-sensor-accel-3d. Sashiko AI review has already identified the problem and provided review comments.
Thanks,
Lixu
^ permalink raw reply
* [PATCH v2] Input: synaptics - add LEN2058 to SMBus passlist for ThinkPad E490
From: contacto @ 2026-05-14 1:35 UTC (permalink / raw)
To: dmitry.torokhov; +Cc: linux-input, linux-kernel, Nicolás Bazaes
In-Reply-To: <agTso46UcMSGaIYx@google.com>
From: Nicolás Bazaes <contacto@bazaes.cl>
The Lenovo ThinkPad E490 (PNP ID: LEN2058) has a Synaptics TM3471-020
touchpad that supports SMBus/RMI4 mode but is not listed in
smbus_pnp_ids[]. Without this entry, RMI4 over SMBus is not enabled
by default, and the touchpad falls back to PS/2 mode.
Adding LEN2058 to the passlist enables automatic RMI4 detection without
requiring the psmouse.synaptics_intertouch parameter, and matches
the behavior of similar ThinkPad models already in the list
(E480/LEN2054, E580/LEN2055).
Tested on ThinkPad E490 with kernel 7.0.5-zen1 and Arch Linux.
RMI4 over SMBus is confirmed working without any kernel parameters.
Signed-off-by: Nicolás Bazaes <contacto@bazaes.cl>
Assisted-by: Claude:claude-sonnet-4-6
---
drivers/input/mouse/synaptics.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
index 26071128f..c70502e24 100644
--- a/drivers/input/mouse/synaptics.c
+++ b/drivers/input/mouse/synaptics.c
@@ -190,6 +190,7 @@ static const char * const smbus_pnp_ids[] = {
"LEN2044", /* L470 */
"LEN2054", /* E480 */
"LEN2055", /* E580 */
+ "LEN2058", /* E490 */
"LEN2068", /* T14 Gen 1 */
"SYN1221", /* TUXEDO InfinityBook Pro 14 v5 */
"SYN3003", /* HP EliteBook 850 G1 */
--
2.54.0
^ permalink raw reply related
* Re: [PATCH] Input: synaptics - add LEN2058 to SMBus passlist for ThinkPad E490
From: sashiko-bot @ 2026-05-14 1:22 UTC (permalink / raw)
To: contacto; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260513045749.185969-1-contacto@bazaes.cl>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The commit message incorrectly states that without the passlist entry, psmouse ignores the `synaptics_intertouch` parameter and falls back to PS/2 mode even when explicitly requested.
--
commit 18925dd22ddb975fc9e545765ec54eb77a26f6f6
Author: Nicolás Bazaes <contacto@bazaes.cl>
Input: synaptics - add LEN2058 to SMBus passlist for ThinkPad E490
The Lenovo ThinkPad E490 has a Synaptics TM3471-020 touchpad that supports
SMBus/RMI4 mode but is not listed in smbus_pnp_ids[]. Adding LEN2058 to
the passlist enables RMI4 over SMBus, which provides better multitouch
support and matches the behavior of similar ThinkPad models.
> Without this entry, psmouse ignores the
> synaptics_intertouch parameter and falls back to PS/2 mode even when
> explicitly requested.
This isn't a bug, but is this description accurate?
Looking at synaptics_setup_intertouch(), the passlist is only checked if
synaptics_intertouch == SYNAPTICS_INTERTOUCH_NOT_SET:
drivers/input/mouse/synaptics.c:synaptics_setup_intertouch() {
...
if (synaptics_intertouch == SYNAPTICS_INTERTOUCH_NOT_SET) {
if (!psmouse_matches_pnp_id(psmouse, topbuttonpad_pnp_ids) &&
!psmouse_matches_pnp_id(psmouse, smbus_pnp_ids)) {
...
}
If a user explicitly requests SMBus mode by passing
psmouse.synaptics_intertouch=1, the condition is false, bypassing the
passlist check entirely and honoring the user's request.
Could the commit message be updated to clarify that the parameter is not
ignored when explicitly requested?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260513045749.185969-1-contacto@bazaes.cl?part=1
^ permalink raw reply
* Re: [PATCH v5 3/3] HID: nintendo: Add unified report format support
From: sashiko-bot @ 2026-05-14 1:06 UTC (permalink / raw)
To: Vicki Pfau; +Cc: linux-input, dmitry.torokhov
In-Reply-To: <20260512200051.2534081-4-vi@endrift.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Insufficient bounds checking for NS2_REPORT_UNIFIED payload leads to out-of-bounds reads.
- [High] Divide-by-zero kernel panic in switch2_report_trigger() caused by attacker-controlled calibration data.
--
commit 6e45b564fde8014fc6ade8452a1b793097b3bee4
Author: Vicki Pfau <vi@endrift.com>
Subject: HID: nintendo: Add unified report format support
This commit adds support for the "unified" report format to the nintendo hid
driver. It introduces new mappings for overlapping button and axis fields
across different controller types.
> diff --git a/drivers/hid/hid-nintendo.c b/drivers/hid/hid-nintendo.c
> index 9801799a90758..9e7eda7b70bfb 100644
> --- a/drivers/hid/hid-nintendo.c
> +++ b/drivers/hid/hid-nintendo.c
[ ... ]
> @@ -3690,11 +3791,48 @@ static int switch2_event(struct hid_device *hdev, struct hid_report *report, uin
>
> switch (report->id) {
> case NS2_REPORT_UNIFIED:
> - /*
> - * TODO
> - * This won't be sent unless the report type gets changed via command
> - * 03-0A, but we should support it at some point regardless.
> - */
> + switch (ns2->ctlr_type) {
> + case NS2_CTLR_TYPE_JCL:
> + switch2_report_stick(input, &ns2->stick_calib[0],
> + ABS_X, false, ABS_Y, true, &raw_data[11]);
> + switch2_report_buttons(input, &raw_data[5],
> + ns2_left_joycon_button_unified_mappings);
> + break;
> + case NS2_CTLR_TYPE_JCR:
> + switch2_report_stick(input, &ns2->stick_calib[0],
> + ABS_X, false, ABS_Y, true, &raw_data[14]);
Is this safe if the report size is exactly 15 bytes?
Earlier in switch2_event(), there is a global size check:
if (size < 15)
return -EINVAL;
However, switch2_report_stick() reads up to 3 bytes from the provided data
pointer (indices 0, 1, and 2). Passing &raw_data[14] means it will read
raw_data[14], raw_data[15], and raw_data[16].
Could this cause an out-of-bounds read if an undersized report is sent?
> + switch2_report_buttons(input, &raw_data[5],
> + ns2_right_joycon_button_unified_mappings);
> + break;
> + case NS2_CTLR_TYPE_GC:
> + input_report_abs(input, ABS_HAT0X,
> + !!(raw_data[7] & NS2_BTN_U3_RIGHT) -
> + !!(raw_data[7] & NS2_BTN_U3_LEFT));
> + input_report_abs(input, ABS_HAT0Y,
> + !!(raw_data[7] & NS2_BTN_U3_DOWN) -
> + !!(raw_data[7] & NS2_BTN_U3_UP));
> + switch2_report_buttons(input, &raw_data[5], ns2_gccon_unified_mappings);
> + switch2_report_stick(input, &ns2->stick_calib[0],
> + ABS_X, false, ABS_Y, true, &raw_data[11]);
> + switch2_report_stick(input, &ns2->stick_calib[1],
> + ABS_RX, false, ABS_RY, true, &raw_data[14]);
> + switch2_report_trigger(input, ns2->lt_zero, ABS_Z, raw_data[0x3d]);
> + switch2_report_trigger(input, ns2->rt_zero, ABS_RZ, raw_data[0x3e]);
Similarly, does this code read past the end of the buffer?
Accessing raw_data[0x3d] (index 61) and raw_data[0x3e] (index 62) requires
the report size to be at least 63 bytes, but the global check only guarantees
15 bytes.
Additionally, could this new path trigger a divide-by-zero panic?
While this issue pre-dates this commit, switch2_report_trigger() contains the
following calculation:
int value = (NS2_TRIGGER_RANGE + 1) * (data - zero) / (232 - zero);
The zero parameter here comes from the device's factory calibration data
(ns2->lt_zero and ns2->rt_zero). If a device advertises 232 as its zero
calibration point, this division evaluates to 0.
Since this patch exposes a new way to call switch2_report_trigger(), might it
be worth adding validation to prevent a zero divisor?
> + break;
> + case NS2_CTLR_TYPE_PRO:
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260512200051.2534081-1-vi@endrift.com?part=3
^ permalink raw reply
* Re: [PATCH v5 2/3] HID: nintendo: Add rumble support for Switch 2 controllers
From: sashiko-bot @ 2026-05-14 0:53 UTC (permalink / raw)
To: Vicki Pfau; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260512200051.2534081-3-vi@endrift.com>
Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [High] Mixing jiffies and milliseconds in delayed work scheduling logic causes an integer underflow, permanently stalling the haptic rumble.
- [High] A guaranteed NULL pointer dereference occurs in the error logging path of switch2_rumble_work, as well as a TOCTOU race condition.
- [Medium] `ns2->ctlr_type` is checked before it is dynamically initialized, causing union corruption for GameCube controllers.
- [Low] Implicit type promotion in the max() macro causes a compiler warning.
- [Low] Kconfig documentation claims individual motor control for the Pro Controller, but the implementation identicalizes the rumble data for both motors.
--
commit d72f0f8e0dadde2cacc27050b92088c2e5cbea0e
Author: Vicki Pfau <vi@endrift.com>
Subject: HID: nintendo: Add rumble support for Switch 2 controllers
This commit introduces rumble support for Switch 2 controllers, handling both HD
Rumble linear resonant actuators and eccentric rotating mass haptics. It adds
state machines for rumble scheduling and output reporting via hdev.
> diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
> index 26ef589f652c6..a7fe74d982475 100644
> --- a/drivers/hid/Kconfig
> +++ b/drivers/hid/Kconfig
> @@ -850,10 +850,10 @@ config NINTENDO_FF
> depends on HID_NINTENDO
> select INPUT_FF_MEMLESS
> help
> - Say Y here if you have a Nintendo Switch controller and want to enable
> - force feedback support for it. This works for both joy-cons, the pro
> - controller, and the NSO N64 controller. For the pro controller, both
> - rumble motors can be controlled individually.
> + Say Y here if you have a Nintendo Switch or Switch 2 controller and want
> + to enable force feedback support for it. This works for Joy-Cons, the Pro
> + Controllers, and the NSO N64 and GameCube controller. For the Pro
> + Controller, both rumble motors can be controlled individually.
This isn't a bug, but is the help text accurate regarding individual motor
control? The implementation in switch2_rumble_work() appears to encode the
exact same rumble configuration to both the left and right motor offsets,
which would make both motors vibrate identically.
> diff --git a/drivers/hid/hid-nintendo.c b/drivers/hid/hid-nintendo.c
> index 2193afcdefabc..9801799a90758 100644
> --- a/drivers/hid/hid-nintendo.c
> +++ b/drivers/hid/hid-nintendo.c
[ ... ]
> @@ -3103,6 +3155,107 @@ static inline bool switch2_ctlr_is_joycon(enum switch2_ctlr_type type)
> return type == NS2_CTLR_TYPE_JCL || type == NS2_CTLR_TYPE_JCR;
> }
>
> +#if IS_ENABLED(CONFIG_NINTENDO_FF)
> +static void switch2_encode_rumble(struct switch2_hd_rumble *rumble, uint8_t buffer[5])
> +{
> + buffer[0] = rumble->hi_freq;
> + buffer[1] = (rumble->hi_freq >> 8) | (rumble->hi_amp << 2);
> + buffer[2] = (rumble->hi_amp >> 6) | (rumble->lo_freq << 4);
> + buffer[3] = (rumble->lo_freq >> 4) | (rumble->lo_amp << 6);
> + buffer[4] = rumble->lo_amp >> 2;
> +}
> +
> +static int switch2_play_effect(struct input_dev *dev, void *data, struct ff_effect *effect)
> +{
> + struct switch2_controller *ns2 = input_get_drvdata(dev);
> +
> + if (effect->type != FF_RUMBLE)
> + return 0;
> +
> + guard(spinlock_irqsave)(&ns2->rumble_lock);
> + if (ns2->ctlr_type == NS2_CTLR_TYPE_GC) {
> + ns2->rumble.sd.amplitude = max(effect->u.rumble.strong_magnitude,
> + effect->u.rumble.weak_magnitude >> 1);
Will this max calculation trigger a compiler warning? The strict type checking
in the max macro might fail because effect->u.rumble.weak_magnitude >> 1
promotes to an int, while strong_magnitude is a u16.
> + } else {
> + ns2->rumble.hd.hi_amp = effect->u.rumble.weak_magnitude * RUMBLE_MAX >> 16;
> + ns2->rumble.hd.lo_amp = effect->u.rumble.strong_magnitude * RUMBLE_MAX >> 16;
> + }
> +
> + if (ns2->hdev)
> + schedule_delayed_work(&ns2->rumble_work, 0);
> +
> + return 0;
> +}
> +
> +static void switch2_rumble_work(struct work_struct *work)
> +{
> + struct switch2_controller *ns2 = container_of(to_delayed_work(work),
> + struct switch2_controller, rumble_work);
> + unsigned long current_ms = jiffies_to_msecs(get_jiffies_64());
> + unsigned long flags;
> + bool active;
> + int ret;
[ ... ]
> + if (active) {
> + unsigned long interval = msecs_to_jiffies(4);
> +
> + if (!ns2->last_rumble_work)
> + ns2->last_rumble_work = current_ms;
> + else
> + ns2->last_rumble_work += interval;
> + schedule_delayed_work(&ns2->rumble_work,
> + ns2->last_rumble_work + interval - current_ms);
Does this code underflow the scheduled delay?
The current_ms variable is retrieved in milliseconds using jiffies_to_msecs(),
but interval is calculated in jiffies using msecs_to_jiffies(). Since
ns2->last_rumble_work is initialized to milliseconds but incremented by
jiffies, subtracting current_ms could yield a negative value.
On systems where HZ is not 1000, could this underflow the delay parameter and
schedule the next work millions of jiffies in the future?
> + } else {
> + ns2->last_rumble_work = 0;
> + }
> + spin_unlock_irqrestore(&ns2->rumble_lock, flags);
> +
> + if (!ns2->hdev) {
> + cancel_delayed_work(&ns2->rumble_work);
> + ret = -ENODEV;
> + } else {
> + ret = hid_hw_output_report(ns2->hdev, ns2->rumble_buffer, 64);
> + }
> +
> + if (ret < 0)
> + hid_dbg(ns2->hdev, "Failed to send output report ret=%d\n", ret);
If ns2->hdev is NULL, we set ret = -ENODEV and then reach this hid_dbg call.
Could this result in a NULL pointer dereference, as the hid_dbg macro passes
ns2->hdev to dev_dbg()?
Also, since ns2->hdev is accessed locklessly and without READ_ONCE(), is there
a time-of-check to time-of-use race where the compiler might reload the pointer
between the check and hid_hw_output_report()?
[ ... ]
> @@ -3881,6 +4048,14 @@ static int switch2_probe(struct hid_device *hdev, const struct hid_device_id *id
>
> switch2_leds_create(ns2);
>
> +#if IS_ENABLED(CONFIG_NINTENDO_FF)
> + if (ns2->ctlr_type != NS2_CTLR_TYPE_GC) {
> + ns2->rumble.hd.hi_freq = RUMBLE_HI_FREQ;
> + ns2->rumble.hd.lo_freq = RUMBLE_LO_FREQ;
> + }
Are we checking ns2->ctlr_type before it is dynamically initialized?
During probe, ns2->ctlr_type appears to be 0 since the struct is freshly
allocated and the type is discovered later during the init state machine.
This would cause the high and low frequencies to always be written.
Since hd and sd share a union, could this corrupt the sd.error and
sd.amplitude fields for actual GameCube controllers?
> + spin_lock_init(&ns2->rumble_lock);
> + INIT_DELAYED_WORK(&ns2->rumble_work, switch2_rumble_work);
> +#endif
> hid_set_drvdata(hdev, ns2);
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260512200051.2534081-1-vi@endrift.com?part=2
^ permalink raw reply
* Re: [PATCH v5 1/3] HID: nintendo: Add preliminary Switch 2 controller driver
From: sashiko-bot @ 2026-05-14 0:15 UTC (permalink / raw)
To: Vicki Pfau; +Cc: linux-input, dmitry.torokhov
In-Reply-To: <20260512200051.2534081-2-vi@endrift.com>
Thank you for your contribution! Sashiko AI review found 11 potential issue(s) to consider:
- [High] Sleeping in atomic context and AB-BA deadlock in `switch2_usb_disconnect`
- [High] Race condition in IN URB tracking causes permanent starvation
- [High] Dangling pointers and use-after-free on probe failure
- [High] Memory corruption and double free due to incorrect devm input lifecycle management
- [High] NULL pointer dereference in USB probe error path
- [High] Division by zero crash from unvalidated trigger calibration data
- [High] BUG_ON kernel panic when `ida_free` is called with U32_MAX
- [High] Out-of-bounds read when parsing NS2_CMD_FW_INFO response
- [High] Race condition due to URB submission before initialization of synchronization primitives
- [Medium] Infinite USB retry storm on protocol errors
- [Medium] Unaligned memory access on byte array casts
--
commit 00cb7265ad5983144c6923702e2ee2c3c047d29e
Author: Vicki Pfau <vi@endrift.com>
Subject: HID: nintendo: Add preliminary Switch 2 controller driver
This adds a new driver for the Switch 2 controllers. The Switch 2 uses an
unusual split-interface design such that input and rumble occur on the main
HID interface, but all other communication occurs over a "configuration"
interface.
> diff --git a/drivers/hid/hid-nintendo.c b/drivers/hid/hid-nintendo.c
> index 29008c2cc5304..2193afcdefabc 100644
> --- a/drivers/hid/hid-nintendo.c
> +++ b/drivers/hid/hid-nintendo.c
[ ... ]
> +static int switch2_init_input(struct switch2_controller *ns2)
> +{
[ ... ]
> + input = devm_input_allocate_device(&hdev->dev);
> + if (!input)
> + return -ENOMEM;
[ ... ]
> + ret = input_register_device(input);
> + if (ret < 0) {
> + hid_err(ns2->hdev, "Failed to register input; ret=%d\n", ret);
> + input_free_device(input);
Calling input_free_device or input_unregister_device on a devm-managed
input device can cause double frees when devres cleanup fires. Can this
lead to memory corruption?
> + return ret;
> + }
[ ... ]
> +static void switch2_controller_put(struct switch2_controller *ns2)
> +{
> + struct input_dev *input;
> + bool do_free;
[ ... ]
> + if (input)
> + input_unregister_device(input);
Calling input_unregister_device on a devm-managed input device can cause
issues. If the input device remains alive (tied to the HID device devres)
and its child LEDs are not released, will reconnecting the USB cable
allocate a new input device and corrupt the LED lists by re-registering
the exact same ns2->leds structures?
> +
> + if (do_free) {
[ ... ]
> +static void switch2_report_trigger(struct input_dev *input, uint8_t zero, int abs, uint8_t data)
> +{
> + int value = (NS2_TRIGGER_RANGE + 1) * (data - zero) / (232 - zero);
The zero variable is loaded directly from the controller's factory calibration
flash. If a device reports a zero point of exactly 232, will this trigger a
division by zero exception and crash the kernel?
> +
> + input_report_abs(input, abs, clamp(value, 0, NS2_TRIGGER_RANGE));
> +}
[ ... ]
> +static int switch2_read_flash(struct switch2_controller *ns2, uint32_t address,
> + uint8_t size)
> +{
> + uint8_t message[8] = { size, 0x7e };
> +
> + if (!ns2->cfg)
> + return -ENOTCONN;
> + *(__le32 *)&message[4] = __cpu_to_le32(address);
The driver directly casts &message[4] to __le32. Since message is a uint8_t
array, it is only guaranteed to be 1-byte aligned. On strict architectures,
will this unaligned memory write trigger a hardware alignment fault?
> + return ns2->cfg->send_command(NS2_CMD_FLASH, NS2_SUBCMD_FLASH_READ, message,
> + sizeof(message), ns2->cfg);
> +}
[ ... ]
> +int switch2_receive_command(struct switch2_controller *ns2,
> + const uint8_t *message, size_t length)
> +{
[ ... ]
> + header = (const struct switch2_cmd_header *)message;
> + if (!(header->flags & NS2_FLAG_OK)) {
> + ret = -EIO;
> + goto exit;
> + }
> + message = &message[8];
> + switch (header->command) {
[ ... ]
> + case NS2_CMD_FW_INFO:
> + if (header->subcommand == NS2_SUBCMD_FW_INFO_GET) {
> + if (length < sizeof(ns2->version)) {
> + ret = -EINVAL;
> + goto exit;
> + }
> + memcpy(&ns2->version, message, sizeof(ns2->version));
The message pointer is advanced by 8 bytes to skip the header, but length
is not decremented. The bounds check compares length against
sizeof(ns2->version). If length is exactly sizeof(ns2->version), the check
passes, but the advanced message pointer only has length - 8 valid bytes
remaining. Will the memcpy over-read into uninitialized memory?
> + ns2->ctlr_type = ns2->version.ctlr_type;
> + switch2_init_step_done(ns2, NS2_INIT_GET_FIRMWARE_INFO);
> + }
> + break;
> + default:
> + break;
> + }
> +
> +exit:
> + if (ns2->init_step < NS2_INIT_DONE)
> + switch2_init_controller(ns2);
If a command response has the NACK flag set or encounters another error,
the code jumps to exit and invokes switch2_init_controller. Since
ns2->init_step has not advanced, the state machine will immediately re-send
the exact same command. Can this cause an infinite tight feedback loop of
failures and retries, flooding the USB bus?
> +
> + return ret;
> +}
[ ... ]
> +int switch2_controller_attach_cfg(const char *phys, struct switch2_cfg_intf *cfg)
> +{
[ ... ]
> + guard(mutex)(&ns2->lock);
> + WARN_ON(ns2->cfg);
> + ns2->cfg = cfg;
> +
> + if (ns2->hdev)
> + return switch2_init_controller(ns2);
If switch2_init_controller fails here or in switch2_probe, the error is
returned immediately without unrolling the state. Will ns2->cfg or ns2->hdev
remain set, potentially leading to a use-after-free when subsequent device
events occur?
> + return 0;
> +}
[ ... ]
> +static void switch2_remove(struct hid_device *hdev)
> +{
> + struct switch2_controller *ns2 = hid_get_drvdata(hdev);
> +
> + hid_hw_close(hdev);
> + mutex_lock(&ns2->lock);
> + WARN_ON(ns2->hdev != hdev);
> + ns2->hdev = NULL;
> + mutex_unlock(&ns2->lock);
> + ida_free(&nintendo_player_id_allocator, ns2->player_id);
If ida_alloc fails during probe, ns2->player_id remains at its default
value of U32_MAX. When ida_free is called, the IDA framework enforces a
check on the ID. Casting U32_MAX to a signed integer yields -1. Will this
trigger the BUG_ON and cause a kernel panic?
> + switch2_controller_put(ns2);
> + hid_hw_stop(hdev);
> +}
[ ... ]
> diff --git a/drivers/input/joystick/nintendo-switch2-usb.c b/drivers/input/joystick/nintendo-switch2-usb.c
> new file mode 100644
> index 0000000000000..ebd89d852e21a
> --- /dev/null
> +++ b/drivers/input/joystick/nintendo-switch2-usb.c
[ ... ]
> +static void switch2_bulk_in(struct urb *urb)
> +{
[ ... ]
> + spin_lock_irqsave(&ns2_usb->bulk_in_lock, flags);
> + for (i = 0; i < NS2_IN_URBS; i++) {
> + int err;
> + struct switch2_urb *ns2_urb;
> +
> + if (ns2_usb->bulk_in[i].urb == urb) {
> + ns2_usb->message_in = i;
If multiple URBs complete in quick succession before the work item executes,
the message_in index is overwritten. Will this cause the work item to only
process the last completed URB, leaving earlier URBs indefinitely active
and leading to starvation of the URB pool?
> + continue;
> + }
[ ... ]
> +static int switch2_usb_probe(struct usb_interface *intf, const struct usb_device_id *id)
> +{
[ ... ]
> + ns2_usb->bulk_in[0].active = true;
> + ret = usb_submit_urb(ns2_usb->bulk_in[0].urb, GFP_ATOMIC);
> + if (ret < 0)
> + goto err_free_out;
> +
> + init_usb_anchor(&ns2_usb->bulk_out_anchor);
> + spin_lock_init(&ns2_usb->bulk_out_lock);
> + init_usb_anchor(&ns2_usb->bulk_in_anchor);
> + spin_lock_init(&ns2_usb->bulk_in_lock);
> + INIT_WORK(&ns2_usb->message_in_work, switch2_usb_message_in_work);
usb_submit_urb is invoked for bulk_in[0] before the initialization of the
spinlocks, anchors, and work struct. If the URB completes immediately, the
completion handler will execute and attempt to use uninitialized
synchronization primitives. Can this cause a crash?
> +
> + usb_set_intfdata(intf, ns2_usb);
[ ... ]
> +err_kill_urb:
> + usb_kill_urb(ns2_usb->bulk_in[0].urb);
> +err_free_out:
> + for (i = 0; i < NS2_OUT_URBS; i++) {
> + usb_free_coherent(ns2_usb->udev, NS2_BULK_SIZE, ns2_usb->bulk_out[i].data,
> + ns2_usb->bulk_out[i].urb->transfer_dma);
> + usb_free_urb(ns2_usb->bulk_out[i].urb);
> + }
> +err_free_in:
> + for (i = 0; i < NS2_IN_URBS; i++) {
> + usb_free_coherent(ns2_usb->udev, NS2_BULK_SIZE, ns2_usb->bulk_in[i].data,
> + ns2_usb->bulk_in[i].urb->transfer_dma);
If usb_alloc_urb fails during the loop, it jumps to err_free_in. During
cleanup, it accesses ns2_usb->bulk_in[i].urb->transfer_dma. Since urb is
NULL for the failed index, will this trigger a NULL pointer dereference?
> + usb_free_urb(ns2_usb->bulk_in[i].urb);
> + }
> + devm_kfree(&intf->dev, ns2_usb);
> +
> + return ret;
> +}
> +
> +static void switch2_usb_disconnect(struct usb_interface *intf)
> +{
> + struct switch2_usb *ns2_usb = usb_get_intfdata(intf);
> + unsigned long flags;
> + int i;
> +
> + spin_lock_irqsave(&ns2_usb->bulk_out_lock, flags);
> + usb_kill_anchored_urbs(&ns2_usb->bulk_out_anchor);
Both usb_kill_anchored_urbs and cancel_work_sync can sleep. Is it safe to
call them while holding a spinlock?
> + for (i = 0; i < NS2_OUT_URBS; i++) {
> + usb_free_coherent(ns2_usb->udev, NS2_BULK_SIZE, ns2_usb->bulk_out[i].data,
> + ns2_usb->bulk_out[i].urb->transfer_dma);
> + usb_free_urb(ns2_usb->bulk_out[i].urb);
> + }
> + spin_unlock_irqrestore(&ns2_usb->bulk_out_lock, flags);
> +
> + spin_lock_irqsave(&ns2_usb->bulk_in_lock, flags);
> + usb_kill_anchored_urbs(&ns2_usb->bulk_in_anchor);
> + cancel_work_sync(&ns2_usb->message_in_work);
Furthermore, cancel_work_sync waits for message_in_work which attempts to
acquire bulk_in_lock. Does this cause a deadlock since the lock is already
held by the disconnecting thread?
> + for (i = 0; i < NS2_IN_URBS; i++) {
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260512200051.2534081-1-vi@endrift.com?part=1
^ permalink raw reply
* [PATCH v2 4/4] HID: hid-msi: Add Rumble Intensity Attributes
From: Derek J. Clark @ 2026-05-13 23:14 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: Pierre-Loup A . Griffais, Denis Benato, Zhouwang Huang,
Derek J . Clark, linux-input, linux-doc, linux-kernel
In-Reply-To: <20260513231445.3213501-1-derekjohn.clark@gmail.com>
Adds intensity adjustment for the left and right rumble motors.
Claude was used during the reverse-engineering data gathering for this
feature done by Zhouwang Huang. As the code had already been affected,
I used Claude to create the initial framing for the feature, then did
manual cleanup of the _show and _store functions afterwards to fix bugs
and keep the coding style consistent. Claude was also used as an initial
reviewer of this patch.
Assisted-by: Claude:claude-sonnet-4-6
Co-developed-by: Zhouwang Huang <honjow311@gmail.com>
Signed-off-by: Zhouwang Huang <honjow311@gmail.com>
Signed-off-by: Derek J. Clark <derekjohn.clark@gmail.com>
---
v2:
- Use pending_profile and sync to rom mutexes.
---
drivers/hid/hid-msi.c | 147 ++++++++++++++++++++++++++++++++++++++++++
1 file changed, 147 insertions(+)
diff --git a/drivers/hid/hid-msi.c b/drivers/hid/hid-msi.c
index a628b77bfb7b5..cffd6ed253ec9 100644
--- a/drivers/hid/hid-msi.c
+++ b/drivers/hid/hid-msi.c
@@ -76,6 +76,8 @@ enum claw_profile_ack_pending {
CLAW_M1_PENDING,
CLAW_M2_PENDING,
CLAW_RGB_PENDING,
+ CLAW_RUMBLE_LEFT_PENDING,
+ CLAW_RUMBLE_RIGHT_PENDING,
};
enum claw_key_index {
@@ -262,6 +264,11 @@ static const u16 button_mapping_addr_new[] = {
static const u16 rgb_addr_old = 0x01fa;
static const u16 rgb_addr_new = 0x024a;
+static const u16 rumble_addr[] = {
+ 0x0022, /* left */
+ 0x0023, /* right */
+};
+
struct claw_command_report {
u8 report_id;
u8 padding[2];
@@ -310,7 +317,10 @@ struct claw_drvdata {
enum claw_gamepad_mode_index gamepad_mode;
u8 m1_codes[CLAW_KEYS_MAX];
u8 m2_codes[CLAW_KEYS_MAX];
+ u8 rumble_intensity_right;
+ u8 rumble_intensity_left;
const u16 *bmap_addr;
+ bool rumble_support;
bool bmap_support;
/* RGB Variables */
@@ -402,6 +412,12 @@ static int claw_profile_event(struct claw_drvdata *drvdata, struct claw_command_
memcpy(&drvdata->rgb_frames[f_idx], &frame->zone_data,
sizeof(struct rgb_frame));
+ break;
+ case CLAW_RUMBLE_LEFT_PENDING:
+ drvdata->rumble_intensity_left = cmd_rep->data[4];
+ break;
+ case CLAW_RUMBLE_RIGHT_PENDING:
+ drvdata->rumble_intensity_right = cmd_rep->data[4];
break;
default:
dev_warn(&drvdata->hdev->dev,
@@ -815,6 +831,126 @@ static ssize_t button_mapping_options_show(struct device *dev,
}
static DEVICE_ATTR_RO(button_mapping_options);
+static ssize_t rumble_intensity_left_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ u8 data[] = { 0x01, (rumble_addr[0] >> 8) & 0xff, rumble_addr[0] & 0xff, 0x01, 0x00 };
+ struct hid_device *hdev = to_hid_device(dev);
+ struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
+ u8 val;
+ int ret;
+
+ ret = kstrtou8(buf, 10, &val);
+ if (ret)
+ return ret;
+
+ if (val > 100)
+ return -EINVAL;
+
+ data[4] = val;
+
+ guard(mutex)(&drvdata->rom_mutex);
+ ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_WRITE_PROFILE_DATA,
+ data, ARRAY_SIZE(data), 8);
+ if (ret)
+ return ret;
+
+ ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_SYNC_TO_ROM, NULL, 0, 8);
+ if (ret)
+ return ret;
+
+ drvdata->rumble_intensity_left = val;
+
+ return count;
+}
+
+static ssize_t rumble_intensity_left_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ u8 data[4] = { 0x01, (rumble_addr[0] >> 8) & 0xff, rumble_addr[0] & 0xff, 0x01 };
+ struct hid_device *hdev = to_hid_device(dev);
+ struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
+ int ret;
+
+ guard(mutex)(&drvdata->profile_mutex);
+ drvdata->profile_pending = CLAW_RUMBLE_LEFT_PENDING;
+ ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_READ_PROFILE, data,
+ ARRAY_SIZE(data), 8);
+ if (ret) {
+ drvdata->profile_pending = CLAW_NO_PENDING;
+ return ret;
+ }
+
+ return sysfs_emit(buf, "%u\n", drvdata->rumble_intensity_left);
+}
+static DEVICE_ATTR_RW(rumble_intensity_left);
+
+static ssize_t rumble_intensity_right_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ u8 data[] = { 0x01, (rumble_addr[1] >> 8) & 0xff, rumble_addr[1] & 0xff, 0x01, 0x00 };
+ struct hid_device *hdev = to_hid_device(dev);
+ struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
+ u8 val;
+ int ret;
+
+ ret = kstrtou8(buf, 10, &val);
+ if (ret)
+ return ret;
+
+ if (val > 100)
+ return -EINVAL;
+
+ data[4] = val;
+
+ guard(mutex)(&drvdata->rom_mutex);
+ ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_WRITE_PROFILE_DATA,
+ data, ARRAY_SIZE(data), 8);
+ if (ret)
+ return ret;
+
+ ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_SYNC_TO_ROM, NULL, 0, 8);
+ if (ret)
+ return ret;
+
+ drvdata->rumble_intensity_right = val;
+
+ return count;
+}
+
+static ssize_t rumble_intensity_right_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ u8 data[4] = { 0x01, (rumble_addr[1] >> 8) & 0xff, rumble_addr[1] & 0xff, 0x01 };
+ struct hid_device *hdev = to_hid_device(dev);
+ struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
+ int ret;
+
+ guard(mutex)(&drvdata->profile_mutex);
+ drvdata->profile_pending = CLAW_RUMBLE_RIGHT_PENDING;
+ ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_READ_PROFILE, data,
+ ARRAY_SIZE(data), 8);
+ if (ret) {
+ drvdata->profile_pending = CLAW_NO_PENDING;
+ return ret;
+ }
+
+ return sysfs_emit(buf, "%u\n", drvdata->rumble_intensity_right);
+}
+static DEVICE_ATTR_RW(rumble_intensity_right);
+
+static ssize_t rumble_intensity_range_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ return sysfs_emit(buf, "0-100\n");
+}
+static DEVICE_ATTR_RO(rumble_intensity_range);
+
static umode_t claw_gamepad_attr_is_visible(struct kobject *kobj, struct attribute *attr,
int n)
{
@@ -835,6 +971,12 @@ static umode_t claw_gamepad_attr_is_visible(struct kobject *kobj, struct attribu
attr == &dev_attr_reset.attr)
return attr->mode;
+ /* Hide rumble attrs if not supported */
+ if (attr == &dev_attr_rumble_intensity_left.attr ||
+ attr == &dev_attr_rumble_intensity_right.attr ||
+ attr == &dev_attr_rumble_intensity_range.attr)
+ return drvdata->rumble_support ? attr->mode : 0;
+
/* Hide button mapping attrs if it isn't supported */
return drvdata->bmap_support ? attr->mode : 0;
}
@@ -848,6 +990,9 @@ static struct attribute *claw_gamepad_attrs[] = {
&dev_attr_mkeys_function.attr,
&dev_attr_mkeys_function_index.attr,
&dev_attr_reset.attr,
+ &dev_attr_rumble_intensity_left.attr,
+ &dev_attr_rumble_intensity_right.attr,
+ &dev_attr_rumble_intensity_range.attr,
NULL,
};
@@ -1312,6 +1457,7 @@ static void claw_features_supported(struct claw_drvdata *drvdata)
drvdata->bmap_support = true;
if (minor >= 0x66) {
drvdata->bmap_addr = button_mapping_addr_new;
+ drvdata->rumble_support = true;
drvdata->rgb_addr = rgb_addr_new;
} else {
drvdata->bmap_addr = button_mapping_addr_old;
@@ -1323,6 +1469,7 @@ static void claw_features_supported(struct claw_drvdata *drvdata)
if ((major == 0x02 && minor >= 0x17) || major >= 0x03) {
drvdata->bmap_support = true;
drvdata->bmap_addr = button_mapping_addr_new;
+ drvdata->rumble_support = true;
drvdata->rgb_addr = rgb_addr_new;
return;
}
--
2.53.0
^ permalink raw reply related
* [PATCH v2 3/4] HID: hid-msi: Add RGB control interface
From: Derek J. Clark @ 2026-05-13 23:14 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: Pierre-Loup A . Griffais, Denis Benato, Zhouwang Huang,
Derek J . Clark, linux-input, linux-doc, linux-kernel
In-Reply-To: <20260513231445.3213501-1-derekjohn.clark@gmail.com>
Adds RGB control interface for MSI Claw devices. The MSI Claw uses a
fairly unique RGB interface. It has 9 total zones (4 per joystick ring
and 1 for the ABXY buttons), and supports up to 8 sequential frames of
RGB zone data. Each frame is written to a specific area of MCU memory by
the profile command, the value of which changes based on the firmware of
the device. Unlike other devices (such as the Legion Go or the OneXPlayer
devices), there are no hard coded effects built into the MCU. Instead,
the basic effects are provided as a series of frame data. I have
mirrored the effects available in Windows in this driver, while keeping
the effect names consistent with the Lenovo drivers for the effects that
are similar.
Initial reverse-engineering and implementation of this feature was done
by Zhouwang Huang. I refactored the overall format to conform to kernel
driver best practices and style guides. Claude was used as an initial
reviewer of this patch.
Assisted-by: Claude:claude-sonnet-4-6
Co-developed-by: Zhouwang Huang <honjow311@gmail.com>
Signed-off-by: Zhouwang Huang <honjow311@gmail.com>
Signed-off-by: Derek J. Clark <derekjohn.clark@gmail.com>
---
v2:
- Use pending_profile mutex
- Remove deadlock in cfg_setup, return on errors.
---
drivers/hid/hid-msi.c | 548 +++++++++++++++++++++++++++++++++++++++++-
1 file changed, 542 insertions(+), 6 deletions(-)
diff --git a/drivers/hid/hid-msi.c b/drivers/hid/hid-msi.c
index 13ba2747fdb66..a628b77bfb7b5 100644
--- a/drivers/hid/hid-msi.c
+++ b/drivers/hid/hid-msi.c
@@ -21,6 +21,7 @@
#include <linux/device.h>
#include <linux/hid.h>
#include <linux/kobject.h>
+#include <linux/led-class-multicolor.h>
#include <linux/leds.h>
#include <linux/module.h>
#include <linux/mutex.h>
@@ -42,6 +43,10 @@
#define CLAW_KEYS_MAX 5
+#define CLAW_RGB_ZONES 9
+#define CLAW_RGB_MAX_FRAMES 8
+#define CLAW_RGB_FRAME_OFFSET 0x24
+
enum claw_command_index {
CLAW_COMMAND_TYPE_READ_PROFILE = 0x04,
CLAW_COMMAND_TYPE_READ_PROFILE_ACK = 0x05,
@@ -70,6 +75,7 @@ enum claw_profile_ack_pending {
CLAW_NO_PENDING,
CLAW_M1_PENDING,
CLAW_M2_PENDING,
+ CLAW_RGB_PENDING,
};
enum claw_key_index {
@@ -227,6 +233,22 @@ static const struct {
{ 0xce, "REL_WHEEL_DOWN" },
};
+enum claw_rgb_effect_index {
+ CLAW_RGB_EFFECT_MONOCOLOR,
+ CLAW_RGB_EFFECT_BREATHE,
+ CLAW_RGB_EFFECT_CHROMA,
+ CLAW_RGB_EFFECT_RAINBOW,
+ CLAW_RGB_EFFECT_FROSTFIRE,
+};
+
+static const char * const claw_rgb_effect_text[] = {
+ [CLAW_RGB_EFFECT_MONOCOLOR] = "monocolor",
+ [CLAW_RGB_EFFECT_BREATHE] = "breathe",
+ [CLAW_RGB_EFFECT_CHROMA] = "chroma",
+ [CLAW_RGB_EFFECT_RAINBOW] = "rainbow",
+ [CLAW_RGB_EFFECT_FROSTFIRE] = "frostfire",
+};
+
static const u16 button_mapping_addr_old[] = {
0x007a, /* M1 */
0x011f, /* M2 */
@@ -237,6 +259,9 @@ static const u16 button_mapping_addr_new[] = {
0x0164, /* M2 */
};
+static const u16 rgb_addr_old = 0x01fa;
+static const u16 rgb_addr_new = 0x024a;
+
struct claw_command_report {
u8 report_id;
u8 padding[2];
@@ -245,6 +270,28 @@ struct claw_command_report {
u8 data[59];
} __packed;
+struct rgb_zone {
+ u8 red;
+ u8 green;
+ u8 blue;
+};
+
+struct rgb_frame {
+ struct rgb_zone zone[CLAW_RGB_ZONES];
+};
+
+struct rgb_report {
+ u8 profile;
+ __be16 read_addr;
+ u8 frame_bytes;
+ u8 padding;
+ u8 frame_count;
+ u8 state; /* Always 0x09 */
+ u8 speed;
+ u8 brightness;
+ struct rgb_frame zone_data;
+} __packed;
+
struct claw_drvdata {
/* MCU General Variables */
enum claw_profile_ack_pending profile_pending;
@@ -265,6 +312,16 @@ struct claw_drvdata {
u8 m2_codes[CLAW_KEYS_MAX];
const u16 *bmap_addr;
bool bmap_support;
+
+ /* RGB Variables */
+ struct rgb_frame rgb_frames[CLAW_RGB_MAX_FRAMES];
+ enum claw_rgb_effect_index rgb_effect;
+ struct led_classdev_mc led_mc;
+ struct delayed_work rgb_queue;
+ u8 rgb_frame_count;
+ bool rgb_enabled;
+ u8 rgb_speed;
+ u16 rgb_addr;
};
static int get_endpoint_address(struct hid_device *hdev)
@@ -296,8 +353,11 @@ static int claw_gamepad_mode_event(struct claw_drvdata *drvdata,
static int claw_profile_event(struct claw_drvdata *drvdata, struct claw_command_report *cmd_rep)
{
- u8 *codes;
- int i;
+ struct rgb_report *frame;
+ u16 rgb_addr, read_addr;
+ u8 *codes, f_idx;
+ u16 frame_calc;
+ int i, ret = 0;
switch (drvdata->profile_pending) {
case CLAW_M1_PENDING:
@@ -308,15 +368,52 @@ static int claw_profile_event(struct claw_drvdata *drvdata, struct claw_command_
for (i = 0; i < CLAW_KEYS_MAX; i++)
codes[i] = (cmd_rep->data[6 + i] != 0xff) ? cmd_rep->data[6 + i] : 0x00;
break;
+ case CLAW_RGB_PENDING:
+ frame = (struct rgb_report *)cmd_rep->data;
+ rgb_addr = drvdata->rgb_addr;
+ read_addr = be16_to_cpu(frame->read_addr);
+ frame_calc = (read_addr - rgb_addr) / CLAW_RGB_FRAME_OFFSET;
+ if (frame_calc > U8_MAX) {
+ dev_err(drvdata->led_mc.led_cdev.dev, "Got unsupported frame index: %x\n",
+ frame_calc);
+ ret = -EINVAL;
+ goto err_pending;
+ }
+ f_idx = frame_calc;
+
+ if (f_idx >= CLAW_RGB_MAX_FRAMES) {
+ dev_err(drvdata->led_mc.led_cdev.dev, "Got illegal frame index: %x\n",
+ f_idx);
+ ret = -EINVAL;
+ goto err_pending;
+ }
+
+ /* Always treat the first frame as the truth for these constants */
+ if (f_idx == 0) {
+ drvdata->rgb_frame_count = frame->frame_count;
+ /* Invert device speed (20-0) to sysfs speed (0-20) */
+ drvdata->rgb_speed = frame->speed;
+ drvdata->led_mc.led_cdev.brightness = frame->brightness;
+ drvdata->led_mc.subled_info[0].intensity = frame->zone_data.zone[0].red;
+ drvdata->led_mc.subled_info[1].intensity = frame->zone_data.zone[0].green;
+ drvdata->led_mc.subled_info[2].intensity = frame->zone_data.zone[0].blue;
+ }
+
+ memcpy(&drvdata->rgb_frames[f_idx], &frame->zone_data,
+ sizeof(struct rgb_frame));
+
+ break;
default:
dev_warn(&drvdata->hdev->dev,
"Got profile event without changes pending from command: %x\n",
cmd_rep->cmd);
- return -EINVAL;
+ ret = -EINVAL;
}
+
+err_pending:
drvdata->profile_pending = CLAW_NO_PENDING;
- return 0;
+ return ret;
}
static int claw_raw_event(struct claw_drvdata *drvdata, struct hid_report *report,
@@ -759,6 +856,397 @@ static const struct attribute_group claw_gamepad_attr_group = {
.is_visible = claw_gamepad_attr_is_visible,
};
+/* Read RGB config from device */
+static int claw_read_rgb_config(struct hid_device *hdev)
+{
+ u8 data[4] = { 0x01, 0x00, 0x00, CLAW_RGB_FRAME_OFFSET };
+ struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
+ u16 read_addr = drvdata->rgb_addr;
+ size_t len = ARRAY_SIZE(data);
+ int ret, i;
+
+ if (!drvdata->rgb_addr)
+ return -ENODEV;
+
+ /* Loop through all 8 pages of RGB data */
+ guard(mutex)(&drvdata->profile_mutex);
+ for (i = 0; i < 8; i++) {
+ drvdata->profile_pending = CLAW_RGB_PENDING;
+ data[1] = (read_addr >> 8) & 0xff;
+ data[2] = read_addr & 0x00ff;
+ ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_READ_PROFILE, data, len, 8);
+ if (ret) {
+ drvdata->profile_pending = CLAW_NO_PENDING;
+ return ret;
+ }
+ read_addr += CLAW_RGB_FRAME_OFFSET;
+ }
+
+ return 0;
+}
+
+/* Send RGB configuration to device */
+static int claw_write_rgb_state(struct claw_drvdata *drvdata)
+{
+ struct rgb_report report = { 0x01, 0x0000, CLAW_RGB_FRAME_OFFSET, 0x00,
+ drvdata->rgb_frame_count, 0x09, drvdata->rgb_speed,
+ drvdata->led_mc.led_cdev.brightness };
+ u16 write_addr = drvdata->rgb_addr;
+ size_t len = sizeof(report);
+ int f, ret;
+
+ if (!drvdata->rgb_addr)
+ return -ENODEV;
+
+ if (!drvdata->rgb_frame_count)
+ return -EINVAL;
+
+ guard(mutex)(&drvdata->rom_mutex);
+ /* Loop through (up to) 8 pages of RGB data */
+ for (f = 0; f < drvdata->rgb_frame_count; f++) {
+ report.zone_data = drvdata->rgb_frames[f];
+
+ /* Set the MCU address to write the frame data to */
+ report.read_addr = cpu_to_be16(write_addr);
+
+ /* Serialize the rgb_report and write it to MCU */
+ ret = claw_hw_output_report(drvdata->hdev, CLAW_COMMAND_TYPE_WRITE_PROFILE_DATA,
+ (u8 *)&report, len, 8);
+ if (ret)
+ return ret;
+
+ /* Increment the write addr by the offset for the next frame */
+ write_addr += CLAW_RGB_FRAME_OFFSET;
+ }
+
+ ret = claw_hw_output_report(drvdata->hdev, CLAW_COMMAND_TYPE_SYNC_TO_ROM, NULL, 0, 8);
+
+ return ret;
+}
+
+/* Fill all zones with the same color */
+static void claw_frame_fill_solid(struct rgb_frame *frame, struct rgb_zone zone)
+{
+ int z;
+
+ for (z = 0; z < CLAW_RGB_ZONES; z++)
+ frame->zone[z] = zone;
+}
+
+/* Apply solid effect (1 frame, all zones same color) */
+static int claw_apply_monocolor(struct claw_drvdata *drvdata)
+{
+ struct mc_subled *subleds = drvdata->led_mc.subled_info;
+ struct rgb_zone zone = { subleds[0].intensity, subleds[1].intensity,
+ subleds[2].intensity };
+
+ drvdata->rgb_frame_count = 1;
+ claw_frame_fill_solid(&drvdata->rgb_frames[0], zone);
+
+ return claw_write_rgb_state(drvdata);
+}
+
+/* Apply breathe effect (2 frames: color -> off) */
+static int claw_apply_breathe(struct claw_drvdata *drvdata)
+{
+ struct mc_subled *subleds = drvdata->led_mc.subled_info;
+ struct rgb_zone zone = { subleds[0].intensity, subleds[1].intensity,
+ subleds[2].intensity };
+ static const struct rgb_zone off = { 0, 0, 0 };
+
+ drvdata->rgb_frame_count = 2;
+ claw_frame_fill_solid(&drvdata->rgb_frames[0], zone);
+ claw_frame_fill_solid(&drvdata->rgb_frames[1], off);
+
+ return claw_write_rgb_state(drvdata);
+}
+
+/* Apply chroma effect (6 frames: rainbow cycle, all zones sync) */
+static int claw_apply_chroma(struct claw_drvdata *drvdata)
+{
+ static const struct rgb_zone colors[] = {
+ {255, 0, 0}, /* red */
+ {255, 255, 0}, /* yellow */
+ { 0, 255, 0}, /* green */
+ { 0, 255, 255}, /* cyan */
+ { 0, 0, 255}, /* blue */
+ {255, 0, 255}, /* magenta */
+ };
+ u8 frame_count = ARRAY_SIZE(colors);
+ int frame;
+
+ drvdata->rgb_frame_count = frame_count;
+
+ for (frame = 0; frame < frame_count; frame++)
+ claw_frame_fill_solid(&drvdata->rgb_frames[frame], colors[frame]);
+
+ return claw_write_rgb_state(drvdata);
+}
+
+/* Apply rainbow effect (4 frames: rotating colors around joysticks) */
+static int claw_apply_rainbow(struct claw_drvdata *drvdata)
+{
+ static const struct rgb_zone colors[] = {
+ {255, 0, 0}, /* red */
+ { 0, 255, 0}, /* green */
+ { 0, 255, 255}, /* cyan */
+ { 0, 0, 255}, /* blue */
+ };
+ u8 frame_count = ARRAY_SIZE(colors);
+ int frame, zone;
+
+ drvdata->rgb_frame_count = frame_count;
+
+ for (frame = 0; frame < frame_count; frame++) {
+ for (zone = 0; zone < 4; zone++) {
+ drvdata->rgb_frames[frame].zone[zone] = colors[(zone + frame) % 4];
+ drvdata->rgb_frames[frame].zone[zone + 4] = colors[(zone + frame) % 4];
+ }
+ drvdata->rgb_frames[frame].zone[8] = colors[frame];
+ }
+
+ return claw_write_rgb_state(drvdata);
+}
+
+/*
+ * Apply frostfire effect (4 frames: fire vs ice rotating)
+ * Right joystick: fire red -> dark -> ice blue -> dark (clockwise)
+ * Left joystick: ice blue -> dark -> fire red -> dark (counter-clockwise)
+ * ABXY: fire red -> dark -> ice blue -> dark
+ */
+static int claw_apply_frostfire(struct claw_drvdata *drvdata)
+{
+ static const struct rgb_zone colors[] = {
+ {255, 0, 0}, /* fire red */
+ { 0, 0, 0}, /* dark */
+ { 0, 0, 255}, /* ice blue */
+ { 0, 0, 0}, /* dark */
+ };
+ u8 frame_count = ARRAY_SIZE(colors);
+ int frame, zone;
+
+ drvdata->rgb_frame_count = frame_count;
+
+ for (frame = 0; frame < frame_count; frame++) {
+ for (zone = 0; zone < 4; zone++) {
+ drvdata->rgb_frames[frame].zone[zone] = colors[(zone + frame) % 4];
+ drvdata->rgb_frames[frame].zone[zone + 4] = colors[(zone - frame + 6) % 4];
+ }
+ drvdata->rgb_frames[frame].zone[8] = colors[frame];
+ }
+
+ return claw_write_rgb_state(drvdata);
+}
+
+/* Apply current state to device */
+static int claw_apply_rgb_state(struct claw_drvdata *drvdata)
+{
+ static const struct rgb_zone off = { 0, 0, 0 };
+
+ if (!drvdata->rgb_enabled) {
+ drvdata->rgb_frame_count = 1;
+ claw_frame_fill_solid(&drvdata->rgb_frames[0], off);
+ return claw_write_rgb_state(drvdata);
+ }
+
+ switch (drvdata->rgb_effect) {
+ case CLAW_RGB_EFFECT_MONOCOLOR:
+ return claw_apply_monocolor(drvdata);
+ case CLAW_RGB_EFFECT_BREATHE:
+ return claw_apply_breathe(drvdata);
+ case CLAW_RGB_EFFECT_CHROMA:
+ return claw_apply_chroma(drvdata);
+ case CLAW_RGB_EFFECT_RAINBOW:
+ return claw_apply_rainbow(drvdata);
+ case CLAW_RGB_EFFECT_FROSTFIRE:
+ return claw_apply_frostfire(drvdata);
+ default:
+ dev_err(drvdata->led_mc.led_cdev.dev,
+ "No supported rgb_effect selected\n");
+ return -EINVAL;
+ }
+}
+
+static void claw_rgb_queue_fn(struct work_struct *work)
+{
+ struct delayed_work *dwork = container_of(work, struct delayed_work, work);
+ struct claw_drvdata *drvdata = container_of(dwork, struct claw_drvdata, rgb_queue);
+ int ret;
+
+ ret = claw_apply_rgb_state(drvdata);
+ if (ret)
+ dev_err(drvdata->led_mc.led_cdev.dev,
+ "Failed to apply RGB state: %d\n", ret);
+}
+
+static ssize_t effect_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct led_classdev *led_cdev = dev_get_drvdata(dev);
+ struct led_classdev_mc *led_mc = container_of(led_cdev, struct led_classdev_mc, led_cdev);
+ struct claw_drvdata *drvdata = container_of(led_mc, struct claw_drvdata, led_mc);
+ int ret;
+
+ ret = sysfs_match_string(claw_rgb_effect_text, buf);
+ if (ret < 0)
+ return ret;
+
+ drvdata->rgb_effect = ret;
+ mod_delayed_work(system_wq, &drvdata->rgb_queue, msecs_to_jiffies(50));
+
+ return count;
+}
+
+static ssize_t effect_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct led_classdev *led_cdev = dev_get_drvdata(dev);
+ struct led_classdev_mc *led_mc = container_of(led_cdev, struct led_classdev_mc, led_cdev);
+ struct claw_drvdata *drvdata = container_of(led_mc, struct claw_drvdata, led_mc);
+
+ if (drvdata->rgb_effect >= ARRAY_SIZE(claw_rgb_effect_text))
+ return -EINVAL;
+
+ return sysfs_emit(buf, "%s\n", claw_rgb_effect_text[drvdata->rgb_effect]);
+}
+
+static DEVICE_ATTR_RW(effect);
+
+static ssize_t effect_index_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ int i, count = 0;
+
+ for (i = 0; i < ARRAY_SIZE(claw_rgb_effect_text); i++)
+ count += sysfs_emit_at(buf, count, "%s ", claw_rgb_effect_text[i]);
+
+ if (count)
+ buf[count - 1] = '\n';
+
+ return count;
+}
+static DEVICE_ATTR_RO(effect_index);
+
+static ssize_t enabled_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct led_classdev *led_cdev = dev_get_drvdata(dev);
+ struct led_classdev_mc *led_mc = container_of(led_cdev, struct led_classdev_mc, led_cdev);
+ struct claw_drvdata *drvdata = container_of(led_mc, struct claw_drvdata, led_mc);
+ bool val;
+ int ret;
+
+ ret = kstrtobool(buf, &val);
+ if (ret)
+ return ret;
+
+ drvdata->rgb_enabled = val;
+ mod_delayed_work(system_wq, &drvdata->rgb_queue, msecs_to_jiffies(50));
+
+ return count;
+}
+
+static ssize_t enabled_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct led_classdev *led_cdev = dev_get_drvdata(dev);
+ struct led_classdev_mc *led_mc = container_of(led_cdev, struct led_classdev_mc, led_cdev);
+ struct claw_drvdata *drvdata = container_of(led_mc, struct claw_drvdata, led_mc);
+
+ return sysfs_emit(buf, "%s\n", drvdata->rgb_enabled ? "true" : "false");
+}
+static DEVICE_ATTR_RW(enabled);
+
+static ssize_t enabled_index_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ return sysfs_emit(buf, "true false\n");
+}
+static DEVICE_ATTR_RO(enabled_index);
+
+static ssize_t speed_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct led_classdev *led_cdev = dev_get_drvdata(dev);
+ struct led_classdev_mc *led_mc = container_of(led_cdev, struct led_classdev_mc, led_cdev);
+ struct claw_drvdata *drvdata = container_of(led_mc, struct claw_drvdata, led_mc);
+ unsigned int val, speed;
+ int ret;
+
+ ret = kstrtouint(buf, 10, &val);
+ if (ret)
+ return ret;
+
+ if (val > 20)
+ return -EINVAL;
+
+ /* 0 is fastest, invert value for intuitive userspace speed */
+ speed = 20 - val;
+
+ drvdata->rgb_speed = speed;
+ mod_delayed_work(system_wq, &drvdata->rgb_queue, msecs_to_jiffies(50));
+
+ return count;
+}
+
+static ssize_t speed_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ struct hid_device *hdev = to_hid_device(dev);
+ struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
+ u8 speed = 20 - drvdata->rgb_speed;
+
+ return sysfs_emit(buf, "%u\n", speed);
+}
+static DEVICE_ATTR_RW(speed);
+
+static ssize_t speed_range_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ return sysfs_emit(buf, "0-20\n");
+}
+static DEVICE_ATTR_RO(speed_range);
+
+static void claw_led_brightness_set(struct led_classdev *led_cdev,
+ enum led_brightness _brightness)
+{
+ struct led_classdev_mc *led_mc = container_of(led_cdev, struct led_classdev_mc, led_cdev);
+ struct claw_drvdata *drvdata = container_of(led_mc, struct claw_drvdata, led_mc);
+
+ mod_delayed_work(system_wq, &drvdata->rgb_queue, msecs_to_jiffies(50));
+}
+
+static struct attribute *claw_rgb_attrs[] = {
+ &dev_attr_effect.attr,
+ &dev_attr_effect_index.attr,
+ &dev_attr_enabled.attr,
+ &dev_attr_enabled_index.attr,
+ &dev_attr_speed.attr,
+ &dev_attr_speed_range.attr,
+ NULL,
+};
+
+static const struct attribute_group rgb_attr_group = {
+ .attrs = claw_rgb_attrs,
+};
+
+static struct mc_subled claw_rgb_subled_info[] = {
+ {
+ .color_index = LED_COLOR_ID_RED,
+ .channel = 0x1,
+ },
+ {
+ .color_index = LED_COLOR_ID_GREEN,
+ .channel = 0x2,
+ },
+ {
+ .color_index = LED_COLOR_ID_BLUE,
+ .channel = 0x3,
+ },
+};
+
static void cfg_setup_fn(struct work_struct *work)
{
struct delayed_work *dwork = container_of(work, struct delayed_work, work);
@@ -772,6 +1260,13 @@ static void cfg_setup_fn(struct work_struct *work)
return;
}
+ ret = claw_read_rgb_config(drvdata->hdev);
+ if (ret) {
+ dev_err(drvdata->led_mc.led_cdev.dev,
+ "Failed to setup device, can't read RGB config: %d\n", ret);
+ return;
+ }
+
/* Add sysfs attributes after we get the device state */
ret = devm_device_add_group(&drvdata->hdev->dev, &claw_gamepad_attr_group);
if (ret) {
@@ -780,7 +1275,15 @@ static void cfg_setup_fn(struct work_struct *work)
return;
}
+ ret = devm_device_add_group(drvdata->led_mc.led_cdev.dev, &rgb_attr_group);
+ if (ret) {
+ dev_err(&drvdata->hdev->dev,
+ "Failed to setup device, can't create led attributes: %d\n", ret);
+ return;
+ }
+
kobject_uevent(&drvdata->hdev->dev.kobj, KOBJ_CHANGE);
+ kobject_uevent(&drvdata->led_mc.led_cdev.dev->kobj, KOBJ_CHANGE);
}
static void cfg_resume_fn(struct work_struct *work)
@@ -790,6 +1293,10 @@ static void cfg_resume_fn(struct work_struct *work)
u8 data[2] = { drvdata->gamepad_mode, drvdata->mkeys_function };
int ret;
+ ret = claw_read_rgb_config(drvdata->hdev);
+ if (ret)
+ dev_err(drvdata->led_mc.led_cdev.dev, "Failed to read RGB config: %d\n", ret);
+
ret = claw_hw_output_report(drvdata->hdev, CLAW_COMMAND_TYPE_SWITCH_MODE, data,
ARRAY_SIZE(data), 0);
if (ret)
@@ -803,18 +1310,24 @@ static void claw_features_supported(struct claw_drvdata *drvdata)
if (major == 0x01) {
drvdata->bmap_support = true;
- if (minor >= 0x66)
+ if (minor >= 0x66) {
drvdata->bmap_addr = button_mapping_addr_new;
- else
+ drvdata->rgb_addr = rgb_addr_new;
+ } else {
drvdata->bmap_addr = button_mapping_addr_old;
+ drvdata->rgb_addr = rgb_addr_old;
+ }
return;
}
if ((major == 0x02 && minor >= 0x17) || major >= 0x03) {
drvdata->bmap_support = true;
drvdata->bmap_addr = button_mapping_addr_new;
+ drvdata->rgb_addr = rgb_addr_new;
return;
}
+
+ drvdata->rgb_addr = rgb_addr_old;
}
static int claw_probe(struct hid_device *hdev, u8 ep)
@@ -844,6 +1357,26 @@ static int claw_probe(struct hid_device *hdev, u8 ep)
if (!drvdata->bmap_support)
dev_dbg(&hdev->dev, "M-Key mapping is not supported. Update firmware to enable.\n");
+ /* Initialize RGB LED */
+ INIT_DELAYED_WORK(&drvdata->rgb_queue, &claw_rgb_queue_fn);
+
+ drvdata->led_mc.led_cdev.name = "msi_claw:rgb:joystick_rings";
+ drvdata->led_mc.led_cdev.brightness = 0x50;
+ drvdata->led_mc.led_cdev.max_brightness = 0x64;
+ drvdata->led_mc.led_cdev.color = LED_COLOR_ID_RGB;
+ drvdata->led_mc.led_cdev.brightness_set = claw_led_brightness_set;
+ drvdata->led_mc.num_colors = 3;
+ drvdata->led_mc.subled_info = devm_kmemdup(&hdev->dev, claw_rgb_subled_info,
+ sizeof(claw_rgb_subled_info), GFP_KERNEL);
+ if (!drvdata->led_mc.subled_info)
+ return -ENOMEM;
+
+ drvdata->rgb_enabled = true;
+
+ ret = devm_led_classdev_multicolor_register(&hdev->dev, &drvdata->led_mc);
+ if (ret)
+ return ret;
+
/* For control interface: open the HID transport for sending commands. */
ret = hid_hw_open(hdev);
if (ret)
@@ -905,6 +1438,9 @@ static void claw_remove(struct hid_device *hdev)
return;
}
+ /* Block writes to brightness/multi_intensity during teardown */
+ drvdata->led_mc.led_cdev.brightness_set = NULL;
+ cancel_delayed_work_sync(&drvdata->rgb_queue);
cancel_delayed_work_sync(&drvdata->cfg_setup);
cancel_delayed_work_sync(&drvdata->cfg_resume);
hid_hw_close(hdev);
--
2.53.0
^ permalink raw reply related
* [PATCH v2 2/4] HID: hid-msi: Add M-key mapping attributes
From: Derek J. Clark @ 2026-05-13 23:14 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: Pierre-Loup A . Griffais, Denis Benato, Zhouwang Huang,
Derek J . Clark, linux-input, linux-doc, linux-kernel
In-Reply-To: <20260513231445.3213501-1-derekjohn.clark@gmail.com>
Adds attributes that allow for remapping the M-keys with up to 5 values
when in macro mode. There are 2 mappable buttons on the rear of the
device, M1 on the right and M2 on the left. When mapped, the events will
fire from one of three event devices: gamepad buttons will fire from the
device handled by xpad, while keyboard and mouse events will fire from
respectively typed evdevs provided by the input core. Names of each
mapping have been kept as close to the event that will fire from the evdev
as possible, with context added to the ABS_ events on the direction of the
movement.
Initial reverse-engineering and implementation of this feature was done
by Zhouwang Huang. I refactored the overall format to conform to kernel
driver best practices and style guides. Claude was used as an initial
reviewer of this patch.
Assisted-by: Claude:claude-sonnet-4-6
Co-developed-by: Zhouwang Huang <honjow311@gmail.com>
Signed-off-by: Zhouwang Huang <honjow311@gmail.com>
Signed-off-by: Derek J. Clark <derekjohn.clark@gmail.com>
---
v2:
- Add mutex for SYNC_TO_ROM commands to ensure every SYNC is completed
before more data is written to the MCU volatile memory.
- Add mutex for profile_pending to ensure every profile action
response is serialized to the generating command.
---
drivers/hid/hid-msi.c | 398 +++++++++++++++++++++++++++++++++++++++++-
1 file changed, 397 insertions(+), 1 deletion(-)
diff --git a/drivers/hid/hid-msi.c b/drivers/hid/hid-msi.c
index 8915942af15e6..13ba2747fdb66 100644
--- a/drivers/hid/hid-msi.c
+++ b/drivers/hid/hid-msi.c
@@ -40,6 +40,8 @@
#define CLAW_DINPUT_CFG_INTF_IN 0x82
#define CLAW_XINPUT_CFG_INTF_IN 0x83
+#define CLAW_KEYS_MAX 5
+
enum claw_command_index {
CLAW_COMMAND_TYPE_READ_PROFILE = 0x04,
CLAW_COMMAND_TYPE_READ_PROFILE_ACK = 0x05,
@@ -64,6 +66,17 @@ static const char * const claw_gamepad_mode_text[] = {
[CLAW_GAMEPAD_MODE_DESKTOP] = "desktop",
};
+enum claw_profile_ack_pending {
+ CLAW_NO_PENDING,
+ CLAW_M1_PENDING,
+ CLAW_M2_PENDING,
+};
+
+enum claw_key_index {
+ CLAW_KEY_M1,
+ CLAW_KEY_M2,
+};
+
enum claw_mkeys_function_index {
CLAW_MKEY_FUNCTION_MACRO,
CLAW_MKEY_FUNCTION_COMBO,
@@ -76,6 +89,154 @@ static const char * const claw_mkeys_function_text[] = {
[CLAW_MKEY_FUNCTION_DISABLED] = "disabled",
};
+static const struct {
+ u8 code;
+ const char *name;
+} claw_button_mapping_key_map[] = {
+ /* Gamepad buttons */
+ { 0x01, "ABS_HAT0Y_UP" },
+ { 0x02, "ABS_HAT0Y_DOWN" },
+ { 0x03, "ABS_HAT0X_LEFT" },
+ { 0x04, "ABS_HAT0X_RIGHT" },
+ { 0x05, "BTN_TL" },
+ { 0x06, "BTN_TR" },
+ { 0x07, "BTN_THUMBL" },
+ { 0x08, "BTN_THUMBR" },
+ { 0x09, "BTN_SOUTH" },
+ { 0x0a, "BTN_EAST" },
+ { 0x0b, "BTN_NORTH" },
+ { 0x0c, "BTN_WEST" },
+ { 0x0d, "BTN_MODE" },
+ { 0x0e, "BTN_SELECT" },
+ { 0x0f, "BTN_START" },
+ { 0x13, "BTN_TL2"},
+ { 0x14, "BTN_TR2"},
+ { 0x15, "ABS_Y_UP"},
+ { 0x16, "ABS_Y_DOWN"},
+ { 0x17, "ABS_X_LEFT"},
+ { 0x18, "ABS_X_LEFT_RIGHT"},
+ { 0x19, "ABS_RY_UP"},
+ { 0x1a, "ABS_RY_DOWN"},
+ { 0x1b, "ABS_RX_LEFT"},
+ { 0x1c, "ABS_RX_RIGHT"},
+ /* Keyboard keys */
+ { 0x32, "KEY_ESC" },
+ { 0x33, "KEY_F1" },
+ { 0x34, "KEY_F2" },
+ { 0x35, "KEY_F3" },
+ { 0x36, "KEY_F4" },
+ { 0x37, "KEY_F5" },
+ { 0x38, "KEY_F6" },
+ { 0x39, "KEY_F7" },
+ { 0x3a, "KEY_F8" },
+ { 0x3b, "KEY_F9" },
+ { 0x3c, "KEY_F10" },
+ { 0x3d, "KEY_F11" },
+ { 0x3e, "KEY_F12" },
+ { 0x3f, "KEY_GRAVE" },
+ { 0x40, "KEY_1" },
+ { 0x41, "KEY_2" },
+ { 0x42, "KEY_3" },
+ { 0x43, "KEY_4" },
+ { 0x44, "KEY_5" },
+ { 0x45, "KEY_6" },
+ { 0x46, "KEY_7" },
+ { 0x47, "KEY_8" },
+ { 0x48, "KEY_9" },
+ { 0x49, "KEY_0" },
+ { 0x4a, "KEY_MINUS" },
+ { 0x4b, "KEY_EQUAL" },
+ { 0x4c, "KEY_BACKSPACE" },
+ { 0x4d, "KEY_TAB" },
+ { 0x4e, "KEY_Q" },
+ { 0x4f, "KEY_W" },
+ { 0x50, "KEY_E" },
+ { 0x51, "KEY_R" },
+ { 0x52, "KEY_T" },
+ { 0x53, "KEY_Y" },
+ { 0x54, "KEY_U" },
+ { 0x55, "KEY_I" },
+ { 0x56, "KEY_O" },
+ { 0x57, "KEY_P" },
+ { 0x58, "KEY_LEFTBRACE" },
+ { 0x59, "KEY_RIGHTBRACE" },
+ { 0x5a, "KEY_BACKSLASH" },
+ { 0x5b, "KEY_CAPSLOCK" },
+ { 0x5c, "KEY_A" },
+ { 0x5d, "KEY_S" },
+ { 0x5e, "KEY_D" },
+ { 0x5f, "KEY_F" },
+ { 0x60, "KEY_G" },
+ { 0x61, "KEY_H" },
+ { 0x62, "KEY_J" },
+ { 0x63, "KEY_K" },
+ { 0x64, "KEY_L" },
+ { 0x65, "KEY_SEMICOLON" },
+ { 0x66, "KEY_APOSTROPHE" },
+ { 0x67, "KEY_ENTER" },
+ { 0x68, "KEY_LEFTSHIFT" },
+ { 0x69, "KEY_Z" },
+ { 0x6a, "KEY_X" },
+ { 0x6b, "KEY_C" },
+ { 0x6c, "KEY_V" },
+ { 0x6d, "KEY_B" },
+ { 0x6e, "KEY_N" },
+ { 0x6f, "KEY_M" },
+ { 0x70, "KEY_COMMA" },
+ { 0x71, "KEY_DOT" },
+ { 0x72, "KEY_SLASH" },
+ { 0x73, "KEY_RIGHTSHIFT" },
+ { 0x74, "KEY_LEFTCTRL" },
+ { 0x75, "KEY_LEFTMETA" },
+ { 0x76, "KEY_LEFTALT" },
+ { 0x77, "KEY_SPACE" },
+ { 0x78, "KEY_RIGHTALT" },
+ { 0x79, "KEY_RIGHTCTRL" },
+ { 0x7a, "KEY_INSERT" },
+ { 0x7b, "KEY_HOME" },
+ { 0x7c, "KEY_PAGEUP" },
+ { 0x7d, "KEY_DELETE" },
+ { 0x7e, "KEY_END" },
+ { 0x7f, "KEY_PAGEDOWN" },
+ { 0x8a, "KEY_KPENTER" },
+ { 0x8b, "KEY_KP0" },
+ { 0x8c, "KEY_KP1" },
+ { 0x8d, "KEY_KP2" },
+ { 0x8e, "KEY_KP3" },
+ { 0x8f, "KEY_KP4" },
+ { 0x90, "KEY_KP5" },
+ { 0x91, "KEY_KP6" },
+ { 0x92, "KEY_KP7" },
+ { 0x93, "KEY_KP8" },
+ { 0x94, "KEY_KP9" },
+ { 0x95, "MD_PLAY" },
+ { 0x96, "MD_STOP" },
+ { 0x97, "MD_NEXT" },
+ { 0x98, "MD_PREV" },
+ { 0x99, "MD_VOL_UP" },
+ { 0x9a, "MD_VOL_DOWN" },
+ { 0x9b, "MD_VOL_MUTE" },
+ { 0x9c, "KEY_F23" },
+ /* Mouse events */
+ { 0xc8, "BTN_LEFT" },
+ { 0xc9, "BTN_MIDDLE" },
+ { 0xca, "BTN_RIGHT" },
+ { 0xcb, "BTN_SIDE" },
+ { 0xcc, "BTN_EXTRA" },
+ { 0xcd, "REL_WHEEL_UP" },
+ { 0xce, "REL_WHEEL_DOWN" },
+};
+
+static const u16 button_mapping_addr_old[] = {
+ 0x007a, /* M1 */
+ 0x011f, /* M2 */
+};
+
+static const u16 button_mapping_addr_new[] = {
+ 0x00bb, /* M1 */
+ 0x0164, /* M2 */
+};
+
struct claw_command_report {
u8 report_id;
u8 padding[2];
@@ -86,16 +247,24 @@ struct claw_command_report {
struct claw_drvdata {
/* MCU General Variables */
+ enum claw_profile_ack_pending profile_pending;
struct completion send_cmd_complete;
struct delayed_work cfg_resume;
struct delayed_work cfg_setup;
+ struct mutex profile_mutex; /* mutex for profile_pending calls */
struct hid_device *hdev;
struct mutex cfg_mutex; /* mutex for synchronous data */
+ struct mutex rom_mutex; /* mutex for SYNC_TO_ROM calls */
+ u16 bcd_device;
u8 ep;
/* Gamepad Variables */
enum claw_mkeys_function_index mkeys_function;
enum claw_gamepad_mode_index gamepad_mode;
+ u8 m1_codes[CLAW_KEYS_MAX];
+ u8 m2_codes[CLAW_KEYS_MAX];
+ const u16 *bmap_addr;
+ bool bmap_support;
};
static int get_endpoint_address(struct hid_device *hdev)
@@ -125,6 +294,31 @@ static int claw_gamepad_mode_event(struct claw_drvdata *drvdata,
return 0;
}
+static int claw_profile_event(struct claw_drvdata *drvdata, struct claw_command_report *cmd_rep)
+{
+ u8 *codes;
+ int i;
+
+ switch (drvdata->profile_pending) {
+ case CLAW_M1_PENDING:
+ case CLAW_M2_PENDING:
+ codes = (drvdata->profile_pending == CLAW_M1_PENDING) ?
+ drvdata->m1_codes : drvdata->m2_codes;
+ /* Extract key codes; replace disabled (0xff) with 0x00, which is (null) in _show */
+ for (i = 0; i < CLAW_KEYS_MAX; i++)
+ codes[i] = (cmd_rep->data[6 + i] != 0xff) ? cmd_rep->data[6 + i] : 0x00;
+ break;
+ default:
+ dev_warn(&drvdata->hdev->dev,
+ "Got profile event without changes pending from command: %x\n",
+ cmd_rep->cmd);
+ return -EINVAL;
+ }
+ drvdata->profile_pending = CLAW_NO_PENDING;
+
+ return 0;
+}
+
static int claw_raw_event(struct claw_drvdata *drvdata, struct hid_report *report,
u8 *data, int size)
{
@@ -146,6 +340,9 @@ static int claw_raw_event(struct claw_drvdata *drvdata, struct hid_report *repor
case CLAW_COMMAND_TYPE_GAMEPAD_MODE_ACK:
ret = claw_gamepad_mode_event(drvdata, cmd_rep);
break;
+ case CLAW_COMMAND_TYPE_READ_PROFILE_ACK:
+ ret = claw_profile_event(drvdata, cmd_rep);
+ break;
case CLAW_COMMAND_TYPE_ACK:
break;
default:
@@ -366,6 +563,161 @@ static ssize_t reset_store(struct device *dev, struct device_attribute *attr,
}
static DEVICE_ATTR_WO(reset);
+static int button_mapping_name_to_code(const char *name)
+{
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(claw_button_mapping_key_map); i++) {
+ if (!strcmp(name, claw_button_mapping_key_map[i].name))
+ return claw_button_mapping_key_map[i].code;
+ }
+
+ return -EINVAL;
+}
+
+static const char *button_mapping_code_to_name(u8 code)
+{
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(claw_button_mapping_key_map); i++) {
+ if (claw_button_mapping_key_map[i].code == code)
+ return claw_button_mapping_key_map[i].name;
+ }
+
+ return NULL;
+}
+
+static int claw_buttons_store(struct device *dev, const char *buf, u8 mkey_idx)
+{
+ struct hid_device *hdev = to_hid_device(dev);
+ struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
+ u8 data[] = { 0x01, (drvdata->bmap_addr[mkey_idx] >> 8) & 0xff,
+ drvdata->bmap_addr[mkey_idx] & 0xff, 0x07,
+ 0x04, 0x00, 0xff, 0xff, 0xff, 0xff, 0xff };
+ size_t len = ARRAY_SIZE(data);
+ int ret, key_count, i;
+ char **raw_keys;
+
+ raw_keys = argv_split(GFP_KERNEL, buf, &key_count);
+ if (!raw_keys)
+ return -ENOMEM;
+
+ guard(mutex)(&drvdata->rom_mutex); /* all err_free paths must be in scope */
+ if (key_count > CLAW_KEYS_MAX) {
+ ret = -EINVAL;
+ goto err_free;
+ }
+
+ if (key_count == 0)
+ goto set_buttons;
+
+ for (i = 0; i < key_count; i++) {
+ ret = button_mapping_name_to_code(raw_keys[i]);
+ if (ret < 0)
+ goto err_free;
+
+ data[6 + i] = ret;
+ }
+
+set_buttons:
+ ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_WRITE_PROFILE_DATA, data, len, 8);
+ if (ret < 0)
+ goto err_free;
+
+ ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_SYNC_TO_ROM, NULL, 0, 8);
+
+err_free:
+ argv_free(raw_keys);
+ return ret;
+}
+
+static int claw_buttons_show(struct device *dev, char *buf, enum claw_key_index m_key)
+{
+ struct hid_device *hdev = to_hid_device(dev);
+ struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
+ u8 data[] = { 0x01, (drvdata->bmap_addr[m_key] >> 8) & 0xff,
+ drvdata->bmap_addr[m_key] & 0xff, 0x07 };
+ size_t len = ARRAY_SIZE(data);
+ int i, ret, count = 0;
+ const char *name;
+ u8 *codes;
+
+ codes = (m_key == CLAW_KEY_M1) ? drvdata->m1_codes : drvdata->m2_codes;
+
+ guard(mutex)(&drvdata->profile_mutex);
+ drvdata->profile_pending = (m_key == CLAW_KEY_M1) ? CLAW_M1_PENDING : CLAW_M2_PENDING;
+
+ ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_READ_PROFILE, data, len, 8);
+ if (ret) {
+ drvdata->profile_pending = CLAW_NO_PENDING;
+ return ret;
+ }
+ for (i = 0; i < CLAW_KEYS_MAX; i++) {
+ name = button_mapping_code_to_name(codes[i]);
+ if (name)
+ count += sysfs_emit_at(buf, count, "%s ", name);
+ }
+
+ if (!count)
+ return sysfs_emit(buf, "(not set)\n");
+
+ buf[count - 1] = '\n';
+
+ return count;
+}
+
+static ssize_t button_m1_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ int ret;
+
+ ret = claw_buttons_store(dev, buf, CLAW_KEY_M1);
+ if (ret)
+ return ret;
+
+ return count;
+}
+
+static ssize_t button_m1_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ return claw_buttons_show(dev, buf, CLAW_KEY_M1);
+}
+static DEVICE_ATTR_RW(button_m1);
+
+static ssize_t button_m2_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ int ret;
+
+ ret = claw_buttons_store(dev, buf, CLAW_KEY_M2);
+ if (ret)
+ return ret;
+
+ return count;
+}
+
+static ssize_t button_m2_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ return claw_buttons_show(dev, buf, CLAW_KEY_M2);
+}
+static DEVICE_ATTR_RW(button_m2);
+
+static ssize_t button_mapping_options_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ int i, count = 0;
+
+ for (i = 0; i < ARRAY_SIZE(claw_button_mapping_key_map); i++)
+ count += sysfs_emit_at(buf, count, "%s ", claw_button_mapping_key_map[i].name);
+
+ buf[count - 1] = '\n';
+
+ return count;
+}
+static DEVICE_ATTR_RO(button_mapping_options);
+
static umode_t claw_gamepad_attr_is_visible(struct kobject *kobj, struct attribute *attr,
int n)
{
@@ -378,10 +730,22 @@ static umode_t claw_gamepad_attr_is_visible(struct kobject *kobj, struct attribu
return 0;
}
- return attr->mode;
+ /* Always show attrs available on all firmware */
+ if (attr == &dev_attr_gamepad_mode.attr ||
+ attr == &dev_attr_gamepad_mode_index.attr ||
+ attr == &dev_attr_mkeys_function.attr ||
+ attr == &dev_attr_mkeys_function_index.attr ||
+ attr == &dev_attr_reset.attr)
+ return attr->mode;
+
+ /* Hide button mapping attrs if it isn't supported */
+ return drvdata->bmap_support ? attr->mode : 0;
}
static struct attribute *claw_gamepad_attrs[] = {
+ &dev_attr_button_m1.attr,
+ &dev_attr_button_m2.attr,
+ &dev_attr_button_mapping_options.attr,
&dev_attr_gamepad_mode.attr,
&dev_attr_gamepad_mode_index.attr,
&dev_attr_mkeys_function.attr,
@@ -432,8 +796,31 @@ static void cfg_resume_fn(struct work_struct *work)
dev_err(&drvdata->hdev->dev, "Failed to set gamepad mode settings: %d\n", ret);
}
+static void claw_features_supported(struct claw_drvdata *drvdata)
+{
+ u8 major = (drvdata->bcd_device >> 8) & 0xff;
+ u8 minor = drvdata->bcd_device & 0xff;
+
+ if (major == 0x01) {
+ drvdata->bmap_support = true;
+ if (minor >= 0x66)
+ drvdata->bmap_addr = button_mapping_addr_new;
+ else
+ drvdata->bmap_addr = button_mapping_addr_old;
+ return;
+ }
+
+ if ((major == 0x02 && minor >= 0x17) || major >= 0x03) {
+ drvdata->bmap_support = true;
+ drvdata->bmap_addr = button_mapping_addr_new;
+ return;
+ }
+}
+
static int claw_probe(struct hid_device *hdev, u8 ep)
{
+ struct usb_interface *intf = to_usb_interface(hdev->dev.parent);
+ struct usb_device *udev = interface_to_usbdev(intf);
struct claw_drvdata *drvdata;
int ret;
@@ -446,8 +833,17 @@ static int claw_probe(struct hid_device *hdev, u8 ep)
drvdata->ep = ep;
mutex_init(&drvdata->cfg_mutex);
+ mutex_init(&drvdata->profile_mutex);
+ mutex_init(&drvdata->rom_mutex);
init_completion(&drvdata->send_cmd_complete);
+ /* Determine feature level from firmware version */
+ drvdata->bcd_device = le16_to_cpu(udev->descriptor.bcdDevice);
+ claw_features_supported(drvdata);
+
+ if (!drvdata->bmap_support)
+ dev_dbg(&hdev->dev, "M-Key mapping is not supported. Update firmware to enable.\n");
+
/* For control interface: open the HID transport for sending commands. */
ret = hid_hw_open(hdev);
if (ret)
--
2.53.0
^ permalink raw reply related
* [PATCH v2 1/4] HID: hid-msi: Add MSI Claw configuration driver
From: Derek J. Clark @ 2026-05-13 23:14 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: Pierre-Loup A . Griffais, Denis Benato, Zhouwang Huang,
Derek J . Clark, linux-input, linux-doc, linux-kernel
In-Reply-To: <20260513231445.3213501-1-derekjohn.clark@gmail.com>
Adds configuration HID driver for the MSI Claw series of handheld PC's.
In this initial patch add the initial driver outline and attributes for
changing the gamepad mode, M-key behavior, and add a WO reset function.
Sending the SWITCH_MODE and RESET commands causes a USB disconnect in
the device. The completion will therefore never get hit and would trigger
an -EIO. To avoid showing the user an error for every write to these
attrs a bypass for the completion handling is introduced when timeout ==
0.
The initial version of this patch was written by Denis Benato, which
contained the initial reverse-engineering and implementation for the
gamepad mode switching. This work was later expanded by Zhouwang Huang
to include more gamepad modes. Finally, I refactored the drivers data
in/out flow and overall format to conform to kernel driver best
practices and style guides. Claude was used as an initial reviewer of
this patch.
Assisted-by: Claude:claude-sonnet-4-6
Co-developed-by: Denis Benato <denis.benato@linux.dev>
Signed-off-by: Denis Benato <denis.benato@linux.dev>
Co-developed-by: Zhouwang Huang <honjow311@gmail.com>
Signed-off-by: Zhouwang Huang <honjow311@gmail.com>
Signed-off-by: Derek J. Clark <derekjohn.clark@gmail.com>
---
v2:
- Rename driver to hid-msi from hid-msi-claw.
- Rename reusable/generic functions to msi_* from claw_*, retaining
claw specific functions.
- Add generic entrypoints for probe, remove, and raw event that route
to claw specific functions.
- Remove deadlock in cfg_setup, return on errors.
---
MAINTAINERS | 6 +
drivers/hid/Kconfig | 12 +
drivers/hid/Makefile | 1 +
drivers/hid/hid-ids.h | 6 +
drivers/hid/hid-msi.c | 582 ++++++++++++++++++++++++++++++++++++++++++
5 files changed, 607 insertions(+)
create mode 100644 drivers/hid/hid-msi.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 6f6517bf4f970..8e2de98b768f7 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -17965,6 +17965,12 @@ S: Odd Fixes
F: Documentation/devicetree/bindings/net/ieee802154/mrf24j40.txt
F: drivers/net/ieee802154/mrf24j40.c
+MSI HID DRIVER
+M: Derek J. Clark <derekjohn.clark@gmail.com>
+L: linux-input@vger.kernel.org
+S: Maintained
+F: drivers/hid/hid-msi.c
+
MSI EC DRIVER
M: Nikita Kravets <teackot@gmail.com>
L: platform-driver-x86@vger.kernel.org
diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index 10c12d8e65579..af146691bd481 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -492,6 +492,18 @@ config HID_GT683R
Currently the following devices are know to be supported:
- MSI GT683R
+config HID_MSI
+ tristate "MSI Claw Gamepad Support"
+ depends on USB_HID
+ select LEDS_CLASS
+ select LEDS_CLASS_MULTICOLOR
+ help
+ Support for the MSI Claw RGB and controller configuration
+
+ Say Y here to include configuration interface support for the MSI Claw Line
+ of Handheld Console Controllers. Say M here to compile this driver as a
+ module. The module will be called hid-msi.
+
config HID_KEYTOUCH
tristate "Keytouch HID devices"
help
diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
index 07dfdb6a49c59..80925a17b059c 100644
--- a/drivers/hid/Makefile
+++ b/drivers/hid/Makefile
@@ -92,6 +92,7 @@ obj-$(CONFIG_HID_MAYFLASH) += hid-mf.o
obj-$(CONFIG_HID_MEGAWORLD_FF) += hid-megaworld.o
obj-$(CONFIG_HID_MICROSOFT) += hid-microsoft.o
obj-$(CONFIG_HID_MONTEREY) += hid-monterey.o
+obj-$(CONFIG_HID_MSI) += hid-msi.o
obj-$(CONFIG_HID_MULTITOUCH) += hid-multitouch.o
obj-$(CONFIG_HID_NINTENDO) += hid-nintendo.o
obj-$(CONFIG_HID_NTI) += hid-nti.o
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 933b7943bdb50..6d0d34806931f 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -1047,7 +1047,13 @@
#define USB_DEVICE_ID_MOZA_R16_R21_2 0x0010
#define USB_VENDOR_ID_MSI 0x1770
+#define USB_VENDOR_ID_MSI_2 0x0db0
#define USB_DEVICE_ID_MSI_GT683R_LED_PANEL 0xff00
+#define USB_DEVICE_ID_MSI_CLAW_XINPUT 0x1901
+#define USB_DEVICE_ID_MSI_CLAW_DINPUT 0x1902
+#define USB_DEVICE_ID_MSI_CLAW_DESKTOP 0x1903
+#define USB_DEVICE_ID_MSI_CLAW_BIOS 0x1904
+
#define USB_VENDOR_ID_NATIONAL_SEMICONDUCTOR 0x0400
#define USB_DEVICE_ID_N_S_HARMONY 0xc359
diff --git a/drivers/hid/hid-msi.c b/drivers/hid/hid-msi.c
new file mode 100644
index 0000000000000..8915942af15e6
--- /dev/null
+++ b/drivers/hid/hid-msi.c
@@ -0,0 +1,582 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * HID driver for MSI Claw Handheld PC gamepads.
+ *
+ * Provides configuration support for the MSI Claw series of handheld PC
+ * gamepads. Multiple iterations of the device firmware has led to some
+ * quirks for how certain attributes are handled. The original firmware
+ * did not support remapping of the M1 (right) and M2 (left) rear paddles.
+ * Additionally, the MCU RAM address for writing configuration data has
+ * changed twice. Checks are done during probe to enumerate these variances.
+ *
+ * Copyright (c) 2026 Zhouwang Huang <honjow311@gmail.com>
+ * Copyright (c) 2026 Denis Benato <denis.benato@linux.dev>
+ * Copyright (c) 2026 Valve Corporation
+ */
+
+#include <linux/array_size.h>
+#include <linux/cleanup.h>
+#include <linux/completion.h>
+#include <linux/container_of.h>
+#include <linux/device.h>
+#include <linux/hid.h>
+#include <linux/kobject.h>
+#include <linux/leds.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/sysfs.h>
+#include <linux/types.h>
+#include <linux/unaligned.h>
+#include <linux/usb.h>
+#include <linux/workqueue.h>
+
+#include "hid-ids.h"
+
+#define CLAW_OUTPUT_REPORT_ID 0x0f
+#define CLAW_INPUT_REPORT_ID 0x10
+
+#define CLAW_PACKET_SIZE 64
+
+#define CLAW_DINPUT_CFG_INTF_IN 0x82
+#define CLAW_XINPUT_CFG_INTF_IN 0x83
+
+enum claw_command_index {
+ CLAW_COMMAND_TYPE_READ_PROFILE = 0x04,
+ CLAW_COMMAND_TYPE_READ_PROFILE_ACK = 0x05,
+ CLAW_COMMAND_TYPE_ACK = 0x06,
+ CLAW_COMMAND_TYPE_WRITE_PROFILE_DATA = 0x21,
+ CLAW_COMMAND_TYPE_SYNC_TO_ROM = 0x22,
+ CLAW_COMMAND_TYPE_SWITCH_MODE = 0x24,
+ CLAW_COMMAND_TYPE_READ_GAMEPAD_MODE = 0x26,
+ CLAW_COMMAND_TYPE_GAMEPAD_MODE_ACK = 0x27,
+ CLAW_COMMAND_TYPE_RESET_DEVICE = 0x28,
+};
+
+enum claw_gamepad_mode_index {
+ CLAW_GAMEPAD_MODE_XINPUT = 0x01,
+ CLAW_GAMEPAD_MODE_DINPUT = 0x02,
+ CLAW_GAMEPAD_MODE_DESKTOP = 0x04,
+};
+
+static const char * const claw_gamepad_mode_text[] = {
+ [CLAW_GAMEPAD_MODE_XINPUT] = "xinput",
+ [CLAW_GAMEPAD_MODE_DINPUT] = "dinput",
+ [CLAW_GAMEPAD_MODE_DESKTOP] = "desktop",
+};
+
+enum claw_mkeys_function_index {
+ CLAW_MKEY_FUNCTION_MACRO,
+ CLAW_MKEY_FUNCTION_COMBO,
+ CLAW_MKEY_FUNCTION_DISABLED,
+};
+
+static const char * const claw_mkeys_function_text[] = {
+ [CLAW_MKEY_FUNCTION_MACRO] = "macro",
+ [CLAW_MKEY_FUNCTION_COMBO] = "combination",
+ [CLAW_MKEY_FUNCTION_DISABLED] = "disabled",
+};
+
+struct claw_command_report {
+ u8 report_id;
+ u8 padding[2];
+ u8 header_tail;
+ u8 cmd;
+ u8 data[59];
+} __packed;
+
+struct claw_drvdata {
+ /* MCU General Variables */
+ struct completion send_cmd_complete;
+ struct delayed_work cfg_resume;
+ struct delayed_work cfg_setup;
+ struct hid_device *hdev;
+ struct mutex cfg_mutex; /* mutex for synchronous data */
+ u8 ep;
+
+ /* Gamepad Variables */
+ enum claw_mkeys_function_index mkeys_function;
+ enum claw_gamepad_mode_index gamepad_mode;
+};
+
+static int get_endpoint_address(struct hid_device *hdev)
+{
+ struct usb_host_endpoint *ep;
+ struct usb_interface *intf;
+
+ intf = to_usb_interface(hdev->dev.parent);
+ ep = intf->cur_altsetting->endpoint;
+ if (ep)
+ return ep->desc.bEndpointAddress;
+
+ return -ENODEV;
+}
+
+static int claw_gamepad_mode_event(struct claw_drvdata *drvdata,
+ struct claw_command_report *cmd_rep)
+{
+ if (cmd_rep->data[0] >= ARRAY_SIZE(claw_gamepad_mode_text) ||
+ !claw_gamepad_mode_text[cmd_rep->data[0]] ||
+ cmd_rep->data[1] >= ARRAY_SIZE(claw_mkeys_function_text))
+ return -EINVAL;
+
+ drvdata->gamepad_mode = cmd_rep->data[0];
+ drvdata->mkeys_function = cmd_rep->data[1];
+
+ return 0;
+}
+
+static int claw_raw_event(struct claw_drvdata *drvdata, struct hid_report *report,
+ u8 *data, int size)
+{
+ struct claw_command_report *cmd_rep;
+ int ret = 0;
+
+ if (size != CLAW_PACKET_SIZE)
+ return 0;
+
+ cmd_rep = (struct claw_command_report *)data;
+
+ if (cmd_rep->report_id != CLAW_INPUT_REPORT_ID || cmd_rep->header_tail != 0x3c)
+ return 0;
+
+ dev_dbg(&drvdata->hdev->dev, "Rx data as raw input report: [%*ph]\n",
+ CLAW_PACKET_SIZE, data);
+
+ switch (cmd_rep->cmd) {
+ case CLAW_COMMAND_TYPE_GAMEPAD_MODE_ACK:
+ ret = claw_gamepad_mode_event(drvdata, cmd_rep);
+ break;
+ case CLAW_COMMAND_TYPE_ACK:
+ break;
+ default:
+ dev_dbg(&drvdata->hdev->dev, "Unknown command: %x\n", cmd_rep->cmd);
+ return 0;
+ }
+
+ complete(&drvdata->send_cmd_complete);
+
+ return ret;
+}
+
+static int msi_raw_event(struct hid_device *hdev, struct hid_report *report,
+ u8 *data, int size)
+{
+ struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
+
+ if (!drvdata || (drvdata->ep != CLAW_XINPUT_CFG_INTF_IN &&
+ drvdata->ep != CLAW_DINPUT_CFG_INTF_IN))
+ return 0;
+
+ return claw_raw_event(drvdata, report, data, size);
+}
+
+static int claw_hw_output_report(struct hid_device *hdev, u8 index, u8 *data,
+ size_t len, unsigned int timeout)
+{
+ unsigned char *dmabuf __free(kfree) = NULL;
+ u8 header[] = { CLAW_OUTPUT_REPORT_ID, 0, 0, 0x3c, index };
+ struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
+ size_t header_size = ARRAY_SIZE(header);
+ int ret;
+
+ if (header_size + len > CLAW_PACKET_SIZE)
+ return -EINVAL;
+
+ /* We can't use a devm_alloc reusable buffer without side effects during suspend */
+ dmabuf = kzalloc(CLAW_PACKET_SIZE, GFP_KERNEL);
+ if (!dmabuf)
+ return -ENOMEM;
+
+ memcpy(dmabuf, header, header_size);
+ if (data && len)
+ memcpy(dmabuf + header_size, data, len);
+
+ /* Don't hold a mutex when timeout=0, those commands cause USB disconnect */
+ if (timeout) {
+ guard(mutex)(&drvdata->cfg_mutex);
+ reinit_completion(&drvdata->send_cmd_complete);
+ }
+
+ dev_dbg(&hdev->dev, "Send data as raw output report: [%*ph]\n",
+ CLAW_PACKET_SIZE, dmabuf);
+
+ ret = hid_hw_output_report(hdev, dmabuf, CLAW_PACKET_SIZE);
+ if (ret < 0)
+ return ret;
+
+ ret = ret == CLAW_PACKET_SIZE ? 0 : -EIO;
+ if (ret)
+ return ret;
+
+ if (timeout) {
+ ret = wait_for_completion_interruptible_timeout(&drvdata->send_cmd_complete,
+ msecs_to_jiffies(timeout));
+
+ dev_dbg(&hdev->dev, "Remaining timeout: %u\n", ret);
+ if (ret >= 0) /* preserve errors */
+ ret = ret == 0 ? -EBUSY : 0; /* timeout occurred : time remained */
+ }
+
+ return ret;
+}
+
+static ssize_t gamepad_mode_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct hid_device *hdev = to_hid_device(dev);
+ struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
+ u8 data[2] = { 0x00, drvdata->mkeys_function };
+ int i, ret = -EINVAL;
+
+ for (i = 0; i < ARRAY_SIZE(claw_gamepad_mode_text); i++) {
+ if (claw_gamepad_mode_text[i] && sysfs_streq(buf, claw_gamepad_mode_text[i])) {
+ ret = i;
+ break;
+ }
+ }
+ if (ret < 0)
+ return ret;
+
+ data[0] = ret;
+
+ ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_SWITCH_MODE, data, ARRAY_SIZE(data), 0);
+ if (ret)
+ return ret;
+
+ return count;
+}
+
+static ssize_t gamepad_mode_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct hid_device *hdev = to_hid_device(dev);
+ struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
+ int ret, i;
+
+ ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_READ_GAMEPAD_MODE, NULL, 0, 8);
+ if (ret)
+ return ret;
+
+ i = drvdata->gamepad_mode;
+
+ if (!claw_gamepad_mode_text[i] || claw_gamepad_mode_text[i][0] == '\0')
+ return sysfs_emit(buf, "unsupported\n");
+
+ return sysfs_emit(buf, "%s\n", claw_gamepad_mode_text[i]);
+}
+static DEVICE_ATTR_RW(gamepad_mode);
+
+static ssize_t gamepad_mode_index_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ ssize_t count = 0;
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(claw_gamepad_mode_text); i++) {
+ if (!claw_gamepad_mode_text[i] || claw_gamepad_mode_text[i][0] == '\0')
+ continue;
+ count += sysfs_emit_at(buf, count, "%s ", claw_gamepad_mode_text[i]);
+ }
+
+ buf[count - 1] = '\n';
+
+ return count;
+}
+static DEVICE_ATTR_RO(gamepad_mode_index);
+
+static ssize_t mkeys_function_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct hid_device *hdev = to_hid_device(dev);
+ struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
+ u8 data[2] = { drvdata->gamepad_mode, 0x00 };
+ int i, ret = -EINVAL;
+
+ for (i = 0; i < ARRAY_SIZE(claw_mkeys_function_text); i++) {
+ if (claw_mkeys_function_text[i] && sysfs_streq(buf, claw_mkeys_function_text[i])) {
+ ret = i;
+ break;
+ }
+ }
+ if (ret < 0)
+ return ret;
+
+ data[1] = ret;
+
+ ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_SWITCH_MODE, data, ARRAY_SIZE(data), 0);
+ if (ret)
+ return ret;
+
+ return count;
+}
+
+static ssize_t mkeys_function_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ struct hid_device *hdev = to_hid_device(dev);
+ struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
+ int ret, i;
+
+ ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_READ_GAMEPAD_MODE, NULL, 0, 8);
+ if (ret)
+ return ret;
+
+ i = drvdata->mkeys_function;
+
+ if (i >= ARRAY_SIZE(claw_mkeys_function_text))
+ return sysfs_emit(buf, "unsupported\n");
+
+ return sysfs_emit(buf, "%s\n", claw_mkeys_function_text[i]);
+}
+static DEVICE_ATTR_RW(mkeys_function);
+
+static ssize_t mkeys_function_index_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ int i, count = 0;
+
+ for (i = 0; i < ARRAY_SIZE(claw_mkeys_function_text); i++)
+ count += sysfs_emit_at(buf, count, "%s ", claw_mkeys_function_text[i]);
+
+ buf[count - 1] = '\n';
+
+ return count;
+}
+static DEVICE_ATTR_RO(mkeys_function_index);
+
+static ssize_t reset_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct hid_device *hdev = to_hid_device(dev);
+ bool val;
+ int ret;
+
+ ret = kstrtobool(buf, &val);
+ if (ret)
+ return ret;
+
+ if (!val)
+ return -EINVAL;
+
+ ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_RESET_DEVICE, NULL, 0, 0);
+ if (ret < 0)
+ return ret;
+
+ return count;
+}
+static DEVICE_ATTR_WO(reset);
+
+static umode_t claw_gamepad_attr_is_visible(struct kobject *kobj, struct attribute *attr,
+ int n)
+{
+ struct hid_device *hdev = to_hid_device(kobj_to_dev(kobj));
+ struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
+
+ if (!drvdata) {
+ dev_warn(&hdev->dev,
+ "Failed to get drvdata from kobj. Gamepad attributes are not available.\n");
+ return 0;
+ }
+
+ return attr->mode;
+}
+
+static struct attribute *claw_gamepad_attrs[] = {
+ &dev_attr_gamepad_mode.attr,
+ &dev_attr_gamepad_mode_index.attr,
+ &dev_attr_mkeys_function.attr,
+ &dev_attr_mkeys_function_index.attr,
+ &dev_attr_reset.attr,
+ NULL,
+};
+
+static const struct attribute_group claw_gamepad_attr_group = {
+ .attrs = claw_gamepad_attrs,
+ .is_visible = claw_gamepad_attr_is_visible,
+};
+
+static void cfg_setup_fn(struct work_struct *work)
+{
+ struct delayed_work *dwork = container_of(work, struct delayed_work, work);
+ struct claw_drvdata *drvdata = container_of(dwork, struct claw_drvdata, cfg_setup);
+ int ret;
+
+ ret = claw_hw_output_report(drvdata->hdev, CLAW_COMMAND_TYPE_READ_GAMEPAD_MODE, NULL, 0, 8);
+ if (ret) {
+ dev_err(&drvdata->hdev->dev,
+ "Failed to setup device, can't read gamepad mode: %d\n", ret);
+ return;
+ }
+
+ /* Add sysfs attributes after we get the device state */
+ ret = devm_device_add_group(&drvdata->hdev->dev, &claw_gamepad_attr_group);
+ if (ret) {
+ dev_err(&drvdata->hdev->dev,
+ "Failed to setup device, can't create gamepad attrs: %d\n", ret);
+ return;
+ }
+
+ kobject_uevent(&drvdata->hdev->dev.kobj, KOBJ_CHANGE);
+}
+
+static void cfg_resume_fn(struct work_struct *work)
+{
+ struct delayed_work *dwork = container_of(work, struct delayed_work, work);
+ struct claw_drvdata *drvdata = container_of(dwork, struct claw_drvdata, cfg_resume);
+ u8 data[2] = { drvdata->gamepad_mode, drvdata->mkeys_function };
+ int ret;
+
+ ret = claw_hw_output_report(drvdata->hdev, CLAW_COMMAND_TYPE_SWITCH_MODE, data,
+ ARRAY_SIZE(data), 0);
+ if (ret)
+ dev_err(&drvdata->hdev->dev, "Failed to set gamepad mode settings: %d\n", ret);
+}
+
+static int claw_probe(struct hid_device *hdev, u8 ep)
+{
+ struct claw_drvdata *drvdata;
+ int ret;
+
+ drvdata = devm_kzalloc(&hdev->dev, sizeof(*drvdata), GFP_KERNEL);
+ if (!drvdata)
+ return -ENOMEM;
+
+ hid_set_drvdata(hdev, drvdata);
+ drvdata->hdev = hdev;
+ drvdata->ep = ep;
+
+ mutex_init(&drvdata->cfg_mutex);
+ init_completion(&drvdata->send_cmd_complete);
+
+ /* For control interface: open the HID transport for sending commands. */
+ ret = hid_hw_open(hdev);
+ if (ret)
+ return ret;
+
+ INIT_DELAYED_WORK(&drvdata->cfg_resume, &cfg_resume_fn);
+ INIT_DELAYED_WORK(&drvdata->cfg_setup, &cfg_setup_fn);
+ schedule_delayed_work(&drvdata->cfg_setup, msecs_to_jiffies(500));
+
+ return 0;
+}
+
+static int msi_probe(struct hid_device *hdev, const struct hid_device_id *id)
+{
+ int ret;
+ u8 ep;
+
+ if (!hid_is_usb(hdev)) {
+ ret = -ENODEV;
+ goto err_probe;
+ }
+
+ ret = hid_parse(hdev);
+ if (ret)
+ goto err_probe;
+
+ /* Set quirk to create separate input devices per HID application */
+ hdev->quirks |= HID_QUIRK_INPUT_PER_APP | HID_QUIRK_MULTI_INPUT;
+ ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
+ if (ret)
+ goto err_probe;
+
+ /* For non-control interfaces (keyboard/mouse), allow userspace to grab the devices. */
+ ret = get_endpoint_address(hdev);
+ if (ret < 0)
+ goto err_stop_hw;
+
+ ep = ret;
+ if (ep == CLAW_XINPUT_CFG_INTF_IN || ep == CLAW_DINPUT_CFG_INTF_IN) {
+ ret = claw_probe(hdev, ep);
+ if (ret)
+ goto err_stop_hw;
+ }
+
+ return 0;
+
+err_stop_hw:
+ hid_hw_stop(hdev);
+err_probe:
+ return dev_err_probe(&hdev->dev, ret, "Failed to init device\n");
+}
+
+static void claw_remove(struct hid_device *hdev)
+{
+ struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
+
+ if (!drvdata) {
+ hid_hw_stop(hdev);
+ return;
+ }
+
+ cancel_delayed_work_sync(&drvdata->cfg_setup);
+ cancel_delayed_work_sync(&drvdata->cfg_resume);
+ hid_hw_close(hdev);
+}
+
+static void msi_remove(struct hid_device *hdev)
+{
+ int ret;
+ u8 ep;
+
+ ret = get_endpoint_address(hdev);
+ if (ret <= 0)
+ goto hw_stop;
+
+ ep = ret;
+ if (ep == CLAW_XINPUT_CFG_INTF_IN || ep == CLAW_DINPUT_CFG_INTF_IN)
+ claw_remove(hdev);
+
+hw_stop:
+ hid_hw_stop(hdev);
+}
+
+static int claw_resume(struct hid_device *hdev)
+{
+ struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
+
+ /* MCU can take up to 500ms to be ready after resume */
+ schedule_delayed_work(&drvdata->cfg_resume, msecs_to_jiffies(500));
+ return 0;
+}
+
+static int msi_resume(struct hid_device *hdev)
+{
+ int ret;
+ u8 ep;
+
+ ret = get_endpoint_address(hdev);
+ if (ret <= 0)
+ return 0;
+
+ ep = ret;
+ if (ep == CLAW_XINPUT_CFG_INTF_IN || ep == CLAW_DINPUT_CFG_INTF_IN)
+ return claw_resume(hdev);
+
+ return 0;
+}
+
+static const struct hid_device_id msi_devices[] = {
+ { HID_USB_DEVICE(USB_VENDOR_ID_MSI_2, USB_DEVICE_ID_MSI_CLAW_XINPUT) },
+ { HID_USB_DEVICE(USB_VENDOR_ID_MSI_2, USB_DEVICE_ID_MSI_CLAW_DINPUT) },
+ { HID_USB_DEVICE(USB_VENDOR_ID_MSI_2, USB_DEVICE_ID_MSI_CLAW_DESKTOP) },
+ { HID_USB_DEVICE(USB_VENDOR_ID_MSI_2, USB_DEVICE_ID_MSI_CLAW_BIOS) },
+ { }
+};
+MODULE_DEVICE_TABLE(hid, msi_devices);
+
+static struct hid_driver msi_driver = {
+ .name = "hid-msi",
+ .id_table = msi_devices,
+ .raw_event = msi_raw_event,
+ .probe = msi_probe,
+ .remove = msi_remove,
+ .resume = msi_resume,
+};
+module_hid_driver(msi_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Denis Benato <denis.benato@linux.dev>");
+MODULE_AUTHOR("Zhouwang Huang <honjow311@gmail.com>");
+MODULE_AUTHOR("Derek J. Clark <derekjohn.clark@gmail.com>");
+MODULE_DESCRIPTION("HID driver for MSI Claw Handheld PC gamepads");
--
2.53.0
^ permalink raw reply related
* [PATCH v2 0/4] Add MSI Claw HID Configuration Driver
From: Derek J. Clark @ 2026-05-13 23:14 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: Pierre-Loup A . Griffais, Denis Benato, Zhouwang Huang,
Derek J . Clark, linux-input, linux-doc, linux-kernel
This series adds an HID Configuration driver for the MSI Claw line of
Handheld Gaming PC's. The MSI Claw HID interface provides multiple
features, such as the ability to switch between xinput, dinput, and a
desktop mode, RGB control, rumble intensity, and mapping of the rear "M"
keys. There are additional gamepad modes that are not included in this
driver as they appear to be used in assembly line testing or are
incomplete in the firmware. During my testing I found them to be unstable.
The initial version of this driver was written by Denis Benato, which
contained the initial reverse-engineering and implementation for the
gamepad mode switching. This work was later expanded by Zhouwang Huang
to include more gamepad modes and additional features. Finally, I
refactored the entire driver, fixed multiple bugs, and refined the overall
format to conform to kernel driver best practices and style guide.
Claude was used initially by Zhouwang Huang to quickly parse HID captures
during the reverse-engineering of some of the features. Since Claude had
already been used, as a test of its capabilities I had it implement the
rumble intensity attribute after I had already rewritten most of the
driver, which I then manually edited to fix some mistakes. I also used
Claude to review the driver and these patches for any mistakes and bugs.
Assisted-by: Claude:claude-sonnet-4-6
Co-developed-by: Denis Benato <denis.benato@linux.dev>
Signed-off-by: Denis Benato <denis.benato@linux.dev>
Co-developed-by: Zhouwang Huang <honjow311@gmail.com>
Signed-off-by: Zhouwang Huang <honjow311@gmail.com>
Signed-off-by: Derek J. Clark <derekjohn.clark@gmail.com>
---
v2:
- Use mutexes to guard SYNC_TO_ROM calls and pending_profile calls.
- Rename driver to hid-msi and add generic entrypoints for
probe/resume/remove that call claw specific functions in order to
future proof the driver for other MSI HID interfaces.
- Fix various bugs and formatting issues.
Derek J. Clark (4):
HID: hid-msi: Add MSI Claw configuration driver
HID: hid-msi: Add M-key mapping attributes
HID: hid-msi: Add RGB control interface
HID: hid-msi: Add Rumble Intensity Attributes
MAINTAINERS | 6 +
drivers/hid/Kconfig | 12 +
drivers/hid/Makefile | 1 +
drivers/hid/hid-ids.h | 6 +
drivers/hid/hid-msi.c | 1663 +++++++++++++++++++++++++++++++++++++++++
5 files changed, 1688 insertions(+)
create mode 100644 drivers/hid/hid-msi.c
--
2.53.0
^ permalink raw reply
* Re: [PATCH v1] Input: atlas - Check ACPI_COMPANION() against NULL
From: sashiko-bot @ 2026-05-13 22:58 UTC (permalink / raw)
To: Rafael J. Wysocki; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <8696590.T7Z3S40VBb@rafael.j.wysocki>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Incomplete Fix for driver_override Abuse (Global State Corruption)
--
commit ef51cbfbb2dd379fb06abd9c22db66e86bb70eb3
Author: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Input: atlas - Check ACPI_COMPANION() against NULL
This commit adds a check to ensure the ACPI companion device is present in
the atlas_btns driver to prevent issues when a device is forced to match
via device_match_driver_override().
> diff --git a/drivers/input/misc/atlas_btns.c b/drivers/input/misc/atlas_btns.c
> index 47b31725e8507..835ad45a9d65c 100644
> --- a/drivers/input/misc/atlas_btns.c
> +++ b/drivers/input/misc/atlas_btns.c
> @@ -60,11 +60,15 @@ static acpi_status acpi_atlas_button_handler(u32 function,
>
> static int atlas_acpi_button_probe(struct platform_device *pdev)
> {
> - struct acpi_device *device = ACPI_COMPANION(&pdev->dev);
> + struct acpi_device *device;
> acpi_status status;
> int i;
> int err;
>
> + device = ACPI_COMPANION(&pdev->dev);
> + if (!device)
> + return -ENODEV;
> +
> input_dev = input_allocate_device();
The commit message mentions this check is to protect against
device_match_driver_override() forcing a match with unexpected devices.
If driver_override is used to force-bind this driver to multiple platform
devices that do have ACPI companions, could this cause issues with the global
input_dev variable?
Since input_dev is overwritten on each successful probe, it looks like earlier
allocated input devices would be leaked if multiple bindings succeed.
Additionally, if these devices are later unbound, or if a subsequent probe
fails after modifying input_dev, could this lead to a double-free or null
pointer dereference when the shared input_dev pointer is cleaned up?
Would it make sense to either reject multiple bindings or store the input
device in the platform device's private data using platform_set_drvdata()?
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/8696590.T7Z3S40VBb@rafael.j.wysocki?part=1
^ permalink raw reply
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