[PATCH V4] leds: rgb: leds-group-multicolor: Implement default-intensity

[PATCH V4] leds: rgb: leds-group-multicolor: Implement default-intensity

[Stefan Wahren]

Currently it's not possible to specify the initial color of a LED
multicolor group during boot. So implement the default-intensity property
similar to the leds-pwm-multicolor driver. In case the property is
missing, the old behavior is kept.

Signed-off-by: Stefan Wahren <wahrenst@gmx.net>
Reviewed-by: Jonas Rebmann <jre@pengutronix.de>
---

Changes in V4:
- use device_property_read_u32 instead of fwnode_property_read_u32
- simplify if statement as suggested by Lee
- drop obvious comment

Changes in V3:
- drop unnecessary patch for leds-group-multicolor.yaml
- add Jonas' RB

Changes in V2:
- adapt to approach (incl. error behavior) by Jonas Rebmann [2]
- address comments by Lee which still apply

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

diff --git a/drivers/leds/rgb/leds-group-multicolor.c b/drivers/leds/rgb/leds-group-multicolor.c
index 548c7dd63ba1..f278d2a2bf5c 100644
--- a/drivers/leds/rgb/leds-group-multicolor.c
+++ b/drivers/leds/rgb/leds-group-multicolor.c
@@ -109,8 +109,10 @@ 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;
+		ret = device_property_read_u32(led_cdev->dev, "default-intensity",
+					       &subled[i].intensity);
+		if (ret || subled[i].intensity > max_brightness)
+			subled[i].intensity = max_brightness;
 	}
 
 	/* Initialise the multicolor's LED class device */
-- 
2.43.0

Re: [PATCH V4] leds: rgb: leds-group-multicolor: Implement default-intensity

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The `default-intensity` property read from individual monochromatic LED nodes is applied to the multicolor group without rescaling, causing incorrect brightness calculations.
--

--- Patch [1]: [PATCH V4] leds: rgb: leds-group-multicolor: Implement default-intensity ---
commit 13399f1aaceaf09afa7e252e35000d7d61ab1925
Author: Stefan Wahren <wahrenst@gmx.net>

leds: rgb: leds-group-multicolor: Implement default-intensity

This commit introduces support for the default-intensity property during
boot for LED multicolor groups. If the property is missing, the code falls
back to the previous behavior of configuring the sub-LED intensity to its
maximum.

> diff --git a/drivers/leds/rgb/leds-group-multicolor.c b/drivers/leds/rgb/leds-group-multicolor.c
> --- a/drivers/leds/rgb/leds-group-multicolor.c
> +++ b/drivers/leds/rgb/leds-group-multicolor.c
> @@ -108,8 +108,10 @@ 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;
> +		ret = device_property_read_u32(led_cdev->dev, "default-intensity",
> +					       &subled[i].intensity);
> +		if (ret || subled[i].intensity > max_brightness)
> +			subled[i].intensity = max_brightness;

[Severity: Medium]
Does this logic properly scale the intensity when the individual LEDs in the
group have different maximum brightness values?

If a multicolor group consists of LEDs with vastly different maximum brightness
capabilities (like a GPIO LED maxing at 1 and a PWM LED maxing at 255), the
overall group max_brightness will evaluate to 255.

When reading the default-intensity from the device tree, the value is typically
specified on the scale of the individual sub-LED's max brightness. If this
value is assigned directly to subled[i].intensity without being scaled up to
the group's max_brightness, the sub-LED might be initialized with only a tiny
fraction of its intended intensity, often resulting in the LED being completely
unlit instead of at the requested intensity.

>  	}
>  
>  	/* Initialise the multicolor's LED class device */

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260813091428.9072-1-wahrenst@gmx.net?part=1

Re: [PATCH V4] leds: rgb: leds-group-multicolor: Implement default-intensity

[Lee Jones]

On Thu, 13 Aug 2026, sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] The `default-intensity` property read from individual monochromatic LED nodes is applied to the multicolor group without rescaling, causing incorrect brightness calculations.

Please review, explain or fix the Sashiko review(s).

If this is a set, this request includes the other patches too.

