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

Nam Tran
2025-04-17T02:06:50Z

On Tue, 15 Apr 2025, Krzysztof Kozlowski wrote:

>On 15/04/2025 11:53, Nam Tran wrote:
>> On Mon, 14 Apr 2025, Krzysztof Kozlowski wrote:
>>
>>> On 14/04/2025 16:57, Nam Tran wrote:
>>>> +
>>>> +description: |
>>>> + The LP5812 is an I2C LED Driver that can support LED matrix 4x3.
>>>> + 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
>>>> +
>>>> + "#address-cells":
>>>> + const: 1
>>>> +
>>>> + "#size-cells":
>>>> + const: 0
>>>
>>> No need for supply?
>>
>> Since the hardware uses an external power supply,
>> we decide not to include the supply property in the binding.
>
>So there is power supply? If so, must be in the binding. Bindings
>describe given hardware (LP5812), not your particular board/setup.

Thank you for the clarification.
The LP5812 is externally powered and has a dedicated VCC pin.
I'll update the binding to include a `vcc-supply` property.

>>
>>>> +
>>>> +patternProperties:
>>>> + "^led@[0-9a-b]$":
>>>> + type: object
>>>> + $ref: common.yaml#
>>>> + unevaluatedProperties: false
>>>> +
>>>> + properties:
>>>> + reg:
>>>> + minimum: 0
>>>> + maximum: 0xb
>>>> +
>>>> + chan-name:
>>>> + $ref: /schemas/types.yaml#/definitions/string
>>>> + description: LED channel name
>>>
>>> My comment stay valid. I don't think LEDs have channels, datasheet also
>>> has nothing about channels, so again - use existing properties. Or
>>> better drop it - I don't see any point in the name. The reg already
>>> defines it.
>>
>> The channel was named for the output channel to each LED, not the LED channels.
>
>I don't understand what you want to say. Please explain why existing
>label property is not correct here.

I understand that the label property is deprecated and that the preferred approach now is to use function and color instead.
However, in the case of the LP5812, which is a matrix LED driver, these properties are not a good fit.
The LP5812 does not associate each output with a specific function (like "status", "activity"),
and the LEDs driven by LP5812 are not fixed to a particular color.

>> but the person who wants to develop LP5812's matrix-related features can use the "channels" for easy mapping.
>
>easy mapping of what? Please show me the usage.

You're right — I cannot provide a meaningful usage example for chan-name.
The chan-name property was intended to give a more descriptive name for each LED channel, mainly for convenience in user space.
But since this isn’t standard and you advised against introducing such a property, we’ve decided to drop it.

>>
>>>
>>> However after dropping this, your example has nodes with only reg -
>>> what's the point of them? Why no properties from common.yaml are
>>> applicable? If they are not applicable, then the entire subnode should
>>> be dropped - you don't need them to describe the hardware.
>>
>> Actually, the "color" property can be applied, but the LP5812 is a matrix LED,
>> so specifying a particular LED color is not necessary when developing LP5812 features.
>
>This does not help me much and based on this I see no points in
>describing individual LEDs, because the only missing information is
>number of them but even that is fixed for given device, isn't it?

Actually, the number of LED outputs on the LP5812 is not strictly fixed — it depends on the selected operating mode.
This mode is configurable by the end user at runtime through sysfs interfaces provided by the driver.

I understand your point — if no additional properties from common.yaml are applicable, these subnodes may not be necessary.
Therefore, we’ve decided to drop them.

Best regards,
Nam Tran