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

[Lee Jones <lee@kernel.org>]

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