Re: [PATCH v2 12/12] arm64: dts: qcom: Update IPQ5210 xo_board to use fixed factor clock

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The DT topology incorrectly parents `xo_board` to the raw `xo_clk` crystal instead of the 48MHz WiFi output clock (`ref_48mhz_clk`).
--

--- Patch [12]: [PATCH v2 12/12] arm64: dts: qcom: Update IPQ5210 xo_board to use fixed factor clock ---
commit a2a14fe4e6a6b7af2b47f570d64cc8151c8401f4
Author: Luo Jie <[email protected]>

arm64: dts: qcom: Update IPQ5210 xo_board to use fixed factor clock

Update the xo_board clock definition to use a fixed factor clock divided
by 2. The commit message indicates this is routed from the 48MHz WiFi
output clock.

> diff --git a/arch/arm64/boot/dts/qcom/ipq5210.dtsi b/arch/arm64/boot/dts/qcom/ipq5210.dtsi
> --- a/arch/arm64/boot/dts/qcom/ipq5210.dtsi
> +++ b/arch/arm64/boot/dts/qcom/ipq5210.dtsi
> @@ -26,7 +26,8 @@ sleep_clk: sleep-clk {
>  		};
>  
>  		xo_board: xo-board-clk {
> -			compatible = "fixed-clock";
> +			compatible = "fixed-factor-clock";
> +			clocks = <&xo_clk>;

[Severity: Medium]
The commit message states that xo_board is routed from the WiFi output
clock (ref_48mhz_clk). Should this be parented to <&ref_48mhz_clk>
instead of <&xo_clk> to correctly reflect the hardware topology?

> +			#clock-cells = <0>;
>  		};

> diff --git a/arch/arm64/boot/dts/qcom/ipq5210-rdp504.dts b/arch/arm64/boot/dts/qcom/ipq5210-rdp504.dts
> --- a/arch/arm64/boot/dts/qcom/ipq5210-rdp504.dts
> +++ b/arch/arm64/boot/dts/qcom/ipq5210-rdp504.dts
[ ... ]
> +/*
> + * The frequency of xo_board is fixed to 24 MHZ, which is routed
> + * from WiFi output clock 48 MHZ divided by 2.
> + */
>  &xo_board {
> -	clock-frequency = <24000000>;
> +	clock-div = <2>;
> +	clock-mult = <1>;
>  };

[Severity: Medium]
Does this implicitly rely on xo_clk being exactly 48MHz?

If a future board uses a different crystal frequency, this setup might force
the board file to specify a compound divider instead of modeling the analog
block's physical divide-by-2 from the WiFi output. If xo_board is parented
to ref_48mhz_clk as described in the commit message, would this physical
divide-by-2 relationship be more accurately preserved?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=12