| Message ID | 20230721160154.874010-3-bvanassche@acm.org |
|---|---|
| State | New |
| Headers | show |
| Series | Fix residual handling in two SCSI LLDs | expand |
> > Only call scsi_set_resid() in case of an underflow. Do not call > scsi_set_resid() in case of an overflow. > > Cc: Avri Altman <avri.altman@wdc.com> > Cc: Adrian Hunter <adrian.hunter@intel.com> > Fixes: cb38845d90fc ("scsi: ufs: core: Set the residual byte count") > Signed-off-by: Bart Van Assche <bvanassche@acm.org> > --- > drivers/ufs/core/ufshcd.c | 19 ++++++++++++++++--- > include/ufs/ufs.h | 6 ++++++ > 2 files changed, 22 insertions(+), 3 deletions(-) > > diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c index > e31242fe0518..b27372f9b488 100644 > --- a/drivers/ufs/core/ufshcd.c > +++ b/drivers/ufs/core/ufshcd.c > @@ -5241,12 +5241,25 @@ static inline int ufshcd_transfer_rsp_status(struct > ufs_hba *hba, struct ufshcd_lrb *lrbp, > struct cq_entry *cqe) { > + bool overflow, underflow; > + u8 upiu_flags; > int result = 0; > int scsi_status; > enum utp_ocs ocs; > - > - scsi_set_resid(lrbp->cmd, > - be32_to_cpu(lrbp->ucd_rsp_ptr->sr.residual_transfer_count)); > + u32 resid; > + > + upiu_flags = be32_to_cpu(lrbp->ucd_rsp_ptr->header.dword_0) >> 16; > + overflow = upiu_flags & UPIU_RSP_FLAG_OVERFLOW; > + underflow = upiu_flags & UPIU_RSP_FLAG_UNDERFLOW; > + resid = be32_to_cpu(lrbp->ucd_rsp_ptr->sr.residual_transfer_count); > + WARN_ON_ONCE(overflow && underflow); > + WARN_ON_ONCE(!overflow && !underflow && resid); Do we really need to debug the hw? - see Table 10.16 (3.1 spec). Thanks, Avri > + /* > + * Test !overflow instead of underflow to support UFS devices that do > + * not set either flag. > + */ > + if (!overflow) > + scsi_set_resid(lrbp->cmd, resid); > > /* overall command status of utrd */ > ocs = ufshcd_get_tr_ocs(lrbp, cqe); diff --git a/include/ufs/ufs.h > b/include/ufs/ufs.h index 8316e2408ac3..bee5ccc6e7ce 100644 > --- a/include/ufs/ufs.h > +++ b/include/ufs/ufs.h > @@ -104,6 +104,12 @@ enum { > UPIU_CMD_FLAGS_READ = 0x40, > }; > > +/* UPIU response flags */ > +enum { > + UPIU_RSP_FLAG_UNDERFLOW = 0x20, > + UPIU_RSP_FLAG_OVERFLOW = 0x40, > +}; > + > /* UPIU Task Attributes */ > enum { > UPIU_TASK_ATTR_SIMPLE = 0x00,
On 7/22/23 22:31, Avri Altman wrote: >> >> + WARN_ON_ONCE(overflow && underflow); >> + WARN_ON_ONCE(!overflow && !underflow && resid); > Do we really need to debug the hw? - see Table 10.16 (3.1 spec). I was wondering about this too while writing this patch. If nobody objects I will leave the above two WARN_ON_ONCE() statements out when reposting this patch. Thanks, Bart.
On 24/07/23 19:02, Bart Van Assche wrote: > On 7/22/23 22:31, Avri Altman wrote: >>> >>> + WARN_ON_ONCE(overflow && underflow); >>> + WARN_ON_ONCE(!overflow && !underflow && resid); >> Do we really need to debug the hw? - see Table 10.16 (3.1 spec). > > I was wondering about this too while writing this patch. If nobody objects I > will leave the above two WARN_ON_ONCE() statements out when reposting this patch. I agree. Please do leave them out.
diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c index e31242fe0518..b27372f9b488 100644 --- a/drivers/ufs/core/ufshcd.c +++ b/drivers/ufs/core/ufshcd.c @@ -5241,12 +5241,25 @@ static inline int ufshcd_transfer_rsp_status(struct ufs_hba *hba, struct ufshcd_lrb *lrbp, struct cq_entry *cqe) { + bool overflow, underflow; + u8 upiu_flags; int result = 0; int scsi_status; enum utp_ocs ocs; - - scsi_set_resid(lrbp->cmd, - be32_to_cpu(lrbp->ucd_rsp_ptr->sr.residual_transfer_count)); + u32 resid; + + upiu_flags = be32_to_cpu(lrbp->ucd_rsp_ptr->header.dword_0) >> 16; + overflow = upiu_flags & UPIU_RSP_FLAG_OVERFLOW; + underflow = upiu_flags & UPIU_RSP_FLAG_UNDERFLOW; + resid = be32_to_cpu(lrbp->ucd_rsp_ptr->sr.residual_transfer_count); + WARN_ON_ONCE(overflow && underflow); + WARN_ON_ONCE(!overflow && !underflow && resid); + /* + * Test !overflow instead of underflow to support UFS devices that do + * not set either flag. + */ + if (!overflow) + scsi_set_resid(lrbp->cmd, resid); /* overall command status of utrd */ ocs = ufshcd_get_tr_ocs(lrbp, cqe); diff --git a/include/ufs/ufs.h b/include/ufs/ufs.h index 8316e2408ac3..bee5ccc6e7ce 100644 --- a/include/ufs/ufs.h +++ b/include/ufs/ufs.h @@ -104,6 +104,12 @@ enum { UPIU_CMD_FLAGS_READ = 0x40, }; +/* UPIU response flags */ +enum { + UPIU_RSP_FLAG_UNDERFLOW = 0x20, + UPIU_RSP_FLAG_OVERFLOW = 0x40, +}; + /* UPIU Task Attributes */ enum { UPIU_TASK_ATTR_SIMPLE = 0x00,
Only call scsi_set_resid() in case of an underflow. Do not call scsi_set_resid() in case of an overflow. Cc: Avri Altman <avri.altman@wdc.com> Cc: Adrian Hunter <adrian.hunter@intel.com> Fixes: cb38845d90fc ("scsi: ufs: core: Set the residual byte count") Signed-off-by: Bart Van Assche <bvanassche@acm.org> --- drivers/ufs/core/ufshcd.c | 19 ++++++++++++++++--- include/ufs/ufs.h | 6 ++++++ 2 files changed, 22 insertions(+), 3 deletions(-)