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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [Low] DT Schema Validation Failure: Disabled 'xpcs' node lacks required properties
- [Low] DT Schema Validation Failure: 'status' property in 'pcs-mii' subnodes
- [Low] Missing Hardware Reset Property in XPCS node
- [Low] Incorrect Clock Modeling: Fake fixed-clock nodes for hardware clocks
--

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

This commit adds the XPCS device node and its associated pcs-mii sub-nodes
to the RK3568 device tree. It also introduces two fixed-clock nodes to serve
as reference clocks for the GMACs.

> diff --git a/arch/arm64/boot/dts/rockchip/rk3568.dtsi b/arch/arm64/boot/dts/rockchip/rk3568.dtsi
> index 3bc653f027f1f..989e164c0eb39 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3568.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3568.dtsi
[ ... ]
> +	xpcs: 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]
Will this cause make dtbs_check to emit validation warnings for boards that
include rk3568.dtsi without overriding the xpcs node? The
rockchip,rk3568-xpcs binding schema unconditionally requires phys and
phy-names at the root level, even when the node is disabled.

[Severity: Low]
Does the xpcs node accurately represent the hardware without a resets
property? The RK3568 hardware provides dedicated resets (like SRST_XPCS)
for this block. Omitting the resets property might leave the hardware
description incomplete, even if the current Linux driver does not actively
use it.

> +
> +		xpcs_mii0: pcs-mii@0 {
> +			reg = <0>;
> +			status = "disabled";

[Severity: Low]
Is the status property permitted in these pcs-mii subnodes? The
rockchip-dwxpcs.yaml schema uses additionalProperties: false for the
^pcs-mii@[0-3]$ pattern and does not explicitly allow the status property.
This might trigger another validation warning during make dtbs_check.

> +		};
[ ... ]
> +
> +	xpcs_gmac0_clk: xpcs-gmac0-clock {
> +		compatible = "fixed-clock";
> +		clock-frequency = <125000000>;

[Severity: Low]
Does modeling these 125 MHz reference clocks as standalone fixed-clock nodes
mask the true hardware dependency? Since these clocks are actually generated
by the XPCS/Combo PHY block, using dummy board-level clocks could hide the
real topology from the device tree, even if a power management race is
currently avoided by a device link in the MAC driver.

> +		clock-output-names = "clk_gmac0_xpcs_mii";
> +		#clock-cells = <0>;
> +	};

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

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