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 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.

Where did he point this out? Do you think about the current version
of the leds/common.txt documentation or there was some other message,
that I don't see?

Besides, I can't understand your point. Could you express it in other
words, please?

The three drivers I've looked also require these properties, which I think
is in the line with the above.

These properties were introduced two months ago and there is no merged
driver which require them. It means that the drivers that are currently
in the mainline assume that they can set the maximum current (please
note that max-microamp property refers to non-flash LEDs too).
The modifications I proposed just validate the current status.

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.

I think this is worth considering now as we can't change this later without
breaking something.


--
Best Regards,
Jacek Anaszewski
--
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




[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux