diff mbox series

dt-bindings: backlight: Convert common backlight bindings to DT schema

Message ID 20200618224413.1115849-1-robh@kernel.org
State Superseded
Headers show
Series dt-bindings: backlight: Convert common backlight bindings to DT schema | expand

Checks

Context Check Description
robh/dt-meta-schema success
robh/checkpatch warning total: 0 errors, 9 warnings, 197 lines checked

Commit Message

Rob Herring June 18, 2020, 10:44 p.m. UTC
Convert the common GPIO, LED, and PWM backlight bindings to DT schema
format.

Given there's only 2 common properties and the descriptions are slightly
different, I opted to not create a common backlight schema.

Cc: Lee Jones <lee.jones@linaro.org>
Cc: Daniel Thompson <daniel.thompson@linaro.org>
Cc: Jingoo Han <jingoohan1@gmail.com>
Signed-off-by: Rob Herring <robh@kernel.org>
---
 .../leds/backlight/gpio-backlight.txt         | 16 ---
 .../leds/backlight/gpio-backlight.yaml        | 41 ++++++++
 .../bindings/leds/backlight/led-backlight.txt | 28 ------
 .../leds/backlight/led-backlight.yaml         | 58 +++++++++++
 .../bindings/leds/backlight/pwm-backlight.txt | 61 ------------
 .../leds/backlight/pwm-backlight.yaml         | 98 +++++++++++++++++++
 6 files changed, 197 insertions(+), 105 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/leds/backlight/gpio-backlight.txt
 create mode 100644 Documentation/devicetree/bindings/leds/backlight/gpio-backlight.yaml
 delete mode 100644 Documentation/devicetree/bindings/leds/backlight/led-backlight.txt
 create mode 100644 Documentation/devicetree/bindings/leds/backlight/led-backlight.yaml
 delete mode 100644 Documentation/devicetree/bindings/leds/backlight/pwm-backlight.txt
 create mode 100644 Documentation/devicetree/bindings/leds/backlight/pwm-backlight.yaml

Comments

Sam Ravnborg June 19, 2020, 9:53 p.m. UTC | #1
Hi Rob.

Good to have these converted. A few comments in the following. One
comment is for the backlight people as you copied the original text.

	Sam

On Thu, Jun 18, 2020 at 04:44:13PM -0600, Rob Herring wrote:
> Convert the common GPIO, LED, and PWM backlight bindings to DT schema
> format.
> 
> Given there's only 2 common properties and the descriptions are slightly
> different, I opted to not create a common backlight schema.
> 
> Cc: Lee Jones <lee.jones@linaro.org>
> Cc: Daniel Thompson <daniel.thompson@linaro.org>
> Cc: Jingoo Han <jingoohan1@gmail.com>
> Signed-off-by: Rob Herring <robh@kernel.org>
> ---
>  .../leds/backlight/gpio-backlight.txt         | 16 ---
>  .../leds/backlight/gpio-backlight.yaml        | 41 ++++++++
>  .../bindings/leds/backlight/led-backlight.txt | 28 ------
>  .../leds/backlight/led-backlight.yaml         | 58 +++++++++++
>  .../bindings/leds/backlight/pwm-backlight.txt | 61 ------------
>  .../leds/backlight/pwm-backlight.yaml         | 98 +++++++++++++++++++
>  6 files changed, 197 insertions(+), 105 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/leds/backlight/gpio-backlight.txt
>  create mode 100644 Documentation/devicetree/bindings/leds/backlight/gpio-backlight.yaml
>  delete mode 100644 Documentation/devicetree/bindings/leds/backlight/led-backlight.txt
>  create mode 100644 Documentation/devicetree/bindings/leds/backlight/led-backlight.yaml
>  delete mode 100644 Documentation/devicetree/bindings/leds/backlight/pwm-backlight.txt
>  create mode 100644 Documentation/devicetree/bindings/leds/backlight/pwm-backlight.yaml
> 
> diff --git a/Documentation/devicetree/bindings/leds/backlight/gpio-backlight.txt b/Documentation/devicetree/bindings/leds/backlight/gpio-backlight.txt
> deleted file mode 100644
> index 321be6640533..000000000000
> --- a/Documentation/devicetree/bindings/leds/backlight/gpio-backlight.txt
> +++ /dev/null
> @@ -1,16 +0,0 @@
> -gpio-backlight bindings
> -
> -Required properties:
> -  - compatible: "gpio-backlight"
> -  - gpios: describes the gpio that is used for enabling/disabling the backlight.
> -    refer to bindings/gpio/gpio.txt for more details.
> -
> -Optional properties:
> -  - default-on: enable the backlight at boot.
> -
> -Example:
> -	backlight {
> -		compatible = "gpio-backlight";
> -		gpios = <&gpio3 4 GPIO_ACTIVE_HIGH>;
> -		default-on;
> -	};
> diff --git a/Documentation/devicetree/bindings/leds/backlight/gpio-backlight.yaml b/Documentation/devicetree/bindings/leds/backlight/gpio-backlight.yaml
> new file mode 100644
> index 000000000000..75cc569b9c55
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/leds/backlight/gpio-backlight.yaml
> @@ -0,0 +1,41 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/leds/backlight/gpio-backlight.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: gpio-backlight bindings
> +
> +maintainers:
> +  - Lee Jones <lee.jones@linaro.org>
> +  - Daniel Thompson <daniel.thompson@linaro.org>
> +  - Jingoo Han <jingoohan1@gmail.com>
> +
> +properties:
> +  compatible:
> +    const: gpio-backlight
> +
> +  gpios:
> +    description: The gpio that is used for enabling/disabling the backlight.
> +    maxItems: 1
> +
> +  default-on:
> +    description: enable the backlight at boot.
> +    type: boolean
> +
> +required:
> +  - compatible
> +  - gpios
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/gpio/gpio.h>
> +    backlight {
> +        compatible = "gpio-backlight";
> +        gpios = <&gpio3 4 GPIO_ACTIVE_HIGH>;
> +        default-on;
> +    };
> +
> +...
> diff --git a/Documentation/devicetree/bindings/leds/backlight/led-backlight.txt b/Documentation/devicetree/bindings/leds/backlight/led-backlight.txt
> deleted file mode 100644
> index 4c7dfbe7f67a..000000000000
> --- a/Documentation/devicetree/bindings/leds/backlight/led-backlight.txt
> +++ /dev/null
> @@ -1,28 +0,0 @@
> -led-backlight bindings
> -
> -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.
> -
> -Required properties:
> -  - compatible: "led-backlight"
> -  - leds: a list of LEDs
> -
> -Optional properties:
> -  - brightness-levels: 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.
> -
> -  - default-brightness-level: The default brightness level.
> -
> -Example:
> -
> -	backlight {
> -		compatible = "led-backlight";
> -
> -		leds = <&led1>, <&led2>;
> -		brightness-levels = <0 4 8 16 32 64 128 255>;
> -		default-brightness-level = <6>;
> -	};
> 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..ae50945d2798
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/leds/backlight/led-backlight.yaml
> @@ -0,0 +1,58 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/leds/backlight/led-backlight.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: led-backlight bindings
> +
> +maintainers:
> +  - Lee Jones <lee.jones@linaro.org>
> +  - Daniel Thompson <daniel.thompson@linaro.org>
> +  - Jingoo Han <jingoohan1@gmail.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:
> +    description: A list of LED nodes
> +    $ref: /schemas/types.yaml#/definitions/phandle-array
> +
> +  brightness-levels:
> +    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.
> +    $ref: /schemas/types.yaml#/definitions/uint32-array
bike-shedding. To me it is a tad easier to read when multi-line
descriptions are on a separate line.
So "description:" on one line, and the text on following lines.
example-schema.yaml does both - so both are official acceptable.

