From: Joel Fernandes <joelagnelf@nvidia.com>
To: "John Hubbard" <jhubbard@nvidia.com>,
"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>,
Alistair Popple <apopple@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 FOR REFERENCE v8 10/10] gpu: nova-core: use the kernel `register!` macro
Date: Mon, 9 Mar 2026 13:51:36 -0400 [thread overview]
Message-ID: <4f7baebe-e993-40b5-9cb2-3d4699da1299@nvidia.com> (raw)
In-Reply-To: <d85a3776-4a73-457d-8722-82d8dfc58591@nvidia.com>
On 3/9/2026 1:34 PM, John Hubbard wrote:
> On 3/9/26 8:43 AM, Joel Fernandes wrote:
>>
>> On 3/9/2026 11:14 AM, Alexandre Courbot wrote:
>>>
>>> @@ -797,26 +792,30 @@ pub(crate) fn start(&self, bar: &Bar0) -> Result<()> {
>>> /// Writes values to the mailbox registers if provided.
>>> pub(crate) fn write_mailboxes(&self, bar: &Bar0, mbox0: Option<u32>, mbox1: Option<u32>) {
>>> if let Some(mbox0) = mbox0 {
>>> - regs::NV_PFALCON_FALCON_MAILBOX0::default()
>>> - .set_value(mbox0)
>>> - .write(bar, &E::ID);
>>> + bar.write(
>>> + WithBase::of::<E>(),
>>> + regs::NV_PFALCON_FALCON_MAILBOX0::zeroed().with_value(mbox0),
>>> + );
>>> }
>>>
>>> if let Some(mbox1) = mbox1 {
>>> - regs::NV_PFALCON_FALCON_MAILBOX1::default()
>>> - .set_value(mbox1)
>>> - .write(bar, &E::ID);
>>> + bar.write(
>>> + WithBase::of::<E>(),
>>> + regs::NV_PFALCON_FALCON_MAILBOX1::zeroed().with_value(mbox1),
>>> + );
>>> }
>>> }
>>
>> TBH, this is quite a readability hit, the previous was much more readable IMHO.
>
> Yes, I think so too, except that "WithBase::of::<E>" makes a lot more
> sense on-screen, than "&E::ID". The latter just looks quite random,
> so this part is an improvement.
>
> Let's break down the remaining troublesome part a bit:
>
> regs::NV_PFALCON_FALCON_MAILBOX1::zeroed().with_value(mbox1)
>
> * "regs::" can be omitted with a "use" statement, right?
>
> * ::zeroed() maybe should be the default behavior here, and then
> it could also be omitted?
The issue with omitting it is it still needs a constructor? so if not zeroed()
it would still need ::default() I think.
>
> * .with_value(mbox1) I'm sure this is necessary, but the construction
> is unfortunately much less clear than .write(value)! Thoughts?
yeah, also perhaps the whole thing should have a macro! to de-sugar. The
patterns seem somewhat repetitive.
thanks,
--
Joel Fernandes
next prev parent reply other threads:[~2026-03-09 17:51 UTC|newest]
Thread overview: 57+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-03-09 15:13 [PATCH v8 00/10] rust: add `register!` macro Alexandre Courbot
2026-03-09 15:13 ` [PATCH v8 01/10] rust: enable the `generic_arg_infer` feature Alexandre Courbot
2026-03-11 0:13 ` Yury Norov
2026-03-11 6:16 ` Miguel Ojeda
2026-03-09 15:13 ` [PATCH v8 02/10] rust: num: add `shr` and `shl` methods to `Bounded` Alexandre Courbot
2026-03-11 0:26 ` Yury Norov
2026-03-09 15:14 ` [PATCH v8 03/10] rust: num: add `into_bool` method " Alexandre Courbot
2026-03-11 0:29 ` Yury Norov
2026-03-09 15:14 ` [PATCH v8 04/10] rust: num: make Bounded::get const Alexandre Courbot
2026-03-09 15:14 ` [PATCH v8 05/10] rust: io: add IoLoc type and generic I/O accessors Alexandre Courbot
2026-03-10 15:15 ` Danilo Krummrich
2026-03-09 15:14 ` [PATCH v8 06/10] rust: io: use generic read/write accessors for primitive accesses Alexandre Courbot
2026-03-09 15:29 ` Gary Guo
2026-03-10 1:57 ` Alexandre Courbot
2026-03-10 1:59 ` Alexandre Courbot
2026-03-10 2:04 ` Gary Guo
2026-03-10 15:17 ` Danilo Krummrich
2026-03-09 15:14 ` [PATCH v8 07/10] rust: io: introduce `IntoIoVal` trait and single-argument `write_val` Alexandre Courbot
2026-03-09 15:30 ` Gary Guo
2026-03-10 2:05 ` Alexandre Courbot
2026-03-10 16:34 ` Danilo Krummrich
2026-03-10 22:33 ` Gary Guo
2026-03-11 13:25 ` Danilo Krummrich
2026-03-11 13:42 ` Alexandre Courbot
2026-03-11 13:28 ` Alexandre Courbot
2026-03-11 14:56 ` Danilo Krummrich
2026-03-11 15:42 ` Gary Guo
2026-03-11 16:22 ` Danilo Krummrich
2026-03-11 17:16 ` Gary Guo
2026-03-12 13:53 ` Alexandre Courbot
2026-03-12 15:54 ` Danilo Krummrich
2026-03-14 1:19 ` Alexandre Courbot
2026-03-09 15:14 ` [PATCH v8 08/10] rust: io: add `register!` macro Alexandre Courbot
2026-03-10 16:39 ` Danilo Krummrich
2026-03-09 15:14 ` [PATCH v8 09/10] sample: rust: pci: use " Alexandre Courbot
2026-03-09 15:14 ` [PATCH FOR REFERENCE v8 10/10] gpu: nova-core: use the kernel " Alexandre Courbot
2026-03-09 15:43 ` Joel Fernandes
2026-03-09 17:34 ` John Hubbard
2026-03-09 17:51 ` Joel Fernandes [this message]
2026-03-09 18:01 ` John Hubbard
2026-03-09 18:28 ` Gary Guo
2026-03-09 18:34 ` John Hubbard
2026-03-09 18:42 ` Danilo Krummrich
2026-03-09 18:47 ` John Hubbard
2026-03-09 18:42 ` Gary Guo
2026-03-09 18:50 ` John Hubbard
2026-03-09 18:04 ` Danilo Krummrich
2026-03-09 18:06 ` John Hubbard
2026-03-10 2:18 ` Alexandre Courbot
2026-03-09 18:05 ` [PATCH v8 00/10] rust: add " John Hubbard
2026-03-09 21:08 ` Daniel Almeida
2026-03-10 17:20 ` Danilo Krummrich
2026-03-11 13:01 ` Alexandre Courbot
2026-03-11 13:07 ` Danilo Krummrich
2026-03-11 13:35 ` Alexandre Courbot
2026-03-11 13:38 ` Danilo Krummrich
2026-03-12 2:23 ` 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=4f7baebe-e993-40b5-9cb2-3d4699da1299@nvidia.com \
--to=joelagnelf@nvidia.com \
--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=gary@garyguo.net \
--cc=jhubbard@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.