rust: add `bitfield!` and `register!` macros

[PATCH 3/6] rust: add `bitfield!` macro

Posted by Alexandre Courbot 2 weeks, 6 days ago

Add a macro for defining bitfield structs 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.

Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
 rust/kernel/bitfield.rs | 503 ++++++++++++++++++++++++++++++++++++++++++++++++
 rust/kernel/lib.rs      |   1 +
 2 files changed, 504 insertions(+)

diff --git a/rust/kernel/bitfield.rs b/rust/kernel/bitfield.rs
new file mode 100644
index 000000000000..2926ab802227
--- /dev/null
+++ b/rust/kernel/bitfield.rs
@@ -0,0 +1,503 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Support for defining bitfields as Rust structures.
+
+/// Defines a bitfield struct with bounds-checked accessors for individual bit ranges.
+///
+/// # Example
+///
+/// ```rust
+/// use kernel::bitfield;
+/// use kernel::num::Bounded;
+///
+/// bitfield! {
+///     pub struct Rgb(u16) {
+///         15:11 blue;
+///         10:5 green;
+///         4:0 red;
+///     }
+/// }
+///
+/// // Setters can be chained. Bounded::new::<N>() does compile-time bounds checking.
+/// let color = Rgb::default()
+///     .set_red(Bounded::<u16, _>::new::<0x10>())
+///     .set_green(Bounded::<u16, _>::new::<0x1f>())
+///     .set_blue(Bounded::<u16, _>::new::<0x18>());
+///
+/// assert_eq!(color.red(), 0x10);
+/// assert_eq!(color.green(), 0x1f);
+/// assert_eq!(color.blue(), 0x18);
+/// assert_eq!(
+///     color.as_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]
+///     pub struct Name(storage_type), "Struct documentation." {
+///         hi:lo field_1, "Field documentation.";
+///         hi:lo field_2 => ConvertedType, "Field documentation.";
+///         hi:lo field_3 ?=> ConvertedType, "Field documentation.";
+///         ...
+///     }
+/// }
+/// ```
+///
+/// - `storage_type`: The underlying integer type (`u8`, `u16`, `u32`, `u64`).
+/// - `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, and [`Default`] and [`Debug`] implementations.
+///
+/// For each field, it also generates:
+/// - `field()` - getter returning a [`Bounded`] (or converted type) for the field,
+/// - `set_field(value)` - setter with compile-time bounds checking,
+/// - `try_set_field(value)` - setter with runtime bounds checking (for fields without type
+///   conversion),
+/// - `FIELD_MASK`, `FIELD_SHIFT`, `FIELD_RANGE` - constants for manual bit manipulation.
+///
+/// # 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::default()
+///     .set_byte_field(0x42_u8)
+///     .set_flag(true);
+///
+/// assert_eq!(flags.as_raw(), (0x42 << Flags::BYTE_FIELD_SHIFT) | 1);
+/// ```
+///
+/// # Runtime bounds checking
+///
+/// When a value is not known at compile time, use `try_set_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_set_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 when all bit patterns map to valid values:
+///
+/// ```rust
+/// use kernel::bitfield;
+/// use kernel::num::Bounded;
+///
+/// #[derive(Debug, Clone, Copy, Default, PartialEq)]
+/// enum Power {
+///     #[default]
+///     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::default().set_power(Power::On);
+/// assert_eq!(ctrl.power(), Power::On);
+/// ```
+///
+/// ## Fallible conversion (`?=>`)
+///
+/// Use when some bit patterns are invalid. The getter returns a [`Result`]:
+///
+/// ```rust
+/// use kernel::bitfield;
+/// use kernel::num::Bounded;
+///
+/// #[derive(Debug, Clone, Copy, Default, PartialEq)]
+/// enum Mode {
+///     #[default]
+///     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::default().set_mode(Mode::Auto);
+/// assert_eq!(cfg.mode(), Ok(Mode::Auto));
+///
+/// // Invalid bit pattern returns an error.
+/// assert_eq!(Config::from(0b11).mode(), Err(3));
+/// ```
+///
+/// [`Bounded`]: kernel::num::Bounded
+#[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)
+            $(, $comment:literal)? { $($fields:tt)* }
+    ) => {
+        ::kernel::bitfield!(@core $(#[$attr])* $vis $name $storage $(, $comment)?);
+        ::kernel::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 $(, $comment:literal)?) => {
+        $(
+        #[doc=$comment]
+        )?
+        $(#[$attr])*
+        #[repr(transparent)]
+        #[derive(Clone, Copy, PartialEq, Eq)]
+        $vis struct $name($storage);
+
+        #[allow(dead_code)]
+        impl $name {
+            /// Returns the raw value of this bitfield.
+            ///
+            /// This is similar to the [`From`] implementation, but is shorter to invoke in
+            /// most cases.
+            $vis fn as_raw(self) -> $storage {
+                self.0
+            }
+        }
+
+        impl ::core::convert::From<$name> for $storage {
+            fn from(val: $name) -> $storage {
+                val.0
+            }
+        }
+
+        impl ::core::convert::From<$storage> for $name {
+            fn from(val: $storage) -> $name {
+                Self(val)
+            }
+        }
+    };
+
+    // Definitions requiring knowledge of individual fields: private and public field accessors,
+    // and `Debug` and `Default` implementations.
+    (@fields $vis:vis $name:ident $storage:ty {
+        $($hi:tt:$lo:tt $field:ident
+            $(?=> $try_into_type:ty)?
+            $(=> $into_type:ty)?
+            $(, $comment:literal)?
+        ;
+        )*
+    }
+    ) => {
+        #[allow(dead_code)]
+        impl $name {
+        $(
+        ::kernel::bitfield!(@private_field_accessors $vis $name $storage : $hi:$lo $field);
+        ::kernel::bitfield!(@public_field_accessors $vis $name $storage : $hi:$lo $field
+            $(?=> $try_into_type)?
+            $(=> $into_type)?
+            $(, $comment)?
+        );
+        )*
+        }
+
+        ::kernel::bitfield!(@debug $name { $($field;)* });
+        ::kernel::bitfield!(@default $name { $($field;)* });
+    };
+
+    // Private field accessors working with the correct `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.0 << ALIGN_TOP
+            );
+            val.shr::<ALIGN_BOTTOM, _>()
+        }
+
+        fn [<__set_ $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.0 = (self.0 & !MASK) | value;
+
+            self
+        }
+        );
+    };
+
+    // Public accessors for fields infallibly (`=>`) converted to a type.
+    (
+        @public_field_accessors $vis:vis $name:ident $storage:ty : $hi:tt:$lo:tt $field:ident
+            => $into_type:ty $(, $comment:literal)?
+    ) => {
+        ::kernel::macros::paste!(
+
+        $(
+        #[doc="Returns the value of this field:"]
+        #[doc=$comment]
+        )?
+        #[inline(always)]
+        $vis fn $field(self) -> $into_type
+        {
+            self.[<__ $field>]().into()
+        }
+
+        $(
+        #[doc="Sets the value of this field:"]
+        #[doc=$comment]
+        )?
+        #[inline(always)]
+        $vis fn [<set_ $field>](self, value: $into_type) -> Self
+        {
+            self.[<__set_ $field>](value.into())
+        }
+
+        /// Private method, for use in the [`Default`] implementation.
+        fn [<$field _default>]() -> $into_type {
+            Default::default()
+        }
+
+        );
+    };
+
+    // Public accessors for fields fallibly (`?=>`) converted to a type.
+    (
+        @public_field_accessors $vis:vis $name:ident $storage:ty : $hi:tt:$lo:tt $field:ident
+            ?=> $try_into_type:ty $(, $comment:literal)?
+    ) => {
+        ::kernel::macros::paste!(
+
+        $(
+        #[doc="Returns the value of this field:"]
+        #[doc=$comment]
+        )?
+        #[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="Sets the value of this field:"]
+        #[doc=$comment]
+        )?
+        #[inline(always)]
+        $vis fn [<set_ $field>](self, value: $try_into_type) -> Self
+        {
+            self.[<__set_ $field>](value.into())
+        }
+
+        /// Private method, for use in the [`Default`] implementation.
+        fn [<$field _default>]() -> $try_into_type {
+            Default::default()
+        }
+
+        );
+    };
+
+    // Public accessors for fields not converted to a type.
+    (
+        @public_field_accessors $vis:vis $name:ident $storage:ty : $hi:tt:$lo:tt $field:ident
+            $(, $comment:literal)?
+    ) => {
+        ::kernel::macros::paste!(
+
+        $(
+        #[doc="Returns the value of this field:"]
+        #[doc=$comment]
+        )?
+        #[inline(always)]
+        $vis fn $field(self) ->
+            ::kernel::num::Bounded<$storage, { $hi + 1 - $lo }>
+        {
+            self.[<__ $field>]()
+        }
+
+        $(
+        #[doc="Sets the value of this field:"]
+        #[doc=$comment]
+        )?
+        #[inline(always)]
+        $vis fn [<set_ $field>]<T>(
+            self,
+            value: T,
+        ) -> Self
+            where T: Into<::kernel::num::Bounded<$storage, { $hi + 1 - $lo }>>,
+        {
+            self.[<__set_ $field>](value.into())
+        }
+
+        $(
+        #[doc="Attempts to set the value of this field:"]
+        #[doc=$comment]
+        )?
+        #[inline(always)]
+        $vis fn [<try_set_ $field>]<T>(
+            self,
+            value: T,
+        ) -> ::kernel::error::Result<Self>
+            where T: ::kernel::num::TryIntoBounded<$storage, { $hi + 1 - $lo }>,
+        {
+            Ok(
+                self.[<__set_ $field>](
+                    value.try_into_bounded().ok_or(::kernel::error::code::EOVERFLOW)?
+                )
+            )
+        }
+
+        /// Private method, for use in the [`Default`] implementation.
+        fn [<$field _default>]() -> ::kernel::num::Bounded<$storage, { $hi + 1 - $lo }> {
+            Default::default()
+        }
+
+        );
+    };
+
+    // `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.0))
+                $(
+                    .field(stringify!($field), &self.$field())
+                )*
+                    .finish()
+            }
+        }
+    };
+
+    // `Default` implementation.
+    (@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 {
+                #[allow(unused_mut)]
+                let mut value = Self(Default::default());
+
+                ::kernel::macros::paste!(
+                $(
+                value = value.[<set_ $field>](Self::[<$field _default>]());
+                )*
+                );
+
+                value
+            }
+        }
+    };
+}
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index f812cf120042..66198e69d1f5 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -68,6 +68,7 @@
 pub mod alloc;
 #[cfg(CONFIG_AUXILIARY_BUS)]
 pub mod auxiliary;
+pub mod bitfield;
 pub mod bitmap;
 pub mod bits;
 #[cfg(CONFIG_BLOCK)]

-- 
2.52.0

Re: [PATCH 3/6] rust: add `bitfield!` macro

Posted by Yury Norov 2 weeks, 4 days ago

On Tue, Jan 20, 2026 at 03:17:56PM +0900, Alexandre Courbot wrote:
> Add a macro for defining bitfield structs 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.
> 
> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
> ---
>  rust/kernel/bitfield.rs | 503 ++++++++++++++++++++++++++++++++++++++++++++++++
>  rust/kernel/lib.rs      |   1 +
>  2 files changed, 504 insertions(+)
> 
> diff --git a/rust/kernel/bitfield.rs b/rust/kernel/bitfield.rs
> new file mode 100644
> index 000000000000..2926ab802227
> --- /dev/null
> +++ b/rust/kernel/bitfield.rs
> @@ -0,0 +1,503 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! Support for defining bitfields as Rust structures.
> +
> +/// Defines a bitfield struct with bounds-checked accessors for individual bit ranges.
> +///
> +/// # Example
> +///
> +/// ```rust
> +/// use kernel::bitfield;
> +/// use kernel::num::Bounded;
> +///
> +/// bitfield! {
> +///     pub struct Rgb(u16) {
> +///         15:11 blue;
> +///         10:5 green;
> +///         4:0 red;
> +///     }
> +/// }
> +///
> +/// // Setters can be chained. Bounded::new::<N>() does compile-time bounds checking.
> +/// let color = Rgb::default()
> +///     .set_red(Bounded::<u16, _>::new::<0x10>())
> +///     .set_green(Bounded::<u16, _>::new::<0x1f>())
> +///     .set_blue(Bounded::<u16, _>::new::<0x18>());

Is there a way to just say:

    let color = Rgb::default().
            .set_red(0x10)
            .set_green(0x1f)
            .set_blue(0x18)

I think it should be the default style. Later in the patch you say: 

    Each field is internally represented as a [`Bounded`]

So, let's keep implementation decoupled from an interface?

> +///
> +/// assert_eq!(color.red(), 0x10);
> +/// assert_eq!(color.green(), 0x1f);
> +/// assert_eq!(color.blue(), 0x18);
> +/// assert_eq!(
> +///     color.as_raw(),
> +///     (0x18 << Rgb::BLUE_SHIFT) + (0x1f << Rgb::GREEN_SHIFT) + 0x10,
> +/// );

What about: 

        bitfield! {
            pub struct Rgb(u16) {
                15:11 blue;
                10:5  Blue;
                4:0   BLUE;
            }
        }

What Rgb::BLUE_SHIFT would mean in this case? Maybe Rgb::SHIFT(blue)?

> +///
> +/// // Convert to/from the backing storage type.
> +/// let raw: u16 = color.into();

What about: 

        bitfield! {
            pub struct Rgb(u16) {
                15:11 blue;
                10:5  set_blue;
                4:0   into;
            }
        }

What color.set_blue() and color.into() would mean? Even if they work,
I think, to stay on safe side there should be a more conventional set
of accessors: color.get(into), color.set(set_blue, 0xff) and son on.

Thanks,
Yury

Re: [PATCH 3/6] rust: add `bitfield!` macro

Posted by Alexandre Courbot 1 week, 6 days ago

On Wed Jan 21, 2026 at 6:16 PM JST, Yury Norov wrote:
> On Tue, Jan 20, 2026 at 03:17:56PM +0900, Alexandre Courbot wrote:
>> Add a macro for defining bitfield structs 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.
>> 
>> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
>> ---
>>  rust/kernel/bitfield.rs | 503 ++++++++++++++++++++++++++++++++++++++++++++++++
>>  rust/kernel/lib.rs      |   1 +
>>  2 files changed, 504 insertions(+)
>> 
>> diff --git a/rust/kernel/bitfield.rs b/rust/kernel/bitfield.rs
>> new file mode 100644
>> index 000000000000..2926ab802227
>> --- /dev/null
>> +++ b/rust/kernel/bitfield.rs
>> @@ -0,0 +1,503 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +
>> +//! Support for defining bitfields as Rust structures.
>> +
>> +/// Defines a bitfield struct with bounds-checked accessors for individual bit ranges.
>> +///
>> +/// # Example
>> +///
>> +/// ```rust
>> +/// use kernel::bitfield;
>> +/// use kernel::num::Bounded;
>> +///
>> +/// bitfield! {
>> +///     pub struct Rgb(u16) {
>> +///         15:11 blue;
>> +///         10:5 green;
>> +///         4:0 red;
>> +///     }
>> +/// }
>> +///
>> +/// // Setters can be chained. Bounded::new::<N>() does compile-time bounds checking.
>> +/// let color = Rgb::default()
>> +///     .set_red(Bounded::<u16, _>::new::<0x10>())
>> +///     .set_green(Bounded::<u16, _>::new::<0x1f>())
>> +///     .set_blue(Bounded::<u16, _>::new::<0x18>());
>
> Is there a way to just say:
>
>     let color = Rgb::default().
>             .set_red(0x10)
>             .set_green(0x1f)
>             .set_blue(0x18)
>
> I think it should be the default style. Later in the patch you say: 
>
>     Each field is internally represented as a [`Bounded`]
>
> So, let's keep implementation decoupled from an interface?

