Add a `bits` method to return a copy of the underlying integer used
to represent the flags. This is useful when passing flag values to
C APIs.

The visibility of the generated `bits` method is controlled by an optional
visibility specifier in the inner-field position of the bitmask struct
declaration. Write `pub` there (e.g., `pub struct Foo(pub u32)`) to make
`bits` callable from outside the defining module; omit it to keep the bit
pattern hidden.

Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
---
@Daniel, I dropped your tag since the patch changed a bit.

Changes in v2:
- Rename the accessor from `into_inner` to `bits`, matching the
  name used by the `bitflags` crate (Gary).
- Tie the visibility of `bits` to the visibility of the inner
  field of the bitmask struct, so callers must explicitly write
  `pub` to expose the bit pattern outside the defining module (Gary).
- Extend the macro's doctest to declare the inner field `pub`
  and exercise the `bits` accessor.
- Link to v1: https://msgid.link/20260212-impl-flags-inner-v1-1-1e2edc96e470@kernel.org

To: Miguel Ojeda <ojeda@kernel.org>
To: Boqun Feng <boqun@kernel.org>
To: Gary Guo <gary@garyguo.net>
To: Björn Roy Baron <bjorn3_gh@protonmail.com>
To: Benno Lossin <lossin@kernel.org>
To: Andreas Hindborg <a.hindborg@kernel.org>
To: Alice Ryhl <aliceryhl@google.com>
To: Trevor Gross <tmgross@umich.edu>
To: Danilo Krummrich <dakr@kernel.org>
Cc: rust-for-linux@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 rust/kernel/impl_flags.rs | 27 +++++++++++++++++++++++++--
 1 file changed, 25 insertions(+), 2 deletions(-)

diff --git a/rust/kernel/impl_flags.rs b/rust/kernel/impl_flags.rs
index e2bd7639da12..1d39d54540dc 100644
--- a/rust/kernel/impl_flags.rs
+++ b/rust/kernel/impl_flags.rs
@@ -16,6 +16,12 @@
 ///   ([`::core::ops::BitOr`], [`::core::ops::BitAnd`], etc.).
 /// - Utility methods such as `.contains()` to check flags.
 ///
+/// An optional visibility specifier in the inner-field position of the
+/// bitmask struct controls the visibility of the generated `bits`
+/// accessor. Declare it `pub` (e.g., `pub struct Permissions(pub u32)`)
+/// to expose the raw integer outside the defining module — useful when
+/// passing the value to C APIs. Omit it to keep `bits` private.
+///
 /// # Examples
 ///
 /// ```
@@ -24,7 +30,7 @@
 /// impl_flags!(
 ///     /// Represents multiple permissions.
 ///     #[derive(Debug, Clone, Default, Copy, PartialEq, Eq)]
-///     pub struct Permissions(u32);
+///     pub struct Permissions(pub u32);
 ///
 ///     /// Represents a single permission.
 ///     #[derive(Debug, Clone, Copy, PartialEq, Eq)]
@@ -65,12 +71,18 @@
 /// let negated = !read_only;
 /// assert!(negated.contains(Permission::Write));
 /// assert!(!negated.contains(Permission::Read));
