Re: [PATCH 1/2] dt-bindings: leds: Mention GPIO triggers

[Linus Walleij]

On Thu, Sep 14, 2023 at 4:27 PM Rob Herring <robh@xxxxxxxxxx> wrote:
> On Thu, Sep 14, 2023 at 3:40 AM Linus Walleij <linus.walleij@xxxxxxxxxx> wrote:
> > On Wed, Sep 13, 2023 at 3:34 PM Rob Herring <robh@xxxxxxxxxx> wrote:
> > > On Tue, Sep 12, 2023 at 03:44:30PM +0200, Linus Walleij wrote:
> > > > We reuse the trigger-sources phandle to just point to
> > > > GPIOs we may want to use as LED triggers.
> > > >
> > > > Example:
> > > >
> > > > gpio: gpio@0 {
> > > >     compatible "my-gpio";
> > > >     gpio-controller;
> > > >     #gpio-cells = <2>;
> > > >     interrupt-controller;
> > > >     #interrupt-cells = <2>;
> > > >     #trigger-source-cells = <2>;
> > >
> > > BTW, this is not documented for any GPIO binding. If we want to specify
> > > the cell size, then it has to be added to every GPIO controller binding.
> > > If not, we then need to reference gpio.yaml in every GPIO controller
> > > binding (along with unevaluatedProperties). Doesn't have to be done for
> > > this patch to go in though.
> >
> > Yeah I mean this trigger-sources = <...>; one-size-fits-all is a bit
> > weird in a way.
> >
> > My other idea was to simply add trigger-gpios to the normal way
> > and be done with it, but now the trigger binding has this weird
> > thing.
> >
> > Would trigger-gpios be better?
>
> Then GPIOs are different than everyone else. I think we have to think
> about other bindings too. While we could standardize the naming here
> with trigger-gpios, that won't work with the foos/foo-names style of
> bindings.
>
> trigger-sources is not widely used as it is just USB ATM and a few
> platforms. We could come up with something different.
> "trigger-sources-<cellname>" is the only idea I have. Then the
> property name gives you the cell name to read. But variable property
> names have their own challenges. We would need to look at all the
> current trigger sources (i.e. the linux,default-trigger ones) and see
> what else might need this.

I think it in a way is elegant with the trigger-sources phandle as it
is so I would stick with this.

I can just add '#trigger-source-cells' to the existing GPIO
bindings and it's a bit tedious since we don't have a common file
for the GPIO chip stuff, but it's just lots of lines.

I guess it would be better to break out gpio-common.yaml and
gpio-common-irq.yaml for GPIO controllers with or without
interrupt support and then add '#trigger-source-cells' to just
those supporting IRQs because I think only that makes sense,
polling for a line to change isn't quite a "trigger".

Yours,
Linus Walleij