That is unfortunately not feasible, but the syntax above should seldomly
be used outside of examples.

>
>> +///
>> +/// assert_eq!(color.red(), 0x10);
>> +/// assert_eq!(color.green(), 0x1f);
>> +/// assert_eq!(color.blue(), 0x18);
>> +/// assert_eq!(
>> +///     color.as_raw(),
>> +///     (0x18 << Rgb::BLUE_SHIFT) + (0x1f << Rgb::GREEN_SHIFT) + 0x10,
>> +/// );
>
> What about: 
>
>         bitfield! {
>             pub struct Rgb(u16) {
>                 15:11 blue;
>                 10:5  Blue;
>                 4:0   BLUE;
>             }
>         }
>

Oh, all of these will name-clash very badly. :) At the end of the day,
it is still a macro.

> What Rgb::BLUE_SHIFT would mean in this case? Maybe Rgb::SHIFT(blue)?

You wouldn't even have the luxury to yse `BLUE_SHIFT` here because where
would be conflicting definitions and thus a build error.

>
>> +///
>> +/// // Convert to/from the backing storage type.
>> +/// let raw: u16 = color.into();
>
> What about: 
>
>         bitfield! {
>             pub struct Rgb(u16) {
>                 15:11 blue;
>                 10:5  set_blue;
>                 4:0   into;
>             }
>         }
>
> What color.set_blue() and color.into() would mean? Even if they work,
> I think, to stay on safe side there should be a more conventional set
> of accessors: color.get(into), color.set(set_blue, 0xff) and son on.

This would just not build.

Re: [PATCH 3/6] rust: add `bitfield!` macro

Posted by Yury Norov 1 week, 6 days ago

On Mon, Jan 26, 2026 at 10:35:49PM +0900, Alexandre Courbot wrote:
> On Wed Jan 21, 2026 at 6:16 PM JST, Yury Norov wrote:
> > On Tue, Jan 20, 2026 at 03:17:56PM +0900, Alexandre Courbot wrote:
> >> Add a macro for defining bitfield structs 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.
> >> 
> >> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
> >> ---
> >>  rust/kernel/bitfield.rs | 503 ++++++++++++++++++++++++++++++++++++++++++++++++
> >>  rust/kernel/lib.rs      |   1 +
> >>  2 files changed, 504 insertions(+)
> >> 
> >> diff --git a/rust/kernel/bitfield.rs b/rust/kernel/bitfield.rs
> >> new file mode 100644
> >> index 000000000000..2926ab802227
> >> --- /dev/null
> >> +++ b/rust/kernel/bitfield.rs
> >> @@ -0,0 +1,503 @@
> >> +// SPDX-License-Identifier: GPL-2.0
> >> +
> >> +//! Support for defining bitfields as Rust structures.
> >> +
> >> +/// Defines a bitfield struct with bounds-checked accessors for individual bit ranges.
> >> +///
> >> +/// # Example
> >> +///
> >> +/// ```rust
> >> +/// use kernel::bitfield;
> >> +/// use kernel::num::Bounded;
> >> +///
> >> +/// bitfield! {
> >> +///     pub struct Rgb(u16) {
> >> +///         15:11 blue;
> >> +///         10:5 green;
> >> +///         4:0 red;
> >> +///     }
> >> +/// }
> >> +///
> >> +/// // Setters can be chained. Bounded::new::<N>() does compile-time bounds checking.
> >> +/// let color = Rgb::default()
> >> +///     .set_red(Bounded::<u16, _>::new::<0x10>())
> >> +///     .set_green(Bounded::<u16, _>::new::<0x1f>())
> >> +///     .set_blue(Bounded::<u16, _>::new::<0x18>());
> >
> > Is there a way to just say:
> >
> >     let color = Rgb::default().
> >             .set_red(0x10)
> >             .set_green(0x1f)
> >             .set_blue(0x18)
> >
> > I think it should be the default style. Later in the patch you say: 
> >
> >     Each field is internally represented as a [`Bounded`]
> >
> > So, let's keep implementation decoupled from an interface?
> 
> That is unfortunately not feasible, but the syntax above should seldomly
> be used outside of examples.

The above short syntax is definitely more desired over that wordy and
non-trivial version that exposes implementation internals.

A regular user doesn't care of the exact mechanism that protects the
bitfields. He wants to just assign numbers to the fields, and let
your machinery to take care of the integrity. 

Can you please explain in details why that's not feasible, please
do it in commit message. If it's an implementation constraint,
please consider to re-implement. 

> >> +///
> >> +/// assert_eq!(color.red(), 0x10);
> >> +/// assert_eq!(color.green(), 0x1f);
> >> +/// assert_eq!(color.blue(), 0x18);
> >> +/// assert_eq!(
> >> +///     color.as_raw(),
> >> +///     (0x18 << Rgb::BLUE_SHIFT) + (0x1f << Rgb::GREEN_SHIFT) + 0x10,
> >> +/// );
> >
> > What about: 
> >
> >         bitfield! {
> >             pub struct Rgb(u16) {
> >                 15:11 blue;
> >                 10:5  Blue;
> >                 4:0   BLUE;
> >             }
> >         }
> >
> 
> Oh, all of these will name-clash very badly. :) At the end of the day,
> it is still a macro.
> 
> > What Rgb::BLUE_SHIFT would mean in this case? Maybe Rgb::SHIFT(blue)?
> 
> You wouldn't even have the luxury to yse `BLUE_SHIFT` here because where
> would be conflicting definitions and thus a build error.
> 
> >
> >> +///
> >> +/// // Convert to/from the backing storage type.
> >> +/// let raw: u16 = color.into();
> >
> > What about: 
> >
> >         bitfield! {
> >             pub struct Rgb(u16) {
> >                 15:11 blue;
> >                 10:5  set_blue;
> >                 4:0   into;
> >             }
> >         }
> >
> > What color.set_blue() and color.into() would mean? Even if they work,
> > I think, to stay on safe side there should be a more conventional set
> > of accessors: color.get(into), color.set(set_blue, 0xff) and son on.
> 
> This would just not build.

I know it wouldn't. I am just trying to understand corner cases that may
(and would!) frustrate people for decades. 

I understand that this implementation works just great for the registers,
but my role is to make sure that it would work equally well for everyone. 
Now, for example, Rust, contrary to C in Linux, actively uses camel case. 
So, the blue vs Blue vs BLUE restriction is a very valid concern. The
same for reserved words like 'into'. As long as the API matures, the
number of such special words would only grow. The .shr() and .shl() that
you add in this series are the quite good examples.

Let's make a step back and just list all limitations that come with this
implementation.

Again, this all is relevant for a basic generic data structure. If we
consider it a supporting layer for the registers, everything is totally
fine. In that case, we should just give it a more specific name, and
probably place in an different directory, closer to IO APIs.  

Thanks,
Yury

Re: [PATCH 3/6] rust: add `bitfield!` macro

Posted by Alexandre Courbot 1 week, 5 days ago

On Tue Jan 27, 2026 at 11:55 AM JST, Yury Norov wrote:
<snip>
>> >> +/// // Setters can be chained. Bounded::new::<N>() does compile-time bounds checking.
>> >> +/// let color = Rgb::default()
>> >> +///     .set_red(Bounded::<u16, _>::new::<0x10>())
>> >> +///     .set_green(Bounded::<u16, _>::new::<0x1f>())
>> >> +///     .set_blue(Bounded::<u16, _>::new::<0x18>());
>> >
>> > Is there a way to just say:
>> >
>> >     let color = Rgb::default().
>> >             .set_red(0x10)
>> >             .set_green(0x1f)
>> >             .set_blue(0x18)
>> >
>> > I think it should be the default style. Later in the patch you say: 
>> >
>> >     Each field is internally represented as a [`Bounded`]
>> >
>> > So, let's keep implementation decoupled from an interface?
>> 
>> That is unfortunately not feasible, but the syntax above should seldomly
>> be used outside of examples.
>
> The above short syntax is definitely more desired over that wordy and
> non-trivial version that exposes implementation internals.
>
> A regular user doesn't care of the exact mechanism that protects the
> bitfields. He wants to just assign numbers to the fields, and let
> your machinery to take care of the integrity. 

So while we cannot achieve exactly the short syntax above (which has its
drawbacks as well, such as the inability to operate in const context),
we can introduce a new setter than works with a const argument and
spares us the need to invoke `Bounded::new` ourselves:

    let color = Rgb::default().
            .with_red::<0x10>()
            .with_green::<0x1f>()
            .with_blue::<0x18>()

This setter knows which type (and thus which Bounded constructor) to
work with, and is much more concise. It also makes it clear that it
operates in const context, i.e. `color` will directly be
set to the final value. Actually let me check whether we can make the
whole chain `const`.

<snip>
>> >
>> >> +///
>> >> +/// // Convert to/from the backing storage type.
>> >> +/// let raw: u16 = color.into();
>> >
>> > What about: 
>> >
>> >         bitfield! {
>> >             pub struct Rgb(u16) {
>> >                 15:11 blue;
>> >                 10:5  set_blue;
>> >                 4:0   into;
>> >             }
>> >         }
>> >
>> > What color.set_blue() and color.into() would mean? Even if they work,
>> > I think, to stay on safe side there should be a more conventional set
>> > of accessors: color.get(into), color.set(set_blue, 0xff) and son on.
>> 
>> This would just not build.
>
> I know it wouldn't. I am just trying to understand corner cases that may
> (and would!) frustrate people for decades. 
>
> I understand that this implementation works just great for the registers,
> but my role is to make sure that it would work equally well for everyone. 
> Now, for example, Rust, contrary to C in Linux, actively uses camel case. 
> So, the blue vs Blue vs BLUE restriction is a very valid concern. The
> same for reserved words like 'into'. As long as the API matures, the
> number of such special words would only grow. The .shr() and .shl() that
> you add in this series are the quite good examples.

The strong Rust naming conventions are actually (partially) shielding us
from name clashing here.

In Rust, fields are by convention typed in snake_case. So the blue vs
Blue vs BLUE example would only be relevant if one breaks these
conventions.

That being said, we could also avoid the capitalization of the field
constants to avoid name clashing. Or we could have a const method
returning the shift, range and masks of a field as a structure. But we
cannot avoid adding new elements to the namespace, unless we start doing
contorted (and probably quite verbose) things, like having an enum type
describing the fields and a unique setter function using it to know
which field to set.

>
> Let's make a step back and just list all limitations that come with this
> implementation.

We should at the very least mention in the documentation what members
are generated so users know the limits of what they can do, yes.

>
> Again, this all is relevant for a basic generic data structure. If we
> consider it a supporting layer for the registers, everything is totally
> fine. In that case, we should just give it a more specific name, and
> probably place in an different directory, closer to IO APIs.  

v2 and v3 of this patchset actually embed the bitfield rules into the
register macro. This is designed to be temporary, but gives us more time
to iron the details you pointed out before extracting the `bitfield!`
macro and making it widely available.

In any case, I will include the `with_` setters in v4 so we can use that
in the register macro already.

Re: [PATCH 3/6] rust: add `bitfield!` macro

Posted by John Hubbard 1 week, 5 days ago

On 1/27/26 1:57 AM, Alexandre Courbot wrote:
> On Tue Jan 27, 2026 at 11:55 AM JST, Yury Norov wrote:
> <snip>
> 
> So while we cannot achieve exactly the short syntax above (which has its
> drawbacks as well, such as the inability to operate in const context),
> we can introduce a new setter than works with a const argument and
> spares us the need to invoke `Bounded::new` ourselves:
> 
>      let color = Rgb::default().
>              .with_red::<0x10>()
>              .with_green::<0x1f>()
>              .with_blue::<0x18>()

Are we sure that .with_red is a better name than, say, .set_red()?

"with" is not so easy to remember, because it is a bit
surprising and different, for setting a value.

"with" feels like a function call or closure: "sort with
qsort", for example. But here we are setting a color
component.

thanks,
John Hubbard

Re: [PATCH 3/6] rust: add `bitfield!` macro

Posted by Gary Guo 1 week, 5 days ago

On Tue Jan 27, 2026 at 9:03 PM GMT, John Hubbard wrote:
> On 1/27/26 1:57 AM, Alexandre Courbot wrote:
>> On Tue Jan 27, 2026 at 11:55 AM JST, Yury Norov wrote:
>> <snip>
>> 
>> So while we cannot achieve exactly the short syntax above (which has its
>> drawbacks as well, such as the inability to operate in const context),
>> we can introduce a new setter than works with a const argument and
>> spares us the need to invoke `Bounded::new` ourselves:
>> 
>>      let color = Rgb::default().
>>              .with_red::<0x10>()
>>              .with_green::<0x1f>()
>>              .with_blue::<0x18>()
>
> Are we sure that .with_red is a better name than, say, .set_red()?
>
> "with" is not so easy to remember, because it is a bit
> surprising and different, for setting a value.
>
> "with" feels like a function call or closure: "sort with
> qsort", for example. But here we are setting a color
> component.

`set_foo` implies that the value is mutated in place (and takes `&mut self`).
`with_foo` implies that value is returned with the specific thing changed. For
example, `pointer::with_addr`, `Path::with_extension`.

Given the signature in the API I would agree with Yury that `with_` is better.

Best,
Gary


>
> thanks,
> John Hubbard

Re: [PATCH 3/6] rust: add `bitfield!` macro

Posted by Alexandre Courbot 1 week, 5 days ago

On Wed Jan 28, 2026 at 6:10 AM JST, Gary Guo wrote:
> On Tue Jan 27, 2026 at 9:03 PM GMT, John Hubbard wrote:
>> On 1/27/26 1:57 AM, Alexandre Courbot wrote:
>>> On Tue Jan 27, 2026 at 11:55 AM JST, Yury Norov wrote:
>>> <snip>
>>> 
>>> So while we cannot achieve exactly the short syntax above (which has its
>>> drawbacks as well, such as the inability to operate in const context),
>>> we can introduce a new setter than works with a const argument and
>>> spares us the need to invoke `Bounded::new` ourselves:
>>> 
>>>      let color = Rgb::default().
>>>              .with_red::<0x10>()
>>>              .with_green::<0x1f>()
>>>              .with_blue::<0x18>()
>>
>> Are we sure that .with_red is a better name than, say, .set_red()?
>>
>> "with" is not so easy to remember, because it is a bit
>> surprising and different, for setting a value.
>>
>> "with" feels like a function call or closure: "sort with
>> qsort", for example. But here we are setting a color
>> component.
>
> `set_foo` implies that the value is mutated in place (and takes `&mut self`).
> `with_foo` implies that value is returned with the specific thing changed. For
> example, `pointer::with_addr`, `Path::with_extension`.
>
> Given the signature in the API I would agree with Yury that `with_` is better.

