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

[Zhipeng Wang (OSS)]

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

NXP Confidential
> On Thu, Oct 08, 2026 at 06:02:26PM +0900, Zhipeng.wang_1@xxxxxxxxxxx
> wrote:
> > From: Zhipeng Wang <zhipeng.wang_1@xxxxxxx>
> >
> > 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:
> >
> >   Unbalanced pm_runtime_enable!
> >
> > Use devm_pm_runtime_set_active_enabled() so the driver core marks the
> > device active (matching the enabled clock) and disables runtime PM
> > again on unbind, keeping the enable balanced across unbind/rebind.
> >
> > This becomes reachable once the driver can be unbound and reloaded,
> > but the imbalance already exists today on unbind, so fix it separately
> > with a Fixes tag for stable.
> >
> > Fixes: 4730d2233311 ("irqchip/imx-irqsteer: Add runtime PM support")
> > Suggested-by: Fabio Estevam <festevam@xxxxxxxxxxxx>
> > Signed-off-by: Zhipeng Wang <zhipeng.wang_1@xxxxxxx>
> > ---
> >  drivers/irqchip/irq-imx-irqsteer.c | 10 ++++++----
> >  1 file changed, 6 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/irqchip/irq-imx-irqsteer.c
> > b/drivers/irqchip/irq-imx-irqsteer.c
> > index d05de3d26fb2..5acc04504e52 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;
> > +
> >     data->domain = irq_domain_create_linear(dev_fwnode(&pdev->dev),
> data->reg_num * 32,
> >                                             &imx_irqsteer_domain_ops, data);
> >     if (!data->domain) {
> > @@ -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;
> >  out:
> >     clk_disable_unprepare(data->ipg_clk);
> > @@ -285,7 +286,8 @@ static void imx_irqsteer_remove(struct
> > platform_device *pdev)
> >
> >     irq_domain_remove(irqsteer_data->domain);
> >
> > -   clk_disable_unprepare(irqsteer_data->ipg_clk);
> > +   if (!pm_runtime_status_suspended(&pdev->dev))
> > +           clk_disable_unprepare(irqsteer_data->ipg_clk);
>
> Are you sure need do this? suppose runtime_pm tear down call runtime pm
> susped to disable clock.
>
> Frank
>
Hi Frank,

I don't think the devres teardown disables the clock here.

devm_pm_runtime_set_active_enabled() registers two devres actions that
run on unbind: pm_runtime_disable() and pm_runtime_set_suspended().
pm_runtime_set_suspended() only sets the status to RPM_SUSPENDED via
__pm_runtime_set_status(); it does not invoke the ->runtime_suspend
callback. So imx_irqsteer_suspend() is not called at teardown and the
clock is not disabled by runtime PM.

At this patch the clock is still managed by hand (devm_clk_get() plus
clk_prepare_enable() in probe()), so remove() has to clk_disable_unprepare()
to balance it. The pm_runtime_status_suspended() guard is there for the
case where the device is already runtime-suspended at unbind: the runtime
suspend callback has then already dropped the clock, so remove() must not
disable it a second time, otherwise the enable count underflows.

The later patch "Let devres own the clock" switches to
devm_clk_get_enabled(), and at that point this manual clk_disable_unprepare()
(and the guard) is removed, replaced by a pm_runtime_resume_and_get() /
pm_runtime_put_noidle() pair.

Thanks,
Zhipeng

> >  }
> >
> >  #ifdef CONFIG_PM
> > --
> > 2.34.1
> >
> >