Message-Id: <DFUCNHHYB6VV.43MOY4Y1NV06@garyguo.net>
Date: Wed, 21 Jan 2026 14:50:19 +0000
From: "Gary Guo" <gary@...yguo.net>
To: "Alexandre Courbot" <acourbot@...dia.com>, "Danilo Krummrich"
 <dakr@...nel.org>, "Alice Ryhl" <aliceryhl@...gle.com>, "Daniel Almeida"
 <daniel.almeida@...labora.com>, "Miguel Ojeda" <ojeda@...nel.org>, "Boqun
 Feng" <boqun.feng@...il.com>, "Gary Guo" <gary@...yguo.net>,
 Björn Roy Baron <bjorn3_gh@...tonmail.com>, "Benno Lossin"
 <lossin@...nel.org>, "Andreas Hindborg" <a.hindborg@...nel.org>, "Trevor
 Gross" <tmgross@...ch.edu>
Cc: "Yury Norov" <yury.norov@...il.com>, "John Hubbard"
 <jhubbard@...dia.com>, "Alistair Popple" <apopple@...dia.com>, "Joel
 Fernandes" <joelagnelf@...dia.com>, "Timur Tabi" <ttabi@...dia.com>, "Edwin
 Peer" <epeer@...dia.com>, "Eliot Courtney" <ecourtney@...dia.com>, "Dirk
 Behme" <dirk.behme@...bosch.com>, "Steven Price" <steven.price@....com>,
 <rust-for-linux@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 4/5] rust: io: add `register!` macro

On Wed Jan 21, 2026 at 7:23 AM GMT, Alexandre Courbot wrote:
> Add a macro for defining hardware register types with I/O accessors.
>
> Each register field is represented as a `Bounded` of the appropriate bit
> width, ensuring field values are never silently truncated.
>
> Fields can optionally be converted to/from custom types, either fallibly
> or infallibly.
>
> The address of registers can be direct, relative, or indexed, supporting
> most of the patterns in which registers are arranged.
>
> Signed-off-by: Alexandre Courbot <acourbot@...dia.com>
> ---
>  rust/kernel/io.rs          |    1 +
>  rust/kernel/io/register.rs | 1198 ++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 1199 insertions(+)
>
> diff --git a/rust/kernel/io.rs b/rust/kernel/io.rs
> index a97eb44a9a87..eccaa176b6b9 100644
> --- a/rust/kernel/io.rs
> +++ b/rust/kernel/io.rs
> @@ -11,6 +11,7 @@
>  
>  pub mod mem;
>  pub mod poll;
> +pub mod register;
>  pub mod resource;
>  
>  pub use resource::Resource;
> diff --git a/rust/kernel/io/register.rs b/rust/kernel/io/register.rs
> new file mode 100644
> index 000000000000..e414aebe4c86
> --- /dev/null
> +++ b/rust/kernel/io/register.rs
> @@ -0,0 +1,1198 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +/// Defines a dedicated type for a register with an absolute offset, including getter and setter
> +/// methods for its fields and methods to read and write it from an `Io` region.
> +///
> +/// A register is essentially a [`bitfield!`] with I/O capabilities. The syntax of the `register!`
> +/// macro reflects that fact, being essentially identical to that of [`bitfield!`] with the
> +/// addition of addressing information after the `@` token.
> +///
> +/// Example:
> +///
> +/// ```
> +/// use kernel::register;
> +///
> +/// register!(pub BOOT_0(u32) @ 0x00000100, "Basic revision information about the chip" {
> +///     7:4 major_revision, "Major revision of the chip";
> +///     3:0 minor_revision, "Minor revision of the chip";
> +/// });

The comment is inserted as doc comment, but it uses the string syntax.

I guess the idea is that you want write everything in a single line so you can
visually align the fields? I think it
looks fine on the fields, but the same-line documentation of the type itself
looks a bit off.

Something like this will definitely feel much more Rusty:

register!(
    /// Basic revision information about the chip.
    pub struct BOOT_0(u32) @ 0x00000100 {
        /// Major revision of the chip.
        major_version: [7:4],
        /// Minor revision of the chip.
        ///
        /// This would also allow you easily expand the documentation into
        /// multiple lines!
        ///
        /// Perhaps useful to document some quirks about the register!
        /// I know currently registers and their fields are very underdocumented
        /// and they probably don't need multiple lines, but I hope that'll not
        /// true in the future and we would have detailed docs in the driver --
        /// in which case visually aligning becomes impossible anyway.
        minor_version: [3:0],
        //           ^~ closer to the variable syntax in Rust
        //             ^~ I keep the hi:lo syntax which I suppose is to reflect Verilog.
    }
)

