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
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 <[email protected]>
---
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
Use macro UFS_WB_BUF_REMAIN_PERCENT() instead to provide
more flexible usage of WriteBooster available buffer values.
Signed-off-by: Stanley Chu <[email protected]>
---
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
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 <[email protected]>
> ---
> 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
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 <[email protected]>
> > ---
> > 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
>
>