linux-kernel - Re: [PATCH 3/3] ARM: dts: exynos: add function and color to aat1290 flash LED node in Galaxy S3

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: <ac1f956bece0151a165afc18ec21898d00dacf31.camel@grimler.se>
Date:   Sat, 18 Jun 2022 23:14:53 +0200
From:   Henrik Grimler <henrik@...mler.se>
To:     Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>,
        Jacek Anaszewski <jacek.anaszewski@...il.com>
Cc:     replicant@...osl.org, Pavel Machek <pavel@....cz>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Alim Akhtar <alim.akhtar@...sung.com>,
        linux-leds@...r.kernel.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
        linux-samsung-soc@...r.kernel.org,
        Simon Shields <simon@...eageos.org>,
        Martin Jücker <martin.juecker@...il.com>
Subject: Re: [PATCH 3/3] ARM: dts: exynos: add function and color to aat1290
 flash LED node in Galaxy S3

Hi Krzysztof and Jacek,

On Sun, 2022-06-12 at 19:06 +0200, Krzysztof Kozlowski wrote:
> On 12/06/2022 17:09, Jacek Anaszewski wrote:
> > On 6/10/22 12:14, Krzysztof Kozlowski wrote:
> > > On 09/06/2022 22:31, Jacek Anaszewski wrote:
> > > > Hi Krzysztof,
> > > > 
> > > > On 6/7/22 10:53, Krzysztof Kozlowski wrote:
> > > > > Add common LED properties - the function and color - to
> > > > > aat1290 flash
> > > > > LED node in Galaxy S3.
> > > > > 
> > > > > Signed-off-by: Krzysztof Kozlowski
> > > > > <krzysztof.kozlowski@...aro.org>
> > > > > ---
> > > > >    arch/arm/boot/dts/exynos4412-galaxy-s3.dtsi | 3 +++
> > > > >    1 file changed, 3 insertions(+)
> > > > > 
> > > > > diff --git a/arch/arm/boot/dts/exynos4412-galaxy-s3.dtsi
> > > > > b/arch/arm/boot/dts/exynos4412-galaxy-s3.dtsi
> > > > > index 72901772fcad..d76f3678dcab 100644
> > > > > --- a/arch/arm/boot/dts/exynos4412-galaxy-s3.dtsi
> > > > > +++ b/arch/arm/boot/dts/exynos4412-galaxy-s3.dtsi
> > > > > @@ -7,6 +7,7 @@
> > > > >     */
> > > > >    
> > > > >    /dts-v1/;
> > > > > +#include <dt-bindings/leds/common.h>
> > > > >    #include "exynos4412-midas.dtsi"
> > > > >    
> > > > >    / {
> > > > > @@ -27,6 +28,8 @@ led-controller {
> > > > >    
> > > > >                 led {
> > > > >                         label = "flash";
> > > > > +                       function = LED_FUNCTION_FLASH;
> > > > > +                       color = <LED_COLOR_ID_WHITE>;
> > > > 
> > > > Addition of these two properties will not change anything
> > > > because
> > > > the label has precedence. It is deprecated, but if you
> > > > introduce
> > > > function and color to the binding instead of the label, the
> > > > resulting
> > > > LED class device name will change.
> > > 
> > > Which is not necessarily what we want, right? Adding these
> > > properties is
> > > a proper description of hardware, regardless whether current
> > > Linux
> > > implementation uses them or not.
> > 
> > Actually I'd just drop label in addition to your change.
> > I don't think it would break anybody seriously - not expecting it
> > has
> > any larger group of users and having uniformly constructed DTS
> > files
> > in the mainline has greater value.
> > 
> 
> What about some PostmarketOSos, LineageOS and other OSes?
> 
> Let me Cc here some folks - Simon, Martin, is the label in flash LED
> node anyhow important for you? Can it be dropped and replaced with
> function+color?
> 

As far as I know LineageOS does not use a mainline-based kernel for the
S3. PostmarketOS and Replicant does though. For PostmarketOS it should
be fine to drop the label, and it sounded like it should be fine for
Replicant also in an IRC discussion, but adding their mailing list to
CC just in case.


Best regards,
Henrik Grimler