[PATCH RFC v2 0/2] leds-lp50xx: Support reg to set multi_index.

[PATCH RFC v2 0/2] leds-lp50xx: Support reg to set multi_index.

[Johan Adolfsson]

Since devicetree nodes are (sometimes?) processed in reverse order,
support reg as the actual multi_index index so yo get well defined
color order presented in the multi_index file.
Not sure if reusing reg for this is the correct way or if another
property such as "multi_index" or similar should be used instead.
Looks like reg is used for similar things at least.
Or should the whole "reverse the devicetree" problem be fixed instead?
Update bindings to match implementation, and add description for the
reg property.

Signed-off-by: Johan Adolfsson <johan.adolfsson@axis.com>
---
Changes in v2:
- Avoid duplicate assignment. dev_err and return -EINVAL on error.
- Update bindings doc.
- Link to v1: https://lore.kernel.org/r/20250506-led-fix-v1-1-56a39b55a7fc@axis.com

---
Johan Adolfsson (2):
      leds: leds-lp50xx: Handle reg to get correct multi_index
      dt-bindings: leds: lp50xx: Document child reg, fix example

 Documentation/devicetree/bindings/leds/leds-lp50xx.yaml | 11 ++++++++---
 drivers/leds/leds-lp50xx.c                              |  8 +++++++-
 2 files changed, 15 insertions(+), 4 deletions(-)
---
base-commit: 38fec10eb60d687e30c8c6b5420d86e8149f7557
change-id: 20250225-led-fix-444fb544584a

Best regards,
-- 
Johan Adolfsson <johan.adolfsson@axis.com>

[PATCH RFC v2 1/2] leds: leds-lp50xx: Handle reg to get correct multi_index

[Johan Adolfsson]

mc_subled used for multi_index needs well defined array indexes,
to guarantee the desired result, optionally use reg for that.

If devicetree child nodes is processed in random or reverse order
you may end up with multi_index "blue green red" instead of the expected
"red green blue".
If user space apps uses multi_index to deduce how to control the leds
they would most likely be broken without this patch if devicetree
processing is reversed (which it appears to be).

arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-fuji.dts has reg set
but I don't see how it can have worked without this change.

If reg is not set, the previous behavior is kept, index will be in
the order nodes are processed.
If reg is out of range, an error is returned.
reg within led child nodes starts with 0, to map to the iout in each bank.

Signed-off-by: Johan Adolfsson <johan.adolfsson@axis.com>
---
 drivers/leds/leds-lp50xx.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/leds/leds-lp50xx.c b/drivers/leds/leds-lp50xx.c
index 02cb1565a9fb..8067aaa916bf 100644
--- a/drivers/leds/leds-lp50xx.c
+++ b/drivers/leds/leds-lp50xx.c
@@ -476,6 +476,7 @@ static int lp50xx_probe_dt(struct lp50xx *priv)
 			return -ENOMEM;
 
 		fwnode_for_each_child_node(child, led_node) {
+			int multi_index = num_colors;
 			ret = fwnode_property_read_u32(led_node, "color",
 						       &color_id);
 			if (ret) {
@@ -483,8 +484,13 @@ static int lp50xx_probe_dt(struct lp50xx *priv)
 				dev_err(priv->dev, "Cannot read color\n");
 				return ret;
 			}
+			ret = fwnode_property_read_u32(led_node, "reg", &multi_index);
+			if (ret == 0 && multi_index >= LP50XX_LEDS_PER_MODULE) {
+				dev_err(priv->dev, "reg %i out of range\n", multi_index);
+				return -EINVAL;
+			}
 
-			mc_led_info[num_colors].color_index = color_id;
+			mc_led_info[multi_index].color_index = color_id;
 			num_colors++;
 		}
 

-- 
2.30.2

[PATCH RFC v2 2/2] dt-bindings: leds: lp50xx: Document child reg, fix example