Indeed, given the signature of the method `with_` is definitely more
idiomatic. But that is also true for the non-const setter, and they
unfortunately cannot share the same name.

Alternative names I can think of for the non-const / const setters:

- `with_foo`/`with_foo_const`, but that's a bit verbose,
- `with_foo`/`const_foo`, but that's not idiomatic either,
- `with_foo_val`/`with_foo`, but that increases the risk of name
  clashes.

... so unless there are better proposals I guess the
`set_foo`/`with_foo` is at least practical, if a bit unconventional.

Re: [PATCH 3/6] rust: add `bitfield!` macro

Posted by John Hubbard 1 week, 5 days ago

On 1/27/26 5:28 PM, Alexandre Courbot wrote:
> On Wed Jan 28, 2026 at 6:10 AM JST, Gary Guo wrote:
>> On Tue Jan 27, 2026 at 9:03 PM GMT, John Hubbard wrote:
>>> On 1/27/26 1:57 AM, Alexandre Courbot wrote:
>>>> On Tue Jan 27, 2026 at 11:55 AM JST, Yury Norov wrote:
>>>> <snip>
>> Given the signature in the API I would agree with Yury that `with_` is better.
> 
> Indeed, given the signature of the method `with_` is definitely more
> idiomatic. But that is also true for the non-const setter, and they
> unfortunately cannot share the same name.
> 

oh that is unfortunate.

> Alternative names I can think of for the non-const / const setters:
> 
> - `with_foo`/`with_foo_const`, but that's a bit verbose,

Yes, but it is also at least slightly self-explanatory: "the name
is a little ugly, but you can see what it is for".

> - `with_foo`/`const_foo`, but that's not idiomatic either,
> - `with_foo_val`/`with_foo`, but that increases the risk of name
>   clashes.
> 
> ... so unless there are better proposals I guess the
> `set_foo`/`with_foo` is at least practical, if a bit unconventional.

...whereas this choice just looks unhinged, at first glance:
"what? whhhyyy???" haha :)

And eventually the reader figures out why. Maybe.


thanks,
-- 
John Hubbard

Re: [PATCH 3/6] rust: add `bitfield!` macro

Posted by John Hubbard 1 week, 5 days ago

On 1/27/26 1:10 PM, Gary Guo wrote:
> On Tue Jan 27, 2026 at 9:03 PM GMT, John Hubbard wrote:
>> On 1/27/26 1:57 AM, Alexandre Courbot wrote:
>>> On Tue Jan 27, 2026 at 11:55 AM JST, Yury Norov wrote:
>>> <snip>
>>>
>>> So while we cannot achieve exactly the short syntax above (which has its
>>> drawbacks as well, such as the inability to operate in const context),
>>> we can introduce a new setter than works with a const argument and
>>> spares us the need to invoke `Bounded::new` ourselves:
>>>
>>>      let color = Rgb::default().
>>>              .with_red::<0x10>()
>>>              .with_green::<0x1f>()
>>>              .with_blue::<0x18>()
>>
>> Are we sure that .with_red is a better name than, say, .set_red()?
>>
>> "with" is not so easy to remember, because it is a bit
>> surprising and different, for setting a value.
>>
>> "with" feels like a function call or closure: "sort with
>> qsort", for example. But here we are setting a color
>> component.
> 
> `set_foo` implies that the value is mutated in place (and takes `&mut self`).
> `with_foo` implies that value is returned with the specific thing changed. For
> example, `pointer::with_addr`, `Path::with_extension`.
> 
> Given the signature in the API I would agree with Yury that `with_` is better.

OK that sounds consistent anyway.

thanks,
-- 
John Hubbard

Re: [PATCH 3/6] rust: add `bitfield!` macro

Posted by Joel Fernandes 1 week, 6 days ago

On Jan 26, 2026, at 9:55 PM, Yury Norov <ynorov@nvidia.com> wrote:
> On Mon, Jan 26, 2026 at 10:35:49PM +0900, Alexandre Courbot wrote:
> > On Wed Jan 21, 2026 at 6:16 PM JST, Yury Norov wrote:
> > > On Tue, Jan 20, 2026 at 03:17:56PM +0900, Alexandre Courbot wrote:
> > > > Add a macro for defining bitfield structs 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.
> > > >
> > > > Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
> > > > ---
> > > > rust/kernel/bitfield.rs | 503 ++++++++++++++++++++++++++++++++++++++++++++++++
> > > > rust/kernel/lib.rs      |   1 +
> > > > 2 files changed, 504 insertions(+)
[...]
> > > > +/// // Setters can be chained. Bounded::new::<N>() does compile-time bounds checking.
> > > > +/// let color = Rgb::default()
> > > > +///     .set_red(Bounded::<u16, _>::new::<0x10>())
> > > > +///     .set_green(Bounded::<u16, _>::new::<0x1f>())
> > > > +///     .set_blue(Bounded::<u16, _>::new::<0x18>());
> > >
> > > Is there a way to just say:
> > >
> > >    let color = Rgb::default().
> > >            .set_red(0x10)
> > >            .set_green(0x1f)
> > >            .set_blue(0x18)
> > >
> > > I think it should be the default style. Later in the patch you say:
> > >
> > >    Each field is internally represented as a [`Bounded`]
> > >
> > > So, let's keep implementation decoupled from an interface?
> >
> > That is unfortunately not feasible, but the syntax above should seldomly
> > be used outside of examples.
>
> The above short syntax is definitely more desired over that wordy and
> non-trivial version that exposes implementation internals.
>
> A regular user doesn't care of the exact mechanism that protects the
> bitfields. He wants to just assign numbers to the fields, and let
> your machinery to take care of the integrity.
>
> Can you please explain in details why that's not feasible, please
> do it in commit message. If it's an implementation constraint,
> please consider to re-implement.

If the issue is the excessive turbofish syntax, how about a macro? For
example:

    let color = Rgb::default()
        .set_red(bounded!(u16, 0x10))
        .set_green(bounded!(u16, 0x1f))
        .set_blue(bounded!(u16, 0x18));

This hides the turbofish and Bounded internals while still providing
compile-time bounds checking.

[...]
> > > What Rgb::BLUE_SHIFT would mean in this case? Maybe Rgb::SHIFT(blue)?
> >
> > You wouldn't even have the luxury to yse `BLUE_SHIFT` here because where
> > would be conflicting definitions and thus a build error.
[...]
> > > What color.set_blue() and color.into() would mean? Even if they work,
> > > I think, to stay on safe side there should be a more conventional set
> > > of accessors: color.get(into), color.set(set_blue, 0xff) and son on.
> >
> > This would just not build.
>
> I know it wouldn't. I am just trying to understand corner cases that may
> (and would!) frustrate people for decades.
>
> I understand that this implementation works just great for the registers,
> but my role is to make sure that it would work equally well for everyone.
> Now, for example, Rust, contrary to C in Linux, actively uses camel case.
> So, the blue vs Blue vs BLUE restriction is a very valid concern. The
> same for reserved words like 'into'. As long as the API matures, the
> number of such special words would only grow. The .shr() and .shl() that
> you add in this series are the quite good examples.
>
> Let's make a step back and just list all limitations that come with this
> implementation.

Why is a build error not sufficient to alert the user to use better judgement
for naming?

This is no different than using reserved keywords in C. For example, this
won't compile:

    int if = 5;      // error: 'if' is a reserved keyword
    int return = 3;  // error: 'return' is a reserved keyword

The user simply learns not to use reserved words, and the compiler enforces
this clearly. The same applies here.

> Again, this all is relevant for a basic generic data structure. If we
> consider it a supporting layer for the registers, everything is totally
> fine. In that case, we should just give it a more specific name, and
> probably place in an different directory, closer to IO APIs.

The Bitfield macro is very much required for non-register use cases too.

--
Joel Fernandes

Re: [PATCH 3/6] rust: add `bitfield!` macro

Posted by Gary Guo 1 week, 5 days ago

On Tue Jan 27, 2026 at 3:25 AM GMT, Joel Fernandes wrote:
> On Jan 26, 2026, at 9:55 PM, Yury Norov <ynorov@nvidia.com> wrote:
>> On Mon, Jan 26, 2026 at 10:35:49PM +0900, Alexandre Courbot wrote:
>> > On Wed Jan 21, 2026 at 6:16 PM JST, Yury Norov wrote:
>> > > On Tue, Jan 20, 2026 at 03:17:56PM +0900, Alexandre Courbot wrote:
>> > > > Add a macro for defining bitfield structs 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.
>> > > >
>> > > > Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
>> > > > ---
>> > > > rust/kernel/bitfield.rs | 503 ++++++++++++++++++++++++++++++++++++++++++++++++
>> > > > rust/kernel/lib.rs      |   1 +
>> > > > 2 files changed, 504 insertions(+)
> [...]
>> > > > +/// // Setters can be chained. Bounded::new::<N>() does compile-time bounds checking.
>> > > > +/// let color = Rgb::default()
>> > > > +///     .set_red(Bounded::<u16, _>::new::<0x10>())
>> > > > +///     .set_green(Bounded::<u16, _>::new::<0x1f>())
>> > > > +///     .set_blue(Bounded::<u16, _>::new::<0x18>());
>> > >
>> > > Is there a way to just say:
>> > >
>> > >    let color = Rgb::default().
>> > >            .set_red(0x10)
>> > >            .set_green(0x1f)
>> > >            .set_blue(0x18)
>> > >
>> > > I think it should be the default style. Later in the patch you say:
>> > >
>> > >    Each field is internally represented as a [`Bounded`]
>> > >
>> > > So, let's keep implementation decoupled from an interface?
>> >
>> > That is unfortunately not feasible, but the syntax above should seldomly
>> > be used outside of examples.
>>
>> The above short syntax is definitely more desired over that wordy and
>> non-trivial version that exposes implementation internals.
>>
>> A regular user doesn't care of the exact mechanism that protects the
>> bitfields. He wants to just assign numbers to the fields, and let
>> your machinery to take care of the integrity.
>>
>> Can you please explain in details why that's not feasible, please
>> do it in commit message. If it's an implementation constraint,
>> please consider to re-implement.
>
> If the issue is the excessive turbofish syntax, how about a macro? For
> example:
>
>     let color = Rgb::default()
>         .set_red(bounded!(u16, 0x10))
>         .set_green(bounded!(u16, 0x1f))
>         .set_blue(bounded!(u16, 0x18));
>
> This hides the turbofish and Bounded internals while still providing
> compile-time bounds checking.

I think this could be the way forward, if we also get type inference working
properly.

    Rgb::default()
        .set_read(bounded!(0x10))
        .set_green(bounded!(0x1f))
        .set_blue(bounded!(0x18))

is roughly the limit that I find acceptable (`Bounded::<u16, _>::new::<0x10>()`
is something way too verbose so I find it unacceptable).

I still think if we can get 

    Rgb::default()
        .set_read(0x10)
        .set_green(0x1f)
        .set_blue(0x18)

to work with implicit `build_assert!` check it'll be ideal, although I
understand the concern about the fragility of `build_assert!()`, especially when
Clippy is used.

I am planning to at least improve the diagnostics when `build_assert!` is used
incorrectly and the build error actually occurs, so hopefully in the long run it
can once again become a tool that we can rely on, but in the meantime,
if all it needed is an extra `bounded!()` call, it doesn't bother me that much
versus the full turbofish.

Best,
Gary

>
> [...]
>> > > What Rgb::BLUE_SHIFT would mean in this case? Maybe Rgb::SHIFT(blue)?
>> >
>> > You wouldn't even have the luxury to yse `BLUE_SHIFT` here because where
>> > would be conflicting definitions and thus a build error.
> [...]
>> > > What color.set_blue() and color.into() would mean? Even if they work,
>> > > I think, to stay on safe side there should be a more conventional set
>> > > of accessors: color.get(into), color.set(set_blue, 0xff) and son on.
>> >
>> > This would just not build.
>>
>> I know it wouldn't. I am just trying to understand corner cases that may
>> (and would!) frustrate people for decades.
>>
>> I understand that this implementation works just great for the registers,
>> but my role is to make sure that it would work equally well for everyone.
>> Now, for example, Rust, contrary to C in Linux, actively uses camel case.
>> So, the blue vs Blue vs BLUE restriction is a very valid concern. The
>> same for reserved words like 'into'. As long as the API matures, the
>> number of such special words would only grow. The .shr() and .shl() that
>> you add in this series are the quite good examples.
>>
>> Let's make a step back and just list all limitations that come with this
>> implementation.
>
> Why is a build error not sufficient to alert the user to use better judgement
> for naming?
>
> This is no different than using reserved keywords in C. For example, this
> won't compile:
>
>     int if = 5;      // error: 'if' is a reserved keyword
>     int return = 3;  // error: 'return' is a reserved keyword
>
> The user simply learns not to use reserved words, and the compiler enforces
> this clearly. The same applies here.
>
>> Again, this all is relevant for a basic generic data structure. If we
>> consider it a supporting layer for the registers, everything is totally
>> fine. In that case, we should just give it a more specific name, and
>> probably place in an different directory, closer to IO APIs.
>
> The Bitfield macro is very much required for non-register use cases too.
>
> --
> Joel Fernandes

Re: [PATCH 3/6] rust: add `bitfield!` macro

Posted by Alexandre Courbot 1 week, 5 days ago

