Re: [PATCH v2 2/2] arm64: zynqmp: Add ifm ZynqMP VHIP6 EvalBoard

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Incorrect use of "ethernet-phy-ieee802.3-c45" compatible string for a Clause 22 PHY, breaking PHY probing.
- [Medium] Conflicting RGMII TX delay configuration for gem1/phy1 overrides `phy-mode` contract.
- [Medium] Suspiciously short PHY reset deassertion delay (10us).
--

commit eea503ea47697121b80dcf7c5965d02c5b346e78
Author: Marek Vasut <marex@nabladev.com>

arm64: zynqmp: Add ifm ZynqMP VHIP6 EvalBoard

Add support for ifm ZynqMP VHIP6 EvalBoard reference design.
The board is populated with four ethernet ports, QSPI NOR,
eMMC, UART and USB host interfaces.

> diff --git a/arch/arm64/boot/dts/xilinx/zynqmp-vhip6-evalboard.dts b/arch/arm64/boot/dts/xilinx/zynqmp-vhip6-evalboard.dts
> new file mode 100644
> index 0000000000000..0108f3d505a7f
> --- /dev/null
> +++ b/arch/arm64/boot/dts/xilinx/zynqmp-vhip6-evalboard.dts
[ ... ]
> +&gem0 {
> +	phy-handle = <&phy0>;
> +	phy-mode = "mii";
> +	status = "okay";
> +
> +	mdio0: mdio {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		phy0: phy@1 {	/* TI DP83822I */
> +			compatible = "ethernet-phy-id2000.a240",
> +				     "ethernet-phy-ieee802.3-c45";

[Severity: High]
Does this compatible string cause the PHY probe to fail?

The TI DP83822I is a standard Clause 22 PHY. Using the C45 fallback string
causes fwnode_mdiobus_register_phy() to parse "ethernet-phy-ieee802.3-c45",
set is_c45 = true, and force get_phy_c45_ids() to read the PHY ID using native
Clause 45 MDIO frames. Since the PHY ignores native C45 accesses, this
read fails and get_phy_c45_ids() returns -ENODEV, aborting the PHY registration.
This configuration is also duplicated for phy1, phy2, and phy3.

> +			reg = <1>;
> +			reset-gpios = <&gpio 78 GPIO_ACTIVE_LOW>;
> +			reset-assert-us = <10>;
> +			reset-deassert-us = <10>;

[Severity: Medium]
Is a 10 microsecond reset deassertion delay long enough?

The MDIO core waits exactly reset-deassert-us after pulling the reset GPIO
high before attempting communication. 10 microseconds is an extremely brief
window. This might cause intermittent MDIO read timeouts (0xffff) and probe
failures if the PHY hasn't completed its hardware reset initialization.
This short delay is also present for phy1, phy2, and phy3.

> +		};
> +	};
> +};
> +
> +&gem1 {
> +	phy-handle = <&phy1>;
> +	phy-mode = "rgmii-rxid";

[Severity: Medium]
Does explicitly setting tx-internal-delay-ps below conflict with the RGMII
contract established here?

The phy-mode = "rgmii-rxid" property dictates that the PHY should apply an
internal RX delay but NO TX delay.

> +	status = "okay";
> +
> +	mdio1: mdio {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		phy1: phy@1 {	/* TI DP83822I */
> +			compatible = "ethernet-phy-id2000.a240",
> +				     "ethernet-phy-ieee802.3-c45";
> +			reg = <1>;
> +			max-speed = <100>;
> +			reset-gpios = <&gpio 8 GPIO_ACTIVE_LOW>;
> +			reset-assert-us = <10>;
> +			reset-deassert-us = <10>;
> +			tx-internal-delay-ps = <1>;

When this is present, the DP83822 driver parses tx-internal-delay-ps via
phy_get_internal_delay(), returning 1. The driver then bypasses the default
RGMII clock shift logic and leaves DP83822_TX_CLK_SHIFT unset, explicitly
enabling the internal TX delay on the PHY. This can double-delay the TX clock
or misalign it entirely, leading to link instability or complete RGMII TX
failure.

> +		};
> +	};
> +};

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260909182641.165383-1-marex@nabladev.com?part=2