drivers/regulator/qcom-rpmh-regulator.c | 3 +++ 1 file changed, 3 insertions(+)
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
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
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
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).
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
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.
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
© 2016 - 2026 Red Hat, Inc.