Despite its name, the regulator vcc3v3_pcie30x1 has nothing to do with
pcie30x1. Instead, it supply power to VBAT1-5 on the M.2 KEY B port as
seen on page 8 of the schematic [1].

pcie30x1 is used for the mini PCIe slot, and as seen on page 9 the
vcc3v3_minipcie regulator is instead related to pcie30x1.

The M.2 KEY B port can be used for WWAN USB2 modules or SATA drives.

Use correct regulator vcc3v3_minipcie for pcie30x1 and enable sata1 node
to fix use of SATA drives on the M.2 slot.

[1] https://dl.radxa.com/cm3p/e25/radxa-e25-v1.4-sch.pdf

Fixes: 2bf2f4d9f673 ("arm64: dts: rockchip: Add Radxa CM3I E25")
Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
---
 .../boot/dts/rockchip/rk3568-radxa-e25.dts    | 24 ++++++++++++-------
 1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/arch/arm64/boot/dts/rockchip/rk3568-radxa-e25.dts b/arch/arm64/boot/dts/rockchip/rk3568-radxa-e25.dts
index 63c4bd873188..72ad74c38a2b 100644
--- a/arch/arm64/boot/dts/rockchip/rk3568-radxa-e25.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3568-radxa-e25.dts
@@ -47,6 +47,9 @@ vbus_typec: vbus-typec-regulator {
 		vin-supply = <&vcc5v0_sys>;
 	};
 
+	/* actually fed by vcc5v0_sys, dependent
+	 * on pi6c clock generator
+	 */
 	vcc3v3_minipcie: vcc3v3-minipcie-regulator {
 		compatible = "regulator-fixed";
 		enable-active-high;
@@ -54,9 +57,9 @@ vcc3v3_minipcie: vcc3v3-minipcie-regulator {
 		pinctrl-names = "default";
 		pinctrl-0 = <&minipcie_enable_h>;
 		regulator-name = "vcc3v3_minipcie";
-		regulator-min-microvolt = <5000000>;
-		regulator-max-microvolt = <5000000>;
-		vin-supply = <&vcc5v0_sys>;
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+		vin-supply = <&vcc3v3_pi6c_05>;
 	};
 
 	vcc3v3_ngff: vcc3v3-ngff-regulator {
@@ -71,9 +74,6 @@ vcc3v3_ngff: vcc3v3-ngff-regulator {
 		vin-supply = <&vcc5v0_sys>;
 	};
 
-	/* actually fed by vcc5v0_sys, dependent
-	 * on pi6c clock generator
-	 */
 	vcc3v3_pcie30x1: vcc3v3-pcie30x1-regulator {
 		compatible = "regulator-fixed";
 		enable-active-high;
@@ -83,7 +83,7 @@ vcc3v3_pcie30x1: vcc3v3-pcie30x1-regulator {
 		regulator-name = "vcc3v3_pcie30x1";
 		regulator-min-microvolt = <3300000>;
 		regulator-max-microvolt = <3300000>;
-		vin-supply = <&vcc3v3_pi6c_05>;
+		vin-supply = <&vcc5v0_sys>;
 	};
 
 	vcc3v3_pi6c_05: vcc3v3-pi6c-05-regulator {
@@ -99,6 +99,10 @@ vcc3v3_pi6c_05: vcc3v3-pi6c-05-regulator {
 	};
 };
 
+&combphy1 {
+	phy-supply = <&vcc3v3_pcie30x1>;
+};
+
 &pcie2x1 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&pcie20_reset_h>;
@@ -117,7 +121,7 @@ &pcie3x1 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&pcie30x1m0_pins>;
 	reset-gpios = <&gpio0 RK_PC3 GPIO_ACTIVE_HIGH>;
-	vpcie3v3-supply = <&vcc3v3_pcie30x1>;
+	vpcie3v3-supply = <&vcc3v3_minipcie>;
 	status = "okay";
 };
 
@@ -178,6 +182,10 @@ &pwm12 {
 	status = "okay";
 };
 
+&sata1 {
+	status = "okay";
+};
+
 &sdmmc0 {
 	bus-width = <4>;
 	cap-sd-highspeed;
-- 
2.41.0

On Mon, 24 Jul 2023 14:52:16 +0000 (UTC), Jonas Karlman wrote:
> Despite its name, the regulator vcc3v3_pcie30x1 has nothing to do with
> pcie30x1. Instead, it supply power to VBAT1-5 on the M.2 KEY B port as
> seen on page 8 of the schematic [1].
> 
> pcie30x1 is used for the mini PCIe slot, and as seen on page 9 the
> vcc3v3_minipcie regulator is instead related to pcie30x1.
> 
> [...]

Applied, thanks!

[1/2] arm64: dts: rockchip: Fix PCIe regulators on Radxa E25
commit: a87852e37f782257ebc57cc44a0d3fbf806471f6

[2/2] arm64: dts: rockchip: Enable SATA on Radxa E25
commit: 2bdfe84fbd57a4ed9fd65a67210442559ce078f0

As you can see, I've split them in two. Please check that I didn't
mess anything up. Having an "and" in the commit subject is always a
very strong indicator that a split is probably a good thing ;-)


Best regards,
-- 
Heiko Stuebner <heiko@sntech.de>

On 2023-07-25 20:32, Heiko Stuebner wrote:
> On Mon, 24 Jul 2023 14:52:16 +0000 (UTC), Jonas Karlman wrote:
>> Despite its name, the regulator vcc3v3_pcie30x1 has nothing to do with
>> pcie30x1. Instead, it supply power to VBAT1-5 on the M.2 KEY B port as
>> seen on page 8 of the schematic [1].
>>
>> pcie30x1 is used for the mini PCIe slot, and as seen on page 9 the
>> vcc3v3_minipcie regulator is instead related to pcie30x1.
>>
>> [...]
> 
> Applied, thanks!
> 
> [1/2] arm64: dts: rockchip: Fix PCIe regulators on Radxa E25
> commit: a87852e37f782257ebc57cc44a0d3fbf806471f6
> 
> [2/2] arm64: dts: rockchip: Enable SATA on Radxa E25
> commit: 2bdfe84fbd57a4ed9fd65a67210442559ce078f0
> 
> As you can see, I've split them in two. Please check that I didn't
> mess anything up. Having an "and" in the commit subject is always a
> very strong indicator that a split is probably a good thing ;-)

Thanks, and they look fine. Should probably have split it in two from
the beginning, I will try to do better next time :-)

Regards,
Jonas

> 
> 
> Best regards,