On 02/01/2017 10:55 PM, Rafał Miłecki wrote: > On 02/01/2017 10:26 PM, Jacek Anaszewski wrote: >> On 02/01/2017 04:56 PM, Rafał Miłecki wrote: >>> On 01/31/2017 10:34 PM, Jacek Anaszewski wrote: >>>> On 01/31/2017 05:11 PM, Rafał Miłecki wrote: >>>>> Thanks a lot Jacek for this explanation (and sorry but I needed a >>>>> bit of >>>>> time to >>>>> think about this). I can finally see your point, I think we're on the >>>>> same page >>>>> now. >>>>> >>>>> I think that for all this time I was thinking from the pure DT >>>>> perspective. If >>>>> you ignore fact that Linux has multiple LED trigger drivers, then >>>>> "led-triggers" >>>>> may not sound that bad. >>>>> >>>>> After reading your description however I can see this property can be >>>>> misleading >>>>> as Linux people may think "drivers" when seeing "triggers". >>>>> >>>>> I still think this is a useful property and I hope we can still find a >>>>> way to >>>>> name it in a sane way: to be nice from DT PoV and march Linux LEDs >>>>> subsystem. >>>> >>>> I also see the need for this property. >>>> >>>>> I think we should still work on some generic property (without linux, >>>>> prefix) as >>>>> this seems to be something generic, not really specific to Linux >>>>> implementation. >>>>> Specifying relation between LED and devices is something than AFAICS >>>>> can be >>>>> reused by other systems as well. >>>>> >>>>> So my suggestion is to try some new name & leave linux,default-trigger >>>>> to allow >>>>> specifying one single trigger driver as required by current Linux >>>>> implementation. >>>>> >>>>> What do you think about this? >>>>> >>>>> There are few suggestions to came to my mind: >>>>> "trigger-sources" >>>>> "trigger-devices" >>>>> "led-trigger-devices" >>>>> "led-related-devices" >>>>> "led-event-devices" >>>> >>>> trigger-devices would work in this specific use case, >>>> where we can give phandles to usb ports. Nonetheless, we >>>> have also other kernel based events serving as trigger >>>> sources - e.g. timer. >>>> >>>> In this case I'd go for trigger-sources, but we'd need >>>> to have some generic way of defining the source like timer. >>> >>> I'd leave timer for later discussion but I'm glad you brought this >>> problem. >>> Maybe we could discuss this on another example that is more supported >>> like >>> GPIOs? >>> >>> We know this property allows specifying USB ports and we want it to >>> support >>> mixing various sources. So a very simple sample case seems to be using >>> it for >>> specifying GPIO as trigger source. >>> >>> Is this possible to mix various entries in a list assigned to single >>> property? >>> Let's say: >>> trigger-sources = >>> <&ohci_port1>, >>> <&ehci_port1>, >>> <&gpio 1 GPIO_ACTIVE_HIGH>; >> >> According to my knowledge all elements in the list are elements >> of one array, no matter if they are comma separated or space separated >> in "<>" brackets. DT maintainer would have to confirm that though. > > This matches my knowledge as well. Having that, we would be limited to a list of fixed size tuples IMHO. > >>> Is this possible to support this? If so, which function I can use to >>> detect >>> what is pointed by a phandle? >>> I'm not that familiar with DT and I'm not sure how to handle this. >> >> I wonder if this is correct way to go. Maybe we should think of >> creating entirely new trigger mechanism that would allow for having >> multiple active triggers at a time. > > I'm OK with reworking Linux's triggers if it need be. I think we should > agree > on binding(s) first though. I was thinking of creating entirely new mechanism (new sysfs file), as we can't break existing users. > >> Of course trigger-sources would be still useful for defining >> a list of devices of the same type like in case of usbport trigger. >> Nonetheless, I have a problem with this property being a part of >> LED class device DT bindings. Triggers are not tightly coupled with >> LED class devices, but trigger-sources being a list of usb ports >> would be applicable only to usbport trigger. What if there was >> another combo trigger e.g. for reporting activity on mulitple GPIOs? > > That was exactly my point with above trigger-sources example including 2 > USB > ports & GPIO. How usbport trigger would make use use of information about GPIO? Does it mean that in this approach triggers would arbitrarily choose trigger-sources applicable for them? > >> Should we have separate list of trigger sources per any possible >> existing trigger type? Aren't we going to far from pure hardware >> description the Device Tree was initially predestined to? > > That would simplify things, but it gets us back to *multiple* properties > like > led-usb-triggers (or led-usb-trigger-sources) > led-pci-triggers (or led-pci-trigger-sources) > led-gpio-triggers (or led-gpio-trigger-sources) > etc. Right, I was rather skeptical in my question - I don't like this solution too. > Last time Rob said he's not going to accept something like this, see: > > On 23 January 2017 at 17:45, Rob Herring <robh@xxxxxxxxxx> wrote: >> 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. > > So I think the main question right now is if this is possible to support > mixed > entries in a list like that > trigger-sources = > <&ohci_port1>, > <&ehci_port1>, > <&gpio 1 GPIO_ACTIVE_HIGH>; > > I posted as example. > > I was also trying to find help on IRC #devicetree channel but didn't get > any > response: > [09:15] <rmilecki> hi, i've a question about mixing various entries in a > single list > [09:16] <rmilecki> is this possible to have different /syntax/ in every > list element > [09:16] <rmilecki> and have driver detect what does it reference? > [09:16] <rmilecki> i'll post an example > [09:17] <rmilecki> trigger-sources = <&ohci_port1>, <&ehci_port1>, > <&gpio 1 GPIO_ACTIVE_HIGH>; > [09:18] <rmilecki> this question is related to my work in [PATCH V2 1/2] > dt-bindings: leds: document new led-triggers property > [09:18] <rmilecki> beware, there is a long discussion in that thread > > Rob: do you still follow this thread? We'd like to use single property > as you > suggested, but is it possible to work with something like trigger-sources > example posted above? > -- Best regards, Jacek Anaszewski
