Re: [PATCH v4 01/12] DT: leds: Improve description of flash LEDs related properties

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




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 devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]
  Powered by Linux