| Message ID | 1622904059-21244-4-git-send-email-pmaliset@codeaurora.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add DT bindings and DT nodes for PCIe and PHY in SC7280 | expand |
On Sat 05 Jun 09:40 CDT 2021, Prasad Malisetty wrote: > In PCIe driver pipe-clk mux needs to switch between pipe_clk > and XO for GDSC enable. This is done by setting pipe_clk mux > as parent of pipe_clk after phy init. But you're not switching between pipe_clk and XO, you're only making sure that the pipe_clk is parented by the PHY's pipe clock. Also, can you please elaborate on how this relates to the GDSC? > > Signed-off-by: Prasad Malisetty <pmaliset@codeaurora.org> > --- > 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..5cbbea4 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")) { > + res->pipe_clk_mux = devm_clk_get(dev, "pipe_src"); > + if (IS_ERR(res->pipe_clk_mux)) > + return PTR_ERR(res->pipe_clk_mux); > + > + res->pipe_ext_src = devm_clk_get(dev, "pipe_ext"); > + 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); > } > @@ -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")) If this is something only found on 7280, you need to document (in the commit message at least) why this does not apply to other platforms with this controller. Thanks, Bjorn > + clk_set_parent(res->pipe_clk_mux, res->pipe_ext_src); > > 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 >
On 2021-06-06 02:56, Stephen Boyd wrote: > Quoting Prasad Malisetty (2021-06-05 07:40:58) >> In PCIe driver pipe-clk mux needs to switch between pipe_clk >> and XO for GDSC enable. This is done by setting pipe_clk mux >> as parent of pipe_clk after phy init. > > Just to confirm, we can't set this parent via assigned-clock-parents > property in DT? > >> This clock setting need be done after phy init. >> Signed-off-by: Prasad Malisetty <pmaliset@codeaurora.org> >> --- >> 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..5cbbea4 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")) >> { >> + res->pipe_clk_mux = devm_clk_get(dev, "pipe_src"); >> + if (IS_ERR(res->pipe_clk_mux)) >> + return PTR_ERR(res->pipe_clk_mux); >> + >> + res->pipe_ext_src = devm_clk_get(dev, "pipe_ext"); >> + if (IS_ERR(res->pipe_ext_src)) >> + return PTR_ERR(res->pipe_ext_src); >> + >> + res->ref_clk_src = devm_clk_get(dev, "ref"); > > Is this going to be used by any code? > Yes, ref clock will be used in system suspend case. currently system suspend changes are in under validation. >> + 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); >> } >> @@ -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); >> } >> -- >> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora >> Forum, >> a Linux Foundation Collaborative Project >>
On 2021-06-06 07:45, Bjorn Andersson wrote: > On Sat 05 Jun 09:40 CDT 2021, Prasad Malisetty wrote: > >> In PCIe driver pipe-clk mux needs to switch between pipe_clk >> and XO for GDSC enable. This is done by setting pipe_clk mux >> as parent of pipe_clk after phy init. > > But you're not switching between pipe_clk and XO, you're only making > sure that the pipe_clk is parented by the PHY's pipe clock. > > Also, can you please elaborate on how this relates to the GDSC? > >> yes we are parenting the pipe clock by PHY's pipe clock. and also >> switching back to XO during suspend. Below is the new requirement for SC7280 as part of LPM sequence. In L1ss low power mode PHY turns the pipe clock off, so each access on slave AXI causes to exit from low power modes. For completing the access, the pipe clock should be active from PHY. In L23 mode, access on slave AXI doesn’t wake the core. For accessing to DBI registers during L23, the SW should switch the pipe clock with 19.2MHz free-running clock (TCXO) using GCC’s registers >> Signed-off-by: Prasad Malisetty <pmaliset@codeaurora.org> >> --- >> 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..5cbbea4 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")) { >> + res->pipe_clk_mux = devm_clk_get(dev, "pipe_src"); >> + if (IS_ERR(res->pipe_clk_mux)) >> + return PTR_ERR(res->pipe_clk_mux); >> + >> + res->pipe_ext_src = devm_clk_get(dev, "pipe_ext"); >> + 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); >> } >> @@ -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")) > > If this is something only found on 7280, you need to document (in the > commit message at least) why this does not apply to other platforms > with > this controller. > > Thanks, > Bjorn > Sure, will add more info about the requirement. >> + clk_set_parent(res->pipe_clk_mux, res->pipe_ext_src); >> >> 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 >>
diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c index 8a7a300..5cbbea4 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")) { + res->pipe_clk_mux = devm_clk_get(dev, "pipe_src"); + if (IS_ERR(res->pipe_clk_mux)) + return PTR_ERR(res->pipe_clk_mux); + + res->pipe_ext_src = devm_clk_get(dev, "pipe_ext"); + 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); } @@ -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); }
In PCIe driver pipe-clk mux needs to switch between pipe_clk and XO for GDSC enable. This is done by setting pipe_clk mux as parent of pipe_clk after phy init. Signed-off-by: Prasad Malisetty <pmaliset@codeaurora.org> --- drivers/pci/controller/dwc/pcie-qcom.c | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+)