linux-scsi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Subhash Jadavani <subhashj@codeaurora.org>
To: Vivek Gautam <vivek.gautam@codeaurora.org>
Cc: kishon <kishon@ti.com>,
	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
Subject: Re: [PATCH 08/10] ufs-qcom: phy/hcd: Refactoring phy clock handling
Date: Wed, 19 Oct 2016 12:18:12 -0700	[thread overview]
Message-ID: <b5e0234aea75ab395bc42ad85d45fe4e@codeaurora.org> (raw)
In-Reply-To: <CAFp+6iGN3_1+9Rj+ewMucMY2gVWpSuszM8_dB2Xu4a_iT+ZAHw@mail.gmail.com>

On 2016-10-19 10:45, Vivek Gautam wrote:
> Hi,
> 
> 
> On Wed, Oct 19, 2016 at 1:43 AM, Subhash Jadavani
> <subhashj@codeaurora.org> 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 <vivek.gautam@codeaurora.org>
>>> ---
>>> 
>>> 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.

> 
> 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.

> 
> 
> [snip]
> 
> 
> Regards
> Vivek

-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

  reply	other threads:[~2016-10-19 19:18 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-10-18 14:28 [PATCH v2 00/10] ufs-qcom: phy/hcd: Clean up qcom-ufs phy and ufs-qcom hcd Vivek Gautam
2016-10-18 14:28 ` [PATCH v2 01/10] phy: qcom-ufs: remove failure when rx/tx_iface_clk are absent Vivek Gautam
2016-10-18 18:38   ` Subhash Jadavani
2016-10-18 21:18   ` Stephen Boyd
2016-10-19 17:20     ` Vivek Gautam
2016-10-29 20:21       ` Vivek Gautam
2016-10-18 14:28 ` [PATCH v2 02/10] phy: qcom-ufs: Remove unnecessary BUG_ON Vivek Gautam
2016-10-18 14:28 ` [PATCH v2 03/10] phy: qcom-ufs: Use devm sibling of kstrdup for regulator names Vivek Gautam
2016-10-18 14:28 ` [PATCH v2 04/10] phy: qcom-ufs-qmp-xx: Discard remove callback for drivers Vivek Gautam
2016-10-26 20:20   ` Kishon Vijay Abraham I
2016-10-29 20:16     ` Vivek Gautam
2016-10-18 14:28 ` [PATCH v2 05/10] phy: qcom-ufs: Cleanup clock and regulator initialization Vivek Gautam
2016-10-18 14:28 ` [PATCH v2 06/10] phy: qcom-ufs: Remove unnecessary function declarations Vivek Gautam
2016-10-18 18:41   ` Subhash Jadavani
2016-10-18 14:28 ` [PATCH v2 07/10] phy: qcom-ufs-qmp-xx: Move clock and regulator init out of phy init Vivek Gautam
2016-10-18 14:28 ` [PATCH 08/10] ufs-qcom: phy/hcd: Refactoring phy clock handling Vivek Gautam
2016-10-18 20:13   ` Subhash Jadavani
2016-10-19 17:45     ` Vivek Gautam
2016-10-19 19:18       ` Subhash Jadavani [this message]
2016-10-20  3:44         ` Vivek Gautam
2016-10-18 14:28 ` [PATCH v2 09/10] scsi/ufs: qcom: Add phy_exit call in hcd exit path Vivek Gautam
2016-10-18 20:19   ` Subhash Jadavani
2016-10-18 14:28 ` [PATCH v2 10/10] phy: qcom-ufs: Remove common layer phy exit callback Vivek Gautam
2016-10-19  0:07   ` Subhash Jadavani
2016-10-25  0:40 ` [PATCH v2 00/10] ufs-qcom: phy/hcd: Clean up qcom-ufs phy and ufs-qcom hcd Martin K. Petersen
2016-10-25  6:00   ` Vivek Gautam

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=b5e0234aea75ab395bc42ad85d45fe4e@codeaurora.org \
    --to=subhashj@codeaurora.org \
    --cc=jejb@linux.vnet.ibm.com \
    --cc=kishon@ti.com \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=martin.petersen@oracle.com \
    --cc=vinholikatti@gmail.com \
    --cc=vivek.gautam@codeaurora.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).