вт, 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
