[PATCH v2 2/5] [SCSI] ufshcd: UFS UTP Transfer requests handling

2012-02-23 Thread Santosh Y
From: Santosh Yaraganavi 

This patch adds support for Transfer request handling.

ufshcd includes following implementations:
 - SCSI queuecommand
 - Compose UPIU(UFS Protocol information unit)
 - Issue commands to UFS host controller
 - Handle completed commands

Signed-off-by: Santosh Yaraganavi 
Signed-off-by: Vinayak Holikatti 
Reviewed-by: Arnd Bergmann 
Reviewed-by: Vishak G 
Reviewed-by: Girish K S 
---
v1 -> v2:
- Use bitops.h defined functions to perform bit operations.
  Ex: set_bit(), __set_bit, clear_bit...
- ufshcd_map_sg(): remove BUG_ON on scsi_dma_map() failure
  and return error value.
- ufshcd_compose_upiu(): add missing break in switch case,
  remove unused parameter "hba".
- ufshcd_slave_alloc(): set sdev->use_10_for_ms = 1, since UFS
  does not support MODE_SENSE_6.
- ufshcd_scsi_cmd_status(): add missing break in switch case.
  Add ufshcd_adjust_lun_qdepth to adjust LUN queue depth
  if SAM_STAT_TASK_SET_FULL is received from the device.
- ufshcd_transfer_rsp_status(): combine cases with same error code.

 drivers/scsi/ufs/ufshcd.c |  500 +
 1 files changed, 500 insertions(+), 0 deletions(-)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index abf617e..e4335f5 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -63,6 +63,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 #include "ufs.h"
@@ -75,6 +76,8 @@ enum {
UFSHCD_MAX_CHANNEL  = 0,
UFSHCD_MAX_ID   = 1,
UFSHCD_MAX_LUNS = 8,
+   UFSHCD_CMD_PER_LUN  = 32,
+   UFSHCD_CAN_QUEUE= 32,
 };
 
 /* UFSHCD states */
