* Re: [PATCH 38/61] net: Prefer IS_ERR_OR_NULL over manual NULL check
From: Przemek Kitszel @ 2026-03-12 16:11 UTC (permalink / raw)
To: Philipp Hahn
Cc: amd-gfx, apparmor, bpf, ceph-devel, cocci, dm-devel, dri-devel,
gfs2, intel-gfx, intel-wired-lan, iommu, kvm, linux-arm-kernel,
linux-block, linux-bluetooth, linux-btrfs, linux-cifs, linux-clk,
linux-erofs, linux-ext4, linux-fsdevel, linux-gpio, linux-hyperv,
linux-input, linux-kernel, linux-leds, linux-media, linux-mips,
linux-mm, linux-modules, linux-mtd, linux-nfs, linux-omap,
linux-phy, linux-pm, linux-rockchip, linux-s390, linux-scsi,
linux-sctp, linux-security-module, linux-sh, linux-sound,
linux-stm32, linux-trace-kernel, linux-usb, linux-wireless,
netdev, ntfs3, samba-technical, sched-ext, target-devel,
tipc-discussion, v9fs, Igor Russkikh, Andrew Lunn,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Pavan Chebbi, Michael Chan, Potnuri Bharat Teja, Tony Nguyen,
Taras Chornyi, Maxime Coquelin, Alexandre Torgue,
Iyappan Subramanian, Keyur Chudgar, Quan Nguyen, Heiner Kallweit,
Russell King
In-Reply-To: <20260310-b4-is_err_or_null-v1-38-bd63b656022d@avm.de>
On 3/10/26 12:49, Philipp Hahn wrote:
> Prefer using IS_ERR_OR_NULL() over using IS_ERR() and a manual NULL
> check.
>
> Change generated with coccinelle.
>
> To: Igor Russkikh <irusskikh@marvell.com>
> To: Andrew Lunn <andrew+netdev@lunn.ch>
> To: "David S. Miller" <davem@davemloft.net>
> To: Eric Dumazet <edumazet@google.com>
> To: Jakub Kicinski <kuba@kernel.org>
> To: Paolo Abeni <pabeni@redhat.com>
> To: Pavan Chebbi <pavan.chebbi@broadcom.com>
> To: Michael Chan <mchan@broadcom.com>
> To: Potnuri Bharat Teja <bharat@chelsio.com>
> To: Tony Nguyen <anthony.l.nguyen@intel.com>
> To: Przemek Kitszel <przemyslaw.kitszel@intel.com>
> To: Taras Chornyi <taras.chornyi@plvision.eu>
> To: Maxime Coquelin <mcoquelin.stm32@gmail.com>
> To: Alexandre Torgue <alexandre.torgue@foss.st.com>
> To: Iyappan Subramanian <iyappan@os.amperecomputing.com>
> To: Keyur Chudgar <keyur@os.amperecomputing.com>
> To: Quan Nguyen <quan@os.amperecomputing.com>
> To: Heiner Kallweit <hkallweit1@gmail.com>
> To: Russell King <linux@armlinux.org.uk>
> Cc: netdev@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Cc: intel-wired-lan@lists.osuosl.org
> Cc: linux-stm32@st-md-mailman.stormreply.com
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-usb@vger.kernel.org
> Signed-off-by: Philipp Hahn <phahn-oss@avm.de>
this is too trivial change, especially when combined like that
https://docs.kernel.org/process/maintainer-netdev.html#clean-up-patches
> ---
> drivers/net/ethernet/aquantia/atlantic/aq_ring.c | 2 +-
> drivers/net/ethernet/broadcom/tg3.c | 2 +-
> drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_flower.c | 3 +--
> drivers/net/ethernet/intel/ice/devlink/devlink.c | 2 +-
> drivers/net/ethernet/marvell/prestera/prestera_router.c | 2 +-
> drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 2 +-
> drivers/net/mdio/mdio-xgene.c | 2 +-
> drivers/net/usb/r8152.c | 2 +-
> 8 files changed, 8 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_ring.c b/drivers/net/ethernet/aquantia/atlantic/aq_ring.c
> index e270327e47fd804cc8ee5cfd53ed1b993c955c41..43edef35c4b1ff606b2f1519a07fad4c9a990ad4 100644
> --- a/drivers/net/ethernet/aquantia/atlantic/aq_ring.c
> +++ b/drivers/net/ethernet/aquantia/atlantic/aq_ring.c
> @@ -810,7 +810,7 @@ static int __aq_ring_xdp_clean(struct aq_ring_s *rx_ring,
> }
>
> skb = aq_xdp_run_prog(aq_nic, &xdp, rx_ring, buff);
> - if (IS_ERR(skb) || !skb)
> + if (IS_ERR_OR_NULL(skb))
> continue;
>
> if (ptp_hwtstamp_len > 0)
> diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c
> index 2328fce336447eb4a796f9300ccc0ab536ff0a35..8ed79f34f03d81184dcc12e6eaff009cb8f7756e 100644
> --- a/drivers/net/ethernet/broadcom/tg3.c
> +++ b/drivers/net/ethernet/broadcom/tg3.c
> @@ -7943,7 +7943,7 @@ static int tg3_tso_bug(struct tg3 *tp, struct tg3_napi *tnapi,
>
> segs = skb_gso_segment(skb, tp->dev->features &
> ~(NETIF_F_TSO | NETIF_F_TSO6));
> - if (IS_ERR(segs) || !segs) {
> + if (IS_ERR_OR_NULL(segs)) {
> tnapi->tx_dropped++;
> goto tg3_tso_bug_end;
> }
> diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_flower.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_flower.c
> index 3307e50426819087ad985178c4a5383f16b8e7b4..1c8a6445d4b2e3535d8f1b7908dd02d8dd2f23fa 100644
> --- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_flower.c
> +++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_flower.c
> @@ -1032,8 +1032,7 @@ static void ch_flower_stats_handler(struct work_struct *work)
> do {
> rhashtable_walk_start(&iter);
>
> - while ((flower_entry = rhashtable_walk_next(&iter)) &&
> - !IS_ERR(flower_entry)) {
> + while (!IS_ERR_OR_NULL((flower_entry = rhashtable_walk_next(&iter)))) {
> ret = cxgb4_get_filter_counters(adap->port[0],
> flower_entry->filter_id,
> &packets, &bytes,
> diff --git a/drivers/net/ethernet/intel/ice/devlink/devlink.c b/drivers/net/ethernet/intel/ice/devlink/devlink.c
> index 6c72bd15db6d75a1d4fa04ef8fefbd26fb6e84bd..3d08b9187fd76ca3198af28111b6f1c1765ea01e 100644
> --- a/drivers/net/ethernet/intel/ice/devlink/devlink.c
> +++ b/drivers/net/ethernet/intel/ice/devlink/devlink.c
> @@ -791,7 +791,7 @@ static void ice_traverse_tx_tree(struct devlink *devlink, struct ice_sched_node
> node->parent->rate_node);
> }
>
> - if (rate_node && !IS_ERR(rate_node))
> + if (!IS_ERR_OR_NULL(rate_node))
> node->rate_node = rate_node;
>
> traverse_children:
> diff --git a/drivers/net/ethernet/marvell/prestera/prestera_router.c b/drivers/net/ethernet/marvell/prestera/prestera_router.c
> index b036b173a308b5f994ad8538eb010fa27196988c..4492938e8a3da91d32efe8d45ccbe2eb437c0e49 100644
> --- a/drivers/net/ethernet/marvell/prestera/prestera_router.c
> +++ b/drivers/net/ethernet/marvell/prestera/prestera_router.c
> @@ -1061,7 +1061,7 @@ static void __prestera_k_arb_hw_state_upd(struct prestera_switch *sw,
> n = NULL;
> }
>
> - if (!IS_ERR(n) && n) {
> + if (!IS_ERR_OR_NULL(n)) {
> neigh_event_send(n, NULL);
> neigh_release(n);
> } else {
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index 6827c99bde8c22db42b363d2d36ad6f26075ed50..356a4e9ce04b1fcf8786d7274d31ace404be2cf6 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -1275,7 +1275,7 @@ static int stmmac_init_phy(struct net_device *dev)
> /* Some DT bindings do not set-up the PHY handle. Let's try to
> * manually parse it
> */
> - if (!phy_fwnode || IS_ERR(phy_fwnode)) {
> + if (IS_ERR_OR_NULL(phy_fwnode)) {
> int addr = priv->plat->phy_addr;
> struct phy_device *phydev;
>
> diff --git a/drivers/net/mdio/mdio-xgene.c b/drivers/net/mdio/mdio-xgene.c
> index a8f91a4b7fed0927ee14e408000cd3a2bfb9b09a..09b30b563295c6085dc1358ac361301e5cf6b2a8 100644
> --- a/drivers/net/mdio/mdio-xgene.c
> +++ b/drivers/net/mdio/mdio-xgene.c
> @@ -265,7 +265,7 @@ struct phy_device *xgene_enet_phy_register(struct mii_bus *bus, int phy_addr)
> struct phy_device *phy_dev;
>
> phy_dev = get_phy_device(bus, phy_addr, false);
> - if (!phy_dev || IS_ERR(phy_dev))
> + if (IS_ERR_OR_NULL(phy_dev))
> return NULL;
>
> if (phy_device_register(phy_dev))
> diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
> index 0c83bbbea2e7c322ee6339893e281237663bd3ae..73f17ebd7d40007eec5004f887a46249defd28ab 100644
> --- a/drivers/net/usb/r8152.c
> +++ b/drivers/net/usb/r8152.c
> @@ -2218,7 +2218,7 @@ static void r8152_csum_workaround(struct r8152 *tp, struct sk_buff *skb,
>
> features &= ~(NETIF_F_SG | NETIF_F_IPV6_CSUM | NETIF_F_TSO6);
> segs = skb_gso_segment(skb, features);
> - if (IS_ERR(segs) || !segs)
> + if (IS_ERR_OR_NULL(segs))
> goto drop;
>
> __skb_queue_head_init(&seg_list);
>
^ permalink raw reply
* Re: [PATCH 00/15] tracepoint: Avoid double static_branch evaluation at guarded call sites
From: Andrii Nakryiko @ 2026-03-12 16:54 UTC (permalink / raw)
To: Vineeth Remanan Pillai
Cc: Mathieu Desnoyers, Steven Rostedt, Peter Zijlstra,
Dmitry Ilvokhin, Masami Hiramatsu, Ingo Molnar, Jens Axboe,
io-uring, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Alexei Starovoitov, Daniel Borkmann,
Marcelo Ricardo Leitner, Xin Long, Jon Maloy, Aaron Conole,
Eelco Chaudron, Ilya Maximets, netdev, bpf, linux-sctp,
tipc-discussion, dev, Oded Gabbay, Koby Elbaz, dri-devel,
Rafael J. Wysocki, Viresh Kumar, Gautham R. Shenoy, Huang Rui,
Mario Limonciello, Len Brown, Srinivas Pandruvada, linux-pm,
MyungJoo Ham, Kyungmin Park, Chanwoo Choi, Christian König,
Sumit Semwal, linaro-mm-sig, Eddie James, Andrew Jeffery,
Joel Stanley, linux-fsi, David Airlie, Simona Vetter,
Alex Deucher, Danilo Krummrich, Matthew Brost, Philipp Stanner,
Harry Wentland, Leo Li, amd-gfx, Jiri Kosina, Benjamin Tissoires,
linux-input, Wolfram Sang, linux-i2c, Mark Brown,
Michael Hennerich, Nuno Sá, linux-spi, James E.J. Bottomley,
Martin K. Petersen, linux-scsi, Chris Mason, David Sterba,
linux-btrfs, linux-trace-kernel, linux-kernel
In-Reply-To: <CAO7JXPjnnruhM5oC6xMgnYaQ9efzYFqMCFiJLNM3HCQ+ZeCiJw@mail.gmail.com>
On Thu, Mar 12, 2026 at 9:15 AM Vineeth Remanan Pillai
<vineeth@bitbyteword.org> wrote:
>
> On Thu, Mar 12, 2026 at 11:49 AM Mathieu Desnoyers
> <mathieu.desnoyers@efficios.com> wrote:
> >
> > On 2026-03-12 11:40, Steven Rostedt wrote:
> > > On Thu, 12 Mar 2026 11:28:07 -0400
> > > Mathieu Desnoyers <mathieu.desnoyers@efficios.com> wrote:
> > >
> > >>> Note, Vineeth came up with the naming. I would have done "do" but when I
> > >>> saw "invoke" I thought it sounded better.
> > >>
> > >> It works as long as you don't have a tracing subsystem called
> > >> "invoke", then you get into identifier clash territory.
> > >
> > > True. Perhaps we should do the double underscore trick.
> > >
> > > Instead of: trace_invoke_foo()
> > >
> > > use: trace_invoke__foo()
> > >
> > >
> > > Which will make it more visible to what the trace event is.
> > >
> > > Hmm, we probably should have used: trace__foo() for all tracepoints, as
> > > there's still functions that are called trace_foo() that are not
> > > tracepoints :-p
> >
> > One certain way to eliminate identifier clash would be to go for a
> > prefix to "trace_", e.g.
> >
> > do_trace_foo()
> > call_trace_foo()
>
> This was the initial idea, but it had conflict in the existing source:
> call_trace_sched_update_nr_running. do_trace_##name also had
> collisions when I checked. So, went with trace_invoke_##name. Did not
> check rest of the suggestions here though.
>
> Thanks,
> Vineeth
>
> > emit_trace_foo()
> > __trace_foo()
this seems like the best approach, IMO. double-underscored variants
are usually used for some specialized/internal version of a function
when we know that some conditions are correct (e.g., lock is already
taken, or something like that). Which fits here: trace_xxx() will
check if tracepoint is enabled, while __trace_xxx() will not check and
just invoke the tracepoint? It's short, it's distinct, and it says "I
know what I am doing".
> > invoke_trace_foo()
> > dispatch_trace_foo()
> >
> > Thanks,
> >
> > Mathieu
> >
> >
> >
> > --
> > Mathieu Desnoyers
> > EfficiOS Inc.
> > https://www.efficios.com
>
^ permalink raw reply
* Re: [PATCH 00/61] treewide: Use IS_ERR_OR_NULL over manual NULL check - refactor
From: Jason Gunthorpe @ 2026-03-12 16:54 UTC (permalink / raw)
To: James Bottomley
Cc: Kuan-Wei Chiu, Philipp Hahn, amd-gfx, apparmor, bpf, ceph-devel,
cocci, dm-devel, dri-devel, gfs2, intel-gfx, intel-wired-lan,
iommu, kvm, linux-arm-kernel, linux-block, linux-bluetooth,
linux-btrfs, linux-cifs, linux-clk, linux-erofs, linux-ext4,
linux-fsdevel, linux-gpio, linux-hyperv, linux-input,
linux-kernel, linux-leds, linux-media, linux-mips, linux-mm,
linux-modules, linux-mtd, linux-nfs, linux-omap, linux-phy,
linux-pm, linux-rockchip, linux-s390, linux-scsi, linux-sctp,
linux-security-module, linux-sh, linux-sound, linux-stm32,
linux-trace-kernel, linux-usb, linux-wireless, netdev, ntfs3,
samba-technical, sched-ext, target-devel, tipc-discussion, v9fs
In-Reply-To: <f5688b895eaebabae6545a0d9baf8f1404e8454e.camel@HansenPartnership.com>
On Thu, Mar 12, 2026 at 11:32:37AM -0400, James Bottomley wrote:
> On Thu, 2026-03-12 at 09:57 -0300, Jason Gunthorpe wrote:
> > On Wed, Mar 11, 2026 at 02:40:36AM +0800, Kuan-Wei Chiu wrote:
> >
> > > IMHO, the necessity of IS_ERR_OR_NULL() often highlights a
> > > confusing or flawed API design. It usually implies that the caller
> > > is unsure whether a failure results in an error pointer or a NULL
> > > pointer.
> >
> > +1
> >
> > IS_ERR_OR_NULL() should always be looked on with suspicion. Very
> > little should be returning some tri-state 'ERR' 'NULL' 'SUCCESS'
> > pointer. What does the middle condition even mean? IS_ERR_OR_NULL()
> > implies ERR and NULL are semanticly the same, so fix the things to
> > always use ERR.
>
> Not in any way supporting the original patch. However, the pattern
> ERR, NULL, PTR is used extensively in the dentry code of filesystems.
> See the try_lookup..() set of functions in fs/namei.c
>
> The meaning is
>
> PTR - I found it
> NULL - It definitely doesn't exist
> ERR - something went wrong during the lookup.
>
> So I don't think you can blanket say this pattern is wrong.
Lots of places also would return ENOENT, I'd argue that is easier to
use..
But yes, I did use the word "suspicion" not blanket wrong :)
Jason
^ permalink raw reply
* Re: [PATCH 00/15] tracepoint: Avoid double static_branch evaluation at guarded call sites
From: Steven Rostedt @ 2026-03-12 17:02 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: Vineeth Remanan Pillai, Mathieu Desnoyers, Peter Zijlstra,
Dmitry Ilvokhin, Masami Hiramatsu, Ingo Molnar, Jens Axboe,
io-uring, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Alexei Starovoitov, Daniel Borkmann,
Marcelo Ricardo Leitner, Xin Long, Jon Maloy, Aaron Conole,
Eelco Chaudron, Ilya Maximets, netdev, bpf, linux-sctp,
tipc-discussion, dev, Oded Gabbay, Koby Elbaz, dri-devel,
Rafael J. Wysocki, Viresh Kumar, Gautham R. Shenoy, Huang Rui,
Mario Limonciello, Len Brown, Srinivas Pandruvada, linux-pm,
MyungJoo Ham, Kyungmin Park, Chanwoo Choi, Christian König,
Sumit Semwal, linaro-mm-sig, Eddie James, Andrew Jeffery,
Joel Stanley, linux-fsi, David Airlie, Simona Vetter,
Alex Deucher, Danilo Krummrich, Matthew Brost, Philipp Stanner,
Harry Wentland, Leo Li, amd-gfx, Jiri Kosina, Benjamin Tissoires,
linux-input, Wolfram Sang, linux-i2c, Mark Brown,
Michael Hennerich, Nuno Sá, linux-spi, James E.J. Bottomley,
Martin K. Petersen, linux-scsi, Chris Mason, David Sterba,
linux-btrfs, linux-trace-kernel, linux-kernel
In-Reply-To: <CAEf4BzbnfyhCqp0ne=2gRnVxp-mdGmuZwDeFRyhRYH+eDcz2-w@mail.gmail.com>
On Thu, 12 Mar 2026 09:54:29 -0700
Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote:
> > > emit_trace_foo()
> > > __trace_foo()
>
> this seems like the best approach, IMO. double-underscored variants
> are usually used for some specialized/internal version of a function
> when we know that some conditions are correct (e.g., lock is already
> taken, or something like that). Which fits here: trace_xxx() will
> check if tracepoint is enabled, while __trace_xxx() will not check and
> just invoke the tracepoint? It's short, it's distinct, and it says "I
> know what I am doing".
Honestly, I consider double underscore as internal only and not something
anyone but the subsystem maintainers use.
This, is a normal function where it's just saying: If you have it already
enabled, then you can use this. Thus, I don't think it qualifies as a "you
know what you are doing".
Perhaps: call_trace_foo() ?
-- Steve
^ permalink raw reply
* Re: [tip:timers/vdso 37/45] progs/hid_bpf_helpers.h:117:31: error: declaration of 'struct bpf_wq' will not be visible outside of this function
From: Benjamin Tissoires @ 2026-03-12 17:34 UTC (permalink / raw)
To: Thomas Weißschuh
Cc: Jiri Kosina, linux-input, kernel test robot, oe-kbuild-all,
linux-kernel, x86, Thomas Gleixner
In-Reply-To: <20260312073810-db531b6e-ed24-4a55-865e-e7b94e90b8e6@linutronix.de>
On Mar 12 2026, Thomas Weißschuh wrote:
> Hi Benjamin,
>
> thanks for the quick response.
>
> On Wed, Mar 11, 2026 at 06:05:25PM +0100, Benjamin Tissoires wrote:
> > On Mar 11 2026, Thomas Wei�schuh wrote:
> > > could you take a look at the report below?
> > > This looks like an issue in the HID BPF subsystem, surfaced by my
> > > unrelated change. Does this ring a bell for you?
> > >
> > > I wasn't able to reproduce it so far, but will continue looking into it.
> >
> > Both struct bpf_wq and struct hid_device should be generated in the
> > vmlinux.h that we include in the selftests. So this is definitely not
> > related to your patch AFAICT.
>
> Ack. To be sure I sent this branch again through 0day and will see if it
> breaks on the same commit.
>
> > Looking in the config, we have `# CONFIG_HID_SUPPORT is not set` -> so
> > that would explain why struct hid_device is not available. But in that
> > case, why are the HID selftests even built?
>
> CONFIG_DEBUG_INFO_BTF is also not set. So there should be no vmlinux.h
> at all in the first. Which is exactly what happens in my attempts
> to reproduce the issue. But even when fixing that up, the issue persists.
>
> > The bpf_wq bits should be related to a similar-ish issue where one
> > config setting is not set and so it's not included in the final BTF.
>
> I'll look into how exactly things end up in vmlinux.h.
> At least the headers for 'struct bpf_wq' are always included somewhere.
> But maybe the type also needs to be used to define some structure.
>
> > I paged out how we can ignore selftests based on the .config, so if you
> > have any hints, that would be nice :)
>
> Inspecting the kernel configuration might be hard, as there might be no file
> for it available. Could you use vmlinux.h itself for feature detection?
>
Actually I think I remember the rationale:
- because working with config is hard, we just hide any struct
definition we need in progs/hid_bpf_helpers.h before including
vmlinux.h
- then we manually define them
So it looks like this is a step I forgot to make when I added the last
few bits: redefine struct bpf_wq and struct hid_device.
Technically we shouldn't even need to redefine the entire struct, but
only the bits we are accessing, because bpf with CO-RE will do the
offsets for us :)
Would the following patch fixes the issue?:
---
From bf4030f8116a4bcafe9f8d84f3d03dd2eedc58a4 Mon Sep 17 00:00:00 2001
From: Benjamin Tissoires <bentiss@kernel.org>
Date: Thu, 12 Mar 2026 18:29:40 +0100
Subject: [PATCH] selftests/hid: fix compilation when bpf_wq and hid_device are
not exported
This can happen in situations when CONFIG_HID_SUPPORT is set to no, or
some complex situations where struct bpf_wq is not exported.
So do the usual dance of hiding them before including vmlinux.h, and
then redefining them and make use of CO-RE to have the correct offsets.
Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
---
tools/testing/selftests/hid/progs/hid_bpf_helpers.h | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/tools/testing/selftests/hid/progs/hid_bpf_helpers.h b/tools/testing/selftests/hid/progs/hid_bpf_helpers.h
index 80ab60905865..2c6ec907dd05 100644
--- a/tools/testing/selftests/hid/progs/hid_bpf_helpers.h
+++ b/tools/testing/selftests/hid/progs/hid_bpf_helpers.h
@@ -8,9 +8,11 @@
/* "undefine" structs and enums in vmlinux.h, because we "override" them below */
#define hid_bpf_ctx hid_bpf_ctx___not_used
#define hid_bpf_ops hid_bpf_ops___not_used
+#define hid_device hid_device___not_used
#define hid_report_type hid_report_type___not_used
#define hid_class_request hid_class_request___not_used
#define hid_bpf_attach_flags hid_bpf_attach_flags___not_used
+#define bpf_wq bpf_wq___not_used
#define HID_INPUT_REPORT HID_INPUT_REPORT___not_used
#define HID_OUTPUT_REPORT HID_OUTPUT_REPORT___not_used
#define HID_FEATURE_REPORT HID_FEATURE_REPORT___not_used
@@ -29,9 +31,11 @@
#undef hid_bpf_ctx
#undef hid_bpf_ops
+#undef hid_device
#undef hid_report_type
#undef hid_class_request
#undef hid_bpf_attach_flags
+#undef bpf_wq
#undef HID_INPUT_REPORT
#undef HID_OUTPUT_REPORT
#undef HID_FEATURE_REPORT
@@ -55,6 +59,14 @@ enum hid_report_type {
HID_REPORT_TYPES,
};
+struct hid_device {
+ unsigned int id;
+} __attribute__((preserve_access_index));
+
+struct bpf_wq {
+ __u64 __opaque[2];
+};
+
struct hid_bpf_ctx {
struct hid_device *hid;
__u32 allocated_size;
--
2.52.0
---
Cheers,
Benjamin
>
> Thomas
>
> > > On Wed, Mar 11, 2026 at 03:29:54PM +0100, kernel test robot wrote:
> > > > tree: https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git timers/vdso
> > > > head: f7178f159b2a36d070fd43b0d751e4e4415ec39e
> > > > commit: 912632a7fd4cc1eac2778828d92e8fe46939d6bd [37/45] vdso/datapage: Trim down unnecessary includes
> > > > config: riscv-allnoconfig-bpf (https://download.01.org/0day-ci/archive/20260311/202603111558.KLCIxsZB-lkp@intel.com/config)
> > > > compiler: riscv64-linux-gnu-gcc (Debian 14.2.0-19) 14.2.0
> > > > reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260311/202603111558.KLCIxsZB-lkp@intel.com/reproduce)
> > > >
> > > > If you fix the issue in a separate patch/commit (i.e. not just a new version of
> > > > the same patch/commit), kindly add following tags
> > > > | Reported-by: kernel test robot <lkp@intel.com>
> > > > | Closes: https://lore.kernel.org/oe-kbuild-all/202603111558.KLCIxsZB-lkp@intel.com/
> > > >
> > > > All errors (new ones prefixed by >>):
> > > >
> > > > In file included from progs/hid.c:3:
> > > > >> progs/hid_bpf_helpers.h:117:31: error: declaration of 'struct bpf_wq' will not be visible outside of this function [-Werror,-Wvisibility]
> > > > 117 | extern int bpf_wq_init(struct bpf_wq *wq, void *p__map, unsigned int flags) __weak __ksym;
> > > > | ^
> > > > progs/hid_bpf_helpers.h:118:32: error: declaration of 'struct bpf_wq' will not be visible outside of this function [-Werror,-Wvisibility]
> > > > 118 | extern int bpf_wq_start(struct bpf_wq *wq, unsigned int flags) __weak __ksym;
> > > > | ^
> > > > progs/hid_bpf_helpers.h:119:39: error: declaration of 'struct bpf_wq' will not be visible outside of this function [-Werror,-Wvisibility]
> > > > 119 | extern int bpf_wq_set_callback(struct bpf_wq *wq,
> > > > | ^
> > > > >> progs/hid.c:448:16: error: field has incomplete type 'struct bpf_wq'
> > > > 448 | struct bpf_wq work;
> > > > | ^
> > > > progs/hid.c:448:9: note: forward declaration of 'struct bpf_wq'
> > > > 448 | struct bpf_wq work;
> > > > | ^
> > > > >> progs/hid.c:487:18: error: incompatible pointer types passing 'struct bpf_wq *' to parameter of type 'struct bpf_wq *' [-Wincompatible-pointer-types]
> > > > 487 | if (bpf_wq_init(wq, &hmap, 0) != 0)
> > > > | ^~
> > > > progs/hid_bpf_helpers.h:117:39: note: passing argument to parameter 'wq' here
> > > > 117 | extern int bpf_wq_init(struct bpf_wq *wq, void *p__map, unsigned int flags) __weak __ksym;
> > > > | ^
> > > > progs/hid.c:490:26: error: incompatible pointer types passing 'struct bpf_wq *' to parameter of type 'struct bpf_wq *' [-Wincompatible-pointer-types]
> > > > 490 | if (bpf_wq_set_callback(wq, wq_cb_sleepable, 0))
> > > > | ^~
> > > > progs/hid_bpf_helpers.h:119:47: note: passing argument to parameter 'wq' here
> > > > 119 | extern int bpf_wq_set_callback(struct bpf_wq *wq,
> > > > | ^
> > > > progs/hid.c:493:19: error: incompatible pointer types passing 'struct bpf_wq *' to parameter of type 'struct bpf_wq *' [-Wincompatible-pointer-types]
> > > > 493 | if (bpf_wq_start(wq, 0))
> > > > | ^~
> > > > progs/hid_bpf_helpers.h:118:40: note: passing argument to parameter 'wq' here
> > > > 118 | extern int bpf_wq_start(struct bpf_wq *wq, unsigned int flags) __weak __ksym;
> > > > | ^
> > > > progs/hid.c:503:24: error: incomplete definition of type 'struct hid_device'
> > > > 503 | int hid = hid_ctx->hid->id;
> > > > | ~~~~~~~~~~~~^
> > > > progs/hid_bpf_helpers.h:59:9: note: forward declaration of 'struct hid_device'
> > > > 59 | struct hid_device *hid;
> > > > | ^
> > > > 8 errors generated.
> > > >
> > > > --
> > > > 0-DAY CI Kernel Test Service
> > > > https://github.com/intel/lkp-tests/wiki
> > >
>
^ permalink raw reply related
* [PATCH v5 2/4] dt-bindings: input: add settling-time-us common property
From: Hugo Villeneuve @ 2026-03-12 18:00 UTC (permalink / raw)
To: robin, andy, geert, robh, krzk+dt, conor+dt, dmitry.torokhov,
hvilleneuve, mkorpershoek, matthias.bgg,
angelogioacchino.delregno, lee, alexander.sverdlin, marek.vasut,
akurz
Cc: devicetree, linux-kernel, linux-input, linux-arm-kernel,
linux-mediatek, hugo
In-Reply-To: <20260312180304.3865850-1-hugo@hugovil.com>
From: Hugo Villeneuve <hvilleneuve@dimonoff.com>
Add common property that can be reused by other bindings.
Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
---
Documentation/devicetree/bindings/input/input.yaml | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/Documentation/devicetree/bindings/input/input.yaml b/Documentation/devicetree/bindings/input/input.yaml
index 502e0b7eb500a..64d1c46cb2f2a 100644
--- a/Documentation/devicetree/bindings/input/input.yaml
+++ b/Documentation/devicetree/bindings/input/input.yaml
@@ -66,6 +66,14 @@ properties:
reset automatically. Device with key pressed reset feature can specify
this property.
+ settling-time-us:
+ description:
+ Delay, in microseconds, when activating an output line/col/row before
+ we can reliably read other input lines that maybe affected by this
+ output. This can be the case for an output with a RC circuit that affects
+ ramp-up/down times.
+ default: 0
+
dependencies:
linux,input-type: [ "linux,code" ]
--
2.47.3
^ permalink raw reply related
* [PATCH v5 0/4] input: add GPIO-based charlieplex keypad
From: Hugo Villeneuve @ 2026-03-12 18:00 UTC (permalink / raw)
To: robin, andy, geert, robh, krzk+dt, conor+dt, dmitry.torokhov,
hvilleneuve, mkorpershoek, matthias.bgg,
angelogioacchino.delregno, lee, alexander.sverdlin, marek.vasut,
akurz
Cc: devicetree, linux-kernel, linux-input, linux-arm-kernel,
linux-mediatek, hugo
From: Hugo Villeneuve <hvilleneuve@dimonoff.com>
Hello,
this patch series add a new GPIO charlieplex keypad driver.
The first two patches simply commonize two properties that are present in
a few bindings, so that the actual patches for the charlieplex keypad driver
can reuse them instead of also redefining them.
I have tested the driver on a custom board with a Solidrun RZ/G2LC SOM
with three charlieplex keyboards, all connected thru a single PCAL6416 I2C GPIO
expander.
Link: [v1] https://lore.kernel.org/all/20260203155023.536103-1-hugo@hugovil.com/
Link: [v2] https://lore.kernel.org/all/20260213171431.2228814-1-hugo@hugovil.com/
Link: [v3] https://lore.kernel.org/all/20260225155409.612478-1-hugo@hugovil.com/
Link: [v4] https://lore.kernel.org/all/20260305192101.2125660-1-hugo@hugovil.com/
Changes for v5:
- Add Reviewed-by tags (Rob)
- Replace GPIO with line in settling-time-us property description (Rob)
- Reorder Makefile entry (Andy)
- Change copyright date to 2026 (Andy)
- Change some signed variables to unsigned (Andy)
- Move some variables declaration within loop (Andy)
- Add proper labels for GPIO pin names (Andy)
Changes for v4:
- Fix indentation in comments (Andy)
- Add missing includes (Andy)
- Remove OF dependency (Andy/Dmitry)
- Uniformize return code variables to "err" (Andy/Dmitry)
- Change signed iterator to unsigned and move within loop (Andy)
- Remove unused platform_set_drvdata() (Andy)
- Fixed typo in cover letter PCAL6416 (Geert)
- Changed name in bindings example (Geert)
- Added pull resistors to bindings doc and example (Geert)
Changes for v3:
- Add ASCII diagram in bindings, and reference to it in example
- Reorder properties alphabetically
- Add patch to define common input settling-time-us property
- Add patch to define common input debounce-delay-ms property
Changes for v2:
- Fix yamllint error for example
- Remove unused debug variable (nkeys)
- Remove support for custom linux,no-autorepeat DT property
- Remove support for custom gpio-activelow DT property
Thank you.
Hugo Villeneuve (4):
dt-bindings: input: add debounce-delay-ms common property
dt-bindings: input: add settling-time-us common property
dt-bindings: input: add GPIO charlieplex keypad
Input: charlieplex_keypad: add GPIO charlieplex keypad
.../bindings/auxdisplay/holtek,ht16k33.yaml | 5 +-
.../bindings/input/cirrus,ep9307-keypad.yaml | 7 +-
.../input/gpio-charlieplex-keypad.yaml | 108 +++++++++
.../bindings/input/gpio-matrix-keypad.yaml | 5 +-
.../devicetree/bindings/input/input.yaml | 16 ++
.../input/mediatek,mt6779-keypad.yaml | 1 +
.../devicetree/bindings/mfd/fsl,mc13xxx.yaml | 2 -
MAINTAINERS | 7 +
drivers/input/keyboard/Kconfig | 14 ++
drivers/input/keyboard/Makefile | 1 +
drivers/input/keyboard/charlieplex_keypad.c | 223 ++++++++++++++++++
11 files changed, 377 insertions(+), 12 deletions(-)
create mode 100644 Documentation/devicetree/bindings/input/gpio-charlieplex-keypad.yaml
create mode 100644 drivers/input/keyboard/charlieplex_keypad.c
base-commit: 6d4b67a2a76a4ff2393fe88119ae4332821b82b4
--
2.47.3
^ permalink raw reply
* [PATCH v5 1/4] dt-bindings: input: add debounce-delay-ms common property
From: Hugo Villeneuve @ 2026-03-12 18:00 UTC (permalink / raw)
To: robin, andy, geert, robh, krzk+dt, conor+dt, dmitry.torokhov,
hvilleneuve, mkorpershoek, matthias.bgg,
angelogioacchino.delregno, lee, alexander.sverdlin, marek.vasut,
akurz
Cc: devicetree, linux-kernel, linux-input, linux-arm-kernel,
linux-mediatek, hugo
In-Reply-To: <20260312180304.3865850-1-hugo@hugovil.com>
From: Hugo Villeneuve <hvilleneuve@dimonoff.com>
A few bindings are already defining a debounce-delay-ms property, so
add it to the input binding to reduce redundant redefines.
Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
---
.../devicetree/bindings/auxdisplay/holtek,ht16k33.yaml | 5 ++---
.../devicetree/bindings/input/cirrus,ep9307-keypad.yaml | 7 +++----
.../devicetree/bindings/input/gpio-matrix-keypad.yaml | 5 ++---
Documentation/devicetree/bindings/input/input.yaml | 8 ++++++++
.../devicetree/bindings/input/mediatek,mt6779-keypad.yaml | 1 +
Documentation/devicetree/bindings/mfd/fsl,mc13xxx.yaml | 2 --
6 files changed, 16 insertions(+), 12 deletions(-)
diff --git a/Documentation/devicetree/bindings/auxdisplay/holtek,ht16k33.yaml b/Documentation/devicetree/bindings/auxdisplay/holtek,ht16k33.yaml
index b90eec2077b4b..c46a2471f8b10 100644
--- a/Documentation/devicetree/bindings/auxdisplay/holtek,ht16k33.yaml
+++ b/Documentation/devicetree/bindings/auxdisplay/holtek,ht16k33.yaml
@@ -10,6 +10,7 @@ maintainers:
- Robin van der Gracht <robin@protonic.nl>
allOf:
+ - $ref: /schemas/input/input.yaml#
- $ref: /schemas/input/matrix-keymap.yaml#
properties:
@@ -33,9 +34,7 @@ properties:
interrupts:
maxItems: 1
- debounce-delay-ms:
- maxItems: 1
- description: Debouncing interval time in milliseconds
+ debounce-delay-ms: true
linux,keymap: true
diff --git a/Documentation/devicetree/bindings/input/cirrus,ep9307-keypad.yaml b/Documentation/devicetree/bindings/input/cirrus,ep9307-keypad.yaml
index a0d2460c55ab6..25b8b29c87d70 100644
--- a/Documentation/devicetree/bindings/input/cirrus,ep9307-keypad.yaml
+++ b/Documentation/devicetree/bindings/input/cirrus,ep9307-keypad.yaml
@@ -10,6 +10,7 @@ maintainers:
- Alexander Sverdlin <alexander.sverdlin@gmail.com>
allOf:
+ - $ref: input.yaml#
- $ref: /schemas/input/matrix-keymap.yaml#
description:
@@ -37,10 +38,8 @@ properties:
clocks:
maxItems: 1
- debounce-delay-ms:
- description: |
- Time in microseconds that key must be pressed or
- released for state change interrupt to trigger.
+ # Time for state change interrupt to trigger
+ debounce-delay-ms: true
cirrus,prescale:
description: row/column counter pre-scaler load value
diff --git a/Documentation/devicetree/bindings/input/gpio-matrix-keypad.yaml b/Documentation/devicetree/bindings/input/gpio-matrix-keypad.yaml
index ebfff9e42a365..69df24a5ae701 100644
--- a/Documentation/devicetree/bindings/input/gpio-matrix-keypad.yaml
+++ b/Documentation/devicetree/bindings/input/gpio-matrix-keypad.yaml
@@ -18,6 +18,7 @@ description:
report the event using GPIO interrupts to the cpu.
allOf:
+ - $ref: input.yaml#
- $ref: /schemas/input/matrix-keymap.yaml#
properties:
@@ -46,9 +47,7 @@ properties:
Force GPIO polarity to active low.
In the absence of this property GPIOs are treated as active high.
- debounce-delay-ms:
- description: Debounce interval in milliseconds.
- default: 0
+ debounce-delay-ms: true
col-scan-delay-us:
description:
diff --git a/Documentation/devicetree/bindings/input/input.yaml b/Documentation/devicetree/bindings/input/input.yaml
index 94f7942189e8c..502e0b7eb500a 100644
--- a/Documentation/devicetree/bindings/input/input.yaml
+++ b/Documentation/devicetree/bindings/input/input.yaml
@@ -14,6 +14,14 @@ properties:
description: Enable autorepeat when key is pressed and held down.
type: boolean
+ debounce-delay-ms:
+ description:
+ Debounce delay in milliseconds. This is the time during which the key
+ press or release signal must remain stable before it is considered valid.
+ minimum: 0
+ maximum: 999
+ default: 0
+
linux,keycodes:
description:
Specifies an array of numeric keycode values to be used for reporting
diff --git a/Documentation/devicetree/bindings/input/mediatek,mt6779-keypad.yaml b/Documentation/devicetree/bindings/input/mediatek,mt6779-keypad.yaml
index e365413732e7b..914dd3283df33 100644
--- a/Documentation/devicetree/bindings/input/mediatek,mt6779-keypad.yaml
+++ b/Documentation/devicetree/bindings/input/mediatek,mt6779-keypad.yaml
@@ -10,6 +10,7 @@ maintainers:
- Mattijs Korpershoek <mkorpershoek@kernel.org>
allOf:
+ - $ref: input.yaml#
- $ref: /schemas/input/matrix-keymap.yaml#
description: |
diff --git a/Documentation/devicetree/bindings/mfd/fsl,mc13xxx.yaml b/Documentation/devicetree/bindings/mfd/fsl,mc13xxx.yaml
index d2886f2686a8d..542ba51144243 100644
--- a/Documentation/devicetree/bindings/mfd/fsl,mc13xxx.yaml
+++ b/Documentation/devicetree/bindings/mfd/fsl,mc13xxx.yaml
@@ -76,8 +76,6 @@ properties:
debounce-delay-ms:
enum: [0, 30, 150, 750]
default: 30
- description:
- Sets the debouncing delay in milliseconds.
active-low:
description: Set active when pin is pulled low.
--
2.47.3
^ permalink raw reply related
* [PATCH v5 3/4] dt-bindings: input: add GPIO charlieplex keypad
From: Hugo Villeneuve @ 2026-03-12 18:00 UTC (permalink / raw)
To: robin, andy, geert, robh, krzk+dt, conor+dt, dmitry.torokhov,
hvilleneuve, mkorpershoek, matthias.bgg,
angelogioacchino.delregno, lee, alexander.sverdlin, marek.vasut,
akurz
Cc: devicetree, linux-kernel, linux-input, linux-arm-kernel,
linux-mediatek, hugo
In-Reply-To: <20260312180304.3865850-1-hugo@hugovil.com>
From: Hugo Villeneuve <hvilleneuve@dimonoff.com>
Add DT bindings for GPIO charlieplex keypad.
Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
---
.../input/gpio-charlieplex-keypad.yaml | 108 ++++++++++++++++++
1 file changed, 108 insertions(+)
create mode 100644 Documentation/devicetree/bindings/input/gpio-charlieplex-keypad.yaml
diff --git a/Documentation/devicetree/bindings/input/gpio-charlieplex-keypad.yaml b/Documentation/devicetree/bindings/input/gpio-charlieplex-keypad.yaml
new file mode 100644
index 0000000000000..c085de6dab854
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/gpio-charlieplex-keypad.yaml
@@ -0,0 +1,108 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+
+$id: http://devicetree.org/schemas/input/gpio-charlieplex-keypad.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: GPIO charlieplex keypad
+
+maintainers:
+ - Hugo Villeneuve <hvilleneuve@dimonoff.com>
+
+description: |
+ The charlieplex keypad supports N^2)-N different key combinations (where N is
+ the number of I/O lines). Key presses and releases are detected by configuring
+ only one line as output at a time, and reading other line states. This process
+ is repeated for each line. Diodes are required to ensure current flows in only
+ one direction between any pair of pins, as well as pull-up or pull-down
+ resistors on all I/O lines.
+ This mechanism doesn't allow to detect simultaneous key presses.
+
+ Wiring example for 3 lines keyboard with 6 switches and 3 diodes (pull-up/down
+ resistors not shown but needed on L0, L1 and L2):
+
+ L0 --+---------------------+----------------------+
+ | | |
+ L1 -------+-----------+---------------------+ |
+ | | | | | |
+ L2 -------------+----------------+-----+ | |
+ | | | | | | | | |
+ | | | | | | | | |
+ | S1 \ S2 \ | S3 \ S4 \ | S5 \ S6 \
+ | | | | | | | | |
+ | +--+--+ | +--+--+ | +--+--+
+ | | | | | |
+ | D1 v | D2 v | D3 v
+ | - (k) | - (k) | - (k)
+ | | | | | |
+ +-------+ +-------+ +-------+
+
+ L: GPIO line
+ S: switch
+ D: diode (k indicates cathode)
+
+allOf:
+ - $ref: input.yaml#
+ - $ref: /schemas/input/matrix-keymap.yaml#
+
+properties:
+ compatible:
+ const: gpio-charlieplex-keypad
+
+ autorepeat: true
+
+ debounce-delay-ms:
+ default: 5
+
+ line-gpios:
+ description:
+ List of GPIOs used as lines. The gpio specifier for this property
+ depends on the gpio controller to which these lines are connected.
+
+ linux,keymap: true
+
+ poll-interval: true
+
+ settling-time-us: true
+
+ wakeup-source: true
+
+required:
+ - compatible
+ - line-gpios
+ - linux,keymap
+ - poll-interval
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/gpio/gpio.h>
+ #include <dt-bindings/input/input.h>
+
+ keyboard {
+ compatible = "gpio-charlieplex-keypad";
+ debounce-delay-ms = <20>;
+ poll-interval = <5>;
+ settling-time-us = <2>;
+
+ line-gpios = <&gpio2 25 (GPIO_ACTIVE_HIGH | GPIO_PULL_DOWN)
+ &gpio2 26 (GPIO_ACTIVE_HIGH | GPIO_PULL_DOWN)
+ &gpio2 27 (GPIO_ACTIVE_HIGH | GPIO_PULL_DOWN)>;
+
+ /* MATRIX_KEY(output, input, key-code) */
+ linux,keymap = <
+ /*
+ * According to wiring diagram above, if L1 is configured as
+ * output and HIGH, and we detect a HIGH level on input L0,
+ * then it means S1 is pressed: MATRIX_KEY(L1, L0, KEY...)
+ */
+ MATRIX_KEY(1, 0, KEY_F1) /* S1 */
+ MATRIX_KEY(2, 0, KEY_F2) /* S2 */
+ MATRIX_KEY(0, 1, KEY_F3) /* S3 */
+ MATRIX_KEY(2, 1, KEY_F4) /* S4 */
+ MATRIX_KEY(1, 2, KEY_F5) /* S5 */
+ MATRIX_KEY(0, 2, KEY_F6) /* S6 */
+ >;
+ };
--
2.47.3
^ permalink raw reply related
* [PATCH v5 4/4] Input: charlieplex_keypad: add GPIO charlieplex keypad
From: Hugo Villeneuve @ 2026-03-12 18:00 UTC (permalink / raw)
To: robin, andy, geert, robh, krzk+dt, conor+dt, dmitry.torokhov,
hvilleneuve, mkorpershoek, matthias.bgg,
angelogioacchino.delregno, lee, alexander.sverdlin, marek.vasut,
akurz
Cc: devicetree, linux-kernel, linux-input, linux-arm-kernel,
linux-mediatek, hugo
In-Reply-To: <20260312180304.3865850-1-hugo@hugovil.com>
From: Hugo Villeneuve <hvilleneuve@dimonoff.com>
Add support for GPIO-based charlieplex keypad, allowing to control
N^2-N keys using N GPIO lines.
Reuse matrix keypad keymap to simplify, even if there is no concept
of rows and columns in this type of keyboard.
Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
---
MAINTAINERS | 7 +
drivers/input/keyboard/Kconfig | 14 ++
drivers/input/keyboard/Makefile | 1 +
drivers/input/keyboard/charlieplex_keypad.c | 223 ++++++++++++++++++++
4 files changed, 245 insertions(+)
create mode 100644 drivers/input/keyboard/charlieplex_keypad.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 9ed6d11a77466..0b2d71f32b400 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5765,6 +5765,13 @@ S: Maintained
F: Documentation/hwmon/powerz.rst
F: drivers/hwmon/powerz.c
+CHARLIEPLEX KEYPAD DRIVER
+M: Hugo Villeneuve <hvilleneuve@dimonoff.com>
+S: Supported
+W: http://www.mosaic-industries.com/embedded-systems/microcontroller-projects/electronic-circuits/matrix-keypad-scan-decode
+F: Documentation/devicetree/bindings/input/gpio-charlieplex-keypad.yaml
+F: drivers/input/keyboard/charlieplex_keypad.c
+
CHECKPATCH
M: Andy Whitcroft <apw@canonical.com>
M: Joe Perches <joe@perches.com>
diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
index 2ff4fef322c24..ae54b4b7e2d8a 100644
--- a/drivers/input/keyboard/Kconfig
+++ b/drivers/input/keyboard/Kconfig
@@ -289,6 +289,20 @@ config KEYBOARD_MATRIX
To compile this driver as a module, choose M here: the
module will be called matrix_keypad.
+config KEYBOARD_CHARLIEPLEX
+ tristate "GPIO driven chearlieplex keypad support"
+ depends on GPIOLIB || COMPILE_TEST
+ select INPUT_MATRIXKMAP
+ help
+ Enable support for GPIO driven charlieplex keypad. A charlieplex
+ keypad allows to use fewer GPIO lines to interface to key switches.
+ For example, an N lines charlieplex keypad can be used to interface
+ to N^2-N different key switches. However, this type of keypad
+ cannot detect more than one key press at a time.
+
+ To compile this driver as a module, choose M here: the
+ module will be called charlieplex_keypad.
+
config KEYBOARD_HIL_OLD
tristate "HP HIL keyboard support (simple driver)"
depends on GSC || HP300
diff --git a/drivers/input/keyboard/Makefile b/drivers/input/keyboard/Makefile
index 2d906e14f3e27..60bb7baf802f7 100644
--- a/drivers/input/keyboard/Makefile
+++ b/drivers/input/keyboard/Makefile
@@ -15,6 +15,7 @@ obj-$(CONFIG_KEYBOARD_ATARI) += atakbd.o
obj-$(CONFIG_KEYBOARD_ATKBD) += atkbd.o
obj-$(CONFIG_KEYBOARD_BCM) += bcm-keypad.o
obj-$(CONFIG_KEYBOARD_CAP11XX) += cap11xx.o
+obj-$(CONFIG_KEYBOARD_CHARLIEPLEX) += charlieplex_keypad.o
obj-$(CONFIG_KEYBOARD_CLPS711X) += clps711x-keypad.o
obj-$(CONFIG_KEYBOARD_CROS_EC) += cros_ec_keyb.o
obj-$(CONFIG_KEYBOARD_CYPRESS_SF) += cypress-sf.o
diff --git a/drivers/input/keyboard/charlieplex_keypad.c b/drivers/input/keyboard/charlieplex_keypad.c
new file mode 100644
index 0000000000000..7064c40e1d839
--- /dev/null
+++ b/drivers/input/keyboard/charlieplex_keypad.c
@@ -0,0 +1,223 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * GPIO driven charlieplex keypad driver
+ *
+ * Copyright (c) 2026 Hugo Villeneuve <hvilleneuve@dimonoff.com>
+ *
+ * Based on matrix_keyboard.c
+ */
+
+#include <linux/bitops.h>
+#include <linux/delay.h>
+#include <linux/dev_printk.h>
+#include <linux/device/devres.h>
+#include <linux/err.h>
+#include <linux/gpio/consumer.h>
+#include <linux/input.h>
+#include <linux/input/matrix_keypad.h>
+#include <linux/math.h>
+#include <linux/module.h>
+#include <linux/mod_devicetable.h>
+#include <linux/platform_device.h>
+#include <linux/property.h>
+#include <linux/string_helpers.h>
+#include <linux/types.h>
+
+struct charlieplex_keypad {
+ struct input_dev *input_dev;
+ struct gpio_descs *line_gpios;
+ unsigned int nlines;
+ unsigned int settling_time_us;
+ unsigned int debounce_threshold;
+ unsigned int debounce_count;
+ int debounce_code;
+ int current_code;
+};
+
+static void charlieplex_keypad_report_key(struct input_dev *input)
+{
+ struct charlieplex_keypad *keypad = input_get_drvdata(input);
+ const unsigned short *keycodes = input->keycode;
+
+ if (keypad->current_code > 0) {
+ input_event(input, EV_MSC, MSC_SCAN, keypad->current_code);
+ input_report_key(input, keycodes[keypad->current_code], 0);
+ }
+
+ if (keypad->debounce_code) {
+ input_event(input, EV_MSC, MSC_SCAN, keypad->debounce_code);
+ input_report_key(input, keycodes[keypad->debounce_code], 1);
+ }
+
+ input_sync(input);
+ keypad->current_code = keypad->debounce_code;
+}
+
+static void charlieplex_keypad_check_switch_change(struct input_dev *input,
+ int code)
+{
+ struct charlieplex_keypad *keypad = input_get_drvdata(input);
+
+ if (code != keypad->debounce_code) {
+ keypad->debounce_count = 0;
+ keypad->debounce_code = code;
+ } else if (keypad->debounce_count < keypad->debounce_threshold) {
+ keypad->debounce_count++;
+
+ if (keypad->debounce_count >= keypad->debounce_threshold &&
+ keypad->debounce_code != keypad->current_code)
+ charlieplex_keypad_report_key(input);
+ }
+}
+
+static void charlieplex_keypad_poll(struct input_dev *input)
+{
+ struct charlieplex_keypad *keypad = input_get_drvdata(input);
+ int code;
+
+ code = 0;
+ for (unsigned int oline = 0; oline < keypad->nlines; oline++) {
+ DECLARE_BITMAP(values, MATRIX_MAX_ROWS);
+ int err;
+
+ /* Activate only one line as output at a time. */
+ gpiod_direction_output(keypad->line_gpios->desc[oline], 1);
+
+ if (keypad->settling_time_us)
+ fsleep(keypad->settling_time_us);
+
+ /* Read input on all other lines. */
+ err = gpiod_get_array_value_cansleep(keypad->line_gpios->ndescs,
+ keypad->line_gpios->desc,
+ keypad->line_gpios->info, values);
+ if (err)
+ return;
+
+ for (unsigned int iline = 0; iline < keypad->nlines; iline++) {
+ if (iline == oline)
+ continue; /* Do not read active output line. */
+
+ /* Check if GPIO is asserted. */
+ if (test_bit(iline, values)) {
+ code = MATRIX_SCAN_CODE(oline, iline,
+ get_count_order(keypad->nlines));
+ /*
+ * Exit loop immediately since we cannot detect
+ * more than one key press at a time.
+ */
+ break;
+ }
+ }
+
+ gpiod_direction_input(keypad->line_gpios->desc[oline]);
+
+ if (code)
+ break;
+ }
+
+ charlieplex_keypad_check_switch_change(input, code);
+}
+
+static int charlieplex_keypad_init_gpio(struct platform_device *pdev,
+ struct charlieplex_keypad *keypad)
+{
+ char **pin_names;
+ char label[32];
+
+ snprintf(label, sizeof(label), "%s-pin", pdev->name);
+
+ keypad->line_gpios = devm_gpiod_get_array(&pdev->dev, "line", GPIOD_IN);
+ if (IS_ERR(keypad->line_gpios))
+ return PTR_ERR(keypad->line_gpios);
+
+ keypad->nlines = keypad->line_gpios->ndescs;
+
+ if (keypad->nlines > MATRIX_MAX_ROWS)
+ return -EINVAL;
+
+ pin_names = devm_kasprintf_strarray(&pdev->dev, label, keypad->nlines);
+ if (IS_ERR(pin_names))
+ return PTR_ERR(pin_names);
+
+ for (unsigned int i = 0; i < keypad->line_gpios->ndescs; i++)
+ gpiod_set_consumer_name(keypad->line_gpios->desc[i], pin_names[i]);
+
+ return 0;
+}
+
+static int charlieplex_keypad_probe(struct platform_device *pdev)
+{
+ struct charlieplex_keypad *keypad;
+ unsigned int debounce_interval_ms;
+ unsigned int poll_interval_ms;
+ struct input_dev *input_dev;
+ int err;
+
+ keypad = devm_kzalloc(&pdev->dev, sizeof(*keypad), GFP_KERNEL);
+ if (!keypad)
+ return -ENOMEM;
+
+ input_dev = devm_input_allocate_device(&pdev->dev);
+ if (!input_dev)
+ return -ENOMEM;
+
+ keypad->input_dev = input_dev;
+
+ device_property_read_u32(&pdev->dev, "poll-interval", &poll_interval_ms);
+ device_property_read_u32(&pdev->dev, "debounce-delay-ms", &debounce_interval_ms);
+ device_property_read_u32(&pdev->dev, "settling-time-us", &keypad->settling_time_us);
+
+ keypad->current_code = -1;
+ keypad->debounce_code = -1;
+ keypad->debounce_threshold = DIV_ROUND_UP(debounce_interval_ms, poll_interval_ms);
+
+ err = charlieplex_keypad_init_gpio(pdev, keypad);
+ if (err)
+ return err;
+
+ input_dev->name = pdev->name;
+ input_dev->id.bustype = BUS_HOST;
+
+ err = matrix_keypad_build_keymap(NULL, NULL, keypad->nlines,
+ keypad->nlines, NULL, input_dev);
+ if (err)
+ dev_err_probe(&pdev->dev, -ENOMEM, "failed to build keymap\n");
+
+ if (device_property_read_bool(&pdev->dev, "autorepeat"))
+ __set_bit(EV_REP, input_dev->evbit);
+
+ input_set_capability(input_dev, EV_MSC, MSC_SCAN);
+
+ err = input_setup_polling(input_dev, charlieplex_keypad_poll);
+ if (err)
+ dev_err_probe(&pdev->dev, err, "unable to set up polling\n");
+
+ input_set_poll_interval(input_dev, poll_interval_ms);
+
+ input_set_drvdata(input_dev, keypad);
+
+ err = input_register_device(keypad->input_dev);
+ if (err)
+ return err;
+
+ return 0;
+}
+
+static const struct of_device_id charlieplex_keypad_dt_match[] = {
+ { .compatible = "gpio-charlieplex-keypad" },
+ { }
+};
+MODULE_DEVICE_TABLE(of, charlieplex_keypad_dt_match);
+
+static struct platform_driver charlieplex_keypad_driver = {
+ .probe = charlieplex_keypad_probe,
+ .driver = {
+ .name = "charlieplex-keypad",
+ .of_match_table = charlieplex_keypad_dt_match,
+ },
+};
+module_platform_driver(charlieplex_keypad_driver);
+
+MODULE_AUTHOR("Hugo Villeneuve <hvilleneuve@dimonoff.com>");
+MODULE_DESCRIPTION("GPIO driven charlieplex keypad driver");
+MODULE_LICENSE("GPL");
--
2.47.3
^ permalink raw reply related
* Re: [PATCH 48/61] mtd: Prefer IS_ERR_OR_NULL over manual NULL check
From: Richard Weinberger @ 2026-03-12 19:33 UTC (permalink / raw)
To: Philipp Hahn
Cc: amd-gfx, apparmor, bpf, ceph-devel, cocci, dm-devel,
DRI mailing list, gfs2, intel-gfx, intel-wired-lan, iommu, kvm,
linux-arm-kernel, linux-block, linux-bluetooth, linux-btrfs,
linux-cifs, linux-clk, linux-erofs, linux-ext4, linux-fsdevel,
linux-gpio, linux-hyperv, linux-input, linux-kernel, linux-leds,
linux-media, linux-mips, linux-mm, linux-modules, linux-mtd,
linux-nfs, linux-omap, linux-phy, linux-pm, linux-rockchip,
linux-s390, linux-scsi, linux-sctp, LSM, linux-sh, linux-sound,
linux-stm32, linux-trace-kernel, linux-usb, linux-wireless,
netdev, ntfs3, samba-technical, sched-ext, target-devel,
tipc-discussion, v9fs, Miquel Raynal, Vignesh Raghavendra
In-Reply-To: <20260310-b4-is_err_or_null-v1-48-bd63b656022d@avm.de>
----- Ursprüngliche Mail -----
> Von: "Philipp Hahn" <phahn-oss@avm.de>
> - if (gpiomtd->nwp && !IS_ERR(gpiomtd->nwp))
> + if (!IS_ERR_OR_NULL(gpiomtd->nwp))
No, please don't.
This makes reading the code not easier.
Thanks,
//richard
^ permalink raw reply
* [PATCH 6.12 139/265] HID: Add HID_CLAIMED_INPUT guards in raw_event callbacks missing them
From: Greg Kroah-Hartman @ 2026-03-12 20:08 UTC (permalink / raw)
To: stable
Cc: Greg Kroah-Hartman, patches, Jiri Kosina, Benjamin Tissoires,
Bastien Nocera, linux-input, stable
In-Reply-To: <20260312201018.128816016@linuxfoundation.org>
6.12-stable review patch. If anyone has any objections, please let me know.
------------------
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
commit ecfa6f34492c493a9a1dc2900f3edeb01c79946b upstream.
In commit 2ff5baa9b527 ("HID: appleir: Fix potential NULL dereference at
raw event handle"), we handle the fact that raw event callbacks
can happen even for a HID device that has not been "claimed" causing a
crash if a broken device were attempted to be connected to the system.
Fix up the remaining in-tree HID drivers that forgot to add this same
check to resolve the same issue.
Cc: Jiri Kosina <jikos@kernel.org>
Cc: Benjamin Tissoires <bentiss@kernel.org>
Cc: Bastien Nocera <hadess@hadess.net>
Cc: linux-input@vger.kernel.org
Cc: stable <stable@kernel.org>
Assisted-by: gkh_clanker_2000
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
drivers/hid/hid-cmedia.c | 2 +-
drivers/hid/hid-creative-sb0540.c | 2 +-
drivers/hid/hid-zydacron.c | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
--- a/drivers/hid/hid-cmedia.c
+++ b/drivers/hid/hid-cmedia.c
@@ -99,7 +99,7 @@ static int cmhid_raw_event(struct hid_de
{
struct cmhid *cm = hid_get_drvdata(hid);
- if (len != CM6533_JD_RAWEV_LEN)
+ if (len != CM6533_JD_RAWEV_LEN || !(hid->claimed & HID_CLAIMED_INPUT))
goto out;
if (memcmp(data+CM6533_JD_SFX_OFFSET, ji_sfx, sizeof(ji_sfx)))
goto out;
--- a/drivers/hid/hid-creative-sb0540.c
+++ b/drivers/hid/hid-creative-sb0540.c
@@ -153,7 +153,7 @@ static int creative_sb0540_raw_event(str
u64 code, main_code;
int key;
- if (len != 6)
+ if (len != 6 || !(hid->claimed & HID_CLAIMED_INPUT))
return 0;
/* From daemons/hw_hiddev.c sb0540_rec() in lirc */
--- a/drivers/hid/hid-zydacron.c
+++ b/drivers/hid/hid-zydacron.c
@@ -114,7 +114,7 @@ static int zc_raw_event(struct hid_devic
unsigned key;
unsigned short index;
- if (report->id == data[0]) {
+ if (report->id == data[0] && (hdev->claimed & HID_CLAIMED_INPUT)) {
/* break keys */
for (index = 0; index < 4; index++) {
^ permalink raw reply
* [PATCH] dt-bindings: touchscreen: trivial-touch: Move allOf: after required:
From: Marek Vasut @ 2026-03-12 22:49 UTC (permalink / raw)
To: devicetree
Cc: Marek Vasut, Conor Dooley, Dmitry Torokhov, Frank Li, Job Noorman,
Krzysztof Kozlowski, Rob Herring, linux-input, linux-renesas-soc
Majority of schemas place allOf: after required: . Documentation
Documentation/devicetree/bindings/writing-schema.rst also hints at
this ordering. Trivially update this schema. No functional change.
Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
---
NOTE: This comes from https://lore.kernel.org/all/20260117-grinning-heavy-crab-11f245@quoll/
where krzk comments "allOf: should be placed after required: block."
---
Cc: Conor Dooley <conor+dt@kernel.org>
Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: Frank Li <Frank.Li@nxp.com>
Cc: Job Noorman <job@noorman.info>
Cc: Krzysztof Kozlowski <krzk+dt@kernel.org>
Cc: Rob Herring <robh@kernel.org>
Cc: devicetree@vger.kernel.org
Cc: linux-input@vger.kernel.org
Cc: linux-renesas-soc@vger.kernel.org
---
.../bindings/input/touchscreen/trivial-touch.yaml | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/Documentation/devicetree/bindings/input/touchscreen/trivial-touch.yaml b/Documentation/devicetree/bindings/input/touchscreen/trivial-touch.yaml
index 6441d21223caf..6316a8d32f39b 100644
--- a/Documentation/devicetree/bindings/input/touchscreen/trivial-touch.yaml
+++ b/Documentation/devicetree/bindings/input/touchscreen/trivial-touch.yaml
@@ -53,14 +53,14 @@ properties:
wakeup-source: true
-allOf:
- - $ref: touchscreen.yaml
-
required:
- compatible
- reg
- interrupts
+allOf:
+ - $ref: touchscreen.yaml
+
unevaluatedProperties: false
examples:
--
2.51.0
^ permalink raw reply related
* Re: [PATCH v6 16/19] HID: Add documentation for Lenovo Legion Go drivers
From: Derek John Clark @ 2026-03-12 22:57 UTC (permalink / raw)
To: Akira Yokosawa
Cc: bentiss, hughsient, jikos, linux-doc, linux-input, linux-kernel,
mario.limonciello, mpearson-lenovo, pgriffais, shaohz1, zhangzx36
In-Reply-To: <7dc9bd14-b21f-461a-9794-070f43db4826@gmail.com>
On Wed, Mar 11, 2026 at 7:44 PM Akira Yokosawa <akiyks@gmail.com> wrote:
>
> Hi,
>
> On Tue, 10 Mar 2026 07:29:34 +0000, Derek J. Clark wrote:
> > Adds ABI documentation for the hid-lenovo-go-s and hid-lenovo-go
> > drivers.
> >
> > Reviewed-by: Mark Pearson <mpearson-lenovo@squebb.ca>
> > Signed-off-by: Derek J. Clark <derekjohn.clark@gmail.com>
> > ---
> > v3:
> > - Remove excess + from every line of patch.
> > ---
> > .../ABI/testing/sysfs-driver-hid-lenovo-go | 724 ++++++++++++++++++
> > .../ABI/testing/sysfs-driver-hid-lenovo-go-s | 304 ++++++++
> > MAINTAINERS | 2 +
> > 3 files changed, 1030 insertions(+)
> > create mode 100644 Documentation/ABI/testing/sysfs-driver-hid-lenovo-go
> > create mode 100644 Documentation/ABI/testing/sysfs-driver-hid-lenovo-go-s
>
> This (commit 168c91839139 in next-20260311) causes a lot of new warnings
> in "make htmldocs" such as:
>
> WARNING: /sys/bus/usb/devices/<busnum>-<devnum>:<config num>.<interface num>/<hid-bus>:<vendor-id>:<product-id>.<num>/os_mode is defined 2 times: /<...>/Documentation/ABI/testing/sysfs-driver-hid-lenovo-go:364; /<...>/Documentation/ABI/testing/sysfs-driver-hid-lenovo-go-s:234
> WARNING: /sys/bus/usb/devices/<busnum>-<devnum>:<config num>.<interface num>/<hid-bus>:<vendor-id>:<product-id>.<num>/os_mode_index is defined 2 times: /<...>/Documentation/ABI/testing/sysfs-driver-hid-lenovo-go:373; /<...>/Documentation/ABI/testing/sysfs-driver-hid-lenovo-go-s:243
> WARNING: /sys/bus/usb/devices/<busnum>-<devnum>:<config num>.<interface num>/<hid-bus>:<vendor-id>:<product-id>.<num>/touchpad/enabled is defined 2 times: /<...>/Documentation/ABI/testing/sysfs-driver-hid-lenovo-go:636; /<...>/Documentation/ABI/testing/sysfs-driver-hid-lenovo-go-s:252
> [snip]
>
> Please fix.
>
> Thanks, Akira
Akira,
What would an appropriate solution look like? There are two separate
drivers in this series that are modeled to provide a sysfs that is as
close to each other as possible. Because of that, they do end up
having identical attributes for many of the implemented features. In
most cases they are prefixed in the sysfs by a different component
name, but that isn't the case for os_mode or touchpad/enabled. I could
combine documentation for both drivers and de-duplicate, but I'm not
sure what that would be called, sysfs-driver-hid-lenovo-go? Add them
all to hid-lenovo docs?
If I do that there are some additional de-duplication that could
reduce the total number of lines. I.E
What: /sys/class/leds/[go|go_s]:rgb:joystick_rings/effect
Would that syntax be alright? Some of them get a little excessive, for
example the auto sleep time is defined three times and would be
combined to something like this:
What: /sys/bus/usb/devices/<busnum>-<devnum>:<config
num>.<interface
num>/<hid-bus>:<vendor-id>:<product-id>.<num>/[left_handle|right_handle|gamepad]/auto_sleep_time
Otherwise, is there some way to flag the same attribute defined in two
separate documents provided by two drivers as not being duplicates?
Thanks,
Derek
^ permalink raw reply
* Re: [PATCH 09/12] dt-bindings: input: Document hid-over-spi DT schema
From: Jingyuan Liang @ 2026-03-13 1:00 UTC (permalink / raw)
To: Val Packett
Cc: Jiri Kosina, Benjamin Tissoires, Jonathan Corbet, Mark Brown,
Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
linux-input, linux-doc, linux-kernel, linux-spi,
linux-trace-kernel, devicetree, hbarnor, Dmitry Antipov,
Jarrett Schultz
In-Reply-To: <1cc6de61-8b56-492e-ab78-e3aa448f58ad@packett.cool>
On Fri, Mar 6, 2026 at 11:25 PM Val Packett <val@packett.cool> wrote:
>
>
> On 3/3/26 3:13 AM, Jingyuan Liang wrote:
> > Documentation describes the required and optional properties for
> > implementing Device Tree for a Microsoft G6 Touch Digitizer that
> > supports HID over SPI Protocol 1.0 specification.
> > […]
> > +properties:
> > + compatible:
> > + oneOf:
> > + - items:
> > + - enum:
> > + - microsoft,g6-touch-digitizer
> > + - const: hid-over-spi
> > + - description: Just "hid-over-spi" alone is allowed, but not recommended.
> > […]
> > +required:
> > + - compatible
> > + - interrupts
> > + - reset-gpios
>
> Why is reset required? Is it so implausible on some device implementing
> the spec there wouldn't be a reset gpio?
>
> > + - vdd-supply
> Linux makes up a dummy regulator if DT doesn't provide one, so can
> regulators even be required?
> > […]
> > + compatible = "hid-over-spi";
> Not following your own recommendation from above :)
Thanks! I will fix this in v2.
> > + reg = <0x0>;
> > + interrupts-extended = <&gpio 42 IRQ_TYPE_EDGE_FALLING>;
> > + reset-gpios = <&gpio 27 GPIO_ACTIVE_LOW>;
> > + vdd-supply = <&pm8350c_l3>;
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&ts_d6_reset_assert &ts_d6_int_bias>;
>
> Heh, "reset_assert" is a name implying it would actually set the value
> from the pinctrl properties, which is what had to be done before
> reset-gpios were supported. But now reset-gpios are supported.
Taken from the original patch. Will fix this in v2.
>
>
> Thanks,
> ~val
>
>
> P.S. happy to see work on this happen again!
>
^ permalink raw reply
* Re: [PATCH 09/12] dt-bindings: input: Document hid-over-spi DT schema
From: Jingyuan Liang @ 2026-03-13 1:14 UTC (permalink / raw)
To: Rob Herring
Cc: Jiri Kosina, Benjamin Tissoires, Jonathan Corbet, Mark Brown,
Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Dmitry Torokhov, Krzysztof Kozlowski, Conor Dooley, linux-input,
linux-doc, LKML, linux-spi, linux-trace-kernel, devicetree,
Henry Barnor, Dmitry Antipov, Jarrett Schultz
In-Reply-To: <CAEe3GZE_79Lqtzv4ZRi6zWuV_DgkCREBDcZJjpR+X9OaGZCA2g@mail.gmail.com>
On Wed, Mar 11, 2026 at 5:58 PM Jingyuan Liang <jingyliang@chromium.org> wrote:
>
> (Resending to the list. Apologies, I accidentally dropped the CCs on
> my initial reply!)
>
> On Tue, Mar 3, 2026 at 5:53 AM Rob Herring <robh@kernel.org> wrote:
> >
> > On Tue, Mar 3, 2026 at 12:14 AM Jingyuan Liang <jingyliang@chromium.org> wrote:
> > >
> > > Documentation describes the required and optional properties for
> > > implementing Device Tree for a Microsoft G6 Touch Digitizer that
> > > supports HID over SPI Protocol 1.0 specification.
> > >
> > > The properties are common to HID over SPI.
> > >
> > > Signed-off-by: Dmitry Antipov <dmanti@microsoft.com>
> > > Signed-off-by: Jarrett Schultz <jaschultz@microsoft.com>
> > > Signed-off-by: Jingyuan Liang <jingyliang@chromium.org>
> > > ---
> > > .../devicetree/bindings/input/hid-over-spi.yaml | 153 +++++++++++++++++++++
> > > 1 file changed, 153 insertions(+)
> > >
> > > diff --git a/Documentation/devicetree/bindings/input/hid-over-spi.yaml b/Documentation/devicetree/bindings/input/hid-over-spi.yaml
> > > new file mode 100644
> > > index 000000000000..b623629ed9d3
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/input/hid-over-spi.yaml
> > > @@ -0,0 +1,153 @@
> > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/input/hid-over-spi.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: HID over SPI Devices
> > > +
> > > +maintainers:
> > > + - Benjamin Tissoires <benjamin.tissoires@redhat.com>
> > > + - Jiri Kosina <jkosina@suse.cz>
> > > +
> > > +description: |+
> > > + HID over SPI provides support for various Human Interface Devices over the
> > > + SPI bus. These devices can be for example touchpads, keyboards, touch screens
> > > + or sensors.
> > > +
> > > + The specification has been written by Microsoft and is currently available here:
> > > + https://www.microsoft.com/en-us/download/details.aspx?id=103325
> > > +
> > > + If this binding is used, the kernel module spi-hid will handle the communication
> > > + with the device and the generic hid core layer will handle the protocol.
> > > +
> > > +allOf:
> > > + - $ref: /schemas/input/touchscreen/touchscreen.yaml#
> > > +
> > > +properties:
> > > + compatible:
> > > + oneOf:
> > > + - items:
> > > + - enum:
> > > + - microsoft,g6-touch-digitizer
> > > + - const: hid-over-spi
> > > + - description: Just "hid-over-spi" alone is allowed, but not recommended.
> > > + const: hid-over-spi
> > > +
> > > + reg:
> > > + maxItems: 1
> > > +
> > > + interrupts:
> > > + maxItems: 1
> > > +
> > > + reset-gpios:
> > > + maxItems: 1
> > > + description:
> > > + GPIO specifier for the digitizer's reset pin (active low). The line must
> > > + be flagged with GPIO_ACTIVE_LOW.
> > > +
> > > + vdd-supply:
> > > + description:
> > > + Regulator for the VDD supply voltage.
> >
> > Is this part of the spec? This won't scale for multiple devices with
> > different power rails.
>
> This is not part of the spec but is needed for power management. Is it okay I
> mark it as optional? Thank you.
>
> >
> > > +
> > > + input-report-header-address:
> > > + $ref: /schemas/types.yaml#/definitions/uint32
> > > + minimum: 0
> > > + maximum: 0xffffff
> > > + description:
> > > + A value to be included in the Read Approval packet, listing an address of
> > > + the input report header to be put on the SPI bus. This address has 24
> > > + bits.
> > > +
> > > + input-report-body-address:
> > > + $ref: /schemas/types.yaml#/definitions/uint32
> > > + minimum: 0
> > > + maximum: 0xffffff
> > > + description:
> > > + A value to be included in the Read Approval packet, listing an address of
> > > + the input report body to be put on the SPI bus. This address has 24 bits.
> > > +
> > > + output-report-address:
> > > + $ref: /schemas/types.yaml#/definitions/uint32
> > > + minimum: 0
> > > + maximum: 0xffffff
> > > + description:
> > > + A value to be included in the Output Report sent by the host, listing an
> > > + address where the output report on the SPI bus is to be written to. This
> > > + address has 24 bits.
> > > +
> > > + post-power-on-delay-ms:
> > > + description:
> > > + Optional time in ms required by the device after enabling its regulators
> > > + or powering it on, before it is ready for communication.
> >
> > Drop. This should be implied by the compatible.
>
> Thank you, I will fix this in v2.
>
> >
> > > +
> > > + minimal-reset-delay-ms:
> > > + description:
> > > + Optional minimum amount of time in ms that device needs to be in reset
> > > + state for the reset to take effect.
> >
> > Drop. This should be implied by the compatible.
>
> I will fix this in v2.
>
> >
> > > +
> > > + read-opcode:
> > > + $ref: /schemas/types.yaml#/definitions/uint8
> > > + description:
> > > + Value to be used in Read Approval packets. 1 byte.
> > > +
> > > + write-opcode:
> > > + $ref: /schemas/types.yaml#/definitions/uint8
> > > + description:
> > > + Value to be used in Write Approval packets. 1 byte.
> >
> > Why are these and the address properties above not defined by the
> > spec? Do they vary for a specific device? If not, then they should be
> > implied by the compatible.
>
> These properties are not defined by the spec:
>
> "The Input Report Address (header or body) and READ opcode are retrieved
> from ACPI."
>
> Same for the output report address and write opcode. I will drop these in v2.
Hi Rob,
Please disregard my previous reply about dropping the read/write opcodes and
input/output addresses. The spec does not define these fields. Instead, it says
these values are retrieved from ACPI _DSM methods. If we remove them from
the binding, the driver is not very generic. Would it be okay to keep them?
>
> >
> > > +
> > > + hid-over-spi-flags:
> > > + $ref: /schemas/types.yaml#/definitions/uint16
> > > + description:
> > > + 16 bits.
> > > + Bits 0-12 - Reserved (must be 0)
> > > + Bit 13 - SPI Write Mode. Possible values -
> > > + * 0b0- Writes are carried out in Single-SPI mode
> > > + * 0b1- Writes are carried out in the Multi-SPI mode specified by bits
> > > + 14-15
> > > + Bits 14-15 - Multi-SPI Mode. Possible values -
> > > + * 0b00- Single SPI
> > > + * 0b01- Dual SPI
> > > + * 0b10- Quad SPI
> >
> > We already have SPI properties to define the bus width for read and write.
>
> Will fix this in v2.
>
> >
> > > +
> > > +required:
> > > + - compatible
> > > + - interrupts
> > > + - reset-gpios
> > > + - vdd-supply
> > > + - input-report-header-address
> > > + - input-report-body-address
> > > + - output-report-address
> > > + - read-opcode
> > > + - write-opcode
> > > + - hid-over-spi-flags
> > > +
> > > +additionalProperties: false
> > > +
> > > +examples:
> > > + - |
> > > + #include <dt-bindings/interrupt-controller/irq.h>
> > > + #include <dt-bindings/gpio/gpio.h>
> > > +
> > > + spi {
> > > + #address-cells = <1>;
> > > + #size-cells = <0>;
> > > +
> > > + hid@0 {
> > > + compatible = "hid-over-spi";
> > > + reg = <0x0>;
> > > + interrupts-extended = <&gpio 42 IRQ_TYPE_EDGE_FALLING>;
> > > + reset-gpios = <&gpio 27 GPIO_ACTIVE_LOW>;
> > > + vdd-supply = <&pm8350c_l3>;
> > > + pinctrl-names = "default";
> > > + pinctrl-0 = <&ts_d6_reset_assert &ts_d6_int_bias>;
> > > + input-report-header-address = <0x1000>;
> > > + input-report-body-address = <0x1004>;
> > > + output-report-address = <0x2000>;
> > > + read-opcode = <0x0b>;
> > > + write-opcode = <0x02>;
> > > + hid-over-spi-flags = <0x0000>;
> > > + post-power-on-delay-ms = <5>;
> > > + minimal-reset-delay-ms = <5>;
> > > + };
> > > + };
> > > \ No newline at end of file
> >
> > Fix this.
>
> Will fix this in v2.
>
> >
> > Rob
^ permalink raw reply
* Re: [PATCH 07/12] HID: spi_hid: add ACPI support for SPI over HID
From: Jingyuan Liang @ 2026-03-13 1:24 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Jiri Kosina, Benjamin Tissoires, Jonathan Corbet, Mark Brown,
Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, linux-input, linux-doc,
linux-kernel, linux-spi, linux-trace-kernel, devicetree, hbarnor,
Angela Czubak
In-Reply-To: <abD6RJZa5D7LN3x0@google.com>
On Tue, Mar 10, 2026 at 10:27 PM Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
>
> On Tue, Mar 03, 2026 at 06:12:59AM +0000, Jingyuan Liang wrote:
> > From: Angela Czubak <acz@semihalf.com>
> >
> > Detect SPI HID devices described in ACPI.
> >
> > Signed-off-by: Angela Czubak <acz@semihalf.com>
> > Signed-off-by: Jingyuan Liang <jingyliang@chromium.org>
> > ---
> > drivers/hid/spi-hid/Kconfig | 15 +++
> > drivers/hid/spi-hid/Makefile | 1 +
> > drivers/hid/spi-hid/spi-hid-acpi.c | 253 +++++++++++++++++++++++++++++++++++++
> > drivers/hid/spi-hid/spi-hid-core.c | 27 +---
> > drivers/hid/spi-hid/spi-hid.h | 44 +++++++
> > 5 files changed, 316 insertions(+), 24 deletions(-)
> >
> > diff --git a/drivers/hid/spi-hid/Kconfig b/drivers/hid/spi-hid/Kconfig
> > index 836fdefe8345..114b1e00da39 100644
> > --- a/drivers/hid/spi-hid/Kconfig
> > +++ b/drivers/hid/spi-hid/Kconfig
> > @@ -10,6 +10,21 @@ menuconfig SPI_HID
> >
> > if SPI_HID
> >
> > +config SPI_HID_ACPI
> > + tristate "HID over SPI transport layer ACPI driver"
> > + depends on ACPI
> > + select SPI_HID_CORE
> > + help
> > + Say Y here if you use a keyboard, a touchpad, a touchscreen, or any
> > + other HID based devices which are connected to your computer via SPI.
> > + This driver supports ACPI-based systems.
> > +
> > + If unsure, say N.
> > +
> > + This support is also available as a module. If so, the module
> > + will be called spi-hid-acpi. It will also build/depend on the
> > + module spi-hid.
> > +
> > config SPI_HID_CORE
> > tristate
> > endif
> > diff --git a/drivers/hid/spi-hid/Makefile b/drivers/hid/spi-hid/Makefile
> > index 92e24cddbfc2..753c7b7a7844 100644
> > --- a/drivers/hid/spi-hid/Makefile
> > +++ b/drivers/hid/spi-hid/Makefile
> > @@ -7,3 +7,4 @@
> >
> > obj-$(CONFIG_SPI_HID_CORE) += spi-hid.o
> > spi-hid-objs = spi-hid-core.o
> > +obj-$(CONFIG_SPI_HID_ACPI) += spi-hid-acpi.o
> > diff --git a/drivers/hid/spi-hid/spi-hid-acpi.c b/drivers/hid/spi-hid/spi-hid-acpi.c
> > new file mode 100644
> > index 000000000000..612e74fe72f9
> > --- /dev/null
> > +++ b/drivers/hid/spi-hid/spi-hid-acpi.c
> > @@ -0,0 +1,253 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * HID over SPI protocol, ACPI related code
> > + *
> > + * Copyright (c) 2021 Microsoft Corporation
> > + * Copyright (c) 2026 Google LLC
> > + *
> > + * This code was forked out of the HID over SPI core code, which is partially
> > + * based on "HID over I2C protocol implementation:
> > + *
> > + * Copyright (c) 2012 Benjamin Tissoires <benjamin.tissoires@gmail.com>
> > + * Copyright (c) 2012 Ecole Nationale de l'Aviation Civile, France
> > + * Copyright (c) 2012 Red Hat, Inc
> > + *
> > + * which in turn is partially based on "USB HID support for Linux":
> > + *
> > + * Copyright (c) 1999 Andreas Gal
> > + * Copyright (c) 2000-2005 Vojtech Pavlik <vojtech@suse.cz>
> > + * Copyright (c) 2005 Michael Haboustak <mike-@cinci.rr.com> for Concept2, Inc
> > + * Copyright (c) 2007-2008 Oliver Neukum
> > + * Copyright (c) 2006-2010 Jiri Kosina
> > + */
> > +
> > +#include <linux/acpi.h>
> > +#include <linux/delay.h>
> > +#include <linux/device.h>
> > +#include <linux/kernel.h>
> > +#include <linux/module.h>
> > +#include <linux/reset.h>
> > +#include <linux/uuid.h>
> > +
> > +#include "spi-hid.h"
> > +
> > +/* Config structure is filled with data from ACPI */
> > +struct spi_hid_acpi_config {
> > + struct spihid_ops ops;
> > +
> > + struct spi_hid_conf property_conf;
> > + u32 post_power_on_delay_ms;
> > + u32 minimal_reset_delay_ms;
> > + struct acpi_device *adev;
> > +};
> > +
> > +/* HID SPI Device: 6e2ac436-0fcf41af-a265-b32a220dcfab */
> > +static guid_t spi_hid_guid =
> > + GUID_INIT(0x6E2AC436, 0x0FCF, 0x41AF,
> > + 0xA2, 0x65, 0xB3, 0x2A, 0x22, 0x0D, 0xCF, 0xAB);
> > +
> > +static int spi_hid_acpi_populate_config(struct spi_hid_acpi_config *conf,
> > + struct acpi_device *adev)
> > +{
> > + acpi_handle handle = acpi_device_handle(adev);
> > + union acpi_object *obj;
> > +
> > + conf->adev = adev;
> > +
> > + /* Revision 3 for HID over SPI V1, see specification. */
> > + obj = acpi_evaluate_dsm_typed(handle, &spi_hid_guid, 3, 1, NULL,
> > + ACPI_TYPE_INTEGER);
> > + if (!obj) {
> > + acpi_handle_err(handle,
> > + "Error _DSM call to get HID input report header address failed");
> > + return -ENODEV;
> > + }
> > + conf->property_conf.input_report_header_address = obj->integer.value;
> > + ACPI_FREE(obj);
> > +
> > + obj = acpi_evaluate_dsm_typed(handle, &spi_hid_guid, 3, 2, NULL,
> > + ACPI_TYPE_INTEGER);
> > + if (!obj) {
> > + acpi_handle_err(handle,
> > + "Error _DSM call to get HID input report body address failed");
> > + return -ENODEV;
> > + }
> > + conf->property_conf.input_report_body_address = obj->integer.value;
> > + ACPI_FREE(obj);
> > +
> > + obj = acpi_evaluate_dsm_typed(handle, &spi_hid_guid, 3, 3, NULL,
> > + ACPI_TYPE_INTEGER);
> > + if (!obj) {
> > + acpi_handle_err(handle,
> > + "Error _DSM call to get HID output report header address failed");
> > + return -ENODEV;
> > + }
> > + conf->property_conf.output_report_address = obj->integer.value;
> > + ACPI_FREE(obj);
> > +
> > + obj = acpi_evaluate_dsm_typed(handle, &spi_hid_guid, 3, 4, NULL,
> > + ACPI_TYPE_BUFFER);
> > + if (!obj) {
> > + acpi_handle_err(handle,
> > + "Error _DSM call to get HID read opcode failed");
> > + return -ENODEV;
> > + }
> > + if (obj->buffer.length == 1) {
> > + conf->property_conf.read_opcode = obj->buffer.pointer[0];
> > + } else {
> > + acpi_handle_err(handle,
> > + "Error _DSM call to get HID read opcode, too long buffer");
> > + ACPI_FREE(obj);
> > + return -ENODEV;
> > + }
> > + ACPI_FREE(obj);
> > +
> > + obj = acpi_evaluate_dsm_typed(handle, &spi_hid_guid, 3, 5, NULL,
> > + ACPI_TYPE_BUFFER);
> > + if (!obj) {
> > + acpi_handle_err(handle,
> > + "Error _DSM call to get HID write opcode failed");
> > + return -ENODEV;
> > + }
> > + if (obj->buffer.length == 1) {
> > + conf->property_conf.write_opcode = obj->buffer.pointer[0];
> > + } else {
> > + acpi_handle_err(handle,
> > + "Error _DSM call to get HID write opcode, too long buffer");
> > + ACPI_FREE(obj);
> > + return -ENODEV;
> > + }
> > + ACPI_FREE(obj);
> > +
> > + /* Value not provided in ACPI,*/
> > + conf->post_power_on_delay_ms = 5;
> > + conf->minimal_reset_delay_ms = 150;
> > +
> > + if (!acpi_has_method(handle, "_RST")) {
> > + acpi_handle_err(handle, "No reset method for acpi handle");
> > + return -ENODEV;
>
> I would return -EINVAL as we have the device with right _DSM but without
> mandated by the spec _RST.
Thanks! Will fix this in v2.
>
> > + }
> > +
> > + /* FIXME: not reading hid-over-spi-flags, multi-SPI not supported */
> > +
> > + return 0;
> > +}
> > +
> > +static int spi_hid_acpi_power_none(struct spihid_ops *ops)
> > +{
> > + return 0;
> > +}
> > +
> > +static int spi_hid_acpi_power_down(struct spihid_ops *ops)
> > +{
> > + struct spi_hid_acpi_config *conf = container_of(ops,
> > + struct spi_hid_acpi_config,
> > + ops);
> > +
> > + return acpi_device_set_power(conf->adev, ACPI_STATE_D3);
> > +}
> > +
> > +static int spi_hid_acpi_power_up(struct spihid_ops *ops)
> > +{
> > + struct spi_hid_acpi_config *conf = container_of(ops,
> > + struct spi_hid_acpi_config,
> > + ops);
> > + int error;
> > +
> > + error = acpi_device_set_power(conf->adev, ACPI_STATE_D0);
> > + if (error) {
> > + dev_err(&conf->adev->dev, "Error could not power up ACPI device: %d.", error);
> > + return error;
> > + }
> > +
> > + if (conf->post_power_on_delay_ms)
> > + msleep(conf->post_power_on_delay_ms);
> > +
> > + return 0;
> > +}
> > +
> > +static int spi_hid_acpi_assert_reset(struct spihid_ops *ops)
> > +{
> > + return 0;
> > +}
> > +
> > +static int spi_hid_acpi_deassert_reset(struct spihid_ops *ops)
> > +{
> > + struct spi_hid_acpi_config *conf = container_of(ops,
> > + struct spi_hid_acpi_config,
> > + ops);
> > +
> > + return device_reset(&conf->adev->dev);
> > +}
> > +
> > +static void spi_hid_acpi_sleep_minimal_reset_delay(struct spihid_ops *ops)
> > +{
> > + struct spi_hid_acpi_config *conf = container_of(ops,
> > + struct spi_hid_acpi_config,
> > + ops);
> > + usleep_range(1000 * conf->minimal_reset_delay_ms,
> > + 1000 * (conf->minimal_reset_delay_ms + 1));
>
> I'd probably use "fsleep(conf->minimal_reset_delay_ms * 1000)".
I will fix this in v2. And do the same for the of driver.
>
> > +}
> > +
> > +static int spi_hid_acpi_probe(struct spi_device *spi)
> > +{
> > + struct device *dev = &spi->dev;
> > + struct acpi_device *adev;
> > + struct spi_hid_acpi_config *config;
> > + int error;
> > +
> > + adev = ACPI_COMPANION(dev);
> > + if (!adev) {
> > + dev_err(dev, "Error could not get ACPI device.");
> > + return -ENODEV;
> > + }
> > +
> > + config = devm_kzalloc(dev, sizeof(struct spi_hid_acpi_config),
> > + GFP_KERNEL);
> > + if (!config)
> > + return -ENOMEM;
> > +
> > + if (acpi_device_power_manageable(adev)) {
> > + config->ops.power_up = spi_hid_acpi_power_up;
> > + config->ops.power_down = spi_hid_acpi_power_down;
> > + } else {
> > + config->ops.power_up = spi_hid_acpi_power_none;
> > + config->ops.power_down = spi_hid_acpi_power_none;
> > + }
> > + config->ops.assert_reset = spi_hid_acpi_assert_reset;
> > + config->ops.deassert_reset = spi_hid_acpi_deassert_reset;
> > + config->ops.sleep_minimal_reset_delay =
> > + spi_hid_acpi_sleep_minimal_reset_delay;
> > +
> > + error = spi_hid_acpi_populate_config(config, adev);
> > + if (error) {
> > + dev_err(dev, "%s: unable to populate config data.", __func__);
> > + return error;
> > + }
>
> I would add a blank line.
Sure! Will fix this in v2.
>
> > + return spi_hid_core_probe(spi, &config->ops, &config->property_conf);
> > +}
> > +
> > +static const struct acpi_device_id spi_hid_acpi_match[] = {
> > + { "ACPI0C51", 0 },
> > + { "PNP0C51", 0 },
> > + { },
>
> No comma on sentinels.
Will fix this in v2.
>
> > +};
> > +MODULE_DEVICE_TABLE(acpi, spi_hid_acpi_match);
> > +
> > +static struct spi_driver spi_hid_acpi_driver = {
> > + .driver = {
> > + .name = "spi_hid_acpi",
> > + .owner = THIS_MODULE,
> > + .acpi_match_table = ACPI_PTR(spi_hid_acpi_match),
>
> This is dependent on ACPI, so no need to sue ACPI_PTR().
Will fix this in v2 and remove of_match_ptr in the of driver as well.
>
> > + .probe_type = PROBE_PREFER_ASYNCHRONOUS,
> > + .dev_groups = spi_hid_groups,
> > + },
> > + .probe = spi_hid_acpi_probe,
> > + .remove = spi_hid_core_remove,
> > +};
> > +
> > +module_spi_driver(spi_hid_acpi_driver);
> > +
> > +MODULE_DESCRIPTION("HID over SPI ACPI transport driver");
> > +MODULE_AUTHOR("Angela Czubak <aczubak@google.com>");
> > +MODULE_LICENSE("GPL");
> > diff --git a/drivers/hid/spi-hid/spi-hid-core.c b/drivers/hid/spi-hid/spi-hid-core.c
> > index e3273846267e..02beb209a92d 100644
> > --- a/drivers/hid/spi-hid/spi-hid-core.c
> > +++ b/drivers/hid/spi-hid/spi-hid-core.c
> > @@ -43,6 +43,9 @@
> > #include <linux/wait.h>
> > #include <linux/workqueue.h>
> >
> > +#include "spi-hid.h"
> > +#include "spi-hid-core.h"
> > +
> > /* Protocol constants */
> > #define SPI_HID_READ_APPROVAL_CONSTANT 0xff
> > #define SPI_HID_INPUT_HEADER_SYNC_BYTE 0x5a
> > @@ -105,30 +108,6 @@ struct spi_hid_output_report {
> > u8 *content;
> > };
> >
> > -/* struct spi_hid_conf - Conf provided to the core */
> > -struct spi_hid_conf {
> > - u32 input_report_header_address;
> > - u32 input_report_body_address;
> > - u32 output_report_address;
> > - u8 read_opcode;
> > - u8 write_opcode;
> > -};
> > -
> > -/**
> > - * struct spihid_ops - Ops provided to the core
> > - * @power_up: do sequencing to power up the device
> > - * @power_down: do sequencing to power down the device
> > - * @assert_reset: do sequencing to assert the reset line
> > - * @deassert_reset: do sequencing to deassert the reset line
> > - */
> > -struct spihid_ops {
> > - int (*power_up)(struct spihid_ops *ops);
> > - int (*power_down)(struct spihid_ops *ops);
> > - int (*assert_reset)(struct spihid_ops *ops);
> > - int (*deassert_reset)(struct spihid_ops *ops);
> > - void (*sleep_minimal_reset_delay)(struct spihid_ops *ops);
> > -};
> > -
> > static struct hid_ll_driver spi_hid_ll_driver;
> >
> > static void spi_hid_populate_read_approvals(const struct spi_hid_conf *conf,
> > diff --git a/drivers/hid/spi-hid/spi-hid.h b/drivers/hid/spi-hid/spi-hid.h
> > new file mode 100644
> > index 000000000000..1fdd45262647
> > --- /dev/null
> > +++ b/drivers/hid/spi-hid/spi-hid.h
> > @@ -0,0 +1,44 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +/*
> > + * Copyright (c) 2021 Microsoft Corporation
> > + * Copyright (c) 2026 Google LLC
> > + */
> > +
> > +#ifndef SPI_HID_H
> > +#define SPI_HID_H
> > +
> > +#include <linux/spi/spi.h>
> > +#include <linux/sysfs.h>
> > +
> > +/* struct spi_hid_conf - Conf provided to the core */
> > +struct spi_hid_conf {
> > + u32 input_report_header_address;
> > + u32 input_report_body_address;
> > + u32 output_report_address;
> > + u8 read_opcode;
> > + u8 write_opcode;
> > +};
> > +
> > +/**
> > + * struct spihid_ops - Ops provided to the core
> > + * @power_up: do sequencing to power up the device
> > + * @power_down: do sequencing to power down the device
> > + * @assert_reset: do sequencing to assert the reset line
> > + * @deassert_reset: do sequencing to deassert the reset line
> > + */
> > +struct spihid_ops {
> > + int (*power_up)(struct spihid_ops *ops);
> > + int (*power_down)(struct spihid_ops *ops);
> > + int (*assert_reset)(struct spihid_ops *ops);
> > + int (*deassert_reset)(struct spihid_ops *ops);
> > + void (*sleep_minimal_reset_delay)(struct spihid_ops *ops);
> > +};
> > +
> > +int spi_hid_core_probe(struct spi_device *spi, struct spihid_ops *ops,
> > + struct spi_hid_conf *conf);
> > +
> > +void spi_hid_core_remove(struct spi_device *spi);
> > +
> > +extern const struct attribute_group *spi_hid_groups[];
> > +
> > +#endif /* SPI_HID_H */
>
> I am not sure if this belongs to this patch or if it should be better in
> the patch introducing the main driver from the beginning.
These definitions were in spi-hid-core.c in the previous patch introducing
the main driver because it was only used in one .c file. This patch introduces
spi-hid-acpi.c and now two .c files need it so I created a separate .h
file here.
>
> For the ACPI part:
>
> Reviewed-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
>
> Thanks.
>
> --
> Dmitry
^ permalink raw reply
* [PATCH 0/2] Input: ft5x06: Add support for FocalTech FT3519
From: Bhushan Shah @ 2026-03-13 6:39 UTC (permalink / raw)
To: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: linux-input, devicetree, linux-kernel, Bhushan Shah
This touchscreen supports upto 10 touch points, add devicetree bindings
and compatible in driver for it.
Signed-off-by: Bhushan Shah <bhushan.shah@machinesoul.in>
---
Bhushan Shah (2):
dt-bindings: input: touchscreen: edt-ft5x06: Add FocalTech FT3519
Input: edt-ft5x06 - add support for FocalTech FT3519
Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.yaml | 1 +
drivers/input/touchscreen/edt-ft5x06.c | 6 ++++++
2 files changed, 7 insertions(+)
---
base-commit: 0257f64bdac7fdca30fa3cae0df8b9ecbec7733a
change-id: 20260313-edt-ft3519-b3e2a33e88ee
Best regards,
--
Bhushan Shah <bhushan.shah@machinesoul.in>
^ permalink raw reply
* [PATCH 1/2] dt-bindings: input: touchscreen: edt-ft5x06: Add FocalTech FT3519
From: Bhushan Shah @ 2026-03-13 6:39 UTC (permalink / raw)
To: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: linux-input, devicetree, linux-kernel, Bhushan Shah
In-Reply-To: <20260313-edt-ft3519-v1-0-fe5ffc632fd2@machinesoul.in>
Document FocalTech FT3519 support by adding the compatible. It's 10
point touchscreen, which works with same driver.
Signed-off-by: Bhushan Shah <bhushan.shah@machinesoul.in>
---
Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.yaml | 1 +
1 file changed, 1 insertion(+)
diff --git a/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.yaml b/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.yaml
index 6f90522de8c0..34161af90156 100644
--- a/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.yaml
+++ b/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.yaml
@@ -40,6 +40,7 @@ properties:
- edt,edt-ft5506
- evervision,ev-ft5726
- focaltech,ft3518
+ - focaltech,ft3519
- focaltech,ft5426
- focaltech,ft5452
- focaltech,ft6236
--
2.53.0
^ permalink raw reply related
* [PATCH 2/2] Input: edt-ft5x06 - add support for FocalTech FT3519
From: Bhushan Shah @ 2026-03-13 6:39 UTC (permalink / raw)
To: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: linux-input, devicetree, linux-kernel, Bhushan Shah
In-Reply-To: <20260313-edt-ft3519-v1-0-fe5ffc632fd2@machinesoul.in>
This driver is compatible with the FocalTech FT3519 touchscreen, which
supports up to 10 concurrent touch points. Add a compatible for it.
Signed-off-by: Bhushan Shah <bhushan.shah@machinesoul.in>
---
drivers/input/touchscreen/edt-ft5x06.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c
index d0ab644be006..52188e1aa9bc 100644
--- a/drivers/input/touchscreen/edt-ft5x06.c
+++ b/drivers/input/touchscreen/edt-ft5x06.c
@@ -1479,6 +1479,10 @@ static const struct edt_i2c_chip_data edt_ft3518_data = {
.max_support_points = 10,
};
+static const struct edt_i2c_chip_data edt_ft3519_data = {
+ .max_support_points = 10,
+};
+
static const struct edt_i2c_chip_data edt_ft5452_data = {
.max_support_points = 5,
};
@@ -1508,6 +1512,7 @@ static const struct i2c_device_id edt_ft5x06_ts_id[] = {
{ .name = "edt-ft5506", .driver_data = (long)&edt_ft5506_data },
{ .name = "ev-ft5726", .driver_data = (long)&edt_ft5506_data },
{ .name = "ft3518", .driver_data = (long)&edt_ft3518_data },
+ { .name = "ft3519", .driver_data = (long)&edt_ft3519_data },
{ .name = "ft5452", .driver_data = (long)&edt_ft5452_data },
/* Note no edt- prefix for compatibility with the ft6236.c driver */
{ .name = "ft6236", .driver_data = (long)&edt_ft6236_data },
@@ -1525,6 +1530,7 @@ static const struct of_device_id edt_ft5x06_of_match[] = {
{ .compatible = "edt,edt-ft5506", .data = &edt_ft5506_data },
{ .compatible = "evervision,ev-ft5726", .data = &edt_ft5506_data },
{ .compatible = "focaltech,ft3518", .data = &edt_ft3518_data },
+ { .compatible = "focaltech,ft3519", .data = &edt_ft3519_data },
{ .compatible = "focaltech,ft5426", .data = &edt_ft5506_data },
{ .compatible = "focaltech,ft5452", .data = &edt_ft5452_data },
/* Note focaltech vendor prefix for compatibility with ft6236.c */
--
2.53.0
^ permalink raw reply related
* [PATCH 0/4] HID: bpf fixes for 7.0/7.1
From: Benjamin Tissoires @ 2026-03-13 7:40 UTC (permalink / raw)
To: Jiri Kosina, Shuah Khan
Cc: linux-input, linux-kselftest, linux-kernel, Benjamin Tissoires,
kernel test robot, stable
Hi,
This is a series that targets a few HID-BPF issues I discovered or I've
been reported:
- first 2 patches should go to for-7.0/upstream-fixes:
- 1/4 fixes a compilation issue when HID is not enabled
- 2/4 is a nasty bug which allows a HID-BPF to crash the running
kernel, so not critical (you need special permissions to load the
HID-BPF program), but not great as you don't expect tinkering with
HID-BPF would crash
- last 2 patches are more 7.1 material: basically the LEDs on the
keyboards are bypassing HID-BPF, and then that made me realize that
the fallback calls in case of an unnumbered report is not correct (and
likely unnoticed because I don't think I've seen unnumbered reports on
anything else than USB devices)
Cheers,
Benjamin
Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
---
Benjamin Tissoires (4):
selftests/hid: fix compilation when bpf_wq and hid_device are not exported
HID: bpf: prevent buffer overflow in hid_hw_request
HID: fix LEDs when report is unnumbered
HID: do not bypass HID-BPF when setting LEDs
drivers/hid/bpf/hid_bpf_dispatch.c | 2 ++
drivers/hid/hid-input.c | 16 +++++++++-------
tools/testing/selftests/hid/progs/hid_bpf_helpers.h | 12 ++++++++++++
3 files changed, 23 insertions(+), 7 deletions(-)
---
base-commit: 48976c0eba2ff3a3b893c35853bdf27369b16655
change-id: 20260313-wip-bpf-fixes-2fe794000870
Best regards,
--
Benjamin Tissoires <bentiss@kernel.org>
^ permalink raw reply
* [PATCH 1/4] selftests/hid: fix compilation when bpf_wq and hid_device are not exported
From: Benjamin Tissoires @ 2026-03-13 7:40 UTC (permalink / raw)
To: Jiri Kosina, Shuah Khan
Cc: linux-input, linux-kselftest, linux-kernel, Benjamin Tissoires,
kernel test robot, stable
In-Reply-To: <20260313-wip-bpf-fixes-v1-0-74b860315060@kernel.org>
This can happen in situations when CONFIG_HID_SUPPORT is set to no, or
some complex situations where struct bpf_wq is not exported.
So do the usual dance of hiding them before including vmlinux.h, and
then redefining them and make use of CO-RE to have the correct offsets.
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202603111558.KLCIxsZB-lkp@intel.com/
Cc: stable@vger.kernel.org
Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
---
tools/testing/selftests/hid/progs/hid_bpf_helpers.h | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/tools/testing/selftests/hid/progs/hid_bpf_helpers.h b/tools/testing/selftests/hid/progs/hid_bpf_helpers.h
index 80ab60905865..2c6ec907dd05 100644
--- a/tools/testing/selftests/hid/progs/hid_bpf_helpers.h
+++ b/tools/testing/selftests/hid/progs/hid_bpf_helpers.h
@@ -8,9 +8,11 @@
/* "undefine" structs and enums in vmlinux.h, because we "override" them below */
#define hid_bpf_ctx hid_bpf_ctx___not_used
#define hid_bpf_ops hid_bpf_ops___not_used
+#define hid_device hid_device___not_used
#define hid_report_type hid_report_type___not_used
#define hid_class_request hid_class_request___not_used
#define hid_bpf_attach_flags hid_bpf_attach_flags___not_used
+#define bpf_wq bpf_wq___not_used
#define HID_INPUT_REPORT HID_INPUT_REPORT___not_used
#define HID_OUTPUT_REPORT HID_OUTPUT_REPORT___not_used
#define HID_FEATURE_REPORT HID_FEATURE_REPORT___not_used
@@ -29,9 +31,11 @@
#undef hid_bpf_ctx
#undef hid_bpf_ops
+#undef hid_device
#undef hid_report_type
#undef hid_class_request
#undef hid_bpf_attach_flags
+#undef bpf_wq
#undef HID_INPUT_REPORT
#undef HID_OUTPUT_REPORT
#undef HID_FEATURE_REPORT
@@ -55,6 +59,14 @@ enum hid_report_type {
HID_REPORT_TYPES,
};
+struct hid_device {
+ unsigned int id;
+} __attribute__((preserve_access_index));
+
+struct bpf_wq {
+ __u64 __opaque[2];
+};
+
struct hid_bpf_ctx {
struct hid_device *hid;
__u32 allocated_size;
--
2.52.0
^ permalink raw reply related
* [PATCH 2/4] HID: bpf: prevent buffer overflow in hid_hw_request
From: Benjamin Tissoires @ 2026-03-13 7:40 UTC (permalink / raw)
To: Jiri Kosina, Shuah Khan
Cc: linux-input, linux-kselftest, linux-kernel, Benjamin Tissoires,
stable
In-Reply-To: <20260313-wip-bpf-fixes-v1-0-74b860315060@kernel.org>
right now the returned value is considered to be always valid. However,
when playing with HID-BPF, the return value can be arbitrary big,
because it's the return value of dispatch_hid_bpf_raw_requests(), which
calls the struct_ops and we have no guarantees that the value makes
sense.
Cc: stable@vger.kernel.org
Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
---
drivers/hid/bpf/hid_bpf_dispatch.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/hid/bpf/hid_bpf_dispatch.c b/drivers/hid/bpf/hid_bpf_dispatch.c
index f3d15994ca1e..50c7b45c59e3 100644
--- a/drivers/hid/bpf/hid_bpf_dispatch.c
+++ b/drivers/hid/bpf/hid_bpf_dispatch.c
@@ -444,6 +444,8 @@ hid_bpf_hw_request(struct hid_bpf_ctx *ctx, __u8 *buf, size_t buf__sz,
(u64)(long)ctx,
true); /* prevent infinite recursions */
+ if (ret > size)
+ ret = size;
if (ret > 0)
memcpy(buf, dma_data, ret);
--
2.52.0
^ permalink raw reply related
* [PATCH 3/4] HID: fix LEDs when report is unnumbered
From: Benjamin Tissoires @ 2026-03-13 7:40 UTC (permalink / raw)
To: Jiri Kosina, Shuah Khan
Cc: linux-input, linux-kselftest, linux-kernel, Benjamin Tissoires
In-Reply-To: <20260313-wip-bpf-fixes-v1-0-74b860315060@kernel.org>
Both usbhid and i2c-hid are expecting the first byte of the incoming
data to be the report ID. It should be set to 0 when the report ID is
not used. Currently, we are not enforcing this, which means that if we
enforce HID-BPF for LEDs, we will not be able to send them.
Note that the allocated buffer takes into account that extra byte, so
this is safe to shift the data buffer by one.
Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
---
drivers/hid/hid-input.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
index d5308adb2894..eb84f63c51b8 100644
--- a/drivers/hid/hid-input.c
+++ b/drivers/hid/hid-input.c
@@ -1830,9 +1830,9 @@ static void hidinput_led_worker(struct work_struct *work)
led_work);
struct hid_field *field;
struct hid_report *report;
+ __u8 *buf, *data;
int ret;
u32 len;
- __u8 *buf;
field = hidinput_get_led_field(hid);
if (!field)
@@ -1863,7 +1863,14 @@ static void hidinput_led_worker(struct work_struct *work)
if (!buf)
return;
- hid_output_report(report, buf);
+ data = buf;
+ if (!report->id) {
+ data = &buf[1];
+ len++;
+ }
+
+ hid_output_report(report, data);
+
/* synchronous output report */
ret = hid_hw_output_report(hid, buf, len);
if (ret == -ENOSYS)
--
2.52.0
^ permalink raw reply related
* [PATCH 4/4] HID: do not bypass HID-BPF when setting LEDs
From: Benjamin Tissoires @ 2026-03-13 7:40 UTC (permalink / raw)
To: Jiri Kosina, Shuah Khan
Cc: linux-input, linux-kselftest, linux-kernel, Benjamin Tissoires
In-Reply-To: <20260313-wip-bpf-fixes-v1-0-74b860315060@kernel.org>
Right now LED workers are using the special .request() which is
asynchronous but bypasses entirely HID-BPF. This means we can not tweak
a HID keyboard.
Drop the asynchronous (only used by usbhid), and rely on the common
implementation.
Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
---
drivers/hid/hid-input.c | 5 -----
1 file changed, 5 deletions(-)
diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
index eb84f63c51b8..f0a77a4b2a42 100644
--- a/drivers/hid/hid-input.c
+++ b/drivers/hid/hid-input.c
@@ -1853,11 +1853,6 @@ static void hidinput_led_worker(struct work_struct *work)
report = field->report;
- /* use custom SET_REPORT request if possible (asynchronous) */
- if (hid->ll_driver->request)
- return hid->ll_driver->request(hid, report, HID_REQ_SET_REPORT);
-
- /* fall back to generic raw-output-report */
len = hid_report_len(report);
buf = hid_alloc_report_buf(report, GFP_KERNEL);
if (!buf)
--
2.52.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox