[PATCH v10 4/6] dts-bindings: leds: Document the naming requirement for LED properties

[PATCH v10 4/6] dts-bindings: leds: Document the naming requirement for LED properties

[Jean-Jacques Hiblot]

LED properties must be named "leds" in the same way that PWM, clocks or
PHY properties are names respectively "pwms", "clocks" and "phys".

Cc: devicetree@vger.kernel.org
Signed-off-by: Jean-Jacques Hiblot <jjhiblot@ti.com>
---
 .../devicetree/bindings/leds/common.txt       | 20 ++++++++++++++++---
 1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/leds/common.txt b/Documentation/devicetree/bindings/leds/common.txt
index 9fa6f9795d50..31b8c1f68d27 100644
--- a/Documentation/devicetree/bindings/leds/common.txt
+++ b/Documentation/devicetree/bindings/leds/common.txt
@@ -10,6 +10,9 @@ can influence the way of the LED device initialization, the LED components
 have to be tightly coupled with the LED device binding. They are represented
 by child nodes of the parent LED device binding.
 
+LED properties should be named "leds". The exact meaning of each leds
+property must be documented in the device tree binding for each device.
+
 
 Optional properties for child nodes:
 - led-sources : List of device current outputs the LED is connected to. The
@@ -165,9 +168,20 @@ led-controller@30 {
 		function-enumerator = <2>;
         };
 
-        led@3 {
+        bkl_led0: led@3 {
 		reg = <3>;
-		function = LED_FUNCTION_INDICATOR;
-		function-enumerator = <3>;
+		function = LED_FUNCTION_BACKLIGHT;
+		function-enumerator = <1>;
         };
+
+        bkl_led1: led@4 {
+		reg = <4>;
+		function = LED_FUNCTION_BACKLIGHT;
+		function-enumerator = <2>;
+        };
+};
+
+
+backlight@40 {
+	leds = <&bkl_led0> , <&bkl_led1>;
 };
-- 
2.17.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v10 4/6] dts-bindings: leds: Document the naming requirement for LED properties

[Rob Herring]

On Wed, Oct 09, 2019 at 10:51:25AM +0200, Jean-Jacques Hiblot wrote:
> LED properties must be named "leds" in the same way that PWM, clocks or
> PHY properties are names respectively "pwms", "clocks" and "phys".
> 
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Jean-Jacques Hiblot <jjhiblot@ti.com>
> ---
>  .../devicetree/bindings/leds/common.txt       | 20 ++++++++++++++++---
>  1 file changed, 17 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/leds/common.txt b/Documentation/devicetree/bindings/leds/common.txt
> index 9fa6f9795d50..31b8c1f68d27 100644
> --- a/Documentation/devicetree/bindings/leds/common.txt
> +++ b/Documentation/devicetree/bindings/leds/common.txt
> @@ -10,6 +10,9 @@ can influence the way of the LED device initialization, the LED components
>  have to be tightly coupled with the LED device binding. They are represented
>  by child nodes of the parent LED device binding.
>  
> +LED properties should be named "leds". The exact meaning of each leds
> +property must be documented in the device tree binding for each device.
> +

This is worded oddly. The property is 'leds' and it is always a list of 
phandles to LED device nodes. It is present in an LED consumer device.

>  
>  Optional properties for child nodes:
>  - led-sources : List of device current outputs the LED is connected to. The
> @@ -165,9 +168,20 @@ led-controller@30 {
>  		function-enumerator = <2>;
>          };
>  
> -        led@3 {
> +        bkl_led0: led@3 {
>  		reg = <3>;
> -		function = LED_FUNCTION_INDICATOR;
> -		function-enumerator = <3>;
> +		function = LED_FUNCTION_BACKLIGHT;
> +		function-enumerator = <1>;
>          };
> +
> +        bkl_led1: led@4 {
> +		reg = <4>;
> +		function = LED_FUNCTION_BACKLIGHT;
> +		function-enumerator = <2>;
> +        };
> +};
> +
> +
> +backlight@40 {

Either needs 'reg' or the unit-address dropped.

> +	leds = <&bkl_led0> , <&bkl_led1>;

drop the space            ^

>  };
> -- 
> 2.17.1
> 
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v10 4/6] dts-bindings: leds: Document the naming requirement for LED properties

[Jean-Jacques Hiblot]

