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

[Kendall Willis]

On 09:51-20260914, Markus Schneider-Pargmann wrote:
> 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@xxxxxx>
> > ---
> >  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.
> 

Thanks for pointing this out, will fix in v2.

Best,
Kendall