All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Benno Lossin" <lossin@kernel.org>
To: "Daniel Almeida" <daniel.almeida@collabora.com>
Cc: "Mark Brown" <broonie@kernel.org>,
	"Miguel Ojeda" <ojeda@kernel.org>,
	"Alex Gaynor" <alex.gaynor@gmail.com>,
	"Boqun Feng" <boqun.feng@gmail.com>,
	"Gary Guo" <gary@garyguo.net>,
	"Björn Roy Baron" <bjorn3_gh@protonmail.com>,
	"Benno Lossin" <benno.lossin@proton.me>,
	"Andreas Hindborg" <a.hindborg@kernel.org>,
	"Alice Ryhl" <aliceryhl@google.com>,
	"Trevor Gross" <tmgross@umich.edu>,
	"Danilo Krummrich" <dakr@kernel.org>,
	"Boris Brezillon" <boris.brezillon@collabora.com>,
	"Sebastian Reichel" <sebastian.reichel@collabora.com>,
	"Liam Girdwood" <lgirdwood@gmail.com>,
	linux-kernel@vger.kernel.org, rust-for-linux@vger.kernel.org
Subject: Re: [PATCH v3] rust: regulator: add a bare minimum regulator abstraction
Date: Wed, 14 May 2025 19:41:05 +0200	[thread overview]
Message-ID: <D9W2GYMMVKLV.WXCSH0FYL4QC@kernel.org> (raw)
In-Reply-To: <B58EF4F3-6ADD-4311-BE9D-CCE82CBAEBF2@collabora.com>

On Wed May 14, 2025 at 6:19 PM CEST, Daniel Almeida wrote:
>> On 14 May 2025, at 13:05, Benno Lossin <lossin@kernel.org> wrote:
>> On Wed May 14, 2025 at 5:50 PM CEST, Mark Brown wrote:
>>> On Wed, May 14, 2025 at 05:38:40PM +0200, Benno Lossin wrote:
>>>> On Wed May 14, 2025 at 4:40 PM CEST, Daniel Almeida wrote:
>>>>> By the way, IIUC, regulator_disable() does not disable a regulator necessarily.
>>>>> It just tells the system that you don't care about it being enabled anymore. It can
>>>>> still remain on if there are other users.
>>> 
>>>> Hmm, so a `struct regulator` might already be enabled and calling
>>>> `regulator_enable` doesn't do anything?
>>> 
>>> It takes a reference to the regulator.  This may or may not result in a
>>> change in an underlying physical regulator.
>> 
>> Gotcha. So calling `regulator_enable` twice on the same regulator is
>> fine?
>> 
>> If that is the case -- and after re-reading the functions exposed on
>> both types `EnabledRegulator` and `Regulator` -- I am confused why we
>> even need two different type states? Both expose the same functions
>> (except `enable` and `disable`) and I don't otherwise see the purpose of
>> having two types.
>> 
>> ---
>> Cheers,
>> Benno
>> 
>
>
> As Mark said:
>
>> IIUC the point is to allow Rust's type system to keep track of the
>> reference on the regulator, otherwise the user code has to keep track of
>> the number of enables it's done like it currently does in C code.
>
> So this all started because keeping track of the enables was rather clunky. See
> v1 [0].
>
> [0] https://lore.kernel.org/rust-for-linux/20250219162517.278362-1-daniel.almeida@collabora.com/

Ahh thanks for the pointer this makes much more sense now. Yeah that's a
reason to have two types.

Please document this properly. A good reference is
`rust/kernel/fs/file.rs`. It also deals with different refcounts and
ownership, so that might give you some idea for how to write the
comments around changes in ownership. Also have a general comment about
the two different refcounts in a `struct regulator` on the two regulator
wrappers.

---
Cheers,
Benno

  reply	other threads:[~2025-05-14 17:41 UTC|newest]

Thread overview: 52+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-05-13 15:44 [PATCH v3] rust: regulator: add a bare minimum regulator abstraction Daniel Almeida
2025-05-13 20:01 ` Benno Lossin
2025-05-14  7:46   ` Mark Brown
2025-05-14  9:37     ` Benno Lossin
2025-05-14 10:16       ` Mark Brown
2025-05-14 10:31         ` Benno Lossin
2025-05-14 11:50           ` Mark Brown
2025-05-14 12:23             ` Benno Lossin
2025-05-14 12:48               ` Mark Brown
2025-05-14 14:06                 ` Benno Lossin
2025-05-14 13:01   ` Daniel Almeida
2025-05-14 13:57     ` Benno Lossin
2025-05-14 14:40       ` Daniel Almeida
2025-05-14 15:38         ` Benno Lossin
2025-05-14 15:50           ` Mark Brown
2025-05-14 16:05             ` Benno Lossin
2025-05-14 16:08               ` Mark Brown
2025-05-14 16:19               ` Daniel Almeida
2025-05-14 17:41                 ` Benno Lossin [this message]
2025-05-14 16:10             ` Daniel Almeida
2025-05-15  8:19               ` Mark Brown
2025-05-14 15:48         ` Mark Brown
2025-05-14  8:27 ` Mark Brown
2025-05-18  2:28 ` Alexandre Courbot
2025-05-18  7:19   ` Benno Lossin
2025-05-18  8:14     ` Alexandre Courbot
2025-05-18  8:30       ` Alexandre Courbot
2025-05-18  9:57         ` Benno Lossin
2025-05-18 11:12           ` Alexandre Courbot
2025-05-18 14:05             ` Benno Lossin
2025-05-19  0:29               ` Alexandre Courbot
2025-05-18 12:20       ` Mark Brown
2025-05-18 12:51         ` Alexandre Courbot
2025-05-19  9:55           ` Mark Brown
2025-05-18 14:04         ` Benno Lossin
2025-05-19  9:56           ` Mark Brown
2025-05-19 11:25             ` Benno Lossin
2025-05-19 11:46               ` Mark Brown
2025-05-19 12:30                 ` Benno Lossin
2025-05-19 12:46                   ` Mark Brown
2025-05-18 12:17   ` Mark Brown
2025-05-18 12:49     ` Alexandre Courbot
2025-05-19  9:54       ` Mark Brown
2025-05-18 15:11   ` Daniel Almeida
2025-05-19  1:25     ` Alexandre Courbot
2025-05-19 10:52       ` Daniel Almeida
2025-05-19 11:01         ` Daniel Almeida
2025-05-19 11:54         ` Benno Lossin
2025-05-19 11:59           ` Miguel Ojeda
2025-05-19 14:43           ` Alexandre Courbot
2025-05-20 18:09             ` Benno Lossin
2025-05-19 14:20         ` 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=D9W2GYMMVKLV.WXCSH0FYL4QC@kernel.org \
    --to=lossin@kernel.org \
    --cc=a.hindborg@kernel.org \
    --cc=alex.gaynor@gmail.com \
    --cc=aliceryhl@google.com \
    --cc=benno.lossin@proton.me \
    --cc=bjorn3_gh@protonmail.com \
    --cc=boqun.feng@gmail.com \
    --cc=boris.brezillon@collabora.com \
    --cc=broonie@kernel.org \
    --cc=dakr@kernel.org \
    --cc=daniel.almeida@collabora.com \
    --cc=gary@garyguo.net \
    --cc=lgirdwood@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=ojeda@kernel.org \
    --cc=rust-for-linux@vger.kernel.org \
    --cc=sebastian.reichel@collabora.com \
    --cc=tmgross@umich.edu \
    /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.