Re: [PATCH 1/3] dt-bindings: leds: Add bindings for the TLC5925 controller

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 23/05/2022 12:14, Krzysztof Kozlowski wrote:
On 23/05/2022 10:49, Jean-Jacques Hiblot wrote:
Add bindings documentation for the TLC5925 LED controller.

Signed-off-by: Jean-Jacques Hiblot <jjhiblot@xxxxxxxxxxxxxxx>
Thank you for your patch. There is something to discuss/improve.

---
devicetree@xxxxxxxxxxxxxxx
  .../bindings/leds/leds-tlc5925.yaml           | 100 ++++++++++++++++++
  1 file changed, 100 insertions(+)
  create mode 100644 Documentation/devicetree/bindings/leds/leds-tlc5925.yaml

diff --git a/Documentation/devicetree/bindings/leds/leds-tlc5925.yaml b/Documentation/devicetree/bindings/leds/leds-tlc5925.yaml
new file mode 100644
index 000000000000..156db599d5a1
--- /dev/null
+++ b/Documentation/devicetree/bindings/leds/leds-tlc5925.yaml
Filename: vendor,device
so "ti,tlc5925-leds.yaml" for example.



@@ -0,0 +1,100 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/leds/leds-tlc5925.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: LEDs connected to TI TLC5925 controller
+
+maintainers:
+  - Jean-Jacques Hiblot <jjhiblot@xxxxxxxxxxxxxxx>
+
+description: |
+  The TLC5925 is a low-power 16-channel constant-current LED sink driver.
+  It is controlled through a SPI interface.
+  It is built around a shift register and latches which convert serial
+  input data into a parallel output. Several TLC5925 can be chained to
+  control more than 16 LEDs with a single chip-select.
+  The brightness level cannot be controlled, each LED is either on or off.
+
+  Each LED is represented as a sub-node of the ti,tlc5925 device.
+
+properties:
+  compatible:
+    const: ti,tlc5925
+
+  shift_register_length:
+    maxItems: 1
No...
1. Did you test your bindings with dt_binding_check? This fails
obviously... please, do not send untested bindings.

2. vendor prefix, no underscores, proper type, maxItems look wrong here

+    description: |
+      The length of the shift register. If several TLC5925 are chained,
+      shift_register_length should be set to 16 times the number of TLC5925.
+      The value must be a multiple of 8.
+
+  "#address-cells":
+    const: 1
+
+  "#size-cells":
+    const: 0
+
+  output-enable-b-gpios:
+    description: |
+      GPIO pins to enable/disable the parallel output. They describe the GPIOs
+      connected to the OE/ pin of the TLC5925s.
maxItems

There is no limitation in the driver itself. The actual number of items here really depends on the number of chips and how they are wired.



+
+patternProperties:
+  "@[a-f0-9]+$":
How many LEDs you can have here? Usually it is limited, so the pattern
should be narrowed.

There is no limitation here either. The chips can be chained to augment the number of LEDs.

The max number of LED is equal to the length of the shift-register.


Jean-Jacques


+    type: object
+
+    $ref: common.yaml#
+
+    properties:
+      reg:
+        items:
Not correct syntax... I will stop reviewing. There is no point to use
reviewers time to do the job of a tool.


+examples:
+  - |
+    &spi0 {
+        leds@2 {
+                compatible = "ti,tlc5925";
Messed up indentation. 4 spaces for DTS example.

+                reg = <0x02>;
+                spi-max-frequency = <30000000>;
+                shift_register_length = <32>;
+                output-enable-b-gpios = <&gpio0b 9 GPIO_ACTIVE_HIGH>, <&gpio0b 7 GPIO_ACTIVE_HIGH>;
+                #address-cells = <1>;
+                #size-cells = <0>;
+
+                led-satus@0 {
+                        reg = <0>;
+                        function = LED_FUNCTION_STATUS;
+                        color = <LED_COLOR_ID_GREEN>;
+                };
+
+                led-satus@4 {
+                        reg = <4>;
+                        function = LED_FUNCTION_STATUS;
+                        color = <LED_COLOR_ID_RED>;
+                };
+
+                led-alive@24 {
+                        reg = <24>;
+                        label = "green:alive"
+                };
+
+                led-panic@31 {
+                        reg = <31>;
+                        label = "red:panic"
+                };
+        };
+    };

Best regards,
Krzysztof



[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux