[PATCH] leds: group-multicolor: Add support for initial value.

[PATCH] leds: group-multicolor: Add support for initial value.

[Martijn de Gouw]

It's possible to set a default state for leds in the dts with
'default-state', but this was not reflected when the LEDs are grouped.
This patch adds support for keeping the default-state value.

Signed-off-by: Martijn de Gouw <martijn.de.gouw@prodrive-technologies.com>
---
 drivers/leds/rgb/leds-group-multicolor.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/leds/rgb/leds-group-multicolor.c b/drivers/leds/rgb/leds-group-multicolor.c
index 548c7dd63ba1e..b3e46a51dfbc7 100644
--- a/drivers/leds/rgb/leds-group-multicolor.c
+++ b/drivers/leds/rgb/leds-group-multicolor.c
@@ -69,6 +69,7 @@ static int leds_gmc_probe(struct platform_device *pdev)
 	struct mc_subled *subled;
 	struct leds_multicolor *priv;
 	unsigned int max_brightness = 0;
+	unsigned int default_brightness = 0;
 	int i, ret, count = 0, common_flags = 0;
 
 	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
@@ -96,6 +97,12 @@ static int leds_gmc_probe(struct platform_device *pdev)
 
 		max_brightness = max(max_brightness, led_cdev->max_brightness);
 
+		/* If any LED is on, set brightness to the max brightness.
+		 * The actual brightness of the LED is set as intensity value.
+		 */
+		if (led_cdev->brightness)
+			default_brightness = max_brightness;
+
 		count++;
 	}
 
@@ -109,14 +116,16 @@ static int leds_gmc_probe(struct platform_device *pdev)
 
 		subled[i].color_index = led_cdev->color;
 
-		/* Configure the LED intensity to its maximum */
-		subled[i].intensity = max_brightness;
+		/* Configure the LED intensity to its current brightness */
+		subled[i].intensity = DIV_ROUND_CLOSEST(led_cdev->brightness * max_brightness,
+							led_cdev->max_brightness);
 	}
 
 	/* Initialise the multicolor's LED class device */
 	cdev = &priv->mc_cdev.led_cdev;
 	cdev->brightness_set_blocking = leds_gmc_set;
 	cdev->max_brightness = max_brightness;
+	cdev->brightness = default_brightness;
 	cdev->color = LED_COLOR_ID_MULTI;
 	priv->mc_cdev.num_colors = count;
 
-- 
2.39.2

Re: [PATCH] leds: group-multicolor: Add support for initial value.

[Lee Jones]

On Tue, 11 Nov 2025, Martijn de Gouw wrote:

> It's possible to set a default state for leds in the dts with
> 'default-state', but this was not reflected when the LEDs are grouped.
> This patch adds support for keeping the default-state value.
> 
> Signed-off-by: Martijn de Gouw <martijn.de.gouw@prodrive-technologies.com>
> ---
>  drivers/leds/rgb/leds-group-multicolor.c | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/leds/rgb/leds-group-multicolor.c b/drivers/leds/rgb/leds-group-multicolor.c
> index 548c7dd63ba1e..b3e46a51dfbc7 100644
> --- a/drivers/leds/rgb/leds-group-multicolor.c
> +++ b/drivers/leds/rgb/leds-group-multicolor.c
> @@ -69,6 +69,7 @@ static int leds_gmc_probe(struct platform_device *pdev)
>  	struct mc_subled *subled;
>  	struct leds_multicolor *priv;
>  	unsigned int max_brightness = 0;
> +	unsigned int default_brightness = 0;
>  	int i, ret, count = 0, common_flags = 0;
>  
>  	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> @@ -96,6 +97,12 @@ static int leds_gmc_probe(struct platform_device *pdev)
>  
>  		max_brightness = max(max_brightness, led_cdev->max_brightness);
>  
> +		/* If any LED is on, set brightness to the max brightness.
> +		 * The actual brightness of the LED is set as intensity value.
> +		 */

I don't know this code well, but if no one complains, I can take your
word for this.

However, the comment needs changing to proper multi-line format.

        /*
         * This is the preferred style for multi-line
         * comments in the Linux kernel source code.
         * Please use it consistently.
         *
         * Description:  A column of asterisks on the left side,
         * with beginning and ending almost-blank lines.
         */

