Re: [RFC] adp1653: Add device tree bindings for LED controller

From: Jacek Anaszewski
Date: Tue Nov 18 2014 - 05:05:16 EST


On 11/18/2014 09:46 AM, Pavel Machek wrote:
On Tue 2014-11-18 09:09:09, Jacek Anaszewski wrote:
Hi Pavel, Sakari,

On 11/17/2014 03:58 PM, Sakari Ailus wrote:
Hi Pavel,

On Sun, Nov 16, 2014 at 08:59:28AM +0100, Pavel Machek wrote:
For device tree people: Yes, I know I'll have to create file in
documentation, but does the binding below look acceptable?

I'll clean up driver code a bit more, remove the printks. Anything
else obviously wrong?

Jacek Anaszewski is working on flash support for LED devices. I think it'd
be good to sync the DT bindings for the two, as the types of devices
supported by the LED API and the V4L2 flash API are quite similar.

Cc Jacek.

I've already submitted a patch [1] that updates leds common bindings.
I hasn't been merged yet, as the related LED Flash class patch [2]
still needs some indicator leds related discussion [3].

I think this is a good moment to discuss the flash related led common
bindings.

Part of problem is that adp1653 is not regarded as "LED" device by
current kernel driver.

It doesn't prevent us from keeping the flash devices related
DT bindings unified across kernel subsystems. The DT bindings
docs for the adp1653 could just provide a reference to the
led/common.txt bindings. In the future, when LED Flash
class will be merged, all the V4L2 Flash drivers might be
moved to the LED subsystem to gain the LED subsystem support.

[1] http://www.spinics.net/lists/linux-leds/msg02121.html

@@ -3,6 +3,17 @@ Common leds properties.
Optional properties for child nodes:
- label : The label for this LED. If omitted, the label is
taken from the node name (excluding the unit address).
+- iout-torch : Array of maximum intensities in microamperes of the
torch
+ led currents in order from sub-led 0 to N-1, where N is the
number
+ of torch sub-leds exposed by the device
+- iout-flash : Array of maximum intensities in microamperes of the
flash
+ led currents in order from sub-led 0 to N-1, where N is the
number
+ of flash sub-leds exposed by the device
+- iout-indicator : Array of maximum intensities in microamperes of
+ the indicator led currents in order from sub-led 0 to N-1,
+ where N is the number of indicator sub-leds exposed by the device
+- flash-timeout : timeout in microseconds after which flash led
+ is turned off

- linux,default-trigger : This parameter, if present, is a
string defining the trigger assigned to the LED. Current
triggers are:
@@ -19,5 +30,10 @@ Examples:
system-status {
label = "Status";
linux,default-trigger = "heartbeat";
+ iout-torch = <500 500>;
+ iout-flash = <1000 1000>;
+ iout-indicator = <100 100>;
+ flash-timeout = <1000>;
+
...
};

I don't get it; system-status describes single LED, why are iout-torch
(and friends) arrays of two?

Some devices can control more than one led. The array is for such
purposes. The system-status should be probably renamed to
something more generic for both common leds and flash leds,
e.g. system-led.

Also, at least on adp1653, these are actually two leds -- white and
red. Torch and flash is white led, indicator is red led.

Then you should define three properties:

iout-torch = <[uA]>;
iout-flash = <[uA]>;
iout-indicator = <[uA]>;

iout-torch and iout-flash properties would determine the current
for the white led in the torch and flash modes respectively and
the iout-indicator property would determine the current for
the indicator led.

[2] http://www.spinics.net/lists/linux-media/msg83100.html
[3] http://www.spinics.net/lists/linux-leds/msg02472.html

What device are you using for testing? Can you cc me on future
patches?

I am using max77693 [1] and aat1290 [2]. OK, I will add you on cc.

Why do we need complex "flash LED class" support, and where is the
V4L2 glue?

The rationale for unification of the LED and V4L2 Flash API
can be found in the discussion [3]. The glue is the v4l2-flash [4]
module which exposes a sub-device, that controls a LED Flash class
device with use of LED Flash class API.

The v4l2-flash sub-device registers with v4l2-async API
in a media device. Exemplary support for v4l2-flash
sub-devices is added to the exynos4-is driver in the patch [5].

Best Regards,
Jacek Anaszewski

[1] http://www.spinics.net/lists/linux-leds/msg02326.html
[2] http://www.spinics.net/lists/linux-media/msg81079.html
[3] http://www.spinics.net/lists/linux-media/msg69012.html
[4] http://www.spinics.net/lists/linux-leds/msg02322.html
[5] http://www.spinics.net/lists/linux-leds/msg02323.html

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/