Re: [PATCH V2] scsi: ufs: add support for query requests

2013-04-21 Thread Santosh Y
 + * ufshcd_compose_upiu - form UFS Protocol Information Unit(UPIU)
 + * @hba - UFS hba
 + * @lrb - pointer to local reference block
 + */
 +static int ufshcd_compose_upiu(struct ufs_hba *hba, struct ufshcd_lrb
 *lrbp)
 +{
 +   u32 upiu_flags;
 +   int ret = 0;
 +
 +   if (!lrbp) {
 +   dev_err(hba-dev, %s: lrbp can not be NULL\n, __func__);
 +   ret = -EINVAL;
 +   } else if (!lrbp-ucd_req_ptr) {
 +   dev_err(hba-dev, %s: ucd_req_ptr can not be NULL\n,
 +   __func__);
 +   ret = -EINVAL;
 +   } else if (!lrbp-utr_descriptor_ptr) {
 +   dev_err(hba-dev, %s: utr_descriptor_ptr can not be
 NULL\n,
 +   __func__);
 +   ret = -EINVAL;
 +   }
 +   if (!ret)
 +   goto exit;
 +

ufshcd_compose_upiu() is not being called from anywhere else other
than ufshcd_queuecommand(), *lrbp* is being used before calling
ufshcd_compose_upiu(). So, if lrbp is NULL, it will give a problem in
ufshcd_queuecommand() itself and lrbp-ucd_req_ptr,
lrbp-utr_descriptor_ptr are being configured in
ufshcd_memory_config(). Also if lrbp can be NULL then why not hba? So,
the right thing would be to remove these checks.

 switch (ocs) {
 case OCS_SUCCESS:
 -
 /* check if the returned transfer response is valid */
 -   result = ufshcd_is_valid_req_rsp(lrbp-ucd_rsp_ptr);
 +   expected_rsp_code = ufshcd_is_query_req(lrbp) ?
 +   UPIU_TRANSACTION_QUERY_RSP :
 UPIU_TRANSACTION_RESPONSE;
 +   result = ufshcd_is_valid_req_rsp(lrbp-ucd_rsp_ptr,
 +
 expected_rsp_code);
 +
 if (result) {
 dev_err(hba-dev,
 Invalid response = %x\n, result);
 break;
 }

 -   /*
 -* get the response UPIU result to extract
 -* the SCSI command status
 -*/
 -   result = ufshcd_get_rsp_upiu_result(lrbp-ucd_rsp_ptr);
 +   if (ufshcd_is_query_req(lrbp)) {
 +   /*
 +*  Return result = ok, since SCSI layer wouldn't
 +*  know how to handle errors from query requests.
 +*  The result is saved with the response so that
 +*  the ufs_core layer will handle it.
 +*/
 +   result |= DID_OK  16;
 +   ufshcd_copy_query_response(hba, lrbp);
 +   } else {
 +   /*
 +* get the response UPIU result to extract
 +* the SCSI command status
 +*/
 +   result =
 ufshcd_get_rsp_upiu_result(lrbp-ucd_rsp_ptr);

 -   /*
 -* get the result based on SCSI status response
 -* to notify the SCSI midlayer of the command status
 -*/
 -   scsi_status = result  MASK_SCSI_STATUS;
 -   result = ufshcd_scsi_cmd_status(lrbp, scsi_status);
 +   /*
 +* get the result based on SCSI status response
 +* to notify the SCSI midlayer of the command
 status
 +*/
 +   scsi_status = result  MASK_SCSI_STATUS;
 +   result = ufshcd_scsi_cmd_status(lrbp,
 scsi_status);
 +   }
 break;


The following would be better,

 static int
  ufshcd_get_req_rsp(struct utp_upiu_rsp *ucd_rsp_ptr)
  {
 return (be32_to_cpu(ucd_rsp_ptr-header.dword_0)  24);
   }


switch (ocs) {
 case OCS_SUCCESS:

 /* check if the returned transfer response is valid */
   result = ufshcd_get_req_rsp(lrbp-ucd_rsp_ptr);

   switch (result) {
 case UPIU_TRANSACTION_RESPONSE:
  /* perform required steps */
 break;
 case UPIU_TRANSACTION_QUERY_RSP:
  /* perform required steps */
 break;
 case /* reject upiu */:
   you can add a check for Reject UPIU also if you want.
/* return error */
 break;
 default:
 /* return error */
}
  ...
}

--
~Santosh
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH V2] scsi: ufs: add support for query requests

2013-04-18 Thread Dolev Raviv
Hi All,

A minor bug was spotted, as shown blow. The fix will be submitted with the
next version.

 +static int ufshcd_compose_upiu(struct ufs_hba *hba, struct ufshcd_lrb
 *lrbp)
 +{
 + u32 upiu_flags;
 + int ret = 0;
 +
 + if (!lrbp) {
 + dev_err(hba-dev, %s: lrbp can not be NULL\n, __func__);
 + ret = -EINVAL;
 + } else if (!lrbp-ucd_req_ptr) {
 + dev_err(hba-dev, %s: ucd_req_ptr can not be NULL\n,
 + __func__);
 + ret = -EINVAL;
 + } else if (!lrbp-utr_descriptor_ptr) {
 + dev_err(hba-dev, %s: utr_descriptor_ptr can not be NULL\n,
 + __func__);
 + ret = -EINVAL;
 + }
 + if (!ret)
 + goto exit;
should be:
if (ret)

Thanks,
Dolev

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


--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH V2] scsi: ufs: add support for query requests

2013-04-15 Thread Dolev Raviv
Add support for sending UFS query requests through tagged command
queuing. This design allows queuing query requests in any open slot
along with other SCSI commands. In this way there is no need to
save a slot in the requests queue and decrease its size.

A query request is posing to a SCSI command to use native flow. But
unlike normal SCSI command flow, the data and response fields are
filled in UFS host data structure instead of passing as arguments
while queuing into SCSI mlqueue (mid-layer SCSI queue, the requests
from this queue are submitted to the device queue). As per specification
only one query request is allowed to be processed by device. Hence a
mutex lock for protecting data and response fields (hba-query.request and
hba-query.response) needs to be held while query request is in
progress.

The API for submitting a query request is ufs_query_request() in
ufshcd.c. This function is responsible for:
1. Obtaining the SCSI device from the host
2. Keeping the query mutex to prevent multiple queries
3. Storing the required data for sending a query request in ufs_hba
4. Queuing a SCSI vendor specific command to trigger a query request
   in the UFS driver.

The callers of ufs_query_request() are expected to fill the query
command data fields and are to provide an allocated response field
for the driver to fill response fields after request completion.

The request and response upiu is extended in a union to enable using the
same data structure, both for command upiu and query request upiu.

The query request flow is separated from the scsi command flow in:
1. Preparing the request
2. Validating response (error) codes
3. Copying data (only used for descriptor read/write query requests)
4. Copying response/sense

Data error can't be handled in the scsi command native flow. Hence,
we pass the code as without a change back to the submitting layer.

UPIU (UFS Protocol Information Unit) size is increased to 512 bytes
from 128. The UPIU header and the transaction specific fields (SCSI
command or Query Request OSF - Op-code Specific Fields) are 32 bytes
together, the rest is used to transfer extra request data (such as
descriptor in query requests). In order to accommodate the largest
descriptor in the UFS spec (256 bytes) we need to increase the UPIU
size.

Signed-off-by: Dolev Raviv dra...@codeaurora.org
--- 
changes for v2:
- Moved ufs_query_request to ufshcd.c and removed ufs_core.c
- Addressed Santosh's comments

diff --git a/drivers/scsi/ufs/ufs.h b/drivers/scsi/ufs/ufs.h
index 139bc06..5d2208a 100644
--- a/drivers/scsi/ufs/ufs.h
+++ b/drivers/scsi/ufs/ufs.h
@@ -36,10 +36,20 @@
 #ifndef _UFS_H
 #define _UFS_H
 
+#include linux/mutex.h
+#include linux/types.h
+
 #define MAX_CDB_SIZE   16
+#define GENERAL_UPIU_REQUEST_SIZE 32
+#define UPIU_HEADER_DATA_SEGMENT_MAX_SIZE  ((ALIGNED_UPIU_SIZE) - \
+   (GENERAL_UPIU_REQUEST_SIZE))
+#define QUERY_OSF_SIZE ((GENERAL_UPIU_REQUEST_SIZE) - \
+   (sizeof(struct utp_upiu_header)))
+#define UFS_QUERY_RESERVED_SCSI_CMD 0xCC
+#define UFS_QUERY_CMD_SIZE 10
 
 #define UPIU_HEADER_DWORD(byte3, byte2, byte1, byte0)\
-   ((byte3  24) | (byte2  16) |\
+   cpu_to_be32((byte3  24) | (byte2  16) |\
 (byte1  8) | (byte0))
 
 /*
@@ -62,7 +72,7 @@ enum {
UPIU_TRANSACTION_COMMAND= 0x01,
UPIU_TRANSACTION_DATA_OUT   = 0x02,
UPIU_TRANSACTION_TASK_REQ   = 0x04,
-   UPIU_TRANSACTION_QUERY_REQ  = 0x26,
+   UPIU_TRANSACTION_QUERY_REQ  = 0x16,
 };
 
 /* UTP UPIU Transaction Codes Target to Initiator */
@@ -90,6 +100,12 @@ enum {
UPIU_TASK_ATTR_ACA  = 0x03,
 };
 
+/* UPIU Query request function */
+enum {
+   UPIU_QUERY_FUNC_STANDARD_READ_REQUEST = 0x01,
+   UPIU_QUERY_FUNC_STANDARD_WRITE_REQUEST = 0x81,
+};
+
 /* UTP QUERY Transaction Specific Fields OpCode */
 enum {
UPIU_QUERY_OPCODE_NOP   = 0x0,
@@ -103,6 +119,21 @@ enum {
UPIU_QUERY_OPCODE_TOGGLE_FLAG   = 0x8,
 };
 
+/* Query response result code */
+enum {
+   QUERY_RESULT_SUCCESS= 0x00,
+   QUERY_RESULT_NOT_READABLE   = 0xF6,
+   QUERY_RESULT_NOT_WRITEABLE  = 0xF7,
+   QUERY_RESULT_ALREADY_WRITTEN= 0xF8,
+   QUERY_RESULT_INVALID_LENGTH = 0xF9,
+   QUERY_RESULT_INVALID_VALUE  = 0xFA,
+   QUERY_RESULT_INVALID_SELECTOR   = 0xFB,
+   QUERY_RESULT_INVALID_INDEX  = 0xFC,
+   QUERY_RESULT_INVALID_IDN= 0xFD,
+   QUERY_RESULT_INVALID_OPCODE = 0xFE,
+   QUERY_RESULT_GENERAL_FAILURE= 0xFF,
+};
+
 /* UTP Transfer Request Command Type (CT) */
 enum {
UPIU_COMMAND_SET_TYPE_SCSI  = 0x0,
@@ -110,10 +141,17 @@ enum {
UPIU_COMMAND_SET_TYPE_QUERY = 0x2,
 };
 
+/* UTP Transfer Request Command