Skip to contentSkip to conversation

leds-lp50xx: Support reg to set multi_index.

PATCH Applied Johan Adolfsson 4 participants 6 messages Jun 17, 2025 – Jun 25, 2025

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

v7 cover leds-lp50xx: Support reg to set multi_index.

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.

---
Changes in v7:
- Skip leftover initialization of multi_index.
- Fix devcietree description and update commit message.
- Link to v6: https://lore.kernel.org/r/[email protected]

Changes in v6:
- Remove maxItems från devicetree binding
- Link to v5: https://lore.kernel.org/r/[email protected]

Changes in v5:
- Fail if reg is not set.
- Adjust devicetree schema, use items.
- Link to v4: https://lore.kernel.org/r/[email protected]

Changes in v4:
- Remove maxItems from devicetree schema, not compatible with minimum and maximum.
- Link to v3: https://lore.kernel.org/r/[email protected]

Changes in v3:
- Update To and Cc.
- Rephrase bindings descriptions, add constraints.
- Link to v2: https://lore.kernel.org/r/[email protected]

Changes in v2:
- Avoid duplicate assignment. dev_err and return -EINVAL on error.
- Update bindings doc.
- Link to v1: https://lore.kernel.org/r/[email protected]

---
Johan Adolfsson (2):

      leds: leds-lp50xx: Handle reg to get correct multi_index
      dt-bindings: leds: lp50xx: Document child reg, fix example
 .../devicetree/bindings/leds/leds-lp50xx.yaml         | 19 ++++++++++++-------
 drivers/leds/leds-lp50xx.c                            | 11 ++++++++++-
 2 files changed, 22 insertions(+), 8 deletions(-)

---
base-commit: 0ff41df1cb268fc69e703a08a57ee14ae967d0ca
change-id: 20250225-led-fix-444fb544584a

Best regards,

Signature · 1 line
Johan Adolfsson <[email protected]>
#1 lore raw

PATCH v7 1/2 leds: leds-lp50xx: Handle reg to get correct multi_index

mc_subled used for multi_index needs well defined array indexes, to guarantee the desired result, 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, an error is returned,
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.

---
 drivers/leds/leds-lp50xx.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)
drivers/leds/leds-lp50xx.c +10 -1
index 02cb1565a9fb..94f8ef6b482c 100644
@@ -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;
 			ret = fwnode_property_read_u32(led_node, "color",
 						       &color_id);
 			if (ret) {
@@ -483,8 +484,16 @@ 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) {
+				dev_err(priv->dev, "reg must be set\n");
+				return -EINVAL;
+			} else if (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++;
 		}
 
Signature · 1 line
2.30.2
Reviewed-by on v7 1/2
#2 lore raw

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

Hi Johan,

Thanks for the update.

Quoted 43 lines · Johan Adolfsson

mc_subled used for multi_index needs well defined array indexes, to guarantee the desired result, 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, an error is returned,
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.

---
  drivers/leds/leds-lp50xx.c | 11 ++++++++++-
  1 file changed, 10 insertions(+), 1 deletion(-)
drivers/leds/leds-lp50xx.c +5 -0
index 02cb1565a9fb..94f8ef6b482c 100644
@@ -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;
  			ret = fwnode_property_read_u32(led_node, "color",
  						       &color_id);
  			if (ret) {
@@ -483,8 +484,16 @@ 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) {
+				dev_err(priv->dev, "reg must be set\n");
+				return -EINVAL;

drivers/leds/leds-lp50xx.c static int lp50xx_probe_dt(struct lp50xx *priv) · line 499

Quoted 10 lines
drivers/leds/leds-lp50xx.c +5 -1
+			} else if (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++;
  		}
 
Signature · 2 lines
Best regards,
Jacek Anaszewski
#3 lore raw

PATCH v7 2/2 dt-bindings: leds: lp50xx: Document child reg, fix example

In reply to Johan Adolfsson

The led child reg node is the index within the bank, document that and update the example accordingly.
The reg property in child node is limited to 0-2 since there are 3 leds per bank, previous value in example was speculative.

---
 .../devicetree/bindings/leds/leds-lp50xx.yaml         | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)
Documentation/devicetree/bindings/leds/leds-lp50xx.yaml +12 -7
index 402c25424525..23f809906ba7 100644
@@ -81,7 +81,12 @@ patternProperties:
 
         properties:
           reg:
-            maxItems: 1
+            items:
+              - minimum: 0
+                maximum: 2
+
+            description:
+              This property denotes the index within the LED bank.
 
         required:
           - reg
@@ -138,18 +143,18 @@ examples:
                 color = <LED_COLOR_ID_RGB>;
                 function = LED_FUNCTION_STANDBY;
 
-                led@3 {
-                    reg = <0x3>;
+                led@0 {
+                    reg = <0x0>;
                     color = <LED_COLOR_ID_RED>;
                 };
 
-                led@4 {
-                    reg = <0x4>;
+                led@1 {
+                    reg = <0x1>;
                     color = <LED_COLOR_ID_GREEN>;
                 };
 
-                led@5 {
-                    reg = <0x5>;
+                led@2 {
+                    reg = <0x2>;
                     color = <LED_COLOR_ID_BLUE>;
                 };
             };
Signature · 1 line
2.30.2
Acked-by on v7 2/2
#4 lore raw

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

Quoted 4 lines · Johan Adolfsson

The led child reg node is the index within the bank, document that and update the example accordingly.
The reg property in child node is limited to 0-2 since there are 3 leds per bank, previous value in example was speculative.

maintainer Applied
#5 lore raw
In reply to Johan Adolfsson
Quoted 11 lines · 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.

[...]

Applied, thanks!

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

      commit: f0022183bfd939f7d3c669cca0bf3aade233c9a5
[2/2] dt-bindings: leds: lp50xx: Document child reg, fix example
      commit: d07927c0ca918a6825ef46cb7c012dbc102ac352

--
Lee Jones [李琼斯]