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

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

[Aren Moynihan]

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

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

diff --git a/drivers/leds/rgb/leds-group-multicolor.c b/drivers/leds/rgb/leds-group-multicolor.c
index 39f58be32af5..194c6a33640b 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, 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)
-- 
2.43.0

[PATCH 2/4] arm64: dts: sun50i-a64-pinephone: Retain leds state in suspend

[Aren Moynihan]

From: Miles Alan <m@milesalan.com>

Allows user to set a led before entering suspend to know that
the phone is still on (or could be used for notifications etc.)

Signed-off-by: Miles Alan <m@milesalan.com>
Signed-off-by: Ondrej Jirman <megi@xff.cz>
Signed-off-by: Aren Moynihan <aren@peacevolution.org>
---

 arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtsi | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtsi
index 87847116ab6d..ad2476ee01e4 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtsi
@@ -43,18 +43,21 @@ led-0 {
 			function = LED_FUNCTION_INDICATOR;
 			color = <LED_COLOR_ID_BLUE>;
 			gpios = <&pio 3 20 GPIO_ACTIVE_HIGH>; /* PD20 */
+			retain-state-suspended;
 		};
 
 		led-1 {
 			function = LED_FUNCTION_INDICATOR;
 			color = <LED_COLOR_ID_GREEN>;
 			gpios = <&pio 3 18 GPIO_ACTIVE_HIGH>; /* PD18 */
+			retain-state-suspended;
 		};
 
 		led-2 {
 			function = LED_FUNCTION_INDICATOR;
 			color = <LED_COLOR_ID_RED>;
 			gpios = <&pio 3 19 GPIO_ACTIVE_HIGH>; /* PD19 */
+			retain-state-suspended;
 		};
 	};
 
-- 
2.43.0

[PATCH 3/4] arm64: dts: sun50i-a64-pinephone: add multicolor led node

[Aren Moynihan]

The red, green, and blue leds currently in the device tree represent a
single rgb led on the front of the PinePhone.

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

 .../boot/dts/allwinner/sun50i-a64-pinephone.dtsi    | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtsi
