[PATCH v2] regulator: qcom-rpmh: Add support for regulator-off-on-delay-us

Saikiran posted 1 patch 1 week, 3 days ago
drivers/regulator/qcom-rpmh-regulator.c | 3 +++
1 file changed, 3 insertions(+)
[PATCH v2] regulator: qcom-rpmh: Add support for regulator-off-on-delay-us
Posted by Saikiran 1 week, 3 days ago
The core regulator framework supports enforcing a physical off-time via
standard properties, but the `qcom-rpmh-regulator` driver currently ignores
them. This prevents boards with slow-discharging rails from enforcing safe
power-cycling constraints.

On the Lenovo Yoga Slim 7x (Snapdragon X Elite), certain camera regulators
rely on passive discharge and require a significant off-time to drop below
brownout thresholds. Without this driver support, we cannot enforce this
constraint via Device Tree, leading to sensor initialization failures during
rapid power cycling.

Add support for parsing the `regulator-off-on-delay-us` property from
the device tree.

I have tested this on the Yoga Slim 7x. When the delay property is present
in the device tree, the regulator core correctly blocks re-enable calls
until the delay passes, fixing the camera brownout issues.

Signed-off-by: Saikiran <bjsaikiran@gmail.com>
---
 drivers/regulator/qcom-rpmh-regulator.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/regulator/qcom-rpmh-regulator.c b/drivers/regulator/qcom-rpmh-regulator.c
