Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932779AbcJTDpE (ORCPT ); Wed, 19 Oct 2016 23:45:04 -0400 Received: from smtp.codeaurora.org ([198.145.29.96]:49767 "EHLO smtp.codeaurora.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932541AbcJTDo7 (ORCPT ); Wed, 19 Oct 2016 23:44:59 -0400 DMARC-Filter: OpenDMARC Filter v1.3.1 smtp.codeaurora.org D2DBC61B25 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: Thu, 20 Oct 2016 09:14:51 +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, Vinayak Holikatti , 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: 3158 Lines: 99 On Thu, Oct 20, 2016 at 12:48 AM, Subhash Jadavani wrote: > On 2016-10-19 10:45, Vivek Gautam wrote: >> >> 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 ? > > > Yes, we want to turn off the ref clock (& other clocks) during aggressive > gating. Ok. > >> >> phy_power_off will also turn off the PHY. Do we want all this for >> aggressive >> clock gating ? > > > Yes, PHY rails can be powered off both during the aggressive clk gating and > runtime suspend. But as the regulator on/off latencies could be higher > (especially for the shared rail), we were turning them off doing it in > runtime suspend only (via phy_power_off()). > Now that phy_power_on/off is managing both clocks and regulators, and we > will really want to turn off the ref clocks during clock gating, there is no > option but to call phy_power_on/off during aggressive gating. Alright then, i will update the change to add phy_power_off/on() during aggressive clock gating. Thanks Vivek -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project