Re: [PATCH v8 1/2] dt: bindings: lm3692x: Add bindings for lm3692x LED driver

From: Rob Herring
Date: Mon Dec 11 2017 - 11:06:12 EST


On Mon, Dec 11, 2017 at 9:51 AM, Rob Herring <robh@xxxxxxxxxx> wrote:
> On Thu, Dec 7, 2017 at 5:04 PM, Dan Murphy <dmurphy@xxxxxx> wrote:
>> Rob
>>
>>
>> On 12/07/2017 04:49 PM, Rob Herring wrote:
>>> On Tue, Dec 05, 2017 at 02:46:29PM -0600, Dan Murphy wrote:
>>>> This adds the devicetree bindings for the LM3692x
>>>> I2C LED string driver.
>>>>
>>>> Acked-by: Pavel Machek <pavel@xxxxxx>
>>>> Signed-off-by: Dan Murphy <dmurphy@xxxxxx>
>>>> ---
>>>>
>>>> v8 - Added address-cells and size-cells as well as child node reg - https://patchwork.kernel.org/patch/10091259/
>>>> v7 - No changes - https://patchwork.kernel.org/patch/10087475/
>>>> v6 - No changes -https://patchwork.kernel.org/patch/10085567/
>>>> v5 - No Changes - https://patchwork.kernel.org/patch/10081071/
>>>> v4 - Fix example node, added trigger entry, removed ambiguous x for compatible and
>>>> added common.txt pointer for label - https://patchwork.kernel.org/patch/10060107
>>>> v3 - No changes
>>>> v2 - No changes - https://patchwork.kernel.org/patch/10056677/
>>>>
>>>> .../devicetree/bindings/leds/leds-lm3692x.txt | 47 ++++++++++++++++++++++
>>>> 1 file changed, 47 insertions(+)
>>>> create mode 100644 Documentation/devicetree/bindings/leds/leds-lm3692x.txt
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/leds/leds-lm3692x.txt b/Documentation/devicetree/bindings/leds/leds-lm3692x.txt
>>>> new file mode 100644
>>>> index 000000000000..84f69342d879
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/leds/leds-lm3692x.txt
>>>> @@ -0,0 +1,47 @@
>>>> +* Texas Instruments - LM3692x Highly Efficient White LED Driver
>>>> +
>>>> +The LM3692x is an ultra-compact, highly efficient,
>>>> +white-LED driver designed for LCD display backlighting.
>>>> +
>>>> +The main difference between the LM36922 and LM36923 is the number of
>>>> +LED strings it supports. The LM36922 supports two strings while the LM36923
>>>> +supports three strings.
>>>> +
>>>> +Required properties:
>>>> + - compatible:
>>>> + "ti,lm36922"
>>>> + "ti,lm36923"
>>>> + - reg : I2C slave address
>>>> + - #address-cells : 1
>>>> + - #size-cells : 0
>>>> +
>>>> +Optional properties:
>>>> + - label : see Documentation/devicetree/bindings/leds/common.txt
>>>
>>> Should be a child prop.
>>
>> Thanks I forgot to move this to Optional Child Properties.
>>
>>>
>>>> + - enable-gpios : gpio pin to enable/disable the device.
>>>> + - vled-supply : LED supply
>>>> + - linux,default-trigger : (optional)
>>>> + see Documentation/devicetree/bindings/leds/common.txt
>>>
>>> Ditto.
>>
>> Ack
>>
>>>
>>>> +
>>>> +Required child properties:
>>>> + - reg : 0
>>>> +
>>>> +Example:
>>>> +
>>>> +lm3692x@36 {
>>>
>>> leds@36
>>
>> Rob why does this need to be leds? Is this because it would be a child of an I2C node?
>
> Because node names are supposed to follow the generic type of device
> they are (e.g. serial, ethernet, bus, etc.), not the model of device.
> And yes, there are many examples that don't follow this.

Actually, it should be led-controller and led or leds be used for the
LED child nodes (and gpio-led or pwd-led bindings), so like this:

led-controller@12 {
reg = <12>;

led@0 {
reg = <0>;
};
led@1 {
reg = <1>;
};
};

Rob