Re: [PATCHv2 4/4] serial: mxs-auart: fix IRQ registration ordering and manage console clock

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

On Tue, Jun 09, 2026 at 03:37:17PM -0700, Rosen Penev wrote:
> Move the main UART IRQ registration after uart_add_one_port so that
> s->port.state and s->port.lock are initialized before the interrupt
> handler can run. Mask all UART interrupts before adding the port to
> prevent spurious IRQs left by the bootloader.

Please use seperate patch to this problem only.

Frank

>
> After probe succeeds, disable the module clock for non-console ports
> since startup will re-enable it on port open. For console ports, keep
> the clock prepared so auart_console_write() can safely call
> clk_enable() from atomic context.
>
> Guard the IRQ handler and get_mctrl with clk_enable/clk_disable since
> GPIO IRQs and serial-core status queries can fire while the clock is
> disabled for non-console ports.
>
> In remove, disable the clock for console ports to balance the enable
> done in probe, preventing a clock leak on unbind.
>
> Assisted-by: opencode:big-pickle
> ---
>  drivers/tty/serial/mxs-auart.c | 49 +++++++++++++++++++++++++++-------
>  1 file changed, 39 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/tty/serial/mxs-auart.c b/drivers/tty/serial/mxs-auart.c
> index 4499e3206e85..e2b656638ab3 100644
> --- a/drivers/tty/serial/mxs-auart.c
> +++ b/drivers/tty/serial/mxs-auart.c
> @@ -738,9 +738,13 @@ static u32 mxs_auart_modem_status(struct mxs_auart_port *s, u32 mctrl)
>  static u32 mxs_auart_get_mctrl(struct uart_port *u)
>  {
>  	struct mxs_auart_port *s = to_auart_port(u);
> -	u32 stat = mxs_read(s, REG_STAT);
> +	u32 stat;
>  	u32 mctrl = 0;
>
> +	clk_enable(s->clk);
> +	stat = mxs_read(s, REG_STAT);
> +	clk_disable(s->clk);
> +
>  	if (stat & AUART_STAT_CTS)
>  		mctrl |= TIOCM_CTS;
>
> @@ -1079,6 +1083,7 @@ static irqreturn_t mxs_auart_irq_handle(int irq, void *context)
>  	struct mxs_auart_port *s = context;
>  	u32 mctrl_temp = s->mctrl_prev;
>
> +	clk_enable(s->clk);
>  	uart_port_lock(&s->port);
>
>  	stat = mxs_read(s, REG_STAT);
> @@ -1118,6 +1123,7 @@ static irqreturn_t mxs_auart_irq_handle(int irq, void *context)
>  	}
>
>  	uart_port_unlock(&s->port);
> +	clk_disable(s->clk);
>
>  	return IRQ_HANDLED;
>  }
> @@ -1603,10 +1609,6 @@ static int mxs_auart_probe(struct platform_device *pdev)
>  	}
>
>  	s->port.irq = irq;
> -	ret = devm_request_irq(&pdev->dev, irq, mxs_auart_irq_handle, 0,
> -			       dev_name(&pdev->dev), s);
> -	if (ret)
> -		goto out_disable_clk;
>
>  	platform_set_drvdata(pdev, s);
>
> @@ -1627,9 +1629,28 @@ static int mxs_auart_probe(struct platform_device *pdev)
>
>  	mxs_auart_reset_deassert(s);
>
> +	/* Mask all UART interrupts to prevent spurious IRQs from bootloader */
> +	mxs_write(0, s, REG_INTR);
> +
>  	ret = uart_add_one_port(&auart_driver, &s->port);
> -	if (ret)
> -		goto out_free_qpio_irq;
> +	if (ret) {
> +		auart_port[s->port.line] = NULL;
> +		goto out_disable_clk;
> +	}
> +
> +	/*
> +	 * Request the main IRQ after uart_add_one_port so that
> +	 * s->port.state and s->port.lock are initialized before
> +	 * the handler can run in response to a bootloader-left
> +	 * interrupt.
> +	 */
> +	ret = devm_request_irq(&pdev->dev, irq, mxs_auart_irq_handle, 0,
> +			       dev_name(&pdev->dev), s);
> +	if (ret) {
> +		uart_remove_one_port(&auart_driver, &s->port);
> +		auart_port[s->port.line] = NULL;
> +		goto out_disable_clk;
> +	}
>
>  	/* ASM9260 don't have version reg */
>  	if (is_asm9260_auart(s)) {
> @@ -1641,10 +1662,16 @@ static int mxs_auart_probe(struct platform_device *pdev)
>  			 (version >> 16) & 0xff, version & 0xffff);
>  	}
>
> -	return 0;
> +	/*
> +	 * Disable clock -- startup will re-enable when the port is opened.
> +	 * For the console port the clock must stay prepared so that
> +	 * auart_console_write() can safely call clk_enable() from
> +	 * atomic context.
> +	 */
> +	if (!uart_console(&s->port))
> +		clk_disable_unprepare(s->clk);
>
> -out_free_qpio_irq:
> -	auart_port[s->port.line] = NULL;
> +	return 0;
>
>  out_disable_clk:
>  	clk_disable_unprepare(s->clk);
> @@ -1657,6 +1684,8 @@ static void mxs_auart_remove(struct platform_device *pdev)
>
>  	uart_remove_one_port(&auart_driver, &s->port);
>  	auart_port[s->port.line] = NULL;
> +	if (uart_console(&s->port))
> +		clk_disable_unprepare(s->clk);
>  }
>
>  static struct platform_driver mxs_auart_driver = {
> --
> 2.54.0
>
>