* [PATCH v7 1/2] leds: leds-lp50xx: Handle reg to get correct multi_index
2025-06-17 10:23 [PATCH v7 0/2] leds-lp50xx: Support reg to set multi_index Johan Adolfsson
@ 2025-06-17 10:23 ` Johan Adolfsson
2025-06-19 14:02 ` Jacek Anaszewski
2025-06-17 10:23 ` [PATCH v7 2/2] dt-bindings: leds: lp50xx: Document child reg, fix example Johan Adolfsson
2025-06-25 9:06 ` [PATCH v7 0/2] leds-lp50xx: Support reg to set multi_index Lee Jones
2 siblings, 1 reply; 6+ messages in thread
From: Johan Adolfsson @ 2025-06-17 10:23 UTC (permalink / raw)
To: Lee Jones, Pavel Machek, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Andrew Davis, Jacek Anaszewski
Cc: linux-leds, linux-kernel, devicetree, Johan Adolfsson, kernel
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.
Signed-off-by: Johan Adolfsson <johan.adolfsson@axis.com>
---
drivers/leds/leds-lp50xx.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/leds/leds-lp50xx.c b/drivers/leds/leds-lp50xx.c
index 02cb1565a9fb..94f8ef6b482c 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;
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++;
}
--
2.30.2
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v7 1/2] leds: leds-lp50xx: Handle reg to get correct multi_index
2025-06-17 10:23 ` [PATCH v7 1/2] leds: leds-lp50xx: Handle reg to get correct multi_index Johan Adolfsson
@ 2025-06-19 14:02 ` Jacek Anaszewski
0 siblings, 0 replies; 6+ messages in thread
From: Jacek Anaszewski @ 2025-06-19 14:02 UTC (permalink / raw)
To: Johan Adolfsson, Lee Jones, Pavel Machek, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Andrew Davis
Cc: linux-leds, linux-kernel, devicetree, kernel
Hi Johan,
Thanks for the update.
On 6/17/25 12:23, Johan Adolfsson wrote:
> 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.
>
> Signed-off-by: Johan Adolfsson <johan.adolfsson@axis.com>
> ---
> drivers/leds/leds-lp50xx.c | 11 ++++++++++-
> 1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/leds/leds-lp50xx.c b/drivers/leds/leds-lp50xx.c
> index 02cb1565a9fb..94f8ef6b482c 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;
> 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++;
> }
>
>
Reviewed-by: Jacek Anaszewski <jacek.anaszewski@gmail.com>
--
Best regards,
Jacek Anaszewski
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v7 2/2] dt-bindings: leds: lp50xx: Document child reg, fix example
2025-06-17 10:23 [PATCH v7 0/2] leds-lp50xx: Support reg to set multi_index Johan Adolfsson
2025-06-17 10:23 ` [PATCH v7 1/2] leds: leds-lp50xx: Handle reg to get correct multi_index Johan Adolfsson
@ 2025-06-17 10:23 ` Johan Adolfsson
2025-06-17 15:49 ` Conor Dooley
2025-06-25 9:06 ` [PATCH v7 0/2] leds-lp50xx: Support reg to set multi_index Lee Jones
2 siblings, 1 reply; 6+ messages in thread
From: Johan Adolfsson @ 2025-06-17 10:23 UTC (permalink / raw)
To: Lee Jones, Pavel Machek, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Andrew Davis, Jacek Anaszewski
Cc: linux-leds, linux-kernel, devicetree, Johan Adolfsson, kernel
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.
Signed-off-by: Johan Adolfsson <johan.adolfsson@axis.com>
---
.../devicetree/bindings/leds/leds-lp50xx.yaml | 19 ++++++++++++-------
1 file changed, 12 insertions(+), 7 deletions(-)
diff --git a/Documentation/devicetree/bindings/leds/leds-lp50xx.yaml b/Documentation/devicetree/bindings/leds/leds-lp50xx.yaml
index 402c25424525..23f809906ba7 100644
--- a/Documentation/devicetree/bindings/leds/leds-lp50xx.yaml
+++ b/Documentation/devicetree/bindings/leds/leds-lp50xx.yaml
@@ -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>;
};
};
--
2.30.2
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v7 2/2] dt-bindings: leds: lp50xx: Document child reg, fix example
2025-06-17 10:23 ` [PATCH v7 2/2] dt-bindings: leds: lp50xx: Document child reg, fix example Johan Adolfsson
@ 2025-06-17 15:49 ` Conor Dooley
0 siblings, 0 replies; 6+ messages in thread
From: Conor Dooley @ 2025-06-17 15:49 UTC (permalink / raw)
To: Johan Adolfsson
Cc: Lee Jones, Pavel Machek, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Andrew Davis, Jacek Anaszewski, linux-leds,
linux-kernel, devicetree, kernel
[-- Attachment #1: Type: text/plain, Size: 354 bytes --]
On Tue, Jun 17, 2025 at 12:23:55PM +0200, Johan Adolfsson wrote:
> 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.
Acked-by: Conor Dooley <conor.dooley@microchip.com>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v7 0/2] leds-lp50xx: Support reg to set multi_index.
2025-06-17 10:23 [PATCH v7 0/2] leds-lp50xx: Support reg to set multi_index Johan Adolfsson
2025-06-17 10:23 ` [PATCH v7 1/2] leds: leds-lp50xx: Handle reg to get correct multi_index Johan Adolfsson
2025-06-17 10:23 ` [PATCH v7 2/2] dt-bindings: leds: lp50xx: Document child reg, fix example Johan Adolfsson
@ 2025-06-25 9:06 ` Lee Jones
2 siblings, 0 replies; 6+ messages in thread
From: Lee Jones @ 2025-06-25 9:06 UTC (permalink / raw)
To: Lee Jones, Pavel Machek, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Andrew Davis, Jacek Anaszewski, Johan Adolfsson
Cc: linux-leds, linux-kernel, devicetree, kernel
On Tue, 17 Jun 2025 12:23:53 +0200, Johan Adolfsson wrote:
> 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 [李琼斯]
^ permalink raw reply [flat|nested] 6+ messages in thread