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

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

On Sat, Oct 10, 2026 at 08:26:44AM +0000, Zhipeng Wang (OSS) wrote:
> > 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@oss.nxp.com
> > wrote:
> > > From: Zhipeng Wang <zhipeng.wang_1@nxp.com>
> > >
> > > 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@nabladev.com>
> > > Signed-off-by: Zhipeng Wang <zhipeng.wang_1@nxp.com>
> > > ---
> > >  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.

Okay

Reviewed-by: Frank Li <Frank.Li@nxp.com>

>
> Thanks,
> Zhipeng
>
> > >  }
> > >
> > >  #ifdef CONFIG_PM
> > > --
> > > 2.34.1
> > >
> > >