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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Disabling the module clock at the end of `probe()` for non-console ports causes a kernel panic when the serial core queries the port status (e.g., via `/proc/tty/driver/serial`). The driver does not implement a `.pm` callback or use `pm_runtime`. Consequently, `uart_change_pm()` acts as a no-op, and `uport->ops->get_mctrl()` performs an unclocked MMIO read of `REG_STAT`.
- [High] A spurious interrupt arriving while the port is closed will cause a kernel panic due to the clock being disabled. The main IRQ is requested without `IRQF_NO_AUTOEN` (or similar mechanisms to mask it at the irqchip). If triggered, the ISR unconditionally accesses unclocked MMIO registers.
--

commit 2e7068bb1afe0ba0b9820a896f0d2535ea3517c9
Author: Rosen Penev <rosenp@gmail.com>

serial: mxs-auart: fix IRQ registration ordering and manage console clock

This commit moves the main UART IRQ registration after uart_add_one_port() to
ensure port states are initialized before the handler runs, and masks
interrupts to prevent spurious IRQs. It also disables the module clock for
non-console ports after probe, assuming startup will re-enable it on port
open.

> diff --git a/drivers/tty/serial/mxs-auart.c b/drivers/tty/serial/mxs-auart.c
> index 60a683a497405..35ba2ae740976 100644
> --- a/drivers/tty/serial/mxs-auart.c
> +++ b/drivers/tty/serial/mxs-auart.c
[ ... ]
> @@ -1627,9 +1623,28 @@ static int mxs_auart_probe(struct platform_device *pdev)
[ ... ]
> +	/*
> +	 * 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);

[Severity: High]
Does this leave us vulnerable to a crash if a spurious interrupt arrives
while the port is closed?

The main IRQ is requested without IRQF_NO_AUTOEN (or similar mechanisms), so
it remains unmasked at the interrupt controller. If an interrupt fires after
the clock is disabled below, mxs_auart_irq_handle() will perform an
unconditional unclocked MMIO read:

drivers/tty/serial/mxs-auart.c:mxs_auart_irq_handle() {
	...
	stat = mxs_read(s, REG_STAT);
	...
}

[ ... ]
> @@ -1641,10 +1656,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);

[Severity: High]
Will this cause a crash when the serial core queries the port status (e.g.,
when reading /proc/tty/driver/serial)?

The serial core invokes uart_line_info(), which calls uport->ops->get_mctrl()
on the port. Because this driver lacks a .pm callback and doesn't use
pm_runtime, the serial core's uart_change_pm() acts as a no-op and the clock
will remain disabled.

This results in mxs_auart_get_mctrl() executing an unclocked MMIO read:

drivers/tty/serial/mxs-auart.c:mxs_auart_get_mctrl() {
	...
	u32 stat = mxs_read(s, REG_STAT);
	...
}

>  
> -out_free_qpio_irq:
> -	auart_port[s->port.line] = NULL;
> +	return 0;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260603025857.287148-1-rosenp@gmail.com?part=3