leds: pwm: Disable PWM when going to suspend

[PATCH] leds: pwm: Disable PWM when going to suspend

Posted by Uwe Kleine-König 1 year, 10 months ago

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

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

base-commit: 4eab358930711bbeb85bf5ee267d0d42d3394c2c
-- 
2.43.0

Re: (subset) [PATCH] leds: pwm: Disable PWM when going to suspend

Posted by Lee Jones 1 year, 9 months ago

On Wed, 17 Apr 2024 17:38:47 +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.
> 
> 

Applied, thanks!

[1/1] leds: pwm: Disable PWM when going to suspend
      commit: 974afccd37947a6951a052ef8118c961e57eaf7b

--
Lee Jones [李琼斯]

Re: (subset) [PATCH] leds: pwm: Disable PWM when going to suspend

Posted by Lee Jones 1 year, 9 months ago

On Thu, 02 May 2024, Lee Jones wrote:

> On Wed, 17 Apr 2024 17:38:47 +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.
> > 
> > 
> 
> Applied, thanks!
> 
> [1/1] leds: pwm: Disable PWM when going to suspend
>       commit: 974afccd37947a6951a052ef8118c961e57eaf7b

If this is important, you may wish to retroactively send it to Stable
once it's been merged into Mainline.

-- 
Lee Jones [李琼斯]

Re: [PATCH] leds: pwm: Disable PWM when going to suspend

Posted by Uwe Kleine-König 1 year, 9 months ago

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

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

Re: [PATCH] leds: pwm: Disable PWM when going to suspend

Posted by Linux regression tracking (Thorsten Leemhuis) 1 year, 9 months ago

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.