Another top-level question I have is whether we should define multiple registers
(perhaps all) in one macro invocation. That's the strategy of `tock-registers`
crate which is widely used in embedded Rust.

(Although, they use a completely different strategy in generating register
mapping; all registers are becoming fields in the same struct).

> +/// ```
> +///
> +/// This defines a `BOOT_0` type which can be read or written from offset `0x100` of an `Io`
> +/// region. For instance, `minor_revision` is made of the 4 least significant bits of the
> +/// register. Each field can be accessed and modified using accessor
> +/// methods:
> +///
> +/// ```no_run
> +/// use kernel::register;
> +/// use kernel::num::Bounded;
> +///
> +/// # register!(pub BOOT_0(u32) @ 0x00000100, "Basic revision information about the chip" {
> +/// #     7:4 major_revision, "Major revision of the chip";
> +/// #     3:0 minor_revision, "Minor revision of the chip";
> +/// # });
> +/// # fn test(bar: &kernel::io::Io) {
> +/// // Read from the register's defined offset (0x100).
> +/// let boot0 = BOOT_0::read(&bar);
> +/// pr_info!("chip revision: {}.{}", boot0.major_revision().get(), boot0.minor_revision().get());
> +///
> +/// // Update some fields and write the value back.
> +/// boot0
> +///     .set_major_revision(Bounded::<u32, _>::new::<3>())
> +///     .set_minor_revision(Bounded::<u32, _>::new::<10>())

This looks very verbose...

> +///     .write(&bar);
> +///
> +/// // Or, just read and update the register in a single step:
> +/// BOOT_0::update(&bar, |r| r
> +///     .set_major_revision(Bounded::<u32, _>::new::<3>())
> +///     .set_minor_revision(Bounded::<u32, _>::new::<10>())
> +/// );
> +/// # }
> +/// ```
> +///
> +/// The documentation strings are optional. If present, they will be added to the type's
> +/// definition, or the field getter and setter methods they are attached to.
> +///
> +/// Attributes can be applied to the generated struct. The `#[allow(non_camel_case_types)]`
> +/// attribute is automatically added since register names typically use SCREAMING_CASE:
> +///
> +/// ```
> +/// use kernel::register;
> +///
> +/// register! {
> +///     pub STATUS(u32) @ 0x00000000, "Status register" {
> +///         0:0 ready, "Device ready flag";
> +///     }
> +/// }
> +/// ```
> +///
> +/// It is also possible to create an alias register by using the `=> ALIAS` syntax. This is useful
> +/// for cases where a register's interpretation depends on the context:
> +///
> +/// ```
> +/// use kernel::register;
> +///
> +/// register!(pub SCRATCH(u32) @ 0x00000200, "Scratch register" {
> +///     31:0 value, "Raw value";
> +/// });
> +///
> +/// register!(pub SCRATCH_BOOT_STATUS(u32) => SCRATCH, "Boot status of the firmware" {

How about "as"?

register!(pub SCRATCH_BOOT_STATUS(u32) as SCRATCH);

> +///     0:0 completed, "Whether the firmware has completed booting";
> +/// });
> +/// ```
> +///
> +/// In this example, `SCRATCH_BOOT_STATUS` uses the same I/O address as `SCRATCH`, while also
> +/// providing its own `completed` field.
> +///
> +/// ## Relative registers
> +///
> +/// A register can be defined as being accessible from a fixed offset of a provided base. For
> +/// instance, imagine the following I/O space:
> +///
> +/// ```text
> +///           +-----------------------------+
> +///           |             ...             |
> +///           |                             |
> +///  0x100--->+------------CPU0-------------+
> +///           |                             |
> +///  0x110--->+-----------------------------+
> +///           |           CPU_CTL           |
> +///           +-----------------------------+
> +///           |             ...             |
> +///           |                             |
> +///           |                             |
> +///  0x200--->+------------CPU1-------------+
> +///           |                             |
> +///  0x210--->+-----------------------------+
> +///           |           CPU_CTL           |
> +///           +-----------------------------+
> +///           |             ...             |
> +///           +-----------------------------+
> +/// ```
> +///
> +/// `CPU0` and `CPU1` both have a `CPU_CTL` register that starts at offset `0x10` of their I/O
> +/// space segment. Since both instances of `CPU_CTL` share the same layout, we don't want to define
> +/// them twice and would prefer a way to select which one to use from a single definition
> +///
> +/// This can be done using the `Base[Offset]` syntax when specifying the register's address.
> +///
> +/// `Base` is an arbitrary type (typically a ZST) to be used as a generic parameter of the
> +/// [`RegisterBase`] trait to provide the base as a constant, i.e. each type providing a base for
> +/// this register needs to implement `RegisterBase<Base>`. Here is the above example translated
> +/// into code:
> +///
> +/// ```no_run
> +/// use kernel::register;
> +/// use kernel::io::register::RegisterBase;
> +///
> +/// // Type used to identify the base.
> +/// pub struct CpuCtlBase;
> +///
> +/// // ZST describing `CPU0`.
> +/// struct Cpu0;
> +/// impl RegisterBase<CpuCtlBase> for Cpu0 {
> +///     const BASE: usize = 0x100;
> +/// }
> +/// // Singleton of `CPU0` used to identify it.
> +/// const CPU0: Cpu0 = Cpu0;
> +///
> +/// // ZST describing `CPU1`.
> +/// struct Cpu1;
> +/// impl RegisterBase<CpuCtlBase> for Cpu1 {
> +///     const BASE: usize = 0x200;
> +/// }
> +/// // Singleton of `CPU1` used to identify it.
> +/// const CPU1: Cpu1 = Cpu1;
> +///
> +/// # fn test(bar: &kernel::io::Io) {
> +/// // This makes `CPU_CTL` accessible from all implementors of `RegisterBase<CpuCtlBase>`.
> +/// register!(pub CPU_CTL(u32) @ CpuCtlBase[0x10], "CPU core control" {

