[PATCH v3 1/2] arm64: dts: rockchip: fix PCIe2 regulators for Radxa ROCK 3A

[PATCH v3 1/2] arm64: dts: rockchip: fix PCIe2 regulators for Radxa ROCK 3A

[FUKAUMI Naoki]

currently "vcc3v3_pcie" regulator is used for PCIe2 M.2 E key
connector, but, by schematic[1], it's incorrect. "vcc3v3_wf" regulator
is correct one. add it and fix related vin-supply.

[1] https://dl.radxa.com/rock3/docs/hw/3a/radxa_rock_3a_v1310_schematic.pdf

Fixes: 0fbbfb0b00d1 ("arm64: dts: rockchip: Enable PCIe controller on rock3a")
Signed-off-by: FUKAUMI Naoki <naoki@radxa.com>
---
Changes in v3:
- fix pinctrl just for reset pin as GPIO
Changes in v2:
- split patches for PCIe2 and PCIe3
- fix pinctrl node name to overwrite rk3568-pinctrl.dtsi
---
 .../boot/dts/rockchip/rk3568-rock-3a.dts      | 24 +++++++++++++------
 1 file changed, 17 insertions(+), 7 deletions(-)

diff --git a/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts b/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
index 59f1403b4fa56..f94cbddf0f0c2 100644
--- a/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
@@ -136,7 +136,17 @@ vcc3v3_sys: vcc3v3-sys-regulator {
 		regulator-boot-on;
 		regulator-min-microvolt = <3300000>;
 		regulator-max-microvolt = <3300000>;
-		vin-supply = <&vcc12v_dcin>;
+		vin-supply = <&vcc5v0_sys>;
+	};
+
+	vcc3v3_wf: vcc3v3-wf-regulator {
+		compatible = "regulator-fixed";
+		regulator-name = "vcc3v3_wf";
+		regulator-always-on;
+		regulator-boot-on;
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+		vin-supply = <&vcc3v3_sys>;
 	};
 
 	vcc5v0_sys: vcc5v0-sys-regulator {
@@ -592,9 +602,9 @@ rgmii_phy1: ethernet-phy@0 {
 
 &pcie2x1 {
 	pinctrl-names = "default";
-	pinctrl-0 = <&pcie_reset_h>;
+	pinctrl-0 = <&pcie20_perstn_m1>;
 	reset-gpios = <&gpio3 RK_PC1 GPIO_ACTIVE_HIGH>;
-	vpcie3v3-supply = <&vcc3v3_pcie>;
+	vpcie3v3-supply = <&vcc3v3_wf>;
 	status = "okay";
 };
 
@@ -643,12 +653,12 @@ led_user_en: led_user_en {
 	};
 
 	pcie {
-		pcie_enable_h: pcie-enable-h {
-			rockchip,pins = <0 RK_PD4 RK_FUNC_GPIO &pcfg_pull_none>;
+		pcie20_perstn_m1: pcie20-perstn-m1 {
+			rockchip,pins = <3 RK_PC1 RK_FUNC_GPIO &pcfg_pull_none>;
 		};
 
-		pcie_reset_h: pcie-reset-h {
-			rockchip,pins = <3 RK_PC1 RK_FUNC_GPIO &pcfg_pull_none>;
+		pcie_enable_h: pcie-enable-h {
+			rockchip,pins = <0 RK_PD4 RK_FUNC_GPIO &pcfg_pull_none>;
 		};
 	};
 
-- 
2.43.0


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

[PATCH v3 2/2] arm64: dts: rockchip: make PCIe3 (M.2 M key) work for Radxa ROCK 3A

[FUKAUMI Naoki]

on Radxa ROCK 3A, GPIO0_D4 is used to enable both pi6c PCIe clock
generator and "vcc3v3_pcie" regulator (PCIe3 M.2 M key connector).

since pi6c needs to be enabled before using PCIe3, GPIO0_D4 need to be
controlled by "vcc3v3_pi6c_03" regulator. then, make "vcc3v3_pi6c_03"
vin-supply for "vcc3v3_pcie30x1".

also, "pcie30_avdd0v9" and "pcie30_avdd1v8" are unused. remove them.

Fixes: 0522cd811220 ("arm64: dts: rockchip: Add PCIe v3 nodes to rock-3a")
Signed-off-by: FUKAUMI Naoki <naoki@radxa.com>
---
Changes in v3:
- fix pinctrl just for reset pin as GPIO
Changes in v2:
- split patches for PCIe2 and PCIe3
- change regulator name from "vcc3v3_pcie" to "vcc3v3_pcie30x1"
- add comment for vin-supply of "vcc3v3_pcie30x1" regulator
- remove unused "pcie30_avdd0v9" and "pcie30_avdd1v8"
- fix pinctrl node name to overwrite rk3568-pinctrl.dtsi
---
 .../boot/dts/rockchip/rk3568-rock-3a.dts      | 45 +++++++------------
 1 file changed, 15 insertions(+), 30 deletions(-)

diff --git a/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts b/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
index f94cbddf0f0c2..6b3f3ee7f22c7 100644
--- a/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
@@ -86,29 +86,13 @@ vcc12v_dcin: vcc12v-dcin-regulator {
 		regulator-boot-on;
 	};
 
-	pcie30_avdd0v9: pcie30-avdd0v9-regulator {
-		compatible = "regulator-fixed";
-		regulator-name = "pcie30_avdd0v9";
-		regulator-always-on;
-		regulator-boot-on;
-		regulator-min-microvolt = <900000>;
-		regulator-max-microvolt = <900000>;
-		vin-supply = <&vcc3v3_sys>;
-	};
-
-	pcie30_avdd1v8: pcie30-avdd1v8-regulator {
-		compatible = "regulator-fixed";
-		regulator-name = "pcie30_avdd1v8";
-		regulator-always-on;
-		regulator-boot-on;
-		regulator-min-microvolt = <1800000>;
-		regulator-max-microvolt = <1800000>;
-		vin-supply = <&vcc3v3_sys>;
-	};
-
 	/* pi6c pcie clock generator */
 	vcc3v3_pi6c_03: vcc3v3-pi6c-03-regulator {
 		compatible = "regulator-fixed";
+		enable-active-high;
+		gpios = <&gpio0 RK_PD4 GPIO_ACTIVE_HIGH>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&pcie_pwren_h>;
 		regulator-name = "vcc3v3_pi6c_03";
 		regulator-always-on;
 		regulator-boot-on;
@@ -117,16 +101,13 @@ vcc3v3_pi6c_03: vcc3v3-pi6c-03-regulator {
 		vin-supply = <&vcc5v0_sys>;
 	};
 
-	vcc3v3_pcie: vcc3v3-pcie-regulator {
+	/* actually fed by vcc5v0_sys, dependent on pi6c clock generator */
+	vcc3v3_pcie30x1: vcc3v3-pcie30x1-regulator {
 		compatible = "regulator-fixed";
-		enable-active-high;
-		gpios = <&gpio0 RK_PD4 GPIO_ACTIVE_HIGH>;
-		pinctrl-names = "default";
-		pinctrl-0 = <&pcie_enable_h>;
-		regulator-name = "vcc3v3_pcie";
+		regulator-name = "vcc3v3_pcie30x1";
 		regulator-min-microvolt = <3300000>;
 		regulator-max-microvolt = <3300000>;
-		vin-supply = <&vcc5v0_sys>;
+		vin-supply = <&vcc3v3_pi6c_03>;
 	};
 
 	vcc3v3_sys: vcc3v3-sys-regulator {
@@ -615,9 +596,9 @@ &pcie30phy {
 
 &pcie3x2 {
 	pinctrl-names = "default";
-	pinctrl-0 = <&pcie30x2m1_pins>;
+	pinctrl-0 = <&pcie30x2_perstn_m1>;
 	reset-gpios = <&gpio2 RK_PD6 GPIO_ACTIVE_HIGH>;
-	vpcie3v3-supply = <&vcc3v3_pcie>;
+	vpcie3v3-supply = <&vcc3v3_pcie30x1>;
 	status = "okay";
 };
 
@@ -657,7 +638,11 @@ pcie20_perstn_m1: pcie20-perstn-m1 {
 			rockchip,pins = <3 RK_PC1 RK_FUNC_GPIO &pcfg_pull_none>;
 		};
 
-		pcie_enable_h: pcie-enable-h {
+		pcie30x2_perstn_m1: pcie30x2-perstn-m1 {
+			rockchip,pins = <2 RK_PD6 RK_FUNC_GPIO &pcfg_pull_none>;
+		};
+
+		pcie_pwren_h: pcie-pwren-h {
 			rockchip,pins = <0 RK_PD4 RK_FUNC_GPIO &pcfg_pull_none>;
 		};
 	};
-- 
2.43.0


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [PATCH v3 1/2] arm64: dts: rockchip: fix PCIe2 regulators for Radxa ROCK 3A

[FUKAUMI Naoki]

Hi,

can anyone review this series?

Best regards,

--
FUKAUMI Naoki
Radxa Computer (Shenzhen) Co., Ltd.

On 9/16/24 10:40, FUKAUMI Naoki wrote:
> currently "vcc3v3_pcie" regulator is used for PCIe2 M.2 E key
> connector, but, by schematic[1], it's incorrect. "vcc3v3_wf" regulator
> is correct one. add it and fix related vin-supply.
> 
> [1] https://dl.radxa.com/rock3/docs/hw/3a/radxa_rock_3a_v1310_schematic.pdf
> 
> Fixes: 0fbbfb0b00d1 ("arm64: dts: rockchip: Enable PCIe controller on rock3a")
> Signed-off-by: FUKAUMI Naoki <naoki@radxa.com>
> ---
> Changes in v3:
> - fix pinctrl just for reset pin as GPIO
> Changes in v2:
> - split patches for PCIe2 and PCIe3
> - fix pinctrl node name to overwrite rk3568-pinctrl.dtsi
> ---
>   .../boot/dts/rockchip/rk3568-rock-3a.dts      | 24 +++++++++++++------
>   1 file changed, 17 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts b/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
> index 59f1403b4fa56..f94cbddf0f0c2 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
> +++ b/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
> @@ -136,7 +136,17 @@ vcc3v3_sys: vcc3v3-sys-regulator {
>   		regulator-boot-on;
>   		regulator-min-microvolt = <3300000>;
>   		regulator-max-microvolt = <3300000>;
> -		vin-supply = <&vcc12v_dcin>;
> +		vin-supply = <&vcc5v0_sys>;
> +	};
> +
> +	vcc3v3_wf: vcc3v3-wf-regulator {
> +		compatible = "regulator-fixed";
> +		regulator-name = "vcc3v3_wf";
> +		regulator-always-on;
> +		regulator-boot-on;
> +		regulator-min-microvolt = <3300000>;
> +		regulator-max-microvolt = <3300000>;
> +		vin-supply = <&vcc3v3_sys>;
>   	};
>   
>   	vcc5v0_sys: vcc5v0-sys-regulator {
> @@ -592,9 +602,9 @@ rgmii_phy1: ethernet-phy@0 {
>   
>   &pcie2x1 {
>   	pinctrl-names = "default";
> -	pinctrl-0 = <&pcie_reset_h>;
> +	pinctrl-0 = <&pcie20_perstn_m1>;
>   	reset-gpios = <&gpio3 RK_PC1 GPIO_ACTIVE_HIGH>;
> -	vpcie3v3-supply = <&vcc3v3_pcie>;
> +	vpcie3v3-supply = <&vcc3v3_wf>;
>   	status = "okay";
>   };
>   
> @@ -643,12 +653,12 @@ led_user_en: led_user_en {
>   	};
>   
>   	pcie {
> -		pcie_enable_h: pcie-enable-h {
> -			rockchip,pins = <0 RK_PD4 RK_FUNC_GPIO &pcfg_pull_none>;
> +		pcie20_perstn_m1: pcie20-perstn-m1 {
> +			rockchip,pins = <3 RK_PC1 RK_FUNC_GPIO &pcfg_pull_none>;
>   		};
>   
> -		pcie_reset_h: pcie-reset-h {
> -			rockchip,pins = <3 RK_PC1 RK_FUNC_GPIO &pcfg_pull_none>;
> +		pcie_enable_h: pcie-enable-h {
> +			rockchip,pins = <0 RK_PD4 RK_FUNC_GPIO &pcfg_pull_none>;
>   		};
>   	};
>   

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [PATCH v3 1/2] arm64: dts: rockchip: fix PCIe2 regulators for Radxa ROCK 3A

[Heiko Stübner]

Hi,

Am Montag, 16. September 2024, 03:40:38 CEST schrieb FUKAUMI Naoki:
> currently "vcc3v3_pcie" regulator is used for PCIe2 M.2 E key
> connector, but, by schematic[1], it's incorrect. "vcc3v3_wf" regulator
> is correct one. add it and fix related vin-supply.
> 
> [1] https://dl.radxa.com/rock3/docs/hw/3a/radxa_rock_3a_v1310_schematic.pdf
> 
> Fixes: 0fbbfb0b00d1 ("arm64: dts: rockchip: Enable PCIe controller on rock3a")
> Signed-off-by: FUKAUMI Naoki <naoki@radxa.com>
> ---
> Changes in v3:
> - fix pinctrl just for reset pin as GPIO
> Changes in v2:
> - split patches for PCIe2 and PCIe3
> - fix pinctrl node name to overwrite rk3568-pinctrl.dtsi
> ---
>  .../boot/dts/rockchip/rk3568-rock-3a.dts      | 24 +++++++++++++------
>  1 file changed, 17 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts b/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
> index 59f1403b4fa56..f94cbddf0f0c2 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
> +++ b/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
> @@ -136,7 +136,17 @@ vcc3v3_sys: vcc3v3-sys-regulator {
>  		regulator-boot-on;
>  		regulator-min-microvolt = <3300000>;
>  		regulator-max-microvolt = <3300000>;
> -		vin-supply = <&vcc12v_dcin>;
> +		vin-supply = <&vcc5v0_sys>;
> +	};
> +
> +	vcc3v3_wf: vcc3v3-wf-regulator {
> +		compatible = "regulator-fixed";
> +		regulator-name = "vcc3v3_wf";
> +		regulator-always-on;
> +		regulator-boot-on;
> +		regulator-min-microvolt = <3300000>;
> +		regulator-max-microvolt = <3300000>;
> +		vin-supply = <&vcc3v3_sys>;
>  	};
>  
>  	vcc5v0_sys: vcc5v0-sys-regulator {
> @@ -592,9 +602,9 @@ rgmii_phy1: ethernet-phy@0 {
>  
>  &pcie2x1 {
>  	pinctrl-names = "default";
> -	pinctrl-0 = <&pcie_reset_h>;
> +	pinctrl-0 = <&pcie20_perstn_m1>;

the patch is about fixing regulator supplies for the pcie controller,
not about changing the pinctrl. So this change is quite unrelated and
definitly needs a separate patch.


Heiko



_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [PATCH v3 2/2] arm64: dts: rockchip: make PCIe3 (M.2 M key) work for Radxa ROCK 3A

[Heiko Stübner]

Hi,

Am Montag, 16. September 2024, 03:40:39 CEST schrieb FUKAUMI Naoki:
> on Radxa ROCK 3A, GPIO0_D4 is used to enable both pi6c PCIe clock
> generator and "vcc3v3_pcie" regulator (PCIe3 M.2 M key connector).

So now I've gone ahead and looked up a rock 3a schematics.

By the way, the Radxa site is broken. On
https://wiki.radxa.com/Rock3/hardware
the v1.3 schematics link only leads to a not-found page.


> since pi6c needs to be enabled before using PCIe3, GPIO0_D4 need to be
> controlled by "vcc3v3_pi6c_03" regulator. then, make "vcc3v3_pi6c_03"
> vin-supply for "vcc3v3_pcie30x1".

You're hacking around the thing here.

That gpio0-d4 controls two separate regulators.
- the one creating VCC3V3_PCIE30X1
- the one creating VCC3V3_PI6C_03

Both are directly supplied from VCC5V0_SYS .

So please don't create artificial hierarchies where none exist.

Instead you could go with just having multiple phandles, like for example
the vcc5v0_usb* set of regulators on the rock 5 itx:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/rockchip/rk3588-rock-5-itx.dts#n182


> also, "pcie30_avdd0v9" and "pcie30_avdd1v8" are unused. remove them.
> 
> Fixes: 0522cd811220 ("arm64: dts: rockchip: Add PCIe v3 nodes to rock-3a")
> Signed-off-by: FUKAUMI Naoki <naoki@radxa.com>
> ---
> Changes in v3:
> - fix pinctrl just for reset pin as GPIO
> Changes in v2:
> - split patches for PCIe2 and PCIe3
> - change regulator name from "vcc3v3_pcie" to "vcc3v3_pcie30x1"
> - add comment for vin-supply of "vcc3v3_pcie30x1" regulator
> - remove unused "pcie30_avdd0v9" and "pcie30_avdd1v8"
> - fix pinctrl node name to overwrite rk3568-pinctrl.dtsi
> ---
>  .../boot/dts/rockchip/rk3568-rock-3a.dts      | 45 +++++++------------
>  1 file changed, 15 insertions(+), 30 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts b/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
> index f94cbddf0f0c2..6b3f3ee7f22c7 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
> +++ b/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
> @@ -86,29 +86,13 @@ vcc12v_dcin: vcc12v-dcin-regulator {
>  		regulator-boot-on;
>  	};
>  
> -	pcie30_avdd0v9: pcie30-avdd0v9-regulator {
> -		compatible = "regulator-fixed";
> -		regulator-name = "pcie30_avdd0v9";
> -		regulator-always-on;
> -		regulator-boot-on;
> -		regulator-min-microvolt = <900000>;
> -		regulator-max-microvolt = <900000>;
> -		vin-supply = <&vcc3v3_sys>;
> -	};
> -
> -	pcie30_avdd1v8: pcie30-avdd1v8-regulator {
> -		compatible = "regulator-fixed";
> -		regulator-name = "pcie30_avdd1v8";
> -		regulator-always-on;
> -		regulator-boot-on;
> -		regulator-min-microvolt = <1800000>;
> -		regulator-max-microvolt = <1800000>;
> -		vin-supply = <&vcc3v3_sys>;
> -	};
> -

This change looks correct, but please make this a separate patch.
One of the pmic regulators provides these voltage inputs to the soc
side of the pcie controller.


>  	/* pi6c pcie clock generator */
>  	vcc3v3_pi6c_03: vcc3v3-pi6c-03-regulator {
>  		compatible = "regulator-fixed";
> +		enable-active-high;
> +		gpios = <&gpio0 RK_PD4 GPIO_ACTIVE_HIGH>;
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&pcie_pwren_h>;
>  		regulator-name = "vcc3v3_pi6c_03";
>  		regulator-always-on;
>  		regulator-boot-on;
> @@ -117,16 +101,13 @@ vcc3v3_pi6c_03: vcc3v3-pi6c-03-regulator {
>  		vin-supply = <&vcc5v0_sys>;
>  	};
>  
> -	vcc3v3_pcie: vcc3v3-pcie-regulator {
> +	/* actually fed by vcc5v0_sys, dependent on pi6c clock generator */
> +	vcc3v3_pcie30x1: vcc3v3-pcie30x1-regulator {
>  		compatible = "regulator-fixed";
> -		enable-active-high;
> -		gpios = <&gpio0 RK_PD4 GPIO_ACTIVE_HIGH>;
> -		pinctrl-names = "default";
> -		pinctrl-0 = <&pcie_enable_h>;
> -		regulator-name = "vcc3v3_pcie";
> +		regulator-name = "vcc3v3_pcie30x1";
>  		regulator-min-microvolt = <3300000>;
>  		regulator-max-microvolt = <3300000>;
> -		vin-supply = <&vcc5v0_sys>;
> +		vin-supply = <&vcc3v3_pi6c_03>;
>  	};
>  
>  	vcc3v3_sys: vcc3v3-sys-regulator {
> @@ -615,9 +596,9 @@ &pcie30phy {
>  
>  &pcie3x2 {
>  	pinctrl-names = "default";
> -	pinctrl-0 = <&pcie30x2m1_pins>;
> +	pinctrl-0 = <&pcie30x2_perstn_m1>;

you're again mixing in pinctrl changes into a patch about the
regulator hirarchy. Please don't do that.


Heiko



_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip