Linux Documentation
 help / color / mirror / Atom feed
* [PATCH v2 04/10] gpu: nova-core: convert GC6 registers to kernel register macro
From: Alexandre Courbot @ 2026-03-20 12:19 UTC (permalink / raw)
  To: Danilo Krummrich, Alice Ryhl, David Airlie, Simona Vetter,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, Miguel Ojeda,
	Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
	Andreas Hindborg, Trevor Gross
  Cc: John Hubbard, Alistair Popple, Joel Fernandes, Timur Tabi,
	Zhi Wang, Eliot Courtney, dri-devel, linux-kernel, linux-riscv,
	linux-doc, rust-for-linux, Alexandre Courbot
In-Reply-To: <20260320-b4-nova-register-v2-0-88fcf103e8d4@nvidia.com>

Convert all GC6 registers to use the kernel's register macro and update
the code accordingly.

Reviewed-by: Eliot Courtney <ecourtney@nvidia.com>
Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
 drivers/gpu/nova-core/falcon/gsp.rs   |  7 ++--
 drivers/gpu/nova-core/fb/hal/ga102.rs |  7 ++--
 drivers/gpu/nova-core/gfw.rs          | 11 ++++--
 drivers/gpu/nova-core/regs.rs         | 67 ++++++++++++++++++-----------------
 4 files changed, 52 insertions(+), 40 deletions(-)

diff --git a/drivers/gpu/nova-core/falcon/gsp.rs b/drivers/gpu/nova-core/falcon/gsp.rs
index 67edef3636c1..e52f57abc223 100644
--- a/drivers/gpu/nova-core/falcon/gsp.rs
+++ b/drivers/gpu/nova-core/falcon/gsp.rs
@@ -1,7 +1,10 @@
 // SPDX-License-Identifier: GPL-2.0
 
 use kernel::{
-    io::poll::read_poll_timeout,
+    io::{
+        poll::read_poll_timeout,
+        Io, //
+    },
     prelude::*,
     time::Delta, //
 };
