| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <f4cfc1b3-2a85-f948-ff2c-27588cbe2210@linaro.org>
Date: Thu, 21 Apr 2022 14:36:05 +0300
From: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
To: Johan Hovold <johan+linaro@...nel.org>,
Andy Gross <agross@...nel.org>,
Bjorn Andersson <bjorn.andersson@...aro.org>,
Lorenzo Pieralisi <lorenzo.pieralisi@....com>,
Kishon Vijay Abraham I <kishon@...com>,
Vinod Koul <vkoul@...nel.org>,
Stephen Boyd <swboyd@...omium.org>
Cc: Rob Herring <robh+dt@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>,
Stanimir Varbanov <svarbanov@...sol.com>,
Krzysztof Wilczyński <kw@...ux.com>,
Bjorn Helgaas <bhelgaas@...gle.com>,
Prasad Malisetty <quic_pmaliset@...cinc.com>,
linux-arm-msm@...r.kernel.org, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-pci@...r.kernel.org,
linux-phy@...ts.infradead.org
Subject: Re: [PATCH RFC 1/5] phy: qcom-qmp: add support for pipe clock muxing
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@...nel.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
Powered by blists - more mailing lists