> +		if (led_cdev->brightness)
> +			default_brightness = max_brightness;
> +
>  		count++;
>  	}
>  
> @@ -109,14 +116,16 @@ static int leds_gmc_probe(struct platform_device *pdev)
>  
>  		subled[i].color_index = led_cdev->color;
>  
> -		/* Configure the LED intensity to its maximum */
> -		subled[i].intensity = max_brightness;
> +		/* Configure the LED intensity to its current brightness */
> +		subled[i].intensity = DIV_ROUND_CLOSEST(led_cdev->brightness * max_brightness,

How does this work?  Won't this value be huge?

> +							led_cdev->max_brightness);

Also we said we were going to set actual brightness with the intensity
in the comment above, but we appear to be using max_brightness again?

>  	}
>  
>  	/* Initialise the multicolor's LED class device */
>  	cdev = &priv->mc_cdev.led_cdev;
>  	cdev->brightness_set_blocking = leds_gmc_set;
>  	cdev->max_brightness = max_brightness;
> +	cdev->brightness = default_brightness;
>  	cdev->color = LED_COLOR_ID_MULTI;
>  	priv->mc_cdev.num_colors = count;
>  
> -- 
> 2.39.2
> 

-- 
Lee Jones [李琼斯]

Re: [PATCH] leds: group-multicolor: Add support for initial value.

[Martijn de Gouw]

Hi,

On 11/19/2025 5:51 PM, Lee Jones wrote:
> On Tue, 11 Nov 2025, Martijn de Gouw wrote:
> 
>> It's possible to set a default state for leds in the dts with
>> 'default-state', but this was not reflected when the LEDs are grouped.
>> This patch adds support for keeping the default-state value.
>>
>> Signed-off-by: Martijn de Gouw <martijn.de.gouw@prodrive-technologies.com>
>> ---
>>  drivers/leds/rgb/leds-group-multicolor.c | 13 +++++++++++--
>>  1 file changed, 11 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/leds/rgb/leds-group-multicolor.c b/drivers/leds/rgb/leds-group-multicolor.c
>> index 548c7dd63ba1e..b3e46a51dfbc7 100644
>> --- a/drivers/leds/rgb/leds-group-multicolor.c
>> +++ b/drivers/leds/rgb/leds-group-multicolor.c
>> @@ -69,6 +69,7 @@ static int leds_gmc_probe(struct platform_device *pdev)
>>  	struct mc_subled *subled;
>>  	struct leds_multicolor *priv;
>>  	unsigned int max_brightness = 0;
>> +	unsigned int default_brightness = 0;
>>  	int i, ret, count = 0, common_flags = 0;
>>  
>>  	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
>> @@ -96,6 +97,12 @@ static int leds_gmc_probe(struct platform_device *pdev)
>>  
>>  		max_brightness = max(max_brightness, led_cdev->max_brightness);
>>  
>> +		/* If any LED is on, set brightness to the max brightness.
>> +		 * The actual brightness of the LED is set as intensity value.
>> +		 */
> 
> I don't know this code well, but if no one complains, I can take your
> word for this.
> 
> However, the comment needs changing to proper multi-line format.
> 
>         /*
>          * This is the preferred style for multi-line
>          * comments in the Linux kernel source code.
>          * Please use it consistently.
>          *
>          * Description:  A column of asterisks on the left side,
>          * with beginning and ending almost-blank lines.
>          */

Will do!

> 
>> +		if (led_cdev->brightness)
>> +			default_brightness = max_brightness;
>> +
>>  		count++;
>>  	}
>>  
>> @@ -109,14 +116,16 @@ static int leds_gmc_probe(struct platform_device *pdev)
>>  
>>  		subled[i].color_index = led_cdev->color;
>>  
>> -		/* Configure the LED intensity to its maximum */
>> -		subled[i].intensity = max_brightness;
>> +		/* Configure the LED intensity to its current brightness */
>> +		subled[i].intensity = DIV_ROUND_CLOSEST(led_cdev->brightness * max_brightness,
> 
> How does this work?  Won't this value be huge?

It calculates the intensity of the led with respect to the max_brightness of the whole group.
So actually it's doing:
subled[i].intensity = group->max_brightness * (led_cdev->brightness/led_cdev->max_brightness).

I highest value I could find for a max_brightness is 4096 (12 bit). Maybe some PWM LEDs use up to 16bit?
So I made the assumption that is should not be an issue.
I wrote it this way, where the multiplication comes first, because integer divisions don't work well.
> 
>> +							led_cdev->max_brightness);
> 
> Also we said we were going to set actual brightness with the intensity
> in the comment above, but we appear to be using max_brightness again?

