Hey Quentin, While working with a Active-Low RGB LED i noticed that it doesn't work correctly. I traced this problem back to the LED PWM driver.
The driver currently disables the PWM hardware when the LED should be turned off. This works for the common active-high case, where driving the output low keeps the LED off. However this does not work correctly for active-low LEDs. For an active-low LED the polarity is inverted: - LED on -> output low - LED off -> output high With PWM this means that a fully-off LED corresponds to a 0% duty cycle with inverted polarity, i.e. the output is constantly driven high. If the PWM controller is disabled instead, the output typically returns to the controller's idle state (often low or high-Z depending on the hardware).In that case the LED may end up unintentionally turned on or left in an undefined state. Because of this, disabling the PWM device to represent the LED off state is not generally correct. Instead the PWM should remain enabled and the LED state should be expressed through the duty cycle while respecting the configured polarity. This ensures correct behaviour for both active-high and active-low LEDs and avoids relying on controller-specific idle levels. (I even tried to force the idle level with pull-up-down settings but without success) This is further backed by the Linux kernel LED PWM driver: https://github.com/torvalds/linux/blob/ 5ee8dbf54602dc340d6235b1d6aa17c0f283f48c/drivers/leds/leds-pwm.c#L59-L64 Therefore i suggest that we keep the PWM enabled when turning off the LED. This behavior was "accidentally" matched when setting it to priv->active_low which, in my special active-low case, was true and kept PWM enabled when turning the LED off. I didn't have any active-high PWM LED to test/verify my change and therefore it slipped through. I am sorry for that. Thank you for catching that little mistake :) I suggest to match the Linux kernel behavior and keep the PWM running at all times. Let us know if you agree and we will update the patch. Greetings Jonas Am Mittwoch, 4. März 2026 um 19:12:39 MEZ hat Svyatoslav Ryhel <[email protected]> Folgendes geschrieben: вт, 3 бер. 2026 р. о 13:07 Quentin Schulz <[email protected]> пише: > > 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? > ok, fair, it seems that this commit got here accidentally, my bad. It will be dropped. > >> 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? > pwm_set_invert sets an invert flag and PWM driver should handle this. > >> 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 am not the right person to ask this question. Ask Linux kernel PWM/PWM-LED/schema maintainers. They will definitely provide a complete explanation. > I unfortunately don't have HW to test this. > > Cheers, > Quentin
