Re: [PATCH v3 16/16] rust: io: register: unify handling of register with/without bitfields

["Alexandre Courbot" <acourbot@nvidia.com>]

On Wed Aug 19, 2026 at 8:09 PM JST, Gary Guo wrote:
> Move the `FixedRegister` to be a property of register to become a property

s/to be/from being?

> of type. Name the new trait `FixedIoLoc` indicating if I/O location of a
> type is unique for a specific base. Thus, bitfields become just a special
> case of this (where type is unique because we're generating it in the
> register macro), and expose feature to registers without inline bitfield
> definition with the `#[unique]` attribute.
>
> Signed-off-by: Gary Guo <gary@garyguo.net>

The Sashiko findings are legit but not blocking, so:

Reviewed-by: Alexandre Courbot <acourbot@nvidia.com>

<...>
> -/// Helper function for register alias implementation.
> -///
> -/// This is used to enforce base matching. Only called during const eval.
> -#[doc(hidden)]
> -#[inline(always)]
> -pub const fn alias_offset<Base: ?Sized, Alias: FixedRegister<Base = Base>>() -> usize {
> -    Alias::OFFSET
> -}
> -
>  /// Helper function for register element alias implementation.
>  ///
>  /// This is used to enforce base matching and provide bounds checking. Only called during const
> @@ -419,6 +380,31 @@ pub const fn element_alias_offset<Base: ?Sized, Alias: RegisterArray<Base = Base
>  /// }
>  /// ```
>  ///
> +/// In case there is a fixed register associated with a specific type in the base, you can apply
> +/// `#[unique]` attribute which enables `write_reg` shorthand. This is automatically applied to
> +/// bitfields instantiated via the `register!` macro.

As Sashiko pointed out, let's add a sentence explaining that this is not
intended to be used on primitive types. I guess that could enable some
interesting syntax in the case of FIFO sub-regions, but generally
speaking this should be a Bad Idea.

<...>
> @@ -217,27 +230,30 @@ pub(crate) fn register(def: RegDef) -> Result<TokenStream> {
>          }
>  
>          match array {
> -            None if bitfield.is_none() => outputs.extend(quote!(
> -                #(#attrs)* #vis const #name: ::kernel::io::register::OffsetLoc<#base, #ty> =
> -                    ::kernel::io::register::OffsetLoc::new(#offset);
> -            )),
> -
> -            _ if bitfield.is_none() => Err(Error::new_spanned(
> -                ty,
> -                "defining without bitfield is not yet supported for this type of register",
> -            ))?,
> -
> -            None => outputs.extend(quote_spanned!(span =>
> -                impl ::kernel::io::register::FixedRegister for #name {
> -                    type Base = #base;
> -                    const OFFSET: usize = #offset;
> +            None => {
> +                if unique {
> +                    outputs.extend(quote!(
> +                        impl ::kernel::io::register::FixedIoLoc<#base> for #ty {
> +                            type Location = ::kernel::io::register::OffsetLoc<#base, #ty>;
> +                            const LOCATION: Self::Location = #name;
> +                        }
> +                    ))
>                  }
>  
> -                #(#attrs)* #vis const #name: ::kernel::io::register::FixedRegisterLoc<#name> =
> -                    ::kernel::io::register::FixedRegisterLoc::<#name>::new();
> -            )),
> +                outputs.extend(quote_spanned!(span =>
> +                    #(#attrs)* #vis const #name: ::kernel::io::register::OffsetLoc<#base, #ty> =
> +                        ::kernel::io::register::OffsetLoc::new(#offset);
> +                ));
> +            }
>  
>              Some(def) => {
> +                if !unique {
> +                    Err(Error::new_spanned(
> +                        &ty,
> +                        "defining without bitfield is not yet supported for this type of register",
> +                    ))?
> +                }

Sashiko's comment about the array validation bypass looks legit. IIUC
the guard also needs to test for `bitfield.is_none()`.