Re: [PATCH v3 1/2] dt-bindings: leds: add cznic,turris-omnia-leds binding

[Jacek Anaszewski <jacek.anaszewski@gmail.com>]

On 7/13/20 5:06 PM, Rob Herring wrote:
> On Mon, Jul 13, 2020 at 12:11:51AM +0200, Jacek Anaszewski wrote:
>> On 7/12/20 11:40 PM, Marek Behun wrote:
>>> On Sun, 12 Jul 2020 23:27:07 +0200
>>> Jacek Anaszewski <jacek.anaszewski@gmail.com> wrote:
>>>
>>>>> +            multi-led@0 {
>>>>> +                reg = <0>;
>>>>> +                color = <LED_COLOR_ID_MULTI>;
>>>>> +                function = LED_FUNCTION_POWER;
>>>>
>>>> Please provide child nodes for each color LED. Let's stick
>>>> to the bindings closely and not make any deviations from
>>>> the beginning.
>>>
>>> Why? It would make sense if there were devices using this controller
>>> having other configuration, but on Omnia, all LEDs are RGB.
>>>
>>> Also, if I do this, should I also make the driver check in the probe
>>> function whether the per-channel child nodes are correct? Eg. if they
>>> are always three: one for red, one for green and one for blue? Or
>>> should the driver ignore this and only the device tree binding specify
>>> it?
>>>
>>> Because the way the driver is written now, it only registers
>>> multi-color RGB LEDs.
>>
>> This is not RGB framework, but multicolor framework. It is not justified
>> to pretend that RGB is default. Unless you would state that clearly in
>> the comment in DT, but that should be agreed upon with Rob.
> 
> If the LEDs are fixed in h/w and never vary for this controller, then
> they don't need to be in DT. However, is it really possible that a
> channel only supports 1 color of LED? I don't think so.

Theoretically we could allow for registering two LEDs as LED multi
color device and the other one as monochrome LED class device
but I admit that this would be a bit convoluted and entail unnecessary
complexity in the driver.

So I'm OK with no child nodes for these bindings, but let's add some
comment justifying that.

-- 
Best regards,
Jacek Anaszewski