Add DT bindings and DT nodes for PCIe and PHY in SC7280

pipe-clk mux needs to switch between pipe_clk and XO as part of LPM squence. This is done by setting pipe_clk mux as parent of pipe_clk after phy init. This is a new requirement for sc7280.
For accessing to DBI registers during L23, need to switch the pipe clock with free-running clock (TCXO) using GCC’s registers

Signed-off-by Prasad Malisetty <[email protected]>

---
 drivers/pci/controller/dwc/pcie-qcom.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

drivers/pci/controller/dwc/pcie-qcom.c +17 -0

index 8a7a300..80e9ee4 100644
@@ -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 *pipe_clk_mux;
+	struct clk *pipe_ext_src;
+	struct clk *ref_clk_src;
 };
 
 union qcom_pcie_resources {

@@ -1167,6 +1170,20 @@ static int qcom_pcie_get_resources_2_7_0(struct
 qcom_pcie *pcie)
 	if (ret < 0)
 		return ret;
 
+	if (of_device_is_compatible(dev->of_node, "qcom,pcie-sc7280")) {
+		res->pipe_clk_mux = devm_clk_get(dev, "pipe_mux");
+		if (IS_ERR(res->pipe_clk_mux))
+			return PTR_ERR(res->pipe_clk_mux);
+
+		res->pipe_ext_src = devm_clk_get(dev, "phy_pipe");
+		if (IS_ERR(res->pipe_ext_src))
+			return PTR_ERR(res->pipe_ext_src);
+
+		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);

}

drivers/pci/controller/dwc/pcie-qcom.c +5 -0

@@ -1255,6 +1272,11 @@ static void qcom_pcie_deinit_2_7_0(struct
 qcom_pcie *pcie)
 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;
+	struct dw_pcie *pci = pcie->pci;
+	struct device *dev = pci->dev;
+
+	if (of_device_is_compatible(dev->of_node, "qcom,pcie-sc7280"))
+		clk_set_parent(res->pipe_clk_mux, res->pipe_ext_src);
 
 	return clk_prepare_enable(res->pipe_clk);

}

pipe-clk mux needs to switch between pipe_clk

and XO as part of LPM squence. This is done by setting pipe_clk mux as parent of pipe_clk after phy init.

This is a new requirement for sc7280.
For accessing to DBI registers during L23,
need to switch the pipe clock with free-running
clock (TCXO) using GCC’s registers

Signed-off-by Prasad Malisetty <[email protected]>

---
 drivers/pci/controller/dwc/pcie-qcom.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

drivers/pci/controller/dwc/pcie-qcom.c +3 -0

index 8a7a300..80e9ee4 100644
@@ -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 *pipe_clk_mux;
+	struct clk *pipe_ext_src;
+	struct clk *ref_clk_src;
 };
 
 union qcom_pcie_resources {

drivers/pci/controller/dwc/pcie-qcom.c +2 -0

+	}
+
 	res->pipe_clk = devm_clk_get(dev, "pipe");
 	return PTR_ERR_OR_ZERO(res->pipe_clk);
 }

drivers/pci/controller/dwc/pcie-qcom.c +0 -0

 	return clk_prepare_enable(res->pipe_clk);
 }

Signature · 2 lines

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

Quoted 1 line · Prasad Malisetty

pipe-clk mux needs to switch between pipe_clk

If you spell "pipe-clk mux" as "gcc_pcie_N_pipe_clk_src" there's no ambiguity in which clock you refer to.

Quoted 2 lines

and XO as part of LPM squence. This is done by setting pipe_clk mux as parent of pipe_clk after phy init.

I thought the two possible cases where:

xo -> gcc_pcie_N_pipe_clk_src -> gcc_pcie_N_pipe_clk PHY::pipe_clk -> gcc_pcie_N_pipe_clk_src -> gcc_pcie_N_pipe_clk

But here you're saying that you're setting the parent of PHY::pipe_clk to gcc_pcie_N_pipe_clk?

Quoted 4 lines

This is a new requirement for sc7280.
For accessing to DBI registers during L23,
need to switch the pipe clock with free-running
clock (TCXO) using GCC’s registers

So in previous platforms we could access DBI registers, in L23, without any clock?

What happens if we use xo as parent for the pipe clock

Quoted 25 lines

