diff mbox series

leds: pwm: Disable PWM when going to suspend

Message ID 20240417153846.271751-2-u.kleine-koenig@pengutronix.de
State New
Headers show
Series leds: pwm: Disable PWM when going to suspend | expand

Commit Message

Uwe Kleine-König April 17, 2024, 3:38 p.m. UTC
On stm32mp1xx based machines (and others) a PWM consumer has to disable
the PWM because an enabled PWM refuses to suspend. So check the
LED_SUSPENDED flag and depending on that set the .enabled property.

Link: https://bugzilla.kernel.org/show_bug.cgi?id=218559
Fixes: 76fe464c8e64 ("leds: pwm: Don't disable the PWM when the LED should be off")
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
Hello,

On Wed, Apr 17, 2024 at 03:49:43PM +0100, Lee Jones wrote:
> On Tue, 16 Apr 2024, Uwe Kleine-König wrote:
> > If you don't consider that suitable, I can create a patch that is easier
> > to pick up.
> 
> Yes, please submit it properly.

Here it comes.

Best regards
Uwe

 drivers/leds/leds-pwm.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)


base-commit: 4eab358930711bbeb85bf5ee267d0d42d3394c2c

Comments

Uwe Kleine-König April 26, 2024, 6:17 a.m. UTC | #1
Hello Lee,

On Wed, Apr 17, 2024 at 05:38:47PM +0200, Uwe Kleine-König wrote:
> On stm32mp1xx based machines (and others) a PWM consumer has to disable
> the PWM because an enabled PWM refuses to suspend. So check the
> LED_SUSPENDED flag and depending on that set the .enabled property.
> 
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=218559
> Fixes: 76fe464c8e64 ("leds: pwm: Don't disable the PWM when the LED should be off")
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
> Hello,
> 
> On Wed, Apr 17, 2024 at 03:49:43PM +0100, Lee Jones wrote:
> > On Tue, 16 Apr 2024, Uwe Kleine-König wrote:
> > > If you don't consider that suitable, I can create a patch that is easier
> > > to pick up.
> > 
> > Yes, please submit it properly.

Gentle ping. Even given the regression was introduced in v6.7-rc1
already, I think this should go into v6.9. If you don't agree that's
fine, but then getting it onto next to queue it for v6.10-rc1 at least
would be great.

Thanks
Uwe
Linux regression tracking (Thorsten Leemhuis) April 26, 2024, 6:30 a.m. UTC | #2
On 26.04.24 08:17, Uwe Kleine-König wrote:
> On Wed, Apr 17, 2024 at 05:38:47PM +0200, Uwe Kleine-König wrote:
>> On stm32mp1xx based machines (and others) a PWM consumer has to disable
>> the PWM because an enabled PWM refuses to suspend. So check the
>> LED_SUSPENDED flag and depending on that set the .enabled property.
>>
>> Link: https://bugzilla.kernel.org/show_bug.cgi?id=218559
>> Fixes: 76fe464c8e64 ("leds: pwm: Don't disable the PWM when the LED should be off")
>> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
>> ---
>> Hello,
>>
>> On Wed, Apr 17, 2024 at 03:49:43PM +0100, Lee Jones wrote:
>>> On Tue, 16 Apr 2024, Uwe Kleine-König wrote:
>>>> If you don't consider that suitable, I can create a patch that is easier
>>>> to pick up.
>>>
>>> Yes, please submit it properly.
> 
> Gentle ping. Even given the regression was introduced in v6.7-rc1
> already, I think this should go into v6.9. [...]

FWIW, I guess Linus would agree (even if it's the second to last release
in this case):

https://lore.kernel.org/all/CAHk-=wis_qQy4oDNynNKi5b7Qhosmxtoj1jxo5wmB6SRUwQUBQ@mail.gmail.com/

Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
--
Everything you wanna know about Linux kernel regression tracking:
https://linux-regtracking.leemhuis.info/about/#tldr
If I did something stupid, please tell me, as explained on that page.
diff mbox series

Patch

diff --git a/drivers/leds/leds-pwm.c b/drivers/leds/leds-pwm.c
index 4e3936a39d0e..e1b414b40353 100644
--- a/drivers/leds/leds-pwm.c
+++ b/drivers/leds/leds-pwm.c
@@ -53,7 +53,13 @@  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 = true;
+	/*
+	 * Disabling a PWM doesn't guarantee that it emits the inactive level.
+	 * So keep it on. Only for suspending the PWM should be disabled because
+	 * otherwise it refuses to suspend. The possible downside is that the
+	 * LED might stay (or even go) on.
+	 */
+	led_dat->pwmstate.enabled = !(led_cdev->flags & LED_SUSPENDED);
 	return pwm_apply_might_sleep(led_dat->pwm, &led_dat->pwmstate);
 }