[PATCH V1 0/4] LU queue depth manament

[PATCH V1 0/4] LU queue depth manament

[Dolev Raviv]

Some UFS devices don't support a LUN queue depth same as the device queue
depth. This requires reading the unit descriptor for extracting the LU's
queue depth.

Dolev Raviv (1):
  scsi: ufs: query descriptor API

Raviv Shvili (2):
  scsi: ufs: device query status and size check
  scsi: ufs: Logical Unit (LU) command queue depth

Sujit Reddy Thumma (1):
  scsi: ufs: Fix queue depth handling for best effort cases

 drivers/scsi/ufs/ufs.h    |  38 +++++-
 drivers/scsi/ufs/ufshcd.c | 318 ++++++++++++++++++++++++++++++++--------------
 2 files changed, 261 insertions(+), 95 deletions(-)

-- 
1.8.5.2

-- 
QUALCOMM ISRAEL, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

[PATCH V1 1/4] scsi: ufs: query descriptor API

[Dolev Raviv]

Introduces the API for sending queries with descriptors.
A descriptor is a block or page of parameters that describe the device.
The descriptors are classified into types and can range in size
from 2 bytes through 255 bytes.
All descriptors have a length value as their first element, and a type
identification element as their second byte.
All descriptors are readable and some may be write once.
They are accessed using their type, index and selector.

Signed-off-by: Dolev Raviv <draviv@codeaurora.org>
Signed-off-by: Raviv Shvili <rshvili@codeaurora.org>
---
 drivers/scsi/ufs/ufs.h    |  17 +++++-
 drivers/scsi/ufs/ufshcd.c | 139 +++++++++++++++++++++++++++++++++++++---------
 2 files changed, 128 insertions(+), 28 deletions(-)

diff --git a/drivers/scsi/ufs/ufs.h b/drivers/scsi/ufs/ufs.h
index f42d1ce..1545cd7 100644
--- a/drivers/scsi/ufs/ufs.h
+++ b/drivers/scsi/ufs/ufs.h
@@ -41,7 +41,8 @@
 
 #define MAX_CDB_SIZE	16
 #define GENERAL_UPIU_REQUEST_SIZE 32
-#define QUERY_DESC_MAX_SIZE       256
+#define QUERY_DESC_MAX_SIZE       255
+#define QUERY_DESC_MIN_SIZE       2
 #define QUERY_OSF_SIZE            (GENERAL_UPIU_REQUEST_SIZE - \
 					(sizeof(struct utp_upiu_header)))
 
@@ -117,6 +118,20 @@ enum attr_idn {
 	QUERY_ATTR_IDN_EE_STATUS	= 0x0E,
 };
 
+/* Descriptor idn for Query requests */
+enum desc_idn {
+	QUERY_DESC_IDN_DEVICE		= 0x0,
+	QUERY_DESC_IDN_CONFIGURAION	= 0x1,
+	QUERY_DESC_IDN_UNIT		= 0x2,
+	QUERY_DESC_IDN_RFU_0		= 0x3,
+	QUERY_DESC_IDN_INTERCONNECT	= 0x4,
+	QUERY_DESC_IDN_STRING		= 0x5,
+	QUERY_DESC_IDN_RFU_1		= 0x6,
+	QUERY_DESC_IDN_GEOMETRY		= 0x7,
+	QUERY_DESC_IDN_POWER		= 0x8,
+	QUERY_DESC_IDN_RFU_2		= 0x9,
+};
+
 /* Exception event mask values */
 enum {
 	MASK_EE_STATUS		= 0xFFFF,
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 0c28772..ed533f4 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -459,7 +459,7 @@ void ufshcd_copy_query_response(struct ufs_hba *hba, struct ufshcd_lrb *lrbp)
 
 	/* Get the descriptor */
 	if (lrbp->ucd_rsp_ptr->qr.opcode == UPIU_QUERY_OPCODE_READ_DESC) {
-		u8 *descp = (u8 *)&lrbp->ucd_rsp_ptr +
+		u8 *descp = (u8 *)lrbp->ucd_rsp_ptr +
 				GENERAL_UPIU_REQUEST_SIZE;
 		u16 len;
 
@@ -1134,6 +1134,30 @@ out_put_tag:
 }
 
 /**
+ * ufshcd_init_query() - init the query response and request parameters
+ * @hba: per-adapter instance
+ * @request: address of the request pointer to be initialized
+ * @response: address of the response pointer to be initialized
+ * @opcode: operation to perform
+ * @idn: flag idn to access
+ * @index: LU number to access
+ * @selector: query/flag/descriptor further identification
+ */
+static inline void ufshcd_init_query(struct ufs_hba *hba,
+		struct ufs_query_req **request, struct ufs_query_res **response,
+		enum query_opcode opcode, u8 idn, u8 index, u8 selector)
+{
+	*request = &hba->dev_cmd.query.request;
+	*response = &hba->dev_cmd.query.response;
+	memset(*request, 0, sizeof(struct ufs_query_req));
+	memset(*response, 0, sizeof(struct ufs_query_res));
+	(*request)->upiu_req.opcode = opcode;
+	(*request)->upiu_req.idn = idn;
+	(*request)->upiu_req.index = index;
+	(*request)->upiu_req.selector = selector;
+}
+
+/**
  * ufshcd_query_flag() - API function for sending flag query requests
  * hba: per-adapter instance
  * query_opcode: flag query to perform
@@ -1145,17 +1169,15 @@ out_put_tag:
 static int ufshcd_query_flag(struct ufs_hba *hba, enum query_opcode opcode,
 			enum flag_idn idn, bool *flag_res)
 {
-	struct ufs_query_req *request;
-	struct ufs_query_res *response;
-	int err;
+	struct ufs_query_req *request = NULL;
+	struct ufs_query_res *response = NULL;
+	int err, index = 0, selector = 0;
 
 	BUG_ON(!hba);
 
 	mutex_lock(&hba->dev_cmd.lock);
-	request = &hba->dev_cmd.query.request;
-	response = &hba->dev_cmd.query.response;
-	memset(request, 0, sizeof(struct ufs_query_req));
-	memset(response, 0, sizeof(struct ufs_query_res));
+	ufshcd_init_query(hba, &request, &response, opcode, idn, index,
+			selector);
 
 	switch (opcode) {
 	case UPIU_QUERY_OPCODE_SET_FLAG:
@@ -1180,12 +1202,8 @@ static int ufshcd_query_flag(struct ufs_hba *hba, enum query_opcode opcode,
 		err = -EINVAL;
 		goto out_unlock;
 	}
-	request->upiu_req.opcode = opcode;
-	request->upiu_req.idn = idn;
 
-	/* Send query request */
-	err = ufshcd_exec_dev_cmd(hba, DEV_CMD_TYPE_QUERY,
-			QUERY_REQ_TIMEOUT);
+	err = ufshcd_exec_dev_cmd(hba, DEV_CMD_TYPE_QUERY, QUERY_REQ_TIMEOUT);
 
 	if (err) {
 		dev_err(hba->dev,
@@ -1217,8 +1235,8 @@ out_unlock:
 static int ufshcd_query_attr(struct ufs_hba *hba, enum query_opcode opcode,
 			enum attr_idn idn, u8 index, u8 selector, u32 *attr_val)
 {
-	struct ufs_query_req *request;
-	struct ufs_query_res *response;
+	struct ufs_query_req *request = NULL;
+	struct ufs_query_res *response = NULL;
 	int err;
 
 	BUG_ON(!hba);
@@ -1231,10 +1249,8 @@ static int ufshcd_query_attr(struct ufs_hba *hba, enum query_opcode opcode,
 	}
 
 	mutex_lock(&hba->dev_cmd.lock);
-	request = &hba->dev_cmd.query.request;
-	response = &hba->dev_cmd.query.response;
-	memset(request, 0, sizeof(struct ufs_query_req));
-	memset(response, 0, sizeof(struct ufs_query_res));
+	ufshcd_init_query(hba, &request, &response, opcode, idn, index,
+			selector);
 
 	switch (opcode) {
 	case UPIU_QUERY_OPCODE_WRITE_ATTR:
@@ -1251,14 +1267,7 @@ static int ufshcd_query_attr(struct ufs_hba *hba, enum query_opcode opcode,
 		goto out_unlock;
 	}
 
-	request->upiu_req.opcode = opcode;
-	request->upiu_req.idn = idn;
-	request->upiu_req.index = index;
-	request->upiu_req.selector = selector;
-
-	/* Send query request */
-	err = ufshcd_exec_dev_cmd(hba, DEV_CMD_TYPE_QUERY,
-						QUERY_REQ_TIMEOUT);
+	err = ufshcd_exec_dev_cmd(hba, DEV_CMD_TYPE_QUERY, QUERY_REQ_TIMEOUT);
 
 	if (err) {
 		dev_err(hba->dev, "%s: opcode 0x%.2x for idn %d failed, err = %d\n",
@@ -1275,6 +1284,82 @@ out:
 }
 
 /**
+ * ufshcd_query_descriptor - API function for sending descriptor requests
+ * hba: per-adapter instance
+ * opcode: attribute opcode
+ * idn: attribute idn to access
+ * index: index field
+ * selector: selector field
+ * desc_buf: the buffer that contains the descriptor
+ * buf_len: length parameter passed to the device
+ *
+ * Returns 0 for success, non-zero in case of failure.
+ * The buf_len parameter will contain, on return, the length parameter
+ * received on the response.
+ */
+int ufshcd_query_descriptor(struct ufs_hba *hba,
+			enum query_opcode opcode, enum desc_idn idn, u8 index,
+			u8 selector, u8 *desc_buf, int *buf_len)
+{
+	struct ufs_query_req *request = NULL;
+	struct ufs_query_res *response = NULL;
+	int err;
+
+	BUG_ON(!hba);
+
+	if (!desc_buf) {
+		dev_err(hba->dev, "%s: descriptor buffer required for opcode 0x%x\n",
+				__func__, opcode);
+		err = -EINVAL;
+		goto out;
+	}
+
+	if (*buf_len <= QUERY_DESC_MIN_SIZE || *buf_len > QUERY_DESC_MAX_SIZE) {
+		dev_err(hba->dev, "%s: descriptor buffer size (%d) is out of range\n",
+				__func__, *buf_len);
+		err = -EINVAL;
+		goto out;
+	}
+
+	mutex_lock(&hba->dev_cmd.lock);
+	ufshcd_init_query(hba, &request, &response, opcode, idn, index,
+			selector);
+	hba->dev_cmd.query.descriptor = desc_buf;
+	request->upiu_req.length = *buf_len;
+
+	switch (opcode) {
+	case UPIU_QUERY_OPCODE_WRITE_DESC:
+		request->query_func = UPIU_QUERY_FUNC_STANDARD_WRITE_REQUEST;
+		break;
+	case UPIU_QUERY_OPCODE_READ_DESC:
+		request->query_func = UPIU_QUERY_FUNC_STANDARD_READ_REQUEST;
+		break;
+	default:
+		dev_err(hba->dev,
+				"%s: Expected query descriptor opcode but got = 0x%.2x\n",
+				__func__, opcode);
+		err = -EINVAL;
+		goto out_unlock;
+	}
+
+	err = ufshcd_exec_dev_cmd(hba, DEV_CMD_TYPE_QUERY, QUERY_REQ_TIMEOUT);
+
+	if (err) {
+		dev_err(hba->dev, "%s: opcode 0x%.2x for idn %d failed, err = %d\n",
+				__func__, opcode, idn, err);
+		goto out_unlock;
+	}
+
+	hba->dev_cmd.query.descriptor = NULL;
+	*buf_len = response->upiu_res.length;
+
+out_unlock:
+	mutex_unlock(&hba->dev_cmd.lock);
+out:
+	return err;
+}
+
+/**
  * ufshcd_memory_alloc - allocate memory for host memory space data structures
  * @hba: per adapter instance
  *
-- 
1.8.5.2

-- 
QUALCOMM ISRAEL, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

Re: [PATCH V1 1/4] scsi: ufs: query descriptor API

[Santosh Y]

On Tue, May 27, 2014 at 12:51 PM, Dolev Raviv <draviv@codeaurora.org> wrote:
> Introduces the API for sending queries with descriptors.
> A descriptor is a block or page of parameters that describe the device.
> The descriptors are classified into types and can range in size
> from 2 bytes through 255 bytes.
> All descriptors have a length value as their first element, and a type
> identification element as their second byte.
> All descriptors are readable and some may be write once.
> They are accessed using their type, index and selector.
>
> Signed-off-by: Dolev Raviv <draviv@codeaurora.org>
> Signed-off-by: Raviv Shvili <rshvili@codeaurora.org>
> ---
>  drivers/scsi/ufs/ufs.h    |  17 +++++-
>  drivers/scsi/ufs/ufshcd.c | 139 +++++++++++++++++++++++++++++++++++++---------
>  2 files changed, 128 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/scsi/ufs/ufs.h b/drivers/scsi/ufs/ufs.h
> index f42d1ce..1545cd7 100644
> --- a/drivers/scsi/ufs/ufs.h
> +++ b/drivers/scsi/ufs/ufs.h
> @@ -41,7 +41,8 @@
>
>  #define MAX_CDB_SIZE   16
>  #define GENERAL_UPIU_REQUEST_SIZE 32
> -#define QUERY_DESC_MAX_SIZE       256
> +#define QUERY_DESC_MAX_SIZE       255
> +#define QUERY_DESC_MIN_SIZE       2
>  #define QUERY_OSF_SIZE            (GENERAL_UPIU_REQUEST_SIZE - \
>                                         (sizeof(struct utp_upiu_header)))
>
> @@ -117,6 +118,20 @@ enum attr_idn {
>         QUERY_ATTR_IDN_EE_STATUS        = 0x0E,
>  };
>
> +/* Descriptor idn for Query requests */
> +enum desc_idn {
> +       QUERY_DESC_IDN_DEVICE           = 0x0,
> +       QUERY_DESC_IDN_CONFIGURAION     = 0x1,
> +       QUERY_DESC_IDN_UNIT             = 0x2,
> +       QUERY_DESC_IDN_RFU_0            = 0x3,
> +       QUERY_DESC_IDN_INTERCONNECT     = 0x4,
> +       QUERY_DESC_IDN_STRING           = 0x5,
> +       QUERY_DESC_IDN_RFU_1            = 0x6,
> +       QUERY_DESC_IDN_GEOMETRY         = 0x7,
> +       QUERY_DESC_IDN_POWER            = 0x8,
> +       QUERY_DESC_IDN_RFU_2            = 0x9,
> +};
> +
>  /* Exception event mask values */
>  enum {
>         MASK_EE_STATUS          = 0xFFFF,
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index 0c28772..ed533f4 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -459,7 +459,7 @@ void ufshcd_copy_query_response(struct ufs_hba *hba, struct ufshcd_lrb *lrbp)
>
>         /* Get the descriptor */
>         if (lrbp->ucd_rsp_ptr->qr.opcode == UPIU_QUERY_OPCODE_READ_DESC) {
> -               u8 *descp = (u8 *)&lrbp->ucd_rsp_ptr +
> +               u8 *descp = (u8 *)lrbp->ucd_rsp_ptr +
>                                 GENERAL_UPIU_REQUEST_SIZE;
>                 u16 len;
>
> @@ -1134,6 +1134,30 @@ out_put_tag:
>  }
>
>  /**
> + * ufshcd_init_query() - init the query response and request parameters
> + * @hba: per-adapter instance
> + * @request: address of the request pointer to be initialized
> + * @response: address of the response pointer to be initialized
> + * @opcode: operation to perform
> + * @idn: flag idn to access
> + * @index: LU number to access
> + * @selector: query/flag/descriptor further identification
> + */
> +static inline void ufshcd_init_query(struct ufs_hba *hba,
> +               struct ufs_query_req **request, struct ufs_query_res **response,
> +               enum query_opcode opcode, u8 idn, u8 index, u8 selector)
> +{
> +       *request = &hba->dev_cmd.query.request;
> +       *response = &hba->dev_cmd.query.response;
> +       memset(*request, 0, sizeof(struct ufs_query_req));
> +       memset(*response, 0, sizeof(struct ufs_query_res));
> +       (*request)->upiu_req.opcode = opcode;
> +       (*request)->upiu_req.idn = idn;
> +       (*request)->upiu_req.index = index;
> +       (*request)->upiu_req.selector = selector;
> +}
> +
> +/**
>   * ufshcd_query_flag() - API function for sending flag query requests
>   * hba: per-adapter instance
>   * query_opcode: flag query to perform
> @@ -1145,17 +1169,15 @@ out_put_tag:
>  static int ufshcd_query_flag(struct ufs_hba *hba, enum query_opcode opcode,
>                         enum flag_idn idn, bool *flag_res)
>  {
> -       struct ufs_query_req *request;
> -       struct ufs_query_res *response;
> -       int err;
> +       struct ufs_query_req *request = NULL;
> +       struct ufs_query_res *response = NULL;
> +       int err, index = 0, selector = 0;
>
>         BUG_ON(!hba);
>
>         mutex_lock(&hba->dev_cmd.lock);
> -       request = &hba->dev_cmd.query.request;
> -       response = &hba->dev_cmd.query.response;
> -       memset(request, 0, sizeof(struct ufs_query_req));
> -       memset(response, 0, sizeof(struct ufs_query_res));
> +       ufshcd_init_query(hba, &request, &response, opcode, idn, index,
> +                       selector);
>
>         switch (opcode) {
>         case UPIU_QUERY_OPCODE_SET_FLAG:
> @@ -1180,12 +1202,8 @@ static int ufshcd_query_flag(struct ufs_hba *hba, enum query_opcode opcode,
>                 err = -EINVAL;
>                 goto out_unlock;
>         }
> -       request->upiu_req.opcode = opcode;
> -       request->upiu_req.idn = idn;
>
> -       /* Send query request */
> -       err = ufshcd_exec_dev_cmd(hba, DEV_CMD_TYPE_QUERY,
> -                       QUERY_REQ_TIMEOUT);
> +       err = ufshcd_exec_dev_cmd(hba, DEV_CMD_TYPE_QUERY, QUERY_REQ_TIMEOUT);
>
>         if (err) {
>                 dev_err(hba->dev,
> @@ -1217,8 +1235,8 @@ out_unlock:
>  static int ufshcd_query_attr(struct ufs_hba *hba, enum query_opcode opcode,
>                         enum attr_idn idn, u8 index, u8 selector, u32 *attr_val)
>  {
> -       struct ufs_query_req *request;
> -       struct ufs_query_res *response;
> +       struct ufs_query_req *request = NULL;
> +       struct ufs_query_res *response = NULL;
>         int err;
>
>         BUG_ON(!hba);
> @@ -1231,10 +1249,8 @@ static int ufshcd_query_attr(struct ufs_hba *hba, enum query_opcode opcode,
>         }
>
>         mutex_lock(&hba->dev_cmd.lock);
> -       request = &hba->dev_cmd.query.request;
> -       response = &hba->dev_cmd.query.response;
> -       memset(request, 0, sizeof(struct ufs_query_req));
> -       memset(response, 0, sizeof(struct ufs_query_res));
> +       ufshcd_init_query(hba, &request, &response, opcode, idn, index,
> +                       selector);
>
>         switch (opcode) {
>         case UPIU_QUERY_OPCODE_WRITE_ATTR:
> @@ -1251,14 +1267,7 @@ static int ufshcd_query_attr(struct ufs_hba *hba, enum query_opcode opcode,
>                 goto out_unlock;
>         }
>
> -       request->upiu_req.opcode = opcode;
> -       request->upiu_req.idn = idn;
> -       request->upiu_req.index = index;
> -       request->upiu_req.selector = selector;
> -
> -       /* Send query request */
> -       err = ufshcd_exec_dev_cmd(hba, DEV_CMD_TYPE_QUERY,
> -                                               QUERY_REQ_TIMEOUT);
> +       err = ufshcd_exec_dev_cmd(hba, DEV_CMD_TYPE_QUERY, QUERY_REQ_TIMEOUT);
>
>         if (err) {
>                 dev_err(hba->dev, "%s: opcode 0x%.2x for idn %d failed, err = %d\n",
> @@ -1275,6 +1284,82 @@ out:
>  }
>
>  /**
> + * ufshcd_query_descriptor - API function for sending descriptor requests
> + * hba: per-adapter instance
> + * opcode: attribute opcode
> + * idn: attribute idn to access
> + * index: index field
> + * selector: selector field
> + * desc_buf: the buffer that contains the descriptor
> + * buf_len: length parameter passed to the device
> + *
> + * Returns 0 for success, non-zero in case of failure.
> + * The buf_len parameter will contain, on return, the length parameter
> + * received on the response.
> + */
> +int ufshcd_query_descriptor(struct ufs_hba *hba,
> +                       enum query_opcode opcode, enum desc_idn idn, u8 index,
> +                       u8 selector, u8 *desc_buf, int *buf_len)
> +{
> +       struct ufs_query_req *request = NULL;
> +       struct ufs_query_res *response = NULL;
> +       int err;
> +
> +       BUG_ON(!hba);
> +
> +       if (!desc_buf) {
> +               dev_err(hba->dev, "%s: descriptor buffer required for opcode 0x%x\n",
> +                               __func__, opcode);
> +               err = -EINVAL;
> +               goto out;
> +       }
> +
> +       if (*buf_len <= QUERY_DESC_MIN_SIZE || *buf_len > QUERY_DESC_MAX_SIZE) {
> +               dev_err(hba->dev, "%s: descriptor buffer size (%d) is out of range\n",
> +                               __func__, *buf_len);
> +               err = -EINVAL;
> +               goto out;
> +       }
> +
> +       mutex_lock(&hba->dev_cmd.lock);
> +       ufshcd_init_query(hba, &request, &response, opcode, idn, index,
> +                       selector);
> +       hba->dev_cmd.query.descriptor = desc_buf;
> +       request->upiu_req.length = *buf_len;
> +
> +       switch (opcode) {
> +       case UPIU_QUERY_OPCODE_WRITE_DESC:
> +               request->query_func = UPIU_QUERY_FUNC_STANDARD_WRITE_REQUEST;
> +               break;
> +       case UPIU_QUERY_OPCODE_READ_DESC:
> +               request->query_func = UPIU_QUERY_FUNC_STANDARD_READ_REQUEST;
> +               break;
> +       default:
> +               dev_err(hba->dev,
> +                               "%s: Expected query descriptor opcode but got = 0x%.2x\n",
> +                               __func__, opcode);
> +               err = -EINVAL;
> +               goto out_unlock;
> +       }
> +
> +       err = ufshcd_exec_dev_cmd(hba, DEV_CMD_TYPE_QUERY, QUERY_REQ_TIMEOUT);
> +
> +       if (err) {
> +               dev_err(hba->dev, "%s: opcode 0x%.2x for idn %d failed, err = %d\n",
> +                               __func__, opcode, idn, err);
> +               goto out_unlock;
> +       }
> +
> +       hba->dev_cmd.query.descriptor = NULL;
> +       *buf_len = response->upiu_res.length;
> +
> +out_unlock:
> +       mutex_unlock(&hba->dev_cmd.lock);
> +out:
> +       return err;
> +}
> +
> +/**
>   * ufshcd_memory_alloc - allocate memory for host memory space data structures
>   * @hba: per adapter instance
>   *
> --
> 1.8.5.2
>

Acked-by: Santosh Y <santoshsy@gmail.com>

-- 
~Santosh

[PATCH V1 2/4] scsi: ufs: device query status and size check

[Dolev Raviv]

Check query response status before copying the response.
Add descriptor query response size check, before copying it to buffer.

Signed-off-by: Dolev Raviv <draviv@codeaurora.org>
Signed-off-by: Raviv Shvili <rshvili@codeaurora.org>
---
 drivers/scsi/ufs/ufshcd.c | 49 +++++++++++++++++++++++++++++++----------------
 1 file changed, 32 insertions(+), 17 deletions(-)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index ed533f4..f3768ec 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -446,30 +446,34 @@ static inline void ufshcd_copy_sense_data(struct ufshcd_lrb *lrbp)
  * @lrb - pointer to local reference block
  */
 static
-void ufshcd_copy_query_response(struct ufs_hba *hba, struct ufshcd_lrb *lrbp)
+int ufshcd_copy_query_response(struct ufs_hba *hba, struct ufshcd_lrb *lrbp)
 {
 	struct ufs_query_res *query_res = &hba->dev_cmd.query.response;
 
-	/* Get the UPIU response */
-	query_res->response = ufshcd_get_rsp_upiu_result(lrbp->ucd_rsp_ptr) >>
-			UPIU_RSP_CODE_OFFSET;
-
 	memcpy(&query_res->upiu_res, &lrbp->ucd_rsp_ptr->qr, QUERY_OSF_SIZE);
 
-
 	/* Get the descriptor */
 	if (lrbp->ucd_rsp_ptr->qr.opcode == UPIU_QUERY_OPCODE_READ_DESC) {
 		u8 *descp = (u8 *)lrbp->ucd_rsp_ptr +
 				GENERAL_UPIU_REQUEST_SIZE;
-		u16 len;
+		u16 resp_len;
+		u16 buf_len;
 
 		/* data segment length */
-		len = be32_to_cpu(lrbp->ucd_rsp_ptr->header.dword_2) &
+		resp_len = be32_to_cpu(lrbp->ucd_rsp_ptr->header.dword_2) &
 						MASK_QUERY_DATA_SEG_LEN;
-
-		memcpy(hba->dev_cmd.query.descriptor, descp,
-				min_t(u16, len, QUERY_DESC_MAX_SIZE));
+		buf_len = hba->dev_cmd.query.request.upiu_req.length;
+		if (likely(buf_len >= resp_len)) {
+			memcpy(hba->dev_cmd.query.descriptor, descp, resp_len);
+		} else {
+			dev_warn(hba->dev,
+				"%s: Response size is bigger than buffer",
+				__func__);
+			return -EINVAL;
+		}
 	}
+
+	return 0;
 }
 
 /**
@@ -797,11 +801,9 @@ static void ufshcd_prepare_utp_query_req_upiu(struct ufs_hba *hba,
 			QUERY_OSF_SIZE);
 
 	/* Copy the Descriptor */
-	if ((len > 0) && (query->request.upiu_req.opcode ==
-					UPIU_QUERY_OPCODE_WRITE_DESC)) {
-		memcpy(descp, query->descriptor,
-				min_t(u16, len, QUERY_DESC_MAX_SIZE));
-	}
+	if (query->request.upiu_req.opcode == UPIU_QUERY_OPCODE_WRITE_DESC)
+		memcpy(descp, query->descriptor, len);
+
 }
 
 static inline void ufshcd_prepare_utp_nop_upiu(struct ufshcd_lrb *lrbp)
@@ -980,6 +982,17 @@ ufshcd_clear_cmd(struct ufs_hba *hba, int tag)
 	return err;
 }
 
+static int
+ufshcd_check_query_response(struct ufs_hba *hba, struct ufshcd_lrb *lrbp)
+{
+	struct ufs_query_res *query_res = &hba->dev_cmd.query.response;
+
+	/* Get the UPIU response */
+	query_res->response = ufshcd_get_rsp_upiu_result(lrbp->ucd_rsp_ptr) >>
+				UPIU_RSP_CODE_OFFSET;
+	return query_res->response;
+}
+
 /**
  * ufshcd_dev_cmd_completion() - handles device management command responses
  * @hba: per adapter instance
@@ -1002,7 +1015,9 @@ ufshcd_dev_cmd_completion(struct ufs_hba *hba, struct ufshcd_lrb *lrbp)
 		}
 		break;
 	case UPIU_TRANSACTION_QUERY_RSP:
-		ufshcd_copy_query_response(hba, lrbp);
+		err = ufshcd_check_query_response(hba, lrbp);
+		if (!err)
+			err = ufshcd_copy_query_response(hba, lrbp);
 		break;
 	case UPIU_TRANSACTION_REJECT_UPIU:
 		/* TODO: handle Reject UPIU Response */
-- 
1.8.5.2

-- 
QUALCOMM ISRAEL, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

Re: [PATCH V1 2/4] scsi: ufs: device query status and size check

[Santosh Y]

On Tue, May 27, 2014 at 12:51 PM, Dolev Raviv <draviv@codeaurora.org> wrote:
> Check query response status before copying the response.
> Add descriptor query response size check, before copying it to buffer.
>
> Signed-off-by: Dolev Raviv <draviv@codeaurora.org>
> Signed-off-by: Raviv Shvili <rshvili@codeaurora.org>
> ---
>  drivers/scsi/ufs/ufshcd.c | 49 +++++++++++++++++++++++++++++++----------------
>  1 file changed, 32 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index ed533f4..f3768ec 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -446,30 +446,34 @@ static inline void ufshcd_copy_sense_data(struct ufshcd_lrb *lrbp)
>   * @lrb - pointer to local reference block
>   */
>  static
> -void ufshcd_copy_query_response(struct ufs_hba *hba, struct ufshcd_lrb *lrbp)
> +int ufshcd_copy_query_response(struct ufs_hba *hba, struct ufshcd_lrb *lrbp)
>  {
>         struct ufs_query_res *query_res = &hba->dev_cmd.query.response;
>
> -       /* Get the UPIU response */
> -       query_res->response = ufshcd_get_rsp_upiu_result(lrbp->ucd_rsp_ptr) >>
> -                       UPIU_RSP_CODE_OFFSET;
> -
>         memcpy(&query_res->upiu_res, &lrbp->ucd_rsp_ptr->qr, QUERY_OSF_SIZE);
>
> -
>         /* Get the descriptor */
>         if (lrbp->ucd_rsp_ptr->qr.opcode == UPIU_QUERY_OPCODE_READ_DESC) {
>                 u8 *descp = (u8 *)lrbp->ucd_rsp_ptr +
>                                 GENERAL_UPIU_REQUEST_SIZE;
> -               u16 len;
> +               u16 resp_len;
> +               u16 buf_len;
>
>                 /* data segment length */
> -               len = be32_to_cpu(lrbp->ucd_rsp_ptr->header.dword_2) &
> +               resp_len = be32_to_cpu(lrbp->ucd_rsp_ptr->header.dword_2) &
>                                                 MASK_QUERY_DATA_SEG_LEN;
> -
> -               memcpy(hba->dev_cmd.query.descriptor, descp,
> -                               min_t(u16, len, QUERY_DESC_MAX_SIZE));
> +               buf_len = hba->dev_cmd.query.request.upiu_req.length;
> +               if (likely(buf_len >= resp_len)) {
> +                       memcpy(hba->dev_cmd.query.descriptor, descp, resp_len);
> +               } else {
> +                       dev_warn(hba->dev,
> +                               "%s: Response size is bigger than buffer",
> +                               __func__);
> +                       return -EINVAL;
> +               }
>         }
> +
> +       return 0;
>  }
>
>  /**
> @@ -797,11 +801,9 @@ static void ufshcd_prepare_utp_query_req_upiu(struct ufs_hba *hba,
>                         QUERY_OSF_SIZE);
>
>         /* Copy the Descriptor */
> -       if ((len > 0) && (query->request.upiu_req.opcode ==
> -                                       UPIU_QUERY_OPCODE_WRITE_DESC)) {
> -               memcpy(descp, query->descriptor,
> -                               min_t(u16, len, QUERY_DESC_MAX_SIZE));
> -       }
> +       if (query->request.upiu_req.opcode == UPIU_QUERY_OPCODE_WRITE_DESC)
> +               memcpy(descp, query->descriptor, len);
> +
>  }
>
>  static inline void ufshcd_prepare_utp_nop_upiu(struct ufshcd_lrb *lrbp)
> @@ -980,6 +982,17 @@ ufshcd_clear_cmd(struct ufs_hba *hba, int tag)
>         return err;
>  }
>
> +static int
> +ufshcd_check_query_response(struct ufs_hba *hba, struct ufshcd_lrb *lrbp)
> +{
> +       struct ufs_query_res *query_res = &hba->dev_cmd.query.response;
> +
> +       /* Get the UPIU response */
> +       query_res->response = ufshcd_get_rsp_upiu_result(lrbp->ucd_rsp_ptr) >>
> +                               UPIU_RSP_CODE_OFFSET;
> +       return query_res->response;
> +}
> +
>  /**
>   * ufshcd_dev_cmd_completion() - handles device management command responses
>   * @hba: per adapter instance
> @@ -1002,7 +1015,9 @@ ufshcd_dev_cmd_completion(struct ufs_hba *hba, struct ufshcd_lrb *lrbp)
>                 }
>                 break;
>         case UPIU_TRANSACTION_QUERY_RSP:
> -               ufshcd_copy_query_response(hba, lrbp);
> +               err = ufshcd_check_query_response(hba, lrbp);
> +               if (!err)
> +                       err = ufshcd_copy_query_response(hba, lrbp);
>                 break;
>         case UPIU_TRANSACTION_REJECT_UPIU:
>                 /* TODO: handle Reject UPIU Response */
> --
> 1.8.5.2
>

Acked-by: Santosh Y <santoshsy@gmail.com>


-- 
~Santosh

[PATCH V1 3/4] scsi: ufs: Logical Unit (LU) command queue depth

[Dolev Raviv]

Some of the UFS devices may support different number of commands
that can be queued per LU. At the current implementation,
SW configure each of the UFS devices LU's according to the
controller capability.

In this patch the queue depth available per LU is read and updated in
the LU's SW structure.

Signed-off-by: Dolev Raviv <draviv@codeaurora.org>
Signed-off-by: Raviv Shvili <rshvili@codeaurora.org>
---
 drivers/scsi/ufs/ufs.h    | 21 +++++++++++++++++++++
 drivers/scsi/ufs/ufshcd.c | 45 ++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 65 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/ufs/ufs.h b/drivers/scsi/ufs/ufs.h
index 1545cd7..fafcf5e 100644
--- a/drivers/scsi/ufs/ufs.h
+++ b/drivers/scsi/ufs/ufs.h
@@ -132,6 +132,27 @@ enum desc_idn {
 	QUERY_DESC_IDN_RFU_2		= 0x9,
 };
 
+#define UNIT_DESC_MAX_SIZE       0x22
+/* Unit descriptor parameters offsets in bytes*/
+enum unit_desc_param {
+	UNIT_DESC_PARAM_LEN			= 0x0,
+	UNIT_DESC_PARAM_TYPE			= 0x1,
+	UNIT_DESC_PARAM_UNIT_INDEX		= 0x2,
+	UNIT_DESC_PARAM_LU_ENABLE		= 0x3,
+	UNIT_DESC_PARAM_BOOT_LUN_ID		= 0x4,
+	UNIT_DESC_PARAM_LU_WR_PROTECT		= 0x5,
+	UNIT_DESC_PARAM_LU_Q_DEPTH		= 0x6,
+	UNIT_DESC_PARAM_MEM_TYPE		= 0x8,
+	UNIT_DESC_PARAM_DATA_RELIABILITY	= 0x9,
+	UNIT_DESC_PARAM_LOGICAL_BLK_SIZE	= 0xA,
+	UNIT_DESC_PARAM_LOGICAL_BLK_COUNT	= 0xB,
+	UNIT_DESC_PARAM_ERASE_BLK_SIZE		= 0x13,
+	UNIT_DESC_PARAM_PROVISIONING_TYPE	= 0x17,
+	UNIT_DESC_PARAM_PHY_MEM_RSRC_CNT	= 0x18,
+	UNIT_DESC_PARAM_CTX_CAPABILITIES	= 0x20,
+	UNIT_DESC_PARAM_LARGE_UNIT_SIZE_M1	= 0x22,
+};
+
 /* Exception event mask values */
 enum {
 	MASK_EE_STATUS		= 0xFFFF,
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index f3768ec..b301ed8 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -110,6 +110,8 @@ static void ufshcd_tmc_handler(struct ufs_hba *hba);
 static void ufshcd_async_scan(void *data, async_cookie_t cookie);
 static int ufshcd_reset_and_restore(struct ufs_hba *hba);
 static int ufshcd_clear_tm_cmd(struct ufs_hba *hba, int tag);
+static int ufshcd_read_sdev_qdepth(struct ufs_hba *hba,
+					struct scsi_device *sdev);
 
 /*
  * ufshcd_wait_for_register - wait for register value to change
@@ -1978,6 +1980,7 @@ static int ufshcd_verify_dev_init(struct ufs_hba *hba)
 static int ufshcd_slave_alloc(struct scsi_device *sdev)
 {
 	struct ufs_hba *hba;
+	int lun_qdepth;
 
 	hba = shost_priv(sdev->host);
 	sdev->tagged_supported = 1;
@@ -1988,6 +1991,14 @@ static int ufshcd_slave_alloc(struct scsi_device *sdev)
 
 	/* allow SCSI layer to restart the device in case of errors */
 	sdev->allow_restart = 1;
+	lun_qdepth = ufshcd_read_sdev_qdepth(hba, sdev);
+	if (lun_qdepth == 0 || lun_qdepth > hba->nutrs) {
+		dev_info(hba->dev, "%s, lun %d queue depth is %d\n", __func__,
+				sdev->lun, lun_qdepth);
+		lun_qdepth = hba->nutrs;
+	} else if (lun_qdepth < 0) {
+		lun_qdepth = 1;
+	}
 
 	/*
 	 * Inform SCSI Midlayer that the LUN queue depth is same as the
@@ -1996,7 +2007,7 @@ static int ufshcd_slave_alloc(struct scsi_device *sdev)
 	 * SAM_STAT_TASK_SET_FULL, the LUN queue depth will be adjusted
 	 * with scsi_adjust_queue_depth.
 	 */
-	scsi_activate_tcq(sdev, hba->nutrs);
+	scsi_activate_tcq(sdev, lun_qdepth);
 	return 0;
 }
 
@@ -3071,6 +3082,38 @@ static int ufshcd_eh_host_reset_handler(struct scsi_cmnd *cmd)
 }
 
 /**
+ * ufshcd_read_sdev_qdepth - read the lun command queue depth
+ * @hba: Pointer to adapter instance
+ * @sdev: pointer to SCSI device
+ *
+ * Return in case of success the lun's queue depth else error.
+ */
+static int ufshcd_read_sdev_qdepth(struct ufs_hba *hba,
+				struct scsi_device *sdev)
+{
+	int ret;
+	int buff_len = UNIT_DESC_MAX_SIZE;
+	u8 desc_buf[UNIT_DESC_MAX_SIZE];
+
+	ret = ufshcd_query_descriptor(hba, UPIU_QUERY_OPCODE_READ_DESC,
+			QUERY_DESC_IDN_UNIT, sdev->lun, 0, desc_buf, &buff_len);
+
+	if (ret || (buff_len < UNIT_DESC_PARAM_LU_Q_DEPTH)) {
+		dev_err(hba->dev,
+			"%s:Failed reading unit descriptor. len = %d ret = %d"
+			, __func__, buff_len, ret);
+		if (!ret)
+			ret = -EINVAL;
+
+		goto out;
+	}
+
+	ret = desc_buf[UNIT_DESC_PARAM_LU_Q_DEPTH] & 0xFF;
+out:
+	return ret;
+}
+
+/**
  * ufshcd_async_scan - asynchronous execution for link startup
  * @data: data pointer to pass to this function
  * @cookie: cookie data
-- 
1.8.5.2

-- 
QUALCOMM ISRAEL, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

Re: [PATCH V1 3/4] scsi: ufs: Logical Unit (LU) command queue depth

[Santosh Y]

On Tue, May 27, 2014 at 12:51 PM, Dolev Raviv <draviv@codeaurora.org> wrote:
> Some of the UFS devices may support different number of commands
> that can be queued per LU. At the current implementation,
> SW configure each of the UFS devices LU's according to the
> controller capability.
>
> In this patch the queue depth available per LU is read and updated in
> the LU's SW structure.
>
> Signed-off-by: Dolev Raviv <draviv@codeaurora.org>
> Signed-off-by: Raviv Shvili <rshvili@codeaurora.org>
> ---
>  drivers/scsi/ufs/ufs.h    | 21 +++++++++++++++++++++
>  drivers/scsi/ufs/ufshcd.c | 45 ++++++++++++++++++++++++++++++++++++++++++++-
>  2 files changed, 65 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/scsi/ufs/ufs.h b/drivers/scsi/ufs/ufs.h
> index 1545cd7..fafcf5e 100644
> --- a/drivers/scsi/ufs/ufs.h
> +++ b/drivers/scsi/ufs/ufs.h
> @@ -132,6 +132,27 @@ enum desc_idn {
>         QUERY_DESC_IDN_RFU_2            = 0x9,
>  };
>
> +#define UNIT_DESC_MAX_SIZE       0x22
> +/* Unit descriptor parameters offsets in bytes*/
> +enum unit_desc_param {
> +       UNIT_DESC_PARAM_LEN                     = 0x0,
> +       UNIT_DESC_PARAM_TYPE                    = 0x1,
> +       UNIT_DESC_PARAM_UNIT_INDEX              = 0x2,
> +       UNIT_DESC_PARAM_LU_ENABLE               = 0x3,
> +       UNIT_DESC_PARAM_BOOT_LUN_ID             = 0x4,
> +       UNIT_DESC_PARAM_LU_WR_PROTECT           = 0x5,
> +       UNIT_DESC_PARAM_LU_Q_DEPTH              = 0x6,
> +       UNIT_DESC_PARAM_MEM_TYPE                = 0x8,
> +       UNIT_DESC_PARAM_DATA_RELIABILITY        = 0x9,
> +       UNIT_DESC_PARAM_LOGICAL_BLK_SIZE        = 0xA,
> +       UNIT_DESC_PARAM_LOGICAL_BLK_COUNT       = 0xB,
> +       UNIT_DESC_PARAM_ERASE_BLK_SIZE          = 0x13,
> +       UNIT_DESC_PARAM_PROVISIONING_TYPE       = 0x17,
> +       UNIT_DESC_PARAM_PHY_MEM_RSRC_CNT        = 0x18,
> +       UNIT_DESC_PARAM_CTX_CAPABILITIES        = 0x20,
> +       UNIT_DESC_PARAM_LARGE_UNIT_SIZE_M1      = 0x22,
> +};
> +
>  /* Exception event mask values */
>  enum {
>         MASK_EE_STATUS          = 0xFFFF,
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index f3768ec..b301ed8 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -110,6 +110,8 @@ static void ufshcd_tmc_handler(struct ufs_hba *hba);
>  static void ufshcd_async_scan(void *data, async_cookie_t cookie);
>  static int ufshcd_reset_and_restore(struct ufs_hba *hba);
>  static int ufshcd_clear_tm_cmd(struct ufs_hba *hba, int tag);
> +static int ufshcd_read_sdev_qdepth(struct ufs_hba *hba,
> +                                       struct scsi_device *sdev);
>
>  /*
>   * ufshcd_wait_for_register - wait for register value to change
> @@ -1978,6 +1980,7 @@ static int ufshcd_verify_dev_init(struct ufs_hba *hba)
>  static int ufshcd_slave_alloc(struct scsi_device *sdev)
>  {
>         struct ufs_hba *hba;
> +       int lun_qdepth;
>
>         hba = shost_priv(sdev->host);
>         sdev->tagged_supported = 1;
> @@ -1988,6 +1991,14 @@ static int ufshcd_slave_alloc(struct scsi_device *sdev)
>
>         /* allow SCSI layer to restart the device in case of errors */
>         sdev->allow_restart = 1;
> +       lun_qdepth = ufshcd_read_sdev_qdepth(hba, sdev);
> +       if (lun_qdepth == 0 || lun_qdepth > hba->nutrs) {
> +               dev_info(hba->dev, "%s, lun %d queue depth is %d\n", __func__,
> +                               sdev->lun, lun_qdepth);
> +               lun_qdepth = hba->nutrs;
> +       } else if (lun_qdepth < 0) {
> +               lun_qdepth = 1;
> +       }
>
>         /*
>          * Inform SCSI Midlayer that the LUN queue depth is same as the
> @@ -1996,7 +2007,7 @@ static int ufshcd_slave_alloc(struct scsi_device *sdev)
>          * SAM_STAT_TASK_SET_FULL, the LUN queue depth will be adjusted
>          * with scsi_adjust_queue_depth.
>          */
> -       scsi_activate_tcq(sdev, hba->nutrs);
> +       scsi_activate_tcq(sdev, lun_qdepth);
>         return 0;
>  }
>
> @@ -3071,6 +3082,38 @@ static int ufshcd_eh_host_reset_handler(struct scsi_cmnd *cmd)
>  }
>
>  /**
> + * ufshcd_read_sdev_qdepth - read the lun command queue depth
> + * @hba: Pointer to adapter instance
> + * @sdev: pointer to SCSI device
> + *
> + * Return in case of success the lun's queue depth else error.
> + */
> +static int ufshcd_read_sdev_qdepth(struct ufs_hba *hba,
> +                               struct scsi_device *sdev)
> +{
> +       int ret;
> +       int buff_len = UNIT_DESC_MAX_SIZE;
> +       u8 desc_buf[UNIT_DESC_MAX_SIZE];
> +
> +       ret = ufshcd_query_descriptor(hba, UPIU_QUERY_OPCODE_READ_DESC,
> +                       QUERY_DESC_IDN_UNIT, sdev->lun, 0, desc_buf, &buff_len);
> +
> +       if (ret || (buff_len < UNIT_DESC_PARAM_LU_Q_DEPTH)) {
> +               dev_err(hba->dev,
> +                       "%s:Failed reading unit descriptor. len = %d ret = %d"
> +                       , __func__, buff_len, ret);
> +               if (!ret)
> +                       ret = -EINVAL;
> +
> +               goto out;
> +       }
> +
> +       ret = desc_buf[UNIT_DESC_PARAM_LU_Q_DEPTH] & 0xFF;
> +out:
> +       return ret;
> +}
> +
> +/**
>   * ufshcd_async_scan - asynchronous execution for link startup
>   * @data: data pointer to pass to this function
>   * @cookie: cookie data
> --
> 1.8.5.2
>

Acked-by: Santosh Y <santoshsy@gmail.com>



-- 
~Santosh

[PATCH V1 4/4] scsi: ufs: Fix queue depth handling for best effort cases

[Dolev Raviv]

Some UFS devices may expose bLUQueueDepth field as zero indicating
that the queue depth depends on the number of resources available
for LUN at a particular instant to handle the outstanding transfer
requests. Currently, when response for SCSI command is TASK_FULL
the LLD decrements the queue depth but fails to increment when the
resources are available. The scsi mid-layer handles the change in
queue depth heuristically and offers simple interface with
->change_queue_depth.

Signed-off-by: Sujit Reddy Thumma <sthumma@codeaurora.org>
Signed-off-by: Dolev Raviv <draviv@codeaurora.org>
---
 drivers/scsi/ufs/ufshcd.c | 97 ++++++++++++++++++++---------------------------
 1 file changed, 42 insertions(+), 55 deletions(-)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index b301ed8..b103e95 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -1991,27 +1991,55 @@ static int ufshcd_slave_alloc(struct scsi_device *sdev)
 
 	/* allow SCSI layer to restart the device in case of errors */
 	sdev->allow_restart = 1;
+
 	lun_qdepth = ufshcd_read_sdev_qdepth(hba, sdev);
-	if (lun_qdepth == 0 || lun_qdepth > hba->nutrs) {
-		dev_info(hba->dev, "%s, lun %d queue depth is %d\n", __func__,
-				sdev->lun, lun_qdepth);
+	if (lun_qdepth <= 0)
+		/* eventually, we can figure out the real queue depth */
 		lun_qdepth = hba->nutrs;
-	} else if (lun_qdepth < 0) {
-		lun_qdepth = 1;
-	}
+	else
+		lun_qdepth = min_t(int, lun_qdepth, hba->nutrs);
 
-	/*
-	 * Inform SCSI Midlayer that the LUN queue depth is same as the
-	 * controller queue depth. If a LUN queue depth is less than the
-	 * controller queue depth and if the LUN reports
-	 * SAM_STAT_TASK_SET_FULL, the LUN queue depth will be adjusted
-	 * with scsi_adjust_queue_depth.
-	 */
+	dev_dbg(hba->dev, "%s: activate tcq with queue depth %d\n",
+			__func__, lun_qdepth);
 	scsi_activate_tcq(sdev, lun_qdepth);
+
 	return 0;
 }
 
 /**
+ * ufshcd_change_queue_depth - change queue depth
+ * @sdev: pointer to SCSI device
+ * @depth: required depth to set
+ * @reason: reason for changing the depth
+ *
+ * Change queue depth according to the reason and make sure
+ * the max. limits are not crossed.
+ */
+int ufshcd_change_queue_depth(struct scsi_device *sdev, int depth, int reason)
+{
+	struct ufs_hba *hba = shost_priv(sdev->host);
+
+	if (depth > hba->nutrs)
+		depth = hba->nutrs;
+
+	switch (reason) {
+	case SCSI_QDEPTH_DEFAULT:
+	case SCSI_QDEPTH_RAMP_UP:
+		if (!sdev->tagged_supported)
+			depth = 1;
+		scsi_adjust_queue_depth(sdev, scsi_get_tag_type(sdev), depth);
+		break;
+	case SCSI_QDEPTH_QFULL:
+		scsi_track_queue_full(sdev, depth);
+		break;
+	default:
+		return -EOPNOTSUPP;
+	}
+
+	return depth;
+}
+
+/**
  * ufshcd_slave_destroy - remove SCSI device configurations
  * @sdev: pointer to SCSI device
  */
@@ -2064,42 +2092,6 @@ static int ufshcd_task_req_compl(struct ufs_hba *hba, u32 index, u8 *resp)
 }
 
 /**
- * ufshcd_adjust_lun_qdepth - Update LUN queue depth if device responds with
- *			      SAM_STAT_TASK_SET_FULL SCSI command status.
- * @cmd: pointer to SCSI command
- */
-static void ufshcd_adjust_lun_qdepth(struct scsi_cmnd *cmd)
-{
-	struct ufs_hba *hba;
-	int i;
-	int lun_qdepth = 0;
-
-	hba = shost_priv(cmd->device->host);
-
-	/*
-	 * LUN queue depth can be obtained by counting outstanding commands
-	 * on the LUN.
-	 */
-	for (i = 0; i < hba->nutrs; i++) {
-		if (test_bit(i, &hba->outstanding_reqs)) {
-
-			/*
-			 * Check if the outstanding command belongs
-			 * to the LUN which reported SAM_STAT_TASK_SET_FULL.
-			 */
-			if (cmd->device->lun == hba->lrb[i].lun)
-				lun_qdepth++;
-		}
-	}
-
-	/*
-	 * LUN queue depth will be total outstanding commands, except the
-	 * command for which the LUN reported SAM_STAT_TASK_SET_FULL.
-	 */
-	scsi_adjust_queue_depth(cmd->device, MSG_SIMPLE_TAG, lun_qdepth - 1);
-}
-
-/**
  * ufshcd_scsi_cmd_status - Update SCSI command result based on SCSI status
  * @lrb: pointer to local reference block of completed command
  * @scsi_status: SCSI command status
@@ -2120,12 +2112,6 @@ ufshcd_scsi_cmd_status(struct ufshcd_lrb *lrbp, int scsi_status)
 			  scsi_status;
 		break;
 	case SAM_STAT_TASK_SET_FULL:
-		/*
-		 * If a LUN reports SAM_STAT_TASK_SET_FULL, then the LUN queue
-		 * depth needs to be adjusted to the exact number of
-		 * outstanding commands the LUN can handle at any given time.
-		 */
-		ufshcd_adjust_lun_qdepth(lrbp->cmd);
 	case SAM_STAT_BUSY:
 	case SAM_STAT_TASK_ABORTED:
 		ufshcd_copy_sense_data(lrbp);
@@ -3156,6 +3142,7 @@ static struct scsi_host_template ufshcd_driver_template = {
 	.queuecommand		= ufshcd_queuecommand,
 	.slave_alloc		= ufshcd_slave_alloc,
 	.slave_destroy		= ufshcd_slave_destroy,
+	.change_queue_depth	= ufshcd_change_queue_depth,
 	.eh_abort_handler	= ufshcd_abort,
 	.eh_device_reset_handler = ufshcd_eh_device_reset_handler,
 	.eh_host_reset_handler   = ufshcd_eh_host_reset_handler,
-- 
1.8.5.2

-- 
QUALCOMM ISRAEL, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

Re: [PATCH V1 4/4] scsi: ufs: Fix queue depth handling for best effort cases

[Santosh Y]

On Tue, May 27, 2014 at 12:51 PM, Dolev Raviv <draviv@codeaurora.org> wrote:
> Some UFS devices may expose bLUQueueDepth field as zero indicating
> that the queue depth depends on the number of resources available
> for LUN at a particular instant to handle the outstanding transfer
> requests. Currently, when response for SCSI command is TASK_FULL
> the LLD decrements the queue depth but fails to increment when the
> resources are available. The scsi mid-layer handles the change in
> queue depth heuristically and offers simple interface with
> ->change_queue_depth.
>
> Signed-off-by: Sujit Reddy Thumma <sthumma@codeaurora.org>
> Signed-off-by: Dolev Raviv <draviv@codeaurora.org>
> ---
>  drivers/scsi/ufs/ufshcd.c | 97 ++++++++++++++++++++---------------------------
>  1 file changed, 42 insertions(+), 55 deletions(-)
>
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index b301ed8..b103e95 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -1991,27 +1991,55 @@ static int ufshcd_slave_alloc(struct scsi_device *sdev)
>
>         /* allow SCSI layer to restart the device in case of errors */
>         sdev->allow_restart = 1;
> +
>         lun_qdepth = ufshcd_read_sdev_qdepth(hba, sdev);
> -       if (lun_qdepth == 0 || lun_qdepth > hba->nutrs) {
> -               dev_info(hba->dev, "%s, lun %d queue depth is %d\n", __func__,
> -                               sdev->lun, lun_qdepth);
> +       if (lun_qdepth <= 0)
> +               /* eventually, we can figure out the real queue depth */
>                 lun_qdepth = hba->nutrs;
> -       } else if (lun_qdepth < 0) {
> -               lun_qdepth = 1;
> -       }
> +       else
> +               lun_qdepth = min_t(int, lun_qdepth, hba->nutrs);
>
> -       /*
> -        * Inform SCSI Midlayer that the LUN queue depth is same as the
> -        * controller queue depth. If a LUN queue depth is less than the
> -        * controller queue depth and if the LUN reports
> -        * SAM_STAT_TASK_SET_FULL, the LUN queue depth will be adjusted
> -        * with scsi_adjust_queue_depth.
> -        */
> +       dev_dbg(hba->dev, "%s: activate tcq with queue depth %d\n",
> +                       __func__, lun_qdepth);
>         scsi_activate_tcq(sdev, lun_qdepth);
> +
>         return 0;
>  }
>
>  /**
> + * ufshcd_change_queue_depth - change queue depth
> + * @sdev: pointer to SCSI device
> + * @depth: required depth to set
> + * @reason: reason for changing the depth
> + *
> + * Change queue depth according to the reason and make sure
> + * the max. limits are not crossed.
> + */
> +int ufshcd_change_queue_depth(struct scsi_device *sdev, int depth, int reason)
> +{
> +       struct ufs_hba *hba = shost_priv(sdev->host);
> +
> +       if (depth > hba->nutrs)
> +               depth = hba->nutrs;
> +
> +       switch (reason) {
> +       case SCSI_QDEPTH_DEFAULT:
> +       case SCSI_QDEPTH_RAMP_UP:
> +               if (!sdev->tagged_supported)
> +                       depth = 1;
> +               scsi_adjust_queue_depth(sdev, scsi_get_tag_type(sdev), depth);
> +               break;
> +       case SCSI_QDEPTH_QFULL:
> +               scsi_track_queue_full(sdev, depth);
> +               break;
> +       default:
> +               return -EOPNOTSUPP;
> +       }
> +
> +       return depth;
> +}
> +
> +/**
>   * ufshcd_slave_destroy - remove SCSI device configurations
>   * @sdev: pointer to SCSI device
>   */
> @@ -2064,42 +2092,6 @@ static int ufshcd_task_req_compl(struct ufs_hba *hba, u32 index, u8 *resp)
>  }
>
>  /**
> - * ufshcd_adjust_lun_qdepth - Update LUN queue depth if device responds with
> - *                           SAM_STAT_TASK_SET_FULL SCSI command status.
> - * @cmd: pointer to SCSI command
> - */
> -static void ufshcd_adjust_lun_qdepth(struct scsi_cmnd *cmd)
> -{
> -       struct ufs_hba *hba;
> -       int i;
> -       int lun_qdepth = 0;
> -
> -       hba = shost_priv(cmd->device->host);
> -
> -       /*
> -        * LUN queue depth can be obtained by counting outstanding commands
> -        * on the LUN.
> -        */
> -       for (i = 0; i < hba->nutrs; i++) {
> -               if (test_bit(i, &hba->outstanding_reqs)) {
> -
> -                       /*
> -                        * Check if the outstanding command belongs
> -                        * to the LUN which reported SAM_STAT_TASK_SET_FULL.
> -                        */
> -                       if (cmd->device->lun == hba->lrb[i].lun)
> -                               lun_qdepth++;
> -               }
> -       }
> -
> -       /*
> -        * LUN queue depth will be total outstanding commands, except the
> -        * command for which the LUN reported SAM_STAT_TASK_SET_FULL.
> -        */
> -       scsi_adjust_queue_depth(cmd->device, MSG_SIMPLE_TAG, lun_qdepth - 1);
> -}
> -
> -/**
>   * ufshcd_scsi_cmd_status - Update SCSI command result based on SCSI status
>   * @lrb: pointer to local reference block of completed command
>   * @scsi_status: SCSI command status
> @@ -2120,12 +2112,6 @@ ufshcd_scsi_cmd_status(struct ufshcd_lrb *lrbp, int scsi_status)
>                           scsi_status;
>                 break;
>         case SAM_STAT_TASK_SET_FULL:
> -               /*
> -                * If a LUN reports SAM_STAT_TASK_SET_FULL, then the LUN queue
> -                * depth needs to be adjusted to the exact number of
> -                * outstanding commands the LUN can handle at any given time.
> -                */
> -               ufshcd_adjust_lun_qdepth(lrbp->cmd);
>         case SAM_STAT_BUSY:
>         case SAM_STAT_TASK_ABORTED:
>                 ufshcd_copy_sense_data(lrbp);
> @@ -3156,6 +3142,7 @@ static struct scsi_host_template ufshcd_driver_template = {
>         .queuecommand           = ufshcd_queuecommand,
>         .slave_alloc            = ufshcd_slave_alloc,
>         .slave_destroy          = ufshcd_slave_destroy,
> +       .change_queue_depth     = ufshcd_change_queue_depth,
>         .eh_abort_handler       = ufshcd_abort,
>         .eh_device_reset_handler = ufshcd_eh_device_reset_handler,
>         .eh_host_reset_handler   = ufshcd_eh_host_reset_handler,
> --
> 1.8.5.2
>
> --

Acked-by: Santosh Y <santoshsy@gmail.com>

-- 
~Santosh