[PATCH v1 0/5] scsi: ufs: allow customizable WriteBooster flush policy

[PATCH v1 0/5] scsi: ufs: allow customizable WriteBooster flush policy

[Stanley Chu]

Hi,

This patch set tries to allow vendors to modify the WriteBooster flush policy.

In the same time, collect all customizable parameters to an unified structure to make UFS driver more clean.

Stanley Chu (5):
  scsi: ufs: introduce ufs_hba_variant_params to collect customizable
    parameters
  scsi: ufs-mediatek: change the way to use customizable parameters
  scsi: ufs: customize flush threshold for WriteBooster
  scsi: ufs: use flexible definition for UFS_WB_BUF_REMAIN_PERCENT
  scsi: ufs-mediatek: customize WriteBooster flush policy

 drivers/scsi/ufs/ufs-mediatek.c |  5 ++--
 drivers/scsi/ufs/ufs.h          |  5 +---
 drivers/scsi/ufs/ufshcd.c       | 46 ++++++++++++++-------------------
 drivers/scsi/ufs/ufshcd.h       |  9 ++++++-
 4 files changed, 32 insertions(+), 33 deletions(-)

-- 
2.18.0
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

[PATCH v1 1/5] scsi: ufs: introduce ufs_hba_variant_params to collect customizable parameters

[Stanley Chu]

There are more and more customizable parameters showed up
in UFS driver. Let's collect them into an unified place to make
the driver more clean.

Signed-off-by: Stanley Chu <stanley.chu@mediatek.com>
---
 drivers/scsi/ufs/ufshcd.c | 38 +++++++++++++++-----------------------
 drivers/scsi/ufs/ufshcd.h |  8 +++++++-
 2 files changed, 22 insertions(+), 24 deletions(-)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 426073a518ef..cdacbe6378a1 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -1353,23 +1353,6 @@ static int ufshcd_devfreq_get_dev_status(struct device *dev,
 	return 0;
 }
 
