Hi Bean,
On Mon, 2020-08-10 at 17:41 +0200, Bean Huo wrote:
> On Thu, 2020-08-06 at 18:07 +0800, Can Guo wrote:
> > Hi Bean,
> >
> > On 2020-08-06 17:50, Bean Huo wrote:
> > > >
> > > > Please check Stanley's recent change to ufshcd_abort, you may
> > > > want to rebase your change on his and do goto cleanup here.
> > > > @Stanley correct me if I am wrong.
> > > >
> > > > But even if you do a goto cleanup here, we still lost the
> > > > chances to dump host infos/regs like it does in the old code.
> > > > If a cmd was completed but without a notifying intr, this is
> > > > kind of a problem that we/host should look into, because it's
> > > > pasted at least 30 sec since the cmd was sent, so those dumps
> > > > are necessary to debug the problem. How about moving blow prints
> > > > in front of this part?
> > > >
> > > > Thanks,
> > > >
> > > > Can Guo.
> > > >
> > > > > }
> > > > >
> > > > > /* Print Transfer Request of aborted task */
> > >
> > > Hi Can
> > >
> > > Thanks, do you mean that change to like this:
> > >
> > >
> > > Author: Bean Huo <beanhuo@xxxxxxxxxx>
> > > Date: Thu Aug 6 11:34:45 2020 +0200
> > >
> > > scsi: ufs: no need to send one Abort Task TM in case the task
> > > in
> > > was cleared
> > >
> > > If the bit corresponds to a task in the Doorbell register has
> > > been
> > > cleared, no need to poll the status of the task on the device
> > > side
> > > and to send an Abort Task TM.
> > > This patch also deletes dispensable dev_err() in case of the
> > > task
> > > already completed.
> > >
> > > Signed-off-by: Bean Huo <beanhuo@xxxxxxxxxx>
> > >
> > > diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> > > index 307622284239..f7c91ce9e294 100644
> > > --- a/drivers/scsi/ufs/ufshcd.c
> > > +++ b/drivers/scsi/ufs/ufshcd.c
> > > @@ -6425,23 +6425,9 @@ static int ufshcd_abort(struct scsi_cmnd
> > > *cmd)
> > > return ufshcd_eh_host_reset_handler(cmd);
> > >
> > > ufshcd_hold(hba, false);
> > > - reg = ufshcd_readl(hba, REG_UTP_TRANSFER_REQ_DOOR_BELL);
> > > /* If command is already aborted/completed, return SUCCESS
> > > */
> > > - if (!(test_bit(tag, &hba->outstanding_reqs))) {
> > > - dev_err(hba->dev,
> > > - "%s: cmd at tag %d already completed,
> > > outstanding=0x%lx, doorbell=0x%x\n",
> > > - __func__, tag, hba->outstanding_reqs, reg);
> > > + if (!(test_bit(tag, &hba->outstanding_reqs)))
> > > goto out;
> > > - }
> > > -
> > > - if (!(reg & (1 << tag))) {
> > > - dev_err(hba->dev,
> > > - "%s: cmd was completed, but without a notifying
> > > intr,
> > > tag = %d",
> > > - __func__, tag);
> > > - }
> > > -
> > > - /* Print Transfer Request of aborted task */
> > > - dev_err(hba->dev, "%s: Device abort task at tag %d\n",
> > > __func__, tag);
> > >
> > > /*
> > > * Print detailed info about aborted request.
> > > @@ -6462,6 +6448,17 @@ static int ufshcd_abort(struct scsi_cmnd
> > > *cmd)
> > > }
> > > hba->req_abort_count++;
> > >
> > > + reg = ufshcd_readl(hba, REG_UTP_TRANSFER_REQ_DOOR_BELL);
> > > + if (!(reg & (1 << tag))) {
> > > + dev_err(hba->dev,
> > > + "%s: cmd was completed, but without a notifying
> > > intr,
> > > tag = %d",
> > > + __func__, tag);
> > > + goto cleanup;
> > > + }
> > > +
> > > + /* Print Transfer Request of aborted task */
> > > + dev_err(hba->dev, "%s: Device abort task at tag %d\n",
> > > __func__, tag);
> > > +
> >
> > The rest looks good but let below two lines stay where they were.
> >
> > /* Print Transfer Request of aborted task */
> > dev_err(hba->dev, "%s: Device abort task at tag %d\n",
> > __func__, tag);
> >
> >
> > Thanks,
> >
> > Can Guo.
> >
> Hi Can
> I will change it in the next version.
>
>
> Hi Stanly
> would you mind I take your patch into my next version patchset? Since
> we both will add a new same goto label. I will keep your patch
> authorship.
Sure, OK to me : )
Thanks,
Stanley Chu
