Message-Id: <DFYCR05BIXCK.24C5YRIM7KK7Y@nvidia.com>
Date: Mon, 26 Jan 2026 16:45:34 +0900
From: "Alexandre Courbot" <acourbot@...dia.com>
To: "Gary Guo" <gary@...yguo.net>
Cc: "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>,
 Björn Roy Baron <bjorn3_gh@...tonmail.com>, "Benno Lossin"
 <lossin@...nel.org>, "Andreas Hindborg" <a.hindborg@...nel.org>, "Trevor
 Gross" <tmgross@...ch.edu>, "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 Mon Jan 26, 2026 at 12:24 PM JST, Alexandre Courbot wrote:
> On Wed Jan 21, 2026 at 11:50 PM JST, Gary Guo wrote:
>> 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.
>>     }
>> )
>
> That would definitely be better, unfortunately since this is a
> declarative macro it cannot match against comments, hence the current
> syntax.

Looks like I might be wrong here as well. Doccomments can apparently be
matched as meta items, which would allow them to be re-emitted inside
the rule. If that works (LSP unfortunately doesn't let me check that at
the moment), then we should be able to do as you suggested, which would
be great.

I also only noticed now the proposed reordering:

    minor_version: [3:0],

I don't have a strong opinion on that, but fwiw SystemRDL seems to do
something like

    field minor_version[3:0];

If we agree to align on it the latter might be preferable?

Powered by blists - more mailing lists