Thread (29 messages) flat view 29 messages, 5 authors, 13d ago

Re: [PATCH PRELIMINARY 4/5] arm64: dts: qcom: shikra-cqs-evk: enable the first Ethernet port

From: sashiko-bot@kernel.org
Date: 2026-09-09 06:24:25

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Ethernet PHY remains permanently unpowered because the `vdda-2p5-supply` property is ignored and the regulator defaults to off.
--

commit b5b6a106e748058add8ac1a40dcbd3ae6ad2a0d0
Author: Mohd Ayaan Anwar [off-list ref]

arm64: dts: qcom: shikra-cqs-evk: enable the first Ethernet port

This commit enables ethernet0 for the Shikra CQS EVK board with a DP83867
RGMII PHY. It also adds a fixed 2.5V regulator controlled by a GPIO to power
the PHY.
quoted hunk ↗ jump to hunk
diff --git a/arch/arm64/boot/dts/qcom/shikra-cqs-evk.dts b/arch/arm64/boot/dts/qcom/shikra-cqs-evk.dts
index e62ba5aef71ff..7b30c67c68e8c 100644
--- a/arch/arm64/boot/dts/qcom/shikra-cqs-evk.dts
+++ b/arch/arm64/boot/dts/qcom/shikra-cqs-evk.dts
@@ -21,6 +21,42 @@ aliases {
 	chosen {
 		stdout-path = "serial0:115200n8";
 	};
+
+	rgmii_phy_supply: regulator-rgmii-phy {
+		compatible = "regulator-fixed";
+		regulator-name = "rgmii-phy-pwr";
[Severity: High]
Will this regulator ever be enabled by the core? 

Since no consumer driver will explicitly request it (see below), and the node 
is missing the regulator-always-on property, it appears the regulator core 
will leave it disabled at boot.
+		regulator-min-microvolt = <2500000>;
+		regulator-max-microvolt = <2500000>;
+		gpio = <&tlmm 149 GPIO_ACTIVE_HIGH>;
+		enable-active-high;
+		pinctrl-0 = <&rgmii_phy_pwr_default>;
+		pinctrl-names = "default";
+	};
+};
[ ... ]
+		rgmii_phy: 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_phy_supply>;
[Severity: High]
Does the kernel actually use the vdda-2p5-supply property? 

It seems this custom property is completely ignored because no driver, 
including the DP83867 driver or the MDIO core, parses vdda-2p5-supply. 
Without a driver acting as a consumer, the PHY will fail to power on.

It looks like this same pattern might also be replicated in patch 5/5 for 
the second Ethernet port.
+		};
+	};
 };
[ ... ]
+&tlmm {
+	rgmii_phy_pwr_default: rgmii-phy-pwr-default-state {
+		pins = "gpio149";
+		function = "gpio";
+		drive-strength = <2>;
+		bias-disable;
+		output-low;
[Severity: High]
Is it intentional to drive the enable GPIO low here at boot? 

The regulator is defined as active-high above, so this default pinctrl 
state explicitly turns off the active-high regulator. Combined with the 
ignored supply property, this would leave the Ethernet PHY permanently 
unpowered and non-functional unconditionally at boot.
+	};
+};
-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260908-shikra_ethernet_dts-v1-0-69c0c5c7c124@oss.qualcomm.com?part=4
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help