From: "Danilo Krummrich" <dakr@kernel.org>
To: "Gary Guo" <gary@garyguo.net>
Cc: "Alexandre Courbot" <acourbot@nvidia.com>,
"Alice Ryhl" <aliceryhl@google.com>,
"Daniel Almeida" <daniel.almeida@collabora.com>,
"Miguel Ojeda" <ojeda@kernel.org>,
"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>,
"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 v7 05/10] rust: io: add IoLoc and IoWrite types
Date: Wed, 04 Mar 2026 22:38:41 +0100 [thread overview]
Message-ID: <DGUBN1HP1F8I.2RZGSFCBO9FQS@kernel.org> (raw)
In-Reply-To: <DGUB44J4FNCA.32QOV1DYI7VBK@garyguo.net>
On Wed Mar 4, 2026 at 10:13 PM CET, Gary Guo wrote:
> On Wed Mar 4, 2026 at 8:37 PM GMT, Danilo Krummrich wrote:
>> On Wed Mar 4, 2026 at 8:48 PM CET, Gary Guo wrote:
>>> On Wed Mar 4, 2026 at 7:38 PM GMT, Danilo Krummrich wrote:
>>>> On Wed Mar 4, 2026 at 7:58 PM CET, Gary Guo wrote:
>>>>> On Wed Mar 4, 2026 at 6:39 PM GMT, Gary Guo wrote:
>>>>>> On Wed Mar 4, 2026 at 4:18 PM GMT, Danilo Krummrich wrote:
>>>>>>> On Tue Mar 3, 2026 at 3:55 PM CET, Alexandre Courbot wrote:
>>>>>>>> So, to get a better idea of these two options I have converted this
>>>>>>>> patchset to use the 2-arguments `write_with` method. Here is the
>>>>>>>> difference between the two - it is particularly interesting to see how
>>>>>>>> nova-core changes:
>>>>>>>>
>>>>>>>> https://github.com/Gnurou/linux/compare/register_1arg..Gnurou:linux:register_2args
>>>>>>>
>>>>>>> This looks good to me, but the fact that this turns out nicely has nothing to do
>>>>>>> with write() now taking two arguments. I.e. there is no reason why we couldn't
>>>>>>> have the exact same write_with() method together with the single argument
>>>>>>> write() method.
>>>>>>>
>>>>>>> The contention point for me with a two arguments write() method still remains
>>>>>>> that the arguments are redundant.
>>>>>>>
>>>>>>> I.e. you first have the location in form of an object instance of a ZST (which
>>>>>>> in the end is just a "trick" to pass in the type itself) and then we have the
>>>>>>> object that actually represents the entire register, describing both the
>>>>>>> location *and* the value.
>>>>>>>
>>>>>>> So, let's say a driver creates a register object with a custom constructor
>>>>>>>
>>>>>>> let reset = regs::MyReg::reset();
>>>>>>>
>>>>>>> then the two argument approach would be
>>>>>>>
>>>>>>> (1) bar.write(regs::MyReg, regs::MyReg::reset());
>>>>>>>
>>>>>>> whereas the single argument approach would just be
>>>>>>>
>>>>>>> (2) bar.write(regs::MyReg::reset());
>>>>>>
>>>>>> That's only for bit field registers that has unique types. I still believe types
>>>>>> of registers should not be tightly coupled with name of registeres.
>>>>>>
>>>>>> Allowing a value of register to be directly used for `write` is also confusing
>>>>>> if a value is not created immediately before written to.
>>>>>>
>>>>>>>
>>>>>>> So, if I would have to write (1), I'd probably be tempted to implement a reset()
>>>>>>> function that takes the bar as argument to hide this, i.e.
>>>>>>>
>>>>>>> regs::MyReg::reset(bar);
>>>>>>>
>>>>>>> I also can't agree with the argument that the notation of write(loc, val) - or
>>>>>>> write(val, loc) as the C side does it - is common and we should stick to it.
>>>>>>>
>>>>>>> This notation is only common because it is necessary when operating on
>>>>>>> primitives or when the two representing types are discrete.
>>>>>>>
>>>>>>> But this isn't the case here, a register object is already distinct in terms of
>>>>>>> its location and value.
>>>>>>
>>>>>> I see no reason why register values for different locations have to be distinct
>>>>>> in terms of value types.
>>>>
>>>> That's not what the register!() macro currently does, a register type always has
>>>> a unique location, or is an array register, etc. In any case a register type is
>>>> assoiciated with a location.
>>>>
>>>> If the proposal is to disconnect location and register type entirely, that would
>>>> be a change to the current design.
>>>
>>> It's not what the macro do today, but I don't want to ask Alex to change it
>>> further before landing the series. I do think it's a worthy follow-up to add the
>>> ability to decouple the location and type. It's not incompatible with current
>>> design anyway.
>>
>> I'm not sure there are any relevant use-cases for this. Do you have real
>> examples that would not be represented with array registers?
>
> Even for the cases where there's a PIO register, I think it's beneficial to just
> get a value without a type.
>
> I don't see why we want people to write
>
> self.io.read(UART_RX).value()
>
> vs
>
> self.io.read(UART_RX)
>
> or
>
> self.io.write(UART_TX::from(byte))
>
> vs
>
> self.io.write(UART_TX, byte)
>
> what benefit does additional type provide?
Well, for FIFO registers this is indeed better. However, my main concern was
this
bar.write(regs::MyReg, regs::MyReg::foo())
for the reasons explained above.
As for FIFO registers, another option could be to leverage the raw accessors and
make a location type dereferece to the offset, e.g.
bar.write8(regs::TX_FIFO, byte)
The same goes for the array case of course.
next prev parent reply other threads:[~2026-03-04 21:38 UTC|newest]
Thread overview: 64+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-02-24 14:21 [PATCH v7 00/10] rust: add `register!` macro Alexandre Courbot
2026-02-24 14:21 ` [PATCH v7 01/10] rust: enable the `generic_arg_infer` feature Alexandre Courbot
2026-02-24 14:21 ` [PATCH v7 02/10] rust: num: add `shr` and `shl` methods to `Bounded` Alexandre Courbot
2026-02-24 14:21 ` [PATCH v7 03/10] rust: num: add `into_bool` method " Alexandre Courbot
2026-02-24 14:21 ` [PATCH v7 04/10] rust: num: make Bounded::get const Alexandre Courbot
2026-02-27 12:33 ` Gary Guo
2026-02-24 14:21 ` [PATCH v7 05/10] rust: io: add IoLoc and IoWrite types Alexandre Courbot
2026-02-27 18:02 ` Gary Guo
2026-02-27 18:16 ` Danilo Krummrich
2026-02-28 0:33 ` Alexandre Courbot
2026-03-01 15:11 ` Gary Guo
2026-03-02 1:44 ` Alexandre Courbot
2026-03-02 12:53 ` Gary Guo
2026-03-02 13:12 ` Danilo Krummrich
2026-03-02 13:39 ` Gary Guo
2026-03-03 8:14 ` Alexandre Courbot
2026-03-03 8:31 ` Alexandre Courbot
2026-03-03 14:55 ` Alexandre Courbot
2026-03-03 15:05 ` Gary Guo
2026-03-04 16:18 ` Danilo Krummrich
2026-03-04 18:39 ` Gary Guo
2026-03-04 18:58 ` Gary Guo
2026-03-04 19:19 ` John Hubbard
2026-03-04 19:53 ` Danilo Krummrich
2026-03-04 19:57 ` John Hubbard
2026-03-04 20:05 ` Gary Guo
2026-03-04 19:38 ` Danilo Krummrich
2026-03-04 19:48 ` Gary Guo
2026-03-04 20:37 ` Danilo Krummrich
2026-03-04 21:13 ` Gary Guo
2026-03-04 21:38 ` Danilo Krummrich [this message]
2026-03-04 21:42 ` Danilo Krummrich
2026-03-04 22:15 ` Gary Guo
2026-03-04 22:22 ` Danilo Krummrich
2026-03-06 5:37 ` Alexandre Courbot
2026-03-06 7:47 ` Alexandre Courbot
2026-03-06 10:42 ` Gary Guo
2026-03-06 11:10 ` Alexandre Courbot
2026-03-06 11:35 ` Gary Guo
2026-03-06 12:50 ` Alexandre Courbot
2026-03-06 13:20 ` Gary Guo
2026-03-06 14:32 ` Alexandre Courbot
2026-03-06 14:52 ` Alexandre Courbot
2026-03-06 15:10 ` Alexandre Courbot
2026-03-06 15:35 ` Alexandre Courbot
2026-03-06 15:35 ` Gary Guo
2026-03-07 0:05 ` Alexandre Courbot
2026-03-07 21:10 ` Gary Guo
2026-03-07 21:40 ` Danilo Krummrich
2026-03-08 11:43 ` Alexandre Courbot
2026-03-08 11:35 ` Alexandre Courbot
2026-03-04 18:53 ` Gary Guo
2026-03-04 22:19 ` Gary Guo
2026-03-05 11:02 ` Alexandre Courbot
2026-02-24 14:21 ` [PATCH v7 06/10] rust: io: use generic read/write accessors for primitive accesses Alexandre Courbot
2026-02-27 18:04 ` Gary Guo
2026-02-24 14:21 ` [PATCH v7 07/10] rust: io: add `register!` macro Alexandre Courbot
2026-02-24 14:21 ` [PATCH v7 08/10] sample: rust: pci: use " Alexandre Courbot
2026-02-24 14:21 ` [PATCH FOR REFERENCE v7 09/10] gpu: nova-core: use the kernel " Alexandre Courbot
2026-02-24 14:21 ` [PATCH v7 10/10] RFC: rust: io: allow fixed register values directly in `write` Alexandre Courbot
2026-02-25 11:58 ` [PATCH v7 00/10] rust: add `register!` macro Dirk Behme
2026-02-25 13:50 ` Alexandre Courbot
2026-02-26 12:01 ` Dirk Behme
2026-02-27 23:30 ` 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=DGUBN1HP1F8I.2RZGSFCBO9FQS@kernel.org \
--to=dakr@kernel.org \
--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=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=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