linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/4] arm64: qcom: x1e80100-qcp: Add power supply and sideband signals for PCIe RC
@ 2025-04-25  9:29 Wenbin Yao
  2025-04-25  9:29 ` [PATCH v2 1/4] arm64: Kconfig: enable PCI Power Control Slot driver for QCOM Wenbin Yao
                   ` (3 more replies)
  0 siblings, 4 replies; 19+ messages in thread
From: Wenbin Yao @ 2025-04-25  9:29 UTC (permalink / raw)
  To: catalin.marinas, will, linux-arm-kernel, andersson, konradybcio,
	robh, krzk+dt, conor+dt, linux-arm-msm, devicetree, linux-kernel
  Cc: krishna.chundru, quic_vbadigan, quic_mrana, quic_cang,
	quic_qianyu, quic_wenbyao

The first patch enables the PCI Power Control driver to control the power
state of PCI slots. The second patch adds the bus topology of PCIe domain 3
on x1e80100 platform. The third patch adds perst, wake and clkreq sideband
signals, and describe the regulators powering the rails of the PCI slots in
the devicetree for PCIe3 controller and PHY device. The fourth patch adds
qref supply for PCIe PHYs.

The patchset has been modified based on comments and suggestions.

Changes in v2:
- Select PCI_PWRCTL_SLOT by ARCH_QCOM in arch/arm64/Kconfig.platforms in
  Patch 1/4.
- Add an empty line before pcie3port node in Patch 2/4.
- Rename regulator-pcie_12v regulator-pcie_3v3_aux and regulator-pcie_3v3
  in Patch 3/4.
- Add Patch 4/4 to describe qref supply of PCIe PHYs.
- Link to v1: https://lore.kernel.org/all/20250320055502.274849-1-quic_wenbyao@quicinc.com/

Qiang Yu (4):
  arm64: Kconfig: enable PCI Power Control Slot driver for QCOM
  arm64: dts: qcom: x1e80100: add bus topology for PCIe domain 3
  arm64: dts: qcom: x1e80100-qcp: enable pcie3 x8 slot for X1E80100-QCP
  arm64: dts: qcom: x1e80100-qcp: Add qref supply for PCIe PHYs

 arch/arm64/Kconfig.platforms              |   1 +
 arch/arm64/boot/dts/qcom/x1e80100-qcp.dts | 121 ++++++++++++++++++++++
 arch/arm64/boot/dts/qcom/x1e80100.dtsi    |  11 ++
 3 files changed, 133 insertions(+)


base-commit: bc8aa6cdadcc00862f2b5720e5de2e17f696a081
prerequisite-patch-id: 8d8c88ca71e145f5f1c5145d9ff3ebe90101aab7
-- 
2.34.1



^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH v2 1/4] arm64: Kconfig: enable PCI Power Control Slot driver for QCOM
  2025-04-25  9:29 [PATCH v2 0/4] arm64: qcom: x1e80100-qcp: Add power supply and sideband signals for PCIe RC Wenbin Yao
@ 2025-04-25  9:29 ` Wenbin Yao
  2025-04-25  9:47   ` Johan Hovold
  2025-04-25  9:29 ` [PATCH v2 2/4] arm64: dts: qcom: x1e80100: add bus topology for PCIe domain 3 Wenbin Yao
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 19+ messages in thread
From: Wenbin Yao @ 2025-04-25  9:29 UTC (permalink / raw)
  To: catalin.marinas, will, linux-arm-kernel, andersson, konradybcio,
	robh, krzk+dt, conor+dt, linux-arm-msm, devicetree, linux-kernel
  Cc: krishna.chundru, quic_vbadigan, quic_mrana, quic_cang,
	quic_qianyu, quic_wenbyao

From: Qiang Yu <quic_qianyu@quicinc.com>

Enable the pwrctrl driver, which is utilized to manage the power supplies
of the devices connected to the PCI slots. This ensures that the voltage
rails of the standard PCI slots on some platforms eg. X1E80100-QCP can be
correctly turned on/off if they are described under PCIe port device tree
node.

Signed-off-by: Qiang Yu <quic_qianyu@quicinc.com>
Signed-off-by: Wenbin Yao <quic_wenbyao@quicinc.com>
---
 arch/arm64/Kconfig.platforms | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms
index 8b76821f1..14d2742c8 100644
--- a/arch/arm64/Kconfig.platforms
+++ b/arch/arm64/Kconfig.platforms
@@ -270,6 +270,7 @@ config ARCH_QCOM
 	select GPIOLIB
 	select PINCTRL
 	select HAVE_PWRCTL if PCI
+	select PCI_PWRCTL_SLOT if PCI
 	help
 	  This enables support for the ARMv8 based Qualcomm chipsets.
 
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v2 2/4] arm64: dts: qcom: x1e80100: add bus topology for PCIe domain 3
  2025-04-25  9:29 [PATCH v2 0/4] arm64: qcom: x1e80100-qcp: Add power supply and sideband signals for PCIe RC Wenbin Yao
  2025-04-25  9:29 ` [PATCH v2 1/4] arm64: Kconfig: enable PCI Power Control Slot driver for QCOM Wenbin Yao
@ 2025-04-25  9:29 ` Wenbin Yao
  2025-04-25 10:22   ` Konrad Dybcio
  2025-04-25  9:29 ` [PATCH v2 3/4] arm64: dts: qcom: x1e80100-qcp: enable pcie3 x8 slot for X1E80100-QCP Wenbin Yao
  2025-04-25  9:29 ` [PATCH v2 4/4] arm64: dts: qcom: x1e80100-qcp: Add qref supply for PCIe PHYs Wenbin Yao
  3 siblings, 1 reply; 19+ messages in thread
From: Wenbin Yao @ 2025-04-25  9:29 UTC (permalink / raw)
  To: catalin.marinas, will, linux-arm-kernel, andersson, konradybcio,
	robh, krzk+dt, conor+dt, linux-arm-msm, devicetree, linux-kernel
  Cc: krishna.chundru, quic_vbadigan, quic_mrana, quic_cang,
	quic_qianyu, quic_wenbyao

From: Qiang Yu <quic_qianyu@quicinc.com>

Add pcie3port node to represent the PCIe bridge of PCIe3 so that PCI slot
voltage rails can be described under this node in the board's dts.

Signed-off-by: Qiang Yu <quic_qianyu@quicinc.com>
Signed-off-by: Wenbin Yao <quic_wenbyao@quicinc.com>
---
 arch/arm64/boot/dts/qcom/x1e80100.dtsi | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/x1e80100.dtsi b/arch/arm64/boot/dts/qcom/x1e80100.dtsi
index 46b79fce9..430f9d567 100644
--- a/arch/arm64/boot/dts/qcom/x1e80100.dtsi
+++ b/arch/arm64/boot/dts/qcom/x1e80100.dtsi
@@ -3287,6 +3287,17 @@ opp-128000000 {
 					opp-peak-kBps = <15753000 1>;
 				};
 			};
+
+			pcie3port: pcie@0 {
+				device_type = "pci";
+				compatible = "pciclass,0604";
+				reg = <0x0 0x0 0x0 0x0 0x0>;
+				bus-range = <0x01 0xff>;
+
+				#address-cells = <3>;
+				#size-cells = <2>;
+				ranges;
+			};
 		};
 
 		pcie3_phy: phy@1be0000 {
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v2 3/4] arm64: dts: qcom: x1e80100-qcp: enable pcie3 x8 slot for X1E80100-QCP
  2025-04-25  9:29 [PATCH v2 0/4] arm64: qcom: x1e80100-qcp: Add power supply and sideband signals for PCIe RC Wenbin Yao
  2025-04-25  9:29 ` [PATCH v2 1/4] arm64: Kconfig: enable PCI Power Control Slot driver for QCOM Wenbin Yao
  2025-04-25  9:29 ` [PATCH v2 2/4] arm64: dts: qcom: x1e80100: add bus topology for PCIe domain 3 Wenbin Yao
@ 2025-04-25  9:29 ` Wenbin Yao
  2025-04-25 10:09   ` Konrad Dybcio
  2025-04-25  9:29 ` [PATCH v2 4/4] arm64: dts: qcom: x1e80100-qcp: Add qref supply for PCIe PHYs Wenbin Yao
  3 siblings, 1 reply; 19+ messages in thread
From: Wenbin Yao @ 2025-04-25  9:29 UTC (permalink / raw)
  To: catalin.marinas, will, linux-arm-kernel, andersson, konradybcio,
	robh, krzk+dt, conor+dt, linux-arm-msm, devicetree, linux-kernel
  Cc: krishna.chundru, quic_vbadigan, quic_mrana, quic_cang,
	quic_qianyu, quic_wenbyao

From: Qiang Yu <quic_qianyu@quicinc.com>

Add perst, wake and clkreq sideband signals and required regulators in
PCIe3 controller and PHY device tree node. Describe the voltage rails of
the x8 PCI slots for PCIe3 port.

Signed-off-by: Qiang Yu <quic_qianyu@quicinc.com>
Signed-off-by: Wenbin Yao <quic_wenbyao@quicinc.com>
---
 arch/arm64/boot/dts/qcom/x1e80100-qcp.dts | 118 ++++++++++++++++++++++
 1 file changed, 118 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/x1e80100-qcp.dts b/arch/arm64/boot/dts/qcom/x1e80100-qcp.dts
index 470c4f826..88dfd2199 100644
--- a/arch/arm64/boot/dts/qcom/x1e80100-qcp.dts
+++ b/arch/arm64/boot/dts/qcom/x1e80100-qcp.dts
@@ -318,6 +318,48 @@ vreg_wcn_3p3: regulator-wcn-3p3 {
 		regulator-boot-on;
 	};
 
+	vreg_pcie_12v: regulator-pcie-12v {
+		compatible = "regulator-fixed";
+
+		regulator-name = "VREG_PCIE_12V";
+		regulator-min-microvolt = <12000000>;
+		regulator-max-microvolt = <12000000>;
+
+		gpio = <&pm8550ve_8_gpios 8 GPIO_ACTIVE_HIGH>;
+		enable-active-high;
+
+		pinctrl-names = "default";
+		pinctrl-0 = <&pcie_x8_12v>;
+	};
+
+	vreg_pcie_3v3_aux: regulator-pcie-3v3-aux {
+		compatible = "regulator-fixed";
+
+		regulator-name = "VREG_PCIE_3P3_AUX";
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+
+		gpio = <&pmc8380_3_gpios 8 GPIO_ACTIVE_HIGH>;
+		enable-active-high;
+
+		pinctrl-names = "default";
+		pinctrl-0 = <&pm_sde7_aux_3p3_en>;
+	};
+
+	vreg_pcie_3v3: regulator-pcie-3v3 {
+		compatible = "regulator-fixed";
+
+		regulator-name = "VREG_PCIE_3P3";
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+
+		gpio = <&pmc8380_3_gpios 6 GPIO_ACTIVE_HIGH>;
+		enable-active-high;
+
+		pinctrl-names = "default";
+		pinctrl-0 = <&pm_sde7_main_3p3_en>;
+};
+
 	usb-1-ss0-sbu-mux {
 		compatible = "onnn,fsusb42", "gpio-sbu-mux";
 
@@ -909,6 +951,59 @@ &mdss_dp3_phy {
 	status = "okay";
 };
 
+&pm8550ve_8_gpios {
+	pcie_x8_12v: pcie-12v-default-state {
+		pins = "gpio8";
+		function = "normal";
+		output-enable;
+		output-high;
+		bias-pull-down;
+		power-source = <0>;
+	};
+};
+
+&pmc8380_3_gpios {
+	pm_sde7_aux_3p3_en: pcie-aux-3p3-default-state {
+		pins = "gpio8";
+		function = "normal";
+		output-enable;
+		output-high;
+		bias-pull-down;
+		power-source = <0>;
+	};
+
+	pm_sde7_main_3p3_en: pcie-main-3p3-default-state {
+		pins = "gpio6";
+		function = "normal";
+		output-enable;
+		output-high;
+		bias-pull-down;
+		power-source = <0>;
+	};
+};
+
+&pcie3 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&pcie3_default>;
+	perst-gpios = <&tlmm 143 GPIO_ACTIVE_LOW>;
+	wake-gpios = <&tlmm 145 GPIO_ACTIVE_LOW>;
+
+	status = "okay";
+};
+
+&pcie3_phy {
+	vdda-phy-supply = <&vreg_l3c_0p8>;
+	vdda-pll-supply = <&vreg_l3e_1p2>;
+
+	status = "okay";
+};
+
+&pcie3port {
+	vpcie12v-supply = <&vreg_pcie_12v>;
+	vpcie3v3-supply = <&vreg_pcie_3v3>;
+	vpcie3v3aux-supply = <&vreg_pcie_3v3_aux>;
+};
+
 &pcie4 {
 	perst-gpios = <&tlmm 146 GPIO_ACTIVE_LOW>;
 	wake-gpios = <&tlmm 148 GPIO_ACTIVE_LOW>;
@@ -1120,6 +1215,29 @@ nvme_reg_en: nvme-reg-en-state {
 		bias-disable;
 	};
 
+	pcie3_default: pcie3-default-state {
+		clkreq-n-pins {
+			pins = "gpio144";
+			function = "pcie3_clk";
+			drive-strength = <2>;
+			bias-pull-up;
+		};
+
+		perst-n-pins {
+			pins = "gpio143";
+			function = "gpio";
+			drive-strength = <2>;
+			bias-pull-down;
+		};
+
+		wake-n-pins {
+			pins = "gpio145";
+			function = "gpio";
+			drive-strength = <2>;
+			bias-pull-up;
+		};
+	};
+
 	pcie4_default: pcie4-default-state {
 		clkreq-n-pins {
 			pins = "gpio147";
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v2 4/4] arm64: dts: qcom: x1e80100-qcp: Add qref supply for PCIe PHYs
  2025-04-25  9:29 [PATCH v2 0/4] arm64: qcom: x1e80100-qcp: Add power supply and sideband signals for PCIe RC Wenbin Yao
                   ` (2 preceding siblings ...)
  2025-04-25  9:29 ` [PATCH v2 3/4] arm64: dts: qcom: x1e80100-qcp: enable pcie3 x8 slot for X1E80100-QCP Wenbin Yao
@ 2025-04-25  9:29 ` Wenbin Yao
  2025-04-25  9:51   ` Johan Hovold
  3 siblings, 1 reply; 19+ messages in thread
From: Wenbin Yao @ 2025-04-25  9:29 UTC (permalink / raw)
  To: catalin.marinas, will, linux-arm-kernel, andersson, konradybcio,
	robh, krzk+dt, conor+dt, linux-arm-msm, devicetree, linux-kernel
  Cc: krishna.chundru, quic_vbadigan, quic_mrana, quic_cang,
	quic_qianyu, quic_wenbyao

From: Qiang Yu <quic_qianyu@quicinc.com>

All PCIe PHYs on X1E80100 require vdda-qref power supplies, but this is
missing in the current PHY device tree node. The PCIe port can still
function because the regulator L3J, which vdda-qref consumes, is voted by
other components.

Since the device tree should accurately describe the hardware, add the
vdda-qref power supply explicitly in all PCIe PHY device nodes.

Signed-off-by: Qiang Yu <quic_qianyu@quicinc.com>
Signed-off-by: Wenbin Yao <quic_wenbyao@quicinc.com>
---
 arch/arm64/boot/dts/qcom/x1e80100-qcp.dts | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/x1e80100-qcp.dts b/arch/arm64/boot/dts/qcom/x1e80100-qcp.dts
index 88dfd2199..10f2ac70e 100644
--- a/arch/arm64/boot/dts/qcom/x1e80100-qcp.dts
+++ b/arch/arm64/boot/dts/qcom/x1e80100-qcp.dts
@@ -994,6 +994,7 @@ &pcie3 {
 &pcie3_phy {
 	vdda-phy-supply = <&vreg_l3c_0p8>;
 	vdda-pll-supply = <&vreg_l3e_1p2>;
+	vdda-qref-supply = <&vreg_l3j_0p8>;
 
 	status = "okay";
 };
@@ -1017,6 +1018,7 @@ &pcie4 {
 &pcie4_phy {
 	vdda-phy-supply = <&vreg_l3i_0p8>;
 	vdda-pll-supply = <&vreg_l3e_1p2>;
+	vdda-qref-supply = <&vreg_l3j_0p8>;
 
 	status = "okay";
 };
@@ -1053,6 +1055,7 @@ &pcie6a {
 &pcie6a_phy {
 	vdda-phy-supply = <&vreg_l1d_0p8>;
 	vdda-pll-supply = <&vreg_l2j_1p2>;
+	vdda-qref-supply = <&vreg_l3j_0p8>;
 
 	status = "okay";
 };
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 1/4] arm64: Kconfig: enable PCI Power Control Slot driver for QCOM
  2025-04-25  9:29 ` [PATCH v2 1/4] arm64: Kconfig: enable PCI Power Control Slot driver for QCOM Wenbin Yao
@ 2025-04-25  9:47   ` Johan Hovold
  0 siblings, 0 replies; 19+ messages in thread
From: Johan Hovold @ 2025-04-25  9:47 UTC (permalink / raw)
  To: Wenbin Yao
  Cc: catalin.marinas, will, linux-arm-kernel, andersson, konradybcio,
	robh, krzk+dt, conor+dt, linux-arm-msm, devicetree, linux-kernel,
	krishna.chundru, quic_vbadigan, quic_mrana, quic_cang,
	quic_qianyu

On Fri, Apr 25, 2025 at 05:29:52PM +0800, Wenbin Yao wrote:
> From: Qiang Yu <quic_qianyu@quicinc.com>
> 
> Enable the pwrctrl driver, which is utilized to manage the power supplies
> of the devices connected to the PCI slots. This ensures that the voltage
> rails of the standard PCI slots on some platforms eg. X1E80100-QCP can be
> correctly turned on/off if they are described under PCIe port device tree
> node.
> 
> Signed-off-by: Qiang Yu <quic_qianyu@quicinc.com>
> Signed-off-by: Wenbin Yao <quic_wenbyao@quicinc.com>
> ---
>  arch/arm64/Kconfig.platforms | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms
> index 8b76821f1..14d2742c8 100644
> --- a/arch/arm64/Kconfig.platforms
> +++ b/arch/arm64/Kconfig.platforms
> @@ -270,6 +270,7 @@ config ARCH_QCOM
>  	select GPIOLIB
>  	select PINCTRL
>  	select HAVE_PWRCTL if PCI
> +	select PCI_PWRCTL_SLOT if PCI

Note that this symbol is getting renamed for 6.16:

	 https://lore.kernel.org/lkml/20250402132634.18065-2-johan+linaro@kernel.org/

Johan


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 4/4] arm64: dts: qcom: x1e80100-qcp: Add qref supply for PCIe PHYs
  2025-04-25  9:29 ` [PATCH v2 4/4] arm64: dts: qcom: x1e80100-qcp: Add qref supply for PCIe PHYs Wenbin Yao
@ 2025-04-25  9:51   ` Johan Hovold
  2025-04-25 10:03     ` Konrad Dybcio
  0 siblings, 1 reply; 19+ messages in thread
From: Johan Hovold @ 2025-04-25  9:51 UTC (permalink / raw)
  To: Wenbin Yao
  Cc: catalin.marinas, will, linux-arm-kernel, andersson, konradybcio,
	robh, krzk+dt, conor+dt, linux-arm-msm, devicetree, linux-kernel,
	krishna.chundru, quic_vbadigan, quic_mrana, quic_cang,
	quic_qianyu

On Fri, Apr 25, 2025 at 05:29:55PM +0800, Wenbin Yao wrote:
> From: Qiang Yu <quic_qianyu@quicinc.com>
> 
> All PCIe PHYs on X1E80100 require vdda-qref power supplies, but this is
> missing in the current PHY device tree node. The PCIe port can still
> function because the regulator L3J, which vdda-qref consumes, is voted by
> other components.
> 
> Since the device tree should accurately describe the hardware, add the
> vdda-qref power supply explicitly in all PCIe PHY device nodes.

AFAIU the PHYs do not use this qref supply directly so it does not
belong in the PHY node (but possibly in the tcsr node that provides the
refclk).