index 6e4cb2871fca..aafba61551b3 100644
--- a/drivers/regulator/qcom-rpmh-regulator.c
+++ b/drivers/regulator/qcom-rpmh-regulator.c
@@ -503,6 +503,9 @@ static int rpmh_regulator_init_vreg(struct rpmh_vreg *vreg, struct device *dev,
 	vreg->always_wait_for_ack = of_property_read_bool(node,
 						"qcom,always-wait-for-ack");
 
+	of_property_read_u32(node, "regulator-off-on-delay-us",
+			     &vreg->rdesc.off_on_delay);
+
 	vreg->rdesc.owner	= THIS_MODULE;
 	vreg->rdesc.type	= REGULATOR_VOLTAGE;
 	vreg->rdesc.ops		= vreg->hw_data->ops;
-- 
2.51.0
Re: [PATCH v2] regulator: qcom-rpmh: Add support for regulator-off-on-delay-us
Posted by Konrad Dybcio 1 week, 2 days ago
On 1/27/26 6:37 PM, Saikiran wrote:
> The core regulator framework supports enforcing a physical off-time via
> standard properties, but the `qcom-rpmh-regulator` driver currently ignores
> them. This prevents boards with slow-discharging rails from enforcing safe
> power-cycling constraints.
> 
> On the Lenovo Yoga Slim 7x (Snapdragon X Elite), certain camera regulators
> rely on passive discharge and require a significant off-time to drop below
> brownout thresholds. Without this driver support, we cannot enforce this
> constraint via Device Tree, leading to sensor initialization failures during
> rapid power cycling.
> 
> Add support for parsing the `regulator-off-on-delay-us` property from
> the device tree.
> 
> I have tested this on the Yoga Slim 7x. When the delay property is present
> in the device tree, the regulator core correctly blocks re-enable calls
> until the delay passes, fixing the camera brownout issues.
> 
> Signed-off-by: Saikiran <bjsaikiran@gmail.com>
> ---
>  drivers/regulator/qcom-rpmh-regulator.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/regulator/qcom-rpmh-regulator.c b/drivers/regulator/qcom-rpmh-regulator.c
> index 6e4cb2871fca..aafba61551b3 100644
> --- a/drivers/regulator/qcom-rpmh-regulator.c
> +++ b/drivers/regulator/qcom-rpmh-regulator.c
> @@ -503,6 +503,9 @@ static int rpmh_regulator_init_vreg(struct rpmh_vreg *vreg, struct device *dev,
>  	vreg->always_wait_for_ack = of_property_read_bool(node,
>  						"qcom,always-wait-for-ack");
>  
> +	of_property_read_u32(node, "regulator-off-on-delay-us",
> +			     &vreg->rdesc.off_on_delay);

Would it not be a better fit for of_regulator.c?

Konrad
Re: [PATCH v2] regulator: qcom-rpmh: Add support for regulator-off-on-delay-us
Posted by Saikiran B 1 week, 2 days ago
On Wed, Jan 28, 2026 at 4:00 PM Konrad Dybcio
<konrad.dybcio@oss.qualcomm.com> wrote:
>
> On 1/27/26 6:37 PM, Saikiran wrote:
> > The core regulator framework supports enforcing a physical off-time via
> > standard properties, but the `qcom-rpmh-regulator` driver currently ignores
> > them. This prevents boards with slow-discharging rails from enforcing safe
> > power-cycling constraints.
> >
> > On the Lenovo Yoga Slim 7x (Snapdragon X Elite), certain camera regulators
> > rely on passive discharge and require a significant off-time to drop below
> > brownout thresholds. Without this driver support, we cannot enforce this
> > constraint via Device Tree, leading to sensor initialization failures during
> > rapid power cycling.
> >
> > Add support for parsing the `regulator-off-on-delay-us` property from
> > the device tree.
> >
> > I have tested this on the Yoga Slim 7x. When the delay property is present
> > in the device tree, the regulator core correctly blocks re-enable calls
> > until the delay passes, fixing the camera brownout issues.
> >
> > Signed-off-by: Saikiran <bjsaikiran@gmail.com>
> > ---
> >  drivers/regulator/qcom-rpmh-regulator.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/drivers/regulator/qcom-rpmh-regulator.c b/drivers/regulator/qcom-rpmh-regulator.c
> > index 6e4cb2871fca..aafba61551b3 100644
> > --- a/drivers/regulator/qcom-rpmh-regulator.c
> > +++ b/drivers/regulator/qcom-rpmh-regulator.c
> > @@ -503,6 +503,9 @@ static int rpmh_regulator_init_vreg(struct rpmh_vreg *vreg, struct device *dev,
> >       vreg->always_wait_for_ack = of_property_read_bool(node,
> >                                               "qcom,always-wait-for-ack");
> >
> > +     of_property_read_u32(node, "regulator-off-on-delay-us",
> > +                          &vreg->rdesc.off_on_delay);
>
> Would it not be a better fit for of_regulator.c?
>
> Konrad

Hi Konrad,

On Tue, Jan 27, 2026 at 4:00 PM Konrad Dybcio wrote:
> Would it not be a better fit for of_regulator.c?

That was my initial thought as well, but there is a limitation:

The `off_on_delay` field resides in `struct regulator_desc`. For most
regulator drivers, this structure is `static const` (immutable) as it describes
fixed silicon characteristics. Generic parsing code in `of_regulator.c`
cannot blindly write to `desc->off_on_delay` without risking a write to
read-only memory.

The `qcom-rpmh-regulator` driver is unique in that it allocates
`vreg->rdesc` dynamically at runtime, which allows us to safely populate
this field from DT.

To support this generically in `of_regulator.c`, we would likely need to
introduce `off_on_delay` into `struct regulator_constraints` instead,
and then update the core regulator handling to check both sources.

I opted for this driver-specific approach to minimize impact on the core
subsystem, given that `qcom-rpmh` is already set up to handle dynamic
descriptors.

Let me know if you would prefer I attempt the core framework change instead.

Regards,
Saikiran
Re: [PATCH v2] regulator: qcom-rpmh: Add support for regulator-off-on-delay-us
Posted by Mark Brown 1 week, 2 days ago
On Wed, Jan 28, 2026 at 05:04:36PM +0530, Saikiran B wrote:

> The `off_on_delay` field resides in `struct regulator_desc`. For most
> regulator drivers, this structure is `static const` (immutable) as it describes
> fixed silicon characteristics. Generic parsing code in `of_regulator.c`
> cannot blindly write to `desc->off_on_delay` without risking a write to
> read-only memory.

Your reading there is right, generally the expectation was that this
should be a property of the regulator rather than the system.  This case
seems pretty unusual thus far but perhaps we'll see more such cases in
future and should move the property but for now having it enabled per
driver seems safer.

> The `qcom-rpmh-regulator` driver is unique in that it allocates
> `vreg->rdesc` dynamically at runtime, which allows us to safely populate
> this field from DT.

> To support this generically in `of_regulator.c`, we would likely need to
> introduce `off_on_delay` into `struct regulator_constraints` instead,
> and then update the core regulator handling to check both sources.

Yes, indeed - if we were supporting the property completely generically
we'd just not have the driver fill in in the information and store the
actual value used separately with the DT property overriding the driver
value (possibly only if it was larger).
Re: [PATCH v2] regulator: qcom-rpmh: Add support for regulator-off-on-delay-us
Posted by Saikiran B 1 week, 2 days ago
On Wed, Jan 28, 2026 at 5:25 PM Mark Brown <broonie@kernel.org> wrote:
>
> On Wed, Jan 28, 2026 at 05:04:36PM +0530, Saikiran B wrote:
>
> > The `off_on_delay` field resides in `struct regulator_desc`. For most
> > regulator drivers, this structure is `static const` (immutable) as it describes
> > fixed silicon characteristics. Generic parsing code in `of_regulator.c`
> > cannot blindly write to `desc->off_on_delay` without risking a write to
> > read-only memory.
>
> Your reading there is right, generally the expectation was that this
> should be a property of the regulator rather than the system.  This case
> seems pretty unusual thus far but perhaps we'll see more such cases in
> future and should move the property but for now having it enabled per
> driver seems safer.
>
> > The `qcom-rpmh-regulator` driver is unique in that it allocates
> > `vreg->rdesc` dynamically at runtime, which allows us to safely populate
> > this field from DT.
>
> > To support this generically in `of_regulator.c`, we would likely need to
> > introduce `off_on_delay` into `struct regulator_constraints` instead,
> > and then update the core regulator handling to check both sources.
>
> Yes, indeed - if we were supporting the property completely generically
> we'd just not have the driver fill in in the information and store the
> actual value used separately with the DT property overriding the driver
> value (possibly only if it was larger).

Thanks for the confirmation, Mark.

Since the driver-specific approach is architecturally acceptable, I will keep
this implementation in mind as a fallback.

However, to address the root cause of the camera brownout once and for all,
I am now investigating porting the Active Discharge logic (sending specific
voltage/mode resource commands) from similar Qualcomm sister boards into
`qcom-rpmh-regulator`.

That would be the superior hardware solution, eliminating the need for
arbitrary delays. Since that involves complex RPMh resource handling and
reverse-engineering the specific commands for this PMIC, it will take some
time to implement and validate.

I will hold off on pursuing this delay patch further until I have exhausted
the Active Discharge path.

I thank you and konrad for providing the feedback.

Regards,
Saikiran
Re: [PATCH v2] regulator: qcom-rpmh: Add support for regulator-off-on-delay-us
Posted by Mark Brown 1 week, 2 days ago
On Wed, Jan 28, 2026 at 07:29:34PM +0530, Saikiran B wrote:

> However, to address the root cause of the camera brownout once and for all,
> I am now investigating porting the Active Discharge logic (sending specific
> voltage/mode resource commands) from similar Qualcomm sister boards into
> `qcom-rpmh-regulator`.

> That would be the superior hardware solution, eliminating the need for
> arbitrary delays. Since that involves complex RPMh resource handling and
> reverse-engineering the specific commands for this PMIC, it will take some
> time to implement and validate.

> I will hold off on pursuing this delay patch further until I have exhausted
> the Active Discharge path.

Yes, active discharge would definitely be a better solution.
Re: [PATCH v2] regulator: qcom-rpmh: Add support for regulator-off-on-delay-us
Posted by Konrad Dybcio 1 week, 2 days ago
On 1/28/26 12:34 PM, Saikiran B wrote:
> On Wed, Jan 28, 2026 at 4:00 PM Konrad Dybcio
> <konrad.dybcio@oss.qualcomm.com> wrote:
>>
>> On 1/27/26 6:37 PM, Saikiran wrote:
>>> The core regulator framework supports enforcing a physical off-time via
>>> standard properties, but the `qcom-rpmh-regulator` driver currently ignores
>>> them. This prevents boards with slow-discharging rails from enforcing safe
>>> power-cycling constraints.
>>>
>>> On the Lenovo Yoga Slim 7x (Snapdragon X Elite), certain camera regulators
>>> rely on passive discharge and require a significant off-time to drop below
>>> brownout thresholds. Without this driver support, we cannot enforce this
>>> constraint via Device Tree, leading to sensor initialization failures during
>>> rapid power cycling.
>>>
>>> Add support for parsing the `regulator-off-on-delay-us` property from
>>> the device tree.
>>>
>>> I have tested this on the Yoga Slim 7x. When the delay property is present
>>> in the device tree, the regulator core correctly blocks re-enable calls
>>> until the delay passes, fixing the camera brownout issues.
>>>
>>> Signed-off-by: Saikiran <bjsaikiran@gmail.com>
>>> ---
>>>  drivers/regulator/qcom-rpmh-regulator.c | 3 +++
>>>  1 file changed, 3 insertions(+)
>>>
>>> diff --git a/drivers/regulator/qcom-rpmh-regulator.c b/drivers/regulator/qcom-rpmh-regulator.c
>>> index 6e4cb2871fca..aafba61551b3 100644
>>> --- a/drivers/regulator/qcom-rpmh-regulator.c
>>> +++ b/drivers/regulator/qcom-rpmh-regulator.c
>>> @@ -503,6 +503,9 @@ static int rpmh_regulator_init_vreg(struct rpmh_vreg *vreg, struct device *dev,
>>>       vreg->always_wait_for_ack = of_property_read_bool(node,
>>>                                               "qcom,always-wait-for-ack");
>>>
>>> +     of_property_read_u32(node, "regulator-off-on-delay-us",
>>> +                          &vreg->rdesc.off_on_delay);
>>
>> Would it not be a better fit for of_regulator.c?
>>
>> Konrad
> 
> Hi Konrad,
> 
> On Tue, Jan 27, 2026 at 4:00 PM Konrad Dybcio wrote:
>> Would it not be a better fit for of_regulator.c?
> 
> That was my initial thought as well, but there is a limitation:
> 
> The `off_on_delay` field resides in `struct regulator_desc`. For most
> regulator drivers, this structure is `static const` (immutable) as it describes
> fixed silicon characteristics. Generic parsing code in `of_regulator.c`
> cannot blindly write to `desc->off_on_delay` without risking a write to
> read-only memory.
> 
> The `qcom-rpmh-regulator` driver is unique in that it allocates
> `vreg->rdesc` dynamically at runtime, which allows us to safely populate
> this field from DT.
> 
> To support this generically in `of_regulator.c`, we would likely need to
> introduce `off_on_delay` into `struct regulator_constraints` instead,
> and then update the core regulator handling to check both sources.
> 
> I opted for this driver-specific approach to minimize impact on the core
> subsystem, given that `qcom-rpmh` is already set up to handle dynamic
> descriptors.
> 
> Let me know if you would prefer I attempt the core framework change instead.

I'm a fly-by reviewer for this sort of thing (as you can see by me not
knowing this reasoning..)

Mark (the maintainer) should be able to give you a more insightful answer

Konrad