On 7/26/2013 7:16 PM, Seungwon Jeon wrote:
Except for 'GOOD' and 'CHECK CONDITION', other status value
in Response UPIU may or may contain sense data. If a non-zero
value is in the Data Segment Length field, it means that UPIU
has Sense Data in the Data Segment area.
Signed-off-by: Seungwon Jeon <tgih.jun@xxxxxxxxxxx>
---
drivers/scsi/ufs/ufs.h | 1 +
drivers/scsi/ufs/ufshcd.c | 27 +++++++++++++++++++--------
2 files changed, 20 insertions(+), 8 deletions(-)
diff --git a/drivers/scsi/ufs/ufs.h b/drivers/scsi/ufs/ufs.h
index 139bc06..737c31b 100644
--- a/drivers/scsi/ufs/ufs.h
+++ b/drivers/scsi/ufs/ufs.h
@@ -114,6 +114,7 @@ enum {
MASK_SCSI_STATUS = 0xFF,
MASK_TASK_RESPONSE = 0xFF00,
MASK_RSP_UPIU_RESULT = 0xFFFF,
+ MASK_RSP_UPIU_DATA_SEG_LEN = 0xFFFF,
};
/* Task management service response */
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 4cf3a2d..688ae0e 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -218,6 +218,20 @@ 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_get_rsp_upiu_data_seg_len - Get the data segment length
+ * from response UPIU
+ * @ucd_rsp_ptr: pointer to response UPIU
+ *
+ * Return the data segment length.
+ */
+static inline int
+ufshcd_get_rsp_upiu_data_seg_len(struct utp_upiu_rsp *ucd_rsp_ptr)
+{
+ return be32_to_cpu(ucd_rsp_ptr->header.dword_2) &
+ MASK_RSP_UPIU_DATA_SEG_LEN;
+}
+
/**
* ufshcd_config_int_aggr - Configure interrupt aggregation values.
* Currently there is no use case where we want to configure
@@ -298,7 +312,8 @@ void ufshcd_send_command(struct ufs_hba *hba, unsigned int task_tag)
static inline void ufshcd_copy_sense_data(struct ufshcd_lrb *lrbp)
{
int len;
- if (lrbp->sense_buffer) {
+ if (lrbp->sense_buffer &&
+ !ufshcd_get_rsp_upiu_data_seg_len(lrbp->ucd_rsp_ptr)) {
len = be16_to_cpu(lrbp->ucd_rsp_ptr->sense_data_len);
memcpy(lrbp->sense_buffer,
lrbp->ucd_rsp_ptr->sense_data,
@@ -1156,21 +1171,17 @@ ufshcd_scsi_cmd_status(struct ufshcd_lrb *lrbp, int scsi_status)
SAM_STAT_CHECK_CONDITION;
The whole function is doing things wrong. It is redundant to interpret
the scsi_status at the LLD layer and make decisions while otherwise the
SCSI layer anyway knows how handle them properly.
Please see scsi_decide_disposition().
Ideally, this is what we should do -
if (response_upiu_sense_data_len > 0 && lrbp->sense_buffer) {
ufshcd_copy_sense_data();
}
result = set_host_byte(DID_OK) |
set_msg_byte(COMMAND_COMPLETE) |
(scsi_status & 0xff);
ufshcd_copy_sense_data(lrbp);
break;
- case SAM_STAT_BUSY:
- result |= SAM_STAT_BUSY;
- 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 |= SAM_STAT_TASK_SET_FULL;
- break;
+ case SAM_STAT_BUSY:
case SAM_STAT_TASK_ABORTED:
- result |= SAM_STAT_TASK_ABORTED;
+ result |= scsi_status;
+ ufshcd_copy_sense_data(lrbp);
break;
default:
result |= DID_ERROR << 16;
--
Regards,
Sujit
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
