All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v7 00/10] irqchip/imx-irqsteer: Allow building as module
@ 2026-11-04  7:37 Zhipeng.wang_1
  2026-11-04  7:37 ` [PATCH v7 01/10] irqchip/imx-irqsteer: Call chained_irq_exit() on the handler error path Zhipeng.wang_1
                   ` (9 more replies)
  0 siblings, 10 replies; 14+ messages in thread
From: Zhipeng.wang_1 @ 2026-11-04  7:37 UTC (permalink / raw)
  To: Thomas Gleixner, Marc Zyngier, Frank Li
  Cc: Radu Rendec, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
	Fabio Estevam, Jindong Yue, xuegang.liu, linux-kernel, imx,
	linux-arm-kernel

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. Dispose of child IRQ mappings in remove()
 9. genirq/irqdomain: add devm_irq_domain_create_linear().
 10. Use devm_irq_domain_create_linear() in the driver.

Changes in v7:
 - Call platform_set_drvdata() before enabling runtime PM, so the runtime
   suspend callback always sees valid drvdata (Sashiko AI, Frank Li).
 - Fix the patch 9 commit message to describe the actual change
   (irq_domain_create_linear() -> devm_irq_domain_create_linear(),
   dropping irq_domain_remove() in remove()) (Frank Li).
 - Dispose of child IRQ mappings in remove() (Sashiko AI, Radu Rendec).

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

v6: https://lore.kernel.org/r/20261008090231.796511-1-Zhipeng.wang_1@oss.nxp.com
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 (8):
  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
  irqchip/imx-irqsteer: Dispose of child IRQ mappings in remove()
  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 | 80 ++++++++++++++++++------------
 include/linux/irqdomain.h          | 30 +++++++++++
 3 files changed, 80 insertions(+), 32 deletions(-)


base-commit: e74798f2f8e1562f2bbd2fed8d81929eeef39ffc
-- 
2.34.1



^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v7 01/10] irqchip/imx-irqsteer: Call chained_irq_exit() on the handler error path
  2026-11-04  7:37 [PATCH v7 00/10] irqchip/imx-irqsteer: Allow building as module Zhipeng.wang_1
@ 2026-11-04  7:37 ` Zhipeng.wang_1
  2026-11-04  7:37 ` [PATCH v7 02/10] irqchip/imx-irqsteer: Validate IRQ count before creating domain Zhipeng.wang_1
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 14+ messages in thread
From: Zhipeng.wang_1 @ 2026-11-04  7:37 UTC (permalink / raw)
  To: Thomas Gleixner, Marc Zyngier, Frank Li
  Cc: Radu Rendec, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
	Fabio Estevam, Jindong Yue, xuegang.liu, linux-kernel, imx,
	linux-arm-kernel

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



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v7 02/10] irqchip/imx-irqsteer: Validate IRQ count before creating domain
  2026-11-04  7:37 [PATCH v7 00/10] irqchip/imx-irqsteer: Allow building as module Zhipeng.wang_1
  2026-11-04  7:37 ` [PATCH v7 01/10] irqchip/imx-irqsteer: Call chained_irq_exit() on the handler error path Zhipeng.wang_1
@ 2026-11-04  7:37 ` Zhipeng.wang_1
  2026-11-04  7:37 ` [PATCH v7 03/10] irqchip/imx-irqsteer: Dispose of parent IRQ mappings in remove() Zhipeng.wang_1
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 14+ messages in thread
From: Zhipeng.wang_1 @ 2026-11-04  7:37 UTC (permalink / raw)
  To: Thomas Gleixner, Marc Zyngier, Frank Li
  Cc: Radu Rendec, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
	Fabio Estevam, Jindong Yue, xuegang.liu, linux-kernel, imx,
	linux-arm-kernel

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>
Reviewed-by: Radu Rendec <radu@rendec.net>
---
 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



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v7 03/10] irqchip/imx-irqsteer: Dispose of parent IRQ mappings in remove()
  2026-11-04  7:37 [PATCH v7 00/10] irqchip/imx-irqsteer: Allow building as module Zhipeng.wang_1
  2026-11-04  7:37 ` [PATCH v7 01/10] irqchip/imx-irqsteer: Call chained_irq_exit() on the handler error path Zhipeng.wang_1
  2026-11-04  7:37 ` [PATCH v7 02/10] irqchip/imx-irqsteer: Validate IRQ count before creating domain Zhipeng.wang_1
@ 2026-11-04  7:37 ` Zhipeng.wang_1
  2026-11-04  7:37 ` [PATCH v7 04/10] irqchip/imx-irqsteer: Convert to devm_pm_runtime_set_active_enabled() Zhipeng.wang_1
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 14+ messages in thread
From: Zhipeng.wang_1 @ 2026-11-04  7:37 UTC (permalink / raw)
  To: Thomas Gleixner, Marc Zyngier, Frank Li
  Cc: Radu Rendec, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
	Fabio Estevam, Jindong Yue, xuegang.liu, linux-kernel, imx,
	linux-arm-kernel

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



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v7 04/10] irqchip/imx-irqsteer: Convert to devm_pm_runtime_set_active_enabled()
  2026-11-04  7:37 [PATCH v7 00/10] irqchip/imx-irqsteer: Allow building as module Zhipeng.wang_1
                   ` (2 preceding siblings ...)
  2026-11-04  7:37 ` [PATCH v7 03/10] irqchip/imx-irqsteer: Dispose of parent IRQ mappings in remove() Zhipeng.wang_1
@ 2026-11-04  7:37 ` Zhipeng.wang_1
  2026-11-04  8:00   ` sashiko-bot
  2026-11-04  7:37 ` [PATCH v7 05/10] irqchip/imx-irqsteer: Mask all interrupts in probe() Zhipeng.wang_1
                   ` (5 subsequent siblings)
  9 siblings, 1 reply; 14+ messages in thread
From: Zhipeng.wang_1 @ 2026-11-04  7:37 UTC (permalink / raw)
  To: Thomas Gleixner, Marc Zyngier, Frank Li
  Cc: Radu Rendec, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
	Fabio Estevam, Jindong Yue, xuegang.liu, linux-kernel, imx,
	linux-arm-kernel

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>
Reviewed-by: Frank Li <Frank.Li@nxp.com>
---
 drivers/irqchip/irq-imx-irqsteer.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/irqchip/irq-imx-irqsteer.c b/drivers/irqchip/irq-imx-irqsteer.c
index d05de3d26fb2..a0546f11936d 100644
--- a/drivers/irqchip/irq-imx-irqsteer.c
+++ b/drivers/irqchip/irq-imx-irqsteer.c
@@ -239,6 +239,12 @@ static int imx_irqsteer_probe(struct platform_device *pdev)
 	if (irqsteer_has_chanctrl(data->devtype_data))
 		writel_relaxed(BIT(data->channel), data->regs + CHANCTRL);
 
+	platform_set_drvdata(pdev, data);
+
+	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) {
@@ -258,11 +264,6 @@ static int imx_irqsteer_probe(struct platform_device *pdev)
 						 data);
 	}
 
-	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



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v7 05/10] irqchip/imx-irqsteer: Mask all interrupts in probe()
  2026-11-04  7:37 [PATCH v7 00/10] irqchip/imx-irqsteer: Allow building as module Zhipeng.wang_1
                   ` (3 preceding siblings ...)
  2026-11-04  7:37 ` [PATCH v7 04/10] irqchip/imx-irqsteer: Convert to devm_pm_runtime_set_active_enabled() Zhipeng.wang_1
@ 2026-11-04  7:37 ` Zhipeng.wang_1
  2026-11-04  7:37 ` [PATCH v7 06/10] irqchip/imx-irqsteer: Let devres own the clock Zhipeng.wang_1
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 14+ messages in thread
From: Zhipeng.wang_1 @ 2026-11-04  7:37 UTC (permalink / raw)
  To: Thomas Gleixner, Marc Zyngier, Frank Li
  Cc: Radu Rendec, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
	Fabio Estevam, Jindong Yue, xuegang.liu, linux-kernel, imx,
	linux-arm-kernel

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 a0546f11936d..14fc56847a8c 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));
+
 	platform_set_drvdata(pdev, data);
 
 	ret = devm_pm_runtime_set_active_enabled(&pdev->dev);
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v7 06/10] irqchip/imx-irqsteer: Let devres own the clock
  2026-11-04  7:37 [PATCH v7 00/10] irqchip/imx-irqsteer: Allow building as module Zhipeng.wang_1
                   ` (4 preceding siblings ...)
  2026-11-04  7:37 ` [PATCH v7 05/10] irqchip/imx-irqsteer: Mask all interrupts in probe() Zhipeng.wang_1
@ 2026-11-04  7:37 ` Zhipeng.wang_1
  2026-11-04  8:03   ` sashiko-bot
  2026-11-04  7:37 ` [PATCH v7 07/10] irqchip/imx-irqsteer: Allow building as module Zhipeng.wang_1
                   ` (3 subsequent siblings)
  9 siblings, 1 reply; 14+ messages in thread
From: Zhipeng.wang_1 @ 2026-11-04  7:37 UTC (permalink / raw)
  To: Thomas Gleixner, Marc Zyngier, Frank Li
  Cc: Radu Rendec, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
	Fabio Estevam, Jindong Yue, xuegang.liu, linux-kernel, imx,
	linux-arm-kernel

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>
Reviewed-by: Frank Li <Frank.Li@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 14fc56847a8c..063a071ba163 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);
@@ -251,14 +245,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)
 	}
 
 	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



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v7 07/10] irqchip/imx-irqsteer: Allow building as module
  2026-11-04  7:37 [PATCH v7 00/10] irqchip/imx-irqsteer: Allow building as module Zhipeng.wang_1
                   ` (5 preceding siblings ...)
  2026-11-04  7:37 ` [PATCH v7 06/10] irqchip/imx-irqsteer: Let devres own the clock Zhipeng.wang_1
@ 2026-11-04  7:37 ` Zhipeng.wang_1
  2026-11-04  7:37 ` [PATCH v7 08/10] irqchip/imx-irqsteer: Dispose of child IRQ mappings in remove() Zhipeng.wang_1
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 14+ messages in thread
From: Zhipeng.wang_1 @ 2026-11-04  7:37 UTC (permalink / raw)
  To: Thomas Gleixner, Marc Zyngier, Frank Li
  Cc: Radu Rendec, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
	Fabio Estevam, Jindong Yue, xuegang.liu, linux-kernel, imx,
	linux-arm-kernel

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 063a071ba163..a14a8b80d60e 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



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v7 08/10] irqchip/imx-irqsteer: Dispose of child IRQ mappings in remove()
  2026-11-04  7:37 [PATCH v7 00/10] irqchip/imx-irqsteer: Allow building as module Zhipeng.wang_1
                   ` (6 preceding siblings ...)
  2026-11-04  7:37 ` [PATCH v7 07/10] irqchip/imx-irqsteer: Allow building as module Zhipeng.wang_1
@ 2026-11-04  7:37 ` Zhipeng.wang_1
  2026-11-04  7:37 ` [PATCH v7 09/10] genirq/irqdomain: Add devm_irq_domain_create_linear() Zhipeng.wang_1
  2026-11-04  7:37 ` [PATCH v7 10/10] irqchip/imx-irqsteer: Use devm_irq_domain_create_linear() Zhipeng.wang_1
  9 siblings, 0 replies; 14+ messages in thread
From: Zhipeng.wang_1 @ 2026-11-04  7:37 UTC (permalink / raw)
  To: Thomas Gleixner, Marc Zyngier, Frank Li
  Cc: Radu Rendec, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
	Fabio Estevam, Jindong Yue, xuegang.liu, linux-kernel, imx,
	linux-arm-kernel

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

Once the driver can be unloaded, the IRQ domain mappings handed out to
downstream consumers must be released on remove(), otherwise:

 - a parent interrupt racing with the teardown can run the chained
   handler while irqsteer_data is freed (no synchronize_irq());
 - downstream irq_descs keep dangling chip pointers into the unloaded
   module.

Dispose of every child hwirq mapping in remove(). irq_dispose_mapping()
calls irq_domain_disassociate(), which runs synchronize_irq() and frees
the descriptor, closing both races.

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

diff --git a/drivers/irqchip/irq-imx-irqsteer.c b/drivers/irqchip/irq-imx-irqsteer.c
index a14a8b80d60e..d15b806b278c 100644
--- a/drivers/irqchip/irq-imx-irqsteer.c
+++ b/drivers/irqchip/irq-imx-irqsteer.c
@@ -272,6 +272,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);
+	unsigned int hwirq;
 	int i, ret;
 
 	/*
@@ -294,6 +295,9 @@ static void imx_irqsteer_remove(struct platform_device *pdev)
 		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);
 
 	if (ret >= 0)
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v7 09/10] genirq/irqdomain: Add devm_irq_domain_create_linear()
  2026-11-04  7:37 [PATCH v7 00/10] irqchip/imx-irqsteer: Allow building as module Zhipeng.wang_1
                   ` (7 preceding siblings ...)
  2026-11-04  7:37 ` [PATCH v7 08/10] irqchip/imx-irqsteer: Dispose of child IRQ mappings in remove() Zhipeng.wang_1
@ 2026-11-04  7:37 ` Zhipeng.wang_1
  2026-11-04  7:37 ` [PATCH v7 10/10] irqchip/imx-irqsteer: Use devm_irq_domain_create_linear() Zhipeng.wang_1
  9 siblings, 0 replies; 14+ messages in thread
From: Zhipeng.wang_1 @ 2026-11-04  7:37 UTC (permalink / raw)
  To: Thomas Gleixner, Marc Zyngier, Frank Li
  Cc: Radu Rendec, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
	Fabio Estevam, Jindong Yue, xuegang.liu, linux-kernel, imx,
	linux-arm-kernel

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>
Reviewed-by: Radu Rendec <radu@rendec.net>
---
 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



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v7 10/10] irqchip/imx-irqsteer: Use devm_irq_domain_create_linear()
  2026-11-04  7:37 [PATCH v7 00/10] irqchip/imx-irqsteer: Allow building as module Zhipeng.wang_1
                   ` (8 preceding siblings ...)
  2026-11-04  7:37 ` [PATCH v7 09/10] genirq/irqdomain: Add devm_irq_domain_create_linear() Zhipeng.wang_1
@ 2026-11-04  7:37 ` Zhipeng.wang_1
  9 siblings, 0 replies; 14+ messages in thread
From: Zhipeng.wang_1 @ 2026-11-04  7:37 UTC (permalink / raw)
  To: Thomas Gleixner, Marc Zyngier, Frank Li
  Cc: Radu Rendec, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
	Fabio Estevam, Jindong Yue, xuegang.liu, linux-kernel, imx,
	linux-arm-kernel

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

Replace irq_domain_create_linear() with the new
devm_irq_domain_create_linear() helper, so the IRQ domain is released
automatically on unbind and remove() no longer has to call
irq_domain_remove(). 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 d15b806b278c..da6e8d1bbec1 100644
--- a/drivers/irqchip/irq-imx-irqsteer.c
+++ b/drivers/irqchip/irq-imx-irqsteer.c
@@ -248,8 +248,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;
@@ -298,8 +299,6 @@ static void imx_irqsteer_remove(struct platform_device *pdev)
 	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);
-
 	if (ret >= 0)
 		pm_runtime_put_noidle(&pdev->dev);
 }
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH v7 04/10] irqchip/imx-irqsteer: Convert to devm_pm_runtime_set_active_enabled()
  2026-11-04  7:37 ` [PATCH v7 04/10] irqchip/imx-irqsteer: Convert to devm_pm_runtime_set_active_enabled() Zhipeng.wang_1
@ 2026-11-04  8:00   ` sashiko-bot
  2026-11-04 20:00     ` Frank Li
  0 siblings, 1 reply; 14+ messages in thread
From: sashiko-bot @ 2026-11-04  8:00 UTC (permalink / raw)
  To: Zhipeng.wang_1; +Cc: imx, Frank.Li

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] irqchip/imx-irqsteer: unclocked MMIO and double clk disable on probe error
- [High] irqchip/imx-irqsteer: clk_disable_unprepare() while runtime PM stays active
--

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

irqchip/imx-irqsteer: Convert to devm_pm_runtime_set_active_enabled()

This commit replaces manual pm_runtime_set_active() and pm_runtime_enable()
calls in imx_irqsteer_probe() with devm_pm_runtime_set_active_enabled(), and
guards clk_disable_unprepare() in imx_irqsteer_remove() with a runtime PM
status check.

> diff --git a/drivers/irqchip/irq-imx-irqsteer.c b/drivers/irqchip/irq-imx-irqsteer.c
> index d05de3d26fb28..a0546f11936d8 100644
> --- a/drivers/irqchip/irq-imx-irqsteer.c
> +++ b/drivers/irqchip/irq-imx-irqsteer.c
> @@ -239,6 +239,12 @@ static int imx_irqsteer_probe(struct platform_device *pdev)
>  	if (irqsteer_has_chanctrl(data->devtype_data))
>  		writel_relaxed(BIT(data->channel), data->regs + CHANCTRL);
>  
> +	platform_set_drvdata(pdev, data);
> +
> +	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) {
> @@ -258,11 +264,6 @@ static int imx_irqsteer_probe(struct platform_device *pdev)
>  						 data);
>  	}
>  
> -	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);

[Severity: High]
If irq_domain_create_linear() fails in imx_irqsteer_probe() in
drivers/irqchip/irq-imx-irqsteer.c after devm_pm_runtime_set_active_enabled()
has succeeded, can this cause an unclocked MMIO access and a double
clk_disable_unprepare() on data->ipg_clk?

When irq_domain_create_linear() returns NULL, imx_irqsteer_probe() jumps to
the out label and calls clk_disable_unprepare(data->ipg_clk) while runtime PM
is still enabled, runtime_status is RPM_ACTIVE, and usage_count is 0.

When imx_irqsteer_probe() then returns an error, device_unbind_cleanup() ->
devres_release_all() executes pm_runtime_disable_action() in
drivers/base/power/runtime.c before clearing drvdata:

static void pm_runtime_disable_action(void *data)
{
	pm_runtime_dont_use_autosuspend(data);
	pm_runtime_disable(data);
}

Because pm_runtime_dont_use_autosuspend() is called while runtime PM is still
enabled, __pm_runtime_use_autosuspend() -> update_autosuspend() ->
rpm_idle(dev, RPM_AUTO) -> rpm_suspend(dev, RPM_AUTO) runs synchronously and
invokes imx_irqsteer_suspend():

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

	return 0;
}

imx_irqsteer_save_regs() reads the CHANMASK MMIO registers while ipg_clk is
already gated, and imx_irqsteer_suspend() then calls clk_disable_unprepare()
on ipg_clk a second time. Similarly, if devm_add_action_or_reset() inside
devm_pm_runtime_enable() fails, it immediately runs
pm_runtime_disable_action() -> imx_irqsteer_suspend() and then the out path
disables ipg_clk a second time.

Note that later commits in this series ("irqchip/imx-irqsteer: Let devres own
the clock" and "irqchip/imx-irqsteer: Use devm_irq_domain_create_linear()")
do not fully avoid this on probe failure: if probe fails after
devm_pm_runtime_set_active_enabled(), devres_release_all() still runs
pm_runtime_disable_action() -> imx_irqsteer_suspend() -> clk_disable(ipg_clk)
before devm_clk_release() calls clk_disable_unprepare(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);
>  }

[Severity: High]
As reported by Frank Li on v6 [1], does calling clk_disable_unprepare() in
imx_irqsteer_remove() in drivers/irqchip/irq-imx-irqsteer.c while runtime PM
is still enabled and RPM_ACTIVE cause imx_irqsteer_suspend() to read
unclocked MMIO registers and disable ipg_clk a second time during devres
teardown?

If imx_irqsteer_remove() runs while runtime_status is RPM_ACTIVE and
usage_count is 0 (for example under CONFIG_DEBUG_TEST_DRIVER_REMOVE or on post-probe failure paths in really_probe()), !pm_runtime_status_suspended(&pdev->dev) is true and imx_irqsteer_remove() calls clk_disable_unprepare(irqsteer_data->ipg_clk) without disabling runtime PM or updating the runtime status.

Immediately afterwards, device_unbind_cleanup() -> devres_release_all() runs
pm_runtime_disable_action() -> pm_runtime_dont_use_autosuspend() ->
update_autosuspend() -> rpm_idle() -> rpm_suspend(), which synchronously
calls imx_irqsteer_suspend(). That reads the CHANMASK registers via
imx_irqsteer_save_regs() with ipg_clk already disabled and unprepared, and
calls clk_disable_unprepare(irqsteer_data->ipg_clk) a second time.

In addition, in the follow-up commit "irqchip/imx-irqsteer: Let devres own
the clock", imx_irqsteer_remove() calls pm_runtime_resume_and_get(&pdev->dev)
followed by pm_runtime_put_noidle(&pdev->dev), which leaves the device in
RPM_ACTIVE with usage_count == 0 on every normal unbind and triggers the same
synchronous imx_irqsteer_suspend() -> clk_disable() before devm_clk_release()
runs clk_disable_unprepare().

[1] https://lore.kernel.org/r/asf_9lgB-ULFooH9@SMW015318

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v7 06/10] irqchip/imx-irqsteer: Let devres own the clock
  2026-11-04  7:37 ` [PATCH v7 06/10] irqchip/imx-irqsteer: Let devres own the clock Zhipeng.wang_1
@ 2026-11-04  8:03   ` sashiko-bot
  0 siblings, 0 replies; 14+ messages in thread
From: sashiko-bot @ 2026-11-04  8:03 UTC (permalink / raw)
  To: Zhipeng.wang_1; +Cc: imx, Frank.Li

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] irqchip/imx-irqsteer: clk_disable() in suspend breaks power domain and SCMI clk
- [Medium] irqchip/imx-irqsteer: clock disable underflow if resume fails in remove()
--

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

irqchip/imx-irqsteer: Let devres own the clock

This commit switches the imx-irqsteer driver to acquire ipg_clk using
devm_clk_get_enabled() so that devres manages the clock's prepare and enable
state across probe and unbind. It also updates the runtime PM callbacks to
use clk_disable() and clk_enable() and resumes the device in remove() before
devres releases the clock.

> diff --git a/drivers/irqchip/irq-imx-irqsteer.c b/drivers/irqchip/irq-imx-irqsteer.c
> index 14fc56847a8c1..063a071ba1630 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");

[ ... ]

> @@ -273,15 +266,23 @@ static int imx_irqsteer_probe(struct platform_device *pdev)
>  	}
>  
>  	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]
As reported by Sashiko on v6 [1], what happens in imx_irqsteer_remove() if
the device is runtime-suspended and pm_runtime_resume_and_get() returns a
negative error (for example due to a power domain or supplier resume failure,
or clk_enable() failing in imx_irqsteer_resume())?

When the device is runtime-suspended, imx_irqsteer_suspend() has already
called clk_disable(irqsteer_data->ipg_clk). If pm_runtime_resume_and_get()
fails, ipg_clk remains disabled when imx_irqsteer_remove() returns, but the
devres action registered by devm_clk_get_enabled() in imx_irqsteer_probe()
still runs unconditionally during device_unbind_cleanup() and calls
clk_disable_unprepare() on ipg_clk.

Wouldn't that underflow the clock enable count and trigger a warning in
clk_core_disable(), or prematurely gate ipg_clk if it is shared with another
active consumer?

>  #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);

[Severity: Medium]
Does replacing clk_disable_unprepare() and clk_prepare_enable() with
clk_disable() and clk_enable() in imx_irqsteer_suspend() and
imx_irqsteer_resume() cause power management regressions on i.MX8, i.MX94,
and S32N79 platforms?

Because imx_irqsteer_probe() acquires ipg_clk via devm_clk_get_enabled(),
only calling clk_disable() in imx_irqsteer_suspend() leaves core->prepare_count
at 1 while the device is runtime-suspended or in system sleep.

In drivers/clk/clk.c, clk_core_unprepare() only drops the runtime PM
reference on a runtime-PM-enabled clock provider device when prepare_count
reaches 0:

drivers/clk/clk.c:clk_core_unprepare() {
	...
	trace_clk_unprepare_complete(core);
	clk_core_unprepare(core->parent);
	clk_pm_runtime_put(core);
}

On i.MX8QM and i.MX8QXP (such as irqsteer_lvds1 in
arch/arm64/boot/dts/freescale/imx8-ss-lvds1.dtsi, as well as the LVDS0,
MIPI0, and MIPI1 subsystems), the irqsteer node and its fsl,imx8qxp-lpcg
clock controller share the same SCU power domain:

	clocks = <&lvds1_lis_lpcg IMX_LPCG_CLK_4>;
	clock-names = "ipg";
	power-domains = <&pd IMX_SC_R_LVDS_1>;

Since prepare_count never drops to 0 during runtime suspend,
clk_pm_runtime_put() is never called on the LPCG clock provider (or its
parent imx-scu-clk device), keeping the clock provider runtime-active and
preventing the shared power domain from ever powering off.

Additionally, on platforms where ipg_clk is provided by a non-atomic clock
controller—such as i.MX94 (where a55_irqsteer uses an SCMI clock over a
non-atomic mailbox transport), S32N79 (when atomic SCMI SMC transport is not
enabled), and i.MX8 SCU clocks—the clock provider only populates .prepare and
.unprepare and leaves .enable and .disable NULL:

drivers/clk/clk-scmi.c:scmi_clk_ops_alloc() {
	if (feats_key & BIT(SCMI_CLK_STATE_CTRL_SUPPORTED)) {
		if (feats_key & BIT(SCMI_CLK_ATOMIC_SUPPORTED)) {
			ops->enable = scmi_clk_atomic_enable;
			ops->disable = scmi_clk_atomic_disable;
		} else {
			ops->prepare = scmi_clk_enable;
			ops->unprepare = scmi_clk_disable;
		}
	}
}

On those platforms, calling clk_disable() and clk_enable() in
imx_irqsteer_suspend() and imx_irqsteer_resume() becomes a no-op and never
gates ipg_clk during runtime suspend or system sleep.

Since imx_irqsteer_suspend() and imx_irqsteer_resume() run in sleepable
context, should they keep calling clk_disable_unprepare() and
clk_prepare_enable()?

[1] https://lore.kernel.org/r/20261008090231.796511-7-Zhipeng.wang_1@oss.nxp.com

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v7 04/10] irqchip/imx-irqsteer: Convert to devm_pm_runtime_set_active_enabled()
  2026-11-04  8:00   ` sashiko-bot
@ 2026-11-04 20:00     ` Frank Li
  0 siblings, 0 replies; 14+ messages in thread
From: Frank Li @ 2026-11-04 20:00 UTC (permalink / raw)
  To: sashiko-reviews; +Cc: Zhipeng.wang_1, imx, Frank.Li

On Wed, Nov 04, 2026 at 08:00:33AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] irqchip/imx-irqsteer: unclocked MMIO and double clk disable on probe error
> - [High] irqchip/imx-irqsteer: clk_disable_unprepare() while runtime PM stays active
> --

Please check this

Frank


^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2026-11-04 20:00 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-11-04  7:37 [PATCH v7 00/10] irqchip/imx-irqsteer: Allow building as module Zhipeng.wang_1
2026-11-04  7:37 ` [PATCH v7 01/10] irqchip/imx-irqsteer: Call chained_irq_exit() on the handler error path Zhipeng.wang_1
2026-11-04  7:37 ` [PATCH v7 02/10] irqchip/imx-irqsteer: Validate IRQ count before creating domain Zhipeng.wang_1
2026-11-04  7:37 ` [PATCH v7 03/10] irqchip/imx-irqsteer: Dispose of parent IRQ mappings in remove() Zhipeng.wang_1
2026-11-04  7:37 ` [PATCH v7 04/10] irqchip/imx-irqsteer: Convert to devm_pm_runtime_set_active_enabled() Zhipeng.wang_1
2026-11-04  8:00   ` sashiko-bot
2026-11-04 20:00     ` Frank Li
2026-11-04  7:37 ` [PATCH v7 05/10] irqchip/imx-irqsteer: Mask all interrupts in probe() Zhipeng.wang_1
2026-11-04  7:37 ` [PATCH v7 06/10] irqchip/imx-irqsteer: Let devres own the clock Zhipeng.wang_1
2026-11-04  8:03   ` sashiko-bot
2026-11-04  7:37 ` [PATCH v7 07/10] irqchip/imx-irqsteer: Allow building as module Zhipeng.wang_1
2026-11-04  7:37 ` [PATCH v7 08/10] irqchip/imx-irqsteer: Dispose of child IRQ mappings in remove() Zhipeng.wang_1
2026-11-04  7:37 ` [PATCH v7 09/10] genirq/irqdomain: Add devm_irq_domain_create_linear() Zhipeng.wang_1
2026-11-04  7:37 ` [PATCH v7 10/10] irqchip/imx-irqsteer: Use devm_irq_domain_create_linear() Zhipeng.wang_1

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.