[PATCH 1/3] dt-bindings: leds: skyworks,aat1290: convert to dtschema

[PATCH 1/3] dt-bindings: leds: skyworks,aat1290: convert to dtschema

[Krzysztof Kozlowski]

Convert the Skyworks Solutions, Inc. AAT1290 Current Regulator bindings
to DT Schema.

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 .../devicetree/bindings/leds/leds-aat1290.txt | 77 ---------------
 .../bindings/leds/skyworks,aat1290.yaml       | 96 +++++++++++++++++++
 2 files changed, 96 insertions(+), 77 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/leds/leds-aat1290.txt
 create mode 100644 Documentation/devicetree/bindings/leds/skyworks,aat1290.yaml

diff --git a/Documentation/devicetree/bindings/leds/leds-aat1290.txt b/Documentation/devicetree/bindings/leds/leds-aat1290.txt
deleted file mode 100644
index 62ed17ec075b..000000000000
--- a/Documentation/devicetree/bindings/leds/leds-aat1290.txt
+++ /dev/null
@@ -1,77 +0,0 @@
-* Skyworks Solutions, Inc. AAT1290 Current Regulator for Flash LEDs
-
-The device is controlled through two pins: FL_EN and EN_SET. The pins when,
-asserted high, enable flash strobe and movie mode (max 1/2 of flash current)
-respectively. In order to add a capability of selecting the strobe signal source
-(e.g. CPU or camera sensor) there is an additional switch required, independent
-of the flash chip. The switch is controlled with pin control.
-
-Required properties:
-
-- compatible : Must be "skyworks,aat1290".
-- flen-gpios : Must be device tree identifier of the flash device FL_EN pin.
-- enset-gpios : Must be device tree identifier of the flash device EN_SET pin.
-
-Optional properties:
-- pinctrl-names : Must contain entries: "default", "host", "isp". Entries
-		"default" and "host" must refer to the same pin configuration
-		node, which sets the host as a strobe signal provider. Entry
-		"isp" must refer to the pin configuration node, which sets the
-		ISP as a strobe signal provider.
-
-A discrete LED element connected to the device must be represented by a child
-node - see Documentation/devicetree/bindings/leds/common.txt.
-
-Required properties of the LED child node:
-- led-max-microamp : see Documentation/devicetree/bindings/leds/common.txt
-- flash-max-microamp : see Documentation/devicetree/bindings/leds/common.txt
-                       Maximum flash LED supply current can be calculated using
-                       following formula: I = 1A * 162kohm / Rset.
-- flash-max-timeout-us : see Documentation/devicetree/bindings/leds/common.txt
-                         Maximum flash timeout can be calculated using following
-                         formula: T = 8.82 * 10^9 * Ct.
-
-Optional properties of the LED child node:
-- function : see Documentation/devicetree/bindings/leds/common.txt
-- color : see Documentation/devicetree/bindings/leds/common.txt
-- label : see Documentation/devicetree/bindings/leds/common.txt (deprecated)
-
-Example (by Ct = 220nF, Rset = 160kohm and exynos4412-trats2 board with
-a switch that allows for routing strobe signal either from the host or from
-the camera sensor):
-
-#include "exynos4412.dtsi"
-#include <dt-bindings/leds/common.h>
-
-led-controller {
-	compatible = "skyworks,aat1290";
-	flen-gpios = <&gpj1 1 GPIO_ACTIVE_HIGH>;
-	enset-gpios = <&gpj1 2 GPIO_ACTIVE_HIGH>;
-
-	pinctrl-names = "default", "host", "isp";
-	pinctrl-0 = <&camera_flash_host>;
-	pinctrl-1 = <&camera_flash_host>;
-	pinctrl-2 = <&camera_flash_isp>;
-
-	camera_flash: led {
-		function = LED_FUNCTION_FLASH;
-		color = <LED_COLOR_ID_WHITE>;
-		led-max-microamp = <520833>;
-		flash-max-microamp = <1012500>;
-		flash-max-timeout-us = <1940000>;
-	};
-};
-
-&pinctrl_0 {
-	camera_flash_host: camera-flash-host {
-		samsung,pins = "gpj1-0";
-		samsung,pin-function = <1>;
-		samsung,pin-val = <0>;
-	};
-
-	camera_flash_isp: camera-flash-isp {
-		samsung,pins = "gpj1-0";
-		samsung,pin-function = <1>;
-		samsung,pin-val = <1>;
-	};
-};
diff --git a/Documentation/devicetree/bindings/leds/skyworks,aat1290.yaml b/Documentation/devicetree/bindings/leds/skyworks,aat1290.yaml
new file mode 100644
index 000000000000..919ee0e30b10
--- /dev/null
+++ b/Documentation/devicetree/bindings/leds/skyworks,aat1290.yaml
@@ -0,0 +1,96 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/leds/skyworks,aat1290.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Skyworks Solutions, Inc. AAT1290 Current Regulator for Flash LEDs
+
+maintainers:
+  - Jacek Anaszewski <jacek.anaszewski@gmail.com>
+  - Krzysztof Kozlowski <krzk@kernel.org>
+
+description: |
+  The device is controlled through two pins:: FL_EN and EN_SET. The pins when,
+  asserted high, enable flash strobe and movie mode (max 1/2 of flash current)
+  respectively. In order to add a capability of selecting the strobe signal
+  source (e.g. CPU or camera sensor) there is an additional switch required,
+  independent of the flash chip. The switch is controlled with pin control.
+
+properties:
+  compatible:
+    const: skyworks,aat1290
+
+  enset-gpios:
+    maxItems: 1
+    description: EN_SET pin
+
+  flen-gpios:
+    maxItems: 1
+    description: FL_EN pin
+
+  led:
+    $ref: common.yaml#
+    unevaluatedProperties: false
+
+    properties:
+      led-max-microamp: true
+
+      flash-max-microamp:
+        description: |
+          Maximum flash LED supply current can be calculated using following
+          formula:: I = 1A * 162 kOhm / Rset.
+
+      flash-max-timeout-us:
+        description: |
+          Maximum flash timeout can be calculated using following formula::
+            T = 8.82 * 10^9 * Ct.
+
+    required:
+      - flash-max-microamp
+      - flash-max-timeout-us
+      - led-max-microamp
+
+  pinctrl-names:
+    items:
+      - const: default
+      - const: host
+      - const: isp
+
+  pinctrl-0: true
+  pinctrl-1: true
+  pinctrl-2: true
+
+required:
+  - compatible
+  - enset-gpios
+  - flen-gpios
+  - led
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+    #include <dt-bindings/leds/common.h>
+
+    // Ct = 220 nF, Rset = 160 kOhm
+    led-controller {
+        compatible = "skyworks,aat1290";
+        flen-gpios = <&gpj1 1 GPIO_ACTIVE_HIGH>;
+        enset-gpios = <&gpj1 2 GPIO_ACTIVE_HIGH>;
+
+        pinctrl-names = "default", "host", "isp";
+        pinctrl-0 = <&camera_flash_host>;
+        pinctrl-1 = <&camera_flash_host>;
+        pinctrl-2 = <&camera_flash_isp>;
+
+        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>;
+        };
+    };
-- 
2.34.1

[PATCH 2/3] ARM: dts: exynos: align aat1290 flash LED node with bindings in Galaxy S3

[Krzysztof Kozlowski]

The bindings expect aat1290 flash LED child node to be named "led".

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 arch/arm/boot/dts/exynos4412-galaxy-s3.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/exynos4412-galaxy-s3.dtsi b/arch/arm/boot/dts/exynos4412-galaxy-s3.dtsi
index 03dffc690b79..72901772fcad 100644
--- a/arch/arm/boot/dts/exynos4412-galaxy-s3.dtsi
+++ b/arch/arm/boot/dts/exynos4412-galaxy-s3.dtsi
@@ -25,7 +25,7 @@ led-controller {
 		pinctrl-1 = <&camera_flash_host>;
 		pinctrl-2 = <&camera_flash_isp>;
 
-		flash-led {
+		led {
 			label = "flash";
 			led-max-microamp = <520833>;
 			flash-max-microamp = <1012500>;
-- 
2.34.1

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

[Krzysztof Kozlowski]

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>;
 			led-max-microamp = <520833>;
 			flash-max-microamp = <1012500>;
 			flash-max-timeout-us = <1940000>;
-- 
2.34.1

Re: [PATCH 1/3] dt-bindings: leds: skyworks,aat1290: convert to dtschema

[Rob Herring]

On Tue, 07 Jun 2022 10:53:41 +0200, Krzysztof Kozlowski wrote:
> Convert the Skyworks Solutions, Inc. AAT1290 Current Regulator bindings
> to DT Schema.
> 
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> ---
>  .../devicetree/bindings/leds/leds-aat1290.txt | 77 ---------------
>  .../bindings/leds/skyworks,aat1290.yaml       | 96 +++++++++++++++++++
>  2 files changed, 96 insertions(+), 77 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/leds/leds-aat1290.txt
>  create mode 100644 Documentation/devicetree/bindings/leds/skyworks,aat1290.yaml
> 

Reviewed-by: Rob Herring <robh@kernel.org>

Re: [PATCH 1/3] dt-bindings: leds: skyworks,aat1290: convert to dtschema

[Jacek Anaszewski]

Hi Krzysztof,

On 6/7/22 10:53, Krzysztof Kozlowski wrote:
> Convert the Skyworks Solutions, Inc. AAT1290 Current Regulator bindings
> to DT Schema.
> 
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
[...]
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/gpio/gpio.h>
> +    #include <dt-bindings/leds/common.h>
> +
> +    // Ct = 220 nF, Rset = 160 kOhm
> +    led-controller {
> +        compatible = "skyworks,aat1290";
> +        flen-gpios = <&gpj1 1 GPIO_ACTIVE_HIGH>;
> +        enset-gpios = <&gpj1 2 GPIO_ACTIVE_HIGH>;
> +
> +        pinctrl-names = "default", "host", "isp";
> +        pinctrl-0 = <&camera_flash_host>;
> +        pinctrl-1 = <&camera_flash_host>;
> +        pinctrl-2 = <&camera_flash_isp>;
> +
> +        led {
> +            label = "flash";

Why are you adding label? It is deprecated, but has the precedence over
new function and color for backwards compatibility, so it would make
those unused by the driver now. Please drop the label from this example.

> +            function = LED_FUNCTION_FLASH;
> +            color = <LED_COLOR_ID_WHITE>;
> +            led-max-microamp = <520833>;
> +            flash-max-microamp = <1012500>;
> +            flash-max-timeout-us = <1940000>;
> +        };
> +    };

-- 
Best regards,
Jacek Anaszewski

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

[Jacek Anaszewski]

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

-- 
Best regards,
Jacek Anaszewski

Re: [PATCH 1/3] dt-bindings: leds: skyworks,aat1290: convert to dtschema

[Krzysztof Kozlowski]

On 09/06/2022 22:28, Jacek Anaszewski wrote:
> Hi Krzysztof,
> 
> On 6/7/22 10:53, Krzysztof Kozlowski wrote:
>> Convert the Skyworks Solutions, Inc. AAT1290 Current Regulator bindings
>> to DT Schema.
>>
>> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> [...]
>> +additionalProperties: false
>> +
>> +examples:
>> +  - |
>> +    #include <dt-bindings/gpio/gpio.h>
>> +    #include <dt-bindings/leds/common.h>
>> +
>> +    // Ct = 220 nF, Rset = 160 kOhm
>> +    led-controller {
>> +        compatible = "skyworks,aat1290";
>> +        flen-gpios = <&gpj1 1 GPIO_ACTIVE_HIGH>;
>> +        enset-gpios = <&gpj1 2 GPIO_ACTIVE_HIGH>;
>> +
>> +        pinctrl-names = "default", "host", "isp";
>> +        pinctrl-0 = <&camera_flash_host>;
>> +        pinctrl-1 = <&camera_flash_host>;
>> +        pinctrl-2 = <&camera_flash_isp>;
>> +
>> +        led {
>> +            label = "flash";
> 
> Why are you adding label? It is deprecated, 

Eh, so it should be marked as deprecated:true, not just mentioned in the
description (common.yaml).

> but has the precedence over
> new function and color for backwards compatibility, so it would make
> those unused by the driver now. Please drop the label from this example.

I synced the example with DTS, but I can drop it. No problem.


Best regards,
Krzysztof

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

[Krzysztof Kozlowski]

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

Re: [PATCH 1/3] dt-bindings: leds: skyworks,aat1290: convert to dtschema

[Jacek Anaszewski]

On 6/10/22 12:12, Krzysztof Kozlowski wrote:
> On 09/06/2022 22:28, Jacek Anaszewski wrote:
>> Hi Krzysztof,
>>
>> On 6/7/22 10:53, Krzysztof Kozlowski wrote:
>>> Convert the Skyworks Solutions, Inc. AAT1290 Current Regulator bindings
>>> to DT Schema.
>>>
>>> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>> [...]
>>> +additionalProperties: false
>>> +
>>> +examples:
>>> +  - |
>>> +    #include <dt-bindings/gpio/gpio.h>
>>> +    #include <dt-bindings/leds/common.h>
>>> +
>>> +    // Ct = 220 nF, Rset = 160 kOhm
>>> +    led-controller {
>>> +        compatible = "skyworks,aat1290";
>>> +        flen-gpios = <&gpj1 1 GPIO_ACTIVE_HIGH>;
>>> +        enset-gpios = <&gpj1 2 GPIO_ACTIVE_HIGH>;
>>> +
>>> +        pinctrl-names = "default", "host", "isp";
>>> +        pinctrl-0 = <&camera_flash_host>;
>>> +        pinctrl-1 = <&camera_flash_host>;
>>> +        pinctrl-2 = <&camera_flash_isp>;
>>> +
>>> +        led {
>>> +            label = "flash";
>>
>> Why are you adding label? It is deprecated,
> 
> Eh, so it should be marked as deprecated:true, not just mentioned in the
> description (common.yaml).

I believe so.

>> but has the precedence over
>> new function and color for backwards compatibility, so it would make
>> those unused by the driver now. Please drop the label from this example.
> 
> I synced the example with DTS, but I can drop it. No problem.

Yeah, let's avoid further confusion.

-- 
Best regards,
Jacek Anaszewski

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

[Jacek Anaszewski]

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.

-- 
Best regards,
Jacek Anaszewski

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

[Krzysztof Kozlowski]

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

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

[Henrik Grimler]

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

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

[Denis 'GNUtoo' Carikli]

[-- Attachment #1: Type: text/plain, Size: 2417 bytes --]

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.

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]