Re: [PATCH v3 1/4] Input: snvs_pwrkey - make use of dev_err_probe()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [Medium] Missing `.remove` callback causes a leak of `struct wake_irq` and leaves dangling wakeup configuration upon driver unbind.
--

commit 07567ee0ff85afeb83d7fcfa00735130f4ec77e0
Author: Joy Zou <joy.zou@nxp.com>
Input: snvs_pwrkey - make use of dev_err_probe()

This commit updates the imx_snvs_pwrkey_probe function to use dev_err_probe
for its error paths. This helps simplify error logging and properly handles
probe deferral conditions.

> diff --git a/drivers/input/keyboard/snvs_pwrkey.c b/drivers/input/keyboard/snvs_pwrkey.c
> --- a/drivers/input/keyboard/snvs_pwrkey.c
> +++ b/drivers/input/keyboard/snvs_pwrkey.c
[ ... ]
> @@ -209,16 +203,12 @@ static int imx_snvs_pwrkey_probe(struct platform_device *pdev)
>  	error = devm_request_irq(&pdev->dev, pdata->irq,
>  			       imx_snvs_pwrkey_interrupt,
>  			       0, pdev->name, pdev);
> -	if (error) {
> -		dev_err(&pdev->dev, "interrupt not available.\n");
> -		return error;
> -	}
> +	if (error)
> +		return dev_err_probe(&pdev->dev, error, "interrupt not available.\n");
>  
>  	error = input_register_device(input);
> -	if (error < 0) {
> -		dev_err(&pdev->dev, "failed to register input device\n");
> -		return error;
> -	}
> +	if (error < 0)
> +		return dev_err_probe(&pdev->dev, error, "failed to register input device\n");
>  
>  	device_init_wakeup(&pdev->dev, pdata->wakeup);
>  	error = dev_pm_set_wake_irq(&pdev->dev, pdata->irq);

[Severity: Medium]
This is a pre-existing issue, but I noticed there might be a resource cleanup
issue here in imx_snvs_pwrkey_probe().

Does this code leak the wakeup resources when the driver is unbound?

It appears that device_init_wakeup() and dev_pm_set_wake_irq() allocate
unmanaged resources and attach them to the device. Because there is no remove
callback or managed action to clean them up via dev_pm_clear_wake_irq() and
device_init_wakeup(dev, false), these resources might be leaked on unbind.

If the driver is later re-bound, could this also cause probe to fail since
dev_pm_attach_wake_irq() returns -EEXIST when the wakeirq is already
initialized?

Would it be appropriate to switch these to devm_device_init_wakeup() and
devm_pm_set_wake_irq() to handle the cleanup automatically?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260615-b4-pwrkey-v3-0-9510b1173f6e@oss.nxp.com?part=1