| Message ID | 20230922142304.1685985-1-u.kleine-koenig@pengutronix.de |
|---|---|
| State | Superseded |
| Headers | show |
| Series | leds: pwm: Don't disable the PWM when the LED should be off | expand |
Hi Uwe, On 22/09/2023 11:23, Uwe Kleine-König wrote: > led_dat->pwmstate.duty_cycle = duty; > - led_dat->pwmstate.enabled = duty > 0; > + led_dat->pwmstate.enabled = 1; Thanks for the fix. Nit: I would suggest: led_dat->pwmstate.enabled = true; Reviewed-by: Fabio Estevam <festevam@denx.de>
diff --git a/drivers/leds/leds-pwm.c b/drivers/leds/leds-pwm.c index 419b710984ab..5e26aa34de01 100644 --- a/drivers/leds/leds-pwm.c +++ b/drivers/leds/leds-pwm.c @@ -53,7 +53,7 @@ static int led_pwm_set(struct led_classdev *led_cdev, duty = led_dat->pwmstate.period - duty; led_dat->pwmstate.duty_cycle = duty; - led_dat->pwmstate.enabled = duty > 0; + led_dat->pwmstate.enabled = 1; return pwm_apply_state(led_dat->pwm, &led_dat->pwmstate); }
Disabling a PWM (i.e. calling pwm_apply_state with .enabled = false) gives no guarantees about what the PWM output does. It might freeze where it currently is, or go in a High-Z state or drive the active or inactive state, it might even continue to toggle. To ensure that the LED gets really disabled when brightness is set to zero, don't disable the PWM. This fixes disabling a leds-pwm LED on i.MX28 (and others). The PWM on this SoC is one of those that freezes its output on disable, so if you disable an LED that is full on, it stays on. If you disable a LED with half brightness it goes off in 50% of the cases and full on in the other 50%. Reported-by: Rogan Dawes <rogan@dawes.za.net> Reported-by: Fabio Estevam <festevam@denx.de> Fixes: 41c42ff5dbe2 ("leds: simple driver for pwm driven LEDs") Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> --- Hello, this is a follow up to https://lore.kernel.org/linux-pwm/20230922121752.344965-1-festevam@gmail.com I knew the problem and already tried to address it a few years ago, see e.g. https://lore.kernel.org/linux-pwm/20180806155129.cjcc7okmwtaujf43@pengutronix.de . Back then the discussion didn't result in a fix, though. So here is another effort to fix it. Since 2018 at least the documentation situation is a bit clearer and we have: As a consumer, don't rely on the output's state for a disabled PWM. If it's easily possible, drivers are supposed to emit the inactive state, but some drivers cannot. If you rely on getting the inactive state, use .duty_cycle=0, .enabled=true. in the docs since commit 80a22fde803a ("pwm: Document that the pinstate of a disabled PWM isn't reliable"). Best regards Uwe drivers/leds/leds-pwm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) base-commit: 0bb80ecc33a8fb5a682236443c1e740d5c917d1d