I'll try to clarify the comments.

> 
>>  	}
>>  
>>  	/* Initialise the multicolor's LED class device */
>>  	cdev = &priv->mc_cdev.led_cdev;
>>  	cdev->brightness_set_blocking = leds_gmc_set;
>>  	cdev->max_brightness = max_brightness;
>> +	cdev->brightness = default_brightness;
>>  	cdev->color = LED_COLOR_ID_MULTI;
>>  	priv->mc_cdev.num_colors = count;
>>  
>> -- 
>> 2.39.2
>>
> 

-- 
Martijn de Gouw
Designer
Prodrive Technologies B.V.
Mobile: +31 63 17 76 161
Phone:  +31 40 26 76 200

[PATCH] leds: group-multicolor: Add support for initial value.

[Martijn de Gouw]

When the driver is loaded, it turned off all LEDs in the group. This
patch changes the driver to take over existing LED states and set
the brighness and intensity in the group accordingly.

Signed-off-by: Martijn de Gouw <martijn.de.gouw@prodrive-technologies.com>
---
 drivers/leds/rgb/leds-group-multicolor.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/leds/rgb/leds-group-multicolor.c b/drivers/leds/rgb/leds-group-multicolor.c
index 548c7dd63ba1e..ab46537697d76 100644
--- a/drivers/leds/rgb/leds-group-multicolor.c
+++ b/drivers/leds/rgb/leds-group-multicolor.c
@@ -69,6 +69,7 @@ static int leds_gmc_probe(struct platform_device *pdev)
 	struct mc_subled *subled;
 	struct leds_multicolor *priv;
 	unsigned int max_brightness = 0;
+	bool is_on = false;
 	int i, ret, count = 0, common_flags = 0;
 
 	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
@@ -96,6 +97,13 @@ static int leds_gmc_probe(struct platform_device *pdev)
 
 		max_brightness = max(max_brightness, led_cdev->max_brightness);
 
+		/*
+		 * If any LED is on, set brightness to the max brightness.
+		 * The actual brightness of the LED is set as intensity value.
+		 */
+		if (led_cdev->brightness)
+			is_on = true;
+
 		count++;
 	}
 
@@ -109,14 +117,16 @@ static int leds_gmc_probe(struct platform_device *pdev)
 
 		subled[i].color_index = led_cdev->color;
 
-		/* Configure the LED intensity to its maximum */
-		subled[i].intensity = max_brightness;
+		/* Configure the LED intensity to its current brightness */
+		subled[i].intensity = DIV_ROUND_CLOSEST(led_cdev->brightness * max_brightness,
+							led_cdev->max_brightness);
 	}
 
 	/* Initialise the multicolor's LED class device */
 	cdev = &priv->mc_cdev.led_cdev;
 	cdev->brightness_set_blocking = leds_gmc_set;
 	cdev->max_brightness = max_brightness;
+	cdev->brightness = is_on ? max_brightness : 0;
 	cdev->color = LED_COLOR_ID_MULTI;
 	priv->mc_cdev.num_colors = count;
 
-- 
2.39.2

Re: [PATCH] leds: group-multicolor: Add support for initial value.

[Lee Jones]

On Mon, 09 Feb 2026, Martijn de Gouw wrote:

> When the driver is loaded, it turned off all LEDs in the group. This
> patch changes the driver to take over existing LED states and set
> the brighness and intensity in the group accordingly.
> 
> Signed-off-by: Martijn de Gouw <martijn.de.gouw@prodrive-technologies.com>
> ---
>  drivers/leds/rgb/leds-group-multicolor.c | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)

You forgot to add a change log and a version to the subject line.

You also forgot to add Jean again.

Please resubmit.

