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
next prev parent 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).