Re: [PATCH 1/2] ARM: dts: mediatek: mt6572: rename fixed clocks

[sashiko-bot@kernel.org]

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

New issues:
- [Low] The fixed-clock nodes are renamed to `clk26m`, `clk13m`, and `clk32k`, which violates the preferred node naming convention in the fixed-clock DT schema.

Pre-existing issues:
- [High] The UART nodes (`uart0` and `uart1`) specify `clock-names = "baud"` but omit the required `"bus"` clock, leading to a guaranteed probe failure in the UART driver.
--

commit b76c9552213f6a611c0faa777e13268895b879c6
Author: Roman Vivchar <rva333@protonmail.com>

ARM: dts: mediatek: mt6572: rename fixed clocks

This commit renames the old-style fixed clocks uart_clk, system_clk, and
rtc_clk to clk26m, clk13m, and clk32k to better reflect the hardware.

> diff --git a/arch/arm/boot/dts/mediatek/mt6572.dtsi b/arch/arm/boot/dts/mediatek/mt6572.dtsi
> index ac70f266d698d..fa0e5df766c20 100644
> --- a/arch/arm/boot/dts/mediatek/mt6572.dtsi
> +++ b/arch/arm/boot/dts/mediatek/mt6572.dtsi
> @@ -28,19 +28,19 @@ cpu@1 {
>  		};
>  	};
>  
> -	uart_clk: dummy26m {
> +	clk26m: clk26m {

[Severity: Low]
Do these new node names violate the preferred device tree schema pattern?

According to the fixed-clock schema:

Documentation/devicetree/bindings/clock/fixed-clock.yaml:
    pattern: "^clock-([0-9]+|[a-z0-9-]+)$"

Should these nodes be named clock-26m, clock-13m, and clock-32k to pass
dtbs_check without warnings?

>  		compatible = "fixed-clock";
>  		clock-frequency = <26000000>;
>  		#clock-cells = <0>;
>  	};

[ ... ]

> @@ -91,7 +91,7 @@ uart0: serial@11005000 {
>  			compatible = "mediatek,mt6572-uart", "mediatek,mt6577-uart";
>  			reg = <0x11005000 0x400>;
>  			interrupts = <GIC_SPI 31 IRQ_TYPE_LEVEL_LOW>;
> -			clocks = <&uart_clk>;
> +			clocks = <&clk26m>;
>  			clock-names = "baud";

[Severity: High]
This is a pre-existing issue, but does providing only the baud clock
cause the UART driver probe to fail?

Looking at the 8250 driver:

drivers/tty/serial/8250/8250_mtk.c:mtk8250_probe_of() {
    ...
    data->bus_clk = devm_clk_get_enabled(&pdev->dev, "bus");
    if (IS_ERR(data->bus_clk))
        return PTR_ERR(data->bus_clk);
    ...
}

If the baud clock is present, the driver skips the legacy fallback
logic and unconditionally tries to fetch the bus clock. Since bus
is omitted from the device tree, this returns an error and aborts the probe.

Should the missing bus clock be added to these UART nodes?

>  			status = "disabled";
>  		};

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260704-6572-phase1-v1-0-61c2fe6be19c@protonmail.com?part=1