[PATCH v2 0/2] scsi: ufs: Fix power drain and hci quirk for WriteBooster

[PATCH v2 0/2] scsi: ufs: Fix power drain and hci quirk for WriteBooster

[Stanley Chu]

Hi,
This series fixes two WriteBooster issues,
1. Fix a corner case that device flush capability is not disabled during system suspend
2. Fix the checking of UFSHCI quirk UFSHCI_QUIRK_SKIP_MANUAL_WB_FLUSH_CTRL

Stanley Chu (2):
  scsi: ufs: Fix possible power drain during system suspend
  scsi: ufs: Relax the condition of
    UFSHCI_QUIRK_SKIP_MANUAL_WB_FLUSH_CTRL

 drivers/scsi/ufs/ufshcd.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

-- 
2.18.0

[PATCH v2 1/2] scsi: ufs: Fix possible power drain during system suspend

[Stanley Chu]

Currently if device needs to do flush or BKOP operations,
the device VCC power is kept during runtime-suspend period.

However, if system suspend is happening while device is
runtime-suspended, such power may not be disabled successfully.

The reasons may be,

1. If current PM level is the same as SPM level, device will
   keep runtime-suspended by ufshcd_system_suspend().

2. Flush recheck work may not be scheduled successfully
   during system suspend period. If it can wake up the system,
   this is also not the intention of the recheck work.

To fix this issue, simply runtime-resume the device if the flush
is allowed during runtime suspend period. Flush capability will be
disabled while leaving runtime suspend, and also not be allowed in
system suspend period.

Fixes: 51dd905bd2f6 ("scsi: ufs: Fix WriteBooster flush during runtime suspend")
Reviewed-by: Chaotian Jing <chaotian.jing@mediatek.com>
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 e221add25a7e..9d61dc3eb842 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -8903,7 +8903,8 @@ int ufshcd_system_suspend(struct ufs_hba *hba)
 	if ((ufs_get_pm_lvl_to_dev_pwr_mode(hba->spm_lvl) ==
 	     hba->curr_dev_pwr_mode) &&
 	    (ufs_get_pm_lvl_to_link_pwr_state(hba->spm_lvl) ==
-	     hba->uic_link_state))
+	     hba->uic_link_state) &&
+	     !hba->dev_info.b_rpm_dev_flush_capable)
 		goto out;
 
 	if (pm_runtime_suspended(hba->dev)) {
-- 
2.18.0

Re: [PATCH v2 1/2] scsi: ufs: Fix possible power drain during system suspend

[Can Guo]

On 2020-12-22 15:29, Stanley Chu wrote:
> Currently if device needs to do flush or BKOP operations,
> the device VCC power is kept during runtime-suspend period.
> 
> However, if system suspend is happening while device is
> runtime-suspended, such power may not be disabled successfully.
> 
> The reasons may be,
> 
> 1. If current PM level is the same as SPM level, device will
>    keep runtime-suspended by ufshcd_system_suspend().
> 
> 2. Flush recheck work may not be scheduled successfully
>    during system suspend period. If it can wake up the system,
>    this is also not the intention of the recheck work.
> 
> To fix this issue, simply runtime-resume the device if the flush
> is allowed during runtime suspend period. Flush capability will be
> disabled while leaving runtime suspend, and also not be allowed in
> system suspend period.
> 
> Fixes: 51dd905bd2f6 ("scsi: ufs: Fix WriteBooster flush during runtime 
> suspend")
> Reviewed-by: Chaotian Jing <chaotian.jing@mediatek.com>
> 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 e221add25a7e..9d61dc3eb842 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -8903,7 +8903,8 @@ int ufshcd_system_suspend(struct ufs_hba *hba)
>  	if ((ufs_get_pm_lvl_to_dev_pwr_mode(hba->spm_lvl) ==
>  	     hba->curr_dev_pwr_mode) &&
>  	    (ufs_get_pm_lvl_to_link_pwr_state(hba->spm_lvl) ==
> -	     hba->uic_link_state))
> +	     hba->uic_link_state) &&
> +	     !hba->dev_info.b_rpm_dev_flush_capable)
>  		goto out;
> 
>  	if (pm_runtime_suspended(hba->dev)) {

Reviewed-by: Can Guo <cang@codeaurora.org>

[PATCH v2 2/2] scsi: ufs: Relax the condition of UFSHCI_QUIRK_SKIP_MANUAL_WB_FLUSH_CTRL

[Stanley Chu]

UFSHCI_QUIRK_SKIP_MANUAL_WB_FLUSH_CTRL is aimed to skip enabling
fWriteBoosterBufferFlushEn while WriteBooster is initializing.
Therefore it is better to apply the checking during WriteBooster
initialization only.

Signed-off-by: Stanley Chu <stanley.chu@mediatek.com>
---
 drivers/scsi/ufs/ufshcd.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 9d61dc3eb842..e50b19925236 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -289,7 +289,8 @@ static inline void ufshcd_wb_config(struct ufs_hba *hba)
 	if (ret)
 		dev_err(hba->dev, "%s: En WB flush during H8: failed: %d\n",
 			__func__, ret);
-	ufshcd_wb_toggle_flush(hba, true);
+	if (!(hba->quirks & UFSHCI_QUIRK_SKIP_MANUAL_WB_FLUSH_CTRL))
+		ufshcd_wb_toggle_flush(hba, true);
 }
 
 static void ufshcd_scsi_unblock_requests(struct ufs_hba *hba)
