[PATCH v7 0/4] Add DT bindings and DT nodes for PCIe and PHY in SC7280

[PATCH v7 0/4] Add DT bindings and DT nodes for PCIe and PHY in SC7280

[Prasad Malisetty]

Changes added in v7:

    * Removed two fallbacks qcom,pcie-sm8250 and snps,dw-pcie.
    * Replaced compatible method in get_resources_2_7_0 with
      flag approach suggested by Bjorn Helgaas .
    * Setting gcc_pcie_1_clk_src as XO in init_2_7_0 for
      gdsc enable.
    * Added specific NVMe GPIO entries for SKU1 and SKU2 support
      in idp.dts and idp2.dts respectively.
    * Moved pcie_1 and pcie_1_phy board specific entries into common
      board file sc7280-idp.dtsi file.

Changes in v6:

    * Removed platform check while setting gcc_pcie_1_pipe_clk_src
      as clk_set_parent will return 0 with nop if platform doesn't
      need to switch pipe clk source.
    * Moved wake-n gpio to board specific file sc7280-idp.dtsi
    * Sorted gpio.h header entry in sc7280.dtsi file

Changes in v5:
    
    * Re ordered PCIe, PHY nodes in Soc and board specific dtsi files.
    * Removed ref_clk entry in current patch [PATCH v4 P4/4].
    * I will add ref clk entry in suspend/ resume commits.
    * Added boolean flag in Soc specific dtsi file to differentiate
      SM8250 and SC7280 platforms. based on boolean flag, platforms will handle
      the pipe clk handling.

Changes in v4 as suggested by Bjorn:

    * Changed pipe clk mux name as gcc_pcie_1_pipe_clk_src.
    * Changed pipe_ext_src as phy_pipe_clk.
    * Updated commit message for [PATCH v4 4/4].

Changes in v3:
    * Changed pipe clock names in dt bindings as pipe_mux and phy_pipe.
    * Moved reset and NVMe GPIO pin configs into board specific file.
    * Updated pipe clk mux commit message.

Changes in v2:
    * Moved pcie pin control settings into IDP file.
    * Replaced pipe_clk_src with pipe_clk_mux in pcie driver
    * Included pipe clk mux setting change set in this series

Prasad Malisetty (4):
  dt-bindings: pci: qcom: Document PCIe bindings for SC7280
  arm64: dts: qcom: sc7280: Add PCIe and PHY related nodes
  arm64: dts: qcom: sc7280: Add PCIe nodes for IDP board
  PCI: qcom: Switch pcie_1_pipe_clk_src after PHY init in SC7280

 .../devicetree/bindings/pci/qcom,pcie.txt          |  17 +++
 arch/arm64/boot/dts/qcom/sc7280-idp.dts            |   9 ++
 arch/arm64/boot/dts/qcom/sc7280-idp.dtsi           |  33 ++++++
 arch/arm64/boot/dts/qcom/sc7280-idp2.dts           |   9 ++
 arch/arm64/boot/dts/qcom/sc7280.dtsi               | 120 +++++++++++++++++++++
 drivers/pci/controller/dwc/pcie-qcom.c             |  89 +++++++++++++--
 6 files changed, 266 insertions(+), 11 deletions(-)

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH v7 1/4] dt-bindings: pci: qcom: Document PCIe bindings for SC7280

[Prasad Malisetty]

Document the PCIe DT bindings for SC7280 SoC.The PCIe IP is similar
to the one used on SM8250. Add the compatible for SC7280.

Signed-off-by: Prasad Malisetty <pmaliset@codeaurora.org>
Acked-by: Rob Herring <robh@kernel.org>
Reviewed-by: Stephen Boyd <swboyd@chromium.org>
---
 Documentation/devicetree/bindings/pci/qcom,pcie.txt | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.txt b/Documentation/devicetree/bindings/pci/qcom,pcie.txt
index 3f64687..ff423cd 100644
--- a/Documentation/devicetree/bindings/pci/qcom,pcie.txt
+++ b/Documentation/devicetree/bindings/pci/qcom,pcie.txt
@@ -12,6 +12,7 @@
 			- "qcom,pcie-ipq4019" for ipq4019
 			- "qcom,pcie-ipq8074" for ipq8074
 			- "qcom,pcie-qcs404" for qcs404
+			- "qcom,pcie-sc7280" for sc7280
 			- "qcom,pcie-sdm845" for sdm845
 			- "qcom,pcie-sm8250" for sm8250
 			- "qcom,pcie-ipq6018" for ipq6018
@@ -144,6 +145,22 @@
 			- "slave_bus"	AXI Slave clock
 
 - clock-names:
+	Usage: required for sc7280
+	Value type: <stringlist>
+	Definition: Should contain the following entries
+			- "aux"         Auxiliary clock
+			- "cfg"         Configuration clock
+			- "bus_master"  Master AXI clock
+			- "bus_slave"   Slave AXI clock
+			- "slave_q2a"   Slave Q2A clock
+			- "tbu"         PCIe TBU clock
+			- "ddrss_sf_tbu" PCIe SF TBU clock
+			- "pipe"        PIPE clock
+			- "pipe_mux"    PIPE MUX
+			- "phy_pipe"    PIPE output clock
+			- "ref"         REFERENCE clock
+
+- clock-names:
 	Usage: required for sdm845
 	Value type: <stringlist>
 	Definition: Should contain the following entries
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH v7 2/4] arm64: dts: qcom: sc7280: Add PCIe and PHY related nodes

[Prasad Malisetty]

Add PCIe controller and PHY nodes for sc7280 SOC.

Signed-off-by: Prasad Malisetty <pmaliset@codeaurora.org>
---
 arch/arm64/boot/dts/qcom/sc7280.dtsi | 120 +++++++++++++++++++++++++++++++++++
 1 file changed, 120 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
index 53a21d0..8f1a0a1 100644
--- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
@@ -7,6 +7,7 @@
 
 #include <dt-bindings/clock/qcom,gcc-sc7280.h>
 #include <dt-bindings/clock/qcom,rpmh.h>
+#include <dt-bindings/gpio/gpio.h>
 #include <dt-bindings/interconnect/qcom,sc7280.h>
 #include <dt-bindings/interrupt-controller/arm-gic.h>
 #include <dt-bindings/mailbox/qcom-ipcc.h>
@@ -586,6 +587,117 @@
 			qcom,bcm-voters = <&apps_bcm_voter>;
 		};
 
