[PATCH v6 0/9] irqchip/imx-irqsteer: Allow building as module

[PATCH v6 0/9] 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, along
with the pre-existing bug fixes that become reachable once the driver
can be unbound and reloaded.

The fixes come first so they can be picked up (and backported) on their
own, followed by the runtime-PM/clock rework, the module conversion and
the irqdomain helper cleanup:

 1. Call chained_irq_exit() on the handler error path.
 2. Validate IRQ count before creating the domain (Fabio).
 3. Dispose of the parent IRQ mappings in remove().
 4. Convert to devm_pm_runtime_set_active_enabled() - fixes the
    "Unbalanced pm_runtime_enable!" on unbind, with a Fixes tag so it can
    be backported to stable (e.g. 6.18).
 5. Let devres own the clock.
 6. Mask all interrupts in probe().
 7. Allow building as module.
 8. genirq/irqdomain: add devm_irq_domain_create_linear().
 9. Use devm_irq_domain_create_linear() in the driver.

Changes in v6:
 - Split the runtime-PM fix out of the devres rework into its own patch
   (patch 4) with Fixes: 4730d2233311, so the "Unbalanced
   pm_runtime_enable!" fix can be backported to stable independently of
   the clock rework (Fabio Estevam). devm_pm_runtime_set_active_enabled()
   is now set up before the IRQ domain is created, so its error path just
   bails out and no err_irq unwind is needed.
 - Drop the err_irq and out labels in probe(); each failure path now
   returns directly (Radu Rendec).
 - In remove(), drop the runtime-PM reference taken by
   pm_runtime_resume_and_get() with pm_runtime_put_noidle() (guarded by
   the return value), so the usage count is not leaked and autosuspend
   keeps working across module reload (Sashiko AI).
 - Reorder so the Fixes-tagged patches come before the rework/cleanup.
 - Rebased onto the current irq/core.

Changes in v5:
 - Consolidate the three previously-scattered series/patches into one
   coherent series (Thomas Gleixner).

v5: https://lore.kernel.org/r/20260821101039.4037925-1-Zhipeng.wang_1@oss.nxp.com
v4: https://lore.kernel.org/r/20260819090543.585131-1-Zhipeng.wang_1@oss.nxp.com
v3: https://lore.kernel.org/r/20260807072346.1222389-1-Zhipeng.wang_1@oss.nxp.com
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

Fabio Estevam (1):
  irqchip/imx-irqsteer: Validate IRQ count before creating domain

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

Zhipeng Wang (7):
  irqchip/imx-irqsteer: Call chained_irq_exit() on the handler error
    path
  irqchip/imx-irqsteer: Dispose of parent IRQ mappings in remove()
  irqchip/imx-irqsteer: Convert to devm_pm_runtime_set_active_enabled()
  irqchip/imx-irqsteer: Mask all interrupts in probe()
  irqchip/imx-irqsteer: Let devres own the clock
  genirq/irqdomain: Add devm_irq_domain_create_linear()
  irqchip/imx-irqsteer: Use devm_irq_domain_create_linear()

 drivers/irqchip/Kconfig            |  2 +-
 drivers/irqchip/irq-imx-irqsteer.c | 74 ++++++++++++++++++------------
 include/linux/irqdomain.h          | 30 ++++++++++++
 3 files changed, 75 insertions(+), 31 deletions(-)


base-commit: a0e1fdb96578ea562a03c487f70673d228824d0a
-- 
2.34.1

[PATCH v6 1/9] irqchip/imx-irqsteer: Call chained_irq_exit() on the handler error path

[Zhipeng.wang_1]

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

A chained handler must pair every chained_irq_enter() with a
chained_irq_exit() before returning, so that the parent interrupt's flow
control is completed (EOI for fasteoi parents, unmask for level-triggered
parents). Skipping it leaves the parent interrupt unacknowledged, blocking
further interrupts multiplexed through that line.

When imx_irqsteer_get_hwirq_base() fails, the handler returned early
without calling chained_irq_exit(). Route the error path through the
existing chained_irq_exit() so the parent interrupt is always completed
before returning.

Fixes: 28528fca4908 ("irqchip/imx-irqsteer: Add multi output interrupts support")
Signed-off-by: Zhipeng Wang <zhipeng.wang_1@nxp.com>
Reviewed-by: Frank Li <Frank.Li@nxp.com>
Reviewed-by: Radu Rendec <radu@rendec.net>
---
 drivers/irqchip/irq-imx-irqsteer.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/irqchip/irq-imx-irqsteer.c b/drivers/irqchip/irq-imx-irqsteer.c
index 87b07f517be3..1b8d0c8eedb9 100644
--- a/drivers/irqchip/irq-imx-irqsteer.c
+++ b/drivers/irqchip/irq-imx-irqsteer.c
@@ -154,7 +154,7 @@ static void imx_irqsteer_irq_handler(struct irq_desc *desc)
 	if (hwirq < 0) {
 		pr_warn("%s: unable to get hwirq base for irq %d\n",
 			__func__, irq);
-		return;
+		goto out;
 	}
 
 	for (i = 0; i < 2; i++, hwirq += 32) {
@@ -172,6 +172,7 @@ static void imx_irqsteer_irq_handler(struct irq_desc *desc)
 			generic_handle_domain_irq(data->domain, pos + hwirq);
 	}
 
+out:
 	chained_irq_exit(irq_desc_get_chip(desc), desc);
 }
 
-- 
2.34.1

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

[Zhipeng.wang_1]

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>
Signed-off-by: Zhipeng Wang <zhipeng.wang_1@nxp.com>
---
 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 1b8d0c8eedb9..766756e76c47 100644
--- a/drivers/irqchip/irq-imx-irqsteer.c
+++ b/drivers/irqchip/irq-imx-irqsteer.c
@@ -218,6 +218,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,
@@ -246,11 +248,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.34.1

[PATCH v6 3/9] irqchip/imx-irqsteer: Dispose of parent IRQ mappings in remove()

[Zhipeng.wang_1]

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

probe() maps the parent output interrupts with irq_of_parse_and_map(),
but remove() only unchains the handlers and never disposes of those
mappings, leaking them on unbind. The child mappings handed out by the
domain are freed by their consumers and, together with the domain, are
now torn down by devres, so remove() only has to dispose of the parent
mappings it created itself.

Dispose of the parent mappings alongside the chained-handler teardown.

Fixes: 0136afa08967 ("irqchip: Add driver for imx-irqsteer controller")
Signed-off-by: Zhipeng Wang <zhipeng.wang_1@nxp.com>
Reviewed-by: Frank Li <Frank.Li@nxp.com>
Reviewed-by: Radu Rendec <radu@rendec.net>
---
 drivers/irqchip/irq-imx-irqsteer.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/irqchip/irq-imx-irqsteer.c b/drivers/irqchip/irq-imx-irqsteer.c
index 766756e76c47..d05de3d26fb2 100644
--- a/drivers/irqchip/irq-imx-irqsteer.c
+++ b/drivers/irqchip/irq-imx-irqsteer.c
@@ -280,6 +280,7 @@ 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]);
 	}
 
 	irq_domain_remove(irqsteer_data->domain);
-- 
2.34.1

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

[Zhipeng.wang_1]

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);
 }
 
 #ifdef CONFIG_PM
-- 
2.34.1

[PATCH v6 5/9] irqchip/imx-irqsteer: Mask all interrupts in probe()

[Zhipeng.wang_1]

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

probe() sets up the chained handlers without first masking the input
interrupts. 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.
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 masks all sources at probe() time.

Masking is only done in probe(), not in remove(): the next probe()
quiesces the hardware 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.

Signed-off-by: Zhipeng Wang <zhipeng.wang_1@nxp.com>
Reviewed-by: Radu Rendec <radu@rendec.net>
---
 drivers/irqchip/irq-imx-irqsteer.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/irqchip/irq-imx-irqsteer.c b/drivers/irqchip/irq-imx-irqsteer.c
index 5acc04504e52..fa233de9bc2b 100644
--- a/drivers/irqchip/irq-imx-irqsteer.c
+++ b/drivers/irqchip/irq-imx-irqsteer.c
@@ -239,6 +239,14 @@ 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 wiring up the chained handlers. CHANMASK
+	 * has inverted polarity (a set bit enables the interrupt), so writing
+	 * zero masks the source.
+	 */
+	for (i = 0; i < data->reg_num; i++)
+		writel_relaxed(0, data->regs + CHANMASK(i, data->reg_num));
+
 	ret = devm_pm_runtime_set_active_enabled(&pdev->dev);
 	if (ret)
 		goto out;
-- 
2.34.1

[PATCH v6 6/9] irqchip/imx-irqsteer: Let devres own the clock

[Zhipeng.wang_1]

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

Let the driver core own the clock lifetime so that the probe() error
path and remove() do not have to hand-balance it:

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

Signed-off-by: Zhipeng Wang <zhipeng.wang_1@nxp.com>
---
 drivers/irqchip/irq-imx-irqsteer.c | 37 +++++++++++++++---------------
 1 file changed, 19 insertions(+), 18 deletions(-)

diff --git a/drivers/irqchip/irq-imx-irqsteer.c b/drivers/irqchip/irq-imx-irqsteer.c
index fa233de9bc2b..15ee50f65d54 100644
--- a/drivers/irqchip/irq-imx-irqsteer.c
+++ b/drivers/irqchip/irq-imx-irqsteer.c
@@ -194,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");
@@ -229,12 +229,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);
@@ -249,14 +243,13 @@ static int imx_irqsteer_probe(struct platform_device *pdev)
 
 	ret = devm_pm_runtime_set_active_enabled(&pdev->dev);
 	if (ret)
-		goto out;
+		return ret;
 
 	data->domain = irq_domain_create_linear(dev_fwnode(&pdev->dev), data->reg_num * 32,
 						&imx_irqsteer_domain_ops, data);
 	if (!data->domain) {
 		dev_err(&pdev->dev, "failed to create IRQ domain\n");
-		ret = -ENOMEM;
-		goto out;
+		return -ENOMEM;
 	}
 	irq_domain_set_pm_device(data->domain, &pdev->dev);
 
@@ -273,15 +266,23 @@ static int imx_irqsteer_probe(struct platform_device *pdev)
 	platform_set_drvdata(pdev, data);
 
 	return 0;
-out:
-	clk_disable_unprepare(data->ipg_clk);
-	return ret;
 }
 
 static void imx_irqsteer_remove(struct platform_device *pdev)
 {
 	struct irqsteer_data *irqsteer_data = platform_get_drvdata(pdev);
-	int i;
+	int i, ret;
+
+	/*
+	 * The device may be runtime-suspended here, in which case the runtime
+	 * suspend callback has already dropped the clock enable count. Resume
+	 * it so the devres clk_disable_unprepare(), which runs after remove(),
+	 * finds the clock enabled and stays balanced. On success drop the
+	 * usage count again with pm_runtime_put_noidle(): it must not trigger a
+	 * suspend (which would re-disable the clock) and must not leak to the
+	 * next probe of this persistent device.
+	 */
+	ret = pm_runtime_resume_and_get(&pdev->dev);
 
 	for (i = 0; i < irqsteer_data->irq_count; i++) {
 		if (!irqsteer_data->irq[i])
@@ -294,8 +295,8 @@ static void imx_irqsteer_remove(struct platform_device *pdev)
 
 	irq_domain_remove(irqsteer_data->domain);
 
-	if (!pm_runtime_status_suspended(&pdev->dev))
-		clk_disable_unprepare(irqsteer_data->ipg_clk);
+	if (ret >= 0)
+		pm_runtime_put_noidle(&pdev->dev);
 }
 
 #ifdef CONFIG_PM
@@ -325,7 +326,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;
 }
@@ -335,7 +336,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;
-- 
2.34.1

[PATCH v6 7/9] 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(), and add the MODULE_DEVICE_TABLE() and module
metadata needed for autoloading.

The clock, runtime PM and IRQ domain are already owned by devres, so no
further teardown is needed to support unbind and reload.

Signed-off-by: Jindong Yue <jindong.yue@nxp.com>
Signed-off-by: Zhipeng Wang <zhipeng.wang_1@nxp.com>
Reviewed-by: Radu Rendec <radu@rendec.net>
---
 drivers/irqchip/Kconfig            | 2 +-
 drivers/irqchip/irq-imx-irqsteer.c | 7 ++++++-
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
index 20b77fbc51ee..105108d2e6ff 100644
--- a/drivers/irqchip/Kconfig
+++ b/drivers/irqchip/Kconfig
@@ -555,7 +555,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 15ee50f65d54..85edd5e33ca9 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>
@@ -359,6 +360,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 = {
@@ -369,4 +371,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

[PATCH v6 8/9] genirq/irqdomain: Add devm_irq_domain_create_linear()

[Zhipeng.wang_1]

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

irq_domain_create_linear() has no devres-managed counterpart, so a
driver that wants a linear revmap domain tied to the device lifetime has
to open-code an irq_domain_info and call devm_irq_domain_instantiate()
directly.

Add devm_irq_domain_create_linear() as the devres-managed sibling of
irq_domain_create_linear(): it builds the same linear-revmap
irq_domain_info and hands it to devm_irq_domain_instantiate(), so the
domain is removed when the owning device is unbound. The return
convention matches irq_domain_create_linear() (NULL on failure).

Signed-off-by: Zhipeng Wang <zhipeng.wang_1@nxp.com>
Reviewed-by: Frank Li <Frank.Li@nxp.com>
---
 include/linux/irqdomain.h | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h
index 73c25d40846c..b6b360cb6525 100644
--- a/include/linux/irqdomain.h
+++ b/include/linux/irqdomain.h
@@ -457,6 +457,36 @@ static inline struct irq_domain *irq_domain_create_linear(struct fwnode_handle *
 	return IS_ERR(d) ? NULL : d;
 }
 
+/**
+ * devm_irq_domain_create_linear - Allocate and register a linear revmap
+ *				   irq_domain tied to the device lifetime.
+ * @dev:	Device that owns the domain. The domain is removed via devres
+ *		when the device is unbound.
+ * @fwnode:	pointer to interrupt controller's FW node.
+ * @size:	Number of interrupts in the domain.
+ * @ops:	map/unmap domain callbacks
+ * @host_data:	Controller private data pointer
+ *
+ * Returns: Newly created irq_domain, or NULL on failure.
+ */
+static inline struct irq_domain *devm_irq_domain_create_linear(struct device *dev,
+							       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(dev, &info);
+
+	return IS_ERR(d) ? NULL : d;
+}
+
 static inline struct irq_domain *irq_domain_create_tree(struct fwnode_handle *fwnode,
 							const struct irq_domain_ops *ops,
 							void *host_data)
-- 
2.34.1

[PATCH v6 9/9] irqchip/imx-irqsteer: Use devm_irq_domain_create_linear()

[Zhipeng.wang_1]

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

The driver open-codes an irq_domain_info and calls
devm_irq_domain_instantiate() only to get a linear revmap domain tied to
the device lifetime. Use the new devm_irq_domain_create_linear() helper
instead. No functional change.

Signed-off-by: Zhipeng Wang <zhipeng.wang_1@nxp.com>
---
 drivers/irqchip/irq-imx-irqsteer.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/irqchip/irq-imx-irqsteer.c b/drivers/irqchip/irq-imx-irqsteer.c
index 85edd5e33ca9..f5601769d036 100644
--- a/drivers/irqchip/irq-imx-irqsteer.c
+++ b/drivers/irqchip/irq-imx-irqsteer.c
@@ -246,8 +246,9 @@ static int imx_irqsteer_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
-	data->domain = irq_domain_create_linear(dev_fwnode(&pdev->dev), data->reg_num * 32,
-						&imx_irqsteer_domain_ops, data);
+	data->domain = devm_irq_domain_create_linear(&pdev->dev, dev_fwnode(&pdev->dev),
+						     data->reg_num * 32,
+						     &imx_irqsteer_domain_ops, data);
 	if (!data->domain) {
 		dev_err(&pdev->dev, "failed to create IRQ domain\n");
 		return -ENOMEM;
@@ -294,8 +295,6 @@ static void imx_irqsteer_remove(struct platform_device *pdev)
 		irq_dispose_mapping(irqsteer_data->irq[i]);
 	}
 
-	irq_domain_remove(irqsteer_data->domain);
-
 	if (ret >= 0)
 		pm_runtime_put_noidle(&pdev->dev);
 }
-- 
2.34.1

Re: [PATCH v6 6/9] irqchip/imx-irqsteer: Let devres own the clock

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] pm: clock underflow warning on devres cleanup if pm_runtime_resume_and_get() fails
--

commit 7ea8370844753a72ff02a8217ace864b04cf2175
Author: Zhipeng Wang <zhipeng.wang_1@nxp.com>

irqchip/imx-irqsteer: Let devres own the clock

This commit transitions the imx-irqsteer driver to use devm_clk_get_enabled()
for clock lifecycle management. It also limits runtime PM callbacks to
clk_enable() and clk_disable() since preparation is handled by devres.

> diff --git a/drivers/irqchip/irq-imx-irqsteer.c b/drivers/irqchip/irq-imx-irqsteer.c
> index fa233de9bc2b2..15ee50f65d54c 100644
> --- a/drivers/irqchip/irq-imx-irqsteer.c
> +++ b/drivers/irqchip/irq-imx-irqsteer.c
[ ... ]
> @@ -273,15 +266,23 @@ static int imx_irqsteer_probe(struct platform_device *pdev)
>  	platform_set_drvdata(pdev, data);
>  
>  	return 0;
> -out:
> -	clk_disable_unprepare(data->ipg_clk);
> -	return ret;
>  }
>  
>  static void imx_irqsteer_remove(struct platform_device *pdev)
>  {
>  	struct irqsteer_data *irqsteer_data = platform_get_drvdata(pdev);
> -	int i;
> +	int i, ret;
> +
> +	/*
> +	 * The device may be runtime-suspended here, in which case the runtime
> +	 * suspend callback has already dropped the clock enable count. Resume
> +	 * it so the devres clk_disable_unprepare(), which runs after remove(),
> +	 * finds the clock enabled and stays balanced. On success drop the
> +	 * usage count again with pm_runtime_put_noidle(): it must not trigger a
> +	 * suspend (which would re-disable the clock) and must not leak to the
> +	 * next probe of this persistent device.
> +	 */
> +	ret = pm_runtime_resume_and_get(&pdev->dev);
>  
>  	for (i = 0; i < irqsteer_data->irq_count; i++) {
>  		if (!irqsteer_data->irq[i])
> @@ -294,8 +295,8 @@ static void imx_irqsteer_remove(struct platform_device *pdev)
>  
>  	irq_domain_remove(irqsteer_data->domain);
>  
> -	if (!pm_runtime_status_suspended(&pdev->dev))
> -		clk_disable_unprepare(irqsteer_data->ipg_clk);
> +	if (ret >= 0)
> +		pm_runtime_put_noidle(&pdev->dev);

[Severity: Medium]
If pm_runtime_resume_and_get() fails, the device remains suspended and its
clock remains disabled.

Since devres owns the clock via devm_clk_get_enabled(), won't devres
unconditionally call clk_disable_unprepare() after imx_irqsteer_remove()
returns?

Could this trigger a WARN(core->enable_count == 0) in the clock framework and
unbalance the clock state?

>  }

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

Re: [PATCH v6 9/9] irqchip/imx-irqsteer: Use devm_irq_domain_create_linear()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] core: code and commit message mismatch regarding old domain API
--

commit 05a4c59bf91334e37803afcd72a82640aa12007b
Author: Zhipeng Wang <zhipeng.wang_1@nxp.com>

irqchip/imx-irqsteer: Use devm_irq_domain_create_linear()

This commit updates the imx-irqsteer driver to use the device-managed
devm_irq_domain_create_linear() function for creating the IRQ domain. It
also removes the manual domain removal code in the driver's remove path.

> The driver open-codes an irq_domain_info and calls
> devm_irq_domain_instantiate() only to get a linear revmap domain tied to
> the device lifetime. Use the new devm_irq_domain_create_linear() helper
> instead. No functional change.

[Severity: Medium]
Does this commit message accurately describe the previous state of the code?

