All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH V4] leds: rgb: leds-group-multicolor: Implement default-intensity
@ 2026-08-13  9:14 Stefan Wahren
  2026-08-13  9:25 ` sashiko-bot
  0 siblings, 1 reply; 4+ messages in thread
From: Stefan Wahren @ 2026-08-13  9:14 UTC (permalink / raw)
  To: Lee Jones, Pavel Machek, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Jean-Jacques Hiblot
  Cc: Jonas Rebmann, linux-leds, devicetree, linux-kernel,
	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


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH V4] leds: rgb: leds-group-multicolor: Implement default-intensity
  2026-08-13  9:14 [PATCH V4] leds: rgb: leds-group-multicolor: Implement default-intensity Stefan Wahren
@ 2026-08-13  9:25 ` sashiko-bot
  2026-08-13  9:53   ` Lee Jones
  0 siblings, 1 reply; 4+ messages in thread
From: sashiko-bot @ 2026-08-13  9:25 UTC (permalink / raw)
  To: Stefan Wahren; +Cc: linux-leds, lee, devicetree, robh, conor+dt

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH V4] leds: rgb: leds-group-multicolor: Implement default-intensity
  2026-08-13  9:25 ` sashiko-bot
@ 2026-08-13  9:53   ` Lee Jones
  2026-08-13 10:50     ` Stefan Wahren
  0 siblings, 1 reply; 4+ messages in thread
From: Lee Jones @ 2026-08-13  9:53 UTC (permalink / raw)
  To: sashiko-reviews; +Cc: Stefan Wahren, linux-leds, devicetree, robh, conor+dt

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH V4] leds: rgb: leds-group-multicolor: Implement default-intensity
  2026-08-13  9:53   ` Lee Jones
@ 2026-08-13 10:50     ` Stefan Wahren
  0 siblings, 0 replies; 4+ messages in thread
From: Stefan Wahren @ 2026-08-13 10:50 UTC (permalink / raw)
  To: Lee Jones, sashiko-reviews
  Cc: linux-leds, devicetree, robh, conor+dt, Jonas Rebmann

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


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-08-13 10:50 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-13  9:14 [PATCH V4] leds: rgb: leds-group-multicolor: Implement default-intensity Stefan Wahren
2026-08-13  9:25 ` sashiko-bot
2026-08-13  9:53   ` Lee Jones
2026-08-13 10:50     ` Stefan Wahren

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.