Re: [PATCH V2 1/2] dt-bindings: leds: document new led-triggers property

[Jacek Anaszewski <jacek.anaszewski@gmail.com>]

On 01/23/2017 05:45 PM, Rob Herring wrote:
> On Fri, Jan 20, 2017 at 11:35:20PM +0100, Jacek Anaszewski wrote:
>> Hi Rafał,
>>
>> On 01/20/2017 10:56 PM, Rafał Miłecki wrote:
>>> From: Rafał Miłecki <rafal@milecki.pl>
>>>
>>> Some LEDs can be related to particular devices described in DT. This
>>> property allows specifying such relations. E.g. USB LED should usually
>>> be used to indicate some USB port(s) state.
>>>
>>> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
>>> ---
>>> V2: Replace "usb-ports" with "led-triggers" property which is more generic and
>>>     allows specifying other devices as well.
>>>
>>> When bindings patch is related to some followup implementation, they usually go
>>> through the same tree.
>>>
>>> Greg: this patch is based on top of e64b8cc72bf9 ("DT: leds: Improve examples by
>>> adding some context") from kernel/git/j.anaszewski/linux-leds.git . Is there any
>>> way to solve this dependency issue? Or should this patch wait until 3.11 is
>>> released?
>>> ---
>>>  Documentation/devicetree/bindings/leds/common.txt | 16 ++++++++++++++++
>>>  1 file changed, 16 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/leds/common.txt b/Documentation/devicetree/bindings/leds/common.txt
>>> index 24b656014089..17632a041196 100644
>>> --- a/Documentation/devicetree/bindings/leds/common.txt
>>> +++ b/Documentation/devicetree/bindings/leds/common.txt
>>> @@ -49,6 +49,17 @@ Optional properties for child nodes:
>>>  - panic-indicator : This property specifies that the LED should be used,
>>>  		    if at all possible, as a panic indicator.
>>>  
>>> +- led-triggers : List of devices that should trigger this LED activity. Some
>>> +		 LEDs can be related to a specific device and should somehow
>>> +		 indicate its state. E.g. USB 2.0 LED may react to device(s) in
>>> +		 a USB 2.0 port(s). Another common example is switch or router
>>> +		 with multiple Ethernet ports each of them having its own LED
>>> +		 assigned (assuminled-trigger-usbportg they are not hardwired).
>>> +		 In such cases this property should contain phandle(s) of
>>> +		 related device(s). In many cases LED can be related to more
>>> +		 than one device (e.g. one USB LED vs. multiple USB ports) so a
>>> +		 list of entries can be specified.
>>> +
>>
>> This implies that it is possible to define multiple triggers for
>> a LED class device but it is not supported by LED Trigger core.
> 
> Not really relevant for designing (and future proofing) a binding.

But relevant for LED Trigger ABI, which would have to be changed to
support the semantics in this form. I think that changing the property
name to linux,trigger-sources would make its purpose more clear.

Note that we have to also associate somehow this property with the
triggers that will make use of the information it provides. I can
imagine also other compound triggers that could benefit on it.

What follows, the trigger sources would trigger LED activity only
if the LED class device was assigned appropriate trigger. We need to
define somewhere which triggers support this property.

Trigger specific bindings?

>> There is linux,default-trigger property which allows to define one
>> trigger that will be initially assigned.
>>
>> I am aware that this is renamed usb-ports property from v1,
>> that attempts to address Rob's comment, but we can't do that this way.
>> Maybe usb-ports property could be documented in led-trigger-usbport's
>> specific bindings and a reference to it could be added next to the
>> related entry on the list of the available LED triggers (which is
>> actually missing) in Documentation/devicetree/bindings/leds/common.txt.
> 
> I'm not going to accept something specific to USB. So the choice is make 
> the existing property work for USB or design something more flexible and 
> generic.


-- 
Best regards,
Jacek Anaszewski