> --
> 
> --- Patch [1]: [PATCH V4] leds: rgb: leds-group-multicolor: Implement default-intensity ---
> commit 13399f1aaceaf09afa7e252e35000d7d61ab1925
> Author: Stefan Wahren <wahrenst@gmx.net>
> 
> leds: rgb: leds-group-multicolor: Implement default-intensity
> 
> This commit introduces support for the default-intensity property during
> boot for LED multicolor groups. If the property is missing, the code falls
> back to the previous behavior of configuring the sub-LED intensity to its
> maximum.
> 
> > diff --git a/drivers/leds/rgb/leds-group-multicolor.c b/drivers/leds/rgb/leds-group-multicolor.c
> > --- a/drivers/leds/rgb/leds-group-multicolor.c
> > +++ b/drivers/leds/rgb/leds-group-multicolor.c
> > @@ -108,8 +108,10 @@ 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;
> > +		ret = device_property_read_u32(led_cdev->dev, "default-intensity",
> > +					       &subled[i].intensity);
> > +		if (ret || subled[i].intensity > max_brightness)
> > +			subled[i].intensity = max_brightness;
> 
> [Severity: Medium]
> Does this logic properly scale the intensity when the individual LEDs in the
> group have different maximum brightness values?
> 
> If a multicolor group consists of LEDs with vastly different maximum brightness
> capabilities (like a GPIO LED maxing at 1 and a PWM LED maxing at 255), the
> overall group max_brightness will evaluate to 255.
> 
> When reading the default-intensity from the device tree, the value is typically
> specified on the scale of the individual sub-LED's max brightness. If this
> value is assigned directly to subled[i].intensity without being scaled up to
> the group's max_brightness, the sub-LED might be initialized with only a tiny
> fraction of its intended intensity, often resulting in the LED being completely
> unlit instead of at the requested intensity.
> 
> >  	}
> >  
> >  	/* Initialise the multicolor's LED class device */
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260813091428.9072-1-wahrenst@gmx.net?part=1

-- 
Lee Jones

Re: [PATCH V4] leds: rgb: leds-group-multicolor: Implement default-intensity

[Stefan Wahren]

Hi,

Am 13.08.26 um 11:53 schrieb Lee Jones:
> On Thu, 13 Aug 2026, sashiko-bot@kernel.org wrote:
>
>> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
>> - [Medium] The `default-intensity` property read from individual monochromatic LED nodes is applied to the multicolor group without rescaling, causing incorrect brightness calculations.
> Please review, explain or fix the Sashiko review(s).
>
> If this is a set, this request includes the other patches too.
Sorry, I don't get the problem with "incorrect brightness calculation", 
I tought the intensity setting is independent from the brightness. Btw 
the intensity is applied to the monochromatic LED not to the group 
itself. hallucination?

Does mixing PWM and GPIO LED in a RGB multicolor group make sense at all?

>
>> --
>>
>> --- Patch [1]: [PATCH V4] leds: rgb: leds-group-multicolor: Implement default-intensity ---
>> commit 13399f1aaceaf09afa7e252e35000d7d61ab1925
>> Author: Stefan Wahren <wahrenst@gmx.net>
>>
>> leds: rgb: leds-group-multicolor: Implement default-intensity
>>
>> This commit introduces support for the default-intensity property during
>> boot for LED multicolor groups. If the property is missing, the code falls
>> back to the previous behavior of configuring the sub-LED intensity to its
>> maximum.
>>
>>> diff --git a/drivers/leds/rgb/leds-group-multicolor.c b/drivers/leds/rgb/leds-group-multicolor.c
>>> --- a/drivers/leds/rgb/leds-group-multicolor.c
>>> +++ b/drivers/leds/rgb/leds-group-multicolor.c
>>> @@ -108,8 +108,10 @@ 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;
>>> +		ret = device_property_read_u32(led_cdev->dev, "default-intensity",
>>> +					       &subled[i].intensity);
>>> +		if (ret || subled[i].intensity > max_brightness)
>>> +			subled[i].intensity = max_brightness;
>> [Severity: Medium]
>> Does this logic properly scale the intensity when the individual LEDs in the
>> group have different maximum brightness values?
>>
>> If a multicolor group consists of LEDs with vastly different maximum brightness
>> capabilities (like a GPIO LED maxing at 1 and a PWM LED maxing at 255), the
>> overall group max_brightness will evaluate to 255.
>>
>> When reading the default-intensity from the device tree, the value is typically
>> specified on the scale of the individual sub-LED's max brightness. If this
>> value is assigned directly to subled[i].intensity without being scaled up to
>> the group's max_brightness, the sub-LED might be initialized with only a tiny
>> fraction of its intended intensity, often resulting in the LED being completely
>> unlit instead of at the requested intensity.
>>
>>>   	}
>>>   
>>>   	/* Initialise the multicolor's LED class device */
>> -- 
>> Sashiko AI review · https://sashiko.dev/#/patchset/20260813091428.9072-1-wahrenst@gmx.net?part=1

Re: [PATCH V4] leds: rgb: leds-group-multicolor: Implement default-intensity

[Lee Jones]

On Thu, 13 Aug 2026, Stefan Wahren wrote:

> Currently it's not possible to specify the initial color of a LED
> multicolor group during boot. So implement the default-intensity property
> similar to the leds-pwm-multicolor driver. In case the property is
> missing, the old behavior is kept.
> 
> Signed-off-by: Stefan Wahren <wahrenst@gmx.net>
> Reviewed-by: Jonas Rebmann <jre@pengutronix.de>
> ---
> 
> Changes in V4:
> - use device_property_read_u32 instead of fwnode_property_read_u32
> - simplify if statement as suggested by Lee
> - drop obvious comment
> 
> Changes in V3:
> - drop unnecessary patch for leds-group-multicolor.yaml
> - add Jonas' RB
> 
> Changes in V2:
> - adapt to approach (incl. error behavior) by Jonas Rebmann [2]
> - address comments by Lee which still apply
> 
>  drivers/leds/rgb/leds-group-multicolor.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/leds/rgb/leds-group-multicolor.c b/drivers/leds/rgb/leds-group-multicolor.c
> index 548c7dd63ba1..f278d2a2bf5c 100644
> --- a/drivers/leds/rgb/leds-group-multicolor.c
> +++ b/drivers/leds/rgb/leds-group-multicolor.c
> @@ -109,8 +109,10 @@ 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;
> +		ret = device_property_read_u32(led_cdev->dev, "default-intensity",
> +					       &subled[i].intensity);

<gemini>

Should we avoid clobbering the function-wide 'ret' variable with an ignored
error from 'device_property_read_u32()'? It might be safer to use a local
variable inside the loop to prevent any future bugs if 'ret' is assumed to
be zero later in the function.

Also, should we use a local 'u32' variable to read the property instead of
passing the address of 'subled[i].intensity' directly? Since 'intensity' is
defined as 'unsigned int', using a temporary 'u32' variable would be
type-safe and avoid potential compiler warnings. If we do this, we should
declare the 'u32' variable at the start of the block and assign it with
the function call on a separate line.

</gemini>

> +		if (ret || subled[i].intensity > max_brightness)
> +			subled[i].intensity = max_brightness;
>  	}
>  
>  	/* Initialise the multicolor's LED class device */
> -- 
> 2.43.0
> 

-- 
Lee Jones

Re: [PATCH V4] leds: rgb: leds-group-multicolor: Implement default-intensity

[Lee Jones]

On Thu, 13 Aug 2026, Stefan Wahren wrote:

> Hi,
> 
> Am 13.08.26 um 11:53 schrieb Lee Jones:
> > On Thu, 13 Aug 2026, sashiko-bot@kernel.org wrote:
> > 
> > > Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> > > - [Medium] The `default-intensity` property read from individual monochromatic LED nodes is applied to the multicolor group without rescaling, causing incorrect brightness calculations.
> > Please review, explain or fix the Sashiko review(s).
> > 
> > If this is a set, this request includes the other patches too.
> Sorry, I don't get the problem with "incorrect brightness calculation", I
> tought the intensity setting is independent from the brightness. Btw the
> intensity is applied to the monochromatic LED not to the group itself.
> hallucination?
> 
> Does mixing PWM and GPIO LED in a RGB multicolor group make sense at all?

Couldn't honestly tell you without digging in myself.

I did just provide some concerns from Gemini though.  PGTAL.

> > > --
> > > 
> > > --- Patch [1]: [PATCH V4] leds: rgb: leds-group-multicolor: Implement default-intensity ---
> > > commit 13399f1aaceaf09afa7e252e35000d7d61ab1925
> > > Author: Stefan Wahren <wahrenst@gmx.net>
> > > 
> > > leds: rgb: leds-group-multicolor: Implement default-intensity
> > > 
> > > This commit introduces support for the default-intensity property during
> > > boot for LED multicolor groups. If the property is missing, the code falls
> > > back to the previous behavior of configuring the sub-LED intensity to its
> > > maximum.
> > > 
> > > > diff --git a/drivers/leds/rgb/leds-group-multicolor.c b/drivers/leds/rgb/leds-group-multicolor.c
> > > > --- a/drivers/leds/rgb/leds-group-multicolor.c
> > > > +++ b/drivers/leds/rgb/leds-group-multicolor.c
> > > > @@ -108,8 +108,10 @@ 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;
> > > > +		ret = device_property_read_u32(led_cdev->dev, "default-intensity",
> > > > +					       &subled[i].intensity);
> > > > +		if (ret || subled[i].intensity > max_brightness)
> > > > +			subled[i].intensity = max_brightness;
> > > [Severity: Medium]
> > > Does this logic properly scale the intensity when the individual LEDs in the
> > > group have different maximum brightness values?
> > > 
> > > If a multicolor group consists of LEDs with vastly different maximum brightness
> > > capabilities (like a GPIO LED maxing at 1 and a PWM LED maxing at 255), the
> > > overall group max_brightness will evaluate to 255.
> > > 
> > > When reading the default-intensity from the device tree, the value is typically
> > > specified on the scale of the individual sub-LED's max brightness. If this
> > > value is assigned directly to subled[i].intensity without being scaled up to
> > > the group's max_brightness, the sub-LED might be initialized with only a tiny
> > > fraction of its intended intensity, often resulting in the LED being completely
> > > unlit instead of at the requested intensity.
> > > 
> > > >   	}
> > > >   	/* Initialise the multicolor's LED class device */
> > > -- 
> > > Sashiko AI review · https://sashiko.dev/#/patchset/20260813091428.9072-1-wahrenst@gmx.net?part=1
> 
> 

