Message ID | 20240417153846.271751-2-u.kleine-koenig@pengutronix.de |
---|---|
State | New |
Headers | show |
Series | leds: pwm: Disable PWM when going to suspend | expand |
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
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.
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 [李琼斯]
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.
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); }