[PATCH v2] irqchip: imx-irqsteer: Convert to devm_pm_runtime_enable()

[PATCH v2] irqchip: imx-irqsteer: Convert to devm_pm_runtime_enable()

[Fabio Estevam]

imx_irqsteer_probe() enables runtime PM, but imx_irqsteer_remove()
does not disable it.

Consequently, runtime PM remains enabled after unbinding the device,
and rebinding it triggers:

Unbalanced pm_runtime_enable!

Use devm_pm_runtime_enable() to automatically disable runtime PM when
the device is removed.

Fixes: 4730d2233311 ("irqchip/imx-irqsteer: Add runtime PM support")
Signed-off-by: Fabio Estevam <festevam@gmail.com>
---
Changes since v1:
- Use devm_pm_runtime_enable() (Frank)

 drivers/irqchip/irq-imx-irqsteer.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/irqchip/irq-imx-irqsteer.c b/drivers/irqchip/irq-imx-irqsteer.c
index 87b07f517be3..ba1203d3edd4 100644
--- a/drivers/irqchip/irq-imx-irqsteer.c
+++ b/drivers/irqchip/irq-imx-irqsteer.c
@@ -263,7 +263,9 @@ 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);
+	ret = devm_pm_runtime_enable(&pdev->dev);
+	if (ret)
+		goto out;
 
 	return 0;
 out:
-- 
2.43.0

Re: [PATCH v2] irqchip: imx-irqsteer: Convert to devm_pm_runtime_enable()

[Radu Rendec]

On Wed, 2026-07-29 at 19:14 -0300, Fabio Estevam wrote:
> imx_irqsteer_probe() enables runtime PM, but imx_irqsteer_remove()
> does not disable it.
> 
> Consequently, runtime PM remains enabled after unbinding the device,
> and rebinding it triggers:
> 
> Unbalanced pm_runtime_enable!
> 
> Use devm_pm_runtime_enable() to automatically disable runtime PM when
> the device is removed.
> 
> Fixes: 4730d2233311 ("irqchip/imx-irqsteer: Add runtime PM support")
> Signed-off-by: Fabio Estevam <festevam@gmail.com>
> ---
> Changes since v1:
> - Use devm_pm_runtime_enable() (Frank)
> 
>  drivers/irqchip/irq-imx-irqsteer.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/irqchip/irq-imx-irqsteer.c b/drivers/irqchip/irq-imx-irqsteer.c
> index 87b07f517be3..ba1203d3edd4 100644
> --- a/drivers/irqchip/irq-imx-irqsteer.c
> +++ b/drivers/irqchip/irq-imx-irqsteer.c
> @@ -263,7 +263,9 @@ 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);
> +	ret = devm_pm_runtime_enable(&pdev->dev);
> +	if (ret)
> +		goto out;
>  
>  	return 0;
>  out:

Reviewed-by: Radu Rendec <radu@rendec.net>

Re: [PATCH v2] irqchip: imx-irqsteer: Convert to devm_pm_runtime_enable()

[Frank Li]

On Wed, Jul 29, 2026 at 07:14:16PM -0300, Fabio Estevam wrote:
> imx_irqsteer_probe() enables runtime PM, but imx_irqsteer_remove()
> does not disable it.
>
> Consequently, runtime PM remains enabled after unbinding the device,
> and rebinding it triggers:
>
> Unbalanced pm_runtime_enable!
>
> Use devm_pm_runtime_enable() to automatically disable runtime PM when
> the device is removed.
>
> Fixes: 4730d2233311 ("irqchip/imx-irqsteer: Add runtime PM support")
> Signed-off-by: Fabio Estevam <festevam@gmail.com>
> ---

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

> Changes since v1:
> - Use devm_pm_runtime_enable() (Frank)
>
>  drivers/irqchip/irq-imx-irqsteer.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/irqchip/irq-imx-irqsteer.c b/drivers/irqchip/irq-imx-irqsteer.c
> index 87b07f517be3..ba1203d3edd4 100644
> --- a/drivers/irqchip/irq-imx-irqsteer.c
> +++ b/drivers/irqchip/irq-imx-irqsteer.c
> @@ -263,7 +263,9 @@ 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);
> +	ret = devm_pm_runtime_enable(&pdev->dev);
> +	if (ret)
> +		goto out;
>
>  	return 0;
>  out:
> --
> 2.43.0
>
>

Re: [PATCH v2] irqchip: imx-irqsteer: Convert to devm_pm_runtime_enable()

[Radu Rendec]