+		pcie1: pci@1c08000 {
+			compatible = "qcom,pcie-sc7280";
+			reg = <0 0x01c08000 0 0x3000>,
+			      <0 0x40000000 0 0xf1d>,
+			      <0 0x40000f20 0 0xa8>,
+			      <0 0x40001000 0 0x1000>,
+			      <0 0x40100000 0 0x100000>;
+
+			reg-names = "parf", "dbi", "elbi", "atu", "config";
+			device_type = "pci";
+			linux,pci-domain = <1>;
+			bus-range = <0x00 0xff>;
+			num-lanes = <2>;
+
+			#address-cells = <3>;
+			#size-cells = <2>;
+
+			ranges = <0x01000000 0x0 0x40200000 0x0 0x40200000 0x0 0x100000>,
+				 <0x02000000 0x0 0x40300000 0x0 0x40300000 0x0 0x1fd00000>;
+
+			interrupts = <GIC_SPI 307 IRQ_TYPE_LEVEL_HIGH>;
+			interrupt-names = "msi";
+			#interrupt-cells = <1>;
+			interrupt-map-mask = <0 0 0 0x7>;
+			interrupt-map = <0 0 0 1 &intc 0 434 IRQ_TYPE_LEVEL_HIGH>,
+					<0 0 0 2 &intc 0 435 IRQ_TYPE_LEVEL_HIGH>,
+					<0 0 0 3 &intc 0 438 IRQ_TYPE_LEVEL_HIGH>,
+					<0 0 0 4 &intc 0 439 IRQ_TYPE_LEVEL_HIGH>;
+
+			clocks = <&gcc GCC_PCIE_1_PIPE_CLK>,
+				 <&gcc GCC_PCIE_1_PIPE_CLK_SRC>,
+				 <&pcie1_lane 0>,
+				 <&rpmhcc RPMH_CXO_CLK>,
+				 <&gcc GCC_PCIE_1_AUX_CLK>,
+				 <&gcc GCC_PCIE_1_CFG_AHB_CLK>,
+				 <&gcc GCC_PCIE_1_MSTR_AXI_CLK>,
+				 <&gcc GCC_PCIE_1_SLV_AXI_CLK>,
+				 <&gcc GCC_PCIE_1_SLV_Q2A_AXI_CLK>,
+				 <&gcc GCC_AGGRE_NOC_PCIE_TBU_CLK>,
+				 <&gcc GCC_DDRSS_PCIE_SF_CLK>;
+
+			clock-names = "pipe",
+				      "pipe_mux",
+				      "phy_pipe",
+				      "ref",
+				      "aux",
+				      "cfg",
+				      "bus_master",
+				      "bus_slave",
+				      "slave_q2a",
+				      "tbu",
+				      "ddrss_sf_tbu";
+
+			assigned-clocks = <&gcc GCC_PCIE_1_AUX_CLK>;
+			assigned-clock-rates = <19200000>;
+
+			resets = <&gcc GCC_PCIE_1_BCR>;
+			reset-names = "pci";
+
+			power-domains = <&gcc GCC_PCIE_1_GDSC>;
+
+			phys = <&pcie1_lane>;
+			phy-names = "pciephy";
+
+			pinctrl-names = "default";
+			pinctrl-0 = <&pcie1_default_state>;
+
+			iommus = <&apps_smmu 0x1c80 0x1>;
+
+			iommu-map = <0x0 &apps_smmu 0x1c80 0x1>,
+				    <0x100 &apps_smmu 0x1c81 0x1>;
+
+			status = "disabled";
+		};
+
+		pcie1_phy: phy@1c0e000 {
+			compatible = "qcom,sm8250-qmp-gen3x2-pcie-phy";
+			reg = <0 0x01c0e000 0 0x1c0>;
+			#address-cells = <2>;
+			#size-cells = <2>;
+			ranges;
+			clocks = <&gcc GCC_PCIE_1_AUX_CLK>,
+				 <&gcc GCC_PCIE_1_CFG_AHB_CLK>,
+				 <&gcc GCC_PCIE_CLKREF_EN>,
+				 <&gcc GCC_PCIE1_PHY_RCHNG_CLK>;
+			clock-names = "aux", "cfg_ahb", "ref", "refgen";
+
+			resets = <&gcc GCC_PCIE_1_PHY_BCR>;
+			reset-names = "phy";
+
+			assigned-clocks = <&gcc GCC_PCIE1_PHY_RCHNG_CLK>;
+			assigned-clock-rates = <100000000>;
+
+			status = "disabled";
+
+			pcie1_lane: lanes@1c0e200 {
+				reg = <0 0x01c0e200 0 0x170>,
+				      <0 0x01c0e400 0 0x200>,
+				      <0 0x01c0ea00 0 0x1f0>,
+				      <0 0x01c0e600 0 0x170>,
+				      <0 0x01c0e800 0 0x200>,
+				      <0 0x01c0ee00 0 0xf4>;
+				clocks = <&gcc GCC_PCIE_1_PIPE_CLK>;
+				clock-names = "pipe0";
+
+				#phy-cells = <0>;
+				#clock-cells = <1>;
+				clock-output-names = "pcie_1_pipe_clk";
+			};
+		};
+
 		ipa: ipa@1e40000 {
 			compatible = "qcom,sc7280-ipa";
 
@@ -1513,6 +1625,14 @@
 			gpio-ranges = <&tlmm 0 0 175>;
 			wakeup-parent = <&pdc>;
 
+			pcie1_default_state: pcie1-default-state {
+				clkreq {
+					pins = "gpio79";
+					function = "pcie1_clkreqn";
+					bias-pull-up;
+				};
+			};
+
 			qup_uart5_default: qup-uart5-default {
 				pins = "gpio46", "gpio47";
 				function = "qup13";
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH v7 3/4] arm64: dts: qcom: sc7280: Add PCIe nodes for IDP board

[Prasad Malisetty]

Enable PCIe controller and PHY for sc7280 IDP board.
Add specific NVMe GPIO entries for SKU1 and SKU2 support.

Signed-off-by: Prasad Malisetty <pmaliset@codeaurora.org>
---
 arch/arm64/boot/dts/qcom/sc7280-idp.dts  |  9 +++++++++
 arch/arm64/boot/dts/qcom/sc7280-idp.dtsi | 32 ++++++++++++++++++++++++++++++++
 arch/arm64/boot/dts/qcom/sc7280-idp2.dts |  9 +++++++++
 3 files changed, 50 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp.dts b/arch/arm64/boot/dts/qcom/sc7280-idp.dts
index 64fc22a..2cc6b0a 100644
--- a/arch/arm64/boot/dts/qcom/sc7280-idp.dts
+++ b/arch/arm64/boot/dts/qcom/sc7280-idp.dts
@@ -61,6 +61,15 @@
 	modem-init;
 };
 
