Hi,
This patchset adds LU dedicated buffer mode support for WriteBooster.
In the meanwhile, enable WriteBooster capability on MediaTek UFS platforms.
v1 -> v2:
- Change the definition name of WriteBooster buffer mode to correspond to specification (Bean Huo)
- Add patch #5: "scsi: ufs: cleanup WriteBooster feature"
Stanley Chu (5):
scsi: ufs: allow legacy UFS devices to enable WriteBooster
scsi: ufs: add "index" in parameter list of ufshcd_query_flag()
scsi: ufs: add LU Dedicated buffer mode support for WriteBooster
scsi: ufs-mediatek: enable WriteBooster capability
scsi: ufs: cleanup WriteBooster feature
drivers/scsi/ufs/ufs-mediatek.c | 3 +
drivers/scsi/ufs/ufs-sysfs.c | 2 +-
drivers/scsi/ufs/ufs.h | 7 ++
drivers/scsi/ufs/ufshcd.c | 112 ++++++++++++++++++++++----------
drivers/scsi/ufs/ufshcd.h | 2 +-
5 files changed, 90 insertions(+), 36 deletions(-)
--
2.18.0
Small cleanup as below items,
1. Use ufshcd_is_wb_allowed() directly instead of ufshcd_wb_sup()
since ufshcd_wb_sup() just returns the result of
ufshcd_is_wb_allowed().
2. In ufshcd_suspend(), "else if (!ufshcd_is_runtime_pm(pm_op))
can be simplified to "else" since both have the same meaning.
This patch does not change any functionality.
Signed-off-by: Stanley Chu <[email protected]>
---
drivers/scsi/ufs/ufshcd.c | 20 +++++++-------------
1 file changed, 7 insertions(+), 13 deletions(-)
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index b970a422a5ea..883a46f7fcc9 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -253,7 +253,6 @@ static int ufshcd_scale_clks(struct ufs_hba *hba, bool scale_up);
static irqreturn_t ufshcd_intr(int irq, void *__hba);
static int ufshcd_change_power_mode(struct ufs_hba *hba,
struct ufs_pa_layer_attr *pwr_mode);
-static bool ufshcd_wb_sup(struct ufs_hba *hba);
static int ufshcd_wb_buf_flush_enable(struct ufs_hba *hba);
static int ufshcd_wb_buf_flush_disable(struct ufs_hba *hba);
static int ufshcd_wb_ctrl(struct ufs_hba *hba, bool enable);
@@ -285,7 +284,7 @@ static inline void ufshcd_wb_config(struct ufs_hba *hba)
{
int ret;
- if (!ufshcd_wb_sup(hba))
+ if (!ufshcd_is_wb_allowed(hba))
return;
ret = ufshcd_wb_ctrl(hba, true);
@@ -5197,11 +5196,6 @@ static void ufshcd_bkops_exception_event_handler(struct ufs_hba *hba)
__func__, err);
}
-static bool ufshcd_wb_sup(struct ufs_hba *hba)
-{
- return ufshcd_is_wb_allowed(hba);
-}
-
static int ufshcd_wb_get_index(struct ufs_hba *hba)
{
if (hba->dev_info.b_wb_buffer_type == WB_BUF_MODE_LU_DEDICATED)
@@ -5216,7 +5210,7 @@ static int ufshcd_wb_ctrl(struct ufs_hba *hba, bool enable)
u8 index;
enum query_opcode opcode;
- if (!ufshcd_wb_sup(hba))
+ if (!ufshcd_is_wb_allowed(hba))
return 0;
if (!(enable ^ hba->wb_enabled))
@@ -5272,7 +5266,7 @@ static int ufshcd_wb_buf_flush_enable(struct ufs_hba *hba)
int ret;
u8 index;
- if (!ufshcd_wb_sup(hba) || hba->wb_buf_flush_enabled)
+ if (!ufshcd_is_wb_allowed(hba) || hba->wb_buf_flush_enabled)
return 0;
index = ufshcd_wb_get_index(hba);
@@ -5294,7 +5288,7 @@ static int ufshcd_wb_buf_flush_disable(struct ufs_hba *hba)
int ret;
u8 index;
- if (!ufshcd_wb_sup(hba) || !hba->wb_buf_flush_enabled)
+ if (!ufshcd_is_wb_allowed(hba) || !hba->wb_buf_flush_enabled)
return 0;
index = ufshcd_wb_get_index(hba);
@@ -5344,7 +5338,7 @@ static bool ufshcd_wb_keep_vcc_on(struct ufs_hba *hba)
int ret;
u32 avail_buf;
- if (!ufshcd_wb_sup(hba))
+ if (!ufshcd_is_wb_allowed(hba))
return false;
/*
* The ufs device needs the vcc to be ON to flush.
@@ -8225,12 +8219,12 @@ static int ufshcd_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op)
* configured WB type is 70% full, keep vcc ON
* for the device to flush the wb buffer
*/
- if ((hba->auto_bkops_enabled && ufshcd_wb_sup(hba)) ||
+ if ((hba->auto_bkops_enabled && ufshcd_is_wb_allowed(hba)) ||
ufshcd_wb_keep_vcc_on(hba))
hba->dev_info.keep_vcc_on = true;
else
hba->dev_info.keep_vcc_on = false;
- } else if (!ufshcd_is_runtime_pm(pm_op)) {
+ } else {
hba->dev_info.keep_vcc_on = false;
}
--
2.18.0
According to UFS specification, there are two WriteBooster mode of
operations: "LU dedicated buffer" mode and "shared buffer" mode.
In the "LU dedicated buffer" mode, the WriteBooster Buffer is
dedicated to a logical unit.
If the device supports the "LU dedicated buffer" mode, this mode is
configured by setting bWriteBoosterBufferType to 00h. The logical
unit WriteBooster Buffer size is configured by setting the
dLUNumWriteBoosterBufferAllocUnits field of the related Unit
Descriptor. Only a value greater than zero enables the WriteBooster
feature in the logical unit.
Modify ufshcd_wb_probe() as above description to support LU Dedicated
buffer type.
Signed-off-by: Stanley Chu <[email protected]>
---
drivers/scsi/ufs/ufs.h | 7 +++++
drivers/scsi/ufs/ufshcd.c | 63 ++++++++++++++++++++++++++++++++-------
2 files changed, 59 insertions(+), 11 deletions(-)
diff --git a/drivers/scsi/ufs/ufs.h b/drivers/scsi/ufs/ufs.h
index daac5053b850..eb3d3cebc87d 100644
--- a/drivers/scsi/ufs/ufs.h
+++ b/drivers/scsi/ufs/ufs.h
@@ -330,6 +330,12 @@ enum health_desc_param {
HEALTH_DESC_PARAM_LIFE_TIME_EST_B = 0x4,
};
+/* WriteBooster buffer mode */
+enum {
+ WB_BUF_MODE_LU_DEDICATED = 0x0,
+ WB_BUF_MODE_SHARED = 0x1,
+};
+
/*
* Logical Unit Write Protect
* 00h: LU not write protected
@@ -559,6 +565,7 @@ struct ufs_dev_info {
bool is_lu_power_on_wp;
/* Maximum number of general LU supported by the UFS device */
u8 max_lu_supported;
+ u8 wb_dedicated_lu;
u16 wmanufacturerid;
/*UFS device Product Name */
u8 *model;
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 465ee023ea4b..b970a422a5ea 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -5202,9 +5202,18 @@ static bool ufshcd_wb_sup(struct ufs_hba *hba)
return ufshcd_is_wb_allowed(hba);
}
+static int ufshcd_wb_get_index(struct ufs_hba *hba)
+{
+ if (hba->dev_info.b_wb_buffer_type == WB_BUF_MODE_LU_DEDICATED)
+ return hba->dev_info.wb_dedicated_lu;
+ else
+ return 0;
+}
+
static int ufshcd_wb_ctrl(struct ufs_hba *hba, bool enable)
{
int ret;
+ u8 index;
enum query_opcode opcode;
if (!ufshcd_wb_sup(hba))
@@ -5217,8 +5226,9 @@ static int ufshcd_wb_ctrl(struct ufs_hba *hba, bool enable)
else
opcode = UPIU_QUERY_OPCODE_CLEAR_FLAG;
+ index = ufshcd_wb_get_index(hba);
ret = ufshcd_query_flag_retry(hba, opcode,
- QUERY_FLAG_IDN_WB_EN, 0, NULL);
+ QUERY_FLAG_IDN_WB_EN, index, NULL);
if (ret) {
dev_err(hba->dev, "%s write booster %s failed %d\n",
__func__, enable ? "enable" : "disable", ret);
@@ -5235,15 +5245,17 @@ static int ufshcd_wb_ctrl(struct ufs_hba *hba, bool enable)
static int ufshcd_wb_toggle_flush_during_h8(struct ufs_hba *hba, bool set)
{
int val;
+ u8 index;
if (set)
val = UPIU_QUERY_OPCODE_SET_FLAG;
else
val = UPIU_QUERY_OPCODE_CLEAR_FLAG;
+ index = ufshcd_wb_get_index(hba);
return ufshcd_query_flag_retry(hba, val,
- QUERY_FLAG_IDN_WB_BUFF_FLUSH_DURING_HIBERN8, 0,
- NULL);
+ QUERY_FLAG_IDN_WB_BUFF_FLUSH_DURING_HIBERN8,
+ index, NULL);
}
static inline void ufshcd_wb_toggle_flush(struct ufs_hba *hba, bool enable)
@@ -5258,13 +5270,15 @@ static inline void ufshcd_wb_toggle_flush(struct ufs_hba *hba, bool enable)
static int ufshcd_wb_buf_flush_enable(struct ufs_hba *hba)
{
int ret;
+ u8 index;
if (!ufshcd_wb_sup(hba) || hba->wb_buf_flush_enabled)
return 0;
+ index = ufshcd_wb_get_index(hba);
ret = ufshcd_query_flag_retry(hba, UPIU_QUERY_OPCODE_SET_FLAG,
QUERY_FLAG_IDN_WB_BUFF_FLUSH_EN,
- 0, NULL);
+ index, NULL);
if (ret)
dev_err(hba->dev, "%s WB - buf flush enable failed %d\n",
__func__, ret);
@@ -5278,12 +5292,15 @@ static int ufshcd_wb_buf_flush_enable(struct ufs_hba *hba)
static int ufshcd_wb_buf_flush_disable(struct ufs_hba *hba)
{
int ret;
+ u8 index;
if (!ufshcd_wb_sup(hba) || !hba->wb_buf_flush_enabled)
return 0;
+ index = ufshcd_wb_get_index(hba);
ret = ufshcd_query_flag_retry(hba, UPIU_QUERY_OPCODE_CLEAR_FLAG,
- QUERY_FLAG_IDN_WB_BUFF_FLUSH_EN, 0, NULL);
+ QUERY_FLAG_IDN_WB_BUFF_FLUSH_EN,
+ index, NULL);
if (ret) {
dev_warn(hba->dev, "%s: WB - buf flush disable failed %d\n",
__func__, ret);
@@ -6802,6 +6819,10 @@ static int ufshcd_scsi_add_wlus(struct ufs_hba *hba)
static void ufshcd_wb_probe(struct ufs_hba *hba, u8 *desc_buf)
{
+ int ret;
+ u8 lun;
+ u32 d_lu_wb_buf_alloc = 0;
+
if (hba->desc_size.dev_desc <= DEVICE_DESC_PARAM_EXT_UFS_FEATURE_SUP)
goto wb_disabled;
@@ -6821,15 +6842,35 @@ static void ufshcd_wb_probe(struct ufs_hba *hba, u8 *desc_buf)
hba->dev_info.b_wb_buffer_type =
desc_buf[DEVICE_DESC_PARAM_WB_TYPE];
- hba->dev_info.d_wb_alloc_units =
- get_unaligned_be32(desc_buf +
- DEVICE_DESC_PARAM_WB_SHARED_ALLOC_UNITS);
hba->dev_info.b_presrv_uspc_en =
desc_buf[DEVICE_DESC_PARAM_WB_PRESRV_USRSPC_EN];
- if (!(hba->dev_info.b_wb_buffer_type &&
- hba->dev_info.d_wb_alloc_units))
- goto wb_disabled;
+ if (hba->dev_info.b_wb_buffer_type == WB_BUF_MODE_SHARED) {
+ hba->dev_info.d_wb_alloc_units =
+ get_unaligned_be32(desc_buf +
+ DEVICE_DESC_PARAM_WB_SHARED_ALLOC_UNITS);
+ if (!hba->dev_info.d_wb_alloc_units)
+ goto wb_disabled;
+ } else {
+ for (lun = 0; lun < hba->dev_info.max_lu_supported; lun++) {
+ ret = ufshcd_read_unit_desc_param(hba,
+ lun,
+ UNIT_DESC_PARAM_WB_BUF_ALLOC_UNITS,
+ (u8 *)&d_lu_wb_buf_alloc,
+ sizeof(d_lu_wb_buf_alloc));
+ if (ret)
+ goto wb_disabled;
+ if (d_lu_wb_buf_alloc) {
+ hba->dev_info.wb_dedicated_lu = lun;
+ break;
+ }
+ }
+
+ if (!d_lu_wb_buf_alloc)
+ goto wb_disabled;
+ }
+
+ return;
wb_disabled:
hba->caps &= ~UFSHCD_CAP_WB_EN;
--
2.18.0
Enable WriteBooster capability on MediaTek UFS platforms.
Signed-off-by: Stanley Chu <[email protected]>
Reviewed-by: Bean Huo <[email protected]>
---
drivers/scsi/ufs/ufs-mediatek.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/scsi/ufs/ufs-mediatek.c b/drivers/scsi/ufs/ufs-mediatek.c
index 673c16596fb2..15b9c420a3a5 100644
--- a/drivers/scsi/ufs/ufs-mediatek.c
+++ b/drivers/scsi/ufs/ufs-mediatek.c
@@ -263,6 +263,9 @@ static int ufs_mtk_init(struct ufs_hba *hba)
/* Enable clock-gating */
hba->caps |= UFSHCD_CAP_CLK_GATING;
+ /* Enable WriteBooster */
+ hba->caps |= UFSHCD_CAP_WB_EN;
+
/*
* ufshcd_vops_init() is invoked after
* ufshcd_setup_clock(true) in ufshcd_hba_init() thus
--
2.18.0
WriteBooster feature may be supported by some legacy UFS devices
(i.e., < 3.1) by upgrading firmware.
To enable WriteBooster feature in such devices, relax the entrance
condition of ufshcd_wb_probe() to allow host driver to check those
devices' WriteBooster capability.
WriteBooster feature can be available if below both conditions are
satisfied,
1. Device descriptor has dExtendedUFSFeaturesSupport field.
2. WriteBooster support is specified in above field.
Signed-off-by: Stanley Chu <[email protected]>
Reviewed-by: Bean Huo <[email protected]>
---
drivers/scsi/ufs/ufshcd.c | 19 +++++++++++++------
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 915e963398c4..111812c5304a 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -6800,9 +6800,16 @@ static int ufshcd_scsi_add_wlus(struct ufs_hba *hba)
static void ufshcd_wb_probe(struct ufs_hba *hba, u8 *desc_buf)
{
+ if (hba->desc_size.dev_desc <= DEVICE_DESC_PARAM_EXT_UFS_FEATURE_SUP)
+ goto wb_disabled;
+
hba->dev_info.d_ext_ufs_feature_sup =
get_unaligned_be32(desc_buf +
DEVICE_DESC_PARAM_EXT_UFS_FEATURE_SUP);
+
+ if (!(hba->dev_info.d_ext_ufs_feature_sup & UFS_DEV_WRITE_BOOSTER_SUP))
+ goto wb_disabled;
+
/*
* WB may be supported but not configured while provisioning.
* The spec says, in dedicated wb buffer mode,
@@ -6818,11 +6825,12 @@ static void ufshcd_wb_probe(struct ufs_hba *hba, u8 *desc_buf)
hba->dev_info.b_presrv_uspc_en =
desc_buf[DEVICE_DESC_PARAM_WB_PRESRV_USRSPC_EN];
- if (!((hba->dev_info.d_ext_ufs_feature_sup &
- UFS_DEV_WRITE_BOOSTER_SUP) &&
- hba->dev_info.b_wb_buffer_type &&
+ if (!(hba->dev_info.b_wb_buffer_type &&
hba->dev_info.d_wb_alloc_units))
- hba->caps &= ~UFSHCD_CAP_WB_EN;
+ goto wb_disabled;
+
+wb_disabled:
+ hba->caps &= ~UFSHCD_CAP_WB_EN;
}
static int ufs_get_device_desc(struct ufs_hba *hba)
@@ -6862,8 +6870,7 @@ static int ufs_get_device_desc(struct ufs_hba *hba)
model_index = desc_buf[DEVICE_DESC_PARAM_PRDCT_NAME];
- /* Enable WB only for UFS-3.1 */
- if (dev_info->wspecversion >= 0x310)
+ if (ufshcd_is_wb_allowed(hba))
ufshcd_wb_probe(hba, desc_buf);
err = ufshcd_read_string_desc(hba, model_index,
--
2.18.0
Hi Stanley,
>
> WriteBooster feature may be supported by some legacy UFS devices
> (i.e., < 3.1) by upgrading firmware.
>
> To enable WriteBooster feature in such devices, relax the entrance
> condition of ufshcd_wb_probe() to allow host driver to check those
> devices' WriteBooster capability.
>
> WriteBooster feature can be available if below both conditions are
> satisfied,
>
> 1. Device descriptor has dExtendedUFSFeaturesSupport field.
> 2. WriteBooster support is specified in above field.
>
> Signed-off-by: Stanley Chu <[email protected]>
> Reviewed-by: Bean Huo <[email protected]>
WB was first introduced as part of UFS3.1, and lately as part of UFS2.2.
Any non-standard behavior should be classified as a quirk.
Thanks,
Avri
>
> static void ufshcd_wb_probe(struct ufs_hba *hba, u8 *desc_buf)
> {
> + int ret;
> + u8 lun;
> + u32 d_lu_wb_buf_alloc = 0;
> +
> if (hba->desc_size.dev_desc <=
> DEVICE_DESC_PARAM_EXT_UFS_FEATURE_SUP)
> goto wb_disabled;
>
> @@ -6821,15 +6842,35 @@ static void ufshcd_wb_probe(struct ufs_hba
> *hba, u8 *desc_buf)
> hba->dev_info.b_wb_buffer_type =
> desc_buf[DEVICE_DESC_PARAM_WB_TYPE];
>
> - hba->dev_info.d_wb_alloc_units =
> - get_unaligned_be32(desc_buf +
> - DEVICE_DESC_PARAM_WB_SHARED_ALLOC_UNITS);
> hba->dev_info.b_presrv_uspc_en =
> desc_buf[DEVICE_DESC_PARAM_WB_PRESRV_USRSPC_EN];
>
> - if (!(hba->dev_info.b_wb_buffer_type &&
> - hba->dev_info.d_wb_alloc_units))
> - goto wb_disabled;
> + if (hba->dev_info.b_wb_buffer_type == WB_BUF_MODE_SHARED) {
> + hba->dev_info.d_wb_alloc_units =
> + get_unaligned_be32(desc_buf +
> + DEVICE_DESC_PARAM_WB_SHARED_ALLOC_UNITS);
> + if (!hba->dev_info.d_wb_alloc_units)
> + goto wb_disabled;
> + } else {
> + for (lun = 0; lun < hba->dev_info.max_lu_supported; lun++) {
> + ret = ufshcd_read_unit_desc_param(hba,
> + lun,
> + UNIT_DESC_PARAM_WB_BUF_ALLOC_UNITS,
> + (u8 *)&d_lu_wb_buf_alloc,
> + sizeof(d_lu_wb_buf_alloc));
> + if (ret)
> + goto wb_disabled;
> + if (d_lu_wb_buf_alloc) {
> + hba->dev_info.wb_dedicated_lu = lun;
> + break;
Why are you allowing only a single WB lun?
You should allow those buffers for lun0..lun7
Hi Avri,
On Thu, 2020-04-30 at 07:44 +0000, Avri Altman wrote:
> Hi Stanley,
>
> >
> > WriteBooster feature may be supported by some legacy UFS devices
> > (i.e., < 3.1) by upgrading firmware.
> >
> > To enable WriteBooster feature in such devices, relax the entrance
> > condition of ufshcd_wb_probe() to allow host driver to check those
> > devices' WriteBooster capability.
> >
> > WriteBooster feature can be available if below both conditions are
> > satisfied,
> >
> > 1. Device descriptor has dExtendedUFSFeaturesSupport field.
> > 2. WriteBooster support is specified in above field.
> >
> > Signed-off-by: Stanley Chu <[email protected]>
> > Reviewed-by: Bean Huo <[email protected]>
> WB was first introduced as part of UFS3.1, and lately as part of UFS2.2.
> Any non-standard behavior should be classified as a quirk.
OK! I will try to revise this patch to allow only standard devices
handled by normal path and non-standard devices handled by device
quirks.
Thanks,
Stanley Chu
Hi Avri,
On Thu, 2020-04-30 at 08:30 +0000, Avri Altman wrote:
> >
> > static void ufshcd_wb_probe(struct ufs_hba *hba, u8 *desc_buf)
> > {
> > + int ret;
> > + u8 lun;
> > + u32 d_lu_wb_buf_alloc = 0;
> > +
> > if (hba->desc_size.dev_desc <=
> > DEVICE_DESC_PARAM_EXT_UFS_FEATURE_SUP)
> > goto wb_disabled;
> >
> > @@ -6821,15 +6842,35 @@ static void ufshcd_wb_probe(struct ufs_hba
> > *hba, u8 *desc_buf)
> > hba->dev_info.b_wb_buffer_type =
> > desc_buf[DEVICE_DESC_PARAM_WB_TYPE];
> >
> > - hba->dev_info.d_wb_alloc_units =
> > - get_unaligned_be32(desc_buf +
> > - DEVICE_DESC_PARAM_WB_SHARED_ALLOC_UNITS);
> > hba->dev_info.b_presrv_uspc_en =
> > desc_buf[DEVICE_DESC_PARAM_WB_PRESRV_USRSPC_EN];
> >
> > - if (!(hba->dev_info.b_wb_buffer_type &&
> > - hba->dev_info.d_wb_alloc_units))
> > - goto wb_disabled;
> > + if (hba->dev_info.b_wb_buffer_type == WB_BUF_MODE_SHARED) {
> > + hba->dev_info.d_wb_alloc_units =
> > + get_unaligned_be32(desc_buf +
> > + DEVICE_DESC_PARAM_WB_SHARED_ALLOC_UNITS);
> > + if (!hba->dev_info.d_wb_alloc_units)
> > + goto wb_disabled;
> > + } else {
> > + for (lun = 0; lun < hba->dev_info.max_lu_supported; lun++) {
> > + ret = ufshcd_read_unit_desc_param(hba,
> > + lun,
> > + UNIT_DESC_PARAM_WB_BUF_ALLOC_UNITS,
> > + (u8 *)&d_lu_wb_buf_alloc,
> > + sizeof(d_lu_wb_buf_alloc));
> > + if (ret)
> > + goto wb_disabled;
> > + if (d_lu_wb_buf_alloc) {
> > + hba->dev_info.wb_dedicated_lu = lun;
> > + break;
> Why are you allowing only a single WB lun?
> You should allow those buffers for lun0..lun7
In UFS 3.1 specification, the valid value of bDeviceMaxWriteBoosterLUs
is 1. Therefore only one LU can have WriteBooster buffer.
Thanks,
Stanley