The commit message states that the driver open-codes an irq_domain_info and
calls devm_irq_domain_instantiate(). However, the diff shows the driver was
actually using irq_domain_create_linear() in imx_irqsteer_probe() and
tearing it down manually with irq_domain_remove() in imx_irqsteer_remove().

Could the commit message have been copied from another patch in the series?

> diff --git a/drivers/irqchip/irq-imx-irqsteer.c b/drivers/irqchip/irq-imx-irqsteer.c
> index 85edd5e33ca9c..f5601769d0363 100644
> --- a/drivers/irqchip/irq-imx-irqsteer.c
> +++ b/drivers/irqchip/irq-imx-irqsteer.c
> @@ -246,8 +246,9 @@ static int imx_irqsteer_probe(struct platform_device *pdev)
>  	if (ret)
>  		return ret;
>  
> -	data->domain = irq_domain_create_linear(dev_fwnode(&pdev->dev), data->reg_num * 32,
> -						&imx_irqsteer_domain_ops, data);
> +	data->domain = devm_irq_domain_create_linear(&pdev->dev, dev_fwnode(&pdev->dev),
> +						     data->reg_num * 32,
> +						     &imx_irqsteer_domain_ops, data);

[ ... ]

> @@ -294,8 +295,6 @@ static void imx_irqsteer_remove(struct platform_device *pdev)
>  		irq_dispose_mapping(irqsteer_data->irq[i]);
>  	}
>  
> -	irq_domain_remove(irqsteer_data->domain);
> -
>  	if (ret >= 0)
>  		pm_runtime_put_noidle(&pdev->dev);
>  }

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

Re: [PATCH v6 5/9] irqchip/imx-irqsteer: Mask all interrupts in probe()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] irqchip/imx-irqsteer: missing wmb() allows interrupt storm race
--

commit e530007174d6dce33c6ec542e006f51e4a17dc73
Author: Zhipeng Wang <zhipeng.wang_1@nxp.com>

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

This commit masks all input interrupts in probe() before setting up
chained handlers. This prevents an interrupt storm if a child interrupt
was left unmasked during driver unload, as the hardware would otherwise
assert it immediately upon re-mapping in the new domain.

> diff --git a/drivers/irqchip/irq-imx-irqsteer.c b/drivers/irqchip/irq-imx-irqsteer.c
> index 5acc04504e520..fa233de9bc2b2 100644
> --- a/drivers/irqchip/irq-imx-irqsteer.c
> +++ b/drivers/irqchip/irq-imx-irqsteer.c

[ ... ]

> @@ -239,6 +239,14 @@ 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 wiring up the chained handlers. CHANMASK
> +	 * has inverted polarity (a set bit enables the interrupt), so writing
> +	 * zero masks the source.
> +	 */
> +	for (i = 0; i < data->reg_num; i++)
> +		writel_relaxed(0, data->regs + CHANMASK(i, data->reg_num));
> +

[Severity: Medium]
Is an explicit memory barrier needed here to ensure the masks are applied
before the parent interrupt is unmasked?

Since writel_relaxed() does not provide a barrier, these writes to the IRQ
Steer hardware could be reordered. The parent GIC interrupt may be unmasked
before the IRQ Steer actually processes the interrupt masks, leaving a window
for the interrupt storm this patch is designed to prevent.

The parent GIC driver uses writel_relaxed() to unmask (for example,
gic_unmask_irq() in GICv2 or gic_poke_irq() in GICv3). Furthermore,
intervening locks like desc->lock in the IRQ core or irq_domain_mutex
provide only ACQUIRE and RELEASE semantics. On ARM64, an intervening RELEASE
followed by an ACQUIRE to different lock addresses can be reordered by the
CPU.

Could this allow the subsequent GIC relaxed write to cross the prior IRQ
Steer relaxed write, meaning a full wmb() is strictly required to order
writes to different I/O peripherals?

>  	ret = devm_pm_runtime_set_active_enabled(&pdev->dev);
>  	if (ret)
>  		goto out;

[ ... ]

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

Re: [PATCH v6 7/9] irqchip/imx-irqsteer: Allow building as module

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] irqchip: imx-irqsteer module unload causes use-after-free
- [High] irqchip: imx-irqsteer UAF due to missing synchronize_irq() on remove
- [Medium] pm: CCF warning in devres teardown if pm_runtime_resume_and_get() fails
--

commit 47b4fb3a979bc15759a7bda151b9b79c2c92332a
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(), and add the MODULE_DEVICE_TABLE() and module
metadata needed for autoloading.

> diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
> index 20b77fbc51ee0..105108d2e6ff7 100644
> --- a/drivers/irqchip/Kconfig
> +++ b/drivers/irqchip/Kconfig
> @@ -555,7 +555,7 @@ config CSKY_APB_INTC
>  	  the controller's register.
>  
>  config IMX_IRQSTEER
> -	bool "i.MX IRQSTEER support"
> +	tristate "i.MX IRQSTEER support"

[Severity: High]
Does converting IMX_IRQSTEER to a tristate expose a use-after-free during
module unload?

The IRQ domain framework does not take a module reference on the irqchip
module. This means the module can be unloaded even while downstream
consumers hold active IRQ mappings in their irq_desc structures.

When unloaded, irq_domain_remove() emits a warning but proceeds to free
the domain. The irqchip module's .text and .rodata sections are then
freed, leaving the downstream irq_desc structures with dangling chip
pointers to the freed imx_irqsteer_irq_chip struct. A subsequent read of
/proc/interrupts will dereference the freed pointer.

> diff --git a/drivers/irqchip/irq-imx-irqsteer.c b/drivers/irqchip/irq-imx-irqsteer.c
> index 15ee50f65d54c..85edd5e33ca9c 100644
> --- a/drivers/irqchip/irq-imx-irqsteer.c
> +++ b/drivers/irqchip/irq-imx-irqsteer.c
[ ... ]
> @@ -369,4 +371,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);

[Severity: High]
Does enabling module unload via module_platform_driver() expose a
use-after-free in imx_irqsteer_remove() due to missing synchronization?

When the module is unloaded, imx_irqsteer_remove() clears the chained
handler:

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

        irq_set_chained_handler_and_data(irqsteer_data->irq[i],
                                         NULL, NULL);
        irq_dispose_mapping(irqsteer_data->irq[i]);
    }

This teardown loop lacks a call to synchronize_irq(). If a parent
interrupt fires concurrently on another CPU just before the handler is
set to NULL, imx_irqsteer_irq_handler() will execute. Since there is no
synchronization, imx_irqsteer_remove() will return and trigger the
devres framework to free irqsteer_data while the handler is still
actively dereferencing it.

[Severity: Medium]
Will enabling module unload also trigger a clock underflow warning if
pm_runtime_resume_and_get() fails in imx_irqsteer_remove()?

During unload, imx_irqsteer_remove() calls:

    ret = pm_runtime_resume_and_get(&pdev->dev);

And later:

    if (ret >= 0)
        pm_runtime_put_noidle(&pdev->dev);

If pm_runtime_resume_and_get() fails (for example, due to a hardware
timeout), the device remains suspended and the clock's enable_count
remains 0. The function ignores this failure and allows devres to
unconditionally call clk_disable_unprepare() on the clock. Calling
clk_disable() on a clock with an enable_count of 0 triggers a kernel
stack trace.

> +
> +MODULE_DESCRIPTION("i.MX IRQSTEER interrupt multiplexer/remapper driver");
> +MODULE_LICENSE("GPL");

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

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] irqchip/imx-irqsteer: NULL pointer dereference in suspend due to early PM enable
--

commit 5322ea0be538e254fa9c813752002c35702c23cf
Author: Zhipeng Wang <zhipeng.wang_1@nxp.com>

irqchip/imx-irqsteer: Convert to devm_pm_runtime_set_active_enabled()

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 an
unbalanced pm_runtime_enable warning. This commit uses
devm_pm_runtime_set_active_enabled() to automatically disable runtime PM
on unbind, fixing the imbalance.

> diff --git a/drivers/irqchip/irq-imx-irqsteer.c b/drivers/irqchip/irq-imx-irqsteer.c
> index d05de3d26fb28..5acc04504e520 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;
> +

[Severity: High]
Does moving the PM runtime enablement before platform_set_drvdata() introduce
a race condition?

The PM runtime is now enabled early, and the IRQ domain is subsequently
created and exposed globally.

If a concurrent child device or userspace sysfs bind maps, requests, and
releases an IRQ from this domain before platform_set_drvdata() is reached,
releasing the IRQ invokes irq_bus_sync_unlock().

This in turn calls pm_runtime_put_autosuspend(), dropping the usage count to
zero and queuing a runtime suspend which invokes imx_irqsteer_suspend().

Since platform_set_drvdata() has not been called yet, will dev_get_drvdata()
return NULL in the suspend callback and cause a NULL pointer dereference when
passed to imx_irqsteer_save_regs()?

>  	data->domain = irq_domain_create_linear(dev_fwnode(&pdev->dev), data->reg_num * 32,
>  						&imx_irqsteer_domain_ops, data);

[ ... ]

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

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

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

[Frank Li]

On Thu, Oct 08, 2026 at 06:02:24PM +0900, Zhipeng.wang_1@oss.nxp.com 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>
> Signed-off-by: Zhipeng Wang <zhipeng.wang_1@nxp.com>
> ---

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

>  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 1b8d0c8eedb9..766756e76c47 100644
> --- a/drivers/irqchip/irq-imx-irqsteer.c
> +++ b/drivers/irqchip/irq-imx-irqsteer.c
> @@ -218,6 +218,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,
> @@ -246,11 +248,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.34.1
>
>

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

[Frank Li]

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

>  }
>
>  #ifdef CONFIG_PM
> --
> 2.34.1
>
>

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

[Frank Li]

On Thu, Oct 08, 2026 at 09:21:24AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] irqchip/imx-irqsteer: NULL pointer dereference in suspend due to early PM enable
> --
>
> commit 5322ea0be538e254fa9c813752002c35702c23cf
> Author: Zhipeng Wang <zhipeng.wang_1@nxp.com>
>
> irqchip/imx-irqsteer: Convert to devm_pm_runtime_set_active_enabled()
>
> 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 an
> unbalanced pm_runtime_enable warning. This commit uses
> devm_pm_runtime_set_active_enabled() to automatically disable runtime PM
> on unbind, fixing the imbalance.
>
> > diff --git a/drivers/irqchip/irq-imx-irqsteer.c b/drivers/irqchip/irq-imx-irqsteer.c
> > index d05de3d26fb28..5acc04504e520 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;
> > +
>
> [Severity: High]
> Does moving the PM runtime enablement before platform_set_drvdata() introduce
> a race condition?
>
> The PM runtime is now enabled early, and the IRQ domain is subsequently
> created and exposed globally.
>
> If a concurrent child device or userspace sysfs bind maps, requests, and
> releases an IRQ from this domain before platform_set_drvdata() is reached,
> releasing the IRQ invokes irq_bus_sync_unlock().
>
> This in turn calls pm_runtime_put_autosuspend(), dropping the usage count to
> zero and queuing a runtime suspend which invokes imx_irqsteer_suspend().
>
> Since platform_set_drvdata() has not been called yet, will dev_get_drvdata()
> return NULL in the suspend callback and cause a NULL pointer dereference when
> passed to imx_irqsteer_save_regs()?

Please fix this problem also

Frank


>
> >  	data->domain = irq_domain_create_linear(dev_fwnode(&pdev->dev), data->reg_num * 32,
> >  						&imx_irqsteer_domain_ops, data);
>
> [ ... ]
>
> > @@ -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;
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20261008090231.796511-1-Zhipeng.wang_1@oss.nxp.com?part=4

Re: [PATCH v6 6/9] irqchip/imx-irqsteer: Let devres own the clock

[Frank Li]

On Thu, Oct 08, 2026 at 06:02:28PM +0900, Zhipeng.wang_1@oss.nxp.com wrote:
> From: Zhipeng Wang <zhipeng.wang_1@nxp.com>
>
> Let the driver core own the clock lifetime so that the probe() error
> path and remove() do not have to hand-balance it:
>
>  - 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.
>
> Signed-off-by: Zhipeng Wang <zhipeng.wang_1@nxp.com>
> ---
>  drivers/irqchip/irq-imx-irqsteer.c | 37 +++++++++++++++---------------
>  1 file changed, 19 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/irqchip/irq-imx-irqsteer.c b/drivers/irqchip/irq-imx-irqsteer.c
> index fa233de9bc2b..15ee50f65d54 100644
> --- a/drivers/irqchip/irq-imx-irqsteer.c
> +++ b/drivers/irqchip/irq-imx-irqsteer.c
> @@ -194,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");
> @@ -229,12 +229,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);
> @@ -249,14 +243,13 @@ static int imx_irqsteer_probe(struct platform_device *pdev)
>
>  	ret = devm_pm_runtime_set_active_enabled(&pdev->dev);
>  	if (ret)
> -		goto out;
> +		return ret;
>
>  	data->domain = irq_domain_create_linear(dev_fwnode(&pdev->dev), data->reg_num * 32,
>  						&imx_irqsteer_domain_ops, data);
>  	if (!data->domain) {
>  		dev_err(&pdev->dev, "failed to create IRQ domain\n");
> -		ret = -ENOMEM;
> -		goto out;
> +		return -ENOMEM;
>  	}
>  	irq_domain_set_pm_device(data->domain, &pdev->dev);
>
> @@ -273,15 +266,23 @@ static int imx_irqsteer_probe(struct platform_device *pdev)
>  	platform_set_drvdata(pdev, data);
>
>  	return 0;
> -out:
> -	clk_disable_unprepare(data->ipg_clk);
> -	return ret;
>  }
>
>  static void imx_irqsteer_remove(struct platform_device *pdev)
>  {
>  	struct irqsteer_data *irqsteer_data = platform_get_drvdata(pdev);
> -	int i;
> +	int i, ret;
> +
> +	/*
> +	 * The device may be runtime-suspended here, in which case the runtime
> +	 * suspend callback has already dropped the clock enable count. Resume
> +	 * it so the devres clk_disable_unprepare(), which runs after remove(),
> +	 * finds the clock enabled and stays balanced. On success drop the
> +	 * usage count again with pm_runtime_put_noidle(): it must not trigger a
> +	 * suspend (which would re-disable the clock) and must not leak to the
> +	 * next probe of this persistent device.
> +	 */
> +	ret = pm_runtime_resume_and_get(&pdev->dev);

I think need new API/method to handle this case, which is quite common.
Anyways, this method should work. let as it now.

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

>
>  	for (i = 0; i < irqsteer_data->irq_count; i++) {
>  		if (!irqsteer_data->irq[i])
> @@ -294,8 +295,8 @@ static void imx_irqsteer_remove(struct platform_device *pdev)
>
>  	irq_domain_remove(irqsteer_data->domain);
>
> -	if (!pm_runtime_status_suspended(&pdev->dev))
> -		clk_disable_unprepare(irqsteer_data->ipg_clk);
> +	if (ret >= 0)
> +		pm_runtime_put_noidle(&pdev->dev);
>  }
>
>  #ifdef CONFIG_PM
> @@ -325,7 +326,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;
>  }
> @@ -335,7 +336,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;
> --
> 2.34.1
>
>