Since commit 031b46b4729b ("phy: qcom: qmp-pcie: drop bogus x1e80100
qref supplies") it also won't have any effect for pcie4 and pcie6.

Johan


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 4/4] arm64: dts: qcom: x1e80100-qcp: Add qref supply for PCIe PHYs
  2025-04-25  9:51   ` Johan Hovold
@ 2025-04-25 10:03     ` Konrad Dybcio
  2025-04-25 12:02       ` Johan Hovold
  0 siblings, 1 reply; 19+ messages in thread
From: Konrad Dybcio @ 2025-04-25 10:03 UTC (permalink / raw)
  To: Johan Hovold, Wenbin Yao
  Cc: catalin.marinas, will, linux-arm-kernel, andersson, konradybcio,
	robh, krzk+dt, conor+dt, linux-arm-msm, devicetree, linux-kernel,
	krishna.chundru, quic_vbadigan, quic_mrana, quic_cang,
	quic_qianyu

On 4/25/25 11:51 AM, Johan Hovold wrote:
> On Fri, Apr 25, 2025 at 05:29:55PM +0800, Wenbin Yao wrote:
>> From: Qiang Yu <quic_qianyu@quicinc.com>
>>
>> All PCIe PHYs on X1E80100 require vdda-qref power supplies, but this is
>> missing in the current PHY device tree node. The PCIe port can still
>> function because the regulator L3J, which vdda-qref consumes, is voted by
>> other components.
>>
>> Since the device tree should accurately describe the hardware, add the
>> vdda-qref power supply explicitly in all PCIe PHY device nodes.
> 
> AFAIU the PHYs do not use this qref supply directly so it does not
> belong in the PHY node (but possibly in the tcsr node that provides the
> refclk).
> 
> Since commit 031b46b4729b ("phy: qcom: qmp-pcie: drop bogus x1e80100
> qref supplies") it also won't have any effect for pcie4 and pcie6.

QREF is a separate hw block distributing the reference clocks across
certain on-SoC peripherals

If its power goes out, I don't think much of the platform would be
functional anyway, so it's redundant here..

It doesn't have its own single register region and it's frankly
one-shot-configured way before Linux starts up, so there should be
no need of describing it at all.

Konrad


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 3/4] arm64: dts: qcom: x1e80100-qcp: enable pcie3 x8 slot for X1E80100-QCP
  2025-04-25  9:29 ` [PATCH v2 3/4] arm64: dts: qcom: x1e80100-qcp: enable pcie3 x8 slot for X1E80100-QCP Wenbin Yao
@ 2025-04-25 10:09   ` Konrad Dybcio
  0 siblings, 0 replies; 19+ messages in thread
From: Konrad Dybcio @ 2025-04-25 10:09 UTC (permalink / raw)
  To: Wenbin Yao, catalin.marinas, will, linux-arm-kernel, andersson,
	konradybcio, robh, krzk+dt, conor+dt, linux-arm-msm, devicetree,
	linux-kernel
  Cc: krishna.chundru, quic_vbadigan, quic_mrana, quic_cang,
	quic_qianyu

On 4/25/25 11:29 AM, Wenbin Yao wrote:
> From: Qiang Yu <quic_qianyu@quicinc.com>
> 
> Add perst, wake and clkreq sideband signals and required regulators in
> PCIe3 controller and PHY device tree node. Describe the voltage rails of
> the x8 PCI slots for PCIe3 port.
> 
> Signed-off-by: Qiang Yu <quic_qianyu@quicinc.com>
> Signed-off-by: Wenbin Yao <quic_wenbyao@quicinc.com>
> ---
>  arch/arm64/boot/dts/qcom/x1e80100-qcp.dts | 118 ++++++++++++++++++++++
>  1 file changed, 118 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/x1e80100-qcp.dts b/arch/arm64/boot/dts/qcom/x1e80100-qcp.dts
> index 470c4f826..88dfd2199 100644
> --- a/arch/arm64/boot/dts/qcom/x1e80100-qcp.dts
> +++ b/arch/arm64/boot/dts/qcom/x1e80100-qcp.dts
> @@ -318,6 +318,48 @@ vreg_wcn_3p3: regulator-wcn-3p3 {
>  		regulator-boot-on;
>  	};
>  
> +	vreg_pcie_12v: regulator-pcie-12v {
> +		compatible = "regulator-fixed";
> +
> +		regulator-name = "VREG_PCIE_12V";
> +		regulator-min-microvolt = <12000000>;
> +		regulator-max-microvolt = <12000000>;
> +
> +		gpio = <&pm8550ve_8_gpios 8 GPIO_ACTIVE_HIGH>;
> +		enable-active-high;
> +
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&pcie_x8_12v>;

Please keep the 

property-n
property-names

order

throughout the patch

with that:

Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>

Konrad


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 2/4] arm64: dts: qcom: x1e80100: add bus topology for PCIe domain 3
  2025-04-25  9:29 ` [PATCH v2 2/4] arm64: dts: qcom: x1e80100: add bus topology for PCIe domain 3 Wenbin Yao
@ 2025-04-25 10:22   ` Konrad Dybcio
  2025-04-25 11:55     ` Johan Hovold
  0 siblings, 1 reply; 19+ messages in thread
From: Konrad Dybcio @ 2025-04-25 10:22 UTC (permalink / raw)
  To: Wenbin Yao, catalin.marinas, will, linux-arm-kernel, andersson,
	konradybcio, robh, krzk+dt, conor+dt, linux-arm-msm, devicetree,
	linux-kernel
  Cc: krishna.chundru, quic_vbadigan, quic_mrana, quic_cang,
	quic_qianyu

On 4/25/25 11:29 AM, Wenbin Yao wrote:
> From: Qiang Yu <quic_qianyu@quicinc.com>
> 
> Add pcie3port node to represent the PCIe bridge of PCIe3 so that PCI slot
> voltage rails can be described under this node in the board's dts.
> 
> Signed-off-by: Qiang Yu <quic_qianyu@quicinc.com>
> Signed-off-by: Wenbin Yao <quic_wenbyao@quicinc.com>
> ---
>  arch/arm64/boot/dts/qcom/x1e80100.dtsi | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/x1e80100.dtsi b/arch/arm64/boot/dts/qcom/x1e80100.dtsi
> index 46b79fce9..430f9d567 100644
> --- a/arch/arm64/boot/dts/qcom/x1e80100.dtsi
> +++ b/arch/arm64/boot/dts/qcom/x1e80100.dtsi
> @@ -3287,6 +3287,17 @@ opp-128000000 {
>  					opp-peak-kBps = <15753000 1>;
>  				};
>  			};
> +
> +			pcie3port: pcie@0 {

@0,0 for PCIe adressing (bus,device)

Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>

Konrad


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 2/4] arm64: dts: qcom: x1e80100: add bus topology for PCIe domain 3
  2025-04-25 10:22   ` Konrad Dybcio
@ 2025-04-25 11:55     ` Johan Hovold
  2025-04-26 10:44       ` Konrad Dybcio
  0 siblings, 1 reply; 19+ messages in thread
From: Johan Hovold @ 2025-04-25 11:55 UTC (permalink / raw)
  To: Konrad Dybcio
  Cc: Wenbin Yao, catalin.marinas, will, linux-arm-kernel, andersson,
	konradybcio, robh, krzk+dt, conor+dt, linux-arm-msm, devicetree,
	linux-kernel, krishna.chundru, quic_vbadigan, quic_mrana,
	quic_cang, quic_qianyu

On Fri, Apr 25, 2025 at 12:22:56PM +0200, Konrad Dybcio wrote:
> On 4/25/25 11:29 AM, Wenbin Yao wrote:
> > From: Qiang Yu <quic_qianyu@quicinc.com>
> > 
> > Add pcie3port node to represent the PCIe bridge of PCIe3 so that PCI slot
> > voltage rails can be described under this node in the board's dts.
> > 
> > Signed-off-by: Qiang Yu <quic_qianyu@quicinc.com>
> > Signed-off-by: Wenbin Yao <quic_wenbyao@quicinc.com>
> > ---
> >  arch/arm64/boot/dts/qcom/x1e80100.dtsi | 11 +++++++++++
> >  1 file changed, 11 insertions(+)
> > 
> > diff --git a/arch/arm64/boot/dts/qcom/x1e80100.dtsi b/arch/arm64/boot/dts/qcom/x1e80100.dtsi
> > index 46b79fce9..430f9d567 100644
> > --- a/arch/arm64/boot/dts/qcom/x1e80100.dtsi
> > +++ b/arch/arm64/boot/dts/qcom/x1e80100.dtsi
> > @@ -3287,6 +3287,17 @@ opp-128000000 {
> >  					opp-peak-kBps = <15753000 1>;
> >  				};
> >  			};
> > +
> > +			pcie3port: pcie@0 {
> 
> @0,0 for PCIe adressing (bus,device)

No, the bus number is not included in the unit address, so just the
device number (0) is correct here (when the function is 0) IIUC.

Johan


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 4/4] arm64: dts: qcom: x1e80100-qcp: Add qref supply for PCIe PHYs
  2025-04-25 10:03     ` Konrad Dybcio
