[PATCH] Input: gpio-keys - add hibernation support

[PATCH] Input: gpio-keys - add hibernation support

[Armando De Leon]

The gpio-keys driver uses DEFINE_SIMPLE_DEV_PM_OPS which maps .freeze
and .restore to the same callbacks as .suspend and .resume. This is
insufficient for hibernation (suspend-to-disk) because the SoC is fully
powered off, unlike suspend-to-RAM where hardware state is preserved.

After hibernation resume, GPIO keys fail to function correctly because
the interrupt controller (e.g. GIC) is fully re-initialized during
restore, resetting all IRQ trigger type configurations to defaults.
GPIO keys require IRQ_TYPE_EDGE_BOTH for proper press/release detection,
but after the interrupt controller re-initialization only a single edge
remains active. This causes buttons to report only release events but
not press events, or vice versa.

The existing gpio_keys_button_disable_wakeup() does restore
IRQ_TYPE_EDGE_BOTH, but only when wakeup_trigger_type is non-zero.
When the device tree does not specify wakeup-event-action (the common
case), wakeup_trigger_type defaults to zero and the IRQ type
restoration is skipped entirely.

Fix this by implementing dedicated .freeze and .restore callbacks:

- gpio_keys_freeze(): Marks all buttons as suspended before the
  hibernate image is created. Unlike .suspend, it does not configure
  wakeup IRQs since the SoC will be powered off.

- gpio_keys_restore(): Re-applies the pinctrl default state to restore
  GPIO pin configuration, explicitly reconfigures IRQ trigger type to
  IRQ_TYPE_EDGE_BOTH for all GPIO-backed buttons, clears the suspended
  flag, and re-syncs current GPIO state with the input subsystem.

The .thaw callback reuses gpio_keys_resume() since the SoC remains
powered when hibernation is aborted and hardware state is intact.

Signed-off-by: Armando De Leon <learmand@amazon.com>
---
 drivers/input/keyboard/gpio_keys.c | 60 +++++++++++++++++++++++++++++-
 1 file changed, 59 insertions(+), 1 deletion(-)

diff --git a/drivers/input/keyboard/gpio_keys.c b/drivers/input/keyboard/gpio_keys.c
index e19617485..9fb77c9aa 100644
--- a/drivers/input/keyboard/gpio_keys.c
+++ b/drivers/input/keyboard/gpio_keys.c
@@ -27,6 +27,7 @@
 #include <linux/gpio/consumer.h>
 #include <linux/of.h>
 #include <linux/of_irq.h>
+#include <linux/pinctrl/consumer.h>
 #include <linux/spinlock.h>
 #include <dt-bindings/input/gpio-keys.h>
 
@@ -1088,7 +1089,64 @@ static int gpio_keys_resume(struct device *dev)
 	return 0;
 }
 
