Re: [PATCH v2 1/2] leds: ncp5623: Add device tree binding documentation

[Florian Vaussard]

Hi Jacek,

Le 27. 06. 16 à 10:11, Jacek Anaszewski a écrit :
> Hi Florian,
> 
> On 06/27/2016 09:03 AM, Florian Vaussard wrote:
>> Add device tree binding documentation for On Semiconductor NCP5623 I2C
>> LED driver. The driver can independently control the PWM of the 3
>> channels with 32 levels of intensity.
>>
>> The current delivered by the current source can also be controlled. To
>> do so, the led-max-microamp property is used by each LED sub-node. The
>> maximum value is then found and used as a limit to compute the final
>> intensity of the current source. If a LED happens to have a lower limit,
>> the PWM is then used to limit the current to the requested value.
>>
>> In order to control the current source, it is also necessary to know
>> the current on the Iref pin, hence the onnn,led-iref-microamp property.
>> It is usually set using an external bias resistor, following
>> Iref = Vref/Rbias with Vref=0.6V.
>>
>> Signed-off-by: Florian Vaussard <florian.vaussard@xxxxxxxxxx>
>> ---
>>   .../devicetree/bindings/leds/leds-ncp5623.txt      | 60 ++++++++++++++++++++++
>>   1 file changed, 60 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/leds/leds-ncp5623.txt
>>
>> diff --git a/Documentation/devicetree/bindings/leds/leds-ncp5623.txt
>> b/Documentation/devicetree/bindings/leds/leds-ncp5623.txt
>> new file mode 100644
>> index 0000000..77dd7ad
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/leds/leds-ncp5623.txt
>> @@ -0,0 +1,60 @@
>> +* ON Semiconductor - NCP5623 3-Channel LED Driver
>> +
>> +The NCP5623 is a 3-channel I2C LED driver. The brightness of each
>> +channel can be independently set using 32 levels. Each LED is represented
>> +as a sub-node of the device.
>> +
>> +Required properties:
>> +  - compatible: Should be "onnn,ncp5623"
>> +  - reg: I2C slave address (fixed to 0x38)
>> +  - #address-cells: must be 1
>> +  - #size-cells: must be 0
>> +  - onnn,led-iref-microamp: Current on the Iref pin in microampere. It depends
>> +    on the value of the external bias resistor Rbias, following
>> +    Iref = Vref / Rbias with Vref = 0.6V. This is used to set the intensity of
>> +    the current that can be provided by the internal current source, based on
>> +    the maximum current permitted by LED sub-nodes (see below), but no more than
>> +    Imax = 2400 * Iref.
>> +
>> +LED sub-nodes
>> +=============
>> +
>> +Required properties:
>> +  - reg : LED channel number (0..2)
>> +  - led-max-microamp: Maximum allowable current inside the LED in microampere.
>> +    This property is used to limit the PWM ratio, based on the intensity of the
>> +    internal current source (see above).
>> +
>> +Optional properties:
>> +  - label: Used for naming LEDs
> 
> Instead of the above description use same reference to common led
> bindings as below.
> 

Ok.

BTW I saw the thread about the "default-state" property on the DT list and
realized that it was not implemented in my driver. I wonder if standard LED
properties (with label and default-trigger) should not be parsed by LED core
(i.e. using a helper function) to avoid such mistake. This could also remove
some boilerplate code in other LED drivers.

Best,
Florian

>> +  - default-trigger: see Documentation/devicetree/bindings/leds/common.txt
>> +
>> +Example
>> +=======
>> +
>> +led1: ncp5623@38 {
>> +    #address-cells = <1>;
>> +    #size-cells = <0>;
>> +    compatible = "onnn,ncp5623";
>> +    reg = <0x38>;
>> +    onnn,led-iref-microamp = <10>;
>> +
>> +    led1r@0 {
>> +        label = "ncp:power:red";
>> +        linux,default-trigger = "default-on";
>> +        reg = <0>;
>> +        led-max-microamp = <20000>;
>> +    };
>> +
>> +    led1b@1 {
>> +        label = "ncp:power:blue";
>> +        reg = <1>;
>> +        led-max-microamp = <20000>;
>> +    };
>> +
>> +    led1g@2 {
>> +        label = "ncp:power:green";
>> +        reg = <2>;
>> +        led-max-microamp = <20000>;
>> +    };
>> +};
>>
> 
>