diff mbox series

[v2] leds: pwm: Don't disable the PWM when the LED should be off

Message ID 20230922192834.1695727-1-u.kleine-koenig@pengutronix.de
State New
Headers show
Series [v2] leds: pwm: Don't disable the PWM when the LED should be off | expand

Commit Message

Uwe Kleine-König Sept. 22, 2023, 7:28 p.m. UTC 
Disabling a PWM (i.e. calling pwm_apply_state with .enabled = false)
gives no guarantees 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, don't disable the PWM even
when .duty_cycle is zero.

This fixes disabling a leds-pwm LED on i.MX28. 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>
Reviewed-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,

changes since (implicit) v1 sent with Message-Id:
20230922142304.1685985-1-u.kleine-koenig@pengutronix.de:

 - Use true instead of 1 to assign the boot .enabled. Thanks Fabio for
   that hint.
 - Add Reviewed-by: tag for Fabio

In reply to the first iteration, Rogan wrote that
"led_dat->pwmstate.enabled = true; also addresses the problem".

BTW, this patch is similar to deaeeda2051f ("backlight: pwm_bl: Don't
rely on a disabled PWM emiting inactive state") which fixed the same
issue for PWM backlights.

Best regards
Uwe

 drivers/leds/leds-pwm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)


base-commit: 0bb80ecc33a8fb5a682236443c1e740d5c917d1d

Comments

Lee Jones Sept. 28, 2023, 12:44 p.m. UTC | #1 
On Fri, 22 Sep 2023 21:28:34 +0200, Uwe Kleine-König wrote:
> Disabling a PWM (i.e. calling pwm_apply_state with .enabled = false)
> gives no guarantees 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, don't disable the PWM even
> when .duty_cycle is zero.
> 
> [...]

Applied, thanks!

[1/1] leds: pwm: Don't disable the PWM when the LED should be off
      commit: 1025d4c1837eb457f9d599611096bf3a4d954333

--
Lee Jones [李琼斯]
diff mbox series

Patch

diff --git a/drivers/leds/leds-pwm.c b/drivers/leds/leds-pwm.c
index 419b710984ab..2b3bf1353b70 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 = true;
 	return pwm_apply_state(led_dat->pwm, &led_dat->pwmstate);
 }