2020-03-25 17:52:30

by Can Guo

[permalink] [raw]
Subject: [PATCH v5 1/2] scsi: ufs: Clean up ufshcd_scale_clks() and clock scaling error out path

From: Subhash Jadavani <[email protected]>

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.

Fixes: a3cd5ec55f6c ("scsi: ufs: add load based scaling of UFS gear")
Signed-off-by: Subhash Jadavani <[email protected]>
Signed-off-by: Can Guo <[email protected]>
---
drivers/scsi/ufs/ufshcd.c | 68 ++++++++++++++++++++++++++++++++---------------
1 file changed, 46 insertions(+), 22 deletions(-)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 2a2a63b..9c26f82 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,40 @@ 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),
+ 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 +1133,32 @@ 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 out_unprepare;
}

ret = ufshcd_scale_clks(hba, scale_up);
if (ret) {
if (!scale_up)
ufshcd_scale_gear(hba, true);
- goto out;
+ goto out_unprepare;
}

/* scale up the gear after scaling up clocks */
if (scale_up) {
ret = ufshcd_scale_gear(hba, true);
- if (ret) {
+ if (ret)
ufshcd_scale_clks(hba, false);
- goto out;
- }
}

- ret = ufshcd_vops_clk_scale_notify(hba, scale_up, POST_CHANGE);
-
-out:
+out_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.


2020-03-26 08:47:33

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH v5 1/2] scsi: ufs: Clean up ufshcd_scale_clks() and clock scaling error out path

Hi Can,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on scsi/for-next]
[also build test ERROR on mkp-scsi/for-next v5.6-rc7 next-20200325]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url: https://github.com/0day-ci/linux/commits/Can-Guo/UFS-driver-general-fixes-bundle-2/20200326-143749
base: https://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi.git for-next
config: m68k-allmodconfig (attached as .config)
compiler: m68k-linux-gcc (GCC) 9.2.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=9.2.0 make.cross ARCH=m68k

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <[email protected]>

All errors (new ones prefixed by >>):

In file included from include/linux/timer.h:6,
from include/linux/workqueue.h:9,
from include/linux/rhashtable-types.h:15,
from include/linux/ipc.h:7,
from include/uapi/linux/sem.h:5,
from include/linux/sem.h:5,
from include/linux/sched.h:15,
from include/linux/ratelimit.h:6,
from include/linux/dev_printk.h:16,
from include/linux/device.h:15,
from include/linux/async.h:14,
from drivers/scsi/ufs/ufshcd.c:40:
drivers/scsi/ufs/ufshcd.c: In function 'ufshcd_scale_clks':
>> drivers/scsi/ufs/ufshcd.c:949:39: error: 'start' undeclared (first use in this function); did you mean 'stat'?
949 | ktime_to_us(ktime_sub(ktime_get(), start)), ret);
| ^~~~~
include/linux/ktime.h:46:39: note: in definition of macro 'ktime_sub'
46 | #define ktime_sub(lhs, rhs) ((lhs) - (rhs))
| ^~~
drivers/scsi/ufs/ufshcd.c:949:39: note: each undeclared identifier is reported only once for each function it appears in
949 | ktime_to_us(ktime_sub(ktime_get(), start)), ret);
| ^~~~~
include/linux/ktime.h:46:39: note: in definition of macro 'ktime_sub'
46 | #define ktime_sub(lhs, rhs) ((lhs) - (rhs))
| ^~~

vim +949 drivers/scsi/ufs/ufshcd.c

