diff mbox series

[v4,1/2] dt-bindings: input: gpio-keys: Allow optional dedicated wakeirq

Message ID 20231124083241.40780-1-tony@atomide.com
State Not Applicable
Headers show
Series [v4,1/2] dt-bindings: input: gpio-keys: Allow optional dedicated wakeirq | expand

Checks

Context Check Description
robh/checkpatch success
robh/patch-applied success
robh/dtbs-check warning build log
robh/dt-meta-schema success

Commit Message

Tony Lindgren Nov. 24, 2023, 8:32 a.m. UTC 
Allow configuring an optional dedicated wakeirq for gpio-keys that
some SoCs have.

Let's use the common interrupt naming "irq" and "wakeup" that we already
have in use for some drivers and subsystems like i2c framework.

Note that the gpio-keys interrupt property is optional. If only a gpio
property is specified, the driver tries to translate the gpio into an
interrupt.

Reviewed-by: Rob Herring <robh@kernel.org>
Signed-off-by: Tony Lindgren <tony@atomide.com>
---

No changes since v3

Changes since v2:
- Fix indentation as noted by Rob's bot

- Add Reviewed-by from Rob

Changes since v1:

- Run make dt_binding_check on the binding

- Add better checks for interrupt-names as suggested by Rob, it is
  now required if two interrupts are configured

- Add more decription entries

- Add a new example for key-wakeup

 .../devicetree/bindings/input/gpio-keys.yaml  | 41 ++++++++++++++++++-
 1 file changed, 40 insertions(+), 1 deletion(-)

Comments

Dmitry Torokhov Nov. 25, 2023, 4:50 a.m. UTC | #1 
Hi Tony,

On Fri, Nov 24, 2023 at 10:32:41AM +0200, Tony Lindgren wrote:
> +	/*
> +	 * Wakeirq shares the handler with the main interrupt, it's only
> +	 * active during system suspend. See gpio_keys_button_enable_wakeup()
> +	 * and gpio_keys_button_disable_wakeup().
> +	 */
> +	error = devm_request_any_context_irq(dev, bdata->wakeirq, isr,
> +					     irqflags, wakedesc, bdata);
> +	if (error < 0) {
> +		dev_err(dev, "Unable to claim wakeirq %d; error %d\n",
> +			bdata->irq, error);
> +		return error;
> +	}
> +
> +	/*
> +	 * Disable wakeirq until suspend. IRQF_NO_AUTOEN won't work if
> +	 * IRQF_SHARED was set based on !button->can_disable.
> +	 */
> +	disable_irq_nosync(bdata->wakeirq);

Why _nosync() here and below? Is there any harm in sing the normal
variant?

Thanks.
Tony Lindgren Nov. 29, 2023, 8:38 a.m. UTC | #2 
* Dmitry Torokhov <dmitry.torokhov@gmail.com> [231125 04:50]:
> Hi Tony,
> 
> On Fri, Nov 24, 2023 at 10:32:41AM +0200, Tony Lindgren wrote:
> > +	/*
> > +	 * Wakeirq shares the handler with the main interrupt, it's only
> > +	 * active during system suspend. See gpio_keys_button_enable_wakeup()
> > +	 * and gpio_keys_button_disable_wakeup().
> > +	 */
> > +	error = devm_request_any_context_irq(dev, bdata->wakeirq, isr,
> > +					     irqflags, wakedesc, bdata);
> > +	if (error < 0) {
> > +		dev_err(dev, "Unable to claim wakeirq %d; error %d\n",
> > +			bdata->irq, error);
> > +		return error;
> > +	}
> > +
> > +	/*
> > +	 * Disable wakeirq until suspend. IRQF_NO_AUTOEN won't work if
> > +	 * IRQF_SHARED was set based on !button->can_disable.
> > +	 */
> > +	disable_irq_nosync(bdata->wakeirq);
> 
> Why _nosync() here and below? Is there any harm in sing the normal
> variant?

Well they are enabled the same time anyways for a while, so I see no
harm using the normal variant here. Will post updated patches after
some testing.

Regards,

Tony
diff mbox series

Patch

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,23 @@  patternProperties:
         maxItems: 1
 
       interrupts:
-        maxItems: 1
+        oneOf:
+          - items:
+              - description: Optional key interrupt or wakeup interrupt
+          - items:
+              - description: Key interrupt
+              - description: Wakeup interrupt
+
+      interrupt-names:
+        description:
+          Optional interrupt names, can be used to specify a separate dedicated
+          wake-up interrupt in addition to the gpio irq
+        oneOf:
+          - items:
+              - enum: [ irq, wakeup ]
+          - items:
+              - const: irq
+              - const: wakeup
 
       label:
         description: Descriptive name of the key.
@@ -97,6 +113,20 @@  patternProperties:
       - required:
           - gpios
 
+    allOf:
+      - if:
+          properties:
+            interrupts:
+              minItems: 2
+          required:
+            - interrupts
+        then:
+          properties:
+            interrupt-names:
+              minItems: 2
+          required:
+            - interrupt-names
+
     dependencies:
       wakeup-event-action: [ wakeup-source ]
       linux,input-value: [ gpios ]
@@ -137,6 +167,15 @@  examples:
             linux,code = <108>;
             interrupts = <1 IRQ_TYPE_EDGE_FALLING>;
         };
+
+        key-wakeup {
+            label = "GPIO Key WAKEUP";
+            linux,code = <143>;
+            interrupts-extended = <&intc 2 IRQ_TYPE_EDGE_FALLING>,
+                                  <&intc_wakeup 0 IRQ_TYPE_LEVEL_HIGH>;
+            interrupt-names = "irq", "wakeup";
+            wakeup-source;
+        };
     };
 
 ...