On Wed, Feb 24, 2016 at 1:34 PM, David Rivshin (Allworx) <drivshin.allworx@xxxxxxxxx> wrote: > 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? You should put in DT whatever makes sense for the controls the h/w has. If you only have a PWM, then only max-brightness makes sense and led-max-microamp does not. Rob -- 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