Signed-off-by Prasad Malisetty <[email protected]>

---
 drivers/pci/controller/dwc/pcie-qcom.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

drivers/pci/controller/dwc/pcie-qcom.c +3 -0

index 8a7a300..80e9ee4 100644
@@ -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 *pipe_clk_mux;
+	struct clk *pipe_ext_src;
+	struct clk *ref_clk_src;
 };
 
 union qcom_pcie_resources {

@@ -1167,6 +1170,20 @@ static int qcom_pcie_get_resources_2_7_0(struct 
 qcom_pcie *pcie)
 	if (ret < 0)
 		return ret;
 

drivers/pci/controller/dwc/pcie-qcom.c static int qcom_pcie_get_resources_2_7_0(struct qcom_pcie *pcie) · line 1173

Quoted 1 line

drivers/pci/controller/dwc/pcie-qcom.c +1 -0

+	if (of_device_is_compatible(dev->of_node, "qcom,pcie-sc7280")) {

So this is the first 2.7.0 that has this need? Are we just going to add more compatibles to this list going forward?

drivers/pci/controller/dwc/pcie-qcom.c static int qcom_pcie_get_resources_2_7_0(struct qcom_pcie *pcie) · line 1176

Quoted 3 lines

drivers/pci/controller/dwc/pcie-qcom.c +3 -0

+		res->pipe_clk_mux = devm_clk_get(dev, "pipe_mux");
+		if (IS_ERR(res->pipe_clk_mux))
+			return PTR_ERR(res->pipe_clk_mux);

So this is gcc_pcie_N_pipe_clk_src?

drivers/pci/controller/dwc/pcie-qcom.c static int qcom_pcie_get_resources_2_7_0(struct qcom_pcie *pcie) · line 1180

Quoted 4 lines

drivers/pci/controller/dwc/pcie-qcom.c +4 -0

+
+		res->pipe_ext_src = devm_clk_get(dev, "phy_pipe");
+		if (IS_ERR(res->pipe_ext_src))
+			return PTR_ERR(res->pipe_ext_src);

And this is the pipe_clk coming out of the PHY (What I call PHY::pipe_clk above)?

drivers/pci/controller/dwc/pcie-qcom.c static int qcom_pcie_get_resources_2_7_0(struct qcom_pcie *pcie) · line 1184

Quoted 4 lines

drivers/pci/controller/dwc/pcie-qcom.c +4 -0

+
+		res->ref_clk_src = devm_clk_get(dev, "ref");
+		if (IS_ERR(res->ref_clk_src))
+			return PTR_ERR(res->ref_clk_src);

And this is TCXO?

Quoted 6 lines

drivers/pci/controller/dwc/pcie-qcom.c +2 -0

+	}
+
 	res->pipe_clk = devm_clk_get(dev, "pipe");
 	return PTR_ERR_OR_ZERO(res->pipe_clk);

}

drivers/pci/controller/dwc/pcie-qcom.c static void qcom_pcie_deinit_2_7_0(struct · line 1281

Quoted 10 lines

drivers/pci/controller/dwc/pcie-qcom.c +5 -0

@@ -1255,6 +1272,11 @@ static void qcom_pcie_deinit_2_7_0(struct 
 qcom_pcie *pcie)
 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;
+	struct dw_pcie *pci = pcie->pci;
+	struct device *dev = pci->dev;
+
+	if (of_device_is_compatible(dev->of_node, "qcom,pcie-sc7280"))
+		clk_set_parent(res->pipe_clk_mux, res->pipe_ext_src);
 

So after phy_power_on() (not "phy init" as you say in the commit message, perhaps you don't mean phy_init() but in general terms "phy initialization") you need to make sure that gcc_pcie_N_pipe_clk_src is actually fed by PHY::pipe_clk?

1) What's the gcc_pcie_N_pipe_clk_src parent before this?

2) Will the PHY initialization really succeed if the pipe_clk feeding back from gcc isn't based on the PHY's pipe_clk? Is this a change in sc7280?

3) In the commit message you're talking about the need to make XO the parent of gcc_pcie_N_pipe_clk_src during L23, where in this patch does that happen?

Regards,
Bjorn

Quoted 6 lines

drivers/pci/controller/dwc/pcie-qcom.c +0 -0

 	return clk_prepare_enable(res->pipe_clk);

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