@ 2025-04-25 12:02       ` Johan Hovold
  2025-04-26 10:48         ` Konrad Dybcio
  0 siblings, 1 reply; 19+ messages in thread
From: Johan Hovold @ 2025-04-25 12:02 UTC (permalink / raw)
  To: Konrad Dybcio
  Cc: Wenbin Yao, catalin.marinas, will, linux-arm-kernel, andersson,
	konradybcio, robh, krzk+dt, conor+dt, linux-arm-msm, devicetree,
	linux-kernel, krishna.chundru, quic_vbadigan, quic_mrana,
	quic_cang, quic_qianyu

On Fri, Apr 25, 2025 at 12:03:06PM +0200, Konrad Dybcio wrote:
> On 4/25/25 11:51 AM, Johan Hovold wrote:
> > On Fri, Apr 25, 2025 at 05:29:55PM +0800, Wenbin Yao wrote:
> >> From: Qiang Yu <quic_qianyu@quicinc.com>
> >>
> >> All PCIe PHYs on X1E80100 require vdda-qref power supplies, but this is
> >> missing in the current PHY device tree node. The PCIe port can still
> >> function because the regulator L3J, which vdda-qref consumes, is voted by
> >> other components.
> >>
> >> Since the device tree should accurately describe the hardware, add the
> >> vdda-qref power supply explicitly in all PCIe PHY device nodes.
> > 
> > AFAIU the PHYs do not use this qref supply directly so it does not
> > belong in the PHY node (but possibly in the tcsr node that provides the
> > refclk).
> > 
> > Since commit 031b46b4729b ("phy: qcom: qmp-pcie: drop bogus x1e80100
> > qref supplies") it also won't have any effect for pcie4 and pcie6.
> 
> QREF is a separate hw block distributing the reference clocks across
> certain on-SoC peripherals
> 
> If its power goes out, I don't think much of the platform would be
> functional anyway, so it's redundant here..
> 
> It doesn't have its own single register region and it's frankly
> one-shot-configured way before Linux starts up, so there should be
> no need of describing it at all.

Then it sounds like the qref supplies should be marked as always-on. Can
they be disabled at all?

Johan


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 2/4] arm64: dts: qcom: x1e80100: add bus topology for PCIe domain 3
  2025-04-25 11:55     ` Johan Hovold
@ 2025-04-26 10:44       ` Konrad Dybcio
  2025-04-28 11:16         ` Johan Hovold
  0 siblings, 1 reply; 19+ messages in thread
From: Konrad Dybcio @ 2025-04-26 10:44 UTC (permalink / raw)
  To: Johan Hovold, Konrad Dybcio
  Cc: Wenbin Yao, catalin.marinas, will, linux-arm-kernel, andersson,
	konradybcio, robh, krzk+dt, conor+dt, linux-arm-msm, devicetree,
	linux-kernel, krishna.chundru, quic_vbadigan, quic_mrana,
	quic_cang, quic_qianyu

On 4/25/25 1:55 PM, Johan Hovold wrote:
> On Fri, Apr 25, 2025 at 12:22:56PM +0200, Konrad Dybcio wrote:
>> On 4/25/25 11:29 AM, Wenbin Yao wrote:
>>> From: Qiang Yu <quic_qianyu@quicinc.com>
>>>
>>> Add pcie3port node to represent the PCIe bridge of PCIe3 so that PCI slot
>>> voltage rails can be described under this node in the board's dts.
>>>
>>> Signed-off-by: Qiang Yu <quic_qianyu@quicinc.com>
>>> Signed-off-by: Wenbin Yao <quic_wenbyao@quicinc.com>
>>> ---
>>>  arch/arm64/boot/dts/qcom/x1e80100.dtsi | 11 +++++++++++
>>>  1 file changed, 11 insertions(+)
>>>
>>> diff --git a/arch/arm64/boot/dts/qcom/x1e80100.dtsi b/arch/arm64/boot/dts/qcom/x1e80100.dtsi
>>> index 46b79fce9..430f9d567 100644
>>> --- a/arch/arm64/boot/dts/qcom/x1e80100.dtsi
>>> +++ b/arch/arm64/boot/dts/qcom/x1e80100.dtsi
>>> @@ -3287,6 +3287,17 @@ opp-128000000 {
>>>  					opp-peak-kBps = <15753000 1>;
>>>  				};
>>>  			};
>>> +
>>> +			pcie3port: pcie@0 {
>>
>> @0,0 for PCIe adressing (bus,device)
> 
> No, the bus number is not included in the unit address, so just the
> device number (0) is correct here (when the function is 0) IIUC.

Some DTs definitely have that, but I couldn't find any documentation to
back the syntax up or explain it properly

e.g.

Apple T8103
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/arch/arm64/boot/dts/apple/t8103.dtsi?h=next-20250424#n930

RK3399 GRU
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi?h=next-20250424#n486

Konrad


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 4/4] arm64: dts: qcom: x1e80100-qcp: Add qref supply for PCIe PHYs
  2025-04-25 12:02       ` Johan Hovold
@ 2025-04-26 10:48         ` Konrad Dybcio
  2025-04-30  4:15           ` Qiang Yu
  0 siblings, 1 reply; 19+ messages in thread
From: Konrad Dybcio @ 2025-04-26 10:48 UTC (permalink / raw)
  To: Johan Hovold, Konrad Dybcio
  Cc: Wenbin Yao, catalin.marinas, will, linux-arm-kernel, andersson,
	konradybcio, robh, krzk+dt, conor+dt, linux-arm-msm, devicetree,
	linux-kernel, krishna.chundru, quic_vbadigan, quic_mrana,
	quic_cang, quic_qianyu

On 4/25/25 2:02 PM, Johan Hovold wrote:
> On Fri, Apr 25, 2025 at 12:03:06PM +0200, Konrad Dybcio wrote:
>> On 4/25/25 11:51 AM, Johan Hovold wrote:
>>> On Fri, Apr 25, 2025 at 05:29:55PM +0800, Wenbin Yao wrote:
>>>> From: Qiang Yu <quic_qianyu@quicinc.com>
>>>>
>>>> All PCIe PHYs on X1E80100 require vdda-qref power supplies, but this is
>>>> missing in the current PHY device tree node. The PCIe port can still
>>>> function because the regulator L3J, which vdda-qref consumes, is voted by
>>>> other components.
>>>>
>>>> Since the device tree should accurately describe the hardware, add the
>>>> vdda-qref power supply explicitly in all PCIe PHY device nodes.
>>>
>>> AFAIU the PHYs do not use this qref supply directly so it does not
>>> belong in the PHY node (but possibly in the tcsr node that provides the
>>> refclk).
>>>
>>> Since commit 031b46b4729b ("phy: qcom: qmp-pcie: drop bogus x1e80100
>>> qref supplies") it also won't have any effect for pcie4 and pcie6.
>>
>> QREF is a separate hw block distributing the reference clocks across
>> certain on-SoC peripherals
>>
>> If its power goes out, I don't think much of the platform would be
>> functional anyway, so it's redundant here..
>>
>> It doesn't have its own single register region and it's frankly
>> one-shot-configured way before Linux starts up, so there should be
>> no need of describing it at all.
> 
> Then it sounds like the qref supplies should be marked as always-on. Can
> they be disabled at all?

The best answer I can say is "maybe". I would (without knowing any better)
assume RPMh wouldn't let you turn them off. QREF predictably takes VDD_CX/MX
and some additional lines

Konrad


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 2/4] arm64: dts: qcom: x1e80100: add bus topology for PCIe domain 3
  2025-04-26 10:44       ` Konrad Dybcio
@ 2025-04-28 11:16         ` Johan Hovold
  2025-04-28 21:08           ` Konrad Dybcio
  0 siblings, 1 reply; 19+ messages in thread
From: Johan Hovold @ 2025-04-28 11:16 UTC (permalink / raw)
  To: Konrad Dybcio
  Cc: Wenbin Yao, catalin.marinas, will, linux-arm-kernel, andersson,
	konradybcio, robh, krzk+dt, conor+dt, linux-arm-msm, devicetree,
	linux-kernel, krishna.chundru, quic_vbadigan, quic_mrana,
	quic_cang, quic_qianyu

On Sat, Apr 26, 2025 at 12:44:57PM +0200, Konrad Dybcio wrote:
> On 4/25/25 1:55 PM, Johan Hovold wrote:
> > On Fri, Apr 25, 2025 at 12:22:56PM +0200, Konrad Dybcio wrote:
> >> On 4/25/25 11:29 AM, Wenbin Yao wrote:
> >>> From: Qiang Yu <quic_qianyu@quicinc.com>
> >>>
> >>> Add pcie3port node to represent the PCIe bridge of PCIe3 so that PCI slot
> >>> voltage rails can be described under this node in the board's dts.
> >>>
> >>> Signed-off-by: Qiang Yu <quic_qianyu@quicinc.com>
> >>> Signed-off-by: Wenbin Yao <quic_wenbyao@quicinc.com>
> >>> ---
> >>>  arch/arm64/boot/dts/qcom/x1e80100.dtsi | 11 +++++++++++
> >>>  1 file changed, 11 insertions(+)
> >>>
> >>> diff --git a/arch/arm64/boot/dts/qcom/x1e80100.dtsi b/arch/arm64/boot/dts/qcom/x1e80100.dtsi
> >>> index 46b79fce9..430f9d567 100644
> >>> --- a/arch/arm64/boot/dts/qcom/x1e80100.dtsi
> >>> +++ b/arch/arm64/boot/dts/qcom/x1e80100.dtsi
> >>> @@ -3287,6 +3287,17 @@ opp-128000000 {
> >>>  					opp-peak-kBps = <15753000 1>;
> >>>  				};
> >>>  			};
> >>> +
> >>> +			pcie3port: pcie@0 {
> >>
> >> @0,0 for PCIe adressing (bus,device)
> > 
> > No, the bus number is not included in the unit address, so just the
> > device number (0) is correct here (when the function is 0) IIUC.
> 
> Some DTs definitely have that, but I couldn't find any documentation to
> back the syntax up or explain it properly

It's part of the spec:

	http://www.devicetree.org/open-firmware/bindings/pci/pci2_1.pdf

The first number is the device number and the second is the function
which can be left out if zero.

Johan


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 2/4] arm64: dts: qcom: x1e80100: add bus topology for PCIe domain 3
  2025-04-28 11:16         ` Johan Hovold
@ 2025-04-28 21:08           ` Konrad Dybcio
  0 siblings, 0 replies; 19+ messages in thread
From: Konrad Dybcio @ 2025-04-28 21:08 UTC (permalink / raw)
  To: Johan Hovold, Konrad Dybcio
  Cc: Wenbin Yao, catalin.marinas, will, linux-arm-kernel, andersson,
	konradybcio, robh, krzk+dt, conor+dt, linux-arm-msm, devicetree,
	linux-kernel, krishna.chundru, quic_vbadigan, quic_mrana,
	quic_cang, quic_qianyu

On 4/28/25 1:16 PM, Johan Hovold wrote:
> On Sat, Apr 26, 2025 at 12:44:57PM +0200, Konrad Dybcio wrote:
>> On 4/25/25 1:55 PM, Johan Hovold wrote:
>>> On Fri, Apr 25, 2025 at 12:22:56PM +0200, Konrad Dybcio wrote:
>>>> On 4/25/25 11:29 AM, Wenbin Yao wrote:
>>>>> From: Qiang Yu <quic_qianyu@quicinc.com>
>>>>>
>>>>> Add pcie3port node to represent the PCIe bridge of PCIe3 so that PCI slot
>>>>> voltage rails can be described under this node in the board's dts.
>>>>>
>>>>> Signed-off-by: Qiang Yu <quic_qianyu@quicinc.com>
>>>>> Signed-off-by: Wenbin Yao <quic_wenbyao@quicinc.com>
>>>>> ---
>>>>>  arch/arm64/boot/dts/qcom/x1e80100.dtsi | 11 +++++++++++
>>>>>  1 file changed, 11 insertions(+)
>>>>>
>>>>> diff --git a/arch/arm64/boot/dts/qcom/x1e80100.dtsi b/arch/arm64/boot/dts/qcom/x1e80100.dtsi
>>>>> index 46b79fce9..430f9d567 100644
>>>>> --- a/arch/arm64/boot/dts/qcom/x1e80100.dtsi
>>>>> +++ b/arch/arm64/boot/dts/qcom/x1e80100.dtsi
>>>>> @@ -3287,6 +3287,17 @@ opp-128000000 {
>>>>>  					opp-peak-kBps = <15753000 1>;
>>>>>  				};
>>>>>  			};
>>>>> +
>>>>> +			pcie3port: pcie@0 {
>>>>
>>>> @0,0 for PCIe adressing (bus,device)
>>>
>>> No, the bus number is not included in the unit address, so just the
>>> device number (0) is correct here (when the function is 0) IIUC.
>>
>> Some DTs definitely have that, but I couldn't find any documentation to
>> back the syntax up or explain it properly
> 
> It's part of the spec:
> 
> 	http://www.devicetree.org/open-firmware/bindings/pci/pci2_1.pdf
> 
> The first number is the device number and the second is the function
> which can be left out if zero.

OK thank you for clarifying

Konrad


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 4/4] arm64: dts: qcom: x1e80100-qcp: Add qref supply for PCIe PHYs
  2025-04-26 10:48         ` Konrad Dybcio
@ 2025-04-30  4:15           ` Qiang Yu
  2025-04-30  7:42             ` Konrad Dybcio
  2025-04-30  7:43             ` Johan Hovold
  0 siblings, 2 replies; 19+ messages in thread
From: Qiang Yu @ 2025-04-30  4:15 UTC (permalink / raw)
  To: Konrad Dybcio, Johan Hovold
  Cc: Wenbin Yao, catalin.marinas, will, linux-arm-kernel, andersson,
	konradybcio, robh, krzk+dt, conor+dt, linux-arm-msm, devicetree,
	linux-kernel, krishna.chundru, quic_vbadigan, quic_mrana,
	quic_cang


On 4/26/2025 6:48 PM, Konrad Dybcio wrote:
> On 4/25/25 2:02 PM, Johan Hovold wrote:
>> On Fri, Apr 25, 2025 at 12:03:06PM +0200, Konrad Dybcio wrote:
>>> On 4/25/25 11:51 AM, Johan Hovold wrote:
>>>> On Fri, Apr 25, 2025 at 05:29:55PM +0800, Wenbin Yao wrote:
>>>>> From: Qiang Yu <quic_qianyu@quicinc.com>
>>>>>
>>>>> All PCIe PHYs on X1E80100 require vdda-qref power supplies, but this is
>>>>> missing in the current PHY device tree node. The PCIe port can still
>>>>> function because the regulator L3J, which vdda-qref consumes, is voted by
>>>>> other components.
>>>>>
>>>>> Since the device tree should accurately describe the hardware, add the
>>>>> vdda-qref power supply explicitly in all PCIe PHY device nodes.
>>>> AFAIU the PHYs do not use this qref supply directly so it does not
>>>> belong in the PHY node (but possibly in the tcsr node that provides the
>>>> refclk).
>>>>
>>>> Since commit 031b46b4729b ("phy: qcom: qmp-pcie: drop bogus x1e80100
>>>> qref supplies") it also won't have any effect for pcie4 and pcie6.
>>> QREF is a separate hw block distributing the reference clocks across
>>> certain on-SoC peripherals
>>>
>>> If its power goes out, I don't think much of the platform would be
>>> functional anyway, so it's redundant here..
>>>
>>> It doesn't have its own single register region and it's frankly
>>> one-shot-configured way before Linux starts up, so there should be
>>> no need of describing it at all.
>> Then it sounds like the qref supplies should be marked as always-on. Can
>> they be disabled at all?
> The best answer I can say is "maybe". I would (without knowing any better)
> assume RPMh wouldn't let you turn them off. QREF predictably takes VDD_CX/MX
> and some additional lines
>
> Konrad
The vdda-qref power supply feeds the QREF clocks, which are consumed by 
PCIe, UFS, USB and display on X1e80100.
For PCIe, QREF clks are provided by the TCSR device with the following 
bindings on X1E80100:
#define TCSR_PCIE_2L_4_CLKREF_EN
#define TCSR_PCIE_2L_5_CLKREF_EN
#define TCSR_PCIE_8L_CLKREF_EN
#define TCSR_PCIE_4L_CLKREF_EN

These QREF clocks are not enabled all the time and are disabled during 
PHY deinit. Hence, vdda-qref should not be an always-on power supply. It 
should be turned off when the QREF clocks are disabled.

Describing vdda-qref in the PHY device tree node is reasonable, as it 
allows the vdda-qref power supply to be enabled or disabled along with 
the QREF clocks during PHY init/deinit.

-- 
With best wishes
Qiang Yu



^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 4/4] arm64: dts: qcom: x1e80100-qcp: Add qref supply for PCIe PHYs
  2025-04-30  4:15           ` Qiang Yu
@ 2025-04-30  7:42             ` Konrad Dybcio
  2025-04-30  7:43             ` Johan Hovold
  1 sibling, 0 replies; 19+ messages in thread
From: Konrad Dybcio @ 2025-04-30  7:42 UTC (permalink / raw)
  To: Qiang Yu, Konrad Dybcio, Johan Hovold
  Cc: Wenbin Yao, catalin.marinas, will, linux-arm-kernel, andersson,
	konradybcio, robh, krzk+dt, conor+dt, linux-arm-msm, devicetree,
	linux-kernel, krishna.chundru, quic_vbadigan, quic_mrana,
	quic_cang

On 4/30/25 6:15 AM, Qiang Yu wrote:
> 
> On 4/26/2025 6:48 PM, Konrad Dybcio wrote:
>> On 4/25/25 2:02 PM, Johan Hovold wrote:
>>> On Fri, Apr 25, 2025 at 12:03:06PM +0200, Konrad Dybcio wrote:
>>>> On 4/25/25 11:51 AM, Johan Hovold wrote:
>>>>> On Fri, Apr 25, 2025 at 05:29:55PM +0800, Wenbin Yao wrote:
>>>>>> From: Qiang Yu <quic_qianyu@quicinc.com>
>>>>>>
>>>>>> All PCIe PHYs on X1E80100 require vdda-qref power supplies, but this is
>>>>>> missing in the current PHY device tree node. The PCIe port can still
>>>>>> function because the regulator L3J, which vdda-qref consumes, is voted by
>>>>>> other components.
>>>>>>
>>>>>> Since the device tree should accurately describe the hardware, add the
>>>>>> vdda-qref power supply explicitly in all PCIe PHY device nodes.
>>>>> AFAIU the PHYs do not use this qref supply directly so it does not
>>>>> belong in the PHY node (but possibly in the tcsr node that provides the
>>>>> refclk).
>>>>>
>>>>> Since commit 031b46b4729b ("phy: qcom: qmp-pcie: drop bogus x1e80100
>>>>> qref supplies") it also won't have any effect for pcie4 and pcie6.
>>>> QREF is a separate hw block distributing the reference clocks across
>>>> certain on-SoC peripherals
>>>>
>>>> If its power goes out, I don't think much of the platform would be
>>>> functional anyway, so it's redundant here..
>>>>
>>>> It doesn't have its own single register region and it's frankly
>>>> one-shot-configured way before Linux starts up, so there should be
>>>> no need of describing it at all.
>>> Then it sounds like the qref supplies should be marked as always-on. Can
>>> they be disabled at all?
>> The best answer I can say is "maybe". I would (without knowing any better)
>> assume RPMh wouldn't let you turn them off. QREF predictably takes VDD_CX/MX
>> and some additional lines
>>
>> Konrad
> The vdda-qref power supply feeds the QREF clocks, which are consumed by PCIe, UFS, USB and display on X1e80100.
> For PCIe, QREF clks are provided by the TCSR device with the following bindings on X1E80100:
> #define TCSR_PCIE_2L_4_CLKREF_EN
> #define TCSR_PCIE_2L_5_CLKREF_EN
> #define TCSR_PCIE_8L_CLKREF_EN
> #define TCSR_PCIE_4L_CLKREF_EN
> 
> These QREF clocks are not enabled all the time and are disabled during PHY deinit. Hence, vdda-qref should not be an always-on power supply. It should be turned off when the QREF clocks are disabled.
> 
> Describing vdda-qref in the PHY device tree node is reasonable, as it allows the vdda-qref power supply to be enabled or disabled along with the QREF clocks during PHY init/deinit.

We were advised to repeat this for all QREF consumers as what I said
before may not hold true for all platforms and nobody's gonna play
whack-a-mole with this

Konrad


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 4/4] arm64: dts: qcom: x1e80100-qcp: Add qref supply for PCIe PHYs
  2025-04-30  4:15           ` Qiang Yu
  2025-04-30  7:42             ` Konrad Dybcio
@ 2025-04-30  7:43             ` Johan Hovold
  1 sibling, 0 replies; 19+ messages in thread
From: Johan Hovold @ 2025-04-30  7:43 UTC (permalink / raw)
  To: Qiang Yu
  Cc: Konrad Dybcio, Wenbin Yao, catalin.marinas, will,
	linux-arm-kernel, andersson, konradybcio, robh, krzk+dt, conor+dt,
	linux-arm-msm, devicetree, linux-kernel, krishna.chundru,
	quic_vbadigan, quic_mrana, quic_cang

[ Qiang, it seems you mail client is removing newlines from your quoted
  replies which makes it hard to read the context. I've tried to add
  them back below, but please try to fix that up. ]

On Wed, Apr 30, 2025 at 12:15:56PM +0800, Qiang Yu wrote:
> On 4/26/2025 6:48 PM, Konrad Dybcio wrote:
> > On 4/25/25 2:02 PM, Johan Hovold wrote:
> >> On Fri, Apr 25, 2025 at 12:03:06PM +0200, Konrad Dybcio wrote:
> >>> On 4/25/25 11:51 AM, Johan Hovold wrote:

> >>>> AFAIU the PHYs do not use this qref supply directly so it does not
> >>>> belong in the PHY node (but possibly in the tcsr node that provides the
> >>>> refclk).
> >>>>
> >>>> Since commit 031b46b4729b ("phy: qcom: qmp-pcie: drop bogus x1e80100
> >>>> qref supplies") it also won't have any effect for pcie4 and pcie6.

> >>> QREF is a separate hw block distributing the reference clocks across
> >>> certain on-SoC peripherals
> >>>
> >>> If its power goes out, I don't think much of the platform would be
> >>> functional anyway, so it's redundant here..
> >>>
> >>> It doesn't have its own single register region and it's frankly
> >>> one-shot-configured way before Linux starts up, so there should be
> >>> no need of describing it at all.

> >> Then it sounds like the qref supplies should be marked as always-on. Can
> >> they be disabled at all?

> > The best answer I can say is "maybe". I would (without knowing any better)
> > assume RPMh wouldn't let you turn them off. QREF predictably takes VDD_CX/MX
> > and some additional lines

> The vdda-qref power supply feeds the QREF clocks, which are consumed by 
> PCIe, UFS, USB and display on X1e80100.
> For PCIe, QREF clks are provided by the TCSR device with the following 
> bindings on X1E80100:
> #define TCSR_PCIE_2L_4_CLKREF_EN
> #define TCSR_PCIE_2L_5_CLKREF_EN
> #define TCSR_PCIE_8L_CLKREF_EN
> #define TCSR_PCIE_4L_CLKREF_EN
> 
> These QREF clocks are not enabled all the time and are disabled during 
> PHY deinit. Hence, vdda-qref should not be an always-on power supply. It 
> should be turned off when the QREF clocks are disabled.
> 
> Describing vdda-qref in the PHY device tree node is reasonable, as it 
> allows the vdda-qref power supply to be enabled or disabled along with 
> the QREF clocks during PHY init/deinit.

It may work, but it seems more reasonable to have the TCSR clock
controller manage them even if that isn't necessarily an entirely
accurate description of the hw either.

On the T14s (X1E), we have the following QREF supplies:

	VDD_A_QREFS_1P2_A
	VDD_A_QREFS_1P2_B

	VDD_A_QREFS_0P875_A
	VDD_A_QREFS_0P875_B
	VDD_A_QREFS_0P875_0
	VDD_A_QREFS_0P875_2
	VDD_A_QREFS_0P875_3

which does not seem to map directly to the three PCIe PHYs (and other
consumers).

Johan


^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2025-04-30  8:03 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-04-25  9:29 [PATCH v2 0/4] arm64: qcom: x1e80100-qcp: Add power supply and sideband signals for PCIe RC Wenbin Yao
2025-04-25  9:29 ` [PATCH v2 1/4] arm64: Kconfig: enable PCI Power Control Slot driver for QCOM Wenbin Yao
2025-04-25  9:47   ` Johan Hovold
2025-04-25  9:29 ` [PATCH v2 2/4] arm64: dts: qcom: x1e80100: add bus topology for PCIe domain 3 Wenbin Yao
2025-04-25 10:22   ` Konrad Dybcio
2025-04-25 11:55     ` Johan Hovold
2025-04-26 10:44       ` Konrad Dybcio
2025-04-28 11:16         ` Johan Hovold
2025-04-28 21:08           ` Konrad Dybcio
2025-04-25  9:29 ` [PATCH v2 3/4] arm64: dts: qcom: x1e80100-qcp: enable pcie3 x8 slot for X1E80100-QCP Wenbin Yao
2025-04-25 10:09   ` Konrad Dybcio
2025-04-25  9:29 ` [PATCH v2 4/4] arm64: dts: qcom: x1e80100-qcp: Add qref supply for PCIe PHYs Wenbin Yao
2025-04-25  9:51   ` Johan Hovold
2025-04-25 10:03     ` Konrad Dybcio
2025-04-25 12:02       ` Johan Hovold
2025-04-26 10:48         ` Konrad Dybcio
2025-04-30  4:15           ` Qiang Yu
2025-04-30  7:42             ` Konrad Dybcio
2025-04-30  7:43             ` Johan Hovold

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).