Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S934158AbeALIqb (ORCPT + 1 other); Fri, 12 Jan 2018 03:46:31 -0500 Received: from smtp.codeaurora.org ([198.145.29.96]:58170 "EHLO smtp.codeaurora.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S934091AbeALIq0 (ORCPT ); Fri, 12 Jan 2018 03:46:26 -0500 DMARC-Filter: OpenDMARC Filter v1.3.2 smtp.codeaurora.org E16CC607DC Authentication-Results: pdx-caf-mail.web.codeaurora.org; dmarc=none (p=none dis=none) header.from=codeaurora.org Authentication-Results: pdx-caf-mail.web.codeaurora.org; spf=none smtp.mailfrom=mgautam@codeaurora.org Subject: Re: [PATCH v4 05/16] phy: qcom-qmp: Fix PHY block reset sequence To: Vivek Gautam Cc: Kishon Vijay Abraham I , Felipe Balbi , linux-arm-msm , Linux USB Mailing List , Varadarajan Narayanan , Viresh Kumar , Wei Yongjun , Fengguang Wu , "open list:GENERIC PHY FRAMEWORK" References: <1514978930-31341-1-git-send-email-mgautam@codeaurora.org> <1514978930-31341-6-git-send-email-mgautam@codeaurora.org> From: Manu Gautam Message-ID: <77b12f94-e496-ff44-fe61-75b21a1a4e76@codeaurora.org> Date: Fri, 12 Jan 2018 14:16:19 +0530 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.5.2 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Content-Language: en-US Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Return-Path: Hi Vivek, On 1/12/2018 2:14 PM, Vivek Gautam wrote: > On Wed, Jan 3, 2018 at 4:58 PM, Manu Gautam 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 >> --- >> 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 > > 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@vger.kernel.org >> 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