[PATCH v3 1/2] irqchip/imx-irqsteer: Convert to devm_pm_runtime_enable()

[PATCH v3 1/2] irqchip/imx-irqsteer: Convert to devm_pm_runtime_enable()

[Fabio Estevam]

From: Fabio Estevam <festevam@nabladev.com>

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. Set up runtime PM before creating the IRQ domain
and registering chained handlers so that a failure cannot leave either
resource pointing at freed driver data.

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

 drivers/irqchip/irq-imx-irqsteer.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/irqchip/irq-imx-irqsteer.c b/drivers/irqchip/irq-imx-irqsteer.c
index 87b07f517be3..653e25115083 100644
--- a/drivers/irqchip/irq-imx-irqsteer.c
+++ b/drivers/irqchip/irq-imx-irqsteer.c
@@ -236,6 +236,11 @@ static int imx_irqsteer_probe(struct platform_device *pdev)
 	if (irqsteer_has_chanctrl(data->devtype_data))
 		writel_relaxed(BIT(data->channel), data->regs + CHANCTRL);
 
+	pm_runtime_set_active(&pdev->dev);
+	ret = devm_pm_runtime_enable(&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) {
@@ -262,9 +267,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);
-- 
2.43.0

[PATCH v3 2/2] irqchip/imx-irqsteer: Validate IRQ count before creating domain

[Fabio Estevam]

From: Fabio Estevam <festevam@nabladev.com>

The IRQ count is validated after creating the IRQ domain. If it is
invalid, probe returns without removing the domain, leaving its host
data pointing at devm-managed memory that is freed on probe failure.

Validate the count before allocating resources to avoid the leak and
dangling pointer.

Fixes: 28528fca4908 ("irqchip/imx-irqsteer: Add multi output interrupts support")
Signed-off-by: Fabio Estevam <festevam@nabladev.com>
---
Changes since v2:
- Newly introduced.

 drivers/irqchip/irq-imx-irqsteer.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/irqchip/irq-imx-irqsteer.c b/drivers/irqchip/irq-imx-irqsteer.c
index 653e25115083..55aec60dee40 100644
--- a/drivers/irqchip/irq-imx-irqsteer.c
+++ b/drivers/irqchip/irq-imx-irqsteer.c
@@ -217,6 +217,8 @@ static int imx_irqsteer_probe(struct platform_device *pdev)
 	 */
 	data->irq_count = DIV_ROUND_UP(irqs_num, 64);
 	data->reg_num = irqs_num / 32;
