The logic in regulator_set_voltage() which checks for a non-convergence
condition on a stepped regulator is flawed.

regulator_set_voltage() checks if the error in target voltage has increased
or decreased, and returns -EWOULDBLOCK if the error has not decreased
enough. The correct non-convergence condition is:

new_delta - delta > -rdev->constraints->max_uV_step

or equivalently:

delta - new_delta < rdev->constraints->max_uV_step

But the currently used condition is:

new_delta - delta > rdev->constraints->max_uV_step

Which may cause an infinite loop if the voltage error doesn't converge.

Fix this by correcting the convergence condition.

Suggested-by: Jon Hunter <jonathanh@nvidia.com>
Fixes: d511206dc7443 ("regulator: core: repeat voltage setting request for stepped regulators")
Signed-off-by: Romain Gantois <romain.gantois@bootlin.com>
---
 drivers/regulator/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 8ed9b96518cf5186c0db147a6895a92bc59fae4e..554d83c4af0c1c98edb43b60dbfacb8844d5e1eb 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -3884,7 +3884,7 @@ static int regulator_set_voltage_unlocked(struct regulator *regulator,
 			new_delta = ret;
 
 			/* check that voltage is converging quickly enough */
-			if (new_delta - delta > rdev->constraints->max_uV_step) {
+			if (delta - new_delta < rdev->constraints->max_uV_step) {
 				ret = -EWOULDBLOCK;
 				goto out;
 			}

---
base-commit: 0bd042ae771d61ef7ccd5882f7aeca59a25f71d9
change-id: 20250729-b4-regulator-stepping-fix-6e23fe03e914

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

On Tue, 29 Jul 2025 11:50:57 +0200, Romain Gantois wrote:
> The logic in regulator_set_voltage() which checks for a non-convergence
> condition on a stepped regulator is flawed.
> 
> regulator_set_voltage() checks if the error in target voltage has increased
> or decreased, and returns -EWOULDBLOCK if the error has not decreased
> enough. The correct non-convergence condition is:
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git for-next

Thanks!

[1/1] regulator: core: correct convergence check in regulator_set_voltage()
      commit: 10dfd36f078423c51602a9a21ed85e8e6c947a00

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

On 29/07/2025 10:50, Romain Gantois wrote:
> The logic in regulator_set_voltage() which checks for a non-convergence
> condition on a stepped regulator is flawed.
> 
> regulator_set_voltage() checks if the error in target voltage has increased
> or decreased, and returns -EWOULDBLOCK if the error has not decreased
> enough. The correct non-convergence condition is:
> 
> new_delta - delta > -rdev->constraints->max_uV_step
> 
> or equivalently:
> 
> delta - new_delta < rdev->constraints->max_uV_step
> 
> But the currently used condition is:
> 
> new_delta - delta > rdev->constraints->max_uV_step
> 
> Which may cause an infinite loop if the voltage error doesn't converge.
> 
> Fix this by correcting the convergence condition.
> 
> Suggested-by: Jon Hunter <jonathanh@nvidia.com>
> Fixes: d511206dc7443 ("regulator: core: repeat voltage setting request for stepped regulators")
> Signed-off-by: Romain Gantois <romain.gantois@bootlin.com>
> ---
>   drivers/regulator/core.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
> index 8ed9b96518cf5186c0db147a6895a92bc59fae4e..554d83c4af0c1c98edb43b60dbfacb8844d5e1eb 100644
> --- a/drivers/regulator/core.c
> +++ b/drivers/regulator/core.c
> @@ -3884,7 +3884,7 @@ static int regulator_set_voltage_unlocked(struct regulator *regulator,
>   			new_delta = ret;
>   
>   			/* check that voltage is converging quickly enough */
> -			if (new_delta - delta > rdev->constraints->max_uV_step) {
> +			if (delta - new_delta < rdev->constraints->max_uV_step) {
>   				ret = -EWOULDBLOCK;
>   				goto out;
>   			}


Thanks for the quick fix!

Tested-by: Jon Hunter <jonathanh@nvidia.com>
Reviewed-by: Jon Hunter <jonathanh@nvidia.com>

Jon

-- 
nvpublic