[Johan Adolfsson]

The led child reg node is the index within the bank, document that
and update the example accordingly.

Signed-off-by: Johan Adolfsson <johan.adolfsson@axis.com>
---
 Documentation/devicetree/bindings/leds/leds-lp50xx.yaml | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/leds/leds-lp50xx.yaml b/Documentation/devicetree/bindings/leds/leds-lp50xx.yaml
index 402c25424525..8ce327d00c2d 100644
--- a/Documentation/devicetree/bindings/leds/leds-lp50xx.yaml
+++ b/Documentation/devicetree/bindings/leds/leds-lp50xx.yaml
@@ -82,6 +82,11 @@ patternProperties:
         properties:
           reg:
             maxItems: 1
+            description:
+              This property denotes the index within the LED bank, 0, 1 or 2.
+              The value is actually optional for backward compatibility reasons
+              but is highly recommended to handle reversed devicetree
+              processing properly.
 
         required:
           - reg
@@ -139,17 +144,17 @@ examples:
                 function = LED_FUNCTION_STANDBY;
 
                 led@3 {
-                    reg = <0x3>;
+                    reg = <0x0>;
                     color = <LED_COLOR_ID_RED>;
                 };
 
                 led@4 {
-                    reg = <0x4>;
+                    reg = <0x1>;
                     color = <LED_COLOR_ID_GREEN>;
                 };
 
                 led@5 {
-                    reg = <0x5>;
+                    reg = <0x2>;
                     color = <LED_COLOR_ID_BLUE>;
                 };
             };

-- 
2.30.2

Re: [PATCH RFC v2 2/2] dt-bindings: leds: lp50xx: Document child reg, fix example

[Krzysztof Kozlowski]

On 22/05/2025 16:14, Johan Adolfsson wrote:
> The led child reg node is the index within the bank, document that
> and update the example accordingly.
> 
> Signed-off-by: Johan Adolfsson <johan.adolfsson@axis.com>


<form letter>
Please use scripts/get_maintainers.pl to get a list of necessary people
and lists to CC. It might happen, that command when run on an older
kernel, gives you outdated entries. Therefore please be sure you base
your patches on recent Linux kernel.

Tools like b4 or scripts/get_maintainer.pl provide you proper list of
people, so fix your workflow. Tools might also fail if you work on some
ancient tree (don't, instead use mainline) or work on fork of kernel
(don't, instead use mainline). Just use b4 and everything should be
fine, although remember about `b4 prep --auto-to-cc` if you added new
patches to the patchset.

You missed at least devicetree list (maybe more), so this won't be
tested by automated tooling. Performing review on untested code might be
a waste of time.

Please kindly resend and include all necessary To/Cc entries.
</form letter>


> ---
>  Documentation/devicetree/bindings/leds/leds-lp50xx.yaml | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/leds/leds-lp50xx.yaml b/Documentation/devicetree/bindings/leds/leds-lp50xx.yaml
> index 402c25424525..8ce327d00c2d 100644
> --- a/Documentation/devicetree/bindings/leds/leds-lp50xx.yaml
> +++ b/Documentation/devicetree/bindings/leds/leds-lp50xx.yaml
> @@ -82,6 +82,11 @@ patternProperties:
>          properties:
>            reg:
>              maxItems: 1
> +            description:
> +              This property denotes the index within the LED bank, 0, 1 or 2.

Do not open code constraints. You probably wanted minimum/maximum
instead of above syntax.

> +              The value is actually optional for backward compatibility reasons

No, it is not optional - see schema.

> +              but is highly recommended to handle reversed devicetree
> +              processing properly.
>  
>          required:
>            - reg
> @@ -139,17 +144,17 @@ examples:
>                  function = LED_FUNCTION_STANDBY;
>  
>                  led@3 {
> -                    reg = <0x3>;
> +                    reg = <0x0>;

Mismatched unit address.


Best regards,
Krzysztof