-- 
Lee Jones

Re: [PATCH V4] leds: rgb: leds-group-multicolor: Implement default-intensity

[Lee Jones]

On Thu, 13 Aug 2026, Stefan Wahren wrote:

> Hi,
> 
> Am 13.08.26 um 11:53 schrieb Lee Jones:
> > On Thu, 13 Aug 2026, sashiko-bot@kernel.org wrote:
> > 
> > > Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> > > - [Medium] The `default-intensity` property read from individual monochromatic LED nodes is applied to the multicolor group without rescaling, causing incorrect brightness calculations.
> > Please review, explain or fix the Sashiko review(s).
> > 
> > If this is a set, this request includes the other patches too.
> Sorry, I don't get the problem with "incorrect brightness calculation", I
> tought the intensity setting is independent from the brightness. Btw the
> intensity is applied to the monochromatic LED not to the group itself.
> hallucination?
> 
> Does mixing PWM and GPIO LED in a RGB multicolor group make sense at all?

I asked my little helper for you:

<gemini>

While mixing a GPIO LED (essentially binary) and a PWM LED in a single
multicolor group is technically possible, it is indeed highly unusual and
unlikely to produce a desirable colour-mixing effect in practice.

Regarding the Sashiko review, there is a valid point hidden in the noise.
The multicolor framework uses 'subled[i].intensity' as a relative ratio
against the overall group's 'max_brightness'. If a sub-LED's native
intensity scale differs from the group's scale, assigning the raw value
from the 'default-intensity' property directly without scaling it could
lead to unexpected dimness or brightness. Should we look into whether we
need to normalise this value against 'max_brightness' to maintain
consistency?

</gemini>

Hope it helps.

> > 
> > > --
> > > 
> > > --- Patch [1]: [PATCH V4] leds: rgb: leds-group-multicolor: Implement default-intensity ---
> > > commit 13399f1aaceaf09afa7e252e35000d7d61ab1925
> > > Author: Stefan Wahren <wahrenst@gmx.net>
> > > 
> > > leds: rgb: leds-group-multicolor: Implement default-intensity
> > > 
> > > This commit introduces support for the default-intensity property during
> > > boot for LED multicolor groups. If the property is missing, the code falls
> > > back to the previous behavior of configuring the sub-LED intensity to its
> > > maximum.
> > > 
> > > > diff --git a/drivers/leds/rgb/leds-group-multicolor.c b/drivers/leds/rgb/leds-group-multicolor.c
> > > > --- a/drivers/leds/rgb/leds-group-multicolor.c
> > > > +++ b/drivers/leds/rgb/leds-group-multicolor.c
> > > > @@ -108,8 +108,10 @@ 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;
> > > > +		ret = device_property_read_u32(led_cdev->dev, "default-intensity",
> 
> Should we verify if 'led_cdev->dev' is the correct device to query here?
> Normally, 'led_cdev->dev' is the class device and might not have the fwnode
> associated directly in the same way as the underlying platform or fwnode
> device. Would using 'led_cdev->dev->parent' or querying the fwnode directly
> be safer?
> 
> > > > +					       &subled[i].intensity);
> > > > +		if (ret || subled[i].intensity > max_brightness)
> > > > +			subled[i].intensity = max_brightness;
> > > [Severity: Medium]
> > > Does this logic properly scale the intensity when the individual LEDs in the
> > > group have different maximum brightness values?
> > > 
> > > If a multicolor group consists of LEDs with vastly different maximum brightness
> > > capabilities (like a GPIO LED maxing at 1 and a PWM LED maxing at 255), the
> > > overall group max_brightness will evaluate to 255.
> > > 
> > > When reading the default-intensity from the device tree, the value is typically
> > > specified on the scale of the individual sub-LED's max brightness. If this
> > > value is assigned directly to subled[i].intensity without being scaled up to
> > > the group's max_brightness, the sub-LED might be initialized with only a tiny
> > > fraction of its intended intensity, often resulting in the LED being completely
> > > unlit instead of at the requested intensity.
> > > 
> > > >   	}
> > > >   	/* Initialise the multicolor's LED class device */
> > > -- 
> > > Sashiko AI review · https://sashiko.dev/#/patchset/20260813091428.9072-1-wahrenst@gmx.net?part=1
> 
> 

-- 
Lee Jones