Re: [PATCH v3 1/5] scsi: ufs: enable WriteBooster on some pre-3.1 UFS devices
From: Can Guo
Date: Sat May 02 2020 - 03:47:49 EST
Hi Stanley,
On 2020-05-01 22:38, Stanley Chu wrote:
WriteBooster feature can be supported by some pre-3.1 UFS devices
by upgrading firmware.
To enable WriteBooster feature in such devices, introduce a device
quirk to 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 all conditions are
satisfied,
1. Host enables WriteBooster capability
2. UFS 3.1 device or UFS pre-3.1 device with quirk
UFS_DEVICE_QUIRK_SUPPORT_EXTENDED_FEATURES enabled
3. Device descriptor has dExtendedUFSFeaturesSupport field
4. WriteBooster support is specified in above field
Signed-off-by: Stanley Chu <stanley.chu@xxxxxxxxxxxx>
---
drivers/scsi/ufs/ufs_quirks.h | 7 ++++
drivers/scsi/ufs/ufshcd.c | 66 ++++++++++++++++++++++-------------
2 files changed, 48 insertions(+), 25 deletions(-)
diff --git a/drivers/scsi/ufs/ufs_quirks.h
b/drivers/scsi/ufs/ufs_quirks.h
index df7a1e6805a3..e3175a63c676 100644
--- a/drivers/scsi/ufs/ufs_quirks.h
+++ b/drivers/scsi/ufs/ufs_quirks.h
@@ -101,4 +101,11 @@ struct ufs_dev_fix {
*/
#define UFS_DEVICE_QUIRK_HOST_VS_DEBUGSAVECONFIGTIME (1 << 9)
+/*
+ * Some pre-3.1 UFS devices can support extended features by upgrading
+ * the firmware. Enable this quirk to make UFS core driver probe and
enable
+ * supported features on such devices.
+ */
+#define UFS_DEVICE_QUIRK_SUPPORT_EXTENDED_FEATURES (1 << 10)
+
#endif /* UFS_QUIRKS_H_ */
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 915e963398c4..c6668799d956 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -229,6 +229,8 @@ static struct ufs_dev_fix ufs_fixups[] = {
UFS_DEVICE_QUIRK_HOST_PA_SAVECONFIGTIME),
UFS_FIX(UFS_VENDOR_SKHYNIX, "hB8aL1" /*H28U62301AMR*/,
UFS_DEVICE_QUIRK_HOST_VS_DEBUGSAVECONFIGTIME),
+ UFS_FIX(UFS_VENDOR_SKHYNIX, "H9HQ21AFAMZDAR",
+ UFS_DEVICE_QUIRK_SUPPORT_EXTENDED_FEATURES),
END_FIX
};
@@ -6800,9 +6802,19 @@ static int ufshcd_scsi_add_wlus(struct ufs_hba
*hba)
static void ufshcd_wb_probe(struct ufs_hba *hba, u8 *desc_buf)
{
+ if (!ufshcd_is_wb_allowed(hba))
+ return;
+
+ 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 +6830,29 @@ 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;
+
+ return;
+
+wb_disabled:
+ hba->caps &= ~UFSHCD_CAP_WB_EN;
+}
+
+static void ufs_fixup_device_setup(struct ufs_hba *hba)
+{
+ struct ufs_dev_fix *f;
+ struct ufs_dev_info *dev_info = &hba->dev_info;
+
+ for (f = ufs_fixups; f->quirk; f++) {
+ if ((f->wmanufacturerid == dev_info->wmanufacturerid ||
+ f->wmanufacturerid == UFS_ANY_VENDOR) &&
+ ((dev_info->model &&
+ STR_PRFX_EQUAL(f->model, dev_info->model)) ||
+ !strcmp(f->model, UFS_ANY_MODEL)))
+ hba->dev_quirks |= f->quirk;
+ }
}
static int ufs_get_device_desc(struct ufs_hba *hba)
@@ -6862,10 +6892,6 @@ 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)
- ufshcd_wb_probe(hba, desc_buf);
-
err = ufshcd_read_string_desc(hba, model_index,
&dev_info->model, SD_ASCII_STD);
if (err < 0) {
@@ -6874,6 +6900,13 @@ static int ufs_get_device_desc(struct ufs_hba
*hba)
goto out;
}
+ ufs_fixup_device_setup(hba);
+
+ /* Enable WB only for UFS-3.1 */
Also update this comment to reflect your change?
+ if (dev_info->wspecversion >= 0x310 ||
+ (hba->dev_quirks & UFS_DEVICE_QUIRK_SUPPORT_EXTENDED_FEATURES))
+ ufshcd_wb_probe(hba, desc_buf);
+
Can we somehow move this after ufshcd_tune_unipro_params() or come up
with
a better way to leverage ufshcd_vops_apply_dev_quirks()? I am asking
this
because if we only rely on adding quirks to ufs_fixups in ufshcd.c, the
table will keep growing and I am sure it will - as flash vendors are
trying
to make their UFS2.1 products to be capable of WB (different densities
and
different NAND processes from different vendors, the combos can be quite
a
few). Meanwhile, some models are specifically made for some customers to
support WB, meaning having them in the table may not help in a
generalized
way, and it is not like some hot fixes that we have to take, it is just
for
a non-standard feature. If we can leverage
ufshcd_vops_apply_dev_quirks(),
SoC vendors can freely add the quirk without touching ufs_fixups table,
which means you don't need to update ufs_fixups every time just for
adding
a new model (GKI rules), you can have your own WB white list in vendor
driver. What do you think?
Thanks,
Can Guo.
/*
* ufshcd_read_string_desc returns size of the string
* reset the error value
@@ -6893,21 +6926,6 @@ static void ufs_put_device_desc(struct ufs_hba
*hba)
dev_info->model = NULL;
}
-static void ufs_fixup_device_setup(struct ufs_hba *hba)
-{
- struct ufs_dev_fix *f;
- struct ufs_dev_info *dev_info = &hba->dev_info;
-
- for (f = ufs_fixups; f->quirk; f++) {
- if ((f->wmanufacturerid == dev_info->wmanufacturerid ||
- f->wmanufacturerid == UFS_ANY_VENDOR) &&
- ((dev_info->model &&
- STR_PRFX_EQUAL(f->model, dev_info->model)) ||
- !strcmp(f->model, UFS_ANY_MODEL)))
- hba->dev_quirks |= f->quirk;
- }
-}
-
/**
* ufshcd_tune_pa_tactivate - Tunes PA_TActivate of local UniPro
* @hba: per-adapter instance
@@ -7244,8 +7262,6 @@ static int ufshcd_device_params_init(struct
ufs_hba *hba)
ufshcd_get_ref_clk_gating_wait(hba);
- ufs_fixup_device_setup(hba);
-
if (!ufshcd_query_flag_retry(hba, UPIU_QUERY_OPCODE_READ_FLAG,
QUERY_FLAG_IDN_PWR_ON_WPE, &flag))
hba->dev_info.f_power_on_wp_en = flag;