Re: [PATCH 2/2] Input: gpio-keys - support wakeup pinctrl state on suspend

["Markus Schneider-Pargmann" <msp@baylibre.com>]

[-- Attachment #1: Type: text/plain, Size: 2777 bytes --]

Hi Kendall,

On Sat Sep 12, 2026 at 11:33 PM CEST, Kendall Willis wrote:
> GPIO can be used as a wakeup source for TI K3 AM62 devices during
> suspend to RAM states. The GPIO controller is powered off in these
> states, so a wakeup enable flag must be set on the GPIO pin in the
> pinctrl in order to allow wakeup.
>
> If the device is wakeup enabled, select the 'wakeup' pinctrl state on
> suspend and restore the default pinctrl state on resume.
>
> Signed-off-by: Kendall Willis <k-willis@ti.com>
> ---
>  drivers/input/keyboard/gpio_keys.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
>
> diff --git a/drivers/input/keyboard/gpio_keys.c b/drivers/input/keyboard/gpio_keys.c
> index 129fc4212a5ed126496b331ce626682d441c53bf..b409e179705195e5fec172ff62088395109cd758 100644
> --- a/drivers/input/keyboard/gpio_keys.c
> +++ b/drivers/input/keyboard/gpio_keys.c
> @@ -28,6 +28,7 @@
>  #include <linux/of.h>
>  #include <linux/of_irq.h>
>  #include <linux/spinlock.h>
> +#include <linux/pinctrl/consumer.h>
>  #include <dt-bindings/input/gpio-keys.h>
>  
>  struct gpio_button_data {
> @@ -60,6 +61,8 @@ struct gpio_keys_drvdata {
>  	struct input_dev *input;
>  	struct mutex disable_lock;
>  	unsigned short *keymap;
> +	struct pinctrl *pinctrl;
> +	struct pinctrl_state *pinctrl_wakeup;
>  	struct gpio_button_data data[];
>  };
>  
> @@ -884,6 +887,10 @@ static int gpio_keys_probe(struct platform_device *pdev)
>  	platform_set_drvdata(pdev, ddata);
>  	input_set_drvdata(input, ddata);
>  
> +	ddata->pinctrl = devm_pinctrl_get(dev);
> +	if (!IS_ERR_OR_NULL(ddata->pinctrl))
> +		ddata->pinctrl_wakeup = pinctrl_lookup_state(ddata->pinctrl, "wakeup");
> +
>  	input->name = pdata->name ? : pdev->name;
>  	input->phys = "gpio-keys/input0";
>  	input->dev.parent = dev;
> @@ -1010,6 +1017,9 @@ gpio_keys_enable_wakeup(struct gpio_keys_drvdata *ddata)
>  	int error;
>  	int i;
>  
> +	if (!IS_ERR_OR_NULL(ddata->pinctrl_wakeup))
> +		pinctrl_select_state(ddata->pinctrl, ddata->pinctrl_wakeup);

If wakeup state selection failed, should it really just continue here?

> +
>  	for (i = 0; i < ddata->pdata->nbuttons; i++) {
>  		bdata = &ddata->data[i];
>  		if (bdata->button->wakeup) {

There is some error handling below in this function. The selected
pinctrl state is not rolled back in the error path.

Best
Markus

> @@ -1039,6 +1049,9 @@ gpio_keys_disable_wakeup(struct gpio_keys_drvdata *ddata)
>  	struct gpio_button_data *bdata;
>  	int i;
>  
> +	if (!IS_ERR_OR_NULL(ddata->pinctrl_wakeup))
> +		pinctrl_pm_select_default_state(ddata->input->dev.parent);
> +
>  	for (i = 0; i < ddata->pdata->nbuttons; i++) {
>  		bdata = &ddata->data[i];
>  		bdata->suspended = false;



[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 289 bytes --]