[PATCH v3 0/5] rust: add `bitfield!` macro

[PATCH v3 0/5] rust: add `bitfield!` macro

[Alexandre Courbot]

This is the continuation of the `bitfield!` macro which started
alongside the `register!` one before being temporarily integrated into
it [1].

Thanks for all the feedback on v2; I believe this version addresses all
of it, modulo Eliot's suggestion for more explicit range error messages.
Since this is not fundamentally broken, I'd like to address this in a
follow-up change to keep the series focused on the macro extraction from
`register!`.

This version is based on today's `rust-next`, which happens to be
`7.1-rc1`. If review proves satisfying, I see several possible merge
strategies, the one with the least friction being patches 1 and 2 are
merged into the rust tree this cycle, followed by patches 3-5 via the
I/O and DRM trees during the following cycle. This path would not
require any signed tag.

[1] https://lore.kernel.org/all/20260120-register-v1-0-723a1743b557@nvidia.com/

Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
Changes in v3:
- Split patch 1 into the addition of the `bitfield!` macro and its use
  in the `io` module.
- Mention reserved field names in `bitfield!`'s doccomment.
- Properly order fields in the KUnit test.
- Add a Kconfig option for building the KUnit tests.
- Document behavior on non-covered bits.
- Document support for signed fields and storage types (TL;DR: not
  supported).
- Move `nova-core`'s `bitfield` module deletion into its own patch.
- Link to v2: https://patch.msgid.link/20260409-bitfield-v2-0-23ac400071cb@nvidia.com

---
Alexandre Courbot (4):
      rust: extract `bitfield!` macro from `register!`
      rust: io: use the `bitfield!` macro in `register!`
      gpu: nova-core: switch to kernel bitfield macro
      gpu: nova-core: remove the driver-local `bitfield!` macro

Joel Fernandes (1):
      rust: bitfield: Add KUnit tests for bitfield

 MAINTAINERS                        |   8 +
 drivers/gpu/nova-core/bitfield.rs  | 329 --------------
 drivers/gpu/nova-core/gsp/fw.rs    |  11 +-
 drivers/gpu/nova-core/nova_core.rs |   3 -
 lib/Kconfig.debug                  |  12 +
 rust/kernel/bitfield.rs            | 865 +++++++++++++++++++++++++++++++++++++
 rust/kernel/io/register.rs         | 246 +----------
 rust/kernel/lib.rs                 |   1 +
 8 files changed, 894 insertions(+), 581 deletions(-)
---
base-commit: 254f49634ee16a731174d2ae34bc50bd5f45e731
change-id: 20260408-bitfield-6e18254f4fdc

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

[PATCH v3 1/5] rust: extract `bitfield!` macro from `register!`

[Alexandre Courbot]

Extract the bitfield-defining part of the `register!` macro into an
independent macro used to define bitfield types with bounds-checked
accessors.

Each field is represented as a `Bounded` of the appropriate bit width,
ensuring field values are never silently truncated.

Fields can optionally be converted to/from custom types, either fallibly
or infallibly.

Appropriate documentation is also added, and a MAINTAINERS entry created
for the new module.

Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
Acked-by: Yury Norov <ynorov@nvidia.com>
---
 MAINTAINERS             |   8 +
 rust/kernel/bitfield.rs | 546 ++++++++++++++++++++++++++++++++++++++++++++++++
 rust/kernel/lib.rs      |   1 +
 3 files changed, 555 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 2fb1c75afd16..b685f364cee4 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -23377,6 +23377,14 @@ T:	git https://github.com/Rust-for-Linux/linux.git alloc-next
 F:	rust/kernel/alloc.rs
 F:	rust/kernel/alloc/
 
+RUST [BITFIELD]
+M:	Alexandre Courbot <acourbot@nvidia.com>
+M:	Joel Fernandes <joelagnelf@nvidia.com>
+R:	Yury Norov <yury.norov@gmail.com>
+L:	rust-for-linux@vger.kernel.org
+S:	Maintained
+F:	rust/kernel/bitfield.rs
+
 RUST [INTEROP]
 M:	Joel Fernandes <joelagnelf@nvidia.com>
 M:	Alexandre Courbot <acourbot@nvidia.com>
