diff mbox series

[v2,3/5] scsi: ufs: add LU Dedicated buffer mode support for WriteBooster

Message ID 20200429135610.23750-4-stanley.chu@mediatek.com (mailing list archive)
State New, archived
Headers show
Series scsi: ufs: support LU Dedicated buffer type for WriteBooster | expand

Commit Message

Stanley Chu April 29, 2020, 1:56 p.m. UTC
According to UFS specification, there are two WriteBooster mode of
operations: "LU dedicated buffer" mode and "shared buffer" mode.
In the "LU dedicated buffer" mode, the WriteBooster Buffer is
dedicated to a logical unit.

If the device supports the "LU dedicated buffer" mode, this mode is
configured by setting bWriteBoosterBufferType to 00h. The logical
unit WriteBooster Buffer size is configured by setting the
dLUNumWriteBoosterBufferAllocUnits field of the related Unit
Descriptor. Only a value greater than zero enables the WriteBooster
feature in the logical unit.

Modify ufshcd_wb_probe() as above description to support LU Dedicated
buffer type.

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

Comments

Avri Altman April 30, 2020, 8:30 a.m. UTC | #1
> 
>  static void ufshcd_wb_probe(struct ufs_hba *hba, u8 *desc_buf)
>  {
> +       int ret;
> +       u8 lun;
> +       u32 d_lu_wb_buf_alloc = 0;
> +
>         if (hba->desc_size.dev_desc <=
> DEVICE_DESC_PARAM_EXT_UFS_FEATURE_SUP)
>                 goto wb_disabled;
> 
> @@ -6821,15 +6842,35 @@ static void ufshcd_wb_probe(struct ufs_hba
> *hba, u8 *desc_buf)
>         hba->dev_info.b_wb_buffer_type =
>                 desc_buf[DEVICE_DESC_PARAM_WB_TYPE];
> 
> -       hba->dev_info.d_wb_alloc_units =
> -               get_unaligned_be32(desc_buf +
> -                                  DEVICE_DESC_PARAM_WB_SHARED_ALLOC_UNITS);
>         hba->dev_info.b_presrv_uspc_en =
>                 desc_buf[DEVICE_DESC_PARAM_WB_PRESRV_USRSPC_EN];
> 
> -       if (!(hba->dev_info.b_wb_buffer_type &&
> -             hba->dev_info.d_wb_alloc_units))
> -               goto wb_disabled;
> +       if (hba->dev_info.b_wb_buffer_type == WB_BUF_MODE_SHARED) {
> +               hba->dev_info.d_wb_alloc_units =
> +               get_unaligned_be32(desc_buf +
> +                                  DEVICE_DESC_PARAM_WB_SHARED_ALLOC_UNITS);
> +               if (!hba->dev_info.d_wb_alloc_units)
> +                       goto wb_disabled;
> +       } else {
> +               for (lun = 0; lun < hba->dev_info.max_lu_supported; lun++) {
> +                       ret = ufshcd_read_unit_desc_param(hba,
> +                                       lun,
> +                                       UNIT_DESC_PARAM_WB_BUF_ALLOC_UNITS,
> +                                       (u8 *)&d_lu_wb_buf_alloc,
> +                                       sizeof(d_lu_wb_buf_alloc));
> +                       if (ret)
> +                               goto wb_disabled;
> +                       if (d_lu_wb_buf_alloc) {
> +                               hba->dev_info.wb_dedicated_lu = lun;
> +                               break;
Why are you allowing only a single WB lun?
You should allow those buffers for lun0..lun7
Stanley Chu April 30, 2020, 8:37 a.m. UTC | #2
Hi Avri,

On Thu, 2020-04-30 at 08:30 +0000, Avri Altman wrote:
> > 
> >  static void ufshcd_wb_probe(struct ufs_hba *hba, u8 *desc_buf)
> >  {
> > +       int ret;
> > +       u8 lun;
> > +       u32 d_lu_wb_buf_alloc = 0;
> > +
> >         if (hba->desc_size.dev_desc <=
> > DEVICE_DESC_PARAM_EXT_UFS_FEATURE_SUP)
> >                 goto wb_disabled;
> > 
> > @@ -6821,15 +6842,35 @@ static void ufshcd_wb_probe(struct ufs_hba
> > *hba, u8 *desc_buf)
> >         hba->dev_info.b_wb_buffer_type =
> >                 desc_buf[DEVICE_DESC_PARAM_WB_TYPE];
> > 
> > -       hba->dev_info.d_wb_alloc_units =
> > -               get_unaligned_be32(desc_buf +
> > -                                  DEVICE_DESC_PARAM_WB_SHARED_ALLOC_UNITS);
> >         hba->dev_info.b_presrv_uspc_en =
> >                 desc_buf[DEVICE_DESC_PARAM_WB_PRESRV_USRSPC_EN];
> > 
> > -       if (!(hba->dev_info.b_wb_buffer_type &&
> > -             hba->dev_info.d_wb_alloc_units))
> > -               goto wb_disabled;
> > +       if (hba->dev_info.b_wb_buffer_type == WB_BUF_MODE_SHARED) {
> > +               hba->dev_info.d_wb_alloc_units =
> > +               get_unaligned_be32(desc_buf +
> > +                                  DEVICE_DESC_PARAM_WB_SHARED_ALLOC_UNITS);
> > +               if (!hba->dev_info.d_wb_alloc_units)
> > +                       goto wb_disabled;
> > +       } else {
> > +               for (lun = 0; lun < hba->dev_info.max_lu_supported; lun++) {
> > +                       ret = ufshcd_read_unit_desc_param(hba,
> > +                                       lun,
> > +                                       UNIT_DESC_PARAM_WB_BUF_ALLOC_UNITS,
> > +                                       (u8 *)&d_lu_wb_buf_alloc,
> > +                                       sizeof(d_lu_wb_buf_alloc));
> > +                       if (ret)
> > +                               goto wb_disabled;
> > +                       if (d_lu_wb_buf_alloc) {
> > +                               hba->dev_info.wb_dedicated_lu = lun;
> > +                               break;
> Why are you allowing only a single WB lun?
> You should allow those buffers for lun0..lun7

In UFS 3.1 specification, the valid value of bDeviceMaxWriteBoosterLUs
is 1. Therefore only one LU can have WriteBooster buffer.

Thanks,
Stanley
diff mbox series

Patch

diff --git a/drivers/scsi/ufs/ufs.h b/drivers/scsi/ufs/ufs.h
index daac5053b850..eb3d3cebc87d 100644
--- a/drivers/scsi/ufs/ufs.h
+++ b/drivers/scsi/ufs/ufs.h
@@ -330,6 +330,12 @@  enum health_desc_param {
 	HEALTH_DESC_PARAM_LIFE_TIME_EST_B	= 0x4,
 };
 
+/* WriteBooster buffer mode */
+enum {
+	WB_BUF_MODE_LU_DEDICATED	= 0x0,
+	WB_BUF_MODE_SHARED		= 0x1,
+};
+
 /*
  * Logical Unit Write Protect
  * 00h: LU not write protected
@@ -559,6 +565,7 @@  struct ufs_dev_info {
 	bool is_lu_power_on_wp;
 	/* Maximum number of general LU supported by the UFS device */
 	u8 max_lu_supported;
+	u8 wb_dedicated_lu;
 	u16 wmanufacturerid;
 	/*UFS device Product Name */
 	u8 *model;
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 465ee023ea4b..b970a422a5ea 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -5202,9 +5202,18 @@  static bool ufshcd_wb_sup(struct ufs_hba *hba)
 	return ufshcd_is_wb_allowed(hba);
 }
 
+static int ufshcd_wb_get_index(struct ufs_hba *hba)
+{
+	if (hba->dev_info.b_wb_buffer_type == WB_BUF_MODE_LU_DEDICATED)
+		return hba->dev_info.wb_dedicated_lu;
+	else
+		return 0;
+}
+
 static int ufshcd_wb_ctrl(struct ufs_hba *hba, bool enable)
 {
 	int ret;
+	u8 index;
 	enum query_opcode opcode;
 
 	if (!ufshcd_wb_sup(hba))
@@ -5217,8 +5226,9 @@  static int ufshcd_wb_ctrl(struct ufs_hba *hba, bool enable)
 	else
 		opcode = UPIU_QUERY_OPCODE_CLEAR_FLAG;
 
+	index = ufshcd_wb_get_index(hba);
 	ret = ufshcd_query_flag_retry(hba, opcode,
-				      QUERY_FLAG_IDN_WB_EN, 0, NULL);
+				      QUERY_FLAG_IDN_WB_EN, index, NULL);
 	if (ret) {
 		dev_err(hba->dev, "%s write booster %s failed %d\n",
 			__func__, enable ? "enable" : "disable", ret);
@@ -5235,15 +5245,17 @@  static int ufshcd_wb_ctrl(struct ufs_hba *hba, bool enable)
 static int ufshcd_wb_toggle_flush_during_h8(struct ufs_hba *hba, bool set)
 {
 	int val;
+	u8 index;
 
 	if (set)
 		val =  UPIU_QUERY_OPCODE_SET_FLAG;
 	else
 		val = UPIU_QUERY_OPCODE_CLEAR_FLAG;
 
+	index = ufshcd_wb_get_index(hba);
 	return ufshcd_query_flag_retry(hba, val,
-			       QUERY_FLAG_IDN_WB_BUFF_FLUSH_DURING_HIBERN8, 0,
-				       NULL);
+				QUERY_FLAG_IDN_WB_BUFF_FLUSH_DURING_HIBERN8,
+				index, NULL);
 }
 
 static inline void ufshcd_wb_toggle_flush(struct ufs_hba *hba, bool enable)
@@ -5258,13 +5270,15 @@  static inline void ufshcd_wb_toggle_flush(struct ufs_hba *hba, bool enable)
 static int ufshcd_wb_buf_flush_enable(struct ufs_hba *hba)
 {
 	int ret;
+	u8 index;
 
 	if (!ufshcd_wb_sup(hba) || hba->wb_buf_flush_enabled)
 		return 0;
 
+	index = ufshcd_wb_get_index(hba);
 	ret = ufshcd_query_flag_retry(hba, UPIU_QUERY_OPCODE_SET_FLAG,
 				      QUERY_FLAG_IDN_WB_BUFF_FLUSH_EN,
-				      0, NULL);
+				      index, NULL);
 	if (ret)
 		dev_err(hba->dev, "%s WB - buf flush enable failed %d\n",
 			__func__, ret);
@@ -5278,12 +5292,15 @@  static int ufshcd_wb_buf_flush_enable(struct ufs_hba *hba)
 static int ufshcd_wb_buf_flush_disable(struct ufs_hba *hba)
 {
 	int ret;
+	u8 index;
 
 	if (!ufshcd_wb_sup(hba) || !hba->wb_buf_flush_enabled)
 		return 0;
 
+	index = ufshcd_wb_get_index(hba);
 	ret = ufshcd_query_flag_retry(hba, UPIU_QUERY_OPCODE_CLEAR_FLAG,
-				      QUERY_FLAG_IDN_WB_BUFF_FLUSH_EN, 0, NULL);
+				      QUERY_FLAG_IDN_WB_BUFF_FLUSH_EN,
+				      index, NULL);
 	if (ret) {
 		dev_warn(hba->dev, "%s: WB - buf flush disable failed %d\n",
 			 __func__, ret);
@@ -6802,6 +6819,10 @@  static int ufshcd_scsi_add_wlus(struct ufs_hba *hba)
 
 static void ufshcd_wb_probe(struct ufs_hba *hba, u8 *desc_buf)
 {
+	int ret;
+	u8 lun;
+	u32 d_lu_wb_buf_alloc = 0;
+
 	if (hba->desc_size.dev_desc <= DEVICE_DESC_PARAM_EXT_UFS_FEATURE_SUP)
 		goto wb_disabled;
 
@@ -6821,15 +6842,35 @@  static void ufshcd_wb_probe(struct ufs_hba *hba, u8 *desc_buf)
 	hba->dev_info.b_wb_buffer_type =
 		desc_buf[DEVICE_DESC_PARAM_WB_TYPE];
 
-	hba->dev_info.d_wb_alloc_units =
-		get_unaligned_be32(desc_buf +
-				   DEVICE_DESC_PARAM_WB_SHARED_ALLOC_UNITS);
 	hba->dev_info.b_presrv_uspc_en =
 		desc_buf[DEVICE_DESC_PARAM_WB_PRESRV_USRSPC_EN];
 
-	if (!(hba->dev_info.b_wb_buffer_type &&
-	      hba->dev_info.d_wb_alloc_units))
-		goto wb_disabled;
+	if (hba->dev_info.b_wb_buffer_type == WB_BUF_MODE_SHARED) {
+		hba->dev_info.d_wb_alloc_units =
+		get_unaligned_be32(desc_buf +
+				   DEVICE_DESC_PARAM_WB_SHARED_ALLOC_UNITS);
+		if (!hba->dev_info.d_wb_alloc_units)
+			goto wb_disabled;
+	} else {
+		for (lun = 0; lun < hba->dev_info.max_lu_supported; lun++) {
+			ret = ufshcd_read_unit_desc_param(hba,
+					lun,
+					UNIT_DESC_PARAM_WB_BUF_ALLOC_UNITS,
+					(u8 *)&d_lu_wb_buf_alloc,
+					sizeof(d_lu_wb_buf_alloc));
+			if (ret)
+				goto wb_disabled;
+			if (d_lu_wb_buf_alloc) {
+				hba->dev_info.wb_dedicated_lu = lun;
+				break;
+			}
+		}
+
+		if (!d_lu_wb_buf_alloc)
+			goto wb_disabled;
+	}
+
+	return;
 
 wb_disabled:
 	hba->caps &= ~UFSHCD_CAP_WB_EN;