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

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

[Jakob Riepler]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 1584 bytes --]

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>
---
Changes in v2:
 - fix wrong line-breaks in patch
Changes in v3:
 - use git send-email
Changes in v4:
 - use correct address in s-o-b

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

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

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

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 1256 bytes --]

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

[Lee Jones]

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 [李琼斯]

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

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 948 bytes --]

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

[Lee Jones]

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. :)

-- 
Lee Jones [李琼斯]

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

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 929 bytes --]

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]