+	if (!data->irq_count || data->irq_count > CHAN_MAX_OUTPUT_INT)
+		return -EINVAL;
 
 	if (IS_ENABLED(CONFIG_PM)) {
 		data->saved_reg = devm_kzalloc(&pdev->dev,
@@ -250,11 +252,6 @@ static int imx_irqsteer_probe(struct platform_device *pdev)
 	}
 	irq_domain_set_pm_device(data->domain, &pdev->dev);
 
-	if (!data->irq_count || data->irq_count > CHAN_MAX_OUTPUT_INT) {
-		ret = -EINVAL;
-		goto out;
-	}
-
 	for (i = 0; i < data->irq_count; i++) {
 		data->irq[i] = irq_of_parse_and_map(np, i);
 		if (!data->irq[i])
-- 
2.43.0

Re: [PATCH v3 1/2] irqchip/imx-irqsteer: Convert to devm_pm_runtime_enable()

[Frank Li]

On Wed, Aug 05, 2026 at 04:27:42PM -0300, Fabio Estevam wrote:
> From: Fabio Estevam <festevam@nabladev.com>
>
> 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. Set up runtime PM before creating the IRQ domain
> and registering chained handlers so that a failure cannot leave either
> resource pointing at freed driver data.
>
> Fixes: 4730d2233311 ("irqchip/imx-irqsteer: Add runtime PM support")
> Signed-off-by: Fabio Estevam <festevam@nabladev.com>
> ---
> Changes since v1:
> - Move devm_pm_runtime_enable() prior to irq_domain_create_linear(). (Frank)

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

>
>  drivers/irqchip/irq-imx-irqsteer.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/irqchip/irq-imx-irqsteer.c b/drivers/irqchip/irq-imx-irqsteer.c
> index 87b07f517be3..653e25115083 100644
> --- a/drivers/irqchip/irq-imx-irqsteer.c
> +++ b/drivers/irqchip/irq-imx-irqsteer.c
> @@ -236,6 +236,11 @@ static int imx_irqsteer_probe(struct platform_device *pdev)
>  	if (irqsteer_has_chanctrl(data->devtype_data))
>  		writel_relaxed(BIT(data->channel), data->regs + CHANCTRL);
>
> +	pm_runtime_set_active(&pdev->dev);
> +	ret = devm_pm_runtime_enable(&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) {
> @@ -262,9 +267,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);
> --
> 2.43.0
>
>

Re: [PATCH v3 2/2] irqchip/imx-irqsteer: Validate IRQ count before creating domain

[Frank Li]

On Wed, Aug 05, 2026 at 04:27:43PM -0300, Fabio Estevam wrote:
> From: Fabio Estevam <festevam@nabladev.com>
>
> The IRQ count is validated after creating the IRQ domain. If it is
> invalid, probe returns without removing the domain, leaving its host
> data pointing at devm-managed memory that is freed on probe failure.
>
> Validate the count before allocating resources to avoid the leak and
> dangling pointer.
>
> Fixes: 28528fca4908 ("irqchip/imx-irqsteer: Add multi output interrupts support")
> Signed-off-by: Fabio Estevam <festevam@nabladev.com>
> ---

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

> Changes since v2:
> - Newly introduced.
>
>  drivers/irqchip/irq-imx-irqsteer.c | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/irqchip/irq-imx-irqsteer.c b/drivers/irqchip/irq-imx-irqsteer.c
> index 653e25115083..55aec60dee40 100644
> --- a/drivers/irqchip/irq-imx-irqsteer.c
> +++ b/drivers/irqchip/irq-imx-irqsteer.c
> @@ -217,6 +217,8 @@ static int imx_irqsteer_probe(struct platform_device *pdev)
>  	 */
>  	data->irq_count = DIV_ROUND_UP(irqs_num, 64);
>  	data->reg_num = irqs_num / 32;
> +	if (!data->irq_count || data->irq_count > CHAN_MAX_OUTPUT_INT)
> +		return -EINVAL;
>
>  	if (IS_ENABLED(CONFIG_PM)) {
>  		data->saved_reg = devm_kzalloc(&pdev->dev,
> @@ -250,11 +252,6 @@ static int imx_irqsteer_probe(struct platform_device *pdev)
>  	}
>  	irq_domain_set_pm_device(data->domain, &pdev->dev);
>
> -	if (!data->irq_count || data->irq_count > CHAN_MAX_OUTPUT_INT) {
> -		ret = -EINVAL;
> -		goto out;
> -	}
> -
>  	for (i = 0; i < data->irq_count; i++) {
>  		data->irq[i] = irq_of_parse_and_map(np, i);
>  		if (!data->irq[i])
> --
> 2.43.0
>
>

Re: [PATCH v3 1/2] irqchip/imx-irqsteer: Convert to devm_pm_runtime_enable()

[Radu Rendec]

On Wed, 2026-08-05 at 16:27 -0300, Fabio Estevam wrote:
> From: Fabio Estevam <festevam@nabladev.com>
> 
> 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. Set up runtime PM before creating the IRQ domain
> and registering chained handlers so that a failure cannot leave either
> resource pointing at freed driver data.
> 
> Fixes: 4730d2233311 ("irqchip/imx-irqsteer: Add runtime PM support")
> Signed-off-by: Fabio Estevam <festevam@nabladev.com>
> ---
> Changes since v1:
> - Move devm_pm_runtime_enable() prior to irq_domain_create_linear(). (Frank)
> 
>  drivers/irqchip/irq-imx-irqsteer.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/irqchip/irq-imx-irqsteer.c b/drivers/irqchip/irq-imx-irqsteer.c
> index 87b07f517be3..653e25115083 100644
> --- a/drivers/irqchip/irq-imx-irqsteer.c
> +++ b/drivers/irqchip/irq-imx-irqsteer.c
> @@ -236,6 +236,11 @@ static int imx_irqsteer_probe(struct platform_device *pdev)
>  	if (irqsteer_has_chanctrl(data->devtype_data))
>  		writel_relaxed(BIT(data->channel), data->regs + CHANCTRL);
>  
> +	pm_runtime_set_active(&pdev->dev);
> +	ret = devm_pm_runtime_enable(&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) {
> @@ -262,9 +267,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);

I still believe there is something off with the way pm_runtime is
handled, and that the double clock disable is possible.

Since (like I said) I have very limited understanding of the runtime_pm
framework, I decided to make a little experiment.

With the dummy module below, I see this:
[  548.253591] pm_dummy pm_dummy: pm_dummy_probe() executed
[  548.254209] pm_dummy pm_dummy: clock enabled; refcount: 1
[  548.255028] pm_dummy pm_dummy: pm_dummy_runtime_suspend() triggered
[  548.255845] pm_dummy pm_dummy: clock disabled; refcount: 0
[  554.261820] pm_dummy pm_dummy: pm_dummy_runtime_resume() triggered
[  554.263100] pm_dummy pm_dummy: clock enabled; refcount: 1
[  554.264006] pm_dummy pm_dummy: pm_dummy_runtime_suspend() triggered
[  554.264997] pm_dummy pm_dummy: clock disabled; refcount: 0
[  554.265819] pm_dummy pm_dummy: pm_dummy_remove() executed
[  554.266609] pm_dummy pm_dummy: clock disabled; refcount: -1
[  554.267468] pm_dummy pm_dummy: **************************************************
[  554.268554] pm_dummy pm_dummy: [BUG DETECTED] Clock disable count underflow! (-1)
[  554.269479] pm_dummy pm_dummy: **************************************************

What I find interesting is that the device is suspended immediately
during probe(), then it's automatically resumed and immediately
suspended again right before remove(). The latter is probably a side
effect of devm_pm_runtime_enable(). But in any case, the clock *is*
disabled twice, and that's even without any explicit suspend or resume,
it's just by loading and unloading the module.

#include <linux/module.h>
#include <linux/kernel.h>
#include <linux/init.h>
#include <linux/platform_device.h>
#include <linux/pm_runtime.h>

MODULE_LICENSE("GPL");
MODULE_AUTHOR("Radu Rendec <radu@rendec.net>");
MODULE_DESCRIPTION("runtime_pm playground");

static int mock_clk_count = 0;

static int mock_clk_prepare_enable(struct device *dev)
{
	mock_clk_count++;
	dev_info(dev, "clock enabled; refcount: %d\n", mock_clk_count);
	return 0;
}

static void mock_clk_disable_unprepare(struct device *dev)
{
	mock_clk_count--;
	dev_info(dev, "clock disabled; refcount: %d\n", mock_clk_count);

	if (mock_clk_count < 0) {
		dev_err(dev, "**************************************************\n");
		dev_err(dev, "[BUG DETECTED] Clock disable count underflow! (%d)\n", mock_clk_count);
		dev_err(dev, "**************************************************\n");
	}
}

static int pm_dummy_runtime_suspend(struct device *dev)
{
	dev_info(dev, "%s() triggered\n", __func__);
	mock_clk_disable_unprepare(dev);
	return 0;
}

static int pm_dummy_runtime_resume(struct device *dev)
{
	dev_info(dev, "%s() triggered\n", __func__);
	return mock_clk_prepare_enable(dev);
}

static const struct dev_pm_ops pm_dummy_pm_ops = {
	SET_RUNTIME_PM_OPS(pm_dummy_runtime_suspend, pm_dummy_runtime_resume, NULL)
};

static int pm_dummy_probe(struct platform_device *pdev)
{
	dev_info(&pdev->dev, "%s() executed\n", __func__);

	mock_clk_prepare_enable(&pdev->dev);
	pm_runtime_set_active(&pdev->dev);
	devm_pm_runtime_enable(&pdev->dev);

	return 0;
}

static void pm_dummy_remove(struct platform_device *pdev)
{
	dev_info(&pdev->dev, "%s() executed\n", __func__);

	mock_clk_disable_unprepare(&pdev->dev);
}

static struct platform_driver pm_dummy = {
	.probe = pm_dummy_probe,
	.remove = pm_dummy_remove,
	.driver = {
		.name = "pm_dummy",
		.pm = &pm_dummy_pm_ops,
	},
};

static struct platform_device *pdev;

static int __init pm_demo_init(void)
{
	int ret;

	ret = platform_driver_register(&pm_dummy);
	if (ret)
		return ret;

	pdev = platform_device_register_simple("pm_dummy", -1, NULL, 0);
	if (IS_ERR(pdev)) {
		platform_driver_unregister(&pm_dummy);
		return PTR_ERR(pdev);
	}

	return 0;
}

static void __exit pm_demo_exit(void)
{
	platform_device_unregister(pdev);
	platform_driver_unregister(&pm_dummy);
}

module_init(pm_demo_init);
module_exit(pm_demo_exit);