* [PATCH v2 1/4] scsi: ufs: Refactor cancelling clkscaling works
2020-12-16 13:16 [PATCH v2 0/4] scsi: ufs: Cleanup and refactor clock scaling Stanley Chu
@ 2020-12-16 13:16 ` Stanley Chu
2020-12-18 6:08 ` Can Guo
2020-12-16 13:16 ` [PATCH v2 2/4] scsi: ufs: Remove redundant null checking of devfreq instance Stanley Chu
` (4 subsequent siblings)
5 siblings, 1 reply; 14+ messages in thread
From: Stanley Chu @ 2020-12-16 13:16 UTC (permalink / raw)
To: linux-scsi, martin.petersen, avri.altman, alim.akhtar, jejb
Cc: beanhuo, asutoshd, cang, matthias.bgg, bvanassche,
linux-mediatek, linux-arm-kernel, linux-kernel, kuohong.wang,
peter.wang, chun-hung.wu, andy.teng, chaotian.jing, cc.chou,
jiajie.hao, alice.chao, Stanley Chu
Cancelling suspend_work and resume_work is only required while
suspending clk-scaling. Thus moving these two invokes into
ufshcd_suspend_clkscaling() function.
Signed-off-by: Stanley Chu <stanley.chu@mediatek.com>
---
drivers/scsi/ufs/ufshcd.c | 17 ++++++-----------
1 file changed, 6 insertions(+), 11 deletions(-)
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 052479a56a6f..a91b73a1fc48 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -1451,6 +1451,9 @@ static void ufshcd_suspend_clkscaling(struct ufs_hba *hba)
if (!ufshcd_is_clkscaling_supported(hba))
return;
+ cancel_work_sync(&hba->clk_scaling.suspend_work);
+ cancel_work_sync(&hba->clk_scaling.resume_work);
+
spin_lock_irqsave(hba->host->host_lock, flags);
if (!hba->clk_scaling.is_suspended) {
suspend = true;
@@ -1514,9 +1517,6 @@ static ssize_t ufshcd_clkscale_enable_store(struct device *dev,
pm_runtime_get_sync(hba->dev);
ufshcd_hold(hba, false);
- cancel_work_sync(&hba->clk_scaling.suspend_work);
- cancel_work_sync(&hba->clk_scaling.resume_work);
-
if (value) {
ufshcd_resume_clkscaling(hba);
} else {
@@ -5663,11 +5663,8 @@ static void ufshcd_err_handling_prepare(struct ufs_hba *hba)
ufshcd_vops_resume(hba, UFS_RUNTIME_PM);
} else {
ufshcd_hold(hba, false);
- if (hba->clk_scaling.is_enabled) {
- cancel_work_sync(&hba->clk_scaling.suspend_work);
- cancel_work_sync(&hba->clk_scaling.resume_work);
+ if (hba->clk_scaling.is_enabled)
ufshcd_suspend_clkscaling(hba);
- }
}
down_write(&hba->clk_scaling_lock);
hba->clk_scaling.is_allowed = false;
@@ -8512,11 +8509,9 @@ static int ufshcd_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op)
ufshcd_hold(hba, false);
hba->clk_gating.is_suspended = true;
- if (hba->clk_scaling.is_enabled) {
- cancel_work_sync(&hba->clk_scaling.suspend_work);
- cancel_work_sync(&hba->clk_scaling.resume_work);
+ if (hba->clk_scaling.is_enabled)
ufshcd_suspend_clkscaling(hba);
- }
+
down_write(&hba->clk_scaling_lock);
hba->clk_scaling.is_allowed = false;
up_write(&hba->clk_scaling_lock);
--
2.18.0
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v2 1/4] scsi: ufs: Refactor cancelling clkscaling works
2020-12-16 13:16 ` [PATCH v2 1/4] scsi: ufs: Refactor cancelling clkscaling works Stanley Chu
@ 2020-12-18 6:08 ` Can Guo
0 siblings, 0 replies; 14+ messages in thread
From: Can Guo @ 2020-12-18 6:08 UTC (permalink / raw)
To: Stanley Chu
Cc: linux-scsi, martin.petersen, avri.altman, alim.akhtar, jejb,
beanhuo, asutoshd, matthias.bgg, bvanassche, linux-mediatek,
linux-arm-kernel, linux-kernel, kuohong.wang, peter.wang,
chun-hung.wu, andy.teng, chaotian.jing, cc.chou, jiajie.hao,
alice.chao
On 2020-12-16 21:16, Stanley Chu wrote:
> Cancelling suspend_work and resume_work is only required while
> suspending clk-scaling. Thus moving these two invokes into
> ufshcd_suspend_clkscaling() function.
>
> Signed-off-by: Stanley Chu <stanley.chu@mediatek.com>
> ---
> drivers/scsi/ufs/ufshcd.c | 17 ++++++-----------
> 1 file changed, 6 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index 052479a56a6f..a91b73a1fc48 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -1451,6 +1451,9 @@ static void ufshcd_suspend_clkscaling(struct
> ufs_hba *hba)
> if (!ufshcd_is_clkscaling_supported(hba))
> return;
>
> + cancel_work_sync(&hba->clk_scaling.suspend_work);
> + cancel_work_sync(&hba->clk_scaling.resume_work);
> +
> spin_lock_irqsave(hba->host->host_lock, flags);
> if (!hba->clk_scaling.is_suspended) {
> suspend = true;
> @@ -1514,9 +1517,6 @@ static ssize_t
> ufshcd_clkscale_enable_store(struct device *dev,
> pm_runtime_get_sync(hba->dev);
> ufshcd_hold(hba, false);
>
> - cancel_work_sync(&hba->clk_scaling.suspend_work);
> - cancel_work_sync(&hba->clk_scaling.resume_work);
> -
> if (value) {
> ufshcd_resume_clkscaling(hba);
> } else {
> @@ -5663,11 +5663,8 @@ static void ufshcd_err_handling_prepare(struct
> ufs_hba *hba)
> ufshcd_vops_resume(hba, UFS_RUNTIME_PM);
> } else {
> ufshcd_hold(hba, false);
> - if (hba->clk_scaling.is_enabled) {
> - cancel_work_sync(&hba->clk_scaling.suspend_work);
> - cancel_work_sync(&hba->clk_scaling.resume_work);
> + if (hba->clk_scaling.is_enabled)
> ufshcd_suspend_clkscaling(hba);
> - }
> }
> down_write(&hba->clk_scaling_lock);
> hba->clk_scaling.is_allowed = false;
> @@ -8512,11 +8509,9 @@ static int ufshcd_suspend(struct ufs_hba *hba,
> enum ufs_pm_op pm_op)
> ufshcd_hold(hba, false);
> hba->clk_gating.is_suspended = true;
>
> - if (hba->clk_scaling.is_enabled) {
> - cancel_work_sync(&hba->clk_scaling.suspend_work);
> - cancel_work_sync(&hba->clk_scaling.resume_work);
> + if (hba->clk_scaling.is_enabled)
> ufshcd_suspend_clkscaling(hba);
> - }
> +
> down_write(&hba->clk_scaling_lock);
> hba->clk_scaling.is_allowed = false;
> up_write(&hba->clk_scaling_lock);
Reviewed-by: Can Guo <cang@codeaurora.org>
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v2 2/4] scsi: ufs: Remove redundant null checking of devfreq instance
2020-12-16 13:16 [PATCH v2 0/4] scsi: ufs: Cleanup and refactor clock scaling Stanley Chu
2020-12-16 13:16 ` [PATCH v2 1/4] scsi: ufs: Refactor cancelling clkscaling works Stanley Chu
@ 2020-12-16 13:16 ` Stanley Chu
2020-12-18 5:58 ` Can Guo
2020-12-18 6:09 ` Can Guo
2020-12-16 13:16 ` [PATCH v2 3/4] scsi: ufs: Cleanup and refactor clk-scaling feature Stanley Chu
` (3 subsequent siblings)
5 siblings, 2 replies; 14+ messages in thread
From: Stanley Chu @ 2020-12-16 13:16 UTC (permalink / raw)
To: linux-scsi, martin.petersen, avri.altman, alim.akhtar, jejb
Cc: beanhuo, asutoshd, cang, matthias.bgg, bvanassche,
linux-mediatek, linux-arm-kernel, linux-kernel, kuohong.wang,
peter.wang, chun-hung.wu, andy.teng, chaotian.jing, cc.chou,
jiajie.hao, alice.chao, Stanley Chu
hba->devfreq is zero-initialized thus it is not required
to check its existence in ufshcd_add_lus() function which
is invoked during initialization only.
Signed-off-by: Stanley Chu <stanley.chu@mediatek.com>
---
drivers/scsi/ufs/ufshcd.c | 14 ++++++--------
1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index a91b73a1fc48..9cc16598136d 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -7636,15 +7636,13 @@ static int ufshcd_add_lus(struct ufs_hba *hba)
&hba->pwr_info,
sizeof(struct ufs_pa_layer_attr));
hba->clk_scaling.saved_pwr_info.is_valid = true;
- if (!hba->devfreq) {
- hba->clk_scaling.is_allowed = true;
- ret = ufshcd_devfreq_init(hba);
- if (ret)
- goto out;
+ hba->clk_scaling.is_allowed = true;
+ ret = ufshcd_devfreq_init(hba);
+ if (ret)
+ goto out;
- hba->clk_scaling.is_enabled = true;
- ufshcd_clkscaling_init_sysfs(hba);
- }
+ hba->clk_scaling.is_enabled = true;
+ ufshcd_clkscaling_init_sysfs(hba);
}
ufs_bsg_probe(hba);
--
2.18.0
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v2 2/4] scsi: ufs: Remove redundant null checking of devfreq instance
2020-12-16 13:16 ` [PATCH v2 2/4] scsi: ufs: Remove redundant null checking of devfreq instance Stanley Chu
@ 2020-12-18 5:58 ` Can Guo
2020-12-18 6:09 ` Can Guo
1 sibling, 0 replies; 14+ messages in thread
From: Can Guo @ 2020-12-18 5:58 UTC (permalink / raw)
To: Stanley Chu
Cc: linux-scsi, martin.petersen, avri.altman, alim.akhtar, jejb,
beanhuo, asutoshd, matthias.bgg, bvanassche, linux-mediatek,
linux-arm-kernel, linux-kernel, kuohong.wang, peter.wang,
chun-hung.wu, andy.teng, chaotian.jing, cc.chou, jiajie.hao,
alice.chao
On 2020-12-16 21:16, Stanley Chu wrote:
> hba->devfreq is zero-initialized thus it is not required
> to check its existence in ufshcd_add_lus() function which
> is invoked during initialization only.
>
> Signed-off-by: Stanley Chu <stanley.chu@mediatek.com>
> ---
> drivers/scsi/ufs/ufshcd.c | 14 ++++++--------
> 1 file changed, 6 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index a91b73a1fc48..9cc16598136d 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -7636,15 +7636,13 @@ static int ufshcd_add_lus(struct ufs_hba *hba)
> &hba->pwr_info,
> sizeof(struct ufs_pa_layer_attr));
> hba->clk_scaling.saved_pwr_info.is_valid = true;
> - if (!hba->devfreq) {
Just FYI, checking this was due to consideration for supporting UFS
cards
which can do hot-plug. But UFS cards seems out of the picture for a long
time.
Anyways, if someday UFS cards come back in future, let's add these
checks back again.
Thanks,
Can Guo.
> - hba->clk_scaling.is_allowed = true;
> - ret = ufshcd_devfreq_init(hba);
> - if (ret)
> - goto out;
> + hba->clk_scaling.is_allowed = true;
> + ret = ufshcd_devfreq_init(hba);
> + if (ret)
> + goto out;
>
> - hba->clk_scaling.is_enabled = true;
> - ufshcd_clkscaling_init_sysfs(hba);
> - }
> + hba->clk_scaling.is_enabled = true;
> + ufshcd_clkscaling_init_sysfs(hba);
> }
>
> ufs_bsg_probe(hba);
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 2/4] scsi: ufs: Remove redundant null checking of devfreq instance
2020-12-16 13:16 ` [PATCH v2 2/4] scsi: ufs: Remove redundant null checking of devfreq instance Stanley Chu
2020-12-18 5:58 ` Can Guo
@ 2020-12-18 6:09 ` Can Guo
1 sibling, 0 replies; 14+ messages in thread
From: Can Guo @ 2020-12-18 6:09 UTC (permalink / raw)
To: Stanley Chu
Cc: linux-scsi, martin.petersen, avri.altman, alim.akhtar, jejb,
beanhuo, asutoshd, matthias.bgg, bvanassche, linux-mediatek,
linux-arm-kernel, linux-kernel, kuohong.wang, peter.wang,
chun-hung.wu, andy.teng, chaotian.jing, cc.chou, jiajie.hao,
alice.chao
On 2020-12-16 21:16, Stanley Chu wrote:
> hba->devfreq is zero-initialized thus it is not required
> to check its existence in ufshcd_add_lus() function which
> is invoked during initialization only.
>
> Signed-off-by: Stanley Chu <stanley.chu@mediatek.com>
> ---
> drivers/scsi/ufs/ufshcd.c | 14 ++++++--------
> 1 file changed, 6 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index a91b73a1fc48..9cc16598136d 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -7636,15 +7636,13 @@ static int ufshcd_add_lus(struct ufs_hba *hba)
> &hba->pwr_info,
> sizeof(struct ufs_pa_layer_attr));
> hba->clk_scaling.saved_pwr_info.is_valid = true;
> - if (!hba->devfreq) {
> - hba->clk_scaling.is_allowed = true;
> - ret = ufshcd_devfreq_init(hba);
> - if (ret)
> - goto out;
> + hba->clk_scaling.is_allowed = true;
> + ret = ufshcd_devfreq_init(hba);
> + if (ret)
> + goto out;
>
> - hba->clk_scaling.is_enabled = true;
> - ufshcd_clkscaling_init_sysfs(hba);
> - }
> + hba->clk_scaling.is_enabled = true;
> + ufshcd_clkscaling_init_sysfs(hba);
> }
>
> ufs_bsg_probe(hba);
Reviewed-by: Can Guo <cang@codeaurora.org>
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v2 3/4] scsi: ufs: Cleanup and refactor clk-scaling feature
2020-12-16 13:16 [PATCH v2 0/4] scsi: ufs: Cleanup and refactor clock scaling Stanley Chu
2020-12-16 13:16 ` [PATCH v2 1/4] scsi: ufs: Refactor cancelling clkscaling works Stanley Chu
2020-12-16 13:16 ` [PATCH v2 2/4] scsi: ufs: Remove redundant null checking of devfreq instance Stanley Chu
@ 2020-12-16 13:16 ` Stanley Chu
2020-12-18 6:16 ` Can Guo
2020-12-16 13:16 ` [PATCH v2 4/4] scsi: ufs: Fix build warning by incorrect function description Stanley Chu
` (2 subsequent siblings)
5 siblings, 1 reply; 14+ messages in thread
From: Stanley Chu @ 2020-12-16 13:16 UTC (permalink / raw)
To: linux-scsi, martin.petersen, avri.altman, alim.akhtar, jejb
Cc: beanhuo, asutoshd, cang, matthias.bgg, bvanassche,
linux-mediatek, linux-arm-kernel, linux-kernel, kuohong.wang,
peter.wang, chun-hung.wu, andy.teng, chaotian.jing, cc.chou,
jiajie.hao, alice.chao, Stanley Chu
Manipulate clock scaling related stuff only if the host capability
supports clock scaling feature to avoid redundant code execution.
Signed-off-by: Stanley Chu <stanley.chu@mediatek.com>
---
drivers/scsi/ufs/ufshcd.c | 64 ++++++++++++++++++++-------------------
1 file changed, 33 insertions(+), 31 deletions(-)
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 9cc16598136d..ce0528f2e2ed 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -1448,9 +1448,6 @@ static void ufshcd_suspend_clkscaling(struct ufs_hba *hba)
unsigned long flags;
bool suspend = false;
- if (!ufshcd_is_clkscaling_supported(hba))
- return;
-
cancel_work_sync(&hba->clk_scaling.suspend_work);
cancel_work_sync(&hba->clk_scaling.resume_work);
@@ -1470,9 +1467,6 @@ static void ufshcd_resume_clkscaling(struct ufs_hba *hba)
unsigned long flags;
bool resume = false;
- if (!ufshcd_is_clkscaling_supported(hba))
- return;
-
spin_lock_irqsave(hba->host->host_lock, flags);
if (hba->clk_scaling.is_suspended) {
resume = true;
@@ -5642,6 +5636,26 @@ static inline void ufshcd_schedule_eh_work(struct ufs_hba *hba)
}
}
+static void ufshcd_clk_scaling_allow(struct ufs_hba *hba, bool allow)
+{
+ down_write(&hba->clk_scaling_lock);
+ hba->clk_scaling.is_allowed = allow;
+ up_write(&hba->clk_scaling_lock);
+}
+
+static void ufshcd_clk_scaling_suspend(struct ufs_hba *hba, bool suspend)
+{
+ if (suspend) {
+ if (hba->clk_scaling.is_enabled)
+ ufshcd_suspend_clkscaling(hba);
+ ufshcd_clk_scaling_allow(hba, false);
+ } else {
+ ufshcd_clk_scaling_allow(hba, true);
+ if (hba->clk_scaling.is_enabled)
+ ufshcd_resume_clkscaling(hba);
+ }
+}
+
static void ufshcd_err_handling_prepare(struct ufs_hba *hba)
{
pm_runtime_get_sync(hba->dev);
@@ -5663,23 +5677,20 @@ static void ufshcd_err_handling_prepare(struct ufs_hba *hba)
ufshcd_vops_resume(hba, UFS_RUNTIME_PM);
} else {
ufshcd_hold(hba, false);
- if (hba->clk_scaling.is_enabled)
+ if (ufshcd_is_clkscaling_supported(hba) &&
+ hba->clk_scaling.is_enabled)
ufshcd_suspend_clkscaling(hba);
}
- down_write(&hba->clk_scaling_lock);
- hba->clk_scaling.is_allowed = false;
- up_write(&hba->clk_scaling_lock);
+ ufshcd_clk_scaling_allow(hba, false);
}
static void ufshcd_err_handling_unprepare(struct ufs_hba *hba)
{
ufshcd_release(hba);
- down_write(&hba->clk_scaling_lock);
- hba->clk_scaling.is_allowed = true;
- up_write(&hba->clk_scaling_lock);
- if (hba->clk_scaling.is_enabled)
- ufshcd_resume_clkscaling(hba);
+ if (ufshcd_is_clkscaling_supported(hba))
+ ufshcd_clk_scaling_suspend(hba, false);
+
pm_runtime_put(hba->dev);
}
@@ -8507,12 +8518,8 @@ static int ufshcd_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op)
ufshcd_hold(hba, false);
hba->clk_gating.is_suspended = true;
- if (hba->clk_scaling.is_enabled)
- ufshcd_suspend_clkscaling(hba);
-
- down_write(&hba->clk_scaling_lock);
- hba->clk_scaling.is_allowed = false;
- up_write(&hba->clk_scaling_lock);
+ if (ufshcd_is_clkscaling_supported(hba))
+ ufshcd_clk_scaling_suspend(hba, true);
if (req_dev_pwr_mode == UFS_ACTIVE_PWR_MODE &&
req_link_state == UIC_LINK_ACTIVE_STATE) {
@@ -8618,11 +8625,9 @@ static int ufshcd_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op)
if (!ufshcd_set_dev_pwr_mode(hba, UFS_ACTIVE_PWR_MODE))
ufshcd_disable_auto_bkops(hba);
enable_gating:
- down_write(&hba->clk_scaling_lock);
- hba->clk_scaling.is_allowed = true;
- up_write(&hba->clk_scaling_lock);
- if (hba->clk_scaling.is_enabled)
- ufshcd_resume_clkscaling(hba);
+ if (ufshcd_is_clkscaling_supported(hba))
+ ufshcd_clk_scaling_suspend(hba, false);
+
hba->clk_gating.is_suspended = false;
hba->dev_info.b_rpm_dev_flush_capable = false;
ufshcd_release(hba);
@@ -8719,11 +8724,8 @@ static int ufshcd_resume(struct ufs_hba *hba, enum ufs_pm_op pm_op)
hba->clk_gating.is_suspended = false;
- down_write(&hba->clk_scaling_lock);
- hba->clk_scaling.is_allowed = true;
- up_write(&hba->clk_scaling_lock);
- if (hba->clk_scaling.is_enabled)
- ufshcd_resume_clkscaling(hba);
+ if (ufshcd_is_clkscaling_supported(hba))
+ ufshcd_clk_scaling_suspend(hba, false);
/* Enable Auto-Hibernate if configured */
ufshcd_auto_hibern8_enable(hba);
--
2.18.0
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v2 3/4] scsi: ufs: Cleanup and refactor clk-scaling feature
2020-12-16 13:16 ` [PATCH v2 3/4] scsi: ufs: Cleanup and refactor clk-scaling feature Stanley Chu
@ 2020-12-18 6:16 ` Can Guo
0 siblings, 0 replies; 14+ messages in thread
From: Can Guo @ 2020-12-18 6:16 UTC (permalink / raw)
To: Stanley Chu
Cc: linux-scsi, martin.petersen, avri.altman, alim.akhtar, jejb,
beanhuo, asutoshd, matthias.bgg, bvanassche, linux-mediatek,
linux-arm-kernel, linux-kernel, kuohong.wang, peter.wang,
chun-hung.wu, andy.teng, chaotian.jing, cc.chou, jiajie.hao,
alice.chao
On 2020-12-16 21:16, Stanley Chu wrote:
> Manipulate clock scaling related stuff only if the host capability
> supports clock scaling feature to avoid redundant code execution.
>
> Signed-off-by: Stanley Chu <stanley.chu@mediatek.com>
> ---
> drivers/scsi/ufs/ufshcd.c | 64 ++++++++++++++++++++-------------------
> 1 file changed, 33 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index 9cc16598136d..ce0528f2e2ed 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -1448,9 +1448,6 @@ static void ufshcd_suspend_clkscaling(struct
> ufs_hba *hba)
> unsigned long flags;
> bool suspend = false;
>
> - if (!ufshcd_is_clkscaling_supported(hba))
> - return;
> -
> cancel_work_sync(&hba->clk_scaling.suspend_work);
> cancel_work_sync(&hba->clk_scaling.resume_work);
>
> @@ -1470,9 +1467,6 @@ static void ufshcd_resume_clkscaling(struct
> ufs_hba *hba)
> unsigned long flags;
> bool resume = false;
>
> - if (!ufshcd_is_clkscaling_supported(hba))
> - return;
> -
> spin_lock_irqsave(hba->host->host_lock, flags);
> if (hba->clk_scaling.is_suspended) {
> resume = true;
> @@ -5642,6 +5636,26 @@ static inline void
> ufshcd_schedule_eh_work(struct ufs_hba *hba)
> }
> }
>
> +static void ufshcd_clk_scaling_allow(struct ufs_hba *hba, bool allow)
> +{
> + down_write(&hba->clk_scaling_lock);
> + hba->clk_scaling.is_allowed = allow;
> + up_write(&hba->clk_scaling_lock);
> +}
> +
> +static void ufshcd_clk_scaling_suspend(struct ufs_hba *hba, bool
> suspend)
> +{
> + if (suspend) {
> + if (hba->clk_scaling.is_enabled)
> + ufshcd_suspend_clkscaling(hba);
> + ufshcd_clk_scaling_allow(hba, false);
> + } else {
> + ufshcd_clk_scaling_allow(hba, true);
> + if (hba->clk_scaling.is_enabled)
> + ufshcd_resume_clkscaling(hba);
> + }
> +}
> +
> static void ufshcd_err_handling_prepare(struct ufs_hba *hba)
> {
> pm_runtime_get_sync(hba->dev);
> @@ -5663,23 +5677,20 @@ static void ufshcd_err_handling_prepare(struct
> ufs_hba *hba)
> ufshcd_vops_resume(hba, UFS_RUNTIME_PM);
> } else {
> ufshcd_hold(hba, false);
> - if (hba->clk_scaling.is_enabled)
> + if (ufshcd_is_clkscaling_supported(hba) &&
> + hba->clk_scaling.is_enabled)
> ufshcd_suspend_clkscaling(hba);
> }
> - down_write(&hba->clk_scaling_lock);
> - hba->clk_scaling.is_allowed = false;
> - up_write(&hba->clk_scaling_lock);
> + ufshcd_clk_scaling_allow(hba, false);
> }
>
> static void ufshcd_err_handling_unprepare(struct ufs_hba *hba)
> {
> ufshcd_release(hba);
>
> - down_write(&hba->clk_scaling_lock);
> - hba->clk_scaling.is_allowed = true;
> - up_write(&hba->clk_scaling_lock);
> - if (hba->clk_scaling.is_enabled)
> - ufshcd_resume_clkscaling(hba);
> + if (ufshcd_is_clkscaling_supported(hba))
> + ufshcd_clk_scaling_suspend(hba, false);
> +
> pm_runtime_put(hba->dev);
> }
>
> @@ -8507,12 +8518,8 @@ static int ufshcd_suspend(struct ufs_hba *hba,
> enum ufs_pm_op pm_op)
> ufshcd_hold(hba, false);
> hba->clk_gating.is_suspended = true;
>
> - if (hba->clk_scaling.is_enabled)
> - ufshcd_suspend_clkscaling(hba);
> -
> - down_write(&hba->clk_scaling_lock);
> - hba->clk_scaling.is_allowed = false;
> - up_write(&hba->clk_scaling_lock);
> + if (ufshcd_is_clkscaling_supported(hba))
> + ufshcd_clk_scaling_suspend(hba, true);
>
> if (req_dev_pwr_mode == UFS_ACTIVE_PWR_MODE &&
> req_link_state == UIC_LINK_ACTIVE_STATE) {
> @@ -8618,11 +8625,9 @@ static int ufshcd_suspend(struct ufs_hba *hba,
> enum ufs_pm_op pm_op)
> if (!ufshcd_set_dev_pwr_mode(hba, UFS_ACTIVE_PWR_MODE))
> ufshcd_disable_auto_bkops(hba);
> enable_gating:
> - down_write(&hba->clk_scaling_lock);
> - hba->clk_scaling.is_allowed = true;
> - up_write(&hba->clk_scaling_lock);
> - if (hba->clk_scaling.is_enabled)
> - ufshcd_resume_clkscaling(hba);
> + if (ufshcd_is_clkscaling_supported(hba))
> + ufshcd_clk_scaling_suspend(hba, false);
> +
> hba->clk_gating.is_suspended = false;
> hba->dev_info.b_rpm_dev_flush_capable = false;
> ufshcd_release(hba);
> @@ -8719,11 +8724,8 @@ static int ufshcd_resume(struct ufs_hba *hba,
> enum ufs_pm_op pm_op)
>
> hba->clk_gating.is_suspended = false;
>
> - down_write(&hba->clk_scaling_lock);
> - hba->clk_scaling.is_allowed = true;
> - up_write(&hba->clk_scaling_lock);
> - if (hba->clk_scaling.is_enabled)
> - ufshcd_resume_clkscaling(hba);
> + if (ufshcd_is_clkscaling_supported(hba))
> + ufshcd_clk_scaling_suspend(hba, false);
>
> /* Enable Auto-Hibernate if configured */
> ufshcd_auto_hibern8_enable(hba);
Thanks for the cleanup
Reviewed-by: Can Guo <cang@codeaurora.org>
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v2 4/4] scsi: ufs: Fix build warning by incorrect function description
2020-12-16 13:16 [PATCH v2 0/4] scsi: ufs: Cleanup and refactor clock scaling Stanley Chu
` (2 preceding siblings ...)
2020-12-16 13:16 ` [PATCH v2 3/4] scsi: ufs: Cleanup and refactor clk-scaling feature Stanley Chu
@ 2020-12-16 13:16 ` Stanley Chu
2020-12-18 6:16 ` Can Guo
2020-12-18 6:20 ` [PATCH v2 0/4] scsi: ufs: Cleanup and refactor clock scaling Can Guo
2021-01-06 2:47 ` Martin K. Petersen
5 siblings, 1 reply; 14+ messages in thread
From: Stanley Chu @ 2020-12-16 13:16 UTC (permalink / raw)
To: linux-scsi, martin.petersen, avri.altman, alim.akhtar, jejb
Cc: beanhuo, asutoshd, cang, matthias.bgg, bvanassche,
linux-mediatek, linux-arm-kernel, linux-kernel, kuohong.wang,
peter.wang, chun-hung.wu, andy.teng, chaotian.jing, cc.chou,
jiajie.hao, alice.chao, Stanley Chu
Fix build warnings as below due to incorrect function description
of ufshcd_try_to_abort_task().
ufshcd.c:6651: warning: Function parameter or member 'hba' not described in 'ufshcd_try_to_abort_task'
ufshcd.c:6651: warning: Function parameter or member 'tag' not described in 'ufshcd_try_to_abort_task'
ufshcd.c:6651: warning: Excess function parameter 'cmd' description in 'ufshcd_try_to_abort_task'
Signed-off-by: Stanley Chu <stanley.chu@mediatek.com>
---
drivers/scsi/ufs/ufshcd.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index ce0528f2e2ed..79287fdd049b 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -6652,7 +6652,8 @@ static void ufshcd_set_req_abort_skip(struct ufs_hba *hba, unsigned long bitmap)
/**
* ufshcd_try_to_abort_task - abort a specific task
- * @cmd: SCSI command pointer
+ * @hba: per adapter instance
+ * @tag: position of the bit to be aborted
*
* Abort the pending command in device by sending UFS_ABORT_TASK task management
* command, and in host controller by clearing the door-bell register. There can
--
2.18.0
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v2 4/4] scsi: ufs: Fix build warning by incorrect function description
2020-12-16 13:16 ` [PATCH v2 4/4] scsi: ufs: Fix build warning by incorrect function description Stanley Chu
@ 2020-12-18 6:16 ` Can Guo
0 siblings, 0 replies; 14+ messages in thread
From: Can Guo @ 2020-12-18 6:16 UTC (permalink / raw)
To: Stanley Chu
Cc: linux-scsi, martin.petersen, avri.altman, alim.akhtar, jejb,
beanhuo, asutoshd, matthias.bgg, bvanassche, linux-mediatek,
linux-arm-kernel, linux-kernel, kuohong.wang, peter.wang,
chun-hung.wu, andy.teng, chaotian.jing, cc.chou, jiajie.hao,
alice.chao
On 2020-12-16 21:16, Stanley Chu wrote:
> Fix build warnings as below due to incorrect function description
> of ufshcd_try_to_abort_task().
>
> ufshcd.c:6651: warning: Function parameter or member 'hba' not
> described in 'ufshcd_try_to_abort_task'
> ufshcd.c:6651: warning: Function parameter or member 'tag' not
> described in 'ufshcd_try_to_abort_task'
> ufshcd.c:6651: warning: Excess function parameter 'cmd' description in
> 'ufshcd_try_to_abort_task'
>
> Signed-off-by: Stanley Chu <stanley.chu@mediatek.com>
> ---
> drivers/scsi/ufs/ufshcd.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index ce0528f2e2ed..79287fdd049b 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -6652,7 +6652,8 @@ static void ufshcd_set_req_abort_skip(struct
> ufs_hba *hba, unsigned long bitmap)
>
> /**
> * ufshcd_try_to_abort_task - abort a specific task
> - * @cmd: SCSI command pointer
> + * @hba: per adapter instance
> + * @tag: position of the bit to be aborted
> *
> * Abort the pending command in device by sending UFS_ABORT_TASK task
> management
> * command, and in host controller by clearing the door-bell
> register. There can
Thanks for the fix.
Reviewed-by: Can Guo <cang@codeaurora.org>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 0/4] scsi: ufs: Cleanup and refactor clock scaling
2020-12-16 13:16 [PATCH v2 0/4] scsi: ufs: Cleanup and refactor clock scaling Stanley Chu
` (3 preceding siblings ...)
2020-12-16 13:16 ` [PATCH v2 4/4] scsi: ufs: Fix build warning by incorrect function description Stanley Chu
@ 2020-12-18 6:20 ` Can Guo
2020-12-18 6:24 ` Stanley Chu
2021-01-06 2:47 ` Martin K. Petersen
5 siblings, 1 reply; 14+ messages in thread
From: Can Guo @ 2020-12-18 6:20 UTC (permalink / raw)
To: Stanley Chu
Cc: linux-scsi, martin.petersen, avri.altman, alim.akhtar, jejb,
beanhuo, asutoshd, matthias.bgg, bvanassche, linux-mediatek,
linux-arm-kernel, linux-kernel, kuohong.wang, peter.wang,
chun-hung.wu, andy.teng, chaotian.jing, cc.chou, jiajie.hao,
alice.chao
On 2020-12-16 21:16, Stanley Chu wrote:
> Hi,
> This series cleans up and refactors clk-scaling feature, and shall not
> change any functionality.
>
> This series is based on Can's series "Three changes related with UFS
> clock scaling" in 5.10/scsi-fixes branch in Martin's tree.
>
Hi Stanley,
Thanks for noticing my changes, will you review them?
I see customers manipulte UFS scaling related sysfs
nodes more often than before, so we may want to fix it asap.
Regards,
Can Guo.
> However this series may not be required to be merged to 5.10. The
> choice of base branch is simply making these patches easy to be
> reviewed because this series is based on clk-scaling fixes by Can. If
> this series is decided not being merged to 5.10, then I would rebase
> it to 5.11/scsi-queue.
>
> Changes since v1:
> - Refactor ufshcd_clk_scaling_suspend() in patch [3/4]
> - Change function name from ufshcd_clk_scaling_pm() to
> ufshcd_clk_scaling_suspend() in patch [3/4]
> - Refine patch titles
>
> Stanley Chu (4):
> scsi: ufs: Refactor cancelling clkscaling works
> scsi: ufs: Remove redundant null checking of devfreq instance
> scsi: ufs: Cleanup and refactor clk-scaling feature
> scsi: ufs: Fix build warning by incorrect function description
>
> drivers/scsi/ufs/ufshcd.c | 90 +++++++++++++++++++--------------------
> 1 file changed, 43 insertions(+), 47 deletions(-)
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 0/4] scsi: ufs: Cleanup and refactor clock scaling
2020-12-18 6:20 ` [PATCH v2 0/4] scsi: ufs: Cleanup and refactor clock scaling Can Guo
@ 2020-12-18 6:24 ` Stanley Chu
2020-12-18 6:30 ` Can Guo
0 siblings, 1 reply; 14+ messages in thread
From: Stanley Chu @ 2020-12-18 6:24 UTC (permalink / raw)
To: Can Guo
Cc: linux-scsi, martin.petersen, avri.altman, alim.akhtar, jejb,
beanhuo, asutoshd, matthias.bgg, bvanassche, linux-mediatek,
linux-arm-kernel, linux-kernel, kuohong.wang, peter.wang,
chun-hung.wu, andy.teng, chaotian.jing, cc.chou, jiajie.hao,
alice.chao
Hi Can,
On Fri, 2020-12-18 at 14:20 +0800, Can Guo wrote:
> On 2020-12-16 21:16, Stanley Chu wrote:
> > Hi,
> > This series cleans up and refactors clk-scaling feature, and shall not
> > change any functionality.
> >
> > This series is based on Can's series "Three changes related with UFS
> > clock scaling" in 5.10/scsi-fixes branch in Martin's tree.
> >
>
> Hi Stanley,
>
> Thanks for noticing my changes, will you review them?
> I see customers manipulte UFS scaling related sysfs
> nodes more often than before, so we may want to fix it asap.
I have gave my review tag in all patches in this series : )
Thanks,
Stanley Chu
>
> Regards,
>
> Can Guo.
>
> > However this series may not be required to be merged to 5.10. The
> > choice of base branch is simply making these patches easy to be
> > reviewed because this series is based on clk-scaling fixes by Can. If
> > this series is decided not being merged to 5.10, then I would rebase
> > it to 5.11/scsi-queue.
> >
> > Changes since v1:
> > - Refactor ufshcd_clk_scaling_suspend() in patch [3/4]
> > - Change function name from ufshcd_clk_scaling_pm() to
> > ufshcd_clk_scaling_suspend() in patch [3/4]
> > - Refine patch titles
> >
> > Stanley Chu (4):
> > scsi: ufs: Refactor cancelling clkscaling works
> > scsi: ufs: Remove redundant null checking of devfreq instance
> > scsi: ufs: Cleanup and refactor clk-scaling feature
> > scsi: ufs: Fix build warning by incorrect function description
> >
> > drivers/scsi/ufs/ufshcd.c | 90 +++++++++++++++++++--------------------
> > 1 file changed, 43 insertions(+), 47 deletions(-)
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 0/4] scsi: ufs: Cleanup and refactor clock scaling
2020-12-18 6:24 ` Stanley Chu
@ 2020-12-18 6:30 ` Can Guo
0 siblings, 0 replies; 14+ messages in thread
From: Can Guo @ 2020-12-18 6:30 UTC (permalink / raw)
To: Stanley Chu
Cc: linux-scsi, martin.petersen, avri.altman, alim.akhtar, jejb,
beanhuo, asutoshd, matthias.bgg, bvanassche, linux-mediatek,
linux-arm-kernel, linux-kernel, kuohong.wang, peter.wang,
chun-hung.wu, andy.teng, chaotian.jing, cc.chou, jiajie.hao,
alice.chao
On 2020-12-18 14:24, Stanley Chu wrote:
> Hi Can,
>
> On Fri, 2020-12-18 at 14:20 +0800, Can Guo wrote:
>> On 2020-12-16 21:16, Stanley Chu wrote:
>> > Hi,
>> > This series cleans up and refactors clk-scaling feature, and shall not
>> > change any functionality.
>> >
>> > This series is based on Can's series "Three changes related with UFS
>> > clock scaling" in 5.10/scsi-fixes branch in Martin's tree.
>> >
>>
>> Hi Stanley,
>>
>> Thanks for noticing my changes, will you review them?
>> I see customers manipulte UFS scaling related sysfs
>> nodes more often than before, so we may want to fix it asap.
>
> I have gave my review tag in all patches in this series : )
>
> Thanks,
> Stanley Chu
>
Hi Stanley,
oops, just saw it - I opened the wrong series.
I will push a new version which incorporates your comments soon.
Since the new changes would only be some words in ufshcd.h, so you
won't need to rebase.
Thanks,
Can Guo.
>>
>> Regards,
>>
>> Can Guo.
>>
>> > However this series may not be required to be merged to 5.10. The
>> > choice of base branch is simply making these patches easy to be
>> > reviewed because this series is based on clk-scaling fixes by Can. If
>> > this series is decided not being merged to 5.10, then I would rebase
>> > it to 5.11/scsi-queue.
>> >
>> > Changes since v1:
>> > - Refactor ufshcd_clk_scaling_suspend() in patch [3/4]
>> > - Change function name from ufshcd_clk_scaling_pm() to
>> > ufshcd_clk_scaling_suspend() in patch [3/4]
>> > - Refine patch titles
>> >
>> > Stanley Chu (4):
>> > scsi: ufs: Refactor cancelling clkscaling works
>> > scsi: ufs: Remove redundant null checking of devfreq instance
>> > scsi: ufs: Cleanup and refactor clk-scaling feature
>> > scsi: ufs: Fix build warning by incorrect function description
>> >
>> > drivers/scsi/ufs/ufshcd.c | 90 +++++++++++++++++++--------------------
>> > 1 file changed, 43 insertions(+), 47 deletions(-)
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 0/4] scsi: ufs: Cleanup and refactor clock scaling
2020-12-16 13:16 [PATCH v2 0/4] scsi: ufs: Cleanup and refactor clock scaling Stanley Chu
` (4 preceding siblings ...)
2020-12-18 6:20 ` [PATCH v2 0/4] scsi: ufs: Cleanup and refactor clock scaling Can Guo
@ 2021-01-06 2:47 ` Martin K. Petersen
5 siblings, 0 replies; 14+ messages in thread
From: Martin K. Petersen @ 2021-01-06 2:47 UTC (permalink / raw)
To: Stanley Chu
Cc: linux-scsi, martin.petersen, avri.altman, alim.akhtar, jejb,
beanhuo, asutoshd, cang, matthias.bgg, bvanassche,
linux-mediatek, linux-arm-kernel, linux-kernel, kuohong.wang,
peter.wang, chun-hung.wu, andy.teng, chaotian.jing, cc.chou,
jiajie.hao, alice.chao
Stanley,
> However this series may not be required to be merged to 5.10. The
> choice of base branch is simply making these patches easy to be
> reviewed because this series is based on clk-scaling fixes by Can.
Please redo your series on top of 5.12/scsi-queue + Can's changes.
Thanks!
--
Martin K. Petersen Oracle Linux Engineering
^ permalink raw reply [flat|nested] 14+ messages in thread