@@ -47,7 +50,7 @@ pub(crate) fn clear_swgen0_intr(&self, bar: &Bar0) {
     /// Checks if GSP reload/resume has completed during the boot process.
     pub(crate) fn check_reload_completed(&self, bar: &Bar0, timeout: Delta) -> Result<bool> {
         read_poll_timeout(
-            || Ok(regs::NV_PGC6_BSI_SECURE_SCRATCH_14::read(bar)),
+            || Ok(bar.read(regs::NV_PGC6_BSI_SECURE_SCRATCH_14)),
             |val| val.boot_stage_3_handoff(),
             Delta::ZERO,
             timeout,
diff --git a/drivers/gpu/nova-core/fb/hal/ga102.rs b/drivers/gpu/nova-core/fb/hal/ga102.rs
index 734605905031..4b9f0f74d0e7 100644
--- a/drivers/gpu/nova-core/fb/hal/ga102.rs
+++ b/drivers/gpu/nova-core/fb/hal/ga102.rs
@@ -1,6 +1,9 @@
 // SPDX-License-Identifier: GPL-2.0
 
-use kernel::prelude::*;
+use kernel::{
+    io::Io,
+    prelude::*, //
+};
 
 use crate::{
     driver::Bar0,
@@ -9,7 +12,7 @@
 };
 
 fn vidmem_size_ga102(bar: &Bar0) -> u64 {
-    regs::NV_USABLE_FB_SIZE_IN_MB::read(bar).usable_fb_size()
+    bar.read(regs::NV_USABLE_FB_SIZE_IN_MB).usable_fb_size()
 }
 
 struct Ga102;
diff --git a/drivers/gpu/nova-core/gfw.rs b/drivers/gpu/nova-core/gfw.rs
index 9121f400046d..fb75dd10a172 100644
--- a/drivers/gpu/nova-core/gfw.rs
+++ b/drivers/gpu/nova-core/gfw.rs
@@ -19,7 +19,10 @@
 //! Note that the devinit sequence also needs to run during suspend/resume.
 
 use kernel::{
-    io::poll::read_poll_timeout,
+    io::{
+        poll::read_poll_timeout,
+        Io, //
+    },
     prelude::*,
     time::Delta, //
 };
@@ -58,9 +61,11 @@ pub(crate) fn wait_gfw_boot_completion(bar: &Bar0) -> Result {
             Ok(
                 // Check that FWSEC has lowered its protection level before reading the GFW_BOOT
                 // status.
-                regs::NV_PGC6_AON_SECURE_SCRATCH_GROUP_05_PRIV_LEVEL_MASK::read(bar)
+                bar.read(regs::NV_PGC6_AON_SECURE_SCRATCH_GROUP_05_PRIV_LEVEL_MASK)
                     .read_protection_level0()
-                    && regs::NV_PGC6_AON_SECURE_SCRATCH_GROUP_05_0_GFW_BOOT::read(bar).completed(),
+                    && bar
+                        .read(regs::NV_PGC6_AON_SECURE_SCRATCH_GROUP_05_0_GFW_BOOT)
+                        .completed(),
             )
         },
         |&gfw_booted| gfw_booted,
diff --git a/drivers/gpu/nova-core/regs.rs b/drivers/gpu/nova-core/regs.rs
index 4f5cd64c2fce..6f49467e78ec 100644
--- a/drivers/gpu/nova-core/regs.rs
+++ b/drivers/gpu/nova-core/regs.rs
@@ -198,29 +198,41 @@ pub(crate) fn higher_bound(self) -> u64 {
 // These scratch registers remain powered on even in a low-power state and have a designated group
 // number.
 
-// Boot Sequence Interface (BSI) register used to determine
-// if GSP reload/resume has completed during the boot process.
-register!(NV_PGC6_BSI_SECURE_SCRATCH_14 @ 0x001180f8 {
-    26:26   boot_stage_3_handoff as bool;
-});
-
-// Privilege level mask register. It dictates whether the host CPU has privilege to access the
-// `PGC6_AON_SECURE_SCRATCH_GROUP_05` register (which it needs to read GFW_BOOT).
-register!(NV_PGC6_AON_SECURE_SCRATCH_GROUP_05_PRIV_LEVEL_MASK @ 0x00118128,
-          "Privilege level mask register" {
-    0:0     read_protection_level0 as bool, "Set after FWSEC lowers its protection level";
-});
-
-// OpenRM defines this as a register array, but doesn't specify its size and only uses its first
-// element. Be conservative until we know the actual size or need to use more registers.
-register!(NV_PGC6_AON_SECURE_SCRATCH_GROUP_05 @ 0x00118234[1] {});
-
-register!(
-    NV_PGC6_AON_SECURE_SCRATCH_GROUP_05_0_GFW_BOOT => NV_PGC6_AON_SECURE_SCRATCH_GROUP_05[0],
-    "Scratch group 05 register 0 used as GFW boot progress indicator" {
-        7:0    progress as u8, "Progress of GFW boot (0xff means completed)";
+io::register! {
+    /// Boot Sequence Interface (BSI) register used to determine
+    /// if GSP reload/resume has completed during the boot process.
+    pub(crate) NV_PGC6_BSI_SECURE_SCRATCH_14(u32) @ 0x001180f8 {
+        26:26   boot_stage_3_handoff => bool;
     }
-);
+
+    /// Privilege level mask register. It dictates whether the host CPU has privilege to access the
+    /// `PGC6_AON_SECURE_SCRATCH_GROUP_05` register (which it needs to read GFW_BOOT).
+    pub(crate) NV_PGC6_AON_SECURE_SCRATCH_GROUP_05_PRIV_LEVEL_MASK(u32) @ 0x00118128 {
+        /// Set after FWSEC lowers its protection level.
+        0:0     read_protection_level0 => bool;
+    }
+
+    /// OpenRM defines this as a register array, but doesn't specify its size and only uses its
+    /// first element. Be conservative until we know the actual size or need to use more registers.
+    pub(crate) NV_PGC6_AON_SECURE_SCRATCH_GROUP_05(u32)[1] @ 0x00118234 {}
+
+    /// Scratch group 05 register 0 used as GFW boot progress indicator.
+    pub(crate) NV_PGC6_AON_SECURE_SCRATCH_GROUP_05_0_GFW_BOOT(u32)
+        => NV_PGC6_AON_SECURE_SCRATCH_GROUP_05[0] {
+        /// Progress of GFW boot (0xff means completed).
+        7:0    progress;
+    }
+
+    pub(crate) NV_PGC6_AON_SECURE_SCRATCH_GROUP_42(u32) @ 0x001183a4 {
+        31:0    value;
+    }
+
+    /// Scratch group 42 register used as framebuffer size.
+    pub(crate) NV_USABLE_FB_SIZE_IN_MB(u32) => NV_PGC6_AON_SECURE_SCRATCH_GROUP_42 {
+        /// Usable framebuffer size, in megabytes.
+        31:0    value;
+    }
+}
 
 impl NV_PGC6_AON_SECURE_SCRATCH_GROUP_05_0_GFW_BOOT {
     /// Returns `true` if GFW boot is completed.
@@ -229,17 +241,6 @@ pub(crate) fn completed(self) -> bool {
     }
 }
 
-register!(NV_PGC6_AON_SECURE_SCRATCH_GROUP_42 @ 0x001183a4 {
-    31:0    value as u32;
-});
-
-register!(
-    NV_USABLE_FB_SIZE_IN_MB => NV_PGC6_AON_SECURE_SCRATCH_GROUP_42,
-    "Scratch group 42 register used as framebuffer size" {
-        31:0    value as u32, "Usable framebuffer size, in megabytes";
-    }
-);
-
 impl NV_USABLE_FB_SIZE_IN_MB {
     /// Returns the usable framebuffer size, in bytes.
     pub(crate) fn usable_fb_size(self) -> u64 {

-- 
2.53.0


^ permalink raw reply related

* [PATCH v2 03/10] gpu: nova-core: convert PFB registers to kernel register macro
From: Alexandre Courbot @ 2026-03-20 12:19 UTC (permalink / raw)
  To: Danilo Krummrich, Alice Ryhl, David Airlie, Simona Vetter,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, Miguel Ojeda,
	Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
	Andreas Hindborg, Trevor Gross
  Cc: John Hubbard, Alistair Popple, Joel Fernandes, Timur Tabi,
	Zhi Wang, Eliot Courtney, dri-devel, linux-kernel, linux-riscv,
	linux-doc, rust-for-linux, Alexandre Courbot
In-Reply-To: <20260320-b4-nova-register-v2-0-88fcf103e8d4@nvidia.com>

Convert all PFB registers to use the kernel's register macro and update
the code accordingly.

NV_PGSP_QUEUE_HEAD was somehow caught in the PFB section, so move it to
its own section and convert it as well.

Reviewed-by: Eliot Courtney <ecourtney@nvidia.com>
Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
 drivers/gpu/nova-core/fb/hal/ga100.rs | 34 +++++++++++++--------
 drivers/gpu/nova-core/fb/hal/tu102.rs | 14 +++++----
 drivers/gpu/nova-core/gsp/boot.rs     |  6 ++--
 drivers/gpu/nova-core/gsp/cmdq.rs     |  9 +++---
 drivers/gpu/nova-core/regs.rs         | 57 ++++++++++++++++++++---------------
 5 files changed, 70 insertions(+), 50 deletions(-)

diff --git a/drivers/gpu/nova-core/fb/hal/ga100.rs b/drivers/gpu/nova-core/fb/hal/ga100.rs
index e0acc41aa7cd..629588c75778 100644
--- a/drivers/gpu/nova-core/fb/hal/ga100.rs
+++ b/drivers/gpu/nova-core/fb/hal/ga100.rs
@@ -1,6 +1,10 @@
 // SPDX-License-Identifier: GPL-2.0
 
-use kernel::prelude::*;
+use kernel::{
+    io::Io,
+    num::Bounded,
+    prelude::*, //
+};
 
 use crate::{
     driver::Bar0,
@@ -13,22 +17,26 @@
 struct Ga100;
 
 pub(super) fn read_sysmem_flush_page_ga100(bar: &Bar0) -> u64 {
-    u64::from(regs::NV_PFB_NISO_FLUSH_SYSMEM_ADDR::read(bar).adr_39_08()) << FLUSH_SYSMEM_ADDR_SHIFT
-        | u64::from(regs::NV_PFB_NISO_FLUSH_SYSMEM_ADDR_HI::read(bar).adr_63_40())
+    u64::from(bar.read(regs::NV_PFB_NISO_FLUSH_SYSMEM_ADDR).adr_39_08()) << FLUSH_SYSMEM_ADDR_SHIFT
+        | u64::from(bar.read(regs::NV_PFB_NISO_FLUSH_SYSMEM_ADDR_HI).adr_63_40())
             << FLUSH_SYSMEM_ADDR_SHIFT_HI
 }
 
 pub(super) fn write_sysmem_flush_page_ga100(bar: &Bar0, addr: u64) {
-    regs::NV_PFB_NISO_FLUSH_SYSMEM_ADDR_HI::default()
-        // CAST: `as u32` is used on purpose since the remaining bits are guaranteed to fit within
-        // a `u32`.
-        .set_adr_63_40((addr >> FLUSH_SYSMEM_ADDR_SHIFT_HI) as u32)
-        .write(bar);
-    regs::NV_PFB_NISO_FLUSH_SYSMEM_ADDR::default()
-        // CAST: `as u32` is used on purpose since we want to strip the upper bits that have been
-        // written to `NV_PFB_NISO_FLUSH_SYSMEM_ADDR_HI`.
-        .set_adr_39_08((addr >> FLUSH_SYSMEM_ADDR_SHIFT) as u32)
-        .write(bar);
+    bar.write_reg(
+        regs::NV_PFB_NISO_FLUSH_SYSMEM_ADDR_HI::zeroed().with_adr_63_40(
+            Bounded::<u64, _>::from(addr)
+                .shr::<FLUSH_SYSMEM_ADDR_SHIFT_HI, _>()
+                .cast(),
+        ),
+    );
+
+    bar.write_reg(
+        regs::NV_PFB_NISO_FLUSH_SYSMEM_ADDR::zeroed()
+            // CAST: `as u32` is used on purpose since we want to strip the upper bits that have
+            // been written to `NV_PFB_NISO_FLUSH_SYSMEM_ADDR_HI`.
+            .with_adr_39_08((addr >> FLUSH_SYSMEM_ADDR_SHIFT) as u32),
+    );
 }
 
 pub(super) fn display_enabled_ga100(bar: &Bar0) -> bool {
diff --git a/drivers/gpu/nova-core/fb/hal/tu102.rs b/drivers/gpu/nova-core/fb/hal/tu102.rs
index eec984f4e816..515d50872224 100644
--- a/drivers/gpu/nova-core/fb/hal/tu102.rs
+++ b/drivers/gpu/nova-core/fb/hal/tu102.rs
@@ -1,6 +1,9 @@
 // SPDX-License-Identifier: GPL-2.0
 
-use kernel::prelude::*;
+use kernel::{
+    io::Io,
+    prelude::*, //
+};
 
 use crate::{
     driver::Bar0,
@@ -13,7 +16,7 @@
 pub(super) const FLUSH_SYSMEM_ADDR_SHIFT: u32 = 8;
 
 pub(super) fn read_sysmem_flush_page_gm107(bar: &Bar0) -> u64 {
-    u64::from(regs::NV_PFB_NISO_FLUSH_SYSMEM_ADDR::read(bar).adr_39_08()) << FLUSH_SYSMEM_ADDR_SHIFT
+    u64::from(bar.read(regs::NV_PFB_NISO_FLUSH_SYSMEM_ADDR).adr_39_08()) << FLUSH_SYSMEM_ADDR_SHIFT
 }
 
 pub(super) fn write_sysmem_flush_page_gm107(bar: &Bar0, addr: u64) -> Result {
@@ -21,9 +24,7 @@ pub(super) fn write_sysmem_flush_page_gm107(bar: &Bar0, addr: u64) -> Result {
     u32::try_from(addr >> FLUSH_SYSMEM_ADDR_SHIFT)
         .map_err(|_| EINVAL)
         .map(|addr| {
-            regs::NV_PFB_NISO_FLUSH_SYSMEM_ADDR::default()
-                .set_adr_39_08(addr)
-                .write(bar)
+            bar.write_reg(regs::NV_PFB_NISO_FLUSH_SYSMEM_ADDR::zeroed().with_adr_39_08(addr))
         })
 }
 
@@ -32,7 +33,8 @@ pub(super) fn display_enabled_gm107(bar: &Bar0) -> bool {
 }
 
 pub(super) fn vidmem_size_gp102(bar: &Bar0) -> u64 {
-    regs::NV_PFB_PRI_MMU_LOCAL_MEMORY_RANGE::read(bar).usable_fb_size()
+    bar.read(regs::NV_PFB_PRI_MMU_LOCAL_MEMORY_RANGE)
+        .usable_fb_size()
 }
 
 struct Tu102;
diff --git a/drivers/gpu/nova-core/gsp/boot.rs b/drivers/gpu/nova-core/gsp/boot.rs
index 06b8313c1219..55d8aef5ff1e 100644
--- a/drivers/gpu/nova-core/gsp/boot.rs
+++ b/drivers/gpu/nova-core/gsp/boot.rs
@@ -58,7 +58,7 @@ fn run_fwsec_frts(
     ) -> Result<()> {
         // Check that the WPR2 region does not already exists - if it does, we cannot run
         // FWSEC-FRTS until the GPU is reset.
-        if regs::NV_PFB_PRI_MMU_WPR2_ADDR_HI::read(bar).higher_bound() != 0 {
+        if bar.read(regs::NV_PFB_PRI_MMU_WPR2_ADDR_HI).higher_bound() != 0 {
             dev_err!(
                 dev,
                 "WPR2 region already exists - GPU needs to be reset to proceed\n"
@@ -103,8 +103,8 @@ fn run_fwsec_frts(
 
         // Check that the WPR2 region has been created as we requested.
         let (wpr2_lo, wpr2_hi) = (
-            regs::NV_PFB_PRI_MMU_WPR2_ADDR_LO::read(bar).lower_bound(),
-            regs::NV_PFB_PRI_MMU_WPR2_ADDR_HI::read(bar).higher_bound(),
+            bar.read(regs::NV_PFB_PRI_MMU_WPR2_ADDR_LO).lower_bound(),
+            bar.read(regs::NV_PFB_PRI_MMU_WPR2_ADDR_HI).higher_bound(),
         );
 
         match (wpr2_lo, wpr2_hi) {
diff --git a/drivers/gpu/nova-core/gsp/cmdq.rs b/drivers/gpu/nova-core/gsp/cmdq.rs
index d36a62ba1c60..b8b0edba5afd 100644
--- a/drivers/gpu/nova-core/gsp/cmdq.rs
+++ b/drivers/gpu/nova-core/gsp/cmdq.rs
@@ -11,7 +11,10 @@
         DmaAddress, //
     },
     dma_write,
-    io::poll::read_poll_timeout,
+    io::{
+        poll::read_poll_timeout,
+        Io, //
+    },
     new_mutex,
     prelude::*,
     sync::{
@@ -503,9 +506,7 @@ fn calculate_checksum<T: Iterator<Item = u8>>(it: T) -> u32 {
 
     /// Notifies the GSP that we have updated the command queue pointers.
     fn notify_gsp(bar: &Bar0) {
-        regs::NV_PGSP_QUEUE_HEAD::default()
-            .set_address(0)
-            .write(bar);
+        bar.write_reg(regs::NV_PGSP_QUEUE_HEAD::zeroed().with_address(0u32));
     }
 
     /// Sends `command` to the GSP and waits for the reply.
diff --git a/drivers/gpu/nova-core/regs.rs b/drivers/gpu/nova-core/regs.rs
index 533d912659ba..4f5cd64c2fce 100644
--- a/drivers/gpu/nova-core/regs.rs
+++ b/drivers/gpu/nova-core/regs.rs
@@ -120,26 +120,35 @@ fn fmt(&self, f: &mut kernel::fmt::Formatter<'_>) -> kernel::fmt::Result {
 
 // PFB
 
-// The following two registers together hold the physical system memory address that is used by the
-// GPU to perform sysmembar operations (see `fb::SysmemFlush`).
+io::register! {
+    /// Low bits of the physical system memory address used by the GPU to perform sysmembar
+    /// operations (see [`crate::fb::SysmemFlush`]).
+    pub(crate) NV_PFB_NISO_FLUSH_SYSMEM_ADDR(u32) @ 0x00100c10 {
+        31:0    adr_39_08;
+    }
 
-register!(NV_PFB_NISO_FLUSH_SYSMEM_ADDR @ 0x00100c10 {
-    31:0    adr_39_08 as u32;
-});
+    /// High bits of the physical system memory address used by the GPU to perform sysmembar
+    /// operations (see [`crate::fb::SysmemFlush`]).
+    pub(crate) NV_PFB_NISO_FLUSH_SYSMEM_ADDR_HI(u32) @ 0x00100c40 {
+        23:0    adr_63_40;
+    }
 
-register!(NV_PFB_NISO_FLUSH_SYSMEM_ADDR_HI @ 0x00100c40 {
-    23:0    adr_63_40 as u32;
-});
+    pub(crate) NV_PFB_PRI_MMU_LOCAL_MEMORY_RANGE(u32) @ 0x00100ce0 {
+        30:30   ecc_mode_enabled => bool;
+        9:4     lower_mag;
+        3:0     lower_scale;
+    }
 
-register!(NV_PFB_PRI_MMU_LOCAL_MEMORY_RANGE @ 0x00100ce0 {
-    3:0     lower_scale as u8;
-    9:4     lower_mag as u8;
-    30:30   ecc_mode_enabled as bool;
-});
+    pub(crate) NV_PFB_PRI_MMU_WPR2_ADDR_LO(u32) @ 0x001fa824 {
+        /// Bits 12..40 of the lower (inclusive) bound of the WPR2 region.
+        31:4    lo_val;
+    }
 
-register!(NV_PGSP_QUEUE_HEAD @ 0x00110c00 {
-    31:0    address as u32;
-});
+    pub(crate) NV_PFB_PRI_MMU_WPR2_ADDR_HI(u32) @ 0x001fa828 {
+        /// Bits 12..40 of the higher (exclusive) bound of the WPR2 region.
+        31:4    hi_val;
+    }
+}
 
 impl NV_PFB_PRI_MMU_LOCAL_MEMORY_RANGE {
     /// Returns the usable framebuffer size, in bytes.
@@ -156,10 +165,6 @@ pub(crate) fn usable_fb_size(self) -> u64 {
     }
 }
 
-register!(NV_PFB_PRI_MMU_WPR2_ADDR_LO@0x001fa824  {
-    31:4    lo_val as u32, "Bits 12..40 of the lower (inclusive) bound of the WPR2 region";
-});
-
 impl NV_PFB_PRI_MMU_WPR2_ADDR_LO {
     /// Returns the lower (inclusive) bound of the WPR2 region.
     pub(crate) fn lower_bound(self) -> u64 {
@@ -167,10 +172,6 @@ pub(crate) fn lower_bound(self) -> u64 {
     }
 }
 
-register!(NV_PFB_PRI_MMU_WPR2_ADDR_HI@0x001fa828  {
-    31:4    hi_val as u32, "Bits 12..40 of the higher (exclusive) bound of the WPR2 region";
-});
-
 impl NV_PFB_PRI_MMU_WPR2_ADDR_HI {
     /// Returns the higher (exclusive) bound of the WPR2 region.
     ///
@@ -180,6 +181,14 @@ pub(crate) fn higher_bound(self) -> u64 {
     }
 }
 
+// PGSP
+
+io::register! {
+    pub(crate) NV_PGSP_QUEUE_HEAD(u32) @ 0x00110c00 {
+        31:0    address;
+    }
+}
+
 // PGC6 register space.
 //
 // `GC6` is a GPU low-power state where VRAM is in self-refresh and the GPU is powered down (except

-- 
2.53.0


^ permalink raw reply related

* [PATCH v2 02/10] gpu: nova-core: convert PBUS registers to kernel register macro
From: Alexandre Courbot @ 2026-03-20 12:19 UTC (permalink / raw)
  To: Danilo Krummrich, Alice Ryhl, David Airlie, Simona Vetter,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, Miguel Ojeda,
	Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
	Andreas Hindborg, Trevor Gross
  Cc: John Hubbard, Alistair Popple, Joel Fernandes, Timur Tabi,
	Zhi Wang, Eliot Courtney, dri-devel, linux-kernel, linux-riscv,
	linux-doc, rust-for-linux, Alexandre Courbot
In-Reply-To: <20260320-b4-nova-register-v2-0-88fcf103e8d4@nvidia.com>

Convert all PBUS registers to use the kernel's register macro and update
the code accordingly.

Reviewed-by: Eliot Courtney <ecourtney@nvidia.com>
Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
 drivers/gpu/nova-core/gsp/boot.rs |  5 ++++-
 drivers/gpu/nova-core/regs.rs     | 12 +++++++-----
 2 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/nova-core/gsp/boot.rs b/drivers/gpu/nova-core/gsp/boot.rs
index 5e73bd769dcc..06b8313c1219 100644
--- a/drivers/gpu/nova-core/gsp/boot.rs
+++ b/drivers/gpu/nova-core/gsp/boot.rs
@@ -5,6 +5,7 @@
     dma::CoherentAllocation,
     dma_write,
     io::poll::read_poll_timeout,
+    io::Io,
     pci,
     prelude::*,
     time::Delta, //
@@ -87,7 +88,9 @@ fn run_fwsec_frts(
         }
 
         // SCRATCH_E contains the error code for FWSEC-FRTS.
-        let frts_status = regs::NV_PBUS_SW_SCRATCH_0E_FRTS_ERR::read(bar).frts_err_code();
+        let frts_status = bar
+            .read(regs::NV_PBUS_SW_SCRATCH_0E_FRTS_ERR)
+            .frts_err_code();
         if frts_status != 0 {
             dev_err!(
                 dev,
diff --git a/drivers/gpu/nova-core/regs.rs b/drivers/gpu/nova-core/regs.rs
index 58fb807605dd..533d912659ba 100644
--- a/drivers/gpu/nova-core/regs.rs
+++ b/drivers/gpu/nova-core/regs.rs
@@ -109,12 +109,14 @@ fn fmt(&self, f: &mut kernel::fmt::Formatter<'_>) -> kernel::fmt::Result {
 
 // PBUS
 
-register!(NV_PBUS_SW_SCRATCH @ 0x00001400[64]  {});
+io::register! {
+    pub(crate) NV_PBUS_SW_SCRATCH(u32)[64] @ 0x00001400 {}
 
-register!(NV_PBUS_SW_SCRATCH_0E_FRTS_ERR => NV_PBUS_SW_SCRATCH[0xe],
-    "scratch register 0xe used as FRTS firmware error code" {
-    31:16   frts_err_code as u16;
-});
+    /// Scratch register 0xe used as FRTS firmware error code.
+    pub(crate) NV_PBUS_SW_SCRATCH_0E_FRTS_ERR(u32) => NV_PBUS_SW_SCRATCH[0xe] {
+        31:16   frts_err_code;
+    }
+}
 
 // PFB
 

-- 
2.53.0


^ permalink raw reply related

* [PATCH v2 01/10] gpu: nova-core: convert PMC registers to kernel register macro
From: Alexandre Courbot @ 2026-03-20 12:19 UTC (permalink / raw)
  To: Danilo Krummrich, Alice Ryhl, David Airlie, Simona Vetter,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, Miguel Ojeda,
	Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
	Andreas Hindborg, Trevor Gross
  Cc: John Hubbard, Alistair Popple, Joel Fernandes, Timur Tabi,
	Zhi Wang, Eliot Courtney, dri-devel, linux-kernel, linux-riscv,
	linux-doc, rust-for-linux, Alexandre Courbot
In-Reply-To: <20260320-b4-nova-register-v2-0-88fcf103e8d4@nvidia.com>

Convert all PMC registers to use the kernel's register macro and update
the code accordingly.

nova-core's registers have some constant properties (like a 32-bit size
and a crate visibility), so introduce the `nv_reg` macro to shorten
their declaration.

Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
 drivers/gpu/nova-core/falcon.rs |  7 ++++--
 drivers/gpu/nova-core/gpu.rs    | 37 +++++++++++++++---------------
 drivers/gpu/nova-core/regs.rs   | 50 ++++++++++++++++++++++++++---------------
 3 files changed, 55 insertions(+), 39 deletions(-)

diff --git a/drivers/gpu/nova-core/falcon.rs b/drivers/gpu/nova-core/falcon.rs
index 7097a206ec3c..5a4f7fc85160 100644
--- a/drivers/gpu/nova-core/falcon.rs
+++ b/drivers/gpu/nova-core/falcon.rs
@@ -13,7 +13,10 @@
         DmaAddress,
         DmaMask, //
     },
-    io::poll::read_poll_timeout,
+    io::{
+        poll::read_poll_timeout,
+        Io, //
+    },
     prelude::*,
     sync::aref::ARef,
     time::Delta,
@@ -532,7 +535,7 @@ pub(crate) fn reset(&self, bar: &Bar0) -> Result {
         self.hal.reset_wait_mem_scrubbing(bar)?;
 
         regs::NV_PFALCON_FALCON_RM::default()
-            .set_value(regs::NV_PMC_BOOT_0::read(bar).into())
+            .set_value(bar.read(regs::NV_PMC_BOOT_0).into())
             .write(bar, &E::ID);
 
         Ok(())
diff --git a/drivers/gpu/nova-core/gpu.rs b/drivers/gpu/nova-core/gpu.rs
index 8579d632e717..cb549c8b254f 100644
--- a/drivers/gpu/nova-core/gpu.rs
+++ b/drivers/gpu/nova-core/gpu.rs
@@ -4,6 +4,8 @@
     device,
     devres::Devres,
     fmt,
+    io::Io,
+    num::Bounded,
     pci,
     prelude::*,
     sync::Arc, //
@@ -129,24 +131,18 @@ fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
 }
 
 /// Enum representation of the GPU generation.
-///
-/// TODO: remove the `Default` trait implementation, and the `#[default]`
-/// attribute, once the register!() macro (which creates Architecture items) no
-/// longer requires it for read-only fields.
-#[derive(fmt::Debug, Default, Copy, Clone)]
-#[repr(u8)]
+#[derive(fmt::Debug, Copy, Clone)]
 pub(crate) enum Architecture {
-    #[default]
     Turing = 0x16,
     Ampere = 0x17,
     Ada = 0x19,
 }
 
-impl TryFrom<u8> for Architecture {
+impl TryFrom<Bounded<u32, 6>> for Architecture {
     type Error = Error;
 
-    fn try_from(value: u8) -> Result<Self> {
-        match value {
+    fn try_from(value: Bounded<u32, 6>) -> Result<Self> {
+        match u8::from(value) {
             0x16 => Ok(Self::Turing),
             0x17 => Ok(Self::Ampere),
             0x19 => Ok(Self::Ada),
@@ -155,23 +151,26 @@ fn try_from(value: u8) -> Result<Self> {
     }
 }
 
-impl From<Architecture> for u8 {
+impl From<Architecture> for Bounded<u32, 6> {
     fn from(value: Architecture) -> Self {
-        // CAST: `Architecture` is `repr(u8)`, so this cast is always lossless.
-        value as u8
+        match value {
+            Architecture::Turing => Bounded::<u32, _>::new::<0x16>(),
+            Architecture::Ampere => Bounded::<u32, _>::new::<0x17>(),
+            Architecture::Ada => Bounded::<u32, _>::new::<0x19>(),
+        }
     }
 }
 
 pub(crate) struct Revision {
-    major: u8,
-    minor: u8,
+    major: Bounded<u8, 4>,
+    minor: Bounded<u8, 4>,
 }
 
 impl From<regs::NV_PMC_BOOT_42> for Revision {
     fn from(boot0: regs::NV_PMC_BOOT_42) -> Self {
         Self {
-            major: boot0.major_revision(),
-            minor: boot0.minor_revision(),
+            major: boot0.major_revision().cast(),
+            minor: boot0.minor_revision().cast(),
         }
     }
 }
@@ -208,13 +207,13 @@ fn new(dev: &device::Device, bar: &Bar0) -> Result<Spec> {
         //     from an earlier (pre-Fermi) era, and then using boot42 to precisely identify the GPU.
         //     Somewhere in the Rubin timeframe, boot0 will no longer have space to add new GPU IDs.
 
-        let boot0 = regs::NV_PMC_BOOT_0::read(bar);
+        let boot0 = bar.read(regs::NV_PMC_BOOT_0);
 
         if boot0.is_older_than_fermi() {
             return Err(ENODEV);
         }
 
-        let boot42 = regs::NV_PMC_BOOT_42::read(bar);
+        let boot42 = bar.read(regs::NV_PMC_BOOT_42);
         Spec::try_from(boot42).inspect_err(|_| {
             dev_err!(dev, "Unsupported chipset: {}\n", boot42);
         })
diff --git a/drivers/gpu/nova-core/regs.rs b/drivers/gpu/nova-core/regs.rs
index 53f412f0ca32..58fb807605dd 100644
--- a/drivers/gpu/nova-core/regs.rs
+++ b/drivers/gpu/nova-core/regs.rs
@@ -8,6 +8,7 @@
 pub(crate) mod macros;
 
 use kernel::{
+    io,
     prelude::*,
     time, //
 };
@@ -37,18 +38,38 @@
 
 // PMC
 
-register!(NV_PMC_BOOT_0 @ 0x00000000, "Basic revision information about the GPU" {
-    3:0     minor_revision as u8, "Minor revision of the chip";
-    7:4     major_revision as u8, "Major revision of the chip";
-    8:8     architecture_1 as u8, "MSB of the architecture";
-    23:20   implementation as u8, "Implementation version of the architecture";
-    28:24   architecture_0 as u8, "Lower bits of the architecture";
-});
+io::register! {
+    /// Basic revision information about the GPU.
+    pub(crate) NV_PMC_BOOT_0(u32) @ 0x00000000 {
+        /// Lower bits of the architecture.
+        28:24   architecture_0;
+        /// Implementation version of the architecture.
+        23:20   implementation;
+        /// MSB of the architecture.
+        8:8     architecture_1;
+        /// Major revision of the chip.
+        7:4     major_revision;
+        /// Minor revision of the chip.
+        3:0     minor_revision;
+    }
+
+    /// Extended architecture information.
+    pub(crate) NV_PMC_BOOT_42(u32) @ 0x00000a00 {
+        /// Architecture value.
+        29:24   architecture ?=> Architecture;
+        /// Implementation version of the architecture.
+        23:20   implementation;
+        /// Major revision of the chip.
+        19:16   major_revision;
+        /// Minor revision of the chip.
+        15:12   minor_revision;
+    }
+}
 
 impl NV_PMC_BOOT_0 {
     pub(crate) fn is_older_than_fermi(self) -> bool {
         // From https://github.com/NVIDIA/open-gpu-doc/tree/master/manuals :
-        const NV_PMC_BOOT_0_ARCHITECTURE_GF100: u8 = 0xc;
+        const NV_PMC_BOOT_0_ARCHITECTURE_GF100: u32 = 0xc;
 
         // Older chips left arch1 zeroed out. That, combined with an arch0 value that is less than
         // GF100, means "older than Fermi".
@@ -56,13 +77,6 @@ pub(crate) fn is_older_than_fermi(self) -> bool {
     }
 }
 
-register!(NV_PMC_BOOT_42 @ 0x00000a00, "Extended architecture information" {
-    15:12   minor_revision as u8, "Minor revision of the chip";
-    19:16   major_revision as u8, "Major revision of the chip";
-    23:20   implementation as u8, "Implementation version of the architecture";
-    29:24   architecture as u8 ?=> Architecture, "Architecture value";
-});
-
 impl NV_PMC_BOOT_42 {
     /// Combines `architecture` and `implementation` to obtain a code unique to the chipset.
     pub(crate) fn chipset(self) -> Result<Chipset> {
@@ -76,8 +90,8 @@ pub(crate) fn chipset(self) -> Result<Chipset> {
 
     /// Returns the raw architecture value from the register.
     fn architecture_raw(self) -> u8 {
-        ((self.0 >> Self::ARCHITECTURE_RANGE.start()) & ((1 << Self::ARCHITECTURE_RANGE.len()) - 1))
-            as u8
+        ((self.into_raw() >> Self::ARCHITECTURE_RANGE.start())
+            & ((1 << Self::ARCHITECTURE_RANGE.len()) - 1)) as u8
     }
 }
 
@@ -86,7 +100,7 @@ fn fmt(&self, f: &mut kernel::fmt::Formatter<'_>) -> kernel::fmt::Result {
         write!(
             f,
             "boot42 = 0x{:08x} (architecture 0x{:x}, implementation 0x{:x})",
-            self.0,
+            self.inner,
             self.architecture_raw(),
             self.implementation()
         )

-- 
2.53.0


^ permalink raw reply related

* [PATCH v2 00/10] gpu: nova-core: convert registers to use the kernel register macro
From: Alexandre Courbot @ 2026-03-20 12:19 UTC (permalink / raw)
  To: Danilo Krummrich, Alice Ryhl, David Airlie, Simona Vetter,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, Miguel Ojeda,
	Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
	Andreas Hindborg, Trevor Gross
  Cc: John Hubbard, Alistair Popple, Joel Fernandes, Timur Tabi,
	Zhi Wang, Eliot Courtney, dri-devel, linux-kernel, linux-riscv,
	linux-doc, rust-for-linux, Alexandre Courbot

nova-core carried its own helper macro to declare register types. Its
purpose was to be temporary since the beginning, and to serve as a
testbed to develop an equivalent that could be used kernel-wide.

That equivalent has now been merged, so it is time to retire the
nova-core local version.

The kernel register macro has evolved into something significantly
different from the one in nova-core, so it cannot be used as a drop-in
replacement. All declarations and sites using registers need to be
updated. No semantic change should happen as a result.

All the patches in this series could also be squashed into a single one
without altering their reviewability significantly. Actually I am
leaning towards that option since it doesn't make much sense to
partially convert the driver anyway. I'm leaving it in split state for
now in case this drives more people towards review. :)

I hope to be able to merge this quickly so we can rebase in-flight
series to use the updated register syntax.

This series is based on drm-rust-next as of 2026-03-20 and has been
tested on Turing.

Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
Changes in v2:
- Reorder register fields in descending order.
- Drop `nv_reg` macro and use the kernel `register` macro directly.
- Make `FUSE_UCODE1_VERSION` registers return a u16 directly.
- Use `into_raw` instead of accessing the inner value of registers directly.
- Use safer macro to generate the `From` and `TryFrom` implementations
  of falcon enum types.
- Link to v1: https://patch.msgid.link/20260318-b4-nova-register-v1-0-22a358aa4c63@nvidia.com

---
Alexandre Courbot (10):
      gpu: nova-core: convert PMC registers to kernel register macro
      gpu: nova-core: convert PBUS registers to kernel register macro
      gpu: nova-core: convert PFB registers to kernel register macro
      gpu: nova-core: convert GC6 registers to kernel register macro
      gpu: nova-core: convert FUSE registers to kernel register macro
      gpu: nova-core: convert PDISP registers to kernel register macro
      gpu: nova-core: falcon: introduce `bounded_enum` macro
      gpu: nova-core: convert falcon registers to kernel register macro
      gpu: nova-core: remove `io::` qualifier to register macro invocations
      Documentation: nova: remove register abstraction task

 Documentation/gpu/nova/core/todo.rst               |  76 ---
 drivers/gpu/nova-core/falcon.rs                    | 582 ++++++++--------
 drivers/gpu/nova-core/falcon/gsp.rs                |  27 +-
 drivers/gpu/nova-core/falcon/hal/ga102.rs          |  70 +-
 drivers/gpu/nova-core/falcon/hal/tu102.rs          |  12 +-
 drivers/gpu/nova-core/falcon/sec2.rs               |  17 +-
 drivers/gpu/nova-core/fb.rs                        |   6 +-
 drivers/gpu/nova-core/fb/hal/ga100.rs              |  37 +-
 drivers/gpu/nova-core/fb/hal/ga102.rs              |   7 +-
 drivers/gpu/nova-core/fb/hal/tu102.rs              |  17 +-
 drivers/gpu/nova-core/firmware/fwsec/bootloader.rs |  19 +-
 drivers/gpu/nova-core/gfw.rs                       |  11 +-
 drivers/gpu/nova-core/gpu.rs                       |  37 +-
 drivers/gpu/nova-core/gsp/boot.rs                  |  11 +-
 drivers/gpu/nova-core/gsp/cmdq.rs                  |   9 +-
 drivers/gpu/nova-core/regs.rs                      | 601 +++++++++--------
 drivers/gpu/nova-core/regs/macros.rs               | 739 ---------------------
 17 files changed, 765 insertions(+), 1513 deletions(-)
---
base-commit: a19457958c3018783881c4416f272cd594f13049
change-id: 20260318-b4-nova-register-6908b5118552

Best regards,
-- 
Alexandre Courbot <acourbot@nvidia.com>


^ permalink raw reply

* Re: [PATCH v14 1/2] rust: gpu: Add GPU buddy allocator bindings
From: Alexandre Courbot @ 2026-03-20 12:16 UTC (permalink / raw)
  To: Joel Fernandes
  Cc: linux-kernel, Miguel Ojeda, Boqun Feng, Gary Guo,
	Björn 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 König, Jani Nikula, Joonas Lahtinen, Rodrigo Vivi,
	Tvrtko Ursulin, Huang Rui, Matthew Auld, Matthew Brost,
	Lucas De Marchi, Thomas Hellström, Helge Deller, Alex Gaynor,
	Boqun Feng, John Hubbard, Alistair Popple, Timur Tabi, Edwin Peer,
	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: <20260320045711.43494-2-joelagnelf@nvidia.com>

On Fri Mar 20, 2026 at 1:57 PM JST, Joel Fernandes wrote:
> Add safe Rust abstractions over the Linux kernel's GPU buddy
> allocator for physical memory management. The GPU buddy allocator
> implements a binary buddy system useful for GPU physical memory
> allocation. nova-core will use it for physical memory allocation.
>
> Cc: Nikola Djukic <ndjukic@nvidia.com>
> Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>

Reviewed-by: Alexandre Courbot <acourbot@nvidia.com>

^ permalink raw reply

* Re: [PATCH v14 2/2] MAINTAINERS: gpu: buddy: Update reviewer
From: Alexandre Courbot @ 2026-03-20 12:16 UTC (permalink / raw)
  To: Joel Fernandes
  Cc: linux-kernel, Miguel Ojeda, Boqun Feng, Gary Guo,
	Björn 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 König, Jani Nikula, Joonas Lahtinen, Rodrigo Vivi,
	Tvrtko Ursulin, Huang Rui, Matthew Auld, Matthew Brost,
	Lucas De Marchi, Thomas Hellström, Helge Deller, Alex Gaynor,
	Boqun Feng, John Hubbard, Alistair Popple, Timur Tabi, Edwin Peer,
	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: <20260320045711.43494-3-joelagnelf@nvidia.com>

On Fri Mar 20, 2026 at 1:57 PM JST, Joel Fernandes wrote:
> Christian Koenig mentioned he'd like to step down from the reviewer
> role for the GPU buddy allocator. Joel Fernandes is stepping in as
> reviewer with agreement from Matthew Auld and Arun Pravin.
>
> Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>

This is missing the Acked-bys you collected on v13.

^ permalink raw reply

* Re: [PATCH v8 02/10] lib: kstrtox: add kstrntoull() helper
From: Rodrigo Alencar @ 2026-03-20 12:08 UTC (permalink / raw)
  To: Andy Shevchenko, Rodrigo Alencar
  Cc: rodrigo.alencar, linux-kernel, linux-iio, devicetree, linux-doc,
	Jonathan Cameron, David Lechner, Andy Shevchenko,
	Lars-Peter Clausen, Michael Hennerich, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Jonathan Corbet, Andrew Morton
In-Reply-To: <ab00clb7LL-bmJWe@ashevche-desk.local>

On 26/03/20 01:50PM, Andy Shevchenko wrote:
> On Fri, Mar 20, 2026 at 11:16:32AM +0000, Rodrigo Alencar wrote:
> > On 26/03/04 10:02AM, Rodrigo Alencar wrote:
> > > On 26/03/03 02:16PM, Rodrigo Alencar wrote:
> > > > On 26/03/03 03:49PM, Andy Shevchenko wrote:
> > > > > On Tue, Mar 03, 2026 at 01:27:07PM +0000, Rodrigo Alencar via B4 Relay wrote:
> > > > > 
> > > > > > Add kstrntoull() function, which converts a string to an ULL with a max
> > > > > > character limit. The function is an alternative integer parsing function
> > > > > > that does not require a null-terminated string. It becomes a better option
> > > > > 
> > > > > null -->  NUL
> > > > > 
> > > > > > over simple_strtoull() or kstrtoull() when parsing integers from a buffer
> > > > > > with custom delimiters without having to create temporary copies.
> > > > > > The function is consumed inside the implementation _kstrtoull(),
> > > > > > promoting reuse.
> > > > > 
> > > > > But this will not properly convert 0000000000000000000000000000000000000000100,
> > > > > for example, if the max_chars say set to 20.
> > > > 
> > > > Why would I want that? truncation will happen in the case and the value will
> > > > be zero. max_chars can be zet to INT_MAX/SIZE_MAX if you want to get 100.
> > > > 
> > > > > Also kstrto*() have a common idea behind to consume the only \n and allowed
> > > > > digits. This (naming) doesn't fit into the kstrto*() category.
> > > > 
> > > > mmm ok, but include/linux/kstrtox.h is the right place for this? how about just
> > > > strntoull()? I feel like a safe_ prefix does not make much sense if it is
> > > > only to differentiate from simple_strto*(), which should have been safe at
> > > > the first place.
> > > 
> > > Also kstrntoull() does not really match kstrto*(), as the 'n' is often used
> > > to indicate a stop condition on amount of characters, which would not need
> > > to require any termination character at all.
> > > The 'k' prefix was add to 'strntoull', mostly because the function is being
> > > added to the include/linux/kstrtox.h file. Other names I could think off:
> > > - bounded_strtoull()
> > > - bstrtoull() - 'b' for bounded
> > > - bstrntoull()
> > > - strtoull_bounded()
> > > - strtoull_limit()
> > > - safe_strntoull() - emphasizes overflow safety over simple_strtoull()
> > > 
> > > Extras considerations:
> > > - Single-letter prefixes (bstrntoull, lstrntoull, etc.) are too cryptic
> > > for a public API
> > > - safe_ prefix is subjective and doesn't describe the actual behavior
> > > 
> > > kstrntoull() is still my first candidate, other than that it would be
> > > bounded_strtoull().
> > 
> > could you provide more feedback here? Thanks!
> 
> I don't know what new I can add here.
> 
> My suggestion was (and still is) to have something in *_strtoull() family
> with additional checks added, but no limitations on the input string (i.e.
> no max_chars).  If you look at the printf() code the max_chars was added
> solely for scanf() and has no use otherwise (yes, I know about and aware
> of initramfs case).

but is it include/linux/kstrtox.h the right place for this?
*_strtoull familly... then can we just expose simple_strntoull(), which is
private to lib/vsprintf.c, by changing its prototype to expose a error return?
In my case the limitation on the input string is useful for the truncation of
decimal places when parsing the fixed point value. It would avoid a 64-bit
division.

-- 
Kind regards,

Rodrigo Alencar

^ permalink raw reply

* Re: [PATCH v8 02/10] lib: kstrtox: add kstrntoull() helper
From: Andy Shevchenko @ 2026-03-20 11:50 UTC (permalink / raw)
  To: Rodrigo Alencar
  Cc: rodrigo.alencar, linux-kernel, linux-iio, devicetree, linux-doc,
	Jonathan Cameron, David Lechner, Andy Shevchenko,
	Lars-Peter Clausen, Michael Hennerich, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Jonathan Corbet, Andrew Morton
In-Reply-To: <rygken7oujyoupeanrdnl7r7smesa7js6sn3jsafbajc4sm3jh@adeslfqwwf3x>

On Fri, Mar 20, 2026 at 11:16:32AM +0000, Rodrigo Alencar wrote:
> On 26/03/04 10:02AM, Rodrigo Alencar wrote:
> > On 26/03/03 02:16PM, Rodrigo Alencar wrote:
> > > On 26/03/03 03:49PM, Andy Shevchenko wrote:
> > > > On Tue, Mar 03, 2026 at 01:27:07PM +0000, Rodrigo Alencar via B4 Relay wrote:
> > > > 
> > > > > Add kstrntoull() function, which converts a string to an ULL with a max
> > > > > character limit. The function is an alternative integer parsing function
> > > > > that does not require a null-terminated string. It becomes a better option
> > > > 
> > > > null -->  NUL
> > > > 
> > > > > over simple_strtoull() or kstrtoull() when parsing integers from a buffer
> > > > > with custom delimiters without having to create temporary copies.
> > > > > The function is consumed inside the implementation _kstrtoull(),
> > > > > promoting reuse.
> > > > 
> > > > But this will not properly convert 0000000000000000000000000000000000000000100,
> > > > for example, if the max_chars say set to 20.
> > > 
> > > Why would I want that? truncation will happen in the case and the value will
> > > be zero. max_chars can be zet to INT_MAX/SIZE_MAX if you want to get 100.
> > > 
> > > > Also kstrto*() have a common idea behind to consume the only \n and allowed
> > > > digits. This (naming) doesn't fit into the kstrto*() category.
> > > 
> > > mmm ok, but include/linux/kstrtox.h is the right place for this? how about just
> > > strntoull()? I feel like a safe_ prefix does not make much sense if it is
> > > only to differentiate from simple_strto*(), which should have been safe at
> > > the first place.
> > 
> > Also kstrntoull() does not really match kstrto*(), as the 'n' is often used
> > to indicate a stop condition on amount of characters, which would not need
> > to require any termination character at all.
> > The 'k' prefix was add to 'strntoull', mostly because the function is being
> > added to the include/linux/kstrtox.h file. Other names I could think off:
> > - bounded_strtoull()
> > - bstrtoull() - 'b' for bounded
> > - bstrntoull()
> > - strtoull_bounded()
> > - strtoull_limit()
> > - safe_strntoull() - emphasizes overflow safety over simple_strtoull()
> > 
> > Extras considerations:
> > - Single-letter prefixes (bstrntoull, lstrntoull, etc.) are too cryptic
> > for a public API
> > - safe_ prefix is subjective and doesn't describe the actual behavior
> > 
> > kstrntoull() is still my first candidate, other than that it would be
> > bounded_strtoull().
> 
> could you provide more feedback here? Thanks!

I don't know what new I can add here.

My suggestion was (and still is) to have something in *_strtoull() family
with additional checks added, but no limitations on the input string (i.e.
no max_chars).  If you look at the printf() code the max_chars was added
solely for scanf() and has no use otherwise (yes, I know about and aware
of initramfs case).

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply

* Re: [PATCH v7 07/10] x86/vmscape: Use static_call() for predictor flush
From: Borislav Petkov @ 2026-03-20 11:31 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Pawan Gupta, x86, Nikolay Borisov, H. Peter Anvin, Josh Poimboeuf,
	David Kaplan, Sean Christopherson, Dave Hansen,
	Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, KP Singh,
	Jiri Olsa, David S. Miller, David Laight, Andy Lutomirski,
	Thomas Gleixner, Ingo Molnar, David Ahern, Martin KaFai Lau,
	Eduard Zingerman, Song Liu, Yonghong Song, John Fastabend,
	Stanislav Fomichev, Hao Luo, Paolo Bonzini, Jonathan Corbet,
	linux-kernel, kvm, Asit Mallick, Tao Zhang, bpf, netdev,
	linux-doc
In-Reply-To: <20260320090340.GN3738786@noisy.programming.kicks-ass.net>

On Fri, Mar 20, 2026 at 10:03:40AM +0100, Peter Zijlstra wrote:
> On Thu, Mar 19, 2026 at 11:22:06PM -0700, Pawan Gupta wrote:
> 
> > This plus extending it to support EXPORT_STATIC_CALL_FOR_KVM() is probably
> > a better solution. Please let me know which one you prefer.
> 
> The EXPORT twiddling will do I suppose. I'll try and not forget looking
> at doing the RO static_call thing some time.

Dunno, but exporting a static_call sounds really really wrong to me. No matter
where. As in: we're exporting the underlying inner workings of it and that
should be a big fat no-no.

So definitely +1 on exporting the helper instead.

I'd say...

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

^ permalink raw reply

* Re: [PATCH RFC v2 1/9] dt-bindings: iio: frequency: add ad9910
From: Rodrigo Alencar @ 2026-03-20 11:21 UTC (permalink / raw)
  To: Conor Dooley, rodrigo.alencar
  Cc: linux-iio, devicetree, linux-kernel, linux-doc,
	Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
	David Lechner, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Philipp Zabel, Jonathan Corbet, Shuah Khan
In-Reply-To: <20260319-annex-varying-afbddcb825b7@spud>

On 26/03/19 05:25PM, Conor Dooley wrote:
> On Wed, Mar 18, 2026 at 05:56:01PM +0000, Rodrigo Alencar via B4 Relay wrote:
> > From: Rodrigo Alencar <rodrigo.alencar@analog.com>
> > 
> > DT-bindings for AD9910, a 1 GSPS DDS with 14-bit DAC. It includes
> > configurations for clocks, DAC current, reset and basic GPIO control.
> > 
> > Signed-off-by: Rodrigo Alencar <rodrigo.alencar@analog.com>

...

> > +
> > +  clock-names:
> > +    oneOf:
> > +      - items:
> > +          - const: ref_clk
> 
> s/_clk//, not like it can be anything else!
> 
> > +      - items:
> > +          - const: ref_clk
> > +          - const: sync_in
> > +
> > +  '#clock-cells':
> > +    const: 1
> > +
> > +  clock-output-names:
> > +    minItems: 1
> > +    maxItems: 3
> > +    items:
> > +      enum: [ sync_clk, pdclk, sync_out ]
> 
> I'd say same here, but then you've got some issues with differentiation,
> so idk.

so I've got the names as they are referred in the device pins in the datasheet

...

> > +dependencies:
> > +  adi,charge-pump-current-microamp: [ 'adi,pll-enable' ]
> > +  adi,refclk-out-drive-strength: [ 'adi,pll-enable' ]
> > +  interrupts: [ interrupt-names ]
> > +  clocks: [ clock-names ]
> > +  '#clock-cells': [ clock-output-names ]
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - clocks
> 
> Worth pointing out, you haven't made either clock-names or
> interrupt-names (when interrupts are used) mandatory, so the properties
> cannot be used by a driver. I suggest you make clock-names mandatory and
> interrupts depend on interrupt-names.

the dependecies is not enought make them required then? understood!
 
-- 
Kind regards,

Rodrigo Alencar

^ permalink raw reply

* Re: [PATCH v8 02/10] lib: kstrtox: add kstrntoull() helper
From: Rodrigo Alencar @ 2026-03-20 11:16 UTC (permalink / raw)
  To: Rodrigo Alencar, Andy Shevchenko, rodrigo.alencar
  Cc: linux-kernel, linux-iio, devicetree, linux-doc, Jonathan Cameron,
	David Lechner, Andy Shevchenko, Lars-Peter Clausen,
	Michael Hennerich, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Jonathan Corbet, Andrew Morton
In-Reply-To: <4mtdzxfj656sjr66npabfvrr7yd7q26l2unhsihjtniz4ossfj@g3qnzonoary6>

On 26/03/04 10:02AM, Rodrigo Alencar wrote:
> On 26/03/03 02:16PM, Rodrigo Alencar wrote:
> > On 26/03/03 03:49PM, Andy Shevchenko wrote:
> > > On Tue, Mar 03, 2026 at 01:27:07PM +0000, Rodrigo Alencar via B4 Relay wrote:
> > > 
> > > > Add kstrntoull() function, which converts a string to an ULL with a max
> > > > character limit. The function is an alternative integer parsing function
> > > > that does not require a null-terminated string. It becomes a better option
> > > 
> > > null -->  NUL
> > > 
> > > > over simple_strtoull() or kstrtoull() when parsing integers from a buffer
> > > > with custom delimiters without having to create temporary copies.
> > > > The function is consumed inside the implementation _kstrtoull(),
> > > > promoting reuse.
> > > 
> > > But this will not properly convert 0000000000000000000000000000000000000000100,
> > > for example, if the max_chars say set to 20.
> > 
> > Why would I want that? truncation will happen in the case and the value will
> > be zero. max_chars can be zet to INT_MAX/SIZE_MAX if you want to get 100.
> > 
> > > Also kstrto*() have a common idea behind to consume the only \n and allowed
> > > digits. This (naming) doesn't fit into the kstrto*() category.
> > 
> > mmm ok, but include/linux/kstrtox.h is the right place for this? how about just
> > strntoull()? I feel like a safe_ prefix does not make much sense if it is
> > only to differentiate from simple_strto*(), which should have been safe at
> > the first place.
> 
> Also kstrntoull() does not really match kstrto*(), as the 'n' is often used
> to indicate a stop condition on amount of characters, which would not need
> to require any termination character at all.
> The 'k' prefix was add to 'strntoull', mostly because the function is being
> added to the include/linux/kstrtox.h file. Other names I could think off:
> - bounded_strtoull()
> - bstrtoull() - 'b' for bounded
> - bstrntoull()
> - strtoull_bounded()
> - strtoull_limit()
> - safe_strntoull() - emphasizes overflow safety over simple_strtoull()
> 
> Extras considerations:
> - Single-letter prefixes (bstrntoull, lstrntoull, etc.) are too cryptic
> for a public API
> - safe_ prefix is subjective and doesn't describe the actual behavior
> 
> kstrntoull() is still my first candidate, other than that it would be
> bounded_strtoull().

Hi Andy,

could you provide more feedback here? Thanks!

-- 
Kind regards,

Rodrigo Alencar

^ permalink raw reply

* Re: [PATCH v3 0/8] RDMA: Enable operation with DMA debug enabled
From: Marek Szyprowski @ 2026-03-20 11:09 UTC (permalink / raw)
  To: Leon Romanovsky, Robin Murphy, Michael S. Tsirkin, Petr Tesarik,
	Jonathan Corbet, Shuah Khan, Jason Wang, Xuan Zhuo,
	Eugenio Pérez, Jason Gunthorpe, Steven Rostedt,
	Masami Hiramatsu, Mathieu Desnoyers, Joerg Roedel, Will Deacon,
	Andrew Morton
  Cc: iommu, linux-kernel, linux-doc, virtualization, linux-rdma,
	linux-trace-kernel, linux-mm
In-Reply-To: <20260316-dma-debug-overlap-v3-0-1dde90a7f08b@nvidia.com>

On 16.03.2026 20:06, Leon Romanovsky wrote:
> Add a new DMA_ATTR_REQUIRE_COHERENT attribute to the DMA API to mark
> mappings that must run on a DMA‑coherent system. Such buffers cannot
> use the SWIOTLB path, may overlap with CPU caches, and do not depend on
> explicit cache flushing.
>
> Mappings using this attribute are rejected on systems where cache
> side‑effects could lead to data corruption, and therefore do not need
> the cache‑overlap debugging logic. This series also includes fixes for
> DMA_ATTR_CPU_CACHE_CLEAN handling.
> Thanks.
>
> Signed-off-by: Leon Romanovsky <leonro@nvidia.com>

Applied to dma-mapping-fixes. Thanks!

> ---
> Changes in v3:
> - Enriched commit messages and documentation
> - Added ROB tags
> - Link to v2: https://protect2.fireeye.com/v1/url?k=9c1ba148-fd90b40f-9c1a2a07-000babff99aa-86ebd022a97425b3&q=1&e=3c8e10cc-4c34-4bf6-aa9d-c339877d6a27&u=https%3A%2F%2Fpatch.msgid.link%2F20260311-dma-debug-overlap-v2-0-e00bc2ca346d%40nvidia.com
>
> Changes in v2:
> - Added DMA_ATTR_REQUIRE_COHERENT attribute
> - Added HMM patch which needs this attribute as well
> - Renamed DMA_ATTR_CPU_CACHE_CLEAN to be DMA_ATTR_DEBUGGING_IGNORE_CACHELINES
> - Link to v1: https://protect2.fireeye.com/v1/url?k=cc0590de-ad8e8599-cc041b91-000babff99aa-07e4da206b7e0d97&q=1&e=3c8e10cc-4c34-4bf6-aa9d-c339877d6a27&u=https%3A%2F%2Fpatch.msgid.link%2F20260307-dma-debug-overlap-v1-0-c034c38872af%40nvidia.com
>
> ---
> Leon Romanovsky (8):
>        dma-debug: Allow multiple invocations of overlapping entries
>        dma-mapping: handle DMA_ATTR_CPU_CACHE_CLEAN in trace output
>        dma-mapping: Clarify valid conditions for CPU cache line overlap
>        dma-mapping: Introduce DMA require coherency attribute
>        dma-direct: prevent SWIOTLB path when DMA_ATTR_REQUIRE_COHERENT is set
>        iommu/dma: add support for DMA_ATTR_REQUIRE_COHERENT attribute
>        RDMA/umem: Tell DMA mapping that UMEM requires coherency
>        mm/hmm: Indicate that HMM requires DMA coherency
>
>   Documentation/core-api/dma-attributes.rst | 38 ++++++++++++++++++++++++-------
>   drivers/infiniband/core/umem.c            |  5 ++--
>   drivers/iommu/dma-iommu.c                 | 21 +++++++++++++----
>   drivers/virtio/virtio_ring.c              | 10 ++++----
>   include/linux/dma-mapping.h               | 15 ++++++++----
>   include/trace/events/dma.h                |  4 +++-
>   kernel/dma/debug.c                        |  9 ++++----
>   kernel/dma/direct.h                       |  7 +++---
>   kernel/dma/mapping.c                      |  6 +++++
>   mm/hmm.c                                  |  4 ++--
>   10 files changed, 86 insertions(+), 33 deletions(-)
> ---
> base-commit: 11439c4635edd669ae435eec308f4ab8a0804808
> change-id: 20260305-dma-debug-overlap-21487c3fa02c
>
> Best regards,
> --
> Leon Romanovsky <leonro@nvidia.com>
>
>
Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland


^ permalink raw reply

* Re: [PATCH v3 0/8] RDMA: Enable operation with DMA debug enabled
From: Marek Szyprowski @ 2026-03-20 11:08 UTC (permalink / raw)
  To: Leon Romanovsky
  Cc: Robin Murphy, Michael S. Tsirkin, Petr Tesarik, Jonathan Corbet,
	Shuah Khan, Jason Wang, Xuan Zhuo, Eugenio Pérez,
	Jason Gunthorpe, Steven Rostedt, Masami Hiramatsu,
	Mathieu Desnoyers, Joerg Roedel, Will Deacon, Andrew Morton,
	iommu, linux-kernel, linux-doc, virtualization, linux-rdma,
	linux-trace-kernel, linux-mm
In-Reply-To: <20260318081858.GE61385@unreal>

Hi Leon,

On 18.03.2026 09:18, Leon Romanovsky wrote:
> On Wed, Mar 18, 2026 at 09:03:00AM +0100, Marek Szyprowski wrote:
>> On 17.03.2026 20:05, Leon Romanovsky wrote:
>>> On Mon, Mar 16, 2026 at 09:06:44PM +0200, Leon Romanovsky wrote:
>>>> Add a new DMA_ATTR_REQUIRE_COHERENT attribute to the DMA API to mark
>>>> mappings that must run on a DMA‑coherent system. Such buffers cannot
>>>> use the SWIOTLB path, may overlap with CPU caches, and do not depend on
>>>> explicit cache flushing.
>>>>
>>>> Mappings using this attribute are rejected on systems where cache
>>>> side‑effects could lead to data corruption, and therefore do not need
>>>> the cache‑overlap debugging logic. This series also includes fixes for
>>>> DMA_ATTR_CPU_CACHE_CLEAN handling.
>>>> Thanks.
>>> <...>
>>>
>>>> ---
>>>> Leon Romanovsky (8):
>>>>         dma-debug: Allow multiple invocations of overlapping entries
>>>>         dma-mapping: handle DMA_ATTR_CPU_CACHE_CLEAN in trace output
>>>>         dma-mapping: Clarify valid conditions for CPU cache line overlap
>>>>         dma-mapping: Introduce DMA require coherency attribute
>>>>         dma-direct: prevent SWIOTLB path when DMA_ATTR_REQUIRE_COHERENT is set
>>>>         iommu/dma: add support for DMA_ATTR_REQUIRE_COHERENT attribute
>>>>         RDMA/umem: Tell DMA mapping that UMEM requires coherency
>>>>         mm/hmm: Indicate that HMM requires DMA coherency
>>>>
>>>>    Documentation/core-api/dma-attributes.rst | 38 ++++++++++++++++++++++++-------
>>>>    drivers/infiniband/core/umem.c            |  5 ++--
>>>>    drivers/iommu/dma-iommu.c                 | 21 +++++++++++++----
>>>>    drivers/virtio/virtio_ring.c              | 10 ++++----
>>>>    include/linux/dma-mapping.h               | 15 ++++++++----
>>>>    include/trace/events/dma.h                |  4 +++-
>>>>    kernel/dma/debug.c                        |  9 ++++----
>>>>    kernel/dma/direct.h                       |  7 +++---
>>>>    kernel/dma/mapping.c                      |  6 +++++
>>>>    mm/hmm.c                                  |  4 ++--
>>>>    10 files changed, 86 insertions(+), 33 deletions(-)
>>> Marek,
>>>
>>> Despite the "RDMA ..." tag in the subject, the diffstat clearly shows that
>>> you are the appropriate person to take this patch.
>> I plan to take the first 2 patches to the dma-mapping-fixes branch
>> (v7.0-rc) and the next to dma-mapping-for-next. Should I also take the
>> RDMA and HMM patches, or do You want a stable branch for merging them
>> via respective subsystem trees?
> I suggest taking all patches into the -fixes branch, as the "RDMA/..." patch
> also resolves the dmesg splat. With -fixes, there is no need to worry about
> a shared branch since we do not expect merge conflicts in that area.
>
> If you still prefer to split the series between -fixes and -next, it would be
> better to use a shared branch in that case. There are patches on the RDMA
> list targeted for -next that touch ib_umem_get().

Okay, I will merge all patches to the -fixes branch then.

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland


^ permalink raw reply

* Re: [PATCH v4] LICENSES: Explicitly allow SPDX-FileCopyrightText
From: Greg Kroah-Hartman @ 2026-03-20 10:37 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Thomas Gleixner, Jonathan Corbet, Shuah Khan, Andy Whitcroft,
	Joe Perches, Dwaipayan Ray, Lukas Bulwahn, linux-spdx, workflows,
	linux-doc, linux-kernel, Laurent Pinchart
In-Reply-To: <aab4d11a-9e46-4c6a-9e7f-cb598195b693@oss.qualcomm.com>

On Fri, Mar 20, 2026 at 10:47:52AM +0100, Krzysztof Kozlowski wrote:
> On 01/03/2026 15:33, Krzysztof Kozlowski wrote:
> > Sources already have SPDX-FileCopyrightText (~40 instances) and more
> > appear on the mailing list, so document that it is allowed.  On the
> > other hand SPDX defines several other tags like SPDX-FileType, so add
> > checkpatch rule to narrow desired tags only to two of them - license and
> > copyright.  That way no new tags would sneak in to the kernel unnoticed.
> > 
> > Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > Cc: Joe Perches <joe@perches.com>
> > Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
> > ---
> > 
> > Other way would be to remove SPDX-FileCopyrightText from existing files
> > and disallow this, but one way or another we should be explicit about
> > it.  Otherwise people will be sending more of these and each maintainer
> > would need to make their own call.
> 
> Hey folks, any further comments here?
> 
> Is there a preference that this tag should be instead removed from the
> kernel and checkpatch should not allow it?
> 
> Honestly, I don't care about the way but I do care of having it
> defined/codified.
> 
> Best regards,
> Krzysztof

Looks sane to me, I'll take in the SPDX tree now, thanks.

greg k-h

^ permalink raw reply

* Re: [PATCH v4] LICENSES: Explicitly allow SPDX-FileCopyrightText
From: Krzysztof Kozlowski @ 2026-03-20  9:47 UTC (permalink / raw)
  To: Thomas Gleixner, Greg Kroah-Hartman, Jonathan Corbet, Shuah Khan,
	Andy Whitcroft, Joe Perches, Dwaipayan Ray, Lukas Bulwahn,
	linux-spdx, workflows, linux-doc, linux-kernel
  Cc: Laurent Pinchart
In-Reply-To: <20260301143322.92894-2-krzysztof.kozlowski@oss.qualcomm.com>

On 01/03/2026 15:33, Krzysztof Kozlowski wrote:
> Sources already have SPDX-FileCopyrightText (~40 instances) and more
> appear on the mailing list, so document that it is allowed.  On the
> other hand SPDX defines several other tags like SPDX-FileType, so add
> checkpatch rule to narrow desired tags only to two of them - license and
> copyright.  That way no new tags would sneak in to the kernel unnoticed.
> 
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Joe Perches <joe@perches.com>
> Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
> ---
> 
> Other way would be to remove SPDX-FileCopyrightText from existing files
> and disallow this, but one way or another we should be explicit about
> it.  Otherwise people will be sending more of these and each maintainer
> would need to make their own call.

Hey folks, any further comments here?

Is there a preference that this tag should be instead removed from the
kernel and checkpatch should not allow it?

Honestly, I don't care about the way but I do care of having it
defined/codified.

Best regards,
Krzysztof

^ permalink raw reply

* [PATCH RFC 0/2] kernel-doc: better handle data prototypes
From: Mauro Carvalho Chehab @ 2026-03-20  9:46 UTC (permalink / raw)
  To: Linux Doc Mailing List, Jonathan Corbet
  Cc: Mauro Carvalho Chehab, Mauro Carvalho Chehab, linux-kernel

Hi Jon,

Don't merge this series. It is just a heads on about what I'm
working right now.

This is basically a proof of concept, not yet integrated with
kernel-doc. It helps to show that investing on a tokenizer
was a good idea.

I'm still testing the code.

Right now, kernel-doc logic to handle data types is very
complex, and the code is split into dump_<type> functions, which
in turn calls several ancillary routines. The most complex ones
are related to handling struct, with involves converting inner
struct/unions into members of the main struct.

By using this new code, all elements from most data types can
be parsed with a single code.

Please notice that the code was designed to pick a single
declaration, as this is how kdoc_parser will use it.
If you try to parse multiple ones, the output won't be right,
as it will pick the first declaration name and create a single
item with all data declarations on it.

As it is not based on regexes, it can properly handle some
problematic cases, like having:

    {};

and:
    ;;;;;

in the middle of a struct/union.

For enums, if one has values inside the declaration, like:

    enum { FOO, BAR } type;

It picks the right data type. Kernel-doc maps this currently as:
    enum type

My plan is to integrate it at Kernel-doc and see how it goes.
It will likely rise some corner cases, but, once we get it right,
this will likely reduce the size and complexity of kdoc_parser.

If you want to test, you can use:

    ./parse_c.py

to use an example hardcoded on it, or it reads from a fname with:

    $ ./parse_c.py x.h
    CDataItem(decl_type=None, decl_name=None, parameterlist=['u16_data'], parametertypes={'u16_data': 'u16 u16_data[sizeof(u64) / sizeof(u16)]'})
    None None

    parameterlist:
      - u16_data

    parametertypes:
      - u16_data: u16 u16_data[sizeof(u64) / sizeof(u16)]

   (on this example, x.h has just:
    u16 u16_data[sizeof(u64) / sizeof(u16)];
   )

The logic stores decl_type and decl_name when the data is
struct/union/enum. If the data is just a declaration, it fills
only one element at parameterlist and at parametertypes.

Mauro Carvalho Chehab (2):
  docs: kdoc: add a class to parse data items
  HACK: add a parse_c.py file to test CDataParser

 parse_c.py                           |  87 +++++++++++
 tools/lib/python/kdoc/data_parser.py | 211 +++++++++++++++++++++++++++
 2 files changed, 298 insertions(+)
 create mode 100755 parse_c.py
 create mode 100644 tools/lib/python/kdoc/data_parser.py

-- 
2.53.0


^ permalink raw reply

* [PATCH RFC 1/2] docs: kdoc: add a class to parse data items
From: Mauro Carvalho Chehab @ 2026-03-20  9:46 UTC (permalink / raw)
  To: Jonathan Corbet, Linux Doc Mailing List
  Cc: Mauro Carvalho Chehab, linux-kernel, Mauro Carvalho Chehab
In-Reply-To: <cover.1773998596.git.mchehab+huawei@kernel.org>

Instead of using very complex regular expressions and hamming
inner structs/unions, use CTokenizer to handle data types.

It should be noticed that this doesn't handle "typedef".

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
 tools/lib/python/kdoc/data_parser.py | 211 +++++++++++++++++++++++++++
 1 file changed, 211 insertions(+)
 create mode 100644 tools/lib/python/kdoc/data_parser.py

diff --git a/tools/lib/python/kdoc/data_parser.py b/tools/lib/python/kdoc/data_parser.py
new file mode 100644
index 000000000000..f04915b67d6b
--- /dev/null
+++ b/tools/lib/python/kdoc/data_parser.py
@@ -0,0 +1,211 @@
+#!/usr/bin/env python3
+# SPDX-License-Identifier: GPL-2.0
+# Copyright(c) 2025: Mauro Carvalho Chehab <mchehab@kernel.org>.
+
+"""
+C lexical parser for variables.
+"""
+
+import logging
+import re
+
+from .c_lex import CTokenizer, CToken
+
+class CDataItem:
+    """
+    Represent a data declaration.
+    """
+    def __init__(self):
+        self.decl_name = None
+        self.decl_type = None
+        self.parameterlist = []
+        self.parametertypes = {}
+
+    def __repr__(self) -> str:
+        """
+        Return contents of the CDataItem.
+        Useful for debugging purposes.
+        """
+        return (f"CDataItem(decl_type={self.decl_type!r}, "
+                f"decl_name={self.decl_name!r}, "
+                f"parameterlist={self.parameterlist!r}, "
+                f"parametertypes={self.parametertypes!r})")
+
+class CDataParser:
+    """
+    Handles a C data prototype, converting it into a data element
+    describing it.
+    """
+
+    IGNORE_TOKENS = [CToken.SPACE, CToken.COMMENT]
+
+    def __init__(self, source):
+        self.source = source
+        self.item = CDataItem()
+
+        self._parse()
+
+    def _push_struct(self, tokens, stack, prev_kind, i):
+        """
+        Handles Structs and enums, picking the identifier just after
+        ``struct`` or ``union``.
+        """
+
+        if prev_kind:
+            j = prev_kind + 1
+            while j < len(tokens) and tokens[j].kind in self.IGNORE_TOKENS:
+                j += 1
+
+            if j < len(tokens) and tokens[j].kind == CToken.NAME:
+                stack.append(tokens[j].value)
+                return
+
+            name = "{unnamed " + tokens[prev_kind].value + "}"
+            stack.append(name)
+            self.item.parameterlist.append(name)
+            return
+
+        #
+        # Empty block. We still need to append for stack levels to match
+        #
+        stack.append(None)
+
+    def _parse(self):
+        """
+        Core algorithm  it is a lightweight rewrite of the
+        walk-the-tokens logic we sketched in the previous answer.
+        """
+        tokens = CTokenizer(self.source).tokens
+
+        stack= []
+        current_type = []
+        parameters = []
+        types = {}
+
+        prev_kind = None
+        get_id = False
+        level = 0
+
+        for i in range(0, len(tokens)):
+            tok = tokens[i]
+            if tok.kind == CToken.COMMENT:
+                continue
+
+            if tok.kind in [CToken.STRUCT, CToken.UNION, CToken.ENUM]:
+                prev_kind = i
+
+            if tok.kind == CToken.BEGIN:
+                if tok.value == "{":
+                    if (prev_kind and
+                        tokens[prev_kind].kind in [CToken.STRUCT, CToken.UNION]):
+
+                        self._push_struct(tokens, stack, prev_kind, i)
+                        if not self.item.decl_name:
+                            self.item.decl_name = stack[0]
+                    else:
+                        stack.append(None)
+
+                        #
+                        # Add previous tokens
+                        #
+                        if prev_kind:
+                            get_id = True
+
+                    if not self.item.decl_type:
+                        self.item.decl_type = tokens[prev_kind].value
+
+                    current_type = []
+
+                    continue
+
+                level += 1
+
+            if tok.kind == CToken.END:
+                if tok.value == "}":
+                    if stack:
+                        stack.pop()
+
+                    if get_id and prev_kind:
+                        current_type = []
+                        for j in range(prev_kind, i + 1):
+                            current_type.append((level, tokens[j]))
+                            if tok.kind == CToken.BEGIN:
+                                break
+
+                        while j < len(tokens):
+                            if tokens[j].kind not in self.IGNORE_TOKENS:
+                                break
+                            j += 1
+
+                        name = None
+
+                        if tokens[j].kind == CToken.NAME:
+                            name = tokens[j].value
+
+                        if not self.item.decl_type and len(stack) ==  1:
+                            self.item.decl_name = stack[0]
+
+                            self.item.parameterlist.append(name)
+                            current_type.append((level, tok))
+
+                    get_id = False
+                    prev_kind = None
+                    continue
+
+                level -= 1
+
+            if tok.kind != CToken.ENDSTMT:
+                current_type.append((level, tok))
+                continue
+
+            #
+            # End of an statement. Parse it if tokens are present
+            #
+
+            if not current_type:
+                current_type = []
+                continue
+
+            #
+            # the last NAME token with level 0 is the field name
+            #
+            name_token = None
+            for pos, t in enumerate(reversed(current_type)):
+                cur_level, cur_tok = t
+                if not cur_level and cur_tok.kind == CToken.NAME:
+                    name_token = cur_tok. value
+                    break
+
+            #
+            # TODO: we should likely emit a Warning here
+            #
+
+            if not name_token:
+                current_type = []
+                continue
+
+            #
+            # As we used reversed, we need to adjust pos here
+            #
+            pos = len(current_type) - pos - 1
+
+            #
+            # For the type, pick everything but the name
+            #
+
+            out = ""
+            for l, t in current_type:
+                out += t.value
+
+            names = []
+            for n in stack[1:] + [name_token]:
+                if n:
+                    if not "{unnamed" in n:
+                        names.append(n)
+
+            full_name = ".".join(names)
+
+            self.item.parameterlist.append(full_name)
+            self.item.parametertypes[full_name] = out.strip()
+
+            current_type = []
-- 
2.53.0


^ permalink raw reply related

* [PATCH RFC 2/2] HACK: add a parse_c.py file to test CDataParser
From: Mauro Carvalho Chehab @ 2026-03-20  9:46 UTC (permalink / raw)
  To: Jonathan Corbet, Linux Doc Mailing List
  Cc: Mauro Carvalho Chehab, linux-kernel, Mauro Carvalho Chehab
In-Reply-To: <cover.1773998596.git.mchehab+huawei@kernel.org>

This patch should not be merged. It is a quick tool to test
CDataParser.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
 parse_c.py | 87 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 87 insertions(+)
 create mode 100755 parse_c.py

diff --git a/parse_c.py b/parse_c.py
new file mode 100755
index 000000000000..740445998965
--- /dev/null
+++ b/parse_c.py
@@ -0,0 +1,87 @@
+#!/usr/bin/env python3
+# example.py
+"""
+Run a quick demo on a real C source file.
+
+Usage
+-----
+    python -m c_struct_parser.example <path/to/c/file.c>
+"""
+
+import argparse
+
+from tools.lib.python.kdoc.data_parser import CDataParser
+
+TEST = """
+struct property_entry {
+	const char *name;
+	size_t length;
+	bool is_inline;   /* TEST */
+	struct foo {
+		char *bar[12];
+		struct foobar {
+			enum enum_type my_enum; /* TEST	2 */
+			struct {
+				uint_t test; /* TEST 3 */
+				static const int anonymous;
+			};
+		};
+		;;        /* This is valid, but should not occur in practice */
+		{};       /* Same here */
+	};
+	enum dev_prop_type type;
+		enum {
+			EXPRESSION_LITERAL,
+			EXPRESSION_BINARY,
+			EXPRESSION_UNARY,
+			EXPRESSION_FUNCTION,
+			EXPRESSION_ARRAY
+		} literal;
+
+	union {
+		const void *pointer;
+		union {
+			u8 boou8_data[sizeof(u64) / sizeof(u8)];
+			u16 u16_data[sizeof(u64) / sizeof(u16)];
+			u32 u32_data[sizeof(u64) / sizeof(u32)];
+			u64 u64_data[sizeof(u64) / sizeof(u64)];
+			const char *str[sizeof(u64) / sizeof(char *)];
+		};
+	};
+	char *prop_name;
+};
+"""
+
+
+def main():
+    p = argparse.ArgumentParser(description="Parse a C struct/union/enum definition.")
+
+    p.add_argument("fname", nargs="?", help="C source file to parse")
+
+    args = p.parse_args()
+
+    if args.fname:
+        with open(args.fname, "r", encoding="utf-8") as f:
+            source = f.read()
+    else:
+        source = TEST
+
+    parser = CDataParser(source)
+
+    item = parser.item
+
+    print(repr(item))
+
+    print(f"{item.decl_type} {item.decl_name}\n")
+
+    print("parameterlist:")
+    for p in item.parameterlist:
+        print(f"  - {p}")
+
+    print("\nparametertypes:")
+    for k, v in item.parametertypes.items():
+        print(f"  - {k}: {v}")
+
+
+if __name__ == "__main__":
+    main()
-- 
2.53.0


^ permalink raw reply related

* Re: [PATCH v7 07/10] x86/vmscape: Use static_call() for predictor flush
From: Peter Zijlstra @ 2026-03-20  9:03 UTC (permalink / raw)
  To: Pawan Gupta
  Cc: x86, Nikolay Borisov, H. Peter Anvin, Josh Poimboeuf,
	David Kaplan, Sean Christopherson, Borislav Petkov, Dave Hansen,
	Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, KP Singh,
	Jiri Olsa, David S. Miller, David Laight, Andy Lutomirski,
	Thomas Gleixner, Ingo Molnar, David Ahern, Martin KaFai Lau,
	Eduard Zingerman, Song Liu, Yonghong Song, John Fastabend,
	Stanislav Fomichev, Hao Luo, Paolo Bonzini, Jonathan Corbet,
	linux-kernel, kvm, Asit Mallick, Tao Zhang, bpf, netdev,
	linux-doc
In-Reply-To: <20260320062206.bdrnmnvho6lhmejw@desk>

On Thu, Mar 19, 2026 at 11:22:06PM -0700, Pawan Gupta wrote:

> This plus extending it to support EXPORT_STATIC_CALL_FOR_KVM() is probably
> a better solution. Please let me know which one you prefer.

The EXPORT twiddling will do I suppose. I'll try and not forget looking
at doing the RO static_call thing some time.

^ permalink raw reply

* [PATCH v4] hwmon: add driver for ARCTIC Fan Controller
From: Aureo Serrano de Souza @ 2026-03-20  8:08 UTC (permalink / raw)
  To: linux-hwmon
  Cc: linux, linux, corbet, skhan, linux-doc, linux-kernel,
	Aureo Serrano de Souza

Add hwmon driver for the ARCTIC Fan Controller, a USB HID device
(VID 0x3904, PID 0xF001) with 10 fan channels. Exposes fan speed in
RPM (read-only) and PWM duty cycle (0-255, read/write) via sysfs.

The device pushes IN reports at ~1 Hz containing RPM readings. PWM is
set via OUT reports; the device applies the new duty cycle and sends
back a 2-byte ACK (Report ID 0x02). The driver waits up to 1 s for
the ACK using a completion. Measured device latency: max ~563 ms over
500 iterations. PWM control is manual-only: the device never changes
duty cycle autonomously.

raw_event() may run in hardirq context, so fan_rpm[] is protected by
a spinlock with irq-save. pwm_duty[] and the report buffer are
serialized by the hwmon core, which holds its lock for the duration of
the read/write callbacks.

Signed-off-by: Aureo Serrano de Souza <aureo.serrano@arctic.de>
---
Thanks to Guenter Roeck and Thomas Weißschuh for the reviews.

Changes since v3:
- buf[]: upgrade from __aligned(8) to ____cacheline_aligned so the
  DMA buffer occupies its own cache line, preventing false sharing with
  adjacent fan_rpm[]/pwm_duty[] fields on non-coherent architectures
- arctic_fan_write(): add write_pending flag (protected by
  in_report_lock) so raw_event() delivers ACKs only while a write is
  in flight; prevents a late ACK from a signal-interrupted write from
  satisfying a subsequent write's completion wait
- arctic_fan_write(): commit pwm_duty[channel] only after the device
  ACKs the command; a failed or timed-out write no longer leaves a
  stale value in the cached duty state
- arctic_fan_probe(): start IO (hid_device_io_start) before registering
  with hwmon; previously a sysfs write arriving between hwmon
  registration and io_start could send an OUT report whose ACK would be
  discarded by the HID core, causing a spurious timeout
- Documentation/hwmon/arctic_fan_controller.rst: document that cached
  PWM values start at 0 (hardware state unknown at probe) and that each
  OUT report carries all 10 channel values

Changes since v2:
- buf[]: add __aligned(8) for DMA safety
- ARCTIC_ACK_TIMEOUT_MS: restore 1000 ms; note observed max ~563 ms
- arctic_fan_parse_report(): replace hwmon_lock/hwmon_unlock with
  spin_lock_irqsave; hwmon_lock() may sleep and is unsafe when
  raw_event() runs in hardirq/softirq context
- arctic_fan_raw_event(): use spin_lock_irqsave for ACK path
- arctic_fan_write(): use spin_lock_irqsave for completion reinit
- arctic_fan_write(): clamp val to [0, 255] before u8 cast
- hardware teardown: register arctic_fan_hw_stop() via
  devm_add_action_or_reset() before hwmon; devm LIFO order ensures
  hwmon unregisters before hid_hw_close/stop; remove() is a no-op
- remove priv->hwmon_dev (no longer needed)

Changes since v1:
- Use hid_dbg() instead of module_param debug flag
- Move hid_device_id table adjacent to hid_driver struct
- Use get_unaligned_le16() for RPM parsing
- Remove impossible bounds/NULL checks; remove retry loop
- Add hid_is_usb() guard
- Do not update pwm_duty from IN reports (device is manual-only)
- Add completion/ACK mechanism for OUT report acknowledgement
- Add Documentation/hwmon/arctic_fan_controller.rst and MAINTAINERS

diff --git a/Documentation/hwmon/arctic_fan_controller.rst b/Documentation/hwmon/arctic_fan_controller.rst
new file mode 100644
index 0000000000..834f09c813
--- /dev/null
+++ b/Documentation/hwmon/arctic_fan_controller.rst
@@ -0,0 +1,49 @@
+.. SPDX-License-Identifier: GPL-2.0-or-later
+
+Kernel driver arctic_fan_controller
+=====================================
+
+Supported devices:
+
+* ARCTIC Fan Controller (USB HID, VID 0x3904, PID 0xF001)
+
+Author: Aureo Serrano de Souza <aureo.serrano@arctic.de>
+
+Description
+-----------
+
+This driver provides hwmon support for the ARCTIC Fan Controller, a USB
+Custom HID device with 10 fan channels. The device sends IN reports about
+once per second containing current RPM values (bytes 11-30, 10 x uint16 LE).
+Fan speed control is manual-only: the device does not change PWM
+autonomously; it only applies a new duty cycle when it receives an OUT
+report from the host.
+
+After the device applies an OUT report, it sends back a 2-byte ACK IN
+report (Report ID 0x02, byte 1 = 0x00 on success) confirming the command
+was applied.
+
+Usage notes
+-----------
+
+Since it is a USB device, hotplug is supported. The device is autodetected.
+
+The device does not support GET_REPORT, so the driver cannot read back the
+current hardware PWM state at probe time. The cached PWM values (readable
+via pwm[1-10]) start at 0 and reflect only values that have been
+successfully written. Because each OUT report carries all 10 channel values,
+writing a single channel also sends the cached values for all other channels.
+Users should set all channels to the desired values before relying on the
+cached state.
+
+Sysfs entries
+-------------
+
+================ ==============================================================
+fan[1-10]_input  Fan speed in RPM (read-only). Updated from IN reports at ~1 Hz.
+pwm[1-10]        PWM duty cycle (0-255). Write: sends an OUT report setting the
+                 duty cycle (scaled from 0-255 to 0-100% for the device);
+                 the cached value is updated only after the device ACKs the
+                 command. Read: returns the last successfully written value;
+                 initialized to 0 at driver load (hardware state unknown).
+================ ==============================================================
diff --git a/Documentation/hwmon/index.rst b/Documentation/hwmon/index.rst
index b2ca8513cf..c34713040e 100644
--- a/Documentation/hwmon/index.rst
+++ b/Documentation/hwmon/index.rst
@@ -42,6 +42,7 @@ Hardware Monitoring Kernel Drivers
    aht10
    amc6821
    aquacomputer_d5next
+   arctic_fan_controller
    asb100
    asc7621
    aspeed-g6-pwm-tach
diff --git a/MAINTAINERS b/MAINTAINERS
index 96ea84948d..ec3112bd41 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2053,6 +2053,13 @@ S:	Maintained
 F:	drivers/net/arcnet/
 F:	include/uapi/linux/if_arcnet.h

+ARCTIC FAN CONTROLLER DRIVER
+M:	Aureo Serrano de Souza <aureo.serrano@arctic.de>
+L:	linux-hwmon@vger.kernel.org
+S:	Maintained
+F:	Documentation/hwmon/arctic_fan_controller.rst
+F:	drivers/hwmon/arctic_fan_controller.c
+
 ARM AND ARM64 SoC SUB-ARCHITECTURES (COMMON PARTS)
 M:	Arnd Bergmann <arnd@arndb.de>
 M:	Krzysztof Kozlowski <krzk@kernel.org>
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 328867242c..6c90a8dd40 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -388,6 +388,18 @@ config SENSORS_APPLESMC
 	  Say Y here if you have an applicable laptop and want to experience
 	  the awesome power of applesmc.

+config SENSORS_ARCTIC_FAN_CONTROLLER
+	tristate "ARCTIC Fan Controller"
+	depends on USB_HID
+	help
+	  If you say yes here you get support for the ARCTIC Fan Controller,
+	  a USB HID device (VID 0x3904, PID 0xF001) with 10 fan channels.
+	  The driver exposes fan speed (RPM) and PWM control via the hwmon
+	  sysfs interface.
+
+	  This driver can also be built as a module. If so, the module
+	  will be called arctic_fan_controller.
+
 config SENSORS_ARM_SCMI
 	tristate "ARM SCMI Sensors"
 	depends on ARM_SCMI_PROTOCOL
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index 5833c807c6..ef831c3375 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -49,6 +49,7 @@ obj-$(CONFIG_SENSORS_ADT7475)	+= adt7475.o
 obj-$(CONFIG_SENSORS_AHT10)	+= aht10.o
 obj-$(CONFIG_SENSORS_APPLESMC)	+= applesmc.o
 obj-$(CONFIG_SENSORS_AQUACOMPUTER_D5NEXT) += aquacomputer_d5next.o
+obj-$(CONFIG_SENSORS_ARCTIC_FAN_CONTROLLER)	+= arctic_fan_controller.o
 obj-$(CONFIG_SENSORS_ARM_SCMI)	+= scmi-hwmon.o
 obj-$(CONFIG_SENSORS_ARM_SCPI)	+= scpi-hwmon.o
 obj-$(CONFIG_SENSORS_AS370)	+= as370-hwmon.o
diff --git a/drivers/hwmon/arctic_fan_controller.c b/drivers/hwmon/arctic_fan_controller.c
new file mode 100644
index 0000000000..7b991735ed
--- /dev/null
+++ b/drivers/hwmon/arctic_fan_controller.c
@@ -0,0 +1,328 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Linux hwmon driver for ARCTIC Fan Controller
+ *
+ * USB Custom HID device with 10 fan channels.
+ * Exposes fan RPM (input) and PWM (0-255) via hwmon. Device pushes IN reports
+ * at ~1 Hz; no GET_REPORT. OUT reports set PWM duty (bytes 1-10, 0-100%).
+ * PWM is manual-only: the device does not change duty autonomously, only
+ * when it receives an OUT report from the host.
+ */
+
+#include <linux/cache.h>
+#include <linux/completion.h>
+#include <linux/err.h>
+#include <linux/hid.h>
+#include <linux/hwmon.h>
+#include <linux/jiffies.h>
+#include <linux/minmax.h>
+#include <linux/module.h>
+#include <linux/spinlock.h>
+#include <linux/unaligned.h>
+
+#define ARCTIC_VID			0x3904
+#define ARCTIC_PID			0xF001
+#define ARCTIC_NUM_FANS			10
+#define ARCTIC_OUTPUT_REPORT_ID		0x01
+#define ARCTIC_REPORT_LEN		32
+#define ARCTIC_RPM_OFFSET		11	/* bytes 11-30: 10 x uint16 LE */
+/* ACK report: device sends Report ID 0x02, 2 bytes (ID + status) after applying OUT report */
+#define ARCTIC_ACK_REPORT_ID		0x02
+#define ARCTIC_ACK_REPORT_LEN		2
+/*
+ * Time to wait for ACK report after send.
+ * Measured over 500 iterations: max ~563 ms. Keep 1 s as margin.
+ */
+#define ARCTIC_ACK_TIMEOUT_MS		1000
+
+struct arctic_fan_data {
+	struct hid_device *hdev;
+	spinlock_t in_report_lock;	/* protects fan_rpm[], ack_status, write_pending */
+	struct completion in_report_received; /* ACK (ID 0x02) received in raw_event */
+	int ack_status;			/* 0 = OK, negative errno on device error */
+	bool write_pending;		/* true while an OUT report ACK is in flight */
+	u32 fan_rpm[ARCTIC_NUM_FANS];
+	u8 pwm_duty[ARCTIC_NUM_FANS];	/* 0-255 matching sysfs range; converted to 0-100 on send */
+	/*
+	 * OUT report buffer. Cache-line aligned so it occupies its own cache
+	 * line, preventing DMA cache-coherency issues with adjacent fields
+	 * (fan_rpm[], pwm_duty[]) on non-coherent architectures.
+	 * Embedded in the devm_kzalloc'd struct so it is heap-allocated and
+	 * passes usb_hcd_map_urb_for_dma(). Serialized by the hwmon core.
+	 */
+	u8 buf[ARCTIC_REPORT_LEN] ____cacheline_aligned;
+};
+
+/*
+ * Parse RPM values from the periodic status report (10 x uint16 LE at rpm_off).
+ * pwm_duty is not updated from the report: the device is manual-only, so the
+ * host cache is the authoritative source for PWM.
+ * Called from raw_event which may run in IRQ context; must not sleep.
+ */
+static void arctic_fan_parse_report(struct arctic_fan_data *priv, u8 *buf,
+				    int len, int rpm_off)
+{
+	unsigned long flags;
+	int i;
+
+	if (len < rpm_off + 20)
+		return;
+
+	spin_lock_irqsave(&priv->in_report_lock, flags);
+	for (i = 0; i < ARCTIC_NUM_FANS; i++)
+		priv->fan_rpm[i] = get_unaligned_le16(&buf[rpm_off + i * 2]);
+	spin_unlock_irqrestore(&priv->in_report_lock, flags);
+}
+
+/*
+ * raw_event: IN reports.
+ *
+ * Status report: Report ID 0x01, 32 bytes:
+ *   byte 0 = report ID, bytes 1-10 = PWM 0-100%, bytes 11-30 = 10 x RPM uint16 LE.
+ *   Device pushes these at ~1 Hz; no GET_REPORT.
+ *
+ * ACK report: Report ID 0x02, 2 bytes:
+ *   byte 0 = 0x02, byte 1 = status (0x00 = OK, 0x01 = ERROR).
+ *   Sent once after accepting and applying an OUT report (ID 0x01).
+ */
+static int arctic_fan_raw_event(struct hid_device *hdev,
+				struct hid_report *report, u8 *data, int size)
+{
+	struct arctic_fan_data *priv = hid_get_drvdata(hdev);
+	unsigned long flags;
+
+	hid_dbg(hdev, "arctic_fan: raw_event id=%u size=%d\n", report->id, size);
+
+	if (report->id == ARCTIC_ACK_REPORT_ID && size == ARCTIC_ACK_REPORT_LEN) {
+		spin_lock_irqsave(&priv->in_report_lock, flags);
+		/*
+		 * Only deliver if a write is in flight. This prevents a
+		 * late-arriving ACK from a signal-interrupted write from
+		 * erroneously satisfying a subsequent write's completion wait.
+		 */
+		if (priv->write_pending) {
+			priv->ack_status = data[1] == 0x00 ? 0 : -EIO;
+			complete(&priv->in_report_received);
+		}
+		spin_unlock_irqrestore(&priv->in_report_lock, flags);
+		return 0;
+	}
+
+	if (report->id != ARCTIC_OUTPUT_REPORT_ID || size != ARCTIC_REPORT_LEN) {
+		hid_dbg(hdev, "arctic_fan: raw_event id=%u size=%d ignored\n",
+			report->id, size);
+		return 0;
+	}
+
+	arctic_fan_parse_report(priv, data, size, ARCTIC_RPM_OFFSET);
+	return 0;
+}
+
+static umode_t arctic_fan_is_visible(const void *data,
+				     enum hwmon_sensor_types type,
+				     u32 attr, int channel)
+{
+	if (type == hwmon_fan && attr == hwmon_fan_input)
+		return 0444;
+	if (type == hwmon_pwm && attr == hwmon_pwm_input)
+		return 0644;
+	return 0;
+}
+
+static int arctic_fan_read(struct device *dev, enum hwmon_sensor_types type,
+			   u32 attr, int channel, long *val)
+{
+	struct arctic_fan_data *priv = dev_get_drvdata(dev);
+	unsigned long flags;
+
+	if (type == hwmon_fan && attr == hwmon_fan_input) {
+		spin_lock_irqsave(&priv->in_report_lock, flags);
+		*val = priv->fan_rpm[channel];
+		spin_unlock_irqrestore(&priv->in_report_lock, flags);
+		return 0;
+	}
+	if (type == hwmon_pwm && attr == hwmon_pwm_input) {
+		/* pwm_duty is modified only in write(), which the hwmon core serializes */
+		*val = priv->pwm_duty[channel];
+		return 0;
+	}
+	return -EINVAL;
+}
+
+static int arctic_fan_write(struct device *dev, enum hwmon_sensor_types type,
+			    u32 attr, int channel, long val)
+{
+	struct arctic_fan_data *priv = dev_get_drvdata(dev);
+	u8 new_duty = (u8)clamp_val(val, 0, 255);
+	unsigned long flags;
+	long t;
+	int i, ret;
+
+	/*
+	 * The hwmon core holds its lock for the duration of this callback,
+	 * serializing concurrent writes. priv->buf is heap-allocated (embedded
+	 * in the devm_kzalloc'd struct), satisfying usb_hcd_map_urb_for_dma().
+	 *
+	 * Use new_duty for the target channel and the cached value for all
+	 * others. pwm_duty[channel] is committed only after a confirmed ACK so
+	 * a failed or timed-out write does not corrupt the cached duty state.
+	 */
+	priv->buf[0] = ARCTIC_OUTPUT_REPORT_ID;
+	for (i = 0; i < ARCTIC_NUM_FANS; i++)
+		priv->buf[1 + i] = DIV_ROUND_CLOSEST(
+			(unsigned int)(i == channel ? new_duty : priv->pwm_duty[i]) * 100, 255);
+
+	/*
+	 * Set write_pending before sending the report and clear it in every
+	 * exit path. raw_event() calls complete() only when write_pending is
+	 * true, so a stale ACK from a signal-interrupted write cannot
+	 * satisfy the next write's completion wait.
+	 * Serialized by the hwmon core: only one arctic_fan_write() at a time.
+	 * Use irqsave to match the IRQ context in which raw_event may run.
+	 */
+	spin_lock_irqsave(&priv->in_report_lock, flags);
+	priv->ack_status = -ETIMEDOUT;
+	priv->write_pending = true;
+	reinit_completion(&priv->in_report_received);
+	spin_unlock_irqrestore(&priv->in_report_lock, flags);
+
+	ret = hid_hw_output_report(priv->hdev, priv->buf, ARCTIC_REPORT_LEN);
+	if (ret < 0) {
+		spin_lock_irqsave(&priv->in_report_lock, flags);
+		priv->write_pending = false;
+		spin_unlock_irqrestore(&priv->in_report_lock, flags);
+		return ret;
+	}
+
+	t = wait_for_completion_interruptible_timeout(&priv->in_report_received,
+						      msecs_to_jiffies(ARCTIC_ACK_TIMEOUT_MS));
+	spin_lock_irqsave(&priv->in_report_lock, flags);
+	priv->write_pending = false;
+	spin_unlock_irqrestore(&priv->in_report_lock, flags);
+
+	if (t < 0)
+		return t; /* interrupted by signal */
+	if (!t)
+		return -ETIMEDOUT;
+
+	/* Commit the new duty only after hardware has acknowledged it */
+	priv->pwm_duty[channel] = new_duty;
+	return priv->ack_status; /* 0=OK, -EIO=device error */
+}
+
+static const struct hwmon_ops arctic_fan_ops = {
+	.is_visible = arctic_fan_is_visible,
+	.read = arctic_fan_read,
+	.write = arctic_fan_write,
+};
+
+static const struct hwmon_channel_info *arctic_fan_info[] = {
+	HWMON_CHANNEL_INFO(fan,
+			   HWMON_F_INPUT, HWMON_F_INPUT, HWMON_F_INPUT,
+			   HWMON_F_INPUT, HWMON_F_INPUT, HWMON_F_INPUT,
+			   HWMON_F_INPUT, HWMON_F_INPUT, HWMON_F_INPUT,
+			   HWMON_F_INPUT),
+	HWMON_CHANNEL_INFO(pwm,
+			   HWMON_PWM_INPUT, HWMON_PWM_INPUT, HWMON_PWM_INPUT,
+			   HWMON_PWM_INPUT, HWMON_PWM_INPUT, HWMON_PWM_INPUT,
+			   HWMON_PWM_INPUT, HWMON_PWM_INPUT, HWMON_PWM_INPUT,
+			   HWMON_PWM_INPUT),
+	NULL
+};
+
+static const struct hwmon_chip_info arctic_fan_chip_info = {
+	.ops = &arctic_fan_ops,
+	.info = arctic_fan_info,
+};
+
+static void arctic_fan_hw_stop(void *data)
+{
+	struct hid_device *hdev = data;
+
+	hid_hw_close(hdev);
+	hid_hw_stop(hdev);
+}
+
+static int arctic_fan_probe(struct hid_device *hdev,
+			    const struct hid_device_id *id)
+{
+	struct arctic_fan_data *priv;
+	struct device *hwmon_dev;
+	int ret;
+
+	if (!hid_is_usb(hdev))
+		return -ENODEV;
+
+	ret = hid_parse(hdev);
+	if (ret)
+		return ret;
+
+	priv = devm_kzalloc(&hdev->dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->hdev = hdev;
+	spin_lock_init(&priv->in_report_lock);
+	init_completion(&priv->in_report_received);
+	hid_set_drvdata(hdev, priv);
+
+	ret = hid_hw_start(hdev, HID_CONNECT_DRIVER);
+	if (ret)
+		return ret;
+
+	ret = hid_hw_open(hdev);
+	if (ret) {
+		hid_hw_stop(hdev);
+		return ret;
+	}
+
+	/*
+	 * Register hardware teardown before hwmon so that devm cleanup runs in
+	 * LIFO order: hwmon unregistered first, then hid_hw_close/stop. This
+	 * ensures no userspace sysfs write can reach an already stopped device.
+	 */
+	ret = devm_add_action_or_reset(&hdev->dev, arctic_fan_hw_stop, hdev);
+	if (ret)
+		return ret;
+
+	/*
+	 * Start IO before registering with hwmon. If IO were started after
+	 * hwmon registration, a sysfs write arriving in that narrow window
+	 * would send an OUT report but the ACK could not be delivered (the HID
+	 * core discards events until io_started), causing a spurious timeout.
+	 */
+	hid_device_io_start(hdev);
+
+	hwmon_dev = devm_hwmon_device_register_with_info(&hdev->dev, "arctic_fan",
+							 priv, &arctic_fan_chip_info,
+							 NULL);
+	if (IS_ERR(hwmon_dev))
+		return PTR_ERR(hwmon_dev);
+
+	return 0;
+}
+
+static void arctic_fan_remove(struct hid_device *hdev)
+{
+	/* devm cleanup (LIFO) handles hid_hw_close/stop after hwmon unregistration */
+}
+
+static const struct hid_device_id arctic_fan_id_table[] = {
+	{ HID_USB_DEVICE(ARCTIC_VID, ARCTIC_PID) },
+	{ }
+};
+MODULE_DEVICE_TABLE(hid, arctic_fan_id_table);
+
+static struct hid_driver arctic_fan_driver = {
+	.name = "arctic_fan",
+	.id_table = arctic_fan_id_table,
+	.probe = arctic_fan_probe,
+	.remove = arctic_fan_remove,
+	.raw_event = arctic_fan_raw_event,
+};
+
+module_hid_driver(arctic_fan_driver);
+
+MODULE_AUTHOR("Aureo Serrano de Souza <aureo.serrano@arctic.de>");
+MODULE_DESCRIPTION("HID hwmon driver for ARCTIC Fan Controller");
+MODULE_LICENSE("GPL");

^ permalink raw reply related

* RE: [PATCH net-next v3 04/13] net: move promiscuity handling into dev_rx_mode_work
From: Loktionov, Aleksandr @ 2026-03-20  8:01 UTC (permalink / raw)
  To: Stanislav Fomichev, netdev@vger.kernel.org
  Cc: davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
	pabeni@redhat.com, horms@kernel.org, corbet@lwn.net,
	skhan@linuxfoundation.org, andrew+netdev@lunn.ch,
	michael.chan@broadcom.com, pavan.chebbi@broadcom.com,
	Nguyen, Anthony L, Kitszel, Przemyslaw, saeedm@nvidia.com,
	tariqt@nvidia.com, mbloch@nvidia.com, alexanderduyck@fb.com,
	kernel-team@meta.com, johannes@sipsolutions.net,
	sd@queasysnail.net, jianbol@nvidia.com, dtatulea@nvidia.com,
	mohsin.bashr@gmail.com, Keller, Jacob E, willemb@google.com,
	skhawaja@google.com, bestswngs@gmail.com, kees@kernel.org,
	linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org,
	intel-wired-lan@lists.osuosl.org, linux-rdma@vger.kernel.org,
	linux-wireless@vger.kernel.org, linux-kselftest@vger.kernel.org,
	leon@kernel.org
In-Reply-To: <20260320012501.2033548-5-sdf@fomichev.me>



> -----Original Message-----
> From: Stanislav Fomichev <sdf@fomichev.me>
> Sent: Friday, March 20, 2026 2:25 AM
> To: netdev@vger.kernel.org
> Cc: davem@davemloft.net; edumazet@google.com; kuba@kernel.org;
> pabeni@redhat.com; horms@kernel.org; corbet@lwn.net;
> skhan@linuxfoundation.org; andrew+netdev@lunn.ch;
> michael.chan@broadcom.com; pavan.chebbi@broadcom.com; Nguyen, Anthony
> L <anthony.l.nguyen@intel.com>; Kitszel, Przemyslaw
> <przemyslaw.kitszel@intel.com>; saeedm@nvidia.com; tariqt@nvidia.com;
> mbloch@nvidia.com; alexanderduyck@fb.com; kernel-team@meta.com;
> johannes@sipsolutions.net; sd@queasysnail.net; jianbol@nvidia.com;
> dtatulea@nvidia.com; sdf@fomichev.me; mohsin.bashr@gmail.com; Keller,
> Jacob E <jacob.e.keller@intel.com>; willemb@google.com;
> skhawaja@google.com; bestswngs@gmail.com; Loktionov, Aleksandr
> <aleksandr.loktionov@intel.com>; kees@kernel.org; linux-
> doc@vger.kernel.org; linux-kernel@vger.kernel.org; intel-wired-
> lan@lists.osuosl.org; linux-rdma@vger.kernel.org; linux-
> wireless@vger.kernel.org; linux-kselftest@vger.kernel.org;
> leon@kernel.org
> Subject: [PATCH net-next v3 04/13] net: move promiscuity handling into
> dev_rx_mode_work
> 
> Move unicast promiscuity tracking into dev_rx_mode_work so it runs
> under netdev_ops_lock instead of under the addr_lock spinlock. This is
> required because __dev_set_promiscuity calls dev_change_rx_flags and
> __dev_notify_flags, both of which may need to sleep.
> 
> Change ASSERT_RTNL() to netdev_ops_assert_locked() in
> __dev_set_promiscuity, netif_set_allmulti and __dev_change_flags since
> these are now called from the work queue under the ops lock.
> 
> Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
> Signed-off-by: Stanislav Fomichev <sdf@fomichev.me>
> ---
>  Documentation/networking/netdevices.rst |  4 ++
>  net/core/dev.c                          | 79 +++++++++++++++++-------
> -
>  2 files changed, 57 insertions(+), 26 deletions(-)
> 
> diff --git a/Documentation/networking/netdevices.rst
> b/Documentation/networking/netdevices.rst
> index dc83d78d3b27..5cdaa1a3dcc8 100644
> --- a/Documentation/networking/netdevices.rst
> +++ b/Documentation/networking/netdevices.rst
> @@ -298,6 +298,10 @@ struct net_device synchronization rules
>  	Notes: Sleepable version of ndo_set_rx_mode. Receives snapshots
>  	of the unicast and multicast address lists.
> 
> +ndo_change_rx_flags:
> +	Synchronization: rtnl_lock() semaphore. In addition, netdev
> instance
> +	lock if the driver implements queue management or shaper API.
> +
>  ndo_setup_tc:
>  	``TC_SETUP_BLOCK`` and ``TC_SETUP_FT`` are running under NFT
> locks
>  	(i.e. no ``rtnl_lock`` and no device instance lock). The rest
> of diff --git a/net/core/dev.c b/net/core/dev.c index
> fedc423306fc..fc5c9b14faa0 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -9574,7 +9574,7 @@ static int __dev_set_promiscuity(struct
> net_device *dev, int inc, bool notify)
>  	kuid_t uid;
>  	kgid_t gid;
> 
> -	ASSERT_RTNL();
> +	netdev_ops_assert_locked(dev);
Can you explain why do you add new hard precondition of ops lock must be held?


> 
>  	promiscuity = dev->promiscuity + inc;
>  	if (promiscuity == 0) {
> @@ -9610,16 +9610,8 @@ static int __dev_set_promiscuity(struct
> net_device *dev, int inc, bool notify)
> 
>  		dev_change_rx_flags(dev, IFF_PROMISC);
>  	}

...

>  	__hw_addr_init(&uc_snap);
> @@ -9704,16 +9720,29 @@ static void dev_rx_mode_work(struct
> work_struct *work)
>  		if (!err)
>  			err = __hw_addr_list_snapshot(&mc_ref, &dev->mc,
>  						      dev->addr_len);
> -		netif_addr_unlock_bh(dev);
> 
>  		if (err) {
>  			netdev_WARN(dev, "failed to sync uc/mc
> addresses\n");
>  			__hw_addr_flush(&uc_snap);
>  			__hw_addr_flush(&uc_ref);
>  			__hw_addr_flush(&mc_snap);
> +			netif_addr_unlock_bh(dev);
>  			goto out;
>  		}
> 
> +		promisc_inc = dev_uc_promisc_update(dev);
> +
> +		netif_addr_unlock_bh(dev);
> +	} else {
> +		netif_addr_lock_bh(dev);
> +		promisc_inc = dev_uc_promisc_update(dev);
> +		netif_addr_unlock_bh(dev);
> +	}
> +
> +	if (promisc_inc)
> +		__dev_set_promiscuity(dev, promisc_inc, false);
But it's being called here without any netdev_lock_ops(dev) ?

> +
> +	if (ops->ndo_set_rx_mode_async) {
>  		ops->ndo_set_rx_mode_async(dev, &uc_snap, &mc_snap);
> 
>  		netif_addr_lock_bh(dev);
> @@ -9722,6 +9751,10 @@ static void dev_rx_mode_work(struct work_struct
> *work)
>  		__hw_addr_list_reconcile(&dev->mc, &mc_snap,
>  					 &mc_ref, dev->addr_len);
>  		netif_addr_unlock_bh(dev);
> +	} else if (ops->ndo_set_rx_mode) {
> +		netif_addr_lock_bh(dev);
> +		ops->ndo_set_rx_mode(dev);
> +		netif_addr_unlock_bh(dev);
>  	}

...

> --
> 2.53.0


^ permalink raw reply

* RE: [PATCH net-next v3 12/13] net: warn ops-locked drivers still using ndo_set_rx_mode
From: Loktionov, Aleksandr @ 2026-03-20  7:55 UTC (permalink / raw)
  To: Stanislav Fomichev, netdev@vger.kernel.org
  Cc: davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
	pabeni@redhat.com, horms@kernel.org, corbet@lwn.net,
	skhan@linuxfoundation.org, andrew+netdev@lunn.ch,
	michael.chan@broadcom.com, pavan.chebbi@broadcom.com,
	Nguyen, Anthony L, Kitszel, Przemyslaw, saeedm@nvidia.com,
	tariqt@nvidia.com, mbloch@nvidia.com, alexanderduyck@fb.com,
	kernel-team@meta.com, johannes@sipsolutions.net,
	sd@queasysnail.net, jianbol@nvidia.com, dtatulea@nvidia.com,
	mohsin.bashr@gmail.com, Keller, Jacob E, willemb@google.com,
	skhawaja@google.com, bestswngs@gmail.com, kees@kernel.org,
	linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org,
	intel-wired-lan@lists.osuosl.org, linux-rdma@vger.kernel.org,
	linux-wireless@vger.kernel.org, linux-kselftest@vger.kernel.org,
	leon@kernel.org
In-Reply-To: <20260320012501.2033548-13-sdf@fomichev.me>



> -----Original Message-----
> From: Stanislav Fomichev <sdf@fomichev.me>
> Sent: Friday, March 20, 2026 2:25 AM
> To: netdev@vger.kernel.org
> Cc: davem@davemloft.net; edumazet@google.com; kuba@kernel.org;
> pabeni@redhat.com; horms@kernel.org; corbet@lwn.net;
> skhan@linuxfoundation.org; andrew+netdev@lunn.ch;
> michael.chan@broadcom.com; pavan.chebbi@broadcom.com; Nguyen, Anthony
> L <anthony.l.nguyen@intel.com>; Kitszel, Przemyslaw
> <przemyslaw.kitszel@intel.com>; saeedm@nvidia.com; tariqt@nvidia.com;
> mbloch@nvidia.com; alexanderduyck@fb.com; kernel-team@meta.com;
> johannes@sipsolutions.net; sd@queasysnail.net; jianbol@nvidia.com;
> dtatulea@nvidia.com; sdf@fomichev.me; mohsin.bashr@gmail.com; Keller,
> Jacob E <jacob.e.keller@intel.com>; willemb@google.com;
> skhawaja@google.com; bestswngs@gmail.com; Loktionov, Aleksandr
> <aleksandr.loktionov@intel.com>; kees@kernel.org; linux-
> doc@vger.kernel.org; linux-kernel@vger.kernel.org; intel-wired-
> lan@lists.osuosl.org; linux-rdma@vger.kernel.org; linux-
> wireless@vger.kernel.org; linux-kselftest@vger.kernel.org;
> leon@kernel.org
> Subject: [PATCH net-next v3 12/13] net: warn ops-locked drivers still
> using ndo_set_rx_mode
> 
> Now that all in-tree ops-locked drivers have been converted to
> ndo_set_rx_mode_async, add a warning in register_netdevice to catch
> any remaining or newly added drivers that use ndo_set_rx_mode with ops
> locking. This ensures future driver authors are guided toward the
> async path.
> 
> Also route ops-locked devices through dev_rx_mode_work even if they
> lack rx_mode NDOs, to ensure netdev_ops_assert_locked() does not fire
> on the legacy path where only RTNL is held.
> 
> Signed-off-by: Stanislav Fomichev <sdf@fomichev.me>
> ---
>  net/core/dev.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/net/core/dev.c b/net/core/dev.c index
> fc5c9b14faa0..f38ab254708b 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -9779,7 +9779,8 @@ void __dev_set_rx_mode(struct net_device *dev)
>  	if (!netif_up_and_present(dev))
>  		return;
> 
> -	if (ops->ndo_set_rx_mode_async || ops->ndo_change_rx_flags) {
> +	if (ops->ndo_set_rx_mode_async || ops->ndo_change_rx_flags ||
> +	    netdev_need_ops_lock(dev)) {
>  		queue_work(rx_mode_wq, &dev->rx_mode_work);
>  		return;
>  	}
> @@ -11471,6 +11472,11 @@ int register_netdevice(struct net_device
> *dev)
>  		goto err_uninit;
>  	}
> 
> +	if (netdev_need_ops_lock(dev) &&
> +	    dev->netdev_ops->ndo_set_rx_mode &&
> +	    !dev->netdev_ops->ndo_set_rx_mode_async)
> +		netdev_WARN(dev, "ops-locked drivers should use
> +ndo_set_rx_mode_async\n");
> +
>  	ret = netdev_do_alloc_pcpu_stats(dev);
>  	if (ret)
>  		goto err_uninit;
> --
> 2.53.0


Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>


^ permalink raw reply

* RE: [PATCH net-next v3 11/13] dummy: convert to ndo_set_rx_mode_async
From: Loktionov, Aleksandr @ 2026-03-20  7:54 UTC (permalink / raw)
  To: Stanislav Fomichev, netdev@vger.kernel.org
  Cc: davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
	pabeni@redhat.com, horms@kernel.org, corbet@lwn.net,
	skhan@linuxfoundation.org, andrew+netdev@lunn.ch,
	michael.chan@broadcom.com, pavan.chebbi@broadcom.com,
	Nguyen, Anthony L, Kitszel, Przemyslaw, saeedm@nvidia.com,
	tariqt@nvidia.com, mbloch@nvidia.com, alexanderduyck@fb.com,
	kernel-team@meta.com, johannes@sipsolutions.net,
	sd@queasysnail.net, jianbol@nvidia.com, dtatulea@nvidia.com,
	mohsin.bashr@gmail.com, Keller, Jacob E, willemb@google.com,
	skhawaja@google.com, bestswngs@gmail.com, kees@kernel.org,
	linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org,
	intel-wired-lan@lists.osuosl.org, linux-rdma@vger.kernel.org,
	linux-wireless@vger.kernel.org, linux-kselftest@vger.kernel.org,
	leon@kernel.org
In-Reply-To: <20260320012501.2033548-12-sdf@fomichev.me>



> -----Original Message-----
> From: Stanislav Fomichev <sdf@fomichev.me>
> Sent: Friday, March 20, 2026 2:25 AM
> To: netdev@vger.kernel.org
> Cc: davem@davemloft.net; edumazet@google.com; kuba@kernel.org;
> pabeni@redhat.com; horms@kernel.org; corbet@lwn.net;
> skhan@linuxfoundation.org; andrew+netdev@lunn.ch;
> michael.chan@broadcom.com; pavan.chebbi@broadcom.com; Nguyen, Anthony
> L <anthony.l.nguyen@intel.com>; Kitszel, Przemyslaw
> <przemyslaw.kitszel@intel.com>; saeedm@nvidia.com; tariqt@nvidia.com;
> mbloch@nvidia.com; alexanderduyck@fb.com; kernel-team@meta.com;
> johannes@sipsolutions.net; sd@queasysnail.net; jianbol@nvidia.com;
> dtatulea@nvidia.com; sdf@fomichev.me; mohsin.bashr@gmail.com; Keller,
> Jacob E <jacob.e.keller@intel.com>; willemb@google.com;
> skhawaja@google.com; bestswngs@gmail.com; Loktionov, Aleksandr
> <aleksandr.loktionov@intel.com>; kees@kernel.org; linux-
> doc@vger.kernel.org; linux-kernel@vger.kernel.org; intel-wired-
> lan@lists.osuosl.org; linux-rdma@vger.kernel.org; linux-
> wireless@vger.kernel.org; linux-kselftest@vger.kernel.org;
> leon@kernel.org
> Subject: [PATCH net-next v3 11/13] dummy: convert to
> ndo_set_rx_mode_async
> 
> Convert dummy driver from ndo_set_rx_mode to ndo_set_rx_mode_async.
> The dummy driver's set_multicast_list is a no-op, so the conversion is
> straightforward: update the signature and the ops assignment.
> 
> Signed-off-by: Stanislav Fomichev <sdf@fomichev.me>
> ---
>  drivers/net/dummy.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/dummy.c b/drivers/net/dummy.c index
> d6bdad4baadd..f8a4eb365c3d 100644
> --- a/drivers/net/dummy.c
> +++ b/drivers/net/dummy.c
> @@ -47,7 +47,9 @@
>  static int numdummies = 1;
> 
>  /* fake multicast ability */
> -static void set_multicast_list(struct net_device *dev)
> +static void set_multicast_list(struct net_device *dev,
> +			       struct netdev_hw_addr_list *uc,
> +			       struct netdev_hw_addr_list *mc)
>  {
>  }
> 
> @@ -87,7 +89,7 @@ static const struct net_device_ops dummy_netdev_ops
> = {
>  	.ndo_init		= dummy_dev_init,
>  	.ndo_start_xmit		= dummy_xmit,
>  	.ndo_validate_addr	= eth_validate_addr,
> -	.ndo_set_rx_mode	= set_multicast_list,
> +	.ndo_set_rx_mode_async	= set_multicast_list,
>  	.ndo_set_mac_address	= eth_mac_addr,
>  	.ndo_get_stats64	= dummy_get_stats64,
>  	.ndo_change_carrier	= dummy_change_carrier,
> --
> 2.53.0

Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>

^ permalink raw reply

* Re: [PATCH v7 5/5] nvmet-tcp: Support KeyUpdate
From: Christoph Hellwig @ 2026-03-20  7:53 UTC (permalink / raw)
  To: alistair23
  Cc: chuck.lever, hare, kernel-tls-handshake, netdev, linux-kernel,
	linux-doc, linux-nvme, linux-nfs, kbusch, axboe, hch, sagi, kch,
	hare, Alistair Francis
In-Reply-To: <20260304053500.590630-6-alistair.francis@wdc.com>

>  static void nvmet_tcp_free_cmd_buffers(struct nvmet_tcp_cmd *cmd);
> +#ifdef CONFIG_NVME_TARGET_TCP_TLS
> +static int nvmet_tcp_tls_handshake(struct nvmet_tcp_queue *queue,
> +				   enum handshake_key_update_type keyupdate);
> +#endif

Can we find a way to just avoid the ifdefered here and just rely on
compiler dead code elimination?

> +#ifdef CONFIG_NVME_TARGET_TCP_TLS
> +static bool nvmet_tls_key_expired(struct nvmet_tcp_queue *queue, int ret)
> +{
> +	return ret == -EKEYEXPIRED &&
> +		queue->state != NVMET_TCP_Q_DISCONNECTING &&
> +		queue->state != NVMET_TCP_Q_TLS_HANDSHAKE;
> +}
> +#else
> +static bool nvmet_tls_key_expired(struct nvmet_tcp_queue *queue, int ret)
> +{
> +	return false;
> +}
> +#endif

This is a pretty clear candidate for IS_ENABLED().

> +	spin_lock_bh(&queue->state_lock);
> +	if (queue->state == NVMET_TCP_Q_TLS_HANDSHAKE) {
> +		/* Socket closed during handshake */
> +		tls_handshake_cancel(queue->sock->sk);
> +	}
> +	if (queue->state != NVMET_TCP_Q_DISCONNECTING) {
> +		queue->state = NVMET_TCP_Q_DISCONNECTING;
> +		kref_put(&queue->kref, nvmet_tcp_release_queue);
> +	}

switch on the queue state?

> +	ret = nvmet_tcp_tls_handshake(queue, HANDSHAKE_KEY_UPDATE_TYPE_RECEIVED);

Overly lone line.

>  		if (unlikely(ret < 0)) {
> +			if (nvmet_tls_key_expired(queue, ret))
> +					goto done;

This has extra indentation.


^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox