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

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

[Jakob Riepler]

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@chaosfield.at>
---
  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.0

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

[Jakob Riepler]

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@chaosfield.at>
---
Changes in v2:
 - fix wrong line-breaks in patch

 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.0

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

[Lee Jones]

On Mon, 02 Dec 2024, 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@chaosfield.at>
> ---
> Changes in v2:
>  - fix wrong line-breaks in patch

I get a warning that this patch is corrupt as well.

How are you creating them?

Please use `git format-patch`, `git send-email` or `b4`.

-- 
Lee Jones [李琼斯]

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

[Uwe Kleine-König]

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

On Mon, Dec 02, 2024 at 01:07:51PM +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@chaosfield.at>

Acked-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com>

Best regards
Uwe

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

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

[Jakob Riepler]

> I get a warning that this patch is corrupt as well.
> 
> How are you creating them?

`git format-patch` and thunderbird with the settings mentioned in the 
kernel documentation.

> Please use `git format-patch`, `git send-email` or `b4`.

If you'd like I can resend with a different mail server - I had trouble 
setting up `git send-email` with my primary mail provider (enforced oauth)

- Jakob

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

[Lee Jones]

On Fri, 13 Dec 2024, Jakob Riepler wrote:

> > I get a warning that this patch is corrupt as well.
> > 
> > How are you creating them?
> 
> `git format-patch` and thunderbird with the settings mentioned in the kernel
> documentation.
> 
> > Please use `git format-patch`, `git send-email` or `b4`.
> 
> If you'd like I can resend with a different mail server - I had trouble
> setting up `git send-email` with my primary mail provider (enforced oauth)

I haven't tried it for sending patches, but I hear b4 is good.

I can't take what you have sent here - it doesn't apply.

-- 
Lee Jones [李琼斯]

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

[Jakob Riepler]

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@chaosfield.at>
---
Changes in v2:
 - fix wrong line-breaks in patch
Changes in v3:
 - use git send-email

Sorry for the issues.
I'm using git send-email with a different mailserver now so the patch
formatting should now be correct - at least in my tests git am was
happy now.

Best regards
Jakob

 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 v3] leds: pwm-multicolor: Disable PWM when going to suspend

[Uwe Kleine-König]

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

Hello Jakob,

On Fri, Dec 13, 2024 at 08:10:41PM +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@chaosfield.at>
> ---
> Changes in v2:
>  - fix wrong line-breaks in patch
> Changes in v3:
>  - use git send-email

Applies fine for me now.

I already gave my Ack for v2, repeating it here to make it easy to pick
up:

Acked-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com>

Please add this to your v4 if you have to send a new version. I don't
know how picky Lee is, but the email address of the sender doesn't match
the S-o-b line ...

Best regards
Uwe

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

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

[Lee Jones]

On Sat, 14 Dec 2024, Uwe Kleine-König wrote:

> Hello Jakob,
> 
> On Fri, Dec 13, 2024 at 08:10:41PM +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@chaosfield.at>
> > ---
> > Changes in v2:
> >  - fix wrong line-breaks in patch
> > Changes in v3:
> >  - use git send-email
> 
> Applies fine for me now.
> 
> I already gave my Ack for v2, repeating it here to make it easy to pick
> up:
> 
> Acked-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com>
> 
> Please add this to your v4 if you have to send a new version. I don't
> know how picky Lee is, but the email address of the sender doesn't match
> the S-o-b line ...

Very picky!  Please resend.

Also, please send subsequent versions as a new thread.

-- 
Lee Jones [李琼斯]

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

[Lee Jones]

On Mon, 02 Dec 2024 13:07:51 +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 [李琼斯]