Re: [PATCHv2 1/2] dt-bindings: backlight: add ti-lmu-backlight binding

[Rob Herring <robh@kernel.org>]

On Mon, Jul 17, 2017 at 03:39:55PM +0200, Sebastian Reichel wrote:
> From: Milo Kim <Milo.Kim@ti.com>
> 
> Add DT binding for ti-lmu devices.
> 
> Signed-off-by: Milo Kim <milo.kim@ti.com>
> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
> ---
>  .../bindings/leds/backlight/ti-lmu-backlight.txt   | 66 ++++++++++++++++++++++
>  1 file changed, 66 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/leds/backlight/ti-lmu-backlight.txt
> 
> diff --git a/Documentation/devicetree/bindings/leds/backlight/ti-lmu-backlight.txt b/Documentation/devicetree/bindings/leds/backlight/ti-lmu-backlight.txt
> new file mode 100644
> index 000000000000..5fb1458a9136
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/leds/backlight/ti-lmu-backlight.txt
> @@ -0,0 +1,66 @@
> +TI LMU backlight device tree bindings
> +
> +Required property:
> + - compatible: Should be one of:
> +        "ti,lm3532-backlight"
> +        "ti,lm3631-backlight"
> +        "ti,lm3632-backlight"
> +        "ti,lm3633-backlight"
> +        "ti,lm3695-backlight"
> +        "ti,lm3697-backlight"

Some of the parts only do backlight, so drop the "-backlight" on those. 
And having a sub-node makes no sense in that case. Perhaps it should be 
2 binding documents separating the single function devices and MFDs. The 
binding doc(s) doesn't have to align with the driver(s). You can have 1 
driver serve 2 bindings.

> +
> +Optional properties:
> + There are two backlight control mode. One is I2C, the other is PWM mode.

The wording here is weird. My take of the datasheet is it is not 
necessarily either I2C or PWM mode as I gathered from this description.

> + Following properties are only specified in PWM mode.
> + Please note that LMU backlight device can have only one PWM channel.
> +
> + - pwms: OF device-tree PWM specification.
> + - pwm-names: a list of names for the PWM devices specified in the "pwms"
> +              property.
> +
> + For the PWM user nodes, please refer to [1].
> +
> +Child nodes:
> + LMU backlight is represented as sub-nodes of the TI LMU device [2].
> + So, LMU backlight should have more than one backlight child node.
> + Each node exactly matches with backlight control bank configuration.
> + Maximum numbers of child nodes depend on the device.
> +
> + 1 = LM3631, LM3632, LM3695
> + 2 = LM3633, LM3697
> + 3 = LM3532
> +
> + Required property of a child node:
> + - led-sources: List of enabled channels from 0 to 2.
> +                Please refer to LED binding [3].
> +                For output channels, please refer to the datasheets [4].
> +
> + Optional properties of a child node:
> + - label: Backlight channel identification.
> +          Please refer to LED binding [3].
> + - default-brightness-level: Backlight initial brightness value.
> +                             Type is <u32>. It is set as soon as backlight
> +                             device is created.
> +                             0 ~ 2047 = LM3631, LM3632, LM3633, LM3695 and
> +                                        LM3697
> +                             0 ~ 255  = LM3532
> + - ramp-up-msec, ramp-down-msec: Light dimming effect properties.
> +                                 Type is <u32>. Unit is millisecond.
> +                                 0 ~ 65 msec    = LM3532
> +                                 0 ~ 4000 msec  = LM3631
> +                                 0 ~ 16000 msec = LM3633 and LM3697
> + - pwm-period: PWM period. Only valid in PWM brightness mode.
> +               Type is <u32>. If this property is missing, then control
> +               mode is set to I2C by default.
> +
> +Examples: Please refer to ti-lmu dt-bindings. [2].
> +
> +[1] ../pwm/pwm.txt
> +[2] ../mfd/ti-lmu.txt
> +[3] ../leds/common.txt
> +[4] LM3532: http://www.ti.com/product/LM3532/datasheet
> +    LM3631: http://www.ti.com/product/LM3631/datasheet
> +    LM3632: http://www.ti.com/product/LM3632A/datasheet
> +    LM3633: http://www.ti.com/product/LM3633/datasheet
> +    LM3695: Datasheet is not opened yet, but only two strings are used.
> +    LM3697: http://www.ti.com/product/LM3697/datasheet
> -- 
> 2.13.2
>