Re: [PATCH v1 1/2] scsi: ufs: Abort tasks before clear them from doorbell

Bean Huo
2020-08-30T18:12:04Z

Hi Can
This patch conflicts and be not in line with this series patches : h
ttps://patchwork.kernel.org/cover/11709279/, which has been applied
into 5.9/scsi-fixes. But they are not apppiled in the 5.9/scsi-queue.

Maybe you should rebase your patch from scsi-fixes branch. or do you
have another better option?

Thanks,
Bean

On Mon, 2020-08-24 at 19:07 -0700, Can Guo wrote:
> To recovery non-fatal errors, no full reset is required, err_handler
> only
> clears those pending TRs/TMRs so that scsi layer can re-issue them.
> In
> current err_handler, TRs are directly cleared from UFS host's
> doorbell but
> not aborted from device side. However, according to the UFSHCI JEDEC
> spec,
> the host software shall use UTP Transfer Request List CLear Register
> to
> clear a task from UFS host's doorbell only when a UTP Transfer
> Request is
> expected to not be completed, e.g. when the host software receives a
> “FUNCTION COMPLETE” Task Management response which means a Transfer
> Request
> was aborted. To follow the UFSHCI JEDEC spec, in err_handler, aborts
> one TR
> before clearing it from doorbell.
>
> Signed-off-by: Can Guo <cang@codeaurora.org>
> ---
> drivers/scsi/ufs/ufshcd.c | 143 ++++++++++++++++++++++++++--------
> ------------
> 1 file changed, 81 insertions(+), 62 deletions(-)
>
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index b8441ad..000895f 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -235,6 +235,7 @@ static int ufshcd_setup_hba_vreg(struct ufs_hba
> *hba, bool on);
> static int ufshcd_setup_vreg(struct ufs_hba *hba, bool on);
> static inline int ufshcd_config_vreg_hpm(struct ufs_hba *hba,
> struct ufs_vreg *vreg);
> +static int ufshcd_try_to_abort_task(struct ufs_hba *hba, int tag);
> static int ufshcd_wb_buf_flush_enable(struct ufs_hba *hba);
> static int ufshcd_wb_buf_flush_disable(struct ufs_hba *hba);
> static int ufshcd_wb_ctrl(struct ufs_hba *hba, bool enable);
> @@ -5657,8 +5658,8 @@ static void ufshcd_err_handler(struct
> work_struct *work)
> {
> struct ufs_hba *hba;
> unsigned long flags;
> - u32 err_xfer = 0;
> - u32 err_tm = 0;
> + bool err_xfer = false;
> + bool err_tm = false;
> int err = 0;
> int tag;
> bool needs_reset = false;
> @@ -5734,7 +5735,7 @@ static void ufshcd_err_handler(struct
> work_struct *work)
> spin_unlock_irqrestore(hba->host->host_lock, flags);
> /* Clear pending transfer requests */
> for_each_set_bit(tag, &hba->outstanding_reqs, hba->nutrs) {
> - if (ufshcd_clear_cmd(hba, tag)) {
> + if (ufshcd_try_to_abort_task(hba, tag)) {
> err_xfer = true;
> goto lock_skip_pending_xfer_clear;
> }
> @@ -6486,7 +6487,7 @@ static void ufshcd_set_req_abort_skip(struct
> ufs_hba *hba, unsigned long bitmap)
> }
>
> /**
> - * ufshcd_abort - abort a specific command
> + * ufshcd_try_to_abort_task - abort a specific task
> * @cmd: SCSI command pointer
> *
> * Abort the pending command in device by sending UFS_ABORT_TASK
> task management
> @@ -6495,6 +6496,80 @@ static void ufshcd_set_req_abort_skip(struct
> ufs_hba *hba, unsigned long bitmap)
> * issued. To avoid that, first issue UFS_QUERY_TASK to check if the
> command is
> * really issued and then try to abort it.
> *
> + * Returns zero on success, non-zero on failure
> + */
> +static int ufshcd_try_to_abort_task(struct ufs_hba *hba, int tag)
> +{
> + struct ufshcd_lrb *lrbp = &hba->lrb[tag];
> + int err = 0;
> + int poll_cnt;
> + u8 resp = 0xF;
> + u32 reg;
> +
> + for (poll_cnt = 100; poll_cnt; poll_cnt--) {
> + err = ufshcd_issue_tm_cmd(hba, lrbp->lun, lrbp-
> >task_tag,
> + UFS_QUERY_TASK, &resp);
> + if (!err && resp ==
> UPIU_TASK_MANAGEMENT_FUNC_SUCCEEDED) {
> + /* cmd pending in the device */
> + dev_err(hba->dev, "%s: cmd pending in the
> device. tag = %d\n",
> + __func__, tag);
> + break;
> + } else if (!err && resp ==
> UPIU_TASK_MANAGEMENT_FUNC_COMPL) {
> + /*
> + * cmd not pending in the device, check if it
> is
> + * in transition.
> + */
> + dev_err(hba->dev, "%s: cmd at tag %d not
> pending in the device.\n",
> + __func__, tag);
> + reg = ufshcd_readl(hba,
> REG_UTP_TRANSFER_REQ_DOOR_BELL);
> + if (reg & (1 << tag)) {
> + /* sleep for max. 200us to stabilize */
> + usleep_range(100, 200);
> + continue;
> + }
> + /* command completed already */
> + dev_err(hba->dev, "%s: cmd at tag %d
> successfully cleared from DB.\n",
> + __func__, tag);
> + goto out;
> + } else {
> + dev_err(hba->dev,
> + "%s: no response from device. tag = %d,
> err %d\n",
> + __func__, tag, err);
> + if (!err)
> + err = resp; /* service response error
> */
> + goto out;
> + }
> + }
> +
> + if (!poll_cnt) {
> + err = -EBUSY;
> + goto out;
> + }
> +
> + err = ufshcd_issue_tm_cmd(hba, lrbp->lun, lrbp->task_tag,
> + UFS_ABORT_TASK, &resp);
> + if (err || resp != UPIU_TASK_MANAGEMENT_FUNC_COMPL) {
> + if (!err) {
> + err = resp; /* service response error */
> + dev_err(hba->dev, "%s: issued. tag = %d, err
> %d\n",
> + __func__, tag, err);
> + }
> + goto out;
> + }
> +
> + err = ufshcd_clear_cmd(hba, tag);
> + if (err)
> + dev_err(hba->dev, "%s: Failed clearing cmd at tag %d,
> err %d\n",
> + __func__, tag, err);
> +
> +out:
> + return err;
> +}
> +
> +/**
> + * ufshcd_abort - scsi host template eh_abort_handler callback
> + * @cmd: SCSI command pointer
> + *
> * Returns SUCCESS/FAILED
> */
> static int ufshcd_abort(struct scsi_cmnd *cmd)
> @@ -6504,8 +6579,6 @@ static int ufshcd_abort(struct scsi_cmnd *cmd)
> unsigned long flags;
> unsigned int tag;
> int err = 0;
> - int poll_cnt;
> - u8 resp = 0xF;
> struct ufshcd_lrb *lrbp;
> u32 reg;
>
> @@ -6574,63 +6647,9 @@ static int ufshcd_abort(struct scsi_cmnd *cmd)
> goto out;
> }
>
> - for (poll_cnt = 100; poll_cnt; poll_cnt--) {
> - err = ufshcd_issue_tm_cmd(hba, lrbp->lun, lrbp-
> >task_tag,
> - UFS_QUERY_TASK, &resp);
> - if (!err && resp ==
> UPIU_TASK_MANAGEMENT_FUNC_SUCCEEDED) {
> - /* cmd pending in the device */
> - dev_err(hba->dev, "%s: cmd pending in the
> device. tag = %d\n",
> - __func__, tag);
> - break;
> - } else if (!err && resp ==
> UPIU_TASK_MANAGEMENT_FUNC_COMPL) {
> - /*
> - * cmd not pending in the device, check if it
> is
> - * in transition.
> - */
> - dev_err(hba->dev, "%s: cmd at tag %d not
> pending in the device.\n",
> - __func__, tag);
> - reg = ufshcd_readl(hba,
> REG_UTP_TRANSFER_REQ_DOOR_BELL);
> - if (reg & (1 << tag)) {
> - /* sleep for max. 200us to stabilize */
> - usleep_range(100, 200);
> - continue;
> - }
> - /* command completed already */
> - dev_err(hba->dev, "%s: cmd at tag %d
> successfully cleared from DB.\n",
> - __func__, tag);
> - goto out;
> - } else {
> - dev_err(hba->dev,
> - "%s: no response from device. tag = %d,
> err %d\n",
> - __func__, tag, err);
> - if (!err)
> - err = resp; /* service response error
> */
> - goto out;
> - }
> - }
> -
> - if (!poll_cnt) {
> - err = -EBUSY;
> - goto out;
> - }
> -
> - err = ufshcd_issue_tm_cmd(hba, lrbp->lun, lrbp->task_tag,
> - UFS_ABORT_TASK, &resp);
> - if (err || resp != UPIU_TASK_MANAGEMENT_FUNC_COMPL) {
> - if (!err) {
> - err = resp; /* service response error */
> - dev_err(hba->dev, "%s: issued. tag = %d, err
> %d\n",
> - __func__, tag, err);
> - }
> - goto out;
> - }
> -
> - err = ufshcd_clear_cmd(hba, tag);
> - if (err) {
> - dev_err(hba->dev, "%s: Failed clearing cmd at tag %d,
> err %d\n",
> - __func__, tag, err);
> + err = ufshcd_try_to_abort_task(hba, tag);
> + if (err)
> goto out;
> - }
>
> spin_lock_irqsave(host->host_lock, flags);
> __ufshcd_transfer_req_compl(hba, (1UL << tag));