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

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

On Fri, Aug 07, 2026 at 04:23:45PM +0900, Zhipeng.wang_1@oss.nxp.com wrote:
> From: Zhipeng Wang <zhipeng.wang_1@nxp.com>
>
> probe() sets up the chained handlers without first masking the input
> interrupts, and remove() leaves the CHANMASK registers untouched. For a
> built-in driver this happened to be harmless because CHANMASK resets to
> all-masked, but once the driver can be unloaded and reloaded a child
> interrupt left unmasked at unload time survives in hardware. On the next
> probe() the parent interrupts are re-mapped and unmasked before the new
> domain is ready, so a still-asserted line immediately storms the parent
> with no handler to service it.
>
> Mask all interrupts in probe() before wiring up the chained handlers, and
> again in remove() so the hardware is left quiesced for the next probe().
> Note CHANMASK uses inverted polarity (a set bit enables the interrupt), so
> masking means writing zero. This mirrors the sibling NXP chained mux
> irq-imx-intmux.c, which has masked all sources in both probe() and remove()
> since commit 2fbb13961e74 ("irqchip: Add NXP INTMUX interrupt multiplexer
> support").
>
> Signed-off-by: Zhipeng Wang <zhipeng.wang_1@nxp.com>
> ---
> Changes in v3:
>  - New patch. Masks all CHANMASK interrupts in probe() and remove() to
>    prevent the interrupt storm on module reload reported by Sashiko AI
>    on v2.
>
>  drivers/irqchip/irq-imx-irqsteer.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
>
> diff --git a/drivers/irqchip/irq-imx-irqsteer.c b/drivers/irqchip/irq-imx-irqsteer.c
> index 4a2fe8ba97f5..0c9c99f1141a 100644
> --- a/drivers/irqchip/irq-imx-irqsteer.c
> +++ b/drivers/irqchip/irq-imx-irqsteer.c
> @@ -236,6 +236,10 @@ static int imx_irqsteer_probe(struct platform_device *pdev)
>  	if (irqsteer_has_chanctrl(data->devtype_data))
>  		writel_relaxed(BIT(data->channel), data->regs + CHANCTRL);
>
> +	/* mask all interrupts before setting up the chained handlers */
> +	for (i = 0; i < data->reg_num; i++)
> +		writel_relaxed(0, data->regs + CHANMASK(i, data->reg_num));
> +
>  	data->domain = irq_domain_create_linear(dev_fwnode(&pdev->dev), data->reg_num * 32,
>  						&imx_irqsteer_domain_ops, data);
>  	if (!data->domain) {
> @@ -279,6 +283,11 @@ static void imx_irqsteer_remove(struct platform_device *pdev)
>  	struct irqsteer_data *irqsteer_data = platform_get_drvdata(pdev);
>  	int hwirq, i;
>
> +	/* mask all interrupts so a stale line cannot storm on the next probe */
> +	for (i = 0; i < irqsteer_data->reg_num; i++)
> +		writel_relaxed(0, irqsteer_data->regs +
> +			       CHANMASK(i, irqsteer_data->reg_num));
> +

You access register here, do you need call runtime pm get to enable clock
first?

Frank

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