[PATCH RFC] regulator: core: fix constraints handling if current state out of range

[PATCH RFC] regulator: core: fix constraints handling if current state out of range

[Andreas Kemnade]

Given a regulator set to an initially out of constraints value,
which cannot be set to the exact voltage given in the min or max constraints,
current code tries to apply a fixed value which obviously fails.

To fix that, allow a range as a constraint in out of bound cases.

A practical use case for this scenario is if there is a quotient
in uV_step which needs to get rounded.
The devicetree should describe the hardware and not depend on the
idea of a specific driver implementation how to round things,
so a small range need to be specified.
Instead of rounding uV_step another user of the devicetree
might be specifying uV_max and uV_min and therefore only needs
to round the end result leading to slightly different results.

Stumbled upon it while creating a regulator with uV_step = 5000000 / 255

Signed-off-by: Andreas Kemnade <akemnade@kernel.org>
Fixes: fa93fd4ecc9c ("regulator: core: Ensure we are at least in bounds for our constraints")
---
not tested yet to avoid magic smoke in case of errors. I rather prefer
having a second pair of eyes on it in this sensitive area.
---
 drivers/regulator/core.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index dd7b10e768c0..6b491c21ec5b 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -1238,13 +1238,9 @@ static int machine_constraints_voltage(struct regulator_dev *rdev,
 		target_min = current_uV;
 		target_max = current_uV;
 
-		if (current_uV < rdev->constraints->min_uV) {
+		if ((current_uV < rdev->constraints->min_uV) ||
+		    (current_uV > rdev->constraints->max_uV)) {
 			target_min = rdev->constraints->min_uV;
-			target_max = rdev->constraints->min_uV;
-		}
-
-		if (current_uV > rdev->constraints->max_uV) {
-			target_min = rdev->constraints->max_uV;
 			target_max = rdev->constraints->max_uV;
 		}
 

---
base-commit: dcb6fa37fd7bc9c3d2b066329b0d27dedf8becaa
change-id: 20251103-regu-fix-d650274afa89

Best regards,
--  
Andreas Kemnade <akemnade@kernel.org>

Re: [PATCH RFC] regulator: core: fix constraints handling if current state out of range

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1176 bytes --]

On Mon, Nov 03, 2025 at 08:32:41PM +0100, Andreas Kemnade wrote:

> -		if (current_uV < rdev->constraints->min_uV) {
> +		if ((current_uV < rdev->constraints->min_uV) ||
> +		    (current_uV > rdev->constraints->max_uV)) {
>  			target_min = rdev->constraints->min_uV;
> -			target_max = rdev->constraints->min_uV;
> -		}
> -
> -		if (current_uV > rdev->constraints->max_uV) {
> -			target_min = rdev->constraints->max_uV;
>  			target_max = rdev->constraints->max_uV;
>  		}

There's a valid issue here if we can't represent the exact constraint
(the hope was that people wouldn't specify constraints that their
hardware wasn't capable of representing but we can't exactly stop
them...) however this change is risky in the case where the voltage is
too high since if we specify a range from minimum to maximum we'll try
to select a voltage as close as possible to the minimum.  That could
result in a large change if the range is wide, and potentially go under
the voltage the hardware needs for it's current configuration.  We were
trying to set the highest voltage in the range to minimise the risk
there.  This isn't a concern in the case where we're raising the
voltage.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH RFC] regulator: core: fix constraints handling if current state out of range

[Andreas Kemnade]

On Tue, 4 Nov 2025 13:52:11 +0000
Mark Brown <broonie@kernel.org> wrote:

> On Mon, Nov 03, 2025 at 08:32:41PM +0100, Andreas Kemnade wrote:
> 
> > -		if (current_uV < rdev->constraints->min_uV) {
> > +		if ((current_uV < rdev->constraints->min_uV) ||
> > +		    (current_uV > rdev->constraints->max_uV)) {
> >  			target_min = rdev->constraints->min_uV;
> > -			target_max = rdev->constraints->min_uV;
> > -		}
> > -
> > -		if (current_uV > rdev->constraints->max_uV) {
> > -			target_min = rdev->constraints->max_uV;
> >  			target_max = rdev->constraints->max_uV;
> >  		}  
> 
> There's a valid issue here if we can't represent the exact constraint
> (the hope was that people wouldn't specify constraints that their
> hardware wasn't capable of representing but we can't exactly stop
> them...) however this change is risky in the case where the voltage is
> too high since if we specify a range from minimum to maximum we'll try
> to select a voltage as close as possible to the minimum.  That could
> result in a large change if the range is wide, and potentially go under
> the voltage the hardware needs for it's current configuration.  We were
> trying to set the highest voltage in the range to minimise the risk
> there.  This isn't a concern in the case where we're raising the
> voltage.

So basically you do not trust the constrains too much (which is
understandable) and think that
a voltage near the boot default is the safest one.
As a hw engineer I would first try to wire things to regulators having
useable defaults (in that case a too wide range does not harm, because
voltage is ok). As a second choice I would use a regulator being off per
default which needs to be configured to a safe voltage. Then of course
ranges matter, but IHMO there is no indication which voltage in the range
is the safest one.
But again, this change is dangerous. maybe another tag like
regulator-prefer-low/high might be interesting.

Having to specify odd values with more than 1 ppm precision for
a regulator with maybe 1 percent precision is also ugly.
So the only improvement possible is to find the nearest possible
voltage via list_voltages() still matching the constraints.

Regards,
Andreas

Re: [PATCH RFC] regulator: core: fix constraints handling if current state out of range

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1505 bytes --]

On Wed, Nov 05, 2025 at 10:08:52AM +0100, Andreas Kemnade wrote:
> Mark Brown <broonie@kernel.org> wrote:

> > There's a valid issue here if we can't represent the exact constraint
> > (the hope was that people wouldn't specify constraints that their
> > hardware wasn't capable of representing but we can't exactly stop
> > them...) however this change is risky in the case where the voltage is

> So basically you do not trust the constrains too much (which is
> understandable) and think that
> a voltage near the boot default is the safest one.

Well, the problem is that the constraints need to be combined with the
current operating state for the hardware and at the point where we're
applying configuration from the constraints for the first time drivers
haven't had a chance to start so we don't really know what state the
hardware is in - we might be running at a high operating point, or have
some device that needs the upper end of the voltage range powered up and
doing something we need.  Ideally the hardware would come to us in a
state matching the constraints so we just don't need to do anything.

> Having to specify odd values with more than 1 ppm precision for
> a regulator with maybe 1 percent precision is also ugly.
> So the only improvement possible is to find the nearest possible
> voltage via list_voltages() still matching the constraints.

Yes, I think that's the only safe thing to do.  For the lower voltage
that's just a normal voltage set, but for the upper it'd be new.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]