Re: [PATCH 7/9] arm64: dts: airoha: en7581: add nodes for all supported uart and hsuart

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] arm64: dts: airoha: undocumented UART compatibles break serial console
- [Medium] arm64: dts: airoha: uart1 retains ns16550 workaround clock-frequency
--

commit 6e4317a9ed5754fa0011125ce3517c077746d85a
Author: Christian Marangi <ansuelsmth@gmail.com>

arm64: dts: airoha: en7581: add nodes for all supported uart and hsuart

This commit adds Device Tree nodes for the HSUART and additional UART
controllers on the Airoha EN7581 SoC, and updates the compatible string
on the existing uart1 node from "ns16550" to "airoha,en7523-uart".

> diff --git a/arch/arm64/boot/dts/airoha/en7581.dtsi b/arch/arm64/boot/dts/airoha/en7581.dtsi
> index 760f0c3e60904..ee1633f01068e 100644
> --- a/arch/arm64/boot/dts/airoha/en7581.dtsi
> +++ b/arch/arm64/boot/dts/airoha/en7581.dtsi
> @@ -470,13 +470,24 @@ fixed-link {
>  			};
>  		};
>  
> +		serial@1fbe1000 {
> +			compatible = "airoha,an7581-hsuart";
> +			reg = <0x0 0x1fbe1000 0x0 0x40>;
> +			reg-io-width = <4>;
> +			reg-shift = <2>;
> +			interrupts = <GIC_SPI 54 IRQ_TYPE_LEVEL_HIGH>;
> +			clock-frequency = <7372800>;
> +
> +			status = "disabled";
> +		};
> +
>  		pbus_csr: syscon@1fbe3400 {
>  			compatible = "airoha,en7581-pbus-csr", "syscon";
>  			reg = <0x0 0x1fbe3400 0x0 0xff>;
>  		};
>  
>  		uart1: serial@1fbf0000 {
> -			compatible = "ns16550";
> +			compatible = "airoha,en7523-uart";
>  			reg = <0x0 0x1fbf0000 0x0 0x30>;
>  			reg-io-width = <4>;
>  			reg-shift = <2>;

[Severity: High]
Does replacing compatible = "ns16550" with "airoha,en7523-uart" on uart1
(serial@1fbf0000) without a fallback compatible break earlycon and the
serial console on EN7581 boards?

In arch/arm64/boot/dts/airoha/en7581-evb.dts, uart1 is selected as the
console via stdout-path:

	aliases {
		serial0 = &uart1;
	};

	chosen {
		stdout-path = "serial0:115200n8";

Neither "airoha,en7523-uart" (used in uart1, serial@1fbf0300,
serial@1fbf0600, and serial@1fbf0700) nor "airoha,an7581-hsuart" (used in
serial@1fbe1000) is documented in Documentation/devicetree/bindings/serial/
or matched by any driver in the tree, such as of_platform_serial_table in
drivers/tty/serial/8250/8250_of.c:

static const struct of_device_id of_platform_serial_table[] = {
	{ .compatible = "ns8250",   .data = (void *)PORT_8250, },
	{ .compatible = "ns16450",  .data = (void *)PORT_16450, },
	{ .compatible = "ns16550a", .data = (void *)PORT_16550A, },
	{ .compatible = "ns16550",  .data = (void *)PORT_16550, },

Without corresponding binding schemas, driver support, or a fallback
compatible on uart1, won't dtbs_check fail on all five serial nodes and
leave uart1 unable to bind during early_init_dt_scan_chosen_stdout() and
of_platform_serial_driver probing?

> @@ -517,6 +528,39 @@ en7581_pwm: pwm {
>  			};
>  		};
>  
> +		serial@1fbf0300 {
> +			compatible = "airoha,en7523-uart";
> +			reg = <0x0 0x1fbf0300 0x0 0x30>;
> +			reg-io-width = <4>;
> +			reg-shift = <2>;
> +			interrupts = <GIC_SPI 32 IRQ_TYPE_LEVEL_HIGH>;
> +			clock-frequency = <7372800>;
> +
> +			status = "disabled";
> +		};

[Severity: Medium]
Should uart1 (serial@1fbf0000) also have its clock-frequency updated to
7372800 when switching its compatible to "airoha,en7523-uart"?

The three new "airoha,en7523-uart" nodes (serial@1fbf0300, serial@1fbf0600,
and serial@1fbf0700) all use clock-frequency = <7372800>, whereas uart1 in
arch/arm64/boot/dts/airoha/en7581.dtsi keeps clock-frequency = <1843200>:

		uart1: serial@1fbf0000 {
			compatible = "airoha,en7523-uart";
			reg = <0x0 0x1fbf0000 0x0 0x30>;
			reg-io-width = <4>;
			reg-shift = <2>;
			interrupts = <GIC_SPI 18 IRQ_TYPE_LEVEL_HIGH>;
			clock-frequency = <1843200>;
		};

As noted in arch/mips/boot/dts/econet/en751221.dtsi for serial@1fbf0000,
1843200 was a workaround for the generic "ns16550" driver to fix the port
at 115200 baud:

	uart: serial@1fbf0000 {
		compatible = "ns16550";
		...
		/*
		 * Conversion of baud rate to clock frequency requires a
		 * computation that is not in the ns16550 driver, so this
		 * uart is fixed at 115200 baud.
		 */
		clock-frequency = <1843200>;
	};

If a driver matching "airoha,en7523-uart" computes baud-rate divisors from
clock-frequency, won't keeping 1843200 on uart1 cause a 4x baud-rate
mismatch compared to the other "airoha,en7523-uart" nodes?

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