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

[Boqun Feng <boqun.feng@gmail.com>]

On Tue, Sep 09, 2025 at 12:04:35PM -0300, Daniel Almeida wrote:
> Hi Boqun, thanks for chiming in!
> 
> > On 9 Sep 2025, at 03:57, Boqun Feng <boqun.feng@gmail.com> wrote:
> > 
> > On Mon, Sep 08, 2025 at 08:10:28PM -0300, Daniel Almeida wrote:
> >> 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>
> >> Signed-off-by: Daniel Almeida <daniel.almeida@collabora.com>
> >> ---
> >> rust/helpers/regulator.c | 10 +++++++++
> >> rust/kernel/regulator.rs | 58 +++++++++++++++++++++++++++++++++++++++++++++++-
> >> 2 files changed, 67 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);
> >> +}
> >> +
> > 
> > These two functions are already EXPORT_SYMBOL_GPL(), so you won't need
> > to add rust_helper for them. Creating rust_helper_*() for them will just
> > export additional symbols.
> 
> These are inlined (stubbed) if CONFIG_REGULATOR is not set, so we need the
> helpers to get around that, IIUC.
> 

Well, then the question is why we want to compiler regulator.rs if
CONFIG_REGULATOR=n? Shouldn't we do:

#[cfg(CONFIG_REGULATOR)]
pub mod regulator

in rust/kernel/lib.rs?

> In fact, doing the change you proposed will result in Intel´s bot
> complaining. Same for all other functions defined in the helper C file.
> 

With above, we probably can remove the whole helper file for regulator.

Regards,
Boqun

> - Daniel
>