> diff --git a/drivers/leds/rgb/leds-group-multicolor.c b/drivers/leds/rgb/leds-group-multicolor.c
> index 548c7dd63ba1e..ab46537697d76 100644
> --- a/drivers/leds/rgb/leds-group-multicolor.c
> +++ b/drivers/leds/rgb/leds-group-multicolor.c
> @@ -69,6 +69,7 @@ static int leds_gmc_probe(struct platform_device *pdev)
>  	struct mc_subled *subled;
>  	struct leds_multicolor *priv;
>  	unsigned int max_brightness = 0;
> +	bool is_on = false;
>  	int i, ret, count = 0, common_flags = 0;
>  
>  	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> @@ -96,6 +97,13 @@ static int leds_gmc_probe(struct platform_device *pdev)
>  
>  		max_brightness = max(max_brightness, led_cdev->max_brightness);
>  
> +		/*
> +		 * If any LED is on, set brightness to the max brightness.
> +		 * The actual brightness of the LED is set as intensity value.
> +		 */
> +		if (led_cdev->brightness)
> +			is_on = true;
> +
>  		count++;
>  	}
>  
> @@ -109,14 +117,16 @@ static int leds_gmc_probe(struct platform_device *pdev)
>  
>  		subled[i].color_index = led_cdev->color;
>  
> -		/* Configure the LED intensity to its maximum */
> -		subled[i].intensity = max_brightness;
> +		/* Configure the LED intensity to its current brightness */
> +		subled[i].intensity = DIV_ROUND_CLOSEST(led_cdev->brightness * max_brightness,
> +							led_cdev->max_brightness);
>  	}
>  
>  	/* Initialise the multicolor's LED class device */
>  	cdev = &priv->mc_cdev.led_cdev;
>  	cdev->brightness_set_blocking = leds_gmc_set;
>  	cdev->max_brightness = max_brightness;
> +	cdev->brightness = is_on ? max_brightness : 0;
>  	cdev->color = LED_COLOR_ID_MULTI;
>  	priv->mc_cdev.num_colors = count;
>  
> -- 
> 2.39.2
> 
> 

-- 
Lee Jones [李琼斯]

[PATCH] leds: group-multicolor: Add support for initial value.

[Martijn de Gouw]

When the driver is loaded, it turned off all LEDs in the group. This
patch changes the driver to take over existing LED states and set
the brighness and intensity in the group accordingly.

Signed-off-by: Martijn de Gouw <martijn.de.gouw@prodrive-technologies.com>
---
Changes in v3:
- Use is_on boolean instead of storing the max_brightness twice
- Link to v2: https://lore.kernel.org/linux-leds/20251124210521.2064660-1-martijn.de.gouw@prodrive-technologies.com/

Changes in v2:
- Fix multiline comments
- Improve comments
- Link to v1: https://lore.kernel.org/linux-leds/20251111204556.2803878-1-martijn.de.gouw@prodrive-technologies.com/

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

diff --git a/drivers/leds/rgb/leds-group-multicolor.c b/drivers/leds/rgb/leds-group-multicolor.c
index 548c7dd63ba1e..ab46537697d76 100644
--- a/drivers/leds/rgb/leds-group-multicolor.c
+++ b/drivers/leds/rgb/leds-group-multicolor.c
@@ -69,6 +69,7 @@ static int leds_gmc_probe(struct platform_device *pdev)
 	struct mc_subled *subled;
 	struct leds_multicolor *priv;
 	unsigned int max_brightness = 0;
+	bool is_on = false;
 	int i, ret, count = 0, common_flags = 0;
 
 	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
@@ -96,6 +97,13 @@ static int leds_gmc_probe(struct platform_device *pdev)
 
 		max_brightness = max(max_brightness, led_cdev->max_brightness);
 
+		/*
+		 * If any LED is on, set brightness to the max brightness.
+		 * The actual brightness of the LED is set as intensity value.
+		 */
+		if (led_cdev->brightness)
+			is_on = true;
+
 		count++;
 	}
 
@@ -109,14 +117,16 @@ static int leds_gmc_probe(struct platform_device *pdev)
 
 		subled[i].color_index = led_cdev->color;
 
