On Wed, 24 Feb 2016 17:04:49 +0100
Jacek Anaszewski <j.anaszewski@xxxxxxxxxxx> wrote:
> Hi David,
>
> Thanks for the patch.
>
> On 02/23/2016 07:17 PM, David Rivshin (Allworx) wrote:
> > From: David Rivshin <drivshin@xxxxxxxxxxx>
> >
> > This adds a binding description for the is31fl3236/35/18/16 I2C LED
> > drivers.
> >
> > Signed-off-by: David Rivshin <drivshin@xxxxxxxxxxx>
> > ---
> > .../devicetree/bindings/leds/leds-is31fl32xx.txt | 51 ++++++++++++++++++++++
> > 1 file changed, 51 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/leds/leds-is31fl32xx.txt
> >
> > diff --git a/Documentation/devicetree/bindings/leds/leds-is31fl32xx.txt b/Documentation/devicetree/bindings/leds/leds-is31fl32xx.txt
> > new file mode 100644
> > index 0000000..0a05a1d
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/leds/leds-is31fl32xx.txt
> > @@ -0,0 +1,51 @@
> > +Binding for ISSI IS31FL32xx LED Drivers
> > +
> > +The IS31FL32xx family of LED drivers are I2C devices with multiple
> > +constant-current channels, each with independent 256-level PWM control.
> > +Each LED is represented as a sub-node of the device.
> > +
> > +Required properties:
> > +- compatible: one of
> > + issi,is31fl3236
> > + issi,is31fl3235
> > + issi,is31fl3218
> > + issi,is31fl3216
> > +- reg: I2C slave address
> > +- address-cells : must be 1
> > +- size-cells : must be 0
> > +
> > +LED sub-node properties:
> > +- reg : LED channel number (1..N)
> > +- max-brightness : (optional) Maximum brightness possible for the LED.
>
> Please use led-max-microamp instead. You can refer to
> Documentation/devicetree/bindings/leds/common.txt for detailed
> description.
FYI, I think I got that property from leds-pwm, since these use PWMs
internally, although I don't actually use it myself.
I can see how led-max-microamp could be a useful property, however
I'm uncertain about computing cdev->max_brightness from it as these
devices look to control their max current via an external resistor.
I suppose either the resistor value, or the resultant max-current
would need to be given in the devicetree, and then that used along
with led-max-microamp to compute cdev->max_brightness. Although I'd
want it to be optional as I suspect most users don't need any
restriction. Also, I don't think I can properly test the result,
which always makes me nervous.
Would it be acceptable to simply remove the max-brightness property
without adding led-max-microamp? It can always be added if a user
turns out to need it in the future. Or do you feel strongly that
led-max-microamp should be in the binding from the start?
>
> > + Default is 255.
> > +- label : (optional)
> > + see Documentation/devicetree/bindings/leds/common.txt
> > +- linux,default-trigger : (optional)
> > + see Documentation/devicetree/bindings/leds/common.txt
> > +
> > +
> > +Example:
> > +
> > +leds: is31fl3236@3c {
> > + compatible = "issi,is31fl3236";
> > + reg = <0x3c>;
>
> You're missing address-cells and size-cells in this example.
Indeed, I missed that when I renamed the child node property
from 'chan' to 'reg'.
>
> > +
> > + led@1 {
> > + reg = <1>;
> > + label = "EB:blue:usr0";
> > + max-brightness = <128>;
> > + };
> > + led@2 {
> > + reg = <2>;
> > + label = "EB:blue:usr1";
> > + };
> > + ...
> > + led@36 {
> > + reg = <36>;
> > + label = "EB:blue:usr35";
> > + max-brightness = <255>;
> > + };
> > +};
> > +
> > +For more product information please see the link below:
> > +http://www.issi.com/US/product-analog-fxled-driver.shtml
> > \ No newline at end of file
> >
>
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
