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

[Florian Vaussard]

Le 27. 06. 16 à 10:33, Jacek Anaszewski a écrit :
> On 06/27/2016 10:30 AM, Florian Vaussard wrote:
>> 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.
> 
> Yes, generic DT parser is a nice-to-have feature for the LED subsystem.
> Would you like to implement it?
> 

Sure, if it can be useful. I will have a look in the coming days.

Best,
Florian