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

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

[Uwe Kleine-König]

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(-)

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);
 }
 

base-commit: 0bb80ecc33a8fb5a682236443c1e740d5c917d1d
-- 
2.40.1

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

[Fabio Estevam]

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>