mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: "Gary Guo" <gary@garyguo.net>
To: "Alexandre Courbot" <acourbot@nvidia.com>,
	"Danilo Krummrich" <dakr@kernel.org>,
	"Alice Ryhl" <aliceryhl@google.com>,
	"Daniel Almeida" <daniel.almeida@collabora.com>,
	"Miguel Ojeda" <ojeda@kernel.org>, "Gary Guo" <gary@garyguo.net>,
	"Björn Roy Baron" <bjorn3_gh@protonmail.com>,
	"Benno Lossin" <lossin@kernel.org>,
	"Andreas Hindborg" <a.hindborg@kernel.org>,
	"Trevor Gross" <tmgross@umich.edu>,
	"Boqun Feng" <boqun@kernel.org>
Cc: "Yury Norov" <yury.norov@gmail.com>,
	"John Hubbard" <jhubbard@nvidia.com>,
	"Alistair Popple" <apopple@nvidia.com>,
	"Joel Fernandes" <joelagnelf@nvidia.com>,
	"Timur Tabi" <ttabi@nvidia.com>, "Edwin Peer" <epeer@nvidia.com>,
	"Eliot Courtney" <ecourtney@nvidia.com>,
	"Dirk Behme" <dirk.behme@de.bosch.com>,
	"Steven Price" <steven.price@arm.com>,
	<rust-for-linux@vger.kernel.org>, <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v9 08/10] rust: io: introduce `write_reg` and `LocatedRegister`
Date: Sat, 14 Mar 2026 13:56:50 +0000	[thread overview]
Message-ID: <DH2K2VEN94JH.3K1WTNSTYEUSU@garyguo.net> (raw)
In-Reply-To: <20260314-register-v9-8-86805b2f7e9d@nvidia.com>

On Sat Mar 14, 2026 at 1:06 AM GMT, Alexandre Courbot wrote:
> Some I/O types, like fixed address registers, carry their location
> alongside their values. For these types, the regular `Io::write` method
> can lead into repeating the location information twice: once to provide
> the location itself, another time to build the value.
>
> We are also considering supporting making all register values carry
> their full location information for convenience and safety.
>
> Add a new `Io::write_reg` method that takes a single argument
> implementing `LocatedRegister`, a trait that decomposes implementors
> into a `(location, value)` tuple. This allows write operations on fixed
> offset registers to be done while specifying their name only once.
>
> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
> ---
>  rust/kernel/io.rs          | 70 ++++++++++++++++++++++++++++++++++++++++++++++
>  rust/kernel/io/register.rs | 35 +++++++++++++++++++++--
>  2 files changed, 103 insertions(+), 2 deletions(-)
>
> diff --git a/rust/kernel/io.rs b/rust/kernel/io.rs
> index bfea30a9acdf..24e6b48b6582 100644
> --- a/rust/kernel/io.rs
> +++ b/rust/kernel/io.rs
> @@ -17,6 +17,8 @@
>  pub use crate::register;
>  pub use resource::Resource;
>  
> +use register::LocatedRegister;
> +
>  /// Physical address type.
>  ///
>  /// This is a type alias to either `u32` or `u64` depending on the config option
> @@ -473,6 +475,40 @@ fn try_write<T, L>(&self, location: L, value: T) -> Result
>          Ok(())
>      }
>  
> +    /// Generic fallible write of a fully-located register value.
> +    ///
> +    /// # Examples
> +    ///
> +    /// Tuples carrying a location and a value can be used with this method:
> +    ///
> +    /// ```no_run
> +    /// use kernel::io::{
> +    ///     register,
> +    ///     Io,
> +    ///     Mmio,
> +    /// };
> +    ///
> +    /// register! {
> +    ///     FIFO_OUT(u32) @ 0x100 {}
> +    /// }
> +    ///
> +    /// fn do_write_reg(io: &Mmio) -> Result {
> +    ///     // `FIFO_OUT` provides us the location of the write operation.
> +    ///     io.try_write_reg(FIFO_OUT::from(10))

This is not a good example, as I want to make FIFO not generate bitfields in the
future and so people write

    io.write(FIFO_OUT, 10)

Perhaps replace with any other register example that has bitfields..

The version register in the updated doc comment below is a very good example...

Best,
Gary

> +    /// }
> +    /// ```
> +    #[inline(always)]
> +    fn try_write_reg<T, L, V>(&self, value: V) -> Result
> +    where
> +        L: IoLoc<T>,
> +        V: LocatedRegister<Location = L, Value = T>,
> +        Self: IoCapable<L::IoType>,
> +    {
> +        let (location, value) = value.into_io_op();
> +
> +        self.try_write(location, value)
> +    }
> +
>      /// Generic fallible update with runtime bounds check.
>      ///
>      /// Note: this does not perform any synchronization. The caller is responsible for ensuring
> @@ -578,6 +614,40 @@ fn write<T, L>(&self, location: L, value: T)
>          unsafe { self.io_write(io_value, address) }
>      }
>  
> +    /// Generic infallible write of a fully-located register value.
> +    ///
> +    /// # Examples
> +    ///
> +    /// Tuples carrying a location and a value can be used with this method:
> +    ///
> +    /// ```no_run
> +    /// use kernel::io::{
> +    ///     register,
> +    ///     Io,
> +    ///     Mmio,
> +    /// };
> +    ///
> +    /// register! {
> +    ///     FIFO_OUT(u32) @ 0x100 {}
> +    /// }
> +    ///
> +    /// fn do_write_reg(io: &Mmio<0x1000>) {
> +    ///     // `FIFO_OUT` provides us the location of the write operation.
> +    ///     io.write_reg(FIFO_OUT::from(10));
> +    /// }
> +    /// ```
> +    #[inline(always)]
> +    fn write_reg<T, L, V>(&self, value: V)
> +    where
> +        L: IoLoc<T>,
> +        V: LocatedRegister<Location = L, Value = T>,
> +        Self: IoKnownSize + IoCapable<L::IoType>,
> +    {
> +        let (location, value) = value.into_io_op();
> +
> +        self.write(location, value)
> +    }
> +
>      /// Generic infallible update with compile-time bounds check.
>      ///
>      /// Note: this does not perform any synchronization. The caller is responsible for ensuring
> diff --git a/rust/kernel/io/register.rs b/rust/kernel/io/register.rs
> index 40085953c831..b26dc2400009 100644
> --- a/rust/kernel/io/register.rs
> +++ b/rust/kernel/io/register.rs
> @@ -80,10 +80,10 @@
>  //!     .with_const_minor_revision::<10>()
>  //!     // Run-time value.
>  //!     .with_vendor_id(obtain_vendor_id());
> -//! io.write((), new_boot0);
> +//! io.write_reg(new_boot0);
>  //!
>  //! // Or, build a new value from zero and write it:
> -//! io.write((), BOOT_0::zeroed()
> +//! io.write_reg(BOOT_0::zeroed()
>  //!     .with_const_major_revision::<3>()
>  //!     .with_const_minor_revision::<10>()
>  //!     .with_vendor_id(obtain_vendor_id())
> @@ -379,6 +379,34 @@ fn offset(self) -> usize {
>      }
>  }
>  
> +/// Trait implemented by items that contain both a register value and the absolute I/O location at
> +/// which to write it.
> +///
> +/// Implementors can be used with [`Io::write_reg`](super::Io::write_reg).
> +pub trait LocatedRegister {
> +    /// Register value to write.
> +    type Value: Register;
> +    /// Full location information at which to write the value.
> +    type Location: IoLoc<Self::Value>;
> +
> +    /// Consumes `self` and returns a `(location, value)` tuple describing a valid I/O write
> +    /// operation.
> +    fn into_io_op(self) -> (Self::Location, Self::Value);
> +}
> +
> +impl<T> LocatedRegister for T
> +where
> +    T: FixedRegister,
> +{
> +    type Location = FixedRegisterLoc<Self::Value>;
> +    type Value = T;
> +
> +    #[inline(always)]
> +    fn into_io_op(self) -> (FixedRegisterLoc<T>, T) {
> +        (FixedRegisterLoc::new(), self)
> +    }
> +}
> +
>  /// Defines a dedicated type for a register, including getter and setter methods for its fields and
>  /// methods to read and write it from an [`Io`](kernel::io::Io) region.
>  ///
> @@ -433,6 +461,9 @@ fn offset(self) -> usize {
>  /// // The location of fixed offset registers is already contained in their type. Thus, the
>  /// // `location` argument of `Io::write` is technically redundant and can be replaced by `()`.
>  /// io.write((), val2);
> +///
> +/// // Or, the single-argument `Io::write_reg` can be used.
> +/// io.write_reg(val2);
>  /// # }
>  ///
>  /// ```


  reply	other threads:[~2026-03-14 13:56 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-14  1:06 [PATCH v9 00/10] rust: add `register!` macro Alexandre Courbot
2026-03-14  1:06 ` [PATCH v9 01/10] rust: enable the `generic_arg_infer` feature Alexandre Courbot
2026-03-14  1:06 ` [PATCH v9 02/10] rust: num: add `shr` and `shl` methods to `Bounded` Alexandre Courbot
2026-03-14  1:06 ` [PATCH v9 03/10] rust: num: add `into_bool` method " Alexandre Courbot
2026-03-14  1:06 ` [PATCH v9 04/10] rust: num: make Bounded::get const Alexandre Courbot
2026-03-14  1:06 ` [PATCH v9 05/10] rust: io: add IoLoc type and generic I/O accessors Alexandre Courbot
2026-03-14  1:06 ` [PATCH v9 06/10] rust: io: use generic read/write accessors for primitive accesses Alexandre Courbot
2026-03-14 13:42   ` Gary Guo
2026-03-14  1:06 ` [PATCH v9 07/10] rust: io: add `register!` macro Alexandre Courbot
2026-03-14 13:53   ` Gary Guo
2026-03-14  1:06 ` [PATCH v9 08/10] rust: io: introduce `write_reg` and `LocatedRegister` Alexandre Courbot
2026-03-14 13:56   ` Gary Guo [this message]
2026-03-15  5:10     ` Alexandre Courbot
2026-03-15 10:57       ` Danilo Krummrich
2026-03-14  1:06 ` [PATCH v9 09/10] sample: rust: pci: use `register!` macro Alexandre Courbot
2026-03-14  1:06 ` [PATCH FOR REFERENCE v9 10/10] gpu: nova-core: use the kernel " Alexandre Courbot
2026-03-15  0:57 ` [PATCH v9 00/10] rust: add " Danilo Krummrich
2026-03-17 19:33   ` Danilo Krummrich
2026-03-18  1:32     ` Alexandre Courbot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DH2K2VEN94JH.3K1WTNSTYEUSU@garyguo.net \
    --to=gary@garyguo.net \
    --cc=a.hindborg@kernel.org \
    --cc=acourbot@nvidia.com \
    --cc=aliceryhl@google.com \
    --cc=apopple@nvidia.com \
    --cc=bjorn3_gh@protonmail.com \
    --cc=boqun@kernel.org \
    --cc=dakr@kernel.org \
    --cc=daniel.almeida@collabora.com \
    --cc=dirk.behme@de.bosch.com \
    --cc=ecourtney@nvidia.com \
    --cc=epeer@nvidia.com \
    --cc=jhubbard@nvidia.com \
    --cc=joelagnelf@nvidia.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lossin@kernel.org \
    --cc=ojeda@kernel.org \
    --cc=rust-for-linux@vger.kernel.org \
    --cc=steven.price@arm.com \
    --cc=tmgross@umich.edu \
    --cc=ttabi@nvidia.com \
    --cc=yury.norov@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®