On 09/10/2019 21:26, Rob Herring wrote:
> On Wed, Oct 09, 2019 at 10:51:25AM +0200, Jean-Jacques Hiblot wrote:
>> LED properties must be named "leds" in the same way that PWM, clocks or
>> PHY properties are names respectively "pwms", "clocks" and "phys".
>>
>> Cc: devicetree@vger.kernel.org
>> Signed-off-by: Jean-Jacques Hiblot <jjhiblot@ti.com>
>> ---
>>   .../devicetree/bindings/leds/common.txt       | 20 ++++++++++++++++---
>>   1 file changed, 17 insertions(+), 3 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/leds/common.txt b/Documentation/devicetree/bindings/leds/common.txt
>> index 9fa6f9795d50..31b8c1f68d27 100644
>> --- a/Documentation/devicetree/bindings/leds/common.txt
>> +++ b/Documentation/devicetree/bindings/leds/common.txt
>> @@ -10,6 +10,9 @@ can influence the way of the LED device initialization, the LED components
>>   have to be tightly coupled with the LED device binding. They are represented
>>   by child nodes of the parent LED device binding.
>>   
>> +LED properties should be named "leds". The exact meaning of each leds
>> +property must be documented in the device tree binding for each device.
>> +
> This is worded oddly. The property is 'leds' and it is always a list of
> phandles to LED device nodes. It is present in an LED consumer device.

How about:

[...]

A LED consumer device has a 'leds' property. This property is always a list
of phandles to LED nodes (child node of a LED device node).

led_device {
     ...

     led0: led@0 {
         ...
     };

     led1: led@1 {
         ...
     };
};

consumer {
     ...
     leds = <led0>, <led1>;
};

>
>>   
>>   Optional properties for child nodes:
>>   - led-sources : List of device current outputs the LED is connected to. The
>> @@ -165,9 +168,20 @@ led-controller@30 {
>>   		function-enumerator = <2>;
>>           };
>>   
>> -        led@3 {
>> +        bkl_led0: led@3 {
>>   		reg = <3>;
>> -		function = LED_FUNCTION_INDICATOR;
>> -		function-enumerator = <3>;
>> +		function = LED_FUNCTION_BACKLIGHT;
>> +		function-enumerator = <1>;
>>           };
>> +
>> +        bkl_led1: led@4 {
>> +		reg = <4>;
>> +		function = LED_FUNCTION_BACKLIGHT;
>> +		function-enumerator = <2>;
>> +        };
>> +};
>> +
>> +
>> +backlight@40 {
> Either needs 'reg' or the unit-address dropped.
>
>> +	leds = <&bkl_led0> , <&bkl_led1>;
> drop the space            ^
>
>>   };
>> -- 
>> 2.17.1
>>

[PATCH v10 5/6] dt-bindings: backlight: Add led-backlight binding

[Jean-Jacques Hiblot]

Add DT binding for led-backlight.

Cc: devicetree@vger.kernel.org
Signed-off-by: Jean-Jacques Hiblot <jjhiblot@ti.com>

---

.../leds/backlight/led-backlight.yaml         | 55 +++++++++++++++++++
 1 file changed, 55 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/leds/backlight/led-backlight.yaml

diff --git a/Documentation/devicetree/bindings/leds/backlight/led-backlight.yaml b/Documentation/devicetree/bindings/leds/backlight/led-backlight.yaml
new file mode 100644
index 000000000000..47ae3d6b4f31
--- /dev/null
+++ b/Documentation/devicetree/bindings/leds/backlight/led-backlight.yaml
@@ -0,0 +1,55 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/leds/backlight/led-backlight.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: LED-based Backlight Device Tree Bindings
+
+maintainers:
+  - Jean-Jacques Hiblot <jjhiblot@ti.com>
+
+description: |
+  This binding is used to describe a basic backlight device made of LEDs.
+  It can also be used to describe a backlight device controlled by the
+  output of a LED driver.
+
+properties:
+  compatible:
+    const: led-backlight
+
+  leds:
+    $ref: /schemas/types.yaml#/definitions/phandle-array
+    description: List of LEDs constitutive of the backlight.
+
+  default-brightness:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: Default brightness level on boot.
+    minimum: 0
+
+  brightness-levels:
+    $ref: /schemas/types.yaml#/definitions/uint32-array
+    description: Array of distinct brightness levels. The levels must be in
+      the range accepted by the underlying LED devices. This is used to
+      translate a backlight brightness level into a LED brightness level. If
+      it is not provided, the identity mapping is used.
+
+required:
+  - compatible
+  - backlight-leds
+
+examples:
+  - |
+    backlight {
+      compatible = "led-backlight";
+      backlight-leds = <&led1>;
+      brightness-levels = <0 4 8 16 32 64 128 255>;
+      default-brightness = <6>; /*6th level => brightness of the LEDs is 128*/
+    };
+  - |
+    backlight {
+      compatible = "led-backlight";
+      backlight-leds = <&led1>, <&led2>;
+      default-brightness = <255>;
+    };
+...
-- 
2.17.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v10 5/6] dt-bindings: backlight: Add led-backlight binding