a67142dac16542 Subhash Jadavani 2020-03-25 920
a67142dac16542 Subhash Jadavani 2020-03-25 921 /**
a67142dac16542 Subhash Jadavani 2020-03-25 922 * ufshcd_scale_clks - scale up or scale down UFS controller clocks
a67142dac16542 Subhash Jadavani 2020-03-25 923 * @hba: per adapter instance
a67142dac16542 Subhash Jadavani 2020-03-25 924 * @scale_up: True if scaling up and false if scaling down
a67142dac16542 Subhash Jadavani 2020-03-25 925 *
a67142dac16542 Subhash Jadavani 2020-03-25 926 * Returns 0 if successful
a67142dac16542 Subhash Jadavani 2020-03-25 927 * Returns < 0 for any other errors
a67142dac16542 Subhash Jadavani 2020-03-25 928 */
a67142dac16542 Subhash Jadavani 2020-03-25 929 static int ufshcd_scale_clks(struct ufs_hba *hba, bool scale_up)
a67142dac16542 Subhash Jadavani 2020-03-25 930 {
a67142dac16542 Subhash Jadavani 2020-03-25 931 int ret = 0;
a67142dac16542 Subhash Jadavani 2020-03-25 932
a67142dac16542 Subhash Jadavani 2020-03-25 933 ret = ufshcd_vops_clk_scale_notify(hba, scale_up, PRE_CHANGE);
a67142dac16542 Subhash Jadavani 2020-03-25 934 if (ret)
a67142dac16542 Subhash Jadavani 2020-03-25 935 return ret;
a67142dac16542 Subhash Jadavani 2020-03-25 936
a67142dac16542 Subhash Jadavani 2020-03-25 937 ret = ufshcd_set_clk_freq(hba, scale_up);
a67142dac16542 Subhash Jadavani 2020-03-25 938 if (ret)
a67142dac16542 Subhash Jadavani 2020-03-25 939 return ret;
a67142dac16542 Subhash Jadavani 2020-03-25 940
a3cd5ec55f6c72 Subhash Jadavani 2017-02-03 941 ret = ufshcd_vops_clk_scale_notify(hba, scale_up, POST_CHANGE);
a67142dac16542 Subhash Jadavani 2020-03-25 942 if (ret) {
a67142dac16542 Subhash Jadavani 2020-03-25 943 ufshcd_set_clk_freq(hba, !scale_up);
a67142dac16542 Subhash Jadavani 2020-03-25 944 return ret;
a67142dac16542 Subhash Jadavani 2020-03-25 945 }
a3cd5ec55f6c72 Subhash Jadavani 2017-02-03 946
a3cd5ec55f6c72 Subhash Jadavani 2017-02-03 947 trace_ufshcd_profile_clk_scaling(dev_name(hba->dev),
a3cd5ec55f6c72 Subhash Jadavani 2017-02-03 948 (scale_up ? "up" : "down"),
a3cd5ec55f6c72 Subhash Jadavani 2017-02-03 @949 ktime_to_us(ktime_sub(ktime_get(), start)), ret);
a67142dac16542 Subhash Jadavani 2020-03-25 950
a3cd5ec55f6c72 Subhash Jadavani 2017-02-03 951 return ret;
a3cd5ec55f6c72 Subhash Jadavani 2017-02-03 952 }
a3cd5ec55f6c72 Subhash Jadavani 2017-02-03 953

:::::: The code at line 949 was first introduced by commit
:::::: a3cd5ec55f6c72834f812f9150deb38ddc019782 scsi: ufs: add load based scaling of UFS gear

:::::: TO: [email protected] <[email protected]>
:::::: CC: Martin K. Petersen <[email protected]>

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]


Attachments:
(No filename) (5.42 kB)
.config.gz (51.55 kB)
Download all attachments

2020-03-26 11:03:00

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH v5 1/2] scsi: ufs: Clean up ufshcd_scale_clks() and clock scaling error out path

Hi Can,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on scsi/for-next]
[also build test ERROR on mkp-scsi/for-next target/for-next v5.6-rc7 next-20200326]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url: https://github.com/0day-ci/linux/commits/Can-Guo/UFS-driver-general-fixes-bundle-2/20200326-143749
base: https://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi.git for-next
config: sparc-allyesconfig (attached as .config)
compiler: sparc64-linux-gcc (GCC) 9.2.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=9.2.0 make.cross ARCH=sparc

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <[email protected]>

All errors (new ones prefixed by >>):

In file included from include/linux/timer.h:6,
from include/linux/workqueue.h:9,
from include/linux/rhashtable-types.h:15,
from include/linux/ipc.h:7,
from include/uapi/linux/sem.h:5,
from include/linux/sem.h:5,
from include/linux/sched.h:15,
from include/linux/ratelimit.h:6,
from include/linux/dev_printk.h:16,
from include/linux/device.h:15,
from include/linux/async.h:14,
from drivers/scsi/ufs/ufshcd.c:40:
drivers/scsi/ufs/ufshcd.c: In function 'ufshcd_scale_clks':
>> drivers/scsi/ufs/ufshcd.c:949:39: error: 'start' undeclared (first use in this function); did you mean '_start'?
949 | ktime_to_us(ktime_sub(ktime_get(), start)), ret);
| ^~~~~
include/linux/ktime.h:46:39: note: in definition of macro 'ktime_sub'
46 | #define ktime_sub(lhs, rhs) ((lhs) - (rhs))
| ^~~
drivers/scsi/ufs/ufshcd.c:949:39: note: each undeclared identifier is reported only once for each function it appears in
949 | ktime_to_us(ktime_sub(ktime_get(), start)), ret);
| ^~~~~
include/linux/ktime.h:46:39: note: in definition of macro 'ktime_sub'
46 | #define ktime_sub(lhs, rhs) ((lhs) - (rhs))
| ^~~

vim +949 drivers/scsi/ufs/ufshcd.c

a67142dac16542 Subhash Jadavani 2020-03-25 920
a67142dac16542 Subhash Jadavani 2020-03-25 921 /**
a67142dac16542 Subhash Jadavani 2020-03-25 922 * ufshcd_scale_clks - scale up or scale down UFS controller clocks
a67142dac16542 Subhash Jadavani 2020-03-25 923 * @hba: per adapter instance
a67142dac16542 Subhash Jadavani 2020-03-25 924 * @scale_up: True if scaling up and false if scaling down
a67142dac16542 Subhash Jadavani 2020-03-25 925 *
a67142dac16542 Subhash Jadavani 2020-03-25 926 * Returns 0 if successful
a67142dac16542 Subhash Jadavani 2020-03-25 927 * Returns < 0 for any other errors
a67142dac16542 Subhash Jadavani 2020-03-25 928 */
a67142dac16542 Subhash Jadavani 2020-03-25 929 static int ufshcd_scale_clks(struct ufs_hba *hba, bool scale_up)
a67142dac16542 Subhash Jadavani 2020-03-25 930 {
a67142dac16542 Subhash Jadavani 2020-03-25 931 int ret = 0;
a67142dac16542 Subhash Jadavani 2020-03-25 932
a67142dac16542 Subhash Jadavani 2020-03-25 933 ret = ufshcd_vops_clk_scale_notify(hba, scale_up, PRE_CHANGE);
a67142dac16542 Subhash Jadavani 2020-03-25 934 if (ret)
a67142dac16542 Subhash Jadavani 2020-03-25 935 return ret;
a67142dac16542 Subhash Jadavani 2020-03-25 936
a67142dac16542 Subhash Jadavani 2020-03-25 937 ret = ufshcd_set_clk_freq(hba, scale_up);
a67142dac16542 Subhash Jadavani 2020-03-25 938 if (ret)
a67142dac16542 Subhash Jadavani 2020-03-25 939 return ret;
a67142dac16542 Subhash Jadavani 2020-03-25 940
a3cd5ec55f6c72 Subhash Jadavani 2017-02-03 941 ret = ufshcd_vops_clk_scale_notify(hba, scale_up, POST_CHANGE);
a67142dac16542 Subhash Jadavani 2020-03-25 942 if (ret) {
a67142dac16542 Subhash Jadavani 2020-03-25 943 ufshcd_set_clk_freq(hba, !scale_up);
a67142dac16542 Subhash Jadavani 2020-03-25 944 return ret;
a67142dac16542 Subhash Jadavani 2020-03-25 945 }
a3cd5ec55f6c72 Subhash Jadavani 2017-02-03 946
a3cd5ec55f6c72 Subhash Jadavani 2017-02-03 947 trace_ufshcd_profile_clk_scaling(dev_name(hba->dev),
a3cd5ec55f6c72 Subhash Jadavani 2017-02-03 948 (scale_up ? "up" : "down"),
a3cd5ec55f6c72 Subhash Jadavani 2017-02-03 @949 ktime_to_us(ktime_sub(ktime_get(), start)), ret);
a67142dac16542 Subhash Jadavani 2020-03-25 950
a3cd5ec55f6c72 Subhash Jadavani 2017-02-03 951 return ret;
a3cd5ec55f6c72 Subhash Jadavani 2017-02-03 952 }
a3cd5ec55f6c72 Subhash Jadavani 2017-02-03 953

:::::: The code at line 949 was first introduced by commit
:::::: a3cd5ec55f6c72834f812f9150deb38ddc019782 scsi: ufs: add load based scaling of UFS gear

:::::: TO: [email protected] <[email protected]>
:::::: CC: Martin K. Petersen <[email protected]>

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]


Attachments:
(No filename) (5.45 kB)
.config.gz (59.73 kB)
Download all attachments