Hi Jacek, On Wed, Apr 08, 2015 at 10:54:52AM +0200, Jacek Anaszewski wrote: > Hi Sakari, > > On 04/03/2015 02:09 PM, Sakari Ailus wrote: > >Hi Jacek, > > > >On Tue, Mar 31, 2015 at 03:52:37PM +0200, 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. > >> > >> > >> Examples: > > > >Pavel pointed out that the brightness between maximum current and the > >maximum *allowed* another current might not be noticeable, leading a > >potential spelling error to cause the LED being run at too high current. > > I think that a board designed so that it can be damaged because of > software bugs should be considered not eligible for commercial use. > Any self-esteeming manufacturer will not connect a LED to the output > that can produce the current greater than the LED's absolute maximum > current. The maximum current *is* used to prevent potential hardware damage. This is how mobile phones typically are, probably also the one you're using. :-) I don't believe there's really a difference between vendors in this respect. We still lack a proper way to model the temperature of the flash LED, so what we have now is a bit incomplete, but at least it prevents causing damage unintentionally. > The DT properties could be useful for devices like aat1290 device I was > writing a driver for, which has the maximum current and timeout values > depending on corresponding capacitor and resistor values respectively. > Such devices should make the properties required in their bindings. > > >The three drivers I've looked also require these properties, which I think > >is in the line with the above. > > > >How about either dropping the patch, or changing maximum to minimum and > >will to should? The drivers could also behave this way instead of requiring > >the properties, but I don't think there's anything wrong with requiring the > >properties either. > > As I mentioned in the previous message in this subject, the max-microamp > property refers also to non-flash LEDs. Since existing LED class devices > does not require them, then it should be left optional and default to > max. It would however be inconsistent with flash LEDs related > properties. I do agree with Pavel here, these should be mandatory (at least for new drivers) OR default to minimum. -- Regards, Sakari Ailus e-mail: sakari.ailus@xxxxxx XMPP: sailus@xxxxxxxxxxxxxx -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
