Hi Vivek, On 1/12/2018 2:14 PM, Vivek Gautam wrote: > On Wed, Jan 3, 2018 at 4:58 PM, Manu Gautam <mgautam@xxxxxxxxxxxxxx> wrote: >> PHY block or asynchronous reset requires signal >> to be asserted before de-asserting. Driver is only >> de-asserting signal which is already low, hence >> reset operation is a no-op. Fix this by asserting >> signal first. Also, resetting requires PHY clocks >> to be turned ON only after reset is finished. Fix >> that as well. >> >> Signed-off-by: Manu Gautam <mgautam@xxxxxxxxxxxxxx> >> --- >> drivers/phy/qualcomm/phy-qcom-qmp.c | 28 +++++++++++++++++++--------- >> 1 file changed, 19 insertions(+), 9 deletions(-) >> >> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp.c b/drivers/phy/qualcomm/phy-qcom-qmp.c >> index 1b82cea..ecff261 100644 >> --- a/drivers/phy/qualcomm/phy-qcom-qmp.c >> +++ b/drivers/phy/qualcomm/phy-qcom-qmp.c >> @@ -752,13 +752,16 @@ static int qcom_qmp_phy_com_init(struct qcom_qmp *qmp) >> goto err_reg_enable; >> } >> >> - ret = clk_bulk_prepare_enable(cfg->num_clks, qmp->clks); >> - if (ret) { >> - dev_err(qmp->dev, "failed to enable clks, err=%d\n", ret); >> - goto err_clk_enable; >> + for (i = 0; i < cfg->num_resets; i++) { >> + ret = reset_control_assert(qmp->resets[i]); >> + if (ret) { >> + dev_err(qmp->dev, "%s reset assert failed\n", >> + cfg->reset_list[i]); >> + goto err_rst_assert; >> + } >> } >> >> - for (i = 0; i < cfg->num_resets; i++) { >> + for (i = cfg->num_resets - 1; i >= 0; i--) { > Do we a dependency on the order in which these resets are > applied? > If not then we can use the 'bulk reset' APIs as well. We need to follow an order for assert and opposite order for de-assert, hence cant use 'bulk reset' APIs. > > With that bulk reset change you can add my review. > > Reviewed-by: Vivek Gautam <vivek.gautam@xxxxxxxxxxxxxx> > > Thanks > Vivek > >> ret = reset_control_deassert(qmp->resets[i]); >> if (ret) { >> dev_err(qmp->dev, "%s reset deassert failed\n", >> @@ -767,6 +770,12 @@ static int qcom_qmp_phy_com_init(struct qcom_qmp *qmp) >> } >> } >> >> + ret = clk_bulk_prepare_enable(cfg->num_clks, qmp->clks); >> + if (ret) { >> + dev_err(qmp->dev, "failed to enable clks, err=%d\n", ret); >> + goto err_rst; >> + } >> + >> if (cfg->has_phy_com_ctrl) >> qphy_setbits(serdes, cfg->regs[QPHY_COM_POWER_DOWN_CONTROL], >> SW_PWRDN); >> @@ -791,7 +800,7 @@ static int qcom_qmp_phy_com_init(struct qcom_qmp *qmp) >> if (ret) { >> dev_err(qmp->dev, >> "phy common block init timed-out\n"); >> - goto err_rst; >> + goto err_com_init; >> } >> } >> >> @@ -799,11 +808,12 @@ static int qcom_qmp_phy_com_init(struct qcom_qmp *qmp) >> >> return 0; >> >> +err_com_init: >> + clk_bulk_disable_unprepare(cfg->num_clks, qmp->clks); >> err_rst: >> - while (--i >= 0) >> + while (++i < cfg->num_resets) >> reset_control_assert(qmp->resets[i]); >> - clk_bulk_disable_unprepare(cfg->num_clks, qmp->clks); >> -err_clk_enable: >> +err_rst_assert: >> regulator_bulk_disable(cfg->num_vregs, qmp->vregs); >> err_reg_enable: >> mutex_unlock(&qmp->phy_mutex); >> -- >> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, >> a Linux Foundation Collaborative Project >> >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in >> the body of a message to majordomo@xxxxxxxxxxxxxxx >> More majordomo info at http://vger.kernel.org/majordomo-info.html > > -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html