lkml.org 
[lkml]   [2022]   [Apr]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH RFC 1/5] phy: qcom-qmp: add support for pipe clock muxing
From
On 21/04/2022 13:20, Johan Hovold wrote:
> Some QMP PHYs need to remux to their pipe clock input to the pipe clock
> output generated by the PHY before powering on the PHY and restore the
> default source during power down.
>
> Add support for an optional pipe clock mux which will be reparented to
> the generated pipe clock before powering on the PHY and restored to the
> default reference source on power off.
>
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> ---
> drivers/phy/qualcomm/phy-qcom-qmp.c | 71 ++++++++++++++++++++++++++---
> 1 file changed, 65 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp.c b/drivers/phy/qualcomm/phy-qcom-qmp.c
> index 7d2d1ab061f7..bc6db9670291 100644
> --- a/drivers/phy/qualcomm/phy-qcom-qmp.c
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp.c
> @@ -3292,6 +3292,8 @@ struct qmp_phy_combo_cfg {
> * @rx2: iomapped memory space for second lane's rx (in dual lane PHYs)
> * @pcs_misc: iomapped memory space for lane's pcs_misc
> * @pipe_clk: pipe clock
> + * @pipemux_clk: pipe clock source mux
> + * @piperef_clk: pipe clock default reference source
> * @index: lane index
> * @qmp: QMP phy to which this lane belongs
> * @lane_rst: lane's reset controller
> @@ -3311,6 +3313,8 @@ struct qmp_phy {
> void __iomem *rx2;
> void __iomem *pcs_misc;
> struct clk *pipe_clk;
> + struct clk *pipemux_clk;
> + struct clk *piperef_clk;
> unsigned int index;
> struct qcom_qmp *qmp;
> struct reset_control *lane_rst;
> @@ -3346,6 +3350,7 @@ struct qcom_qmp {
> void __iomem *dp_com;
>
> struct clk_bulk_data *clks;
> + struct clk *pipe_clksrc;
> struct reset_control **resets;
> struct regulator_bulk_data *vregs;
>
> @@ -5355,6 +5360,42 @@ static int qcom_qmp_phy_init(struct phy *phy)
> return 0;
> }
>
> +static int qcom_qmp_phy_pipe_clk_enable(struct qmp_phy *qphy)
> +{
> + struct qcom_qmp *qmp = qphy->qmp;
> + int ret;
> +
> + ret = clk_set_parent(qphy->pipemux_clk, qmp->pipe_clksrc);
> + if (ret)
> + dev_err(qmp->dev, "failed to reparent pipe clock: %d\n", ret);
> +
> +
> + ret = clk_prepare_enable(qphy->pipe_clk);
> + if (ret) {
> + dev_err(qmp->dev, "failed to enable pipe clock: %d\n", ret);
> + goto err_restore_parent;
> + }

So, what you do here is you manually set the parent of
GCC_PCIE_1_PIPE_CLK_SRC to PHY pipe clock right before enabling
GCC_PCIE_1_PIPE_CLK and set it back to XO after disabling
GCC_PCIE_1_PIPE_CLK.

My proposal is doing exactly the same, but doing that automatically
through the clock infrastructure. After removing pipe_clock handling
from pcie driver itself, we can be sure that nobody is playing dirty
tricks around the pipe_clock.

> +
> + return 0;
> +
> +err_restore_parent:
> + clk_set_parent(qphy->pipemux_clk, qphy->piperef_clk);
> +
> + return ret;
> +}
> +
> +static void qcom_qmp_phy_pipe_clk_disable(struct qmp_phy *qphy)
> +{
> + struct qcom_qmp *qmp = qphy->qmp;
> + int ret;
> +
> + clk_disable_unprepare(qphy->pipe_clk);
> +
> + ret = clk_set_parent(qphy->pipemux_clk, qphy->piperef_clk);
> + if (ret)
> + dev_err(qmp->dev, "failed to reparent pipe clock: %d\n", ret);
> +}
> +
> static int qcom_qmp_phy_power_on(struct phy *phy)
> {
> struct qmp_phy *qphy = phy_get_drvdata(phy);
> @@ -5379,11 +5420,9 @@ static int qcom_qmp_phy_power_on(struct phy *phy)
> }
> }
>
> - ret = clk_prepare_enable(qphy->pipe_clk);
> - if (ret) {
> - dev_err(qmp->dev, "pipe_clk enable failed err=%d\n", ret);
> + ret = qcom_qmp_phy_pipe_clk_enable(qphy);
> + if (ret)
> goto err_reset_lane;
> - }
>
> /* Tx, Rx, and PCS configurations */
> qcom_qmp_phy_configure_lane(tx, cfg->regs,
> @@ -5478,7 +5517,7 @@ static int qcom_qmp_phy_power_on(struct phy *phy)
> return 0;
>
> err_disable_pipe_clk:
> - clk_disable_unprepare(qphy->pipe_clk);
> + qcom_qmp_phy_pipe_clk_disable(qphy);
> err_reset_lane:
> if (cfg->has_lane_rst)
> reset_control_assert(qphy->lane_rst);
> @@ -5491,7 +5530,7 @@ static int qcom_qmp_phy_power_off(struct phy *phy)
> struct qmp_phy *qphy = phy_get_drvdata(phy);
> const struct qmp_phy_cfg *cfg = qphy->cfg;
>
> - clk_disable_unprepare(qphy->pipe_clk);
> + qcom_qmp_phy_pipe_clk_disable(qphy);
>
> if (cfg->type == PHY_TYPE_DP) {
> /* Assert DP PHY power down */
> @@ -5777,6 +5816,8 @@ static int phy_pipe_clk_register(struct qcom_qmp *qmp, struct device_node *np)
> if (ret)
> return ret;
>
> + qmp->pipe_clksrc = fixed->hw.clk;
> +
> ret = of_clk_add_hw_provider(np, of_clk_hw_simple_get, &fixed->hw);
> if (ret)
> return ret;
> @@ -6091,6 +6132,24 @@ int qcom_qmp_phy_create(struct device *dev, struct device_node *np, int id,
> qphy->pipe_clk = NULL;
> }
>
> + /* Get optional pipe clock mux and default reference source clock. */
> + qphy->pipemux_clk = of_clk_get_by_name(np, "mux");
> + if (IS_ERR(qphy->pipemux_clk)) {
> + ret = PTR_ERR(qphy->pipemux_clk);
> + if (ret == -EPROBE_DEFER)
> + return ret;
> +
> + qphy->pipemux_clk = NULL;
> + } else {
> + qphy->piperef_clk = of_clk_get_by_name(np, "ref");
> + if (IS_ERR(qphy->piperef_clk)) {
> + ret = PTR_ERR(qphy->piperef_clk);
> + return dev_err_probe(dev, ret,
> + "failed to get lane%d piperef_clk\n",
> + id);
> + }
> + }
> +
> /* Get lane reset, if any */
> if (cfg->has_lane_rst) {
> snprintf(prop_name, sizeof(prop_name), "lane%d", id);


--
With best wishes
Dmitry

\
 
 \ /
  Last update: 2022-04-21 13:36    [W:0.187 / U:0.004 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site