On Wed, 2026-07-29 at 22:22 -0400, Radu Rendec wrote:
> On Wed, 2026-07-29 at 19:14 -0300, Fabio Estevam wrote:
> > imx_irqsteer_probe() enables runtime PM, but imx_irqsteer_remove()
> > does not disable it.
> > 
> > Consequently, runtime PM remains enabled after unbinding the device,
> > and rebinding it triggers:
> > 
> > Unbalanced pm_runtime_enable!
> > 
> > Use devm_pm_runtime_enable() to automatically disable runtime PM when
> > the device is removed.
> > 
> > Fixes: 4730d2233311 ("irqchip/imx-irqsteer: Add runtime PM support")
> > Signed-off-by: Fabio Estevam <festevam@gmail.com>
> > ---
> > Changes since v1:
> > - Use devm_pm_runtime_enable() (Frank)
> > 
> >  drivers/irqchip/irq-imx-irqsteer.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/irqchip/irq-imx-irqsteer.c b/drivers/irqchip/irq-imx-irqsteer.c
> > index 87b07f517be3..ba1203d3edd4 100644
> > --- a/drivers/irqchip/irq-imx-irqsteer.c
> > +++ b/drivers/irqchip/irq-imx-irqsteer.c
> > @@ -263,7 +263,9 @@ 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);
> > +	ret = devm_pm_runtime_enable(&pdev->dev);
> > +	if (ret)
> > +		goto out;
> >  
> >  	return 0;
> >  out:
> 
> Reviewed-by: Radu Rendec <radu@rendec.net>

I take that back. At a closer look, I think both issues identified by
Sashiko are valid. While the second issue is marked as "pre-existing",
the proposed patch introduces a new path where the issue can occur
(leaking the irq domain).

While at it, I would also fix the existing issue that the bot
identified, where the domain is leaked on this path:

        if (!data->irq_count || data->irq_count > CHAN_MAX_OUTPUT_INT) {
                ret = -EINVAL;
                goto out;
        }

Re: [PATCH v2] irqchip: imx-irqsteer: Convert to devm_pm_runtime_enable()

[Fabio Estevam]

On Sat, Aug 1, 2026 at 3:54 PM Radu Rendec <radu@rendec.net> wrote:

> I take that back. At a closer look, I think both issues identified by
> Sashiko are valid. While the second issue is marked as "pre-existing",
> the proposed patch introduces a new path where the issue can occur
> (leaking the irq domain).

Would the first version of the patch be acceptable?

Re: [PATCH v2] irqchip: imx-irqsteer: Convert to devm_pm_runtime_enable()

[Radu Rendec]

On Tue, 2026-08-04 at 08:09 -0300, Fabio Estevam wrote:
> On Sat, Aug 1, 2026 at 3:54 PM Radu Rendec <radu@rendec.net> wrote:
> 
> > I take that back. At a closer look, I think both issues identified by
> > Sashiko are valid. While the second issue is marked as "pre-existing",
> > the proposed patch introduces a new path where the issue can occur
> > (leaking the irq domain).
> 
> Would the first version of the patch be acceptable?

It certainly avoids the problem of the extra error path that
devm_pm_runtime_enable() introduces.

But Sashiko had a comment on the first version too, and I think at
least the first issue - about the double-disable of the clock (if the
device is suspended when it's removed) - is real.

In my (very limited) understanding of the runtime_pm framework, there
are two ways to handle this cleanly:
 * Manage the activation and deactivation of the clock indirectly
   through suspend/resume requests. The driver already enables the
   clock in its resume callback and disables the clock in its suspend
   callback. Instead of enabling/disabling the clock explicitly in the
   probe/remove function, use pm_runtime_get_sync() and
   pm_runtime_put(), which indirectly resume and respectively suspend
   the device. This is what drivers/irqchip/irq-renesas-intc-irqpin.c
   does (except it doesn't manage a clock).
 * Call pm_runtime_get_sync() at the beginning of the remove function
   (which requires a matching pm_runtime_put() in the probe function).
   That will indirectly resume the device first, then it's safe to
   disable the clock.

At a high level, the second option above would like this:

static int imx_irqsteer_probe(struct platform_device *pdev)
{
 /* Initial setup / clock enablement */

 pm_runtime_set_active(&pdev->dev);
 pm_runtime_enable(&pdev->dev);
 /* Mark device idle so it can autosuspend when unused */
 pm_runtime_put_autosuspend(&pdev->dev);

 return 0;
}

static void imx_irqsteer_remove(struct platform_device *pdev)
{
    pm_runtime_get_sync(&pdev->dev);
    pm_runtime_disable(&pdev->dev);

    /* Final clean up and disable clock */
}

That's closer to what the driver already does. But the part that I
don't like is that the pm_runtime_put_autosuspend() call may suspend
the device immediately, and I don't fully understand the implications
of that.