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

[Frank Li]

On Tue, Jul 28, 2026 at 06:22:19PM +0900, Zhipeng.wang_1@xxxxxxxxxxx wrote:
> From: Jindong Yue <jindong.yue@xxxxxxx>
>
> 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().
>
> The existing remove() was incomplete for the module case, where the
> driver can now be unloaded and reloaded. Fix it up:
>
>  - dispose the parent IRQ mappings from irq_of_parse_and_map(), which
>    were leaked on unload;
>  - dispose the child mappings before irq_domain_remove(), otherwise the
>    irq_descs keep pointing at imx_irqsteer_irq_chip in the freed module,
>    which blows up on the next /proc/interrupts read;
>  - resume the device before clk_disable_unprepare(), since it may be
>    runtime suspended with the clock already off, and skip the disable if
>    the resume fails;
>  - disable runtime PM to balance the pm_runtime_enable() from probe().
>
> Signed-off-by: Jindong Yue <jindong.yue@xxxxxxx>
> Signed-off-by: Zhipeng Wang <zhipeng.wang_1@xxxxxxx>
> ---
> Changes in v2 (addressing Sashiko AI review of v1):
>  - remove(): dispose child and parent IRQ mappings before
>    irq_domain_remove(), fixing a use-after-free of imx_irqsteer_irq_chip
>    (in freed module memory) on the next /proc/interrupts read after
>    unload;
>  - remove(): disable runtime PM to balance the pm_runtime_enable() from
>    probe(), so reload no longer warns about unbalanced runtime PM; resume
>    the device first and skip clk_disable_unprepare() if the resume fails,
>    keeping the ipg clock balanced.
>
> v1: https://lore.kernel.org/r/20260724090136.3595894-1-Zhipeng.wang_1@xxxxxxxxxxx
>  drivers/irqchip/Kconfig            |  2 +-
>  drivers/irqchip/irq-imx-irqsteer.c | 26 +++++++++++++++++++++++---
>  2 files changed, 24 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
> index 42f2278a702d..07db3b678f94 100644
> --- a/drivers/irqchip/Kconfig
> +++ b/drivers/irqchip/Kconfig
> @@ -560,7 +560,7 @@ config CSKY_APB_INTC
>  	  the controller's register.
>
>  config IMX_IRQSTEER
> -	bool "i.MX IRQSTEER support"
> +	tristate "i.MX IRQSTEER support"
>  	depends on ARCH_MXC || ARCH_S32 || COMPILE_TEST
>  	default y if ARCH_MXC || ARCH_S32
>  	select IRQ_DOMAIN
> diff --git a/drivers/irqchip/irq-imx-irqsteer.c b/drivers/irqchip/irq-imx-irqsteer.c
> index 87b07f517be3..da3889036182 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>
> @@ -274,7 +275,7 @@ static int imx_irqsteer_probe(struct platform_device *pdev)
>  static void imx_irqsteer_remove(struct platform_device *pdev)
>  {
>  	struct irqsteer_data *irqsteer_data = platform_get_drvdata(pdev);
> -	int i;
> +	int hwirq, i, ret;
>
>  	for (i = 0; i < irqsteer_data->irq_count; i++) {
>  		if (!irqsteer_data->irq[i])
> @@ -282,11 +283,26 @@ static void imx_irqsteer_remove(struct platform_device *pdev)
>
>  		irq_set_chained_handler_and_data(irqsteer_data->irq[i],
>  						 NULL, NULL);
> +		irq_dispose_mapping(irqsteer_data->irq[i]);
>  	}
>
> +	for (hwirq = 0; hwirq < irqsteer_data->reg_num * 32; hwirq++)
> +		irq_dispose_mapping(irq_find_mapping(irqsteer_data->domain,
> +						     hwirq));
> +
>  	irq_domain_remove(irqsteer_data->domain);
>
> -	clk_disable_unprepare(irqsteer_data->ipg_clk);
> +	/*
> +	 * Device may be runtime suspended, with the ipg clock already gated.
> +	 * Resume it so the clock is balanced by the disable below; if resume
> +	 * fails the clock stays gated, so skip the disable in that case.
> +	 */
> +	ret = pm_runtime_get_sync(&pdev->dev);
> +	pm_runtime_disable(&pdev->dev);
> +	pm_runtime_put_noidle(&pdev->dev);
> +
> +	if (ret >= 0)
> +		clk_disable_unprepare(irqsteer_data->ipg_clk);

Is it okay by use devm_pm_rutime_set_acitve_enable() at probe, so you
needn't manage clock.

Frank

>  }
>
>  #ifdef CONFIG_PM
> @@ -349,6 +365,7 @@ static const struct of_device_id imx_irqsteer_dt_ids[] = {
>  	{ .compatible = "nxp,s32n79-irqsteer",	.data = &s32n79_data },
>  	{},
>  };
> +MODULE_DEVICE_TABLE(of, imx_irqsteer_dt_ids);
>
>  static struct platform_driver imx_irqsteer_driver = {
>  	.driver = {
> @@ -359,4 +376,7 @@ static struct platform_driver imx_irqsteer_driver = {
>  	.probe		= imx_irqsteer_probe,
>  	.remove		= imx_irqsteer_remove,
>  };
> -builtin_platform_driver(imx_irqsteer_driver);
> +module_platform_driver(imx_irqsteer_driver);
> +
> +MODULE_DESCRIPTION("i.MX IRQSTEER interrupt multiplexer/remapper driver");
> +MODULE_LICENSE("GPL");
> --
> 2.34.1
>
>