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

[Daniel Almeida]

> On 9 Sep 2025, at 12:38, Boqun Feng <boqun.feng@xxxxxxxxx> wrote:
> 
> 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@xxxxxxxxx> 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@xxxxxxxxxx>
>>>> Signed-off-by: Daniel Almeida <daniel.almeida@xxxxxxxxxxxxx>
>>>> ---
>>>> 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:

Yes, we do want to compile regulator .rs. There’s been prior discussion on
this (see [0] below, but also [1]).

> 
> #[cfg(CONFIG_REGULATOR)]
> pub mod regulator
> 
> in rust/kernel/lib.rs?

This was the original approach, but this is incorrect behavior (again, see
[0]). The right thing to do is to call the stubs if CONFIG_REGULATOR is not
set, not make the regulator API unavailable.

> 
>> 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.

Given the above, we cannot remove the file, and we must also add the devm
helpers as they are in the current patch.

— Daniel

[0] https://lore.kernel.org/rust-for-linux/a1b3561d-f5de-4474-85ef-1525a6c36bc5@xxxxxxx/T/#mdf9d4005ee99929d0009ccc988efbc0789164b6d

[1] https://lore.kernel.org/rust-for-linux/25e9a9b6-4d81-4731-98fa-add40ccd4aab@xxxxxxxxxxxxx/