* [PATCH 1/2] scsi: ufs: Clean up ufshcd_scale_clks() and clock scaling error out path [not found] <1584342373-10282-1-git-send-email-cang@codeaurora.org> @ 2020-03-16 7:06 ` Can Guo 2020-03-18 8:19 ` Avri Altman [not found] ` <1584342373-10282-3-git-send-email-cang@codeaurora.org> 1 sibling, 1 reply; 6+ messages in thread From: Can Guo @ 2020-03-16 7:06 UTC (permalink / raw) To: asutoshd, nguyenb, hongwus, rnayak, linux-scsi, kernel-team, saravanak, salyzyn, cang Cc: Subhash Jadavani, Alim Akhtar, Avri Altman, James E.J. Bottomley, Martin K. Petersen, Stanley Chu, Bean Huo, Bart Van Assche, Venkat Gopalakrishnan, Tomas Winkler, open list From: Subhash Jadavani <subhashj@codeaurora.org> This change introduces a func ufshcd_set_clk_freq() to explicitly set clock frequency so that it can be used in reset_and_resotre path and in ufshcd_scale_clks(). Meanwhile, this change cleans up the clock scaling error out path. Signed-off-by: Subhash Jadavani <subhashj@codeaurora.org> Signed-off-by: Can Guo <cang@codeaurora.org> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c index 2a2a63b..63aaa88f 100644 --- a/drivers/scsi/ufs/ufshcd.c +++ b/drivers/scsi/ufs/ufshcd.c @@ -855,28 +855,29 @@ static bool ufshcd_is_unipro_pa_params_tuning_req(struct ufs_hba *hba) return false; } -static int ufshcd_scale_clks(struct ufs_hba *hba, bool scale_up) +/** + * ufshcd_set_clk_freq - set UFS controller clock frequencies + * @hba: per adapter instance + * @scale_up: If True, set max possible frequency othewise set low frequency + * + * Returns 0 if successful + * Returns < 0 for any other errors + */ +static int ufshcd_set_clk_freq(struct ufs_hba *hba, bool scale_up) { int ret = 0; struct ufs_clk_info *clki; struct list_head *head = &hba->clk_list_head; - ktime_t start = ktime_get(); - bool clk_state_changed = false; if (list_empty(head)) goto out; - ret = ufshcd_vops_clk_scale_notify(hba, scale_up, PRE_CHANGE); - if (ret) - return ret; - list_for_each_entry(clki, head, list) { if (!IS_ERR_OR_NULL(clki->clk)) { if (scale_up && clki->max_freq) { if (clki->curr_freq == clki->max_freq) continue; - clk_state_changed = true; ret = clk_set_rate(clki->clk, clki->max_freq); if (ret) { dev_err(hba->dev, "%s: %s clk set rate(%dHz) failed, %d\n", @@ -895,7 +896,6 @@ static int ufshcd_scale_clks(struct ufs_hba *hba, bool scale_up) if (clki->curr_freq == clki->min_freq) continue; - clk_state_changed = true; ret = clk_set_rate(clki->clk, clki->min_freq); if (ret) { dev_err(hba->dev, "%s: %s clk set rate(%dHz) failed, %d\n", @@ -914,13 +914,36 @@ static int ufshcd_scale_clks(struct ufs_hba *hba, bool scale_up) clki->name, clk_get_rate(clki->clk)); } +out: + return ret; +} + +/** + * ufshcd_scale_clks - scale up or scale down UFS controller clocks + * @hba: per adapter instance + * @scale_up: True if scaling up and false if scaling down + * + * Returns 0 if successful + * Returns < 0 for any other errors + */ +static int ufshcd_scale_clks(struct ufs_hba *hba, bool scale_up) +{ + int ret = 0; + + ret = ufshcd_vops_clk_scale_notify(hba, scale_up, PRE_CHANGE); + if (ret) + return ret; + + ret = ufshcd_set_clk_freq(hba, scale_up); + if (ret) + return ret; + ret = ufshcd_vops_clk_scale_notify(hba, scale_up, POST_CHANGE); + if (ret) { + ufshcd_set_clk_freq(hba, !scale_up); + return ret; + } -out: - if (clk_state_changed) - trace_ufshcd_profile_clk_scaling(dev_name(hba->dev), - (scale_up ? "up" : "down"), - ktime_to_us(ktime_sub(ktime_get(), start)), ret); return ret; } @@ -1106,35 +1129,36 @@ static int ufshcd_devfreq_scale(struct ufs_hba *hba, bool scale_up) ret = ufshcd_clock_scaling_prepare(hba); if (ret) - return ret; + goto out; /* scale down the gear before scaling down clocks */ if (!scale_up) { ret = ufshcd_scale_gear(hba, false); if (ret) - goto out; + goto clk_scaling_unprepare; } ret = ufshcd_scale_clks(hba, scale_up); - if (ret) { - if (!scale_up) - ufshcd_scale_gear(hba, true); - goto out; - } + if (ret) + goto scale_up_gear; /* scale up the gear after scaling up clocks */ if (scale_up) { ret = ufshcd_scale_gear(hba, true); if (ret) { ufshcd_scale_clks(hba, false); - goto out; + goto clk_scaling_unprepare; } } - ret = ufshcd_vops_clk_scale_notify(hba, scale_up, POST_CHANGE); + goto clk_scaling_unprepare; -out: +scale_up_gear: + if (!scale_up) + ufshcd_scale_gear(hba, true); +clk_scaling_unprepare: ufshcd_clock_scaling_unprepare(hba); +out: ufshcd_release(hba); return ret; } @@ -6251,7 +6275,7 @@ static int ufshcd_host_reset_and_restore(struct ufs_hba *hba) spin_unlock_irqrestore(hba->host->host_lock, flags); /* scale up clocks to max frequency before full reinitialization */ - ufshcd_scale_clks(hba, true); + ufshcd_set_clk_freq(hba, true); err = ufshcd_hba_enable(hba); if (err) -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project. ^ permalink raw reply related [flat|nested] 6+ messages in thread
* RE: [PATCH 1/2] scsi: ufs: Clean up ufshcd_scale_clks() and clock scaling error out path 2020-03-16 7:06 ` [PATCH 1/2] scsi: ufs: Clean up ufshcd_scale_clks() and clock scaling error out path Can Guo @ 2020-03-18 8:19 ` Avri Altman 2020-03-25 8:56 ` Can Guo 0 siblings, 1 reply; 6+ messages in thread From: Avri Altman @ 2020-03-18 8:19 UTC (permalink / raw) To: Can Guo, asutoshd, nguyenb, hongwus, rnayak, linux-scsi, kernel-team, saravanak, salyzyn Cc: Subhash Jadavani, Alim Akhtar, James E.J. Bottomley, Martin K. Petersen, Stanley Chu, Bean Huo, Bart Van Assche, Venkat Gopalakrishnan, Tomas Winkler, open list Hi, > > From: Subhash Jadavani <subhashj@codeaurora.org> > > This change introduces a func ufshcd_set_clk_freq() to explicitly > set clock frequency so that it can be used in reset_and_resotre path and > in ufshcd_scale_clks(). Meanwhile, this change cleans up the clock scaling > error out path. > > Signed-off-by: Subhash Jadavani <subhashj@codeaurora.org> > Signed-off-by: Can Guo <cang@codeaurora.org> > > diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c > index 2a2a63b..63aaa88f 100644 > --- a/drivers/scsi/ufs/ufshcd.c > +++ b/drivers/scsi/ufs/ufshcd.c > @@ -855,28 +855,29 @@ static bool > ufshcd_is_unipro_pa_params_tuning_req(struct ufs_hba *hba) > return false; > } > > -static int ufshcd_scale_clks(struct ufs_hba *hba, bool scale_up) > +/** > + * ufshcd_set_clk_freq - set UFS controller clock frequencies > + * @hba: per adapter instance > + * @scale_up: If True, set max possible frequency othewise set low > frequency > + * > + * Returns 0 if successful > + * Returns < 0 for any other errors > + */ > +static int ufshcd_set_clk_freq(struct ufs_hba *hba, bool scale_up) Personally I prefer using the convention of "__scale_clks" to describe the privet core of scale_clks, But I know that many people are against it. > { > int ret = 0; > struct ufs_clk_info *clki; > struct list_head *head = &hba->clk_list_head; > - ktime_t start = ktime_get(); > - bool clk_state_changed = false; > > if (list_empty(head)) > goto out; > > - ret = ufshcd_vops_clk_scale_notify(hba, scale_up, PRE_CHANGE); > - if (ret) > - return ret; > - > list_for_each_entry(clki, head, list) { > if (!IS_ERR_OR_NULL(clki->clk)) { > if (scale_up && clki->max_freq) { > if (clki->curr_freq == clki->max_freq) > continue; > > - clk_state_changed = true; > ret = clk_set_rate(clki->clk, clki->max_freq); > if (ret) { > dev_err(hba->dev, "%s: %s clk set rate(%dHz) failed, > %d\n", > @@ -895,7 +896,6 @@ static int ufshcd_scale_clks(struct ufs_hba *hba, > bool scale_up) > if (clki->curr_freq == clki->min_freq) > continue; > > - clk_state_changed = true; > ret = clk_set_rate(clki->clk, clki->min_freq); > if (ret) { > dev_err(hba->dev, "%s: %s clk set rate(%dHz) failed, > %d\n", > @@ -914,13 +914,36 @@ static int ufshcd_scale_clks(struct ufs_hba *hba, > bool scale_up) > clki->name, clk_get_rate(clki->clk)); > } > > +out: > + return ret; > +} > + > +/** > + * ufshcd_scale_clks - scale up or scale down UFS controller clocks > + * @hba: per adapter instance > + * @scale_up: True if scaling up and false if scaling down > + * > + * Returns 0 if successful > + * Returns < 0 for any other errors > + */ > +static int ufshcd_scale_clks(struct ufs_hba *hba, bool scale_up) > +{ > + int ret = 0; > + > + ret = ufshcd_vops_clk_scale_notify(hba, scale_up, PRE_CHANGE); > + if (ret) > + return ret; > + > + ret = ufshcd_set_clk_freq(hba, scale_up); > + if (ret) > + return ret; > + > ret = ufshcd_vops_clk_scale_notify(hba, scale_up, POST_CHANGE); > + if (ret) { > + ufshcd_set_clk_freq(hba, !scale_up); > + return ret; > + } > > -out: > - if (clk_state_changed) > - trace_ufshcd_profile_clk_scaling(dev_name(hba->dev), > - (scale_up ? "up" : "down"), > - ktime_to_us(ktime_sub(ktime_get(), start)), ret); Why remove the ufshcd_profile_clk_scaling trace? > return ret; > } > > @@ -1106,35 +1129,36 @@ static int ufshcd_devfreq_scale(struct ufs_hba > *hba, bool scale_up) > > ret = ufshcd_clock_scaling_prepare(hba); > if (ret) > - return ret; > + goto out; Are you fixing here a hold without release? Should make note of that. > > /* scale down the gear before scaling down clocks */ > if (!scale_up) { > ret = ufshcd_scale_gear(hba, false); > if (ret) > - goto out; > + goto clk_scaling_unprepare; > } > > ret = ufshcd_scale_clks(hba, scale_up); > - if (ret) { > - if (!scale_up) > - ufshcd_scale_gear(hba, true); > - goto out; > - } > + if (ret) > + goto scale_up_gear; > > /* scale up the gear after scaling up clocks */ > if (scale_up) { > ret = ufshcd_scale_gear(hba, true); > if (ret) { > ufshcd_scale_clks(hba, false); > - goto out; > + goto clk_scaling_unprepare; > } > } > > - ret = ufshcd_vops_clk_scale_notify(hba, scale_up, POST_CHANGE); > + goto clk_scaling_unprepare; I think you should find a way to make this function more readable. Adding all those "spaghetti" gotos making it even harder to read. Thanks, Avri > > -out: > +scale_up_gear: > + if (!scale_up) > + ufshcd_scale_gear(hba, true); > +clk_scaling_unprepare: > ufshcd_clock_scaling_unprepare(hba); > +out: > ufshcd_release(hba); > return ret; > } > @@ -6251,7 +6275,7 @@ static int ufshcd_host_reset_and_restore(struct > ufs_hba *hba) > spin_unlock_irqrestore(hba->host->host_lock, flags); > > /* scale up clocks to max frequency before full reinitialization */ > - ufshcd_scale_clks(hba, true); > + ufshcd_set_clk_freq(hba, true); > > err = ufshcd_hba_enable(hba); > if (err) > -- > Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a > Linux Foundation Collaborative Project. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] scsi: ufs: Clean up ufshcd_scale_clks() and clock scaling error out path 2020-03-18 8:19 ` Avri Altman @ 2020-03-25 8:56 ` Can Guo 0 siblings, 0 replies; 6+ messages in thread From: Can Guo @ 2020-03-25 8:56 UTC (permalink / raw) To: Avri Altman Cc: asutoshd, nguyenb, hongwus, rnayak, linux-scsi, kernel-team, saravanak, salyzyn, Subhash Jadavani, Alim Akhtar, James E.J. Bottomley, Martin K. Petersen, Stanley Chu, Bean Huo, Bart Van Assche, Venkat Gopalakrishnan, Tomas Winkler, open list On 2020-03-18 16:19, Avri Altman wrote: > Hi, > >> >> From: Subhash Jadavani <subhashj@codeaurora.org> >> >> This change introduces a func ufshcd_set_clk_freq() to explicitly >> set clock frequency so that it can be used in reset_and_resotre path >> and >> in ufshcd_scale_clks(). Meanwhile, this change cleans up the clock >> scaling >> error out path. >> >> Signed-off-by: Subhash Jadavani <subhashj@codeaurora.org> >> Signed-off-by: Can Guo <cang@codeaurora.org> >> >> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c >> index 2a2a63b..63aaa88f 100644 >> --- a/drivers/scsi/ufs/ufshcd.c >> +++ b/drivers/scsi/ufs/ufshcd.c >> @@ -855,28 +855,29 @@ static bool >> ufshcd_is_unipro_pa_params_tuning_req(struct ufs_hba *hba) >> return false; >> } >> >> -static int ufshcd_scale_clks(struct ufs_hba *hba, bool scale_up) >> +/** >> + * ufshcd_set_clk_freq - set UFS controller clock frequencies >> + * @hba: per adapter instance >> + * @scale_up: If True, set max possible frequency othewise set low >> frequency >> + * >> + * Returns 0 if successful >> + * Returns < 0 for any other errors >> + */ >> +static int ufshcd_set_clk_freq(struct ufs_hba *hba, bool scale_up) > Personally I prefer using the convention of "__scale_clks" to describe > the privet core of scale_clks, > But I know that many people are against it. > >> { >> int ret = 0; >> struct ufs_clk_info *clki; >> struct list_head *head = &hba->clk_list_head; >> - ktime_t start = ktime_get(); >> - bool clk_state_changed = false; >> >> if (list_empty(head)) >> goto out; >> >> - ret = ufshcd_vops_clk_scale_notify(hba, scale_up, PRE_CHANGE); >> - if (ret) >> - return ret; >> - >> list_for_each_entry(clki, head, list) { >> if (!IS_ERR_OR_NULL(clki->clk)) { >> if (scale_up && clki->max_freq) { >> if (clki->curr_freq == clki->max_freq) >> continue; >> >> - clk_state_changed = true; >> ret = clk_set_rate(clki->clk, >> clki->max_freq); >> if (ret) { >> dev_err(hba->dev, "%s: %s clk >> set rate(%dHz) failed, >> %d\n", >> @@ -895,7 +896,6 @@ static int ufshcd_scale_clks(struct ufs_hba *hba, >> bool scale_up) >> if (clki->curr_freq == clki->min_freq) >> continue; >> >> - clk_state_changed = true; >> ret = clk_set_rate(clki->clk, >> clki->min_freq); >> if (ret) { >> dev_err(hba->dev, "%s: %s clk >> set rate(%dHz) failed, >> %d\n", >> @@ -914,13 +914,36 @@ static int ufshcd_scale_clks(struct ufs_hba >> *hba, >> bool scale_up) >> clki->name, clk_get_rate(clki->clk)); >> } >> >> +out: >> + return ret; >> +} >> + >> +/** >> + * ufshcd_scale_clks - scale up or scale down UFS controller clocks >> + * @hba: per adapter instance >> + * @scale_up: True if scaling up and false if scaling down >> + * >> + * Returns 0 if successful >> + * Returns < 0 for any other errors >> + */ >> +static int ufshcd_scale_clks(struct ufs_hba *hba, bool scale_up) >> +{ >> + int ret = 0; >> + >> + ret = ufshcd_vops_clk_scale_notify(hba, scale_up, PRE_CHANGE); >> + if (ret) >> + return ret; >> + >> + ret = ufshcd_set_clk_freq(hba, scale_up); >> + if (ret) >> + return ret; >> + >> ret = ufshcd_vops_clk_scale_notify(hba, scale_up, >> POST_CHANGE); >> + if (ret) { >> + ufshcd_set_clk_freq(hba, !scale_up); >> + return ret; >> + } >> >> -out: >> - if (clk_state_changed) >> - trace_ufshcd_profile_clk_scaling(dev_name(hba->dev), >> - (scale_up ? "up" : "down"), >> - ktime_to_us(ktime_sub(ktime_get(), start)), >> ret); > > Why remove the ufshcd_profile_clk_scaling trace? > > Shall add it back, this is just a collection of Subhash's changes. >> return ret; >> } >> >> @@ -1106,35 +1129,36 @@ static int ufshcd_devfreq_scale(struct ufs_hba >> *hba, bool scale_up) >> >> ret = ufshcd_clock_scaling_prepare(hba); >> if (ret) >> - return ret; >> + goto out; > Are you fixing here a hold without release? > Should make note of that. > Yes, true >> >> /* scale down the gear before scaling down clocks */ >> if (!scale_up) { >> ret = ufshcd_scale_gear(hba, false); >> if (ret) >> - goto out; >> + goto clk_scaling_unprepare; >> } >> >> ret = ufshcd_scale_clks(hba, scale_up); >> - if (ret) { >> - if (!scale_up) >> - ufshcd_scale_gear(hba, true); >> - goto out; >> - } >> + if (ret) >> + goto scale_up_gear; >> >> /* scale up the gear after scaling up clocks */ >> if (scale_up) { >> ret = ufshcd_scale_gear(hba, true); >> if (ret) { >> ufshcd_scale_clks(hba, false); >> - goto out; >> + goto clk_scaling_unprepare; >> } >> } >> >> - ret = ufshcd_vops_clk_scale_notify(hba, scale_up, >> POST_CHANGE); >> + goto clk_scaling_unprepare; > I think you should find a way to make this function more readable. > Adding all those "spaghetti" gotos making it even harder to read. > > Thanks, > Avri > This is just a collection of Subhash's changes. I think the 2 clk_scaling_unprepare and out gotos make sense, but the scale_up_gear goto is a bit vague. I will remove the scale_up_gear goto. Thanks, Can Guo. >> >> -out: >> +scale_up_gear: >> + if (!scale_up) >> + ufshcd_scale_gear(hba, true); >> +clk_scaling_unprepare: >> ufshcd_clock_scaling_unprepare(hba); >> +out: >> ufshcd_release(hba); >> return ret; >> } >> @@ -6251,7 +6275,7 @@ static int ufshcd_host_reset_and_restore(struct >> ufs_hba *hba) >> spin_unlock_irqrestore(hba->host->host_lock, flags); >> >> /* scale up clocks to max frequency before full >> reinitialization */ >> - ufshcd_scale_clks(hba, true); >> + ufshcd_set_clk_freq(hba, true); >> >> err = ufshcd_hba_enable(hba); >> if (err) >> -- >> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a >> Linux Foundation Collaborative Project. ^ permalink raw reply [flat|nested] 6+ messages in thread
[parent not found: <1584342373-10282-3-git-send-email-cang@codeaurora.org>]
* Re: [PATCH 2/2] scsi: ufs: Do not rely on prefetched data [not found] ` <1584342373-10282-3-git-send-email-cang@codeaurora.org> @ 2020-03-16 13:47 ` Stanley Chu 2020-03-18 8:36 ` Avri Altman 1 sibling, 0 replies; 6+ messages in thread From: Stanley Chu @ 2020-03-16 13:47 UTC (permalink / raw) To: Can Guo Cc: asutoshd, nguyenb, hongwus, rnayak, linux-scsi, kernel-team, saravanak, salyzyn, Alim Akhtar, Avri Altman, James E.J. Bottomley, Martin K. Petersen, Bean Huo, Bart Van Assche, Venkat Gopalakrishnan, Tomas Winkler, Bjorn Andersson, open list Hi Can, On Mon, 2020-03-16 at 00:06 -0700, Can Guo wrote: > We were setting bActiveICCLevel attribute for UFS device only once but > type of this attribute has changed from persistent to volatile since UFS > device specification v2.1. This attribute is set to the default value after > power cycle or hardware reset event. It isn't safe to rely on prefetched > data (only used for bActiveICCLevel attribute now). Hence this change > removes the code related to data prefetching and set this parameter on > every attempt to probe the UFS device. > > Signed-off-by: Can Guo <cang@codeaurora.org> Reviewed and Tested by: Stanley Chu <stanley.chu@mediatek.com> ^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH 2/2] scsi: ufs: Do not rely on prefetched data [not found] ` <1584342373-10282-3-git-send-email-cang@codeaurora.org> 2020-03-16 13:47 ` [PATCH 2/2] scsi: ufs: Do not rely on prefetched data Stanley Chu @ 2020-03-18 8:36 ` Avri Altman 1 sibling, 0 replies; 6+ messages in thread From: Avri Altman @ 2020-03-18 8:36 UTC (permalink / raw) To: Can Guo, asutoshd, nguyenb, hongwus, rnayak, linux-scsi, kernel-team, saravanak, salyzyn Cc: Alim Akhtar, James E.J. Bottomley, Martin K. Petersen, Stanley Chu, Bean Huo, Bart Van Assche, Venkat Gopalakrishnan, Tomas Winkler, Bjorn Andersson, open list > > We were setting bActiveICCLevel attribute for UFS device only once but > type of this attribute has changed from persistent to volatile since UFS > device specification v2.1. This attribute is set to the default value after > power cycle or hardware reset event. It isn't safe to rely on prefetched > data (only used for bActiveICCLevel attribute now). Hence this change > removes the code related to data prefetching and set this parameter on > every attempt to probe the UFS device. > > Signed-off-by: Can Guo <cang@codeaurora.org> Reviewed-by: Avri Altman <avri.altman@wdc.com> Thanks for fixing this. Avri ^ permalink raw reply [flat|nested] 6+ messages in thread
[parent not found: <1584339655-20337-1-git-send-email-cang@codeaurora.org>]
* [PATCH 1/2] scsi: ufs: Clean up ufshcd_scale_clks() and clock scaling error out path [not found] <1584339655-20337-1-git-send-email-cang@codeaurora.org> @ 2020-03-16 6:20 ` Can Guo 0 siblings, 0 replies; 6+ messages in thread From: Can Guo @ 2020-03-16 6:20 UTC (permalink / raw) To: asutoshd, nguyenb, hongwus, rnayak, linux-scsi, kernel-team, saravanak, salyzyn, cang Cc: Subhash Jadavani, Alim Akhtar, Avri Altman, James E.J. Bottomley, Martin K. Petersen, Stanley Chu, Bean Huo, Bart Van Assche, Venkat Gopalakrishnan, Tomas Winkler, open list From: Subhash Jadavani <subhashj@codeaurora.org> This change introduces a func ufshcd_set_clk_freq() to explicitly set clock frequency so that it can be used in reset_and_resotre path and in ufshcd_scale_clks(). Meanwhile, this change cleans up the clock scaling error out path. Signed-off-by: Subhash Jadavani <subhashj@codeaurora.org> Signed-off-by: Can Guo <cang@codeaurora.org> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c index 2a2a63b..63aaa88f 100644 --- a/drivers/scsi/ufs/ufshcd.c +++ b/drivers/scsi/ufs/ufshcd.c @@ -855,28 +855,29 @@ static bool ufshcd_is_unipro_pa_params_tuning_req(struct ufs_hba *hba) return false; } -static int ufshcd_scale_clks(struct ufs_hba *hba, bool scale_up) +/** + * ufshcd_set_clk_freq - set UFS controller clock frequencies + * @hba: per adapter instance + * @scale_up: If True, set max possible frequency othewise set low frequency + * + * Returns 0 if successful + * Returns < 0 for any other errors + */ +static int ufshcd_set_clk_freq(struct ufs_hba *hba, bool scale_up) { int ret = 0; struct ufs_clk_info *clki; struct list_head *head = &hba->clk_list_head; - ktime_t start = ktime_get(); - bool clk_state_changed = false; if (list_empty(head)) goto out; - ret = ufshcd_vops_clk_scale_notify(hba, scale_up, PRE_CHANGE); - if (ret) - return ret; - list_for_each_entry(clki, head, list) { if (!IS_ERR_OR_NULL(clki->clk)) { if (scale_up && clki->max_freq) { if (clki->curr_freq == clki->max_freq) continue; - clk_state_changed = true; ret = clk_set_rate(clki->clk, clki->max_freq); if (ret) { dev_err(hba->dev, "%s: %s clk set rate(%dHz) failed, %d\n", @@ -895,7 +896,6 @@ static int ufshcd_scale_clks(struct ufs_hba *hba, bool scale_up) if (clki->curr_freq == clki->min_freq) continue; - clk_state_changed = true; ret = clk_set_rate(clki->clk, clki->min_freq); if (ret) { dev_err(hba->dev, "%s: %s clk set rate(%dHz) failed, %d\n", @@ -914,13 +914,36 @@ static int ufshcd_scale_clks(struct ufs_hba *hba, bool scale_up) clki->name, clk_get_rate(clki->clk)); } +out: + return ret; +} + +/** + * ufshcd_scale_clks - scale up or scale down UFS controller clocks + * @hba: per adapter instance + * @scale_up: True if scaling up and false if scaling down + * + * Returns 0 if successful + * Returns < 0 for any other errors + */ +static int ufshcd_scale_clks(struct ufs_hba *hba, bool scale_up) +{ + int ret = 0; + + ret = ufshcd_vops_clk_scale_notify(hba, scale_up, PRE_CHANGE); + if (ret) + return ret; + + ret = ufshcd_set_clk_freq(hba, scale_up); + if (ret) + return ret; + ret = ufshcd_vops_clk_scale_notify(hba, scale_up, POST_CHANGE); + if (ret) { + ufshcd_set_clk_freq(hba, !scale_up); + return ret; + } -out: - if (clk_state_changed) - trace_ufshcd_profile_clk_scaling(dev_name(hba->dev), - (scale_up ? "up" : "down"), - ktime_to_us(ktime_sub(ktime_get(), start)), ret); return ret; } @@ -1106,35 +1129,36 @@ static int ufshcd_devfreq_scale(struct ufs_hba *hba, bool scale_up) ret = ufshcd_clock_scaling_prepare(hba); if (ret) - return ret; + goto out; /* scale down the gear before scaling down clocks */ if (!scale_up) { ret = ufshcd_scale_gear(hba, false); if (ret) - goto out; + goto clk_scaling_unprepare; } ret = ufshcd_scale_clks(hba, scale_up); - if (ret) { - if (!scale_up) - ufshcd_scale_gear(hba, true); - goto out; - } + if (ret) + goto scale_up_gear; /* scale up the gear after scaling up clocks */ if (scale_up) { ret = ufshcd_scale_gear(hba, true); if (ret) { ufshcd_scale_clks(hba, false); - goto out; + goto clk_scaling_unprepare; } } - ret = ufshcd_vops_clk_scale_notify(hba, scale_up, POST_CHANGE); + goto clk_scaling_unprepare; -out: +scale_up_gear: + if (!scale_up) + ufshcd_scale_gear(hba, true); +clk_scaling_unprepare: ufshcd_clock_scaling_unprepare(hba); +out: ufshcd_release(hba); return ret; } @@ -6251,7 +6275,7 @@ static int ufshcd_host_reset_and_restore(struct ufs_hba *hba) spin_unlock_irqrestore(hba->host->host_lock, flags); /* scale up clocks to max frequency before full reinitialization */ - ufshcd_scale_clks(hba, true); + ufshcd_set_clk_freq(hba, true); err = ufshcd_hba_enable(hba); if (err) -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project. ^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-03-25 8:56 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <1584342373-10282-1-git-send-email-cang@codeaurora.org>
2020-03-16 7:06 ` [PATCH 1/2] scsi: ufs: Clean up ufshcd_scale_clks() and clock scaling error out path Can Guo
2020-03-18 8:19 ` Avri Altman
2020-03-25 8:56 ` Can Guo
[not found] ` <1584342373-10282-3-git-send-email-cang@codeaurora.org>
2020-03-16 13:47 ` [PATCH 2/2] scsi: ufs: Do not rely on prefetched data Stanley Chu
2020-03-18 8:36 ` Avri Altman
[not found] <1584339655-20337-1-git-send-email-cang@codeaurora.org>
2020-03-16 6:20 ` [PATCH 1/2] scsi: ufs: Clean up ufshcd_scale_clks() and clock scaling error out path Can Guo
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).