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

2016-09-28 Thread Pavel Machek
On Wed 2016-09-28 12:02:41, Florian Vaussard wrote:
> Hi Pavel,
> 
> Le 24. 09. 16 à 13:58, Pavel Machek a écrit :
> > Hi!
> > 
> >> +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";
> > ...
> >> +  led1b@1 {
> >> +  label = "ncp:power:blue";
> >> +  reg = <1>;
> > 
> > Actually... the three LEDs are packaged such as this is one colorful
> > light to the user, right? Some day we'll need to group them, so that
> > kernel can automatically tell this is one led, and probably add extra
> > attributes, such as values that produce white light.
> > 
> 
> Actually, it's up to the hardware designer to choose. On my board for 
> instance,
> this chip is driving an RGB LED, but it can really drive three independent 
> LEDs
> if you want.

Yup. And driving RGB LED is really a bit different from driving three
independent LEDs: you'd for example like to be able to set the RGB LED
to white, and you need to know relative intensities for that.

So it would be good to have hardware description that captures
difference between RGB LED and three LEDs.

(And then, we'll want pattern engine to drive that. One day :-) ).

> I agree that the RGB case is quite common nowadays and currently not very well
> managed by the LED subsystem. But I do not think that this is specific to this
> driver.

No, it is not.

Best regards,
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature


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

2016-09-28 Thread Florian Vaussard
Hello Jacek,

Le 24. 09. 16 à 21:06, Jacek Anaszewski a écrit :
> On 09/24/2016 01:58 PM, Pavel Machek wrote:
>> Hi!
>>
>>> +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";
>> ...
>>> +led1b@1 {
>>> +label = "ncp:power:blue";
>>> +reg = <1>;
>>
>> Actually... the three LEDs are packaged such as this is one colorful
>> light to the user, right? Some day we'll need to group them, so that
>> kernel can automatically tell this is one led, and probably add extra
>> attributes, such as values that produce white light.
> 
> We could try out the trigger approach we discussed few months ago.
> Unfortunately I currently don't have enough time to propose the
> implementation. Probably this work could be done on the occasion of
> addition of RGB LED class driver like this, if the author had free
> bandwidth for that.
> 

Unfortunately my bandwidth is pretty well used at the moment :)

Best regards,
Florian


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

2016-09-28 Thread Florian Vaussard
Hi Pavel,

Le 24. 09. 16 à 13:58, Pavel Machek a écrit :
> Hi!
> 
>> +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";
> ...
>> +led1b@1 {
>> +label = "ncp:power:blue";
>> +reg = <1>;
> 
> Actually... the three LEDs are packaged such as this is one colorful
> light to the user, right? Some day we'll need to group them, so that
> kernel can automatically tell this is one led, and probably add extra
> attributes, such as values that produce white light.
> 

Actually, it's up to the hardware designer to choose. On my board for instance,
this chip is driving an RGB LED, but it can really drive three independent LEDs
if you want.

I agree that the RGB case is quite common nowadays and currently not very well
managed by the LED subsystem. But I do not think that this is specific to this
driver.

Best regards,
Florian


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

2016-09-24 Thread Jacek Anaszewski

On 09/24/2016 01:58 PM, Pavel Machek wrote:

Hi!


+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";

...

+   led1b@1 {
+   label = "ncp:power:blue";
+   reg = <1>;


Actually... the three LEDs are packaged such as this is one colorful
light to the user, right? Some day we'll need to group them, so that
kernel can automatically tell this is one led, and probably add extra
attributes, such as values that produce white light.


We could try out the trigger approach we discussed few months ago.
Unfortunately I currently don't have enough time to propose the
implementation. Probably this work could be done on the occasion of
addition of RGB LED class driver like this, if the author had free
bandwidth for that.

--
Best regards,
Jacek Anaszewski


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

2016-09-24 Thread Pavel Machek
Hi!

> +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";
...
> + led1b@1 {
> + label = "ncp:power:blue";
> + reg = <1>;

Actually... the three LEDs are packaged such as this is one colorful
light to the user, right? Some day we'll need to group them, so that
kernel can automatically tell this is one led, and probably add extra
attributes, such as values that produce white light.

Best regards,
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature


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

2016-09-23 Thread Rob Herring
On Fri, Sep 16, 2016 at 01:34:31PM +0200, 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 
> ---
>  .../devicetree/bindings/leds/leds-ncp5623.txt  | 60 
> ++
>  1 file changed, 60 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/leds/leds-ncp5623.txt

Acked-by: Rob Herring 


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

2016-09-16 Thread Florian Vaussard
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 
---
 .../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 000..d83e509
--- /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: see Documentation/devicetree/bindings/leds/common.txt
+  - linux,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 = <2>;
+   };
+
+   led1b@1 {
+   label = "ncp:power:blue";
+   reg = <1>;
+   led-max-microamp = <2>;
+   };
+
+   led1g@2 {
+   label = "ncp:power:green";
+   reg = <2>;
+   led-max-microamp = <2>;
+   };
+};
-- 
2.5.5