-static struct devfreq_dev_profile ufs_devfreq_profile = {
-	.polling_ms	= 100,
-	.target		= ufshcd_devfreq_target,
-	.get_dev_status	= ufshcd_devfreq_get_dev_status,
-};
-
-#if IS_ENABLED(CONFIG_DEVFREQ_GOV_SIMPLE_ONDEMAND)
-static struct devfreq_simple_ondemand_data ufs_ondemand_data = {
-	.upthreshold = 70,
-	.downdifferential = 5,
-};
-
-static void *gov_data = &ufs_ondemand_data;
-#else
-static void *gov_data; /* NULL */
-#endif
-
 static int ufshcd_devfreq_init(struct ufs_hba *hba)
 {
 	struct list_head *clk_list = &hba->clk_list_head;
@@ -1385,12 +1368,12 @@ static int ufshcd_devfreq_init(struct ufs_hba *hba)
 	dev_pm_opp_add(hba->dev, clki->min_freq, 0);
 	dev_pm_opp_add(hba->dev, clki->max_freq, 0);
 
-	ufshcd_vops_config_scaling_param(hba, &ufs_devfreq_profile,
-					 gov_data);
+	ufshcd_vops_config_scaling_param(hba, &hba->vps->devfreq_profile,
+					 &hba->vps->ondemand_data);
 	devfreq = devfreq_add_device(hba->dev,
-			&ufs_devfreq_profile,
+			&hba->vps->devfreq_profile,
 			DEVFREQ_GOV_SIMPLE_ONDEMAND,
-			gov_data);
+			&hba->vps->ondemand_data);
 	if (IS_ERR(devfreq)) {
 		ret = PTR_ERR(devfreq);
 		dev_err(hba->dev, "Unable to register with devfreq %d\n", ret);
@@ -4314,7 +4297,7 @@ int ufshcd_hba_enable(struct ufs_hba *hba)
 	 * instruction might be read back.
 	 * This delay can be changed based on the controller.
 	 */
-	ufshcd_delay_us(hba->hba_enable_delay_us, 100);
+	ufshcd_delay_us(hba->vps->hba_enable_delay_us, 100);
 
 	/* wait for the host controller to complete initialization */
 	retry = 50;
@@ -7477,6 +7460,15 @@ static const struct attribute_group *ufshcd_driver_groups[] = {
 	NULL,
 };
 
+static struct ufs_hba_variant_params ufs_hba_vps = {
+	.hba_enable_delay_us		= 1000,
+	.devfreq_profile.polling_ms	= 100,
+	.devfreq_profile.target		= ufshcd_devfreq_target,
+	.devfreq_profile.get_dev_status	= ufshcd_devfreq_get_dev_status,
+	.ondemand_data.upthreshold	= 70,
+	.ondemand_data.downdifferential	= 5,
+};
+
 static struct scsi_host_template ufshcd_driver_template = {
 	.module			= THIS_MODULE,
 	.name			= UFSHCD,
@@ -8724,7 +8716,7 @@ int ufshcd_init(struct ufs_hba *hba, void __iomem *mmio_base, unsigned int irq)
 
 	hba->mmio_base = mmio_base;
 	hba->irq = irq;
-	hba->hba_enable_delay_us = 1000;
+	hba->vps = &ufs_hba_vps;
 
 	err = ufshcd_hba_init(hba);
 	if (err)
diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
index 23a434c03c2a..f7bdf52ba8b0 100644
--- a/drivers/scsi/ufs/ufshcd.h
+++ b/drivers/scsi/ufs/ufshcd.h
@@ -566,6 +566,12 @@ enum ufshcd_caps {
 	UFSHCD_CAP_WB_EN				= 1 << 7,
 };
 
+struct ufs_hba_variant_params {
+	struct devfreq_dev_profile devfreq_profile;
+	struct devfreq_simple_ondemand_data ondemand_data;
+	u16 hba_enable_delay_us;
+};
+
 /**
  * struct ufs_hba - per adapter private structure
  * @mmio_base: UFSHCI base register address
@@ -663,6 +669,7 @@ struct ufs_hba {
 	int nutmrs;
 	u32 ufs_version;
 	const struct ufs_hba_variant_ops *vops;
+	struct ufs_hba_variant_params *vps;
 	void *priv;
 	unsigned int irq;
 	bool is_irq_enabled;
@@ -684,7 +691,6 @@ struct ufs_hba {
 	u32 eh_flags;
 	u32 intr_mask;
 	u16 ee_ctrl_mask;
-	u16 hba_enable_delay_us;
 	bool is_powered;
 
 	/* Work Queues */
-- 
2.18.0
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

[PATCH v1 2/5] scsi: ufs-mediatek: change the way to use customizable parameters

[Stanley Chu]

Now all customizable parameters have been moved to hba->vps,
thus modify the way to use them.

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

diff --git a/drivers/scsi/ufs/ufs-mediatek.c b/drivers/scsi/ufs/ufs-mediatek.c
index c543142554d3..56620f7d88ce 100644
--- a/drivers/scsi/ufs/ufs-mediatek.c
+++ b/drivers/scsi/ufs/ufs-mediatek.c
@@ -79,9 +79,9 @@ static int ufs_mtk_hce_enable_notify(struct ufs_hba *hba,
 
 	if (status == PRE_CHANGE) {
 		if (host->unipro_lpm)
-			hba->hba_enable_delay_us = 0;
+			hba->vps->hba_enable_delay_us = 0;
 		else
-			hba->hba_enable_delay_us = 600;
+			hba->vps->hba_enable_delay_us = 600;
 	}
 
 	return 0;
-- 
2.18.0
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

[PATCH v1 3/5] scsi: ufs: customize flush threshold for WriteBooster

[Stanley Chu]

Allow flush threshold for WriteBooster to be customizable by
vendors. To achieve this, make the value as a variable in struct
ufs_hba first.

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

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index cdacbe6378a1..9a0ce6550c2f 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -5301,8 +5301,8 @@ static bool ufshcd_wb_presrv_usrspc_keep_vcc_on(struct ufs_hba *hba,
 			 cur_buf);
 		return false;
 	}
-	/* Let it continue to flush when >60% full */
-	if (avail_buf < UFS_WB_40_PERCENT_BUF_REMAIN)
+	/* Let it continue to flush when available buffer exceeds threshold */
+	if (avail_buf < hba->vps->wb_flush_threshold)
 		return true;
 
 	return false;
@@ -6839,6 +6839,7 @@ static void ufshcd_wb_probe(struct ufs_hba *hba, u8 *desc_buf)
 		if (!d_lu_wb_buf_alloc)
 			goto wb_disabled;
 	}
+
 	return;
 
 wb_disabled:
@@ -7462,6 +7463,7 @@ static const struct attribute_group *ufshcd_driver_groups[] = {
 
 static struct ufs_hba_variant_params ufs_hba_vps = {
 	.hba_enable_delay_us		= 1000,
+	.wb_flush_threshold		= UFS_WB_40_PERCENT_BUF_REMAIN,
 	.devfreq_profile.polling_ms	= 100,
 	.devfreq_profile.target		= ufshcd_devfreq_target,
 	.devfreq_profile.get_dev_status	= ufshcd_devfreq_get_dev_status,
diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
index f7bdf52ba8b0..e3dfb48e669e 100644
--- a/drivers/scsi/ufs/ufshcd.h
+++ b/drivers/scsi/ufs/ufshcd.h
@@ -570,6 +570,7 @@ struct ufs_hba_variant_params {
 	struct devfreq_dev_profile devfreq_profile;
 	struct devfreq_simple_ondemand_data ondemand_data;
 	u16 hba_enable_delay_us;
+	u32 wb_flush_threshold;
 };
 
 /**
-- 
2.18.0
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

[PATCH v1 4/5] scsi: ufs: use flexible definition for UFS_WB_BUF_REMAIN_PERCENT

[Stanley Chu]

Use macro UFS_WB_BUF_REMAIN_PERCENT() instead to provide
more flexible usage of WriteBooster available buffer values.

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

diff --git a/drivers/scsi/ufs/ufs.h b/drivers/scsi/ufs/ufs.h
index b3135344ab3f..fadba3a3bbcd 100644
--- a/drivers/scsi/ufs/ufs.h
+++ b/drivers/scsi/ufs/ufs.h
@@ -478,10 +478,7 @@ enum ufs_dev_pwr_mode {
 	UFS_POWERDOWN_PWR_MODE	= 3,
 };
 
-enum ufs_dev_wb_buf_avail_size {
-	UFS_WB_10_PERCENT_BUF_REMAIN = 0x1,
-	UFS_WB_40_PERCENT_BUF_REMAIN = 0x4,
-};
+#define UFS_WB_BUF_REMAIN_PERCENT(val) ((val) / 10)
 
 /**
  * struct utp_cmd_rsp - Response UPIU structure
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 9a0ce6550c2f..bcc7a9ea8d2b 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -5336,7 +5336,7 @@ static bool ufshcd_wb_keep_vcc_on(struct ufs_hba *hba)
 	}
 
 	if (!hba->dev_info.b_presrv_uspc_en) {
-		if (avail_buf <= UFS_WB_10_PERCENT_BUF_REMAIN)
+		if (avail_buf <= UFS_WB_BUF_REMAIN_PERCENT(10))
 			return true;
 		return false;
 	}
@@ -7463,7 +7463,7 @@ static const struct attribute_group *ufshcd_driver_groups[] = {
 
 static struct ufs_hba_variant_params ufs_hba_vps = {
 	.hba_enable_delay_us		= 1000,
-	.wb_flush_threshold		= UFS_WB_40_PERCENT_BUF_REMAIN,
+	.wb_flush_threshold		= UFS_WB_BUF_REMAIN_PERCENT(40),
 	.devfreq_profile.polling_ms	= 100,
 	.devfreq_profile.target		= ufshcd_devfreq_target,
 	.devfreq_profile.get_dev_status	= ufshcd_devfreq_get_dev_status,
-- 
2.18.0
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

[PATCH v1 5/5] scsi: ufs-mediatek: customize WriteBooster flush policy

[Stanley Chu]

Change the WriteBooster policy to keep VCC on during
runtime suspend if available WriteBooster buffer is less
than 80%.

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

diff --git a/drivers/scsi/ufs/ufs-mediatek.c b/drivers/scsi/ufs/ufs-mediatek.c
index 56620f7d88ce..94e97701f456 100644
--- a/drivers/scsi/ufs/ufs-mediatek.c
+++ b/drivers/scsi/ufs/ufs-mediatek.c
@@ -271,6 +271,7 @@ static int ufs_mtk_init(struct ufs_hba *hba)
 
 	/* Enable WriteBooster */
 	hba->caps |= UFSHCD_CAP_WB_EN;
+	hba->vps->wb_flush_threshold = UFS_WB_BUF_REMAIN_PERCENT(80);
 
 	/*
 	 * ufshcd_vops_init() is invoked after
-- 
2.18.0
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

Re: [PATCH v1 3/5] scsi: ufs: customize flush threshold for WriteBooster

[Asutosh Das (asd)]

On 5/8/2020 10:15 AM, Stanley Chu wrote:
> Allow flush threshold for WriteBooster to be customizable by
> vendors. To achieve this, make the value as a variable in struct
> ufs_hba first.
> 
> Signed-off-by: Stanley Chu <stanley.chu@mediatek.com>
> ---
>   drivers/scsi/ufs/ufshcd.c | 6 ++++--
>   drivers/scsi/ufs/ufshcd.h | 1 +
>   2 files changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index cdacbe6378a1..9a0ce6550c2f 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -5301,8 +5301,8 @@ static bool ufshcd_wb_presrv_usrspc_keep_vcc_on(struct ufs_hba *hba,
>   			 cur_buf);
>   		return false;
>   	}
> -	/* Let it continue to flush when >60% full */
> -	if (avail_buf < UFS_WB_40_PERCENT_BUF_REMAIN)
> +	/* Let it continue to flush when available buffer exceeds threshold */
> +	if (avail_buf < hba->vps->wb_flush_threshold)
>   		return true;
>   
>   	return false;
> @@ -6839,6 +6839,7 @@ static void ufshcd_wb_probe(struct ufs_hba *hba, u8 *desc_buf)
>   		if (!d_lu_wb_buf_alloc)
>   			goto wb_disabled;
>   	}
> +
Is this newline needed?

>   	return;
>   
>   wb_disabled:
> @@ -7462,6 +7463,7 @@ static const struct attribute_group *ufshcd_driver_groups[] = {
>   
>   static struct ufs_hba_variant_params ufs_hba_vps = {
>   	.hba_enable_delay_us		= 1000,
> +	.wb_flush_threshold		= UFS_WB_40_PERCENT_BUF_REMAIN,
>   	.devfreq_profile.polling_ms	= 100,
>   	.devfreq_profile.target		= ufshcd_devfreq_target,
>   	.devfreq_profile.get_dev_status	= ufshcd_devfreq_get_dev_status,
> diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
> index f7bdf52ba8b0..e3dfb48e669e 100644
> --- a/drivers/scsi/ufs/ufshcd.h
> +++ b/drivers/scsi/ufs/ufshcd.h
> @@ -570,6 +570,7 @@ struct ufs_hba_variant_params {
>   	struct devfreq_dev_profile devfreq_profile;
>   	struct devfreq_simple_ondemand_data ondemand_data;
>   	u16 hba_enable_delay_us;
> +	u32 wb_flush_threshold;
>   };
>   
>   /**
> 

Patch[3] & [4] may be combined into a single patch perhaps?
Patch[4] just redoes what [3] did in a different way, so might as well 
just do what patch[4] does right away.


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

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

Re: [PATCH v1 3/5] scsi: ufs: customize flush threshold for WriteBooster

[Stanley Chu]

Hi Asutosh,

Thanks for your review.

On Fri, 2020-05-08 at 11:12 -0700, Asutosh Das (asd) wrote:
> On 5/8/2020 10:15 AM, Stanley Chu wrote:
> > Allow flush threshold for WriteBooster to be customizable by
> > vendors. To achieve this, make the value as a variable in struct
> > ufs_hba first.
> > 
> > Signed-off-by: Stanley Chu <stanley.chu@mediatek.com>
> > ---
> >   drivers/scsi/ufs/ufshcd.c | 6 ++++--
> >   drivers/scsi/ufs/ufshcd.h | 1 +
> >   2 files changed, 5 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> > index cdacbe6378a1..9a0ce6550c2f 100644
> > --- a/drivers/scsi/ufs/ufshcd.c
> > +++ b/drivers/scsi/ufs/ufshcd.c
> > @@ -5301,8 +5301,8 @@ static bool ufshcd_wb_presrv_usrspc_keep_vcc_on(struct ufs_hba *hba,
> >   			 cur_buf);
> >   		return false;
> >   	}
> > -	/* Let it continue to flush when >60% full */
> > -	if (avail_buf < UFS_WB_40_PERCENT_BUF_REMAIN)
> > +	/* Let it continue to flush when available buffer exceeds threshold */
> > +	if (avail_buf < hba->vps->wb_flush_threshold)
> >   		return true;
> >   
> >   	return false;
> > @@ -6839,6 +6839,7 @@ static void ufshcd_wb_probe(struct ufs_hba *hba, u8 *desc_buf)
> >   		if (!d_lu_wb_buf_alloc)
> >   			goto wb_disabled;
> >   	}
> > +
> Is this newline needed?

Oops, I will remove this in next version.
> 
> >   	return;
> >   
> >   wb_disabled:
> > @@ -7462,6 +7463,7 @@ static const struct attribute_group *ufshcd_driver_groups[] = {
> >   
> >   static struct ufs_hba_variant_params ufs_hba_vps = {
> >   	.hba_enable_delay_us		= 1000,
> > +	.wb_flush_threshold		= UFS_WB_40_PERCENT_BUF_REMAIN,
> >   	.devfreq_profile.polling_ms	= 100,
> >   	.devfreq_profile.target		= ufshcd_devfreq_target,
> >   	.devfreq_profile.get_dev_status	= ufshcd_devfreq_get_dev_status,
> > diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
> > index f7bdf52ba8b0..e3dfb48e669e 100644
> > --- a/drivers/scsi/ufs/ufshcd.h
> > +++ b/drivers/scsi/ufs/ufshcd.h
> > @@ -570,6 +570,7 @@ struct ufs_hba_variant_params {
> >   	struct devfreq_dev_profile devfreq_profile;
> >   	struct devfreq_simple_ondemand_data ondemand_data;
> >   	u16 hba_enable_delay_us;
> > +	u32 wb_flush_threshold;
> >   };
> >   
> >   /**
> > 
> 
> Patch[3] & [4] may be combined into a single patch perhaps?
> Patch[4] just redoes what [3] did in a different way, so might as well 
> just do what patch[4] does right away.

OK! I will try to merge both to a single patch in next version.
Let me know if you have any other comments for the whole series.

Thanks,
Stanley Chu

> 
> 

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek