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

[Frank Li <Frank.li@oss.nxp.com>]

On Thu, Oct 08, 2026 at 09:21:24AM +0000, sashiko-bot@kernel.org wrote:
> 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()?

Please fix this problem also

Frank


>
> >  	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