[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 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 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 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 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"

[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

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

[Mark Brown]

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

On Fri, Nov 06, 2026 at 10:45:31PM +0100, Martin Blumenstingl wrote:
> 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:

> > > 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:

...

> > > 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.

The PWM driver is a bit weird here since everything is very software
defined.  If the regulator is disabled and the state that the software
will apply on enable is known then it should return a value, if we have
no idea what the hardware will do on enable then we can't safely say
what the value that will be provided is.

[-- 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"

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1551 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").

> 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

Are you sure?  We have specific handling for -ENOTRECOVERABLE which just
blindly writes in the range specified in the constraints if the current
voltage is unreadable.  If that mechanism isn't working we should fix it
rather than changing drivers.

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

>  	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 rest of the function relies on this to avoid having to deal with
wrapping when things are out of range.

[-- 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 22:57, Martin Blumenstingl wrote:
> 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)?

We don't know, that's the point I'm trying to make. If the PWM is 
disabled, likely its clock is also turned off, and the actual state of 
the output is not known. Depending on hardware, it could be high, low, 
tristated, pulled up/down, ...

What may happen now is:

- The PWM is set at 50% and disabled.

- This function returns "0%" as being the current level during probe of 
the supply.

- The core wants it to be at 0%, so it doesn't change the setting and 
enables the PWM.

- The PWM now runs at 50% and the core thinks it's at 0%.


What happens in my case:

- This function returns 0% because the PWM is still disabled

- Core wants it to be 50%, so it calls set_voltage() which sets the PWM 
but doesn't enable it yet

- Core calls this function again and (still) gets 0% as being the setting.

- This goes on until the supply (and hence the PWM) is enabled.


What the callers expect is that this function returns the value that 
would be set if the PWM were enabled.


>>> 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.

Here's the DTS fragment for my power supply. You can trigger the 
behaviour with "pwm-dutycycle-range":

             dsp7_supply_pwm: dsp7-supply-pwm {
                 compatible = "pwm-regulator";
                 pwms = <&pwm 1 50000 0>; /* Period of 50000ns means 
20kHz */
                 regulator-name = "dsp7-pwm";
                 /* VCOM = 3.9V + 0.9V * duty_cycle. */
                 regulator-min-microvolt = <3900000>;
                 regulator-max-microvolt = <4800000>;
                 pwm-dutycycle-unit = <100>;
                 pwm-dutycycle-range = <50 100>; /* Must be >= 4.35V */
                 pwm-supply = <&dsp7_supply>;
             };

>>> 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)?

This also triggers that. It will get rid of ENOTRECOVERABLE but it still 
may cause the PWM to be set to incorrect values.

> 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

I honestly don't see how...


>
> Best regards,
> Martin


-- 
Mike Looijmans

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

[Mike Looijmans]

On 08-11-2026 16:13, Mark Brown 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").
>> 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
> Are you sure?  We have specific handling for -ENOTRECOVERABLE which just
> blindly writes in the range specified in the constraints if the current
> voltage is unreadable.  If that mechanism isn't working we should fix it
> rather than changing drivers.

In core.c:

         if (current_uV == -ENOTRECOVERABLE) {
             /* This regulator can't be read and must be initialized */
             rdev_info(rdev, "Setting %d-%duV\n",
                   rdev->constraints->min_uV,
                   rdev->constraints->max_uV);
             _regulator_do_set_voltage(rdev,
                           rdev->constraints->min_uV,
                           rdev->constraints->max_uV);
             current_uV = regulator_get_voltage_rdev(rdev);
         }

         if (current_uV < 0) {
             if (current_uV != -EPROBE_DEFER)
                 rdev_err(rdev,
                      "failed to get the current voltage: %pe\n",
                      ERR_PTR(current_uV));
             return current_uV;
         }

After _regulator_do_set_voltage() it calls regulator_get_voltage_rdev() 
again, which keeps returning -ENOTRECOVERABLE, and thus the error path 
kicks in.


>
>> Besides that, regulator_get_voltage() is not supposed to return any
>> state information, as is mentioned in the API documentation.
>>   	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 rest of the function relies on this to avoid having to deal with
> wrapping when things are out of range.

It's core.c that will handle that. It expects regulator_get_voltage() to 
return something out of range, and will correct that situation.



-- 
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

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

[Mike Looijmans]

On 06-11-2026 22:45, Martin Blumenstingl wrote:
> 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?

https://docs.kernel.org/driver-api/regulator.html#c.regulator_get_voltage

And the text above clearly states "can't be read at bootup and hasn't 
been set yet", which is not the same as "whenever it's set out of range".


> 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


-- 
Mike Looijmans