linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v5 0/2] leds-lp50xx: Support reg to set multi_index.
@ 2025-06-16  7:57 Johan Adolfsson
  2025-06-16  7:57 ` [PATCH v5 1/2] leds: leds-lp50xx: Handle reg to get correct multi_index Johan Adolfsson
  2025-06-16  7:57 ` [PATCH v5 2/2] dt-bindings: leds: lp50xx: Document child reg, fix example Johan Adolfsson
  0 siblings, 2 replies; 4+ messages in thread
From: Johan Adolfsson @ 2025-06-16  7:57 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

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 v5:
- Fail if reg is not set.
- Adjust devicetree schema, use items.
- Link to v4: https://lore.kernel.org/r/20250526-led-fix-v4-0-33345f6c4a78@axis.com

Changes in v4:
- Remove maxItems from devicetree schema, not compatible with minimum
  and maximum.
- Link to v3: https://lore.kernel.org/r/20250523-led-fix-v3-0-86d2690d2698@axis.com

Changes in v3:
- Update To and Cc.
- Rephrase bindings descriptions, add constraints.
- Link to v2: https://lore.kernel.org/r/20250522-led-fix-v2-0-652046323ec3@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

 .../devicetree/bindings/leds/leds-lp50xx.yaml        | 20 ++++++++++++++------
 drivers/leds/leds-lp50xx.c                           | 11 ++++++++++-
 2 files changed, 24 insertions(+), 7 deletions(-)
---
base-commit: 0ff41df1cb268fc69e703a08a57ee14ae967d0ca
change-id: 20250225-led-fix-444fb544584a

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


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH v5 1/2] leds: leds-lp50xx: Handle reg to get correct multi_index
  2025-06-16  7:57 [PATCH v5 0/2] leds-lp50xx: Support reg to set multi_index Johan Adolfsson
@ 2025-06-16  7:57 ` Johan Adolfsson
  2025-06-16  7:57 ` [PATCH v5 2/2] dt-bindings: leds: lp50xx: Document child reg, fix example Johan Adolfsson
  1 sibling, 0 replies; 4+ messages in thread
From: Johan Adolfsson @ 2025-06-16  7:57 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..344791b6c575 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,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] 4+ messages in thread

* [PATCH v5 2/2] dt-bindings: leds: lp50xx: Document child reg, fix example
  2025-06-16  7:57 [PATCH v5 0/2] leds-lp50xx: Support reg to set multi_index Johan Adolfsson
  2025-06-16  7:57 ` [PATCH v5 1/2] leds: leds-lp50xx: Handle reg to get correct multi_index Johan Adolfsson
@ 2025-06-16  7:57 ` Johan Adolfsson
  2025-06-16  9:59   ` Rob Herring (Arm)
  1 sibling, 1 reply; 4+ messages in thread
From: Johan Adolfsson @ 2025-06-16  7:57 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.

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

diff --git a/Documentation/devicetree/bindings/leds/leds-lp50xx.yaml b/Documentation/devicetree/bindings/leds/leds-lp50xx.yaml
index 402c25424525..edcc88db1ea7 100644
--- a/Documentation/devicetree/bindings/leds/leds-lp50xx.yaml
+++ b/Documentation/devicetree/bindings/leds/leds-lp50xx.yaml
@@ -82,6 +82,14 @@ patternProperties:
         properties:
           reg:
             maxItems: 1
+            items:
+              - minimum: 0
+                maximum: 2
+
+            description:
+              This property denotes the index within the LED bank.
+              The value will act as the index in the multi_index file to give
+              consistent result independent of devicetree processing order.
 
         required:
           - reg
@@ -138,18 +146,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] 4+ messages in thread

* Re: [PATCH v5 2/2] dt-bindings: leds: lp50xx: Document child reg, fix example
  2025-06-16  7:57 ` [PATCH v5 2/2] dt-bindings: leds: lp50xx: Document child reg, fix example Johan Adolfsson
@ 2025-06-16  9:59   ` Rob Herring (Arm)
  0 siblings, 0 replies; 4+ messages in thread
From: Rob Herring (Arm) @ 2025-06-16  9:59 UTC (permalink / raw)
  To: Johan Adolfsson
  Cc: Andrew Davis, Lee Jones, kernel, Conor Dooley, Jacek Anaszewski,
	linux-leds, devicetree, linux-kernel, Pavel Machek,
	Krzysztof Kozlowski


On Mon, 16 Jun 2025 09:57:09 +0200, 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>
> ---
>  .../devicetree/bindings/leds/leds-lp50xx.yaml        | 20 ++++++++++++++------
>  1 file changed, 14 insertions(+), 6 deletions(-)
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/leds/leds-lp50xx.yaml: patternProperties:^multi-led@[0-9a-f]$:patternProperties:^led@[0-9a-f]+$:properties:reg: {'maxItems': 1, 'items': [{'minimum': 0, 'maximum': 2}], 'description': 'This property denotes the index within the LED bank. The value will act as the index in the multi_index file to give consistent result independent of devicetree processing order.'} should not be valid under {'required': ['maxItems']}
	hint: "maxItems" is not needed with an "items" list
	from schema $id: http://devicetree.org/meta-schemas/items.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/leds/leds-lp50xx.yaml: patternProperties:^multi-led@[0-9a-f]$:patternProperties:^led@[0-9a-f]+$:properties:reg: 'anyOf' conditional failed, one must be fixed:
	'items' is not one of ['maxItems', 'description', 'deprecated']
		hint: Only "maxItems" is required for a single entry if there are no constraints defined for the values.
	'maxItems' is not one of ['description', 'deprecated', 'const', 'enum', 'minimum', 'maximum', 'multipleOf', 'default', '$ref', 'oneOf']
	'items' is not one of ['description', 'deprecated', 'const', 'enum', 'minimum', 'maximum', 'multipleOf', 'default', '$ref', 'oneOf']
	1 is less than the minimum of 2
		hint: Arrays must be described with a combination of minItems/maxItems/items
	hint: cell array properties must define how many entries and what the entries are when there is more than one entry.
	from schema $id: http://devicetree.org/meta-schemas/core.yaml#

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20250616-led-fix-v5-2-f59c740831ab@axis.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2025-06-16  9:59 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-16  7:57 [PATCH v5 0/2] leds-lp50xx: Support reg to set multi_index Johan Adolfsson
2025-06-16  7:57 ` [PATCH v5 1/2] leds: leds-lp50xx: Handle reg to get correct multi_index Johan Adolfsson
2025-06-16  7:57 ` [PATCH v5 2/2] dt-bindings: leds: lp50xx: Document child reg, fix example Johan Adolfsson
2025-06-16  9:59   ` Rob Herring (Arm)

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).