* [PATCH v3 0/2] rust: regulator: improve the ergonomics of Rust regulators
@ 2025-09-10 17:54 Daniel Almeida
2025-09-10 17:54 ` [PATCH v3 1/2] rust: regulator: remove Regulator<Dynamic> Daniel Almeida
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Daniel Almeida @ 2025-09-10 17:54 UTC (permalink / raw)
To: Liam Girdwood, Mark Brown, Miguel Ojeda, Alex Gaynor, Boqun Feng,
Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Alice Ryhl, Trevor Gross, Danilo Krummrich
Cc: linux-kernel, rust-for-linux, Daniel Almeida, Alexandre Courbot
This small series comes after some extensive discussion on a few minor
changes that can improve the current Rust regulator API.
Patch 1 removes Regulator<Dynamic>, as we have now established that
there is no usecase that can't use the safer Regulator<Enabled> and
Regulator<Disabled> APIs instead.
Patch 2 makes "devm_regulator_enable_get" and
"devm_regulator_enable_get_optional" available in Rust. This comes after
realizing that a lot of drivers simply care about whether regulators are
enabled for as long as the device is bound.
Signed-off-by: Daniel Almeida <daniel.almeida@collabora.com>
---
Changes in v3:
- Rework the docs (Danilo, Boqun)
- Pick up tags
- Link to v2: https://lore.kernel.org/r/20250908-regulator-remove-dynamic-v2-0-e575ae2cde6a@collabora.com
Changes in v2:
- Picked up tags
- Rebased on regulator/for-next
- Renamed enable() and enable_optional() to devm_enable() and
devm_enable_optional().
- Renamed patch 2/2 to pick up the above change
- Added more docs and links where applicable (thanks, Alex)
- Link to v1: https://lore.kernel.org/r/20250829-regulator-remove-dynamic-v1-0-deb59205e8e9@collabora.com
---
Daniel Almeida (2):
rust: regulator: remove Regulator<Dynamic>
rust: regulator: add devm_enable and devm_enable_optional
rust/helpers/regulator.c | 10 ++++
rust/kernel/regulator.rs | 148 +++++++++++++++++++----------------------------
2 files changed, 70 insertions(+), 88 deletions(-)
---
base-commit: 59e8e7b7f2206d7097e43266722b625715720dfa
change-id: 20250829-regulator-remove-dynamic-f1a6b8c0c1b0
Best regards,
--
Daniel Almeida <daniel.almeida@collabora.com>
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH v3 1/2] rust: regulator: remove Regulator<Dynamic>
2025-09-10 17:54 [PATCH v3 0/2] rust: regulator: improve the ergonomics of Rust regulators Daniel Almeida
@ 2025-09-10 17:54 ` Daniel Almeida
2025-09-10 17:54 ` [PATCH v3 2/2] rust: regulator: add devm_enable and devm_enable_optional Daniel Almeida
2025-09-11 23:23 ` [PATCH v3 0/2] rust: regulator: improve the ergonomics of Rust regulators Mark Brown
2 siblings, 0 replies; 4+ messages in thread
From: Daniel Almeida @ 2025-09-10 17:54 UTC (permalink / raw)
To: Liam Girdwood, Mark Brown, Miguel Ojeda, Alex Gaynor, Boqun Feng,
Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Alice Ryhl, Trevor Gross, Danilo Krummrich
Cc: linux-kernel, rust-for-linux, Daniel Almeida, Alexandre Courbot
After some experimenting and further discussion, it is starting to look
like Regulator<Dynamic> might be a footgun. It turns out that one can
get the same behavior by correctly using just Regulator<Enabled> and
Regulator<Disabled>, so there is no need to directly expose the manual
refcounting ability of Regulator<Dynamic> to clients.
Remove it while we do not have any other users.
Suggested-by: Danilo Krummrich <dakr@kernel.org>
Reviewed-by: Alice Ryhl <aliceryhl@google.com>
Reviewed-by: Danilo Krummrich <dakr@kernel.org>
Reviewed-by: Alexandre Courbot <acourbot@nvidia.com>
Signed-off-by: Daniel Almeida <daniel.almeida@collabora.com>
---
rust/kernel/regulator.rs | 88 +-----------------------------------------------
1 file changed, 1 insertion(+), 87 deletions(-)
diff --git a/rust/kernel/regulator.rs b/rust/kernel/regulator.rs
index 34bb24ec8d4d436437d92a344c61fc3a46de0b5d..5ea2307f02df4a10c1c8c07b3b8c134d13519b69 100644
--- a/rust/kernel/regulator.rs
+++ b/rust/kernel/regulator.rs
@@ -30,7 +30,6 @@ pub trait Sealed {}
impl Sealed for super::Enabled {}
impl Sealed for super::Disabled {}
- impl Sealed for super::Dynamic {}
}
/// A trait representing the different states a [`Regulator`] can be in.
@@ -50,13 +49,6 @@ pub trait RegulatorState: private::Sealed + 'static {
/// own an `enable` reference count, but the regulator may still be on.
pub struct Disabled;
-/// A state that models the C API. The [`Regulator`] can be either enabled or
-/// disabled, and the user is in control of the reference count. This is also
-/// the default state.
-///
-/// Use [`Regulator::is_enabled`] to check the regulator's current state.
-pub struct Dynamic;
-
impl RegulatorState for Enabled {
const DISABLE_ON_DROP: bool = true;
}
@@ -65,14 +57,9 @@ impl RegulatorState for Disabled {
const DISABLE_ON_DROP: bool = false;
}
-impl RegulatorState for Dynamic {
- const DISABLE_ON_DROP: bool = false;
-}
-
/// A trait that abstracts the ability to check if a [`Regulator`] is enabled.
pub trait IsEnabled: RegulatorState {}
impl IsEnabled for Disabled {}
-impl IsEnabled for Dynamic {}
/// An error that can occur when trying to convert a [`Regulator`] between states.
pub struct Error<State: RegulatorState> {
@@ -183,64 +170,13 @@ pub struct Error<State: RegulatorState> {
/// }
/// ```
///
-/// ## Using [`Regulator<Dynamic>`]
-///
-/// This example mimics the behavior of the C API, where the user is in
-/// control of the enabled reference count. This is useful for drivers that
-/// might call enable and disable to manage the `enable` reference count at
-/// runtime, perhaps as a result of `open()` and `close()` calls or whatever
-/// other driver-specific or subsystem-specific hooks.
-///
-/// ```
-/// # use kernel::prelude::*;
-/// # use kernel::c_str;
-/// # use kernel::device::Device;
-/// # use kernel::regulator::{Regulator, Dynamic};
-/// struct PrivateData {
-/// regulator: Regulator<Dynamic>,
-/// }
-///
-/// // A fictictious probe function that obtains a regulator and sets it up.
-/// fn probe(dev: &Device) -> Result<PrivateData> {
-/// // Obtain a reference to a (fictitious) regulator.
-/// let regulator = Regulator::<Dynamic>::get(dev, c_str!("vcc"))?;
-///
-/// Ok(PrivateData { regulator })
-/// }
-///
-/// // A fictictious function that indicates that the device is going to be used.
-/// fn open(dev: &Device, data: &PrivateData) -> Result {
-/// // Increase the `enabled` reference count.
-/// data.regulator.enable()?;
-///
-/// Ok(())
-/// }
-///
-/// fn close(dev: &Device, data: &PrivateData) -> Result {
-/// // Decrease the `enabled` reference count.
-/// data.regulator.disable()?;
-///
-/// Ok(())
-/// }
-///
-/// fn remove(dev: &Device, data: PrivateData) -> Result {
-/// // `PrivateData` is dropped here, which will drop the
-/// // `Regulator<Dynamic>` in turn.
-/// //
-/// // The reference that was obtained by `regulator_get()` will be
-/// // released, but it is up to the user to make sure that the number of calls
-/// // to `enable()` and `disabled()` are balanced before this point.
-/// Ok(())
-/// }
-/// ```
-///
/// # Invariants
///
/// - `inner` is a non-null wrapper over a pointer to a `struct
/// regulator` obtained from [`regulator_get()`].
///
/// [`regulator_get()`]: https://docs.kernel.org/driver-api/regulator.html#c.regulator_get
-pub struct Regulator<State = Dynamic>
+pub struct Regulator<State>
where
State: RegulatorState,
{
@@ -351,28 +287,6 @@ pub fn try_into_disabled(self) -> Result<Regulator<Disabled>, Error<Enabled>> {
}
}
-impl Regulator<Dynamic> {
- /// Obtains a [`Regulator`] instance from the system. The current state of
- /// the regulator is unknown and it is up to the user to manage the enabled
- /// reference count.
- ///
- /// This closely mimics the behavior of the C API and can be used to
- /// dynamically manage the enabled reference count at runtime.
- pub fn get(dev: &Device, name: &CStr) -> Result<Self> {
- Regulator::get_internal(dev, name)
- }
-
- /// Increases the `enabled` reference count.
- pub fn enable(&self) -> Result {
- self.enable_internal()
- }
-
- /// Decreases the `enabled` reference count.
- pub fn disable(&self) -> Result {
- self.disable_internal()
- }
-}
-
impl<T: IsEnabled> Regulator<T> {
/// Checks if the regulator is enabled.
pub fn is_enabled(&self) -> bool {
--
2.51.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH v3 2/2] rust: regulator: add devm_enable and devm_enable_optional
2025-09-10 17:54 [PATCH v3 0/2] rust: regulator: improve the ergonomics of Rust regulators Daniel Almeida
2025-09-10 17:54 ` [PATCH v3 1/2] rust: regulator: remove Regulator<Dynamic> Daniel Almeida
@ 2025-09-10 17:54 ` Daniel Almeida
2025-09-11 23:23 ` [PATCH v3 0/2] rust: regulator: improve the ergonomics of Rust regulators Mark Brown
2 siblings, 0 replies; 4+ messages in thread
From: Daniel Almeida @ 2025-09-10 17:54 UTC (permalink / raw)
To: Liam Girdwood, Mark Brown, Miguel Ojeda, Alex Gaynor, Boqun Feng,
Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Alice Ryhl, Trevor Gross, Danilo Krummrich
Cc: linux-kernel, rust-for-linux, Daniel Almeida, Alexandre Courbot
A lot of drivers only care about enabling the regulator for as long as
the underlying Device is bound. This can be easily observed due to the
extensive use of `devm_regulator_get_enable` and
`devm_regulator_get_enable_optional` throughout the kernel.
Therefore, make this helper available in Rust. Also add an example
noting how it should be the default API unless the driver needs more
fine-grained control over the regulator.
Suggested-by: Danilo Krummrich <dakr@kernel.org>
Reviewed-by: Boqun Feng <boqun.feng@gmail.com>
Reviewed-by: Danilo Krummrich <dakr@kernel.org>
Reviewed-by: Alexandre Courbot <acourbot@nvidia.com>
Signed-off-by: Daniel Almeida <daniel.almeida@collabora.com>
---
rust/helpers/regulator.c | 10 ++++++++
rust/kernel/regulator.rs | 60 +++++++++++++++++++++++++++++++++++++++++++++++-
2 files changed, 69 insertions(+), 1 deletion(-)
diff --git a/rust/helpers/regulator.c b/rust/helpers/regulator.c
index cd8b7ba648ee33dd14326c9242fb6c96ab8e32a7..11bc332443bd064f4b5afd350ffc045badff9076 100644
--- a/rust/helpers/regulator.c
+++ b/rust/helpers/regulator.c
@@ -40,4 +40,14 @@ int rust_helper_regulator_is_enabled(struct regulator *regulator)
return regulator_is_enabled(regulator);
}
+int rust_helper_devm_regulator_get_enable(struct device *dev, const char *id)
+{
+ return devm_regulator_get_enable(dev, id);
+}
+
+int rust_helper_devm_regulator_get_enable_optional(struct device *dev, const char *id)
+{
+ return devm_regulator_get_enable_optional(dev, id);
+}
+
#endif
diff --git a/rust/kernel/regulator.rs b/rust/kernel/regulator.rs
index 5ea2307f02df4a10c1c8c07b3b8c134d13519b69..b55a201e5029fdfbf51d2d54180dc82372d7a571 100644
--- a/rust/kernel/regulator.rs
+++ b/rust/kernel/regulator.rs
@@ -18,7 +18,7 @@
use crate::{
bindings,
- device::Device,
+ device::{Bound, Device},
error::{from_err_ptr, to_result, Result},
prelude::*,
};
@@ -69,6 +69,41 @@ pub struct Error<State: RegulatorState> {
/// The regulator that caused the error, so that the operation may be retried.
pub regulator: Regulator<State>,
}
+/// Obtains and enables a [`devres`]-managed regulator for a device.
+///
+/// This calls [`regulator_disable()`] and [`regulator_put()`] automatically on
+/// driver detach.
+///
+/// This API is identical to `devm_regulator_get_enable()`, and should be
+/// preferred over the [`Regulator<T: RegulatorState>`] API if the caller only
+/// cares about the regulator being enabled.
+///
+/// [`devres`]: https://docs.kernel.org/driver-api/driver-model/devres.html
+/// [`regulator_disable()`]: https://docs.kernel.org/driver-api/regulator.html#c.regulator_disable
+/// [`regulator_put()`]: https://docs.kernel.org/driver-api/regulator.html#c.regulator_put
+pub fn devm_enable(dev: &Device<Bound>, name: &CStr) -> Result {
+ // SAFETY: `dev` is a valid and bound device, while `name` is a valid C
+ // string.
+ to_result(unsafe { bindings::devm_regulator_get_enable(dev.as_raw(), name.as_ptr()) })
+}
+
+/// Same as [`devm_enable`], but calls `devm_regulator_get_enable_optional`
+/// instead.
+///
+/// This obtains and enables a [`devres`]-managed regulator for a device, but
+/// does not print a message nor provides a dummy if the regulator is not found.
+///
+/// This calls [`regulator_disable()`] and [`regulator_put()`] automatically on
+/// driver detach.
+///
+/// [`devres`]: https://docs.kernel.org/driver-api/driver-model/devres.html
+/// [`regulator_disable()`]: https://docs.kernel.org/driver-api/regulator.html#c.regulator_disable
+/// [`regulator_put()`]: https://docs.kernel.org/driver-api/regulator.html#c.regulator_put
+pub fn devm_enable_optional(dev: &Device<Bound>, name: &CStr) -> Result {
+ // SAFETY: `dev` is a valid and bound device, while `name` is a valid C
+ // string.
+ to_result(unsafe { bindings::devm_regulator_get_enable_optional(dev.as_raw(), name.as_ptr()) })
+}
/// A `struct regulator` abstraction.
///
@@ -146,6 +181,29 @@ pub struct Error<State: RegulatorState> {
/// }
/// ```
///
+/// If a driver only cares about the regulator being on for as long it is bound
+/// to a device, then it should use [`devm_enable`] or [`devm_enable_optional`].
+/// This should be the default use-case unless more fine-grained control over
+/// the regulator's state is required.
+///
+/// [`devm_enable`]: crate::regulator::devm_enable
+/// [`devm_optional`]: crate::regulator::devm_enable_optional
+///
+/// ```
+/// # use kernel::prelude::*;
+/// # use kernel::c_str;
+/// # use kernel::device::{Bound, Device};
+/// # use kernel::regulator;
+/// fn enable(dev: &Device<Bound>) -> Result {
+/// // Obtain a reference to a (fictitious) regulator and enable it. This
+/// // call only returns whether the operation succeeded.
+/// regulator::devm_enable(dev, c_str!("vcc"))?;
+///
+/// // The regulator will be disabled and put when `dev` is unbound.
+/// Ok(())
+/// }
+/// ```
+///
/// ## Disabling a regulator
///
/// ```
--
2.51.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v3 0/2] rust: regulator: improve the ergonomics of Rust regulators
2025-09-10 17:54 [PATCH v3 0/2] rust: regulator: improve the ergonomics of Rust regulators Daniel Almeida
2025-09-10 17:54 ` [PATCH v3 1/2] rust: regulator: remove Regulator<Dynamic> Daniel Almeida
2025-09-10 17:54 ` [PATCH v3 2/2] rust: regulator: add devm_enable and devm_enable_optional Daniel Almeida
@ 2025-09-11 23:23 ` Mark Brown
2 siblings, 0 replies; 4+ messages in thread
From: Mark Brown @ 2025-09-11 23:23 UTC (permalink / raw)
To: Liam Girdwood, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Danilo Krummrich, Daniel Almeida
Cc: linux-kernel, rust-for-linux, Alexandre Courbot
On Wed, 10 Sep 2025 14:54:30 -0300, Daniel Almeida wrote:
> This small series comes after some extensive discussion on a few minor
> changes that can improve the current Rust regulator API.
>
> Patch 1 removes Regulator<Dynamic>, as we have now established that
> there is no usecase that can't use the safer Regulator<Enabled> and
> Regulator<Disabled> APIs instead.
>
> [...]
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git for-next
Thanks!
[1/2] rust: regulator: remove Regulator<Dynamic>
commit: b87ecbc54f22382ace1cf41645e8652a4ce44d52
[2/2] rust: regulator: add devm_enable and devm_enable_optional
commit: 2e0fd4583d0efcdc260e61a22666c8368f505353
All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying
to this mail.
Thanks,
Mark
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2025-09-11 23:23 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-09-10 17:54 [PATCH v3 0/2] rust: regulator: improve the ergonomics of Rust regulators Daniel Almeida
2025-09-10 17:54 ` [PATCH v3 1/2] rust: regulator: remove Regulator<Dynamic> Daniel Almeida
2025-09-10 17:54 ` [PATCH v3 2/2] rust: regulator: add devm_enable and devm_enable_optional Daniel Almeida
2025-09-11 23:23 ` [PATCH v3 0/2] rust: regulator: improve the ergonomics of Rust regulators Mark Brown
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).