Thread (27 messages) flat view 27 messages, 2 authors, 16h ago

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

From: netdev-bot+sashiko@kernel.org
Date: 2026-09-14 20:25:00
Also in: linux-arm-kernel, linux-devicetree, linux-phy, linux-renesas-soc, linux-rockchip, lkml

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

Critical: 0 · High: 0 · Medium: 1 · Low: 1

- [Medium] DT/binding mismatch: the new `xpcs: ethernet-pcs@fda00000`
  node in `arch/arm64/boot/dts/rockchip/rk3568.dtsi` (line 113) omits…
- [Low] Claimed missing driver/consumer for the new DT nodes: no driver
  matches `rockchip,rk3568-xpcs`, the `ethernet-pcs-mii@N` children are…

--

arm64: dts: rockchip: rk3568: add XPCS and fixed-clock nodes

Adds the RK3568 XPCS node at 0xfda00000 with its four ethernet-pcs-mii
child nodes, all disabled by default, plus two 125 MHz fixed-clock nodes
intended as assigned-clock-parents for gmac0 and gmac1.
quoted hunk ↗ jump to hunk
diff --git a/arch/arm64/boot/dts/rockchip/rk3568.dtsi b/arch/arm64/boot/dts/rockchip/rk3568.dtsi
index 3bc653f027f1f..227d03e336043 100644
--- 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: Medium]
Does this node validate against the binding added earlier in the series?

Documentation/devicetree/bindings/net/pcs/rockchip,rk3568-xpcs.yaml lists
phys and phy-names in its top-level required array, unconditionally, and
sets additionalProperties: false:

  required:
    - compatible
    - reg
    - "#address-cells"
    - "#size-cells"
    - clocks
    - clock-names
    - phys
    - phy-names
    - power-domains

The node here supplies clocks and power-domains but not phys/phy-names.

dtschema validates nodes irrespective of status = "disabled", so does
make dtbs_check now report "'phys' is a required property" and
"'phy-names' is a required property" for every rk3568 board DTB?  At the
end of the series only rk3568-photonicat.dts fills them in:

  &xpcs {
  	phys = <&combphy2 PHY_TYPE_SGMII>;
  	phy-names = "serdes";
  	status = "okay";
  };

The other 21 device trees that include rk3568.dtsi (rk3568-bpi-r2-pro.dts,
rk3568-rock-3a.dts, rk3568-odroid-m1.dts, rk3568-nanopi-r5s.dtsi,
rk3568-qnap-ts433.dts, ...) do not.

The commit message says "The XPCS node also requires a reference to the
appropriate Naneng Combo PHY via the phys property at the board level."
If that is the intended model, should phys/phy-names be dropped from the
binding's required list (or gated), rather than left mandatory?
Alternatively the SoC dtsi could carry a default phandle for boards to
override.
+
+		xpcs_mii0: ethernet-pcs-mii@0 {
+			reg = <0>;
+			status = "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]
One point that was checked and turned out not to be an issue, noted here
only for completeness: the compatible string, the ethernet-pcs-mii@N
children and these two fixed clocks all gain in-tree consumers later in
the same series, so there is nothing to change here.

drivers/net/pcs/pcs-xpcs-rk.c matches the compatible:

  static const struct of_device_id xpcs_rk_of_ids[] = {
  	{ .compatible = "rockchip,rk3568-xpcs" },
  	{ /* sentinel */ },
  };

and rk3568-photonicat.dts consumes both the child node and the clock:

  assigned-clocks = <&cru SCLK_GMAC0_RX_TX>;
  assigned-clock-parents = <&clk_gmac0_xpcs_mii>;
  managed = "in-band-status";
  pcs-handle = <&xpcs_mii0>;

Until the driver patch lands the nodes are disabled and inert, which is
the usual bindings-then-dtsi-then-driver ordering.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260913185849.907479-1-coiaprant%40gmail.com
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help