+&pcie1_default_state {
+	nvme-n {
+		pins = "gpio19";
+		function = "gpio";
+
+		bias-pull-up;
+	};
+};
+
 &pmk8350_vadc {
 	pmr735a_die_temp {
 		reg = <PMR735A_ADC7_DIE_TEMP>;
diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
index 371a2a9..e73a7c2 100644
--- a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
@@ -199,6 +199,38 @@
 	modem-init;
 };
 
+&pcie1 {
+	status = "okay";
+
+	perst-gpio = <&tlmm 2 GPIO_ACTIVE_LOW>;
+};
+
+&pcie1_phy {
+	status = "okay";
+
+	vdda-phy-supply = <&vreg_l10c_0p8>;
+	vdda-pll-supply = <&vreg_l6b_1p2>;
+};
+
+&pcie1_default_state {
+	reset-n {
+		pins = "gpio2";
+		function = "gpio";
+
+		drive-strength = <16>;
+		output-low;
+		bias-disable;
+	};
+
+	wake-n {
+		pins = "gpio3";
+		function = "gpio";
+
+		drive-strength = <2>;
+		bias-pull-up;
+	};
+};
+
 &pmk8350_vadc {
 	pmk8350_die_temp {
 		reg = <PMK8350_ADC7_DIE_TEMP>;
diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp2.dts b/arch/arm64/boot/dts/qcom/sc7280-idp2.dts
index 1fc2add..8432afc 100644
--- a/arch/arm64/boot/dts/qcom/sc7280-idp2.dts
+++ b/arch/arm64/boot/dts/qcom/sc7280-idp2.dts
@@ -21,3 +21,12 @@
 		stdout-path = "serial0:115200n8";
 	};
 };
+
+&pcie1_default_state {
+	nvme-n {
+		pins = "gpio51";
+		function = "gpio";
+
+		bias-pull-up;
+	};
+};
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH v7 4/4] PCI: qcom: Switch pcie_1_pipe_clk_src after PHY init in SC7280

[Prasad Malisetty]

On the SC7280, the clock source for gcc_pcie_1_pipe_clk_src
must be the TCXO while gdsc is enabled. After PHY init successful
clock source should switch to pipe clock for gcc_pcie_1_pipe_clk_src.

Signed-off-by: Prasad Malisetty <pmaliset@codeaurora.org>
---
 drivers/pci/controller/dwc/pcie-qcom.c | 90 +++++++++++++++++++++++++++++-----
 1 file changed, 79 insertions(+), 11 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index 8a7a300..380c962 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -166,6 +166,9 @@ struct qcom_pcie_resources_2_7_0 {
 	struct regulator_bulk_data supplies[2];
 	struct reset_control *pci_reset;
 	struct clk *pipe_clk;
+	struct clk *gcc_pcie_1_pipe_clk_src;
+	struct clk *phy_pipe_clk;
+	struct clk *ref_clk_src;
 };
 
 union qcom_pcie_resources {
@@ -189,6 +192,11 @@ struct qcom_pcie_ops {
 	int (*config_sid)(struct qcom_pcie *pcie);
 };
 
+struct qcom_pcie_cfg {
+	const struct qcom_pcie_ops *ops;
+	bool pcie_1_pipe_clk_src_switch;
+};
+
 struct qcom_pcie {
 	struct dw_pcie *pci;
 	void __iomem *parf;			/* DT parf */
@@ -197,6 +205,7 @@ struct qcom_pcie {
 	struct phy *phy;
 	struct gpio_desc *reset;
 	const struct qcom_pcie_ops *ops;
+	bool pcie_1_pipe_clk_src_switch;
 };
 
 #define to_qcom_pcie(x)		dev_get_drvdata((x)->dev)
@@ -1167,6 +1176,20 @@ static int qcom_pcie_get_resources_2_7_0(struct qcom_pcie *pcie)
 	if (ret < 0)
 		return ret;
 
+	if (pcie->pcie_1_pipe_clk_src_switch) {
+		res->gcc_pcie_1_pipe_clk_src = devm_clk_get(dev, "pipe_mux");
+		if (IS_ERR(res->gcc_pcie_1_pipe_clk_src))
+			return PTR_ERR(res->gcc_pcie_1_pipe_clk_src);
+
+		res->phy_pipe_clk = devm_clk_get(dev, "phy_pipe");
+		if (IS_ERR(res->phy_pipe_clk))
+			return PTR_ERR(res->phy_pipe_clk);
+
+		res->ref_clk_src = devm_clk_get(dev, "ref");
+		if (IS_ERR(res->ref_clk_src))
+			return PTR_ERR(res->ref_clk_src);
+	}
+
 	res->pipe_clk = devm_clk_get(dev, "pipe");
 	return PTR_ERR_OR_ZERO(res->pipe_clk);
 }
@@ -1185,6 +1208,10 @@ static int qcom_pcie_init_2_7_0(struct qcom_pcie *pcie)
 		return ret;
 	}
 
+	/* Set TCXO as clock source for gcc_pcie_1_pipe_clk_src */
+	if (pcie->pcie_1_pipe_clk_src_switch)
+		clk_set_parent(res->gcc_pcie_1_pipe_clk_src, res->ref_clk_src);
+
 	ret = clk_bulk_prepare_enable(res->num_clks, res->clks);
 	if (ret < 0)
 		goto err_disable_regulators;
@@ -1256,6 +1283,10 @@ static int qcom_pcie_post_init_2_7_0(struct qcom_pcie *pcie)
 {
 	struct qcom_pcie_resources_2_7_0 *res = &pcie->res.v2_7_0;
 
+	/* Set pipe clock as clock source for gcc_pcie_1_pipe_clk_src */
+	if (pcie->pcie_1_pipe_clk_src_switch)
+		clk_set_parent(res->gcc_pcie_1_pipe_clk_src, res->phy_pipe_clk);
+
 	return clk_prepare_enable(res->pipe_clk);
 }
 
@@ -1456,6 +1487,39 @@ static const struct qcom_pcie_ops ops_1_9_0 = {
 	.config_sid = qcom_pcie_config_sid_sm8250,
 };
 
