[PATCH 1/2] regulator: pwm-regulator: Revert "Calculate the output voltage for disabled PWMs"

[PATCH 1/2] regulator: pwm-regulator: Revert "Calculate the output voltage for disabled PWMs"

[Mike Looijmans]

This reverts commit 6a7d11efd691 ("regulator: pwm-regulator: Calculate the
output voltage for disabled PWMs")

A disabled PWM is not the same as "set to 0%". If the PWM is disabled,
this could also mean that its output pin is tristated for example.

If the PWM range is limited to something else than 0..100% this causes the
probe function to fail with messages like:
 dsp7-pwm: failed to get the current voltage: -ENOTRECOVERABLE
 pwm-regulator dsp7-supply-pwm: error -ENOTRECOVERABLE: Failed to register regulator pwm-regulator
 pwm-regulator dsp7-supply-pwm: probe with driver pwm-regulator failed with error -131

The reason is that regulator_register() calls get_voltage(). The PWM is
disabled at this point, and then the validity check triggers.

Anything that calls get_voltage() while the PWM is disabled would get an
-ENOTRECOVERABLE return value for a pwm-regulator that doesn't allow 0%.

Signed-off-by: Mike Looijmans <mike.looijmans@topic.nl>
---

 drivers/regulator/pwm-regulator.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/drivers/regulator/pwm-regulator.c b/drivers/regulator/pwm-regulator.c
index 7434b6b22d325..f50530c77d92f 100644
--- a/drivers/regulator/pwm-regulator.c
+++ b/drivers/regulator/pwm-regulator.c
@@ -157,13 +157,6 @@ static int pwm_regulator_get_voltage(struct regulator_dev *rdev)
 
 	pwm_get_state(drvdata->pwm, &pstate);
 
-	if (!pstate.enabled) {
-		if (pstate.polarity == PWM_POLARITY_INVERSED)
-			pstate.duty_cycle = pstate.period;
-		else
-			pstate.duty_cycle = 0;
-	}
-
 	voltage = pwm_get_relative_duty_cycle(&pstate, duty_unit);
 	if (voltage < min(max_uV_duty, min_uV_duty) ||
 	    voltage > max(max_uV_duty, min_uV_duty))
---
base-commit: f66469d9905b39d2c260a15d7d6db450f4db67c1
branch: linux-master-pwm-regulator

-- 
2.43.0


Met vriendelijke groet / kind regards,

Mike Looijmans
System Expert


TOPIC Embedded Products B.V.
Materiaalweg 4, 5681 RJ Best
The Netherlands

T: +31 (0) 499 33 69 69
E: mike.looijmans@topic.nl
W: www.topic.nl

Please consider the environment before printing this e-mail

[PATCH 2/2] regulator: pwm-regulator: Revert "Add validity checks in continuous .get_voltage"

[Mike Looijmans]

This reverts commit c92688cac239 ("regulator: pwm-regulator: Add validity
checks in continuous .get_voltage").

The core, and in particular the machine_constraints_voltage() method,
expects the regulator to report what the current setting is, even if that
is outside the allowed range. The core will then bring the voltage into
that range. It cannot do that if the regulator refuses to return its
setting, and causes probe() to fail for a pwm-regulator when it has
restrictions to the PWM range with messages like:
  dsp7-pwm: failed to get the current voltage: -ENOTRECOVERABLE
  pwm-regulator dsp7-supply-pwm: error -ENOTRECOVERABLE: Failed to register
    regulator pwm-regulator
  pwm-regulator dsp7-supply-pwm: probe with driver pwm-regulator failed
    with error -131

Besides that, regulator_get_voltage() is not supposed to return any
state information, as is mentioned in the API documentation.

Signed-off-by: Mike Looijmans <mike.looijmans@topic.nl>
---

 drivers/regulator/pwm-regulator.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/regulator/pwm-regulator.c b/drivers/regulator/pwm-regulator.c
index f50530c77d92f..00fcf9a172b47 100644
--- a/drivers/regulator/pwm-regulator.c
+++ b/drivers/regulator/pwm-regulator.c
@@ -158,9 +158,6 @@ static int pwm_regulator_get_voltage(struct regulator_dev *rdev)
 	pwm_get_state(drvdata->pwm, &pstate);
 
 	voltage = pwm_get_relative_duty_cycle(&pstate, duty_unit);
-	if (voltage < min(max_uV_duty, min_uV_duty) ||
-	    voltage > max(max_uV_duty, min_uV_duty))
-		return -ENOTRECOVERABLE;
 
 	/*
 	 * The dutycycle for min_uV might be greater than the one for max_uV.
---
base-commit: f66469d9905b39d2c260a15d7d6db450f4db67c1
branch: linux-master-pwm-regulator

-- 
2.43.0


Met vriendelijke groet / kind regards,

Mike Looijmans
System Expert


TOPIC Embedded Products B.V.
Materiaalweg 4, 5681 RJ Best
The Netherlands

T: +31 (0) 499 33 69 69
E: mike.looijmans@topic.nl
W: www.topic.nl

Please consider the environment before printing this e-mail

Re: [PATCH 2/2] regulator: pwm-regulator: Revert "Add validity checks in continuous .get_voltage"

[Mark Brown]

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

On Fri, Nov 06, 2026 at 01:59:56PM +0100, Mike Looijmans wrote:
> This reverts commit c92688cac239 ("regulator: pwm-regulator: Add validity
> checks in continuous .get_voltage").

And again...  please don't try to sneak changes past people :/

> The core, and in particular the machine_constraints_voltage() method,
> expects the regulator to report what the current setting is, even if that
> is outside the allowed range. The core will then bring the voltage into
> that range. It cannot do that if the regulator refuses to return its
> setting, and causes probe() to fail for a pwm-regulator when it has
> restrictions to the PWM range with messages like:
>   dsp7-pwm: failed to get the current voltage: -ENOTRECOVERABLE
>   pwm-regulator dsp7-supply-pwm: error -ENOTRECOVERABLE: Failed to register
>     regulator pwm-regulator
>   pwm-regulator dsp7-supply-pwm: probe with driver pwm-regulator failed
>     with error -131
> 
> Besides that, regulator_get_voltage() is not supposed to return any
> state information, as is mentioned in the API documentation.
> 
> Signed-off-by: Mike Looijmans <mike.looijmans@topic.nl>
> ---
> 
>  drivers/regulator/pwm-regulator.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/drivers/regulator/pwm-regulator.c b/drivers/regulator/pwm-regulator.c
> index f50530c77d92f..00fcf9a172b47 100644
> --- a/drivers/regulator/pwm-regulator.c
> +++ b/drivers/regulator/pwm-regulator.c
> @@ -158,9 +158,6 @@ static int pwm_regulator_get_voltage(struct regulator_dev *rdev)
>  	pwm_get_state(drvdata->pwm, &pstate);
>  
>  	voltage = pwm_get_relative_duty_cycle(&pstate, duty_unit);
> -	if (voltage < min(max_uV_duty, min_uV_duty) ||
> -	    voltage > max(max_uV_duty, min_uV_duty))
> -		return -ENOTRECOVERABLE;
>  
>  	/*
>  	 * The dutycycle for min_uV might be greater than the one for max_uV.
> ---
> base-commit: f66469d9905b39d2c260a15d7d6db450f4db67c1
> branch: linux-master-pwm-regulator
> 
> -- 
> 2.43.0
> 
> 
> Met vriendelijke groet / kind regards,
> 
> Mike Looijmans
> System Expert
> 
> 
> TOPIC Embedded Products B.V.
> Materiaalweg 4, 5681 RJ Best
> The Netherlands
> 
> T: +31 (0) 499 33 69 69
> E: mike.looijmans@topic.nl
> W: www.topic.nl
> 
> Please consider the environment before printing this e-mail

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

Re: [PATCH 2/2] regulator: pwm-regulator: Revert "Add validity checks in continuous .get_voltage"

[Martin Blumenstingl]

On Fri, Nov 6, 2026 at 2:46 PM Mark Brown <broonie@kernel.org> wrote:
>
> On Fri, Nov 06, 2026 at 01:59:56PM +0100, Mike Looijmans wrote:
> > This reverts commit c92688cac239 ("regulator: pwm-regulator: Add validity
> > checks in continuous .get_voltage").
>
> And again...  please don't try to sneak changes past people :/
Thank you for looping me in, Mark!

> > The core, and in particular the machine_constraints_voltage() method,
> > expects the regulator to report what the current setting is, even if that
> > is outside the allowed range. The core will then bring the voltage into
> > that range. It cannot do that if the regulator refuses to return its
> > setting, and causes probe() to fail for a pwm-regulator when it has
> > restrictions to the PWM range with messages like:
> >   dsp7-pwm: failed to get the current voltage: -ENOTRECOVERABLE
> >   pwm-regulator dsp7-supply-pwm: error -ENOTRECOVERABLE: Failed to register
> >     regulator pwm-regulator
> >   pwm-regulator dsp7-supply-pwm: probe with driver pwm-regulator failed
> >     with error -131
> >
> > Besides that, regulator_get_voltage() is not supposed to return any
> > state information, as is mentioned in the API documentation.
according to include/linux/regulator/driver.h:
 * @get_voltage: Return the currently configured voltage for the regulator;
 *                   return -ENOTRECOVERABLE if regulator can't be read at
 *                   bootup and hasn't been set yet.

Can you please specify which API documentation you are referring to?

I believe back when this change was authored it didn't make a
difference on my board (but made sense based on review feedback).
So I'm not against reverting as long as we have a proper explanation
why the original patch is incorrect.


Best regards,
Martin

Re: [PATCH 1/2] regulator: pwm-regulator: Revert "Calculate the output voltage for disabled PWMs"

[Mark Brown]

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

On Fri, Nov 06, 2026 at 01:59:55PM +0100, Mike Looijmans wrote:
> This reverts commit 6a7d11efd691 ("regulator: pwm-regulator: Calculate the
> output voltage for disabled PWMs")

Adding Martin who wrote that patch - please CC people if you're directly
updating their patches.

> 
> A disabled PWM is not the same as "set to 0%". If the PWM is disabled,
> this could also mean that its output pin is tristated for example.
> 
> If the PWM range is limited to something else than 0..100% this causes the
> probe function to fail with messages like:
>  dsp7-pwm: failed to get the current voltage: -ENOTRECOVERABLE
>  pwm-regulator dsp7-supply-pwm: error -ENOTRECOVERABLE: Failed to register regulator pwm-regulator
>  pwm-regulator dsp7-supply-pwm: probe with driver pwm-regulator failed with error -131
> 
> The reason is that regulator_register() calls get_voltage(). The PWM is
> disabled at this point, and then the validity check triggers.
> 
> Anything that calls get_voltage() while the PWM is disabled would get an
> -ENOTRECOVERABLE return value for a pwm-regulator that doesn't allow 0%.
> 
> Signed-off-by: Mike Looijmans <mike.looijmans@topic.nl>
> ---
> 
>  drivers/regulator/pwm-regulator.c | 7 -------
>  1 file changed, 7 deletions(-)
> 
> diff --git a/drivers/regulator/pwm-regulator.c b/drivers/regulator/pwm-regulator.c
> index 7434b6b22d325..f50530c77d92f 100644
> --- a/drivers/regulator/pwm-regulator.c
> +++ b/drivers/regulator/pwm-regulator.c
> @@ -157,13 +157,6 @@ static int pwm_regulator_get_voltage(struct regulator_dev *rdev)
>  
>  	pwm_get_state(drvdata->pwm, &pstate);
>  
> -	if (!pstate.enabled) {
> -		if (pstate.polarity == PWM_POLARITY_INVERSED)
> -			pstate.duty_cycle = pstate.period;
> -		else
> -			pstate.duty_cycle = 0;
> -	}
> -
>  	voltage = pwm_get_relative_duty_cycle(&pstate, duty_unit);
>  	if (voltage < min(max_uV_duty, min_uV_duty) ||
>  	    voltage > max(max_uV_duty, min_uV_duty))
> ---
> base-commit: f66469d9905b39d2c260a15d7d6db450f4db67c1
> branch: linux-master-pwm-regulator
> 
> -- 
> 2.43.0
> 
> 
> Met vriendelijke groet / kind regards,
> 
> Mike Looijmans
> System Expert
> 
> 
> TOPIC Embedded Products B.V.
> Materiaalweg 4, 5681 RJ Best
> The Netherlands
> 
> T: +31 (0) 499 33 69 69
> E: mike.looijmans@topic.nl
> W: www.topic.nl
> 
> Please consider the environment before printing this e-mail

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

Re: [PATCH 1/2] regulator: pwm-regulator: Revert "Calculate the output voltage for disabled PWMs"

[Mike Looijmans]

On 06-11-2026 14:36, Mark Brown wrote:
> On Fri, Nov 06, 2026 at 01:59:55PM +0100, Mike Looijmans wrote:
>> This reverts commit 6a7d11efd691 ("regulator: pwm-regulator: Calculate the
>> output voltage for disabled PWMs")
> Adding Martin who wrote that patch - please CC people if you're directly
> updating their patches.

Sorry, that wasn't intentional. I was just used to get_maintainer.pl and 
patman "doing the right thing" that I didn't consider checking that. I 
promise to do better next time.


>
>> A disabled PWM is not the same as "set to 0%". If the PWM is disabled,
>> this could also mean that its output pin is tristated for example.
>>
>> If the PWM range is limited to something else than 0..100% this causes the
>> probe function to fail with messages like:
>>   dsp7-pwm: failed to get the current voltage: -ENOTRECOVERABLE
>>   pwm-regulator dsp7-supply-pwm: error -ENOTRECOVERABLE: Failed to register regulator pwm-regulator
>>   pwm-regulator dsp7-supply-pwm: probe with driver pwm-regulator failed with error -131
>>
>> The reason is that regulator_register() calls get_voltage(). The PWM is
>> disabled at this point, and then the validity check triggers.
>>
>> Anything that calls get_voltage() while the PWM is disabled would get an
>> -ENOTRECOVERABLE return value for a pwm-regulator that doesn't allow 0%.
>>
>> Signed-off-by: Mike Looijmans <mike.looijmans@topic.nl>
>> ---
>>
>>   drivers/regulator/pwm-regulator.c | 7 -------
>>   1 file changed, 7 deletions(-)
>>
>> diff --git a/drivers/regulator/pwm-regulator.c b/drivers/regulator/pwm-regulator.c
>> index 7434b6b22d325..f50530c77d92f 100644
>> --- a/drivers/regulator/pwm-regulator.c
>> +++ b/drivers/regulator/pwm-regulator.c
>> @@ -157,13 +157,6 @@ static int pwm_regulator_get_voltage(struct regulator_dev *rdev)
>>   
>>   	pwm_get_state(drvdata->pwm, &pstate);
>>   
>> -	if (!pstate.enabled) {
>> -		if (pstate.polarity == PWM_POLARITY_INVERSED)
>> -			pstate.duty_cycle = pstate.period;
>> -		else
>> -			pstate.duty_cycle = 0;
>> -	}
>> -
>>   	voltage = pwm_get_relative_duty_cycle(&pstate, duty_unit);
>>   	if (voltage < min(max_uV_duty, min_uV_duty) ||
>>   	    voltage > max(max_uV_duty, min_uV_duty))
>> ---
>> base-commit: f66469d9905b39d2c260a15d7d6db450f4db67c1
>> branch: linux-master-pwm-regulator
>>
>> -- 
>> 2.43.0
>>
>>

-- 
Mike Looijmans

Re: [PATCH 1/2] regulator: pwm-regulator: Revert "Calculate the output voltage for disabled PWMs"

[Martin Blumenstingl]

On Fri, Nov 6, 2026 at 2:42 PM Mark Brown <broonie@kernel.org> wrote:
>
> On Fri, Nov 06, 2026 at 01:59:55PM +0100, Mike Looijmans wrote:
> > This reverts commit 6a7d11efd691 ("regulator: pwm-regulator: Calculate the
> > output voltage for disabled PWMs")
>
> Adding Martin who wrote that patch - please CC people if you're directly
> updating their patches.
IIRC get_maintainers.pl includes the author of the patch blamed by the
Fixes: tag (if there's one).

> >
> > A disabled PWM is not the same as "set to 0%". If the PWM is disabled,
> > this could also mean that its output pin is tristated for example.
How do we know if it's 0% or tri-stated (and what's the behavior if
it's tri-stated)?

> > If the PWM range is limited to something else than 0..100% this causes the
> > probe function to fail with messages like:
> >  dsp7-pwm: failed to get the current voltage: -ENOTRECOVERABLE
> >  pwm-regulator dsp7-supply-pwm: error -ENOTRECOVERABLE: Failed to register regulator pwm-regulator
> >  pwm-regulator dsp7-supply-pwm: probe with driver pwm-regulator failed with error -131
What board are you testing with? I'd like to see the .dts to get a
better understanding of where your board differs from mine.
I think Uwe (PWM maintainer) also previously mentioned that there's no
defined behavior for disabling a PWM and then reading it's state
again. I'll see if I can find his explanation and whether I can come
up with a patch that may help here.

> > The reason is that regulator_register() calls get_voltage(). The PWM is
> > disabled at this point, and then the validity check triggers.
> >
> > Anything that calls get_voltage() while the PWM is disabled would get an
> > -ENOTRECOVERABLE return value for a pwm-regulator that doesn't allow 0%.
Why is the revert from your patch 2/2 not enough (it gets rid of
-ENOTRECOVERABLE)?
The problem with this specific revert (your patch 1/2) is that (from
memory) it will break these boards:
- arch/arm/boot/dts/amlogic/meson8b-ec100.dts
- arch/arm/boot/dts/amlogic/meson8b-mxq.dts
- arch/arm/boot/dts/amlogic/meson8b-odroidc1.dts
- arch/arm/boot/dts/amlogic/meson8m2-wetek-core.dts


Best regards,
Martin