Hi Pavel,
On 11/18/2014 02:21 PM, Pavel Machek wrote:
Hi!
@@ -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.
You got this wrong, sorry.
In my case, there are three physical devices:
adp1653
white LED
red LED
You've mentioned that your white led is torch/flash and indicator
is the red led. They are probably connected to the HPLED and
ILED pins of the ADP1653 device respectively. The device is just
a regulator, that delivers electric current to the leds connected
to it. Kernel cannot directly activate leds, but has to talk
to the device through I2C bus. One I2C device can have only one
related device tree binding.
Each LED should have an label, and probably default trigger -- default
trigger for red one should be "we are recording video" and for white
should be "this is flash for default camera".
default-trigger is not mandatory, the device doesn't have to have
associated led-trigger. I think that you should look at
Documentation/leds/leds-class.txt and drivers/leds/triggers for
more detailed information. In a nutshell triggers are kernel
sources of led events. You can set e.g. "heartbeat", "timer"
trigger etc.
As for now the driver belongs to the V4L2 subsystem it doesn't
support triggers. Moreover your event "we are recording a video"
should be activated by setting V4L2_CID_FLASH_INDICATOR_INTENSITY
v4l2 control followed by V4L2_FLASH_LED_MODE_TORCH. Your event
"this is flash for default camera" seems to be flash strobe,
that can be activated by setting V4L2_CID_FLASH_STROBE control.
The driver by default sets the indicator current for both actions
to the value previously set with V4L2_CID_FLASH_INDICATOR_INTENSITY.
If the hardware LED changes with one that needs different current, the
block for the adp1653 stays the same, but white LED block should be
updated with different value.
I think that you are talking about sub nodes. Indeed I am leaning
towards this type of design.
default-trigger property should also be an array of strings.
That is not how it currently works.
OK, I agree.
I agree that flash-timeout should be per led - an array, similarly
as in case of iout's.
Agreed about per-led, disagreed about the array. As all the fields
would need arrays, and as LED system currently does not use arrays for
label and linux,default-trigger, I believe we should follow existing
design and model it as three devices. (It _is_ physically three devices.)
Right, I missed that the leds/common.txt describes child node.
I propose following modifications to the binding:
Optional properties for child nodes:
- iout-mode-led : maximum intensity in microamperes of the LED
(torch LED for flash devices)
- iout-mode-flash : initial intensity in microamperes of the
flash LED; it is required to enable support
for the flash led
- iout-mode-indicator : initial intensity in microamperes of the
indicator LED; it is required to enable support
for the indicator led
- max-iout-mode-led : maximum intensity in microamperes of the LED
(torch LED for flash devices)
- max-iout-mode-flash : maximum intensity in microamperes of the
flash LED
- max-iout-mode-indicator : maximum intensity in microamperes of the
indicator LED
- flash-timeout : timeout in microseconds after which flash
led is turned off
system-status {
label = "max77693_1";
iout-mode-led = <500>;
max-iout-mode-led = <500>;
...
};
camera-flash1 {
label = "max77693_2";
iout-mode-led = <500>;
iout-mode-flash = <1000>;
iout-mode-indicator = <100>;
max-iout-mode-led = <500>;
max-iout-mode-flash = <1000>;
max-iout-mode-indicator = <100>;
flash-timeout = <1000>;
...
};
I propose to avoid name "torch", as for ordinary leds it would
be misleading.
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
