* [PATCH v12 0/4] kunit: Add support for suppressing warning backtraces
From: Albert Esteve @ 2026-05-15 8:52 UTC (permalink / raw)
To: Arnd Bergmann, Brendan Higgins, David Gow, Rae Moar,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Jonathan Corbet, Shuah Khan, Andrew Morton,
Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti
Cc: linux-kernel, linux-arch, linux-kselftest, kunit-dev, dri-devel,
workflows, linux-riscv, linux-doc, peterz, Alessandro Carminati,
Guenter Roeck, Kees Cook, Albert Esteve,
Linux Kernel Functional Testing, Maíra Canal, Dan Carpenter,
Kees Cook, Simona Vetter
Some unit tests intentionally trigger warning backtraces by passing bad
parameters to kernel API functions. Such unit tests typically check the
return value from such calls, not the existence of the warning backtrace.
Such intentionally generated warning backtraces are neither desirable
nor useful for a number of reasons:
- They can result in overlooked real problems.
- A warning that suddenly starts to show up in unit tests needs to be
investigated and has to be marked to be ignored, for example by
adjusting filter scripts. Such filters are ad hoc because there is
no real standard format for warnings. On top of that, such filter
scripts would require constant maintenance.
One option to address the problem would be to add messages such as
"expected warning backtraces start/end here" to the kernel log.
However, that would again require filter scripts, might result in
missing real problematic warning backtraces triggered while the test
is running, and the irrelevant backtrace(s) would still clog the
kernel log.
Solve the problem by providing a means to suppress warning backtraces
originating from the current kthread while executing test code.
Since each KUnit test runs in its own kthread, this effectively scopes
suppression to the test that enabled it, without requiring any
architecture-specific code.
Overview:
Patch#1 Introduces the suppression infrastructure integrated into
KUnit's hook mechanism.
Patch#2 Adds selftests to validate the functionality.
Patch#3 Demonstrates real-world usage in the DRM subsystem.
Patch#4 Documents the new API and usage guidelines.
Design Notes:
Suppression is integrated into the existing KUnit hooks infrastructure,
reusing the kunit_running static branch for zero overhead
when no tests are running. The implementation lives entirely in the
kunit module; only a static-inline wrapper and a function pointer
slot are added to built-in code.
Suppression is checked at three points in the warning path:
- In `warn_slowpath_fmt()` (kernel/panic.c), for architectures without
__WARN_FLAGS. The check runs before any output, fully suppressing
both message and backtrace.
- In `__warn_printk()` (kernel/panic.c), for architectures that define
__WARN_FLAGS but not their own __WARN_printf (arm64, loongarch,
parisc, powerpc, riscv, sh). The check suppresses the warning message
text that is printed before the trap enters __report_bug().
- In `__report_bug()` (lib/bug.c), for architectures that define
__WARN_FLAGS. The check runs before `__warn()` is called, suppressing
the backtrace and stack dump.
To avoid double-counting on architectures where both `__warn_printk()`
and `__report_bug()` run for the same warning, the hook takes a bool
parameter: true to increment the suppression counter, false to suppress
without counting.
The suppression state is dynamically allocated via kunit_kzalloc() and
tied to the KUnit test lifecycle via `kunit_add_action()`, ensuring
automatic cleanup at test exit. Writer-side access to the global
suppression list is serialized with a spinlock; readers use RCU.
Two API forms are provided:
- kunit_warning_suppress(test) { ... }: scoped blocks with automatic
cleanup. The suppression handle is not accessible outside the block,
so warning counts (if needed) must be checked inside. Multiple
suppression blocks are allowed.
- kunit_start/end_suppress_warning(test): direct functions that return
an explicit handle. Use when the handle needs to be retained, or passed
across helpers. Multiple suppression blocks are allowed.
This series is based on the RFC patch and subsequent discussion at
https://patchwork.kernel.org/project/linux-kselftest/patch/02546e59-1afe-4b08-ba81-d94f3b691c9a@moroto.mountain/
and offers a more comprehensive solution of the problem discussed there.
Changes since RFC:
- Introduced CONFIG_KUNIT_SUPPRESS_BACKTRACE
- Minor cleanups and bug fixes
- Added support for all affected architectures
- Added support for counting suppressed warnings
- Added unit tests using those counters
- Added patch to suppress warning backtraces in dev_addr_lists tests
Changes since v1:
- Rebased to v6.9-rc1
- Added Tested-by:, Acked-by:, and Reviewed-by: tags
[I retained those tags since there have been no functional changes]
- Introduced KUNIT_SUPPRESS_BACKTRACE configuration option, enabled by
default.
Changes since v2:
- Rebased to v6.9-rc2
- Added comments to drm warning suppression explaining why it is needed.
- Added patch to move conditional code in arch/sh/include/asm/bug.h
to avoid kerneldoc warning
- Added architecture maintainers to Cc: for architecture specific patches
- No functional changes
Changes since v3:
- Rebased to v6.14-rc6
- Dropped net: "kunit: Suppress lock warning noise at end of dev_addr_lists tests"
since 3db3b62955cd6d73afde05a17d7e8e106695c3b9
- Added __kunit_ and KUNIT_ prefixes.
- Tested on interessed architectures.
Changes since v4:
- Rebased to v6.15-rc7
- Dropped all code in __report_bug()
- Moved all checks in WARN*() macros.
- Dropped all architecture specific code.
- Made __kunit_is_suppressed_warning nice to noinstr functions.
Changes since v5:
- Rebased to v7.0-rc3
- Added RCU protection for the suppressed warnings list.
- Added static key and branching optimization.
- Removed custom `strcmp` implementation and reworked
__kunit_is_suppressed_warning() entrypoint function.
Changes since v6:
- Moved suppression checks from WARN*() macros to warn_slowpath_fmt()
and __report_bug().
- Replaced stack-allocated suppression struct with kunit_kzalloc() heap
allocation tied to the KUnit test lifecycle.
- Changed suppression strategy from function-name matching to task-scoped:
all warnings on the current task are suppressed between START and END,
rather than only warnings originating from a specific named function.
- Simplified macro API: removed KUNIT_DECLARE_SUPPRESSED_WARNING(),
the START macro now takes (test) and handles allocation internally.
- Removed static key and branching optiomization, as by the time it
was executed, callers are already in warn slowpaths.
- Link to v6: https://lore.kernel.org/r/20260317-kunit_add_support-v6-0-dd22aeb3fe5d@redhat.com
Changes since v7:
- Integrated suppression into existing KUnit hooks infrastructure
- Removed CONFIG_KUNIT_SUPPRESS_BACKTRACE
- Added suppression check in __warn_printk()
- Added spinlock for writer-side RCU protection
- Replaced explicit rcu_read_lock/unlock with guard(rcu)()
- Added scoped API (kunit_warning_suppress) using __cleanup attribute
- Updated DRM patch to use scoped API
- Expanded self-tests: incremental counting, cross-kthread isolation
- Rewrote documentation covering all three API forms with examples
- Link to v7: https://lore.kernel.org/r/20260420-kunit_add_support-v7-0-e8bc6e0f70de@redhat.com
Changes since v8:
- Rebased to v7.1-rc2
- Remove KUNIT_START/END_SUPPRESSED_WARNING() macros
- Add KUNIT_EXPECT_SUPPRESSED_WARNING_COUNT checks to drm tests
- Link to v8: https://lore.kernel.org/r/20260504-kunit_add_support-v8-0-3e5957cdd235@redhat.com
Changes since v9:
- Fix silent false-pass when kunit_start_suppress_warning() returns NULL
- Fix RCU lockdep splat for kunit_is_suppressed_warning() calls
- Move disable_trace_on_warning() in __report_bug()
- Make suppress counter atomic
- Mark helper warn functions in selftest as noinline
- Add kunit_skip() for CONFIG_BUG=n in selftests
- Fix potentially uninitialized data.was_active in kthread seltest
- Add kthread_stop() in kthread selftest early exit
- Initialize scaling_factor to INT_MIN in DRM scaling tests
- Add include for bool in test-bug.h to fix CONFIG_KUNIT=n case
- Link to v9: https://lore.kernel.org/r/20260508-kunit_add_support-v9-0-99df7aa880f6@redhat.com
Changes since v10:
- Remove synchronize_rcu() to avoid sleeping in atomic context
- Pin task_struct refcount to prevent ABA false-positive matches
- Loop in suppression selftest to prevent use-after-free on kthread exit
- Skip DRM rect tests on CONFIG_BUG=n
- Link to v10: https://lore.kernel.org/r/20260513-kunit_add_support-v10-0-e379d206c8cd@redhat.com
Changes since v11:
- Use call_rcu() to defer free without blocking
- Remove #ifdef CONFIG_KUNIT guard in lib/bug.c
- Remove stale config checks from selftest
- Replace skip on DRM rect tests with conditional expectation
- Link to v11: https://lore.kernel.org/r/20260514-kunit_add_support-v11-0-b36a530a6d8f@redhat.com
--
2.34.1
---
To: Brendan Higgins <brendan.higgins@linux.dev>
To: David Gow <david@davidgow.net>
To: Rae Moar <raemoar63@gmail.com>
To: Andrew Morton <akpm@linux-foundation.org>
To: Paul Walmsley <pjw@kernel.org>
To: Palmer Dabbelt <palmer@dabbelt.com>
To: Albert Ou <aou@eecs.berkeley.edu>
To: Alexandre Ghiti <alex@ghiti.fr>
To: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
To: Maxime Ripard <mripard@kernel.org>
To: Thomas Zimmermann <tzimmermann@suse.de>
To: David Airlie <airlied@gmail.com>
To: Simona Vetter <simona@ffwll.ch>
To: Jonathan Corbet <corbet@lwn.net>
To: Shuah Khan <skhan@linuxfoundation.org>
Cc: linux-kernel@vger.kernel.org
Cc: linux-kselftest@vger.kernel.org
Cc: kunit-dev@googlegroups.com
Cc: linux-riscv@lists.infradead.org
Cc: dri-devel@lists.freedesktop.org
Cc: workflows@vger.kernel.org
Cc: linux-doc@vger.kernel.org
---
Alessandro Carminati (1):
bug/kunit: Core support for suppressing warning backtraces
Guenter Roeck (3):
kunit: Add backtrace suppression self-tests
drm: Suppress intentional warning backtraces in scaling unit tests
kunit: Add documentation for warning backtrace suppression API
Documentation/dev-tools/kunit/usage.rst | 46 +++++++-
drivers/gpu/drm/tests/drm_rect_test.c | 36 +++++-
include/kunit/test-bug.h | 26 +++++
include/kunit/test.h | 98 ++++++++++++++++
kernel/panic.c | 11 ++
lib/bug.c | 12 +-
lib/kunit/Makefile | 4 +-
lib/kunit/backtrace-suppression-test.c | 192 ++++++++++++++++++++++++++++++++
lib/kunit/bug.c | 131 ++++++++++++++++++++++
lib/kunit/hooks-impl.h | 2 +
10 files changed, 548 insertions(+), 10 deletions(-)
---
base-commit: 74fe02ce122a6103f207d29fafc8b3a53de6abaf
change-id: 20260312-kunit_add_support-2f35806b19dd
Best regards,
--
Albert Esteve <aesteve@redhat.com>
^ permalink raw reply
* Re: [PATCH v11 2/4] kunit: Add backtrace suppression self-tests
From: Albert Esteve @ 2026-05-15 8:30 UTC (permalink / raw)
To: Arnd Bergmann, Brendan Higgins, David Gow, Rae Moar,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Jonathan Corbet, Shuah Khan, Andrew Morton,
Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti
Cc: linux-kernel, linux-arch, linux-kselftest, kunit-dev, dri-devel,
workflows, linux-riscv, linux-doc, peterz, Guenter Roeck,
Linux Kernel Functional Testing, Alessandro Carminati,
Dan Carpenter, Kees Cook
In-Reply-To: <20260514-kunit_add_support-v11-2-b36a530a6d8f@redhat.com>
On Thu, May 14, 2026 at 1:07 PM Albert Esteve <aesteve@redhat.com> wrote:
>
> From: Guenter Roeck <linux@roeck-us.net>
>
> Add unit tests to verify that warning backtrace suppression works.
>
> Tests cover both API forms:
> - Scoped: kunit_warning_suppress() with in-block count verification
> and post-block inactivity check.
> - Direct functions: kunit_start/end_suppress_warning() with
> sequential independent suppression blocks and per-block counts.
>
> Furthermore, tests verify incremental warning counting, that
> kunit_has_active_suppress_warning() transitions correctly around
> suppression boundaries, and that suppression active in the test
> kthread does not leak to a separate kthread.
>
> If backtrace suppression does _not_ work, the unit tests will likely
> trigger unsuppressed backtraces, which should actually help to get
> the affected architectures / platforms fixed.
>
> Tested-by: Linux Kernel Functional Testing <lkft@linaro.org>
> Acked-by: Dan Carpenter <dan.carpenter@linaro.org>
> Reviewed-by: Kees Cook <keescook@chromium.org>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> Signed-off-by: Alessandro Carminati <acarmina@redhat.com>
> Reviewed-by: David Gow <david@davidgow.net>
> Signed-off-by: Albert Esteve <aesteve@redhat.com>
> ---
> lib/kunit/Makefile | 1 +
> lib/kunit/backtrace-suppression-test.c | 198 +++++++++++++++++++++++++++++++++
> 2 files changed, 199 insertions(+)
>
> diff --git a/lib/kunit/Makefile b/lib/kunit/Makefile
> index 4592f9d0aa8dd..2e8a6b71a2ab0 100644
> --- a/lib/kunit/Makefile
> +++ b/lib/kunit/Makefile
> @@ -22,6 +22,7 @@ obj-$(if $(CONFIG_KUNIT),y) += hooks.o
>
> obj-$(CONFIG_KUNIT_TEST) += kunit-test.o
> obj-$(CONFIG_KUNIT_TEST) += platform-test.o
> +obj-$(CONFIG_KUNIT_TEST) += backtrace-suppression-test.o
>
> # string-stream-test compiles built-in only.
> ifeq ($(CONFIG_KUNIT_TEST),y)
> diff --git a/lib/kunit/backtrace-suppression-test.c b/lib/kunit/backtrace-suppression-test.c
> new file mode 100644
> index 0000000000000..7a2a59c6a780d
> --- /dev/null
> +++ b/lib/kunit/backtrace-suppression-test.c
> @@ -0,0 +1,198 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * KUnit test for suppressing warning tracebacks.
> + *
> + * Copyright (C) 2024, Guenter Roeck
> + * Author: Guenter Roeck <linux@roeck-us.net>
> + */
> +
> +#include <kunit/test.h>
> +#include <linux/bug.h>
> +#include <linux/completion.h>
> +#include <linux/kthread.h>
> +
> +static void backtrace_suppression_test_warn_direct(struct kunit *test)
> +{
> + if (!IS_ENABLED(CONFIG_BUG))
> + kunit_skip(test, "requires CONFIG_BUG");
> +
> + kunit_warning_suppress(test) {
> + WARN(1, "This backtrace should be suppressed");
> + /*
> + * Count must be checked inside the scope; the handle
> + * is not accessible after the block exits.
> + */
> + KUNIT_EXPECT_SUPPRESSED_WARNING_COUNT(test, 1);
> + }
> + KUNIT_EXPECT_FALSE(test, kunit_has_active_suppress_warning());
> +}
> +
> +static noinline void trigger_backtrace_warn(void)
> +{
> + WARN(1, "This backtrace should be suppressed");
> +}
> +
> +static void backtrace_suppression_test_warn_indirect(struct kunit *test)
> +{
> + if (!IS_ENABLED(CONFIG_BUG))
> + kunit_skip(test, "requires CONFIG_BUG");
> +
> + kunit_warning_suppress(test) {
> + trigger_backtrace_warn();
> + KUNIT_EXPECT_SUPPRESSED_WARNING_COUNT(test, 1);
> + }
> +}
> +
> +static void backtrace_suppression_test_warn_multi(struct kunit *test)
> +{
> + if (!IS_ENABLED(CONFIG_BUG))
> + kunit_skip(test, "requires CONFIG_BUG");
> +
> + kunit_warning_suppress(test) {
> + WARN(1, "This backtrace should be suppressed");
> + trigger_backtrace_warn();
> + KUNIT_EXPECT_SUPPRESSED_WARNING_COUNT(test, 2);
> + }
> +}
> +
> +static void backtrace_suppression_test_warn_on_direct(struct kunit *test)
> +{
> + if (!IS_ENABLED(CONFIG_BUG))
> + kunit_skip(test, "requires CONFIG_BUG");
> + if (!IS_ENABLED(CONFIG_DEBUG_BUGVERBOSE) && !IS_ENABLED(CONFIG_KALLSYMS))
> + kunit_skip(test, "requires CONFIG_DEBUG_BUGVERBOSE or CONFIG_KALLSYMS");
> +
> + kunit_warning_suppress(test) {
> + WARN_ON(1);
> + KUNIT_EXPECT_SUPPRESSED_WARNING_COUNT(test, 1);
> + }
> +}
> +
> +static noinline void trigger_backtrace_warn_on(void)
> +{
> + WARN_ON(1);
> +}
> +
> +static void backtrace_suppression_test_warn_on_indirect(struct kunit *test)
> +{
> + if (!IS_ENABLED(CONFIG_BUG))
> + kunit_skip(test, "requires CONFIG_BUG");
> + if (!IS_ENABLED(CONFIG_DEBUG_BUGVERBOSE))
> + kunit_skip(test, "requires CONFIG_DEBUG_BUGVERBOSE");
Sashiko says:
"""
Is there a reason why backtrace_suppression_test_warn_on_direct() falls back
to checking CONFIG_KALLSYMS while
backtrace_suppression_test_warn_on_indirect() does not?
The core warning suppression logic matches on the task, so it seems like
CONFIG_KALLSYMS should be sufficient for both.
Could this cause tests to be unnecessarily skipped on systems with
CONFIG_KALLSYMS enabled but CONFIG_DEBUG_BUGVERBOSE disabled?
"""
This is interesting. I am not sure why they were different; it was
probably an oversight. But after looking at it, this is probably a
leftover from when suppression occurred at the macros. They do not
seem necessary anymore. So I will remove them.
> +
> + kunit_warning_suppress(test) {
> + trigger_backtrace_warn_on();
> + KUNIT_EXPECT_SUPPRESSED_WARNING_COUNT(test, 1);
> + }
> +}
> +
> +static void backtrace_suppression_test_count(struct kunit *test)
> +{
> + if (!IS_ENABLED(CONFIG_BUG))
> + kunit_skip(test, "requires CONFIG_BUG");
> +
> + kunit_warning_suppress(test) {
> + KUNIT_EXPECT_SUPPRESSED_WARNING_COUNT(test, 0);
> +
> + WARN(1, "suppressed");
> + KUNIT_EXPECT_SUPPRESSED_WARNING_COUNT(test, 1);
> +
> + WARN(1, "suppressed again");
> + KUNIT_EXPECT_SUPPRESSED_WARNING_COUNT(test, 2);
> + }
> +}
> +
> +static void backtrace_suppression_test_active_state(struct kunit *test)
> +{
> + KUNIT_EXPECT_FALSE(test, kunit_has_active_suppress_warning());
> +
> + kunit_warning_suppress(test) {
> + KUNIT_EXPECT_TRUE(test, kunit_has_active_suppress_warning());
> + }
> +
> + KUNIT_EXPECT_FALSE(test, kunit_has_active_suppress_warning());
> +
> + kunit_warning_suppress(test) {
> + KUNIT_EXPECT_TRUE(test, kunit_has_active_suppress_warning());
> + }
> +
> + KUNIT_EXPECT_FALSE(test, kunit_has_active_suppress_warning());
> +}
> +
> +static void backtrace_suppression_test_multi_scope(struct kunit *test)
> +{
> + struct kunit_suppressed_warning *sw1, *sw2;
> +
> + if (!IS_ENABLED(CONFIG_BUG))
> + kunit_skip(test, "requires CONFIG_BUG");
> + if (!IS_ENABLED(CONFIG_DEBUG_BUGVERBOSE))
> + kunit_skip(test, "requires CONFIG_DEBUG_BUGVERBOSE");
> +
> + sw1 = kunit_start_suppress_warning(test);
> + trigger_backtrace_warn_on();
> + WARN(1, "suppressed by sw1");
> + kunit_end_suppress_warning(test, sw1);
> +
> + sw2 = kunit_start_suppress_warning(test);
> + WARN(1, "suppressed by sw2");
> + kunit_end_suppress_warning(test, sw2);
> +
> + KUNIT_EXPECT_EQ(test, kunit_suppressed_warning_count(sw1), 2);
> + KUNIT_EXPECT_EQ(test, kunit_suppressed_warning_count(sw2), 1);
> +}
> +
> +struct cross_kthread_data {
> + bool was_active;
> + struct completion done;
> +};
> +
> +static int cross_kthread_fn(void *data)
> +{
> + struct cross_kthread_data *d = data;
> +
> + d->was_active = kunit_has_active_suppress_warning();
> + complete(&d->done);
> + while (!kthread_should_stop())
> + schedule();
> + return 0;
> +}
> +
> +static void backtrace_suppression_test_cross_kthread(struct kunit *test)
> +{
> + struct cross_kthread_data data;
> + struct task_struct *task;
> +
> + data.was_active = false;
> + init_completion(&data.done);
> +
> + kunit_warning_suppress(test) {
> + task = kthread_run(cross_kthread_fn, &data, "kunit-cross-test");
> + KUNIT_ASSERT_FALSE(test, IS_ERR(task));
> + wait_for_completion(&data.done);
> + kthread_stop(task);
> + }
> +
> + KUNIT_EXPECT_FALSE(test, data.was_active);
> +}
> +
> +static struct kunit_case backtrace_suppression_test_cases[] = {
> + KUNIT_CASE(backtrace_suppression_test_warn_direct),
> + KUNIT_CASE(backtrace_suppression_test_warn_indirect),
> + KUNIT_CASE(backtrace_suppression_test_warn_multi),
> + KUNIT_CASE(backtrace_suppression_test_warn_on_direct),
> + KUNIT_CASE(backtrace_suppression_test_warn_on_indirect),
> + KUNIT_CASE(backtrace_suppression_test_count),
> + KUNIT_CASE(backtrace_suppression_test_active_state),
> + KUNIT_CASE(backtrace_suppression_test_multi_scope),
> + KUNIT_CASE(backtrace_suppression_test_cross_kthread),
> + {}
> +};
> +
> +static struct kunit_suite backtrace_suppression_test_suite = {
> + .name = "backtrace-suppression-test",
> + .test_cases = backtrace_suppression_test_cases,
> +};
> +kunit_test_suites(&backtrace_suppression_test_suite);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_DESCRIPTION("KUnit test to verify warning backtrace suppression");
>
> --
> 2.53.0
>
^ permalink raw reply
* Re: [PATCH v7 13/20] KVM: arm64: Apply dynamic guest counter reservations
From: James Clark @ 2026-05-15 8:28 UTC (permalink / raw)
To: Colton Lewis
Cc: alexandru.elisei, pbonzini, corbet, linux, catalin.marinas, will,
maz, oliver.upton, mizhang, joey.gouly, suzuki.poulose, yuzenghui,
mark.rutland, shuah, gankulkarni, linux-doc, linux-kernel,
linux-arm-kernel, kvmarm, linux-perf-users, linux-kselftest, kvm
In-Reply-To: <gsntlddlbylw.fsf@coltonlewis-kvm.c.googlers.com>
On 14/05/2026 8:05 pm, Colton Lewis wrote:
> James Clark <james.clark@linaro.org> writes:
>
>> On 13/05/2026 5:45 pm, Colton Lewis wrote:
>>> James Clark <james.clark@linaro.org> writes:
>
>>>> On 04/05/2026 10:18 pm, Colton Lewis wrote:
>>>>> Apply dynamic guest counter reservations by checking if the requested
>>>>> guest mask collides with any events the host has scheduled and calling
>>>>> pmu_perf_resched_update() with a hook that updates the mask of
>>>>> available counters in between schedule out and schedule in.
>
>>>>> Signed-off-by: Colton Lewis <coltonlewis@google.com>
>>>>> ---
>>>>> arch/arm64/kvm/pmu-direct.c | 69 ++++++++++++++++++++++++++++++++
>>>>> ++++
>>>>> include/linux/perf/arm_pmu.h | 1 +
>>>>> 2 files changed, 70 insertions(+)
>
>>>>> diff --git a/arch/arm64/kvm/pmu-direct.c b/arch/arm64/kvm/pmu-direct.c
>>>>> index 2252d3b905db9..14cc419dbafad 100644
>>>>> --- a/arch/arm64/kvm/pmu-direct.c
>>>>> +++ b/arch/arm64/kvm/pmu-direct.c
>>>>> @@ -100,6 +100,73 @@ u8 kvm_pmu_hpmn(struct kvm_vcpu *vcpu)
>>>>> return *host_data_ptr(nr_event_counters);
>>>>> }
>
>>>>> +/* Callback to update counter mask between perf scheduling */
>>>>> +static void kvm_pmu_update_mask(struct pmu *pmu, void *data)
>>>>> +{
>>>>> + struct arm_pmu *arm_pmu = to_arm_pmu(pmu);
>>>>> + unsigned long *new_mask = data;
>>>>> +
>>>>> + bitmap_copy(arm_pmu->cntr_mask, new_mask, ARMPMU_MAX_HWEVENTS);
>>>>> +}
>>>>> +
>>>>> +/**
>>>>> + * kvm_pmu_set_guest_counters() - Handle dynamic counter reservations
>>>>> + * @cpu_pmu: struct arm_pmu to potentially modify
>>>>> + * @guest_mask: new guest mask for the pmu
>>>>> + *
>>>>> + * Check if guest counters will interfere with current host events
>>>>> and
>>>>> + * call into perf_pmu_resched_update if a reschedule is required.
>>>>> + */
>>>>> +static void kvm_pmu_set_guest_counters(struct arm_pmu *cpu_pmu, u64
>>>>> guest_mask)
>>>>> +{
>>>>> + struct pmu_hw_events *cpuc = this_cpu_ptr(cpu_pmu->hw_events);
>>>>> + DECLARE_BITMAP(guest_bitmap, ARMPMU_MAX_HWEVENTS);
>>>>> + DECLARE_BITMAP(new_mask, ARMPMU_MAX_HWEVENTS);
>>>>> + bool need_resched = false;
>>>>> +
>>>>> + bitmap_from_arr64(guest_bitmap, &guest_mask,
>>>>> ARMPMU_MAX_HWEVENTS);
>>>>> + bitmap_copy(new_mask, cpu_pmu->hw_cntr_mask,
>>>>> ARMPMU_MAX_HWEVENTS);
>>>>> +
>>>>> + if (guest_mask) {
>>>>> + /* Subtract guest counters from available host mask */
>>>>> + bitmap_andnot(new_mask, new_mask, guest_bitmap,
>>>>> ARMPMU_MAX_HWEVENTS);
>>>>> +
>>>>> + /* Did we collide with an active host event? */
>>>>> + if (bitmap_intersects(cpuc->used_mask, guest_bitmap,
>>>>> ARMPMU_MAX_HWEVENTS)) {
>>>>> + int idx;
>>>>> +
>>>>> + need_resched = true;
>>>>> + cpuc->host_squeezed = true;
>>>>> +
>>>>> + /* Look for pinned events that are about to be
>>>>> preempted */
>>>>> + for_each_set_bit(idx, guest_bitmap,
>>>>> ARMPMU_MAX_HWEVENTS) {
>>>>> + if (test_bit(idx, cpuc->used_mask) && cpuc-
>>>>> >events[idx] &&
>>>>> + cpuc->events[idx]->attr.pinned) {
>>>>> + pr_warn_ratelimited("perf: Pinned host event
>>>>> squeezed out by KVM guest PMU partition\n");
>
>>>> Hi Colton,
>
>>>> I get "perf: Pinned host event squeezed out by KVM guest PMU partition"
>>>> even with arm_pmuv3.reserved_host_counters=3 for example. I would have
>>>> expected any non zero value to stop the warning.
>
>>>> I think armv8pmu_get_single_idx() needs to be changed to allocate from
>>>> the high end host counters first. A more complicated option would be
>>>> checking to see if there are any non-pinned counters in the host
>>>> reserved half when a new pinned counter is opened, then swapping the
>>>> places of the new pinned and existing non-pinned counters so pinned
>>>> always prefer being put into the host half. But it's probably not worth
>>>> doing that.
>
>>>> James
>
>
>>> I agree it makes the most sense to allocate from the top, but I'm happy
>>> the basic idea works.
>
>
>> Another thing I forgot to mention is that even with the ratelimited
>> warning, this spams the logs any time the host and guest are both using
>> the PMU and I'm not sure how useful that is.
>
> I'm sure it does. I'll delete it.
>
A warn_once might save someone a few hours of debugging, but we probably
don't need more than that.
>>>>> + break;
>>>>> + }
>>>>> + }
>>>>> + }
>>>>> + } else {
>>>>> + /*
>>>>> + * Restoring to hw_cntr_mask.
>>>>> + * Only resched if we previously squeezed an event.
>>>>> + */
>>>>> + if (cpuc->host_squeezed) {
>>>>> + need_resched = true;
>>>>> + cpuc->host_squeezed = false;
>>>>> + }
>>>>> + }
>>>>> +
>>>>> + if (need_resched) {
>>>>> + /* Collision: run full perf reschedule */
>>>>> + perf_pmu_resched_update(&cpu_pmu->pmu, kvm_pmu_update_mask,
>>>>> new_mask);
>>>>> + } else {
>>>>> + /* Host was never using guest counters anyway */
>>>>> + bitmap_copy(cpu_pmu->cntr_mask, new_mask,
>>>>> ARMPMU_MAX_HWEVENTS);
>>>>> + }
>>>>> +}
>>>>> +
>>>>> /**
>>>>> * kvm_pmu_host_counter_mask() - Compute bitmask of host-reserved
>>>>> counters
>>>>> * @pmu: Pointer to arm_pmu struct
>>>>> @@ -218,6 +285,7 @@ void kvm_pmu_load(struct kvm_vcpu *vcpu)
>
>>>>> pmu = vcpu->kvm->arch.arm_pmu;
>>>>> guest_counters = kvm_pmu_guest_counter_mask(pmu);
>>>>> + kvm_pmu_set_guest_counters(pmu, guest_counters);
>>>>> kvm_pmu_apply_event_filter(vcpu);
>
>>>>> for_each_set_bit(i, &guest_counters, ARMPMU_MAX_HWEVENTS) {
>>>>> @@ -319,5 +387,6 @@ void kvm_pmu_put(struct kvm_vcpu *vcpu)
>>>>> val = read_sysreg(pmintenset_el1);
>>>>> __vcpu_assign_sys_reg(vcpu, PMINTENSET_EL1, val & mask);
>
>>>>> + kvm_pmu_set_guest_counters(pmu, 0);
>>>>> preempt_enable();
>>>>> }
>>>>> diff --git a/include/linux/perf/arm_pmu.h b/include/linux/perf/
>>>>> arm_pmu.h
>>>>> index f7b000bb3eca8..63f88fec5e80f 100644
>>>>> --- a/include/linux/perf/arm_pmu.h
>>>>> +++ b/include/linux/perf/arm_pmu.h
>>>>> @@ -75,6 +75,7 @@ struct pmu_hw_events {
>
>>>>> /* Active events requesting branch records */
>>>>> unsigned int branch_users;
>>>>> + bool host_squeezed;
>>>>> };
>
>>>>> enum armpmu_attr_groups {
^ permalink raw reply
* Re: [PATCH v10 8/9] platform/chrome: Protect cros_ec_device lifecycle with revocable
From: Tzung-Bi Shih @ 2026-05-15 8:28 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Arnd Bergmann, Greg Kroah-Hartman, Bartosz Golaszewski,
Linus Walleij, Benson Leung, linux-kernel, chrome-platform,
driver-core, linux-doc, linux-gpio, Rafael J. Wysocki,
Danilo Krummrich, Jonathan Corbet, Shuah Khan, Laurent Pinchart,
Wolfram Sang, Johan Hovold, Paul E . McKenney
In-Reply-To: <20260514160214.GH787748@nvidia.com>
On Thu, May 14, 2026 at 01:02:14PM -0300, Jason Gunthorpe wrote:
> On Thu, May 14, 2026 at 03:33:55AM +0000, Tzung-Bi Shih wrote:
>
> > > Given you say this is such a bug I think you really should be sending
> > > a series that is patches 5 through 7 from the other series and a
> > > simple rwsem instead of misc_deregister_sync() to deal with this bug
> > > ASAP. No need to complicate a simple bug fix in a driver with all
> > > these core changes.
> >
> > Apologies for missing this suggestion.
> >
> > For "patches 5 through 7 from the other series" I guess you're referring:
> > - https://lore.kernel.org/all/20260427134659.95181-6-tzungbi@kernel.org
> > - https://lore.kernel.org/all/20260427134659.95181-7-tzungbi@kernel.org
> > - https://lore.kernel.org/all/20260427134659.95181-8-tzungbi@kernel.org
>
> Yes
>
> > Could you provide a bit more detail on the rwsem approach? I'm not
> > entirely clear on what data or operations the rwsem would be protecting.
>
> Just put a rwsem, or even scru, inside the driver's fops.
>
> You can refactor that out to a misc or revocable later.
I see. Thank you for your suggestion. I will explore it and send out a
new version.
^ permalink raw reply
* htmldocs: Documentation/scheduler/sched-arch.rst:108: WARNING: Block quote ends without a blank line; unexpected unindent. [docutils]
From: kernel test robot @ 2026-05-15 8:10 UTC (permalink / raw)
To: Shrikanth Hegde; +Cc: oe-kbuild-all, 0day robot, linux-doc
tree: https://github.com/intel-lab-lkp/linux/commits/Shrikanth-Hegde/sched-debug-Remove-unused-schedstats/20260515-054159
head: 7c2ea3b4f5530a92ddf8bf0b9835101138cbcefb
commit: a9c3470cec4255c02ebdc8637ef447726b6b792f sched/docs: Document cpu_preferred_mask and Preferred CPU concept
date: 10 hours ago
compiler: clang version 20.1.8 (https://github.com/llvm/llvm-project 87f0227cb60147a26a1eeb4fb06e3b505e9c7261)
docutils: docutils (Docutils 0.21.2, Python 3.13.5, on linux)
reproduce: (https://download.01.org/0day-ci/archive/20260515/202605151011.KtNAJKrV-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202605151011.KtNAJKrV-lkp@intel.com/
All warnings (new ones prefixed by >>):
Checksumming on output with GSO
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [docutils]
MAINTAINERS:40: WARNING: Inline strong start-string without end-string. [docutils]
Documentation/scheduler/sched-arch.rst:107: ERROR: Unexpected indentation. [docutils]
>> Documentation/scheduler/sched-arch.rst:108: WARNING: Block quote ends without a blank line; unexpected unindent. [docutils]
Documentation/userspace-api/landlock:504: ./security/landlock/errata/abi-4.h:5: ERROR: Unexpected section title.
vim +108 Documentation/scheduler/sched-arch.rst
102
103 Notes:
104 1. This feature is available under CONFIG_PREFERRED_CPU
105 2. This feature works for FAIR/RT class.
106 3. A task pinned, which can't be moved to preferred CPUs will continue
107 to run based on its affinity. But no load balancing happens
> 108 4. If needed, steal time based governors/arch dependent method
109 could be used to cater to different types of cpu numbers.
110 Arch can do so by implementing its own hooks.
111 5. Decision to use/not use is driven by kernel. Hence it shouldn't
112 break user affinities. One of the main reason why CPU hotplug
113 or Isolated cpuset partitions was not a solution.
114
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* Re: [PATCH] driver core: Add cmdline option to force probe type
From: Greg KH @ 2026-05-15 8:05 UTC (permalink / raw)
To: Jianlin Lv
Cc: corbet, skhan, rafael, dakr, jianlv, linux-kernel, linux-doc,
driver-core
In-Reply-To: <CAFA-uR8Jj+MJaauAeEUoS+OfuudrV8C4mG2we-nd7-Cvofokhw@mail.gmail.com>
On Fri, May 15, 2026 at 03:54:17PM +0800, Jianlin Lv wrote:
> On Fri, May 15, 2026 at 2:13 PM Greg KH <gregkh@linuxfoundation.org> wrote:
> >
> > On Thu, May 14, 2026 at 11:09:03PM +0800, Jianlin Lv wrote:
> > > On Thu, May 14, 2026 at 9:49 PM Greg KH <gregkh@linuxfoundation.org> wrote:
> > > >
> > > > On Thu, May 14, 2026 at 09:35:08PM +0800, Jianlin Lv wrote:
> > > > > On Thu, May 14, 2026 at 6:16 PM Greg KH <gregkh@linuxfoundation.org> wrote:
> > > > > >
> > > > > > On Thu, May 14, 2026 at 05:49:55PM +0800, Jianlin Lv wrote:
> > > > > > > From: Jianlin Lv <iecedge@gmail.com>
> > > > > > >
> > > > > > > Device drivers that use asynchronous probing can cause non-deterministic
> > > > > > > device ordering and naming across reboots. A typical example is storage
> > > > > > > drivers (like sd/nvme): asynchronous probing can lead to inconsistent disk
> > > > > > > logical names after reboot. In scenarios where disk naming consistency is
> > > > > > > critical, the probe type should be set to synchronous.
> > > > > > >
> > > > > > > This patch introduces a driver_probe kernel parameter that overrides any
> > > > > > > driver's hard-coded probe type settings and allows runtime control without
> > > > > > > requiring kernel recompilation:
> > > > > > >
> > > > > > > driver_probe=PROBE_TYPE_SYNC,nvme,sd # Force specific drivers sync
> > > > > > > driver_probe=PROBE_TYPE_ASYNC,*,usb # Force all async except usb
> > > > > > > driver_probe=PROBE_TYPE_SYNC,* # Force all drivers synchronous
> > > > > > >
> > > > > > > The implementation replaces the limited driver_async_probe parameter with
> > > > > > > a more flexible interface that can force either synchronous or asynchronous
> > > > > > > probing as needed.
> > > > > > >
> > > > > > > Signed-off-by: Jianlin Lv <iecedge@gmail.com>
> > > > > > > ---
> > > > > > > .../admin-guide/kernel-parameters.txt | 27 +++++--
> > > > > > > drivers/base/dd.c | 71 ++++++++++++++-----
> > > > > > > 2 files changed, 74 insertions(+), 24 deletions(-)
> > > > > > >
> > > > > > > diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> > > > > > > index 4d0f545fb3ec..b43a8bd20356 100644
> > > > > > > --- a/Documentation/admin-guide/kernel-parameters.txt
> > > > > > > +++ b/Documentation/admin-guide/kernel-parameters.txt
> > > > > > > @@ -1377,12 +1377,27 @@ Kernel parameters
> > > > > > > it becomes active and is searched during signature
> > > > > > > verification.
> > > > > > >
> > > > > > > - driver_async_probe= [KNL]
> > > > > > > - List of driver names to be probed asynchronously. *
> > > > > > > - matches with all driver names. If * is specified, the
> > > > > > > - rest of the listed driver names are those that will NOT
> > > > > > > - match the *.
> > > > > > > - Format: <driver_name1>,<driver_name2>...
> > > > > >
> > > > > > You can not remove an existing user/kernel api, sorry, that is not
> > > > > > allowed as you just broke all systems that were relying on this :(
> > > > > >
> > > > > Could you provide more suggestions on how to improve this patch?
> > > >
> > > > Not really, sorry, I don't think this is a change that should be done at
> > > > all. disk naming is a long-solved issue, to think that you can fix that
> > > > by doing sync/async device probing is not understanding both the issues
> > > > involved, and how we solved it already :)
> > >
> > > Do you mean referencing disks via by-path/by-id?
> >
> > No, use something that does not change, like filesystem labels or
> > serial numbers, or something else that is guaranteed unique.
> >
> > > In our production env
> > > they can also be unstable; this is an example I encountered before:
> > > https://lore.kernel.org/all/CAFA-uR_jk6jCmf9DTebSVBRwtoLuXuyvf1Biq+OObqRVAOZbBw@mail.gmail.com/
> >
> > Yes, paths can, and will, change. Don't use them.
> >
> > Why not use a UUID, that is explicitly what those are designed for.
>
> For cloud deployment, the local volume provisioner detects
> and creates PVs for each local disk on the host, and it cleans up
> the disks when they are released.
> The local volume provisioner is deployed in the cluster as
> DaemonSet. For the same SKU, it uses a single, unified configuration
> file to initialize local disks. In this scenario, UUIDs are not applicable.
> In our case, logical names are used to identify the disk.
Ok, but again, there is NEVER a guarantee that those logical names are
going to be the same across boots. Please NEVER rely on them if you
wish to mount things properly. Use a label on the filesystem instead.
Or something else, you have loads of possibilities here. Attempting to
manipulate driver sync/async order is not going to solve your problem at
all.
> > > I understand that device naming in the kernel can change at any time. However,
> > > Is it necessary to provide an interface that allows users to choose
> > > the probe mode themselves?
> >
> > It's not going to solve your problem, so I wouldn't worry about it. And
> > you can't remove it, although I really would like to :)
>
> OK,Is the reason we can’t change the priority of driver_async_probe setting that
> the original logic has changed, i.e. it would alter the original behavior?
Because it is a user api that people might be relying on for various
unknown reasons.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH v11 1/4] bug/kunit: Core support for suppressing warning backtraces
From: Albert Esteve @ 2026-05-15 7:58 UTC (permalink / raw)
To: Arnd Bergmann, Brendan Higgins, David Gow, Rae Moar,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Jonathan Corbet, Shuah Khan, Andrew Morton,
Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti
Cc: linux-kernel, linux-arch, linux-kselftest, kunit-dev, dri-devel,
workflows, linux-riscv, linux-doc, peterz, Alessandro Carminati,
Guenter Roeck, Kees Cook
In-Reply-To: <20260514-kunit_add_support-v11-1-b36a530a6d8f@redhat.com>
On Thu, May 14, 2026 at 1:07 PM Albert Esteve <aesteve@redhat.com> wrote:
>
> From: Alessandro Carminati <acarmina@redhat.com>
>
> Some unit tests intentionally trigger warning backtraces by passing bad
> parameters to kernel API functions. Such unit tests typically check the
> return value from such calls, not the existence of the warning backtrace.
>
> Such intentionally generated warning backtraces are neither desirable
> nor useful for a number of reasons:
> - They can result in overlooked real problems.
> - A warning that suddenly starts to show up in unit tests needs to be
> investigated and has to be marked to be ignored, for example by
> adjusting filter scripts. Such filters are ad hoc because there is
> no real standard format for warnings. On top of that, such filter
> scripts would require constant maintenance.
>
> Solve the problem by providing a means to suppress warning backtraces
> originating from the current kthread while executing test code. Since
> each KUnit test runs in its own kthread, this effectively scopes
> suppression to the test that enabled it. Limit changes to generic code
> to the absolute minimum.
>
> Implementation details:
> Suppression is integrated into the existing KUnit hooks infrastructure
> in test-bug.h, reusing the kunit_running static branch for zero
> overhead when no tests are running.
>
> Suppression is checked at three points in the warning path:
> - In warn_slowpath_fmt(), the check runs before any output, fully
> suppressing both message and backtrace. This covers architectures
> without __WARN_FLAGS.
> - In __warn_printk(), the check suppresses the warning message text.
> This covers architectures that define __WARN_FLAGS but not their own
> __WARN_printf (arm64, loongarch, parisc, powerpc, riscv, sh), where
> the message is printed before the trap enters __report_bug().
> - In __report_bug(), the check runs before __warn() is called,
> suppressing the backtrace and stack dump.
>
> To avoid double-counting on architectures where both __warn_printk()
> and __report_bug() run for the same warning, kunit_is_suppressed_warning()
> takes a bool parameter: true to increment the suppression counter
> (used in warn_slowpath_fmt and __report_bug), false to check only
> (used in __warn_printk).
>
> The suppression state is dynamically allocated via kunit_kzalloc() and
> tied to the KUnit test lifecycle via kunit_add_action(), ensuring
> automatic cleanup at test exit. Writer-side access to the global
> suppression list is serialized with a spinlock; readers use RCU.
>
> Two API forms are provided:
> - kunit_warning_suppress(test) { ... }: scoped, uses __cleanup for
> automatic teardown on scope exit, kunit_add_action() as safety net
> for abnormal exits (e.g. kthread_exit from failed assertions).
> Suppression handle is only accessible inside the block.
> - kunit_start/end_suppress_warning(test): direct functions returning
> an explicit handle, for retaining the handle within the test,
> or for cross-function usage.
>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> Signed-off-by: Alessandro Carminati <acarmina@redhat.com>
> Reviewed-by: Kees Cook <kees@kernel.org>
> Reviewed-by: David Gow <david@davidgow.net>
> Signed-off-by: Albert Esteve <aesteve@redhat.com>
> ---
> include/kunit/test-bug.h | 26 ++++++++++
> include/kunit/test.h | 98 ++++++++++++++++++++++++++++++++++++++
> kernel/panic.c | 11 +++++
> lib/bug.c | 14 +++++-
> lib/kunit/Makefile | 3 +-
> lib/kunit/bug.c | 120 +++++++++++++++++++++++++++++++++++++++++++++++
> lib/kunit/hooks-impl.h | 2 +
> 7 files changed, 271 insertions(+), 3 deletions(-)
>
> diff --git a/include/kunit/test-bug.h b/include/kunit/test-bug.h
> index 47aa8f21ccce8..99869029fc686 100644
> --- a/include/kunit/test-bug.h
> +++ b/include/kunit/test-bug.h
> @@ -10,6 +10,7 @@
> #define _KUNIT_TEST_BUG_H
>
> #include <linux/stddef.h> /* for NULL */
> +#include <linux/types.h> /* for bool */
>
> #if IS_ENABLED(CONFIG_KUNIT)
>
> @@ -23,6 +24,7 @@ DECLARE_STATIC_KEY_FALSE(kunit_running);
> extern struct kunit_hooks_table {
> __printf(3, 4) void (*fail_current_test)(const char*, int, const char*, ...);
> void *(*get_static_stub_address)(struct kunit *test, void *real_fn_addr);
> + bool (*is_suppressed_warning)(bool count);
> } kunit_hooks;
>
> /**
> @@ -60,9 +62,33 @@ static inline struct kunit *kunit_get_current_test(void)
> } \
> } while (0)
>
> +/**
> + * kunit_is_suppressed_warning() - Check if warnings are being suppressed
> + * by the current KUnit test.
> + * @count: if true, increment the suppression counter on match.
> + *
> + * Returns true if the current task has active warning suppression.
> + * Uses the kunit_running static branch for zero overhead when no tests run.
> + *
> + * A single WARN*() may traverse multiple call sites in the warning path
> + * (e.g., __warn_printk() and __report_bug()). Pass @count = true at the
> + * primary suppression point to count each warning exactly once, and
> + * @count = false at secondary points to suppress output without
> + * inflating the count.
> + */
> +static inline bool kunit_is_suppressed_warning(bool count)
> +{
> + if (!static_branch_unlikely(&kunit_running))
> + return false;
> +
> + return kunit_hooks.is_suppressed_warning &&
> + kunit_hooks.is_suppressed_warning(count);
> +}
> +
> #else
>
> static inline struct kunit *kunit_get_current_test(void) { return NULL; }
> +static inline bool kunit_is_suppressed_warning(bool count) { return false; }
>
> #define kunit_fail_current_test(fmt, ...) do {} while (0)
>
> diff --git a/include/kunit/test.h b/include/kunit/test.h
> index 9cd1594ab697d..be71612f61655 100644
> --- a/include/kunit/test.h
> +++ b/include/kunit/test.h
> @@ -1795,4 +1795,102 @@ do { \
> // include resource.h themselves if they need it.
> #include <kunit/resource.h>
>
> +/*
> + * Warning backtrace suppression API.
> + *
> + * Suppresses WARN*() backtraces on the current task while active. Two forms
> + * are provided:
> + *
> + * - Scoped: kunit_warning_suppress(test) { ... }
> + * Suppression is active for the duration of the block. On normal exit,
> + * the for-loop increment deactivates suppression. On early exit (break,
> + * return, goto), the __cleanup attribute fires. On kthread_exit() (e.g.,
> + * a failed KUnit assertion), kunit_add_action() cleans up at test
> + * teardown. The suppression handle is only accessible inside the block,
> + * so warning counts must be checked before the block exits.
> + *
> + * - Direct: kunit_start_suppress_warning() / kunit_end_suppress_warning()
> + * The underlying functions, returning an explicit handle pointer. Use
> + * when the handle needs to be retained (e.g., for post-suppression
> + * count checks) or passed across helper functions.
> + */
> +struct kunit_suppressed_warning;
> +
> +struct kunit_suppressed_warning *
> +kunit_start_suppress_warning(struct kunit *test);
> +void kunit_end_suppress_warning(struct kunit *test,
> + struct kunit_suppressed_warning *w);
> +int kunit_suppressed_warning_count(struct kunit_suppressed_warning *w);
> +void __kunit_suppress_auto_cleanup(struct kunit_suppressed_warning **wp);
> +bool kunit_has_active_suppress_warning(void);
> +
> +/**
> + * kunit_warning_suppress() - Suppress WARN*() backtraces for the duration
> + * of a block.
> + * @test: The test context object.
> + *
> + * Scoped form of the suppression API. Suppression starts when the block is
> + * entered and ends automatically when the block exits through any path. See
> + * the section comment above for the cleanup guarantees on each exit path.
> + * Fails the test if suppression is already active; nesting is not supported.
> + *
> + * The warning count can be checked inside the block via
> + * KUNIT_EXPECT_SUPPRESSED_WARNING_COUNT(). The handle is not accessible
> + * after the block exits.
> + *
> + * Example::
> + *
> + * kunit_warning_suppress(test) {
> + * trigger_warning();
> + * KUNIT_EXPECT_SUPPRESSED_WARNING_COUNT(test, 1);
> + * }
> + */
> +#define kunit_warning_suppress(test) \
> + for (struct kunit_suppressed_warning *__kunit_suppress \
> + __cleanup(__kunit_suppress_auto_cleanup) = \
> + kunit_start_suppress_warning(test); \
> + __kunit_suppress; \
> + kunit_end_suppress_warning(test, __kunit_suppress), \
> + __kunit_suppress = NULL)
> +
> +/**
> + * KUNIT_SUPPRESSED_WARNING_COUNT() - Returns the suppressed warning count.
> + *
> + * Returns the number of WARN*() calls suppressed since the current
> + * suppression block started, or 0 if the handle is NULL. Usable inside a
> + * kunit_warning_suppress() block.
> + */
> +#define KUNIT_SUPPRESSED_WARNING_COUNT() \
> + kunit_suppressed_warning_count(__kunit_suppress)
> +
> +/**
> + * KUNIT_EXPECT_SUPPRESSED_WARNING_COUNT() - Sets an expectation that the
> + * suppressed warning count equals
> + * @expected.
> + * @test: The test context object.
> + * @expected: an expression that evaluates to the expected warning count.
> + *
> + * Sets an expectation that the number of suppressed WARN*() calls equals
> + * @expected. This is semantically equivalent to
> + * KUNIT_EXPECT_EQ(@test, KUNIT_SUPPRESSED_WARNING_COUNT(), @expected).
> + * See KUNIT_EXPECT_EQ() for more information.
> + */
> +#define KUNIT_EXPECT_SUPPRESSED_WARNING_COUNT(test, expected) \
> + KUNIT_EXPECT_EQ(test, KUNIT_SUPPRESSED_WARNING_COUNT(), expected)
> +
> +/**
> + * KUNIT_ASSERT_SUPPRESSED_WARNING_COUNT() - Sets an assertion that the
> + * suppressed warning count equals
> + * @expected.
> + * @test: The test context object.
> + * @expected: an expression that evaluates to the expected warning count.
> + *
> + * Sets an assertion that the number of suppressed WARN*() calls equals
> + * @expected. This is the same as KUNIT_EXPECT_SUPPRESSED_WARNING_COUNT(),
> + * except it causes an assertion failure (see KUNIT_ASSERT_TRUE()) when the
> + * assertion is not met.
> + */
> +#define KUNIT_ASSERT_SUPPRESSED_WARNING_COUNT(test, expected) \
> + KUNIT_ASSERT_EQ(test, KUNIT_SUPPRESSED_WARNING_COUNT(), expected)
> +
> #endif /* _KUNIT_TEST_H */
> diff --git a/kernel/panic.c b/kernel/panic.c
> index 20feada5319d4..213725b612aa1 100644
> --- a/kernel/panic.c
> +++ b/kernel/panic.c
> @@ -39,6 +39,7 @@
> #include <linux/sys_info.h>
> #include <trace/events/error_report.h>
> #include <asm/sections.h>
> +#include <kunit/test-bug.h>
>
> #define PANIC_TIMER_STEP 100
> #define PANIC_BLINK_SPD 18
> @@ -1124,6 +1125,11 @@ void warn_slowpath_fmt(const char *file, int line, unsigned taint,
> bool rcu = warn_rcu_enter();
> struct warn_args args;
>
> + if (kunit_is_suppressed_warning(true)) {
> + warn_rcu_exit(rcu);
> + return;
> + }
> +
> pr_warn(CUT_HERE);
>
> if (!fmt) {
> @@ -1146,6 +1152,11 @@ void __warn_printk(const char *fmt, ...)
> bool rcu = warn_rcu_enter();
> va_list args;
>
> + if (kunit_is_suppressed_warning(false)) {
> + warn_rcu_exit(rcu);
> + return;
> + }
> +
> pr_warn(CUT_HERE);
>
> va_start(args, fmt);
> diff --git a/lib/bug.c b/lib/bug.c
> index 224f4cfa4aa31..d99e369bc1103 100644
> --- a/lib/bug.c
> +++ b/lib/bug.c
> @@ -48,6 +48,7 @@
> #include <linux/rculist.h>
> #include <linux/ftrace.h>
> #include <linux/context_tracking.h>
> +#include <kunit/test-bug.h>
>
> extern struct bug_entry __start___bug_table[], __stop___bug_table[];
>
> @@ -209,8 +210,6 @@ static enum bug_trap_type __report_bug(struct bug_entry *bug, unsigned long buga
> return BUG_TRAP_TYPE_NONE;
> }
>
> - disable_trace_on_warning();
> -
> bug_get_file_line(bug, &file, &line);
> fmt = bug_get_format(bug);
>
> @@ -220,6 +219,17 @@ static enum bug_trap_type __report_bug(struct bug_entry *bug, unsigned long buga
> no_cut = bug->flags & BUGFLAG_NO_CUT_HERE;
> has_args = bug->flags & BUGFLAG_ARGS;
>
> +#ifdef CONFIG_KUNIT
Sashiko says:
"""
Is the CONFIG_KUNIT check sufficient here?
CONFIG_KUNIT is a tristate configuration option. When KUnit is built as a
module, the preprocessor macro CONFIG_KUNIT_MODULE is defined instead,
leaving CONFIG_KUNIT undefined.
Because lib/bug.c is compiled into the core kernel, this block will be
silently stripped out during a module build. This prevents warning
suppression from working on all architectures that rely on __report_bug().
Could this use IS_ENABLED(CONFIG_KUNIT) instead, or be dropped completely
since include/kunit/test-bug.h provides a safe stub?
"""
Ugh, it is right. I did not consider the module case. Not only is it
safe now as it says, but iirc we added it for performance, however,
since we now have static_branch, it is not really needed. I think
removing it is the right thing to do here.
> + /*
> + * Before the once logic so suppressed warnings do not consume
> + * the single-fire budget of WARN_ON_ONCE().
> + */
> + if (warning && kunit_is_suppressed_warning(true))
> + return BUG_TRAP_TYPE_WARN;
> +#endif
> +
> + disable_trace_on_warning();
> +
> if (warning && once) {
> if (done)
> return BUG_TRAP_TYPE_WARN;
> diff --git a/lib/kunit/Makefile b/lib/kunit/Makefile
> index 656f1fa35abcc..4592f9d0aa8dd 100644
> --- a/lib/kunit/Makefile
> +++ b/lib/kunit/Makefile
> @@ -10,7 +10,8 @@ kunit-objs += test.o \
> executor.o \
> attributes.o \
> device.o \
> - platform.o
> + platform.o \
> + bug.o
>
> ifeq ($(CONFIG_KUNIT_DEBUGFS),y)
> kunit-objs += debugfs.o
> diff --git a/lib/kunit/bug.c b/lib/kunit/bug.c
> new file mode 100644
> index 0000000000000..8579235c9ca68
> --- /dev/null
> +++ b/lib/kunit/bug.c
> @@ -0,0 +1,120 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * KUnit helpers for backtrace suppression
> + *
> + * Copyright (C) 2025 Alessandro Carminati <acarmina@redhat.com>
> + * Copyright (C) 2024 Guenter Roeck <linux@roeck-us.net>
> + */
> +
> +#include <kunit/resource.h>
> +#include <linux/export.h>
> +#include <linux/rculist.h>
> +#include <linux/sched.h>
> +#include <linux/sched/task.h>
> +#include <linux/spinlock.h>
> +
> +#include "hooks-impl.h"
> +
> +struct kunit_suppressed_warning {
> + struct list_head node;
> + struct task_struct *task;
> + struct kunit *test;
> + atomic_t counter;
> +};
> +
> +static LIST_HEAD(suppressed_warnings);
> +static DEFINE_SPINLOCK(suppressed_warnings_lock);
> +
> +static void kunit_suppress_warning_remove(struct kunit_suppressed_warning *w)
> +{
> + unsigned long flags;
> +
> + spin_lock_irqsave(&suppressed_warnings_lock, flags);
> + list_del_rcu(&w->node);
> + spin_unlock_irqrestore(&suppressed_warnings_lock, flags);
> + put_task_struct(w->task);
Sashiko says:
"""
Does this code introduce a use-after-free regression for concurrent RCU
readers?
Because the suppression handle is allocated using kunit_kzalloc() below,
the KUnit framework will automatically free it with a synchronous kfree()
at the end of the test.
Since the handle is unlinked using list_del_rcu() here, but there is no
synchronize_rcu() or kfree_rcu() between the list removal and the memory
free, a concurrent task evaluating warnings under rcu_read_lock() could
dereference the pointer after it has been freed.
Would it be safer to allocate the handle with kzalloc() and explicitly free
it using kfree_rcu() inside this cleanup action?
"""
It is taking a few iterations to get this right...
In the previous version we ruled out synchronize_rcu() because it is a
blocking call that can deadlock if exited while holding the RCU lock.
On the other hand, the suggested kfree_rcu(), only frees memory, but
we also need to release the task reference in the w struct after the
grace period. Reading
`Documentation/RCU/Design/Memory-Ordering/Tree-RCU-Memory-Ordering.rst`,
a solution could be to hold an `rcu_head` in the suppressed warning
struct and invoke call_rcu directly (and explicitly free as suggested
by Sashiko, so I'd need to change kunit_kzalloc() too).
I hope that clears all races.
> +}
> +
> +KUNIT_DEFINE_ACTION_WRAPPER(kunit_suppress_warning_cleanup,
> + kunit_suppress_warning_remove,
> + struct kunit_suppressed_warning *);
> +
> +bool kunit_has_active_suppress_warning(void)
> +{
> + return __kunit_is_suppressed_warning_impl(false);
> +}
> +EXPORT_SYMBOL_GPL(kunit_has_active_suppress_warning);
> +
> +struct kunit_suppressed_warning *
> +kunit_start_suppress_warning(struct kunit *test)
> +{
> + struct kunit_suppressed_warning *w;
> + unsigned long flags;
> + int ret;
> +
> + if (kunit_has_active_suppress_warning()) {
> + KUNIT_FAIL(test, "Another suppression block is already active");
> + return NULL;
> + }
> +
> + w = kunit_kzalloc(test, sizeof(*w), GFP_KERNEL);
> + if (!w) {
> + KUNIT_FAIL(test, "Failed to allocate suppression handle.");
> + return NULL;
> + }
> +
> + w->task = get_task_struct(current);
> + w->test = test;
> +
> + spin_lock_irqsave(&suppressed_warnings_lock, flags);
> + list_add_rcu(&w->node, &suppressed_warnings);
> + spin_unlock_irqrestore(&suppressed_warnings_lock, flags);
> +
> + ret = kunit_add_action_or_reset(test,
> + kunit_suppress_warning_cleanup, w);
> + if (ret) {
> + KUNIT_FAIL(test, "Failed to add suppression cleanup action.");
> + return NULL;
> + }
> +
> + return w;
> +}
> +EXPORT_SYMBOL_GPL(kunit_start_suppress_warning);
> +
> +void kunit_end_suppress_warning(struct kunit *test,
> + struct kunit_suppressed_warning *w)
> +{
> + if (!w)
> + return;
> + kunit_release_action(test, kunit_suppress_warning_cleanup, w);
> +}
> +EXPORT_SYMBOL_GPL(kunit_end_suppress_warning);
> +
> +void __kunit_suppress_auto_cleanup(struct kunit_suppressed_warning **wp)
> +{
> + if (*wp)
> + kunit_end_suppress_warning((*wp)->test, *wp);
> +}
> +EXPORT_SYMBOL_GPL(__kunit_suppress_auto_cleanup);
> +
> +int kunit_suppressed_warning_count(struct kunit_suppressed_warning *w)
> +{
> + return w ? atomic_read(&w->counter) : 0;
> +}
> +EXPORT_SYMBOL_GPL(kunit_suppressed_warning_count);
> +
> +bool __kunit_is_suppressed_warning_impl(bool count)
> +{
> + struct kunit_suppressed_warning *w;
> +
> + guard(rcu)();
> + list_for_each_entry_rcu(w, &suppressed_warnings, node) {
> + if (w->task == current) {
> + if (count)
> + atomic_inc(&w->counter);
> + return true;
> + }
> + }
> +
> + return false;
> +}
> diff --git a/lib/kunit/hooks-impl.h b/lib/kunit/hooks-impl.h
> index 4e71b2d0143ba..d8720f2616925 100644
> --- a/lib/kunit/hooks-impl.h
> +++ b/lib/kunit/hooks-impl.h
> @@ -19,6 +19,7 @@ void __printf(3, 4) __kunit_fail_current_test_impl(const char *file,
> int line,
> const char *fmt, ...);
> void *__kunit_get_static_stub_address_impl(struct kunit *test, void *real_fn_addr);
> +bool __kunit_is_suppressed_warning_impl(bool count);
>
> /* Code to set all of the function pointers. */
> static inline void kunit_install_hooks(void)
> @@ -26,6 +27,7 @@ static inline void kunit_install_hooks(void)
> /* Install the KUnit hook functions. */
> kunit_hooks.fail_current_test = __kunit_fail_current_test_impl;
> kunit_hooks.get_static_stub_address = __kunit_get_static_stub_address_impl;
> + kunit_hooks.is_suppressed_warning = __kunit_is_suppressed_warning_impl;
> }
>
> #endif /* _KUNIT_HOOKS_IMPL_H */
>
> --
> 2.53.0
>
^ permalink raw reply
* Re: [PATCH] driver core: Add cmdline option to force probe type
From: Jianlin Lv @ 2026-05-15 7:54 UTC (permalink / raw)
To: Greg KH
Cc: corbet, skhan, rafael, dakr, jianlv, linux-kernel, linux-doc,
driver-core
In-Reply-To: <2026051532-pettiness-gave-1127@gregkh>
On Fri, May 15, 2026 at 2:13 PM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Thu, May 14, 2026 at 11:09:03PM +0800, Jianlin Lv wrote:
> > On Thu, May 14, 2026 at 9:49 PM Greg KH <gregkh@linuxfoundation.org> wrote:
> > >
> > > On Thu, May 14, 2026 at 09:35:08PM +0800, Jianlin Lv wrote:
> > > > On Thu, May 14, 2026 at 6:16 PM Greg KH <gregkh@linuxfoundation.org> wrote:
> > > > >
> > > > > On Thu, May 14, 2026 at 05:49:55PM +0800, Jianlin Lv wrote:
> > > > > > From: Jianlin Lv <iecedge@gmail.com>
> > > > > >
> > > > > > Device drivers that use asynchronous probing can cause non-deterministic
> > > > > > device ordering and naming across reboots. A typical example is storage
> > > > > > drivers (like sd/nvme): asynchronous probing can lead to inconsistent disk
> > > > > > logical names after reboot. In scenarios where disk naming consistency is
> > > > > > critical, the probe type should be set to synchronous.
> > > > > >
> > > > > > This patch introduces a driver_probe kernel parameter that overrides any
> > > > > > driver's hard-coded probe type settings and allows runtime control without
> > > > > > requiring kernel recompilation:
> > > > > >
> > > > > > driver_probe=PROBE_TYPE_SYNC,nvme,sd # Force specific drivers sync
> > > > > > driver_probe=PROBE_TYPE_ASYNC,*,usb # Force all async except usb
> > > > > > driver_probe=PROBE_TYPE_SYNC,* # Force all drivers synchronous
> > > > > >
> > > > > > The implementation replaces the limited driver_async_probe parameter with
> > > > > > a more flexible interface that can force either synchronous or asynchronous
> > > > > > probing as needed.
> > > > > >
> > > > > > Signed-off-by: Jianlin Lv <iecedge@gmail.com>
> > > > > > ---
> > > > > > .../admin-guide/kernel-parameters.txt | 27 +++++--
> > > > > > drivers/base/dd.c | 71 ++++++++++++++-----
> > > > > > 2 files changed, 74 insertions(+), 24 deletions(-)
> > > > > >
> > > > > > diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> > > > > > index 4d0f545fb3ec..b43a8bd20356 100644
> > > > > > --- a/Documentation/admin-guide/kernel-parameters.txt
> > > > > > +++ b/Documentation/admin-guide/kernel-parameters.txt
> > > > > > @@ -1377,12 +1377,27 @@ Kernel parameters
> > > > > > it becomes active and is searched during signature
> > > > > > verification.
> > > > > >
> > > > > > - driver_async_probe= [KNL]
> > > > > > - List of driver names to be probed asynchronously. *
> > > > > > - matches with all driver names. If * is specified, the
> > > > > > - rest of the listed driver names are those that will NOT
> > > > > > - match the *.
> > > > > > - Format: <driver_name1>,<driver_name2>...
> > > > >
> > > > > You can not remove an existing user/kernel api, sorry, that is not
> > > > > allowed as you just broke all systems that were relying on this :(
> > > > >
> > > > Could you provide more suggestions on how to improve this patch?
> > >
> > > Not really, sorry, I don't think this is a change that should be done at
> > > all. disk naming is a long-solved issue, to think that you can fix that
> > > by doing sync/async device probing is not understanding both the issues
> > > involved, and how we solved it already :)
> >
> > Do you mean referencing disks via by-path/by-id?
>
> No, use something that does not change, like filesystem labels or
> serial numbers, or something else that is guaranteed unique.
>
> > In our production env
> > they can also be unstable; this is an example I encountered before:
> > https://lore.kernel.org/all/CAFA-uR_jk6jCmf9DTebSVBRwtoLuXuyvf1Biq+OObqRVAOZbBw@mail.gmail.com/
>
> Yes, paths can, and will, change. Don't use them.
>
> Why not use a UUID, that is explicitly what those are designed for.
For cloud deployment, the local volume provisioner detects
and creates PVs for each local disk on the host, and it cleans up
the disks when they are released.
The local volume provisioner is deployed in the cluster as
DaemonSet. For the same SKU, it uses a single, unified configuration
file to initialize local disks. In this scenario, UUIDs are not applicable.
In our case, logical names are used to identify the disk.
>
> > I understand that device naming in the kernel can change at any time. However,
> > Is it necessary to provide an interface that allows users to choose
> > the probe mode themselves?
>
> It's not going to solve your problem, so I wouldn't worry about it. And
> you can't remove it, although I really would like to :)
OK,Is the reason we can’t change the priority of driver_async_probe setting that
the original logic has changed, i.e. it would alter the original behavior?
Regards,
Jianlin
>
> thanks,
>
> greg k-h
^ permalink raw reply
* Re: [PATCH v2 3/3] drivers: add deprecated remarks to strlcat()
From: Geert Uytterhoeven @ 2026-05-15 7:32 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Manuel Ebner, Kees Cook, Jonathan Corbet, Shuah Khan,
Andy Whitcroft, Joe Perches, Dwaipayan Ray, Lukas Bulwahn,
David Laight, Randy Dunlap, Jani Nikula, Heiko Carstens,
open list:DOCUMENTATION PROCESS, open list:DOCUMENTATION,
open list
In-Reply-To: <CAHp75VfhHK9E+W83k+w3RWEMq3-HeXC31cJcKE7OiUY9U-wLcQ@mail.gmail.com>
Hi Andy,
On Fri, 15 May 2026 at 09:31, Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> On Fri, May 15, 2026 at 10:23 AM Geert Uytterhoeven
> <geert@linux-m68k.org> wrote:
> > On Thu, 14 May 2026 at 18:32, Manuel Ebner <manuelebner@mailbox.org> wrote:
> > > add kernel-doc comment to strlcat() function definitions
>
> ...
>
> > > +/**
> > > + * strlcat - Append a string to an existing string
> > > + *
> > > + * @dest: pointer to %NUL-terminated string to append to
> > > + * @src: pointer to %NUL-terminated string to append from
> > > + * @count: Maximum bytes available in @dest
> > > + *
> >
> > Missing "Returns ...".
>
> Documentation says "Return:" as
> - the section (note important colon)
> - the singular (however plural is undocumented and supported)
Trailing "s" is not always plural in English ;-)
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCH v2 3/3] drivers: add deprecated remarks to strlcat()
From: Andy Shevchenko @ 2026-05-15 7:31 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Manuel Ebner, Kees Cook, Jonathan Corbet, Shuah Khan,
Andy Whitcroft, Joe Perches, Dwaipayan Ray, Lukas Bulwahn,
David Laight, Randy Dunlap, Jani Nikula, Heiko Carstens,
open list:DOCUMENTATION PROCESS, open list:DOCUMENTATION,
open list
In-Reply-To: <CAHp75VfhHK9E+W83k+w3RWEMq3-HeXC31cJcKE7OiUY9U-wLcQ@mail.gmail.com>
On Fri, May 15, 2026 at 10:30 AM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Fri, May 15, 2026 at 10:23 AM Geert Uytterhoeven
> <geert@linux-m68k.org> wrote:
> > On Thu, 14 May 2026 at 18:32, Manuel Ebner <manuelebner@mailbox.org> wrote:
> > > add kernel-doc comment to strlcat() function definitions
...
> > > +/**
> > > + * strlcat - Append a string to an existing string
> > > + *
> > > + * @dest: pointer to %NUL-terminated string to append to
> > > + * @src: pointer to %NUL-terminated string to append from
> > > + * @count: Maximum bytes available in @dest
> > > + *
> >
> > Missing "Returns ...".
>
> Documentation says "Return:" as
I mean the kernel-doc documentation. Here the section is missed and
needs to be added, indeed.
> - the section (note important colon)
> - the singular (however plural is undocumented and supported)
>
> > > + * Do not use this function. Prefer building the string with
> > > + * formatting, via scnprintf(), seq_buf, or similar.
> > > + *
> > > + */
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v2 3/3] drivers: add deprecated remarks to strlcat()
From: Andy Shevchenko @ 2026-05-15 7:30 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Manuel Ebner, Kees Cook, Jonathan Corbet, Shuah Khan,
Andy Whitcroft, Joe Perches, Dwaipayan Ray, Lukas Bulwahn,
David Laight, Randy Dunlap, Jani Nikula, Heiko Carstens,
open list:DOCUMENTATION PROCESS, open list:DOCUMENTATION,
open list
In-Reply-To: <CAMuHMdXFBFbb+3CqaJGRLqUubRm0pt-yYSds0fitm_wv07kYxw@mail.gmail.com>
On Fri, May 15, 2026 at 10:23 AM Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> On Thu, 14 May 2026 at 18:32, Manuel Ebner <manuelebner@mailbox.org> wrote:
> > add kernel-doc comment to strlcat() function definitions
...
> > +/**
> > + * strlcat - Append a string to an existing string
> > + *
> > + * @dest: pointer to %NUL-terminated string to append to
> > + * @src: pointer to %NUL-terminated string to append from
> > + * @count: Maximum bytes available in @dest
> > + *
>
> Missing "Returns ...".
Documentation says "Return:" as
- the section (note important colon)
- the singular (however plural is undocumented and supported)
> > + * Do not use this function. Prefer building the string with
> > + * formatting, via scnprintf(), seq_buf, or similar.
> > + *
> > + */
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v2 3/3] drivers: add deprecated remarks to strlcat()
From: Geert Uytterhoeven @ 2026-05-15 7:22 UTC (permalink / raw)
To: Manuel Ebner
Cc: Andy Shevchenko, Kees Cook, Jonathan Corbet, Shuah Khan,
Andy Whitcroft, Joe Perches, Dwaipayan Ray, Lukas Bulwahn,
David Laight, Randy Dunlap, Jani Nikula, Heiko Carstens,
open list:DOCUMENTATION PROCESS, open list:DOCUMENTATION,
open list
In-Reply-To: <20260514163033.108009-2-manuelebner@mailbox.org>
Hi Manuel,
On Thu, 14 May 2026 at 18:32, Manuel Ebner <manuelebner@mailbox.org> wrote:
> add kernel-doc comment to strlcat() function definitions
>
> Signed-off-by: Manuel Ebner <manuelebner@mailbox.org>
Thanks for your patch!
> --- a/lib/string.c
> +++ b/lib/string.c
> @@ -249,6 +249,17 @@ EXPORT_SYMBOL(strncat);
> #endif
>
> #ifndef __HAVE_ARCH_STRLCAT
> +/**
> + * strlcat - Append a string to an existing string
> + *
> + * @dest: pointer to %NUL-terminated string to append to
> + * @src: pointer to %NUL-terminated string to append from
> + * @count: Maximum bytes available in @dest
> + *
Missing "Returns ...".
> + * Do not use this function. Prefer building the string with
> + * formatting, via scnprintf(), seq_buf, or similar.
> + *
> + */
> size_t strlcat(char *dest, const char *src, size_t count)
> {
> size_t dsize = strlen(dest);
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCH v7 2/6] mm/memory-failure: surface unhandlable kernel pages as -ENOTRECOVERABLE
From: Lance Yang @ 2026-05-15 7:03 UTC (permalink / raw)
To: leitao
Cc: linmiaohe, akpm, david, ljs, vbabka, rppt, surenb, mhocko, shuah,
nao.horiguchi, rostedt, mhiramat, mathieu.desnoyers, corbet,
skhan, liam, linux-mm, linux-kernel, linux-doc, linux-kselftest,
linux-trace-kernel, kernel-team, Lance Yang
In-Reply-To: <agXcPleVC9LGVCmj@gmail.com>
On Thu, May 14, 2026 at 07:37:14AM -0700, Breno Leitao wrote:
>On Thu, May 14, 2026 at 09:28:30PM +0800, Lance Yang wrote:
>>
>> On Wed, May 13, 2026 at 08:39:33AM -0700, Breno Leitao wrote:
>> >get_any_page() collapses three different failure modes into a single
>> >-EIO return:
>> >
>> > * the put_page race in the !count_increased path;
>> > * the HWPoisonHandlable() rejection that bounces out of
>> > __get_hwpoison_page() with -EBUSY and exhausts shake_page() retries;
>> > * the HWPoisonHandlable() rejection that goes through the
>> > count_increased / put_page / shake_page retry loop.
>> >
>> >The first is transient (the page is racing with the allocator). The
>> >second can be either transient (a userspace folio briefly off LRU
>> >during migration/compaction) or stable (slab/vmalloc/page-table/
>> >kernel-stack pages). The third describes a stable kernel-owned page
>> >that the count_increased=true caller already held a reference on.
>> >
>> >Distinguish them on the return path: keep -EIO for both the put_page
>> >race and the -EBUSY-after-retries branch (shake_page() cannot drag a
>> >folio back from active migration, so we cannot prove the page is
>> >permanently kernel-owned from there), keep -EBUSY for the allocation
>> >race (unchanged), and return -ENOTRECOVERABLE only from the
>> >count_increased-true HWPoisonHandlable() rejection that exhausts its
>> >retries -- the caller's reference is structural evidence that the
>> >page is owned by the kernel.
>> >
>> >Extend the unhandlable-page pr_err() to fire for either errno and
>> >update the get_hwpoison_page() kerneldoc.
>> >
>> >memory_failure() still folds every negative return into
>> >MF_MSG_GET_HWPOISON via its existing "else if (res < 0)" branch, so
>> >this patch is a no-op for users of memory_failure() and only changes
>> >the errno that soft_offline_page() can propagate to its callers. A
>> >follow-up wires the new return code through memory_failure() and
>> >reports MF_MSG_KERNEL for the unrecoverable cases.
>> >
>> >Suggested-by: David Hildenbrand <david@kernel.org>
>> >Signed-off-by: Breno Leitao <leitao@debian.org>
>> >---
>> > mm/memory-failure.c | 18 +++++++++++++++---
>> > 1 file changed, 15 insertions(+), 3 deletions(-)
>> >
>> >diff --git a/mm/memory-failure.c b/mm/memory-failure.c
>> >index 49bcfbd04d213..bae883df3ccb2 100644
>> >--- a/mm/memory-failure.c
>> >+++ b/mm/memory-failure.c
>> >@@ -1408,6 +1408,15 @@ static int get_any_page(struct page *p, unsigned long flags)
>> > shake_page(p);
>> > goto try_again;
>> > }
>> >+ /*
>> >+ * Return -EIO rather than -ENOTRECOVERABLE: this
>> >+ * branch is also reached for pages that are merely
>> >+ * off-LRU transiently (e.g. a folio in the middle
>> >+ * of migration or compaction), which shake_page()
>> >+ * cannot drag back. The caller cannot prove the
>> >+ * page is permanently kernel-owned from here, so
>> >+ * keep it on the recoverable errno.
>> >+ */
>> > ret = -EIO;
>> > goto out;
>> > }
>> >@@ -1427,10 +1436,10 @@ static int get_any_page(struct page *p, unsigned long flags)
>> > goto try_again;
>> > }
>> > put_page(p);
>> >- ret = -EIO;
>> >+ ret = -ENOTRECOVERABLE;
>> > }
>> > out:
>> >- if (ret == -EIO)
>> >+ if (ret == -EIO || ret == -ENOTRECOVERABLE)
>> > pr_err("%#lx: unhandlable page.\n", page_to_pfn(p));
>> >
>> > return ret;
>> >@@ -1487,7 +1496,10 @@ static int __get_unpoison_page(struct page *page)
>> > * -EIO for pages on which we can not handle memory errors,
>> > * -EBUSY when get_hwpoison_page() has raced with page lifecycle
>> > * operations like allocation and free,
>> >- * -EHWPOISON when the page is hwpoisoned and taken off from buddy.
>> >+ * -EHWPOISON when the page is hwpoisoned and taken off from buddy,
>> >+ * -ENOTRECOVERABLE for stable kernel-owned pages the handler
>> >+ * cannot recover (PG_reserved, slab, vmalloc, page tables,
>> >+ * kernel stacks, and similar non-LRU/non-buddy pages).
>>
>> Did you test this patch series? I don't see how we ever get to
>> -ENOTRECOVERABLE there ...
>
>Yes, I did. I am using the following test case:
Okay.
>https://github.com/leitao/linux/commit/cfebe84ddeab5ac34ed456331db980d57e7025dc
>
> # RUN_DESTRUCTIVE=1 tools/testing/selftests/mm/hwpoison-panic.sh
> # enabling /proc/sys/vm/panic_on_unrecoverable_memory_failure
> # injecting hwpoison at phys 0x2a00000 (Kernel rodata)
> # expecting kernel panic: 'Memory failure: <pfn>: unrecoverable page'
> [ 501.113256] Memory failure: 0x2a00: recovery action for reserved kernel page: Ignored
> [ 501.113956] Kernel panic - not syncing: Memory failure: 0x2a00: unrecoverable page
>
>
>> Even with MF_COUNT_INCREASED, the first pass does:
>>
>> if (flags & MF_COUNT_INCREASED)
>> count_increased = true;
>>
>> [...]
>>
>> if (PageHuge(p) || HWPoisonHandlable(p, flags)) {
>> ret = 1;
>> } else {
>> if (pass++ < GET_PAGE_MAX_RETRY_NUM) { <-
>> put_page(p);
>> shake_page(p);
>> count_increased = false;
>> goto try_again; <-
>> }
>> put_page(p);
>> ret = -ENOTRECOVERABLE;
>> }
>>
>> Then we come back with count_increased=false:
>>
>> try_again:
>> if (!count_increased) {
>> ret = __get_hwpoison_page(p, flags); <-
>> if (!ret) {
>> [...]
>> } else if (ret == -EBUSY) { <-
>> [...]
>> ret = -EIO;
>> goto out; <-
>> }
>> }
>>
>> For slab/vmalloc/page-table pages, __get_hwpoison_page() returns -EBUSY:
>>
>> if (!HWPoisonHandlable(&folio->page, flags))
>> return -EBUSY;
>>
>> so they still seem to end up as -EIO ... Am I missing something?
>
>You are not, and thanks for catching this. I traced it again and the
>-ENOTRECOVERABLE branch is unreachable for slab/vmalloc/page-table pages
>exactly as you described. The __get_hwpoison_page() → -EBUSY → shake → retry
>loop catches them first and they exit as -EIO.
Wonder if it would be simpler to just do a positive check near the top
of get_any_page() instead. Something like:
static bool hwpoison_unrecoverable_kernel_page(struct page *page,
unsigned long flags)
{
if ((flags & MF_SOFT_OFFLINE) && page_has_movable_ops(page))
return false;
return PageReserved(page) || PageSlab(page) ||
PageTable(page) || PageLargeKmalloc(page);
}
static int get_any_page(struct page *p, unsigned long flags)
{
int ret = 0, pass = 0;
bool count_increased = false;
if (flags & MF_COUNT_INCREASED)
count_increased = true;
if (hwpoison_unrecoverable_kernel_page(p, flags)) {
if (count_increased)
put_page(p);
ret = -ENOTRECOVERABLE;
goto out;
}
[...]
}
Then get_any_page() could return -ENOTRECOVERABLE only for page types we
can positively identify as kernel-owned.
These types always fail HWPoisonHandlable(), so retrying does not really
buy us anything for them.
Won't cover everything (vmalloc, kernel stacks, etc. have no page_type
to key off), but that's fine - best effort, right?
Cheers, Lance
>
>The selftest I am using (link above) only validated the PageReserved
>short-circuit added in patch 3, which lives in memory_failure() and never
>reaches get_any_page().
>
>I even thought about this code path, and I was not convinced we should return
>-ENOTRECOVERABLE, thus I documented the following (as in this current patch)
>
> @@ -1408,6 +1408,15 @@ static int get_any_page(struct page *p, unsigned long flags)
> shake_page(p);
> goto try_again;
> }
> + /*
> + * Return -EIO rather than -ENOTRECOVERABLE: this
> + * branch is also reached for pages that are merely
> + * off-LRU transiently (e.g. a folio in the middle
> + * of migration or compaction), which shake_page()
> + * cannot drag back. The caller cannot prove the
> + * page is permanently kernel-owned from here, so
> + * keep it on the recoverable errno.
> + */
> ret = -EIO;
>
^ permalink raw reply
* Re: [PATCH v2 3/3] drivers: add deprecated remarks to strlcat()
From: Andy Shevchenko @ 2026-05-15 6:59 UTC (permalink / raw)
To: Manuel Ebner
Cc: Kees Cook, Jonathan Corbet, Shuah Khan, Andy Whitcroft,
Joe Perches, Dwaipayan Ray, Lukas Bulwahn, Geert Uytterhoeven,
David Laight, Randy Dunlap, Jani Nikula, Heiko Carstens,
open list:DOCUMENTATION PROCESS, open list:DOCUMENTATION,
open list
In-Reply-To: <20260514163033.108009-2-manuelebner@mailbox.org>
On Thu, May 14, 2026 at 7:32 PM Manuel Ebner <manuelebner@mailbox.org> wrote:
>
> add kernel-doc comment to strlcat() function definitions
Add
definitions.
...
> --- a/tools/include/nolibc/string.h
> +++ b/tools/include/nolibc/string.h
I probably missed a discussion, but nolibc is not a kernel binary,
they can choose themselves what to use, no?
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v4 1/2] dt-bindings: trivial-devices: Add Murata D1U74T PSU
From: Krzysztof Kozlowski @ 2026-05-15 6:43 UTC (permalink / raw)
To: Abdurrahman Hussain
Cc: Guenter Roeck, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Jonathan Corbet, Shuah Khan, linux-hwmon, devicetree,
linux-kernel, linux-doc
In-Reply-To: <20260514-d1u74t-v4-1-1f1ee7b002ec@nexthop.ai>
On Thu, May 14, 2026 at 08:03:25PM -0700, Abdurrahman Hussain wrote:
> The Murata D1U74T-W series are hot-pluggable 1U AC/DC front-end
> power supplies in the Intel CRPS-185 / OCP M-CRPS form factor.
> Each variant delivers a 12 V main output plus a 12 V standby output
> from a wide AC input (90-264 Vac) or HVDC supply, and includes an
> internal variable-speed cooling fan and on-board voltage, current,
> power, fan-speed, and temperature telemetry.
>
> The host-side digital interface is a PMBus 1.2 port on I2C. The
> PSU's other electrical signals (status, alert, current-share) live
> on the CRPS edge connector and are consumed by the chassis
> controller rather than the host SoC, so there are no host-described
> supplies, GPIOs, clocks, or interrupts. Add the compatible to
> trivial-devices.yaml rather than carrying a standalone binding file.
>
> Signed-off-by: Abdurrahman Hussain <abdurrahman@nexthop.ai>
Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH] driver core: Add cmdline option to force probe type
From: Greg KH @ 2026-05-15 6:13 UTC (permalink / raw)
To: Jianlin Lv
Cc: corbet, skhan, rafael, dakr, jianlv, linux-kernel, linux-doc,
driver-core
In-Reply-To: <CAFA-uR9=rHPbiVFYgDBAdRHCoujKjqtTxsMVupEwGhGmXoLw8w@mail.gmail.com>
On Thu, May 14, 2026 at 11:09:03PM +0800, Jianlin Lv wrote:
> On Thu, May 14, 2026 at 9:49 PM Greg KH <gregkh@linuxfoundation.org> wrote:
> >
> > On Thu, May 14, 2026 at 09:35:08PM +0800, Jianlin Lv wrote:
> > > On Thu, May 14, 2026 at 6:16 PM Greg KH <gregkh@linuxfoundation.org> wrote:
> > > >
> > > > On Thu, May 14, 2026 at 05:49:55PM +0800, Jianlin Lv wrote:
> > > > > From: Jianlin Lv <iecedge@gmail.com>
> > > > >
> > > > > Device drivers that use asynchronous probing can cause non-deterministic
> > > > > device ordering and naming across reboots. A typical example is storage
> > > > > drivers (like sd/nvme): asynchronous probing can lead to inconsistent disk
> > > > > logical names after reboot. In scenarios where disk naming consistency is
> > > > > critical, the probe type should be set to synchronous.
> > > > >
> > > > > This patch introduces a driver_probe kernel parameter that overrides any
> > > > > driver's hard-coded probe type settings and allows runtime control without
> > > > > requiring kernel recompilation:
> > > > >
> > > > > driver_probe=PROBE_TYPE_SYNC,nvme,sd # Force specific drivers sync
> > > > > driver_probe=PROBE_TYPE_ASYNC,*,usb # Force all async except usb
> > > > > driver_probe=PROBE_TYPE_SYNC,* # Force all drivers synchronous
> > > > >
> > > > > The implementation replaces the limited driver_async_probe parameter with
> > > > > a more flexible interface that can force either synchronous or asynchronous
> > > > > probing as needed.
> > > > >
> > > > > Signed-off-by: Jianlin Lv <iecedge@gmail.com>
> > > > > ---
> > > > > .../admin-guide/kernel-parameters.txt | 27 +++++--
> > > > > drivers/base/dd.c | 71 ++++++++++++++-----
> > > > > 2 files changed, 74 insertions(+), 24 deletions(-)
> > > > >
> > > > > diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> > > > > index 4d0f545fb3ec..b43a8bd20356 100644
> > > > > --- a/Documentation/admin-guide/kernel-parameters.txt
> > > > > +++ b/Documentation/admin-guide/kernel-parameters.txt
> > > > > @@ -1377,12 +1377,27 @@ Kernel parameters
> > > > > it becomes active and is searched during signature
> > > > > verification.
> > > > >
> > > > > - driver_async_probe= [KNL]
> > > > > - List of driver names to be probed asynchronously. *
> > > > > - matches with all driver names. If * is specified, the
> > > > > - rest of the listed driver names are those that will NOT
> > > > > - match the *.
> > > > > - Format: <driver_name1>,<driver_name2>...
> > > >
> > > > You can not remove an existing user/kernel api, sorry, that is not
> > > > allowed as you just broke all systems that were relying on this :(
> > > >
> > > Could you provide more suggestions on how to improve this patch?
> >
> > Not really, sorry, I don't think this is a change that should be done at
> > all. disk naming is a long-solved issue, to think that you can fix that
> > by doing sync/async device probing is not understanding both the issues
> > involved, and how we solved it already :)
>
> Do you mean referencing disks via by-path/by-id?
No, use something that does not change, like filesystem labels or
serial numbers, or something else that is guaranteed unique.
> In our production env
> they can also be unstable; this is an example I encountered before:
> https://lore.kernel.org/all/CAFA-uR_jk6jCmf9DTebSVBRwtoLuXuyvf1Biq+OObqRVAOZbBw@mail.gmail.com/
Yes, paths can, and will, change. Don't use them.
Why not use a UUID, that is explicitly what those are designed for.
> I understand that device naming in the kernel can change at any time. However,
> Is it necessary to provide an interface that allows users to choose
> the probe mode themselves?
It's not going to solve your problem, so I wouldn't worry about it. And
you can't remove it, although I really would like to :)
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH v3 2/4] PCI: endpoint: Add DOE mailbox support for endpoint functions
From: Aksh Garg @ 2026-05-15 5:35 UTC (permalink / raw)
To: Manivannan Sadhasivam
Cc: linux-pci, linux-doc, kwilczynski, bhelgaas, corbet, kishon,
skhan, lukas, cassel, alistair, linux-arm-kernel, linux-kernel,
s-vadapalli, danishanwar, srk
In-Reply-To: <hohf2lui4dyu6fzypl7kkwfvgf73ldmvinok7dfukhaornhkqp@n336bwjkvb6f>
On 14/05/26 13:33, Manivannan Sadhasivam wrote:
> On Mon, Apr 27, 2026 at 10:47:23AM +0530, Aksh Garg wrote:
>> DOE (Data Object Exchange) is a standard PCIe extended capability
>> feature introduced in the Data Object Exchange (DOE) ECN for
>> PCIe r5.0. It provides a communication mechanism primarily used for
>> implementing PCIe security features such as device authentication, and
>> secure link establishment. Think of DOE as a sophisticated mailbox
>> system built into PCIe. The root complex can send structured requests
>> to the endpoint device through DOE mailboxes, and the endpoint device
>> responds with appropriate data.
>>
>> Add the DOE support for PCIe endpoint devices, enabling endpoint
>> functions to process the DOE requests from the host. The implementation
>> provides framework APIs for EPC core driver and controller drivers to
>> register mailboxes, and request processing with workqueues ensuring
>> sequential handling per mailbox, and parallel handling across mailboxes.
>> The Discovery protocol is handled internally by the DOE core.
>>
>> This implementation complements the existing DOE implementation for
>> root complex in drivers/pci/doe.c.
>>
>> Co-developed-by: Siddharth Vadapalli <s-vadapalli@ti.com>
>> Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
>> Signed-off-by: Aksh Garg <a-garg7@ti.com>
>> ---
>> +
>> +/*
>> + * Global registry of protocol handlers.
>> + * When a new DOE protocol, library is added, add an entry to this array.
>> + */
>> +static const struct pci_doe_protocol pci_doe_protocols[] = {
>> + {
>> + .vid = PCI_VENDOR_ID_PCI_SIG,
>> + .type = PCI_DOE_FEATURE_DISCOVERY,
>> + .handler = pci_ep_doe_handle_discovery,
>> + },
>> +};
>> +
>> +/*
>> + * Combines function number and capability offset into a unique lookup key
>> + * for storing/retrieving DOE mailboxes in an xarray.
>> + */
>> +#define PCI_DOE_MB_KEY(func, offset) \
>> + (((unsigned long)(func) << 16) | (offset))
>> +#define PCI_DOE_PROTOCOL_COUNT ARRAY_SIZE(pci_doe_protocols)
>> +
>> +/**
>> + * pci_ep_doe_init() - Initialize the DOE framework for a controller in EP mode
>> + * @epc: PCI endpoint controller
>> + *
>> + * Initialize the DOE framework data structures. This only initializes
>> + * the xarray that will hold the mailboxes.
>> + *
>> + * RETURNS: 0 on success, -errno on failure
>
> kernel-doc format to describe return value is 'Return:' or 'Returns:".
Thanks for pointing this out. I will update this.
>
>> + */
>> +int pci_ep_doe_init(struct pci_epc *epc)
>> +{
>> + if (!epc)
>> + return -EINVAL;
>> +
>> + xa_init(&epc->doe_mbs);
>> + return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(pci_ep_doe_init);
>> +
[...]
>> +
>> +/**
>> + * pci_ep_doe_process_request() - Process DOE request on endpoint
>> + * @epc: PCI endpoint controller
>> + * @func_no: Physical function number
>> + * @cap_offset: DOE capability offset
>> + * @vendor: Vendor ID from request header
>> + * @type: Protocol type from request header
>> + * @request: Request payload in CPU-native format
>> + * @request_sz: Size of request payload (bytes)
>> + * @complete: Callback to invoke upon completion
>> + *
>> + * Asynchronously process a DOE request received on the endpoint. The request
>> + * payload should not include the DOE header (vendor/type/length). The protocol
>> + * handler will allocate the response buffer, which the caller (controller driver)
>> + * must free after use.
>> + *
>> + * This function returns immediately after queuing the request. The completion
>> + * callback will be invoked asynchronously from workqueue context once the
>> + * request is processed. The callback receives the function number and capability
>> + * offset to identify the mailbox, along with a status code (0 on success, -errno
>> + * on failure), and other required arguments.
>> + *
>> + * As per DOE specification, a mailbox processes one request at a time.
>> + * Therefore, this function will never be called concurrently for the same
>> + * mailbox by different callers.
>> + *
>> + * The caller is responsible for the conversion of the received DOE request
>> + * with le32_to_cpu() before calling this function.
>> + * Similarly, it is responsible for converting the response payload with
>> + * cpu_to_le32() before sending it back over the DOE mailbox.
>> + *
>> + * The caller is also responsible for ensuring that the request size
>> + * is within the limits defined by PCI_DOE_MAX_LENGTH.
>> + *
>> + * RETURNS: 0 if the request was successfully queued, -errno on failure
>> + */
>> +int pci_ep_doe_process_request(struct pci_epc *epc, u8 func_no, u16 cap_offset,
>> + u16 vendor, u8 type, const void *request, size_t request_sz,
>> + pci_ep_doe_complete_t complete)
>> +{
>> + struct pci_ep_doe_mb *doe_mb;
>> + struct pci_ep_doe_task *task;
>> + int rc;
>> +
>> + doe_mb = pci_ep_doe_get_mailbox(epc, func_no, cap_offset);
>> + if (!doe_mb) {
>> + kfree(request);
>> + return -ENODEV;
>> + }
>> +
>> + task = kzalloc_obj(*task, GFP_KERNEL);
>> + if (!task) {
>> + kfree(request);
>> + return -ENOMEM;
>> + }
>> +
>> + task->feat.vid = vendor;
>> + task->feat.type = type;
>> + task->request_pl = request;
>> + task->request_pl_sz = request_sz;
>> + task->response_pl = NULL;
>> + task->response_pl_sz = 0;
>> + task->complete = complete;
>> +
>> + rc = pci_ep_doe_submit_task(doe_mb, task);
>> + if (rc) {
>> + kfree(request);
>> + kfree(task);
>> + return rc;
>> + }
>> +
>> + return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(pci_ep_doe_process_request);
>
> So who is supposed to call this API? EPC driver that receives the DOE interrupt?
Yes, the EPC drivers that receive the DOE interrupts are expected to
call this API.
> But I don't see the any callers of this and below exported APIs in this series.
> Either you should add the callers or limit this series just to adding the DOE
> skeleton implementation with a clear follow-up.
I currently am working on the EPC driver implementation for a platform
which has not been up-streamed yet. I plan to use these APIs to support
the DOE feature for that driver. Currently, I am not aware of any
platform whose EPC driver supports DOE feature and its interrupts, hence
I see no real callers of these APIs to include in this patch series.
Would it be appropriate to add a dummy [NOT-FOR-MERGING] demonstration
patch over an existing EPC driver, showing how these DOE APIs would be
integrated into an EPC driver?
>
> But since you've limited the scope of this series to support only DOE Discovery
> Data Object Protocol, it'd be good to add the EPC implementation to get the full
> picture.>
> - Mani
>
^ permalink raw reply
* Re: [PATCH v3 3/4] PCI: endpoint: Add API for DOE initialization and setup in EPC core
From: Aksh Garg @ 2026-05-15 4:51 UTC (permalink / raw)
To: Manivannan Sadhasivam
Cc: linux-pci, linux-doc, kwilczynski, bhelgaas, corbet, kishon,
skhan, lukas, cassel, alistair, linux-arm-kernel, linux-kernel,
s-vadapalli, danishanwar, srk
In-Reply-To: <m4z3q3pe3ro5vkl4uq4zkewpjdqccgeact2hj4tjnkonttx4vr@ndan37zzwgxc>
On 14/05/26 13:38, Manivannan Sadhasivam wrote:
> On Mon, Apr 27, 2026 at 10:47:24AM +0530, Aksh Garg wrote:
>> Add pci_epc_setup_doe() API in EPC core driver to initialize and setup
>> the DOE framework for an endpoint controller. The API discovers the DOE
>> capabilities (extended capability ID 0x2E), and registers each discovered
>> DOE mailbox for all the functions in the endpoint controller. This API
>> should be invoked by the controller driver during probe based on the
>> doe_capable feature.
>>
>> Add pci_epc_destroy_doe() API in EPC core driver for cleanup of DOE
>> resources, which should be invoked by the controller driver during
>> controller cleanup based on the doe_capable feature.
>>
>> Co-developed-by: Siddharth Vadapalli <s-vadapalli@ti.com>
>> Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
>> Signed-off-by: Aksh Garg <a-garg7@ti.com>
>> ---
>>
>> Changes from v2 to v3:
>> - Rebased on 7.1-rc1.
>>
>> Changes since v1:
>> - New patch added to v2 (not present in v1)
>>
>> v2: https://lore.kernel.org/all/20260401073022.215805-4-a-garg7@ti.com/
>>
>> This patch is introduced based on the feedback provided by Manivannan
>> Sadhasivam at [1].
>>
>
> Sweet! But I was expecting you to add atleast one EPC driver implementation to
> make use of these APIs.
>
> Also, why can't you call these APIs from the EPC core directly? Maybe during
> pci_epc_init_notify() once the register accesses become valid.
Can we add the DOE initialization API to pci_epc_init_notify()? This
API seems to be called to notify the EPF drivers that the EPC device's
initialization has been completed, as the name and description suggests.
As 'pci_epc_doe_setup' is a part of EPC initialization, I thought the
EPC drivers should call this API before calling the pci_epc_init_notify().
However, I agree with your suggestion to call the DOE setup API directly
from the EPC core instead of sprinkling over the EPC drivers. I would
recommend renaming the pci_epc_init_notify() API (and hence the
pci_epc_deinit_notify() as well) to something like
pci_epc_init_complete(), and add the DOE setup API/logic just before the
logic of notifying the EPF devices.
Please suggest if the above would be acceptable.
Regards,
Aksh Garg
>
> - Mani
>
>> [1]: https://lore.kernel.org/all/p57x6jleaim5w7t2k3v7tioujnaxuovfpj5euop5ogefvw23se@y5fw3che5p5d/
>>
>> drivers/pci/endpoint/pci-epc-core.c | 71 +++++++++++++++++++++++++++++
>> include/linux/pci-epc.h | 21 +++++++++
>> 2 files changed, 92 insertions(+)
>>
>> diff --git a/drivers/pci/endpoint/pci-epc-core.c b/drivers/pci/endpoint/pci-epc-core.c
>> index 6c3c58185fc5..5a95a07b7d3a 100644
>> --- a/drivers/pci/endpoint/pci-epc-core.c
>> +++ b/drivers/pci/endpoint/pci-epc-core.c
>> @@ -14,6 +14,8 @@
>> #include <linux/pci-epf.h>
>> #include <linux/pci-ep-cfs.h>
>>
>> +#include "../pci.h"
>> +
>> static const struct class pci_epc_class = {
>> .name = "pci_epc",
>> };
>> @@ -548,6 +550,75 @@ void pci_epc_mem_unmap(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
>> }
>> EXPORT_SYMBOL_GPL(pci_epc_mem_unmap);
>>
>> +/**
>> + * pci_epc_doe_setup() - Setup and discover DOE mailboxes for all functions
>> + * @epc: the EPC device on which DOE mailboxes has to be setup
>> + *
>> + * Discover DOE (Data Object Exchange) capabilities for all physical functions
>> + * in the endpoint controller and register DOE mailboxes.
>> + *
>> + * This API should be called by the controller driver during initialization
>> + * if DOE support is available (indicated by doe_capable in pci_epc_features).
>> + *
>> + * RETURNS: 0 on success, -errno on failure
>> + */
>> +int pci_epc_doe_setup(struct pci_epc *epc)
>> +{
>> + u16 cap_offset = 0;
>> + u8 func_no;
>> + int ret;
>> +
>> + if (!epc || !epc->ops || !epc->ops->find_ext_capability)
>> + return -EINVAL;
>> +
>> + /* Initialize DOE framework for this controller */
>> + ret = pci_ep_doe_init(epc);
>> + if (ret)
>> + return ret;
>> +
>> + /* Discover DOE capabilities for all functions */
>> + for (func_no = 0; func_no < epc->max_functions; func_no++) {
>> + while ((cap_offset = epc->ops->find_ext_capability(epc, func_no, 0,
>> + cap_offset,
>> + PCI_EXT_CAP_ID_DOE))) {
>> + /* Register this DOE mailbox */
>> + ret = pci_ep_doe_add_mailbox(epc, func_no, cap_offset);
>> + if (ret) {
>> + dev_err(&epc->dev,
>> + "[pf%d:offset %x] failed to add DOE mailbox\n",
>> + func_no, cap_offset);
>> + }
>> + }
>> + }
>> +
>> + dev_dbg(&epc->dev, "DOE mailboxes setup complete\n");
>> + return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(pci_epc_doe_setup);
>> +
>> +/**
>> + * pci_epc_doe_destroy() - Destroy and cleanup DOE mailboxes
>> + * @epc: the EPC device on which DOE mailboxes has to be destroyed
>> + *
>> + * Destroy all DOE mailboxes registered on this endpoint controller and
>> + * free associated resources.
>> + *
>> + * This API should be called by the controller driver during controller cleanup
>> + * if DOE support is available (indicated by doe_capable in pci_epc_features).
>> + *
>> + * RETURNS: 0 on success, -errno on failure
>> + */
>> +int pci_epc_doe_destroy(struct pci_epc *epc)
>> +{
>> + if (!epc)
>> + return -EINVAL;
>> +
>> + pci_ep_doe_destroy(epc);
>> + dev_dbg(&epc->dev, "DOE mailboxes destroyed\n");
>> + return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(pci_epc_doe_destroy);
>> +
>> /**
>> * pci_epc_clear_bar() - reset the BAR
>> * @epc: the EPC device for which the BAR has to be cleared
>> diff --git a/include/linux/pci-epc.h b/include/linux/pci-epc.h
>> index dd26294c8175..7b0f258ef330 100644
>> --- a/include/linux/pci-epc.h
>> +++ b/include/linux/pci-epc.h
>> @@ -84,6 +84,8 @@ struct pci_epc_map {
>> * @start: ops to start the PCI link
>> * @stop: ops to stop the PCI link
>> * @get_features: ops to get the features supported by the EPC
>> + * @find_ext_capability: ops to find extended capability offset for a function
>> + * in endpoint controller
>> * @owner: the module owner containing the ops
>> */
>> struct pci_epc_ops {
>> @@ -115,6 +117,8 @@ struct pci_epc_ops {
>> void (*stop)(struct pci_epc *epc);
>> const struct pci_epc_features* (*get_features)(struct pci_epc *epc,
>> u8 func_no, u8 vfunc_no);
>> + u16 (*find_ext_capability)(struct pci_epc *epc, u8 func_no,
>> + u8 vfunc_no, u16 start, u8 cap);
>> struct module *owner;
>> };
>>
>> @@ -270,6 +274,7 @@ struct pci_epc_bar_desc {
>> * @msi_capable: indicate if the endpoint function has MSI capability
>> * @msix_capable: indicate if the endpoint function has MSI-X capability
>> * @intx_capable: indicate if the endpoint can raise INTx interrupts
>> + * @doe_capable: indicate if the endpoint function has DOE capability
>> * @bar: array specifying the hardware description for each BAR
>> * @align: alignment size required for BAR buffer allocation
>> */
>> @@ -280,6 +285,7 @@ struct pci_epc_features {
>> unsigned int msi_capable : 1;
>> unsigned int msix_capable : 1;
>> unsigned int intx_capable : 1;
>> + unsigned int doe_capable : 1;
>> struct pci_epc_bar_desc bar[PCI_STD_NUM_BARS];
>> size_t align;
>> };
>> @@ -368,6 +374,21 @@ int pci_epc_mem_map(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
>> void pci_epc_mem_unmap(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
>> struct pci_epc_map *map);
>>
>> +#ifdef CONFIG_PCI_ENDPOINT_DOE
>> +int pci_epc_doe_setup(struct pci_epc *epc);
>> +int pci_epc_doe_destroy(struct pci_epc *epc);
>> +#else
>> +static inline int pci_epc_doe_setup(struct pci_epc *epc)
>> +{
>> + return -EOPNOTSUPP;
>> +}
>> +
>> +static inline int pci_epc_doe_destroy(struct pci_epc *epc)
>> +{
>> + return -EOPNOTSUPP;
>> +}
>> +#endif
>> +
>> #else
>> static inline void pci_epc_init_notify(struct pci_epc *epc)
>> {
>> --
>> 2.34.1
>>
>
^ permalink raw reply
* Re: [PATCH RESEND bpf-next v10 2/8] bpf: clear list node owner and unlink before drop
From: Kaitao Cheng @ 2026-05-15 4:34 UTC (permalink / raw)
To: Alexei Starovoitov, Eduard Zingerman
Cc: bpf, linux-kernel, linux-doc, ast, memxor, corbet, martin.lau,
daniel, andrii, song, yonghong.song, john.fastabend, kpsingh, sdf,
haoluo, jolsa, shuah, chengkaitao, skhan, vmalik, linux-kselftest,
martin.lau, clm, ihor.solodrai, bot+bpf-ci
In-Reply-To: <DII0TT9LXYCX.2GMM6QA4Q9BPZ@gmail.com>
在 2026/5/14 09:50, Alexei Starovoitov 写道:
> On Wed May 13, 2026 at 3:53 PM PDT, Eduard Zingerman wrote:
>> On Tue, 2026-05-12 at 06:41 +0000, bot+bpf-ci@kernel.org wrote:
>>
>> [...]
>>
>>> When a BPF program holds an owning or refcount-acquired reference to
>>> one of these nodes (node X), which is structurally supported because
>>> __bpf_obj_drop_impl() uses refcount_dec_and_test() and only frees at
>>> refcount 0, a concurrent push to a DIFFERENT bpf_list_head becomes a
>>> corruption:
>>>
>>> CPU 0 (bpf_list_head_free, lock released) CPU 1 (BPF prog, refcount X)
>>> ----------------------------------------- ----------------------------
>>> (owner of X == NULL, X linked in drain)
>>> bpf_list_push_back(other, X)
>>> __bpf_list_add: spin_lock()
>>> cmpxchg(X->owner, NULL,
>>> POISON) -> OK
>>> list_add_tail(&X->list_head,
>>> other_head)
>>> -> overwrites X->next,
>>> X->prev, corrupts
>>> other_head's chain
>>> because X is still
>>> stitched into drain
>>> pos = drain.next; (may be X or neighbor using X's stale next)
>>> list_del_init(pos); reads X->next/prev now pointing into other_head,
>>> corrupts other_head's list and/or drain
>>
>>
>> Kaitao, this scenario seem plausible, could you please comment on it?
>
> I think bot is correct.
> This patch looks buggy.
> It seems to me an optimization that breaks the concurrent logic.
> May be just drop this patch and reorder the other one, so that bot
> sees nonown suffix logic first.
This patch is still necessary because it addresses the problem discussed
in this thread:
https://lore.kernel.org/all/DH846C0P88QU.16YT12I1LXBZM@etsalapatis.com/
The patch does have a bug, however. To fix the issues we are seeing now,
I propose the additional changes below and would appreciate feedback.
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -2263,8 +2263,10 @@ void bpf_list_head_free(const struct btf_field *field, void *list_head,
if (!head->next || list_empty(head))
goto unlock;
list_for_each_safe(pos, n, head) {
- WRITE_ONCE(container_of(pos,
- struct bpf_list_node_kern, list_head)->owner, NULL);
+ struct bpf_list_node_kern *node;
+
+ node = container_of(pos, struct bpf_list_node_kern, list_head);
+ WRITE_ONCE(node->owner, BPF_PTR_POISON);
list_move_tail(pos, &drain);
}
unlock:
@@ -2272,8 +2274,12 @@ void bpf_list_head_free(const struct btf_field *field, void *list_head,
__bpf_spin_unlock_irqrestore(spin_lock);
while (!list_empty(&drain)) {
+ struct bpf_list_node_kern *node;
+
pos = drain.next;
+ node = container_of(pos, struct bpf_list_node_kern, list_head);
list_del_init(pos);
+ WRITE_ONCE(node->owner, NULL);
/* The contained type can also have resources, including a
* bpf_list_head which needs to be freed.
*/
@@ -2481,6 +2487,14 @@ static int __bpf_list_add(struct bpf_list_node_kern *node,
if (unlikely(!h->next))
INIT_LIST_HEAD(h);
+ /* bpf_list_head_free() marks nodes being detached with BPF_PTR_POISON
+ * before list_del_init(). cmpxchg(NULL, POISON) below would fail with
+ * that old value and fall into the generic error path, which wrongly
+ * calls __bpf_obj_drop_impl(). Reject POISON up front instead.
+ */
+ if (READ_ONCE(node->owner) == BPF_PTR_POISON)
+ return -EINVAL;
+
/* node->owner != NULL implies !list_empty(n), no need to separately
* check the latter
*/
--
Thanks
Kaitao Cheng
^ permalink raw reply
* Re: [PATCH] killswitch: add per-function short-circuit mitigation primitive
From: Paul Moore @ 2026-05-15 3:48 UTC (permalink / raw)
To: Sasha Levin
Cc: corbet, akpm, skhan, linux-doc, linux-kernel, linux-kselftest,
gregkh, linux-security-module
In-Reply-To: <20260507070547.2268452-1-sashal@kernel.org>
On Thu, May 7, 2026 at 3:05 AM Sasha Levin <sashal@kernel.org> wrote:
>
> When a (security) issue goes public, fleets stay exposed until a patched kernel
> is built, distributed, and rebooted into.
>
> For many such issues the simplest mitigation is to stop calling the buggy
> function. Killswitch provides that. An admin writes:
>
> echo "engage af_alg_sendmsg -1" \
> > /sys/kernel/security/killswitch/control
>
> After this, af_alg_sendmsg() returns -EPERM on every call without
> running its body. The mitigation takes effect immediately, and is dropped on
> the next reboot.
>
> A lot of recent kernel issues sit in code paths most installs only have enabled
> to support a relative minority of users: AF_ALG, ksmbd, nf_tables, vsock, ax25,
> and friends.
>
> For most users, the cost of "this socket family stops working for the day" is
> much smaller than the cost of running a known vulnerable kernel until the fix
> land.
>
> Assisted-by: Claude:claude-opus-4-7
> Signed-off-by: Sasha Levin <sashal@kernel.org>
> ---
> Documentation/admin-guide/index.rst | 1 +
> Documentation/admin-guide/killswitch.rst | 159 ++++
> Documentation/admin-guide/tainted-kernels.rst | 8 +
> MAINTAINERS | 11 +
> include/linux/killswitch.h | 19 +
> include/linux/panic.h | 3 +-
> init/Kconfig | 2 +
> kernel/Kconfig.killswitch | 31 +
> kernel/Makefile | 1 +
> kernel/killswitch.c | 798 ++++++++++++++++++
> kernel/panic.c | 1 +
> lib/Kconfig.debug | 13 +
> lib/Makefile | 1 +
> lib/test_killswitch.c | 85 ++
> tools/testing/selftests/Makefile | 1 +
> tools/testing/selftests/killswitch/.gitignore | 1 +
> tools/testing/selftests/killswitch/Makefile | 8 +
> .../selftests/killswitch/cve_31431_test.c | 162 ++++
> .../selftests/killswitch/killswitch_test.sh | 147 ++++
> 19 files changed, 1451 insertions(+), 1 deletion(-)
> create mode 100644 Documentation/admin-guide/killswitch.rst
> create mode 100644 include/linux/killswitch.h
> create mode 100644 kernel/Kconfig.killswitch
> create mode 100644 kernel/killswitch.c
> create mode 100644 lib/test_killswitch.c
> create mode 100644 tools/testing/selftests/killswitch/.gitignore
> create mode 100644 tools/testing/selftests/killswitch/Makefile
> create mode 100644 tools/testing/selftests/killswitch/cve_31431_test.c
> create mode 100755 tools/testing/selftests/killswitch/killswitch_test.sh
If we made Lockdown an LSM, we should probably also make killswitch an LSM.
For the LSM crowd who might be seeing this for the first time, the
original thread can be found on lore via the link below:
https://lore.kernel.org/all/20260507070547.2268452-1-sashal@kernel.org
--
paul-moore.com
^ permalink raw reply
* [PATCH v3 4/4] HID: hid-msi: Add Rumble Intensity Attributes
From: Derek J. Clark @ 2026-05-15 3:36 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: <20260515033622.2095277-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 06dc4290d2bb5..349a59f75e471 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];
@@ -311,7 +318,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 */
@@ -397,6 +407,12 @@ static int claw_profile_event(struct claw_drvdata *drvdata, struct claw_command_
drvdata->led_mc.subled_info[2].intensity = frame->zone_data.zone[0].blue;
}
+ 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,
@@ -810,6 +826,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)
{
@@ -830,6 +966,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;
}
@@ -843,6 +985,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,
};
@@ -1314,6 +1459,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;
@@ -1325,6 +1471,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 v3 3/4] HID: hid-msi: Add RGB control interface
From: Derek J. Clark @ 2026-05-15 3:36 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: <20260515033622.2095277-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>
---
v3:
- Add mutex for read/write of rgb frame data.
- Remove setting rgb_frame_count when reading rgb profiles as it always
returns garbage data.
- Ensure rgb_speed is getting drvdata from a valid lookup (not hdev).
v2:
- Use pending_profile mutex
---
drivers/hid/hid-msi.c | 548 +++++++++++++++++++++++++++++++++++++++++-
1 file changed, 542 insertions(+), 6 deletions(-)
diff --git a/drivers/hid/hid-msi.c b/drivers/hid/hid-msi.c
index 6ccb9666daedf..06dc4290d2bb5 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;
@@ -252,6 +299,7 @@ struct claw_drvdata {
struct delayed_work cfg_resume;
struct delayed_work cfg_setup;
struct mutex profile_mutex; /* mutex for profile_pending calls */
+ struct mutex frame_mutex; /* mutex for read/write rgb_frames */
struct hid_device *hdev;
struct mutex cfg_mutex; /* mutex for synchronous data */
struct mutex rom_mutex; /* mutex for SYNC_TO_ROM calls */
@@ -265,6 +313,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 +354,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 +369,46 @@ 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 > CLAW_RGB_MAX_FRAMES) {
+ 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;
+
+ scoped_guard(mutex, &drvdata->frame_mutex) {
+ memcpy(&drvdata->rgb_frames[f_idx], &frame->zone_data,
+ sizeof(struct rgb_frame));
+
+ /* Only use frame 0 for remaining variable assignment */
+ if (f_idx != 0)
+ break;
+
+ drvdata->rgb_speed = frame->speed;
+ drvdata->led_mc.led_cdev.brightness = frame->brightness;
+ drvdata->led_mc.subled_info[0].intensity = frame->zone_data.zone[0].red;
+ drvdata->led_mc.subled_info[1].intensity = frame->zone_data.zone[0].green;
+ drvdata->led_mc.subled_info[2].intensity = frame->zone_data.zone[0].blue;
+ }
+
+ break;
default:
dev_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 +851,404 @@ 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 };
+
+ guard(mutex)(&drvdata->frame_mutex);
+ 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 };
+
+ guard(mutex)(&drvdata->frame_mutex);
+ 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;
+
+ guard(mutex)(&drvdata->frame_mutex);
+ 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;
+
+ guard(mutex)(&drvdata->frame_mutex);
+ 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;
+
+ guard(mutex)(&drvdata->frame_mutex);
+ 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) {
+ guard(mutex)(&drvdata->frame_mutex);
+ 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 led_classdev *led_cdev = dev_get_drvdata(dev);
+ struct led_classdev_mc *led_mc = container_of(led_cdev, struct led_classdev_mc, led_cdev);
+ struct claw_drvdata *drvdata = container_of(led_mc, struct claw_drvdata, led_mc);
+ u8 speed = 20 - drvdata->rgb_speed;
+
+ 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 claw_rgb_attr_group = {
+ .attrs = claw_rgb_attrs,
+};
+
+static struct mc_subled claw_rgb_subled_info[] = {
+ {
+ .color_index = LED_COLOR_ID_RED,
+ .channel = 0x1,
+ },
+ {
+ .color_index = LED_COLOR_ID_GREEN,
+ .channel = 0x2,
+ },
+ {
+ .color_index = LED_COLOR_ID_BLUE,
+ .channel = 0x3,
+ },
+};
+
static void cfg_setup_fn(struct work_struct *work)
{
struct delayed_work *dwork = container_of(work, struct delayed_work, work);
@@ -772,6 +1262,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 = device_add_group(&drvdata->hdev->dev, &claw_gamepad_attr_group);
if (ret) {
@@ -780,7 +1277,15 @@ static void cfg_setup_fn(struct work_struct *work)
return;
}
+ ret = device_add_group(drvdata->led_mc.led_cdev.dev, &claw_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 +1295,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 +1312,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)
@@ -829,6 +1344,7 @@ static int claw_probe(struct hid_device *hdev, u8 ep)
return -ENOMEM;
drvdata->gamepad_mode = CLAW_GAMEPAD_MODE_XINPUT;
+ drvdata->rgb_enabled = true;
drvdata->hdev = hdev;
drvdata->ep = ep;
@@ -839,12 +1355,28 @@ 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");
+ drvdata->led_mc.led_cdev.name = "msi_claw:rgb:joystick_rings";
+ drvdata->led_mc.led_cdev.brightness = 0x50;
+ drvdata->led_mc.led_cdev.max_brightness = 0x64;
+ drvdata->led_mc.led_cdev.color = LED_COLOR_ID_RGB;
+ drvdata->led_mc.led_cdev.brightness_set = claw_led_brightness_set;
+ drvdata->led_mc.num_colors = 3;
+ drvdata->led_mc.subled_info = devm_kmemdup(&hdev->dev, claw_rgb_subled_info,
+ sizeof(claw_rgb_subled_info), GFP_KERNEL);
+ if (!drvdata->led_mc.subled_info)
+ return -ENOMEM;
+
mutex_init(&drvdata->cfg_mutex);
mutex_init(&drvdata->profile_mutex);
mutex_init(&drvdata->rom_mutex);
init_completion(&drvdata->send_cmd_complete);
INIT_DELAYED_WORK(&drvdata->cfg_resume, &cfg_resume_fn);
INIT_DELAYED_WORK(&drvdata->cfg_setup, &cfg_setup_fn);
+ INIT_DELAYED_WORK(&drvdata->rgb_queue, &claw_rgb_queue_fn);
+
+ 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);
@@ -906,7 +1438,11 @@ 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;
+ device_remove_group(drvdata->led_mc.led_cdev.dev, &claw_rgb_attr_group);
device_remove_group(&hdev->dev, &claw_gamepad_attr_group);
+ 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 v3 2/4] HID: hid-msi: Add M-key mapping attributes
From: Derek J. Clark @ 2026-05-15 3:36 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: <20260515033622.2095277-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>
---
v3:
- Use scoped_guard where necessary.
v2:
- Add mutex for SYNC_TO_ROM commands to ensure every SYNC is completed
before more data is written to the MCU volatile memory.
- Add mutex for profile_pending to ensure every profile action
response is serialized to the generating command.
---
drivers/hid/hid-msi.c | 400 +++++++++++++++++++++++++++++++++++++++++-
1 file changed, 399 insertions(+), 1 deletion(-)
diff --git a/drivers/hid/hid-msi.c b/drivers/hid/hid-msi.c
index 89bb32f00bfc3..6ccb9666daedf 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:
@@ -364,6 +561,163 @@ 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;
+
+ 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:
+ scoped_guard(mutex, &drvdata->rom_mutex) {
+ 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)
{
@@ -376,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,
@@ -430,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;
@@ -443,7 +832,16 @@ static int claw_probe(struct hid_device *hdev, u8 ep)
drvdata->hdev = hdev;
drvdata->ep = ep;
+ /* Determine feature level from firmware version */
+ drvdata->bcd_device = le16_to_cpu(udev->descriptor.bcdDevice);
+ claw_features_supported(drvdata);
+
+ if (!drvdata->bmap_support)
+ dev_dbg(&hdev->dev, "M-Key mapping is not supported. Update firmware to enable.\n");
+
mutex_init(&drvdata->cfg_mutex);
+ mutex_init(&drvdata->profile_mutex);
+ mutex_init(&drvdata->rom_mutex);
init_completion(&drvdata->send_cmd_complete);
INIT_DELAYED_WORK(&drvdata->cfg_resume, &cfg_resume_fn);
INIT_DELAYED_WORK(&drvdata->cfg_setup, &cfg_setup_fn);
--
2.53.0
^ permalink raw reply related
* [PATCH v3 1/4] HID: hid-msi: Add MSI Claw configuration driver
From: Derek J. Clark @ 2026-05-15 3:36 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: <20260515033622.2095277-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>
---
v3:
- Ensure claw_hw_output_report is properly guarded.
- Reoder claw_probe to ensure all mutex, completion, and variable
assignments are in place prior to setting drvdata.
- Ensure gamepad_mode is set to a valid enum value in claw_probe.
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.
---
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..89bb32f00bfc3
--- /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);
+
+ guard(mutex)(&drvdata->cfg_mutex);
+ if (timeout)
+ 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 = 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;
+
+ drvdata->gamepad_mode = CLAW_GAMEPAD_MODE_XINPUT;
+ drvdata->hdev = hdev;
+ drvdata->ep = ep;
+
+ mutex_init(&drvdata->cfg_mutex);
+ init_completion(&drvdata->send_cmd_complete);
+ INIT_DELAYED_WORK(&drvdata->cfg_resume, &cfg_resume_fn);
+ INIT_DELAYED_WORK(&drvdata->cfg_setup, &cfg_setup_fn);
+
+ /* For control interface: open the HID transport for sending commands. */
+ ret = hid_hw_open(hdev);
+ if (ret)
+ return ret;
+
+ hid_set_drvdata(hdev, drvdata);
+ 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;
+ }
+
+ device_remove_group(&hdev->dev, &claw_gamepad_attr_group);
+ 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 v3 0/4] Add MSI Claw HID Configuration Driver
From: Derek J. Clark @ 2026-05-15 3:36 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>
---
v3:
- Add mutex for read/write if rgb frame data.
- Ensure claw_hw_output_report is properly guarded.
- Remove setting rgb_frame_count when reading rgb profiles as it always
returns garbage data.
- Ensure rgb_speed is getting drvdata from a valid lookup (not hdev).
- Use scoped_guard where necessary.
- Reoder claw_probe to ensure all mutex, completion, and variable
assignments are in place prior to setting drvdata.
- Ensure gamepad_mode is set to a valid enum value in claw_probe.
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 | 1662 +++++++++++++++++++++++++++++++++++++++++
5 files changed, 1687 insertions(+)
create mode 100644 drivers/hid/hid-msi.c
--
2.53.0
^ 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