Re: [PATCH 1/4] dt-bindings: leds: document property for LED triggers

[Rob Herring <robh+dt@kernel.org>]

On Wed, Mar 1, 2017 at 3:04 PM, Jacek Anaszewski <jacek.anaszewski@gmail.com> wrote:
> On 02/28/2017 11:12 PM, Rob Herring wrote:
>> On Tue, Feb 28, 2017 at 3:51 PM, Rafał Miłecki <zajec5@gmail.com> wrote:
>>> On 02/28/2017 10:38 PM, Jacek Anaszewski wrote:
>>>>
>>>> I think that it would be simpler if we could initially see
>>>> a complete sample dts implementation containing all required DT
>>>> nodes. The example could contain timer trigger as well as usb-port
>>>> trigger specific bindings.
>>>
>>>
>>> Please take a look at attached patch. I used it on Tenda AC9 with:
>>
>> I'm not sure about this extra level of indirection. I don't see the need.
>>
>>> usb_trigger: usb-trigger {
>>>         trigger-type = "usbport";
>>
>> Why do we need to know the type? The trigger device knows what type it
>> is. All we should need to know here is what device(s) controls an LED.
>> The rest the kernel can figure out.
>
> The thing is that in the proposed approach the trigger is not necessary
> a device. The trigger node is here only a container with initialization
> data.

That sounds like a questionable use of DT.

> We could have e,g, two such nodes with different set of ports
> (say usb1-trigger and usb2-trigger). Then if we had two LED nodes usb1
> and usb2, the former could have its triggers property initialized
> to &usb1-trigger and the latter to &usb2-trigger. Thanks to that both
> LEDs after executing "echo usbport > triggers" would listen to events
> from different set of usb ports.

This would still work if the trigger property points directly

> Also, e.g. for the timer trigger we could define two separate DT nodes
> with different delay intervals.

That sounds like user config to me.

>
>>>         ports = <&ohci_port1>, <&ehci_port1>;
>>> };
>>>
>>> usb {
>>>         label = "bcm53xx:blue:usb";
>>>         gpios = <&chipcommon 1 GPIO_ACTIVE_HIGH>;
>>>         triggers = <&usb_trigger>;
>>> };
>>
>
> --
> Best regards,
> Jacek Anaszewski