linux-kernel - Re: [PATCH v3] rust: regulator: add a bare minimum regulator abstraction

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <D9Z3R4EYAXV9.211IFNRTOPM6O@kernel.org>
Date: Sun, 18 May 2025 09:19:23 +0200
From: "Benno Lossin" <lossin@...nel.org>
To: "Alexandre Courbot" <acourbot@...dia.com>, "Daniel Almeida"
 <daniel.almeida@...labora.com>, "Miguel Ojeda" <ojeda@...nel.org>, "Alex
 Gaynor" <alex.gaynor@...il.com>, "Boqun Feng" <boqun.feng@...il.com>, "Gary
 Guo" <gary@...yguo.net>, Björn Roy Baron
 <bjorn3_gh@...tonmail.com>, "Benno Lossin" <benno.lossin@...ton.me>,
 "Andreas Hindborg" <a.hindborg@...nel.org>, "Alice Ryhl"
 <aliceryhl@...gle.com>, "Trevor Gross" <tmgross@...ch.edu>, "Danilo
 Krummrich" <dakr@...nel.org>, "Boris Brezillon"
 <boris.brezillon@...labora.com>, "Sebastian Reichel"
 <sebastian.reichel@...labora.com>, "Liam Girdwood" <lgirdwood@...il.com>,
 "Mark Brown" <broonie@...nel.org>
Cc: <linux-kernel@...r.kernel.org>, <rust-for-linux@...r.kernel.org>
Subject: Re: [PATCH v3] rust: regulator: add a bare minimum regulator
 abstraction

On Sun May 18, 2025 at 4:28 AM CEST, Alexandre Courbot wrote:
> On Wed May 14, 2025 at 12:44 AM JST, Daniel Almeida wrote:
>> +//! Regulator abstractions, providing a standard kernel interface to control
>> +//! voltage and current regulators.
>> +//!
>> +//! The intention is to allow systems to dynamically control regulator power
>> +//! output in order to save power and prolong battery life. This applies to both
>> +//! voltage regulators (where voltage output is controllable) and current sinks
>> +//! (where current limit is controllable).
>> +//!
>> +//! C header: [`include/linux/regulator/consumer.h`](srctree/include/linux/regulator/consumer.h)
>> +//!
>> +//! Regulators are modeled in Rust with two types: [`Regulator`] and
>> +//! [`EnabledRegulator`].
>> +//!
>> +//! The transition between these types is done by calling
>> +//! [`Regulator::enable()`] and [`EnabledRegulator::disable()`] respectively.
>> +//!
>> +//! Use an enum or [`kernel::types::Either`] to gracefully transition between
>> +//! the two states at runtime if needed. Store [`EnabledRegulator`] directly
>> +//! otherwise.
>
> Having the enabled or disabled state baked into the type is indeed
> valuable for drivers that just need to acquire and enable a regulator at
> probe time. However, there are also more dynamic use cases and I don't
> think the burden of managing this aspect - by either performing a manual
> match to call any method (even the shared ones), or implementing custom
> dispatch types (which will lead to many similar ad-hoc implementations)
> - should fall on the user. Thus I strongly suggest that this module
> provides a solution for this as well.
>
> It has been proposed earlier to use a typestate, and this would indeed
> provide several benefits, the first one being the ability to have shared
> impl blocks (and shared documentation) between the enabled and disabled
> states for methods like set/get_voltage().
>
> But the key benefit I see is that it could also address the
> aforementioned dynamic management problem through the introduction of a
> third state.
>
> Alongside the `Enabled` and `Disabled` states, there would be a third
> state (`Dynamic`?) in which the regulator could either be enabled or
> disabled. This `Dynamic` state is the only one providing `enable` and
> `disable` methods (as well as `is_enabled`) to change its operational
> state without affecting its type.
>
> All three states then implement `set_voltage` and `get_voltage` through
> a common impl block, that could be extended with other methods from the
> C API that are independent of the state, as needed.
>
> To handle typestate transitions:
>
> - The `Disabled` and `Dynamic` states provide a `try_into_enabled()`
>   method to transition the regulator to the `Enabled` state.
> - The `Enabled` and `Dynamic` states provide `try_into_disabled()`.
> - `Enabled` and `Disabled` also provide `into_dynamic()` (which cannot
>   fail).
>
> Essentially, the `Enabled` and `Disabled` states simply enforce an
> additional operational state invariant on the underlying regulator, and
> do not provide methods to change it.
>
> The `Dynamic` state would be the default for `Regulator`, so by just
> using `Regulator`, the user gets an interface that works very similarly
> to the C API it abstracts, making it intuitive to those familiar with
> it.

How will the `Dynamic` typestate track the enable refcount? AFAIK one
has to drop all enable refcounts before removing the regulator. Also
what happens when I call `disable` without any enable calls before?

---
Cheers,
Benno