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.1On Thu Sep 4, 2025 at 6:54 AM JST, Joel Fernandes wrote:
> Add support for custom visiblity to allow for users to control visibility
> of the structure and helpers.
>
> Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
> ---
> drivers/gpu/nova-core/bitstruct.rs | 46 ++++++++++++++--------------
> drivers/gpu/nova-core/regs/macros.rs | 16 +++++-----
> 2 files changed, 31 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/gpu/nova-core/bitstruct.rs b/drivers/gpu/nova-core/bitstruct.rs
> index 068334c86981..1047c5c17e2d 100644
> --- a/drivers/gpu/nova-core/bitstruct.rs
> +++ b/drivers/gpu/nova-core/bitstruct.rs
> @@ -9,7 +9,7 @@
> ///
> /// ```rust
> /// bitstruct! {
> -/// struct ControlReg: u32 {
> +/// pub struct ControlReg: u32 {
> /// 3:0 mode as u8 ?=> Mode;
> /// 7:4 state as u8 => State;
> /// }
Maybe mention in the documentation that the field accessors are given
the same visibility as the type - otherwise one might be led into
thinking that they can specify visibility for individual fields as well
(I'm wondering whether we might ever want that in the future?).
> @@ -34,21 +34,21 @@
> /// and returns the result. This is useful with fields for which not all values are valid.
> macro_rules! bitstruct {
> // Main entry point - defines the bitfield struct with fields
> - (struct $name:ident : $storage:ty $(, $comment:literal)? { $($fields:tt)* }) => {
> - bitstruct!(@core $name $storage $(, $comment)? { $($fields)* });
> + ($vis:vis struct $name:ident : $storage:ty $(, $comment:literal)? { $($fields:tt)* }) => {
> + bitstruct!(@core $name $vis $storage $(, $comment)? { $($fields)* });
> };
>
> // All rules below are helpers.
>
> // Defines the wrapper `$name` type, as well as its relevant implementations (`Debug`,
> // `Default`, `BitOr`, and conversion to the value type) and field accessor methods.
> - (@core $name:ident $storage:ty $(, $comment:literal)? { $($fields:tt)* }) => {
> + (@core $name:ident $vis:vis $storage:ty $(, $comment:literal)? { $($fields:tt)* }) => {
Being very nitpicky here, but for consistency why not put `$vis` before
`$name` since it is the order they are given by the caller?
> $(
> #[doc=$comment]
> )?
> #[repr(transparent)]
> #[derive(Clone, Copy)]
> - pub(crate) struct $name($storage);
> + $vis struct $name($vis $storage);
`$storage` should probably be kept private - we already have accessors
for it, and the visibility parameter is for the outer type, not its
internals.
On Mon, Sep 08, 2025 at 12:40:17PM +0900, Alexandre Courbot wrote:
> On Thu Sep 4, 2025 at 6:54 AM JST, Joel Fernandes wrote:
> > Add support for custom visiblity to allow for users to control visibility
> > of the structure and helpers.
> >
> > Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
> > ---
> > drivers/gpu/nova-core/bitstruct.rs | 46 ++++++++++++++--------------
> > drivers/gpu/nova-core/regs/macros.rs | 16 +++++-----
> > 2 files changed, 31 insertions(+), 31 deletions(-)
> >
> > diff --git a/drivers/gpu/nova-core/bitstruct.rs b/drivers/gpu/nova-core/bitstruct.rs
> > index 068334c86981..1047c5c17e2d 100644
> > --- a/drivers/gpu/nova-core/bitstruct.rs
> > +++ b/drivers/gpu/nova-core/bitstruct.rs
> > @@ -9,7 +9,7 @@
> > ///
> > /// ```rust
> > /// bitstruct! {
> > -/// struct ControlReg: u32 {
> > +/// pub struct ControlReg: u32 {
> > /// 3:0 mode as u8 ?=> Mode;
> > /// 7:4 state as u8 => State;
> > /// }
>
> Maybe mention in the documentation that the field accessors are given
> the same visibility as the type - otherwise one might be led into
> thinking that they can specify visibility for individual fields as well
> (I'm wondering whether we might ever want that in the future?).
Sure, good idea, done.
> > @@ -34,21 +34,21 @@
> > /// and returns the result. This is useful with fields for which not all values are valid.
> > macro_rules! bitstruct {
> > // Main entry point - defines the bitfield struct with fields
> > - (struct $name:ident : $storage:ty $(, $comment:literal)? { $($fields:tt)* }) => {
> > - bitstruct!(@core $name $storage $(, $comment)? { $($fields)* });
> > + ($vis:vis struct $name:ident : $storage:ty $(, $comment:literal)? { $($fields:tt)* }) => {
> > + bitstruct!(@core $name $vis $storage $(, $comment)? { $($fields)* });
> > };
> >
> > // All rules below are helpers.
> >
> > // Defines the wrapper `$name` type, as well as its relevant implementations (`Debug`,
> > // `Default`, `BitOr`, and conversion to the value type) and field accessor methods.
> > - (@core $name:ident $storage:ty $(, $comment:literal)? { $($fields:tt)* }) => {
> > + (@core $name:ident $vis:vis $storage:ty $(, $comment:literal)? { $($fields:tt)* }) => {
>
> Being very nitpicky here, but for consistency why not put `$vis` before
> `$name` since it is the order they are given by the caller?
Perfect comment, changed it.
> > $(
> > #[doc=$comment]
> > )?
> > #[repr(transparent)]
> > #[derive(Clone, Copy)]
> > - pub(crate) struct $name($storage);
> > + $vis struct $name($vis $storage);
>
> `$storage` should probably be kept private - we already have accessors
> for it, and the visibility parameter is for the outer type, not its
> internals.
Already done for next revision. Thanks,
thanks,
- Joel
On Mon Sep 8, 2025 at 12:40 PM JST, Alexandre Courbot wrote:
> On Thu Sep 4, 2025 at 6:54 AM JST, Joel Fernandes wrote:
>> Add support for custom visiblity to allow for users to control visibility
>> of the structure and helpers.
>>
>> Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
>> ---
>> drivers/gpu/nova-core/bitstruct.rs | 46 ++++++++++++++--------------
>> drivers/gpu/nova-core/regs/macros.rs | 16 +++++-----
>> 2 files changed, 31 insertions(+), 31 deletions(-)
>>
>> diff --git a/drivers/gpu/nova-core/bitstruct.rs b/drivers/gpu/nova-core/bitstruct.rs
>> index 068334c86981..1047c5c17e2d 100644
>> --- a/drivers/gpu/nova-core/bitstruct.rs
>> +++ b/drivers/gpu/nova-core/bitstruct.rs
>> @@ -9,7 +9,7 @@
>> ///
>> /// ```rust
>> /// bitstruct! {
>> -/// struct ControlReg: u32 {
>> +/// pub struct ControlReg: u32 {
>> /// 3:0 mode as u8 ?=> Mode;
>> /// 7:4 state as u8 => State;
>> /// }
>
> Maybe mention in the documentation that the field accessors are given
> the same visibility as the type - otherwise one might be led into
> thinking that they can specify visibility for individual fields as well
> (I'm wondering whether we might ever want that in the future?).
Answering my own question: it could be useful! One example is
nova-core's `NV_PFALCON_FALCON_HWCFG2::mem_scrubbing` field. It turns
into `0` when scrubbing is completed, which is misleading. So to paliate
that we introduced a `mem_scrubbing_done` method that works as we want,
but the `mem_scrubbing` accessors are still present and can be called by
driver code. Making them private would force all callers to use
`mem_scrubbing_done`.
Another related feature would be a way to make some fields read-only or
write-only through an optional parameter.
I'm just mentioning these for the record; I'm not suggesting they need
to be done for the current series. :)
On Mon, Sep 08, 2025 at 12:46:57PM +0900, Alexandre Courbot wrote:
> On Mon Sep 8, 2025 at 12:40 PM JST, Alexandre Courbot wrote:
> > On Thu Sep 4, 2025 at 6:54 AM JST, Joel Fernandes wrote:
> >> Add support for custom visiblity to allow for users to control visibility
> >> of the structure and helpers.
> >>
> >> Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
> >> ---
> >> drivers/gpu/nova-core/bitstruct.rs | 46 ++++++++++++++--------------
> >> drivers/gpu/nova-core/regs/macros.rs | 16 +++++-----
> >> 2 files changed, 31 insertions(+), 31 deletions(-)
> >>
> >> diff --git a/drivers/gpu/nova-core/bitstruct.rs b/drivers/gpu/nova-core/bitstruct.rs
> >> index 068334c86981..1047c5c17e2d 100644
> >> --- a/drivers/gpu/nova-core/bitstruct.rs
> >> +++ b/drivers/gpu/nova-core/bitstruct.rs
> >> @@ -9,7 +9,7 @@
> >> ///
> >> /// ```rust
> >> /// bitstruct! {
> >> -/// struct ControlReg: u32 {
> >> +/// pub struct ControlReg: u32 {
> >> /// 3:0 mode as u8 ?=> Mode;
> >> /// 7:4 state as u8 => State;
> >> /// }
> >
> > Maybe mention in the documentation that the field accessors are given
> > the same visibility as the type - otherwise one might be led into
> > thinking that they can specify visibility for individual fields as well
> > (I'm wondering whether we might ever want that in the future?).
>
> Answering my own question: it could be useful! One example is
> nova-core's `NV_PFALCON_FALCON_HWCFG2::mem_scrubbing` field. It turns
> into `0` when scrubbing is completed, which is misleading. So to paliate
> that we introduced a `mem_scrubbing_done` method that works as we want,
> but the `mem_scrubbing` accessors are still present and can be called by
> driver code. Making them private would force all callers to use
> `mem_scrubbing_done`.
Sounds reasonable. I actually ran into this myself for the inverted 'valid'
bit in page directory entries in Nova. So what I ended up doing is calling it
valid_inverted. Though, agreed privatizing it is a bit better than calling it
mem_scrubbing_inverted, but that is another option if we don't want to
compilicate the macro more.
> Another related feature would be a way to make some fields read-only or
> write-only through an optional parameter.
Agreed, that would be useful.
> I'm just mentioning these for the record; I'm not suggesting they need
> to be done for the current series. :)
Sounds good. :)
thanks,
- Joel
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>
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>
© 2016 - 2025 Red Hat, Inc.