Hi Svyatoslav,
On 3/3/26 8:13 AM, Svyatoslav Ryhel wrote:
пн, 2 бер. 2026 р. о 15:07 Quentin Schulz <[email protected]> пише:
Hi Svyatoslav, Jonas,
On 2/9/26 7:07 PM, Svyatoslav Ryhel wrote:
From: Jonas Schwöbel <[email protected]>
In the case of active-low behavior the Duty Cycle needs to be set to 100%.
The PWM driver takes care of this but the LED_PWM driver does not take
this into account. Adjust LED_PWM to take into account polarity of PWM.
Signed-off-by: Jonas Schwöbel <[email protected]>
Signed-off-by: Svyatoslav Ryhel <[email protected]>
---
drivers/led/led_pwm.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/led/led_pwm.c b/drivers/led/led_pwm.c
index 15dd836509b..7edfa286468 100644
--- a/drivers/led/led_pwm.c
+++ b/drivers/led/led_pwm.c
@@ -34,7 +34,7 @@ static int led_pwm_enable(struct udevice *dev)
if (ret)
return ret;
- ret = pwm_set_enable(priv->pwm, priv->channel, true);
+ ret = pwm_set_enable(priv->pwm, priv->channel, priv->active_low);
if (ret)
return ret;
@@ -52,7 +52,7 @@ static int led_pwm_disable(struct udevice *dev)
if (ret)
return ret;
- ret = pwm_set_enable(priv->pwm, priv->channel, false);
+ ret = pwm_set_enable(priv->pwm, priv->channel, priv->active_low);
This feels wrong. Why would both enable and disable paths use the exact
same value for pwm_set_enable()?
Hello Quentin!
You are mixing up the LED with the PWM that controls it.
pwm_set_enable() triggers a PWM configuration update; therefore, the
process is the same for both enabling and disabling.
Does it? All drivers in drivers/pwm actually write to their registers in
the set_config callback, some even re-enable the PWM controller if it
was running before entering the set_config function.
The sunxi driver notably does not do anything on set_invert callback,
it's only applied on next call to set_enable.
pwm_set_enable() clearly states it's for enabling or disabling the PWM.
So why would we call pwm_set_enable(,,false) in led_pwm_enable() if
active-low property isn't set in the DT? Why would we call
pwm_set_enable(,,true) in led_pwm_disable() if the active-low property
is set?
I can also see that the disable path doesn't actually call
pwm_set_invert() unlike in the enable path. I'm assuming this means we
cannot call disable before enable and have consistent behavior?
The inversion setting is only relevant when the PWM is enabled. Since
the disable operation forces the duty cycle to 0, the inversion state
is inconsequential.
If you have inverted polarity, I'm assuming you need 100% duty cycle to
turn the LED off.. So we definitely need to have the PWM running and the
inversion setting configured, no?
The kernel seems to use active_low as a way to invert the duty cycle if
I'm understanding the code properly. In U-Boot, it seems we don't handle
that at all and always pass the "normal" duty cycle. Maybe there's
something we need to do there.
In U-Boot PWM should handle this in the set_invert operation.
I'm also assuming there are setups where PWM should be enabled but with
a duty cycle of 0?
A duty cycle of 0 is equivalent to pwm being turned off. In general
I don't think that's necessarily true? What happens to the PWM pin when
the PWM controller is disabled? You're not driving the line low (or high
for inverted polarity) so who's to say the state of the LED is off?
most devices won't work on very low duty cycles. Fan needs at least
30%, LEDs make no sense below 10%, also most Display are not usable
below 10%
I'm also unsure what's the difference between active-low and
PWM_POLARITY_INVERTED (I've seen them both mentioned or mixed in the
kernel DTSes).
These are properties of two different device classes: active-low is
used with PWM-LEDs, while PWM_POLARITY_INVERTED is strictly a PWM
property.
Sure... but don't they mean the same thing? It seems we are not reading
the flag property in the PWM cell in the PWM LED driver and only read
the active-low property to decide on the polarity. What are we supposed
to do if we only have one of both enabled? Assume it's inverted
polarity? Then what do we do if both are enabled, is it doubly-inverted,
meaning normal polarity? Or is it a simple binary OR?
I unfortunately don't have HW to test this.
Cheers,
Quentin
