lkml.org 
[lkml]   [2014]   [Feb]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCH 10/10] Documentation: Add device tree bindings for TI LMU devices
On Fri, Feb 14, 2014 at 06:32:44AM +0000, Milo Kim wrote:
> Bindings for TI LMU, backlight, LM3631 regulator and LM3633 LED are added.
>
> Cc: devicetree@vger.kernel.org
> Cc: Bryan Wu <cooloney@gmail.com>
> Cc: Jingoo Han <jg1.han@samsung.com>
> Cc: Lee Jones <lee.jones@linaro.org>
> Cc: Mark Brown <broonie@kernel.org>
> Cc: Samuel Ortiz <sameo@linux.intel.com>
> Signed-off-by: Milo Kim <milo.kim@ti.com>
> ---
> .../devicetree/bindings/leds/leds-lm3633.txt | 39 +++++
> Documentation/devicetree/bindings/mfd/ti-lmu.txt | 182 ++++++++++++++++++++
> .../bindings/regulator/lm3631-regulator.txt | 49 ++++++
> .../bindings/video/backlight/ti-lmu-backlight.txt | 127 ++++++++++++++
> 4 files changed, 397 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/leds/leds-lm3633.txt
> create mode 100644 Documentation/devicetree/bindings/mfd/ti-lmu.txt
> create mode 100644 Documentation/devicetree/bindings/regulator/lm3631-regulator.txt
> create mode 100644 Documentation/devicetree/bindings/video/backlight/ti-lmu-backlight.txt
>
> diff --git a/Documentation/devicetree/bindings/leds/leds-lm3633.txt b/Documentation/devicetree/bindings/leds/leds-lm3633.txt
> new file mode 100644
> index 0000000..4adeb62
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/leds/leds-lm3633.txt
> @@ -0,0 +1,39 @@
> +TI LMU LM3633 LED device tree bindings
> +
> +Required properties:
> + - compatible: "ti,lm3633-leds"
> + - lvled1-used, lvled2-used, lvled3-used, lvled4-used, lvled5-used, lvled6-used
> + : LED string configuration. Each child node should include this information
> + about which LED string is used.
Which child nodes? They weren't mentioned until this point.

If properties need to be in a child node, mention the child node first,
and make it clear where the properties are expected to be.

> +
> +Optional properties:
> + - chan-name: LED channel name

Any reason to abbreviate "channel" to "chan"?

What's this for?

> + - max-current-milliamp: Max current setting. Unit is mA.

The code and examples treat this as an 8-bit value, but this fact isn't
mentioned here.

> +
> +Example:
> +
> +lm3633@36 {
> + compatible = "ti,lm3633";

It wasn't mentioned that this had to be a sub-node of a "ti,lm3633"
node. Please describe above and refer to the document for the
"ti,lm3633" binding.

[...]

> diff --git a/Documentation/devicetree/bindings/mfd/ti-lmu.txt b/Documentation/devicetree/bindings/mfd/ti-lmu.txt
> new file mode 100644
> index 0000000..2b3ecca
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/ti-lmu.txt
> @@ -0,0 +1,182 @@
> +TI LMU(Lighting Management Unit) device tree bindings
> +
> +TI LMU driver supports lighting devices belows.
> +
> + Name Device tree properties
> + ------ ------------------------
> + LM3532 Backlight
> + LM3631 Backlight and regulator
> + LM3633 Backlight and LED
> + LM3695 Backlight
> + LM3697 Backlight
> +
> +Those have shared device tree properties.
> +
> +Required properties:
> + - compatible: "ti,lm3532", "ti,lm3631", "ti,lm3633", "ti,lm3695", "ti,lm3697"

Should be one of, rather than all at once?

> + - reg: I2C slave address.
> + 0x38 is LM3532
> + 0x29 is LM3631
> + 0x36 is LM3633, LM3697
> + 0x63 is LM3695
> + - ti,enable-gpio: GPIO number of hardware enable pin

We refer to GPIOs with more than a number...

> +
> +For the TI LMU backlight properties, please refer to:
> +Documentation/devicetree/bindings/video/backlight/ti-lmu-backlight.txt
> +
> +For the LM3631 regulator properties, please refer to:
> +Documentation/devicetree/bindings/regulator/lm3631-regulator.txt
> +
> +For the LM3633 LED properties, please refer to:
> +Documentation/devicetree/bindings/leds/leds-lm3633.txt

Are these expected as subnodes?

> +
> +Examples:
> +
> +lm3532@38 {
> + compatible = "ti,lm3532";
> + reg = <0x38>;
> +
> + /* GPIO134 for HWEN pin */
> + ti,enable-gpio = <&gpio5 6 GPIO_ACTIVE_HIGH>;
> +
> + backlight {
> + compatible = "ti,lmu-backlight", "ti,lm3532-backlight";

This looks backwards. The most general string should be later in the
list. This applies elsewhere too.

[...]

> diff --git a/Documentation/devicetree/bindings/video/backlight/ti-lmu-backlight.txt b/Documentation/devicetree/bindings/video/backlight/ti-lmu-backlight.txt
> new file mode 100644
> index 0000000..554ddca
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/video/backlight/ti-lmu-backlight.txt
> @@ -0,0 +1,127 @@
> +TI LMU backlight device tree bindings
> +
> +Required properties:
> + - compatible: One of lists below with "ti,lmu-backlight" should be set.
> + "ti,lm3532-backlight"
> + "ti,lm3631-backlight"
> + "ti,lm3633-backlight"
> + "ti,lm3695-backlight"
> + "ti,lm3697-backlight"

Do you mean that "ti,lmu-backlight" should be a fallback entry in the
compatible list?

> + - hvled1-used, hvled2-used, hvled3-used: Backlight string configuration.
> + Each backlight child node should include this information about
> + which backlight string is used.
> +
> +Optional properties
> + - bl-name: Backlight device name

Why bother abbreviating backlight to bl?

What's this for anyway? Surely something else has to link to the
backlight node, and a meaningful name should be implied.

> + - max-current-milliamp: Max current setting. Unit is mA.

Type?

> + - initial-brightness: Backlight initial brightness

Type? Units?

> + - ramp-up: Light effect for ramp up rate. Unit is msec.
> + - ramp-down: Light effect for ramp down rate. Unit is msec.
> + - pwm-period: PWM period. Only valid for PWM brightness control mode.

Type? Units?

> + - pwms, pwm-names: For the PWM user nodes, please refer to

How many do you expect?

What are each of them for?

What are they named?

Thanks,
Mark.


\
 
 \ /
  Last update: 2014-02-14 11:41    [W:0.082 / U:0.008 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site