* [PATCH v2 1/4] nova-core: bitstruct: Move bitfield-specific code from register! into new macro
2025-09-03 21:54 [PATCH v2 0/4] Improve bitfield support in Rust Joel Fernandes
@ 2025-09-03 21:54 ` Joel Fernandes
2025-09-08 3:12 ` Alexandre Courbot
2025-09-03 21:54 ` [PATCH v2 2/4] nova-core: bitstruct: Add support for different storage widths Joel Fernandes
` (2 subsequent siblings)
3 siblings, 1 reply; 28+ messages in thread
From: Joel Fernandes @ 2025-09-03 21:54 UTC (permalink / raw)
To: linux-kernel, dri-devel, dakr, acourbot
Cc: Alistair Popple, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
bjorn3_gh, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, David Airlie, Simona Vetter, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, John Hubbard, Joel Fernandes,
Timur Tabi, joel, Elle Rhumsaa, Daniel Almeida, nouveau,
rust-for-linux
The bitfield-specific into new macro. This will be used to define
structs with bitfields, similar to C language.
Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
---
drivers/gpu/nova-core/bitstruct.rs | 271 +++++++++++++++++++++++++++
drivers/gpu/nova-core/nova_core.rs | 3 +
drivers/gpu/nova-core/regs/macros.rs | 247 +-----------------------
3 files changed, 282 insertions(+), 239 deletions(-)
create mode 100644 drivers/gpu/nova-core/bitstruct.rs
diff --git a/drivers/gpu/nova-core/bitstruct.rs b/drivers/gpu/nova-core/bitstruct.rs
new file mode 100644
index 000000000000..1dd9edab7d07
--- /dev/null
+++ b/drivers/gpu/nova-core/bitstruct.rs
@@ -0,0 +1,271 @@
+// SPDX-License-Identifier: GPL-2.0
+//
+// bitstruct.rs — Bitfield library for Rust structures
+//
+// A library that provides support for defining bit fields in Rust
+// structures. Also used from things that need bitfields like register macro.
+///
+/// # Syntax
+///
+/// ```rust
+/// bitstruct! {
+/// struct ControlReg {
+/// 3:0 mode as u8 ?=> Mode;
+/// 7:4 state as u8 => State;
+/// }
+/// }
+/// ```
+///
+/// This generates a struct with:
+/// - Field accessors: `mode()`, `state()`, etc.
+/// - Field setters: `set_mode()`, `set_state()`, etc. (supports builder pattern).
+/// - Debug and Default implementations
+///
+/// The field setters can be used with the builder pattern, example:
+/// ControlReg::default().set_mode(mode).set_state(state);
+///
+/// Fields are defined as follows:
+///
+/// - `as <type>` simply returns the field value casted to <type>, typically `u32`, `u16`, `u8` or
+/// `bool`. Note that `bool` fields must have a range of 1 bit.
+/// - `as <type> => <into_type>` calls `<into_type>`'s `From::<<type>>` implementation and returns
+/// the result.
+/// - `as <type> ?=> <try_into_type>` calls `<try_into_type>`'s `TryFrom::<<type>>` implementation
+/// and returns the result. This is useful with fields for which not all values are valid.
+macro_rules! bitstruct {
+ // Main entry point - defines the bitfield struct with fields
+ (struct $name:ident $(, $comment:literal)? { $($fields:tt)* }) => {
+ bitstruct!(@core $name $(, $comment)? { $($fields)* });
+ };
+
+ // All rules below are helpers.
+
+ // Defines the wrapper `$name` type, as well as its relevant implementations (`Debug`,
+ // `Default`, `BitOr`, and conversion to the value type) and field accessor methods.
+ (@core $name:ident $(, $comment:literal)? { $($fields:tt)* }) => {
+ $(
+ #[doc=$comment]
+ )?
+ #[repr(transparent)]
+ #[derive(Clone, Copy)]
+ pub(crate) struct $name(u32);
+
+ impl ::core::ops::BitOr for $name {
+ type Output = Self;
+
+ fn bitor(self, rhs: Self) -> Self::Output {
+ Self(self.0 | rhs.0)
+ }
+ }
+
+ impl ::core::convert::From<$name> for u32 {
+ fn from(val: $name) -> u32 {
+ val.0
+ }
+ }
+
+ bitstruct!(@fields_dispatcher $name { $($fields)* });
+ };
+
+ // Captures the fields and passes them to all the implementers that require field information.
+ //
+ // Used to simplify the matching rules for implementers, so they don't need to match the entire
+ // complex fields rule even though they only make use of part of it.
+ (@fields_dispatcher $name:ident {
+ $($hi:tt:$lo:tt $field:ident as $type:tt
+ $(?=> $try_into_type:ty)?
+ $(=> $into_type:ty)?
+ $(, $comment:literal)?
+ ;
+ )*
+ }
+ ) => {
+ bitstruct!(@field_accessors $name {
+ $(
+ $hi:$lo $field as $type
+ $(?=> $try_into_type)?
+ $(=> $into_type)?
+ $(, $comment)?
+ ;
+ )*
+ });
+ bitstruct!(@debug $name { $($field;)* });
+ bitstruct!(@default $name { $($field;)* });
+ };
+
+ // Defines all the field getter/setter methods for `$name`.
+ (
+ @field_accessors $name:ident {
+ $($hi:tt:$lo:tt $field:ident as $type:tt
+ $(?=> $try_into_type:ty)?
+ $(=> $into_type:ty)?
+ $(, $comment:literal)?
+ ;
+ )*
+ }
+ ) => {
+ $(
+ bitstruct!(@check_field_bounds $hi:$lo $field as $type);
+ )*
+
+ #[allow(dead_code)]
+ impl $name {
+ $(
+ bitstruct!(@field_accessor $name $hi:$lo $field as $type
+ $(?=> $try_into_type)?
+ $(=> $into_type)?
+ $(, $comment)?
+ ;
+ );
+ )*
+ }
+ };
+
+ // Boolean fields must have `$hi == $lo`.
+ (@check_field_bounds $hi:tt:$lo:tt $field:ident as bool) => {
+ #[allow(clippy::eq_op)]
+ const _: () = {
+ ::kernel::build_assert!(
+ $hi == $lo,
+ concat!("boolean field `", stringify!($field), "` covers more than one bit")
+ );
+ };
+ };
+
+ // Non-boolean fields must have `$hi >= $lo`.
+ (@check_field_bounds $hi:tt:$lo:tt $field:ident as $type:tt) => {
+ #[allow(clippy::eq_op)]
+ const _: () = {
+ ::kernel::build_assert!(
+ $hi >= $lo,
+ concat!("field `", stringify!($field), "`'s MSB is smaller than its LSB")
+ );
+ };
+ };
+
+ // Catches fields defined as `bool` and convert them into a boolean value.
+ (
+ @field_accessor $name:ident $hi:tt:$lo:tt $field:ident as bool => $into_type:ty
+ $(, $comment:literal)?;
+ ) => {
+ bitstruct!(
+ @leaf_accessor $name $hi:$lo $field
+ { |f| <$into_type>::from(if f != 0 { true } else { false }) }
+ $into_type => $into_type $(, $comment)?;
+ );
+ };
+
+ // Shortcut for fields defined as `bool` without the `=>` syntax.
+ (
+ @field_accessor $name:ident $hi:tt:$lo:tt $field:ident as bool $(, $comment:literal)?;
+ ) => {
+ bitstruct!(@field_accessor $name $hi:$lo $field as bool => bool $(, $comment)?;);
+ };
+
+ // Catches the `?=>` syntax for non-boolean fields.
+ (
+ @field_accessor $name:ident $hi:tt:$lo:tt $field:ident as $type:tt ?=> $try_into_type:ty
+ $(, $comment:literal)?;
+ ) => {
+ bitstruct!(@leaf_accessor $name $hi:$lo $field
+ { |f| <$try_into_type>::try_from(f as $type) } $try_into_type =>
+ ::core::result::Result<
+ $try_into_type,
+ <$try_into_type as ::core::convert::TryFrom<$type>>::Error
+ >
+ $(, $comment)?;);
+ };
+
+ // Catches the `=>` syntax for non-boolean fields.
+ (
+ @field_accessor $name:ident $hi:tt:$lo:tt $field:ident as $type:tt => $into_type:ty
+ $(, $comment:literal)?;
+ ) => {
+ bitstruct!(@leaf_accessor $name $hi:$lo $field
+ { |f| <$into_type>::from(f as $type) } $into_type => $into_type $(, $comment)?;);
+ };
+
+ // Shortcut for non-boolean fields defined without the `=>` or `?=>` syntax.
+ (
+ @field_accessor $name:ident $hi:tt:$lo:tt $field:ident as $type:tt
+ $(, $comment:literal)?;
+ ) => {
+ bitstruct!(@field_accessor $name $hi:$lo $field as $type => $type $(, $comment)?;);
+ };
+
+ // Generates the accessor methods for a single field.
+ (
+ @leaf_accessor $name:ident $hi:tt:$lo:tt $field:ident
+ { $process:expr } $to_type:ty => $res_type:ty $(, $comment:literal)?;
+ ) => {
+ ::kernel::macros::paste!(
+ const [<$field:upper _RANGE>]: ::core::ops::RangeInclusive<u8> = $lo..=$hi;
+ const [<$field:upper _MASK>]: u32 = ((((1 << $hi) - 1) << 1) + 1) - ((1 << $lo) - 1);
+ const [<$field:upper _SHIFT>]: u32 = Self::[<$field:upper _MASK>].trailing_zeros();
+ );
+
+ $(
+ #[doc="Returns the value of this field:"]
+ #[doc=$comment]
+ )?
+ #[inline(always)]
+ pub(crate) fn $field(self) -> $res_type {
+ ::kernel::macros::paste!(
+ const MASK: u32 = $name::[<$field:upper _MASK>];
+ const SHIFT: u32 = $name::[<$field:upper _SHIFT>];
+ );
+ let field = ((self.0 & MASK) >> SHIFT);
+
+ $process(field)
+ }
+
+ ::kernel::macros::paste!(
+ $(
+ #[doc="Sets the value of this field:"]
+ #[doc=$comment]
+ )?
+ #[inline(always)]
+ pub(crate) fn [<set_ $field>](mut self, value: $to_type) -> Self {
+ const MASK: u32 = $name::[<$field:upper _MASK>];
+ const SHIFT: u32 = $name::[<$field:upper _SHIFT>];
+ let value = (u32::from(value) << SHIFT) & MASK;
+ self.0 = (self.0 & !MASK) | value;
+
+ self
+ }
+ );
+ };
+
+ // Generates the `Debug` implementation for `$name`.
+ (@debug $name:ident { $($field:ident;)* }) => {
+ impl ::core::fmt::Debug for $name {
+ fn fmt(&self, f: &mut ::core::fmt::Formatter<'_>) -> ::core::fmt::Result {
+ f.debug_struct(stringify!($name))
+ .field("<raw>", &format_args!("{:#x}", &self.0))
+ $(
+ .field(stringify!($field), &self.$field())
+ )*
+ .finish()
+ }
+ }
+ };
+
+ // Generates the `Default` implementation for `$name`.
+ (@default $name:ident { $($field:ident;)* }) => {
+ /// Returns a value for the bitstruct 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.[<set_ $field>](Default::default());
+ )*
+ );
+
+ value
+ }
+ }
+ };
+}
diff --git a/drivers/gpu/nova-core/nova_core.rs b/drivers/gpu/nova-core/nova_core.rs
index cb2bbb30cba1..b218a2d42573 100644
--- a/drivers/gpu/nova-core/nova_core.rs
+++ b/drivers/gpu/nova-core/nova_core.rs
@@ -2,6 +2,9 @@
//! Nova Core GPU Driver
+#[macro_use]
+mod bitstruct;
+
mod dma;
mod driver;
mod falcon;
diff --git a/drivers/gpu/nova-core/regs/macros.rs b/drivers/gpu/nova-core/regs/macros.rs
index 754c14ee7f40..3fb6852dff04 100644
--- a/drivers/gpu/nova-core/regs/macros.rs
+++ b/drivers/gpu/nova-core/regs/macros.rs
@@ -284,25 +284,25 @@ pub(crate) trait RegisterBase<T> {
macro_rules! register {
// Creates a register at a fixed offset of the MMIO space.
($name:ident @ $offset:literal $(, $comment:literal)? { $($fields:tt)* } ) => {
- register!(@core $name $(, $comment)? { $($fields)* } );
+ bitstruct!(struct $name $(, $comment)? { $($fields)* } );
register!(@io_fixed $name @ $offset);
};
// Creates an alias register of fixed offset register `alias` with its own fields.
($name:ident => $alias:ident $(, $comment:literal)? { $($fields:tt)* } ) => {
- register!(@core $name $(, $comment)? { $($fields)* } );
+ bitstruct!(struct $name $(, $comment)? { $($fields)* } );
register!(@io_fixed $name @ $alias::OFFSET);
};
// Creates a register at a relative offset from a base address provider.
($name:ident @ $base:ty [ $offset:literal ] $(, $comment:literal)? { $($fields:tt)* } ) => {
- register!(@core $name $(, $comment)? { $($fields)* } );
+ bitstruct!(struct $name $(, $comment)? { $($fields)* } );
register!(@io_relative $name @ $base [ $offset ]);
};
// Creates an alias register of relative offset register `alias` with its own fields.
($name:ident => $base:ty [ $alias:ident ] $(, $comment:literal)? { $($fields:tt)* }) => {
- register!(@core $name $(, $comment)? { $($fields)* } );
+ bitstruct!(struct $name $(, $comment)? { $($fields)* } );
register!(@io_relative $name @ $base [ $alias::OFFSET ]);
};
@@ -313,7 +313,7 @@ macro_rules! register {
}
) => {
static_assert!(::core::mem::size_of::<u32>() <= $stride);
- register!(@core $name $(, $comment)? { $($fields)* } );
+ bitstruct!(struct $name $(, $comment)? { $($fields)* } );
register!(@io_array $name @ $offset [ $size ; $stride ]);
};
@@ -334,7 +334,7 @@ macro_rules! register {
$(, $comment:literal)? { $($fields:tt)* }
) => {
static_assert!(::core::mem::size_of::<u32>() <= $stride);
- register!(@core $name $(, $comment)? { $($fields)* } );
+ bitstruct!(struct $name $(, $comment)? { $($fields)* } );
register!(@io_relative_array $name @ $base [ $offset [ $size ; $stride ] ]);
};
@@ -356,7 +356,7 @@ macro_rules! register {
}
) => {
static_assert!($idx < $alias::SIZE);
- register!(@core $name $(, $comment)? { $($fields)* } );
+ bitstruct!(struct $name $(, $comment)? { $($fields)* } );
register!(@io_relative $name @ $base [ $alias::OFFSET + $idx * $alias::STRIDE ] );
};
@@ -365,241 +365,10 @@ macro_rules! register {
// to avoid it being interpreted in place of the relative register array alias rule.
($name:ident => $alias:ident [ $idx:expr ] $(, $comment:literal)? { $($fields:tt)* }) => {
static_assert!($idx < $alias::SIZE);
- register!(@core $name $(, $comment)? { $($fields)* } );
+ bitstruct!(struct $name $(, $comment)? { $($fields)* } );
register!(@io_fixed $name @ $alias::OFFSET + $idx * $alias::STRIDE );
};
- // All rules below are helpers.
-
- // Defines the wrapper `$name` type, as well as its relevant implementations (`Debug`,
- // `Default`, `BitOr`, and conversion to the value type) and field accessor methods.
- (@core $name:ident $(, $comment:literal)? { $($fields:tt)* }) => {
- $(
- #[doc=$comment]
- )?
- #[repr(transparent)]
- #[derive(Clone, Copy)]
- pub(crate) struct $name(u32);
-
- impl ::core::ops::BitOr for $name {
- type Output = Self;
-
- fn bitor(self, rhs: Self) -> Self::Output {
- Self(self.0 | rhs.0)
- }
- }
-
- impl ::core::convert::From<$name> for u32 {
- fn from(reg: $name) -> u32 {
- reg.0
- }
- }
-
- register!(@fields_dispatcher $name { $($fields)* });
- };
-
- // Captures the fields and passes them to all the implementers that require field information.
- //
- // Used to simplify the matching rules for implementers, so they don't need to match the entire
- // complex fields rule even though they only make use of part of it.
- (@fields_dispatcher $name:ident {
- $($hi:tt:$lo:tt $field:ident as $type:tt
- $(?=> $try_into_type:ty)?
- $(=> $into_type:ty)?
- $(, $comment:literal)?
- ;
- )*
- }
- ) => {
- register!(@field_accessors $name {
- $(
- $hi:$lo $field as $type
- $(?=> $try_into_type)?
- $(=> $into_type)?
- $(, $comment)?
- ;
- )*
- });
- register!(@debug $name { $($field;)* });
- register!(@default $name { $($field;)* });
- };
-
- // Defines all the field getter/methods methods for `$name`.
- (
- @field_accessors $name:ident {
- $($hi:tt:$lo:tt $field:ident as $type:tt
- $(?=> $try_into_type:ty)?
- $(=> $into_type:ty)?
- $(, $comment:literal)?
- ;
- )*
- }
- ) => {
- $(
- register!(@check_field_bounds $hi:$lo $field as $type);
- )*
-
- #[allow(dead_code)]
- impl $name {
- $(
- register!(@field_accessor $name $hi:$lo $field as $type
- $(?=> $try_into_type)?
- $(=> $into_type)?
- $(, $comment)?
- ;
- );
- )*
- }
- };
-
- // Boolean fields must have `$hi == $lo`.
- (@check_field_bounds $hi:tt:$lo:tt $field:ident as bool) => {
- #[allow(clippy::eq_op)]
- const _: () = {
- ::kernel::build_assert!(
- $hi == $lo,
- concat!("boolean field `", stringify!($field), "` covers more than one bit")
- );
- };
- };
-
- // Non-boolean fields must have `$hi >= $lo`.
- (@check_field_bounds $hi:tt:$lo:tt $field:ident as $type:tt) => {
- #[allow(clippy::eq_op)]
- const _: () = {
- ::kernel::build_assert!(
- $hi >= $lo,
- concat!("field `", stringify!($field), "`'s MSB is smaller than its LSB")
- );
- };
- };
-
- // Catches fields defined as `bool` and convert them into a boolean value.
- (
- @field_accessor $name:ident $hi:tt:$lo:tt $field:ident as bool => $into_type:ty
- $(, $comment:literal)?;
- ) => {
- register!(
- @leaf_accessor $name $hi:$lo $field
- { |f| <$into_type>::from(if f != 0 { true } else { false }) }
- $into_type => $into_type $(, $comment)?;
- );
- };
-
- // Shortcut for fields defined as `bool` without the `=>` syntax.
- (
- @field_accessor $name:ident $hi:tt:$lo:tt $field:ident as bool $(, $comment:literal)?;
- ) => {
- register!(@field_accessor $name $hi:$lo $field as bool => bool $(, $comment)?;);
- };
-
- // Catches the `?=>` syntax for non-boolean fields.
- (
- @field_accessor $name:ident $hi:tt:$lo:tt $field:ident as $type:tt ?=> $try_into_type:ty
- $(, $comment:literal)?;
- ) => {
- register!(@leaf_accessor $name $hi:$lo $field
- { |f| <$try_into_type>::try_from(f as $type) } $try_into_type =>
- ::core::result::Result<
- $try_into_type,
- <$try_into_type as ::core::convert::TryFrom<$type>>::Error
- >
- $(, $comment)?;);
- };
-
- // Catches the `=>` syntax for non-boolean fields.
- (
- @field_accessor $name:ident $hi:tt:$lo:tt $field:ident as $type:tt => $into_type:ty
- $(, $comment:literal)?;
- ) => {
- register!(@leaf_accessor $name $hi:$lo $field
- { |f| <$into_type>::from(f as $type) } $into_type => $into_type $(, $comment)?;);
- };
-
- // Shortcut for non-boolean fields defined without the `=>` or `?=>` syntax.
- (
- @field_accessor $name:ident $hi:tt:$lo:tt $field:ident as $type:tt
- $(, $comment:literal)?;
- ) => {
- register!(@field_accessor $name $hi:$lo $field as $type => $type $(, $comment)?;);
- };
-
- // Generates the accessor methods for a single field.
- (
- @leaf_accessor $name:ident $hi:tt:$lo:tt $field:ident
- { $process:expr } $to_type:ty => $res_type:ty $(, $comment:literal)?;
- ) => {
- ::kernel::macros::paste!(
- const [<$field:upper _RANGE>]: ::core::ops::RangeInclusive<u8> = $lo..=$hi;
- const [<$field:upper _MASK>]: u32 = ((((1 << $hi) - 1) << 1) + 1) - ((1 << $lo) - 1);
- const [<$field:upper _SHIFT>]: u32 = Self::[<$field:upper _MASK>].trailing_zeros();
- );
-
- $(
- #[doc="Returns the value of this field:"]
- #[doc=$comment]
- )?
- #[inline(always)]
- pub(crate) fn $field(self) -> $res_type {
- ::kernel::macros::paste!(
- const MASK: u32 = $name::[<$field:upper _MASK>];
- const SHIFT: u32 = $name::[<$field:upper _SHIFT>];
- );
- let field = ((self.0 & MASK) >> SHIFT);
-
- $process(field)
- }
-
- ::kernel::macros::paste!(
- $(
- #[doc="Sets the value of this field:"]
- #[doc=$comment]
- )?
- #[inline(always)]
- pub(crate) fn [<set_ $field>](mut self, value: $to_type) -> Self {
- const MASK: u32 = $name::[<$field:upper _MASK>];
- const SHIFT: u32 = $name::[<$field:upper _SHIFT>];
- let value = (u32::from(value) << SHIFT) & MASK;
- self.0 = (self.0 & !MASK) | value;
-
- self
- }
- );
- };
-
- // Generates the `Debug` implementation for `$name`.
- (@debug $name:ident { $($field:ident;)* }) => {
- impl ::core::fmt::Debug for $name {
- fn fmt(&self, f: &mut ::core::fmt::Formatter<'_>) -> ::core::fmt::Result {
- f.debug_struct(stringify!($name))
- .field("<raw>", &format_args!("{:#x}", &self.0))
- $(
- .field(stringify!($field), &self.$field())
- )*
- .finish()
- }
- }
- };
-
- // Generates the `Default` implementation for `$name`.
- (@default $name:ident { $($field:ident;)* }) => {
- /// Returns a value for the register 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.[<set_ $field>](Default::default());
- )*
- );
-
- value
- }
- }
- };
-
// Generates the IO accessors for a fixed offset register.
(@io_fixed $name:ident @ $offset:expr) => {
#[allow(dead_code)]
--
2.34.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH v2 1/4] nova-core: bitstruct: Move bitfield-specific code from register! into new macro
2025-09-03 21:54 ` [PATCH v2 1/4] nova-core: bitstruct: Move bitfield-specific code from register! into new macro Joel Fernandes
@ 2025-09-08 3:12 ` Alexandre Courbot
2025-09-08 17:16 ` Joel Fernandes
0 siblings, 1 reply; 28+ messages in thread
From: Alexandre Courbot @ 2025-09-08 3:12 UTC (permalink / raw)
To: Joel Fernandes, linux-kernel, dri-devel, dakr
Cc: Alistair Popple, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
bjorn3_gh, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, David Airlie, Simona Vetter, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, John Hubbard, Timur Tabi, joel,
Elle Rhumsaa, Daniel Almeida, nouveau, rust-for-linux
On Thu Sep 4, 2025 at 6:54 AM JST, Joel Fernandes wrote:
> The bitfield-specific into new macro. This will be used to define
> structs with bitfields, similar to C language.
>
> Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
> ---
> drivers/gpu/nova-core/bitstruct.rs | 271 +++++++++++++++++++++++++++
> drivers/gpu/nova-core/nova_core.rs | 3 +
> drivers/gpu/nova-core/regs/macros.rs | 247 +-----------------------
> 3 files changed, 282 insertions(+), 239 deletions(-)
> create mode 100644 drivers/gpu/nova-core/bitstruct.rs
>
> diff --git a/drivers/gpu/nova-core/bitstruct.rs b/drivers/gpu/nova-core/bitstruct.rs
> new file mode 100644
> index 000000000000..1dd9edab7d07
> --- /dev/null
> +++ b/drivers/gpu/nova-core/bitstruct.rs
> @@ -0,0 +1,271 @@
> +// SPDX-License-Identifier: GPL-2.0
> +//
> +// bitstruct.rs — Bitfield library for Rust structures
> +//
> +// A library that provides support for defining bit fields in Rust
> +// structures. Also used from things that need bitfields like register macro.
> +///
> +/// # Syntax
> +///
> +/// ```rust
> +/// bitstruct! {
> +/// struct ControlReg {
The `struct` naming here looks a bit confusing to me - as of this patch,
this is a u32, right? And eventually these types will be limited to primitive types,
so why not just `ControlReg: u32 {` ?
> +/// 3:0 mode as u8 ?=> Mode;
> +/// 7:4 state as u8 => State;
> +/// }
> +/// }
> +/// ```
As this will move to the kernel crate, it is particularly important to
make sure that this example can compile and run - so please provide
simple definitions for `Mode` and `State` to make sure the kunit tests
will pass after patch 4 (in the current state I'm pretty sure they won't).
> +///
> +/// This generates a struct with:
> +/// - Field accessors: `mode()`, `state()`, etc.
> +/// - Field setters: `set_mode()`, `set_state()`, etc. (supports builder pattern).
> +/// - Debug and Default implementations
> +///
> +/// The field setters can be used with the builder pattern, example:
> +/// ControlReg::default().set_mode(mode).set_state(state);
> +///
> +/// Fields are defined as follows:
> +///
> +/// - `as <type>` simply returns the field value casted to <type>, typically `u32`, `u16`, `u8` or
> +/// `bool`. Note that `bool` fields must have a range of 1 bit.
> +/// - `as <type> => <into_type>` calls `<into_type>`'s `From::<<type>>` implementation and returns
> +/// the result.
> +/// - `as <type> ?=> <try_into_type>` calls `<try_into_type>`'s `TryFrom::<<type>>` implementation
> +/// and returns the result. This is useful with fields for which not all values are valid.
Can you remove the related documentation from `register!` and replace it
with a sentence like "Please look at the [`bitstruct`] macro for the
complete syntax of fields definitions"? This will ensure we don't have
to update the documentation twice if/when the syntax gets updated.
The rest of the patch is a perfect move (with a few renames) of the
internal rules from one macro to the other, which makes it really easy
to review. Thanks for doing this!
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v2 1/4] nova-core: bitstruct: Move bitfield-specific code from register! into new macro
2025-09-08 3:12 ` Alexandre Courbot
@ 2025-09-08 17:16 ` Joel Fernandes
2025-09-09 2:37 ` Alexandre Courbot
0 siblings, 1 reply; 28+ messages in thread
From: Joel Fernandes @ 2025-09-08 17:16 UTC (permalink / raw)
To: Alexandre Courbot, linux-kernel, dri-devel, dakr
Cc: Alistair Popple, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
bjorn3_gh, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, David Airlie, Simona Vetter, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, John Hubbard, Timur Tabi, joel,
Elle Rhumsaa, Daniel Almeida, nouveau, rust-for-linux
Hi Alex,
On 9/7/2025 11:12 PM, Alexandre Courbot wrote:
> On Thu Sep 4, 2025 at 6:54 AM JST, Joel Fernandes wrote:
>> The bitfield-specific into new macro. This will be used to define
>> structs with bitfields, similar to C language.
>>
>> Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
>> ---
>> drivers/gpu/nova-core/bitstruct.rs | 271 +++++++++++++++++++++++++++
>> drivers/gpu/nova-core/nova_core.rs | 3 +
>> drivers/gpu/nova-core/regs/macros.rs | 247 +-----------------------
>> 3 files changed, 282 insertions(+), 239 deletions(-)
>> create mode 100644 drivers/gpu/nova-core/bitstruct.rs
>>
>> diff --git a/drivers/gpu/nova-core/bitstruct.rs b/drivers/gpu/nova-core/bitstruct.rs
>> new file mode 100644
>> index 000000000000..1dd9edab7d07
>> --- /dev/null
>> +++ b/drivers/gpu/nova-core/bitstruct.rs
>> @@ -0,0 +1,271 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +//
>> +// bitstruct.rs — Bitfield library for Rust structures
>> +//
>> +// A library that provides support for defining bit fields in Rust
>> +// structures. Also used from things that need bitfields like register macro.
>> +///
>> +/// # Syntax
>> +///
>> +/// ```rust
>> +/// bitstruct! {
>> +/// struct ControlReg {
>
> The `struct` naming here looks a bit confusing to me - as of this patch,
> this is a u32, right? And eventually these types will be limited to primitive types,
> so why not just `ControlReg: u32 {` ?
This is done in a later patch. This patch is only code movement, in later patch
we add precisely the syntax you're describing when we add storage types, and
update the register! macro. In this patch bitstruct is only u32.
>
>> +/// 3:0 mode as u8 ?=> Mode;
>> +/// 7:4 state as u8 => State;
>> +/// }
>> +/// }
>> +/// ```
>
> As this will move to the kernel crate, it is particularly important to
> make sure that this example can compile and run - so please provide
> simple definitions for `Mode` and `State` to make sure the kunit tests
> will pass after patch 4 (in the current state I'm pretty sure they won't).
Good catch. This will blow up the example though. I will change it to no_run
like the register! macro did if that's Ok.
>
>> +///
>> +/// This generates a struct with:
>> +/// - Field accessors: `mode()`, `state()`, etc.
>> +/// - Field setters: `set_mode()`, `set_state()`, etc. (supports builder pattern).
>> +/// - Debug and Default implementations
>> +///
>> +/// The field setters can be used with the builder pattern, example:
>> +/// ControlReg::default().set_mode(mode).set_state(state);
>> +///
>> +/// Fields are defined as follows:
>> +///
>> +/// - `as <type>` simply returns the field value casted to <type>, typically `u32`, `u16`, `u8` or
>> +/// `bool`. Note that `bool` fields must have a range of 1 bit.
>> +/// - `as <type> => <into_type>` calls `<into_type>`'s `From::<<type>>` implementation and returns
>> +/// the result.
>> +/// - `as <type> ?=> <try_into_type>` calls `<try_into_type>`'s `TryFrom::<<type>>` implementation
>> +/// and returns the result. This is useful with fields for which not all values are valid.
>
> Can you remove the related documentation from `register!` and replace it
> with a sentence like "Please look at the [`bitstruct`] macro for the
> complete syntax of fields definitions"? This will ensure we don't have
> to update the documentation twice if/when the syntax gets updated.
Sure!
>
> The rest of the patch is a perfect move (with a few renames) of the
> internal rules from one macro to the other, which makes it really easy
> to review. Thanks for doing this!
My pleasure, thanks for the suggestion!
- Joel
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v2 1/4] nova-core: bitstruct: Move bitfield-specific code from register! into new macro
2025-09-08 17:16 ` Joel Fernandes
@ 2025-09-09 2:37 ` Alexandre Courbot
2025-09-09 18:55 ` Joel Fernandes
0 siblings, 1 reply; 28+ messages in thread
From: Alexandre Courbot @ 2025-09-09 2:37 UTC (permalink / raw)
To: Joel Fernandes, linux-kernel, dri-devel, dakr
Cc: Alistair Popple, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
bjorn3_gh, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, David Airlie, Simona Vetter, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, John Hubbard, Timur Tabi, joel,
Elle Rhumsaa, Daniel Almeida, nouveau, rust-for-linux
On Tue Sep 9, 2025 at 2:16 AM JST, Joel Fernandes wrote:
> Hi Alex,
>
> On 9/7/2025 11:12 PM, Alexandre Courbot wrote:
>> On Thu Sep 4, 2025 at 6:54 AM JST, Joel Fernandes wrote:
>>> The bitfield-specific into new macro. This will be used to define
>>> structs with bitfields, similar to C language.
>>>
>>> Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
>>> ---
>>> drivers/gpu/nova-core/bitstruct.rs | 271 +++++++++++++++++++++++++++
>>> drivers/gpu/nova-core/nova_core.rs | 3 +
>>> drivers/gpu/nova-core/regs/macros.rs | 247 +-----------------------
>>> 3 files changed, 282 insertions(+), 239 deletions(-)
>>> create mode 100644 drivers/gpu/nova-core/bitstruct.rs
>>>
>>> diff --git a/drivers/gpu/nova-core/bitstruct.rs b/drivers/gpu/nova-core/bitstruct.rs
>>> new file mode 100644
>>> index 000000000000..1dd9edab7d07
>>> --- /dev/null
>>> +++ b/drivers/gpu/nova-core/bitstruct.rs
>>> @@ -0,0 +1,271 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>> +//
>>> +// bitstruct.rs — Bitfield library for Rust structures
>>> +//
>>> +// A library that provides support for defining bit fields in Rust
>>> +// structures. Also used from things that need bitfields like register macro.
>>> +///
>>> +/// # Syntax
>>> +///
>>> +/// ```rust
>>> +/// bitstruct! {
>>> +/// struct ControlReg {
>>
>> The `struct` naming here looks a bit confusing to me - as of this patch,
>> this is a u32, right? And eventually these types will be limited to primitive types,
>> so why not just `ControlReg: u32 {` ?
>
> This is done in a later patch. This patch is only code movement, in later patch
> we add precisely the syntax you're describing when we add storage types, and
> update the register! macro. In this patch bitstruct is only u32.
My point was, is the `struct` keyword needed at all? Isn't it a bit
confusing since these types are technically not Rust structs?
I agree the `: u32` can be introduced later, the original `register!`
macro did not specify any type information so there is indeed no reason
to add it in this patch.
>
>>
>>> +/// 3:0 mode as u8 ?=> Mode;
>>> +/// 7:4 state as u8 => State;
>>> +/// }
>>> +/// }
>>> +/// ```
>>
>> As this will move to the kernel crate, it is particularly important to
>> make sure that this example can compile and run - so please provide
>> simple definitions for `Mode` and `State` to make sure the kunit tests
>> will pass after patch 4 (in the current state I'm pretty sure they won't).
>
> Good catch. This will blow up the example though. I will change it to no_run
> like the register! macro did if that's Ok.
If you reduce `State` to 1 bit and change its type to `bool`, and limit
`Mode` to two or three variants, the example should remain short. I
think it is valuable to provide a complete working example here as the
syntax is not obvious at first sight.
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v2 1/4] nova-core: bitstruct: Move bitfield-specific code from register! into new macro
2025-09-09 2:37 ` Alexandre Courbot
@ 2025-09-09 18:55 ` Joel Fernandes
0 siblings, 0 replies; 28+ messages in thread
From: Joel Fernandes @ 2025-09-09 18:55 UTC (permalink / raw)
To: Alexandre Courbot
Cc: linux-kernel, dri-devel, dakr, Alistair Popple, Miguel Ojeda,
Alex Gaynor, Boqun Feng, Gary Guo, bjorn3_gh, Benno Lossin,
Andreas Hindborg, Alice Ryhl, Trevor Gross, David Airlie,
Simona Vetter, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, John Hubbard, Timur Tabi, joel, Elle Rhumsaa,
Daniel Almeida, nouveau, rust-for-linux
On Tue, Sep 09, 2025 at 11:37:15AM +0900, Alexandre Courbot wrote:
> On Tue Sep 9, 2025 at 2:16 AM JST, Joel Fernandes wrote:
> > Hi Alex,
> >
> > On 9/7/2025 11:12 PM, Alexandre Courbot wrote:
> >> On Thu Sep 4, 2025 at 6:54 AM JST, Joel Fernandes wrote:
> >>> The bitfield-specific into new macro. This will be used to define
> >>> structs with bitfields, similar to C language.
> >>>
> >>> Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
> >>> ---
> >>> drivers/gpu/nova-core/bitstruct.rs | 271 +++++++++++++++++++++++++++
> >>> drivers/gpu/nova-core/nova_core.rs | 3 +
> >>> drivers/gpu/nova-core/regs/macros.rs | 247 +-----------------------
> >>> 3 files changed, 282 insertions(+), 239 deletions(-)
> >>> create mode 100644 drivers/gpu/nova-core/bitstruct.rs
> >>>
> >>> diff --git a/drivers/gpu/nova-core/bitstruct.rs b/drivers/gpu/nova-core/bitstruct.rs
> >>> new file mode 100644
> >>> index 000000000000..1dd9edab7d07
> >>> --- /dev/null
> >>> +++ b/drivers/gpu/nova-core/bitstruct.rs
> >>> @@ -0,0 +1,271 @@
> >>> +// SPDX-License-Identifier: GPL-2.0
> >>> +//
> >>> +// bitstruct.rs — Bitfield library for Rust structures
> >>> +//
> >>> +// A library that provides support for defining bit fields in Rust
> >>> +// structures. Also used from things that need bitfields like register macro.
> >>> +///
> >>> +/// # Syntax
> >>> +///
> >>> +/// ```rust
> >>> +/// bitstruct! {
> >>> +/// struct ControlReg {
> >>
> >> The `struct` naming here looks a bit confusing to me - as of this patch,
> >> this is a u32, right? And eventually these types will be limited to primitive types,
> >> so why not just `ControlReg: u32 {` ?
> >
> > This is done in a later patch. This patch is only code movement, in later patch
> > we add precisely the syntax you're describing when we add storage types, and
> > update the register! macro. In this patch bitstruct is only u32.
>
> My point was, is the `struct` keyword needed at all? Isn't it a bit
> confusing since these types are technically not Rust structs?
Now that bitstruct has changed to bitfield, I would really insist on leaving
'struct' in there.
So it will look like this:
//! bitfield! {
//! struct ControlReg {
//! 3:0 mode as u8 ?=> Mode;
//! 7 state as bool => State;
//! }
//! }
Sounds reasonable?
> I agree the `: u32` can be introduced later, the original `register!`
> macro did not specify any type information so there is indeed no reason
> to add it in this patch.
Yep.
> >
> >>
> >>> +/// 3:0 mode as u8 ?=> Mode;
> >>> +/// 7:4 state as u8 => State;
> >>> +/// }
> >>> +/// }
> >>> +/// ```
> >>
> >> As this will move to the kernel crate, it is particularly important to
> >> make sure that this example can compile and run - so please provide
> >> simple definitions for `Mode` and `State` to make sure the kunit tests
> >> will pass after patch 4 (in the current state I'm pretty sure they won't).
> >
> > Good catch. This will blow up the example though. I will change it to no_run
> > like the register! macro did if that's Ok.
>
> If you reduce `State` to 1 bit and change its type to `bool`, and limit
> `Mode` to two or three variants, the example should remain short. I
> think it is valuable to provide a complete working example here as the
> syntax is not obvious at first sight.
Ok, so it will look like this, still about 40 lines more, but that works for me.
@@ -7,11 +7,54 @@
//!
//! # Syntax
//!
-//! ```no_run
+//! ```rust
+//! #[derive(Debug, Clone, Copy)]
+//! enum Mode {
+//! Low = 0,
+//! High = 1,
+//! Auto = 2,
+//! }
+//!
+//! impl TryFrom<u8> for Mode {
+//! type Error = u8;
+//! fn try_from(value: u8) -> Result<Self, Self::Error> {
+//! match value {
+//! 0 => Ok(Mode::Low),
+//! 1 => Ok(Mode::High),
+//! 2 => Ok(Mode::Auto),
+//! _ => Err(value),
+//! }
+//! }
+//! }
+//!
+//! impl From<Mode> for u32 {
+//! fn from(mode: Mode) -> u32 {
+//! mode as u32
+//! }
+//! }
+//!
+//! #[derive(Debug, Clone, Copy)]
+//! enum State {
+//! Inactive = 0,
+//! Active = 1,
+//! }
+//!
+//! impl From<bool> for State {
+//! fn from(value: bool) -> Self {
+//! if value { State::Active } else { State::Inactive }
+//! }
+//! }
+//!
+//! impl From<State> for u32 {
+//! fn from(state: State) -> u32 {
+//! state as u32
+//! }
+//! }
+//!
//! bitfield! {
//! struct ControlReg {
//! 3:0 mode as u8 ?=> Mode;
-//! 7:4 state as u8 => State;
+//! 7 state as bool => State;
//! }
//! }
//! ```
thanks,
- Joel
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v2 2/4] nova-core: bitstruct: Add support for different storage widths
2025-09-03 21:54 [PATCH v2 0/4] Improve bitfield support in Rust Joel Fernandes
2025-09-03 21:54 ` [PATCH v2 1/4] nova-core: bitstruct: Move bitfield-specific code from register! into new macro Joel Fernandes
@ 2025-09-03 21:54 ` Joel Fernandes
2025-09-05 22:21 ` Elle Rhumsaa
2025-09-08 3:26 ` Alexandre Courbot
2025-09-03 21:54 ` [PATCH v2 3/4] nova-core: bitstruct: Add support for custom visiblity Joel Fernandes
2025-09-03 21:54 ` [PATCH v2 4/4] rust: Move register and bitstruct macros out of Nova Joel Fernandes
3 siblings, 2 replies; 28+ messages in thread
From: Joel Fernandes @ 2025-09-03 21:54 UTC (permalink / raw)
To: linux-kernel, dri-devel, dakr, acourbot
Cc: Alistair Popple, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
bjorn3_gh, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, David Airlie, Simona Vetter, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, John Hubbard, Joel Fernandes,
Timur Tabi, joel, Elle Rhumsaa, Daniel Almeida, nouveau,
rust-for-linux
Previously, bitstructs were hardcoded to use u32 as the underlying
storage type. Add support for different storage types (u8, u16, u32,
u64) to the bitstruct macro.
New syntax is: struct Name: <type ex., u32> { ... }
Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
---
drivers/gpu/nova-core/bitstruct.rs | 71 ++++++++++++++++------------
drivers/gpu/nova-core/regs/macros.rs | 16 +++----
2 files changed, 48 insertions(+), 39 deletions(-)
diff --git a/drivers/gpu/nova-core/bitstruct.rs b/drivers/gpu/nova-core/bitstruct.rs
index 1dd9edab7d07..068334c86981 100644
--- a/drivers/gpu/nova-core/bitstruct.rs
+++ b/drivers/gpu/nova-core/bitstruct.rs
@@ -9,7 +9,7 @@
///
/// ```rust
/// bitstruct! {
-/// struct ControlReg {
+/// struct ControlReg: u32 {
/// 3:0 mode as u8 ?=> Mode;
/// 7:4 state as u8 => State;
/// }
@@ -34,21 +34,21 @@
/// and returns the result. This is useful with fields for which not all values are valid.
macro_rules! bitstruct {
// Main entry point - defines the bitfield struct with fields
- (struct $name:ident $(, $comment:literal)? { $($fields:tt)* }) => {
- bitstruct!(@core $name $(, $comment)? { $($fields)* });
+ (struct $name:ident : $storage:ty $(, $comment:literal)? { $($fields:tt)* }) => {
+ bitstruct!(@core $name $storage $(, $comment)? { $($fields)* });
};
// All rules below are helpers.
// Defines the wrapper `$name` type, as well as its relevant implementations (`Debug`,
// `Default`, `BitOr`, and conversion to the value type) and field accessor methods.
- (@core $name:ident $(, $comment:literal)? { $($fields:tt)* }) => {
+ (@core $name:ident $storage:ty $(, $comment:literal)? { $($fields:tt)* }) => {
$(
#[doc=$comment]
)?
#[repr(transparent)]
#[derive(Clone, Copy)]
- pub(crate) struct $name(u32);
+ pub(crate) struct $name($storage);
impl ::core::ops::BitOr for $name {
type Output = Self;
@@ -58,20 +58,20 @@ fn bitor(self, rhs: Self) -> Self::Output {
}
}
- impl ::core::convert::From<$name> for u32 {
- fn from(val: $name) -> u32 {
+ impl ::core::convert::From<$name> for $storage {
+ fn from(val: $name) -> $storage {
val.0
}
}
- bitstruct!(@fields_dispatcher $name { $($fields)* });
+ bitstruct!(@fields_dispatcher $name $storage { $($fields)* });
};
// Captures the fields and passes them to all the implementers that require field information.
//
// Used to simplify the matching rules for implementers, so they don't need to match the entire
// complex fields rule even though they only make use of part of it.
- (@fields_dispatcher $name:ident {
+ (@fields_dispatcher $name:ident $storage:ty {
$($hi:tt:$lo:tt $field:ident as $type:tt
$(?=> $try_into_type:ty)?
$(=> $into_type:ty)?
@@ -80,7 +80,7 @@ fn from(val: $name) -> u32 {
)*
}
) => {
- bitstruct!(@field_accessors $name {
+ bitstruct!(@field_accessors $name $storage {
$(
$hi:$lo $field as $type
$(?=> $try_into_type)?
@@ -89,13 +89,13 @@ fn from(val: $name) -> u32 {
;
)*
});
- bitstruct!(@debug $name { $($field;)* });
- bitstruct!(@default $name { $($field;)* });
+ bitstruct!(@debug $name $storage { $($field;)* });
+ bitstruct!(@default $name $storage { $($field;)* });
};
// Defines all the field getter/setter methods for `$name`.
(
- @field_accessors $name:ident {
+ @field_accessors $name:ident $storage:ty {
$($hi:tt:$lo:tt $field:ident as $type:tt
$(?=> $try_into_type:ty)?
$(=> $into_type:ty)?
@@ -111,7 +111,7 @@ fn from(val: $name) -> u32 {
#[allow(dead_code)]
impl $name {
$(
- bitstruct!(@field_accessor $name $hi:$lo $field as $type
+ bitstruct!(@field_accessor $name $storage, $hi:$lo $field as $type
$(?=> $try_into_type)?
$(=> $into_type)?
$(, $comment)?
@@ -145,11 +145,11 @@ impl $name {
// Catches fields defined as `bool` and convert them into a boolean value.
(
- @field_accessor $name:ident $hi:tt:$lo:tt $field:ident as bool => $into_type:ty
+ @field_accessor $name:ident $storage:ty, $hi:tt:$lo:tt $field:ident as bool => $into_type:ty
$(, $comment:literal)?;
) => {
bitstruct!(
- @leaf_accessor $name $hi:$lo $field
+ @leaf_accessor $name $storage, $hi:$lo $field
{ |f| <$into_type>::from(if f != 0 { true } else { false }) }
$into_type => $into_type $(, $comment)?;
);
@@ -157,17 +157,17 @@ impl $name {
// Shortcut for fields defined as `bool` without the `=>` syntax.
(
- @field_accessor $name:ident $hi:tt:$lo:tt $field:ident as bool $(, $comment:literal)?;
+ @field_accessor $name:ident $storage:ty, $hi:tt:$lo:tt $field:ident as bool $(, $comment:literal)?;
) => {
- bitstruct!(@field_accessor $name $hi:$lo $field as bool => bool $(, $comment)?;);
+ bitstruct!(@field_accessor $name $storage, $hi:$lo $field as bool => bool $(, $comment)?;);
};
// Catches the `?=>` syntax for non-boolean fields.
(
- @field_accessor $name:ident $hi:tt:$lo:tt $field:ident as $type:tt ?=> $try_into_type:ty
+ @field_accessor $name:ident $storage:ty, $hi:tt:$lo:tt $field:ident as $type:tt ?=> $try_into_type:ty
$(, $comment:literal)?;
) => {
- bitstruct!(@leaf_accessor $name $hi:$lo $field
+ bitstruct!(@leaf_accessor $name $storage, $hi:$lo $field
{ |f| <$try_into_type>::try_from(f as $type) } $try_into_type =>
::core::result::Result<
$try_into_type,
@@ -178,29 +178,38 @@ impl $name {
// Catches the `=>` syntax for non-boolean fields.
(
- @field_accessor $name:ident $hi:tt:$lo:tt $field:ident as $type:tt => $into_type:ty
+ @field_accessor $name:ident $storage:ty, $hi:tt:$lo:tt $field:ident as $type:tt => $into_type:ty
$(, $comment:literal)?;
) => {
- bitstruct!(@leaf_accessor $name $hi:$lo $field
+ bitstruct!(@leaf_accessor $name $storage, $hi:$lo $field
{ |f| <$into_type>::from(f as $type) } $into_type => $into_type $(, $comment)?;);
};
// Shortcut for non-boolean fields defined without the `=>` or `?=>` syntax.
(
- @field_accessor $name:ident $hi:tt:$lo:tt $field:ident as $type:tt
+ @field_accessor $name:ident $storage:ty, $hi:tt:$lo:tt $field:ident as $type:tt
$(, $comment:literal)?;
) => {
- bitstruct!(@field_accessor $name $hi:$lo $field as $type => $type $(, $comment)?;);
+ bitstruct!(@field_accessor $name $storage, $hi:$lo $field as $type => $type $(, $comment)?;);
};
// Generates the accessor methods for a single field.
(
- @leaf_accessor $name:ident $hi:tt:$lo:tt $field:ident
+ @leaf_accessor $name:ident $storage:ty, $hi:tt:$lo:tt $field:ident
{ $process:expr } $to_type:ty => $res_type:ty $(, $comment:literal)?;
) => {
::kernel::macros::paste!(
const [<$field:upper _RANGE>]: ::core::ops::RangeInclusive<u8> = $lo..=$hi;
- const [<$field:upper _MASK>]: u32 = ((((1 << $hi) - 1) << 1) + 1) - ((1 << $lo) - 1);
+ const [<$field:upper _MASK>]: $storage = {
+ // Generate mask for shifting
+ match ::core::mem::size_of::<$storage>() {
+ 1 => ::kernel::bits::genmask_u8($lo..=$hi) as $storage,
+ 2 => ::kernel::bits::genmask_u16($lo..=$hi) as $storage,
+ 4 => ::kernel::bits::genmask_u32($lo..=$hi) as $storage,
+ 8 => ::kernel::bits::genmask_u64($lo..=$hi) as $storage,
+ _ => <$storage>::MAX
+ }
+ };
const [<$field:upper _SHIFT>]: u32 = Self::[<$field:upper _MASK>].trailing_zeros();
);
@@ -211,7 +220,7 @@ impl $name {
#[inline(always)]
pub(crate) fn $field(self) -> $res_type {
::kernel::macros::paste!(
- const MASK: u32 = $name::[<$field:upper _MASK>];
+ const MASK: $storage = $name::[<$field:upper _MASK>];
const SHIFT: u32 = $name::[<$field:upper _SHIFT>];
);
let field = ((self.0 & MASK) >> SHIFT);
@@ -226,9 +235,9 @@ pub(crate) fn $field(self) -> $res_type {
)?
#[inline(always)]
pub(crate) fn [<set_ $field>](mut self, value: $to_type) -> Self {
- const MASK: u32 = $name::[<$field:upper _MASK>];
+ const MASK: $storage = $name::[<$field:upper _MASK>];
const SHIFT: u32 = $name::[<$field:upper _SHIFT>];
- let value = (u32::from(value) << SHIFT) & MASK;
+ let value = (<$storage>::from(value) << SHIFT) & MASK;
self.0 = (self.0 & !MASK) | value;
self
@@ -237,7 +246,7 @@ pub(crate) fn [<set_ $field>](mut self, value: $to_type) -> Self {
};
// Generates the `Debug` implementation for `$name`.
- (@debug $name:ident { $($field:ident;)* }) => {
+ (@debug $name:ident $storage:ty { $($field:ident;)* }) => {
impl ::core::fmt::Debug for $name {
fn fmt(&self, f: &mut ::core::fmt::Formatter<'_>) -> ::core::fmt::Result {
f.debug_struct(stringify!($name))
@@ -251,7 +260,7 @@ fn fmt(&self, f: &mut ::core::fmt::Formatter<'_>) -> ::core::fmt::Result {
};
// Generates the `Default` implementation for `$name`.
- (@default $name:ident { $($field:ident;)* }) => {
+ (@default $name:ident $storage:ty { $($field:ident;)* }) => {
/// Returns a value for the bitstruct where all fields are set to their default value.
impl ::core::default::Default for $name {
fn default() -> Self {
diff --git a/drivers/gpu/nova-core/regs/macros.rs b/drivers/gpu/nova-core/regs/macros.rs
index 3fb6852dff04..bbfeab147c9f 100644
--- a/drivers/gpu/nova-core/regs/macros.rs
+++ b/drivers/gpu/nova-core/regs/macros.rs
@@ -284,25 +284,25 @@ pub(crate) trait RegisterBase<T> {
macro_rules! register {
// Creates a register at a fixed offset of the MMIO space.
($name:ident @ $offset:literal $(, $comment:literal)? { $($fields:tt)* } ) => {
- bitstruct!(struct $name $(, $comment)? { $($fields)* } );
+ bitstruct!(struct $name: u32 $(, $comment)? { $($fields)* } );
register!(@io_fixed $name @ $offset);
};
// Creates an alias register of fixed offset register `alias` with its own fields.
($name:ident => $alias:ident $(, $comment:literal)? { $($fields:tt)* } ) => {
- bitstruct!(struct $name $(, $comment)? { $($fields)* } );
+ bitstruct!(struct $name: u32 $(, $comment)? { $($fields)* } );
register!(@io_fixed $name @ $alias::OFFSET);
};
// Creates a register at a relative offset from a base address provider.
($name:ident @ $base:ty [ $offset:literal ] $(, $comment:literal)? { $($fields:tt)* } ) => {
- bitstruct!(struct $name $(, $comment)? { $($fields)* } );
+ bitstruct!(struct $name: u32 $(, $comment)? { $($fields)* } );
register!(@io_relative $name @ $base [ $offset ]);
};
// Creates an alias register of relative offset register `alias` with its own fields.
($name:ident => $base:ty [ $alias:ident ] $(, $comment:literal)? { $($fields:tt)* }) => {
- bitstruct!(struct $name $(, $comment)? { $($fields)* } );
+ bitstruct!(struct $name: u32 $(, $comment)? { $($fields)* } );
register!(@io_relative $name @ $base [ $alias::OFFSET ]);
};
@@ -313,7 +313,7 @@ macro_rules! register {
}
) => {
static_assert!(::core::mem::size_of::<u32>() <= $stride);
- bitstruct!(struct $name $(, $comment)? { $($fields)* } );
+ bitstruct!(struct $name: u32 $(, $comment)? { $($fields)* } );
register!(@io_array $name @ $offset [ $size ; $stride ]);
};
@@ -334,7 +334,7 @@ macro_rules! register {
$(, $comment:literal)? { $($fields:tt)* }
) => {
static_assert!(::core::mem::size_of::<u32>() <= $stride);
- bitstruct!(struct $name $(, $comment)? { $($fields)* } );
+ bitstruct!(struct $name: u32 $(, $comment)? { $($fields)* } );
register!(@io_relative_array $name @ $base [ $offset [ $size ; $stride ] ]);
};
@@ -356,7 +356,7 @@ macro_rules! register {
}
) => {
static_assert!($idx < $alias::SIZE);
- bitstruct!(struct $name $(, $comment)? { $($fields)* } );
+ bitstruct!(struct $name: u32 $(, $comment)? { $($fields)* } );
register!(@io_relative $name @ $base [ $alias::OFFSET + $idx * $alias::STRIDE ] );
};
@@ -365,7 +365,7 @@ macro_rules! register {
// to avoid it being interpreted in place of the relative register array alias rule.
($name:ident => $alias:ident [ $idx:expr ] $(, $comment:literal)? { $($fields:tt)* }) => {
static_assert!($idx < $alias::SIZE);
- bitstruct!(struct $name $(, $comment)? { $($fields)* } );
+ bitstruct!(struct $name: u32 $(, $comment)? { $($fields)* } );
register!(@io_fixed $name @ $alias::OFFSET + $idx * $alias::STRIDE );
};
--
2.34.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH v2 2/4] nova-core: bitstruct: Add support for different storage widths
2025-09-03 21:54 ` [PATCH v2 2/4] nova-core: bitstruct: Add support for different storage widths Joel Fernandes
@ 2025-09-05 22:21 ` Elle Rhumsaa
2025-09-08 3:26 ` Alexandre Courbot
1 sibling, 0 replies; 28+ messages in thread
From: Elle Rhumsaa @ 2025-09-05 22:21 UTC (permalink / raw)
To: Joel Fernandes
Cc: linux-kernel, dri-devel, dakr, acourbot, Alistair Popple,
Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo, bjorn3_gh,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
David Airlie, Simona Vetter, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, John Hubbard, Timur Tabi, joel, Daniel Almeida,
nouveau, rust-for-linux
On Wed, Sep 03, 2025 at 05:54:26PM -0400, Joel Fernandes wrote:
> Previously, bitstructs were hardcoded to use u32 as the underlying
> storage type. Add support for different storage types (u8, u16, u32,
> u64) to the bitstruct macro.
>
> New syntax is: struct Name: <type ex., u32> { ... }
>
> Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
> ---
> drivers/gpu/nova-core/bitstruct.rs | 71 ++++++++++++++++------------
> drivers/gpu/nova-core/regs/macros.rs | 16 +++----
> 2 files changed, 48 insertions(+), 39 deletions(-)
>
> diff --git a/drivers/gpu/nova-core/bitstruct.rs b/drivers/gpu/nova-core/bitstruct.rs
> index 1dd9edab7d07..068334c86981 100644
> --- a/drivers/gpu/nova-core/bitstruct.rs
> +++ b/drivers/gpu/nova-core/bitstruct.rs
> @@ -9,7 +9,7 @@
> ///
> /// ```rust
> /// bitstruct! {
> -/// struct ControlReg {
> +/// struct ControlReg: u32 {
> /// 3:0 mode as u8 ?=> Mode;
> /// 7:4 state as u8 => State;
> /// }
> @@ -34,21 +34,21 @@
> /// and returns the result. This is useful with fields for which not all values are valid.
> macro_rules! bitstruct {
> // Main entry point - defines the bitfield struct with fields
> - (struct $name:ident $(, $comment:literal)? { $($fields:tt)* }) => {
> - bitstruct!(@core $name $(, $comment)? { $($fields)* });
> + (struct $name:ident : $storage:ty $(, $comment:literal)? { $($fields:tt)* }) => {
> + bitstruct!(@core $name $storage $(, $comment)? { $($fields)* });
> };
>
> // All rules below are helpers.
>
> // Defines the wrapper `$name` type, as well as its relevant implementations (`Debug`,
> // `Default`, `BitOr`, and conversion to the value type) and field accessor methods.
> - (@core $name:ident $(, $comment:literal)? { $($fields:tt)* }) => {
> + (@core $name:ident $storage:ty $(, $comment:literal)? { $($fields:tt)* }) => {
> $(
> #[doc=$comment]
> )?
> #[repr(transparent)]
> #[derive(Clone, Copy)]
> - pub(crate) struct $name(u32);
> + pub(crate) struct $name($storage);
>
> impl ::core::ops::BitOr for $name {
> type Output = Self;
> @@ -58,20 +58,20 @@ fn bitor(self, rhs: Self) -> Self::Output {
> }
> }
>
> - impl ::core::convert::From<$name> for u32 {
> - fn from(val: $name) -> u32 {
> + impl ::core::convert::From<$name> for $storage {
> + fn from(val: $name) -> $storage {
> val.0
> }
> }
>
> - bitstruct!(@fields_dispatcher $name { $($fields)* });
> + bitstruct!(@fields_dispatcher $name $storage { $($fields)* });
> };
>
> // Captures the fields and passes them to all the implementers that require field information.
> //
> // Used to simplify the matching rules for implementers, so they don't need to match the entire
> // complex fields rule even though they only make use of part of it.
> - (@fields_dispatcher $name:ident {
> + (@fields_dispatcher $name:ident $storage:ty {
> $($hi:tt:$lo:tt $field:ident as $type:tt
> $(?=> $try_into_type:ty)?
> $(=> $into_type:ty)?
> @@ -80,7 +80,7 @@ fn from(val: $name) -> u32 {
> )*
> }
> ) => {
> - bitstruct!(@field_accessors $name {
> + bitstruct!(@field_accessors $name $storage {
> $(
> $hi:$lo $field as $type
> $(?=> $try_into_type)?
> @@ -89,13 +89,13 @@ fn from(val: $name) -> u32 {
> ;
> )*
> });
> - bitstruct!(@debug $name { $($field;)* });
> - bitstruct!(@default $name { $($field;)* });
> + bitstruct!(@debug $name $storage { $($field;)* });
> + bitstruct!(@default $name $storage { $($field;)* });
> };
>
> // Defines all the field getter/setter methods for `$name`.
> (
> - @field_accessors $name:ident {
> + @field_accessors $name:ident $storage:ty {
> $($hi:tt:$lo:tt $field:ident as $type:tt
> $(?=> $try_into_type:ty)?
> $(=> $into_type:ty)?
> @@ -111,7 +111,7 @@ fn from(val: $name) -> u32 {
> #[allow(dead_code)]
> impl $name {
> $(
> - bitstruct!(@field_accessor $name $hi:$lo $field as $type
> + bitstruct!(@field_accessor $name $storage, $hi:$lo $field as $type
> $(?=> $try_into_type)?
> $(=> $into_type)?
> $(, $comment)?
> @@ -145,11 +145,11 @@ impl $name {
>
> // Catches fields defined as `bool` and convert them into a boolean value.
> (
> - @field_accessor $name:ident $hi:tt:$lo:tt $field:ident as bool => $into_type:ty
> + @field_accessor $name:ident $storage:ty, $hi:tt:$lo:tt $field:ident as bool => $into_type:ty
> $(, $comment:literal)?;
> ) => {
> bitstruct!(
> - @leaf_accessor $name $hi:$lo $field
> + @leaf_accessor $name $storage, $hi:$lo $field
> { |f| <$into_type>::from(if f != 0 { true } else { false }) }
> $into_type => $into_type $(, $comment)?;
> );
> @@ -157,17 +157,17 @@ impl $name {
>
> // Shortcut for fields defined as `bool` without the `=>` syntax.
> (
> - @field_accessor $name:ident $hi:tt:$lo:tt $field:ident as bool $(, $comment:literal)?;
> + @field_accessor $name:ident $storage:ty, $hi:tt:$lo:tt $field:ident as bool $(, $comment:literal)?;
> ) => {
> - bitstruct!(@field_accessor $name $hi:$lo $field as bool => bool $(, $comment)?;);
> + bitstruct!(@field_accessor $name $storage, $hi:$lo $field as bool => bool $(, $comment)?;);
> };
>
> // Catches the `?=>` syntax for non-boolean fields.
> (
> - @field_accessor $name:ident $hi:tt:$lo:tt $field:ident as $type:tt ?=> $try_into_type:ty
> + @field_accessor $name:ident $storage:ty, $hi:tt:$lo:tt $field:ident as $type:tt ?=> $try_into_type:ty
> $(, $comment:literal)?;
> ) => {
> - bitstruct!(@leaf_accessor $name $hi:$lo $field
> + bitstruct!(@leaf_accessor $name $storage, $hi:$lo $field
> { |f| <$try_into_type>::try_from(f as $type) } $try_into_type =>
> ::core::result::Result<
> $try_into_type,
> @@ -178,29 +178,38 @@ impl $name {
>
> // Catches the `=>` syntax for non-boolean fields.
> (
> - @field_accessor $name:ident $hi:tt:$lo:tt $field:ident as $type:tt => $into_type:ty
> + @field_accessor $name:ident $storage:ty, $hi:tt:$lo:tt $field:ident as $type:tt => $into_type:ty
> $(, $comment:literal)?;
> ) => {
> - bitstruct!(@leaf_accessor $name $hi:$lo $field
> + bitstruct!(@leaf_accessor $name $storage, $hi:$lo $field
> { |f| <$into_type>::from(f as $type) } $into_type => $into_type $(, $comment)?;);
> };
>
> // Shortcut for non-boolean fields defined without the `=>` or `?=>` syntax.
> (
> - @field_accessor $name:ident $hi:tt:$lo:tt $field:ident as $type:tt
> + @field_accessor $name:ident $storage:ty, $hi:tt:$lo:tt $field:ident as $type:tt
> $(, $comment:literal)?;
> ) => {
> - bitstruct!(@field_accessor $name $hi:$lo $field as $type => $type $(, $comment)?;);
> + bitstruct!(@field_accessor $name $storage, $hi:$lo $field as $type => $type $(, $comment)?;);
> };
>
> // Generates the accessor methods for a single field.
> (
> - @leaf_accessor $name:ident $hi:tt:$lo:tt $field:ident
> + @leaf_accessor $name:ident $storage:ty, $hi:tt:$lo:tt $field:ident
> { $process:expr } $to_type:ty => $res_type:ty $(, $comment:literal)?;
> ) => {
> ::kernel::macros::paste!(
> const [<$field:upper _RANGE>]: ::core::ops::RangeInclusive<u8> = $lo..=$hi;
> - const [<$field:upper _MASK>]: u32 = ((((1 << $hi) - 1) << 1) + 1) - ((1 << $lo) - 1);
> + const [<$field:upper _MASK>]: $storage = {
> + // Generate mask for shifting
> + match ::core::mem::size_of::<$storage>() {
> + 1 => ::kernel::bits::genmask_u8($lo..=$hi) as $storage,
> + 2 => ::kernel::bits::genmask_u16($lo..=$hi) as $storage,
> + 4 => ::kernel::bits::genmask_u32($lo..=$hi) as $storage,
> + 8 => ::kernel::bits::genmask_u64($lo..=$hi) as $storage,
Not really important for current users, but something to think about for
the future is fields that are represented using byte arrays (e.g. a
`u24` represented as `[u8; 3]`).
I think your approach here could be fairly easily modified to handle
those use-cases, just something to think about.
For the byte array storage types, you would also need to think about
endianness (MSB v. LSB).
> + _ => <$storage>::MAX
> + }
> + };
> const [<$field:upper _SHIFT>]: u32 = Self::[<$field:upper _MASK>].trailing_zeros();
> );
>
> @@ -211,7 +220,7 @@ impl $name {
> #[inline(always)]
> pub(crate) fn $field(self) -> $res_type {
> ::kernel::macros::paste!(
> - const MASK: u32 = $name::[<$field:upper _MASK>];
> + const MASK: $storage = $name::[<$field:upper _MASK>];
> const SHIFT: u32 = $name::[<$field:upper _SHIFT>];
> );
> let field = ((self.0 & MASK) >> SHIFT);
> @@ -226,9 +235,9 @@ pub(crate) fn $field(self) -> $res_type {
> )?
> #[inline(always)]
> pub(crate) fn [<set_ $field>](mut self, value: $to_type) -> Self {
> - const MASK: u32 = $name::[<$field:upper _MASK>];
> + const MASK: $storage = $name::[<$field:upper _MASK>];
> const SHIFT: u32 = $name::[<$field:upper _SHIFT>];
> - let value = (u32::from(value) << SHIFT) & MASK;
> + let value = (<$storage>::from(value) << SHIFT) & MASK;
> self.0 = (self.0 & !MASK) | value;
>
> self
> @@ -237,7 +246,7 @@ pub(crate) fn [<set_ $field>](mut self, value: $to_type) -> Self {
> };
>
> // Generates the `Debug` implementation for `$name`.
> - (@debug $name:ident { $($field:ident;)* }) => {
> + (@debug $name:ident $storage:ty { $($field:ident;)* }) => {
> impl ::core::fmt::Debug for $name {
> fn fmt(&self, f: &mut ::core::fmt::Formatter<'_>) -> ::core::fmt::Result {
> f.debug_struct(stringify!($name))
> @@ -251,7 +260,7 @@ fn fmt(&self, f: &mut ::core::fmt::Formatter<'_>) -> ::core::fmt::Result {
> };
>
> // Generates the `Default` implementation for `$name`.
> - (@default $name:ident { $($field:ident;)* }) => {
> + (@default $name:ident $storage:ty { $($field:ident;)* }) => {
> /// Returns a value for the bitstruct where all fields are set to their default value.
> impl ::core::default::Default for $name {
> fn default() -> Self {
> diff --git a/drivers/gpu/nova-core/regs/macros.rs b/drivers/gpu/nova-core/regs/macros.rs
> index 3fb6852dff04..bbfeab147c9f 100644
> --- a/drivers/gpu/nova-core/regs/macros.rs
> +++ b/drivers/gpu/nova-core/regs/macros.rs
> @@ -284,25 +284,25 @@ pub(crate) trait RegisterBase<T> {
> macro_rules! register {
> // Creates a register at a fixed offset of the MMIO space.
> ($name:ident @ $offset:literal $(, $comment:literal)? { $($fields:tt)* } ) => {
> - bitstruct!(struct $name $(, $comment)? { $($fields)* } );
> + bitstruct!(struct $name: u32 $(, $comment)? { $($fields)* } );
> register!(@io_fixed $name @ $offset);
> };
>
> // Creates an alias register of fixed offset register `alias` with its own fields.
> ($name:ident => $alias:ident $(, $comment:literal)? { $($fields:tt)* } ) => {
> - bitstruct!(struct $name $(, $comment)? { $($fields)* } );
> + bitstruct!(struct $name: u32 $(, $comment)? { $($fields)* } );
> register!(@io_fixed $name @ $alias::OFFSET);
> };
>
> // Creates a register at a relative offset from a base address provider.
> ($name:ident @ $base:ty [ $offset:literal ] $(, $comment:literal)? { $($fields:tt)* } ) => {
> - bitstruct!(struct $name $(, $comment)? { $($fields)* } );
> + bitstruct!(struct $name: u32 $(, $comment)? { $($fields)* } );
> register!(@io_relative $name @ $base [ $offset ]);
> };
>
> // Creates an alias register of relative offset register `alias` with its own fields.
> ($name:ident => $base:ty [ $alias:ident ] $(, $comment:literal)? { $($fields:tt)* }) => {
> - bitstruct!(struct $name $(, $comment)? { $($fields)* } );
> + bitstruct!(struct $name: u32 $(, $comment)? { $($fields)* } );
> register!(@io_relative $name @ $base [ $alias::OFFSET ]);
> };
>
> @@ -313,7 +313,7 @@ macro_rules! register {
> }
> ) => {
> static_assert!(::core::mem::size_of::<u32>() <= $stride);
> - bitstruct!(struct $name $(, $comment)? { $($fields)* } );
> + bitstruct!(struct $name: u32 $(, $comment)? { $($fields)* } );
> register!(@io_array $name @ $offset [ $size ; $stride ]);
> };
>
> @@ -334,7 +334,7 @@ macro_rules! register {
> $(, $comment:literal)? { $($fields:tt)* }
> ) => {
> static_assert!(::core::mem::size_of::<u32>() <= $stride);
> - bitstruct!(struct $name $(, $comment)? { $($fields)* } );
> + bitstruct!(struct $name: u32 $(, $comment)? { $($fields)* } );
> register!(@io_relative_array $name @ $base [ $offset [ $size ; $stride ] ]);
> };
>
> @@ -356,7 +356,7 @@ macro_rules! register {
> }
> ) => {
> static_assert!($idx < $alias::SIZE);
> - bitstruct!(struct $name $(, $comment)? { $($fields)* } );
> + bitstruct!(struct $name: u32 $(, $comment)? { $($fields)* } );
> register!(@io_relative $name @ $base [ $alias::OFFSET + $idx * $alias::STRIDE ] );
> };
>
> @@ -365,7 +365,7 @@ macro_rules! register {
> // to avoid it being interpreted in place of the relative register array alias rule.
> ($name:ident => $alias:ident [ $idx:expr ] $(, $comment:literal)? { $($fields:tt)* }) => {
> static_assert!($idx < $alias::SIZE);
> - bitstruct!(struct $name $(, $comment)? { $($fields)* } );
> + bitstruct!(struct $name: u32 $(, $comment)? { $($fields)* } );
> register!(@io_fixed $name @ $alias::OFFSET + $idx * $alias::STRIDE );
> };
>
> --
> 2.34.1
>
>
Reviewed-by: Elle Rhumsaa <elle@weathered-steel.dev>
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v2 2/4] nova-core: bitstruct: Add support for different storage widths
2025-09-03 21:54 ` [PATCH v2 2/4] nova-core: bitstruct: Add support for different storage widths Joel Fernandes
2025-09-05 22:21 ` Elle Rhumsaa
@ 2025-09-08 3:26 ` Alexandre Courbot
2025-09-09 18:26 ` Joel Fernandes
1 sibling, 1 reply; 28+ messages in thread
From: Alexandre Courbot @ 2025-09-08 3:26 UTC (permalink / raw)
To: Joel Fernandes, linux-kernel, dri-devel, dakr
Cc: Alistair Popple, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
bjorn3_gh, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, David Airlie, Simona Vetter, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, John Hubbard, Timur Tabi, joel,
Elle Rhumsaa, Daniel Almeida, nouveau, rust-for-linux
On Thu Sep 4, 2025 at 6:54 AM JST, Joel Fernandes wrote:
> Previously, bitstructs were hardcoded to use u32 as the underlying
> storage type. Add support for different storage types (u8, u16, u32,
> u64) to the bitstruct macro.
>
> New syntax is: struct Name: <type ex., u32> { ... }
>
> Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
<snip>
> // Generates the accessor methods for a single field.
> (
> - @leaf_accessor $name:ident $hi:tt:$lo:tt $field:ident
> + @leaf_accessor $name:ident $storage:ty, $hi:tt:$lo:tt $field:ident
> { $process:expr } $to_type:ty => $res_type:ty $(, $comment:literal)?;
> ) => {
> ::kernel::macros::paste!(
> const [<$field:upper _RANGE>]: ::core::ops::RangeInclusive<u8> = $lo..=$hi;
> - const [<$field:upper _MASK>]: u32 = ((((1 << $hi) - 1) << 1) + 1) - ((1 << $lo) - 1);
> + const [<$field:upper _MASK>]: $storage = {
> + // Generate mask for shifting
> + match ::core::mem::size_of::<$storage>() {
> + 1 => ::kernel::bits::genmask_u8($lo..=$hi) as $storage,
> + 2 => ::kernel::bits::genmask_u16($lo..=$hi) as $storage,
> + 4 => ::kernel::bits::genmask_u32($lo..=$hi) as $storage,
> + 8 => ::kernel::bits::genmask_u64($lo..=$hi) as $storage,
> + _ => <$storage>::MAX
Since this is a const expression, you can use `build_error!` to make
compilation fail in the unlikely event the `_` is taken due to bug in
the code.
> + }
> + };
> const [<$field:upper _SHIFT>]: u32 = Self::[<$field:upper _MASK>].trailing_zeros();
> );
>
> @@ -211,7 +220,7 @@ impl $name {
> #[inline(always)]
> pub(crate) fn $field(self) -> $res_type {
> ::kernel::macros::paste!(
> - const MASK: u32 = $name::[<$field:upper _MASK>];
> + const MASK: $storage = $name::[<$field:upper _MASK>];
> const SHIFT: u32 = $name::[<$field:upper _SHIFT>];
> );
> let field = ((self.0 & MASK) >> SHIFT);
> @@ -226,9 +235,9 @@ pub(crate) fn $field(self) -> $res_type {
> )?
> #[inline(always)]
> pub(crate) fn [<set_ $field>](mut self, value: $to_type) -> Self {
> - const MASK: u32 = $name::[<$field:upper _MASK>];
> + const MASK: $storage = $name::[<$field:upper _MASK>];
> const SHIFT: u32 = $name::[<$field:upper _SHIFT>];
> - let value = (u32::from(value) << SHIFT) & MASK;
> + let value = (<$storage>::from(value) << SHIFT) & MASK;
> self.0 = (self.0 & !MASK) | value;
>
> self
> @@ -237,7 +246,7 @@ pub(crate) fn [<set_ $field>](mut self, value: $to_type) -> Self {
> };
>
> // Generates the `Debug` implementation for `$name`.
> - (@debug $name:ident { $($field:ident;)* }) => {
> + (@debug $name:ident $storage:ty { $($field:ident;)* }) => {
This rule doesn't make use of the `$storage` argument.
> impl ::core::fmt::Debug for $name {
> fn fmt(&self, f: &mut ::core::fmt::Formatter<'_>) -> ::core::fmt::Result {
> f.debug_struct(stringify!($name))
> @@ -251,7 +260,7 @@ fn fmt(&self, f: &mut ::core::fmt::Formatter<'_>) -> ::core::fmt::Result {
> };
>
> // Generates the `Default` implementation for `$name`.
> - (@default $name:ident { $($field:ident;)* }) => {
> + (@default $name:ident $storage:ty { $($field:ident;)* }) => {
Neither does this one.
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v2 2/4] nova-core: bitstruct: Add support for different storage widths
2025-09-08 3:26 ` Alexandre Courbot
@ 2025-09-09 18:26 ` Joel Fernandes
0 siblings, 0 replies; 28+ messages in thread
From: Joel Fernandes @ 2025-09-09 18:26 UTC (permalink / raw)
To: Alexandre Courbot
Cc: linux-kernel, dri-devel, dakr, Alistair Popple, Miguel Ojeda,
Alex Gaynor, Boqun Feng, Gary Guo, bjorn3_gh, Benno Lossin,
Andreas Hindborg, Alice Ryhl, Trevor Gross, David Airlie,
Simona Vetter, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, John Hubbard, Timur Tabi, joel, Elle Rhumsaa,
Daniel Almeida, nouveau, rust-for-linux
On Mon, Sep 08, 2025 at 12:26:43PM +0900, Alexandre Courbot wrote:
> On Thu Sep 4, 2025 at 6:54 AM JST, Joel Fernandes wrote:
> > Previously, bitstructs were hardcoded to use u32 as the underlying
> > storage type. Add support for different storage types (u8, u16, u32,
> > u64) to the bitstruct macro.
> >
> > New syntax is: struct Name: <type ex., u32> { ... }
> >
> > Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
> <snip>
> > // Generates the accessor methods for a single field.
> > (
> > - @leaf_accessor $name:ident $hi:tt:$lo:tt $field:ident
> > + @leaf_accessor $name:ident $storage:ty, $hi:tt:$lo:tt $field:ident
> > { $process:expr } $to_type:ty => $res_type:ty $(, $comment:literal)?;
> > ) => {
> > ::kernel::macros::paste!(
> > const [<$field:upper _RANGE>]: ::core::ops::RangeInclusive<u8> = $lo..=$hi;
> > - const [<$field:upper _MASK>]: u32 = ((((1 << $hi) - 1) << 1) + 1) - ((1 << $lo) - 1);
> > + const [<$field:upper _MASK>]: $storage = {
> > + // Generate mask for shifting
> > + match ::core::mem::size_of::<$storage>() {
> > + 1 => ::kernel::bits::genmask_u8($lo..=$hi) as $storage,
> > + 2 => ::kernel::bits::genmask_u16($lo..=$hi) as $storage,
> > + 4 => ::kernel::bits::genmask_u32($lo..=$hi) as $storage,
> > + 8 => ::kernel::bits::genmask_u64($lo..=$hi) as $storage,
> > + _ => <$storage>::MAX
>
> Since this is a const expression, you can use `build_error!` to make
> compilation fail in the unlikely event the `_` is taken due to bug in
> the code.
Makes sense, changed.
>
> > + }
> > + };
> > const [<$field:upper _SHIFT>]: u32 = Self::[<$field:upper _MASK>].trailing_zeros();
> > );
> >
> > @@ -211,7 +220,7 @@ impl $name {
> > #[inline(always)]
> > pub(crate) fn $field(self) -> $res_type {
> > ::kernel::macros::paste!(
> > - const MASK: u32 = $name::[<$field:upper _MASK>];
> > + const MASK: $storage = $name::[<$field:upper _MASK>];
> > const SHIFT: u32 = $name::[<$field:upper _SHIFT>];
> > );
> > let field = ((self.0 & MASK) >> SHIFT);
> > @@ -226,9 +235,9 @@ pub(crate) fn $field(self) -> $res_type {
> > )?
> > #[inline(always)]
> > pub(crate) fn [<set_ $field>](mut self, value: $to_type) -> Self {
> > - const MASK: u32 = $name::[<$field:upper _MASK>];
> > + const MASK: $storage = $name::[<$field:upper _MASK>];
> > const SHIFT: u32 = $name::[<$field:upper _SHIFT>];
> > - let value = (u32::from(value) << SHIFT) & MASK;
> > + let value = (<$storage>::from(value) << SHIFT) & MASK;
> > self.0 = (self.0 & !MASK) | value;
> >
> > self
> > @@ -237,7 +246,7 @@ pub(crate) fn [<set_ $field>](mut self, value: $to_type) -> Self {
> > };
> >
> > // Generates the `Debug` implementation for `$name`.
> > - (@debug $name:ident { $($field:ident;)* }) => {
> > + (@debug $name:ident $storage:ty { $($field:ident;)* }) => {
>
> This rule doesn't make use of the `$storage` argument.
Removed unused arg, thanks.
> > impl ::core::fmt::Debug for $name {
> > fn fmt(&self, f: &mut ::core::fmt::Formatter<'_>) -> ::core::fmt::Result {
> > f.debug_struct(stringify!($name))
> > @@ -251,7 +260,7 @@ fn fmt(&self, f: &mut ::core::fmt::Formatter<'_>) -> ::core::fmt::Result {
> > };
> >
> > // Generates the `Default` implementation for `$name`.
> > - (@default $name:ident { $($field:ident;)* }) => {
> > + (@default $name:ident $storage:ty { $($field:ident;)* }) => {
>
> Neither does this one.
Removed unused arg, thanks.
thanks,
- Joel
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v2 3/4] nova-core: bitstruct: Add support for custom visiblity
2025-09-03 21:54 [PATCH v2 0/4] Improve bitfield support in Rust Joel Fernandes
2025-09-03 21:54 ` [PATCH v2 1/4] nova-core: bitstruct: Move bitfield-specific code from register! into new macro Joel Fernandes
2025-09-03 21:54 ` [PATCH v2 2/4] nova-core: bitstruct: Add support for different storage widths Joel Fernandes
@ 2025-09-03 21:54 ` Joel Fernandes
2025-09-05 22:22 ` Elle Rhumsaa
` (2 more replies)
2025-09-03 21:54 ` [PATCH v2 4/4] rust: Move register and bitstruct macros out of Nova Joel Fernandes
3 siblings, 3 replies; 28+ messages in thread
From: Joel Fernandes @ 2025-09-03 21:54 UTC (permalink / raw)
To: linux-kernel, dri-devel, dakr, acourbot
Cc: Alistair Popple, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
bjorn3_gh, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, David Airlie, Simona Vetter, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, John Hubbard, Joel Fernandes,
Timur Tabi, joel, Elle Rhumsaa, Daniel Almeida, nouveau,
rust-for-linux
Add support for custom visiblity to allow for users to control visibility
of the structure and helpers.
Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
---
drivers/gpu/nova-core/bitstruct.rs | 46 ++++++++++++++--------------
drivers/gpu/nova-core/regs/macros.rs | 16 +++++-----
2 files changed, 31 insertions(+), 31 deletions(-)
diff --git a/drivers/gpu/nova-core/bitstruct.rs b/drivers/gpu/nova-core/bitstruct.rs
index 068334c86981..1047c5c17e2d 100644
--- a/drivers/gpu/nova-core/bitstruct.rs
+++ b/drivers/gpu/nova-core/bitstruct.rs
@@ -9,7 +9,7 @@
///
/// ```rust
/// bitstruct! {
-/// struct ControlReg: u32 {
+/// pub struct ControlReg: u32 {
/// 3:0 mode as u8 ?=> Mode;
/// 7:4 state as u8 => State;
/// }
@@ -34,21 +34,21 @@
/// and returns the result. This is useful with fields for which not all values are valid.
macro_rules! bitstruct {
// Main entry point - defines the bitfield struct with fields
- (struct $name:ident : $storage:ty $(, $comment:literal)? { $($fields:tt)* }) => {
- bitstruct!(@core $name $storage $(, $comment)? { $($fields)* });
+ ($vis:vis struct $name:ident : $storage:ty $(, $comment:literal)? { $($fields:tt)* }) => {
+ bitstruct!(@core $name $vis $storage $(, $comment)? { $($fields)* });
};
// All rules below are helpers.
// Defines the wrapper `$name` type, as well as its relevant implementations (`Debug`,
// `Default`, `BitOr`, and conversion to the value type) and field accessor methods.
- (@core $name:ident $storage:ty $(, $comment:literal)? { $($fields:tt)* }) => {
+ (@core $name:ident $vis:vis $storage:ty $(, $comment:literal)? { $($fields:tt)* }) => {
$(
#[doc=$comment]
)?
#[repr(transparent)]
#[derive(Clone, Copy)]
- pub(crate) struct $name($storage);
+ $vis struct $name($vis $storage);
impl ::core::ops::BitOr for $name {
type Output = Self;
@@ -64,14 +64,14 @@ fn from(val: $name) -> $storage {
}
}
- bitstruct!(@fields_dispatcher $name $storage { $($fields)* });
+ bitstruct!(@fields_dispatcher $name $vis $storage { $($fields)* });
};
// Captures the fields and passes them to all the implementers that require field information.
//
// Used to simplify the matching rules for implementers, so they don't need to match the entire
// complex fields rule even though they only make use of part of it.
- (@fields_dispatcher $name:ident $storage:ty {
+ (@fields_dispatcher $name:ident $vis:vis $storage:ty {
$($hi:tt:$lo:tt $field:ident as $type:tt
$(?=> $try_into_type:ty)?
$(=> $into_type:ty)?
@@ -80,7 +80,7 @@ fn from(val: $name) -> $storage {
)*
}
) => {
- bitstruct!(@field_accessors $name $storage {
+ bitstruct!(@field_accessors $name $vis $storage {
$(
$hi:$lo $field as $type
$(?=> $try_into_type)?
@@ -95,7 +95,7 @@ fn from(val: $name) -> $storage {
// Defines all the field getter/setter methods for `$name`.
(
- @field_accessors $name:ident $storage:ty {
+ @field_accessors $name:ident $vis:vis $storage:ty {
$($hi:tt:$lo:tt $field:ident as $type:tt
$(?=> $try_into_type:ty)?
$(=> $into_type:ty)?
@@ -111,7 +111,7 @@ fn from(val: $name) -> $storage {
#[allow(dead_code)]
impl $name {
$(
- bitstruct!(@field_accessor $name $storage, $hi:$lo $field as $type
+ bitstruct!(@field_accessor $name $vis $storage, $hi:$lo $field as $type
$(?=> $try_into_type)?
$(=> $into_type)?
$(, $comment)?
@@ -145,11 +145,11 @@ impl $name {
// Catches fields defined as `bool` and convert them into a boolean value.
(
- @field_accessor $name:ident $storage:ty, $hi:tt:$lo:tt $field:ident as bool => $into_type:ty
+ @field_accessor $name:ident $vis:vis $storage:ty, $hi:tt:$lo:tt $field:ident as bool => $into_type:ty
$(, $comment:literal)?;
) => {
bitstruct!(
- @leaf_accessor $name $storage, $hi:$lo $field
+ @leaf_accessor $name $vis $storage, $hi:$lo $field
{ |f| <$into_type>::from(if f != 0 { true } else { false }) }
$into_type => $into_type $(, $comment)?;
);
@@ -157,17 +157,17 @@ impl $name {
// Shortcut for fields defined as `bool` without the `=>` syntax.
(
- @field_accessor $name:ident $storage:ty, $hi:tt:$lo:tt $field:ident as bool $(, $comment:literal)?;
+ @field_accessor $name:ident $vis:vis $storage:ty, $hi:tt:$lo:tt $field:ident as bool $(, $comment:literal)?;
) => {
- bitstruct!(@field_accessor $name $storage, $hi:$lo $field as bool => bool $(, $comment)?;);
+ bitstruct!(@field_accessor $name $vis $storage, $hi:$lo $field as bool => bool $(, $comment)?;);
};
// Catches the `?=>` syntax for non-boolean fields.
(
- @field_accessor $name:ident $storage:ty, $hi:tt:$lo:tt $field:ident as $type:tt ?=> $try_into_type:ty
+ @field_accessor $name:ident $vis:vis $storage:ty, $hi:tt:$lo:tt $field:ident as $type:tt ?=> $try_into_type:ty
$(, $comment:literal)?;
) => {
- bitstruct!(@leaf_accessor $name $storage, $hi:$lo $field
+ bitstruct!(@leaf_accessor $name $vis $storage, $hi:$lo $field
{ |f| <$try_into_type>::try_from(f as $type) } $try_into_type =>
::core::result::Result<
$try_into_type,
@@ -178,24 +178,24 @@ impl $name {
// Catches the `=>` syntax for non-boolean fields.
(
- @field_accessor $name:ident $storage:ty, $hi:tt:$lo:tt $field:ident as $type:tt => $into_type:ty
+ @field_accessor $name:ident $vis:vis $storage:ty, $hi:tt:$lo:tt $field:ident as $type:tt => $into_type:ty
$(, $comment:literal)?;
) => {
- bitstruct!(@leaf_accessor $name $storage, $hi:$lo $field
+ bitstruct!(@leaf_accessor $name $vis $storage, $hi:$lo $field
{ |f| <$into_type>::from(f as $type) } $into_type => $into_type $(, $comment)?;);
};
// Shortcut for non-boolean fields defined without the `=>` or `?=>` syntax.
(
- @field_accessor $name:ident $storage:ty, $hi:tt:$lo:tt $field:ident as $type:tt
+ @field_accessor $name:ident $vis:vis $storage:ty, $hi:tt:$lo:tt $field:ident as $type:tt
$(, $comment:literal)?;
) => {
- bitstruct!(@field_accessor $name $storage, $hi:$lo $field as $type => $type $(, $comment)?;);
+ bitstruct!(@field_accessor $name $vis $storage, $hi:$lo $field as $type => $type $(, $comment)?;);
};
// Generates the accessor methods for a single field.
(
- @leaf_accessor $name:ident $storage:ty, $hi:tt:$lo:tt $field:ident
+ @leaf_accessor $name:ident $vis:vis $storage:ty, $hi:tt:$lo:tt $field:ident
{ $process:expr } $to_type:ty => $res_type:ty $(, $comment:literal)?;
) => {
::kernel::macros::paste!(
@@ -218,7 +218,7 @@ impl $name {
#[doc=$comment]
)?
#[inline(always)]
- pub(crate) fn $field(self) -> $res_type {
+ $vis fn $field(self) -> $res_type {
::kernel::macros::paste!(
const MASK: $storage = $name::[<$field:upper _MASK>];
const SHIFT: u32 = $name::[<$field:upper _SHIFT>];
@@ -234,7 +234,7 @@ pub(crate) fn $field(self) -> $res_type {
#[doc=$comment]
)?
#[inline(always)]
- pub(crate) fn [<set_ $field>](mut self, value: $to_type) -> Self {
+ $vis fn [<set_ $field>](mut self, value: $to_type) -> Self {
const MASK: $storage = $name::[<$field:upper _MASK>];
const SHIFT: u32 = $name::[<$field:upper _SHIFT>];
let value = (<$storage>::from(value) << SHIFT) & MASK;
diff --git a/drivers/gpu/nova-core/regs/macros.rs b/drivers/gpu/nova-core/regs/macros.rs
index bbfeab147c9f..22a53a73b765 100644
--- a/drivers/gpu/nova-core/regs/macros.rs
+++ b/drivers/gpu/nova-core/regs/macros.rs
@@ -284,25 +284,25 @@ pub(crate) trait RegisterBase<T> {
macro_rules! register {
// Creates a register at a fixed offset of the MMIO space.
($name:ident @ $offset:literal $(, $comment:literal)? { $($fields:tt)* } ) => {
- bitstruct!(struct $name: u32 $(, $comment)? { $($fields)* } );
+ bitstruct!(pub(crate) struct $name: u32 $(, $comment)? { $($fields)* } );
register!(@io_fixed $name @ $offset);
};
// Creates an alias register of fixed offset register `alias` with its own fields.
($name:ident => $alias:ident $(, $comment:literal)? { $($fields:tt)* } ) => {
- bitstruct!(struct $name: u32 $(, $comment)? { $($fields)* } );
+ bitstruct!(pub(crate) struct $name: u32 $(, $comment)? { $($fields)* } );
register!(@io_fixed $name @ $alias::OFFSET);
};
// Creates a register at a relative offset from a base address provider.
($name:ident @ $base:ty [ $offset:literal ] $(, $comment:literal)? { $($fields:tt)* } ) => {
- bitstruct!(struct $name: u32 $(, $comment)? { $($fields)* } );
+ bitstruct!(pub(crate) struct $name: u32 $(, $comment)? { $($fields)* } );
register!(@io_relative $name @ $base [ $offset ]);
};
// Creates an alias register of relative offset register `alias` with its own fields.
($name:ident => $base:ty [ $alias:ident ] $(, $comment:literal)? { $($fields:tt)* }) => {
- bitstruct!(struct $name: u32 $(, $comment)? { $($fields)* } );
+ bitstruct!(pub(crate) struct $name: u32 $(, $comment)? { $($fields)* } );
register!(@io_relative $name @ $base [ $alias::OFFSET ]);
};
@@ -313,7 +313,7 @@ macro_rules! register {
}
) => {
static_assert!(::core::mem::size_of::<u32>() <= $stride);
- bitstruct!(struct $name: u32 $(, $comment)? { $($fields)* } );
+ bitstruct!(pub(crate) struct $name: u32 $(, $comment)? { $($fields)* } );
register!(@io_array $name @ $offset [ $size ; $stride ]);
};
@@ -334,7 +334,7 @@ macro_rules! register {
$(, $comment:literal)? { $($fields:tt)* }
) => {
static_assert!(::core::mem::size_of::<u32>() <= $stride);
- bitstruct!(struct $name: u32 $(, $comment)? { $($fields)* } );
+ bitstruct!(pub(crate) struct $name: u32 $(, $comment)? { $($fields)* } );
register!(@io_relative_array $name @ $base [ $offset [ $size ; $stride ] ]);
};
@@ -356,7 +356,7 @@ macro_rules! register {
}
) => {
static_assert!($idx < $alias::SIZE);
- bitstruct!(struct $name: u32 $(, $comment)? { $($fields)* } );
+ bitstruct!(pub(crate) struct $name: u32 $(, $comment)? { $($fields)* } );
register!(@io_relative $name @ $base [ $alias::OFFSET + $idx * $alias::STRIDE ] );
};
@@ -365,7 +365,7 @@ macro_rules! register {
// to avoid it being interpreted in place of the relative register array alias rule.
($name:ident => $alias:ident [ $idx:expr ] $(, $comment:literal)? { $($fields:tt)* }) => {
static_assert!($idx < $alias::SIZE);
- bitstruct!(struct $name: u32 $(, $comment)? { $($fields)* } );
+ bitstruct!(pub(crate) struct $name: u32 $(, $comment)? { $($fields)* } );
register!(@io_fixed $name @ $alias::OFFSET + $idx * $alias::STRIDE );
};
--
2.34.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH v2 3/4] nova-core: bitstruct: Add support for custom visiblity
2025-09-03 21:54 ` [PATCH v2 3/4] nova-core: bitstruct: Add support for custom visiblity Joel Fernandes
@ 2025-09-05 22:22 ` Elle Rhumsaa
2025-09-05 22:23 ` Elle Rhumsaa
2025-09-08 3:40 ` Alexandre Courbot
2 siblings, 0 replies; 28+ messages in thread
From: Elle Rhumsaa @ 2025-09-05 22:22 UTC (permalink / raw)
To: Joel Fernandes
Cc: linux-kernel, dri-devel, dakr, acourbot, Alistair Popple,
Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo, bjorn3_gh,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
David Airlie, Simona Vetter, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, John Hubbard, Timur Tabi, joel, Daniel Almeida,
nouveau, rust-for-linux
On Wed, Sep 03, 2025 at 05:54:27PM -0400, Joel Fernandes wrote:
> Add support for custom visiblity to allow for users to control visibility
> of the structure and helpers.
>
> Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
> ---
> drivers/gpu/nova-core/bitstruct.rs | 46 ++++++++++++++--------------
> drivers/gpu/nova-core/regs/macros.rs | 16 +++++-----
> 2 files changed, 31 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/gpu/nova-core/bitstruct.rs b/drivers/gpu/nova-core/bitstruct.rs
> index 068334c86981..1047c5c17e2d 100644
> --- a/drivers/gpu/nova-core/bitstruct.rs
> +++ b/drivers/gpu/nova-core/bitstruct.rs
> @@ -9,7 +9,7 @@
> ///
> /// ```rust
> /// bitstruct! {
> -/// struct ControlReg: u32 {
> +/// pub struct ControlReg: u32 {
> /// 3:0 mode as u8 ?=> Mode;
> /// 7:4 state as u8 => State;
> /// }
> @@ -34,21 +34,21 @@
> /// and returns the result. This is useful with fields for which not all values are valid.
> macro_rules! bitstruct {
> // Main entry point - defines the bitfield struct with fields
> - (struct $name:ident : $storage:ty $(, $comment:literal)? { $($fields:tt)* }) => {
> - bitstruct!(@core $name $storage $(, $comment)? { $($fields)* });
> + ($vis:vis struct $name:ident : $storage:ty $(, $comment:literal)? { $($fields:tt)* }) => {
> + bitstruct!(@core $name $vis $storage $(, $comment)? { $($fields)* });
> };
>
> // All rules below are helpers.
>
> // Defines the wrapper `$name` type, as well as its relevant implementations (`Debug`,
> // `Default`, `BitOr`, and conversion to the value type) and field accessor methods.
> - (@core $name:ident $storage:ty $(, $comment:literal)? { $($fields:tt)* }) => {
> + (@core $name:ident $vis:vis $storage:ty $(, $comment:literal)? { $($fields:tt)* }) => {
> $(
> #[doc=$comment]
> )?
> #[repr(transparent)]
> #[derive(Clone, Copy)]
> - pub(crate) struct $name($storage);
> + $vis struct $name($vis $storage);
>
> impl ::core::ops::BitOr for $name {
> type Output = Self;
> @@ -64,14 +64,14 @@ fn from(val: $name) -> $storage {
> }
> }
>
> - bitstruct!(@fields_dispatcher $name $storage { $($fields)* });
> + bitstruct!(@fields_dispatcher $name $vis $storage { $($fields)* });
> };
>
> // Captures the fields and passes them to all the implementers that require field information.
> //
> // Used to simplify the matching rules for implementers, so they don't need to match the entire
> // complex fields rule even though they only make use of part of it.
> - (@fields_dispatcher $name:ident $storage:ty {
> + (@fields_dispatcher $name:ident $vis:vis $storage:ty {
> $($hi:tt:$lo:tt $field:ident as $type:tt
> $(?=> $try_into_type:ty)?
> $(=> $into_type:ty)?
> @@ -80,7 +80,7 @@ fn from(val: $name) -> $storage {
> )*
> }
> ) => {
> - bitstruct!(@field_accessors $name $storage {
> + bitstruct!(@field_accessors $name $vis $storage {
> $(
> $hi:$lo $field as $type
> $(?=> $try_into_type)?
> @@ -95,7 +95,7 @@ fn from(val: $name) -> $storage {
>
> // Defines all the field getter/setter methods for `$name`.
> (
> - @field_accessors $name:ident $storage:ty {
> + @field_accessors $name:ident $vis:vis $storage:ty {
> $($hi:tt:$lo:tt $field:ident as $type:tt
> $(?=> $try_into_type:ty)?
> $(=> $into_type:ty)?
> @@ -111,7 +111,7 @@ fn from(val: $name) -> $storage {
> #[allow(dead_code)]
> impl $name {
> $(
> - bitstruct!(@field_accessor $name $storage, $hi:$lo $field as $type
> + bitstruct!(@field_accessor $name $vis $storage, $hi:$lo $field as $type
> $(?=> $try_into_type)?
> $(=> $into_type)?
> $(, $comment)?
> @@ -145,11 +145,11 @@ impl $name {
>
> // Catches fields defined as `bool` and convert them into a boolean value.
> (
> - @field_accessor $name:ident $storage:ty, $hi:tt:$lo:tt $field:ident as bool => $into_type:ty
> + @field_accessor $name:ident $vis:vis $storage:ty, $hi:tt:$lo:tt $field:ident as bool => $into_type:ty
> $(, $comment:literal)?;
> ) => {
> bitstruct!(
> - @leaf_accessor $name $storage, $hi:$lo $field
> + @leaf_accessor $name $vis $storage, $hi:$lo $field
> { |f| <$into_type>::from(if f != 0 { true } else { false }) }
> $into_type => $into_type $(, $comment)?;
> );
> @@ -157,17 +157,17 @@ impl $name {
>
> // Shortcut for fields defined as `bool` without the `=>` syntax.
> (
> - @field_accessor $name:ident $storage:ty, $hi:tt:$lo:tt $field:ident as bool $(, $comment:literal)?;
> + @field_accessor $name:ident $vis:vis $storage:ty, $hi:tt:$lo:tt $field:ident as bool $(, $comment:literal)?;
> ) => {
> - bitstruct!(@field_accessor $name $storage, $hi:$lo $field as bool => bool $(, $comment)?;);
> + bitstruct!(@field_accessor $name $vis $storage, $hi:$lo $field as bool => bool $(, $comment)?;);
> };
>
> // Catches the `?=>` syntax for non-boolean fields.
> (
> - @field_accessor $name:ident $storage:ty, $hi:tt:$lo:tt $field:ident as $type:tt ?=> $try_into_type:ty
> + @field_accessor $name:ident $vis:vis $storage:ty, $hi:tt:$lo:tt $field:ident as $type:tt ?=> $try_into_type:ty
> $(, $comment:literal)?;
> ) => {
> - bitstruct!(@leaf_accessor $name $storage, $hi:$lo $field
> + bitstruct!(@leaf_accessor $name $vis $storage, $hi:$lo $field
> { |f| <$try_into_type>::try_from(f as $type) } $try_into_type =>
> ::core::result::Result<
> $try_into_type,
> @@ -178,24 +178,24 @@ impl $name {
>
> // Catches the `=>` syntax for non-boolean fields.
> (
> - @field_accessor $name:ident $storage:ty, $hi:tt:$lo:tt $field:ident as $type:tt => $into_type:ty
> + @field_accessor $name:ident $vis:vis $storage:ty, $hi:tt:$lo:tt $field:ident as $type:tt => $into_type:ty
> $(, $comment:literal)?;
> ) => {
> - bitstruct!(@leaf_accessor $name $storage, $hi:$lo $field
> + bitstruct!(@leaf_accessor $name $vis $storage, $hi:$lo $field
> { |f| <$into_type>::from(f as $type) } $into_type => $into_type $(, $comment)?;);
> };
>
> // Shortcut for non-boolean fields defined without the `=>` or `?=>` syntax.
> (
> - @field_accessor $name:ident $storage:ty, $hi:tt:$lo:tt $field:ident as $type:tt
> + @field_accessor $name:ident $vis:vis $storage:ty, $hi:tt:$lo:tt $field:ident as $type:tt
> $(, $comment:literal)?;
> ) => {
> - bitstruct!(@field_accessor $name $storage, $hi:$lo $field as $type => $type $(, $comment)?;);
> + bitstruct!(@field_accessor $name $vis $storage, $hi:$lo $field as $type => $type $(, $comment)?;);
> };
>
> // Generates the accessor methods for a single field.
> (
> - @leaf_accessor $name:ident $storage:ty, $hi:tt:$lo:tt $field:ident
> + @leaf_accessor $name:ident $vis:vis $storage:ty, $hi:tt:$lo:tt $field:ident
> { $process:expr } $to_type:ty => $res_type:ty $(, $comment:literal)?;
> ) => {
> ::kernel::macros::paste!(
> @@ -218,7 +218,7 @@ impl $name {
> #[doc=$comment]
> )?
> #[inline(always)]
> - pub(crate) fn $field(self) -> $res_type {
> + $vis fn $field(self) -> $res_type {
> ::kernel::macros::paste!(
> const MASK: $storage = $name::[<$field:upper _MASK>];
> const SHIFT: u32 = $name::[<$field:upper _SHIFT>];
> @@ -234,7 +234,7 @@ pub(crate) fn $field(self) -> $res_type {
> #[doc=$comment]
> )?
> #[inline(always)]
> - pub(crate) fn [<set_ $field>](mut self, value: $to_type) -> Self {
> + $vis fn [<set_ $field>](mut self, value: $to_type) -> Self {
> const MASK: $storage = $name::[<$field:upper _MASK>];
> const SHIFT: u32 = $name::[<$field:upper _SHIFT>];
> let value = (<$storage>::from(value) << SHIFT) & MASK;
> diff --git a/drivers/gpu/nova-core/regs/macros.rs b/drivers/gpu/nova-core/regs/macros.rs
> index bbfeab147c9f..22a53a73b765 100644
> --- a/drivers/gpu/nova-core/regs/macros.rs
> +++ b/drivers/gpu/nova-core/regs/macros.rs
> @@ -284,25 +284,25 @@ pub(crate) trait RegisterBase<T> {
> macro_rules! register {
> // Creates a register at a fixed offset of the MMIO space.
> ($name:ident @ $offset:literal $(, $comment:literal)? { $($fields:tt)* } ) => {
> - bitstruct!(struct $name: u32 $(, $comment)? { $($fields)* } );
> + bitstruct!(pub(crate) struct $name: u32 $(, $comment)? { $($fields)* } );
> register!(@io_fixed $name @ $offset);
> };
>
> // Creates an alias register of fixed offset register `alias` with its own fields.
> ($name:ident => $alias:ident $(, $comment:literal)? { $($fields:tt)* } ) => {
> - bitstruct!(struct $name: u32 $(, $comment)? { $($fields)* } );
> + bitstruct!(pub(crate) struct $name: u32 $(, $comment)? { $($fields)* } );
> register!(@io_fixed $name @ $alias::OFFSET);
> };
>
> // Creates a register at a relative offset from a base address provider.
> ($name:ident @ $base:ty [ $offset:literal ] $(, $comment:literal)? { $($fields:tt)* } ) => {
> - bitstruct!(struct $name: u32 $(, $comment)? { $($fields)* } );
> + bitstruct!(pub(crate) struct $name: u32 $(, $comment)? { $($fields)* } );
> register!(@io_relative $name @ $base [ $offset ]);
> };
>
> // Creates an alias register of relative offset register `alias` with its own fields.
> ($name:ident => $base:ty [ $alias:ident ] $(, $comment:literal)? { $($fields:tt)* }) => {
> - bitstruct!(struct $name: u32 $(, $comment)? { $($fields)* } );
> + bitstruct!(pub(crate) struct $name: u32 $(, $comment)? { $($fields)* } );
> register!(@io_relative $name @ $base [ $alias::OFFSET ]);
> };
>
> @@ -313,7 +313,7 @@ macro_rules! register {
> }
> ) => {
> static_assert!(::core::mem::size_of::<u32>() <= $stride);
> - bitstruct!(struct $name: u32 $(, $comment)? { $($fields)* } );
> + bitstruct!(pub(crate) struct $name: u32 $(, $comment)? { $($fields)* } );
> register!(@io_array $name @ $offset [ $size ; $stride ]);
> };
>
> @@ -334,7 +334,7 @@ macro_rules! register {
> $(, $comment:literal)? { $($fields:tt)* }
> ) => {
> static_assert!(::core::mem::size_of::<u32>() <= $stride);
> - bitstruct!(struct $name: u32 $(, $comment)? { $($fields)* } );
> + bitstruct!(pub(crate) struct $name: u32 $(, $comment)? { $($fields)* } );
> register!(@io_relative_array $name @ $base [ $offset [ $size ; $stride ] ]);
> };
>
> @@ -356,7 +356,7 @@ macro_rules! register {
> }
> ) => {
> static_assert!($idx < $alias::SIZE);
> - bitstruct!(struct $name: u32 $(, $comment)? { $($fields)* } );
> + bitstruct!(pub(crate) struct $name: u32 $(, $comment)? { $($fields)* } );
> register!(@io_relative $name @ $base [ $alias::OFFSET + $idx * $alias::STRIDE ] );
> };
>
> @@ -365,7 +365,7 @@ macro_rules! register {
> // to avoid it being interpreted in place of the relative register array alias rule.
> ($name:ident => $alias:ident [ $idx:expr ] $(, $comment:literal)? { $($fields:tt)* }) => {
> static_assert!($idx < $alias::SIZE);
> - bitstruct!(struct $name: u32 $(, $comment)? { $($fields)* } );
> + bitstruct!(pub(crate) struct $name: u32 $(, $comment)? { $($fields)* } );
> register!(@io_fixed $name @ $alias::OFFSET + $idx * $alias::STRIDE );
> };
>
> --
> 2.34.1
>
>
Reviewed-by: Elle Rhumsaa <elle@weathered-steel.dev>
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v2 3/4] nova-core: bitstruct: Add support for custom visiblity
2025-09-03 21:54 ` [PATCH v2 3/4] nova-core: bitstruct: Add support for custom visiblity Joel Fernandes
2025-09-05 22:22 ` Elle Rhumsaa
@ 2025-09-05 22:23 ` Elle Rhumsaa
2025-09-08 3:40 ` Alexandre Courbot
2 siblings, 0 replies; 28+ messages in thread
From: Elle Rhumsaa @ 2025-09-05 22:23 UTC (permalink / raw)
To: Joel Fernandes
Cc: linux-kernel, dri-devel, dakr, acourbot, Alistair Popple,
Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo, bjorn3_gh,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
David Airlie, Simona Vetter, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, John Hubbard, Timur Tabi, joel, Daniel Almeida,
nouveau, rust-for-linux
On Wed, Sep 03, 2025 at 05:54:27PM -0400, Joel Fernandes wrote:
> Add support for custom visiblity to allow for users to control visibility
> of the structure and helpers.
>
> Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
> ---
> drivers/gpu/nova-core/bitstruct.rs | 46 ++++++++++++++--------------
> drivers/gpu/nova-core/regs/macros.rs | 16 +++++-----
> 2 files changed, 31 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/gpu/nova-core/bitstruct.rs b/drivers/gpu/nova-core/bitstruct.rs
> index 068334c86981..1047c5c17e2d 100644
> --- a/drivers/gpu/nova-core/bitstruct.rs
> +++ b/drivers/gpu/nova-core/bitstruct.rs
> @@ -9,7 +9,7 @@
> ///
> /// ```rust
> /// bitstruct! {
> -/// struct ControlReg: u32 {
> +/// pub struct ControlReg: u32 {
> /// 3:0 mode as u8 ?=> Mode;
> /// 7:4 state as u8 => State;
> /// }
> @@ -34,21 +34,21 @@
> /// and returns the result. This is useful with fields for which not all values are valid.
> macro_rules! bitstruct {
> // Main entry point - defines the bitfield struct with fields
> - (struct $name:ident : $storage:ty $(, $comment:literal)? { $($fields:tt)* }) => {
> - bitstruct!(@core $name $storage $(, $comment)? { $($fields)* });
> + ($vis:vis struct $name:ident : $storage:ty $(, $comment:literal)? { $($fields:tt)* }) => {
> + bitstruct!(@core $name $vis $storage $(, $comment)? { $($fields)* });
> };
>
> // All rules below are helpers.
>
> // Defines the wrapper `$name` type, as well as its relevant implementations (`Debug`,
> // `Default`, `BitOr`, and conversion to the value type) and field accessor methods.
> - (@core $name:ident $storage:ty $(, $comment:literal)? { $($fields:tt)* }) => {
> + (@core $name:ident $vis:vis $storage:ty $(, $comment:literal)? { $($fields:tt)* }) => {
> $(
> #[doc=$comment]
> )?
> #[repr(transparent)]
> #[derive(Clone, Copy)]
> - pub(crate) struct $name($storage);
> + $vis struct $name($vis $storage);
>
> impl ::core::ops::BitOr for $name {
> type Output = Self;
> @@ -64,14 +64,14 @@ fn from(val: $name) -> $storage {
> }
> }
>
> - bitstruct!(@fields_dispatcher $name $storage { $($fields)* });
> + bitstruct!(@fields_dispatcher $name $vis $storage { $($fields)* });
> };
>
> // Captures the fields and passes them to all the implementers that require field information.
> //
> // Used to simplify the matching rules for implementers, so they don't need to match the entire
> // complex fields rule even though they only make use of part of it.
> - (@fields_dispatcher $name:ident $storage:ty {
> + (@fields_dispatcher $name:ident $vis:vis $storage:ty {
> $($hi:tt:$lo:tt $field:ident as $type:tt
> $(?=> $try_into_type:ty)?
> $(=> $into_type:ty)?
> @@ -80,7 +80,7 @@ fn from(val: $name) -> $storage {
> )*
> }
> ) => {
> - bitstruct!(@field_accessors $name $storage {
> + bitstruct!(@field_accessors $name $vis $storage {
> $(
> $hi:$lo $field as $type
> $(?=> $try_into_type)?
> @@ -95,7 +95,7 @@ fn from(val: $name) -> $storage {
>
> // Defines all the field getter/setter methods for `$name`.
> (
> - @field_accessors $name:ident $storage:ty {
> + @field_accessors $name:ident $vis:vis $storage:ty {
> $($hi:tt:$lo:tt $field:ident as $type:tt
> $(?=> $try_into_type:ty)?
> $(=> $into_type:ty)?
> @@ -111,7 +111,7 @@ fn from(val: $name) -> $storage {
> #[allow(dead_code)]
> impl $name {
> $(
> - bitstruct!(@field_accessor $name $storage, $hi:$lo $field as $type
> + bitstruct!(@field_accessor $name $vis $storage, $hi:$lo $field as $type
> $(?=> $try_into_type)?
> $(=> $into_type)?
> $(, $comment)?
> @@ -145,11 +145,11 @@ impl $name {
>
> // Catches fields defined as `bool` and convert them into a boolean value.
> (
> - @field_accessor $name:ident $storage:ty, $hi:tt:$lo:tt $field:ident as bool => $into_type:ty
> + @field_accessor $name:ident $vis:vis $storage:ty, $hi:tt:$lo:tt $field:ident as bool => $into_type:ty
> $(, $comment:literal)?;
> ) => {
> bitstruct!(
> - @leaf_accessor $name $storage, $hi:$lo $field
> + @leaf_accessor $name $vis $storage, $hi:$lo $field
> { |f| <$into_type>::from(if f != 0 { true } else { false }) }
> $into_type => $into_type $(, $comment)?;
> );
> @@ -157,17 +157,17 @@ impl $name {
>
> // Shortcut for fields defined as `bool` without the `=>` syntax.
> (
> - @field_accessor $name:ident $storage:ty, $hi:tt:$lo:tt $field:ident as bool $(, $comment:literal)?;
> + @field_accessor $name:ident $vis:vis $storage:ty, $hi:tt:$lo:tt $field:ident as bool $(, $comment:literal)?;
> ) => {
> - bitstruct!(@field_accessor $name $storage, $hi:$lo $field as bool => bool $(, $comment)?;);
> + bitstruct!(@field_accessor $name $vis $storage, $hi:$lo $field as bool => bool $(, $comment)?;);
> };
>
> // Catches the `?=>` syntax for non-boolean fields.
> (
> - @field_accessor $name:ident $storage:ty, $hi:tt:$lo:tt $field:ident as $type:tt ?=> $try_into_type:ty
> + @field_accessor $name:ident $vis:vis $storage:ty, $hi:tt:$lo:tt $field:ident as $type:tt ?=> $try_into_type:ty
> $(, $comment:literal)?;
> ) => {
> - bitstruct!(@leaf_accessor $name $storage, $hi:$lo $field
> + bitstruct!(@leaf_accessor $name $vis $storage, $hi:$lo $field
> { |f| <$try_into_type>::try_from(f as $type) } $try_into_type =>
> ::core::result::Result<
> $try_into_type,
> @@ -178,24 +178,24 @@ impl $name {
>
> // Catches the `=>` syntax for non-boolean fields.
> (
> - @field_accessor $name:ident $storage:ty, $hi:tt:$lo:tt $field:ident as $type:tt => $into_type:ty
> + @field_accessor $name:ident $vis:vis $storage:ty, $hi:tt:$lo:tt $field:ident as $type:tt => $into_type:ty
> $(, $comment:literal)?;
> ) => {
> - bitstruct!(@leaf_accessor $name $storage, $hi:$lo $field
> + bitstruct!(@leaf_accessor $name $vis $storage, $hi:$lo $field
> { |f| <$into_type>::from(f as $type) } $into_type => $into_type $(, $comment)?;);
> };
>
> // Shortcut for non-boolean fields defined without the `=>` or `?=>` syntax.
> (
> - @field_accessor $name:ident $storage:ty, $hi:tt:$lo:tt $field:ident as $type:tt
> + @field_accessor $name:ident $vis:vis $storage:ty, $hi:tt:$lo:tt $field:ident as $type:tt
> $(, $comment:literal)?;
> ) => {
> - bitstruct!(@field_accessor $name $storage, $hi:$lo $field as $type => $type $(, $comment)?;);
> + bitstruct!(@field_accessor $name $vis $storage, $hi:$lo $field as $type => $type $(, $comment)?;);
> };
>
> // Generates the accessor methods for a single field.
> (
> - @leaf_accessor $name:ident $storage:ty, $hi:tt:$lo:tt $field:ident
> + @leaf_accessor $name:ident $vis:vis $storage:ty, $hi:tt:$lo:tt $field:ident
> { $process:expr } $to_type:ty => $res_type:ty $(, $comment:literal)?;
> ) => {
> ::kernel::macros::paste!(
> @@ -218,7 +218,7 @@ impl $name {
> #[doc=$comment]
> )?
> #[inline(always)]
> - pub(crate) fn $field(self) -> $res_type {
> + $vis fn $field(self) -> $res_type {
> ::kernel::macros::paste!(
> const MASK: $storage = $name::[<$field:upper _MASK>];
> const SHIFT: u32 = $name::[<$field:upper _SHIFT>];
> @@ -234,7 +234,7 @@ pub(crate) fn $field(self) -> $res_type {
> #[doc=$comment]
> )?
> #[inline(always)]
> - pub(crate) fn [<set_ $field>](mut self, value: $to_type) -> Self {
> + $vis fn [<set_ $field>](mut self, value: $to_type) -> Self {
> const MASK: $storage = $name::[<$field:upper _MASK>];
> const SHIFT: u32 = $name::[<$field:upper _SHIFT>];
> let value = (<$storage>::from(value) << SHIFT) & MASK;
> diff --git a/drivers/gpu/nova-core/regs/macros.rs b/drivers/gpu/nova-core/regs/macros.rs
> index bbfeab147c9f..22a53a73b765 100644
> --- a/drivers/gpu/nova-core/regs/macros.rs
> +++ b/drivers/gpu/nova-core/regs/macros.rs
> @@ -284,25 +284,25 @@ pub(crate) trait RegisterBase<T> {
> macro_rules! register {
> // Creates a register at a fixed offset of the MMIO space.
> ($name:ident @ $offset:literal $(, $comment:literal)? { $($fields:tt)* } ) => {
> - bitstruct!(struct $name: u32 $(, $comment)? { $($fields)* } );
> + bitstruct!(pub(crate) struct $name: u32 $(, $comment)? { $($fields)* } );
> register!(@io_fixed $name @ $offset);
> };
>
> // Creates an alias register of fixed offset register `alias` with its own fields.
> ($name:ident => $alias:ident $(, $comment:literal)? { $($fields:tt)* } ) => {
> - bitstruct!(struct $name: u32 $(, $comment)? { $($fields)* } );
> + bitstruct!(pub(crate) struct $name: u32 $(, $comment)? { $($fields)* } );
> register!(@io_fixed $name @ $alias::OFFSET);
> };
>
> // Creates a register at a relative offset from a base address provider.
> ($name:ident @ $base:ty [ $offset:literal ] $(, $comment:literal)? { $($fields:tt)* } ) => {
> - bitstruct!(struct $name: u32 $(, $comment)? { $($fields)* } );
> + bitstruct!(pub(crate) struct $name: u32 $(, $comment)? { $($fields)* } );
> register!(@io_relative $name @ $base [ $offset ]);
> };
>
> // Creates an alias register of relative offset register `alias` with its own fields.
> ($name:ident => $base:ty [ $alias:ident ] $(, $comment:literal)? { $($fields:tt)* }) => {
> - bitstruct!(struct $name: u32 $(, $comment)? { $($fields)* } );
> + bitstruct!(pub(crate) struct $name: u32 $(, $comment)? { $($fields)* } );
> register!(@io_relative $name @ $base [ $alias::OFFSET ]);
> };
>
> @@ -313,7 +313,7 @@ macro_rules! register {
> }
> ) => {
> static_assert!(::core::mem::size_of::<u32>() <= $stride);
> - bitstruct!(struct $name: u32 $(, $comment)? { $($fields)* } );
> + bitstruct!(pub(crate) struct $name: u32 $(, $comment)? { $($fields)* } );
> register!(@io_array $name @ $offset [ $size ; $stride ]);
> };
>
> @@ -334,7 +334,7 @@ macro_rules! register {
> $(, $comment:literal)? { $($fields:tt)* }
> ) => {
> static_assert!(::core::mem::size_of::<u32>() <= $stride);
> - bitstruct!(struct $name: u32 $(, $comment)? { $($fields)* } );
> + bitstruct!(pub(crate) struct $name: u32 $(, $comment)? { $($fields)* } );
> register!(@io_relative_array $name @ $base [ $offset [ $size ; $stride ] ]);
> };
>
> @@ -356,7 +356,7 @@ macro_rules! register {
> }
> ) => {
> static_assert!($idx < $alias::SIZE);
> - bitstruct!(struct $name: u32 $(, $comment)? { $($fields)* } );
> + bitstruct!(pub(crate) struct $name: u32 $(, $comment)? { $($fields)* } );
> register!(@io_relative $name @ $base [ $alias::OFFSET + $idx * $alias::STRIDE ] );
> };
>
> @@ -365,7 +365,7 @@ macro_rules! register {
> // to avoid it being interpreted in place of the relative register array alias rule.
> ($name:ident => $alias:ident [ $idx:expr ] $(, $comment:literal)? { $($fields:tt)* }) => {
> static_assert!($idx < $alias::SIZE);
> - bitstruct!(struct $name: u32 $(, $comment)? { $($fields)* } );
> + bitstruct!(pub(crate) struct $name: u32 $(, $comment)? { $($fields)* } );
> register!(@io_fixed $name @ $alias::OFFSET + $idx * $alias::STRIDE );
> };
>
> --
> 2.34.1
>
>
Reviewed-by: Elle Rhumsaa <elle@weathered-steel.dev>
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v2 3/4] nova-core: bitstruct: Add support for custom visiblity
2025-09-03 21:54 ` [PATCH v2 3/4] nova-core: bitstruct: Add support for custom visiblity Joel Fernandes
2025-09-05 22:22 ` Elle Rhumsaa
2025-09-05 22:23 ` Elle Rhumsaa
@ 2025-09-08 3:40 ` Alexandre Courbot
2025-09-08 3:46 ` Alexandre Courbot
2025-09-09 18:30 ` Joel Fernandes
2 siblings, 2 replies; 28+ messages in thread
From: Alexandre Courbot @ 2025-09-08 3:40 UTC (permalink / raw)
To: Joel Fernandes, linux-kernel, dri-devel, dakr
Cc: Alistair Popple, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
bjorn3_gh, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, David Airlie, Simona Vetter, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, John Hubbard, Timur Tabi, joel,
Elle Rhumsaa, Daniel Almeida, nouveau, rust-for-linux
On Thu Sep 4, 2025 at 6:54 AM JST, Joel Fernandes wrote:
> Add support for custom visiblity to allow for users to control visibility
> of the structure and helpers.
>
> Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
> ---
> drivers/gpu/nova-core/bitstruct.rs | 46 ++++++++++++++--------------
> drivers/gpu/nova-core/regs/macros.rs | 16 +++++-----
> 2 files changed, 31 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/gpu/nova-core/bitstruct.rs b/drivers/gpu/nova-core/bitstruct.rs
> index 068334c86981..1047c5c17e2d 100644
> --- a/drivers/gpu/nova-core/bitstruct.rs
> +++ b/drivers/gpu/nova-core/bitstruct.rs
> @@ -9,7 +9,7 @@
> ///
> /// ```rust
> /// bitstruct! {
> -/// struct ControlReg: u32 {
> +/// pub struct ControlReg: u32 {
> /// 3:0 mode as u8 ?=> Mode;
> /// 7:4 state as u8 => State;
> /// }
Maybe mention in the documentation that the field accessors are given
the same visibility as the type - otherwise one might be led into
thinking that they can specify visibility for individual fields as well
(I'm wondering whether we might ever want that in the future?).
> @@ -34,21 +34,21 @@
> /// and returns the result. This is useful with fields for which not all values are valid.
> macro_rules! bitstruct {
> // Main entry point - defines the bitfield struct with fields
> - (struct $name:ident : $storage:ty $(, $comment:literal)? { $($fields:tt)* }) => {
> - bitstruct!(@core $name $storage $(, $comment)? { $($fields)* });
> + ($vis:vis struct $name:ident : $storage:ty $(, $comment:literal)? { $($fields:tt)* }) => {
> + bitstruct!(@core $name $vis $storage $(, $comment)? { $($fields)* });
> };
>
> // All rules below are helpers.
>
> // Defines the wrapper `$name` type, as well as its relevant implementations (`Debug`,
> // `Default`, `BitOr`, and conversion to the value type) and field accessor methods.
> - (@core $name:ident $storage:ty $(, $comment:literal)? { $($fields:tt)* }) => {
> + (@core $name:ident $vis:vis $storage:ty $(, $comment:literal)? { $($fields:tt)* }) => {
Being very nitpicky here, but for consistency why not put `$vis` before
`$name` since it is the order they are given by the caller?
> $(
> #[doc=$comment]
> )?
> #[repr(transparent)]
> #[derive(Clone, Copy)]
> - pub(crate) struct $name($storage);
> + $vis struct $name($vis $storage);
`$storage` should probably be kept private - we already have accessors
for it, and the visibility parameter is for the outer type, not its
internals.
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v2 3/4] nova-core: bitstruct: Add support for custom visiblity
2025-09-08 3:40 ` Alexandre Courbot
@ 2025-09-08 3:46 ` Alexandre Courbot
2025-09-09 19:05 ` Joel Fernandes
2025-09-09 18:30 ` Joel Fernandes
1 sibling, 1 reply; 28+ messages in thread
From: Alexandre Courbot @ 2025-09-08 3:46 UTC (permalink / raw)
To: Alexandre Courbot, Joel Fernandes, linux-kernel, dri-devel, dakr
Cc: Alistair Popple, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
bjorn3_gh, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, David Airlie, Simona Vetter, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, John Hubbard, Timur Tabi, joel,
Elle Rhumsaa, Daniel Almeida, nouveau, rust-for-linux
On Mon Sep 8, 2025 at 12:40 PM JST, Alexandre Courbot wrote:
> On Thu Sep 4, 2025 at 6:54 AM JST, Joel Fernandes wrote:
>> Add support for custom visiblity to allow for users to control visibility
>> of the structure and helpers.
>>
>> Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
>> ---
>> drivers/gpu/nova-core/bitstruct.rs | 46 ++++++++++++++--------------
>> drivers/gpu/nova-core/regs/macros.rs | 16 +++++-----
>> 2 files changed, 31 insertions(+), 31 deletions(-)
>>
>> diff --git a/drivers/gpu/nova-core/bitstruct.rs b/drivers/gpu/nova-core/bitstruct.rs
>> index 068334c86981..1047c5c17e2d 100644
>> --- a/drivers/gpu/nova-core/bitstruct.rs
>> +++ b/drivers/gpu/nova-core/bitstruct.rs
>> @@ -9,7 +9,7 @@
>> ///
>> /// ```rust
>> /// bitstruct! {
>> -/// struct ControlReg: u32 {
>> +/// pub struct ControlReg: u32 {
>> /// 3:0 mode as u8 ?=> Mode;
>> /// 7:4 state as u8 => State;
>> /// }
>
> Maybe mention in the documentation that the field accessors are given
> the same visibility as the type - otherwise one might be led into
> thinking that they can specify visibility for individual fields as well
> (I'm wondering whether we might ever want that in the future?).
Answering my own question: it could be useful! One example is
nova-core's `NV_PFALCON_FALCON_HWCFG2::mem_scrubbing` field. It turns
into `0` when scrubbing is completed, which is misleading. So to paliate
that we introduced a `mem_scrubbing_done` method that works as we want,
but the `mem_scrubbing` accessors are still present and can be called by
driver code. Making them private would force all callers to use
`mem_scrubbing_done`.
Another related feature would be a way to make some fields read-only or
write-only through an optional parameter.
I'm just mentioning these for the record; I'm not suggesting they need
to be done for the current series. :)
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v2 3/4] nova-core: bitstruct: Add support for custom visiblity
2025-09-08 3:46 ` Alexandre Courbot
@ 2025-09-09 19:05 ` Joel Fernandes
0 siblings, 0 replies; 28+ messages in thread
From: Joel Fernandes @ 2025-09-09 19:05 UTC (permalink / raw)
To: Alexandre Courbot
Cc: linux-kernel, dri-devel, dakr, Alistair Popple, Miguel Ojeda,
Alex Gaynor, Boqun Feng, Gary Guo, bjorn3_gh, Benno Lossin,
Andreas Hindborg, Alice Ryhl, Trevor Gross, David Airlie,
Simona Vetter, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, John Hubbard, Timur Tabi, joel, Elle Rhumsaa,
Daniel Almeida, nouveau, rust-for-linux
On Mon, Sep 08, 2025 at 12:46:57PM +0900, Alexandre Courbot wrote:
> On Mon Sep 8, 2025 at 12:40 PM JST, Alexandre Courbot wrote:
> > On Thu Sep 4, 2025 at 6:54 AM JST, Joel Fernandes wrote:
> >> Add support for custom visiblity to allow for users to control visibility
> >> of the structure and helpers.
> >>
> >> Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
> >> ---
> >> drivers/gpu/nova-core/bitstruct.rs | 46 ++++++++++++++--------------
> >> drivers/gpu/nova-core/regs/macros.rs | 16 +++++-----
> >> 2 files changed, 31 insertions(+), 31 deletions(-)
> >>
> >> diff --git a/drivers/gpu/nova-core/bitstruct.rs b/drivers/gpu/nova-core/bitstruct.rs
> >> index 068334c86981..1047c5c17e2d 100644
> >> --- a/drivers/gpu/nova-core/bitstruct.rs
> >> +++ b/drivers/gpu/nova-core/bitstruct.rs
> >> @@ -9,7 +9,7 @@
> >> ///
> >> /// ```rust
> >> /// bitstruct! {
> >> -/// struct ControlReg: u32 {
> >> +/// pub struct ControlReg: u32 {
> >> /// 3:0 mode as u8 ?=> Mode;
> >> /// 7:4 state as u8 => State;
> >> /// }
> >
> > Maybe mention in the documentation that the field accessors are given
> > the same visibility as the type - otherwise one might be led into
> > thinking that they can specify visibility for individual fields as well
> > (I'm wondering whether we might ever want that in the future?).
>
> Answering my own question: it could be useful! One example is
> nova-core's `NV_PFALCON_FALCON_HWCFG2::mem_scrubbing` field. It turns
> into `0` when scrubbing is completed, which is misleading. So to paliate
> that we introduced a `mem_scrubbing_done` method that works as we want,
> but the `mem_scrubbing` accessors are still present and can be called by
> driver code. Making them private would force all callers to use
> `mem_scrubbing_done`.
Sounds reasonable. I actually ran into this myself for the inverted 'valid'
bit in page directory entries in Nova. So what I ended up doing is calling it
valid_inverted. Though, agreed privatizing it is a bit better than calling it
mem_scrubbing_inverted, but that is another option if we don't want to
compilicate the macro more.
> Another related feature would be a way to make some fields read-only or
> write-only through an optional parameter.
Agreed, that would be useful.
> I'm just mentioning these for the record; I'm not suggesting they need
> to be done for the current series. :)
Sounds good. :)
thanks,
- Joel
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 3/4] nova-core: bitstruct: Add support for custom visiblity
2025-09-08 3:40 ` Alexandre Courbot
2025-09-08 3:46 ` Alexandre Courbot
@ 2025-09-09 18:30 ` Joel Fernandes
1 sibling, 0 replies; 28+ messages in thread
From: Joel Fernandes @ 2025-09-09 18:30 UTC (permalink / raw)
To: Alexandre Courbot
Cc: linux-kernel, dri-devel, dakr, Alistair Popple, Miguel Ojeda,
Alex Gaynor, Boqun Feng, Gary Guo, bjorn3_gh, Benno Lossin,
Andreas Hindborg, Alice Ryhl, Trevor Gross, David Airlie,
Simona Vetter, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, John Hubbard, Timur Tabi, joel, Elle Rhumsaa,
Daniel Almeida, nouveau, rust-for-linux
On Mon, Sep 08, 2025 at 12:40:17PM +0900, Alexandre Courbot wrote:
> On Thu Sep 4, 2025 at 6:54 AM JST, Joel Fernandes wrote:
> > Add support for custom visiblity to allow for users to control visibility
> > of the structure and helpers.
> >
> > Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
> > ---
> > drivers/gpu/nova-core/bitstruct.rs | 46 ++++++++++++++--------------
> > drivers/gpu/nova-core/regs/macros.rs | 16 +++++-----
> > 2 files changed, 31 insertions(+), 31 deletions(-)
> >
> > diff --git a/drivers/gpu/nova-core/bitstruct.rs b/drivers/gpu/nova-core/bitstruct.rs
> > index 068334c86981..1047c5c17e2d 100644
> > --- a/drivers/gpu/nova-core/bitstruct.rs
> > +++ b/drivers/gpu/nova-core/bitstruct.rs
> > @@ -9,7 +9,7 @@
> > ///
> > /// ```rust
> > /// bitstruct! {
> > -/// struct ControlReg: u32 {
> > +/// pub struct ControlReg: u32 {
> > /// 3:0 mode as u8 ?=> Mode;
> > /// 7:4 state as u8 => State;
> > /// }
>
> Maybe mention in the documentation that the field accessors are given
> the same visibility as the type - otherwise one might be led into
> thinking that they can specify visibility for individual fields as well
> (I'm wondering whether we might ever want that in the future?).
Sure, good idea, done.
> > @@ -34,21 +34,21 @@
> > /// and returns the result. This is useful with fields for which not all values are valid.
> > macro_rules! bitstruct {
> > // Main entry point - defines the bitfield struct with fields
> > - (struct $name:ident : $storage:ty $(, $comment:literal)? { $($fields:tt)* }) => {
> > - bitstruct!(@core $name $storage $(, $comment)? { $($fields)* });
> > + ($vis:vis struct $name:ident : $storage:ty $(, $comment:literal)? { $($fields:tt)* }) => {
> > + bitstruct!(@core $name $vis $storage $(, $comment)? { $($fields)* });
> > };
> >
> > // All rules below are helpers.
> >
> > // Defines the wrapper `$name` type, as well as its relevant implementations (`Debug`,
> > // `Default`, `BitOr`, and conversion to the value type) and field accessor methods.
> > - (@core $name:ident $storage:ty $(, $comment:literal)? { $($fields:tt)* }) => {
> > + (@core $name:ident $vis:vis $storage:ty $(, $comment:literal)? { $($fields:tt)* }) => {
>
> Being very nitpicky here, but for consistency why not put `$vis` before
> `$name` since it is the order they are given by the caller?
Perfect comment, changed it.
> > $(
> > #[doc=$comment]
> > )?
> > #[repr(transparent)]
> > #[derive(Clone, Copy)]
> > - pub(crate) struct $name($storage);
> > + $vis struct $name($vis $storage);
>
> `$storage` should probably be kept private - we already have accessors
> for it, and the visibility parameter is for the outer type, not its
> internals.
Already done for next revision. Thanks,
thanks,
- Joel
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v2 4/4] rust: Move register and bitstruct macros out of Nova
2025-09-03 21:54 [PATCH v2 0/4] Improve bitfield support in Rust Joel Fernandes
` (2 preceding siblings ...)
2025-09-03 21:54 ` [PATCH v2 3/4] nova-core: bitstruct: Add support for custom visiblity Joel Fernandes
@ 2025-09-03 21:54 ` Joel Fernandes
2025-09-03 21:56 ` Daniel Almeida
` (3 more replies)
3 siblings, 4 replies; 28+ messages in thread
From: Joel Fernandes @ 2025-09-03 21:54 UTC (permalink / raw)
To: linux-kernel, dri-devel, dakr, acourbot
Cc: Alistair Popple, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
bjorn3_gh, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, David Airlie, Simona Vetter, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, John Hubbard, Joel Fernandes,
Timur Tabi, joel, Elle Rhumsaa, Daniel Almeida, nouveau,
rust-for-linux
Out of broad need for these macros in Rust, move them out. Several folks
have shown interest (Nova, Tyr GPU drivers).
bitstruct - defines bitfields in Rust structs similar to C.
register - support for defining hardware registers and accessors.
Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
---
drivers/gpu/nova-core/falcon.rs | 2 +-
drivers/gpu/nova-core/falcon/gsp.rs | 3 +-
drivers/gpu/nova-core/falcon/sec2.rs | 2 +-
drivers/gpu/nova-core/nova_core.rs | 3 -
drivers/gpu/nova-core/regs.rs | 5 +-
.../nova-core => rust/kernel}/bitstruct.rs | 31 ++++---
rust/kernel/lib.rs | 2 +
rust/kernel/prelude.rs | 2 +
.../regs/macros.rs => rust/kernel/register.rs | 92 ++++++++++---------
9 files changed, 74 insertions(+), 68 deletions(-)
rename {drivers/gpu/nova-core => rust/kernel}/bitstruct.rs (92%)
rename drivers/gpu/nova-core/regs/macros.rs => rust/kernel/register.rs (90%)
diff --git a/drivers/gpu/nova-core/falcon.rs b/drivers/gpu/nova-core/falcon.rs
index be91aac6976a..06da6ce24482 100644
--- a/drivers/gpu/nova-core/falcon.rs
+++ b/drivers/gpu/nova-core/falcon.rs
@@ -7,6 +7,7 @@
use kernel::bindings;
use kernel::device;
use kernel::prelude::*;
+use kernel::register::RegisterBase;
use kernel::sync::aref::ARef;
use kernel::time::Delta;
@@ -14,7 +15,6 @@
use crate::driver::Bar0;
use crate::gpu::Chipset;
use crate::regs;
-use crate::regs::macros::RegisterBase;
use crate::util;
pub(crate) mod gsp;
diff --git a/drivers/gpu/nova-core/falcon/gsp.rs b/drivers/gpu/nova-core/falcon/gsp.rs
index f17599cb49fa..9287ab148da8 100644
--- a/drivers/gpu/nova-core/falcon/gsp.rs
+++ b/drivers/gpu/nova-core/falcon/gsp.rs
@@ -3,8 +3,9 @@
use crate::{
driver::Bar0,
falcon::{Falcon, FalconEngine, PFalcon2Base, PFalconBase},
- regs::{self, macros::RegisterBase},
+ regs,
};
+use kernel::register::RegisterBase;
/// Type specifying the `Gsp` falcon engine. Cannot be instantiated.
pub(crate) struct Gsp(());
diff --git a/drivers/gpu/nova-core/falcon/sec2.rs b/drivers/gpu/nova-core/falcon/sec2.rs
index 815786c8480d..8f7b63b6c2b2 100644
--- a/drivers/gpu/nova-core/falcon/sec2.rs
+++ b/drivers/gpu/nova-core/falcon/sec2.rs
@@ -1,7 +1,7 @@
// SPDX-License-Identifier: GPL-2.0
use crate::falcon::{FalconEngine, PFalcon2Base, PFalconBase};
-use crate::regs::macros::RegisterBase;
+use kernel::register::RegisterBase;
/// Type specifying the `Sec2` falcon engine. Cannot be instantiated.
pub(crate) struct Sec2(());
diff --git a/drivers/gpu/nova-core/nova_core.rs b/drivers/gpu/nova-core/nova_core.rs
index b218a2d42573..cb2bbb30cba1 100644
--- a/drivers/gpu/nova-core/nova_core.rs
+++ b/drivers/gpu/nova-core/nova_core.rs
@@ -2,9 +2,6 @@
//! Nova Core GPU Driver
-#[macro_use]
-mod bitstruct;
-
mod dma;
mod driver;
mod falcon;
diff --git a/drivers/gpu/nova-core/regs.rs b/drivers/gpu/nova-core/regs.rs
index 206dab2e1335..6d2f20623259 100644
--- a/drivers/gpu/nova-core/regs.rs
+++ b/drivers/gpu/nova-core/regs.rs
@@ -4,9 +4,6 @@
// but are mapped to types.
#![allow(non_camel_case_types)]
-#[macro_use]
-pub(crate) mod macros;
-
use crate::falcon::{
DmaTrfCmdSize, FalconCoreRev, FalconCoreRevSubversion, FalconFbifMemType, FalconFbifTarget,
FalconModSelAlgo, FalconSecurityModel, PFalcon2Base, PFalconBase, PeregrineCoreSelect,
@@ -331,6 +328,7 @@ pub(crate) fn mem_scrubbing_done(self) -> bool {
pub(crate) mod gm107 {
// FUSE
+ use kernel::prelude::*;
register!(NV_FUSE_STATUS_OPT_DISPLAY @ 0x00021c04 {
0:0 display_disabled as bool;
@@ -339,6 +337,7 @@ pub(crate) mod gm107 {
pub(crate) mod ga100 {
// FUSE
+ use kernel::prelude::*;
register!(NV_FUSE_STATUS_OPT_DISPLAY @ 0x00820c04 {
0:0 display_disabled as bool;
diff --git a/drivers/gpu/nova-core/bitstruct.rs b/rust/kernel/bitstruct.rs
similarity index 92%
rename from drivers/gpu/nova-core/bitstruct.rs
rename to rust/kernel/bitstruct.rs
index 1047c5c17e2d..06e5435df383 100644
--- a/drivers/gpu/nova-core/bitstruct.rs
+++ b/rust/kernel/bitstruct.rs
@@ -1,9 +1,9 @@
// SPDX-License-Identifier: GPL-2.0
-//
-// bitstruct.rs — Bitfield library for Rust structures
-//
-// A library that provides support for defining bit fields in Rust
-// structures. Also used from things that need bitfields like register macro.
+
+//! Bitfield library for Rust structures
+//!
+//! A library that provides support for defining bit fields in Rust
+//! structures. Also used from things that need bitfields like register macro.
///
/// # Syntax
///
@@ -32,6 +32,7 @@
/// the result.
/// - `as <type> ?=> <try_into_type>` calls `<try_into_type>`'s `TryFrom::<<type>>` implementation
/// and returns the result. This is useful with fields for which not all values are valid.
+#[macro_export]
macro_rules! bitstruct {
// Main entry point - defines the bitfield struct with fields
($vis:vis struct $name:ident : $storage:ty $(, $comment:literal)? { $($fields:tt)* }) => {
@@ -125,7 +126,7 @@ impl $name {
(@check_field_bounds $hi:tt:$lo:tt $field:ident as bool) => {
#[allow(clippy::eq_op)]
const _: () = {
- ::kernel::build_assert!(
+ build_assert!(
$hi == $lo,
concat!("boolean field `", stringify!($field), "` covers more than one bit")
);
@@ -136,7 +137,7 @@ impl $name {
(@check_field_bounds $hi:tt:$lo:tt $field:ident as $type:tt) => {
#[allow(clippy::eq_op)]
const _: () = {
- ::kernel::build_assert!(
+ build_assert!(
$hi >= $lo,
concat!("field `", stringify!($field), "`'s MSB is smaller than its LSB")
);
@@ -198,15 +199,15 @@ impl $name {
@leaf_accessor $name:ident $vis:vis $storage:ty, $hi:tt:$lo:tt $field:ident
{ $process:expr } $to_type:ty => $res_type:ty $(, $comment:literal)?;
) => {
- ::kernel::macros::paste!(
+ $crate::macros::paste!(
const [<$field:upper _RANGE>]: ::core::ops::RangeInclusive<u8> = $lo..=$hi;
const [<$field:upper _MASK>]: $storage = {
// Generate mask for shifting
match ::core::mem::size_of::<$storage>() {
- 1 => ::kernel::bits::genmask_u8($lo..=$hi) as $storage,
- 2 => ::kernel::bits::genmask_u16($lo..=$hi) as $storage,
- 4 => ::kernel::bits::genmask_u32($lo..=$hi) as $storage,
- 8 => ::kernel::bits::genmask_u64($lo..=$hi) as $storage,
+ 1 => $crate::bits::genmask_u8($lo..=$hi) as $storage,
+ 2 => $crate::bits::genmask_u16($lo..=$hi) as $storage,
+ 4 => $crate::bits::genmask_u32($lo..=$hi) as $storage,
+ 8 => $crate::bits::genmask_u64($lo..=$hi) as $storage,
_ => <$storage>::MAX
}
};
@@ -219,7 +220,7 @@ impl $name {
)?
#[inline(always)]
$vis fn $field(self) -> $res_type {
- ::kernel::macros::paste!(
+ $crate::macros::paste!(
const MASK: $storage = $name::[<$field:upper _MASK>];
const SHIFT: u32 = $name::[<$field:upper _SHIFT>];
);
@@ -228,7 +229,7 @@ impl $name {
$process(field)
}
- ::kernel::macros::paste!(
+ $crate::macros::paste!(
$(
#[doc="Sets the value of this field:"]
#[doc=$comment]
@@ -267,7 +268,7 @@ fn default() -> Self {
#[allow(unused_mut)]
let mut value = Self(Default::default());
- ::kernel::macros::paste!(
+ $crate::macros::paste!(
$(
value.[<set_ $field>](Default::default());
)*
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index c859a8984bae..9c492fa10967 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -64,6 +64,7 @@
#[cfg(CONFIG_AUXILIARY_BUS)]
pub mod auxiliary;
pub mod bits;
+pub mod bitstruct;
#[cfg(CONFIG_BLOCK)]
pub mod block;
pub mod bug;
@@ -112,6 +113,7 @@
pub mod prelude;
pub mod print;
pub mod rbtree;
+pub mod register;
pub mod regulator;
pub mod revocable;
pub mod security;
diff --git a/rust/kernel/prelude.rs b/rust/kernel/prelude.rs
index 25fe97aafd02..a98c7b7ab6af 100644
--- a/rust/kernel/prelude.rs
+++ b/rust/kernel/prelude.rs
@@ -39,6 +39,8 @@
pub use super::static_assert;
+pub use super::{bitstruct, register};
+
pub use super::error::{code::*, Error, Result};
pub use super::{str::CStr, ThisModule};
diff --git a/drivers/gpu/nova-core/regs/macros.rs b/rust/kernel/register.rs
similarity index 90%
rename from drivers/gpu/nova-core/regs/macros.rs
rename to rust/kernel/register.rs
index 22a53a73b765..1f48c5335e70 100644
--- a/drivers/gpu/nova-core/regs/macros.rs
+++ b/rust/kernel/register.rs
@@ -16,7 +16,8 @@
/// The `T` generic argument is used to distinguish which base to use, in case a type provides
/// several bases. It is given to the `register!` macro to restrict the use of the register to
/// implementors of this particular variant.
-pub(crate) trait RegisterBase<T> {
+pub trait RegisterBase<T> {
+ /// The base address for the register.
const BASE: usize;
}
@@ -281,6 +282,7 @@ pub(crate) trait RegisterBase<T> {
/// # Ok(())
/// # }
/// ```
+#[macro_export]
macro_rules! register {
// Creates a register at a fixed offset of the MMIO space.
($name:ident @ $offset:literal $(, $comment:literal)? { $($fields:tt)* } ) => {
@@ -378,7 +380,7 @@ impl $name {
/// Read the register from its address in `io`.
#[inline(always)]
pub(crate) fn read<const SIZE: usize, T>(io: &T) -> Self where
- T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>,
+ T: ::core::ops::Deref<Target = $crate::io::Io<SIZE>>,
{
Self(io.read32($offset))
}
@@ -386,7 +388,7 @@ pub(crate) fn read<const SIZE: usize, T>(io: &T) -> Self where
/// Write the value contained in `self` to the register address in `io`.
#[inline(always)]
pub(crate) fn write<const SIZE: usize, T>(self, io: &T) where
- T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>,
+ T: ::core::ops::Deref<Target = $crate::io::Io<SIZE>>,
{
io.write32(self.0, $offset)
}
@@ -398,7 +400,7 @@ pub(crate) fn alter<const SIZE: usize, T, F>(
io: &T,
f: F,
) where
- T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>,
+ T: ::core::ops::Deref<Target = $crate::io::Io<SIZE>>,
F: ::core::ops::FnOnce(Self) -> Self,
{
let reg = f(Self::read(io));
@@ -421,13 +423,13 @@ pub(crate) fn read<const SIZE: usize, T, B>(
#[allow(unused_variables)]
base: &B,
) -> Self where
- T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>,
- B: crate::regs::macros::RegisterBase<$base>,
+ T: ::core::ops::Deref<Target = $crate::io::Io<SIZE>>,
+ B: $crate::register::RegisterBase<$base>,
{
const OFFSET: usize = $name::OFFSET;
let value = io.read32(
- <B as crate::regs::macros::RegisterBase<$base>>::BASE + OFFSET
+ <B as $crate::register::RegisterBase<$base>>::BASE + OFFSET
);
Self(value)
@@ -442,14 +444,14 @@ pub(crate) fn write<const SIZE: usize, T, B>(
#[allow(unused_variables)]
base: &B,
) where
- T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>,
- B: crate::regs::macros::RegisterBase<$base>,
+ T: ::core::ops::Deref<Target = $crate::io::Io<SIZE>>,
+ B: $crate::register::RegisterBase<$base>,
{
const OFFSET: usize = $name::OFFSET;
io.write32(
self.0,
- <B as crate::regs::macros::RegisterBase<$base>>::BASE + OFFSET
+ <B as $crate::register::RegisterBase<$base>>::BASE + OFFSET
);
}
@@ -462,8 +464,8 @@ pub(crate) fn alter<const SIZE: usize, T, B, F>(
base: &B,
f: F,
) where
- T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>,
- B: crate::regs::macros::RegisterBase<$base>,
+ T: ::core::ops::Deref<Target = $crate::io::Io<SIZE>>,
+ B: $crate::register::RegisterBase<$base>,
F: ::core::ops::FnOnce(Self) -> Self,
{
let reg = f(Self::read(io, base));
@@ -486,7 +488,7 @@ pub(crate) fn read<const SIZE: usize, T>(
io: &T,
idx: usize,
) -> Self where
- T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>,
+ T: ::core::ops::Deref<Target = $crate::io::Io<SIZE>>,
{
build_assert!(idx < Self::SIZE);
@@ -503,7 +505,7 @@ pub(crate) fn write<const SIZE: usize, T>(
io: &T,
idx: usize
) where
- T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>,
+ T: ::core::ops::Deref<Target = $crate::io::Io<SIZE>>,
{
build_assert!(idx < Self::SIZE);
@@ -520,7 +522,7 @@ pub(crate) fn alter<const SIZE: usize, T, F>(
idx: usize,
f: F,
) where
- T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>,
+ T: ::core::ops::Deref<Target = $crate::io::Io<SIZE>>,
F: ::core::ops::FnOnce(Self) -> Self,
{
let reg = f(Self::read(io, idx));
@@ -535,13 +537,13 @@ pub(crate) fn alter<const SIZE: usize, T, F>(
pub(crate) fn try_read<const SIZE: usize, T>(
io: &T,
idx: usize,
- ) -> ::kernel::error::Result<Self> where
- T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>,
+ ) -> $crate::error::Result<Self> where
+ T: ::core::ops::Deref<Target = $crate::io::Io<SIZE>>,
{
if idx < Self::SIZE {
Ok(Self::read(io, idx))
} else {
- Err(EINVAL)
+ Err($crate::error::code::EINVAL)
}
}
@@ -554,13 +556,13 @@ pub(crate) fn try_write<const SIZE: usize, T>(
self,
io: &T,
idx: usize,
- ) -> ::kernel::error::Result where
- T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>,
+ ) -> $crate::error::Result where
+ T: ::core::ops::Deref<Target = $crate::io::Io<SIZE>>,
{
if idx < Self::SIZE {
Ok(self.write(io, idx))
} else {
- Err(EINVAL)
+ Err($crate::error::code::EINVAL)
}
}
@@ -574,14 +576,14 @@ pub(crate) fn try_alter<const SIZE: usize, T, F>(
io: &T,
idx: usize,
f: F,
- ) -> ::kernel::error::Result where
- T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>,
+ ) -> $crate::error::Result where
+ T: ::core::ops::Deref<Target = $crate::io::Io<SIZE>>,
F: ::core::ops::FnOnce(Self) -> Self,
{
if idx < Self::SIZE {
Ok(Self::alter(io, idx, f))
} else {
- Err(EINVAL)
+ Err($crate::error::code::EINVAL)
}
}
}
@@ -607,12 +609,12 @@ pub(crate) fn read<const SIZE: usize, T, B>(
base: &B,
idx: usize,
) -> Self where
- T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>,
- B: crate::regs::macros::RegisterBase<$base>,
+ T: ::core::ops::Deref<Target = $crate::io::Io<SIZE>>,
+ B: $crate::register::RegisterBase<$base>,
{
build_assert!(idx < Self::SIZE);
- let offset = <B as crate::regs::macros::RegisterBase<$base>>::BASE +
+ let offset = <B as $crate::register::RegisterBase<$base>>::BASE +
Self::OFFSET + (idx * Self::STRIDE);
let value = io.read32(offset);
@@ -629,12 +631,12 @@ pub(crate) fn write<const SIZE: usize, T, B>(
base: &B,
idx: usize
) where
- T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>,
- B: crate::regs::macros::RegisterBase<$base>,
+ T: ::core::ops::Deref<Target = $crate::io::Io<SIZE>>,
+ B: $crate::register::RegisterBase<$base>,
{
build_assert!(idx < Self::SIZE);
- let offset = <B as crate::regs::macros::RegisterBase<$base>>::BASE +
+ let offset = <B as $crate::register::RegisterBase<$base>>::BASE +
Self::OFFSET + (idx * Self::STRIDE);
io.write32(self.0, offset);
@@ -650,8 +652,8 @@ pub(crate) fn alter<const SIZE: usize, T, B, F>(
idx: usize,
f: F,
) where
- T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>,
- B: crate::regs::macros::RegisterBase<$base>,
+ T: ::core::ops::Deref<Target = $crate::io::Io<SIZE>>,
+ B: $crate::register::RegisterBase<$base>,
F: ::core::ops::FnOnce(Self) -> Self,
{
let reg = f(Self::read(io, base, idx));
@@ -668,14 +670,14 @@ pub(crate) fn try_read<const SIZE: usize, T, B>(
io: &T,
base: &B,
idx: usize,
- ) -> ::kernel::error::Result<Self> where
- T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>,
- B: crate::regs::macros::RegisterBase<$base>,
+ ) -> $crate::error::Result<Self> where
+ T: ::core::ops::Deref<Target = $crate::io::Io<SIZE>>,
+ B: $crate::register::RegisterBase<$base>,
{
if idx < Self::SIZE {
Ok(Self::read(io, base, idx))
} else {
- Err(EINVAL)
+ Err($crate::error::code::EINVAL)
}
}
@@ -690,14 +692,14 @@ pub(crate) fn try_write<const SIZE: usize, T, B>(
io: &T,
base: &B,
idx: usize,
- ) -> ::kernel::error::Result where
- T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>,
- B: crate::regs::macros::RegisterBase<$base>,
+ ) -> $crate::error::Result where
+ T: ::core::ops::Deref<Target = $crate::io::Io<SIZE>>,
+ B: $crate::register::RegisterBase<$base>,
{
if idx < Self::SIZE {
Ok(self.write(io, base, idx))
} else {
- Err(EINVAL)
+ Err($crate::error::code::EINVAL)
}
}
@@ -713,17 +715,19 @@ pub(crate) fn try_alter<const SIZE: usize, T, B, F>(
base: &B,
idx: usize,
f: F,
- ) -> ::kernel::error::Result where
- T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>,
- B: crate::regs::macros::RegisterBase<$base>,
+ ) -> $crate::error::Result where
+ T: ::core::ops::Deref<Target = $crate::io::Io<SIZE>>,
+ B: $crate::register::RegisterBase<$base>,
F: ::core::ops::FnOnce(Self) -> Self,
{
if idx < Self::SIZE {
Ok(Self::alter(io, base, idx, f))
} else {
- Err(EINVAL)
+ Err($crate::error::code::EINVAL)
}
}
}
};
}
+
+pub use register;
--
2.34.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH v2 4/4] rust: Move register and bitstruct macros out of Nova
2025-09-03 21:54 ` [PATCH v2 4/4] rust: Move register and bitstruct macros out of Nova Joel Fernandes
@ 2025-09-03 21:56 ` Daniel Almeida
2025-09-03 21:57 ` Joel Fernandes
2025-09-05 22:24 ` Elle Rhumsaa
` (2 subsequent siblings)
3 siblings, 1 reply; 28+ messages in thread
From: Daniel Almeida @ 2025-09-03 21:56 UTC (permalink / raw)
To: Joel Fernandes
Cc: linux-kernel, dri-devel, dakr, acourbot, Alistair Popple,
Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo, bjorn3_gh,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
David Airlie, Simona Vetter, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, John Hubbard, Timur Tabi, joel, Elle Rhumsaa,
nouveau, rust-for-linux
> On 3 Sep 2025, at 18:54, Joel Fernandes <joelagnelf@nvidia.com> wrote:
>
> Out of broad need for these macros in Rust, move them out. Several folks
> have shown interest (Nova, Tyr GPU drivers).
>
> bitstruct - defines bitfields in Rust structs similar to C.
> register - support for defining hardware registers and accessors.
>
> Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
Thanks a lot Joel, I will pick this up on Tyr and let you know how it went.
Expect a Tested-by tag in the coming days.
— Daniel
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v2 4/4] rust: Move register and bitstruct macros out of Nova
2025-09-03 21:56 ` Daniel Almeida
@ 2025-09-03 21:57 ` Joel Fernandes
0 siblings, 0 replies; 28+ messages in thread
From: Joel Fernandes @ 2025-09-03 21:57 UTC (permalink / raw)
To: Daniel Almeida
Cc: linux-kernel, dri-devel, dakr, acourbot, Alistair Popple,
Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo, bjorn3_gh,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
David Airlie, Simona Vetter, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, John Hubbard, Timur Tabi, joel, Elle Rhumsaa,
nouveau, rust-for-linux
On 9/3/2025 5:56 PM, Daniel Almeida wrote:
>
>
>> On 3 Sep 2025, at 18:54, Joel Fernandes <joelagnelf@nvidia.com> wrote:
>>
>> Out of broad need for these macros in Rust, move them out. Several folks
>> have shown interest (Nova, Tyr GPU drivers).
>>
>> bitstruct - defines bitfields in Rust structs similar to C.
>> register - support for defining hardware registers and accessors.
>>
>> Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
>
> Thanks a lot Joel, I will pick this up on Tyr and let you know how it went.
>
> Expect a Tested-by tag in the coming days.
>
Looking forward to it, thanks!
- Joel
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 4/4] rust: Move register and bitstruct macros out of Nova
2025-09-03 21:54 ` [PATCH v2 4/4] rust: Move register and bitstruct macros out of Nova Joel Fernandes
2025-09-03 21:56 ` Daniel Almeida
@ 2025-09-05 22:24 ` Elle Rhumsaa
2025-09-07 18:14 ` Miguel Ojeda
2025-09-08 3:52 ` Alexandre Courbot
3 siblings, 0 replies; 28+ messages in thread
From: Elle Rhumsaa @ 2025-09-05 22:24 UTC (permalink / raw)
To: Joel Fernandes
Cc: linux-kernel, dri-devel, dakr, acourbot, Alistair Popple,
Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo, bjorn3_gh,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
David Airlie, Simona Vetter, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, John Hubbard, Timur Tabi, joel, Daniel Almeida,
nouveau, rust-for-linux
On Wed, Sep 03, 2025 at 05:54:28PM -0400, Joel Fernandes wrote:
> Out of broad need for these macros in Rust, move them out. Several folks
> have shown interest (Nova, Tyr GPU drivers).
>
> bitstruct - defines bitfields in Rust structs similar to C.
> register - support for defining hardware registers and accessors.
>
> Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
> ---
> drivers/gpu/nova-core/falcon.rs | 2 +-
> drivers/gpu/nova-core/falcon/gsp.rs | 3 +-
> drivers/gpu/nova-core/falcon/sec2.rs | 2 +-
> drivers/gpu/nova-core/nova_core.rs | 3 -
> drivers/gpu/nova-core/regs.rs | 5 +-
> .../nova-core => rust/kernel}/bitstruct.rs | 31 ++++---
> rust/kernel/lib.rs | 2 +
> rust/kernel/prelude.rs | 2 +
> .../regs/macros.rs => rust/kernel/register.rs | 92 ++++++++++---------
> 9 files changed, 74 insertions(+), 68 deletions(-)
> rename {drivers/gpu/nova-core => rust/kernel}/bitstruct.rs (92%)
> rename drivers/gpu/nova-core/regs/macros.rs => rust/kernel/register.rs (90%)
>
> diff --git a/drivers/gpu/nova-core/falcon.rs b/drivers/gpu/nova-core/falcon.rs
> index be91aac6976a..06da6ce24482 100644
> --- a/drivers/gpu/nova-core/falcon.rs
> +++ b/drivers/gpu/nova-core/falcon.rs
> @@ -7,6 +7,7 @@
> use kernel::bindings;
> use kernel::device;
> use kernel::prelude::*;
> +use kernel::register::RegisterBase;
> use kernel::sync::aref::ARef;
> use kernel::time::Delta;
>
> @@ -14,7 +15,6 @@
> use crate::driver::Bar0;
> use crate::gpu::Chipset;
> use crate::regs;
> -use crate::regs::macros::RegisterBase;
> use crate::util;
>
> pub(crate) mod gsp;
> diff --git a/drivers/gpu/nova-core/falcon/gsp.rs b/drivers/gpu/nova-core/falcon/gsp.rs
> index f17599cb49fa..9287ab148da8 100644
> --- a/drivers/gpu/nova-core/falcon/gsp.rs
> +++ b/drivers/gpu/nova-core/falcon/gsp.rs
> @@ -3,8 +3,9 @@
> use crate::{
> driver::Bar0,
> falcon::{Falcon, FalconEngine, PFalcon2Base, PFalconBase},
> - regs::{self, macros::RegisterBase},
> + regs,
> };
> +use kernel::register::RegisterBase;
>
> /// Type specifying the `Gsp` falcon engine. Cannot be instantiated.
> pub(crate) struct Gsp(());
> diff --git a/drivers/gpu/nova-core/falcon/sec2.rs b/drivers/gpu/nova-core/falcon/sec2.rs
> index 815786c8480d..8f7b63b6c2b2 100644
> --- a/drivers/gpu/nova-core/falcon/sec2.rs
> +++ b/drivers/gpu/nova-core/falcon/sec2.rs
> @@ -1,7 +1,7 @@
> // SPDX-License-Identifier: GPL-2.0
>
> use crate::falcon::{FalconEngine, PFalcon2Base, PFalconBase};
> -use crate::regs::macros::RegisterBase;
> +use kernel::register::RegisterBase;
>
> /// Type specifying the `Sec2` falcon engine. Cannot be instantiated.
> pub(crate) struct Sec2(());
> diff --git a/drivers/gpu/nova-core/nova_core.rs b/drivers/gpu/nova-core/nova_core.rs
> index b218a2d42573..cb2bbb30cba1 100644
> --- a/drivers/gpu/nova-core/nova_core.rs
> +++ b/drivers/gpu/nova-core/nova_core.rs
> @@ -2,9 +2,6 @@
>
> //! Nova Core GPU Driver
>
> -#[macro_use]
> -mod bitstruct;
> -
> mod dma;
> mod driver;
> mod falcon;
> diff --git a/drivers/gpu/nova-core/regs.rs b/drivers/gpu/nova-core/regs.rs
> index 206dab2e1335..6d2f20623259 100644
> --- a/drivers/gpu/nova-core/regs.rs
> +++ b/drivers/gpu/nova-core/regs.rs
> @@ -4,9 +4,6 @@
> // but are mapped to types.
> #![allow(non_camel_case_types)]
>
> -#[macro_use]
> -pub(crate) mod macros;
> -
> use crate::falcon::{
> DmaTrfCmdSize, FalconCoreRev, FalconCoreRevSubversion, FalconFbifMemType, FalconFbifTarget,
> FalconModSelAlgo, FalconSecurityModel, PFalcon2Base, PFalconBase, PeregrineCoreSelect,
> @@ -331,6 +328,7 @@ pub(crate) fn mem_scrubbing_done(self) -> bool {
>
> pub(crate) mod gm107 {
> // FUSE
> + use kernel::prelude::*;
>
> register!(NV_FUSE_STATUS_OPT_DISPLAY @ 0x00021c04 {
> 0:0 display_disabled as bool;
> @@ -339,6 +337,7 @@ pub(crate) mod gm107 {
>
> pub(crate) mod ga100 {
> // FUSE
> + use kernel::prelude::*;
>
> register!(NV_FUSE_STATUS_OPT_DISPLAY @ 0x00820c04 {
> 0:0 display_disabled as bool;
> diff --git a/drivers/gpu/nova-core/bitstruct.rs b/rust/kernel/bitstruct.rs
> similarity index 92%
> rename from drivers/gpu/nova-core/bitstruct.rs
> rename to rust/kernel/bitstruct.rs
> index 1047c5c17e2d..06e5435df383 100644
> --- a/drivers/gpu/nova-core/bitstruct.rs
> +++ b/rust/kernel/bitstruct.rs
> @@ -1,9 +1,9 @@
> // SPDX-License-Identifier: GPL-2.0
> -//
> -// bitstruct.rs — Bitfield library for Rust structures
> -//
> -// A library that provides support for defining bit fields in Rust
> -// structures. Also used from things that need bitfields like register macro.
> +
> +//! Bitfield library for Rust structures
> +//!
> +//! A library that provides support for defining bit fields in Rust
> +//! structures. Also used from things that need bitfields like register macro.
> ///
> /// # Syntax
> ///
> @@ -32,6 +32,7 @@
> /// the result.
> /// - `as <type> ?=> <try_into_type>` calls `<try_into_type>`'s `TryFrom::<<type>>` implementation
> /// and returns the result. This is useful with fields for which not all values are valid.
> +#[macro_export]
> macro_rules! bitstruct {
> // Main entry point - defines the bitfield struct with fields
> ($vis:vis struct $name:ident : $storage:ty $(, $comment:literal)? { $($fields:tt)* }) => {
> @@ -125,7 +126,7 @@ impl $name {
> (@check_field_bounds $hi:tt:$lo:tt $field:ident as bool) => {
> #[allow(clippy::eq_op)]
> const _: () = {
> - ::kernel::build_assert!(
> + build_assert!(
> $hi == $lo,
> concat!("boolean field `", stringify!($field), "` covers more than one bit")
> );
> @@ -136,7 +137,7 @@ impl $name {
> (@check_field_bounds $hi:tt:$lo:tt $field:ident as $type:tt) => {
> #[allow(clippy::eq_op)]
> const _: () = {
> - ::kernel::build_assert!(
> + build_assert!(
> $hi >= $lo,
> concat!("field `", stringify!($field), "`'s MSB is smaller than its LSB")
> );
> @@ -198,15 +199,15 @@ impl $name {
> @leaf_accessor $name:ident $vis:vis $storage:ty, $hi:tt:$lo:tt $field:ident
> { $process:expr } $to_type:ty => $res_type:ty $(, $comment:literal)?;
> ) => {
> - ::kernel::macros::paste!(
> + $crate::macros::paste!(
> const [<$field:upper _RANGE>]: ::core::ops::RangeInclusive<u8> = $lo..=$hi;
> const [<$field:upper _MASK>]: $storage = {
> // Generate mask for shifting
> match ::core::mem::size_of::<$storage>() {
> - 1 => ::kernel::bits::genmask_u8($lo..=$hi) as $storage,
> - 2 => ::kernel::bits::genmask_u16($lo..=$hi) as $storage,
> - 4 => ::kernel::bits::genmask_u32($lo..=$hi) as $storage,
> - 8 => ::kernel::bits::genmask_u64($lo..=$hi) as $storage,
> + 1 => $crate::bits::genmask_u8($lo..=$hi) as $storage,
> + 2 => $crate::bits::genmask_u16($lo..=$hi) as $storage,
> + 4 => $crate::bits::genmask_u32($lo..=$hi) as $storage,
> + 8 => $crate::bits::genmask_u64($lo..=$hi) as $storage,
> _ => <$storage>::MAX
> }
> };
> @@ -219,7 +220,7 @@ impl $name {
> )?
> #[inline(always)]
> $vis fn $field(self) -> $res_type {
> - ::kernel::macros::paste!(
> + $crate::macros::paste!(
> const MASK: $storage = $name::[<$field:upper _MASK>];
> const SHIFT: u32 = $name::[<$field:upper _SHIFT>];
> );
> @@ -228,7 +229,7 @@ impl $name {
> $process(field)
> }
>
> - ::kernel::macros::paste!(
> + $crate::macros::paste!(
> $(
> #[doc="Sets the value of this field:"]
> #[doc=$comment]
> @@ -267,7 +268,7 @@ fn default() -> Self {
> #[allow(unused_mut)]
> let mut value = Self(Default::default());
>
> - ::kernel::macros::paste!(
> + $crate::macros::paste!(
> $(
> value.[<set_ $field>](Default::default());
> )*
> diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> index c859a8984bae..9c492fa10967 100644
> --- a/rust/kernel/lib.rs
> +++ b/rust/kernel/lib.rs
> @@ -64,6 +64,7 @@
> #[cfg(CONFIG_AUXILIARY_BUS)]
> pub mod auxiliary;
> pub mod bits;
> +pub mod bitstruct;
> #[cfg(CONFIG_BLOCK)]
> pub mod block;
> pub mod bug;
> @@ -112,6 +113,7 @@
> pub mod prelude;
> pub mod print;
> pub mod rbtree;
> +pub mod register;
> pub mod regulator;
> pub mod revocable;
> pub mod security;
> diff --git a/rust/kernel/prelude.rs b/rust/kernel/prelude.rs
> index 25fe97aafd02..a98c7b7ab6af 100644
> --- a/rust/kernel/prelude.rs
> +++ b/rust/kernel/prelude.rs
> @@ -39,6 +39,8 @@
>
> pub use super::static_assert;
>
> +pub use super::{bitstruct, register};
> +
> pub use super::error::{code::*, Error, Result};
>
> pub use super::{str::CStr, ThisModule};
> diff --git a/drivers/gpu/nova-core/regs/macros.rs b/rust/kernel/register.rs
> similarity index 90%
> rename from drivers/gpu/nova-core/regs/macros.rs
> rename to rust/kernel/register.rs
> index 22a53a73b765..1f48c5335e70 100644
> --- a/drivers/gpu/nova-core/regs/macros.rs
> +++ b/rust/kernel/register.rs
> @@ -16,7 +16,8 @@
> /// The `T` generic argument is used to distinguish which base to use, in case a type provides
> /// several bases. It is given to the `register!` macro to restrict the use of the register to
> /// implementors of this particular variant.
> -pub(crate) trait RegisterBase<T> {
> +pub trait RegisterBase<T> {
> + /// The base address for the register.
> const BASE: usize;
> }
>
> @@ -281,6 +282,7 @@ pub(crate) trait RegisterBase<T> {
> /// # Ok(())
> /// # }
> /// ```
> +#[macro_export]
> macro_rules! register {
> // Creates a register at a fixed offset of the MMIO space.
> ($name:ident @ $offset:literal $(, $comment:literal)? { $($fields:tt)* } ) => {
> @@ -378,7 +380,7 @@ impl $name {
> /// Read the register from its address in `io`.
> #[inline(always)]
> pub(crate) fn read<const SIZE: usize, T>(io: &T) -> Self where
> - T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>,
> + T: ::core::ops::Deref<Target = $crate::io::Io<SIZE>>,
> {
> Self(io.read32($offset))
> }
> @@ -386,7 +388,7 @@ pub(crate) fn read<const SIZE: usize, T>(io: &T) -> Self where
> /// Write the value contained in `self` to the register address in `io`.
> #[inline(always)]
> pub(crate) fn write<const SIZE: usize, T>(self, io: &T) where
> - T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>,
> + T: ::core::ops::Deref<Target = $crate::io::Io<SIZE>>,
> {
> io.write32(self.0, $offset)
> }
> @@ -398,7 +400,7 @@ pub(crate) fn alter<const SIZE: usize, T, F>(
> io: &T,
> f: F,
> ) where
> - T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>,
> + T: ::core::ops::Deref<Target = $crate::io::Io<SIZE>>,
> F: ::core::ops::FnOnce(Self) -> Self,
> {
> let reg = f(Self::read(io));
> @@ -421,13 +423,13 @@ pub(crate) fn read<const SIZE: usize, T, B>(
> #[allow(unused_variables)]
> base: &B,
> ) -> Self where
> - T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>,
> - B: crate::regs::macros::RegisterBase<$base>,
> + T: ::core::ops::Deref<Target = $crate::io::Io<SIZE>>,
> + B: $crate::register::RegisterBase<$base>,
> {
> const OFFSET: usize = $name::OFFSET;
>
> let value = io.read32(
> - <B as crate::regs::macros::RegisterBase<$base>>::BASE + OFFSET
> + <B as $crate::register::RegisterBase<$base>>::BASE + OFFSET
> );
>
> Self(value)
> @@ -442,14 +444,14 @@ pub(crate) fn write<const SIZE: usize, T, B>(
> #[allow(unused_variables)]
> base: &B,
> ) where
> - T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>,
> - B: crate::regs::macros::RegisterBase<$base>,
> + T: ::core::ops::Deref<Target = $crate::io::Io<SIZE>>,
> + B: $crate::register::RegisterBase<$base>,
> {
> const OFFSET: usize = $name::OFFSET;
>
> io.write32(
> self.0,
> - <B as crate::regs::macros::RegisterBase<$base>>::BASE + OFFSET
> + <B as $crate::register::RegisterBase<$base>>::BASE + OFFSET
> );
> }
>
> @@ -462,8 +464,8 @@ pub(crate) fn alter<const SIZE: usize, T, B, F>(
> base: &B,
> f: F,
> ) where
> - T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>,
> - B: crate::regs::macros::RegisterBase<$base>,
> + T: ::core::ops::Deref<Target = $crate::io::Io<SIZE>>,
> + B: $crate::register::RegisterBase<$base>,
> F: ::core::ops::FnOnce(Self) -> Self,
> {
> let reg = f(Self::read(io, base));
> @@ -486,7 +488,7 @@ pub(crate) fn read<const SIZE: usize, T>(
> io: &T,
> idx: usize,
> ) -> Self where
> - T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>,
> + T: ::core::ops::Deref<Target = $crate::io::Io<SIZE>>,
> {
> build_assert!(idx < Self::SIZE);
>
> @@ -503,7 +505,7 @@ pub(crate) fn write<const SIZE: usize, T>(
> io: &T,
> idx: usize
> ) where
> - T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>,
> + T: ::core::ops::Deref<Target = $crate::io::Io<SIZE>>,
> {
> build_assert!(idx < Self::SIZE);
>
> @@ -520,7 +522,7 @@ pub(crate) fn alter<const SIZE: usize, T, F>(
> idx: usize,
> f: F,
> ) where
> - T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>,
> + T: ::core::ops::Deref<Target = $crate::io::Io<SIZE>>,
> F: ::core::ops::FnOnce(Self) -> Self,
> {
> let reg = f(Self::read(io, idx));
> @@ -535,13 +537,13 @@ pub(crate) fn alter<const SIZE: usize, T, F>(
> pub(crate) fn try_read<const SIZE: usize, T>(
> io: &T,
> idx: usize,
> - ) -> ::kernel::error::Result<Self> where
> - T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>,
> + ) -> $crate::error::Result<Self> where
> + T: ::core::ops::Deref<Target = $crate::io::Io<SIZE>>,
> {
> if idx < Self::SIZE {
> Ok(Self::read(io, idx))
> } else {
> - Err(EINVAL)
> + Err($crate::error::code::EINVAL)
> }
> }
>
> @@ -554,13 +556,13 @@ pub(crate) fn try_write<const SIZE: usize, T>(
> self,
> io: &T,
> idx: usize,
> - ) -> ::kernel::error::Result where
> - T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>,
> + ) -> $crate::error::Result where
> + T: ::core::ops::Deref<Target = $crate::io::Io<SIZE>>,
> {
> if idx < Self::SIZE {
> Ok(self.write(io, idx))
> } else {
> - Err(EINVAL)
> + Err($crate::error::code::EINVAL)
> }
> }
>
> @@ -574,14 +576,14 @@ pub(crate) fn try_alter<const SIZE: usize, T, F>(
> io: &T,
> idx: usize,
> f: F,
> - ) -> ::kernel::error::Result where
> - T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>,
> + ) -> $crate::error::Result where
> + T: ::core::ops::Deref<Target = $crate::io::Io<SIZE>>,
> F: ::core::ops::FnOnce(Self) -> Self,
> {
> if idx < Self::SIZE {
> Ok(Self::alter(io, idx, f))
> } else {
> - Err(EINVAL)
> + Err($crate::error::code::EINVAL)
> }
> }
> }
> @@ -607,12 +609,12 @@ pub(crate) fn read<const SIZE: usize, T, B>(
> base: &B,
> idx: usize,
> ) -> Self where
> - T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>,
> - B: crate::regs::macros::RegisterBase<$base>,
> + T: ::core::ops::Deref<Target = $crate::io::Io<SIZE>>,
> + B: $crate::register::RegisterBase<$base>,
> {
> build_assert!(idx < Self::SIZE);
>
> - let offset = <B as crate::regs::macros::RegisterBase<$base>>::BASE +
> + let offset = <B as $crate::register::RegisterBase<$base>>::BASE +
> Self::OFFSET + (idx * Self::STRIDE);
> let value = io.read32(offset);
>
> @@ -629,12 +631,12 @@ pub(crate) fn write<const SIZE: usize, T, B>(
> base: &B,
> idx: usize
> ) where
> - T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>,
> - B: crate::regs::macros::RegisterBase<$base>,
> + T: ::core::ops::Deref<Target = $crate::io::Io<SIZE>>,
> + B: $crate::register::RegisterBase<$base>,
> {
> build_assert!(idx < Self::SIZE);
>
> - let offset = <B as crate::regs::macros::RegisterBase<$base>>::BASE +
> + let offset = <B as $crate::register::RegisterBase<$base>>::BASE +
> Self::OFFSET + (idx * Self::STRIDE);
>
> io.write32(self.0, offset);
> @@ -650,8 +652,8 @@ pub(crate) fn alter<const SIZE: usize, T, B, F>(
> idx: usize,
> f: F,
> ) where
> - T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>,
> - B: crate::regs::macros::RegisterBase<$base>,
> + T: ::core::ops::Deref<Target = $crate::io::Io<SIZE>>,
> + B: $crate::register::RegisterBase<$base>,
> F: ::core::ops::FnOnce(Self) -> Self,
> {
> let reg = f(Self::read(io, base, idx));
> @@ -668,14 +670,14 @@ pub(crate) fn try_read<const SIZE: usize, T, B>(
> io: &T,
> base: &B,
> idx: usize,
> - ) -> ::kernel::error::Result<Self> where
> - T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>,
> - B: crate::regs::macros::RegisterBase<$base>,
> + ) -> $crate::error::Result<Self> where
> + T: ::core::ops::Deref<Target = $crate::io::Io<SIZE>>,
> + B: $crate::register::RegisterBase<$base>,
> {
> if idx < Self::SIZE {
> Ok(Self::read(io, base, idx))
> } else {
> - Err(EINVAL)
> + Err($crate::error::code::EINVAL)
> }
> }
>
> @@ -690,14 +692,14 @@ pub(crate) fn try_write<const SIZE: usize, T, B>(
> io: &T,
> base: &B,
> idx: usize,
> - ) -> ::kernel::error::Result where
> - T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>,
> - B: crate::regs::macros::RegisterBase<$base>,
> + ) -> $crate::error::Result where
> + T: ::core::ops::Deref<Target = $crate::io::Io<SIZE>>,
> + B: $crate::register::RegisterBase<$base>,
> {
> if idx < Self::SIZE {
> Ok(self.write(io, base, idx))
> } else {
> - Err(EINVAL)
> + Err($crate::error::code::EINVAL)
> }
> }
>
> @@ -713,17 +715,19 @@ pub(crate) fn try_alter<const SIZE: usize, T, B, F>(
> base: &B,
> idx: usize,
> f: F,
> - ) -> ::kernel::error::Result where
> - T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>,
> - B: crate::regs::macros::RegisterBase<$base>,
> + ) -> $crate::error::Result where
> + T: ::core::ops::Deref<Target = $crate::io::Io<SIZE>>,
> + B: $crate::register::RegisterBase<$base>,
> F: ::core::ops::FnOnce(Self) -> Self,
> {
> if idx < Self::SIZE {
> Ok(Self::alter(io, base, idx, f))
> } else {
> - Err(EINVAL)
> + Err($crate::error::code::EINVAL)
> }
> }
> }
> };
> }
> +
> +pub use register;
> --
> 2.34.1
>
>
Reviewed-by: Elle Rhumsaa <elle@weathered-steel.dev>
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v2 4/4] rust: Move register and bitstruct macros out of Nova
2025-09-03 21:54 ` [PATCH v2 4/4] rust: Move register and bitstruct macros out of Nova Joel Fernandes
2025-09-03 21:56 ` Daniel Almeida
2025-09-05 22:24 ` Elle Rhumsaa
@ 2025-09-07 18:14 ` Miguel Ojeda
2025-09-08 17:06 ` Joel Fernandes
2025-09-08 3:52 ` Alexandre Courbot
3 siblings, 1 reply; 28+ messages in thread
From: Miguel Ojeda @ 2025-09-07 18:14 UTC (permalink / raw)
To: Joel Fernandes
Cc: linux-kernel, dri-devel, dakr, acourbot, Alistair Popple,
Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo, bjorn3_gh,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
David Airlie, Simona Vetter, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, John Hubbard, Timur Tabi, joel, Elle Rhumsaa,
Daniel Almeida, nouveau, rust-for-linux
Hi Joel,
I didn't check the macros, but a couple nits I noticed in this patch
in particular given it moved it to `kernel`...
On Wed, Sep 3, 2025 at 11:54 PM Joel Fernandes <joelagnelf@nvidia.com> wrote:
>
> +//! A library that provides support for defining bit fields in Rust
What about just "Support for defining bit fields in ..." or similar?
> +//! structures. Also used from things that need bitfields like register macro.
The "register macro" part sounds like it should be formatted with
Markdown plus an intra-doc link.
> - ::kernel::build_assert!(
> + build_assert!(
Is this path unqualified for some reason? Does it mean the user needs
to have imported the prelude?
> +pub use super::{bitstruct, register};
Please justify in the commit message why we want them in the prelude,
e.g. are they expected to be common? Does it simplify some code? etc.
Thanks!
Cheers,
Miguel
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v2 4/4] rust: Move register and bitstruct macros out of Nova
2025-09-07 18:14 ` Miguel Ojeda
@ 2025-09-08 17:06 ` Joel Fernandes
2025-09-08 18:39 ` Miguel Ojeda
0 siblings, 1 reply; 28+ messages in thread
From: Joel Fernandes @ 2025-09-08 17:06 UTC (permalink / raw)
To: Miguel Ojeda
Cc: linux-kernel, dri-devel, dakr, acourbot, Alistair Popple,
Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo, bjorn3_gh,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
David Airlie, Simona Vetter, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, John Hubbard, Timur Tabi, joel, Elle Rhumsaa,
Daniel Almeida, nouveau, rust-for-linux
On 9/7/2025 2:14 PM, Miguel Ojeda wrote:
> Hi Joel,
>
> I didn't check the macros, but a couple nits I noticed in this patch
> in particular given it moved it to `kernel`...
>
> On Wed, Sep 3, 2025 at 11:54 PM Joel Fernandes <joelagnelf@nvidia.com> wrote:
>>
>> +//! A library that provides support for defining bit fields in Rust
>
> What about just "Support for defining bit fields in ..." or similar?
>
>> +//! structures. Also used from things that need bitfields like register macro.
Ok, I changed it to:
"Also used from things that need bitfields like [`register!`] macro." for next
revision.
>
> The "register macro" part sounds like it should be formatted with
> Markdown plus an intra-doc link.
>
>> - ::kernel::build_assert!(
>> + build_assert!(
>
> Is this path unqualified for some reason? Does it mean the user needs
> to have imported the prelude?
Yes, for register macro importing prelude is required (I commented more below).
>
>> +pub use super::{bitstruct, register};
>
> Please justify in the commit message why we want them in the prelude,
> e.g. are they expected to be common? Does it simplify some code? etc.
>
The issue I ran into is, without adding it to prelude, the users of register!
macro will have to import both bitfield! and register! macros explictly, even
though they're only using register!. I tried to make it work without adding to
prelude, but couldn't:
use kernel::{bitfield, register};
Also not adding it to prelude, means register! macro has to invoke bitfield with
$crate prefixed ($crate::bitfield).
I think the prelude-way is clean, but let me know if there's any other trick I
can try.
I will also add this rationale to the commit message as you suggested.
Thanks!
- Joel
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v2 4/4] rust: Move register and bitstruct macros out of Nova
2025-09-08 17:06 ` Joel Fernandes
@ 2025-09-08 18:39 ` Miguel Ojeda
2025-09-08 20:40 ` Joel Fernandes
2025-09-08 21:24 ` Joel Fernandes
0 siblings, 2 replies; 28+ messages in thread
From: Miguel Ojeda @ 2025-09-08 18:39 UTC (permalink / raw)
To: Joel Fernandes
Cc: linux-kernel, dri-devel, dakr, acourbot, Alistair Popple,
Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo, bjorn3_gh,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
David Airlie, Simona Vetter, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, John Hubbard, Timur Tabi, joel, Elle Rhumsaa,
Daniel Almeida, nouveau, rust-for-linux
On Mon, Sep 8, 2025 at 7:06 PM Joel Fernandes <joelagnelf@nvidia.com> wrote:
>
> The issue I ran into is, without adding it to prelude, the users of register!
> macro will have to import both bitfield! and register! macros explictly, even
> though they're only using register!. I tried to make it work without adding to
> prelude, but couldn't:
>
> use kernel::{bitfield, register};
>
> Also not adding it to prelude, means register! macro has to invoke bitfield with
> $crate prefixed ($crate::bitfield).
I am not sure I follow -- macros should use qualified paths in general
so that they assume as little as possible from the calling
environment.
It should work without the prelude -- what didn't work?
Thanks!
Cheers,
Miguel
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v2 4/4] rust: Move register and bitstruct macros out of Nova
2025-09-08 18:39 ` Miguel Ojeda
@ 2025-09-08 20:40 ` Joel Fernandes
2025-09-08 21:24 ` Joel Fernandes
1 sibling, 0 replies; 28+ messages in thread
From: Joel Fernandes @ 2025-09-08 20:40 UTC (permalink / raw)
To: Miguel Ojeda
Cc: linux-kernel, dri-devel, dakr, acourbot, Alistair Popple,
Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo, bjorn3_gh,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
David Airlie, Simona Vetter, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, John Hubbard, Timur Tabi, joel, Elle Rhumsaa,
Daniel Almeida, nouveau, rust-for-linux
On 9/8/2025 2:39 PM, Miguel Ojeda wrote:
> On Mon, Sep 8, 2025 at 7:06 PM Joel Fernandes <joelagnelf@nvidia.com> wrote:
>>
>> The issue I ran into is, without adding it to prelude, the users of register!
>> macro will have to import both bitfield! and register! macros explictly, even
>> though they're only using register!. I tried to make it work without adding to
>> prelude, but couldn't:
>>
>> use kernel::{bitfield, register};
>>
>> Also not adding it to prelude, means register! macro has to invoke bitfield with
>> $crate prefixed ($crate::bitfield).
>
> I am not sure I follow -- macros should use qualified paths in general
> so that they assume as little as possible from the calling
> environment.
>
> It should work without the prelude -- what didn't work?
Ah, I guess my intent was to not use the qualified path. But that's better than
the users having to import prelude. Ok, thanks! - I will try this out.
- Joel
>
> Thanks!
>
> Cheers,
> Miguel
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v2 4/4] rust: Move register and bitstruct macros out of Nova
2025-09-08 18:39 ` Miguel Ojeda
2025-09-08 20:40 ` Joel Fernandes
@ 2025-09-08 21:24 ` Joel Fernandes
1 sibling, 0 replies; 28+ messages in thread
From: Joel Fernandes @ 2025-09-08 21:24 UTC (permalink / raw)
To: Miguel Ojeda
Cc: linux-kernel, dri-devel, dakr, acourbot, Alistair Popple,
Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo, bjorn3_gh,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
David Airlie, Simona Vetter, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, John Hubbard, Timur Tabi, joel, Elle Rhumsaa,
Daniel Almeida, nouveau, rust-for-linux
On Mon, Sep 08, 2025 at 08:39:19PM +0200, Miguel Ojeda wrote:
> On Mon, Sep 8, 2025 at 7:06 PM Joel Fernandes <joelagnelf@nvidia.com> wrote:
> >
> > The issue I ran into is, without adding it to prelude, the users of register!
> > macro will have to import both bitfield! and register! macros explictly, even
> > though they're only using register!. I tried to make it work without adding to
> > prelude, but couldn't:
> >
> > use kernel::{bitfield, register};
> >
> > Also not adding it to prelude, means register! macro has to invoke bitfield with
> > $crate prefixed ($crate::bitfield).
>
> I am not sure I follow -- macros should use qualified paths in general
> so that they assume as little as possible from the calling
> environment.
It works! Can you look if the below looks good to you now? No more prelude. Thanks!
---8<----
From: Joel Fernandes <joelagnelf@nvidia.com>
Subject: [PATCH] rust: Move register and bitfield macros out of Nova
Out of broad need for these macros in Rust, move them out. Several folks
have shown interest (Nova, Tyr GPU drivers).
bitfield moved into bits modules - defines bitfields in Rust structs similar to C.
register moved into io module - defines hardware registers and accessors.
Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
---
drivers/gpu/nova-core/falcon.rs | 2 +-
drivers/gpu/nova-core/falcon/gsp.rs | 3 +-
drivers/gpu/nova-core/falcon/sec2.rs | 2 +-
drivers/gpu/nova-core/nova_core.rs | 3 --
drivers/gpu/nova-core/regs.rs | 6 +--
rust/kernel/bits.rs | 2 +
.../kernel/bits}/bitfield.rs | 27 ++++++-----
rust/kernel/io.rs | 1 +
.../macros.rs => rust/kernel/io/register.rs | 48 ++++++++++---------
9 files changed, 50 insertions(+), 44 deletions(-)
rename {drivers/gpu/nova-core => rust/kernel/bits}/bitfield.rs (90%)
rename drivers/gpu/nova-core/regs/macros.rs => rust/kernel/io/register.rs (93%)
diff --git a/drivers/gpu/nova-core/falcon.rs b/drivers/gpu/nova-core/falcon.rs
index 938f25b556a8..c69bc2adb33c 100644
--- a/drivers/gpu/nova-core/falcon.rs
+++ b/drivers/gpu/nova-core/falcon.rs
@@ -7,6 +7,7 @@
use kernel::device;
use kernel::dma::DmaAddress;
use kernel::prelude::*;
+use kernel::io::register::RegisterBase;
use kernel::sync::aref::ARef;
use kernel::time::Delta;
@@ -14,7 +15,6 @@
use crate::driver::Bar0;
use crate::gpu::Chipset;
use crate::regs;
-use crate::regs::macros::RegisterBase;
use crate::util;
pub(crate) mod gsp;
diff --git a/drivers/gpu/nova-core/falcon/gsp.rs b/drivers/gpu/nova-core/falcon/gsp.rs
index c9ab375fd8a1..f315344b4256 100644
--- a/drivers/gpu/nova-core/falcon/gsp.rs
+++ b/drivers/gpu/nova-core/falcon/gsp.rs
@@ -1,12 +1,13 @@
// SPDX-License-Identifier: GPL-2.0
use kernel::prelude::*;
+use kernel::io::register::RegisterBase;
use kernel::time::Delta;
use crate::{
driver::Bar0,
falcon::{Falcon, FalconEngine, PFalcon2Base, PFalconBase},
- regs::{self, macros::RegisterBase},
+ regs,
util::wait_on,
};
diff --git a/drivers/gpu/nova-core/falcon/sec2.rs b/drivers/gpu/nova-core/falcon/sec2.rs
index 815786c8480d..81717868a8a8 100644
--- a/drivers/gpu/nova-core/falcon/sec2.rs
+++ b/drivers/gpu/nova-core/falcon/sec2.rs
@@ -1,7 +1,7 @@
// SPDX-License-Identifier: GPL-2.0
use crate::falcon::{FalconEngine, PFalcon2Base, PFalconBase};
-use crate::regs::macros::RegisterBase;
+use kernel::io::register::RegisterBase;
/// Type specifying the `Sec2` falcon engine. Cannot be instantiated.
pub(crate) struct Sec2(());
diff --git a/drivers/gpu/nova-core/nova_core.rs b/drivers/gpu/nova-core/nova_core.rs
index eaba6ad22f7a..4dbc7e5daae3 100644
--- a/drivers/gpu/nova-core/nova_core.rs
+++ b/drivers/gpu/nova-core/nova_core.rs
@@ -2,9 +2,6 @@
//! Nova Core GPU Driver
-#[macro_use]
-mod bitfield;
-
mod dma;
mod driver;
mod falcon;
diff --git a/drivers/gpu/nova-core/regs.rs b/drivers/gpu/nova-core/regs.rs
index c214f8056d6e..07533eb6f64e 100644
--- a/drivers/gpu/nova-core/regs.rs
+++ b/drivers/gpu/nova-core/regs.rs
@@ -4,15 +4,13 @@
// but are mapped to types.
#![allow(non_camel_case_types)]
-#[macro_use]
-pub(crate) mod macros;
-
use crate::falcon::{
DmaTrfCmdSize, FalconCoreRev, FalconCoreRevSubversion, FalconFbifMemType, FalconFbifTarget,
FalconModSelAlgo, FalconSecurityModel, PFalcon2Base, PFalconBase, PeregrineCoreSelect,
};
use crate::gpu::{Architecture, Chipset};
use kernel::prelude::*;
+use kernel::register;
// PMC
@@ -352,6 +350,7 @@ pub(crate) fn mem_scrubbing_done(self) -> bool {
pub(crate) mod gm107 {
// FUSE
+ use kernel::register;
register!(NV_FUSE_STATUS_OPT_DISPLAY @ 0x00021c04 {
0:0 display_disabled as bool;
@@ -360,6 +359,7 @@ pub(crate) mod gm107 {
pub(crate) mod ga100 {
// FUSE
+ use kernel::register;
register!(NV_FUSE_STATUS_OPT_DISPLAY @ 0x00820c04 {
0:0 display_disabled as bool;
diff --git a/rust/kernel/bits.rs b/rust/kernel/bits.rs
index 553d50265883..590a77d99ad7 100644
--- a/rust/kernel/bits.rs
+++ b/rust/kernel/bits.rs
@@ -201,3 +201,5 @@ pub const fn [<genmask_ $ty>](range: RangeInclusive<u32>) -> $ty {
/// assert_eq!(genmask_u8(0..=7), u8::MAX);
/// ```
);
+
+pub mod bitfield;
diff --git a/drivers/gpu/nova-core/bitfield.rs b/rust/kernel/bits/bitfield.rs
similarity index 90%
rename from drivers/gpu/nova-core/bitfield.rs
rename to rust/kernel/bits/bitfield.rs
index 2ca5f3725246..b49bfc1abcd5 100644
--- a/drivers/gpu/nova-core/bitfield.rs
+++ b/rust/kernel/bits/bitfield.rs
@@ -33,10 +33,13 @@
//! - `as <type> ?=> <try_into_type>` calls `<try_into_type>`'s `TryFrom::<<type>>` implementation
//! and returns the result. This is useful with fields for which not all values are valid.
//!
+
+/// bitfield macro definition
+#[macro_export]
macro_rules! bitfield {
// Main entry point - defines the bitfield struct with fields
($vis:vis struct $name:ident : $storage:ty $(, $comment:literal)? { $($fields:tt)* }) => {
- bitfield!(@core $name $vis $storage $(, $comment)? { $($fields)* });
+ ::kernel::bitfield!(@core $name $vis $storage $(, $comment)? { $($fields)* });
};
// All rules below are helpers.
@@ -71,7 +74,7 @@ fn from(val: $storage) -> Self {
}
}
- bitfield!(@fields_dispatcher $name $vis $storage { $($fields)* });
+ ::kernel::bitfield!(@fields_dispatcher $name $vis $storage { $($fields)* });
};
// Captures the fields and passes them to all the implementers that require field information.
@@ -87,7 +90,7 @@ fn from(val: $storage) -> Self {
)*
}
) => {
- bitfield!(@field_accessors $name $vis $storage {
+ ::kernel::bitfield!(@field_accessors $name $vis $storage {
$(
$hi:$lo $field as $type
$(?=> $try_into_type)?
@@ -96,8 +99,8 @@ fn from(val: $storage) -> Self {
;
)*
});
- bitfield!(@debug $name $storage { $($field;)* });
- bitfield!(@default $name $storage { $($field;)* });
+ ::kernel::bitfield!(@debug $name $storage { $($field;)* });
+ ::kernel::bitfield!(@default $name $storage { $($field;)* });
};
// Defines all the field getter/setter methods for `$name`.
@@ -112,7 +115,7 @@ fn from(val: $storage) -> Self {
}
) => {
$(
- bitfield!(@check_field_bounds $hi:$lo $field as $type);
+ ::kernel::bitfield!(@check_field_bounds $hi:$lo $field as $type);
)*
#[allow(dead_code)]
@@ -124,7 +127,7 @@ impl $name {
}
$(
- bitfield!(@field_accessor $name $vis $storage, $hi:$lo $field as $type
+ ::kernel::bitfield!(@field_accessor $name $vis $storage, $hi:$lo $field as $type
$(?=> $try_into_type)?
$(=> $into_type)?
$(, $comment)?
@@ -161,7 +164,7 @@ impl $name {
@field_accessor $name:ident $vis:vis $storage:ty, $hi:tt:$lo:tt $field:ident as bool => $into_type:ty
$(, $comment:literal)?;
) => {
- bitfield!(
+ ::kernel::bitfield!(
@leaf_accessor $name $vis $storage, $hi:$lo $field
{ |f| <$into_type>::from(if f != 0 { true } else { false }) }
$into_type => $into_type $(, $comment)?;
@@ -172,7 +175,7 @@ impl $name {
(
@field_accessor $name:ident $vis:vis $storage:ty, $hi:tt:$lo:tt $field:ident as bool $(, $comment:literal)?;
) => {
- bitfield!(@field_accessor $name $vis $storage, $hi:$lo $field as bool => bool $(, $comment)?;);
+ ::kernel::bitfield!(@field_accessor $name $vis $storage, $hi:$lo $field as bool => bool $(, $comment)?;);
};
// Catches the `?=>` syntax for non-boolean fields.
@@ -180,7 +183,7 @@ impl $name {
@field_accessor $name:ident $vis:vis $storage:ty, $hi:tt:$lo:tt $field:ident as $type:tt ?=> $try_into_type:ty
$(, $comment:literal)?;
) => {
- bitfield!(@leaf_accessor $name $vis $storage, $hi:$lo $field
+ ::kernel::bitfield!(@leaf_accessor $name $vis $storage, $hi:$lo $field
{ |f| <$try_into_type>::try_from(f as $type) } $try_into_type =>
::core::result::Result<
$try_into_type,
@@ -194,7 +197,7 @@ impl $name {
@field_accessor $name:ident $vis:vis $storage:ty, $hi:tt:$lo:tt $field:ident as $type:tt => $into_type:ty
$(, $comment:literal)?;
) => {
- bitfield!(@leaf_accessor $name $vis $storage, $hi:$lo $field
+ ::kernel::bitfield!(@leaf_accessor $name $vis $storage, $hi:$lo $field
{ |f| <$into_type>::from(f as $type) } $into_type => $into_type $(, $comment)?;);
};
@@ -203,7 +206,7 @@ impl $name {
@field_accessor $name:ident $vis:vis $storage:ty, $hi:tt:$lo:tt $field:ident as $type:tt
$(, $comment:literal)?;
) => {
- bitfield!(@field_accessor $name $vis $storage, $hi:$lo $field as $type => $type $(, $comment)?;);
+ ::kernel::bitfield!(@field_accessor $name $vis $storage, $hi:$lo $field as $type => $type $(, $comment)?;);
};
// Generates the accessor methods for a single field.
diff --git a/rust/kernel/io.rs b/rust/kernel/io.rs
index 03b467722b86..a79b603604b1 100644
--- a/rust/kernel/io.rs
+++ b/rust/kernel/io.rs
@@ -8,6 +8,7 @@
use crate::{bindings, build_assert, ffi::c_void};
pub mod mem;
+pub mod register;
pub mod resource;
pub use resource::Resource;
diff --git a/drivers/gpu/nova-core/regs/macros.rs b/rust/kernel/io/register.rs
similarity index 93%
rename from drivers/gpu/nova-core/regs/macros.rs
rename to rust/kernel/io/register.rs
index fab407c50668..a212b12de0f1 100644
--- a/drivers/gpu/nova-core/regs/macros.rs
+++ b/rust/kernel/io/register.rs
@@ -16,7 +16,8 @@
/// The `T` generic argument is used to distinguish which base to use, in case a type provides
/// several bases. It is given to the `register!` macro to restrict the use of the register to
/// implementors of this particular variant.
-pub(crate) trait RegisterBase<T> {
+pub trait RegisterBase<T> {
+ /// The base address for the register.
const BASE: usize;
}
@@ -55,7 +56,7 @@ pub(crate) trait RegisterBase<T> {
/// ```
///
///
-/// Please look at the [`bitfield`] macro for the complete syntax of fields definitions.
+/// Please look at the [`bitfield`][crate::bits::bitfield::bitfield] macro for the complete syntax of fields definitions.
///
/// The documentation strings are optional. If present, they will be added to the type's
/// definition, or the field getter and setter methods they are attached to.
@@ -275,28 +276,29 @@ pub(crate) trait RegisterBase<T> {
/// # Ok(())
/// # }
/// ```
+#[macro_export]
macro_rules! register {
// Creates a register at a fixed offset of the MMIO space.
($name:ident @ $offset:literal $(, $comment:literal)? { $($fields:tt)* } ) => {
- bitfield!(pub(crate) struct $name: u32 $(, $comment)? { $($fields)* } );
+ ::kernel::bitfield!(pub(crate) struct $name: u32 $(, $comment)? { $($fields)* } );
register!(@io_fixed $name @ $offset);
};
// Creates an alias register of fixed offset register `alias` with its own fields.
($name:ident => $alias:ident $(, $comment:literal)? { $($fields:tt)* } ) => {
- bitfield!(pub(crate) struct $name: u32 $(, $comment)? { $($fields)* } );
+ ::kernel::bitfield!(pub(crate) struct $name: u32 $(, $comment)? { $($fields)* } );
register!(@io_fixed $name @ $alias::OFFSET);
};
// Creates a register at a relative offset from a base address provider.
($name:ident @ $base:ty [ $offset:literal ] $(, $comment:literal)? { $($fields:tt)* } ) => {
- bitfield!(pub(crate) struct $name: u32 $(, $comment)? { $($fields)* } );
+ ::kernel::bitfield!(pub(crate) struct $name: u32 $(, $comment)? { $($fields)* } );
register!(@io_relative $name @ $base [ $offset ]);
};
// Creates an alias register of relative offset register `alias` with its own fields.
($name:ident => $base:ty [ $alias:ident ] $(, $comment:literal)? { $($fields:tt)* }) => {
- bitfield!(pub(crate) struct $name: u32 $(, $comment)? { $($fields)* } );
+ ::kernel::bitfield!(pub(crate) struct $name: u32 $(, $comment)? { $($fields)* } );
register!(@io_relative $name @ $base [ $alias::OFFSET ]);
};
@@ -307,7 +309,7 @@ macro_rules! register {
}
) => {
static_assert!(::core::mem::size_of::<u32>() <= $stride);
- bitfield!(pub(crate) struct $name: u32 $(, $comment)? { $($fields)* } );
+ ::kernel::bitfield!(pub(crate) struct $name: u32 $(, $comment)? { $($fields)* } );
register!(@io_array $name @ $offset [ $size ; $stride ]);
};
@@ -328,7 +330,7 @@ macro_rules! register {
$(, $comment:literal)? { $($fields:tt)* }
) => {
static_assert!(::core::mem::size_of::<u32>() <= $stride);
- bitfield!(pub(crate) struct $name: u32 $(, $comment)? { $($fields)* } );
+ ::kernel::bitfield!(pub(crate) struct $name: u32 $(, $comment)? { $($fields)* } );
register!(@io_relative_array $name @ $base [ $offset [ $size ; $stride ] ]);
};
@@ -350,7 +352,7 @@ macro_rules! register {
}
) => {
static_assert!($idx < $alias::SIZE);
- bitfield!(pub(crate) struct $name: u32 $(, $comment)? { $($fields)* } );
+ ::kernel::bitfield!(pub(crate) struct $name: u32 $(, $comment)? { $($fields)* } );
register!(@io_relative $name @ $base [ $alias::OFFSET + $idx * $alias::STRIDE ] );
};
@@ -359,7 +361,7 @@ macro_rules! register {
// to avoid it being interpreted in place of the relative register array alias rule.
($name:ident => $alias:ident [ $idx:expr ] $(, $comment:literal)? { $($fields:tt)* }) => {
static_assert!($idx < $alias::SIZE);
- bitfield!(pub(crate) struct $name: u32 $(, $comment)? { $($fields)* } );
+ ::kernel::bitfield!(pub(crate) struct $name: u32 $(, $comment)? { $($fields)* } );
register!(@io_fixed $name @ $alias::OFFSET + $idx * $alias::STRIDE );
};
@@ -416,12 +418,12 @@ pub(crate) fn read<const SIZE: usize, T, B>(
base: &B,
) -> Self where
T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>,
- B: crate::regs::macros::RegisterBase<$base>,
+ B: ::kernel::io::register::RegisterBase<$base>,
{
const OFFSET: usize = $name::OFFSET;
let value = io.read32(
- <B as crate::regs::macros::RegisterBase<$base>>::BASE + OFFSET
+ <B as ::kernel::io::register::RegisterBase<$base>>::BASE + OFFSET
);
Self(value)
@@ -437,13 +439,13 @@ pub(crate) fn write<const SIZE: usize, T, B>(
base: &B,
) where
T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>,
- B: crate::regs::macros::RegisterBase<$base>,
+ B: ::kernel::io::register::RegisterBase<$base>,
{
const OFFSET: usize = $name::OFFSET;
io.write32(
self.0,
- <B as crate::regs::macros::RegisterBase<$base>>::BASE + OFFSET
+ <B as ::kernel::io::register::RegisterBase<$base>>::BASE + OFFSET
);
}
@@ -457,7 +459,7 @@ pub(crate) fn alter<const SIZE: usize, T, B, F>(
f: F,
) where
T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>,
- B: crate::regs::macros::RegisterBase<$base>,
+ B: ::kernel::io::register::RegisterBase<$base>,
F: ::core::ops::FnOnce(Self) -> Self,
{
let reg = f(Self::read(io, base));
@@ -602,11 +604,11 @@ pub(crate) fn read<const SIZE: usize, T, B>(
idx: usize,
) -> Self where
T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>,
- B: crate::regs::macros::RegisterBase<$base>,
+ B: ::kernel::io::register::RegisterBase<$base>,
{
build_assert!(idx < Self::SIZE);
- let offset = <B as crate::regs::macros::RegisterBase<$base>>::BASE +
+ let offset = <B as ::kernel::io::register::RegisterBase<$base>>::BASE +
Self::OFFSET + (idx * Self::STRIDE);
let value = io.read32(offset);
@@ -624,11 +626,11 @@ pub(crate) fn write<const SIZE: usize, T, B>(
idx: usize
) where
T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>,
- B: crate::regs::macros::RegisterBase<$base>,
+ B: ::kernel::io::register::RegisterBase<$base>,
{
build_assert!(idx < Self::SIZE);
- let offset = <B as crate::regs::macros::RegisterBase<$base>>::BASE +
+ let offset = <B as ::kernel::io::register::RegisterBase<$base>>::BASE +
Self::OFFSET + (idx * Self::STRIDE);
io.write32(self.0, offset);
@@ -645,7 +647,7 @@ pub(crate) fn alter<const SIZE: usize, T, B, F>(
f: F,
) where
T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>,
- B: crate::regs::macros::RegisterBase<$base>,
+ B: ::kernel::io::register::RegisterBase<$base>,
F: ::core::ops::FnOnce(Self) -> Self,
{
let reg = f(Self::read(io, base, idx));
@@ -664,7 +666,7 @@ pub(crate) fn try_read<const SIZE: usize, T, B>(
idx: usize,
) -> ::kernel::error::Result<Self> where
T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>,
- B: crate::regs::macros::RegisterBase<$base>,
+ B: ::kernel::io::register::RegisterBase<$base>,
{
if idx < Self::SIZE {
Ok(Self::read(io, base, idx))
@@ -686,7 +688,7 @@ pub(crate) fn try_write<const SIZE: usize, T, B>(
idx: usize,
) -> ::kernel::error::Result where
T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>,
- B: crate::regs::macros::RegisterBase<$base>,
+ B: ::kernel::io::register::RegisterBase<$base>,
{
if idx < Self::SIZE {
Ok(self.write(io, base, idx))
@@ -709,7 +711,7 @@ pub(crate) fn try_alter<const SIZE: usize, T, B, F>(
f: F,
) -> ::kernel::error::Result where
T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>,
- B: crate::regs::macros::RegisterBase<$base>,
+ B: ::kernel::io::register::RegisterBase<$base>,
F: ::core::ops::FnOnce(Self) -> Self,
{
if idx < Self::SIZE {
--
2.34.1
^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [PATCH v2 4/4] rust: Move register and bitstruct macros out of Nova
2025-09-03 21:54 ` [PATCH v2 4/4] rust: Move register and bitstruct macros out of Nova Joel Fernandes
` (2 preceding siblings ...)
2025-09-07 18:14 ` Miguel Ojeda
@ 2025-09-08 3:52 ` Alexandre Courbot
2025-09-08 20:14 ` Joel Fernandes
3 siblings, 1 reply; 28+ messages in thread
From: Alexandre Courbot @ 2025-09-08 3:52 UTC (permalink / raw)
To: Joel Fernandes, linux-kernel, dri-devel, dakr
Cc: Alistair Popple, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
bjorn3_gh, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, David Airlie, Simona Vetter, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, John Hubbard, Timur Tabi, joel,
Elle Rhumsaa, Daniel Almeida, nouveau, rust-for-linux
On Thu Sep 4, 2025 at 6:54 AM JST, Joel Fernandes wrote:
> Out of broad need for these macros in Rust, move them out. Several folks
> have shown interest (Nova, Tyr GPU drivers).
>
> bitstruct - defines bitfields in Rust structs similar to C.
> register - support for defining hardware registers and accessors.
>
> Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
<snip>
> diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> index c859a8984bae..9c492fa10967 100644
> --- a/rust/kernel/lib.rs
> +++ b/rust/kernel/lib.rs
> @@ -64,6 +64,7 @@
> #[cfg(CONFIG_AUXILIARY_BUS)]
> pub mod auxiliary;
> pub mod bits;
> +pub mod bitstruct;
> #[cfg(CONFIG_BLOCK)]
> pub mod block;
> pub mod bug;
> @@ -112,6 +113,7 @@
> pub mod prelude;
> pub mod print;
> pub mod rbtree;
> +pub mod register;
I remember a discussion with Danilo where he mentioned the register
macro should end up being in `kernel::io::register`.
Also wondering whether `bitstruct` should not be in the
appropriately-named `bits` module standing right above it. :)
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v2 4/4] rust: Move register and bitstruct macros out of Nova
2025-09-08 3:52 ` Alexandre Courbot
@ 2025-09-08 20:14 ` Joel Fernandes
0 siblings, 0 replies; 28+ messages in thread
From: Joel Fernandes @ 2025-09-08 20:14 UTC (permalink / raw)
To: Alexandre Courbot
Cc: linux-kernel, dri-devel, dakr, Alistair Popple, Miguel Ojeda,
Alex Gaynor, Boqun Feng, Gary Guo, bjorn3_gh, Benno Lossin,
Andreas Hindborg, Alice Ryhl, Trevor Gross, David Airlie,
Simona Vetter, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, John Hubbard, Timur Tabi, joel, Elle Rhumsaa,
Daniel Almeida, nouveau, rust-for-linux
On Mon, Sep 08, 2025 at 12:52:57PM +0900, Alexandre Courbot wrote:
> On Thu Sep 4, 2025 at 6:54 AM JST, Joel Fernandes wrote:
> > Out of broad need for these macros in Rust, move them out. Several folks
> > have shown interest (Nova, Tyr GPU drivers).
> >
> > bitstruct - defines bitfields in Rust structs similar to C.
> > register - support for defining hardware registers and accessors.
> >
> > Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
> <snip>
>
> > diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> > index c859a8984bae..9c492fa10967 100644
> > --- a/rust/kernel/lib.rs
> > +++ b/rust/kernel/lib.rs
> > @@ -64,6 +64,7 @@
> > #[cfg(CONFIG_AUXILIARY_BUS)]
> > pub mod auxiliary;
> > pub mod bits;
> > +pub mod bitstruct;
> > #[cfg(CONFIG_BLOCK)]
> > pub mod block;
> > pub mod bug;
> > @@ -112,6 +113,7 @@
> > pub mod prelude;
> > pub mod print;
> > pub mod rbtree;
> > +pub mod register;
>
> I remember a discussion with Danilo where he mentioned the register
> macro should end up being in `kernel::io::register`.
I talked with Danilo, and I moved it to io for next revision.
> Also wondering whether `bitstruct` should not be in the
> appropriately-named `bits` module standing right above it. :)
I think now that I renamed it to bitfield, I'll move it to bits. That does
make a lot of sense.
thanks,
- Joel
^ permalink raw reply [flat|nested] 28+ messages in thread