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

From: Jean-Jacques Hiblot
Date: Mon Jun 27 2022 - 03:57:13 EST



On 18/06/2022 01:19, Rob Herring wrote:
On Tue, Jun 14, 2022 at 05:42:43PM +0200, Jean-Jacques Hiblot wrote:
Add bindings documentation for the TLC5925 LED controller.

Signed-off-by: Jean-Jacques Hiblot <jjhiblot@xxxxxxxxxxxxxxx>
---
.../devicetree/bindings/leds/ti,tlc5925.yaml | 107 ++++++++++++++++++
1 file changed, 107 insertions(+)
create mode 100644 Documentation/devicetree/bindings/leds/ti,tlc5925.yaml

diff --git a/Documentation/devicetree/bindings/leds/ti,tlc5925.yaml b/Documentation/devicetree/bindings/leds/ti,tlc5925.yaml
new file mode 100644
index 000000000000..12a71e48f854
--- /dev/null
+++ b/Documentation/devicetree/bindings/leds/ti,tlc5925.yaml
@@ -0,0 +1,107 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/leds/ti,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.
+
+$ref: /schemas/spi/spi-peripheral-props.yaml#
+
+properties:
+ compatible:
+ const: ti,tlc5925
+
+ ti,shift-register-length:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ minimum: 8
+ description: |
Don't need '|' if no formatting to preserve.

+ 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.
multipleOf: 8

Though I'm confused why it's not 16.

The reason why it is 8 instead of 16, is that there exists other LEDs controllers that are compatible with this driver and use only a 8-bits shift register.

The main reason is that the granularity of the size of a SPI transfer is 8 bits.

+
+ "#address-cells":
+ const: 1
+
+ "#size-cells":
+ const: 0
+
+ output-enable-b-gpios:
+ $ref: /schemas/types.yaml#/definitions/phandle-array
Already has a type.

+ description: |
+ Optional GPIO pins to enable/disable the parallel output. They describe
+ the GPIOs connected to the OE/ pin of the TLC5925s.
+
+patternProperties:
+ "@[0-9a-f]+$":
+ type: object
+ $ref: common.yaml#
unevaluatedProperties: false

+
+ properties:
+ reg:
+ $ref: /schemas/types.yaml#/definitions/uint32
Don't need a type here.

+ description: |
+ LED pin number (must be lower than ti,shift-register-length).
+ The furthest LED down the chain has the pin number 0.
+
+ required:
+ - reg
+
+required:
+ - "#address-cells"
+ - "#size-cells"
+ - ti,shift-register-length
Is there not a default when not chained?

A default value could be used indeed. I'll rework the driver to use a default value of 16

Thanks

Jean-Jacques


+
+unevaluatedProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/gpio/gpio.h>
+ #include <dt-bindings/leds/common.h>
+
+ spi0 {
spi {

+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ leds@2 {
+ compatible = "ti,tlc5925";
+ reg = <0x02>;
+ spi-max-frequency = <30000000>;
+ ti,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@0 {
+ reg = <0>;
+ function = LED_FUNCTION_STATUS;
+ color = <LED_COLOR_ID_GREEN>;
+ };
+
+ led@4 {
+ reg = <4>;
+ function = LED_FUNCTION_STATUS;
+ color = <LED_COLOR_ID_RED>;
+ };
+
+ led@1f {
+ reg = <31>;
+ function = LED_FUNCTION_PANIC;
+ color = <LED_COLOR_ID_RED>;
+ };
+ };
+
+ };
--
2.25.1