@@ -5401,9 +5402,6 @@ static int ufshcd_wb_toggle_flush_during_h8(struct ufs_hba *hba, bool set)
 
 static inline void ufshcd_wb_toggle_flush(struct ufs_hba *hba, bool enable)
 {
-	if (hba->quirks & UFSHCI_QUIRK_SKIP_MANUAL_WB_FLUSH_CTRL)
-		return;
-
 	if (enable)
 		ufshcd_wb_buf_flush_enable(hba);
 	else
-- 
2.18.0

Re: [PATCH v2 2/2] scsi: ufs: Relax the condition of UFSHCI_QUIRK_SKIP_MANUAL_WB_FLUSH_CTRL

[Can Guo]

On 2020-12-22 15:29, Stanley Chu wrote:
> UFSHCI_QUIRK_SKIP_MANUAL_WB_FLUSH_CTRL is aimed to skip enabling
> fWriteBoosterBufferFlushEn while WriteBooster is initializing.
> Therefore it is better to apply the checking during WriteBooster
> initialization only.
> 
> Signed-off-by: Stanley Chu <stanley.chu@mediatek.com>
> ---
>  drivers/scsi/ufs/ufshcd.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index 9d61dc3eb842..e50b19925236 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -289,7 +289,8 @@ static inline void ufshcd_wb_config(struct ufs_hba 
> *hba)
>  	if (ret)
>  		dev_err(hba->dev, "%s: En WB flush during H8: failed: %d\n",
>  			__func__, ret);
> -	ufshcd_wb_toggle_flush(hba, true);
> +	if (!(hba->quirks & UFSHCI_QUIRK_SKIP_MANUAL_WB_FLUSH_CTRL))
> +		ufshcd_wb_toggle_flush(hba, true);
>  }
> 
>  static void ufshcd_scsi_unblock_requests(struct ufs_hba *hba)
> @@ -5401,9 +5402,6 @@ static int
> ufshcd_wb_toggle_flush_during_h8(struct ufs_hba *hba, bool set)
> 
>  static inline void ufshcd_wb_toggle_flush(struct ufs_hba *hba, bool 
> enable)
>  {
> -	if (hba->quirks & UFSHCI_QUIRK_SKIP_MANUAL_WB_FLUSH_CTRL)
> -		return;
> -
>  	if (enable)
>  		ufshcd_wb_buf_flush_enable(hba);
>  	else

Thanks for the change, I was thinking about the same.

Reviewed-by: Can Guo <cang@codeaurora.org>

Re: [PATCH v2 0/2] scsi: ufs: Fix power drain and hci quirk for WriteBooster

[Martin K. Petersen]

On Tue, 22 Dec 2020 15:29:03 +0800, Stanley Chu wrote:

> This series fixes two WriteBooster issues,
> 1. Fix a corner case that device flush capability is not disabled during system suspend
> 2. Fix the checking of UFSHCI quirk UFSHCI_QUIRK_SKIP_MANUAL_WB_FLUSH_CTRL
> 
> Stanley Chu (2):
>   scsi: ufs: Fix possible power drain during system suspend
>   scsi: ufs: Relax the condition of
>     UFSHCI_QUIRK_SKIP_MANUAL_WB_FLUSH_CTRL
> 
> [...]

Applied to 5.11/scsi-fixes, thanks!

[1/2] scsi: ufs: Fix possible power drain during system suspend
      https://git.kernel.org/mkp/scsi/c/1d53864c3617
[2/2] scsi: ufs: Relax the condition of UFSHCI_QUIRK_SKIP_MANUAL_WB_FLUSH_CTRL
      https://git.kernel.org/mkp/scsi/c/21acf4601cc6

-- 
Martin K. Petersen	Oracle Linux Engineering