Re: [PATCH RFC 2/2] leds: rgb: leds-group-multicolor: Implement default-intensity

[Lee Jones <lee@kernel.org>]

On Thu, 09 Jul 2026, Stefan Wahren wrote:

> Until now the driver initialized all LEDs with maximum intensity.
> This isn't useful for LEDs, which needs to be initialized via DT.
> So introduce a new DT property to define the default intensity of all
> indiviual LEDs. In case the property is missing, the old behavior
> is kept.
> 
> Additionally this also works with triggers. So it should be possible
> to blink with a specific RGB color.
> 
> Signed-off-by: Stefan Wahren <wahrenst@gmx.net>
> ---
>  drivers/leds/rgb/leds-group-multicolor.c | 21 +++++++++++++++++++--
>  1 file changed, 19 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/leds/rgb/leds-group-multicolor.c b/drivers/leds/rgb/leds-group-multicolor.c
> index 548c7dd63ba1..19913a43b8a1 100644
> --- a/drivers/leds/rgb/leds-group-multicolor.c
> +++ b/drivers/leds/rgb/leds-group-multicolor.c
> @@ -17,6 +17,7 @@
>  #include <linux/math.h>
>  #include <linux/module.h>
>  #include <linux/mod_devicetable.h>
> +#include <linux/of.h>
>  #include <linux/platform_device.h>
>  #include <linux/property.h>
>  
> @@ -106,11 +107,27 @@ static int leds_gmc_probe(struct platform_device *pdev)
>  
>  	for (i = 0; i < count; i++) {
>  		struct led_classdev *led_cdev = priv->monochromatics[i];
> +		u32 intensity;
>  
>  		subled[i].color_index = led_cdev->color;
>  
> -		/* Configure the LED intensity to its maximum */
> -		subled[i].intensity = max_brightness;
> +		ret = of_property_read_u32_index(pdev->dev.of_node, "default-intensity",
> +						 i, &intensity);

What about using the firmware agnostic 'device_property_read_u32_array()'
before the loop to save some cycles?

> +		if (ret) {
> +			if (ret != -EINVAL && ret != -ENOSYS) {
> +				return dev_err_probe(dev, ret,
> +						     "Unable to get default-intensity[%d]\n", i);
> +			}

Deserves a comment I think.

> +			subled[i].intensity = max_brightness;
> +		} else if (intensity > max_brightness) {
> +			return dev_err_probe(dev, -EINVAL, "default-intensity[%d] is invalid\n",
> +					     i);

Nit: This is an ugly place to wrap.

> +		} else {
> +			subled[i].intensity = intensity;
> +		}
> +
> +		dev_dbg(dev, "subled[%d]: color_index: %u, intensity: %u\n",
> +			i, subled[i].color_index, subled[i].intensity);

How helpful is this outside of development?

-- 
Lee Jones