| Message ID | 20220607085343.72414-3-krzysztof.kozlowski@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [1/3] dt-bindings: leds: skyworks,aat1290: convert to dtschema | expand |
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. > led-max-microamp = <520833>; > flash-max-microamp = <1012500>; > flash-max-timeout-us = <1940000>;
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. Best regards, Krzysztof
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.
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? https://lore.kernel.org/all/20220607085343.72414-3-krzysztof.kozlowski@linaro.org/ Best regards, Krzysztof
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
On Sun, 12 Jun 2022 19:06:09 +0200 Krzysztof Kozlowski via Replicant <replicant@osuosl.org> 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? We don't have flash or camera support yet with Replicant version(s) that use kernel(s) based on upstream Linux, so it won't break anything. Denis.
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>; led-max-microamp = <520833>; flash-max-microamp = <1012500>; flash-max-timeout-us = <1940000>;
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(+)