On Mon, July 29, 2013, Sujit Reddy Thumma wrote:
> 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 <[email protected]>
> > ---
> > 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.
Yes, scsi status can be set simply here.
>
> 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) |
How can we determine DID_OK in ufshcd_scsi_cmd_status?
I think we can add this only in case of OCS_SUCCESS in
ufshcd_transfer_rsp_status.
> set_msg_byte(COMMAND_COMPLETE) |
I'm not sure that we need to set COMMAND_COMPLETE?
Message is related to SCSI-2.
I didn't get this information on SAM, SBC and SPC.
Do you have any idea?
Thanks,
Seungwon Jeon
> (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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