-		/* Configure the LED intensity to its maximum */
-		subled[i].intensity = max_brightness;
+		/* Configure the LED intensity to its current brightness */
+		subled[i].intensity = DIV_ROUND_CLOSEST(led_cdev->brightness * max_brightness,
+							led_cdev->max_brightness);
 	}
 
 	/* Initialise the multicolor's LED class device */
 	cdev = &priv->mc_cdev.led_cdev;
 	cdev->brightness_set_blocking = leds_gmc_set;
 	cdev->max_brightness = max_brightness;
+	cdev->brightness = is_on ? max_brightness : 0;
 	cdev->color = LED_COLOR_ID_MULTI;
 	priv->mc_cdev.num_colors = count;
 
-- 
2.39.2

Re: [PATCH] leds: group-multicolor: Add support for initial value.

[Lee Jones]

On Mon, 16 Mar 2026, Martijn de Gouw wrote:

> When the driver is loaded, it turned off all LEDs in the group. This
> patch changes the driver to take over existing LED states and set
> the brighness and intensity in the group accordingly.
> 
> Signed-off-by: Martijn de Gouw <martijn.de.gouw@prodrive-technologies.com>
> ---
> Changes in v3:
> - Use is_on boolean instead of storing the max_brightness twice
> - Link to v2: https://lore.kernel.org/linux-leds/20251124210521.2064660-1-martijn.de.gouw@prodrive-technologies.com/
> 
> Changes in v2:
> - Fix multiline comments
> - Improve comments
> - Link to v1: https://lore.kernel.org/linux-leds/20251111204556.2803878-1-martijn.de.gouw@prodrive-technologies.com/
> 
> ---
>  drivers/leds/rgb/leds-group-multicolor.c | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/leds/rgb/leds-group-multicolor.c b/drivers/leds/rgb/leds-group-multicolor.c
> index 548c7dd63ba1e..ab46537697d76 100644
> --- a/drivers/leds/rgb/leds-group-multicolor.c
> +++ b/drivers/leds/rgb/leds-group-multicolor.c
> @@ -69,6 +69,7 @@ static int leds_gmc_probe(struct platform_device *pdev)
>  	struct mc_subled *subled;
>  	struct leds_multicolor *priv;
>  	unsigned int max_brightness = 0;
> +	bool is_on = false;
>  	int i, ret, count = 0, common_flags = 0;
>  
>  	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> @@ -96,6 +97,13 @@ static int leds_gmc_probe(struct platform_device *pdev)
>  
>  		max_brightness = max(max_brightness, led_cdev->max_brightness);
>  
> +		/*
> +		 * If any LED is on, set brightness to the max brightness.
> +		 * The actual brightness of the LED is set as intensity value.
> +		 */
> +		if (led_cdev->brightness)
> +			is_on = true;
> +
>  		count++;
>  	}
>  
> @@ -109,14 +117,16 @@ static int leds_gmc_probe(struct platform_device *pdev)
>  
>  		subled[i].color_index = led_cdev->color;
>  
> -		/* Configure the LED intensity to its maximum */
> -		subled[i].intensity = max_brightness;
> +		/* Configure the LED intensity to its current brightness */
> +		subled[i].intensity = DIV_ROUND_CLOSEST(led_cdev->brightness * max_brightness,
> +							led_cdev->max_brightness);

There are a couple of potential issues on this line.

'led_cdev->max_brightness' could theoretically be zero, which would
cause a division-by-zero fault.  While the core tries to prevent this, a
defensive check might be worth while.

The multiplication 'led_cdev->brightness * max_brightness' could overflow
if both values are large.  It would be safer to cast the multiplication
to u64, like: (u64)led_cdev->brightness

Finally, and this applies to the read in the first loop as well, the
brightness field of a led_classdev is protected by 'led_sem'. To avoid a
race condition with userspace (e.g., writing to the brightness via
sysfs), we should acquire this lock before reading 'led_cdev->brightness'.

>  	}
>  
>  	/* Initialise the multicolor's LED class device */
>  	cdev = &priv->mc_cdev.led_cdev;
>  	cdev->brightness_set_blocking = leds_gmc_set;
>  	cdev->max_brightness = max_brightness;
> +	cdev->brightness = is_on ? max_brightness : 0;
>  	cdev->color = LED_COLOR_ID_MULTI;
>  	priv->mc_cdev.num_colors = count;
>  
> -- 
> 2.39.2
> 
> 

-- 
Lee Jones [李琼斯]