On 2021-07-12 21:33, Bjorn Andersson wrote:
On Tue 22 Jun 11:00 CDT 2021, Prasad Malisetty wrote:
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.
Sure, it looks fine, I will modify accordingly.
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?
Yeah, I will correct that statement.
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
From SC7280 onwards, POR value for "gcc_pcie_N_pipe_clk_src" is TCX0. we
need TCXO as parent to enable gdsc and then once PHY init successful we
are changing gcc_pcie_N_pipe_clk_src to PHY pipe clk. In system suspend
call back GDSC will be disabled and gcc_pcie_N_pipe_clk_src changed to
TCX0. In the same way resume call back switching back
gcc_pcie_N_pipe_clk_src to PHY pipe clk .
Signed-off-by: Prasad Malisetty <pmaliset@xxxxxxxxxxxxxx>
---
drivers/pci/controller/dwc/pcie-qcom.c | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/drivers/pci/controller/dwc/pcie-qcom.c
b/drivers/pci/controller/dwc/pcie-qcom.c
index 8a7a300..80e9ee4 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 *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")) {
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?
Will check and confirm whether above change will be applicable to future
targets or not.
+ 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?
Yes
+
+ 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)?
Yes
+
+ 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?
Yes
+ }
+
res->pipe_clk = devm_clk_get(dev, "pipe");
return PTR_ERR_OR_ZERO(res->pipe_clk);
}
@@ -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?
TCXO is POR value.
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?
PHY init will be successful but PCIe link will not be initialized.
yes, this change is only applicable to 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?
Changes are in validation stage. will submit them soon in coming
releases.
Just mentioned the purpose of mux settings.
Regards,
Bjorn
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