+static const struct qcom_pcie_cfg apq8084_cfg = {
+	.ops = &ops_1_0_0,
+};
+
+static const struct qcom_pcie_cfg ipq8064_cfg = {
+	.ops = &ops_2_1_0,
+};
+
+static const struct qcom_pcie_cfg msm8996_cfg = {
+	.ops = &ops_2_3_2,
+};
+
+static const struct qcom_pcie_cfg ipq8074_cfg = {
+	.ops = &ops_2_3_3,
+};
+
+static const struct qcom_pcie_cfg ipq4019_cfg = {
+	.ops = &ops_2_4_0,
+};
+
+static const struct qcom_pcie_cfg sdm845_cfg = {
+	.ops = &ops_2_7_0,
+};
+
+static const struct qcom_pcie_cfg sm8250_cfg = {
+	.ops = &ops_1_9_0,
+};
+
+static const struct qcom_pcie_cfg sc7280_cfg = {
+	.ops = &ops_1_9_0,
+	.pcie_1_pipe_clk_src_switch = true,
+};
+
 static const struct dw_pcie_ops dw_pcie_ops = {
 	.link_up = qcom_pcie_link_up,
 	.start_link = qcom_pcie_start_link,
@@ -1467,6 +1531,7 @@ static int qcom_pcie_probe(struct platform_device *pdev)
 	struct pcie_port *pp;
 	struct dw_pcie *pci;
 	struct qcom_pcie *pcie;
+	const struct qcom_pcie_cfg *pcie_cfg = NULL;
 	int ret;
 
 	pcie = devm_kzalloc(dev, sizeof(*pcie), GFP_KERNEL);
@@ -1488,7 +1553,9 @@ static int qcom_pcie_probe(struct platform_device *pdev)
 
 	pcie->pci = pci;
 
-	pcie->ops = of_device_get_match_data(dev);
+	pcie_cfg = of_device_get_match_data(dev);
+	pcie->ops = pcie_cfg->ops;
+	pcie->pcie_1_pipe_clk_src_switch = pcie_cfg->pcie_1_pipe_clk_src_switch;
 
 	pcie->reset = devm_gpiod_get_optional(dev, "perst", GPIOD_OUT_HIGH);
 	if (IS_ERR(pcie->reset)) {
@@ -1545,16 +1612,17 @@ static int qcom_pcie_probe(struct platform_device *pdev)
 }
 
 static const struct of_device_id qcom_pcie_match[] = {
-	{ .compatible = "qcom,pcie-apq8084", .data = &ops_1_0_0 },
-	{ .compatible = "qcom,pcie-ipq8064", .data = &ops_2_1_0 },
-	{ .compatible = "qcom,pcie-ipq8064-v2", .data = &ops_2_1_0 },
-	{ .compatible = "qcom,pcie-apq8064", .data = &ops_2_1_0 },
-	{ .compatible = "qcom,pcie-msm8996", .data = &ops_2_3_2 },
-	{ .compatible = "qcom,pcie-ipq8074", .data = &ops_2_3_3 },
-	{ .compatible = "qcom,pcie-ipq4019", .data = &ops_2_4_0 },
-	{ .compatible = "qcom,pcie-qcs404", .data = &ops_2_4_0 },
-	{ .compatible = "qcom,pcie-sdm845", .data = &ops_2_7_0 },
-	{ .compatible = "qcom,pcie-sm8250", .data = &ops_1_9_0 },
+	{ .compatible = "qcom,pcie-apq8084", .data = &apq8084_cfg },
+	{ .compatible = "qcom,pcie-ipq8064", .data = &ipq8064_cfg },
+	{ .compatible = "qcom,pcie-ipq8064-v2", .data = &ipq8064_cfg },
+	{ .compatible = "qcom,pcie-apq8064", .data = &ipq8064_cfg },
+	{ .compatible = "qcom,pcie-msm8996", .data = &msm8996_cfg },
+	{ .compatible = "qcom,pcie-ipq8074", .data = &ipq8074_cfg },
+	{ .compatible = "qcom,pcie-ipq4019", .data = &ipq4019_cfg },
+	{ .compatible = "qcom,pcie-qcs404", .data = &ipq4019_cfg },
+	{ .compatible = "qcom,pcie-sdm845", .data = &sdm845_cfg },
+	{ .compatible = "qcom,pcie-sm8250", .data = &sm8250_cfg },
+	{ .compatible = "qcom,pcie-sc7280", .data = &sc7280_cfg },
 	{ }
 };
 
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v7 4/4] PCI: qcom: Switch pcie_1_pipe_clk_src after PHY init in SC7280

[Bjorn Helgaas]

