Hi Sylwester, On 04/08/2015 12:03 PM, Sylwester Nawrocki wrote:
Hello, On 31/03/15 15:52, Jacek Anaszewski wrote:Description of flash LEDs related properties was not precise regarding the state of corresponding settings in case a property is missing. Add relevant statements. Removed is also the requirement making the flash-max-microamp property obligatory for flash LEDs. It was inconsistent as the property is defined as optional. Devices which require the property will have to assert this in their DT bindings. Signed-off-by: Jacek Anaszewski <j.anaszewski@xxxxxxxxxxx> Acked-by: Kyungmin Park <kyungmin.park@xxxxxxxxxxx> Cc: Bryan Wu <cooloney@xxxxxxxxx> Cc: Richard Purdie <rpurdie@xxxxxxxxx> Cc: Pavel Machek <pavel@xxxxxx> Cc: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx> Cc: devicetree@xxxxxxxxxxxxxxx --- Documentation/devicetree/bindings/leds/common.txt | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/Documentation/devicetree/bindings/leds/common.txt b/Documentation/devicetree/bindings/leds/common.txt index 747c538..21a25e4 100644 --- a/Documentation/devicetree/bindings/leds/common.txt +++ b/Documentation/devicetree/bindings/leds/common.txt @@ -29,13 +29,15 @@ Optional properties for child nodes: "ide-disk" - LED indicates disk activity "timer" - LED flashes at a fixed, configurable rate -- max-microamp : maximum intensity in microamperes of the LED - (torch LED for flash devices) -- flash-max-microamp : maximum intensity in microamperes of the - flash LED; it is mandatory if the LED should - support the flash mode -- flash-timeout-us : timeout in microseconds after which the flash - LED is turned off +- max-microamp : Maximum intensity in microamperes of the LED + (torch LED for flash devices). If omitted this will default + to the maximum current allowed by the device. +- flash-max-microamp : Maximum intensity in microamperes of the flash LED. + If omitted this will default to the maximum + current allowed by the device. +- flash-timeout-us : Timeout in microseconds after which the flash + LED is turned off. If omitted this will default to the + maximum timeout allowed by the device.Sorry about late comments on that, but since we can still change these properties and it seems we're going to do that, I'd like throw in my few preferences on the colour of this bike... IMO "max-microamp" is a poor property name, how about: s/max-microamp/led-max-current-ua, s/flash-max-microamp/flash-max-current-ua, so we have more consistent set of properties like: led-max-current-ua flash-max-current-ua flash-timeout-us
The "-microamp' suffix is consistent with regulator bindings. Please refer to [1].
Also expressing light intensity in micro-amperes seems technically wrong. I would propose to substitute word "intensity in microamperes" with "LED supply current in microamperes".
OK, I will address this in the next version of the patch.
I also think we should require the maximum current properties and the driver should warn if they are missing and limit current to some potentially safe value, e.g. small fraction of the maximum current.
TBD.
Also from the description it should be clear whether the current limits refer to capabilities of a LED or the desired settings we want to be applied at the LED driver device. We could, for example, add a sentence after the above 3 properties: "Required properties for Flash LEDs: - led-max-current-ua - flash-max-current-ua - flash-timeout-us These properties determine a LED driver IC settings required for safe operation." Or something along these lines.
OK. [1] http://www.spinics.net/lists/linux-leds/msg02674.html -- Best Regards, Jacek Anaszewski -- 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
