[PATCH] regulator: core: do not ignore repeated requests on stepped regulators

[PATCH] regulator: core: do not ignore repeated requests on stepped regulators

[Romain Gantois]

Currently, the regulator_set_voltage() function will assume a noop if a
consumer requests the same voltage range twice in a row.

This can lead to unexpected behavior if the target regulator has a maximum
voltage step constraint. With such constraints, the regulator core may
clamp the requested voltage to a lesser value, to ensure that the voltage
delta stays under the specified limit.

This means that the resulting regulator voltage depends on the current
voltage, as well as the requested range, which invalidates the assumption
that a repeated request for a specific voltage range will amount to a noop.

Considering the case of a regulator with a maximum voltage step constraint
of 1V:

initial voltage: 2.5V

consumer requests 4V
expected result: 3.5V
resulting voltage: 3.5V

consumer requests 4V again
expected result: 4V
actual result: 3.5V

Do not ignore repeated calls to regulator_set_voltage() if the regulator
has a voltage step constraint.

Signed-off-by: Romain Gantois <romain.gantois@bootlin.com>
---
 drivers/regulator/core.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 7a248dc8d2e2ffd21e7daf729de9b33a5efc1937..4196b1d79fd53bfdd2d8e780272f5037d5ddab0e 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -3810,8 +3810,13 @@ static int regulator_set_voltage_unlocked(struct regulator *regulator,
 	/* If we're setting the same range as last time the change
 	 * should be a noop (some cpufreq implementations use the same
 	 * voltage for multiple frequencies, for example).
+	 *
+	 * This isn't true for regulator devices with a "max_uV_step"
+	 * constraint, as they can progressively step their voltage with each
+	 * subsequent request.
 	 */
-	if (voltage->min_uV == min_uV && voltage->max_uV == max_uV)
+	if (voltage->min_uV == min_uV && voltage->max_uV == max_uV &&
+	    !rdev->constraints->max_uV_step)
 		goto out;
 
 	/* If we're trying to set a range that overlaps the current voltage,

---
base-commit: a02c7665c216471413ed5442637a34364221e91c
change-id: 20250521-regulator-stepping-1bfdc34e5039

Best regards,
-- 
Romain Gantois <romain.gantois@bootlin.com>

Re: [PATCH] regulator: core: do not ignore repeated requests on stepped regulators

[Mark Brown]

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

On Wed, May 21, 2025 at 10:47:24AM +0200, Romain Gantois wrote:
> Currently, the regulator_set_voltage() function will assume a noop if a
> consumer requests the same voltage range twice in a row.
> 
> This can lead to unexpected behavior if the target regulator has a maximum
> voltage step constraint. With such constraints, the regulator core may
> clamp the requested voltage to a lesser value, to ensure that the voltage
> delta stays under the specified limit.

No, if there's an issue here we should handle it the first time we set
the voltage by doing multiple steps in one set_voltage() call.  Having
individual client drivers all having to repeatedly call set_voltage() is
obviously not a good API.

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

Re: [PATCH] regulator: core: do not ignore repeated requests on stepped regulators

[Romain Gantois]

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

On Wednesday, 21 May 2025 12:07:05 CEST Mark Brown wrote:
> On Wed, May 21, 2025 at 10:47:24AM +0200, Romain Gantois wrote:
> > Currently, the regulator_set_voltage() function will assume a noop if a
> > consumer requests the same voltage range twice in a row.
> > 
> > This can lead to unexpected behavior if the target regulator has a maximum
> > voltage step constraint. With such constraints, the regulator core may
> > clamp the requested voltage to a lesser value, to ensure that the voltage
> > delta stays under the specified limit.
> 
> No, if there's an issue here we should handle it the first time we set
> the voltage by doing multiple steps in one set_voltage() call.  Having
> individual client drivers all having to repeatedly call set_voltage() is
> obviously not a good API.

Understood, would it make sense to handle this directly in 
regulator_set_voltage_unlocked()?  For example by checking for a max_uV_step 
condition and repeating calls to regulator_do_balance_voltage() until the 
resulting voltage stabilizes?

Thanks,

-- 
Romain Gantois, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] regulator: core: do not ignore repeated requests on stepped regulators

[Mark Brown]

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

On Wed, May 21, 2025 at 01:50:22PM +0200, Romain Gantois wrote:
> On Wednesday, 21 May 2025 12:07:05 CEST Mark Brown wrote:

> > No, if there's an issue here we should handle it the first time we set
> > the voltage by doing multiple steps in one set_voltage() call.  Having
> > individual client drivers all having to repeatedly call set_voltage() is
> > obviously not a good API.

> Understood, would it make sense to handle this directly in 
> regulator_set_voltage_unlocked()?  For example by checking for a max_uV_step 
> condition and repeating calls to regulator_do_balance_voltage() until the 
> resulting voltage stabilizes?

Yes, handling it there seems sensible.

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