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

[Henrik Grimler <henrik@grimler.se>]

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@linaro.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