Re: [PATCH v2 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 3 potential issue(s) to consider:
- [Medium] Array length mismatch between `assigned-clocks` and `assigned-clock-parents` in `&serdes_wiz2`.
- [Low] Mismatch between node unit-address and 'reg' property in SerDes PHY links.
- [Low] Dangling continuation backslash on the last item of a Makefile list immediately preceding a comment.
--

commit bc5648306dcc07a1bc8095cecaf3da3724e52d5d
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/Makefile b/arch/arm64/boot/dts/ti/Makefile
> index 5269c9619b65c..72c91f8f7814d 100644
> --- a/arch/arm64/boot/dts/ti/Makefile
> +++ b/arch/arm64/boot/dts/ti/Makefile
[ ... ]
> @@ -337,7 +343,8 @@ dtb- += k3-am625-beagleplay-csi2-ov5640.dtb \
>  	k3-j784s4-evm-pcie0-pcie1-ep.dtb \
>  	k3-j784s4-evm-quad-port-eth-exp1.dtb \
>  	k3-j784s4-evm-usb0-type-a.dtb \
> -	k3-j784s4-evm-usxgmii-exp1-exp2.dtb
> -
> +	k3-j784s4-evm-usxgmii-exp1-exp2.dtb \
> +	k3-j784s4-evm-usxgmii-sgmii-exp1-exp2_board2.dtb \
> +	k3-j784s4-evm-sgmii-usxgmii-exp1-exp2_board1.dtb \
>  # Enable support for device-tree overlays
>  DTC_FLAGS := -@

[Severity: Low]
Does this trailing backslash escape the newline before the comment? Since
Make processes continuations before comments, this merges the comment onto
the dtb- line. If the comment is ever removed, DTC_FLAGS := -@ would be
mistakenly pulled into the dtb- variable.

> 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..4ceb99c5c8088
> --- /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>;
> +	status = "okay";
> +};

[Severity: Medium]
Will this 2-element assigned-clocks array work correctly without overriding
the assigned-clock-parents array as well? Since assigned-clock-parents has
only 1 element in the base device tree, the clock framework evaluates
num_parents based on that length and ignores the second clock here, leaving
it unparented.

[ ... ]
> +&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>;
> +	};

[Severity: Low]
Does this node name phy@1 violate the standard device tree schema when the
reg property is set to 2? This mismatch will trigger a unit_address_vs_reg
dtc compiler warning.

> +	serdes2_usxgmii_link: phy@0 {
> +		reg = <3>;
> +		cdns,num-lanes = <1>;
> +		#phy-cells = <0>;
> +		cdns,phy-type = <PHY_TYPE_USXGMII>;
> +		resets = <&serdes_wiz2 4>;
> +	};

[Severity: Low]
Similarly here, shouldn't the unit-address in phy@0 match the reg property
of 3?

> 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..ef4ebb5515469
> --- /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>;
> +	status = "okay";
> +};

[Severity: Medium]
Like in board1, will this assigned-clocks override fail to assign the second
clock because it doesn't also update assigned-clock-parents to be a
2-element array?

[ ... ]
> +	serdes2_sgmii2_link: phy@1 {
> +		reg = <3>;
> +		cdns,num-lanes = <1>;
> +		#phy-cells = <0>;
> +		cdns,phy-type = <PHY_TYPE_SGMII>;
> +		resets = <&serdes_wiz2 4>;
> +	};

[Severity: Low]
Does this unit-address phy@1 need to be updated to phy@3 to match the reg
property?

> +	serdes2_usxgmii_link: phy@0 {
> +		reg = <2>;
> +		cdns,num-lanes = <1>;
> +		#phy-cells = <0>;
> +		cdns,phy-type = <PHY_TYPE_USXGMII>;
> +		resets = <&serdes_wiz2 3>;
> +	};

[Severity: Low]
Does this unit-address phy@0 need to be updated to phy@2 to match the reg
property?

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