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

["Benno Lossin" <lossin@kernel.org>]

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