Maybe `CpuCtlBase + 0x10`? I think this means the offset is going to be differ
by 0x10, not that it's going to be the 17th register?

> +///     0:0 start, "Start the CPU core";
> +/// });
> +///
> +/// // The `read`, `write` and `update` methods of relative registers take an extra `base` argument
> +/// // that is used to resolve its final address by adding its `BASE` to the offset of the
> +/// // register.
> +///
> +/// // Start `CPU0`.
> +/// CPU_CTL::update(&bar, &CPU0, |r| r.set_start(true));
> +///
> +/// // Start `CPU1`.
> +/// CPU_CTL::update(&bar, &CPU1, |r| r.set_start(true));
> +///
> +/// // Aliases can also be defined for relative register.
> +/// register!(pub CPU_CTL_ALIAS(u32) => CpuCtlBase[CPU_CTL], "Alias to CPU core control" {
> +///     1:1 alias_start, "Start the aliased CPU core";
> +/// });
> +///
> +/// // Start the aliased `CPU0`.
> +/// CPU_CTL_ALIAS::update(&bar, &CPU0, |r| r.set_alias_start(true));
> +/// # }
> +/// ```
> +///
> +/// ## Arrays of registers
> +///
> +/// Some I/O areas contain consecutive values that can be interpreted in the same way. These areas
> +/// can be defined as an array of identical registers, allowing them to be accessed by index with
> +/// compile-time or runtime bound checking. Simply define their address as `Address[Size]`, and add
> +/// an `idx` parameter to their `read`, `write` and `update` methods:
> +///
> +/// ```no_run
> +/// use kernel::register;
> +///
> +/// # fn no_run(bar: &kernel::io::Io) -> Result<(), Error> {
> +/// # fn get_scratch_idx() -> usize {
> +/// #   0x15
> +/// # }
> +/// // Array of 64 consecutive registers with the same layout starting at offset `0x80`.
> +/// register!(pub SCRATCH(u32) @ 0x00000080[64], "Scratch registers" {

This syntax is way to close to the base syntax above.

Perhaps `pub SCRATCH([u32; 64]) @ 0x00000080` would make more sense?

