[RFC PATCH v2 0/3] Support vendor specific operations for WB

[RFC PATCH v2 0/3] Support vendor specific operations for WB

[SEO HOYOUNG]

Hi all,
Here is v2 of the patchset.
This patchs for supporting WB of vendor specific.
- when UFS reset and restore, need to cleare WB buffer.
- need to check WB buffer and flush operate before enter suspend
- do not enable WB with UFS probe.

[v1 -> v2]
- The ufshcd_reset_vendor() fuction for WB reset.
So I modified function name.
- uploade vendor wb code.


SEO HOYOUNG (3):
  scsi: ufs: modify write booster
  scsi: ufs: modify function call name When ufs reset and restore, need
    to disable write booster
  scsi: ufs: add vendor specific write booster To support the fuction of
    writebooster by vendor. The WB behavior that the vendor wants is
    slightly different. But we have to support it

 drivers/scsi/ufs/Makefile     |   1 +
 drivers/scsi/ufs/ufs-exynos.c |   6 +
 drivers/scsi/ufs/ufs_ctmwb.c  | 279 ++++++++++++++++++++++++++++++++++
 drivers/scsi/ufs/ufs_ctmwb.h  |  27 ++++
 drivers/scsi/ufs/ufshcd.c     |  23 ++-
 drivers/scsi/ufs/ufshcd.h     |  43 ++++++
 6 files changed, 374 insertions(+), 5 deletions(-)
 create mode 100644 drivers/scsi/ufs/ufs_ctmwb.c
 create mode 100644 drivers/scsi/ufs/ufs_ctmwb.h

-- 
2.26.0

[RFC PATCH v2 1/3] scsi: ufs: modify write booster

[SEO HOYOUNG]

Add vendor specific functions for WB
Use callback additional setting when use write booster.

Signed-off-by: SEO HOYOUNG <hy50.seo@samsung.com>
---
 drivers/scsi/ufs/ufshcd.c | 23 ++++++++++++++++-----
 drivers/scsi/ufs/ufshcd.h | 43 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 61 insertions(+), 5 deletions(-)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index efc0a6cbfe22..efa16bf4fd76 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -3306,11 +3306,11 @@ int ufshcd_read_string_desc(struct ufs_hba *hba, u8 desc_index,
  *
  * Return 0 in case of success, non-zero otherwise
  */
-static inline int ufshcd_read_unit_desc_param(struct ufs_hba *hba,
-					      int lun,
-					      enum unit_desc_param param_offset,
-					      u8 *param_read_buf,
-					      u32 param_size)
+int ufshcd_read_unit_desc_param(struct ufs_hba *hba,
+				int lun,
+				enum unit_desc_param param_offset,
+				u8 *param_read_buf,
+				u32 param_size)
 {
 	/*
 	 * Unit descriptors are only available for general purpose LUs (LUN id
@@ -3322,6 +3322,7 @@ static inline int ufshcd_read_unit_desc_param(struct ufs_hba *hba,
 	return ufshcd_read_desc_param(hba, QUERY_DESC_IDN_UNIT, lun,
 				      param_offset, param_read_buf, param_size);
 }
+EXPORT_SYMBOL_GPL(ufshcd_read_unit_desc_param);
 
 static int ufshcd_get_ref_clk_gating_wait(struct ufs_hba *hba)
 {
@@ -5257,6 +5258,10 @@ static int ufshcd_wb_ctrl(struct ufs_hba *hba, bool enable)
 
 	if (!(enable ^ hba->wb_enabled))
 		return 0;
+
+	if (!ufshcd_wb_ctrl_vendor(hba, enable))
+		return 0;
+
 	if (enable)
 		opcode = UPIU_QUERY_OPCODE_SET_FLAG;
 	else
@@ -6610,6 +6615,8 @@ static int ufshcd_reset_and_restore(struct ufs_hba *hba)
 	int err = 0;
 	int retries = MAX_HOST_RESET_RETRIES;
 
+	ufshcd_reset_vendor(hba);
+
 	do {
 		/* Reset the attached device */
 		ufshcd_vops_device_reset(hba);
@@ -6903,6 +6910,9 @@ static void ufshcd_wb_probe(struct ufs_hba *hba, u8 *desc_buf)
 	if (!(dev_info->d_ext_ufs_feature_sup & UFS_DEV_WRITE_BOOSTER_SUP))
 		goto wb_disabled;
 
+	if (!ufshcd_wb_alloc_units_vendor(hba))
+		return;
+
 	/*
 	 * WB may be supported but not configured while provisioning.
 	 * The spec says, in dedicated wb buffer mode,
@@ -8260,6 +8270,7 @@ static int ufshcd_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op)
 			/* make sure that auto bkops is disabled */
 			ufshcd_disable_auto_bkops(hba);
 		}
+
 		/*
 		 * If device needs to do BKOP or WB buffer flush during
 		 * Hibern8, keep device power mode as "active power mode"
@@ -8273,6 +8284,8 @@ static int ufshcd_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op)
 			ufshcd_wb_need_flush(hba));
 	}
 
+	ufshcd_wb_toggle_flush_vendor(hba, pm_op);
+
 	if (req_dev_pwr_mode != hba->curr_dev_pwr_mode) {
 		if ((ufshcd_is_runtime_pm(pm_op) && !hba->auto_bkops_enabled) ||
 		    !ufshcd_is_runtime_pm(pm_op)) {
diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
index 656c0691c858..deb9577e0eaa 100644
--- a/drivers/scsi/ufs/ufshcd.h
+++ b/drivers/scsi/ufs/ufshcd.h
@@ -254,6 +254,13 @@ struct ufs_pwr_mode_info {
 	struct ufs_pa_layer_attr info;
 };
 
+struct ufs_wb_ops {
+	int (*wb_toggle_flush_vendor)(struct ufs_hba *hba, enum ufs_pm_op pm_op);
+	int (*wb_alloc_units_vendor)(struct ufs_hba *hba);
+	int (*wb_ctrl_vendor)(struct ufs_hba *hba, bool enable);
+	int (*wb_reset_vendor)(struct ufs_hba *hba, bool force);
+};
+
 /**
  * struct ufs_hba_variant_ops - variant specific callbacks
  * @name: variant name
@@ -752,6 +759,7 @@ struct ufs_hba {
 	struct request_queue	*bsg_queue;
 	bool wb_buf_flush_enabled;
 	bool wb_enabled;
+	struct ufs_wb_ops *wb_ops;
 	struct delayed_work rpm_dev_flush_recheck_work;
 
 #ifdef CONFIG_SCSI_UFS_CRYPTO
@@ -1004,6 +1012,10 @@ int ufshcd_exec_raw_upiu_cmd(struct ufs_hba *hba,
 			     u8 *desc_buff, int *buff_len,
 			     enum query_opcode desc_op);
 
+int ufshcd_read_unit_desc_param(struct ufs_hba *hba,
+				int lun, enum unit_desc_param param_offset,
+				u8 *param_read_buf, u32 param_size);
+
 /* Wrapper functions for safely calling variant operations */
 static inline const char *ufshcd_get_var_name(struct ufs_hba *hba)
 {
@@ -1181,4 +1193,35 @@ static inline u8 ufshcd_scsi_to_upiu_lun(unsigned int scsi_lun)
 int ufshcd_dump_regs(struct ufs_hba *hba, size_t offset, size_t len,
 		     const char *prefix);
 
+static inline int ufshcd_wb_toggle_flush_vendor(struct ufs_hba *hba, enum ufs_pm_op pm_op)
+{
+	if (!hba->wb_ops || !hba->wb_ops->wb_toggle_flush_vendor)
+		return -1;
+
+	return hba->wb_ops->wb_toggle_flush_vendor(hba, pm_op);
+}
+
+static int ufshcd_wb_alloc_units_vendor(struct ufs_hba *hba)
+{
+	if (!hba->wb_ops || !hba->wb_ops->wb_alloc_units_vendor)
+		return -1;
+
+	return hba->wb_ops->wb_alloc_units_vendor(hba);
+}
+
+static int ufshcd_wb_ctrl_vendor(struct ufs_hba *hba, bool enable)
+{
+	if (!hba->wb_ops || !hba->wb_ops->wb_ctrl_vendor)
+		return -1;
+
+	return hba->wb_ops->wb_ctrl_vendor(hba, enable);
+}
+
+static int ufshcd_reset_vendor(struct ufs_hba *hba)
+{
+	if (!hba->wb_ops || !hba->wb_ops->wb_reset_vendor)
+		return -1;
+
+	return hba->wb_ops->wb_reset_vendor(hba, false);
+}
 #endif /* End of Header */
-- 
2.26.0

[RFC PATCH v2 2/3] scsi: ufs: modify function call name When ufs reset and restore, need to disable write booster

[SEO HOYOUNG]

Signed-off-by: SEO HOYOUNG <hy50.seo@samsung.com>
---
 drivers/scsi/ufs/ufshcd.c | 2 +-
 drivers/scsi/ufs/ufshcd.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index efa16bf4fd76..419d3dd7e183 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -6615,7 +6615,7 @@ static int ufshcd_reset_and_restore(struct ufs_hba *hba)
 	int err = 0;
 	int retries = MAX_HOST_RESET_RETRIES;
 
-	ufshcd_reset_vendor(hba);
+	ufshcd_wb_reset_vendor(hba);
 
 	do {
 		/* Reset the attached device */
diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
index deb9577e0eaa..61ae5259c62a 100644
--- a/drivers/scsi/ufs/ufshcd.h
+++ b/drivers/scsi/ufs/ufshcd.h
@@ -1217,7 +1217,7 @@ static int ufshcd_wb_ctrl_vendor(struct ufs_hba *hba, bool enable)
 	return hba->wb_ops->wb_ctrl_vendor(hba, enable);
 }
 
-static int ufshcd_reset_vendor(struct ufs_hba *hba)
+static int ufshcd_wb_reset_vendor(struct ufs_hba *hba)
 {
 	if (!hba->wb_ops || !hba->wb_ops->wb_reset_vendor)
 		return -1;
-- 
2.26.0

[RFC PATCH v2 3/3] scsi: ufs: add vendor specific write booster To support the fuction of writebooster by vendor. The WB behavior that the vendor wants is slightly different. But we have to support it

[SEO HOYOUNG]

Signed-off-by: SEO HOYOUNG <hy50.seo@samsung.com>
---
 drivers/scsi/ufs/Makefile     |   1 +
 drivers/scsi/ufs/ufs-exynos.c |   6 +
 drivers/scsi/ufs/ufs_ctmwb.c  | 279 ++++++++++++++++++++++++++++++++++
 drivers/scsi/ufs/ufs_ctmwb.h  |  27 ++++
 4 files changed, 313 insertions(+)
 create mode 100644 drivers/scsi/ufs/ufs_ctmwb.c
 create mode 100644 drivers/scsi/ufs/ufs_ctmwb.h

diff --git a/drivers/scsi/ufs/Makefile b/drivers/scsi/ufs/Makefile
index 9810963bc049..b1ba36c7d66f 100644
--- a/drivers/scsi/ufs/Makefile
+++ b/drivers/scsi/ufs/Makefile
@@ -5,6 +5,7 @@ obj-$(CONFIG_SCSI_UFS_DWC_TC_PLATFORM) += tc-dwc-g210-pltfrm.o ufshcd-dwc.o tc-d
 obj-$(CONFIG_SCSI_UFS_CDNS_PLATFORM) += cdns-pltfrm.o
 obj-$(CONFIG_SCSI_UFS_QCOM) += ufs-qcom.o
 obj-$(CONFIG_SCSI_UFS_EXYNOS) += ufs-exynos.o
+obj-$(CONFIG_SCSI_UFS_VENDOR_WB) += ufs_ctmwb.o
 obj-$(CONFIG_SCSI_UFSHCD) += ufshcd-core.o
 ufshcd-core-y				+= ufshcd.o ufs-sysfs.o
 ufshcd-core-$(CONFIG_SCSI_UFS_BSG)	+= ufs_bsg.o
diff --git a/drivers/scsi/ufs/ufs-exynos.c b/drivers/scsi/ufs/ufs-exynos.c
index 32b61ba77241..f127f5f2bf36 100644
--- a/drivers/scsi/ufs/ufs-exynos.c
+++ b/drivers/scsi/ufs/ufs-exynos.c
@@ -22,6 +22,9 @@
 
 #include "ufs-exynos.h"
 
+#ifdef CONFIG_SCSI_UFS_VENDOR_WB
+#include "ufs_ctmwb.h"
+#endif
 /*
  * Exynos's Vendor specific registers for UFSHCI
  */
@@ -989,6 +992,9 @@ static int exynos_ufs_init(struct ufs_hba *hba)
 		goto phy_off;
 
 	ufs->hba = hba;
+#ifdef CONFIG_SCSI_UFS_VENDOR_WB
+	ufs->hba->wb_ops = ufshcd_ctmwb_init();
+#endif
 	ufs->opts = ufs->drv_data->opts;
 	ufs->rx_sel_idx = PA_MAXDATALANES;
 	if (ufs->opts & EXYNOS_UFS_OPT_BROKEN_RX_SEL_IDX)
diff --git a/drivers/scsi/ufs/ufs_ctmwb.c b/drivers/scsi/ufs/ufs_ctmwb.c
new file mode 100644
index 000000000000..ab39f40721ae
--- /dev/null
+++ b/drivers/scsi/ufs/ufs_ctmwb.c
@@ -0,0 +1,279 @@
+#include "ufshcd.h"
+#include "ufshci.h"
+#include "ufs_ctmwb.h"
+
+static struct ufshba_ctmwb hba_ctmwb;
+
+/* Query request retries */
+#define QUERY_REQ_RETRIES 3
+
+static int ufshcd_query_attr_retry(struct ufs_hba *hba,
+	enum query_opcode opcode, enum attr_idn idn, u8 index, u8 selector,
+	u32 *attr_val)
+{
+	int ret = 0;
+	u32 retries;
+
+	 for (retries = QUERY_REQ_RETRIES; retries > 0; retries--) {
+		ret = ufshcd_query_attr(hba, opcode, idn, index,
+						selector, attr_val);
+		if (ret)
+			dev_dbg(hba->dev, "%s: failed with error %d, retries %d\n",
+				__func__, ret, retries);
+		else
+			break;
+	}
+
+	if (ret)
+		dev_err(hba->dev,
+			"%s: query attribute, idn %d, failed with error %d after %d retires\n",
+			__func__, idn, ret, QUERY_REQ_RETRIES);
+	return ret;
+}
+
+static int ufshcd_query_flag_retry(struct ufs_hba *hba,
+	enum query_opcode opcode, enum flag_idn idn, bool *flag_res)
+{
+	int ret;
+	int retries;
+
+	for (retries = 0; retries < QUERY_REQ_RETRIES; retries++) {
+		ret = ufshcd_query_flag(hba, opcode, idn, flag_res);
+		if (ret)
+			dev_dbg(hba->dev,
+				"%s: failed with error %d, retries %d\n",
+				__func__, ret, retries);
+		else
+			break;
+	}
+
+	if (ret)
+		dev_err(hba->dev,
+			"%s: query attribute, opcode %d, idn %d, failed with error %d after %d retries\n",
+			__func__, (int)opcode, (int)idn, ret, retries);
+	return ret;
+}
+
+static int ufshcd_reset_ctmwb(struct ufs_hba *hba, bool force)
+{
+	int err = 0;
+
+	if (!hba_ctmwb.support_ctmwb)
+		return 0;
+
+	if (ufshcd_is_ctmwb_off(hba_ctmwb)) {
+		dev_info(hba->dev, "%s: turbo write already disabled. ctmwb_state = %d\n",
+			__func__, hba_ctmwb.ufs_ctmwb_state);
+		return 0;
+	}
+
+	if (ufshcd_is_ctmwb_err(hba_ctmwb))
+		dev_err(hba->dev, "%s: previous turbo write control was failed.\n",
+			__func__);
+
+	if (force)
+		err = ufshcd_query_flag_retry(hba, UPIU_QUERY_OPCODE_CLEAR_FLAG,
+				QUERY_FLAG_IDN_WB_EN, NULL);
+
+	if (err) {
+		ufshcd_set_ctmwb_err(hba_ctmwb);
+		dev_err(hba->dev, "%s: disable turbo write failed. err = %d\n",
+			__func__, err);
+	} else {
+		ufshcd_set_ctmwb_off(hba_ctmwb);
+		dev_info(hba->dev, "%s: ufs turbo write disabled \n", __func__);
+	}
+
+	return 0;
+}
+
+static int ufshcd_get_ctmwb_buf_status(struct ufs_hba *hba, u32 *status)
+{
+	return ufshcd_query_attr_retry(hba, UPIU_QUERY_OPCODE_READ_ATTR,
+			QUERY_ATTR_IDN_AVAIL_WB_BUFF_SIZE, 0, 0, status);
+}
+
+static int ufshcd_ctmwb_manual_flush_ctrl(struct ufs_hba *hba, int en)
+{
+	int err = 0;
+
+	dev_info(hba->dev, "%s: %sable turbo write manual flush\n",
+				__func__, en ? "en" : "dis");
+	if (en) {
+		err = ufshcd_query_flag_retry(hba, UPIU_QUERY_OPCODE_SET_FLAG,
+					QUERY_FLAG_IDN_WB_BUFF_FLUSH_EN, NULL);
+		if (err)
+			dev_err(hba->dev, "%s: enable turbo write failed. err = %d\n",
+				__func__, err);
+	} else {
+		err = ufshcd_query_flag_retry(hba, UPIU_QUERY_OPCODE_CLEAR_FLAG,
+					QUERY_FLAG_IDN_WB_BUFF_FLUSH_EN, NULL);
+		if (err)
+			dev_err(hba->dev, "%s: disable turbo write failed. err = %d\n",
+				__func__, err);
+	}
+
+	return err;
+}
+
+static int ufshcd_ctmwb_flush_ctrl(struct ufs_hba *hba)
+{
+	int err = 0;
+	u32 curr_status = 0;
+
+	err = ufshcd_get_ctmwb_buf_status(hba, &curr_status);
+
+	if (!err && (curr_status <= UFS_WB_MANUAL_FLUSH_THRESHOLD)) {
+		dev_info(hba->dev, "%s: enable ctmwb manual flush, buf status : %d\n",
+				__func__, curr_status);
+		scsi_block_requests(hba->host);
+		err = ufshcd_ctmwb_manual_flush_ctrl(hba, 1);
+		if (!err) {
+			mdelay(100);
+			err = ufshcd_ctmwb_manual_flush_ctrl(hba, 0);
+			if (err)
+				dev_err(hba->dev, "%s: disable ctmwb manual flush failed. err = %d\n",
+						__func__, err);
+		} else
+			dev_err(hba->dev, "%s: enable ctmwb manual flush failed. err = %d\n",
+					__func__, err);
+		scsi_unblock_requests(hba->host);
+	}
+	return err;
+}
+
+static int ufshcd_ctmwb_ctrl(struct ufs_hba *hba, bool enable)
+{
+	int err;
+#if 0
+	if (!hba->support_ctmwb)
+		return;
+
+	if (hba->pm_op_in_progress) {
+		dev_err(hba->dev, "%s: ctmwb ctrl during pm operation is not allowed.\n",
+			__func__);
+		return;
+	}
+
+	if (hba->ufshcd_state != UFSHCD_STATE_OPERATIONAL) {
+		dev_err(hba->dev, "%s: ufs host is not available.\n",
+			__func__);
+		return;
+	}
+	if (ufshcd_is_ctmwb_err(hba_ctmwb))
+		dev_err(hba->dev, "%s: previous turbo write control was failed.\n",
+			__func__);
+#endif
+	if (enable) {
+		if (ufshcd_is_ctmwb_on(hba_ctmwb)) {
+			dev_err(hba->dev, "%s: turbo write already enabled. ctmwb_state = %d\n",
+				__func__, hba_ctmwb.ufs_ctmwb_state);
+			return 0;
+		}
+		pm_runtime_get_sync(hba->dev);
+		err = ufshcd_query_flag_retry(hba, UPIU_QUERY_OPCODE_SET_FLAG,
+					QUERY_FLAG_IDN_WB_EN, NULL);
+		if (err) {
+			ufshcd_set_ctmwb_err(hba_ctmwb);
+			dev_err(hba->dev, "%s: enable turbo write failed. err = %d\n",
+				__func__, err);
+		} else {
+			ufshcd_set_ctmwb_on(hba_ctmwb);
+			dev_info(hba->dev, "%s: ufs turbo write enabled \n", __func__);
+		}
+	} else {
+		if (ufshcd_is_ctmwb_off(hba_ctmwb)) {
+			dev_err(hba->dev, "%s: turbo write already disabled. ctmwb_state = %d\n",
+				__func__, hba_ctmwb.ufs_ctmwb_state);
+			return 0;
+		}
+		pm_runtime_get_sync(hba->dev);
+		err = ufshcd_query_flag_retry(hba, UPIU_QUERY_OPCODE_CLEAR_FLAG,
+					QUERY_FLAG_IDN_WB_EN, NULL);
+		if (err) {
+			ufshcd_set_ctmwb_err(hba_ctmwb);
+			dev_err(hba->dev, "%s: disable turbo write failed. err = %d\n",
+				__func__, err);
+		} else {
+			ufshcd_set_ctmwb_off(hba_ctmwb);
+			dev_info(hba->dev, "%s: ufs turbo write disabled \n", __func__);
+		}
+	}
+
+	pm_runtime_put_sync(hba->dev);
+
+	return 0;
+}
+
+/**
+ * ufshcd_get_ctmwbbuf_unit - get ctmwb buffer alloc units
+ * @sdev: pointer to SCSI device
+ *
+ * Read dLUNumTurboWriteBufferAllocUnits in UNIT Descriptor
+ * to check if LU supports turbo write feature
+ */
+static int ufshcd_get_ctmwbbuf_unit(struct ufs_hba *hba)
+{
+	struct scsi_device *sdev = hba->sdev_ufs_device;
+	struct ufshba_ctmwb *hba_ctmwb = (struct ufshba_ctmwb *)hba->wb_ops;
+	int ret = 0;
+
+	u32 dLUNumTurboWriteBufferAllocUnits = 0;
+	u8 desc_buf[4];
+
+	if (!hba_ctmwb->support_ctmwb)
+		return 0;
+
+	ret = ufshcd_read_unit_desc_param(hba,
+			ufshcd_scsi_to_upiu_lun(sdev->lun),
+			UNIT_DESC_PARAM_WB_BUF_ALLOC_UNITS,
+			desc_buf,
+			sizeof(dLUNumTurboWriteBufferAllocUnits));
+
+	/* Some WLUN doesn't support unit descriptor */
+	if ((ret == -EOPNOTSUPP) || scsi_is_wlun(sdev->lun)){
+		hba_ctmwb->support_ctmwb_lu = false;
+		dev_info(hba->dev,"%s: do not support WB\n", __func__);
+		return 0;
+	}
+
+	dLUNumTurboWriteBufferAllocUnits = ((desc_buf[0] << 24)|
+			(desc_buf[1] << 16) |
+			(desc_buf[2] << 8) |
+			desc_buf[3]);
+
+	if (dLUNumTurboWriteBufferAllocUnits) {
+		hba_ctmwb->support_ctmwb_lu = true;
+		dev_info(hba->dev, "%s: LU %d supports ctmwb, ctmwbbuf unit : 0x%x\n",
+				__func__, (int)sdev->lun, dLUNumTurboWriteBufferAllocUnits);
+	} else
+		hba_ctmwb->support_ctmwb_lu = false;
+
+	return 0;
+}
+
+static inline int ufshcd_ctmwb_toggle_flush(struct ufs_hba *hba, enum ufs_pm_op pm_op)
+{
+	ufshcd_ctmwb_flush_ctrl(hba);
+
+	if (ufshcd_is_system_pm(pm_op))
+		ufshcd_reset_ctmwb(hba, true);
+
+	return 0;
+}
+
+static struct ufs_wb_ops exynos_ctmwb_ops = {
+	.wb_toggle_flush_vendor = ufshcd_ctmwb_toggle_flush,
+	.wb_alloc_units_vendor = ufshcd_get_ctmwbbuf_unit,
+	.wb_ctrl_vendor = ufshcd_ctmwb_ctrl,
+	.wb_reset_vendor = ufshcd_reset_ctmwb,
+};
+
+struct ufs_wb_ops *ufshcd_ctmwb_init(void)
+{
+	hba_ctmwb.support_ctmwb = 1;
+
+	return &exynos_ctmwb_ops;
+}
+EXPORT_SYMBOL_GPL(ufshcd_ctmwb_init);
+
diff --git a/drivers/scsi/ufs/ufs_ctmwb.h b/drivers/scsi/ufs/ufs_ctmwb.h
new file mode 100644
index 000000000000..073e21a4900b
--- /dev/null
+++ b/drivers/scsi/ufs/ufs_ctmwb.h
@@ -0,0 +1,27 @@
+#ifndef _UFS_CTMWB_H_
+#define _UFS_CTMWB_H_
+
+enum ufs_ctmwb_state {
+       UFS_WB_OFF_STATE	= 0,    /* turbo write disabled state */
+       UFS_WB_ON_STATE	= 1,            /* turbo write enabled state */
+       UFS_WB_ERR_STATE	= 2,            /* turbo write error state */
+};
+
+#define ufshcd_is_ctmwb_off(hba) ((hba).ufs_ctmwb_state == UFS_WB_OFF_STATE)
+#define ufshcd_is_ctmwb_on(hba) ((hba).ufs_ctmwb_state == UFS_WB_ON_STATE)
+#define ufshcd_is_ctmwb_err(hba) ((hba).ufs_ctmwb_state == UFS_WB_ERR_STATE)
+#define ufshcd_set_ctmwb_off(hba) ((hba).ufs_ctmwb_state = UFS_WB_OFF_STATE)
+#define ufshcd_set_ctmwb_on(hba) ((hba).ufs_ctmwb_state = UFS_WB_ON_STATE)
+#define ufshcd_set_ctmwb_err(hba) ((hba).ufs_ctmwb_state = UFS_WB_ERR_STATE)
+
+#define UFS_WB_MANUAL_FLUSH_THRESHOLD	5
+
+struct ufshba_ctmwb {
+	enum ufs_ctmwb_state ufs_ctmwb_state;
+	bool support_ctmwb;
+
+	bool support_ctmwb_lu;
+};
+
+struct ufs_wb_ops *ufshcd_ctmwb_init(void);
+#endif
-- 
2.26.0

Re: [RFC PATCH v2 1/3] scsi: ufs: modify write booster

[Asutosh Das (asd)]

On 7/20/2020 3:40 AM, SEO HOYOUNG wrote:
> Add vendor specific functions for WB
> Use callback additional setting when use write booster.
> 
> Signed-off-by: SEO HOYOUNG <hy50.seo@samsung.com>
> ---
>   drivers/scsi/ufs/ufshcd.c | 23 ++++++++++++++++-----
>   drivers/scsi/ufs/ufshcd.h | 43 +++++++++++++++++++++++++++++++++++++++
>   2 files changed, 61 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index efc0a6cbfe22..efa16bf4fd76 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -3306,11 +3306,11 @@ int ufshcd_read_string_desc(struct ufs_hba *hba, u8 desc_index,
>    *
>    * Return 0 in case of success, non-zero otherwise
>    */
> -static inline int ufshcd_read_unit_desc_param(struct ufs_hba *hba,
> -					      int lun,
> -					      enum unit_desc_param param_offset,
> -					      u8 *param_read_buf,
> -					      u32 param_size)
> +int ufshcd_read_unit_desc_param(struct ufs_hba *hba,
> +				int lun,
> +				enum unit_desc_param param_offset,
> +				u8 *param_read_buf,
> +				u32 param_size)
>   {
>   	/*
>   	 * Unit descriptors are only available for general purpose LUs (LUN id
> @@ -3322,6 +3322,7 @@ static inline int ufshcd_read_unit_desc_param(struct ufs_hba *hba,
>   	return ufshcd_read_desc_param(hba, QUERY_DESC_IDN_UNIT, lun,
>   				      param_offset, param_read_buf, param_size);
>   }
> +EXPORT_SYMBOL_GPL(ufshcd_read_unit_desc_param);
>   
>   static int ufshcd_get_ref_clk_gating_wait(struct ufs_hba *hba)
>   {
> @@ -5257,6 +5258,10 @@ static int ufshcd_wb_ctrl(struct ufs_hba *hba, bool enable)
>   
>   	if (!(enable ^ hba->wb_enabled))
>   		return 0;
> +
> +	if (!ufshcd_wb_ctrl_vendor(hba, enable))
> +		return 0;
> +
>   	if (enable)
>   		opcode = UPIU_QUERY_OPCODE_SET_FLAG;
>   	else
> @@ -6610,6 +6615,8 @@ static int ufshcd_reset_and_restore(struct ufs_hba *hba)
>   	int err = 0;
>   	int retries = MAX_HOST_RESET_RETRIES;
>   
> +	ufshcd_reset_vendor(hba);
> +
>   	do {
>   		/* Reset the attached device */
>   		ufshcd_vops_device_reset(hba);
> @@ -6903,6 +6910,9 @@ static void ufshcd_wb_probe(struct ufs_hba *hba, u8 *desc_buf)
>   	if (!(dev_info->d_ext_ufs_feature_sup & UFS_DEV_WRITE_BOOSTER_SUP))
>   		goto wb_disabled;
>   
> +	if (!ufshcd_wb_alloc_units_vendor(hba))
> +		return;
I didn't understand this check. Have you considered this - 
ufshcd_is_wb_allowed(...).
> +
>   	/*
>   	 * WB may be supported but not configured while provisioning.
>   	 * The spec says, in dedicated wb buffer mode,
> @@ -8260,6 +8270,7 @@ static int ufshcd_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op)
>   			/* make sure that auto bkops is disabled */
>   			ufshcd_disable_auto_bkops(hba);
>   		}
> +
Unnecessary new line, perhaps?
>   		/*
>   		 * If device needs to do BKOP or WB buffer flush during
>   		 * Hibern8, keep device power mode as "active power mode"
> @@ -8273,6 +8284,8 @@ static int ufshcd_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op)
>   			ufshcd_wb_need_flush(hba));
>   	}
>   
> +	ufshcd_wb_toggle_flush_vendor(hba, pm_op);
> +
>   	if (req_dev_pwr_mode != hba->curr_dev_pwr_mode) {
>   		if ((ufshcd_is_runtime_pm(pm_op) && !hba->auto_bkops_enabled) ||
>   		    !ufshcd_is_runtime_pm(pm_op)) {
> diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
> index 656c0691c858..deb9577e0eaa 100644
> --- a/drivers/scsi/ufs/ufshcd.h
> +++ b/drivers/scsi/ufs/ufshcd.h
> @@ -254,6 +254,13 @@ struct ufs_pwr_mode_info {
>   	struct ufs_pa_layer_attr info;
>   };
>   
> +struct ufs_wb_ops {
> +	int (*wb_toggle_flush_vendor)(struct ufs_hba *hba, enum ufs_pm_op pm_op);
> +	int (*wb_alloc_units_vendor)(struct ufs_hba *hba);
> +	int (*wb_ctrl_vendor)(struct ufs_hba *hba, bool enable);
> +	int (*wb_reset_vendor)(struct ufs_hba *hba, bool force);
> +};
> +
>   /**
>    * struct ufs_hba_variant_ops - variant specific callbacks
>    * @name: variant name
> @@ -752,6 +759,7 @@ struct ufs_hba {
>   	struct request_queue	*bsg_queue;
>   	bool wb_buf_flush_enabled;
>   	bool wb_enabled;
> +	struct ufs_wb_ops *wb_ops;
>   	struct delayed_work rpm_dev_flush_recheck_work;
>   
>   #ifdef CONFIG_SCSI_UFS_CRYPTO
> @@ -1004,6 +1012,10 @@ int ufshcd_exec_raw_upiu_cmd(struct ufs_hba *hba,
>   			     u8 *desc_buff, int *buff_len,
>   			     enum query_opcode desc_op);
>   
> +int ufshcd_read_unit_desc_param(struct ufs_hba *hba,
> +				int lun, enum unit_desc_param param_offset,
> +				u8 *param_read_buf, u32 param_size);
> +
>   /* Wrapper functions for safely calling variant operations */
>   static inline const char *ufshcd_get_var_name(struct ufs_hba *hba)
>   {
> @@ -1181,4 +1193,35 @@ static inline u8 ufshcd_scsi_to_upiu_lun(unsigned int scsi_lun)
>   int ufshcd_dump_regs(struct ufs_hba *hba, size_t offset, size_t len,
>   		     const char *prefix);
>   
> +static inline int ufshcd_wb_toggle_flush_vendor(struct ufs_hba *hba, enum ufs_pm_op pm_op)
> +{
> +	if (!hba->wb_ops || !hba->wb_ops->wb_toggle_flush_vendor)
> +		return -1;
> +
> +	return hba->wb_ops->wb_toggle_flush_vendor(hba, pm_op);
> +}
> +
> +static int ufshcd_wb_alloc_units_vendor(struct ufs_hba *hba)
> +{
> +	if (!hba->wb_ops || !hba->wb_ops->wb_alloc_units_vendor)
> +		return -1;
> +
> +	return hba->wb_ops->wb_alloc_units_vendor(hba);
> +}
> +
> +static int ufshcd_wb_ctrl_vendor(struct ufs_hba *hba, bool enable)
> +{
> +	if (!hba->wb_ops || !hba->wb_ops->wb_ctrl_vendor)
> +		return -1;
> +
> +	return hba->wb_ops->wb_ctrl_vendor(hba, enable);
> +}
> +
> +static int ufshcd_reset_vendor(struct ufs_hba *hba)
> +{
> +	if (!hba->wb_ops || !hba->wb_ops->wb_reset_vendor)
> +		return -1;
> +
> +	return hba->wb_ops->wb_reset_vendor(hba, false);
> +}
>   #endif /* End of Header */
> 


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

Re: [RFC PATCH v2 3/3] scsi: ufs: add vendor specific write booster To support the fuction of writebooster by vendor. The WB behavior that the vendor wants is slightly different. But we have to support it

[Asutosh Das (asd)]

On 7/20/2020 3:40 AM, SEO HOYOUNG wrote:
> Signed-off-by: SEO HOYOUNG <hy50.seo@samsung.com>
> ---
>   drivers/scsi/ufs/Makefile     |   1 +
>   drivers/scsi/ufs/ufs-exynos.c |   6 +
>   drivers/scsi/ufs/ufs_ctmwb.c  | 279 ++++++++++++++++++++++++++++++++++
>   drivers/scsi/ufs/ufs_ctmwb.h  |  27 ++++
>   4 files changed, 313 insertions(+)
>   create mode 100644 drivers/scsi/ufs/ufs_ctmwb.c
>   create mode 100644 drivers/scsi/ufs/ufs_ctmwb.h
> 
> diff --git a/drivers/scsi/ufs/Makefile b/drivers/scsi/ufs/Makefile
> index 9810963bc049..b1ba36c7d66f 100644
> --- a/drivers/scsi/ufs/Makefile
> +++ b/drivers/scsi/ufs/Makefile
> @@ -5,6 +5,7 @@ obj-$(CONFIG_SCSI_UFS_DWC_TC_PLATFORM) += tc-dwc-g210-pltfrm.o ufshcd-dwc.o tc-d
>   obj-$(CONFIG_SCSI_UFS_CDNS_PLATFORM) += cdns-pltfrm.o
>   obj-$(CONFIG_SCSI_UFS_QCOM) += ufs-qcom.o
>   obj-$(CONFIG_SCSI_UFS_EXYNOS) += ufs-exynos.o
> +obj-$(CONFIG_SCSI_UFS_VENDOR_WB) += ufs_ctmwb.o
>   obj-$(CONFIG_SCSI_UFSHCD) += ufshcd-core.o
>   ufshcd-core-y				+= ufshcd.o ufs-sysfs.o
>   ufshcd-core-$(CONFIG_SCSI_UFS_BSG)	+= ufs_bsg.o
> diff --git a/drivers/scsi/ufs/ufs-exynos.c b/drivers/scsi/ufs/ufs-exynos.c
> index 32b61ba77241..f127f5f2bf36 100644
> --- a/drivers/scsi/ufs/ufs-exynos.c
> +++ b/drivers/scsi/ufs/ufs-exynos.c
> @@ -22,6 +22,9 @@
>   

To me it looks like, you want to have your own flush policy & 
initializations etc, is that understanding correct?
I don't understand why though. The current implementation is spec 
compliant. If there're benefits that you see in this implementation, 
please highlight those. It'd be interesting to see that.


>   #include "ufs-exynos.h"
>   
> +#ifdef CONFIG_SCSI_UFS_VENDOR_WB
> +#include "ufs_ctmwb.h"
> +#endif
>   /*
>    * Exynos's Vendor specific registers for UFSHCI
>    */
> @@ -989,6 +992,9 @@ static int exynos_ufs_init(struct ufs_hba *hba)
>   		goto phy_off;
>   
>   	ufs->hba = hba;
> +#ifdef CONFIG_SCSI_UFS_VENDOR_WB
> +	ufs->hba->wb_ops = ufshcd_ctmwb_init();
> +#endif
>   	ufs->opts = ufs->drv_data->opts;
>   	ufs->rx_sel_idx = PA_MAXDATALANES;
>   	if (ufs->opts & EXYNOS_UFS_OPT_BROKEN_RX_SEL_IDX)
> diff --git a/drivers/scsi/ufs/ufs_ctmwb.c b/drivers/scsi/ufs/ufs_ctmwb.c
> new file mode 100644
> index 000000000000..ab39f40721ae
> --- /dev/null
> +++ b/drivers/scsi/ufs/ufs_ctmwb.c
> @@ -0,0 +1,279 @@
> +#include "ufshcd.h"
> +#include "ufshci.h"
> +#include "ufs_ctmwb.h"
> +
> +static struct ufshba_ctmwb hba_ctmwb;
> +
> +/* Query request retries */
> +#define QUERY_REQ_RETRIES 3
> +
> +static int ufshcd_query_attr_retry(struct ufs_hba *hba,
> +	enum query_opcode opcode, enum attr_idn idn, u8 index, u8 selector,
> +	u32 *attr_val)
> +{
> +	int ret = 0;
> +	u32 retries;
> +
> +	 for (retries = QUERY_REQ_RETRIES; retries > 0; retries--) {
> +		ret = ufshcd_query_attr(hba, opcode, idn, index,
> +						selector, attr_val);
> +		if (ret)
> +			dev_dbg(hba->dev, "%s: failed with error %d, retries %d\n",
> +				__func__, ret, retries);
> +		else
> +			break;
> +	}
> +
> +	if (ret)
> +		dev_err(hba->dev,
> +			"%s: query attribute, idn %d, failed with error %d after %d retires\n",
> +			__func__, idn, ret, QUERY_REQ_RETRIES);
> +	return ret;
> +}
> +
> +static int ufshcd_query_flag_retry(struct ufs_hba *hba,
> +	enum query_opcode opcode, enum flag_idn idn, bool *flag_res)
> +{
> +	int ret;
> +	int retries;
> +
> +	for (retries = 0; retries < QUERY_REQ_RETRIES; retries++) {
> +		ret = ufshcd_query_flag(hba, opcode, idn, flag_res);
> +		if (ret)
> +			dev_dbg(hba->dev,
> +				"%s: failed with error %d, retries %d\n",
> +				__func__, ret, retries);
> +		else
> +			break;
> +	}
> +
> +	if (ret)
> +		dev_err(hba->dev,
> +			"%s: query attribute, opcode %d, idn %d, failed with error %d after %d retries\n",
> +			__func__, (int)opcode, (int)idn, ret, retries);
> +	return ret;
> +}
> +
> +static int ufshcd_reset_ctmwb(struct ufs_hba *hba, bool force)
> +{
> +	int err = 0;
> +
> +	if (!hba_ctmwb.support_ctmwb)
> +		return 0;
> +
> +	if (ufshcd_is_ctmwb_off(hba_ctmwb)) {
> +		dev_info(hba->dev, "%s: turbo write already disabled. ctmwb_state = %d\n",
> +			__func__, hba_ctmwb.ufs_ctmwb_state);
> +		return 0;
> +	}
> +
> +	if (ufshcd_is_ctmwb_err(hba_ctmwb))
> +		dev_err(hba->dev, "%s: previous turbo write control was failed.\n",
> +			__func__);
> +
> +	if (force)
> +		err = ufshcd_query_flag_retry(hba, UPIU_QUERY_OPCODE_CLEAR_FLAG,
> +				QUERY_FLAG_IDN_WB_EN, NULL);
> +
> +	if (err) {
> +		ufshcd_set_ctmwb_err(hba_ctmwb);
> +		dev_err(hba->dev, "%s: disable turbo write failed. err = %d\n",
> +			__func__, err);
> +	} else {
> +		ufshcd_set_ctmwb_off(hba_ctmwb);
> +		dev_info(hba->dev, "%s: ufs turbo write disabled \n", __func__);
> +	}
> +
> +	return 0;
> +}
> +
> +static int ufshcd_get_ctmwb_buf_status(struct ufs_hba *hba, u32 *status)
> +{
> +	return ufshcd_query_attr_retry(hba, UPIU_QUERY_OPCODE_READ_ATTR,
> +			QUERY_ATTR_IDN_AVAIL_WB_BUFF_SIZE, 0, 0, status);
> +}
> +
> +static int ufshcd_ctmwb_manual_flush_ctrl(struct ufs_hba *hba, int en)
> +{
> +	int err = 0;
> +
> +	dev_info(hba->dev, "%s: %sable turbo write manual flush\n",
> +				__func__, en ? "en" : "dis");
> +	if (en) {
> +		err = ufshcd_query_flag_retry(hba, UPIU_QUERY_OPCODE_SET_FLAG,
> +					QUERY_FLAG_IDN_WB_BUFF_FLUSH_EN, NULL);
> +		if (err)
> +			dev_err(hba->dev, "%s: enable turbo write failed. err = %d\n",
> +				__func__, err);
> +	} else {
> +		err = ufshcd_query_flag_retry(hba, UPIU_QUERY_OPCODE_CLEAR_FLAG,
> +					QUERY_FLAG_IDN_WB_BUFF_FLUSH_EN, NULL);
> +		if (err)
> +			dev_err(hba->dev, "%s: disable turbo write failed. err = %d\n",
> +				__func__, err);
> +	}
> +
> +	return err;
> +}
> +
> +static int ufshcd_ctmwb_flush_ctrl(struct ufs_hba *hba)
> +{
> +	int err = 0;
> +	u32 curr_status = 0;
> +
> +	err = ufshcd_get_ctmwb_buf_status(hba, &curr_status);
> +
> +	if (!err && (curr_status <= UFS_WB_MANUAL_FLUSH_THRESHOLD)) {
> +		dev_info(hba->dev, "%s: enable ctmwb manual flush, buf status : %d\n",
> +				__func__, curr_status);
> +		scsi_block_requests(hba->host);
> +		err = ufshcd_ctmwb_manual_flush_ctrl(hba, 1);
> +		if (!err) {
> +			mdelay(100);
> +			err = ufshcd_ctmwb_manual_flush_ctrl(hba, 0);
> +			if (err)
> +				dev_err(hba->dev, "%s: disable ctmwb manual flush failed. err = %d\n",
> +						__func__, err);
> +		} else
> +			dev_err(hba->dev, "%s: enable ctmwb manual flush failed. err = %d\n",
> +					__func__, err);
> +		scsi_unblock_requests(hba->host);
> +	}
> +	return err;
> +}
> +
> +static int ufshcd_ctmwb_ctrl(struct ufs_hba *hba, bool enable)
> +{
> +	int err;
> +#if 0
Did you miss removing these #if 0?

> +	if (!hba->support_ctmwb)
> +		return;
> +
> +	if (hba->pm_op_in_progress) {
> +		dev_err(hba->dev, "%s: ctmwb ctrl during pm operation is not allowed.\n",
> +			__func__);
> +		return;
> +	}
> +
> +	if (hba->ufshcd_state != UFSHCD_STATE_OPERATIONAL) {
> +		dev_err(hba->dev, "%s: ufs host is not available.\n",
> +			__func__);
> +		return;
> +	}
> +	if (ufshcd_is_ctmwb_err(hba_ctmwb))
> +		dev_err(hba->dev, "%s: previous turbo write control was failed.\n",
> +			__func__);
> +#endif
> +	if (enable) {
> +		if (ufshcd_is_ctmwb_on(hba_ctmwb)) {
> +			dev_err(hba->dev, "%s: turbo write already enabled. ctmwb_state = %d\n",
> +				__func__, hba_ctmwb.ufs_ctmwb_state);
> +			return 0;
> +		}
> +		pm_runtime_get_sync(hba->dev);
> +		err = ufshcd_query_flag_retry(hba, UPIU_QUERY_OPCODE_SET_FLAG,
> +					QUERY_FLAG_IDN_WB_EN, NULL);
> +		if (err) {
> +			ufshcd_set_ctmwb_err(hba_ctmwb);
> +			dev_err(hba->dev, "%s: enable turbo write failed. err = %d\n",
> +				__func__, err);
> +		} else {
> +			ufshcd_set_ctmwb_on(hba_ctmwb);
> +			dev_info(hba->dev, "%s: ufs turbo write enabled \n", __func__);
> +		}
> +	} else {
> +		if (ufshcd_is_ctmwb_off(hba_ctmwb)) {
> +			dev_err(hba->dev, "%s: turbo write already disabled. ctmwb_state = %d\n",
> +				__func__, hba_ctmwb.ufs_ctmwb_state);
> +			return 0;
> +		}
> +		pm_runtime_get_sync(hba->dev);
> +		err = ufshcd_query_flag_retry(hba, UPIU_QUERY_OPCODE_CLEAR_FLAG,
> +					QUERY_FLAG_IDN_WB_EN, NULL);
> +		if (err) {
> +			ufshcd_set_ctmwb_err(hba_ctmwb);
> +			dev_err(hba->dev, "%s: disable turbo write failed. err = %d\n",
> +				__func__, err);
> +		} else {
> +			ufshcd_set_ctmwb_off(hba_ctmwb);
> +			dev_info(hba->dev, "%s: ufs turbo write disabled \n", __func__);
What is 'turbo write'?
> +		}
> +	}
> +
> +	pm_runtime_put_sync(hba->dev);
> +
> +	return 0;
> +}
> +
> +/**
> + * ufshcd_get_ctmwbbuf_unit - get ctmwb buffer alloc units
> + * @sdev: pointer to SCSI device
> + *
> + * Read dLUNumTurboWriteBufferAllocUnits in UNIT Descriptor
> + * to check if LU supports turbo write feature
> + */
> +static int ufshcd_get_ctmwbbuf_unit(struct ufs_hba *hba)
> +{
> +	struct scsi_device *sdev = hba->sdev_ufs_device;
> +	struct ufshba_ctmwb *hba_ctmwb = (struct ufshba_ctmwb *)hba->wb_ops;
> +	int ret = 0;
> +
> +	u32 dLUNumTurboWriteBufferAllocUnits = 0;
> +	u8 desc_buf[4];
> +
> +	if (!hba_ctmwb->support_ctmwb)
> +		return 0;
> +
> +	ret = ufshcd_read_unit_desc_param(hba,
> +			ufshcd_scsi_to_upiu_lun(sdev->lun),
> +			UNIT_DESC_PARAM_WB_BUF_ALLOC_UNITS,
> +			desc_buf,
> +			sizeof(dLUNumTurboWriteBufferAllocUnits));
> +
> +	/* Some WLUN doesn't support unit descriptor */
> +	if ((ret == -EOPNOTSUPP) || scsi_is_wlun(sdev->lun)){
> +		hba_ctmwb->support_ctmwb_lu = false;
> +		dev_info(hba->dev,"%s: do not support WB\n", __func__);
> +		return 0;
> +	}
> +
> +	dLUNumTurboWriteBufferAllocUnits = ((desc_buf[0] << 24)|
> +			(desc_buf[1] << 16) |
> +			(desc_buf[2] << 8) |
> +			desc_buf[3]);
> +
> +	if (dLUNumTurboWriteBufferAllocUnits) {
> +		hba_ctmwb->support_ctmwb_lu = true;
> +		dev_info(hba->dev, "%s: LU %d supports ctmwb, ctmwbbuf unit : 0x%x\n",
> +				__func__, (int)sdev->lun, dLUNumTurboWriteBufferAllocUnits);
> +	} else
> +		hba_ctmwb->support_ctmwb_lu = false;
> +
> +	return 0;
> +}
> +
> +static inline int ufshcd_ctmwb_toggle_flush(struct ufs_hba *hba, enum ufs_pm_op pm_op)
> +{
> +	ufshcd_ctmwb_flush_ctrl(hba);
> +
> +	if (ufshcd_is_system_pm(pm_op))
> +		ufshcd_reset_ctmwb(hba, true);
> +
> +	return 0;
> +}
> +
> +static struct ufs_wb_ops exynos_ctmwb_ops = {
> +	.wb_toggle_flush_vendor = ufshcd_ctmwb_toggle_flush,
> +	.wb_alloc_units_vendor = ufshcd_get_ctmwbbuf_unit,
> +	.wb_ctrl_vendor = ufshcd_ctmwb_ctrl,
> +	.wb_reset_vendor = ufshcd_reset_ctmwb,
> +};
> +
> +struct ufs_wb_ops *ufshcd_ctmwb_init(void)
> +{
> +	hba_ctmwb.support_ctmwb = 1;
> +
> +	return &exynos_ctmwb_ops;
> +}
> +EXPORT_SYMBOL_GPL(ufshcd_ctmwb_init);
> +
> diff --git a/drivers/scsi/ufs/ufs_ctmwb.h b/drivers/scsi/ufs/ufs_ctmwb.h
> new file mode 100644
> index 000000000000..073e21a4900b
> --- /dev/null
> +++ b/drivers/scsi/ufs/ufs_ctmwb.h
> @@ -0,0 +1,27 @@
> +#ifndef _UFS_CTMWB_H_
> +#define _UFS_CTMWB_H_
> +
> +enum ufs_ctmwb_state {
> +       UFS_WB_OFF_STATE	= 0,    /* turbo write disabled state */
> +       UFS_WB_ON_STATE	= 1,            /* turbo write enabled state */
> +       UFS_WB_ERR_STATE	= 2,            /* turbo write error state */
> +};
> +
> +#define ufshcd_is_ctmwb_off(hba) ((hba).ufs_ctmwb_state == UFS_WB_OFF_STATE)
> +#define ufshcd_is_ctmwb_on(hba) ((hba).ufs_ctmwb_state == UFS_WB_ON_STATE)
> +#define ufshcd_is_ctmwb_err(hba) ((hba).ufs_ctmwb_state == UFS_WB_ERR_STATE)
> +#define ufshcd_set_ctmwb_off(hba) ((hba).ufs_ctmwb_state = UFS_WB_OFF_STATE)
> +#define ufshcd_set_ctmwb_on(hba) ((hba).ufs_ctmwb_state = UFS_WB_ON_STATE)
> +#define ufshcd_set_ctmwb_err(hba) ((hba).ufs_ctmwb_state = UFS_WB_ERR_STATE)
> +
> +#define UFS_WB_MANUAL_FLUSH_THRESHOLD	5
> +
> +struct ufshba_ctmwb {
> +	enum ufs_ctmwb_state ufs_ctmwb_state;
> +	bool support_ctmwb;
> +
> +	bool support_ctmwb_lu;
> +};
> +
> +struct ufs_wb_ops *ufshcd_ctmwb_init(void);
> +#endif
> 

-Asutosh

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

RE: [RFC PATCH v2 2/3] scsi: ufs: modify function call name When ufs reset and restore, need to disable write booster

[Avri Altman]

Commit log please.

Thanks,
Avri
> 
> 
> Signed-off-by: SEO HOYOUNG <hy50.seo@samsung.com>
> ---
>  drivers/scsi/ufs/ufshcd.c | 2 +-
>  drivers/scsi/ufs/ufshcd.h | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index efa16bf4fd76..419d3dd7e183 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -6615,7 +6615,7 @@ static int ufshcd_reset_and_restore(struct ufs_hba
> *hba)
>         int err = 0;
>         int retries = MAX_HOST_RESET_RETRIES;
> 
> -       ufshcd_reset_vendor(hba);
> +       ufshcd_wb_reset_vendor(hba);
> 
>         do {
>                 /* Reset the attached device */
> diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
> index deb9577e0eaa..61ae5259c62a 100644
> --- a/drivers/scsi/ufs/ufshcd.h
> +++ b/drivers/scsi/ufs/ufshcd.h
> @@ -1217,7 +1217,7 @@ static int ufshcd_wb_ctrl_vendor(struct ufs_hba
> *hba, bool enable)
>         return hba->wb_ops->wb_ctrl_vendor(hba, enable);
>  }
> 
> -static int ufshcd_reset_vendor(struct ufs_hba *hba)
> +static int ufshcd_wb_reset_vendor(struct ufs_hba *hba)
>  {
>         if (!hba->wb_ops || !hba->wb_ops->wb_reset_vendor)
>                 return -1;
> --
> 2.26.0

RE: [RFC PATCH v2 3/3] scsi: ufs: add vendor specific write booster To support the fuction of writebooster by vendor. The WB behavior that the vendor wants is slightly different. But we have to support it

[Avri Altman]

Commit log please.
Looks like your log slips into your title.

Thanks,
Avri
> 
> 
> Signed-off-by: SEO HOYOUNG <hy50.seo@samsung.com>
> ---
>  drivers/scsi/ufs/Makefile     |   1 +
>  drivers/scsi/ufs/ufs-exynos.c |   6 +
>  drivers/scsi/ufs/ufs_ctmwb.c  | 279 ++++++++++++++++++++++++++++++++++
>  drivers/scsi/ufs/ufs_ctmwb.h  |  27 ++++
>  4 files changed, 313 insertions(+)
>  create mode 100644 drivers/scsi/ufs/ufs_ctmwb.c
>  create mode 100644 drivers/scsi/ufs/ufs_ctmwb.h
> 
> diff --git a/drivers/scsi/ufs/Makefile b/drivers/scsi/ufs/Makefile
> index 9810963bc049..b1ba36c7d66f 100644
> --- a/drivers/scsi/ufs/Makefile
> +++ b/drivers/scsi/ufs/Makefile
> @@ -5,6 +5,7 @@ obj-$(CONFIG_SCSI_UFS_DWC_TC_PLATFORM) += tc-dwc-
> g210-pltfrm.o ufshcd-dwc.o tc-d
>  obj-$(CONFIG_SCSI_UFS_CDNS_PLATFORM) += cdns-pltfrm.o
>  obj-$(CONFIG_SCSI_UFS_QCOM) += ufs-qcom.o
>  obj-$(CONFIG_SCSI_UFS_EXYNOS) += ufs-exynos.o
> +obj-$(CONFIG_SCSI_UFS_VENDOR_WB) += ufs_ctmwb.o
>  obj-$(CONFIG_SCSI_UFSHCD) += ufshcd-core.o
>  ufshcd-core-y                          += ufshcd.o ufs-sysfs.o
>  ufshcd-core-$(CONFIG_SCSI_UFS_BSG)     += ufs_bsg.o
> diff --git a/drivers/scsi/ufs/ufs-exynos.c b/drivers/scsi/ufs/ufs-exynos.c
> index 32b61ba77241..f127f5f2bf36 100644
> --- a/drivers/scsi/ufs/ufs-exynos.c
> +++ b/drivers/scsi/ufs/ufs-exynos.c
> @@ -22,6 +22,9 @@
> 
>  #include "ufs-exynos.h"
> 
> +#ifdef CONFIG_SCSI_UFS_VENDOR_WB
> +#include "ufs_ctmwb.h"
> +#endif
>  /*
>   * Exynos's Vendor specific registers for UFSHCI
>   */
> @@ -989,6 +992,9 @@ static int exynos_ufs_init(struct ufs_hba *hba)
>                 goto phy_off;
> 
>         ufs->hba = hba;
> +#ifdef CONFIG_SCSI_UFS_VENDOR_WB
> +       ufs->hba->wb_ops = ufshcd_ctmwb_init();
> +#endif
>         ufs->opts = ufs->drv_data->opts;
>         ufs->rx_sel_idx = PA_MAXDATALANES;
>         if (ufs->opts & EXYNOS_UFS_OPT_BROKEN_RX_SEL_IDX)
> diff --git a/drivers/scsi/ufs/ufs_ctmwb.c b/drivers/scsi/ufs/ufs_ctmwb.c
> new file mode 100644
> index 000000000000..ab39f40721ae
> --- /dev/null
> +++ b/drivers/scsi/ufs/ufs_ctmwb.c
> @@ -0,0 +1,279 @@
> +#include "ufshcd.h"
> +#include "ufshci.h"
> +#include "ufs_ctmwb.h"
> +
> +static struct ufshba_ctmwb hba_ctmwb;
> +
> +/* Query request retries */
> +#define QUERY_REQ_RETRIES 3
> +
> +static int ufshcd_query_attr_retry(struct ufs_hba *hba,
> +       enum query_opcode opcode, enum attr_idn idn, u8 index, u8 selector,
> +       u32 *attr_val)
> +{
> +       int ret = 0;
> +       u32 retries;
> +
> +        for (retries = QUERY_REQ_RETRIES; retries > 0; retries--) {
> +               ret = ufshcd_query_attr(hba, opcode, idn, index,
> +                                               selector, attr_val);
> +               if (ret)
> +                       dev_dbg(hba->dev, "%s: failed with error %d, retries %d\n",
> +                               __func__, ret, retries);
> +               else
> +                       break;
> +       }
> +
> +       if (ret)
> +               dev_err(hba->dev,
> +                       "%s: query attribute, idn %d, failed with error %d after %d
> retires\n",
> +                       __func__, idn, ret, QUERY_REQ_RETRIES);
> +       return ret;
> +}
> +
> +static int ufshcd_query_flag_retry(struct ufs_hba *hba,
> +       enum query_opcode opcode, enum flag_idn idn, bool *flag_res)
> +{
> +       int ret;
> +       int retries;
> +
> +       for (retries = 0; retries < QUERY_REQ_RETRIES; retries++) {
> +               ret = ufshcd_query_flag(hba, opcode, idn, flag_res);
> +               if (ret)
> +                       dev_dbg(hba->dev,
> +                               "%s: failed with error %d, retries %d\n",
> +                               __func__, ret, retries);
> +               else
> +                       break;
> +       }
> +
> +       if (ret)
> +               dev_err(hba->dev,
> +                       "%s: query attribute, opcode %d, idn %d, failed with error %d
> after %d retries\n",
> +                       __func__, (int)opcode, (int)idn, ret, retries);
> +       return ret;
> +}
> +
> +static int ufshcd_reset_ctmwb(struct ufs_hba *hba, bool force)
> +{
> +       int err = 0;
> +
> +       if (!hba_ctmwb.support_ctmwb)
> +               return 0;
> +
> +       if (ufshcd_is_ctmwb_off(hba_ctmwb)) {
> +               dev_info(hba->dev, "%s: turbo write already disabled. ctmwb_state
> = %d\n",
> +                       __func__, hba_ctmwb.ufs_ctmwb_state);
> +               return 0;
> +       }
> +
> +       if (ufshcd_is_ctmwb_err(hba_ctmwb))
> +               dev_err(hba->dev, "%s: previous turbo write control was failed.\n",
> +                       __func__);
> +
> +       if (force)
> +               err = ufshcd_query_flag_retry(hba,
> UPIU_QUERY_OPCODE_CLEAR_FLAG,
> +                               QUERY_FLAG_IDN_WB_EN, NULL);
> +
> +       if (err) {
> +               ufshcd_set_ctmwb_err(hba_ctmwb);
> +               dev_err(hba->dev, "%s: disable turbo write failed. err = %d\n",
> +                       __func__, err);
> +       } else {
> +               ufshcd_set_ctmwb_off(hba_ctmwb);
> +               dev_info(hba->dev, "%s: ufs turbo write disabled \n", __func__);
> +       }
> +
> +       return 0;
> +}
> +
> +static int ufshcd_get_ctmwb_buf_status(struct ufs_hba *hba, u32 *status)
> +{
> +       return ufshcd_query_attr_retry(hba, UPIU_QUERY_OPCODE_READ_ATTR,
> +                       QUERY_ATTR_IDN_AVAIL_WB_BUFF_SIZE, 0, 0, status);
> +}
> +
> +static int ufshcd_ctmwb_manual_flush_ctrl(struct ufs_hba *hba, int en)
> +{
> +       int err = 0;
> +
> +       dev_info(hba->dev, "%s: %sable turbo write manual flush\n",
> +                               __func__, en ? "en" : "dis");
> +       if (en) {
> +               err = ufshcd_query_flag_retry(hba,
> UPIU_QUERY_OPCODE_SET_FLAG,
> +                                       QUERY_FLAG_IDN_WB_BUFF_FLUSH_EN, NULL);
> +               if (err)
> +                       dev_err(hba->dev, "%s: enable turbo write failed. err = %d\n",
> +                               __func__, err);
> +       } else {
> +               err = ufshcd_query_flag_retry(hba,
> UPIU_QUERY_OPCODE_CLEAR_FLAG,
> +                                       QUERY_FLAG_IDN_WB_BUFF_FLUSH_EN, NULL);
> +               if (err)
> +                       dev_err(hba->dev, "%s: disable turbo write failed. err = %d\n",
> +                               __func__, err);
> +       }
> +
> +       return err;
> +}
> +
> +static int ufshcd_ctmwb_flush_ctrl(struct ufs_hba *hba)
> +{
> +       int err = 0;
> +       u32 curr_status = 0;
> +
> +       err = ufshcd_get_ctmwb_buf_status(hba, &curr_status);
> +
> +       if (!err && (curr_status <= UFS_WB_MANUAL_FLUSH_THRESHOLD)) {
> +               dev_info(hba->dev, "%s: enable ctmwb manual flush, buf status :
> %d\n",
> +                               __func__, curr_status);
> +               scsi_block_requests(hba->host);
> +               err = ufshcd_ctmwb_manual_flush_ctrl(hba, 1);
> +               if (!err) {
> +                       mdelay(100);
> +                       err = ufshcd_ctmwb_manual_flush_ctrl(hba, 0);
> +                       if (err)
> +                               dev_err(hba->dev, "%s: disable ctmwb manual flush failed.
> err = %d\n",
> +                                               __func__, err);
> +               } else
> +                       dev_err(hba->dev, "%s: enable ctmwb manual flush failed. err =
> %d\n",
> +                                       __func__, err);
> +               scsi_unblock_requests(hba->host);
> +       }
> +       return err;
> +}
> +
> +static int ufshcd_ctmwb_ctrl(struct ufs_hba *hba, bool enable)
> +{
> +       int err;
> +#if 0
> +       if (!hba->support_ctmwb)
> +               return;
> +
> +       if (hba->pm_op_in_progress) {
> +               dev_err(hba->dev, "%s: ctmwb ctrl during pm operation is not
> allowed.\n",
> +                       __func__);
> +               return;
> +       }
> +
> +       if (hba->ufshcd_state != UFSHCD_STATE_OPERATIONAL) {
> +               dev_err(hba->dev, "%s: ufs host is not available.\n",
> +                       __func__);
> +               return;
> +       }
> +       if (ufshcd_is_ctmwb_err(hba_ctmwb))
> +               dev_err(hba->dev, "%s: previous turbo write control was failed.\n",
> +                       __func__);
> +#endif
> +       if (enable) {
> +               if (ufshcd_is_ctmwb_on(hba_ctmwb)) {
> +                       dev_err(hba->dev, "%s: turbo write already enabled.
> ctmwb_state = %d\n",
> +                               __func__, hba_ctmwb.ufs_ctmwb_state);
> +                       return 0;
> +               }
> +               pm_runtime_get_sync(hba->dev);
> +               err = ufshcd_query_flag_retry(hba,
> UPIU_QUERY_OPCODE_SET_FLAG,
> +                                       QUERY_FLAG_IDN_WB_EN, NULL);
> +               if (err) {
> +                       ufshcd_set_ctmwb_err(hba_ctmwb);
> +                       dev_err(hba->dev, "%s: enable turbo write failed. err = %d\n",
> +                               __func__, err);
> +               } else {
> +                       ufshcd_set_ctmwb_on(hba_ctmwb);
> +                       dev_info(hba->dev, "%s: ufs turbo write enabled \n",
> __func__);
> +               }
> +       } else {
> +               if (ufshcd_is_ctmwb_off(hba_ctmwb)) {
> +                       dev_err(hba->dev, "%s: turbo write already disabled.
> ctmwb_state = %d\n",
> +                               __func__, hba_ctmwb.ufs_ctmwb_state);
> +                       return 0;
> +               }
> +               pm_runtime_get_sync(hba->dev);
> +               err = ufshcd_query_flag_retry(hba,
> UPIU_QUERY_OPCODE_CLEAR_FLAG,
> +                                       QUERY_FLAG_IDN_WB_EN, NULL);
> +               if (err) {
> +                       ufshcd_set_ctmwb_err(hba_ctmwb);
> +                       dev_err(hba->dev, "%s: disable turbo write failed. err = %d\n",
> +                               __func__, err);
> +               } else {
> +                       ufshcd_set_ctmwb_off(hba_ctmwb);
> +                       dev_info(hba->dev, "%s: ufs turbo write disabled \n",
> __func__);
> +               }
> +       }
> +
> +       pm_runtime_put_sync(hba->dev);
> +
> +       return 0;
> +}
> +
> +/**
> + * ufshcd_get_ctmwbbuf_unit - get ctmwb buffer alloc units
> + * @sdev: pointer to SCSI device
> + *
> + * Read dLUNumTurboWriteBufferAllocUnits in UNIT Descriptor
> + * to check if LU supports turbo write feature
> + */
> +static int ufshcd_get_ctmwbbuf_unit(struct ufs_hba *hba)
> +{
> +       struct scsi_device *sdev = hba->sdev_ufs_device;
> +       struct ufshba_ctmwb *hba_ctmwb = (struct ufshba_ctmwb *)hba-
> >wb_ops;
> +       int ret = 0;
> +
> +       u32 dLUNumTurboWriteBufferAllocUnits = 0;
> +       u8 desc_buf[4];
> +
> +       if (!hba_ctmwb->support_ctmwb)
> +               return 0;
> +
> +       ret = ufshcd_read_unit_desc_param(hba,
> +                       ufshcd_scsi_to_upiu_lun(sdev->lun),
> +                       UNIT_DESC_PARAM_WB_BUF_ALLOC_UNITS,
> +                       desc_buf,
> +                       sizeof(dLUNumTurboWriteBufferAllocUnits));
> +
> +       /* Some WLUN doesn't support unit descriptor */
> +       if ((ret == -EOPNOTSUPP) || scsi_is_wlun(sdev->lun)){
> +               hba_ctmwb->support_ctmwb_lu = false;
> +               dev_info(hba->dev,"%s: do not support WB\n", __func__);
> +               return 0;
> +       }
> +
> +       dLUNumTurboWriteBufferAllocUnits = ((desc_buf[0] << 24)|
> +                       (desc_buf[1] << 16) |
> +                       (desc_buf[2] << 8) |
> +                       desc_buf[3]);
> +
> +       if (dLUNumTurboWriteBufferAllocUnits) {
> +               hba_ctmwb->support_ctmwb_lu = true;
> +               dev_info(hba->dev, "%s: LU %d supports ctmwb, ctmwbbuf unit :
> 0x%x\n",
> +                               __func__, (int)sdev->lun,
> dLUNumTurboWriteBufferAllocUnits);
> +       } else
> +               hba_ctmwb->support_ctmwb_lu = false;
> +
> +       return 0;
> +}
> +
> +static inline int ufshcd_ctmwb_toggle_flush(struct ufs_hba *hba, enum
> ufs_pm_op pm_op)
> +{
> +       ufshcd_ctmwb_flush_ctrl(hba);
> +
> +       if (ufshcd_is_system_pm(pm_op))
> +               ufshcd_reset_ctmwb(hba, true);
> +
> +       return 0;
> +}
> +
> +static struct ufs_wb_ops exynos_ctmwb_ops = {
> +       .wb_toggle_flush_vendor = ufshcd_ctmwb_toggle_flush,
> +       .wb_alloc_units_vendor = ufshcd_get_ctmwbbuf_unit,
> +       .wb_ctrl_vendor = ufshcd_ctmwb_ctrl,
> +       .wb_reset_vendor = ufshcd_reset_ctmwb,
> +};
> +
> +struct ufs_wb_ops *ufshcd_ctmwb_init(void)
> +{
> +       hba_ctmwb.support_ctmwb = 1;
> +
> +       return &exynos_ctmwb_ops;
> +}
> +EXPORT_SYMBOL_GPL(ufshcd_ctmwb_init);
> +
> diff --git a/drivers/scsi/ufs/ufs_ctmwb.h b/drivers/scsi/ufs/ufs_ctmwb.h
> new file mode 100644
> index 000000000000..073e21a4900b
> --- /dev/null
> +++ b/drivers/scsi/ufs/ufs_ctmwb.h
> @@ -0,0 +1,27 @@
> +#ifndef _UFS_CTMWB_H_
> +#define _UFS_CTMWB_H_
> +
> +enum ufs_ctmwb_state {
> +       UFS_WB_OFF_STATE        = 0,    /* turbo write disabled state */
> +       UFS_WB_ON_STATE = 1,            /* turbo write enabled state */
> +       UFS_WB_ERR_STATE        = 2,            /* turbo write error state */
> +};
> +
> +#define ufshcd_is_ctmwb_off(hba) ((hba).ufs_ctmwb_state ==
> UFS_WB_OFF_STATE)
> +#define ufshcd_is_ctmwb_on(hba) ((hba).ufs_ctmwb_state ==
> UFS_WB_ON_STATE)
> +#define ufshcd_is_ctmwb_err(hba) ((hba).ufs_ctmwb_state ==
> UFS_WB_ERR_STATE)
> +#define ufshcd_set_ctmwb_off(hba) ((hba).ufs_ctmwb_state =
> UFS_WB_OFF_STATE)
> +#define ufshcd_set_ctmwb_on(hba) ((hba).ufs_ctmwb_state =
> UFS_WB_ON_STATE)
> +#define ufshcd_set_ctmwb_err(hba) ((hba).ufs_ctmwb_state =
> UFS_WB_ERR_STATE)
> +
> +#define UFS_WB_MANUAL_FLUSH_THRESHOLD  5
> +
> +struct ufshba_ctmwb {
> +       enum ufs_ctmwb_state ufs_ctmwb_state;
> +       bool support_ctmwb;
> +
> +       bool support_ctmwb_lu;
> +};
> +
> +struct ufs_wb_ops *ufshcd_ctmwb_init(void);
> +#endif
> --
> 2.26.0

RE: [RFC PATCH v2 1/3] scsi: ufs: modify write booster

[서호영]

> -----Original Message-----
> From: asutoshd=codeaurora.org@mg.codeaurora.org
> [mailto:asutoshd=codeaurora.org@mg.codeaurora.org] On Behalf Of Asutosh
> Das (asd)
> Sent: Tuesday, July 21, 2020 1:47 AM
> To: SEO HOYOUNG; linux-scsi@vger.kernel.org; alim.akhtar@samsung.com;
> avri.altman@wdc.com; jejb@linux.ibm.com; martin.petersen@oracle.com;
> beanhuo@micron.com; cang@codeaurora.org; bvanassche@acm.org;
> grant.jung@samsung.com
> Subject: Re: [RFC PATCH v2 1/3] scsi: ufs: modify write booster
> 
> On 7/20/2020 3:40 AM, SEO HOYOUNG wrote:
> > Add vendor specific functions for WB
> > Use callback additional setting when use write booster.
> >
> > Signed-off-by: SEO HOYOUNG <hy50.seo@samsung.com>
> > ---
> >   drivers/scsi/ufs/ufshcd.c | 23 ++++++++++++++++-----
> >   drivers/scsi/ufs/ufshcd.h | 43 +++++++++++++++++++++++++++++++++++++++
> >   2 files changed, 61 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> > index efc0a6cbfe22..efa16bf4fd76 100644
> > --- a/drivers/scsi/ufs/ufshcd.c
> > +++ b/drivers/scsi/ufs/ufshcd.c
> > @@ -3306,11 +3306,11 @@ int ufshcd_read_string_desc(struct ufs_hba *hba,
> u8 desc_index,
> >    *
> >    * Return 0 in case of success, non-zero otherwise
> >    */
> > -static inline int ufshcd_read_unit_desc_param(struct ufs_hba *hba,
> > -					      int lun,
> > -					      enum unit_desc_param param_offset,
> > -					      u8 *param_read_buf,
> > -					      u32 param_size)
> > +int ufshcd_read_unit_desc_param(struct ufs_hba *hba,
> > +				int lun,
> > +				enum unit_desc_param param_offset,
> > +				u8 *param_read_buf,
> > +				u32 param_size)
> >   {
> >   	/*
> >   	 * Unit descriptors are only available for general purpose LUs (LUN
> > id @@ -3322,6 +3322,7 @@ static inline int
> ufshcd_read_unit_desc_param(struct ufs_hba *hba,
> >   	return ufshcd_read_desc_param(hba, QUERY_DESC_IDN_UNIT, lun,
> >   				      param_offset, param_read_buf, param_size);
> >   }
> > +EXPORT_SYMBOL_GPL(ufshcd_read_unit_desc_param);
> >
> >   static int ufshcd_get_ref_clk_gating_wait(struct ufs_hba *hba)
> >   {
> > @@ -5257,6 +5258,10 @@ static int ufshcd_wb_ctrl(struct ufs_hba *hba,
> > bool enable)
> >
> >   	if (!(enable ^ hba->wb_enabled))
> >   		return 0;
> > +
> > +	if (!ufshcd_wb_ctrl_vendor(hba, enable))
> > +		return 0;
> > +
> >   	if (enable)
> >   		opcode = UPIU_QUERY_OPCODE_SET_FLAG;
> >   	else
> > @@ -6610,6 +6615,8 @@ static int ufshcd_reset_and_restore(struct ufs_hba
> *hba)
> >   	int err = 0;
> >   	int retries = MAX_HOST_RESET_RETRIES;
> >
> > +	ufshcd_reset_vendor(hba);
> > +
> >   	do {
> >   		/* Reset the attached device */
> >   		ufshcd_vops_device_reset(hba);
> > @@ -6903,6 +6910,9 @@ static void ufshcd_wb_probe(struct ufs_hba *hba,
> u8 *desc_buf)
> >   	if (!(dev_info->d_ext_ufs_feature_sup & UFS_DEV_WRITE_BOOSTER_SUP))
> >   		goto wb_disabled;
> >
> > +	if (!ufshcd_wb_alloc_units_vendor(hba))
> > +		return;
> I didn't understand this check. Have you considered this -
> ufshcd_is_wb_allowed(...).
Ok. I will modify to using this function

> > +
> >   	/*
> >   	 * WB may be supported but not configured while provisioning.
> >   	 * The spec says, in dedicated wb buffer mode, @@ -8260,6 +8270,7
> > @@ static int ufshcd_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op)
> >   			/* make sure that auto bkops is disabled */
> >   			ufshcd_disable_auto_bkops(hba);
> >   		}
> > +
> Unnecessary new line, perhaps?
I will remove new line.
> >   		/*
> >   		 * If device needs to do BKOP or WB buffer flush during
> >   		 * Hibern8, keep device power mode as "active power mode"
> > @@ -8273,6 +8284,8 @@ static int ufshcd_suspend(struct ufs_hba *hba,
> enum ufs_pm_op pm_op)
> >   			ufshcd_wb_need_flush(hba));
> >   	}
> >
> > +	ufshcd_wb_toggle_flush_vendor(hba, pm_op);
> > +
> >   	if (req_dev_pwr_mode != hba->curr_dev_pwr_mode) {
> >   		if ((ufshcd_is_runtime_pm(pm_op) && !hba->auto_bkops_enabled)
> ||
> >   		    !ufshcd_is_runtime_pm(pm_op)) { diff --git
> > a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h index
> > 656c0691c858..deb9577e0eaa 100644
> > --- a/drivers/scsi/ufs/ufshcd.h
> > +++ b/drivers/scsi/ufs/ufshcd.h
> > @@ -254,6 +254,13 @@ struct ufs_pwr_mode_info {
> >   	struct ufs_pa_layer_attr info;
> >   };
> >
> > +struct ufs_wb_ops {
> > +	int (*wb_toggle_flush_vendor)(struct ufs_hba *hba, enum ufs_pm_op
> pm_op);
> > +	int (*wb_alloc_units_vendor)(struct ufs_hba *hba);
> > +	int (*wb_ctrl_vendor)(struct ufs_hba *hba, bool enable);
> > +	int (*wb_reset_vendor)(struct ufs_hba *hba, bool force); };
> > +
> >   /**
> >    * struct ufs_hba_variant_ops - variant specific callbacks
> >    * @name: variant name
> > @@ -752,6 +759,7 @@ struct ufs_hba {
> >   	struct request_queue	*bsg_queue;
> >   	bool wb_buf_flush_enabled;
> >   	bool wb_enabled;
> > +	struct ufs_wb_ops *wb_ops;
> >   	struct delayed_work rpm_dev_flush_recheck_work;
> >
> >   #ifdef CONFIG_SCSI_UFS_CRYPTO
> > @@ -1004,6 +1012,10 @@ int ufshcd_exec_raw_upiu_cmd(struct ufs_hba *hba,
> >   			     u8 *desc_buff, int *buff_len,
> >   			     enum query_opcode desc_op);
> >
> > +int ufshcd_read_unit_desc_param(struct ufs_hba *hba,
> > +				int lun, enum unit_desc_param param_offset,
> > +				u8 *param_read_buf, u32 param_size);
> > +
> >   /* Wrapper functions for safely calling variant operations */
> >   static inline const char *ufshcd_get_var_name(struct ufs_hba *hba)
> >   {
> > @@ -1181,4 +1193,35 @@ static inline u8 ufshcd_scsi_to_upiu_lun(unsigned
> int scsi_lun)
> >   int ufshcd_dump_regs(struct ufs_hba *hba, size_t offset, size_t len,
> >   		     const char *prefix);
> >
> > +static inline int ufshcd_wb_toggle_flush_vendor(struct ufs_hba *hba,
> > +enum ufs_pm_op pm_op) {
> > +	if (!hba->wb_ops || !hba->wb_ops->wb_toggle_flush_vendor)
> > +		return -1;
> > +
> > +	return hba->wb_ops->wb_toggle_flush_vendor(hba, pm_op); }
> > +
> > +static int ufshcd_wb_alloc_units_vendor(struct ufs_hba *hba) {
> > +	if (!hba->wb_ops || !hba->wb_ops->wb_alloc_units_vendor)
> > +		return -1;
> > +
> > +	return hba->wb_ops->wb_alloc_units_vendor(hba);
> > +}
> > +
> > +static int ufshcd_wb_ctrl_vendor(struct ufs_hba *hba, bool enable) {
> > +	if (!hba->wb_ops || !hba->wb_ops->wb_ctrl_vendor)
> > +		return -1;
> > +
> > +	return hba->wb_ops->wb_ctrl_vendor(hba, enable); }
> > +
> > +static int ufshcd_reset_vendor(struct ufs_hba *hba) {
> > +	if (!hba->wb_ops || !hba->wb_ops->wb_reset_vendor)
> > +		return -1;
> > +
> > +	return hba->wb_ops->wb_reset_vendor(hba, false); }
> >   #endif /* End of Header */
> >
> 
> 
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> Linux Foundation Collaborative Project

RE: [RFC PATCH v2 3/3] scsi: ufs: add vendor specific write booster To support the fuction of writebooster by vendor. The WB behavior that the vendor wants is slightly different. But we have to support it

[서호영]

> -----Original Message-----
> From: asutoshd=codeaurora.org@mg.codeaurora.org
> [mailto:asutoshd=codeaurora.org@mg.codeaurora.org] On Behalf Of Asutosh
> Das (asd)
> Sent: Tuesday, July 21, 2020 1:55 AM
> To: SEO HOYOUNG; linux-scsi@vger.kernel.org; alim.akhtar@samsung.com;
> avri.altman@wdc.com; jejb@linux.ibm.com; martin.petersen@oracle.com;
> beanhuo@micron.com; cang@codeaurora.org; bvanassche@acm.org;
> grant.jung@samsung.com
> Subject: Re: [RFC PATCH v2 3/3] scsi: ufs: add vendor specific write
> booster To support the fuction of writebooster by vendor. The WB behavior
> that the vendor wants is slightly different. But we have to support it
> 
> On 7/20/2020 3:40 AM, SEO HOYOUNG wrote:
> > Signed-off-by: SEO HOYOUNG <hy50.seo@samsung.com>
> > ---
> >   drivers/scsi/ufs/Makefile     |   1 +
> >   drivers/scsi/ufs/ufs-exynos.c |   6 +
> >   drivers/scsi/ufs/ufs_ctmwb.c  | 279 ++++++++++++++++++++++++++++++++++
> >   drivers/scsi/ufs/ufs_ctmwb.h  |  27 ++++
> >   4 files changed, 313 insertions(+)
> >   create mode 100644 drivers/scsi/ufs/ufs_ctmwb.c
> >   create mode 100644 drivers/scsi/ufs/ufs_ctmwb.h
> >
> > diff --git a/drivers/scsi/ufs/Makefile b/drivers/scsi/ufs/Makefile
> > index 9810963bc049..b1ba36c7d66f 100644
> > --- a/drivers/scsi/ufs/Makefile
> > +++ b/drivers/scsi/ufs/Makefile
> > @@ -5,6 +5,7 @@ obj-$(CONFIG_SCSI_UFS_DWC_TC_PLATFORM) += tc-dwc-g210-
> pltfrm.o ufshcd-dwc.o tc-d
> >   obj-$(CONFIG_SCSI_UFS_CDNS_PLATFORM) += cdns-pltfrm.o
> >   obj-$(CONFIG_SCSI_UFS_QCOM) += ufs-qcom.o
> >   obj-$(CONFIG_SCSI_UFS_EXYNOS) += ufs-exynos.o
> > +obj-$(CONFIG_SCSI_UFS_VENDOR_WB) += ufs_ctmwb.o
> >   obj-$(CONFIG_SCSI_UFSHCD) += ufshcd-core.o
> >   ufshcd-core-y				+= ufshcd.o ufs-sysfs.o
> >   ufshcd-core-$(CONFIG_SCSI_UFS_BSG)	+= ufs_bsg.o
> > diff --git a/drivers/scsi/ufs/ufs-exynos.c
> > b/drivers/scsi/ufs/ufs-exynos.c index 32b61ba77241..f127f5f2bf36
> > 100644
> > --- a/drivers/scsi/ufs/ufs-exynos.c
> > +++ b/drivers/scsi/ufs/ufs-exynos.c
> > @@ -22,6 +22,9 @@
> >
> 
> To me it looks like, you want to have your own flush policy &
> initializations etc, is that understanding correct?
> I don't understand why though. The current implementation is spec
> compliant. If there're benefits that you see in this implementation,
> please highlight those. It'd be interesting to see that.

Yes. I want to own flush policy, initialization.. 
I already know current implementation is spec compliant.
But some vendor want to change flush policy.
So we modify below code.
Additionally when use below code, we can use WB without UFS 3.1 devices
> 
> 
> >   #include "ufs-exynos.h"
> >
> > +#ifdef CONFIG_SCSI_UFS_VENDOR_WB
> > +#include "ufs_ctmwb.h"
> > +#endif
> >   /*
> >    * Exynos's Vendor specific registers for UFSHCI
> >    */
> > @@ -989,6 +992,9 @@ static int exynos_ufs_init(struct ufs_hba *hba)
> >   		goto phy_off;
> >
> >   	ufs->hba = hba;
> > +#ifdef CONFIG_SCSI_UFS_VENDOR_WB
> > +	ufs->hba->wb_ops = ufshcd_ctmwb_init(); #endif
> >   	ufs->opts = ufs->drv_data->opts;
> >   	ufs->rx_sel_idx = PA_MAXDATALANES;
> >   	if (ufs->opts & EXYNOS_UFS_OPT_BROKEN_RX_SEL_IDX) diff --git
> > a/drivers/scsi/ufs/ufs_ctmwb.c b/drivers/scsi/ufs/ufs_ctmwb.c new file
> > mode 100644 index 000000000000..ab39f40721ae
> > --- /dev/null
> > +++ b/drivers/scsi/ufs/ufs_ctmwb.c
> > @@ -0,0 +1,279 @@
> > +#include "ufshcd.h"
> > +#include "ufshci.h"
> > +#include "ufs_ctmwb.h"
> > +
> > +static struct ufshba_ctmwb hba_ctmwb;
> > +
> > +/* Query request retries */
> > +#define QUERY_REQ_RETRIES 3
> > +
> > +static int ufshcd_query_attr_retry(struct ufs_hba *hba,
> > +	enum query_opcode opcode, enum attr_idn idn, u8 index, u8 selector,
> > +	u32 *attr_val)
> > +{
> > +	int ret = 0;
> > +	u32 retries;
> > +
> > +	 for (retries = QUERY_REQ_RETRIES; retries > 0; retries--) {
> > +		ret = ufshcd_query_attr(hba, opcode, idn, index,
> > +						selector, attr_val);
> > +		if (ret)
> > +			dev_dbg(hba->dev, "%s: failed with error %d,
> retries %d\n",
> > +				__func__, ret, retries);
> > +		else
> > +			break;
> > +	}
> > +
> > +	if (ret)
> > +		dev_err(hba->dev,
> > +			"%s: query attribute, idn %d, failed with error %d
> after %d retires\n",
> > +			__func__, idn, ret, QUERY_REQ_RETRIES);
> > +	return ret;
> > +}
> > +
> > +static int ufshcd_query_flag_retry(struct ufs_hba *hba,
> > +	enum query_opcode opcode, enum flag_idn idn, bool *flag_res) {
> > +	int ret;
> > +	int retries;
> > +
> > +	for (retries = 0; retries < QUERY_REQ_RETRIES; retries++) {
> > +		ret = ufshcd_query_flag(hba, opcode, idn, flag_res);
> > +		if (ret)
> > +			dev_dbg(hba->dev,
> > +				"%s: failed with error %d, retries %d\n",
> > +				__func__, ret, retries);
> > +		else
> > +			break;
> > +	}
> > +
> > +	if (ret)
> > +		dev_err(hba->dev,
> > +			"%s: query attribute, opcode %d, idn %d, failed with
> error %d after %d retries\n",
> > +			__func__, (int)opcode, (int)idn, ret, retries);
> > +	return ret;
> > +}
> > +
> > +static int ufshcd_reset_ctmwb(struct ufs_hba *hba, bool force) {
> > +	int err = 0;
> > +
> > +	if (!hba_ctmwb.support_ctmwb)
> > +		return 0;
> > +
> > +	if (ufshcd_is_ctmwb_off(hba_ctmwb)) {
> > +		dev_info(hba->dev, "%s: turbo write already disabled.
> ctmwb_state = %d\n",
> > +			__func__, hba_ctmwb.ufs_ctmwb_state);
> > +		return 0;
> > +	}
> > +
> > +	if (ufshcd_is_ctmwb_err(hba_ctmwb))
> > +		dev_err(hba->dev, "%s: previous turbo write control was
> failed.\n",
> > +			__func__);
> > +
> > +	if (force)
> > +		err = ufshcd_query_flag_retry(hba,
> UPIU_QUERY_OPCODE_CLEAR_FLAG,
> > +				QUERY_FLAG_IDN_WB_EN, NULL);
> > +
> > +	if (err) {
> > +		ufshcd_set_ctmwb_err(hba_ctmwb);
> > +		dev_err(hba->dev, "%s: disable turbo write failed. err
> = %d\n",
> > +			__func__, err);
> > +	} else {
> > +		ufshcd_set_ctmwb_off(hba_ctmwb);
> > +		dev_info(hba->dev, "%s: ufs turbo write disabled \n",
> __func__);
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static int ufshcd_get_ctmwb_buf_status(struct ufs_hba *hba, u32
> > +*status) {
> > +	return ufshcd_query_attr_retry(hba, UPIU_QUERY_OPCODE_READ_ATTR,
> > +			QUERY_ATTR_IDN_AVAIL_WB_BUFF_SIZE, 0, 0, status); }
> > +
> > +static int ufshcd_ctmwb_manual_flush_ctrl(struct ufs_hba *hba, int
> > +en) {
> > +	int err = 0;
> > +
> > +	dev_info(hba->dev, "%s: %sable turbo write manual flush\n",
> > +				__func__, en ? "en" : "dis");
> > +	if (en) {
> > +		err = ufshcd_query_flag_retry(hba,
> UPIU_QUERY_OPCODE_SET_FLAG,
> > +					QUERY_FLAG_IDN_WB_BUFF_FLUSH_EN, NULL);
> > +		if (err)
> > +			dev_err(hba->dev, "%s: enable turbo write failed. err
> = %d\n",
> > +				__func__, err);
> > +	} else {
> > +		err = ufshcd_query_flag_retry(hba,
> UPIU_QUERY_OPCODE_CLEAR_FLAG,
> > +					QUERY_FLAG_IDN_WB_BUFF_FLUSH_EN, NULL);
> > +		if (err)
> > +			dev_err(hba->dev, "%s: disable turbo write failed. err
> = %d\n",
> > +				__func__, err);
> > +	}
> > +
> > +	return err;
> > +}
> > +
> > +static int ufshcd_ctmwb_flush_ctrl(struct ufs_hba *hba) {
> > +	int err = 0;
> > +	u32 curr_status = 0;
> > +
> > +	err = ufshcd_get_ctmwb_buf_status(hba, &curr_status);
> > +
> > +	if (!err && (curr_status <= UFS_WB_MANUAL_FLUSH_THRESHOLD)) {
> > +		dev_info(hba->dev, "%s: enable ctmwb manual flush, buf
> status : %d\n",
> > +				__func__, curr_status);
> > +		scsi_block_requests(hba->host);
> > +		err = ufshcd_ctmwb_manual_flush_ctrl(hba, 1);
> > +		if (!err) {
> > +			mdelay(100);
> > +			err = ufshcd_ctmwb_manual_flush_ctrl(hba, 0);
> > +			if (err)
> > +				dev_err(hba->dev, "%s: disable ctmwb manual
> flush failed. err = %d\n",
> > +						__func__, err);
> > +		} else
> > +			dev_err(hba->dev, "%s: enable ctmwb manual flush
> failed. err = %d\n",
> > +					__func__, err);
> > +		scsi_unblock_requests(hba->host);
> > +	}
> > +	return err;
> > +}
> > +
> > +static int ufshcd_ctmwb_ctrl(struct ufs_hba *hba, bool enable) {
> > +	int err;
> > +#if 0
> Did you miss removing these #if 0?
I will modify this code.
> 
> > +	if (!hba->support_ctmwb)
> > +		return;
> > +
> > +	if (hba->pm_op_in_progress) {
> > +		dev_err(hba->dev, "%s: ctmwb ctrl during pm operation is not
> allowed.\n",
> > +			__func__);
> > +		return;
> > +	}
> > +
> > +	if (hba->ufshcd_state != UFSHCD_STATE_OPERATIONAL) {
> > +		dev_err(hba->dev, "%s: ufs host is not available.\n",
> > +			__func__);
> > +		return;
> > +	}
> > +	if (ufshcd_is_ctmwb_err(hba_ctmwb))
> > +		dev_err(hba->dev, "%s: previous turbo write control was
> failed.\n",
> > +			__func__);
> > +#endif
> > +	if (enable) {
> > +		if (ufshcd_is_ctmwb_on(hba_ctmwb)) {
> > +			dev_err(hba->dev, "%s: turbo write already enabled.
> ctmwb_state = %d\n",
> > +				__func__, hba_ctmwb.ufs_ctmwb_state);
> > +			return 0;
> > +		}
> > +		pm_runtime_get_sync(hba->dev);
> > +		err = ufshcd_query_flag_retry(hba,
> UPIU_QUERY_OPCODE_SET_FLAG,
> > +					QUERY_FLAG_IDN_WB_EN, NULL);
> > +		if (err) {
> > +			ufshcd_set_ctmwb_err(hba_ctmwb);
> > +			dev_err(hba->dev, "%s: enable turbo write failed. err
> = %d\n",
> > +				__func__, err);
> > +		} else {
> > +			ufshcd_set_ctmwb_on(hba_ctmwb);
> > +			dev_info(hba->dev, "%s: ufs turbo write enabled \n",
> __func__);
> > +		}
> > +	} else {
> > +		if (ufshcd_is_ctmwb_off(hba_ctmwb)) {
> > +			dev_err(hba->dev, "%s: turbo write already disabled.
> ctmwb_state = %d\n",
> > +				__func__, hba_ctmwb.ufs_ctmwb_state);
> > +			return 0;
> > +		}
> > +		pm_runtime_get_sync(hba->dev);
> > +		err = ufshcd_query_flag_retry(hba,
> UPIU_QUERY_OPCODE_CLEAR_FLAG,
> > +					QUERY_FLAG_IDN_WB_EN, NULL);
> > +		if (err) {
> > +			ufshcd_set_ctmwb_err(hba_ctmwb);
> > +			dev_err(hba->dev, "%s: disable turbo write failed. err
> = %d\n",
> > +				__func__, err);
> > +		} else {
> > +			ufshcd_set_ctmwb_off(hba_ctmwb);
> > +			dev_info(hba->dev, "%s: ufs turbo write disabled \n",
> __func__);
> What is 'turbo write'?
I wrote it wrong. I will collect it.

> > +		}
> > +	}
> > +
> > +	pm_runtime_put_sync(hba->dev);
> > +
> > +	return 0;
> > +}
> > +
> > +/**
> > + * ufshcd_get_ctmwbbuf_unit - get ctmwb buffer alloc units
> > + * @sdev: pointer to SCSI device
> > + *
> > + * Read dLUNumTurboWriteBufferAllocUnits in UNIT Descriptor
> > + * to check if LU supports turbo write feature  */ static int
> > +ufshcd_get_ctmwbbuf_unit(struct ufs_hba *hba) {
> > +	struct scsi_device *sdev = hba->sdev_ufs_device;
> > +	struct ufshba_ctmwb *hba_ctmwb = (struct ufshba_ctmwb *)hba->wb_ops;
> > +	int ret = 0;
> > +
> > +	u32 dLUNumTurboWriteBufferAllocUnits = 0;
> > +	u8 desc_buf[4];
> > +
> > +	if (!hba_ctmwb->support_ctmwb)
> > +		return 0;
> > +
> > +	ret = ufshcd_read_unit_desc_param(hba,
> > +			ufshcd_scsi_to_upiu_lun(sdev->lun),
> > +			UNIT_DESC_PARAM_WB_BUF_ALLOC_UNITS,
> > +			desc_buf,
> > +			sizeof(dLUNumTurboWriteBufferAllocUnits));
> > +
> > +	/* Some WLUN doesn't support unit descriptor */
> > +	if ((ret == -EOPNOTSUPP) || scsi_is_wlun(sdev->lun)){
> > +		hba_ctmwb->support_ctmwb_lu = false;
> > +		dev_info(hba->dev,"%s: do not support WB\n", __func__);
> > +		return 0;
> > +	}
> > +
> > +	dLUNumTurboWriteBufferAllocUnits = ((desc_buf[0] << 24)|
> > +			(desc_buf[1] << 16) |
> > +			(desc_buf[2] << 8) |
> > +			desc_buf[3]);
> > +
> > +	if (dLUNumTurboWriteBufferAllocUnits) {
> > +		hba_ctmwb->support_ctmwb_lu = true;
> > +		dev_info(hba->dev, "%s: LU %d supports ctmwb, ctmwbbuf unit :
> 0x%x\n",
> > +				__func__, (int)sdev->lun,
> dLUNumTurboWriteBufferAllocUnits);
> > +	} else
> > +		hba_ctmwb->support_ctmwb_lu = false;
> > +
> > +	return 0;
> > +}
> > +
> > +static inline int ufshcd_ctmwb_toggle_flush(struct ufs_hba *hba, enum
> > +ufs_pm_op pm_op) {
> > +	ufshcd_ctmwb_flush_ctrl(hba);
> > +
> > +	if (ufshcd_is_system_pm(pm_op))
> > +		ufshcd_reset_ctmwb(hba, true);
> > +
> > +	return 0;
> > +}
> > +
> > +static struct ufs_wb_ops exynos_ctmwb_ops = {
> > +	.wb_toggle_flush_vendor = ufshcd_ctmwb_toggle_flush,
> > +	.wb_alloc_units_vendor = ufshcd_get_ctmwbbuf_unit,
> > +	.wb_ctrl_vendor = ufshcd_ctmwb_ctrl,
> > +	.wb_reset_vendor = ufshcd_reset_ctmwb, };
> > +
> > +struct ufs_wb_ops *ufshcd_ctmwb_init(void) {
> > +	hba_ctmwb.support_ctmwb = 1;
> > +
> > +	return &exynos_ctmwb_ops;
> > +}
> > +EXPORT_SYMBOL_GPL(ufshcd_ctmwb_init);
> > +
> > diff --git a/drivers/scsi/ufs/ufs_ctmwb.h
> > b/drivers/scsi/ufs/ufs_ctmwb.h new file mode 100644 index
> > 000000000000..073e21a4900b
> > --- /dev/null
> > +++ b/drivers/scsi/ufs/ufs_ctmwb.h
> > @@ -0,0 +1,27 @@
> > +#ifndef _UFS_CTMWB_H_
> > +#define _UFS_CTMWB_H_
> > +
> > +enum ufs_ctmwb_state {
> > +       UFS_WB_OFF_STATE	= 0,    /* turbo write disabled state */
> > +       UFS_WB_ON_STATE	= 1,            /* turbo write enabled state */
> > +       UFS_WB_ERR_STATE	= 2,            /* turbo write error state */
> > +};
> > +
> > +#define ufshcd_is_ctmwb_off(hba) ((hba).ufs_ctmwb_state ==
> > +UFS_WB_OFF_STATE) #define ufshcd_is_ctmwb_on(hba)
> > +((hba).ufs_ctmwb_state == UFS_WB_ON_STATE) #define
> > +ufshcd_is_ctmwb_err(hba) ((hba).ufs_ctmwb_state == UFS_WB_ERR_STATE)
> > +#define ufshcd_set_ctmwb_off(hba) ((hba).ufs_ctmwb_state =
> > +UFS_WB_OFF_STATE) #define ufshcd_set_ctmwb_on(hba)
> > +((hba).ufs_ctmwb_state = UFS_WB_ON_STATE) #define
> > +ufshcd_set_ctmwb_err(hba) ((hba).ufs_ctmwb_state = UFS_WB_ERR_STATE)
> > +
> > +#define UFS_WB_MANUAL_FLUSH_THRESHOLD	5
> > +
> > +struct ufshba_ctmwb {
> > +	enum ufs_ctmwb_state ufs_ctmwb_state;
> > +	bool support_ctmwb;
> > +
> > +	bool support_ctmwb_lu;
> > +};
> > +
> > +struct ufs_wb_ops *ufshcd_ctmwb_init(void); #endif
> >
> 
> -Asutosh
> 
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> Linux Foundation Collaborative Project

Re: [RFC PATCH v2 3/3] scsi: ufs: add vendor specific write booster To support the fuction of writebooster by vendor. The WB behavior that the vendor wants is slightly different. But we have to support it

[Asutosh Das (asd)]

On 7/21/2020 2:47 AM, 서호영 wrote:
> 
> 
>> -----Original Message-----
>> From: asutoshd=codeaurora.org@mg.codeaurora.org
>> [mailto:asutoshd=codeaurora.org@mg.codeaurora.org] On Behalf Of Asutosh
>> Das (asd)
>> Sent: Tuesday, July 21, 2020 1:55 AM
>> To: SEO HOYOUNG; linux-scsi@vger.kernel.org; alim.akhtar@samsung.com;
>> avri.altman@wdc.com; jejb@linux.ibm.com; martin.petersen@oracle.com;
>> beanhuo@micron.com; cang@codeaurora.org; bvanassche@acm.org;
>> grant.jung@samsung.com
>> Subject: Re: [RFC PATCH v2 3/3] scsi: ufs: add vendor specific write
>> booster To support the fuction of writebooster by vendor. The WB behavior
>> that the vendor wants is slightly different. But we have to support it
>>
>> On 7/20/2020 3:40 AM, SEO HOYOUNG wrote:
>>> Signed-off-by: SEO HOYOUNG <hy50.seo@samsung.com>
>>> ---
>>>    drivers/scsi/ufs/Makefile     |   1 +
>>>    drivers/scsi/ufs/ufs-exynos.c |   6 +
>>>    drivers/scsi/ufs/ufs_ctmwb.c  | 279 ++++++++++++++++++++++++++++++++++
>>>    drivers/scsi/ufs/ufs_ctmwb.h  |  27 ++++
>>>    4 files changed, 313 insertions(+)
>>>    create mode 100644 drivers/scsi/ufs/ufs_ctmwb.c
>>>    create mode 100644 drivers/scsi/ufs/ufs_ctmwb.h
>>>
>>> diff --git a/drivers/scsi/ufs/Makefile b/drivers/scsi/ufs/Makefile
>>> index 9810963bc049..b1ba36c7d66f 100644
>>> --- a/drivers/scsi/ufs/Makefile
>>> +++ b/drivers/scsi/ufs/Makefile
>>> @@ -5,6 +5,7 @@ obj-$(CONFIG_SCSI_UFS_DWC_TC_PLATFORM) += tc-dwc-g210-
>> pltfrm.o ufshcd-dwc.o tc-d
>>>    obj-$(CONFIG_SCSI_UFS_CDNS_PLATFORM) += cdns-pltfrm.o
>>>    obj-$(CONFIG_SCSI_UFS_QCOM) += ufs-qcom.o
>>>    obj-$(CONFIG_SCSI_UFS_EXYNOS) += ufs-exynos.o
>>> +obj-$(CONFIG_SCSI_UFS_VENDOR_WB) += ufs_ctmwb.o
>>>    obj-$(CONFIG_SCSI_UFSHCD) += ufshcd-core.o
>>>    ufshcd-core-y				+= ufshcd.o ufs-sysfs.o
>>>    ufshcd-core-$(CONFIG_SCSI_UFS_BSG)	+= ufs_bsg.o
>>> diff --git a/drivers/scsi/ufs/ufs-exynos.c
>>> b/drivers/scsi/ufs/ufs-exynos.c index 32b61ba77241..f127f5f2bf36
>>> 100644
>>> --- a/drivers/scsi/ufs/ufs-exynos.c
>>> +++ b/drivers/scsi/ufs/ufs-exynos.c
>>> @@ -22,6 +22,9 @@
>>>
>>
>> To me it looks like, you want to have your own flush policy &
>> initializations etc, is that understanding correct?
>> I don't understand why though. The current implementation is spec
>> compliant. If there're benefits that you see in this implementation,
>> please highlight those. It'd be interesting to see that.
> 
> Yes. I want to own flush policy, initialization..
> I already know current implementation is spec compliant.
> But some vendor want to change flush policy.
Ok. It'd be interesting to know the benefits of your flush policy over 
the current one. If it's better, we can replace the current policy, perhaps?
So please can you highlight those benefits.
> So we modify below code.
> Additionally when use below code, we can use WB without UFS 3.1 devices
I guess non-standard stuff should be kept out of ufshcd.c to vendor 
specific files, which I guess you're doing.
>>
>>
>>>    #include "ufs-exynos.h"
>>>
>>> +#ifdef CONFIG_SCSI_UFS_VENDOR_WB
>>> +#include "ufs_ctmwb.h"
>>> +#endif
>>>    /*
>>>     * Exynos's Vendor specific registers for UFSHCI
>>>     */
>>> @@ -989,6 +992,9 @@ static int exynos_ufs_init(struct ufs_hba *hba)
>>>    		goto phy_off;
>>>
>>>    	ufs->hba = hba;
>>> +#ifdef CONFIG_SCSI_UFS_VENDOR_WB
>>> +	ufs->hba->wb_ops = ufshcd_ctmwb_init(); #endif
>>>    	ufs->opts = ufs->drv_data->opts;
>>>    	ufs->rx_sel_idx = PA_MAXDATALANES;
>>>    	if (ufs->opts & EXYNOS_UFS_OPT_BROKEN_RX_SEL_IDX) diff --git
>>> a/drivers/scsi/ufs/ufs_ctmwb.c b/drivers/scsi/ufs/ufs_ctmwb.c new file
>>> mode 100644 index 000000000000..ab39f40721ae
>>> --- /dev/null
>>> +++ b/drivers/scsi/ufs/ufs_ctmwb.c
>>> @@ -0,0 +1,279 @@
>>> +#include "ufshcd.h"
>>> +#include "ufshci.h"
>>> +#include "ufs_ctmwb.h"
>>> +
>>> +static struct ufshba_ctmwb hba_ctmwb;
>>> +
>>> +/* Query request retries */
>>> +#define QUERY_REQ_RETRIES 3
>>> +
>>> +static int ufshcd_query_attr_retry(struct ufs_hba *hba,
>>> +	enum query_opcode opcode, enum attr_idn idn, u8 index, u8 selector,
>>> +	u32 *attr_val)
>>> +{
>>> +	int ret = 0;
>>> +	u32 retries;
>>> +
>>> +	 for (retries = QUERY_REQ_RETRIES; retries > 0; retries--) {
>>> +		ret = ufshcd_query_attr(hba, opcode, idn, index,
>>> +						selector, attr_val);
>>> +		if (ret)
>>> +			dev_dbg(hba->dev, "%s: failed with error %d,
>> retries %d\n",
>>> +				__func__, ret, retries);
>>> +		else
>>> +			break;
>>> +	}
>>> +
>>> +	if (ret)
>>> +		dev_err(hba->dev,
>>> +			"%s: query attribute, idn %d, failed with error %d
>> after %d retires\n",
>>> +			__func__, idn, ret, QUERY_REQ_RETRIES);
>>> +	return ret;
>>> +}
>>> +
>>> +static int ufshcd_query_flag_retry(struct ufs_hba *hba,
>>> +	enum query_opcode opcode, enum flag_idn idn, bool *flag_res) {
>>> +	int ret;
>>> +	int retries;
>>> +
>>> +	for (retries = 0; retries < QUERY_REQ_RETRIES; retries++) {
>>> +		ret = ufshcd_query_flag(hba, opcode, idn, flag_res);
>>> +		if (ret)
>>> +			dev_dbg(hba->dev,
>>> +				"%s: failed with error %d, retries %d\n",
>>> +				__func__, ret, retries);
>>> +		else
>>> +			break;
>>> +	}
>>> +
>>> +	if (ret)
>>> +		dev_err(hba->dev,
>>> +			"%s: query attribute, opcode %d, idn %d, failed with
>> error %d after %d retries\n",
>>> +			__func__, (int)opcode, (int)idn, ret, retries);
>>> +	return ret;
>>> +}
>>> +
>>> +static int ufshcd_reset_ctmwb(struct ufs_hba *hba, bool force) {
>>> +	int err = 0;
>>> +
>>> +	if (!hba_ctmwb.support_ctmwb)
>>> +		return 0;
>>> +
>>> +	if (ufshcd_is_ctmwb_off(hba_ctmwb)) {
>>> +		dev_info(hba->dev, "%s: turbo write already disabled.
>> ctmwb_state = %d\n",
>>> +			__func__, hba_ctmwb.ufs_ctmwb_state);
>>> +		return 0;
>>> +	}
>>> +
>>> +	if (ufshcd_is_ctmwb_err(hba_ctmwb))
>>> +		dev_err(hba->dev, "%s: previous turbo write control was
>> failed.\n",
>>> +			__func__);
>>> +
>>> +	if (force)
>>> +		err = ufshcd_query_flag_retry(hba,
>> UPIU_QUERY_OPCODE_CLEAR_FLAG,
>>> +				QUERY_FLAG_IDN_WB_EN, NULL);
>>> +
>>> +	if (err) {
>>> +		ufshcd_set_ctmwb_err(hba_ctmwb);
>>> +		dev_err(hba->dev, "%s: disable turbo write failed. err
>> = %d\n",
>>> +			__func__, err);
>>> +	} else {
>>> +		ufshcd_set_ctmwb_off(hba_ctmwb);
>>> +		dev_info(hba->dev, "%s: ufs turbo write disabled \n",
>> __func__);
>>> +	}
>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +static int ufshcd_get_ctmwb_buf_status(struct ufs_hba *hba, u32
>>> +*status) {
>>> +	return ufshcd_query_attr_retry(hba, UPIU_QUERY_OPCODE_READ_ATTR,
>>> +			QUERY_ATTR_IDN_AVAIL_WB_BUFF_SIZE, 0, 0, status); }
>>> +
>>> +static int ufshcd_ctmwb_manual_flush_ctrl(struct ufs_hba *hba, int
>>> +en) {
>>> +	int err = 0;
>>> +
>>> +	dev_info(hba->dev, "%s: %sable turbo write manual flush\n",
>>> +				__func__, en ? "en" : "dis");
>>> +	if (en) {
>>> +		err = ufshcd_query_flag_retry(hba,
>> UPIU_QUERY_OPCODE_SET_FLAG,
>>> +					QUERY_FLAG_IDN_WB_BUFF_FLUSH_EN, NULL);
>>> +		if (err)
>>> +			dev_err(hba->dev, "%s: enable turbo write failed. err
>> = %d\n",
>>> +				__func__, err);
>>> +	} else {
>>> +		err = ufshcd_query_flag_retry(hba,
>> UPIU_QUERY_OPCODE_CLEAR_FLAG,
>>> +					QUERY_FLAG_IDN_WB_BUFF_FLUSH_EN, NULL);
>>> +		if (err)
>>> +			dev_err(hba->dev, "%s: disable turbo write failed. err
>> = %d\n",
>>> +				__func__, err);
>>> +	}
>>> +
>>> +	return err;
>>> +}
>>> +
>>> +static int ufshcd_ctmwb_flush_ctrl(struct ufs_hba *hba) {
>>> +	int err = 0;
>>> +	u32 curr_status = 0;
>>> +
>>> +	err = ufshcd_get_ctmwb_buf_status(hba, &curr_status);
>>> +
>>> +	if (!err && (curr_status <= UFS_WB_MANUAL_FLUSH_THRESHOLD)) {
>>> +		dev_info(hba->dev, "%s: enable ctmwb manual flush, buf
>> status : %d\n",
>>> +				__func__, curr_status);
>>> +		scsi_block_requests(hba->host);
>>> +		err = ufshcd_ctmwb_manual_flush_ctrl(hba, 1);
>>> +		if (!err) {
>>> +			mdelay(100);
>>> +			err = ufshcd_ctmwb_manual_flush_ctrl(hba, 0);
>>> +			if (err)
>>> +				dev_err(hba->dev, "%s: disable ctmwb manual
>> flush failed. err = %d\n",
>>> +						__func__, err);
>>> +		} else
>>> +			dev_err(hba->dev, "%s: enable ctmwb manual flush
>> failed. err = %d\n",
>>> +					__func__, err);
>>> +		scsi_unblock_requests(hba->host);
>>> +	}
>>> +	return err;
>>> +}
>>> +
>>> +static int ufshcd_ctmwb_ctrl(struct ufs_hba *hba, bool enable) {
>>> +	int err;
>>> +#if 0
>> Did you miss removing these #if 0?
> I will modify this code.
>>
>>> +	if (!hba->support_ctmwb)
>>> +		return;
>>> +
>>> +	if (hba->pm_op_in_progress) {
>>> +		dev_err(hba->dev, "%s: ctmwb ctrl during pm operation is not
>> allowed.\n",
>>> +			__func__);
>>> +		return;
>>> +	}
>>> +
>>> +	if (hba->ufshcd_state != UFSHCD_STATE_OPERATIONAL) {
>>> +		dev_err(hba->dev, "%s: ufs host is not available.\n",
>>> +			__func__);
>>> +		return;
>>> +	}
>>> +	if (ufshcd_is_ctmwb_err(hba_ctmwb))
>>> +		dev_err(hba->dev, "%s: previous turbo write control was
>> failed.\n",
>>> +			__func__);
>>> +#endif
>>> +	if (enable) {
>>> +		if (ufshcd_is_ctmwb_on(hba_ctmwb)) {
>>> +			dev_err(hba->dev, "%s: turbo write already enabled.
>> ctmwb_state = %d\n",
>>> +				__func__, hba_ctmwb.ufs_ctmwb_state);
>>> +			return 0;
>>> +		}
>>> +		pm_runtime_get_sync(hba->dev);
>>> +		err = ufshcd_query_flag_retry(hba,
>> UPIU_QUERY_OPCODE_SET_FLAG,
>>> +					QUERY_FLAG_IDN_WB_EN, NULL);
>>> +		if (err) {
>>> +			ufshcd_set_ctmwb_err(hba_ctmwb);
>>> +			dev_err(hba->dev, "%s: enable turbo write failed. err
>> = %d\n",
>>> +				__func__, err);
>>> +		} else {
>>> +			ufshcd_set_ctmwb_on(hba_ctmwb);
>>> +			dev_info(hba->dev, "%s: ufs turbo write enabled \n",
>> __func__);
>>> +		}
>>> +	} else {
>>> +		if (ufshcd_is_ctmwb_off(hba_ctmwb)) {
>>> +			dev_err(hba->dev, "%s: turbo write already disabled.
>> ctmwb_state = %d\n",
>>> +				__func__, hba_ctmwb.ufs_ctmwb_state);
>>> +			return 0;
>>> +		}
>>> +		pm_runtime_get_sync(hba->dev);
>>> +		err = ufshcd_query_flag_retry(hba,
>> UPIU_QUERY_OPCODE_CLEAR_FLAG,
>>> +					QUERY_FLAG_IDN_WB_EN, NULL);
>>> +		if (err) {
>>> +			ufshcd_set_ctmwb_err(hba_ctmwb);
>>> +			dev_err(hba->dev, "%s: disable turbo write failed. err
>> = %d\n",
>>> +				__func__, err);
>>> +		} else {
>>> +			ufshcd_set_ctmwb_off(hba_ctmwb);
>>> +			dev_info(hba->dev, "%s: ufs turbo write disabled \n",
>> __func__);
>> What is 'turbo write'?
> I wrote it wrong. I will collect it.
> 
>>> +		}
>>> +	}
>>> +
>>> +	pm_runtime_put_sync(hba->dev);
>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +/**
>>> + * ufshcd_get_ctmwbbuf_unit - get ctmwb buffer alloc units
>>> + * @sdev: pointer to SCSI device
>>> + *
>>> + * Read dLUNumTurboWriteBufferAllocUnits in UNIT Descriptor
>>> + * to check if LU supports turbo write feature  */ static int
>>> +ufshcd_get_ctmwbbuf_unit(struct ufs_hba *hba) {
>>> +	struct scsi_device *sdev = hba->sdev_ufs_device;
>>> +	struct ufshba_ctmwb *hba_ctmwb = (struct ufshba_ctmwb *)hba->wb_ops;
>>> +	int ret = 0;
>>> +
>>> +	u32 dLUNumTurboWriteBufferAllocUnits = 0;
>>> +	u8 desc_buf[4];
>>> +
>>> +	if (!hba_ctmwb->support_ctmwb)
>>> +		return 0;
>>> +
>>> +	ret = ufshcd_read_unit_desc_param(hba,
>>> +			ufshcd_scsi_to_upiu_lun(sdev->lun),
>>> +			UNIT_DESC_PARAM_WB_BUF_ALLOC_UNITS,
>>> +			desc_buf,
>>> +			sizeof(dLUNumTurboWriteBufferAllocUnits));
>>> +
>>> +	/* Some WLUN doesn't support unit descriptor */
>>> +	if ((ret == -EOPNOTSUPP) || scsi_is_wlun(sdev->lun)){
>>> +		hba_ctmwb->support_ctmwb_lu = false;
>>> +		dev_info(hba->dev,"%s: do not support WB\n", __func__);
>>> +		return 0;
>>> +	}
>>> +
>>> +	dLUNumTurboWriteBufferAllocUnits = ((desc_buf[0] << 24)|
>>> +			(desc_buf[1] << 16) |
>>> +			(desc_buf[2] << 8) |
>>> +			desc_buf[3]);
>>> +
>>> +	if (dLUNumTurboWriteBufferAllocUnits) {
>>> +		hba_ctmwb->support_ctmwb_lu = true;
>>> +		dev_info(hba->dev, "%s: LU %d supports ctmwb, ctmwbbuf unit :
>> 0x%x\n",
>>> +				__func__, (int)sdev->lun,
>> dLUNumTurboWriteBufferAllocUnits);
>>> +	} else
>>> +		hba_ctmwb->support_ctmwb_lu = false;
>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +static inline int ufshcd_ctmwb_toggle_flush(struct ufs_hba *hba, enum
>>> +ufs_pm_op pm_op) {
>>> +	ufshcd_ctmwb_flush_ctrl(hba);
>>> +
>>> +	if (ufshcd_is_system_pm(pm_op))
>>> +		ufshcd_reset_ctmwb(hba, true);
>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +static struct ufs_wb_ops exynos_ctmwb_ops = {
>>> +	.wb_toggle_flush_vendor = ufshcd_ctmwb_toggle_flush,
>>> +	.wb_alloc_units_vendor = ufshcd_get_ctmwbbuf_unit,
>>> +	.wb_ctrl_vendor = ufshcd_ctmwb_ctrl,
>>> +	.wb_reset_vendor = ufshcd_reset_ctmwb, };
>>> +
>>> +struct ufs_wb_ops *ufshcd_ctmwb_init(void) {
>>> +	hba_ctmwb.support_ctmwb = 1;
>>> +
>>> +	return &exynos_ctmwb_ops;
>>> +}
>>> +EXPORT_SYMBOL_GPL(ufshcd_ctmwb_init);
>>> +
>>> diff --git a/drivers/scsi/ufs/ufs_ctmwb.h
>>> b/drivers/scsi/ufs/ufs_ctmwb.h new file mode 100644 index
>>> 000000000000..073e21a4900b
>>> --- /dev/null
>>> +++ b/drivers/scsi/ufs/ufs_ctmwb.h
>>> @@ -0,0 +1,27 @@
>>> +#ifndef _UFS_CTMWB_H_
>>> +#define _UFS_CTMWB_H_
>>> +
>>> +enum ufs_ctmwb_state {
>>> +       UFS_WB_OFF_STATE	= 0,    /* turbo write disabled state */
>>> +       UFS_WB_ON_STATE	= 1,            /* turbo write enabled state */
>>> +       UFS_WB_ERR_STATE	= 2,            /* turbo write error state */
>>> +};
>>> +
>>> +#define ufshcd_is_ctmwb_off(hba) ((hba).ufs_ctmwb_state ==
>>> +UFS_WB_OFF_STATE) #define ufshcd_is_ctmwb_on(hba)
>>> +((hba).ufs_ctmwb_state == UFS_WB_ON_STATE) #define
>>> +ufshcd_is_ctmwb_err(hba) ((hba).ufs_ctmwb_state == UFS_WB_ERR_STATE)
>>> +#define ufshcd_set_ctmwb_off(hba) ((hba).ufs_ctmwb_state =
>>> +UFS_WB_OFF_STATE) #define ufshcd_set_ctmwb_on(hba)
>>> +((hba).ufs_ctmwb_state = UFS_WB_ON_STATE) #define
>>> +ufshcd_set_ctmwb_err(hba) ((hba).ufs_ctmwb_state = UFS_WB_ERR_STATE)
>>> +
>>> +#define UFS_WB_MANUAL_FLUSH_THRESHOLD	5
>>> +
>>> +struct ufshba_ctmwb {
>>> +	enum ufs_ctmwb_state ufs_ctmwb_state;
>>> +	bool support_ctmwb;
>>> +
>>> +	bool support_ctmwb_lu;
>>> +};
>>> +
>>> +struct ufs_wb_ops *ufshcd_ctmwb_init(void); #endif
>>>
>>
>> -Asutosh
>>
>> --
>> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
>> Linux Foundation Collaborative Project
> 

-Asutosh

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

RE: [RFC PATCH v2 3/3] scsi: ufs: add vendor specific write booster To support the fuction of writebooster by vendor. The WB behavior that the vendor wants is slightly different. But we have to support it

[서호영]

> >> On 7/20/2020 3:40 AM, SEO HOYOUNG wrote:
> >>> Signed-off-by: SEO HOYOUNG <hy50.seo@samsung.com>
> >>> ---
> >>>    drivers/scsi/ufs/Makefile     |   1 +
> >>>    drivers/scsi/ufs/ufs-exynos.c |   6 +
> >>>    drivers/scsi/ufs/ufs_ctmwb.c  | 279
> ++++++++++++++++++++++++++++++++++
> >>>    drivers/scsi/ufs/ufs_ctmwb.h  |  27 ++++
> >>>    4 files changed, 313 insertions(+)
> >>>    create mode 100644 drivers/scsi/ufs/ufs_ctmwb.c
> >>>    create mode 100644 drivers/scsi/ufs/ufs_ctmwb.h
> >>>
> >>> diff --git a/drivers/scsi/ufs/Makefile b/drivers/scsi/ufs/Makefile
> >>> index 9810963bc049..b1ba36c7d66f 100644
> >>> --- a/drivers/scsi/ufs/Makefile
> >>> +++ b/drivers/scsi/ufs/Makefile
> >>> @@ -5,6 +5,7 @@ obj-$(CONFIG_SCSI_UFS_DWC_TC_PLATFORM) +=
> >>> tc-dwc-g210-
> >> pltfrm.o ufshcd-dwc.o tc-d
> >>>    obj-$(CONFIG_SCSI_UFS_CDNS_PLATFORM) += cdns-pltfrm.o
> >>>    obj-$(CONFIG_SCSI_UFS_QCOM) += ufs-qcom.o
> >>>    obj-$(CONFIG_SCSI_UFS_EXYNOS) += ufs-exynos.o
> >>> +obj-$(CONFIG_SCSI_UFS_VENDOR_WB) += ufs_ctmwb.o
> >>>    obj-$(CONFIG_SCSI_UFSHCD) += ufshcd-core.o
> >>>    ufshcd-core-y				+= ufshcd.o ufs-sysfs.o
> >>>    ufshcd-core-$(CONFIG_SCSI_UFS_BSG)	+= ufs_bsg.o
> >>> diff --git a/drivers/scsi/ufs/ufs-exynos.c
> >>> b/drivers/scsi/ufs/ufs-exynos.c index 32b61ba77241..f127f5f2bf36
> >>> 100644
> >>> --- a/drivers/scsi/ufs/ufs-exynos.c
> >>> +++ b/drivers/scsi/ufs/ufs-exynos.c
> >>> @@ -22,6 +22,9 @@
> >>>
> >>
> >> To me it looks like, you want to have your own flush policy &
> >> initializations etc, is that understanding correct?
> >> I don't understand why though. The current implementation is spec
> >> compliant. If there're benefits that you see in this implementation,
> >> please highlight those. It'd be interesting to see that.
> >
> > Yes. I want to own flush policy, initialization..
> > I already know current implementation is spec compliant.
> > But some vendor want to change flush policy.
> Ok. It'd be interesting to know the benefits of your flush policy over the
> current one. If it's better, we can replace the current policy, perhaps?
> So please can you highlight those benefits.
At first, our vendor device did not support gear scaling. So we can't use gear scaling code at mainline.
And we want to WB disable after probe. If IO state busy, then we will enable WB with IO monitoring.
I think always enable WB, then maybe occur power related problems.
So we want to modify those.
> > So we modify below code.
> > Additionally when use below code, we can use WB without UFS 3.1
> > devices
> I guess non-standard stuff should be kept out of ufshcd.c to vendor
> specific files, which I guess you're doing.
> >>
> >>
> >>>    #include "ufs-exynos.h"
> >>>
> >>> +#ifdef CONFIG_SCSI_UFS_VENDOR_WB
> >>> +#include "ufs_ctmwb.h"
> >>> +#endif
> >>>    /*
> >>>     * Exynos's Vendor specific registers for UFSHCI
> >>>     */
> >>> @@ -989,6 +992,9 @@ static int exynos_ufs_init(struct ufs_hba *hba)
> >>>    		goto phy_off;
> >>>
> >>>    	ufs->hba = hba;
> >>> +#ifdef CONFIG_SCSI_UFS_VENDOR_WB
> >>> +	ufs->hba->wb_ops = ufshcd_ctmwb_init(); #endif
> >>>    	ufs->opts = ufs->drv_data->opts;
> >>>    	ufs->rx_sel_idx = PA_MAXDATALANES;
> >>>    	if (ufs->opts & EXYNOS_UFS_OPT_BROKEN_RX_SEL_IDX) diff --git
> >>> a/drivers/scsi/ufs/ufs_ctmwb.c b/drivers/scsi/ufs/ufs_ctmwb.c new
> >>> file mode 100644 index 000000000000..ab39f40721ae
> >>> --- /dev/null
> >>> +++ b/drivers/scsi/ufs/ufs_ctmwb.c
> >>> @@ -0,0 +1,279 @@
> >>> +#include "ufshcd.h"
> >>> +#include "ufshci.h"
> >>> +#include "ufs_ctmwb.h"
> >>> +
> >>> +static struct ufshba_ctmwb hba_ctmwb;
> >>> +
> >>> +/* Query request retries */
> >>> +#define QUERY_REQ_RETRIES 3
> >>> +
> >>> +static int ufshcd_query_attr_retry(struct ufs_hba *hba,
> >>> +	enum query_opcode opcode, enum attr_idn idn, u8 index, u8 selector,
> >>> +	u32 *attr_val)
> >>> +{
> >>> +	int ret = 0;
> >>> +	u32 retries;
> >>> +
> >>> +	 for (retries = QUERY_REQ_RETRIES; retries > 0; retries--) {
> >>> +		ret = ufshcd_query_attr(hba, opcode, idn, index,
> >>> +						selector, attr_val);
> >>> +		if (ret)
> >>> +			dev_dbg(hba->dev, "%s: failed with error %d,
> >> retries %d\n",
> >>> +				__func__, ret, retries);
> >>> +		else
> >>> +			break;
> >>> +	}
> >>> +
> >>> +	if (ret)
> >>> +		dev_err(hba->dev,
> >>> +			"%s: query attribute, idn %d, failed with error %d
> >> after %d retires\n",
> >>> +			__func__, idn, ret, QUERY_REQ_RETRIES);
> >>> +	return ret;
> >>> +}
> >>> +
> >>> +static int ufshcd_query_flag_retry(struct ufs_hba *hba,
> >>> +	enum query_opcode opcode, enum flag_idn idn, bool *flag_res) {
> >>> +	int ret;
> >>> +	int retries;
> >>> +
> >>> +	for (retries = 0; retries < QUERY_REQ_RETRIES; retries++) {
> >>> +		ret = ufshcd_query_flag(hba, opcode, idn, flag_res);
> >>> +		if (ret)
> >>> +			dev_dbg(hba->dev,
> >>> +				"%s: failed with error %d, retries %d\n",
> >>> +				__func__, ret, retries);
> >>> +		else
> >>> +			break;
> >>> +	}
> >>> +
> >>> +	if (ret)
> >>> +		dev_err(hba->dev,
> >>> +			"%s: query attribute, opcode %d, idn %d, failed with
> >> error %d after %d retries\n",
> >>> +			__func__, (int)opcode, (int)idn, ret, retries);
> >>> +	return ret;
> >>> +}
> >>> +
> >>> +static int ufshcd_reset_ctmwb(struct ufs_hba *hba, bool force) {
> >>> +	int err = 0;
> >>> +
> >>> +	if (!hba_ctmwb.support_ctmwb)
> >>> +		return 0;
> >>> +
> >>> +	if (ufshcd_is_ctmwb_off(hba_ctmwb)) {
> >>> +		dev_info(hba->dev, "%s: turbo write already disabled.
> >> ctmwb_state = %d\n",
> >>> +			__func__, hba_ctmwb.ufs_ctmwb_state);
> >>> +		return 0;
> >>> +	}
> >>> +
> >>> +	if (ufshcd_is_ctmwb_err(hba_ctmwb))
> >>> +		dev_err(hba->dev, "%s: previous turbo write control was
> >> failed.\n",
> >>> +			__func__);
> >>> +
> >>> +	if (force)
> >>> +		err = ufshcd_query_flag_retry(hba,
> >> UPIU_QUERY_OPCODE_CLEAR_FLAG,
> >>> +				QUERY_FLAG_IDN_WB_EN, NULL);
> >>> +
> >>> +	if (err) {
> >>> +		ufshcd_set_ctmwb_err(hba_ctmwb);
> >>> +		dev_err(hba->dev, "%s: disable turbo write failed. err
> >> = %d\n",
> >>> +			__func__, err);
> >>> +	} else {
> >>> +		ufshcd_set_ctmwb_off(hba_ctmwb);
> >>> +		dev_info(hba->dev, "%s: ufs turbo write disabled \n",
> >> __func__);
> >>> +	}
> >>> +
> >>> +	return 0;
> >>> +}
> >>> +
> >>> +static int ufshcd_get_ctmwb_buf_status(struct ufs_hba *hba, u32
> >>> +*status) {
> >>> +	return ufshcd_query_attr_retry(hba, UPIU_QUERY_OPCODE_READ_ATTR,
> >>> +			QUERY_ATTR_IDN_AVAIL_WB_BUFF_SIZE, 0, 0, status); }
> >>> +
> >>> +static int ufshcd_ctmwb_manual_flush_ctrl(struct ufs_hba *hba, int
> >>> +en) {
> >>> +	int err = 0;
> >>> +
> >>> +	dev_info(hba->dev, "%s: %sable turbo write manual flush\n",
> >>> +				__func__, en ? "en" : "dis");
> >>> +	if (en) {
> >>> +		err = ufshcd_query_flag_retry(hba,
> >> UPIU_QUERY_OPCODE_SET_FLAG,
> >>> +					QUERY_FLAG_IDN_WB_BUFF_FLUSH_EN, NULL);
> >>> +		if (err)
> >>> +			dev_err(hba->dev, "%s: enable turbo write failed. err
> >> = %d\n",
> >>> +				__func__, err);
> >>> +	} else {
> >>> +		err = ufshcd_query_flag_retry(hba,
> >> UPIU_QUERY_OPCODE_CLEAR_FLAG,
> >>> +					QUERY_FLAG_IDN_WB_BUFF_FLUSH_EN, NULL);
> >>> +		if (err)
> >>> +			dev_err(hba->dev, "%s: disable turbo write failed. err
> >> = %d\n",
> >>> +				__func__, err);
> >>> +	}
> >>> +
> >>> +	return err;
> >>> +}
> >>> +
> >>> +static int ufshcd_ctmwb_flush_ctrl(struct ufs_hba *hba) {
> >>> +	int err = 0;
> >>> +	u32 curr_status = 0;
> >>> +
> >>> +	err = ufshcd_get_ctmwb_buf_status(hba, &curr_status);
> >>> +
> >>> +	if (!err && (curr_status <= UFS_WB_MANUAL_FLUSH_THRESHOLD)) {
> >>> +		dev_info(hba->dev, "%s: enable ctmwb manual flush, buf
> >> status : %d\n",
> >>> +				__func__, curr_status);
> >>> +		scsi_block_requests(hba->host);
> >>> +		err = ufshcd_ctmwb_manual_flush_ctrl(hba, 1);
> >>> +		if (!err) {
> >>> +			mdelay(100);
> >>> +			err = ufshcd_ctmwb_manual_flush_ctrl(hba, 0);
> >>> +			if (err)
> >>> +				dev_err(hba->dev, "%s: disable ctmwb manual
> >> flush failed. err = %d\n",
> >>> +						__func__, err);
> >>> +		} else
> >>> +			dev_err(hba->dev, "%s: enable ctmwb manual flush
> >> failed. err = %d\n",
> >>> +					__func__, err);
> >>> +		scsi_unblock_requests(hba->host);
> >>> +	}
> >>> +	return err;
> >>> +}
> >>> +
> >>> +static int ufshcd_ctmwb_ctrl(struct ufs_hba *hba, bool enable) {
> >>> +	int err;
> >>> +#if 0
> >> Did you miss removing these #if 0?
> > I will modify this code.
> >>
> >>> +	if (!hba->support_ctmwb)
> >>> +		return;
> >>> +
> >>> +	if (hba->pm_op_in_progress) {
> >>> +		dev_err(hba->dev, "%s: ctmwb ctrl during pm operation is not
> >> allowed.\n",
> >>> +			__func__);
> >>> +		return;
> >>> +	}
> >>> +
> >>> +	if (hba->ufshcd_state != UFSHCD_STATE_OPERATIONAL) {
> >>> +		dev_err(hba->dev, "%s: ufs host is not available.\n",
> >>> +			__func__);
> >>> +		return;
> >>> +	}
> >>> +	if (ufshcd_is_ctmwb_err(hba_ctmwb))
> >>> +		dev_err(hba->dev, "%s: previous turbo write control was
> >> failed.\n",
> >>> +			__func__);
> >>> +#endif
> >>> +	if (enable) {
> >>> +		if (ufshcd_is_ctmwb_on(hba_ctmwb)) {
> >>> +			dev_err(hba->dev, "%s: turbo write already enabled.
> >> ctmwb_state = %d\n",
> >>> +				__func__, hba_ctmwb.ufs_ctmwb_state);
> >>> +			return 0;
> >>> +		}
> >>> +		pm_runtime_get_sync(hba->dev);
> >>> +		err = ufshcd_query_flag_retry(hba,
> >> UPIU_QUERY_OPCODE_SET_FLAG,
> >>> +					QUERY_FLAG_IDN_WB_EN, NULL);
> >>> +		if (err) {
> >>> +			ufshcd_set_ctmwb_err(hba_ctmwb);
> >>> +			dev_err(hba->dev, "%s: enable turbo write failed. err
> >> = %d\n",
> >>> +				__func__, err);
> >>> +		} else {
> >>> +			ufshcd_set_ctmwb_on(hba_ctmwb);
> >>> +			dev_info(hba->dev, "%s: ufs turbo write enabled \n",
> >> __func__);
> >>> +		}
> >>> +	} else {
> >>> +		if (ufshcd_is_ctmwb_off(hba_ctmwb)) {
> >>> +			dev_err(hba->dev, "%s: turbo write already disabled.
> >> ctmwb_state = %d\n",
> >>> +				__func__, hba_ctmwb.ufs_ctmwb_state);
> >>> +			return 0;
> >>> +		}
> >>> +		pm_runtime_get_sync(hba->dev);
> >>> +		err = ufshcd_query_flag_retry(hba,
> >> UPIU_QUERY_OPCODE_CLEAR_FLAG,
> >>> +					QUERY_FLAG_IDN_WB_EN, NULL);
> >>> +		if (err) {
> >>> +			ufshcd_set_ctmwb_err(hba_ctmwb);
> >>> +			dev_err(hba->dev, "%s: disable turbo write failed. err
> >> = %d\n",
> >>> +				__func__, err);
> >>> +		} else {
> >>> +			ufshcd_set_ctmwb_off(hba_ctmwb);
> >>> +			dev_info(hba->dev, "%s: ufs turbo write disabled \n",
> >> __func__);
> >> What is 'turbo write'?
> > I wrote it wrong. I will collect it.
> >
> >>> +		}
> >>> +	}
> >>> +
> >>> +	pm_runtime_put_sync(hba->dev);
> >>> +
> >>> +	return 0;
> >>> +}
> >>> +
> >>> +/**
> >>> + * ufshcd_get_ctmwbbuf_unit - get ctmwb buffer alloc units
> >>> + * @sdev: pointer to SCSI device
> >>> + *
> >>> + * Read dLUNumTurboWriteBufferAllocUnits in UNIT Descriptor
> >>> + * to check if LU supports turbo write feature  */ static int
> >>> +ufshcd_get_ctmwbbuf_unit(struct ufs_hba *hba) {
> >>> +	struct scsi_device *sdev = hba->sdev_ufs_device;
> >>> +	struct ufshba_ctmwb *hba_ctmwb = (struct ufshba_ctmwb *)hba->wb_ops;
> >>> +	int ret = 0;
> >>> +
> >>> +	u32 dLUNumTurboWriteBufferAllocUnits = 0;
> >>> +	u8 desc_buf[4];
> >>> +
> >>> +	if (!hba_ctmwb->support_ctmwb)
> >>> +		return 0;
> >>> +
> >>> +	ret = ufshcd_read_unit_desc_param(hba,
> >>> +			ufshcd_scsi_to_upiu_lun(sdev->lun),
> >>> +			UNIT_DESC_PARAM_WB_BUF_ALLOC_UNITS,
> >>> +			desc_buf,
> >>> +			sizeof(dLUNumTurboWriteBufferAllocUnits));
> >>> +
> >>> +	/* Some WLUN doesn't support unit descriptor */
> >>> +	if ((ret == -EOPNOTSUPP) || scsi_is_wlun(sdev->lun)){
> >>> +		hba_ctmwb->support_ctmwb_lu = false;
> >>> +		dev_info(hba->dev,"%s: do not support WB\n", __func__);
> >>> +		return 0;
> >>> +	}
> >>> +
> >>> +	dLUNumTurboWriteBufferAllocUnits = ((desc_buf[0] << 24)|
> >>> +			(desc_buf[1] << 16) |
> >>> +			(desc_buf[2] << 8) |
> >>> +			desc_buf[3]);
> >>> +
> >>> +	if (dLUNumTurboWriteBufferAllocUnits) {
> >>> +		hba_ctmwb->support_ctmwb_lu = true;
> >>> +		dev_info(hba->dev, "%s: LU %d supports ctmwb, ctmwbbuf unit :
> >> 0x%x\n",
> >>> +				__func__, (int)sdev->lun,
> >> dLUNumTurboWriteBufferAllocUnits);
> >>> +	} else
> >>> +		hba_ctmwb->support_ctmwb_lu = false;
> >>> +
> >>> +	return 0;
> >>> +}
> >>> +
> >>> +static inline int ufshcd_ctmwb_toggle_flush(struct ufs_hba *hba,
> >>> +enum ufs_pm_op pm_op) {
> >>> +	ufshcd_ctmwb_flush_ctrl(hba);
> >>> +
> >>> +	if (ufshcd_is_system_pm(pm_op))
> >>> +		ufshcd_reset_ctmwb(hba, true);
> >>> +
> >>> +	return 0;
> >>> +}
> >>> +
> >>> +static struct ufs_wb_ops exynos_ctmwb_ops = {
> >>> +	.wb_toggle_flush_vendor = ufshcd_ctmwb_toggle_flush,
> >>> +	.wb_alloc_units_vendor = ufshcd_get_ctmwbbuf_unit,
> >>> +	.wb_ctrl_vendor = ufshcd_ctmwb_ctrl,
> >>> +	.wb_reset_vendor = ufshcd_reset_ctmwb, };
> >>> +
> >>> +struct ufs_wb_ops *ufshcd_ctmwb_init(void) {
> >>> +	hba_ctmwb.support_ctmwb = 1;
> >>> +
> >>> +	return &exynos_ctmwb_ops;
> >>> +}
> >>> +EXPORT_SYMBOL_GPL(ufshcd_ctmwb_init);
> >>> +
> >>> diff --git a/drivers/scsi/ufs/ufs_ctmwb.h
> >>> b/drivers/scsi/ufs/ufs_ctmwb.h new file mode 100644 index
> >>> 000000000000..073e21a4900b
> >>> --- /dev/null
> >>> +++ b/drivers/scsi/ufs/ufs_ctmwb.h
> >>> @@ -0,0 +1,27 @@
> >>> +#ifndef _UFS_CTMWB_H_
> >>> +#define _UFS_CTMWB_H_
> >>> +
> >>> +enum ufs_ctmwb_state {
> >>> +       UFS_WB_OFF_STATE	= 0,    /* turbo write disabled state */
> >>> +       UFS_WB_ON_STATE	= 1,            /* turbo write enabled state */
> >>> +       UFS_WB_ERR_STATE	= 2,            /* turbo write error state */
> >>> +};
> >>> +
> >>> +#define ufshcd_is_ctmwb_off(hba) ((hba).ufs_ctmwb_state ==
> >>> +UFS_WB_OFF_STATE) #define ufshcd_is_ctmwb_on(hba)
> >>> +((hba).ufs_ctmwb_state == UFS_WB_ON_STATE) #define
> >>> +ufshcd_is_ctmwb_err(hba) ((hba).ufs_ctmwb_state ==
> >>> +UFS_WB_ERR_STATE) #define ufshcd_set_ctmwb_off(hba)
> >>> +((hba).ufs_ctmwb_state =
> >>> +UFS_WB_OFF_STATE) #define ufshcd_set_ctmwb_on(hba)
> >>> +((hba).ufs_ctmwb_state = UFS_WB_ON_STATE) #define
> >>> +ufshcd_set_ctmwb_err(hba) ((hba).ufs_ctmwb_state =
> >>> +UFS_WB_ERR_STATE)
> >>> +
> >>> +#define UFS_WB_MANUAL_FLUSH_THRESHOLD	5
> >>> +
> >>> +struct ufshba_ctmwb {
> >>> +	enum ufs_ctmwb_state ufs_ctmwb_state;
> >>> +	bool support_ctmwb;
> >>> +
> >>> +	bool support_ctmwb_lu;
> >>> +};
> >>> +
> >>> +struct ufs_wb_ops *ufshcd_ctmwb_init(void); #endif
> >>>
> >>
> >> -Asutosh
> >>
> >> --
> >> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
> >> Forum, Linux Foundation Collaborative Project
> >
> 
> -Asutosh
> 
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> Linux Foundation Collaborative Project

Re: [RFC PATCH v2 3/3] scsi: ufs: add vendor specific write booster To support the fuction of writebooster by vendor. The WB behavior that the vendor wants is slightly different. But we have to support it

[Asutosh Das (asd)]

On 7/24/2020 2:42 AM, 서호영 wrote:
>>>> On 7/20/2020 3:40 AM, SEO HOYOUNG wrote:
>>>>> Signed-off-by: SEO HOYOUNG <hy50.seo@samsung.com>
>>>>> ---
>>>>>     drivers/scsi/ufs/Makefile     |   1 +
>>>>>     drivers/scsi/ufs/ufs-exynos.c |   6 +
>>>>>     drivers/scsi/ufs/ufs_ctmwb.c  | 279
>> ++++++++++++++++++++++++++++++++++
>>>>>     drivers/scsi/ufs/ufs_ctmwb.h  |  27 ++++
>>>>>     4 files changed, 313 insertions(+)
>>>>>     create mode 100644 drivers/scsi/ufs/ufs_ctmwb.c
>>>>>     create mode 100644 drivers/scsi/ufs/ufs_ctmwb.h
>>>>>
>>>>> diff --git a/drivers/scsi/ufs/Makefile b/drivers/scsi/ufs/Makefile
>>>>> index 9810963bc049..b1ba36c7d66f 100644
>>>>> --- a/drivers/scsi/ufs/Makefile
>>>>> +++ b/drivers/scsi/ufs/Makefile
>>>>> @@ -5,6 +5,7 @@ obj-$(CONFIG_SCSI_UFS_DWC_TC_PLATFORM) +=
>>>>> tc-dwc-g210-
>>>> pltfrm.o ufshcd-dwc.o tc-d
>>>>>     obj-$(CONFIG_SCSI_UFS_CDNS_PLATFORM) += cdns-pltfrm.o
>>>>>     obj-$(CONFIG_SCSI_UFS_QCOM) += ufs-qcom.o
>>>>>     obj-$(CONFIG_SCSI_UFS_EXYNOS) += ufs-exynos.o
>>>>> +obj-$(CONFIG_SCSI_UFS_VENDOR_WB) += ufs_ctmwb.o
>>>>>     obj-$(CONFIG_SCSI_UFSHCD) += ufshcd-core.o
>>>>>     ufshcd-core-y				+= ufshcd.o ufs-sysfs.o
>>>>>     ufshcd-core-$(CONFIG_SCSI_UFS_BSG)	+= ufs_bsg.o
>>>>> diff --git a/drivers/scsi/ufs/ufs-exynos.c
>>>>> b/drivers/scsi/ufs/ufs-exynos.c index 32b61ba77241..f127f5f2bf36
>>>>> 100644
>>>>> --- a/drivers/scsi/ufs/ufs-exynos.c
>>>>> +++ b/drivers/scsi/ufs/ufs-exynos.c
>>>>> @@ -22,6 +22,9 @@
>>>>>
>>>>
>>>> To me it looks like, you want to have your own flush policy &
>>>> initializations etc, is that understanding correct?
>>>> I don't understand why though. The current implementation is spec
>>>> compliant. If there're benefits that you see in this implementation,
>>>> please highlight those. It'd be interesting to see that.
>>>
>>> Yes. I want to own flush policy, initialization..
>>> I already know current implementation is spec compliant.
>>> But some vendor want to change flush policy.
>> Ok. It'd be interesting to know the benefits of your flush policy over the
>> current one. If it's better, we can replace the current policy, perhaps?
>> So please can you highlight those benefits.
> At first, our vendor device did not support gear scaling. So we can't use gear scaling code at mainline.
Fair enough.
> And we want to WB disable after probe. If IO state busy, then we will enable WB with IO monitoring.
> I think always enable WB, then maybe occur power related problems.
> So we want to modify those.
FWIW - I'd done power profiling of WB but didn't see any worrisome 
power-impact.
>>> So we modify below code.
>>> Additionally when use below code, we can use WB without UFS 3.1
>>> devices
>> I guess non-standard stuff should be kept out of ufshcd.c to vendor
>> specific files, which I guess you're doing.
>>>>
>>>>
>>>>>     #include "ufs-exynos.h"
>>>>>
>>>>> +#ifdef CONFIG_SCSI_UFS_VENDOR_WB
>>>>> +#include "ufs_ctmwb.h"
>>>>> +#endif
>>>>>     /*
>>>>>      * Exynos's Vendor specific registers for UFSHCI
>>>>>      */
>>>>> @@ -989,6 +992,9 @@ static int exynos_ufs_init(struct ufs_hba *hba)
>>>>>     		goto phy_off;
>>>>>
>>>>>     	ufs->hba = hba;
>>>>> +#ifdef CONFIG_SCSI_UFS_VENDOR_WB
>>>>> +	ufs->hba->wb_ops = ufshcd_ctmwb_init(); #endif
>>>>>     	ufs->opts = ufs->drv_data->opts;
>>>>>     	ufs->rx_sel_idx = PA_MAXDATALANES;
>>>>>     	if (ufs->opts & EXYNOS_UFS_OPT_BROKEN_RX_SEL_IDX) diff --git
>>>>> a/drivers/scsi/ufs/ufs_ctmwb.c b/drivers/scsi/ufs/ufs_ctmwb.c new
>>>>> file mode 100644 index 000000000000..ab39f40721ae
>>>>> --- /dev/null
>>>>> +++ b/drivers/scsi/ufs/ufs_ctmwb.c
>>>>> @@ -0,0 +1,279 @@
>>>>> +#include "ufshcd.h"
>>>>> +#include "ufshci.h"
>>>>> +#include "ufs_ctmwb.h"
>>>>> +
>>>>> +static struct ufshba_ctmwb hba_ctmwb;
>>>>> +
>>>>> +/* Query request retries */
>>>>> +#define QUERY_REQ_RETRIES 3
>>>>> +
>>>>> +static int ufshcd_query_attr_retry(struct ufs_hba *hba,
>>>>> +	enum query_opcode opcode, enum attr_idn idn, u8 index, u8 selector,
>>>>> +	u32 *attr_val)
>>>>> +{
>>>>> +	int ret = 0;
>>>>> +	u32 retries;
>>>>> +
>>>>> +	 for (retries = QUERY_REQ_RETRIES; retries > 0; retries--) {
>>>>> +		ret = ufshcd_query_attr(hba, opcode, idn, index,
>>>>> +						selector, attr_val);
>>>>> +		if (ret)
>>>>> +			dev_dbg(hba->dev, "%s: failed with error %d,
>>>> retries %d\n",
>>>>> +				__func__, ret, retries);
>>>>> +		else
>>>>> +			break;
>>>>> +	}
>>>>> +
>>>>> +	if (ret)
>>>>> +		dev_err(hba->dev,
>>>>> +			"%s: query attribute, idn %d, failed with error %d
>>>> after %d retires\n",
>>>>> +			__func__, idn, ret, QUERY_REQ_RETRIES);
>>>>> +	return ret;
>>>>> +}
>>>>> +
>>>>> +static int ufshcd_query_flag_retry(struct ufs_hba *hba,
>>>>> +	enum query_opcode opcode, enum flag_idn idn, bool *flag_res) {
>>>>> +	int ret;
>>>>> +	int retries;
>>>>> +
>>>>> +	for (retries = 0; retries < QUERY_REQ_RETRIES; retries++) {
>>>>> +		ret = ufshcd_query_flag(hba, opcode, idn, flag_res);
>>>>> +		if (ret)
>>>>> +			dev_dbg(hba->dev,
>>>>> +				"%s: failed with error %d, retries %d\n",
>>>>> +				__func__, ret, retries);
>>>>> +		else
>>>>> +			break;
>>>>> +	}
>>>>> +
>>>>> +	if (ret)
>>>>> +		dev_err(hba->dev,
>>>>> +			"%s: query attribute, opcode %d, idn %d, failed with
>>>> error %d after %d retries\n",
>>>>> +			__func__, (int)opcode, (int)idn, ret, retries);
>>>>> +	return ret;
>>>>> +}
>>>>> +
>>>>> +static int ufshcd_reset_ctmwb(struct ufs_hba *hba, bool force) {
>>>>> +	int err = 0;
>>>>> +
>>>>> +	if (!hba_ctmwb.support_ctmwb)
>>>>> +		return 0;
>>>>> +
>>>>> +	if (ufshcd_is_ctmwb_off(hba_ctmwb)) {
>>>>> +		dev_info(hba->dev, "%s: turbo write already disabled.
>>>> ctmwb_state = %d\n",
>>>>> +			__func__, hba_ctmwb.ufs_ctmwb_state);
>>>>> +		return 0;
>>>>> +	}
>>>>> +
>>>>> +	if (ufshcd_is_ctmwb_err(hba_ctmwb))
>>>>> +		dev_err(hba->dev, "%s: previous turbo write control was
>>>> failed.\n",
>>>>> +			__func__);
>>>>> +
>>>>> +	if (force)
>>>>> +		err = ufshcd_query_flag_retry(hba,
>>>> UPIU_QUERY_OPCODE_CLEAR_FLAG,
>>>>> +				QUERY_FLAG_IDN_WB_EN, NULL);
>>>>> +
>>>>> +	if (err) {
>>>>> +		ufshcd_set_ctmwb_err(hba_ctmwb);
>>>>> +		dev_err(hba->dev, "%s: disable turbo write failed. err
>>>> = %d\n",
>>>>> +			__func__, err);
>>>>> +	} else {
>>>>> +		ufshcd_set_ctmwb_off(hba_ctmwb);
>>>>> +		dev_info(hba->dev, "%s: ufs turbo write disabled \n",
>>>> __func__);
>>>>> +	}
>>>>> +
>>>>> +	return 0;
>>>>> +}
>>>>> +
>>>>> +static int ufshcd_get_ctmwb_buf_status(struct ufs_hba *hba, u32
>>>>> +*status) {
>>>>> +	return ufshcd_query_attr_retry(hba, UPIU_QUERY_OPCODE_READ_ATTR,
>>>>> +			QUERY_ATTR_IDN_AVAIL_WB_BUFF_SIZE, 0, 0, status); }
>>>>> +
>>>>> +static int ufshcd_ctmwb_manual_flush_ctrl(struct ufs_hba *hba, int
>>>>> +en) {
>>>>> +	int err = 0;
>>>>> +
>>>>> +	dev_info(hba->dev, "%s: %sable turbo write manual flush\n",
>>>>> +				__func__, en ? "en" : "dis");
>>>>> +	if (en) {
>>>>> +		err = ufshcd_query_flag_retry(hba,
>>>> UPIU_QUERY_OPCODE_SET_FLAG,
>>>>> +					QUERY_FLAG_IDN_WB_BUFF_FLUSH_EN, NULL);
>>>>> +		if (err)
>>>>> +			dev_err(hba->dev, "%s: enable turbo write failed. err
>>>> = %d\n",
>>>>> +				__func__, err);
>>>>> +	} else {
>>>>> +		err = ufshcd_query_flag_retry(hba,
>>>> UPIU_QUERY_OPCODE_CLEAR_FLAG,
>>>>> +					QUERY_FLAG_IDN_WB_BUFF_FLUSH_EN, NULL);
>>>>> +		if (err)
>>>>> +			dev_err(hba->dev, "%s: disable turbo write failed. err
>>>> = %d\n",
>>>>> +				__func__, err);
>>>>> +	}
>>>>> +
>>>>> +	return err;
>>>>> +}
>>>>> +
>>>>> +static int ufshcd_ctmwb_flush_ctrl(struct ufs_hba *hba) {
>>>>> +	int err = 0;
>>>>> +	u32 curr_status = 0;
>>>>> +
>>>>> +	err = ufshcd_get_ctmwb_buf_status(hba, &curr_status);
>>>>> +
>>>>> +	if (!err && (curr_status <= UFS_WB_MANUAL_FLUSH_THRESHOLD)) {
>>>>> +		dev_info(hba->dev, "%s: enable ctmwb manual flush, buf
>>>> status : %d\n",
>>>>> +				__func__, curr_status);
>>>>> +		scsi_block_requests(hba->host);
>>>>> +		err = ufshcd_ctmwb_manual_flush_ctrl(hba, 1);
>>>>> +		if (!err) {
>>>>> +			mdelay(100);
>>>>> +			err = ufshcd_ctmwb_manual_flush_ctrl(hba, 0);
>>>>> +			if (err)
>>>>> +				dev_err(hba->dev, "%s: disable ctmwb manual
>>>> flush failed. err = %d\n",
>>>>> +						__func__, err);
>>>>> +		} else
>>>>> +			dev_err(hba->dev, "%s: enable ctmwb manual flush
>>>> failed. err = %d\n",
>>>>> +					__func__, err);
>>>>> +		scsi_unblock_requests(hba->host);
>>>>> +	}
>>>>> +	return err;
>>>>> +}
>>>>> +
>>>>> +static int ufshcd_ctmwb_ctrl(struct ufs_hba *hba, bool enable) {
>>>>> +	int err;
>>>>> +#if 0
>>>> Did you miss removing these #if 0?
>>> I will modify this code.
>>>>
>>>>> +	if (!hba->support_ctmwb)
>>>>> +		return;
>>>>> +
>>>>> +	if (hba->pm_op_in_progress) {
>>>>> +		dev_err(hba->dev, "%s: ctmwb ctrl during pm operation is not
>>>> allowed.\n",
>>>>> +			__func__);
>>>>> +		return;
>>>>> +	}
>>>>> +
>>>>> +	if (hba->ufshcd_state != UFSHCD_STATE_OPERATIONAL) {
>>>>> +		dev_err(hba->dev, "%s: ufs host is not available.\n",
>>>>> +			__func__);
>>>>> +		return;
>>>>> +	}
>>>>> +	if (ufshcd_is_ctmwb_err(hba_ctmwb))
>>>>> +		dev_err(hba->dev, "%s: previous turbo write control was
>>>> failed.\n",
>>>>> +			__func__);
>>>>> +#endif
>>>>> +	if (enable) {
>>>>> +		if (ufshcd_is_ctmwb_on(hba_ctmwb)) {
>>>>> +			dev_err(hba->dev, "%s: turbo write already enabled.
>>>> ctmwb_state = %d\n",
>>>>> +				__func__, hba_ctmwb.ufs_ctmwb_state);
>>>>> +			return 0;
>>>>> +		}
>>>>> +		pm_runtime_get_sync(hba->dev);
>>>>> +		err = ufshcd_query_flag_retry(hba,
>>>> UPIU_QUERY_OPCODE_SET_FLAG,
>>>>> +					QUERY_FLAG_IDN_WB_EN, NULL);
>>>>> +		if (err) {
>>>>> +			ufshcd_set_ctmwb_err(hba_ctmwb);
>>>>> +			dev_err(hba->dev, "%s: enable turbo write failed. err
>>>> = %d\n",
>>>>> +				__func__, err);
>>>>> +		} else {
>>>>> +			ufshcd_set_ctmwb_on(hba_ctmwb);
>>>>> +			dev_info(hba->dev, "%s: ufs turbo write enabled \n",
>>>> __func__);
>>>>> +		}
>>>>> +	} else {
>>>>> +		if (ufshcd_is_ctmwb_off(hba_ctmwb)) {
>>>>> +			dev_err(hba->dev, "%s: turbo write already disabled.
>>>> ctmwb_state = %d\n",
>>>>> +				__func__, hba_ctmwb.ufs_ctmwb_state);
>>>>> +			return 0;
>>>>> +		}
>>>>> +		pm_runtime_get_sync(hba->dev);
>>>>> +		err = ufshcd_query_flag_retry(hba,
>>>> UPIU_QUERY_OPCODE_CLEAR_FLAG,
>>>>> +					QUERY_FLAG_IDN_WB_EN, NULL);
>>>>> +		if (err) {
>>>>> +			ufshcd_set_ctmwb_err(hba_ctmwb);
>>>>> +			dev_err(hba->dev, "%s: disable turbo write failed. err
>>>> = %d\n",
>>>>> +				__func__, err);
>>>>> +		} else {
>>>>> +			ufshcd_set_ctmwb_off(hba_ctmwb);
>>>>> +			dev_info(hba->dev, "%s: ufs turbo write disabled \n",
>>>> __func__);
>>>> What is 'turbo write'?
>>> I wrote it wrong. I will collect it.
>>>
>>>>> +		}
>>>>> +	}
>>>>> +
>>>>> +	pm_runtime_put_sync(hba->dev);
>>>>> +
>>>>> +	return 0;
>>>>> +}
>>>>> +
>>>>> +/**
>>>>> + * ufshcd_get_ctmwbbuf_unit - get ctmwb buffer alloc units
>>>>> + * @sdev: pointer to SCSI device
>>>>> + *
>>>>> + * Read dLUNumTurboWriteBufferAllocUnits in UNIT Descriptor
>>>>> + * to check if LU supports turbo write feature  */ static int
>>>>> +ufshcd_get_ctmwbbuf_unit(struct ufs_hba *hba) {
>>>>> +	struct scsi_device *sdev = hba->sdev_ufs_device;
>>>>> +	struct ufshba_ctmwb *hba_ctmwb = (struct ufshba_ctmwb *)hba->wb_ops;
>>>>> +	int ret = 0;
>>>>> +
>>>>> +	u32 dLUNumTurboWriteBufferAllocUnits = 0;
>>>>> +	u8 desc_buf[4];
>>>>> +
>>>>> +	if (!hba_ctmwb->support_ctmwb)
>>>>> +		return 0;
>>>>> +
>>>>> +	ret = ufshcd_read_unit_desc_param(hba,
>>>>> +			ufshcd_scsi_to_upiu_lun(sdev->lun),
>>>>> +			UNIT_DESC_PARAM_WB_BUF_ALLOC_UNITS,
>>>>> +			desc_buf,
>>>>> +			sizeof(dLUNumTurboWriteBufferAllocUnits));
>>>>> +
>>>>> +	/* Some WLUN doesn't support unit descriptor */
>>>>> +	if ((ret == -EOPNOTSUPP) || scsi_is_wlun(sdev->lun)){
>>>>> +		hba_ctmwb->support_ctmwb_lu = false;
>>>>> +		dev_info(hba->dev,"%s: do not support WB\n", __func__);
>>>>> +		return 0;
>>>>> +	}
>>>>> +
>>>>> +	dLUNumTurboWriteBufferAllocUnits = ((desc_buf[0] << 24)|
>>>>> +			(desc_buf[1] << 16) |
>>>>> +			(desc_buf[2] << 8) |
>>>>> +			desc_buf[3]);
>>>>> +
>>>>> +	if (dLUNumTurboWriteBufferAllocUnits) {
>>>>> +		hba_ctmwb->support_ctmwb_lu = true;
>>>>> +		dev_info(hba->dev, "%s: LU %d supports ctmwb, ctmwbbuf unit :
>>>> 0x%x\n",
>>>>> +				__func__, (int)sdev->lun,
>>>> dLUNumTurboWriteBufferAllocUnits);
>>>>> +	} else
>>>>> +		hba_ctmwb->support_ctmwb_lu = false;
>>>>> +
>>>>> +	return 0;
>>>>> +}
>>>>> +
>>>>> +static inline int ufshcd_ctmwb_toggle_flush(struct ufs_hba *hba,
>>>>> +enum ufs_pm_op pm_op) {
>>>>> +	ufshcd_ctmwb_flush_ctrl(hba);
>>>>> +
>>>>> +	if (ufshcd_is_system_pm(pm_op))
>>>>> +		ufshcd_reset_ctmwb(hba, true);
>>>>> +
>>>>> +	return 0;
>>>>> +}
>>>>> +
>>>>> +static struct ufs_wb_ops exynos_ctmwb_ops = {
>>>>> +	.wb_toggle_flush_vendor = ufshcd_ctmwb_toggle_flush,
>>>>> +	.wb_alloc_units_vendor = ufshcd_get_ctmwbbuf_unit,
>>>>> +	.wb_ctrl_vendor = ufshcd_ctmwb_ctrl,
>>>>> +	.wb_reset_vendor = ufshcd_reset_ctmwb, };
>>>>> +
>>>>> +struct ufs_wb_ops *ufshcd_ctmwb_init(void) {
>>>>> +	hba_ctmwb.support_ctmwb = 1;
>>>>> +
>>>>> +	return &exynos_ctmwb_ops;
>>>>> +}
>>>>> +EXPORT_SYMBOL_GPL(ufshcd_ctmwb_init);
>>>>> +
>>>>> diff --git a/drivers/scsi/ufs/ufs_ctmwb.h
>>>>> b/drivers/scsi/ufs/ufs_ctmwb.h new file mode 100644 index
>>>>> 000000000000..073e21a4900b
>>>>> --- /dev/null
>>>>> +++ b/drivers/scsi/ufs/ufs_ctmwb.h
>>>>> @@ -0,0 +1,27 @@
>>>>> +#ifndef _UFS_CTMWB_H_
>>>>> +#define _UFS_CTMWB_H_
>>>>> +
>>>>> +enum ufs_ctmwb_state {
>>>>> +       UFS_WB_OFF_STATE	= 0,    /* turbo write disabled state */
>>>>> +       UFS_WB_ON_STATE	= 1,            /* turbo write enabled state */
>>>>> +       UFS_WB_ERR_STATE	= 2,            /* turbo write error state */
>>>>> +};
>>>>> +
>>>>> +#define ufshcd_is_ctmwb_off(hba) ((hba).ufs_ctmwb_state ==
>>>>> +UFS_WB_OFF_STATE) #define ufshcd_is_ctmwb_on(hba)
>>>>> +((hba).ufs_ctmwb_state == UFS_WB_ON_STATE) #define
>>>>> +ufshcd_is_ctmwb_err(hba) ((hba).ufs_ctmwb_state ==
>>>>> +UFS_WB_ERR_STATE) #define ufshcd_set_ctmwb_off(hba)
>>>>> +((hba).ufs_ctmwb_state =
>>>>> +UFS_WB_OFF_STATE) #define ufshcd_set_ctmwb_on(hba)
>>>>> +((hba).ufs_ctmwb_state = UFS_WB_ON_STATE) #define
>>>>> +ufshcd_set_ctmwb_err(hba) ((hba).ufs_ctmwb_state =
>>>>> +UFS_WB_ERR_STATE)
>>>>> +
>>>>> +#define UFS_WB_MANUAL_FLUSH_THRESHOLD	5
>>>>> +
>>>>> +struct ufshba_ctmwb {
>>>>> +	enum ufs_ctmwb_state ufs_ctmwb_state;
>>>>> +	bool support_ctmwb;
>>>>> +
>>>>> +	bool support_ctmwb_lu;
>>>>> +};
>>>>> +
>>>>> +struct ufs_wb_ops *ufshcd_ctmwb_init(void); #endif
>>>>>
>>>>
>>>> -Asutosh
>>>>
>>>> --
>>>> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
>>>> Forum, Linux Foundation Collaborative Project
>>>
>>
>> -Asutosh
>>
>> --
>> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
>> Linux Foundation Collaborative Project
> 


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