Re: [PATCH] dt-bindings: backlight: Convert common backlight bindings to DT schema
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.
Re: [PATCH] dt-bindings: backlight: Convert common backlight bindings to DT schema
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
Re: [PATCH] dt-bindings: backlight: Convert common backlight bindings to DT schema
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
Re: [PATCH] dt-bindings: backlight: Convert common backlight bindings to DT schema
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
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-de...@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] dt-bindings: backlight: Convert common backlight bindings to DT schema
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.
Re: [PATCH] dt-bindings: backlight: Convert common backlight bindings to DT schema
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.
Re: [PATCH] dt-bindings: backlight: Convert common backlight bindings to DT schema
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 >