[PATCH V2 0/2] leds: rgb: leds-group-multicolor: Implement default-intensity

[PATCH V2 0/2] 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. This series is the replacement
for the recent series "leds: rgb: leds-group-multicolor: Introduce
default-intensity" [1].

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


[1] - https://lore.kernel.org/linux-leds/20260708224652.106632-1-wahrenst@gmx.net/
[2] - https://lore.kernel.org/linux-leds/20260605-multicolor-default-v2-0-ed07271df6b0@pengutronix.de/

Stefan Wahren (2):
  dt-bindings: leds-group-multicolor: Enable default-intensity
  leds: rgb: leds-group-multicolor: Implement default-intensity

 .../bindings/leds/leds-group-multicolor.yaml           |  2 ++
 drivers/leds/rgb/leds-group-multicolor.c               | 10 ++++++++--
 2 files changed, 10 insertions(+), 2 deletions(-)

-- 
2.43.0

[PATCH V2 1/2] dt-bindings: leds-group-multicolor: Enable default-intensity

[Stefan Wahren]

Since commit 2ebd1cb772b0 ("dt-bindings: leds: Add default-intensity
property") there is a property to specify the default color on
multicolor LEDs.

This is also helpful for leds-group-multicolor, so enable it.

Signed-off-by: Stefan Wahren <wahrenst@gmx.net>
---
 .../devicetree/bindings/leds/leds-group-multicolor.yaml         | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/leds/leds-group-multicolor.yaml b/Documentation/devicetree/bindings/leds/leds-group-multicolor.yaml
index 5c9cfa39396b..1f308cdb71c8 100644
--- a/Documentation/devicetree/bindings/leds/leds-group-multicolor.yaml
+++ b/Documentation/devicetree/bindings/leds/leds-group-multicolor.yaml
@@ -19,6 +19,8 @@ properties:
 
   leds: true
 
+  default-intensity: true
+
 required:
   - leds
 
-- 
2.43.0

Re: [PATCH V2 1/2] dt-bindings: leds-group-multicolor: Enable default-intensity

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The scalar `default-intensity` property is structurally misapplied to the top-level multi-color parent node instead of the individual monochromatic sub-LED nodes.
--

--- Patch [1]: [PATCH V2 1/2] dt-bindings: leds-group-multicolor: Enable default-intensity ---
commit bf29f5ac49b0a3cb58ee37092ceaf9652d2451d0
Author: Stefan Wahren <wahrenst@gmx.net>

dt-bindings: leds-group-multicolor: Enable default-intensity

This commit updates the leds-group-multicolor binding to allow the
default-intensity property on the top-level multicolor LED node.

> diff --git a/Documentation/devicetree/bindings/leds/leds-group-multicolor.yaml b/Documentation/devicetree/bindings/leds/leds-group-multicolor.yaml
> index 5c9cfa39396b0..1f308cdb71c86 100644
> --- a/Documentation/devicetree/bindings/leds/leds-group-multicolor.yaml
> +++ b/Documentation/devicetree/bindings/leds/leds-group-multicolor.yaml
> @@ -19,6 +19,8 @@ properties:
>  
>    leds: true
>  
> +  default-intensity: true
> +

[Severity: Medium]
Does this misapply the scalar default-intensity property to the top-level
multicolor parent node?

Because default-intensity is defined as a scalar uint32 type in common.yaml,
placing it on a multicolor aggregate node prevents it from conveying the
per-color-component intensity values required to define a default color mix.
If a developer attempts to define a mix using an array (e.g. <255 128 0>),
schema validation will fail due to the scalar type constraint.

Would it be more appropriate to apply this property exclusively to the
referenced monochromatic sub-LED nodes (e.g. gpio-leds), which already
inherit this property, rather than the parent group?

>  required:
>    - leds

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

[PATCH V2 2/2] 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>
---
 drivers/leds/rgb/leds-group-multicolor.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/leds/rgb/leds-group-multicolor.c b/drivers/leds/rgb/leds-group-multicolor.c
index 548c7dd63ba1..69b203ec89fa 100644
--- a/drivers/leds/rgb/leds-group-multicolor.c
+++ b/drivers/leds/rgb/leds-group-multicolor.c
@@ -109,8 +109,14 @@ 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 = fwnode_property_read_u32(led_cdev->dev->fwnode, "default-intensity",
+					       &subled[i].intensity);
+
+		/* In case default-intensity is missing, fallback to maximum */
+		if (ret)
+			subled[i].intensity = max_brightness;
+		else if (subled[i].intensity > max_brightness)
+			subled[i].intensity = max_brightness;
 	}
 
 	/* Initialise the multicolor's LED class device */
-- 
2.43.0

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Medium] Device Tree Binding & Implementation Mismatch for `default-intensity`

Pre-existing issues:
- [High] 32-bit unsigned integer overflow and potential division by zero in brightness scaling calculation
--

--- Patch [2]: [PATCH V2 2/2] leds: rgb: leds-group-multicolor: Implement default-intensity ---
commit 3299ece4693e9a064de7eb3b6ca548536d4ca2ef
Author: Stefan Wahren <wahrenst@gmx.net>

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

Currently it is not possible to specify the initial color of a LED multicolor
group during boot. This commit implements the default-intensity property
similar to the leds-pwm-multicolor driver, falling back to the maximum
intensity if the property is missing.

> diff --git a/drivers/leds/rgb/leds-group-multicolor.c b/drivers/leds/rgb/leds-group-multicolor.c
> index a707d51c6a4bd..f0e6c2a25a070 100644
> --- a/drivers/leds/rgb/leds-group-multicolor.c
> +++ b/drivers/leds/rgb/leds-group-multicolor.c
> @@ -108,8 +108,14 @@ 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 = fwnode_property_read_u32(led_cdev->dev->fwnode, "default-intensity",
> +					       &subled[i].intensity);

[Severity: Medium]
Will this parse the default-intensity property from the correct node?

Based on the device tree binding documentation for leds-group-multicolor, the
default-intensity property is expected to be located in the parent multicolor
group node.

However, in leds_gmc_probe(), the driver attempts to read it from the
monochromatic sub-LED fwnode instead:

    ret = fwnode_property_read_u32(led_cdev->dev->fwnode, "default-intensity",
                                   &subled[i].intensity);

Does this code ignore valid configurations where the property is placed in the
parent node according to the schema?

> +
> +		/* In case default-intensity is missing, fallback to maximum */
> +		if (ret)
> +			subled[i].intensity = max_brightness;
> +		else if (subled[i].intensity > max_brightness)
> +			subled[i].intensity = max_brightness;
>  	}
>  
>  	/* Initialise the multicolor's LED class device */

[Severity: High]
This is a pre-existing issue, but is there a potential integer overflow and
division by zero when scaling brightness in leds_gmc_set()?

When leds_gmc_set() calculates the brightness for the monochromatic sub-LEDs,
it uses 32-bit unsigned arithmetic:

drivers/leds/rgb/leds-group-multicolor.c:leds_gmc_set() {
    ...
    mono_brightness = DIV_ROUND_CLOSEST(brightness * intensity * mono_max_brightness,
                                        group_max_brightness * group_max_brightness);
    ...
}

If the underlying LEDs have high max_brightness values configured (for example,
65535 for a PWM period), the numerator can wrap around, resulting in a
calculated brightness of 0.

Furthermore, if group_max_brightness happens to be exactly 65536, the
denominator overflows to exactly 0, which would lead to a division by zero
panic during brightness adjustment.

Could this arithmetic be adjusted to prevent overflows with high brightness
values?

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