Re: [PATCH] dt-bindings: backlight: Convert common backlight bindings to DT schema

2020-06-30 Thread Daniel Thompson
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
> >  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 ..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 
> > > > > +  - Daniel Thompson 
> > > > > +  - Jingoo Han 
> > > > > +
> > > > > +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.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH] dt-bindings: backlight: Convert common backlight bindings to DT schema

2020-06-29 Thread Rob Herring
On Fri, Jun 19, 2020 at 3:53 PM Sam Ravnborg  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 
> > Cc: Daniel Thompson 
> > Cc: Jingoo Han 
> > Signed-off-by: Rob Herring 
> > ---
> >  .../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..
> > --- 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 = < 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 ..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 
> > +  - Daniel Thompson 
> > +  - Jingoo Han 
> > +
> > +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 
> > +backlight {
> > +compatible = "gpio-backlight";
> > +gpios = < 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..
> > --- 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.
> > 

Re: [PATCH] dt-bindings: backlight: Convert common backlight bindings to DT schema

2020-06-29 Thread Sam Ravnborg
On Mon, Jun 29, 2020 at 11:57:37AM -0600, Rob Herring wrote:
> On Mon, Jun 22, 2020 at 10:57 AM Daniel Thompson
>  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 ..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 
> > > > +  - Daniel Thompson 
> > > > +  - Jingoo Han 
> > > > +
> > > > +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
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH] dt-bindings: backlight: Convert common backlight bindings to DT schema

2020-06-29 Thread Rob Herring
On Mon, Jun 22, 2020 at 10:57 AM Daniel Thompson
 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 ..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 
> > > +  - Daniel Thompson 
> > > +  - Jingoo Han 
> > > +
> > > +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
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH] dt-bindings: backlight: Convert common backlight bindings to DT schema

2020-06-22 Thread Daniel Thompson
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 ..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 
> > +  - Daniel Thompson 
> > +  - Jingoo Han 
> > +
> > +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 = < 0 500>;
> > +
> > +brightness-levels = <0 4 8 16 32 64 128 255>;
> > +default-brightness-level = <6>;
> > +
> > +power-supply = <_bl_reg>;
> > +enable-gpios = < 58 0>;
> > +post-pwm-on-delay-ms = <10>;
> > +pwm-off-delay-ms = <10>;
> > +};
> > +
> > +  - |
> > +// Example using num-interpolation-steps:
> > +backlight {
> > +compatible = "pwm-backlight";
> > +pwms = < 0 500>;
> > +
> > +brightness-levels = <0 2048 4096 8192 16384 65535>;
> > +num-interpolated-steps = <2048>;
> > +default-brightness-level = <4096>;
> > +
> > +power-supply = <_bl_reg>;
> > +enable-gpios = < 58 0>;
> > +};
> > +
> > +...
> > -- 
> > 2.25.1
> > 
> > ___
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org

Re: [PATCH] dt-bindings: backlight: Convert common backlight bindings to DT schema

2020-06-22 Thread Daniel Thompson
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 
> Cc: Daniel Thompson 
> Cc: Jingoo Han 
> Signed-off-by: Rob Herring 

...


> 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 ..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 
> +  - Daniel Thompson 
> +  - Jingoo Han 
> +
> +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 ..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 
> +  - Daniel Thompson 
> +  - Jingoo Han 
> +
> +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.
___
dri-devel mailing list

Re: [PATCH] dt-bindings: backlight: Convert common backlight bindings to DT schema

2020-06-22 Thread Daniel Thompson
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 ..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.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH] dt-bindings: backlight: Convert common backlight bindings to DT schema

2020-06-19 Thread Sam Ravnborg
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 
> Cc: Daniel Thompson 
> Cc: Jingoo Han 
> Signed-off-by: Rob Herring 
> ---
>  .../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..
> --- 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 = < 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 ..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 
> +  - Daniel Thompson 
> +  - Jingoo Han 
> +
> +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 
> +backlight {
> +compatible = "gpio-backlight";
> +gpios = < 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..
> --- 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 = <>, <>;
> - 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 
> 

[PATCH] dt-bindings: backlight: Convert common backlight bindings to DT schema

2020-06-18 Thread Rob Herring
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 
Cc: Daniel Thompson 
Cc: Jingoo Han 
Signed-off-by: Rob Herring 
---
 .../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..
--- 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 = < 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 ..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 
+  - Daniel Thompson 
+  - Jingoo Han 
+
+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 
+backlight {
+compatible = "gpio-backlight";
+gpios = < 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..
--- 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 = <>, <>;
-   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 ..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 
+  - Daniel Thompson 
+  - Jingoo