index ad2476ee01e4..6eab61a12cd8 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtsi
@@ -39,21 +39,21 @@ chosen {
 	leds {
 		compatible = "gpio-leds";
 
-		led-0 {
+		led0: led-0 {
 			function = LED_FUNCTION_INDICATOR;
 			color = <LED_COLOR_ID_BLUE>;
 			gpios = <&pio 3 20 GPIO_ACTIVE_HIGH>; /* PD20 */
 			retain-state-suspended;
 		};
 
-		led-1 {
+		led1: led-1 {
 			function = LED_FUNCTION_INDICATOR;
 			color = <LED_COLOR_ID_GREEN>;
 			gpios = <&pio 3 18 GPIO_ACTIVE_HIGH>; /* PD18 */
 			retain-state-suspended;
 		};
 
-		led-2 {
+		led2: led-2 {
 			function = LED_FUNCTION_INDICATOR;
 			color = <LED_COLOR_ID_RED>;
 			gpios = <&pio 3 19 GPIO_ACTIVE_HIGH>; /* PD19 */
@@ -61,6 +61,13 @@ led-2 {
 		};
 	};
 
+	multi-led {
+		compatible = "leds-group-multicolor";
+		color = <LED_COLOR_ID_RGB>;
+		function = LED_FUNCTION_INDICATOR;
+		leds = <&led0>, <&led1>, <&led2>;
+	};
+
 	reg_ps: ps-regulator {
 		compatible = "regulator-fixed";
 		regulator-name = "ps";
-- 
2.43.0

[PATCH 4/4] arm64: dts: sun50i-a64-pinephone: change led type to status

[Aren Moynihan]

The status function is described in the documentation as being a rgb led
used for system notifications on phones[1][2]. This is exactly what this
led is used for on the PinePhone, so using status is probably more
accurate than indicator.

1: Documentation/leds/well-known-leds.txt
2: include/dt-bindings/leds/common.h

Signed-off-by: Aren Moynihan <aren@peacevolution.org>
---
I can't find any documentation describing the indicator function, so
it's definitely less specific than status, but besides that I'm not sure
how it compares.

Please ignore this patch if it's not useful and/or just causing churn.

 arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtsi | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtsi
index 6eab61a12cd8..4f39cfeb13ec 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtsi
@@ -40,21 +40,21 @@ leds {
 		compatible = "gpio-leds";
 
 		led0: led-0 {
-			function = LED_FUNCTION_INDICATOR;
+			function = LED_FUNCTION_STATUS;
 			color = <LED_COLOR_ID_BLUE>;
 			gpios = <&pio 3 20 GPIO_ACTIVE_HIGH>; /* PD20 */
 			retain-state-suspended;
 		};
 
 		led1: led-1 {
-			function = LED_FUNCTION_INDICATOR;
+			function = LED_FUNCTION_STATUS;
 			color = <LED_COLOR_ID_GREEN>;
 			gpios = <&pio 3 18 GPIO_ACTIVE_HIGH>; /* PD18 */
 			retain-state-suspended;
 		};
 
 		led2: led-2 {
-			function = LED_FUNCTION_INDICATOR;
+			function = LED_FUNCTION_STATUS;
 			color = <LED_COLOR_ID_RED>;
 			gpios = <&pio 3 19 GPIO_ACTIVE_HIGH>; /* PD19 */
 			retain-state-suspended;
@@ -64,7 +64,7 @@ led2: led-2 {
 	multi-led {
 		compatible = "leds-group-multicolor";
 		color = <LED_COLOR_ID_RGB>;
-		function = LED_FUNCTION_INDICATOR;
+		function = LED_FUNCTION_STATUS;
 		leds = <&led0>, <&led1>, <&led2>;
 	};
 
-- 
2.43.0

Re: [PATCH 2/4] arm64: dts: sun50i-a64-pinephone: Retain leds state in suspend

[Jernej Škrabec]

Dne nedelja, 28. januar 2024 ob 21:45:08 CET je Aren Moynihan napisal(a):
> From: Miles Alan <m@milesalan.com>
> 
> Allows user to set a led before entering suspend to know that
> the phone is still on (or could be used for notifications etc.)
> 
> Signed-off-by: Miles Alan <m@milesalan.com>
> Signed-off-by: Ondrej Jirman <megi@xff.cz>
> Signed-off-by: Aren Moynihan <aren@peacevolution.org>

Where is patch 1 and possibly cover letter? Please resend with all patches.

However, this particular patch is:
Reviewed-by: Jernej Skrabec <jernej.skrabec@gmail.com>

Best regards,
Jernej

> ---
> 
>  arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtsi | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtsi
> index 87847116ab6d..ad2476ee01e4 100644
> --- a/arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtsi
> +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtsi
> @@ -43,18 +43,21 @@ led-0 {
>  			function = LED_FUNCTION_INDICATOR;
>  			color = <LED_COLOR_ID_BLUE>;
>  			gpios = <&pio 3 20 GPIO_ACTIVE_HIGH>; /* PD20 */
> +			retain-state-suspended;
>  		};
>  
>  		led-1 {
>  			function = LED_FUNCTION_INDICATOR;
>  			color = <LED_COLOR_ID_GREEN>;
>  			gpios = <&pio 3 18 GPIO_ACTIVE_HIGH>; /* PD18 */
> +			retain-state-suspended;
>  		};
>  
>  		led-2 {
>  			function = LED_FUNCTION_INDICATOR;
>  			color = <LED_COLOR_ID_RED>;
>  			gpios = <&pio 3 19 GPIO_ACTIVE_HIGH>; /* PD19 */
> +			retain-state-suspended;
>  		};
>  	};
>  
> 

Re: [PATCH 3/4] arm64: dts: sun50i-a64-pinephone: add multicolor led node

[Jernej Škrabec]

Dne nedelja, 28. januar 2024 ob 21:45:09 CET je Aren Moynihan napisal(a):
> The red, green, and blue leds currently in the device tree represent a
> single rgb led on the front of the PinePhone.
> 
> Signed-off-by: Aren Moynihan <aren@peacevolution.org>
> ---
> 
>  .../boot/dts/allwinner/sun50i-a64-pinephone.dtsi    | 13 ++++++++++---
>  1 file changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtsi
> index ad2476ee01e4..6eab61a12cd8 100644
> --- a/arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtsi
> +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtsi
> @@ -39,21 +39,21 @@ chosen {
>  	leds {
>  		compatible = "gpio-leds";
>  
> -		led-0 {
> +		led0: led-0 {
>  			function = LED_FUNCTION_INDICATOR;
>  			color = <LED_COLOR_ID_BLUE>;
>  			gpios = <&pio 3 20 GPIO_ACTIVE_HIGH>; /* PD20 */
>  			retain-state-suspended;
>  		};
>  
> -		led-1 {
> +		led1: led-1 {
>  			function = LED_FUNCTION_INDICATOR;
>  			color = <LED_COLOR_ID_GREEN>;
>  			gpios = <&pio 3 18 GPIO_ACTIVE_HIGH>; /* PD18 */
>  			retain-state-suspended;
>  		};
>  
> -		led-2 {
> +		led2: led-2 {
>  			function = LED_FUNCTION_INDICATOR;
>  			color = <LED_COLOR_ID_RED>;
>  			gpios = <&pio 3 19 GPIO_ACTIVE_HIGH>; /* PD19 */
> @@ -61,6 +61,13 @@ led-2 {
>  		};
>  	};
>  
> +	multi-led {
> +		compatible = "leds-group-multicolor";
> +		color = <LED_COLOR_ID_RGB>;
> +		function = LED_FUNCTION_INDICATOR;

Does it make sense to have function specified here and above? Example
specifies it only in multi-led node.

Best regards,
Jernej

> +		leds = <&led0>, <&led1>, <&led2>;
> +	};
> +
>  	reg_ps: ps-regulator {
>  		compatible = "regulator-fixed";
>  		regulator-name = "ps";
> 

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

[Jean-Jacques Hiblot]

On 28/01/2024 21:45, 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.
> 
> Signed-off-by: Aren Moynihan <aren@peacevolution.org>
> ---
> 
>   drivers/leds/rgb/leds-group-multicolor.c | 8 ++++++--
>   1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/leds/rgb/leds-group-multicolor.c b/drivers/leds/rgb/leds-group-multicolor.c
> index 39f58be32af5..194c6a33640b 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, common_flags = 0;

count is not initialized anymore. Isn't it a problem ?
>   
>   	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)

Re: [PATCH 3/4] arm64: dts: sun50i-a64-pinephone: add multicolor led node

[Aren]

On Tue, Jan 30, 2024 at 08:41:14PM +0100, Jernej Škrabec wrote:
> Dne nedelja, 28. januar 2024 ob 21:45:09 CET je Aren Moynihan napisal(a):
> > The red, green, and blue leds currently in the device tree represent a
> > single rgb led on the front of the PinePhone.
> > 
> > Signed-off-by: Aren Moynihan <aren@peacevolution.org>
> > ---
> > 
> >  .../boot/dts/allwinner/sun50i-a64-pinephone.dtsi    | 13 ++++++++++---
> >  1 file changed, 10 insertions(+), 3 deletions(-)
> > 
> > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtsi
> > index ad2476ee01e4..6eab61a12cd8 100644
> > --- a/arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtsi
> > +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtsi
> > @@ -39,21 +39,21 @@ chosen {
> >  	leds {
> >  		compatible = "gpio-leds";
> >  
> > -		led-0 {
> > +		led0: led-0 {
> >  			function = LED_FUNCTION_INDICATOR;
> >  			color = <LED_COLOR_ID_BLUE>;
> >  			gpios = <&pio 3 20 GPIO_ACTIVE_HIGH>; /* PD20 */
> >  			retain-state-suspended;
> >  		};
> >  
> > -		led-1 {
> > +		led1: led-1 {
> >  			function = LED_FUNCTION_INDICATOR;
> >  			color = <LED_COLOR_ID_GREEN>;
> >  			gpios = <&pio 3 18 GPIO_ACTIVE_HIGH>; /* PD18 */
> >  			retain-state-suspended;
> >  		};
> >  
> > -		led-2 {
> > +		led2: led-2 {
> >  			function = LED_FUNCTION_INDICATOR;
> >  			color = <LED_COLOR_ID_RED>;
> >  			gpios = <&pio 3 19 GPIO_ACTIVE_HIGH>; /* PD19 */
> > @@ -61,6 +61,13 @@ led-2 {
> >  		};
> >  	};
> >  
> > +	multi-led {
> > +		compatible = "leds-group-multicolor";
> > +		color = <LED_COLOR_ID_RGB>;
> > +		function = LED_FUNCTION_INDICATOR;
> 
> Does it make sense to have function specified here and above? Example
> specifies it only in multi-led node.

I'm not sure it makes much of a difference, besides perhaps confusing
userspace. From what I can tell the only thing it'll change is the name
of the directory in sysfs from "<color>:status" to "<color>:". I'll
change this in v2 unless anyone has a reason not to.

Thanks
 - Aren

> Best regards,
> Jernej
> 
> > +		leds = <&led0>, <&led1>, <&led2>;
> > +	};
> > +
> >  	reg_ps: ps-regulator {
> >  		compatible = "regulator-fixed";
> >  		regulator-name = "ps";

Re: [PATCH 2/4] arm64: dts: sun50i-a64-pinephone: Retain leds state in suspend

[Aren]

On Tue, Jan 30, 2024 at 08:06:24PM +0100, Jernej Škrabec wrote:
> Dne nedelja, 28. januar 2024 ob 21:45:08 CET je Aren Moynihan napisal(a):
> > From: Miles Alan <m@milesalan.com>
> > 
> > Allows user to set a led before entering suspend to know that
> > the phone is still on (or could be used for notifications etc.)
> > 
> > Signed-off-by: Miles Alan <m@milesalan.com>
> > Signed-off-by: Ondrej Jirman <megi@xff.cz>
> > Signed-off-by: Aren Moynihan <aren@peacevolution.org>
> 
> Where is patch 1 and possibly cover letter? Please resend with all patches.

Oops, sorry about that, I'm still getting used to get_maintainer.pl.
I'll resend this properly when I have time this weekend, until then the
patch you missed is available at
https://lore.kernel.org/lkml/20240128204740.2355092-1-aren@peacevolution.org/

> However, this particular patch is:
> Reviewed-by: Jernej Skrabec <jernej.skrabec@gmail.com>

Thanks
 - Aren

> Best regards,
> Jernej
> 
> > ---
> > 
> >  arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtsi | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtsi
> > index 87847116ab6d..ad2476ee01e4 100644
> > --- a/arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtsi
> > +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtsi
> > @@ -43,18 +43,21 @@ led-0 {
> >  			function = LED_FUNCTION_INDICATOR;
> >  			color = <LED_COLOR_ID_BLUE>;
> >  			gpios = <&pio 3 20 GPIO_ACTIVE_HIGH>; /* PD20 */
> > +			retain-state-suspended;
> >  		};
> >  
> >  		led-1 {
> >  			function = LED_FUNCTION_INDICATOR;
> >  			color = <LED_COLOR_ID_GREEN>;
> >  			gpios = <&pio 3 18 GPIO_ACTIVE_HIGH>; /* PD18 */
> > +			retain-state-suspended;
> >  		};
> >  
> >  		led-2 {
> >  			function = LED_FUNCTION_INDICATOR;
> >  			color = <LED_COLOR_ID_RED>;
> >  			gpios = <&pio 3 19 GPIO_ACTIVE_HIGH>; /* PD19 */
> > +			retain-state-suspended;
> >  		};
> >  	};
> >  
> > 
> 
> 
> 
> 

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

[Aren]

On Wed, Jan 31, 2024 at 08:39:01AM +0100, Jean-Jacques Hiblot wrote:
> 
> 
> On 28/01/2024 21:45, 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.
> > 
> > Signed-off-by: Aren Moynihan <aren@peacevolution.org>
> > ---
> > 
> >   drivers/leds/rgb/leds-group-multicolor.c | 8 ++++++--
> >   1 file changed, 6 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/leds/rgb/leds-group-multicolor.c b/drivers/leds/rgb/leds-group-multicolor.c
> > index 39f58be32af5..194c6a33640b 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, common_flags = 0;
> 
> count is not initialized anymore. Isn't it a problem ?

Yes, good catch, thanks!

I'm not sure how I missed that when I tested this, I must've gotten
(un)lucky with the value that was in ram before.

 - Aren

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

Re: [PATCH 2/4] arm64: dts: sun50i-a64-pinephone: Retain leds state in suspend

[Jernej Škrabec]

Dne četrtek, 01. februar 2024 ob 02:36:46 CET je Aren napisal(a):
> On Tue, Jan 30, 2024 at 08:06:24PM +0100, Jernej Škrabec wrote:
> > Dne nedelja, 28. januar 2024 ob 21:45:08 CET je Aren Moynihan napisal(a):
> > > From: Miles Alan <m@milesalan.com>
> > > 
> > > Allows user to set a led before entering suspend to know that
> > > the phone is still on (or could be used for notifications etc.)
> > > 
> > > Signed-off-by: Miles Alan <m@milesalan.com>
> > > Signed-off-by: Ondrej Jirman <megi@xff.cz>
> > > Signed-off-by: Aren Moynihan <aren@peacevolution.org>
> > 
> > Where is patch 1 and possibly cover letter? Please resend with all patches.
> 
> Oops, sorry about that, I'm still getting used to get_maintainer.pl.
> I'll resend this properly when I have time this weekend, until then the
> patch you missed is available at
> https://lore.kernel.org/lkml/20240128204740.2355092-1-aren@peacevolution.org/

When sending patch series it's customary to send all patches to all
maintainers and mailing lists (to have a context). In case of large patch
series, you can send only selected patches to each maintainer and mailing
lists, but then send cover letter to all involved and explain general idea
behind the series.

Best regards,
Jernej

> 
> > However, this particular patch is:
> > Reviewed-by: Jernej Skrabec <jernej.skrabec@gmail.com>
> 
> Thanks
>  - Aren
> 
> > Best regards,
> > Jernej
> > 
> > > ---
> > > 
> > >  arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtsi | 3 +++
> > >  1 file changed, 3 insertions(+)
> > > 
> > > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtsi
> > > index 87847116ab6d..ad2476ee01e4 100644
> > > --- a/arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtsi
> > > +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtsi
> > > @@ -43,18 +43,21 @@ led-0 {
> > >  			function = LED_FUNCTION_INDICATOR;
> > >  			color = <LED_COLOR_ID_BLUE>;
> > >  			gpios = <&pio 3 20 GPIO_ACTIVE_HIGH>; /* PD20 */
> > > +			retain-state-suspended;
> > >  		};
> > >  
> > >  		led-1 {
> > >  			function = LED_FUNCTION_INDICATOR;
> > >  			color = <LED_COLOR_ID_GREEN>;
> > >  			gpios = <&pio 3 18 GPIO_ACTIVE_HIGH>; /* PD18 */
> > > +			retain-state-suspended;
> > >  		};
> > >  
> > >  		led-2 {
> > >  			function = LED_FUNCTION_INDICATOR;
> > >  			color = <LED_COLOR_ID_RED>;
> > >  			gpios = <&pio 3 19 GPIO_ACTIVE_HIGH>; /* PD19 */
> > > +			retain-state-suspended;
> > >  		};
> > >  	};
> > >  
> > > 
> > 
> > 
> > 
> > 
>