diff mbox series

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

Message ID 20240913230604.696-1-naoki@radxa.com (mailing list archive)
State New
Headers show
Series [v2,1/2] arm64: dts: rockchip: fix PCIe2 regulators for Radxa ROCK 3A | expand

Commit Message

FUKAUMI Naoki Sept. 13, 2024, 11:06 p.m. UTC
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.

changes for pinctrl are to claim all 3 pins instead of only reset pin
as like as rk3568-rock-3b.dts.

[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 v2:
- split patches for PCIe2 and PCIe3
- fix pinctrl node name to overwrite rk3568-pinctrl.dtsi
---
 .../boot/dts/rockchip/rk3568-rock-3a.dts      | 28 +++++++++++++++----
 1 file changed, 23 insertions(+), 5 deletions(-)

Comments

Kever Yang Sept. 14, 2024, 7:54 a.m. UTC | #1
Hi Naoki,

On 2024/9/14 07:06, 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.
The fix is fine.
> changes for pinctrl are to claim all 3 pins instead of only reset pin
> as like as rk3568-rock-3b.dts.

I may missing some discussion about this, any help about this?

Seems the rk3568-rock-3b.dts is from Jonas, so I add Jonas to the mail.

> [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 v2:
> - split patches for PCIe2 and PCIe3
> - fix pinctrl node name to overwrite rk3568-pinctrl.dtsi
> ---
>   .../boot/dts/rockchip/rk3568-rock-3a.dts      | 28 +++++++++++++++----
>   1 file changed, 23 insertions(+), 5 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..6f6b31b4442cd 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 = <&pcie20m1_pins>;
>   	reset-gpios = <&gpio3 RK_PC1 GPIO_ACTIVE_HIGH>;
> -	vpcie3v3-supply = <&vcc3v3_pcie>;
> +	vpcie3v3-supply = <&vcc3v3_wf>;
>   	status = "okay";
>   };
>   
> @@ -646,9 +656,17 @@ pcie {
>   		pcie_enable_h: pcie-enable-h {
>   			rockchip,pins = <0 RK_PD4 RK_FUNC_GPIO &pcfg_pull_none>;
>   		};
> +	};
>   
> -		pcie_reset_h: pcie-reset-h {
> -			rockchip,pins = <3 RK_PC1 RK_FUNC_GPIO &pcfg_pull_none>;
> +	pcie20 {
> +		pcie20m1_pins: pcie20m1-pins {
> +			rockchip,pins =
> +				/* pcie20_clkreqnm1 */
> +				<2 RK_PD0 4 &pcfg_pull_none>,

The CLKREQ is used when L1 Substate is enabled, and seems like we don't 
actually use this feature,

we may need an "supports-clkreq" like nvidia platform to support the 
clkreq signal.

Keep the CLKREQ as default GPIO function is a better choice before we 
really need it.

> +				/* pcie20_perstnm1 */
> +				<3 RK_PC1 RK_FUNC_GPIO &pcfg_pull_none>,
> +				/* pcie20_wakenm1 */
> +				<2 RK_PD1 4 &pcfg_pull_none>;

I believe no one is using the wake up function with mainline code, 
because this is a sideband wakeup

by GPIO interrupt to CPU, not by PCIe controller.

So the SoC default setting as GPIO should be correct instead of set it 
as PCIe function.


Thanks,
- Kever
>   		};
>   	};
>
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts b/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
index 59f1403b4fa56..6f6b31b4442cd 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 = <&pcie20m1_pins>;
 	reset-gpios = <&gpio3 RK_PC1 GPIO_ACTIVE_HIGH>;
-	vpcie3v3-supply = <&vcc3v3_pcie>;
+	vpcie3v3-supply = <&vcc3v3_wf>;
 	status = "okay";
 };
 
@@ -646,9 +656,17 @@  pcie {
 		pcie_enable_h: pcie-enable-h {
 			rockchip,pins = <0 RK_PD4 RK_FUNC_GPIO &pcfg_pull_none>;
 		};
+	};
 
-		pcie_reset_h: pcie-reset-h {
-			rockchip,pins = <3 RK_PC1 RK_FUNC_GPIO &pcfg_pull_none>;
+	pcie20 {
+		pcie20m1_pins: pcie20m1-pins {
+			rockchip,pins =
+				/* pcie20_clkreqnm1 */
+				<2 RK_PD0 4 &pcfg_pull_none>,
+				/* pcie20_perstnm1 */
+				<3 RK_PC1 RK_FUNC_GPIO &pcfg_pull_none>,
+				/* pcie20_wakenm1 */
+				<2 RK_PD1 4 &pcfg_pull_none>;
 		};
 	};