On Wed Jan 28, 2026 at 12:02 AM JST, Gary Guo wrote:
> On Tue Jan 27, 2026 at 3:25 AM GMT, Joel Fernandes wrote:
>> On Jan 26, 2026, at 9:55 PM, Yury Norov <ynorov@nvidia.com> wrote:
>>> On Mon, Jan 26, 2026 at 10:35:49PM +0900, Alexandre Courbot wrote:
>>> > On Wed Jan 21, 2026 at 6:16 PM JST, Yury Norov wrote:
>>> > > On Tue, Jan 20, 2026 at 03:17:56PM +0900, Alexandre Courbot wrote:
>>> > > > Add a macro for defining bitfield structs 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.
>>> > > >
>>> > > > Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
>>> > > > ---
>>> > > > rust/kernel/bitfield.rs | 503 ++++++++++++++++++++++++++++++++++++++++++++++++
>>> > > > rust/kernel/lib.rs      |   1 +
>>> > > > 2 files changed, 504 insertions(+)
>> [...]
>>> > > > +/// // Setters can be chained. Bounded::new::<N>() does compile-time bounds checking.
>>> > > > +/// let color = Rgb::default()
>>> > > > +///     .set_red(Bounded::<u16, _>::new::<0x10>())
>>> > > > +///     .set_green(Bounded::<u16, _>::new::<0x1f>())
>>> > > > +///     .set_blue(Bounded::<u16, _>::new::<0x18>());
>>> > >
>>> > > Is there a way to just say:
>>> > >
>>> > >    let color = Rgb::default().
>>> > >            .set_red(0x10)
>>> > >            .set_green(0x1f)
>>> > >            .set_blue(0x18)
>>> > >
>>> > > I think it should be the default style. Later in the patch you say:
>>> > >
>>> > >    Each field is internally represented as a [`Bounded`]
>>> > >
>>> > > So, let's keep implementation decoupled from an interface?
>>> >
>>> > That is unfortunately not feasible, but the syntax above should seldomly
>>> > be used outside of examples.
>>>
>>> The above short syntax is definitely more desired over that wordy and
>>> non-trivial version that exposes implementation internals.
>>>
>>> A regular user doesn't care of the exact mechanism that protects the
>>> bitfields. He wants to just assign numbers to the fields, and let
>>> your machinery to take care of the integrity.
>>>
>>> Can you please explain in details why that's not feasible, please
>>> do it in commit message. If it's an implementation constraint,
>>> please consider to re-implement.
>>
>> If the issue is the excessive turbofish syntax, how about a macro? For
>> example:
>>
>>     let color = Rgb::default()
>>         .set_red(bounded!(u16, 0x10))
>>         .set_green(bounded!(u16, 0x1f))
>>         .set_blue(bounded!(u16, 0x18));
>>
>> This hides the turbofish and Bounded internals while still providing
>> compile-time bounds checking.
>
> I think this could be the way forward, if we also get type inference working
> properly.
>
>     Rgb::default()
>         .set_read(bounded!(0x10))
>         .set_green(bounded!(0x1f))
>         .set_blue(bounded!(0x18))
>
> is roughly the limit that I find acceptable (`Bounded::<u16, _>::new::<0x10>()`
> is something way too verbose so I find it unacceptable).
>
> I still think if we can get 
>
>     Rgb::default()
>         .set_read(0x10)
>         .set_green(0x1f)
>         .set_blue(0x18)
>
> to work with implicit `build_assert!` check it'll be ideal, although I
> understand the concern about the fragility of `build_assert!()`, especially when
> Clippy is used.
>
> I am planning to at least improve the diagnostics when `build_assert!` is used
> incorrectly and the build error actually occurs, so hopefully in the long run it
> can once again become a tool that we can rely on, but in the meantime,
> if all it needed is an extra `bounded!()` call, it doesn't bother me that much
> versus the full turbofish.

I think having a dedicated const setter method is better though. On top
of not making use of `build_assert!`, it can also be used in const
contexts to build const values, something that should be pretty useful
once we extract the `bitfield!` macro for wider use.

Re: [PATCH 3/6] rust: add `bitfield!` macro

Posted by Yury Norov 1 week, 5 days ago

On Wed, Jan 28, 2026 at 10:23:36AM +0900, Alexandre Courbot wrote:
> tatus: O
> Content-Length: 4095
> Lines: 108
> 
> On Wed Jan 28, 2026 at 12:02 AM JST, Gary Guo wrote:
> > On Tue Jan 27, 2026 at 3:25 AM GMT, Joel Fernandes wrote:
> >> On Jan 26, 2026, at 9:55 PM, Yury Norov <ynorov@nvidia.com> wrote:
> >>> On Mon, Jan 26, 2026 at 10:35:49PM +0900, Alexandre Courbot wrote:
> >>> > On Wed Jan 21, 2026 at 6:16 PM JST, Yury Norov wrote:
> >>> > > On Tue, Jan 20, 2026 at 03:17:56PM +0900, Alexandre Courbot wrote:
> >>> > > > Add a macro for defining bitfield structs 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.
> >>> > > >
> >>> > > > Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
> >>> > > > ---
> >>> > > > rust/kernel/bitfield.rs | 503 ++++++++++++++++++++++++++++++++++++++++++++++++
> >>> > > > rust/kernel/lib.rs      |   1 +
> >>> > > > 2 files changed, 504 insertions(+)
> >> [...]
> >>> > > > +/// // Setters can be chained. Bounded::new::<N>() does compile-time bounds checking.
> >>> > > > +/// let color = Rgb::default()
> >>> > > > +///     .set_red(Bounded::<u16, _>::new::<0x10>())
> >>> > > > +///     .set_green(Bounded::<u16, _>::new::<0x1f>())
> >>> > > > +///     .set_blue(Bounded::<u16, _>::new::<0x18>());
> >>> > >
> >>> > > Is there a way to just say:
> >>> > >
> >>> > >    let color = Rgb::default().
> >>> > >            .set_red(0x10)
> >>> > >            .set_green(0x1f)
> >>> > >            .set_blue(0x18)
> >>> > >
> >>> > > I think it should be the default style. Later in the patch you say:
> >>> > >
> >>> > >    Each field is internally represented as a [`Bounded`]
> >>> > >
> >>> > > So, let's keep implementation decoupled from an interface?
> >>> >
> >>> > That is unfortunately not feasible, but the syntax above should seldomly
> >>> > be used outside of examples.
> >>>
> >>> The above short syntax is definitely more desired over that wordy and
> >>> non-trivial version that exposes implementation internals.
> >>>
> >>> A regular user doesn't care of the exact mechanism that protects the
> >>> bitfields. He wants to just assign numbers to the fields, and let
> >>> your machinery to take care of the integrity.
> >>>
> >>> Can you please explain in details why that's not feasible, please
> >>> do it in commit message. If it's an implementation constraint,
> >>> please consider to re-implement.
> >>
> >> If the issue is the excessive turbofish syntax, how about a macro? For
> >> example:
> >>
> >>     let color = Rgb::default()
> >>         .set_red(bounded!(u16, 0x10))
> >>         .set_green(bounded!(u16, 0x1f))
> >>         .set_blue(bounded!(u16, 0x18));
> >>
> >> This hides the turbofish and Bounded internals while still providing
> >> compile-time bounds checking.
> >
> > I think this could be the way forward, if we also get type inference working
> > properly.
> >
> >     Rgb::default()
> >         .set_read(bounded!(0x10))
> >         .set_green(bounded!(0x1f))
> >         .set_blue(bounded!(0x18))
> >
> > is roughly the limit that I find acceptable (`Bounded::<u16, _>::new::<0x10>()`
> > is something way too verbose so I find it unacceptable).

I agree, this version is on the edge. It probably may be acceptable
because it highlights that the numbers passed in setters are some
special numbers. But yeah, it's a weak excuse.

If it was C, it could be just as simple as 

        #define set_red(v) __set_red(bounded(v))

So...

I'm not a rust professional, but I've been told many times that macro
rules in rust are so powerful that they can do any magic, even mimic
another languages.

For fun, I asked AI to draw an example where rust structure is
initialized just like normal python does, and that's what I've got:

  struct Foo {
      bar: i32,
      baz: String,
  }
  
  // Your specific constructor logic
  fn construct_bar(v: i32) -> i32 { v * 2 }
  fn construct_baz(v: i32) -> String { v.to_string() }
  
  // Helper macro to select the right function for a single field
  macro_rules! get_ctor {
      (bar, $val:expr) => { construct_bar($val) };
      (baz, $val:expr) => { construct_baz($val) };
  }
  
  macro_rules! python_init {
      ($t:ident { $($field:ident = $val:expr),* $(,)? }) => {
          $t {
              // For each field, we call the dispatcher separately
              $($field: get_ctor!($field, $val)),*
          }
      };
  }
  
  fn main() {
      let foo = python_init!(Foo { bar = 10, baz = 500 });
  
      println!("bar: {}", foo.bar); // Output: 20
      println!("baz: {}", foo.baz); // Output: "500"
  }

Indeed it's possible!

Again, I'm not a rust professional and I can't evaluate quality of the
AI-generated code, neither I can ensure there's no nasty pitfalls.

But as a user, I can say that 
        
        let rgb = bitfield!(Rgb { red: 0x10, green: 0x1f, blue: 0x18 })

would be way more readable than this beast:

   let color = Rgb::default()
       .set_red(Bounded::<u16, _>::new::<0x10>())
       .set_green(Bounded::<u16, _>::new::<0x1f>())
       .set_blue(Bounded::<u16, _>::new::<0x18>());

Thanks,
Yury

Re: [PATCH 3/6] rust: add `bitfield!` macro

Posted by Alexandre Courbot 1 week, 4 days ago

On Wed Jan 28, 2026 at 1:33 PM JST, Yury Norov wrote:
> On Wed, Jan 28, 2026 at 10:23:36AM +0900, Alexandre Courbot wrote:
>> tatus: O
>> Content-Length: 4095
>> Lines: 108
>> 
>> On Wed Jan 28, 2026 at 12:02 AM JST, Gary Guo wrote:
>> > On Tue Jan 27, 2026 at 3:25 AM GMT, Joel Fernandes wrote:
>> >> On Jan 26, 2026, at 9:55 PM, Yury Norov <ynorov@nvidia.com> wrote:
>> >>> On Mon, Jan 26, 2026 at 10:35:49PM +0900, Alexandre Courbot wrote:
>> >>> > On Wed Jan 21, 2026 at 6:16 PM JST, Yury Norov wrote:
>> >>> > > On Tue, Jan 20, 2026 at 03:17:56PM +0900, Alexandre Courbot wrote:
>> >>> > > > Add a macro for defining bitfield structs 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.
>> >>> > > >
>> >>> > > > Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
>> >>> > > > ---
>> >>> > > > rust/kernel/bitfield.rs | 503 ++++++++++++++++++++++++++++++++++++++++++++++++
>> >>> > > > rust/kernel/lib.rs      |   1 +
>> >>> > > > 2 files changed, 504 insertions(+)
>> >> [...]
>> >>> > > > +/// // Setters can be chained. Bounded::new::<N>() does compile-time bounds checking.
>> >>> > > > +/// let color = Rgb::default()
>> >>> > > > +///     .set_red(Bounded::<u16, _>::new::<0x10>())
>> >>> > > > +///     .set_green(Bounded::<u16, _>::new::<0x1f>())
>> >>> > > > +///     .set_blue(Bounded::<u16, _>::new::<0x18>());
>> >>> > >
>> >>> > > Is there a way to just say:
>> >>> > >
>> >>> > >    let color = Rgb::default().
>> >>> > >            .set_red(0x10)
>> >>> > >            .set_green(0x1f)
>> >>> > >            .set_blue(0x18)
>> >>> > >
>> >>> > > I think it should be the default style. Later in the patch you say:
>> >>> > >
>> >>> > >    Each field is internally represented as a [`Bounded`]
>> >>> > >
>> >>> > > So, let's keep implementation decoupled from an interface?
>> >>> >
>> >>> > That is unfortunately not feasible, but the syntax above should seldomly
>> >>> > be used outside of examples.
>> >>>
>> >>> The above short syntax is definitely more desired over that wordy and
>> >>> non-trivial version that exposes implementation internals.
>> >>>
>> >>> A regular user doesn't care of the exact mechanism that protects the
>> >>> bitfields. He wants to just assign numbers to the fields, and let
>> >>> your machinery to take care of the integrity.
>> >>>
>> >>> Can you please explain in details why that's not feasible, please
>> >>> do it in commit message. If it's an implementation constraint,
>> >>> please consider to re-implement.
>> >>
>> >> If the issue is the excessive turbofish syntax, how about a macro? For
>> >> example:
>> >>
>> >>     let color = Rgb::default()
>> >>         .set_red(bounded!(u16, 0x10))
>> >>         .set_green(bounded!(u16, 0x1f))
>> >>         .set_blue(bounded!(u16, 0x18));
>> >>
>> >> This hides the turbofish and Bounded internals while still providing
>> >> compile-time bounds checking.
>> >
>> > I think this could be the way forward, if we also get type inference working
>> > properly.
>> >
>> >     Rgb::default()
>> >         .set_read(bounded!(0x10))
>> >         .set_green(bounded!(0x1f))
>> >         .set_blue(bounded!(0x18))
>> >
>> > is roughly the limit that I find acceptable (`Bounded::<u16, _>::new::<0x10>()`
>> > is something way too verbose so I find it unacceptable).
>
> I agree, this version is on the edge. It probably may be acceptable
> because it highlights that the numbers passed in setters are some
> special numbers. But yeah, it's a weak excuse.
>
> If it was C, it could be just as simple as 
>
>         #define set_red(v) __set_red(bounded(v))
>
> So...
>
> I'm not a rust professional, but I've been told many times that macro
> rules in rust are so powerful that they can do any magic, even mimic
> another languages.
>
> For fun, I asked AI to draw an example where rust structure is
> initialized just like normal python does, and that's what I've got:
>
>   struct Foo {
>       bar: i32,
>       baz: String,
>   }
>   
>   // Your specific constructor logic
>   fn construct_bar(v: i32) -> i32 { v * 2 }
>   fn construct_baz(v: i32) -> String { v.to_string() }
>   
>   // Helper macro to select the right function for a single field
>   macro_rules! get_ctor {
>       (bar, $val:expr) => { construct_bar($val) };
>       (baz, $val:expr) => { construct_baz($val) };
>   }
>   
>   macro_rules! python_init {
>       ($t:ident { $($field:ident = $val:expr),* $(,)? }) => {
>           $t {
>               // For each field, we call the dispatcher separately
>               $($field: get_ctor!($field, $val)),*
>           }
>       };
>   }
>   
>   fn main() {
>       let foo = python_init!(Foo { bar = 10, baz = 500 });
>   
>       println!("bar: {}", foo.bar); // Output: 20
>       println!("baz: {}", foo.baz); // Output: "500"
>   }
>
> Indeed it's possible!

Oh yeah you can do all sorts of crazy sh** with Rust macros. :)

>
> Again, I'm not a rust professional and I can't evaluate quality of the
> AI-generated code, neither I can ensure there's no nasty pitfalls.
>
> But as a user, I can say that 
>         
>         let rgb = bitfield!(Rgb { red: 0x10, green: 0x1f, blue: 0x18 })
>
> would be way more readable than this beast:
>
>    let color = Rgb::default()
>        .set_red(Bounded::<u16, _>::new::<0x10>())
>        .set_green(Bounded::<u16, _>::new::<0x1f>())
>        .set_blue(Bounded::<u16, _>::new::<0x18>());

Without having tested the idea, a macro wrapping the whole bitfield (and
not just trying to create a bounded) looks doable. Of course, it would
have to rely on some underlying mechanism to set the fields, which could
be the abomination above, or something a bit more convenient.

It looks like we are converging towards introducing the
`with_const_field` setter for now with registers ; when we extract the
`bitfield!` I think I would like to entertain the introduction of a
macro close to what you suggested above.

Re: [PATCH 3/6] rust: add `bitfield!` macro

Posted by Yury Norov 1 week, 4 days ago

On Wed, Jan 28, 2026 at 11:02:03PM +0900, Alexandre Courbot wrote:
> On Wed Jan 28, 2026 at 1:33 PM JST, Yury Norov wrote:
> > On Wed, Jan 28, 2026 at 10:23:36AM +0900, Alexandre Courbot wrote:
> >> tatus: O
> >> Content-Length: 4095
> >> Lines: 108
> >> 
> >> On Wed Jan 28, 2026 at 12:02 AM JST, Gary Guo wrote:
> >> > On Tue Jan 27, 2026 at 3:25 AM GMT, Joel Fernandes wrote:
> >> >> On Jan 26, 2026, at 9:55 PM, Yury Norov <ynorov@nvidia.com> wrote:
> >> >>> On Mon, Jan 26, 2026 at 10:35:49PM +0900, Alexandre Courbot wrote:
> >> >>> > On Wed Jan 21, 2026 at 6:16 PM JST, Yury Norov wrote:
> >> >>> > > On Tue, Jan 20, 2026 at 03:17:56PM +0900, Alexandre Courbot wrote:
> >> >>> > > > Add a macro for defining bitfield structs 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.
> >> >>> > > >
> >> >>> > > > Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
> >> >>> > > > ---
> >> >>> > > > rust/kernel/bitfield.rs | 503 ++++++++++++++++++++++++++++++++++++++++++++++++
> >> >>> > > > rust/kernel/lib.rs      |   1 +
> >> >>> > > > 2 files changed, 504 insertions(+)
> >> >> [...]
> >> >>> > > > +/// // Setters can be chained. Bounded::new::<N>() does compile-time bounds checking.
> >> >>> > > > +/// let color = Rgb::default()
> >> >>> > > > +///     .set_red(Bounded::<u16, _>::new::<0x10>())
> >> >>> > > > +///     .set_green(Bounded::<u16, _>::new::<0x1f>())
> >> >>> > > > +///     .set_blue(Bounded::<u16, _>::new::<0x18>());
> >> >>> > >
> >> >>> > > Is there a way to just say:
> >> >>> > >
> >> >>> > >    let color = Rgb::default().
> >> >>> > >            .set_red(0x10)
> >> >>> > >            .set_green(0x1f)
> >> >>> > >            .set_blue(0x18)
> >> >>> > >
> >> >>> > > I think it should be the default style. Later in the patch you say:
> >> >>> > >
> >> >>> > >    Each field is internally represented as a [`Bounded`]
> >> >>> > >
> >> >>> > > So, let's keep implementation decoupled from an interface?
> >> >>> >
> >> >>> > That is unfortunately not feasible, but the syntax above should seldomly
> >> >>> > be used outside of examples.
> >> >>>
> >> >>> The above short syntax is definitely more desired over that wordy and
> >> >>> non-trivial version that exposes implementation internals.
> >> >>>
> >> >>> A regular user doesn't care of the exact mechanism that protects the
> >> >>> bitfields. He wants to just assign numbers to the fields, and let
> >> >>> your machinery to take care of the integrity.
> >> >>>
> >> >>> Can you please explain in details why that's not feasible, please
> >> >>> do it in commit message. If it's an implementation constraint,
> >> >>> please consider to re-implement.
> >> >>
> >> >> If the issue is the excessive turbofish syntax, how about a macro? For
> >> >> example:
> >> >>
> >> >>     let color = Rgb::default()
> >> >>         .set_red(bounded!(u16, 0x10))
> >> >>         .set_green(bounded!(u16, 0x1f))
> >> >>         .set_blue(bounded!(u16, 0x18));
> >> >>
> >> >> This hides the turbofish and Bounded internals while still providing
> >> >> compile-time bounds checking.
> >> >
> >> > I think this could be the way forward, if we also get type inference working
> >> > properly.
> >> >
> >> >     Rgb::default()
> >> >         .set_read(bounded!(0x10))
> >> >         .set_green(bounded!(0x1f))
> >> >         .set_blue(bounded!(0x18))
> >> >
> >> > is roughly the limit that I find acceptable (`Bounded::<u16, _>::new::<0x10>()`
> >> > is something way too verbose so I find it unacceptable).
> >
> > I agree, this version is on the edge. It probably may be acceptable
> > because it highlights that the numbers passed in setters are some
> > special numbers. But yeah, it's a weak excuse.
> >
> > If it was C, it could be just as simple as 
> >
> >         #define set_red(v) __set_red(bounded(v))
> >
> > So...
> >
> > I'm not a rust professional, but I've been told many times that macro
> > rules in rust are so powerful that they can do any magic, even mimic
> > another languages.
> >
> > For fun, I asked AI to draw an example where rust structure is
> > initialized just like normal python does, and that's what I've got:
> >
> >   struct Foo {
> >       bar: i32,
> >       baz: String,
> >   }
> >   
> >   // Your specific constructor logic
> >   fn construct_bar(v: i32) -> i32 { v * 2 }
> >   fn construct_baz(v: i32) -> String { v.to_string() }
> >   
> >   // Helper macro to select the right function for a single field
> >   macro_rules! get_ctor {
> >       (bar, $val:expr) => { construct_bar($val) };
> >       (baz, $val:expr) => { construct_baz($val) };
> >   }
> >   
> >   macro_rules! python_init {
> >       ($t:ident { $($field:ident = $val:expr),* $(,)? }) => {
> >           $t {
> >               // For each field, we call the dispatcher separately
> >               $($field: get_ctor!($field, $val)),*
> >           }
> >       };
> >   }
> >   
> >   fn main() {
> >       let foo = python_init!(Foo { bar = 10, baz = 500 });
> >   
> >       println!("bar: {}", foo.bar); // Output: 20
> >       println!("baz: {}", foo.baz); // Output: "500"
> >   }
> >
> > Indeed it's possible!
> 
> Oh yeah you can do all sorts of crazy sh** with Rust macros. :)
> 
> >
> > Again, I'm not a rust professional and I can't evaluate quality of the
> > AI-generated code, neither I can ensure there's no nasty pitfalls.
> >
> > But as a user, I can say that 
> >         
> >         let rgb = bitfield!(Rgb { red: 0x10, green: 0x1f, blue: 0x18 })
> >
> > would be way more readable than this beast:
> >
> >    let color = Rgb::default()
> >        .set_red(Bounded::<u16, _>::new::<0x10>())
> >        .set_green(Bounded::<u16, _>::new::<0x1f>())
> >        .set_blue(Bounded::<u16, _>::new::<0x18>());
> 
> Without having tested the idea, a macro wrapping the whole bitfield (and
> not just trying to create a bounded) looks doable. Of course, it would
> have to rely on some underlying mechanism to set the fields, which could
> be the abomination above, or something a bit more convenient.

As soon as it's not exposed, it's fine.
 
> It looks like we are converging towards introducing the
> `with_const_field` setter for now with registers ; when we extract the
> `bitfield!` I think I would like to entertain the introduction of a
> macro close to what you suggested above.

Good. Happy you find it useful.

Thanks,
Yury

Re: [PATCH 3/6] rust: add `bitfield!` macro

Posted by Yury Norov 1 week, 6 days ago

On Mon, Jan 26, 2026 at 10:25:54PM -0500, Joel Fernandes wrote:
> On Jan 26, 2026, at 9:55 PM, Yury Norov <ynorov@nvidia.com> wrote:
> > On Mon, Jan 26, 2026 at 10:35:49PM +0900, Alexandre Courbot wrote:
> > > On Wed Jan 21, 2026 at 6:16 PM JST, Yury Norov wrote:
> > > > On Tue, Jan 20, 2026 at 03:17:56PM +0900, Alexandre Courbot wrote:
> > > > > Add a macro for defining bitfield structs 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.
> > > > >
> > > > > Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
> > > > > ---
> > > > > rust/kernel/bitfield.rs | 503 ++++++++++++++++++++++++++++++++++++++++++++++++
> > > > > rust/kernel/lib.rs      |   1 +
> > > > > 2 files changed, 504 insertions(+)
> [...]
> > > > > +/// // Setters can be chained. Bounded::new::<N>() does compile-time bounds checking.
> > > > > +/// let color = Rgb::default()
> > > > > +///     .set_red(Bounded::<u16, _>::new::<0x10>())
> > > > > +///     .set_green(Bounded::<u16, _>::new::<0x1f>())
> > > > > +///     .set_blue(Bounded::<u16, _>::new::<0x18>());
> > > >
> > > > Is there a way to just say:
> > > >
> > > >    let color = Rgb::default().
> > > >            .set_red(0x10)
> > > >            .set_green(0x1f)
> > > >            .set_blue(0x18)
> > > >
> > > > I think it should be the default style. Later in the patch you say:
> > > >
> > > >    Each field is internally represented as a [`Bounded`]
> > > >
> > > > So, let's keep implementation decoupled from an interface?
> > >
> > > That is unfortunately not feasible, but the syntax above should seldomly
> > > be used outside of examples.
> >
> > The above short syntax is definitely more desired over that wordy and
> > non-trivial version that exposes implementation internals.
> >
> > A regular user doesn't care of the exact mechanism that protects the
> > bitfields. He wants to just assign numbers to the fields, and let
> > your machinery to take care of the integrity.
> >
> > Can you please explain in details why that's not feasible, please
> > do it in commit message. If it's an implementation constraint,
> > please consider to re-implement.
> 
> If the issue is the excessive turbofish syntax, how about a macro? For
> example:
> 
>     let color = Rgb::default()
>         .set_red(bounded!(u16, 0x10))
>         .set_green(bounded!(u16, 0x1f))
>         .set_blue(bounded!(u16, 0x18));
>
> This hides the turbofish and Bounded internals while still providing
> compile-time bounds checking.

No it does not. I clearly see this 'bounded' in your version. Can you
completely hide it inside the setter? 
 
> [...]
> > > > What Rgb::BLUE_SHIFT would mean in this case? Maybe Rgb::SHIFT(blue)?
> > >
> > > You wouldn't even have the luxury to yse `BLUE_SHIFT` here because where
> > > would be conflicting definitions and thus a build error.
> [...]
> > > > What color.set_blue() and color.into() would mean? Even if they work,
> > > > I think, to stay on safe side there should be a more conventional set
> > > > of accessors: color.get(into), color.set(set_blue, 0xff) and son on.
> > >
> > > This would just not build.
> >
> > I know it wouldn't. I am just trying to understand corner cases that may
> > (and would!) frustrate people for decades.
> >
> > I understand that this implementation works just great for the registers,
> > but my role is to make sure that it would work equally well for everyone.
> > Now, for example, Rust, contrary to C in Linux, actively uses camel case.
> > So, the blue vs Blue vs BLUE restriction is a very valid concern. The
> > same for reserved words like 'into'. As long as the API matures, the
> > number of such special words would only grow. The .shr() and .shl() that
> > you add in this series are the quite good examples.
> >
> > Let's make a step back and just list all limitations that come with this
> > implementation.
> 
> Why is a build error not sufficient to alert the user to use better judgement
> for naming?

There should be no error at all - that's why. Any name that one can
use for a regular variable should be acceptable as a field name.

> This is no different than using reserved keywords in C. For example, this
> won't compile:
> 
>     int if = 5;      // error: 'if' is a reserved keyword
>     int return = 3;  // error: 'return' is a reserved keyword

'Into', 'as_raw', 'shr' and 'shl' *are not* the reserved words in Rust.
Rust makes difference between upper and lower cases and doesn't break
build if you do:

        let blue = 1;
        let BLUE = 2;

So the bitfields should.

> The user simply learns not to use reserved words, and the compiler enforces
> this clearly. The same applies here.

Most likely he will learn not to use this API at all. The worst thing about
those 'reserved' words is that the set of them is not definite and will
constantly grow.

I'm trying to say that this approach is not scalable. If a random client
gives name 'invert' to one of his fields, and you'll need to implement a
method inverting bits, what are you going to do?

> > Again, this all is relevant for a basic generic data structure. If we
> > consider it a supporting layer for the registers, everything is totally
> > fine. In that case, we should just give it a more specific name, and
> > probably place in an different directory, closer to IO APIs.
> 
> The Bitfield macro is very much required for non-register use cases too.

Then let's implement it better. Can you comment why the suggested API
doesn't work for you?

        color.set(blue, 10);
        color.get(blue);

I think it should solve the problem with name clashing.

Can you share more about the other potential users?

Thanks,
Yury

Re: [PATCH 3/6] rust: add `bitfield!` macro

Posted by Joel Fernandes 1 week, 5 days ago

> On Jan 26, 2026, at 11:49 PM, Yury Norov <ynorov@nvidia.com> wrote:
>
> On Mon, Jan 26, 2026 at 10:25:54PM -0500, Joel Fernandes wrote:
>>> On Jan 26, 2026, at 9:55 PM, Yury Norov <ynorov@nvidia.com> wrote:
>>> On Mon, Jan 26, 2026 at 10:35:49PM +0900, Alexandre Courbot wrote:
>>>> On Wed Jan 21, 2026 at 6:16 PM JST, Yury Norov wrote:
>>>>> On Tue, Jan 20, 2026 at 03:17:56PM +0900, Alexandre Courbot wrote:
>>>>>> Add a macro for defining bitfield structs 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.
>>>>>>
>>>>>> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
>>>>>> ---
>>>>>> rust/kernel/bitfield.rs | 503 ++++++++++++++++++++++++++++++++++++++++++++++++
>>>>>> rust/kernel/lib.rs      |   1 +
>>>>>> 2 files changed, 504 insertions(+)
>> [...]
>>>>>> +/// // Setters can be chained. Bounded::new::<N>() does compile-time bounds checking.
>>>>>> +/// let color = Rgb::default()
>>>>>> +///     .set_red(Bounded::<u16, _>::new::<0x10>())
>>>>>> +///     .set_green(Bounded::<u16, _>::new::<0x1f>())
>>>>>> +///     .set_blue(Bounded::<u16, _>::new::<0x18>());
>>>>>
>>>>> Is there a way to just say:
>>>>>
>>>>>   let color = Rgb::default().
>>>>>           .set_red(0x10)
>>>>>           .set_green(0x1f)
>>>>>           .set_blue(0x18)
>>>>>
>>>>> I think it should be the default style. Later in the patch you say:
>>>>>
>>>>>   Each field is internally represented as a [`Bounded`]
>>>>>
>>>>> So, let's keep implementation decoupled from an interface?
>>>>
>>>> That is unfortunately not feasible, but the syntax above should seldomly
>>>> be used outside of examples.
>>>
>>> The above short syntax is definitely more desired over that wordy and
>>> non-trivial version that exposes implementation internals.
>>>
>>> A regular user doesn't care of the exact mechanism that protects the
>>> bitfields. He wants to just assign numbers to the fields, and let
>>> your machinery to take care of the integrity.
>>>
>>> Can you please explain in details why that's not feasible, please
>>> do it in commit message. If it's an implementation constraint,
>>> please consider to re-implement.
>>
>> If the issue is the excessive turbofish syntax, how about a macro? For
>> example:
>>
>>    let color = Rgb::default()
>>        .set_red(bounded!(u16, 0x10))
>>        .set_green(bounded!(u16, 0x1f))
>>        .set_blue(bounded!(u16, 0x18));
>>
>> This hides the turbofish and Bounded internals while still providing
>> compile-time bounds checking.
>
> No it does not. I clearly see this 'bounded' in your version. Can you
> completely hide it inside the setter?

Looks like Alex replied with something that does that. So it is good.

>
>> [...]
>>>>> What Rgb::BLUE_SHIFT would mean in this case? Maybe Rgb::SHIFT(blue)?
>>>>
>>>> You wouldn't even have the luxury to yse `BLUE_SHIFT` here because where
>>>> would be conflicting definitions and thus a build error.
>> [...]
>>>>> What color.set_blue() and color.into() would mean? Even if they work,
>>>>> I think, to stay on safe side there should be a more conventional set
>>>>> of accessors: color.get(into), color.set(set_blue, 0xff) and son on.
>>>>
>>>> This would just not build.
>>>
>>> I know it wouldn't. I am just trying to understand corner cases that may
>>> (and would!) frustrate people for decades.
>>>
>>> I understand that this implementation works just great for the registers,
>>> but my role is to make sure that it would work equally well for everyone.
>>> Now, for example, Rust, contrary to C in Linux, actively uses camel case.
>>> So, the blue vs Blue vs BLUE restriction is a very valid concern. The
>>> same for reserved words like 'into'. As long as the API matures, the
>>> number of such special words would only grow. The .shr() and .shl() that
>>> you add in this series are the quite good examples.
>>>
>>> Let's make a step back and just list all limitations that come with this
>>> implementation.
>>
>> Why is a build error not sufficient to alert the user to use better judgement
>> for naming?
>
> There should be no error at all - that's why. Any name that one can
> use for a regular variable should be acceptable as a field name.

Disagreed. Here is why: the Bitfield macro is required because we are extending
the language. We could very well get this as a language feature in the future.
In the event of that, it is non sensical to allow users to use reserved
keywords. The same reason you would run into errors on the C side.

>> This is no different than using reserved keywords in C. For example, this
>> won't compile:
>>
>>    int if = 5;      // error: 'if' is a reserved keyword
>>    int return = 3;  // error: 'return' is a reserved keyword
>
> 'Into', 'as_raw', 'shr' and 'shl' *are not* the reserved words in Rust.

They are reserved as far as our Bitfield syntax extension goes? Note that this
is a macro that introduces custom syntax so it is more akin to a language
extension than an api. Note well also that macros in rust for Linux have been
used extensively for filling gaps language features (see pin initialization for
instance).

> Rust makes difference between upper and lower cases and doesn't break
> build if you do:
>
>        let blue = 1;
>        let BLUE = 2;
>
> So the bitfields should.

Why on earth would anyone want to do this though? It’s utterly nonsensical to
do so and I would rather break the build than encourage bad code. Mixing case
for 2 fields in the same struct is not a usecase anyone should have.

>> The user simply learns not to use reserved words, and the compiler enforces
>> this clearly. The same applies here.
>
> Most likely he will learn not to use this API at all. The worst thing about
> those 'reserved' words is that the set of them is not definite and will
> constantly grow.
>
> I'm trying to say that this approach is not scalable. If a random client
> gives name 'invert' to one of his fields, and you'll need to implement a
> method inverting bits, what are you going to do?

I would error the build out. See above for comments on language extensions and
reserved keywords.

>>> Again, this all is relevant for a basic generic data structure. If we
>>> consider it a supporting layer for the registers, everything is totally
>>> fine. In that case, we should just give it a more specific name, and
>>> probably place in an different directory, closer to IO APIs.
>>
>> The Bitfield macro is very much required for non-register use cases too.
>
> Then let's implement it better. Can you comment why the suggested API
> doesn't work for you?
>
>        color.set(blue, 10);
>        color.get(blue);

For the right reasons, I do not mind it but it certainly cannot be that we want
to support mixed case of the same field IMO. And the reserved keyword argument
is weak at best, for a language extension.

> I think it should solve the problem with name clashing.
>
> Can you share more about the other potential users?

Sure, take a look at my memory management patches for nova. [1]

[1] https://lore.kernel.org/all/20260120204303.3229303-1-joelagnelf@nvidia.com/

-- 
Joel Fernandes

Re: [PATCH 3/6] rust: add `bitfield!` macro

Posted by Alexandre Courbot 1 week, 5 days ago

On Tue Jan 27, 2026 at 1:49 PM JST, Yury Norov wrote:
> On Mon, Jan 26, 2026 at 10:25:54PM -0500, Joel Fernandes wrote:
>> On Jan 26, 2026, at 9:55 PM, Yury Norov <ynorov@nvidia.com> wrote:
>> > On Mon, Jan 26, 2026 at 10:35:49PM +0900, Alexandre Courbot wrote:
>> > > On Wed Jan 21, 2026 at 6:16 PM JST, Yury Norov wrote:
>> > > > On Tue, Jan 20, 2026 at 03:17:56PM +0900, Alexandre Courbot wrote:
>> > > > > Add a macro for defining bitfield structs 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.
>> > > > >
>> > > > > Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
>> > > > > ---
>> > > > > rust/kernel/bitfield.rs | 503 ++++++++++++++++++++++++++++++++++++++++++++++++
>> > > > > rust/kernel/lib.rs      |   1 +
>> > > > > 2 files changed, 504 insertions(+)
>> [...]
>> > > > > +/// // Setters can be chained. Bounded::new::<N>() does compile-time bounds checking.
>> > > > > +/// let color = Rgb::default()
>> > > > > +///     .set_red(Bounded::<u16, _>::new::<0x10>())
>> > > > > +///     .set_green(Bounded::<u16, _>::new::<0x1f>())
>> > > > > +///     .set_blue(Bounded::<u16, _>::new::<0x18>());
>> > > >
>> > > > Is there a way to just say:
>> > > >
>> > > >    let color = Rgb::default().
>> > > >            .set_red(0x10)
>> > > >            .set_green(0x1f)
>> > > >            .set_blue(0x18)
>> > > >
>> > > > I think it should be the default style. Later in the patch you say:
>> > > >
>> > > >    Each field is internally represented as a [`Bounded`]
>> > > >
>> > > > So, let's keep implementation decoupled from an interface?
>> > >
>> > > That is unfortunately not feasible, but the syntax above should seldomly
>> > > be used outside of examples.
>> >
>> > The above short syntax is definitely more desired over that wordy and
>> > non-trivial version that exposes implementation internals.
>> >
>> > A regular user doesn't care of the exact mechanism that protects the
>> > bitfields. He wants to just assign numbers to the fields, and let
>> > your machinery to take care of the integrity.
>> >
>> > Can you please explain in details why that's not feasible, please
>> > do it in commit message. If it's an implementation constraint,
>> > please consider to re-implement.
>> 
>> If the issue is the excessive turbofish syntax, how about a macro? For
>> example:
>> 
>>     let color = Rgb::default()
>>         .set_red(bounded!(u16, 0x10))
>>         .set_green(bounded!(u16, 0x1f))
>>         .set_blue(bounded!(u16, 0x18));
>>
>> This hides the turbofish and Bounded internals while still providing
>> compile-time bounds checking.
>
> No it does not. I clearly see this 'bounded' in your version. Can you
> completely hide it inside the setter? 

Hopefully the `with_` setter I proposed in my other email settles this
issue.

>  
>> [...]
>> > > > What Rgb::BLUE_SHIFT would mean in this case? Maybe Rgb::SHIFT(blue)?
>> > >
>> > > You wouldn't even have the luxury to yse `BLUE_SHIFT` here because where
>> > > would be conflicting definitions and thus a build error.
>> [...]
>> > > > What color.set_blue() and color.into() would mean? Even if they work,
>> > > > I think, to stay on safe side there should be a more conventional set
>> > > > of accessors: color.get(into), color.set(set_blue, 0xff) and son on.
>> > >
>> > > This would just not build.
>> >
>> > I know it wouldn't. I am just trying to understand corner cases that may
>> > (and would!) frustrate people for decades.
>> >
>> > I understand that this implementation works just great for the registers,
>> > but my role is to make sure that it would work equally well for everyone.
>> > Now, for example, Rust, contrary to C in Linux, actively uses camel case.
>> > So, the blue vs Blue vs BLUE restriction is a very valid concern. The
>> > same for reserved words like 'into'. As long as the API matures, the
>> > number of such special words would only grow. The .shr() and .shl() that
>> > you add in this series are the quite good examples.
>> >
>> > Let's make a step back and just list all limitations that come with this
>> > implementation.
>> 
>> Why is a build error not sufficient to alert the user to use better judgement
>> for naming?
>
> There should be no error at all - that's why. Any name that one can
> use for a regular variable should be acceptable as a field name.
>
>> This is no different than using reserved keywords in C. For example, this
>> won't compile:
>> 
>>     int if = 5;      // error: 'if' is a reserved keyword
>>     int return = 3;  // error: 'return' is a reserved keyword
>
> 'Into', 'as_raw', 'shr' and 'shl' *are not* the reserved words in Rust.
> Rust makes difference between upper and lower cases and doesn't break
> build if you do:
>
>         let blue = 1;
>         let BLUE = 2;
>
> So the bitfields should.

We can solve this case if we break the Rust naming conventions for
constants and keep the original case for the field, e.g. `blue_SHIFT`
and `BLUE_SHIFT`.

But again, by convention fields should all be snake_case or it would
look weird in Rust code.

>
>> The user simply learns not to use reserved words, and the compiler enforces
>> this clearly. The same applies here.
>
> Most likely he will learn not to use this API at all. The worst thing about
> those 'reserved' words is that the set of them is not definite and will
> constantly grow.
>
> I'm trying to say that this approach is not scalable. If a random client
> gives name 'invert' to one of his fields, and you'll need to implement a
> method inverting bits, what are you going to do?

Bitfields are limited to a _get, a _set, and an _update methods (and
possibly try_ variants for the last two). If an `invert` method needs to
be implemented, it can be done on top of _update which takes a closure.
So I am pretty confident we won't need to extend the API beyond these
(famous last words).

>
>> > Again, this all is relevant for a basic generic data structure. If we
>> > consider it a supporting layer for the registers, everything is totally
>> > fine. In that case, we should just give it a more specific name, and
>> > probably place in an different directory, closer to IO APIs.
>> 
>> The Bitfield macro is very much required for non-register use cases too.
>
> Then let's implement it better. Can you comment why the suggested API
> doesn't work for you?
>
>         color.set(blue, 10);
>         color.get(blue);
>
> I think it should solve the problem with name clashing.

That syntax cannot be implemented as it is written. What type is `blue` here?

The closest we could get would be a macro, that would look like

    bitfield_set!(color, blue, 10);

And beyond the scenes it would call some more intricate (and unsightly)
machinery. I'd rather define the constraints clearly for users - they
are not so drastic.

>
> Can you share more about the other potential users?

I know Joel is using bitfield for page table structures, but there are
of course many others. Basically any structure with fields defined as a
subset of its bits is a candidate. Registers just happen to be bitfields
with extra properties for I/O.

Re: [PATCH 3/6] rust: add `bitfield!` macro

Posted by Yury Norov 1 week, 5 days ago

On Tue, Jan 27, 2026 at 07:41:50PM +0900, Alexandre Courbot wrote:
> On Tue Jan 27, 2026 at 1:49 PM JST, Yury Norov wrote:
> > On Mon, Jan 26, 2026 at 10:25:54PM -0500, Joel Fernandes wrote:
> >> On Jan 26, 2026, at 9:55 PM, Yury Norov <ynorov@nvidia.com> wrote:
> >> > On Mon, Jan 26, 2026 at 10:35:49PM +0900, Alexandre Courbot wrote:
> >> > > On Wed Jan 21, 2026 at 6:16 PM JST, Yury Norov wrote:
> >> > > > On Tue, Jan 20, 2026 at 03:17:56PM +0900, Alexandre Courbot wrote:
> >> > > > > Add a macro for defining bitfield structs with bounds-checked accessors.
> >> > > > >

...

> > 'Into', 'as_raw', 'shr' and 'shl' *are not* the reserved words in Rust.
> > Rust makes difference between upper and lower cases and doesn't break
> > build if you do:
> >
> >         let blue = 1;
> >         let BLUE = 2;
> >
> > So the bitfields should.
> 
> We can solve this case if we break the Rust naming conventions for
> constants and keep the original case for the field, e.g. `blue_SHIFT`
> and `BLUE_SHIFT`.

Or like C++ handles static methods: Rgb::SHIFT(blue)

> But again, by convention fields should all be snake_case or it would
> look weird in Rust code.

I'm pretty sure that for such a basic data structure people will
eventually find a case that will overweight the convention.

> >> The user simply learns not to use reserved words, and the compiler enforces
> >> this clearly. The same applies here.
> >
> > Most likely he will learn not to use this API at all. The worst thing about
> > those 'reserved' words is that the set of them is not definite and will
> > constantly grow.
> >
> > I'm trying to say that this approach is not scalable. If a random client
> > gives name 'invert' to one of his fields, and you'll need to implement a
> > method inverting bits, what are you going to do?
> 
> Bitfields are limited to a _get, a _set, and an _update methods (and
> possibly try_ variants for the last two). If an `invert` method needs to
> be implemented, it can be done on top of _update which takes a closure.
> So I am pretty confident we won't need to extend the API beyond these
> (famous last words).
 
Not sure I understand this. You already have into(), shr(), shl() and
others. By the way, maybe again follow C++ style, like:
        
        my_bitfield.shr         // field
        my_bitfield.shr()       // method

> >> > Again, this all is relevant for a basic generic data structure. If we
> >> > consider it a supporting layer for the registers, everything is totally
> >> > fine. In that case, we should just give it a more specific name, and
> >> > probably place in an different directory, closer to IO APIs.
> >> 
> >> The Bitfield macro is very much required for non-register use cases too.
> >
> > Then let's implement it better. Can you comment why the suggested API
> > doesn't work for you?
> >
> >         color.set(blue, 10);
> >         color.get(blue);
> >
> > I think it should solve the problem with name clashing.
> 
> That syntax cannot be implemented as it is written. What type is `blue` here?

'blue' has no type because it is not a variable but keyword. We do
such things in C all the time:

        DEFINE_FREE(kfree, void *, if (_T) kfree(_T))
                    ^^^^^
                    keyword that becomes a part of cleanup function name

And in another email I seemingly do similar thing for python_init!()
macro in rust to pick the right constructor.

> The closest we could get would be a macro, that would look like
> 
>     bitfield_set!(color, blue, 10);
> 
> And beyond the scenes it would call some more intricate (and unsightly)
> machinery. I'd rather define the constraints clearly for users - they
> are not so drastic.
 
But that would not be chainable, I guess. I recall, Joel said it's an
important feature for some reason.

> > Can you share more about the other potential users?
> 
> I know Joel is using bitfield for page table structures, but there are
> of course many others. Basically any structure with fields defined as a
> subset of its bits is a candidate. Registers just happen to be bitfields
> with extra properties for I/O.

OK. Can I take a look at how bitfields are used there?

Thanks,
Yury

Re: [PATCH 3/6] rust: add `bitfield!` macro

Posted by Alexandre Courbot 1 week, 4 days ago

On Wed Jan 28, 2026 at 2:27 PM JST, Yury Norov wrote:
<snip>
>> >> The user simply learns not to use reserved words, and the compiler enforces
>> >> this clearly. The same applies here.
>> >
>> > Most likely he will learn not to use this API at all. The worst thing about
>> > those 'reserved' words is that the set of them is not definite and will
>> > constantly grow.
>> >
>> > I'm trying to say that this approach is not scalable. If a random client
>> > gives name 'invert' to one of his fields, and you'll need to implement a
>> > method inverting bits, what are you going to do?
>> 
>> Bitfields are limited to a _get, a _set, and an _update methods (and
>> possibly try_ variants for the last two). If an `invert` method needs to
>> be implemented, it can be done on top of _update which takes a closure.
>> So I am pretty confident we won't need to extend the API beyond these
>> (famous last words).
>  
> Not sure I understand this. You already have into(), shr(), shl() and
> others. By the way, maybe again follow C++ style, like:
>         
>         my_bitfield.shr         // field
>         my_bitfield.shr()       // method

The confusion comes from the fact that `shr` and pals are methods of
`Bounded`, not the bitfield structure. I.e. you call them on the field
that you obtained using the getter method - they are not part of the
bitfield interface itself which is only concerned with 3 basic
operations: get a field, set a field, update a field with a closure.

