[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()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] The chained IRQ handler accesses hardware registers without checking if the device is active, risking a system crash on spurious interrupts.
- [High] The driver fails to explicitly mask hardware interrupts during initialization, risking an unhandled interrupt storm during probe.
--

--- Patch [1]: [PATCH v3 1/4] irqchip/imx-irqsteer: Fix error handling path in probe() ---
commit bda073233892b00ff41d8b06dd3aac163410e9a2
Author: Zhipeng Wang <zhipeng.wang_1@nxp.com>

irqchip/imx-irqsteer: Fix error handling path in probe()

This commit splits the error handling path in imx_irqsteer_probe() so that
failures occurring after the IRQ domain creation properly remove the domain
before disabling the clock. This prevents an IRQ domain leak during probe
failures.

> diff --git a/drivers/irqchip/irq-imx-irqsteer.c b/drivers/irqchip/irq-imx-irqsteer.c
> index 87b07f517be35..a2f0629b22a37 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->irq_count || data->irq_count > CHAN_MAX_OUTPUT_INT) {
>  		ret = -EINVAL;
> -		goto out;
> +		goto err_domain;
>  	}
>  
>  	for (i = 0; i < data->irq_count; i++) {

[Severity: High]
This is a pre-existing issue, but does this initialization fail to explicitly
mask hardware interrupts before registering the chained handler?

When irq_set_chained_handler_and_data() is called in this loop to register
imx_irqsteer_irq_handler(), the parent IRQ is unmasked at the GIC. If the
bootloader leaves any input interrupt lines unmasked and asserted in the
CHANMASK and CHANSTATUS registers, the GIC will fire immediately.

Since no child handler is registered yet, the interrupt is never cleared,
which causes chained_irq_exit() to re-unmask the parent IRQ. Does this risk
an infinite unhandled interrupt storm during boot?

Also, regarding imx_irqsteer_irq_handler() which is registered in this
loop:

imx_irqsteer_irq_handler() {
    ...
    irqmap = readl_relaxed(data->regs + CHANSTATUS(idx, data->reg_num));
    ...
}

[Severity: High]
This isn't a bug introduced by this patch, but does the chained handler
access hardware registers without checking if the device is active?

If no child interrupts are requested, the IRQ core drops its PM reference
and the device enters runtime suspend, disabling the IPG clock. If a
spurious interrupt arrives on the parent IRQ line from the GIC, the handler
is invoked and unconditionally reads the CHANSTATUS register with the clock
off.

Could this trigger a synchronous external abort (kernel panic)? Should the
handler verify the device's PM runtime state using pm_runtime_get_if_active()
before accessing registers?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260807072346.1222389-1-Zhipeng.wang_1@oss.nxp.com?part=1

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Missing `pm_runtime_get_sync()` in `imx_irqsteer_remove()` causes a fatal bus error on device unbind.
--

--- Patch [3]: [PATCH v3 3/4] irqchip/imx-irqsteer: Mask all interrupts in probe() and remove() ---
commit cdd5c75cd302eeb1d325e53989eb54042df373f6
Author: Zhipeng Wang <zhipeng.wang_1@nxp.com>

irqchip/imx-irqsteer: Mask all interrupts in probe() and remove()

This commit masks all interrupts in probe before wiring up chained handlers,
and again in remove to quiesce the hardware for the next probe. This is
intended to prevent interrupt storms on module reload.

> diff --git a/drivers/irqchip/irq-imx-irqsteer.c b/drivers/irqchip/irq-imx-irqsteer.c
> index 4a2fe8ba97f50..0c9c99f1141a9 100644
> --- a/drivers/irqchip/irq-imx-irqsteer.c
> +++ b/drivers/irqchip/irq-imx-irqsteer.c
[ ... ]
> @@ -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));
[Severity: High]
Will this hardware register access cause a fatal bus error during device unbind?

During driver unbind (via module unload or sysfs), the driver core explicitly
calls pm_runtime_put_sync() before calling the driver's remove callback. This
drops the runtime PM usage count and synchronously suspends the device, which
disables the device's clock.

If the hardware is already unclocked, does calling writel_relaxed() on the
CHANMASK hardware registers without first waking the device up via
pm_runtime_get_sync() trigger a synchronous external abort on i.MX platforms?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260807072346.1222389-1-Zhipeng.wang_1@oss.nxp.com?part=3

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] Missing explicit runtime PM management in `remove()` causes a hardware access crash and a clock enable count underflow during devres cleanup.

Pre-existing issues:
- [High] The chained IRQ handler accesses hardware registers unconditionally without ensuring the device's clock is enabled, which can crash the system on spurious interrupts.
--

--- Patch [4]: [PATCH v3 4/4] irqchip/imx-irqsteer: Allow building as module ---
commit d2409da6cb54e5e7df533908b454c22db10f15a7
Author: Jindong Yue <jindong.yue@nxp.com>

irqchip/imx-irqsteer: Allow building as module

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(). Let the driver core own the clock and runtime PM
lifetime so that remove() does not have to hand-balance them.

> diff --git a/drivers/irqchip/irq-imx-irqsteer.c b/drivers/irqchip/irq-imx-irqsteer.c
> index 0c9c99f1141a9..e62862617b8d1 100644
> --- a/drivers/irqchip/irq-imx-irqsteer.c
> +++ b/drivers/irqchip/irq-imx-irqsteer.c

[ ... ]

> @@ -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;

[Severity: High]
This is a pre-existing issue, but does the chained IRQ handler access hardware
registers unconditionally without ensuring the device's clock is enabled?

If no child interrupts are requested by client drivers, the irqsteer device
drops its PM usage count to 0 and enters runtime suspend, disabling ipg_clk.
However, the parent interrupt handler imx_irqsteer_irq_handler remains
registered and active with the core IRQ subsystem.

If a spurious interrupt arrives while the device is suspended, the handler
executes and directly reads the CHANSTATUS register:

drivers/irqchip/irq-imx-irqsteer.c:imx_irqsteer_irq_handler() {
	...
	irqmap = readl_relaxed(data->regs + CHANSTATUS(idx, data->reg_num));
	...
}

Accessing memory-mapped registers of an IP block with a gated interface clock
can trigger a hardware fault (external abort / bus fault).

Should the handler use pm_runtime_get_if_active() to verify the power state
before accessing hardware?

[ ... ]

> @@ -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);
>  }

[Severity: High]
Does this change cause a hardware access crash during driver unbind?

On driver unbind, the driver core drops its PM reference by calling
pm_runtime_put_sync() before invoking the driver's remove callback. Since no
children are holding PM references, the device suspends synchronously, and
imx_irqsteer_suspend() disables ipg_clk.

When imx_irqsteer_remove() executes, it writes to CHANMASK hardware registers
with a disabled clock, which can trigger a fatal bus fault:

drivers/irqchip/irq-imx-irqsteer.c:imx_irqsteer_remove() {
	...
	for (i = 0; i < irqsteer_data->reg_num; i++)
		writel_relaxed(0, irqsteer_data->regs +
			       CHANMASK(i, irqsteer_data->reg_num));
	...
}

Additionally, devres teardown runs after remove() in LIFO order and unwinds
devm_clk_get_enabled() by calling clk_disable_unprepare(). This decrements
the already-zero clock enable count, throwing a WARN and corrupting the clock
tree.

Should the driver explicitly wake the device in remove() (e.g., via
pm_runtime_get_sync()) before accessing registers?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260807072346.1222389-1-Zhipeng.wang_1@oss.nxp.com?part=4

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
>
>