* [PATCH v3 3/8] riscv: stacktrace: disable KASAN and KCOV instrumentation for stacktrace.o
From: Wang Han @ 2026-06-09 6:29 UTC (permalink / raw)
To: Paul Walmsley, Palmer Dabbelt, Albert Ou
Cc: Steven Rostedt, Alexandre Ghiti, Masami Hiramatsu, Mark Rutland,
Catalin Marinas, Chen Pei, Andy Chiu, Björn Töpel,
Deepak Gupta, Puranjay Mohan, Conor Dooley, Josh Poimboeuf,
Jiri Kosina, Miroslav Benes, Petr Mladek, Joe Lawrence,
Shuah Khan, Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, oliver.yang, xueshuai, zhuo.song, jkchen,
linux-riscv, linux-kernel, linux-trace-kernel, live-patching,
linux-kselftest, linux-perf-users
In-Reply-To: <cover.194d76e3a15b.v3.riscv-livepatch.wanghan@linux.alibaba.com>
KASAN records stack traces for every alloc/free, which means it walks
the unwinder very frequently. Instrumenting the stack trace collection
code itself adds substantial overhead and makes the traces themselves
noisier.
KCOV instruments every basic-block edge. The unwinder is a hot path,
especially with KASAN enabled, so KCOV instrumentation has the same kind
of cost and noise problem here.
Mark stacktrace.o as not KASAN- or KCOV-instrumented, matching the x86
treatment of its stack unwinding code. RISC-V keeps the relevant unwinder
code in stacktrace.o, so a single translation-unit annotation covers the
equivalent scope. This is a prerequisite preference for the upcoming
reliable unwinder, but the change is valid on its own.
Signed-off-by: Wang Han <wanghan@linux.alibaba.com>
---
arch/riscv/kernel/Makefile | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/arch/riscv/kernel/Makefile b/arch/riscv/kernel/Makefile
index cabb99cadfb6..c565a72a36f3 100644
--- a/arch/riscv/kernel/Makefile
+++ b/arch/riscv/kernel/Makefile
@@ -44,6 +44,12 @@ CFLAGS_REMOVE_return_address.o = $(CC_FLAGS_FTRACE)
CFLAGS_REMOVE_sbi_ecall.o = $(CC_FLAGS_FTRACE)
endif
+# When KASAN is enabled, a stack trace is recorded for every alloc/free, which
+# can significantly impact performance. Avoid instrumenting the stack trace
+# collection code to minimize this impact.
+KASAN_SANITIZE_stacktrace.o := n
+KCOV_INSTRUMENT_stacktrace.o := n
+
always-$(KBUILD_BUILTIN) += vmlinux.lds
obj-y += head.o
--
2.43.0
^ permalink raw reply related
* [PATCH v3 0/8] riscv: Add reliable stack unwinding for livepatch
From: Wang Han @ 2026-06-09 6:29 UTC (permalink / raw)
To: Paul Walmsley, Palmer Dabbelt, Albert Ou
Cc: Steven Rostedt, Alexandre Ghiti, Masami Hiramatsu, Mark Rutland,
Catalin Marinas, Chen Pei, Andy Chiu, Björn Töpel,
Deepak Gupta, Puranjay Mohan, Conor Dooley, Josh Poimboeuf,
Jiri Kosina, Miroslav Benes, Petr Mladek, Joe Lawrence,
Shuah Khan, Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, oliver.yang, xueshuai, zhuo.song, jkchen,
linux-riscv, linux-kernel, linux-trace-kernel, live-patching,
linux-kselftest, linux-perf-users
In-Reply-To: <20260528082310.1994388-1-wanghan@linux.alibaba.com>
Hi,
This is v3 of the RISC-V reliable stack unwinding series for livepatch.
The series is still based on riscv/for-next commit 0ca1724b56af
("riscv: ftrace: select HAVE_BUILDTIME_MCOUNT_SORT").
Patch 1 fixes the build-time mcount sorting regression for RISC-V
patchable function entries. It is independent from the livepatch
enablement work and can be picked separately if that is preferred.
Patches 2-7 add the reliable frame-pointer unwinder in reviewable
steps, following the arm64 metadata-frame-record and kunwind model but
using the RISC-V {fp, ra} frame-record convention.
Patch 8 adds the RISC-V syscall wrapper prefix used by the livepatch
selftest module.
Problem
=======
Livepatch relies on HAVE_RELIABLE_STACKTRACE to decide whether a task
can safely switch to a patched implementation. RISC-V has a
frame-pointer stack walker, but it is not yet reliable enough for
livepatch. Three pieces are missing:
* arch_stack_walk_reliable() itself, plus the strict stack-bound
checks and forward-progress invariants a reliable unwinder needs.
* Explicit unwind metadata at exception, task-entry and IRQ-stack
boundaries, so the unwinder can distinguish a final user-to-kernel
transition from a nested kernel pt_regs frame instead of guessing
from return addresses.
* Agreement between the ftrace function-graph, perf callchain and
mcount paths and the same frame-record assumptions used by the
reliable unwinder.
There is also a prerequisite ftrace issue on the current riscv/for-next
base. Commit 0ca1724b56af ("riscv: ftrace: select
HAVE_BUILDTIME_MCOUNT_SORT") enabled build-time sorting of the mcount
table. RISC-V uses patchable function entries, and the recorded patch
site is placed before the function symbol. scripts/sorttable currently
does not take that RISC-V layout into account, so valid ftrace sites
can be filtered out before the kernel boots.
Solution
========
Patch 1 fixes scripts/sorttable so the RISC-V build-time mcount sort
path accepts patchable function entries which precede the function
symbol. The fix carries a Fixes: tag for commit 0ca1724b56af ("riscv:
ftrace: select HAVE_BUILDTIME_MCOUNT_SORT") and is otherwise
independent.
Patches 2-7 add the reliable unwinder in small, individually
reviewable steps. The design follows the same FP + metadata model
arm64 already uses for livepatch in production: the metadata frame
record in pt_regs, the unwind-state stack-bound bookkeeping, the
exception boundary handling, and the fgraph / kretprobe return-address
recovery are direct adaptations of arch/arm64/kernel/stacktrace.c,
retargeted to the RISC-V {fp, ra} frame record convention.
Changes since v2
================
* Patch 1:
- Split the arm64-only RELA weak-function fixup comment from the
arm64/RISC-V shared patchable-entry offset handling.
- Add Reviewed-by tags from Steven, Shuai and Chen Pei.
* Patch 2:
- Initialize frame-record metadata in the kernel stack overflow
path as FRAME_META_TYPE_PT_REGS.
- Explicitly set user-fork pt_regs metadata to
FRAME_META_TYPE_FINAL.
- Expand the commit log to document that the call_on_irq_stack
frame-record adjustment fixes a latent RV32 issue where the
aligned stack slot is larger than the raw {fp, ra} record.
* Patch 3:
- Disable KCOV instrumentation for stacktrace.o as well, and update
the subject and commit log accordingly.
* Patch 4:
- Clarify the s0 preservation rationale in the commit log.
- Add Shuai's Reviewed-by tag.
* Patch 5:
- Fix the new header copyright year.
- Add Shuai's Reviewed-by tag.
* Patch 6:
- Keep state->regs set after kunwind_next_regs_pc(), matching
kunwind_init_from_regs() and the arm64 reference.
- Use RISC-V "ra" terminology instead of "LR" in a reliable
unwinder comment.
* Patch 7:
- Document that the 64BIT dependency is a tested-scope guard rather
than a hard technical requirement, and can be relaxed after RV32
receives equivalent coverage.
- Add Shuai's Reviewed-by tag.
* Patch 8:
- Add Reviewed-by tags from Marcos and Shuai.
v2: https://lore.kernel.org/all/20260528082310.1994388-1-wanghan@linux.alibaba.com/
v1: https://lore.kernel.org/all/20260527123530.2593918-1-wanghan@linux.alibaba.com/
Wang Han (8):
scripts/sorttable: Handle RISC-V patchable ftrace entries
riscv: stacktrace: Add frame record metadata
riscv: stacktrace: disable KASAN and KCOV instrumentation for
stacktrace.o
riscv: ftrace: always preserve s0 in dynamic ftrace register frame
riscv: stacktrace: introduce stack-bound tracking helpers
riscv: stacktrace: switch to frame-pointer based unwinder
riscv: Kconfig: enable HAVE_RELIABLE_STACKTRACE and HAVE_LIVEPATCH
selftests/livepatch: Add RISC-V syscall wrapper prefix
arch/riscv/Kconfig | 4 +
arch/riscv/include/asm/ptrace.h | 9 +
arch/riscv/include/asm/stacktrace.h | 65 +-
arch/riscv/include/asm/stacktrace/common.h | 159 +++++
arch/riscv/include/asm/stacktrace/frame.h | 53 ++
arch/riscv/kernel/Makefile | 6 +
arch/riscv/kernel/asm-offsets.c | 4 +
arch/riscv/kernel/entry.S | 43 +-
arch/riscv/kernel/ftrace.c | 6 +-
arch/riscv/kernel/head.S | 23 +
arch/riscv/kernel/mcount-dyn.S | 4 -
arch/riscv/kernel/perf_callchain.c | 2 +-
arch/riscv/kernel/process.c | 33 +-
arch/riscv/kernel/stacktrace.c | 559 +++++++++++++++---
scripts/sorttable.c | 11 +-
.../livepatch/test_modules/test_klp_syscall.c | 2 +
16 files changed, 872 insertions(+), 111 deletions(-)
create mode 100644 arch/riscv/include/asm/stacktrace/common.h
create mode 100644 arch/riscv/include/asm/stacktrace/frame.h
Range-diff against v2:
1: 42147458c15b ! 1: e93530c5718e scripts/sorttable: Handle RISC-V patchable ftrace entries
@@ Commit message
Fixes: 0ca1724b56af ("riscv: ftrace: select HAVE_BUILDTIME_MCOUNT_SORT")
Suggested-by: Steven Rostedt (Google) <rostedt@goodmis.org>
+ Reviewed-by: Steven Rostedt <rostedt@goodmis.org>
+ Reviewed-by: Shuai Xue <xueshuai@linux.alibaba.com>
+ Reviewed-by: Chen Pei <cp0613@linux.alibaba.com>
Link: https://lore.kernel.org/all/20260527113028.4b21a5de@fedora/
Signed-off-by: Wang Han <wanghan@linux.alibaba.com>
@@ scripts/sorttable.c: static int do_file(char const *const fname, void *addr)
- case EM_AARCH64:
#ifdef MCOUNT_SORT_ENABLED
+ case EM_AARCH64:
++ /* arm64 also needs RELA-based weak-function fixups. */
sort_reloc = true;
rela_type = 0x403;
- /* arm64 uses patchable function entry placing before function */
+ /* fallthrough */
+ case EM_RISCV:
-+ /* arm64 and RISC-V place patchable entries before the function */
++ /* arm64 and RISC-V place patchable entries before the function. */
before_func = 8;
+#else
+ case EM_AARCH64:
2: 9f6a4bf60d10 ! 2: 5b6b411e4d9a riscv: stacktrace: Add frame record metadata
@@ Commit message
future reliable unwinder.
Add a small metadata frame record to pt_regs and initialize it on
- exception entry, kernel thread fork, user fork, and early idle task
- setup. The record uses a zero {fp, ra} sentinel plus a type field so a
- later unwinder can distinguish a final user-to-kernel boundary from a
- nested kernel pt_regs boundary.
+ exception entry, kernel stack overflow, kernel thread fork, user fork,
+ and early idle task setup. The record uses a zero {fp, ra} sentinel plus
+ a type field so a later unwinder can distinguish a final user-to-kernel
+ boundary from a nested kernel pt_regs boundary.
This follows the arm64 metadata frame-record model, adapted to the
RISC-V {fp, ra} frame record convention.
@@ Commit message
* exception entry clears the metadata {fp, ra} pair and uses SPP
(or MPP in M-mode) to record whether the pt_regs frame is the final
user-to-kernel boundary or a nested kernel boundary;
+ * the kernel stack overflow path builds a nested pt_regs metadata
+ record on the overflow stack so an unwinder can resume from the
+ pre-overflow s0 saved in PT_S0;
* _start_kernel builds the init task's final metadata record, while
the secondary CPU path sets up s0 before smp_callin() so idle-task
unwinding does not inherit an undefined caller frame;
@@ Commit message
saved {fp, ra} with the raw frame-record size so s0 points at the
RISC-V frame record rather than past the alignment padding.
+ The call_on_irq_stack adjustment fixes a latent RV32 issue. On RV64,
+ sizeof(struct stackframe) is equal to the stack alignment, so the old
+ s0 value happened to point just above the saved {fp, ra}. On RV32, the
+ raw frame record is 8 bytes while the reserved stack slot is 16-byte
+ aligned, so the old s0 value pointed into the padding. Using the raw
+ record size makes s0 point above the saved frame record on both RV32
+ and RV64 while still reserving the aligned slot.
+
These changes keep s0 reserved for the frame-pointer chain at task and
stack-switch boundaries.
@@ arch/riscv/kernel/entry.S: SYM_CODE_START(handle_exception)
/*
* Set the scratch register to 0, so that if a recursive exception
* occurs, the exception vector knows it came from the kernel
+@@ arch/riscv/kernel/entry.S: SYM_CODE_START_LOCAL(handle_kernel_stack_overflow)
+ REG_S s3, PT_BADADDR(sp)
+ REG_S s4, PT_CAUSE(sp)
+ REG_S s5, PT_TP(sp)
++
++ /*
++ * Create a metadata frame record for the overflow pt_regs. The
++ * overflow path is entered from kernel context, so this is a nested
++ * pt_regs boundary and the unwinder can resume from the pre-overflow
++ * frame pointer saved in PT_S0.
++ */
++ REG_S zero, (S_STACKFRAME + STACKFRAME_FP)(sp)
++ REG_S zero, (S_STACKFRAME + STACKFRAME_RA)(sp)
++ li t0, FRAME_META_TYPE_PT_REGS
++ REG_S t0, S_STACKFRAME_TYPE(sp)
++ addi s0, sp, S_STACKFRAME + STACKFRAME_RECORD_SIZE
++
+ move a0, sp
+ tail handle_bad_stack
+ SYM_CODE_END(handle_kernel_stack_overflow)
@@ arch/riscv/kernel/entry.S: ASM_NOKPROBE(handle_kernel_stack_overflow)
SYM_CODE_START(ret_from_fork_kernel_asm)
@@ arch/riscv/kernel/process.c: int copy_thread(struct task_struct *p, const struct
+ /*
+ * Set up the unwind boundary: ensure the metadata
+ * frame record has its {fp,ra} sentinel zeroed and
-+ * point fp/s0 above the metadata record. The type
-+ * field is inherited from the parent's pt_regs.
++ * point fp/s0 above the metadata record. Mark it as
++ * FINAL since this is the outermost kernel entry for
++ * the new task.
+ */
+ childregs->stackframe.record.fp = 0;
+ childregs->stackframe.record.ra = 0;
++ childregs->stackframe.type = FRAME_META_TYPE_FINAL;
+ p->thread.s[0] = (unsigned long)(&childregs->stackframe)
+ + sizeof(struct frame_record);
+
3: c1cc1fdba771 ! 3: dc86baa5b148 riscv: stacktrace: disable KASAN instrumentation for stacktrace.o
@@ Metadata
Author: Wang Han <wanghan@linux.alibaba.com>
## Commit message ##
- riscv: stacktrace: disable KASAN instrumentation for stacktrace.o
+ riscv: stacktrace: disable KASAN and KCOV instrumentation for stacktrace.o
KASAN records stack traces for every alloc/free, which means it walks
the unwinder very frequently. Instrumenting the stack trace collection
code itself adds substantial overhead and makes the traces themselves
noisier.
- Mark stacktrace.o as not KASAN-instrumented, matching the arm, arm64
- and x86 treatment of their stack unwinding code. This is a prerequisite
- preference for the upcoming reliable unwinder, but the change is valid
- on its own.
+ KCOV instruments every basic-block edge. The unwinder is a hot path,
+ especially with KASAN enabled, so KCOV instrumentation has the same kind
+ of cost and noise problem here.
+
+ Mark stacktrace.o as not KASAN- or KCOV-instrumented, matching the x86
+ treatment of its stack unwinding code. RISC-V keeps the relevant unwinder
+ code in stacktrace.o, so a single translation-unit annotation covers the
+ equivalent scope. This is a prerequisite preference for the upcoming
+ reliable unwinder, but the change is valid on its own.
Signed-off-by: Wang Han <wanghan@linux.alibaba.com>
@@ arch/riscv/kernel/Makefile: CFLAGS_REMOVE_return_address.o = $(CC_FLAGS_FTRACE)
+# can significantly impact performance. Avoid instrumenting the stack trace
+# collection code to minimize this impact.
+KASAN_SANITIZE_stacktrace.o := n
++KCOV_INSTRUMENT_stacktrace.o := n
+
always-$(KBUILD_BUILTIN) += vmlinux.lds
4: 8960c3c96143 ! 4: a2d474a996f9 riscv: ftrace: always preserve s0 in dynamic ftrace register frame
@@ Metadata
## Commit message ##
riscv: ftrace: always preserve s0 in dynamic ftrace register frame
- The dynamic ftrace entry/exit only saved s0 (the architectural frame
- pointer) when HAVE_FUNCTION_GRAPH_FP_TEST was selected. The upcoming
- reliable frame-pointer unwinder needs s0 to be present in
- ftrace_regs unconditionally so it can use the frame pointer as the
- function-graph return-address cookie regardless of FP_TEST.
+ struct __arch_ftrace_regs declares s0 unconditionally, and both
+ ftrace_regs_get_frame_pointer() and ftrace_partial_regs() read it
+ unconditionally. But the SAVE_ABI_REGS / RESTORE_ABI_REGS macros in
+ mcount-dyn.S only stored s0 under HAVE_FUNCTION_GRAPH_FP_TEST
+ (CONFIG_FUNCTION_GRAPH_TRACER && CONFIG_FRAME_POINTER). With
+ CONFIG_FRAME_POINTER=n the slot held whatever was on the stack before,
+ so any callback going through ftrace_partial_regs() saw a garbage
+ regs->s0. RISC-V kernels default to FRAME_POINTER=y, which is why this
+ has not bitten in practice.
Save and restore s0 unconditionally in the dynamic ftrace ABI register
- frame. The cost is one extra REG_S/REG_L pair per traced call, which is
- negligible compared to the overall ftrace cost; the benefit is a
- consistent ftrace_regs layout for the unwinder.
+ frame. This fixes the latent garbage-s0 case, brings the dynamic ftrace
+ path in line with the static _mcount path (mcount.S SAVE_ABI_STATE
+ already saves s0 unconditionally), and matches the frame layout already
+ documented in the comment above SAVE_ABI_REGS. It is also a prerequisite
+ for the upcoming reliable unwinder, which reads
+ ftrace_regs_get_frame_pointer(fregs) directly.
+ The cost is one extra REG_S/REG_L pair per traced call, negligible
+ compared to the overall ftrace cost; the existing FREGS_SIZE_ON_STACK
+ already reserved the slot, so no extra stack space is used.
+
+ Reviewed-by: Shuai Xue <xueshuai@linux.alibaba.com>
Signed-off-by: Wang Han <wanghan@linux.alibaba.com>
## arch/riscv/kernel/mcount-dyn.S ##
5: 5fb2633c7e6e ! 5: b74577e4a6b1 riscv: stacktrace: introduce stack-bound tracking helpers
@@ Commit message
on_thread_stack() with the same semantics as before, just expressed in
terms of the new helpers.
+ Reviewed-by: Shuai Xue <xueshuai@linux.alibaba.com>
Signed-off-by: Wang Han <wanghan@linux.alibaba.com>
## arch/riscv/include/asm/stacktrace.h ##
@@ arch/riscv/include/asm/stacktrace/common.h (new)
+ * See: arch/arm64/include/asm/stacktrace/common.h for the reference
+ * implementation.
+ *
-+ * Copyright (C) 2024
++ * Copyright (C) 2026
+ */
+#ifndef __ASM_RISCV_STACKTRACE_COMMON_H
+#define __ASM_RISCV_STACKTRACE_COMMON_H
6: 6b3ec0c98cd8 ! 6: ac01a5cf8317 riscv: stacktrace: switch to frame-pointer based unwinder
@@ arch/riscv/kernel/stacktrace.c: unsigned long __get_wchan(struct task_struct *ta
+ state->regs = regs;
+ state->common.pc = regs->epc;
+ state->common.fp = frame_pointer(regs);
-+ state->regs = NULL;
+ state->source = KUNWIND_SOURCE_REGS_PC;
+ return 0;
+}
@@ arch/riscv/kernel/stacktrace.c: unsigned long __get_wchan(struct task_struct *ta
+{
+ /*
+ * At an exception boundary we can reliably consume the saved PC. We do
-+ * not know whether the LR was live when the exception was taken, and
++ * not know whether ra was live when the exception was taken, and
+ * so we cannot perform the next unwind step reliably.
+ *
+ * All that matters is whether the *entire* unwind is reliable, so give
7: 90fcaa590d57 ! 7: cd40c6ddb5d1 riscv: Kconfig: enable HAVE_RELIABLE_STACKTRACE and HAVE_LIVEPATCH
@@ Commit message
to the rest of the kernel:
* select HAVE_RELIABLE_STACKTRACE under FRAME_POINTER && 64BIT, so
- only the configurations that actually have the metadata records
- and the FP-based reliable walker enable it.
+ only the configurations with the tested metadata records and
+ FP-based reliable walker enable it.
* select HAVE_LIVEPATCH under the same condition and source
kernel/livepatch/Kconfig so the livepatch menu is reachable from
the RISC-V configuration.
+ The 64BIT dependency is conservative scoping rather than a hard
+ technical requirement: the metadata frame record, kunwind state machine
+ and arch_stack_walk_reliable() also build on RV32, and the IRQ-stack
+ frame-record adjustment fixes a latent RV32 issue. However, the syscall
+ livepatch selftest and module relocation path have only been exercised
+ on RV64 QEMU virt so far. The 64BIT gate can be relaxed in a follow-up
+ once RV32 has equivalent coverage.
+
This is split out from the unwinder change so the policy decision and
the implementation can be reviewed and reverted independently.
+ Reviewed-by: Shuai Xue <xueshuai@linux.alibaba.com>
Signed-off-by: Wang Han <wanghan@linux.alibaba.com>
## arch/riscv/Kconfig ##
8: 9590be5df884 ! 8: 194d76e3a15b selftests/livepatch: Add RISC-V syscall wrapper prefix
@@ Commit message
RISC-V target symbol, and the syscall-related livepatch test fails on
RISC-V.
+ Reviewed-by: Marcos Paulo de Souza <mpdesouza@suse.com>
+ Reviewed-by: Shuai Xue <xueshuai@linux.alibaba.com>
Signed-off-by: Wang Han <wanghan@linux.alibaba.com>
## tools/testing/selftests/livepatch/test_modules/test_klp_syscall.c ##
base-commit: 0ca1724b56af054e304a9f3f60623b02a81aba3f
--
2.43.0
^ permalink raw reply
* [PATCH 0/2] arm64: ftrace: support DIRECT_CALLS without CALL_OPS
From: Jose Fernandez (Anthropic) @ 2026-06-09 5:19 UTC (permalink / raw)
To: Steven Rostedt, Masami Hiramatsu, Mark Rutland, Catalin Marinas,
Will Deacon, Nathan Chancellor, Nick Desaulniers, Bill Wendling,
Justin Stitt
Cc: linux-kernel, linux-trace-kernel, linux-arm-kernel, llvm, bpf,
Florent Revest, Puranjay Mohan, Xu Kuohai,
Jose Fernandez (Anthropic)
On arm64, HAVE_DYNAMIC_FTRACE_WITH_DIRECT_CALLS is currently selected
only when DYNAMIC_FTRACE_WITH_CALL_OPS is available. CALL_OPS, in
turn, is mutually exclusive with kCFI: the pre-function NOPs it needs
would change the offset of the pre-function type hash (see
baaf553d3bc3 ("arm64: Implement HAVE_DYNAMIC_FTRACE_WITH_CALL_OPS")),
and the compiler support needed to reconcile the two does not exist
yet.
The result is that a CONFIG_CFI=y arm64 kernel has no
ftrace direct calls at all, so register_fentry() fails with -ENOTSUPP
and no BPF trampoline can attach: fentry/fexit, fmod_ret and BPF LSM
programs are all unavailable. Deployments that want both kCFI
hardening and BPF-based security monitoring currently have to give
one of them up. systemd's bpf-restrict-fs feature hits this today:
https://lore.kernel.org/all/20250610232418.GA3544567@ax162/
CALL_OPS is an optimization for direct calls, not a dependency.
In-BL-range trampolines are reached by a direct branch without
consulting the ops pointer, and out-of-range trampolines already
fall back to ftrace_caller, where the DIRECT_CALLS machinery
(call_direct_funcs() storing the trampoline in ftrace_regs, the
ftrace_caller tail-call) is gated on DIRECT_CALLS alone. s390 and
loongarch ship HAVE_DYNAMIC_FTRACE_WITH_DIRECT_CALLS this way,
without having CALL_OPS at all.
Patch 1 prepares ftrace_modify_call() to build without CALL_OPS by
widening its #ifdef and using the existing ftrace_rec_update_ops()
wrapper (no functional change for current configurations). Patch 2
drops the CALL_OPS requirement from the DIRECT_CALLS select.
Configurations that keep CALL_OPS (clang !CFI, and GCC without
CC_OPTIMIZE_FOR_SIZE) are unchanged. We verified this: in an arm64
clang build, every object file is byte-identical before and after
the series except ftrace.o itself, and its disassembly is identical.
CFI builds (and GCC -Os builds) gain working direct calls, with
out-of-range attachments taking the ftrace_caller dispatch path
instead of the per-callsite fast path.
We tested on a 6.18.y-based kernel and on this base with clang
kCFI builds (CONFIG_CFI=y, enforcing) under qemu (TCG, and KVM on an
arm64 host) and on GB200-based arm64 hardware: fentry/fexit, fmod_ret
and BPF LSM programs load, attach and execute; the ftrace-direct
sample modules (including both modify samples, exercising
ftrace_modify_call()) run cleanly; no CFI violations observed. The
fentry_test, fexit_test, fentry_fexit, fexit_sleep, fexit_stress,
modify_return, tracing_struct, lsm and trampoline_count selftests and
the ftrace direct-call selftests (test.d/direct) pass on the new
configuration with results identical to a CALL_OPS kernel built from
the same tree, and a broader test_progs sweep showed no differences
attributable to this series. Without the series, all of the above
fail at attach time with -ENOTSUPP.
riscv has the same gap (its DIRECT_CALLS select also requires
CALL_OPS, and its CALL_OPS is likewise !CFI); if this approach is
acceptable for arm64 we can follow up there.
---
Jose Fernandez (Anthropic) (2):
arm64: ftrace: prepare ftrace_modify_call() for use without CALL_OPS
arm64: ftrace: allow DIRECT_CALLS without CALL_OPS
arch/arm64/Kconfig | 2 +-
arch/arm64/kernel/ftrace.c | 5 +++--
2 files changed, 4 insertions(+), 3 deletions(-)
---
base-commit: 254f49634ee16a731174d2ae34bc50bd5f45e731
change-id: 20260607-arm64-ftrace-direct-calls-152230ef7077
Best regards,
--
Jose Fernandez (Anthropic) <jose.fernandez@linux.dev>
^ permalink raw reply
* [PATCH 2/2] arm64: ftrace: allow DIRECT_CALLS without CALL_OPS
From: Jose Fernandez (Anthropic) @ 2026-06-09 5:19 UTC (permalink / raw)
To: Steven Rostedt, Masami Hiramatsu, Mark Rutland, Catalin Marinas,
Will Deacon, Nathan Chancellor, Nick Desaulniers, Bill Wendling,
Justin Stitt
Cc: linux-kernel, linux-trace-kernel, linux-arm-kernel, llvm, bpf,
Florent Revest, Puranjay Mohan, Xu Kuohai,
Jose Fernandez (Anthropic)
In-Reply-To: <20260609-arm64-ftrace-direct-calls-v1-0-4a46f266697f@linux.dev>
arm64 gained ftrace direct calls in commit 2aa6ac03516d ("arm64:
ftrace: Add direct call support") on top of
DYNAMIC_FTRACE_WITH_CALL_OPS, using the per-callsite ops pointer as a
fast path to reach the direct trampoline. Since commit baaf553d3bc3
("arm64: Implement HAVE_DYNAMIC_FTRACE_WITH_CALL_OPS"), CALL_OPS is
mutually exclusive with CFI: the pre-function NOPs would change the
offset of the pre-function kCFI type hash, and the compiler support
needed to keep that offset consistent does not exist yet.
The result is that a CONFIG_CFI=y kernel loses CALL_OPS, and with it
DIRECT_CALLS, and with it every BPF trampoline attachment to kernel
functions: register_fentry() returns -ENOTSUPP, so fentry/fexit,
fmod_ret and BPF LSM programs cannot attach at all. This is a real
problem for hardened arm64 deployments that rely on BPF LSM for
security monitoring while keeping kCFI enabled.
CALL_OPS is an optimization for direct calls, not a dependency. When
the direct trampoline is within BL range, the callsite branches
straight to it and ftrace_caller is not involved. When it is out of
range, ftrace_find_callable_addr() already falls back to
ftrace_caller, and the DIRECT_CALLS machinery there
(FREGS_DIRECT_TRAMP, ftrace_caller_direct_late) is gated on
DIRECT_CALLS alone, not CALL_OPS: the ops dispatch invokes
call_direct_funcs(), which stores the trampoline address in
ftrace_regs, and ftrace_caller tail-calls it. s390 and loongarch use
this same mechanism for HAVE_DYNAMIC_FTRACE_WITH_DIRECT_CALLS without
having CALL_OPS at all, and DYNAMIC_FTRACE_WITH_ARGS without CALL_OPS
is already a supported arm64 configuration (GCC builds with
CC_OPTIMIZE_FOR_SIZE do not satisfy the CALL_OPS select condition).
Drop the CALL_OPS requirement from the
HAVE_DYNAMIC_FTRACE_WITH_DIRECT_CALLS select. Configurations that
keep CALL_OPS (!CFI clang builds, and GCC builds without
CC_OPTIMIZE_FOR_SIZE) are unchanged. CALL_OPS-less configurations
take the ftrace_caller ops-dispatch path for out-of-range direct
calls, trading the per-callsite fast path for working BPF
trampolines; in-range attachments still branch directly with no
overhead. GCC -Os builds also gain DIRECT_CALLS as a side effect.
That is intended: s390 and loongarch already ship DIRECT_CALLS
without any per-callsite fast path.
Assisted-by: Claude:unspecified
Signed-off-by: Jose Fernandez (Anthropic) <jose.fernandez@linux.dev>
---
arch/arm64/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index fe60738e5943b..2cd7d536671c9 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -188,7 +188,7 @@ config ARM64
if (GCC_SUPPORTS_DYNAMIC_FTRACE_WITH_ARGS || \
CLANG_SUPPORTS_DYNAMIC_FTRACE_WITH_ARGS)
select HAVE_DYNAMIC_FTRACE_WITH_DIRECT_CALLS \
- if DYNAMIC_FTRACE_WITH_ARGS && DYNAMIC_FTRACE_WITH_CALL_OPS
+ if DYNAMIC_FTRACE_WITH_ARGS
select HAVE_DYNAMIC_FTRACE_WITH_CALL_OPS \
if (DYNAMIC_FTRACE_WITH_ARGS && !CFI && \
(CC_IS_CLANG || !CC_OPTIMIZE_FOR_SIZE))
--
2.52.0
^ permalink raw reply related
* [PATCH 1/2] arm64: ftrace: prepare ftrace_modify_call() for use without CALL_OPS
From: Jose Fernandez (Anthropic) @ 2026-06-09 5:19 UTC (permalink / raw)
To: Steven Rostedt, Masami Hiramatsu, Mark Rutland, Catalin Marinas,
Will Deacon, Nathan Chancellor, Nick Desaulniers, Bill Wendling,
Justin Stitt
Cc: linux-kernel, linux-trace-kernel, linux-arm-kernel, llvm, bpf,
Florent Revest, Puranjay Mohan, Xu Kuohai,
Jose Fernandez (Anthropic)
In-Reply-To: <20260609-arm64-ftrace-direct-calls-v1-0-4a46f266697f@linux.dev>
ftrace_modify_call() is guarded by CONFIG_DYNAMIC_FTRACE_WITH_CALL_OPS
and calls ftrace_rec_set_ops(rec, arm64_rec_get_ops(rec)) directly,
which only exists when CALL_OPS is enabled.
Generic ftrace also needs ftrace_modify_call() when
CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS is enabled, to retarget a
callsite between two non-FTRACE_ADDR destinations, as happens when a
direct trampoline is modified. The next patch allows DIRECT_CALLS without
CALL_OPS, so widen the guard to cover both configurations and switch
the body to the ftrace_rec_update_ops() wrapper, which already has a
stub for the !CALL_OPS case. ftrace_make_call() already uses the same
wrapper today.
No functional change: with CALL_OPS enabled, ftrace_rec_update_ops()
expands to the exact call this replaces.
Assisted-by: Claude:unspecified
Signed-off-by: Jose Fernandez (Anthropic) <jose.fernandez@linux.dev>
---
arch/arm64/kernel/ftrace.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/kernel/ftrace.c b/arch/arm64/kernel/ftrace.c
index 5a1554a441628..e1a3c0b3a0514 100644
--- a/arch/arm64/kernel/ftrace.c
+++ b/arch/arm64/kernel/ftrace.c
@@ -409,7 +409,8 @@ int ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
return ftrace_modify_code(pc, old, new, true);
}
-#ifdef CONFIG_DYNAMIC_FTRACE_WITH_CALL_OPS
+#if defined(CONFIG_DYNAMIC_FTRACE_WITH_CALL_OPS) || \
+ defined(CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS)
int ftrace_modify_call(struct dyn_ftrace *rec, unsigned long old_addr,
unsigned long addr)
{
@@ -417,7 +418,7 @@ int ftrace_modify_call(struct dyn_ftrace *rec, unsigned long old_addr,
u32 old, new;
int ret;
- ret = ftrace_rec_set_ops(rec, arm64_rec_get_ops(rec));
+ ret = ftrace_rec_update_ops(rec);
if (ret)
return ret;
--
2.52.0
^ permalink raw reply related
* [PATCH] tracing/osnoise: Call synchronize_rcu() when unregistering
From: Crystal Wood @ 2026-06-09 4:54 UTC (permalink / raw)
To: Steven Rostedt
Cc: linux-trace-kernel, John Kacur, Tomas Glozar, Costa Shulyupin,
Wander Lairson Costa, Crystal Wood
This ensures that any RCU readers traversing the instance list
have finished, before releasing the reference on the tracer that
the instance points to.
Suggested-by: Steven Rostedt <rostedt@goodmis.org>
Signed-off-by: Crystal Wood <crwood@redhat.com>
---
kernel/trace/trace_osnoise.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/kernel/trace/trace_osnoise.c b/kernel/trace/trace_osnoise.c
index 5e83c4f6f2b4..0e1265acd1cc 100644
--- a/kernel/trace/trace_osnoise.c
+++ b/kernel/trace/trace_osnoise.c
@@ -179,7 +179,9 @@ static void osnoise_unregister_instance(struct trace_array *tr)
if (!found)
return;
- kvfree_rcu_mightsleep(inst);
+ /* Do a full sync to ensure that tr remains valid, not just inst */
+ synchronize_rcu();
+ kvfree(inst);
}
/*
--
2.54.0
^ permalink raw reply related
* Re: [PATCH] rethook: Use tsk->on_cpu to check task execution state
From: Masami Hiramatsu @ 2026-06-09 4:41 UTC (permalink / raw)
To: Peter Zijlstra
Cc: bpf, Tengda Wu, Steven Rostedt, Mathieu Desnoyers,
Alexei Starovoitov, linux-trace-kernel, linux-kernel,
Josh Poimboeuf, jikos, mbenes, pmladek
In-Reply-To: <20260608140654.GE3102624@noisy.programming.kicks-ass.net>
On Mon, 8 Jun 2026 16:06:54 +0200
Peter Zijlstra <peterz@infradead.org> wrote:
> On Mon, Jun 08, 2026 at 10:08:11PM +0900, Masami Hiramatsu wrote:
>
> > > > Anyway, I'm wondering what the purpose of this check here is, there is
> > > > no real comment, and commit 5120d167e21c ("rethook: Remove warning
> > > > messages printed for finding return address of a frame.") is just pure
> > > > voodoo as well.
> > >
> > > FWIW, you should have had this discussion then.
> >
> > Indeed. The rethook is making a shadow stack by list, thus caller must
> > guarantee the target process is blocked at least during this function.
> >
> > The commit messages suggest that when BPF takes a backtrace, it also
> > includes other running tasks. Is that safe?
>
> Well, you get to keep the pieces. At this point safe only pertains to
> 'doesn't-crash', all correctness is out the window.
>
> I always forget the crazy BPF does ;-)
>
> > > > Also, note the comment that goes with the usage of
> > > > task_on_another_cpu(); that thing is racy as all heck.
> > > >
> > > > So it really comes down to what the purpose of this check is.
> >
> > This check has been introduced when it is copied from
> > kretprobe_find_ret_addr(). It has the comment:
> >
> > * The @tsk must be 'current' or a task which is not running. @fp is a hint
> >
> > IIRC, I added this check to explicitly verify this condition.
>
> Right, but it is a prescriptive comment, not an explanatory one. That
> is, it doesn't explain the condition.
>
> > > > I suspect the issue at hand is that tsk->rethook elements, such as
> > > > iterated by __rethook_find_ret_addr() are not safe to be accessed for a
> > > > running task.
> > > >
> > > > Notably while rethook_recycle() has some RCU thing on, that objpool
> > > > thing (and the recycle name itself) seems to strongly suggest iterating
> > > > these things is not sound (you could start with things from this task,
> > > > hit a recycled entry and continue iterating rethooks from another task).
> > > >
> > > > Also note that the current check is also racy, nothing really prevents a
> > > > wakeup from happening right after you observe task_is_running() being
> > > > false. The task can then get scheduled in on another CPU and tear down
> > > > its rethooks concurrent with __rethook_find_ret_addr().
> >
> > Yeah, but is there any way to ensure the task is blocked? Even if it is
> > blocked, like TASK_UNINTERRUPTIBLE, unless holding the actual lock in
> > the rethook, it may not be possible to ensure it?
> >
> > Of course, we could give up on checking within this function and leave
> > everything to the caller to guarantee - as kretprobe does.
> >
> > BTW, the reason why we made it possible to pass tasks other than current
> > is that the stack unwinding code itself supported unwinding tasks other
> > than current, so we had no choice but to create this interface.
> >
> > However, it is a bad idea to check this in deep inside of unwinding.
>
> This, you cannot take locks in unwinding. The only thing you can do is
> try to do the best you can without crashing.
>
> Typically unwind only happens on self -- this is natural, a task crashes
> and unwinds itself, or a task does something (takes a lock, hits a
> tracepoint, etc) and takes a snapshot of its own stack, and this is
> safe.
>
> Things like live-patch use task_call_func(), which ensures the callback
> function is done while holding sufficient locks for the task to not
> change state.
Hmm, is there any way to ensure the function is called from task_call_func()?
(Maybe checking p->pi_lock, but this is not sure the lock owner is this
context?) If not, I need to make this available only for current task
(anyway it just return kretprobe trampoline address, no critical issue)
or, introduce a spinlock.
Or, eventually it may be better to replace kretprobe/rethook with
fprobe return handler.
Thank you,
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
^ permalink raw reply
* [PATCH v4 11/11] HID: spi-hid: add panel follower support
From: Jingyuan Liang @ 2026-06-09 4:41 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Jonathan Corbet, Mark Brown,
Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: linux-input, linux-doc, linux-kernel, linux-spi,
linux-trace-kernel, devicetree, hbarnor, tfiga, Jingyuan Liang
In-Reply-To: <20260609-send-upstream-v4-0-b843d5e6ced3@chromium.org>
Add support to spi-hid to be a panel follower.
Signed-off-by: Jingyuan Liang <jingyliang@chromium.org>
---
drivers/hid/spi-hid/spi-hid-core.c | 164 ++++++++++++++++++++++++++++++-------
drivers/hid/spi-hid/spi-hid-core.h | 7 ++
2 files changed, 142 insertions(+), 29 deletions(-)
diff --git a/drivers/hid/spi-hid/spi-hid-core.c b/drivers/hid/spi-hid/spi-hid-core.c
index 517f06913477..27f25d95ed28 100644
--- a/drivers/hid/spi-hid/spi-hid-core.c
+++ b/drivers/hid/spi-hid/spi-hid-core.c
@@ -1281,6 +1281,106 @@ const struct attribute_group *spi_hid_groups[] = {
};
EXPORT_SYMBOL_GPL(spi_hid_groups);
+/*
+ * At the end of probe we initialize the device:
+ * 0) assert reset, bias the interrupt line
+ * 1) sleep minimal reset delay
+ * 2) power up the device
+ * 3) deassert reset (high)
+ * After this we expect an IRQ with a reset response.
+ */
+static int spi_hid_dev_init(struct spi_hid *shid)
+{
+ struct spi_device *spi = shid->spi;
+ struct device *dev = &spi->dev;
+ int error;
+
+ shid->ops->assert_reset(shid->ops);
+
+ shid->ops->sleep_minimal_reset_delay(shid->ops);
+
+ error = shid->ops->power_up(shid->ops);
+ if (error) {
+ dev_err(dev, "%s: could not power up\n", __func__);
+ shid->regulator_error_count++;
+ shid->regulator_last_error = error;
+ return error;
+ }
+
+ shid->ops->deassert_reset(shid->ops);
+
+ enable_irq(spi->irq);
+
+ return 0;
+}
+
+static void spi_hid_panel_follower_work(struct work_struct *work)
+{
+ struct spi_hid *shid = container_of(work, struct spi_hid,
+ panel_follower_work);
+ int error;
+
+ if (!shid->desc.hid_version)
+ error = spi_hid_dev_init(shid);
+ else
+ error = spi_hid_resume(shid);
+ if (error)
+ dev_warn(&shid->spi->dev, "Power on failed: %d\n", error);
+ else
+ WRITE_ONCE(shid->panel_follower_work_finished, true);
+}
+
+static int spi_hid_panel_follower_resume(struct drm_panel_follower *follower)
+{
+ struct spi_hid *shid = container_of(follower, struct spi_hid, panel_follower);
+
+ /*
+ * Powering on a touchscreen can be a slow process. Queue the work to
+ * the system workqueue so we don't block the panel's power up.
+ */
+ WRITE_ONCE(shid->panel_follower_work_finished, false);
+ schedule_work(&shid->panel_follower_work);
+
+ return 0;
+}
+
+static int spi_hid_panel_follower_suspend(struct drm_panel_follower *follower)
+{
+ struct spi_hid *shid = container_of(follower, struct spi_hid, panel_follower);
+
+ cancel_work_sync(&shid->panel_follower_work);
+
+ if (!READ_ONCE(shid->panel_follower_work_finished))
+ return 0;
+
+ return spi_hid_suspend(shid);
+}
+
+static const struct drm_panel_follower_funcs
+ spi_hid_panel_follower_prepare_funcs = {
+ .panel_prepared = spi_hid_panel_follower_resume,
+ .panel_unpreparing = spi_hid_panel_follower_suspend,
+};
+
+static int spi_hid_register_panel_follower(struct spi_hid *shid)
+{
+ struct device *dev = &shid->spi->dev;
+
+ shid->panel_follower.funcs = &spi_hid_panel_follower_prepare_funcs;
+
+ /*
+ * If we're not in control of our own power up/power down then we can't
+ * do the logic to manage wakeups. Give a warning if a user thought
+ * that was possible then force the capability off.
+ */
+ if (device_can_wakeup(dev)) {
+ dev_warn(dev, "Can't wakeup if following panel\n");
+ device_set_wakeup_capable(dev, false);
+ }
+
+ return drm_panel_add_follower(dev, &shid->panel_follower);
+}
+
int spi_hid_core_probe(struct spi_device *spi, struct spihid_ops *ops,
struct spi_hid_conf *conf)
{
@@ -1300,6 +1400,7 @@ int spi_hid_core_probe(struct spi_device *spi, struct spihid_ops *ops,
shid->ops = ops;
shid->conf = conf;
set_bit(SPI_HID_RESET_PENDING, &shid->flags);
+ shid->is_panel_follower = drm_is_panel_follower(&spi->dev);
spi_set_drvdata(spi, shid);
@@ -1313,6 +1414,7 @@ int spi_hid_core_probe(struct spi_device *spi, struct spihid_ops *ops,
init_completion(&shid->output_done);
INIT_WORK(&shid->reset_work, spi_hid_reset_work);
+ INIT_WORK(&shid->panel_follower_work, spi_hid_panel_follower_work);
/*
* We need to allocate the buffer without knowing the maximum
@@ -1323,20 +1425,6 @@ int spi_hid_core_probe(struct spi_device *spi, struct spihid_ops *ops,
if (error)
return error;
- /*
- * At the end of probe we initialize the device:
- * 0) assert reset, bias the interrupt line
- * 1) sleep minimal reset delay
- * 2) request IRQ
- * 3) power up the device
- * 4) deassert reset (high)
- * After this we expect an IRQ with a reset response.
- */
-
- shid->ops->assert_reset(shid->ops);
-
- shid->ops->sleep_minimal_reset_delay(shid->ops);
-
error = devm_request_threaded_irq(dev, spi->irq, NULL, spi_hid_dev_irq,
IRQF_ONESHOT | IRQF_NO_AUTOEN, dev_name(&spi->dev), shid);
if (error) {
@@ -1351,22 +1439,28 @@ int spi_hid_core_probe(struct spi_device *spi, struct spihid_ops *ops,
}
}
- error = shid->ops->power_up(shid->ops);
- if (error) {
- dev_err(dev, "%s: could not power up\n", __func__);
- if (device_may_wakeup(dev))
- dev_pm_clear_wake_irq(dev);
- return error;
+ if (shid->is_panel_follower) {
+ error = spi_hid_register_panel_follower(shid);
+ if (error) {
+ dev_err_probe(dev, error,
+ "Failed to register panel follower");
+ goto err_wake_irq;
+ }
+ } else {
+ error = spi_hid_dev_init(shid);
+ if (error)
+ goto err_wake_irq;
}
- shid->ops->deassert_reset(shid->ops);
-
- enable_irq(spi->irq);
-
dev_dbg(dev, "%s: d3 -> %s\n", __func__,
spi_hid_power_mode_string(shid->power_state));
return 0;
+
+err_wake_irq:
+ if (device_may_wakeup(dev))
+ dev_pm_clear_wake_irq(dev);
+ return error;
}
EXPORT_SYMBOL_GPL(spi_hid_core_probe);
@@ -1376,15 +1470,21 @@ void spi_hid_core_remove(struct spi_device *spi)
struct device *dev = &spi->dev;
int error;
- disable_irq(spi->irq);
+ if (shid->is_panel_follower)
+ drm_panel_remove_follower(&shid->panel_follower);
+ else
+ disable_irq(spi->irq);
+
cancel_work_sync(&shid->reset_work);
spi_hid_stop_hid(shid);
- shid->ops->assert_reset(shid->ops);
- error = shid->ops->power_down(shid->ops);
- if (error)
- dev_err(dev, "failed to disable regulator\n");
+ if (shid->power_state != HIDSPI_OFF) {
+ shid->ops->assert_reset(shid->ops);
+ error = shid->ops->power_down(shid->ops);
+ if (error)
+ dev_err(dev, "failed to disable regulator\n");
+ }
if (device_may_wakeup(dev))
dev_pm_clear_wake_irq(dev);
@@ -1395,6 +1495,9 @@ static int spi_hid_core_pm_suspend(struct device *dev)
{
struct spi_hid *shid = dev_get_drvdata(dev);
+ if (shid->is_panel_follower)
+ return 0;
+
return spi_hid_suspend(shid);
}
@@ -1402,6 +1505,9 @@ static int spi_hid_core_pm_resume(struct device *dev)
{
struct spi_hid *shid = dev_get_drvdata(dev);
+ if (shid->is_panel_follower)
+ return 0;
+
return spi_hid_resume(shid);
}
diff --git a/drivers/hid/spi-hid/spi-hid-core.h b/drivers/hid/spi-hid/spi-hid-core.h
index 293e2cfcfbf7..261b2fd7f332 100644
--- a/drivers/hid/spi-hid/spi-hid-core.h
+++ b/drivers/hid/spi-hid/spi-hid-core.h
@@ -10,6 +10,8 @@
#include <linux/hid-over-spi.h>
#include <linux/spi/spi.h>
+#include <drm/drm_panel.h>
+
/* Protocol message size constants */
#define SPI_HID_READ_APPROVAL_LEN 5
#define SPI_HID_OUTPUT_HEADER_LEN 8
@@ -56,6 +58,10 @@ struct spi_hid {
struct spi_hid_input_buf *input; /* Input buffer. */
struct spi_hid_input_buf *response; /* Response buffer. */
+ struct drm_panel_follower panel_follower;
+ bool is_panel_follower;
+ bool panel_follower_work_finished;
+
u16 response_length;
u16 bufsize;
@@ -66,6 +72,7 @@ struct spi_hid {
unsigned long flags; /* device flags. */
struct work_struct reset_work;
+ struct work_struct panel_follower_work;
/* Control lock to ensure complete output transaction. */
struct mutex output_lock;
--
2.54.0.1064.gd145956f57-goog
^ permalink raw reply related
* [PATCH v4 09/11] dt-bindings: input: Document hid-over-spi DT schema
From: Jingyuan Liang @ 2026-06-09 4:41 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Jonathan Corbet, Mark Brown,
Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: linux-input, linux-doc, linux-kernel, linux-spi,
linux-trace-kernel, devicetree, hbarnor, tfiga, Jingyuan Liang,
Dmitry Antipov, Jarrett Schultz
In-Reply-To: <20260609-send-upstream-v4-0-b843d5e6ced3@chromium.org>
Documentation describes the required and optional properties for
implementing Device Tree for a Microsoft G6 Touch Digitizer that
supports HID over SPI Protocol 1.0 specification.
The properties are common to HID over SPI.
Signed-off-by: Dmitry Antipov <dmanti@microsoft.com>
Signed-off-by: Jarrett Schultz <jaschultz@microsoft.com>
Signed-off-by: Jingyuan Liang <jingyliang@chromium.org>
---
.../devicetree/bindings/input/hid-over-spi.yaml | 128 +++++++++++++++++++++
1 file changed, 128 insertions(+)
diff --git a/Documentation/devicetree/bindings/input/hid-over-spi.yaml b/Documentation/devicetree/bindings/input/hid-over-spi.yaml
new file mode 100644
index 000000000000..27cf311e0aab
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/hid-over-spi.yaml
@@ -0,0 +1,128 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/input/hid-over-spi.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: HID over SPI Devices
+
+maintainers:
+ - Benjamin Tissoires <benjamin.tissoires@redhat.com>
+ - Jiri Kosina <jkosina@suse.cz>
+ - Jingyuan Liang <jingyliang@chromium.org>
+
+description: |+
+ HID over SPI provides support for various Human Interface Devices over the
+ SPI bus. These devices can be for example touchpads, keyboards, touch screens
+ or sensors.
+
+ The specification has been written by Microsoft and is currently available
+ here: https://www.microsoft.com/en-us/download/details.aspx?id=103325
+
+ The Microsoft HID over SPI specification explicitly dictates that SPI
+ opcodes and register addresses (such as input/output report addresses)
+ are not standardized. Instead, the specification requires the system
+ firmware (e.g., ACPI or Device Tree) to provide these board-specific
+ parameters to the OS. Therefore, these varying parameters must be
+ defined as properties in the Device Tree.
+
+allOf:
+ - $ref: /schemas/input/touchscreen/touchscreen.yaml#
+ - $ref: /schemas/spi/spi-peripheral-props.yaml#
+
+properties:
+ compatible:
+ items:
+ - enum:
+ - microsoft,g6-touch-digitizer
+ - const: hid-over-spi
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ maxItems: 1
+
+ reset-gpios:
+ maxItems: 1
+ description:
+ GPIO specifier for the digitizer's reset pin (active low). The line must
+ be flagged with GPIO_ACTIVE_LOW.
+
+ vdd-supply:
+ description:
+ Regulator for the VDD supply voltage.
+
+ input-report-header-address:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ minimum: 0
+ maximum: 0xffffff
+ description:
+ A value to be included in the Read Approval packet, listing an address of
+ the input report header to be put on the SPI bus. This address has 24
+ bits.
+
+ input-report-body-address:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ minimum: 0
+ maximum: 0xffffff
+ description:
+ A value to be included in the Read Approval packet, listing an address of
+ the input report body to be put on the SPI bus. This address has 24 bits.
+
+ output-report-address:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ minimum: 0
+ maximum: 0xffffff
+ description:
+ A value to be included in the Output Report sent by the host, listing an
+ address where the output report on the SPI bus is to be written to. This
+ address has 24 bits.
+
+ read-opcode:
+ $ref: /schemas/types.yaml#/definitions/uint8
+ description:
+ Value to be used in Read Approval packets. 1 byte.
+
+ write-opcode:
+ $ref: /schemas/types.yaml#/definitions/uint8
+ description:
+ Value to be used in Write Approval packets. 1 byte.
+
+required:
+ - compatible
+ - interrupts
+ - reset-gpios
+ - vdd-supply
+ - input-report-header-address
+ - input-report-body-address
+ - output-report-address
+ - read-opcode
+ - write-opcode
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/irq.h>
+ #include <dt-bindings/gpio/gpio.h>
+
+ spi {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ hid@0 {
+ compatible = "microsoft,g6-touch-digitizer", "hid-over-spi";
+ reg = <0x0>;
+ interrupts-extended = <&gpio 42 IRQ_TYPE_EDGE_FALLING>;
+ reset-gpios = <&gpio 27 GPIO_ACTIVE_LOW>;
+ vdd-supply = <&pm8350c_l3>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&ts_d6_int_bias>;
+ input-report-header-address = <0x1000>;
+ input-report-body-address = <0x1004>;
+ output-report-address = <0x2000>;
+ read-opcode = /bits/ 8 <0x0b>;
+ write-opcode = /bits/ 8 <0x02>;
+ };
+ };
--
2.54.0.1064.gd145956f57-goog
^ permalink raw reply related
* [PATCH v4 10/11] HID: spi-hid: add power management implementation
From: Jingyuan Liang @ 2026-06-09 4:41 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Jonathan Corbet, Mark Brown,
Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: linux-input, linux-doc, linux-kernel, linux-spi,
linux-trace-kernel, devicetree, hbarnor, tfiga, Jingyuan Liang
In-Reply-To: <20260609-send-upstream-v4-0-b843d5e6ced3@chromium.org>
Implement HID over SPI driver power management callbacks.
Signed-off-by: Jingyuan Liang <jingyliang@chromium.org>
---
drivers/hid/spi-hid/spi-hid-acpi.c | 1 +
drivers/hid/spi-hid/spi-hid-core.c | 133 +++++++++++++++++++++++++++++++++++--
drivers/hid/spi-hid/spi-hid-of.c | 1 +
drivers/hid/spi-hid/spi-hid.h | 1 +
4 files changed, 131 insertions(+), 5 deletions(-)
diff --git a/drivers/hid/spi-hid/spi-hid-acpi.c b/drivers/hid/spi-hid/spi-hid-acpi.c
index 298e3ba44d8a..15cfc4e6cc2f 100644
--- a/drivers/hid/spi-hid/spi-hid-acpi.c
+++ b/drivers/hid/spi-hid/spi-hid-acpi.c
@@ -238,6 +238,7 @@ static struct spi_driver spi_hid_acpi_driver = {
.driver = {
.name = "spi_hid_acpi",
.owner = THIS_MODULE,
+ .pm = &spi_hid_core_pm,
.acpi_match_table = spi_hid_acpi_match,
.probe_type = PROBE_PREFER_ASYNCHRONOUS,
.dev_groups = spi_hid_groups,
diff --git a/drivers/hid/spi-hid/spi-hid-core.c b/drivers/hid/spi-hid/spi-hid-core.c
index 698e72102c11..517f06913477 100644
--- a/drivers/hid/spi-hid/spi-hid-core.c
+++ b/drivers/hid/spi-hid/spi-hid-core.c
@@ -36,6 +36,8 @@
#include <linux/list.h>
#include <linux/module.h>
#include <linux/mutex.h>
+#include <linux/pm.h>
+#include <linux/pm_wakeirq.h>
#include <linux/slab.h>
#include <linux/spi/spi.h>
#include <linux/string.h>
@@ -245,6 +247,96 @@ static const char *spi_hid_power_mode_string(enum hidspi_power_state power_state
}
}
+static int spi_hid_suspend(struct spi_hid *shid)
+{
+ int error;
+ struct device *dev = &shid->spi->dev;
+
+ guard(mutex)(&shid->power_lock);
+ if (shid->power_state == HIDSPI_OFF)
+ return 0;
+
+ if (shid->hid) {
+ error = hid_driver_suspend(shid->hid, PMSG_SUSPEND);
+ if (error) {
+ dev_err(dev, "%s failed to suspend hid driver: %d\n",
+ __func__, error);
+ return error;
+ }
+ }
+
+ disable_irq(shid->spi->irq);
+
+ if (!device_may_wakeup(dev)) {
+ set_bit(SPI_HID_RESET_PENDING, &shid->flags);
+
+ shid->ops->assert_reset(shid->ops);
+
+ error = shid->ops->power_down(shid->ops);
+ if (error) {
+ dev_err(dev, "%s: could not power down\n", __func__);
+ shid->regulator_error_count++;
+ shid->regulator_last_error = error;
+ /* Undo partial suspend before returning error */
+ shid->ops->deassert_reset(shid->ops);
+ clear_bit(SPI_HID_RESET_PENDING, &shid->flags);
+ enable_irq(shid->spi->irq);
+ if (shid->hid)
+ hid_driver_reset_resume(shid->hid);
+ return error;
+ }
+
+ shid->power_state = HIDSPI_OFF;
+ }
+ return 0;
+}
+
+static int spi_hid_resume(struct spi_hid *shid)
+{
+ int error;
+ struct device *dev = &shid->spi->dev;
+
+ guard(mutex)(&shid->power_lock);
+
+ if (!device_may_wakeup(dev)) {
+ if (shid->power_state == HIDSPI_OFF) {
+ shid->ops->assert_reset(shid->ops);
+
+ shid->ops->sleep_minimal_reset_delay(shid->ops);
+
+ error = shid->ops->power_up(shid->ops);
+ if (error) {
+ dev_err(dev, "%s: could not power up\n", __func__);
+ shid->regulator_error_count++;
+ shid->regulator_last_error = error;
+ return error;
+ }
+ shid->power_state = HIDSPI_ON;
+ shid->ops->deassert_reset(shid->ops);
+ }
+ }
+
+ enable_irq(shid->spi->irq);
+
+ if (shid->hid) {
+ error = hid_driver_reset_resume(shid->hid);
+ if (error) {
+ dev_err(dev, "%s: failed to reset resume hid driver: %d\n",
+ __func__, error);
+ /* Undo partial resume before returning error */
+ disable_irq(shid->spi->irq);
+ if (!device_may_wakeup(dev)) {
+ set_bit(SPI_HID_RESET_PENDING, &shid->flags);
+ shid->ops->assert_reset(shid->ops);
+ shid->ops->power_down(shid->ops);
+ shid->power_state = HIDSPI_OFF;
+ }
+ return error;
+ }
+ }
+ return 0;
+}
+
static void spi_hid_stop_hid(struct spi_hid *shid)
{
struct hid_device *hid;
@@ -795,6 +887,11 @@ static irqreturn_t spi_hid_dev_irq(int irq, void *_shid)
trace_spi_hid_header_transfer(shid);
scoped_guard(mutex, &shid->io_lock) {
+ if (shid->power_state == HIDSPI_OFF) {
+ dev_warn(dev, "Device is off, ignoring interrupt\n");
+ goto out;
+ }
+
error = spi_hid_input_sync(shid, shid->input->header,
sizeof(shid->input->header), true);
if (error) {
@@ -802,11 +899,6 @@ static irqreturn_t spi_hid_dev_irq(int irq, void *_shid)
goto err;
}
- if (shid->power_state == HIDSPI_OFF) {
- dev_warn(dev, "Device is off after header was received\n");
- goto out;
- }
-
trace_spi_hid_input_header_complete(shid,
shid->input_transfer[0].tx_buf,
shid->input_transfer[0].len,
@@ -1251,10 +1343,19 @@ int spi_hid_core_probe(struct spi_device *spi, struct spihid_ops *ops,
dev_err(dev, "%s: unable to request threaded IRQ\n", __func__);
return error;
}
+ if (device_may_wakeup(dev)) {
+ error = dev_pm_set_wake_irq(dev, spi->irq);
+ if (error) {
+ dev_err(dev, "%s: failed to set wake IRQ\n", __func__);
+ return error;
+ }
+ }
error = shid->ops->power_up(shid->ops);
if (error) {
dev_err(dev, "%s: could not power up\n", __func__);
+ if (device_may_wakeup(dev))
+ dev_pm_clear_wake_irq(dev);
return error;
}
@@ -1284,9 +1385,31 @@ void spi_hid_core_remove(struct spi_device *spi)
error = shid->ops->power_down(shid->ops);
if (error)
dev_err(dev, "failed to disable regulator\n");
+
+ if (device_may_wakeup(dev))
+ dev_pm_clear_wake_irq(dev);
}
EXPORT_SYMBOL_GPL(spi_hid_core_remove);
+static int spi_hid_core_pm_suspend(struct device *dev)
+{
+ struct spi_hid *shid = dev_get_drvdata(dev);
+
+ return spi_hid_suspend(shid);
+}
+
+static int spi_hid_core_pm_resume(struct device *dev)
+{
+ struct spi_hid *shid = dev_get_drvdata(dev);
+
+ return spi_hid_resume(shid);
+}
+
+const struct dev_pm_ops spi_hid_core_pm = {
+ SYSTEM_SLEEP_PM_OPS(spi_hid_core_pm_suspend, spi_hid_core_pm_resume)
+};
+EXPORT_SYMBOL_GPL(spi_hid_core_pm);
+
MODULE_DESCRIPTION("HID over SPI transport driver");
MODULE_AUTHOR("Dmitry Antipov <dmanti@microsoft.com>");
MODULE_LICENSE("GPL");
diff --git a/drivers/hid/spi-hid/spi-hid-of.c b/drivers/hid/spi-hid/spi-hid-of.c
index ba7d5338f5d8..561cf453e44a 100644
--- a/drivers/hid/spi-hid/spi-hid-of.c
+++ b/drivers/hid/spi-hid/spi-hid-of.c
@@ -230,6 +230,7 @@ static struct spi_driver spi_hid_of_driver = {
.driver = {
.name = "spi_hid_of",
.owner = THIS_MODULE,
+ .pm = &spi_hid_core_pm,
.of_match_table = spi_hid_of_match,
.probe_type = PROBE_PREFER_ASYNCHRONOUS,
.dev_groups = spi_hid_groups,
diff --git a/drivers/hid/spi-hid/spi-hid.h b/drivers/hid/spi-hid/spi-hid.h
index f5a5f4d54beb..17b2fdf192ed 100644
--- a/drivers/hid/spi-hid/spi-hid.h
+++ b/drivers/hid/spi-hid/spi-hid.h
@@ -41,5 +41,6 @@ int spi_hid_core_probe(struct spi_device *spi, struct spihid_ops *ops,
void spi_hid_core_remove(struct spi_device *spi);
extern const struct attribute_group *spi_hid_groups[];
+extern const struct dev_pm_ops spi_hid_core_pm;
#endif /* SPI_HID_H */
--
2.54.0.1064.gd145956f57-goog
^ permalink raw reply related
* [PATCH v4 08/11] HID: spi_hid: add device tree support for SPI over HID
From: Jingyuan Liang @ 2026-06-09 4:40 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Jonathan Corbet, Mark Brown,
Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: linux-input, linux-doc, linux-kernel, linux-spi,
linux-trace-kernel, devicetree, hbarnor, tfiga, Jingyuan Liang,
Jarrett Schultz, Dmitry Antipov
In-Reply-To: <20260609-send-upstream-v4-0-b843d5e6ced3@chromium.org>
From: Jarrett Schultz <jaschultz@microsoft.com>
Signed-off-by: Dmitry Antipov <dmanti@microsoft.com>
Signed-off-by: Jingyuan Liang <jingyliang@chromium.org>
---
drivers/hid/spi-hid/Kconfig | 15 +++
drivers/hid/spi-hid/Makefile | 1 +
drivers/hid/spi-hid/spi-hid-of.c | 246 +++++++++++++++++++++++++++++++++++++++
3 files changed, 262 insertions(+)
diff --git a/drivers/hid/spi-hid/Kconfig b/drivers/hid/spi-hid/Kconfig
index 114b1e00da39..76a2cd587a3e 100644
--- a/drivers/hid/spi-hid/Kconfig
+++ b/drivers/hid/spi-hid/Kconfig
@@ -25,6 +25,21 @@ config SPI_HID_ACPI
will be called spi-hid-acpi. It will also build/depend on the
module spi-hid.
+config SPI_HID_OF
+ tristate "HID over SPI transport layer Open Firmware driver"
+ depends on OF
+ select SPI_HID_CORE
+ help
+ Say Y here if you use a keyboard, a touchpad, a touchscreen, or any
+ other HID based devices which are connected to your computer via SPI.
+ This driver supports Open Firmware (Device Tree)-based systems.
+
+ If unsure, say N.
+
+ This support is also available as a module. If so, the module
+ will be called spi-hid-of. It will also build/depend on the
+ module spi-hid.
+
config SPI_HID_CORE
tristate
endif
diff --git a/drivers/hid/spi-hid/Makefile b/drivers/hid/spi-hid/Makefile
index 3ca326602643..31192e71edae 100644
--- a/drivers/hid/spi-hid/Makefile
+++ b/drivers/hid/spi-hid/Makefile
@@ -9,3 +9,4 @@ obj-$(CONFIG_SPI_HID_CORE) += spi-hid.o
spi-hid-objs = spi-hid-core.o
CFLAGS_spi-hid-core.o := -I$(src)
obj-$(CONFIG_SPI_HID_ACPI) += spi-hid-acpi.o
+obj-$(CONFIG_SPI_HID_OF) += spi-hid-of.o
diff --git a/drivers/hid/spi-hid/spi-hid-of.c b/drivers/hid/spi-hid/spi-hid-of.c
new file mode 100644
index 000000000000..ba7d5338f5d8
--- /dev/null
+++ b/drivers/hid/spi-hid/spi-hid-of.c
@@ -0,0 +1,246 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * HID over SPI protocol, Open Firmware related code
+ *
+ * Copyright (c) 2021 Microsoft Corporation
+ *
+ * This code was forked out of the HID over SPI core code, which is partially
+ * based on "HID over I2C protocol implementation:
+ *
+ * Copyright (c) 2012 Benjamin Tissoires <benjamin.tissoires@gmail.com>
+ * Copyright (c) 2012 Ecole Nationale de l'Aviation Civile, France
+ * Copyright (c) 2012 Red Hat, Inc
+ *
+ * which in turn is partially based on "USB HID support for Linux":
+ *
+ * Copyright (c) 1999 Andreas Gal
+ * Copyright (c) 2000-2005 Vojtech Pavlik <vojtech@suse.cz>
+ * Copyright (c) 2005 Michael Haboustak <mike-@cinci.rr.com> for Concept2, Inc
+ * Copyright (c) 2007-2008 Oliver Neukum
+ * Copyright (c) 2006-2010 Jiri Kosina
+ */
+
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/regulator/consumer.h>
+#include <linux/gpio/consumer.h>
+#include <linux/delay.h>
+
+#include "spi-hid.h"
+
+struct spi_hid_timing_data {
+ u32 post_power_on_delay_ms;
+ u32 minimal_reset_delay_ms;
+};
+
+/* Config structure is filled with data from Device Tree */
+struct spi_hid_of_config {
+ struct spihid_ops ops;
+
+ struct spi_hid_conf property_conf;
+ const struct spi_hid_timing_data *timing_data;
+
+ struct gpio_desc *reset_gpio;
+ struct regulator *supply;
+ bool supply_enabled;
+ u16 hid_over_spi_flags;
+};
+
+static const struct spi_hid_timing_data timing_data = {
+ .post_power_on_delay_ms = 10,
+ .minimal_reset_delay_ms = 100,
+};
+
+static int spi_hid_of_populate_config(struct spi_hid_of_config *conf,
+ struct device *dev)
+{
+ int error;
+ u32 val;
+
+ error = device_property_read_u32(dev, "input-report-header-address",
+ &val);
+ if (error) {
+ dev_err(dev, "Input report header address not provided.");
+ return -ENODEV;
+ }
+ conf->property_conf.input_report_header_address = val;
+
+ error = device_property_read_u32(dev, "input-report-body-address", &val);
+ if (error) {
+ dev_err(dev, "Input report body address not provided.");
+ return -ENODEV;
+ }
+ conf->property_conf.input_report_body_address = val;
+
+ error = device_property_read_u32(dev, "output-report-address", &val);
+ if (error) {
+ dev_err(dev, "Output report address not provided.");
+ return -ENODEV;
+ }
+ conf->property_conf.output_report_address = val;
+
+ error = device_property_read_u32(dev, "read-opcode", &val);
+ if (error) {
+ dev_err(dev, "Read opcode not provided.");
+ return -ENODEV;
+ }
+ conf->property_conf.read_opcode = val;
+
+ error = device_property_read_u32(dev, "write-opcode", &val);
+ if (error) {
+ dev_err(dev, "Write opcode not provided.");
+ return -ENODEV;
+ }
+ conf->property_conf.write_opcode = val;
+
+ conf->supply = devm_regulator_get(dev, "vdd");
+ if (IS_ERR(conf->supply)) {
+ if (PTR_ERR(conf->supply) != -EPROBE_DEFER)
+ dev_err(dev, "Failed to get regulator: %ld.",
+ PTR_ERR(conf->supply));
+ return PTR_ERR(conf->supply);
+ }
+ conf->supply_enabled = false;
+
+ conf->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
+ if (IS_ERR(conf->reset_gpio)) {
+ dev_err(dev, "%s: error getting reset GPIO.", __func__);
+ return PTR_ERR(conf->reset_gpio);
+ }
+
+ return 0;
+}
+
+static int spi_hid_of_power_down(struct spihid_ops *ops)
+{
+ struct spi_hid_of_config *conf = container_of(ops,
+ struct spi_hid_of_config,
+ ops);
+ int error;
+
+ if (!conf->supply_enabled)
+ return 0;
+
+ error = regulator_disable(conf->supply);
+ if (error == 0)
+ conf->supply_enabled = false;
+
+ return error;
+}
+
+static int spi_hid_of_power_up(struct spihid_ops *ops)
+{
+ struct spi_hid_of_config *conf = container_of(ops,
+ struct spi_hid_of_config,
+ ops);
+ int error;
+
+ if (conf->supply_enabled)
+ return 0;
+
+ error = regulator_enable(conf->supply);
+
+ if (error == 0) {
+ conf->supply_enabled = true;
+ fsleep(1000 * conf->timing_data->post_power_on_delay_ms);
+ }
+
+ return error;
+}
+
+static int spi_hid_of_assert_reset(struct spihid_ops *ops)
+{
+ struct spi_hid_of_config *conf = container_of(ops,
+ struct spi_hid_of_config,
+ ops);
+
+ gpiod_set_value(conf->reset_gpio, 1);
+ return 0;
+}
+
+static int spi_hid_of_deassert_reset(struct spihid_ops *ops)
+{
+ struct spi_hid_of_config *conf = container_of(ops,
+ struct spi_hid_of_config,
+ ops);
+
+ gpiod_set_value(conf->reset_gpio, 0);
+ return 0;
+}
+
+static void spi_hid_of_sleep_minimal_reset_delay(struct spihid_ops *ops)
+{
+ struct spi_hid_of_config *conf = container_of(ops,
+ struct spi_hid_of_config,
+ ops);
+ fsleep(1000 * conf->timing_data->minimal_reset_delay_ms);
+}
+
+static int spi_hid_of_probe(struct spi_device *spi)
+{
+ struct device *dev = &spi->dev;
+ struct spi_hid_of_config *config;
+ int error;
+
+ config = devm_kzalloc(dev, sizeof(struct spi_hid_of_config),
+ GFP_KERNEL);
+ if (!config)
+ return -ENOMEM;
+
+ config->ops.power_up = spi_hid_of_power_up;
+ config->ops.power_down = spi_hid_of_power_down;
+ config->ops.assert_reset = spi_hid_of_assert_reset;
+ config->ops.deassert_reset = spi_hid_of_deassert_reset;
+ config->ops.sleep_minimal_reset_delay =
+ spi_hid_of_sleep_minimal_reset_delay;
+
+ config->timing_data = device_get_match_data(dev);
+ if (!config->timing_data)
+ config->timing_data = &timing_data;
+
+ /*
+ * FIXME: hid_over_spi_flags could be retrieved from spi mode.
+ * It is always 0 because multi-SPI not supported.
+ */
+ config->hid_over_spi_flags = 0;
+
+ error = spi_hid_of_populate_config(config, dev);
+ if (error) {
+ dev_err(dev, "%s: unable to populate config data.", __func__);
+ return error;
+ }
+
+ return spi_hid_core_probe(spi, &config->ops, &config->property_conf);
+}
+
+static const struct of_device_id spi_hid_of_match[] = {
+ { .compatible = "hid-over-spi", .data = &timing_data },
+ {}
+};
+MODULE_DEVICE_TABLE(of, spi_hid_of_match);
+
+static const struct spi_device_id spi_hid_of_id_table[] = {
+ { "hid", 0 },
+ { "hid-over-spi", 0 },
+ { }
+};
+MODULE_DEVICE_TABLE(spi, spi_hid_of_id_table);
+
+static struct spi_driver spi_hid_of_driver = {
+ .driver = {
+ .name = "spi_hid_of",
+ .owner = THIS_MODULE,
+ .of_match_table = spi_hid_of_match,
+ .probe_type = PROBE_PREFER_ASYNCHRONOUS,
+ .dev_groups = spi_hid_groups,
+ },
+ .probe = spi_hid_of_probe,
+ .remove = spi_hid_core_remove,
+ .id_table = spi_hid_of_id_table,
+};
+
+module_spi_driver(spi_hid_of_driver);
+
+MODULE_DESCRIPTION("HID over SPI OF transport driver");
+MODULE_AUTHOR("Dmitry Antipov <dmanti@microsoft.com>");
+MODULE_LICENSE("GPL");
--
2.54.0.1064.gd145956f57-goog
^ permalink raw reply related
* [PATCH v4 07/11] HID: spi_hid: add ACPI support for SPI over HID
From: Jingyuan Liang @ 2026-06-09 4:40 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Jonathan Corbet, Mark Brown,
Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: linux-input, linux-doc, linux-kernel, linux-spi,
linux-trace-kernel, devicetree, hbarnor, tfiga, Jingyuan Liang,
Angela Czubak
In-Reply-To: <20260609-send-upstream-v4-0-b843d5e6ced3@chromium.org>
From: Angela Czubak <acz@semihalf.com>
Detect SPI HID devices described in ACPI.
Signed-off-by: Angela Czubak <acz@semihalf.com>
Reviewed-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Signed-off-by: Jingyuan Liang <jingyliang@chromium.org>
---
drivers/hid/spi-hid/Kconfig | 15 +++
drivers/hid/spi-hid/Makefile | 1 +
drivers/hid/spi-hid/spi-hid-acpi.c | 253 +++++++++++++++++++++++++++++++++++++
drivers/hid/spi-hid/spi-hid-core.c | 26 +---
drivers/hid/spi-hid/spi-hid.h | 45 +++++++
5 files changed, 315 insertions(+), 25 deletions(-)
diff --git a/drivers/hid/spi-hid/Kconfig b/drivers/hid/spi-hid/Kconfig
index 836fdefe8345..114b1e00da39 100644
--- a/drivers/hid/spi-hid/Kconfig
+++ b/drivers/hid/spi-hid/Kconfig
@@ -10,6 +10,21 @@ menuconfig SPI_HID
if SPI_HID
+config SPI_HID_ACPI
+ tristate "HID over SPI transport layer ACPI driver"
+ depends on ACPI
+ select SPI_HID_CORE
+ help
+ Say Y here if you use a keyboard, a touchpad, a touchscreen, or any
+ other HID based devices which are connected to your computer via SPI.
+ This driver supports ACPI-based systems.
+
+ If unsure, say N.
+
+ This support is also available as a module. If so, the module
+ will be called spi-hid-acpi. It will also build/depend on the
+ module spi-hid.
+
config SPI_HID_CORE
tristate
endif
diff --git a/drivers/hid/spi-hid/Makefile b/drivers/hid/spi-hid/Makefile
index 733e006df56e..3ca326602643 100644
--- a/drivers/hid/spi-hid/Makefile
+++ b/drivers/hid/spi-hid/Makefile
@@ -8,3 +8,4 @@
obj-$(CONFIG_SPI_HID_CORE) += spi-hid.o
spi-hid-objs = spi-hid-core.o
CFLAGS_spi-hid-core.o := -I$(src)
+obj-$(CONFIG_SPI_HID_ACPI) += spi-hid-acpi.o
diff --git a/drivers/hid/spi-hid/spi-hid-acpi.c b/drivers/hid/spi-hid/spi-hid-acpi.c
new file mode 100644
index 000000000000..298e3ba44d8a
--- /dev/null
+++ b/drivers/hid/spi-hid/spi-hid-acpi.c
@@ -0,0 +1,253 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * HID over SPI protocol, ACPI related code
+ *
+ * Copyright (c) 2021 Microsoft Corporation
+ * Copyright (c) 2026 Google LLC
+ *
+ * This code was forked out of the HID over SPI core code, which is partially
+ * based on "HID over I2C protocol implementation:
+ *
+ * Copyright (c) 2012 Benjamin Tissoires <benjamin.tissoires@gmail.com>
+ * Copyright (c) 2012 Ecole Nationale de l'Aviation Civile, France
+ * Copyright (c) 2012 Red Hat, Inc
+ *
+ * which in turn is partially based on "USB HID support for Linux":
+ *
+ * Copyright (c) 1999 Andreas Gal
+ * Copyright (c) 2000-2005 Vojtech Pavlik <vojtech@suse.cz>
+ * Copyright (c) 2005 Michael Haboustak <mike-@cinci.rr.com> for Concept2, Inc
+ * Copyright (c) 2007-2008 Oliver Neukum
+ * Copyright (c) 2006-2010 Jiri Kosina
+ */
+
+#include <linux/acpi.h>
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/reset.h>
+#include <linux/uuid.h>
+
+#include "spi-hid.h"
+
+/* Config structure is filled with data from ACPI */
+struct spi_hid_acpi_config {
+ struct spihid_ops ops;
+
+ struct spi_hid_conf property_conf;
+ u32 post_power_on_delay_ms;
+ u32 minimal_reset_delay_ms;
+ struct acpi_device *adev;
+};
+
+/* HID SPI Device: 6e2ac436-0fcf41af-a265-b32a220dcfab */
+static guid_t spi_hid_guid =
+ GUID_INIT(0x6E2AC436, 0x0FCF, 0x41AF,
+ 0xA2, 0x65, 0xB3, 0x2A, 0x22, 0x0D, 0xCF, 0xAB);
+
+static int spi_hid_acpi_populate_config(struct spi_hid_acpi_config *conf,
+ struct acpi_device *adev)
+{
+ acpi_handle handle = acpi_device_handle(adev);
+ union acpi_object *obj;
+
+ conf->adev = adev;
+
+ /* Revision 3 for HID over SPI V1, see specification. */
+ obj = acpi_evaluate_dsm_typed(handle, &spi_hid_guid, 3, 1, NULL,
+ ACPI_TYPE_INTEGER);
+ if (!obj) {
+ acpi_handle_err(handle,
+ "Error _DSM call to get HID input report header address failed");
+ return -ENODEV;
+ }
+ conf->property_conf.input_report_header_address = obj->integer.value;
+ ACPI_FREE(obj);
+
+ obj = acpi_evaluate_dsm_typed(handle, &spi_hid_guid, 3, 2, NULL,
+ ACPI_TYPE_INTEGER);
+ if (!obj) {
+ acpi_handle_err(handle,
+ "Error _DSM call to get HID input report body address failed");
+ return -ENODEV;
+ }
+ conf->property_conf.input_report_body_address = obj->integer.value;
+ ACPI_FREE(obj);
+
+ obj = acpi_evaluate_dsm_typed(handle, &spi_hid_guid, 3, 3, NULL,
+ ACPI_TYPE_INTEGER);
+ if (!obj) {
+ acpi_handle_err(handle,
+ "Error _DSM call to get HID output report header address failed");
+ return -ENODEV;
+ }
+ conf->property_conf.output_report_address = obj->integer.value;
+ ACPI_FREE(obj);
+
+ obj = acpi_evaluate_dsm_typed(handle, &spi_hid_guid, 3, 4, NULL,
+ ACPI_TYPE_BUFFER);
+ if (!obj) {
+ acpi_handle_err(handle,
+ "Error _DSM call to get HID read opcode failed");
+ return -ENODEV;
+ }
+ if (obj->buffer.length == 1) {
+ conf->property_conf.read_opcode = obj->buffer.pointer[0];
+ } else {
+ acpi_handle_err(handle,
+ "Error _DSM call to get HID read opcode, too long buffer");
+ ACPI_FREE(obj);
+ return -ENODEV;
+ }
+ ACPI_FREE(obj);
+
+ obj = acpi_evaluate_dsm_typed(handle, &spi_hid_guid, 3, 5, NULL,
+ ACPI_TYPE_BUFFER);
+ if (!obj) {
+ acpi_handle_err(handle,
+ "Error _DSM call to get HID write opcode failed");
+ return -ENODEV;
+ }
+ if (obj->buffer.length == 1) {
+ conf->property_conf.write_opcode = obj->buffer.pointer[0];
+ } else {
+ acpi_handle_err(handle,
+ "Error _DSM call to get HID write opcode, too long buffer");
+ ACPI_FREE(obj);
+ return -ENODEV;
+ }
+ ACPI_FREE(obj);
+
+ /* Value not provided in ACPI,*/
+ conf->post_power_on_delay_ms = 5;
+ conf->minimal_reset_delay_ms = 150;
+
+ if (!acpi_has_method(handle, "_RST")) {
+ acpi_handle_err(handle, "No reset method for acpi handle");
+ return -EINVAL;
+ }
+
+ /* FIXME: not reading hid-over-spi-flags, multi-SPI not supported */
+
+ return 0;
+}
+
+static int spi_hid_acpi_power_none(struct spihid_ops *ops)
+{
+ return 0;
+}
+
+static int spi_hid_acpi_power_down(struct spihid_ops *ops)
+{
+ struct spi_hid_acpi_config *conf = container_of(ops,
+ struct spi_hid_acpi_config,
+ ops);
+
+ return acpi_device_set_power(conf->adev, ACPI_STATE_D3);
+}
+
+static int spi_hid_acpi_power_up(struct spihid_ops *ops)
+{
+ struct spi_hid_acpi_config *conf = container_of(ops,
+ struct spi_hid_acpi_config,
+ ops);
+ int error;
+
+ error = acpi_device_set_power(conf->adev, ACPI_STATE_D0);
+ if (error) {
+ dev_err(&conf->adev->dev, "Error could not power up ACPI device: %d.", error);
+ return error;
+ }
+
+ if (conf->post_power_on_delay_ms)
+ msleep(conf->post_power_on_delay_ms);
+
+ return 0;
+}
+
+static int spi_hid_acpi_assert_reset(struct spihid_ops *ops)
+{
+ return 0;
+}
+
+static int spi_hid_acpi_deassert_reset(struct spihid_ops *ops)
+{
+ struct spi_hid_acpi_config *conf = container_of(ops,
+ struct spi_hid_acpi_config,
+ ops);
+
+ return device_reset(&conf->adev->dev);
+}
+
+static void spi_hid_acpi_sleep_minimal_reset_delay(struct spihid_ops *ops)
+{
+ struct spi_hid_acpi_config *conf = container_of(ops,
+ struct spi_hid_acpi_config,
+ ops);
+ fsleep(1000 * conf->minimal_reset_delay_ms);
+}
+
+static int spi_hid_acpi_probe(struct spi_device *spi)
+{
+ struct device *dev = &spi->dev;
+ struct acpi_device *adev;
+ struct spi_hid_acpi_config *config;
+ int error;
+
+ adev = ACPI_COMPANION(dev);
+ if (!adev) {
+ dev_err(dev, "Error could not get ACPI device.");
+ return -ENODEV;
+ }
+
+ config = devm_kzalloc(dev, sizeof(struct spi_hid_acpi_config),
+ GFP_KERNEL);
+ if (!config)
+ return -ENOMEM;
+
+ if (acpi_device_power_manageable(adev)) {
+ config->ops.power_up = spi_hid_acpi_power_up;
+ config->ops.power_down = spi_hid_acpi_power_down;
+ } else {
+ config->ops.power_up = spi_hid_acpi_power_none;
+ config->ops.power_down = spi_hid_acpi_power_none;
+ }
+ config->ops.assert_reset = spi_hid_acpi_assert_reset;
+ config->ops.deassert_reset = spi_hid_acpi_deassert_reset;
+ config->ops.sleep_minimal_reset_delay =
+ spi_hid_acpi_sleep_minimal_reset_delay;
+
+ error = spi_hid_acpi_populate_config(config, adev);
+ if (error) {
+ dev_err(dev, "%s: unable to populate config data.", __func__);
+ return error;
+ }
+
+ return spi_hid_core_probe(spi, &config->ops, &config->property_conf);
+}
+
+static const struct acpi_device_id spi_hid_acpi_match[] = {
+ { "ACPI0C51", 0 },
+ { "PNP0C51", 0 },
+ { }
+};
+MODULE_DEVICE_TABLE(acpi, spi_hid_acpi_match);
+
+static struct spi_driver spi_hid_acpi_driver = {
+ .driver = {
+ .name = "spi_hid_acpi",
+ .owner = THIS_MODULE,
+ .acpi_match_table = spi_hid_acpi_match,
+ .probe_type = PROBE_PREFER_ASYNCHRONOUS,
+ .dev_groups = spi_hid_groups,
+ },
+ .probe = spi_hid_acpi_probe,
+ .remove = spi_hid_core_remove,
+};
+
+module_spi_driver(spi_hid_acpi_driver);
+
+MODULE_DESCRIPTION("HID over SPI ACPI transport driver");
+MODULE_AUTHOR("Angela Czubak <aczubak@google.com>");
+MODULE_LICENSE("GPL");
diff --git a/drivers/hid/spi-hid/spi-hid-core.c b/drivers/hid/spi-hid/spi-hid-core.c
index ef527999d6dc..698e72102c11 100644
--- a/drivers/hid/spi-hid/spi-hid-core.c
+++ b/drivers/hid/spi-hid/spi-hid-core.c
@@ -44,6 +44,7 @@
#include <linux/wait.h>
#include <linux/workqueue.h>
+#include "spi-hid.h"
#include "spi-hid-core.h"
#define CREATE_TRACE_POINTS
@@ -111,31 +112,6 @@ struct spi_hid_output_report {
u8 *content;
};
-/* struct spi_hid_conf - Conf provided to the core */
-struct spi_hid_conf {
- u32 input_report_header_address;
- u32 input_report_body_address;
- u32 output_report_address;
- u8 read_opcode;
- u8 write_opcode;
-};
-
-/**
- * struct spihid_ops - Ops provided to the core
- * @power_up: do sequencing to power up the device
- * @power_down: do sequencing to power down the device
- * @assert_reset: do sequencing to assert the reset line
- * @deassert_reset: do sequencing to deassert the reset line
- * @sleep_minimal_reset_delay: minimal sleep delay during reset
- */
-struct spihid_ops {
- int (*power_up)(struct spihid_ops *ops);
- int (*power_down)(struct spihid_ops *ops);
- int (*assert_reset)(struct spihid_ops *ops);
- int (*deassert_reset)(struct spihid_ops *ops);
- void (*sleep_minimal_reset_delay)(struct spihid_ops *ops);
-};
-
static struct hid_ll_driver spi_hid_ll_driver;
static void spi_hid_populate_read_approvals(const struct spi_hid_conf *conf,
diff --git a/drivers/hid/spi-hid/spi-hid.h b/drivers/hid/spi-hid/spi-hid.h
new file mode 100644
index 000000000000..f5a5f4d54beb
--- /dev/null
+++ b/drivers/hid/spi-hid/spi-hid.h
@@ -0,0 +1,45 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (c) 2021 Microsoft Corporation
+ * Copyright (c) 2026 Google LLC
+ */
+
+#ifndef SPI_HID_H
+#define SPI_HID_H
+
+#include <linux/spi/spi.h>
+#include <linux/sysfs.h>
+
+/* struct spi_hid_conf - Conf provided to the core */
+struct spi_hid_conf {
+ u32 input_report_header_address;
+ u32 input_report_body_address;
+ u32 output_report_address;
+ u8 read_opcode;
+ u8 write_opcode;
+};
+
+/**
+ * struct spihid_ops - Ops provided to the core
+ * @power_up: do sequencing to power up the device
+ * @power_down: do sequencing to power down the device
+ * @assert_reset: do sequencing to assert the reset line
+ * @deassert_reset: do sequencing to deassert the reset line
+ * @sleep_minimal_reset_delay: minimal sleep delay during reset
+ */
+struct spihid_ops {
+ int (*power_up)(struct spihid_ops *ops);
+ int (*power_down)(struct spihid_ops *ops);
+ int (*assert_reset)(struct spihid_ops *ops);
+ int (*deassert_reset)(struct spihid_ops *ops);
+ void (*sleep_minimal_reset_delay)(struct spihid_ops *ops);
+};
+
+int spi_hid_core_probe(struct spi_device *spi, struct spihid_ops *ops,
+ struct spi_hid_conf *conf);
+
+void spi_hid_core_remove(struct spi_device *spi);
+
+extern const struct attribute_group *spi_hid_groups[];
+
+#endif /* SPI_HID_H */
--
2.54.0.1064.gd145956f57-goog
^ permalink raw reply related
* [PATCH v4 05/11] HID: spi-hid: add HID SPI protocol implementation
From: Jingyuan Liang @ 2026-06-09 4:40 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Jonathan Corbet, Mark Brown,
Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: linux-input, linux-doc, linux-kernel, linux-spi,
linux-trace-kernel, devicetree, hbarnor, tfiga, Jingyuan Liang,
Dmitry Antipov, Angela Czubak
In-Reply-To: <20260609-send-upstream-v4-0-b843d5e6ced3@chromium.org>
This driver follows HID Over SPI Protocol Specification 1.0 available at
https://www.microsoft.com/en-us/download/details.aspx?id=103325. The
initial version of the driver does not support: 1) multi-fragment input
reports, 2) sending GET_INPUT and COMMAND output report types and
processing their respective acknowledge input reports, and 3) device
sleep power state.
Signed-off-by: Dmitry Antipov <dmanti@microsoft.com>
Signed-off-by: Angela Czubak <acz@semihalf.com>
Signed-off-by: Jingyuan Liang <jingyliang@chromium.org>
---
drivers/hid/spi-hid/spi-hid-core.c | 608 ++++++++++++++++++++++++++++++++++++-
1 file changed, 592 insertions(+), 16 deletions(-)
diff --git a/drivers/hid/spi-hid/spi-hid-core.c b/drivers/hid/spi-hid/spi-hid-core.c
index 72c2e1ce3e8d..f6ea2d4365a7 100644
--- a/drivers/hid/spi-hid/spi-hid-core.c
+++ b/drivers/hid/spi-hid/spi-hid-core.c
@@ -20,14 +20,20 @@
* Copyright (c) 2006-2010 Jiri Kosina
*/
+#include <linux/cache.h>
#include <linux/completion.h>
#include <linux/crc32.h>
#include <linux/device.h>
+#include <linux/dma-mapping.h>
#include <linux/err.h>
#include <linux/hid.h>
#include <linux/hid-over-spi.h>
+#include <linux/input.h>
#include <linux/interrupt.h>
+#include <linux/irq.h>
#include <linux/jiffies.h>
+#include <linux/kernel.h>
+#include <linux/list.h>
#include <linux/module.h>
#include <linux/mutex.h>
#include <linux/slab.h>
@@ -35,12 +41,22 @@
#include <linux/string.h>
#include <linux/sysfs.h>
#include <linux/unaligned.h>
+#include <linux/wait.h>
+#include <linux/workqueue.h>
+
+/* Protocol constants */
+#define SPI_HID_READ_APPROVAL_CONSTANT 0xff
+#define SPI_HID_INPUT_HEADER_SYNC_BYTE 0x5a
+#define SPI_HID_INPUT_HEADER_VERSION 0x03
+#define SPI_HID_SUPPORTED_VERSION 0x0300
#define SPI_HID_OUTPUT_REPORT_CONTENT_ID_DESC_REQUEST 0x00
-#define SPI_HID_RESP_TIMEOUT 1000
+#define SPI_HID_MAX_RESET_ATTEMPTS 3
+#define SPI_HID_RESP_TIMEOUT 1000
/* Protocol message size constants */
+#define SPI_HID_READ_APPROVAL_LEN 5
#define SPI_HID_OUTPUT_HEADER_LEN 8
/* flags */
@@ -49,6 +65,22 @@
* requests. The FW becomes ready after sending the report descriptor.
*/
#define SPI_HID_READY 0
+/*
+ * refresh_in_progress is set to true while the refresh_device worker
+ * thread is destroying and recreating the hidraw device. When this flag
+ * is set to true, the ll_close and ll_open functions will not cause
+ * power state changes.
+ */
+#define SPI_HID_REFRESH_IN_PROGRESS 1
+/*
+ * reset_pending indicates that the device is being reset. When this flag
+ * is set to true, garbage interrupts triggered during reset will be
+ * dropped and will not cause error handling.
+ */
+#define SPI_HID_RESET_PENDING 2
+#define SPI_HID_RESET_RESPONSE 3
+#define SPI_HID_CREATE_DEVICE 4
+#define SPI_HID_ERROR 5
/* Raw input buffer with data from the bus */
struct spi_hid_input_buf {
@@ -57,6 +89,22 @@ struct spi_hid_input_buf {
u8 content[];
};
+/* Processed data from input report header */
+struct spi_hid_input_header {
+ u8 version;
+ u16 report_length;
+ u8 last_fragment_flag;
+ u8 sync_const;
+};
+
+/* Processed data from an input report */
+struct spi_hid_input_report {
+ u8 report_type;
+ u16 content_length;
+ u8 content_id;
+ u8 *content;
+};
+
/* Raw output report buffer to be put on the bus */
struct spi_hid_output_buf {
u8 header[SPI_HID_OUTPUT_HEADER_LEN];
@@ -114,6 +162,9 @@ struct spi_hid {
struct spi_device *spi; /* spi device. */
struct hid_device *hid; /* pointer to corresponding HID dev. */
+ struct spi_transfer input_transfer[2]; /* Transfer buffer for read and write. */
+ struct spi_message input_message; /* used to execute a sequence of spi transfers. */
+
struct spihid_ops *ops;
struct spi_hid_conf *conf;
@@ -131,8 +182,15 @@ struct spi_hid {
unsigned long flags; /* device flags. */
- /* Control lock to make sure one output transaction at a time. */
+ struct work_struct reset_work;
+
+ /* Control lock to ensure complete output transaction. */
struct mutex output_lock;
+ /* Power lock to make sure one power state change at a time. */
+ struct mutex power_lock;
+ /* Protect bus I/O and shid->hid pointer lifecycle. */
+ struct mutex io_lock;
+
struct completion output_done;
u32 report_descriptor_crc32; /* HID report descriptor crc32 checksum. */
@@ -142,10 +200,74 @@ struct spi_hid {
u32 bus_error_count;
int bus_last_error;
u32 dir_count; /* device initiated reset count. */
+
+ /* DMA-safe transfer buffers */
+ u8 read_approval_header[SPI_HID_READ_APPROVAL_LEN] ____cacheline_aligned;
+ u8 read_approval_body[SPI_HID_READ_APPROVAL_LEN];
};
static struct hid_ll_driver spi_hid_ll_driver;
+static void spi_hid_populate_read_approvals(const struct spi_hid_conf *conf,
+ u8 *header_buf, u8 *body_buf)
+{
+ header_buf[0] = conf->read_opcode;
+ put_unaligned_be24(conf->input_report_header_address, &header_buf[1]);
+ header_buf[4] = SPI_HID_READ_APPROVAL_CONSTANT;
+
+ body_buf[0] = conf->read_opcode;
+ put_unaligned_be24(conf->input_report_body_address, &body_buf[1]);
+ body_buf[4] = SPI_HID_READ_APPROVAL_CONSTANT;
+}
+
+static void spi_hid_parse_dev_desc(const struct hidspi_dev_descriptor *raw,
+ struct spi_hid_device_descriptor *desc)
+{
+ desc->hid_version = le16_to_cpu(raw->bcd_ver);
+ desc->report_descriptor_length = le16_to_cpu(raw->rep_desc_len);
+ desc->max_input_length = le16_to_cpu(raw->max_input_len);
+ desc->max_output_length = le16_to_cpu(raw->max_output_len);
+
+ /* FIXME: multi-fragment not supported, field below not used */
+ desc->max_fragment_length = le16_to_cpu(raw->max_frag_len);
+
+ desc->vendor_id = le16_to_cpu(raw->vendor_id);
+ desc->product_id = le16_to_cpu(raw->product_id);
+ desc->version_id = le16_to_cpu(raw->version_id);
+ desc->no_output_report_ack = le16_to_cpu(raw->flags) & BIT(0);
+}
+
+static void spi_hid_populate_input_header(const u8 *buf,
+ struct spi_hid_input_header *header)
+{
+ header->version = buf[0] & 0xf;
+ header->report_length = (get_unaligned_le16(&buf[1]) & 0x3fff) * 4;
+ header->last_fragment_flag = (buf[2] & 0x40) >> 6;
+ header->sync_const = buf[3];
+}
+
+static void spi_hid_populate_input_body(const u8 *buf,
+ struct input_report_body_header *body)
+{
+ body->input_report_type = buf[0];
+ body->content_len = get_unaligned_le16(&buf[1]);
+ body->content_id = buf[3];
+}
+
+static void spi_hid_input_report_prepare(struct spi_hid_input_buf *buf,
+ struct spi_hid_input_report *report)
+{
+ struct spi_hid_input_header header;
+ struct input_report_body_header body;
+
+ spi_hid_populate_input_header(buf->header, &header);
+ spi_hid_populate_input_body(buf->body, &body);
+ report->report_type = body.input_report_type;
+ report->content_length = body.content_len;
+ report->content_id = body.content_id;
+ report->content = buf->content;
+}
+
static void spi_hid_populate_output_header(u8 *buf,
const struct spi_hid_conf *conf,
const struct spi_hid_output_report *report)
@@ -157,6 +279,33 @@ static void spi_hid_populate_output_header(u8 *buf,
buf[7] = report->content_id;
}
+static int spi_hid_input_sync(struct spi_hid *shid, void *buf, u16 length,
+ bool is_header)
+{
+ int error;
+
+ shid->input_transfer[0].tx_buf = is_header ?
+ shid->read_approval_header :
+ shid->read_approval_body;
+ shid->input_transfer[0].len = SPI_HID_READ_APPROVAL_LEN;
+
+ shid->input_transfer[1].rx_buf = buf;
+ shid->input_transfer[1].len = length;
+
+ spi_message_init_with_transfers(&shid->input_message,
+ shid->input_transfer, 2);
+
+ error = spi_sync(shid->spi, &shid->input_message);
+ if (error) {
+ dev_err(&shid->spi->dev, "Error starting sync transfer: %d\n", error);
+ shid->bus_error_count++;
+ shid->bus_last_error = error;
+ return error;
+ }
+
+ return 0;
+}
+
static int spi_hid_output(struct spi_hid *shid, const void *buf, u16 length)
{
int error;
@@ -187,15 +336,64 @@ static const char *spi_hid_power_mode_string(enum hidspi_power_state power_state
static void spi_hid_stop_hid(struct spi_hid *shid)
{
- struct hid_device *hid = shid->hid;
+ struct hid_device *hid;
- shid->hid = NULL;
- clear_bit(SPI_HID_READY, &shid->flags);
+ scoped_guard(mutex, &shid->io_lock) {
+ hid = shid->hid;
+ shid->hid = NULL;
+ clear_bit(SPI_HID_READY, &shid->flags);
+ }
if (hid)
hid_destroy_device(hid);
}
+static void spi_hid_error_handler(struct spi_hid *shid)
+{
+ struct device *dev = &shid->spi->dev;
+ int error;
+
+ guard(mutex)(&shid->power_lock);
+ if (shid->power_state == HIDSPI_OFF)
+ return;
+
+ guard(disable_irq)(&shid->spi->irq);
+
+ if (shid->reset_attempts++ >= SPI_HID_MAX_RESET_ATTEMPTS) {
+ dev_err(dev, "unresponsive device, aborting\n");
+ spi_hid_stop_hid(shid);
+ shid->ops->assert_reset(shid->ops);
+ error = shid->ops->power_down(shid->ops);
+ if (error) {
+ dev_err(dev, "failed to disable regulator\n");
+ shid->regulator_error_count++;
+ shid->regulator_last_error = error;
+ }
+ return;
+ }
+
+ clear_bit(SPI_HID_READY, &shid->flags);
+ set_bit(SPI_HID_RESET_PENDING, &shid->flags);
+
+ shid->ops->assert_reset(shid->ops);
+
+ shid->power_state = HIDSPI_OFF;
+
+ /*
+ * We want to cancel pending reset work as the device is being reset
+ * to recover from an error. cancel_work_sync will put us in a deadlock
+ * because this function is scheduled in 'reset_work' and we should
+ * avoid waiting for itself.
+ */
+ cancel_work(&shid->reset_work);
+
+ shid->ops->sleep_minimal_reset_delay(shid->ops);
+
+ shid->power_state = HIDSPI_ON;
+
+ shid->ops->deassert_reset(shid->ops);
+}
+
static int __spi_hid_send_output_report(struct spi_hid *shid,
struct spi_hid_output_report *report)
{
@@ -213,6 +411,7 @@ static int __spi_hid_send_output_report(struct spi_hid *shid,
return -E2BIG;
}
+ guard(mutex)(&shid->io_lock);
spi_hid_populate_output_header(buf->header, shid->conf, report);
if (report->content_length)
@@ -263,6 +462,88 @@ static int spi_hid_sync_request(struct spi_hid *shid,
return 0;
}
+/*
+ * Handle the reset response from the FW by sending a request for the device
+ * descriptor.
+ */
+static void spi_hid_reset_response(struct spi_hid *shid)
+{
+ struct device *dev = &shid->spi->dev;
+ struct spi_hid_output_report report = {
+ .report_type = DEVICE_DESCRIPTOR,
+ .content_length = 0x0,
+ .content_id = SPI_HID_OUTPUT_REPORT_CONTENT_ID_DESC_REQUEST,
+ .content = NULL,
+ };
+ int error;
+
+ if (test_bit(SPI_HID_READY, &shid->flags)) {
+ dev_err(dev, "Spontaneous FW reset!\n");
+ clear_bit(SPI_HID_READY, &shid->flags);
+ shid->dir_count++;
+ }
+
+ if (shid->power_state == HIDSPI_OFF)
+ return;
+
+ error = spi_hid_sync_request(shid, &report);
+ if (error) {
+ dev_WARN_ONCE(dev, true,
+ "Failed to send device descriptor request: %d\n", error);
+ set_bit(SPI_HID_ERROR, &shid->flags);
+ schedule_work(&shid->reset_work);
+ }
+}
+
+static int spi_hid_input_report_handler(struct spi_hid *shid,
+ struct spi_hid_input_buf *buf)
+{
+ struct device *dev = &shid->spi->dev;
+ struct spi_hid_input_report r;
+ int error = 0;
+
+ guard(mutex)(&shid->io_lock);
+
+ if (!test_bit(SPI_HID_READY, &shid->flags) ||
+ test_bit(SPI_HID_REFRESH_IN_PROGRESS, &shid->flags) || !shid->hid) {
+ dev_err(dev, "HID not ready\n");
+ return 0;
+ }
+
+ spi_hid_input_report_prepare(buf, &r);
+
+ error = hid_input_report(shid->hid, HID_INPUT_REPORT,
+ r.content - 1, r.content_length + 1, 1);
+
+ if (error == -ENODEV || error == -EBUSY) {
+ dev_err(dev, "ignoring report --> %d\n", error);
+ return 0;
+ } else if (error) {
+ dev_err(dev, "Bad input report: %d\n", error);
+ }
+
+ return error;
+}
+
+static void spi_hid_response_handler(struct spi_hid *shid,
+ struct input_report_body_header *body)
+{
+ shid->response_length = body->content_len;
+ /* completion_done returns 0 if there are waiters, otherwise 1 */
+ if (completion_done(&shid->output_done)) {
+ dev_err(&shid->spi->dev, "Unexpected response report\n");
+ } else {
+ if (body->input_report_type == REPORT_DESCRIPTOR_RESPONSE ||
+ body->input_report_type == GET_FEATURE_RESPONSE) {
+ memcpy(shid->response->body, shid->input->body,
+ sizeof(shid->input->body));
+ memcpy(shid->response->content, shid->input->content,
+ body->content_len);
+ }
+ complete(&shid->output_done);
+ }
+}
+
/*
* This function returns the length of the report descriptor, or a negative
* error code if something went wrong.
@@ -282,6 +563,8 @@ static int spi_hid_report_descriptor_request(struct spi_hid *shid)
if (ret) {
dev_err(dev,
"Expected report descriptor not received: %d\n", ret);
+ set_bit(SPI_HID_ERROR, &shid->flags);
+ schedule_work(&shid->reset_work);
return ret;
}
@@ -320,7 +603,9 @@ static int spi_hid_create_device(struct spi_hid *shid)
hid->vendor, hid->product);
strscpy(hid->phys, dev_name(&shid->spi->dev), sizeof(hid->phys));
- shid->hid = hid;
+ scoped_guard(mutex, &shid->io_lock) {
+ shid->hid = hid;
+ }
error = hid_add_device(hid);
if (error) {
@@ -336,6 +621,208 @@ static int spi_hid_create_device(struct spi_hid *shid)
return 0;
}
+static void spi_hid_refresh_device(struct spi_hid *shid)
+{
+ struct device *dev = &shid->spi->dev;
+ u32 new_crc32 = 0;
+ int error = 0;
+
+ error = spi_hid_report_descriptor_request(shid);
+ if (error < 0) {
+ dev_err(dev,
+ "%s: failed report descriptor request: %d\n",
+ __func__, error);
+ return;
+ }
+ new_crc32 = crc32_le(0, (unsigned char const *)shid->response->content,
+ (size_t)error);
+
+ /* Same report descriptor, so no need to create a new hid device. */
+ if (new_crc32 == shid->report_descriptor_crc32) {
+ set_bit(SPI_HID_READY, &shid->flags);
+ return;
+ }
+
+ shid->report_descriptor_crc32 = new_crc32;
+
+ set_bit(SPI_HID_REFRESH_IN_PROGRESS, &shid->flags);
+
+ spi_hid_stop_hid(shid);
+
+ error = spi_hid_create_device(shid);
+ if (error) {
+ dev_err(dev, "%s: Failed to create hid device: %d\n", __func__, error);
+ return;
+ }
+
+ clear_bit(SPI_HID_REFRESH_IN_PROGRESS, &shid->flags);
+}
+
+static void spi_hid_reset_work(struct work_struct *work)
+{
+ struct spi_hid *shid =
+ container_of(work, struct spi_hid, reset_work);
+ struct device *dev = &shid->spi->dev;
+ int error = 0;
+ bool resched = false;
+
+ if (test_and_clear_bit(SPI_HID_RESET_RESPONSE, &shid->flags)) {
+ spi_hid_reset_response(shid);
+ resched = true;
+ } else if (test_and_clear_bit(SPI_HID_CREATE_DEVICE, &shid->flags)) {
+ guard(mutex)(&shid->power_lock);
+ if (shid->power_state != HIDSPI_OFF) {
+ if (!shid->hid) {
+ error = spi_hid_create_device(shid);
+ if (error) {
+ dev_err(dev, "%s: Failed to create hid device: %d\n",
+ __func__, error);
+ }
+ } else {
+ spi_hid_refresh_device(shid);
+ }
+ } else {
+ dev_err(dev, "%s: Powered off, returning\n", __func__);
+ }
+ resched = true;
+ } else if (test_and_clear_bit(SPI_HID_ERROR, &shid->flags)) {
+ spi_hid_error_handler(shid);
+ }
+
+ /*
+ * If other flags are still pending, safely reschedule ourselves
+ * to process them in the next workqueue cycle.
+ */
+ if (resched && (shid->flags & (BIT(SPI_HID_RESET_RESPONSE) |
+ BIT(SPI_HID_CREATE_DEVICE) |
+ BIT(SPI_HID_ERROR)))) {
+ schedule_work(&shid->reset_work);
+ }
+}
+
+static int spi_hid_process_input_report(struct spi_hid *shid,
+ struct spi_hid_input_buf *buf)
+{
+ struct spi_hid_input_header header;
+ struct input_report_body_header body;
+ struct device *dev = &shid->spi->dev;
+ struct hidspi_dev_descriptor *raw;
+
+ spi_hid_populate_input_header(buf->header, &header);
+ spi_hid_populate_input_body(buf->body, &body);
+
+ if (HIDSPI_INPUT_BODY_SIZE(body.content_len) > header.report_length) {
+ dev_err(dev, "Bad body length %zu > %u\n", HIDSPI_INPUT_BODY_SIZE(body.content_len),
+ header.report_length);
+ return -EPROTO;
+ }
+
+ switch (body.input_report_type) {
+ case DATA:
+ return spi_hid_input_report_handler(shid, buf);
+ case RESET_RESPONSE:
+ clear_bit(SPI_HID_RESET_PENDING, &shid->flags);
+ set_bit(SPI_HID_RESET_RESPONSE, &shid->flags);
+ schedule_work(&shid->reset_work);
+ break;
+ case DEVICE_DESCRIPTOR_RESPONSE:
+ /* Mark the completion done to avoid timeout */
+ spi_hid_response_handler(shid, &body);
+
+ /* Reset attempts at every device descriptor fetch */
+ shid->reset_attempts = 0;
+ raw = (struct hidspi_dev_descriptor *)buf->content;
+
+ /* Validate device descriptor length before parsing */
+ if (body.content_len != HIDSPI_DEVICE_DESCRIPTOR_SIZE) {
+ dev_err(dev, "Invalid content length %d, expected %zu\n",
+ body.content_len,
+ HIDSPI_DEVICE_DESCRIPTOR_SIZE);
+ return -EPROTO;
+ }
+
+ if (le16_to_cpu(raw->dev_desc_len) !=
+ HIDSPI_DEVICE_DESCRIPTOR_SIZE) {
+ dev_err(dev,
+ "Invalid wDeviceDescLength %d, expected %zu\n",
+ le16_to_cpu(raw->dev_desc_len),
+ HIDSPI_DEVICE_DESCRIPTOR_SIZE);
+ return -EPROTO;
+ }
+
+ spi_hid_parse_dev_desc(raw, &shid->desc);
+
+ if (shid->desc.hid_version != SPI_HID_SUPPORTED_VERSION) {
+ dev_err(dev,
+ "Unsupported device descriptor version %4x\n",
+ shid->desc.hid_version);
+ return -EPROTONOSUPPORT;
+ }
+
+ set_bit(SPI_HID_CREATE_DEVICE, &shid->flags);
+ schedule_work(&shid->reset_work);
+
+ break;
+ case OUTPUT_REPORT_RESPONSE:
+ if (shid->desc.no_output_report_ack) {
+ dev_err(dev, "Unexpected output report response\n");
+ break;
+ }
+ fallthrough;
+ case GET_FEATURE_RESPONSE:
+ case SET_FEATURE_RESPONSE:
+ case REPORT_DESCRIPTOR_RESPONSE:
+ spi_hid_response_handler(shid, &body);
+ break;
+ /*
+ * FIXME: sending GET_INPUT and COMMAND reports not supported, thus
+ * throw away responses to those, they should never come.
+ */
+ case GET_INPUT_REPORT_RESPONSE:
+ case COMMAND_RESPONSE:
+ dev_err(dev, "Not a supported report type: 0x%x\n",
+ body.input_report_type);
+ break;
+ default:
+ dev_err(dev, "Unknown input report: 0x%x\n", body.input_report_type);
+ return -EPROTO;
+ }
+
+ return 0;
+}
+
+static int spi_hid_bus_validate_header(struct spi_hid *shid,
+ struct spi_hid_input_header *header)
+{
+ struct device *dev = &shid->spi->dev;
+
+ if (header->version != SPI_HID_INPUT_HEADER_VERSION) {
+ dev_err(dev, "Unknown input report version (v 0x%x)\n",
+ header->version);
+ return -EINVAL;
+ }
+
+ if (shid->desc.max_input_length != 0 &&
+ header->report_length > shid->desc.max_input_length) {
+ dev_err(dev, "Input report body size %u > max expected of %u\n",
+ header->report_length, shid->desc.max_input_length);
+ return -EMSGSIZE;
+ }
+
+ if (header->last_fragment_flag != 1) {
+ dev_err(dev, "Multi-fragment reports not supported\n");
+ return -EOPNOTSUPP;
+ }
+
+ if (header->sync_const != SPI_HID_INPUT_HEADER_SYNC_BYTE) {
+ dev_err(dev, "Invalid input report sync constant (0x%x)\n",
+ header->sync_const);
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
static int spi_hid_get_request(struct spi_hid *shid, u8 content_id)
{
struct device *dev = &shid->spi->dev;
@@ -352,6 +839,8 @@ static int spi_hid_get_request(struct spi_hid *shid, u8 content_id)
dev_err(dev,
"Expected get request response not received! Error %d\n",
error);
+ set_bit(SPI_HID_ERROR, &shid->flags);
+ schedule_work(&shid->reset_work);
return error;
}
@@ -371,9 +860,83 @@ static int spi_hid_set_request(struct spi_hid *shid, u8 *arg_buf, u16 arg_len,
return spi_hid_sync_request(shid, &report);
}
-/* This is a placeholder. Will be implemented in the next patch. */
static irqreturn_t spi_hid_dev_irq(int irq, void *_shid)
{
+ struct spi_hid *shid = _shid;
+ struct device *dev = &shid->spi->dev;
+ struct spi_hid_input_header header;
+ int error = 0;
+
+ scoped_guard(mutex, &shid->io_lock) {
+ error = spi_hid_input_sync(shid, shid->input->header,
+ sizeof(shid->input->header), true);
+ if (error) {
+ dev_err(dev, "Failed to transfer header: %d\n", error);
+ goto err;
+ }
+
+ if (shid->power_state == HIDSPI_OFF) {
+ dev_warn(dev, "Device is off after header was received\n");
+ goto out;
+ }
+
+ if (shid->input_message.status < 0) {
+ dev_warn(dev, "Error reading header: %d\n",
+ shid->input_message.status);
+ shid->bus_error_count++;
+ shid->bus_last_error = shid->input_message.status;
+ goto err;
+ }
+
+ spi_hid_populate_input_header(shid->input->header, &header);
+
+ error = spi_hid_bus_validate_header(shid, &header);
+ if (error) {
+ if (!test_bit(SPI_HID_RESET_PENDING, &shid->flags)) {
+ dev_err(dev, "Failed to validate header: %d\n", error);
+ print_hex_dump(KERN_ERR, "spi_hid: header buffer: ",
+ DUMP_PREFIX_NONE, 16, 1, shid->input->header,
+ sizeof(shid->input->header), false);
+ shid->bus_error_count++;
+ shid->bus_last_error = error;
+ goto err;
+ }
+ goto out;
+ }
+
+ error = spi_hid_input_sync(shid, shid->input->body, header.report_length,
+ false);
+ if (error) {
+ dev_err(dev, "Failed to transfer body: %d\n", error);
+ goto err;
+ }
+
+ if (shid->power_state == HIDSPI_OFF) {
+ dev_warn(dev, "Device is off after body was received\n");
+ goto out;
+ }
+
+ if (shid->input_message.status < 0) {
+ dev_warn(dev, "Error reading body: %d\n",
+ shid->input_message.status);
+ shid->bus_error_count++;
+ shid->bus_last_error = shid->input_message.status;
+ goto err;
+ }
+ }
+
+ error = spi_hid_process_input_report(shid, shid->input);
+ if (error) {
+ dev_err(dev, "Failed to process input report: %d\n", error);
+ goto err;
+ }
+
+out:
+ return IRQ_HANDLED;
+
+err:
+ set_bit(SPI_HID_ERROR, &shid->flags);
+ schedule_work(&shid->reset_work);
return IRQ_HANDLED;
}
@@ -610,10 +1173,13 @@ static int spi_hid_ll_output_report(struct hid_device *hid, __u8 *buf,
return -ENODEV;
}
- if (shid->desc.no_output_report_ack)
- error = spi_hid_send_output_report(shid, &report);
- else
+ if (shid->desc.no_output_report_ack) {
+ scoped_guard(mutex, &shid->output_lock) {
+ error = spi_hid_send_output_report(shid, &report);
+ }
+ } else {
error = spi_hid_sync_request(shid, &report);
+ }
if (error) {
dev_err(dev, "failed to send output report\n");
@@ -701,14 +1267,23 @@ int spi_hid_core_probe(struct spi_device *spi, struct spihid_ops *ops,
shid->power_state = HIDSPI_ON;
shid->ops = ops;
shid->conf = conf;
+ set_bit(SPI_HID_RESET_PENDING, &shid->flags);
spi_set_drvdata(spi, shid);
+ /* Using now populated conf let's pre-calculate the read approvals */
+ spi_hid_populate_read_approvals(shid->conf, shid->read_approval_header,
+ shid->read_approval_body);
+
mutex_init(&shid->output_lock);
+ mutex_init(&shid->power_lock);
+ mutex_init(&shid->io_lock);
init_completion(&shid->output_done);
+ INIT_WORK(&shid->reset_work, spi_hid_reset_work);
+
/*
- * we need to allocate the buffer without knowing the maximum
+ * We need to allocate the buffer without knowing the maximum
* size of the reports. Let's use SZ_2K, then we do the
* real computation later.
*/
@@ -731,7 +1306,7 @@ int spi_hid_core_probe(struct spi_device *spi, struct spihid_ops *ops,
shid->ops->sleep_minimal_reset_delay(shid->ops);
error = devm_request_threaded_irq(dev, spi->irq, NULL, spi_hid_dev_irq,
- IRQF_ONESHOT, dev_name(&spi->dev), shid);
+ IRQF_ONESHOT | IRQF_NO_AUTOEN, dev_name(&spi->dev), shid);
if (error) {
dev_err(dev, "%s: unable to request threaded IRQ\n", __func__);
return error;
@@ -745,13 +1320,11 @@ int spi_hid_core_probe(struct spi_device *spi, struct spihid_ops *ops,
shid->ops->deassert_reset(shid->ops);
+ enable_irq(spi->irq);
+
dev_dbg(dev, "%s: d3 -> %s\n", __func__,
spi_hid_power_mode_string(shid->power_state));
- error = spi_hid_create_device(shid);
- if (error)
- return error;
-
return 0;
}
EXPORT_SYMBOL_GPL(spi_hid_core_probe);
@@ -762,6 +1335,9 @@ void spi_hid_core_remove(struct spi_device *spi)
struct device *dev = &spi->dev;
int error;
+ disable_irq(spi->irq);
+ cancel_work_sync(&shid->reset_work);
+
spi_hid_stop_hid(shid);
shid->ops->assert_reset(shid->ops);
--
2.54.0.1064.gd145956f57-goog
^ permalink raw reply related
* [PATCH v4 06/11] HID: spi_hid: add spi_hid traces
From: Jingyuan Liang @ 2026-06-09 4:40 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Jonathan Corbet, Mark Brown,
Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: linux-input, linux-doc, linux-kernel, linux-spi,
linux-trace-kernel, devicetree, hbarnor, tfiga, Jingyuan Liang,
Dmitry Antipov, Angela Czubak
In-Reply-To: <20260609-send-upstream-v4-0-b843d5e6ced3@chromium.org>
Add traces for purposed of debugging spi_hid driver.
Signed-off-by: Dmitry Antipov <dmanti@microsoft.com>
Signed-off-by: Angela Czubak <acz@semihalf.com>
Signed-off-by: Jingyuan Liang <jingyliang@chromium.org>
---
drivers/hid/spi-hid/Makefile | 1 +
drivers/hid/spi-hid/spi-hid-core.c | 114 +++++++++---------------
drivers/hid/spi-hid/spi-hid-core.h | 91 +++++++++++++++++++
drivers/hid/spi-hid/spi-hid-trace.h | 169 ++++++++++++++++++++++++++++++++++++
4 files changed, 300 insertions(+), 75 deletions(-)
diff --git a/drivers/hid/spi-hid/Makefile b/drivers/hid/spi-hid/Makefile
index 92e24cddbfc2..733e006df56e 100644
--- a/drivers/hid/spi-hid/Makefile
+++ b/drivers/hid/spi-hid/Makefile
@@ -7,3 +7,4 @@
obj-$(CONFIG_SPI_HID_CORE) += spi-hid.o
spi-hid-objs = spi-hid-core.o
+CFLAGS_spi-hid-core.o := -I$(src)
diff --git a/drivers/hid/spi-hid/spi-hid-core.c b/drivers/hid/spi-hid/spi-hid-core.c
index f6ea2d4365a7..ef527999d6dc 100644
--- a/drivers/hid/spi-hid/spi-hid-core.c
+++ b/drivers/hid/spi-hid/spi-hid-core.c
@@ -44,6 +44,11 @@
#include <linux/wait.h>
#include <linux/workqueue.h>
+#include "spi-hid-core.h"
+
+#define CREATE_TRACE_POINTS
+#include "spi-hid-trace.h"
+
/* Protocol constants */
#define SPI_HID_READ_APPROVAL_CONSTANT 0xff
#define SPI_HID_INPUT_HEADER_SYNC_BYTE 0x5a
@@ -82,13 +87,6 @@
#define SPI_HID_CREATE_DEVICE 4
#define SPI_HID_ERROR 5
-/* Raw input buffer with data from the bus */
-struct spi_hid_input_buf {
- u8 header[HIDSPI_INPUT_HEADER_SIZE];
- u8 body[HIDSPI_INPUT_BODY_HEADER_SIZE];
- u8 content[];
-};
-
/* Processed data from input report header */
struct spi_hid_input_header {
u8 version;
@@ -105,12 +103,6 @@ struct spi_hid_input_report {
u8 *content;
};
-/* Raw output report buffer to be put on the bus */
-struct spi_hid_output_buf {
- u8 header[SPI_HID_OUTPUT_HEADER_LEN];
- u8 content[];
-};
-
/* Data necessary to send an output report */
struct spi_hid_output_report {
u8 report_type;
@@ -119,19 +111,6 @@ struct spi_hid_output_report {
u8 *content;
};
-/* Processed data from a device descriptor */
-struct spi_hid_device_descriptor {
- u16 hid_version;
- u16 report_descriptor_length;
- u16 max_input_length;
- u16 max_output_length;
- u16 max_fragment_length;
- u16 vendor_id;
- u16 product_id;
- u16 version_id;
- u8 no_output_report_ack;
-};
-
/* struct spi_hid_conf - Conf provided to the core */
struct spi_hid_conf {
u32 input_report_header_address;
@@ -157,55 +136,6 @@ struct spihid_ops {
void (*sleep_minimal_reset_delay)(struct spihid_ops *ops);
};
-/* Driver context */
-struct spi_hid {
- struct spi_device *spi; /* spi device. */
- struct hid_device *hid; /* pointer to corresponding HID dev. */
-
- struct spi_transfer input_transfer[2]; /* Transfer buffer for read and write. */
- struct spi_message input_message; /* used to execute a sequence of spi transfers. */
-
- struct spihid_ops *ops;
- struct spi_hid_conf *conf;
-
- struct spi_hid_device_descriptor desc; /* HID device descriptor. */
- struct spi_hid_output_buf *output; /* Output buffer. */
- struct spi_hid_input_buf *input; /* Input buffer. */
- struct spi_hid_input_buf *response; /* Response buffer. */
-
- u16 response_length;
- u16 bufsize;
-
- enum hidspi_power_state power_state;
-
- u8 reset_attempts; /* The number of reset attempts. */
-
- unsigned long flags; /* device flags. */
-
- struct work_struct reset_work;
-
- /* Control lock to ensure complete output transaction. */
- struct mutex output_lock;
- /* Power lock to make sure one power state change at a time. */
- struct mutex power_lock;
- /* Protect bus I/O and shid->hid pointer lifecycle. */
- struct mutex io_lock;
-
- struct completion output_done;
-
- u32 report_descriptor_crc32; /* HID report descriptor crc32 checksum. */
-
- u32 regulator_error_count;
- int regulator_last_error;
- u32 bus_error_count;
- int bus_last_error;
- u32 dir_count; /* device initiated reset count. */
-
- /* DMA-safe transfer buffers */
- u8 read_approval_header[SPI_HID_READ_APPROVAL_LEN] ____cacheline_aligned;
- u8 read_approval_body[SPI_HID_READ_APPROVAL_LEN];
-};
-
static struct hid_ll_driver spi_hid_ll_driver;
static void spi_hid_populate_read_approvals(const struct spi_hid_conf *conf,
@@ -295,6 +225,11 @@ static int spi_hid_input_sync(struct spi_hid *shid, void *buf, u16 length,
spi_message_init_with_transfers(&shid->input_message,
shid->input_transfer, 2);
+ trace_spi_hid_input_sync(shid, shid->input_transfer[0].tx_buf,
+ shid->input_transfer[0].len,
+ shid->input_transfer[1].rx_buf,
+ shid->input_transfer[1].len, 0);
+
error = spi_sync(shid->spi, &shid->input_message);
if (error) {
dev_err(&shid->spi->dev, "Error starting sync transfer: %d\n", error);
@@ -353,6 +288,8 @@ static void spi_hid_error_handler(struct spi_hid *shid)
struct device *dev = &shid->spi->dev;
int error;
+ trace_spi_hid_error_handler(shid);
+
guard(mutex)(&shid->power_lock);
if (shid->power_state == HIDSPI_OFF)
return;
@@ -477,6 +414,8 @@ static void spi_hid_reset_response(struct spi_hid *shid)
};
int error;
+ trace_spi_hid_reset_response(shid);
+
if (test_bit(SPI_HID_READY, &shid->flags)) {
dev_err(dev, "Spontaneous FW reset!\n");
clear_bit(SPI_HID_READY, &shid->flags);
@@ -503,6 +442,7 @@ static int spi_hid_input_report_handler(struct spi_hid *shid,
int error = 0;
guard(mutex)(&shid->io_lock);
+ trace_spi_hid_input_report_handler(shid);
if (!test_bit(SPI_HID_READY, &shid->flags) ||
test_bit(SPI_HID_REFRESH_IN_PROGRESS, &shid->flags) || !shid->hid) {
@@ -528,6 +468,8 @@ static int spi_hid_input_report_handler(struct spi_hid *shid,
static void spi_hid_response_handler(struct spi_hid *shid,
struct input_report_body_header *body)
{
+ trace_spi_hid_response_handler(shid);
+
shid->response_length = body->content_len;
/* completion_done returns 0 if there are waiters, otherwise 1 */
if (completion_done(&shid->output_done)) {
@@ -584,6 +526,8 @@ static int spi_hid_create_device(struct spi_hid *shid)
struct device *dev = &shid->spi->dev;
int error;
+ trace_spi_hid_create_device(shid);
+
hid = hid_allocate_device();
error = PTR_ERR_OR_ZERO(hid);
if (error) {
@@ -627,6 +571,8 @@ static void spi_hid_refresh_device(struct spi_hid *shid)
u32 new_crc32 = 0;
int error = 0;
+ trace_spi_hid_refresh_device(shid);
+
error = spi_hid_report_descriptor_request(shid);
if (error < 0) {
dev_err(dev,
@@ -708,6 +654,8 @@ static int spi_hid_process_input_report(struct spi_hid *shid,
struct device *dev = &shid->spi->dev;
struct hidspi_dev_descriptor *raw;
+ trace_spi_hid_process_input_report(shid);
+
spi_hid_populate_input_header(buf->header, &header);
spi_hid_populate_input_body(buf->body, &body);
@@ -867,6 +815,9 @@ static irqreturn_t spi_hid_dev_irq(int irq, void *_shid)
struct spi_hid_input_header header;
int error = 0;
+ trace_spi_hid_dev_irq(shid, irq);
+ trace_spi_hid_header_transfer(shid);
+
scoped_guard(mutex, &shid->io_lock) {
error = spi_hid_input_sync(shid, shid->input->header,
sizeof(shid->input->header), true);
@@ -880,6 +831,13 @@ static irqreturn_t spi_hid_dev_irq(int irq, void *_shid)
goto out;
}
+ trace_spi_hid_input_header_complete(shid,
+ shid->input_transfer[0].tx_buf,
+ shid->input_transfer[0].len,
+ shid->input_transfer[1].rx_buf,
+ shid->input_transfer[1].len,
+ shid->input_message.status);
+
if (shid->input_message.status < 0) {
dev_warn(dev, "Error reading header: %d\n",
shid->input_message.status);
@@ -916,6 +874,12 @@ static irqreturn_t spi_hid_dev_irq(int irq, void *_shid)
goto out;
}
+ trace_spi_hid_input_body_complete(shid, shid->input_transfer[0].tx_buf,
+ shid->input_transfer[0].len,
+ shid->input_transfer[1].rx_buf,
+ shid->input_transfer[1].len,
+ shid->input_message.status);
+
if (shid->input_message.status < 0) {
dev_warn(dev, "Error reading body: %d\n",
shid->input_message.status);
diff --git a/drivers/hid/spi-hid/spi-hid-core.h b/drivers/hid/spi-hid/spi-hid-core.h
new file mode 100644
index 000000000000..293e2cfcfbf7
--- /dev/null
+++ b/drivers/hid/spi-hid/spi-hid-core.h
@@ -0,0 +1,91 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (c) 2021 Microsoft Corporation
+ * Copyright (c) 2026 Google LLC
+ */
+
+#ifndef SPI_HID_CORE_H
+#define SPI_HID_CORE_H
+
+#include <linux/hid-over-spi.h>
+#include <linux/spi/spi.h>
+
+/* Protocol message size constants */
+#define SPI_HID_READ_APPROVAL_LEN 5
+#define SPI_HID_OUTPUT_HEADER_LEN 8
+
+/* Raw input buffer with data from the bus */
+struct spi_hid_input_buf {
+ u8 header[HIDSPI_INPUT_HEADER_SIZE];
+ u8 body[HIDSPI_INPUT_BODY_HEADER_SIZE];
+ u8 content[];
+};
+
+/* Raw output report buffer to be put on the bus */
+struct spi_hid_output_buf {
+ u8 header[SPI_HID_OUTPUT_HEADER_LEN];
+ u8 content[];
+};
+
+/* Processed data from a device descriptor */
+struct spi_hid_device_descriptor {
+ u16 hid_version;
+ u16 report_descriptor_length;
+ u16 max_input_length;
+ u16 max_output_length;
+ u16 max_fragment_length;
+ u16 vendor_id;
+ u16 product_id;
+ u16 version_id;
+ u8 no_output_report_ack;
+};
+
+/* Driver context */
+struct spi_hid {
+ struct spi_device *spi; /* spi device. */
+ struct hid_device *hid; /* pointer to corresponding HID dev. */
+
+ struct spi_transfer input_transfer[2]; /* Transfer buffer for read and write. */
+ struct spi_message input_message; /* used to execute a sequence of spi transfers. */
+
+ struct spihid_ops *ops;
+ struct spi_hid_conf *conf;
+
+ struct spi_hid_device_descriptor desc; /* HID device descriptor. */
+ struct spi_hid_output_buf *output; /* Output buffer. */
+ struct spi_hid_input_buf *input; /* Input buffer. */
+ struct spi_hid_input_buf *response; /* Response buffer. */
+
+ u16 response_length;
+ u16 bufsize;
+
+ enum hidspi_power_state power_state;
+
+ u8 reset_attempts; /* The number of reset attempts. */
+
+ unsigned long flags; /* device flags. */
+
+ struct work_struct reset_work;
+
+ /* Control lock to ensure complete output transaction. */
+ struct mutex output_lock;
+ /* Power lock to make sure one power state change at a time. */
+ struct mutex power_lock;
+ /* I/O lock to prevent concurrent output writes during the input read. */
+ struct mutex io_lock;
+
+ struct completion output_done;
+
+ u8 read_approval_header[SPI_HID_READ_APPROVAL_LEN];
+ u8 read_approval_body[SPI_HID_READ_APPROVAL_LEN];
+
+ u32 report_descriptor_crc32; /* HID report descriptor crc32 checksum. */
+
+ u32 regulator_error_count;
+ int regulator_last_error;
+ u32 bus_error_count;
+ int bus_last_error;
+ u32 dir_count; /* device initiated reset count. */
+};
+
+#endif /* SPI_HID_CORE_H */
diff --git a/drivers/hid/spi-hid/spi-hid-trace.h b/drivers/hid/spi-hid/spi-hid-trace.h
new file mode 100644
index 000000000000..841ec491826d
--- /dev/null
+++ b/drivers/hid/spi-hid/spi-hid-trace.h
@@ -0,0 +1,169 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (c) 2021 Microsoft Corporation
+ */
+
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM spi_hid
+
+#if !defined(_SPI_HID_TRACE_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _SPI_HID_TRACE_H
+
+#include <linux/types.h>
+#include <linux/tracepoint.h>
+#include "spi-hid-core.h"
+
+DECLARE_EVENT_CLASS(spi_hid_transfer,
+ TP_PROTO(struct spi_hid *shid, const void *tx_buf, int tx_len,
+ const void *rx_buf, u16 rx_len, int ret),
+
+ TP_ARGS(shid, tx_buf, tx_len, rx_buf, rx_len, ret),
+
+ TP_STRUCT__entry(
+ __field(int, bus_num)
+ __field(int, chip_select)
+ __field(int, ret)
+ __dynamic_array(u8, rx_buf, rx_len)
+ __dynamic_array(u8, tx_buf, tx_len)
+ ),
+
+ TP_fast_assign(
+ __entry->bus_num = shid->spi->controller->bus_num;
+ __entry->chip_select = spi_get_chipselect(shid->spi, 0);
+ __entry->ret = ret;
+
+ memcpy(__get_dynamic_array(tx_buf), tx_buf, tx_len);
+ memcpy(__get_dynamic_array(rx_buf), rx_buf, rx_len);
+ ),
+
+ TP_printk("spi%d.%d: len=%d tx=[%*phD] rx=[%*phD] --> %d",
+ __entry->bus_num, __entry->chip_select,
+ __get_dynamic_array_len(tx_buf) + __get_dynamic_array_len(rx_buf),
+ __get_dynamic_array_len(tx_buf), __get_dynamic_array(tx_buf),
+ __get_dynamic_array_len(rx_buf), __get_dynamic_array(rx_buf),
+ __entry->ret)
+);
+
+DEFINE_EVENT(spi_hid_transfer, spi_hid_input_sync,
+ TP_PROTO(struct spi_hid *shid, const void *tx_buf, int tx_len,
+ const void *rx_buf, u16 rx_len, int ret),
+ TP_ARGS(shid, tx_buf, tx_len, rx_buf, rx_len, ret));
+
+DEFINE_EVENT(spi_hid_transfer, spi_hid_input_header_complete,
+ TP_PROTO(struct spi_hid *shid, const void *tx_buf, int tx_len,
+ const void *rx_buf, u16 rx_len, int ret),
+ TP_ARGS(shid, tx_buf, tx_len, rx_buf, rx_len, ret));
+
+DEFINE_EVENT(spi_hid_transfer, spi_hid_input_body_complete,
+ TP_PROTO(struct spi_hid *shid, const void *tx_buf, int tx_len,
+ const void *rx_buf, u16 rx_len, int ret),
+ TP_ARGS(shid, tx_buf, tx_len, rx_buf, rx_len, ret));
+
+DECLARE_EVENT_CLASS(spi_hid_irq,
+ TP_PROTO(struct spi_hid *shid, int irq),
+
+ TP_ARGS(shid, irq),
+
+ TP_STRUCT__entry(
+ __field(int, bus_num)
+ __field(int, chip_select)
+ __field(int, irq)
+ ),
+
+ TP_fast_assign(
+ __entry->bus_num = shid->spi->controller->bus_num;
+ __entry->chip_select = spi_get_chipselect(shid->spi, 0);
+ __entry->irq = irq;
+ ),
+
+ TP_printk("spi%d.%d: IRQ %d",
+ __entry->bus_num, __entry->chip_select, __entry->irq)
+);
+
+DEFINE_EVENT(spi_hid_irq, spi_hid_dev_irq,
+ TP_PROTO(struct spi_hid *shid, int irq), TP_ARGS(shid, irq));
+
+DECLARE_EVENT_CLASS(spi_hid,
+ TP_PROTO(struct spi_hid *shid),
+
+ TP_ARGS(shid),
+
+ TP_STRUCT__entry(
+ __field(int, bus_num)
+ __field(int, chip_select)
+ __field(int, power_state)
+ __field(u32, flags)
+
+ __field(int, vendor_id)
+ __field(int, product_id)
+ __field(int, max_input_length)
+ __field(int, max_output_length)
+ __field(u16, hid_version)
+ __field(u16, report_descriptor_length)
+ __field(u16, version_id)
+ ),
+
+ TP_fast_assign(
+ __entry->bus_num = shid->spi->controller->bus_num;
+ __entry->chip_select = spi_get_chipselect(shid->spi, 0);
+ __entry->power_state = shid->power_state;
+ __entry->flags = shid->flags;
+
+ __entry->vendor_id = shid->desc.vendor_id;
+ __entry->product_id = shid->desc.product_id;
+ __entry->max_input_length = shid->desc.max_input_length;
+ __entry->max_output_length = shid->desc.max_output_length;
+ __entry->hid_version = shid->desc.hid_version;
+ __entry->report_descriptor_length =
+ shid->desc.report_descriptor_length;
+ __entry->version_id = shid->desc.version_id;
+ ),
+
+ TP_printk("spi%d.%d: (%04x:%04x v%d) HID v%d.%d state p:%d len i:%d o:%d r:%d flags 0x%08x",
+ __entry->bus_num, __entry->chip_select,
+ __entry->vendor_id, __entry->product_id, __entry->version_id,
+ __entry->hid_version >> 8, __entry->hid_version & 0xff,
+ __entry->power_state, __entry->max_input_length,
+ __entry->max_output_length, __entry->report_descriptor_length,
+ __entry->flags)
+);
+
+DEFINE_EVENT(spi_hid, spi_hid_header_transfer, TP_PROTO(struct spi_hid *shid),
+ TP_ARGS(shid));
+
+DEFINE_EVENT(spi_hid, spi_hid_process_input_report,
+ TP_PROTO(struct spi_hid *shid), TP_ARGS(shid));
+
+DEFINE_EVENT(spi_hid, spi_hid_input_report_handler,
+ TP_PROTO(struct spi_hid *shid), TP_ARGS(shid));
+
+DEFINE_EVENT(spi_hid, spi_hid_reset_response, TP_PROTO(struct spi_hid *shid),
+ TP_ARGS(shid));
+
+DEFINE_EVENT(spi_hid, spi_hid_create_device, TP_PROTO(struct spi_hid *shid),
+ TP_ARGS(shid));
+
+DEFINE_EVENT(spi_hid, spi_hid_refresh_device, TP_PROTO(struct spi_hid *shid),
+ TP_ARGS(shid));
+
+DEFINE_EVENT(spi_hid, spi_hid_response_handler, TP_PROTO(struct spi_hid *shid),
+ TP_ARGS(shid));
+
+DEFINE_EVENT(spi_hid, spi_hid_error_handler, TP_PROTO(struct spi_hid *shid),
+ TP_ARGS(shid));
+
+#endif /* _SPI_HID_TRACE_H */
+
+/*
+ * The following must be outside the protection of the above #if block.
+ */
+#undef TRACE_INCLUDE_PATH
+#undef TRACE_INCLUDE_FILE
+#define TRACE_INCLUDE_PATH .
+
+/*
+ * It is required that the TRACE_INCLUDE_FILE be the same
+ * as this file without the ".h".
+ */
+#define TRACE_INCLUDE_FILE spi-hid-trace
+#include <trace/define_trace.h>
--
2.54.0.1064.gd145956f57-goog
^ permalink raw reply related
* [PATCH v4 04/11] HID: spi-hid: add spi-hid driver HID layer
From: Jingyuan Liang @ 2026-06-09 4:40 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Jonathan Corbet, Mark Brown,
Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: linux-input, linux-doc, linux-kernel, linux-spi,
linux-trace-kernel, devicetree, hbarnor, tfiga, Jingyuan Liang,
Dmitry Antipov, Angela Czubak
In-Reply-To: <20260609-send-upstream-v4-0-b843d5e6ced3@chromium.org>
Add HID low level driver callbacks to register SPI as a HID driver, and
an external touch device as a HID device.
Signed-off-by: Dmitry Antipov <dmanti@microsoft.com>
Signed-off-by: Angela Czubak <acz@semihalf.com>
Signed-off-by: Jingyuan Liang <jingyliang@chromium.org>
---
drivers/hid/spi-hid/spi-hid-core.c | 563 +++++++++++++++++++++++++++++++++++++
1 file changed, 563 insertions(+)
diff --git a/drivers/hid/spi-hid/spi-hid-core.c b/drivers/hid/spi-hid/spi-hid-core.c
index 02a7608c4b88..72c2e1ce3e8d 100644
--- a/drivers/hid/spi-hid/spi-hid-core.c
+++ b/drivers/hid/spi-hid/spi-hid-core.c
@@ -20,13 +20,69 @@
* Copyright (c) 2006-2010 Jiri Kosina
*/
+#include <linux/completion.h>
+#include <linux/crc32.h>
#include <linux/device.h>
+#include <linux/err.h>
#include <linux/hid.h>
#include <linux/hid-over-spi.h>
#include <linux/interrupt.h>
+#include <linux/jiffies.h>
#include <linux/module.h>
+#include <linux/mutex.h>
#include <linux/slab.h>
#include <linux/spi/spi.h>
+#include <linux/string.h>
+#include <linux/sysfs.h>
+#include <linux/unaligned.h>
+
+#define SPI_HID_OUTPUT_REPORT_CONTENT_ID_DESC_REQUEST 0x00
+
+#define SPI_HID_RESP_TIMEOUT 1000
+
+/* Protocol message size constants */
+#define SPI_HID_OUTPUT_HEADER_LEN 8
+
+/* flags */
+/*
+ * ready flag indicates that the FW is ready to accept commands and
+ * requests. The FW becomes ready after sending the report descriptor.
+ */
+#define SPI_HID_READY 0
+
+/* Raw input buffer with data from the bus */
+struct spi_hid_input_buf {
+ u8 header[HIDSPI_INPUT_HEADER_SIZE];
+ u8 body[HIDSPI_INPUT_BODY_HEADER_SIZE];
+ u8 content[];
+};
+
+/* Raw output report buffer to be put on the bus */
+struct spi_hid_output_buf {
+ u8 header[SPI_HID_OUTPUT_HEADER_LEN];
+ u8 content[];
+};
+
+/* Data necessary to send an output report */
+struct spi_hid_output_report {
+ u8 report_type;
+ u16 content_length;
+ u8 content_id;
+ u8 *content;
+};
+
+/* Processed data from a device descriptor */
+struct spi_hid_device_descriptor {
+ u16 hid_version;
+ u16 report_descriptor_length;
+ u16 max_input_length;
+ u16 max_output_length;
+ u16 max_fragment_length;
+ u16 vendor_id;
+ u16 product_id;
+ u16 version_id;
+ u8 no_output_report_ack;
+};
/* struct spi_hid_conf - Conf provided to the core */
struct spi_hid_conf {
@@ -61,8 +117,26 @@ struct spi_hid {
struct spihid_ops *ops;
struct spi_hid_conf *conf;
+ struct spi_hid_device_descriptor desc; /* HID device descriptor. */
+ struct spi_hid_output_buf *output; /* Output buffer. */
+ struct spi_hid_input_buf *input; /* Input buffer. */
+ struct spi_hid_input_buf *response; /* Response buffer. */
+
+ u16 response_length;
+ u16 bufsize;
+
enum hidspi_power_state power_state;
+ u8 reset_attempts; /* The number of reset attempts. */
+
+ unsigned long flags; /* device flags. */
+
+ /* Control lock to make sure one output transaction at a time. */
+ struct mutex output_lock;
+ struct completion output_done;
+
+ u32 report_descriptor_crc32; /* HID report descriptor crc32 checksum. */
+
u32 regulator_error_count;
int regulator_last_error;
u32 bus_error_count;
@@ -70,6 +144,33 @@ struct spi_hid {
u32 dir_count; /* device initiated reset count. */
};
+static struct hid_ll_driver spi_hid_ll_driver;
+
+static void spi_hid_populate_output_header(u8 *buf,
+ const struct spi_hid_conf *conf,
+ const struct spi_hid_output_report *report)
+{
+ buf[0] = conf->write_opcode;
+ put_unaligned_be24(conf->output_report_address, &buf[1]);
+ buf[4] = report->report_type;
+ put_unaligned_le16(report->content_length, &buf[5]);
+ buf[7] = report->content_id;
+}
+
+static int spi_hid_output(struct spi_hid *shid, const void *buf, u16 length)
+{
+ int error;
+
+ error = spi_write(shid->spi, buf, length);
+
+ if (error) {
+ shid->bus_error_count++;
+ shid->bus_last_error = error;
+ }
+
+ return error;
+}
+
static const char *spi_hid_power_mode_string(enum hidspi_power_state power_state)
{
switch (power_state) {
@@ -84,11 +185,455 @@ static const char *spi_hid_power_mode_string(enum hidspi_power_state power_state
}
}
+static void spi_hid_stop_hid(struct spi_hid *shid)
+{
+ struct hid_device *hid = shid->hid;
+
+ shid->hid = NULL;
+ clear_bit(SPI_HID_READY, &shid->flags);
+
+ if (hid)
+ hid_destroy_device(hid);
+}
+
+static int __spi_hid_send_output_report(struct spi_hid *shid,
+ struct spi_hid_output_report *report)
+{
+ struct spi_hid_output_buf *buf = shid->output;
+ struct device *dev = &shid->spi->dev;
+ u16 report_length;
+ u16 padded_length;
+ u8 padding;
+ int error;
+
+ if (report->content_length > shid->desc.max_output_length ||
+ report->content_length > shid->bufsize) {
+ dev_err(dev, "Output report too big, content_length 0x%x\n",
+ report->content_length);
+ return -E2BIG;
+ }
+
+ spi_hid_populate_output_header(buf->header, shid->conf, report);
+
+ if (report->content_length)
+ memcpy(&buf->content, report->content, report->content_length);
+
+ report_length = sizeof(buf->header) + report->content_length;
+ padded_length = round_up(report_length, 4);
+ padding = padded_length - report_length;
+ memset(&buf->content[report->content_length], 0, padding);
+
+ error = spi_hid_output(shid, buf, padded_length);
+ if (error)
+ dev_err(dev, "Failed output transfer: %d\n", error);
+
+ return error;
+}
+
+static int spi_hid_send_output_report(struct spi_hid *shid,
+ struct spi_hid_output_report *report)
+{
+ guard(mutex)(&shid->output_lock);
+ return __spi_hid_send_output_report(shid, report);
+}
+
+static int spi_hid_sync_request(struct spi_hid *shid,
+ struct spi_hid_output_report *report)
+{
+ struct device *dev = &shid->spi->dev;
+ int error;
+
+ guard(mutex)(&shid->output_lock);
+
+ reinit_completion(&shid->output_done);
+
+ error = __spi_hid_send_output_report(shid, report);
+ if (error)
+ return error;
+
+ error = wait_for_completion_interruptible_timeout(&shid->output_done,
+ msecs_to_jiffies(SPI_HID_RESP_TIMEOUT));
+ if (error == 0) {
+ dev_err(dev, "Response timed out\n");
+ return -ETIMEDOUT;
+ }
+ if (error < 0)
+ return error;
+
+ return 0;
+}
+
+/*
+ * This function returns the length of the report descriptor, or a negative
+ * error code if something went wrong.
+ */
+static int spi_hid_report_descriptor_request(struct spi_hid *shid)
+{
+ struct device *dev = &shid->spi->dev;
+ struct spi_hid_output_report report = {
+ .report_type = REPORT_DESCRIPTOR,
+ .content_length = 0,
+ .content_id = SPI_HID_OUTPUT_REPORT_CONTENT_ID_DESC_REQUEST,
+ .content = NULL,
+ };
+ int ret;
+
+ ret = spi_hid_sync_request(shid, &report);
+ if (ret) {
+ dev_err(dev,
+ "Expected report descriptor not received: %d\n", ret);
+ return ret;
+ }
+
+ ret = shid->response_length;
+ if (ret != shid->desc.report_descriptor_length) {
+ ret = min_t(unsigned int, ret, shid->desc.report_descriptor_length);
+ dev_err(dev, "Received report descriptor length doesn't match device descriptor field, using min of the two: %d\n",
+ ret);
+ }
+
+ return ret;
+}
+
+static int spi_hid_create_device(struct spi_hid *shid)
+{
+ struct hid_device *hid;
+ struct device *dev = &shid->spi->dev;
+ int error;
+
+ hid = hid_allocate_device();
+ error = PTR_ERR_OR_ZERO(hid);
+ if (error) {
+ dev_err(dev, "Failed to allocate hid device: %d\n", error);
+ return error;
+ }
+
+ hid->driver_data = shid->spi;
+ hid->ll_driver = &spi_hid_ll_driver;
+ hid->dev.parent = &shid->spi->dev;
+ hid->bus = BUS_SPI;
+ hid->version = shid->desc.hid_version;
+ hid->vendor = shid->desc.vendor_id;
+ hid->product = shid->desc.product_id;
+
+ snprintf(hid->name, sizeof(hid->name), "spi %04X:%04X",
+ hid->vendor, hid->product);
+ strscpy(hid->phys, dev_name(&shid->spi->dev), sizeof(hid->phys));
+
+ shid->hid = hid;
+
+ error = hid_add_device(hid);
+ if (error) {
+ dev_err(dev, "Failed to add hid device: %d\n", error);
+ /*
+ * We likely got here because report descriptor request timed
+ * out. Let's disconnect and destroy the hid_device structure.
+ */
+ spi_hid_stop_hid(shid);
+ return error;
+ }
+
+ return 0;
+}
+
+static int spi_hid_get_request(struct spi_hid *shid, u8 content_id)
+{
+ struct device *dev = &shid->spi->dev;
+ struct spi_hid_output_report report = {
+ .report_type = GET_FEATURE,
+ .content_length = 0,
+ .content_id = content_id,
+ .content = NULL,
+ };
+ int error;
+
+ error = spi_hid_sync_request(shid, &report);
+ if (error) {
+ dev_err(dev,
+ "Expected get request response not received! Error %d\n",
+ error);
+ return error;
+ }
+
+ return 0;
+}
+
+static int spi_hid_set_request(struct spi_hid *shid, u8 *arg_buf, u16 arg_len,
+ u8 content_id)
+{
+ struct spi_hid_output_report report = {
+ .report_type = SET_FEATURE,
+ .content_length = arg_len,
+ .content_id = content_id,
+ .content = arg_buf,
+ };
+
+ return spi_hid_sync_request(shid, &report);
+}
+
+/* This is a placeholder. Will be implemented in the next patch. */
static irqreturn_t spi_hid_dev_irq(int irq, void *_shid)
{
return IRQ_HANDLED;
}
+static int spi_hid_alloc_buffers(struct spi_hid *shid, size_t report_size)
+{
+ struct device *dev = &shid->spi->dev;
+ int inbufsize = round_up(sizeof(shid->input->header) +
+ sizeof(shid->input->body) + report_size, 4);
+ int outbufsize = round_up(sizeof(shid->output->header) + report_size, 4);
+ void *tmp;
+
+ tmp = devm_krealloc(dev, shid->output, outbufsize, GFP_KERNEL | __GFP_ZERO);
+ if (!tmp)
+ return -ENOMEM;
+ shid->output = tmp;
+
+ tmp = devm_krealloc(dev, shid->input, inbufsize, GFP_KERNEL | __GFP_ZERO);
+ if (!tmp)
+ return -ENOMEM;
+ shid->input = tmp;
+
+ tmp = devm_krealloc(dev, shid->response, inbufsize, GFP_KERNEL | __GFP_ZERO);
+ if (!tmp)
+ return -ENOMEM;
+ shid->response = tmp;
+
+ if (!shid->output || !shid->input || !shid->response)
+ return -ENOMEM;
+
+ shid->bufsize = report_size;
+
+ return 0;
+}
+
+static int spi_hid_get_report_length(struct hid_report *report)
+{
+ return DIV_ROUND_UP(report->size, 8) +
+ report->device->report_enum[report->type].numbered + 2;
+}
+
+/*
+ * Traverse the supplied list of reports and find the longest
+ */
+static void spi_hid_find_max_report(struct hid_device *hid, u32 type,
+ u16 *max)
+{
+ struct hid_report *report;
+ u16 size;
+
+ /*
+ * We should not rely on wMaxInputLength, as some devices may set it to
+ * a wrong length.
+ */
+ list_for_each_entry(report, &hid->report_enum[type].report_list, list) {
+ size = spi_hid_get_report_length(report);
+ if (*max < size)
+ *max = size;
+ }
+}
+
+/* hid_ll_driver interface functions */
+
+static int spi_hid_ll_start(struct hid_device *hid)
+{
+ struct spi_device *spi = hid->driver_data;
+ struct spi_hid *shid = spi_get_drvdata(spi);
+ int error = 0;
+ u16 bufsize = 0;
+
+ spi_hid_find_max_report(hid, HID_INPUT_REPORT, &bufsize);
+ spi_hid_find_max_report(hid, HID_OUTPUT_REPORT, &bufsize);
+ spi_hid_find_max_report(hid, HID_FEATURE_REPORT, &bufsize);
+
+ if (bufsize < HID_MIN_BUFFER_SIZE) {
+ dev_err(&spi->dev,
+ "HID_MIN_BUFFER_SIZE > max_input_length (%d)\n",
+ bufsize);
+ return -EINVAL;
+ }
+
+ if (bufsize > shid->bufsize) {
+ guard(disable_irq)(&shid->spi->irq);
+
+ error = spi_hid_alloc_buffers(shid, bufsize);
+ if (error)
+ return error;
+ }
+
+ return 0;
+}
+
+static void spi_hid_ll_stop(struct hid_device *hid)
+{
+ hid->claimed = 0;
+}
+
+static int spi_hid_ll_open(struct hid_device *hid)
+{
+ struct spi_device *spi = hid->driver_data;
+ struct spi_hid *shid = spi_get_drvdata(spi);
+
+ set_bit(SPI_HID_READY, &shid->flags);
+ return 0;
+}
+
+static void spi_hid_ll_close(struct hid_device *hid)
+{
+ struct spi_device *spi = hid->driver_data;
+ struct spi_hid *shid = spi_get_drvdata(spi);
+
+ clear_bit(SPI_HID_READY, &shid->flags);
+ shid->reset_attempts = 0;
+}
+
+static int spi_hid_ll_power(struct hid_device *hid, int level)
+{
+ struct spi_device *spi = hid->driver_data;
+ struct spi_hid *shid = spi_get_drvdata(spi);
+ int error = 0;
+
+ guard(mutex)(&shid->output_lock);
+ if (!shid->hid)
+ error = -ENODEV;
+
+ return error;
+}
+
+static int spi_hid_ll_parse(struct hid_device *hid)
+{
+ struct spi_device *spi = hid->driver_data;
+ struct spi_hid *shid = spi_get_drvdata(spi);
+ struct device *dev = &spi->dev;
+ unsigned int rsize = shid->desc.report_descriptor_length;
+ int error, len;
+
+ if (rsize > HID_MAX_DESCRIPTOR_SIZE) {
+ dev_err(dev,
+ "Report descriptor size %d is greater than HID_MAX_DESCRIPTOR_SIZE %d\n",
+ rsize, HID_MAX_DESCRIPTOR_SIZE);
+ return -EINVAL;
+ }
+
+ if (rsize > shid->bufsize) {
+ error = spi_hid_alloc_buffers(shid, rsize);
+ if (error)
+ return error;
+ }
+
+ len = spi_hid_report_descriptor_request(shid);
+ if (len < 0) {
+ dev_err(dev, "Report descriptor request failed, %d\n", len);
+ return len;
+ }
+
+ /*
+ * FIXME: below call returning 0 doesn't mean that the report descriptor
+ * is good. We might be caching a crc32 of a corrupted r. d. or who
+ * knows what the FW sent. Need to have a feedback loop about r. d.
+ * being ok and only then cache it.
+ */
+ error = hid_parse_report(hid, (u8 *)shid->response->content, len);
+ if (error) {
+ dev_err(dev, "failed parsing report: %d\n", error);
+ return error;
+ }
+ shid->report_descriptor_crc32 = crc32_le(0,
+ (unsigned char const *)shid->response->content,
+ len);
+
+ return 0;
+}
+
+static int spi_hid_ll_raw_request(struct hid_device *hid,
+ unsigned char reportnum, __u8 *buf,
+ size_t len, unsigned char rtype, int reqtype)
+{
+ struct spi_device *spi = hid->driver_data;
+ struct spi_hid *shid = spi_get_drvdata(spi);
+ struct device *dev = &spi->dev;
+ int ret;
+
+ switch (reqtype) {
+ case HID_REQ_SET_REPORT:
+ if (buf[0] != reportnum) {
+ dev_err(dev, "report id mismatch\n");
+ return -EINVAL;
+ }
+
+ ret = spi_hid_set_request(shid, &buf[1], len - 1,
+ reportnum);
+ if (ret) {
+ dev_err(dev, "failed to set report\n");
+ return ret;
+ }
+
+ ret = len;
+ break;
+ case HID_REQ_GET_REPORT:
+ ret = spi_hid_get_request(shid, reportnum);
+ if (ret) {
+ dev_err(dev, "failed to get report\n");
+ return ret;
+ }
+
+ ret = min_t(size_t, len,
+ (shid->response->body[1] | (shid->response->body[2] << 8)) + 1);
+ buf[0] = shid->response->body[3];
+ memcpy(&buf[1], &shid->response->content, ret);
+ break;
+ default:
+ dev_err(dev, "invalid request type\n");
+ return -EIO;
+ }
+
+ return ret;
+}
+
+static int spi_hid_ll_output_report(struct hid_device *hid, __u8 *buf,
+ size_t len)
+{
+ struct spi_device *spi = hid->driver_data;
+ struct spi_hid *shid = spi_get_drvdata(spi);
+ struct device *dev = &spi->dev;
+ struct spi_hid_output_report report = {
+ .report_type = OUTPUT_REPORT,
+ .content_length = len - 1,
+ .content_id = buf[0],
+ .content = &buf[1],
+ };
+ int error;
+
+ if (!test_bit(SPI_HID_READY, &shid->flags)) {
+ dev_err(dev, "%s called in unready state\n", __func__);
+ return -ENODEV;
+ }
+
+ if (shid->desc.no_output_report_ack)
+ error = spi_hid_send_output_report(shid, &report);
+ else
+ error = spi_hid_sync_request(shid, &report);
+
+ if (error) {
+ dev_err(dev, "failed to send output report\n");
+ return error;
+ }
+
+ return len;
+}
+
+static struct hid_ll_driver spi_hid_ll_driver = {
+ .start = spi_hid_ll_start,
+ .stop = spi_hid_ll_stop,
+ .open = spi_hid_ll_open,
+ .close = spi_hid_ll_close,
+ .power = spi_hid_ll_power,
+ .parse = spi_hid_ll_parse,
+ .output_report = spi_hid_ll_output_report,
+ .raw_request = spi_hid_ll_raw_request,
+};
+
static ssize_t bus_error_count_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
@@ -159,6 +704,18 @@ int spi_hid_core_probe(struct spi_device *spi, struct spihid_ops *ops,
spi_set_drvdata(spi, shid);
+ mutex_init(&shid->output_lock);
+ init_completion(&shid->output_done);
+
+ /*
+ * we need to allocate the buffer without knowing the maximum
+ * size of the reports. Let's use SZ_2K, then we do the
+ * real computation later.
+ */
+ error = spi_hid_alloc_buffers(shid, SZ_2K);
+ if (error)
+ return error;
+
/*
* At the end of probe we initialize the device:
* 0) assert reset, bias the interrupt line
@@ -191,6 +748,10 @@ int spi_hid_core_probe(struct spi_device *spi, struct spihid_ops *ops,
dev_dbg(dev, "%s: d3 -> %s\n", __func__,
spi_hid_power_mode_string(shid->power_state));
+ error = spi_hid_create_device(shid);
+ if (error)
+ return error;
+
return 0;
}
EXPORT_SYMBOL_GPL(spi_hid_core_probe);
@@ -201,6 +762,8 @@ void spi_hid_core_remove(struct spi_device *spi)
struct device *dev = &spi->dev;
int error;
+ spi_hid_stop_hid(shid);
+
shid->ops->assert_reset(shid->ops);
error = shid->ops->power_down(shid->ops);
if (error)
--
2.54.0.1064.gd145956f57-goog
^ permalink raw reply related
* [PATCH v4 03/11] HID: spi-hid: add transport driver skeleton for HID over SPI bus
From: Jingyuan Liang @ 2026-06-09 4:40 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Jonathan Corbet, Mark Brown,
Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: linux-input, linux-doc, linux-kernel, linux-spi,
linux-trace-kernel, devicetree, hbarnor, tfiga, Jingyuan Liang,
Angela Czubak, Dmitry Antipov
In-Reply-To: <20260609-send-upstream-v4-0-b843d5e6ced3@chromium.org>
From: Angela Czubak <acz@semihalf.com>
Create spi-hid folder and add Kconfig and Makefile for spi-hid driver.
Add basic device structure, definitions, and probe/remove functions.
Signed-off-by: Dmitry Antipov <dmanti@microsoft.com>
Signed-off-by: Angela Czubak <acz@semihalf.com>
Signed-off-by: Jingyuan Liang <jingyliang@chromium.org>
---
drivers/hid/Kconfig | 2 +
drivers/hid/Makefile | 2 +
drivers/hid/spi-hid/Kconfig | 15 +++
drivers/hid/spi-hid/Makefile | 9 ++
drivers/hid/spi-hid/spi-hid-core.c | 213 +++++++++++++++++++++++++++++++++++++
5 files changed, 241 insertions(+)
diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index 920a64b66b25..c6ae23bfb75d 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -1434,6 +1434,8 @@ source "drivers/hid/bpf/Kconfig"
source "drivers/hid/i2c-hid/Kconfig"
+source "drivers/hid/spi-hid/Kconfig"
+
source "drivers/hid/intel-ish-hid/Kconfig"
source "drivers/hid/amd-sfh-hid/Kconfig"
diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
index 361a7daedeb8..6b43e789b39a 100644
--- a/drivers/hid/Makefile
+++ b/drivers/hid/Makefile
@@ -169,6 +169,8 @@ obj-$(CONFIG_USB_KBD) += usbhid/
obj-$(CONFIG_I2C_HID_CORE) += i2c-hid/
+obj-$(CONFIG_SPI_HID_CORE) += spi-hid/
+
obj-$(CONFIG_INTEL_ISH_HID) += intel-ish-hid/
obj-$(CONFIG_AMD_SFH_HID) += amd-sfh-hid/
diff --git a/drivers/hid/spi-hid/Kconfig b/drivers/hid/spi-hid/Kconfig
new file mode 100644
index 000000000000..836fdefe8345
--- /dev/null
+++ b/drivers/hid/spi-hid/Kconfig
@@ -0,0 +1,15 @@
+# SPDX-License-Identifier: GPL-2.0-only
+#
+# Copyright (c) 2021 Microsoft Corporation
+#
+
+menuconfig SPI_HID
+ tristate "SPI HID support"
+ default y
+ depends on SPI
+
+if SPI_HID
+
+config SPI_HID_CORE
+ tristate
+endif
diff --git a/drivers/hid/spi-hid/Makefile b/drivers/hid/spi-hid/Makefile
new file mode 100644
index 000000000000..92e24cddbfc2
--- /dev/null
+++ b/drivers/hid/spi-hid/Makefile
@@ -0,0 +1,9 @@
+# SPDX-License-Identifier: GPL-2.0-only
+#
+# Makefile for the SPI HID input drivers
+#
+# Copyright (c) 2021 Microsoft Corporation
+#
+
+obj-$(CONFIG_SPI_HID_CORE) += spi-hid.o
+spi-hid-objs = spi-hid-core.o
diff --git a/drivers/hid/spi-hid/spi-hid-core.c b/drivers/hid/spi-hid/spi-hid-core.c
new file mode 100644
index 000000000000..02a7608c4b88
--- /dev/null
+++ b/drivers/hid/spi-hid/spi-hid-core.c
@@ -0,0 +1,213 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * HID over SPI protocol implementation
+ *
+ * Copyright (c) 2021 Microsoft Corporation
+ * Copyright (c) 2026 Google LLC
+ *
+ * This code is partly based on "HID over I2C protocol implementation:
+ *
+ * Copyright (c) 2012 Benjamin Tissoires <benjamin.tissoires@gmail.com>
+ * Copyright (c) 2012 Ecole Nationale de l'Aviation Civile, France
+ * Copyright (c) 2012 Red Hat, Inc
+ *
+ * which in turn is partly based on "USB HID support for Linux":
+ *
+ * Copyright (c) 1999 Andreas Gal
+ * Copyright (c) 2000-2005 Vojtech Pavlik <vojtech@suse.cz>
+ * Copyright (c) 2005 Michael Haboustak <mike-@cinci.rr.com> for Concept2, Inc
+ * Copyright (c) 2007-2008 Oliver Neukum
+ * Copyright (c) 2006-2010 Jiri Kosina
+ */
+
+#include <linux/device.h>
+#include <linux/hid.h>
+#include <linux/hid-over-spi.h>
+#include <linux/interrupt.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/spi/spi.h>
+
+/* struct spi_hid_conf - Conf provided to the core */
+struct spi_hid_conf {
+ u32 input_report_header_address;
+ u32 input_report_body_address;
+ u32 output_report_address;
+ u8 read_opcode;
+ u8 write_opcode;
+};
+
+/**
+ * struct spihid_ops - Ops provided to the core
+ * @power_up: do sequencing to power up the device
+ * @power_down: do sequencing to power down the device
+ * @assert_reset: do sequencing to assert the reset line
+ * @deassert_reset: do sequencing to deassert the reset line
+ * @sleep_minimal_reset_delay: minimal sleep delay during reset
+ */
+struct spihid_ops {
+ int (*power_up)(struct spihid_ops *ops);
+ int (*power_down)(struct spihid_ops *ops);
+ int (*assert_reset)(struct spihid_ops *ops);
+ int (*deassert_reset)(struct spihid_ops *ops);
+ void (*sleep_minimal_reset_delay)(struct spihid_ops *ops);
+};
+
+/* Driver context */
+struct spi_hid {
+ struct spi_device *spi; /* spi device. */
+ struct hid_device *hid; /* pointer to corresponding HID dev. */
+
+ struct spihid_ops *ops;
+ struct spi_hid_conf *conf;
+
+ enum hidspi_power_state power_state;
+
+ u32 regulator_error_count;
+ int regulator_last_error;
+ u32 bus_error_count;
+ int bus_last_error;
+ u32 dir_count; /* device initiated reset count. */
+};
+
+static const char *spi_hid_power_mode_string(enum hidspi_power_state power_state)
+{
+ switch (power_state) {
+ case HIDSPI_ON:
+ return "d0";
+ case HIDSPI_SLEEP:
+ return "d2";
+ case HIDSPI_OFF:
+ return "d3";
+ default:
+ return "unknown";
+ }
+}
+
+static irqreturn_t spi_hid_dev_irq(int irq, void *_shid)
+{
+ return IRQ_HANDLED;
+}
+
+static ssize_t bus_error_count_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct spi_hid *shid = dev_get_drvdata(dev);
+
+ return sysfs_emit(buf, "%u (%d)\n",
+ shid->bus_error_count, shid->bus_last_error);
+}
+static DEVICE_ATTR_RO(bus_error_count);
+
+static ssize_t regulator_error_count_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct spi_hid *shid = dev_get_drvdata(dev);
+
+ return sysfs_emit(buf, "%u (%d)\n",
+ shid->regulator_error_count,
+ shid->regulator_last_error);
+}
+static DEVICE_ATTR_RO(regulator_error_count);
+
+static ssize_t device_initiated_reset_count_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct spi_hid *shid = dev_get_drvdata(dev);
+
+ return sysfs_emit(buf, "%u\n", shid->dir_count);
+}
+static DEVICE_ATTR_RO(device_initiated_reset_count);
+
+static struct attribute *spi_hid_attrs[] = {
+ &dev_attr_bus_error_count.attr,
+ &dev_attr_regulator_error_count.attr,
+ &dev_attr_device_initiated_reset_count.attr,
+ NULL /* Terminator */
+};
+
+static const struct attribute_group spi_hid_group = {
+ .attrs = spi_hid_attrs,
+};
+
+const struct attribute_group *spi_hid_groups[] = {
+ &spi_hid_group,
+ NULL
+};
+EXPORT_SYMBOL_GPL(spi_hid_groups);
+
+int spi_hid_core_probe(struct spi_device *spi, struct spihid_ops *ops,
+ struct spi_hid_conf *conf)
+{
+ struct device *dev = &spi->dev;
+ struct spi_hid *shid;
+ int error;
+
+ if (spi->irq <= 0)
+ return dev_err_probe(dev, spi->irq ?: -EINVAL, "Missing IRQ\n");
+
+ shid = devm_kzalloc(dev, sizeof(*shid), GFP_KERNEL);
+ if (!shid)
+ return -ENOMEM;
+
+ shid->spi = spi;
+ shid->power_state = HIDSPI_ON;
+ shid->ops = ops;
+ shid->conf = conf;
+
+ spi_set_drvdata(spi, shid);
+
+ /*
+ * At the end of probe we initialize the device:
+ * 0) assert reset, bias the interrupt line
+ * 1) sleep minimal reset delay
+ * 2) request IRQ
+ * 3) power up the device
+ * 4) deassert reset (high)
+ * After this we expect an IRQ with a reset response.
+ */
+
+ shid->ops->assert_reset(shid->ops);
+
+ shid->ops->sleep_minimal_reset_delay(shid->ops);
+
+ error = devm_request_threaded_irq(dev, spi->irq, NULL, spi_hid_dev_irq,
+ IRQF_ONESHOT, dev_name(&spi->dev), shid);
+ if (error) {
+ dev_err(dev, "%s: unable to request threaded IRQ\n", __func__);
+ return error;
+ }
+
+ error = shid->ops->power_up(shid->ops);
+ if (error) {
+ dev_err(dev, "%s: could not power up\n", __func__);
+ return error;
+ }
+
+ shid->ops->deassert_reset(shid->ops);
+
+ dev_dbg(dev, "%s: d3 -> %s\n", __func__,
+ spi_hid_power_mode_string(shid->power_state));
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(spi_hid_core_probe);
+
+void spi_hid_core_remove(struct spi_device *spi)
+{
+ struct spi_hid *shid = spi_get_drvdata(spi);
+ struct device *dev = &spi->dev;
+ int error;
+
+ shid->ops->assert_reset(shid->ops);
+ error = shid->ops->power_down(shid->ops);
+ if (error)
+ dev_err(dev, "failed to disable regulator\n");
+}
+EXPORT_SYMBOL_GPL(spi_hid_core_remove);
+
+MODULE_DESCRIPTION("HID over SPI transport driver");
+MODULE_AUTHOR("Dmitry Antipov <dmanti@microsoft.com>");
+MODULE_LICENSE("GPL");
--
2.54.0.1064.gd145956f57-goog
^ permalink raw reply related
* [PATCH v4 02/11] HID: Add BUS_SPI support and define HID_SPI_DEVICE macro
From: Jingyuan Liang @ 2026-06-09 4:40 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Jonathan Corbet, Mark Brown,
Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: linux-input, linux-doc, linux-kernel, linux-spi,
linux-trace-kernel, devicetree, hbarnor, tfiga, Jingyuan Liang,
Jarrett Schultz, Dmitry Antipov
In-Reply-To: <20260609-send-upstream-v4-0-b843d5e6ced3@chromium.org>
From: Jarrett Schultz <jaschultz@microsoft.com>
If connecting a hid_device with bus field indicating BUS_SPI print out
"SPI" in the debug print.
Macro sets the bus field to BUS_SPI and uses arguments to set vendor
product fields.
Signed-off-by: Dmitry Antipov <dmanti@microsoft.com>
Reviewed-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Signed-off-by: Jingyuan Liang <jingyliang@chromium.org>
---
drivers/hid/hid-core.c | 3 +++
include/linux/hid.h | 2 ++
2 files changed, 5 insertions(+)
diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index a5b3a8ca2fcb..813c9c743ccd 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -2316,6 +2316,9 @@ int hid_connect(struct hid_device *hdev, unsigned int connect_mask)
case BUS_I2C:
bus = "I2C";
break;
+ case BUS_SPI:
+ bus = "SPI";
+ break;
case BUS_SDW:
bus = "SOUNDWIRE";
break;
diff --git a/include/linux/hid.h b/include/linux/hid.h
index dce862cafbbd..957f322a0ebd 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -786,6 +786,8 @@ struct hid_descriptor {
.bus = BUS_BLUETOOTH, .vendor = (ven), .product = (prod)
#define HID_I2C_DEVICE(ven, prod) \
.bus = BUS_I2C, .vendor = (ven), .product = (prod)
+#define HID_SPI_DEVICE(ven, prod) \
+ .bus = BUS_SPI, .vendor = (ven), .product = (prod)
#define HID_REPORT_ID(rep) \
.report_type = (rep)
--
2.54.0.1064.gd145956f57-goog
^ permalink raw reply related
* [PATCH v4 01/11] Documentation: Correction in HID output_report callback description.
From: Jingyuan Liang @ 2026-06-09 4:40 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Jonathan Corbet, Mark Brown,
Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: linux-input, linux-doc, linux-kernel, linux-spi,
linux-trace-kernel, devicetree, hbarnor, tfiga, Jingyuan Liang,
Jarrett Schultz, Dmitry Antipov
In-Reply-To: <20260609-send-upstream-v4-0-b843d5e6ced3@chromium.org>
From: Jarrett Schultz <jaschultz@microsoft.com>
Originally output_report callback was described as must-be asynchronous,
but that is not the case in some implementations, namely i2c-hid.
Correct the documentation to say that it may be asynchronous.
Signed-off-by: Dmitry Antipov <dmanti@microsoft.com>
Reviewed-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Signed-off-by: Jingyuan Liang <jingyliang@chromium.org>
---
Documentation/hid/hid-transport.rst | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/Documentation/hid/hid-transport.rst b/Documentation/hid/hid-transport.rst
index 6f1692da296c..2008cf432af1 100644
--- a/Documentation/hid/hid-transport.rst
+++ b/Documentation/hid/hid-transport.rst
@@ -327,8 +327,8 @@ The available HID callbacks are:
Send raw output report via intr channel. Used by some HID device drivers
which require high throughput for outgoing requests on the intr channel. This
- must not cause SET_REPORT calls! This must be implemented as asynchronous
- output report on the intr channel!
+ must not cause SET_REPORT calls! This call might be asynchronous, so the
+ caller should not expect an immediate response!
::
--
2.54.0.1064.gd145956f57-goog
^ permalink raw reply related
* [PATCH v4 00/11] Add spi-hid transport driver
From: Jingyuan Liang @ 2026-06-09 4:40 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Jonathan Corbet, Mark Brown,
Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: linux-input, linux-doc, linux-kernel, linux-spi,
linux-trace-kernel, devicetree, hbarnor, tfiga, Jingyuan Liang,
Jarrett Schultz, Dmitry Antipov, Angela Czubak
This series picks up the spi-hid driver work originally started by
Microsoft. The patch breakdown has been modified and the implementation
has been refactored to address upstream feedback and testing issues. We
are submitting this as a new series while keeping the original sign-off
chain to reflect the history.
Same as the original series, there is a change to HID documentation, some
HID core changes to support a SPI device, the SPI HID transport driver,
and HID over SPI Device Tree binding. We have added the HID over SPI ACPI
support, power management, panel follower, and quirks for Ilitek touch
controllers.
Original authors: Jarrett Schultz <jaschultz@microsoft.com>,
Dmitry Antipov <dmanti@microsoft.com>
Link: https://lore.kernel.org/r/86b63b7b-afda-d7f4-7bfa-175085d5a8ef@gmail.com
Signed-off-by: Jingyuan Liang <jingyliang@chromium.org>
---
Changes in v4:
- Extended io_lock scope to protect the shid->hid pointer lifecycle
against races with the IRQ handler
- Cacheline-aligned DMA buffers, enforced 4-byte alignment
- Added error rollback for failed suspend/resume transitions
- Moved IRQ request to probe with IRQF_NO_AUTOEN (disabled by default)
- DT Bindings & OF Driver:
- Required a device-specific compatible string in the schema.
- Added description to explain why opcodes and addresses properties
need to be defined in the schema
- Added `spi-peripheral-props.yaml` reference and switched to
`unevaluatedProperties: false`.
- Added fallback to default timing parameters in OF driver if match
data is missing.
- Link to v3: https://lore.kernel.org/r/20260402-send-upstream-v3-0-6091c458d357@chromium.org
Changes in v3:
- Add io_lock init
- Relocate tracepoints to drivers/hid/spi-hid/ and fix tracepoint macros
- Add tracepoints for sync, error handling, reset, and report processing
- Clean up internal includes and fix Makefile CFLAGS
- Add more details in v2 changelog
- Link to v2: https://lore.kernel.org/r/20260324-send-upstream-v2-0-521ce8afff86@chromium.org
Changes in v2:
- Clean up DT bindings: fix formatting and remove timing and flags properties
- Update DT binding example: use a device-specific compatible and drop
reset_assert
- Simplify ACPI/OF match tables by removing ACPI_PTR/of_match_ptr
- Refactor OF driver to use match data for timing parameters instead
of DT properties
- Switch to fsleep() for delays in ACPI and OF drivers
- Drop patch 12 as it is vendor specific
- Add a lock to fix input/output concurrency race
- Link to v1: https://lore.kernel.org/r/20260303-send-upstream-v1-0-1515ba218f3d@chromium.org
---
Angela Czubak (2):
HID: spi-hid: add transport driver skeleton for HID over SPI bus
HID: spi_hid: add ACPI support for SPI over HID
Jarrett Schultz (3):
Documentation: Correction in HID output_report callback description.
HID: Add BUS_SPI support and define HID_SPI_DEVICE macro
HID: spi_hid: add device tree support for SPI over HID
Jingyuan Liang (6):
HID: spi-hid: add spi-hid driver HID layer
HID: spi-hid: add HID SPI protocol implementation
HID: spi_hid: add spi_hid traces
dt-bindings: input: Document hid-over-spi DT schema
HID: spi-hid: add power management implementation
HID: spi-hid: add panel follower support
.../devicetree/bindings/input/hid-over-spi.yaml | 128 ++
Documentation/hid/hid-transport.rst | 4 +-
drivers/hid/Kconfig | 2 +
drivers/hid/Makefile | 2 +
drivers/hid/hid-core.c | 3 +
drivers/hid/spi-hid/Kconfig | 45 +
drivers/hid/spi-hid/Makefile | 12 +
drivers/hid/spi-hid/spi-hid-acpi.c | 254 ++++
drivers/hid/spi-hid/spi-hid-core.c | 1521 ++++++++++++++++++++
drivers/hid/spi-hid/spi-hid-core.h | 98 ++
drivers/hid/spi-hid/spi-hid-of.c | 247 ++++
drivers/hid/spi-hid/spi-hid-trace.h | 169 +++
drivers/hid/spi-hid/spi-hid.h | 46 +
include/linux/hid.h | 2 +
14 files changed, 2531 insertions(+), 2 deletions(-)
---
base-commit: 05f7e89ab9731565d8a62e3b5d1ec206485eeb0b
change-id: 20260212-send-upstream-75f6fd9ed92e
Best regards,
--
Jingyuan Liang <jingyliang@chromium.org>
^ permalink raw reply
* Re: [PATCH 1/2] ring-buffer: Fix event length with forced 8-byte alignment
From: Hui Wang @ 2026-06-09 4:22 UTC (permalink / raw)
To: Steven Rostedt, Masami Hiramatsu (Google)
Cc: mathieu.desnoyers, pjw, linux-trace-kernel, shuah, wangfushuai,
linux-kselftest
In-Reply-To: <20260608125254.2598ef4e@fedora>
On 6/9/26 00:52, Steven Rostedt wrote:
> On Mon, 8 Jun 2026 18:02:45 +0900
> Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote:
>
>> On Sun, 7 Jun 2026 15:24:30 +0800
>> Hui Wang <hui.wang@canonical.com> wrote:
>>
>>> When RB_FORCE_8BYTE_ALIGNMENT is true, rb_calculate_event_length()
>>> reserves the space of event->array[0] for placing the data length and
>>> rb_update_event() stores the data length in event->array[0]
>>> accordingly. As a result the whole event length will add extra 4 bytes
>>> for sizeof(event.array[0]) unconditionally.
>>>
>>> But ring_buffer_event_length() only subtracts the
>>> sizeof(event->array[0]) for events larger than RB_MAX_SMALL_DATA +
>>> sizeof(event->array[0]). As a result, small events on architectures
>>> with RB_FORCE_8BYTE_ALIGNMENT=true report a data length that is 4
>>> bytes larger than expected.
>>>
>>> To fix it, add the RB_FORCE_8BYTE_ALIGNMENT as a condition to subtract
>>> the size of that length field whenever RB_FORCE_8BYTE_ALIGNMENT is
>>> true.
>>>
>>> This issue is observed in a riscv64 kernel with
>>> CONFIG_HAVE_64BIT_ALIGNED_ACCESS set to y, when we run ftrace selftest
>>> trace_marker_raw.tc, we get the weird log: for cases where the id is
>>> 1..100, the number of data field is 8*N, but once id exceeds 100, the
>>> number of data field becomes 8*N+4:
>>> # 1 buf: 58 00 00 00 80 5e d1 63 (number of data field is 8*1)
>>> ...
>>> # a buf: 58 ... (number of data field is 8*2)
>>> ...
>>> # 64 buf: 58 ... (number of data field is 8*13)
>>> # 65 buf: 58 ... (number of data field is 8*13+4)
>>>
>>> After applying this change, the number of data field keeps being 8*N+4
>>> consistently.
>>>
>> Good catch!
>>
>> This looks good to me.
>>
>> Reviewed-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> This is the patch I meant to reply to.
>
> NACK as the test is broken and not the kernel.
>
> There's a pending fix already:
>
> https://lore.kernel.org/all/20260601023251.1916483-1-dtcccc@linux.alibaba.com/
>
>
> -- Steve
Hi Steven,
Thanks for the pointer. I reverted my two patches and applied the patch
you referenced, but unfortunately it doesn't resolve the problem — the
testcase still fails in my environment (riscv64 kernel with
CONFIG_HAVE_64BIT_ALIGNED_ACCESS enabled).
From what I can tell, that fix addresses a different problem than the
one I'm hitting: it targets a 64K page-size issue, whereas my failure is
caused by the 64-bit alignment requirement
(CONFIG_HAVE_64BIT_ALIGNED_ACCESS). So I don't think they're the same
root cause.
So can you please take a look at them again.
Thanks,
Hui.
^ permalink raw reply
* [PATCH] mm/lruvec: trace LRU add drains and drain-all queuing
From: JP Kobryn @ 2026-06-09 4:11 UTC (permalink / raw)
To: linux-mm, willy, shakeel.butt, usama.arif, akpm, vbabka, mhocko,
rostedt, mhiramat, mathieu.desnoyers, kasong, qi.zheng, baohua,
axelrasmussen, yuanchu, weixugc, chrisl, shikemeng, nphamcs,
baoquan.he, youngjun.park
Cc: linux-kernel, linux-trace-kernel
LRU add batches can be drained before they reach capacity. This can be a
source of LRU lock contention, but it is not currently possible to
attribute these drains to callers with existing tracepoints.
Add mm_lru_add_drain to report the CPU and lru_add batch count when an
lru_add batch is drained. This allows tracing to distinguish full drains
from partial drains and attribute them to the calling stack.
Add mm_lru_drain_all_queue to report when lru_add_drain_all() queues
per-CPU drain work. This captures the requester stack and target CPU for
remote drain work. The event is named as a drain-all queue event because
the queued work can be needed for batches other than lru_add.
Signed-off-by: JP Kobryn <jp.kobryn@linux.dev>
---
include/trace/events/pagemap.h | 40 ++++++++++++++++++++++++++++++++++
mm/swap.c | 6 ++++-
2 files changed, 45 insertions(+), 1 deletion(-)
diff --git a/include/trace/events/pagemap.h b/include/trace/events/pagemap.h
index 171524d3526d..ea8fc46bedb0 100644
--- a/include/trace/events/pagemap.h
+++ b/include/trace/events/pagemap.h
@@ -77,6 +77,46 @@ TRACE_EVENT(mm_lru_activate,
TP_printk("folio=%p pfn=0x%lx", __entry->folio, __entry->pfn)
);
+TRACE_EVENT(mm_lru_add_drain,
+
+ TP_PROTO(int cpu, unsigned int nr),
+
+ TP_ARGS(cpu, nr),
+
+ TP_STRUCT__entry(
+ __field(int, cpu )
+ __field(unsigned int, nr )
+ ),
+
+ TP_fast_assign(
+ __entry->cpu = cpu;
+ __entry->nr = nr;
+ ),
+
+ TP_printk("cpu=%d nr=%u", __entry->cpu, __entry->nr)
+);
+
+TRACE_EVENT(mm_lru_drain_all_queue,
+
+ TP_PROTO(int target_cpu, bool force_all_cpus),
+
+ TP_ARGS(target_cpu, force_all_cpus),
+
+ TP_STRUCT__entry(
+ __field(int, target_cpu )
+ __field(bool, force_all_cpus )
+ ),
+
+ TP_fast_assign(
+ __entry->target_cpu = target_cpu;
+ __entry->force_all_cpus = force_all_cpus;
+ ),
+
+ TP_printk("target_cpu=%d force_all_cpus=%s",
+ __entry->target_cpu,
+ __entry->force_all_cpus ? "true" : "false")
+);
+
#endif /* _TRACE_PAGEMAP_H */
/* This part must be outside protection */
diff --git a/mm/swap.c b/mm/swap.c
index 588f50d8f1a8..c385b93582eb 100644
--- a/mm/swap.c
+++ b/mm/swap.c
@@ -694,9 +694,12 @@ void lru_add_drain_cpu(int cpu)
{
struct cpu_fbatches *fbatches = &per_cpu(cpu_fbatches, cpu);
struct folio_batch *fbatch = &fbatches->lru_add;
+ unsigned int nr_folios_add = folio_batch_count(fbatch);
- if (folio_batch_count(fbatch))
+ if (nr_folios_add) {
folio_batch_move_lru(fbatch, lru_add);
+ trace_mm_lru_add_drain(cpu, nr_folios_add);
+ }
fbatch = &fbatches->lru_move_tail;
/* Disabling interrupts below acts as a compiler barrier. */
@@ -928,6 +931,7 @@ static inline void __lru_add_drain_all(bool force_all_cpus)
if (cpu_needs_drain(cpu)) {
INIT_WORK(work, lru_add_drain_per_cpu);
queue_work_on(cpu, mm_percpu_wq, work);
+ trace_mm_lru_drain_all_queue(cpu, force_all_cpus);
__cpumask_set_cpu(cpu, &has_work);
}
}
--
2.54.0
^ permalink raw reply related
* Re: [PATCH v8 2/6] mm/memory-failure: surface unhandlable kernel pages as -ENOTRECOVERABLE
From: Miaohe Lin @ 2026-06-09 2:39 UTC (permalink / raw)
To: Breno Leitao, David Hildenbrand (Arm)
Cc: linux-mm, linux-kernel, linux-doc, linux-kselftest,
linux-trace-kernel, kernel-team, Lance Yang, Andrew Morton,
Lorenzo Stoakes, Vlastimil Babka, Mike Rapoport,
Suren Baghdasaryan, Michal Hocko, Shuah Khan, Naoya Horiguchi,
Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Jonathan Corbet, Shuah Khan, Liam R. Howlett
In-Reply-To: <aibN4osY_QF1Rejh@gmail.com>
On 2026/6/8 22:15, Breno Leitao wrote:
> On Fri, Jun 05, 2026 at 11:42:53AM +0200, David Hildenbrand (Arm) wrote:
>> On 6/5/26 11:35, Breno Leitao wrote:
>>> On Wed, Jun 03, 2026 at 10:33:04AM +0800, Miaohe Lin wrote:
>>>> On 2026/6/2 17:41, David Hildenbrand (Arm) wrote:
>>>>>
>>>>> Races are fine. We might miss some pages, but that can happen on races either way.
>>>>>
>>>>>
>>>>> I'd just do something like
>>>>>
>>>>> if (PageReserved(page))
>>>>> return true;
>>>>>
>>>>> head = compound_head(page);
>>>>
>>>> If @head is split just after compound_head. And then @head is freed into buddy and re-allocated as slab
>>>> page while @page is still in the buddy. We would panic on this scene as @head is PageSlab. But we were
>>>> supposed to successfully handle @page. Or am I miss something?
>>>
>>> You're right that it is racy, but I think it is an acceptable race here.
>>>
>>
>> I mean, any such races can currently already happen one way or the other?
>>
>> Really, the only way to not get races is to tryget the (compound)page,
>> revalidate that the page is still part of the compound page.
>>
>> I'm not sure if that's really a good idea.
>>
>> But my memory is a bit vague in which scenarios we already hold a page reference
>> here to prevent any concurrent freeing?
>
> No, we don't hold one here in the case that matters.
>
> HWPoisonKernelOwned() runs at the very top of get_any_page(), before
> try_again: and before __get_hwpoison_page(). The first refcount taken in
> the whole path is the folio_try_get() inside __get_hwpoison_page(), which
> runs *after* the short-circuit.
>
> So get_any_page() itself never holds a reference at the check -- the only way
> one exists is if the caller passed MF_COUNT_INCREASED (count_increased ==
> true).
>
> So on the MCE/GHES path -- the one this panic option exists for -- no
> reference is held when HWPoisonKernelOwned() does its compound_head() +
> PageSlab()/PageTable()/PageLargeKmalloc() checks.
>
> Given that, I'd rather keep it racy and take no refcount than add a
> tryget + revalidate purely for this check. As I've said earleir, an operator
Would it be acceptable to add a simple recheck? Something like below:
retry:
head = compound_head(page);
PageSlab()/PageTable()/PageLargeKmalloc() checks
if (head != compound_head(page))
goto retry
Thanks both.
.
> who enabled it has chosen to crash rather than run on corrupted memory;
> mis-attributing one such rare, genuinely-poisoned page is within that contract.
> .
>
^ permalink raw reply
* Re: [PATCH mm-unstable v19 11/14] mm/khugepaged: Introduce mTHP collapse support
From: Lance Yang @ 2026-06-09 1:52 UTC (permalink / raw)
To: david
Cc: npache, linux-doc, linux-kernel, linux-mm, linux-trace-kernel,
aarcange, akpm, anshuman.khandual, apopple, baohua, baolin.wang,
byungchul, catalin.marinas, cl, corbet, dave.hansen, dev.jain,
gourry, hannes, hughd, jack, jackmanb, jannh, jglisse,
joshua.hahnjy, kas, liam, ljs, mathieu.desnoyers, matthew.brost,
mhiramat, mhocko, peterx, pfalcato, rakie.kim, raquini, rdunlap,
richard.weiyang, rientjes, rostedt, rppt, ryan.roberts, shivankg,
sunnanyong, surenb, thomas.hellstrom, tiwai, usamaarif642, vbabka,
vishal.moola, wangkefeng.wang, will, willy, yang, ying.huang, ziy,
zokeefe
In-Reply-To: <20260608162612.27122-1-lance.yang@linux.dev>
On 2026/6/9 00:26, Lance Yang wrote:
>
> On Mon, Jun 08, 2026 at 04:56:37PM +0200, David Hildenbrand (Arm) wrote:
>> On 6/6/26 12:28, Lance Yang wrote:
>>>
>>> On Fri, Jun 05, 2026 at 10:14:18AM -0600, Nico Pache wrote:
>>>> Enable khugepaged to collapse to mTHP orders. This patch implements the
>>>> main scanning logic using a bitmap to track occupied pages and the
>>>> algorithm to find optimal collapse sizes.
>>>>
>>>> Previous to this patch, PMD collapse had 3 main phases, a light weight
>>>> scanning phase (mmap_read_lock) that determines a potential PMD
>>>> collapse, an alloc phase (mmap unlocked), then finally heavier collapse
>>>> phase (mmap_write_lock).
>>>>
>>>> To enabled mTHP collapse we make the following changes:
>>>>
>>>> During PMD scan phase, track occupied pages in a bitmap. When mTHP
>>>> orders are enabled, we remove the restriction of max_ptes_none during the
>>>> scan phase to avoid missing potential mTHP collapse candidates. Once we
>>>> have scanned the full PMD range and updated the bitmap to track occupied
>>>> pages, we use the bitmap to find the optimal mTHP size.
>>>>
>>>> Implement mthp_collapse() to walk forward through the bitmap and
>>>> determine the best eligible order for each naturally-aligned region. The
>>>> algorithm starts at the beginning of the PMD range and, for each offset,
>>>> tries the highest order that fits the alignment. If the number of
>>>> occupied PTEs in that region satisfies the max_ptes_none threshold for
>>>> that order, a collapse is attempted. On failure, the order is
>>>> decremented and the same offset is retried at the next smaller size. Once
>>>> the smallest enabled order is exhausted (or a collapse succeeds), the
>>>> offset advances past the region just processed, and the next attempt
>>>> starts at the highest order permitted by the new offset's natural
>>>> alignment.
>>>>
>>>> The algorithm works as follows:
>>>> 1) set offset=0 and order=HPAGE_PMD_ORDER
>>>> 2) if the order is not enabled, go to step (5)
>>>> 3) count occupied PTEs in the (offset, order) range using
>>>> bitmap_weight_from()
>>>> 4) if the count satisfies the max_ptes_none threshold, attempt
>>>> collapse; on success, advance to step (6)
>>>> 5) if a smaller enabled order exists, decrement order and retry
>>>> from step (2) at the same offset
>>>> 6) advance offset past the current region and compute the next
>>>> order from the new offset's natural alignment via __ffs(offset),
>>>> capped at HPAGE_PMD_ORDER
>>>> 7) repeat from step (2) until the full PMD range is covered
>>>>
>>>> mTHP collapses reject regions containing swapped out or shared pages.
>>>> This is because adding new entries can lead to new none pages, and these
>>>> may lead to constant promotion into a higher order mTHP. A similar
>>>> issue can occur with "max_ptes_none > HPAGE_PMD_NR/2" due to a collapse
>>>> introducing at least 2x the number of pages, and on a future scan will
>>>> satisfy the promotion condition once again. This issue is prevented via
>>>> the collapse_max_ptes_none() function which imposes the max_ptes_none
>>>> restrictions above.
>>>>
>>>> We currently only support mTHP collapse for max_ptes_none values of 0
>>>> and HPAGE_PMD_NR - 1. resulting in the following behavior:
>>>>
>>>> - max_ptes_none=0: Never introduce new empty pages during collapse
>>>> - max_ptes_none=HPAGE_PMD_NR-1: Always try collapse to the highest
>>>> available mTHP order
>>>>
>>>> Any other max_ptes_none value will emit a warning and default mTHP
>>>> collapse to max_ptes_none=0. There should be no behavior change for PMD
>>>> collapse.
>>>>
>>>> Once we determine what mTHP sizes fits best in that PMD range a collapse
>>>> is attempted. A minimum collapse order of 2 is used as this is the lowest
>>>> order supported by anon memory as defined by THP_ORDERS_ALL_ANON.
>>>>
>>>> Currently madv_collapse is not supported and will only attempt PMD
>>>> collapse.
>>>>
>>>> We can also remove the check for is_khugepaged inside the PMD scan as
>>>> the collapse_max_ptes_none() function handles this logic now.
>>>>
>>>> Signed-off-by: Nico Pache <npache@redhat.com>
>>>> ---
>>>> mm/khugepaged.c | 146 +++++++++++++++++++++++++++++++++++++++++++++---
>>>> 1 file changed, 138 insertions(+), 8 deletions(-)
>>>>
>>>> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
>>>> index ec886a031952..430047316f43 100644
>>>> --- a/mm/khugepaged.c
>>>> +++ b/mm/khugepaged.c
>>>> @@ -99,6 +99,8 @@ static DEFINE_READ_MOSTLY_HASHTABLE(mm_slots_hash, MM_SLOTS_HASH_BITS);
>>>>
>>>> static struct kmem_cache *mm_slot_cache __ro_after_init;
>>>>
>>>> +#define KHUGEPAGED_MIN_MTHP_ORDER 2
>>>> +
>>>> struct collapse_control {
>>>> bool is_khugepaged;
>>>>
>>>> @@ -110,6 +112,9 @@ struct collapse_control {
>>>>
>>>> /* nodemask for allocation fallback */
>>>> nodemask_t alloc_nmask;
>>>> +
>>>> + /* Each bit represents a single occupied (!none/zero) page. */
>>>> + DECLARE_BITMAP(mthp_present_ptes, MAX_PTRS_PER_PTE);
>>>> };
>>>>
>>>> /**
>>>> @@ -1440,20 +1445,130 @@ static enum scan_result collapse_huge_page(struct mm_struct *mm, unsigned long s
>>>> return result;
>>>> }
>>>>
>>>> +/* Return the highest naturally aligned order that fits at @offset within a PMD. */
>>>> +static unsigned int max_order_from_offset(unsigned int offset)
>>>> +{
>>>> + if (offset == 0)
>>>> + return HPAGE_PMD_ORDER;
>>>> +
>>>> + return min_t(unsigned int, __ffs(offset), HPAGE_PMD_ORDER);
>>>> +}
>>>> +
>>>> +/*
>>>> + * mthp_collapse() consumes the bitmap that is generated during
>>>> + * collapse_scan_pmd() to determine what regions and mTHP orders fit best.
>>>> + *
>>>> + * Each bit in cc->mthp_present_ptes represents a single occupied (!none/zero)
>>>> + * page. We start at the PMD order and check if it is eligible for collapse;
>>>> + * if not, we check the left and right halves of the PTE page table we are
>>>> + * examining at a lower order.
>>>> + *
>>>> + * For each of these, we determine how many PTE entries are occupied in the
>>>> + * range of PTE entries we propose to collapse, then we compare this to a
>>>> + * threshold number of PTE entries which would need to be occupied for a
>>>> + * collapse to be permitted at that order (accounting for max_ptes_none).
>>>> + *
>>>> + * If a collapse is permitted, we attempt to collapse the PTE range into a
>>>> + * mTHP.
>>>> + */
>>>> +static enum scan_result mthp_collapse(struct mm_struct *mm,
>>>> + unsigned long address, int referenced, int unmapped,
>>>> + struct collapse_control *cc, unsigned long enabled_orders)
>>>> +{
>>>> + unsigned int nr_occupied_ptes, nr_ptes, max_ptes_none;
>>>> + enum scan_result last_result = SCAN_FAIL;
>>>> + int collapsed = 0;
>>>> + bool alloc_failed = false;
>>>> + unsigned long collapse_address;
>>>> + unsigned int offset = 0;
>>>> + unsigned int order = HPAGE_PMD_ORDER;
>>>> +
>>>> + while (offset < HPAGE_PMD_NR) {
>>>> + nr_ptes = 1UL << order;
>>>> +
>>>> + if (!test_bit(order, &enabled_orders))
>>>> + goto next_order;
>>>> +
>>>> + max_ptes_none = collapse_max_ptes_none(cc, NULL, order);
>>>> + nr_occupied_ptes = bitmap_weight_from(cc->mthp_present_ptes, offset,
>>>> + offset + nr_ptes);
>>>> +
>>>> + if (nr_occupied_ptes >= nr_ptes - max_ptes_none) {
>>>
>>> Looks broken for swap PTEs in PMD collapse ...
>>>
>>> collapse_scan_pmd() allows them up to max_ptes_swap and record them in
>>> unmapped, but they don't get a bit in mthp_present_ptes. And then
>>> mthp_collapse() does the check above:
>>
>> Right. I assumed this is implicitly handled by the optimization in collapse_scan_pmd:
>>
>> if (enabled_orders != BIT(HPAGE_PMD_ORDER))
>> max_ptes_none = KHUGEPAGED_MAX_PTES_LIMIT;
>>
>> But we perform the check a second time.
>
> Note that once lower orders are enabled, the scan *relaxes* max_ptes_none
> only so it can cover the whole PMD and build the bitmap ...
>
>>>
>>> nr_occupied_ptes >= nr_ptes - max_ptes_none
>>>
>>> So max_ptes_none=0 + 511 present PTEs + one allowed swap PTE won't even
>>> call collapse_huge_page() for PMD order.
>>>
>>> Shouldn't we account for them in the PMD-order check? Something like:
>>>
>>> if (is_pmd_order(order))
>>> nr_occupied_ptes += unmapped;
>> As an alternative, we could either 1) skip the check there for
>> pmd order (as the check was already done); or 2) introduce+maintain
>
> Yeah, skipping the check would do the trick, since isolate will check
> max_ptes_none again later :)
In addition, that later check is rather late, we may have already
allocated the folio and swapped in pages before isolate rejects
the range :)
>> a bitmap that tracks non-present PTEs.
>>
>> @@ -1475,7 +1477,9 @@ static enum scan_result mthp_collapse(struct mm_struct *mm,
>> nr_occupied_ptes = bitmap_weight_from(cc->mthp_present_ptes, offset,
>> offset + nr_ptes);
>>
>> - if (nr_occupied_ptes >= nr_ptes - max_ptes_none) {
So I'd still slightly prefer keeping this check and accounting
unmapped for PMD order.
if (is_pmd_order(order))
nr_occupied_ptes += unmapped;
>> + /* Check was already done in the caller. */
>
> This check is not quite redundant for PMD order, though. It avoids
> entering collapse_huge_page() for a range that already exceeds
> max_ptes_none for that order.
>
>> + if (is_pmd_order(order) ||
>> + nr_occupied_ptes >= nr_ptes - max_ptes_none) {
>> enum scan_result ret;
>>
>> collapse_address = address + offset * PAGE_SIZE;
>>
>> 2) would probably be cleanest long-term.
>
> Yeah, Agreed.
^ permalink raw reply
* Re: [PATCH v3 0/6] bootconfig: embed kernel.* cmdline at build time
From: Masami Hiramatsu @ 2026-06-09 1:46 UTC (permalink / raw)
To: Breno Leitao
Cc: Andrew Morton, Nathan Chancellor, paulmck, Nicolas Schier,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, linux-kernel, linux-trace-kernel, linux-kbuild,
bpf, kernel-team
In-Reply-To: <20260608-bootconfig_using_tools-v3-0-4ddd079a0696@debian.org>
On Mon, 08 Jun 2026 09:23:57 -0700
Breno Leitao <leitao@debian.org> wrote:
> The userspace pieces (xbc_snprint_cmdline() in lib/, tools/bootconfig -C)
> already landed; this series wires the rendered cmdline into the kernel.
>
> Motivation: today the embedded bootconfig is parsed at runtime, after
> parse_early_param() has already run, so early_param() handlers can't
> see embedded values. Folding the kernel.* subtree into the cmdline at
> build time gives a CONFIG_CMDLINE-equivalent for embedded-bootconfig
> users without forcing them to maintain two cmdline sources.
>
> Behaviorally, the "kernel" subtree is rendered to a flat string at
> build time and stashed in .init.rodata. setup_arch() prepends it to
> boot_command_line before parse_early_param() runs. Overflow is a soft
> error: the helper logs and leaves boot_command_line untouched rather
> than panicking, so an oversized embedded bconf cannot brick a boot.
>
Sashiko still leaves some comments.
https://sashiko.dev/#/patchset/20260608-bootconfig_using_tools-v3-0-4ddd079a0696%40debian.org
BTW, can you also update the document (Documentation/admin-guide/bootconfig.rst)
about what is the expected behavior of this feature (kconfigs, examples)?
Thank you,
> Signed-off-by: Breno Leitao <leitao@debian.org>
> ---
> Changes in v3:
> - Patch 3: Move HOSTCC override to the kernel-side rule; tool keeps
> $(CC) for standalone/cross builds.
> - Patch 6: Drop the false fail-safe wording; document the
> BOOT_CONFIG_FORCE=y default interaction.
> - Link to v2:
> https://lore.kernel.org/r/20260605-bootconfig_using_tools-v2-0-d309f544b5f7@debian.org
>
> Changes in v2 (addressing review of v1):
> - Split out a standalone fix for the NULL-pointer arithmetic in
> xbc_snprint_cmdline() so the build-time render cannot trip host
> UBSan/FORTIFY_SOURCE.
> - Rework the leaf-root handling: instead of returning early, skip @root
> inside the loop so a root carrying both a value and subkeys
> (kernel = x together with kernel.foo = bar) still renders its
> descendant keys.
> - Build tools/bootconfig with $(HOSTCC) so cross-compiled (ARCH=...)
> builds render the cmdline on the build host instead of failing with
> "Exec format error".
> - Mark the embedded cmdline section read-only (drop the "w" flag from
> .init.rodata).
> - Add a make-clean hook so tools/bootconfig artifacts are removed by
> make clean.
> - Gate the x86 prepend on "bootconfig" being present on the command
> line (or CONFIG_BOOT_CONFIG_FORCE), matching the init.* opt-in
> semantics documented in bootconfig.rst and preserving fail-safe
> recovery: dropping "bootconfig" from the bootloader cmdline now also
> disables the embedded kernel.* keys.
> - Link to v1: https://patch.msgid.link/20260527-bootconfig_using_tools-v1-0-b6906a86e7d5@debian.org
>
> ---
> Breno Leitao (6):
> bootconfig: fix NULL-pointer arithmetic in xbc_snprint_cmdline()
> bootconfig: render descendant keys when xbc_snprint_cmdline() root has a value
> bootconfig: render embedded bootconfig as a kernel cmdline at build time
> bootconfig: clean build-time tools/bootconfig from make clean
> bootconfig: add xbc_prepend_embedded_cmdline() helper
> x86/setup: prepend embedded bootconfig cmdline before parse_early_param
>
> MAINTAINERS | 1 +
> Makefile | 24 +++++++++-
> arch/x86/Kconfig | 1 +
> arch/x86/kernel/setup.c | 16 +++++++
> include/linux/bootconfig.h | 9 ++++
> init/Kconfig | 36 +++++++++++++++
> init/main.c | 25 ++++++++--
> lib/Makefile | 16 +++++++
> lib/bootconfig.c | 112 ++++++++++++++++++++++++++++++++++++++++++---
> lib/embedded-cmdline.S | 16 +++++++
> tools/bootconfig/Makefile | 4 +-
> 11 files changed, 247 insertions(+), 13 deletions(-)
> ---
> base-commit: e7e28506af98ce4e1059e5ec59334b335c00a246
> change-id: 20260508-bootconfig_using_tools-cfa7aa9d6a5a
>
> Best regards,
> --
> Breno Leitao <leitao@debian.org>
>
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
^ permalink raw reply
* Re: [RFC PATCH 0/7] tracing/probes: Add more typecast features
From: Masami Hiramatsu @ 2026-06-09 1:42 UTC (permalink / raw)
To: Masami Hiramatsu (Google)
Cc: Steven Rostedt, Mathieu Desnoyers, Jonathan Corbet, Shuah Khan,
linux-kernel, linux-trace-kernel, linux-doc, linux-kselftest
In-Reply-To: <178092865666.163648.10457567771536160909.stgit@devnote2>
Hi,
On Mon, 8 Jun 2026 23:24:16 +0900
"Masami Hiramatsu (Google)" <mhiramat@kernel.org> wrote:
> Hi,
>
> Here is a series of patches to introduce more typecast features
> to probe events, which includes 1. expanding BTF typecast to
> fprobe and kprobe events, 2. introducing container_of like typecst
> option, 3. supporting nested typecast, 4. adding $current special
> variable support, 5. adding per-cpu dereference support, 6. adding
> a testcase to check typecasts.
Sashiko found many issues on this series. I'll fix those.
BTW, for $current, I think it should typecasted to task_struct without
typecast. (except if it is the container_of() type typecasting)
In this case, ctx->strcut_btf will be updated automatically if $current
is specified.
Also, I'm thinking redesign +CPU/+PCPU/this_cpu_ptr().
Instead of those, what about introducing followings?
- this_cpu_read(VAR)
- this_cpu_ptr(VAR)
Comments are welcome!
Thanks,
>
> Steve introduced BTF typecast feature for eprobe[1].
> This series extends it and add more options:
>
> 1. Expanding BTF typecast to kprobe and fprobe.
> (currently only function entry/exit)
>
> 2. Introduce container_of like typecast. This adds a "assigned
> member" option to the typecast.
>
> (STRUCT,MEMBER)VAR->ANOTHER_MEMBER
>
> This casts VAR to STRUCT type but the VAR is as the address
> of STRUCT.MEMBER. In C, it is:
>
> container_of(VAR, STRUCT, MEMBER)->ANOTHER_MEMBER
>
> 3. Support nested typecast, e.g.
>
> (STRUCT)((STRUCT2)VAR->MEMBER2)->MEMBER
>
> the nest level must be smaller than 3.
>
> 4. Add $current variable to point "current" task_struct.
> This is useful with typecast, e.g.
>
> (task_struct)$current->pid
>
> 5. per-cpu dereference support.
>
> +CPU(VAR) is the same as this_cpu_read(VAR), and
> +PCPU(VAR) is the same as this_cpu_ptr(VAR).
> Also, "this_cpu_ptr(VAR)" is available. This is good
> with nesting expression.
>
> (STRUCT)(this_cpu_ptr(VAR))->MEMBER
>
> (However, it might be better to allow a special way to omit
> parentheses for thi_cpu_ptr())
>
> And added a test script to test part of them.
>
> [1] https://lore.kernel.org/all/20260601130746.2139d926@gandalf.local.home/
>
>
> ---
>
> Masami Hiramatsu (Google) (7):
> tracing/probes: Support typecast for various probe events
> tracing/probes: Support nested typecast
> tracing/probes: Support field specifier option for typecast
> tracing/probes: Add $current variable support
> tracing/probes: Add +CPU() and +PCPU() dereference method to fetcharg
> tracing/probes: Support reserved this_cpu_ptr() method
> tracing/probes: Add a new testcase for BTF typecasts
>
>
> Documentation/trace/eprobetrace.rst | 11 +
> Documentation/trace/fprobetrace.rst | 11 +
> Documentation/trace/kprobetrace.rst | 12 +
> kernel/trace/trace.c | 6
> kernel/trace/trace_probe.c | 312 +++++++++++++++-----
> kernel/trace/trace_probe.h | 12 +
> kernel/trace/trace_probe_tmpl.h | 33 ++
> samples/trace_events/trace-events-sample.c | 38 ++
> samples/trace_events/trace-events-sample.h | 34 ++
> .../ftrace/test.d/dynevent/btf_probe_event.tc | 52 +++
> 10 files changed, 422 insertions(+), 99 deletions(-)
> create mode 100644 tools/testing/selftests/ftrace/test.d/dynevent/btf_probe_event.tc
>
> --
> Signature
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
^ 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