diff --git a/rust/kernel/bitfield.rs b/rust/kernel/bitfield.rs
new file mode 100644
index 000000000000..4083e7b7a307
--- /dev/null
+++ b/rust/kernel/bitfield.rs
@@ -0,0 +1,546 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Support for defining bitfields as Rust structures.
+//!
+//! The [`bitfield!`](kernel::bitfield!) macro declares integer types that are split into distinct
+//! bit fields of arbitrary length. Each field is typed using [`Bounded`](kernel::num::Bounded) to
+//! ensure values are properly validated and to avoid implicit data loss.
+//!
+//! # Example
+//!
+//! ```rust
+//! use kernel::bitfield;
+//! use kernel::num::Bounded;
+//!
+//! bitfield! {
+//!     pub struct Rgb(u16) {
+//!         15:11 blue;
+//!         10:5 green;
+//!         4:0 red;
+//!     }
+//! }
+//!
+//! // Valid value for the `blue` field.
+//! let blue = Bounded::<u16, 5>::new::<0x18>();
+//!
+//! // Setters can be chained. Values ranges are checked at compile-time.
+//! let color = Rgb::zeroed()
+//!     // Compile-time bounds check of constant value.
+//!     .with_const_red::<0x10>()
+//!     .with_const_green::<0x1f>()
+//!     // A `Bounded` can also be passed.
+//!     .with_blue(blue);
+//!
+//! assert_eq!(color.red(), 0x10);
+//! assert_eq!(color.green(), 0x1f);
+//! assert_eq!(color.blue(), 0x18);
+//! assert_eq!(
+//!     color.into_raw(),
+//!     (0x18 << Rgb::BLUE_SHIFT) + (0x1f << Rgb::GREEN_SHIFT) + 0x10,
+//! );
+//!
+//! // Convert to/from the backing storage type.
+//! let raw: u16 = color.into();
+//! assert_eq!(Rgb::from(raw), color);
+//! ```
+//!
+//! # Syntax
+//!
+//! ```text
+//! bitfield! {
+//!     #[attributes]
+//!     // Documentation for `Name`.
+//!     pub struct Name(storage_type) {
+//!         // `field_1` documentation.
+//!         hi:lo field_1;
+//!         // `field_2` documentation.
+//!         hi:lo field_2 => ConvertedType;
+//!         // `field_3` documentation.
+//!         hi:lo field_3 ?=> ConvertedType;
+//!         ...
+//!     }
+//! }
+//! ```
+//!
+//! - `storage_type`: The underlying unsigned integer type (`u8`, `u16`, `u32`, `u64`).
+//!   Signed integer storage types are not supported.
+//! - `hi:lo`: Bit range (inclusive), where `hi >= lo`.
+//! - `=> Type`: Optional infallible conversion (see [below](#infallible-conversion-)).
+//! - `?=> Type`: Optional fallible conversion (see [below](#fallible-conversion-)).
+//! - Documentation strings and attributes are optional.
+//!
+//! # Generated code
+//!
+//! Each field is internally represented as a [`Bounded`] parameterized by its bit width. Field
+//! values can either be set/retrieved directly, or converted from/to another type.
+//!
+//! The use of `Bounded` for each field enforces bounds-checking (at build time or runtime) of every
+//! value assigned to a field. This ensures that data is never accidentally truncated.
+//!
+//! The macro generates the bitfield type, [`From`] and [`Into`] implementations for its storage
+//! type, as well as [`Debug`] and [`Zeroable`](pin_init::Zeroable) implementations.
+//!
+//! For each field, it also generates:
+//!
+//! - `field()`: Getter method for the field value.
+//! - `with_field(value)`: Infallible setter; the argument type must fit within the field's width.
+//! - `with_const_field::<VALUE>()`: `const` setter; the value is validated at compile time.
+//!   Usually shorter to use than `with_field` for constant values as it doesn't require
+//!   constructing a [`Bounded`].
+//! - `try_with_field(value)`: Fallible setter. Returns an error if the value is out of range.
+//! - `FIELD_MASK`, `FIELD_SHIFT`, `FIELD_RANGE`: Constants for manual bit manipulation.
+//!
+//! # Reserved names for field identifiers
+//!
+//! Field identifiers are used to generate methods and associated constants on the bitfield type.
+//! For a field named `field`, the macro may generate methods named `field`, `with_field`,
+//! `with_const_field`, `try_with_field`, `__field` and `__with_field`, as well as constants named
+//! `FIELD_MASK`, `FIELD_SHIFT` and `FIELD_RANGE`.
+//!
+//! Therefore, field identifiers must not use names that would collide with generated items for
+//! any field in the same bitfield. The following prefixes are thus reserved for field identifiers:
+//!
+//! - `with_`
+//! - `const_`
+//! - `try_with_`
+//! - `__`
+//!
+//! The field identifiers `from_raw`, `into_raw`, and `into` are also reserved.
+//!
+//! In addition, field identifiers should follow Rust `snake_case` conventions, since the associated
+//! constants are generated by uppercasing the field name.
+//!
+//! # Implicit conversions
+//!
+//! Types that fit entirely within a field's bit width can be used directly with setters. For
+//! example, `bool` works with single-bit fields, and `u8` works with 8-bit fields:
+//!
+//! ```rust
+//! use kernel::bitfield;
+//!
+//! bitfield! {
+//!     pub struct Flags(u32) {
+//!         15:8 byte_field;
+//!         0:0 flag;
+//!     }
+//! }
+//!
+//! let flags = Flags::zeroed()
+//!     .with_byte_field(0x42_u8)
+//!     .with_flag(true);
+//!
+//! assert_eq!(flags.into_raw(), (0x42 << Flags::BYTE_FIELD_SHIFT) | 1);
+//! ```
+//!
+//! # Runtime bounds checking
+//!
+//! When a value is not known at compile time, use `try_with_field()` to check bounds at runtime:
+//!
+//! ```rust
+//! use kernel::bitfield;
+//!
+//! bitfield! {
+//!     pub struct Config(u8) {
+//!         3:0 nibble;
+//!     }
+//! }
+//!
+//! fn set_nibble(config: Config, value: u8) -> Result<Config, Error> {
+//!     // Returns `EOVERFLOW` if `value > 0xf`.
+//!     config.try_with_nibble(value)
+//! }
+//! # Ok::<(), Error>(())
+//! ```
+//!
+//! # Type conversion
+//!
+//! Fields can be automatically converted to/from a custom type using `=>` (infallible) or `?=>`
+//! (fallible). The custom type must implement the appropriate `From` or `TryFrom` traits with
+//! `Bounded`.
+//!
+//! ## Infallible conversion (`=>`)
+//!
+//! Use this when all possible bit patterns of a field map to valid values:
+//!
+//! ```rust
+//! use kernel::bitfield;
+//! use kernel::num::Bounded;
+//!
+//! #[derive(Debug, Clone, Copy, PartialEq)]
+//! enum Power {
+//!     Off,
+//!     On,
+//! }
+//!
+//! impl From<Bounded<u32, 1>> for Power {
+//!     fn from(v: Bounded<u32, 1>) -> Self {
+//!         match *v {
+//!             0 => Power::Off,
+//!             _ => Power::On,
+//!         }
+//!     }
+//! }
+//!
+//! impl From<Power> for Bounded<u32, 1> {
+//!     fn from(p: Power) -> Self {
+//!         (p as u32 != 0).into()
+//!     }
+//! }
+//!
+//! bitfield! {
+//!     pub struct Control(u32) {
+//!         0:0 power => Power;
+//!     }
+//! }
+//!
+//! let ctrl = Control::zeroed().with_power(Power::On);
+//! assert_eq!(ctrl.power(), Power::On);
+//! ```
+//!
+//! ## Fallible conversion (`?=>`)
+//!
+//! Use this when some bit patterns of a field are invalid. The getter returns a [`Result`]:
+//!
+//! ```rust
+//! use kernel::bitfield;
+//! use kernel::num::Bounded;
+//!
+//! #[derive(Debug, Clone, Copy, PartialEq)]
+//! enum Mode {
+//!     Low = 0,
+//!     High = 1,
+//!     Auto = 2,
+//!     // 3 is invalid
+//! }
+//!
+//! impl TryFrom<Bounded<u32, 2>> for Mode {
+//!     type Error = u32;
+//!
+//!     fn try_from(v: Bounded<u32, 2>) -> Result<Self, u32> {
+//!         match *v {
+//!             0 => Ok(Mode::Low),
+//!             1 => Ok(Mode::High),
+//!             2 => Ok(Mode::Auto),
+//!             n => Err(n),
+//!         }
+//!     }
+//! }
+//!
+//! impl From<Mode> for Bounded<u32, 2> {
+//!     fn from(m: Mode) -> Self {
+//!         match m {
+//!             Mode::Low => Bounded::<u32, _>::new::<0>(),
+//!             Mode::High => Bounded::<u32, _>::new::<1>(),
+//!             Mode::Auto => Bounded::<u32, _>::new::<2>(),
+//!         }
+//!     }
+//! }
+//!
+//! bitfield! {
+//!     pub struct Config(u32) {
+//!         1:0 mode ?=> Mode;
+//!     }
+//! }
+//!
+//! let cfg = Config::zeroed().with_mode(Mode::Auto);
+//! assert_eq!(cfg.mode(), Ok(Mode::Auto));
+//!
+//! // Invalid bit pattern returns an error.
+//! assert_eq!(Config::from(0b11).mode(), Err(3));
+//! ```
+//!
+//! # Bits outside of declared fields
+//!
+//! Bits of the storage type that are not part of any declared field are preserved by the setter
+//! methods, and can only be modified through `from_raw` or the [`From`] implementation from the
+//! storage type.
+//!
+//! ```rust
+//! use kernel::bitfield;
+//!
+//! bitfield! {
+//!     pub struct Sparse(u8) {
+//!         7:6 high;
+//!         // Bits 5:1 are not covered by any field.
+//!         0:0 low;
+//!     }
+//! }
+//!
+//! // Set the gap bits via `from_raw`, then mutate the declared fields.
+//! let val = Sparse::from_raw(0b0010_1010)
+//!     .with_const_high::<0b11>()
+//!     .with_low(true);
+//!
+//! // Bits 5:1 are unchanged.
+//! assert_eq!(val.into_raw(), 0b1110_1011);
+//! ```
+//!
+//! # Signed field values
+//!
+//! Bitfield storage types are unsigned. Since field getter methods return a [`Bounded`] of the
+//! storage type, fields are also unsigned by default.
+//!
+//! If a field needs to encode a signed value, use a custom conversion type with `=>` or `?=>` to
+//! perform the sign interpretation explicitly.
+//!
+//! [`Bounded`]: kernel::num::Bounded
+
+/// Defines a bitfield struct with bounds-checked accessors for individual bit ranges.
+///
+/// See the [`mod@kernel::bitfield`] module for full documentation and examples.
+#[macro_export]
+macro_rules! bitfield {
+    // Entry point defining the bitfield struct, its implementations and its field accessors.
+    (
+        $(#[$attr:meta])* $vis:vis struct $name:ident($storage:ty) { $($fields:tt)* }
+    ) => {
+        $crate::bitfield!(@core
+            #[allow(non_camel_case_types)]
+            $(#[$attr])* $vis $name $storage
+        );
+        $crate::bitfield!(@fields $vis $name $storage { $($fields)* });
+    };
+
+    // All rules below are helpers.
+
+    // Defines the wrapper `$name` type and its conversions from/to the storage type.
+    (@core $(#[$attr:meta])* $vis:vis $name:ident $storage:ty) => {
+        $(#[$attr])*
+        #[repr(transparent)]
+        #[derive(Clone, Copy, PartialEq, Eq)]
+        $vis struct $name {
+            inner: $storage,
+        }
+
+        #[allow(dead_code)]
+        impl $name {
+            /// Creates a bitfield from a raw value.
+            #[inline(always)]
+            $vis const fn from_raw(value: $storage) -> Self {
+                Self{ inner: value }
+            }
+
+            /// Turns this bitfield into its raw value.
+            ///
+            /// This is similar to the [`From`] implementation, but is shorter to invoke in
+            /// most cases.
+            #[inline(always)]
+            $vis const fn into_raw(self) -> $storage {
+                self.inner
+            }
+        }
+
+        // SAFETY: `$storage` is `Zeroable` and `$name` is transparent.
+        unsafe impl ::pin_init::Zeroable for $name {}
+
+        impl ::core::convert::From<$name> for $storage {
+            #[inline(always)]
+            fn from(val: $name) -> $storage {
+                val.into_raw()
+            }
+        }
+
+        impl ::core::convert::From<$storage> for $name {
+            #[inline(always)]
+            fn from(val: $storage) -> $name {
+                Self::from_raw(val)
+            }
+        }
+    };
+
+    // Definitions requiring knowledge of individual fields: private and public field accessors,
+    // and `Debug` implementation.
+    (@fields $vis:vis $name:ident $storage:ty {
+        $($(#[doc = $doc:expr])* $hi:literal:$lo:literal $field:ident
+            $(?=> $try_into_type:ty)?
+            $(=> $into_type:ty)?
+        ;
+        )*
+    }
+    ) => {
+        #[allow(dead_code)]
+        impl $name {
+        $(
+        $crate::bitfield!(@private_field_accessors $vis $name $storage : $hi:$lo $field);
+        $crate::bitfield!(
+            @public_field_accessors $(#[doc = $doc])* $vis $name $storage : $hi:$lo $field
+            $(?=> $try_into_type)?
+            $(=> $into_type)?
+        );
+        )*
+        }
+
+        $crate::bitfield!(@debug $name { $($field;)* });
+    };
+
+    // Private field accessors working with the exact `Bounded` type for the field.
+    (
+        @private_field_accessors $vis:vis $name:ident $storage:ty : $hi:tt:$lo:tt $field:ident
+    ) => {
+        ::kernel::macros::paste!(
+        $vis const [<$field:upper _RANGE>]: ::core::ops::RangeInclusive<u8> = $lo..=$hi;
+        $vis const [<$field:upper _MASK>]: $storage =
+            ((((1 << $hi) - 1) << 1) + 1) - ((1 << $lo) - 1);
+        $vis const [<$field:upper _SHIFT>]: u32 = $lo;
+        );
+
+        ::kernel::macros::paste!(
+        fn [<__ $field>](self) ->
+            ::kernel::num::Bounded<$storage, { $hi + 1 - $lo }> {
+            // Left shift to align the field's MSB with the storage MSB.
+            const ALIGN_TOP: u32 = $storage::BITS - ($hi + 1);
+            // Right shift to move the top-aligned field to bit 0 of the storage.
+            const ALIGN_BOTTOM: u32 = ALIGN_TOP + $lo;
+
+            // Extract the field using two shifts. `Bounded::shr` produces the correctly-sized
+            // output type.
+            let val = ::kernel::num::Bounded::<$storage, { $storage::BITS }>::from(
+                self.inner << ALIGN_TOP
+            );
+            val.shr::<ALIGN_BOTTOM, { $hi + 1 - $lo } >()
+        }
+
+        const fn [<__with_ $field>](
+            mut self,
+            value: ::kernel::num::Bounded<$storage, { $hi + 1 - $lo }>,
+        ) -> Self
+        {
+            const MASK: $storage = <$name>::[<$field:upper _MASK>];
+            const SHIFT: u32 = <$name>::[<$field:upper _SHIFT>];
+
+            let value = value.get() << SHIFT;
+            self.inner = (self.inner & !MASK) | value;
+
+            self
+        }
+        );
+    };
+
+    // Public accessors for fields infallibly (`=>`) converted to a type.
+    (
+        @public_field_accessors $(#[doc = $doc:expr])* $vis:vis $name:ident $storage:ty :
+            $hi:literal:$lo:literal $field:ident => $into_type:ty
+    ) => {
+        ::kernel::macros::paste!(
+
+        $(#[doc = $doc])*
+        #[doc = "Returns the value of this field."]
+        #[inline(always)]
+        $vis fn $field(self) -> $into_type
+        {
+            self.[<__ $field>]().into()
+        }
+
+        $(#[doc = $doc])*
+        #[doc = "Sets this field to the given `value`."]
+        #[inline(always)]
+        $vis fn [<with_ $field>](self, value: $into_type) -> Self
+        {
+            self.[<__with_ $field>](value.into())
+        }
+
+        );
+    };
+
+    // Public accessors for fields fallibly (`?=>`) converted to a type.
+    (
+        @public_field_accessors $(#[doc = $doc:expr])* $vis:vis $name:ident $storage:ty :
+            $hi:tt:$lo:tt $field:ident ?=> $try_into_type:ty
+    ) => {
+        ::kernel::macros::paste!(
+
+        $(#[doc = $doc])*
+        #[doc = "Returns the value of this field."]
+        #[inline(always)]
+        $vis fn $field(self) ->
+            Result<
+                $try_into_type,
+                <$try_into_type as ::core::convert::TryFrom<
+                    ::kernel::num::Bounded<$storage, { $hi + 1 - $lo }>
+                >>::Error
+            >
+        {
+            self.[<__ $field>]().try_into()
+        }
+
+        $(#[doc = $doc])*
+        #[doc = "Sets this field to the given `value`."]
+        #[inline(always)]
+        $vis fn [<with_ $field>](self, value: $try_into_type) -> Self
+        {
+            self.[<__with_ $field>](value.into())
+        }
+
+        );
+    };
+
+    // Public accessors for fields not converted to a type.
+    (
+        @public_field_accessors $(#[doc = $doc:expr])* $vis:vis $name:ident $storage:ty :
+            $hi:tt:$lo:tt $field:ident
+    ) => {
+        ::kernel::macros::paste!(
+
+        $(#[doc = $doc])*
+        #[doc = "Returns the value of this field."]
+        #[inline(always)]
+        $vis fn $field(self) ->
+            ::kernel::num::Bounded<$storage, { $hi + 1 - $lo }>
+        {
+            self.[<__ $field>]()
+        }
+
+        $(#[doc = $doc])*
+        #[doc = "Sets this field to the compile-time constant `VALUE`."]
+        #[inline(always)]
+        $vis const fn [<with_const_ $field>]<const VALUE: $storage>(self) -> Self {
+            self.[<__with_ $field>](
+                ::kernel::num::Bounded::<$storage, { $hi + 1 - $lo }>::new::<VALUE>()
+            )
+        }
+
+        $(#[doc = $doc])*
+        #[doc = "Sets this field to the given `value`."]
+        #[inline(always)]
+        $vis fn [<with_ $field>]<T>(
+            self,
+            value: T,
+        ) -> Self
+            where T: Into<::kernel::num::Bounded<$storage, { $hi + 1 - $lo }>>,
+        {
+            self.[<__with_ $field>](value.into())
+        }
+
+        $(#[doc = $doc])*
+        #[doc = "Tries to set this field to `value`, returning an error if it is out of range."]
+        #[inline(always)]
+        $vis fn [<try_with_ $field>]<T>(
+            self,
+            value: T,
+        ) -> ::kernel::error::Result<Self>
+            where T: ::kernel::num::TryIntoBounded<$storage, { $hi + 1 - $lo }>,
+        {
+            Ok(
+                self.[<__with_ $field>](
+                    value.try_into_bounded().ok_or(::kernel::error::code::EOVERFLOW)?
+                )
+            )
+        }
+
+        );
+    };
+
+    // `Debug` implementation.
+    (@debug $name:ident { $($field:ident;)* }) => {
+        impl ::kernel::fmt::Debug for $name {
+            fn fmt(&self, f: &mut ::kernel::fmt::Formatter<'_>) -> ::kernel::fmt::Result {
+                f.debug_struct(stringify!($name))
+                    .field("<raw>", &::kernel::prelude::fmt!("{:#x}", self.inner))
+                $(
+                    .field(stringify!($field), &self.$field())
+                )*
+                    .finish()
+            }
+        }
+    };
+}
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index b72b2fbe046d..9512af7156df 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -44,6 +44,7 @@
 pub mod alloc;
 #[cfg(CONFIG_AUXILIARY_BUS)]
 pub mod auxiliary;
+pub mod bitfield;
 pub mod bitmap;
 pub mod bits;
 #[cfg(CONFIG_BLOCK)]

-- 
2.54.0

[PATCH v3 2/5] rust: bitfield: Add KUnit tests for bitfield

[Alexandre Courbot]

From: Joel Fernandes <joelagnelf@nvidia.com>

Add KUNIT tests to make sure the macro is working correctly. The unit
tests are put behind the new `RUST_KERNEL_BITFIELD_KUNIT_TEST` Kconfig
option.

Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
Co-developed-by: Alexandre Courbot <acourbot@nvidia.com>
Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
Reviewed-by: Eliot Courtney <ecourtney@nvidia.com>
---
 lib/Kconfig.debug       |  12 ++
 rust/kernel/bitfield.rs | 319 ++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 331 insertions(+)

diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 8ff5adcfe1e0..916bf066c016 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -3608,6 +3608,18 @@ config RUST_KERNEL_DOCTESTS
 
 	  If unsure, say N.
 
+config RUST_KERNEL_BITFIELD_KUNIT_TEST
+	bool "KUnit tests for the Rust `bitfield!` macro" if !KUNIT_ALL_TESTS
+	depends on RUST && KUNIT=y
+	default KUNIT_ALL_TESTS
+	help
+	  This builds the KUnit tests for the Rust `bitfield!` macro.
+
+	  For more information on KUnit and unit tests in general please refer
+	  to the KUnit documentation in Documentation/dev-tools/kunit/.
+
+	  If unsure, say N.
+
 config RUST_INLINE_HELPERS
 	bool "Inline C helpers into Rust code (EXPERIMENTAL)"
 	depends on RUST && RUSTC_CLANG_LLVM_COMPATIBLE
diff --git a/rust/kernel/bitfield.rs b/rust/kernel/bitfield.rs
index 4083e7b7a307..66358e6371f1 100644
--- a/rust/kernel/bitfield.rs
+++ b/rust/kernel/bitfield.rs
@@ -544,3 +544,322 @@ fn fmt(&self, f: &mut ::kernel::fmt::Formatter<'_>) -> ::kernel::fmt::Result {
         }
     };
 }
+
+#[cfg(CONFIG_RUST_KERNEL_BITFIELD_KUNIT_TEST)]
+#[::kernel::macros::kunit_tests(kernel_bitfield)]
+mod tests {
+    use core::convert::TryFrom;
+
+    use pin_init::Zeroable;
+
+    use kernel::num::Bounded;
+
+    // Enum types for testing => and ?=> conversions
+    #[derive(Debug, Clone, Copy, PartialEq)]
+    enum MemoryType {
+        Unmapped = 0,
+        Normal = 1,
+        Device = 2,
+        Reserved = 3,
+    }
+
+    impl TryFrom<Bounded<u64, 4>> for MemoryType {
+        type Error = u64;
+        fn try_from(value: Bounded<u64, 4>) -> Result<Self, Self::Error> {
+            match value.get() {
+                0 => Ok(MemoryType::Unmapped),
+                1 => Ok(MemoryType::Normal),
+                2 => Ok(MemoryType::Device),
+                3 => Ok(MemoryType::Reserved),
+                _ => Err(value.get()),
+            }
+        }
+    }
+
+    impl From<MemoryType> for Bounded<u64, 4> {
+        fn from(mt: MemoryType) -> Bounded<u64, 4> {
+            Bounded::from_expr(mt as u64)
+        }
+    }
+
+    #[derive(Debug, Clone, Copy, PartialEq)]
+    enum Priority {
+        Low = 0,
+        Medium = 1,
+        High = 2,
+        Critical = 3,
+    }
+
+    impl From<Bounded<u16, 2>> for Priority {
+        fn from(value: Bounded<u16, 2>) -> Self {
+            match value & 0x3 {
+                0 => Priority::Low,
+                1 => Priority::Medium,
+                2 => Priority::High,
+                _ => Priority::Critical,
+            }
+        }
+    }
+
+    impl From<Priority> for Bounded<u16, 2> {
+        fn from(p: Priority) -> Bounded<u16, 2> {
+            Bounded::from_expr(p as u16)
+        }
+    }
+
+    bitfield! {
+        struct TestPageTableEntry(u64) {
+            61:52     available2;
+            51:16     pfn;
+            15:12     mem_type ?=> MemoryType;
+            11:9      available;
+            1:1       writable;
+            0:0       present;
+        }
+    }
+
+    bitfield! {
+        struct TestControlRegister(u16) {
+            15:8      channel;
+            7:4       priority_nibble;
+            5:4       priority => Priority;
+            3:1       mode;
+            0:0       enable;
+        }
+    }
+
+    bitfield! {
+        struct TestStatusRegister(u8) {
+            7:0       full_byte;  // For entire register
+            7:4       reserved;
+            3:2       state;
+            1:1       error;
+            0:0       ready;
+        }
+    }
+
+    #[test]
+    fn test_single_bits() {
+        let mut pte = TestPageTableEntry::zeroed();
+
+        assert!(!pte.present().into_bool());
+        assert!(!pte.writable().into_bool());
+        assert_eq!(u64::from(pte), 0x0);
+
+        pte = pte.with_present(true);
+        assert!(pte.present().into_bool());
+        assert_eq!(u64::from(pte), 0x1);
+
+        pte = pte.with_writable(true);
+        assert!(pte.writable().into_bool());
+        assert_eq!(u64::from(pte), 0x3);
+
+        pte = pte.with_writable(false);
+        assert!(!pte.writable().into_bool());
+        assert_eq!(u64::from(pte), 0x1);
+
+        assert_eq!(pte.available(), 0);
+        pte = pte.with_const_available::<0x5>();
+        assert_eq!(pte.available(), 0x5);
+        assert_eq!(u64::from(pte), 0xA01);
+    }
+
+    #[test]
+    fn test_range_fields() {
+        let mut pte = TestPageTableEntry::zeroed();
+        assert_eq!(u64::from(pte), 0x0);
+
+        pte = pte.with_const_pfn::<0x123456>();
+        assert_eq!(pte.pfn(), 0x123456);
+        assert_eq!(u64::from(pte), 0x1234560000);
+
+        pte = pte.with_const_available::<0x7>();
+        assert_eq!(pte.available(), 0x7);
+        assert_eq!(u64::from(pte), 0x1234560E00);
+
+        pte = pte.with_const_available2::<0x3FF>();
+        assert_eq!(pte.available2(), 0x3FF);
+        assert_eq!(u64::from(pte), 0x3FF0_0012_3456_0E00u64);
+
+        // Test TryFrom with ?=> for MemoryType
+        pte = pte.with_mem_type(MemoryType::Device);
+        assert_eq!(pte.mem_type(), Ok(MemoryType::Device));
+        assert_eq!(u64::from(pte), 0x3FF0_0012_3456_2E00u64);
+
+        pte = pte.with_mem_type(MemoryType::Normal);
+        assert_eq!(pte.mem_type(), Ok(MemoryType::Normal));
+        assert_eq!(u64::from(pte), 0x3FF0_0012_3456_1E00u64);
+
+        // Test all valid values for mem_type
+        pte = pte.with_mem_type(MemoryType::Reserved);
+        assert_eq!(pte.mem_type(), Ok(MemoryType::Reserved));
+        assert_eq!(u64::from(pte), 0x3FF0_0012_3456_3E00u64);
+
+        // Test failure case using mem_type field which has 4 bits (0-15)
+        // MemoryType only handles 0-3, so values 4-15 should return Err
+        let mut raw = pte.into_raw();
+        // Set bits 15:12 to 7 (invalid for MemoryType)
+        raw = (raw & !::kernel::bits::genmask_u64(12..=15)) | (0x7 << 12);
+        let invalid_pte = TestPageTableEntry::from_raw(raw);
+        // Should return Err with the invalid value
+        assert_eq!(invalid_pte.mem_type(), Err(0x7));
+
+        // Test a valid value after testing invalid to ensure both cases work
+        // Set bits 15:12 to 2 (valid: Device)
+        raw = (raw & !::kernel::bits::genmask_u64(12..=15)) | (0x2 << 12);
+        let valid_pte = TestPageTableEntry::from_raw(raw);
+        assert_eq!(valid_pte.mem_type(), Ok(MemoryType::Device));
+
+        const MAX_PFN: u64 = ::kernel::bits::genmask_u64(0..=35);
+        pte = pte.with_const_pfn::<{ MAX_PFN }>();
+        assert_eq!(pte.pfn(), MAX_PFN);
+    }
+
+    #[test]
+    fn test_builder_pattern() {
+        let pte = TestPageTableEntry::zeroed()
+            .with_present(true)
+            .with_writable(true)
+            .with_const_available::<0x7>()
+            .with_const_pfn::<0xABCDEF>()
+            .with_mem_type(MemoryType::Reserved)
+            .with_const_available2::<0x3FF>();
+
+        assert!(pte.present().into_bool());
+        assert!(pte.writable().into_bool());
+        assert_eq!(pte.available(), 0x7);
+        assert_eq!(pte.pfn(), 0xABCDEF);
+        assert_eq!(pte.mem_type(), Ok(MemoryType::Reserved));
+        assert_eq!(pte.available2(), 0x3FF);
+    }
+
+    #[test]
+    fn test_raw_operations() {
+        let raw_value = 0x3FF0000031233E03u64;
+
+        let pte = TestPageTableEntry::from_raw(raw_value);
+        assert_eq!(u64::from(pte), raw_value);
+
+        assert!(pte.present().into_bool());
+        assert!(pte.writable().into_bool());
+        assert_eq!(pte.available(), 0x7);
+        assert_eq!(pte.pfn(), 0x3123);
+        assert_eq!(pte.mem_type(), Ok(MemoryType::Reserved));
+        assert_eq!(pte.available2(), 0x3FF);
+
+        // Test using direct constructor syntax TestStruct(value)
+        let pte2 = TestPageTableEntry::from_raw(raw_value);
+        assert_eq!(u64::from(pte2), raw_value);
+    }
+
+    #[test]
+    fn test_u16_bitfield() {
+        let mut ctrl = TestControlRegister::zeroed();
+
+        assert!(!ctrl.enable().into_bool());
+        assert_eq!(ctrl.mode(), 0);
+        assert_eq!(ctrl.priority(), Priority::Low);
+        assert_eq!(ctrl.priority_nibble(), 0);
+        assert_eq!(ctrl.channel(), 0);
+
+        ctrl = ctrl.with_enable(true);
+        assert!(ctrl.enable().into_bool());
+
+        ctrl = ctrl.with_const_mode::<0x5>();
+        assert_eq!(ctrl.mode(), 0x5);
+
+        // Test From conversion with =>
+        ctrl = ctrl.with_priority(Priority::High);
+        assert_eq!(ctrl.priority(), Priority::High);
+        assert_eq!(ctrl.priority_nibble(), 0x2); // High = 2 in bits 5:4
+
+        ctrl = ctrl.with_channel(0xAB);
+        assert_eq!(ctrl.channel(), 0xAB);
+
+        // Test overlapping fields
+        ctrl = ctrl.with_const_priority_nibble::<0xF>();
+        assert_eq!(ctrl.priority_nibble(), 0xF);
+        assert_eq!(ctrl.priority(), Priority::Critical); // bits 5:4 = 0x3
+
+        let ctrl2 = TestControlRegister::zeroed()
+            .with_enable(true)
+            .with_const_mode::<0x3>()
+            .with_priority(Priority::Medium)
+            .with_channel(0x42);
+
+        assert!(ctrl2.enable().into_bool());
+        assert_eq!(ctrl2.mode(), 0x3);
+        assert_eq!(ctrl2.priority(), Priority::Medium);
+        assert_eq!(ctrl2.channel(), 0x42);
+
+        let raw_value: u16 = 0x4217;
+        let ctrl3 = TestControlRegister::from_raw(raw_value);
+        assert_eq!(u16::from(ctrl3), raw_value);
+        assert!(ctrl3.enable().into_bool());
+        assert_eq!(ctrl3.priority(), Priority::Medium);
+        assert_eq!(ctrl3.priority_nibble(), 0x1);
+        assert_eq!(ctrl3.channel(), 0x42);
+    }
+
+    #[test]
+    fn test_u8_bitfield() {
+        let mut status = TestStatusRegister::zeroed();
+
+        assert!(!status.ready().into_bool());
+        assert!(!status.error().into_bool());
+        assert_eq!(status.state(), 0);
+        assert_eq!(status.reserved(), 0);
+        assert_eq!(status.full_byte(), 0);
+
+        status = status.with_ready(true);
+        assert!(status.ready().into_bool());
+        assert_eq!(status.full_byte(), 0x01);
+
+        status = status.with_error(true);
+        assert!(status.error().into_bool());
+        assert_eq!(status.full_byte(), 0x03);
+
+        status = status.with_const_state::<0x3>();
+        assert_eq!(status.state(), 0x3);
+        assert_eq!(status.full_byte(), 0x0F);
+
+        status = status.with_const_reserved::<0xA>();
+        assert_eq!(status.reserved(), 0xA);
+        assert_eq!(status.full_byte(), 0xAF);
+
+        // Test overlapping field
+        status = status.with_full_byte(0x55);
+        assert_eq!(status.full_byte(), 0x55);
+        assert!(status.ready().into_bool());
+        assert!(!status.error().into_bool());
+        assert_eq!(status.state(), 0x1);
+        assert_eq!(status.reserved(), 0x5);
+
+        let status2 = TestStatusRegister::zeroed()
+            .with_ready(true)
+            .with_const_state::<0x2>()
+            .with_const_reserved::<0x5>();
+
+        assert!(status2.ready().into_bool());
+        assert!(!status2.error().into_bool());
+        assert_eq!(status2.state(), 0x2);
+        assert_eq!(status2.reserved(), 0x5);
+        assert_eq!(status2.full_byte(), 0x59);
+
+        let raw_value: u8 = 0x59;
+        let status3 = TestStatusRegister::from_raw(raw_value);
+        assert_eq!(u8::from(status3), raw_value);
+        assert!(status3.ready().into_bool());
+        assert!(!status3.error().into_bool());
+        assert_eq!(status3.state(), 0x2);
+        assert_eq!(status3.reserved(), 0x5);
+        assert_eq!(status3.full_byte(), 0x59);
+
+        let status4 = TestStatusRegister::from_raw(0xFF);
+        assert!(status4.ready().into_bool());
+        assert!(status4.error().into_bool());
+        assert_eq!(status4.state(), 0x3);
+        assert_eq!(status4.reserved(), 0xF);
+        assert_eq!(status4.full_byte(), 0xFF);
+    }
+}

-- 
2.54.0

[PATCH v3 3/5] rust: io: use the `bitfield!` macro in `register!`

[Alexandre Courbot]

Replace the local bitfield rules by the equivalent invocation of the
`bitfield!` macro.

No functional change should be introduced as the `bitfield!` macro has
been extracted from the rules of `register!`.

Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
 rust/kernel/io/register.rs | 246 +--------------------------------------------
 1 file changed, 2 insertions(+), 244 deletions(-)

diff --git a/rust/kernel/io/register.rs b/rust/kernel/io/register.rs
index abc49926abfe..388647f28292 100644
--- a/rust/kernel/io/register.rs
+++ b/rust/kernel/io/register.rs
@@ -956,11 +956,10 @@ macro_rules! register {
     (
         @bitfield $(#[$attr:meta])* $vis:vis struct $name:ident($storage:ty) { $($fields:tt)* }
     ) => {
-        $crate::register!(@bitfield_core
+        $crate::bitfield!(
             #[allow(non_camel_case_types)]
-            $(#[$attr])* $vis $name $storage
+            $(#[$attr])* $vis struct $name($storage) { $($fields)* }
         );
-        $crate::register!(@bitfield_fields $vis $name $storage { $($fields)* });
     };
 
     // Implementations shared by all registers types.
@@ -1016,245 +1015,4 @@ impl $crate::io::register::RegisterArray for $name {
 
         impl $crate::io::register::RelativeRegisterArray for $name {}
     };
-
-    // Defines the wrapper `$name` type and its conversions from/to the storage type.
-    (@bitfield_core $(#[$attr:meta])* $vis:vis $name:ident $storage:ty) => {
-        $(#[$attr])*
-        #[repr(transparent)]
-        #[derive(Clone, Copy, PartialEq, Eq)]
-        $vis struct $name {
-            inner: $storage,
-        }
-
-        #[allow(dead_code)]
-        impl $name {
-            /// Creates a bitfield from a raw value.
-            #[inline(always)]
-            $vis const fn from_raw(value: $storage) -> Self {
-                Self{ inner: value }
-            }
-
-            /// Turns this bitfield into its raw value.
-            ///
-            /// This is similar to the [`From`] implementation, but is shorter to invoke in
-            /// most cases.
-            #[inline(always)]
-            $vis const fn into_raw(self) -> $storage {
-                self.inner
-            }
-        }
-
-        // SAFETY: `$storage` is `Zeroable` and `$name` is transparent.
-        unsafe impl ::pin_init::Zeroable for $name {}
-
-        impl ::core::convert::From<$name> for $storage {
-            #[inline(always)]
-            fn from(val: $name) -> $storage {
-                val.into_raw()
-            }
-        }
-
-        impl ::core::convert::From<$storage> for $name {
-            #[inline(always)]
-            fn from(val: $storage) -> $name {
-                Self::from_raw(val)
-            }
-        }
-    };
-
-    // Definitions requiring knowledge of individual fields: private and public field accessors,
-    // and `Debug` implementation.
-    (@bitfield_fields $vis:vis $name:ident $storage:ty {
-        $($(#[doc = $doc:expr])* $hi:literal:$lo:literal $field:ident
-            $(?=> $try_into_type:ty)?
-            $(=> $into_type:ty)?
-        ;
-        )*
-    }
-    ) => {
-        #[allow(dead_code)]
-        impl $name {
-        $(
-        $crate::register!(@private_field_accessors $vis $name $storage : $hi:$lo $field);
-        $crate::register!(
-            @public_field_accessors $(#[doc = $doc])* $vis $name $storage : $hi:$lo $field
-            $(?=> $try_into_type)?
-            $(=> $into_type)?
-        );
-        )*
-        }
-
-        $crate::register!(@debug $name { $($field;)* });
-    };
-
-    // Private field accessors working with the exact `Bounded` type for the field.
-    (
-        @private_field_accessors $vis:vis $name:ident $storage:ty : $hi:tt:$lo:tt $field:ident
-    ) => {
-        ::kernel::macros::paste!(
-        $vis const [<$field:upper _RANGE>]: ::core::ops::RangeInclusive<u8> = $lo..=$hi;
-        $vis const [<$field:upper _MASK>]: $storage =
-            ((((1 << $hi) - 1) << 1) + 1) - ((1 << $lo) - 1);
-        $vis const [<$field:upper _SHIFT>]: u32 = $lo;
-        );
-
-        ::kernel::macros::paste!(
-        fn [<__ $field>](self) ->
-            ::kernel::num::Bounded<$storage, { $hi + 1 - $lo }> {
-            // Left shift to align the field's MSB with the storage MSB.
-            const ALIGN_TOP: u32 = $storage::BITS - ($hi + 1);
-            // Right shift to move the top-aligned field to bit 0 of the storage.
-            const ALIGN_BOTTOM: u32 = ALIGN_TOP + $lo;
-
-            // Extract the field using two shifts. `Bounded::shr` produces the correctly-sized
-            // output type.
-            let val = ::kernel::num::Bounded::<$storage, { $storage::BITS }>::from(
-                self.inner << ALIGN_TOP
-            );
-            val.shr::<ALIGN_BOTTOM, { $hi + 1 - $lo } >()
-        }
-
-        const fn [<__with_ $field>](
-            mut self,
-            value: ::kernel::num::Bounded<$storage, { $hi + 1 - $lo }>,
-        ) -> Self
-        {
-            const MASK: $storage = <$name>::[<$field:upper _MASK>];
-            const SHIFT: u32 = <$name>::[<$field:upper _SHIFT>];
-
-            let value = value.get() << SHIFT;
-            self.inner = (self.inner & !MASK) | value;
-
-            self
-        }
-        );
-    };
-
-    // Public accessors for fields infallibly (`=>`) converted to a type.
-    (
-        @public_field_accessors $(#[doc = $doc:expr])* $vis:vis $name:ident $storage:ty :
-            $hi:literal:$lo:literal $field:ident => $into_type:ty
-    ) => {
-        ::kernel::macros::paste!(
-
-        $(#[doc = $doc])*
-        #[doc = "Returns the value of this field."]
-        #[inline(always)]
-        $vis fn $field(self) -> $into_type
-        {
-            self.[<__ $field>]().into()
-        }
-
-        $(#[doc = $doc])*
-        #[doc = "Sets this field to the given `value`."]
-        #[inline(always)]
-        $vis fn [<with_ $field>](self, value: $into_type) -> Self
-        {
-            self.[<__with_ $field>](value.into())
-        }
-
-        );
-    };
-
-    // Public accessors for fields fallibly (`?=>`) converted to a type.
-    (
-        @public_field_accessors $(#[doc = $doc:expr])* $vis:vis $name:ident $storage:ty :
-            $hi:tt:$lo:tt $field:ident ?=> $try_into_type:ty
-    ) => {
-        ::kernel::macros::paste!(
-
-        $(#[doc = $doc])*
-        #[doc = "Returns the value of this field."]
-        #[inline(always)]
-        $vis fn $field(self) ->
-            Result<
-                $try_into_type,
-                <$try_into_type as ::core::convert::TryFrom<
-                    ::kernel::num::Bounded<$storage, { $hi + 1 - $lo }>
-                >>::Error
-            >
-        {
-            self.[<__ $field>]().try_into()
-        }
-
-        $(#[doc = $doc])*
-        #[doc = "Sets this field to the given `value`."]
-        #[inline(always)]
-        $vis fn [<with_ $field>](self, value: $try_into_type) -> Self
-        {
-            self.[<__with_ $field>](value.into())
-        }
-
-        );
-    };
-
-    // Public accessors for fields not converted to a type.
-    (
-        @public_field_accessors $(#[doc = $doc:expr])* $vis:vis $name:ident $storage:ty :
-            $hi:tt:$lo:tt $field:ident
-    ) => {
-        ::kernel::macros::paste!(
-
-        $(#[doc = $doc])*
-        #[doc = "Returns the value of this field."]
-        #[inline(always)]
-        $vis fn $field(self) ->
-            ::kernel::num::Bounded<$storage, { $hi + 1 - $lo }>
-        {
-            self.[<__ $field>]()
-        }
-
-        $(#[doc = $doc])*
-        #[doc = "Sets this field to the compile-time constant `VALUE`."]
-        #[inline(always)]
-        $vis const fn [<with_const_ $field>]<const VALUE: $storage>(self) -> Self {
-            self.[<__with_ $field>](
-                ::kernel::num::Bounded::<$storage, { $hi + 1 - $lo }>::new::<VALUE>()
-            )
-        }
-
-        $(#[doc = $doc])*
-        #[doc = "Sets this field to the given `value`."]
-        #[inline(always)]
-        $vis fn [<with_ $field>]<T>(
-            self,
-            value: T,
-        ) -> Self
-            where T: Into<::kernel::num::Bounded<$storage, { $hi + 1 - $lo }>>,
-        {
-            self.[<__with_ $field>](value.into())
-        }
-
-        $(#[doc = $doc])*
-        #[doc = "Tries to set this field to `value`, returning an error if it is out of range."]
-        #[inline(always)]
-        $vis fn [<try_with_ $field>]<T>(
-            self,
-            value: T,
-        ) -> ::kernel::error::Result<Self>
-            where T: ::kernel::num::TryIntoBounded<$storage, { $hi + 1 - $lo }>,
-        {
-            Ok(
-                self.[<__with_ $field>](
-                    value.try_into_bounded().ok_or(::kernel::error::code::EOVERFLOW)?
-                )
-            )
-        }
-
-        );
-    };
-
-    // `Debug` implementation.
-    (@debug $name:ident { $($field:ident;)* }) => {
-        impl ::kernel::fmt::Debug for $name {
-            fn fmt(&self, f: &mut ::kernel::fmt::Formatter<'_>) -> ::kernel::fmt::Result {
-                f.debug_struct(stringify!($name))
-                    .field("<raw>", &::kernel::prelude::fmt!("{:#x}", self.inner))
-                $(
-                    .field(stringify!($field), &self.$field())
-                )*
-                    .finish()
-            }
-        }
-    };
 }

-- 
2.54.0

[PATCH v3 4/5] gpu: nova-core: switch to kernel bitfield macro

[Alexandre Courbot]

Replace uses of the Nova-internal `bitfield!` macro with the kernel one.

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

diff --git a/drivers/gpu/nova-core/gsp/fw.rs b/drivers/gpu/nova-core/gsp/fw.rs
index 0c8a74f0e8ac..d54688904740 100644
--- a/drivers/gpu/nova-core/gsp/fw.rs
+++ b/drivers/gpu/nova-core/gsp/fw.rs
@@ -9,6 +9,7 @@
 use core::ops::Range;
 
 use kernel::{
+    bitfield,
     dma::Coherent,
     prelude::*,
     ptr::{
@@ -728,8 +729,8 @@ unsafe impl AsBytes for MsgqRxHeader {}
 
 bitfield! {
     struct MsgHeaderVersion(u32) {
-        31:24 major as u8;
-        23:16 minor as u8;
+        31:24 major;
+        23:16 minor;
     }
 }
 
@@ -738,9 +739,9 @@ impl MsgHeaderVersion {
     const MINOR_TOT: u8 = 0;
 
     fn new() -> Self {
-        Self::default()
-            .set_major(Self::MAJOR_TOT)
-            .set_minor(Self::MINOR_TOT)
+        Self::zeroed()
+            .with_major(Self::MAJOR_TOT)
+            .with_minor(Self::MINOR_TOT)
     }
 }
 
diff --git a/drivers/gpu/nova-core/nova_core.rs b/drivers/gpu/nova-core/nova_core.rs
index 04a1fa6b25f8..3a0c45481a92 100644
--- a/drivers/gpu/nova-core/nova_core.rs
+++ b/drivers/gpu/nova-core/nova_core.rs
@@ -10,9 +10,6 @@
     InPlaceModule, //
 };
 
-#[macro_use]
-mod bitfield;
-
 mod driver;
 mod falcon;
 mod fb;

-- 
2.54.0

[PATCH v3 5/5] gpu: nova-core: remove the driver-local `bitfield!` macro

[Alexandre Courbot]

This module is now orphaned code replaced by a kernel-global
implementation, so remove it.

Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
Reviewed-by: Eliot Courtney <ecourtney@nvidia.com>
---
 drivers/gpu/nova-core/bitfield.rs | 329 --------------------------------------
 1 file changed, 329 deletions(-)

diff --git a/drivers/gpu/nova-core/bitfield.rs b/drivers/gpu/nova-core/bitfield.rs
deleted file mode 100644
index 02efdcf78d89..000000000000
--- a/drivers/gpu/nova-core/bitfield.rs
+++ /dev/null
@@ -1,329 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-
-//! Bitfield library for Rust structures
-//!
-//! Support for defining bitfields in Rust structures. Also used by the [`register!`] macro.
-
-/// Defines a struct with accessors to access bits within an inner unsigned integer.
-///
-/// # Syntax
-///
-/// ```rust
-/// use nova_core::bitfield;
-///
-/// #[derive(Debug, Clone, Copy, Default)]
-/// enum Mode {
-///     #[default]
-///     Low = 0,
-///     High = 1,
-///     Auto = 2,
-/// }
-///
-/// impl TryFrom<u8> for Mode {
-///     type Error = u8;
-///     fn try_from(value: u8) -> Result<Self, Self::Error> {
-///         match value {
-///             0 => Ok(Mode::Low),
-///             1 => Ok(Mode::High),
-///             2 => Ok(Mode::Auto),
-///             _ => Err(value),
-///         }
-///     }
-/// }
-///
-/// impl From<Mode> for u8 {
-///     fn from(mode: Mode) -> u8 {
-///         mode as u8
-///     }
-/// }
-///
-/// #[derive(Debug, Clone, Copy, Default)]
-/// enum State {
-///     #[default]
-///     Inactive = 0,
-///     Active = 1,
-/// }
-///
-/// impl From<bool> for State {
-///     fn from(value: bool) -> Self {
-///         if value { State::Active } else { State::Inactive }
-///     }
-/// }
-///
-/// impl From<State> for bool {
-///     fn from(state: State) -> bool {
-///         match state {
-///             State::Inactive => false,
-///             State::Active => true,
-///         }
-///     }
-/// }
-///
-/// bitfield! {
-///     pub struct ControlReg(u32) {
-///         7:7 state as bool => State;
-///         3:0 mode as u8 ?=> Mode;
-///     }
-/// }
-/// ```
-///
-/// This generates a struct with:
-/// - Field accessors: `mode()`, `state()`, etc.
-/// - Field setters: `set_mode()`, `set_state()`, etc. (supports chaining with builder pattern).
-///   Note that the compiler will error out if the size of the setter's arg exceeds the
-///   struct's storage size.
-/// - Debug and Default implementations.
-///
-/// Note: Field accessors and setters inherit the same visibility as the struct itself.
-/// In the example above, both `mode()` and `set_mode()` methods will be `pub`.
-///
-/// Fields are defined as follows:
-///
-/// - `as <type>` simply returns the field value casted to <type>, typically `u32`, `u16`, `u8` or
-///   `bool`. Note that `bool` fields must have a range of 1 bit.
-/// - `as <type> => <into_type>` calls `<into_type>`'s `From::<<type>>` implementation and returns
-///   the result.
-/// - `as <type> ?=> <try_into_type>` calls `<try_into_type>`'s `TryFrom::<<type>>` implementation
-///   and returns the result. This is useful with fields for which not all values are valid.
-macro_rules! bitfield {
-    // Main entry point - defines the bitfield struct with fields
-    ($vis:vis struct $name:ident($storage:ty) $(, $comment:literal)? { $($fields:tt)* }) => {
-        bitfield!(@core $vis $name $storage $(, $comment)? { $($fields)* });
-    };
-
-    // All rules below are helpers.
-
-    // Defines the wrapper `$name` type, as well as its relevant implementations (`Debug`,
-    // `Default`, and conversion to the value type) and field accessor methods.
-    (@core $vis:vis $name:ident $storage:ty $(, $comment:literal)? { $($fields:tt)* }) => {
-        $(
-        #[doc=$comment]
-        )?
-        #[repr(transparent)]
-        #[derive(Clone, Copy)]
-        $vis struct $name($storage);
-
-        impl ::core::convert::From<$name> for $storage {
-            fn from(val: $name) -> $storage {
-                val.0
-            }
-        }
-
-        bitfield!(@fields_dispatcher $vis $name $storage { $($fields)* });
-    };
-
-    // Captures the fields and passes them to all the implementers that require field information.
-    //
-    // Used to simplify the matching rules for implementers, so they don't need to match the entire
-    // complex fields rule even though they only make use of part of it.
-    (@fields_dispatcher $vis:vis $name:ident $storage:ty {
-        $($hi:tt:$lo:tt $field:ident as $type:tt
-            $(?=> $try_into_type:ty)?
-            $(=> $into_type:ty)?
-            $(, $comment:literal)?
-        ;
-        )*
-    }
-    ) => {
-        bitfield!(@field_accessors $vis $name $storage {
-            $(
-                $hi:$lo $field as $type
-                $(?=> $try_into_type)?
-                $(=> $into_type)?
-                $(, $comment)?
-            ;
-            )*
-        });
-        bitfield!(@debug $name { $($field;)* });
-        bitfield!(@default $name { $($field;)* });
-    };
-
-    // Defines all the field getter/setter methods for `$name`.
-    (
-        @field_accessors $vis:vis $name:ident $storage:ty {
-        $($hi:tt:$lo:tt $field:ident as $type:tt
-            $(?=> $try_into_type:ty)?
-            $(=> $into_type:ty)?
-            $(, $comment:literal)?
-        ;
-        )*
-        }
-    ) => {
-        $(
-            bitfield!(@check_field_bounds $hi:$lo $field as $type);
-        )*
-
-        #[allow(dead_code)]
-        impl $name {
-            $(
-            bitfield!(@field_accessor $vis $name $storage, $hi:$lo $field as $type
-                $(?=> $try_into_type)?
-                $(=> $into_type)?
-                $(, $comment)?
-                ;
-            );
-            )*
-        }
-    };
-
-    // Boolean fields must have `$hi == $lo`.
-    (@check_field_bounds $hi:tt:$lo:tt $field:ident as bool) => {
-        #[allow(clippy::eq_op)]
-        const _: () = {
-            ::kernel::build_assert!(
-                $hi == $lo,
-                concat!("boolean field `", stringify!($field), "` covers more than one bit")
-            );
-        };
-    };
-
-    // Non-boolean fields must have `$hi >= $lo`.
-    (@check_field_bounds $hi:tt:$lo:tt $field:ident as $type:tt) => {
-        #[allow(clippy::eq_op)]
-        const _: () = {
-            ::kernel::build_assert!(
-                $hi >= $lo,
-                concat!("field `", stringify!($field), "`'s MSB is smaller than its LSB")
-            );
-        };
-    };
-
-    // Catches fields defined as `bool` and convert them into a boolean value.
-    (
-        @field_accessor $vis:vis $name:ident $storage:ty, $hi:tt:$lo:tt $field:ident as bool
-            => $into_type:ty $(, $comment:literal)?;
-    ) => {
-        bitfield!(
-            @leaf_accessor $vis $name $storage, $hi:$lo $field
-            { |f| <$into_type>::from(f != 0) }
-            bool $into_type => $into_type $(, $comment)?;
-        );
-    };
-
-    // Shortcut for fields defined as `bool` without the `=>` syntax.
-    (
-        @field_accessor $vis:vis $name:ident $storage:ty, $hi:tt:$lo:tt $field:ident as bool
-            $(, $comment:literal)?;
-    ) => {
-        bitfield!(
-            @field_accessor $vis $name $storage, $hi:$lo $field as bool => bool $(, $comment)?;
-        );
-    };
-
-    // Catches the `?=>` syntax for non-boolean fields.
-    (
-        @field_accessor $vis:vis $name:ident $storage:ty, $hi:tt:$lo:tt $field:ident as $type:tt
-            ?=> $try_into_type:ty $(, $comment:literal)?;
-    ) => {
-        bitfield!(@leaf_accessor $vis $name $storage, $hi:$lo $field
-            { |f| <$try_into_type>::try_from(f as $type) } $type $try_into_type =>
-            ::core::result::Result<
-                $try_into_type,
-                <$try_into_type as ::core::convert::TryFrom<$type>>::Error
-            >
-            $(, $comment)?;);
-    };
-
-    // Catches the `=>` syntax for non-boolean fields.
-    (
-        @field_accessor $vis:vis $name:ident $storage:ty, $hi:tt:$lo:tt $field:ident as $type:tt
-            => $into_type:ty $(, $comment:literal)?;
-    ) => {
-        bitfield!(@leaf_accessor $vis $name $storage, $hi:$lo $field
-            { |f| <$into_type>::from(f as $type) } $type $into_type => $into_type $(, $comment)?;);
-    };
-
-    // Shortcut for non-boolean fields defined without the `=>` or `?=>` syntax.
-    (
-        @field_accessor $vis:vis $name:ident $storage:ty, $hi:tt:$lo:tt $field:ident as $type:tt
-            $(, $comment:literal)?;
-    ) => {
-        bitfield!(
-            @field_accessor $vis $name $storage, $hi:$lo $field as $type => $type $(, $comment)?;
-        );
-    };
-
-    // Generates the accessor methods for a single field.
-    (
-        @leaf_accessor $vis:vis $name:ident $storage:ty, $hi:tt:$lo:tt $field:ident
-            { $process:expr } $prim_type:tt $to_type:ty => $res_type:ty $(, $comment:literal)?;
-    ) => {
-        ::kernel::macros::paste!(
-        const [<$field:upper _RANGE>]: ::core::ops::RangeInclusive<u8> = $lo..=$hi;
-        const [<$field:upper _MASK>]: $storage = {
-            // Generate mask for shifting
-            match ::core::mem::size_of::<$storage>() {
-                1 => ::kernel::bits::genmask_u8($lo..=$hi) as $storage,
-                2 => ::kernel::bits::genmask_u16($lo..=$hi) as $storage,
-                4 => ::kernel::bits::genmask_u32($lo..=$hi) as $storage,
-                8 => ::kernel::bits::genmask_u64($lo..=$hi) as $storage,
-                _ => ::kernel::build_error!("Unsupported storage type size")
-            }
-        };
-        const [<$field:upper _SHIFT>]: u32 = $lo;
-        );
-
-        $(
-        #[doc="Returns the value of this field:"]
-        #[doc=$comment]
-        )?
-        #[inline(always)]
-        $vis fn $field(self) -> $res_type {
-            ::kernel::macros::paste!(
-            const MASK: $storage = $name::[<$field:upper _MASK>];
-            const SHIFT: u32 = $name::[<$field:upper _SHIFT>];
-            );
-            let field = ((self.0 & MASK) >> SHIFT);
-
-            $process(field)
-        }
-
-        ::kernel::macros::paste!(
-        $(
-        #[doc="Sets the value of this field:"]
-        #[doc=$comment]
-        )?
-        #[inline(always)]
-        $vis fn [<set_ $field>](mut self, value: $to_type) -> Self {
-            const MASK: $storage = $name::[<$field:upper _MASK>];
-            const SHIFT: u32 = $name::[<$field:upper _SHIFT>];
-            let value = ($storage::from($prim_type::from(value)) << SHIFT) & MASK;
-            self.0 = (self.0 & !MASK) | value;
-
-            self
-        }
-        );
-    };
-
-    // Generates the `Debug` implementation for `$name`.
-    (@debug $name:ident { $($field:ident;)* }) => {
-        impl ::kernel::fmt::Debug for $name {
-            fn fmt(&self, f: &mut ::kernel::fmt::Formatter<'_>) -> ::kernel::fmt::Result {
-                f.debug_struct(stringify!($name))
-                    .field("<raw>", &::kernel::prelude::fmt!("{:#x}", &self.0))
-                $(
-                    .field(stringify!($field), &self.$field())
-                )*
-                    .finish()
-            }
-        }
-    };
-
-    // Generates the `Default` implementation for `$name`.
-    (@default $name:ident { $($field:ident;)* }) => {
-        /// Returns a value for the bitfield where all fields are set to their default value.
-        impl ::core::default::Default for $name {
-            fn default() -> Self {
-                let value = Self(Default::default());
-
-                ::kernel::macros::paste!(
-                $(
-                let value = value.[<set_ $field>](Default::default());
-                )*
-                );
-
-                value
-            }
-        }
-    };
-}

-- 
2.54.0

Re: [PATCH v3 2/5] rust: bitfield: Add KUnit tests for bitfield

[Yury Norov]

On Fri, May 01, 2026 at 03:03:19PM +0900, Alexandre Courbot wrote:
> From: Joel Fernandes <joelagnelf@nvidia.com>
> 
> Add KUNIT tests to make sure the macro is working correctly. The unit
> tests are put behind the new `RUST_KERNEL_BITFIELD_KUNIT_TEST` Kconfig
> option.
> 
> Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
> Co-developed-by: Alexandre Courbot <acourbot@nvidia.com>
> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
> Reviewed-by: Eliot Courtney <ecourtney@nvidia.com>
> ---
>  lib/Kconfig.debug       |  12 ++
>  rust/kernel/bitfield.rs | 319 ++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 331 insertions(+)
 
> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index 8ff5adcfe1e0..916bf066c016 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -3608,6 +3608,18 @@ config RUST_KERNEL_DOCTESTS
>  
>  	  If unsure, say N.
>  
> +config RUST_KERNEL_BITFIELD_KUNIT_TEST

Those KUnit tests are all kernel unit tests. Maybe just 
RUST_BITFIELD_KUNIT_TEST?

> +	bool "KUnit tests for the Rust `bitfield!` macro" if !KUNIT_ALL_TESTS
> +	depends on RUST && KUNIT=y
> +	default KUNIT_ALL_TESTS
> +	help
> +	  This builds the KUnit tests for the Rust `bitfield!` macro.
> +
> +	  For more information on KUnit and unit tests in general please refer
> +	  to the KUnit documentation in Documentation/dev-tools/kunit/.

This sentence travels from config to config, adding no value. Maybe
drop it?

> +	  If unsure, say N.
> +

Can you build the series on top of my "rust: add Kconfig.test" series,
and add this config to the rust/kernel/Kconfig.test?

It would be better, because the series creates a "Rust KUnit tests"
menu, so all the tests will be in the same place in menuconfig.

Thanks,
Yury

Re: [PATCH v3 3/5] rust: io: use the `bitfield!` macro in `register!`

[Yury Norov]

On Fri, May 01, 2026 at 03:03:20PM +0900, Alexandre Courbot wrote:
> Replace the local bitfield rules by the equivalent invocation of the
> `bitfield!` macro.
> 
> No functional change should be introduced as the `bitfield!` macro has
> been extracted from the rules of `register!`.
> 
> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>

Acked-by: Yury Norov <yury.norov@gmail.com>

If it comes to another round, maybe split switching to a bitfields and
getting rid of the bitfield_core? For maintainability reasons.
> ---
>  rust/kernel/io/register.rs | 246 +--------------------------------------------
>  1 file changed, 2 insertions(+), 244 deletions(-)
> 
> diff --git a/rust/kernel/io/register.rs b/rust/kernel/io/register.rs
> index abc49926abfe..388647f28292 100644
> --- a/rust/kernel/io/register.rs
> +++ b/rust/kernel/io/register.rs
> @@ -956,11 +956,10 @@ macro_rules! register {
>      (
>          @bitfield $(#[$attr:meta])* $vis:vis struct $name:ident($storage:ty) { $($fields:tt)* }
>      ) => {
> -        $crate::register!(@bitfield_core
> +        $crate::bitfield!(
>              #[allow(non_camel_case_types)]
> -            $(#[$attr])* $vis $name $storage
> +            $(#[$attr])* $vis struct $name($storage) { $($fields)* }
>          );
> -        $crate::register!(@bitfield_fields $vis $name $storage { $($fields)* });
>      };
>  
>      // Implementations shared by all registers types.
> @@ -1016,245 +1015,4 @@ impl $crate::io::register::RegisterArray for $name {
>  
>          impl $crate::io::register::RelativeRegisterArray for $name {}
>      };
> -
> -    // Defines the wrapper `$name` type and its conversions from/to the storage type.
> -    (@bitfield_core $(#[$attr:meta])* $vis:vis $name:ident $storage:ty) => {
> -        $(#[$attr])*
> -        #[repr(transparent)]
> -        #[derive(Clone, Copy, PartialEq, Eq)]
> -        $vis struct $name {
> -            inner: $storage,
> -        }
> -
> -        #[allow(dead_code)]
> -        impl $name {
> -            /// Creates a bitfield from a raw value.
> -            #[inline(always)]
> -            $vis const fn from_raw(value: $storage) -> Self {
> -                Self{ inner: value }
> -            }
> -
> -            /// Turns this bitfield into its raw value.
> -            ///
> -            /// This is similar to the [`From`] implementation, but is shorter to invoke in
> -            /// most cases.
> -            #[inline(always)]
> -            $vis const fn into_raw(self) -> $storage {
> -                self.inner
> -            }
> -        }
> -
> -        // SAFETY: `$storage` is `Zeroable` and `$name` is transparent.
> -        unsafe impl ::pin_init::Zeroable for $name {}
> -
> -        impl ::core::convert::From<$name> for $storage {
> -            #[inline(always)]
> -            fn from(val: $name) -> $storage {
> -                val.into_raw()
> -            }
> -        }
> -
> -        impl ::core::convert::From<$storage> for $name {
> -            #[inline(always)]
> -            fn from(val: $storage) -> $name {
> -                Self::from_raw(val)
> -            }
> -        }
> -    };
> -
> -    // Definitions requiring knowledge of individual fields: private and public field accessors,
> -    // and `Debug` implementation.
> -    (@bitfield_fields $vis:vis $name:ident $storage:ty {
> -        $($(#[doc = $doc:expr])* $hi:literal:$lo:literal $field:ident
> -            $(?=> $try_into_type:ty)?
> -            $(=> $into_type:ty)?
> -        ;
> -        )*
> -    }
> -    ) => {
> -        #[allow(dead_code)]
> -        impl $name {
> -        $(
> -        $crate::register!(@private_field_accessors $vis $name $storage : $hi:$lo $field);
> -        $crate::register!(
> -            @public_field_accessors $(#[doc = $doc])* $vis $name $storage : $hi:$lo $field
> -            $(?=> $try_into_type)?
> -            $(=> $into_type)?
> -        );
> -        )*
> -        }
> -
> -        $crate::register!(@debug $name { $($field;)* });
> -    };
> -
> -    // Private field accessors working with the exact `Bounded` type for the field.
> -    (
> -        @private_field_accessors $vis:vis $name:ident $storage:ty : $hi:tt:$lo:tt $field:ident
> -    ) => {
> -        ::kernel::macros::paste!(
> -        $vis const [<$field:upper _RANGE>]: ::core::ops::RangeInclusive<u8> = $lo..=$hi;
> -        $vis const [<$field:upper _MASK>]: $storage =
> -            ((((1 << $hi) - 1) << 1) + 1) - ((1 << $lo) - 1);
> -        $vis const [<$field:upper _SHIFT>]: u32 = $lo;
> -        );
> -
> -        ::kernel::macros::paste!(
> -        fn [<__ $field>](self) ->
> -            ::kernel::num::Bounded<$storage, { $hi + 1 - $lo }> {
> -            // Left shift to align the field's MSB with the storage MSB.
> -            const ALIGN_TOP: u32 = $storage::BITS - ($hi + 1);
> -            // Right shift to move the top-aligned field to bit 0 of the storage.
> -            const ALIGN_BOTTOM: u32 = ALIGN_TOP + $lo;
> -
> -            // Extract the field using two shifts. `Bounded::shr` produces the correctly-sized
> -            // output type.
> -            let val = ::kernel::num::Bounded::<$storage, { $storage::BITS }>::from(
> -                self.inner << ALIGN_TOP
> -            );
> -            val.shr::<ALIGN_BOTTOM, { $hi + 1 - $lo } >()
> -        }
> -
> -        const fn [<__with_ $field>](
> -            mut self,
> -            value: ::kernel::num::Bounded<$storage, { $hi + 1 - $lo }>,
> -        ) -> Self
> -        {
> -            const MASK: $storage = <$name>::[<$field:upper _MASK>];
> -            const SHIFT: u32 = <$name>::[<$field:upper _SHIFT>];
> -
> -            let value = value.get() << SHIFT;
> -            self.inner = (self.inner & !MASK) | value;
> -
> -            self
> -        }
> -        );
> -    };
> -
> -    // Public accessors for fields infallibly (`=>`) converted to a type.
> -    (
> -        @public_field_accessors $(#[doc = $doc:expr])* $vis:vis $name:ident $storage:ty :
> -            $hi:literal:$lo:literal $field:ident => $into_type:ty
> -    ) => {
> -        ::kernel::macros::paste!(
> -
> -        $(#[doc = $doc])*
> -        #[doc = "Returns the value of this field."]
> -        #[inline(always)]
> -        $vis fn $field(self) -> $into_type
> -        {
> -            self.[<__ $field>]().into()
> -        }
> -
> -        $(#[doc = $doc])*
> -        #[doc = "Sets this field to the given `value`."]
> -        #[inline(always)]
> -        $vis fn [<with_ $field>](self, value: $into_type) -> Self
> -        {
> -            self.[<__with_ $field>](value.into())
> -        }
> -
> -        );
> -    };
> -
> -    // Public accessors for fields fallibly (`?=>`) converted to a type.
> -    (
> -        @public_field_accessors $(#[doc = $doc:expr])* $vis:vis $name:ident $storage:ty :
> -            $hi:tt:$lo:tt $field:ident ?=> $try_into_type:ty
> -    ) => {
> -        ::kernel::macros::paste!(
> -
> -        $(#[doc = $doc])*
> -        #[doc = "Returns the value of this field."]
> -        #[inline(always)]
> -        $vis fn $field(self) ->
> -            Result<
> -                $try_into_type,
> -                <$try_into_type as ::core::convert::TryFrom<
> -                    ::kernel::num::Bounded<$storage, { $hi + 1 - $lo }>
> -                >>::Error
> -            >
> -        {
> -            self.[<__ $field>]().try_into()
> -        }
> -
> -        $(#[doc = $doc])*
> -        #[doc = "Sets this field to the given `value`."]
> -        #[inline(always)]
> -        $vis fn [<with_ $field>](self, value: $try_into_type) -> Self
> -        {
> -            self.[<__with_ $field>](value.into())
> -        }
> -
> -        );
> -    };
> -
> -    // Public accessors for fields not converted to a type.
> -    (
> -        @public_field_accessors $(#[doc = $doc:expr])* $vis:vis $name:ident $storage:ty :
> -            $hi:tt:$lo:tt $field:ident
> -    ) => {
> -        ::kernel::macros::paste!(
> -
> -        $(#[doc = $doc])*
> -        #[doc = "Returns the value of this field."]
> -        #[inline(always)]
> -        $vis fn $field(self) ->
> -            ::kernel::num::Bounded<$storage, { $hi + 1 - $lo }>
> -        {
> -            self.[<__ $field>]()
> -        }
> -
> -        $(#[doc = $doc])*
> -        #[doc = "Sets this field to the compile-time constant `VALUE`."]
> -        #[inline(always)]
> -        $vis const fn [<with_const_ $field>]<const VALUE: $storage>(self) -> Self {
> -            self.[<__with_ $field>](
> -                ::kernel::num::Bounded::<$storage, { $hi + 1 - $lo }>::new::<VALUE>()
> -            )
> -        }
> -
> -        $(#[doc = $doc])*
> -        #[doc = "Sets this field to the given `value`."]
> -        #[inline(always)]
> -        $vis fn [<with_ $field>]<T>(
> -            self,
> -            value: T,
> -        ) -> Self
> -            where T: Into<::kernel::num::Bounded<$storage, { $hi + 1 - $lo }>>,
> -        {
> -            self.[<__with_ $field>](value.into())
> -        }
> -
> -        $(#[doc = $doc])*
> -        #[doc = "Tries to set this field to `value`, returning an error if it is out of range."]
> -        #[inline(always)]
> -        $vis fn [<try_with_ $field>]<T>(
> -            self,
> -            value: T,
> -        ) -> ::kernel::error::Result<Self>
> -            where T: ::kernel::num::TryIntoBounded<$storage, { $hi + 1 - $lo }>,
> -        {
> -            Ok(
> -                self.[<__with_ $field>](
> -                    value.try_into_bounded().ok_or(::kernel::error::code::EOVERFLOW)?
> -                )
> -            )
> -        }
> -
> -        );
> -    };
> -
> -    // `Debug` implementation.
> -    (@debug $name:ident { $($field:ident;)* }) => {
> -        impl ::kernel::fmt::Debug for $name {
> -            fn fmt(&self, f: &mut ::kernel::fmt::Formatter<'_>) -> ::kernel::fmt::Result {
> -                f.debug_struct(stringify!($name))
> -                    .field("<raw>", &::kernel::prelude::fmt!("{:#x}", self.inner))
> -                $(
> -                    .field(stringify!($field), &self.$field())
> -                )*
> -                    .finish()
> -            }
> -        }
> -    };
>  }
> 
> -- 
> 2.54.0

Re: [PATCH v3 3/5] rust: io: use the `bitfield!` macro in `register!`

[Yury Norov]

On Fri, May 01, 2026 at 01:47:47PM -0400, Yury Norov wrote:
> On Fri, May 01, 2026 at 03:03:20PM +0900, Alexandre Courbot wrote:
> > Replace the local bitfield rules by the equivalent invocation of the
> > `bitfield!` macro.
> > 
> > No functional change should be introduced as the `bitfield!` macro has
> > been extracted from the rules of `register!`.
> > 
> > Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
> 
> Acked-by: Yury Norov <yury.norov@gmail.com>
> 
> If it comes to another round, maybe split switching to a bitfields and
> getting rid of the bitfield_core? For maintainability reasons.

Moreover, you do the same for the other piece of bitfields in #4 and 5.

Re: [PATCH v3 3/5] rust: io: use the `bitfield!` macro in `register!`

[Danilo Krummrich]

On Fri May 1, 2026 at 7:47 PM CEST, Yury Norov wrote:
> If it comes to another round, maybe split switching to a bitfields and
> getting rid of the bitfield_core? For maintainability reasons.

I'm happy to take it as a single patch -- the deleted rules are dead code the
moment the switch happens, and the code is local to the same macro, so the
intermediate state doesn't seem to add any value, or structure in terms of
touching different components etc.

Did you have anything specific in mind when you mention maintainability reasons?

Re: [PATCH v3 3/5] rust: io: use the `bitfield!` macro in `register!`

[Yury Norov]

On Fri, May 01, 2026 at 08:19:31PM +0200, Danilo Krummrich wrote:
> On Fri May 1, 2026 at 7:47 PM CEST, Yury Norov wrote:
> > If it comes to another round, maybe split switching to a bitfields and
> > getting rid of the bitfield_core? For maintainability reasons.
> 
> I'm happy to take it as a single patch -- the deleted rules are dead code the
> moment the switch happens, and the code is local to the same macro, so the
> intermediate state doesn't seem to add any value, or structure in terms of
> touching different components etc.
> 
> Did you have anything specific in mind when you mention maintainability reasons?

I only mean that if there will be weird regression found in the new
implementation, it's better to be able to revert just a single patch
to restore the original code with no side effects.

Just a suggestion. If you're happy with this, I'm happy as well.

Re: [PATCH v3 3/5] rust: io: use the `bitfield!` macro in `register!`

[Danilo Krummrich]

On Fri May 1, 2026 at 10:41 PM CEST, Yury Norov wrote:
> On Fri, May 01, 2026 at 08:19:31PM +0200, Danilo Krummrich wrote:
>> On Fri May 1, 2026 at 7:47 PM CEST, Yury Norov wrote:
>> > If it comes to another round, maybe split switching to a bitfields and
>> > getting rid of the bitfield_core? For maintainability reasons.
>> 
>> I'm happy to take it as a single patch -- the deleted rules are dead code the
>> moment the switch happens, and the code is local to the same macro, so the
>> intermediate state doesn't seem to add any value, or structure in terms of
>> touching different components etc.
>> 
>> Did you have anything specific in mind when you mention maintainability reasons?
>
> I only mean that if there will be weird regression found in the new
> implementation, it's better to be able to revert just a single patch
> to restore the original code with no side effects.

Indeed, but by splitting it up that wouldn't be possible anymore, no?

Am I missing something?

Re: [PATCH v3 3/5] rust: io: use the `bitfield!` macro in `register!`

[Yury Norov]

On Fri, May 01, 2026 at 11:55:03PM +0200, Danilo Krummrich wrote:
> On Fri May 1, 2026 at 10:41 PM CEST, Yury Norov wrote:
> > On Fri, May 01, 2026 at 08:19:31PM +0200, Danilo Krummrich wrote:
> >> On Fri May 1, 2026 at 7:47 PM CEST, Yury Norov wrote:
> >> > If it comes to another round, maybe split switching to a bitfields and
> >> > getting rid of the bitfield_core? For maintainability reasons.
> >> 
> >> I'm happy to take it as a single patch -- the deleted rules are dead code the
> >> moment the switch happens, and the code is local to the same macro, so the
> >> intermediate state doesn't seem to add any value, or structure in terms of
> >> touching different components etc.
> >> 
> >> Did you have anything specific in mind when you mention maintainability reasons?
> >
> > I only mean that if there will be weird regression found in the new
> > implementation, it's better to be able to revert just a single patch
> > to restore the original code with no side effects.
> 
> Indeed, but by splitting it up that wouldn't be possible anymore, no?
> 
> Am I missing something?

The thing is that you most likely will not be able to revert the patch
cleanly - because the existing code evolves. The non-existing code
doesn't evolve. So unless it'is moved to another file, or something
like that, the revert is simpler.