>
>> >> > Again, this all is relevant for a basic generic data structure. If we
>> >> > consider it a supporting layer for the registers, everything is totally
>> >> > fine. In that case, we should just give it a more specific name, and
>> >> > probably place in an different directory, closer to IO APIs.
>> >> 
>> >> The Bitfield macro is very much required for non-register use cases too.
>> >
>> > Then let's implement it better. Can you comment why the suggested API
>> > doesn't work for you?
>> >
>> >         color.set(blue, 10);
>> >         color.get(blue);
>> >
>> > I think it should solve the problem with name clashing.
>> 
>> That syntax cannot be implemented as it is written. What type is `blue` here?
>
> 'blue' has no type because it is not a variable but keyword. We do
> such things in C all the time:
>
>         DEFINE_FREE(kfree, void *, if (_T) kfree(_T))
>                     ^^^^^
>                     keyword that becomes a part of cleanup function name
>
> And in another email I seemingly do similar thing for python_init!()
> macro in rust to pick the right constructor.

Yup, and we could do the same in Rust with a macro, but your example
above was not macro code (macros are always ending with a `!`).

>
>> The closest we could get would be a macro, that would look like
>> 
>>     bitfield_set!(color, blue, 10);
>> 
>> And beyond the scenes it would call some more intricate (and unsightly)
>> machinery. I'd rather define the constraints clearly for users - they
>> are not so drastic.
>  
> But that would not be chainable, I guess. I recall, Joel said it's an
> important feature for some reason.

We could make the macros return the modified bitfield, which would make
them chainable. And the bitfield-englobing macro you proposed in your
other email could also reduce the need to do so anyway.

>
>> > Can you share more about the other potential users?
>> 
>> I know Joel is using bitfield for page table structures, but there are
>> of course many others. Basically any structure with fields defined as a
>> subset of its bits is a candidate. Registers just happen to be bitfields
>> with extra properties for I/O.
>
> OK. Can I take a look at how bitfields are used there?

Check out these patches:

https://lore.kernel.org/all/20260120204303.3229303-10-joelagnelf@nvidia.com/
https://lore.kernel.org/all/20260120204303.3229303-12-joelagnelf@nvidia.com/
https://lore.kernel.org/all/20260120204303.3229303-13-joelagnelf@nvidia.com/

But be aware that they use the `bitfield!` macro of nova-core, this one
doesn't use `Bounded` yet.

Re: [PATCH 3/6] rust: add `bitfield!` macro

Posted by Yury Norov 1 week, 4 days ago

On Wed, Jan 28, 2026 at 11:12:27PM +0900, Alexandre Courbot wrote:
> On Wed Jan 28, 2026 at 2:27 PM JST, Yury Norov wrote:

 <snip>

> >> > Then let's implement it better. Can you comment why the suggested API
> >> > doesn't work for you?
> >> >
> >> >         color.set(blue, 10);
> >> >         color.get(blue);
> >> >
> >> > I think it should solve the problem with name clashing.
> >> 
> >> That syntax cannot be implemented as it is written. What type is `blue` here?
> >
> > 'blue' has no type because it is not a variable but keyword. We do
> > such things in C all the time:
> >
> >         DEFINE_FREE(kfree, void *, if (_T) kfree(_T))
> >                     ^^^^^
> >                     keyword that becomes a part of cleanup function name
> >
> > And in another email I seemingly do similar thing for python_init!()
> > macro in rust to pick the right constructor.
> 
> Yup, and we could do the same in Rust with a macro, but your example
> above was not macro code (macros are always ending with a `!`).

color.set!(blue, 10) is equally good, if it helps.

If someone will point to the excessive use of macros, you're safe to
ignore it. This is the very basic fundamental type, and I believe that
it brings enough attention and expertise to make it safe.

> >> The closest we could get would be a macro, that would look like
> >> 
> >>     bitfield_set!(color, blue, 10);

Or maybe bitfield_set!(color.blue, 10), but the above looks more natural.
Is it possible to make it color.blue = 10 by any chance?

Again (and again) - usability and readability is a king, and I'm glad
we're moving toward the right direction.

> >> And beyond the scenes it would call some more intricate (and unsightly)
> >> machinery. I'd rather define the constraints clearly for users - they
> >> are not so drastic.
> >  
> > But that would not be chainable, I guess. I recall, Joel said it's an
> > important feature for some reason.
> 
> We could make the macros return the modified bitfield, which would make
> them chainable. And the bitfield-englobing macro you proposed in your
> other email could also reduce the need to do so anyway.
> 
> >
> >> > Can you share more about the other potential users?
> >> 
> >> I know Joel is using bitfield for page table structures, but there are
> >> of course many others. Basically any structure with fields defined as a
> >> subset of its bits is a candidate. Registers just happen to be bitfields
> >> with extra properties for I/O.
> >
> > OK. Can I take a look at how bitfields are used there?
> 
> Check out these patches:
> 
> https://lore.kernel.org/all/20260120204303.3229303-10-joelagnelf@nvidia.com/
> https://lore.kernel.org/all/20260120204303.3229303-12-joelagnelf@nvidia.com/
> https://lore.kernel.org/all/20260120204303.3229303-13-joelagnelf@nvidia.com/
> 
> But be aware that they use the `bitfield!` macro of nova-core, this one
> doesn't use `Bounded` yet.

Re: [PATCH 3/6] rust: add `bitfield!` macro

Posted by Alexandre Courbot 1 week, 3 days ago

On Thu Jan 29, 2026 at 3:05 AM JST, Yury Norov wrote:
> On Wed, Jan 28, 2026 at 11:12:27PM +0900, Alexandre Courbot wrote:
>> On Wed Jan 28, 2026 at 2:27 PM JST, Yury Norov wrote:
>
>  <snip>
>
>> >> > Then let's implement it better. Can you comment why the suggested API
>> >> > doesn't work for you?
>> >> >
>> >> >         color.set(blue, 10);
>> >> >         color.get(blue);
>> >> >
>> >> > I think it should solve the problem with name clashing.
>> >> 
>> >> That syntax cannot be implemented as it is written. What type is `blue` here?
>> >
>> > 'blue' has no type because it is not a variable but keyword. We do
>> > such things in C all the time:
>> >
>> >         DEFINE_FREE(kfree, void *, if (_T) kfree(_T))
>> >                     ^^^^^
>> >                     keyword that becomes a part of cleanup function name
>> >
>> > And in another email I seemingly do similar thing for python_init!()
>> > macro in rust to pick the right constructor.
>> 
>> Yup, and we could do the same in Rust with a macro, but your example
>> above was not macro code (macros are always ending with a `!`).
>
> color.set!(blue, 10) is equally good, if it helps.

But not doable unfortunately, since macros cannot be declared as
methods. The closest we could get is

  bitfield_set!(color, blue, 10);

>
> If someone will point to the excessive use of macros, you're safe to
> ignore it. This is the very basic fundamental type, and I believe that
> it brings enough attention and expertise to make it safe.

I think the consensus is that macros are fine if they can hide
boilerplace. For instance, the `vec!` macro declares and initializes a
vector with content and spares you the need to write a loop. A proper
and elegant bitfield initializer would fit within that category imho.

>
>> >> The closest we could get would be a macro, that would look like
>> >> 
>> >>     bitfield_set!(color, blue, 10);
>
> Or maybe bitfield_set!(color.blue, 10), but the above looks more natural.
> Is it possible to make it color.blue = 10 by any chance?

the `= 10` part is no problem. However I am not sure we can parse
`color.blue` safely since the macro should also support
`object.bitfield.color` as an argument and won't be able to
differenciate between the path to the bitfield and the field itself
(hopefully that makes sense).

>
> Again (and again) - usability and readability is a king, and I'm glad
> we're moving toward the right direction.

I am confident we can find something that is both nice to use and
efficient under the hood when we start extracting the bitfield macro out
of `register!` during the next cycle. We've already made good progress.

Re: [PATCH 3/6] rust: add `bitfield!` macro

Posted by Miguel Ojeda 1 week, 3 days ago

On Thu, Jan 29, 2026 at 2:41 PM Alexandre Courbot <acourbot@nvidia.com> wrote:
>
> But not doable unfortunately, since macros cannot be declared as
> methods.

Indeed -- it may be doable in the future with postfix macros if they land:

  https://github.com/rust-lang/rfcs/pull/2442

(The proposed ones do not do type-based dispatch).

Cheers,
Miguel

Re: [PATCH 3/6] rust: add `bitfield!` macro

Posted by Miguel Ojeda 1 week, 5 days ago

On Tue, Jan 27, 2026 at 11:41 AM Alexandre Courbot <acourbot@nvidia.com> wrote:
>
> The closest we could get would be a macro, that would look like
>
>     bitfield_set!(color, blue, 10);
>
> And beyond the scenes it would call some more intricate (and unsightly)
> machinery. I'd rather define the constraints clearly for users - they
> are not so drastic.

Yeah, our guideline has always been to avoid macros unless really needed.

Cheers,
Miguel

Re: [PATCH 3/6] rust: add `bitfield!` macro

Posted by Dirk Behme 2 weeks, 5 days ago

Hi Alexandre,

