Re: [PATCH 3/3] arm64: dts: ti: k3-j784s4: Add overlay for dual port USXGMII+SGMII mode

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Incomplete clock configuration in `&serdes_wiz2` leaves a dangling `assigned-clocks` element as dead code.
- [Low] The unit addresses in the PHY node names do not match their `reg` property values.
--

commit ec16658036ad6f4bdcb6018a15184b9bd2b3c4aa
Author: Gokul Praveen <g-praveen@ti.com>

arm64: dts: ti: k3-j784s4: Add overlay for dual port USXGMII+SGMII mode

The CPSW9G instance of the CPSW Ethernet Switch supports SGMII and
USXGMII mode with MAC Ports 1 and 2 of the instance, which are
connected to ENET Expansion 1 and ENET Expansion 2 slots on
the EVM respectively, through the Serdes2 instance of the SERDES.

Enable CPSW9G MAC Ports 1 and 2 in fixed-link configuration
SGMII and USXGMII mode at 1Gbps and 5 Gbps each.

> diff --git a/arch/arm64/boot/dts/ti/k3-j784s4-evm-sgmii-usxgmii-exp1-exp2_board1.dtso b/arch/arm64/boot/dts/ti/k3-j784s4-evm-sgmii-usxgmii-exp1-exp2_board1.dtso
> new file mode 100644
> index 0000000000000..553546b281a9e
> --- /dev/null
> +++ b/arch/arm64/boot/dts/ti/k3-j784s4-evm-sgmii-usxgmii-exp1-exp2_board1.dtso

[ ... ]

> +&serdes_wiz2 {
> +	assigned-clocks = <&k3_clks 406 6>,  <&k3_clks 406 5>;
> +	//assigned-clock-parents = <&k3_clks 406 9>; /* Use 100 MHz clock for SGMII */
> +	status = "okay";
> +};

[Severity: Medium]
The assigned-clock-parents override is commented out here, but the
assigned-clocks array was expanded to two elements.

Because the base device tree only defines a single parent, the clock framework
will only process the first clock and silently ignore the second element.

Does this leave the second clock unconfigured, and should
assigned-clock-parents be uncommented to resolve this?

> +
> +&serdes2 {
> +	#address-cells = <1>;
> +	#size-cells = <0>;
> +	status = "okay";
> +
> +	assigned-clock-parents = <&k3_clks 406 5>,
> +						 <&k3_clks 406 6>,
> +						 <&k3_clks 406 5>;
> +
> +	serdes2_sgmii2_link: phy@1 {
> +		reg = <2>;
> +		cdns,num-lanes = <1>;
> +		#phy-cells = <0>;
> +		cdns,phy-type = <PHY_TYPE_SGMII>;
> +		resets = <&serdes_wiz2 3>;
> +	};
> +	serdes2_usxgmii_link: phy@0 {
> +		reg = <3>;

[Severity: Low]
Do the unit addresses phy@1 and phy@0 need to match their reg property
values <2> and <3>?

The dtc compiler will generate unit_address_vs_reg warnings when the unit
address and reg property do not match.

[ ... ]

> diff --git a/arch/arm64/boot/dts/ti/k3-j784s4-evm-usxgmii-sgmii-exp1-exp2_board2.dtso b/arch/arm64/boot/dts/ti/k3-j784s4-evm-usxgmii-sgmii-exp1-exp2_board2.dtso
> new file mode 100644
> index 0000000000000..c98be3bbf2c8b
> --- /dev/null
> +++ b/arch/arm64/boot/dts/ti/k3-j784s4-evm-usxgmii-sgmii-exp1-exp2_board2.dtso

[ ... ]

> +&serdes_wiz2 {
> +	assigned-clocks = <&k3_clks 406 6>,  <&k3_clks 406 5>;
> +	//assigned-clock-parents = <&k3_clks 406 9>; /* Use 156.25 MHz clock for USXGMII */
> +	status = "okay";
> +};

[Severity: Medium]
Similar to the first overlay, the assigned-clock-parents override is commented
out while assigned-clocks contains two elements.

Is the second clock intended to be left unconfigured here?

> +
> +&serdes2 {
> +	#address-cells = <1>;
> +	#size-cells = <0>;
> +	status = "okay";
> +
> +	assigned-clock-parents = <&k3_clks 406 5>,
> +						 <&k3_clks 406 6>,
> +						 <&k3_clks 406 5>;
> +
> +
> +
> +	serdes2_sgmii2_link: phy@1 {
> +		reg = <3>;
> +		cdns,num-lanes = <1>;
> +		#phy-cells = <0>;
> +		cdns,phy-type = <PHY_TYPE_SGMII>;
> +		resets = <&serdes_wiz2 4>;
> +	};
> +	serdes2_usxgmii_link: phy@0 {
> +		reg = <2>;

[Severity: Low]
Are the unit addresses for these nodes correct?

They are named phy@1 and phy@0, but their reg properties are set to <3>
and <2>. Should they be updated to match the reg properties to prevent dtc
warnings?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260625120805.28948-1-g-praveen@ti.com?part=3