Re: [PATCH v3 1/2] dt-bindings: leds: Add multicolor PWM LED bindings

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

On 1/28/22 9:36 PM, Marek Behún wrote:
> On Thu, 27 Jan 2022 22:24:21 +0100
> Jacek Anaszewski <jacek.anaszewski@gmail.com> wrote:
> 
>> Hi Sven,
>>
>> On 1/26/22 11:48 AM, sven@svenschwermer.de wrote:
>>> From: Sven Schwermer <sven.schwermer@disruptive-technologies.com>
>>>
>>> This allows to group multiple PWM-connected monochrome LEDs into
>>> multicolor LEDs, e.g. RGB LEDs.
>>>
>>> Signed-off-by: Sven Schwermer <sven.schwermer@disruptive-technologies.com>
>>> ---
>> [...]
>>> +
>>> +additionalProperties: false
>>> +
>>> +examples:
>>> +  - |
>>> +    #include <dt-bindings/leds/common.h>
>>> +
>>> +    rgb-led {
>>> +        compatible = "pwm-leds-multicolor";
>>> +
>>> +        multi-led {
>>> +          color = <LED_COLOR_ID_RGB>;
>>> +          function = LED_FUNCTION_INDICATOR;
>>> +          max-brightness = <65535>;
>>
>> It doesn't make much sense to have such a big resolution of global
>> multi color brightness. 255 will be sufficient.
> 
> If the PWM supports it, why not?
> On Omnia the default is 255, and since it is PWM, the change from 0/255
> to 1/255 is much bigger then from, say, 15/255 to 16/255. So if 1/255
> is too bright, you are then unable to set it less bright. I think 1024
> or ever 65535 makes sense with PWMs.

With values other than 255 we will not achieve 24-bit RGB, which is one
problem, and the other one is non-linear brightness that can be achieved
with PWM. So probably we would need to add an additional note in the
documentation [0], saying that changing global brightness allows to
preserve combined LED hue only when all sub-leds are linear, and that it
will not be the case for PWM LEDs.

And I propose to change multi-led 'color' DT property value from
LED_COLOR_ID_RGB to LED_COLOR_ID_MULTI to avoid the impression that it
will work as traditional 24-bit RGB.

[0] Documentation/leds/leds-class-multicolor.rst

-- 
Best regards,
Jacek Anaszewski