* [PATCH v2 0/4] scsi: ufs: Cleanup and refactor clock scaling
@ 2020-12-16 13:16 Stanley Chu
2020-12-16 13:16 ` [PATCH v2 1/4] scsi: ufs: Refactor cancelling clkscaling works Stanley Chu
` (5 more replies)
0 siblings, 6 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
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.
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(-)
--
2.18.0
^ permalink raw reply [flat|nested] 14+ messages in thread
* [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
* [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
* [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
* [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 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 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
* 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
* 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
* 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
end of thread, other threads:[~2021-01-06 3:00 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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-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
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
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
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
2020-12-18 6:24 ` Stanley Chu
2020-12-18 6:30 ` Can Guo
2021-01-06 2:47 ` Martin K. Petersen
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).