Re: [PATCH v2 2/2] rust: regulator: add devm_enable and devm_enable_optional

["Danilo Krummrich" <dakr@kernel.org>]

On Tue Sep 9, 2025 at 1:10 AM CEST, Daniel Almeida wrote:
> +/// Enables a regulator whose lifetime is tied to the lifetime of `dev` through
> +/// [`devres`].
> +///
> +/// This calls `regulator_disable()` and `regulator_put()` automatically on
> +/// driver detach.
> +///
> +/// This API is identical to `devm_regulator_get_enable()`, and should be
> +/// preferred if the caller only cares about the regulator being on.

Preferred over what? :)

I'd link to the alternative Regulator API.

> +///
> +/// [`devres`]: https://docs.kernel.org/driver-api/driver-model/devres.html
> +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 enables a regulator whose lifetime is tied to the lifetime of `dev`
> +/// through [`devres`], 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
> +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.
>  ///
>  /// # Examples
> @@ -146,6 +179,29 @@ pub struct Error<State: RegulatorState> {
>  /// }
>  /// ```
>  ///
> +/// If a driver only cares about the regulator being on for as long it is bound

s/on/enabled/

> +/// to a device, then it should use [`devm_enable`] or [`devm_enable_optional`].
> +/// This should be the default use-case unless they need more fine-grained
> +/// control over the regulator's state.

"unless more fine-grained control over the regulator's state is required"

With those nits addressed and the unnecessary helpers removed,

Reviewed-by: Danilo Krummrich <dakr@kernel.org>