@@ -128,6 +131,7 @@ struct uic_command {
  * @host: Scsi_Host instance of the driver
  * @pdev: PCI device handle
  * @lrb: local reference block
+ * @outstanding_reqs: Bits representing outstanding transfer requests
  * @capabilities: UFS Controller Capabilities
  * @nutrs: Transfer Request Queue depth supported by controller
  * @nutmrs: Task Management Queue depth supported by controller
@@ -154,6 +158,8 @@ struct ufs_hba {
 
struct ufshcd_lrb *lrb;
 
+   unsigned long outstanding_reqs;
+
u32 capabilities;
int nutrs;
int nutmrs;
@@ -174,12 +180,28 @@ struct ufs_hba {
  * @ucd_cmd_ptr: UCD address of the command
  * @ucd_rsp_ptr: Response UPIU address for this command
  * @ucd_prdt_ptr: PRDT address of the command
+ * @cmd: pointer to SCSI command
+ * @sense_buffer: pointer sense buffer address of the SCSI command
+ * @sense_bufflen: Length of the sense buffer
+ * @scsi_status: SCSI status of the command
+ * @command_type: SCSI, UFS, Query.
+ * @task_tag: Task tag of the command
+ * @lun: LUN of the command
  */
 struct ufshcd_lrb {
struct utp_transfer_req_desc *utr_descriptor_ptr;
struct utp_upiu_cmd *ucd_cmd_ptr;
struct utp_upiu_rsp *ucd_rsp_ptr;
struct ufshcd_sg_entry *ucd_prdt_ptr;
+
+   struct scsi_cmnd *cmd;
+   u8 *sense_buffer;
+   unsigned int sense_bufflen;
+   int scsi_status;
+
+   int command_type;
+   int task_tag;
+   unsigned int lun;
 };
 
 /**
@@ -206,6 +228,18 @@ static inline int ufshcd_is_device_present(u32 reg_hcs)
 }
 
 /**
+ * ufshcd_get_tr_ocs - Get the UTRD Overall Command Status
+ * @lrb: pointer to local command reference block
+ *
+ * This function is used to get the OCS field from UTRD
+ * Returns the OCS field in the UTRD
+ */
+static inline int ufshcd_get_tr_ocs(struct ufshcd_lrb *lrbp)
+{
+   return lrbp->utr_descriptor_ptr->header.dword_2 & MASK_OCS;
+}
+
+/**
  * ufshcd_get_lists_status - Check UCRDY, UTRLRDY and UTMRLRDY
  * @reg: Register value of host controller status
  *
@@ -273,6 +307,36 @@ static inline void ufshcd_free_hba_memory(struct ufs_hba 
*hba)
 }
 
 /**
+ * ufshcd_is_valid_req_rsp - checks if controller TR response is valid
+ * @ucd_rsp_ptr: pointer to response UPIU
+ *
+ * This function checks the response UPIU for valid transaction type in
+ * response field
+ * Returns 0 on success, non-zero on failure
+ */
+static inline int
+ufshcd_is_valid_req_rsp(struct utp_upiu_rsp *ucd_rsp_ptr)
+{
+   return ((be32_to_cpu(ucd_rsp_ptr->header.dword_0) >> 24) ==
+UPIU_TRANSACTION_RESPONSE) ? 0 :
+ (DID_ERROR << 16 |
+  COMMAND_COMPLETE << 8);
+}
+
+/**
+ * ufshcd_get_rsp_upiu_result - Get the result from response UPIU
+ * @ucd_rsp_ptr: pointer to response UPIU
+ *
+ * This function gets the response status and scsi_status from response UPIU
+ * Returns the response result code.
+ */
+static inline int
+ufshcd_get_rsp_upiu_result(struct utp_upiu_rsp *ucd_rsp_ptr)
+{
+   return be32_to_cpu(ucd_rsp_ptr->header.dword_1) & MASK_RSP_UPIU_RESULT;
+}
+
+/**
  * ufshcd_config_int_aggr - Configure interrupt aggregation values
  *  

Re: [PATCH v2 2/5] [SCSI] ufshcd: UFS UTP Transfer requests handling

2012-02-25 Thread Mike Christie
On 02/24/2012 01:19 AM, Santosh Y wrote:
> From: Santosh Yaraganavi 
> 
> This patch adds support for Transfer request handling.
> 
> ufshcd includes following implementations:
>  - SCSI queuecommand
>  - Compose UPIU(UFS Protocol information unit)
>  - Issue commands to UFS host controller
>  - Handle completed commands
> 
> Signed-off-by: Santosh Yaraganavi 
> Signed-off-by: Vinayak Holikatti 
> Reviewed-by: Arnd Bergmann 
> Reviewed-by: Vishak G 
> Reviewed-by: Girish K S 
> ---
> v1 -> v2:
>   - Use bitops.h defined functions to perform bit operations.
> Ex: set_bit(), __set_bit, clear_bit...
>   - ufshcd_map_sg(): remove BUG_ON on scsi_dma_map() failure
> and return error value.
>   - ufshcd_compose_upiu(): add missing break in switch case,
> remove unused parameter "hba".
>   - ufshcd_slave_alloc(): set sdev->use_10_for_ms = 1, since UFS
> does not support MODE_SENSE_6.
>   - ufshcd_scsi_cmd_status(): add missing break in switch case.
> Add ufshcd_adjust_lun_qdepth to adjust LUN queue depth
> if SAM_STAT_TASK_SET_FULL is received from the device.
>   - ufshcd_transfer_rsp_status(): combine cases with same error code.
> 
>  drivers/scsi/ufs/ufshcd.c |  500 
> +
>  1 files changed, 500 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index abf617e..e4335f5 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -63,6 +63,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  
>  #include "ufs.h"
> @@ -75,6 +76,8 @@ enum {
>   UFSHCD_MAX_CHANNEL  = 0,
>   UFSHCD_MAX_ID   = 1,
>   UFSHCD_MAX_LUNS = 8,
> + UFSHCD_CMD_PER_LUN  = 32,
> + UFSHCD_CAN_QUEUE= 32,


Is the can queue right, or are you working around a issue in the shared
tag map code, or is this due to how you are tracking outstanding reqs
(just a unsigned long so limited by that)?

Can you support multiple luns per host? If so, this seems low for normal
HBAs. Is this low due to the hw or something? If you have multiple luns
then you are going to get a burst of 32 IOs and the host will work on
them, then when those are done we will send IO to the next device, then
repeat. For normal HBAs we would have can_queue = cmd_per_lun * X, so we
can do IO on X devices at the same time.



>  };
>  
>  /* UFSHCD states */
> @@ -128,6 +131,7 @@ struct uic_command {
>   * @host: Scsi_Host instance of the driver
>   * @pdev: PCI device handle
>   * @lrb: local reference block
> + * @outstanding_reqs: Bits representing outstanding transfer requests
>   * @capabilities: UFS Controller Capabilities
>   * @nutrs: Transfer Request Queue depth supported by controller
>   * @nutmrs: Task Management Queue depth supported by controller
> @@ -154,6 +158,8 @@ struct ufs_hba {
>  
>   struct ufshcd_lrb *lrb;
>  
> + unsigned long outstanding_reqs;
> +
>   u32 capabilities;
>   int nutrs;
>   int nutmrs;
> @@ -174,12 +180,28 @@ struct ufs_hba {
>   * @ucd_cmd_ptr: UCD address of the command
>   * @ucd_rsp_ptr: Response UPIU address for this command
>   * @ucd_prdt_ptr: PRDT address of the command
> + * @cmd: pointer to SCSI command
> + * @sense_buffer: pointer sense buffer address of the SCSI command
> + * @sense_bufflen: Length of the sense buffer
> + * @scsi_status: SCSI status of the command
> + * @command_type: SCSI, UFS, Query.
> + * @task_tag: Task tag of the command
> + * @lun: LUN of the command
>   */
>  struct ufshcd_lrb {
>   struct utp_transfer_req_desc *utr_descriptor_ptr;
>   struct utp_upiu_cmd *ucd_cmd_ptr;
>   struct utp_upiu_rsp *ucd_rsp_ptr;
>   struct ufshcd_sg_entry *ucd_prdt_ptr;
> +
> + struct scsi_cmnd *cmd;
> + u8 *sense_buffer;
> + unsigned int sense_bufflen;
> + int scsi_status;
> +
> + int command_type;
> + int task_tag;
> + unsigned int lun;
>  };
>  
>  /**
> @@ -206,6 +228,18 @@ static inline int ufshcd_is_device_present(u32 reg_hcs)
>  }
>  
>  /**
> + * ufshcd_get_tr_ocs - Get the UTRD Overall Command Status
> + * @lrb: pointer to local command reference block
> + *
> + * This function is used to get the OCS field from UTRD
> + * Returns the OCS field in the UTRD
> + */
> +static inline int ufshcd_get_tr_ocs(struct ufshcd_lrb *lrbp)
> +{
> + return lrbp->utr_descriptor_ptr->header.dword_2 & MASK_OCS;
> +}
> +
> +/**
>   * ufshcd_get_lists_status - Check UCRDY, UTRLRDY and UTMRLRDY
>   * @reg: Register value of host controller status
>   *
> @@ -273,6 +307,36 @@ static inline void ufshcd_free_hba_memory(struct ufs_hba 
> *hba)
>  }
>  
>  /**
> + * ufshcd_is_valid_req_rsp - checks if controller TR response is valid
> + * @ucd_rsp_ptr: pointer to response UPIU
> + *
> + * This function checks the response UPIU for valid transaction type in
> + * response field
> + * Returns 0 on success, non-zero on failure
> + */
>

Re: [PATCH v2 2/5] [SCSI] ufshcd: UFS UTP Transfer requests handling

2012-02-26 Thread Santosh Y
>>  #include "ufs.h"
>> @@ -75,6 +76,8 @@ enum {
>>       UFSHCD_MAX_CHANNEL      = 0,
>>       UFSHCD_MAX_ID           = 1,
>>       UFSHCD_MAX_LUNS         = 8,
>> +     UFSHCD_CMD_PER_LUN      = 32,
>> +     UFSHCD_CAN_QUEUE        = 32,
>
>
> Is the can queue right, or are you working around a issue in the shared
> tag map code, or is this due to how you are tracking outstanding reqs
> (just a unsigned long so limited by that)?
>
> Can you support multiple luns per host? If so, this seems low for normal
> HBAs. Is this low due to the hw or something? If you have multiple luns
> then you are going to get a burst of 32 IOs and the host will work on
> them, then when those are done we will send IO to the next device, then
> repeat. For normal HBAs we would have can_queue = cmd_per_lun * X, so we
> can do IO on X devices at the same time.
>

The host can support multiple LUNS, but the UFS host controller can
support only 32 outstanding commands.
So can queue is set to 32.

>> + *
>> + * Returns 0 for success, non-zero in case of failure
>> + */
>> +static int ufshcd_queuecommand(struct Scsi_Host *host, struct scsi_cmnd 
>> *cmd)
>> +{
>> +     struct ufshcd_lrb *lrbp;
>> +     struct ufs_hba *hba;
>> +     unsigned long flags;
>> +     int tag;
>> +     int err = 0;
>> +
>> +     hba = (struct ufs_hba *) &host->hostdata[0];
>
>
> Use shost_priv instead of accessing hostdata. Check the rest of the
> driver for this.
>

ok, will use shost_priv.

>> +
>> +/**
>> + * 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 = (struct ufs_hba *) cmd->device->host->hostdata;
>> +
>> +     /*
>> +      * 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
>> + *
>> + * Returns value base on SCSI command status
>> + */
>> +static inline int
>> +ufshcd_scsi_cmd_status(struct ufshcd_lrb *lrbp, int scsi_status)
>> +{
>> +     int result = 0;
>> +
>> +     switch (scsi_status) {
>> +     case SAM_STAT_GOOD:
>> +             result |= DID_OK << 16 |
>> +                       COMMAND_COMPLETE << 8 |
>> +                       SAM_STAT_GOOD;
>> +             break;
>> +     case SAM_STAT_CHECK_CONDITION:
>> +             result |= DRIVER_SENSE << 24 |
>
> scsi ml will set the driver sense bit for you when it completes the
> command in scsi_finish_command. No need for the driver to touch this.
>

ok, i'll change it.

>
>> +                       DRIVER_OK << 16 |
>> +                       COMMAND_COMPLETE << 8 |
>> +                       SAM_STAT_CHECK_CONDITION;
>> +             ufshcd_copy_sense_data(lrbp);
>> +             break;
>> +     case SAM_STAT_BUSY:
>> +             result |= DID_BUS_BUSY << 16 |
>> +                       SAM_STAT_BUSY;
>
> If you got sam stat busy just set that. You do not need to set the host
> byte and you do not want to, because the scsi_error.c handling will be
> incorrect. Instead of retrying until sam stat busy is no longer returned
> (or until cmd->retries * cmd->timeout) you only get 5 retries.
>

Ok, I'll change it.

>
>> +             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);
>> +             result |= DID_SOFT_ERROR << 16 |
>> +                       SAM_STAT_TASK_SET_FULL;
>
>
> Same here. Just set the sam stat part.
>
>
>> +             break;
>> +     case SAM_STAT_TASK_ABORTED:
>> +             result |=  DID_ABORT << 16 |
>> +                        ABORT_TASK << 8 |
>> +                        SAM_STAT_T