-static DEFINE_SIMPLE_DEV_PM_OPS(gpio_keys_pm_ops, gpio_keys_suspend, gpio_keys_resume);
+static int gpio_keys_freeze(struct device *dev)
+{
+	struct gpio_keys_drvdata *ddata = dev_get_drvdata(dev);
+	struct gpio_button_data *bdata;
+	int i;
+
+	for (i = 0; i < ddata->pdata->nbuttons; i++) {
+		bdata = &ddata->data[i];
+		bdata->suspended = true;
+	}
+
+	return 0;
+}
+
+static int gpio_keys_restore(struct device *dev)
+{
+	struct gpio_keys_drvdata *ddata = dev_get_drvdata(dev);
+	struct gpio_button_data *bdata;
+	int error;
+	int i;
+
+	error = pinctrl_pm_select_default_state(dev);
+	if (error)
+		dev_warn(dev, "failed to restore pinctrl default state: %d\n",
+			 error);
+
+	for (i = 0; i < ddata->pdata->nbuttons; i++) {
+		bdata = &ddata->data[i];
+		bdata->suspended = false;
+
+		/*
+		 * After hibernation the interrupt controller is
+		 * re-initialized and loses its configuration.
+		 * Restore dual-edge triggering for GPIO-backed
+		 * buttons so both press and release are detected.
+		 */
+		if (bdata->gpiod) {
+			error = irq_set_irq_type(bdata->irq,
+						 IRQ_TYPE_EDGE_BOTH);
+			if (error)
+				dev_warn(dev,
+					 "failed to restore IRQ %d trigger: %d\n",
+					 bdata->irq, error);
+		}
+	}
+
+	gpio_keys_report_state(ddata);
+	return 0;
+}
+
+static const struct dev_pm_ops gpio_keys_pm_ops = {
+	.suspend	= gpio_keys_suspend,
+	.resume		= gpio_keys_resume,
+	.freeze		= gpio_keys_freeze,
+	.restore	= gpio_keys_restore,
+	.thaw		= gpio_keys_resume,
+	.poweroff	= gpio_keys_suspend,
+};
 
 static void gpio_keys_shutdown(struct platform_device *pdev)
 {
-- 
2.43.0

Re: [PATCH] Input: gpio-keys - add hibernation support

[Dmitry Torokhov]

Hi Armando,

On Mon, Apr 06, 2026 at 09:04:37AM -0700, Armando De Leon wrote:
> The gpio-keys driver uses DEFINE_SIMPLE_DEV_PM_OPS which maps .freeze
> and .restore to the same callbacks as .suspend and .resume. This is
> insufficient for hibernation (suspend-to-disk) because the SoC is fully
> powered off, unlike suspend-to-RAM where hardware state is preserved.
> 
> After hibernation resume, GPIO keys fail to function correctly because
> the interrupt controller (e.g. GIC) is fully re-initialized during
> restore, resetting all IRQ trigger type configurations to defaults.
> GPIO keys require IRQ_TYPE_EDGE_BOTH for proper press/release detection,
> but after the interrupt controller re-initialization only a single edge
> remains active. This causes buttons to report only release events but
> not press events, or vice versa.

I believe you just described a bug in the interrupt controller handling
of hibernation. It needs to be fixed there, not in consumer driver.

Thanks.

-- 
Dmitry

Re: [PATCH] Input: gpio-keys - add hibernation support

[Armando De Leon]

Hi Dmitry,

Thank you for the review.

The interrupt controller (GICv3) is re-initialized by platform
firmware during hibernate restore - this is expected behavior, not
a bug. The IRQ trigger type (EDGE_BOTH) was originally configured
by devm_request_any_context_irq() during probe(), which does not
run again after hibernate restore.

The TLMM/pinctrl registers are correctly saved and restored by the
platform's syscore_ops - I verified this with register dumps. The
issue is specifically that the IRQ trigger type configured at the
GIC level during probe is lost and not re-applied.

Should the generic IRQ core be responsible for restoring trigger
types across hibernate? Otherwise, consumer drivers like gpio-keys need to handle
this in their .restore callback.

Either way, gpio-keys currently lacks .freeze/.restore callbacks
entirely, which is needed for proper hibernation support.

Thanks,
Armando

Re: [PATCH] Input: gpio-keys - add hibernation support

[Dmitry Torokhov]

On Mon, Apr 06, 2026 at 09:58:06AM -0700, Armando De Leon wrote:
> Hi Dmitry,
> 
> Thank you for the review.
> 
> The interrupt controller (GICv3) is re-initialized by platform
> firmware during hibernate restore - this is expected behavior, not
> a bug. The IRQ trigger type (EDGE_BOTH) was originally configured
> by devm_request_any_context_irq() during probe(), which does not
> run again after hibernate restore.
> 
> The TLMM/pinctrl registers are correctly saved and restored by the
> platform's syscore_ops - I verified this with register dumps. The
> issue is specifically that the IRQ trigger type configured at the
> GIC level during probe is lost and not re-applied.
> 
> Should the generic IRQ core be responsible for restoring trigger
> types across hibernate? Otherwise, consumer drivers like gpio-keys need to handle
> this in their .restore callback.

I am not sure if it is job of IRQ core vs. particular interrupt
controller, but they should restore the trigger types along with
entirety of the interrupts and pins states to exactly the same condition
that they were before entering hibernation.

> 
> Either way, gpio-keys currently lacks .freeze/.restore callbacks
> entirely, which is needed for proper hibernation support.

Drivers only need dedicated freeze and restore handlers if the behavior
should be different between suspend-to-ram vs suspend-to-disk. For the
vast majority of the drivers they behave exactly the same and I do not
see why it would be different for gpio-keys.

Thanks.

-- 
Dmitry

Re: [PATCH] Input: gpio-keys - add hibernation support

[Armando De Leon]

Hi Dmitry,

Thanks for the guidance. I investigated further:

The upstream GICv3 driver (irq-gic-v3.c) has no suspend/resume or
syscore_ops for the distributor - it does not save or restore any
per-IRQ configuration across power cycles. On platforms where the
GIC is re-initialized by firmware during hibernate restore, all
trigger type configurations set by consumer drivers during probe()
are lost.

The IRQ core does preserve the trigger type in irq_data
(irqd_get_trigger_type), but resume_irq() in kernel/irq/pm.c only
re-enables the interrupt without re-applying the trigger type to
hardware.

A fix in the IRQ PM resume path (having resume_irq() call
irq_set_irq_type() using the saved trigger type) would fix all
consumer drivers generically. However, such a change would have
broad consequences and impact across all platforms and interrupt
controllers, and would need very careful review and testing.

Given that the gpio-keys fix is a single irq_set_irq_type() call
in the .restore callback - minimal, self-contained, and low risk -
would it be acceptable to handle it at the driver level for now?
This avoids the risk of a sweeping IRQ core change while solving
the immediate problem for gpio-keys users with hibernation support.

Thanks,
Armando

Re: [PATCH] Input: gpio-keys - add hibernation support

[Dmitry Torokhov]

Hi Armando,

On Mon, Apr 06, 2026 at 01:39:10PM -0700, Armando De Leon wrote:
> Hi Dmitry,
> 
> Thanks for the guidance. I investigated further:
> 
> The upstream GICv3 driver (irq-gic-v3.c) has no suspend/resume or
> syscore_ops for the distributor - it does not save or restore any
> per-IRQ configuration across power cycles. On platforms where the
> GIC is re-initialized by firmware during hibernate restore, all
> trigger type configurations set by consumer drivers during probe()
> are lost.
> 
> The IRQ core does preserve the trigger type in irq_data
> (irqd_get_trigger_type), but resume_irq() in kernel/irq/pm.c only
> re-enables the interrupt without re-applying the trigger type to
> hardware.
> 
> A fix in the IRQ PM resume path (having resume_irq() call
> irq_set_irq_type() using the saved trigger type) would fix all
> consumer drivers generically. However, such a change would have
> broad consequences and impact across all platforms and interrupt
> controllers, and would need very careful review and testing.
> 
> Given that the gpio-keys fix is a single irq_set_irq_type() call
> in the .restore callback - minimal, self-contained, and low risk -
> would it be acceptable to handle it at the driver level for now?
> This avoids the risk of a sweeping IRQ core change while solving
> the immediate problem for gpio-keys users with hibernation support.

I am sorry but as I mentioned, the proper fix is in IRQ/irqchip code,
which will ensure that not only gpio-keys but also the rest of the
consumer drivers have consistent state post hibernate.

I understand that you might need your change for a product; I recommend
applying the patch to your internal tree while you are sorting out a
generic solution.

Thanks.

-- 
Dmitry