On 20/01/2026 07:17, Alexandre Courbot wrote:
> Add a macro for defining bitfield structs 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.
> 
> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
> ---
>   rust/kernel/bitfield.rs | 503 ++++++++++++++++++++++++++++++++++++++++++++++++
>   rust/kernel/lib.rs      |   1 +
>   2 files changed, 504 insertions(+)
> 
> diff --git a/rust/kernel/bitfield.rs b/rust/kernel/bitfield.rs
> new file mode 100644
> index 000000000000..2926ab802227
> --- /dev/null
> +++ b/rust/kernel/bitfield.rs
> @@ -0,0 +1,503 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! Support for defining bitfields as Rust structures.
> +
> +/// Defines a bitfield struct with bounds-checked accessors for individual bit ranges.
> +///
> +/// # Example
> +///
> +/// ```rust
> +/// use kernel::bitfield;
> +/// use kernel::num::Bounded;
> +///
> +/// bitfield! {
> +///     pub struct Rgb(u16) {
> +///         15:11 blue;
> +///         10:5 green;
> +///         4:0 red;
> +///     }
> +/// }
> +///
> +/// // Setters can be chained. Bounded::new::<N>() does compile-time bounds checking.
> +/// let color = Rgb::default()
> +///     .set_red(Bounded::<u16, _>::new::<0x10>())
> +///     .set_green(Bounded::<u16, _>::new::<0x1f>())
> +///     .set_blue(Bounded::<u16, _>::new::<0x18>());
> +///
> +/// assert_eq!(color.red(), 0x10);
> +/// assert_eq!(color.green(), 0x1f);
> +/// assert_eq!(color.blue(), 0x18);
> +/// assert_eq!(
> +///     color.as_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]
> +///     pub struct Name(storage_type), "Struct documentation." {
> +///         hi:lo field_1, "Field documentation.";
> +///         hi:lo field_2 => ConvertedType, "Field documentation.";
> +///         hi:lo field_3 ?=> ConvertedType, "Field documentation.";
> +///         ...
> +///     }
> +/// }
> +/// ```
> +///
> +/// - `storage_type`: The underlying integer type (`u8`, `u16`, `u32`, `u64`).
> +/// - `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, and [`Default`] and [`Debug`] implementations.
> +///
> +/// For each field, it also generates:
> +/// - `field()` - getter returning a [`Bounded`] (or converted type) for the field,
> +/// - `set_field(value)` - setter with compile-time bounds checking,
> +/// - `try_set_field(value)` - setter with runtime bounds checking (for fields without type
> +///   conversion),
> +/// - `FIELD_MASK`, `FIELD_SHIFT`, `FIELD_RANGE` - constants for manual bit manipulation.
> +///
> +/// # 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::default()
> +///     .set_byte_field(0x42_u8)
> +///     .set_flag(true);
> +///
> +/// assert_eq!(flags.as_raw(), (0x42 << Flags::BYTE_FIELD_SHIFT) | 1);
> +/// ```
> +///
> +/// # Runtime bounds checking
> +///
> +/// When a value is not known at compile time, use `try_set_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_set_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 when all bit patterns map to valid values:
> +///
> +/// ```rust
> +/// use kernel::bitfield;
> +/// use kernel::num::Bounded;
> +///
> +/// #[derive(Debug, Clone, Copy, Default, PartialEq)]
> +/// enum Power {
> +///     #[default]
> +///     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::default().set_power(Power::On);
> +/// assert_eq!(ctrl.power(), Power::On);
> +/// ```
> +///
> +/// ## Fallible conversion (`?=>`)
> +///
> +/// Use when some bit patterns are invalid. The getter returns a [`Result`]:
> +///
> +/// ```rust
> +/// use kernel::bitfield;
> +/// use kernel::num::Bounded;
> +///
> +/// #[derive(Debug, Clone, Copy, Default, PartialEq)]
> +/// enum Mode {
> +///     #[default]
> +///     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::default().set_mode(Mode::Auto);
> +/// assert_eq!(cfg.mode(), Ok(Mode::Auto));
> +///
> +/// // Invalid bit pattern returns an error.
> +/// assert_eq!(Config::from(0b11).mode(), Err(3));
> +/// ```
> +///
> +/// [`Bounded`]: kernel::num::Bounded
> +#[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)
> +            $(, $comment:literal)? { $($fields:tt)* }
> +    ) => {
> +        ::kernel::bitfield!(@core $(#[$attr])* $vis $name $storage $(, $comment)?);
> +        ::kernel::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 $(, $comment:literal)?) => {
> +        $(
> +        #[doc=$comment]
> +        )?
> +        $(#[$attr])*
> +        #[repr(transparent)]
> +        #[derive(Clone, Copy, PartialEq, Eq)]
> +        $vis struct $name($storage);
> +
> +        #[allow(dead_code)]
> +        impl $name {
> +            /// Returns the raw value of this bitfield.
> +            ///
> +            /// This is similar to the [`From`] implementation, but is shorter to invoke in
> +            /// most cases.
> +            $vis fn as_raw(self) -> $storage {
> +                self.0
> +            }
> +        }
> +
> +        impl ::core::convert::From<$name> for $storage {
> +            fn from(val: $name) -> $storage {
> +                val.0
> +            }
> +        }
> +
> +        impl ::core::convert::From<$storage> for $name {
> +            fn from(val: $storage) -> $name {
> +                Self(val)
> +            }
> +        }
> +    };
> +
> +    // Definitions requiring knowledge of individual fields: private and public field accessors,
> +    // and `Debug` and `Default` implementations.
> +    (@fields $vis:vis $name:ident $storage:ty {
> +        $($hi:tt:$lo:tt $field:ident
> +            $(?=> $try_into_type:ty)?
> +            $(=> $into_type:ty)?
> +            $(, $comment:literal)?
> +        ;
> +        )*
> +    }
> +    ) => {
> +        #[allow(dead_code)]
> +        impl $name {
> +        $(
> +        ::kernel::bitfield!(@private_field_accessors $vis $name $storage : $hi:$lo $field);
> +        ::kernel::bitfield!(@public_field_accessors $vis $name $storage : $hi:$lo $field
> +            $(?=> $try_into_type)?
> +            $(=> $into_type)?
> +            $(, $comment)?
> +        );
> +        )*
> +        }
> +
> +        ::kernel::bitfield!(@debug $name { $($field;)* });
> +        ::kernel::bitfield!(@default $name { $($field;)* });
> +    };
> +
> +    // Private field accessors working with the correct `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.0 << ALIGN_TOP
> +            );
> +            val.shr::<ALIGN_BOTTOM, _>()

What have I missed?

error[E0747]: type provided when a constant was expected
    --> rust/kernel/bitfield.rs:318:37
     |
318 |               val.shr::<ALIGN_BOTTOM, _>()
     |                                       ^
...
566 | /     bitfield! {
567 | |         struct TestPageTableEntry(u64) {
568 | |             0:0       present;
569 | |             1:1       writable;
...   |
574 | |         }
575 | |     }
     | |_____- in this macro invocation
     |
     = help: const arguments cannot yet be inferred with `_`
     = note: this error originates in the macro `::kernel::bitfield` 
which comes from the expansion of the macro `bitfield` (in Nightly 
builds, run with -Z macro-backtrace for more info)
help: add `#![feature(generic_arg_infer)]` to the crate attributes to enable
    --> rust/kernel/lib.rs:59:1
     |
59  + #![feature(generic_arg_infer)]


$ rustc --version
rustc 1.81.0 (eeb90cda1 2024-09-04)

Thanks

Dirk

Re: [PATCH 3/6] rust: add `bitfield!` macro

Posted by Alexandre Courbot 2 weeks, 5 days ago

Hi Dirk,

On Tue Jan 20, 2026 at 8:45 PM JST, Dirk Behme wrote:
> Hi Alexandre,
>
> On 20/01/2026 07:17, Alexandre Courbot wrote:
>> Add a macro for defining bitfield structs 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.
>> 
>> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
>> ---
>>   rust/kernel/bitfield.rs | 503 ++++++++++++++++++++++++++++++++++++++++++++++++
>>   rust/kernel/lib.rs      |   1 +
>>   2 files changed, 504 insertions(+)
>> 
>> diff --git a/rust/kernel/bitfield.rs b/rust/kernel/bitfield.rs
>> new file mode 100644
>> index 000000000000..2926ab802227
>> --- /dev/null
>> +++ b/rust/kernel/bitfield.rs
>> @@ -0,0 +1,503 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +
>> +//! Support for defining bitfields as Rust structures.
>> +
>> +/// Defines a bitfield struct with bounds-checked accessors for individual bit ranges.
>> +///
>> +/// # Example
>> +///
>> +/// ```rust
>> +/// use kernel::bitfield;
>> +/// use kernel::num::Bounded;
>> +///
>> +/// bitfield! {
>> +///     pub struct Rgb(u16) {
>> +///         15:11 blue;
>> +///         10:5 green;
>> +///         4:0 red;
>> +///     }
>> +/// }
>> +///
>> +/// // Setters can be chained. Bounded::new::<N>() does compile-time bounds checking.
>> +/// let color = Rgb::default()
>> +///     .set_red(Bounded::<u16, _>::new::<0x10>())
>> +///     .set_green(Bounded::<u16, _>::new::<0x1f>())
>> +///     .set_blue(Bounded::<u16, _>::new::<0x18>());
>> +///
>> +/// assert_eq!(color.red(), 0x10);
>> +/// assert_eq!(color.green(), 0x1f);
>> +/// assert_eq!(color.blue(), 0x18);
>> +/// assert_eq!(
>> +///     color.as_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]
>> +///     pub struct Name(storage_type), "Struct documentation." {
>> +///         hi:lo field_1, "Field documentation.";
>> +///         hi:lo field_2 => ConvertedType, "Field documentation.";
>> +///         hi:lo field_3 ?=> ConvertedType, "Field documentation.";
>> +///         ...
>> +///     }
>> +/// }
>> +/// ```
>> +///
>> +/// - `storage_type`: The underlying integer type (`u8`, `u16`, `u32`, `u64`).
>> +/// - `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, and [`Default`] and [`Debug`] implementations.
>> +///
>> +/// For each field, it also generates:
>> +/// - `field()` - getter returning a [`Bounded`] (or converted type) for the field,
>> +/// - `set_field(value)` - setter with compile-time bounds checking,
>> +/// - `try_set_field(value)` - setter with runtime bounds checking (for fields without type
>> +///   conversion),
>> +/// - `FIELD_MASK`, `FIELD_SHIFT`, `FIELD_RANGE` - constants for manual bit manipulation.
>> +///
>> +/// # 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::default()
>> +///     .set_byte_field(0x42_u8)
>> +///     .set_flag(true);
>> +///
>> +/// assert_eq!(flags.as_raw(), (0x42 << Flags::BYTE_FIELD_SHIFT) | 1);
>> +/// ```
>> +///
>> +/// # Runtime bounds checking
>> +///
>> +/// When a value is not known at compile time, use `try_set_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_set_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 when all bit patterns map to valid values:
>> +///
>> +/// ```rust
>> +/// use kernel::bitfield;
>> +/// use kernel::num::Bounded;
>> +///
>> +/// #[derive(Debug, Clone, Copy, Default, PartialEq)]
>> +/// enum Power {
>> +///     #[default]
>> +///     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::default().set_power(Power::On);
>> +/// assert_eq!(ctrl.power(), Power::On);
>> +/// ```
>> +///
>> +/// ## Fallible conversion (`?=>`)
>> +///
>> +/// Use when some bit patterns are invalid. The getter returns a [`Result`]:
>> +///
>> +/// ```rust
>> +/// use kernel::bitfield;
>> +/// use kernel::num::Bounded;
>> +///
>> +/// #[derive(Debug, Clone, Copy, Default, PartialEq)]
>> +/// enum Mode {
>> +///     #[default]
>> +///     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::default().set_mode(Mode::Auto);
>> +/// assert_eq!(cfg.mode(), Ok(Mode::Auto));
>> +///
>> +/// // Invalid bit pattern returns an error.
>> +/// assert_eq!(Config::from(0b11).mode(), Err(3));
>> +/// ```
>> +///
>> +/// [`Bounded`]: kernel::num::Bounded
>> +#[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)
>> +            $(, $comment:literal)? { $($fields:tt)* }
>> +    ) => {
>> +        ::kernel::bitfield!(@core $(#[$attr])* $vis $name $storage $(, $comment)?);
>> +        ::kernel::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 $(, $comment:literal)?) => {
>> +        $(
>> +        #[doc=$comment]
>> +        )?
>> +        $(#[$attr])*
>> +        #[repr(transparent)]
>> +        #[derive(Clone, Copy, PartialEq, Eq)]
>> +        $vis struct $name($storage);
>> +
>> +        #[allow(dead_code)]
>> +        impl $name {
>> +            /// Returns the raw value of this bitfield.
>> +            ///
>> +            /// This is similar to the [`From`] implementation, but is shorter to invoke in
>> +            /// most cases.
>> +            $vis fn as_raw(self) -> $storage {
>> +                self.0
>> +            }
>> +        }
>> +
>> +        impl ::core::convert::From<$name> for $storage {
>> +            fn from(val: $name) -> $storage {
>> +                val.0
>> +            }
>> +        }
>> +
>> +        impl ::core::convert::From<$storage> for $name {
>> +            fn from(val: $storage) -> $name {
>> +                Self(val)
>> +            }
>> +        }
>> +    };
>> +
>> +    // Definitions requiring knowledge of individual fields: private and public field accessors,
>> +    // and `Debug` and `Default` implementations.
>> +    (@fields $vis:vis $name:ident $storage:ty {
>> +        $($hi:tt:$lo:tt $field:ident
>> +            $(?=> $try_into_type:ty)?
>> +            $(=> $into_type:ty)?
>> +            $(, $comment:literal)?
>> +        ;
>> +        )*
>> +    }
>> +    ) => {
>> +        #[allow(dead_code)]
>> +        impl $name {
>> +        $(
>> +        ::kernel::bitfield!(@private_field_accessors $vis $name $storage : $hi:$lo $field);
>> +        ::kernel::bitfield!(@public_field_accessors $vis $name $storage : $hi:$lo $field
>> +            $(?=> $try_into_type)?
>> +            $(=> $into_type)?
>> +            $(, $comment)?
>> +        );
>> +        )*
>> +        }
>> +
>> +        ::kernel::bitfield!(@debug $name { $($field;)* });
>> +        ::kernel::bitfield!(@default $name { $($field;)* });
>> +    };
>> +
>> +    // Private field accessors working with the correct `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.0 << ALIGN_TOP
>> +            );
>> +            val.shr::<ALIGN_BOTTOM, _>()
>
> What have I missed?
>
> error[E0747]: type provided when a constant was expected
>     --> rust/kernel/bitfield.rs:318:37
>      |
> 318 |               val.shr::<ALIGN_BOTTOM, _>()
>      |                                       ^
> ...
> 566 | /     bitfield! {
> 567 | |         struct TestPageTableEntry(u64) {
> 568 | |             0:0       present;
> 569 | |             1:1       writable;
> ...   |
> 574 | |         }
> 575 | |     }
>      | |_____- in this macro invocation
>      |
>      = help: const arguments cannot yet be inferred with `_`
>      = note: this error originates in the macro `::kernel::bitfield` 
> which comes from the expansion of the macro `bitfield` (in Nightly 
> builds, run with -Z macro-backtrace for more info)
> help: add `#![feature(generic_arg_infer)]` to the crate attributes to enable
>     --> rust/kernel/lib.rs:59:1
>      |
> 59  + #![feature(generic_arg_infer)]
>
>
> $ rustc --version
> rustc 1.81.0 (eeb90cda1 2024-09-04)

Ah, I'm the one who missed something - this code builds fine with rustc
1.92, but on older versions the second generic parameter of `shr` cannot
be inferred and we need to specify the expected number of bits
explicitly. I will fix that in v2 as we need to support 1.78 anyway.

Re: [PATCH 3/6] rust: add `bitfield!` macro

Posted by Miguel Ojeda 2 weeks, 5 days ago

On Tue, Jan 20, 2026 at 12:45 PM Dirk Behme <dirk.behme@de.bosch.com> wrote:
>
> What have I missed?

I think the patch is missing enabling the (already stable by now)
feature. If you add it at the top of `rust/kernel/lib.rs`, does it
work for you?

Thanks for catching that!

Cheers,
Miguel

Re: [PATCH 3/6] rust: add `bitfield!` macro

Posted by Dirk Behme 2 weeks, 5 days ago

On 20/01/2026 13:37, Miguel Ojeda wrote:
> On Tue, Jan 20, 2026 at 12:45 PM Dirk Behme <dirk.behme@de.bosch.com> wrote:
>>
>> What have I missed?
> 
> I think the patch is missing enabling the (already stable by now)
> feature. If you add it at the top of `rust/kernel/lib.rs`, does it
> work for you?

Yes :)

And I think we need the same for the doctests where it fails as well then:

error[E0747]: type provided when a constant was expected
     --> rust/doctests_kernel_generated.rs:2758:1
      |
2758 | / bitfield! {
2759 | |     pub struct Config(u8) {
2760 | |         3:0 nibble;
2761 | |     }
2762 | | }
      | |_^
      |
      = help: const arguments cannot yet be inferred with `_`
      = help: add `#![feature(generic_arg_infer)]` to the crate 
attributes to enable
      = note: this error originates in the macro `::kernel::bitfield` 
which comes from the expansion of the macro `bitfield` (in Nightly 
builds, run with -Z macro-backtrace for more info)

error[E0747]: type provided when a constant was expected
     --> rust/doctests_kernel_generated.rs:2846:1
      |
2846 | / bitfield! {
2847 | |     pub struct Control(u32) {
2848 | |         0:0 power => Power;
2849 | |     }
2850 | | }
      | |_^
      |
      = help: const arguments cannot yet be inferred with `_`
      = help: add `#![feature(generic_arg_infer)]` to the crate 
attributes to enable
      = note: this error originates in the macro `::kernel::bitfield` 
which comes from the expansion of the macro `bitfield` (in Nightly 
builds, run with -Z macro-backtrace for more info)

error[E0747]: type provided when a constant was expected
     --> rust/doctests_kernel_generated.rs:2934:41
      |
2934 |             Mode::Low => Bounded::<u32, _>::new::<0>(),
      |                                         ^
      |
      = help: const arguments cannot yet be inferred with `_`
      = help: add `#![feature(generic_arg_infer)]` to the crate 
attributes to enable

...


Thanks!

Dirk

Re: [PATCH 3/6] rust: add `bitfield!` macro

Posted by Miguel Ojeda 2 weeks, 5 days ago

On Tue, Jan 20, 2026 at 1:47 PM Dirk Behme <dirk.behme@de.bosch.com> wrote:
>
> And I think we need the same for the doctests where it fails as well then:

Yeah, since it is a macro meant for other crates, if we want to use
the feature, then we should have it in the set of Rust allowed
features for all kernel code.

But I see Alexandre has already replied and IIUC he plans to provide
it explicitly instead?

I wonder if we could just use it instead. Hmm... I see there was a PR
1.86 that significantly reworked the implementation, and Debian has
1.85 only, so perhaps it is a good idea to conservatively avoid the
feature, even if we may not hit any differences in practice.

Cheers,
Miguel

Re: [PATCH 3/6] rust: add `bitfield!` macro

Posted by Alexandre Courbot 2 weeks, 5 days ago

On Tue Jan 20, 2026 at 10:08 PM JST, Miguel Ojeda wrote:
> On Tue, Jan 20, 2026 at 1:47 PM Dirk Behme <dirk.behme@de.bosch.com> wrote:
>>
>> And I think we need the same for the doctests where it fails as well then:
>
> Yeah, since it is a macro meant for other crates, if we want to use
> the feature, then we should have it in the set of Rust allowed
> features for all kernel code.
>
> But I see Alexandre has already replied and IIUC he plans to provide
> it explicitly instead?
>
> I wonder if we could just use it instead. Hmm... I see there was a PR
> 1.86 that significantly reworked the implementation, and Debian has
> 1.85 only, so perhaps it is a good idea to conservatively avoid the
> feature, even if we may not hit any differences in practice.

We can avoid the feature altogether, but it is of course more convenient
if we can infer these generic parameters when possible. But if you
prefer we eschew that, no big deal. One can even argue the code is more
readable that way.

I haven't found how to enable it for doctests anyway - maybe this is not
currently doable?

Re: [PATCH 3/6] rust: add `bitfield!` macro

Posted by Miguel Ojeda 2 weeks, 5 days ago

On Tue, Jan 20, 2026 at 2:20 PM Alexandre Courbot <acourbot@nvidia.com> wrote:
>
> We can avoid the feature altogether, but it is of course more convenient
> if we can infer these generic parameters when possible. But if you
> prefer we eschew that, no big deal. One can even argue the code is more
> readable that way.

As long as it builds (infers) properly with the minimum version, it
seems fine to me to use.

> I haven't found how to enable it for doctests anyway - maybe this is not
> currently doable?

"World-available" features are enabled for the entire kernel,
including doctests (this is what I meant in my reply to Dirk).

We list them in the `rust_allowed_features` line in `scripts/Makefile.build`.

Thanks!

Cheers,
Miguel