Re: [PATCH] serial: 8250: Add Armada 38x earlycon support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Runtime PM circular dependency during port registration in `dw8250_probe()`.
--

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

serial: 8250: Add Armada 38x earlycon support

This commit registers the Armada 38x UART compatible with the built-in 8250
earlycon implementation. This allows stdout-path to enable an early console
while preserving the vendor-specific compatible to retain DesignWare driver
runtime handling.

> diff --git a/drivers/tty/serial/8250/8250_early.c b/drivers/tty/serial/8250/8250_early.c
> index 44ec209f37c47..265d49850e150 100644
> --- a/drivers/tty/serial/8250/8250_early.c
> +++ b/drivers/tty/serial/8250/8250_early.c
> @@ -175,6 +175,7 @@ EARLYCON_DECLARE(uart8250, early_serial8250_setup);
>  EARLYCON_DECLARE(uart, early_serial8250_setup);
>  OF_EARLYCON_DECLARE(ns16550, "ns16550", early_serial8250_setup);
>  OF_EARLYCON_DECLARE(ns16550a, "ns16550a", early_serial8250_setup);
> +OF_EARLYCON_DECLARE(uart, "marvell,armada-38x-uart", early_serial8250_setup);

[Severity: High]
This is a pre-existing issue, but since this patch relies on preserving the
DesignWare driver's runtime handling, could the current runtime PM logic
cause a circular dependency during port registration?

Looking at dw8250_probe() in drivers/tty/serial/8250/8250_dw.c, port
registration happens before runtime PM is enabled:

drivers/tty/serial/8250/8250_dw.c:dw8250_probe() {
	...
	data->data.line = serial8250_register_8250_port(up);
	if (data->data.line < 0)
		return data->data.line;

	platform_set_drvdata(pdev, data);

	pm_runtime_enable(dev);
	...
}

The registration call eventually reaches uart_add_one_port(), which
synchronously invokes the pm callback dw8250_do_pm(). This callback
attempts to use runtime PM APIs:

drivers/tty/serial/8250/8250_dw.c:dw8250_do_pm() {
	if (!state)
		pm_runtime_get_sync(port->dev);

	serial8250_do_pm(port, state, old);

	if (state)
		pm_runtime_put_sync_suspend(port->dev);
}

Since pm_runtime_enable() hasn't been called yet when the callback runs,
would pm_runtime_get_sync() operate on an uninitialized device? Does this
sequence risk blocking tasks and hanging worker threads during device probe?

>  OF_EARLYCON_DECLARE(uart, "nvidia,tegra20-uart", early_serial8250_setup);
>  OF_EARLYCON_DECLARE(uart, "snps,dw-apb-uart", early_serial8250_setup);

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