* Re: [PATCH 15/33] rust: rust_is_available: remove warning for 0.66.[01] buggy versions
From: Miguel Ojeda @ 2026-04-02 7:12 UTC (permalink / raw)
To: Gary Guo
Cc: Miguel Ojeda, Nathan Chancellor, Nicolas Schier, Danilo Krummrich,
Andreas Hindborg, Catalin Marinas, Will Deacon, Paul Walmsley,
Palmer Dabbelt, Albert Ou, Alexandre Courbot, David Airlie,
Simona Vetter, Brendan Higgins, David Gow, Greg Kroah-Hartman,
Arve Hjønnevåg, Todd Kjos, Christian Brauner,
Carlos Llamas, Alice Ryhl, Jonathan Corbet, Boqun Feng,
Björn Roy Baron, Benno Lossin, Trevor Gross, rust-for-linux,
linux-kbuild, Lorenzo Stoakes, Vlastimil Babka, Liam R . Howlett,
Uladzislau Rezki, linux-block, moderated for non-subscribers,
Alexandre Ghiti, linux-riscv, nouveau, dri-devel, Rae Moar,
linux-kselftest, kunit-dev, Nick Desaulniers, Bill Wendling,
Justin Stitt, llvm, linux-kernel, Shuah Khan, linux-doc
In-Reply-To: <DHHWO582XLXH.1DU3CO41F1PV7@garyguo.net>
On Wed, Apr 1, 2026 at 4:58 PM Gary Guo <gary@garyguo.net> wrote:
>
> The scripts/rust_is_available.sh change looks correct to me, although I couldn't
> get scripts/rust_is_available_test.py to run on NixOS.
>
> Looks like it filtered out PATH but uses /usr/bin/env to find python binary? For
> obvious reasons that will only work if python is located /usr/bin/python.
Yeah, the script has some assumptions on it (e.g. it also assumes
`dash` behavior vs. `bash` in a couple tests which I should relax
too). Happy to change that.
Thanks for testing it :)
Cheers,
Miguel
^ permalink raw reply
* Re: [PATCH v10 07/21] gpu: nova-core: mm: Add TLB flush support
From: Matthew Brost @ 2026-04-02 5:59 UTC (permalink / raw)
To: Joel Fernandes
Cc: linux-kernel, Miguel Ojeda, Boqun Feng, Gary Guo, Bjorn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Dave Airlie, Daniel Almeida, Koen Koning,
dri-devel, rust-for-linux, Nikola Djukic, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter,
Jonathan Corbet, Alex Deucher, Christian Koenig, Jani Nikula,
Joonas Lahtinen, Rodrigo Vivi, Tvrtko Ursulin, Huang Rui,
Matthew Auld, Lucas De Marchi, Thomas Hellstrom, Helge Deller,
Alex Gaynor, Boqun Feng, John Hubbard, Alistair Popple,
Timur Tabi, Edwin Peer, Alexandre Courbot, Andrea Righi,
Andy Ritger, Zhi Wang, Balbir Singh, Philipp Stanner,
Elle Rhumsaa, alexeyi, Eliot Courtney, joel, linux-doc, amd-gfx,
intel-gfx, intel-xe, linux-fbdev
In-Reply-To: <20260331212048.2229260-8-joelagnelf@nvidia.com>
On Tue, Mar 31, 2026 at 05:20:34PM -0400, Joel Fernandes wrote:
> Add TLB (Translation Lookaside Buffer) flush support for GPU MMU.
>
> After modifying page table entries, the GPU's TLB must be invalidated
> to ensure the new mappings take effect. The Tlb struct provides flush
> functionality through BAR0 registers.
>
> The flush operation writes the page directory base address and triggers
> an invalidation, polling for completion with a 2 second timeout matching
> the Nouveau driver.
>
> Cc: Nikola Djukic <ndjukic@nvidia.com>
> Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
> ---
> drivers/gpu/nova-core/mm.rs | 1 +
> drivers/gpu/nova-core/mm/tlb.rs | 95 +++++++++++++++++++++++++++++++++
> drivers/gpu/nova-core/regs.rs | 42 +++++++++++++++
> 3 files changed, 138 insertions(+)
> create mode 100644 drivers/gpu/nova-core/mm/tlb.rs
>
> diff --git a/drivers/gpu/nova-core/mm.rs b/drivers/gpu/nova-core/mm.rs
> index 8f3089a5fa88..cfe9cbe11d57 100644
> --- a/drivers/gpu/nova-core/mm.rs
> +++ b/drivers/gpu/nova-core/mm.rs
> @@ -5,6 +5,7 @@
> #![expect(dead_code)]
>
> pub(crate) mod pramin;
> +pub(crate) mod tlb;
>
> use kernel::sizes::SZ_4K;
>
> diff --git a/drivers/gpu/nova-core/mm/tlb.rs b/drivers/gpu/nova-core/mm/tlb.rs
> new file mode 100644
> index 000000000000..cd3cbcf4c739
> --- /dev/null
> +++ b/drivers/gpu/nova-core/mm/tlb.rs
> @@ -0,0 +1,95 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! TLB (Translation Lookaside Buffer) flush support for GPU MMU.
> +//!
> +//! After modifying page table entries, the GPU's TLB must be flushed to
> +//! ensure the new mappings take effect. This module provides TLB flush
> +//! functionality for virtual memory managers.
> +//!
> +//! # Example
> +//!
> +//! ```ignore
> +//! use crate::mm::tlb::Tlb;
> +//!
> +//! fn page_table_update(tlb: &Tlb, pdb_addr: VramAddress) -> Result<()> {
> +//! // ... modify page tables ...
> +//!
> +//! // Flush TLB to make changes visible (polls for completion).
> +//! tlb.flush(pdb_addr)?;
> +//!
> +//! Ok(())
> +//! }
> +//! ```
> +
> +use kernel::{
> + devres::Devres,
> + io::poll::read_poll_timeout,
> + io::Io,
> + new_mutex,
> + prelude::*,
> + sync::{
> + Arc,
> + Mutex, //
> + },
> + time::Delta, //
> +};
> +
> +use crate::{
> + driver::Bar0,
> + mm::VramAddress,
> + regs, //
> +};
> +
> +/// TLB manager for GPU translation buffer operations.
> +#[pin_data]
> +pub(crate) struct Tlb {
> + bar: Arc<Devres<Bar0>>,
> + /// TLB flush serialization lock: This lock is acquired during the
> + /// DMA fence signalling critical path. It must NEVER be held across any
> + /// reclaimable CPU memory allocations because the memory reclaim path can
> + /// call `dma_fence_wait()`, which would deadlock with this lock held.
> + #[pin]
> + lock: Mutex<()>,
> +}
> +
> +impl Tlb {
> + /// Create a new TLB manager.
> + pub(super) fn new(bar: Arc<Devres<Bar0>>) -> impl PinInit<Self> {
> + pin_init!(Self {
> + bar,
> + lock <- new_mutex!((), "tlb_flush"),
> + })
> + }
> +
> + /// Flush the GPU TLB for a specific page directory base.
> + ///
> + /// This invalidates all TLB entries associated with the given PDB address.
> + /// Must be called after modifying page table entries to ensure the GPU sees
> + /// the updated mappings.
> + pub(crate) fn flush(&self, pdb_addr: VramAddress) -> Result {
This landed on my list randomly, so I took a look.
Wouldn’t you want to virtualize the invalidation based on your device?
For example, what if you need to register interface changes on future hardware?
Or, if you’re a VF, can you even do MMIO?
I’d relayer this.
Matt
> + let _guard = self.lock.lock();
> +
> + let bar = self.bar.try_access().ok_or(ENODEV)?;
> +
> + // Write PDB address.
> + bar.write_reg(regs::NV_TLB_FLUSH_PDB_LO::from_pdb_addr(pdb_addr.raw_u64()));
> + bar.write_reg(regs::NV_TLB_FLUSH_PDB_HI::from_pdb_addr(pdb_addr.raw_u64()));
> +
> + // Trigger flush: invalidate all pages and enable.
> + bar.write_reg(
> + regs::NV_TLB_FLUSH_CTRL::zeroed()
> + .with_page_all(true)
> + .with_enable(true),
> + );
> +
> + // Poll for completion - enable bit clears when flush is done.
> + read_poll_timeout(
> + || Ok(bar.read(regs::NV_TLB_FLUSH_CTRL)),
> + |ctrl: ®s::NV_TLB_FLUSH_CTRL| !ctrl.enable(),
> + Delta::ZERO,
> + Delta::from_secs(2),
> + )?;
> +
> + Ok(())
> + }
> +}
> diff --git a/drivers/gpu/nova-core/regs.rs b/drivers/gpu/nova-core/regs.rs
> index a3ca02345e20..5e3f5933a55c 100644
> --- a/drivers/gpu/nova-core/regs.rs
> +++ b/drivers/gpu/nova-core/regs.rs
> @@ -548,3 +548,45 @@ pub(crate) mod ga100 {
> }
> }
> }
> +
> +// MMU TLB
> +
> +register! {
> + /// TLB flush register: PDB address bits [39:8].
> + pub(crate) NV_TLB_FLUSH_PDB_LO(u32) @ 0x00b830a0 {
> + /// PDB address bits [39:8].
> + 31:0 pdb_lo => u32;
> + }
> +
> + /// TLB flush register: PDB address bits [47:40].
> + pub(crate) NV_TLB_FLUSH_PDB_HI(u32) @ 0x00b830a4 {
> + /// PDB address bits [47:40].
> + 7:0 pdb_hi => u8;
> + }
> +
> + /// TLB flush control register.
> + pub(crate) NV_TLB_FLUSH_CTRL(u32) @ 0x00b830b0 {
> + /// Invalidate all pages.
> + 0:0 page_all => bool;
> + /// Enable/trigger flush (clears when flush completes).
> + 31:31 enable => bool;
> + }
> +}
> +
> +impl NV_TLB_FLUSH_PDB_LO {
> + /// Create a register value from a PDB address.
> + ///
> + /// Extracts bits [39:8] of the address and shifts it right by 8 bits.
> + pub(crate) fn from_pdb_addr(addr: u64) -> Self {
> + Self::zeroed().with_pdb_lo(((addr >> 8) & 0xFFFF_FFFF) as u32)
> + }
> +}
> +
> +impl NV_TLB_FLUSH_PDB_HI {
> + /// Create a register value from a PDB address.
> + ///
> + /// Extracts bits [47:40] of the address and shifts it right by 40 bits.
> + pub(crate) fn from_pdb_addr(addr: u64) -> Self {
> + Self::zeroed().with_pdb_hi(((addr >> 40) & 0xFF) as u8)
> + }
> +}
> --
> 2.34.1
>
^ permalink raw reply
* Re: [PATCH v10 21/21] gpu: nova-core: Use runtime BAR1 size instead of hardcoded 256MB
From: Eliot Courtney @ 2026-04-02 5:54 UTC (permalink / raw)
To: Joel Fernandes, linux-kernel
Cc: Miguel Ojeda, Boqun Feng, Gary Guo, Bjorn Roy Baron, Benno Lossin,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
Dave Airlie, Daniel Almeida, Koen Koning, dri-devel,
rust-for-linux, Nikola Djukic, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Jonathan Corbet,
Alex Deucher, Christian Koenig, Jani Nikula, Joonas Lahtinen,
Rodrigo Vivi, Tvrtko Ursulin, Huang Rui, Matthew Auld,
Matthew Brost, Lucas De Marchi, Thomas Hellstrom, Helge Deller,
Alex Gaynor, Boqun Feng, John Hubbard, Alistair Popple,
Timur Tabi, Edwin Peer, Alexandre Courbot, Andrea Righi,
Andy Ritger, Zhi Wang, Balbir Singh, Philipp Stanner,
Elle Rhumsaa, alexeyi, Eliot Courtney, joel, linux-doc, amd-gfx,
intel-gfx, intel-xe, linux-fbdev
In-Reply-To: <20260331212048.2229260-22-joelagnelf@nvidia.com>
On Wed Apr 1, 2026 at 6:20 AM JST, Joel Fernandes wrote:
> From: Zhi Wang <zhiw@nvidia.com>
>
> Remove the hardcoded BAR1_SIZE = SZ_256M constant. On GPUs like L40 the
> BAR1 aperture is larger than 256MB; using a hardcoded size prevents large
> BAR1 from working and mapping it would fail.
>
> Signed-off-by: Zhi Wang <zhiw@nvidia.com>
> Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
> ---
> drivers/gpu/nova-core/driver.rs | 8 ++------
> drivers/gpu/nova-core/gpu.rs | 7 +------
> 2 files changed, 3 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/nova-core/driver.rs b/drivers/gpu/nova-core/driver.rs
> index b1aafaff0cee..6f95f8672158 100644
> --- a/drivers/gpu/nova-core/driver.rs
> +++ b/drivers/gpu/nova-core/driver.rs
> @@ -13,10 +13,7 @@
> Vendor, //
> },
> prelude::*,
> - sizes::{
> - SZ_16M,
> - SZ_256M, //
> - },
> + sizes::SZ_16M,
> sync::{
> atomic::{
> Atomic,
> @@ -40,7 +37,6 @@ pub(crate) struct NovaCore {
> }
>
> const BAR0_SIZE: usize = SZ_16M;
> -pub(crate) const BAR1_SIZE: usize = SZ_256M;
>
> // For now we only support Ampere which can use up to 47-bit DMA addresses.
> //
> @@ -51,7 +47,7 @@ pub(crate) struct NovaCore {
> const GPU_DMA_BITS: u32 = 47;
>
> pub(crate) type Bar0 = pci::Bar<BAR0_SIZE>;
> -pub(crate) type Bar1 = pci::Bar<BAR1_SIZE>;
> +pub(crate) type Bar1 = pci::Bar;
>
> kernel::pci_device_table!(
> PCI_TABLE,
> diff --git a/drivers/gpu/nova-core/gpu.rs b/drivers/gpu/nova-core/gpu.rs
> index 8206ec015b26..ba6f1f6f0485 100644
> --- a/drivers/gpu/nova-core/gpu.rs
> +++ b/drivers/gpu/nova-core/gpu.rs
> @@ -353,16 +353,11 @@ pub(crate) fn run_selftests(
>
> #[cfg(CONFIG_NOVA_MM_SELFTESTS)]
> fn run_mm_selftests(self: Pin<&mut Self>, pdev: &pci::Device<device::Bound>) -> Result {
> - use crate::driver::BAR1_SIZE;
> -
> // PRAMIN aperture self-tests.
> crate::mm::pramin::run_self_test(pdev.as_ref(), self.mm.pramin(), self.spec.chipset)?;
>
> // BAR1 self-tests.
> - let bar1 = Arc::pin_init(
> - pdev.iomap_region_sized::<BAR1_SIZE>(1, c"nova-core/bar1"),
> - GFP_KERNEL,
> - )?;
> + let bar1 = Arc::pin_init(pdev.iomap_region(1, c"nova-core/bar1"), GFP_KERNEL)?;
> let bar1_access = bar1.access(pdev.as_ref())?;
>
> crate::mm::bar_user::run_self_test(
Can we move this directly after patch 17 which adds the fixed bar1? Or
alternatively fold it in while preserving Zhi's attribution (I am not
sure what the conventional method for this is).
^ permalink raw reply
* Re: [PATCH v10 02/21] gpu: nova-core: gsp: Extract usable FB region from GSP
From: Eliot Courtney @ 2026-04-02 5:49 UTC (permalink / raw)
To: Joel Fernandes, Eliot Courtney, linux-kernel
Cc: Miguel Ojeda, Boqun Feng, Gary Guo, Bjorn Roy Baron, Benno Lossin,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
Dave Airlie, Daniel Almeida, Koen Koning, dri-devel,
rust-for-linux, Nikola Djukic, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Jonathan Corbet,
Alex Deucher, Christian Koenig, Jani Nikula, Joonas Lahtinen,
Rodrigo Vivi, Tvrtko Ursulin, Huang Rui, Matthew Auld,
Matthew Brost, Lucas De Marchi, Thomas Hellstrom, Helge Deller,
Alex Gaynor, Boqun Feng, John Hubbard, Alistair Popple,
Timur Tabi, Edwin Peer, Alexandre Courbot, Andrea Righi,
Andy Ritger, Zhi Wang, Balbir Singh, Philipp Stanner,
Elle Rhumsaa, alexeyi, joel, linux-doc, amd-gfx, intel-gfx,
intel-xe, linux-fbdev
In-Reply-To: <d6fe3e40-0310-4b90-ac8c-8beeec886f90@nvidia.com>
On Thu Apr 2, 2026 at 8:24 AM JST, Joel Fernandes wrote:
>
>
> On 4/1/2026 4:27 AM, Eliot Courtney wrote:
>> On Wed Apr 1, 2026 at 6:20 AM JST, Joel Fernandes wrote:
>>> Add first_usable_fb_region() to GspStaticConfigInfo to extract the first
>>> usable FB region from GSP's fbRegionInfoParams. Usable regions are those
>>> that are not reserved or protected.
>>>
>>> The extracted region is stored in GetGspStaticInfoReply and exposed as
>>> usable_fb_region field for use by the memory subsystem.
>>>
>>> Cc: Nikola Djukic <ndjukic@nvidia.com>
>>> Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
>>> ---
>>
>> Please see my feedback from v9[1] which still applies.
>>
>> [1]: https://lore.kernel.org/all/DH1GK30TUB4V.2GR6ANXIZDFFQ@nvidia.com/
>
> Yeah, I am seeing it now. Amidst making the earlier 7.1 merge window for
> the DRM buddy and earlier patches in the series, I missed this. They seem
> to be simple nits and I will address them in the next revision. thanks,
>
> --
> Joel Fernandes
No worries. Sorry I have not gotten to more of the patches yet. Trying
to get through some more now. Thanks!
^ permalink raw reply
* Re: [PATCH v10 07/21] gpu: nova-core: mm: Add TLB flush support
From: Eliot Courtney @ 2026-04-02 5:49 UTC (permalink / raw)
To: Joel Fernandes, linux-kernel
Cc: Miguel Ojeda, Boqun Feng, Gary Guo, Bjorn Roy Baron, Benno Lossin,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
Dave Airlie, Daniel Almeida, Koen Koning, dri-devel,
rust-for-linux, Nikola Djukic, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Jonathan Corbet,
Alex Deucher, Christian Koenig, Jani Nikula, Joonas Lahtinen,
Rodrigo Vivi, Tvrtko Ursulin, Huang Rui, Matthew Auld,
Matthew Brost, Lucas De Marchi, Thomas Hellstrom, Helge Deller,
Alex Gaynor, Boqun Feng, John Hubbard, Alistair Popple,
Timur Tabi, Edwin Peer, Alexandre Courbot, Andrea Righi,
Andy Ritger, Zhi Wang, Balbir Singh, Philipp Stanner,
Elle Rhumsaa, alexeyi, Eliot Courtney, joel, linux-doc, amd-gfx,
intel-gfx, intel-xe, linux-fbdev
In-Reply-To: <20260331212048.2229260-8-joelagnelf@nvidia.com>
On Wed Apr 1, 2026 at 6:20 AM JST, Joel Fernandes wrote:
> Add TLB (Translation Lookaside Buffer) flush support for GPU MMU.
>
> After modifying page table entries, the GPU's TLB must be invalidated
> to ensure the new mappings take effect. The Tlb struct provides flush
> functionality through BAR0 registers.
>
> The flush operation writes the page directory base address and triggers
> an invalidation, polling for completion with a 2 second timeout matching
> the Nouveau driver.
>
> Cc: Nikola Djukic <ndjukic@nvidia.com>
> Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
> ---
> drivers/gpu/nova-core/mm.rs | 1 +
> drivers/gpu/nova-core/mm/tlb.rs | 95 +++++++++++++++++++++++++++++++++
> drivers/gpu/nova-core/regs.rs | 42 +++++++++++++++
> 3 files changed, 138 insertions(+)
> create mode 100644 drivers/gpu/nova-core/mm/tlb.rs
>
> diff --git a/drivers/gpu/nova-core/mm.rs b/drivers/gpu/nova-core/mm.rs
> index 8f3089a5fa88..cfe9cbe11d57 100644
> --- a/drivers/gpu/nova-core/mm.rs
> +++ b/drivers/gpu/nova-core/mm.rs
> @@ -5,6 +5,7 @@
> #![expect(dead_code)]
>
> pub(crate) mod pramin;
> +pub(crate) mod tlb;
>
> use kernel::sizes::SZ_4K;
>
> diff --git a/drivers/gpu/nova-core/mm/tlb.rs b/drivers/gpu/nova-core/mm/tlb.rs
> new file mode 100644
> index 000000000000..cd3cbcf4c739
> --- /dev/null
> +++ b/drivers/gpu/nova-core/mm/tlb.rs
> @@ -0,0 +1,95 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! TLB (Translation Lookaside Buffer) flush support for GPU MMU.
> +//!
> +//! After modifying page table entries, the GPU's TLB must be flushed to
> +//! ensure the new mappings take effect. This module provides TLB flush
> +//! functionality for virtual memory managers.
> +//!
> +//! # Example
> +//!
> +//! ```ignore
> +//! use crate::mm::tlb::Tlb;
> +//!
> +//! fn page_table_update(tlb: &Tlb, pdb_addr: VramAddress) -> Result<()> {
> +//! // ... modify page tables ...
> +//!
> +//! // Flush TLB to make changes visible (polls for completion).
> +//! tlb.flush(pdb_addr)?;
> +//!
> +//! Ok(())
> +//! }
> +//! ```
> +
> +use kernel::{
> + devres::Devres,
> + io::poll::read_poll_timeout,
> + io::Io,
> + new_mutex,
> + prelude::*,
> + sync::{
> + Arc,
> + Mutex, //
> + },
> + time::Delta, //
> +};
> +
> +use crate::{
> + driver::Bar0,
> + mm::VramAddress,
> + regs, //
> +};
> +
> +/// TLB manager for GPU translation buffer operations.
> +#[pin_data]
> +pub(crate) struct Tlb {
> + bar: Arc<Devres<Bar0>>,
> + /// TLB flush serialization lock: This lock is acquired during the
> + /// DMA fence signalling critical path. It must NEVER be held across any
> + /// reclaimable CPU memory allocations because the memory reclaim path can
> + /// call `dma_fence_wait()`, which would deadlock with this lock held.
> + #[pin]
This comment says that the lock is acquired during the DMA fence
signalling critical path, but IIUC we don't have anything like that
right now. Is this based on future yet to be done work? Can we reword
this in a way so it makes sense in the current state?
> + lock: Mutex<()>,
> +}
> +
> +impl Tlb {
> + /// Create a new TLB manager.
> + pub(super) fn new(bar: Arc<Devres<Bar0>>) -> impl PinInit<Self> {
> + pin_init!(Self {
> + bar,
> + lock <- new_mutex!((), "tlb_flush"),
> + })
> + }
> +
> + /// Flush the GPU TLB for a specific page directory base.
> + ///
> + /// This invalidates all TLB entries associated with the given PDB address.
> + /// Must be called after modifying page table entries to ensure the GPU sees
> + /// the updated mappings.
If this must be called after every operation like that, I wonder if we
can change the design to require a guard like pattern something to
ensure flush is called. WDYT?
> + pub(crate) fn flush(&self, pdb_addr: VramAddress) -> Result {
Hopefully we don't need to be calling flush() from anywhere in the
entire crate. Can you tighten the visibility here and in other places?
Many things seem to be pub(crate) that don't need to be.
> + let _guard = self.lock.lock();
> +
> + let bar = self.bar.try_access().ok_or(ENODEV)?;
> +
> + // Write PDB address.
> + bar.write_reg(regs::NV_TLB_FLUSH_PDB_LO::from_pdb_addr(pdb_addr.raw_u64()));
> + bar.write_reg(regs::NV_TLB_FLUSH_PDB_HI::from_pdb_addr(pdb_addr.raw_u64()));
> +
> + // Trigger flush: invalidate all pages and enable.
> + bar.write_reg(
> + regs::NV_TLB_FLUSH_CTRL::zeroed()
> + .with_page_all(true)
> + .with_enable(true),
> + );
> +
> + // Poll for completion - enable bit clears when flush is done.
> + read_poll_timeout(
> + || Ok(bar.read(regs::NV_TLB_FLUSH_CTRL)),
> + |ctrl: ®s::NV_TLB_FLUSH_CTRL| !ctrl.enable(),
> + Delta::ZERO,
> + Delta::from_secs(2),
> + )?;
This has zero delay on the read_poll_timeout - what about adding a small
delay of a microsecond or so?
^ permalink raw reply
* Re: [PATCH v10 10/21] gpu: nova-core: mm: Add MMU v2 page table types
From: Eliot Courtney @ 2026-04-02 5:41 UTC (permalink / raw)
To: Joel Fernandes, linux-kernel
Cc: Miguel Ojeda, Boqun Feng, Gary Guo, Bjorn Roy Baron, Benno Lossin,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
Dave Airlie, Daniel Almeida, Koen Koning, dri-devel,
rust-for-linux, Nikola Djukic, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Jonathan Corbet,
Alex Deucher, Christian Koenig, Jani Nikula, Joonas Lahtinen,
Rodrigo Vivi, Tvrtko Ursulin, Huang Rui, Matthew Auld,
Matthew Brost, Lucas De Marchi, Thomas Hellstrom, Helge Deller,
Alex Gaynor, Boqun Feng, John Hubbard, Alistair Popple,
Timur Tabi, Edwin Peer, Alexandre Courbot, Andrea Righi,
Andy Ritger, Zhi Wang, Balbir Singh, Philipp Stanner,
Elle Rhumsaa, alexeyi, Eliot Courtney, joel, linux-doc, amd-gfx,
intel-gfx, intel-xe, linux-fbdev
In-Reply-To: <20260331212048.2229260-11-joelagnelf@nvidia.com>
On Wed Apr 1, 2026 at 6:20 AM JST, Joel Fernandes wrote:
> Add page table entry and directory structures for MMU version 2
> used by Turing/Ampere/Ada GPUs.
>
> Cc: Nikola Djukic <ndjukic@nvidia.com>
> Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
> ---
> drivers/gpu/nova-core/mm/pagetable.rs | 2 +
> drivers/gpu/nova-core/mm/pagetable/ver2.rs | 232 +++++++++++++++++++++
> 2 files changed, 234 insertions(+)
> create mode 100644 drivers/gpu/nova-core/mm/pagetable/ver2.rs
>
> diff --git a/drivers/gpu/nova-core/mm/pagetable.rs b/drivers/gpu/nova-core/mm/pagetable.rs
> index 50b76d5e5aaf..38d88f8f09a9 100644
> --- a/drivers/gpu/nova-core/mm/pagetable.rs
> +++ b/drivers/gpu/nova-core/mm/pagetable.rs
> @@ -8,6 +8,8 @@
>
> #![expect(dead_code)]
>
> +pub(crate) mod ver2;
> +
This looks like it has more visibility than necessary. And it seems
incorrect for anyone in the crate to care about MMU version details.
This can probably be just 'mod ver2'. There are a lot of other types /
functions in this series that could have tighter visibility. Could you
go through and see if you can reduce a bunch to private or pub(super)?
thanks!
^ permalink raw reply
* RE: [PATCH v3 17/27] mtd: spi-nor: debugfs: Add locking support
From: Takahiro.Kuwano @ 2026-04-02 5:40 UTC (permalink / raw)
To: miquel.raynal, pratyush, mwalle, richard, vigneshr, corbet
Cc: tudor.ambarus, sean.anderson, thomas.petazzoni, STLin2, linux-mtd,
linux-kernel, linux-doc
In-Reply-To: <20260317-winbond-v6-18-rc1-spi-nor-swp-v3-17-2ca9ea4e7b9b@bootlin.com>
Hi Miquel,
> The ioctl output may be counter intuitive in some cases. Asking for a
> "locked status" over a region that is only partially locked will return
> "unlocked" whereas in practice maybe the biggest part is actually
> locked.
>
> Knowing what is the real software locking state through debugfs would be
> very convenient for development/debugging purposes, hence this proposal
> for adding an extra block at the end of the file: a "locked sectors"
> array which lists every section, if it is locked or not, showing both
> the address ranges and the sizes in numbers of blocks.
>
> Here is an example of output, what is after the "sector map" is new.
>
> $ cat /sys/kernel/debug/spi-nor/spi0.0/params
> name (null)
> id ef a0 20 00 00 00
> size 64.0 MiB
> write size 1
> page size 256
> address nbytes 4
> flags HAS_SR_TB | 4B_OPCODES | HAS_4BAIT | HAS_LOCK | HAS_16BIT_SR | HAS_SR_TB_BIT6 | HAS_4BIT_BP |
> SOFT_RESET | NO_WP
>
> opcodes
> read 0xec
> dummy cycles 6
> erase 0xdc
> program 0x34
> 8D extension none
>
> protocols
> read 1S-4S-4S
> write 1S-1S-4S
> register 1S-1S-1S
>
> erase commands
> 21 (4.00 KiB) [1]
> dc (64.0 KiB) [3]
> c7 (64.0 MiB)
>
> sector map
> region (in hex) | erase mask | overlaid
> ------------------+------------+---------
> 00000000-03ffffff | [ 3] | no
>
> locked sectors
> region (in hex) | status | #blocks
> ------------------+----------+--------
> 00000000-03ffffff | unlocked | 1024
>
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
> Here are below more examples of output with various situations. The full
> output of the "params" content has been manually removed to only show
> what has been added and how it behaves.
>
> $ flash_lock -l /dev/mtd0 0x3f00000 16
> $ cat /sys/kernel/debug/spi-nor/spi0.0/params
> locked sectors
> region (in hex) | status | #blocks
> ------------------+----------+--------
> 00000000-03efffff | unlocked | 1008
> 03f00000-03ffffff | locked | 16
> $
> $ flash_lock -u /dev/mtd0 0x3f00000 8
> $ cat /sys/kernel/debug/spi-nor/spi0.0/params
> locked sectors
> region (in hex) | status | #blocks
> ------------------+----------+--------
> 00000000-03f7ffff | unlocked | 1016
> 03f80000-03ffffff | locked | 8
> $
> $ flash_lock -u /dev/mtd0
> $ cat /sys/kernel/debug/spi-nor/spi0.0/params
> locked sectors
> region (in hex) | status | #blocks
> ------------------+----------+--------
> 00000000-03ffffff | unlocked | 1024
> $
> $ flash_lock -l /dev/mtd0
> $ cat /sys/kernel/debug/spi-nor/spi0.0/params
> locked sectors
> region (in hex) | status | #blocks
> ------------------+----------+--------
> 00000000-03ffffff | locked | 1024
> $
> $ flash_lock -u /dev/mtd0 0x20000 1022
> $ cat /sys/kernel/debug/spi-nor/spi0.0/params
> locked sectors
> region (in hex) | status | #blocks
> ------------------+----------+--------
> 00000000-0001ffff | locked | 2
> 00020000-03ffffff | unlocked | 1022
> ---
> drivers/mtd/spi-nor/core.h | 4 ++++
> drivers/mtd/spi-nor/debugfs.c | 22 ++++++++++++++++++++++
> drivers/mtd/spi-nor/swp.c | 11 +++++++----
> 3 files changed, 33 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
> index 091eb934abe4..99ed6c54b90f 100644
> --- a/drivers/mtd/spi-nor/core.h
> +++ b/drivers/mtd/spi-nor/core.h
> @@ -707,6 +707,10 @@ static inline bool spi_nor_needs_sfdp(const struct spi_nor *nor)
> return !nor->info->size;
> }
>
> +u64 spi_nor_get_min_prot_length_sr(struct spi_nor *nor);
> +void spi_nor_get_locked_range_sr(struct spi_nor *nor, const u8 *sr, loff_t *ofs, u64 *len);
> +bool spi_nor_is_locked_sr(struct spi_nor *nor, loff_t ofs, u64 len, const u8 *sr);
> +
It would be better to have generic helper functions rather than using SR-based
functions directly. The locking_ops is vendor/chip specific and can provide
other locking mechanism than SR-based block protection. For instance, Infineon,
Micron, Macronix (and maybe other vendors) offer protection mechanisms that can
protect sectors individually with volatile or non-volatile manner.
> #ifdef CONFIG_DEBUG_FS
> void spi_nor_debugfs_register(struct spi_nor *nor);
> void spi_nor_debugfs_shutdown(void);
> diff --git a/drivers/mtd/spi-nor/debugfs.c b/drivers/mtd/spi-nor/debugfs.c
> index d0191eb9f879..821fbc9587dc 100644
> --- a/drivers/mtd/spi-nor/debugfs.c
> +++ b/drivers/mtd/spi-nor/debugfs.c
> @@ -77,10 +77,12 @@ static void spi_nor_print_flags(struct seq_file *s, unsigned long flags,
> static int spi_nor_params_show(struct seq_file *s, void *data)
> {
> struct spi_nor *nor = s->private;
> + unsigned int min_prot_len = spi_nor_get_min_prot_length_sr(nor);
> struct spi_nor_flash_parameter *params = nor->params;
> struct spi_nor_erase_map *erase_map = ¶ms->erase_map;
> struct spi_nor_erase_region *region = erase_map->regions;
> const struct flash_info *info = nor->info;
> + loff_t lock_start, lock_length;
> char buf[16], *str;
> unsigned int i;
>
> @@ -159,6 +161,26 @@ static int spi_nor_params_show(struct seq_file *s, void *data)
> region[i].overlaid ? "yes" : "no");
> }
>
Don't we need to check 'SNOR_F_HAS_LOCK' flag here?
> + seq_puts(s, "\nlocked sectors\n");
> + seq_puts(s, " region (in hex) | status | #blocks\n");
> + seq_puts(s, " ------------------+----------+--------\n");
> +
> + spi_nor_get_locked_range_sr(nor, nor->dfs_sr_cache, &lock_start, &lock_length);
> + if (!lock_length || lock_length == params->size) {
> + seq_printf(s, " %08llx-%08llx | %s | %llu\n", 0ULL, params->size - 1,
> + lock_length ? " locked" : "unlocked", params->size / min_prot_len);
div_u64() is needed.
I got undefined reference to `__aeabi_uldivmod' for my 32-bit ARM platform.
Same for following four seq_printf().
> + } else if (!lock_start) {
> + seq_printf(s, " %08llx-%08llx | %s | %llu\n", 0ULL, lock_length - 1,
> + " locked", lock_length / min_prot_len);
> + seq_printf(s, " %08llx-%08llx | %s | %llu\n", lock_length, params->size - 1,
> + "unlocked", (params->size - lock_length) / min_prot_len);
> + } else {
> + seq_printf(s, " %08llx-%08llx | %s | %llu\n", 0ULL, lock_start - 1,
> + "unlocked", lock_start / min_prot_len);
> + seq_printf(s, " %08llx-%08llx | %s | %llu\n", lock_start, params->size - 1,
> + " locked", lock_length / min_prot_len);
> + }
> +
> return 0;
> }
> DEFINE_SHOW_ATTRIBUTE(spi_nor_params);
> diff --git a/drivers/mtd/spi-nor/swp.c b/drivers/mtd/spi-nor/swp.c
> index 7a6c2b8ef921..8de8459e8e90 100644
> --- a/drivers/mtd/spi-nor/swp.c
> +++ b/drivers/mtd/spi-nor/swp.c
> @@ -32,7 +32,7 @@ static u8 spi_nor_get_sr_tb_mask(struct spi_nor *nor)
> return SR_TB_BIT5;
> }
>
> -static u64 spi_nor_get_min_prot_length_sr(struct spi_nor *nor)
> +u64 spi_nor_get_min_prot_length_sr(struct spi_nor *nor)
> {
> unsigned int bp_slots, bp_slots_needed;
> /*
> @@ -53,8 +53,8 @@ static u64 spi_nor_get_min_prot_length_sr(struct spi_nor *nor)
> return sector_size;
> }
>
> -static void spi_nor_get_locked_range_sr(struct spi_nor *nor, const u8 *sr, loff_t *ofs,
> - u64 *len)
> +void spi_nor_get_locked_range_sr(struct spi_nor *nor, const u8 *sr, loff_t *ofs,
> + u64 *len)
> {
> u64 min_prot_len;
> u8 bp_mask = spi_nor_get_sr_bp_mask(nor);
> @@ -112,7 +112,7 @@ static bool spi_nor_check_lock_status_sr(struct spi_nor *nor, loff_t ofs,
> return (ofs >= lock_offs_max) || (offs_max <= lock_offs);
> }
>
> -static bool spi_nor_is_locked_sr(struct spi_nor *nor, loff_t ofs, u64 len, const u8 *sr)
> +bool spi_nor_is_locked_sr(struct spi_nor *nor, loff_t ofs, u64 len, const u8 *sr)
> {
> return spi_nor_check_lock_status_sr(nor, ofs, len, sr, true);
> }
> @@ -410,6 +410,9 @@ static int spi_nor_sr_is_locked(struct spi_nor *nor, loff_t ofs, u64 len)
> * -is_locked(): Checks if the region is *fully* locked, returns false otherwise.
> * This feeback may be misleading because users may get an "unlocked"
> * status even though a subpart of the region is effectively locked.
> + *
> + * If in doubt during development, check-out the debugfs output which tries to
> + * be more user friendly.
> */
> static const struct spi_nor_locking_ops spi_nor_sr_locking_ops = {
> .lock = spi_nor_sr_lock,
>
> --
> 2.51.1
Thanks!
Takahiro
^ permalink raw reply
* Re: [PATCH v10 12/21] gpu: nova-core: mm: Add unified page table entry wrapper enums
From: Eliot Courtney @ 2026-04-02 5:40 UTC (permalink / raw)
To: Joel Fernandes, linux-kernel
Cc: Miguel Ojeda, Boqun Feng, Gary Guo, Bjorn Roy Baron, Benno Lossin,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
Dave Airlie, Daniel Almeida, Koen Koning, dri-devel,
rust-for-linux, Nikola Djukic, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Jonathan Corbet,
Alex Deucher, Christian Koenig, Jani Nikula, Joonas Lahtinen,
Rodrigo Vivi, Tvrtko Ursulin, Huang Rui, Matthew Auld,
Matthew Brost, Lucas De Marchi, Thomas Hellstrom, Helge Deller,
Alex Gaynor, Boqun Feng, John Hubbard, Alistair Popple,
Timur Tabi, Edwin Peer, Alexandre Courbot, Andrea Righi,
Andy Ritger, Zhi Wang, Balbir Singh, Philipp Stanner,
Elle Rhumsaa, alexeyi, Eliot Courtney, joel, linux-doc, amd-gfx,
intel-gfx, intel-xe, linux-fbdev
In-Reply-To: <20260331212048.2229260-13-joelagnelf@nvidia.com>
On Wed Apr 1, 2026 at 6:20 AM JST, Joel Fernandes wrote:
> Add unified Pte, Pde, and DualPde wrapper enums that abstract over
> MMU v2 and v3 page table entry formats. These enums allow the page
> table walker and VMM to work with both MMU versions.
>
> Each unified type:
> - Takes MmuVersion parameter in constructors
> - Wraps both ver2 and ver3 variants
> - Delegates method calls to the appropriate variant
>
> This enables version-agnostic page table operations while keeping
> version-specific implementation details encapsulated in the ver2
> and ver3 modules.
>
> Cc: Nikola Djukic <ndjukic@nvidia.com>
> Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
> ---
> drivers/gpu/nova-core/mm/pagetable.rs | 330 ++++++++++++++++++++++++++
> 1 file changed, 330 insertions(+)
>
> diff --git a/drivers/gpu/nova-core/mm/pagetable.rs b/drivers/gpu/nova-core/mm/pagetable.rs
> index 6e01a1af5222..909df37c3ee8 100644
> --- a/drivers/gpu/nova-core/mm/pagetable.rs
> +++ b/drivers/gpu/nova-core/mm/pagetable.rs
> @@ -12,6 +12,13 @@
> pub(crate) mod ver3;
>
> use crate::gpu::Architecture;
> +use crate::mm::{
> + pramin,
> + Pfn,
> + VirtualAddress,
> + VramAddress, //
> +};
> +use kernel::prelude::*;
>
> /// Extracts the page table index at a given level from a virtual address.
> pub(crate) trait VaLevelIndex {
> @@ -84,6 +91,96 @@ pub(crate) const fn as_index(&self) -> u64 {
> }
> }
>
> +impl MmuVersion {
> + /// Get the `PDE` levels (excluding PTE level) for page table walking.
> + pub(crate) fn pde_levels(&self) -> &'static [PageTableLevel] {
> + match self {
> + Self::V2 => ver2::PDE_LEVELS,
> + Self::V3 => ver3::PDE_LEVELS,
> + }
> + }
> +
> + /// Get the PTE level for this MMU version.
> + pub(crate) fn pte_level(&self) -> PageTableLevel {
> + match self {
> + Self::V2 => ver2::PTE_LEVEL,
> + Self::V3 => ver3::PTE_LEVEL,
> + }
> + }
> +
> + /// Get the dual PDE level (128-bit entries) for this MMU version.
> + pub(crate) fn dual_pde_level(&self) -> PageTableLevel {
> + match self {
> + Self::V2 => ver2::DUAL_PDE_LEVEL,
> + Self::V3 => ver3::DUAL_PDE_LEVEL,
> + }
> + }
> +
> + /// Get the number of PDE levels for this MMU version.
> + pub(crate) fn pde_level_count(&self) -> usize {
> + self.pde_levels().len()
> + }
> +
> + /// Get the entry size in bytes for a given level.
> + pub(crate) fn entry_size(&self, level: PageTableLevel) -> usize {
> + if level == self.dual_pde_level() {
> + 16 // 128-bit dual PDE
> + } else {
> + 8 // 64-bit PDE/PTE
> + }
> + }
> +
> + /// Get the number of entries per page table page for a given level.
> + pub(crate) fn entries_per_page(&self, level: PageTableLevel) -> usize {
> + match self {
> + Self::V2 => match level {
> + // TODO: Calculate these values from the bitfield dynamically
> + // instead of hardcoding them.
> + PageTableLevel::Pdb => 4, // PD3 root: bits [48:47] = 2 bits
> + PageTableLevel::L3 => 256, // PD0 dual: bits [28:21] = 8 bits
> + _ => 512, // PD2, PD1, PT: 9 bits each
> + },
> + Self::V3 => match level {
> + PageTableLevel::Pdb => 2, // PDE4 root: bit [56] = 1 bit, 2 entries
> + PageTableLevel::L4 => 256, // PDE0 dual: bits [28:21] = 8 bits
> + _ => 512, // PDE3, PDE2, PDE1, PT: 9 bits each
> + },
> + }
> + }
> +
> + /// Extract the page table index at `level` from `va` for this MMU version.
> + pub(crate) fn level_index(&self, va: VirtualAddress, level: u64) -> u64 {
> + match self {
> + Self::V2 => ver2::VirtualAddressV2::new(va).level_index(level),
> + Self::V3 => ver3::VirtualAddressV3::new(va).level_index(level),
> + }
> + }
> +
> + /// Compute upper bound on page table pages needed for `num_virt_pages`.
> + ///
> + /// Walks from PTE level up through PDE levels, accumulating the tree.
> + pub(crate) fn pt_pages_upper_bound(&self, num_virt_pages: usize) -> usize {
> + let mut total = 0;
> +
> + // PTE pages at the leaf level.
> + let pte_epp = self.entries_per_page(self.pte_level());
> + let mut pages_at_level = num_virt_pages.div_ceil(pte_epp);
> + total += pages_at_level;
> +
> + // Walk PDE levels bottom-up (reverse of pde_levels()).
> + for &level in self.pde_levels().iter().rev() {
> + let epp = self.entries_per_page(level);
> +
> + // How many pages at this level do we need to point to
> + // the previous pages_at_level?
> + pages_at_level = pages_at_level.div_ceil(epp);
> + total += pages_at_level;
> + }
> +
> + total
> + }
> +}
> +
We have a lot of matches on the MMU version here (and below in Pte, Pde,
DualPde). What about making MmuVersion into a trait (e.g. Mmu) with
associated types for Pte, Pde, DualPde which can implement traits
defining their common operations too? Then you can parameterise
Vmm/PtWalk on this type.
^ permalink raw reply
* Re: [PATCH 5/5] types: Add standard __ob_trap and __ob_wrap scalar types
From: Peter Zijlstra @ 2026-04-02 5:38 UTC (permalink / raw)
To: Kees Cook
Cc: Linus Torvalds, Justin Stitt, Miguel Ojeda, Nathan Chancellor,
Andrew Morton, Andy Shevchenko, Arnd Bergmann, Mark Rutland,
Matthew Wilcox (Oracle), Suren Baghdasaryan, Thomas Gleixner,
Finn Thain, Geert Uytterhoeven, Thomas Weißschuh, llvm,
Marco Elver, Jonathan Corbet, Nicolas Schier, Greg Kroah-Hartman,
linux-kernel, kasan-dev, linux-hardening, linux-doc, linux-kbuild
In-Reply-To: <202604011328.D3821379@keescook>
On Wed, Apr 01, 2026 at 01:52:26PM -0700, Kees Cook wrote:
> (Though I would note that GCC does _not_ refuse the jump when there is a
> cleanup; it only see the other two uninitialized values.)
Yeah.. I know, but since we also build with clang, any such issue will
get discovered.
> So that makes it not totally broken, but it does make it a bit fragile.
Right.
> Another concern I have is dealing with older compilers and how to
> "hide" the label and its code. e.g. if I remove the "goto" from above:
>
> ../drivers/misc/lkdtm/bugs.c:1060:1: warning: label 'weird' defined but not used [-Wunused-label]
> 1060 | weird:
> | ^~~~~
>
> Oddly, the unreachable code isn't a problem, so we could just wrap the
> label is some macro like:
>
> #define force_label(x) if (0) goto x; x
>
> force_label(weird):
> pr_info("value: %lu\n", value);
> pr_info("outcome: %zu\n", outcome);
>
__maybe_unused also works on labels. Like:
__overflow: __maybe_unused
dead-code-here;
^ permalink raw reply
* Re: [PATCH v10 03/21] gpu: nova-core: gsp: Expose total physical VRAM end from FB region info
From: Eliot Courtney @ 2026-04-02 5:37 UTC (permalink / raw)
To: Joel Fernandes, linux-kernel
Cc: Miguel Ojeda, Boqun Feng, Gary Guo, Bjorn Roy Baron, Benno Lossin,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
Dave Airlie, Daniel Almeida, Koen Koning, dri-devel,
rust-for-linux, Nikola Djukic, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Jonathan Corbet,
Alex Deucher, Christian Koenig, Jani Nikula, Joonas Lahtinen,
Rodrigo Vivi, Tvrtko Ursulin, Huang Rui, Matthew Auld,
Matthew Brost, Lucas De Marchi, Thomas Hellstrom, Helge Deller,
Alex Gaynor, Boqun Feng, John Hubbard, Alistair Popple,
Timur Tabi, Edwin Peer, Alexandre Courbot, Andrea Righi,
Andy Ritger, Zhi Wang, Balbir Singh, Philipp Stanner,
Elle Rhumsaa, alexeyi, Eliot Courtney, joel, linux-doc, amd-gfx,
intel-gfx, intel-xe, linux-fbdev
In-Reply-To: <20260331212048.2229260-4-joelagnelf@nvidia.com>
On Wed Apr 1, 2026 at 6:20 AM JST, Joel Fernandes wrote:
> Add `total_fb_end()` to `GspStaticConfigInfo` that computes the
> exclusive end address of the highest valid FB region covering both
> usable and GSP-reserved areas.
>
> This allows callers to know the full physical VRAM extent, not just
> the allocatable portion.
>
> Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
> ---
> drivers/gpu/nova-core/gsp/commands.rs | 6 ++++++
> drivers/gpu/nova-core/gsp/fw/commands.rs | 7 +++++++
> 2 files changed, 13 insertions(+)
>
> diff --git a/drivers/gpu/nova-core/gsp/commands.rs b/drivers/gpu/nova-core/gsp/commands.rs
> index 41742c1633c8..5e0649024637 100644
> --- a/drivers/gpu/nova-core/gsp/commands.rs
> +++ b/drivers/gpu/nova-core/gsp/commands.rs
> @@ -196,6 +196,9 @@ pub(crate) struct GetGspStaticInfoReply {
> /// Usable FB (VRAM) region for driver memory allocation.
> #[expect(dead_code)]
> pub(crate) usable_fb_region: Range<u64>,
> + /// End of VRAM.
> + #[expect(dead_code)]
> + pub(crate) total_fb_end: u64,
> }
>
> impl MessageFromGsp for GetGspStaticInfoReply {
> @@ -209,9 +212,12 @@ fn read(
> ) -> Result<Self, Self::InitError> {
> let (base, size) = msg.first_usable_fb_region().ok_or(ENODEV)?;
>
> + let total_fb_end = msg.total_fb_end().ok_or(ENODEV)?;
> +
> Ok(GetGspStaticInfoReply {
> gpu_name: msg.gpu_name_str(),
> usable_fb_region: base..base.saturating_add(size),
> + total_fb_end,
> })
> }
> }
> diff --git a/drivers/gpu/nova-core/gsp/fw/commands.rs b/drivers/gpu/nova-core/gsp/fw/commands.rs
> index 9fffa74d03f9..46932d5c8c1d 100644
> --- a/drivers/gpu/nova-core/gsp/fw/commands.rs
> +++ b/drivers/gpu/nova-core/gsp/fw/commands.rs
> @@ -163,6 +163,13 @@ pub(crate) fn first_usable_fb_region(&self) -> Option<(u64, u64)> {
> }
> })
> }
> +
> + /// Compute the end of physical VRAM from all FB regions.
> + pub(crate) fn total_fb_end(&self) -> Option<u64> {
> + self.fb_regions()
> + .map(|reg| reg.limit.saturating_add(1))
I think it would be better to used checked_add here.
> + .max()
> + }
> }
>
> // SAFETY: Padding is explicit and will not contain uninitialized data.
^ permalink raw reply
* Re: [PATCH v3 0/5] Support the FEAT_HDBSS introduced in Armv9.5
From: Tian Zheng @ 2026-04-02 2:40 UTC (permalink / raw)
To: Leonardo Bras
Cc: maz, oupton, catalin.marinas, corbet, pbonzini, will, yuzenghui,
wangzhou1, liuyonglong, Jonathan.Cameron, yezhenyu2, linuxarm,
joey.gouly, kvmarm, kvm, linux-arm-kernel, linux-doc,
linux-kernel, skhan, suzuki.poulose
In-Reply-To: <acvWnjG0AZBHL21q@devkitleo>
On 3/31/2026 10:13 PM, Leonardo Bras wrote:
> On Wed, Feb 25, 2026 at 12:04:16PM +0800, Tian Zheng wrote:
>> This series of patches add support to the Hardware Dirty state tracking
>> Structure(HDBSS) feature, which is introduced by the ARM architecture
>> in the DDI0601(ID121123) version.
>>
>> The HDBSS feature is an extension to the architecture that enhances
>> tracking translation table descriptors' dirty state, identified as
>> FEAT_HDBSS. This feature utilizes hardware assistance to achieve dirty
>> page tracking, aiming to significantly reduce the overhead of scanning
>> for dirty pages.
>>
>> The purpose of this feature is to make the execution overhead of live
>> migration lower to both the guest and the host, compared to existing
>> approaches (write-protect or search stage 2 tables).
>>
>> After these patches, users(such as qemu) can use the
>> KVM_CAP_ARM_HW_DIRTY_STATE_TRACK ioctl to enable or disable the HDBSS
>> feature before and after the live migration.
>>
>> v2:
>> https://lore.kernel.org/linux-arm-kernel/20251121092342.3393318-1-zhengtian10@huawei.com/
>>
>> v2->v3 changes:
>> - Remove the ARM64_HDBSS configuration option and ensure this feature
>> is only enabled in VHE mode.
>> - Move HDBSS-related variables to the arch-independent portion of the
>> kvm structure.
>> - Remove error messages during HDBSS enable/disable operations
>> - Change HDBSS buffer flushing from handle_exit to vcpu_put,
>> check_vcpu_requests, and kvm_handle_guest_abort.
>> - Add fault handling for HDBSS including buffer full, external abort,
>> and general protection fault (GPF).
>> - Add support for a 4KB HDBSS buffer size, mapped to the value 0b0000.
>> - Add a second argument to the ioctl to turn HDBSS on or off.
>>
>> Tian Zheng (1):
>> KVM: arm64: Document HDBSS ioctl
>>
>> eillon (4):
>> arm64/sysreg: Add HDBSS related register information
>> KVM: arm64: Add support to set the DBM attr during memory abort
>> KVM: arm64: Add support for FEAT_HDBSS
>> KVM: arm64: Enable HDBSS support and handle HDBSSF events
>>
>> Documentation/virt/kvm/api.rst | 16 +++++
>> arch/arm64/include/asm/cpufeature.h | 5 ++
>> arch/arm64/include/asm/esr.h | 7 ++
>> arch/arm64/include/asm/kvm_host.h | 17 +++++
>> arch/arm64/include/asm/kvm_mmu.h | 1 +
>> arch/arm64/include/asm/kvm_pgtable.h | 4 ++
>> arch/arm64/include/asm/sysreg.h | 11 +++
>> arch/arm64/kernel/cpufeature.c | 12 ++++
>> arch/arm64/kvm/arm.c | 102 +++++++++++++++++++++++++++
>> arch/arm64/kvm/hyp/pgtable.c | 6 ++
>> arch/arm64/kvm/hyp/vhe/switch.c | 19 +++++
>> arch/arm64/kvm/mmu.c | 70 ++++++++++++++++++
>> arch/arm64/kvm/reset.c | 3 +
>> arch/arm64/tools/cpucaps | 1 +
>> arch/arm64/tools/sysreg | 29 ++++++++
>> include/uapi/linux/kvm.h | 1 +
>> tools/include/uapi/linux/kvm.h | 1 +
>> 17 files changed, 305 insertions(+)
>>
>> --
>> 2.33.0
>>
>
> Hi Tian,
>
> I was thinking: maybe instead of putting the HDBSS (and HACDBS) stuff
> across a bunch of KVM files, we should try to focus them all on a single
> arch/arm64/kvm/dirty_bit.c file (plus a header such as
> arch/arm64/include/asm/kvm_dirty_bit.h).
>
> What is your opinion on that?
>
> Thanks!
> Leo
Sorry for the late reply. Yes, I had the same thought before. In the
next version, I will
move all the HDBSS-related content into the same file, such as
arch/arm64/kvm/dirty_bit.c
and arch/arm64/include/asm/kvm_dirty_bit.h.
Tian
>
>
^ permalink raw reply
* [PATCH v3 11/11] HID: spi-hid: add panel follower support
From: Jingyuan Liang @ 2026-04-02 1:59 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Jonathan Corbet, Mark Brown,
Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: linux-input, linux-doc, linux-kernel, linux-spi,
linux-trace-kernel, devicetree, hbarnor, tfiga, Jingyuan Liang
In-Reply-To: <20260402-send-upstream-v3-0-6091c458d357@chromium.org>
Add support to spi-hid to be a panel follower.
Signed-off-by: Jingyuan Liang <jingyliang@chromium.org>
---
drivers/hid/spi-hid/spi-hid-core.c | 199 +++++++++++++++++++++++++++++--------
drivers/hid/spi-hid/spi-hid-core.h | 7 ++
2 files changed, 163 insertions(+), 43 deletions(-)
diff --git a/drivers/hid/spi-hid/spi-hid-core.c b/drivers/hid/spi-hid/spi-hid-core.c
index 5f7a5bb692d9..9eedd4f1cba7 100644
--- a/drivers/hid/spi-hid/spi-hid-core.c
+++ b/drivers/hid/spi-hid/spi-hid-core.c
@@ -246,21 +246,21 @@ static const char *spi_hid_power_mode_string(enum hidspi_power_state power_state
}
}
-static void spi_hid_suspend(struct spi_hid *shid)
+static int spi_hid_suspend(struct spi_hid *shid)
{
int error;
struct device *dev = &shid->spi->dev;
guard(mutex)(&shid->power_lock);
if (shid->power_state == HIDSPI_OFF)
- return;
+ return 0;
if (shid->hid) {
error = hid_driver_suspend(shid->hid, PMSG_SUSPEND);
if (error) {
dev_err(dev, "%s failed to suspend hid driver: %d",
__func__, error);
- return;
+ return error;
}
}
@@ -278,21 +278,22 @@ static void spi_hid_suspend(struct spi_hid *shid)
dev_err(dev, "%s: could not power down.", __func__);
shid->regulator_error_count++;
shid->regulator_last_error = error;
- return;
+ return error;
}
shid->power_state = HIDSPI_OFF;
}
+ return 0;
}
-static void spi_hid_resume(struct spi_hid *shid)
+static int spi_hid_resume(struct spi_hid *shid)
{
int error;
struct device *dev = &shid->spi->dev;
guard(mutex)(&shid->power_lock);
if (shid->power_state == HIDSPI_ON)
- return;
+ return 0;
enable_irq(shid->spi->irq);
@@ -306,7 +307,7 @@ static void spi_hid_resume(struct spi_hid *shid)
dev_err(dev, "%s: could not power up.", __func__);
shid->regulator_error_count++;
shid->regulator_last_error = error;
- return;
+ return error;
}
shid->power_state = HIDSPI_ON;
@@ -315,10 +316,13 @@ static void spi_hid_resume(struct spi_hid *shid)
if (shid->hid) {
error = hid_driver_reset_resume(shid->hid);
- if (error)
+ if (error) {
dev_err(dev, "%s: failed to reset resume hid driver: %d.",
__func__, error);
+ return error;
+ }
}
+ return 0;
}
static void spi_hid_stop_hid(struct spi_hid *shid)
@@ -1215,6 +1219,132 @@ const struct attribute_group *spi_hid_groups[] = {
};
EXPORT_SYMBOL_GPL(spi_hid_groups);
+/*
+ * At the end of probe we initialize the device:
+ * 0) assert reset, bias the interrupt line
+ * 1) sleep minimal reset delay
+ * 2) request IRQ
+ * 3) power up the device
+ * 4) deassert reset (high)
+ * After this we expect an IRQ with a reset response.
+ */
+static int spi_hid_dev_init(struct spi_hid *shid)
+{
+ struct spi_device *spi = shid->spi;
+ struct device *dev = &spi->dev;
+ int error;
+
+ shid->ops->assert_reset(shid->ops);
+
+ shid->ops->sleep_minimal_reset_delay(shid->ops);
+
+ error = devm_request_threaded_irq(dev, spi->irq, NULL, spi_hid_dev_irq,
+ IRQF_ONESHOT, dev_name(&spi->dev), shid);
+ if (error) {
+ dev_err(dev, "%s: unable to request threaded IRQ.", __func__);
+ return error;
+ }
+ if (device_may_wakeup(dev)) {
+ error = dev_pm_set_wake_irq(dev, spi->irq);
+ if (error) {
+ dev_err(dev, "%s: failed to set wake IRQ.", __func__);
+ return error;
+ }
+ }
+
+ error = shid->ops->power_up(shid->ops);
+ if (error) {
+ dev_err(dev, "%s: could not power up.", __func__);
+ shid->regulator_error_count++;
+ shid->regulator_last_error = error;
+ return error;
+ }
+
+ shid->ops->deassert_reset(shid->ops);
+
+ return 0;
+}
+
+static void spi_hid_panel_follower_work(struct work_struct *work)
+{
+ struct spi_hid *shid = container_of(work, struct spi_hid,
+ panel_follower_work);
+ int error;
+
+ if (!shid->desc.hid_version)
+ error = spi_hid_dev_init(shid);
+ else
+ error = spi_hid_resume(shid);
+ if (error)
+ dev_warn(&shid->spi->dev, "Power on failed: %d", error);
+ else
+ WRITE_ONCE(shid->panel_follower_work_finished, true);
+
+ /*
+ * The work APIs provide a number of memory ordering guarantees
+ * including one that says that memory writes before schedule_work()
+ * are always visible to the work function, but they don't appear to
+ * guarantee that a write that happened in the work is visible after
+ * cancel_work_sync(). We'll add a write memory barrier here to match
+ * with spi_hid_panel_unpreparing() to ensure that our write to
+ * panel_follower_work_finished is visible there.
+ */
+ smp_wmb();
+}
+
+static int spi_hid_panel_follower_resume(struct drm_panel_follower *follower)
+{
+ struct spi_hid *shid = container_of(follower, struct spi_hid, panel_follower);
+
+ /*
+ * Powering on a touchscreen can be a slow process. Queue the work to
+ * the system workqueue so we don't block the panel's power up.
+ */
+ WRITE_ONCE(shid->panel_follower_work_finished, false);
+ schedule_work(&shid->panel_follower_work);
+
+ return 0;
+}
+
+static int spi_hid_panel_follower_suspend(struct drm_panel_follower *follower)
+{
+ struct spi_hid *shid = container_of(follower, struct spi_hid, panel_follower);
+
+ cancel_work_sync(&shid->panel_follower_work);
+
+ /* Match with shid_core_panel_follower_work() */
+ smp_rmb();
+ if (!READ_ONCE(shid->panel_follower_work_finished))
+ return 0;
+
+ return spi_hid_suspend(shid);
+}
+
+static const struct drm_panel_follower_funcs
+ spi_hid_panel_follower_prepare_funcs = {
+ .panel_prepared = spi_hid_panel_follower_resume,
+ .panel_unpreparing = spi_hid_panel_follower_suspend,
+};
+
+static int spi_hid_register_panel_follower(struct spi_hid *shid)
+{
+ struct device *dev = &shid->spi->dev;
+
+ shid->panel_follower.funcs = &spi_hid_panel_follower_prepare_funcs;
+
+ /*
+ * If we're not in control of our own power up/power down then we can't
+ * do the logic to manage wakeups. Give a warning if a user thought
+ * that was possible then force the capability off.
+ */
+ if (device_can_wakeup(dev)) {
+ dev_warn(dev, "Can't wakeup if following panel\n");
+ device_set_wakeup_capable(dev, false);
+ }
+
+ return drm_panel_add_follower(dev, &shid->panel_follower);
+}
+
int spi_hid_core_probe(struct spi_device *spi, struct spihid_ops *ops,
struct spi_hid_conf *conf)
{
@@ -1234,6 +1364,7 @@ int spi_hid_core_probe(struct spi_device *spi, struct spihid_ops *ops,
shid->ops = ops;
shid->conf = conf;
set_bit(SPI_HID_RESET_PENDING, &shid->flags);
+ shid->is_panel_follower = drm_is_panel_follower(&spi->dev);
spi_set_drvdata(spi, shid);
@@ -1247,6 +1378,7 @@ int spi_hid_core_probe(struct spi_device *spi, struct spihid_ops *ops,
init_completion(&shid->output_done);
INIT_WORK(&shid->reset_work, spi_hid_reset_work);
+ INIT_WORK(&shid->panel_follower_work, spi_hid_panel_follower_work);
/*
* We need to allocate the buffer without knowing the maximum
@@ -1257,42 +1389,18 @@ int spi_hid_core_probe(struct spi_device *spi, struct spihid_ops *ops,
if (error)
return error;
- /*
- * At the end of probe we initialize the device:
- * 0) assert reset, bias the interrupt line
- * 1) sleep minimal reset delay
- * 2) request IRQ
- * 3) power up the device
- * 4) deassert reset (high)
- * After this we expect an IRQ with a reset response.
- */
-
- shid->ops->assert_reset(shid->ops);
-
- shid->ops->sleep_minimal_reset_delay(shid->ops);
-
- error = devm_request_threaded_irq(dev, spi->irq, NULL, spi_hid_dev_irq,
- IRQF_ONESHOT, dev_name(&spi->dev), shid);
- if (error) {
- dev_err(dev, "%s: unable to request threaded IRQ.", __func__);
- return error;
- }
- if (device_may_wakeup(dev)) {
- error = dev_pm_set_wake_irq(dev, spi->irq);
+ if (shid->is_panel_follower) {
+ error = spi_hid_register_panel_follower(shid);
if (error) {
- dev_err(dev, "%s: failed to set wake IRQ.", __func__);
+ dev_err(dev, "%s: could not add panel follower.", __func__);
return error;
}
+ } else {
+ error = spi_hid_dev_init(shid);
+ if (error)
+ return error;
}
- error = shid->ops->power_up(shid->ops);
- if (error) {
- dev_err(dev, "%s: could not power up.", __func__);
- return error;
- }
-
- shid->ops->deassert_reset(shid->ops);
-
dev_dbg(dev, "%s: d3 -> %s.", __func__,
spi_hid_power_mode_string(shid->power_state));
@@ -1306,6 +1414,9 @@ void spi_hid_core_remove(struct spi_device *spi)
struct device *dev = &spi->dev;
int error;
+ if (shid->is_panel_follower)
+ drm_panel_remove_follower(&shid->panel_follower);
+
spi_hid_stop_hid(shid);
shid->ops->assert_reset(shid->ops);
@@ -1319,18 +1430,20 @@ static int spi_hid_core_pm_suspend(struct device *dev)
{
struct spi_hid *shid = dev_get_drvdata(dev);
- spi_hid_suspend(shid);
+ if (shid->is_panel_follower)
+ return 0;
- return 0;
+ return spi_hid_suspend(shid);
}
static int spi_hid_core_pm_resume(struct device *dev)
{
struct spi_hid *shid = dev_get_drvdata(dev);
- spi_hid_resume(shid);
+ if (shid->is_panel_follower)
+ return 0;
- return 0;
+ return spi_hid_resume(shid);
}
const struct dev_pm_ops spi_hid_core_pm = {
diff --git a/drivers/hid/spi-hid/spi-hid-core.h b/drivers/hid/spi-hid/spi-hid-core.h
index 293e2cfcfbf7..261b2fd7f332 100644
--- a/drivers/hid/spi-hid/spi-hid-core.h
+++ b/drivers/hid/spi-hid/spi-hid-core.h
@@ -10,6 +10,8 @@
#include <linux/hid-over-spi.h>
#include <linux/spi/spi.h>
+#include <drm/drm_panel.h>
+
/* Protocol message size constants */
#define SPI_HID_READ_APPROVAL_LEN 5
#define SPI_HID_OUTPUT_HEADER_LEN 8
@@ -56,6 +58,10 @@ struct spi_hid {
struct spi_hid_input_buf *input; /* Input buffer. */
struct spi_hid_input_buf *response; /* Response buffer. */
+ struct drm_panel_follower panel_follower;
+ bool is_panel_follower;
+ bool panel_follower_work_finished;
+
u16 response_length;
u16 bufsize;
@@ -66,6 +72,7 @@ struct spi_hid {
unsigned long flags; /* device flags. */
struct work_struct reset_work;
+ struct work_struct panel_follower_work;
/* Control lock to ensure complete output transaction. */
struct mutex output_lock;
--
2.53.0.1185.g05d4b7b318-goog
^ permalink raw reply related
* [PATCH v3 10/11] HID: spi-hid: add power management implementation
From: Jingyuan Liang @ 2026-04-02 1:59 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Jonathan Corbet, Mark Brown,
Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: linux-input, linux-doc, linux-kernel, linux-spi,
linux-trace-kernel, devicetree, hbarnor, tfiga, Jingyuan Liang
In-Reply-To: <20260402-send-upstream-v3-0-6091c458d357@chromium.org>
Implement HID over SPI driver power management callbacks.
Signed-off-by: Jingyuan Liang <jingyliang@chromium.org>
---
drivers/hid/spi-hid/spi-hid-acpi.c | 1 +
drivers/hid/spi-hid/spi-hid-core.c | 107 +++++++++++++++++++++++++++++++++++++
drivers/hid/spi-hid/spi-hid-of.c | 1 +
drivers/hid/spi-hid/spi-hid.h | 1 +
4 files changed, 110 insertions(+)
diff --git a/drivers/hid/spi-hid/spi-hid-acpi.c b/drivers/hid/spi-hid/spi-hid-acpi.c
index 298e3ba44d8a..15cfc4e6cc2f 100644
--- a/drivers/hid/spi-hid/spi-hid-acpi.c
+++ b/drivers/hid/spi-hid/spi-hid-acpi.c
@@ -238,6 +238,7 @@ static struct spi_driver spi_hid_acpi_driver = {
.driver = {
.name = "spi_hid_acpi",
.owner = THIS_MODULE,
+ .pm = &spi_hid_core_pm,
.acpi_match_table = spi_hid_acpi_match,
.probe_type = PROBE_PREFER_ASYNCHRONOUS,
.dev_groups = spi_hid_groups,
diff --git a/drivers/hid/spi-hid/spi-hid-core.c b/drivers/hid/spi-hid/spi-hid-core.c
index d48175c764b9..5f7a5bb692d9 100644
--- a/drivers/hid/spi-hid/spi-hid-core.c
+++ b/drivers/hid/spi-hid/spi-hid-core.c
@@ -35,6 +35,8 @@
#include <linux/list.h>
#include <linux/module.h>
#include <linux/mutex.h>
+#include <linux/pm.h>
+#include <linux/pm_wakeirq.h>
#include <linux/slab.h>
#include <linux/spi/spi.h>
#include <linux/string.h>
@@ -244,6 +246,81 @@ static const char *spi_hid_power_mode_string(enum hidspi_power_state power_state
}
}
+static void spi_hid_suspend(struct spi_hid *shid)
+{
+ int error;
+ struct device *dev = &shid->spi->dev;
+
+ guard(mutex)(&shid->power_lock);
+ if (shid->power_state == HIDSPI_OFF)
+ return;
+
+ if (shid->hid) {
+ error = hid_driver_suspend(shid->hid, PMSG_SUSPEND);
+ if (error) {
+ dev_err(dev, "%s failed to suspend hid driver: %d",
+ __func__, error);
+ return;
+ }
+ }
+
+ disable_irq(shid->spi->irq);
+
+ clear_bit(SPI_HID_READY, &shid->flags);
+
+ if (!device_may_wakeup(dev)) {
+ set_bit(SPI_HID_RESET_PENDING, &shid->flags);
+
+ shid->ops->assert_reset(shid->ops);
+
+ error = shid->ops->power_down(shid->ops);
+ if (error) {
+ dev_err(dev, "%s: could not power down.", __func__);
+ shid->regulator_error_count++;
+ shid->regulator_last_error = error;
+ return;
+ }
+
+ shid->power_state = HIDSPI_OFF;
+ }
+}
+
+static void spi_hid_resume(struct spi_hid *shid)
+{
+ int error;
+ struct device *dev = &shid->spi->dev;
+
+ guard(mutex)(&shid->power_lock);
+ if (shid->power_state == HIDSPI_ON)
+ return;
+
+ enable_irq(shid->spi->irq);
+
+ if (!device_may_wakeup(dev)) {
+ shid->ops->assert_reset(shid->ops);
+
+ shid->ops->sleep_minimal_reset_delay(shid->ops);
+
+ error = shid->ops->power_up(shid->ops);
+ if (error) {
+ dev_err(dev, "%s: could not power up.", __func__);
+ shid->regulator_error_count++;
+ shid->regulator_last_error = error;
+ return;
+ }
+ shid->power_state = HIDSPI_ON;
+
+ shid->ops->deassert_reset(shid->ops);
+ }
+
+ if (shid->hid) {
+ error = hid_driver_reset_resume(shid->hid);
+ if (error)
+ dev_err(dev, "%s: failed to reset resume hid driver: %d.",
+ __func__, error);
+ }
+}
+
static void spi_hid_stop_hid(struct spi_hid *shid)
{
struct hid_device *hid = shid->hid;
@@ -1200,6 +1277,13 @@ int spi_hid_core_probe(struct spi_device *spi, struct spihid_ops *ops,
dev_err(dev, "%s: unable to request threaded IRQ.", __func__);
return error;
}
+ if (device_may_wakeup(dev)) {
+ error = dev_pm_set_wake_irq(dev, spi->irq);
+ if (error) {
+ dev_err(dev, "%s: failed to set wake IRQ.", __func__);
+ return error;
+ }
+ }
error = shid->ops->power_up(shid->ops);
if (error) {
@@ -1231,6 +1315,29 @@ void spi_hid_core_remove(struct spi_device *spi)
}
EXPORT_SYMBOL_GPL(spi_hid_core_remove);
+static int spi_hid_core_pm_suspend(struct device *dev)
+{
+ struct spi_hid *shid = dev_get_drvdata(dev);
+
+ spi_hid_suspend(shid);
+
+ return 0;
+}
+
+static int spi_hid_core_pm_resume(struct device *dev)
+{
+ struct spi_hid *shid = dev_get_drvdata(dev);
+
+ spi_hid_resume(shid);
+
+ return 0;
+}
+
+const struct dev_pm_ops spi_hid_core_pm = {
+ SYSTEM_SLEEP_PM_OPS(spi_hid_core_pm_suspend, spi_hid_core_pm_resume)
+};
+EXPORT_SYMBOL_GPL(spi_hid_core_pm);
+
MODULE_DESCRIPTION("HID over SPI transport driver");
MODULE_AUTHOR("Dmitry Antipov <dmanti@microsoft.com>");
MODULE_LICENSE("GPL");
diff --git a/drivers/hid/spi-hid/spi-hid-of.c b/drivers/hid/spi-hid/spi-hid-of.c
index 651456b6906d..80c481b77149 100644
--- a/drivers/hid/spi-hid/spi-hid-of.c
+++ b/drivers/hid/spi-hid/spi-hid-of.c
@@ -227,6 +227,7 @@ static struct spi_driver spi_hid_of_driver = {
.driver = {
.name = "spi_hid_of",
.owner = THIS_MODULE,
+ .pm = &spi_hid_core_pm,
.of_match_table = spi_hid_of_match,
.probe_type = PROBE_PREFER_ASYNCHRONOUS,
.dev_groups = spi_hid_groups,
diff --git a/drivers/hid/spi-hid/spi-hid.h b/drivers/hid/spi-hid/spi-hid.h
index f5a5f4d54beb..17b2fdf192ed 100644
--- a/drivers/hid/spi-hid/spi-hid.h
+++ b/drivers/hid/spi-hid/spi-hid.h
@@ -41,5 +41,6 @@ int spi_hid_core_probe(struct spi_device *spi, struct spihid_ops *ops,
void spi_hid_core_remove(struct spi_device *spi);
extern const struct attribute_group *spi_hid_groups[];
+extern const struct dev_pm_ops spi_hid_core_pm;
#endif /* SPI_HID_H */
--
2.53.0.1185.g05d4b7b318-goog
^ permalink raw reply related
* [PATCH v3 09/11] dt-bindings: input: Document hid-over-spi DT schema
From: Jingyuan Liang @ 2026-04-02 1:59 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Jonathan Corbet, Mark Brown,
Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: linux-input, linux-doc, linux-kernel, linux-spi,
linux-trace-kernel, devicetree, hbarnor, tfiga, Jingyuan Liang,
Dmitry Antipov, Jarrett Schultz
In-Reply-To: <20260402-send-upstream-v3-0-6091c458d357@chromium.org>
Documentation describes the required and optional properties for
implementing Device Tree for a Microsoft G6 Touch Digitizer that
supports HID over SPI Protocol 1.0 specification.
The properties are common to HID over SPI.
Signed-off-by: Dmitry Antipov <dmanti@microsoft.com>
Signed-off-by: Jarrett Schultz <jaschultz@microsoft.com>
Signed-off-by: Jingyuan Liang <jingyliang@chromium.org>
---
.../devicetree/bindings/input/hid-over-spi.yaml | 126 +++++++++++++++++++++
1 file changed, 126 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..d1b0a2e26c32
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/hid-over-spi.yaml
@@ -0,0 +1,126 @@
+# 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.
+
+ input-report-header-address:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ minimum: 0
+ maximum: 0xffffff
+ description:
+ A value to be included in the Read Approval packet, listing an address of
+ the input report header to be put on the SPI bus. This address has 24
+ bits.
+
+ input-report-body-address:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ minimum: 0
+ maximum: 0xffffff
+ description:
+ A value to be included in the Read Approval packet, listing an address of
+ the input report body to be put on the SPI bus. This address has 24 bits.
+
+ output-report-address:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ minimum: 0
+ maximum: 0xffffff
+ description:
+ A value to be included in the Output Report sent by the host, listing an
+ address where the output report on the SPI bus is to be written to. This
+ address has 24 bits.
+
+ read-opcode:
+ $ref: /schemas/types.yaml#/definitions/uint8
+ description:
+ Value to be used in Read Approval packets. 1 byte.
+
+ write-opcode:
+ $ref: /schemas/types.yaml#/definitions/uint8
+ description:
+ Value to be used in Write Approval packets. 1 byte.
+
+required:
+ - compatible
+ - interrupts
+ - reset-gpios
+ - vdd-supply
+ - input-report-header-address
+ - input-report-body-address
+ - output-report-address
+ - read-opcode
+ - write-opcode
+
+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 = "microsoft,g6-touch-digitizer", "hid-over-spi";
+ reg = <0x0>;
+ interrupts-extended = <&gpio 42 IRQ_TYPE_EDGE_FALLING>;
+ reset-gpios = <&gpio 27 GPIO_ACTIVE_LOW>;
+ vdd-supply = <&pm8350c_l3>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&ts_d6_int_bias>;
+ input-report-header-address = <0x1000>;
+ input-report-body-address = <0x1004>;
+ output-report-address = <0x2000>;
+ read-opcode = /bits/ 8 <0x0b>;
+ write-opcode = /bits/ 8 <0x02>;
+ };
+ };
--
2.53.0.1185.g05d4b7b318-goog
^ permalink raw reply related
* [PATCH v3 08/11] HID: spi_hid: add device tree support for SPI over HID
From: Jingyuan Liang @ 2026-04-02 1:59 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Jonathan Corbet, Mark Brown,
Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: linux-input, linux-doc, linux-kernel, linux-spi,
linux-trace-kernel, devicetree, hbarnor, tfiga, Jingyuan Liang,
Jarrett Schultz, Dmitry Antipov
In-Reply-To: <20260402-send-upstream-v3-0-6091c458d357@chromium.org>
From: Jarrett Schultz <jaschultz@microsoft.com>
Signed-off-by: Dmitry Antipov <dmanti@microsoft.com>
Signed-off-by: Jingyuan Liang <jingyliang@chromium.org>
---
drivers/hid/spi-hid/Kconfig | 15 +++
drivers/hid/spi-hid/Makefile | 1 +
drivers/hid/spi-hid/spi-hid-of.c | 243 +++++++++++++++++++++++++++++++++++++++
3 files changed, 259 insertions(+)
diff --git a/drivers/hid/spi-hid/Kconfig b/drivers/hid/spi-hid/Kconfig
index 114b1e00da39..76a2cd587a3e 100644
--- a/drivers/hid/spi-hid/Kconfig
+++ b/drivers/hid/spi-hid/Kconfig
@@ -25,6 +25,21 @@ config SPI_HID_ACPI
will be called spi-hid-acpi. It will also build/depend on the
module spi-hid.
+config SPI_HID_OF
+ tristate "HID over SPI transport layer Open Firmware driver"
+ depends on OF
+ select SPI_HID_CORE
+ help
+ Say Y here if you use a keyboard, a touchpad, a touchscreen, or any
+ other HID based devices which are connected to your computer via SPI.
+ This driver supports Open Firmware (Device Tree)-based systems.
+
+ If unsure, say N.
+
+ This support is also available as a module. If so, the module
+ will be called spi-hid-of. It will also build/depend on the
+ module spi-hid.
+
config SPI_HID_CORE
tristate
endif
diff --git a/drivers/hid/spi-hid/Makefile b/drivers/hid/spi-hid/Makefile
index 3ca326602643..31192e71edae 100644
--- a/drivers/hid/spi-hid/Makefile
+++ b/drivers/hid/spi-hid/Makefile
@@ -9,3 +9,4 @@ obj-$(CONFIG_SPI_HID_CORE) += spi-hid.o
spi-hid-objs = spi-hid-core.o
CFLAGS_spi-hid-core.o := -I$(src)
obj-$(CONFIG_SPI_HID_ACPI) += spi-hid-acpi.o
+obj-$(CONFIG_SPI_HID_OF) += spi-hid-of.o
diff --git a/drivers/hid/spi-hid/spi-hid-of.c b/drivers/hid/spi-hid/spi-hid-of.c
new file mode 100644
index 000000000000..651456b6906d
--- /dev/null
+++ b/drivers/hid/spi-hid/spi-hid-of.c
@@ -0,0 +1,243 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * HID over SPI protocol, Open Firmware related code
+ *
+ * Copyright (c) 2021 Microsoft Corporation
+ *
+ * This code was forked out of the HID over SPI core code, which is partially
+ * based on "HID over I2C protocol implementation:
+ *
+ * Copyright (c) 2012 Benjamin Tissoires <benjamin.tissoires@gmail.com>
+ * Copyright (c) 2012 Ecole Nationale de l'Aviation Civile, France
+ * Copyright (c) 2012 Red Hat, Inc
+ *
+ * which in turn is partially based on "USB HID support for Linux":
+ *
+ * Copyright (c) 1999 Andreas Gal
+ * Copyright (c) 2000-2005 Vojtech Pavlik <vojtech@suse.cz>
+ * Copyright (c) 2005 Michael Haboustak <mike-@cinci.rr.com> for Concept2, Inc
+ * Copyright (c) 2007-2008 Oliver Neukum
+ * Copyright (c) 2006-2010 Jiri Kosina
+ */
+
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/regulator/consumer.h>
+#include <linux/gpio/consumer.h>
+#include <linux/delay.h>
+
+#include "spi-hid.h"
+
+struct spi_hid_timing_data {
+ u32 post_power_on_delay_ms;
+ u32 minimal_reset_delay_ms;
+};
+
+/* Config structure is filled with data from Device Tree */
+struct spi_hid_of_config {
+ struct spihid_ops ops;
+
+ struct spi_hid_conf property_conf;
+ const struct spi_hid_timing_data *timing_data;
+
+ struct gpio_desc *reset_gpio;
+ struct regulator *supply;
+ bool supply_enabled;
+ u16 hid_over_spi_flags;
+};
+
+static int spi_hid_of_populate_config(struct spi_hid_of_config *conf,
+ struct device *dev)
+{
+ int error;
+ u32 val;
+
+ error = device_property_read_u32(dev, "input-report-header-address",
+ &val);
+ if (error) {
+ dev_err(dev, "Input report header address not provided.");
+ return -ENODEV;
+ }
+ conf->property_conf.input_report_header_address = val;
+
+ error = device_property_read_u32(dev, "input-report-body-address", &val);
+ if (error) {
+ dev_err(dev, "Input report body address not provided.");
+ return -ENODEV;
+ }
+ conf->property_conf.input_report_body_address = val;
+
+ error = device_property_read_u32(dev, "output-report-address", &val);
+ if (error) {
+ dev_err(dev, "Output report address not provided.");
+ return -ENODEV;
+ }
+ conf->property_conf.output_report_address = val;
+
+ error = device_property_read_u32(dev, "read-opcode", &val);
+ if (error) {
+ dev_err(dev, "Read opcode not provided.");
+ return -ENODEV;
+ }
+ conf->property_conf.read_opcode = val;
+
+ error = device_property_read_u32(dev, "write-opcode", &val);
+ if (error) {
+ dev_err(dev, "Write opcode not provided.");
+ return -ENODEV;
+ }
+ conf->property_conf.write_opcode = val;
+
+ conf->supply = devm_regulator_get(dev, "vdd");
+ if (IS_ERR(conf->supply)) {
+ if (PTR_ERR(conf->supply) != -EPROBE_DEFER)
+ dev_err(dev, "Failed to get regulator: %ld.",
+ PTR_ERR(conf->supply));
+ return PTR_ERR(conf->supply);
+ }
+ conf->supply_enabled = false;
+
+ conf->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
+ if (IS_ERR(conf->reset_gpio)) {
+ dev_err(dev, "%s: error getting reset GPIO.", __func__);
+ return PTR_ERR(conf->reset_gpio);
+ }
+
+ return 0;
+}
+
+static int spi_hid_of_power_down(struct spihid_ops *ops)
+{
+ struct spi_hid_of_config *conf = container_of(ops,
+ struct spi_hid_of_config,
+ ops);
+ int error;
+
+ if (!conf->supply_enabled)
+ return 0;
+
+ error = regulator_disable(conf->supply);
+ if (error == 0)
+ conf->supply_enabled = false;
+
+ return error;
+}
+
+static int spi_hid_of_power_up(struct spihid_ops *ops)
+{
+ struct spi_hid_of_config *conf = container_of(ops,
+ struct spi_hid_of_config,
+ ops);
+ int error;
+
+ if (conf->supply_enabled)
+ return 0;
+
+ error = regulator_enable(conf->supply);
+
+ if (error == 0) {
+ conf->supply_enabled = true;
+ fsleep(1000 * conf->timing_data->post_power_on_delay_ms);
+ }
+
+ return error;
+}
+
+static int spi_hid_of_assert_reset(struct spihid_ops *ops)
+{
+ struct spi_hid_of_config *conf = container_of(ops,
+ struct spi_hid_of_config,
+ ops);
+
+ gpiod_set_value(conf->reset_gpio, 1);
+ return 0;
+}
+
+static int spi_hid_of_deassert_reset(struct spihid_ops *ops)
+{
+ struct spi_hid_of_config *conf = container_of(ops,
+ struct spi_hid_of_config,
+ ops);
+
+ gpiod_set_value(conf->reset_gpio, 0);
+ return 0;
+}
+
+static void spi_hid_of_sleep_minimal_reset_delay(struct spihid_ops *ops)
+{
+ struct spi_hid_of_config *conf = container_of(ops,
+ struct spi_hid_of_config,
+ ops);
+ fsleep(1000 * conf->timing_data->minimal_reset_delay_ms);
+}
+
+static int spi_hid_of_probe(struct spi_device *spi)
+{
+ struct device *dev = &spi->dev;
+ struct spi_hid_of_config *config;
+ int error;
+
+ config = devm_kzalloc(dev, sizeof(struct spi_hid_of_config),
+ GFP_KERNEL);
+ if (!config)
+ return -ENOMEM;
+
+ config->ops.power_up = spi_hid_of_power_up;
+ config->ops.power_down = spi_hid_of_power_down;
+ config->ops.assert_reset = spi_hid_of_assert_reset;
+ config->ops.deassert_reset = spi_hid_of_deassert_reset;
+ config->ops.sleep_minimal_reset_delay =
+ spi_hid_of_sleep_minimal_reset_delay;
+
+ config->timing_data = device_get_match_data(dev);
+ /*
+ * FIXME: hid_over_spi_flags could be retrieved from spi mode.
+ * It is always 0 because multi-SPI not supported.
+ */
+ config->hid_over_spi_flags = 0;
+
+ error = spi_hid_of_populate_config(config, dev);
+ if (error) {
+ dev_err(dev, "%s: unable to populate config data.", __func__);
+ return error;
+ }
+
+ return spi_hid_core_probe(spi, &config->ops, &config->property_conf);
+}
+
+const struct spi_hid_timing_data timing_data = {
+ .post_power_on_delay_ms = 10,
+ .minimal_reset_delay_ms = 100,
+};
+
+const struct of_device_id spi_hid_of_match[] = {
+ { .compatible = "hid-over-spi", .data = &timing_data },
+ {}
+};
+MODULE_DEVICE_TABLE(of, spi_hid_of_match);
+
+static const struct spi_device_id spi_hid_of_id_table[] = {
+ { "hid", 0 },
+ { "hid-over-spi", 0 },
+ { }
+};
+MODULE_DEVICE_TABLE(spi, spi_hid_of_id_table);
+
+static struct spi_driver spi_hid_of_driver = {
+ .driver = {
+ .name = "spi_hid_of",
+ .owner = THIS_MODULE,
+ .of_match_table = spi_hid_of_match,
+ .probe_type = PROBE_PREFER_ASYNCHRONOUS,
+ .dev_groups = spi_hid_groups,
+ },
+ .probe = spi_hid_of_probe,
+ .remove = spi_hid_core_remove,
+ .id_table = spi_hid_of_id_table,
+};
+
+module_spi_driver(spi_hid_of_driver);
+
+MODULE_DESCRIPTION("HID over SPI OF transport driver");
+MODULE_AUTHOR("Dmitry Antipov <dmanti@microsoft.com>");
+MODULE_LICENSE("GPL");
--
2.53.0.1185.g05d4b7b318-goog
^ permalink raw reply related
* [PATCH v3 07/11] HID: spi_hid: add ACPI support for SPI over HID
From: Jingyuan Liang @ 2026-04-02 1:59 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Jonathan Corbet, Mark Brown,
Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: linux-input, linux-doc, linux-kernel, linux-spi,
linux-trace-kernel, devicetree, hbarnor, tfiga, Jingyuan Liang,
Angela Czubak
In-Reply-To: <20260402-send-upstream-v3-0-6091c458d357@chromium.org>
From: Angela Czubak <acz@semihalf.com>
Detect SPI HID devices described in ACPI.
Signed-off-by: Angela Czubak <acz@semihalf.com>
Reviewed-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Signed-off-by: Jingyuan Liang <jingyliang@chromium.org>
---
drivers/hid/spi-hid/Kconfig | 15 +++
drivers/hid/spi-hid/Makefile | 1 +
drivers/hid/spi-hid/spi-hid-acpi.c | 253 +++++++++++++++++++++++++++++++++++++
drivers/hid/spi-hid/spi-hid-core.c | 26 +---
drivers/hid/spi-hid/spi-hid.h | 45 +++++++
5 files changed, 315 insertions(+), 25 deletions(-)
diff --git a/drivers/hid/spi-hid/Kconfig b/drivers/hid/spi-hid/Kconfig
index 836fdefe8345..114b1e00da39 100644
--- a/drivers/hid/spi-hid/Kconfig
+++ b/drivers/hid/spi-hid/Kconfig
@@ -10,6 +10,21 @@ menuconfig SPI_HID
if SPI_HID
+config SPI_HID_ACPI
+ tristate "HID over SPI transport layer ACPI driver"
+ depends on ACPI
+ select SPI_HID_CORE
+ help
+ Say Y here if you use a keyboard, a touchpad, a touchscreen, or any
+ other HID based devices which are connected to your computer via SPI.
+ This driver supports ACPI-based systems.
+
+ If unsure, say N.
+
+ This support is also available as a module. If so, the module
+ will be called spi-hid-acpi. It will also build/depend on the
+ module spi-hid.
+
config SPI_HID_CORE
tristate
endif
diff --git a/drivers/hid/spi-hid/Makefile b/drivers/hid/spi-hid/Makefile
index 733e006df56e..3ca326602643 100644
--- a/drivers/hid/spi-hid/Makefile
+++ b/drivers/hid/spi-hid/Makefile
@@ -8,3 +8,4 @@
obj-$(CONFIG_SPI_HID_CORE) += spi-hid.o
spi-hid-objs = spi-hid-core.o
CFLAGS_spi-hid-core.o := -I$(src)
+obj-$(CONFIG_SPI_HID_ACPI) += spi-hid-acpi.o
diff --git a/drivers/hid/spi-hid/spi-hid-acpi.c b/drivers/hid/spi-hid/spi-hid-acpi.c
new file mode 100644
index 000000000000..298e3ba44d8a
--- /dev/null
+++ b/drivers/hid/spi-hid/spi-hid-acpi.c
@@ -0,0 +1,253 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * HID over SPI protocol, ACPI related code
+ *
+ * Copyright (c) 2021 Microsoft Corporation
+ * Copyright (c) 2026 Google LLC
+ *
+ * This code was forked out of the HID over SPI core code, which is partially
+ * based on "HID over I2C protocol implementation:
+ *
+ * Copyright (c) 2012 Benjamin Tissoires <benjamin.tissoires@gmail.com>
+ * Copyright (c) 2012 Ecole Nationale de l'Aviation Civile, France
+ * Copyright (c) 2012 Red Hat, Inc
+ *
+ * which in turn is partially based on "USB HID support for Linux":
+ *
+ * Copyright (c) 1999 Andreas Gal
+ * Copyright (c) 2000-2005 Vojtech Pavlik <vojtech@suse.cz>
+ * Copyright (c) 2005 Michael Haboustak <mike-@cinci.rr.com> for Concept2, Inc
+ * Copyright (c) 2007-2008 Oliver Neukum
+ * Copyright (c) 2006-2010 Jiri Kosina
+ */
+
+#include <linux/acpi.h>
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/reset.h>
+#include <linux/uuid.h>
+
+#include "spi-hid.h"
+
+/* Config structure is filled with data from ACPI */
+struct spi_hid_acpi_config {
+ struct spihid_ops ops;
+
+ struct spi_hid_conf property_conf;
+ u32 post_power_on_delay_ms;
+ u32 minimal_reset_delay_ms;
+ struct acpi_device *adev;
+};
+
+/* HID SPI Device: 6e2ac436-0fcf41af-a265-b32a220dcfab */
+static guid_t spi_hid_guid =
+ GUID_INIT(0x6E2AC436, 0x0FCF, 0x41AF,
+ 0xA2, 0x65, 0xB3, 0x2A, 0x22, 0x0D, 0xCF, 0xAB);
+
+static int spi_hid_acpi_populate_config(struct spi_hid_acpi_config *conf,
+ struct acpi_device *adev)
+{
+ acpi_handle handle = acpi_device_handle(adev);
+ union acpi_object *obj;
+
+ conf->adev = adev;
+
+ /* Revision 3 for HID over SPI V1, see specification. */
+ obj = acpi_evaluate_dsm_typed(handle, &spi_hid_guid, 3, 1, NULL,
+ ACPI_TYPE_INTEGER);
+ if (!obj) {
+ acpi_handle_err(handle,
+ "Error _DSM call to get HID input report header address failed");
+ return -ENODEV;
+ }
+ conf->property_conf.input_report_header_address = obj->integer.value;
+ ACPI_FREE(obj);
+
+ obj = acpi_evaluate_dsm_typed(handle, &spi_hid_guid, 3, 2, NULL,
+ ACPI_TYPE_INTEGER);
+ if (!obj) {
+ acpi_handle_err(handle,
+ "Error _DSM call to get HID input report body address failed");
+ return -ENODEV;
+ }
+ conf->property_conf.input_report_body_address = obj->integer.value;
+ ACPI_FREE(obj);
+
+ obj = acpi_evaluate_dsm_typed(handle, &spi_hid_guid, 3, 3, NULL,
+ ACPI_TYPE_INTEGER);
+ if (!obj) {
+ acpi_handle_err(handle,
+ "Error _DSM call to get HID output report header address failed");
+ return -ENODEV;
+ }
+ conf->property_conf.output_report_address = obj->integer.value;
+ ACPI_FREE(obj);
+
+ obj = acpi_evaluate_dsm_typed(handle, &spi_hid_guid, 3, 4, NULL,
+ ACPI_TYPE_BUFFER);
+ if (!obj) {
+ acpi_handle_err(handle,
+ "Error _DSM call to get HID read opcode failed");
+ return -ENODEV;
+ }
+ if (obj->buffer.length == 1) {
+ conf->property_conf.read_opcode = obj->buffer.pointer[0];
+ } else {
+ acpi_handle_err(handle,
+ "Error _DSM call to get HID read opcode, too long buffer");
+ ACPI_FREE(obj);
+ return -ENODEV;
+ }
+ ACPI_FREE(obj);
+
+ obj = acpi_evaluate_dsm_typed(handle, &spi_hid_guid, 3, 5, NULL,
+ ACPI_TYPE_BUFFER);
+ if (!obj) {
+ acpi_handle_err(handle,
+ "Error _DSM call to get HID write opcode failed");
+ return -ENODEV;
+ }
+ if (obj->buffer.length == 1) {
+ conf->property_conf.write_opcode = obj->buffer.pointer[0];
+ } else {
+ acpi_handle_err(handle,
+ "Error _DSM call to get HID write opcode, too long buffer");
+ ACPI_FREE(obj);
+ return -ENODEV;
+ }
+ ACPI_FREE(obj);
+
+ /* Value not provided in ACPI,*/
+ conf->post_power_on_delay_ms = 5;
+ conf->minimal_reset_delay_ms = 150;
+
+ if (!acpi_has_method(handle, "_RST")) {
+ acpi_handle_err(handle, "No reset method for acpi handle");
+ return -EINVAL;
+ }
+
+ /* FIXME: not reading hid-over-spi-flags, multi-SPI not supported */
+
+ return 0;
+}
+
+static int spi_hid_acpi_power_none(struct spihid_ops *ops)
+{
+ return 0;
+}
+
+static int spi_hid_acpi_power_down(struct spihid_ops *ops)
+{
+ struct spi_hid_acpi_config *conf = container_of(ops,
+ struct spi_hid_acpi_config,
+ ops);
+
+ return acpi_device_set_power(conf->adev, ACPI_STATE_D3);
+}
+
+static int spi_hid_acpi_power_up(struct spihid_ops *ops)
+{
+ struct spi_hid_acpi_config *conf = container_of(ops,
+ struct spi_hid_acpi_config,
+ ops);
+ int error;
+
+ error = acpi_device_set_power(conf->adev, ACPI_STATE_D0);
+ if (error) {
+ dev_err(&conf->adev->dev, "Error could not power up ACPI device: %d.", error);
+ return error;
+ }
+
+ if (conf->post_power_on_delay_ms)
+ msleep(conf->post_power_on_delay_ms);
+
+ return 0;
+}
+
+static int spi_hid_acpi_assert_reset(struct spihid_ops *ops)
+{
+ return 0;
+}
+
+static int spi_hid_acpi_deassert_reset(struct spihid_ops *ops)
+{
+ struct spi_hid_acpi_config *conf = container_of(ops,
+ struct spi_hid_acpi_config,
+ ops);
+
+ return device_reset(&conf->adev->dev);
+}
+
+static void spi_hid_acpi_sleep_minimal_reset_delay(struct spihid_ops *ops)
+{
+ struct spi_hid_acpi_config *conf = container_of(ops,
+ struct spi_hid_acpi_config,
+ ops);
+ fsleep(1000 * conf->minimal_reset_delay_ms);
+}
+
+static int spi_hid_acpi_probe(struct spi_device *spi)
+{
+ struct device *dev = &spi->dev;
+ struct acpi_device *adev;
+ struct spi_hid_acpi_config *config;
+ int error;
+
+ adev = ACPI_COMPANION(dev);
+ if (!adev) {
+ dev_err(dev, "Error could not get ACPI device.");
+ return -ENODEV;
+ }
+
+ config = devm_kzalloc(dev, sizeof(struct spi_hid_acpi_config),
+ GFP_KERNEL);
+ if (!config)
+ return -ENOMEM;
+
+ if (acpi_device_power_manageable(adev)) {
+ config->ops.power_up = spi_hid_acpi_power_up;
+ config->ops.power_down = spi_hid_acpi_power_down;
+ } else {
+ config->ops.power_up = spi_hid_acpi_power_none;
+ config->ops.power_down = spi_hid_acpi_power_none;
+ }
+ config->ops.assert_reset = spi_hid_acpi_assert_reset;
+ config->ops.deassert_reset = spi_hid_acpi_deassert_reset;
+ config->ops.sleep_minimal_reset_delay =
+ spi_hid_acpi_sleep_minimal_reset_delay;
+
+ error = spi_hid_acpi_populate_config(config, adev);
+ if (error) {
+ dev_err(dev, "%s: unable to populate config data.", __func__);
+ return error;
+ }
+
+ return spi_hid_core_probe(spi, &config->ops, &config->property_conf);
+}
+
+static const struct acpi_device_id spi_hid_acpi_match[] = {
+ { "ACPI0C51", 0 },
+ { "PNP0C51", 0 },
+ { }
+};
+MODULE_DEVICE_TABLE(acpi, spi_hid_acpi_match);
+
+static struct spi_driver spi_hid_acpi_driver = {
+ .driver = {
+ .name = "spi_hid_acpi",
+ .owner = THIS_MODULE,
+ .acpi_match_table = spi_hid_acpi_match,
+ .probe_type = PROBE_PREFER_ASYNCHRONOUS,
+ .dev_groups = spi_hid_groups,
+ },
+ .probe = spi_hid_acpi_probe,
+ .remove = spi_hid_core_remove,
+};
+
+module_spi_driver(spi_hid_acpi_driver);
+
+MODULE_DESCRIPTION("HID over SPI ACPI transport driver");
+MODULE_AUTHOR("Angela Czubak <aczubak@google.com>");
+MODULE_LICENSE("GPL");
diff --git a/drivers/hid/spi-hid/spi-hid-core.c b/drivers/hid/spi-hid/spi-hid-core.c
index 802615565541..d48175c764b9 100644
--- a/drivers/hid/spi-hid/spi-hid-core.c
+++ b/drivers/hid/spi-hid/spi-hid-core.c
@@ -43,6 +43,7 @@
#include <linux/wait.h>
#include <linux/workqueue.h>
+#include "spi-hid.h"
#include "spi-hid-core.h"
#define CREATE_TRACE_POINTS
@@ -110,31 +111,6 @@ struct spi_hid_output_report {
u8 *content;
};
-/* struct spi_hid_conf - Conf provided to the core */
-struct spi_hid_conf {
- u32 input_report_header_address;
- u32 input_report_body_address;
- u32 output_report_address;
- u8 read_opcode;
- u8 write_opcode;
-};
-
-/**
- * struct spihid_ops - Ops provided to the core
- * @power_up: do sequencing to power up the device
- * @power_down: do sequencing to power down the device
- * @assert_reset: do sequencing to assert the reset line
- * @deassert_reset: do sequencing to deassert the reset line
- * @sleep_minimal_reset_delay: minimal sleep delay during reset
- */
-struct spihid_ops {
- int (*power_up)(struct spihid_ops *ops);
- int (*power_down)(struct spihid_ops *ops);
- int (*assert_reset)(struct spihid_ops *ops);
- int (*deassert_reset)(struct spihid_ops *ops);
- void (*sleep_minimal_reset_delay)(struct spihid_ops *ops);
-};
-
static struct hid_ll_driver spi_hid_ll_driver;
static void spi_hid_populate_read_approvals(const struct spi_hid_conf *conf,
diff --git a/drivers/hid/spi-hid/spi-hid.h b/drivers/hid/spi-hid/spi-hid.h
new file mode 100644
index 000000000000..f5a5f4d54beb
--- /dev/null
+++ b/drivers/hid/spi-hid/spi-hid.h
@@ -0,0 +1,45 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (c) 2021 Microsoft Corporation
+ * Copyright (c) 2026 Google LLC
+ */
+
+#ifndef SPI_HID_H
+#define SPI_HID_H
+
+#include <linux/spi/spi.h>
+#include <linux/sysfs.h>
+
+/* struct spi_hid_conf - Conf provided to the core */
+struct spi_hid_conf {
+ u32 input_report_header_address;
+ u32 input_report_body_address;
+ u32 output_report_address;
+ u8 read_opcode;
+ u8 write_opcode;
+};
+
+/**
+ * struct spihid_ops - Ops provided to the core
+ * @power_up: do sequencing to power up the device
+ * @power_down: do sequencing to power down the device
+ * @assert_reset: do sequencing to assert the reset line
+ * @deassert_reset: do sequencing to deassert the reset line
+ * @sleep_minimal_reset_delay: minimal sleep delay during reset
+ */
+struct spihid_ops {
+ int (*power_up)(struct spihid_ops *ops);
+ int (*power_down)(struct spihid_ops *ops);
+ int (*assert_reset)(struct spihid_ops *ops);
+ int (*deassert_reset)(struct spihid_ops *ops);
+ void (*sleep_minimal_reset_delay)(struct spihid_ops *ops);
+};
+
+int spi_hid_core_probe(struct spi_device *spi, struct spihid_ops *ops,
+ struct spi_hid_conf *conf);
+
+void spi_hid_core_remove(struct spi_device *spi);
+
+extern const struct attribute_group *spi_hid_groups[];
+
+#endif /* SPI_HID_H */
--
2.53.0.1185.g05d4b7b318-goog
^ permalink raw reply related
* [PATCH v3 06/11] HID: spi_hid: add spi_hid traces
From: Jingyuan Liang @ 2026-04-02 1:59 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Jonathan Corbet, Mark Brown,
Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: linux-input, linux-doc, linux-kernel, linux-spi,
linux-trace-kernel, devicetree, hbarnor, tfiga, Jingyuan Liang,
Dmitry Antipov, Angela Czubak
In-Reply-To: <20260402-send-upstream-v3-0-6091c458d357@chromium.org>
Add traces for purposed of debugging spi_hid driver.
Signed-off-by: Dmitry Antipov <dmanti@microsoft.com>
Signed-off-by: Angela Czubak <acz@semihalf.com>
Signed-off-by: Jingyuan Liang <jingyliang@chromium.org>
---
drivers/hid/spi-hid/Makefile | 1 +
drivers/hid/spi-hid/spi-hid-core.c | 118 +++++++++----------------
drivers/hid/spi-hid/spi-hid-core.h | 91 +++++++++++++++++++
drivers/hid/spi-hid/spi-hid-trace.h | 169 ++++++++++++++++++++++++++++++++++++
4 files changed, 303 insertions(+), 76 deletions(-)
diff --git a/drivers/hid/spi-hid/Makefile b/drivers/hid/spi-hid/Makefile
index 92e24cddbfc2..733e006df56e 100644
--- a/drivers/hid/spi-hid/Makefile
+++ b/drivers/hid/spi-hid/Makefile
@@ -7,3 +7,4 @@
obj-$(CONFIG_SPI_HID_CORE) += spi-hid.o
spi-hid-objs = spi-hid-core.o
+CFLAGS_spi-hid-core.o := -I$(src)
diff --git a/drivers/hid/spi-hid/spi-hid-core.c b/drivers/hid/spi-hid/spi-hid-core.c
index 00b9718ba2c3..802615565541 100644
--- a/drivers/hid/spi-hid/spi-hid-core.c
+++ b/drivers/hid/spi-hid/spi-hid-core.c
@@ -43,6 +43,11 @@
#include <linux/wait.h>
#include <linux/workqueue.h>
+#include "spi-hid-core.h"
+
+#define CREATE_TRACE_POINTS
+#include "spi-hid-trace.h"
+
/* Protocol constants */
#define SPI_HID_READ_APPROVAL_CONSTANT 0xff
#define SPI_HID_INPUT_HEADER_SYNC_BYTE 0x5a
@@ -81,13 +86,6 @@
#define SPI_HID_CREATE_DEVICE 4
#define SPI_HID_ERROR 5
-/* Raw input buffer with data from the bus */
-struct spi_hid_input_buf {
- u8 header[HIDSPI_INPUT_HEADER_SIZE];
- u8 body[HIDSPI_INPUT_BODY_HEADER_SIZE];
- u8 content[];
-};
-
/* Processed data from input report header */
struct spi_hid_input_header {
u8 version;
@@ -104,12 +102,6 @@ struct spi_hid_input_report {
u8 *content;
};
-/* Raw output report buffer to be put on the bus */
-struct spi_hid_output_buf {
- u8 header[SPI_HID_OUTPUT_HEADER_LEN];
- u8 content[];
-};
-
/* Data necessary to send an output report */
struct spi_hid_output_report {
u8 report_type;
@@ -118,19 +110,6 @@ struct spi_hid_output_report {
u8 *content;
};
-/* Processed data from a device descriptor */
-struct spi_hid_device_descriptor {
- u16 hid_version;
- u16 report_descriptor_length;
- u16 max_input_length;
- u16 max_output_length;
- u16 max_fragment_length;
- u16 vendor_id;
- u16 product_id;
- u16 version_id;
- u8 no_output_report_ack;
-};
-
/* struct spi_hid_conf - Conf provided to the core */
struct spi_hid_conf {
u32 input_report_header_address;
@@ -156,54 +135,6 @@ struct spihid_ops {
void (*sleep_minimal_reset_delay)(struct spihid_ops *ops);
};
-/* Driver context */
-struct spi_hid {
- struct spi_device *spi; /* spi device. */
- struct hid_device *hid; /* pointer to corresponding HID dev. */
-
- struct spi_transfer input_transfer[2]; /* Transfer buffer for read and write. */
- struct spi_message input_message; /* used to execute a sequence of spi transfers. */
-
- struct spihid_ops *ops;
- struct spi_hid_conf *conf;
-
- struct spi_hid_device_descriptor desc; /* HID device descriptor. */
- struct spi_hid_output_buf *output; /* Output buffer. */
- struct spi_hid_input_buf *input; /* Input buffer. */
- struct spi_hid_input_buf *response; /* Response buffer. */
-
- u16 response_length;
- u16 bufsize;
-
- enum hidspi_power_state power_state;
-
- u8 reset_attempts; /* The number of reset attempts. */
-
- unsigned long flags; /* device flags. */
-
- struct work_struct reset_work;
-
- /* Control lock to ensure complete output transaction. */
- struct mutex output_lock;
- /* Power lock to make sure one power state change at a time. */
- struct mutex power_lock;
- /* I/O lock to prevent concurrent output writes during the input read. */
- struct mutex io_lock;
-
- struct completion output_done;
-
- u8 read_approval_header[SPI_HID_READ_APPROVAL_LEN];
- u8 read_approval_body[SPI_HID_READ_APPROVAL_LEN];
-
- u32 report_descriptor_crc32; /* HID report descriptor crc32 checksum. */
-
- u32 regulator_error_count;
- int regulator_last_error;
- u32 bus_error_count;
- int bus_last_error;
- u32 dir_count; /* device initiated reset count. */
-};
-
static struct hid_ll_driver spi_hid_ll_driver;
static void spi_hid_populate_read_approvals(const struct spi_hid_conf *conf,
@@ -293,6 +224,11 @@ static int spi_hid_input_sync(struct spi_hid *shid, void *buf, u16 length,
spi_message_init_with_transfers(&shid->input_message,
shid->input_transfer, 2);
+ trace_spi_hid_input_sync(shid, shid->input_transfer[0].tx_buf,
+ shid->input_transfer[0].len,
+ shid->input_transfer[1].rx_buf,
+ shid->input_transfer[1].len, 0);
+
error = spi_sync(shid->spi, &shid->input_message);
if (error) {
dev_err(&shid->spi->dev, "Error starting sync transfer: %d.", error);
@@ -343,11 +279,13 @@ static void spi_hid_stop_hid(struct spi_hid *shid)
hid_destroy_device(hid);
}
-static void spi_hid_error(struct spi_hid *shid)
+static void spi_hid_error_handler(struct spi_hid *shid)
{
struct device *dev = &shid->spi->dev;
int error;
+ trace_spi_hid_error_handler(shid);
+
guard(mutex)(&shid->power_lock);
if (shid->power_state == HIDSPI_OFF)
return;
@@ -457,6 +395,8 @@ static void spi_hid_reset_response(struct spi_hid *shid)
};
int error;
+ trace_spi_hid_reset_response(shid);
+
if (test_bit(SPI_HID_READY, &shid->flags)) {
dev_err(dev, "Spontaneous FW reset!");
clear_bit(SPI_HID_READY, &shid->flags);
@@ -482,6 +422,8 @@ static int spi_hid_input_report_handler(struct spi_hid *shid,
struct spi_hid_input_report r;
int error = 0;
+ trace_spi_hid_input_report_handler(shid);
+
if (!test_bit(SPI_HID_READY, &shid->flags) ||
test_bit(SPI_HID_REFRESH_IN_PROGRESS, &shid->flags) || !shid->hid) {
dev_err(dev, "HID not ready");
@@ -506,6 +448,8 @@ static int spi_hid_input_report_handler(struct spi_hid *shid,
static void spi_hid_response_handler(struct spi_hid *shid,
struct input_report_body_header *body)
{
+ trace_spi_hid_response_handler(shid);
+
shid->response_length = body->content_len;
/* completion_done returns 0 if there are waiters, otherwise 1 */
if (completion_done(&shid->output_done)) {
@@ -562,6 +506,8 @@ static int spi_hid_create_device(struct spi_hid *shid)
struct device *dev = &shid->spi->dev;
int error;
+ trace_spi_hid_create_device(shid);
+
hid = hid_allocate_device();
error = PTR_ERR_OR_ZERO(hid);
if (error) {
@@ -603,6 +549,8 @@ static void spi_hid_refresh_device(struct spi_hid *shid)
u32 new_crc32 = 0;
int error = 0;
+ trace_spi_hid_refresh_device(shid);
+
error = spi_hid_report_descriptor_request(shid);
if (error < 0) {
dev_err(dev,
@@ -668,7 +616,7 @@ static void spi_hid_reset_work(struct work_struct *work)
}
if (test_and_clear_bit(SPI_HID_ERROR, &shid->flags)) {
- spi_hid_error(shid);
+ spi_hid_error_handler(shid);
return;
}
}
@@ -681,6 +629,8 @@ static int spi_hid_process_input_report(struct spi_hid *shid,
struct device *dev = &shid->spi->dev;
struct hidspi_dev_descriptor *raw;
+ trace_spi_hid_process_input_report(shid);
+
spi_hid_populate_input_header(buf->header, &header);
spi_hid_populate_input_body(buf->body, &body);
@@ -840,6 +790,9 @@ static irqreturn_t spi_hid_dev_irq(int irq, void *_shid)
struct spi_hid_input_header header;
int error = 0;
+ trace_spi_hid_dev_irq(shid, irq);
+ trace_spi_hid_header_transfer(shid);
+
scoped_guard(mutex, &shid->io_lock) {
error = spi_hid_input_sync(shid, shid->input->header,
sizeof(shid->input->header), true);
@@ -853,6 +806,13 @@ static irqreturn_t spi_hid_dev_irq(int irq, void *_shid)
goto out;
}
+ trace_spi_hid_input_header_complete(shid,
+ shid->input_transfer[0].tx_buf,
+ shid->input_transfer[0].len,
+ shid->input_transfer[1].rx_buf,
+ shid->input_transfer[1].len,
+ shid->input_message.status);
+
if (shid->input_message.status < 0) {
dev_warn(dev, "Error reading header: %d.",
shid->input_message.status);
@@ -889,6 +849,12 @@ static irqreturn_t spi_hid_dev_irq(int irq, void *_shid)
goto out;
}
+ trace_spi_hid_input_body_complete(shid, shid->input_transfer[0].tx_buf,
+ shid->input_transfer[0].len,
+ shid->input_transfer[1].rx_buf,
+ shid->input_transfer[1].len,
+ shid->input_message.status);
+
if (shid->input_message.status < 0) {
dev_warn(dev, "Error reading body: %d.",
shid->input_message.status);
diff --git a/drivers/hid/spi-hid/spi-hid-core.h b/drivers/hid/spi-hid/spi-hid-core.h
new file mode 100644
index 000000000000..293e2cfcfbf7
--- /dev/null
+++ b/drivers/hid/spi-hid/spi-hid-core.h
@@ -0,0 +1,91 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (c) 2021 Microsoft Corporation
+ * Copyright (c) 2026 Google LLC
+ */
+
+#ifndef SPI_HID_CORE_H
+#define SPI_HID_CORE_H
+
+#include <linux/hid-over-spi.h>
+#include <linux/spi/spi.h>
+
+/* Protocol message size constants */
+#define SPI_HID_READ_APPROVAL_LEN 5
+#define SPI_HID_OUTPUT_HEADER_LEN 8
+
+/* Raw input buffer with data from the bus */
+struct spi_hid_input_buf {
+ u8 header[HIDSPI_INPUT_HEADER_SIZE];
+ u8 body[HIDSPI_INPUT_BODY_HEADER_SIZE];
+ u8 content[];
+};
+
+/* Raw output report buffer to be put on the bus */
+struct spi_hid_output_buf {
+ u8 header[SPI_HID_OUTPUT_HEADER_LEN];
+ u8 content[];
+};
+
+/* Processed data from a device descriptor */
+struct spi_hid_device_descriptor {
+ u16 hid_version;
+ u16 report_descriptor_length;
+ u16 max_input_length;
+ u16 max_output_length;
+ u16 max_fragment_length;
+ u16 vendor_id;
+ u16 product_id;
+ u16 version_id;
+ u8 no_output_report_ack;
+};
+
+/* Driver context */
+struct spi_hid {
+ struct spi_device *spi; /* spi device. */
+ struct hid_device *hid; /* pointer to corresponding HID dev. */
+
+ struct spi_transfer input_transfer[2]; /* Transfer buffer for read and write. */
+ struct spi_message input_message; /* used to execute a sequence of spi transfers. */
+
+ struct spihid_ops *ops;
+ struct spi_hid_conf *conf;
+
+ struct spi_hid_device_descriptor desc; /* HID device descriptor. */
+ struct spi_hid_output_buf *output; /* Output buffer. */
+ struct spi_hid_input_buf *input; /* Input buffer. */
+ struct spi_hid_input_buf *response; /* Response buffer. */
+
+ u16 response_length;
+ u16 bufsize;
+
+ enum hidspi_power_state power_state;
+
+ u8 reset_attempts; /* The number of reset attempts. */
+
+ unsigned long flags; /* device flags. */
+
+ struct work_struct reset_work;
+
+ /* Control lock to ensure complete output transaction. */
+ struct mutex output_lock;
+ /* Power lock to make sure one power state change at a time. */
+ struct mutex power_lock;
+ /* I/O lock to prevent concurrent output writes during the input read. */
+ struct mutex io_lock;
+
+ struct completion output_done;
+
+ u8 read_approval_header[SPI_HID_READ_APPROVAL_LEN];
+ u8 read_approval_body[SPI_HID_READ_APPROVAL_LEN];
+
+ u32 report_descriptor_crc32; /* HID report descriptor crc32 checksum. */
+
+ u32 regulator_error_count;
+ int regulator_last_error;
+ u32 bus_error_count;
+ int bus_last_error;
+ u32 dir_count; /* device initiated reset count. */
+};
+
+#endif /* SPI_HID_CORE_H */
diff --git a/drivers/hid/spi-hid/spi-hid-trace.h b/drivers/hid/spi-hid/spi-hid-trace.h
new file mode 100644
index 000000000000..cc13d71a14de
--- /dev/null
+++ b/drivers/hid/spi-hid/spi-hid-trace.h
@@ -0,0 +1,169 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (c) 2021 Microsoft Corporation
+ */
+
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM spi_hid
+
+#if !defined(_SPI_HID_TRACE_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _SPI_HID_TRACE_H
+
+#include <linux/types.h>
+#include <linux/tracepoint.h>
+#include "spi-hid-core.h"
+
+DECLARE_EVENT_CLASS(spi_hid_transfer,
+ TP_PROTO(struct spi_hid *shid, const void *tx_buf, int tx_len,
+ const void *rx_buf, u16 rx_len, int ret),
+
+ TP_ARGS(shid, tx_buf, tx_len, rx_buf, rx_len, ret),
+
+ TP_STRUCT__entry(
+ __field(int, bus_num)
+ __field(int, chip_select)
+ __field(int, ret)
+ __dynamic_array(u8, rx_buf, rx_len)
+ __dynamic_array(u8, tx_buf, tx_len)
+ ),
+
+ TP_fast_assign(
+ __entry->bus_num = shid->spi->controller->bus_num;
+ __entry->chip_select = shid->spi->chip_select;
+ __entry->ret = ret;
+
+ memcpy(__get_dynamic_array(tx_buf), tx_buf, tx_len);
+ memcpy(__get_dynamic_array(rx_buf), rx_buf, rx_len);
+ ),
+
+ TP_printk("spi%d.%d: len=%d tx=[%*phD] rx=[%*phD] --> %d",
+ __entry->bus_num, __entry->chip_select,
+ __get_dynamic_array_len(tx_buf) + __get_dynamic_array_len(rx_buf),
+ __get_dynamic_array_len(tx_buf), __get_dynamic_array(tx_buf),
+ __get_dynamic_array_len(rx_buf), __get_dynamic_array(rx_buf),
+ __entry->ret)
+);
+
+DEFINE_EVENT(spi_hid_transfer, spi_hid_input_sync,
+ TP_PROTO(struct spi_hid *shid, const void *tx_buf, int tx_len,
+ const void *rx_buf, u16 rx_len, int ret),
+ TP_ARGS(shid, tx_buf, tx_len, rx_buf, rx_len, ret));
+
+DEFINE_EVENT(spi_hid_transfer, spi_hid_input_header_complete,
+ TP_PROTO(struct spi_hid *shid, const void *tx_buf, int tx_len,
+ const void *rx_buf, u16 rx_len, int ret),
+ TP_ARGS(shid, tx_buf, tx_len, rx_buf, rx_len, ret));
+
+DEFINE_EVENT(spi_hid_transfer, spi_hid_input_body_complete,
+ TP_PROTO(struct spi_hid *shid, const void *tx_buf, int tx_len,
+ const void *rx_buf, u16 rx_len, int ret),
+ TP_ARGS(shid, tx_buf, tx_len, rx_buf, rx_len, ret));
+
+DECLARE_EVENT_CLASS(spi_hid_irq,
+ TP_PROTO(struct spi_hid *shid, int irq),
+
+ TP_ARGS(shid, irq),
+
+ TP_STRUCT__entry(
+ __field(int, bus_num)
+ __field(int, chip_select)
+ __field(int, irq)
+ ),
+
+ TP_fast_assign(
+ __entry->bus_num = shid->spi->controller->bus_num;
+ __entry->chip_select = shid->spi->chip_select;
+ __entry->irq = irq;
+ ),
+
+ TP_printk("spi%d.%d: IRQ %d",
+ __entry->bus_num, __entry->chip_select, __entry->irq)
+);
+
+DEFINE_EVENT(spi_hid_irq, spi_hid_dev_irq,
+ TP_PROTO(struct spi_hid *shid, int irq), TP_ARGS(shid, irq));
+
+DECLARE_EVENT_CLASS(spi_hid,
+ TP_PROTO(struct spi_hid *shid),
+
+ TP_ARGS(shid),
+
+ TP_STRUCT__entry(
+ __field(int, bus_num)
+ __field(int, chip_select)
+ __field(int, power_state)
+ __field(u32, flags)
+
+ __field(int, vendor_id)
+ __field(int, product_id)
+ __field(int, max_input_length)
+ __field(int, max_output_length)
+ __field(u16, hid_version)
+ __field(u16, report_descriptor_length)
+ __field(u16, version_id)
+ ),
+
+ TP_fast_assign(
+ __entry->bus_num = shid->spi->controller->bus_num;
+ __entry->chip_select = shid->spi->chip_select;
+ __entry->power_state = shid->power_state;
+ __entry->flags = shid->flags;
+
+ __entry->vendor_id = shid->desc.vendor_id;
+ __entry->product_id = shid->desc.product_id;
+ __entry->max_input_length = shid->desc.max_input_length;
+ __entry->max_output_length = shid->desc.max_output_length;
+ __entry->hid_version = shid->desc.hid_version;
+ __entry->report_descriptor_length =
+ shid->desc.report_descriptor_length;
+ __entry->version_id = shid->desc.version_id;
+ ),
+
+ TP_printk("spi%d.%d: (%04x:%04x v%d) HID v%d.%d state p:%d len i:%d o:%d r:%d flags 0x%08x",
+ __entry->bus_num, __entry->chip_select,
+ __entry->vendor_id, __entry->product_id, __entry->version_id,
+ __entry->hid_version >> 8, __entry->hid_version & 0xff,
+ __entry->power_state, __entry->max_input_length,
+ __entry->max_output_length, __entry->report_descriptor_length,
+ __entry->flags)
+);
+
+DEFINE_EVENT(spi_hid, spi_hid_header_transfer, TP_PROTO(struct spi_hid *shid),
+ TP_ARGS(shid));
+
+DEFINE_EVENT(spi_hid, spi_hid_process_input_report,
+ TP_PROTO(struct spi_hid *shid), TP_ARGS(shid));
+
+DEFINE_EVENT(spi_hid, spi_hid_input_report_handler,
+ TP_PROTO(struct spi_hid *shid), TP_ARGS(shid));
+
+DEFINE_EVENT(spi_hid, spi_hid_reset_response, TP_PROTO(struct spi_hid *shid),
+ TP_ARGS(shid));
+
+DEFINE_EVENT(spi_hid, spi_hid_create_device, TP_PROTO(struct spi_hid *shid),
+ TP_ARGS(shid));
+
+DEFINE_EVENT(spi_hid, spi_hid_refresh_device, TP_PROTO(struct spi_hid *shid),
+ TP_ARGS(shid));
+
+DEFINE_EVENT(spi_hid, spi_hid_response_handler, TP_PROTO(struct spi_hid *shid),
+ TP_ARGS(shid));
+
+DEFINE_EVENT(spi_hid, spi_hid_error_handler, TP_PROTO(struct spi_hid *shid),
+ TP_ARGS(shid));
+
+#endif /* _SPI_HID_TRACE_H */
+
+/*
+ * The following must be outside the protection of the above #if block.
+ */
+#undef TRACE_INCLUDE_PATH
+#undef TRACE_INCLUDE_FILE
+#define TRACE_INCLUDE_PATH .
+
+/*
+ * It is required that the TRACE_INCLUDE_FILE be the same
+ * as this file without the ".h".
+ */
+#define TRACE_INCLUDE_FILE spi-hid-trace
+#include <trace/define_trace.h>
--
2.53.0.1185.g05d4b7b318-goog
^ permalink raw reply related
* [PATCH v3 05/11] HID: spi-hid: add HID SPI protocol implementation
From: Jingyuan Liang @ 2026-04-02 1:59 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Jonathan Corbet, Mark Brown,
Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: linux-input, linux-doc, linux-kernel, linux-spi,
linux-trace-kernel, devicetree, hbarnor, tfiga, Jingyuan Liang,
Dmitry Antipov, Angela Czubak
In-Reply-To: <20260402-send-upstream-v3-0-6091c458d357@chromium.org>
This driver follows HID Over SPI Protocol Specification 1.0 available at
https://www.microsoft.com/en-us/download/details.aspx?id=103325. The
initial version of the driver does not support: 1) multi-fragment input
reports, 2) sending GET_INPUT and COMMAND output report types and
processing their respective acknowledge input reports, and 3) device
sleep power state.
Signed-off-by: Dmitry Antipov <dmanti@microsoft.com>
Signed-off-by: Angela Czubak <acz@semihalf.com>
Signed-off-by: Jingyuan Liang <jingyliang@chromium.org>
---
drivers/hid/spi-hid/spi-hid-core.c | 582 ++++++++++++++++++++++++++++++++++++-
1 file changed, 572 insertions(+), 10 deletions(-)
diff --git a/drivers/hid/spi-hid/spi-hid-core.c b/drivers/hid/spi-hid/spi-hid-core.c
index 4723b87346d4..00b9718ba2c3 100644
--- a/drivers/hid/spi-hid/spi-hid-core.c
+++ b/drivers/hid/spi-hid/spi-hid-core.c
@@ -23,11 +23,16 @@
#include <linux/completion.h>
#include <linux/crc32.h>
#include <linux/device.h>
+#include <linux/dma-mapping.h>
#include <linux/err.h>
#include <linux/hid.h>
#include <linux/hid-over-spi.h>
+#include <linux/input.h>
#include <linux/interrupt.h>
+#include <linux/irq.h>
#include <linux/jiffies.h>
+#include <linux/kernel.h>
+#include <linux/list.h>
#include <linux/module.h>
#include <linux/mutex.h>
#include <linux/slab.h>
@@ -35,12 +40,22 @@
#include <linux/string.h>
#include <linux/sysfs.h>
#include <linux/unaligned.h>
+#include <linux/wait.h>
+#include <linux/workqueue.h>
+
+/* Protocol constants */
+#define SPI_HID_READ_APPROVAL_CONSTANT 0xff
+#define SPI_HID_INPUT_HEADER_SYNC_BYTE 0x5a
+#define SPI_HID_INPUT_HEADER_VERSION 0x03
+#define SPI_HID_SUPPORTED_VERSION 0x0300
#define SPI_HID_OUTPUT_REPORT_CONTENT_ID_DESC_REQUEST 0x00
-#define SPI_HID_RESP_TIMEOUT 1000
+#define SPI_HID_MAX_RESET_ATTEMPTS 3
+#define SPI_HID_RESP_TIMEOUT 1000
/* Protocol message size constants */
+#define SPI_HID_READ_APPROVAL_LEN 5
#define SPI_HID_OUTPUT_HEADER_LEN 8
/* flags */
@@ -49,6 +64,22 @@
* requests. The FW becomes ready after sending the report descriptor.
*/
#define SPI_HID_READY 0
+/*
+ * refresh_in_progress is set to true while the refresh_device worker
+ * thread is destroying and recreating the hidraw device. When this flag
+ * is set to true, the ll_close and ll_open functions will not cause
+ * power state changes.
+ */
+#define SPI_HID_REFRESH_IN_PROGRESS 1
+/*
+ * reset_pending indicates that the device is being reset. When this flag
+ * is set to true, garbage interrupts triggered during reset will be
+ * dropped and will not cause error handling.
+ */
+#define SPI_HID_RESET_PENDING 2
+#define SPI_HID_RESET_RESPONSE 3
+#define SPI_HID_CREATE_DEVICE 4
+#define SPI_HID_ERROR 5
/* Raw input buffer with data from the bus */
struct spi_hid_input_buf {
@@ -57,6 +88,22 @@ struct spi_hid_input_buf {
u8 content[];
};
+/* Processed data from input report header */
+struct spi_hid_input_header {
+ u8 version;
+ u16 report_length;
+ u8 last_fragment_flag;
+ u8 sync_const;
+};
+
+/* Processed data from an input report */
+struct spi_hid_input_report {
+ u8 report_type;
+ u16 content_length;
+ u8 content_id;
+ u8 *content;
+};
+
/* Raw output report buffer to be put on the bus */
struct spi_hid_output_buf {
u8 header[SPI_HID_OUTPUT_HEADER_LEN];
@@ -114,6 +161,9 @@ struct spi_hid {
struct spi_device *spi; /* spi device. */
struct hid_device *hid; /* pointer to corresponding HID dev. */
+ struct spi_transfer input_transfer[2]; /* Transfer buffer for read and write. */
+ struct spi_message input_message; /* used to execute a sequence of spi transfers. */
+
struct spihid_ops *ops;
struct spi_hid_conf *conf;
@@ -131,10 +181,20 @@ struct spi_hid {
unsigned long flags; /* device flags. */
- /* Control lock to make sure one output transaction at a time. */
+ struct work_struct reset_work;
+
+ /* Control lock to ensure complete output transaction. */
struct mutex output_lock;
+ /* Power lock to make sure one power state change at a time. */
+ struct mutex power_lock;
+ /* I/O lock to prevent concurrent output writes during the input read. */
+ struct mutex io_lock;
+
struct completion output_done;
+ u8 read_approval_header[SPI_HID_READ_APPROVAL_LEN];
+ u8 read_approval_body[SPI_HID_READ_APPROVAL_LEN];
+
u32 report_descriptor_crc32; /* HID report descriptor crc32 checksum. */
u32 regulator_error_count;
@@ -146,6 +206,66 @@ struct spi_hid {
static struct hid_ll_driver spi_hid_ll_driver;
+static void spi_hid_populate_read_approvals(const struct spi_hid_conf *conf,
+ u8 *header_buf, u8 *body_buf)
+{
+ header_buf[0] = conf->read_opcode;
+ put_unaligned_be24(conf->input_report_header_address, &header_buf[1]);
+ header_buf[4] = SPI_HID_READ_APPROVAL_CONSTANT;
+
+ body_buf[0] = conf->read_opcode;
+ put_unaligned_be24(conf->input_report_body_address, &body_buf[1]);
+ body_buf[4] = SPI_HID_READ_APPROVAL_CONSTANT;
+}
+
+static void spi_hid_parse_dev_desc(const struct hidspi_dev_descriptor *raw,
+ struct spi_hid_device_descriptor *desc)
+{
+ desc->hid_version = le16_to_cpu(raw->bcd_ver);
+ desc->report_descriptor_length = le16_to_cpu(raw->rep_desc_len);
+ desc->max_input_length = le16_to_cpu(raw->max_input_len);
+ desc->max_output_length = le16_to_cpu(raw->max_output_len);
+
+ /* FIXME: multi-fragment not supported, field below not used */
+ desc->max_fragment_length = le16_to_cpu(raw->max_frag_len);
+
+ desc->vendor_id = le16_to_cpu(raw->vendor_id);
+ desc->product_id = le16_to_cpu(raw->product_id);
+ desc->version_id = le16_to_cpu(raw->version_id);
+ desc->no_output_report_ack = le16_to_cpu(raw->flags) & BIT(0);
+}
+
+static void spi_hid_populate_input_header(const u8 *buf,
+ struct spi_hid_input_header *header)
+{
+ header->version = buf[0] & 0xf;
+ header->report_length = (get_unaligned_le16(&buf[1]) & 0x3fff) * 4;
+ header->last_fragment_flag = (buf[2] & 0x40) >> 6;
+ header->sync_const = buf[3];
+}
+
+static void spi_hid_populate_input_body(const u8 *buf,
+ struct input_report_body_header *body)
+{
+ body->input_report_type = buf[0];
+ body->content_len = get_unaligned_le16(&buf[1]);
+ body->content_id = buf[3];
+}
+
+static void spi_hid_input_report_prepare(struct spi_hid_input_buf *buf,
+ struct spi_hid_input_report *report)
+{
+ struct spi_hid_input_header header;
+ struct input_report_body_header body;
+
+ spi_hid_populate_input_header(buf->header, &header);
+ spi_hid_populate_input_body(buf->body, &body);
+ report->report_type = body.input_report_type;
+ report->content_length = body.content_len;
+ report->content_id = body.content_id;
+ report->content = buf->content;
+}
+
static void spi_hid_populate_output_header(u8 *buf,
const struct spi_hid_conf *conf,
const struct spi_hid_output_report *report)
@@ -157,6 +277,33 @@ static void spi_hid_populate_output_header(u8 *buf,
buf[7] = report->content_id;
}
+static int spi_hid_input_sync(struct spi_hid *shid, void *buf, u16 length,
+ bool is_header)
+{
+ int error;
+
+ shid->input_transfer[0].tx_buf = is_header ?
+ shid->read_approval_header :
+ shid->read_approval_body;
+ shid->input_transfer[0].len = SPI_HID_READ_APPROVAL_LEN;
+
+ shid->input_transfer[1].rx_buf = buf;
+ shid->input_transfer[1].len = length;
+
+ spi_message_init_with_transfers(&shid->input_message,
+ shid->input_transfer, 2);
+
+ error = spi_sync(shid->spi, &shid->input_message);
+ if (error) {
+ dev_err(&shid->spi->dev, "Error starting sync transfer: %d.", error);
+ shid->bus_error_count++;
+ shid->bus_last_error = error;
+ return error;
+ }
+
+ return 0;
+}
+
static int spi_hid_output(struct spi_hid *shid, const void *buf, u16 length)
{
int error;
@@ -196,6 +343,50 @@ static void spi_hid_stop_hid(struct spi_hid *shid)
hid_destroy_device(hid);
}
+static void spi_hid_error(struct spi_hid *shid)
+{
+ struct device *dev = &shid->spi->dev;
+ int error;
+
+ guard(mutex)(&shid->power_lock);
+ if (shid->power_state == HIDSPI_OFF)
+ return;
+
+ if (shid->reset_attempts++ >= SPI_HID_MAX_RESET_ATTEMPTS) {
+ dev_err(dev, "unresponsive device, aborting.");
+ spi_hid_stop_hid(shid);
+ shid->ops->assert_reset(shid->ops);
+ error = shid->ops->power_down(shid->ops);
+ if (error) {
+ dev_err(dev, "failed to disable regulator.");
+ shid->regulator_error_count++;
+ shid->regulator_last_error = error;
+ }
+ return;
+ }
+
+ clear_bit(SPI_HID_READY, &shid->flags);
+ set_bit(SPI_HID_RESET_PENDING, &shid->flags);
+
+ shid->ops->assert_reset(shid->ops);
+
+ shid->power_state = HIDSPI_OFF;
+
+ /*
+ * We want to cancel pending reset work as the device is being reset
+ * to recover from an error. cancel_work_sync will put us in a deadlock
+ * because this function is scheduled in 'reset_work' and we should
+ * avoid waiting for itself.
+ */
+ cancel_work(&shid->reset_work);
+
+ shid->ops->sleep_minimal_reset_delay(shid->ops);
+
+ shid->power_state = HIDSPI_ON;
+
+ shid->ops->deassert_reset(shid->ops);
+}
+
static int spi_hid_send_output_report(struct spi_hid *shid,
struct spi_hid_output_report *report)
{
@@ -206,13 +397,13 @@ static int spi_hid_send_output_report(struct spi_hid *shid,
u8 padding;
int error;
- guard(mutex)(&shid->output_lock);
if (report->content_length > shid->desc.max_output_length) {
dev_err(dev, "Output report too big, content_length 0x%x.",
report->content_length);
return -E2BIG;
}
+ guard(mutex)(&shid->io_lock);
spi_hid_populate_output_header(buf->header, shid->conf, report);
if (report->content_length)
@@ -236,6 +427,7 @@ static int spi_hid_sync_request(struct spi_hid *shid,
struct device *dev = &shid->spi->dev;
int error;
+ guard(mutex)(&shid->output_lock);
error = spi_hid_send_output_report(shid, report);
if (error)
return error;
@@ -250,6 +442,86 @@ static int spi_hid_sync_request(struct spi_hid *shid,
return 0;
}
+/*
+ * Handle the reset response from the FW by sending a request for the device
+ * descriptor.
+ */
+static void spi_hid_reset_response(struct spi_hid *shid)
+{
+ struct device *dev = &shid->spi->dev;
+ struct spi_hid_output_report report = {
+ .report_type = DEVICE_DESCRIPTOR,
+ .content_length = 0x0,
+ .content_id = SPI_HID_OUTPUT_REPORT_CONTENT_ID_DESC_REQUEST,
+ .content = NULL,
+ };
+ int error;
+
+ if (test_bit(SPI_HID_READY, &shid->flags)) {
+ dev_err(dev, "Spontaneous FW reset!");
+ clear_bit(SPI_HID_READY, &shid->flags);
+ shid->dir_count++;
+ }
+
+ if (shid->power_state == HIDSPI_OFF)
+ return;
+
+ error = spi_hid_sync_request(shid, &report);
+ if (error) {
+ dev_WARN_ONCE(dev, true,
+ "Failed to send device descriptor request: %d.", error);
+ set_bit(SPI_HID_ERROR, &shid->flags);
+ schedule_work(&shid->reset_work);
+ }
+}
+
+static int spi_hid_input_report_handler(struct spi_hid *shid,
+ struct spi_hid_input_buf *buf)
+{
+ struct device *dev = &shid->spi->dev;
+ struct spi_hid_input_report r;
+ int error = 0;
+
+ if (!test_bit(SPI_HID_READY, &shid->flags) ||
+ test_bit(SPI_HID_REFRESH_IN_PROGRESS, &shid->flags) || !shid->hid) {
+ dev_err(dev, "HID not ready");
+ return 0;
+ }
+
+ spi_hid_input_report_prepare(buf, &r);
+
+ error = hid_input_report(shid->hid, HID_INPUT_REPORT,
+ r.content - 1, r.content_length + 1, 1);
+
+ if (error == -ENODEV || error == -EBUSY) {
+ dev_err(dev, "ignoring report --> %d.", error);
+ return 0;
+ } else if (error) {
+ dev_err(dev, "Bad input report: %d.", error);
+ }
+
+ return error;
+}
+
+static void spi_hid_response_handler(struct spi_hid *shid,
+ struct input_report_body_header *body)
+{
+ shid->response_length = body->content_len;
+ /* completion_done returns 0 if there are waiters, otherwise 1 */
+ if (completion_done(&shid->output_done)) {
+ dev_err(&shid->spi->dev, "Unexpected response report.");
+ } else {
+ if (body->input_report_type == REPORT_DESCRIPTOR_RESPONSE ||
+ body->input_report_type == GET_FEATURE_RESPONSE) {
+ memcpy(shid->response->body, shid->input->body,
+ sizeof(shid->input->body));
+ memcpy(shid->response->content, shid->input->content,
+ body->content_len);
+ }
+ complete(&shid->output_done);
+ }
+}
+
/*
* This function returns the length of the report descriptor, or a negative
* error code if something went wrong.
@@ -269,6 +541,8 @@ static int spi_hid_report_descriptor_request(struct spi_hid *shid)
if (ret) {
dev_err(dev,
"Expected report descriptor not received: %d.", ret);
+ set_bit(SPI_HID_ERROR, &shid->flags);
+ schedule_work(&shid->reset_work);
return ret;
}
@@ -323,6 +597,205 @@ static int spi_hid_create_device(struct spi_hid *shid)
return 0;
}
+static void spi_hid_refresh_device(struct spi_hid *shid)
+{
+ struct device *dev = &shid->spi->dev;
+ u32 new_crc32 = 0;
+ int error = 0;
+
+ error = spi_hid_report_descriptor_request(shid);
+ if (error < 0) {
+ dev_err(dev,
+ "%s: failed report descriptor request: %d",
+ __func__, error);
+ return;
+ }
+ new_crc32 = crc32_le(0, (unsigned char const *)shid->response->content,
+ (size_t)error);
+
+ /* Same report descriptor, so no need to create a new hid device. */
+ if (new_crc32 == shid->report_descriptor_crc32) {
+ set_bit(SPI_HID_READY, &shid->flags);
+ return;
+ }
+
+ shid->report_descriptor_crc32 = new_crc32;
+
+ set_bit(SPI_HID_REFRESH_IN_PROGRESS, &shid->flags);
+
+ spi_hid_stop_hid(shid);
+
+ error = spi_hid_create_device(shid);
+ if (error) {
+ dev_err(dev, "%s: Failed to create hid device: %d.", __func__, error);
+ return;
+ }
+
+ clear_bit(SPI_HID_REFRESH_IN_PROGRESS, &shid->flags);
+}
+
+static void spi_hid_reset_work(struct work_struct *work)
+{
+ struct spi_hid *shid =
+ container_of(work, struct spi_hid, reset_work);
+ struct device *dev = &shid->spi->dev;
+ int error = 0;
+
+ if (test_and_clear_bit(SPI_HID_RESET_RESPONSE, &shid->flags)) {
+ spi_hid_reset_response(shid);
+ return;
+ }
+
+ if (test_and_clear_bit(SPI_HID_CREATE_DEVICE, &shid->flags)) {
+ guard(mutex)(&shid->power_lock);
+ if (shid->power_state == HIDSPI_OFF) {
+ dev_err(dev, "%s: Powered off, returning", __func__);
+ return;
+ }
+
+ if (!shid->hid) {
+ error = spi_hid_create_device(shid);
+ if (error) {
+ dev_err(dev, "%s: Failed to create hid device: %d.",
+ __func__, error);
+ return;
+ }
+ } else {
+ spi_hid_refresh_device(shid);
+ }
+
+ return;
+ }
+
+ if (test_and_clear_bit(SPI_HID_ERROR, &shid->flags)) {
+ spi_hid_error(shid);
+ return;
+ }
+}
+
+static int spi_hid_process_input_report(struct spi_hid *shid,
+ struct spi_hid_input_buf *buf)
+{
+ struct spi_hid_input_header header;
+ struct input_report_body_header body;
+ struct device *dev = &shid->spi->dev;
+ struct hidspi_dev_descriptor *raw;
+
+ spi_hid_populate_input_header(buf->header, &header);
+ spi_hid_populate_input_body(buf->body, &body);
+
+ if (body.content_len > header.report_length) {
+ dev_err(dev, "Bad body length %d > %d.", body.content_len,
+ header.report_length);
+ return -EPROTO;
+ }
+
+ switch (body.input_report_type) {
+ case DATA:
+ return spi_hid_input_report_handler(shid, buf);
+ case RESET_RESPONSE:
+ clear_bit(SPI_HID_RESET_PENDING, &shid->flags);
+ set_bit(SPI_HID_RESET_RESPONSE, &shid->flags);
+ schedule_work(&shid->reset_work);
+ break;
+ case DEVICE_DESCRIPTOR_RESPONSE:
+ /* Mark the completion done to avoid timeout */
+ spi_hid_response_handler(shid, &body);
+
+ /* Reset attempts at every device descriptor fetch */
+ shid->reset_attempts = 0;
+ raw = (struct hidspi_dev_descriptor *)buf->content;
+
+ /* Validate device descriptor length before parsing */
+ if (body.content_len != HIDSPI_DEVICE_DESCRIPTOR_SIZE) {
+ dev_err(dev, "Invalid content length %d, expected %lu.",
+ body.content_len,
+ HIDSPI_DEVICE_DESCRIPTOR_SIZE);
+ return -EPROTO;
+ }
+
+ if (le16_to_cpu(raw->dev_desc_len) !=
+ HIDSPI_DEVICE_DESCRIPTOR_SIZE) {
+ dev_err(dev,
+ "Invalid wDeviceDescLength %d, expected %lu.",
+ raw->dev_desc_len,
+ HIDSPI_DEVICE_DESCRIPTOR_SIZE);
+ return -EPROTO;
+ }
+
+ spi_hid_parse_dev_desc(raw, &shid->desc);
+
+ if (shid->desc.hid_version != SPI_HID_SUPPORTED_VERSION) {
+ dev_err(dev,
+ "Unsupported device descriptor version %4x.",
+ shid->desc.hid_version);
+ return -EPROTONOSUPPORT;
+ }
+
+ set_bit(SPI_HID_CREATE_DEVICE, &shid->flags);
+ schedule_work(&shid->reset_work);
+
+ break;
+ case OUTPUT_REPORT_RESPONSE:
+ if (shid->desc.no_output_report_ack) {
+ dev_err(dev, "Unexpected output report response.");
+ break;
+ }
+ fallthrough;
+ case GET_FEATURE_RESPONSE:
+ case SET_FEATURE_RESPONSE:
+ case REPORT_DESCRIPTOR_RESPONSE:
+ spi_hid_response_handler(shid, &body);
+ break;
+ /*
+ * FIXME: sending GET_INPUT and COMMAND reports not supported, thus
+ * throw away responses to those, they should never come.
+ */
+ case GET_INPUT_REPORT_RESPONSE:
+ case COMMAND_RESPONSE:
+ dev_err(dev, "Not a supported report type: 0x%x.",
+ body.input_report_type);
+ break;
+ default:
+ dev_err(dev, "Unknown input report: 0x%x.", body.input_report_type);
+ return -EPROTO;
+ }
+
+ return 0;
+}
+
+static int spi_hid_bus_validate_header(struct spi_hid *shid,
+ struct spi_hid_input_header *header)
+{
+ struct device *dev = &shid->spi->dev;
+
+ if (header->version != SPI_HID_INPUT_HEADER_VERSION) {
+ dev_err(dev, "Unknown input report version (v 0x%x).",
+ header->version);
+ return -EINVAL;
+ }
+
+ if (shid->desc.max_input_length != 0 &&
+ header->report_length > shid->desc.max_input_length) {
+ dev_err(dev, "Input report body size %u > max expected of %u.",
+ header->report_length, shid->desc.max_input_length);
+ return -EMSGSIZE;
+ }
+
+ if (header->last_fragment_flag != 1) {
+ dev_err(dev, "Multi-fragment reports not supported.");
+ return -EOPNOTSUPP;
+ }
+
+ if (header->sync_const != SPI_HID_INPUT_HEADER_SYNC_BYTE) {
+ dev_err(dev, "Invalid input report sync constant (0x%x).",
+ header->sync_const);
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
static int spi_hid_get_request(struct spi_hid *shid, u8 content_id)
{
struct device *dev = &shid->spi->dev;
@@ -339,6 +812,8 @@ static int spi_hid_get_request(struct spi_hid *shid, u8 content_id)
dev_err(dev,
"Expected get request response not received! Error %d.",
error);
+ set_bit(SPI_HID_ERROR, &shid->flags);
+ schedule_work(&shid->reset_work);
return error;
}
@@ -358,9 +833,83 @@ static int spi_hid_set_request(struct spi_hid *shid, u8 *arg_buf, u16 arg_len,
return spi_hid_sync_request(shid, &report);
}
-/* This is a placeholder. Will be implemented in the next patch. */
static irqreturn_t spi_hid_dev_irq(int irq, void *_shid)
{
+ struct spi_hid *shid = _shid;
+ struct device *dev = &shid->spi->dev;
+ struct spi_hid_input_header header;
+ int error = 0;
+
+ scoped_guard(mutex, &shid->io_lock) {
+ error = spi_hid_input_sync(shid, shid->input->header,
+ sizeof(shid->input->header), true);
+ if (error) {
+ dev_err(dev, "Failed to transfer header: %d.", error);
+ goto err;
+ }
+
+ if (shid->power_state == HIDSPI_OFF) {
+ dev_warn(dev, "Device is off after header was received.");
+ goto out;
+ }
+
+ if (shid->input_message.status < 0) {
+ dev_warn(dev, "Error reading header: %d.",
+ shid->input_message.status);
+ shid->bus_error_count++;
+ shid->bus_last_error = shid->input_message.status;
+ goto err;
+ }
+
+ spi_hid_populate_input_header(shid->input->header, &header);
+
+ error = spi_hid_bus_validate_header(shid, &header);
+ if (error) {
+ if (!test_bit(SPI_HID_RESET_PENDING, &shid->flags)) {
+ dev_err(dev, "Failed to validate header: %d.", error);
+ print_hex_dump(KERN_ERR, "spi_hid: header buffer: ",
+ DUMP_PREFIX_NONE, 16, 1, shid->input->header,
+ sizeof(shid->input->header), false);
+ shid->bus_error_count++;
+ shid->bus_last_error = error;
+ goto err;
+ }
+ goto out;
+ }
+
+ error = spi_hid_input_sync(shid, shid->input->body, header.report_length,
+ false);
+ if (error) {
+ dev_err(dev, "Failed to transfer body: %d.", error);
+ goto err;
+ }
+
+ if (shid->power_state == HIDSPI_OFF) {
+ dev_warn(dev, "Device is off after body was received.");
+ goto out;
+ }
+
+ if (shid->input_message.status < 0) {
+ dev_warn(dev, "Error reading body: %d.",
+ shid->input_message.status);
+ shid->bus_error_count++;
+ shid->bus_last_error = shid->input_message.status;
+ goto err;
+ }
+ }
+
+ error = spi_hid_process_input_report(shid, shid->input);
+ if (error) {
+ dev_err(dev, "Failed to process input report: %d.", error);
+ goto err;
+ }
+
+out:
+ return IRQ_HANDLED;
+
+err:
+ set_bit(SPI_HID_ERROR, &shid->flags);
+ schedule_work(&shid->reset_work);
return IRQ_HANDLED;
}
@@ -571,10 +1120,13 @@ static int spi_hid_ll_output_report(struct hid_device *hid, __u8 *buf,
return -ENODEV;
}
- if (shid->desc.no_output_report_ack)
- error = spi_hid_send_output_report(shid, &report);
- else
+ if (shid->desc.no_output_report_ack) {
+ scoped_guard(mutex, &shid->output_lock) {
+ error = spi_hid_send_output_report(shid, &report);
+ }
+ } else {
error = spi_hid_sync_request(shid, &report);
+ }
if (error) {
dev_err(dev, "failed to send output report.");
@@ -662,11 +1214,23 @@ int spi_hid_core_probe(struct spi_device *spi, struct spihid_ops *ops,
shid->power_state = HIDSPI_ON;
shid->ops = ops;
shid->conf = conf;
+ set_bit(SPI_HID_RESET_PENDING, &shid->flags);
spi_set_drvdata(spi, shid);
+ /* Using now populated conf let's pre-calculate the read approvals */
+ spi_hid_populate_read_approvals(shid->conf, shid->read_approval_header,
+ shid->read_approval_body);
+
+ mutex_init(&shid->output_lock);
+ mutex_init(&shid->power_lock);
+ mutex_init(&shid->io_lock);
+ init_completion(&shid->output_done);
+
+ INIT_WORK(&shid->reset_work, spi_hid_reset_work);
+
/*
- * we need to allocate the buffer without knowing the maximum
+ * We need to allocate the buffer without knowing the maximum
* size of the reports. Let's use SZ_2K, then we do the
* real computation later.
*/
@@ -706,8 +1270,6 @@ int spi_hid_core_probe(struct spi_device *spi, struct spihid_ops *ops,
dev_dbg(dev, "%s: d3 -> %s.", __func__,
spi_hid_power_mode_string(shid->power_state));
- spi_hid_create_device(shid);
-
return 0;
}
EXPORT_SYMBOL_GPL(spi_hid_core_probe);
--
2.53.0.1185.g05d4b7b318-goog
^ permalink raw reply related
* [PATCH v3 04/11] HID: spi-hid: add spi-hid driver HID layer
From: Jingyuan Liang @ 2026-04-02 1:59 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Jonathan Corbet, Mark Brown,
Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: linux-input, linux-doc, linux-kernel, linux-spi,
linux-trace-kernel, devicetree, hbarnor, tfiga, Jingyuan Liang,
Dmitry Antipov, Angela Czubak
In-Reply-To: <20260402-send-upstream-v3-0-6091c458d357@chromium.org>
Add HID low level driver callbacks to register SPI as a HID driver, and
an external touch device as a HID device.
Signed-off-by: Dmitry Antipov <dmanti@microsoft.com>
Signed-off-by: Angela Czubak <acz@semihalf.com>
Signed-off-by: Jingyuan Liang <jingyliang@chromium.org>
---
drivers/hid/spi-hid/spi-hid-core.c | 519 +++++++++++++++++++++++++++++++++++++
1 file changed, 519 insertions(+)
diff --git a/drivers/hid/spi-hid/spi-hid-core.c b/drivers/hid/spi-hid/spi-hid-core.c
index d7b4d4adad95..4723b87346d4 100644
--- a/drivers/hid/spi-hid/spi-hid-core.c
+++ b/drivers/hid/spi-hid/spi-hid-core.c
@@ -20,13 +20,69 @@
* Copyright (c) 2006-2010 Jiri Kosina
*/
+#include <linux/completion.h>
+#include <linux/crc32.h>
#include <linux/device.h>
+#include <linux/err.h>
#include <linux/hid.h>
#include <linux/hid-over-spi.h>
#include <linux/interrupt.h>
+#include <linux/jiffies.h>
#include <linux/module.h>
+#include <linux/mutex.h>
#include <linux/slab.h>
#include <linux/spi/spi.h>
+#include <linux/string.h>
+#include <linux/sysfs.h>
+#include <linux/unaligned.h>
+
+#define SPI_HID_OUTPUT_REPORT_CONTENT_ID_DESC_REQUEST 0x00
+
+#define SPI_HID_RESP_TIMEOUT 1000
+
+/* Protocol message size constants */
+#define SPI_HID_OUTPUT_HEADER_LEN 8
+
+/* flags */
+/*
+ * ready flag indicates that the FW is ready to accept commands and
+ * requests. The FW becomes ready after sending the report descriptor.
+ */
+#define SPI_HID_READY 0
+
+/* Raw input buffer with data from the bus */
+struct spi_hid_input_buf {
+ u8 header[HIDSPI_INPUT_HEADER_SIZE];
+ u8 body[HIDSPI_INPUT_BODY_HEADER_SIZE];
+ u8 content[];
+};
+
+/* Raw output report buffer to be put on the bus */
+struct spi_hid_output_buf {
+ u8 header[SPI_HID_OUTPUT_HEADER_LEN];
+ u8 content[];
+};
+
+/* Data necessary to send an output report */
+struct spi_hid_output_report {
+ u8 report_type;
+ u16 content_length;
+ u8 content_id;
+ u8 *content;
+};
+
+/* Processed data from a device descriptor */
+struct spi_hid_device_descriptor {
+ u16 hid_version;
+ u16 report_descriptor_length;
+ u16 max_input_length;
+ u16 max_output_length;
+ u16 max_fragment_length;
+ u16 vendor_id;
+ u16 product_id;
+ u16 version_id;
+ u8 no_output_report_ack;
+};
/* struct spi_hid_conf - Conf provided to the core */
struct spi_hid_conf {
@@ -61,8 +117,26 @@ struct spi_hid {
struct spihid_ops *ops;
struct spi_hid_conf *conf;
+ struct spi_hid_device_descriptor desc; /* HID device descriptor. */
+ struct spi_hid_output_buf *output; /* Output buffer. */
+ struct spi_hid_input_buf *input; /* Input buffer. */
+ struct spi_hid_input_buf *response; /* Response buffer. */
+
+ u16 response_length;
+ u16 bufsize;
+
enum hidspi_power_state power_state;
+ u8 reset_attempts; /* The number of reset attempts. */
+
+ unsigned long flags; /* device flags. */
+
+ /* Control lock to make sure one output transaction at a time. */
+ struct mutex output_lock;
+ struct completion output_done;
+
+ u32 report_descriptor_crc32; /* HID report descriptor crc32 checksum. */
+
u32 regulator_error_count;
int regulator_last_error;
u32 bus_error_count;
@@ -70,6 +144,33 @@ struct spi_hid {
u32 dir_count; /* device initiated reset count. */
};
+static struct hid_ll_driver spi_hid_ll_driver;
+
+static void spi_hid_populate_output_header(u8 *buf,
+ const struct spi_hid_conf *conf,
+ const struct spi_hid_output_report *report)
+{
+ buf[0] = conf->write_opcode;
+ put_unaligned_be24(conf->output_report_address, &buf[1]);
+ buf[4] = report->report_type;
+ put_unaligned_le16(report->content_length, &buf[5]);
+ buf[7] = report->content_id;
+}
+
+static int spi_hid_output(struct spi_hid *shid, const void *buf, u16 length)
+{
+ int error;
+
+ error = spi_write(shid->spi, buf, length);
+
+ if (error) {
+ shid->bus_error_count++;
+ shid->bus_last_error = error;
+ }
+
+ return error;
+}
+
static const char *spi_hid_power_mode_string(enum hidspi_power_state power_state)
{
switch (power_state) {
@@ -84,11 +185,416 @@ static const char *spi_hid_power_mode_string(enum hidspi_power_state power_state
}
}
+static void spi_hid_stop_hid(struct spi_hid *shid)
+{
+ struct hid_device *hid = shid->hid;
+
+ shid->hid = NULL;
+ clear_bit(SPI_HID_READY, &shid->flags);
+
+ if (hid)
+ hid_destroy_device(hid);
+}
+
+static int spi_hid_send_output_report(struct spi_hid *shid,
+ struct spi_hid_output_report *report)
+{
+ struct spi_hid_output_buf *buf = shid->output;
+ struct device *dev = &shid->spi->dev;
+ u16 report_length;
+ u16 padded_length;
+ u8 padding;
+ int error;
+
+ guard(mutex)(&shid->output_lock);
+ if (report->content_length > shid->desc.max_output_length) {
+ dev_err(dev, "Output report too big, content_length 0x%x.",
+ report->content_length);
+ return -E2BIG;
+ }
+
+ spi_hid_populate_output_header(buf->header, shid->conf, report);
+
+ if (report->content_length)
+ memcpy(&buf->content, report->content, report->content_length);
+
+ report_length = sizeof(buf->header) + report->content_length;
+ padded_length = round_up(report_length, 4);
+ padding = padded_length - report_length;
+ memset(&buf->content[report->content_length], 0, padding);
+
+ error = spi_hid_output(shid, buf, padded_length);
+ if (error)
+ dev_err(dev, "Failed output transfer: %d.", error);
+
+ return error;
+}
+
+static int spi_hid_sync_request(struct spi_hid *shid,
+ struct spi_hid_output_report *report)
+{
+ struct device *dev = &shid->spi->dev;
+ int error;
+
+ error = spi_hid_send_output_report(shid, report);
+ if (error)
+ return error;
+
+ error = wait_for_completion_interruptible_timeout(&shid->output_done,
+ msecs_to_jiffies(SPI_HID_RESP_TIMEOUT));
+ if (error == 0) {
+ dev_err(dev, "Response timed out.");
+ return -ETIMEDOUT;
+ }
+
+ return 0;
+}
+
+/*
+ * This function returns the length of the report descriptor, or a negative
+ * error code if something went wrong.
+ */
+static int spi_hid_report_descriptor_request(struct spi_hid *shid)
+{
+ struct device *dev = &shid->spi->dev;
+ struct spi_hid_output_report report = {
+ .report_type = REPORT_DESCRIPTOR,
+ .content_length = 0,
+ .content_id = SPI_HID_OUTPUT_REPORT_CONTENT_ID_DESC_REQUEST,
+ .content = NULL,
+ };
+ int ret;
+
+ ret = spi_hid_sync_request(shid, &report);
+ if (ret) {
+ dev_err(dev,
+ "Expected report descriptor not received: %d.", ret);
+ return ret;
+ }
+
+ ret = shid->response_length;
+ if (ret != shid->desc.report_descriptor_length) {
+ ret = min_t(unsigned int, ret, shid->desc.report_descriptor_length);
+ dev_err(dev, "Received report descriptor length doesn't match device descriptor field, using min of the two: %d.",
+ ret);
+ }
+
+ return ret;
+}
+
+static int spi_hid_create_device(struct spi_hid *shid)
+{
+ struct hid_device *hid;
+ struct device *dev = &shid->spi->dev;
+ int error;
+
+ hid = hid_allocate_device();
+ error = PTR_ERR_OR_ZERO(hid);
+ if (error) {
+ dev_err(dev, "Failed to allocate hid device: %d.", error);
+ return error;
+ }
+
+ hid->driver_data = shid->spi;
+ hid->ll_driver = &spi_hid_ll_driver;
+ hid->dev.parent = &shid->spi->dev;
+ hid->bus = BUS_SPI;
+ hid->version = shid->desc.hid_version;
+ hid->vendor = shid->desc.vendor_id;
+ hid->product = shid->desc.product_id;
+
+ snprintf(hid->name, sizeof(hid->name), "spi %04X:%04X",
+ hid->vendor, hid->product);
+ strscpy(hid->phys, dev_name(&shid->spi->dev), sizeof(hid->phys));
+
+ shid->hid = hid;
+
+ error = hid_add_device(hid);
+ if (error) {
+ dev_err(dev, "Failed to add hid device: %d.", error);
+ /*
+ * We likely got here because report descriptor request timed
+ * out. Let's disconnect and destroy the hid_device structure.
+ */
+ spi_hid_stop_hid(shid);
+ return error;
+ }
+
+ return 0;
+}
+
+static int spi_hid_get_request(struct spi_hid *shid, u8 content_id)
+{
+ struct device *dev = &shid->spi->dev;
+ struct spi_hid_output_report report = {
+ .report_type = GET_FEATURE,
+ .content_length = 0,
+ .content_id = content_id,
+ .content = NULL,
+ };
+ int error;
+
+ error = spi_hid_sync_request(shid, &report);
+ if (error) {
+ dev_err(dev,
+ "Expected get request response not received! Error %d.",
+ error);
+ return error;
+ }
+
+ return 0;
+}
+
+static int spi_hid_set_request(struct spi_hid *shid, u8 *arg_buf, u16 arg_len,
+ u8 content_id)
+{
+ struct spi_hid_output_report report = {
+ .report_type = SET_FEATURE,
+ .content_length = arg_len,
+ .content_id = content_id,
+ .content = arg_buf,
+ };
+
+ return spi_hid_sync_request(shid, &report);
+}
+
+/* This is a placeholder. Will be implemented in the next patch. */
static irqreturn_t spi_hid_dev_irq(int irq, void *_shid)
{
return IRQ_HANDLED;
}
+static int spi_hid_alloc_buffers(struct spi_hid *shid, size_t report_size)
+{
+ struct device *dev = &shid->spi->dev;
+ int inbufsize = sizeof(shid->input->header) + sizeof(shid->input->body) + report_size;
+ int outbufsize = sizeof(shid->output->header) + report_size;
+
+ // devm_krealloc with __GFP_ZERO ensures the new memory is initialized
+ shid->output = devm_krealloc(dev, shid->output, outbufsize, GFP_KERNEL | __GFP_ZERO);
+ shid->input = devm_krealloc(dev, shid->input, inbufsize, GFP_KERNEL | __GFP_ZERO);
+ shid->response = devm_krealloc(dev, shid->response, inbufsize, GFP_KERNEL | __GFP_ZERO);
+
+ if (!shid->output || !shid->input || !shid->response)
+ return -ENOMEM;
+
+ shid->bufsize = report_size;
+
+ return 0;
+}
+
+static int spi_hid_get_report_length(struct hid_report *report)
+{
+ return ((report->size - 1) >> 3) + 1 +
+ report->device->report_enum[report->type].numbered + 2;
+}
+
+/*
+ * Traverse the supplied list of reports and find the longest
+ */
+static void spi_hid_find_max_report(struct hid_device *hid, u32 type,
+ u16 *max)
+{
+ struct hid_report *report;
+ u16 size;
+
+ /*
+ * We should not rely on wMaxInputLength, as some devices may set it to
+ * a wrong length.
+ */
+ list_for_each_entry(report, &hid->report_enum[type].report_list, list) {
+ size = spi_hid_get_report_length(report);
+ if (*max < size)
+ *max = size;
+ }
+}
+
+/* hid_ll_driver interface functions */
+
+static int spi_hid_ll_start(struct hid_device *hid)
+{
+ struct spi_device *spi = hid->driver_data;
+ struct spi_hid *shid = spi_get_drvdata(spi);
+ int error = 0;
+ u16 bufsize = 0;
+
+ spi_hid_find_max_report(hid, HID_INPUT_REPORT, &bufsize);
+ spi_hid_find_max_report(hid, HID_OUTPUT_REPORT, &bufsize);
+ spi_hid_find_max_report(hid, HID_FEATURE_REPORT, &bufsize);
+
+ if (bufsize < HID_MIN_BUFFER_SIZE) {
+ dev_err(&spi->dev,
+ "HID_MIN_BUFFER_SIZE > max_input_length (%d).",
+ bufsize);
+ return -EINVAL;
+ }
+
+ if (bufsize > shid->bufsize) {
+ guard(disable_irq)(&shid->spi->irq);
+
+ error = spi_hid_alloc_buffers(shid, bufsize);
+ if (error)
+ return error;
+ }
+
+ return 0;
+}
+
+static void spi_hid_ll_stop(struct hid_device *hid)
+{
+ hid->claimed = 0;
+}
+
+static int spi_hid_ll_open(struct hid_device *hid)
+{
+ struct spi_device *spi = hid->driver_data;
+ struct spi_hid *shid = spi_get_drvdata(spi);
+
+ set_bit(SPI_HID_READY, &shid->flags);
+ return 0;
+}
+
+static void spi_hid_ll_close(struct hid_device *hid)
+{
+ struct spi_device *spi = hid->driver_data;
+ struct spi_hid *shid = spi_get_drvdata(spi);
+
+ clear_bit(SPI_HID_READY, &shid->flags);
+ shid->reset_attempts = 0;
+}
+
+static int spi_hid_ll_power(struct hid_device *hid, int level)
+{
+ struct spi_device *spi = hid->driver_data;
+ struct spi_hid *shid = spi_get_drvdata(spi);
+ int error = 0;
+
+ guard(mutex)(&shid->output_lock);
+ if (!shid->hid)
+ error = -ENODEV;
+
+ return error;
+}
+
+static int spi_hid_ll_parse(struct hid_device *hid)
+{
+ struct spi_device *spi = hid->driver_data;
+ struct spi_hid *shid = spi_get_drvdata(spi);
+ struct device *dev = &spi->dev;
+ int error, len;
+
+ len = spi_hid_report_descriptor_request(shid);
+ if (len < 0) {
+ dev_err(dev, "Report descriptor request failed, %d.", len);
+ return len;
+ }
+
+ /*
+ * FIXME: below call returning 0 doesn't mean that the report descriptor
+ * is good. We might be caching a crc32 of a corrupted r. d. or who
+ * knows what the FW sent. Need to have a feedback loop about r. d.
+ * being ok and only then cache it.
+ */
+ error = hid_parse_report(hid, (u8 *)shid->response->content, len);
+ if (error) {
+ dev_err(dev, "failed parsing report: %d.", error);
+ return error;
+ }
+ shid->report_descriptor_crc32 = crc32_le(0,
+ (unsigned char const *)shid->response->content,
+ len);
+
+ return 0;
+}
+
+static int spi_hid_ll_raw_request(struct hid_device *hid,
+ unsigned char reportnum, __u8 *buf,
+ size_t len, unsigned char rtype, int reqtype)
+{
+ struct spi_device *spi = hid->driver_data;
+ struct spi_hid *shid = spi_get_drvdata(spi);
+ struct device *dev = &spi->dev;
+ int ret;
+
+ switch (reqtype) {
+ case HID_REQ_SET_REPORT:
+ if (buf[0] != reportnum) {
+ dev_err(dev, "report id mismatch.");
+ return -EINVAL;
+ }
+
+ ret = spi_hid_set_request(shid, &buf[1], len - 1,
+ reportnum);
+ if (ret) {
+ dev_err(dev, "failed to set report.");
+ return ret;
+ }
+
+ ret = len;
+ break;
+ case HID_REQ_GET_REPORT:
+ ret = spi_hid_get_request(shid, reportnum);
+ if (ret) {
+ dev_err(dev, "failed to get report.");
+ return ret;
+ }
+
+ ret = min_t(size_t, len,
+ (shid->response->body[1] | (shid->response->body[2] << 8)) + 1);
+ buf[0] = shid->response->body[3];
+ memcpy(&buf[1], &shid->response->content, ret);
+ break;
+ default:
+ dev_err(dev, "invalid request type.");
+ return -EIO;
+ }
+
+ return ret;
+}
+
+static int spi_hid_ll_output_report(struct hid_device *hid, __u8 *buf,
+ size_t len)
+{
+ struct spi_device *spi = hid->driver_data;
+ struct spi_hid *shid = spi_get_drvdata(spi);
+ struct device *dev = &spi->dev;
+ struct spi_hid_output_report report = {
+ .report_type = OUTPUT_REPORT,
+ .content_length = len - 1,
+ .content_id = buf[0],
+ .content = &buf[1],
+ };
+ int error;
+
+ if (!test_bit(SPI_HID_READY, &shid->flags)) {
+ dev_err(dev, "%s called in unready state", __func__);
+ return -ENODEV;
+ }
+
+ if (shid->desc.no_output_report_ack)
+ error = spi_hid_send_output_report(shid, &report);
+ else
+ error = spi_hid_sync_request(shid, &report);
+
+ if (error) {
+ dev_err(dev, "failed to send output report.");
+ return error;
+ }
+
+ return len;
+}
+
+static struct hid_ll_driver spi_hid_ll_driver = {
+ .start = spi_hid_ll_start,
+ .stop = spi_hid_ll_stop,
+ .open = spi_hid_ll_open,
+ .close = spi_hid_ll_close,
+ .power = spi_hid_ll_power,
+ .parse = spi_hid_ll_parse,
+ .output_report = spi_hid_ll_output_report,
+ .raw_request = spi_hid_ll_raw_request,
+};
+
static ssize_t bus_error_count_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
@@ -159,6 +665,15 @@ int spi_hid_core_probe(struct spi_device *spi, struct spihid_ops *ops,
spi_set_drvdata(spi, shid);
+ /*
+ * we need to allocate the buffer without knowing the maximum
+ * size of the reports. Let's use SZ_2K, then we do the
+ * real computation later.
+ */
+ error = spi_hid_alloc_buffers(shid, SZ_2K);
+ if (error)
+ return error;
+
/*
* At the end of probe we initialize the device:
* 0) assert reset, bias the interrupt line
@@ -191,6 +706,8 @@ int spi_hid_core_probe(struct spi_device *spi, struct spihid_ops *ops,
dev_dbg(dev, "%s: d3 -> %s.", __func__,
spi_hid_power_mode_string(shid->power_state));
+ spi_hid_create_device(shid);
+
return 0;
}
EXPORT_SYMBOL_GPL(spi_hid_core_probe);
@@ -201,6 +718,8 @@ void spi_hid_core_remove(struct spi_device *spi)
struct device *dev = &spi->dev;
int error;
+ spi_hid_stop_hid(shid);
+
shid->ops->assert_reset(shid->ops);
error = shid->ops->power_down(shid->ops);
if (error)
--
2.53.0.1185.g05d4b7b318-goog
^ permalink raw reply related
* [PATCH v3 03/11] HID: spi-hid: add transport driver skeleton for HID over SPI bus
From: Jingyuan Liang @ 2026-04-02 1:59 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Jonathan Corbet, Mark Brown,
Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: linux-input, linux-doc, linux-kernel, linux-spi,
linux-trace-kernel, devicetree, hbarnor, tfiga, Jingyuan Liang,
Angela Czubak, Dmitry Antipov
In-Reply-To: <20260402-send-upstream-v3-0-6091c458d357@chromium.org>
From: Angela Czubak <acz@semihalf.com>
Create spi-hid folder and add Kconfig and Makefile for spi-hid driver.
Add basic device structure, definitions, and probe/remove functions.
Signed-off-by: Dmitry Antipov <dmanti@microsoft.com>
Signed-off-by: Angela Czubak <acz@semihalf.com>
Signed-off-by: Jingyuan Liang <jingyliang@chromium.org>
---
drivers/hid/Kconfig | 2 +
drivers/hid/Makefile | 2 +
drivers/hid/spi-hid/Kconfig | 15 +++
drivers/hid/spi-hid/Makefile | 9 ++
drivers/hid/spi-hid/spi-hid-core.c | 213 +++++++++++++++++++++++++++++++++++++
5 files changed, 241 insertions(+)
diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index 920a64b66b25..c6ae23bfb75d 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -1434,6 +1434,8 @@ source "drivers/hid/bpf/Kconfig"
source "drivers/hid/i2c-hid/Kconfig"
+source "drivers/hid/spi-hid/Kconfig"
+
source "drivers/hid/intel-ish-hid/Kconfig"
source "drivers/hid/amd-sfh-hid/Kconfig"
diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
index 361a7daedeb8..6b43e789b39a 100644
--- a/drivers/hid/Makefile
+++ b/drivers/hid/Makefile
@@ -169,6 +169,8 @@ obj-$(CONFIG_USB_KBD) += usbhid/
obj-$(CONFIG_I2C_HID_CORE) += i2c-hid/
+obj-$(CONFIG_SPI_HID_CORE) += spi-hid/
+
obj-$(CONFIG_INTEL_ISH_HID) += intel-ish-hid/
obj-$(CONFIG_AMD_SFH_HID) += amd-sfh-hid/
diff --git a/drivers/hid/spi-hid/Kconfig b/drivers/hid/spi-hid/Kconfig
new file mode 100644
index 000000000000..836fdefe8345
--- /dev/null
+++ b/drivers/hid/spi-hid/Kconfig
@@ -0,0 +1,15 @@
+# SPDX-License-Identifier: GPL-2.0-only
+#
+# Copyright (c) 2021 Microsoft Corporation
+#
+
+menuconfig SPI_HID
+ tristate "SPI HID support"
+ default y
+ depends on SPI
+
+if SPI_HID
+
+config SPI_HID_CORE
+ tristate
+endif
diff --git a/drivers/hid/spi-hid/Makefile b/drivers/hid/spi-hid/Makefile
new file mode 100644
index 000000000000..92e24cddbfc2
--- /dev/null
+++ b/drivers/hid/spi-hid/Makefile
@@ -0,0 +1,9 @@
+# SPDX-License-Identifier: GPL-2.0-only
+#
+# Makefile for the SPI HID input drivers
+#
+# Copyright (c) 2021 Microsoft Corporation
+#
+
+obj-$(CONFIG_SPI_HID_CORE) += spi-hid.o
+spi-hid-objs = spi-hid-core.o
diff --git a/drivers/hid/spi-hid/spi-hid-core.c b/drivers/hid/spi-hid/spi-hid-core.c
new file mode 100644
index 000000000000..d7b4d4adad95
--- /dev/null
+++ b/drivers/hid/spi-hid/spi-hid-core.c
@@ -0,0 +1,213 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * HID over SPI protocol implementation
+ *
+ * Copyright (c) 2021 Microsoft Corporation
+ * Copyright (c) 2026 Google LLC
+ *
+ * This code is partly based on "HID over I2C protocol implementation:
+ *
+ * Copyright (c) 2012 Benjamin Tissoires <benjamin.tissoires@gmail.com>
+ * Copyright (c) 2012 Ecole Nationale de l'Aviation Civile, France
+ * Copyright (c) 2012 Red Hat, Inc
+ *
+ * which in turn is partly based on "USB HID support for Linux":
+ *
+ * Copyright (c) 1999 Andreas Gal
+ * Copyright (c) 2000-2005 Vojtech Pavlik <vojtech@suse.cz>
+ * Copyright (c) 2005 Michael Haboustak <mike-@cinci.rr.com> for Concept2, Inc
+ * Copyright (c) 2007-2008 Oliver Neukum
+ * Copyright (c) 2006-2010 Jiri Kosina
+ */
+
+#include <linux/device.h>
+#include <linux/hid.h>
+#include <linux/hid-over-spi.h>
+#include <linux/interrupt.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/spi/spi.h>
+
+/* struct spi_hid_conf - Conf provided to the core */
+struct spi_hid_conf {
+ u32 input_report_header_address;
+ u32 input_report_body_address;
+ u32 output_report_address;
+ u8 read_opcode;
+ u8 write_opcode;
+};
+
+/**
+ * struct spihid_ops - Ops provided to the core
+ * @power_up: do sequencing to power up the device
+ * @power_down: do sequencing to power down the device
+ * @assert_reset: do sequencing to assert the reset line
+ * @deassert_reset: do sequencing to deassert the reset line
+ * @sleep_minimal_reset_delay: minimal sleep delay during reset
+ */
+struct spihid_ops {
+ int (*power_up)(struct spihid_ops *ops);
+ int (*power_down)(struct spihid_ops *ops);
+ int (*assert_reset)(struct spihid_ops *ops);
+ int (*deassert_reset)(struct spihid_ops *ops);
+ void (*sleep_minimal_reset_delay)(struct spihid_ops *ops);
+};
+
+/* Driver context */
+struct spi_hid {
+ struct spi_device *spi; /* spi device. */
+ struct hid_device *hid; /* pointer to corresponding HID dev. */
+
+ struct spihid_ops *ops;
+ struct spi_hid_conf *conf;
+
+ enum hidspi_power_state power_state;
+
+ u32 regulator_error_count;
+ int regulator_last_error;
+ u32 bus_error_count;
+ int bus_last_error;
+ u32 dir_count; /* device initiated reset count. */
+};
+
+static const char *spi_hid_power_mode_string(enum hidspi_power_state power_state)
+{
+ switch (power_state) {
+ case HIDSPI_ON:
+ return "d0";
+ case HIDSPI_SLEEP:
+ return "d2";
+ case HIDSPI_OFF:
+ return "d3";
+ default:
+ return "unknown";
+ }
+}
+
+static irqreturn_t spi_hid_dev_irq(int irq, void *_shid)
+{
+ return IRQ_HANDLED;
+}
+
+static ssize_t bus_error_count_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct spi_hid *shid = dev_get_drvdata(dev);
+
+ return sysfs_emit(buf, "%d (%d)\n",
+ shid->bus_error_count, shid->bus_last_error);
+}
+static DEVICE_ATTR_RO(bus_error_count);
+
+static ssize_t regulator_error_count_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct spi_hid *shid = dev_get_drvdata(dev);
+
+ return sysfs_emit(buf, "%d (%d)\n",
+ shid->regulator_error_count,
+ shid->regulator_last_error);
+}
+static DEVICE_ATTR_RO(regulator_error_count);
+
+static ssize_t device_initiated_reset_count_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct spi_hid *shid = dev_get_drvdata(dev);
+
+ return sysfs_emit(buf, "%d\n", shid->dir_count);
+}
+static DEVICE_ATTR_RO(device_initiated_reset_count);
+
+static struct attribute *spi_hid_attrs[] = {
+ &dev_attr_bus_error_count.attr,
+ &dev_attr_regulator_error_count.attr,
+ &dev_attr_device_initiated_reset_count.attr,
+ NULL /* Terminator */
+};
+
+static const struct attribute_group spi_hid_group = {
+ .attrs = spi_hid_attrs,
+};
+
+const struct attribute_group *spi_hid_groups[] = {
+ &spi_hid_group,
+ NULL
+};
+EXPORT_SYMBOL_GPL(spi_hid_groups);
+
+int spi_hid_core_probe(struct spi_device *spi, struct spihid_ops *ops,
+ struct spi_hid_conf *conf)
+{
+ struct device *dev = &spi->dev;
+ struct spi_hid *shid;
+ int error;
+
+ if (spi->irq <= 0)
+ return dev_err_probe(dev, spi->irq ?: -EINVAL, "Missing IRQ\n");
+
+ shid = devm_kzalloc(dev, sizeof(*shid), GFP_KERNEL);
+ if (!shid)
+ return -ENOMEM;
+
+ shid->spi = spi;
+ shid->power_state = HIDSPI_ON;
+ shid->ops = ops;
+ shid->conf = conf;
+
+ spi_set_drvdata(spi, shid);
+
+ /*
+ * At the end of probe we initialize the device:
+ * 0) assert reset, bias the interrupt line
+ * 1) sleep minimal reset delay
+ * 2) request IRQ
+ * 3) power up the device
+ * 4) deassert reset (high)
+ * After this we expect an IRQ with a reset response.
+ */
+
+ shid->ops->assert_reset(shid->ops);
+
+ shid->ops->sleep_minimal_reset_delay(shid->ops);
+
+ error = devm_request_threaded_irq(dev, spi->irq, NULL, spi_hid_dev_irq,
+ IRQF_ONESHOT, dev_name(&spi->dev), shid);
+ if (error) {
+ dev_err(dev, "%s: unable to request threaded IRQ.", __func__);
+ return error;
+ }
+
+ error = shid->ops->power_up(shid->ops);
+ if (error) {
+ dev_err(dev, "%s: could not power up.", __func__);
+ return error;
+ }
+
+ shid->ops->deassert_reset(shid->ops);
+
+ dev_dbg(dev, "%s: d3 -> %s.", __func__,
+ spi_hid_power_mode_string(shid->power_state));
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(spi_hid_core_probe);
+
+void spi_hid_core_remove(struct spi_device *spi)
+{
+ struct spi_hid *shid = spi_get_drvdata(spi);
+ struct device *dev = &spi->dev;
+ int error;
+
+ shid->ops->assert_reset(shid->ops);
+ error = shid->ops->power_down(shid->ops);
+ if (error)
+ dev_err(dev, "failed to disable regulator.");
+}
+EXPORT_SYMBOL_GPL(spi_hid_core_remove);
+
+MODULE_DESCRIPTION("HID over SPI transport driver");
+MODULE_AUTHOR("Dmitry Antipov <dmanti@microsoft.com>");
+MODULE_LICENSE("GPL");
--
2.53.0.1185.g05d4b7b318-goog
^ permalink raw reply related
* [PATCH v3 02/11] HID: Add BUS_SPI support and define HID_SPI_DEVICE macro
From: Jingyuan Liang @ 2026-04-02 1:59 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Jonathan Corbet, Mark Brown,
Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: linux-input, linux-doc, linux-kernel, linux-spi,
linux-trace-kernel, devicetree, hbarnor, tfiga, Jingyuan Liang,
Jarrett Schultz, Dmitry Antipov
In-Reply-To: <20260402-send-upstream-v3-0-6091c458d357@chromium.org>
From: Jarrett Schultz <jaschultz@microsoft.com>
If connecting a hid_device with bus field indicating BUS_SPI print out
"SPI" in the debug print.
Macro sets the bus field to BUS_SPI and uses arguments to set vendor
product fields.
Signed-off-by: Dmitry Antipov <dmanti@microsoft.com>
Reviewed-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Signed-off-by: Jingyuan Liang <jingyliang@chromium.org>
---
drivers/hid/hid-core.c | 3 +++
include/linux/hid.h | 2 ++
2 files changed, 5 insertions(+)
diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index a5b3a8ca2fcb..813c9c743ccd 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -2316,6 +2316,9 @@ int hid_connect(struct hid_device *hdev, unsigned int connect_mask)
case BUS_I2C:
bus = "I2C";
break;
+ case BUS_SPI:
+ bus = "SPI";
+ break;
case BUS_SDW:
bus = "SOUNDWIRE";
break;
diff --git a/include/linux/hid.h b/include/linux/hid.h
index dce862cafbbd..957f322a0ebd 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -786,6 +786,8 @@ struct hid_descriptor {
.bus = BUS_BLUETOOTH, .vendor = (ven), .product = (prod)
#define HID_I2C_DEVICE(ven, prod) \
.bus = BUS_I2C, .vendor = (ven), .product = (prod)
+#define HID_SPI_DEVICE(ven, prod) \
+ .bus = BUS_SPI, .vendor = (ven), .product = (prod)
#define HID_REPORT_ID(rep) \
.report_type = (rep)
--
2.53.0.1185.g05d4b7b318-goog
^ permalink raw reply related
* [PATCH v3 01/11] Documentation: Correction in HID output_report callback description.
From: Jingyuan Liang @ 2026-04-02 1:59 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Jonathan Corbet, Mark Brown,
Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: linux-input, linux-doc, linux-kernel, linux-spi,
linux-trace-kernel, devicetree, hbarnor, tfiga, Jingyuan Liang,
Jarrett Schultz, Dmitry Antipov
In-Reply-To: <20260402-send-upstream-v3-0-6091c458d357@chromium.org>
From: Jarrett Schultz <jaschultz@microsoft.com>
Originally output_report callback was described as must-be asynchronous,
but that is not the case in some implementations, namely i2c-hid.
Correct the documentation to say that it may be asynchronous.
Signed-off-by: Dmitry Antipov <dmanti@microsoft.com>
Reviewed-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Signed-off-by: Jingyuan Liang <jingyliang@chromium.org>
---
Documentation/hid/hid-transport.rst | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/Documentation/hid/hid-transport.rst b/Documentation/hid/hid-transport.rst
index 6f1692da296c..2008cf432af1 100644
--- a/Documentation/hid/hid-transport.rst
+++ b/Documentation/hid/hid-transport.rst
@@ -327,8 +327,8 @@ The available HID callbacks are:
Send raw output report via intr channel. Used by some HID device drivers
which require high throughput for outgoing requests on the intr channel. This
- must not cause SET_REPORT calls! This must be implemented as asynchronous
- output report on the intr channel!
+ must not cause SET_REPORT calls! This call might be asynchronous, so the
+ caller should not expect an immediate response!
::
--
2.53.0.1185.g05d4b7b318-goog
^ permalink raw reply related
* [PATCH v3 00/11] Add spi-hid transport driver
From: Jingyuan Liang @ 2026-04-02 1:59 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Jonathan Corbet, Mark Brown,
Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: linux-input, linux-doc, linux-kernel, linux-spi,
linux-trace-kernel, devicetree, hbarnor, tfiga, Jingyuan Liang,
Jarrett Schultz, Dmitry Antipov, Angela Czubak
This series picks up the spi-hid driver work originally started by
Microsoft. The patch breakdown has been modified and the implementation
has been refactored to address upstream feedback and testing issues. We
are submitting this as a new series while keeping the original sign-off
chain to reflect the history.
Same as the original series, there is a change to HID documentation, some
HID core changes to support a SPI device, the SPI HID transport driver,
and HID over SPI Device Tree binding. We have added the HID over SPI ACPI
support, power management, panel follower, and quirks for Ilitek touch
controllers.
Original authors: Jarrett Schultz <jaschultz@microsoft.com>,
Dmitry Antipov <dmanti@microsoft.com>
Link: https://lore.kernel.org/r/86b63b7b-afda-d7f4-7bfa-175085d5a8ef@gmail.com
Signed-off-by: Jingyuan Liang <jingyliang@chromium.org>
---
Changes in v3:
- Add io_lock init
- Relocate tracepoints to drivers/hid/spi-hid/ and fix tracepoint macros
- Add tracepoints for sync, error handling, reset, and report processing
- Clean up internal includes and fix Makefile CFLAGS
- Add more details in v2 changelog
- Link to v2: https://lore.kernel.org/r/20260324-send-upstream-v2-0-521ce8afff86@chromium.org
Changes in v2:
- Clean up DT bindings: fix formatting and remove timing and flags properties
- Update DT binding example: use a device-specific compatible and drop
reset_assert
- Simplify ACPI/OF match tables by removing ACPI_PTR/of_match_ptr
- Refactor OF driver to use match data for timing parameters instead
of DT properties
- Switch to fsleep() for delays in ACPI and OF drivers
- Drop patch 12 as it is vendor specific
- Add a lock to fix input/output concurrency race
- Link to v1: https://lore.kernel.org/r/20260303-send-upstream-v1-0-1515ba218f3d@chromium.org
---
Angela Czubak (2):
HID: spi-hid: add transport driver skeleton for HID over SPI bus
HID: spi_hid: add ACPI support for SPI over HID
Jarrett Schultz (3):
Documentation: Correction in HID output_report callback description.
HID: Add BUS_SPI support and define HID_SPI_DEVICE macro
HID: spi_hid: add device tree support for SPI over HID
Jingyuan Liang (6):
HID: spi-hid: add spi-hid driver HID layer
HID: spi-hid: add HID SPI protocol implementation
HID: spi_hid: add spi_hid traces
dt-bindings: input: Document hid-over-spi DT schema
HID: spi-hid: add power management implementation
HID: spi-hid: add panel follower support
.../devicetree/bindings/input/hid-over-spi.yaml | 126 ++
Documentation/hid/hid-transport.rst | 4 +-
drivers/hid/Kconfig | 2 +
drivers/hid/Makefile | 2 +
drivers/hid/hid-core.c | 3 +
drivers/hid/spi-hid/Kconfig | 45 +
drivers/hid/spi-hid/Makefile | 12 +
drivers/hid/spi-hid/spi-hid-acpi.c | 254 ++++
drivers/hid/spi-hid/spi-hid-core.c | 1456 ++++++++++++++++++++
drivers/hid/spi-hid/spi-hid-core.h | 98 ++
drivers/hid/spi-hid/spi-hid-of.c | 244 ++++
drivers/hid/spi-hid/spi-hid-trace.h | 169 +++
drivers/hid/spi-hid/spi-hid.h | 46 +
include/linux/hid.h | 2 +
14 files changed, 2461 insertions(+), 2 deletions(-)
---
base-commit: 05f7e89ab9731565d8a62e3b5d1ec206485eeb0b
change-id: 20260212-send-upstream-75f6fd9ed92e
Best regards,
--
Jingyuan Liang <jingyliang@chromium.org>
^ permalink raw reply
* Re: [PATCH v2 00/11] Add spi-hid transport driver
From: Jingyuan Liang @ 2026-04-02 1:54 UTC (permalink / raw)
To: Krzysztof Kozlowski
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, tfiga, Jarrett Schultz,
Dmitry Antipov, Angela Czubak
In-Reply-To: <20260325-naughty-hungry-wapiti-658e83@quoll>
On Wed, Mar 25, 2026 at 1:49 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On Tue, Mar 24, 2026 at 06:39:33AM +0000, Jingyuan Liang wrote:
> > This series picks up the spi-hid driver work originally started by
> > Microsoft. The patch breakdown has been modified and the implementation
> > has been refactored to address upstream feedback and testing issues. We
> > are submitting this as a new series while keeping the original sign-off
> > chain to reflect the history.
> >
> > Same as the original series, there is a change to HID documentation, some
> > HID core changes to support a SPI device, the SPI HID transport driver,
> > and HID over SPI Device Tree binding. We have added the HID over SPI ACPI
> > support, power management, panel follower, and quirks for Ilitek touch
> > controllers.
> >
> > Original authors: Jarrett Schultz <jaschultz@microsoft.com>,
> > Dmitry Antipov <dmanti@microsoft.com>
> > Link: https://lore.kernel.org/r/86b63b7b-afda-d7f4-7bfa-175085d5a8ef@gmail.com
> >
> > Signed-off-by: Jingyuan Liang <jingyliang@chromium.org>
> > ---
> > Changes in v2:
> > - Fix style problems and remove unnecessary fields from the DT binding file
>
> Style and removal? So other comments were skipped?
>
> Please write detailed changelogs, otherwise it feels you just ignore
> parts of the feedback.
>
> Best regards,
> Krzysztof
>
Comments are either resolved or awaiting further confirmation. I will
add more details
to v2 changelog in v3.
^ permalink raw reply
* Re: [PATCH 13/33] rust: block: update `const_refs_to_static` MSRV TODO comment
From: Gary Guo @ 2026-04-02 1:43 UTC (permalink / raw)
To: Tamir Duberstein, Miguel Ojeda
Cc: Nathan Chancellor, Nicolas Schier, Danilo Krummrich,
Andreas Hindborg, Catalin Marinas, Will Deacon, Paul Walmsley,
Palmer Dabbelt, Albert Ou, Alexandre Courbot, David Airlie,
Simona Vetter, Brendan Higgins, David Gow, Greg Kroah-Hartman,
Arve Hjønnevåg, Todd Kjos, Christian Brauner,
Carlos Llamas, Alice Ryhl, Jonathan Corbet, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Trevor Gross, rust-for-linux,
linux-kbuild, Lorenzo Stoakes, Vlastimil Babka, Liam R . Howlett,
Uladzislau Rezki, linux-block, linux-arm-kernel, Alexandre Ghiti,
linux-riscv, nouveau, dri-devel, Rae Moar, linux-kselftest,
kunit-dev, Nick Desaulniers, Bill Wendling, Justin Stitt, llvm,
linux-kernel, Shuah Khan, linux-doc
In-Reply-To: <177508434450.73816.3437422593454602087.b4-review@b4>
On Wed Apr 1, 2026 at 11:59 PM BST, Tamir Duberstein wrote:
> On Wed, 01 Apr 2026 13:45:20 +0200, Miguel Ojeda <ojeda@kernel.org> wrote:
>> diff --git a/rust/kernel/block/mq/gen_disk.rs b/rust/kernel/block/mq/gen_disk.rs
>> index c8b0ecb17082..912cb805caf5 100644
>> --- a/rust/kernel/block/mq/gen_disk.rs
>> +++ b/rust/kernel/block/mq/gen_disk.rs
>> @@ -140,9 +140,7 @@ pub fn build<T: Operations>(
>> devnode: None,
>> alternative_gpt_sector: None,
>> get_unique_id: None,
>> - // TODO: Set to THIS_MODULE. Waiting for const_refs_to_static feature to
>> - // be merged (unstable in rustc 1.78 which is staged for linux 6.10)
>> - // <https://github.com/rust-lang/rust/issues/119618>
>> + // TODO: Set to `THIS_MODULE`.
>
> Good first issue?
It's actually quite tricky. There're already attempts on the list, although I'm
not very happy with the solution.
The tricky part is that `THIS_MODULE` needs to be supplied by the leaf module,
it cannot be constructed inside the kernel crate.
Best,
Gary
>
> Reviewed-by: Tamir Duberstein <tamird@kernel.org>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox