[PATCH v2 00/21] Add PCIe bridge node in DT for Qcom SoCs

[PATCH v2 00/21] Add PCIe bridge node in DT for Qcom SoCs

[Manivannan Sadhasivam]

On Qcom SoCs, the PCIe host bridge is connected to a single PCIe bridge
for each controller instance. Hence, this series adds a DT node for the
PCIe bridges across all SoCs.

There is no functionality change with this series, but the PCIe bridge
representation in DT will be necessary to add the DT node for the client
devices like the one proposed in power sequencing series [1].

- Mani

[1] https://lore.kernel.org/linux-arm-msm/20240216203215.40870-8-brgl@bgdev.pl/

Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
Changes in v2:
- Added label for bridges in sc8280xp
- Collected reviews
- Link to v1: https://lore.kernel.org/r/20240221-pcie-qcom-bridge-dts-v1-0-6c6df0f9450d@linaro.org

---
Manivannan Sadhasivam (21):
      arm64: dts: qcom: sm8250: Add PCIe bridge node
      arm64: dts: qcom: sdm845: Add PCIe bridge node
      arm64: dts: qcom: sm8150: Add PCIe bridge node
      arm64: dts: qcom: sm8350: Add PCIe bridge node
      arm64: dts: qcom: sm8450: Add PCIe bridge node
      arm64: dts: qcom: sm8550: Add PCIe bridge node
      arm64: dts: qcom: sm8650: Add PCIe bridge node
      arm64: dts: qcom: sa8775p: Add PCIe bridge node
      arm64: dts: qcom: sc8280xp: Add PCIe bridge node
      arm64: dts: qcom: msm8998: Add PCIe bridge node
      arm64: dts: qcom: sc7280: Add PCIe bridge node
      arm64: dts: qcom: qcs404: Add PCIe bridge node
      arm64: dts: qcom: sc8180x: Add PCIe bridge node
      arm64: dts: qcom: msm8996: Add PCIe bridge node
      arm64: dts: qcom: ipq8074: Add PCIe bridge node
      arm64: dts: qcom: ipq6018: Add PCIe bridge node
      ARM: dts: qcom: ipq8064: Add PCIe bridge node
      ARM: dts: qcom: ipq4019: Add PCIe bridge node
      ARM: dts: qcom: apq8064: Add PCIe bridge node
      ARM: dts: qcom: sdx55: Add PCIe bridge node
      arm64: dts: qcom: sm8650: Use "pcie" as the node name instead of "pci"

 arch/arm/boot/dts/qcom/qcom-apq8064.dtsi           | 10 +++++
 arch/arm/boot/dts/qcom/qcom-ipq4019.dtsi           | 10 +++++
 arch/arm/boot/dts/qcom/qcom-ipq8064.dtsi           | 30 +++++++++++++
 arch/arm/boot/dts/qcom/qcom-sdx55.dtsi             | 10 +++++
 arch/arm64/boot/dts/qcom/ipq6018.dtsi              | 10 +++++
 arch/arm64/boot/dts/qcom/ipq8074.dtsi              | 20 +++++++++
 arch/arm64/boot/dts/qcom/msm8996.dtsi              | 30 +++++++++++++
 arch/arm64/boot/dts/qcom/msm8998.dtsi              | 10 +++++
 arch/arm64/boot/dts/qcom/qcs404.dtsi               | 10 +++++
 arch/arm64/boot/dts/qcom/sa8775p.dtsi              | 20 +++++++++
 arch/arm64/boot/dts/qcom/sc7280.dtsi               | 10 +++++
 arch/arm64/boot/dts/qcom/sc8180x.dtsi              | 40 +++++++++++++++++
 .../dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts     | 20 +++------
 arch/arm64/boot/dts/qcom/sc8280xp.dtsi             | 50 ++++++++++++++++++++++
 arch/arm64/boot/dts/qcom/sdm845.dtsi               | 20 +++++++++
 arch/arm64/boot/dts/qcom/sm8150.dtsi               | 20 +++++++++
 arch/arm64/boot/dts/qcom/sm8250.dtsi               | 30 +++++++++++++
 arch/arm64/boot/dts/qcom/sm8350.dtsi               | 20 +++++++++
 arch/arm64/boot/dts/qcom/sm8450.dtsi               | 20 +++++++++
 arch/arm64/boot/dts/qcom/sm8550.dtsi               | 20 +++++++++
 arch/arm64/boot/dts/qcom/sm8650.dtsi               | 24 ++++++++++-
 21 files changed, 418 insertions(+), 16 deletions(-)
---
base-commit: 10569bb9fb9732cec670faa38cf1460cabeffa09
change-id: 20240221-pcie-qcom-bridge-dts-b83c0d1b642b

Best regards,
-- 
Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

[PATCH v2 01/21] arm64: dts: qcom: sm8250: Add PCIe bridge node

[Manivannan Sadhasivam]

On Qcom SoCs, the PCIe host bridge is connected to a single PCIe bridge
for each controller instance. Hence, add a node to represent the bridge.

Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
 arch/arm64/boot/dts/qcom/sm8250.dtsi | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sm8250.dtsi b/arch/arm64/boot/dts/qcom/sm8250.dtsi
index 39bd8f0eba1e..fe5485256b22 100644
--- a/arch/arm64/boot/dts/qcom/sm8250.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi
@@ -2203,6 +2203,16 @@ pcie0: pcie@1c00000 {
 			dma-coherent;
 
 			status = "disabled";
+
+			pcie@0 {
+				device_type = "pci";
+				reg = <0x0 0x0 0x0 0x0 0x0>;
+				bus-range = <0x01 0xff>;
+
+				#address-cells = <3>;
+				#size-cells = <2>;
+				ranges;
+			};
 		};
 
 		pcie0_phy: phy@1c06000 {
@@ -2318,6 +2328,16 @@ pcie1: pcie@1c08000 {
 			dma-coherent;
 
 			status = "disabled";
+
+			pcie@0 {
+				device_type = "pci";
+				reg = <0x0 0x0 0x0 0x0 0x0>;
+				bus-range = <0x01 0xff>;
+
+				#address-cells = <3>;
+				#size-cells = <2>;
+				ranges;
+			};
 		};
 
 		pcie1_phy: phy@1c0e000 {
@@ -2433,6 +2453,16 @@ pcie2: pcie@1c10000 {
 			dma-coherent;
 
 			status = "disabled";
+
+			pcie@0 {
+				device_type = "pci";
+				reg = <0x0 0x0 0x0 0x0 0x0>;
+				bus-range = <0x01 0xff>;
+
+				#address-cells = <3>;
+				#size-cells = <2>;
+				ranges;
+			};
 		};
 
 		pcie2_phy: phy@1c16000 {

-- 
2.25.1

[PATCH v2 02/21] arm64: dts: qcom: sdm845: Add PCIe bridge node

[Manivannan Sadhasivam]

On Qcom SoCs, the PCIe host bridge is connected to a single PCIe bridge
for each controller instance. Hence, add a node to represent the bridge.

Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
 arch/arm64/boot/dts/qcom/sdm845.dtsi | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
index 2f20be99ee7e..10de2bd46ffc 100644
--- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
+++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
@@ -2375,6 +2375,16 @@ pcie0: pcie@1c00000 {
 			phy-names = "pciephy";
 
 			status = "disabled";
+
+			pcie@0 {
+				device_type = "pci";
+				reg = <0x0 0x0 0x0 0x0 0x0>;
+				bus-range = <0x01 0xff>;
+
+				#address-cells = <3>;
+				#size-cells = <2>;
+				ranges;
+			};
 		};
 
 		pcie0_phy: phy@1c06000 {
@@ -2479,6 +2489,16 @@ pcie1: pcie@1c08000 {
 			phy-names = "pciephy";
 
 			status = "disabled";
+
+			pcie@0 {
+				device_type = "pci";
+				reg = <0x0 0x0 0x0 0x0 0x0>;
+				bus-range = <0x01 0xff>;
+
+				#address-cells = <3>;
+				#size-cells = <2>;
+				ranges;
+			};
 		};
 
 		pcie1_phy: phy@1c0a000 {

-- 
2.25.1

[PATCH v2 03/21] arm64: dts: qcom: sm8150: Add PCIe bridge node

[Manivannan Sadhasivam]

On Qcom SoCs, the PCIe host bridge is connected to a single PCIe bridge
for each controller instance. Hence, add a node to represent the bridge.

Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
 arch/arm64/boot/dts/qcom/sm8150.dtsi | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sm8150.dtsi b/arch/arm64/boot/dts/qcom/sm8150.dtsi
index a35c0852b5a1..ff22e4346660 100644
--- a/arch/arm64/boot/dts/qcom/sm8150.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8150.dtsi
@@ -1901,6 +1901,16 @@ pcie0: pcie@1c00000 {
 			pinctrl-0 = <&pcie0_default_state>;
 
 			status = "disabled";
+
+			pcie@0 {
+				device_type = "pci";
+				reg = <0x0 0x0 0x0 0x0 0x0>;
+				bus-range = <0x01 0xff>;
+
+				#address-cells = <3>;
+				#size-cells = <2>;
+				ranges;
+			};
 		};
 
 		pcie0_phy: phy@1c06000 {
@@ -2011,6 +2021,16 @@ pcie1: pcie@1c08000 {
 			pinctrl-0 = <&pcie1_default_state>;
 
 			status = "disabled";
+
+			pcie@0 {
+				device_type = "pci";
+				reg = <0x0 0x0 0x0 0x0 0x0>;
+				bus-range = <0x01 0xff>;
+
+				#address-cells = <3>;
+				#size-cells = <2>;
+				ranges;
+			};
 		};
 
 		pcie1_phy: phy@1c0e000 {

-- 
2.25.1

[PATCH v2 04/21] arm64: dts: qcom: sm8350: Add PCIe bridge node

[Manivannan Sadhasivam]

On Qcom SoCs, the PCIe host bridge is connected to a single PCIe bridge
for each controller instance. Hence, add a node to represent the bridge.

Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
 arch/arm64/boot/dts/qcom/sm8350.dtsi | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sm8350.dtsi b/arch/arm64/boot/dts/qcom/sm8350.dtsi
index a5e7dbbd8c6c..a7346b817400 100644
--- a/arch/arm64/boot/dts/qcom/sm8350.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8350.dtsi
@@ -1572,6 +1572,16 @@ pcie0: pcie@1c00000 {
 			phy-names = "pciephy";
 
 			status = "disabled";
+
+			pcie@0 {
+				device_type = "pci";
+				reg = <0x0 0x0 0x0 0x0 0x0>;
+				bus-range = <0x01 0xff>;
+
+				#address-cells = <3>;
+				#size-cells = <2>;
+				ranges;
+			};
 		};
 
 		pcie0_phy: phy@1c06000 {
@@ -1669,6 +1679,16 @@ pcie1: pcie@1c08000 {
 			phy-names = "pciephy";
 
 			status = "disabled";
+
+			pcie@0 {
+				device_type = "pci";
+				reg = <0x0 0x0 0x0 0x0 0x0>;
+				bus-range = <0x01 0xff>;
+
+				#address-cells = <3>;
+				#size-cells = <2>;
+				ranges;
+			};
 		};
 
 		pcie1_phy: phy@1c0e000 {

-- 
2.25.1

[PATCH v2 05/21] arm64: dts: qcom: sm8450: Add PCIe bridge node

[Manivannan Sadhasivam]

On Qcom SoCs, the PCIe host bridge is connected to a single PCIe bridge
for each controller instance. Hence, add a node to represent the bridge.

Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
 arch/arm64/boot/dts/qcom/sm8450.dtsi | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sm8450.dtsi b/arch/arm64/boot/dts/qcom/sm8450.dtsi
index b86be34a912b..b42e44b922de 100644
--- a/arch/arm64/boot/dts/qcom/sm8450.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8450.dtsi
@@ -1850,6 +1850,16 @@ pcie0: pcie@1c00000 {
 			pinctrl-0 = <&pcie0_default_state>;
 
 			status = "disabled";
+
+			pcie@0 {
+				device_type = "pci";
+				reg = <0x0 0x0 0x0 0x0 0x0>;
+				bus-range = <0x01 0xff>;
+
+				#address-cells = <3>;
+				#size-cells = <2>;
+				ranges;
+			};
 		};
 
 		pcie0_phy: phy@1c06000 {
@@ -1971,6 +1981,16 @@ pcie1: pcie@1c08000 {
 			pinctrl-0 = <&pcie1_default_state>;
 
 			status = "disabled";
+
+			pcie@0 {
+				device_type = "pci";
+				reg = <0x0 0x0 0x0 0x0 0x0>;
+				bus-range = <0x01 0xff>;
+
+				#address-cells = <3>;
+				#size-cells = <2>;
+				ranges;
+			};
 		};
 
 		pcie1_phy: phy@1c0e000 {

-- 
2.25.1

[PATCH v2 06/21] arm64: dts: qcom: sm8550: Add PCIe bridge node

[Manivannan Sadhasivam]

On Qcom SoCs, the PCIe host bridge is connected to a single PCIe bridge
for each controller instance. Hence, add a node to represent the bridge.

Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
 arch/arm64/boot/dts/qcom/sm8550.dtsi | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sm8550.dtsi b/arch/arm64/boot/dts/qcom/sm8550.dtsi
index 3904348075f6..760b6a6cb59c 100644
--- a/arch/arm64/boot/dts/qcom/sm8550.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8550.dtsi
@@ -1770,6 +1770,16 @@ pcie0: pcie@1c00000 {
 			phy-names = "pciephy";
 
 			status = "disabled";
+
+			pcie@0 {
+				device_type = "pci";
+				reg = <0x0 0x0 0x0 0x0 0x0>;
+				bus-range = <0x01 0xff>;
+
+				#address-cells = <3>;
+				#size-cells = <2>;
+				ranges;
+			};
 		};
 
 		pcie0_phy: phy@1c06000 {
@@ -1883,6 +1893,16 @@ pcie1: pcie@1c08000 {
 			phy-names = "pciephy";
 
 			status = "disabled";
+
+			pcie@0 {
+				device_type = "pci";
+				reg = <0x0 0x0 0x0 0x0 0x0>;
+				bus-range = <0x01 0xff>;
+
+				#address-cells = <3>;
+				#size-cells = <2>;
+				ranges;
+			};
 		};
 
 		pcie1_phy: phy@1c0e000 {

-- 
2.25.1

[PATCH v2 07/21] arm64: dts: qcom: sm8650: Add PCIe bridge node

[Manivannan Sadhasivam]

On Qcom SoCs, the PCIe host bridge is connected to a single PCIe bridge
for each controller instance. Hence, add a node to represent the bridge.

Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
 arch/arm64/boot/dts/qcom/sm8650.dtsi | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sm8650.dtsi b/arch/arm64/boot/dts/qcom/sm8650.dtsi
index ba72d8f38420..06d2b6432ab1 100644
--- a/arch/arm64/boot/dts/qcom/sm8650.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8650.dtsi
@@ -2294,6 +2294,16 @@ &mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>,
 			dma-coherent;
 
 			status = "disabled";
+
+			pcie@0 {
+				device_type = "pci";
+				reg = <0x0 0x0 0x0 0x0 0x0>;
+				bus-range = <0x01 0xff>;
+
+				#address-cells = <3>;
+				#size-cells = <2>;
+				ranges;
+			};
 		};
 
 		pcie0_phy: phy@1c06000 {
@@ -2422,6 +2432,16 @@ &mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>,
 				 <0x02000000 0 0x40300000 0 0x40300000 0 0x1fd00000>;
 
 			status = "disabled";
+
+			pcie@0 {
+				device_type = "pci";
+				reg = <0x0 0x0 0x0 0x0 0x0>;
+				bus-range = <0x01 0xff>;
+
+				#address-cells = <3>;
+				#size-cells = <2>;
+				ranges;
+			};
 		};
 
 		pcie1_phy: phy@1c0e000 {

-- 
2.25.1

[PATCH v2 08/21] arm64: dts: qcom: sa8775p: Add PCIe bridge node

[Manivannan Sadhasivam]

On Qcom SoCs, the PCIe host bridge is connected to a single PCIe bridge
for each controller instance. Hence, add a node to represent the bridge.

Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
 arch/arm64/boot/dts/qcom/sa8775p.dtsi | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sa8775p.dtsi b/arch/arm64/boot/dts/qcom/sa8775p.dtsi
index 231cea1f0fa8..31de73594839 100644
--- a/arch/arm64/boot/dts/qcom/sa8775p.dtsi
+++ b/arch/arm64/boot/dts/qcom/sa8775p.dtsi
@@ -3677,6 +3677,16 @@ pcie0: pcie@1c00000 {
 		phy-names = "pciephy";
 
 		status = "disabled";
+
+		pcie@0 {
+			device_type = "pci";
+			reg = <0x0 0x0 0x0 0x0 0x0>;
+			bus-range = <0x01 0xff>;
+
+			#address-cells = <3>;
+			#size-cells = <2>;
+			ranges;
+		};
 	};
 
 	pcie0_phy: phy@1c04000 {
@@ -3777,6 +3787,16 @@ pcie1: pcie@1c10000 {
 		phy-names = "pciephy";
 
 		status = "disabled";
+
+		pcie@0 {
+			device_type = "pci";
+			reg = <0x0 0x0 0x0 0x0 0x0>;
+			bus-range = <0x01 0xff>;
+
+			#address-cells = <3>;
+			#size-cells = <2>;
+			ranges;
+		};
 	};
 
 	pcie1_phy: phy@1c14000 {

-- 
2.25.1

[PATCH v2 09/21] arm64: dts: qcom: sc8280xp: Add PCIe bridge node

[Manivannan Sadhasivam]

On Qcom SoCs, the PCIe host bridge is connected to a single PCIe bridge
for each controller instance. Hence, add a node to represent the bridge.

While at it, let's remove the bridge properties from board dts as they are
now redundant.

Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
 .../dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts     | 20 +++------
 arch/arm64/boot/dts/qcom/sc8280xp.dtsi             | 50 ++++++++++++++++++++++
 2 files changed, 56 insertions(+), 14 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts b/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts
index 15ae94c1602d..caf7dff446a6 100644
--- a/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts
+++ b/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts
@@ -731,22 +731,14 @@ &pcie4 {
 	pinctrl-0 = <&pcie4_default>;
 
 	status = "okay";
+};
 
-	pcie@0 {
-		device_type = "pci";
-		reg = <0x0 0x0 0x0 0x0 0x0>;
-		#address-cells = <3>;
-		#size-cells = <2>;
-		ranges;
-
-		bus-range = <0x01 0xff>;
-
-		wifi@0 {
-			compatible = "pci17cb,1103";
-			reg = <0x10000 0x0 0x0 0x0 0x0>;
+&pcie4_port0 {
+	wifi@0 {
+		compatible = "pci17cb,1103";
+		reg = <0x10000 0x0 0x0 0x0 0x0>;
 
-			qcom,ath11k-calibration-variant = "LE_X13S";
-		};
+		qcom,ath11k-calibration-variant = "LE_X13S";
 	};
 };
 
diff --git a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
index a5b194813079..c7feebcb28b9 100644
--- a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
@@ -1779,6 +1779,16 @@ pcie4: pcie@1c00000 {
 			phy-names = "pciephy";
 
 			status = "disabled";
+
+			pcie4_port0: pcie@0 {
+				device_type = "pci";
+				reg = <0x0 0x0 0x0 0x0 0x0>;
+				bus-range = <0x01 0xff>;
+
+				#address-cells = <3>;
+				#size-cells = <2>;
+				ranges;
+			};
 		};
 
 		pcie4_phy: phy@1c06000 {
@@ -1877,6 +1887,16 @@ pcie3b: pcie@1c08000 {
 			phy-names = "pciephy";
 
 			status = "disabled";
+
+			pcie3b_port0: pcie@0 {
+				device_type = "pci";
+				reg = <0x0 0x0 0x0 0x0 0x0>;
+				bus-range = <0x01 0xff>;
+
+				#address-cells = <3>;
+				#size-cells = <2>;
+				ranges;
+			};
 		};
 
 		pcie3b_phy: phy@1c0e000 {
@@ -1975,6 +1995,16 @@ pcie3a: pcie@1c10000 {
 			phy-names = "pciephy";
 
 			status = "disabled";
+
+			pcie3a_port0: pcie@0 {
+				device_type = "pci";
+				reg = <0x0 0x0 0x0 0x0 0x0>;
+				bus-range = <0x01 0xff>;
+
+				#address-cells = <3>;
+				#size-cells = <2>;
+				ranges;
+			};
 		};
 
 		pcie3a_phy: phy@1c14000 {
@@ -2076,6 +2106,16 @@ pcie2b: pcie@1c18000 {
 			phy-names = "pciephy";
 
 			status = "disabled";
+
+			pcie2b_port0: pcie@0 {
+				device_type = "pci";
+				reg = <0x0 0x0 0x0 0x0 0x0>;
+				bus-range = <0x01 0xff>;
+
+				#address-cells = <3>;
+				#size-cells = <2>;
+				ranges;
+			};
 		};
 
 		pcie2b_phy: phy@1c1e000 {
@@ -2174,6 +2214,16 @@ pcie2a: pcie@1c20000 {
 			phy-names = "pciephy";
 
 			status = "disabled";
+
+			pcie2a_port0: pcie@0 {
+				device_type = "pci";
+				reg = <0x0 0x0 0x0 0x0 0x0>;
+				bus-range = <0x01 0xff>;
+
+				#address-cells = <3>;
+				#size-cells = <2>;
+				ranges;
+			};
 		};
 
 		pcie2a_phy: phy@1c24000 {

-- 
2.25.1

[PATCH v2 10/21] arm64: dts: qcom: msm8998: Add PCIe bridge node

[Manivannan Sadhasivam]

On Qcom SoCs, the PCIe host bridge is connected to a single PCIe bridge
for each controller instance. Hence, add a node to represent the bridge.

Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
 arch/arm64/boot/dts/qcom/msm8998.dtsi | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/msm8998.dtsi b/arch/arm64/boot/dts/qcom/msm8998.dtsi
index 4dfe2d09ac28..d795b2bbe133 100644
--- a/arch/arm64/boot/dts/qcom/msm8998.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8998.dtsi
@@ -972,6 +972,16 @@ pcie0: pcie@1c00000 {
 			power-domains = <&gcc PCIE_0_GDSC>;
 			iommu-map = <0x100 &anoc1_smmu 0x1480 1>;
 			perst-gpios = <&tlmm 35 GPIO_ACTIVE_LOW>;
+
+			pcie@0 {
+				device_type = "pci";
+				reg = <0x0 0x0 0x0 0x0 0x0>;
+				bus-range = <0x01 0xff>;
+
+				#address-cells = <3>;
+				#size-cells = <2>;
+				ranges;
+			};
 		};
 
 		pcie_phy: phy@1c06000 {

-- 
2.25.1

[PATCH v2 11/21] arm64: dts: qcom: sc7280: Add PCIe bridge node

[Manivannan Sadhasivam]

On Qcom SoCs, the PCIe host bridge is connected to a single PCIe bridge
for each controller instance. Hence, add a node to represent the bridge.

Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
 arch/arm64/boot/dts/qcom/sc7280.dtsi | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
index 7e7f0f0fb41b..3ed6cc50b18d 100644
--- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
@@ -2273,6 +2273,16 @@ pcie1: pcie@1c08000 {
 				    <0x100 &apps_smmu 0x1c81 0x1>;
 
 			status = "disabled";
+
+			pcie@0 {
+				device_type = "pci";
+				reg = <0x0 0x0 0x0 0x0 0x0>;
+				bus-range = <0x01 0xff>;
+
+				#address-cells = <3>;
+				#size-cells = <2>;
+				ranges;
+			};
 		};
 
 		pcie1_phy: phy@1c0e000 {

-- 
2.25.1

[PATCH v2 12/21] arm64: dts: qcom: qcs404: Add PCIe bridge node

[Manivannan Sadhasivam]

On Qcom SoCs, the PCIe host bridge is connected to a single PCIe bridge
for each controller instance. Hence, add a node to represent the bridge.

Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
 arch/arm64/boot/dts/qcom/qcs404.dtsi | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/qcs404.dtsi b/arch/arm64/boot/dts/qcom/qcs404.dtsi
index a05d0234f7fc..ac451f378056 100644
--- a/arch/arm64/boot/dts/qcom/qcs404.dtsi
+++ b/arch/arm64/boot/dts/qcom/qcs404.dtsi
@@ -1516,6 +1516,16 @@ pcie: pcie@10000000 {
 			phy-names = "pciephy";
 
 			status = "disabled";
+
+			pcie@0 {
+				device_type = "pci";
+				reg = <0x0 0x0 0x0 0x0 0x0>;
+				bus-range = <0x01 0xff>;
+
+				#address-cells = <3>;
+				#size-cells = <2>;
+				ranges;
+			};
 		};
 	};
 

-- 
2.25.1

[PATCH v2 13/21] arm64: dts: qcom: sc8180x: Add PCIe bridge node

[Manivannan Sadhasivam]

On Qcom SoCs, the PCIe host bridge is connected to a single PCIe bridge
for each controller instance. Hence, add a node to represent the bridge.

Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
 arch/arm64/boot/dts/qcom/sc8180x.dtsi | 40 +++++++++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sc8180x.dtsi b/arch/arm64/boot/dts/qcom/sc8180x.dtsi
index 32afc78d5b76..322ead0389c9 100644
--- a/arch/arm64/boot/dts/qcom/sc8180x.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc8180x.dtsi
@@ -1777,6 +1777,16 @@ pcie0: pcie@1c00000 {
 			dma-coherent;
 
 			status = "disabled";
+
+			pcie@0 {
+				device_type = "pci";
+				reg = <0x0 0x0 0x0 0x0 0x0>;
+				bus-range = <0x01 0xff>;
+
+				#address-cells = <3>;
+				#size-cells = <2>;
+				ranges;
+			};
 		};
 
 		pcie0_phy: phy@1c06000 {
@@ -1888,6 +1898,16 @@ pcie3: pcie@1c08000 {
 			dma-coherent;
 
 			status = "disabled";
+
+			pcie@0 {
+				device_type = "pci";
+				reg = <0x0 0x0 0x0 0x0 0x0>;
+				bus-range = <0x01 0xff>;
+
+				#address-cells = <3>;
+				#size-cells = <2>;
+				ranges;
+			};
 		};
 
 		pcie3_phy: phy@1c0c000 {
@@ -2000,6 +2020,16 @@ pcie1: pcie@1c10000 {
 			dma-coherent;
 
 			status = "disabled";
+
+			pcie@0 {
+				device_type = "pci";
+				reg = <0x0 0x0 0x0 0x0 0x0>;
+				bus-range = <0x01 0xff>;
+
+				#address-cells = <3>;
+				#size-cells = <2>;
+				ranges;
+			};
 		};
 
 		pcie1_phy: phy@1c16000 {
@@ -2112,6 +2142,16 @@ pcie2: pcie@1c18000 {
 			dma-coherent;
 
 			status = "disabled";
+
+			pcie@0 {
+				device_type = "pci";
+				reg = <0x0 0x0 0x0 0x0 0x0>;
+				bus-range = <0x01 0xff>;
+
+				#address-cells = <3>;
+				#size-cells = <2>;
+				ranges;
+			};
 		};
 
 		pcie2_phy: phy@1c1c000 {

-- 
2.25.1

[PATCH v2 14/21] arm64: dts: qcom: msm8996: Add PCIe bridge node

[Manivannan Sadhasivam]

On Qcom SoCs, the PCIe host bridge is connected to a single PCIe bridge
for each controller instance. Hence, add a node to represent the bridge.

Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
 arch/arm64/boot/dts/qcom/msm8996.dtsi | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/msm8996.dtsi b/arch/arm64/boot/dts/qcom/msm8996.dtsi
index 1601e46549e7..8d2cb6f41095 100644
--- a/arch/arm64/boot/dts/qcom/msm8996.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8996.dtsi
@@ -1929,6 +1929,16 @@ pcie0: pcie@600000 {
 						"cfg",
 						"bus_master",
 						"bus_slave";
+
+				pcie@0 {
+					device_type = "pci";
+					reg = <0x0 0x0 0x0 0x0 0x0>;
+					bus-range = <0x01 0xff>;
+
+					#address-cells = <3>;
+					#size-cells = <2>;
+					ranges;
+				};
 			};
 
 			pcie1: pcie@608000 {
@@ -1982,6 +1992,16 @@ pcie1: pcie@608000 {
 						"cfg",
 						"bus_master",
 						"bus_slave";
+
+				pcie@0 {
+					device_type = "pci";
+					reg = <0x0 0x0 0x0 0x0 0x0>;
+					bus-range = <0x01 0xff>;
+
+					#address-cells = <3>;
+					#size-cells = <2>;
+					ranges;
+				};
 			};
 
 			pcie2: pcie@610000 {
@@ -2032,6 +2052,16 @@ pcie2: pcie@610000 {
 						"cfg",
 						"bus_master",
 						"bus_slave";
+
+				pcie@0 {
+					device_type = "pci";
+					reg = <0x0 0x0 0x0 0x0 0x0>;
+					bus-range = <0x01 0xff>;
+
+					#address-cells = <3>;
+					#size-cells = <2>;
+					ranges;
+				};
 			};
 		};
 

-- 
2.25.1

[PATCH v2 15/21] arm64: dts: qcom: ipq8074: Add PCIe bridge node

[Manivannan Sadhasivam]

On Qcom SoCs, the PCIe host bridge is connected to a single PCIe bridge
for each controller instance. Hence, add a node to represent the bridge.

Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
 arch/arm64/boot/dts/qcom/ipq8074.dtsi | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/ipq8074.dtsi b/arch/arm64/boot/dts/qcom/ipq8074.dtsi
index e5b89753aa5c..12324841d1b0 100644
--- a/arch/arm64/boot/dts/qcom/ipq8074.dtsi
+++ b/arch/arm64/boot/dts/qcom/ipq8074.dtsi
@@ -864,6 +864,16 @@ IRQ_TYPE_LEVEL_HIGH>, /* int_c */
 				      "ahb",
 				      "axi_m_sticky";
 			status = "disabled";
+
+			pcie@0 {
+				device_type = "pci";
+				reg = <0x0 0x0 0x0 0x0 0x0>;
+				bus-range = <0x01 0xff>;
+
+				#address-cells = <3>;
+				#size-cells = <2>;
+				ranges;
+			};
 		};
 
 		pcie0: pcie@20000000 {
@@ -929,6 +939,16 @@ IRQ_TYPE_LEVEL_HIGH>, /* int_c */
 				      "axi_m_sticky",
 				      "axi_s_sticky";
 			status = "disabled";
+
+			pcie@0 {
+				device_type = "pci";
+				reg = <0x0 0x0 0x0 0x0 0x0>;
+				bus-range = <0x01 0xff>;
+
+				#address-cells = <3>;
+				#size-cells = <2>;
+				ranges;
+			};
 		};
 	};
 

-- 
2.25.1

[PATCH v2 16/21] arm64: dts: qcom: ipq6018: Add PCIe bridge node

[Manivannan Sadhasivam]

On Qcom SoCs, the PCIe host bridge is connected to a single PCIe bridge
for each controller instance. Hence, add a node to represent the bridge.

Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
 arch/arm64/boot/dts/qcom/ipq6018.dtsi | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/ipq6018.dtsi b/arch/arm64/boot/dts/qcom/ipq6018.dtsi
index 4e29adea570a..17ab6c475958 100644
--- a/arch/arm64/boot/dts/qcom/ipq6018.dtsi
+++ b/arch/arm64/boot/dts/qcom/ipq6018.dtsi
@@ -907,6 +907,16 @@ pcie0: pcie@20000000 {
 				      "axi_s_sticky";
 
 			status = "disabled";
+
+			pcie@0 {
+				device_type = "pci";
+				reg = <0x0 0x0 0x0 0x0 0x0>;
+				bus-range = <0x01 0xff>;
+
+				#address-cells = <3>;
+				#size-cells = <2>;
+				ranges;
+			};
 		};
 	};
 

-- 
2.25.1

[PATCH v2 17/21] ARM: dts: qcom: ipq8064: Add PCIe bridge node

[Manivannan Sadhasivam]

On Qcom SoCs, the PCIe host bridge is connected to a single PCIe bridge
for each controller instance. Hence, add a node to represent the bridge.

Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
 arch/arm/boot/dts/qcom/qcom-ipq8064.dtsi | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/arch/arm/boot/dts/qcom/qcom-ipq8064.dtsi b/arch/arm/boot/dts/qcom/qcom-ipq8064.dtsi
index 2eb6758b6a3a..f128510d8445 100644
--- a/arch/arm/boot/dts/qcom/qcom-ipq8064.dtsi
+++ b/arch/arm/boot/dts/qcom/qcom-ipq8064.dtsi
@@ -1121,6 +1121,16 @@ pcie0: pcie@1b500000 {
 
 			status = "disabled";
 			perst-gpios = <&qcom_pinmux 3 GPIO_ACTIVE_LOW>;
+
+			pcie@0 {
+				device_type = "pci";
+				reg = <0x0 0x0 0x0 0x0 0x0>;
+				bus-range = <0x01 0xff>;
+
+				#address-cells = <3>;
+				#size-cells = <2>;
+				ranges;
+			};
 		};
 
 		pcie1: pcie@1b700000 {
@@ -1172,6 +1182,16 @@ pcie1: pcie@1b700000 {
 
 			status = "disabled";
 			perst-gpios = <&qcom_pinmux 48 GPIO_ACTIVE_LOW>;
+
+			pcie@0 {
+				device_type = "pci";
+				reg = <0x0 0x0 0x0 0x0 0x0>;
+				bus-range = <0x01 0xff>;
+
+				#address-cells = <3>;
+				#size-cells = <2>;
+				ranges;
+			};
 		};
 
 		pcie2: pcie@1b900000 {
@@ -1223,6 +1243,16 @@ pcie2: pcie@1b900000 {
 
 			status = "disabled";
 			perst-gpios = <&qcom_pinmux 63 GPIO_ACTIVE_LOW>;
+
+			pcie@0 {
+				device_type = "pci";
+				reg = <0x0 0x0 0x0 0x0 0x0>;
+				bus-range = <0x01 0xff>;
+
+				#address-cells = <3>;
+				#size-cells = <2>;
+				ranges;
+			};
 		};
 
 		qsgmii_csr: syscon@1bb00000 {

-- 
2.25.1

[PATCH v2 18/21] ARM: dts: qcom: ipq4019: Add PCIe bridge node

[Manivannan Sadhasivam]

On Qcom SoCs, the PCIe host bridge is connected to a single PCIe bridge
for each controller instance. Hence, add a node to represent the bridge.

Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
 arch/arm/boot/dts/qcom/qcom-ipq4019.dtsi | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/arm/boot/dts/qcom/qcom-ipq4019.dtsi b/arch/arm/boot/dts/qcom/qcom-ipq4019.dtsi
index 681cb3fc8085..f09c123d9fa2 100644
--- a/arch/arm/boot/dts/qcom/qcom-ipq4019.dtsi
+++ b/arch/arm/boot/dts/qcom/qcom-ipq4019.dtsi
@@ -470,6 +470,16 @@ pcie0: pcie@40000000 {
 				      "phy_ahb";
 
 			status = "disabled";
+
+			pcie@0 {
+				device_type = "pci";
+				reg = <0x0 0x0 0x0 0x0 0x0>;
+				bus-range = <0x01 0xff>;
+
+				#address-cells = <3>;
+				#size-cells = <2>;
+				ranges;
+			};
 		};
 
 		qpic_bam: dma-controller@7984000 {

-- 
2.25.1

[PATCH v2 19/21] ARM: dts: qcom: apq8064: Add PCIe bridge node

[Manivannan Sadhasivam]

On Qcom SoCs, the PCIe host bridge is connected to a single PCIe bridge
for each controller instance. Hence, add a node to represent the bridge.

Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
 arch/arm/boot/dts/qcom/qcom-apq8064.dtsi | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/arm/boot/dts/qcom/qcom-apq8064.dtsi b/arch/arm/boot/dts/qcom/qcom-apq8064.dtsi
index 9a5ba978775a..dbe0ae2c8770 100644
--- a/arch/arm/boot/dts/qcom/qcom-apq8064.dtsi
+++ b/arch/arm/boot/dts/qcom/qcom-apq8064.dtsi
@@ -1334,6 +1334,16 @@ pcie: pcie@1b500000 {
 				 <&gcc PCIE_PHY_RESET>;
 			reset-names = "axi", "ahb", "por", "pci", "phy";
 			status = "disabled";
+
+			pcie@0 {
+				device_type = "pci";
+				reg = <0x0 0x0 0x0 0x0 0x0>;
+				bus-range = <0x01 0xff>;
+
+				#address-cells = <3>;
+				#size-cells = <2>;
+				ranges;
+			};
 		};
 
 		hdmi: hdmi-tx@4a00000 {

-- 
2.25.1

[PATCH v2 20/21] ARM: dts: qcom: sdx55: Add PCIe bridge node

[Manivannan Sadhasivam]

On Qcom SoCs, the PCIe host bridge is connected to a single PCIe bridge
for each controller instance. Hence, add a node to represent the bridge.

Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
 arch/arm/boot/dts/qcom/qcom-sdx55.dtsi | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/arm/boot/dts/qcom/qcom-sdx55.dtsi b/arch/arm/boot/dts/qcom/qcom-sdx55.dtsi
index edc9aaf828c8..68fa5859d263 100644
--- a/arch/arm/boot/dts/qcom/qcom-sdx55.dtsi
+++ b/arch/arm/boot/dts/qcom/qcom-sdx55.dtsi
@@ -378,6 +378,16 @@ pcie_rc: pcie@1c00000 {
 			phy-names = "pciephy";
 
 			status = "disabled";
+
+			pcie@0 {
+				device_type = "pci";
+				reg = <0x0 0x0 0x0 0x0 0x0>;
+				bus-range = <0x01 0xff>;
+
+				#address-cells = <3>;
+				#size-cells = <2>;
+				ranges;
+			};
 		};
 
 		pcie_ep: pcie-ep@1c00000 {

-- 
2.25.1

[PATCH v2 21/21] arm64: dts: qcom: sm8650: Use "pcie" as the node name instead of "pci"

[Manivannan Sadhasivam]

Qcom SoCs doesn't support legacy PCI, but only PCIe. So use the correct
node name for the controller instances.

Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>
Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
 arch/arm64/boot/dts/qcom/sm8650.dtsi | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/sm8650.dtsi b/arch/arm64/boot/dts/qcom/sm8650.dtsi
index 06d2b6432ab1..b25fefd6a786 100644
--- a/arch/arm64/boot/dts/qcom/sm8650.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8650.dtsi
@@ -2208,7 +2208,7 @@ rng: rng@10c3000 {
 			reg = <0 0x010c3000 0 0x1000>;
 		};
 
-		pcie0: pci@1c00000 {
+		pcie0: pcie@1c00000 {
 			device_type = "pci";
 			compatible = "qcom,pcie-sm8650", "qcom,pcie-sm8550";
 			reg = <0 0x01c00000 0 0x3000>,
@@ -2337,7 +2337,7 @@ pcie0_phy: phy@1c06000 {
 			status = "disabled";
 		};
 
-		pcie1: pci@1c08000 {
+		pcie1: pcie@1c08000 {
 			device_type = "pci";
 			compatible = "qcom,pcie-sm8650", "qcom,pcie-sm8550";
 			reg = <0 0x01c08000 0 0x3000>,

-- 
2.25.1

Re: [PATCH v2 00/21] Add PCIe bridge node in DT for Qcom SoCs

[Konrad Dybcio]

On 21.03.2024 12:16, Manivannan Sadhasivam wrote:
> On Qcom SoCs, the PCIe host bridge is connected to a single PCIe bridge
> for each controller instance. Hence, this series adds a DT node for the
> PCIe bridges across all SoCs.
> 
> There is no functionality change with this series, but the PCIe bridge
> representation in DT will be necessary to add the DT node for the client
> devices like the one proposed in power sequencing series [1].
> 
> - Mani
> 
> [1] https://lore.kernel.org/linux-arm-msm/20240216203215.40870-8-brgl@bgdev.pl/
> 
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> ---

Everything looks good

Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>

Konrad

Re: (subset) [PATCH v2 00/21] Add PCIe bridge node in DT for Qcom SoCs

[Bjorn Andersson]

On Thu, 21 Mar 2024 16:46:20 +0530, Manivannan Sadhasivam wrote:
> On Qcom SoCs, the PCIe host bridge is connected to a single PCIe bridge
> for each controller instance. Hence, this series adds a DT node for the
> PCIe bridges across all SoCs.
> 
> There is no functionality change with this series, but the PCIe bridge
> representation in DT will be necessary to add the DT node for the client
> devices like the one proposed in power sequencing series [1].
> 
> [...]

Applied, thanks!

[17/21] ARM: dts: qcom: ipq8064: Add PCIe bridge node
        commit: 0c4d19b125401957123989a25094972cf0e77670
[18/21] ARM: dts: qcom: ipq4019: Add PCIe bridge node
        commit: ed9b196418d4e2fa4f6c27b61a92c2038e1ba04d
[19/21] ARM: dts: qcom: apq8064: Add PCIe bridge node
        commit: 27cb9eccf94cb163f9bf3b945f249ab7c42861db
[20/21] ARM: dts: qcom: sdx55: Add PCIe bridge node
        commit: 669841a2eff4c0132841dea3ae40d9148a36f257

Best regards,
-- 
Bjorn Andersson <andersson@kernel.org>

Re: (subset) [PATCH v2 00/21] Add PCIe bridge node in DT for Qcom SoCs

[Bjorn Andersson]

On Thu, 21 Mar 2024 16:46:20 +0530, Manivannan Sadhasivam wrote:
> On Qcom SoCs, the PCIe host bridge is connected to a single PCIe bridge
> for each controller instance. Hence, this series adds a DT node for the
> PCIe bridges across all SoCs.
> 
> There is no functionality change with this series, but the PCIe bridge
> representation in DT will be necessary to add the DT node for the client
> devices like the one proposed in power sequencing series [1].
> 
> [...]

Applied, thanks!

[21/21] arm64: dts: qcom: sm8650: Use "pcie" as the node name instead of "pci"
        commit: 2f2120a15251097f9afcab5b4db7894ce03b2933

Best regards,
-- 
Bjorn Andersson <andersson@kernel.org>

Re: [PATCH v2 01/21] arm64: dts: qcom: sm8250: Add PCIe bridge node

[Bjorn Helgaas]

On Thu, Mar 21, 2024 at 04:46:21PM +0530, Manivannan Sadhasivam wrote:
> On Qcom SoCs, the PCIe host bridge is connected to a single PCIe bridge
> for each controller instance. Hence, add a node to represent the bridge.
> 
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> ---
>  arch/arm64/boot/dts/qcom/sm8250.dtsi | 30 ++++++++++++++++++++++++++++++
>  1 file changed, 30 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sm8250.dtsi b/arch/arm64/boot/dts/qcom/sm8250.dtsi
> index 39bd8f0eba1e..fe5485256b22 100644
> --- a/arch/arm64/boot/dts/qcom/sm8250.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi
> @@ -2203,6 +2203,16 @@ pcie0: pcie@1c00000 {
>  			dma-coherent;
>  
>  			status = "disabled";
> +
> +			pcie@0 {
> +				device_type = "pci";
> +				reg = <0x0 0x0 0x0 0x0 0x0>;
> +				bus-range = <0x01 0xff>;

Hi Mani, most or all of the patches in this series add this
"bus-range" property.  IIUC, these are all Root Ports and hence the
secondary/subordinate bus numbers should be programmable.

If that's the case, I don't think we need to include "bus-range" in DT
for them, do we?

Bjorn

Re: [PATCH v2 01/21] arm64: dts: qcom: sm8250: Add PCIe bridge node

[Manivannan Sadhasivam]

Hi Bjorn,

On Fri, Jan 03, 2025 at 03:05:31PM -0600, Bjorn Helgaas wrote:
> On Thu, Mar 21, 2024 at 04:46:21PM +0530, Manivannan Sadhasivam wrote:
> > On Qcom SoCs, the PCIe host bridge is connected to a single PCIe bridge
> > for each controller instance. Hence, add a node to represent the bridge.
> > 
> > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > ---
> >  arch/arm64/boot/dts/qcom/sm8250.dtsi | 30 ++++++++++++++++++++++++++++++
> >  1 file changed, 30 insertions(+)
> > 
> > diff --git a/arch/arm64/boot/dts/qcom/sm8250.dtsi b/arch/arm64/boot/dts/qcom/sm8250.dtsi
> > index 39bd8f0eba1e..fe5485256b22 100644
> > --- a/arch/arm64/boot/dts/qcom/sm8250.dtsi
> > +++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi
> > @@ -2203,6 +2203,16 @@ pcie0: pcie@1c00000 {
> >  			dma-coherent;
> >  
> >  			status = "disabled";
> > +
> > +			pcie@0 {
> > +				device_type = "pci";
> > +				reg = <0x0 0x0 0x0 0x0 0x0>;
> > +				bus-range = <0x01 0xff>;
> 
> Hi Mani, most or all of the patches in this series add this
> "bus-range" property.  IIUC, these are all Root Ports and hence the
> secondary/subordinate bus numbers should be programmable.
> 

Right. It is not a functional dependency.

> If that's the case, I don't think we need to include "bus-range" in DT
> for them, do we?
> 

We mostly include it to silence the below bindings check for the endpoint device
node:

Warning (pci_device_bus_num): /soc@0/pcie@1c00000/pcie@0/wifi@0: PCI bus number 1 out of range, expected (0 - 0)

DTC check is happy if the 'bus-range' property is absent in the bridge node. But
while validating the endpoint node (if defined), it currently relies on the
parent 'bus-range' property to verify the bus number provided in the endpoint
'reg' property.

I don't know else the check can verify the correctness of the endpoint bus
number. So deferring to Rob here.

- Mani

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH v2 01/21] arm64: dts: qcom: sm8250: Add PCIe bridge node

[Bjorn Helgaas]

[+cc Nícolas]

On Sun, Jan 05, 2025 at 03:46:12PM +0530, Manivannan Sadhasivam wrote:
> On Fri, Jan 03, 2025 at 03:05:31PM -0600, Bjorn Helgaas wrote:
> > On Thu, Mar 21, 2024 at 04:46:21PM +0530, Manivannan Sadhasivam wrote:
> > > On Qcom SoCs, the PCIe host bridge is connected to a single PCIe bridge
> > > for each controller instance. Hence, add a node to represent the bridge.
> > > 
> > > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > > ---
> > >  arch/arm64/boot/dts/qcom/sm8250.dtsi | 30 ++++++++++++++++++++++++++++++
> > >  1 file changed, 30 insertions(+)
> > > 
> > > diff --git a/arch/arm64/boot/dts/qcom/sm8250.dtsi b/arch/arm64/boot/dts/qcom/sm8250.dtsi
> > > index 39bd8f0eba1e..fe5485256b22 100644
> > > --- a/arch/arm64/boot/dts/qcom/sm8250.dtsi
> > > +++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi
> > > @@ -2203,6 +2203,16 @@ pcie0: pcie@1c00000 {
> > >  			dma-coherent;
> > >  
> > >  			status = "disabled";
> > > +
> > > +			pcie@0 {
> > > +				device_type = "pci";
> > > +				reg = <0x0 0x0 0x0 0x0 0x0>;
> > > +				bus-range = <0x01 0xff>;
> > 
> > Hi Mani, most or all of the patches in this series add this
> > "bus-range" property.  IIUC, these are all Root Ports and hence the
> > secondary/subordinate bus numbers should be programmable.
> 
> Right. It is not a functional dependency.
> 
> > If that's the case, I don't think we need to include "bus-range" in DT
> > for them, do we?
> 
> We mostly include it to silence the below bindings check for the
> endpoint device node:
> 
> Warning (pci_device_bus_num): /soc@0/pcie@1c00000/pcie@0/wifi@0: PCI bus number 1 out of range, expected (0 - 0)
> 
> DTC check is happy if the 'bus-range' property is absent in the
> bridge node. But while validating the endpoint node (if defined), it
> currently relies on the parent 'bus-range' property to verify the
> bus number provided in the endpoint 'reg' property.
> 
> I don't know else the check can verify the correctness of the
> endpoint bus number. So deferring to Rob here.

I should know more about how this works in DT, but I don't.

I guess https://git.kernel.org/linus/83d2a0a1e2b9 ("arm64: dts: qcom:
sm8250: Add PCIe bridge node") added this (subsequently renamed to
"pcieport0"):

  +			pcie@0 {
  +				device_type = "pci";
  +				reg = <0x0 0x0 0x0 0x0 0x0>;
  +				bus-range = <0x01 0xff>;

which is used at places like
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/qcom/qrb5165-rb5.dts?id=v6.12#n788:

  &pcieport0 {
	  wifi@0 {
		  compatible = "pci17cb,1101";
		  reg = <0x10000 0x0 0x0 0x0 0x0>;

Based on
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/pci/pci.txt?id=v6.12#n46
(which is written for Root Ports and Switch Ports, but presumably
applies to endpoints like wifi as well), "reg" contains the device's
bus/device/function:

  - reg:
     Identifies the PCI-PCI bridge. As defined in the IEEE Std 1275-1994
     document, it is a five-cell address encoded as (phys.hi phys.mid
     phys.lo size.hi size.lo). phys.hi should contain the device's BDF as
     0b00000000 bbbbbbbb dddddfff 00000000. The other cells should be zero.

So 0x10000 would decode to 01:00.0, which matches the <1 1> bus-range.

I don't know the reason for requiring the BDF there, but the venerable
https://www.devicetree.org/open-firmware/bindings/pci/pci2_1.pdf, sec
4.1.1, says "reg" is mandatory for PCI Child Nodes, and the first
entry must be the config space address (bus/device/function).

I suppose maybe the BDF is needed to associate the properties with the
correct device, and if the OS were to reprogram the bridge secondary
bus number, it would have to remember the original value to preserve
this association.  I don't think Linux *does* remember that, but it
also generally leaves the bridge bus numbers alone.

Bjorn

Re: [PATCH v2 01/21] arm64: dts: qcom: sm8250: Add PCIe bridge node

[Manivannan Sadhasivam]

On Mon, Jan 06, 2025 at 05:07:05PM -0600, Bjorn Helgaas wrote:
> [+cc Nícolas]
> 
> On Sun, Jan 05, 2025 at 03:46:12PM +0530, Manivannan Sadhasivam wrote:
> > On Fri, Jan 03, 2025 at 03:05:31PM -0600, Bjorn Helgaas wrote:
> > > On Thu, Mar 21, 2024 at 04:46:21PM +0530, Manivannan Sadhasivam wrote:
> > > > On Qcom SoCs, the PCIe host bridge is connected to a single PCIe bridge
> > > > for each controller instance. Hence, add a node to represent the bridge.
> > > > 
> > > > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > > > ---
> > > >  arch/arm64/boot/dts/qcom/sm8250.dtsi | 30 ++++++++++++++++++++++++++++++
> > > >  1 file changed, 30 insertions(+)
> > > > 
> > > > diff --git a/arch/arm64/boot/dts/qcom/sm8250.dtsi b/arch/arm64/boot/dts/qcom/sm8250.dtsi
> > > > index 39bd8f0eba1e..fe5485256b22 100644
> > > > --- a/arch/arm64/boot/dts/qcom/sm8250.dtsi
> > > > +++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi
> > > > @@ -2203,6 +2203,16 @@ pcie0: pcie@1c00000 {
> > > >  			dma-coherent;
> > > >  
> > > >  			status = "disabled";
> > > > +
> > > > +			pcie@0 {
> > > > +				device_type = "pci";
> > > > +				reg = <0x0 0x0 0x0 0x0 0x0>;
> > > > +				bus-range = <0x01 0xff>;
> > > 
> > > Hi Mani, most or all of the patches in this series add this
> > > "bus-range" property.  IIUC, these are all Root Ports and hence the
> > > secondary/subordinate bus numbers should be programmable.
> > 
> > Right. It is not a functional dependency.
> > 
> > > If that's the case, I don't think we need to include "bus-range" in DT
> > > for them, do we?
> > 
> > We mostly include it to silence the below bindings check for the
> > endpoint device node:
> > 
> > Warning (pci_device_bus_num): /soc@0/pcie@1c00000/pcie@0/wifi@0: PCI bus number 1 out of range, expected (0 - 0)
> > 
> > DTC check is happy if the 'bus-range' property is absent in the
> > bridge node. But while validating the endpoint node (if defined), it
> > currently relies on the parent 'bus-range' property to verify the
> > bus number provided in the endpoint 'reg' property.
> > 
> > I don't know else the check can verify the correctness of the
> > endpoint bus number. So deferring to Rob here.
> 
> I should know more about how this works in DT, but I don't.
> 
> I guess https://git.kernel.org/linus/83d2a0a1e2b9 ("arm64: dts: qcom:
> sm8250: Add PCIe bridge node") added this (subsequently renamed to
> "pcieport0"):
> 
>   +			pcie@0 {
>   +				device_type = "pci";
>   +				reg = <0x0 0x0 0x0 0x0 0x0>;
>   +				bus-range = <0x01 0xff>;
> 
> which is used at places like
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/qcom/qrb5165-rb5.dts?id=v6.12#n788:
> 
>   &pcieport0 {
> 	  wifi@0 {
> 		  compatible = "pci17cb,1101";
> 		  reg = <0x10000 0x0 0x0 0x0 0x0>;
> 
> Based on
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/pci/pci.txt?id=v6.12#n46
> (which is written for Root Ports and Switch Ports, but presumably
> applies to endpoints like wifi as well), "reg" contains the device's
> bus/device/function:
> 
>   - reg:
>      Identifies the PCI-PCI bridge. As defined in the IEEE Std 1275-1994
>      document, it is a five-cell address encoded as (phys.hi phys.mid
>      phys.lo size.hi size.lo). phys.hi should contain the device's BDF as
>      0b00000000 bbbbbbbb dddddfff 00000000. The other cells should be zero.
> 
> So 0x10000 would decode to 01:00.0, which matches the <1 1> bus-range.
> 
> I don't know the reason for requiring the BDF there, but the venerable
> https://www.devicetree.org/open-firmware/bindings/pci/pci2_1.pdf, sec
> 4.1.1, says "reg" is mandatory for PCI Child Nodes, and the first
> entry must be the config space address (bus/device/function).
> 
> I suppose maybe the BDF is needed to associate the properties with the
> correct device, and if the OS were to reprogram the bridge secondary
> bus number, it would have to remember the original value to preserve
> this association.  I don't think Linux *does* remember that, but it
> also generally leaves the bridge bus numbers alone.
> 

Device drivers need to parse the properties defined in the device DT node. And
the only way to identify the node is by using its 'reg' property which has the
BDF identifier. This is common to other busses where the device address is
encoded in the 'reg' property.

- Mani

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH v2 01/21] arm64: dts: qcom: sm8250: Add PCIe bridge node

[Bjorn Helgaas]

On Wed, Jan 15, 2025 at 04:24:31PM +0530, Manivannan Sadhasivam wrote:
> On Mon, Jan 06, 2025 at 05:07:05PM -0600, Bjorn Helgaas wrote:
> > On Sun, Jan 05, 2025 at 03:46:12PM +0530, Manivannan Sadhasivam wrote:
> > > On Fri, Jan 03, 2025 at 03:05:31PM -0600, Bjorn Helgaas wrote:
> > > > On Thu, Mar 21, 2024 at 04:46:21PM +0530, Manivannan Sadhasivam wrote:
> > > > > On Qcom SoCs, the PCIe host bridge is connected to a single PCIe bridge
> > > > > for each controller instance. Hence, add a node to represent the bridge.
> > > > > 
> > > > > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > > > > ---
> > > > >  arch/arm64/boot/dts/qcom/sm8250.dtsi | 30 ++++++++++++++++++++++++++++++
> > > > >  1 file changed, 30 insertions(+)
> > > > > 
> > > > > diff --git a/arch/arm64/boot/dts/qcom/sm8250.dtsi b/arch/arm64/boot/dts/qcom/sm8250.dtsi
> > > > > index 39bd8f0eba1e..fe5485256b22 100644
> > > > > --- a/arch/arm64/boot/dts/qcom/sm8250.dtsi
> > > > > +++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi
> > > > > @@ -2203,6 +2203,16 @@ pcie0: pcie@1c00000 {
> > > > >  			dma-coherent;
> > > > >  
> > > > >  			status = "disabled";
> > > > > +
> > > > > +			pcie@0 {
> > > > > +				device_type = "pci";
> > > > > +				reg = <0x0 0x0 0x0 0x0 0x0>;
> > > > > +				bus-range = <0x01 0xff>;
> > > > 
> > > > Hi Mani, most or all of the patches in this series add this
> > > > "bus-range" property.  IIUC, these are all Root Ports and hence the
> > > > secondary/subordinate bus numbers should be programmable.
> > > 
> > > Right. It is not a functional dependency.
> > > 
> > > > If that's the case, I don't think we need to include "bus-range" in DT
> > > > for them, do we?
> > > 
> > > We mostly include it to silence the below bindings check for the
> > > endpoint device node:
> > > 
> > > Warning (pci_device_bus_num): /soc@0/pcie@1c00000/pcie@0/wifi@0: PCI bus number 1 out of range, expected (0 - 0)
> > > 
> > > DTC check is happy if the 'bus-range' property is absent in the
> > > bridge node. But while validating the endpoint node (if defined), it
> > > currently relies on the parent 'bus-range' property to verify the
> > > bus number provided in the endpoint 'reg' property.
> > > 
> > > I don't know else the check can verify the correctness of the
> > > endpoint bus number. So deferring to Rob here.
> > 
> > I should know more about how this works in DT, but I don't.
> > 
> > I guess https://git.kernel.org/linus/83d2a0a1e2b9 ("arm64: dts: qcom:
> > sm8250: Add PCIe bridge node") added this (subsequently renamed to
> > "pcieport0"):
> > 
> >   +			pcie@0 {
> >   +				device_type = "pci";
> >   +				reg = <0x0 0x0 0x0 0x0 0x0>;
> >   +				bus-range = <0x01 0xff>;
> > 
> > which is used at places like
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/qcom/qrb5165-rb5.dts?id=v6.12#n788:
> > 
> >   &pcieport0 {
> > 	  wifi@0 {
> > 		  compatible = "pci17cb,1101";
> > 		  reg = <0x10000 0x0 0x0 0x0 0x0>;
> > 
> > Based on
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/pci/pci.txt?id=v6.12#n46
> > (which is written for Root Ports and Switch Ports, but presumably
> > applies to endpoints like wifi as well), "reg" contains the device's
> > bus/device/function:
> > 
> >   - reg:
> >      Identifies the PCI-PCI bridge. As defined in the IEEE Std 1275-1994
> >      document, it is a five-cell address encoded as (phys.hi phys.mid
> >      phys.lo size.hi size.lo). phys.hi should contain the device's BDF as
> >      0b00000000 bbbbbbbb dddddfff 00000000. The other cells should be zero.
> > 
> > So 0x10000 would decode to 01:00.0, which matches the <1 1> bus-range.
> > 
> > I don't know the reason for requiring the BDF there, but the venerable
> > https://www.devicetree.org/open-firmware/bindings/pci/pci2_1.pdf, sec
> > 4.1.1, says "reg" is mandatory for PCI Child Nodes, and the first
> > entry must be the config space address (bus/device/function).
> > 
> > I suppose maybe the BDF is needed to associate the properties with the
> > correct device, and if the OS were to reprogram the bridge secondary
> > bus number, it would have to remember the original value to preserve
> > this association.  I don't think Linux *does* remember that, but it
> > also generally leaves the bridge bus numbers alone.
> 
> Device drivers need to parse the properties defined in the device DT
> node. And the only way to identify the node is by using its 'reg'
> property which has the BDF identifier. This is common to other
> busses where the device address is encoded in the 'reg' property.

Does this assume there is some firmware to configure these bridges
before Linux boots?  If bridges are completely unconfigured after
power-on, their secondary and subordinate bus numbers will be zero, so
a bus-range property for the bridge can only be an assumption about
what Linux will do.

Bjorn

Re: [PATCH v2 01/21] arm64: dts: qcom: sm8250: Add PCIe bridge node

[Manivannan Sadhasivam]

On Wed, Jan 15, 2025 at 11:42:10AM -0600, Bjorn Helgaas wrote:
> On Wed, Jan 15, 2025 at 04:24:31PM +0530, Manivannan Sadhasivam wrote:
> > On Mon, Jan 06, 2025 at 05:07:05PM -0600, Bjorn Helgaas wrote:
> > > On Sun, Jan 05, 2025 at 03:46:12PM +0530, Manivannan Sadhasivam wrote:
> > > > On Fri, Jan 03, 2025 at 03:05:31PM -0600, Bjorn Helgaas wrote:
> > > > > On Thu, Mar 21, 2024 at 04:46:21PM +0530, Manivannan Sadhasivam wrote:
> > > > > > On Qcom SoCs, the PCIe host bridge is connected to a single PCIe bridge
> > > > > > for each controller instance. Hence, add a node to represent the bridge.
> > > > > > 
> > > > > > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > > > > > ---
> > > > > >  arch/arm64/boot/dts/qcom/sm8250.dtsi | 30 ++++++++++++++++++++++++++++++
> > > > > >  1 file changed, 30 insertions(+)
> > > > > > 
> > > > > > diff --git a/arch/arm64/boot/dts/qcom/sm8250.dtsi b/arch/arm64/boot/dts/qcom/sm8250.dtsi
> > > > > > index 39bd8f0eba1e..fe5485256b22 100644
> > > > > > --- a/arch/arm64/boot/dts/qcom/sm8250.dtsi
> > > > > > +++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi
> > > > > > @@ -2203,6 +2203,16 @@ pcie0: pcie@1c00000 {
> > > > > >  			dma-coherent;
> > > > > >  
> > > > > >  			status = "disabled";
> > > > > > +
> > > > > > +			pcie@0 {
> > > > > > +				device_type = "pci";
> > > > > > +				reg = <0x0 0x0 0x0 0x0 0x0>;
> > > > > > +				bus-range = <0x01 0xff>;
> > > > > 
> > > > > Hi Mani, most or all of the patches in this series add this
> > > > > "bus-range" property.  IIUC, these are all Root Ports and hence the
> > > > > secondary/subordinate bus numbers should be programmable.
> > > > 
> > > > Right. It is not a functional dependency.
> > > > 
> > > > > If that's the case, I don't think we need to include "bus-range" in DT
> > > > > for them, do we?
> > > > 
> > > > We mostly include it to silence the below bindings check for the
> > > > endpoint device node:
> > > > 
> > > > Warning (pci_device_bus_num): /soc@0/pcie@1c00000/pcie@0/wifi@0: PCI bus number 1 out of range, expected (0 - 0)
> > > > 
> > > > DTC check is happy if the 'bus-range' property is absent in the
> > > > bridge node. But while validating the endpoint node (if defined), it
> > > > currently relies on the parent 'bus-range' property to verify the
> > > > bus number provided in the endpoint 'reg' property.
> > > > 
> > > > I don't know else the check can verify the correctness of the
> > > > endpoint bus number. So deferring to Rob here.
> > > 
> > > I should know more about how this works in DT, but I don't.
> > > 
> > > I guess https://git.kernel.org/linus/83d2a0a1e2b9 ("arm64: dts: qcom:
> > > sm8250: Add PCIe bridge node") added this (subsequently renamed to
> > > "pcieport0"):
> > > 
> > >   +			pcie@0 {
> > >   +				device_type = "pci";
> > >   +				reg = <0x0 0x0 0x0 0x0 0x0>;
> > >   +				bus-range = <0x01 0xff>;
> > > 
> > > which is used at places like
> > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/qcom/qrb5165-rb5.dts?id=v6.12#n788:
> > > 
> > >   &pcieport0 {
> > > 	  wifi@0 {
> > > 		  compatible = "pci17cb,1101";
> > > 		  reg = <0x10000 0x0 0x0 0x0 0x0>;
> > > 
> > > Based on
> > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/pci/pci.txt?id=v6.12#n46
> > > (which is written for Root Ports and Switch Ports, but presumably
> > > applies to endpoints like wifi as well), "reg" contains the device's
> > > bus/device/function:
> > > 
> > >   - reg:
> > >      Identifies the PCI-PCI bridge. As defined in the IEEE Std 1275-1994
> > >      document, it is a five-cell address encoded as (phys.hi phys.mid
> > >      phys.lo size.hi size.lo). phys.hi should contain the device's BDF as
> > >      0b00000000 bbbbbbbb dddddfff 00000000. The other cells should be zero.
> > > 
> > > So 0x10000 would decode to 01:00.0, which matches the <1 1> bus-range.
> > > 
> > > I don't know the reason for requiring the BDF there, but the venerable
> > > https://www.devicetree.org/open-firmware/bindings/pci/pci2_1.pdf, sec
> > > 4.1.1, says "reg" is mandatory for PCI Child Nodes, and the first
> > > entry must be the config space address (bus/device/function).
> > > 
> > > I suppose maybe the BDF is needed to associate the properties with the
> > > correct device, and if the OS were to reprogram the bridge secondary
> > > bus number, it would have to remember the original value to preserve
> > > this association.  I don't think Linux *does* remember that, but it
> > > also generally leaves the bridge bus numbers alone.
> > 
> > Device drivers need to parse the properties defined in the device DT
> > node. And the only way to identify the node is by using its 'reg'
> > property which has the BDF identifier. This is common to other
> > busses where the device address is encoded in the 'reg' property.
> 
> Does this assume there is some firmware to configure these bridges
> before Linux boots?

No.

>  If bridges are completely unconfigured after
> power-on, their secondary and subordinate bus numbers will be zero, so
> a bus-range property for the bridge can only be an assumption about
> what Linux will do.
> 

Secondary bus number for sure is not an assumption as it depends on the hardware
topology which linux would know from DT. But subordinate number could be
considered as an assumption.

FWIW, DT is not tied to any OS. So linux assumption doesn't really matter even
though it is the most commonly used reference.

- Mani

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH v2 01/21] arm64: dts: qcom: sm8250: Add PCIe bridge node

[Bjorn Helgaas]

On Wed, Jan 15, 2025 at 11:29:18PM +0530, Manivannan Sadhasivam wrote:
> On Wed, Jan 15, 2025 at 11:42:10AM -0600, Bjorn Helgaas wrote:
> > On Wed, Jan 15, 2025 at 04:24:31PM +0530, Manivannan Sadhasivam wrote:
> > > On Mon, Jan 06, 2025 at 05:07:05PM -0600, Bjorn Helgaas wrote:
> > > > On Sun, Jan 05, 2025 at 03:46:12PM +0530, Manivannan Sadhasivam wrote:
> > > > > On Fri, Jan 03, 2025 at 03:05:31PM -0600, Bjorn Helgaas wrote:
> > > > > > On Thu, Mar 21, 2024 at 04:46:21PM +0530, Manivannan Sadhasivam wrote:
> > > > > > > On Qcom SoCs, the PCIe host bridge is connected to a single PCIe bridge
> > > > > > > for each controller instance. Hence, add a node to represent the bridge.
> > > > > > > 
> > > > > > > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > > > > > > ---
> > > > > > >  arch/arm64/boot/dts/qcom/sm8250.dtsi | 30 ++++++++++++++++++++++++++++++
> > > > > > >  1 file changed, 30 insertions(+)
> > > > > > > 
> > > > > > > diff --git a/arch/arm64/boot/dts/qcom/sm8250.dtsi b/arch/arm64/boot/dts/qcom/sm8250.dtsi
> > > > > > > index 39bd8f0eba1e..fe5485256b22 100644
> > > > > > > --- a/arch/arm64/boot/dts/qcom/sm8250.dtsi
> > > > > > > +++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi
> > > > > > > @@ -2203,6 +2203,16 @@ pcie0: pcie@1c00000 {
> > > > > > >  			dma-coherent;
> > > > > > >  
> > > > > > >  			status = "disabled";
> > > > > > > +
> > > > > > > +			pcie@0 {
> > > > > > > +				device_type = "pci";
> > > > > > > +				reg = <0x0 0x0 0x0 0x0 0x0>;
> > > > > > > +				bus-range = <0x01 0xff>;
> > > > > > 
> > > > > > Hi Mani, most or all of the patches in this series add this
> > > > > > "bus-range" property.  IIUC, these are all Root Ports and hence the
> > > > > > secondary/subordinate bus numbers should be programmable.
> > > > > 
> > > > > Right. It is not a functional dependency.
> > > > > 
> > > > > > If that's the case, I don't think we need to include "bus-range" in DT
> > > > > > for them, do we?
> > > > > 
> > > > > We mostly include it to silence the below bindings check for the
> > > > > endpoint device node:
> > > > > 
> > > > > Warning (pci_device_bus_num): /soc@0/pcie@1c00000/pcie@0/wifi@0: PCI bus number 1 out of range, expected (0 - 0)
> > > > > 
> > > > > DTC check is happy if the 'bus-range' property is absent in the
> > > > > bridge node. But while validating the endpoint node (if defined), it
> > > > > currently relies on the parent 'bus-range' property to verify the
> > > > > bus number provided in the endpoint 'reg' property.
> > > > > 
> > > > > I don't know else the check can verify the correctness of the
> > > > > endpoint bus number. So deferring to Rob here.
> > > > 
> > > > I should know more about how this works in DT, but I don't.
> > > > 
> > > > I guess https://git.kernel.org/linus/83d2a0a1e2b9 ("arm64: dts: qcom:
> > > > sm8250: Add PCIe bridge node") added this (subsequently renamed to
> > > > "pcieport0"):
> > > > 
> > > >   +			pcie@0 {
> > > >   +				device_type = "pci";
> > > >   +				reg = <0x0 0x0 0x0 0x0 0x0>;
> > > >   +				bus-range = <0x01 0xff>;
> > > > 
> > > > which is used at places like
> > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/qcom/qrb5165-rb5.dts?id=v6.12#n788:
> > > > 
> > > >   &pcieport0 {
> > > > 	  wifi@0 {
> > > > 		  compatible = "pci17cb,1101";
> > > > 		  reg = <0x10000 0x0 0x0 0x0 0x0>;
> > > > 
> > > > Based on
> > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/pci/pci.txt?id=v6.12#n46
> > > > (which is written for Root Ports and Switch Ports, but presumably
> > > > applies to endpoints like wifi as well), "reg" contains the device's
> > > > bus/device/function:
> > > > 
> > > >   - reg:
> > > >      Identifies the PCI-PCI bridge. As defined in the IEEE Std 1275-1994
> > > >      document, it is a five-cell address encoded as (phys.hi phys.mid
> > > >      phys.lo size.hi size.lo). phys.hi should contain the device's BDF as
> > > >      0b00000000 bbbbbbbb dddddfff 00000000. The other cells should be zero.
> > > > 
> > > > So 0x10000 would decode to 01:00.0, which matches the <1 1> bus-range.
> > > > 
> > > > I don't know the reason for requiring the BDF there, but the venerable
> > > > https://www.devicetree.org/open-firmware/bindings/pci/pci2_1.pdf, sec
> > > > 4.1.1, says "reg" is mandatory for PCI Child Nodes, and the first
> > > > entry must be the config space address (bus/device/function).
> > > > 
> > > > I suppose maybe the BDF is needed to associate the properties with the
> > > > correct device, and if the OS were to reprogram the bridge secondary
> > > > bus number, it would have to remember the original value to preserve
> > > > this association.  I don't think Linux *does* remember that, but it
> > > > also generally leaves the bridge bus numbers alone.
> > > 
> > > Device drivers need to parse the properties defined in the device DT
> > > node. And the only way to identify the node is by using its 'reg'
> > > property which has the BDF identifier. This is common to other
> > > busses where the device address is encoded in the 'reg' property.
> > 
> > Does this assume there is some firmware to configure these bridges
> > before Linux boots?
> 
> No.
> 
> >  If bridges are completely unconfigured after
> > power-on, their secondary and subordinate bus numbers will be zero, so
> > a bus-range property for the bridge can only be an assumption about
> > what Linux will do.
> 
> Secondary bus number for sure is not an assumption as it depends on
> the hardware topology which linux would know from DT. But
> subordinate number could be considered as an assumption.

If there's no firmware and the secondary bus number is 0 when Linux
enumerates the bridge, does Linux know how to get the bus-range from
DT and program the bridge's secondary bus?

And does Linux know how to update the subordinate bus number in the
case where several Root Ports specify 0xff in bus-range?

Bjorn

Re: [PATCH v2 01/21] arm64: dts: qcom: sm8250: Add PCIe bridge node

[Manivannan Sadhasivam]

On Wed, Jan 15, 2025 at 12:13:40PM -0600, Bjorn Helgaas wrote:
> On Wed, Jan 15, 2025 at 11:29:18PM +0530, Manivannan Sadhasivam wrote:
> > On Wed, Jan 15, 2025 at 11:42:10AM -0600, Bjorn Helgaas wrote:
> > > On Wed, Jan 15, 2025 at 04:24:31PM +0530, Manivannan Sadhasivam wrote:
> > > > On Mon, Jan 06, 2025 at 05:07:05PM -0600, Bjorn Helgaas wrote:
> > > > > On Sun, Jan 05, 2025 at 03:46:12PM +0530, Manivannan Sadhasivam wrote:
> > > > > > On Fri, Jan 03, 2025 at 03:05:31PM -0600, Bjorn Helgaas wrote:
> > > > > > > On Thu, Mar 21, 2024 at 04:46:21PM +0530, Manivannan Sadhasivam wrote:
> > > > > > > > On Qcom SoCs, the PCIe host bridge is connected to a single PCIe bridge
> > > > > > > > for each controller instance. Hence, add a node to represent the bridge.
> > > > > > > > 
> > > > > > > > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > > > > > > > ---
> > > > > > > >  arch/arm64/boot/dts/qcom/sm8250.dtsi | 30 ++++++++++++++++++++++++++++++
> > > > > > > >  1 file changed, 30 insertions(+)
> > > > > > > > 
> > > > > > > > diff --git a/arch/arm64/boot/dts/qcom/sm8250.dtsi b/arch/arm64/boot/dts/qcom/sm8250.dtsi
> > > > > > > > index 39bd8f0eba1e..fe5485256b22 100644
> > > > > > > > --- a/arch/arm64/boot/dts/qcom/sm8250.dtsi
> > > > > > > > +++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi
> > > > > > > > @@ -2203,6 +2203,16 @@ pcie0: pcie@1c00000 {
> > > > > > > >  			dma-coherent;
> > > > > > > >  
> > > > > > > >  			status = "disabled";
> > > > > > > > +
> > > > > > > > +			pcie@0 {
> > > > > > > > +				device_type = "pci";
> > > > > > > > +				reg = <0x0 0x0 0x0 0x0 0x0>;
> > > > > > > > +				bus-range = <0x01 0xff>;
> > > > > > > 
> > > > > > > Hi Mani, most or all of the patches in this series add this
> > > > > > > "bus-range" property.  IIUC, these are all Root Ports and hence the
> > > > > > > secondary/subordinate bus numbers should be programmable.
> > > > > > 
> > > > > > Right. It is not a functional dependency.
> > > > > > 
> > > > > > > If that's the case, I don't think we need to include "bus-range" in DT
> > > > > > > for them, do we?
> > > > > > 
> > > > > > We mostly include it to silence the below bindings check for the
> > > > > > endpoint device node:
> > > > > > 
> > > > > > Warning (pci_device_bus_num): /soc@0/pcie@1c00000/pcie@0/wifi@0: PCI bus number 1 out of range, expected (0 - 0)
> > > > > > 
> > > > > > DTC check is happy if the 'bus-range' property is absent in the
> > > > > > bridge node. But while validating the endpoint node (if defined), it
> > > > > > currently relies on the parent 'bus-range' property to verify the
> > > > > > bus number provided in the endpoint 'reg' property.
> > > > > > 
> > > > > > I don't know else the check can verify the correctness of the
> > > > > > endpoint bus number. So deferring to Rob here.
> > > > > 
> > > > > I should know more about how this works in DT, but I don't.
> > > > > 
> > > > > I guess https://git.kernel.org/linus/83d2a0a1e2b9 ("arm64: dts: qcom:
> > > > > sm8250: Add PCIe bridge node") added this (subsequently renamed to
> > > > > "pcieport0"):
> > > > > 
> > > > >   +			pcie@0 {
> > > > >   +				device_type = "pci";
> > > > >   +				reg = <0x0 0x0 0x0 0x0 0x0>;
> > > > >   +				bus-range = <0x01 0xff>;
> > > > > 
> > > > > which is used at places like
> > > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/qcom/qrb5165-rb5.dts?id=v6.12#n788:
> > > > > 
> > > > >   &pcieport0 {
> > > > > 	  wifi@0 {
> > > > > 		  compatible = "pci17cb,1101";
> > > > > 		  reg = <0x10000 0x0 0x0 0x0 0x0>;
> > > > > 
> > > > > Based on
> > > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/pci/pci.txt?id=v6.12#n46
> > > > > (which is written for Root Ports and Switch Ports, but presumably
> > > > > applies to endpoints like wifi as well), "reg" contains the device's
> > > > > bus/device/function:
> > > > > 
> > > > >   - reg:
> > > > >      Identifies the PCI-PCI bridge. As defined in the IEEE Std 1275-1994
> > > > >      document, it is a five-cell address encoded as (phys.hi phys.mid
> > > > >      phys.lo size.hi size.lo). phys.hi should contain the device's BDF as
> > > > >      0b00000000 bbbbbbbb dddddfff 00000000. The other cells should be zero.
> > > > > 
> > > > > So 0x10000 would decode to 01:00.0, which matches the <1 1> bus-range.
> > > > > 
> > > > > I don't know the reason for requiring the BDF there, but the venerable
> > > > > https://www.devicetree.org/open-firmware/bindings/pci/pci2_1.pdf, sec
> > > > > 4.1.1, says "reg" is mandatory for PCI Child Nodes, and the first
> > > > > entry must be the config space address (bus/device/function).
> > > > > 
> > > > > I suppose maybe the BDF is needed to associate the properties with the
> > > > > correct device, and if the OS were to reprogram the bridge secondary
> > > > > bus number, it would have to remember the original value to preserve
> > > > > this association.  I don't think Linux *does* remember that, but it
> > > > > also generally leaves the bridge bus numbers alone.
> > > > 
> > > > Device drivers need to parse the properties defined in the device DT
> > > > node. And the only way to identify the node is by using its 'reg'
> > > > property which has the BDF identifier. This is common to other
> > > > busses where the device address is encoded in the 'reg' property.
> > > 
> > > Does this assume there is some firmware to configure these bridges
> > > before Linux boots?
> > 
> > No.
> > 
> > >  If bridges are completely unconfigured after
> > > power-on, their secondary and subordinate bus numbers will be zero, so
> > > a bus-range property for the bridge can only be an assumption about
> > > what Linux will do.
> > 
> > Secondary bus number for sure is not an assumption as it depends on
> > the hardware topology which linux would know from DT. But
> > subordinate number could be considered as an assumption.
> 
> If there's no firmware and the secondary bus number is 0 when Linux
> enumerates the bridge, does Linux know how to get the bus-range from
> DT and program the bridge's secondary bus?
> 

Linux doesn't seem to make use of the secondary bus number from DT node of a
bridge, but there is no guarantee that other OSes making use of DT won't do.

> And does Linux know how to update the subordinate bus number in the
> case where several Root Ports specify 0xff in bus-range?
> 

Same answer as above.

- Mani

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH v2 01/21] arm64: dts: qcom: sm8250: Add PCIe bridge node

[Bjorn Helgaas]

On Sun, Jan 19, 2025 at 08:55:34PM +0530, Manivannan Sadhasivam wrote:
> On Wed, Jan 15, 2025 at 12:13:40PM -0600, Bjorn Helgaas wrote:
> > On Wed, Jan 15, 2025 at 11:29:18PM +0530, Manivannan Sadhasivam wrote:
> > > On Wed, Jan 15, 2025 at 11:42:10AM -0600, Bjorn Helgaas wrote:
> > > > On Wed, Jan 15, 2025 at 04:24:31PM +0530, Manivannan Sadhasivam wrote:
> > > > > On Mon, Jan 06, 2025 at 05:07:05PM -0600, Bjorn Helgaas wrote:
> > > > > > On Sun, Jan 05, 2025 at 03:46:12PM +0530, Manivannan Sadhasivam wrote:
> > > > > > > On Fri, Jan 03, 2025 at 03:05:31PM -0600, Bjorn Helgaas wrote:
> > > > > > > > On Thu, Mar 21, 2024 at 04:46:21PM +0530, Manivannan Sadhasivam wrote:
> > > > > > > > > On Qcom SoCs, the PCIe host bridge is connected to a single PCIe bridge
> > > > > > > > > for each controller instance. Hence, add a node to represent the bridge.
> > > > > > > > > 
> > > > > > > > > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > > > > > > > > ---
> > > > > > > > >  arch/arm64/boot/dts/qcom/sm8250.dtsi | 30 ++++++++++++++++++++++++++++++
> > > > > > > > >  1 file changed, 30 insertions(+)
> > > > > > > > > 
> > > > > > > > > diff --git a/arch/arm64/boot/dts/qcom/sm8250.dtsi b/arch/arm64/boot/dts/qcom/sm8250.dtsi
> > > > > > > > > index 39bd8f0eba1e..fe5485256b22 100644
> > > > > > > > > --- a/arch/arm64/boot/dts/qcom/sm8250.dtsi
> > > > > > > > > +++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi
> > > > > > > > > @@ -2203,6 +2203,16 @@ pcie0: pcie@1c00000 {
> > > > > > > > >  			dma-coherent;
> > > > > > > > >  
> > > > > > > > >  			status = "disabled";
> > > > > > > > > +
> > > > > > > > > +			pcie@0 {
> > > > > > > > > +				device_type = "pci";
> > > > > > > > > +				reg = <0x0 0x0 0x0 0x0 0x0>;
> > > > > > > > > +				bus-range = <0x01 0xff>;
> > > > > > > > 
> > > > > > > > Hi Mani, most or all of the patches in this series add this
> > > > > > > > "bus-range" property.  IIUC, these are all Root Ports and hence the
> > > > > > > > secondary/subordinate bus numbers should be programmable.
> > > > > > > 
> > > > > > > Right. It is not a functional dependency.
> > > > > > > 
> > > > > > > > If that's the case, I don't think we need to include "bus-range" in DT
> > > > > > > > for them, do we?
> > > > > > > 
> > > > > > > We mostly include it to silence the below bindings check for the
> > > > > > > endpoint device node:
> > > > > > > 
> > > > > > > Warning (pci_device_bus_num): /soc@0/pcie@1c00000/pcie@0/wifi@0: PCI bus number 1 out of range, expected (0 - 0)
> > > > > > > 
> > > > > > > DTC check is happy if the 'bus-range' property is absent in the
> > > > > > > bridge node. But while validating the endpoint node (if defined), it
> > > > > > > currently relies on the parent 'bus-range' property to verify the
> > > > > > > bus number provided in the endpoint 'reg' property.
> > > > > > > 
> > > > > > > I don't know else the check can verify the correctness of the
> > > > > > > endpoint bus number. So deferring to Rob here.
> > > > > > 
> > > > > > I should know more about how this works in DT, but I don't.
> > > > > > 
> > > > > > I guess https://git.kernel.org/linus/83d2a0a1e2b9 ("arm64: dts: qcom:
> > > > > > sm8250: Add PCIe bridge node") added this (subsequently renamed to
> > > > > > "pcieport0"):
> > > > > > 
> > > > > >   +			pcie@0 {
> > > > > >   +				device_type = "pci";
> > > > > >   +				reg = <0x0 0x0 0x0 0x0 0x0>;
> > > > > >   +				bus-range = <0x01 0xff>;
> > > > > > 
> > > > > > which is used at places like
> > > > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/qcom/qrb5165-rb5.dts?id=v6.12#n788:
> > > > > > 
> > > > > >   &pcieport0 {
> > > > > > 	  wifi@0 {
> > > > > > 		  compatible = "pci17cb,1101";
> > > > > > 		  reg = <0x10000 0x0 0x0 0x0 0x0>;
> > > > > > 
> > > > > > Based on
> > > > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/pci/pci.txt?id=v6.12#n46
> > > > > > (which is written for Root Ports and Switch Ports, but presumably
> > > > > > applies to endpoints like wifi as well), "reg" contains the device's
> > > > > > bus/device/function:
> > > > > > 
> > > > > >   - reg:
> > > > > >      Identifies the PCI-PCI bridge. As defined in the IEEE Std 1275-1994
> > > > > >      document, it is a five-cell address encoded as (phys.hi phys.mid
> > > > > >      phys.lo size.hi size.lo). phys.hi should contain the device's BDF as
> > > > > >      0b00000000 bbbbbbbb dddddfff 00000000. The other cells should be zero.
> > > > > > 
> > > > > > So 0x10000 would decode to 01:00.0, which matches the <1 1> bus-range.
> > > > > > 
> > > > > > I don't know the reason for requiring the BDF there, but the venerable
> > > > > > https://www.devicetree.org/open-firmware/bindings/pci/pci2_1.pdf, sec
> > > > > > 4.1.1, says "reg" is mandatory for PCI Child Nodes, and the first
> > > > > > entry must be the config space address (bus/device/function).
> > > > > > 
> > > > > > I suppose maybe the BDF is needed to associate the properties with the
> > > > > > correct device, and if the OS were to reprogram the bridge secondary
> > > > > > bus number, it would have to remember the original value to preserve
> > > > > > this association.  I don't think Linux *does* remember that, but it
> > > > > > also generally leaves the bridge bus numbers alone.
> > > > > 
> > > > > Device drivers need to parse the properties defined in the device DT
> > > > > node. And the only way to identify the node is by using its 'reg'
> > > > > property which has the BDF identifier. This is common to other
> > > > > busses where the device address is encoded in the 'reg' property.
> > > > 
> > > > Does this assume there is some firmware to configure these bridges
> > > > before Linux boots?
> > > 
> > > No.
> > > 
> > > >  If bridges are completely unconfigured after
> > > > power-on, their secondary and subordinate bus numbers will be zero, so
> > > > a bus-range property for the bridge can only be an assumption about
> > > > what Linux will do.
> > > 
> > > Secondary bus number for sure is not an assumption as it depends on
> > > the hardware topology which linux would know from DT. But
> > > subordinate number could be considered as an assumption.
> > 
> > If there's no firmware and the secondary bus number is 0 when Linux
> > enumerates the bridge, does Linux know how to get the bus-range from
> > DT and program the bridge's secondary bus?
> 
> Linux doesn't seem to make use of the secondary bus number from DT node of a
> bridge, but there is no guarantee that other OSes making use of DT won't do.
> 
> > And does Linux know how to update the subordinate bus number in the
> > case where several Root Ports specify 0xff in bus-range?
> 
> Same answer as above.

Let me back up; I don't think we're understanding each other.  This
DT:

  pcie@0 {
    bus-range = <0x01 0xff>;

    &pcieport0 {
      wifi@0 {
	reg = <0x10000 0x0 0x0 0x0 0x0>;

says that wifi@0 is at 01:00.0, which is only true if the pcie@0
secondary bus number is 01.  The power-up default is 00, so it's only
01 if either firmware or Linux has programmed it that way.

I claim this DT assumes the pcie@0 secondary bus number is programmed
either by firmware or Linux.  This makes me a bit nervous because
AFAIK there's nothing that guarantees Linux would choose bus 01.

Maybe Linux just needs to get smarter and program it based on the
start of the DT bus-range?

Re: [PATCH v2 01/21] arm64: dts: qcom: sm8250: Add PCIe bridge node

[Manivannan Sadhasivam]

On Tue, Jan 21, 2025 at 05:11:31PM -0600, Bjorn Helgaas wrote:
> On Sun, Jan 19, 2025 at 08:55:34PM +0530, Manivannan Sadhasivam wrote:
> > On Wed, Jan 15, 2025 at 12:13:40PM -0600, Bjorn Helgaas wrote:
> > > On Wed, Jan 15, 2025 at 11:29:18PM +0530, Manivannan Sadhasivam wrote:
> > > > On Wed, Jan 15, 2025 at 11:42:10AM -0600, Bjorn Helgaas wrote:
> > > > > On Wed, Jan 15, 2025 at 04:24:31PM +0530, Manivannan Sadhasivam wrote:
> > > > > > On Mon, Jan 06, 2025 at 05:07:05PM -0600, Bjorn Helgaas wrote:
> > > > > > > On Sun, Jan 05, 2025 at 03:46:12PM +0530, Manivannan Sadhasivam wrote:
> > > > > > > > On Fri, Jan 03, 2025 at 03:05:31PM -0600, Bjorn Helgaas wrote:
> > > > > > > > > On Thu, Mar 21, 2024 at 04:46:21PM +0530, Manivannan Sadhasivam wrote:
> > > > > > > > > > On Qcom SoCs, the PCIe host bridge is connected to a single PCIe bridge
> > > > > > > > > > for each controller instance. Hence, add a node to represent the bridge.
> > > > > > > > > > 
> > > > > > > > > > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > > > > > > > > > ---
> > > > > > > > > >  arch/arm64/boot/dts/qcom/sm8250.dtsi | 30 ++++++++++++++++++++++++++++++
> > > > > > > > > >  1 file changed, 30 insertions(+)
> > > > > > > > > > 
> > > > > > > > > > diff --git a/arch/arm64/boot/dts/qcom/sm8250.dtsi b/arch/arm64/boot/dts/qcom/sm8250.dtsi
> > > > > > > > > > index 39bd8f0eba1e..fe5485256b22 100644
> > > > > > > > > > --- a/arch/arm64/boot/dts/qcom/sm8250.dtsi
> > > > > > > > > > +++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi
> > > > > > > > > > @@ -2203,6 +2203,16 @@ pcie0: pcie@1c00000 {
> > > > > > > > > >  			dma-coherent;
> > > > > > > > > >  
> > > > > > > > > >  			status = "disabled";
> > > > > > > > > > +
> > > > > > > > > > +			pcie@0 {
> > > > > > > > > > +				device_type = "pci";
> > > > > > > > > > +				reg = <0x0 0x0 0x0 0x0 0x0>;
> > > > > > > > > > +				bus-range = <0x01 0xff>;
> > > > > > > > > 
> > > > > > > > > Hi Mani, most or all of the patches in this series add this
> > > > > > > > > "bus-range" property.  IIUC, these are all Root Ports and hence the
> > > > > > > > > secondary/subordinate bus numbers should be programmable.
> > > > > > > > 
> > > > > > > > Right. It is not a functional dependency.
> > > > > > > > 
> > > > > > > > > If that's the case, I don't think we need to include "bus-range" in DT
> > > > > > > > > for them, do we?
> > > > > > > > 
> > > > > > > > We mostly include it to silence the below bindings check for the
> > > > > > > > endpoint device node:
> > > > > > > > 
> > > > > > > > Warning (pci_device_bus_num): /soc@0/pcie@1c00000/pcie@0/wifi@0: PCI bus number 1 out of range, expected (0 - 0)
> > > > > > > > 
> > > > > > > > DTC check is happy if the 'bus-range' property is absent in the
> > > > > > > > bridge node. But while validating the endpoint node (if defined), it
> > > > > > > > currently relies on the parent 'bus-range' property to verify the
> > > > > > > > bus number provided in the endpoint 'reg' property.
> > > > > > > > 
> > > > > > > > I don't know else the check can verify the correctness of the
> > > > > > > > endpoint bus number. So deferring to Rob here.
> > > > > > > 
> > > > > > > I should know more about how this works in DT, but I don't.
> > > > > > > 
> > > > > > > I guess https://git.kernel.org/linus/83d2a0a1e2b9 ("arm64: dts: qcom:
> > > > > > > sm8250: Add PCIe bridge node") added this (subsequently renamed to
> > > > > > > "pcieport0"):
> > > > > > > 
> > > > > > >   +			pcie@0 {
> > > > > > >   +				device_type = "pci";
> > > > > > >   +				reg = <0x0 0x0 0x0 0x0 0x0>;
> > > > > > >   +				bus-range = <0x01 0xff>;
> > > > > > > 
> > > > > > > which is used at places like
> > > > > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/qcom/qrb5165-rb5.dts?id=v6.12#n788:
> > > > > > > 
> > > > > > >   &pcieport0 {
> > > > > > > 	  wifi@0 {
> > > > > > > 		  compatible = "pci17cb,1101";
> > > > > > > 		  reg = <0x10000 0x0 0x0 0x0 0x0>;
> > > > > > > 
> > > > > > > Based on
> > > > > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/pci/pci.txt?id=v6.12#n46
> > > > > > > (which is written for Root Ports and Switch Ports, but presumably
> > > > > > > applies to endpoints like wifi as well), "reg" contains the device's
> > > > > > > bus/device/function:
> > > > > > > 
> > > > > > >   - reg:
> > > > > > >      Identifies the PCI-PCI bridge. As defined in the IEEE Std 1275-1994
> > > > > > >      document, it is a five-cell address encoded as (phys.hi phys.mid
> > > > > > >      phys.lo size.hi size.lo). phys.hi should contain the device's BDF as
> > > > > > >      0b00000000 bbbbbbbb dddddfff 00000000. The other cells should be zero.
> > > > > > > 
> > > > > > > So 0x10000 would decode to 01:00.0, which matches the <1 1> bus-range.
> > > > > > > 
> > > > > > > I don't know the reason for requiring the BDF there, but the venerable
> > > > > > > https://www.devicetree.org/open-firmware/bindings/pci/pci2_1.pdf, sec
> > > > > > > 4.1.1, says "reg" is mandatory for PCI Child Nodes, and the first
> > > > > > > entry must be the config space address (bus/device/function).
> > > > > > > 
> > > > > > > I suppose maybe the BDF is needed to associate the properties with the
> > > > > > > correct device, and if the OS were to reprogram the bridge secondary
> > > > > > > bus number, it would have to remember the original value to preserve
> > > > > > > this association.  I don't think Linux *does* remember that, but it
> > > > > > > also generally leaves the bridge bus numbers alone.
> > > > > > 
> > > > > > Device drivers need to parse the properties defined in the device DT
> > > > > > node. And the only way to identify the node is by using its 'reg'
> > > > > > property which has the BDF identifier. This is common to other
> > > > > > busses where the device address is encoded in the 'reg' property.
> > > > > 
> > > > > Does this assume there is some firmware to configure these bridges
> > > > > before Linux boots?
> > > > 
> > > > No.
> > > > 
> > > > >  If bridges are completely unconfigured after
> > > > > power-on, their secondary and subordinate bus numbers will be zero, so
> > > > > a bus-range property for the bridge can only be an assumption about
> > > > > what Linux will do.
> > > > 
> > > > Secondary bus number for sure is not an assumption as it depends on
> > > > the hardware topology which linux would know from DT. But
> > > > subordinate number could be considered as an assumption.
> > > 
> > > If there's no firmware and the secondary bus number is 0 when Linux
> > > enumerates the bridge, does Linux know how to get the bus-range from
> > > DT and program the bridge's secondary bus?
> > 
> > Linux doesn't seem to make use of the secondary bus number from DT node of a
> > bridge, but there is no guarantee that other OSes making use of DT won't do.
> > 
> > > And does Linux know how to update the subordinate bus number in the
> > > case where several Root Ports specify 0xff in bus-range?
> > 
> > Same answer as above.
> 
> Let me back up; I don't think we're understanding each other.  This
> DT:
> 
>   pcie@0 {
>     bus-range = <0x01 0xff>;
> 
>     &pcieport0 {
>       wifi@0 {
> 	reg = <0x10000 0x0 0x0 0x0 0x0>;
> 
> says that wifi@0 is at 01:00.0, which is only true if the pcie@0
> secondary bus number is 01.  The power-up default is 00, so it's only
> 01 if either firmware or Linux has programmed it that way.
> 
> I claim this DT assumes the pcie@0 secondary bus number is programmed
> either by firmware or Linux.  This makes me a bit nervous because
> AFAIK there's nothing that guarantees Linux would choose bus 01.
> 

Why do you think so? PCI devices are scanned in a depth-first manner, so the
bus number should match the DT order. Like, while scanning the bus under pcie@0,
it should use 01 as the secondary bus number as it is going to be the first bus
after the root bus. I don't know how linux or any other OS would end up using a
different bus number.
 
Also, do note that the bus numbering also takes into account of the domain
number which is different for each controller instance.

Please let me know if my understanding is wrong.

- Mani

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH v2 01/21] arm64: dts: qcom: sm8250: Add PCIe bridge node

[Bjorn Helgaas]

On Tue, Jan 28, 2025 at 07:15:14PM +0530, Manivannan Sadhasivam wrote:
> On Tue, Jan 21, 2025 at 05:11:31PM -0600, Bjorn Helgaas wrote:
> ...

> > Let me back up; I don't think we're understanding each other.  This
> > DT:
> > 
> >   pcie@0 {
> >     bus-range = <0x01 0xff>;
> > 
> >     &pcieport0 {
> >       wifi@0 {
> > 	reg = <0x10000 0x0 0x0 0x0 0x0>;
> > 
> > says that wifi@0 is at 01:00.0, which is only true if the pcie@0
> > secondary bus number is 01.  The power-up default is 00, so it's
> > only 01 if either firmware or Linux has programmed it that way.
> > 
> > I claim this DT assumes the pcie@0 secondary bus number is
> > programmed either by firmware or Linux.  This makes me a bit
> > nervous because AFAIK there's nothing that guarantees Linux would
> > choose bus 01.
> 
> Why do you think so? PCI devices are scanned in a depth-first
> manner, so the bus number should match the DT order. Like, while
> scanning the bus under pcie@0, it should use 01 as the secondary bus
> number as it is going to be the first bus after the root bus. I
> don't know how linux or any other OS would end up using a different
> bus number.

In this case of the first bridge on the root bus, it's very likely
that the secondary bus will be 01.

If there are more bridges, it's dangerous to make assumptions about
their secondary bus numbers because those bus numbers depend on what
hierarchies have already been enumerated and any additional space
assigned in anticipation of hotplug.

I don't know of any spec that requires the OS to assign bus numbers in
a certain way, and it feels kind of subtle if this kind of DT
description is only reliable for things below the first bridge on a
root bus.

Bjorn

Re: [PATCH v2 01/21] arm64: dts: qcom: sm8250: Add PCIe bridge node

[Manivannan Sadhasivam]

On Tue, Jan 28, 2025 at 10:16:12AM -0600, Bjorn Helgaas wrote:
> On Tue, Jan 28, 2025 at 07:15:14PM +0530, Manivannan Sadhasivam wrote:
> > On Tue, Jan 21, 2025 at 05:11:31PM -0600, Bjorn Helgaas wrote:
> > ...
> 
> > > Let me back up; I don't think we're understanding each other.  This
> > > DT:
> > > 
> > >   pcie@0 {
> > >     bus-range = <0x01 0xff>;
> > > 
> > >     &pcieport0 {
> > >       wifi@0 {
> > > 	reg = <0x10000 0x0 0x0 0x0 0x0>;
> > > 
> > > says that wifi@0 is at 01:00.0, which is only true if the pcie@0
> > > secondary bus number is 01.  The power-up default is 00, so it's
> > > only 01 if either firmware or Linux has programmed it that way.
> > > 
> > > I claim this DT assumes the pcie@0 secondary bus number is
> > > programmed either by firmware or Linux.  This makes me a bit
> > > nervous because AFAIK there's nothing that guarantees Linux would
> > > choose bus 01.
> > 
> > Why do you think so? PCI devices are scanned in a depth-first
> > manner, so the bus number should match the DT order. Like, while
> > scanning the bus under pcie@0, it should use 01 as the secondary bus
> > number as it is going to be the first bus after the root bus. I
> > don't know how linux or any other OS would end up using a different
> > bus number.
> 
> In this case of the first bridge on the root bus, it's very likely
> that the secondary bus will be 01.
> 
> If there are more bridges, it's dangerous to make assumptions about
> their secondary bus numbers because those bus numbers depend on what
> hierarchies have already been enumerated and any additional space
> assigned in anticipation of hotplug.
> 

The enumeration hierarchy should match the DT order. I'm not sure how there can
be a deviation if the firmware enumerates the bridges in order. Or you are
fearing that the firmware *could* do something crazy?

> I don't know of any spec that requires the OS to assign bus numbers in
> a certain way, and it feels kind of subtle if this kind of DT
> description is only reliable for things below the first bridge on a
> root bus.
> 

This is almost similar to how domain numbers are assigned today. For DT based
systems, 'linux,pci-domain' property is parsed to find the domain number if
available. And this domain number is not guaranteed to match the order in which
the RC nodes are defined in DT.

But I guess no one bothered to parse 'bus-range' property for PCI bridges since
it is not bound to change.

- Mani

-- 

Re: [PATCH v2 01/21] arm64: dts: qcom: sm8250: Add PCIe bridge node

[Rob Herring]

On Sun, Jan 05, 2025 at 03:46:12PM +0530, Manivannan Sadhasivam wrote:
> Hi Bjorn,
> 
> On Fri, Jan 03, 2025 at 03:05:31PM -0600, Bjorn Helgaas wrote:
> > On Thu, Mar 21, 2024 at 04:46:21PM +0530, Manivannan Sadhasivam wrote:
> > > On Qcom SoCs, the PCIe host bridge is connected to a single PCIe bridge
> > > for each controller instance. Hence, add a node to represent the bridge.
> > > 
> > > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > > ---
> > >  arch/arm64/boot/dts/qcom/sm8250.dtsi | 30 ++++++++++++++++++++++++++++++
> > >  1 file changed, 30 insertions(+)
> > > 
> > > diff --git a/arch/arm64/boot/dts/qcom/sm8250.dtsi b/arch/arm64/boot/dts/qcom/sm8250.dtsi
> > > index 39bd8f0eba1e..fe5485256b22 100644
> > > --- a/arch/arm64/boot/dts/qcom/sm8250.dtsi
> > > +++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi
> > > @@ -2203,6 +2203,16 @@ pcie0: pcie@1c00000 {
> > >  			dma-coherent;
> > >  
> > >  			status = "disabled";
> > > +
> > > +			pcie@0 {
> > > +				device_type = "pci";
> > > +				reg = <0x0 0x0 0x0 0x0 0x0>;
> > > +				bus-range = <0x01 0xff>;
> > 
> > Hi Mani, most or all of the patches in this series add this
> > "bus-range" property.  IIUC, these are all Root Ports and hence the
> > secondary/subordinate bus numbers should be programmable.
> > 
> 
> Right. It is not a functional dependency.
> 
> > If that's the case, I don't think we need to include "bus-range" in DT
> > for them, do we?
> > 
> 
> We mostly include it to silence the below bindings check for the endpoint device
> node:
> 
> Warning (pci_device_bus_num): /soc@0/pcie@1c00000/pcie@0/wifi@0: PCI bus number 1 out of range, expected (0 - 0)

The mistake is using bus number 1 instead of 0.

> DTC check is happy if the 'bus-range' property is absent in the bridge node. But
> while validating the endpoint node (if defined), it currently relies on the
> parent 'bus-range' property to verify the bus number provided in the endpoint
> 'reg' property.
> 
> I don't know else the check can verify the correctness of the endpoint bus
> number. So deferring to Rob here.

Sorry I'm late to the party, but found this from another thread 
today[1]. More details there.

You should not have 'bus-range' at all in your DT and the bus for every 
BDF address should be 0. You only need 'bus-range' if your h/w is 
broken.

Rob

[1] https://lore.kernel.org/all/CAL_Jsq+z+5_=YXiyCW1sbKDe0cjGNG7Qk=uRQ3efAFTd1J2ayQ@mail.gmail.com/

> 
> - Mani
> 
> -- 
> மணிவண்ணன் சதாசிவம்