> +
> +  default-brightness-level:
> +    description: The default brightness level (index into the array defined
> +      by the "brightness-levels" property).
This description does not match my understading.
The description says "index into", but in reality this is a value that
matches somewhere in the range specified by brightness-levels.
So it is not an index.

Maybe I just read it wrong and the description is fine. But when I read
index the when it says 6 I would look for brightness-levels[6] equals
128 in the example below.
And this is not how it is coded.

> +    $ref: /schemas/types.yaml#/definitions/uint32
> +
> +dependencies:
> +  default-brightness-level: [brightness-levels]
So if we have efault-brightness-level then we must have
brightness-levels.
Sounds right.

> +
> +required:
> +  - compatible
> +  - leds
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    backlight {
> +        compatible = "led-backlight";
> +
> +        leds = <&led1>, <&led2>;
> +        brightness-levels = <0 4 8 16 32 64 128 255>;
> +        default-brightness-level = <6>;
> +    };
> +
> +...
> diff --git a/Documentation/devicetree/bindings/leds/backlight/pwm-backlight.txt b/Documentation/devicetree/bindings/leds/backlight/pwm-backlight.txt
> deleted file mode 100644
> index 64fa2fbd98c9..000000000000
> --- a/Documentation/devicetree/bindings/leds/backlight/pwm-backlight.txt
> +++ /dev/null
> @@ -1,61 +0,0 @@
> -pwm-backlight bindings
> -
> -Required properties:
> -  - compatible: "pwm-backlight"
> -  - pwms: OF device-tree PWM specification (see PWM binding[0])
> -  - power-supply: regulator for supply voltage
> -
> -Optional properties:
> -  - pwm-names: a list of names for the PWM devices specified in the
> -               "pwms" property (see PWM binding[0])
> -  - enable-gpios: contains a single GPIO specifier for the GPIO which enables
> -                  and disables the backlight (see GPIO binding[1])
> -  - post-pwm-on-delay-ms: Delay in ms between setting an initial (non-zero) PWM
> -                          and enabling the backlight using GPIO.
> -  - pwm-off-delay-ms: Delay in ms between disabling the backlight using GPIO
> -                      and setting PWM value to 0.
> -  - brightness-levels: Array of distinct brightness levels. Typically these
> -                       are in the range from 0 to 255, but any range starting at
> -                       0 will do. The actual brightness level (PWM duty cycle)
> -                       will be interpolated from these values. 0 means a 0% duty
> -                       cycle (darkest/off), while the last value in the array
> -                       represents a 100% duty cycle (brightest).
> -  - default-brightness-level: The default brightness level (index into the
> -                              array defined by the "brightness-levels" property).
> -  - num-interpolated-steps: Number of interpolated steps between each value
> -                            of brightness-levels table. This way a high
> -                            resolution pwm duty cycle can be used without
> -                            having to list out every possible value in the
> -                            brightness-level array.
> -
> -[0]: Documentation/devicetree/bindings/pwm/pwm.txt
> -[1]: Documentation/devicetree/bindings/gpio/gpio.txt
> -
> -Example:
> -
> -	backlight {
> -		compatible = "pwm-backlight";
> -		pwms = <&pwm 0 5000000>;
> -
> -		brightness-levels = <0 4 8 16 32 64 128 255>;
> -		default-brightness-level = <6>;
> -
> -		power-supply = <&vdd_bl_reg>;
> -		enable-gpios = <&gpio 58 0>;
> -		post-pwm-on-delay-ms = <10>;
> -		pwm-off-delay-ms = <10>;
> -	};
> -
> -Example using num-interpolation-steps:
> -
> -	backlight {
> -		compatible = "pwm-backlight";
> -		pwms = <&pwm 0 5000000>;
> -
> -		brightness-levels = <0 2048 4096 8192 16384 65535>;
> -		num-interpolated-steps = <2048>;
> -		default-brightness-level = <4096>;
> -
> -		power-supply = <&vdd_bl_reg>;
> -		enable-gpios = <&gpio 58 0>;
> -	};
> diff --git a/Documentation/devicetree/bindings/leds/backlight/pwm-backlight.yaml b/Documentation/devicetree/bindings/leds/backlight/pwm-backlight.yaml
> new file mode 100644
> index 000000000000..7e1f109a38a4
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/leds/backlight/pwm-backlight.yaml
> @@ -0,0 +1,98 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/leds/backlight/pwm-backlight.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: pwm-backlight bindings
> +
> +maintainers:
> +  - Lee Jones <lee.jones@linaro.org>
> +  - Daniel Thompson <daniel.thompson@linaro.org>
> +  - Jingoo Han <jingoohan1@gmail.com>
> +
> +properties:
> +  compatible:
> +    const: pwm-backlight
> +
> +  pwms:
> +    maxItems: 1
> +
> +  pwm-names: true
> +
> +  power-supply:
> +    description: regulator for supply voltage
> +
> +  enable-gpios:
> +    description: Contains a single GPIO specifier for the GPIO which enables
> +      and disables the backlight
> +    maxItems: 1
> +
> +  post-pwm-on-delay-ms:
> +    description: Delay in ms between setting an initial (non-zero) PWM and
> +      enabling the backlight using GPIO.
> +
> +  pwm-off-delay-ms:
> +    description: Delay in ms between disabling the backlight using GPIO
> +      and setting PWM value to 0.
> +
> +  brightness-levels:
> +    description: Array of distinct brightness levels. Typically these are
> +      in the range from 0 to 255, but any range starting at 0 will do. The
> +      actual brightness level (PWM duty cycle) will be interpolated from
> +      these values. 0 means a 0% duty cycle (darkest/off), while the last
> +      value in the array represents a 100% duty cycle (brightest).
> +    $ref: /schemas/types.yaml#/definitions/uint32-array
> +
> +  default-brightness-level:
> +    description: The default brightness level (index into the array defined
> +      by the "brightness-levels" property).
> +    $ref: /schemas/types.yaml#/definitions/uint32
Same comment as before...

> +
> +  num-interpolated-steps:
> +    description: Number of interpolated steps between each value of brightness-levels
> +      table. This way a high resolution pwm duty cycle can be used without
> +      having to list out every possible value in the brightness-level array.
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +
> +dependencies:
> +  default-brightness-level: [brightness-levels]
> +  num-interpolated-steps: [brightness-levels]
> +
> +required:
> +  - compatible
> +  - pwms
> +  - power-supply
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    backlight {
> +        compatible = "pwm-backlight";
> +        pwms = <&pwm 0 5000000>;
> +
> +        brightness-levels = <0 4 8 16 32 64 128 255>;
> +        default-brightness-level = <6>;
> +
> +        power-supply = <&vdd_bl_reg>;
> +        enable-gpios = <&gpio 58 0>;
> +        post-pwm-on-delay-ms = <10>;
> +        pwm-off-delay-ms = <10>;
> +    };
> +
> +  - |
> +    // Example using num-interpolation-steps:
> +    backlight {
> +        compatible = "pwm-backlight";
> +        pwms = <&pwm 0 5000000>;
> +
> +        brightness-levels = <0 2048 4096 8192 16384 65535>;
> +        num-interpolated-steps = <2048>;
> +        default-brightness-level = <4096>;
> +
> +        power-supply = <&vdd_bl_reg>;
> +        enable-gpios = <&gpio 58 0>;
> +    };
> +
> +...
> -- 
> 2.25.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
Daniel Thompson June 22, 2020, 10:39 a.m. UTC | #2
On Fri, Jun 19, 2020 at 11:53:41PM +0200, Sam Ravnborg wrote:
> Good to have these converted. A few comments in the following. One
> comment is for the backlight people as you copied the original text.

... and I've sliced out everything except that in this reply.

> On Thu, Jun 18, 2020 at 04:44:13PM -0600, Rob Herring wrote:
> > 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..ae50945d2798
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/leds/backlight/led-backlight.yaml
> > @@ -0,0 +1,58 @@
> > ...
> > +
> > +  default-brightness-level:
> > +    description: The default brightness level (index into the array defined
> > +      by the "brightness-levels" property).
>
> This description does not match my understading.
> The description says "index into", but in reality this is a value that
> matches somewhere in the range specified by brightness-levels.
> So it is not an index.
> 
> Maybe I just read it wrong and the description is fine. But when I read
> index the when it says 6 I would look for brightness-levels[6] equals
> 128 in the example below.

When the brightness-levels array is not present then backlight
brightness and led brightness have a 1:1 mapping. In this case the
meaning of "default brightness level" is (hopefully) obvious and the
parenthetic text does not apply.

When the array is present then we have two different scales of
brightness and it is important to describe which scale we use for the
default brightness. The language about "index into" was adopted to avoid
having to introduce extra terminology whilst making it clear that
setting the default brightness-level to 128 is invalid (because it is
not an acceptable index) and that a value of 6 will result in the LED
brightness being set to 128.


> And this is not how it is coded.

That had me worried for a moment but I think the code and bindings are
in agreement.

There is some code to map the LED scale (128) to the backlight scale (6)
but this used to ensure we supply the correct brightness level to user
space when an active backlight is handed over from bootloader to kernel.

If a default-brightness-level is provided then the above logic is
overridden and the value read from the DT gets placed directly into
props.brightness where it will act as in index into the brightness
table.


Daniel.
Daniel Thompson June 22, 2020, 2:22 p.m. UTC | #3
On Thu, Jun 18, 2020 at 04:44:13PM -0600, Rob Herring wrote:
> Convert the common GPIO, LED, and PWM backlight bindings to DT schema
> format.
> 
> Given there's only 2 common properties and the descriptions are slightly
> different, I opted to not create a common backlight schema.
> 
> Cc: Lee Jones <lee.jones@linaro.org>
> Cc: Daniel Thompson <daniel.thompson@linaro.org>
> Cc: Jingoo Han <jingoohan1@gmail.com>
> Signed-off-by: Rob Herring <robh@kernel.org>

...


> 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..ae50945d2798
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/leds/backlight/led-backlight.yaml
> @@ -0,0 +1,58 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/leds/backlight/led-backlight.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: led-backlight bindings
> +
> +maintainers:
> +  - Lee Jones <lee.jones@linaro.org>
> +  - Daniel Thompson <daniel.thompson@linaro.org>
> +  - Jingoo Han <jingoohan1@gmail.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:
> +    description: A list of LED nodes
> +    $ref: /schemas/types.yaml#/definitions/phandle-array
> +
> +  brightness-levels:
> +    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.
> +    $ref: /schemas/types.yaml#/definitions/uint32-array
> +
> +  default-brightness-level:
> +    description: The default brightness level (index into the array defined
> +      by the "brightness-levels" property).
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +
> +dependencies:
> +  default-brightness-level: [brightness-levels]

I don't think there is a dependency here. default-brightness-level still
makes sense with there is no mapping table present.

Based on Sam's feedback perhaps adding ("if one is provided") to the
parenthetic text.


> diff --git a/Documentation/devicetree/bindings/leds/backlight/pwm-backlight.yaml b/Documentation/devicetree/bindings/leds/backlight/pwm-backlight.yaml
> new file mode 100644
> index 000000000000..7e1f109a38a4
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/leds/backlight/pwm-backlight.yaml
> @@ -0,0 +1,98 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/leds/backlight/pwm-backlight.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: pwm-backlight bindings
> +
> +maintainers:
> +  - Lee Jones <lee.jones@linaro.org>
> +  - Daniel Thompson <daniel.thompson@linaro.org>
> +  - Jingoo Han <jingoohan1@gmail.com>
> +
> +properties:
> +  compatible:
> +    const: pwm-backlight
> +
> +  pwms:
> +    maxItems: 1
> +
> +  pwm-names: true
> +
> +  power-supply:
> +    description: regulator for supply voltage
> +
> +  enable-gpios:
> +    description: Contains a single GPIO specifier for the GPIO which enables
> +      and disables the backlight
> +    maxItems: 1
> +
> +  post-pwm-on-delay-ms:
> +    description: Delay in ms between setting an initial (non-zero) PWM and
> +      enabling the backlight using GPIO.
> +
> +  pwm-off-delay-ms:
> +    description: Delay in ms between disabling the backlight using GPIO
> +      and setting PWM value to 0.
> +
> +  brightness-levels:
> +    description: Array of distinct brightness levels. Typically these are
> +      in the range from 0 to 255, but any range starting at 0 will do. The
> +      actual brightness level (PWM duty cycle) will be interpolated from
> +      these values. 0 means a 0% duty cycle (darkest/off), while the last
> +      value in the array represents a 100% duty cycle (brightest).
> +    $ref: /schemas/types.yaml#/definitions/uint32-array
> +
> +  default-brightness-level:
> +    description: The default brightness level (index into the array defined
> +      by the "brightness-levels" property).
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +
> +  num-interpolated-steps:
> +    description: Number of interpolated steps between each value of brightness-levels
> +      table. This way a high resolution pwm duty cycle can be used without
> +      having to list out every possible value in the brightness-level array.
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +
> +dependencies:
> +  default-brightness-level: [brightness-levels]
> +  num-interpolated-steps: [brightness-levels]

