Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754699AbeALJMM (ORCPT + 1 other); Fri, 12 Jan 2018 04:12:12 -0500 Received: from smtp.codeaurora.org ([198.145.29.96]:46492 "EHLO smtp.codeaurora.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754488AbeALJME (ORCPT ); Fri, 12 Jan 2018 04:12:04 -0500 DMARC-Filter: OpenDMARC Filter v1.3.2 smtp.codeaurora.org 87E6C60B17 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=vivek.gautam@codeaurora.org X-Google-Smtp-Source: ACJfBosd1Wo3567+Affaa1aHMAPiwu0zhZtUoZwCD0jFZWm9rQO8lWg+FDgnnDUbtLj20+iExlqTRjNoyeCxKENWV7g= MIME-Version: 1.0 In-Reply-To: <77b12f94-e496-ff44-fe61-75b21a1a4e76@codeaurora.org> References: <1514978930-31341-1-git-send-email-mgautam@codeaurora.org> <1514978930-31341-6-git-send-email-mgautam@codeaurora.org> <77b12f94-e496-ff44-fe61-75b21a1a4e76@codeaurora.org> From: Vivek Gautam Date: Fri, 12 Jan 2018 14:42:01 +0530 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH v4 05/16] phy: qcom-qmp: Fix PHY block reset sequence To: Manu 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" Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Return-Path: On Fri, Jan 12, 2018 at 2:16 PM, Manu Gautam wrote: > 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. Okay, you can add my review then. Thanks. > >> >> 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 > -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation