[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20230814052558.GN11676@atomide.com>
Date: Mon, 14 Aug 2023 08:25:58 +0300
From: Tony Lindgren <tony@...mide.com>
To: Rob Herring <robh@...nel.org>
Cc: Dmitry Torokhov <dmitry.torokhov@...il.com>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
Conor Dooley <conor+dt@...nel.org>,
linux-input@...r.kernel.org, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH 1/2] dt-bindings: input: gpio-keys: Allow optional
dedicated wakeirq
* Rob Herring <robh@...nel.org> [230811 15:10]:
> On Fri, Aug 11, 2023 at 02:04:31PM +0300, Tony Lindgren wrote:
> > Allow configuring optional dedicated wakeirq that some SoCs have.
> > Let's use the interrupt naming "irq" and "wakeup" that we already have
> > in use for some drivers and subsystems like i2c.
> >
> > Signed-off-by: Tony Lindgren <tony@...mide.com>
> > ---
> > .../devicetree/bindings/input/gpio-keys.yaml | 15 ++++++++++++++-
> > 1 file changed, 14 insertions(+), 1 deletion(-)
> >
> > diff --git a/Documentation/devicetree/bindings/input/gpio-keys.yaml b/Documentation/devicetree/bindings/input/gpio-keys.yaml
> > --- a/Documentation/devicetree/bindings/input/gpio-keys.yaml
> > +++ b/Documentation/devicetree/bindings/input/gpio-keys.yaml
> > @@ -31,7 +31,17 @@ patternProperties:
> > maxItems: 1
> >
> > interrupts:
> > - maxItems: 1
> > + description:
> > + Optional interrupts if different from the gpio interrupt
> > + maxItems: 2
> > +
> > + interrupt-names:
> > + description:
> > + Optional interrupt names, can be used to specify a separate
> > + dedicated wake-up interrupt
> > + items:
> > + -const: irq
> > + -const: wakeup
>
> Also need a space after '-'.
Oops sorry about that, obviously I did not run make dtbs_check on this
binding. I guess I just grepped so we don't have interrupt-names in use
right now.
> >
> > label:
> > description: Descriptive name of the key.
> > @@ -130,6 +140,9 @@ examples:
> > label = "GPIO Key UP";
> > linux,code = <103>;
> > gpios = <&gpio1 0 1>;
> > + interrupts-extended = <&intc_wakeup 0 IRQ_TYPE_LEVEL_HIGH>;
> > + interrupt-names = "wakeup";
>
> That's not what your schema allows. You need:
>
> minItems: 1
> items:
> - enum: [ irq, wakeup ]
> - const: wakeup
>
> (repeating 'wakeup' is disallowed globally for ".*-names".)
OK
> > + wakeup-source;
>
> Of course with this, a single interrupt is the wake-up source and
> doesn't need a name. So you could define that 'interrupt-names' is only
> used when there are 2 interrupts. In that case, the schema is right and
> the example is wrong.
OK. So here's what gpio-keys currently allows:
1. gpios property with no interrupt in the dts, the driver tries to
find the interrupt based on the gpio
2. gpios property with one interrupts property and no interrupt-names
And here's what we could allow in the binding with the wakeirq support
added:
1. gpios property with no interrupt in the dts, the driver tries to
find the interrupt based on the gpio
2. gpios property with one interrupts property and no interrupt-names
3. gpios property with one interrupts property and interrupt-names = "irq"
4. gpios property with one wakeirq and interrupt-names = "wakeirq", the
driver tries to find the io interrupt based on the gpio
5. gpios property with two interrupts and interrupt-names =
"irq", "wakeirq"
So yeah we could only allow interrupt-names if there are two interrupts
like the attempted binding has. This would leave out #3 and #4 options
above. No need to limit these options from driver point of view though.
Any preferences on what the binding should have?
Regards,
Tony
Powered by blists - more mailing lists