Just for the record, these dependencies are OK. Iit isn't really a good
idea to map 1:1 to a PWM since we end up with a gazillion
indistinguishable levels so the bindings (and the driver) to not allow
such a mode.


Daniel.
Daniel Thompson June 22, 2020, 4:57 p.m. UTC | #4
On Fri, Jun 19, 2020 at 11:53:41PM +0200, Sam Ravnborg wrote:
> > diff --git a/Documentation/devicetree/bindings/leds/backlight/pwm-backlight.yaml b/Documentation/devicetree/bindings/leds/backlight/pwm-backlight.yaml
> > new file mode 100644
> > index 000000000000..7e1f109a38a4
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/leds/backlight/pwm-backlight.yaml
> > @@ -0,0 +1,98 @@
> > +# SPDX-License-Identifier: GPL-2.0-only
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/leds/backlight/pwm-backlight.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: pwm-backlight bindings
> > +
> > +maintainers:
> > +  - Lee Jones <lee.jones@linaro.org>
> > +  - Daniel Thompson <daniel.thompson@linaro.org>
> > +  - Jingoo Han <jingoohan1@gmail.com>
> > +
> > +properties:
> > +  compatible:
> > +    const: pwm-backlight
> > +
> > +  pwms:
> > +    maxItems: 1
> > +
> > +  pwm-names: true
> > +
> > +  power-supply:
> > +    description: regulator for supply voltage
> > +
> > +  enable-gpios:
> > +    description: Contains a single GPIO specifier for the GPIO which enables
> > +      and disables the backlight
> > +    maxItems: 1
> > +
> > +  post-pwm-on-delay-ms:
> > +    description: Delay in ms between setting an initial (non-zero) PWM and
> > +      enabling the backlight using GPIO.
> > +
> > +  pwm-off-delay-ms:
> > +    description: Delay in ms between disabling the backlight using GPIO
> > +      and setting PWM value to 0.
> > +
> > +  brightness-levels:
> > +    description: Array of distinct brightness levels. Typically these are
> > +      in the range from 0 to 255, but any range starting at 0 will do. The
> > +      actual brightness level (PWM duty cycle) will be interpolated from
> > +      these values. 0 means a 0% duty cycle (darkest/off), while the last
> > +      value in the array represents a 100% duty cycle (brightest).
> > +    $ref: /schemas/types.yaml#/definitions/uint32-array
> > +
> > +  default-brightness-level:
> > +    description: The default brightness level (index into the array defined
> > +      by the "brightness-levels" property).
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> Same comment as before...

Sorry the "ditto" meant I didn't thing about PWM as much as I should
have.

The situation for PWM is a little different to LED. That's mostly
because we decided not to clutter the LED code with
"num-interpolated-steps".

The PWM code implements the default-brightness-level as an index into
the brightness array *after* it has been expanded using interpolation.
In other words today Linux treats the default-brightness-level more
like[1].

    description: The default brightness level. When
      num-interpolated-steps is not set this is simply an index into
      the array defined by the "brightness-levels" property. If
      num-interpolated-steps is set the brightness array will be
      expanded by interpolation before we index to get a default
      level.

This is the best I have come up with so far... but I concede it still
lacks elegance.


Daniel.


[1] I don't know my way round the BSD code bases to be sure what they
    do... I did a couple of web searches but didn't pull up anything
    definitive.


> 
> > +
> > +  num-interpolated-steps:
> > +    description: Number of interpolated steps between each value of brightness-levels
> > +      table. This way a high resolution pwm duty cycle can be used without
> > +      having to list out every possible value in the brightness-level array.
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +
> > +dependencies:
> > +  default-brightness-level: [brightness-levels]
> > +  num-interpolated-steps: [brightness-levels]
> > +
> > +required:
> > +  - compatible
> > +  - pwms
> > +  - power-supply
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    backlight {
> > +        compatible = "pwm-backlight";
> > +        pwms = <&pwm 0 5000000>;
> > +
> > +        brightness-levels = <0 4 8 16 32 64 128 255>;
> > +        default-brightness-level = <6>;
> > +
> > +        power-supply = <&vdd_bl_reg>;
> > +        enable-gpios = <&gpio 58 0>;
> > +        post-pwm-on-delay-ms = <10>;
> > +        pwm-off-delay-ms = <10>;
> > +    };
> > +
> > +  - |
> > +    // Example using num-interpolation-steps:
> > +    backlight {
> > +        compatible = "pwm-backlight";
> > +        pwms = <&pwm 0 5000000>;
> > +
> > +        brightness-levels = <0 2048 4096 8192 16384 65535>;
> > +        num-interpolated-steps = <2048>;
> > +        default-brightness-level = <4096>;
> > +
> > +        power-supply = <&vdd_bl_reg>;
> > +        enable-gpios = <&gpio 58 0>;
> > +    };
> > +
> > +...
> > -- 
> > 2.25.1
> > 
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
Rob Herring June 29, 2020, 5:57 p.m. UTC | #5
On Mon, Jun 22, 2020 at 10:57 AM Daniel Thompson
<daniel.thompson@linaro.org> wrote:
>
> On Fri, Jun 19, 2020 at 11:53:41PM +0200, Sam Ravnborg wrote:
> > > diff --git a/Documentation/devicetree/bindings/leds/backlight/pwm-backlight.yaml b/Documentation/devicetree/bindings/leds/backlight/pwm-backlight.yaml
> > > new file mode 100644
> > > index 000000000000..7e1f109a38a4
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/leds/backlight/pwm-backlight.yaml
> > > @@ -0,0 +1,98 @@
> > > +# SPDX-License-Identifier: GPL-2.0-only
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/leds/backlight/pwm-backlight.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: pwm-backlight bindings
> > > +
> > > +maintainers:
> > > +  - Lee Jones <lee.jones@linaro.org>
> > > +  - Daniel Thompson <daniel.thompson@linaro.org>
> > > +  - Jingoo Han <jingoohan1@gmail.com>
> > > +
> > > +properties:
> > > +  compatible:
> > > +    const: pwm-backlight
> > > +
> > > +  pwms:
> > > +    maxItems: 1
> > > +
> > > +  pwm-names: true
> > > +
> > > +  power-supply:
> > > +    description: regulator for supply voltage
> > > +
> > > +  enable-gpios:
> > > +    description: Contains a single GPIO specifier for the GPIO which enables
> > > +      and disables the backlight
> > > +    maxItems: 1
> > > +
> > > +  post-pwm-on-delay-ms:
> > > +    description: Delay in ms between setting an initial (non-zero) PWM and
> > > +      enabling the backlight using GPIO.
> > > +
> > > +  pwm-off-delay-ms:
> > > +    description: Delay in ms between disabling the backlight using GPIO
> > > +      and setting PWM value to 0.
> > > +
> > > +  brightness-levels:
> > > +    description: Array of distinct brightness levels. Typically these are
> > > +      in the range from 0 to 255, but any range starting at 0 will do. The
> > > +      actual brightness level (PWM duty cycle) will be interpolated from
> > > +      these values. 0 means a 0% duty cycle (darkest/off), while the last
> > > +      value in the array represents a 100% duty cycle (brightest).
> > > +    $ref: /schemas/types.yaml#/definitions/uint32-array
> > > +
> > > +  default-brightness-level:
> > > +    description: The default brightness level (index into the array defined
> > > +      by the "brightness-levels" property).
> > > +    $ref: /schemas/types.yaml#/definitions/uint32
> > Same comment as before...
>
> Sorry the "ditto" meant I didn't thing about PWM as much as I should
> have.
>
> The situation for PWM is a little different to LED. That's mostly
> because we decided not to clutter the LED code with
> "num-interpolated-steps".
>
> The PWM code implements the default-brightness-level as an index into
> the brightness array *after* it has been expanded using interpolation.
> In other words today Linux treats the default-brightness-level more
> like[1].
>
>     description: The default brightness level. When
>       num-interpolated-steps is not set this is simply an index into
>       the array defined by the "brightness-levels" property. If
>       num-interpolated-steps is set the brightness array will be
>       expanded by interpolation before we index to get a default
>       level.
>
> This is the best I have come up with so far... but I concede it still
> lacks elegance.

