[PATCH v4 0/2] ufs: introduce skipping manual flush for wb

[PATCH v4 0/2] ufs: introduce skipping manual flush for wb

[Kiwoong Kim]

v3 -> v4: migrate these to 5.10
v2 -> v3: modify some commit messages
v1 -> v2: enable the quirk in exynos

We have two knobs to flush for write booster, i.e.
fWriteBoosterBufferFlushDuringHibernate and fWriteBoosterBufferFlushEn.
However, many product makers uses only fWriteBoosterBufferFlushDuringHibernate,
because this can reportedly cover most scenarios and
there have been some reports that flush by fWriteBoosterBufferFlushEn
could lead to raise power consumption thanks to unexpected internal
operations. So we need a way to enable or disable fWriteBoosterEn
operations. For those case, this quirk will allow to avoid manual flush

Kiwoong Kim (2):
  ufs: introduce skipping manual flush for wb
  ufs: exynos: enable UFSHCI_QUIRK_SKIP_MANUAL_WB_FLUSH_CTRL

 drivers/scsi/ufs/ufs-exynos.c | 3 ++-
 drivers/scsi/ufs/ufshcd.c     | 3 +++
 drivers/scsi/ufs/ufshcd.h     | 5 +++++
 3 files changed, 10 insertions(+), 1 deletion(-)

-- 
2.7.4

[PATCH v4 1/2] ufs: introduce skipping manual flush for wb

[Kiwoong Kim]

We have two knobs to flush for write booster, i.e.
fWriteBoosterBufferFlushDuringHibernate and fWriteBoosterBufferFlushEn.
However, many product makers uses only fWriteBoosterBufferFlushDuringHibernate,
because this can reportedly cover most scenarios and
there have been some reports that flush by fWriteBoosterBufferFlushEn
could lead to raise power consumption thanks to unexpected internal
operations. So we need a way to enable or disable fWriteBoosterEn
operations. For those case, this quirk will allow to avoid manual flush

Signed-off-by: Kiwoong Kim <kwmad.kim@samsung.com>
Reviewed-by: Avri Altman <avri.altman@wdc.com>
---
 drivers/scsi/ufs/ufshcd.c | 3 +++
 drivers/scsi/ufs/ufshcd.h | 5 +++++
 2 files changed, 8 insertions(+)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 64bd59c..54a2259 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -5307,6 +5307,9 @@ 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
diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
index 02bd405..e99efee 100644
--- a/drivers/scsi/ufs/ufshcd.h
+++ b/drivers/scsi/ufs/ufshcd.h
@@ -531,6 +531,11 @@ enum ufshcd_quirks {
 	 * OCS FATAL ERROR with device error through sense data
 	 */
 	UFSHCD_QUIRK_BROKEN_OCS_FATAL_ERROR		= 1 << 10,
+
+	/*
+	 * This quirk needs to disable manual flush for write booster
+	 */
+	UFSHCI_QUIRK_SKIP_MANUAL_WB_FLUSH_CTRL		= 1 << 11,
 };
 
 enum ufshcd_caps {
-- 
2.7.4

Re: [PATCH v4 1/2] ufs: introduce skipping manual flush for wb

[James Bottomley]

On Sat, 2020-09-05 at 15:06 +0900, Kiwoong Kim wrote:
[...]
> +
> +	/*
> +	 * This quirk needs to disable manual flush for write booster
> +	 */
> +	UFSHCI_QUIRK_SKIP_MANUAL_WB_FLUSH_CTRL		= 1 << 11,

You can't have 1 << 11 it's already taken by:

8da76f71fef7 scsi: ufs-pci: Add quirk for broken auto-hibernate for Intel EHL

	/*
         * This quirk needs to disable manual flush for write booster
         */
        UFSHCI_QUIRK_SKIP_MANUAL_WB_FLUSH_CTRL          = 1 << 11,

I take it 1 << 12 is OK?

James

RE: [PATCH v4 1/2] ufs: introduce skipping manual flush for wb

[Kiwoong Kim]

> [...]
> > +
> > +	/*
> > +	 * This quirk needs to disable manual flush for write booster
> > +	 */
> > +	UFSHCI_QUIRK_SKIP_MANUAL_WB_FLUSH_CTRL		= 1 << 11,
> 
> You can't have 1 << 11 it's already taken by:
> 
> 8da76f71fef7 scsi: ufs-pci: Add quirk for broken auto-hibernate for Intel
> EHL
> 
> 	/*
>          * This quirk needs to disable manual flush for write booster
>          */
>         UFSHCI_QUIRK_SKIP_MANUAL_WB_FLUSH_CTRL          = 1 << 11,
> 
> I take it 1 << 12 is OK?
> 
> James
> 
Sure, no problem.

Thanks.
Kiwoong Kim

[PATCH v4 2/2] ufs: exynos: enable UFSHCI_QUIRK_SKIP_MANUAL_WB_FLUSH_CTRL

[Kiwoong Kim]

For Exynos, only flush during hibern8 is enough for
sustaining performance and I think that there is
a possiblity of raising current thanks to an increase
of internal operations for manual flush.

Signed-off-by: Kiwoong Kim <kwmad.kim@samsung.com>
Reviewed-by: Avri Altman <avri.altman@wdc.com>
---
 drivers/scsi/ufs/ufs-exynos.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/ufs/ufs-exynos.c b/drivers/scsi/ufs/ufs-exynos.c
index 7d383e3..4584f5d 100644
--- a/drivers/scsi/ufs/ufs-exynos.c
+++ b/drivers/scsi/ufs/ufs-exynos.c
@@ -1290,7 +1290,8 @@ struct exynos_ufs_drv_data exynos_ufs_drvs = {
 				  UFSHCI_QUIRK_BROKEN_REQ_LIST_CLR |
 				  UFSHCI_QUIRK_BROKEN_HCE |
 				  UFSHCI_QUIRK_SKIP_RESET_INTR_AGGR |
-				  UFSHCD_QUIRK_BROKEN_OCS_FATAL_ERROR,
+				  UFSHCD_QUIRK_BROKEN_OCS_FATAL_ERROR |
+				  UFSHCI_QUIRK_SKIP_MANUAL_WB_FLUSH_CTRL,
 	.opts			= EXYNOS_UFS_OPT_HAS_APB_CLK_CTRL |
 				  EXYNOS_UFS_OPT_BROKEN_AUTO_CLK_CTRL |
 				  EXYNOS_UFS_OPT_BROKEN_RX_SEL_IDX |
-- 
2.7.4

Re: [PATCH v4 0/2] ufs: introduce skipping manual flush for wb

[Asutosh Das (asd)]

On 9/4/2020 11:06 PM, Kiwoong Kim wrote:
> v3 -> v4: migrate these to 5.10
> v2 -> v3: modify some commit messages
> v1 -> v2: enable the quirk in exynos
> 
> We have two knobs to flush for write booster, i.e.
> fWriteBoosterBufferFlushDuringHibernate and fWriteBoosterBufferFlushEn.
> However, many product makers uses only fWriteBoosterBufferFlushDuringHibernate,
> because this can reportedly cover most scenarios and
> there have been some reports that flush by fWriteBoosterBufferFlushEn
> could lead to raise power consumption thanks to unexpected internal
> operations. So we need a way to enable or disable fWriteBoosterEn
> operations. For those case, this quirk will allow to avoid manual flush
> 
> Kiwoong Kim (2):
>    ufs: introduce skipping manual flush for wb
>    ufs: exynos: enable UFSHCI_QUIRK_SKIP_MANUAL_WB_FLUSH_CTRL
> 
>   drivers/scsi/ufs/ufs-exynos.c | 3 ++-
>   drivers/scsi/ufs/ufshcd.c     | 3 +++
>   drivers/scsi/ufs/ufshcd.h     | 5 +++++
>   3 files changed, 10 insertions(+), 1 deletion(-)
> 

Reviewed-by: Asutosh Das <asutoshd@codeaurora.org>

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
Linux Foundation Collaborative Project

Re: [PATCH v4 0/2] ufs: introduce skipping manual flush for wb

[Martin K. Petersen]

On Sat, 5 Sep 2020 15:06:50 +0900, Kiwoong Kim wrote:

> v3 -> v4: migrate these to 5.10
> v2 -> v3: modify some commit messages
> v1 -> v2: enable the quirk in exynos
> 
> We have two knobs to flush for write booster, i.e.
> fWriteBoosterBufferFlushDuringHibernate and fWriteBoosterBufferFlushEn.
> However, many product makers uses only fWriteBoosterBufferFlushDuringHibernate,
> because this can reportedly cover most scenarios and
> there have been some reports that flush by fWriteBoosterBufferFlushEn
> could lead to raise power consumption thanks to unexpected internal
> operations. So we need a way to enable or disable fWriteBoosterEn
> operations. For those case, this quirk will allow to avoid manual flush
> 
> [...]

Applied to 5.10/scsi-queue, thanks!

[1/2] scsi: ufs: Introduce skipping manual flush for Write Booster
      https://git.kernel.org/mkp/scsi/c/5df6f2def50c
[2/2] scsi: ufs: exynos: Enable UFSHCI_QUIRK_SKIP_MANUAL_WB_FLUSH_CTRL
      https://git.kernel.org/mkp/scsi/c/7973b8ac669e

-- 
Martin K. Petersen	Oracle Linux Engineering