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

[Kendall Willis <k-willis@ti.com>]

On 14:11-20260930, Markus Schneider-Pargmann wrote:
> On Fri Sep 25, 2026 at 11:52 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 | 19 +++++++++++++++++++
> >  1 file changed, 19 insertions(+)
> >
> > diff --git a/drivers/input/keyboard/gpio_keys.c b/drivers/input/keyboard/gpio_keys.c
> > index 129fc4212a5ed126496b331ce626682d441c53bf..d3fbc9776ac8cbdc7349a28df990ee609edf9e59 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))
> 
> Should this check for -EPROBE_DEFER and error out in that case?
> 
> Otherwise looks good to me.
> 
> Best
> Markus

Yep, I can propagate the error from devm_pinctrl_get() if it is
-EPROBE_DEFER. Will fix in the next version.

Thanks,
Kendall

> 
> > +		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,12 @@ gpio_keys_enable_wakeup(struct gpio_keys_drvdata *ddata)
> >  	int error;
> >  	int i;
> >  
> > +	if (!IS_ERR_OR_NULL(ddata->pinctrl_wakeup)) {
> > +		error = pinctrl_select_state(ddata->pinctrl, ddata->pinctrl_wakeup);
> > +		if (error)
> > +			return error;
> > +	}
> > +
> >  	for (i = 0; i < ddata->pdata->nbuttons; i++) {
> >  		bdata = &ddata->data[i];
> >  		if (bdata->button->wakeup) {
> > @@ -1023,6 +1036,9 @@ gpio_keys_enable_wakeup(struct gpio_keys_drvdata *ddata)
> >  	return 0;
> >  
> >  err_out:
> > +	if (!IS_ERR_OR_NULL(ddata->pinctrl_wakeup))
> > +		pinctrl_pm_select_default_state(ddata->input->dev.parent);
> > +
> >  	while (i--) {
> >  		bdata = &ddata->data[i];
> >  		if (bdata->button->wakeup)
> > @@ -1039,6 +1055,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;
> 
>