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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] input: gpio-keys: silent masking of pinctrl resource acquisition errors
--

commit f85782ff9c9eb4bfa803e47eea6427b48cb93f09
Author: Kendall Willis <k-willis@ti.com>

Input: gpio-keys - support wakeup pinctrl state on suspend

This commit adds support for selecting a wakeup pinctrl state during suspend
to RAM on TI K3 AM62 devices. It retrieves the pinctrl state using
devm_pinctrl_get() and looks up the wakeup state if the device is enabled.

> diff --git a/drivers/input/keyboard/gpio_keys.c b/drivers/input/keyboard/gpio_keys.c
> --- a/drivers/input/keyboard/gpio_keys.c
> +++ b/drivers/input/keyboard/gpio_keys.c

[ ... ]

> @@ -884,6 +887,15 @@ 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(ddata->pinctrl)) {
> +		error = PTR_ERR(ddata->pinctrl);
> +		if (error == -EPROBE_DEFER)
> +			return error;

[Severity: Medium]
Does this code intentionally swallow all error codes from devm_pinctrl_get()
other than -EPROBE_DEFER?

If devm_pinctrl_get() fails for other reasons, such as a memory allocation
failure (-ENOMEM) or an invalid device tree configuration (-EINVAL), the
error is silently ignored. This allows the driver to probe successfully but
with silently broken wakeup functionality. 

Should the error be returned for all failure cases to avoid masking resource
acquisition failures?

> +	} else if (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;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261001-upstream-gpio-wakeup-v3-0-6f3bcde367a3@ti.com?part=2