* [PATCH v2 0/4] Improve bitfield support in Rust
@ 2025-09-03 21:54 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
` (3 more replies)
0 siblings, 4 replies; 12+ 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
These patches extract and enhance the bitfield support in the register macro
and introduces a new macro called bitstruct which allows to define Rust
structures with bitfields. This is extremely useful as it allows clean Rust
structure definitions without requiring explicit masks and shifts.
See [1] for an example of code I am working on.
[1] https://git.kernel.org/pub/scm/linux/kernel/git/jfern/linux.git/patch/?id=76797b31facae8f1a1be139412c78568df1da9f3
v1 of the patches is at:
https://lore.kernel.org/all/20250824135954.2243774-1-joelagnelf@nvidia.com/
v1->v2:
* Use build_assert in bitstruct
* Split move and enhance patches for easier review
* Move out of Nova into kernel crate for other drivers like Tyr which will use.
* Miscellaneous cosmetic improvements.
Joel Fernandes (4):
nova-core: bitstruct: Move bitfield-specific code from register! into
new macro
nova-core: bitstruct: Add support for different storage widths
nova-core: bitstruct: Add support for custom visiblity
rust: Move register and bitstruct macros out of Nova
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/regs.rs | 5 +-
rust/kernel/bitstruct.rs | 281 +++++++++++++++
rust/kernel/lib.rs | 2 +
rust/kernel/prelude.rs | 2 +
.../regs/macros.rs => rust/kernel/register.rs | 339 +++---------------
8 files changed, 347 insertions(+), 289 deletions(-)
create mode 100644 rust/kernel/bitstruct.rs
rename drivers/gpu/nova-core/regs/macros.rs => rust/kernel/register.rs (70%)
--
2.34.1
^ permalink raw reply [flat|nested] 12+ messages in thread
* [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-03 21:54 ` [PATCH v2 2/4] nova-core: bitstruct: Add support for different storage widths Joel Fernandes
` (2 subsequent siblings)
3 siblings, 0 replies; 12+ 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] 12+ 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-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, 1 reply; 12+ 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] 12+ 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
2025-09-05 22:23 ` Elle Rhumsaa
2025-09-03 21:54 ` [PATCH v2 4/4] rust: Move register and bitstruct macros out of Nova Joel Fernandes
3 siblings, 2 replies; 12+ 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] 12+ 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
` (2 more replies)
3 siblings, 3 replies; 12+ 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] 12+ 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
2025-09-07 18:14 ` Miguel Ojeda
2 siblings, 1 reply; 12+ 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] 12+ 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; 12+ 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] 12+ 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
0 siblings, 0 replies; 12+ 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] 12+ 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
1 sibling, 0 replies; 12+ 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] 12+ 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
1 sibling, 0 replies; 12+ 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] 12+ 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
2 siblings, 0 replies; 12+ 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] 12+ 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
2 siblings, 0 replies; 12+ 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] 12+ messages in thread
end of thread, other threads:[~2025-09-07 18:15 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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-05 22:21 ` Elle Rhumsaa
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-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
2025-09-07 18:14 ` Miguel Ojeda
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).