Re: [PATCH net-next v8 05/11] arm64: dts: rockchip: rk3568: add XPCS and fixed-clock nodes

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Low] devicetree: fixed-clock anti-pattern for internal XPCS clocks
- [Low] devicetree: missing required phys property in xpcs node
--

commit 752959013e963f31a1ffc2b59a8a556bc481a7c3
Author: Coia Prant <coiaprant@gmail.com>
arm64: dts: rockchip: rk3568: add XPCS and fixed-clock nodes

This commit adds the XPCS device node and associated fixed-clock nodes to
the RK3568 SoC device tree to support 1000BASE-X, SGMII, and QSGMII
interfaces. The XPCS node requires board-level activation and PHY
assignments, while the fixed-clock nodes are introduced to provide a
static 125 MHz reference.

> diff --git a/arch/arm64/boot/dts/rockchip/rk3568.dtsi b/arch/arm64/boot/dts/rockchip/rk3568.dtsi
> --- a/arch/arm64/boot/dts/rockchip/rk3568.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3568.dtsi
> @@ -110,6 +110,51 @@ sata0: sata@fc000000 {
>  		status = "disabled";
>  	};
>  
> +	xpcs: ethernet-pcs@fda00000 {
> +		compatible = "rockchip,rk3568-xpcs";
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +		reg = <0x0 0xfda00000 0x0 0x200000>;
> +		clocks = <&cru PCLK_XPCS>, <&cru CLK_XPCS_EEE>;
> +		clock-names = "csr", "eee";
> +		power-domains = <&power RK3568_PD_PIPE>;
> +		status = "disabled";

[Severity: Low]
The commit message states that "dtbs_check only validates required properties
for enabled nodes, so the SoC dtsi does not provide phys/phy-names". Is this
assumption accurate?

dt-schema evaluates all nodes against the required list regardless of the
status property, unless the schema explicitly conditions the requirement. Will
this omission cause make dtbs_check to throw validation errors for all RK3568
boards that leave the XPCS node disabled?

[ ... ]

> +	clk_gmac0_xpcs_mii: clock-xpcs-gmac0 {
> +		compatible = "fixed-clock";
> +		clock-frequency = <125000000>;
> +		clock-output-names = "clk_gmac0_xpcs_mii";
> +		#clock-cells = <0>;
> +	};

[Severity: Low]
Does using a fixed-clock here accurately model the hardware?

Since the 125 MHz reference clock is generated internally by the XPCS or
Combo PHY, representing it as a fixed root oscillator appears to violate
devicetree hardware modeling principles.

Should the IP block that actually generates this clock act as the clock
provider, instead of injecting a dummy fixed-clock node into the device tree?

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

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy