linux-kernel - Re: [PATCH] dt-bindings: leds: Expand LED_COLOR

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <49ade311-76b6-4520-3a8c-fe38f1ee2a52@schinagl.nl>
Date:   Wed, 10 Aug 2022 19:08:43 +0200
From:   Olliver Schinagl <oliver@...inagl.nl>
To:     Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>
Cc:     Jacek Anaszewski <jacek.anaszewski@...il.com>,
        Baolin Wang <baolin.wang@...aro.org>,
        Daniel Mack <daniel@...que.org>,
        Linus Walleij <linus.walleij@...aro.org>,
        Oleh Kravchenko <oleg@....org.ua>,
        Sakari Ailus <sakari.ailus@...ux.intel.com>,
        Simon Shields <simon@...eageos.org>,
        Olliver Schinagl <oliver+list@...inagl.nl>,
        devicetree@...r.kernel.org,
        linux-kernel <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] dt-bindings: leds: Expand LED_COLOR_ID definitions

Hey Krzysztof,

On 10-08-2022 15:27, Krzysztof Kozlowski wrote:
> On 10/08/2022 14:07, Olliver Schinagl wrote:
>> In commit 853a78a7d6c7 (dt-bindings: leds: Add LED_COLOR_ID definitions,
>> Sun Jun 9 20:19:04 2019 +0200) the most basic color definitions where
>> added. However, there's a little more very common LED colors.
>>
>> While the documentation states 'add what is missing', engineers tend to
>> be lazy and will just use what currently exists. So this patch will take
>> (a) list from online retailers [0], [1], [2] and use the common LED
>> colors from
>> there, this being reasonable as this is what is currently available to
>> purchase.
>>
>> Note, that LIME seems to be the modern take to 'Yellow-green' or
>> 'Yellowish-green' from some older datasheets.
>>
>> [0]: https://www.digikey.com/en/products/filter/led-lighting-color/125
>> [1]:
>> https://eu.mouser.com/c/optoelectronics/led-lighting/led-emitters/standard-leds-smd
>> [2]:
>> https://nl.farnell.com/en-NL/c/optoelectronics-displays/led-products/standard-single-colour-leds-under-75ma
>>
>> Signed-off-by: Olliver Schinagl <oliver@...inagl.nl>
>> ---
>>    include/dt-bindings/leds/common.h | 28 ++++++++++++++++------------
>>    1 file changed, 16 insertions(+), 12 deletions(-)
>>
>> diff --git a/include/dt-bindings/leds/common.h
>> b/include/dt-bindings/leds/common.h
>> index 3be89a7c20a9..1d9b955267cc 100644
>> --- a/include/dt-bindings/leds/common.h
>> +++ b/include/dt-bindings/leds/common.h
>> @@ -22,18 +22,22 @@
>>    #define LEDS_BOOST_FIXED    2
>>
>>    /* Standard LED colors */
>> -#define LED_COLOR_ID_WHITE    0
>> -#define LED_COLOR_ID_RED    1
>> -#define LED_COLOR_ID_GREEN    2
>> -#define LED_COLOR_ID_BLUE    3
>> -#define LED_COLOR_ID_AMBER    4
>> -#define LED_COLOR_ID_VIOLET    5
>> -#define LED_COLOR_ID_YELLOW    6
>> -#define LED_COLOR_ID_IR        7
>> -#define LED_COLOR_ID_MULTI    8    /* For multicolor LEDs */
>> -#define LED_COLOR_ID_RGB    9    /* For multicolor LEDs that can do
>> arbitrary color,
>> -                       so this would include RGBW and similar */
>> -#define LED_COLOR_ID_MAX    10
>> +#define LED_COLOR_ID_WHITE      0
>> +#define LED_COLOR_ID_RED        1
>> +#define LED_COLOR_ID_YELLOW     2
>> +#define LED_COLOR_ID_BLUE       3
>> +#define LED_COLOR_ID_AMBER      4
>> +#define LED_COLOR_ID_GREEN      5
>> +#define LED_COLOR_ID_VIOLET     6
>> +#define LED_COLOR_ID_PUPRPLE    7
>> +#define LED_COLOR_ID_ORANGE     8
>> +#define LED_COLOR_ID_PINK       9
>> +#define LED_COLOR_ID_CYAN      10
>> +#define LED_COLOR_ID_LIME      11
> 
> All this is a incompatible bindings change, so shortly ABI break. You
> cannot change defines. Feel free to add new ones.
Yep, I realized this the moment I got your message, I was even thinking 
about it, and figured, 'its internal defines, so should be fine' but of 
course, backwards compatibility with old DT's indeed. In hindsight, this 
probably could have been better as a string in the dt itself instead.

I'll re-spin it, which will make the order a bit wonky, but so be it.

Thanks, and sorry.
> 
> Best regards,
> Krzysztof