+///
+/// // The `pub` in `pub struct Permissions(pub u32)` makes `bits` callable
+/// // from outside the defining module, returning the raw integer
+/// // representation — useful for passing the value to a C API.
+/// let raw: u32 = read_only.bits();
+/// assert_eq!(raw, Permission::Read as u32);
 /// ```
 #[macro_export]
 macro_rules! impl_flags {
     (
         $(#[$outer_flags:meta])*
-        $vis_flags:vis struct $flags:ident($ty:ty);
+        $vis_flags:vis struct $flags:ident($inner_vis:vis $ty:ty);
 
         $(#[$outer_flag:meta])*
         $vis_flag:vis enum $flag:ident {
@@ -267,6 +279,17 @@ pub fn contains_any(self, flags: $flags) -> bool {
             pub fn contains_all(self, flags: $flags) -> bool {
                 (self.0 & flags.0) == flags.0
             }
+
+            /// Return a copy of the inner representation of the flags.
+            ///
+            /// The visibility of this method is controlled by the optional
+            /// visibility specifier in the inner-field position of the
+            /// bitmask struct declaration.
+            #[inline]
+            #[allow(dead_code)]
+            $inner_vis fn bits(self) -> $ty {
+                self.0
+            }
         }
     };
 }

---
base-commit: 7fd2df204f342fc17d1a0bfcd474b24232fb0f32
change-id: 20260212-impl-flags-inner-c61974b27b18

Best regards,
--  
Andreas Hindborg <a.hindborg@kernel.org>

On Fri, Jun 5, 2026 at 12:55 PM Andreas Hindborg <a.hindborg@kernel.org> wrote:
>
> +/// An optional visibility specifier in the inner-field position of the
> +/// bitmask struct controls the visibility of the generated `bits`
> +/// accessor. Declare it `pub` (e.g., `pub struct Permissions(pub u32)`)
> +/// to expose the raw integer outside the defining module — useful when
> +/// passing the value to C APIs. Omit it to keep `bits` private.

Hmm... It is clever, but it can be quite confusing. Could we use some
other way of "tagging" it?

I would also put `()` in `bits`, and make it a intra-doc links, to
make it extra clear it is the method we are talking about here.

And why do we call this "private", given the `From` as Sashiko points out?

i.e. your example works if you do (without the patch):

    /// let raw: u32 = read_only.into();
    /// assert_eq!(raw, Permission::Read as u32);

>  ///     /// Represents multiple permissions.
>  ///     #[derive(Debug, Clone, Default, Copy, PartialEq, Eq)]
> -///     pub struct Permissions(u32);
> +///     pub struct Permissions(pub u32);

I would prefer if we had separate examples for "optional features"
like this; otherwise, people may think this is the "common" way of
doing it -- and I assume in cases like this we want users to avoid
exposing if not needed, no?

> +            /// The visibility of this method is controlled by the optional
> +            /// visibility specifier in the inner-field position of the
> +            /// bitmask struct declaration.

I would make this paragraph a comment as Sashiko points out.

Cheers,
Miguel

On Sat Jun 6, 2026 at 12:01 AM BST, Miguel Ojeda wrote:
> On Fri, Jun 5, 2026 at 12:55 PM Andreas Hindborg <a.hindborg@kernel.org> wrote:
>>
>> +/// An optional visibility specifier in the inner-field position of the
>> +/// bitmask struct controls the visibility of the generated `bits`
>> +/// accessor. Declare it `pub` (e.g., `pub struct Permissions(pub u32)`)
>> +/// to expose the raw integer outside the defining module — useful when
>> +/// passing the value to C APIs. Omit it to keep `bits` private.
>
> Hmm... It is clever, but it can be quite confusing. Could we use some
> other way of "tagging" it?
>
> I would also put `()` in `bits`, and make it a intra-doc links, to
> make it extra clear it is the method we are talking about here.
>
> And why do we call this "private", given the `From` as Sashiko points out?

I think I suggested this approach because I don't want people to be able to
easily be able to get integers while enums could be used. But I didn't know that
the `From` impl already exists for flags.

Given the case we could probably just unconditionally expose the `bits` method
(and whatever mechanism we use to avoid exposing bits would also remove the
From/TryFrom/bits/all_bits/...)

Best,
Gary

>
> i.e. your example works if you do (without the patch):
>
>     /// let raw: u32 = read_only.into();
>     /// assert_eq!(raw, Permission::Read as u32);
>
>>  ///     /// Represents multiple permissions.
>>  ///     #[derive(Debug, Clone, Default, Copy, PartialEq, Eq)]
>> -///     pub struct Permissions(u32);
>> +///     pub struct Permissions(pub u32);
>
> I would prefer if we had separate examples for "optional features"
> like this; otherwise, people may think this is the "common" way of
> doing it -- and I assume in cases like this we want users to avoid
> exposing if not needed, no?
>
>> +            /// The visibility of this method is controlled by the optional
>> +            /// visibility specifier in the inner-field position of the
>> +            /// bitmask struct declaration.
>
> I would make this paragraph a comment as Sashiko points out.
>
> Cheers,
> Miguel

On Sat, Jun 6, 2026 at 2:22 PM Gary Guo <gary@garyguo.net> wrote:
>
> I think I suggested this approach because I don't want people to be able to
> easily be able to get integers while enums could be used. But I didn't know that
> the `From` impl already exists for flags.
>
> Given the case we could probably just unconditionally expose the `bits` method
> (and whatever mechanism we use to avoid exposing bits would also remove the
> From/TryFrom/bits/all_bits/...)

I see, so having it really private. That makes more sense, including
the `pub` word. Someone may still expect the field to be actually
public, but at least it makes sense, and using `enum`s and guiding
users to avoid exposing things they don't need to expose is usually
good a good principle.

Maybe another word (like your flavors in the other series :) could
also work... But I don't mind `pub` too much if the other option is
truly private.

Cheers,
Miguel