diff mbox series

[v2,1/4] leds: rgb: leds-group-multicolor: allow leds to stay on in suspend

Message ID 20240206185400.596979-1-aren@peacevolution.org
State New
Headers
Series [v2,1/4] leds: rgb: leds-group-multicolor: allow leds to stay on in suspend |

Commit Message

Aren Feb. 6, 2024, 6:13 p.m. UTC 
  If none of the managed leds enable LED_CORE_SUSPENDRESUME, then we
shouldn't need to set it here. This makes it possible to use multicolor
groups with gpio leds that enable retain-state-suspended in the device
tree.

Signed-off-by: Aren Moynihan <aren@peacevolution.org>
---

Changes in v2:
 - make sure count gets initialized
 - send the patch to (hopefully) all the correct people this time

 drivers/leds/rgb/leds-group-multicolor.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

Comments

Pavel Machek Feb. 22, 2024, 9:36 p.m. UTC | #1 
Hi!

> If none of the managed leds enable LED_CORE_SUSPENDRESUME, then we
> shouldn't need to set it here. This makes it possible to use multicolor
> groups with gpio leds that enable retain-state-suspended in the device
> tree.
> 
> Signed-off-by: Aren Moynihan <aren@peacevolution.org>

Thanks for the series.

Acked-by: Pavel Machek <pavel@ucw.cz>

Note this will change userland API and maybe break the LED for code
expecting old setup and hardcoding paths. I guess we should not
backport this to stable. But we should do this, because it is really
one LED and not three.

Best regards,
								Pavel
Lee Jones Feb. 23, 2024, 10:28 a.m. UTC | #2 
On Thu, 22 Feb 2024, Pavel Machek wrote:

> Hi!
> 
> > If none of the managed leds enable LED_CORE_SUSPENDRESUME, then we
> > shouldn't need to set it here. This makes it possible to use multicolor
> > groups with gpio leds that enable retain-state-suspended in the device
> > tree.
> > 
> > Signed-off-by: Aren Moynihan <aren@peacevolution.org>
> 
> Thanks for the series.
> 
> Acked-by: Pavel Machek <pavel@ucw.cz>
> 
> Note this will change userland API and maybe break the LED for code
> expecting old setup and hardcoding paths. I guess we should not
> backport this to stable. But we should do this, because it is really
> one LED and not three.

Thanks Pavel.

Is this tied to the other patches in the set?

Will thing break if this is applied on its own?
Lee Jones Feb. 23, 2024, 10:31 a.m. UTC | #3 
On Tue, 06 Feb 2024 13:13:17 -0500, Aren Moynihan wrote:
> If none of the managed leds enable LED_CORE_SUSPENDRESUME, then we
> shouldn't need to set it here. This makes it possible to use multicolor
> groups with gpio leds that enable retain-state-suspended in the device
> tree.
> 
> 

Applied, thanks!

[1/4] leds: rgb: leds-group-multicolor: allow leds to stay on in suspend
      commit: 68552911e71d59e62dd5e50e9ac56ebfc32f0c71

--
Lee Jones [李琼斯]
Lee Jones Feb. 23, 2024, 10:35 a.m. UTC | #4 
On Fri, 23 Feb 2024, Lee Jones wrote:

> On Tue, 06 Feb 2024 13:13:17 -0500, Aren Moynihan wrote:
> > If none of the managed leds enable LED_CORE_SUSPENDRESUME, then we
> > shouldn't need to set it here. This makes it possible to use multicolor
> > groups with gpio leds that enable retain-state-suspended in the device
> > tree.
> > 
> > 
> 
> Applied, thanks!
> 
> [1/4] leds: rgb: leds-group-multicolor: allow leds to stay on in suspend
>       commit: 68552911e71d59e62dd5e50e9ac56ebfc32f0c71

Note that I changed a bunch of grammatical issues.

led  => LED
gpio => GPIO

Capitalised the first word of the comment, etc.
Aren Feb. 23, 2024, 3:32 p.m. UTC | #5 
On Fri, Feb 23, 2024 at 10:35:37AM +0000, Lee Jones wrote:
> On Fri, 23 Feb 2024, Lee Jones wrote:
> 
> > On Tue, 06 Feb 2024 13:13:17 -0500, Aren Moynihan wrote:
> > > If none of the managed leds enable LED_CORE_SUSPENDRESUME, then we
> > > shouldn't need to set it here. This makes it possible to use multicolor
> > > groups with gpio leds that enable retain-state-suspended in the device
> > > tree.
> > > 
> > > 
> > 
> > Applied, thanks!
> > 
> > [1/4] leds: rgb: leds-group-multicolor: allow leds to stay on in suspend
> >       commit: 68552911e71d59e62dd5e50e9ac56ebfc32f0c71
> 
> Note that I changed a bunch of grammatical issues.
> 
> led  => LED
> gpio => GPIO
> 
> Capitalised the first word of the comment, etc.

Awesome, thank you
 - Aren

> -- 
> Lee Jones [李琼斯]
diff mbox series

Patch

diff --git a/drivers/leds/rgb/leds-group-multicolor.c b/drivers/leds/rgb/leds-group-multicolor.c
index 39f58be32af5..b6c7679015fd 100644
--- a/drivers/leds/rgb/leds-group-multicolor.c
+++ b/drivers/leds/rgb/leds-group-multicolor.c
@@ -69,7 +69,7 @@  static int leds_gmc_probe(struct platform_device *pdev)
 	struct mc_subled *subled;
 	struct leds_multicolor *priv;
 	unsigned int max_brightness = 0;
-	int i, ret, count = 0;
+	int i, ret, count = 0, common_flags = 0;
 
 	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
 	if (!priv)
@@ -91,6 +91,7 @@  static int leds_gmc_probe(struct platform_device *pdev)
 		if (!priv->monochromatics)
 			return -ENOMEM;
 
+		common_flags |= led_cdev->flags;
 		priv->monochromatics[count] = led_cdev;
 
 		max_brightness = max(max_brightness, led_cdev->max_brightness);
@@ -114,12 +115,15 @@  static int leds_gmc_probe(struct platform_device *pdev)
 
 	/* Initialise the multicolor's LED class device */
 	cdev = &priv->mc_cdev.led_cdev;
-	cdev->flags = LED_CORE_SUSPENDRESUME;
 	cdev->brightness_set_blocking = leds_gmc_set;
 	cdev->max_brightness = max_brightness;
 	cdev->color = LED_COLOR_ID_MULTI;
 	priv->mc_cdev.num_colors = count;
 
+	/* we only need suspend/resume if a sub-led requests it */
+	if (common_flags & LED_CORE_SUSPENDRESUME)
+		cdev->flags = LED_CORE_SUSPENDRESUME;
+
 	init_data.fwnode = dev_fwnode(dev);
 	ret = devm_led_classdev_multicolor_register_ext(dev, &priv->mc_cdev, &init_data);
 	if (ret)