[Rob Herring]

On Wed, Oct 09, 2019 at 10:51:26AM +0200, Jean-Jacques Hiblot wrote:
> Add DT binding for led-backlight.
> 
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Jean-Jacques Hiblot <jjhiblot@ti.com>
> 
> ---
> 
> .../leds/backlight/led-backlight.yaml         | 55 +++++++++++++++++++
>  1 file changed, 55 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/leds/backlight/led-backlight.yaml
> 
> diff --git a/Documentation/devicetree/bindings/leds/backlight/led-backlight.yaml b/Documentation/devicetree/bindings/leds/backlight/led-backlight.yaml
> new file mode 100644
> index 000000000000..47ae3d6b4f31
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/leds/backlight/led-backlight.yaml
> @@ -0,0 +1,55 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/leds/backlight/led-backlight.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: LED-based Backlight Device Tree Bindings
> +
> +maintainers:
> +  - Jean-Jacques Hiblot <jjhiblot@ti.com>
> +
> +description: |
> +  This binding is used to describe a basic backlight device made of LEDs.
> +  It can also be used to describe a backlight device controlled by the
> +  output of a LED driver.
> +
> +properties:
> +  compatible:
> +    const: led-backlight
> +
> +  leds:
> +    $ref: /schemas/types.yaml#/definitions/phandle-array
> +    description: List of LEDs constitutive of the backlight.
> +
> +  default-brightness:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: Default brightness level on boot.

It's not clear that this is an index when 'brightness-levels' is present 
and absolute level when not. I wonder if we've been consistent on that?

> +    minimum: 0

Implied by being a uint32.

> +
> +  brightness-levels:
> +    $ref: /schemas/types.yaml#/definitions/uint32-array
> +    description: Array of distinct brightness levels. The levels must be in
> +      the range accepted by the underlying LED devices. This is used to
> +      translate a backlight brightness level into a LED brightness level. If
> +      it is not provided, the identity mapping is used.
> +
> +required:
> +  - compatible
> +  - backlight-leds

leds?


Add 'additionalProperties: false' here.

> +
> +examples:
> +  - |
> +    backlight {

Please also define $nodename is 'backlight'.

> +      compatible = "led-backlight";
> +      backlight-leds = <&led1>;

leds?

> +      brightness-levels = <0 4 8 16 32 64 128 255>;
> +      default-brightness = <6>; /*6th level => brightness of the LEDs is 128*/
> +    };
> +  - |
> +    backlight {
> +      compatible = "led-backlight";
> +      backlight-leds = <&led1>, <&led2>;
> +      default-brightness = <255>;
> +    };
> +...
> -- 
> 2.17.1
> 

Re: [PATCH v10 5/6] dt-bindings: backlight: Add led-backlight binding

[Tony Lindgren]

Hi,

* Rob Herring <robh@kernel.org> [700101 00:00]:
> On Wed, Oct 09, 2019 at 10:51:26AM +0200, Jean-Jacques Hiblot wrote:
> > Add DT binding for led-backlight.
...
> > new file mode 100644
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/leds/backlight/led-backlight.yaml
...

> > +  default-brightness:
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    description: Default brightness level on boot.
> 
> It's not clear that this is an index when 'brightness-levels' is present 
> and absolute level when not. I wonder if we've been consistent on that?

Yeah.. And should we use "default-brightness-level" here like we do
in the kernel now?

Sorry if I've missed some discussion on this before..

> > +      brightness-levels = <0 4 8 16 32 64 128 255>;

What we're using for droid4 with an earlier version of this
patch set for the brightness-levels is generated backwards
with:

$ for i in 0 1 2 3 4 5 6 7; do echo "255 - ${i} * (256 / 8)" | bc; done

This produces the following range that seem to behave nicely:

brightness-levels = <31 63 95 127 159 191 223 255>;

Of course depends on the backing hardware, this is with
leds_lm3532 on droid4. But I think also the current example
in the binding might be from Pavel also for droid4?

If so, you might want to update the range :)

Regards,

Tony