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

[Mike Looijmans <mike.looijmans@topic.nl>]

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