Re: [PATCH v3 3/4] Input: snvs_pwrkey - use local device pointer to simple code

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] The driver calls `dev_pm_set_wake_irq()` but does not use the `devm_` variant or provide a `.remove` function, leading to a resource leak and potentially broken system suspend on driver unbind.
--

commit d7d5ed812213c739d1459c472b4e230aac0a6e8d
Author: Joy Zou <joy.zou@nxp.com>

Input: snvs_pwrkey - use local device pointer to simple code

This commit introduces a local struct device pointer in the probe function
to reduce boilerplate code when referencing the platform device. It is
intended to be a non-functional change.

> 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
[ ... ]
> @@ -193,27 +194,27 @@ static int imx_snvs_pwrkey_probe(struct platform_device *pdev)
[ ... ]
> -	device_init_wakeup(&pdev->dev, pdata->wakeup);
> -	error = dev_pm_set_wake_irq(&pdev->dev, pdata->irq);
> +	device_init_wakeup(dev, pdata->wakeup);
> +	error = dev_pm_set_wake_irq(dev, pdata->irq);

[Severity: High]
This is a pre-existing issue, but does this leak the wake IRQ on driver 
unbind?

Looking at imx_snvs_pwrkey_probe() in drivers/input/keyboard/snvs_pwrkey.c,
the driver configures the wake IRQ using dev_pm_set_wake_irq() but it does
not use the devm_ variant, nor does it provide a .remove callback to call
dev_pm_clear_wake_irq().

When the driver unbinds, devres automatically frees the IRQ, but the memory
for the wake IRQ structure remains allocated and attached to
dev->power.wakeirq. 

Will a subsequent system suspend then attempt to call enable_irq_wake() on a
freed IRQ number? Also, if the driver is later re-bound, would it trigger a
warning in dev_pm_attach_wake_irq() because dev->power.wakeirq is already
initialized?

Should this be updated to use devm_pm_set_wake_irq() instead?

>  	if (error)
> -		dev_err(&pdev->dev, "irq wake enable failed.\n");
> +		dev_err(dev, "irq wake enable failed.\n");
>  
>  	return 0;
>  }

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