* [PATCH 0/2] phy: qcom-qmp: pipe clock fixes @ 2022-04-20 15:23 Johan Hovold 2022-04-20 15:23 ` [PATCH 1/2] phy: qcom-qmp: fix phy-descriptor kernel-doc typo Johan Hovold 2022-04-20 15:23 ` [PATCH 2/2] phy: qcom-qmp: fix pipe-clock imbalance on power-on failure Johan Hovold 0 siblings, 2 replies; 7+ messages in thread From: Johan Hovold @ 2022-04-20 15:23 UTC (permalink / raw) To: Kishon Vijay Abraham I, Vinod Koul Cc: Andy Gross, Bjorn Andersson, linux-arm-msm, linux-phy, linux-kernel, Johan Hovold This series fixes a kernel doc typo and pipe clock imbalance on PHY power on failures. Johan Johan Hovold (2): phy: qcom-qmp: fix phy-descriptor kernel-doc typo phy: qcom-qmp: fix pipe-clock imbalance on power-on failure drivers/phy/qualcomm/phy-qcom-qmp.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) -- 2.35.1 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/2] phy: qcom-qmp: fix phy-descriptor kernel-doc typo 2022-04-20 15:23 [PATCH 0/2] phy: qcom-qmp: pipe clock fixes Johan Hovold @ 2022-04-20 15:23 ` Johan Hovold 2022-05-02 10:10 ` Vinod Koul 2022-04-20 15:23 ` [PATCH 2/2] phy: qcom-qmp: fix pipe-clock imbalance on power-on failure Johan Hovold 1 sibling, 1 reply; 7+ messages in thread From: Johan Hovold @ 2022-04-20 15:23 UTC (permalink / raw) To: Kishon Vijay Abraham I, Vinod Koul Cc: Andy Gross, Bjorn Andersson, linux-arm-msm, linux-phy, linux-kernel, Johan Hovold Fix misspelled "clock" in the description of the pipe_clk field in the PHY-descriptor kernel-doc comment. Signed-off-by: Johan Hovold <johan+linaro@kernel.org> --- drivers/phy/qualcomm/phy-qcom-qmp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/phy/qualcomm/phy-qcom-qmp.c b/drivers/phy/qualcomm/phy-qcom-qmp.c index 972f27c0e0a2..8c2300bfe489 100644 --- a/drivers/phy/qualcomm/phy-qcom-qmp.c +++ b/drivers/phy/qualcomm/phy-qcom-qmp.c @@ -3291,7 +3291,7 @@ struct qmp_phy_combo_cfg { * @tx2: iomapped memory space for second lane's tx (in dual lane PHYs) * @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 lock + * @pipe_clk: pipe clock * @index: lane index * @qmp: QMP phy to which this lane belongs * @lane_rst: lane's reset controller -- 2.35.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] phy: qcom-qmp: fix phy-descriptor kernel-doc typo 2022-04-20 15:23 ` [PATCH 1/2] phy: qcom-qmp: fix phy-descriptor kernel-doc typo Johan Hovold @ 2022-05-02 10:10 ` Vinod Koul 0 siblings, 0 replies; 7+ messages in thread From: Vinod Koul @ 2022-05-02 10:10 UTC (permalink / raw) To: Johan Hovold Cc: Kishon Vijay Abraham I, Andy Gross, Bjorn Andersson, linux-arm-msm, linux-phy, linux-kernel On 20-04-22, 17:23, Johan Hovold wrote: > Fix misspelled "clock" in the description of the pipe_clk field in the > PHY-descriptor kernel-doc comment. Applied, thanks -- ~Vinod ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 2/2] phy: qcom-qmp: fix pipe-clock imbalance on power-on failure 2022-04-20 15:23 [PATCH 0/2] phy: qcom-qmp: pipe clock fixes Johan Hovold 2022-04-20 15:23 ` [PATCH 1/2] phy: qcom-qmp: fix phy-descriptor kernel-doc typo Johan Hovold @ 2022-04-20 15:23 ` Johan Hovold 2022-05-02 10:09 ` Vinod Koul 1 sibling, 1 reply; 7+ messages in thread From: Johan Hovold @ 2022-04-20 15:23 UTC (permalink / raw) To: Kishon Vijay Abraham I, Vinod Koul Cc: Andy Gross, Bjorn Andersson, linux-arm-msm, linux-phy, linux-kernel, Johan Hovold, Evan Green Make sure to disable the pipe clock also if ufs-reset deassertion fails during power on. Note that the ufs-reset is asserted in qcom_qmp_phy_com_exit(). Fixes: c9b589791fc1 ("phy: qcom: Utilize UFS reset controller") Cc: Evan Green <evgreen@chromium.org> Signed-off-by: Johan Hovold <johan+linaro@kernel.org> --- drivers/phy/qualcomm/phy-qcom-qmp.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/drivers/phy/qualcomm/phy-qcom-qmp.c b/drivers/phy/qualcomm/phy-qcom-qmp.c index 8c2300bfe489..7d2d1ab061f7 100644 --- a/drivers/phy/qualcomm/phy-qcom-qmp.c +++ b/drivers/phy/qualcomm/phy-qcom-qmp.c @@ -5375,14 +5375,14 @@ static int qcom_qmp_phy_power_on(struct phy *phy) if (ret) { dev_err(qmp->dev, "lane%d reset deassert failed\n", qphy->index); - goto err_lane_rst; + return ret; } } ret = clk_prepare_enable(qphy->pipe_clk); if (ret) { dev_err(qmp->dev, "pipe_clk enable failed err=%d\n", ret); - goto err_clk_enable; + goto err_reset_lane; } /* Tx, Rx, and PCS configurations */ @@ -5433,7 +5433,7 @@ static int qcom_qmp_phy_power_on(struct phy *phy) ret = reset_control_deassert(qmp->ufs_reset); if (ret) - goto err_lane_rst; + goto err_disable_pipe_clk; qcom_qmp_phy_configure(pcs_misc, cfg->regs, cfg->pcs_misc_tbl, cfg->pcs_misc_tbl_num); @@ -5472,17 +5472,17 @@ static int qcom_qmp_phy_power_on(struct phy *phy) PHY_INIT_COMPLETE_TIMEOUT); if (ret) { dev_err(qmp->dev, "phy initialization timed-out\n"); - goto err_pcs_ready; + goto err_disable_pipe_clk; } } return 0; -err_pcs_ready: +err_disable_pipe_clk: clk_disable_unprepare(qphy->pipe_clk); -err_clk_enable: +err_reset_lane: if (cfg->has_lane_rst) reset_control_assert(qphy->lane_rst); -err_lane_rst: + return ret; } -- 2.35.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] phy: qcom-qmp: fix pipe-clock imbalance on power-on failure 2022-04-20 15:23 ` [PATCH 2/2] phy: qcom-qmp: fix pipe-clock imbalance on power-on failure Johan Hovold @ 2022-05-02 10:09 ` Vinod Koul 2022-05-02 10:27 ` Johan Hovold 0 siblings, 1 reply; 7+ messages in thread From: Vinod Koul @ 2022-05-02 10:09 UTC (permalink / raw) To: Johan Hovold Cc: Kishon Vijay Abraham I, Andy Gross, Bjorn Andersson, linux-arm-msm, linux-phy, linux-kernel, Evan Green On 20-04-22, 17:23, Johan Hovold wrote: > Make sure to disable the pipe clock also if ufs-reset deassertion fails > during power on. > > Note that the ufs-reset is asserted in qcom_qmp_phy_com_exit(). > > Fixes: c9b589791fc1 ("phy: qcom: Utilize UFS reset controller") > Cc: Evan Green <evgreen@chromium.org> > Signed-off-by: Johan Hovold <johan+linaro@kernel.org> > --- > drivers/phy/qualcomm/phy-qcom-qmp.c | 14 +++++++------- > 1 file changed, 7 insertions(+), 7 deletions(-) > > diff --git a/drivers/phy/qualcomm/phy-qcom-qmp.c b/drivers/phy/qualcomm/phy-qcom-qmp.c > index 8c2300bfe489..7d2d1ab061f7 100644 > --- a/drivers/phy/qualcomm/phy-qcom-qmp.c > +++ b/drivers/phy/qualcomm/phy-qcom-qmp.c > @@ -5375,14 +5375,14 @@ static int qcom_qmp_phy_power_on(struct phy *phy) > if (ret) { > dev_err(qmp->dev, "lane%d reset deassert failed\n", > qphy->index); > - goto err_lane_rst; > + return ret; This can be skipped if we retain the err_lane_rst label > } > } > > ret = clk_prepare_enable(qphy->pipe_clk); > if (ret) { > dev_err(qmp->dev, "pipe_clk enable failed err=%d\n", ret); > - goto err_clk_enable; > + goto err_reset_lane; > } > > /* Tx, Rx, and PCS configurations */ > @@ -5433,7 +5433,7 @@ static int qcom_qmp_phy_power_on(struct phy *phy) > > ret = reset_control_deassert(qmp->ufs_reset); > if (ret) > - goto err_lane_rst; > + goto err_disable_pipe_clk; this is the actual fix... > > qcom_qmp_phy_configure(pcs_misc, cfg->regs, cfg->pcs_misc_tbl, > cfg->pcs_misc_tbl_num); > @@ -5472,17 +5472,17 @@ static int qcom_qmp_phy_power_on(struct phy *phy) > PHY_INIT_COMPLETE_TIMEOUT); > if (ret) { > dev_err(qmp->dev, "phy initialization timed-out\n"); > - goto err_pcs_ready; > + goto err_disable_pipe_clk; same rename here > } > } > return 0; > > -err_pcs_ready: > +err_disable_pipe_clk: > clk_disable_unprepare(qphy->pipe_clk); > -err_clk_enable: > +err_reset_lane: > if (cfg->has_lane_rst) > reset_control_assert(qphy->lane_rst); > -err_lane_rst: > + > return ret; while I feel the names given by this patch are better, they should not be in a fix patch. We should just add the one line fix here and do renames later > } > > -- > 2.35.1 -- ~Vinod ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] phy: qcom-qmp: fix pipe-clock imbalance on power-on failure 2022-05-02 10:09 ` Vinod Koul @ 2022-05-02 10:27 ` Johan Hovold 2022-05-02 12:01 ` Vinod Koul 0 siblings, 1 reply; 7+ messages in thread From: Johan Hovold @ 2022-05-02 10:27 UTC (permalink / raw) To: Vinod Koul Cc: Johan Hovold, Kishon Vijay Abraham I, Andy Gross, Bjorn Andersson, linux-arm-msm, linux-phy, linux-kernel, Evan Green On Mon, May 02, 2022 at 03:39:41PM +0530, Vinod Koul wrote: > On 20-04-22, 17:23, Johan Hovold wrote: > > Make sure to disable the pipe clock also if ufs-reset deassertion fails > > during power on. > > > > Note that the ufs-reset is asserted in qcom_qmp_phy_com_exit(). > > > > Fixes: c9b589791fc1 ("phy: qcom: Utilize UFS reset controller") > > Cc: Evan Green <evgreen@chromium.org> > > Signed-off-by: Johan Hovold <johan+linaro@kernel.org> > > --- > > drivers/phy/qualcomm/phy-qcom-qmp.c | 14 +++++++------- > > 1 file changed, 7 insertions(+), 7 deletions(-) > > > > diff --git a/drivers/phy/qualcomm/phy-qcom-qmp.c b/drivers/phy/qualcomm/phy-qcom-qmp.c > > index 8c2300bfe489..7d2d1ab061f7 100644 > > --- a/drivers/phy/qualcomm/phy-qcom-qmp.c > > +++ b/drivers/phy/qualcomm/phy-qcom-qmp.c > > @@ -5375,14 +5375,14 @@ static int qcom_qmp_phy_power_on(struct phy *phy) > > if (ret) { > > dev_err(qmp->dev, "lane%d reset deassert failed\n", > > qphy->index); > > - goto err_lane_rst; > > + return ret; > > This can be skipped if we retain the err_lane_rst label > > > } > > } > > > > ret = clk_prepare_enable(qphy->pipe_clk); > > if (ret) { > > dev_err(qmp->dev, "pipe_clk enable failed err=%d\n", ret); > > - goto err_clk_enable; > > + goto err_reset_lane; > > } > > > > /* Tx, Rx, and PCS configurations */ > > @@ -5433,7 +5433,7 @@ static int qcom_qmp_phy_power_on(struct phy *phy) > > > > ret = reset_control_deassert(qmp->ufs_reset); > > if (ret) > > - goto err_lane_rst; > > + goto err_disable_pipe_clk; > > this is the actual fix... Right, but with a one-line fix this would read goto err_pcs_ready; which makes no sense at all. Is that really what you prefer? Renaming just this one and the below one, would leave the error labels named using two different schemes, which also isn't very nice, but perhaps ok. > > > > qcom_qmp_phy_configure(pcs_misc, cfg->regs, cfg->pcs_misc_tbl, > > cfg->pcs_misc_tbl_num); > > @@ -5472,17 +5472,17 @@ static int qcom_qmp_phy_power_on(struct phy *phy) > > PHY_INIT_COMPLETE_TIMEOUT); > > if (ret) { > > dev_err(qmp->dev, "phy initialization timed-out\n"); > > - goto err_pcs_ready; > > + goto err_disable_pipe_clk; > > same rename here > > > } > > } > > return 0; > > > > -err_pcs_ready: > > +err_disable_pipe_clk: > > clk_disable_unprepare(qphy->pipe_clk); > > -err_clk_enable: > > +err_reset_lane: > > if (cfg->has_lane_rst) > > reset_control_assert(qphy->lane_rst); > > -err_lane_rst: > > + > > return ret; > > while I feel the names given by this patch are better, they should not > be in a fix patch. We should just add the one line fix here and do > renames later I'll respin if you prefer, just want to double check that you really want a one line fix (i.e. goto err_pcs_ready). Johan ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] phy: qcom-qmp: fix pipe-clock imbalance on power-on failure 2022-05-02 10:27 ` Johan Hovold @ 2022-05-02 12:01 ` Vinod Koul 0 siblings, 0 replies; 7+ messages in thread From: Vinod Koul @ 2022-05-02 12:01 UTC (permalink / raw) To: Johan Hovold Cc: Johan Hovold, Kishon Vijay Abraham I, Andy Gross, Bjorn Andersson, linux-arm-msm, linux-phy, linux-kernel, Evan Green On 02-05-22, 12:27, Johan Hovold wrote: > On Mon, May 02, 2022 at 03:39:41PM +0530, Vinod Koul wrote: > > On 20-04-22, 17:23, Johan Hovold wrote: > > > Make sure to disable the pipe clock also if ufs-reset deassertion fails > > > during power on. > > > > > > Note that the ufs-reset is asserted in qcom_qmp_phy_com_exit(). > > > > > > Fixes: c9b589791fc1 ("phy: qcom: Utilize UFS reset controller") > > > Cc: Evan Green <evgreen@chromium.org> > > > Signed-off-by: Johan Hovold <johan+linaro@kernel.org> > > > --- > > > drivers/phy/qualcomm/phy-qcom-qmp.c | 14 +++++++------- > > > 1 file changed, 7 insertions(+), 7 deletions(-) > > > > > > diff --git a/drivers/phy/qualcomm/phy-qcom-qmp.c b/drivers/phy/qualcomm/phy-qcom-qmp.c > > > index 8c2300bfe489..7d2d1ab061f7 100644 > > > --- a/drivers/phy/qualcomm/phy-qcom-qmp.c > > > +++ b/drivers/phy/qualcomm/phy-qcom-qmp.c > > > @@ -5375,14 +5375,14 @@ static int qcom_qmp_phy_power_on(struct phy *phy) > > > if (ret) { > > > dev_err(qmp->dev, "lane%d reset deassert failed\n", > > > qphy->index); > > > - goto err_lane_rst; > > > + return ret; > > > > This can be skipped if we retain the err_lane_rst label > > > > > } > > > } > > > > > > ret = clk_prepare_enable(qphy->pipe_clk); > > > if (ret) { > > > dev_err(qmp->dev, "pipe_clk enable failed err=%d\n", ret); > > > - goto err_clk_enable; > > > + goto err_reset_lane; > > > } > > > > > > /* Tx, Rx, and PCS configurations */ > > > @@ -5433,7 +5433,7 @@ static int qcom_qmp_phy_power_on(struct phy *phy) > > > > > > ret = reset_control_deassert(qmp->ufs_reset); > > > if (ret) > > > - goto err_lane_rst; > > > + goto err_disable_pipe_clk; > > > > this is the actual fix... > > Right, but with a one-line fix this would read > > goto err_pcs_ready; > > which makes no sense at all. Is that really what you prefer? Yes I feel for a fix we should always have the minimal changes.. makes porting easier > > Renaming just this one and the below one, would leave the error labels > named using two different schemes, which also isn't very nice, but > perhaps ok. > > > > > > > qcom_qmp_phy_configure(pcs_misc, cfg->regs, cfg->pcs_misc_tbl, > > > cfg->pcs_misc_tbl_num); > > > @@ -5472,17 +5472,17 @@ static int qcom_qmp_phy_power_on(struct phy *phy) > > > PHY_INIT_COMPLETE_TIMEOUT); > > > if (ret) { > > > dev_err(qmp->dev, "phy initialization timed-out\n"); > > > - goto err_pcs_ready; > > > + goto err_disable_pipe_clk; > > > > same rename here > > > > > } > > > } > > > return 0; > > > > > > -err_pcs_ready: > > > +err_disable_pipe_clk: > > > clk_disable_unprepare(qphy->pipe_clk); > > > -err_clk_enable: > > > +err_reset_lane: > > > if (cfg->has_lane_rst) > > > reset_control_assert(qphy->lane_rst); > > > -err_lane_rst: > > > + > > > return ret; > > > > while I feel the names given by this patch are better, they should not > > be in a fix patch. We should just add the one line fix here and do > > renames later > > I'll respin if you prefer, just want to double check that you really > want a one line fix (i.e. goto err_pcs_ready). Yes please -- ~Vinod ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2022-05-02 12:01 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-04-20 15:23 [PATCH 0/2] phy: qcom-qmp: pipe clock fixes Johan Hovold 2022-04-20 15:23 ` [PATCH 1/2] phy: qcom-qmp: fix phy-descriptor kernel-doc typo Johan Hovold 2022-05-02 10:10 ` Vinod Koul 2022-04-20 15:23 ` [PATCH 2/2] phy: qcom-qmp: fix pipe-clock imbalance on power-on failure Johan Hovold 2022-05-02 10:09 ` Vinod Koul 2022-05-02 10:27 ` Johan Hovold 2022-05-02 12:01 ` Vinod Koul
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).