On Tue, Sep 14, 2021 at 11:49:10PM +0530, Prasad Malisetty wrote:
> On the SC7280, the clock source for gcc_pcie_1_pipe_clk_src
> must be the TCXO while gdsc is enabled. After PHY init successful
> clock source should switch to pipe clock for gcc_pcie_1_pipe_clk_src.
> 
> Signed-off-by: Prasad Malisetty <pmaliset@codeaurora.org>
> ---
>  drivers/pci/controller/dwc/pcie-qcom.c | 90 +++++++++++++++++++++++++++++-----
>  1 file changed, 79 insertions(+), 11 deletions(-)
> 
> +struct qcom_pcie_cfg {
> +	const struct qcom_pcie_ops *ops;
> +	bool pcie_1_pipe_clk_src_switch;

This is OK, but all things being equal I like "unsigned int x:1" a
little better.  Here's some background:

  https://lore.kernel.org/r/CA+55aFzKQ6Pj18TB8p4Yr0M4t+S+BsiHH=BJNmn=76-NcjTj-g@mail.gmail.com/
  https://lore.kernel.org/r/CA+55aFxnePDimkVKVtv3gNmRGcwc8KQ5mHYvUxY8sAQg6yvVYg@mail.gmail.com/

> @@ -1467,6 +1531,7 @@ static int qcom_pcie_probe(struct platform_device *pdev)
>  	struct pcie_port *pp;
>  	struct dw_pcie *pci;
>  	struct qcom_pcie *pcie;
> +	const struct qcom_pcie_cfg *pcie_cfg = NULL;

No need to initialize this, since you always assign it before using
it.

>  	int ret;
>  
>  	pcie = devm_kzalloc(dev, sizeof(*pcie), GFP_KERNEL);
> @@ -1488,7 +1553,9 @@ static int qcom_pcie_probe(struct platform_device *pdev)
>  
>  	pcie->pci = pci;
>  
> -	pcie->ops = of_device_get_match_data(dev);
> +	pcie_cfg = of_device_get_match_data(dev);
> +	pcie->ops = pcie_cfg->ops;
> +	pcie->pcie_1_pipe_clk_src_switch = pcie_cfg->pcie_1_pipe_clk_src_switch;
>  
>  	pcie->reset = devm_gpiod_get_optional(dev, "perst", GPIOD_OUT_HIGH);
>  	if (IS_ERR(pcie->reset)) {

Looks good, thanks for working on this!

Re: [PATCH v7 2/4] arm64: dts: qcom: sc7280: Add PCIe and PHY related nodes

[Stephen Boyd]

Quoting Prasad Malisetty (2021-09-14 11:19:08)
> Add PCIe controller and PHY nodes for sc7280 SOC.
>
> Signed-off-by: Prasad Malisetty <pmaliset@codeaurora.org>
> ---

Reviewed-by: Stephen Boyd <swboyd@chromium.org>

Re: [PATCH v7 3/4] arm64: dts: qcom: sc7280: Add PCIe nodes for IDP board

[Stephen Boyd]

Quoting Prasad Malisetty (2021-09-14 11:19:09)
> Enable PCIe controller and PHY for sc7280 IDP board.
> Add specific NVMe GPIO entries for SKU1 and SKU2 support.
>
> Signed-off-by: Prasad Malisetty <pmaliset@codeaurora.org>
> ---
>  arch/arm64/boot/dts/qcom/sc7280-idp.dts  |  9 +++++++++
>  arch/arm64/boot/dts/qcom/sc7280-idp.dtsi | 32 ++++++++++++++++++++++++++++++++
>  arch/arm64/boot/dts/qcom/sc7280-idp2.dts |  9 +++++++++
>  3 files changed, 50 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp.dts b/arch/arm64/boot/dts/qcom/sc7280-idp.dts
> index 64fc22a..2cc6b0a 100644
> --- a/arch/arm64/boot/dts/qcom/sc7280-idp.dts
> +++ b/arch/arm64/boot/dts/qcom/sc7280-idp.dts
> @@ -61,6 +61,15 @@
>         modem-init;
>  };
>
> +&pcie1_default_state {
> +       nvme-n {
> +               pins = "gpio19";
> +               function = "gpio";
> +
> +               bias-pull-up;
> +       };

I don't think the style is to have a single container node anymore.
Instead, each pin gets a different node and then pinctrl-0 has a list of
phandles to the different nodes. qcom maintainers may have more input
here.

Also, this should really go into a different section than here. I
thought the style was to have a 'board specific' pinctrl section.

> +};
> +
>  &pmk8350_vadc {
>         pmr735a_die_temp {
>                 reg = <PMR735A_ADC7_DIE_TEMP>;

Re: [PATCH v7 3/4] arm64: dts: qcom: sc7280: Add PCIe nodes for IDP board

[Bjorn Andersson]

On Tue 14 Sep 18:13 PDT 2021, Stephen Boyd wrote:

> Quoting Prasad Malisetty (2021-09-14 11:19:09)
> > Enable PCIe controller and PHY for sc7280 IDP board.
> > Add specific NVMe GPIO entries for SKU1 and SKU2 support.
> >
> > Signed-off-by: Prasad Malisetty <pmaliset@codeaurora.org>
> > ---
> >  arch/arm64/boot/dts/qcom/sc7280-idp.dts  |  9 +++++++++
> >  arch/arm64/boot/dts/qcom/sc7280-idp.dtsi | 32 ++++++++++++++++++++++++++++++++
> >  arch/arm64/boot/dts/qcom/sc7280-idp2.dts |  9 +++++++++
> >  3 files changed, 50 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp.dts b/arch/arm64/boot/dts/qcom/sc7280-idp.dts
> > index 64fc22a..2cc6b0a 100644
> > --- a/arch/arm64/boot/dts/qcom/sc7280-idp.dts
> > +++ b/arch/arm64/boot/dts/qcom/sc7280-idp.dts
> > @@ -61,6 +61,15 @@
> >         modem-init;
> >  };
> >
> > +&pcie1_default_state {
> > +       nvme-n {
> > +               pins = "gpio19";
> > +               function = "gpio";
> > +
> > +               bias-pull-up;
> > +       };
> 
> I don't think the style is to have a single container node anymore.
> Instead, each pin gets a different node and then pinctrl-0 has a list of
> phandles to the different nodes. qcom maintainers may have more input
> here.
> 

Having a container that defines the state and each part thereof is
preferred (see other PCIe users or how we typically define the Bluetooth
UART). But is "nvme_n" really part of the "PCI state" or is this related
to the NVME device instead?

> Also, this should really go into a different section than here. I
> thought the style was to have a 'board specific' pinctrl section.
> 

Yes, pushing pinctrl states to the bottom is preferred and rather than
amending the existing state I think this goes in a state of its own and
the pinctrl-N amended to include this new state as well.

Regards,
Bjorn

> > +};
> > +
> >  &pmk8350_vadc {
> >         pmr735a_die_temp {
> >                 reg = <PMR735A_ADC7_DIE_TEMP>;

Re: [PATCH v7 4/4] PCI: qcom: Switch pcie_1_pipe_clk_src after PHY init in SC7280

[Bjorn Andersson]

On Tue 14 Sep 11:19 PDT 2021, Prasad Malisetty wrote:

> On the SC7280, the clock source for gcc_pcie_1_pipe_clk_src
> must be the TCXO while gdsc is enabled. After PHY init successful
> clock source should switch to pipe clock for gcc_pcie_1_pipe_clk_src.
> 
> Signed-off-by: Prasad Malisetty <pmaliset@codeaurora.org>
> ---
>  drivers/pci/controller/dwc/pcie-qcom.c | 90 +++++++++++++++++++++++++++++-----
>  1 file changed, 79 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> index 8a7a300..380c962 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> @@ -166,6 +166,9 @@ struct qcom_pcie_resources_2_7_0 {
>  	struct regulator_bulk_data supplies[2];
>  	struct reset_control *pci_reset;
>  	struct clk *pipe_clk;
> +	struct clk *gcc_pcie_1_pipe_clk_src;

Afaict you have both 2 PCIe controllers on sc7280, so I think it seems
more reasonable to shorted this to "pipe_clk_src".

> +	struct clk *phy_pipe_clk;
> +	struct clk *ref_clk_src;
>  };
>  
>  union qcom_pcie_resources {
> @@ -189,6 +192,11 @@ struct qcom_pcie_ops {
>  	int (*config_sid)(struct qcom_pcie *pcie);
>  };
>  
> +struct qcom_pcie_cfg {
> +	const struct qcom_pcie_ops *ops;
> +	bool pcie_1_pipe_clk_src_switch;

Perhaps something little bit more generic, like pipe_clk_need_muxing?

> +};
> +
>  struct qcom_pcie {
>  	struct dw_pcie *pci;
>  	void __iomem *parf;			/* DT parf */
> @@ -197,6 +205,7 @@ struct qcom_pcie {
>  	struct phy *phy;
>  	struct gpio_desc *reset;
>  	const struct qcom_pcie_ops *ops;
> +	bool pcie_1_pipe_clk_src_switch;
>  };
>  
>  #define to_qcom_pcie(x)		dev_get_drvdata((x)->dev)
> @@ -1167,6 +1176,20 @@ static int qcom_pcie_get_resources_2_7_0(struct qcom_pcie *pcie)
>  	if (ret < 0)
>  		return ret;
>  
> +	if (pcie->pcie_1_pipe_clk_src_switch) {

This looks much better, now it will easily scale to other platforms that
have inherited this need.

> +		res->gcc_pcie_1_pipe_clk_src = devm_clk_get(dev, "pipe_mux");
> +		if (IS_ERR(res->gcc_pcie_1_pipe_clk_src))
> +			return PTR_ERR(res->gcc_pcie_1_pipe_clk_src);
> +
> +		res->phy_pipe_clk = devm_clk_get(dev, "phy_pipe");
> +		if (IS_ERR(res->phy_pipe_clk))
> +			return PTR_ERR(res->phy_pipe_clk);
> +
> +		res->ref_clk_src = devm_clk_get(dev, "ref");
> +		if (IS_ERR(res->ref_clk_src))
> +			return PTR_ERR(res->ref_clk_src);
> +	}
> +
>  	res->pipe_clk = devm_clk_get(dev, "pipe");
>  	return PTR_ERR_OR_ZERO(res->pipe_clk);
>  }
> @@ -1185,6 +1208,10 @@ static int qcom_pcie_init_2_7_0(struct qcom_pcie *pcie)
>  		return ret;
>  	}
>  
> +	/* Set TCXO as clock source for gcc_pcie_1_pipe_clk_src */
> +	if (pcie->pcie_1_pipe_clk_src_switch)
> +		clk_set_parent(res->gcc_pcie_1_pipe_clk_src, res->ref_clk_src);
> +
>  	ret = clk_bulk_prepare_enable(res->num_clks, res->clks);
>  	if (ret < 0)
>  		goto err_disable_regulators;
> @@ -1256,6 +1283,10 @@ static int qcom_pcie_post_init_2_7_0(struct qcom_pcie *pcie)
>  {
>  	struct qcom_pcie_resources_2_7_0 *res = &pcie->res.v2_7_0;
>  
> +	/* Set pipe clock as clock source for gcc_pcie_1_pipe_clk_src */
> +	if (pcie->pcie_1_pipe_clk_src_switch)
> +		clk_set_parent(res->gcc_pcie_1_pipe_clk_src, res->phy_pipe_clk);
> +
>  	return clk_prepare_enable(res->pipe_clk);
>  }
>  
> @@ -1456,6 +1487,39 @@ static const struct qcom_pcie_ops ops_1_9_0 = {
>  	.config_sid = qcom_pcie_config_sid_sm8250,
>  };
>  
> +static const struct qcom_pcie_cfg apq8084_cfg = {
> +	.ops = &ops_1_0_0,
> +};
> +
> +static const struct qcom_pcie_cfg ipq8064_cfg = {
> +	.ops = &ops_2_1_0,
> +};
> +
> +static const struct qcom_pcie_cfg msm8996_cfg = {
> +	.ops = &ops_2_3_2,
> +};
> +
> +static const struct qcom_pcie_cfg ipq8074_cfg = {
> +	.ops = &ops_2_3_3,
> +};
> +
> +static const struct qcom_pcie_cfg ipq4019_cfg = {
> +	.ops = &ops_2_4_0,
> +};
> +
> +static const struct qcom_pcie_cfg sdm845_cfg = {
> +	.ops = &ops_2_7_0,
> +};
> +
> +static const struct qcom_pcie_cfg sm8250_cfg = {
> +	.ops = &ops_1_9_0,
> +};
> +
> +static const struct qcom_pcie_cfg sc7280_cfg = {
> +	.ops = &ops_1_9_0,
> +	.pcie_1_pipe_clk_src_switch = true,
> +};
> +
>  static const struct dw_pcie_ops dw_pcie_ops = {
>  	.link_up = qcom_pcie_link_up,
>  	.start_link = qcom_pcie_start_link,
> @@ -1467,6 +1531,7 @@ static int qcom_pcie_probe(struct platform_device *pdev)
>  	struct pcie_port *pp;
>  	struct dw_pcie *pci;
>  	struct qcom_pcie *pcie;
> +	const struct qcom_pcie_cfg *pcie_cfg = NULL;

First use of this variable is an assignment, so I don't see a need for
zero initialize it.

Regards,
Bjorn

>  	int ret;
>  
>  	pcie = devm_kzalloc(dev, sizeof(*pcie), GFP_KERNEL);
> @@ -1488,7 +1553,9 @@ static int qcom_pcie_probe(struct platform_device *pdev)
>  
>  	pcie->pci = pci;
>  
> -	pcie->ops = of_device_get_match_data(dev);
> +	pcie_cfg = of_device_get_match_data(dev);
> +	pcie->ops = pcie_cfg->ops;
> +	pcie->pcie_1_pipe_clk_src_switch = pcie_cfg->pcie_1_pipe_clk_src_switch;
>  
>  	pcie->reset = devm_gpiod_get_optional(dev, "perst", GPIOD_OUT_HIGH);
>  	if (IS_ERR(pcie->reset)) {
> @@ -1545,16 +1612,17 @@ static int qcom_pcie_probe(struct platform_device *pdev)
>  }
>  
>  static const struct of_device_id qcom_pcie_match[] = {
> -	{ .compatible = "qcom,pcie-apq8084", .data = &ops_1_0_0 },
> -	{ .compatible = "qcom,pcie-ipq8064", .data = &ops_2_1_0 },
> -	{ .compatible = "qcom,pcie-ipq8064-v2", .data = &ops_2_1_0 },
> -	{ .compatible = "qcom,pcie-apq8064", .data = &ops_2_1_0 },
> -	{ .compatible = "qcom,pcie-msm8996", .data = &ops_2_3_2 },
> -	{ .compatible = "qcom,pcie-ipq8074", .data = &ops_2_3_3 },
> -	{ .compatible = "qcom,pcie-ipq4019", .data = &ops_2_4_0 },
> -	{ .compatible = "qcom,pcie-qcs404", .data = &ops_2_4_0 },
> -	{ .compatible = "qcom,pcie-sdm845", .data = &ops_2_7_0 },
> -	{ .compatible = "qcom,pcie-sm8250", .data = &ops_1_9_0 },
> +	{ .compatible = "qcom,pcie-apq8084", .data = &apq8084_cfg },
> +	{ .compatible = "qcom,pcie-ipq8064", .data = &ipq8064_cfg },
> +	{ .compatible = "qcom,pcie-ipq8064-v2", .data = &ipq8064_cfg },
> +	{ .compatible = "qcom,pcie-apq8064", .data = &ipq8064_cfg },
> +	{ .compatible = "qcom,pcie-msm8996", .data = &msm8996_cfg },
> +	{ .compatible = "qcom,pcie-ipq8074", .data = &ipq8074_cfg },
> +	{ .compatible = "qcom,pcie-ipq4019", .data = &ipq4019_cfg },
> +	{ .compatible = "qcom,pcie-qcs404", .data = &ipq4019_cfg },
> +	{ .compatible = "qcom,pcie-sdm845", .data = &sdm845_cfg },
> +	{ .compatible = "qcom,pcie-sm8250", .data = &sm8250_cfg },
> +	{ .compatible = "qcom,pcie-sc7280", .data = &sc7280_cfg },
>  	{ }
>  };
>  
> -- 
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
> 

Re: [PATCH v7 4/4] PCI: qcom: Switch pcie_1_pipe_clk_src after PHY init in SC7280

[Prasad Malisetty]

On 2021-09-15 00:22, Bjorn Helgaas wrote:
> On Tue, Sep 14, 2021 at 11:49:10PM +0530, Prasad Malisetty wrote:
>> On the SC7280, the clock source for gcc_pcie_1_pipe_clk_src
>> must be the TCXO while gdsc is enabled. After PHY init successful
>> clock source should switch to pipe clock for gcc_pcie_1_pipe_clk_src.
>> 
>> Signed-off-by: Prasad Malisetty <pmaliset@codeaurora.org>
>> ---
>>  drivers/pci/controller/dwc/pcie-qcom.c | 90 
>> +++++++++++++++++++++++++++++-----
>>  1 file changed, 79 insertions(+), 11 deletions(-)
>> 
>> +struct qcom_pcie_cfg {
>> +	const struct qcom_pcie_ops *ops;
>> +	bool pcie_1_pipe_clk_src_switch;
> 
> This is OK, but all things being equal I like "unsigned int x:1" a
> little better.  Here's some background:
> 
> 
Hi Bjorn,

Thanks for sharing the details.

Sure, I will replace bool with unsigned int in next patch version.

Thanks
-Prasad

> https://lore.kernel.org/r/CA+55aFzKQ6Pj18TB8p4Yr0M4t+S+BsiHH=BJNmn=76-NcjTj-g@mail.gmail.com/
> 
> https://lore.kernel.org/r/CA+55aFxnePDimkVKVtv3gNmRGcwc8KQ5mHYvUxY8sAQg6yvVYg@mail.gmail.com/
> 
>> @@ -1467,6 +1531,7 @@ static int qcom_pcie_probe(struct 
>> platform_device *pdev)
>>  	struct pcie_port *pp;
>>  	struct dw_pcie *pci;
>>  	struct qcom_pcie *pcie;
>> +	const struct qcom_pcie_cfg *pcie_cfg = NULL;
> 
> No need to initialize this, since you always assign it before using
> it.
> 
Agree, I will remove initialization in next patch series.

>>  	int ret;
>> 
>>  	pcie = devm_kzalloc(dev, sizeof(*pcie), GFP_KERNEL);
>> @@ -1488,7 +1553,9 @@ static int qcom_pcie_probe(struct 
>> platform_device *pdev)
>> 
>>  	pcie->pci = pci;
>> 
>> -	pcie->ops = of_device_get_match_data(dev);
>> +	pcie_cfg = of_device_get_match_data(dev);
>> +	pcie->ops = pcie_cfg->ops;
>> +	pcie->pcie_1_pipe_clk_src_switch = 
>> pcie_cfg->pcie_1_pipe_clk_src_switch;
>> 
>>  	pcie->reset = devm_gpiod_get_optional(dev, "perst", GPIOD_OUT_HIGH);
>>  	if (IS_ERR(pcie->reset)) {
> 
> Looks good, thanks for working on this!

Thanks Bjorn for the review.

Re: [PATCH v7 4/4] PCI: qcom: Switch pcie_1_pipe_clk_src after PHY init in SC7280

[Prasad Malisetty]

On 2021-09-15 08:14, Bjorn Andersson wrote:
> On Tue 14 Sep 11:19 PDT 2021, Prasad Malisetty wrote:
> 
>> On the SC7280, the clock source for gcc_pcie_1_pipe_clk_src
>> must be the TCXO while gdsc is enabled. After PHY init successful
>> clock source should switch to pipe clock for gcc_pcie_1_pipe_clk_src.
>> 
>> Signed-off-by: Prasad Malisetty <pmaliset@codeaurora.org>
>> ---
>>  drivers/pci/controller/dwc/pcie-qcom.c | 90 
>> +++++++++++++++++++++++++++++-----
>>  1 file changed, 79 insertions(+), 11 deletions(-)
>> 
>> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c 
>> b/drivers/pci/controller/dwc/pcie-qcom.c
>> index 8a7a300..380c962 100644
>> --- a/drivers/pci/controller/dwc/pcie-qcom.c
>> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
>> @@ -166,6 +166,9 @@ struct qcom_pcie_resources_2_7_0 {
>>  	struct regulator_bulk_data supplies[2];
>>  	struct reset_control *pci_reset;
>>  	struct clk *pipe_clk;
>> +	struct clk *gcc_pcie_1_pipe_clk_src;
> 
> Afaict you have both 2 PCIe controllers on sc7280, so I think it seems
> more reasonable to shorted this to "pipe_clk_src".
> 
Agree. I will make it generic and update in next patch version.

>> +	struct clk *phy_pipe_clk;
>> +	struct clk *ref_clk_src;
>>  };
>> 
>>  union qcom_pcie_resources {
>> @@ -189,6 +192,11 @@ struct qcom_pcie_ops {
>>  	int (*config_sid)(struct qcom_pcie *pcie);
>>  };
>> 
>> +struct qcom_pcie_cfg {
>> +	const struct qcom_pcie_ops *ops;
>> +	bool pcie_1_pipe_clk_src_switch;
> 
> Perhaps something little bit more generic, like pipe_clk_need_muxing?
> 
Sure, I will change it and update in next patch version.

>> +};
>> +
>>  struct qcom_pcie {
>>  	struct dw_pcie *pci;
>>  	void __iomem *parf;			/* DT parf */
>> @@ -197,6 +205,7 @@ struct qcom_pcie {
>>  	struct phy *phy;
>>  	struct gpio_desc *reset;
>>  	const struct qcom_pcie_ops *ops;
>> +	bool pcie_1_pipe_clk_src_switch;
>>  };
>> 
>>  #define to_qcom_pcie(x)		dev_get_drvdata((x)->dev)
>> @@ -1167,6 +1176,20 @@ static int qcom_pcie_get_resources_2_7_0(struct 
>> qcom_pcie *pcie)
>>  	if (ret < 0)
>>  		return ret;
>> 
>> +	if (pcie->pcie_1_pipe_clk_src_switch) {
> 
> This looks much better, now it will easily scale to other platforms 
> that
> have inherited this need.

Thanks Bjorn.

> 
>> +		res->gcc_pcie_1_pipe_clk_src = devm_clk_get(dev, "pipe_mux");
>> +		if (IS_ERR(res->gcc_pcie_1_pipe_clk_src))
>> +			return PTR_ERR(res->gcc_pcie_1_pipe_clk_src);
>> +
>> +		res->phy_pipe_clk = devm_clk_get(dev, "phy_pipe");
>> +		if (IS_ERR(res->phy_pipe_clk))
>> +			return PTR_ERR(res->phy_pipe_clk);
>> +
>> +		res->ref_clk_src = devm_clk_get(dev, "ref");
>> +		if (IS_ERR(res->ref_clk_src))
>> +			return PTR_ERR(res->ref_clk_src);
>> +	}
>> +
>>  	res->pipe_clk = devm_clk_get(dev, "pipe");
>>  	return PTR_ERR_OR_ZERO(res->pipe_clk);
>>  }
>> @@ -1185,6 +1208,10 @@ static int qcom_pcie_init_2_7_0(struct 
>> qcom_pcie *pcie)
>>  		return ret;
>>  	}
>> 
>> +	/* Set TCXO as clock source for gcc_pcie_1_pipe_clk_src */
>> +	if (pcie->pcie_1_pipe_clk_src_switch)
>> +		clk_set_parent(res->gcc_pcie_1_pipe_clk_src, res->ref_clk_src);
>> +
>>  	ret = clk_bulk_prepare_enable(res->num_clks, res->clks);
>>  	if (ret < 0)
>>  		goto err_disable_regulators;
>> @@ -1256,6 +1283,10 @@ static int qcom_pcie_post_init_2_7_0(struct 
>> qcom_pcie *pcie)
>>  {
>>  	struct qcom_pcie_resources_2_7_0 *res = &pcie->res.v2_7_0;
>> 
>> +	/* Set pipe clock as clock source for gcc_pcie_1_pipe_clk_src */
>> +	if (pcie->pcie_1_pipe_clk_src_switch)
>> +		clk_set_parent(res->gcc_pcie_1_pipe_clk_src, res->phy_pipe_clk);
>> +
>>  	return clk_prepare_enable(res->pipe_clk);
>>  }
>> 
>> @@ -1456,6 +1487,39 @@ static const struct qcom_pcie_ops ops_1_9_0 = {
>>  	.config_sid = qcom_pcie_config_sid_sm8250,
>>  };
>> 
>> +static const struct qcom_pcie_cfg apq8084_cfg = {
>> +	.ops = &ops_1_0_0,
>> +};
>> +
>> +static const struct qcom_pcie_cfg ipq8064_cfg = {
>> +	.ops = &ops_2_1_0,
>> +};
>> +
>> +static const struct qcom_pcie_cfg msm8996_cfg = {
>> +	.ops = &ops_2_3_2,
>> +};
>> +
>> +static const struct qcom_pcie_cfg ipq8074_cfg = {
>> +	.ops = &ops_2_3_3,
>> +};
>> +
>> +static const struct qcom_pcie_cfg ipq4019_cfg = {
>> +	.ops = &ops_2_4_0,
>> +};
>> +
>> +static const struct qcom_pcie_cfg sdm845_cfg = {
>> +	.ops = &ops_2_7_0,
>> +};
>> +
>> +static const struct qcom_pcie_cfg sm8250_cfg = {
>> +	.ops = &ops_1_9_0,
>> +};
>> +
>> +static const struct qcom_pcie_cfg sc7280_cfg = {
>> +	.ops = &ops_1_9_0,
>> +	.pcie_1_pipe_clk_src_switch = true,
>> +};
>> +
>>  static const struct dw_pcie_ops dw_pcie_ops = {
>>  	.link_up = qcom_pcie_link_up,
>>  	.start_link = qcom_pcie_start_link,
>> @@ -1467,6 +1531,7 @@ static int qcom_pcie_probe(struct 
>> platform_device *pdev)
>>  	struct pcie_port *pp;
>>  	struct dw_pcie *pci;
>>  	struct qcom_pcie *pcie;
>> +	const struct qcom_pcie_cfg *pcie_cfg = NULL;
> 
> First use of this variable is an assignment, so I don't see a need for
> zero initialize it.
> 
> Regards,
> Bjorn
> 
Sure, I will incorporate the changes in next patch version.

>>  	int ret;
>> 
>>  	pcie = devm_kzalloc(dev, sizeof(*pcie), GFP_KERNEL);
>> @@ -1488,7 +1553,9 @@ static int qcom_pcie_probe(struct 
>> platform_device *pdev)
>> 
>>  	pcie->pci = pci;
>> 
>> -	pcie->ops = of_device_get_match_data(dev);
>> +	pcie_cfg = of_device_get_match_data(dev);
>> +	pcie->ops = pcie_cfg->ops;
>> +	pcie->pcie_1_pipe_clk_src_switch = 
>> pcie_cfg->pcie_1_pipe_clk_src_switch;
>> 
>>  	pcie->reset = devm_gpiod_get_optional(dev, "perst", GPIOD_OUT_HIGH);
>>  	if (IS_ERR(pcie->reset)) {
>> @@ -1545,16 +1612,17 @@ static int qcom_pcie_probe(struct 
>> platform_device *pdev)
>>  }
>> 
>>  static const struct of_device_id qcom_pcie_match[] = {
>> -	{ .compatible = "qcom,pcie-apq8084", .data = &ops_1_0_0 },
>> -	{ .compatible = "qcom,pcie-ipq8064", .data = &ops_2_1_0 },
>> -	{ .compatible = "qcom,pcie-ipq8064-v2", .data = &ops_2_1_0 },
>> -	{ .compatible = "qcom,pcie-apq8064", .data = &ops_2_1_0 },
>> -	{ .compatible = "qcom,pcie-msm8996", .data = &ops_2_3_2 },
>> -	{ .compatible = "qcom,pcie-ipq8074", .data = &ops_2_3_3 },
>> -	{ .compatible = "qcom,pcie-ipq4019", .data = &ops_2_4_0 },
>> -	{ .compatible = "qcom,pcie-qcs404", .data = &ops_2_4_0 },
>> -	{ .compatible = "qcom,pcie-sdm845", .data = &ops_2_7_0 },
>> -	{ .compatible = "qcom,pcie-sm8250", .data = &ops_1_9_0 },
>> +	{ .compatible = "qcom,pcie-apq8084", .data = &apq8084_cfg },
>> +	{ .compatible = "qcom,pcie-ipq8064", .data = &ipq8064_cfg },
>> +	{ .compatible = "qcom,pcie-ipq8064-v2", .data = &ipq8064_cfg },
>> +	{ .compatible = "qcom,pcie-apq8064", .data = &ipq8064_cfg },
>> +	{ .compatible = "qcom,pcie-msm8996", .data = &msm8996_cfg },
>> +	{ .compatible = "qcom,pcie-ipq8074", .data = &ipq8074_cfg },
>> +	{ .compatible = "qcom,pcie-ipq4019", .data = &ipq4019_cfg },
>> +	{ .compatible = "qcom,pcie-qcs404", .data = &ipq4019_cfg },
>> +	{ .compatible = "qcom,pcie-sdm845", .data = &sdm845_cfg },
>> +	{ .compatible = "qcom,pcie-sm8250", .data = &sm8250_cfg },
>> +	{ .compatible = "qcom,pcie-sc7280", .data = &sc7280_cfg },
>>  	{ }
>>  };
>> 
>> --
>> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora 
>> Forum,
>> a Linux Foundation Collaborative Project
>>