| Message ID | 20241216213754.18374-2-jakob+lkml@paranoidlabs.org |
|---|---|
| State | New |
| Headers | show |
| Series | [v4] leds: pwm-multicolor: Disable PWM when going to suspend | expand |
On Mon, Dec 16, 2024 at 10:37:55PM +0100, Jakob Riepler wrote: > This fixes suspend on platforms like stm32mp1xx, where the PWM consumer > has to be disabled for the PWM to enter suspend. > Another positive side effect is that active-low LEDs now properly > turn off instead of going back to full brightness when they are set to 0. > > Link: https://lore.kernel.org/all/20240417153846.271751-2-u.kleine-koenig@pengutronix.de/ > Signed-off-by: Jakob Riepler <jakob+lkml@paranoidlabs.org> > Acked-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com> Now there are just nitpicks left: - Your S-o-b usually comes last. - The encoding of your mail is strange. It claims 8bit us-ascii which doesn't make sense. Depending on the mail reader and its locale my name might show as Uwe Kleine-K��nig. See also https://lore.kernel.org/all/20241216213754.18374-2-jakob+lkml@paranoidlabs.org/ which says "Warning: decoded text below may be mangled, UTF-8 assumed". Many maintainers don't care about the first item and if Lee has a sane environment my name makes it correctly into the git history. So from my side that's no reason for a v5, but for future patches it would be great to improve here. Best regards Uwe
On Mon, 16 Dec 2024 22:37:55 +0100, Jakob Riepler wrote: > This fixes suspend on platforms like stm32mp1xx, where the PWM consumer > has to be disabled for the PWM to enter suspend. > Another positive side effect is that active-low LEDs now properly > turn off instead of going back to full brightness when they are set to 0. > > Applied, thanks! [1/1] leds: pwm-multicolor: Disable PWM when going to suspend commit: 29df7025cff00dd9fa7cacbec979ede97ee775eb -- Lee Jones [李琼斯]
Hello Lee, On Tue, Dec 17, 2024 at 03:05:06PM +0000, Lee Jones wrote: > On Mon, 16 Dec 2024 22:37:55 +0100, Jakob Riepler wrote: > > This fixes suspend on platforms like stm32mp1xx, where the PWM consumer > > has to be disabled for the PWM to enter suspend. > > Another positive side effect is that active-low LEDs now properly > > turn off instead of going back to full brightness when they are set to 0. > > > > > > Applied, thanks! > > [1/1] leds: pwm-multicolor: Disable PWM when going to suspend > commit: 29df7025cff00dd9fa7cacbec979ede97ee775eb Where is this applied? I checked today's next and even after for b in for-leds-next for-leds-next-next leds-fixes master; do git fetch git://git.kernel.org/pub/scm/linux/kernel/git/lee/leds.git refs/heads/$b; done I don't have the above commit object and these branches don't seem to contain the commit with a different commit id either. Best regards Uwe
On Thu, 19 Dec 2024, Uwe Kleine-König wrote: > Hello Lee, > > On Tue, Dec 17, 2024 at 03:05:06PM +0000, Lee Jones wrote: > > On Mon, 16 Dec 2024 22:37:55 +0100, Jakob Riepler wrote: > > > This fixes suspend on platforms like stm32mp1xx, where the PWM consumer > > > has to be disabled for the PWM to enter suspend. > > > Another positive side effect is that active-low LEDs now properly > > > turn off instead of going back to full brightness when they are set to 0. > > > > > > > > > > Applied, thanks! > > > > [1/1] leds: pwm-multicolor: Disable PWM when going to suspend > > commit: 29df7025cff00dd9fa7cacbec979ede97ee775eb > > Where is this applied? I checked today's next and even after > > for b in for-leds-next for-leds-next-next leds-fixes master; do git fetch git://git.kernel.org/pub/scm/linux/kernel/git/lee/leds.git refs/heads/$b; done > > I don't have the above commit object and these branches don't seem to > contain the commit with a different commit id either. Check again tomorrow. :)
Hello Lee, On Thu, Dec 19, 2024 at 08:25:04AM +0000, Lee Jones wrote: > On Thu, 19 Dec 2024, Uwe Kleine-König wrote: > > On Tue, Dec 17, 2024 at 03:05:06PM +0000, Lee Jones wrote: > > > Applied, thanks! > > > > > > [1/1] leds: pwm-multicolor: Disable PWM when going to suspend > > > commit: 29df7025cff00dd9fa7cacbec979ede97ee775eb > > > > Where is this applied? I checked today's next and even after > > > > for b in for-leds-next for-leds-next-next leds-fixes master; do git fetch git://git.kernel.org/pub/scm/linux/kernel/git/lee/leds.git refs/heads/$b; done > > > > I don't have the above commit object and these branches don't seem to > > contain the commit with a different commit id either. > > Check again tomorrow. :) I was impatient and found the commit now in your for-leds-next branch. I wanted to double check that there is no encoding problem. Everything is fine. Thanks Uwe
diff --git a/drivers/leds/rgb/leds-pwm-multicolor.c b/drivers/leds/rgb/leds-pwm-multicolor.c index e1a81e0109e8..f80a06cc31f8 100644 --- a/drivers/leds/rgb/leds-pwm-multicolor.c +++ b/drivers/leds/rgb/leds-pwm-multicolor.c @@ -50,7 +50,13 @@ static int led_pwm_mc_set(struct led_classdev *cdev, duty = priv->leds[i].state.period - duty; priv->leds[i].state.duty_cycle = duty; - priv->leds[i].state.enabled = duty > 0; + /* + * 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. + */ + priv->leds[i].state.enabled = !(cdev->flags & LED_SUSPENDED); ret = pwm_apply_might_sleep(priv->leds[i].pwm, &priv->leds[i].state); if (ret)