[PATCH v3 0/4] irqchip/imx-irqsteer: Allow building as module

[PATCH v3 0/4] irqchip/imx-irqsteer: Allow building as module

[Zhipeng.wang_1]

From: Zhipeng Wang <zhipeng.wang_1@nxp.com>

This series makes the i.MX IRQSTEER driver buildable as a module.

v2 was a single patch that folded the module conversion together with
the unload-path fixes. Following review, v3 splits it into four logical
changes: three fixes for pre-existing bugs that only become reachable
once the driver can be unbound/reloaded, followed by the module
conversion itself.

The three fixes address the module-related issues raised on v2:

 - Sashiko AI reported an irq_domain leak on the probe() error path that
   turns into a use-after-free once the module can be unloaded. Patch
   1/4 fixes the error path.
 - The IRQ mappings created in probe() were leaked on unload, and the
   child irq_descs kept pointing at the driver's irq_chip past
   irq_domain_remove(). Patch 2/4 disposes of them in remove().
 - Sashiko AI reported an interrupt storm on module reload because the
   CHANMASK registers retain their previous state. Patch 3/4 masks all
   interrupts in probe() and remove().

Patch 4/4 then converts the driver to a module. Per Frank Li's review,
it lets devres own the clock and runtime PM
(devm_clk_get_enabled() + devm_pm_runtime_set_active_enabled()) instead
of hand-balancing them in remove(), which also drops the manual
pm_runtime_get_sync()/clk_disable_unprepare() dance from the v2 remove().

The two remaining pre-existing handler issues Sashiko AI flagged (the
missing chained_irq_exit() on the handler error path, and register
access while runtime-suspended) are unrelated to module enablement and
are out of scope for this series; the chained_irq_exit() fix is sent
separately.

Changes in v3:
 - Split the single v2 patch into four patches.
 - Add the probe() error-path fix as patch 1/4 (Sashiko AI).
 - Add CHANMASK masking in probe()/remove() as patch 3/4 (Sashiko AI).
 - In the module conversion, let devres own the clock and runtime PM via
   devm_clk_get_enabled() and devm_pm_runtime_set_active_enabled(),
   dropping the manual runtime-PM/clock balancing from the v2 remove()
   (Frank Li).

v2: https://lore.kernel.org/r/20260728092219.525449-1-Zhipeng.wang_1@oss.nxp.com
v1: https://lore.kernel.org/r/20260724090136.3595894-1-Zhipeng.wang_1@oss.nxp.com

Jindong Yue (1):
  irqchip/imx-irqsteer: Allow building as module

Zhipeng Wang (3):
  irqchip/imx-irqsteer: Fix error handling path in probe()
  irqchip/imx-irqsteer: Dispose of IRQ mappings in remove()
  irqchip/imx-irqsteer: Mask all interrupts in probe() and remove()

 drivers/irqchip/Kconfig            |  2 +-
 drivers/irqchip/irq-imx-irqsteer.c | 60 ++++++++++++++++++++----------
 2 files changed, 41 insertions(+), 21 deletions(-)


base-commit: c0a27675eaf08255017b3cabc28c99c0cd71f468
-- 
2.34.1

[PATCH v3 1/4] irqchip/imx-irqsteer: Fix error handling path in probe()

[Zhipeng.wang_1]

From: Zhipeng Wang <zhipeng.wang_1@nxp.com>

If the fsl,num-irqs sanity check rejects the value after the IRQ domain
has already been created, probe() jumps to a single label that only calls
clk_disable_unprepare(), leaving the freshly created IRQ domain leaked.
The domain-creation failure path shares the same label, which is correct
only because the domain is NULL there.

Split the error path so that a failure after the domain has been created
removes it before disabling the clock, and a failure before that goes
straight to the clock cleanup.

Fixes: 28528fca4908 ("irqchip/imx-irqsteer: Add multi output interrupts support")
Signed-off-by: Zhipeng Wang <zhipeng.wang_1@nxp.com>
---
Changes in v3:
 - New patch, split out of the single v2 patch. Fixes the irq_domain
   leak on the probe() error path reported by Sashiko AI on v2.

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

diff --git a/drivers/irqchip/irq-imx-irqsteer.c b/drivers/irqchip/irq-imx-irqsteer.c
index 87b07f517be3..a2f0629b22a3 100644
--- a/drivers/irqchip/irq-imx-irqsteer.c
+++ b/drivers/irqchip/irq-imx-irqsteer.c
@@ -241,13 +241,13 @@ static int imx_irqsteer_probe(struct platform_device *pdev)
 	if (!data->domain) {
 		dev_err(&pdev->dev, "failed to create IRQ domain\n");
 		ret = -ENOMEM;
-		goto out;
+		goto err_clk;
 	}
 	irq_domain_set_pm_device(data->domain, &pdev->dev);
 
 	if (!data->irq_count || data->irq_count > CHAN_MAX_OUTPUT_INT) {
 		ret = -EINVAL;
-		goto out;
+		goto err_domain;
 	}
 
 	for (i = 0; i < data->irq_count; i++) {
@@ -266,7 +266,10 @@ static int imx_irqsteer_probe(struct platform_device *pdev)
 	pm_runtime_enable(&pdev->dev);
 
 	return 0;
-out:
+
+err_domain:
+	irq_domain_remove(data->domain);
+err_clk:
 	clk_disable_unprepare(data->ipg_clk);
 	return ret;
 }
-- 
2.34.1

[PATCH v3 2/4] irqchip/imx-irqsteer: Dispose of IRQ mappings in remove()

[Zhipeng.wang_1]

From: Zhipeng Wang <zhipeng.wang_1@nxp.com>

remove() tears down the chained handlers and the IRQ domain but never
disposes of the IRQ mappings it created. The parent mappings from
irq_of_parse_and_map() and the child mappings handed out by the domain
are leaked, and the child irq_descs are left pointing at the driver's
irq_chip past irq_domain_remove().

Dispose of the parent mappings alongside the chained handler teardown,
and dispose of the child mappings before removing the domain.

Fixes: 0136afa08967 ("irqchip: Add driver for imx-irqsteer controller")
Signed-off-by: Zhipeng Wang <zhipeng.wang_1@nxp.com>
---
Changes in v3:
 - Split out of the single v2 patch. In v2 this was folded into the
   module-conversion patch; no functional change.

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

diff --git a/drivers/irqchip/irq-imx-irqsteer.c b/drivers/irqchip/irq-imx-irqsteer.c
index a2f0629b22a3..4a2fe8ba97f5 100644
--- a/drivers/irqchip/irq-imx-irqsteer.c
+++ b/drivers/irqchip/irq-imx-irqsteer.c
@@ -277,7 +277,7 @@ static int imx_irqsteer_probe(struct platform_device *pdev)
 static void imx_irqsteer_remove(struct platform_device *pdev)
 {
 	struct irqsteer_data *irqsteer_data = platform_get_drvdata(pdev);
-	int i;
+	int hwirq, i;
 
 	for (i = 0; i < irqsteer_data->irq_count; i++) {
 		if (!irqsteer_data->irq[i])
@@ -285,8 +285,13 @@ static void imx_irqsteer_remove(struct platform_device *pdev)
 
 		irq_set_chained_handler_and_data(irqsteer_data->irq[i],
 						 NULL, NULL);
+		irq_dispose_mapping(irqsteer_data->irq[i]);
 	}
 
+	for (hwirq = 0; hwirq < irqsteer_data->reg_num * 32; hwirq++)
+		irq_dispose_mapping(irq_find_mapping(irqsteer_data->domain,
+						     hwirq));
+
 	irq_domain_remove(irqsteer_data->domain);
 
 	clk_disable_unprepare(irqsteer_data->ipg_clk);
-- 
2.34.1

[PATCH v3 3/4] irqchip/imx-irqsteer: Mask all interrupts in probe() and remove()

[Zhipeng.wang_1]

From: Zhipeng Wang <zhipeng.wang_1@nxp.com>

probe() sets up the chained handlers without first masking the input
interrupts, and remove() leaves the CHANMASK registers untouched. For a
built-in driver this happened to be harmless because CHANMASK resets to
all-masked, but once the driver can be unloaded and reloaded a child
interrupt left unmasked at unload time survives in hardware. On the next
probe() the parent interrupts are re-mapped and unmasked before the new
domain is ready, so a still-asserted line immediately storms the parent
with no handler to service it.

Mask all interrupts in probe() before wiring up the chained handlers, and
again in remove() so the hardware is left quiesced for the next probe().
Note CHANMASK uses inverted polarity (a set bit enables the interrupt), so
masking means writing zero. This mirrors the sibling NXP chained mux
irq-imx-intmux.c, which has masked all sources in both probe() and remove()
since commit 2fbb13961e74 ("irqchip: Add NXP INTMUX interrupt multiplexer
support").

Signed-off-by: Zhipeng Wang <zhipeng.wang_1@nxp.com>
---
Changes in v3:
 - New patch. Masks all CHANMASK interrupts in probe() and remove() to
   prevent the interrupt storm on module reload reported by Sashiko AI
   on v2.

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

diff --git a/drivers/irqchip/irq-imx-irqsteer.c b/drivers/irqchip/irq-imx-irqsteer.c
index 4a2fe8ba97f5..0c9c99f1141a 100644
--- a/drivers/irqchip/irq-imx-irqsteer.c
+++ b/drivers/irqchip/irq-imx-irqsteer.c
@@ -236,6 +236,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);
 
+	/* mask all interrupts before setting up the chained handlers */
+	for (i = 0; i < data->reg_num; i++)
+		writel_relaxed(0, data->regs + CHANMASK(i, data->reg_num));
+
 	data->domain = irq_domain_create_linear(dev_fwnode(&pdev->dev), data->reg_num * 32,
 						&imx_irqsteer_domain_ops, data);
 	if (!data->domain) {
@@ -279,6 +283,11 @@ static void imx_irqsteer_remove(struct platform_device *pdev)
 	struct irqsteer_data *irqsteer_data = platform_get_drvdata(pdev);
 	int hwirq, i;
 
+	/* mask all interrupts so a stale line cannot storm on the next probe */
+	for (i = 0; i < irqsteer_data->reg_num; i++)
+		writel_relaxed(0, irqsteer_data->regs +
+			       CHANMASK(i, irqsteer_data->reg_num));
+
 	for (i = 0; i < irqsteer_data->irq_count; i++) {
 		if (!irqsteer_data->irq[i])
 			break;
-- 
2.34.1

[PATCH v3 4/4] irqchip/imx-irqsteer: Allow building as module

[Zhipeng.wang_1]

From: Jindong Yue <jindong.yue@nxp.com>

Make the driver buildable as a module by turning the Kconfig symbol into
a tristate and using module_platform_driver() instead of
builtin_platform_driver().

Now that the driver can be unloaded and reloaded, let the driver core
own the clock and runtime PM lifetime so that remove() does not have to
hand-balance them:

 - acquire the clock with devm_clk_get_enabled() instead of a bare
   devm_clk_get() followed by a manual clk_prepare_enable(), so it is
   prepared/enabled for the device lifetime and released on unbind;
 - keep only clk_enable()/clk_disable() in the runtime PM callbacks,
   since prepare/unprepare is now handled once by devres;
 - enable runtime PM with devm_pm_runtime_set_active_enabled(), which
   marks the device active (matching the enabled clock) and disables
   runtime PM on unbind.

With the clock and runtime PM owned by devres, the probe() error path
and remove() only need to tear down the IRQ mappings and the domain.

Signed-off-by: Jindong Yue <jindong.yue@nxp.com>
Signed-off-by: Zhipeng Wang <zhipeng.wang_1@nxp.com>
---
Changes in v3:
 - Let devres own the clock and runtime PM: acquire the clock with
   devm_clk_get_enabled() and enable runtime PM with
   devm_pm_runtime_set_active_enabled(), dropping the manual
   pm_runtime_get_sync()/pm_runtime_disable()/clk_disable_unprepare()
   balancing that v2 open-coded in remove() (Frank Li).
 - The IRQ mapping disposal and the CHANMASK masking that v2 folded into
   this patch are now separate patches (2/4 and 3/4).

 drivers/irqchip/Kconfig            |  2 +-
 drivers/irqchip/irq-imx-irqsteer.c | 39 ++++++++++++++++--------------
 2 files changed, 22 insertions(+), 19 deletions(-)

diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
index 42f2278a702d..07db3b678f94 100644
--- a/drivers/irqchip/Kconfig
+++ b/drivers/irqchip/Kconfig
@@ -560,7 +560,7 @@ config CSKY_APB_INTC
 	  the controller's register.
 
 config IMX_IRQSTEER
-	bool "i.MX IRQSTEER support"
+	tristate "i.MX IRQSTEER support"
 	depends on ARCH_MXC || ARCH_S32 || COMPILE_TEST
 	default y if ARCH_MXC || ARCH_S32
 	select IRQ_DOMAIN
diff --git a/drivers/irqchip/irq-imx-irqsteer.c b/drivers/irqchip/irq-imx-irqsteer.c
index 0c9c99f1141a..e62862617b8d 100644
--- a/drivers/irqchip/irq-imx-irqsteer.c
+++ b/drivers/irqchip/irq-imx-irqsteer.c
@@ -10,6 +10,7 @@
 #include <linux/irqchip/chained_irq.h>
 #include <linux/irqdomain.h>
 #include <linux/kernel.h>
+#include <linux/module.h>
 #include <linux/of.h>
 #include <linux/of_irq.h>
 #include <linux/platform_device.h>
@@ -193,7 +194,7 @@ static int imx_irqsteer_probe(struct platform_device *pdev)
 		return PTR_ERR(data->regs);
 	}
 
-	data->ipg_clk = devm_clk_get(&pdev->dev, "ipg");
+	data->ipg_clk = devm_clk_get_enabled(&pdev->dev, "ipg");
 	if (IS_ERR(data->ipg_clk))
 		return dev_err_probe(&pdev->dev, PTR_ERR(data->ipg_clk),
 				     "failed to get ipg clk\n");
@@ -226,12 +227,6 @@ static int imx_irqsteer_probe(struct platform_device *pdev)
 			return -ENOMEM;
 	}
 
-	ret = clk_prepare_enable(data->ipg_clk);
-	if (ret) {
-		dev_err(&pdev->dev, "failed to enable ipg clk: %d\n", ret);
-		return ret;
-	}
-
 	/* steer all IRQs into configured channel */
 	if (irqsteer_has_chanctrl(data->devtype_data))
 		writel_relaxed(BIT(data->channel), data->regs + CHANCTRL);
@@ -244,8 +239,7 @@ static int imx_irqsteer_probe(struct platform_device *pdev)
 						&imx_irqsteer_domain_ops, data);
 	if (!data->domain) {
 		dev_err(&pdev->dev, "failed to create IRQ domain\n");
-		ret = -ENOMEM;
-		goto err_clk;
+		return -ENOMEM;
 	}
 	irq_domain_set_pm_device(data->domain, &pdev->dev);
 
