Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S943212AbcJSRpT (ORCPT ); Wed, 19 Oct 2016 13:45:19 -0400 Received: from smtp.codeaurora.org ([198.145.29.96]:35835 "EHLO smtp.codeaurora.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S941581AbcJSRpR (ORCPT ); Wed, 19 Oct 2016 13:45:17 -0400 DMARC-Filter: OpenDMARC Filter v1.3.1 smtp.codeaurora.org 63FDA61887 Authentication-Results: pdx-caf-mail.web.codeaurora.org; dmarc=none header.from=codeaurora.org Authentication-Results: pdx-caf-mail.web.codeaurora.org; spf=pass smtp.mailfrom=vivek.gautam@codeaurora.org MIME-Version: 1.0 In-Reply-To: References: <1476800897-19898-1-git-send-email-vivek.gautam@codeaurora.org> <1476800897-19898-9-git-send-email-vivek.gautam@codeaurora.org> From: Vivek Gautam Date: Wed, 19 Oct 2016 23:15:14 +0530 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH 08/10] ufs-qcom: phy/hcd: Refactoring phy clock handling To: Subhash Jadavani Cc: kishon , jejb@linux.vnet.ibm.com, vinholikatti@gmail.com, martin.petersen@oracle.com, "linux-kernel@vger.kernel.org" , linux-scsi@vger.kernel.org, linux-arm-msm@vger.kernel.org Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2207 Lines: 68 Hi, On Wed, Oct 19, 2016 at 1:43 AM, Subhash Jadavani wrote: > On 2016-10-18 07:28, Vivek Gautam wrote: >> >> Add phy clock enable code to phy_power_on/off callbacks, and >> remove explicit calls to enable these phy clocks from the >> ufs-qcom hcd driver. >> >> Signed-off-by: Vivek Gautam >> --- >> >> Changes since v1: >> - staticized ufs_qcom_phy_enable(/disable)_ref_clk(), >> - staticized ufs_qcom_phy_enable(/disable)_iface_clk() >> - removed function declaration and export symbol for these APIs. [snip] >> --- a/drivers/scsi/ufs/ufs-qcom.c >> +++ b/drivers/scsi/ufs/ufs-qcom.c >> @@ -1112,17 +1112,6 @@ static int ufs_qcom_setup_clocks(struct ufs_hba >> *hba, bool on) >> return 0; >> >> if (on) { >> - err = ufs_qcom_phy_enable_iface_clk(host->generic_phy); >> - if (err) >> - goto out; >> - >> - err = ufs_qcom_phy_enable_ref_clk(host->generic_phy); >> - if (err) { >> - dev_err(hba->dev, "%s enable phy ref clock failed, >> err=%d\n", >> - __func__, err); >> - ufs_qcom_phy_disable_iface_clk(host->generic_phy); >> - goto out; >> - } > > > Now that you are moving these ref clk enable/disable to phy_power_on/off and > these phy_power_on/off are called only in runtime suspend/resume (3 seconds > after last UFS access). > Goal is to disable the phy reference clock during aggressive gating (10ms > from last UFS access) so shouldn't we call the phy_power_on/off from these > setup_clocks() function as well? > So setup_clocks() is called for aggressive clock gating as well ? If that's the case then yes, we may need to call. But we should try to understand here. The phy_power_off turns off all the clocks - reflclk, and other interface clocks. Do we want all of them to be turned off ? phy_power_off will also turn off the PHY. Do we want all this for aggressive clock gating ? [snip] Regards Vivek -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project