* [PATCH] Input: wm831x-ts - drain pen-down work at teardown
From: Fan Wu @ 2026-07-22 2:45 UTC (permalink / raw)
To: linux-input; +Cc: Dmitry Torokhov, linux-kernel, Fan Wu, stable
The pen-down and data IRQ handlers queue pd_data_work to switch between the
two IRQs. The worker obtains wm831x_ts with container_of() and calls
enable_irq(). free_irq() synchronizes an IRQ handler, but does not drain a
work item already queued by that handler.
wm831x_ts_remove() can therefore free the IRQ actions while pd_data_work is
pending or running. The work may then dereference wm831x_ts after devres
frees it, or enable an IRQ after its action has been freed.
Disable both IRQs before cancelling the work, so neither handler can queue
another instance while cancel_work_sync() drains it. Free the IRQ actions
only after the work has stopped. Apply the same sequence to err_pd_irq:
both IRQ actions exist there when input_register_device() fails.
This issue was found by an in-house static analysis tool and confirmed by
manual code review.
Fixes: f5346668150c ("Input: wm831x-ts - fix races with IRQ management")
Cc: stable@vger.kernel.org
Assisted-by: Codex:gpt-5.6
Signed-off-by: Fan Wu <fanwu01@zju.edu.cn>
---
drivers/input/touchscreen/wm831x-ts.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/input/touchscreen/wm831x-ts.c b/drivers/input/touchscreen/wm831x-ts.c
index 98f8ec408cad..705772025648 100644
--- a/drivers/input/touchscreen/wm831x-ts.c
+++ b/drivers/input/touchscreen/wm831x-ts.c
@@ -366,6 +366,10 @@ static int wm831x_ts_probe(struct platform_device *pdev)
return 0;
err_pd_irq:
+ disable_irq(wm831x_ts->pd_irq);
+ disable_irq(wm831x_ts->data_irq);
+ cancel_work_sync(&wm831x_ts->pd_data_work);
+
free_irq(wm831x_ts->pd_irq, wm831x_ts);
err_data_irq:
free_irq(wm831x_ts->data_irq, wm831x_ts);
@@ -378,6 +382,10 @@ static void wm831x_ts_remove(struct platform_device *pdev)
{
struct wm831x_ts *wm831x_ts = platform_get_drvdata(pdev);
+ disable_irq(wm831x_ts->pd_irq);
+ disable_irq(wm831x_ts->data_irq);
+ cancel_work_sync(&wm831x_ts->pd_data_work);
+
free_irq(wm831x_ts->pd_irq, wm831x_ts);
free_irq(wm831x_ts->data_irq, wm831x_ts);
}
--
2.34.1
^ permalink raw reply related
* Re: [PATCH] Input: wm831x-ts - drain pen-down work at teardown
From: sashiko-bot @ 2026-07-22 3:03 UTC (permalink / raw)
To: Fan Wu; +Cc: linux-input, dmitry.torokhov
In-Reply-To: <20260722024518.3253280-1-fanwu01@zju.edu.cn>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Use-After-Free / Lifecycle Bug due to devres and manual IRQ teardown mismatch.
--
--- Patch [1]: [PATCH] Input: wm831x-ts - drain pen-down work at teardown ---
commit 084d696661fe76ff6ec20cb061d22484763d2522
Author: Fan Wu <fanwu01@zju.edu.cn>
Input: wm831x-ts - drain pen-down work at teardown
This patch explicitly disables the pen-down and data IRQs and cancels the
pd_data_work before freeing the IRQs in the error path and during device
removal. This aims to fix races where the work might run or be pending
while the IRQs are being freed.
> diff --git a/drivers/input/touchscreen/wm831x-ts.c b/drivers/input/touchscreen/wm831x-ts.c
> index 98f8ec408cad6..7057720256487 100644
> --- a/drivers/input/touchscreen/wm831x-ts.c
> +++ b/drivers/input/touchscreen/wm831x-ts.c
[ ... ]
> @@ -378,6 +382,10 @@ static void wm831x_ts_remove(struct platform_device *pdev)
> {
> struct wm831x_ts *wm831x_ts = platform_get_drvdata(pdev);
>
> + disable_irq(wm831x_ts->pd_irq);
> + disable_irq(wm831x_ts->data_irq);
> + cancel_work_sync(&wm831x_ts->pd_data_work);
> +
> free_irq(wm831x_ts->pd_irq, wm831x_ts);
> free_irq(wm831x_ts->data_irq, wm831x_ts);
> }
[Severity: High]
This is a pre-existing issue, but does freeing the IRQs here lead to a
use-after-free when the input device is closed during devres cleanup?
Because the input device is managed via devres (allocated with
devm_input_allocate_device()), the input core will automatically unregister
it after wm831x_ts_remove() returns.
If the input device is held open by userspace while the driver is unbound,
the unregistration process will call the device's close callback. That callback
operates on the IRQs that have already been freed:
wm831x_ts_input_close() {
...
if (wm831x_ts->pen_down) {
disable_irq(wm831x_ts->data_irq);
enable_irq(wm831x_ts->pd_irq);
wm831x_ts->pen_down = false;
}
...
}
Calling enable_irq() on an already freed IRQ can drop its depth to 0 and
unmask it without an action handler, leading to an IRQ shutdown and a loud
kernel 'irq X: nobody cared' warning. In a shared IRQ environment (like an MFD),
could this disrupt other devices sharing the same line?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260722024518.3253280-1-fanwu01@zju.edu.cn?part=1
^ permalink raw reply
* Re: [PATCH v3 7/9] platform/chrome: of_hw_prober: Add delay for hana trackpads
From: Chen-Yu Tsai @ 2026-07-22 3:33 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Matthias Brugger, AngeloGioacchino Del Regno, Benson Leung,
Tzung-Bi Shih, Dmitry Torokhov, Jiri Kosina, Andi Shyti,
linux-mediatek, devicetree, linux-arm-kernel, chrome-platform,
linux-input, linux-i2c, linux-kernel
In-Reply-To: <al9JivV0SZCiOgK8@ashevche-desk.local>
On Tue, Jul 21, 2026 at 6:27 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Tue, Jul 21, 2026 at 03:52:21PM +0800, Chen-Yu Tsai wrote:
> > Up until now, the MT8173 elm/hana device tree has set the dedicated
> > regulator supplying the trackpad as always-on, simply because the Elan
> > driver was missing proper delays. As a result the delay for the
> > Synaptics trackpad was also omitted, as it was not strictly required
> > under such a model and delayed the availability of the trackpad to the
> > user.
> >
> > The Elan driver recently gained proper delays after power-up, with
> > adaptive skipping of the delay if the regulator was originally
> > on. The I2C HID driver and I2C OF component prober library gained
> > similar adaptive delay skipping. The device tree will be fixed to have
> > the regulator not be always on, and proper post-power-on delay time
> > added to the I2C HID device.
> >
> > Also add the post-power-on delay to the ChromeOS OF component prober,
> > so that if the regulator is off at the time of probing, the prober knows
> > to wait for the hardware to initialize.
>
> ...
>
> > +++ b/drivers/platform/chrome/chromeos_of_hw_prober.c
>
> > /*
> > * ELAN trackpad needs 2 ms for H/W init and 100 ms for F/W init.
> > * Synaptics trackpad needs 100 ms.
> > - * However, the regulator is set to "always-on", presumably to
> > - * avoid this delay. The ELAN driver is also missing delays.
> > */
> > - .post_power_on_delay_ms = 0,
> > + .post_power_on_delay_ms = 110,
>
> This and the other delay in the previous patches are all HW-related. What if
> another touchpad gets connected? I mean that the delays for the certain HW
> should not affect other possible devices connected to the platform.
This is a complete laptop, not a development board. You can't just randomly
swap out the trackpad. The model AFAIK is no longer in production, and we
have already added all possible configurations to the device tree. And
there's no extra connector or pin header to attach additional peripherals.
If someone hacks their own device to swap out a trackpad, then handling
the additional bring-up should fall on them.
If you are referring to the delay for the other trackpad option, that is
100 ms, and is described in the device tree. See patch 8 of this series.
Any other device with different delay requirements should have its own
entry, not reuse this one. The information in the entry is tightly
coupled to the device tree anyway. I already corrected the other device
that incorrectly reused this entry [1]. We have some additional offenders
downstream, but nothing that can't be corrected in the same manner.
ChenYu
[1] https://lore.kernel.org/all/20260625060859.1020483-1-wenst@chromium.org/
^ permalink raw reply
* [PATCH v2] Input: wm831x-ts - drain pen-down work at teardown
From: Fan Wu @ 2026-07-22 4:58 UTC (permalink / raw)
To: linux-input; +Cc: Dmitry Torokhov, linux-kernel, Fan Wu, stable
In-Reply-To: <20260722024518.3253280-1-fanwu01@zju.edu.cn>
The pen-down and data IRQ handlers queue pd_data_work to switch between the
two IRQs. The worker obtains wm831x_ts with container_of() and calls
enable_irq(). free_irq() synchronizes an IRQ handler, but does not drain a
work item already queued by that handler.
wm831x_ts_remove() can therefore free the IRQ actions while pd_data_work is
pending or running. The work may then dereference wm831x_ts after devres
frees it (use-after-free), or re-enable an IRQ whose action has been freed.
The input device is devm-allocated, so the input core unregisters it after
wm831x_ts_remove() returns. If a userspace handle is still open, that
unregister drives the input close callback, which on pen-down calls
enable_irq(pd_irq) - after wm831x_ts_remove() has already freed the pd_irq
action. Unregister the input device before freeing the IRQs, so the close
callback runs while the actions are still installed. This must precede the
IRQ teardown: disabling and cancelling first would still let the close
callback re-enable pd_irq after its action is gone.
After unregistering, drain the work: disable both IRQs so neither handler
can queue another instance, cancel_work_sync() to make the drain final,
then free the actions. Apply the same drain to err_pd_irq; the input
device is not registered there, so it is not unregistered.
This issue was found by an in-house static analysis tool and confirmed by
manual code review.
Fixes: f5346668150c ("Input: wm831x-ts - fix races with IRQ management")
Cc: stable@vger.kernel.org
Assisted-by: Codex:gpt-5.6
Signed-off-by: Fan Wu <fanwu01@zju.edu.cn>
---
Changes since v1:
- Unregister the input device before freeing the IRQs. v1 drained the
work in wm831x_ts_remove(), but the input device is devm-managed and is
unregistered by the input core only after remove() returns; with a
userspace handle open, that unregister drives the close callback, which
on pen-down calls enable_irq(pd_irq) after its action has been freed.
drivers/input/touchscreen/wm831x-ts.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/drivers/input/touchscreen/wm831x-ts.c b/drivers/input/touchscreen/wm831x-ts.c
index 98f8ec408cad..a465b1b96fd8 100644
--- a/drivers/input/touchscreen/wm831x-ts.c
+++ b/drivers/input/touchscreen/wm831x-ts.c
@@ -366,6 +366,10 @@ static int wm831x_ts_probe(struct platform_device *pdev)
return 0;
err_pd_irq:
+ disable_irq(wm831x_ts->pd_irq);
+ disable_irq(wm831x_ts->data_irq);
+ cancel_work_sync(&wm831x_ts->pd_data_work);
+
free_irq(wm831x_ts->pd_irq, wm831x_ts);
err_data_irq:
free_irq(wm831x_ts->data_irq, wm831x_ts);
@@ -378,6 +382,12 @@ static void wm831x_ts_remove(struct platform_device *pdev)
{
struct wm831x_ts *wm831x_ts = platform_get_drvdata(pdev);
+ input_unregister_device(wm831x_ts->input_dev);
+
+ disable_irq(wm831x_ts->pd_irq);
+ disable_irq(wm831x_ts->data_irq);
+ cancel_work_sync(&wm831x_ts->pd_data_work);
+
free_irq(wm831x_ts->pd_irq, wm831x_ts);
free_irq(wm831x_ts->data_irq, wm831x_ts);
}
--
2.34.1
^ permalink raw reply related
* Re: [PATCH v2] Input: wm831x-ts - drain pen-down work at teardown
From: sashiko-bot @ 2026-07-22 5:09 UTC (permalink / raw)
To: Fan Wu; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260722045853.3255262-1-fanwu01@zju.edu.cn>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Explicitly calling `input_unregister_device()` on a device allocated via `devm_input_allocate_device()` violates input subsystem guidelines.
--
--- Patch [1]: [PATCH v2] Input: wm831x-ts - drain pen-down work at teardown ---
commit f1080238ec479acff6be9e5861469736f7ad1c61
Author: Fan Wu <fanwu01@zju.edu.cn>
Input: wm831x-ts - drain pen-down work at teardown
This commit fixes a use-after-free race condition by draining the pending
pen-down work item during teardown. It explicitly unregisters the input
device before freeing the IRQs so that the close callback runs while actions
are still installed.
> diff --git a/drivers/input/touchscreen/wm831x-ts.c b/drivers/input/touchscreen/wm831x-ts.c
> --- a/drivers/input/touchscreen/wm831x-ts.c
> +++ b/drivers/input/touchscreen/wm831x-ts.c
[ ... ]
> @@ -378,6 +382,12 @@ static void wm831x_ts_remove(struct platform_device *pdev)
> {
> struct wm831x_ts *wm831x_ts = platform_get_drvdata(pdev);
>
> + input_unregister_device(wm831x_ts->input_dev);
[Severity: Medium]
Is it safe to explicitly call input_unregister_device() here?
Since the input device is allocated using devm_input_allocate_device() during
wm831x_ts_probe(), the input subsystem already tracks it as a managed resource
and will automatically unregister it when the provider is unbound.
Explicitly calling input_unregister_device() on a devm-managed device mixes
managed and manual lifecycles, which can potentially lead to double-unregister
issues during teardown.
Could this race condition be addressed instead by fully migrating the driver to
managed resources? For example, using devm_request_threaded_irq() for the
IRQs and devm_add_action_or_reset() to handle the workqueue cancellation might
ensure the correct cleanup ordering without needing manual unregistration.
> +
> + disable_irq(wm831x_ts->pd_irq);
> + disable_irq(wm831x_ts->data_irq);
> + cancel_work_sync(&wm831x_ts->pd_data_work);
> +
> free_irq(wm831x_ts->pd_irq, wm831x_ts);
> free_irq(wm831x_ts->data_irq, wm831x_ts);
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260722045853.3255262-1-fanwu01@zju.edu.cn?part=1
^ permalink raw reply
* Re: [PATCH] HID: asus: add new Asus EC hid device for keyboard backlight and FN HotKeys
From: Alexandru Serdeliuc @ 2026-07-22 6:25 UTC (permalink / raw)
To: Jiri Kosina; +Cc: Benjamin Tissoires, linux-kernel, linux-input
In-Reply-To: <5o7r0493-19r1-208p-404q-n0nq453170q6@xreary.bet>
Hi Jiri,
Thanks for the feedback! No problem at all, December is always a busy
time of year, and there was no rush on this anyway.
I am going to test integrating the logic into hid-asus locally first on
the Zenbook A14 to confirm everything builds and functions properly on
this platform (aarch64).
Once I verify that it works as expected, I will report back and send
over a v2 patch series.
Thanks,
Alexandru
On 7/1/26 16:12, Jiri Kosina wrote:
> On Tue, 30 Jun 2026, Alexandru Serdeliuc wrote:
>
>> Hi Jiri, Benjamin,
>>
>> Enquiring if anyone has had a chance to look over this patch series from
>> December?
>>
>> To provide a bit of architectural context, this is a 100% pure HID client
>> driver.
>>
>> This laptop (Asus Zenbook A14 UX3407QA) runs on the Qualcomm ARM64 Snapdragon
>> platform.
>>
>> Because it is an ARM device, it does not use traditional legacy x86 ACPI/WMI
>> code.
>>
>> Instead, the Fn hotkeys and keyboard backlight controls are routed completely
>> through standard vendor-specific HID reports (using Report ID 0x5A) over the
>> existing I2C-HID transport layer.
>>
>> It requires absolutely zero Device Tree modifications or external platform
>> subsystem hooks to bind or function.
>>
>> The driver matches strictly on Vendor/Product ID (0x0B05 / 0x0220) passed
>> directly through the core HID subsystem.
>>
>> I would highly appreciate any architectural feedback or a quick review!
>
> Hi Alexandru,
>
> first - sorry for not having handled this, it somehow fell in between
> cracks.
>
> I have just one question -- wouldn't it be better to have this device
> handled as part of generic hid-asus driver? Giving too many Kconfig
> configurations for every specific model feels a little bit too
> overwhelming.
>
> (yes, we'd need to have hid-asus depend on I2C ... I don't think that's a
> big issue).
>
> Thanks, and sorry again,
>
^ permalink raw reply
* Re: [PATCH v2 1/4] selftests: add shared lib.bpf.mk to build BPF progs and skeletons
From: Viktor Malik @ 2026-07-22 6:50 UTC (permalink / raw)
To: Ziyang Men, Shuah Khan, Tejun Heo, Johannes Weiner,
Michal Koutný, Jiri Kosina, Benjamin Tissoires, David Vernet,
Eduard Zingerman
Cc: Andrea Righi, Changwoo Min, Michal Hocko, Roman Gushchin,
Shakeel Butt, Muchun Song, Andrew Morton, JP Kobryn,
Mykola Lysenko, Nathan Chancellor, linux-kselftest, cgroups,
linux-input, sched-ext, linux-mm, kernel-team, bpf, llvm,
linux-kernel
In-Reply-To: <20260721174833.1232771-2-ziyang.meme@gmail.com>
On 7/21/26 19:48, Ziyang Men wrote:
> The libbpf + bpftool + vmlinux.h + BPF-object + skeleton build tool-chain
> is currently duplicated across tools/testing/selftests/{bpf,sched_ext,
> hid}/, each carrying ~100-140 lines of near-identical Makefile. As more
> subsystems grow BPF-based selftests, the duplication scales poorly.
>
> Add tools/testing/selftests/lib.bpf.mk, a single includable fragment that
> provides the whole chain end-to-end. It builds the in-tree libbpf.a and a
> host bpftool, generates vmlinux.h from the kernel's BTF, compiles *.bpf.c
> into BPF objects (clang --target=bpf) and generates their skeletons. It
> also provides what the user-space test binary needs to build: the header
> search paths (so #includes resolve), a bpf_link macro that wraps the link
> command, and BPF_LDLIBS, so the test can be statically linked against
> libbpf.a.
>
> To use: set BPF_SRCS and OVERRIDE_TARGETS := 1 before including ../lib.mk
> (so lib.mk's default link rule is suppressed), then include ../lib.bpf.mk
> and list $(BPF_SKELS) as prerequisites of the test binary, e.g.,:
>
> BPF_SRCS := progs/foo.bpf.c
> OVERRIDE_TARGETS := 1
> include ../lib.mk
> include ../lib.bpf.mk
> $(OUTPUT)/foo_test: foo_test.c $(BPF_SKELS)
> $(call bpf_link,$@,$<)
>
> This saves much work for configuring selftests in other folder, such the cgroup.
>
> net/bpf.mk (which only builds *.bpf.o, without skeleton or vmlinux.h
> generation) is left unchanged; replacing the existing duplication
> mentioned above is the next step.
>
> Suggested-by: Shakeel Butt <shakeel.butt@linux.dev>
> Suggested-by: Eduard Zingerman <eddyz87@gmail.com>
> Suggested-by: Mykola Lysenko <mykolal@meta.com>
> Assisted-by: Claude:claude-opus-4-8
> Signed-off-by: Ziyang Men <ziyang.meme@gmail.com>
> ---
> tools/testing/selftests/lib.bpf.mk | 247 +++++++++++++++++++++++++++++
> 1 file changed, 247 insertions(+)
> create mode 100644 tools/testing/selftests/lib.bpf.mk
>
> diff --git a/tools/testing/selftests/lib.bpf.mk b/tools/testing/selftests/lib.bpf.mk
> new file mode 100644
> index 000000000000..6f175c6568e9
> --- /dev/null
> +++ b/tools/testing/selftests/lib.bpf.mk
> @@ -0,0 +1,247 @@
> +# SPDX-License-Identifier: GPL-2.0
> +#
> +# Shared fragment for selftests that compile *.bpf.c into BPF objects +
> +# skeletons and link them into userspace test binaries, without each
> +# subsystem's Makefile re-implementing the libbpf/bpftool/vmlinux.h machinery.
> +#
> +# Caller contract (per-test Makefile):
> +#
> +# BPF_SRCS := foo.bpf.c bar.bpf.c
> +# TEST_GEN_PROGS := foo_test
> +# OVERRIDE_TARGETS := 1 # MUST be set before lib.mk
> +# include ../lib.mk # defines OUTPUT, CC, Q, msg,
> +# include ../lib.bpf.mk # selfdir, top_srcdir; honours OVERRIDE
> +#
> +# $(OUTPUT)/foo_test: foo_test.c $(BPF_SKELS)
> +# $(call bpf_link,$@,$<)
> +#
> +# Optional knobs (set before including lib.bpf.mk):
> +# BPF_PROG_EXT - source suffix, default .bpf.c; set to .c for the legacy
> +# "progs/foo.c -> foo.bpf.o" layout.
> +# BPF_EXTRA_HDRS - extra prerequisites (headers) for the BPF objects.
> +# BPF_EXTRA_CFLAGS - appended to BPF_CFLAGS for the BPF compile.
> +# BPF_SKEL_EXT - skeleton suffix, default .skel.h (e.g. .bpf.skel.h).
> +# BPF_GEN_SUBSKEL - if set, also emit a subskeleton next to each skeleton.
> +# BPF_OBJ_DIR - dir for generated *.bpf.o (default $(OUTPUT)).
> +# BPF_SKEL_DIR - dir for generated skeletons (default $(OUTPUT)).
> +# A caller with unusual compile needs may override BPF_CFLAGS wholesale after
> +# the include (the object recipe expands it lazily).
> +# BPF_SRCS entries may live in a subdirectory (e.g. progs/foo.bpf.c); the
> +# objects and skeletons are always emitted flat under $(OUTPUT), keyed by the
> +# source basename (foo.bpf.o / foo.skel.h).
> +#
> +# lib.mk MUST be included first: this fragment consumes the vars it defines
> +# (OUTPUT, top_srcdir, CC, CLANG, Q, msg) and needs OVERRIDE_TARGETS to have
> +# already suppressed lib.mk's default link rule.
> +
> +include $(top_srcdir)/tools/scripts/Makefile.arch # ARCH / SRCARCH
> +# Pull in the shared toolchain definitions (HOSTCC/HOSTLD/CLANG) so this
> +# fragment picks the *same* host compiler the libbpf and bpftool sub-makes
> +# will: in particular HOSTCC becomes clang under LLVM=1 (gcc otherwise).
> +# Without this the host bpftool and its bootstrap libbpf can end up built with
> +# a mix of gcc and clang, which trips clang on gcc-only flags (-Wstrict-aliasing=3).
> +#
> +# Makefile.include's allow-override resets CC to a bare "clang", which would
> +# drop the --target=<arch> flag lib.mk set for LLVM=1 cross builds (make LLVM=1
> +# ARCH=arm64). lib.mk ran first and configured CC for the target, so save it
> +# across the include and restore it afterwards.
> +lib_bpf_mk_saved_cc := $(CC)
> +include $(top_srcdir)/tools/scripts/Makefile.include
> +CC := $(lib_bpf_mk_saved_cc)
> +
> +CLANG ?= clang
> +HOSTCC ?= gcc
> +HOSTLD ?= ld
> +ifneq ($(V),1)
> +submake_extras := feature_display=0
> +endif
> +
> +# ---- paths & tools --------------------------------------------------------
> +TOOLSDIR := $(top_srcdir)/tools
> +LIBDIR := $(TOOLSDIR)/lib
> +BPFDIR := $(LIBDIR)/bpf
> +TOOLSINCDIR := $(TOOLSDIR)/include
> +BPFTOOLDIR := $(TOOLSDIR)/bpf/bpftool
> +APIDIR := $(TOOLSINCDIR)/uapi
> +
> +# Everything generated lives under $(OUTPUT) so O= and in-tree both work and
> +# per-test builds (distinct $(OUTPUT)) never collide.
> +SCRATCH_DIR := $(OUTPUT)/tools
> +BUILD_DIR := $(SCRATCH_DIR)/build
> +INCLUDE_DIR := $(SCRATCH_DIR)/include
> +BPFOBJ := $(BUILD_DIR)/libbpf/libbpf.a
> +
> +# bpftool must run on the *host*; split the host toolchain out when cross-building.
> +ifneq ($(CROSS_COMPILE),)
> +HOST_BUILD_DIR := $(BUILD_DIR)/host
> +HOST_SCRATCH_DIR := $(OUTPUT)/host-tools
> +else
> +HOST_BUILD_DIR := $(BUILD_DIR)
> +HOST_SCRATCH_DIR := $(SCRATCH_DIR)
> +endif
> +HOST_BPFOBJ := $(HOST_BUILD_DIR)/libbpf/libbpf.a
> +DEFAULT_BPFTOOL := $(HOST_SCRATCH_DIR)/sbin/bpftool
> +BPFTOOL ?= $(DEFAULT_BPFTOOL)
> +
> +# ---- vmlinux BTF discovery -----------------------------------------------
> +VMLINUX_BTF_PATHS ?= $(if $(O),$(O)/vmlinux) \
> + $(if $(KBUILD_OUTPUT),$(KBUILD_OUTPUT)/vmlinux) \
> + $(top_srcdir)/vmlinux \
> + /sys/kernel/btf/vmlinux \
> + /boot/vmlinux-$(shell uname -r)
> +VMLINUX_BTF ?= $(abspath $(firstword $(wildcard $(VMLINUX_BTF_PATHS))))
> +ifeq ($(VMLINUX_BTF),)
> +$(error Cannot find a vmlinux for VMLINUX_BTF at any of "$(VMLINUX_BTF_PATHS)")
> +endif
> +
> +# ---- clang flags ----------------------------------------------------------
> +# Clang's default system includes (not the ones seen under --target=bpf); fixes
> +# "missing" asm/byteorder.h etc. '-idirafter' so we never shadow real includes.
> +define get_sys_includes
> +$(shell $(1) $(2) -v -E - </dev/null 2>&1 \
> + | sed -n '/<...> search starts here:/,/End of search list./{ s| \(/.*\)|-idirafter \1|p }') \
> +$(shell $(1) $(2) -dM -E - </dev/null | grep '__riscv_xlen ' | awk '{printf("-D__riscv_xlen=%d -D__BITS_PER_LONG=%d", $$3, $$3)}')
> +endef
> +ifneq ($(CROSS_COMPILE),)
> +CLANG_TARGET_ARCH = --target=$(notdir $(CROSS_COMPILE:%-=%))
> +endif
> +CLANG_SYS_INCLUDES = $(call get_sys_includes,$(CLANG),$(CLANG_TARGET_ARCH))
> +
> +IS_LITTLE_ENDIAN = $(shell $(CC) -dM -E - </dev/null | \
> + grep 'define __BYTE_ORDER__ __ORDER_LITTLE_ENDIAN__')
> +MENDIAN = $(if $(IS_LITTLE_ENDIAN),-mlittle-endian,-mbig-endian)
> +
> +# Prefer -mcpu=v3; fall back to v2 on clang too old to know it (probed once).
> +CLANG_BPF_CPU := $(shell $(CLANG) --target=bpf -mcpu=help 2>&1 | grep -q 'v3' \
> + && echo v3 || echo v2)
> +
> +# -fms-extensions + -Wno-microsoft-anon-tag: required so clang accepts the
> +# anonymous nested struct/union members bpftool emits into vmlinux.h.
> +BPF_CFLAGS = -g -Wall -Werror -D__TARGET_ARCH_$(SRCARCH) $(MENDIAN) \
> + -I$(INCLUDE_DIR) -I$(APIDIR) -I$(TOOLSINCDIR) \
> + -std=gnu11 \
> + -fno-strict-aliasing \
> + -fms-extensions -Wno-microsoft-anon-tag \
> + -Wno-compare-distinct-pointer-types \
> + $(CLANG_SYS_INCLUDES) $(BPF_EXTRA_CFLAGS)
> +
> +# $1 = src .bpf.c, $2 = dst .bpf.o
> +define BPF_BUILD_RULE
> + $(call msg,CLNG-BPF,,$2)
> + $(Q)$(CLANG) $(BPF_CFLAGS) -O2 --target=bpf -mcpu=$(CLANG_BPF_CPU) -c $1 -o $2
> +endef
> +
> +# ---- output dirs for generated objects/skeletons --------------------------
> +# Default: flat under $(OUTPUT) (what bpf/, hid/, cgroup/ do). A caller may
> +# segregate the generated files into subdirs, e.g. BPF_SKEL_DIR := $(OUTPUT)/...
> +BPF_OBJ_DIR ?= $(OUTPUT)
> +BPF_SKEL_DIR ?= $(OUTPUT)
> +
> +# ---- scratch dirs ---------------------------------------------------------
> +MAKE_DIRS := $(sort $(BUILD_DIR)/libbpf $(HOST_BUILD_DIR)/libbpf \
> + $(HOST_BUILD_DIR)/bpftool $(INCLUDE_DIR) \
> + $(filter-out $(OUTPUT),$(BPF_OBJ_DIR) $(BPF_SKEL_DIR)))
> +$(MAKE_DIRS):
> + $(call msg,MKDIR,,$@)
> + $(Q)mkdir -p $@
> +
> +# ---- libbpf (target) ------------------------------------------------------
> +# Pass ARCH/CROSS_COMPILE/CC through: lib.mk's CC is file-origin and is not
> +# exported, so without this the libbpf sub-make would rebuild for the host under
> +# a pure-LLVM cross build (make LLVM=1 ARCH=<arch>). -fPIC keeps the static
> +# libbpf linkable into position-independent (PIE) test binaries.
> +$(BPFOBJ): $(wildcard $(BPFDIR)/*.[ch] $(BPFDIR)/Makefile) \
> + $(APIDIR)/linux/bpf.h | $(BUILD_DIR)/libbpf
> + $(Q)$(MAKE) $(submake_extras) -C $(BPFDIR) OUTPUT=$(BUILD_DIR)/libbpf/ \
> + ARCH=$(ARCH) CROSS_COMPILE=$(CROSS_COMPILE) CC="$(CC)" \
> + EXTRA_CFLAGS='-g -O0 -fPIC' \
Looking at the same line in tools/testing/selftests/bpf/Makefile:
EXTRA_CFLAGS='-g $(OPT_FLAGS) $(SAN_CFLAGS) $(EXTRA_CFLAGS)' \
is there a reason why this is not respected and -O0 and -fPIC are
hard-coded? What if someone wants to build selftests (including libbpf)
with -O2 and some additional flags? Is that possible with the new
lib.bpf.mk? It would be nice if it used at least EXTRA_CFLAGS or
USERCFLAGS defined in lib.mk.
Same questions would apply to other rules for libbpf and bpftool below.
Thanks!
Viktor
> + DESTDIR=$(SCRATCH_DIR) prefix= all install_headers
> +
> +# ---- libbpf (host) -- a distinct rule only when cross-compiling -----------
> +ifneq ($(BPFOBJ),$(HOST_BPFOBJ))
> +$(HOST_BPFOBJ): $(wildcard $(BPFDIR)/*.[ch] $(BPFDIR)/Makefile) \
> + | $(HOST_BUILD_DIR)/libbpf
> + $(Q)$(MAKE) $(submake_extras) -C $(BPFDIR) ARCH= CROSS_COMPILE= \
> + OUTPUT=$(HOST_BUILD_DIR)/libbpf/ CC=$(HOSTCC) LD=$(HOSTLD) \
> + EXTRA_CFLAGS='-g -O0' \
> + DESTDIR=$(HOST_SCRATCH_DIR) prefix= all install_headers
> +endif
> +
> +# ---- bpftool (host) -------------------------------------------------------
> +$(DEFAULT_BPFTOOL): $(wildcard $(BPFTOOLDIR)/*.[ch] $(BPFTOOLDIR)/Makefile) \
> + $(HOST_BPFOBJ) | $(HOST_BUILD_DIR)/bpftool
> + $(Q)$(MAKE) $(submake_extras) -C $(BPFTOOLDIR) \
> + ARCH= CROSS_COMPILE= CC=$(HOSTCC) LD=$(HOSTLD) \
> + EXTRA_CFLAGS='-g -O0' \
> + OUTPUT=$(HOST_BUILD_DIR)/bpftool/ \
> + LIBBPF_OUTPUT=$(HOST_BUILD_DIR)/libbpf/ \
> + LIBBPF_DESTDIR=$(HOST_SCRATCH_DIR)/ \
> + prefix= DESTDIR=$(HOST_SCRATCH_DIR)/ install-bin
> +
> +# ---- vmlinux.h ------------------------------------------------------------
> +$(INCLUDE_DIR)/vmlinux.h: $(VMLINUX_BTF) $(BPFTOOL) | $(INCLUDE_DIR)
> +ifeq ($(VMLINUX_H),)
> + $(call msg,GEN,,$@)
> + $(Q)$(BPFTOOL) btf dump file $(VMLINUX_BTF) format c > $@
> +else
> + $(call msg,CP,,$@)
> + $(Q)cp "$(VMLINUX_H)" $@
> +endif
> +
> +# ---- BPF objects + skeletons ---------------------------------------------
> +# Sources may sit in a subdir and use the modern *.bpf.c or the legacy *.c
> +# suffix (BPF_PROG_EXT); objects go in $(BPF_OBJ_DIR) and skeletons in
> +# $(BPF_SKEL_DIR) (both default to $(OUTPUT)), keyed by the source basename.
> +BPF_PROG_EXT ?= .bpf.c
> +bpf_stems := $(patsubst %$(BPF_PROG_EXT),%,$(notdir $(BPF_SRCS)))
> +# The output namespace is flat, so two sources with the same basename would
> +# collapse into one object/skeleton; fail loudly instead of silently dropping.
> +ifneq ($(words $(bpf_stems)),$(words $(sort $(bpf_stems))))
> +$(error lib.bpf.mk: BPF_SRCS has colliding basenames: $(BPF_SRCS))
> +endif
> +# BPF_SKEL_EXT lets a caller pick the skeleton suffix (default .skel.h; e.g.
> +# .bpf.skel.h). With BPF_GEN_SUBSKEL set, a matching subskeleton is emitted
> +# alongside (foo.subskel.h / foo.bpf.subskel.h).
> +BPF_SKEL_EXT ?= .skel.h
> +BPF_SUBSKEL_EXT := $(patsubst %skel.h,%subskel.h,$(BPF_SKEL_EXT))
> +BPF_OBJS := $(addprefix $(BPF_OBJ_DIR)/,$(addsuffix .bpf.o,$(bpf_stems)))
> +BPF_SKELS := $(addprefix $(BPF_SKEL_DIR)/,$(addsuffix $(BPF_SKEL_EXT),$(bpf_stems)))
> +
> +# Locate the sources wherever the caller keeps them (e.g. progs/).
> +vpath %$(BPF_PROG_EXT) $(sort $(dir $(BPF_SRCS)))
> +
> +$(BPF_OBJS): $(BPF_OBJ_DIR)/%.bpf.o: %$(BPF_PROG_EXT) $(BPF_EXTRA_HDRS) \
> + $(wildcard *.bpf.h) $(INCLUDE_DIR)/vmlinux.h | $(BPF_OBJ_DIR) $(BPFOBJ)
> + $(call BPF_BUILD_RULE,$<,$@)
> +
> +$(BPF_SKELS): $(BPF_SKEL_DIR)/%$(BPF_SKEL_EXT): $(BPF_OBJ_DIR)/%.bpf.o $(BPFTOOL) | $(BPF_SKEL_DIR)
> + $(call msg,GEN-SKEL,,$@)
> + $(Q)$(BPFTOOL) gen object $(<:.o=.linked.o) $<
> + $(Q)$(BPFTOOL) gen skeleton $(<:.o=.linked.o) name $(notdir $(<:.bpf.o=)) > $@
> +ifneq ($(BPF_GEN_SUBSKEL),)
> + $(Q)$(BPFTOOL) gen subskeleton $(<:.o=.linked.o) name $(notdir $(<:.bpf.o=)) > $(@:$(BPF_SKEL_EXT)=$(BPF_SUBSKEL_EXT))
> +endif
> +
> +# ---- exports consumed by the caller --------------------------------------
> +# -I$(OUTPUT)/-I$(BPF_SKEL_DIR): so the test .c can #include "foo.skel.h".
> +# -I$(INCLUDE_DIR): so userspace can pull in the generated vmlinux.h if needed.
> +CFLAGS += -I$(OUTPUT) -I$(BPF_SKEL_DIR) -I$(INCLUDE_DIR)
> +
> +# Static libbpf.a first, then its deps. libbpf may pull in zstd (BTF decompress)
> +# only when built against it; link -lzstd only if libzstd is present.
> +BPF_LDLIBS := $(BPFOBJ) -lelf -lz
> +ifneq ($(shell pkg-config --exists libzstd 2>/dev/null && echo y),)
> +BPF_LDLIBS += -lzstd
> +endif
> +
> +TEST_GEN_FILES += $(BPF_OBJS)
> +
> +# Link helper: $1 = output binary, $2 = test .c (skels are the target's deps).
> +define bpf_link
> + $(call msg,BINARY,,$1)
> + $(Q)$(CC) $(CFLAGS) $2 $(BPF_LDLIBS) $(LDLIBS) -o $1
> +endef
> +
> +EXTRA_CLEAN += $(SCRATCH_DIR) $(HOST_SCRATCH_DIR) \
> + $(addprefix $(BPF_OBJ_DIR)/,*.bpf.o *.linked.o) \
> + $(addprefix $(BPF_SKEL_DIR)/,*$(BPF_SKEL_EXT) *$(BPF_SUBSKEL_EXT))
^ permalink raw reply
* Re: [PATCH v3 4/4] dt-bindings: leds: backlight: Convert TPS65217 to DT schema
From: Krzysztof Kozlowski @ 2026-07-22 7:44 UTC (permalink / raw)
To: Eduard Bostina
Cc: Aaro Koskinen, Andreas Kemnade, Conor Dooley, Daniel Thompson,
devicetree, Dmitry Torokhov, dri-devel, Jingoo Han, Kevin Hilman,
Krzysztof Kozlowski, Lee Jones, linux-input, linux-kernel,
linux-leds, linux-omap, Mark Jackson, Pavel Machek, Rob Herring,
Roger Quadros, Tony Lindgren, daniel.baluta, simona.toaca,
goledhruva, m-chawdhry
In-Reply-To: <20260721112133.70382-5-egbostina@gmail.com>
On Tue, Jul 21, 2026 at 11:21:28AM +0000, Eduard Bostina wrote:
> Convert the Texas Instruments TPS65217 bindings to DT schema.
>
> During the conversion, the following updates were made:
> - Documented the 'regulators', 'charger' and 'pwrbutton' child nodes,
> which are used by the boards but were missing from the old txt
> binding.
> - Documented the 'interrupts', 'interrupt-controller', '#interrupt-cells'
> and 'ti,pmic-shutdown-controller' properties, which are used by the
> am335x boards and read by the driver.
>
> Signed-off-by: Eduard Bostina <egbostina@gmail.com>
> ---
> .../bindings/leds/backlight/ti,tps65217.yaml | 170 ++++++++++++++++++
> .../leds/backlight/tps65217-backlight.txt | 27 ---
Biinding looks fine to me, but as pointed out you missed to remove old
TXT binding.
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v3 6/9] i2c: of-prober: Defer regulator_disable() on successful probe in simple helper
From: Chen-Yu Tsai @ 2026-07-22 8:34 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Matthias Brugger, AngeloGioacchino Del Regno, Benson Leung,
Tzung-Bi Shih, Dmitry Torokhov, Jiri Kosina, Andi Shyti,
linux-mediatek, devicetree, linux-arm-kernel, chrome-platform,
linux-input, linux-i2c, linux-kernel
In-Reply-To: <al9IwVR7Whw2DiXU@ashevche-desk.local>
On Tue, Jul 21, 2026 at 6:24 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Tue, Jul 21, 2026 at 03:52:20PM +0800, Chen-Yu Tsai wrote:
> > When a I2C component is found, it's device node is immediately enabled.
> > This triggers device creation and driver binding. The prober will hold
> > the regulator enable reference across this part. If the driver probes
> > synchronously, then it happens within this window. On the other hand,
> > if the driver probes asynchronously, there is high chance that it
> > happens after the prober's cleanup function was called, in which case
> > the regulator would have been disabled when the driver's probe function
> > is called. This would then require the driver to wait 100 ms for the
> > hardware to reinitialize, even if the probe function was just a split
> > second late and the regulator was disabled a few milliseconds ago.
> >
> > Recently, some of the drivers for the component that are targeted by the
> > I2C OF component prober gained the ability to skip waiting for hardware
> > initialization if the regulator was left enabled. This happens when the
> > PMIC has them on by default, or if the component prober left them on
> > after probing the component.
> >
> > Wait a bit of time before dropping the enable refcount on our end so
> > that the actual driver has the opportunity to catch and increase the
> > refcount on their end. The 100 ms delay was arbitrarily chosen.
>
> ...
>
> > + /*
> > + * Wait a bit of time for async drivers to probe and increase the
> > + * regulator enable count. This allows the drivers to check and
> > + * skip waiting for re-initialization.
> > + */
> > + if (defer_disable) {
> > + dev_dbg(dev, "Deferring regulator disable\n");
> > + msleep(100);
>
> How was this value chosen?
I just chose an arbitrary round value.
If both the prober and the driver for the probed device (trackpad in
this example) are builtin, the time between enabling the node and the
driver asynchronously probing is between 5 ms and 30 ms, but could
also see outliers exceeding 100 ms.
If you're worried the value won't suit every user, I could make it a
parameter of the simple helpers? And the user itself could be made to
probe asynchronously to not block the main code path.
ChenYu
^ permalink raw reply
* [PATCH] HID: i2c-hid: goodix: Disable VDD on VDDIO enable failure
From: Chao Huang @ 2026-07-22 9:36 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: Douglas Anderson, Matthias Kaehlcke, Dmitry Torokhov, linux-input,
linux-kernel, Chao Huang
From: Chao Huang <huangchao@kylinos.cn>
If enabling VDDIO fails after VDD has been enabled, the power-up
path returns without disabling VDD. This leaves the regulator enabled
and its enable count unbalanced.
Disable VDD before returning the VDDIO error.
Fixes: eb16f59e8e58 ("HID: i2c-hid: goodix: Add mainboard-vddio-supply")
Signed-off-by: Chao Huang <huangchao@kylinos.cn>
---
drivers/hid/i2c-hid/i2c-hid-of-goodix.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/hid/i2c-hid/i2c-hid-of-goodix.c b/drivers/hid/i2c-hid/i2c-hid-of-goodix.c
index f1597ad67e7c..f4dbcd1d1d47 100644
--- a/drivers/hid/i2c-hid/i2c-hid-of-goodix.c
+++ b/drivers/hid/i2c-hid/i2c-hid-of-goodix.c
@@ -51,8 +51,10 @@ static int goodix_i2c_hid_power_up(struct i2chid_ops *ops)
return ret;
ret = regulator_enable(ihid_goodix->vddio);
- if (ret)
+ if (ret) {
+ regulator_disable(ihid_goodix->vdd);
return ret;
+ }
if (ihid_goodix->timings->post_power_delay_ms)
msleep(ihid_goodix->timings->post_power_delay_ms);
--
2.25.1
^ permalink raw reply related
* Re: [PATCH v2 2/4] selftests/cgroup: add memcg_stat_cross_cpu correctness test for flush
From: Michal Koutný @ 2026-07-22 9:43 UTC (permalink / raw)
To: Ziyang Men
Cc: Shuah Khan, Tejun Heo, Johannes Weiner, Jiri Kosina,
Benjamin Tissoires, David Vernet, Eduard Zingerman, Andrea Righi,
Changwoo Min, Michal Hocko, Roman Gushchin, Shakeel Butt,
Muchun Song, Andrew Morton, JP Kobryn, Mykola Lysenko,
Nathan Chancellor, linux-kselftest, cgroups, linux-input,
sched-ext, linux-mm, kernel-team, bpf, llvm, linux-kernel
In-Reply-To: <20260721174833.1232771-3-ziyang.meme@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 16747 bytes --]
Hello Ziyang.
On Tue, Jul 21, 2026 at 10:48:31AM -0700, Ziyang Men <ziyang.meme@gmail.com> wrote:
> Add a test_progs selftest that verifies the memory-cgroup BPF kfuncs
> return values that agree with what userspace reads from cgroupfs, across
> a whole cgroup subtree that has been charged on many CPUs.
>
> It complements the existing cgroup_iter_memcg test. cgroup_iter_memcg
> calls the memcg kfuncs on a single cgroup (BPF_CGROUP_ITER_SELF_ONLY) and
> only asserts each value is greater than zero, so it never checks that a
> value is actually correct. This test compares the kfunc values against
> what userspace reads from memory.stat, and checks every node in the
> cgroup tree.
>
> Moreover, cgroup_iter_memcg does not exercise the rstat flush
> (mem_cgroup_flush_stats()). This test does: it launches a process on
> each leaf that charges and holds memory across multiple CPUs (so the
> leaf's rstat is dirty on K per-cpu trees), then confirms the flush works
> for both the BPF and the file path by checking two properties:
>
> 1. after the flush, each leaf's anon is at least what the process
> charged there;
>
> 2. after the flush, the sum of the leaves' charged memory equals the
> amount at the root.
>
> The BPF reader and the file reader run in two separate rounds. Each
> round builds the same cgroup tree from scratch and charges the same
> amount of memory, so both rounds start from the same state and the
> numbers are comparable. In detail:
>
> - round 1 (BPF) builds the subtree, forks one child per leaf that
> charges the leaf across K CPUs and then blocks holding the charge,
> walks the tree with a SEC("iter.s/cgroup") program that flushes the
> subtree at the root and reads each cgroup via the memcg kfuncs
> (bpf_get_mem_cgroup, bpf_mem_cgroup_flush_stats,
> bpf_mem_cgroup_page_state, bpf_mem_cgroup_vm_events,
> bpf_put_mem_cgroup) into a hash map;
>
> - round 2 (cgroupfs) builds and charges an identical tree the same way,
> then reads every cgroup's memory.stat / memory.current from
> userspace.
>
> The subtests differ in how many CPUs each leaf is charged on (a single
> CPU or across K CPUs), and run on two cgroup tree sizes. The charging
> children pin CPUs and there is one per leaf, so the test is registered
> serial.
>
> The traditional path reads memory.stat / memory.current through a new
> read_cgroup_file() helper added to cgroup_helpers (the read counterpart
> of write_cgroup_file). When the memcg kfuncs are unavailable
> (CONFIG_MEMCG=n) the test skips cleanly; the base selftest config now
> selects CONFIG_MEMCG=y.
>
> Suggested-by: Shakeel Butt <shakeel.butt@linux.dev>
> Assisted-by: Claude:claude-opus-4-8
> Signed-off-by: Ziyang Men <ziyang.meme@gmail.com>
On one hand, this migth be useful to ensure somewhat stable behavior of
the reported stats, OTOH, it's exposing quite some implementation
details (flushing, per-cpu charging).
I'd say that flushing for BPF progs should work (which is what
cgroup_iter_memcg() should test IIUC) and yield some precision effect,
however, the comparison with memory.stat may be source of false
positives due to noise.
Also, this is supposed to measure convergence in some "static" situation
but in reality the things are moving (memcg charges/uncharges) in
various pace so some differences would still occur (IMO such deviations
are OK, perhaps the test should spell out its preconditions and limitations).
As for the implementation in general -- it'd need some tighter
connection with existing cgroup selftests (e.g. use integer types like
cgroup selftest, utilization/adaptation of existing helpers to reduce
copies of same/similar code).
> --- a/tools/testing/selftests/cgroup/Makefile
> +++ b/tools/testing/selftests/cgroup/Makefile
> @@ -14,14 +14,29 @@ TEST_GEN_PROGS += test_freezer
> TEST_GEN_PROGS += test_hugetlb_memcg
> TEST_GEN_PROGS += test_kill
> TEST_GEN_PROGS += test_kmem
> +TEST_GEN_PROGS += test_memcg_stat_cross_cpu
> TEST_GEN_PROGS += test_memcontrol
Yeah, this is perhaps so much different from test_memcontrol that it
deserves a separate prog, OTOH it'd be good to share some common
cg/memcg functions across the two (some notes below).
> +unsigned long long cg_get_id(const char *cgroup)
This duplicates get_cgroup_id_from_path() from
tools/testing/selftests/bpf/cgroup_helpers.c
> +/* ---- allowed CPU set --------------------------------------------------- */
> +
> +static int *cpu_list; /* ids of the CPUs this process may run on */
I think it'd be simpler to just use cpu_set_t and the standard helper
macros/functions for this.
> +/* Recursively create children of @path. @path must already exist and be recorded. */
> +static int build_children(const char *path, int fanout, int depth)
> +{
> + char child[PATH_MAX];
> + int i;
> +
> + if (depth == 0)
> + return 0;
> +
> + /* Enable memory on this interior node so its children get a memcg. */
> + if (cg_write(path, "cgroup.subtree_control", "+memory"))
> + return -1;
> +
> + for (i = 0; i < fanout; i++) {
> + snprintf(child, sizeof(child), "%s/c%d", path, i);
> + if (add_node(child, depth == 1, NULL))
> + return -1;
is_leaf := depth == 1
doesn't look correct (I see below the test builds hierarchies with
depth > 1)
Ah, it's non-conventional meaning of "leaf", I'd suggest a subtree or
partition or similar (IIUC the purpose).
> + if (build_children(child, fanout, depth - 1))
> + return -1;
> + }
> + return 0;
> +}
> +
> +static size_t tree_capacity(int fanout, int depth)
> +{
> + size_t total = 1, level = 1;
> + int d;
> +
> + for (d = 0; d < depth; d++) {
> + level *= fanout;
> + total += level;
> + }
> + return total;
> +}
> +
> +/* The tree is arranged in the DFS order within an array */
> +static int build_tree(int fanout, int depth, int *root_fd)
> +{
> + n_nodes = 0;
> + n_leaves = 0;
> + nodes = calloc(tree_capacity(fanout, depth), sizeof(*nodes));
> + if (!nodes)
> + return -1;
> +
> + if (add_node(subtree_root, depth == 0, root_fd))
> + return -1;
> + return build_children(subtree_root, fanout, depth);
> +}
> +
> +/* ---- cross-CPU charge (one pinning child per leaf) --------------------- */
> +
> +static pid_t *charger_pids;
> +static int n_chargers;
> +/* parent pid, for the children's PR_SET_PDEATHSIG race check */
> +static pid_t test_pid;
> +static int charge_ready[2] = { -1, -1 }; /* child -> parent "ready" barrier */
> +static int charge_ctrl[2] = { -1, -1 }; /* parent -> child "exit" (close to signal) */
> +
> +/*
> + * One charging child, dedicated to a single leaf and spread over K CPUs. It
> + * joins its leaf, maps a resident anon region, then faults the region in K
> + * slices, each on a different CPU, so this leaf's rstat ends up dirty on K
> + * per-cpu trees. The region stays mapped, so the charge persists while the
> + * parent reads. After signalling readiness the child blocks (holding the
> + * charge) until the parent closes the control pipe. Never returns.
> + *
> + * @base is this child's starting index into cpu_list; its K CPUs are
> + * (base + 0..K-1) mod n_cpu.
> + */
> +static void charger_child(const struct cg_node *leaf, int base, int k,
> + size_t resident_bytes)
> +{
It'd be nicer if this shared alloc_* functions from test_memcontrol.c
(module adjustoment to cater both users).
> +static int file_read_node(const char *path, struct file_snap *o)
> +{
> + char buf[8192];
> +
> + memset(o, 0, sizeof(*o));
> +
> + if (cg_read(path, "memory.stat", buf, sizeof(buf)))
> + return -1;
> + parse_stat(buf, o);
We have cg_read_key_long() for such extractions.
> +
> +/* ---- correctness comparison -------------------------------------------- */
> +
> +static bool close_enough(__u64 a, __u64 b, __u64 tol)
> +{
> + return (a > b ? a - b : b - a) <= tol;
> +}
See
tools/testing/selftests/cgroup/lib/include/cgroup_util.h:values_close()
> +
> +/* Dump one node's bpf-vs-file stats; called when a mismatch is detected. */
> +static void dump_node(int i, const struct memcg_stat_snapshot *b,
> + const struct file_snap *f)
> +{
> + ksft_print_msg("node %d bpf : anon=%llu file=%llu shmem=%llu fmapped=%llu pgfault=%llu\n",
> + i, b->anon, b->file, b->shmem, b->file_mapped, b->pgfault);
> + ksft_print_msg("node %d file: anon=%llu file=%llu shmem=%llu fmapped=%llu pgfault=%llu\n",
> + i, f->anon, f->file, f->shmem, f->file_mapped, f->pgfault);
> +}
> +
> +/*
> + * Compare the BPF kfunc snapshots (round 1) against the memory.stat values
> + * (round 2), node by node. The two rounds are independent, equivalently
> + * charged trees, so the flushed stats are compared within a small tolerance
> + * that absorbs per-round overhead (i.e., a charging child's own stack pages).
> + * A wrong unit, enum or field in the kfunc path would miss by far more.
> + *
> + * Two per-round checks verify the flush itself: each leaf was charged
> + * resident_bytes of anon spread over K CPUs, so its flushed anon must be at
> + * least that; and the root's recursive anon must equal the sum of the leaves'
> + * anon (rstat propagated the charge up the tree).
> + *
> + * Returns 0 if every check passes, -1 otherwise.
> + */
> +static int check_correctness(const struct memcg_stat_snapshot *bpf,
> + const struct file_snap *file, const bool *is_leaf,
> + int n, size_t resident_bytes)
> +{
> + __u64 stat_tol = 64 * page_size;
> + __u64 pgf_tol = 1024;
These are central values of the test. It's necessary to explain the
choice of them.
> + __u64 broot = 0, bsum = 0, froot = 0, fsum = 0;
> + int i, mism = 0, flush_bad = 0;
> +
> + for (i = 0; i < n; i++) {
> + const struct memcg_stat_snapshot *b = &bpf[i];
> + const struct file_snap *f = &file[i];
> + __u64 bcur = b->usage_pages * page_size;
> +
> + /* kfunc path (round 1) compared with memory.stat path (round 2) */
> + if (!close_enough(b->anon, f->anon, stat_tol) ||
> + !close_enough(b->file, f->file, stat_tol) ||
> + !close_enough(b->shmem, f->shmem, stat_tol) ||
> + !close_enough(b->file_mapped, f->file_mapped, stat_tol) ||
> + !close_enough(b->pgfault, f->pgfault, pgf_tol)) {
> + mism++;
> + dump_node(i, b, f);
> + }
> +
> + /* memory.current is live (no flush) and must bound the flushed anon */
> + if (b->anon == 0 || b->anon > bcur) {
> + flush_bad++;
> + ksft_print_msg("node %d: anon=%llu exceeds current=%llu\n",
> + i, b->anon, bcur);
> + }
> +
> + /* each leaf's flush must have gathered the full cross-CPU charge */
> + if (is_leaf[i]) {
> + if (b->anon < resident_bytes || f->anon < resident_bytes) {
> + flush_bad++;
> + ksft_print_msg("node %d: short flush bpf=%llu file=%llu\n",
> + i, b->anon, f->anon);
> + }
> + bsum += b->anon;
> + fsum += f->anon;
> + }
> + if (i == 0) { /* nodes[0] == subtree_root */
> + broot = b->anon;
> + froot = f->anon;
> + }
> + }
> +
> + if (mism) {
> + ksft_print_msg("bpf (round 1) disagrees with memory.stat (round 2)\n");
> + return -1;
> + }
> + if (flush_bad) {
> + ksft_print_msg("flush did not aggregate the cross-cpu charge\n");
> + return -1;
> + }
> + if (broot != bsum || froot != fsum) {
> + ksft_print_msg("root anon != sum of leaf anon: bpf %llu/%llu file %llu/%llu\n",
> + broot, bsum, froot, fsum);
> + return -1;
> + }
> + if (bsum == 0) {
> + ksft_print_msg("tree carries no anon\n");
> + return -1;
> + }
> + return 0;
> +}
> +
> +/* ---- one case ---------------------------------------------------------- */
> +
> +struct testcase {
> + const char *name;
> + int fanout;
> + int depth;
> + int cpus_per_leaf; /* K: CPUs each leaf is charged on; 0 = all CPUs */
> + size_t resident_bytes; /* anon charged per leaf */
> +};
> +
> +/*
> + * Remove the subtree in reverse creation order. Nodes are recorded in DFS
> + * order (a parent precedes all its descendants), so iterating backwards
> + * removes every child before its parent.
> + */
> +static void destroy_tree(void)
> +{
> + int i;
> +
> + if (!nodes)
> + return;
> + for (i = n_nodes - 1; i >= 0; i--)
> + cg_destroy(nodes[i].path);
> + free(nodes);
> + nodes = NULL;
> +}
> +
> +/*
> + * Round 1: build and charge a fresh tree, walk it with the BPF iterator (which
> + * flushes and reads each cgroup via the memcg kfuncs), and capture one snapshot
> + * per node into @snap. @is_leaf records the tree shape so the later comparison
> + * can run after the tree is gone. Returns the node count, or -1 on failure.
> + * The tree is always torn down before returning.
> + */
> +static int capture_bpf_round(const struct testcase *tc,
> + struct memcg_stat_snapshot *snap, bool *is_leaf)
> +{
> + struct memcg_stat_cross_cpu *skel = NULL;
> + struct bpf_link *link = NULL;
> + int root_fd = -1, ret = -1, i, mfd;
> +
> + if (build_tree(tc->fanout, tc->depth, &root_fd)) {
> + ksft_print_msg("build tree (bpf) failed\n");
> + goto out;
> + }
> + if (start_chargers(tc->cpus_per_leaf, tc->resident_bytes))
> + goto out;
> +
> + skel = memcg_stat_cross_cpu__open();
> + if (!skel) {
> + ksft_print_msg("skel open failed\n");
> + goto out;
> + }
> + if (bpf_map__set_max_entries(skel->maps.results, n_nodes + 8)) {
> + ksft_print_msg("set max_entries failed\n");
> + goto out;
> + }
> + if (memcg_stat_cross_cpu__load(skel)) {
> + ksft_print_msg("skel load failed\n");
> + goto out;
> + }
> +
> + DECLARE_LIBBPF_OPTS(bpf_iter_attach_opts, opts);
> + union bpf_iter_link_info linfo = {};
> +
> + linfo.cgroup.cgroup_fd = root_fd;
> + linfo.cgroup.order = BPF_CGROUP_ITER_DESCENDANTS_PRE;
> + opts.link_info = &linfo;
> + opts.link_info_len = sizeof(linfo);
> +
> + link = bpf_program__attach_iter(skel->progs.cgroup_memcg_stat_cross_cpu,
> + &opts);
> + if (!link) {
> + ksft_print_msg("attach iter failed\n");
> + goto out;
> + }
> +
> + /* bpf walk through the cgroup tree and fetch result */
> + if (bpf_walk_once(link)) {
> + ksft_print_msg("bpf walk failed\n");
> + goto out;
> + }
> +
> + mfd = bpf_map__fd(skel->maps.results);
> + for (i = 0; i < n_nodes; i++) {
> + /* Save the position for the leaf, used for later correctness check */
> + is_leaf[i] = nodes[i].is_leaf;
> + if (bpf_map_lookup_elem(mfd, &nodes[i].id, &snap[i])) {
> + ksft_print_msg("map lookup failed for node %d\n", i);
> + goto out;
> + }
> + }
> + ret = n_nodes;
> +out:
> + bpf_link__destroy(link);
> + memcg_stat_cross_cpu__destroy(skel);
> + if (root_fd >= 0)
> + close(root_fd);
> + stop_chargers();
> + destroy_tree();
> + return ret;
> +}
> +
> +/*
> + * Round 2: build and charge an identical fresh tree, then read every cgroup via
> + * memory.stat / memory.current. The tree is reset after bpf read, so each
> + * read does a real rstat flush. Returns the node count, or -1 on failure.
> + * The tree is always clear before returning.
> + */
> +static int capture_file_round(const struct testcase *tc, struct file_snap *snap)
> +{
> + int root_fd = -1, ret = -1, i;
> +
> + if (build_tree(tc->fanout, tc->depth, &root_fd)) {
> + ksft_print_msg("build tree (file) failed\n");
> + goto out;
> + }
> + if (start_chargers(tc->cpus_per_leaf, tc->resident_bytes))
> + goto out;
> +
> + for (i = 0; i < n_nodes; i++)
> + if (file_read_node(nodes[i].path, &snap[i])) {
> + ksft_print_msg("file read failed for node %d\n", i);
> + goto out;
> + }
> + ret = n_nodes;
> +out:
> + if (root_fd >= 0)
> + close(root_fd);
> + stop_chargers();
> + destroy_tree();
> + return ret;
> +}
> +
> +static int run_case(const struct testcase *tc)
> +{
> + struct memcg_stat_snapshot *bpf = NULL;
> + struct file_snap *file = NULL;
> + bool *is_leaf = NULL;
> + size_t cap = tree_capacity(tc->fanout, tc->depth);
> + int nb, nf, ret = KSFT_FAIL;
> +
> + bpf = calloc(cap, sizeof(*bpf));
> + file = calloc(cap, sizeof(*file));
> + is_leaf = calloc(cap, sizeof(*is_leaf));
(This caught my eye, it looks like those could be better members of
struct cg_node. But I'm not 100% leaned to it.)
> + /* round 2: file reader flushes and reads a fresh, equivalent tree */
Hm, I think that's quite a strong assumption that the 2nd tree would be
equivalent (wrt memcg state). Could you back it up with some arguments
if you want to rely on it?
Thanks for an interesting comparer,
Michal
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 265 bytes --]
^ permalink raw reply
* Re: [PATCH] HID: i2c-hid: goodix: Disable VDD on VDDIO enable failure
From: sashiko-bot @ 2026-07-22 9:47 UTC (permalink / raw)
To: Chao Huang; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <tencent_FE1CA7ED99022E1734F8C9CAB538AB477405@qq.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Missing NULL check on device_get_match_data() return value leads to NULL pointer dereference if the driver is instantiated via sysfs.
--
--- Patch [1]: [PATCH] HID: i2c-hid: goodix: Disable VDD on VDDIO enable failure ---
commit 2325dfd5a42520477bfee927eabe09ddf852ba83
Author: Chao Huang <huangchao@kylinos.cn>
Subject: HID: i2c-hid: goodix: Disable VDD on VDDIO enable failure
This commit prevents a regulator enable count imbalance during device power up.
It disables the VDD regulator if enabling the VDDIO regulator fails.
> diff --git a/drivers/hid/i2c-hid/i2c-hid-of-goodix.c b/drivers/hid/i2c-hid/i2c-hid-of-goodix.c
> index f1597ad67e7c8..f4dbcd1d1d472 100644
> --- a/drivers/hid/i2c-hid/i2c-hid-of-goodix.c
> +++ b/drivers/hid/i2c-hid/i2c-hid-of-goodix.c
> @@ -51,8 +51,10 @@ static int goodix_i2c_hid_power_up(struct i2chid_ops *ops)
> return ret;
>
> ret = regulator_enable(ihid_goodix->vddio);
> - if (ret)
> + if (ret) {
> + regulator_disable(ihid_goodix->vdd);
> return ret;
> + }
>
> if (ihid_goodix->timings->post_power_delay_ms)
[Severity: High]
This is a pre-existing issue, but can this lead to a NULL pointer
dereference if the driver is instantiated via the I2C sysfs new_device
interface?
In i2c_hid_of_goodix_probe(), the driver unconditionally assigns the
result of device_get_match_data() to ihid_goodix->timings:
drivers/hid/i2c-hid/i2c-hid-of-goodix.c:i2c_hid_of_goodix_probe() {
...
ihid_goodix->timings = device_get_match_data(&client->dev);
...
}
When created via sysfs (e.g., echo gt7375p 0x12 > new_device), the device
lacks an OF/fwnode, so device_get_match_data() returns NULL.
Does this cause a crash when ihid_goodix->timings is subsequently
dereferenced here in goodix_i2c_hid_power_up()?
> msleep(ihid_goodix->timings->post_power_delay_ms);
--
Sashiko AI review · https://sashiko.dev/#/patchset/tencent_FE1CA7ED99022E1734F8C9CAB538AB477405@qq.com?part=1
^ permalink raw reply
* Re: [PATCH v2 1/4] selftests: add shared lib.bpf.mk to build BPF progs and skeletons
From: bot+bpf-ci @ 2026-07-22 9:56 UTC (permalink / raw)
To: ziyang.meme, shuah, tj, hannes, mkoutny, jikos, bentiss, void,
eddyz87
Cc: arighi, changwoo, mhocko, roman.gushchin, shakeel.butt,
muchun.song, akpm, inwardvessel, mykolal, nathan, linux-kselftest,
cgroups, linux-input, sched-ext, linux-mm, kernel-team, bpf, llvm,
linux-kernel, ziyang.meme, ast, andrii, daniel, martin.lau,
eddyz87, yonghong.song, clm, ihor.solodrai
In-Reply-To: <20260721174833.1232771-2-ziyang.meme@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 3210 bytes --]
> diff --git a/tools/testing/selftests/lib.bpf.mk b/tools/testing/selftests/lib.bpf.mk
> new file mode 100644
> index 000000000000..6f175c6568e9
> --- /dev/null
> +++ b/tools/testing/selftests/lib.bpf.mk
[ ... ]
> +# ---- libbpf (target) ------------------------------------------------------
> +# Pass ARCH/CROSS_COMPILE/CC through: lib.mk's CC is file-origin and is not
> +# exported, so without this the libbpf sub-make would rebuild for the host under
> +# a pure-LLVM cross build (make LLVM=1 ARCH=<arch>). -fPIC keeps the static
> +# libbpf linkable into position-independent (PIE) test binaries.
> +$(BPFOBJ): $(wildcard $(BPFDIR)/*.[ch] $(BPFDIR)/Makefile) \
> + $(APIDIR)/linux/bpf.h | $(BUILD_DIR)/libbpf
> + $(Q)$(MAKE) $(submake_extras) -C $(BPFDIR) OUTPUT=$(BUILD_DIR)/libbpf/ \
> + ARCH=$(ARCH) CROSS_COMPILE=$(CROSS_COMPILE) CC="$(CC)" \
> + EXTRA_CFLAGS='-g -O0 -fPIC' \
> + DESTDIR=$(SCRATCH_DIR) prefix= all install_headers
This rule hardcodes EXTRA_CFLAGS='-g -O0 -fPIC' for the libbpf sub-make.
tools/testing/selftests/bpf/Makefile instead passes
EXTRA_CFLAGS='-g $(OPT_FLAGS) $(SAN_CFLAGS) $(EXTRA_CFLAGS)'.
With -O0 pinned here, is there a way for someone to build the selftests
(including libbpf) with -O2, or with sanitizer flags? Could this at least
honor the EXTRA_CFLAGS or USERCFLAGS that lib.mk already defines?
The same hardcoding shows up in the host libbpf and host bpftool rules
below, so the same question applies to them:
> +# ---- libbpf (host) -- a distinct rule only when cross-compiling -----------
> +ifneq ($(BPFOBJ),$(HOST_BPFOBJ))
> +$(HOST_BPFOBJ): $(wildcard $(BPFDIR)/*.[ch] $(BPFDIR)/Makefile) \
> + | $(HOST_BUILD_DIR)/libbpf
> + $(Q)$(MAKE) $(submake_extras) -C $(BPFDIR) ARCH= CROSS_COMPILE= \
> + OUTPUT=$(HOST_BUILD_DIR)/libbpf/ CC=$(HOSTCC) LD=$(HOSTLD) \
> + EXTRA_CFLAGS='-g -O0' \
> + DESTDIR=$(HOST_SCRATCH_DIR) prefix= all install_headers
> +endif
> +
> +# ---- bpftool (host) -------------------------------------------------------
> +$(DEFAULT_BPFTOOL): $(wildcard $(BPFTOOLDIR)/*.[ch] $(BPFTOOLDIR)/Makefile) \
> + $(HOST_BPFOBJ) | $(HOST_BUILD_DIR)/bpftool
> + $(Q)$(MAKE) $(submake_extras) -C $(BPFTOOLDIR) \
> + ARCH= CROSS_COMPILE= CC=$(HOSTCC) LD=$(HOSTLD) \
> + EXTRA_CFLAGS='-g -O0' \
> + OUTPUT=$(HOST_BUILD_DIR)/bpftool/ \
> + LIBBPF_OUTPUT=$(HOST_BUILD_DIR)/libbpf/ \
> + LIBBPF_DESTDIR=$(HOST_SCRATCH_DIR)/ \
> + prefix= DESTDIR=$(HOST_SCRATCH_DIR)/ install-bin
[ ... ]
On the commit message:
> This saves much work for configuring selftests in other folder, such the cgroup.
This isn't a bug, but could this sentence be tightened? "in other
folder" and "such the cgroup" read a little awkwardly, and the preceding
text already explains the deduplication rationale. Perhaps something like
"This eases adding BPF-based selftests in other directories, such as
cgroup."
---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
CI run summary: https://github.com/kernel-patches/bpf/actions/runs/29908293077
^ permalink raw reply
* [PATCH RFC v2 RESEND] HID: BPF: add keyboard behavioral anomaly detection
From: Krish Gulati @ 2026-07-22 11:48 UTC (permalink / raw)
To: bentiss, jikos; +Cc: linux-input, linux-kernel, krishgulati7
This patch implements a HID-BPF struct_ops program that detects automated
HID injection attacks. It does this by measuring post-enumeration delay
and tracking inter-keystroke timing using Welford's online variance algorithm.
State is stored in a hash map keyed by the HID ID.
Detection results are currently surfaced via bpf_printk(). A
BPF_MAP_TYPE_RINGBUF interface with configurable userspace daemon
is planned; deferred pending validation of detection heuristics.
Signal design is grounded in: Neuner et al., "USBlock: Blocking
USB-based Keylogger Attacks", DBSec 2018.
Link: https://lore.kernel.org/linux-input/adSxXidgeWF0-Ewn@beelink/
Signed-off-by: Krish Gulati <krishgulati7@gmail.com>
Suggested-by: Sashiko-bot <sashiko-bot@kernel.org>
---
Changes since v1:
- Added bpf_spin_lock/unlock around the Welford compound update in
kbd_hook() to close a data race on concurrent execution (was
lockless read-modify-write).
- Switched dev_details from LRU_HASH to plain HASH so eviction
under map pressure fails the insert instead of silently
displacing an existing tracked device.
- Store report_id in dev_info and gate kbd_hook() on it, so
composite devices no longer misattribute other report types'
bytes as keystrokes.
- Zero dev_info with __builtin_memset before populating in probe()
to satisfy the verifier's stack-init tracking (was relying on a
partial designated initializer).
- Replaced active-byte-count heuristic with bytewise diff/mask
(standard_boot_keypress/nkro_keypress) so consecutive keystrokes
without an intervening empty report are still detected.
- Verified bucket-size rounding (8/16/32/64) does not drop events
or read out-of-bounds, tested via hid-replay with synthetic
report sizes (9, 14 bytes, etc.).
Known limitations, not addressed in this version:
- No cleanup on device disconnect: hid_bpf_ops has no disconnect
hook, so dev_details entries persist until the map fills.
Deferred pending maintainer guidance on the right approach.
- find_keyboard_field() returns on the first Keyboard-typed
(GenericDesktop/Keyboard) Application Collection found; a device
with multiple such collections within a single report descriptor
(as opposed to multiple HID interfaces, which are already handled
correctly and verified on real composite hardware) will only have
the first one tracked. Untested; the only hardware available for
testing exhibits the multiple-interface pattern rather than the
single-descriptor multi-collection pattern, so this path has not
been exercised.
Resending as a top-level thread (v2 was sent as a reply to v1 on July 9).
Original thread: https://lore.kernel.org/linux-input/20260709104958.38303-1-krishgulati7@gmail.com
src/bpf/testing/0010-Generic__keyboard.bpf.c | 311 +++++++++++++------
src/bpf/testing/meson.build | 1 +
2 files changed, 217 insertions(+), 95 deletions(-)
diff --git a/src/bpf/testing/0010-Generic__keyboard.bpf.c b/src/bpf/testing/0010-Generic__keyboard.bpf.c
index 9114587..c3f2364 100644
--- a/src/bpf/testing/0010-Generic__keyboard.bpf.c
+++ b/src/bpf/testing/0010-Generic__keyboard.bpf.c
@@ -24,11 +24,11 @@
#define HID_GUARD_PED_SUSPICIOUS_THRESH (50 * HID_GUARD_NSEC_PER_MSEC)
#define HID_GUARD_PED_WARNING_THRESH (300 * HID_GUARD_NSEC_PER_MSEC)
-/*not derived from real typing data yet*/
+/*not derived from real typing raw_buffer yet*/
#define HID_GUARD_MIN_SAMPLES 5
/*
- * Variance is "too metronomic
+ * Variance is "too metronomic"
* to be a human," expressed in ms^2 so we never need sqrt().
*/
#define HID_GUARD_VARIANCE_THRESH_MS2 (40ULL * 40ULL)
@@ -57,9 +57,17 @@ enum hid_guard_ped_flag {
};
struct dev_info {
- __u64 connection_time;
+ struct bpf_spin_lock lock;
+ __u8 report_id;
+ __u8 field_type;
+ __u8 prev_report[64];
+ __u16 bits_start;
+ __u16 bits_end;
+ __u16 usage_id;
__u32 report_size;
- __u64 prev_report[32];
+ __u32 keyboard_offset;
+ __u64 connection_time;
+ __u64 raw_buffer_length;
/*welford's variables*/
__u64 prev_keydown_ts;
__u64 count;
@@ -71,22 +79,8 @@ struct dev_info {
*/
};
-/*
- * BPF_MAP_TYPE_LRU_HASH:
- * Using an LRU map automatically prevents exhaustion by silently evicting
- * the oldest idle devices. It requires no syntax changes to the rest of the
- * code (lookup/update helpers work identically), but introduces behavioral
- * trade-offs:
- *
- * 1. Eviction wipes Welford variance history. An attacker
- * could theoretically flood the map to flush their device and reset their
- * score.
- * 2. PED Blindspot: Eviction deletes the 'connection_time' set during probe().
- * If an evicted device wakes up, it will bypass post-enumeration delay
- * checks.
- */
struct {
- __uint(type, BPF_MAP_TYPE_LRU_HASH);
+ __uint(type, BPF_MAP_TYPE_HASH);
__type(key, __u32);
__type(value, struct dev_info);
__uint(max_entries, 128);
@@ -138,11 +132,29 @@ static __always_inline void welford(struct dev_info *dev_state,
delta2 = x - dev_state->mean;
dev_state->M2 += (__u64)(delta * delta2);
+}
- bpf_printk("W[Count:%llu] Int:%llu ms, scaled_x:%lld\n",
- dev_state->count, interval_ms, x);
- bpf_printk(" -> delta1:%lld, mean:%lld\n", delta, dev_state->mean);
- bpf_printk(" -> delta2:%lld, M2:%llu\n", delta2, dev_state->M2);
+static __always_inline bool standard_boot_keypress(__u8 *current_report,
+ __u8 *prev_report)
+{
+ for (int i = 2; i < 8; i++) {
+ if (current_report[i] != prev_report[i])
+ return 1;
+ }
+ return 0;
+}
+
+static __always_inline bool
+nkro_keypress(__u8 *current_report, __u8 *prev_report, __u64 buffer_length)
+{
+ for (int i = 0; i < 64; i++) {
+ if (i >= buffer_length)
+ break;
+
+ if (current_report[i] & ~prev_report[i])
+ return 1;
+ }
+ return 0;
}
HID_BPF_CONFIG(HID_DEVICE(BUS_USB, HID_GROUP_ANY, HID_VID_ANY, HID_PID_ANY),
@@ -150,7 +162,7 @@ HID_BPF_CONFIG(HID_DEVICE(BUS_USB, HID_GROUP_ANY, HID_VID_ANY, HID_PID_ANY),
HID_PID_ANY));
SEC(HID_BPF_DEVICE_EVENT)
-int BPF_PROG(kdb_hook, struct hid_bpf_ctx *hctx)
+int BPF_PROG(kbd_hook, struct hid_bpf_ctx *hctx)
{
__u32 hid_id = hctx->hid->id;
@@ -161,124 +173,233 @@ int BPF_PROG(kdb_hook, struct hid_bpf_ctx *hctx)
if (!info)
return 0;
- __u32 size = info->report_size;
- __u32 fetch_size;
- __u8 *data;
+ __u8 *raw_buffer;
- if (size <= 8)
- fetch_size = 8;
- else if (size <= 16)
- fetch_size = 16;
- else
- fetch_size = 32;
+ __u8 *report_id_data = hid_bpf_get_data(hctx, 0, 1);
- data = hid_bpf_get_data(hctx, 0, fetch_size);
+ if (!report_id_data)
+ return 0;
- if (!data)
+ if (info->report_id != 0 && report_id_data[0] != info->report_id)
return 0;
- int now_active_ks = 0, was_active_ks = 0;
-#pragma unroll
- for (int i = 0; i < 32; i++) {
- if (i >= fetch_size)
- break;
- now_active_ks += ((__u8)data[i] + 255) >> 8;
- was_active_ks += ((__u8)info->prev_report[i] + 255) >> 8;
- info->prev_report[i] = data[i];
+ __u32 offset = info->keyboard_offset + (info->report_id != 0 ? 1 : 0);
+
+ __u64 buffer_length = info->raw_buffer_length;
+
+ if (buffer_length <= 8)
+ raw_buffer = hid_bpf_get_data(hctx, offset, 8);
+ else if (buffer_length <= 16)
+ raw_buffer = hid_bpf_get_data(hctx, offset, 16);
+ else if (buffer_length <= 32)
+ raw_buffer = hid_bpf_get_data(hctx, offset, 32);
+ else
+ raw_buffer = hid_bpf_get_data(hctx, offset, 64);
+
+ if (!raw_buffer)
+ return 0;
+
+ bool new_key_pressed = false;
+
+ if (info->report_id == 0) {
+ new_key_pressed =
+ standard_boot_keypress(raw_buffer, info->prev_report);
+ } else {
+ new_key_pressed = nkro_keypress(raw_buffer, info->prev_report,
+ buffer_length);
}
- __u64 current_ms = bpf_ktime_get_ns() / HID_GUARD_NSEC_PER_MSEC;
+ __u64 now = bpf_ktime_get_ns();
- if (now_active_ks > was_active_ks) {
+ __u64 interval_ms = 0;
+ __u64 variance_m2 = 0;
+ bool is_idle_gap = false;
+ bool is_suspicious = false;
+ enum hid_guard_ped_flag ped_flag = HID_GUARD_PED_NO_ENTRY;
+ bool run_ped = false;
+
+ bpf_spin_lock(&info->lock);
+
+ if (new_key_pressed) {
if (info->prev_keydown_ts != 0) {
- __u64 interval_ms = current_ms - info->prev_keydown_ts;
+ interval_ms = (now - info->prev_keydown_ts) /
+ HID_GUARD_NSEC_PER_MSEC;
- if (interval_ms < HID_GUARD_IDLE_GAP_THRESH_MS) {
+ if (interval_ms < HID_GUARD_IDLE_GAP_THRESH_MS)
welford(info, interval_ms);
- } else {
- bpf_printk(
- "hid %d: idle gap %llu ms excluded from sample\n",
- hid_id, interval_ms);
- }
+ else
+ is_idle_gap = true;
}
+ /*
+ * Variance check runs after welford() so the current
+ * sample is already folded in before we decide.
+ * Guard on MIN_SAMPLES: Welford's unbiased estimator
+ * (M2 / (count - 1)) is undefined for count < 2, and
+ * unreliable until a few samples have accumulated.
+ */
if (info->count >= HID_GUARD_MIN_SAMPLES) {
- __u64 variance_m2 =
+ variance_m2 =
info->M2 /
((__u64)HID_GUARD_WELFORD_SCALE *
HID_GUARD_WELFORD_SCALE * (info->count - 1));
- /*
- * the initial interval x was multiplied by HID_GUARD_WELFORD_SCALE, both
- * delta and delta2 are also scaled by that factor,
- * thus scale^2 in the denominator
- */
- if (variance_m2 < HID_GUARD_VARIANCE_THRESH_MS2) {
- bpf_printk(
- "hid %d: Suspeciously regular typing, variance=%llu ms^2\n",
- hid_id, variance_m2);
- }
+ if (variance_m2 < HID_GUARD_VARIANCE_THRESH_MS2)
+ is_suspicious = true;
}
- info->prev_keydown_ts = current_ms;
+ info->prev_keydown_ts = now;
}
- if (info->connection_time != 0) {
- enum hid_guard_ped_flag ped_flag =
- post_enumeration_delay(info, bpf_ktime_get_ns());
-
- bpf_printk("PED flag for hid %d: %d\n", hid_id, ped_flag);
+ for (int i = 0; i < 64; i++) {
+ if (i >= buffer_length)
+ break;
+ info->prev_report[i] = raw_buffer[i];
+ }
- /*
- * Prevent re-evaluation on subsequent packets for this device
- */
+ /*
+ * PED: fires exactly once per device lifetime. connection_time
+ * is set at probe() time; we clear it here so subsequent events
+ * skip this branch entirely.
+ */
+ if (info->connection_time != 0) {
+ ped_flag = post_enumeration_delay(info, now);
info->connection_time = 0;
+ run_ped = true;
}
+
+ bpf_spin_unlock(&info->lock);
+
+ if (new_key_pressed) {
+ if (is_idle_gap)
+ bpf_printk(
+ "hid %d: idle gap %llu ms excluded from sample\n",
+ hid_id, interval_ms);
+ if (is_suspicious)
+ bpf_printk(
+ "hid %d: suspiciously regular typing, variance=%llu ms^2\n",
+ hid_id, variance_m2);
+ }
+
+ if (run_ped)
+ bpf_printk("hid %d: PED flag=%d\n", hid_id, ped_flag);
+
return 0;
}
HID_BPF_OPS(hook_keyboard) = {
- .hid_device_event = (void *)kdb_hook,
+ .hid_device_event = (void *)kbd_hook,
};
struct hid_rdesc_descriptor HID_REPORT_DESCRIPTOR;
-SEC("syscall")
-int probe(struct hid_bpf_probe_args *ctx)
+static __always_inline bool find_keyboard_field(__u16 *bits_start,
+ __u16 *bits_end,
+ __u8 *report_id,
+ __u32 *size_in_bytes)
{
struct hid_rdesc_report *input;
struct hid_rdesc_field *field;
struct hid_rdesc_collection *col;
hid_bpf_for_each_input_report(&HID_REPORT_DESCRIPTOR, input) {
- __u32 size_in_bytes = (input->size_in_bits + 7) / 8;
-
- bpf_printk("Report size: %d\n", size_in_bytes);
- if (input->report_id != 0)
- size_in_bytes += 1;
-
- bpf_printk("Report size after report_id: %d\n", size_in_bytes);
-
hid_bpf_for_each_field(input, field) {
hid_bpf_for_each_collection(field, col) {
if (col->usage_page ==
HidUsagePage_GenericDesktop &&
col->usage_id == HidUsage_GD_Keyboard) {
- __u32 key = ctx->hid;
- struct dev_info info = {
- .connection_time =
- bpf_ktime_get_ns(),
- .count = 0,
- .report_size = size_in_bytes
- };
- bpf_map_update_elem(&dev_details, &key,
- &info, BPF_ANY);
- ctx->retval = 0;
- return 0;
+ *size_in_bytes =
+ input->size_in_bits / 8;
+
+ if (input->report_id != 0)
+ *size_in_bytes += 1;
+
+ *bits_start = field->bits_start;
+ *bits_end = field->bits_end;
+ *report_id = input->report_id;
+ return true;
}
}
}
}
- ctx->retval = -EINVAL;
+ return false;
+}
+
+SEC("syscall")
+int probe(struct hid_bpf_probe_args *ctx)
+{
+ __u8 report_id;
+ __u16 bits_start, bits_end;
+ __u32 size_in_bytes;
+ __u32 hid_id = ctx->hid;
+
+ struct dev_info *existing_info =
+ bpf_map_lookup_elem(&dev_details, &hid_id);
+
+ struct dev_info info;
+
+ __builtin_memset(&info, 0, sizeof(info));
+
+ if (!find_keyboard_field(&bits_start, &bits_end, &report_id,
+ &size_in_bytes)) {
+ ctx->retval = -EINVAL;
+ return 0;
+ }
+
+ if (existing_info != NULL) {
+ __u64 now = bpf_ktime_get_ns();
+
+ bpf_spin_lock(&existing_info->lock);
+
+ existing_info->connection_time = now;
+
+ existing_info->bits_start = bits_start;
+ existing_info->bits_end = bits_end;
+ existing_info->keyboard_offset = bits_start / 8;
+ existing_info->report_size = size_in_bytes;
+
+ if (existing_info->keyboard_offset >
+ existing_info->report_size) {
+ bpf_spin_unlock(&existing_info->lock);
+ ctx->retval = -EINVAL;
+ return 0;
+ }
+
+ existing_info->report_id = report_id;
+ existing_info->raw_buffer_length =
+ existing_info->report_size -
+ (existing_info->keyboard_offset -
+ (report_id != 0 ? 1 : 0));
+
+ existing_info->count = 0;
+ existing_info->mean = 0;
+ existing_info->M2 = 0;
+ existing_info->prev_keydown_ts = 0;
+
+ __builtin_memset(existing_info->prev_report, 0,
+ sizeof(existing_info->prev_report));
+
+ bpf_spin_unlock(&existing_info->lock);
+ ctx->retval = 0;
+ return 0;
+ }
+
+ info.connection_time = bpf_ktime_get_ns();
+ info.bits_start = bits_start;
+ info.bits_end = bits_end;
+ info.keyboard_offset = bits_start / 8;
+ info.report_size = size_in_bytes;
+
+ if (info.keyboard_offset > info.report_size) {
+ ctx->retval = -EINVAL;
+ return 0;
+ }
+
+ info.report_id = report_id;
+ info.raw_buffer_length = info.report_size - (info.keyboard_offset -
+ (report_id != 0 ? 1 : 0));
+
+ bpf_map_update_elem(&dev_details, &hid_id, &info, BPF_NOEXIST);
+ ctx->retval = 0;
return 0;
}
diff --git a/src/bpf/testing/meson.build b/src/bpf/testing/meson.build
index 9d1b5d3..2586aea 100644
--- a/src/bpf/testing/meson.build
+++ b/src/bpf/testing/meson.build
@@ -11,6 +11,7 @@ tracing_sources = [
# 'sources' are BPF programs only compatible with
# struct_ops (kernel v6.11+)
sources = [
+ '0010-Generic__keyboard.bpf.c',
]
foreach bpf: tracing_sources
--
2.55.0
^ permalink raw reply related
* [PATCH v3] HID: appletb-kbd: support layer switching on Fn double press
From: Andre Eikmeyer @ 2026-07-22 14:12 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: linux-input, linux-kernel, Aditya Garg, Andre Eikmeyer
From: Aditya Garg <gargaditya08@proton.me>
Holding Fn temporarily switches the Touch Bar between media controls and
function keys. Users who want to keep the alternate layer currently need
to change the mode through sysfs.
An optional double_press_switch_time module parameter makes a double press
persistently switch the default layer. Its value specifies the double-press
interval in milliseconds, while zero leaves the behavior disabled.
Signed-off-by: Aditya Garg <gargaditya08@proton.me>
Signed-off-by: Andre Eikmeyer <dev@deq.rocks>
---
Changes in v3:
- Renamed the backing variable to match the module parameter.
- Fixed the reported code style issues.
drivers/hid/hid-appletb-kbd.c | 54 ++++++++++++++++++++++++++++++-----
1 file changed, 47 insertions(+), 7 deletions(-)
diff --git a/drivers/hid/hid-appletb-kbd.c b/drivers/hid/hid-appletb-kbd.c
index 462010a75899..b7bbb81a8d47 100644
--- a/drivers/hid/hid-appletb-kbd.c
+++ b/drivers/hid/hid-appletb-kbd.c
@@ -56,6 +56,12 @@ static int appletb_tb_idle_timeout = 15;
module_param_named(idle_timeout, appletb_tb_idle_timeout, int, 0644);
MODULE_PARM_DESC(idle_timeout, "Idle timeout in sec");
+static int appletb_tb_double_press_switch_time;
+module_param_named(double_press_switch_time,
+ appletb_tb_double_press_switch_time, int, 0644);
+MODULE_PARM_DESC(double_press_switch_time,
+ "Fn double-press interval in ms (0 disables layer switching)");
+
struct appletb_kbd {
struct hid_field *mode_field;
struct input_handler inp_handler;
@@ -68,6 +74,7 @@ struct appletb_kbd {
bool has_turned_off;
u8 saved_mode;
u8 current_mode;
+ unsigned long last_fn_press;
};
static const struct key_entry appletb_kbd_keymap[] = {
@@ -243,6 +250,18 @@ static int appletb_kbd_hid_event(struct hid_device *hdev, struct hid_field *fiel
return kbd->current_mode == APPLETB_KBD_MODE_OFF;
}
+static u8 appletb_switch_mode(u8 mode)
+{
+ switch (mode) {
+ case APPLETB_KBD_MODE_SPCL:
+ return APPLETB_KBD_MODE_FN;
+ case APPLETB_KBD_MODE_FN:
+ return APPLETB_KBD_MODE_SPCL;
+ default:
+ return mode;
+ }
+}
+
static void appletb_kbd_inp_event(struct input_handle *handle, unsigned int type,
unsigned int code, int value)
{
@@ -250,15 +269,36 @@ static void appletb_kbd_inp_event(struct input_handle *handle, unsigned int type
reset_inactivity_timer(kbd);
- if (type == EV_KEY && code == KEY_FN && appletb_tb_fn_toggle &&
- (kbd->current_mode == APPLETB_KBD_MODE_SPCL ||
- kbd->current_mode == APPLETB_KBD_MODE_FN)) {
+ if (type == EV_KEY && code == KEY_FN &&
+ (kbd->current_mode == APPLETB_KBD_MODE_SPCL ||
+ kbd->current_mode == APPLETB_KBD_MODE_FN)) {
if (value == 1) {
- kbd->saved_mode = kbd->current_mode;
- appletb_kbd_set_mode(kbd, kbd->current_mode == APPLETB_KBD_MODE_SPCL
- ? APPLETB_KBD_MODE_FN : APPLETB_KBD_MODE_SPCL);
+ if (appletb_tb_double_press_switch_time > 0) {
+ unsigned long now = jiffies;
+
+ if (time_before(now, kbd->last_fn_press +
+ msecs_to_jiffies(appletb_tb_double_press_switch_time))) {
+ appletb_tb_def_mode =
+ appletb_switch_mode(appletb_tb_def_mode);
+
+ appletb_kbd_set_mode(kbd, appletb_tb_def_mode);
+
+ kbd->saved_mode = appletb_tb_def_mode;
+ kbd->last_fn_press = 0;
+
+ return;
+ }
+
+ kbd->last_fn_press = now;
+ }
+ if (appletb_tb_fn_toggle) {
+ kbd->saved_mode = kbd->current_mode;
+
+ appletb_kbd_set_mode(kbd, appletb_switch_mode(kbd->current_mode));
+ }
} else if (value == 0) {
- if (kbd->saved_mode != kbd->current_mode)
+ if (appletb_tb_fn_toggle &&
+ kbd->saved_mode != kbd->current_mode)
appletb_kbd_set_mode(kbd, kbd->saved_mode);
}
}
--
2.55.0
^ permalink raw reply related
* Re: [PATCH v3] HID: appletb-kbd: support layer switching on Fn double press
From: sashiko-bot @ 2026-07-22 14:23 UTC (permalink / raw)
To: Andre Eikmeyer; +Cc: linux-input, dmitry.torokhov
In-Reply-To: <20260722141221.13844-1-dev@deq.rocks>
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
New issues:
- [High] The double-press logic mutates a read-only global module parameter dynamically from a per-device interrupt-driven event handler, causing a data race and state corruption.
- [Medium] Failure of `appletb_kbd_set_mode()` is ignored in the double-press path, causing software tracking state to desynchronize from actual hardware state.
Pre-existing issues:
- [High] Calling `appletb_kbd_set_mode()` from the atomic input event handler causes a sleep in atomic context (BUG: scheduling while atomic).
--
--- Patch [1]: [PATCH v3] HID: appletb-kbd: support layer switching on Fn double press ---
commit 3f8289667f750dcd9fc87072f493735127cd7f85
Author: Aditya Garg <gargaditya08@proton.me>
HID: appletb-kbd: support layer switching on Fn double press
Holding Fn temporarily switches the Touch Bar between media controls and
function keys. An optional double_press_switch_time module parameter makes
a double press persistently switch the default layer.
> diff --git a/drivers/hid/hid-appletb-kbd.c b/drivers/hid/hid-appletb-kbd.c
> index 462010a758993..b7bbb81a8d473 100644
> --- a/drivers/hid/hid-appletb-kbd.c
> +++ b/drivers/hid/hid-appletb-kbd.c
[ ... ]
> @@ -250,15 +269,36 @@ static void appletb_kbd_inp_event(struct input_handle *handle, unsigned int type
>
> reset_inactivity_timer(kbd);
>
> - if (type == EV_KEY && code == KEY_FN && appletb_tb_fn_toggle &&
> - (kbd->current_mode == APPLETB_KBD_MODE_SPCL ||
> - kbd->current_mode == APPLETB_KBD_MODE_FN)) {
> + if (type == EV_KEY && code == KEY_FN &&
> + (kbd->current_mode == APPLETB_KBD_MODE_SPCL ||
> + kbd->current_mode == APPLETB_KBD_MODE_FN)) {
> if (value == 1) {
> - kbd->saved_mode = kbd->current_mode;
> - appletb_kbd_set_mode(kbd, kbd->current_mode == APPLETB_KBD_MODE_SPCL
> - ? APPLETB_KBD_MODE_FN : APPLETB_KBD_MODE_SPCL);
> + if (appletb_tb_double_press_switch_time > 0) {
> + unsigned long now = jiffies;
> +
> + if (time_before(now, kbd->last_fn_press +
> + msecs_to_jiffies(appletb_tb_double_press_switch_time))) {
> + appletb_tb_def_mode =
> + appletb_switch_mode(appletb_tb_def_mode);
[Severity: High]
Does this double-press logic mutate a read-only global module parameter
dynamically from a per-device interrupt-driven event handler? Modifying
it changes the initialization default for any subsequently probed devices.
If a user changes the mode via sysfs, a subsequent double-press will blindly
toggle the global variable and overwrite the user's per-device sysfs
configuration.
> +
> + appletb_kbd_set_mode(kbd, appletb_tb_def_mode);
[Severity: High]
This is a pre-existing issue, but does calling appletb_kbd_set_mode() from
the atomic input event handler cause a sleep in atomic context?
The input core invokes appletb_kbd_inp_event() from input_pass_values()
which holds dev->event_lock (spinlock) and disables local interrupts.
The callback then calls appletb_kbd_set_mode(), which calls
hid_hw_power(hdev, PM_HINT_FULLON). For USB HID devices, this maps to
usbhid_power(), which calls usb_autopm_get_interface(), subsequently
calling pm_runtime_get_sync() which sleeps.
[Severity: Medium]
Is the return value of appletb_kbd_set_mode() ignored here? If the hardware
mode switch fails, the software tracking variables are updated anyway,
causing the software to assume the mode switched successfully.
> +
> + kbd->saved_mode = appletb_tb_def_mode;
> + kbd->last_fn_press = 0;
> +
> + return;
> + }
> +
> + kbd->last_fn_press = now;
> + }
> + if (appletb_tb_fn_toggle) {
> + kbd->saved_mode = kbd->current_mode;
> +
> + appletb_kbd_set_mode(kbd, appletb_switch_mode(kbd->current_mode));
> + }
> } else if (value == 0) {
> - if (kbd->saved_mode != kbd->current_mode)
> + if (appletb_tb_fn_toggle &&
> + kbd->saved_mode != kbd->current_mode)
> appletb_kbd_set_mode(kbd, kbd->saved_mode);
> }
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260722141221.13844-1-dev@deq.rocks?part=1
^ permalink raw reply
* [PATCH v13 0/2] Lenovo Yoga Book 9 keyboard dock detection
From: Dave Carey @ 2026-07-22 14:27 UTC (permalink / raw)
To: platform-driver-x86
Cc: hdegoede, W_Armin, ilpo.jarvinen, jikos, linux-input,
linux-kernel, Dave Carey
Changes in v13:
- Resend of v12: v12 was corrupted in transit. Root cause: the ABI
documentation uses RST table separator lines (== ====...) which
contain '=' characters; git-send-email selected quoted-printable
transfer encoding due to non-ASCII in the changelog (UTF-8 author
name), encoding '=' as '=3D' and breaking git-am on the receiving
end. Fixed by setting sendemail.transferEncoding=8bit. No content
changes from v12.
Changes in v12 (addressing Ilpo Järvinen's v11 review of patch 2/2):
- Add #include <linux/sysfs.h> (for sysfs_emit)
- Add #include <linux/types.h> (for __le32 and __packed)
Changes in v11 (addressing Armin Wolf's v10 review of patch 2/2):
- ABI doc: correct Date to "June 2027" and KernelVersion to "7.3";
these fields should reflect when the ABI lands in the kernel, not
the submission date.
Patch 1/2 (lenovo-ymc) is unchanged from v10.
Changes in v10 (addressing Armin Wolf's v9 review of patch 2/2):
- Update ABI doc: Date: June 2026, KernelVersion: 6.16
- Add #include <linux/compiler_attributes.h> and <linux/types.h>
- yb9_kbdock_query(): change return to pointer-out u32 *bkbd to avoid
a 0xFFFFFFFF status value being misread as -EPERM
- struct yb9_kbdock_data: add spinlock_t lock
- yb9_kbdock_report(): hold lock across input_report_switch + input_sync
- yb9_kbdock_sync(): return int, propagate query error to callers
- yb9_kbdock_resume(), yb9_kbdock_block_probe(): propagate sync error
- struct wmi_buffer out: remove zero-initializer (not needed)
- yb9_kbdock_notify_new(): drop manual length check; use
.min_event_size = sizeof(__le32) in struct wmi_driver instead
Patch 1/2 (lenovo-ymc) is unchanged from v9.
Dave Carey (2):
platform/x86/lenovo: lenovo-ymc: Suppress probe on Yoga Book 9 14IAH10
platform/x86/lenovo: Add Yoga Book 9 keyboard dock detection driver
Documentation/ABI/testing/sysfs-driver-lenovo-yb9-kbdock | 19 ++
MAINTAINERS | 7 +
drivers/platform/x86/lenovo/Kconfig | 14 +
drivers/platform/x86/lenovo/Makefile | 1 +
drivers/platform/x86/lenovo/yb9-kbdock.c | 312 ++++++++++++
drivers/platform/x86/lenovo/ymc.c | 19 ++
6 files changed, 372 insertions(+)
2.55.0
^ permalink raw reply
* [PATCH v13 1/2] platform/x86/lenovo: lenovo-ymc: Suppress probe on Yoga Book 9 14IAH10
From: Dave Carey @ 2026-07-22 14:27 UTC (permalink / raw)
To: platform-driver-x86
Cc: hdegoede, W_Armin, ilpo.jarvinen, jikos, linux-input,
linux-kernel, Dave Carey
In-Reply-To: <20260722142712.814597-1-carvsdriver@gmail.com>
The Yoga Book 9 14IAH10 (DMI product name "83KJ") has a dedicated
yb9-kbdock WMI driver that registers an input device reporting
SW_TABLET_MODE to track the detachable Bluetooth keyboard.
lenovo-ymc also loads on this machine and creates an input node with the
SW_TABLET_MODE capability bit set. For input switches, the presence of
the capability bit has semantic meaning: userspace (e.g. GNOME) reads
the switch state at startup from every node advertising the capability
and does not expect more than one such node.
Add a DMI match for the Yoga Book 9 14IAH10 to probe() so that
lenovo-ymc returns -ENODEV on this hardware, leaving yb9-kbdock as the
sole SW_TABLET_MODE source. The ymc_ec_trigger EC write, the only
other action taken in response to a YMC event, is guarded by a separate
DMI table that excludes this machine; no other functionality is affected.
Signed-off-by: Dave Carey <carvsdriver@gmail.com>
---
drivers/platform/x86/lenovo/ymc.c | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/drivers/platform/x86/lenovo/ymc.c b/drivers/platform/x86/lenovo/ymc.c
index 470d53e3c9d29..a13c89dcb285a 100644
--- a/drivers/platform/x86/lenovo/ymc.c
+++ b/drivers/platform/x86/lenovo/ymc.c
@@ -24,6 +24,22 @@ static bool force;
module_param(force, bool, 0444);
MODULE_PARM_DESC(force, "Force loading on boards without a convertible DMI chassis-type");
+static const struct dmi_system_id lenovo_ymc_nosupport_dmi_table[] = {
+ {
+ /*
+ * Yoga Book 9 14IAH10: SW_TABLET_MODE is reported by the
+ * yb9-kbdock driver. Suppress lenovo-ymc on this machine to
+ * avoid userspace seeing two input nodes that both advertise
+ * the SW_TABLET_MODE capability.
+ */
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
+ DMI_MATCH(DMI_PRODUCT_NAME, "83KJ"),
+ },
+ },
+ { }
+};
+
static const struct dmi_system_id allowed_chasis_types_dmi_table[] = {
{
.matches = {
@@ -101,6 +117,9 @@ static int lenovo_ymc_probe(struct wmi_device *wdev, const void *ctx)
struct input_dev *input_dev;
int err;
+ if (dmi_check_system(lenovo_ymc_nosupport_dmi_table))
+ return -ENODEV;
+
if (!dmi_check_system(allowed_chasis_types_dmi_table)) {
if (force)
dev_info(&wdev->dev, "Force loading Lenovo YMC support\n");
--
2.55.0
^ permalink raw reply related
* [PATCH v13 2/2] platform/x86/lenovo: Add Yoga Book 9 keyboard dock detection driver
From: Dave Carey @ 2026-07-22 14:27 UTC (permalink / raw)
To: platform-driver-x86
Cc: hdegoede, W_Armin, ilpo.jarvinen, jikos, linux-input,
linux-kernel, Dave Carey, Hans de Goede
In-Reply-To: <20260722142712.814597-1-carvsdriver@gmail.com>
The Lenovo Yoga Book 9 14IAH10 ships with a detachable Bluetooth keyboard
that magnetically attaches to the bottom (secondary) screen in one of two
positions. The Embedded Controller tracks the attachment state in a 2-bit
field called BKBD and signals changes via WMI event GUID
806BD2A2-177B-481D-BFB5-3BA0BB4A2285 (notify ID 0xEB on the WM10 ACPI
device, _UID "GMZN").
The device contains embedded BMOF data (WQDD, 20705 bytes) documenting
both WMI interfaces used by this driver:
LENOVO_BTKBD_EVENT (event GUID): WmiDataId(1) uint32 Status.
The ACPI _WED(0xEB) method returns EC.BKBD directly as an integer,
so the notify callback receives BKBD without a separate query.
LENOVO_FEATURE_STATUS_DATA (block GUID, WQAF method): returns an
8-byte buffer {uint32 IDs=0x00060000, uint32 Status=BKBD}.
Used for the initial state read on probe and after resume.
BKBD encoding:
0 = keyboard detached
1 = keyboard docked on top half of bottom screen
2 = keyboard docked on bottom half of bottom screen
3 = reserved (not observed in practice)
This driver registers two WMI drivers sharing a module-level
BLOCKING_NOTIFIER_HEAD:
- The event driver (LENOVO_BTKBD_EVENT) uses .notify_new() to receive
a pre-parsed wmi_buffer and fires the notifier chain with the BKBD
value extracted from the buffer.
- The block driver (LENOVO_FEATURE_STATUS_DATA) owns the input_dev in
its per-device private struct. At probe time it registers a
notifier_block on the chain and reads the initial BKBD state via
wmidev_query_block(). The WMI buffer is parsed as
struct lenovo_feature_status { __le32 id; __le32 status; }, and the
ID field is verified before the status is used.
- SW_TABLET_MODE=1 is reported when the keyboard is detached;
SW_TABLET_MODE=0 when docked in either position (keyboard present).
- The raw BKBD value is exposed via read-only sysfs attribute
"keyboard_position".
- BKBD state is re-read via wmidev_query_block() on resume from
suspend or hibernation.
Tested on: Lenovo Yoga Book 9 14IAH10 (model 83KJ), kernel 7.0.
Acked-by: Hans de Goede <johannes.goede@oss.qualcomm.com>
Reviewed-by: Armin Wolf <W_Armin@gmx.de>
Signed-off-by: Dave Carey <carvsdriver@gmail.com>
---
v13:
- Resend of v12: patch was corrupted in transit. git-send-email chose
quoted-printable encoding (triggered by non-ASCII in the changelog)
which encoded '=' as '=3D', corrupting the RST table separators in
the ABI doc and breaking git-am. No content changes from v12.
v12:
- Add #include <linux/sysfs.h> (for sysfs_emit; Ilpo Järvinen)
- Add #include <linux/types.h> (for __le32 and __packed; Ilpo Järvinen)
v11 (addressing Armin Wolf's v10 review):
- ABI doc: correct Date to "June 2027" and KernelVersion to "7.3"
v10 (addressing Armin Wolf's v9 review):
- Update ABI doc: Date: June 2026, KernelVersion: 6.16
- Add #include <linux/compiler_attributes.h> and <linux/types.h>
- yb9_kbdock_query(): change return to pointer-out u32 *bkbd
- struct yb9_kbdock_data: add spinlock_t lock
- yb9_kbdock_report(): hold lock across input_report_switch + input_sync
- yb9_kbdock_sync(): return int, propagate query error to callers
- yb9_kbdock_resume(), yb9_kbdock_block_probe(): propagate sync error
- struct wmi_buffer out: remove zero-initializer
- yb9_kbdock_notify_new(): drop manual length check; use
.min_event_size = sizeof(__le32) in struct wmi_driver instead
.../testing/sysfs-driver-lenovo-yb9-kbdock | 19 ++
MAINTAINERS | 7 +
drivers/platform/x86/lenovo/Kconfig | 14 +
drivers/platform/x86/lenovo/Makefile | 1 +
drivers/platform/x86/lenovo/yb9-kbdock.c | 312 ++++++++++++++++++
5 files changed, 353 insertions(+)
create mode 100644 Documentation/ABI/testing/sysfs-driver-lenovo-yb9-kbdock
create mode 100644 drivers/platform/x86/lenovo/yb9-kbdock.c
diff --git a/Documentation/ABI/testing/sysfs-driver-lenovo-yb9-kbdock b/Documentation/ABI/testing/sysfs-driver-lenovo-yb9-kbdock
new file mode 100644
index 0000000..ebe713a
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-driver-lenovo-yb9-kbdock
@@ -0,0 +1,19 @@
+What: /sys/bus/wmi/drivers/lenovo-yb9-kbdock/<guid>/keyboard_position
+Date: June 2027
+KernelVersion: 7.3
+Contact: Dave Carey <carvsdriver@gmail.com>
+Description:
+ Read-only attribute reporting the current keyboard dock position
+ as reported by the Embedded Controller on the Lenovo Yoga Book 9
+ 14IAH10.
+
+ Possible values:
+
+ == =============================================================
+ 0 keyboard is not docked to any screen (detached)
+ 1 keyboard docked on the top half of the bottom screen
+ 2 keyboard docked on the bottom half of the bottom screen
+ == =============================================================
+
+ SW_TABLET_MODE input events are also emitted: 0 when the keyboard
+ is docked (either position), 1 when detached.
diff --git a/MAINTAINERS b/MAINTAINERS
index d1cc0e1..00e8275 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -14479,6 +14479,13 @@ L: platform-driver-x86@vger.kernel.org
S: Maintained
F: drivers/platform/x86/lenovo/wmi-hotkey-utilities.c
+LENOVO YOGA BOOK 9 KEYBOARD DOCK DRIVER
+M: Dave Carey <carvsdriver@gmail.com>
+L: platform-driver-x86@vger.kernel.org
+S: Maintained
+F: Documentation/ABI/testing/sysfs-driver-lenovo-yb9-kbdock
+F: drivers/platform/x86/lenovo/yb9-kbdock.c
+
LETSKETCH HID TABLET DRIVER
M: Hans de Goede <hansg@kernel.org>
L: linux-input@vger.kernel.org
diff --git a/drivers/platform/x86/lenovo/Kconfig b/drivers/platform/x86/lenovo/Kconfig
index 9c48487..938b361 100644
--- a/drivers/platform/x86/lenovo/Kconfig
+++ b/drivers/platform/x86/lenovo/Kconfig
@@ -43,6 +43,20 @@ config LENOVO_WMI_CAMERA
To compile this driver as a module, choose M here: the module
will be called lenovo-wmi-camera.
+config LENOVO_YB9_KBDOCK
+ tristate "Lenovo Yoga Book 9 keyboard dock detection"
+ depends on ACPI_WMI
+ depends on DMI
+ depends on INPUT
+ help
+ Say Y here to enable keyboard dock detection on the Lenovo Yoga Book 9
+ 14IAH10. The detachable Bluetooth keyboard magnetically attaches to
+ either screen; this driver reports SW_TABLET_MODE input events based
+ on the attachment state and exposes the raw position in sysfs.
+
+ To compile this driver as a module, choose M here: the module will be
+ called lenovo-yb9-kbdock.
+
config LENOVO_YMC
tristate "Lenovo Yoga Tablet Mode Control"
depends on ACPI_WMI
diff --git a/drivers/platform/x86/lenovo/Makefile b/drivers/platform/x86/lenovo/Makefile
index 7b2128e..2842d7d 100644
--- a/drivers/platform/x86/lenovo/Makefile
+++ b/drivers/platform/x86/lenovo/Makefile
@@ -8,6 +8,7 @@ obj-$(CONFIG_THINKPAD_LMI) += think-lmi.o
obj-$(CONFIG_THINKPAD_ACPI) += thinkpad_acpi.o
lenovo-target-$(CONFIG_LENOVO_WMI_HOTKEY_UTILITIES) += wmi-hotkey-utilities.o
+lenovo-target-$(CONFIG_LENOVO_YB9_KBDOCK) += yb9-kbdock.o
lenovo-target-$(CONFIG_LENOVO_YMC) += ymc.o
lenovo-target-$(CONFIG_YOGABOOK) += yogabook.o
lenovo-target-$(CONFIG_YT2_1380) += yoga-tab2-pro-1380-fastcharger.o
diff --git a/drivers/platform/x86/lenovo/yb9-kbdock.c b/drivers/platform/x86/lenovo/yb9-kbdock.c
new file mode 100644
index 0000000..69c68a0
--- /dev/null
+++ b/drivers/platform/x86/lenovo/yb9-kbdock.c
@@ -0,0 +1,312 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Lenovo Yoga Book 9 keyboard-dock detection
+ *
+ * The Yoga Book 9 ships with a detachable Bluetooth keyboard that magnetically
+ * attaches to the bottom screen in one of two positions. The EC tracks
+ * attachment state in a 2-bit field called BKBD and signals changes via WMI
+ * event 0xEB on the WM10 ACPI device (_UID "GMZN").
+ *
+ * BKBD values:
+ * 0 = keyboard detached
+ * 1 = keyboard docked on the top half of the bottom screen
+ * 2 = keyboard docked on the bottom half of the bottom screen
+ * 3 = reserved / not observed
+ *
+ * Two WMI interfaces are used (documented in embedded BMOF, WQDD, 20705 bytes):
+ *
+ * LENOVO_BTKBD_EVENT (event GUID, 806BD2A2-...)
+ * WmiDataId(1) uint32 Status — _WED(0xEB) returns EC.BKBD directly.
+ * The notify callback receives BKBD as an integer; no separate query needed.
+ *
+ * LENOVO_FEATURE_STATUS_DATA (block GUID, E7F300FA-...)
+ * WmiDataId(1) uint32 IDs = 0x00060000 (feature selector)
+ * WmiDataId(2) uint32 Status = BKBD value
+ * Used on probe and resume to read initial state.
+ *
+ * The event driver (LENOVO_BTKBD_EVENT) fires a notifier chain on each WMI
+ * event. The block driver (LENOVO_FEATURE_STATUS_DATA) owns the input_dev
+ * and registers a notifier_block to receive those events, eliminating the
+ * need for shared global state or a mutex.
+ *
+ * SW_TABLET_MODE=1 is reported when the keyboard is detached;
+ * SW_TABLET_MODE=0 when docked in either position (keyboard present).
+ * The raw BKBD value is exposed via the sysfs attribute "keyboard_position".
+ *
+ * Copyright (C) 2026 Dave Carey <carvsdriver@gmail.com>
+ */
+
+#include <linux/acpi.h>
+#include <linux/cleanup.h>
+#include <linux/dev_printk.h>
+#include <linux/dmi.h>
+#include <linux/input.h>
+#include <linux/module.h>
+#include <linux/notifier.h>
+#include <linux/pm.h>
+#include <linux/slab.h>
+#include <linux/sysfs.h>
+#include <linux/types.h>
+#include <linux/wmi.h>
+
+#define YB9_KBDOCK_EVENT_GUID "806BD2A2-177B-481D-BFB5-3BA0BB4A2285"
+#define YB9_KBDOCK_QUERY_GUID "E7F300FA-21CD-4003-ADAC-2696135982E6"
+
+/* BKBD encoding */
+#define BKBD_DETACHED 0
+
+/* LENOVO_FEATURE_STATUS_DATA feature selector */
+#define YB9_FEATURE_STATUS_ID 0x00060000u
+
+/*
+ * LENOVO_FEATURE_STATUS_DATA: 8-byte buffer {uint32 IDs, uint32 Status}.
+ * IDs is always 0x00060000; Status holds the BKBD value (0–3).
+ */
+struct lenovo_feature_status {
+ __le32 id;
+ __le32 status;
+} __packed;
+
+/* ------------------------------------------------------------------
+ * Notifier chain — event driver fires it, block driver listens
+ * ------------------------------------------------------------------ */
+
+static BLOCKING_NOTIFIER_HEAD(yb9_kbdock_chain_head);
+
+static void devm_yb9_kbdock_unregister_notifier(void *data)
+{
+ struct notifier_block *nb = data;
+
+ blocking_notifier_chain_unregister(&yb9_kbdock_chain_head, nb);
+}
+
+static int devm_yb9_kbdock_register_notifier(struct device *dev,
+ struct notifier_block *nb)
+{
+ int ret;
+
+ ret = blocking_notifier_chain_register(&yb9_kbdock_chain_head, nb);
+ if (ret < 0)
+ return ret;
+
+ return devm_add_action_or_reset(dev, devm_yb9_kbdock_unregister_notifier, nb);
+}
+
+/* ------------------------------------------------------------------
+ * Block WMI driver — LENOVO_FEATURE_STATUS_DATA
+ * (owns input_dev, sysfs, PM resume)
+ * ------------------------------------------------------------------ */
+
+struct yb9_kbdock_data {
+ struct wmi_device *wdev;
+ struct input_dev *input_dev;
+ struct notifier_block nb;
+};
+
+static int yb9_kbdock_query(struct yb9_kbdock_data *d)
+{
+ struct wmi_buffer out = {};
+ int ret;
+
+ ret = wmidev_query_block(d->wdev, 0, &out,
+ sizeof(struct lenovo_feature_status));
+ if (ret)
+ return ret;
+
+ struct lenovo_feature_status *fs __free(kfree) = out.data;
+
+ if (le32_to_cpu(fs->id) != YB9_FEATURE_STATUS_ID)
+ return -EIO;
+
+ return le32_to_cpu(fs->status);
+}
+
+static void yb9_kbdock_report(struct yb9_kbdock_data *d, u32 bkbd)
+{
+ int tablet = (bkbd == BKBD_DETACHED) ? 1 : 0;
+
+ input_report_switch(d->input_dev, SW_TABLET_MODE, tablet);
+ input_sync(d->input_dev);
+ dev_dbg(&d->wdev->dev, "BKBD=%u SW_TABLET_MODE=%d\n", bkbd, tablet);
+}
+
+static void yb9_kbdock_sync(struct yb9_kbdock_data *d)
+{
+ int bkbd = yb9_kbdock_query(d);
+
+ if (bkbd >= 0)
+ yb9_kbdock_report(d, bkbd);
+}
+
+static int yb9_kbdock_nb_call(struct notifier_block *nb,
+ unsigned long bkbd, void *unused)
+{
+ struct yb9_kbdock_data *d =
+ container_of(nb, struct yb9_kbdock_data, nb);
+
+ yb9_kbdock_report(d, bkbd);
+ return NOTIFY_DONE;
+}
+
+static ssize_t keyboard_position_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct yb9_kbdock_data *d = dev_get_drvdata(dev);
+ int bkbd = yb9_kbdock_query(d);
+
+ if (bkbd < 0)
+ return bkbd;
+ return sysfs_emit(buf, "%d\n", bkbd);
+}
+static DEVICE_ATTR_RO(keyboard_position);
+
+static const struct attribute * const yb9_kbdock_attrs[] = {
+ &dev_attr_keyboard_position.attr,
+ NULL,
+};
+ATTRIBUTE_GROUPS(yb9_kbdock);
+
+static int yb9_kbdock_resume(struct device *dev)
+{
+ struct yb9_kbdock_data *d = dev_get_drvdata(dev);
+
+ yb9_kbdock_sync(d);
+ return 0;
+}
+static DEFINE_SIMPLE_DEV_PM_OPS(yb9_kbdock_pm_ops, NULL, yb9_kbdock_resume);
+
+static int yb9_kbdock_block_probe(struct wmi_device *wdev, const void *ctx)
+{
+ struct yb9_kbdock_data *d;
+ struct input_dev *input_dev;
+ int ret;
+
+ d = devm_kzalloc(&wdev->dev, sizeof(*d), GFP_KERNEL);
+ if (!d)
+ return -ENOMEM;
+
+ d->wdev = wdev;
+
+ input_dev = devm_input_allocate_device(&wdev->dev);
+ if (!input_dev)
+ return -ENOMEM;
+
+ input_dev->name = "Lenovo Yoga Book 9 keyboard dock switch";
+ input_dev->phys = YB9_KBDOCK_QUERY_GUID "/input0";
+ input_dev->id.bustype = BUS_HOST;
+ input_set_capability(input_dev, EV_SW, SW_TABLET_MODE);
+
+ ret = input_register_device(input_dev);
+ if (ret)
+ return ret;
+
+ d->input_dev = input_dev;
+ d->nb.notifier_call = yb9_kbdock_nb_call;
+
+ ret = devm_yb9_kbdock_register_notifier(&wdev->dev, &d->nb);
+ if (ret)
+ return ret;
+
+ dev_set_drvdata(&wdev->dev, d);
+ yb9_kbdock_sync(d);
+ return 0;
+}
+
+static const struct wmi_device_id yb9_kbdock_block_id_table[] = {
+ { .guid_string = YB9_KBDOCK_QUERY_GUID },
+ { }
+};
+
+static struct wmi_driver yb9_kbdock_block_driver = {
+ .driver = {
+ .name = "lenovo-yb9-kbdock",
+ .dev_groups = yb9_kbdock_groups,
+ .pm = pm_sleep_ptr(&yb9_kbdock_pm_ops),
+ },
+ .id_table = yb9_kbdock_block_id_table,
+ .no_singleton = true,
+ .probe = yb9_kbdock_block_probe,
+};
+
+/* ------------------------------------------------------------------
+ * Event WMI driver — LENOVO_BTKBD_EVENT
+ * (fires the notifier chain on each WMI event)
+ * ------------------------------------------------------------------ */
+
+static void yb9_kbdock_notify_new(struct wmi_device *wdev,
+ const struct wmi_buffer *data)
+{
+ /*
+ * _WED(0xEB) returns EC.BKBD directly as a 32-bit integer
+ * (LENOVO_BTKBD_EVENT WmiDataId(1) uint32 Status).
+ */
+ if (!data || data->length < sizeof(__le32))
+ return;
+
+ u32 bkbd = le32_to_cpu(*(const __le32 *)data->data);
+
+ blocking_notifier_call_chain(&yb9_kbdock_chain_head, bkbd, NULL);
+}
+
+static const struct wmi_device_id yb9_kbdock_event_id_table[] = {
+ { .guid_string = YB9_KBDOCK_EVENT_GUID },
+ { }
+};
+MODULE_DEVICE_TABLE(wmi, yb9_kbdock_event_id_table);
+
+static struct wmi_driver yb9_kbdock_event_driver = {
+ .driver = {
+ .name = "lenovo-yb9-kbdock-event",
+ },
+ .id_table = yb9_kbdock_event_id_table,
+ .no_singleton = true,
+ .notify_new = yb9_kbdock_notify_new,
+};
+
+/* ------------------------------------------------------------------
+ * Module init / exit
+ * ------------------------------------------------------------------ */
+
+static const struct dmi_system_id yb9_kbdock_dmi_table[] __initconst = {
+ {
+ /* Lenovo Yoga Book 9 14IAH10 */
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
+ DMI_MATCH(DMI_PRODUCT_NAME, "83KJ"),
+ },
+ },
+ { }
+};
+
+static int __init yb9_kbdock_init(void)
+{
+ int ret;
+
+ if (!dmi_check_system(yb9_kbdock_dmi_table))
+ return -ENODEV;
+
+ ret = wmi_driver_register(&yb9_kbdock_event_driver);
+ if (ret)
+ return ret;
+
+ ret = wmi_driver_register(&yb9_kbdock_block_driver);
+ if (ret) {
+ wmi_driver_unregister(&yb9_kbdock_event_driver);
+ return ret;
+ }
+
+ return 0;
+}
+module_init(yb9_kbdock_init);
+
+static void __exit yb9_kbdock_exit(void)
+{
+ wmi_driver_unregister(&yb9_kbdock_block_driver);
+ wmi_driver_unregister(&yb9_kbdock_event_driver);
+}
+module_exit(yb9_kbdock_exit);
+
+MODULE_AUTHOR("Dave Carey <carvsdriver@gmail.com>");
+MODULE_DESCRIPTION("Lenovo Yoga Book 9 keyboard dock detection");
+MODULE_LICENSE("GPL");
--
2.55.0
^ permalink raw reply related
* Re: [PATCH v13 2/2] platform/x86/lenovo: Add Yoga Book 9 keyboard dock detection driver
From: Ilpo Järvinen @ 2026-07-22 14:42 UTC (permalink / raw)
To: Dave Carey
Cc: platform-driver-x86, hdegoede, W_Armin, jikos, linux-input, LKML,
Hans de Goede
In-Reply-To: <20260722142712.814597-3-carvsdriver@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 10431 bytes --]
On Wed, 22 Jul 2026, Dave Carey wrote:
> The Lenovo Yoga Book 9 14IAH10 ships with a detachable Bluetooth keyboard
> that magnetically attaches to the bottom (secondary) screen in one of two
> positions. The Embedded Controller tracks the attachment state in a 2-bit
> field called BKBD and signals changes via WMI event GUID
> 806BD2A2-177B-481D-BFB5-3BA0BB4A2285 (notify ID 0xEB on the WM10 ACPI
> device, _UID "GMZN").
>
> The device contains embedded BMOF data (WQDD, 20705 bytes) documenting
> both WMI interfaces used by this driver:
>
> LENOVO_BTKBD_EVENT (event GUID): WmiDataId(1) uint32 Status.
> The ACPI _WED(0xEB) method returns EC.BKBD directly as an integer,
> so the notify callback receives BKBD without a separate query.
>
> LENOVO_FEATURE_STATUS_DATA (block GUID, WQAF method): returns an
> 8-byte buffer {uint32 IDs=0x00060000, uint32 Status=BKBD}.
> Used for the initial state read on probe and after resume.
>
> BKBD encoding:
> 0 = keyboard detached
> 1 = keyboard docked on top half of bottom screen
> 2 = keyboard docked on bottom half of bottom screen
> 3 = reserved (not observed in practice)
>
> This driver registers two WMI drivers sharing a module-level
> BLOCKING_NOTIFIER_HEAD:
>
> - The event driver (LENOVO_BTKBD_EVENT) uses .notify_new() to receive
> a pre-parsed wmi_buffer and fires the notifier chain with the BKBD
> value extracted from the buffer.
>
> - The block driver (LENOVO_FEATURE_STATUS_DATA) owns the input_dev in
> its per-device private struct. At probe time it registers a
> notifier_block on the chain and reads the initial BKBD state via
> wmidev_query_block(). The WMI buffer is parsed as
> struct lenovo_feature_status { __le32 id; __le32 status; }, and the
> ID field is verified before the status is used.
>
> - SW_TABLET_MODE=1 is reported when the keyboard is detached;
> SW_TABLET_MODE=0 when docked in either position (keyboard present).
>
> - The raw BKBD value is exposed via read-only sysfs attribute
> "keyboard_position".
>
> - BKBD state is re-read via wmidev_query_block() on resume from
> suspend or hibernation.
>
> Tested on: Lenovo Yoga Book 9 14IAH10 (model 83KJ), kernel 7.0.
>
> Acked-by: Hans de Goede <johannes.goede@oss.qualcomm.com>
> Reviewed-by: Armin Wolf <W_Armin@gmx.de>
> Signed-off-by: Dave Carey <carvsdriver@gmail.com>
> ---
> v13:
> - Resend of v12: patch was corrupted in transit. git-send-email chose
> quoted-printable encoding (triggered by non-ASCII in the changelog)
> which encoded '=' as '=3D', corrupting the RST table separators in
> the ABI doc and breaking git-am. No content changes from v12.
>
> v12:
> - Add #include <linux/sysfs.h> (for sysfs_emit; Ilpo Järvinen)
> - Add #include <linux/types.h> (for __le32 and __packed; Ilpo Järvinen)
>
> v11 (addressing Armin Wolf's v10 review):
> - ABI doc: correct Date to "June 2027" and KernelVersion to "7.3"
>
> v10 (addressing Armin Wolf's v9 review):
> - Update ABI doc: Date: June 2026, KernelVersion: 6.16
> - Add #include <linux/compiler_attributes.h> and <linux/types.h>
> - yb9_kbdock_query(): change return to pointer-out u32 *bkbd
> - struct yb9_kbdock_data: add spinlock_t lock
> - yb9_kbdock_report(): hold lock across input_report_switch + input_sync
> - yb9_kbdock_sync(): return int, propagate query error to callers
> - yb9_kbdock_resume(), yb9_kbdock_block_probe(): propagate sync error
> - struct wmi_buffer out: remove zero-initializer
> - yb9_kbdock_notify_new(): drop manual length check; use
> .min_event_size = sizeof(__le32) in struct wmi_driver instead
>
> .../testing/sysfs-driver-lenovo-yb9-kbdock | 19 ++
> MAINTAINERS | 7 +
> drivers/platform/x86/lenovo/Kconfig | 14 +
> drivers/platform/x86/lenovo/Makefile | 1 +
> drivers/platform/x86/lenovo/yb9-kbdock.c | 312 ++++++++++++++++++
> 5 files changed, 353 insertions(+)
> create mode 100644 Documentation/ABI/testing/sysfs-driver-lenovo-yb9-kbdock
> create mode 100644 drivers/platform/x86/lenovo/yb9-kbdock.c
>
> diff --git a/Documentation/ABI/testing/sysfs-driver-lenovo-yb9-kbdock b/Documentation/ABI/testing/sysfs-driver-lenovo-yb9-kbdock
> new file mode 100644
> index 0000000..ebe713a
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-driver-lenovo-yb9-kbdock
> @@ -0,0 +1,19 @@
> +What: /sys/bus/wmi/drivers/lenovo-yb9-kbdock/<guid>/keyboard_position
> +Date: June 2027
> +KernelVersion: 7.3
> +Contact: Dave Carey <carvsdriver@gmail.com>
> +Description:
> + Read-only attribute reporting the current keyboard dock position
> + as reported by the Embedded Controller on the Lenovo Yoga Book 9
> + 14IAH10.
> +
> + Possible values:
> +
> + == =============================================================
> + 0 keyboard is not docked to any screen (detached)
> + 1 keyboard docked on the top half of the bottom screen
> + 2 keyboard docked on the bottom half of the bottom screen
> + == =============================================================
> +
> + SW_TABLET_MODE input events are also emitted: 0 when the keyboard
> + is docked (either position), 1 when detached.
> diff --git a/MAINTAINERS b/MAINTAINERS
> index d1cc0e1..00e8275 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -14479,6 +14479,13 @@ L: platform-driver-x86@vger.kernel.org
> S: Maintained
> F: drivers/platform/x86/lenovo/wmi-hotkey-utilities.c
>
> +LENOVO YOGA BOOK 9 KEYBOARD DOCK DRIVER
> +M: Dave Carey <carvsdriver@gmail.com>
> +L: platform-driver-x86@vger.kernel.org
> +S: Maintained
> +F: Documentation/ABI/testing/sysfs-driver-lenovo-yb9-kbdock
> +F: drivers/platform/x86/lenovo/yb9-kbdock.c
> +
> LETSKETCH HID TABLET DRIVER
> M: Hans de Goede <hansg@kernel.org>
> L: linux-input@vger.kernel.org
> diff --git a/drivers/platform/x86/lenovo/Kconfig b/drivers/platform/x86/lenovo/Kconfig
> index 9c48487..938b361 100644
> --- a/drivers/platform/x86/lenovo/Kconfig
> +++ b/drivers/platform/x86/lenovo/Kconfig
> @@ -43,6 +43,20 @@ config LENOVO_WMI_CAMERA
> To compile this driver as a module, choose M here: the module
> will be called lenovo-wmi-camera.
>
> +config LENOVO_YB9_KBDOCK
> + tristate "Lenovo Yoga Book 9 keyboard dock detection"
> + depends on ACPI_WMI
> + depends on DMI
> + depends on INPUT
> + help
> + Say Y here to enable keyboard dock detection on the Lenovo Yoga Book 9
> + 14IAH10. The detachable Bluetooth keyboard magnetically attaches to
> + either screen; this driver reports SW_TABLET_MODE input events based
> + on the attachment state and exposes the raw position in sysfs.
> +
> + To compile this driver as a module, choose M here: the module will be
> + called lenovo-yb9-kbdock.
> +
> config LENOVO_YMC
> tristate "Lenovo Yoga Tablet Mode Control"
> depends on ACPI_WMI
> diff --git a/drivers/platform/x86/lenovo/Makefile b/drivers/platform/x86/lenovo/Makefile
> index 7b2128e..2842d7d 100644
> --- a/drivers/platform/x86/lenovo/Makefile
> +++ b/drivers/platform/x86/lenovo/Makefile
> @@ -8,6 +8,7 @@ obj-$(CONFIG_THINKPAD_LMI) += think-lmi.o
> obj-$(CONFIG_THINKPAD_ACPI) += thinkpad_acpi.o
>
> lenovo-target-$(CONFIG_LENOVO_WMI_HOTKEY_UTILITIES) += wmi-hotkey-utilities.o
> +lenovo-target-$(CONFIG_LENOVO_YB9_KBDOCK) += yb9-kbdock.o
> lenovo-target-$(CONFIG_LENOVO_YMC) += ymc.o
> lenovo-target-$(CONFIG_YOGABOOK) += yogabook.o
> lenovo-target-$(CONFIG_YT2_1380) += yoga-tab2-pro-1380-fastcharger.o
> diff --git a/drivers/platform/x86/lenovo/yb9-kbdock.c b/drivers/platform/x86/lenovo/yb9-kbdock.c
> new file mode 100644
> index 0000000..69c68a0
> --- /dev/null
> +++ b/drivers/platform/x86/lenovo/yb9-kbdock.c
> @@ -0,0 +1,312 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Lenovo Yoga Book 9 keyboard-dock detection
> + *
> + * The Yoga Book 9 ships with a detachable Bluetooth keyboard that magnetically
> + * attaches to the bottom screen in one of two positions. The EC tracks
> + * attachment state in a 2-bit field called BKBD and signals changes via WMI
> + * event 0xEB on the WM10 ACPI device (_UID "GMZN").
> + *
> + * BKBD values:
> + * 0 = keyboard detached
> + * 1 = keyboard docked on the top half of the bottom screen
> + * 2 = keyboard docked on the bottom half of the bottom screen
> + * 3 = reserved / not observed
> + *
> + * Two WMI interfaces are used (documented in embedded BMOF, WQDD, 20705 bytes):
> + *
> + * LENOVO_BTKBD_EVENT (event GUID, 806BD2A2-...)
> + * WmiDataId(1) uint32 Status — _WED(0xEB) returns EC.BKBD directly.
> + * The notify callback receives BKBD as an integer; no separate query needed.
> + *
> + * LENOVO_FEATURE_STATUS_DATA (block GUID, E7F300FA-...)
> + * WmiDataId(1) uint32 IDs = 0x00060000 (feature selector)
> + * WmiDataId(2) uint32 Status = BKBD value
> + * Used on probe and resume to read initial state.
> + *
> + * The event driver (LENOVO_BTKBD_EVENT) fires a notifier chain on each WMI
> + * event. The block driver (LENOVO_FEATURE_STATUS_DATA) owns the input_dev
> + * and registers a notifier_block to receive those events, eliminating the
> + * need for shared global state or a mutex.
> + *
> + * SW_TABLET_MODE=1 is reported when the keyboard is detached;
> + * SW_TABLET_MODE=0 when docked in either position (keyboard present).
> + * The raw BKBD value is exposed via the sysfs attribute "keyboard_position".
> + *
> + * Copyright (C) 2026 Dave Carey <carvsdriver@gmail.com>
> + */
> +
> +#include <linux/acpi.h>
> +#include <linux/cleanup.h>
> +#include <linux/dev_printk.h>
> +#include <linux/dmi.h>
> +#include <linux/input.h>
> +#include <linux/module.h>
> +#include <linux/notifier.h>
> +#include <linux/pm.h>
> +#include <linux/slab.h>
> +#include <linux/sysfs.h>
> +#include <linux/types.h>
> +#include <linux/wmi.h>
Hi,
Are you sure you sent the latest version, AFAICT, v12 had this:
+#include <linux/acpi.h>
+#include <linux/cleanup.h>
+#include <linux/compiler_attributes.h>
+#include <linux/dev_printk.h>
...
+#include <linux/slab.h>
+#include <linux/spinlock.h>
+#include <linux/sysfs.h>
+#include <linux/types.h>
+#include <linux/wmi.h>
?
--
i.
^ permalink raw reply
* Re: [PATCH v12 0/3] HID: nintendo: Add preliminary Switch 2 controller
From: Silvan Jegen @ 2026-07-22 15:14 UTC (permalink / raw)
To: Vicki Pfau; +Cc: Dmitry Torokhov, Jiri Kosina, Benjamin Tissoires, linux-input
In-Reply-To: <20260715033409.3599913-1-vi@endrift.com>
Heyhey!
Vicki Pfau <vi@endrift.com> wrote:
> This series adds preliminary support for Switch 2 controllers using the
> same split-driver model as previous versions. This is a minor iteration on
> v11, fixing some nitpicks. It should be fully ready now, pending review
> from Dmitry and a HID tree maintainer.
>
> Vicki Pfau (3):
> HID: nintendo: Add preliminary Switch 2 controller driver
> HID: nintendo: Add rumble support for Switch 2 controllers
> HID: nintendo: Add unified report format support
>
> MAINTAINERS | 1 +
> drivers/hid/Kconfig | 19 +-
> drivers/hid/hid-ids.h | 4 +
> drivers/hid/hid-nintendo.c | 1682 ++++++++++++++++-
> drivers/hid/hid-nintendo.h | 72 +
> drivers/input/joystick/Kconfig | 11 +
> drivers/input/joystick/Makefile | 1 +
> drivers/input/joystick/nintendo-switch2-usb.c | 475 +++++
> 8 files changed, 2224 insertions(+), 41 deletions(-)
> create mode 100644 drivers/hid/hid-nintendo.h
> create mode 100644 drivers/input/joystick/nintendo-switch2-usb.c
I had another look at the changes compared to v11 and everything looks
good to me!
The whole series is
Reviewed-by: Silvan Jegen <s.jegen@gmail.com>
I have also tested the Nintendo Switch 2 Pro Controller with evtest
again so that part is
Tested-by: Silvan Jegen <s.jegen@gmail.com>
Cheers,
Silvan
^ permalink raw reply
* Re: [PATCH v3 4/4] dt-bindings: leds: backlight: Convert TPS65217 to DT schema
From: Eduard Bostina @ 2026-07-22 15:18 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Aaro Koskinen, Andreas Kemnade, Conor Dooley, Daniel Thompson,
devicetree, Dmitry Torokhov, dri-devel, Jingoo Han, Kevin Hilman,
Krzysztof Kozlowski, Lee Jones, linux-input, linux-kernel,
linux-leds, linux-omap, Mark Jackson, Pavel Machek, Rob Herring,
Roger Quadros, Tony Lindgren, daniel.baluta, simona.toaca,
goledhruva, m-chawdhry
In-Reply-To: <20260722-ochre-alpaca-of-tact-ef17e3@quoll>
On Wed, Jul 22, 2026 at 10:44, Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On Tue, Jul 21, 2026 at 11:21:28AM +0000, Eduard Bostina wrote:
> > Convert the Texas Instruments TPS65217 bindings to DT schema.
> >
> > During the conversion, the following updates were made:
> > - Documented the 'regulators', 'charger' and 'pwrbutton' child nodes,
> > which are used by the boards but were missing from the old txt
> > binding.
> > - Documented the 'interrupts', 'interrupt-controller', '#interrupt-cells'
> > and 'ti,pmic-shutdown-controller' properties, which are used by the
> > am335x boards and read by the driver.
> >
> > Signed-off-by: Eduard Bostina <egbostina@gmail.com>
> > ---
> > .../bindings/leds/backlight/ti,tps65217.yaml | 170 ++++++++++++++++++
> > .../leds/backlight/tps65217-backlight.txt | 27 ---
>
> Biinding looks fine to me, but as pointed out you missed to remove old
> TXT binding.
>
> Best regards,
> Krzysztof
>
Thanks. Will send v4 that removes regulator/tps65217.txt as well.
Should I move the binding to another place (mfd/ or regulator/ as
suggested)? leds/backlight/ is arguably wrong now that the schema
describes the whole pmic.
Eduard
^ permalink raw reply
* Re: [PATCH] HID: nintendo: fix out-of-bounds read in joycon_ctlr_read_handler()
From: Silvan Jegen @ 2026-07-22 15:31 UTC (permalink / raw)
To: Ibrahim Hashimov; +Cc: jikos, bentiss, linux-input, linux-kernel, stable
In-Reply-To: <20260715115253.91029-1-security@auditcode.ai>
heyhey!
Ibrahim Hashimov <security@auditcode.ai> wrote:
> joycon_ctlr_read_handler() casts an incoming HID input report to
> struct joycon_input_report and parses it, guarding the cast only with a
> 12-byte length check:
>
> if (size >= 12) /* make sure it contains the input report */
> joycon_parse_report(ctlr, (struct joycon_input_report *)data);
>
> struct joycon_input_report is 49 bytes: a 13-byte header followed by a
> union whose IMU arm is 36 bytes. For an IMU report joycon_parse_report()
> -> joycon_parse_imu_report() walks that union (struct offsets 13..48),
> so a report of exactly 12 bytes with data[0] == JC_INPUT_IMU_DATA passes
> the guard yet is read up to 37 bytes past its declared length. The
> over-read bytes are decoded into accelerometer/gyroscope values and
> forwarded to userspace through the "(IMU)" input device, leaking
> driver-internal memory. data[0] and size are fully controlled by a
> malicious or spoofed Joy-Con/Pro Controller.
>
> Receive buffers are sized to the maximum report length, so this is an
> over-read within the allocation rather than a slab OOB, but the decoded
> bytes still reach userspace.
>
> The sibling subcmd path in joycon_ctlr_handle_event() already bounds the
> same cast correctly:
>
> if (size < sizeof(struct joycon_input_report) ||
> data[0] != JC_INPUT_SUBCMD_REPLY)
> break;
>
> Use the same sizeof(struct joycon_input_report) bound here.
>
> Fixes: 2af16c1f846b ("HID: nintendo: add nintendo switch controller driver")
> Cc: stable@vger.kernel.org
> Signed-off-by: Ibrahim Hashimov <security@auditcode.ai>
> Assisted-by: AuditCode-AI:2026.07
> ---
> drivers/hid/hid-nintendo.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
This LGTM!
Reviewed-by: Silvan Jegen <s.jegen@gmail.com>
Cheers,
Silvan
>
> diff --git a/drivers/hid/hid-nintendo.c b/drivers/hid/hid-nintendo.c
> index e7302ec01ff1..11b5fe05acf4 100644
> --- a/drivers/hid/hid-nintendo.c
> +++ b/drivers/hid/hid-nintendo.c
> @@ -2607,7 +2607,12 @@ static int joycon_ctlr_read_handler(struct joycon_ctlr *ctlr, u8 *data,
> {
> if (data[0] == JC_INPUT_SUBCMD_REPLY || data[0] == JC_INPUT_IMU_DATA ||
> data[0] == JC_INPUT_MCU_DATA) {
> - if (size >= 12) /* make sure it contains the input report */
> + /*
> + * The whole struct is cast and parsed below, including the
> + * IMU/subcmd union, not just the 12-byte partial header this
> + * used to check for.
> + */
> + if (size >= sizeof(struct joycon_input_report))
> joycon_parse_report(ctlr,
> (struct joycon_input_report *)data);
> }
^ permalink raw reply
* [PATCH] iio: hid-sensor: als: scale each channel individually
From: Daniel Schaefer via B4 Relay @ 2026-07-22 17:18 UTC (permalink / raw)
To: Jiri Kosina, Jonathan Cameron, Srinivas Pandruvada, David Lechner,
Nuno Sá, Andy Shevchenko
Cc: linux-input, linux-iio, linux-kernel, Daniel Schaefer,
Sven Vainküla, linux
From: Daniel Schaefer <dhs@frame.work>
Some sensors have multiple channels (not just brightness but color ALS
sensors) with different scaling factors.
Tested on Asus ProArt PX13 laptop by Sven.
Signed-off-by: Daniel Schaefer <dhs@frame.work>
Link: https://gitlab.freedesktop.org/hadess/iio-sensor-proxy/-/merge_requests/413
Cc: Sven Vainküla <sven@xn--vainkla-r2a.ee>
Cc: Jiri Kosina <jikos@kernel.org>
Cc: Jonathan Cameron <jic23@kernel.org>
Cc: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Cc: David Lechner <dlechner@baylibre.com>
Cc: Nuno Sá <nuno.sa@analog.com>
Cc: Andy Shevchenko <andy@kernel.org>
Cc: linux-input@vger.kernel.org
Cc: linux-iio@vger.kernel.org
Cc: linux@frame.work
---
drivers/iio/light/hid-sensor-als.c | 22 ++++++++++++----------
1 file changed, 12 insertions(+), 10 deletions(-)
diff --git a/drivers/iio/light/hid-sensor-als.c b/drivers/iio/light/hid-sensor-als.c
index 384572844162..232cb58e0a9f 100644
--- a/drivers/iio/light/hid-sensor-als.c
+++ b/drivers/iio/light/hid-sensor-als.c
@@ -33,9 +33,9 @@ struct als_state {
u32 illum[CHANNEL_SCAN_INDEX_MAX];
aligned_s64 timestamp;
} scan;
- int scale_pre_decml;
- int scale_post_decml;
- int scale_precision;
+ int scale_pre_decml[CHANNEL_SCAN_INDEX_MAX];
+ int scale_post_decml[CHANNEL_SCAN_INDEX_MAX];
+ int scale_precision[CHANNEL_SCAN_INDEX_MAX];
int value_offset;
int num_channels;
s64 timestamp;
@@ -186,9 +186,11 @@ static int als_read_raw(struct iio_dev *indio_dev,
ret_type = IIO_VAL_INT;
break;
case IIO_CHAN_INFO_SCALE:
- *val = als_state->scale_pre_decml;
- *val2 = als_state->scale_post_decml;
- ret_type = als_state->scale_precision;
+ if (chan->scan_index >= CHANNEL_SCAN_INDEX_MAX)
+ return -EINVAL;
+ *val = als_state->scale_pre_decml[chan->scan_index];
+ *val2 = als_state->scale_post_decml[chan->scan_index];
+ ret_type = als_state->scale_precision[chan->scan_index];
break;
case IIO_CHAN_INFO_OFFSET:
*val = als_state->value_offset;
@@ -338,6 +340,10 @@ static int als_parse_report(struct platform_device *pdev,
als_adjust_channel_bit_mask(channels, index, st->als[i].size);
++index;
+ st->scale_precision[i] = hid_sensor_format_scale(usage_id,
+ &st->als[i], &st->scale_pre_decml[i],
+ &st->scale_post_decml[i]);
+
dev_dbg(&pdev->dev, "als %x:%x\n", st->als[i].index,
st->als[i].report_id);
}
@@ -347,10 +353,6 @@ static int als_parse_report(struct platform_device *pdev,
if (index)
ret = 0;
- st->scale_precision = hid_sensor_format_scale(usage_id,
- &st->als[CHANNEL_SCAN_INDEX_INTENSITY],
- &st->scale_pre_decml, &st->scale_post_decml);
-
return ret;
}
---
base-commit: 194a48576843858afcd9c00ed4f105ec2b398e6c
change-id: 20260723-chromaticity-88fec417f00c
Best regards,
--
Daniel Schaefer <dhs@frame.work>
^ permalink raw reply related
* Re: [PATCH] iio: hid-sensor: als: scale each channel individually
From: sashiko-bot @ 2026-07-22 17:35 UTC (permalink / raw)
To: Daniel Schaefer; +Cc: linux-input, dmitry.torokhov
In-Reply-To: <20260723-chromaticity-v1-1-5ef5746ace2a@frame.work>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues:
- [Medium] The patch fails to expose the individual scale for the CHROMATICITY_Y channel because it still uses `info_mask_shared_by_type`.
Pre-existing issues:
- [High] The driver's IIO buffer layout uses a fixed structure, but `available_scan_masks` is dynamically populated, causing misaligned buffer reads and data corruption.
--
--- Patch [1]: [PATCH] iio: hid-sensor: als: scale each channel individually ---
commit 6d49cfd4cf380845a584116e06dcf23fc8c25ad6
Author: Daniel Schaefer <dhs@frame.work>
iio: hid-sensor: als: scale each channel individually
This commit updates the hid-sensor-als driver to support individual scaling
factors for each channel, instead of using a single global scale. It stores
the scale attributes in arrays indexed by channel.
Link: https://gitlab.freedesktop.org/hadess/iio-sensor-proxy/-/merge_requests/413
> diff --git a/drivers/iio/light/hid-sensor-als.c b/drivers/iio/light/hid-sensor-als.c
> index 384572844162c..232cb58e0a9ff 100644
> --- a/drivers/iio/light/hid-sensor-als.c
> +++ b/drivers/iio/light/hid-sensor-als.c
> @@ -33,9 +33,9 @@ struct als_state {
> u32 illum[CHANNEL_SCAN_INDEX_MAX];
> aligned_s64 timestamp;
> } scan;
[Severity: High]
This is a pre-existing issue, but does this fixed struct buffer layout
cause data corruption?
The driver captures samples in als_capture_sample() into a static sparse
array (als_state->scan.illum) based on fixed channel indices. However,
indio_dev->available_scan_masks is dynamically constructed based on the
attributes present on the sensor.
The IIO core calculates scan_bytes and channel offsets assuming the buffer is
tightly packed.
When iio_push_to_buffers_with_ts() is called in als_proc_event(), it writes
the timestamp at the end of the tightly packed length, which overwrites valid
channel data in the sparse array.
The IIO demuxer then reads from incorrect offsets, scrambling the data sent to
userspace.
Should the driver pack the samples sequentially instead of using fixed index
offsets?
> - int scale_pre_decml;
> - int scale_post_decml;
> - int scale_precision;
> + int scale_pre_decml[CHANNEL_SCAN_INDEX_MAX];
> + int scale_post_decml[CHANNEL_SCAN_INDEX_MAX];
> + int scale_precision[CHANNEL_SCAN_INDEX_MAX];
> int value_offset;
> int num_channels;
> s64 timestamp;
[ ... ]
> @@ -186,9 +186,11 @@ static int als_read_raw(struct iio_dev *indio_dev,
> ret_type = IIO_VAL_INT;
> break;
> case IIO_CHAN_INFO_SCALE:
> - *val = als_state->scale_pre_decml;
> - *val2 = als_state->scale_post_decml;
> - ret_type = als_state->scale_precision;
> + if (chan->scan_index >= CHANNEL_SCAN_INDEX_MAX)
> + return -EINVAL;
> + *val = als_state->scale_pre_decml[chan->scan_index];
> + *val2 = als_state->scale_post_decml[chan->scan_index];
> + ret_type = als_state->scale_precision[chan->scan_index];
> break;
[Severity: Medium]
Since the patch introduces per-channel scaling factors, can userspace actually
read the scale for the CHROMATICITY_Y channel?
The als_channels array defines both CHANNEL_SCAN_INDEX_CHROMATICITY_X and Y
with .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE).
Because of this, the IIO core will create a single in_chromaticity_scale sysfs
attribute for both.
When read, IIO queries the first registered channel of that type (X), leaving
the Y scale factor completely inaccessible.
Does the info_mask_shared_by_type for these channels need to be changed to
info_mask_separate so userspace can read the newly added scale for
CHROMATICITY_Y?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260723-chromaticity-v1-1-5ef5746ace2a@frame.work?part=1
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox