On 11/18/2014 12:32 PM, Pavel Machek wrote:
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.
Yeah, that makses sense.
@@ -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.
No, sorry. The Documentation/devicetree/bindings/leds/common.txt
describes binding for _one LED_. Yes, your device can have two leds,
so your devices should have two such blocks in the device tree... Each
led should have its own label and default trigger, for example. And I
guess flash-timeout be per-LED, too.
I think that a device tree binding describes a single physical device.
No matter how many sub-leds a device controls, it is still one
piece of hardware.
default-trigger property should also be an array of strings.
I agree that flash-timeout should be per led - an array, similarly
as in case of iout's.
Would it make sense to include "-uA" and "-usec" suffixes in the dt
property names?
I don't see this type of naming anywhere. The documentation should
contain the information about units. Nonetheless I am not a device tree
specialist, so an opinion of the relevant maintainer will be welcome.
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.
Yes, that would work. I have used
+ max-flash-timeout-usec = <500000>;
+ max-flash-intensity-uA = <320000>;
+ max-torch-intensity-uA = <50000>;
+ max-indicator-intensity-uA = <17500>;
. Which is pretty similar. (Actually, maybe the longer property names
are easier to understand for more people?) (And yes, I should probably
separate red and white led into separate groups).
Documentation of a binding should provide sufficient explanation
for the people to understand what a property is for.
And I would stay by arrays as argued above.
[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.
Thanks for cc and thanks for links!
I see max77693 has two different "white" leds, aat1290 has one "white"
led, and neither of them has secondary red led for indication?
You're right.
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].
Thanks for the links. It seems that aside from moving adp1653 driver
to device tree, it should be moved to the LED framework, but that's a
topic for another patch.
Like I mentioned in the previous message the LED Flash class patch
isn't in its final shape yet. Nonetheless I think that we should
agree on the leds/common.txt documentation improvements and
define DT documentation for adp1653 accordingly.
Regards,
Jacek
--
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