Happy to add this or whatever folks want if there's agreement, but I
don't want to get bogged down on re-reviewing and re-writing the
binding on what is just a conversion. There's a mountain of bindings
to convert.

Rob
Sam Ravnborg June 29, 2020, 7:18 p.m. UTC | #6
On Mon, Jun 29, 2020 at 11:57:37AM -0600, Rob Herring wrote:
> On Mon, Jun 22, 2020 at 10:57 AM Daniel Thompson
> <daniel.thompson@linaro.org> wrote:
> >
> > On Fri, Jun 19, 2020 at 11:53:41PM +0200, Sam Ravnborg wrote:
> > > > diff --git a/Documentation/devicetree/bindings/leds/backlight/pwm-backlight.yaml b/Documentation/devicetree/bindings/leds/backlight/pwm-backlight.yaml
> > > > new file mode 100644
> > > > index 000000000000..7e1f109a38a4
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/leds/backlight/pwm-backlight.yaml
> > > > @@ -0,0 +1,98 @@
> > > > +# SPDX-License-Identifier: GPL-2.0-only
> > > > +%YAML 1.2
> > > > +---
> > > > +$id: http://devicetree.org/schemas/leds/backlight/pwm-backlight.yaml#
> > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > +
> > > > +title: pwm-backlight bindings
> > > > +
> > > > +maintainers:
> > > > +  - Lee Jones <lee.jones@linaro.org>
> > > > +  - Daniel Thompson <daniel.thompson@linaro.org>
> > > > +  - Jingoo Han <jingoohan1@gmail.com>
> > > > +
> > > > +properties:
> > > > +  compatible:
> > > > +    const: pwm-backlight
> > > > +
> > > > +  pwms:
> > > > +    maxItems: 1
> > > > +
> > > > +  pwm-names: true
> > > > +
> > > > +  power-supply:
> > > > +    description: regulator for supply voltage
> > > > +
> > > > +  enable-gpios:
> > > > +    description: Contains a single GPIO specifier for the GPIO which enables
> > > > +      and disables the backlight
> > > > +    maxItems: 1
> > > > +
> > > > +  post-pwm-on-delay-ms:
> > > > +    description: Delay in ms between setting an initial (non-zero) PWM and
> > > > +      enabling the backlight using GPIO.
> > > > +
> > > > +  pwm-off-delay-ms:
> > > > +    description: Delay in ms between disabling the backlight using GPIO
> > > > +      and setting PWM value to 0.
> > > > +
> > > > +  brightness-levels:
> > > > +    description: Array of distinct brightness levels. Typically these are
> > > > +      in the range from 0 to 255, but any range starting at 0 will do. The
> > > > +      actual brightness level (PWM duty cycle) will be interpolated from
> > > > +      these values. 0 means a 0% duty cycle (darkest/off), while the last
> > > > +      value in the array represents a 100% duty cycle (brightest).
> > > > +    $ref: /schemas/types.yaml#/definitions/uint32-array
> > > > +
> > > > +  default-brightness-level:
> > > > +    description: The default brightness level (index into the array defined
> > > > +      by the "brightness-levels" property).
> > > > +    $ref: /schemas/types.yaml#/definitions/uint32
> > > Same comment as before...
> >
> > Sorry the "ditto" meant I didn't thing about PWM as much as I should
> > have.
> >
> > The situation for PWM is a little different to LED. That's mostly
> > because we decided not to clutter the LED code with
> > "num-interpolated-steps".
> >
> > The PWM code implements the default-brightness-level as an index into
> > the brightness array *after* it has been expanded using interpolation.
> > In other words today Linux treats the default-brightness-level more
> > like[1].
> >
> >     description: The default brightness level. When
> >       num-interpolated-steps is not set this is simply an index into
> >       the array defined by the "brightness-levels" property. If
> >       num-interpolated-steps is set the brightness array will be
> >       expanded by interpolation before we index to get a default
> >       level.
> >
> > This is the best I have come up with so far... but I concede it still
> > lacks elegance.
> 
> Happy to add this or whatever folks want if there's agreement, but I
> don't want to get bogged down on re-reviewing and re-writing the
> binding on what is just a conversion. There's a mountain of bindings
> to convert.
The original explanation is ok, as pointed out by Daniel.
So I suggest moving forward with that and then others can improve the
descriptions later as necessary.

	Sam
Rob Herring June 29, 2020, 7:37 p.m. UTC | #7
On Fri, Jun 19, 2020 at 3:53 PM Sam Ravnborg <sam@ravnborg.org> wrote:
>
> Hi Rob.
>
> Good to have these converted. A few comments in the following. One
> comment is for the backlight people as you copied the original text.
>
>         Sam
>
> On Thu, Jun 18, 2020 at 04:44:13PM -0600, Rob Herring wrote:
> > Convert the common GPIO, LED, and PWM backlight bindings to DT schema
> > format.
> >
> > Given there's only 2 common properties and the descriptions are slightly
> > different, I opted to not create a common backlight schema.
> >
> > Cc: Lee Jones <lee.jones@linaro.org>
> > Cc: Daniel Thompson <daniel.thompson@linaro.org>
> > Cc: Jingoo Han <jingoohan1@gmail.com>
> > Signed-off-by: Rob Herring <robh@kernel.org>
> > ---
> >  .../leds/backlight/gpio-backlight.txt         | 16 ---
> >  .../leds/backlight/gpio-backlight.yaml        | 41 ++++++++
> >  .../bindings/leds/backlight/led-backlight.txt | 28 ------
> >  .../leds/backlight/led-backlight.yaml         | 58 +++++++++++
> >  .../bindings/leds/backlight/pwm-backlight.txt | 61 ------------
> >  .../leds/backlight/pwm-backlight.yaml         | 98 +++++++++++++++++++
> >  6 files changed, 197 insertions(+), 105 deletions(-)
> >  delete mode 100644 Documentation/devicetree/bindings/leds/backlight/gpio-backlight.txt
> >  create mode 100644 Documentation/devicetree/bindings/leds/backlight/gpio-backlight.yaml
> >  delete mode 100644 Documentation/devicetree/bindings/leds/backlight/led-backlight.txt
> >  create mode 100644 Documentation/devicetree/bindings/leds/backlight/led-backlight.yaml
> >  delete mode 100644 Documentation/devicetree/bindings/leds/backlight/pwm-backlight.txt
> >  create mode 100644 Documentation/devicetree/bindings/leds/backlight/pwm-backlight.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/leds/backlight/gpio-backlight.txt b/Documentation/devicetree/bindings/leds/backlight/gpio-backlight.txt
> > deleted file mode 100644
> > index 321be6640533..000000000000
> > --- a/Documentation/devicetree/bindings/leds/backlight/gpio-backlight.txt
> > +++ /dev/null
> > @@ -1,16 +0,0 @@
> > -gpio-backlight bindings
> > -
> > -Required properties:
> > -  - compatible: "gpio-backlight"
> > -  - gpios: describes the gpio that is used for enabling/disabling the backlight.
> > -    refer to bindings/gpio/gpio.txt for more details.
> > -
> > -Optional properties:
> > -  - default-on: enable the backlight at boot.
> > -
> > -Example:
> > -     backlight {
> > -             compatible = "gpio-backlight";
> > -             gpios = <&gpio3 4 GPIO_ACTIVE_HIGH>;
> > -             default-on;
> > -     };
> > diff --git a/Documentation/devicetree/bindings/leds/backlight/gpio-backlight.yaml b/Documentation/devicetree/bindings/leds/backlight/gpio-backlight.yaml
> > new file mode 100644
> > index 000000000000..75cc569b9c55
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/leds/backlight/gpio-backlight.yaml
> > @@ -0,0 +1,41 @@
> > +# SPDX-License-Identifier: GPL-2.0-only
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/leds/backlight/gpio-backlight.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: gpio-backlight bindings
> > +
> > +maintainers:
> > +  - Lee Jones <lee.jones@linaro.org>
> > +  - Daniel Thompson <daniel.thompson@linaro.org>
> > +  - Jingoo Han <jingoohan1@gmail.com>
> > +
> > +properties:
> > +  compatible:
> > +    const: gpio-backlight
> > +
> > +  gpios:
> > +    description: The gpio that is used for enabling/disabling the backlight.
> > +    maxItems: 1
> > +
> > +  default-on:
> > +    description: enable the backlight at boot.
> > +    type: boolean
> > +
> > +required:
> > +  - compatible
> > +  - gpios
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    #include <dt-bindings/gpio/gpio.h>
> > +    backlight {
> > +        compatible = "gpio-backlight";
> > +        gpios = <&gpio3 4 GPIO_ACTIVE_HIGH>;
> > +        default-on;
> > +    };
> > +
> > +...
> > diff --git a/Documentation/devicetree/bindings/leds/backlight/led-backlight.txt b/Documentation/devicetree/bindings/leds/backlight/led-backlight.txt
> > deleted file mode 100644
> > index 4c7dfbe7f67a..000000000000
> > --- a/Documentation/devicetree/bindings/leds/backlight/led-backlight.txt
> > +++ /dev/null
> > @@ -1,28 +0,0 @@
> > -led-backlight bindings
> > -
> > -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.
> > -
> > -Required properties:
> > -  - compatible: "led-backlight"
> > -  - leds: a list of LEDs
> > -
> > -Optional properties:
> > -  - brightness-levels: 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.
> > -
> > -  - default-brightness-level: The default brightness level.
> > -
> > -Example:
> > -
> > -     backlight {
> > -             compatible = "led-backlight";
> > -
> > -             leds = <&led1>, <&led2>;
> > -             brightness-levels = <0 4 8 16 32 64 128 255>;
> > -             default-brightness-level = <6>;
> > -     };
> > 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..ae50945d2798
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/leds/backlight/led-backlight.yaml
> > @@ -0,0 +1,58 @@
> > +# SPDX-License-Identifier: GPL-2.0-only
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/leds/backlight/led-backlight.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: led-backlight bindings
> > +
> > +maintainers:
> > +  - Lee Jones <lee.jones@linaro.org>
> > +  - Daniel Thompson <daniel.thompson@linaro.org>
> > +  - Jingoo Han <jingoohan1@gmail.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:
> > +    description: A list of LED nodes
> > +    $ref: /schemas/types.yaml#/definitions/phandle-array
> > +
> > +  brightness-levels:
> > +    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.
> > +    $ref: /schemas/types.yaml#/definitions/uint32-array
> bike-shedding. To me it is a tad easier to read when multi-line
> descriptions are on a separate line.
> So "description:" on one line, and the text on following lines.
> example-schema.yaml does both - so both are official acceptable.

I agree, but the one issue is ruamel yaml wants it the above way
unless you do '|' (or really '>' may be the correct annotation here).
That's mainly an issue if doing tree wide yaml->python
processing->yaml transformations. But if the line lengths don't match
exactly what ruamel is set to, then it reformats it anyways, so in the
end it doesn't really matter. I just have to filter out unwanted
reformatting (until ruamel can really do roundtrips with no
reformatting).

Rob
Daniel Thompson June 30, 2020, 4:35 p.m. UTC | #8
On Mon, Jun 29, 2020 at 09:18:47PM +0200, Sam Ravnborg wrote:
> On Mon, Jun 29, 2020 at 11:57:37AM -0600, Rob Herring wrote:
> > On Mon, Jun 22, 2020 at 10:57 AM Daniel Thompson
> > <daniel.thompson@linaro.org> wrote:
> > >
> > > On Fri, Jun 19, 2020 at 11:53:41PM +0200, Sam Ravnborg wrote:
> > > > > diff --git a/Documentation/devicetree/bindings/leds/backlight/pwm-backlight.yaml b/Documentation/devicetree/bindings/leds/backlight/pwm-backlight.yaml
> > > > > new file mode 100644
> > > > > index 000000000000..7e1f109a38a4
> > > > > --- /dev/null
> > > > > +++ b/Documentation/devicetree/bindings/leds/backlight/pwm-backlight.yaml
> > > > > @@ -0,0 +1,98 @@
> > > > > +# SPDX-License-Identifier: GPL-2.0-only
> > > > > +%YAML 1.2
> > > > > +---
> > > > > +$id: http://devicetree.org/schemas/leds/backlight/pwm-backlight.yaml#
> > > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > > +
> > > > > +title: pwm-backlight bindings
> > > > > +
> > > > > +maintainers:
> > > > > +  - Lee Jones <lee.jones@linaro.org>
> > > > > +  - Daniel Thompson <daniel.thompson@linaro.org>
> > > > > +  - Jingoo Han <jingoohan1@gmail.com>
> > > > > +
> > > > > +properties:
> > > > > +  compatible:
> > > > > +    const: pwm-backlight
> > > > > +
> > > > > +  pwms:
> > > > > +    maxItems: 1
> > > > > +
> > > > > +  pwm-names: true
> > > > > +
> > > > > +  power-supply:
> > > > > +    description: regulator for supply voltage
> > > > > +
> > > > > +  enable-gpios:
> > > > > +    description: Contains a single GPIO specifier for the GPIO which enables
> > > > > +      and disables the backlight
> > > > > +    maxItems: 1
> > > > > +
> > > > > +  post-pwm-on-delay-ms:
> > > > > +    description: Delay in ms between setting an initial (non-zero) PWM and
> > > > > +      enabling the backlight using GPIO.
> > > > > +
> > > > > +  pwm-off-delay-ms:
> > > > > +    description: Delay in ms between disabling the backlight using GPIO
> > > > > +      and setting PWM value to 0.
> > > > > +
> > > > > +  brightness-levels:
> > > > > +    description: Array of distinct brightness levels. Typically these are
> > > > > +      in the range from 0 to 255, but any range starting at 0 will do. The
> > > > > +      actual brightness level (PWM duty cycle) will be interpolated from
> > > > > +      these values. 0 means a 0% duty cycle (darkest/off), while the last
> > > > > +      value in the array represents a 100% duty cycle (brightest).
> > > > > +    $ref: /schemas/types.yaml#/definitions/uint32-array
> > > > > +
> > > > > +  default-brightness-level:
> > > > > +    description: The default brightness level (index into the array defined
> > > > > +      by the "brightness-levels" property).
> > > > > +    $ref: /schemas/types.yaml#/definitions/uint32
> > > > Same comment as before...
> > >
> > > Sorry the "ditto" meant I didn't thing about PWM as much as I should
> > > have.
> > >
> > > The situation for PWM is a little different to LED. That's mostly
> > > because we decided not to clutter the LED code with
> > > "num-interpolated-steps".
> > >
> > > The PWM code implements the default-brightness-level as an index into
> > > the brightness array *after* it has been expanded using interpolation.
> > > In other words today Linux treats the default-brightness-level more
> > > like[1].
> > >
> > >     description: The default brightness level. When
> > >       num-interpolated-steps is not set this is simply an index into
> > >       the array defined by the "brightness-levels" property. If
> > >       num-interpolated-steps is set the brightness array will be
> > >       expanded by interpolation before we index to get a default
> > >       level.
> > >
> > > This is the best I have come up with so far... but I concede it still
> > > lacks elegance.
> > 
> > Happy to add this or whatever folks want if there's agreement, but I
> > don't want to get bogged down on re-reviewing and re-writing the
> > binding on what is just a conversion. There's a mountain of bindings
> > to convert.
> The original explanation is ok, as pointed out by Daniel.
> So I suggest moving forward with that and then others can improve the
> descriptions later as necessary.

That's fine for me to. It would be clearer in version history (improving
definitions during a conversion hides them when mining the changelog).


Daniel.
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/leds/backlight/gpio-backlight.txt b/Documentation/devicetree/bindings/leds/backlight/gpio-backlight.txt
deleted file mode 100644
index 321be6640533..000000000000
--- a/Documentation/devicetree/bindings/leds/backlight/gpio-backlight.txt
+++ /dev/null
@@ -1,16 +0,0 @@ 
-gpio-backlight bindings
-
-Required properties:
-  - compatible: "gpio-backlight"
-  - gpios: describes the gpio that is used for enabling/disabling the backlight.
-    refer to bindings/gpio/gpio.txt for more details.
-
-Optional properties:
-  - default-on: enable the backlight at boot.
-
-Example:
-	backlight {
-		compatible = "gpio-backlight";
-		gpios = <&gpio3 4 GPIO_ACTIVE_HIGH>;
-		default-on;
-	};
diff --git a/Documentation/devicetree/bindings/leds/backlight/gpio-backlight.yaml b/Documentation/devicetree/bindings/leds/backlight/gpio-backlight.yaml
new file mode 100644
index 000000000000..75cc569b9c55
--- /dev/null
+++ b/Documentation/devicetree/bindings/leds/backlight/gpio-backlight.yaml
@@ -0,0 +1,41 @@ 
+# SPDX-License-Identifier: GPL-2.0-only
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/leds/backlight/gpio-backlight.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: gpio-backlight bindings
+
+maintainers:
+  - Lee Jones <lee.jones@linaro.org>
+  - Daniel Thompson <daniel.thompson@linaro.org>
+  - Jingoo Han <jingoohan1@gmail.com>
+
+properties:
+  compatible:
+    const: gpio-backlight
+
+  gpios:
+    description: The gpio that is used for enabling/disabling the backlight.
+    maxItems: 1
+
+  default-on:
+    description: enable the backlight at boot.
+    type: boolean
+
+required:
+  - compatible
+  - gpios
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+    backlight {
+        compatible = "gpio-backlight";
+        gpios = <&gpio3 4 GPIO_ACTIVE_HIGH>;
+        default-on;
+    };
+
+...
diff --git a/Documentation/devicetree/bindings/leds/backlight/led-backlight.txt b/Documentation/devicetree/bindings/leds/backlight/led-backlight.txt
deleted file mode 100644
index 4c7dfbe7f67a..000000000000
--- a/Documentation/devicetree/bindings/leds/backlight/led-backlight.txt
+++ /dev/null
@@ -1,28 +0,0 @@ 
-led-backlight bindings
-
-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.
-
-Required properties:
-  - compatible: "led-backlight"
-  - leds: a list of LEDs
-
-Optional properties:
-  - brightness-levels: 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.
-
-  - default-brightness-level: The default brightness level.
-
-Example:
-
-	backlight {
-		compatible = "led-backlight";
-
-		leds = <&led1>, <&led2>;
-		brightness-levels = <0 4 8 16 32 64 128 255>;
-		default-brightness-level = <6>;
-	};
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..ae50945d2798
--- /dev/null
+++ b/Documentation/devicetree/bindings/leds/backlight/led-backlight.yaml
@@ -0,0 +1,58 @@ 
+# SPDX-License-Identifier: GPL-2.0-only
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/leds/backlight/led-backlight.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: led-backlight bindings
+
+maintainers:
+  - Lee Jones <lee.jones@linaro.org>
+  - Daniel Thompson <daniel.thompson@linaro.org>
+  - Jingoo Han <jingoohan1@gmail.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:
+    description: A list of LED nodes
+    $ref: /schemas/types.yaml#/definitions/phandle-array
+
+  brightness-levels:
+    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.
+    $ref: /schemas/types.yaml#/definitions/uint32-array
+
+  default-brightness-level:
+    description: The default brightness level (index into the array defined
+      by the "brightness-levels" property).
+    $ref: /schemas/types.yaml#/definitions/uint32
+
+dependencies:
+  default-brightness-level: [brightness-levels]
+
+required:
+  - compatible
+  - leds
+
+additionalProperties: false
+
+examples:
+  - |
+    backlight {
+        compatible = "led-backlight";
+
+        leds = <&led1>, <&led2>;
+        brightness-levels = <0 4 8 16 32 64 128 255>;
+        default-brightness-level = <6>;
+    };
+
+...
diff --git a/Documentation/devicetree/bindings/leds/backlight/pwm-backlight.txt b/Documentation/devicetree/bindings/leds/backlight/pwm-backlight.txt
deleted file mode 100644
index 64fa2fbd98c9..000000000000
--- a/Documentation/devicetree/bindings/leds/backlight/pwm-backlight.txt
+++ /dev/null
@@ -1,61 +0,0 @@ 
-pwm-backlight bindings
-
-Required properties:
-  - compatible: "pwm-backlight"
-  - pwms: OF device-tree PWM specification (see PWM binding[0])
-  - power-supply: regulator for supply voltage
-
-Optional properties:
-  - pwm-names: a list of names for the PWM devices specified in the
-               "pwms" property (see PWM binding[0])
-  - enable-gpios: contains a single GPIO specifier for the GPIO which enables
-                  and disables the backlight (see GPIO binding[1])
-  - post-pwm-on-delay-ms: Delay in ms between setting an initial (non-zero) PWM
-                          and enabling the backlight using GPIO.
-  - pwm-off-delay-ms: Delay in ms between disabling the backlight using GPIO
-                      and setting PWM value to 0.
-  - brightness-levels: Array of distinct brightness levels. Typically these
-                       are in the range from 0 to 255, but any range starting at
-                       0 will do. The actual brightness level (PWM duty cycle)
-                       will be interpolated from these values. 0 means a 0% duty
-                       cycle (darkest/off), while the last value in the array
-                       represents a 100% duty cycle (brightest).
-  - default-brightness-level: The default brightness level (index into the
-                              array defined by the "brightness-levels" property).
-  - num-interpolated-steps: Number of interpolated steps between each value
-                            of brightness-levels table. This way a high
-                            resolution pwm duty cycle can be used without
-                            having to list out every possible value in the
-                            brightness-level array.
-
-[0]: Documentation/devicetree/bindings/pwm/pwm.txt
-[1]: Documentation/devicetree/bindings/gpio/gpio.txt
-
-Example:
-
-	backlight {
-		compatible = "pwm-backlight";
-		pwms = <&pwm 0 5000000>;
-
-		brightness-levels = <0 4 8 16 32 64 128 255>;
-		default-brightness-level = <6>;
-
-		power-supply = <&vdd_bl_reg>;
-		enable-gpios = <&gpio 58 0>;
-		post-pwm-on-delay-ms = <10>;
-		pwm-off-delay-ms = <10>;
-	};
-
-Example using num-interpolation-steps:
-
-	backlight {
-		compatible = "pwm-backlight";
-		pwms = <&pwm 0 5000000>;
-
-		brightness-levels = <0 2048 4096 8192 16384 65535>;
-		num-interpolated-steps = <2048>;
-		default-brightness-level = <4096>;
-
-		power-supply = <&vdd_bl_reg>;
-		enable-gpios = <&gpio 58 0>;
-	};
diff --git a/Documentation/devicetree/bindings/leds/backlight/pwm-backlight.yaml b/Documentation/devicetree/bindings/leds/backlight/pwm-backlight.yaml
new file mode 100644
index 000000000000..7e1f109a38a4
--- /dev/null
+++ b/Documentation/devicetree/bindings/leds/backlight/pwm-backlight.yaml
@@ -0,0 +1,98 @@ 
+# SPDX-License-Identifier: GPL-2.0-only
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/leds/backlight/pwm-backlight.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: pwm-backlight bindings
+
+maintainers:
+  - Lee Jones <lee.jones@linaro.org>
+  - Daniel Thompson <daniel.thompson@linaro.org>
+  - Jingoo Han <jingoohan1@gmail.com>
+
+properties:
+  compatible:
+    const: pwm-backlight
+
+  pwms:
+    maxItems: 1
+
+  pwm-names: true
+
+  power-supply:
+    description: regulator for supply voltage
+
+  enable-gpios:
+    description: Contains a single GPIO specifier for the GPIO which enables
+      and disables the backlight
+    maxItems: 1
+
+  post-pwm-on-delay-ms:
+    description: Delay in ms between setting an initial (non-zero) PWM and
+      enabling the backlight using GPIO.
+
+  pwm-off-delay-ms:
+    description: Delay in ms between disabling the backlight using GPIO
+      and setting PWM value to 0.
+
+  brightness-levels:
+    description: Array of distinct brightness levels. Typically these are
+      in the range from 0 to 255, but any range starting at 0 will do. The
+      actual brightness level (PWM duty cycle) will be interpolated from
+      these values. 0 means a 0% duty cycle (darkest/off), while the last
+      value in the array represents a 100% duty cycle (brightest).
+    $ref: /schemas/types.yaml#/definitions/uint32-array
+
+  default-brightness-level:
+    description: The default brightness level (index into the array defined
+      by the "brightness-levels" property).
+    $ref: /schemas/types.yaml#/definitions/uint32
+
+  num-interpolated-steps:
+    description: Number of interpolated steps between each value of brightness-levels
+      table. This way a high resolution pwm duty cycle can be used without
+      having to list out every possible value in the brightness-level array.
+    $ref: /schemas/types.yaml#/definitions/uint32
+
+dependencies:
+  default-brightness-level: [brightness-levels]
+  num-interpolated-steps: [brightness-levels]
+
+required:
+  - compatible
+  - pwms
+  - power-supply
+
+additionalProperties: false
+
+examples:
+  - |
+    backlight {
+        compatible = "pwm-backlight";
+        pwms = <&pwm 0 5000000>;
+
+        brightness-levels = <0 4 8 16 32 64 128 255>;
+        default-brightness-level = <6>;
+
+        power-supply = <&vdd_bl_reg>;
+        enable-gpios = <&gpio 58 0>;
+        post-pwm-on-delay-ms = <10>;
+        pwm-off-delay-ms = <10>;
+    };
+
+  - |
+    // Example using num-interpolation-steps:
+    backlight {
+        compatible = "pwm-backlight";
+        pwms = <&pwm 0 5000000>;
+
+        brightness-levels = <0 2048 4096 8192 16384 65535>;
+        num-interpolated-steps = <2048>;
+        default-brightness-level = <4096>;
+
+        power-supply = <&vdd_bl_reg>;
+        enable-gpios = <&gpio 58 0>;
+    };
+
+...