Re: [PATCH v2 11/16] rust: io: register: support fixed offset register without bitfield

On Wed Aug 5, 2026 at 5:50 PM BST, sashiko-bot wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Low] The `register!` macro documentation provides an example defining a FIFO register (`TX_FIFO`) with a `u32` type. This violates the subsystem guidelines which mandate that FIFOs must use stream accessors (no byteswapping). Using a `u32` register type implies standard register access (e.g., `writel`), which will byteswap on big-endian architectures and corrupt stream data. Additionally, the macro's syntax rule `$ty: ident $(:: $path_frag:ident)*` restricts the type to paths, preventing developers from directly specifying a byte array (e.g., `[u8; 4]`) as the type to properly model a byteswap-free stream.
> --
>
> --- Patch [11]: [PATCH v2 11/16] rust: io: register: support fixed offset register without bitfield ---
> commit a13a899311e93c249f5925eff4ce304f82a4cb69
> Author: Gary Guo <[email protected]>
>
> rust: io: register: support fixed offset register without bitfield
>
> Add a rule to allow creating `IoLoc` in `regiser!()` using an existing type
> and not create a bitfield. Add an example to demonstrate this for FIFO
> registers.
>
>> diff --git a/rust/kernel/io/register.rs b/rust/kernel/io/register.rs
>> index dc800fc711722..49a61da106c7d 100644
>> --- a/rust/kernel/io/register.rs
>> +++ b/rust/kernel/io/register.rs
>
> [ ... ]
>
>> @@ -499,6 +516,19 @@ fn into_io_op(self) -> (FixedRegisterLoc<T>, T) {
>>  /// In this example, `SCRATCH_BOOT_STATUS` uses the same I/O address as `SCRATCH`, while providing
>>  /// its own `completed` field.
>>  ///
>> +/// If you do not wish to have a bitfield defined, you can also create a register using an existing
>> +/// type.
>> +///
>> +/// ```no_run
>> +/// # use kernel::io::*;
>> +/// register! {
>> +///     base: Region<0x1000>;
>> +///
>> +///     /// TX FIFO register.
>> +///     pub TX_FIFO: u32 @ 0x00001000;
>
> [Severity: Low]
> Does this example promote an anti-pattern for FIFO access?
>
> Using u32 for a FIFO implies standard register I/O accessors (like writel or
> readl), which will perform byte-swapping on big-endian architectures.
> According to the I/O accessor subsystem guidelines, FIFOs should use stream
> accessors without byteswapping to prevent data corruption.

Sashiko is misunderstanding the use case here, this is for intent here is for
PIO FIFO where you read one word as a time for a fixed location, not streaming
(otherwise the signature here would be `pub TX_FIFO: [u32; FIFO_SIZE] @ ...`).

I'll see if I can have a better example in the next version. Perhaps

    pub UART_RX: u8 @ ...;

?

Best,
Gary

>
> Would it be better to avoid using a FIFO as the example here, or use a type
> that enforces stream semantics?
>
>> +/// }
>> +/// ```
>> +///
>>  /// ## Relative registers
>
> [ ... ]
>
>> @@ -826,6 +856,23 @@ fn into_io_op(self) -> (FixedRegisterLoc<T>, T) {
>>  macro_rules! register {
>>      (base: $reg_base:ty;) => {};
>>  
>> +    // Creates a register at a fixed offset of the MMIO space with provided type.
>> +    (
>> +        base: $reg_base:ty;
>> +        // `$ty` cannot be `:ty` due to follow-set restrictions.
>> +        $(#[$attr:meta])* $vis:vis $name:ident: $ty: ident $(:: $path_frag:ident)*
>
> [Severity: Low]
> Does this macro rule prevent developers from specifying inline byte arrays
> (like [u8; 4]) as the register type?
>
> Since the type matcher is restricted to identifiers and paths, it seems
> developers wouldn't be able to directly define a byte array to properly model
> a byteswap-free stream without defining a separate type alias first.
>
> Could this be adjusted to support array syntax, or is the limitation strict
> enough to require type aliases for stream buffers?
>
>> +            $(@ $offset:literal)?
>> +            $(=> $alias:path $([$alias_idx:expr])? )?;
>> +        $($rest:tt)*
>> +    ) => {