Re: [PATCH PRELIMINARY 5/5] arm64: dts: qcom: shikra-iqs-evk: enable both Ethernet ports

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The custom `vdda-2p5-supply` property is not supported by the DP83867 PHY driver or the kernel's MDIO/PHY core, causing the fixed regulators (`rgmii-phy0-pwr` and `rgmii-phy1-pwr`) to never be enabled. As their pinctrl states explicitly drive the GPIOs low, the PHYs will remain permanently unpowered and Ethernet will fail to function.
--

commit 6a680ac089e27638d63a04a13237e86b52add97e
Author: Mohd Ayaan Anwar <mohd.anwar@oss.qualcomm.com>

arm64: dts: qcom: shikra-iqs-evk: enable both Ethernet ports
    
Enable ethernet0 and ethernet1 on the IQS EVK with their respective
TI DP83867 RGMII PHYs. Both PHYs are powered by GPIO-controlled 2.5V
regulators.

> diff --git a/arch/arm64/boot/dts/qcom/shikra-iqs-evk.dts b/arch/arm64/boot/dts/qcom/shikra-iqs-evk.dts
> index 727809430fd15..049feb5b4d965 100644
> --- a/arch/arm64/boot/dts/qcom/shikra-iqs-evk.dts
> +++ b/arch/arm64/boot/dts/qcom/shikra-iqs-evk.dts
> @@ -21,6 +21,78 @@ aliases {
>  	chosen {
>  		stdout-path = "serial0:115200n8";
>  	};
> +
> +	rgmii_phy0_supply: regulator-rgmii-phy0 {
> +		compatible = "regulator-fixed";
> +		regulator-name = "rgmii-phy0-pwr";
> +		regulator-min-microvolt = <2500000>;
> +		regulator-max-microvolt = <2500000>;
> +		gpio = <&tlmm 66 GPIO_ACTIVE_HIGH>;
> +		enable-active-high;
> +		pinctrl-0 = <&rgmii_phy0_pwr_default>;
> +		pinctrl-names = "default";
> +	};

[Severity: High]
Does this regulator rely on the PHY driver to enable it? It lacks the
regulator-always-on property and is configured to default off via its pinctrl
state (output-low below). If it is not enabled by a driver, it will remain
disabled.

[ ... ]

> +&ethernet0 {
> +	phy-handle = <&rgmii_phy0>;
> +	phy-mode = "rgmii-id";
> +
> +	pinctrl-0 = <&ethernet0_default>;
> +	pinctrl-names = "default";
> +
> +	status = "okay";
> +
> +	mdio {
> +		compatible = "snps,dwmac-mdio";
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		rgmii_phy0: ethernet-phy@7 {
> +			compatible = "ethernet-phy-id2000.a231";
> +			reg = <0x7>;
> +			reset-gpios = <&tlmm 135 GPIO_ACTIVE_LOW>;
> +			reset-assert-us = <10000>;
> +			reset-deassert-us = <50000>;
> +			vdda-2p5-supply = <&rgmii_phy0_supply>;

[Severity: High]
Is the vdda-2p5-supply property supported by the DP83867 PHY driver or the
kernel's MDIO/PHY core?

If this custom property is silently ignored by the kernel, the fixed regulator
above will never be enabled. Combined with the pinctrl state explicitly driving
the GPIO low during boot, won't this leave the PHY permanently unpowered and
cause the Ethernet interface to fail?

[ ... ]

> +&tlmm {
> +	rgmii_phy0_pwr_default: rgmii-phy0-pwr-default-state {
> +		pins = "gpio66";
> +		function = "gpio";
> +		drive-strength = <2>;
> +		bias-disable;
> +		output-low;
> +	};

[Severity: High]
This explicitly drives the regulator GPIO low (off) when the pinctrl state is
applied during boot, actively disabling power. Since the regulator isn't
re-enabled later due to the unsupported supply property above, does this leave
the PHY unpowered?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260908-shikra_ethernet_dts-v1-0-69c0c5c7c124@oss.qualcomm.com?part=5