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

[Frank Li <Frank.li@oss.nxp.com>]

On Wed, Aug 19, 2026 at 06:05:43PM +0900, Zhipeng.wang_1@oss.nxp.com wrote:
> From: Jindong Yue <jindong.yue@nxp.com>
>
> Make the driver buildable as a module by turning the Kconfig symbol into
> a tristate and using module_platform_driver() instead of
> builtin_platform_driver().
>
> Now that the driver can be unloaded and reloaded, let the driver core
> own the clock and runtime PM lifetime so that the probe() error path and
> remove() do not have to hand-balance them:
>
>  - acquire the clock with devm_clk_get_enabled() instead of a bare
>    devm_clk_get() followed by a manual clk_prepare_enable(), so it is
>    prepared/enabled for the device lifetime and released on unbind;
>  - keep only clk_enable()/clk_disable() in the runtime PM callbacks,
>    since prepare/unprepare is now handled once by devres;

devm_clk_get_enable() will do prepare() and enable(). tear down also do
unprepare() and disable()

To avoid both devm_clk() and runtime pm suspend both unprepare and
disable() clock to make wrong clock refererence.

need in remove function

    /*
     * Resume the device so runtime_resume() re-enables the clock.
     * devm cleanup (clk_disable_unprepare) runs after .remove() returns,
     * so the clock will be enabled and the disable is safe.
     */
    pm_runtime_resume_and_get(dev);

    /* devm_clk_get_enable and devm_pm_runtime_enable clean up automatically */

Frank

>  - enable runtime PM with devm_pm_runtime_set_active_enabled(), which
>    marks the device active (matching the enabled clock) and disables
>    runtime PM on unbind.
>
> With the clock, runtime PM and IRQ domain all owned by devres, remove()
> and the probe() error path only have to dispose of the parent IRQ
> mappings.
>
> Signed-off-by: Jindong Yue <jindong.yue@nxp.com>
> Signed-off-by: Zhipeng Wang <zhipeng.wang_1@nxp.com>
> ---
>  drivers/irqchip/Kconfig            |  2 +-
>  drivers/irqchip/irq-imx-irqsteer.c | 36 +++++++++++++++++-------------
>  2 files changed, 22 insertions(+), 16 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 a9909ecb6fef..a60cc527e619 100644
> --- a/drivers/irqchip/irq-imx-irqsteer.c
> +++ b/drivers/irqchip/irq-imx-irqsteer.c
> @@ -10,6 +10,7 @@
>  #include <linux/irqchip/chained_irq.h>
>  #include <linux/irqdomain.h>
>  #include <linux/kernel.h>
> +#include <linux/module.h>
>  #include <linux/of.h>
>  #include <linux/of_irq.h>
>  #include <linux/platform_device.h>
> @@ -193,7 +194,7 @@ static int imx_irqsteer_probe(struct platform_device *pdev)
>  		return PTR_ERR(data->regs);
>  	}
>
> -	data->ipg_clk = devm_clk_get(&pdev->dev, "ipg");
> +	data->ipg_clk = devm_clk_get_enabled(&pdev->dev, "ipg");
>  	if (IS_ERR(data->ipg_clk))
>  		return dev_err_probe(&pdev->dev, PTR_ERR(data->ipg_clk),
>  				     "failed to get ipg clk\n");
> @@ -226,12 +227,6 @@ static int imx_irqsteer_probe(struct platform_device *pdev)
>  			return -ENOMEM;
>  	}
>
> -	ret = clk_prepare_enable(data->ipg_clk);
> -	if (ret) {
> -		dev_err(&pdev->dev, "failed to enable ipg clk: %d\n", ret);
> -		return ret;
> -	}
> -
>  	/* steer all IRQs into configured channel */
>  	if (irqsteer_has_chanctrl(data->devtype_data))
>  		writel_relaxed(BIT(data->channel), data->regs + CHANCTRL);
> @@ -271,12 +266,21 @@ static int imx_irqsteer_probe(struct platform_device *pdev)
>
>  	platform_set_drvdata(pdev, data);
>
> -	pm_runtime_set_active(&pdev->dev);
> -	pm_runtime_enable(&pdev->dev);
> +	ret = devm_pm_runtime_set_active_enabled(&pdev->dev);
> +	if (ret)
> +		goto err_irq;
>
>  	return 0;
> +
> +err_irq:
> +	for (i = 0; i < data->irq_count; i++) {
> +		if (!data->irq[i])
> +			break;
> +
> +		irq_set_chained_handler_and_data(data->irq[i], NULL, NULL);
> +		irq_dispose_mapping(data->irq[i]);
> +	}
>  out:
> -	clk_disable_unprepare(data->ipg_clk);
>  	return ret;
>  }
>
> @@ -293,8 +297,6 @@ static void imx_irqsteer_remove(struct platform_device *pdev)
>  						 NULL, NULL);
>  		irq_dispose_mapping(irqsteer_data->irq[i]);
>  	}
> -
> -	clk_disable_unprepare(irqsteer_data->ipg_clk);
>  }
>
>  #ifdef CONFIG_PM
> @@ -324,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;
>  }
> @@ -334,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;
> @@ -357,6 +359,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 = {
> @@ -367,4 +370,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
>
>