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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2021-08-11 01:07, Stephen Boyd wrote:
Quoting Prasad Malisetty (2021-08-09 21:08:36)
On the SC7280, By default the clock source for pcie_1_pipe is
TCXO for gdsc enable. But after the PHY is initialized, the clock
source must be switched to gcc_pcie_1_pipe_clk from TCXO.

Signed-off-by: Prasad Malisetty <pmaliset@xxxxxxxxxxxxxx>
---
 drivers/pci/controller/dwc/pcie-qcom.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index 8a7a300..39e3b21 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -1167,6 +1169,16 @@ 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->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->pipe_clk = devm_clk_get(dev, "pipe");
        return PTR_ERR_OR_ZERO(res->pipe_clk);
 }
@@ -1255,6 +1267,12 @@ 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;
+       struct device_node *node = dev->of_node;
+
+       if (of_property_read_bool(node, "pipe-clk-source-switch"))

This can be straightline code. If gcc_pcie_1_pipe_clk_src is NULL,
calling clk_set_parent() on it is a nop, return 0, so drop the property
check and only assign the clk pointer if it needs to be done.

+ clk_set_parent(res->gcc_pcie_1_pipe_clk_src, res->phy_pipe_clk);

Please check the return value and fail if it fails to set the parent.
I'd also prefer a comment indicating that we have to set the parent
because the GDSC must be enabled with the clk at XO speed. The DT should
probably also have an assigned clock parent of XO so when the driver
probes it is set to XO parent for gdsc enable and then this driver code
can change the parent to the phy pipe clk.


        return clk_prepare_enable(res->pipe_clk);
 }

Hi Stephen,

Thanks for your review and inputs.

Yes, clk_set_parent function returning NULL if src pointer is NULL. we can call clk_set_parent function without any check.

I will validate and incorporate the changes in next version.

Thanks
-Prasad



[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux