Thread (33 messages) 33 messages, 6 authors, 2016-03-02

Re: [PATCH RFC 2/3] DT: leds: Add binding for the ISSI IS31FL32xx family of LED drivers

From: Rob Herring <robh+dt@kernel.org>
Date: 2016-02-24 19:49:46
Also in: linux-leds

On Wed, Feb 24, 2016 at 1:34 PM, David Rivshin (Allworx)
[off-list ref] wrote:
On Wed, 24 Feb 2016 17:04:49 +0100
Jacek Anaszewski [off-list ref] wrote:
quoted
Hi David,

Thanks for the patch.

On 02/23/2016 07:17 PM, David Rivshin (Allworx) wrote:
quoted
From: David Rivshin <redacted>

This adds a binding description for the is31fl3236/35/18/16 I2C LED
drivers.

Signed-off-by: David Rivshin <redacted>
---
  .../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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help