@@ -266,15 +260,22 @@ 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_set_active_enabled(&pdev->dev);
+	if (ret)
+		goto err_irq;
 
 	return 0;
 
+err_irq:
+	for (i = 0; i < data->irq_count; i++) {
+		if (!data->irq[i])
+			break;
+
+		irq_set_chained_handler_and_data(data->irq[i], NULL, NULL);
+		irq_dispose_mapping(data->irq[i]);
+	}
 err_domain:
 	irq_domain_remove(data->domain);
-err_clk:
-	clk_disable_unprepare(data->ipg_clk);
 	return ret;
 }
 
@@ -302,8 +303,6 @@ static void imx_irqsteer_remove(struct platform_device *pdev)
 						     hwirq));
 
 	irq_domain_remove(irqsteer_data->domain);
-
-	clk_disable_unprepare(irqsteer_data->ipg_clk);
 }
 
 #ifdef CONFIG_PM
@@ -333,7 +332,7 @@ static int imx_irqsteer_suspend(struct device *dev)
 	struct irqsteer_data *irqsteer_data = dev_get_drvdata(dev);
 
 	imx_irqsteer_save_regs(irqsteer_data);
-	clk_disable_unprepare(irqsteer_data->ipg_clk);
+	clk_disable(irqsteer_data->ipg_clk);
 
 	return 0;
 }
@@ -343,7 +342,7 @@ static int imx_irqsteer_resume(struct device *dev)
 	struct irqsteer_data *irqsteer_data = dev_get_drvdata(dev);
 	int ret;
 
-	ret = clk_prepare_enable(irqsteer_data->ipg_clk);
+	ret = clk_enable(irqsteer_data->ipg_clk);
 	if (ret) {
 		dev_err(dev, "failed to enable ipg clk: %d\n", ret);
 		return ret;
@@ -366,6 +365,7 @@ static const struct of_device_id imx_irqsteer_dt_ids[] = {
 	{ .compatible = "nxp,s32n79-irqsteer",	.data = &s32n79_data },
 	{},
 };
+MODULE_DEVICE_TABLE(of, imx_irqsteer_dt_ids);
 
 static struct platform_driver imx_irqsteer_driver = {
 	.driver = {
@@ -376,4 +376,7 @@ static struct platform_driver imx_irqsteer_driver = {
 	.probe		= imx_irqsteer_probe,
 	.remove		= imx_irqsteer_remove,
 };
-builtin_platform_driver(imx_irqsteer_driver);
+module_platform_driver(imx_irqsteer_driver);
+
+MODULE_DESCRIPTION("i.MX IRQSTEER interrupt multiplexer/remapper driver");
+MODULE_LICENSE("GPL");
-- 
2.34.1

Re: [PATCH v3 1/4] irqchip/imx-irqsteer: Fix error handling path in probe()

[Frank Li]

On Fri, Aug 07, 2026 at 04:23:43PM +0900, Zhipeng.wang_1@oss.nxp.com wrote:
> From: Zhipeng Wang <zhipeng.wang_1@nxp.com>
>
> If the fsl,num-irqs sanity check rejects the value after the IRQ domain
> has already been created, probe() jumps to a single label that only calls
> clk_disable_unprepare(), leaving the freshly created IRQ domain leaked.
> The domain-creation failure path shares the same label, which is correct
> only because the domain is NULL there.
>
> Split the error path so that a failure after the domain has been created
> removes it before disabling the clock, and a failure before that goes
> straight to the clock cleanup.
>
> Fixes: 28528fca4908 ("irqchip/imx-irqsteer: Add multi output interrupts support")
> Signed-off-by: Zhipeng Wang <zhipeng.wang_1@nxp.com>
> ---
> Changes in v3:
>  - New patch, split out of the single v2 patch. Fixes the irq_domain
>    leak on the probe() error path reported by Sashiko AI on v2.
>
>  drivers/irqchip/irq-imx-irqsteer.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)

I suggest create helper devm_irq_domain_create_leaner()

static inline struct irq_domain *
devm_irq_domain_create_linear(struct fwnode_handle *fwnode,
							  unsigned int size,
							  const struct irq_domain_ops *ops,
							  void *host_data)
{
	const struct irq_domain_info info = {
		.fwnode		= fwnode,
		.size		= size,
		.hwirq_max	= size,
		.ops		= ops,
		.host_data	= host_data,
	};
	struct irq_domain *d = devm_irq_domain_instantiate(&info);

	return IS_ERR(d) ? NULL : d;
}

Then imx-irqsteer this devm version. So other drivers can get beneafit also

Frank


>
> diff --git a/drivers/irqchip/irq-imx-irqsteer.c b/drivers/irqchip/irq-imx-irqsteer.c
> index 87b07f517be3..a2f0629b22a3 100644
> --- a/drivers/irqchip/irq-imx-irqsteer.c
> +++ b/drivers/irqchip/irq-imx-irqsteer.c
> @@ -241,13 +241,13 @@ static int imx_irqsteer_probe(struct platform_device *pdev)
>  	if (!data->domain) {
>  		dev_err(&pdev->dev, "failed to create IRQ domain\n");
>  		ret = -ENOMEM;
> -		goto out;
> +		goto err_clk;
>  	}
>  	irq_domain_set_pm_device(data->domain, &pdev->dev);
>
>  	if (!data->irq_count || data->irq_count > CHAN_MAX_OUTPUT_INT) {
>  		ret = -EINVAL;
> -		goto out;
> +		goto err_domain;
>  	}
>
>  	for (i = 0; i < data->irq_count; i++) {
> @@ -266,7 +266,10 @@ static int imx_irqsteer_probe(struct platform_device *pdev)
>  	pm_runtime_enable(&pdev->dev);
>
>  	return 0;
> -out:
> +
> +err_domain:
> +	irq_domain_remove(data->domain);
> +err_clk:
>  	clk_disable_unprepare(data->ipg_clk);
>  	return ret;
>  }
> --
> 2.34.1
>
>

Re: [PATCH v3 2/4] irqchip/imx-irqsteer: Dispose of IRQ mappings in remove()

[Frank Li]

On Fri, Aug 07, 2026 at 04:23:44PM +0900, Zhipeng.wang_1@oss.nxp.com wrote:
> From: Zhipeng Wang <zhipeng.wang_1@nxp.com>
>
> remove() tears down the chained handlers and the IRQ domain but never
> disposes of the IRQ mappings it created. The parent mappings from
> irq_of_parse_and_map() and the child mappings handed out by the domain
> are leaked, and the child irq_descs are left pointing at the driver's
> irq_chip past irq_domain_remove().
>
> Dispose of the parent mappings alongside the chained handler teardown,
> and dispose of the child mappings before removing the domain.
>
> Fixes: 0136afa08967 ("irqchip: Add driver for imx-irqsteer controller")
> Signed-off-by: Zhipeng Wang <zhipeng.wang_1@nxp.com>
> ---
> Changes in v3:
>  - Split out of the single v2 patch. In v2 this was folded into the
>    module-conversion patch; no functional change.
>
>  drivers/irqchip/irq-imx-irqsteer.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/irqchip/irq-imx-irqsteer.c b/drivers/irqchip/irq-imx-irqsteer.c
> index a2f0629b22a3..4a2fe8ba97f5 100644
> --- a/drivers/irqchip/irq-imx-irqsteer.c
> +++ b/drivers/irqchip/irq-imx-irqsteer.c
> @@ -277,7 +277,7 @@ static int imx_irqsteer_probe(struct platform_device *pdev)
>  static void imx_irqsteer_remove(struct platform_device *pdev)
>  {
>  	struct irqsteer_data *irqsteer_data = platform_get_drvdata(pdev);
> -	int i;
> +	int hwirq, i;
>
>  	for (i = 0; i < irqsteer_data->irq_count; i++) {
>  		if (!irqsteer_data->irq[i])
> @@ -285,8 +285,13 @@ static void imx_irqsteer_remove(struct platform_device *pdev)
>
>  		irq_set_chained_handler_and_data(irqsteer_data->irq[i],
>  						 NULL, NULL);
> +		irq_dispose_mapping(irqsteer_data->irq[i]);
>  	}
>
> +	for (hwirq = 0; hwirq < irqsteer_data->reg_num * 32; hwirq++)
> +		irq_dispose_mapping(irq_find_mapping(irqsteer_data->domain,
> +						     hwirq));
> +

only call once irq_of_parse_and_map(), why need irq_dispose_mapping() twice?

Frank

>  	irq_domain_remove(irqsteer_data->domain);
>
>  	clk_disable_unprepare(irqsteer_data->ipg_clk);
> --
> 2.34.1
>
>

Re: [PATCH v3 3/4] irqchip/imx-irqsteer: Mask all interrupts in probe() and remove()

[Frank Li]

On Fri, Aug 07, 2026 at 04:23:45PM +0900, Zhipeng.wang_1@oss.nxp.com wrote:
> From: Zhipeng Wang <zhipeng.wang_1@nxp.com>
>
> probe() sets up the chained handlers without first masking the input
> interrupts, and remove() leaves the CHANMASK registers untouched. For a
> built-in driver this happened to be harmless because CHANMASK resets to
> all-masked, but once the driver can be unloaded and reloaded a child
> interrupt left unmasked at unload time survives in hardware. On the next
> probe() the parent interrupts are re-mapped and unmasked before the new
> domain is ready, so a still-asserted line immediately storms the parent
> with no handler to service it.
>
> Mask all interrupts in probe() before wiring up the chained handlers, and
> again in remove() so the hardware is left quiesced for the next probe().
> Note CHANMASK uses inverted polarity (a set bit enables the interrupt), so
> masking means writing zero. This mirrors the sibling NXP chained mux
> irq-imx-intmux.c, which has masked all sources in both probe() and remove()
> since commit 2fbb13961e74 ("irqchip: Add NXP INTMUX interrupt multiplexer
> support").
>
> Signed-off-by: Zhipeng Wang <zhipeng.wang_1@nxp.com>
> ---
> Changes in v3:
>  - New patch. Masks all CHANMASK interrupts in probe() and remove() to
>    prevent the interrupt storm on module reload reported by Sashiko AI
>    on v2.
>
>  drivers/irqchip/irq-imx-irqsteer.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
>
> diff --git a/drivers/irqchip/irq-imx-irqsteer.c b/drivers/irqchip/irq-imx-irqsteer.c
> index 4a2fe8ba97f5..0c9c99f1141a 100644
> --- a/drivers/irqchip/irq-imx-irqsteer.c
> +++ b/drivers/irqchip/irq-imx-irqsteer.c
> @@ -236,6 +236,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);
>
> +	/* mask all interrupts before setting up the chained handlers */
> +	for (i = 0; i < data->reg_num; i++)
> +		writel_relaxed(0, data->regs + CHANMASK(i, data->reg_num));
> +
>  	data->domain = irq_domain_create_linear(dev_fwnode(&pdev->dev), data->reg_num * 32,
>  						&imx_irqsteer_domain_ops, data);
>  	if (!data->domain) {
> @@ -279,6 +283,11 @@ static void imx_irqsteer_remove(struct platform_device *pdev)
>  	struct irqsteer_data *irqsteer_data = platform_get_drvdata(pdev);
>  	int hwirq, i;
>
> +	/* mask all interrupts so a stale line cannot storm on the next probe */
> +	for (i = 0; i < irqsteer_data->reg_num; i++)
> +		writel_relaxed(0, irqsteer_data->regs +
> +			       CHANMASK(i, irqsteer_data->reg_num));
> +

You access register here, do you need call runtime pm get to enable clock
first?

Frank

>  	for (i = 0; i < irqsteer_data->irq_count; i++) {
>  		if (!irqsteer_data->irq[i])
>  			break;
> --
> 2.34.1
>
>

RE: [PATCH v3 1/4] irqchip/imx-irqsteer: Fix error handling path in probe()

[Zhipeng Wang (OSS)]

> > From: Zhipeng Wang <zhipeng.wang_1@nxp.com>
> >
> > If the fsl,num-irqs sanity check rejects the value after the IRQ
> > domain has already been created, probe() jumps to a single label that
> > only calls clk_disable_unprepare(), leaving the freshly created IRQ domain
> leaked.
> > The domain-creation failure path shares the same label, which is
> > correct only because the domain is NULL there.
> >
> > Split the error path so that a failure after the domain has been
> > created removes it before disabling the clock, and a failure before
> > that goes straight to the clock cleanup.
> >
> > Fixes: 28528fca4908 ("irqchip/imx-irqsteer: Add multi output
> > interrupts support")
> > Signed-off-by: Zhipeng Wang <zhipeng.wang_1@nxp.com>
> > ---
> > Changes in v3:
> >  - New patch, split out of the single v2 patch. Fixes the irq_domain
> >    leak on the probe() error path reported by Sashiko AI on v2.
> >
> >  drivers/irqchip/irq-imx-irqsteer.c | 9 ++++++---
> >  1 file changed, 6 insertions(+), 3 deletions(-)
>
> I suggest create helper devm_irq_domain_create_leaner()
>
> static inline struct irq_domain *
> devm_irq_domain_create_linear(struct fwnode_handle *fwnode,
>                                                         unsigned int size,
>                                                         const struct irq_domain_ops *ops,
>                                                         void *host_data)
> {
>       const struct irq_domain_info info = {
>               .fwnode         = fwnode,
>               .size           = size,
>               .hwirq_max      = size,
>               .ops            = ops,
>               .host_data      = host_data,
>       };
>       struct irq_domain *d = devm_irq_domain_instantiate(&info);
>
>       return IS_ERR(d) ? NULL : d;
> }
>
> Then imx-irqsteer this devm version. So other drivers can get beneafit also
>
> Frank
>
>
Thanks, good idea. In v4 I added devm_irq_domain_create_linear() as the
devres sibling of irq_domain_create_linear() (patch 1/5), and switched
imx-irqsteer over to it (patch 2/5). With the domain owned by devres the
leak on the probe() error path is fixed structurally, so the hand-rolled
err_domain label is gone and remove() no longer needs irq_domain_remove().
The helper lives in include/linux/irqdomain.h so other drivers can use it
too.

BRs,
Zhipeng
> >
> > diff --git a/drivers/irqchip/irq-imx-irqsteer.c
> > b/drivers/irqchip/irq-imx-irqsteer.c
> > index 87b07f517be3..a2f0629b22a3 100644
> > --- a/drivers/irqchip/irq-imx-irqsteer.c
> > +++ b/drivers/irqchip/irq-imx-irqsteer.c
> > @@ -241,13 +241,13 @@ static int imx_irqsteer_probe(struct
> platform_device *pdev)
> >     if (!data->domain) {
> >             dev_err(&pdev->dev, "failed to create IRQ domain\n");
> >             ret = -ENOMEM;
> > -           goto out;
> > +           goto err_clk;
> >     }
> >     irq_domain_set_pm_device(data->domain, &pdev->dev);
> >
> >     if (!data->irq_count || data->irq_count > CHAN_MAX_OUTPUT_INT) {
> >             ret = -EINVAL;
> > -           goto out;
> > +           goto err_domain;
> >     }
> >
> >     for (i = 0; i < data->irq_count; i++) { @@ -266,7 +266,10 @@ static
> > int imx_irqsteer_probe(struct platform_device *pdev)
> >     pm_runtime_enable(&pdev->dev);
> >
> >     return 0;
> > -out:
> > +
> > +err_domain:
> > +   irq_domain_remove(data->domain);
> > +err_clk:
> >     clk_disable_unprepare(data->ipg_clk);
> >     return ret;
> >  }
> > --
> > 2.34.1
> >
> >

NXP Confidential

RE: [PATCH v3 2/4] irqchip/imx-irqsteer: Dispose of IRQ mappings in remove()

[Zhipeng Wang (OSS)]

NXP Confidential
> -----Original Message-----
> From: Frank Li (OSS) <frank.li@oss.nxp.com>
> Sent: 2026年8月8日 3:11
> To: Zhipeng Wang (OSS) <zhipeng.wang_1@oss.nxp.com>
> Cc: Thomas Gleixner <tglx@kernel.org>; Marc Zyngier <maz@kernel.org>;
> Frank Li <frank.li@nxp.com>; Sascha Hauer <s.hauer@pengutronix.de>;
> Pengutronix Kernel Team <kernel@pengutronix.de>; Fabio Estevam
> <festevam@gmail.com>; Jindong Yue <jindong.yue@nxp.com>; Xuegang Liu
> <xuegang.liu@nxp.com>; linux-kernel@vger.kernel.org; imx@lists.linux.dev;
> linux-arm-kernel@lists.infradead.org
> Subject: Re: [PATCH v3 2/4] irqchip/imx-irqsteer: Dispose of IRQ mappings in
> remove()
>
> On Fri, Aug 07, 2026 at 04:23:44PM +0900, Zhipeng.wang_1@oss.nxp.com
> wrote:
> > From: Zhipeng Wang <zhipeng.wang_1@nxp.com>
> >
> > remove() tears down the chained handlers and the IRQ domain but never
> > disposes of the IRQ mappings it created. The parent mappings from
> > irq_of_parse_and_map() and the child mappings handed out by the domain
> > are leaked, and the child irq_descs are left pointing at the driver's
> > irq_chip past irq_domain_remove().
> >
> > Dispose of the parent mappings alongside the chained handler teardown,
> > and dispose of the child mappings before removing the domain.
> >
> > Fixes: 0136afa08967 ("irqchip: Add driver for imx-irqsteer
> > controller")
> > Signed-off-by: Zhipeng Wang <zhipeng.wang_1@nxp.com>
> > ---
> > Changes in v3:
> >  - Split out of the single v2 patch. In v2 this was folded into the
> >    module-conversion patch; no functional change.
> >
> >  drivers/irqchip/irq-imx-irqsteer.c | 7 ++++++-
> >  1 file changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/irqchip/irq-imx-irqsteer.c
> > b/drivers/irqchip/irq-imx-irqsteer.c
> > index a2f0629b22a3..4a2fe8ba97f5 100644
> > --- a/drivers/irqchip/irq-imx-irqsteer.c
> > +++ b/drivers/irqchip/irq-imx-irqsteer.c
> > @@ -277,7 +277,7 @@ static int imx_irqsteer_probe(struct
> > platform_device *pdev)  static void imx_irqsteer_remove(struct
> > platform_device *pdev)  {
> >     struct irqsteer_data *irqsteer_data = platform_get_drvdata(pdev);
> > -   int i;
> > +   int hwirq, i;
> >
> >     for (i = 0; i < irqsteer_data->irq_count; i++) {
> >             if (!irqsteer_data->irq[i])
> > @@ -285,8 +285,13 @@ static void imx_irqsteer_remove(struct
> > platform_device *pdev)
> >
> >             irq_set_chained_handler_and_data(irqsteer_data->irq[i],
> >                                              NULL, NULL);
> > +           irq_dispose_mapping(irqsteer_data->irq[i]);
> >     }
> >
> > +   for (hwirq = 0; hwirq < irqsteer_data->reg_num * 32; hwirq++)
> > +           irq_dispose_mapping(irq_find_mapping(irqsteer_data->domain,
> > +                                                hwirq));
> > +
>
> only call once irq_of_parse_and_map(), why need irq_dispose_mapping()
> twice?
>
> Frank
The two loops disposed of two different sets of mappings, not the same
ones twice:

 - the first loop disposed of the parent output mappings the driver
   created itself with irq_of_parse_and_map() (irq_count of them);
 - the second loop disposed of the child input mappings the domain
   handed out to downstream consumers (reg_num * 32 hwirqs).

That said, the child mappings are owned and freed by their consumers,
and with the domain now managed by devres (patch 2/5) they are torn down
automatically. So the child loop was both unnecessary and not the
driver's job. In v4 (patch 3/5) remove() only disposes of the parent
mappings it created, matching the sibling irq-imx-intmux.c.

BRs,
Zhipeng

>
> >     irq_domain_remove(irqsteer_data->domain);
> >
> >     clk_disable_unprepare(irqsteer_data->ipg_clk);
> > --
> > 2.34.1
> >
> >

RE: [PATCH v3 3/4] irqchip/imx-irqsteer: Mask all interrupts in probe() and remove()

[Zhipeng Wang (OSS)]

NXP Confidential
> -----Original Message-----
> From: Frank Li (OSS) <frank.li@oss.nxp.com>
> Sent: 2026年8月8日 3:14
> To: Zhipeng Wang (OSS) <zhipeng.wang_1@oss.nxp.com>
> Cc: Thomas Gleixner <tglx@kernel.org>; Marc Zyngier <maz@kernel.org>;
> Frank Li <frank.li@nxp.com>; Sascha Hauer <s.hauer@pengutronix.de>;
> Pengutronix Kernel Team <kernel@pengutronix.de>; Fabio Estevam
> <festevam@gmail.com>; Jindong Yue <jindong.yue@nxp.com>; Xuegang Liu
> <xuegang.liu@nxp.com>; linux-kernel@vger.kernel.org; imx@lists.linux.dev;
> linux-arm-kernel@lists.infradead.org
> Subject: Re: [PATCH v3 3/4] irqchip/imx-irqsteer: Mask all interrupts in probe()
> and remove()
>
> On Fri, Aug 07, 2026 at 04:23:45PM +0900, Zhipeng.wang_1@oss.nxp.com
> wrote:
> > From: Zhipeng Wang <zhipeng.wang_1@nxp.com>
> >
> > probe() sets up the chained handlers without first masking the input
> > interrupts, and remove() leaves the CHANMASK registers untouched. For
> > a built-in driver this happened to be harmless because CHANMASK resets
> > to all-masked, but once the driver can be unloaded and reloaded a
> > child interrupt left unmasked at unload time survives in hardware. On
> > the next
> > probe() the parent interrupts are re-mapped and unmasked before the
> > new domain is ready, so a still-asserted line immediately storms the
> > parent with no handler to service it.
> >
> > Mask all interrupts in probe() before wiring up the chained handlers,
> > and again in remove() so the hardware is left quiesced for the next probe().
> > Note CHANMASK uses inverted polarity (a set bit enables the
> > interrupt), so masking means writing zero. This mirrors the sibling
> > NXP chained mux irq-imx-intmux.c, which has masked all sources in both
> > probe() and remove() since commit 2fbb13961e74 ("irqchip: Add NXP
> > INTMUX interrupt multiplexer support").
> >
> > Signed-off-by: Zhipeng Wang <zhipeng.wang_1@nxp.com>
> > ---
> > Changes in v3:
> >  - New patch. Masks all CHANMASK interrupts in probe() and remove() to
> >    prevent the interrupt storm on module reload reported by Sashiko AI
> >    on v2.
> >
> >  drivers/irqchip/irq-imx-irqsteer.c | 9 +++++++++
> >  1 file changed, 9 insertions(+)
> >
> > diff --git a/drivers/irqchip/irq-imx-irqsteer.c
> > b/drivers/irqchip/irq-imx-irqsteer.c
> > index 4a2fe8ba97f5..0c9c99f1141a 100644
> > --- a/drivers/irqchip/irq-imx-irqsteer.c
> > +++ b/drivers/irqchip/irq-imx-irqsteer.c
> > @@ -236,6 +236,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);
> >
> > +   /* mask all interrupts before setting up the chained handlers */
> > +   for (i = 0; i < data->reg_num; i++)
> > +           writel_relaxed(0, data->regs + CHANMASK(i, data->reg_num));
> > +
> >     data->domain = irq_domain_create_linear(dev_fwnode(&pdev->dev),
> data->reg_num * 32,
> >                                             &imx_irqsteer_domain_ops, data);
> >     if (!data->domain) {
> > @@ -279,6 +283,11 @@ static void imx_irqsteer_remove(struct
> platform_device *pdev)
> >     struct irqsteer_data *irqsteer_data = platform_get_drvdata(pdev);
> >     int hwirq, i;
> >
> > +   /* mask all interrupts so a stale line cannot storm on the next probe */
> > +   for (i = 0; i < irqsteer_data->reg_num; i++)
> > +           writel_relaxed(0, irqsteer_data->regs +
> > +                          CHANMASK(i, irqsteer_data->reg_num));
> > +
>
> You access register here, do you need call runtime pm get to enable clock first?
>
> Frank
>

Good catch. Rather than adding a runtime PM get/put around the register
access, I dropped the masking in remove() entirely (v4 patch 4/5).

The remove()-time masking was unnecessary: the next probe() masks all
sources before it re-maps and unmasks the parent interrupts, which is
the only window in which a stale line could storm. Masking in remove()
would also mean touching CHANMASK while the device may already be
runtime-suspended with the clock gated - exactly the problem you point
out. So v4 masks only in probe().

BRs,
Zhipeng

> >     for (i = 0; i < irqsteer_data->irq_count; i++) {
> >             if (!irqsteer_data->irq[i])
> >                     break;
> > --
> > 2.34.1
> >
> >