Re: [PATCH v18 1/3] dt-bindings: leds: add TI/National Semiconductor LP5812 LED Driver

[Krzysztof Kozlowski <krzk@kernel.org>]

On 24/11/2025 08:57, Krzysztof Kozlowski wrote:
> On Mon, Nov 24, 2025 at 02:10:40AM +0700, Nam Tran wrote:
>> The LP5812 is a 4x3 RGB LED driver with an autonomous animation
>> engine and time-cross-multiplexing (TCM) support for up to 12 LEDs
>> or 4 RGB LEDs. It supports both analog (256 levels) and PWM (8-bit)
>> dimming, including exponential PWM for smooth brightness control.
>>
>> Signed-off-by: Nam Tran <trannamatk@gmail.com>
>> ---
>>  .../devicetree/bindings/leds/ti,lp5812.yaml   | 251 ++++++++++++++++++
>>  MAINTAINERS                                   |   6 +
>>  2 files changed, 257 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/leds/ti,lp5812.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/leds/ti,lp5812.yaml b/Documentation/devicetree/bindings/leds/ti,lp5812.yaml
>> new file mode 100644
>> index 000000000000..ea9d6ae92344
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/leds/ti,lp5812.yaml
>> @@ -0,0 +1,251 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/leds/ti,lp5812.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: TI LP5812 4x3 Matrix RGB LED Driver with Autonomous Control
>> +
>> +maintainers:
>> +  - Nam Tran <trannamatk@gmail.com>
>> +
>> +description: |
>> +  The LP5812 is a 4x3 matrix RGB LED driver with I2C interface
>> +  and autonomous animation engine control.
>> +  For more product information please see the link below:
>> +  https://www.ti.com/product/LP5812#tech-docs
>> +
>> +properties:
>> +  compatible:
>> +    const: ti,lp5812
>> +
>> +  reg:
>> +    maxItems: 1
>> +
>> +  ti,scan-mode:
>> +    description: |
>> +      Selects the LED scan mode of the LP5812. The device supports
>> +      three modes:
>> +        - Direct-drive mode (by default if 'ti,scan-mode' is omitted)
>> +        drives up to 4 LEDs directly by internal current sinks (LED0-LED3).
>> +        - TCM-drive mode ("tcm:<n>:<order...>") drives up to 12 LEDs
>> +        (4 RGB) using 1-4 scan multiplexing. The <n> specifies the number
>> +        of scans (1-4), and <order...> defines the scan order of the outputs.
>> +        - Mix-drive mode ("mix:<n>:<direct>:<order...>") combines
>> +        direct-drive and TCM-drive outputs. The <n> specifies the number
>> +        of scans, <direct> selects the direct-drive outputs, and <order...>
>> +        defines the scan order.
>> +    $ref: /schemas/types.yaml#/definitions/string
>> +    pattern: '^(tcm|mix):[1-4](:[0-3]){1,4}$'
>> +
>> +  vcc-supply:
>> +    description: Regulator providing power to the 'VCC' pin.
>> +
>> +  "#address-cells":
>> +    const: 1
>> +
>> +  "#size-cells":
>> +    const: 0
>> +
>> +patternProperties:
>> +  "^led@[0-3]$":
>> +    type: object
>> +    $ref: common.yaml#
>> +    unevaluatedProperties: false
>> +
>> +    properties:
>> +      reg:
>> +        minimum: 0
>> +        maximum: 3
>> +
>> +    required:
>> +      - reg
>> +      - label
> 
> No, why? That's legacy property and color and function are preferred.
> 
>> +
>> +  "^multi-led@[4-7]$":
>> +    type: object
>> +    $ref: leds-class-multicolor.yaml#
>> +    unevaluatedProperties: false
>> +
>> +    properties:
>> +      reg:
>> +        minimum: 4
>> +        maximum: 7
>> +
>> +      "#address-cells":
>> +        const: 1
>> +
>> +      "#size-cells":
>> +        const: 0
>> +
>> +    patternProperties:
>> +      "^led@[4-9a-f]$":
>> +        type: object
>> +        $ref: common.yaml#
>> +        unevaluatedProperties: false
>> +
>> +        properties:
>> +          reg:
>> +            minimum: 4
>> +            maximum: 15
>> +
>> +        required:
>> +          - reg
>> +
>> +    required:
>> +      - reg
>> +      - label
> 
> Why? Same problems.
> 
> Please stop making continuous changes to the binding.
> 
> 
>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> +  - |
>> +    #include <dt-bindings/leds/common.h>
>> +
>> +    i2c {
>> +        #address-cells = <1>;
>> +        #size-cells = <0>;
>> +
>> +        led-controller@1b {
>> +            #address-cells = <1>;
>> +            #size-cells = <0>;
>> +            compatible = "ti,lp5812";
>> +            reg = <0x1b>;
>> +            ti,scan-mode = "tcm:4:0:1:2:3";
>> +            vcc-supply = <&vdd_3v3_reg>;
>> +
>> +            led@0 {
>> +                reg = <0x0>;
>> +                label = "LED0";
>> +                led-max-microamp = <25500>;
>> +            };
>> +
>> +            led@1 {
>> +                reg = <0x1>;
>> +                label = "LED1";
> 
> Completely useless label... You require labels, so people need to write
> something but since they do not know what to write they call LED 1 a
> LED1. This is just not helping.
> 
> Use color and function properties. Same everywhere else.
>

And now I went to older versions and I see they were correct - you had
color! You replace correct code with wrong one and drop review. This
patchset is not really improving.

BTW, You actually received review also at v6, so this was reviewed 3 or
more times. Way too many times.

Best regards,
Krzysztof