Re: [PATCH v4 07/10] gpio: mxc: switch probe error paths to dev_err_probe()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] pm: double put of runtime PM usage count on driver unbind
--

commit 30dda5fb34eeb7a2c552816b7e65724d4491a05c
Author: Peng Fan <peng.fan@nxp.com>

gpio: mxc: switch probe error paths to dev_err_probe()

Replace bare return statements with dev_err_probe() across all probe
error paths for consistent diagnostics and deferred-probe support.

Signed-off-by: Peng Fan <peng.fan@nxp.com>

> diff --git a/drivers/gpio/gpio-mxc.c b/drivers/gpio/gpio-mxc.c
> index 54b09f1a4b509..f5f9ef1564cdf 100644
> --- a/drivers/gpio/gpio-mxc.c
> +++ b/drivers/gpio/gpio-mxc.c

[ ... ]

> @@ -550,14 +550,15 @@ static int mxc_gpio_probe(struct platform_device *pdev)
>  
>  	port->domain = devm_irq_domain_instantiate(&pdev->dev, &d_info);
>  	if (IS_ERR(port->domain))
> -		return PTR_ERR(port->domain);
> +		return dev_err_probe(&pdev->dev, PTR_ERR(port->domain),
> +				     "Failed to create irq domain\n");
>  
>  	irq_domain_set_pm_device(port->domain, &pdev->dev);
>  
>  	/* gpio-mxc can be a generic irq chip */
>  	err = mxc_gpio_init_gc(port, irq_base);
>  	if (err < 0)
> -		return err;
> +		return dev_err_probe(&pdev->dev, err, "Failed to init generic irq chip\n");
>  
>  	list_add_tail(&port->node, &mxc_gpio_ports);
>  

[Severity: Medium]
Since this series replaces pm_runtime_get_noresume() with
devm_pm_runtime_get_noresume(), does this leave an unbalanced devres action
on driver unbind?

Earlier in mxc_gpio_probe(), the devres-managed get is called:

	err = devm_pm_runtime_get_noresume(&pdev->dev);

But at the end of the probe function on success, the count is still manually
dropped:

	pm_runtime_put_autosuspend(&pdev->dev);

If the device is ever unbound (e.g., via DT overlay removal or parent device
unregistration), won't devres execute pm_runtime_put_noidle_action and drop
the reference a second time?

While pm_runtime_put_noidle() uses atomic_add_unless() to prevent underflow
if the count is exactly 0, could it erroneously steal a reference if the
count is > 0 (such as from an open sysfs file or an active child device)?

When the legitimate holder later drops their reference, wouldn't the count
then underflow and trigger a warning, breaking the device's runtime PM state?

Even if suppress_bind_attrs is used, does that only prevent manual sysfs
unbinds, rather than programmatic device removal?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261007-gpio-mxc-cleanup-v4-0-c9f5a8b50df9@nxp.com?part=7