diff mbox series

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

Message ID 20220607085343.72414-3-krzysztof.kozlowski@linaro.org
State New
Headers show
Series None | expand

Commit Message

Krzysztof Kozlowski June 7, 2022, 8:53 a.m. UTC
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(+)

Comments

Jacek Anaszewski June 9, 2022, 8:31 p.m. UTC | #1
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>;
Krzysztof Kozlowski June 10, 2022, 10:14 a.m. UTC | #2
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
Henrik Grimler June 18, 2022, 9:14 p.m. UTC | #3
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
diff mbox series

Patch

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>;