Re: [PATCH v6 4/9] irqchip/imx-irqsteer: Convert to devm_pm_runtime_set_active_enabled()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] irqchip/imx-irqsteer: NULL pointer dereference in suspend due to early PM enable
--

commit 5322ea0be538e254fa9c813752002c35702c23cf
Author: Zhipeng Wang <zhipeng.wang_1@nxp.com>

irqchip/imx-irqsteer: Convert to devm_pm_runtime_set_active_enabled()

imx_irqsteer_probe() enables runtime PM with pm_runtime_enable(), but
imx_irqsteer_remove() never disables it. Runtime PM therefore stays
enabled after the device is unbound, and rebinding it triggers an
unbalanced pm_runtime_enable warning. This commit uses
devm_pm_runtime_set_active_enabled() to automatically disable runtime PM
on unbind, fixing the imbalance.

> diff --git a/drivers/irqchip/irq-imx-irqsteer.c b/drivers/irqchip/irq-imx-irqsteer.c
> index d05de3d26fb28..5acc04504e520 100644
> --- a/drivers/irqchip/irq-imx-irqsteer.c
> +++ b/drivers/irqchip/irq-imx-irqsteer.c
> @@ -239,6 +239,10 @@ static int imx_irqsteer_probe(struct platform_device *pdev)
>  	if (irqsteer_has_chanctrl(data->devtype_data))
>  		writel_relaxed(BIT(data->channel), data->regs + CHANCTRL);
>  
> +	ret = devm_pm_runtime_set_active_enabled(&pdev->dev);
> +	if (ret)
> +		goto out;
> +

[Severity: High]
Does moving the PM runtime enablement before platform_set_drvdata() introduce
a race condition?

The PM runtime is now enabled early, and the IRQ domain is subsequently
created and exposed globally.

If a concurrent child device or userspace sysfs bind maps, requests, and
releases an IRQ from this domain before platform_set_drvdata() is reached,
releasing the IRQ invokes irq_bus_sync_unlock().

This in turn calls pm_runtime_put_autosuspend(), dropping the usage count to
zero and queuing a runtime suspend which invokes imx_irqsteer_suspend().

Since platform_set_drvdata() has not been called yet, will dev_get_drvdata()
return NULL in the suspend callback and cause a NULL pointer dereference when
passed to imx_irqsteer_save_regs()?

>  	data->domain = irq_domain_create_linear(dev_fwnode(&pdev->dev), data->reg_num * 32,
>  						&imx_irqsteer_domain_ops, data);

[ ... ]

> @@ -260,9 +264,6 @@ static int imx_irqsteer_probe(struct platform_device *pdev)
>  
>  	platform_set_drvdata(pdev, data);
>  
> -	pm_runtime_set_active(&pdev->dev);
> -	pm_runtime_enable(&pdev->dev);
> -
>  	return 0;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261008090231.796511-1-Zhipeng.wang_1@oss.nxp.com?part=4