> +///     31:0 value;
> +/// });
> +///
> +/// // Read scratch register 0, i.e. I/O address `0x80`.
> +/// let scratch_0 = SCRATCH::read(&bar, 0).value();
> +/// // Read scratch register 15, i.e. I/O address `0x80 + (15 * 4)`.
> +/// let scratch_15 = SCRATCH::read(&bar, 15).value();
> +///
> +/// // This is out of bounds and won't build.
> +/// // let scratch_128 = SCRATCH::read(&bar, 128).value();
> +///
> +/// // Runtime-obtained array index.
> +/// let scratch_idx = get_scratch_idx();
> +/// // Access on a runtime index returns an error if it is out-of-bounds.
> +/// let some_scratch = SCRATCH::try_read(&bar, scratch_idx)?.value();
> +///
> +/// // Alias to a particular register in an array.
> +/// // Here `SCRATCH[8]` is used to convey the firmware exit code.
> +/// register!(pub FIRMWARE_STATUS(u32) => SCRATCH[8], "Firmware exit status code" {
> +///     7:0 status;
> +/// });
> +///
> +/// let status = FIRMWARE_STATUS::read(&bar).status();
> +///
> +/// // Non-contiguous register arrays can be defined by adding a stride parameter.
> +/// // Here, each of the 16 registers of the array are separated by 8 bytes, meaning that the
> +/// // registers of the two declarations below are interleaved.
> +/// register!(pub SCRATCH_INTERLEAVED_0(u32) @ 0x000000c0[16 ; 8], "Scratch registers bank 0" {
> +///     31:0 value;
> +/// });
> +/// register!(pub SCRATCH_INTERLEAVED_1(u32) @ 0x000000c4[16 ; 8], "Scratch registers bank 1" {
> +///     31:0 value;
> +/// });
> +/// # Ok(())
> +/// # }
> +/// ```
> +///
> +/// ## Relative arrays of registers
> +///
> +/// Combining the two features described in the sections above, arrays of registers accessible from
> +/// a base can also be defined:
> +///
> +/// ```no_run
> +/// use kernel::register;
> +/// use kernel::io::register::RegisterBase;
> +///
> +/// # fn no_run(bar: &kernel::io::Io) -> Result<(), Error> {
> +/// # fn get_scratch_idx() -> usize {
> +/// #   0x15
> +/// # }
> +/// // Type used as parameter of `RegisterBase` to specify the base.
> +/// pub struct CpuCtlBase;
> +///
> +/// // ZST describing `CPU0`.
> +/// struct Cpu0;
> +/// impl RegisterBase<CpuCtlBase> for Cpu0 {
> +///     const BASE: usize = 0x100;
> +/// }
> +/// // Singleton of `CPU0` used to identify it.
> +/// const CPU0: Cpu0 = Cpu0;
> +///
> +/// // ZST describing `CPU1`.
> +/// struct Cpu1;
> +/// impl RegisterBase<CpuCtlBase> for Cpu1 {
> +///     const BASE: usize = 0x200;
> +/// }
> +/// // Singleton of `CPU1` used to identify it.
> +/// const CPU1: Cpu1 = Cpu1;
> +///
> +/// // 64 per-cpu scratch registers, arranged as a contiguous array.
> +/// register!(pub CPU_SCRATCH(u32) @ CpuCtlBase[0x00000080[64]], "Per-CPU scratch registers" {

Ah... I don't like this.

> +///     31:0 value;
> +/// });
> +///
> +/// let cpu0_scratch_0 = CPU_SCRATCH::read(&bar, &Cpu0, 0).value();
> +/// let cpu1_scratch_15 = CPU_SCRATCH::read(&bar, &Cpu1, 15).value();
> +///
> +/// // This won't build.
> +/// // let cpu0_scratch_128 = CPU_SCRATCH::read(&bar, &Cpu0, 128).value();
> +///
> +/// // Runtime-obtained array index.
> +/// let scratch_idx = get_scratch_idx();
> +/// // Access on a runtime value returns an error if it is out-of-bounds.
> +/// let cpu0_some_scratch = CPU_SCRATCH::try_read(&bar, &Cpu0, scratch_idx)?.value();
> +///
> +/// // `SCRATCH[8]` is used to convey the firmware exit code.
> +/// register!(pub CPU_FIRMWARE_STATUS(u32) => CpuCtlBase[CPU_SCRATCH[8]],
> +///     "Per-CPU firmware exit status code" {
> +///     7:0 status;
> +/// });
> +///
> +/// let cpu0_status = CPU_FIRMWARE_STATUS::read(&bar, &Cpu0).status();
> +///
> +/// // Non-contiguous register arrays can be defined by adding a stride parameter.
> +/// // Here, each of the 16 registers of the array are separated by 8 bytes, meaning that the
> +/// // registers of the two declarations below are interleaved.
> +/// register!(pub CPU_SCRATCH_INTERLEAVED_0(u32) @ CpuCtlBase[0x00000d00[16 ; 8]],

For striding, SystemRDL uses syntax like this:

    type name[len] @ addr += stride;

which I think is better as this won't be confused with array length syntax in
Rust.

Crazy idea: is it possible to just use SystemRDL (maybe via a proc macro).

Best,
Gary

> +///     "Scratch registers bank 0" {
> +///     31:0 value;
> +/// });
> +/// register!(pub CPU_SCRATCH_INTERLEAVED_1(u32) @ CpuCtlBase[0x00000d04[16 ; 8]],
> +///     "Scratch registers bank 1" {
> +///     31:0 value;
> +/// });
> +/// # Ok(())
> +/// # }
> +/// ```
> +/// [`bitfield!`]: crate::bitfield!

Powered by blists - more mailing lists