Re: [PATCH v6 5/9] irqchip/imx-irqsteer: Mask all interrupts in probe()

[Frank Li]

On Thu, Oct 08, 2026 at 09:18:11AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] irqchip/imx-irqsteer: missing wmb() allows interrupt storm race
> --
>
> commit e530007174d6dce33c6ec542e006f51e4a17dc73
> Author: Zhipeng Wang <zhipeng.wang_1@nxp.com>
>
> irqchip/imx-irqsteer: Mask all interrupts in probe()
>
> This commit masks all input interrupts in probe() before setting up
> chained handlers. This prevents an interrupt storm if a child interrupt
> was left unmasked during driver unload, as the hardware would otherwise
> assert it immediately upon re-mapping in the new domain.
>
> > diff --git a/drivers/irqchip/irq-imx-irqsteer.c b/drivers/irqchip/irq-imx-irqsteer.c
> > index 5acc04504e520..fa233de9bc2b2 100644
> > --- a/drivers/irqchip/irq-imx-irqsteer.c
> > +++ b/drivers/irqchip/irq-imx-irqsteer.c
>
> [ ... ]
>
> > @@ -239,6 +239,14 @@ 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 wiring up the chained handlers. CHANMASK
> > +	 * has inverted polarity (a set bit enables the interrupt), so writing
> > +	 * zero masks the source.
> > +	 */
> > +	for (i = 0; i < data->reg_num; i++)
> > +		writel_relaxed(0, data->regs + CHANMASK(i, data->reg_num));
> > +
>
> [Severity: Medium]
> Is an explicit memory barrier needed here to ensure the masks are applied
> before the parent interrupt is unmasked?
>
> Since writel_relaxed() does not provide a barrier, these writes to the IRQ
> Steer hardware could be reordered. The parent GIC interrupt may be unmasked
> before the IRQ Steer actually processes the interrupt masks, leaving a window
> for the interrupt storm this patch is designed to prevent.
>
> The parent GIC driver uses writel_relaxed() to unmask (for example,
> gic_unmask_irq() in GICv2 or gic_poke_irq() in GICv3). Furthermore,
> intervening locks like desc->lock in the IRQ core or irq_domain_mutex
> provide only ACQUIRE and RELEASE semantics. On ARM64, an intervening RELEASE
> followed by an ACQUIRE to different lock addresses can be reordered by the
> CPU.
>
> Could this allow the subsequent GIC relaxed write to cross the prior IRQ
> Steer relaxed write, meaning a full wmb() is strictly required to order
> writes to different I/O peripherals?

On our platform, all IO read/write is strict orderred. So no this problem

Frank
>
> >  	ret = devm_pm_runtime_set_active_enabled(&pdev->dev);
> >  	if (ret)
> >  		goto out;
>
> [ ... ]
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20261008090231.796511-1-Zhipeng.wang_1@oss.nxp.com?part=5

Re: [PATCH v6 9/9] irqchip/imx-irqsteer: Use devm_irq_domain_create_linear()

[Frank Li]

On Thu, Oct 08, 2026 at 06:02:31PM +0900, Zhipeng.wang_1@oss.nxp.com wrote:
> From: Zhipeng Wang <zhipeng.wang_1@nxp.com>
>
> The driver open-codes an irq_domain_info and calls
> devm_irq_domain_instantiate() only to get a linear revmap domain tied to

Miss match actual code change.

Frank

> the device lifetime. Use the new devm_irq_domain_create_linear() helper
> instead. No functional change.
>
> Signed-off-by: Zhipeng Wang <zhipeng.wang_1@nxp.com>
> ---
>  drivers/irqchip/irq-imx-irqsteer.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/irqchip/irq-imx-irqsteer.c b/drivers/irqchip/irq-imx-irqsteer.c
> index 85edd5e33ca9..f5601769d036 100644
> --- a/drivers/irqchip/irq-imx-irqsteer.c
> +++ b/drivers/irqchip/irq-imx-irqsteer.c
> @@ -246,8 +246,9 @@ static int imx_irqsteer_probe(struct platform_device *pdev)
>  	if (ret)
>  		return ret;
>
> -	data->domain = irq_domain_create_linear(dev_fwnode(&pdev->dev), data->reg_num * 32,
> -						&imx_irqsteer_domain_ops, data);
> +	data->domain = devm_irq_domain_create_linear(&pdev->dev, dev_fwnode(&pdev->dev),
> +						     data->reg_num * 32,
> +						     &imx_irqsteer_domain_ops, data);
>  	if (!data->domain) {
>  		dev_err(&pdev->dev, "failed to create IRQ domain\n");
>  		return -ENOMEM;
> @@ -294,8 +295,6 @@ static void imx_irqsteer_remove(struct platform_device *pdev)
>  		irq_dispose_mapping(irqsteer_data->irq[i]);
>  	}
>
> -	irq_domain_remove(irqsteer_data->domain);
> -
>  	if (ret >= 0)
>  		pm_runtime_put_noidle(&pdev->dev);
>  }
> --
> 2.34.1
>
>